The happy path passed hundreds of tests — the real critical bug was hiding in the "what to do when it fails" instructions
This is the English version of a post originally written in Korean for my algorithmic trading system devlog(new tab).
I rewrote the path where money actually moves as a saga. A capital event — a deposit or withdrawal reflecting into the account's book — gets split into several steps, and if something fails midway, a compensation step rolls it back.
I finished the implementation and ran a code review. The most critical defect turned out to be nowhere near the execution logic I'd sweated over — it was somewhere else entirely.
This post is about that "somewhere else." This path is part of the order-execution and safety layer, not the stock-picking logic, and I wrote up its full design separately in the live-account post(new tab).
What happened
For one capital event to land in the book, it goes through roughly these steps: declare the event, execute it, record it in the ledger, then re-anchor the day's reference net asset value.
The problem was when the recording step failed somewhere in the middle. You end up in a half-finished state — the event was declared, but no row made it into the ledger.
For exactly that case, I'd embedded "here's what to do" recovery guidance in several places. The code review flagged that guidance itself as the critical defect.
All four of those messages pushed the operator the same way: "run it again." But that re-run path was not idempotent. The same deposit could be applied twice.
What I thought at first
Honestly, I assumed the risk lived in the execution logic. I'd attached more than twenty new tests to the happy path, and several hundred related suites were passing.
So I asked the review to focus on rare execution-failure cases. I expected it to surface a race condition or a partial failure — some edge I'd missed.
The AI I handed the review to pointed somewhere else entirely. The most dangerous thing, it said, was the recovery text a human reads after a failure.
At first I felt a little defensive. That's a string, not code — right? But the more I sat with it, the more the point held.
Nobody reads recovery guidance in normal times. It only gets read once something has already gone wrong. And that moment is exactly when a stressed operator follows the instructions literally.
In other words, the recovery guidance wasn't "documentation" — it was effectively part of the execution path. And that path pointed at a non-idempotent retry, so the double-execution was a latent bug baked into the code.
The real cause
Thinking it through, the defect wasn't one thing — it was three layers.
First, the re-run path itself wasn't idempotent. There was no key to decide "has this capital event already been booked?", so a second run simply booked it again.
Second, the guidance lumped together two failure situations with opposite correct responses. An event created by an automated sweep should only be closed and never re-credited, while an event that genuinely failed to record needs to be completed. One message covered both — which means it was guaranteed to prescribe the wrong action for one of them.
Third, the ordering was wrong. The flow let you re-run before checking whether a ledger row already existed.
Stack those three, and the most natural human reaction — "I'm nervous, let me just run it one more time" — led straight to a double deposit.
How I fixed it
The core principle was one line: when it's ambiguous, don't guess — fail closed.
If a capital event is in an ambiguous partial-failure state (declared, no ledger row, not re-anchored), the round gate records nothing. It then blocks that round's trading sleeve entirely until the event is explicitly resolved.
A blocked round can be revived later; a double deposit is hard to undo. So when in doubt, I chose to stop.
Next, I put an idempotency key on the recording step. Each operation now carries a unique identifier, so re-running the same event makes the second attempt a no-op. Instead of hoping the human does the right thing, I made the retry safe by construction.
I also split the guidance by failure type. Sweep events now say "resolve only, never manually credit," and genuinely unrecorded events enforce an order: resolve → re-confirm no ledger row exists → only then re-run.
Finally, I added a command to inspect a specific capital event's current state directly. So the operator checks ground truth with their own eyes instead of trusting what the message says.
On re-review, critical findings dropped to zero. Deployment did come with one extra condition — restart every resident process that imports this path. If even one old-generation process lingered, that same double-application could come right back.
Generalizing
The lessons here apply far beyond trading.
Error messages and recovery runbooks are part of your safety surface. They're not documentation you bolt on later. They're read at the worst possible moment — after something has already failed — and followed literally. A "try again" sitting on a non-idempotent path is a latent double-execution bug.
Guard side-effecting paths with an idempotency key. Whether it's money or an external API call, take an operation identifier and make a retry a no-op. Don't make safety depend on "the human won't slip."
When state is ambiguous, don't guess — fail closed. Better to block and page a human than to be clever with compensation logic and get it wrong. A blocked job is recoverable; a side effect that already fired often isn't.
A pile of happy-path tests is not reassurance. This code passed hundreds of tests and still carried a critical defect, because the bug lived in the failure-and-recovery layer that tests rarely reach — specifically in the guidance text. Put the recovery paths, and their instructions, into the test scope.
Distinguish failures that look alike but need opposite responses. One generic message that fits both cases is, in fact, a bug for one of them.
What I'd invested the most in was the execution logic — but what actually protected the system was redesigning "what to do when it fails." From now on, once a feature is built, I plan to review its recovery guidance as part of the feature itself.
Top comments (0)