feat(review): delegate verification to the repo's CI and attribute red checks

Merged
KH
2mo ago
+493-40

Pieces 1 and 3 of the shift to CI-delegated verification. Piece 2 (making the CI reaction authoritative) mostly falls out of the existing check_run.completed trigger; piece 4 (bounded fix-forward, then back out) is deliberately not here — see below.

1 — Stop verifying in-box

The prompt told the harness to install dependencies and run the repo's full build/test/typecheck so it could verify its own edits "the way CI does". That is what exhausted an 8 GiB sandbox on a large monorepo and produced the exit-137 OOMs — tsc alone takes multiple GiB, and a test runner defaults to one worker per core.

It also never scaled. The cost is set by the customer's repo size, which we don't control, while the box is capped at Daytona's hard per-sandbox maximum. The repo's own CI is by definition sized for the repo.

This removes the memory consumer rather than capping it. The caps from #134 stay as a safety net, but nothing installs, builds or tests in the sandbox any more.

The part that makes it safe

Delegation only works if it narrows what the agent edits. Pushing edits it cannot verify is precisely how three PRs were broken in a day (cloud#1926), and I flagged that risk before we chose this direction.

So the prompt now permits an edit only when it can be justified by reading the code, and requires anything that would need a build or test run to be raised as a suggestion instead. Generated artifacts and cross-package importers — the blast radius the full build used to catch — are named explicitly as suggest-don't-edit territory. Reading the whole checkout is still encouraged; that's how an edit gets justified without running anything.

3 — Attribute the red check

The agent now finds out about its own breakage the way a human does: a failing check afterwards. That's only actionable if it can tell "I broke this" from "this was already red" — otherwise it either chases someone else's pre-existing failure forever or ignores damage it caused.

attributeCiFailure compares the CI state observed before the pass that left edits against the head the failing check belongs to, and the prompt steers on the result:

  • ours → treat it as your regression and the first thing to fix
  • pre-existing → not your job; report it, don't chase it (silently clearing unrelated CI is the scope creep the rest of the prompt forbids)
  • unknown → read the output before assuming either way

It does not claim certainty, by design. A human pushing to the same PR also moves the head, so ours means "our edits are in the range that could have caused this", not proof. And leftEdits is an upper bound — the agent can't see cloud's push outcome, so a pass that changed nothing still records true. That biases attribution toward blaming ourselves first, which is the safe direction: investigating our own edit costs a read, missing our own regression costs the author a broken PR. It steers attention only and takes no destructive action.

Test changes, called out explicitly

Two existing prompt assertions encoded the old in-box contract (verify it the way CI does, canonical build and test command end to end). Those expectations were updated to the new contract rather than deleted — the guarantee they protected (don't break what you can't see) still holds, only the mechanism changed, so the replacement asserts the suggest-don't-edit rule instead.

A third failure was my own regression: the serial-execution lines I added in #134 landed mid-sentence and split the git restore carve-out across two lines, silently shortening what that test could assert. Fixed in the source, not the test.

Verification

  • 5 new tests for attribution and prompt steering, plus the rewritten delegation test.
  • Mutation-tested, each reverted after measuring: dropping the leftEdits guard, dropping the already-red guard, ignoring the head-moved condition, and removing the guidance from the prompt — all four fail the suite. A fifth mutation (removing the green-CI guard) didn't compile, so it's inconclusive rather than passing; that path is covered by the doesNotMatch(/CI is RED/) assertions.
  • npm test: 365 pass. The 2 failures are the pre-existing environmental ones (../workforce/packages/harness-kit has only dist/).
  • npx tsc --noEmit: clean.

Not in this PR

Piece 4 — bounded fix-forward, then back out. The agent still cannot revert its own push, so today a regression it can't fix forward stays on the PR. That's the guardrail that would have contained cloud#1926, and I'd want it before this runs widely on customer repos.

Worth deciding alongside it: whether the agent should be suggestion-only until the CI-reaction loop has proven itself. That would get the OOM fix with zero push risk.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BmMcdDakbnDkHBxG4pit7z

Summary by cubic

Delegates review verification to the repo's CI and attributes red checks, removing the in-box install/build/test that exhausted the 8 GiB sandbox on large repos. The agent now only edits what it can justify by reading code; anything needing a build or test becomes a suggestion.

CI failure attribution

  • Records the CI state seen before each pass and compares it to the failing check's head to classify failures as ours, pre-existing, or unknown.
  • Own regressions are fixed first; pre-existing failures are reported but not chased; unknown ones require reading the output before deciding.
  • Ownership is sticky until CI is green, so a failed repair pass can't hand the regression back to the author.
  • The failing check's name, conclusion, and output are lifted from the webhook payload and handed to the harness, since it can't run builds or tests itself.
  • readPr now reads the head sha from the shapes GitHub actually sends on check_run events, which previously left attribution inert in production.
  • Deliberately conservative—biases toward blaming our own edits, which is the safe direction.

Two prompt assertions encoded the old in-box contract and were updated to the new one; a third failure was a regression in the source, fixed in source. The bounded fix-forward loop (revert when a fix can't be made) is deliberately not in this PR.