Public: anyone can read this; the crew can change it.

`checkout.session.completed` binds the paid checkout to its forge, customer and subscription #11289

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

A verified `checkout.session.completed` now binds the paid checkout to the forge named in its metadata. The change is pushed on the branch.

shipped production is serving it deploy a5a2512

opened this

A verified checkout.session.completed now binds the paid checkout to the forge named in its metadata. The change is pushed on the branch.

What changed

  • New src/billing/checkout_completed.ts (bindCheckout). It reads metadata.forge and the session's customer and subscription. If the forge exists, it writes its entitlement as active, provider stripe, source stripe, with both ids stored. The change is kept in entitlement_change and put on the event outbox as billing.plan.
  • src/http/stripe_hook.ts calls it only when the event id was newly stored, so a replay still answers 200 and does nothing.
  • An unknown forge, a malformed forge name or an unusable customer or subscription id changes nothing. The event is already recorded in stripe_event, so that row is the record. I added no migration.
  • Tests in test/stripe-hook.test.ts cover binding, an unknown forge (recorded, no entitlement written) and malformed sessions. The file passes (15 tests), bun x tsc --noEmit is clean, and bun scripts/gate.ts printed nothing.

Choices to look at

  • Nothing in the repo creates checkout sessions yet, so the key names are my choice. metadata.forge names the forge, and an optional metadata.plan picks the plan; without a valid plan the forge keeps its current one. Whatever creates the session later must set metadata.forge.
  • scripts/fast.ts named 1609 test files for this diff, so I ran only the stripe hook test file, not those.
  • I added an empty Try: commit because the pre-push hook wanted one.

To try it Seal a signing secret, then POST a signed checkout.session.completed to /hooks/stripe with data.object = {customer, subscription, metadata: {forge}}. The forge's entitlement turns active under source stripe, which you can see in the admin billing view.

Closes #11246

commented

A verified checkout.session.completed now binds the paid checkout to the forge named in its metadata. The change is pushed on the branch.

What changed

  • New src/billing/checkout_completed.ts (bindCheckout). It reads metadata.forge and the session's customer and subscription. If the forge exists, it writes its entitlement as active, provider stripe, source stripe, with both ids stored. The change is kept in entitlement_change and put on the event outbox as billing.plan.
  • src/http/stripe_hook.ts calls it only when the event id was newly stored, so a replay still answers 200 and does nothing.
  • An unknown forge, a malformed forge name or an unusable customer or subscription id changes nothing. The event is already recorded in stripe_event, so that row is the record. I added no migration.
  • Tests in test/stripe-hook.test.ts cover binding, an unknown forge (recorded, no entitlement written) and malformed sessions. The file passes (15 tests), bun x tsc --noEmit is clean, and bun scripts/gate.ts printed nothing.

Choices to look at

  • Nothing in the repo creates checkout sessions yet, so the key names are my choice. metadata.forge names the forge, and an optional metadata.plan picks the plan; without a valid plan the forge keeps its current one. Whatever creates the session later must set metadata.forge.
  • scripts/fast.ts named 1609 test files for this diff, so I ran only the stripe hook test file, not those.
  • I added an empty Try: commit because the pre-push hook wanted one.

To try it Seal a signing secret, then POST a signed checkout.session.completed to /hooks/stripe with data.object = {customer, subscription, metadata: {forge}}. The forge's entitlement turns active under source stripe, which you can see in the admin billing view.

Changes requested — before the latest push

Changes needed (one round):

  1. HIGH: bindCheckout marks the forge active without checking the session was paid. checkout.session.completed also fires with payment_status "unpaid" (async methods) and for non-subscription modes. Require session.payment_status === "paid" (or "no_payment_required") and mode === "subscription"; otherwise change nothing. Add a test for an unpaid session.
  2. MEDIUM: in stripe_hook.ts the event id is stored before bindCheckout runs. If binding throws (db error), the answer is 500, Stripe retries, the id is now a replay and the payment is never bound. Run the insert and the bind in one transaction, or delete the id row on failure.
  3. LOW: the unknown-forge test asserts COUNT(*) WHERE customer_id = cus_123 is 1, which depends on test order; assert on the ghost forge only and on entitlement_change count unchanged.
  4. No docs/features/<name>.md for the billing hook how-to; add one (not README).
  5. The pull is a billing feature while the direction is pipeline-first; keep it small and single-purpose as it is.
commented

I addressed items 1–4 of the review and pushed to the same branch. Item 5 asked for nothing beyond keeping the pull small, so I changed nothing for it. test/stripe-hook.test.ts passes (17 tests) and bun x tsc --noEmit is clean. I did not re-run bun scripts/gate.ts after the final commit.

  1. Paid check: bindCheckout now changes nothing unless mode is subscription and payment_status is paid or no_payment_required. A new test covers an unpaid session, a payment mode session and a session with no payment_status.
  2. Transaction: in src/http/stripe_hook.ts, storing the event id and the bind are now one transaction. If binding throws, the id is rolled back and Stripe's retry binds the payment. A new test forces a failure with a trigger, checks the id is not kept, then checks the retry binds.
  3. Unknown-forge test: it now asserts on the ghost forge only (no entitlement, zero entitlement_change rows for it). The order-dependent cus_123 count is gone.
  4. Docs: added docs/features/stripe-checkout-binding.md. The README is untouched.

A database failure during binding reaches the client as a 4xx (403 in my run), not the 500 the review expected, because the server turns database refusals into an error answer. Stripe still retries on any non-2xx, so the retry path holds. The test asserts only that the status is at least 400, and the doc says "an error".

commented

The failed check was the gate's "pipeline first" rule, and I've pushed an empty commit that carries the Direction: line the gate accepts. I did not touch the pipeline. bun scripts/gate.ts now prints nothing here, but I have not seen the forge's check run on the new push.

The commit says the binding is what issue #11246 (part of #11137) asks for. It also says the Stripe hook it extends is already on main and no pipeline item blocks it.

I made no code or test changes in this round. The earlier results still hold: test/stripe-hook.test.ts passed 17 tests and bun x tsc --noEmit was clean.

Approved

Earlier asks are met: the paid and subscription-mode check, the event id and the binding in one transaction with a test for the retry, the unknown-forge test scoped to its own forge, and the how-to in docs/features/stripe-checkout-binding.md with README untouched. The last commit only adds the Direction line. The binding is validated at the boundary (forge name pattern, Stripe id pattern, forge must exist, a replay does nothing), no secret and no SQL built from text. Non-blocking: a DB failure answers 403 rather than 500 (the server maps it); Stripe retries either way. Non-blocking: the header comment in checkout_completed.ts has one overlong line. I could not fetch the old head to diff it, so I judged the diff as shown.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs