From ec8d4704d0cb707ab8fd9a2f1a1ed60c83293b94 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Thu, 26 Mar 2026 09:55:52 -0700 Subject: [PATCH] fix(desktop): avoid stale historical PR matches --- .../workspaces/utils/github/github.test.ts | 48 +++++++++++++++++++ .../workspaces/utils/github/pr-resolution.ts | 41 ++++++++++++++-- 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts index 56ffbe24169..a8794a67daa 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts @@ -10,6 +10,7 @@ import { branchMatchesPR, getPRHeadBranchCandidates, prMatchesLocalBranch, + shouldAcceptPRMatch, } from "./pr-resolution"; import { getPullRequestRepoArgs, @@ -407,6 +408,53 @@ describe("prMatchesLocalBranch", () => { }); }); +describe("shouldAcceptPRMatch", () => { + test("keeps open PR matches even when local HEAD differs", () => { + expect( + shouldAcceptPRMatch({ + localBranch: "feature/my-thing", + headSha: "local-head-sha", + pr: { + headRefName: "feature/my-thing", + headRefOid: "remote-head-sha", + headRepositoryOwner: null, + state: "OPEN", + }, + }), + ).toBe(true); + }); + + test("rejects historical PR matches when the head commit differs", () => { + expect( + shouldAcceptPRMatch({ + localBranch: "feature/my-thing", + headSha: "new-head-sha", + pr: { + headRefName: "feature/my-thing", + headRefOid: "old-pr-head-sha", + headRepositoryOwner: null, + state: "MERGED", + }, + }), + ).toBe(false); + }); + + test("accepts historical PR matches when the head commit still matches", () => { + expect( + shouldAcceptPRMatch({ + localBranch: "feature/my-thing", + headSha: "same-head-sha", + pr: { + headRefName: "feature/my-thing", + headRefOid: "same-head-sha", + headRepositoryOwner: null, + state: "MERGED", + }, + }), + ).toBe(true); + }); +}); + describe("resolveRemoteBranchNameForGitHubStatus", () => { test("prefers the tracked upstream branch name", () => { expect( diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts index 9366bd3c689..90f09953237 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts @@ -17,7 +17,11 @@ export async function getPRForBranch( repoContext?: RepoContext, headSha?: string, ): Promise { - const byTracking = await getPRByBranchTracking(worktreePath, localBranch); + const byTracking = await getPRByBranchTracking( + worktreePath, + localBranch, + headSha, + ); if (byTracking) { return byTracking; } @@ -90,6 +94,36 @@ export function prMatchesLocalBranch( return pr.headRepositoryOwner?.login?.toLowerCase() === ownerPrefix; } +function isHistoricalPullRequestState(state: GHPRResponse["state"]): boolean { + return state === "CLOSED" || state === "MERGED"; +} + +export function shouldAcceptPRMatch({ + localBranch, + pr, + headSha, +}: { + localBranch: string; + pr: Pick< + GHPRResponse, + "headRefName" | "headRefOid" | "headRepositoryOwner" | "state" + >; + headSha?: string; +}): boolean { + if (!prMatchesLocalBranch(localBranch, pr)) { + return false; + } + + // Historical PRs should only attach when this workspace still points at the + // exact PR head commit. Otherwise, reusing a branch name can surface an old, + // unrelated closed or merged PR. + if (headSha && isHistoricalPullRequestState(pr.state)) { + return pr.headRefOid === headSha; + } + + return true; +} + function sortPRCandidates( candidates: GHPRResponse[], headSha?: string, @@ -133,6 +167,7 @@ function sortPRCandidates( async function getPRByBranchTracking( worktreePath: string, localBranch: string, + headSha?: string, ): Promise { try { const { stdout } = await execWithShellEnv( @@ -150,7 +185,7 @@ async function getPRByBranchTracking( // `gh pr view` can match via stale tracking refs (e.g. refs/pull/N/head) // left over from a previous `gh pr checkout`, causing a new workspace // to incorrectly show an old, unrelated PR. - if (!prMatchesLocalBranch(localBranch, data)) { + if (!shouldAcceptPRMatch({ localBranch, pr: data, headSha })) { return null; } @@ -199,7 +234,7 @@ async function findPRByHeadBranch( ); for (const candidate of parsePRListResponse(stdout)) { - if (prMatchesLocalBranch(localBranch, candidate)) { + if (shouldAcceptPRMatch({ localBranch, pr: candidate, headSha })) { matches.set(candidate.number, candidate); } }