Re-poll CI state at delivery to skip superseded failures

[subsetpark]

Summary

• When a Ci Respond action finally dispatches, the failure that triggered it may have been superseded by a newer run. The orchestrator snapshot is only refreshed once per poll cycle (30s), and the semaphore wait can push actual delivery minutes later — so we wake agents for failures that have already cleared.
• The runner now re-polls GitHub just before the delivery slot (same placement as the existing Review_comments prefetch). If the fetch fails or no current failures exist, we log and short-circuit to Skip_empty; otherwise we write the fresh list into orchestrator state via set_ci_checks so respond_delivery reads a current agent.ci_checks.
• The Ci-specific is_empty path moves out of respond_delivery entirely — freshness is the caller's concern (it requires I/O), the pure decision function just builds the payload from current state.

Test plan

• dune build clean
• dune runtest — existing RD-1/2a/2c/3/4/5/6/7 still pass; RD-2b/RD-8/RD-9 removed (tested behavior that moved out of the pure function or a reverted parameter)
• dune fmt clean
• Observe on a live CI-failing PR: confirm "Fetching fresh CI state from GitHub" log, then either delivery or "Fresh CI state shows no failures — skipping CI delivery"

🤖 Generated with Claude Code

---

Summary by cubic

Re-polls GitHub for CI right before delivery and skips superseded failures with precise skip reasons to avoid stale alerts and wasted Claude slots. Fresh CI checks are written under the busy guard so respond_delivery builds from current state.

• Bug Fixes

  • Re-poll PR state before CI delivery; if no PR number, fetch fails, or no failures remain, log the reason and short-circuit to Skip_empty before taking a Claude slot.
  • Persist fresh ci_checks via Orchestrator.set_ci_checks inside the busy guard (before the agent re-read) to avoid races with the poller.

• Refactors

  • respond_delivery reads CI checks from agent and keeps a belt-and-suspenders guard using Ci_check.is_failure; the caller owns freshness and the set_ci_checks write. Simplified the CI skip fast path by removing a redundant is_ci guard.
  • Unified GitHub prefetch for Review comments and CI; improved skip logs to distinguish fetch failure vs no current failures vs no PR number.

^{Written for commit db3a9de. Summary will update on new commits.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
onton	Ready	Preview, Comment	Apr 17, 2026 8:33pm

[coderabbitai]

Warning

Rate limit exceeded

@subsetpark has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 4 seconds before requesting another review.

Your PR has hit rate limit. Contact your admin to purchase credits for running PR reviews or try again in 7 minutes and 4 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 019c530c-b9b6-4c9b-a7cc-fdb3d089a928

📥 Commits

Reviewing files that changed from the base of the PR and between b75df6e and db3a9de.

📒 Files selected for processing (4)

• bin/main.ml
• lib/patch_decision.ml
• lib/patch_decision.mli
• test/test_patch_decision.ml

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch zax--ci-freshness-gate

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 4 files

[flowglad-review-service]

Code Review: 4 comments

[flowglad-review-service]

Review Summary

The PR correctly identifies and addresses the staleness problem for Ci deliveries by adding a GitHub re-poll before the claude slot. The design split (freshness gate in the runner, payload building in the pure function) is sound. However there are two correctness issues and one quality concern:

1. set_ci_checks fires before with_busy_guard (must-fix, bin/main.ml): The orchestrator write happens outside the busy-guard scope, so in a stale/cancelled delivery the write is spurious and races with the poller's concurrent set_ci_checks. Moving it inside with_busy_guard (after the agent re-read) fixes this.

2. respond_delivery can emit Ci_payload { failed_checks = [] } (must-fix, lib/patch_decision.ml): Removing the is_empty guard for Ci makes the pure function unsafe in isolation. A belt-and-suspenders not (List.exists agent.ci_checks ~f:Ci_check.is_failure) check should be re-added in the is_empty arm, making the function correct regardless of whether the caller remembered to populate ci_checks. This also justifies reinstating test RD-2b.

3. Fetch-failure skip reason is lost in the terminal log (should-fix, bin/main.ml): The Skip_empty log inside with_claude_slot says "nothing to deliver" even when the real cause is a transient GitHub API error, making debugging harder.
   | Severity | Count |
   |----------|-------|
   | Must-fix | 2 |
   | Should-fix | 1 |
   | Note | 1 |

---

4 comments posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 338840 in / 7590 out · Cache: 0 created / 60644 read · Context: 5 items · Review mode: agentic · Turns: 15 · Tool calls: 19 · Forced submit: yes · Tool mix: read_file=14, search_codebase=5

[flowglad-review-service]

Code Review: 3 comments

[flowglad-review-service]

Review Summary

The four must-fix issues from Turn 1 are now correctly addressed: set_ci_checks is placed inside with_busy_guard, the CI skip log distinguishes the reason, the belt-and-suspenders is_empty guard is restored in respond_delivery with documentation, and RD-2b is back in the test suite. The core correctness concern is resolved.

Two new should-fix observations on the newly introduced code:

1. Silent pr_number = None branch misattributed as "fetch failed" (line 2687): when a CI Respond fires before the PR exists, fresh_pr_state = None without any log, then ci_skip_reason = Some "fetch failed" even though no HTTP fetch was attempted. The comment "Log already emitted by the fetch branch" is only true for the HTTP-error sub-case.

2. Semaphore slot consumed for a predetermined skip (line 2723): when ci_skip_reason = Some _, with_claude_slot is still entered just to return Skip_empty, needlessly holding a Claude concurrency slot. The short-circuit should happen before the semaphore acquire.

One note about Respond_stale being unreachable on the ci-skip path — observable only in logs, and state cleanup is correct.

Severity	Count
Must-fix	0
Should-fix	2
Note	1

---

3 comments posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 261669 in / 6796 out · Cache: 0 created / 60644 read · Context: 5 items · Review mode: agentic · Turns: 15 · Tool calls: 19 · Tool mix: read_file=14, search_codebase=5

[flowglad-review-service]

Code Review: 1 comment

[flowglad-review-service]

Review Summary

All seven issues raised across the two previous review turns have been fully addressed in this diff:

1. set_ci_checks write now inside with_busy_guard (Turn 1 must-fix) — The write to agent.ci_checks via Runtime.update_orchestrator now occurs inside with_busy_guard, inside with_claude_slot, and before the agent re-read, eliminating both the poller race and the stale-delivery corruption risk.

2. Distinct skip-reason log messages (Turn 1 should-fix + Turn 2 should-fix) — "no PR number", "fetch failed", and "no current failures" are all distinguishable in logs. The no-PR-number path now emits its own diagnostic ("No PR number yet — skipping CI state fetch") before computing ci_skip_reason.

3. Belt-and-suspenders guard restored in respond_delivery (Turn 1 must-fix) — is_empty for Ci now uses not (List.exists agent.ci_checks ~f:Ci_check.is_failure), preventing an empty Ci_payload from leaking through if a future caller skips the freshness hop.

4. RD-2b test re-added (Turn 1 note) — The belt-and-suspenders path is covered by a unit test again.

5. with_claude_slot no longer consumed on skip (Turn 2 should-fix) — The fast path short-circuits before with_claude_slot, preserving semaphore capacity for agents doing real work.

6. Stale-during-skip window eliminated (Turn 2 note) — Because the fast path exits before any semaphore wait, the window where a stale agent could produce a misleading log is gone.

One cosmetic observation remains (new in this diff): the when is_ci guard on line 2728 is a dead condition since ci_skip_reason is structurally None when not is_ci.

Severity	Count
Must-fix	0
Should-fix	0
Note	1

---

1 comment posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 362674 in / 4631 out · Cache: 0 created / 60644 read · Context: 5 items · Review mode: agentic · Turns: 15 · Tool calls: 16 · Tool mix: read_file=11, search_codebase=5

[flowglad-review-service]

Review Summary

All seven outstanding issues from the three previous review turns have been fully addressed in this diff:

1. set_ci_checks write ordering (Turn 1, must-fix): Now called inside with_busy_guard / with_claude_slot, before the agent re-read, eliminating both the poller race and the stale-delivery state corruption path.

2. Misleading skip log (Turn 1, should-fix): The skip path emits "Skipped ci delivery — %s" reason with a concrete, accurate reason string rather than the generic "nothing to deliver".

3. Belt-and-suspenders is_empty guard (Turn 1, must-fix): Restored in respond_delivery using Ci_check.is_failure, ensuring the pure function never emits an empty Ci_payload even if a future caller omits the pre-fetch.

4. RD-2b test (Turn 1, note): Re-added with an updated comment explaining the belt-and-suspenders role.

5. No log for pr_number = None (Turn 2, should-fix): Explicit log "No PR number yet — skipping CI state fetch" is now emitted, and the reason string is "no PR number" (not the misleading "fetch failed").

6. with_claude_slot consumed on skip (Turn 2, should-fix): ci_skip_reason = Some _ short-circuits before entering with_claude_slot, preserving concurrency capacity for real work.

7. Dead when is_ci guard (Turn 3, note): The ci_skip_reason match has no spurious when is_ci guard; the condition is expressed structurally.

No new issues were found in this diff. The implementation is correct.

Severity	Count
Must-fix	0
Should-fix	0
Note	0

---

0 comments posted · Model: claude-sonnet-4-6 (sonnet) · Tokens: 290692 in / 3381 out · Cache: 0 created / 48446 read · Context: 5 items · Review mode: agentic · Turns: 12 · Tool calls: 13 · Tool mix: read_file=9, search_codebase=4