diff --git a/src/main/github/client.test.ts b/src/main/github/client.test.ts index cd3f08783c4..c6736738efe 100644 --- a/src/main/github/client.test.ts +++ b/src/main/github/client.test.ts @@ -33,7 +33,9 @@ const { getRemoteUrlForRepoMock: vi.fn(), gitExecFileAsyncMock: vi.fn(), getRateLimitMock: vi.fn(), - rateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })), + rateLimitGuardMock: vi.fn<(bucket?: string) => RateLimitGuardResult>(() => ({ + blocked: false + })), noteRateLimitSpendMock: vi.fn(), ghRepoExecOptionsMock: vi.fn((context) => context.connectionId @@ -121,6 +123,7 @@ import { __resetTrackedUpstreamBranchCacheForTests } from './client' import { __resetPRConflictSummaryGitCapabilityCacheForTests } from './conflict-summary' +import { resetMergedPRCommitMembershipCacheForTest } from './merged-pr-commit-membership' describe('checkOrcaStarred', () => { beforeEach(() => { @@ -192,6 +195,7 @@ describe('getPRForBranch', () => { _resetMergeQueueCacheForTests() __resetTrackedUpstreamBranchCacheForTests() __resetPRConflictSummaryGitCapabilityCacheForTests() + resetMergedPRCommitMembershipCacheForTest() }) it('queries GitHub by head branch when the remote is on github.com', async () => { @@ -746,6 +750,176 @@ describe('getPRForBranch', () => { }) }) + const mockMergedBranchPRLookupBehindHead = (prNumber = 6011): void => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: prNumber, + title: 'Merged PR with unpulled final head', + state: 'closed', + merged_at: '2026-07-03T21:27:36Z', + html_url: `https://github.com/acme/widgets/pull/${prNumber}`, + updated_at: '2026-07-03T21:27:36Z', + draft: false, + mergeable_state: 'clean', + head: { ref: 'fix-hibernation-wake', sha: 'aaaa1111aaaa1111' }, + base: { ref: 'main', sha: 'base-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: prNumber, + title: 'Merged PR with unpulled final head', + state: 'MERGED', + url: `https://github.com/acme/widgets/pull/${prNumber}`, + statusCheckRollup: [], + updatedAt: '2026-07-03T21:27:36Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'fix-hibernation-wake', + baseRefOid: 'base-oid', + headRefOid: 'aaaa1111aaaa1111' + }) + }) + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: 'bbbb2222bbbb2222\n', + stderr: '' + }) + } + + it('shows a merged branch PR when the worktree head is one of its own commits', async () => { + mockMergedBranchPRLookupBehindHead() + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify([{ number: 6011 }, { number: 42 }]) + }) + + const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake') + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 3, + ['api', 'repos/acme/widgets/commits/bbbb2222bbbb2222/pulls?per_page=100'], + expect.anything() + ) + expect(pr).toMatchObject({ + number: 6011, + state: 'merged', + headSha: 'aaaa1111aaaa1111', + confirmedContainedHeadOid: 'bbbb2222bbbb2222' + }) + }) + + it('keeps hiding a merged branch PR when the head belongs to a different PR (reused branch)', async () => { + mockMergedBranchPRLookupBehindHead() + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify([{ number: 42 }]) + }) + + const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake') + + expect(pr).toBeNull() + }) + + it('keeps hiding a merged branch PR when the commit membership probe fails', async () => { + mockMergedBranchPRLookupBehindHead() + ghExecFileAsyncMock.mockRejectedValueOnce(new Error('HTTP 422: No commit found')) + + const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake') + + expect(pr).toBeNull() + }) + + it('skips the membership probe while the core rate-limit budget is blocked', async () => { + mockMergedBranchPRLookupBehindHead() + rateLimitGuardMock.mockImplementation((bucket?: string) => + bucket === 'core' + ? { blocked: true, remaining: 0, limit: 5000, resetAt: 0 } + : { blocked: false } + ) + + const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake') + + expect(pr).toBeNull() + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2) + }) + + it('reuses the cached membership answer instead of re-querying per poll', async () => { + mockMergedBranchPRLookupBehindHead() + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify([{ number: 6011 }]) + }) + const first = await getPRForBranch('/repo-root', 'fix-hibernation-wake') + expect(first).toMatchObject({ number: 6011, confirmedContainedHeadOid: 'bbbb2222bbbb2222' }) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(3) + + mockMergedBranchPRLookupBehindHead() + const second = await getPRForBranch('/repo-root', 'fix-hibernation-wake') + + expect(second).toMatchObject({ number: 6011, confirmedContainedHeadOid: 'bbbb2222bbbb2222' }) + // No fourth membership call: the confirmed answer is immutable and cached. + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(5) + }) + + it('uses the caller-supplied worktree head for the membership probe without shelling out', async () => { + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock + .mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 6012, + title: 'Merged PR queried by checks panel', + state: 'closed', + merged_at: '2026-07-03T21:27:36Z', + html_url: 'https://github.com/acme/widgets/pull/6012', + updated_at: '2026-07-03T21:27:36Z', + draft: false, + mergeable_state: 'clean', + head: { ref: 'fix-hibernation-wake', sha: 'aaaa1111aaaa1111' }, + base: { ref: 'main', sha: 'base-oid' } + } + ]) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify({ + number: 6012, + title: 'Merged PR queried by checks panel', + state: 'MERGED', + url: 'https://github.com/acme/widgets/pull/6012', + statusCheckRollup: [], + updatedAt: '2026-07-03T21:27:36Z', + isDraft: false, + mergeable: 'MERGEABLE', + baseRefName: 'main', + headRefName: 'fix-hibernation-wake', + baseRefOid: 'base-oid', + headRefOid: 'aaaa1111aaaa1111' + }) + }) + .mockResolvedValueOnce({ + stdout: JSON.stringify([{ number: 6012 }]) + }) + + const outcome = await getPRForBranchOutcome( + '/repo-root', + 'fix-hibernation-wake', + null, + null, + null, + { + currentHeadOid: 'cccc3333cccc3333' + } + ) + + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + expect(outcome).toMatchObject({ + kind: 'found', + pr: { number: 6012, confirmedContainedHeadOid: 'cccc3333cccc3333' } + }) + }) + 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 a066732f1f4..4cbdb8f3257 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -62,6 +62,7 @@ import { type LocalGitExecOptions, type OwnerRepo } from './gh-utils' +import { isCommitPartOfMergedPR } from './merged-pr-commit-membership' import { getSshGitProvider } from '../providers/ssh-git-dispatch' import { hasHostedReviewLocalGitOptions, @@ -2808,7 +2809,30 @@ export async function getPRForBranchOutcome( typeof options.currentHeadOid === 'string' && options.currentHeadOid.trim().length > 0 ? options.currentHeadOid.trim() : null - const hideMergedImplicitPR = async (candidate: PullRequestLookupData | null) => { + let confirmedContainedHeadOid: string | null = null + const mergedPRContainsHead = async ( + candidate: PullRequestLookupData, + candidateRepo: OwnerRepo | null, + headOid: string | null + ): Promise => { + if (!candidateRepo || !headOid) { + return false + } + const contained = await isCommitPartOfMergedPR({ + ownerRepo: candidateRepo, + prNumber: candidate.number, + commitOid: headOid, + ghOptions + }) + if (contained) { + confirmedContainedHeadOid = headOid + } + return contained + } + const hideMergedImplicitPR = async ( + candidate: PullRequestLookupData | null, + candidateRepo: OwnerRepo | null + ) => { if (!candidate || !isMergedImplicitPR(candidate, linkedPRNumber)) { return false } @@ -2819,7 +2843,18 @@ export async function getPRForBranchOutcome( explicitCurrentHeadOid !== null ? explicitCurrentHeadOid : await getCurrentHeadOid(repoPath, connectionId, localGitOptions) - return shouldHideMergedImplicitPR(candidate, linkedPRNumber, currentHeadOidForMergedImplicit) + if (!shouldHideMergedImplicitPR(candidate, linkedPRNumber, currentHeadOidForMergedImplicit)) { + return false + } + // Why: a worktree can sit behind its own PR's final head (update-branch + // 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 + )) } if (typeof linkedPRNumber === 'number') { @@ -2886,7 +2921,7 @@ export async function getPRForBranchOutcome( } } let mergedBranchLookupNumber: number | null = null - if (await hideMergedImplicitPR(data)) { + if (await hideMergedImplicitPR(data, dataRepo)) { mergedBranchLookupNumber = data?.number ?? null data = null dataRepo = null @@ -2913,14 +2948,15 @@ export async function getPRForBranchOutcome( data.number === fallbackPRNumber const explicitHeadHidesMergedImplicitPR = explicitCurrentHeadOid !== null && - shouldHideMergedImplicitPR(data, linkedPRNumber, explicitCurrentHeadOid) + shouldHideMergedImplicitPR(data, linkedPRNumber, explicitCurrentHeadOid) && + !(await mergedPRContainsHead(data, dataRepo, explicitCurrentHeadOid)) const shouldPreserveMergedFallback = !explicitHeadHidesMergedImplicitPR && (fallbackConfirmedMergedBranch || options.acceptMergedFallbackPR === true) // Why: a currently visible PR can be merged outside Orca; when the caller // 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 ((await hideMergedImplicitPR(data)) && !shouldPreserveMergedFallback) { + if ((await hideMergedImplicitPR(data, dataRepo)) && !shouldPreserveMergedFallback) { return { kind: 'no-pr', fetchedAt: Date.now() } } @@ -2962,6 +2998,7 @@ export async function getPRForBranchOutcome( : {}), ...(data.mergeStateStatus !== undefined ? { mergeStateStatus: data.mergeStateStatus } : {}), headSha: data.headRefOid, + ...(confirmedContainedHeadOid ? { confirmedContainedHeadOid } : {}), ...(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 new file mode 100644 index 00000000000..6ab3f7e39e5 --- /dev/null +++ b/src/main/github/merged-pr-commit-membership.ts @@ -0,0 +1,95 @@ +import { ghExecFileAsync } from './gh-utils' +import { noteRateLimitSpend, rateLimitGuard } from './rate-limit' +import type { OwnerRepo } from './github-repository-identity' + +type GhExecOptions = Parameters[1] + +// Why: a merged PR's commit set is immutable, so confirmations never expire in +// practice; the TTLs only bound memory and let "commit not on GitHub yet" +// answers retry after a future push. +const MEMBERSHIP_CACHE_MAX_ENTRIES = 200 +const MEMBERSHIP_CONFIRMED_TTL_MS = 6 * 60 * 60 * 1000 +const MEMBERSHIP_NEGATIVE_TTL_MS = 5 * 60 * 1000 + +const membershipCache = new Map() + +function pruneMergedPRCommitMembershipCache(now = Date.now()): void { + for (const [cacheKey, cached] of membershipCache) { + if (cached.expiresAt <= now) { + membershipCache.delete(cacheKey) + } + } + while (membershipCache.size > MEMBERSHIP_CACHE_MAX_ENTRIES) { + const oldestKey = membershipCache.keys().next().value + if (oldestKey === undefined) { + break + } + membershipCache.delete(oldestKey) + } +} + +export function resetMergedPRCommitMembershipCacheForTest(): void { + membershipCache.clear() +} + +/** + * Whether `commitOid` is part of pull request `prNumber` on GitHub — i.e. the + * 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. + */ +export async function isCommitPartOfMergedPR(args: { + ownerRepo: OwnerRepo + prNumber: number + commitOid: string + ghOptions: GhExecOptions +}): Promise { + const oid = args.commitOid.trim().toLowerCase() + if (!/^[0-9a-f]{4,64}$/.test(oid) || !Number.isInteger(args.prNumber)) { + return false + } + const owner = args.ownerRepo.owner + const repo = args.ownerRepo.repo + const cacheKey = `${owner.toLowerCase()}/${repo.toLowerCase()}#${args.prNumber}@${oid}` + const now = Date.now() + pruneMergedPRCommitMembershipCache(now) + const cached = membershipCache.get(cacheKey) + 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. + if (rateLimitGuard('core').blocked) { + return false + } + 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( + (entry) => + typeof entry === 'object' && + entry !== null && + (entry as { number?: unknown }).number === args.prNumber + ) + membershipCache.set(cacheKey, { + value, + expiresAt: now + (value ? MEMBERSHIP_CONFIRMED_TTL_MS : MEMBERSHIP_NEGATIVE_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, + expiresAt: now + MEMBERSHIP_NEGATIVE_TTL_MS + }) + return false + } +} diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index bb975bd2158..3a29853a18f 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -36,8 +36,7 @@ import type { Worktree, Repo, IssueInfo, - LinearIssue, - PRInfo + LinearIssue } from '../../../../shared/types' import { CONFLICT_OPERATION_LABELS } from './WorktreeCardHelpers' import { @@ -48,7 +47,10 @@ import { } from './WorktreeCardMeta' import { WorktreeCardPortsDetails, WorktreeCardPortsTrigger } from './WorktreeCardPorts' import { writeWorkspaceDragData } from './workspace-status' -import { getWorktreeCardPrDisplay } from './worktree-card-pr-display' +import { + getWorktreeCardPrDisplay, + isCachedMergedBranchPRCurrentForWorktree +} from './worktree-card-pr-display' import type { WorktreeCardPrDisplay } from './worktree-card-pr-display' import { coerceWorktreeCardVisibleTitle, @@ -153,20 +155,6 @@ function isWebClient(): boolean { return Boolean((window as unknown as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__) } -function isCachedMergedBranchPRCurrentForWorktree( - cachedPR: PRInfo | null | undefined, - worktree: Worktree -): boolean { - return ( - cachedPR?.state === 'merged' && - typeof cachedPR.headSha === 'string' && - cachedPR.headSha.length > 0 && - typeof worktree.head === 'string' && - worktree.head.length > 0 && - cachedPR.headSha === worktree.head - ) -} - function getDirectoryName(folderPath: string): string { const normalized = folderPath.replace(/[\\/]+$/, '') const parts = normalized.split(/[\\/]+/) diff --git a/src/renderer/src/components/sidebar/worktree-card-pr-display.test.ts b/src/renderer/src/components/sidebar/worktree-card-pr-display.test.ts index 4b2896852a2..71702a23b5b 100644 --- a/src/renderer/src/components/sidebar/worktree-card-pr-display.test.ts +++ b/src/renderer/src/components/sidebar/worktree-card-pr-display.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from 'vitest' import type { HostedReviewInfo } from '../../../../shared/hosted-review' -import { getWorktreeCardPrDisplay } from './worktree-card-pr-display' +import type { PRInfo } from '../../../../shared/types' +import { + getWorktreeCardPrDisplay, + isCachedMergedBranchPRCurrentForWorktree +} from './worktree-card-pr-display' const pr: HostedReviewInfo = { provider: 'github', @@ -104,3 +108,38 @@ describe('getWorktreeCardPrDisplay', () => { expect(getWorktreeCardPrDisplay(bitbucketReview, null)).toBe(bitbucketReview) }) }) + +describe('isCachedMergedBranchPRCurrentForWorktree', () => { + const mergedPR: PRInfo = { + number: 55, + title: 'Merged PR', + state: 'merged', + url: 'https://github.com/stablyai/orca/pull/55', + checksStatus: 'success', + updatedAt: '2026-07-03T00:00:00.000Z', + mergeable: 'MERGEABLE', + headSha: 'final-head' + } + + it('matches when the worktree sits exactly on the merged head', () => { + expect(isCachedMergedBranchPRCurrentForWorktree(mergedPR, { head: 'final-head' })).toBe(true) + }) + + it('matches when the worktree head is a confirmed commit of the merged PR', () => { + expect( + isCachedMergedBranchPRCurrentForWorktree( + { ...mergedPR, confirmedContainedHeadOid: 'behind-head' }, + { head: 'behind-head' } + ) + ).toBe(true) + }) + + it('rejects a merged PR when the worktree head is neither the final head nor confirmed', () => { + expect( + isCachedMergedBranchPRCurrentForWorktree( + { ...mergedPR, confirmedContainedHeadOid: 'other-head' }, + { head: 'reused-branch-head' } + ) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-card-pr-display.ts b/src/renderer/src/components/sidebar/worktree-card-pr-display.ts index 406f022902e..1ab80bdf16e 100644 --- a/src/renderer/src/components/sidebar/worktree-card-pr-display.ts +++ b/src/renderer/src/components/sidebar/worktree-card-pr-display.ts @@ -1,7 +1,24 @@ import type { HostedReviewInfo } from '../../../../shared/hosted-review' +import type { PRInfo, Worktree } from '../../../../shared/types' type LinkedReviewMetadataProvider = Exclude +export function isCachedMergedBranchPRCurrentForWorktree( + cachedPR: PRInfo | null | undefined, + worktree: Pick +): boolean { + return ( + cachedPR?.state === 'merged' && + typeof cachedPR.headSha === 'string' && + cachedPR.headSha.length > 0 && + typeof worktree.head === 'string' && + worktree.head.length > 0 && + // Why: a worktree behind its own merged PR (update-branch/web commits) is + // still that PR's line of work; match the main-process visibility rule. + (cachedPR.headSha === worktree.head || cachedPR.confirmedContainedHeadOid === worktree.head) + ) +} + type LinkedReviewNumbers = { linkedPR: number | null linkedGitLabMR: number | null diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index aed7dcad6b1..318eebaa5f1 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -2264,6 +2264,55 @@ describe('createGitHubSlice.fetchPRForBranch', () => { expect(mockApi.cache.setGitHub).not.toHaveBeenCalled() }) + it('preserves cached merged PR data when the worktree head is a confirmed PR commit', async () => { + vi.useFakeTimers() + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/merged-pr-behind-head' + const worktreeId = 'wt-merged-behind-head' + const cachedPR = makePR({ + number: 13, + title: 'Merged PR with unpulled final head', + state: 'merged', + headSha: 'merged-final-head', + confirmedContainedHeadOid: 'behind-head' + }) + + store.setState({ + repos: [{ id: repoId, path: repoPath, name: 'repo', kind: 'git' }], + worktreesByRepo: { + [repoId]: [ + makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'behind-head' + }) + ] + }, + prCache: { + [`${repoId}::${branch}`]: { + data: cachedPR, + fetchedAt: 1 + } + } + } as unknown as Partial) + mockApi.gh.refreshPRNow.mockResolvedValueOnce({ kind: 'no-pr', fetchedAt: 2 }) + + await expect( + store.getState().fetchPRForBranch(repoPath, branch, { + force: true, + repoId, + worktreeId + }) + ).resolves.toEqual(cachedPR) + expect(store.getState().prCache[`${repoId}::${branch}`]).toEqual({ + data: cachedPR, + fetchedAt: 1 + }) + }) + it.each([ { name: 'worktree id is missing', @@ -3482,6 +3531,53 @@ describe('createGitHubSlice.refreshGitHubForWorktreeIfStale', () => { expect(mockApi.gh.prForBranch).not.toHaveBeenCalled() }) + it('keeps a confirmed behind-head merged PR as the refresh fallback number', async () => { + const store = createTestStore() + const repoPath = '/repo' + const repoId = 'repo-1' + const branch = 'feature/merged-pr-behind-head' + const worktreeId = 'wt-merged-behind-fallback' + mockApi.gh.enqueuePRRefresh.mockResolvedValueOnce({ kind: 'queued' }) + + store.setState({ + repos: [{ id: repoId, path: repoPath, name: 'repo', kind: 'git' }], + worktreesByRepo: { + [repoId]: [ + makePRRefreshWorktree({ + id: worktreeId, + repoId, + branch, + head: 'behind-head' + }) + ] + }, + prCache: { + [`${repoId}::${branch}`]: { + data: makePR({ + number: 13, + title: 'Merged PR with unpulled final head', + state: 'merged', + headSha: 'merged-final-head', + confirmedContainedHeadOid: 'behind-head' + }), + fetchedAt: 1 + } + } + } as unknown as Partial) + + store.getState().enqueueGitHubPRRefresh(worktreeId, 'active', 80) + await Promise.resolve() + + // Why 13: a merged PR confirmed to contain this worktree head is still the + // branch's PR; losing the fallback number would blank the panel whenever + // GitHub stops reporting the deleted head by branch name. + expect(mockApi.gh.enqueuePRRefresh).toHaveBeenCalledWith( + expect.objectContaining({ + candidate: expect.objectContaining({ fallbackPRNumber: 13 }) + }) + ) + }) + it('direct-fetches when enqueue returns an explicit fallback result', async () => { 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 c0f73e621d6..718f4d9b29d 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -1039,7 +1039,10 @@ function buildPRRefreshCandidate( ) const cachedFallbackPRNumber = cachedPR?.number ?? null const cachedMergedPRMovedPastHead = - worktree.linkedPR == null && cachedPR?.state === 'merged' && cachedPR.headSha !== worktree.head + worktree.linkedPR == null && + cachedPR?.state === 'merged' && + cachedPR.headSha !== worktree.head && + cachedPR.confirmedContainedHeadOid !== worktree.head const fallbackPRNumber = worktree.linkedPR == null && !cachedMergedPRMovedPastHead ? (cachedFallbackPRNumber ?? hostedReviewFallbackPRNumber) @@ -1264,7 +1267,8 @@ function shouldPreserveExistingPRForFallbackMiss(args: { const worktree = args.worktreeId ? findWorktreeById(args.state, args.worktreeId) : null const worktreeHead = worktree?.head // Why: merged branch PRs are only safe to keep when cached PR metadata still - // matches the commit this stored worktree is actually on. + // matches the commit this stored worktree is actually on — exactly, or via a + // head main confirmed to be part of the merged PR. const preservesMergedPRForCurrentHead = args.nextPR === null && args.linkedPRNumber == null && @@ -1273,7 +1277,8 @@ function shouldPreserveExistingPRForFallbackMiss(args: { args.currentPR.headSha.length > 0 && typeof worktreeHead === 'string' && worktreeHead.length > 0 && - args.currentPR.headSha === worktreeHead + (args.currentPR.headSha === worktreeHead || + args.currentPR.confirmedContainedHeadOid === worktreeHead) // Why: fallback PR numbers come from already-visible cache, not durable // worktree metadata. A branch/fallback miss is weaker than the current exact diff --git a/src/renderer/src/store/slices/hosted-review.test.ts b/src/renderer/src/store/slices/hosted-review.test.ts index 8a07ede5595..7bd6c4f3dda 100644 --- a/src/renderer/src/store/slices/hosted-review.test.ts +++ b/src/renderer/src/store/slices/hosted-review.test.ts @@ -547,6 +547,39 @@ describe('hosted review slice', () => { expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(2) }) + it('serves a cached merged review whose confirmed contained head matches the worktree', async () => { + const mergedBehindHead: HostedReviewInfo = { + provider: 'github', + number: 7, + title: 'Merged with unpulled final head', + state: 'merged', + url: 'https://github.com/acme/orca/pull/7', + status: 'success', + updatedAt: '2026-05-10T00:00:00.000Z', + mergeable: 'MERGEABLE', + headSha: 'aaaaaaa', + confirmedContainedHeadOid: 'bbbbbbb' + } + mockApi.hostedReview.forBranch.mockResolvedValueOnce(mergedBehindHead) + const store = makeStore() + + await expect( + store.getState().fetchHostedReviewForBranch('/repo', 'feature/merged', { + currentHeadOid: 'bbbbbbb' + }) + ).resolves.toEqual(mergedBehindHead) + + // Why one call: a merged review confirmed for this worktree head is not + // stale, so the second read must reuse the branch-scoped cache instead of + // refetching every poll. + await expect( + store.getState().fetchHostedReviewForBranch('/repo', 'feature/merged', { + currentHeadOid: 'bbbbbbb' + }) + ).resolves.toEqual(mergedBehindHead) + expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(1) + }) + it('does not preserve a merged GitHub review after the worktree moves off its head', async () => { const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) const cacheKey = getHostedReviewCacheKey( diff --git a/src/renderer/src/store/slices/hosted-review.ts b/src/renderer/src/store/slices/hosted-review.ts index 710bdcc2233..a638d964c57 100644 --- a/src/renderer/src/store/slices/hosted-review.ts +++ b/src/renderer/src/store/slices/hosted-review.ts @@ -100,9 +100,10 @@ function isStaleMergedGitHubReviewForHead( cached: CacheEntry | undefined, currentHeadOid: string | null | undefined ): boolean { - // Why: a merged GitHub PR is only shown when the worktree sits on its head. - // The cache key is branch-scoped, so a worktree that advanced off the merged - // head must not reuse (or, on failure, preserve) the now-stale merged review. + // Why: a merged GitHub PR is only shown when the worktree sits on its head + // or on a commit confirmed to be part of the PR. The cache key is + // branch-scoped, so a worktree that advanced off the merged line of work + // must not reuse (or, on failure, preserve) the now-stale merged review. const head = typeof currentHeadOid === 'string' ? currentHeadOid.trim() : '' if (head.length === 0) { return false @@ -113,7 +114,8 @@ function isStaleMergedGitHubReviewForHead( data.state === 'merged' && typeof data.headSha === 'string' && data.headSha.length > 0 && - data.headSha !== head + data.headSha !== head && + data.confirmedContainedHeadOid !== head ) } diff --git a/src/shared/hosted-review-github.ts b/src/shared/hosted-review-github.ts index 1e376003223..e7db9111f03 100644 --- a/src/shared/hosted-review-github.ts +++ b/src/shared/hosted-review-github.ts @@ -120,6 +120,9 @@ export function hostedReviewInfoFromGitHubPRInfo(pr: PRInfo): HostedReviewInfo { ...(pr.mergeQueueRequired !== undefined ? { mergeQueueRequired: pr.mergeQueueRequired } : {}), ...(pr.mergeStateStatus !== undefined ? { mergeStateStatus: pr.mergeStateStatus } : {}), ...(pr.headSha ? { headSha: pr.headSha } : {}), + ...(pr.confirmedContainedHeadOid + ? { confirmedContainedHeadOid: pr.confirmedContainedHeadOid } + : {}), ...(pr.conflictSummary ? { conflictSummary: pr.conflictSummary } : {}) } } diff --git a/src/shared/hosted-review.ts b/src/shared/hosted-review.ts index 9a403cb80f5..7bf3dd24181 100644 --- a/src/shared/hosted-review.ts +++ b/src/shared/hosted-review.ts @@ -25,6 +25,9 @@ export type HostedReviewInfo = { mergeQueueRequired?: boolean | null mergeStateStatus?: string | null headSha?: string + // Why: mirrors PRInfo.confirmedContainedHeadOid so merged-review staleness + // checks accept a worktree head confirmed to be part of the merged PR. + confirmedContainedHeadOid?: string /** Target branch name for review-created worktree compare-base repair. */ baseRefName?: string conflictSummary?: PRConflictSummary diff --git a/src/shared/types.ts b/src/shared/types.ts index a17889b49cf..ffd0f8e4c9c 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -1133,6 +1133,10 @@ export type PRInfo = { // Keeping the head SHA in cached PR metadata lets the checks panel poll the // correct commit without re-querying GitHub or guessing from local branch refs. headSha?: string + // Why: a merged branch-matched PR stays visible when the worktree head is one + // of the PR's own commits (behind update-branch/web commits). Cache staleness + // checks must honor that confirmation without re-querying GitHub. + confirmedContainedHeadOid?: string /** Target branch name for PR-created worktree compare-base repair. */ baseRefName?: string prRepo?: GitHubRepositoryIdentity