diff --git a/packages/ui/src/stores/useGitHubPrStatusStore.test.ts b/packages/ui/src/stores/useGitHubPrStatusStore.test.ts index f8affe7c..351cf765 100644 --- a/packages/ui/src/stores/useGitHubPrStatusStore.test.ts +++ b/packages/ui/src/stores/useGitHubPrStatusStore.test.ts @@ -440,4 +440,104 @@ describe("GitHub PR status stale terminal associations", () => { expect(useGitHubPrStatusStore.getState().entries[key]?.status?.pr?.number).toBe(12) expect(useGitHubPrStatusStore.getState().entries[key]?.error).toBe("GitHub unavailable") }) + + test("does not persist a merged branch association", () => { + const merged: GitHubPullRequestStatus = { + connected: true, + fetchedAt: 1_000, + pr: { number: 12, title: "old", url: "u12", state: "merged", draft: false, base: "main", head: "feature" }, + } + const key = getGitHubPrStatusKey("/repo", "feature", "origin") + useGitHubPrStatusStore.getState().ensureEntry(key) + useGitHubPrStatusStore.getState().setParams(key, params({} as RuntimeAPIs["github"], "feature")) + useGitHubPrStatusStore.setState((state) => ({ + entries: { + ...state.entries, + [key]: { + ...state.entries[key]!, + status: merged, + isInitialStatusResolved: true, + lastRefreshAt: Date.now(), + }, + }, + })) + + const persisted = useGitHubPrStatusStore.persist.getOptions().partialize?.( + useGitHubPrStatusStore.getState(), + ) as { entries?: Record } | undefined + expect(persisted?.entries?.[key]).toBeUndefined() + }) + + test("still persists an open branch association", () => { + const open: GitHubPullRequestStatus = { + connected: true, + fetchedAt: 1_000, + pr: { number: 15, title: "new", url: "u15", state: "open", draft: false, base: "main", head: "feature" }, + } + const key = getGitHubPrStatusKey("/repo", "feature", "origin") + useGitHubPrStatusStore.getState().ensureEntry(key) + useGitHubPrStatusStore.getState().setParams(key, params({} as RuntimeAPIs["github"], "feature")) + useGitHubPrStatusStore.setState((state) => ({ + entries: { + ...state.entries, + [key]: { + ...state.entries[key]!, + status: open, + isInitialStatusResolved: true, + lastRefreshAt: Date.now(), + }, + }, + })) + + const persisted = useGitHubPrStatusStore.persist.getOptions().partialize?.( + useGitHubPrStatusStore.getState(), + ) as { entries?: Record } | undefined + expect(persisted?.entries?.[key]?.status?.pr?.number).toBe(15) + }) + + test("hydrate strips a legacy persisted merged PR and marks it unresolved", () => { + const key = getGitHubPrStatusKey("/repo", "feature", "origin") + const hydrated = useGitHubPrStatusStore.persist.getOptions().merge?.( + { + entries: { + [key]: { + status: { + connected: true, + fetchedAt: 1_000, + repo: { owner: "acme", repo: "app", url: "https://github.com/acme/app" }, + pr: { number: 12, title: "old", url: "u12", state: "merged", draft: false, base: "main", head: "feature" }, + checks: { state: "success", total: 1, success: 1, failure: 0, pending: 0 }, + canMerge: true, + }, + isInitialStatusResolved: true, + lastRefreshAt: Date.now(), + lastDiscoveryPollAt: 0, + identity: { + runtimeKey: "runtime-a", + directory: "/repo", + branch: "feature", + remoteName: "origin", + }, + resolvedRemoteName: "origin", + }, + }, + }, + useGitHubPrStatusStore.getState(), + ) as { + entries: Record + } + + expect(hydrated.entries[key]?.status?.pr).toBeNull() + expect(hydrated.entries[key]?.status?.repo).toEqual({ + owner: "acme", + repo: "app", + url: "https://github.com/acme/app", + }) + expect(hydrated.entries[key]?.status?.checks).toBeUndefined() + expect(hydrated.entries[key]?.status?.canMerge).toBeUndefined() + expect(hydrated.entries[key]?.isInitialStatusResolved).toBe(false) + }) }) diff --git a/packages/web/server/lib/github/pr-status.js b/packages/web/server/lib/github/pr-status.js index 523f12d6..f31cb30c 100644 --- a/packages/web/server/lib/github/pr-status.js +++ b/packages/web/server/lib/github/pr-status.js @@ -514,14 +514,11 @@ const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates, // Branch status associates the current head with an open PR only. Returning a // closed/merged PR here made the client cache a terminal status that could not - // self-heal until a manual forced refresh. - const state = 'open'; - // Shared per-repo list first: one pulls.list answers every branch of the - // repo within the TTL. A miss in a complete list is authoritative — skip - // the per-branch query fan entirely. + // self-heal until a manual forced refresh. A miss also lets the next repo + // target run, so an open upstream PR wins over a merged fork PR. let listWasComplete = false; try { - const listEntry = await getRepoPulls(octokit, target.repo, state, { force }); + const listEntry = await getRepoPulls(octokit, target.repo, 'open', { force }); const fromList = pickPreferred(listEntry.prs); if (fromList) { return fromList; @@ -539,7 +536,7 @@ const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates, const directCandidates = await safeListPulls(octokit, { owner: target.repo.owner, repo: target.repo.repo, - state, + state: 'open', head: `${owner}:${branch}`, per_page: 100, }); diff --git a/packages/web/server/lib/github/pr-status.test.js b/packages/web/server/lib/github/pr-status.test.js index 14bc8bfb..a481deca 100644 --- a/packages/web/server/lib/github/pr-status.test.js +++ b/packages/web/server/lib/github/pr-status.test.js @@ -86,4 +86,20 @@ describe('findFirstMatchingPr open-only branch status', () => { expect(listMock.mock.calls.every((call) => call[0]?.state === 'open')).toBe(true); expect(listMock.mock.calls.some((call) => call[0]?.state === 'closed')).toBe(false); }); + + test('does not query closed PRs when the open list is complete and empty', async () => { + listMock.mockImplementation(async () => ({ data: [] })); + + const pr = await findFirstMatchingPr({ + octokit: { rest: { pulls: { list: listMock } } }, + target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' }, + branch: 'feature', + sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }], + force: true, + }); + + expect(pr).toBeNull(); + expect(listMock.mock.calls).toHaveLength(1); + expect(listMock.mock.calls[0]?.[0]?.state).toBe('open'); + }); });