Five rounds of adversarial review on a payment path, and every bug lived in the previous fix
I don't hand-write the code. I direct an AI to build it, and lately I've been learning what that does and doesn't protect you from on the part of a product that moves money.
We had a refund path with a green test suite and a working demo. Then we ran it through repeated adversarial review: each round a fresh reviewer whose only job was to break what the last round fixed. Five rounds. Every round found a real defect, and every defect was hiding inside the previous round's fix.
The pattern never changed. The fix was correct for the exact case we tested and wrong for the case one step to the side. A refund that fails at the bank. The same webhook delivered twice. A retry that succeeds after a failure. None of them showed up in a unit test, because a unit test asserts the case you already thought of.
What actually caught them, in order: a second reviewer who assumed the fix was wrong and went looking; reading the database state back under concurrency instead of trusting what the function returned; testing the money path with two connections racing, not one call at a time.
What made it converge wasn't smarter code. It was refusing to treat "the tests pass" as evidence that the money is right. On a payment path, correct-in-the-common-case is the failure mode, because the common case is the one everyone tests and the one that never breaks.
If you're shipping anything that touches money and you didn't write the code by hand, one review pass is not enough. Curious what others here have used to verify a money path from outside the code itself.
Floor first: I have never had to prove a race fix on a money path. The only nondeterministic result I have personally had to defend was on a small A/B rig, and it went the wrong way - a 2-for-2 failure that a pre-registered rerun turned into 8-for-8 clean. So read this as what I do now, not as something battle-tested.
Short answer: I don't trust a red-then-green result on a timing case. I try to stop it being a timing case.
First, take the schedule out of luck. If the test can drive both connections by hand - open two transactions, let A take the per-invoice lock, then let B hit the same invoice and assert what B does while A still holds it - the interleaving is chosen, not sampled. That turns the red against the old commit into a deterministic red. Same for the doubled webhook: deliver the same payload twice in the exact sequence you are afraid of, not just twice.
Where the schedule cannot be controlled, I stop reading single runs and count. Write the run count and the pass line down before running - something like "against the old commit this has to go red at least 20 of 50, against the new one 50 of 50" - and treat the numbers as the result. A red that shows up 1 in 50 against the old code is telling you the test barely reaches the bug, which is worth knowing before it turns green.
And the thing I would actually rest on is the invariant check you already named, not the red-green itself. After every run, whatever the interleaving was, assert the end state: exactly one applied revision per invoice, no dropped redelivery. Red-then-green says the test saw the bug once. The invariant holding across every run is what says the bug is gone.
What I can't tell you is what the right counts are for your path. That one is yours.
Yeah, that lines up with where we landed. The part that stuck with me is writing the pass line down before the run instead of eyeballing a red-then-green once. Turns "it failed, then it didn't" into something you can actually stand behind. Same with leaning on the end state rather than the transition. Appreciate you spelling it out.
The core insight here is that "tests pass" was measuring the wrong thing. Unit tests measure the happy path - one invoice, one connection, one attempt, no concurrency, no timing. On a payment system, the happy path is your false confidence signal.
The real measurement you need is: does the money actually move under the conditions that only happen in production - concurrent writes, retries, double-deliveries, network partitions, unprovisioned accounts. Each round found a bug that lived inside the previous "fix" because the fix addressed the test failure but not the money failure. You weren't measuring money correctness until round 2.
That's why adversarial review works here - it's a human forcing the measurement signal to include the cases your automated tests ignore.
Right, the happy path is where the false confidence lives. Automated tests keep asserting the case you already thought of, and on a money path the ones that bite are the ones you didn't. That's the whole argument for a human going at it trying to break it. Thanks for weighing in.
I have not shipped a payment path, so discount this accordingly - my version of the same problem moves files between two machines, not money.
The line I would push on is "every defect was hiding inside the previous round's fix." I hit that shape last week. My worker's auto-commit used a pathspec, which bypasses the index, so it quietly swept unstaged and unrelated edits into job commits. Every test I had stayed green the whole time, because they were all asserting the case I already thought of.
What changed afterwards was not the fix, it was a rule about what a fix is allowed to ship with: the test has to FAIL against the old code. I wrote four checks and ran them against the pre-fix version. Three failed, and those three are the only ones I trust. The fourth passed against the broken code, so whatever it is measuring, it is not that bug.
If the fixes from those rounds went in as separate commits, this costs you almost nothing, because the expensive part is already paid for: run each round's test against the commit immediately before its own fix. Anything that passes there is not the thing standing between you and that defect, and you can decide today whether it gets rewritten or dropped. One caveat from your own list - the racing connections and the twice-delivered webhook are timing-dependent, so a single green run against the old commit settles nothing there; those have to be repeated before either result means anything.
It also gives aryan_sinh's question an answer from the repo rather than from memory: revert one fix at a time and see which of the other tests light up. If dropping round two's fix breaks round four's test, those rounds were poking the same workflow.
The test-must-fail-against-the-old-commit rule is the part we didn't have a name for, and it's right. Our rounds went in as separate commits, so running each round's test against the commit just before its own fix is nearly free, and we're going to.
The timing cases are where you're most right. The racing connections and the twice-delivered webhook sit behind a per-invoice advisory lock plus idempotency rows that keep a revision instead of dropping a changed redelivery, so one green run against the old commit proves almost nothing there. Those get repeated runs and a check on the lock invariant, not a checkmark.
And reverting one fix at a time is how we found which round each defect actually belonged to.
For a timing-dependent case, what makes you trust a red-then-green result instead of a lucky interleaving?
The repeated-fix pattern is the interesting part.
Did each fresh review catch a different class of failure, or mostly edge cases around the same workflow?
Different classes, but clustered on one workflow. The refund path was the common ground, and the failures on it were not the same kind twice. One round was a refund event that landed inert. The next was two connections racing on the same invoice. Another was a lifecycle-ordering bug where a later status quietly outranked the real one. The last was an access check that failed open for an unprovisioned account, which is a different animal from the money bugs. What made it feel like edge cases is that each one only became reachable after the previous fix moved the surface, which is tjgarage's point about testing against the pre-fix commit.