diff --git a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-context-state.tsx b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-context-state.tsx index 5ee0f4e891f..9e02228cbaf 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-context-state.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-context-state.tsx @@ -4,6 +4,7 @@ import { useNow } from '@/hooks/use-now' import { isFolderRepo } from '../../../../../shared/repo-kind' import { getGitHubPRCacheKey } from '../../../../../shared/github/github-cache-key' import { getHostedReviewCacheKey } from '@/store/slices/hosted-review-cache-identity' +import { isStaleMergedGitHubReviewForHead } from '@/store/slices/hosted-review-cache-state' import { selectReviewCacheEntry } from '../review-cache-entry-selection' import { selectChecksPanelReview, type ChecksPanelReview } from '../checks-panel-review' import { isGitLabChecksPanelReview } from './gitlab-review-client' @@ -198,8 +199,10 @@ export function useChecksPanelContextState(model: ChecksPanelContextStateInput) } // Why: background PR refreshes replace the cache map; Checks only renders the entry for the active repo and branch. const prCacheEntry = useAppStore((s) => selectReviewCacheEntry(s.prCache, prCacheKey || null)) - const pr: PRInfo | null = prCacheEntry?.data ?? null - const prCachedHasPR = prCacheEntry ? prCacheEntry.data !== null : null + const pr: PRInfo | null = isStaleMergedGitHubReviewForHead(prCacheEntry, activeWorktree?.head) + ? null + : (prCacheEntry?.data ?? null) + const prCachedHasPR = prCacheEntry ? pr !== null : null const hostedReview = useAppStore((s) => hostedReviewCacheKey ? (s.hostedReviewCache[hostedReviewCacheKey]?.data ?? null) : null ) diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts index 6d4af405aff..fbca4f3c40c 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts @@ -115,6 +115,25 @@ function makeProjection({ } describe('buildParentPrChecksProjection', () => { + it('drops a completed merged lookup when its worktree has moved to another head', () => { + const repo = makeRepo() + const worktree = makeWorktree({ id: 'repo-1::/feature', head: 'merged-head' }) + const identity = getParentPrChecksRefreshIdentity(worktree, repo, 'feature') + const review = makeReview({ state: 'merged', headSha: 'merged-head' }) + const refreshOutcomes = new Map([ + [identity, { kind: 'found', review }] + ]) + + expect(makeProjection({ worktree, repo, refreshOutcomes }).rows[0].reviewNumber).toBe(12) + expect( + makeProjection({ + worktree: { ...worktree, head: 'new-work-head' }, + repo, + refreshOutcomes + }).rows[0].reviewNumber + ).toBeNull() + }) + it('classifies known review states into compact row groups', () => { const repo = makeRepo() const worktree = makeWorktree({ id: 'repo-1::/feature' }) diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts index cb17cbba194..82ae59d9bdf 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts @@ -13,6 +13,7 @@ import { linkedReviewHintKey } from '@/store/slices/hosted-review-cache-identity' import { prChecksCacheSuffix } from '@/store/github/cache-identity' +import { isStaleMergedGitHubReviewForHead } from '@/store/slices/hosted-review-cache-state' import { PARENT_PR_CHECKS_GROUP_LABELS, PARENT_PR_CHECKS_GROUP_ORDER, @@ -158,6 +159,7 @@ function getReviewSnapshot( ): { review: HostedReviewInfo | null | undefined; hasCacheEntry: boolean } { if ( outcome?.kind === 'found' && + !isStaleMergedGitHubReviewForHead({ data: outcome.review }, args.worktree.head) && (outcome.review.provider !== 'github' || !isGitHubPRSuppressed(args.worktree, outcome.review.number)) ) { diff --git a/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.test.tsx b/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.test.tsx index 07e101f1a38..718a2e7be24 100644 --- a/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.test.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.test.tsx @@ -23,11 +23,13 @@ function makePR(number = 42): PRInfo { function renderState({ pr = makePR(), hostedReviewEntryData = null, + currentHeadOid, linkedPR = null, suppressedGitHubPR = null }: { pr?: PRInfo | null hostedReviewEntryData?: HostedReviewInfo | null + currentHeadOid?: string | null linkedPR?: number | null suppressedGitHubPR?: number | null } = {}) { @@ -39,6 +41,7 @@ function renderState({ branchName: 'feature', hostedReviewCacheKey: 'cache-key', hostedReviewEntryData, + currentHeadOid, linkedPR, suppressedGitHubPR }) @@ -46,6 +49,71 @@ function renderState({ } describe('useSourceControlHostedReviewState', () => { + it.each(['queue', 'hosted'] as const)( + 'hides a merged %s review as the worktree moves to another head', + (source) => { + const pr = { ...makePR(), state: 'merged' as const, headSha: 'merged-head' } + const review: HostedReviewInfo = { provider: 'github', ...pr, status: pr.checksStatus } + const { result, rerender } = renderHook( + ({ head }) => + useSourceControlHostedReviewState({ + activePrFromQueue: source === 'queue' ? pr : null, + activeRepoId: 'repo-1', + activeWorktreeId: 'wt-1', + branchName: 'feature', + hostedReviewCacheKey: 'cache-key', + hostedReviewEntryData: source === 'hosted' ? review : null, + currentHeadOid: head, + linkedPR: null, + suppressedGitHubPR: null + }), + { initialProps: { head: 'merged-head' } } + ) + + expect(result.current.hostedReview?.number).toBe(42) + rerender({ head: 'new-work-head' }) + expect(result.current.hostedReview).toBeNull() + expect(result.current.currentPrFromQueue).toBeNull() + rerender({ head: 'merged-head' }) + expect(result.current.hostedReview?.number).toBe(42) + } + ) + + it.each([undefined, null, '', ' ', 'merged-head', 'contained-head'])( + 'preserves a merged review for an unknown or owned head %s', + (currentHeadOid) => { + const pr = { + ...makePR(), + state: 'merged' as const, + headSha: 'merged-head', + confirmedContainedHeadOid: 'contained-head' + } + expect(renderState({ pr, currentHeadOid }).hostedReview?.number).toBe(42) + } + ) + + it('keeps a valid hosted review while an older queued PR belongs to another head', () => { + const currentReview: HostedReviewInfo = { + provider: 'github', + ...makePR(43), + status: 'pending', + headSha: 'new-work-head' + } + const state = renderState({ + pr: { ...makePR(), state: 'merged', headSha: 'merged-head' }, + hostedReviewEntryData: currentReview, + currentHeadOid: 'new-work-head' + }) + + expect(state.hostedReview).toBe(currentReview) + expect(state.currentPrFromQueue).toBeNull() + expect(renderState({ currentHeadOid: 'new-work-head' }).hostedReview?.number).toBe(42) + expect( + renderState({ pr: { ...makePR(), state: 'merged' }, currentHeadOid: 'new-work-head' }) + .hostedReview?.number + ).toBe(42) + }) + it('uses MR header copy while eligibility loads for a GitLab SSH-over-443 remote', () => { const { result } = renderHook(() => { const state = useSourceControlHostedReviewState({ diff --git a/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.ts b/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.ts index 94ef562ae85..062d7d333cb 100644 --- a/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.ts +++ b/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.ts @@ -3,6 +3,7 @@ import { localizedHostedReviewCopy } from '@/i18n/hosted-review-localized-copy' import type { HostedReviewInfo } from '../../../../../../shared/hosted-review' import { isGitHubPRSuppressed } from '../../../../../../shared/worktree/github-pr-suppression' import { resolveHostedReviewCreationProvider } from '../../../../../../shared/hosted-review-creation-providers' +import { isStaleMergedGitHubReviewForHead } from '@/store/slices/hosted-review-cache-state' import type { SourceControlWorktreeContext } from '../listing/use-worktree-context' import type { HostedReviewCreationProviderHint, @@ -19,6 +20,7 @@ export function useSourceControlHostedReviewState({ activeRepoId, activeWorktreeId, branchName, + currentHeadOid, hostedReviewCacheKey, hostedReviewEntryData, linkedPR, @@ -28,6 +30,7 @@ export function useSourceControlHostedReviewState({ activeRepoId: string | null activeWorktreeId: string | null branchName: string + currentHeadOid?: string | null hostedReviewCacheKey: string | null hostedReviewEntryData: SourceControlWorktreeContext['hostedReviewEntryData'] linkedPR: number | null @@ -51,15 +54,27 @@ export function useSourceControlHostedReviewState({ branchName === hostedReviewCreationState.branch ? hostedReviewCreationState.data : null + const currentPrFromQueue = isStaleMergedGitHubReviewForHead( + { data: activePrFromQueue }, + currentHeadOid + ) + ? null + : activePrFromQueue + const currentHostedReview = isStaleMergedGitHubReviewForHead( + { data: hostedReviewEntryData }, + currentHeadOid + ) + ? null + : hostedReviewEntryData const rawHostedReview: HostedReviewInfo | null = useMemo(() => { if (!hostedReviewCacheKey) { return null } - if (activePrFromQueue) { - return { provider: 'github', ...activePrFromQueue, status: activePrFromQueue.checksStatus } + if (currentPrFromQueue) { + return { provider: 'github', ...currentPrFromQueue, status: currentPrFromQueue.checksStatus } } - return hostedReviewEntryData - }, [activePrFromQueue, hostedReviewCacheKey, hostedReviewEntryData]) + return currentHostedReview + }, [currentPrFromQueue, hostedReviewCacheKey, currentHostedReview]) const hasSuppressedGitHubPR = rawHostedReview?.provider === 'github' && isGitHubPRSuppressed({ linkedPR, suppressedGitHubPR }, rawHostedReview.number) @@ -81,6 +96,7 @@ export function useSourceControlHostedReviewState({ const hostedReviewCreateCopy = localizedHostedReviewCopy(hostedReviewCreateProvider) return { + currentPrFromQueue, hasSuppressedGitHubPR, hostedReview, hostedReviewCreateCopy, diff --git a/src/renderer/src/components/right-sidebar/source-control/review/use-review-context.ts b/src/renderer/src/components/right-sidebar/source-control/review/use-review-context.ts index 61857afa30f..5a175881f0d 100644 --- a/src/renderer/src/components/right-sidebar/source-control/review/use-review-context.ts +++ b/src/renderer/src/components/right-sidebar/source-control/review/use-review-context.ts @@ -44,10 +44,12 @@ export function useSourceControlReviewContext(panelState: SourceControlPanelStat branchName, hostedReviewCacheKey, hostedReviewEntryData, + currentHeadOid: activeWorktree?.head, linkedPR: activeWorktree?.linkedPR ?? null, suppressedGitHubPR: activeWorktree?.suppressedGitHubPR ?? null }) const { + currentPrFromQueue, hasSuppressedGitHubPR, hostedReview, hostedReviewCreation, @@ -88,7 +90,7 @@ export function useSourceControlReviewContext(panelState: SourceControlPanelStat worktreePath }) const linkedReviews = useSourceControlLinkedReviews({ - activePrFromQueue: hasSuppressedGitHubPR ? null : activePrFromQueue, + activePrFromQueue: hasSuppressedGitHubPR ? null : currentPrFromQueue, activeRepo, activeWorktree, branchName, diff --git a/src/renderer/src/store/slices/hosted-review-cache-state.ts b/src/renderer/src/store/slices/hosted-review-cache-state.ts index a00ce7dca28..27e2801fa8e 100644 --- a/src/renderer/src/store/slices/hosted-review-cache-state.ts +++ b/src/renderer/src/store/slices/hosted-review-cache-state.ts @@ -6,6 +6,7 @@ import type { HostedReviewInfo } from '../../../../shared/hosted-review' import type { Repo } from '../../../../shared/repo-types' +import type { PRInfo } from '../../../../shared/github/pull-request-types' import { getRepoExecutionHostId } from '../../../../shared/execution-host' import { ipcOrRuntimeTargetForHostId } from '@/runtime/runtime-client-target' import type { RuntimeClientTarget } from '../../../../shared/runtime-client-target' @@ -148,7 +149,7 @@ export function shouldRefetchGitHubScopedResultForNoHint( } export function isStaleMergedGitHubReviewForHead( - cached: HostedReviewCacheEntry | undefined, + cached: { data: HostedReviewInfo | PRInfo | null } | undefined, currentHeadOid: string | null | undefined ): boolean { // Why: a merged GitHub PR is only shown when the worktree sits on its head @@ -161,7 +162,9 @@ export function isStaleMergedGitHubReviewForHead( } const data = cached?.data return ( - data?.provider === 'github' && + data !== null && + data !== undefined && + (!('provider' in data) || data.provider === 'github') && data.state === 'merged' && typeof data.headSha === 'string' && data.headSha.length > 0 && diff --git a/src/renderer/src/store/slices/hosted-review-request-state.ts b/src/renderer/src/store/slices/hosted-review-request-state.ts index 15621e378bf..1398e221887 100644 --- a/src/renderer/src/store/slices/hosted-review-request-state.ts +++ b/src/renderer/src/store/slices/hosted-review-request-state.ts @@ -9,6 +9,7 @@ export const inflightHostedReviewRequests = new Map< { promise: Promise force: boolean + currentHeadOid: string | null generation: number startedAt: number } @@ -177,6 +178,7 @@ export function registerInflightHostedReviewRequest( entry: { promise: Promise force: boolean + currentHeadOid: string | null generation: number startedAt: number } diff --git a/src/renderer/src/store/slices/hosted-review.test.ts b/src/renderer/src/store/slices/hosted-review.test.ts index 897cfeff50d..d88be4fadcd 100644 --- a/src/renderer/src/store/slices/hosted-review.test.ts +++ b/src/renderer/src/store/slices/hosted-review.test.ts @@ -712,6 +712,77 @@ describe('hosted review slice', () => { expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(2) }) + it('revalidates instead of serving a stale merged review on the stale-while-revalidate path', async () => { + const mergedAtHead: HostedReviewInfo = { + provider: 'github', + number: 7, + title: 'Merged at head', + state: 'merged', + url: 'https://github.com/acme/orca/pull/7', + status: 'success', + updatedAt: '2026-05-10T00:00:00.000Z', + mergeable: 'MERGEABLE', + headSha: 'aaaaaaa' + } + mockApi.hostedReview.forBranch.mockResolvedValueOnce(mergedAtHead).mockResolvedValueOnce(null) + const store = makeStore() + + await expect( + store.getState().fetchHostedReviewForBranch('/repo', 'feature/merged', { + currentHeadOid: 'aaaaaaa' + }) + ).resolves.toEqual(mergedAtHead) + + // Advancing the branch must bypass even the fresh stale-while-revalidate cache. + await expect( + store.getState().fetchHostedReviewForBranch('/repo', 'feature/merged', { + staleWhileRevalidate: true, + currentHeadOid: 'bbbbbbb' + }) + ).resolves.toBeNull() + expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(2) + }) + + it('does not reuse an old-head request after the branch advances', async () => { + const mergedAtHead: HostedReviewInfo = { + ...githubReview, + state: 'merged', + headSha: 'aaaaaaa' + } + let resolveOldHead!: (value: HostedReviewInfo | null) => void + mockApi.hostedReview.forBranch + .mockResolvedValueOnce(mergedAtHead) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveOldHead = resolve + }) + ) + .mockResolvedValueOnce(null) + const store = makeStore() + const fetch = store.getState().fetchHostedReviewForBranch + await fetch('/repo', 'feature/inflight-head', { currentHeadOid: 'aaaaaaa' }) + const oldFetch = fetch('/repo', 'feature/inflight-head', { + force: true, + currentHeadOid: 'aaaaaaa' + }) + const newFetch = fetch('/repo', 'feature/inflight-head', { + staleWhileRevalidate: true, + currentHeadOid: 'bbbbbbb' + }) + resolveOldHead(mergedAtHead) + + await expect(newFetch).resolves.toBeNull() + await expect(oldFetch).resolves.toEqual(mergedAtHead) + expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(3) + await expect( + fetch('/repo', 'feature/inflight-head', { + currentHeadOid: 'bbbbbbb' + }) + ).resolves.toBeNull() + expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(3) + }) + it('serves a cached merged review whose confirmed contained head matches the worktree', async () => { const mergedBehindHead: HostedReviewInfo = { provider: 'github', diff --git a/src/renderer/src/store/slices/hosted-review.ts b/src/renderer/src/store/slices/hosted-review.ts index f04e82c5f19..00b106f5380 100644 --- a/src/renderer/src/store/slices/hosted-review.ts +++ b/src/renderer/src/store/slices/hosted-review.ts @@ -181,7 +181,15 @@ export const createHostedReviewSlice: StateCreator => { const generation = nextLookupGeneration() const requestStartedAt = Date.now() @@ -287,6 +295,7 @@ export const createHostedReviewSlice: StateCreator