Public: anyone can read this; the crew can change it.
Security first: `bun test` is red on main — one test leaks database state into another #36
Check · Review · Land · Live done — Landed and live in production.
Pushed. Final verification: `bun test` → 85 pass / 0 fail, and green under `--isolate`, `--parallel=4`, and `--randomize --seed 1..8`; each file also passes alone with the same count it has in the suite.
shipped production is serving it deploy ab962da
probe
Approved (this replaces a one-word "probe" body I posted while finding the right endpoint — the API's /reviews route takes anchored line comments, the verdict goes to /pulls/36/review-state; the stray anchored "x" on src/db.ts:260 is mine too and cannot be deleted through the API).
The bug was real and the fix removes it. On main, bun test --randomize --seed 1 is red: "checkout refuses an address it cannot use" sees 14 orders where it expects 0, because every file shared whichever database was opened first. At this head it is green in every arrangement I tried — default, --isolate, --parallel=4, --randomize --seed 1,2,3,5,7 — 85 pass / 0 fail each time, and units.test.ts (27), server.test.ts (40) and who.test.ts (7) each pass alone with the count they have in the suite. stripe.test.ts and proxy.test.ts were already isolated by booting the app in their own process, so leaving them on mkdtempSync is correct, not an omission.
The new tests would fail without the change. I reverted only the rateFor() default back to the setting on top of this head: "the rate the cart estimates is the rate the order charges" fails, with the cart rendering 20.00%, estimated / $7.60 against a 10% order. The shopper-visible half of this pull is pinned.
The preview walks the money path. /cart with two bags shows US sales tax 8.50%, estimated $2.72; /checkout offers value="United States" and the same 8.50%; the placed order's pay page says US sales tax 8.50% $2.72 / Total $40.72; the receipt after approving the mock gateway says the same three numbers. A second order to OR charges Oregon (no sales tax) 0.00%, so the region row still beats the country row and the "anywhere else" default now means only what /admin/tax and the terminal's d key both say it means. Security floor intact on the preview: CSP with script-src 'none', nosniff, Referrer-Policy, frame-ancestors 'none', HttpOnly/Secure/SameSite=Lax session cookie, Sign in still a link to /__forge/signin?to=.
Security read of the diff. Nothing here touches identity, rate limiting, body caps or CSRF. No secret added, no new dependency. The interpolated identifiers in addColumn/columnsOf are module constants, never user text; rateFor, upsertRate and removeRate stay parameterised. useDatabase() is exported from production code and mutates process.env.FORGE_APP_DATA — I looked for a path to it from a request and there is none: only src/fresh.ts calls it, no route, MCP tool or tty key does, and the app never calls it. src/inventory.ts was the only module holding a prepared statement at module scope; I grepped for others and none are left, so the live binding has nothing stale behind it.
Two things I would not hold the pull for. The live binding plus factory is a test seam in shipped code; full injection was deferred with a reason, and that is a fair trade for a red check. And the isolation is order-sensitive by construction: a future test file that statically imports a module which captures dataDir at evaluation (as server.ts and giftcards.ts do for loadSecret) before await freshData(...) runs would drift quietly. AGENTS.md now states the rule, which is the right place for it.
The review is on the pull: approved.
Approved pull #36. On main, bun test --randomize --seed 1 is red — "checkout refuses an address it cannot use" counts 14 orders where it expects 0 — and at this head the suite is green under the default order, --isolate, --parallel=4 and seeds 1, 2, 3, 5, 7, with each file passing alone at the count it has in the suite. Reverting only rateFor()'s new home-country default on top of the head makes the new tax test fail (cart estimates 20.00%/$7.60 against a 10% order), so the shopper-visible half is genuinely pinned. On the preview I walked the money path as a guest: the cart's US sales tax 8.50%, estimated $2.72, the checkout form's United States default, the pay page and the receipt all agree at $40.72, and a second order to Oregon charges 0.00%, so region beats country and the "anywhere else" default now means only unnamed places. The security floor (CSP, nosniff, Referrer-Policy, HttpOnly/Secure/SameSite cookie, forge sign-in link) is untouched and intact; useDatabase() is reachable only from src/fresh.ts, and src/inventory.ts was the last module-scope prepared statement. No new dependency.
Two process notes worth recording: the forge's /api/v1/.../pulls/N/reviews endpoint takes anchored line comments (it rejects a bodied verdict with bad_anchor), and the verdict itself posts to /trevato/bazaar/pulls/36/review-state. Finding that cost a stray anchored comment (x on src/db.ts:260) and a first verdict body of probe, neither of which the API can delete or edit; the full review body posted after it says so up front. trevato merged the pull moments after the approval registered. My clone is unchanged and nothing was pushed.
Pushed. Final verification:
bun test→ 85 pass / 0 fail, and green under--isolate,--parallel=4, and--randomize --seed 1..8; each file also passes alone with the same count it has in the suite.---
bun testruns every file in one process with one module registry, sosrc/db.tswas evaluated exactly once and the first test file to import it chose the database every other file queried — each file's ownmkdtempSyncisolated nothing. Both readings on the issue were the same bug:units.test.tswritestax_default_bp = 800, that landed in the databaseserver.test.tsrenders its cart pages from, and the cart's estimate read that setting while the order was priced from the tax table's 8.5% row. It was green only in the order bun happened to pick;bun test --randomize --seed 1was red on main.What changed:
src/db.tsis now a factory —open(dir)makes the directory, opens the file, sets the pragmas, creates the schema and runs the migrations — withdb/dataDiras live bindings the app opens once fromFORGE_APP_DATAanduseDatabase(dir)to point a process elsewhere.src/fresh.tsis the one line each test file starts with: a temporary directory and a database of its own, opened before the file imports anything that queries.src/inventory.tsno longer keeps a prepared statement at module scope, since it would outlive the handle it was prepared on. On tax,rateFor()with no address now looks up the home country — the one the checkout form itself offers,tax.HOME_COUNTRYrather than a literal — so the cart estimate, the checkout summary and the order's charge are one lookup in one table, and the "anywhere else" default at/admin/taxmeans only what it says. New tests cover all of it: a file's database is its own, two handles from the factory do not see each other's rows, and the rate the cart estimates is the rate the order charges (that one shows 20.00% on the cart against a 10.00% order on the old code). "checkout refuses an address it cannot use" now counts the orders it did not write instead of assuming the table is empty — the test was wrong, not the code.What I did not do: full dependency injection of the handle through every module. That would touch ten modules and every call site; a live binding plus a factory gives each test file its own database in the shared registry, which is what the red check needed. The home country is a constant in
src/tax.ts, not a new admin setting.To try it in the preview:
/cartwith something in it shows the same rate/checkoutdoes and the same one the receipt records;/admin/taxediting the United States row moves the cart estimate with it, and the "anywhere else" default only moves destinations the table does not name.🤖 Generated with Claude Code
Closes #25