diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 9f54c3c8470..1ffcaa6c2cb 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -14,6 +14,7 @@ import { } from 'lucide-react' import { useAppStore, type AppState } from '@/store' import { + getGitHubPRRefreshStateExpiryAt, mergePRCommentIntoList, prChecksCacheSuffix, prCommentsCacheSuffix @@ -94,6 +95,7 @@ import { getChecksPanelEmptyStateCopy, shouldShowChecksPanelPublishBranchAction } from './checks-panel-empty-state' +import { hasAmbiguousGitHubHostedReviewForChecksPanel } from './checks-panel-ambiguous-github-review' import { cancelRuntimeGeneratePullRequestFields, generateRuntimePullRequestFields, @@ -372,6 +374,7 @@ export default function ChecksPanel(): React.JSX.Element { const prCache = useAppStore((s) => s.prCache) const fetchPRForBranch = useAppStore((s) => s.fetchPRForBranch) const fetchHostedReviewForBranch = useAppStore((s) => s.fetchHostedReviewForBranch) + const expireGitHubPRRefreshState = useAppStore((s) => s.expireGitHubPRRefreshState) const getHostedReviewCreationEligibility = useAppStore( (s) => s.getHostedReviewCreationEligibility ) @@ -559,6 +562,7 @@ export default function ChecksPanel(): React.JSX.Element { // Done during render (not useEffect) so the reset takes effect on the same // paint as the context change; useEffect would leave one stale render. const [prevPanelContextKey, setPrevPanelContextKey] = useState(panelContextKey) + const [prRefreshStateNow, setPrRefreshStateNow] = useState(() => Date.now()) if (panelContextKey !== prevPanelContextKey) { setPrevPanelContextKey(panelContextKey) setEditingTitle(false) @@ -572,6 +576,7 @@ export default function ChecksPanel(): React.JSX.Element { setIsRefreshing(false) setEmptyRefreshing(false) setConflictDetailsRefreshing(false) + setPrRefreshStateNow(Date.now()) createPrInFlightRef.current = null setIsCreatingPr(false) setCreatePrError(null) @@ -619,10 +624,16 @@ export default function ChecksPanel(): React.JSX.Element { refreshContextKeyRef.current = refreshContextKey refreshRequestKeyRef.current = null } - const pr: PRInfo | null = prCacheKey ? (prCache[prCacheKey]?.data ?? null) : null + const prCacheEntry = prCacheKey ? prCache[prCacheKey] : undefined + const pr: PRInfo | null = prCacheEntry?.data ?? null const hostedReview = useAppStore((s) => hostedReviewCacheKey ? (s.hostedReviewCache[hostedReviewCacheKey]?.data ?? null) : null ) + const hasAmbiguousGitHubHostedReview = hasAmbiguousGitHubHostedReviewForChecksPanel({ + hostedReview, + prCacheEntry, + prCacheKey + }) // Fetch PR data when the active worktree/branch changes. // Why: branch lookup is lossy for fork/deleted-head PRs; reuse a known PR // number from metadata or the visible cache whenever we have one. @@ -640,10 +651,36 @@ export default function ChecksPanel(): React.JSX.Element { const isGitLabReviewContext = Boolean(activeGitLabReview || linkedGitLabMR !== null) const activeConflictReview = activeReview?.mergeable === 'CONFLICTING' ? activeReview : null const prRefreshState = useAppStore((s) => - prCacheKey ? s.prRefreshStates[prCacheKey] : undefined + prCacheKey ? s.getEffectiveGitHubPRRefreshState(prCacheKey, prRefreshStateNow) : undefined ) const prNumber = pr?.number ?? null + useEffect(() => { + const expiryAt = getGitHubPRRefreshStateExpiryAt(prRefreshState) + if (!prCacheKey || expiryAt === null) { + return + } + const timeout = window.setTimeout( + () => { + setPrRefreshStateNow(Date.now()) + const storeState = useAppStore.getState() + const rawState = storeState.prRefreshStates[prCacheKey] + if (!rawState) { + return + } + // Why: time alone does not publish Zustand updates; this timeout clears + // abandoned active refresh UI without treating expiry as no-PR evidence. + storeState.expireGitHubPRRefreshState(prCacheKey, { + sequence: storeState.prRefreshSequences[prCacheKey] ?? 0, + status: rawState.status, + updatedAt: rawState.updatedAt + }) + }, + Math.max(0, expiryAt - Date.now() + 1) + ) + return () => window.clearTimeout(timeout) + }, [prCacheKey, prRefreshState]) + // Why: select only timestamps (not whole cache records) so the entry-refresh // effect doesn't re-run on every cache mutation. See // docs/refresh-on-checks-tab.md. @@ -1807,13 +1844,29 @@ export default function ChecksPanel(): React.JSX.Element { } return } - const refreshedPR = await fetchPRForBranch(repo.path, branch, { - force: true, - repoId: repo.id, - worktreeId: activeWorktreeId ?? undefined, - linkedPRNumber: linkedPR, - fallbackPRNumber: fallbackGitHubPRNumber - }) + const refreshStoreState = useAppStore.getState() + const rawPRRefreshState = refreshStoreState.prRefreshStates[prCacheKey] + const startedPRRefreshToken = rawPRRefreshState + ? { + sequence: refreshStoreState.prRefreshSequences[prCacheKey] ?? 0, + status: rawPRRefreshState.status, + updatedAt: rawPRRefreshState.updatedAt + } + : null + let refreshedPR: PRInfo | null = null + try { + refreshedPR = await fetchPRForBranch(repo.path, branch, { + force: true, + repoId: repo.id, + worktreeId: activeWorktreeId ?? undefined, + linkedPRNumber: linkedPR, + fallbackPRNumber: fallbackGitHubPRNumber + }) + } finally { + if (startedPRRefreshToken) { + expireGitHubPRRefreshState(prCacheKey, startedPRRefreshToken) + } + } if (!isCurrentRequest()) { return } @@ -1941,6 +1994,7 @@ export default function ChecksPanel(): React.JSX.Element { fetchPRChecks, fetchPRComments, fetchHostedReviewForBranch, + expireGitHubPRRefreshState, isCurrentAsyncResult ]) @@ -3297,7 +3351,8 @@ export default function ChecksPanel(): React.JSX.Element { hasUpstream: publishActionRemoteStatus?.hasUpstream, hasCurrentBranch: Boolean(branch), reviewLabel: emptyReviewLabel, - reviewShortLabel: emptyReviewShortLabel + reviewShortLabel: emptyReviewShortLabel, + hasAmbiguousGitHubHostedReview }) return (
diff --git a/src/renderer/src/components/right-sidebar/checks-panel-ambiguous-github-review.test.ts b/src/renderer/src/components/right-sidebar/checks-panel-ambiguous-github-review.test.ts new file mode 100644 index 00000000000..db51fd6ef33 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/checks-panel-ambiguous-github-review.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' +import { hasAmbiguousGitHubHostedReviewForChecksPanel } from './checks-panel-ambiguous-github-review' + +describe('hasAmbiguousGitHubHostedReviewForChecksPanel', () => { + it('treats missing PR cache as ambiguous when a GitHub hosted review exists', () => { + expect( + hasAmbiguousGitHubHostedReviewForChecksPanel({ + hostedReview: { provider: 'github' }, + prCacheEntry: undefined, + prCacheKey: 'repo::feature' + }) + ).toBe(true) + }) + + it('treats cached no-PR data as ambiguous when a GitHub hosted review exists', () => { + expect( + hasAmbiguousGitHubHostedReviewForChecksPanel({ + hostedReview: { provider: 'github' }, + prCacheEntry: { data: null }, + prCacheKey: 'repo::feature' + }) + ).toBe(true) + }) + + it('does not treat existing PR cache data as ambiguous', () => { + expect( + hasAmbiguousGitHubHostedReviewForChecksPanel({ + hostedReview: { provider: 'github' }, + prCacheEntry: { data: { number: 12 } }, + prCacheKey: 'repo::feature' + }) + ).toBe(false) + }) + + it('does not treat non-GitHub hosted reviews as ambiguous', () => { + expect( + hasAmbiguousGitHubHostedReviewForChecksPanel({ + hostedReview: { provider: 'gitlab' }, + prCacheEntry: { data: null }, + prCacheKey: 'repo::feature' + }) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/checks-panel-ambiguous-github-review.ts b/src/renderer/src/components/right-sidebar/checks-panel-ambiguous-github-review.ts new file mode 100644 index 00000000000..3444abee022 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/checks-panel-ambiguous-github-review.ts @@ -0,0 +1,19 @@ +type HostedReviewProviderLike = { + provider?: string | null +} + +type PRCacheEntryLike = { + data?: unknown +} + +export function hasAmbiguousGitHubHostedReviewForChecksPanel(input: { + hostedReview: HostedReviewProviderLike | null | undefined + prCacheEntry: PRCacheEntryLike | null | undefined + prCacheKey: string +}): boolean { + return ( + input.hostedReview?.provider === 'github' && + input.prCacheKey !== '' && + input.prCacheEntry?.data == null + ) +} diff --git a/src/renderer/src/components/right-sidebar/checks-panel-empty-state.test.ts b/src/renderer/src/components/right-sidebar/checks-panel-empty-state.test.ts index d81e46351af..40738435ac6 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-empty-state.test.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-empty-state.test.ts @@ -104,6 +104,81 @@ describe('getChecksPanelEmptyStateCopy', () => { description: 'Create a merge request to start checks and review.' }) }) + + it('uses neutral copy when GitHub hosted-review data exists without PR cache', () => { + expect( + getChecksPanelEmptyStateCopy({ + operationLabel: null, + prRefreshStatus: undefined, + hostedReviewBlockedReason: null, + hasUpstream: true, + hasAmbiguousGitHubHostedReview: true + }) + ).toEqual({ + title: 'Pull request status unavailable', + description: 'Refresh GitHub status for this branch to load checks and review.' + }) + }) + + it('does not show no-PR copy for paused ambiguous GitHub hosted-review data', () => { + expect( + getChecksPanelEmptyStateCopy({ + operationLabel: null, + prRefreshStatus: 'paused', + hostedReviewBlockedReason: null, + hasUpstream: true, + hasAmbiguousGitHubHostedReview: true + }).title + ).toBe('Pull request status unavailable') + }) + + it('keeps refresh errors for ambiguous GitHub hosted-review data', () => { + expect( + getChecksPanelEmptyStateCopy({ + operationLabel: null, + prRefreshStatus: 'error', + hostedReviewBlockedReason: null, + hasUpstream: true, + hasAmbiguousGitHubHostedReview: true + }).title + ).toBe('Could not refresh pull request') + }) + + it('uses neutral copy for ambiguous GitHub hosted-review data before unpublished branch copy', () => { + expect( + getChecksPanelEmptyStateCopy({ + operationLabel: null, + prRefreshStatus: undefined, + hostedReviewBlockedReason: 'no_upstream', + hasUpstream: false, + hasAmbiguousGitHubHostedReview: true + }).title + ).toBe('Pull request status unavailable') + }) + + it('uses neutral copy for ambiguous GitHub hosted-review data before remote fallback publish copy', () => { + expect( + getChecksPanelEmptyStateCopy({ + operationLabel: null, + prRefreshStatus: undefined, + hostedReviewBlockedReason: undefined, + hasUpstream: false, + hasAmbiguousGitHubHostedReview: true + }).title + ).toBe('Pull request status unavailable') + }) + + it('uses neutral copy for ambiguous GitHub hosted-review data before unpushed commit copy', () => { + expect( + getChecksPanelEmptyStateCopy({ + operationLabel: null, + prRefreshStatus: undefined, + hostedReviewBlockedReason: 'needs_push', + hasUpstream: true, + hasAmbiguousGitHubHostedReview: true + }).title + ).toBe('Pull request status unavailable') + }) }) describe('shouldShowChecksPanelPublishBranchAction', () => { diff --git a/src/renderer/src/components/right-sidebar/checks-panel-empty-state.ts b/src/renderer/src/components/right-sidebar/checks-panel-empty-state.ts index 0810c0ffeb3..b36495f2714 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-empty-state.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-empty-state.ts @@ -11,6 +11,7 @@ type ChecksPanelEmptyStateInput = { hasCurrentBranch?: boolean reviewLabel?: 'pull request' | 'merge request' reviewShortLabel?: 'PR' | 'MR' + hasAmbiguousGitHubHostedReview?: boolean } type ChecksPanelEmptyStateCopy = { @@ -39,6 +40,24 @@ export function getChecksPanelEmptyStateCopy( } const blockedReason = input.hostedReviewBlockedReason + if ( + input.hasAmbiguousGitHubHostedReview === true && + (input.prRefreshStatus === 'paused' || + input.prRefreshStatus === 'skipped' || + input.prRefreshStatus === undefined) + ) { + return { + title: translate( + 'auto.components.right.sidebar.checks.panel.empty.state.3322603418', + 'Pull request status unavailable' + ), + description: translate( + 'auto.components.right.sidebar.checks.panel.empty.state.b597440265', + 'Refresh GitHub status for this branch to load checks and review.' + ) + } + } + if ( shouldShowChecksPanelPublishBranchAction({ hostedReviewBlockedReason: blockedReason, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 7e004df6698..8118136b07c 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -8911,6 +8911,7 @@ }, "empty": { "state": { + "3322603418": "Pull request status unavailable", "5b0cfae9a5": "Create a {{value0}} to start checks and review.", "13e1c7d5ed": "No {{value0}} found", "d372072df1": "GitHub refresh is paused by the current rate-limit budget", @@ -8925,7 +8926,8 @@ "f8543140cc": "Publish this branch before creating a {{value0}}.", "41252bc53f": "Branch not published", "05e4aec17b": "{{value0}} checks will be available after the operation completes", - "d77c513c1e": "{{value0}} in progress" + "d77c513c1e": "{{value0}} in progress", + "b597440265": "Refresh GitHub status for this branch to load checks and review." } } } diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index d01aecb6416..f4ac115dc62 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -8911,6 +8911,7 @@ }, "empty": { "state": { + "3322603418": "Pull request status unavailable", "5b0cfae9a5": "Cree un {{value0}} para iniciar comprobaciones y revisiones.", "13e1c7d5ed": "No se encontró {{value0}}", "d372072df1": "La actualización de GitHub está pausada por el presupuesto de límite de velocidad actual", @@ -8925,7 +8926,8 @@ "f8543140cc": "Publique esta rama antes de crear un {{value0}}.", "41252bc53f": "Sucursal no publicada", "05e4aec17b": "{{value0}} cheques estarán disponibles una vez completada la operación.", - "d77c513c1e": "{{value0}} en progreso" + "d77c513c1e": "{{value0}} en progreso", + "b597440265": "Refresh GitHub status for this branch to load checks and review." } } } diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 138e10813fd..531f739720c 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -8911,6 +8911,7 @@ }, "empty": { "state": { + "3322603418": "Pull request status unavailable", "5b0cfae9a5": "{{value0}} を作成してチェックとレビューを開始します。", "13e1c7d5ed": "{{value0}} が見つかりませんでした", "d372072df1": "現在のレート制限予算により GitHub の更新が一時停止されています", @@ -8925,7 +8926,8 @@ "f8543140cc": "{{value0}} を作成する前に、このブランチを公開してください。", "41252bc53f": "ブランチは公開されていません", "05e4aec17b": "{{value0}} チェックは操作の完了後に利用可能になります", - "d77c513c1e": "{{value0}} が進行中です" + "d77c513c1e": "{{value0}} が進行中です", + "b597440265": "Refresh GitHub status for this branch to load checks and review." } } } diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 64bf5cd45da..35323ea88fc 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -8911,6 +8911,7 @@ }, "empty": { "state": { + "3322603418": "Pull request status unavailable", "5b0cfae9a5": "체크와 리뷰를 시작하려면 {{value0}}을(를) 만드세요.", "13e1c7d5ed": "{{value0}}을(를) 찾을 수 없습니다.", "d372072df1": "현재 속도 제한 예산으로 인해 GitHub 새로 고침이 일시 중지되었습니다.", @@ -8925,7 +8926,8 @@ "f8543140cc": "{{value0}}을 만들기 전에 이 브랜치를 게시하세요.", "41252bc53f": "게시되지 않은 브랜치", "05e4aec17b": "{{value0}} 검사는 작업이 완료된 후에 사용할 수 있습니다.", - "d77c513c1e": "{{value0}} 진행 중" + "d77c513c1e": "{{value0}} 진행 중", + "b597440265": "Refresh GitHub status for this branch to load checks and review." } } } diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 1e46ef2fb90..8a95f561d38 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -8911,6 +8911,7 @@ }, "empty": { "state": { + "3322603418": "Pull request status unavailable", "5b0cfae9a5": "创建 {{value0}} 以开始检查和评审。", "13e1c7d5ed": "未找到 {{value0}}", "d372072df1": "GitHub 刷新因当前速率限制预算而暂停", @@ -8925,7 +8926,8 @@ "f8543140cc": "在创建 {{value0}} 之前发布此分支。", "41252bc53f": "分支未发布", "05e4aec17b": "{{value0}} 检查将在操作完成后可用", - "d77c513c1e": "{{value0}} 正在进行中" + "d77c513c1e": "{{value0}} 正在进行中", + "b597440265": "Refresh GitHub status for this branch to load checks and review." } } } diff --git a/src/renderer/src/store/slices/github-pr-refresh-states-leak.test.ts b/src/renderer/src/store/slices/github-pr-refresh-states-leak.test.ts index d9d1a91ecc4..74096867101 100644 --- a/src/renderer/src/store/slices/github-pr-refresh-states-leak.test.ts +++ b/src/renderer/src/store/slices/github-pr-refresh-states-leak.test.ts @@ -25,14 +25,15 @@ const MAX_ENTRIES = 2000 // prRefreshStates entry shapes (the module's PRRefreshState type is not exported). type SeedState = { - status: 'in-flight' | 'error' + status: 'queued' | 'in-flight' | 'paused' | 'skipped' | 'error' reason: GitHubPRRefreshReason updatedAt: number + pausedUntil?: number message?: string } function inFlightState(): SeedState { - return { status: 'in-flight', reason: 'visible', updatedAt: 0 } + return { status: 'in-flight', reason: 'visible', updatedAt: Date.now() } } function errorState(): SeedState { @@ -150,4 +151,263 @@ describe('prRefreshStates stays bounded (leak regression)', () => { expect(states['seed-1']).toBeUndefined() expect(states['newcomer']).toBeDefined() }) + + it('hides expired active states through the effective reader without mutating during read', () => { + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + try { + const store = createTestStore() + store.setState({ + prRefreshStates: { + stale: { status: 'in-flight', reason: 'visible', updatedAt: 1_000_000 - 121_000 }, + fresh: { status: 'in-flight', reason: 'visible', updatedAt: 1_000_000 - 10_000 } + } + }) + + expect(store.getState().getEffectiveGitHubPRRefreshState('stale', 1_000_000)).toBeUndefined() + expect(store.getState().getEffectiveGitHubPRRefreshState('fresh', 1_000_000)).toMatchObject({ + status: 'in-flight' + }) + expect(store.getState().prRefreshStates.stale).toBeDefined() + } finally { + vi.useRealTimers() + } + }) + + it('expires paused states after pausedUntil grace and rejects missing pause timestamps', () => { + const store = createTestStore() + store.setState({ + prRefreshStates: { + currentPause: { + status: 'paused', + reason: 'visible', + updatedAt: 1_000, + pausedUntil: 2_000 + }, + missingPause: { status: 'paused', reason: 'visible', updatedAt: 1_000 } + } + }) + + expect(store.getState().getEffectiveGitHubPRRefreshState('currentPause', 2_004)).toMatchObject({ + status: 'paused' + }) + expect(store.getState().getEffectiveGitHubPRRefreshState('currentPause', 7_001)).toBeUndefined() + expect(store.getState().getEffectiveGitHubPRRefreshState('missingPause', 1_500)).toBeUndefined() + }) + + it('prunes expired active states on refreshAll without writing a PR miss', () => { + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + try { + const store = createTestStore() + store.setState({ + prRefreshStates: { + stale: { status: 'queued', reason: 'visible', updatedAt: 1_000_000 - 121_000 }, + settled: { status: 'error', reason: 'visible', updatedAt: 1, message: 'boom' } + }, + prCache: { + stale: { data: null, fetchedAt: 1 } + }, + repos: [], + worktreesByRepo: {} + } as unknown as Partial) + + store.getState().refreshAllGitHub() + + expect(store.getState().prRefreshStates.stale).toBeUndefined() + expect(store.getState().prRefreshStates.settled).toBeDefined() + expect(store.getState().prCache.stale).toEqual({ data: null, fetchedAt: 1 }) + } finally { + vi.useRealTimers() + } + }) + + it('manual expiry does not remove a newer active state for the same cache key', () => { + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + try { + const store = createTestStore() + const token = { + sequence: 1, + status: 'in-flight' as const, + updatedAt: 1_000_000 - 121_000 + } + store.setState({ + prRefreshSequences: { branch: 2 }, + prRefreshStates: { + branch: { status: 'in-flight', reason: 'manual', updatedAt: 1_000_000 } + } + }) + + store.getState().expireGitHubPRRefreshState('branch', token, 1_000_000) + + expect(store.getState().prRefreshStates.branch).toMatchObject({ + status: 'in-flight', + updatedAt: 1_000_000 + }) + } finally { + vi.useRealTimers() + } + }) + + it('manual expiry does not notify subscribers when the token no longer matches', () => { + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + try { + const store = createTestStore() + const subscriber = vi.fn() + const unsubscribe = store.subscribe(subscriber) + store.setState({ + prRefreshSequences: { branch: 2 }, + prRefreshStates: { + branch: { status: 'in-flight', reason: 'manual', updatedAt: 1_000_000 } + } + }) + subscriber.mockClear() + + store.getState().expireGitHubPRRefreshState( + 'branch', + { + sequence: 1, + status: 'in-flight', + updatedAt: 1_000_000 - 121_000 + }, + 1_000_000 + ) + + expect(subscriber).not.toHaveBeenCalled() + unsubscribe() + } finally { + vi.useRealTimers() + } + }) + + it('manual expiry does not notify subscribers for an active state that is still fresh', () => { + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + try { + const store = createTestStore() + const subscriber = vi.fn() + store.setState({ + prRefreshSequences: { branch: 1 }, + prRefreshStates: { + branch: { status: 'in-flight', reason: 'manual', updatedAt: 1_000_000 } + } + }) + const unsubscribe = store.subscribe(subscriber) + + store.getState().expireGitHubPRRefreshState( + 'branch', + { + sequence: 1, + status: 'in-flight', + updatedAt: 1_000_000 + }, + 1_000_000 + ) + + expect(subscriber).not.toHaveBeenCalled() + unsubscribe() + } finally { + vi.useRealTimers() + } + }) + + it('manual expiry notifies subscribers when it removes the captured stale active state', () => { + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + try { + const store = createTestStore() + const subscriber = vi.fn() + store.setState({ + prRefreshSequences: { branch: 1 }, + prRefreshStates: { + branch: { status: 'in-flight', reason: 'manual', updatedAt: 1_000_000 - 121_000 } + } + }) + const unsubscribe = store.subscribe(subscriber) + + store.getState().expireGitHubPRRefreshState( + 'branch', + { + sequence: 1, + status: 'in-flight', + updatedAt: 1_000_000 - 121_000 + }, + 1_000_000 + ) + + expect(subscriber).toHaveBeenCalledTimes(1) + expect(store.getState().prRefreshStates.branch).toBeUndefined() + unsubscribe() + } finally { + vi.useRealTimers() + } + }) + + it('expires the stale active state captured by a rejected forced PR refresh', async () => { + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + try { + const store = createTestStore() + const repoPath = '/repo' + const branch = 'feature/rejected-refresh' + const cacheKey = `${repoPath}::${branch}` + mockApi.gh.refreshPRNow.mockRejectedValueOnce(new Error('stale repo path')) + store.setState({ + prRefreshSequences: { [cacheKey]: 1 }, + prRefreshStates: { + [cacheKey]: { status: 'in-flight', reason: 'manual', updatedAt: 1_000_000 - 121_000 } + } + }) + + await expect( + store.getState().fetchPRForBranch(repoPath, branch, { force: true }) + ).resolves.toBeNull() + + expect(store.getState().prRefreshStates[cacheKey]).toBeUndefined() + expect(store.getState().prCache[cacheKey]).toBeUndefined() + } finally { + vi.useRealTimers() + } + }) + + it('accepts a late terminal outcome after the active status timed out', () => { + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + try { + const store = createTestStore() + const repoPath = '/repo' + const branch = 'feature/late-outcome' + const cacheKey = `${repoPath}::${branch}` + + store.getState().applyGitHubPRRefreshEvent(statusEvent(cacheKey, 1)) + vi.setSystemTime(1_000_000 + 121_000) + expect(store.getState().getEffectiveGitHubPRRefreshState(cacheKey)).toBeUndefined() + + store.getState().applyGitHubPRRefreshEvent({ + sequence: 1, + reason: 'visible', + aliases: [{ cacheKey, repoPath, branch }], + outcome: { + kind: 'found', + pr: { + number: 12, + title: 'Late PR', + state: 'open', + url: 'https://github.com/acme/orca/pull/12', + checksStatus: 'pending', + updatedAt: '2026-03-28T00:00:00Z', + mergeable: 'UNKNOWN' + }, + fetchedAt: 1_000_000 + 121_000 + } + }) + + expect(store.getState().prCache[cacheKey]?.data).toMatchObject({ title: 'Late PR' }) + expect(store.getState().prRefreshStates[cacheKey]).toBeUndefined() + } finally { + vi.useRealTimers() + } + }) }) diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 9062068f222..947cc15e1c4 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -629,7 +629,7 @@ type RepoScopedFetchOptions = FetchOptions & { repoId?: string } -type PRRefreshState = { +export type PRRefreshState = { status: 'queued' | 'in-flight' | 'paused' | 'skipped' | 'error' reason: GitHubPRRefreshReason updatedAt: number @@ -637,6 +637,15 @@ type PRRefreshState = { message?: string } +type PRRefreshStateClearToken = { + sequence: number + status: PRRefreshState['status'] + updatedAt: number +} + +const PR_REFRESH_ACTIVE_STALE_MS = 120_000 +const PR_REFRESH_PAUSED_GRACE_MS = 5_000 + function bypassesGitHubPRRefreshFreshness(reason: GitHubPRRefreshReason): boolean { return reason === 'manual' || reason === 'active' || reason === 'post-push' } @@ -1464,6 +1473,64 @@ function capPrRefreshSequences( // realistic usage never reaches. Evicted entries self-heal on the next refresh event. const MAX_PR_REFRESH_STATE_ENTRIES = 2000 const SETTLED_PR_REFRESH_STATUSES = new Set(['error', 'skipped']) +const ACTIVE_PR_REFRESH_STATUSES = new Set([ + 'queued', + 'in-flight', + 'paused' +]) + +function isPRRefreshStateExpired(state: PRRefreshState, now: number): boolean { + const expiryAt = getGitHubPRRefreshStateExpiryAt(state) + return expiryAt !== null && now > expiryAt +} + +export function getGitHubPRRefreshStateExpiryAt(state: PRRefreshState | undefined): number | null { + if (!state) { + return null + } + if (state.status === 'queued' || state.status === 'in-flight') { + return Number.isFinite(state.updatedAt) ? state.updatedAt + PR_REFRESH_ACTIVE_STALE_MS : 0 + } + if (state.status === 'paused') { + return Number.isFinite(state.pausedUntil) + ? (state.pausedUntil ?? 0) + PR_REFRESH_PAUSED_GRACE_MS + : 0 + } + return null +} + +function isExpiredActivePRRefreshState(state: PRRefreshState, now: number): boolean { + return ACTIVE_PR_REFRESH_STATUSES.has(state.status) && isPRRefreshStateExpired(state, now) +} + +export function getEffectiveGitHubPRRefreshState( + states: Record, + cacheKey: string, + now = Date.now() +): PRRefreshState | undefined { + const state = states[cacheKey] + if (!state || isExpiredActivePRRefreshState(state, now)) { + return undefined + } + return state +} + +function pruneExpiredPRRefreshStates( + states: Record, + now = Date.now() +): Record { + let next: Record | null = null + for (const [cacheKey, state] of Object.entries(states)) { + if (!isExpiredActivePRRefreshState(state, now)) { + continue + } + if (!next) { + next = { ...states } + } + delete next[cacheKey] + } + return next ?? states +} function capPrRefreshStates( states: Record, @@ -1614,6 +1681,12 @@ export type GitHubSlice = { reportVisibleGitHubPRRefreshCandidates: (worktreeIds: string[], generation: number) => void bumpGitHubPRVisibleRefreshGeneration: () => void applyGitHubPRRefreshEvent: (event: GitHubPRRefreshEvent) => void + getEffectiveGitHubPRRefreshState: (cacheKey: string, now?: number) => PRRefreshState | undefined + expireGitHubPRRefreshState: ( + cacheKey: string, + token: PRRefreshStateClearToken, + now?: number + ) => void /** * Why: returns cached work items immediately (null if none) and fires a * background refresh when stale. Callers can render the cached list while @@ -1806,6 +1879,40 @@ export const createGitHubSlice: StateCreator = (s workItemsInvalidationNonce: 0, projectViewCache: {}, + getEffectiveGitHubPRRefreshState: (cacheKey, now) => + getEffectiveGitHubPRRefreshState(get().prRefreshStates, cacheKey, now), + + expireGitHubPRRefreshState: (cacheKey, token, now = Date.now()) => { + const currentState = get() + const currentRefreshState = currentState.prRefreshStates[cacheKey] + if ( + !currentRefreshState || + !ACTIVE_PR_REFRESH_STATUSES.has(currentRefreshState.status) || + !isExpiredActivePRRefreshState(currentRefreshState, now) || + (currentState.prRefreshSequences[cacheKey] ?? 0) !== token.sequence || + currentRefreshState.status !== token.status || + currentRefreshState.updatedAt !== token.updatedAt + ) { + return + } + set((s) => { + const state = s.prRefreshStates[cacheKey] + if ( + !state || + !ACTIVE_PR_REFRESH_STATUSES.has(state.status) || + !isExpiredActivePRRefreshState(state, now) || + (s.prRefreshSequences[cacheKey] ?? 0) !== token.sequence || + state.status !== token.status || + state.updatedAt !== token.updatedAt + ) { + return s + } + const nextStates = { ...s.prRefreshStates } + delete nextStates[cacheKey] + return { prRefreshStates: nextStates } + }) + }, + fetchProjectViewTable: async (args, options) => { const target = getActiveRuntimeTarget(get().settings) const sourceScope = projectViewSourceScope(get().settings) @@ -2709,6 +2816,14 @@ export const createGitHubSlice: StateCreator = (s const generation = (prRequestGenerations.get(cacheKey) ?? 0) + 1 const requestStartedAt = Date.now() const requestStartedHostedReviewEntry = get().hostedReviewCache[hostedReviewCacheKey] + const requestStartedPRRefreshState = get().prRefreshStates[cacheKey] + const requestStartedPRRefreshToken = requestStartedPRRefreshState + ? { + sequence: get().prRefreshSequences[cacheKey] ?? 0, + status: requestStartedPRRefreshState.status, + updatedAt: requestStartedPRRefreshState.updatedAt + } + : null prRequestGenerations.set(cacheKey, generation) const request = (async () => { @@ -2828,6 +2943,9 @@ export const createGitHubSlice: StateCreator = (s prRequestGenerations.delete(cacheKey) } } + if (requestStartedPRRefreshToken) { + get().expireGitHubPRRefreshState(cacheKey, requestStartedPRRefreshToken) + } } })() @@ -3511,10 +3629,11 @@ export const createGitHubSlice: StateCreator = (s applyGitHubPRRefreshEvent: (event) => { set((s) => { const nextSequences = { ...s.prRefreshSequences } - const nextStates = { ...s.prRefreshStates } + const prunedStates = pruneExpiredPRRefreshStates(s.prRefreshStates) + const nextStates = { ...prunedStates } let nextPRCache = s.prCache let nextHostedReviewCache = s.hostedReviewCache ?? {} - let changed = false + let changed = prunedStates !== s.prRefreshStates for (const alias of event.aliases) { const aliasExecutionHostId = getRefreshAliasExecutionHostId(alias) @@ -3706,7 +3825,8 @@ export const createGitHubSlice: StateCreator = (s issueCache: evictStaleEntries(s.issueCache), checksCache: evictStaleEntries(s.checksCache), workItemsCache: evictStaleEntries(s.workItemsCache), - projectViewCache: evictStaleEntries(s.projectViewCache) + projectViewCache: evictStaleEntries(s.projectViewCache), + prRefreshStates: pruneExpiredPRRefreshStates(s.prRefreshStates) })) // Why: prRequestGenerations tracks only live inflight fetches and is