Repository navigation
pr-review re-reviews an escalated PR while needs-human-review is present, then removes the hold on fix-request #2090
Description
Problem
Once pr-review escalates a PR to a human (review cycle 3/3, needs-human-review added), it should stop until a person removes the label or @mentions the bot. review-one-pr.sh is meant to enforce this: when it sees an escalation marker and the label is still there, it emits noop human-escalated. On #1902 that didn't happen:
| Time (UTC, 2026-10-09) | Event |
|---|---|
| 04:44:28 | pr-review escalates #1902 at cycle 3/3 (human-escalation reset=04:44:28Z) and adds needs-human-review. |
| ~04:44 | Auto-rebase merges main into the branch. The head becomes f5303859; the PR's own code is unchanged. |
| 05:03:22 | dev-lead posts its hold notice for needs-human-review, so the label was present. |
| 05:04:30 | pr-review posts a full-cascade fix-requested, cycle 1/3 on f5303859. |
| 05:10:49 | pr-review posts a full-cascade fix-requested, cycle 2/3 on the same f5303859. |
| ~05:11 | needs-human-review has been removed. No person touched the PR. dev-lead's hold notice flips to "hold was lifted". |
So a PR escalated to a human:
- was reviewed twice more by the full multi-model cascade, the second time on a commit it had just reviewed;
- had its human hold removed by automation, through
clear_automated_hold_labelinpost-pr-review.shon the fix-request path. Because the label was automation-set, the provenance check allows the removal.
This may be why needs-human-review has been seen vanishing from other PRs.
Why it matters
- The human-escalation hold (Runaway PR #860: agents self-triggering — add per-PR churn breaker, human-gated cap resets, and self-trigger guards #926/pr-review counts a human-escalation as a posted review — run reports "1 reviews posted" with nothing on the PR #1754) can be undone without a human, so the 3-cycle cap doesn't hold. Auto-rebase moves every open PR's head on each push to
main(Merge protections are unsatisfiable at N>1 open PRs: one merge to main forces a full re-review of every other PR — ~154 forced re-reviews in 48h, zero merges #1864), so this can happen to any escalated PR. - Each extra pass is a full triage → deep + duck → audit run.
Hypotheses (not yet confirmed)
FORCE_REVIEW is only set by a repository_dispatch with force_review: true, which is the mention path. No mention was posted. Candidates:
- Ring version: consumers run
pr-review/v1-next. Itsreview-one-pr.shmay predate the label-aware escalation block onmain, or match a different escalation marker. - Sweep: a sweep re-dispatch (
sweep-stuck-reviews.sh, or the rate-limit sweep that feat(#1894): answer "why isn't this PR approved?" from the real pr-review gate chain (+ why-not-approved agent) #1902's 04:14rate-limited … reset=05:14:19Zmarker points at) may reach the cascade without going through the escalation no-op. - Snapshot: the escalation check may have read a PR snapshot without the label.
The run logs for the 05:04 and 05:10 pr-review runs on #1902 will show which path ran: look for the noop:, re-engage: or force-review: line.
Acceptance criteria
- While
needs-human-reviewis present after an escalation, no automated path (synchronize, sweep, rate-limit retry, auto-rebase) runs the cascade. Only a human removing the label or @mentioning the bot re-engages it. - Automation never removes
needs-human-reviewon a PR whose latest escalation is still in effect. - pr-review never runs the full cascade twice on the same head SHA unless forced.
- A regression test reproduces the feat(#1894): answer "why isn't this PR approved?" from the real pr-review gate chain (+ why-not-approved agent) #1902 sequence: escalate, base merge moves the head, re-trigger, expect a no-op.
Activity
github-actions commentedon Oct 9, 2026
Thanks for the thorough write-up and timeline on #1902, which makes this much easier to dig into. To help us pin down which of your hypotheses applies, could you share the pr-review run logs for the 05:04 and 05:10 runs on #1902, specifically which
noop:,re-engage:orforce-review:line each one printed (or the trigger event if none did)?don-petry commentedon Oct 9, 2026
Another data point: pr-review posted a third review on #1902 at 05:13:57Z (cycle 3/3, single-reviewer mode), again on the unchanged head
f5303859. That makes three automated reviews of the same SHA in 10 minutes after a human escalation.The 04:14 marker on #1902 promised a re-review "after 2026-10-09T05:14:19Z", so this third run lands just before that reset time. That fits hypothesis 2: a sweep or rate-limit re-dispatch reaches the cascade without passing through the escalation no-op. The fix should guard every dispatch path on the escalation and label state, not only the synchronize path.
Generated by Claude Code
test(pr-review): pin that an automated dispatch never re-reviews or u…
don-petry commentedon Oct 9, 2026
Root cause confirmed from the run logs. It's hypothesis 1 (ring version), triggered by the pr-auto-review dispatch path.
What the runs show. All three post-escalation reviews on #1902 were
PR Review Agent — Triggerruns from arepository_dispatch:pr-review.yml@pr-review/v1-nextresolved tof34a765(Oct 6), withFORCE_REVIEW: true.FORCE_REVIEWwasgithub.event_name == 'repository_dispatch'alone, which is the pr-review approve/dismiss loop: an automatedpr-review-mentiondispatch runs with FORCE_REVIEW=true, approves past the maintainer-comment gate, and the next event run dismisses it — repeats until the automation budget is exhausted #2034 bug. So pr-auto-review's routine dispatch was treated as the human @mention break-glass.force-review: escalation marker present, but FORCE_REVIEW=true — re-engaging cascade. The later runs also show the same-SHA idempotency bypass.clear_automated_hold_labelremoved the automation-setneeds-human-review.Fix. #2078 (merged Oct 8) derives
FORCE_REVIEWonly from an explicitclient_payload.force_review.v1-nexthas since moved to6415eab, which includes it.What #2091 adds: regression tests (
tests/test_review_one_pr_escalation_hold.bats) that replay the #1902 sequence against current code. Automated dispatches stay paused on the hold and never remove the label. A structural pin on the #2078 expression fails against thef34a765workflow.Still exposed. Downstream consumers pin
pr-review/v1-stable(776c958), which predates #2078. They keep the bypass until pr-review is promoted.Generated by Claude Code
fix(pr-review): re-evaluate at the same head after its own gate dismi…
fix(holds/pr-review): honour every hold label; pin the escalation hol…