I filed a bug report, got corrected by a stranger, and my "fix" would have made things worse

A developer who maintains open-source libraries for exactly-once execution found a bug in a payments MCP server and proposed a fix, but a stranger pointed out that the fix would have caused silent data loss. The developer rewrote the pull request with a wider idempotency key and built a scanner called fencescan to help identify similar issues.

I maintain a couple of open-source libraries for exactly-once execution — the problem where an agent retries a tool call after a timeout, doesn't know whether the first attempt landed, and ends up doing it twice. A charge, a message, a payment. Nothing crashes. It just happens twice. Part of the work is reading other people's MCP servers looking for this exact bug, and yesterday I found one: a payments tool that signs and sends a transaction on every call, with no idempotency key anywhere in its schema. I opened an issue. I proposed a fix: derive a default key from the URL, the amount, and the wallet, so a retried request collapses into one settlement. A stranger replied and told me my fix was wrong. Not wrong in a nitpick way. Wrong in a way that would have been worse than the bug I was reporting. My key was too narrow — it only looked at three fields. Two offers for the same resource, at the same price, from the same wallet, but with a different payee, network, or asset , are different payments. My derivation would have collapsed them into one key. Which means: a legitimate second purchase would have silently returned the first purchase's receipt. Sit with that for a second, because it's the part that made me actually stop and think. A double-charge is bad, but it's visible . Someone sees two line items and files a dispute. A swallowed purchase looks like a success . The buyer thinks they paid and got the thing. Nobody investigates a transaction that appears to have worked. I'd built a bug that hides better than the one I was fixing. The actual mistake wasn't the three fields I chose. It's that I chose three fields from a template instead of asking the only question that matters: what makes two operations the same operation, and what makes them different? I pattern-matched to "amount + wallet + url" because that's what similar fixes usually look at. I never sat down and enumerated the full shape of "a payment" for this specific system. That's the actual lesson, and it's more general than payments: an idempotency key is a claim about identity. If you get the identity wrong in one direction, you get false duplicates annoying, safe, loud . Get it wrong in the other direction, and you get silent data loss dangerous, quiet, expensive . Most advice — mine included, until last week — only warns about the first kind. I could tell you I gracefully updated my proposal and moved on. What I did was rewrite the PR with the wider key adding payTo , network , and asset , credit the person who caught it by name, and say plainly in the pull request that my original suggestion was wrong. Not because it's noble — because the alternative is worse. Silently fixing it and hoping nobody checks the diff against the original issue is how you end up with a reputation for being defensively wrong instead of gracefully wrong, and defensively wrong is the expensive kind. The PR is open now: https://github.com/CryptoAPIs-io/cryptoapis-mcp-x402-pay/pull/2 https://github.com/CryptoAPIs-io/cryptoapis-mcp-x402-pay/pull/2 . Whether it gets merged is a separate question from whether the correction was right. It was. I built a small scanner — fencescan — that looks for tool calls in a codebase that could fire the same effect twice, and reports candidates with evidence , never a verdict. No install: npx fencescan I built it partly because of this experience. An outsider reading a repo usually can't prove a double-fire — the guard often lives in a service the repo calls, and a function that looks like a write might only build a payload for someone else to sign. The scanner's job is to point at the ten places worth reading closely, not to accuse anyone of anything. That restraint isn't modesty; an earlier, cruder version of this scanner was wrong on 4 of its first 7 real targets, for reasons that are genuinely interesting if you like that kind of postmortem mostly: a regex with no word boundary couldn't match camelCase, so it accused a repo of having zero idempotency guards when it shipped an entire module of them . If it flags something in your code and you think it's wrong, that's worth an issue. A false positive here costs more than a miss — I'd rather know. Update, the day before publishing: the first maintainer merge landed — the Kibana MCP server https://github.com/TocharianOU/mcp-server-kibana took the retry-safety note, and the maintainer's reply did two things at once: corrected an endpoint mixup in my own docstring create-with-id is POST, not PUT — my slip, their catch , and then extended my finding one line further than I had — their 409 error message was advising callers to do the exact thing the new docs warn against. They asked if I wanted to fix that too. I did, same day. Which means this article's thesis held all the way through its own publication: the corrections flowed both directions, and both parties' code got safer. That's the whole point. Repos: effectfence Rust , once-kernel TypeScript, with a Python twin , fencescan. All MIT/Apache, no strings.