mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 16:03:17 +00:00
fix(checks): stop the transient "PR already exists" card before the PR loads
The Checks panel briefly showed the terminal "Pull request already exists" card (Open Review only) on a cold mount, and only rendered the real PR panel after a manual refresh. Root cause (regression from #9428): the eligibility probe resolves `existing_review` from cache faster than the renderable PR hydrates, and the new exhaustive selector ranks `existing_review` as a terminal safety blocker above every loading state. So during the fetch window the panel settled on the terminal card instead of a self-updating loading state. The auto-fetch also used a lazy SWR priority, so the PR landed slowly unless the user forced a foreground refresh. Fix: - Selector: while a PR fetch is queued/in-flight and we have positive-but-not- yet-renderable review evidence (existing_review or positive_unresolved), show the self-updating "Checking status" state instead of the terminal card. It flips to the review once details land. A concurrent branch blocker still owns the copy. - Fetch: when a review is known to exist but no renderable PR is cached, escalate the auto-fetch to a foreground `active`/80 request (matching manual refresh) so it resolves promptly. Reuses existing i18n keys; adds unit coverage for both parts.
This commit is contained in:
@@ -1364,7 +1364,12 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
const refreshRequest = resolveChecksPanelPRRefreshRequest({
|
||||
cachedHasPR: prCachedHasPR,
|
||||
cachedFetchedAt: prFetchedAt ?? null,
|
||||
panelVisibleSince: panelVisibleSinceRef.current
|
||||
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'
|
||||
})
|
||||
enqueueGitHubPRRefresh(activeWorktreeId, refreshRequest.reason, refreshRequest.priority)
|
||||
}
|
||||
@@ -1372,9 +1377,11 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
}, [
|
||||
activeWorktreeId,
|
||||
branch,
|
||||
checksPanelReviewLookup,
|
||||
enqueueGitHubPRRefresh,
|
||||
fallbackGitHubPRNumber,
|
||||
fetchHostedReviewForBranch,
|
||||
hostedReviewCreation?.blockedReason,
|
||||
isFolder,
|
||||
isGitHubReviewContext,
|
||||
isPanelVisible,
|
||||
|
||||
@@ -129,6 +129,44 @@ describe('getChecksPanelReviewState — precedence', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('existing_review with a fetch in flight shows Checking status, not the terminal card', () => {
|
||||
// Regression (#9428): the terminal "already exists" card short-circuited the
|
||||
// in-flight fetch window, so the panel stalled until a manual refresh.
|
||||
const state = getChecksPanelReviewState(
|
||||
input({ eligibilityBlockedReason: 'existing_review', refresh: { status: 'in-flight' } })
|
||||
)
|
||||
expect(state.title).toBe('Checking pull request status')
|
||||
expect(state.renderReview).toBe(false)
|
||||
expect(state.recovery).toEqual([])
|
||||
})
|
||||
|
||||
it('positive_unresolved with a queued fetch shows Checking status', () => {
|
||||
const state = getChecksPanelReviewState(
|
||||
input({ reviewLookup: 'positive_unresolved', refresh: { status: 'queued' } })
|
||||
)
|
||||
expect(state.title).toBe('Checking pull request status')
|
||||
})
|
||||
|
||||
it('existing_review with no active fetch keeps the terminal already-exists card', () => {
|
||||
const state = getChecksPanelReviewState(
|
||||
input({ eligibilityBlockedReason: 'existing_review', openReviewUrl: 'https://x/pull/1' })
|
||||
)
|
||||
expect(state.title).toBe('Pull request already exists')
|
||||
expect(state.recovery).toContain('open_review')
|
||||
})
|
||||
|
||||
it('a concurrent branch blocker still owns the copy over an in-flight fetch', () => {
|
||||
const state = getChecksPanelReviewState(
|
||||
input({
|
||||
eligibilityBlockedReason: 'no_upstream',
|
||||
reviewLookup: 'positive_unresolved',
|
||||
hasUpstream: false,
|
||||
refresh: { status: 'in-flight' }
|
||||
})
|
||||
)
|
||||
expect(state.title).toBe('No upstream configured')
|
||||
})
|
||||
|
||||
it('hard refresh error hides the composer', () => {
|
||||
const state = getChecksPanelReviewState(
|
||||
input({ refresh: { status: 'error', errorType: 'auth' } })
|
||||
|
||||
@@ -74,6 +74,37 @@ 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.
|
||||
const reviewFetchInFlight =
|
||||
input.refresh?.status === 'queued' || input.refresh?.status === 'in-flight'
|
||||
const hasPendingReviewEvidence =
|
||||
input.reviewLookup === 'positive_unresolved' || blockedReason === 'existing_review'
|
||||
if (reviewFetchInFlight && hasPendingReviewEvidence && !isBranchBlocker(blockedReason)) {
|
||||
return {
|
||||
renderReview: false,
|
||||
title: translate(
|
||||
'auto.components.right.sidebar.checks.panel.review.active.title',
|
||||
'Checking {{reviewLabel}} status',
|
||||
{ reviewLabel }
|
||||
),
|
||||
description: translate(
|
||||
'auto.components.right.sidebar.checks.panel.review.active.body',
|
||||
'Orca is checking {{provider}} for a {{reviewLabel}} on this branch.',
|
||||
{ reviewLabel, provider: providerName }
|
||||
),
|
||||
composerMode: 'hidden',
|
||||
workflowAction: null,
|
||||
recovery: []
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Eligibility safety states own their guidance (existing_review is a hard
|
||||
// create block in the positive-evidence family).
|
||||
if (blockedReason && ELIGIBILITY_SAFETY_BLOCKERS.has(blockedReason)) {
|
||||
|
||||
@@ -22,6 +22,17 @@ describe('resolveChecksPanelPRRefreshRequest', () => {
|
||||
).toEqual({ reason: 'swr', priority: 30 })
|
||||
})
|
||||
|
||||
it('foreground-fetches a known-but-unrendered review so the panel resolves off the transient card', () => {
|
||||
expect(
|
||||
resolveChecksPanelPRRefreshRequest({
|
||||
cachedHasPR: null,
|
||||
cachedFetchedAt: null,
|
||||
panelVisibleSince: 200,
|
||||
hasUnrenderedReviewEvidence: true
|
||||
})
|
||||
).toEqual({ reason: 'active', priority: 80 })
|
||||
})
|
||||
|
||||
it('keeps populated or unknown cache entries on the background path', () => {
|
||||
expect(
|
||||
resolveChecksPanelPRRefreshRequest({
|
||||
|
||||
@@ -4,6 +4,10 @@ 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.
|
||||
hasUnrenderedReviewEvidence?: boolean
|
||||
}
|
||||
|
||||
type ChecksPanelPRRefreshRequest = {
|
||||
@@ -20,9 +24,10 @@ export function resolveChecksPanelPRRefreshRequest(
|
||||
input.panelVisibleSince !== null &&
|
||||
input.cachedFetchedAt < input.panelVisibleSince
|
||||
|
||||
if (cachedMissPredatesVisiblePanel) {
|
||||
// Why: external agents can create/merge a PR after Orca cached "none";
|
||||
// visible empty-state checks need one foreground lookup to recover.
|
||||
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.
|
||||
return { reason: 'active', priority: 80 }
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user