auto-merge-on-approval waits full CodeRabbit window even when Codex already cleared — wasted CI minutes / agent turns

[nathanjohnpayne]

Summary

auto-merge-on-approval's "Wait for CodeRabbit review" step (.github/workflows/agent-review.yml:900-1011, delegating to scripts/coderabbit-wait.sh) blocks merge for up to coderabbit.max_wait_seconds (currently 1245s / ~20min) on every approved PR whenever coderabbit.enabled: true — with no check of whether Codex has already given an affirmative verdict on the same HEAD. Codex is the actual blocking bot-review gate (fleet-wide, CodeRabbit is comment-only/advisory — never CHANGES_REQUESTED); once Codex has cleared, waiting out CodeRabbit's own timeline adds pure latency and burns CI minutes + agent poll/approval turns for no additional gating value in the common case.

Concrete instance: PR #725

nathanjohnpayne/mergepath#725 — a single-word docs fix (visibility: private → public), about as low-risk a change as exists in this repo.

• Codex posted an affirmative verdict at 2026-07-07T02:09:38Z (~5 min after the trigger).
• Reviewer-identity approval landed at 2026-07-07T02:13:58Z.
• The auto-merge-on-approval run (28836689381) then spent 5m40s (02:14:17Z → 02:19:57Z) in "Wait for CodeRabbit review" — nearly the entirety of the job's 6-minute runtime — before merging, even though Codex clearance was already ~5 minutes old by the time that step even started.

Systemic evidence

docs/audits/data/review-latency-2026-07/ (produced for #623, already committed) gives the fleet-wide distributions:

bot	metric	p50	p90	p99	max
Codex	trigger → verdict (n=100)	217s	426s	630s	830s
CodeRabbit	commit → review (n=142/143, rate_limited=false)	414s	861s	1136s	1219s

Codex's median verdict lands in well under half the time CodeRabbit's median review takes. Since the agent-driven flow typically posts reviewer approval right after Codex clears, this means on a large share of PRs — not just trivial ones — the auto-merge-on-approval wait step is very likely to actually block for real, non-trivial time purely because CodeRabbit hasn't finished yet, independent of whether the change is substantive enough to need that second opinion.

A quick sample of 51 recent pull_request_review-triggered Agent Review Pipeline runs on this repo found only 3 where auto-merge-on-approval actually ran the wait step (most approvals didn't re-arm the job or the job was skipped for other reasons) — of those, 1 (PR #725, above) waited 340s and 2 waited ~2s (CodeRabbit had already finished by the time the job ran). So the failure mode is intermittent by nature (depends on how fast the reviewer-approval lands relative to CodeRabbit's own timeline) rather than universal, but it is real and the fleet-wide latency data says it should recur often.

Why this isn't a trivial fix

CodeRabbit and Codex are intentionally complementary per REVIEW_POLICY.md § Phase 2.5 — CodeRabbit's Potential issue / ⚠️ findings are meant to be addressed independent of Codex's own review, and the wait step's rc=2 (findings present) branch does block merge on those. So "just skip CodeRabbit whenever Codex approves" would reduce review coverage, not merely trim latency, and shouldn't be done unilaterally. This issue is to track the tradeoff and land a fix, not to prescribe one. Some candidate directions:

1. Short grace window post-Codex-clearance: once Codex has an affirmative verdict on HEAD, cap the CodeRabbit wait at something much shorter than the full max_wait_seconds (e.g. a bounded probe), and treat a still-pending CodeRabbit review the same as the existing rc=4 "grace window elapsed" path (advisory, proceed) rather than the full ~20min ceiling.
2. Start the CodeRabbit wait earlier, e.g. in parallel with the Codex trigger (codex-review-request.sh) rather than only once at auto-merge-on-approval time, so by the time reviewer approval lands CodeRabbit has had the same head start Codex did. (Partially already happens by luck when approval is slow to land — the 51-run sample's 2 near-zero-wait cases — but isn't structural.)
3. Downgrade CodeRabbit to fully async/post-merge advisory once both Codex clearance and reviewer approval are present, filing any post-hoc CodeRabbit findings as a follow-up issue instead of blocking merge. This is the most aggressive option and most directly matches the "CodeRabbit does not need to be triggered if Codex has already approved" framing, but most clearly trades away the Phase 2.5 guarantee and should get explicit sign-off before landing.

Pointers

• .github/workflows/agent-review.yml:900-1011 — the unconditional wait step.
• scripts/coderabbit-wait.sh — the underlying poll loop and its exit-code contract.
• scripts/merge-clearance-gate.sh — already has a related precedent: --derive-rate-limit-protection lets a CodeRabbit rate-limit (rc=5) downgrade to non-blocking when Codex/Phase-4b clearance is already satisfied on HEAD (coderabbit-wait + codex: auto-request @codex while CodeRabbit is rate-limited, revert when the window elapses #489/auto-merge-on-approval fail-closes external-review PRs when CodeRabbit is rate-limited (cleared needs-external-review label mis-read as under-threshold) #713/Fix CodeRabbit rate-limit auto-merge after Phase 4b clearance #714). No analogous fast-path exists for the "CodeRabbit is just still running" case this issue is about.
• docs/audits/data/review-latency-2026-07/ — the latency dataset cited above (Measure actual Codex review latency and retune every folklore-tuned wait window from the data #623).
• review flow: add wait-for-CodeRabbit state before auto-merge-on-approval #136 — original issue that added the wait-for-CodeRabbit step; rationale was a pre-review-merge race, not "wait the full window regardless of other clearance."

Surfaced during merge of PR #725 (fix for #724).

[nathanpayne-codex]

This was generated by AI during triage.

Recommendation

Use a two-step fix.

1. Land a narrow bug fix first: make scripts/coderabbit-wait.sh calculate rate-limit sleep against the CodeRabbit rate-limit comment's fresh_at / created_at, rather than starting the full parsed window from when the helper first notices the comment. In fix(724): flip stale visibility: private example in templated-propagation.md #725 the rate-limit notice was already old when auto-merge-on-approval saw it, but the helper still slept the full parsed window. This preserves the current review policy and removes avoidable delay.

2. Then add a policy-controlled fast path for auto-merge-on-approval: when the current head already has verified Codex or Phase 4b clearance and a reviewer-identity approval is present, cap the remaining CodeRabbit wait to a short grace window, for example coderabbit.post_clearance_max_wait_seconds: 240. If CodeRabbit clears or posts Potential issue / ⚠️ inside that window, handle it exactly as today. If CodeRabbit is still pending after the cap, proceed like the existing rc=4 advisory timeout.

This fast path should not activate when Codex / Phase 4b was never called or has not cleared the current head. In that case CodeRabbit remains the only bot-review signal, so it should keep the full coderabbit.max_wait_seconds behavior.

I would avoid the fully async/post-merge CodeRabbit option for now. It weakens Phase 2.5's current contract that CodeRabbit runs on enabled PRs and substantive findings are addressed before merge. The short-cap option trims wasted time after another current-head external clearance exists without removing CodeRabbit's chance to stop the auto-merge on real findings.

State recommendation: keep decision-needed until the maintainer accepts the short-cap semantics; then move to ready-for-agent with the two tasks above.

[nathanpayne-claude]

Implementing the two-step fix from the triage recommendation (maintainer accepted the short-cap semantics):

• Step 1 — the narrow bug fix: fix(727): sleep only the remaining CodeRabbit rate-limit window (step 1 of 2) #728 — coderabbit-wait.sh now sleeps only the remaining rate-limit window ((window + buffer) − already-elapsed, clamped to 0) instead of a fresh full copy. Directly kills the wasted 210s re-wait observed on PR fix(724): flip stale visibility: private example in templated-propagation.md #725, where the notice's 180s window had expired ~5 min before the wait step even started. Under-threshold; CI green.
• Step 2 — the policy-controlled fast path: feat(727): post-clearance CodeRabbit fast path (step 2 of 2) #729 (stacked on fix(727): sleep only the remaining CodeRabbit rate-limit window (step 1 of 2) #728) — new knob coderabbit.post_clearance_max_wait_seconds (default 240). When HEAD already has a verified Codex/Phase-4b clearance and a reviewer-identity APPROVED (codex-review-check.sh gate (b)+(c)), the auto-merge wait is capped to the short window; CodeRabbit still gets that window and any Potential issue/⚠️ is handled exactly as today, while a still-pending CodeRabbit proceeds via the existing advisory exit-4 path. Fail-safe: the fast path stays disarmed (full budget) whenever Codex/Phase-4b has not cleared HEAD. Protected-path Phase 4.

Per the recommendation, step 2 is deliberately not the fully-async/post-merge option — CodeRabbit keeps its chance to stop the auto-merge on a real finding.