diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index eb7943257..2138ba0aa 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -801,7 +801,7 @@ describe('getPRForBranch', () => { expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( 3, - ['api', 'repos/acme/widgets/commits/bbbb2222bbbb2222/pulls?per_page=100'], + ['api', 'repos/acme/widgets/commits/bbbb2222bbbb2222/pulls?per_page=100&page=1'], expect.anything() ) expect(pr).toMatchObject({ @@ -929,6 +929,209 @@ describe('getPRForBranch', () => { }) }) + function mockMergedLinkedPRLookup(prNumber = 7447) { + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ + candidates: [{ owner: 'acme', repo: 'widgets' }], + headRepo: { owner: 'acme', repo: 'widgets' } + }) + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: prNumber, + title: 'Merged linked PR', + state: 'MERGED', + url: `https://github.com/acme/widgets/pull/${prNumber}`, + statusCheckRollup: [], + updatedAt: '2026-07-03T21:27:36Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'old-linked-branch', + baseRefOid: 'base-oid', + headRefOid: 'aaaa1111aaaa1111' + }) + }) + } + + it('stamps confirmedContainedHeadOid for a linked merged PR when HEAD is its commit', async () => { + mockMergedLinkedPRLookup() + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify([{ number: 7447 }]) + }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null, { + currentHeadOid: 'bbbb2222bbbb2222' + }) + + expect(outcome).toMatchObject({ + kind: 'found', + pr: { + number: 7447, + state: 'merged', + confirmedContainedHeadOid: 'bbbb2222bbbb2222' + } + }) + expect(outcome.kind === 'found' ? outcome.pr.headDivergedFromMergedPRAtOid : undefined).toBe( + undefined + ) + }) + + it('stamps headDivergedFromMergedPRAtOid for a linked merged PR with a definite miss', async () => { + mockMergedLinkedPRLookup() + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify([{ number: 42 }]) + }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null, { + currentHeadOid: 'bbbb2222bbbb2222' + }) + + expect(outcome).toMatchObject({ + kind: 'found', + pr: { + number: 7447, + state: 'merged', + headDivergedFromMergedPRAtOid: 'bbbb2222bbbb2222' + } + }) + }) + + it('stamps linked merged divergence when a later membership page proves absence', async () => { + mockMergedLinkedPRLookup() + // Page 1 is full and omits the linked PR (truncated), but page 2 is short and + // still omits it — that pair definitively proves the head is not contained. + const fullPage = Array.from({ length: 100 }, (_, index) => ({ number: 1000 + index })) + ghExecFileAsyncMock + .mockResolvedValueOnce({ stdout: JSON.stringify(fullPage) }) + .mockResolvedValueOnce({ stdout: JSON.stringify([{ number: 2000 }]) }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null, { + currentHeadOid: 'bbbb2222bbbb2222' + }) + + expect(outcome).toMatchObject({ + kind: 'found', + pr: { number: 7447, state: 'merged', headDivergedFromMergedPRAtOid: 'bbbb2222bbbb2222' } + }) + }) + + it('leaves linked merged divergence unset when membership pages stay full to the cap', async () => { + mockMergedLinkedPRLookup() + // Every page up to the cap is full and omits the linked PR, so absence can + // never be proven — the probe must stay unknown rather than clear the link. + const fullPage = Array.from({ length: 100 }, (_, index) => ({ number: 1000 + index })) + for (let page = 0; page < 5; page += 1) { + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: JSON.stringify(fullPage) }) + } + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null, { + currentHeadOid: 'bbbb2222bbbb2222' + }) + + expect(outcome).toMatchObject({ kind: 'found', pr: { number: 7447, state: 'merged' } }) + expect(outcome.kind === 'found' ? outcome.pr.headDivergedFromMergedPRAtOid : undefined).toBe( + undefined + ) + }) + + it('stamps linked merged divergence via the PR url when no repo candidates resolve', async () => { + // Fallback path: no resolved candidates, so `gh pr view` returns the PR with + // dataRepo=null. The membership probe must still run against the repo derived + // from the PR's own URL so a diverged merged linked PR can clear. + resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ candidates: [], headRepo: null }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 7447, + title: 'Merged linked PR', + state: 'MERGED', + url: 'https://github.com/acme/widgets/pull/7447', + statusCheckRollup: [], + updatedAt: '2026-07-03T21:27:36Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'old-linked-branch', + baseRefOid: 'base-oid', + headRefOid: 'aaaa1111aaaa1111' + }) + }) + .mockResolvedValueOnce({ stdout: JSON.stringify([{ number: 2000 }]) }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null, { + currentHeadOid: 'bbbb2222bbbb2222' + }) + + expect(outcome).toMatchObject({ + kind: 'found', + pr: { number: 7447, state: 'merged', headDivergedFromMergedPRAtOid: 'bbbb2222bbbb2222' } + }) + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + ['api', 'repos/acme/widgets/commits/bbbb2222bbbb2222/pulls?per_page=100&page=1'], + expect.anything() + ) + }) + + it('leaves linked merged divergence unset when the membership probe is rate-limited', async () => { + mockMergedLinkedPRLookup() + rateLimitGuardMock.mockImplementation((bucket?: string) => + bucket === 'core' + ? { blocked: true, remaining: 0, limit: 5000, resetAt: 0 } + : { blocked: false } + ) + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null, { + currentHeadOid: 'bbbb2222bbbb2222' + }) + + expect(outcome).toMatchObject({ kind: 'found', pr: { number: 7447, state: 'merged' } }) + expect(outcome.kind === 'found' ? outcome.pr.headDivergedFromMergedPRAtOid : undefined).toBe( + undefined + ) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + + it('leaves linked merged divergence unset when the membership probe throws', async () => { + mockMergedLinkedPRLookup() + ghExecFileAsyncMock.mockRejectedValueOnce(new Error('HTTP 422: No commit found')) + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null, { + currentHeadOid: 'bbbb2222bbbb2222' + }) + + expect(outcome).toMatchObject({ kind: 'found', pr: { number: 7447, state: 'merged' } }) + expect(outcome.kind === 'found' ? outcome.pr.headDivergedFromMergedPRAtOid : undefined).toBe( + undefined + ) + }) + + it('leaves linked merged divergence unset when the membership probe returns a non-array payload', async () => { + mockMergedLinkedPRLookup() + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify({ message: 'Server Error' }) + }) + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null, { + currentHeadOid: 'bbbb2222bbbb2222' + }) + + expect(outcome).toMatchObject({ kind: 'found', pr: { number: 7447, state: 'merged' } }) + expect(outcome.kind === 'found' ? outcome.pr.headDivergedFromMergedPRAtOid : undefined).toBe( + undefined + ) + }) + + it('leaves linked merged divergence unset without a current head oid', async () => { + mockMergedLinkedPRLookup() + + const outcome = await getPRForBranchOutcome('/repo-root', 'new-work', 7447, null, null) + + expect(outcome).toMatchObject({ kind: 'found', pr: { number: 7447, state: 'merged' } }) + expect(outcome.kind === 'found' ? outcome.pr.headDivergedFromMergedPRAtOid : undefined).toBe( + undefined + ) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + 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 0c86d547e..8d0030dfe 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -62,7 +62,10 @@ import { type LocalGitExecOptions, type OwnerRepo } from './gh-utils' -import { isCommitPartOfMergedPR } from './merged-pr-commit-membership' +import { + isCommitPartOfMergedPR, + type MergedPRCommitMembership +} from './merged-pr-commit-membership' import { getSshGitProvider } from '../providers/ssh-git-dispatch' import { hasHostedReviewLocalGitOptions, @@ -2771,6 +2774,15 @@ export async function getPRForBranch( return outcome.kind === 'found' ? outcome.pr : null } +// Why: the exact-linked fallback (`gh pr view` with no resolved repo candidates) +// returns dataRepo=null, which would leave the merged-PR membership probe unable +// to run. Derive the PR's own repo from its web URL so a diverged merged linked +// PR can still be confirmed and cleared. Host-agnostic to cover GitHub Enterprise. +function ownerRepoFromPullRequestUrl(url: string): OwnerRepo | null { + const match = url.match(/^https?:\/\/[^/\s]+\/([^/\s]+)\/([^/\s]+)\/pull\/\d+/) + return match ? { owner: match[1], repo: match[2] } : null +} + export async function getPRForBranchOutcome( repoPath: string, branch: string, @@ -2810,24 +2822,49 @@ export async function getPRForBranchOutcome( ? options.currentHeadOid.trim() : null let confirmedContainedHeadOid: string | null = null + let headDivergedFromMergedPRAtOid: string | null = null const mergedPRContainsHead = async ( candidate: PullRequestLookupData, candidateRepo: OwnerRepo | null, headOid: string | null - ): Promise => { + ): Promise => { if (!candidateRepo || !headOid) { - return false + return 'unknown' } - const contained = await isCommitPartOfMergedPR({ + const membership = await isCommitPartOfMergedPR({ ownerRepo: candidateRepo, prNumber: candidate.number, commitOid: headOid, ghOptions }) - if (contained) { + if (membership === 'contained') { confirmedContainedHeadOid = headOid } - return contained + return membership + } + const recordLinkedMergedPRDivergence = async ( + candidate: PullRequestLookupData | null, + candidateRepo: OwnerRepo | null + ): Promise => { + if ( + typeof linkedPRNumber !== 'number' || + !candidate || + mapPRState(candidate.state, candidate.isDraft) !== 'merged' || + explicitCurrentHeadOid === null || + candidate.headRefOid === explicitCurrentHeadOid + ) { + return + } + const membership = await mergedPRContainsHead( + candidate, + candidateRepo ?? ownerRepoFromPullRequestUrl(candidate.url), + explicitCurrentHeadOid + ) + if (membership === 'not-contained') { + // explicitCurrentHeadOid is non-null here (guarded above); record the + // exact head so consumers only clear the worktree that actually diverged. + headDivergedFromMergedPRAtOid = explicitCurrentHeadOid + } } const hideMergedImplicitPR = async ( candidate: PullRequestLookupData | null, @@ -2850,11 +2887,10 @@ export async function getPRForBranchOutcome( // merges, web-committed suggestions). A head that is one of the PR's own // commits is the same line of work, not a reused branch name — keep the // merged PR visible instead of offering "create a pull request". - return !(await mergedPRContainsHead( - candidate, - candidateRepo, - currentHeadOidForMergedImplicit - )) + return ( + (await mergedPRContainsHead(candidate, candidateRepo, currentHeadOidForMergedImplicit)) !== + 'contained' + ) } if (typeof linkedPRNumber === 'number') { @@ -2942,6 +2978,7 @@ export async function getPRForBranchOutcome( } return { kind: 'no-pr', fetchedAt: Date.now() } } + await recordLinkedMergedPRDivergence(data, dataRepo) const fallbackConfirmedMergedBranch = typeof fallbackPRNumber === 'number' && mergedBranchLookupNumber === fallbackPRNumber && @@ -2949,7 +2986,7 @@ export async function getPRForBranchOutcome( const explicitHeadHidesMergedImplicitPR = explicitCurrentHeadOid !== null && shouldHideMergedImplicitPR(data, linkedPRNumber, explicitCurrentHeadOid) && - !(await mergedPRContainsHead(data, dataRepo, explicitCurrentHeadOid)) + (await mergedPRContainsHead(data, dataRepo, explicitCurrentHeadOid)) !== 'contained' // Why no lazy-HEAD re-check on preservation: fallback numbers come from // callers that already gated them on head equality or confirmed // containment; re-hiding against the main-repo HEAD would blank @@ -3003,6 +3040,7 @@ export async function getPRForBranchOutcome( ...(data.mergeStateStatus !== undefined ? { mergeStateStatus: data.mergeStateStatus } : {}), headSha: data.headRefOid, ...(confirmedContainedHeadOid ? { confirmedContainedHeadOid } : {}), + ...(headDivergedFromMergedPRAtOid ? { headDivergedFromMergedPRAtOid } : {}), ...(data.baseRefName ? { baseRefName: data.baseRefName } : {}), prRepo: dataRepo ?? undefined, headRepo: dataHeadRepo ?? undefined, diff --git a/src/main/github/merged-pr-commit-membership.ts b/src/main/github/merged-pr-commit-membership.ts index a09f43ef0..0bb390f64 100644 --- a/src/main/github/merged-pr-commit-membership.ts +++ b/src/main/github/merged-pr-commit-membership.ts @@ -11,8 +11,15 @@ type GhExecOptions = Parameters[1] const MEMBERSHIP_CACHE_MAX_ENTRIES = 200 const MEMBERSHIP_DEFINITIVE_TTL_MS = 6 * 60 * 60 * 1000 const MEMBERSHIP_ERROR_TTL_MS = 5 * 60 * 1000 +const COMMIT_PULLS_PAGE_SIZE = 100 +// Why: a worktree HEAD is associated with ~1 PR, so page 1 is short in practice +// and this cap is never reached; it only bounds the pathological case of a commit +// linked to hundreds of PRs, where staying 'unknown' is the safe answer. +const COMMIT_PULLS_MAX_PAGES = 5 -const membershipCache = new Map() +export type MergedPRCommitMembership = 'contained' | 'not-contained' | 'unknown' + +const membershipCache = new Map() function pruneMergedPRCommitMembershipCache(now = Date.now()): void { for (const [cacheKey, cached] of membershipCache) { @@ -38,17 +45,17 @@ export function resetMergedPRCommitMembershipCacheForTest(): void { * commit belongs to that PR's history rather than merely sharing a branch name. * A worktree sitting on such a commit is on the PR's own line of work (for * example behind web-committed suggestions or an update-branch merge), not a - * reused branch name. Conservative on any failure: returns false. + * reused branch name. Conservative on any failure: returns unknown. */ export async function isCommitPartOfMergedPR(args: { ownerRepo: OwnerRepo prNumber: number commitOid: string ghOptions: GhExecOptions -}): Promise { +}): Promise { const oid = args.commitOid.trim().toLowerCase() if (!/^[0-9a-f]{4,64}$/.test(oid) || !Number.isInteger(args.prNumber)) { - return false + return 'unknown' } const owner = args.ownerRepo.owner const repo = args.ownerRepo.repo @@ -59,38 +66,78 @@ export async function isCommitPartOfMergedPR(args: { if (cached && cached.expiresAt > now) { return cached.value } - // Why blocked → uncached false: keep the merged PR hidden without burning - // budget; the next poll re-asks once the rate-limit window recovers. + // Why blocked stays unknown: hiding a transient branch match is safe, but + // callers must not clear a durable linked PR when the probe never ran. if (rateLimitGuard('core').blocked) { - return false + return 'unknown' } try { - noteRateLimitSpend('core') - const { stdout } = await ghExecFileAsync( - ['api', `repos/${owner}/${repo}/commits/${oid}/pulls?per_page=100`], - args.ghOptions - ) - const parsed = JSON.parse(stdout) as unknown - const value = - Array.isArray(parsed) && - parsed.some( + // Why paginate: a full page that omits the target PR may just be truncated (a + // commit can belong to many PRs). Reading only page 1 and calling it + // 'not-contained' would wrongly clear a durable link; calling it 'unknown' + // would never clear one that genuinely diverged. Walk pages until the PR is + // found (contained) or a short page proves absence (not-contained); only the + // pathological all-full case up to the cap stays 'unknown'. + for (let page = 1; page <= COMMIT_PULLS_MAX_PAGES; page += 1) { + if (page > 1 && rateLimitGuard('core').blocked) { + membershipCache.set(cacheKey, { + value: 'unknown', + expiresAt: now + MEMBERSHIP_ERROR_TTL_MS + }) + return 'unknown' + } + noteRateLimitSpend('core') + const { stdout } = await ghExecFileAsync( + [ + 'api', + `repos/${owner}/${repo}/commits/${oid}/pulls?per_page=${COMMIT_PULLS_PAGE_SIZE}&page=${page}` + ], + args.ghOptions + ) + const parsed = JSON.parse(stdout) as unknown + // Why: a non-array success payload is a shape mismatch, not an empty page; + // caching it as definitive not-contained could wrongly clear a durable link. + if (!Array.isArray(parsed)) { + membershipCache.set(cacheKey, { + value: 'unknown', + expiresAt: now + MEMBERSHIP_ERROR_TTL_MS + }) + return 'unknown' + } + const entries = parsed + const contained = entries.some( (entry) => typeof entry === 'object' && entry !== null && (entry as { number?: unknown }).number === args.prNumber ) + if (contained) { + membershipCache.set(cacheKey, { + value: 'contained', + expiresAt: now + MEMBERSHIP_DEFINITIVE_TTL_MS + }) + return 'contained' + } + if (entries.length < COMMIT_PULLS_PAGE_SIZE) { + membershipCache.set(cacheKey, { + value: 'not-contained', + expiresAt: now + MEMBERSHIP_DEFINITIVE_TTL_MS + }) + return 'not-contained' + } + } membershipCache.set(cacheKey, { - value, - expiresAt: now + MEMBERSHIP_DEFINITIVE_TTL_MS - }) - return value - } catch { - // Why: the common failure is 422 for a commit never pushed to GitHub — - // definitive "new local work" today, but a push can change it; retry later. - membershipCache.set(cacheKey, { - value: false, + value: 'unknown', expiresAt: now + MEMBERSHIP_ERROR_TTL_MS }) - return false + return 'unknown' + } catch { + // Why: 422 often means "new local work" today, but a later push can make + // the answer knowable; preserve durable links until a probe succeeds. + membershipCache.set(cacheKey, { + value: 'unknown', + expiresAt: now + MEMBERSHIP_ERROR_TTL_MS + }) + return 'unknown' } } diff --git a/src/main/github/pr-refresh-coordinator.test.ts b/src/main/github/pr-refresh-coordinator.test.ts index 766a734f2..76bd24603 100644 --- a/src/main/github/pr-refresh-coordinator.test.ts +++ b/src/main/github/pr-refresh-coordinator.test.ts @@ -116,6 +116,25 @@ describe('pr-refresh-coordinator', () => { ) }) + it('copies the candidate worktree head onto broadcast aliases', async () => { + const { reportVisiblePRRefreshCandidates } = await import('./pr-refresh-coordinator') + getPRForBranchOutcomeMock.mockResolvedValueOnce({ + kind: 'found', + pr: makePR({ state: 'merged' }), + fetchedAt: Date.now() + }) + + reportVisiblePRRefreshCandidates([makeCandidate({ currentHeadOid: 'worktree-head-oid' })], 1, 1) + await vi.runOnlyPendingTimersAsync() + + // Why: the renderer clear of a diverged merged linked PR is head-scoped, so + // the broadcast alias must carry the request-time head it was probed against. + const outcomeEvent = sendMock.mock.calls + .map(([, event]) => event) + .find((event) => event.outcome) + expect(outcomeEvent?.aliases[0]?.currentHeadOid).toBe('worktree-head-oid') + }) + it('does not show visible background refreshes as queued', async () => { const { reportVisiblePRRefreshCandidates } = await import('./pr-refresh-coordinator') getPRForBranchOutcomeMock.mockResolvedValueOnce({ @@ -633,6 +652,103 @@ describe('pr-refresh-coordinator', () => { expect(getPRForBranchOutcomeMock).toHaveBeenCalledTimes(2) }) + it('probes with the survivor head after the representative alias is invalidated', async () => { + const { enqueuePRRefresh } = await import('./pr-refresh-coordinator') + getPRForBranchOutcomeMock.mockResolvedValue({ + kind: 'found', + pr: makePR({ state: 'merged' }), + fetchedAt: Date.now() + }) + + const survivor = makeCandidate({ + cacheKey: '/repo::feature/b', + branch: 'feature/b', + linkedPRNumber: 12, + worktreeId: 'wt-b', + currentHeadOid: 'head-b' + }) + const representative = makeCandidate({ + cacheKey: '/repo::feature/a', + branch: 'feature/a', + linkedPRNumber: 12, + worktreeId: 'wt-a', + currentHeadOid: 'head-a' + }) + + // Enqueue the survivor first, then the representative (active coalescing + // promotes the latter to representative), then invalidate the representative + // so the still-queued entry rebinds to the survivor before draining. + enqueuePRRefresh(survivor, 'active', 80, 1) + enqueuePRRefresh(representative, 'active', 80, 1) + enqueuePRRefresh({ ...representative, isArchived: true }, 'active', 80, 1) + await vi.runOnlyPendingTimersAsync() + + const probedHeads = getPRForBranchOutcomeMock.mock.calls.map((call) => call[5]?.currentHeadOid) + expect(probedHeads).toContain('head-b') + expect(probedHeads).not.toContain('head-a') + }) + + it('probes with the survivor head after the representative worktree is pruned', async () => { + const { enqueuePRRefresh, pruneWorktreePRRefreshAliases } = + await import('./pr-refresh-coordinator') + getPRForBranchOutcomeMock.mockResolvedValue({ + kind: 'found', + pr: makePR({ state: 'merged' }), + fetchedAt: Date.now() + }) + + const survivor = makeCandidate({ + cacheKey: '/repo::feature/b', + branch: 'feature/b', + linkedPRNumber: 12, + worktreeId: 'wt-b', + currentHeadOid: 'head-b' + }) + const representative = makeCandidate({ + cacheKey: '/repo::feature/a', + branch: 'feature/a', + linkedPRNumber: 12, + worktreeId: 'wt-a', + currentHeadOid: 'head-a' + }) + + enqueuePRRefresh(survivor, 'active', 80, 1) + enqueuePRRefresh(representative, 'active', 80, 1) + pruneWorktreePRRefreshAliases('wt-a') + await vi.runOnlyPendingTimersAsync() + + const probedHeads = getPRForBranchOutcomeMock.mock.calls.map((call) => call[5]?.currentHeadOid) + expect(probedHeads).toContain('head-b') + expect(probedHeads).not.toContain('head-a') + }) + + it('refreshes the representative head when the same worktree re-reports a moved head', async () => { + const { reportVisiblePRRefreshCandidates } = await import('./pr-refresh-coordinator') + getPRForBranchOutcomeMock.mockResolvedValue({ + kind: 'found', + pr: makePR({ state: 'merged' }), + fetchedAt: Date.now() + }) + + // Same worktree/branch, moved head, coalescing visible→visible (no promote): + // the representative head must track the newest report before the drain. + reportVisiblePRRefreshCandidates( + [makeCandidate({ linkedPRNumber: 12, worktreeId: 'wt-a', currentHeadOid: 'head-a' })], + 1, + 1 + ) + reportVisiblePRRefreshCandidates( + [makeCandidate({ linkedPRNumber: 12, worktreeId: 'wt-a', currentHeadOid: 'head-b' })], + 2, + 1 + ) + await vi.runOnlyPendingTimersAsync() + + const probedHeads = getPRForBranchOutcomeMock.mock.calls.map((call) => call[5]?.currentHeadOid) + expect(probedHeads).toContain('head-b') + expect(probedHeads).not.toContain('head-a') + }) + it('includes request start time on manual refresh events', async () => { const { refreshPRNow } = await import('./pr-refresh-coordinator') getPRForBranchOutcomeMock.mockResolvedValueOnce({ diff --git a/src/main/github/pr-refresh-coordinator.ts b/src/main/github/pr-refresh-coordinator.ts index 5bb70a970..a2576e480 100644 --- a/src/main/github/pr-refresh-coordinator.ts +++ b/src/main/github/pr-refresh-coordinator.ts @@ -188,7 +188,11 @@ export function pruneWorktreePRRefreshAliases(worktreeId: string): void { ...entry.candidate, cacheKey: replacementAlias.cacheKey, branch: replacementAlias.branch, - worktreeId: replacementAlias.worktreeId + worktreeId: replacementAlias.worktreeId, + // Why: the probe now represents the replacement worktree, so it must + // use that worktree's head — otherwise divergence is stamped for the + // removed worktree's head and the survivor's link is never cleared. + currentHeadOid: replacementAlias.currentHeadOid ?? null } } } @@ -321,6 +325,7 @@ function aliasFromCandidate(candidate: GitHubPRRefreshCandidate): GitHubPRRefres branch: candidate.branch, worktreeId: candidate.worktreeId, connectionId: candidate.connectionId ?? null, + currentHeadOid: candidate.currentHeadOid ?? null, linkedPRNumber: candidate.linkedPRNumber ?? null, fallbackPRNumber: candidate.linkedPRNumber == null ? (candidate.fallbackPRNumber ?? null) : null, @@ -393,6 +398,10 @@ function removeQueuedAliasForInvalidCandidate(key: string, alias: GitHubPRRefres cacheKey: replacementAlias.cacheKey, branch: replacementAlias.branch, worktreeId: replacementAlias.worktreeId, + // Why: the probe now represents the replacement worktree, so it must use + // that worktree's head — otherwise divergence is stamped for the pruned + // candidate's head and the survivor's link is never cleared. + currentHeadOid: replacementAlias.currentHeadOid ?? null, isArchived: false, isBare: false } @@ -809,6 +818,18 @@ export function enqueuePRRefresh( existing.activeDelayNotified = false existing.candidate = candidate existing.windowId = windowId ?? existing.windowId + } else if (existing.candidate.worktreeId === candidate.worktreeId) { + // Why: a non-promoting coalesce (e.g. visible→visible) keeps the existing + // representative, but the representative drives the probe head. If its own + // worktree moved head/branch, refresh those probe inputs so divergence is + // stamped for the current head — otherwise the head-scoped clear never + // matches and a merged linked PR lingers after a branch switch. + existing.candidate = { + ...existing.candidate, + cacheKey: candidate.cacheKey, + branch: candidate.branch, + currentHeadOid: candidate.currentHeadOid ?? null + } } } else { diagnosticsCounters.enqueued += 1 diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index e17be5050..8cbe73b4f 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -127,6 +127,49 @@ function makePRRefreshWorktree(overrides: Partial = {}): Worktree { } } +function installLinkedPRClearStub( + store: ReturnType, + args: { + repoId: string + repoPath: string + branch: string + worktree: Worktree + } +) { + const cacheKey = `${args.repoId}::${args.branch}` + const updateWorktreeMeta = vi.fn( + async ( + worktreeId: string, + updates: Parameters[1], + options?: Parameters[2] + ) => { + const currentWorktree = store + .getState() + .worktreesByRepo[args.repoId]?.find((worktree) => worktree.id === worktreeId) + if (options?.shouldApply && !options.shouldApply(currentWorktree)) { + return + } + store.setState((state) => { + const nextWorktrees = { + ...state.worktreesByRepo, + [args.repoId]: (state.worktreesByRepo[args.repoId] ?? []).map((worktree) => + worktree.id === worktreeId ? { ...worktree, ...updates } : worktree + ) + } + const nextPRCache = { ...state.prCache } + delete nextPRCache[cacheKey] + return { worktreesByRepo: nextWorktrees, prCache: nextPRCache } as Partial + }) + } + ) + store.setState({ + repos: [{ id: args.repoId, path: args.repoPath, name: 'repo', kind: 'git' }], + worktreesByRepo: { [args.repoId]: [args.worktree] }, + updateWorktreeMeta + } as unknown as Partial) + return updateWorktreeMeta +} + function githubSourceContext( hostId: TaskSourceContext['hostId'], repoId = 'source-repo-id' @@ -2133,6 +2176,421 @@ describe('createGitHubSlice.fetchPRForBranch', () => { expect(store.getState().hostedReviewCache[hostedReviewCacheKey]).toBeUndefined() }) + it('clears a linked merged PR when the resolved PR definitively diverged from the request head', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/new-work' + const worktreeId = 'wt-diverged-linked-pr' + const worktree = makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 12 + }) + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree + }) + mockApi.gh.refreshPRNow.mockResolvedValueOnce({ + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + headDivergedFromMergedPRAtOid: 'current-head' + }), + fetchedAt: 2 + }) + + await expect( + store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId, + linkedPRNumber: 12 + }) + ).resolves.toMatchObject({ number: 12 }) + + expect(updateWorktreeMeta).toHaveBeenCalledWith( + worktreeId, + { linkedPR: null }, + { shouldApply: expect.any(Function) } + ) + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBeNull() + expect(store.getState().prCache[`${repoId}::${branch}`]).toBeUndefined() + }) + + it('clears a linked merged PR on a fresh cache hit that already carries a head-scoped divergence signal', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/cached-diverged' + const worktreeId = 'wt-cached-diverged' + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 12 + }) + }) + store.setState({ + prCache: { + [`${repoId}::${branch}`]: { + data: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + headDivergedFromMergedPRAtOid: 'current-head' + }), + fetchedAt: Date.now() + } + } + } as unknown as Partial) + + const result = await store.getState().fetchPRForBranch(repoPath, branch, { + repoId, + worktreeId, + linkedPRNumber: 12 + }) + + expect(result).toMatchObject({ number: 12 }) + expect(mockApi.gh.refreshPRNow).not.toHaveBeenCalled() + expect(updateWorktreeMeta).toHaveBeenCalledWith( + worktreeId, + { linkedPR: null }, + { shouldApply: expect.any(Function) } + ) + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBeNull() + }) + + it('does not clear a linked merged PR when the request head equals the PR head', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/at-pr-head' + const worktreeId = 'wt-at-pr-head' + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'same-head', + linkedPR: 12 + }) + }) + mockApi.gh.refreshPRNow.mockResolvedValueOnce({ + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'same-head', + headDivergedFromMergedPRAtOid: 'same-head' + }), + fetchedAt: 2 + }) + + await store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId, + linkedPRNumber: 12 + }) + + expect(updateWorktreeMeta).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBe(12) + }) + + it('does not clear a linked merged PR when the request head is confirmed contained', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/contained' + const worktreeId = 'wt-contained' + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'contained-head', + linkedPR: 12 + }) + }) + mockApi.gh.refreshPRNow.mockResolvedValueOnce({ + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + confirmedContainedHeadOid: 'contained-head', + headDivergedFromMergedPRAtOid: 'contained-head' + }), + fetchedAt: 2 + }) + + await store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId, + linkedPRNumber: 12 + }) + + expect(updateWorktreeMeta).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBe(12) + }) + + it('does not clear a linked open PR even when a divergence bit is present', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/open-pr' + const worktreeId = 'wt-open-pr' + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 12 + }) + }) + mockApi.gh.refreshPRNow.mockResolvedValueOnce({ + kind: 'found', + pr: makePR({ + number: 12, + state: 'open', + headSha: 'pr-head', + headDivergedFromMergedPRAtOid: 'current-head' + }), + fetchedAt: 2 + }) + + await store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId, + linkedPRNumber: 12 + }) + + expect(updateWorktreeMeta).not.toHaveBeenCalled() + }) + + it('does not clear a linked PR on a null PR result', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/null-pr' + const worktreeId = 'wt-null-pr' + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 12 + }) + }) + mockApi.gh.refreshPRNow.mockResolvedValueOnce({ kind: 'no-pr', fetchedAt: 2 }) + + await store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId, + linkedPRNumber: 12 + }) + + expect(updateWorktreeMeta).not.toHaveBeenCalled() + }) + + it('does not clear when divergence is unset even if containment does not match the head', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/unknown-probe' + const worktreeId = 'wt-unknown-probe' + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 12 + }) + }) + mockApi.gh.refreshPRNow.mockResolvedValueOnce({ + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + confirmedContainedHeadOid: 'other-head' + }), + fetchedAt: 2 + }) + + await store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId, + linkedPRNumber: 12 + }) + + expect(updateWorktreeMeta).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBe(12) + }) + + it('does not clear when the linked PR number changed before the lookup completed', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/relinked' + const worktreeId = 'wt-relinked' + let resolveRefresh: ( + value: Awaited> + ) => void = () => {} + const refresh = new Promise>>((resolve) => { + resolveRefresh = resolve + }) + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 12 + }) + }) + mockApi.gh.refreshPRNow.mockReturnValueOnce(refresh) + + const request = store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId, + linkedPRNumber: 12 + }) + store.setState({ + worktreesByRepo: { + [repoId]: [ + makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 13 + }) + ] + } + } as unknown as Partial) + resolveRefresh({ + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + headDivergedFromMergedPRAtOid: 'current-head' + }), + fetchedAt: 2 + }) + + await expect(request).resolves.toBeNull() + expect(updateWorktreeMeta).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBe(13) + }) + + it('does not clear when the worktree head moved after the lookup started', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/head-moved' + const worktreeId = 'wt-head-moved' + let resolveRefresh: ( + value: Awaited> + ) => void = () => {} + const refresh = new Promise>>((resolve) => { + resolveRefresh = resolve + }) + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'request-head', + linkedPR: 12 + }) + }) + mockApi.gh.refreshPRNow.mockReturnValueOnce(refresh) + + const request = store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId, + linkedPRNumber: 12 + }) + store.setState({ + worktreesByRepo: { + [repoId]: [ + makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'new-head', + linkedPR: 12 + }) + ] + } + } as unknown as Partial) + resolveRefresh({ + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + headDivergedFromMergedPRAtOid: 'request-head' + }), + fetchedAt: 2 + }) + + await expect(request).resolves.toMatchObject({ number: 12 }) + expect(updateWorktreeMeta).toHaveBeenCalledWith( + worktreeId, + { linkedPR: null }, + { shouldApply: expect.any(Function) } + ) + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBe(12) + expect(store.getState().prCache[`${repoId}::${branch}`]).toMatchObject({ + data: expect.objectContaining({ number: 12 }) + }) + }) + it('preserves cached PR data when a forced coordinator refresh errors', async () => { const store = createTestStore() const repoPath = '/repo' @@ -2547,6 +3005,192 @@ describe('createGitHubSlice.fetchPRForBranch', () => { }) }) + it('clears a linked merged PR from a coordinator refresh event when the request head diverged', () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/coordinator-diverged' + const worktreeId = 'wt-coordinator-diverged' + const cacheKey = `${repoId}::${branch}` + const worktree = makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 12 + }) + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree + }) + + store.getState().applyGitHubPRRefreshEvent({ + sequence: 1, + reason: 'swr', + aliases: [ + { + cacheKey, + repoPath, + repoId, + branch, + worktreeId, + linkedPRNumber: 12, + currentHeadOid: 'current-head' + } + ], + outcome: { + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + headDivergedFromMergedPRAtOid: 'current-head' + }), + fetchedAt: 2 + } + }) + + expect(updateWorktreeMeta).toHaveBeenCalledWith( + worktreeId, + { linkedPR: null }, + { shouldApply: expect.any(Function) } + ) + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBeNull() + }) + + it('does not clear a linked merged PR from a coordinator refresh event without a request head', () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/coordinator-no-head' + const worktreeId = 'wt-coordinator-no-head' + const cacheKey = `${repoId}::${branch}` + const updateWorktreeMeta = installLinkedPRClearStub(store, { + repoId, + repoPath, + branch, + worktree: makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'current-head', + linkedPR: 12 + }) + }) + + store.getState().applyGitHubPRRefreshEvent({ + sequence: 1, + reason: 'swr', + aliases: [ + { cacheKey, repoPath, repoId, branch, worktreeId, linkedPRNumber: 12, currentHeadOid: null } + ], + outcome: { + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + headDivergedFromMergedPRAtOid: 'current-head' + }), + fetchedAt: 2 + } + }) + + expect(updateWorktreeMeta).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo[repoId]?.[0]?.linkedPR).toBe(12) + }) + + it('clears only the diverged worktree when a PR-number-coalesced event fans out to sibling aliases', () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const worktreeA = makePRRefreshWorktree({ + id: 'wt-a', + repoId, + branch: 'feature/a', + head: 'head-a', + linkedPR: 12 + }) + const worktreeB = makePRRefreshWorktree({ + id: 'wt-b', + repoId, + branch: 'feature/b', + head: 'head-b', + linkedPR: 12 + }) + const updateWorktreeMeta = vi.fn( + async ( + worktreeId: string, + updates: Parameters[1], + options?: Parameters[2] + ) => { + const current = store + .getState() + .worktreesByRepo[repoId]?.find((worktree) => worktree.id === worktreeId) + if (options?.shouldApply && !options.shouldApply(current)) { + return + } + store.setState((state) => ({ + worktreesByRepo: { + ...state.worktreesByRepo, + [repoId]: (state.worktreesByRepo[repoId] ?? []).map((worktree) => + worktree.id === worktreeId ? { ...worktree, ...updates } : worktree + ) + } + })) + } + ) + store.setState({ + repos: [{ id: repoId, path: repoPath, name: 'repo', kind: 'git' }], + worktreesByRepo: { [repoId]: [worktreeA, worktreeB] }, + updateWorktreeMeta + } as unknown as Partial) + + // The coordinator coalesces linked PR refreshes by PR number, so one probe + // (worktree A's head) is broadcast to both aliases. Only A actually diverged; + // B is still on a contained commit and must keep its link. + store.getState().applyGitHubPRRefreshEvent({ + sequence: 1, + reason: 'swr', + aliases: [ + { + cacheKey: `${repoId}::feature/a`, + repoPath, + repoId, + branch: 'feature/a', + worktreeId: 'wt-a', + linkedPRNumber: 12, + currentHeadOid: 'head-a' + }, + { + cacheKey: `${repoId}::feature/b`, + repoPath, + repoId, + branch: 'feature/b', + worktreeId: 'wt-b', + linkedPRNumber: 12, + currentHeadOid: 'head-b' + } + ], + outcome: { + kind: 'found', + pr: makePR({ + number: 12, + state: 'merged', + headSha: 'merged-pr-head', + headDivergedFromMergedPRAtOid: 'head-a' + }), + fetchedAt: 2 + } + }) + + const worktrees = store.getState().worktreesByRepo[repoId] ?? [] + expect(worktrees.find((worktree) => worktree.id === 'wt-a')?.linkedPR).toBeNull() + expect(worktrees.find((worktree) => worktree.id === 'wt-b')?.linkedPR).toBe(12) + }) + it('preserves visible cached PR data when a fallback refresh event misses', () => { const store = createTestStore() const repoPath = '/repo' diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 8478c5198..bc935dcba 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -1005,6 +1005,44 @@ function isStaleExactLinkedPRLookup( return findWorktreeById(state, worktreeId)?.linkedPR !== linkedPRNumber } +function shouldClearDivergedLinkedMergedPR(args: { + pr: PRInfo | null + linkedPRNumber: number | null + requestHeadOid: string | null +}): boolean { + const { pr, linkedPRNumber, requestHeadOid } = args + return ( + linkedPRNumber != null && + requestHeadOid !== null && + pr?.number === linkedPRNumber && + pr.state === 'merged' && + // Head-scoped: only clear the worktree whose exact head diverged, so a + // PR-number-coalesced refresh broadcast cannot clear a sibling worktree that + // is still on the PR's line of work. + pr.headDivergedFromMergedPRAtOid === requestHeadOid && + pr.headSha !== requestHeadOid && + pr.confirmedContainedHeadOid !== requestHeadOid + ) +} + +function shouldApplyDivergedLinkedPRClear(args: { + worktree: Pick | undefined + linkedPRNumber: number + branch: string + requestHeadOid: string | null +}): boolean { + const { worktree, linkedPRNumber, branch, requestHeadOid } = args + return ( + Boolean(worktree) && + requestHeadOid !== null && + worktree?.linkedPR === linkedPRNumber && + worktree.branch.replace(/^refs\/heads\//, '') === branch && + worktree.head === requestHeadOid && + worktree.isBare !== true && + worktree.isArchived !== true + ) +} + function buildPRRefreshCandidate( state: AppState, worktree: Worktree, @@ -2923,6 +2961,38 @@ export const createGitHubSlice: StateCreator = (s const linkedRefetch = cached?.data === null && (linkedPRNumber !== null || fallbackPRNumber !== null) if (!options?.force && !linkedRefetch && isFresh(cached)) { + // Why: a fresh cache hit still carries the head-scoped divergence signal. + // If a prior clear was declined because the head moved mid-request and the + // worktree is now back on that diverged head, clear the durable link the + // cache would otherwise keep serving until it expires. + if ( + options?.worktreeId && + linkedPRNumber != null && + cached?.data?.headDivergedFromMergedPRAtOid != null + ) { + const currentHeadOid = findWorktreeById(get(), options.worktreeId)?.head ?? null + if ( + shouldClearDivergedLinkedMergedPR({ + pr: cached.data, + linkedPRNumber, + requestHeadOid: currentHeadOid + }) + ) { + void get().updateWorktreeMeta( + options.worktreeId, + { linkedPR: null }, + { + shouldApply: (worktree) => + shouldApplyDivergedLinkedPRClear({ + worktree, + linkedPRNumber, + branch, + requestHeadOid: currentHeadOid + }) + } + ) + } + } return cached.data } @@ -2953,6 +3023,7 @@ export const createGitHubSlice: StateCreator = (s const candidateWorktree = options?.worktreeId ? findWorktreeById(get(), options.worktreeId) : null + const requestHeadOid = candidateWorktree?.head ?? null const outcome = runtimeRepo ? await callRuntimeRpc( runtimeRepo.target, @@ -2961,7 +3032,7 @@ export const createGitHubSlice: StateCreator = (s repo: runtimeRepo.repo.id, branch, linkedPRNumber, - currentHeadOid: candidateWorktree?.head ?? null, + currentHeadOid: requestHeadOid, ...(fallbackPRNumber !== null ? { fallbackPRNumber, acceptMergedFallbackPR: fallbackPRSource !== null } : {}) @@ -2980,7 +3051,7 @@ export const createGitHubSlice: StateCreator = (s branch, cacheKey, worktreeId: options?.worktreeId, - currentHeadOid: candidateWorktree?.head ?? null, + currentHeadOid: requestHeadOid, linkedPRNumber, fallbackPRNumber, fallbackPRSource, @@ -3004,7 +3075,7 @@ export const createGitHubSlice: StateCreator = (s fallbackPRNumber, acceptMergedFallbackPR: fallbackPRNumber !== null && fallbackPRSource !== null, - currentHeadOid: candidateWorktree?.head ?? null + currentHeadOid: requestHeadOid }) .then((pr) => pr @@ -3054,6 +3125,27 @@ export const createGitHubSlice: StateCreator = (s if (didUpdatePRCache) { debouncedSaveCache(get()) } + if ( + options?.worktreeId && + linkedPRNumber != null && + shouldClearDivergedLinkedMergedPR({ pr, linkedPRNumber, requestHeadOid }) + ) { + // Why: only clear the durable link that produced this exact probe; + // branch/head drift means the stale result no longer owns the worktree. + void get().updateWorktreeMeta( + options.worktreeId, + { linkedPR: null }, + { + shouldApply: (worktree) => + shouldApplyDivergedLinkedPRClear({ + worktree, + linkedPRNumber, + branch, + requestHeadOid + }) + } + ) + } } if ( shouldPreserveExistingPRForFallbackMiss({ @@ -3773,6 +3865,34 @@ export const createGitHubSlice: StateCreator = (s }, applyGitHubPRRefreshEvent: (event) => { + // Why: the sidebar/left-list refresh for local repos flows through the main + // PR coordinator (not fetchPRForBranch), so it must run the same guarded + // clear when main stamps a merged linked PR whose head has diverged. + const divergedLinkedPRClears: { + worktreeId: string + linkedPRNumber: number + branch: string + requestHeadOid: string | null + }[] = [] + if (event.outcome?.kind === 'found') { + const pr = event.outcome.pr + for (const alias of event.aliases) { + const linkedPRNumber = alias.linkedPRNumber ?? null + const requestHeadOid = alias.currentHeadOid ?? null + if ( + alias.worktreeId && + linkedPRNumber != null && + shouldClearDivergedLinkedMergedPR({ pr, linkedPRNumber, requestHeadOid }) + ) { + divergedLinkedPRClears.push({ + worktreeId: alias.worktreeId, + linkedPRNumber, + branch: alias.branch, + requestHeadOid + }) + } + } + } let didUpdatePRCache = false set((s) => { const nextSequences = { ...s.prRefreshSequences } @@ -3969,6 +4089,21 @@ export const createGitHubSlice: StateCreator = (s if (didUpdatePRCache && event.outcome && event.outcome.kind !== 'upstream-error') { debouncedSaveCache(get()) } + for (const clear of divergedLinkedPRClears) { + void get().updateWorktreeMeta( + clear.worktreeId, + { linkedPR: null }, + { + shouldApply: (worktree) => + shouldApplyDivergedLinkedPRClear({ + worktree, + linkedPRNumber: clear.linkedPRNumber, + branch: clear.branch, + requestHeadOid: clear.requestHeadOid + }) + } + ) + } }, refreshAllGitHub: () => { diff --git a/src/renderer/src/web/web-preload-api.test.ts b/src/renderer/src/web/web-preload-api.test.ts index 0dd6b240f..4a1d7b36f 100644 --- a/src/renderer/src/web/web-preload-api.test.ts +++ b/src/renderer/src/web/web-preload-api.test.ts @@ -2490,6 +2490,7 @@ describe('web GitHub preload API', () => { repoId: 'repo-1', repoPath, branch: 'feature', + currentHeadOid: 'head-oid', linkedPRNumber: null, fallbackPRNumber: 9, fallbackPRSource: 'pr-cache' @@ -2510,6 +2511,7 @@ describe('web GitHub preload API', () => { branch: 'feature', linkedPRNumber: null, fallbackPRNumber: 9, + currentHeadOid: 'head-oid', acceptMergedFallbackPR: true } } diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 8647c9b0d..5cf3a3c8e 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -1805,6 +1805,7 @@ function createGitHubApi(): WebGitHubApi { branch: candidate.branch, linkedPRNumber: candidate.linkedPRNumber ?? null, fallbackPRNumber: candidate.fallbackPRNumber ?? null, + currentHeadOid: candidate.currentHeadOid ?? null, ...(acceptMergedFallbackPR ? { acceptMergedFallbackPR: true } : {}) }) return pr diff --git a/src/shared/types.ts b/src/shared/types.ts index 7e35a9208..5b84818af 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -1137,6 +1137,13 @@ export type PRInfo = { // of the PR's own commits (behind update-branch/web commits). Cache staleness // checks must honor that confirmation without re-querying GitHub. confirmedContainedHeadOid?: string + // Why: the worktree HEAD OID this merged linked PR was confirmed to have + // diverged from (a definite not-contained probe). Head-scoped, not a bare + // boolean, so a PR-number-coalesced refresh broadcast cannot clear a sibling + // worktree whose own head is still on the PR's line of work. Clearing a + // durable linked PR requires this positive signal for that exact head, never + // the mere absence of a containment confirmation after a rate-limit/error. + headDivergedFromMergedPRAtOid?: string /** Target branch name for PR-created worktree compare-base repair. */ baseRefName?: string prRepo?: GitHubRepositoryIdentity @@ -1179,6 +1186,10 @@ export type GitHubPRRefreshAlias = { linkedPRNumber?: number | null fallbackPRNumber?: number | null fallbackPRSource?: 'explicit' | 'pr-cache' | 'hosted-review' | null + // Why: request-time worktree HEAD. Merged branch-matched PRs are only visible + // for heads that belong to the PR, and refresh consumers need this snapshot to + // clear a durable linked PR once main confirms the head diverged. + currentHeadOid?: string | null } export type GitHubPRRefreshCandidate = GitHubPRRefreshAlias & { @@ -1196,10 +1207,6 @@ export type GitHubPRRefreshCandidate = GitHubPRRefreshAlias & { cachedMergeable?: PRMergeableState | null cachedMergeStateStatus?: string | null localGitOptions?: { wslDistro?: string } - // Why: merged branch-matched PRs are only visible for heads that belong to - // the PR; without the worktree head, a panel-supplied fallback number would - // keep a merged PR alive head-blind after the branch moves on. - currentHeadOid?: string | null } export type GitHubPRRefreshSkippedReason =