From 89eb4b3f5559d36d04c0764391a3f5660f3e08d2 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 20 Jun 2026 17:56:38 -0700 Subject: [PATCH] fix: show merged PR for current branch head (#5877) --- src/main/github/client.test.ts | 59 +++++++++++++++++++++++++++++++++- src/main/github/client.ts | 51 +++++++++++++++++++++++++++-- 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index 1ef156e4e..8493317a4 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -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 diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 3380a6a5a..6bf0976ba 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -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 { + 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 ) {