Skip to content
Writing/Detail

Seven review rounds on a payments path, and not one broken feature

25/07/2026 (edited)
Tech
876 Words
4Min read

I built a payments and refunds path with an agent doing most of the typing. It took seven rounds of review.

Not one round was a broken feature. Everything ran. Tests passed. The code did what it said.

Every finding was the code being wrong about the world rather than wrong about itself. Tests catch the second kind. Only a review catches the first, and that distinction is now the thing I plan review time around.

The three money bugs

One ticket regardless of quantity. Someone buys three seats. The code issues one. Internally consistent, correct against its own types, and would have produced a queue of furious people at a door.

Any refund voids every ticket. Refund one seat from a group booking and everybody in that group loses entry. Each step is reasonable in isolation: refund means void, so void the tickets. Nothing in the code knows that a booking can be partly refunded, because nothing in the code has ever stood at a door.

A timestamp inside an idempotency key. An idempotency key exists so a retry is recognised as the same request. Put the current time in it and every retry is a new key, which is a duplicate charge with extra steps.

That one is worth following, because the fix was also wrong. The replacement key dropped the timestamp but had no buyer component — so two different people buying the same ticket type would have generated the same key and shared one single-use checkout session. One of them pays, the other gets an error, and the system considers it a success.

Two wrong answers in a row on one small string, both plausible, both caught by reading rather than running.

The test that passed against two wrong implementations

A daylight-saving test had been written into the plan using a timestamp of 2026-10-25T00:30:00Z.

Correct timezone handling returns the 25th. Naive GMT handling returns the 25th. Naive local-time handling returns the 25th. The test passes against the right implementation and both common wrong ones.

Replaced with 2026-10-24T23:30:00Z, which lands a full day apart depending on whether the conversion is correct, with a comment explaining why the awkward value is the point and asking the next person not to tidy it.

A test around a boundary has to use a value that is actually near the boundary. A date during a DST changeover is not automatically a DST test.

Two things worth refusing to build

A payouts panel fed from the internal ledger. The design called for “available balance”, “next payout” and “payout history”, filled from our own records of what had been charged.

Those are payment-processor facts, and they differ from an internal ledger by processing fees, the pending-versus-available split, payout timing and dispute holds. Every number would have been wrong, under a label promising it was right.

An empty panel saying “not connected yet” is honest. A populated panel with plausible wrong numbers is worse than no panel, because somebody will reconcile against it.

Two columns removed from a migration. A summary read had summed a column that was never written to, and returned a confident zero under a label promising “net”. The column was not yet in production, so rather than leave it permanently null and hope nobody read it again, it came out of the migration.

A field that is always zero is a lie with a schema.

The plan itself had gone stale

Before any of that, the plan was two weeks old. Migration numbering had moved on several versions. One task targeted a route that had since been redirected away.

Executing it faithfully would have built a second, wrong home for the payments UI and left the honest placeholder pages empty forever — passing every acceptance criterion in the document.

Reconcile the plan against the repository before executing it. An agent will follow a stale plan perfectly, and perfect execution of the wrong plan is harder to spot than sloppy execution of the right one.

The pattern

Three more findings had the same shape. An absent field meaning opposite things at two levels of one payload, so a partial update silently deleted data while reporting that it had removed nothing. A read-then-write race the repository’s own conventions forbid in writing. A loop reimplementing a check that already existed a few lines away — and partial reuse is worse than none, because it looks shared, so nobody re-reads it.

None of these are failures of the model. They are failures of context. The code was correct with respect to everything it could see, and wrong with respect to things nobody had written down: that bookings have groups, that refunds are partial, that a processor’s balance is not your ledger.

That is the job now. The agent does the typing. What is left is knowing what the code needs to be true about the world, and checking it against that — which is most of what senior engineering was anyway, minus the typing.

Back to writing
End of Post