Skip to content

Money: a hold Stripe declined on our behalf is released (stacked on #954; no migration) - #962

Closed
NewWorldVenture wants to merge 1 commit into
claude/card-hold-settles-7q6vtffrom
claude/card-hold-timeout-release-7q6vtf
Closed

NewWorldVenture wants to merge 1 commit into
claude/card-hold-settles-7q6vtffrom
claude/card-hold-timeout-release-7q6vtf

Conversation

@NewWorldVenture

Copy link
Copy Markdown
Owner

Draft for review: do not merge without the owner.

What was wrong

A hold for a purchase Stripe declined on our behalf was never released.

  • If Stripe does not have our decision within 2 seconds, it approves or declines by the account's timeout setting. It reports that decision only in issuing_authorization.created, with request_history.reason = webhook_timeout (Stripe: real-time authorizations).
  • handleAuthorizationRequest reserves the hold first, then calls the deprecated authorizations.approve API. So when a slow run misses the window, the hold is already committed and the approve call fails.
  • The route ignored .created, and nothing else released the hold. The child's money stayed held for good.
  • Handling .created alone would not be enough: it can be processed before the late hold commits.

The setup docs and the code disagreed.

  • docs/AGENT_HANDOFF.md step 4 subscribes the money endpoint to .request, issuing_transaction.created and account.updated only. It leaves out issuing_authorization.updated, which releases a reversed or expired hold (and runs Money: card holds follow each authorization request and each capture, in any event order (0487, stacked on #925) #954's settle-before-close).
  • docs/runbooks/LB-006 listed .created, which the route answered as unhandled.
  • I cannot see the live Stripe endpoint configuration. If it was set up from step 4, holds are not being released on reversal or expiry today. Please check that the endpoint subscribes to every event listed below.

The change (app only)

  • app/api/webhooks/money/route.ts: handles issuing_authorization.created through the same path as .updated. Stripe sends .created for every authorization, and handling is idempotent, so both events may arrive.
  • lib/stripe/webhook.ts, handleAuthorizationUpdated:
    • It first releases, by its key, the hold of every request Stripe declined, whatever the authorization's status. A declined increase no longer waits for the close.
    • It then settles and closes as before.
  • When our approve call fails, the handler retrieves the authorization:
    • Over (closed, reversed or expired): it is settled and closed exactly as its own event would do it.
    • This request declined for good: that request's hold only is released. "This request" means the entry at its position in request_history, matched by the amount it asked for. An increase never frees the purchase it raised.
    • Approved (the timeout setting approve), undecided, not recognisably this request, or Stripe cannot be asked: the hold stays. Money is held, never freed.
    • The answer to Stripe is still a 500, and nothing in this path can replace that error.
  • network_fallback is not a decline we act on early. Stripe declined but the card network decided; when the network approved, Stripe's docs say it may still be captured.
    • Its hold stays while the authorization is open.
    • The close releases it like any other, so the wait is bounded.
  • lib/wallet/server.ts: releaseRequestHold releases one request's hold by its key and the amount it asked for, so Stripe's record and our hold must agree before money moves. It confirms its write (.select('id')), as the unconfirmed-write ratchet requires.
  • Docs: both now list exactly HANDLED_EVENT_TYPES, and a test pins them to the route:
    • issuing_authorization.request, .created and .updated;
    • issuing_transaction.created;
    • issuing_card.created and .updated;
    • account.updated.

Tests

tests/a-hold-stripe-declined-for-us-is-released.test.ts, 23 tests. Signed events go through the real route, with Stripe's API faked and wallet_reserve_card_auth emulated statement by statement from 0155.

  • Red before the fix (6):
    • a closed .created releasing the late hold;
    • an approve failure on a declined purchase, and on a declined increase, releasing that hold;
    • the route handling .created;
    • both docs listing every handled event.
  • .created:
    • both orders of .created and .updated;
    • the 0487 wallet_close_card_auth path.
  • .updated on a pending authorization:
    • a declined increase's hold is released, and the purchase's stays;
    • with mixed entries, a network_fallback decline waits for the close, and the close releases it;
    • a declined entry whose amount is not the hold's is not trusted.
  • Approve failure:
    • a closed authorization with a capture listed is settled and closed;
    • a reversed authorization is closed whole;
    • a release that fails never replaces the error (asserted on the thrown message);
    • Stripe approved, undecided, or unreachable: the hold stays;
    • only this request's entry counts, pinned with an equal-amount later entry;
    • an amount mismatch is not trusted;
    • network_fallback keeps the hold.
  • Decline failure: nothing is released, and Stripe is not asked.
  • Mutation: 12 app mutants, all killed:
    • .created not routed;
    • releasing the whole authorization;
    • reading the authorization's approved instead of the request's;
    • releasing whatever was decided;
    • asking Stripe after a failed decline;
    • the last entry instead of this request's;
    • no amount check;
    • network_fallback counted as a decline;
    • no per-request release on .updated;
    • no terminal path;
    • no amount filter on the release;
    • the inner catch rethrowing.

Independent review, two rounds:

  • Round 1:

    • a blocking ratchet failure: the release did not confirm its write;
    • network_fallback;
    • a terminal authorization on the approve-failure path;
    • the request's identity, now checked by its amount;
    • per-request release on .updated;
    • three weak tests.

    All addressed.

  • Round 2: no new way for capturable money to become spendable.

    • One trade-off fixed: round 1's fix kept a closed network_fallback authorization open for good, which would strand its holds. That also happens when the network declined too, the usual case without a dedicated BIN. Its hold now waits only for the close.
    • Also from round 2: the amount check on the per-request release, and four untested paths.

Local validation on 8fd4d4c2e: tsc is clean, lint has 0 errors (its 15 warnings are all in files this PR does not touch), the i18n gate is clean, and the full vitest suite passes on Node 24.21, the repo's pin: 1,898 files, 25,899 tests.

Not covered (residuals)

  • The decision is still made by an API call inside the 2-second window. Stripe's current pattern is to respond to the webhook directly with { approved }, which drops a network round trip from the window. That changes the handler's contract, and it is left for a decision.

  • The account's timeout setting. On approve, a purchase we would refuse can still be approved, and then captured with no hold. decline is the safe setting for this app.

  • Holds that can still strand:

    • Our approve call fails and Stripe cannot be asked: only a log line, nothing durable to reconcile from.
    • The function dies between the reserve commit and the approve call.
    • A replayed .request: 0155 treats only a processing hold as already reserved, so a replay after the hold was released reserves again (0342's replay note).

    A periodic reconcile of processing holds against Stripe would close all three.

  • A capture after the close of a network-approved (network_fallback) authorization is debited with no hold left, like any late capture (Money: card holds follow each authorization request and each capture, in any event order (0487, stacked on #925) #954's expired residual).

  • The billing webhook claims every event type in the shared stripe_webhook_events ledger. If it is ever subscribed to Issuing events, it can consume one first, and this endpoint then answers duplicate (pre-existing).

  • docs/AGENT_HANDOFF.md around line 3179 is a historical changelog that names the events the route handled at the time. It is left as written.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS


Generated by Claude Code

If Stripe does not have our decision within 2 seconds it approves or
declines by the account's timeout setting, and reports that only in
issuing_authorization.created (request_history.reason webhook_timeout).
handleAuthorizationRequest reserves the hold first and then calls the
deprecated approve API, so a slow run had committed the hold when the
approve call failed; the route ignored .created and nothing released it.
The child's money stayed held for good.

- The route handles issuing_authorization.created through the same path
  as .updated. That path now also releases, by its key, the hold of every
  request Stripe declined, whatever the authorization's status, so a
  declined increase no longer waits for the close.
- When our approve call fails, the handler asks Stripe what it decided:
  an authorization that is over is settled and closed as its own event
  would be; a request declined for good (matched by its position and the
  amount it asked for) releases that request's hold only. Approved,
  undecided or unknown: the hold stays.
- network_fallback (Stripe declined, the card network decided) is not
  acted on early: when the network approved, Stripe says it may still be
  captured, so its hold waits for the close, which releases it.
- releaseRequestHold releases one request's hold by key and amount, and
  confirms its write (.select), as the unconfirmed-write ratchet requires.
- docs/AGENT_HANDOFF.md step 4 subscribed the money endpoint to three
  events and left out issuing_authorization.updated, which releases a
  reversed or expired hold; LB-006 listed .created, which the route did
  not handle. Both now list exactly HANDLED_EVENT_TYPES, pinned by a test.

Found by reading; red tests first. Two independent review rounds are
addressed (ratchet, network_fallback without stranding holds, terminal
retrieve, request identity by amount, per-request release on .updated,
error never replaced, test gaps); 12 app mutants killed.

Claim: #771 5981789351. App-only: no migration.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS
@vercel

vercel Bot commented Oct 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
bubaly Ignored Ignored Oct 4, 2026 5:30pm UTC

Request Review

Copy link
Copy Markdown
Owner Author

Closing at the owner's direction to clear every open pull request. The branch is kept; nothing is merged or deleted. Please do not reopen or open a replacement without the owner's say-so.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants