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

Two pull requests drew 72 Codex findings. I couldn't fault one; both closed unmerged. One grew from 35 lines to 2,136 for a requirement that later shipped in 377. I was asked five times how to proceed, never whether the machinery under review still belonged in the product.

Published
September 6, 2026
Updated
Author
Nathan Payne
Reading time
16 min read
Topics
AIProductDecision RightsCode ReviewSystems

Key takeaways

  • The question I was asked was rigged: there is a real bug, should we fix it? The question I needed was whether the machinery containing the bug still belonged in the product. Thirty-five lines became 2,136 for a requirement that shipped in 377.
  • I answered five prompts, asked for another round, and ordered a merge. The prompts reported rounds and findings. None showed the added guarantees, the diff's size, or an option to remove machinery. I had authorization to give and too little information to decide the scope.
  • Two questions settle it without reading code: does this mechanism exist because the issue requires it, and what does leaving the residual unfixed cost? The merged controls removed the machinery; the closed pull requests kept it, or removed it at the wrong layer.
  • I don't need to adjudicate a P1 about an ABA race to decide whether the mechanism containing it belongs in the product. I need named guarantees, where each came from, and the cost of leaving the defect unfixed. Turning that history into a score would repeat the measurement error.

When an automated reviewer flags a P1 correctness defect, I usually don’t have the engineering knowledge to prove it wrong. I trust the reviewer to find bugs. So “there is a real bug; should we fix it?” isn’t much of a decision. That question is rigged. Of course I say yes. The question I needed was whether the machinery containing the bug still belonged in the product I had asked for.

Over eleven days in late August and early September, two pull requests in Mergepath, the repository that sets the review policy for my coding agents, kept asking that question. Between them they drew 72 findings from the Codex GitHub App across 31 review rounds. I’ve read every one. I couldn’t fault a single finding. Whether each defect needed fixing before merge was a separate product decision, and it was mine to make. Both pull requests closed without merging. One was replaced by a change to a single word.

I started writing a post about automated review causing scope creep. Then I checked three pull requests that had just as many rounds and merged fine, and that explanation fell over. 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.

Two pull requests

The first, #1112, implemented #1056. When Mergepath bootstraps a consumer repository, it should record the template revision it used, so a later drift measurement has a baseline. The issue proposed the implementation in one line: put the commit hash in the initial commit’s subject and a trailer. The first commit did that in 32 lines.

The second, #1189, implemented #1188. The merge workflow published a diagnostic check-run when it hit an infrastructure error, but only as a failure. Nothing published a success under the same name. One transient error left a pull request’s head red permanently, and the local guard then demanded a break-glass merge even with every required check green.

#1112#1189
First commit+35 / −5+275 / −7
At close+2,136 / −17+1,179 / −19
Commits3921
Codex review rounds1912
Codex findings, of which P146, 1726, 3
Active review time21 hours, then idle 9 days22 hours
Replacement#1197, +377, merged in 68 minutes#1196, +54, merged in 20 minutes

What happened to the provenance change, #1112

Round 4 was the pivot. The first three rounds found genuine defects: the hash could be wrong or unresolvable in three different ways. All were fixed, and the recut kept two of those checks. Then Codex pointed out that a dirty source checkout could supply changed bytes while the mirror recorded its clean commit hash. It asked the author to require the source to be clean before accepting its hash. The implementation accepted it, and that is where it took on a guarantee the issue hadn’t asked for: “record which revision this was based on” became “prove the recorded hash characterizes the bytes that were mirrored.”

Round 8 was where it broke, through a choice the authoring agent made. The resume path reruns a partially completed bootstrap. Codex found that it 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. One line would have kept the promise honest: don’t attribute on resume. Instead, the session added --delete to the rsync invocation and built a residue-reconciliation engine around it. A provenance feature now deleted things.

The reviewers kept finding real defects in that engine. In round 12, a target directory named after an excluded path caused the engine to delete the entire target, repository and operator work included. After a round-15 trailing-slash fix, CodeRabbit noticed that / 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 was real, every one concerned code added after the pull request opened, and every one was 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, trailer, origin and reachability checks, and a plain clean-tree gate. It dropped the proof, configuration pinning and engine.

The rigged question

The session’s transcript shows what I was asked to decide.

After the task list at 01:36 UTC on August 27, rounds 1 through 10 ran without input from me. From the afternoon on, the session asked how to proceed five times. Each prompt had three options. These are all five, with my choice listed first.

Figure 1

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 my only free-form instruction of the afternoon: “Then do one more @codex round.” At 22:28 I pasted a link to another finding. A minute later I typed “fix that and admin merge.” The merge never happened. The pull request sat untouched for nine days.

Every prompt told me the loop wasn’t converging. The 17:48 prompt said the pull request had introduced two regressions of its own, and the 20:16 prompt called the next finding “the same class as several already-fixed spots.” But none connected the trouble to the issue. None quoted a line count; the session’s own text first gave the diff’s size on September 6, after I’d read it myself. None listed the guarantees added beyond the single one #1056 asked for, or explained that the current P1 was in an engine added in round 8 to answer a round-8 finding. None offered “remove the mechanism,” “weaken the guarantee,” “return to the original requirement,” or “close and recut.” Four of the five menus were neutral. I still clicked the same slot, because every option accepted the same design.

Seventeen of the session’s review replies quoted a round or finding count. Its three deferrals for non-convergence correctly described the findings as “new, distinct, genuinely valid edge cases.” They never said that one decision in round 8 had created the class, or that the decision could be reversed.

I trust the reviewer, and I won’t knowingly ship a P1. Given “fix it,” “ship with the defect,” or “go read the code yourself,” I’m going to pick “fix it.” “Remove the thing creating the P1” would have been a decision I could answer in a sentence. Nineteen rounds and forty-six findings describe what the review consumed. Thirty-five lines becoming 2,136 for a requirement that shipped in 377 describes what needed a decision.

Everything else I typed about #1112 came on September 6. First I asked the session to resolve merge conflicts. Then I read the diff, decided to close it, gave the contract for the recut, and added two notes. My decision was that an issue labeled small shouldn’t need changes capable of deleting repositories and operator work. Once it did, the correctness work already invested in the branch wasn’t a reason to keep it.

The session closed #1112 and opened #1197 against my three-check contract. Two Codex rounds and a CodeRabbit pass still drew three findings. Two were real violations of the clean-tree check and were fixed. One concerned an adversarial caller’s environment and was rebutted as outside the contract. Freezing the contract gave the agent a reason to say no to that finding. It didn’t stop reviewers finding bugs.

What happened to the diagnostic-clearing change, #1189

#1189 had a different problem. I described it as uncontrolled growth at first. The unnecessary guarantee was already in its opening commit.

An agent filed #1188 at 03:59 UTC on September 5, listing three possible fixes. The third was to publish the diagnostic as neutral instead of failure: visible, but non-blocking. The issue called it “the cheapest option and closest to what the record actually means.” Sixty-four minutes later, the pull request opened with option one: publish a success to clear the failure. The first commit’s header explicitly rejected the cheaper fix: “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 approve that sentence. The agent had made a product decision in a code comment, before any reviewer saw it. The diagnostic wasn’t a required status check on the hub or any of the three consumer repositories checked. It blocked only the guard script. Twelve rounds then hardened a requirement that didn’t need to exist.

The first round found two real false greens: the clearing path created a competing success run instead of updating the failure, and a clear wasn’t pinned to the head that produced the verdict. Ordering defects followed. The session deferred two to follow-up issues, explaining the cost. The external reviewer, a Codex pass under a separate identity with a merge veto, overruled both: “Both affect the core merge-gating guarantee and should be resolved before merge.” The session complied. Its note on complying said: “The review overruled my deferral and it was right to.”

Half right. The defects were real. Requiring their fixes before merge was a product judgment. The demanded watermark reopened the race over which conclusion buried which, in the opposite direction, and was split back out 95 minutes later. In all, five mechanisms tried to establish which of two workflow invocations happened first, over an API with no atomic operation for it. Each closed one interleaving and opened another. Findings per round never reached zero and spiked to six in round 6. Roughly half the later findings came from interactions between rules added the round before.

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

Seven minutes after that round, #1196 opened. One word: failure became neutral. With no red state to leave behind, there was nothing to clear. It merged in twenty minutes.

Explanations that didn’t hold up

Two failures could support almost any explanation, so I checked the merged pull requests too. Of the 507 pull requests closed by 2026-09-06, 15 closed unmerged. Twenty-six had eight or more Codex rounds; only these two didn’t merge. The comparison uses the 21 of those opened between July 4 and September 6, of which 19 merged. Four explanations looked convincing against the failures and fell apart against a control. None separates the two closed pull requests from the merged ones, which undermines each as a simple diagnostic. It doesn’t establish what caused either outcome.

Too many rounds. #1084 ran 19 rounds, drew 66 findings, and merged. #925 ran 18 and merged at 3,369 lines. Long reviews can merge. That alone doesn’t make them healthy or worth the cost.

Findings in code added to satisfy earlier findings. #1139 was an 85-line routing change with a bootstrap guard the author volunteered on top. Fifteen of its 21 findings concerned the guard. Seven were successive holes in one flag extractor: “the fifth instance of one root cause,” the author wrote, then “the sixth way this extractor has validated a subset.” It merged. Its share of such findings, a third, was higher than #1189’s.

A frozen contract quiets review. Both frozen contracts drew valid findings: the two P1s inside #1189’s five guarantees and the two clean-tree violations inside #1197’s three checks. The contract gave the agent a basis for accepting or rejecting a finding. It didn’t make review go quiet.

Implementation growth. #1112 grew 61× from open to close. The largest growth among the 19 merged pull requests with many review rounds was 11×. That separates #1112 alone. #1189 grew an unremarkable 4.3×. Its opening 275 lines already implemented a guarantee whose cheapest listed alternative was one word. Measuring growth from open couldn’t see the gap between the issue and that first commit.

What actually separated them

The comparison left two differences: which stronger guarantees entered scope, and whether the author removed the machinery causing the findings.

PRCodex RoundsFindings, Both ReviewersStronger Guarantees Accepted into ScopeA Mechanism Deleted Mid-ReviewOutcome
#117611301yes, after round 7merged
#113911211yes, after round 10merged at +85, from a peak of +286
#108419663, all input edge casespartly, after round 13merged
#11121951about 10neverclosed
#118912275, all ordering mechanisms, 3 later removedtwice, at the wrong layerclosed

I classified the stronger guarantees by hand; that column is approximate and single-rater. Each merged control accepted one to three stronger guarantees and treated the rest as defects within a contract already broad at open. Each also removed machinery during review, without a human asking. #1176, after round 7: “removing the thing that produced this finding rather than patching it a fourth time.” #1139, after round 10: “I am splitting the guard out rather than taking a fourteenth round on it.” That reply counted every reviewer’s pass, not just Codex’s. #1084, after round 13: “Both fixed, by deleting the mechanism that caused them.” Its hand-rolled field reader stayed and drew findings until merge, which is why the table says partly.

The provenance change never made that move. The diagnostic-clearing change made it twice, but removed machinery above the clearing path while keeping the path itself. The agent questioned its ordering tokens and kept the unnecessary requirement it had started with. My read is that an agent has the hardest time questioning the premise it opened with. That’s the one place in this story where I wasn’t optional.

The same issue was implemented twice, ten days apart.

Figure 2

The reviewers stayed the same. I changed the contract and held the agent to it. The recut also benefited from ten days of hindsight, so the comparison shows what the two bought together, not the contract alone. This is how #1112’s guarantees accumulated:

Figure 3

The question I can actually own

The merged controls showed that long, repetitive reviews can work when somebody asks whether the thing containing the error should still exist.

I can answer that without reading the implementation. Start by accepting the reviewer’s diagnosis: yes, that’s a real bug. Then ask:

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

What happens if I leave the residual unfixed?

That second question leaves room to accept the residual, as well as fix or defer it. #1084 merged while its hand-rolled parser still drew findings because a wrong answer could skip a review wait, not the review itself. #1196 shipped on an assumption about GitHub’s handling of neutral that couldn’t be verified in advance. If it was wrong, the failure would be today’s behavior; the remedy was reverting one word. Neither decision required reading code. Both needed the cost of being wrong stated in terms of the product.

My rule now is to fix a real error unless it concerns machinery added beyond the original requirement. Then stop and reconsider that machinery first. I’ve deliberately kept it a rule I can’t automate. An issue rarely names every property its outcome needs; “beyond” means an additional commitment, not merely something the issue didn’t mention. Omitting attribution on resume kept #1056’s promise honest without a deletion engine. I have to compare the guarantee with the issue’s problem, not the pull request’s design. Otherwise #1189’s clearing path counts as original and the rule catches nothing.

To use that rule, I need the agent to keep track of the original ask, the guarantees added beyond it, the mechanism each guarantee required, the finding that prompted it, and whether the latest finding concerns required behavior or added machinery. That’s enough history to decide without reading the code. #1112’s “new, distinct, genuinely valid” replies lost it. The merged controls recovered it by naming repeated instances of one root cause.

Those guarantees need names, not a score. A number invites a threshold that can be met without anyone thinking. #1112 ran past an owner-set ten-round budget that applied throughout. At round 12, this is the information I could have used:

Figure 4

The session had the facts in that figure and didn’t report them. The cost of removing the engine is my reconstruction of what an escalation should explain. I don’t need to understand rsync --delete to notice something is off. A system that recommends “fix” because the finding is valid has answered the engineering question and skipped the product one.

Since the review

The operating rules that followed put decision rights at the points they govern, not in a second rulebook beside the review policy. The owner ratifies the contract against the issue when a pull request opens, with named guarantees and the cost of residual defects visible. That’s where #1189 should have stopped. At escalation, the agent should report where each guarantee came from and what’s been added since open, and offer reduce, remove, recut and accept. Reviewers should keep finding defects.

Three claims corrected in this work were mine, or repeated by me. The size-S label I’d treated as the original estimate was added by a backlog audit eight days after the pull request opened. The first draft of the rules said #1189 ran ten rounds and seventeen commits; the API says twelve and twenty-one, and Codex caught the error in its first round. The first published version of this post counted the provenance change’s later rounds as the session prompts did, one higher than the sidebar’s rule, until a reader’s review prompted the correction. All three were unmeasured claims in material arguing for measurement before accepting an obligation. Each was caught by checking. Correcting a count still didn’t decide whether to add machinery, weaken a guarantee, defer a finding, or abandon the approach.

What I need from the loop

When AI writes and AI reviews, keeping a human in the loop is not enough. The last post measured closure when it cared about coverage. Here I was shown review consumption when I needed to decide which guarantees to keep.

I was in the loop all day. I answered every prompt and ordered a merge. Five times, the system asked for authorization without telling me which guarantees had been added, where they came from, or what removing one would cost. It didn’t offer removal at all. I need that information when a guarantee enters scope, with the cost stated in product terms. The instruments are tracked as open issues. If they’re built and the same problem 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.