mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 00:03:20 +00:00
fix(checks): bound unresolved review foreground refreshes
This commit is contained in:
@@ -525,6 +525,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
const prevChecksRef = useRef<string>('')
|
||||
const conflictSummaryRefreshKeyRef = useRef<string | null>(null)
|
||||
const panelVisibleSinceRef = useRef<number | null>(null)
|
||||
const foregroundedUnrenderedReviewKeyRef = useRef<string | null>(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({
|
||||
|
||||
@@ -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 =
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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 }
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user