fix(payments): a crash mid-settlement lost the order, silently and forever - #823
Merged
Merged
Conversation
…rever
handlePaymentConfirmed flips the intent to `paid` FIRST — deliberately, because
that conditional update is the lock that makes settlement run exactly once — and
only then writes the order, decrements inventory, notifies the seller, grants
entitlements and fans out webhooks.
A crash in that gap lost every one of them, permanently and in total silence:
* the row is paid, so every later observer's claimPaidTransition returns false
and skips the side-effects as "already settled";
* refreshPaymentStatus short-circuits on terminal statuses;
* the reconcile cron sweeps only CREATED / INVOICE_READY /
PENDING_CONFIRMATION, so it never looks at a paid row again.
The buyer's money is gone, their order sits in pending_payment forever, and
nothing anywhere reports a problem. #563 finding 4.
`side_effects_at` is the marker: stamped when the settlement path completes, on
both the tip branch and the main one. NULL on a paid intent means the
side-effects did not finish, and the reconcile sweep now reports those — riding
the existing cron tick rather than adding a second timer, and running even when
there is nothing to reconcile, since an incomplete settlement is a PAID row and
never appears among the sweep's own candidates.
WHY IT REPORTS AND DOES NOT REPLAY
Replaying looks like the obvious fix and is a worse bug. decrement_inventory is
a blind `inventory_count - 1` with no idempotency key — read from the live
database, not assumed — so re-running settlement for one sale decrements twice
and quietly destroys stock; plan grants and entitlements have the same shape.
Trading an invisible loss for a silent corruption is not progress. Safe replay
needs per-effect receipts, which is a separate and larger piece of work. This
converts a permanent silent loss into a named, actionable one, which is the part
that could not wait.
The marker also does not claim every async fan-out landed: several effects are
deliberately fire-and-forget, so it says the settlement path RAN TO COMPLETION.
Stated in the code rather than implied.
The column is deliberately NOT granted to anon/authenticated — payment_intents
uses column-level SELECT grants and this is an internal operations marker. A
partial index on (paid_at) WHERE status='paid' AND side_effects_at IS NULL keeps
the per-minute check off a seq scan, and is empty whenever things are healthy.
Mutation-proven: dropping the side_effects_at filter fails the predicate test,
and downgrading the report from error to warn fails the loudness test. A
detector that goes quiet is indistinguishable from one finding nothing, which is
this whole bug.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018waGt1ieA9TjpscqrbrnGb
…now makes two CI caught this, which is what it is for. Adding the incomplete-settlement check to runPaymentReconcileSweep broke four existing tests in payment-reconcile-cron.test.ts, and the failure was entirely mine. Its admin fixture had a single `eq` that always RESOLVED and recorded the id into `polled` — correct for the stamping path, `update().eq(id)`. The new check is `select().eq().is().lt().order().limit()`, where `eq` must keep CHAINING. So the second query blew up mid-chain AND pushed 'paid' into `polled`, breaking assertions that had nothing to do with it. The fixture now tracks which shape it is in: `update()` marks the resolving path, `is()` marks the check, and `limit()` returns no rows for the check so the existing assertions stay about reconciliation. The fixture was under-specified for the code it exercises; the production query is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018waGt1ieA9TjpscqrbrnGb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes finding 4 of #563 — the one with money behind it.
handlePaymentConfirmedflips the intent topaidfirst — deliberately, because that conditional update is the lock making settlement run exactly once — and only then writes the order, decrements inventory, notifies the seller, grants entitlements and fans out webhooks.A crash in that gap lost every one of them, permanently and in total silence:
paid, so every later observer'sclaimPaidTransitionreturns false and skips the side-effects as "already settled";refreshPaymentStatusshort-circuits on terminal statuses;CREATED/INVOICE_READY/PENDING_CONFIRMATION, so it never looks at a paid row again.The buyer's money is gone, their order sits in
pending_paymentforever, and nothing anywhere reports a problem.The marker
side_effects_at, stamped when the settlement path completes — on the tip branch and the main one. NULL on a paid intent means the side-effects didn't finish, and the reconcile sweep now reports those.It rides the existing cron tick rather than adding a second timer, and runs even when there's nothing to reconcile: an incomplete settlement is a
paidrow, so it never appears among the sweep's own candidates. Skipping the check on a quiet tick would hide exactly the case it exists for.Why it reports and does not replay
Replaying looks like the obvious fix and is a worse bug.
decrement_inventoryis a blindinventory_count - 1with no idempotency key — read from the live database, not assumed — so re-running settlement for one sale decrements twice and quietly destroys stock. Plan grants and entitlements have the same shape.Trading an invisible loss for a silent corruption is not progress. Safe replay needs per-effect receipts, a separate and larger piece of work. This converts a permanent silent loss into a named, actionable one, which is the part that couldn't wait.
The marker also doesn't claim every async fan-out landed — several effects are deliberately fire-and-forget, so it says the settlement path ran to completion. Stated in the code rather than implied.
Schema
Deliberately not granted to anon/authenticated:
payment_intentsuses column-level SELECT grants and this is an internal operations marker — same reasoning asnwc_connection_uri. A partial index on(paid_at) WHERE status='paid' AND side_effects_at IS NULLkeeps the per-minute check off a seq scan, and is empty whenever things are healthy.Verification
type-checkgreen,check:schema-columnsgreen, migration versions unique (61), 5 new tests.Mutation-proven: dropping the
side_effects_atfilter fails the predicate test; downgrading the report fromerrortowarnfails the loudness test. A detector that goes quiet is indistinguishable from one finding nothing — which is this whole bug.