I Could Not Reproduce It and Fixed It Anyway

A reviewer I had finished building two days earlier told me that a repository under review could hand it a different model to think with. I spent most of an hour trying to make that happen, from four directions, and never once got it to work. Then I deleted the mechanism it had named.

That is not how I expected the thing to earn its keep.

Reviewing your own handwriting

Nearly every line of code in this workspace is written by a Claude agent, and most of it is then reviewed by a Claude agent. That arrangement catches a great deal. It also has a structural problem: the author and the reviewer share a training run, a house style, and a set of habits, which means they share a set of things neither of them thinks to look at.

I have a dated example. On the seventh of August I wrote a rule into this workspace’s charter and pasted it verbatim into the standing instructions every agent here reads: the presence of an environment variable is diagnostic evidence, never authorization. Any local process can set one, so a marker may gate logging and refusal messages, and may never gate authority.

Seventeen days later I shipped a tool with an environment variable that let any local process swap out the executable after the billing guard had already approved the run, and inherit the review’s arguments on the way through.

I wrote the rule. I wrote the violation. Reviews happened in between, and nothing came back.

So I built a wrapper that runs a different vendor’s model over a diff, read-only, with the intent behind the change fed in beside it. The disagreements are the entire product. The sharpest target is the code that supervises my build agents, which those agents are forbidden to edit, and which is therefore at once the highest-consequence and the least-reviewed code I own.

Six defects, sixteen passing tests

I pointed it at itself first.

It came back with six defects in code that already had sixteen passing tests. Two were not ordinary bugs but holes in the tool’s own authority. The environment variable above was one. The other was that every review loaded my user-level config file for the underlying CLI, and that file can name a custom model provider backed by metered billing, or register helper servers whose commands run outside the sandbox. The tool has exactly two guarantees, that it can never spend money and can never write, and both of them had a door.

The other four were smaller and more embarrassing. The best of them: a roster of project paths beginning with a tilde, which the runtime does not expand, so the sweep meant to cover five repositories resolved zero.

The interesting part is not the list. Two of the six were sitting in argument-parsing code that lived inside the executable itself, where no test could import it. Sixteen tests, and the functions deciding what the tool actually does were reachable by none of them. Moving two of them into a module was most of the fix for the whole category.

The one I could not reproduce

I fixed the six, then ran a second review over the repaired code. It confirmed five of the fixes, disputed one, and raised three new findings.

The disputed one said my config escape was still open, through a different door: the reviewed repository’s own project-level config, which would load because the invocation marked the target as trusted before reading it.

I tried to make it happen. I planted a model in a project config and ran with the trust marker, without it, with the user-config lockout, and without. Version 0.149.1 ignored the planted file every time. I could not get that layer to load at all, and I still cannot.

I removed the trust marker anyway.

Not because the reviewer was right, because on the mechanism it was not. I removed it because of the sentence I would have had to write to keep it: I mark a repository as trusted in order to review it. Marking something trusted so that you can go and find out whether it deserves trust is backwards regardless of whether the trust currently buys anything. The headless runs, which is all of them, never needed it. The finding was wrong about the code and right about the design, which is a thing a review can be, and which I would have missed equally by believing it or by dismissing it.

There have only ever been three of these reviews. The third used up the account’s allowance for the month and it does not reset until late September, so the tool has still never run against the supervisor code I built it for. When the allowance ran out the underlying CLI refused to continue rather than fall back to metered billing, which is the one guarantee I built three separate layers to enforce, and so far the only one that anything other than me has tested.

The two it never mentioned

Verifying those three findings meant reading the argument handling properly for the first time. The two best fixes of the whole exercise came out of that, and neither review said a word about either.

The first: one flag took a git revision in one command and a human date in another. Same flag, same tool, two grammars, and no reason on earth for me to carry the distinction in my head.

The second is underneath it. Git’s date parser has no failure mode. Feed it a word that is not a date and it does not object, it returns approximately now. So a mistyped revision quietly became a date, and the two directions of typo failed in opposite ways: a garbled word resolved to the current commit, and the tool announced there was nothing to review; a garbled revision expression, a tilde and a number and a fumbled suffix, resolved to a date far in the past, and the tool reviewed the repository’s entire history. One character wrong, and either answer looks like an answer.

The guard for it is small. Nothing containing a tilde, a caret, or an at-brace is a date, so anything carrying those characters was meant as a revision, and if it does not resolve it is a mistake rather than a timestamp.

Which promptly rejected the tool’s own starting point. When the sweep meets a repository whose history begins inside the review window there is no earlier commit to diff against, so it uses git’s empty tree: a real object, not a commit, and therefore indistinguishable from a fumbled suffix to a check that asks only whether a commit exists. The base for every new repository the sweep will ever meet was thrown out as a typo by a guard written to catch typos, and I found it because a reviewer had told me something that was not true.


← all writing