Every Reviewer Was Right, and the Pull Request Was Still Wrong

I am a product manager, not an engineer. When an automated reviewer tells me a change has a P1 correctness defect, I am usually not in a position to prove it wrong, and I do not try to. That is what the reviewers are for. So “there is a real bug; should we fix it?” is not much of a decision. Of course I say yes.

Over eleven days in late August and early September, two pull requests in Mergepath, the review-policy repository that governs how my coding agents open, review and merge work, faced that question over and over. Between them they drew 72 findings from the Codex GitHub App across 31 review rounds. I have now read every one, and I cannot find a finding that was wrong. A correct finding is not a justified demand, though: whether a defect had to be fixed before merge was a product question, never the reviewers’ to answer. Both pull requests closed without merging. One was replaced by a change to a single word.

The tempting article is that automated review caused scope creep. I started writing it, ran the same analysis on three pull requests that had just as many rounds and merged fine, and it fell over. This is the one that survived: the reviewers were doing their job the whole time, and nobody was doing the other job, which was mine. I was there all day, answering every prompt. A neutral menu still constrains decision rights when every option on it accepts the framing.

Two Loops, Same Pipeline, Same Fortnight

The first pull request, #1112, was a provenance change. It implemented #1056: when Mergepath bootstraps a new consumer repository, record which revision of the template it was built from, so a later drift measurement has a baseline. The issue proposed the whole implementation in one line, the commit hash in the initial commit’s subject and a trailer, and the first commit did that in 32 lines.

The second, #1189, was a diagnostic-clearing change. It implemented #1188: a diagnostic check-run that the merge workflow publishes on an infrastructure error could only be published as a failure. Nothing published a success for the same name, so one transient error left a pull request’s head red permanently, and a local guard then demanded a break-glass merge for a pull request with every required check green.

What Happened to the Provenance Change, #1112

The pivot is round 4. The first three rounds found genuine defects in the original ask, a wrong or unresolvable hash in three different ways; all were fixed, and the recut kept two of the resulting checks. Round 4 raised something different: if the source checkout had uncommitted changes, the mirror would copy those bytes while recording the clean commit hash, so, the finding said, require the source to be clean before accepting its hash. That is the moment the implementation acquired a guarantee the issue had not asked for: “record which revision this was based on” became “prove the recorded hash characterizes the bytes that were mirrored.”

The break is round 8, and the reviewer did not cause it. Codex found that the resume path, which reruns a partially completed bootstrap, could leave stale files in the target while still recording a clean source hash, and asked the author to validate the resumed target before attributing it. The contract-preserving fix was one line: do not attribute on resume. The authoring session instead added --delete to the rsync invocation and built a residue-reconciliation engine around it. A provenance feature now deleted things.

Everything after that is the reviewers being right about the engine. In round 12, when the target directory happened to be named after an excluded path, the engine deleted the entire target, repository and operator work included. After a round-15 trailing-slash fix, CodeRabbit noticed that a target of / now normalized to an empty string, so rsync ran with --delete against the filesystem root. Ten data-loss findings, nine of them P1s, every one real, every one in code that did not exist when the pull request opened, and every one fixed with a regression test.

By the end, the bootstrap script had grown from 1,498 lines to 2,102 and its test file from 2,088 to 3,607; the diff was 61 times the size of the first commit. The recut, #1197, kept the hash, the trailer, the origin and reachability checks and a plain clean-tree gate, and dropped the proof, the configuration pinning and the engine.

The Rigged Question

Here is what I saw of the provenance change as it happened, from the session’s transcript, read directly.

After the task list at 01:36 UTC on August 27, rounds 1 through 10 ran with no input from me. From the afternoon on, the session asked me how to proceed five times, each time with three options; here are all five, with the option I chose first in each.

I clicked the first option all five times. Only the 17:48 prompt marked it Recommended, with the gloss “matches what you asked for last time”; at 20:16 the gloss was “same instruction as before.” After the third click, at 20:23, I typed the only free-form instruction of the afternoon: “Then do one more @codex round.” At 22:28 I pasted a link to one more finding, and a minute later I typed “fix that and admin merge.” The merge never happened, and the pull request sat untouched for nine days.

The prompts were not hiding trouble. Every one said the loop was not converging; the 17:48 prompt said the pull request had already introduced two regressions of its own, and the 20:16 prompt called the next finding “the same class as several already-fixed spots.” What none of them did was relate any of that to the issue. Not one quoted a line count; the size of the diff does not appear in the record until September 6, after I had read it myself. What no prompt listed was the guarantees the implementation had accumulated beyond the single one #1056 asked for. Not one said that the current P1 sat in an engine added in round 8 to satisfy a round-8 finding. And not one offered “remove the mechanism,” “weaken the guarantee,” “return to the original requirement,” or “close and recut.” Four of the five menus were neutral, and I clicked the same slot anyway. A neutral menu still constrains decision rights when every option on it accepts the framing.

The session’s own review replies show where that framing came from: seventeen of them quote a round or finding count, and its three non-convergence deferrals, correct each time, called the findings “new, distinct, genuinely valid edge cases” without once saying that the class existed because of one decision made in round 8, which could be reversed.

For a product manager who trusts the reviewer and will not knowingly ship a P1, “fix it” against “ship with the defect” or “go read the code yourself” is not a real choice. “Remove the thing creating the P1” would have been, and I could have answered it in a sentence.

Everything else I typed to the session about #1112 came on September 6: an instruction to resolve merge conflicts, then, after I read the diff, the decision that closed it, the contract for the recut, and two notes on it. In the decision I wrote that an issue labelled small should not need changes capable of deleting repositories and operator work, and that once it did, preserving the branch because a lot of correctness work had gone into it was the wrong optimization.

The session closed #1112 and cut #1197 against a three-check contract I gave it. Under that contract the next round still drew three findings: two real violations of the stated clean-tree check, fixed, and one about an adversarial caller’s environment, rebutted as outside the contract. That is what a frozen contract buys: not fewer findings, a basis for saying no to one.

What Happened to the Diagnostic-Clearing Change, #1189

#1189 is a different shape, and I got it wrong the first time I described it: it did not grow out of control, and its problem, a guarantee rather than a line count, was in the first commit.

#1188, the issue, was filed by an agent at 03:59 UTC on September 5 and listed three shapes for a fix. The third was to publish the diagnostic as neutral instead of failure, visible but non-blocking, which the issue called “the cheapest option and closest to what the record actually means.” Sixty-four minutes later the pull request opened with option one, a clearing path that would publish a success to supersede the failure, and the first commit’s header rejected the cheap option explicitly: “The failure conclusion stays failure rather than softening to neutral. ‘We could not verify this is safe to merge’ should block; the defect was the missing exit, not the severity.”

Nobody was asked to agree with that sentence. It was a product decision, made in a code comment, and it was wrong, because the diagnostic is not a required status check on the hub or on any of the three consumer repositories checked. It blocked nothing except our own guard script. Twelve rounds of review then hardened the implementation of a requirement that did not need to exist.

And hardened it correctly. The first round found two real false greens: the clearing path created a competing success run instead of updating the failure, and a clear was not pinned to the head that produced the verdict. Then the ordering problems began. The authoring session deferred two of them to follow-up issues, with cost arguments. The external reviewer, a Codex pass under a separate identity that holds a merge veto in this repository, overruled both: “Both affect the core merge-gating guarantee and should be resolved before merge.” The session complied, and its note on complying is the most honest sentence in the record: “The review overruled my deferral and it was right to.”

It was not: the diagnoses were right, and the demand was a product judgment. The watermark the review had demanded re-opened, in the opposite direction, the race over which conclusion buries which, and was split back out 95 minutes later. In all, five mechanisms were built to establish which of two workflow invocations happened first, over an API with no atomic primitive for it, and each closed one interleaving while opening another. Findings per round never reached zero and spiked to six in round 6, and roughly half the later ones were interactions between rules added the round before.

At 02:55 UTC on September 6, the pull request’s contract was frozen to five guarantees, with a false red left by concurrent invocations declared an accepted residual, and the external reviewer approved with zero findings. Then Codex reviewed against the frozen contract and found two more real P1s inside it: a runner whose clock ran slightly ahead of GitHub’s could classify an unobserved failure as older and clear it, and a base branch that advanced under an unchanged head could authorize a clear for a merge context that was never evaluated. Both false greens. Both inside the five guarantees.

Seven minutes after that round, #1196 opened. One word: failure became neutral. With no red state to leave, nothing has to clear it, so there is no clearing path to get right. It merged in twenty minutes.

Four Articles the Controls Killed

Two spectacular failures support almost any explanation, so I ran the same analysis on healthy pull requests. Of the 507 closed pull requests in the repository’s history, 15 closed unmerged; of the 21 with eight or more Codex rounds, 19 merged, and only these two closed. Each of the following explanations survived every inspection of the two failures and failed against a control. That is a narrower result than it sounds: none separates the two closed pull requests from the merged ones, which undermines the simple diagnostic without naming the cause of either outcome.

Too many review rounds cause divergence. #1084 ran 19 rounds, drew 66 findings, and merged. #925 ran 18 and merged at 3,369 lines. A long review that merged shows only that long reviews can merge, not that the loop was healthy or worth its cost.

Findings in machinery added for earlier findings indicate divergence. #1139 is an 85-line routing change whose author volunteered a bootstrap guard on top of it. Fifteen of its 21 findings were about the guard, seven of them successive holes in one flag extractor: “the fifth instance of one root cause,” the author wrote, and then “the sixth way this extractor has validated a subset.” It merged. Its share of such findings, a third, is higher than #1189’s.

Freeze the contract and review goes quiet. Both frozen contracts drew further valid findings, the two P1s inside #1189’s five guarantees and the two clean-tree violations inside #1197’s three checks. Freezing did not quiet review; it changed the basis for accepting or rejecting a finding.

Large implementation growth explains both failures. #1112 grew 61× from open to close, and the largest growth on any of the 19 merged high-round pull requests is 11×. That number is real and isolates #1112 alone; #1189 grew 4.3×, which is unremarkable. Its problem was a guarantee accepted at open, 275 lines for an issue whose cheapest listed option was one word, and no open-to-close measure can see a gap between the issue and the opening commit.

What Actually Separated Them

Two differences survive the comparison.

The middle column is approximate and single-rater; the pattern is not subtle. Every merged control accepted one to three stronger guarantees and fixed the rest as defects in a contract that was broad at open, and every one contains the same move, made by the authoring agent without a human asking: it deleted the thing the findings were about. #1176, answering round 7: “removing the thing that produced this finding rather than patching it a fourth time.” #1139, answering round 10, in a reply that counted every reviewer’s pass rather than Codex’s alone: “I am splitting the guard out rather than taking a fourteenth round on it.” #1084, answering round 12: “Both fixed, by deleting the mechanism that caused them,” though its hand-rolled field reader stayed and drew findings until the merge, which is why its row says partly.

That move never happened on the provenance change. It happened twice on the diagnostic-clearing change and did not help, because the mechanisms being deleted sat on top of a clearing path that was itself the unnecessary requirement. The agent questioned its ordering tokens and never its clearing arm. An agent is least able to question the premise it opened with, and that is the one place in this record where the human was not optional.

The cleanest evidence is the natural experiment I did not design.

The reviewers did not change. The contract did, and someone was there to hold it. The recut also had ten days of hindsight, so the figure shows what a contract plus hindsight bought, not the contract alone. And here is where #1112’s guarantees came from, by round.

The Question I Can Actually Own

The controls show that long, repetitive, technically frustrating loops can be healthy. What the healthy ones had was somebody periodically asking whether the thing containing the error should exist.

That is a question I can own without reading the implementation. Grant the reviewer its premise: yes, that is a real bug. Then ask two things the reviewer is not answering:

Does the mechanism containing this finding exist because the original issue requires it?

And what happens if we leave the residual unfixed?

The second question gives a third answer beside fix and defer: accept it. #1084 merged while its hand-rolled parser was still drawing findings, because the pull request had bounded the cost of a wrong answer to a skipped review wait, not a skipped review. #1196 shipped on an assumption about how GitHub treats neutral that could not be verified in advance, with the failure mode stated as today’s behaviour and the remedy as reverting one word. Neither decision needed a line of code read. Both needed the failure cost stated in product units.

So my decision rule is now simple, and deliberately not automatable. When a reviewer finds a real error, fix it, unless the fix is in machinery added beyond the original requirement. Then stop and reconsider the machinery before fixing the error. “Beyond” does not mean unmentioned: an issue rarely spells out every property its outcome needs, so the test is whether a guarantee is necessary to deliver what was asked for or is an additional commitment; not attributing on resume kept #1056’s promise honest without a deletion engine. The reference is the issue’s problem statement, not the pull request’s chosen design, or #1189’s clearing arm passes as original and the rule catches nothing.

For someone in my position to use that rule, the agent has to carry what it now throws away: what was originally asked; which guarantees the implementation now provides beyond that; which mechanism each guarantee required; which finding caused each mechanism; and whether the latest finding concerns required behaviour or added machinery. That lineage is what makes the decision possible without the decider reading the code. #1112’s “new, distinct, genuinely valid” framing destroyed that information; the healthy controls reconstructed it in prose, by counting instances of one root cause.

And the guarantees have to be a named list, never a score. A number invites a threshold, and a threshold is satisfiable without anyone thinking; #1112 ran past a ten-round escalation policy that was in force throughout. The escalation I can imagine deciding from, filled in for #1112 at round 12, looks like this:

The facts in that figure are ones the session had and did not report; the removal cost is my reconstruction, the reasoning an escalation should carry. I do not need to understand rsync --delete to notice that something is off in it. And a system that marks “fix” as recommended because the finding is valid has answered the engineering question and skipped the product one.

Where the Decision Rights Go

The operating rules that came out of this put the decision rights at the points they govern, not in a second rulebook beside the review policy: the owner ratifies the proposed contract against the issue when the pull request opens, with the named guarantees and the residual cost visible, which is where #1189 should have stopped; the agent reports lineage and every guarantee added since open at escalation, with a menu that includes reduce, remove, recut and accept; reviewers keep finding defects.

What matters more than the document’s rules is that three claims caught in this work were mine, or repeated by me. The size-S label I had cited as the issue’s original estimate was, the label history showed, applied by a backlog audit nine days after the pull request opened. The document’s first draft said #1189 ran ten rounds and seventeen commits; the API says twelve and twenty-one, and Codex caught it in its first round. And the first published version of this post numbered the provenance change’s later rounds the way the session’s prompts counted them, one higher than the rule in the sidebar, until a reader’s review caught it. Three unmeasured claims, in material whose entire thesis is to measure before accepting an obligation, each caught by the thing the material was about. That is the thesis demonstrated, not evidence against it: measurement owned the facts, and could not decide, on any of the three, whether the corrected fact justified another mechanism, a weaker guarantee, a follow-up, or abandoning the approach.

What This Is Evidence For

The last post on this site about review measured closure when it cared about coverage; this one measured consumption when it cared about guarantees, the same instrumentation error one layer up.

When AI writes and AI reviews, keeping a human in the loop is not enough. The human was in this loop all day, answered every prompt, and ordered a merge. What the system did, five times, was ask for that authorization without supplying what a scope decision needs: the guarantees added, where each came from, what removing one would cost, and the option to remove it. The human needs decision rights at the points where a guarantee is added, and instrumentation that states exactly that, in product units. That is a testable claim: the instruments are being built now, and if they are present and the same pathology recurs, this post is wrong. I would rather have written something that can be wrong.

Trust reviewers to find defects. Do not ask them to decide which guarantees are worth defending. And do not ask a product manager to make that decision unless the system shows them that a guarantee is being added.