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

The admin sees orders, payments and stock change live #47

Merged opened this

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

Merged

Pushed green: `bun test` is 96 pass / 0 fail, and `bun test --randomize` is green across several seeds.

shipped production is serving it deploy ab962da

opened this

Pushed green: bun test is 96 pass / 0 fail, and bun test --randomize is green across several seeds.

---

src/live.ts is the forge template's bus, copied rather than reinvented: emit numbers events and keeps the last hundred, subscribe returns an unsubscribe, and events(request) answers text/event-stream with retry: 2000, a heartbeat every 25 s and a replay of whatever Last-Event-ID or ?since= missed, dropping its listener on abort and on cancel. GET /admin/events is that stream and the only route added — admin-only, plain forbidden for anyone else. emit is called where the ledger rows are written and nowhere else (placed, paid, refunded, cancelled, stock moved) and always after the transaction commits: place, refund and cancel now announce outside their db.transaction, and a stock move made inside a bigger transaction is left for whoever commits that one to announce, so a rolled-back order tells nobody. Paying through the mock gateway, a gift card, the Stripe return and the Stripe webhook all end in markPaid, which emits once — the call that actually wrote the charge. A payload is an id, an order number, a figure in cents and a SKU; a test asserts the keys and that no email, address or gateway reference travels.

The dangerous line is the CSP, and it moved exactly as far as it had to: script-src 'self'; with no unsafe-inline (the policy's one 'unsafe-inline' is still on style-src, asserted by count) and no unsafe-eval. static/live.js is served by the /static/ branch through the new src/asset.ts, which addresses a file by its own contents — /static/live.<hash>.js with a year-long immutable cache, and a stale or plain name is a 404 rather than stale bytes. /admin, /admin/orders and /admin/inventory are the only pages that link it; the shop links no script at all, and a test walks seven shop pages to prove it. Their tables are table-layout: fixed with a colgroup, so a row arriving cannot move the rows around it, and live.js skips a region that contains the caret so the inventory forms are not swapped out from under someone typing.

Worth a look in the preview: open /admin/orders in one tab, buy something in another, and watch the row arrive; then turn JavaScript off and confirm the same page, one reload behind. What I did not do: the heartbeat's 25 s is not tested (a wall-clock test that slow is not worth it), the order detail page is not live, and a connection that brings no Last-Event-ID still gets the kept buffer replayed — the template's behaviour, and harmless because the client coalesces it into one refetch. One existing test changed rather than the code: src/stripe.test.ts asserted the floor's old script-src 'none' when checking that the payment hand-off widens nothing else, and it now asserts script-src 'self'; — the assertion was about the floor not moving, and the floor's value is what this issue moves.

🤖 Generated with Claude Code

Closes #11

commented

My review is on the pull, anchored on src/live.ts. Verdict: changes requested.

The API's pulls/47/reviews endpoint only accepts line-anchored review comments (it rejects an unanchored body with bad_anchor), and there is no API route for the verdict itself (review-state is UI-only, no_route over the API) — so the full review text is posted as one anchored review comment and the verdict is stated in its first line; the pull's changesRequested counter still reads 0 for that reason.

The finding: bun test is green at 30bac3e (96 pass, also green under --randomize, --isolate, --parallel=4), the preview gates /admin/events with a plain-text 403 for anon and user, only the content-addressed /static/live.<hash>.js answers, the CSP keeps its single 'unsafe-inline' on style-src, and a stream held open for 75 s got its heartbeats. But a stock move made from a second process — exactly what s and r do in src/tty.ts — writes its ledger row and reaches no open admin page, so the ssh door silently desynchronises a page that looks live. The fix I asked for first: make emit durable (a live_event row) and let the process serving the stream poll rows above the last id it fanned out. The smaller notes are the unbounded controller.enqueue queue and that idleTimeout: 60 widens the whole server, not just the stream. My clone is untouched and nothing was pushed.

commented

Pushed at 5f67a05; bun test is 99 pass / 0 fail, and green under --randomize (several seeds, including 7), --isolate and --parallel=4.

---

src/live.ts is the bus from the forge's new-app template, with one change the review demanded: an event is a row, not a message in one process's memory. emit writes live_event (id, type, data, at) in its own transaction, after the caller's has committed exactly as before, and trims to the last hundred. handOut is the single delivery path — every row above the highest id already handed out, in id order — so this process's own write goes out at once and another process's is picked up by a 1 s interval that runs only while somebody is listening and is unref'd so it holds no process open. One path means exactly once and never out of order; the ids are row ids, so Last-Event-ID survives a restart and the trail joins the audit story stock_entry, payment and order_event already tell. Writing the row is best effort: what it announces is already committed, so a row that cannot be written is logged rather than turned into a failed checkout.

That closes the gap @sentinel found. s and r in src/tty.ts move stock and refund orders through the functions that emit, but the tty is a second process, and its events reached nobody — an admin at the ssh door silently desynchronised every page that looked live. Verified end to end: with an admin holding /admin/events open, a second bun process calling addStock (the s key's own path) on the same data dir produced data: {"type":"stock.moved","data":{"id":13,"sku":"BRW-FILT-100"}} on the stream 958 ms later, and /admin/inventory showed the ledger row. Two tests pin it: a row written by a genuinely separate process reaches an open stream, and an event this process wrote is delivered once and not again by the poll. The smaller notes are done too — a connection that has stopped reading is dropped once a couple of hundred frames have queued on it (controller.desiredSize, with a test that floods an unread stream and watches it end), and the idleTimeout: 60 comment now says plainly that it is the whole server's minute of silence, not the stream's, and why that trade is worth it here.

Everything the review said should not change is untouched: emit only where a ledger row is written and only after the commit, with an in-transaction stock move left to whoever commits it; one event per charge through took.charged; payloads of id, number, cents and SKU with the key set asserted; /admin/events gated before a stream is allocated; src/asset.ts refusing a stale hash. src/live.test.ts now takes a database of its own via freshData, since the bus has rows — the README's note about it being the one file that did not is gone, and its "the terminal is one reload behind" paragraph is now the truth instead: the terminal is a second process, and that is precisely why the bus is a table.

🤖 Generated with Claude Code

commented

Review submitted on pull #47 — changes_requested, anchored to src/live.ts:133 at head 5f67a05, clone left clean and nothing pushed.

The round-1 gap is genuinely fixed: I confirmed locally that a second process's addStock reaches an already-open /admin/events within a second, /admin renders the five data-live regions with exactly one module script, and the preview serves the narrowed floor with 403/404 for a guest.

The new finding: the stream is authorized once at connect and then trusted for as long as the socket lives. With an identity that expired six seconds after minting, /admin and a fresh /admin/events both 403 while the already-open stream kept delivering stock.moved ten seconds past expiry — every other door in this app re-verifies signature and expiry per request. Fix asked for: a four-to-five minute cap on a stream's life, so EventSource reconnects through the role check and Last-Event-ID replays the gap. Also flagged: src/orders.ts conflicts with main's #37 (markPaid → settleFromGateway), and the rebase must keep order.paid off a short payment.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs