fix(github): sweep updates BEHIND branches — two-in-flight PRs no longer strand#911
Conversation
…ger strand Fifth stall mode, hit twice today (#909/#910): with strict branch protection, the first merge moves main under every other in-flight PR; GitHub's native auto-merge does not update branches, so an approved+green PR sits BLOCKED(behind) forever. This is the open question ws-2b5 originally asked — the answer is yes, an auto-update step is needed whenever two PRs are in flight at once. The merge-completion path now handles it: when the sweep's merge attempt is refused with 405 and the PR's mergeable_state is 'behind', it PUTs update-branch with expected_head_sha (same push-race guard as the merge call) and drops the watch — the synchronize event runs CI + re-review on the fresh head and the normal promote→merge cycle converges. Any other 405 keeps the existing retry-next-sweep behavior. The sweep's decision ladder is now: approve → merge → rescue-review → update-behind — one chokepoint for all five observed stall modes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #911 | fix(github): sweep updates BEHIND branches — two-in-flight PRs no longer strand
VERDICT: WARN (CI pending — auto-promotes to APPROVED on terminal-green)
CI Status
- test: queued
- dashboard: in_progress
Diff Review
- 2 files, +66/−5:
lib/plugins/github.ts+ test - Production: 405 refusal path now probes
mergeable_state; if "behind", putsupdate-branchwithexpected_head_shaand drops the stale watch. Non-behind 405s fall through to existing retry behavior. - Test: new "BEHIND branch → update-branch" case verifies merge attempt, update-branch PUT with correct SHA guard, and watch cleanup.
Observations
- LOW: CI still settling. The approve-on-green policy promotes this review to APPROVED automatically once every check passes terminal-green — no further action needed.
- LOW: clawpatch structural review unavailable (internal error) — not a blocker for a 2-file, 71-line diff.
- No blocking findings. The fix is targeted, well-tested, and completes the stall-mode ladder: approve → merge → rescue-review → update-behind. The 409 path now explicitly
return falseafter deleting the watch (previously fell through to the generic warn) — a minor behavioral improvement beyond the stated fix.
— Quinn, QA Engineer
|
Submitted COMMENT review on #911. |
|
✅ CI went terminal-green with no blockers on the prior review — promoting it to APPROVED per the approve-on-green policy (#748). |
There was a problem hiding this comment.
CI terminal-green, no blockers on prior review — auto-approving on green (#748).
1 participant
Fifth stall mode, hit twice this afternoon (#909 and #910 each went BEHIND when the other's merge moved main).
The failure
Strict branch protection requires up-to-date branches, but nothing updates them: native auto-merge doesn't, and the sweep's merge attempt gets a 405 and retried forever. Any time two PRs are in flight, the second strands as approved+green+BLOCKED(behind) until a human clicks update. This is the exact question ws-2b5 left open ("do BEHIND branches need an auto-update-branch step?") — empirically yes.
The fix
In the merge-completion path: a 405 refusal now probes
mergeable_state; ifbehind, the sweep PUTsupdate-branchwithexpected_head_sha(same push-race guard as the merge call) and drops the watch. The synchronize event runs CI + Quinn's re-review on the fresh head and the normal promote→merge cycle converges — typically two sweep cycles end-to-end. Non-behind 405s keep the existing retry behavior.The sweep's decision ladder is now approve → merge → rescue-review → update-behind: one chokepoint covering all five stall modes observed in production (missed CI webhook #888, unregistered repo #901, wedged auto-merge #907, lost review dispatch #909, behind branch — this PR).
Verification
1,190 tests green (new: 405+behind → update-branch with SHA guard + watch drop; existing 405-keeps-watch and 409-drops-watch behaviors unchanged),
NODE_ENV=production bun testgreen, tsc clean.Note the bootstrap irony applies one last time: if this PR itself goes behind or wedges, that's the pre-fix world — the deployed backstop handles wedges but not behind. One
update-branchclick/call fixes it, and it's the last one.🤖 Generated with Claude Code