The Bug That Would Fail Every First Win-Back Conversion

Six PM, six bugs, and one that would have passed every test I ran

At 1:20 PM I shipped the win-back flow to our review branch: a mailer link that lets someone whose trial on our volleyball-league platform expired sign back in and convert the site to a paid account in the same motion. By 8:44 PM I’d found and fixed six more bugs in it, on top of the eight I’d already caught earlier that afternoon. One of the six would have silently failed on the very first real conversion anyone made, and every test I’d run up to that point would have passed clean.

The flow has four cases. Sign in from the tagged mailer link and the trial restarts silently, no dialog, no choice to make. Sign out and back in and the system flips you to the right site by matching an internal ID (mtID) instead of asking. Sign in with the wrong account and you get a “Different Account” page instead of a confusing merge. And if you land from the schedule notice instead of the mailer, a dialog tied to that notice offers the same conversion with an explicit choice attached to it. Four entry points, one underlying action: mark the site converted, then restart the expired trial clock.

That second part, mark-then-restart, is where the bug lived, and it’s the kind that never shows up in a straight-line test.

Where the order bug actually was

restartTrial() had a guard in front of it to stop a page refresh from re-firing the restart and quietly extending someone’s trial twice. The guard checked for an existing conversion record before doing the restart, on the theory that if a conversion record already exists, this must be a retry, so skip the restart and just show success again.

The problem was sequencing. The actual call order in the handler was markConverted(), then the guard’s existence check, then restartTrial(). On a first-time conversion, markConverted() had already written the row by the time the guard ran a few lines later. So the guard looked at the database, saw a conversion record, and concluded this must be a repeat, even though it was the first and only time this account had ever converted. It skipped the restart. The user landed on the confirmation screen, the site converted, everything read as success, and the trial underneath it stayed exactly as expired as it had been five seconds earlier.

Nothing in the response, the UI, or the logs distinguished this from a correct run. I only caught it because I was reading the site’s trial-expiration timestamp directly against the database after the request, not trusting the 200 the endpoint returned.

The fix I tried first, and what it cost

My first instinct was to just swap the order: run the guard check before markConverted() instead of after. Simple, one-line diff, obviously correct.

I tested it by doing what an actual user does when a page feels slow: I hit refresh on the confirmation screen. The guard now ran early, before any conversion record existed, so it correctly let the restart through on the first load. But on the refresh, the guard ran early again, saw no conversion record yet (the first request’s write hadn’t landed, or the page reloaded mid-request), and let restartTrial() fire a second time. The trial’s expiration got pushed forward twice from one user action. Reordering the two calls hadn’t fixed the race, it had just moved it to the other side of the function and made it worse, because now the failure mode was a silent double-grant instead of a silent no-op.

I reverted the reorder. The actual fix was to stop using “does a conversion record exist” as the retry signal at all, since that record’s existence depends entirely on where in the sequence you happen to check it. I added an explicit idempotency key, a conversion ID generated once per attempt and passed through both calls, and made restartTrial() check whether a restart had already happened for that specific ID rather than inferring it from a side effect of a different function. Order stopped mattering because the guard no longer depended on it.

What the other five were

The rest were smaller and mostly UI-adjacent: a focus outline missing on the players page that had nothing to do with the win-back logic but sat in the same diff, a mismatched-login case that briefly showed the wrong account’s site name before the Different Account page rendered, and a couple of places where the mtID site-flip picked the right site but left the previous one’s session data hanging around in a way that would have shown stale data for one page load. None of them were dangerous on their own. What made them worth logging individually instead of folding into one commit message was that each one was a different kind of wrong: a race, a stale read, a missing style, a leftover session key. A flow that touches sign-in, site identity, and billing state all at once doesn’t fail in one place. It fails in whichever seam you didn’t specifically go looking for.

The flow is verified now across all four entry cases on our review branch. It’s not on production yet. Shipping it to master and turning on the mailer audience both still need an explicit go-ahead, separate decisions, made by a person looking at real numbers, not by a green test run. The order bug is exactly why: the version that looked done by every test I had would have converted the site and lied about the trial to every single person who tried it first.

dxdev LESSON · paste into your AI coding agent
LESSON: Use Explicit Idempotency Keys Instead Of Inferring Retries From Side Effects
SOURCE: dxdev.com/blog/2026-09-17_win-back-flow-golden-path-bug-prevented
WHAT HAPPENED: The handler marked a site converted before checking whether a conversion record existed, then restarted the expired trial. Because the conversion record had already been written, the guard treated every first conversion as a retry and skipped the restart. Moving the guard before the write allowed the first restart, but a refresh before the initial write landed could trigger a second restart and extend the trial twice. The fix generated one conversion ID per attempt, passed it to both operations, and made the restart check whether that specific ID had already restarted a trial. The flow was then verified across all four entry cases on the review branch.
THE RULE: Do not infer whether an operation is a retry from a side effect whose visibility depends on call order or timing. Use an explicit idempotency key that identifies the individual attempt and guard the operation against that key.
CHECK MY CODE, then report PASS or FAIL with file:line for each:
      1. Each retryable state-changing request carries an attempt-specific idempotency key generated once and propagated through its dependent operations.
      2. The duplicate-operation guard checks whether the same idempotency key has already completed the protected action, rather than whether a related record exists.
      3. Tests cover a refresh or concurrent repeat that occurs before the first request's writes are visible, and confirm the protected action happens exactly once.
THEN PRINT: a table (check, PASS/FAIL, evidence, fix) + a verdict (applies / partially / OUT_OF_SCOPE / no) + the single most important next action.