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

The webhook settles only the session its own order started #70

Closed opened this
Closed without merging

An order now settles only for the Checkout Session it started. That session is kept in a new `order.checkout_session_id` beside `payment_ref` — `payment_ref` becomes the payment intent the moment the order settles, so it cannot be the thing a replayed webhook is matched against; the migration backfills the column from any `payment_ref` that still looks like `cs_…`, so a pending order placed before…

stopped closed without shipping deploy b136628

opened this

An order now settles only for the Checkout Session it started. That session is kept in a new order.checkout_session_id beside payment_ref — payment_ref becomes the payment intent the moment the order settles, so it cannot be the thing a replayed webhook is matched against; the migration backfills the column from any payment_ref that still looks like cs_…, so a pending order placed before this change stays settleable. shop.stripeWebhook now matches the event's session.id against that column: a mismatch writes an unmatched order_event naming the session that arrived and the session the order started, once per session named, and answers 200 so Stripe stops retrying. An event whose order number is not BZ-… or not an order here is answered 200 and logged as one line, with no database write at all — the number in the log is only ever the shape it passed. The return path is held to the same rule: it reads the session from the new column, requires Stripe to answer with that same session still naming this order, and returns early when a charge is already recorded (previously that early exit came for free from payment_ref being clobbered, which was an accident, not a rule). /admin/orders/:number shows unmatched deliveries beside the payments, and the terminal's orders tab shows the same line under the order.

Worth looking at: orders.startedSession/noteUnmatched in src/orders.ts, the two settle paths in src/shop.ts, and the four new tests in src/stripe.test.ts (another order's session settles nothing and writes unmatched; an unknown order number writes no order_event row anywhere; the same good event delivered twice settles once; a returning session renamed at Stripe does not settle). bun test is 106 passing, green under --randomize. No dependency was added.

What I did not do: no gateway_event table — delivery-level bookkeeping stays for #16, and nothing here writes outside the order. The mock gateway path is untouched: it never goes through session matching, since it settles directly.

🤖 Generated with Claude Code

Closes #33

commented

probe-do-not-post

commented

Review submitted on pull #70: approved, anchored at src/shop.ts:522.

Verified at head 487d4111: read the full diff, ran bun test in a scratch copy (106 pass, 0 fail, green again under --randomize), and walked the preview end to end (placed and paid BZ-VDHUX674 through the mock gateway, security-floor headers intact, unsigned webhook 400, /admin 403 to an anon). The session-match rule is enforced from data the app owns on both the webhook and the return path, identifiers and order numbers are shape-checked before they reach a query or a log, output is escaped, and the terminal's orders tab keeps its admin gate. I probed the one hazard I saw — an older session being paid after a retry — and the checkout:<number> idempotency key closes it. Flagged for the rebase: the branch is two commits behind main and conflicts in src/tty.ts, and the unmatched event should emit on src/live.ts's bus once merged. My clone is unchanged; one stray probe comment on the pull is acknowledged in the review body.

commented

The preview is up and the floor holds. Work is pushed.

---

The webhook settles only the session the order itself started, now merged with main and wired into the live bus.

main moved twice under this branch, so this push carries the merge as well as the review's two follow-ups. The one conflict was src/tty.ts's import list — recent left with main's orders tab, unmatchedOf kept. On top of that: orders.noteUnmatched now emits order.unmatched after the row is written and only when a row was written, so a delivery Stripe retries writes nothing and says nothing a second time; the payload is the order's id and number, because the README already promises an event never carries a gateway reference and the session that was named belongs on the page behind the admin's door. The admin's order page is live for the first time, with the payments column (notice, payments, history) as its region, so a settlement, a refund typed at the terminal and an unmatched delivery all arrive on an open page instead of waiting for a reload. orders.unmatchedIn is now the one place the word is spelled and pages/admin.ts filters the events it was already given through it, replacing the inline test the review flagged. The return path bounds the session it writes down with the same gatewayId shape the webhook path uses, so both directions record identifiers in one alphabet.

Two test changes, both because main moved and neither because a test was wrong about the money: the four new fake-Stripe tests bought as guests, which #32 no longer allows, so they buy as accounts like every test beside them. New coverage: a unit test that an unmatched delivery reaches the bus once per session named, never on a retry, and without the session id in the payload; and an assertion that /admin/orders/<number> carries the order-payments region.

bun test is 132 pass, 0 fail, green under --randomize. On the preview the security floor is on every response, an unsigned POST /webhooks/stripe is 400, /admin is 403 to an anon, and / answers in ~150 ms cold. Worth looking at: the payments column of any order under /admin/orders/<number> with a second tab open, and src/orders.ts:144 for the emit-once rule.

Not done: the review's suggestion to widen startedSession to the order's checkout-started events is left alone — it is only needed if the checkout:<number> idempotency key ever changes, and changing it now would be inventing work the money path does not need.

🤖 Generated with Claude Code

commented

You've hit your weekly limit · resets Sep 20, 2am (UTC)

commented

The preview is up on the merged branch and the gates hold.

---

The webhook settles only the session the order itself started, merged with main and now saying whose hand each refusal was.

main gained the actor columns and /admin/audit (#77) and a request-body deadline (#87). Three files conflicted and both sides meant the same thing in each: orders.OrderEvent was named by both, so main's version — the one carrying the actor columns — wins whole; shop.ts keeps main's actor arguments on note and settleFromGateway with this branch's session-match rule and rememberCheckoutSession around them; and in pages/admin.ts main's who column and this branch's unmatchedNotice each landed where the other was, so the notice now carries a who column too, inside the live region the payments column became.

The substantive resolution is orders.noteUnmatched, which now takes an actor and writes the three columns like every other row in this ledger. Who that is differs by path, truthfully: stripe when a webhook brought the delivery unasked, and the account paying when it was a shopper's return that read the session. The dedupe still keys on the order and the detail alone, so the same bad session seen from both paths is still one row and still one line on the bus — there is a test for exactly that. /admin/audit reads every order_event row, so an unmatched delivery joins the money trail with no new code; that is asserted rather than assumed, and the fake-Stripe tests now pin the actor on both paths.

One change is not this branch's work and should be read as such: #77's audit paging test asserts an "older" link, which needs more rows than a page holds, and it took them from whichever tests happened to run before it — so it failed under --randomize on main as well as here. It writes its own rows now. bun test is 141 pass, 0 fail, green under six different seeds; the preview answers / in ~120 ms, an unsigned webhook is 400, and /admin and /admin/audit are 403 to an anon. Worth looking at: src/orders.ts:161 for who a refusal names, and the payments column of any order under /admin/orders/<number>.

🤖 Generated with Claude Code

commented

main is merged into this branch. Three files conflicted and each was a case of the two branches saying the same thing in different words, so both meanings are kept:

  • src/admin.ts — both sides made /admin/orders/<number> a live page. Kept main's line with this branch's comment explaining why the money can move while the page is open (a webhook settling it, a refund typed at the terminal, a delivery naming a session the order never started).
  • README.md — kept main's fuller description of the order page (status, totals, what is left to refund, payments and history moving together) and folded the unmatched delivery into the list of things that arrive there.
  • src/server.test.ts — both sides had rewritten the audit-paging setup so the test writes its own trail instead of depending on rows earlier tests left behind. Kept main's version, which stocks a shelf of its own rather than inserting order_event rows by hand.

Everything else auto-merged: main's out-of-stock race and gift-card staleness checks in orders.place, the src/pages/oops.ts refusal page behind tooMany, and the read deadline in src/http.ts all sit beside this branch's session-matching settle path without touching it. bun test passes — 164 tests, in file order and under --randomize — and the app boots with / answering 200.

In the preview, the thing to look at is /admin/orders/<number>: it is live now for both reasons, so an unmatched delivery shows up in the payments column without a reload, next to a settlement or a refund.

🤖 Generated with Claude Code

commented

Closed in the owner's sweep of 2026-10-12: a week old and in conflict with main now; the branch stays if anyone wants it back.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs