fix: show merged PR for current branch head (#5877)
This commit is contained in:
parent
14eb4091fd
commit
89eb4b3f55
|
|
@ -635,7 +635,7 @@ describe('getPRForBranch', () => {
|
|||
})
|
||||
})
|
||||
|
||||
it('ignores merged PRs discovered only by branch lookup', async () => {
|
||||
it('ignores merged PRs discovered only by branch lookup when the branch moved on', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
|
|
@ -670,12 +670,69 @@ describe('getPRForBranch', () => {
|
|||
headRefOid: 'head-oid'
|
||||
})
|
||||
})
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: 'new-local-head-oid\n',
|
||||
stderr: ''
|
||||
})
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'add-guide-for-mobile-emulator-use')
|
||||
|
||||
expect(pr).toBeNull()
|
||||
})
|
||||
|
||||
it('shows a merged branch PR when it still matches the current HEAD', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
number: 5875,
|
||||
title: 'Merged current branch PR',
|
||||
state: 'closed',
|
||||
merged_at: '2026-06-20T04:53:05Z',
|
||||
html_url: 'https://github.com/acme/widgets/pull/5875',
|
||||
updated_at: '2026-06-20T04:53:05Z',
|
||||
draft: false,
|
||||
mergeable_state: 'clean',
|
||||
head: { ref: 'fix-tab-strip-layout-test', sha: 'current-head-oid' },
|
||||
base: { ref: 'main', sha: 'base-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
number: 5875,
|
||||
title: 'Merged current branch PR',
|
||||
state: 'MERGED',
|
||||
url: 'https://github.com/acme/widgets/pull/5875',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-06-20T04:53:05Z',
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE',
|
||||
baseRefName: 'main',
|
||||
headRefName: 'fix-tab-strip-layout-test',
|
||||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'current-head-oid'
|
||||
})
|
||||
})
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: 'current-head-oid\n',
|
||||
stderr: ''
|
||||
})
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'fix-tab-strip-layout-test')
|
||||
|
||||
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['rev-parse', 'HEAD'], {
|
||||
cwd: '/repo-root'
|
||||
})
|
||||
expect(pr).toMatchObject({
|
||||
number: 5875,
|
||||
title: 'Merged current branch PR',
|
||||
state: 'merged',
|
||||
headSha: 'current-head-oid'
|
||||
})
|
||||
})
|
||||
|
||||
it('prefers branch lookup over a fallback PR number', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
|
|
|
|||
|
|
@ -2036,6 +2036,38 @@ function isMergedImplicitPR(data: PullRequestLookupData, linkedPRNumber?: number
|
|||
return typeof linkedPRNumber !== 'number' && mapPRState(data.state, data.isDraft) === 'merged'
|
||||
}
|
||||
|
||||
async function getCurrentHeadOid(
|
||||
repoPath: string,
|
||||
connectionId?: string | null,
|
||||
localGitOptions: { wslDistro?: string } = {}
|
||||
): Promise<string | null> {
|
||||
try {
|
||||
const provider = connectionId ? getSshGitProvider(connectionId) : null
|
||||
const result = provider
|
||||
? await provider.exec(['rev-parse', 'HEAD'], repoPath)
|
||||
: await gitExecFileAsync(['rev-parse', 'HEAD'], {
|
||||
cwd: repoPath,
|
||||
...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {})
|
||||
})
|
||||
return result.stdout.trim() || null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function shouldHideMergedImplicitPR(
|
||||
data: PullRequestLookupData | null,
|
||||
linkedPRNumber: number | null | undefined,
|
||||
currentHeadOid: string | null
|
||||
): boolean {
|
||||
if (!data || !isMergedImplicitPR(data, linkedPRNumber)) {
|
||||
return false
|
||||
}
|
||||
// Why: keep hiding historical merged branch matches, but preserve the merged
|
||||
// PR for the exact commit currently checked out in the sidebar.
|
||||
return !currentHeadOid || data.headRefOid !== currentHeadOid
|
||||
}
|
||||
|
||||
function normalizePullRequestLookupData(data: PullRequestLookupData): PullRequestLookupData {
|
||||
return {
|
||||
...data,
|
||||
|
|
@ -2502,6 +2534,19 @@ export async function getPRForBranchOutcome(
|
|||
let data: PullRequestLookupData | null = null
|
||||
let dataRepo: OwnerRepo | null = null
|
||||
let dataHeadRepo: OwnerRepo | null = headRepo
|
||||
let currentHeadOidForMergedImplicit: string | null | undefined
|
||||
|
||||
const hideMergedImplicitPR = async (candidate: PullRequestLookupData | null) => {
|
||||
if (!candidate || !isMergedImplicitPR(candidate, linkedPRNumber)) {
|
||||
return false
|
||||
}
|
||||
currentHeadOidForMergedImplicit ??= await getCurrentHeadOid(
|
||||
repoPath,
|
||||
connectionId,
|
||||
localGitOptions
|
||||
)
|
||||
return shouldHideMergedImplicitPR(candidate, linkedPRNumber, currentHeadOidForMergedImplicit)
|
||||
}
|
||||
|
||||
if (typeof linkedPRNumber === 'number') {
|
||||
const exactLookup = await lookupPRByNumber({
|
||||
|
|
@ -2554,8 +2599,8 @@ export async function getPRForBranchOutcome(
|
|||
}
|
||||
}
|
||||
let mergedBranchLookupNumber: number | null = null
|
||||
if (data && isMergedImplicitPR(data, linkedPRNumber)) {
|
||||
mergedBranchLookupNumber = data.number
|
||||
if (await hideMergedImplicitPR(data)) {
|
||||
mergedBranchLookupNumber = data?.number ?? null
|
||||
data = null
|
||||
dataRepo = null
|
||||
dataHeadRepo = headRepo
|
||||
|
|
@ -2580,7 +2625,7 @@ export async function getPRForBranchOutcome(
|
|||
// marks the fallback as visible review state, keep its lifecycle fresh even
|
||||
// if GitHub no longer reports it by branch (for example deleted heads).
|
||||
if (
|
||||
isMergedImplicitPR(data, linkedPRNumber) &&
|
||||
(await hideMergedImplicitPR(data)) &&
|
||||
!fallbackConfirmedMergedBranch &&
|
||||
options.acceptMergedFallbackPR !== true
|
||||
) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue