From 00bf0938e55022275af98aaa2c5a4330ead4deb1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:34:29 -0700 Subject: [PATCH] fix(checks): bound unresolved review foreground refreshes --- .../components/right-sidebar/ChecksPanel.tsx | 56 ++++++++++++++++--- .../right-sidebar/checks-panel-empty-state.ts | 9 +-- .../checks-panel-pr-refresh-request.test.ts | 12 ++++ .../checks-panel-pr-refresh-request.ts | 14 ++--- 4 files changed, 68 insertions(+), 23 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 867eb4c0cbe..587855007e8 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -525,6 +525,7 @@ export default function ChecksPanel(): React.JSX.Element { const prevChecksRef = useRef('') const conflictSummaryRefreshKeyRef = useRef(null) const panelVisibleSinceRef = useRef(null) + const foregroundedUnrenderedReviewKeyRef = useRef(null) commentsRef.current = comments const prGenerationRecords = useAppStore((s) => s.pullRequestGenerationRecords) const allocatePullRequestGenerationRequestId = useAppStore( @@ -954,6 +955,15 @@ export default function ChecksPanel(): React.JSX.Element { eligibilityReview: hostedReviewCreation?.review ?? null }) const checksPanelReviewLookup = checksPanelReviewLookupResult.state + const hasUnrenderedReviewEvidence = + checksPanelReviewLookup === 'positive_unresolved' || + hostedReviewCreation?.blockedReason === 'existing_review' + const unrenderedReviewEvidenceIdentity = + linkedReviewNumber ?? + hostedReview?.number ?? + hostedReviewCreation?.review?.number ?? + checksPanelReviewLookupResult.openReviewUrl ?? + 'unknown' // Confirmed readiness from the last eligibility snapshot, not live canCreate (which would be circular and flap during transient failures). const hardErrorObservedAt = isGitHubReviewContext && hardRefreshError && hardRefreshError.contextKey === panelContextKey @@ -1364,12 +1374,7 @@ export default function ChecksPanel(): React.JSX.Element { const refreshRequest = resolveChecksPanelPRRefreshRequest({ cachedHasPR: prCachedHasPR, cachedFetchedAt: prFetchedAt ?? null, - panelVisibleSince: panelVisibleSinceRef.current, - // A known-but-unrendered review must foreground-fetch so the panel - // resolves to the review instead of stalling on a transient card. - hasUnrenderedReviewEvidence: - checksPanelReviewLookup === 'positive_unresolved' || - hostedReviewCreation?.blockedReason === 'existing_review' + panelVisibleSince: panelVisibleSinceRef.current }) enqueueGitHubPRRefresh(activeWorktreeId, refreshRequest.reason, refreshRequest.priority) } @@ -1377,11 +1382,9 @@ export default function ChecksPanel(): React.JSX.Element { }, [ activeWorktreeId, branch, - checksPanelReviewLookup, enqueueGitHubPRRefresh, fallbackGitHubPRNumber, fetchHostedReviewForBranch, - hostedReviewCreation?.blockedReason, isFolder, isGitHubReviewContext, isPanelVisible, @@ -1396,6 +1399,43 @@ export default function ChecksPanel(): React.JSX.Element { repo ]) + useEffect(() => { + if (!hasUnrenderedReviewEvidence || !isPanelVisible) { + foregroundedUnrenderedReviewKeyRef.current = null + return + } + if (!repo || isFolder || !branch || !activeWorktreeId || !isGitHubReviewContext) { + return + } + const evidenceKey = `${refreshContextKey}::${unrenderedReviewEvidenceIdentity}` + const refreshRequest = resolveChecksPanelPRRefreshRequest({ + cachedHasPR: prCachedHasPR, + cachedFetchedAt: prFetchedAt ?? null, + panelVisibleSince: panelVisibleSinceRef.current, + hasUnrenderedReviewEvidence: true, + hasRequestedForegroundRefresh: foregroundedUnrenderedReviewKeyRef.current === evidenceKey + }) + if (refreshRequest.reason !== 'active') { + return + } + // One forced lookup per evidence revision prevents a settled miss from creating a provider-request loop. + foregroundedUnrenderedReviewKeyRef.current = evidenceKey + enqueueGitHubPRRefresh(activeWorktreeId, refreshRequest.reason, refreshRequest.priority) + }, [ + activeWorktreeId, + branch, + enqueueGitHubPRRefresh, + hasUnrenderedReviewEvidence, + isFolder, + isGitHubReviewContext, + isPanelVisible, + prCachedHasPR, + prFetchedAt, + refreshContextKey, + repo, + unrenderedReviewEvidenceIdentity + ]) + useEffect(() => { if ( !shouldPollChecksPanelRuntimeSshStatus({ 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 294ac9d59c3..c6fc27b0828 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 @@ -74,14 +74,7 @@ export function getChecksPanelReviewState( const blockedReason = input.eligibilityBlockedReason - // 1.5 Positive review evidence while the lookup is still in flight → a - // self-updating "Checking status" state, never a terminal card. - // `existing_review` and positive-unresolved evidence both mean a - // renderable review is inbound; #9428 short-circuited them to a terminal - // "already exists" card during the fetch window, so the panel stalled on - // that card until a manual refresh even though the review was about to - // render. A concurrent branch blocker (publish/sync/auth/needs_push) - // still owns the copy. + // 1.5 Keep active positive-review lookups self-updating; concurrent branch blockers still own their guidance. const reviewFetchInFlight = input.refresh?.status === 'queued' || input.refresh?.status === 'in-flight' const hasPendingReviewEvidence = diff --git a/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.test.ts b/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.test.ts index 8dde075b7ff..2b2f024602e 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.test.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.test.ts @@ -33,6 +33,18 @@ describe('resolveChecksPanelPRRefreshRequest', () => { ).toEqual({ reason: 'active', priority: 80 }) }) + it('does not repeatedly force provider work for the same unrendered review evidence', () => { + expect( + resolveChecksPanelPRRefreshRequest({ + cachedHasPR: false, + cachedFetchedAt: 100, + panelVisibleSince: 200, + hasUnrenderedReviewEvidence: true, + hasRequestedForegroundRefresh: true + }) + ).toEqual({ reason: 'swr', priority: 30 }) + }) + it('keeps populated or unknown cache entries on the background path', () => { expect( resolveChecksPanelPRRefreshRequest({ diff --git a/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.ts b/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.ts index 51e10ebe05d..26157a6d846 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-pr-refresh-request.ts @@ -4,10 +4,9 @@ type ChecksPanelPRRefreshRequestInput = { cachedHasPR: boolean | null cachedFetchedAt: number | null panelVisibleSince: number | null - // Why: a review is known to exist (positive evidence / existing_review) but no - // renderable PR is cached yet. A lazy SWR fetch leaves the panel on a transient - // "Checking status" state; a foreground lookup resolves it to the review. + // A known-but-unrendered review needs one foreground lookup to resolve its transient state. hasUnrenderedReviewEvidence?: boolean + hasRequestedForegroundRefresh?: boolean } type ChecksPanelPRRefreshRequest = { @@ -24,10 +23,11 @@ export function resolveChecksPanelPRRefreshRequest( input.panelVisibleSince !== null && input.cachedFetchedAt < input.panelVisibleSince - if (cachedMissPredatesVisiblePanel || input.hasUnrenderedReviewEvidence) { - // Why: external agents can create/merge a PR after Orca cached "none", and - // a known-but-unrendered review must resolve to the panel rather than stall - // on a transient card — both need one foreground lookup to recover. + if ( + !input.hasRequestedForegroundRefresh && + (cachedMissPredatesVisiblePanel || input.hasUnrenderedReviewEvidence) + ) { + // A stale miss or new positive evidence needs one foreground lookup to recover. return { reason: 'active', priority: 80 } }