Skip to main content
Back to Blog
ai applied-ai agents reliability evals

A half-finished review looks exactly like a passing one

· 7 min read

I run an agent pipeline over my own GitHub issues. It picks up an issue, writes a plan, checks the plan against a list of paths it is not allowed to touch, has a second model judge the plan, builds the change, reviews its own diff, runs fix rounds against the review, and only then commits and opens a pull request. Eleven or so places where it can stop. Most of them are shell, not model calls, which is deliberate.

Yesterday it nearly pushed code that nothing had reviewed. I want to write down why, because the bug has nothing to do with model quality and everything to do with how these pipelines carry state.

The resume path

A full run costs twenty to thirty minutes. When a run dies late, after the build and the review, throwing the work away and starting from origin/HEAD wastes all of it. So there is a predicate that decides whether an existing worktree can be resumed:

produce_should_reuse_worktree() {
  local wt="${1:-}"
  [ -n "$wt" ] && [ -d "$wt" ] || return 1
  git -C "$wt" rev-parse --is-inside-work-tree >/dev/null 2>&1 || return 1
  [ -s "$wt/.agent-review.json" ] || return 1
  jq -e '.verdict=="pass"' "$wt/.agent-review.json" >/dev/null 2>&1 || return 1
  git -C "$wt" status --porcelain | grep -vE '\.agent-' | grep -q .
}

Read it and it seems careful. A real git worktree, a review artifact that exists and is non-empty, a verdict of pass, and actual uncommitted changes that are not the pipeline’s own bookkeeping files. If all of that holds, the previous run got as far as a reviewed change, so the retry skips planning, building and reviewing and goes straight to committing and pushing.

What the review stage actually does

Review is not one model call. A reviewer reads the diff and writes its verdict. Then a second model argues with it, and the debater’s blocking findings are merged over the first verdict at the end of the stage.

The first reviewer’s verdict is written to disk before the debate starts.

So if the reviewer process is killed during the debate, which happens for ordinary reasons—a budget stop, a timeout, or a machine going to sleep—what is left on disk is a well-formed .agent-review.json containing one reviewer’s pass. It is a real file. It parses. Its verdict field says pass.

The next attempt calls the predicate above. Real worktree, review file present, verdict is pass, changes are there. It resumes, and resuming means skipping plan, build and review. It commits and pushes a diff that no completed review has ever looked at.

Two timelines of the same review stage. In the first, reviewer one writes a pass verdict, the debate runs, and reviewer two's findings are merged over it to form the final verdict, leaving a review file marked reviewed. In the second, the debate is killed partway and reviewer two never runs, leaving a review file at the same path with the same pass verdict, marked not reviewed. The caption notes the artifact holding the result is also the only signal that the stage finished.

What stopped it

A rebase conflict.

The push path rebased onto the updated base branch, the rebase conflicted, and the run stopped there. That is the only reason this shows up in my notes as a near miss rather than as a merged pull request. No gate caught it. The gates were all satisfied, because from their point of view the run had a passing review.

The pipeline drawn as a row of stages. Plan with its deny check, build, and review with fix rounds are greyed out inside a dashed box labelled skipped on resume, while commit and rebase-and-push stay active. An arrow labelled retry sees a reusable worktree jumps over the greyed stages and lands on commit. A panel below lists the four conditions the resume predicate checks and notes that all of them pass and none asks whether the review stage ran to completion.

The shape of the bug

An agent pipeline is a chain of stages that hand state to each other through files on disk. That design is right: the alternative is holding a thirty-minute run in memory and losing everything when anything dies, and things die constantly.

But it means every file is read later by something that was not there when the file was written, and that reader has to infer from the file whether the stage that produced it finished. Here the file said pass, and pass was the result of half a stage. The artifact recording the outcome and the signal that the stage completed were the same object, so a stage that stopped halfway published a verdict it had not finished forming.

Nothing in the pipeline was lying. Reviewer one really did pass the diff. The claim the file appears to make, that this change has been reviewed, was never a claim the file was designed to carry.

The fix is in two halves, which is what tells you it is a state problem rather than a logic error. First, the gates now explicitly delete the review file whenever the review stage exits nonzero, so an unfinished review gracefully leaves no verdict behind. Second, the predicate refuses any worktree without one.

However, either half alone still leaves a window. A shell trap or exit-code cleanup fails on hard crashes like SIGKILL, a runner timeout, or an OOM kill. The general version of the fix, which I now apply to every file this pipeline writes, is atomic writes.

If this process is killed right now, what will the next run conclude from what is on disk? A stage that writes its result before it is final needs to write to a temporary path (e.g., .agent-review.json.tmp), and mv it into place atomically at the absolute end of the stage. Any of the three—temp files, separate completion markers, or clearing on failure—is better than nothing. Just not the thing I had, where an intermediate value sits at the final path wearing the final name.

Two panels comparing write strategies. On the left, writing in place: the review file is written at t1 with only reviewer one's verdict, the process is killed at t2 so no cleanup runs, and a partial result is left sitting at the final path where the next run reads it as approval. On the right, writing to a temporary path and moving it: the file is written to a .tmp path so nothing exists at the final path, the kill orphans the temporary file, the final path stays absent, and the next run rebuilds from scratch.

The test, and what it admits

The regression test has two halves, matching the fix.

The first half is a real behavioral test. It creates a git worktree in a temp directory, writes a passing review file, asserts the predicate accepts it, deletes the file, asserts the predicate now refuses. That exercises the actual function against the actual condition.

The second half is an awk script over the gate source, checking that a rm -f "$REVIEW_FILE" appears within twelve lines of the branch that handles a nonzero review.

That second half is a test of source text. It would pass if the deletion were unreachable. I wrote it that way because exercising the real path means killing a reviewer subprocess mid-debate inside a test harness, and I have not built the fixture that makes that cheap.

I am leaving it in and leaving this paragraph in, because the honest position is that half of my regression test for this bug is a grep. That is the recurring shape of testing agent pipelines. The failures live in the seams between long, expensive, nondeterministic stages, and the seams are exactly where a cheap test cannot reach. Every time I take the cheap test, I should know which of the two things I bought.

Every stage in a pipeline like this will be killed partway through eventually. Mine are killed most days. The question worth asking of the whole design is not whether the model reviewed well. It is what each file on disk means to the next process that reads it, and whether that meaning survives the writer dying mid-sentence.

Have a similar challenge?

I help SaaS companies and WordPress platforms solve their most complex technical problems.

Let's Talk