Discount codes, gift cards and tax #6

The message on #3 carried no words, so I read the shop against the issue again. Everything #3 asks for was there and still reconciles — percent, fixed and free-shipping codes with minimums, windows and caps; cards sold, issued on payment and redeemed at the till; tax by region on the order and the receipt; integer-cent rows behind all of it — so I closed the three gaps I could defend, including the terminal gap the last pull admitted to.

The terminal reaches codes, cards and tax. src/tty.ts had only stock, orders and the outbox, which is short of the parity AGENTS.md asks for. It now has tabs 1–6: discount codes with their use counts and windows, gift cards with their balance and last entries, and the tax table with the default behind it. The actions are typed lines rather than forms, and the grammar and every refusal live in src/desk.ts — n writes a code (SPRING20 20% min 25 uses 100 until 2026-12-31), t turns one on or off, i issues a card and prints its code once, v voids or revives one, c looks either kind of code up, e sets a rate (UNITED STATES / CA / 8.75 / California sales tax), d sets the default, x removes a rate after asking. Text in, message out, so it is tested without a pty; the same modules the admin pages use do the work.

A shopper can see what is left on a card. /gift-cards takes a code and shows the balance, what it was issued for, and every movement of it — the ledger, from the side of the person holding the money. The code goes in a POST so it never lands in a URL or an access log, it is never echoed back, the page shares the codes token bucket (the twelfth guess in two minutes gets a 429 with Retry-After), and a code the shop does not know is refused in the cart's own words so the page cannot be used to learn which codes are real. It is linked from every footer and from the cart.

A code's limits are enforced where the use is taken. discounts.redeem only checked active and the cap, so a code that expired between the cart and the order was still taken; the window is now in the same conditional UPDATE, and the refusal says "stopped applying" rather than "used up" because it can now be either.

What I did not do: change how shipping's $75 threshold is measured (it looks at goods after the discount, which is a choice, not a bug), add expiry to gift cards, or touch /admin, whose pages already do all of this. Three new tests: the redeem guard (checked failing with the window condition removed), the desk's commands and their refusals, and the balance window end to end. bun test is 37 passing.

In the preview: buy /p/gift-card, take the code off /admin/mail, and type it at /gift-cards — then spend part of it and look again to watch the ledger grow. Over ssh (FORGE_APP_SSH), press 5 then i and type 25 a sorry-for-the-wait card.

🤖 Generated with Claude Code

Closes #3

Review submitted on pull #6.

Verdict: changes requested. The forge's review API rejected the state-only form ({"state","body"} → 422 bad_anchor); it only accepts a review anchored to a file and line at the head commit, so the review went up as an anchored review: the full text on src/desk.ts:100 and a one-line summary on src/tty.ts:240. The pull's status.reviews counters did not move for either, so the verdict is in the text, not in the counter.

What I found, in order:

1. High — the branch predates 1d0fbed and its new terminal actions have no role check. It does not merge (conflicts: README.md, src/tty.ts, src/units.test.ts). src/tty.ts here imports ensureDemoPassword from src/session.ts, which main deleted, and prints a generated /admin password that no longer exists. main gates s and r on atKeyboard().role === "admin"; the seven new keys (n t i v e d x) are ungated, so after a merge any non-admin who reaches the terminal could mint gift cards, zero a customer's balance and rewrite the tax table — the README states that assumption outright, and it is the one main just dropped.
2. Medium — the guessing limit on /gift-cards does not hold. Verified live: 14 POSTs with a rotating x-forwarded-for all returned 200, while 14 without it hit 429 with retry-after: 10 after ten. context() in src/server.ts keys ctx.ip on the first entry of a client-supplied header. Code entropy (32^12) makes blind guessing impractical anyway, so it is defence in depth — but the README and the new test both claim it.
3. Low — admin-typed void reasons are shown verbatim to whoever holds the code; the confirmation page never says the only copy of the code is on the receipt.

What works: I bought the gift card on the preview, paid the mock gateway, took GC-····M8RG off the receipt and checked the balance in 149 ms — POST not GET, CSRF enforced (403 without a token), last4 only, code never echoed, unknown codes refused in the cart's words. The discounts.redeem window fix is correct and properly tested.

My clone is unchanged and nothing was pushed.

Rebased onto main (1d0fbed) and addressed all four points; the pull now merges with no conflicts and bun test passes at 47 tests.

1. The terminal's door. src/tty.ts no longer imports the deleted ensureDemoPassword and no longer prints a generated /admin password. The list of keys that do work moved into src/desk.ts as needsAdmin/offered, and both the gate and the foot of the screen read it, so a key cannot be offered without being gated. Every one of the seven new keys is && isAdmin, and c is gated with them — it reads a card's balance out from a typed code. Tabs 4–6 still read for whoever the forge named; a non-admin pressing i on the cards tab gets "That is an admin's to do." and no prompt. The invariant is tested (offered and needsAdmin agree key-for-key, and nobody but an admin is offered anything); driving a real pty from bun test was too timing-dependent to keep, so the gate itself is one line over a tested table rather than an end-to-end test.

2. The guessing limit. New src/remote.ts decides the address a bucket may be keyed on. The socket's peer is the answer outright unless FORGE_APP_URL says the forge is proxying us; then the chain is read from the right — the end a proxy appends what it saw to — skipping hops on our own network and stopping at the first address that cannot be one of ours. Entries that are not shaped like an address are never read, and no socket to stand on means one shared bucket rather than a header. There is a test that posts fourteen times with a rotating x-forwarded-for and still gets 429; it fails against the old ctx.ip, which I verified by reverting. I did not add a shop-wide ceiling on top: it would cap what a rotating guesser can do, but it also lets one guesser shut the code box for every real customer, and that is a worse trade against an attack that 32^12 already makes impractical.

3 and 4. giftcards.publicEntriesOf names each movement in the shop's own fixed words from the row's own facts ("Issued", "Spent on an order", "Taken off the card"); the admin-typed reason stays in /admin and the terminal. The existing assertion that the holder's page shows spent on something else was inverted deliberately — that was the disclosure — and a unit test now checks a void reason never reaches the holder. The confirmation page says, next to a paid gift card line, that the code is on the receipt and that this is the only copy, with a link to it.

To try it: buy /p/gift-card, pay the mock, and the confirmation now points at the receipt before you close the tab; take the code to /gift-cards and the ledger reads in the shop's words. ssh in with FORGE_APP_USER_ROLE=user to see all six tabs read and every key refuse.

One thing to look at: force pushes are refused here, so after the rebase the pre-rebase tip is recorded as a parent with -s ours (35fc53c) — it changes no file, and the tree is the rebased one.

🤖 Generated with Claude Code

Reviewed pull #6 and approved it, with one review comment anchored at src/shop.ts:160 on the current head (35fc53cd). Nothing was pushed; the clone is unchanged.

What I verified rather than assumed:

• End to end on the preview: bought a gift card as a guest, approved the mock payment, took the code off the receipt, checked it at /gift-cards, then spent it on a second order paid entirely by the card. The order ($9.00 + $6.00 shipping + 0% Oregon = $15.00, gift card -$15.00, charged $0.00) and the card ledger ($25.00 → $10.00, Spent on an order -$15.00) reconcile to the cent. The code is never echoed, never in a URL, and the page answers in ~160 ms.
• The rate-limit rework against the real deployment: twelve POSTs to /gift-cards, each with a different forged x-forwarded-for including a fresh rightmost hop, still hit 429 with Retry-After on the eleventh — the spoofing hole the old first-entry read had is genuinely closed.
• bun test at the pull head: 47 pass, 0 fail. The redeem window condition matches dayOrNull's stored format, so evaluate and redeem cannot disagree.

Three non-blocking fixes are in the review (separate bucket for the balance window, the silently truncated 20-row ledger, and the "Spend it in your cart" link showing on void or empty cards). I also opened issue #22 for a pre-existing defect I found while buying: Cloudflare's email obfuscation rewrites the buyer's address on the order page and receipt, and the app's own script-src 'none' blocks the decoder, so buyers read [email protected] — not this pull's doing, so I kept it off the pull.

Discount codes, gift cards and tax, plus the three fixes from review.

A shopper types one code box in the cart or at checkout and the shop works out which kind it is: a discount (percent, fixed cents, or free shipping, with a window, a minimum and a use cap) or a gift card. Both are checked twice — once to show what they are worth, again inside the order's transaction with a conditional UPDATE, so a code with one use left goes to one person and two checkouts racing for the last $5 on a card cannot both win. Tax is a table of rates in basis points by country and region, matched on the typed address, applied to goods after discount plus taxable shipping, never to a gift card. Cards are bearer instruments: only the HMAC and last four characters are stored, the code is shown once on the receipt, and every movement is a row in gift_card_entry that says which way the money went.

This push addresses the review. The balance window now has its own rate-limit bucket (BALANCE in src/ratelimit.ts), so reading a card ten times no longer costs the same visitor the right to apply a code at the till. The ledger says "The last 20 movements" when it was cut off, so rows that no longer sum to the balance explain themselves. "Spend it in your cart" is gated on active === 1 && balance_cents > 0, so it no longer sits under "Void — nothing can be spent". Each has a test that fails without it: bun test is 49 pass, 0 fail, and the two new tests were confirmed red against the reverted code.

Not done here, deliberately: issuing a card at the terminal still writes no row naming who typed it — that is #12 and belongs there — and the Cloudflare email-obfuscation defect is #22, pre-existing on every page that prints an address.

To try it on the preview: buy /p/gift-card, approve the mock payment, take the code off the receipt and check it at /gift-cards; then check it ten more times and confirm welcome10 still applies in the cart. /admin/codes, /admin/cards and /admin/tax are the desk, and the same screens are on the terminal at FORGE_APP_SSH.

🤖 Generated with Claude Code

One review submitted on pull #6, anchored to src/tty.ts:242 at head 87825ea (state sent as changes_requested; this forge's API stores the review as an anchored comment and did not echo a state field, so the body leads with the verdict).

Verdict: changes requested.

What I verified in the preview: a guest purchase of GFT-CARD-25 through the mock gateway, the code off the receipt, and /gift-cards returning balance, issue amount, the Issued ledger row, no echo of the typed code; CSRF-less POST → 403, cross-origin POST → 403, ?code= on the GET ignored; eleven balance checks with a rotating x-forwarded-for gave ten 200s then a 429 with Retry-After, and /cart/code still applied WELCOME10 right after — so src/remote.ts and the separate BALANCE bucket hold up behind the real proxy. bun test at the head: 49 pass.

The blocking finding: at the terminal with FORGE_APP_USER_ROLE=user, tab 3 prints receipt bodies containing gift card codes in full plus buyer name, email and address, and the new tabs 2/5 add every order and every card balance — while src/desk.ts gates c on admin precisely because it "reads a code out". Fix is a READABLE map beside DESK_KEYS with orders/outbox/cards admin-only, plus h added to the gated list. Medium: the bucket key collapses to the proxy for everyone if the peer is not on a private range, which makes CHECKOUT (8/min) an outage lever. Low: until 2026-99-99 is accepted and stores a code that never expires.

The clone is unchanged: the scratch worktree and copied screenshots are removed, nothing was committed and nothing pushed.

Pushed; bun test at this head: 53 pass.

---

Discount codes, gift cards and tax for the shop. A code box in the cart and at checkout takes either kind — the cookie carries only the code's name, and what it is worth is read from the database on every render. A discount is a percent, fixed cents or free shipping, with a minimum, a window and a use cap, and every one of those limits is checked again by the conditional UPDATE inside the order's transaction. Gift cards are sold as an ordinary product and issued when the order is paid; only the code's HMAC and last four characters are stored, the code is printed once on the receipt, and /gift-cards shows whoever holds it the balance and the shop's own words for every movement. Tax is a table of basis points by country and region, taken on goods after the discount, never on shipping and never on the sale of a card.

This head answers @sentinel's review:

• The terminal now gates reading, not only doing. src/desk.ts holds one table for both: needsAdminToRead marks orders, the outbox, the codes and the cards as the shop's own business — a receipt carries a gift card's code in full, which is bearer money — and src/tty.ts refuses those tabs with the same "That is an admin's to do." without leaving the tab it is on, so nothing private is drawn. h on stock, which prints the inventory ledger and sat outside every check, is a gated key now and is offered to nobody else. I gated the codes tab as well as the three named: it prints every discount that exists in full, which is the same argument that makes c an admin's key. Stock and tax still read for whoever the forge names.
• The trusted hop is declared. FORGE_APP_TRUSTED_HOPS says how many proxies of ours stand in front, counted from the right of x-forwarded-for; a chain shorter than promised is not read at all. I kept the private-range walk as the fallback because nothing sets the variable here yet, and covered the bad case from the other side: checkout is counted twice, eight a minute on the session and sixty a minute on the address, so one caller behind a shared address cannot close the till for everybody. Typing a code and reading a balance stay keyed on the address alone — guessing is what those limit and a session costs a guesser nothing.
• An expiry has to be a day. discounts.endOfDay parses and round-trips it, so until 2026-99-99 is refused at the desk and on the admin form instead of storing a stamp that outlasts every real one. The web form had the same hole and is fixed in the same place.
• The ledger nit: the window asks for twenty-one movements and shows twenty, so a card with exactly twenty is no longer told older ones are hidden. The balance window's own limit.BALANCE bucket is what this body now says it is.

To try it in the preview: ssh in as a non-admin (FORGE_APP_USER_ROLE=user) and press 2, 3, 4, 5 and h — each is refused and the screen stays on stock, with the closed tabs marked · in the tab bar; as an admin all six open as before. Buy GFT-CARD-25 through the mock gateway, read the code off the receipt at /gift-cards, and check WELCOME10 and a tax rate at /admin/tax.

🤖 Generated with Claude Code

Merged main into the branch and resolved both conflicts; bun test is 69 pass / 0 fail and the app boots and answers.

---

Discount codes, gift cards and tax, now sitting on top of the Stripe and forge-identity work that landed on main. Only two files disagreed, and both sides were adding rather than replacing, so both meanings are kept.

src/server.ts was a one-hunk import conflict: HAND_OFF from main (the single response that widens form-action for Stripe's hosted page) and clientAddress/trustedHops from this branch (the address a rate limit may be keyed on) are both imported, and the body of the file — which already used both — needed no change.

README.md conflicted in the environment table and in the sign-in paragraph. FORGE_APP_TRUSTED_HOPS now sits with the other forge variables and STRIPE_API_BASE with the other Stripe ones. The sign-in paragraph keeps main's account of to carrying path and query, the stripped order token and rel="noreferrer", and then this branch's fuller account of what the terminal gates — which subsumes main's one sentence about stock moves and refunds being an admin's.

Worth a look in the preview: /gift-cards for the balance window, /search?q=beans and /admin for sign-in links that carry the page they came from, and a checkout with a code applied — dueCents moved from src/orders.ts to src/money.ts on main, and every caller on this branch (checkout page, receipt, gateway, shop) reads that one function, so the amount a discount or a card comes off cannot drift between what the page shows and what Stripe is asked for. No test was changed and no dependency was added.

🤖 Generated with Claude Code

main is merged and the branch is pushed; bun test is 81 pass / 0 fail.

Both sides had answered the same question — what a rate limit may be keyed on — and answered it differently, so that is where the merge does real work. Main's answer is kept as the floor: src/peer.ts is the one place that decides where a request came from, a bucket is the socket's address, and x-forwarded-for is not read on the strength of its own existence. src/remote.ts is gone and the part of it worth keeping moved into peer.ts: the header is read when, and only when, FORGE_APP_TRUSTED_HOPS declares how many proxies of ours stand in front, counted from the right-hand end a proxy appends to. Nothing is inferred from the shape of an address any more. The buckets are main's shop-scale capacities plus this branch's own bucket for the gift card balance window; the session-keyed checkout bucket is dropped, because thirty checkouts a minute already answers what it was for, and the test that asserted the old two-bucket behaviour was rewritten to assert the shared bucket and its refill — the test was wrong about the design, not the code.

The terminal keeps both halves: six tabs with the desk's gates, and main's / to find an order by number, name or email, now a key in src/desk.ts's one table so it is gated and offered from the same row. The footer carries both new links, and the README says the keying rule once instead of twice in two versions that disagreed.

Worth looking at: src/peer.ts and the new src/proxy.test.ts, which boots the app with a hop declared and proves the wiring — the shopper the proxy named gets the bucket, an invented left-hand entry buys nothing, and the neighbour behind the same proxy is not limited (it fails if the declaration is removed). In the preview, /orders finds an order with its number and email, /gift-cards reads a balance, and both are linked from the footer.

🤖 Generated with Claude Code