Standards vs Spec review: a second agent found duplicate writes past 1,572 green tests

·5 mins

This is what the review agent reported, just before it listed its findings:

The test evidence is clean: Ruff passes, Python reports 1,572 passed / 56 skipped /
1 expected failure, and the GUI reports 418 passed / 4 skipped. The review nevertheless
found safety gaps the tests do not exercise — most importantly, two write clients retry
"500-after-commit" before their advertised idempotency/verification guard can run.

A coding agent had built the branch: 53 commits, 147 files. A different model reviewed it and found two paths that could put the same invoice in the ERP twice and report plain success. No test covered them, and no linter could. They came from a reviewer with one job: check the code against what the branch said it does.

The setup

The repository vendors the two-axis code-review skill from mattpocock/skills . It pins a fixed point and diffs against the merge base, git diff master...HEAD, then forks two subagents in parallel:

  • Standards: does the diff follow the repo’s documented rules (glossary, ADRs, a baseline of Fowler code smells)?
  • Spec: does the diff do what the originating issue or PRD asked for?

The parent agent runs lint and tests meanwhile, “as supporting evidence, not as a substitute for the diff review”. It then reports the two axes under separate headings and doesn’t rerank them, so a pile of naming nits can’t bury a P1.

The Spec brief is short:

Report: (a) requirements the spec asked for that are missing or partial;
(b) behavior in the diff that wasn't asked for (scope creep);
(c) requirements that look implemented but where the implementation looks wrong.
Quote the spec line for each finding, under 400 words.

What each axis found

There was no PRD for the branch, so the Spec reviewer used the commit messages and the ADRs. One commit claimed that the MCP server’s idempotency check made it safe to retry a write that got a 5xx. The reviewer traced the call: the check ran before _request(), the key was recorded after success, and the retry happened inside _request(), in between. A webhook worker had the same shape, with a verify-by-GET that ran only if the request finally raised. A retry that succeeds never raises. (The full HTTP 500 story is here .)

The Standards reviewer found something else. Two functions called urlopen() directly, past the shared transport that an ADR says owns all retries. It also found an import that went through a module the ADRs forbid as a re-export facade, and an unfinished vocabulary migration.

None of the Standards findings overlapped with the Spec ones. All six were fixed the same evening, in one commit.

Why the tests stayed green

The tests came from the same author as the claim. No test let a 500 commit the write, let the retry succeed, and counted the writes, because the commit said the idempotency check covered that case. A suite tests what its author thought could break.

The Spec reviewer starts from the claim instead. A commit message says the code is safe, so it goes looking for the line that makes it safe, and here that line ran on the wrong side of the retry.

Two more runs, same pattern

In the next review, a few hours later on a six-commit range, a guard was undone downstream. Every write had to carry exactly one confidence label, and create_* checked that. But when label setup had failed, a fallback further down stripped all labels from the payload. The reviewer reproduced a labeled invoice leaving without its label. It also found a write that went through a raw put() and skipped the scope check that limits which record types may be written at all.

The third review had nothing to go on but commit messages. The branch added a gate: if an invoice is addressed to another company, warn and refuse to post it. The reviewer said it was inferring the spec from the commits, so I pasted the original request and the Spec subagent re-checked against that. The request held a second requirement the commit summary had left out, which turned out to be implemented. The review also found that a replay of staged payloads called the push function directly and skipped the new gate. And while company registration numbers were checksum-validated, VAT IDs relied on regex alone, so one misread digit could block a legitimate invoice. Both failed the first test, then got fixed.

How to use it

  • Run Standards and Spec as separate subagents, so neither sees the other’s findings, and report them under separate headings.
  • Give the Spec reviewer the authoritative spec. If you only have commit messages, say so, and swap in the real request the moment you have it.
  • Treat claims in commit messages and ADRs (“X makes this safe”, “Y owns all retries”) as requirements. They are the cheapest spec you have, and the author’s tests already agree with them.
  • If you can, review with a different model than the one that wrote the code. It is less likely to share the writer’s blind spots.

Takeaway

A green suite tells you the code does what its author expected. A Spec review by a second agent tells you whether that matches what the author promised. The duplicate writes were in the gap between the two.

Take the last branch your agent wrote, and have a second agent check each “this makes X safe” in its commit messages against the code.