Having a second agent review the first agent’s work, and what it found

Mullet’s own rule is that whoever writes a change doesn’t get to judge it. A worker writes, the gates check, and a separate Claude session with no part in the writing reviews the diff. The reviewer that can only say no is about that reviewer.

I hold the work on Mullet to the same rule. The changes for 2.3.0 were written by one agent. Then, as a separate pass, a second agent that had no part in the writing was given the diffs and asked to break them. It did that twice this round. Some of what it found was in the new container backend, and that is in a container backend stricter than the usual Docker recipe. This post is about the rest.

The one that mattered

2.3.0 added a re-ask. Sometimes a review session ends without its JSON verdict, and before this round that paused the whole job until I came back to it. The re-ask resumes the same session and asks for the verdict alone:

Your review ended without its verdict. Don't review again or read more files.
Answer now with only the JSON verdict (verdict, findings, reasons) for the
change you have just reviewed, following the instructions you were given.

The first version asked again whenever a review had no verdict. That included a review cut off by its turn limit or its clock. A review that stopped there may have stopped halfway through reading the change, and “answer now, don’t read more files” gets a verdict on whatever it had read so far. If that verdict was not a block, and every other gate had passed, the step went through. A feature meant to save a paused job had become a way to pass a step whose review never finished.

The fix is that only a session that ended its turn itself may be asked again:

def _finished_on_its_own(result: ExecResult) -> bool:
    """True when the session ended its turn itself: not cut off by a turn limit, its clock or a crash.

    Only such a session may be asked again for its verdict. The executor sets
    `error` for every other ending (the turn limit's `error_max_turns`, "timed
    out", an idle stop), and `aborted` for a crash.
    """
    return not result.aborted and result.paused_until is None and result.error is None

A review cut off part way pauses the job, as it did before the re-ask existed. Two rules went in beside it. A verdict that only came on a second asking is marked, and on a review that’s forced for safety (a one-way step, or a risky step in a ladder job) it doesn’t let the step through: the job pauses for me. A block on a second asking still fails the attempt, since blocking is the safe answer.

The rest

None of these were dramatic, and none showed up on the happy path.

• The reviewer’s findings were dropped when it named a file by its absolute path in the clone, or with a leading ./. Mullet compared the path with the list of changed files, found no match and treated the file as untouched, so the next attempt never saw a precise, located finding. Both forms now reduce to the repo-relative path. A path outside the clone still never counts.
• A git command on a job clone that timed out was killed with SIGKILL straight away. Git had no chance to remove its lock files, and the next git command on that clone failed on them. It now gets SIGTERM, then 5 seconds to clean up, then SIGKILL.
• YAML reads .inf as infinity and .nan as not a number. Either one in a jobs: setting broke every git command on a job clone. Now a bad value is reported by mullet config show and its default kept, and the jobs settings never raise.
• Planner, worker and reviewer text reached the terminal with its escape sequences intact, so text a model wrote could recolour the screen or move the cursor. It’s now stripped of escape sequences and control characters before it reaches the terminal or a commit, and single-line fields are flattened so a plan can’t fake a section of the approval view. mullet job show --attempt --diff strips escapes on a terminal and stays exact when piped.

How the review was set up

Three things made it worth doing.

The passes were separate. The agent that wrote a change never reviewed it, and the reviewer started from the diffs, not from the conversation that produced them. I didn’t want it reading the author’s reasons before it had read the change.

The reviewer had to report concrete failing scenarios, not opinions. Each finding said what to do and what went wrong when you did it: a review stops at its turn limit and is asked again; the reviewer names a file by its absolute path. A scenario can be checked. An opinion that a function “looks fragile” can only be argued with.

Every fix started with a failing test that reproduced the scenario, and then the change that made it pass. The re-ask has tests for a review cut off by its turn limit or its clock, and for a re-ask that is itself cut off. The settings have tests for .inf, -.inf and .nan in each numeric jobs: value, and one that runs a git command on a job clone with a bad setting in place. The git timeout has a test for a git that ignores SIGTERM. Mullet has about 18,500 tests, and the full suite runs in about five minutes, so running all of it after every fix was cheap.

What the review didn’t cover

It read the diffs for this round. Code the round didn’t touch wasn’t in front of it, and any bug that lives there is still there. It didn’t run real jobs against real providers; what it found, it found by reading and by the tests its scenarios turned into. And a review that finds nothing proves nothing. I can count what it found. I can’t count what it missed.

That is also why the reviewer inside Mullet can’t pass a step. A second reader is good at finding faults and no use as a guarantee, so its job is to add a way to fail.

The code is at https://github.com/tomgould/mullet-public.