You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Two mechanisms strand a PR in a state no watchdog recovers: a merge-train waiter whose blocker resolves by any means other than a merge, and a verdict-flip guard that counts honest fix cycles as abuse and never decays.
Both end with a contributor's PR sitting indefinitely with no signal, which in a one-shot-review culture is a silent rejection.
1. Merge-train waiter is never re-driven unless the blocker merges
src/services/agent-action-executor.ts:714-716 — the merge-train denial does audit("denied", …); continue. No re-enqueue, no trailing re-review. PR B's pass then completes normally.
When blocker A resolves without merging, nothing wakes B:
A closed unmerged (bot red-CI close, cap close, its author, or a maintainer): the sibling wake returns early — src/queue/processors.ts:1416:
if(action!=="closed"||!mergedAt)return;
A ages past MERGE_TRAIN_MAX_WAIT_MS (src/review/merge-train.ts:131-135, measured from A's createdAt): the cap would now let B through, but it is only consulted at B's evaluation time and nothing schedules one.
The scheduled sweep is permanently ineligible — src/settings/agent-sweep.ts:161-174, #never-endless-reregate: one regate per PR, full stop.
Surface repair does not match: B's surface is current and its gate check exists (processors.ts:1186-1239); the #orb-stale-recheck-priority arm requires mergeableState !== "clean", and a merge-ready B is clean (:1203).
The stale-recheck-denial priority deliberately excludes merge-train waits as "durable, externally-actioned" (src/db/repositories.ts:3122-3129, STALE_RECHECK_DENIAL_DETAIL_PATTERN matches only duplicate-winner, base-conflict and review-thread denials). That classification is wrong here: closing A fires a webhook about A, never about B — precisely the "no webhook ever reaches this PR" gap the mechanism exists for.
Even the merged case is only partly covered: maybeEnqueueSiblingRegateForMergedPr prioritises shared linked-issue siblings (processors.ts:1418-1422), but the train also blocks on shared file overlap; a file-overlap waiter rides the oldest-first fallback capped at MERGE_WAKE_MAX_PRS (15) and can be excluded on a busy repo — the #7438 shape one tier down.
Deliverables
Pick one and implement: (a) fire the sibling wake on any terminal transition of an open PR, including closed-unmerged; (b) fold recent agent.action.merge_train_blocked audit rows into surfaceRepairPriorityPullNumbers keyed by blocking PR (the audit row already carries blockingPr); or (c) on a merge-train denial, enqueue a delayed agent-regate-pr at min(blocker age remaining to 24h, N minutes), mirroring scheduleTrailingMergeableStateReReview (processors.ts:3875-3877).
Correct the STALE_RECHECK_DENIAL_DETAIL_PATTERN classification comment.
Test: B blocked by A; A closed unmerged ⇒ B is re-driven and merges. Also for A aging past the cap, and A gaining the manual-review label.
2. Verdict-flip guard counts cross-head changes and never resets
nextVerdictFlipState (src/review/verdict-flip-guard.ts:27-31) is keyed per (repo, pull) only (verdict-flip-store.ts, migration 0183) — no head or fingerprint scoping, no decay, no reset.
Honest iteration reads as abuse.#9016's actual exploit is same-head, no-op recommit re-rolls. But a flip is counted across genuinely different heads: defect(head A) → contributor fixes → clean(head B) = flip 1; a new real issue on head C = flip 2; fixed on head D = flip 3 ⇒ escalate ⇒ manual-review hold, on a PR that is now consistently clean after real content changes. The threshold of 3 is two honest fix cycles.
It is an absorbing state. Once flipCount >= 3, escalate is true for every future fresh verdict forever — the count never decreases. The escalation finding's own user-facing advice, "push a substantive fix so the next review reflects real content change" (processors.ts:11446), is structurally ineffective: a substantive fix produces a fresh verdict, which re-escalates. Only maintainer label-removal plus a cache-hit pass (which skips the flip check, processors.ts:11431-11438) lets it through.
Deliverables
Advance flipCount only when the review's content fingerprint is unchanged from the prior fresh verdict — inputFingerprint is already persisted in cache metadata (processors.ts:11491) — or reset the counter whenever the fingerprint changes.
Add decay, or a reset after N consistent same-direction verdicts, so the state cannot be permanent.
Make the escalation copy accurate about what actually clears it.
Summary
Two mechanisms strand a PR in a state no watchdog recovers: a merge-train waiter whose blocker resolves by any means other than a merge, and a verdict-flip guard that counts honest fix cycles as abuse and never decays.
Both end with a contributor's PR sitting indefinitely with no signal, which in a one-shot-review culture is a silent rejection.
1. Merge-train waiter is never re-driven unless the blocker merges
src/services/agent-action-executor.ts:714-716— the merge-train denial doesaudit("denied", …); continue. No re-enqueue, no trailing re-review. PR B's pass then completes normally.When blocker A resolves without merging, nothing wakes B:
src/queue/processors.ts:1416:MERGE_TRAIN_MAX_WAIT_MS(src/review/merge-train.ts:131-135, measured from A'screatedAt): the cap would now let B through, but it is only consulted at B's evaluation time and nothing schedules one.And nothing else picks B up:
src/settings/agent-sweep.ts:161-174,#never-endless-reregate: one regate per PR, full stop.processors.ts:1186-1239); the#orb-stale-recheck-priorityarm requiresmergeableState !== "clean", and a merge-ready B is clean (:1203).src/db/repositories.ts:3122-3129,STALE_RECHECK_DENIAL_DETAIL_PATTERNmatches only duplicate-winner, base-conflict and review-thread denials). That classification is wrong here: closing A fires a webhook about A, never about B — precisely the "no webhook ever reaches this PR" gap the mechanism exists for.agent-action-executor.ts:63-64); a stranded B produces exactly one denial, ever.Even the merged case is only partly covered:
maybeEnqueueSiblingRegateForMergedPrprioritises shared linked-issue siblings (processors.ts:1418-1422), but the train also blocks on shared file overlap; a file-overlap waiter rides the oldest-first fallback capped atMERGE_WAKE_MAX_PRS(15) and can be excluded on a busy repo — the #7438 shape one tier down.Deliverables
agent.action.merge_train_blockedaudit rows intosurfaceRepairPriorityPullNumberskeyed by blocking PR (the audit row already carriesblockingPr); or (c) on a merge-train denial, enqueue a delayedagent-regate-pratmin(blocker age remaining to 24h, N minutes), mirroringscheduleTrailingMergeableStateReReview(processors.ts:3875-3877).STALE_RECHECK_DENIAL_DETAIL_PATTERNclassification comment.2. Verdict-flip guard counts cross-head changes and never resets
nextVerdictFlipState(src/review/verdict-flip-guard.ts:27-31) is keyed per(repo, pull)only (verdict-flip-store.ts, migration 0183) — no head or fingerprint scoping, no decay, no reset.Honest iteration reads as abuse. #9016's actual exploit is same-head, no-op recommit re-rolls. But a flip is counted across genuinely different heads: defect(head A) → contributor fixes → clean(head B) = flip 1; a new real issue on head C = flip 2; fixed on head D = flip 3 ⇒ escalate ⇒ manual-review hold, on a PR that is now consistently clean after real content changes. The threshold of 3 is two honest fix cycles.
It is an absorbing state. Once
flipCount >= 3,escalateis true for every future fresh verdict forever — the count never decreases. The escalation finding's own user-facing advice, "push a substantive fix so the next review reflects real content change" (processors.ts:11446), is structurally ineffective: a substantive fix produces a fresh verdict, which re-escalates. Only maintainer label-removal plus a cache-hit pass (which skips the flip check,processors.ts:11431-11438) lets it through.Deliverables
flipCountonly when the review's content fingerprint is unchanged from the prior fresh verdict —inputFingerprintis already persisted in cache metadata (processors.ts:11491) — or reset the counter whenever the fingerprint changes.