carlos / platform Public

Multi-currency: the sixteenth review's findings, the rejected-events door

keep is accepted only for an intent whose bound session reads back as
its own, complete and carrying a subscription (anything else could never
record and would later expire unrecorded: both reviewers); the
resolution's outcome is written with it, part of what a replay must
repeat, and in the audit; the unreachable later retirement check is
gone. The operator can list the webhook events the console refused
(GET /api/billing/rejected). The provisioning script's step-2 notes say
what the console and its check accept about currency options. The
sixteenth review's reports; dispositions "Review sixteen".
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev db130cfcb5b2c67d098f77ad6bb655dc4cd88c53 parent 3527a06
11 files changed, +505 −36
  • docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md +1 −1
  • docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review16.md +123 −0
  • docs/superpowers/specs/DISPOSITIONS-multi-currency.md +18 −3
  • docs/superpowers/specs/OPUS-REVIEW-multi-currency-review16.md +142 −0
  • implementation/carloku-paid-launch/stripe-provision.sh +5 −3
  • internal/console/billingcurrency_test.go +71 −1
  • internal/console/billingoperator.go +106 −2
  • internal/console/billingoperator_test.go +1 −1
  • internal/console/billingservice.go +3 −10
  • internal/console/billingstore.go +34 −15
  • internal/console/server.go +1 −0

CI passed — run details

diff --git a/docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md b/docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md
index 2c333189e..c26671f76 100644
--- a/docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md
+++ b/docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md
@@ -149,7 +149,7 @@ New columns, both `TEXT NOT NULL DEFAULT ''`, on `billing_checkout_intents` and
- **Checkout** (`POST /a/{sqid}/billing/checkout`): one form, with the country and currency controls, the plan, the postal code and the consent. The form carries what the page priced (`quoted_country`, `quoted_currency`) and whether the currency shown was a choice (`currency_chosen`). A post whose country or currency is not what the page priced, judged on what would be bought (the chosen currency, else the posted country's own), or whose quote the page did not sign (`quote_sig`: an HMAC over the session token, the account, the book, the quoted country and its band at render time, the quoted currency, the choice flag and the member count the page priced at, keyed by a secret derived from the billing secret key, which only the server holds), is not a purchase: it is answered with a redirect to the page quoting the posted country (and the posted currency when it is a choice), so nothing is bought at a price a page did not show this session. The member count the page priced at is compared again when the door charges (under the billing lock; the intent snapshot's membership-locked check then holds that count against the account); a count that moved is a conflict (`?error=conflict`, the existing banner), not a higher charge. A currency shown as the country's default and not chosen is given to the door as no currency, and the billing country's own currency is bought in; a chosen one (the URL's, the pricing page's, the account's, or changed on the form) is the purchase's. A currency field that is present and is not exactly `usd`, `eur` or `gbp` (no trimming, no case folding, not empty): 400, nothing is created, before the quote is looked at; on the page, a `?currency=` that names such a thing is said (`billing.currency.unknown`, unreviewed system copy like `billing.quote.unknown`) and the default shown is not a choice. The quote comparison is made on what would be bought (the chosen currency, else the posted country's own), so a form whose fields disagree with each other, a choice flag erased, or a form with no currency control against a page that priced a choice, is answered with the quote for what it would buy. The intent stores the chosen currency and book. A retry with a different currency while an intent is open is a different intent, exactly as a different plan is today. `POST /a/{sqid}/billing/quote` is the form's "Show prices for this country" button: the same form, answered with the same redirect, buying nothing; it asks for the session and the form's token, no step-up.
- **A currency already held**: checkout is refused while the account's subscription has not ended (`canceled` or `incomplete_expired`), and after it has ended a checkout in any currency but the one bought in is refused, the billing country's default included; the page offers that currency alone, as a choice, for a purchase from before price books too (EUR). A change of currency waits on what Stripe permits for a customer whose subscription ended (test-mode item 4) and is then an operator's door, not the form's.
- **Stripe price identity**: the configured price for (book, plan, band, currency) must have that `currency`, `unit_amount` equal to the book's unit price, no `currency_options` but, at most, an entry under its own currency at its own amount and tax behaviour (which an expanded read of a single-currency price may carry), and metadata `price_book`, `currency`, `terms_version`, `plan`, `band` that all match. The checks that exist today (per-unit, yearly, licensed, exclusive tax, active, mode) stay.
-- **Checkout Session**: created with `currency` and `adaptive_pricing[enabled]=false`, and with `currency`, `price_book` in the session and subscription metadata. A purchase with no book (only a console with no book configured opens one, and an intent from before books is replayed as that build sent it) is asked for exactly as before: no currency, Adaptive Pricing on. Every session the console creates, replays, lists, reads back or binds passes one check against its intent; one that is this intent's and not its purchase (another currency, a conversion, Adaptive Pricing) is expired and the intent retired, and one already paid is left for a person with a review filed once. A subscription whose price is not the configured one for its stored (book, plan, band, currency), or that the provider reports in another currency, is an identity mismatch. Once an intent is bound, a read of its bound session that names another account, customer, intent or mode is a contradiction at the provider: the console expires that session by the id it bound (the answer proves nothing and retires nothing), files the intent-keyed review naming it, and leaves the intent standing until the operator resolves it: nothing records on an intent under an open review (the record's own transaction refuses), and the resolution either retires the intent (the default) or keeps it standing for a purchase corrected at the provider to record. A first purchase's subscription that names the bound intent and is refused for anything (price, currency, plan, band, terms, quantity) files the same review, after expiring the bound session if it is still open; a subscription naming another intent, or an intent the operator retired, is refused and settled.
+- **Checkout Session**: created with `currency` and `adaptive_pricing[enabled]=false`, and with `currency`, `price_book` in the session and subscription metadata. A purchase with no book (only a console with no book configured opens one, and an intent from before books is replayed as that build sent it) is asked for exactly as before: no currency, Adaptive Pricing on. Every session the console creates, replays, lists, reads back or binds passes one check against its intent; one that is this intent's and not its purchase (another currency, a conversion, Adaptive Pricing) is expired and the intent retired, and one already paid is left for a person with a review filed once. A subscription whose price is not the configured one for its stored (book, plan, band, currency), or that the provider reports in another currency, is an identity mismatch. Once an intent is bound, a read of its bound session that names another account, customer, intent or mode is a contradiction at the provider: the console expires that session by the id it bound (the answer proves nothing and retires nothing), files the intent-keyed review naming it, and leaves the intent standing until the operator resolves it: nothing records on an intent under an open review (the record's own transaction refuses), and the resolution either retires the intent (the default) or, when its bound session is complete with a subscription, keeps it standing for a purchase corrected at the provider to record; the outcome is written with the resolution and a replay must repeat it. A first purchase's subscription that names the bound intent and is refused for anything (price, currency, plan, band, terms, quantity) files the same review, after expiring the bound session if it is still open; a subscription naming another intent, or an intent the operator retired, is refused and settled.
- **Configuration**: `CONSOLE_BILLING_PRICE_BOOK` names the book and is required. `CONSOLE_STRIPE_PRICES` holds the 36 entries of the configured book, keyed `"<price_book>/<plan>/<band>/<currency>"`. Beside them it may hold the whole catalog of any other book this build has, and the twelve legacy keys `"<terms>/<plan>/<band>"`, each set all or none; those recognise subscriptions already bought and sell nothing. No price under two keys. Live mode with a draft book is refused.
- **Monthly calculation**: a month priced from a book is a version 2 snapshot: `currency`, `price_book`, and amounts under currency-neutral names. A version 1 snapshot is EUR, is read as it was written, and is never priced again. A stored calculation whose currency is not the month's assignment currency is refused.
- **Overage invoice**: the invoice and every invoice item are created in the calculation's currency and verified against it on every read. The currency is part of each request's parameters and so of its request hash; legacy EUR requests hash as they do today.
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review16.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review16.md
new file mode 100644
index 000000000..819e0b4aa
--- /dev/null
+++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review16.md
@@ -0,0 +1,123 @@
+The fifteenth review’s three numbered defects are closed. I found one new P3 in resolution outcome handling.
+
+All platform references below are at `3527a066`; site references are at `a7283e7`. Test counterfactuals are from source inspection. I modified nothing, ran no tests or mutations, and used no network commands.
+
+| Fifteenth-review finding or claim | Closed? | Would its test pass without the fix? |
+|---|---|---|
+| **Opus P3-1 / Astra 1:** corrected purchase records while its refusal review remains open | **Yes.** The consuming delete now excludes intents with an open refusal review; failure rolls back the recording transaction (`internal/console/billingstore.go:763`). `keep` provides the acceptance path Opus requested. | **No.** `TestACorrectedPurchaseWaitsForTheOperatorUnderItsReview` requires 503, unchanged billing state and a standing intent before resolution (`internal/console/billingcurrency_test.go:3905`). Removing the guard produces 204 and recorded state. Ignoring `keep` fails the standing-intent assertion at `:3920`. |
+| **Opus claim 1 / Astra lifecycle claim:** the intent stands until resolution | **Yes for an open refusal review.** Both ordinary retirement and first-purchase consumption are guarded (`billingstore.go:454`, `:763`). Resolution can now retire or keep it. | **No** for the corresponding cleanup and corrected-purchase regressions. |
+| **Opus claim 2:** unknown-terms comment incorrectly says the intent’s terms are known | **Code comment corrected** at `internal/console/billingservice.go:849`. The older disposition row still repeats the assertion; see claims below. | Not a behavioral change; tests cannot establish comment accuracy. |
+| **Opus claim 3:** early retirement lookup not distinguished from the later lookup | **Yes.** The unknown-terms event after resolution reaches the early lookup before validation (`billingcurrency_test.go:2953`). | **No.** Without the early lookup it gets 503 instead of 204. |
+| **Astra 2:** retired subscription retries while replacement intent stands | **Yes.** Retirement is checked for every subscription other than the recorded one, independently of `pending` (`billingservice.go:795`). | **No.** The replacement-standing unknown-terms and `incomplete` cases at `billingcurrency_test.go:3982` and `:3988` require 204. |
+| **Astra 3:** token check accepts `250 MB` inside `1,250 MB` | **Yes.** Site `hack/token.mjs:4` excludes grouping commas on the left too. | **No.** The new counterexamples at site `test/pricing-pricebook.test.mjs:234` fail with the former expression. |
+| **Astra claims:** review-four mutation explanation, provisioning’s twelve keys, lifecycle prose | **Mostly corrected.** The delete-only redundancy explanation is now right; provisioning says 36. The cited resolution test still does not exercise the distinguishing lookup history. | The delete-only mutant remains equivalent behind the exact lookup. Provisioning assertions inspect script text. |
+| **Astra claims:** literal provenance and test limitations | **Not fully corrected.** Output equality is tested; literal provenance is not. The site title/comment remain unchanged despite the disposition claiming a qualification. | **Yes:** replacing a derived value with its identical current literal still passes. |
+
+Both fifteenth reports classify the overlapping recording defect **P3**. Their disagreement is principally the remedy: Astra’s transactional hold satisfies the stated invariant; Opus correctly identified the need for a way to accept a corrected purchase. The combination of the guard and `keep` addresses both. Astra was also right that replacement-standing retries were a defect: the previous code provided no guarantee that the replacement would disappear within Stripe’s retry window. The earlier recording-after-retirement race warranted Astra’s P2; it remains closed.
+
+**Findings, most severe first**
+
+1. **P3 — Resolution outcomes are neither recorded nor included in replay identity.**
+
+ **Where:** `internal/console/billingstore.go:922`, `:950`, `:963`; `internal/console/billingoperator.go:229`.
+
+ **Scenario:** An operator resolves occurrence 1 with `outcome:"keep"` and note `"reviewed"`. Before the corrected purchase records, the same operator submits occurrence 1 with the same note and `outcome:"retire"`.
+
+ The replay branch compares only actor and note and returns success before examining `Outcome`. The second request therefore receives **200, `resolved:true`, `retired:false`**, while the intent remains eligible to record. Reversing the outcomes also succeeds without applying the requested change.
+
+ The resolution table has no outcome column (`billingstore.go:165`), its insert omits the outcome, and the separate audit call records only the reason (`billingoperator.go:232`). Consequently, the immutable resolution history cannot establish which decision was made for a particular occurrence. A retirement record cannot recover the outcome of every earlier `keep`, especially after recurrence.
+
+ **Smallest acceptable fix:** Normalize the default to `retire`, persist the outcome in the resolution transaction, and include it in exact-replay comparison. Return conflict for an opposite-outcome replay. Return the committed resolution result rather than reconstructing it from a later retirement lookup. Add tests for both replay directions and the persisted outcome.
+
+ The new corrected-purchase test passes with this defect: it exercises one `keep`, not an opposite-outcome replay or the audit.
+
+**Other requested checks**
+
+- **GBP purchase end to end — A.** `?country=IN&currency=gbp` renders band D in GBP. Personal is £7.70 annually with 29.75 awake hours, 0.7 GB storage and 0.7 GB transfer. The posted form needs India’s postal information. The handler validates the effective currency and signature (`billinghandlers.go:71`, `:102`); checkout verifies the configured GBP price and stores GBP/book on the intent (`billingservice.go:482`, `:514`, `:576`). Binding and reconciliation compare purchase identity before recording. Assignments retain those terms (`billingstore.go:737`), history reads both assignment tables (`billinginvoicestore.go:529`), and `AllowanceForMonth` requires consistent paid-span terms (`internal/plans/month.go:173`). Version-2 calculations, invoice creation, recovery, line verification and quantity updates use that stored currency (`billingcalculation.go:174`; `billinginvoice.go:859`, `:901`, `:1024`; `billingquantity.go:1016`). I found no conversion or cross-currency recording path.
+
+- **One form, effective currency and signature — B/M/Q/U/Z.** The server-derived HMAC covers session token, account, book, country, band, currency, choice flag and member count (`billingpage.go:446`). The disclosed token is an input, not the key. Another session, account, book, changed token or changed signed tuple invalidates it. An old tab can reuse its own unchanged quote; it cannot borrow another tab’s displayed prices. All selectable plans were shown. Quantity follows membership and plan minimums; the service checks membership and the store recounts under the membership lock (`billingservice.go:467`; `billingstore.go:348`). Postal affects tax, disclosed separately at checkout.
+
+ Present currency fields must contain exactly one supported lowercase value; empty, repeated, padded and unsupported values fail with 400 on both doors after their preceding authentication/form gates (`billinghandlers.go:176`). Removing the field uses the country default on a book console and EUR on a legacy console, still subject to quote comparison. Forging `quoted_*` or flipping the choice flag cannot forge the signature. Changed purchase inputs redirect to a quote. The quote endpoint performs an encoded redirect to that account’s billing page; it does not create a purchase (`:137`).
+
+- **Cookies and held currency — C/O.** Present-empty clears, absent falls back, invalid changes nothing (`billingcurrency.go:43`). Thus an invalid parameter can leave an earlier cookie available on a subsequent parameter-absent GET; an explicit empty parameter clears it. Cookies preselect the form but do not choose independently of its submitted fields. A previous purchase holds its currency, including legacy EUR, irrespective of URL or cookie (`billingpage.go:301`). Checkout independently refuses another currency (`billingservice.go:563`).
+
+- **Ownership and bound-session authority — D/L/R/W/Y/AA/DD/EE/FF/HH–KK/NN.** `ownSession` checks provider mode, both account identifiers, intent ID, known customer and bound session ID (`billingservice.go:272`). Both create sites, recovery listings and resume paths apply ownership before accepting or expiring on provider proof. The pass judges against the account of its expected intent; its second binder read does likewise before deriving an account from response metadata.
+
+ There are deliberate exceptions to calling this predicate before acting: `ownBoundSession` and reconciliation’s `refuse()` may expire the console’s **previously validated bound ID**, after confirming the read returned that ID (`:297`, `:810`). The response supplies whether expiry is needed, not which session to expire. This is the justified bound-ID authority identified by Opus. An unowned create/listing response gives no equivalent authority and remains open at the provider.
+
+ Read or expiry failures stop review filing and produce webhook retries. Completion during expiry can cause the expiry to fail; a later completed read can file the review. Completed mismatches are not expired. Concurrent binder/reconcile filings share one intent-keyed open review (`billingstore.go:1006`), including its reference. Reconciliation also checks `intentSession`, so a subscription-first event cannot record a purchase whose completed session the binder would review.
+
+ The already-recorded binder fast path returns an existing subscription ID; it does not record a new purchase. Reconciliation then checks that subscription. Some preliminary status/mode checks return before ownership, but confer no binding, retirement or recording authority.
+
+- **Subscription refusals and retirement — II/LL/OO/PP/WW.** A correct first purchase matches the intent’s fixed quantity. Membership changes do not legitimately mutate that initial provider purchase behind the intent. Already-recorded subscription updates and renewals do not require a standing intent. A canceled recorded subscription’s late event stays on that recorded-subscription path.
+
+ Retirement lookup is account-scoped (`billingstore.go:809`), so another account’s retirement cannot settle this account’s purchase. New intent IDs are generated from 24 random bytes (`:338`); there is no supported reuse path. Replays of genuinely retired purchases settle with identity rejection and 204. Already-recorded subscriptions are exempt from this early retirement check.
+
+ Unknown terms still retry when no matching bound standing intent attributes the mismatch; this allows a newer build to understand them. Pending payment, incomplete status, missing binding, unavailable reads and invalid period data can also return 503. These are not covered by a blanket “every first-purchase failure is reviewed” claim. A different non-retired intent is refused once identity comparison is reached; malformed unknown terms or `incomplete` can return earlier.
+
+- **Lifecycle and races — S/V/BB/GG/RR–VV.** The implemented invariant is: **an open refusal review prevents ordinary intent deletion and first-purchase recording; resolution either retires that exact intent permanently or removes the hold while keeping the intent subject to ordinary validation.**
+
+ | Path | Effect on that invariant |
+ |---|---|
+ | Checkout’s previous/current-intent reads; recovery create/listing | Ownership and purchase checks precede acceptance. Expiry retirement cannot delete under an open review. |
+ | Background pass; account-deletion recovery | Use the same guarded retirement/binding paths. |
+ | Stillborn-intent cleanup | Deletes only an unready, unbound intent, before checkout creation is possible (`billingstore.go:489`). |
+ | Webhook/direct/expected-intent binding | Conditional binding cannot resurrect a deleted intent. Late review filing requires the exact intent still to stand. |
+ | First-purchase recording | Exact intent consumption and absence of an open review are required in the recording transaction. |
+ | Recorded-subscription events and renewals | Do not require re-consuming an intent. Revision checks protect concurrent state changes. |
+ | Operator `retire` | Archives and deletes only the named account/intent; settles only its pending checkout quantity events (`billingstore.go:963`). |
+ | Operator `keep` | Removes the hold without accepting provider data. Corrected purchases can record; still-wrong purchases are refused again. Outcome replay/audit has Finding 1. |
+
+ A recording conflict rolls back account state, assignment, event and entitlement writes together; quantity refresh is in the same transaction. Resolution needs no service lock for these protections. A stale pass cannot delete a replacement intent.
+
+ `keep` does not verify that correction happened. Keeping an expired session cannot activate its hand-made subscription: recording still requires a completed, matching session. Ordinary expiry cleanup can subsequently remove that intent. Keeping a still-mismatched completed purchase allows its review to recur. `retire` is therefore the appropriate default after settlement/refund.
+
+ The distinct second session and late replay at `billingcurrency_test.go:3023` now check unchanged account state and replacement binding; recording the retired completion would fail those assertions. That helper still discards HTTP status. The separate no-replacement assertion checks 204.
+
+- **History barrier, writers and rollout — E/H/N/T.** The trigger’s positive-span predicate matches `AllowanceForMonth`: active non-Free assignment, clipped by month boundaries, paid-through endpoint and the next assignment across both tables, with revision resolving same-second order (`billingstore.go:208`; `internal/plans/month.go:146`). A same-second superseded assignment contributes no span; Free/inactive successors terminate predecessors. I found neither the requested older-build version-1 escape nor an erroneous refusal of this build’s valid legacy-only month.
+
+ The production book-assignment insert copies the subscription’s `period_end`; proven renewals and quantity changes alter `effective_at`, not that meaning (`billingstore.go:624`, `:662`, `:737`). New-trigger creation precedes retirement of the old name. The retirement transaction checks that the replacement exists; failure retains the old trigger, successful restart is idempotent, and interruption can leave both temporarily but not neither (`:835`).
+
+ Startup migrates an older database before normal use. Older binaries remain an unsupported rollback once book rows exist: the barrier prevents incomplete legacy freezes, but does not make their account writers currency-aware.
+
+- **Inclusions, books, provisioning and export — F/G/J.** Console inclusions divide the pooled allowance by the charged quantity, using the fallback quote’s own quantity for unavailable Personal (`billingpage.go:395`). Current figures are exact across supported quantities; `1/3` is deliberately truncated by `formatFigure`, but no current plan/band share requires that approximation. Band-A sentences are pinned in the inclusion test.
+
+ Validation covers the matrix, half-up member prices, overflow, duplicate keys, country mappings and legacy EUR equality (`internal/plans/pricebook.go:149`, `:231`). Other-book catalogs are whole or absent and provider IDs cannot be shared (`cmd/console/billing.go:134`). Provisioning accepts at least the selected book’s 36 keys and the own-only currency-options shape (`stripe-provision.sh:424`, `:434`). The public export contains commercial definitions, not customer data or secrets.
+
+- **Quantity history — X/CC.** All three outcome-admission checks use `quantityOutcomesSQL`; startup replaces a different guard body (`billingquantityaudit.go:20`). No heal sweep remains. Summary updates preserve an existing gap and add clock-rollback evidence rather than clearing it (`billingmembership.go:462`). Pre-history and clock-rollback holds survive restart.
+
+- **Site — K/MM/QQ.** The site’s book is byte-identical to the platform export by SHA-256. Rates are rendered exactly; paid inclusions are band-scaled with integral-share and display-exactness checks. Country options and defaults come from the export. Both link parameters are always present; currency is forwarded only when explicitly chosen (`src/site.js:75`). A locale-only GBP display can lead to a console USD default because the link intentionally conveys no currency choice; the console supplies the purchase quote.
+
+ The draft marker is first-line, both gates require exactly `PRICEBOOK_ALLOW_DRAFT=1`, and the ordinary build invokes the checker. Incorrect substitutions in the tested price slots fail output comparisons; an identical literal does not establish provenance.
+
+**Claims the code contradicts**
+
+- **Resolution history records the complete decision:** it does not record `keep` versus `retire`. The “exact resolution” replay contract at `billingstore.go:887` is incomplete now that outcome changes behavior. Finding 1 applies.
+
+- **The site test has a title qualification distinguishing equality from provenance:** dispositions `:313` says this, but site `test/pricing-pricebook.test.mjs:190` still claims no amount is typed in the script, and `:193` claims a literal substitution fails. An identical literal passes.
+
+- **The resolution test covers the distinguishing exact-intent lookup history:** dispositions `:108` overstates it. The cited replay at `billingcurrency_test.go:3017` is already resolved and returns at `billingstore.go:922`, before the lookup at `:965`. The delete-only mutant is indeed redundant, but this test does not establish the lookup’s protection against an open old review while a newer intent stands.
+
+- **A matching bound intent necessarily has terms known to this build:** the service comment is corrected, but dispositions `:292` still says its terms are known. Rollback from a newer build contradicts that assertion.
+
+- **Every resolution retires the intent:** comments at `billingservice.go:294`, `:333`, `:1148` and `billingstore.go:954` retain the earlier unconditional description. `keep` now explicitly does otherwise.
+
+- **Mutation evidence:** review-three’s equivalent default-currency mutant is correctly qualified. Review-four’s successful migration test does not prove interruption ordering, and provisioning text assertions do not execute the generated verifier. Historical mutation-run counts cannot be confirmed from source.
+
+- **Test dependencies:** the fake clock starts at real time (`billingservice_test.go:134`), and authentication/webhook tests use real time. Cookie cases deliberately create fresh clients (`billingcurrency_test.go:1628`). I found no additional concrete failure caused by ordering, temporary-root choice or a shared cookie jar. The new outcome test does not inspect audit contents.
+
+**What must be verified against Stripe test mode before enablement**
+
+1. **Catalog and checkout:** verify all 36 price identities, exact amounts, currency-options expansion, tax behavior, quantities and disabled Adaptive Pricing. Complete USD/EUR/GBP purchases, including IN/GBP and Community GBP bands B/D. Session, subscription and item currencies must agree.
+
+2. **Independent identity mutations and event order:** change session and subscription purchase fields separately, delivering subscription-first, completion-first and pass-first. Valid purchases must record once; attributable mismatches must produce one referenced review and no activation.
+
+3. **Bound-session expiry:** exercise metadata contradictions, failed reads, failed expiry and completion during expiry. Verify expiry responses retain the fields required for retirement, and that an expired URL cannot accept payment. Failed reads/expiry must not falsely authorize resolution.
+
+4. **Review lifecycle:** correct a purchase while its review remains open; confirm activation stays blocked. Exercise `keep`, default `retire`, subsequent events and replacement-standing retired replays, including unknown terms and `incomplete`. Reviewed purchases must remain held; retired purchases must never activate and their events must settle. Opposite-outcome replay and audit require the local fix above.
+
+5. **Overage invoices and recovery:** create, recover, finalize and pay in every currency. Verify invoice/item/line currencies, exact allocations, floors, tax, discounts, house waivers and legacy EUR replay request shapes.
+
+6. **Quantity and subscription lifecycle:** run the quantity-fence verification, member increases, proration, renewal and cancellation. Confirm allowance dating and customer-currency restrictions after cancellation and with outstanding invoice items.
+
+**Verdict: accept with fixes.**
\ No newline at end of file
diff --git a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md
index 0e3b85a8a..fe44d155f 100644
--- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md
+++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md
@@ -105,7 +105,7 @@ The fixes are in the commit that adds this section and have not been read by a r
## Review four (at 8958392b; Opus accepted with fixes, Astra rejected)
-Reports: `OPUS-REVIEW-multi-currency-review4.md`, `CODEX-FINDINGS-multi-currency-review4.md`. Both found the third round's code changes present and no path that stores or sends money in a currency other than the purchase's (Astra marked ownership, the crafted form and the trigger migration partly closed, which its findings were about); the disputed trigger reading went Astra's way. Every code finding of either that is marked fixed below has a test that fails without it, with three qualifications review ten named: the provisioning script's checks are tested by reading the script's text, the trigger's create-then-drop order is tested by a migration that succeeds (not by one that would fail in the other order), and the resolution's delete scoped by account alone is not told from the delete by intent by any test: the exact-intent lookup earlier in the same transaction already gates the delete, so the mutant is redundant rather than wrong (a review of an old intent while a newer one stands is a distinguishing history for the lookup, which the resolution test covers, not for the delete alone); eight mutants, one per fix, were killed in a scratch copy at the time (the runs are not reproducible from this file); two items are left open and say so.
+Reports: `OPUS-REVIEW-multi-currency-review4.md`, `CODEX-FINDINGS-multi-currency-review4.md`. Both found the third round's code changes present and no path that stores or sends money in a currency other than the purchase's (Astra marked ownership, the crafted form and the trigger migration partly closed, which its findings were about); the disputed trigger reading went Astra's way. Every code finding of either that is marked fixed below has a test that fails without it, with three qualifications review ten named: the provisioning script's checks are tested by reading the script's text, the trigger's create-then-drop order is tested by a migration that succeeds (not by one that would fail in the other order), and the resolution's delete scoped by account alone is not told from the delete by intent by any test: the exact-intent lookup earlier in the same transaction already gates the delete, so the mutant is redundant rather than wrong (a review of an old intent while a newer one stands is a distinguishing history for the lookup; no test builds it, since a review is filed only while its intent stands and a newer intent cannot open beside a standing one, so the history needs a retired intent's review re-filed, which filing refuses); eight mutants, one per fix, were killed in a scratch copy at the time (the runs are not reproducible from this file); two items are left open and say so.
| Finding | Disposition | Where | Reproduction |
| --- | --- | --- | --- |
@@ -289,7 +289,7 @@ Reports: `OPUS-REVIEW-multi-currency-review14.md`, `CODEX-FINDINGS-multi-currenc
| --- | --- | --- | --- |
| Astra 1 (P2): an in-flight reconciliation could record a purchase after the operator retired its intent (the resolution took no service lock and changed no revision; the record deleted the intent without counting rows). | Fixed in the database: for a first purchase the record transaction deletes the exact standing intent and refuses (`errBillingConflict`, nothing recorded, no entitlement queued, no event recorded) when it deleted none, so the record and the intent's consumption are one transaction and no process lock is relied on. | `billingstore.go` `applyBillingStateDated(consumeIntent)`, `billingservice.go` | `TestAResolutionDuringAReconciliationLeavesNothingRecorded` (the fake's customer read pauses the reconciliation while the operator resolves; the reconciliation returns a conflict; no state, assignment or event is recorded; the retirement record stands; later events are settled) |
| Opus P3-1, Astra 2 (P3): the session `refuse()` had expired was retired by the next pass, so the intent stopped standing under its open review. | Fixed: `retireIntent` deletes nothing while a `billing_checkout_currency_refused` review of the intent is open; the resolution alone retires it, and writes the retirement record. | `billingstore.go` `retireIntent` | `TestAHandMadeSubscriptionExpiresTheOpenBoundSession` (a pass after the review; the intent stands; the resolution retires it with its record) |
-| Opus P3-2, Astra 3 (P3): a terms version edited on the subscription alone was a raw error (503 for ever, no review). | Fixed: when the subscription names the bound standing intent, an unknown terms version is the review (the intent's own terms are known here, so no later build could record it); with no such intent it stays a retry. | `billingservice.go` | `TestASubscriptionRefusedForItsPriceAloneIsReviewed` ("a terms version this build does not know", "a book the console does not sell") |
+| Opus P3-2, Astra 3 (P3): a terms version edited on the subscription alone was a raw error (503 for ever, no review). | Fixed: when the subscription names the bound standing intent, an unknown terms version is the review (the intent's terms are not compared there; an intent made by a newer build before a rollback is reviewed rather than retried, as the binder reviews its session [corrected after review sixteen]); with no such intent it stays a retry. | `billingservice.go` | `TestASubscriptionRefusedForItsPriceAloneIsReviewed` ("a terms version this build does not know", "a book the console does not sell") |
| Astra 4 (P3): the completion of a retired intent's session, and a subscription whose retirement check came after validations, were 503 for ever. | Fixed: the binder settles a completion whose intent is in `billing_retired_intents`; reconcile checks the retirement first, before any validation whose retry could never record. | `billingservice.go` | `TestRefusedPaidSessionReviewThroughTheWebhookAndItsResolution` (the retired session's completion after the resolution, no replacement standing: 204) |
| Astra 5 (P3): `refuse()` filed the review when its read of the bound session was answered with another session. | Fixed: another id is a failed read; nothing is filed or expired, and the provider tries again. | `billingservice.go` `refuse` | `TestARefusalWhoseSessionReadIsAnsweredWithAnotherFilesNothing` |
| Opus P3-3, Astra 6 (P3): the site test left bands B to D of the inclusions unpinned, matched plan slots by prefix, left Free's currencies unpinned, admitted a two-decimal rate form, and the figure-token check found 10 in 10,000. | Fixed (site `f291a9e`): every band of every inclusion recomputed from the export; every plan, band and currency slot equal to its exported amount with the plan's suffix; Free's three currencies; floors spelt as prices and rates as they are; a token not followed by a grouping comma and digits. | site `test/pricing-pricebook.test.mjs`, `copy-review/pricing/build.mjs` | |
@@ -310,4 +310,19 @@ Reports: `OPUS-REVIEW-multi-currency-review15.md`, `CODEX-FINDINGS-multi-currenc
| Opus claim 3: the early retirement check was not told from the later one. | Fixed: a retired subscription's event with unknown terms after the resolution is settled (the later check would not be reached). | tests | `TestRefusedPaidSessionReviewThroughTheWebhookAndItsResolution` |
| Astra claims: the review-four deletion-mutant explanation; the provisioning script's "12 keys"; the design and this file's lifecycle sentences. | Corrected: the delete-only mutant is redundant because the exact-intent lookup in the same transaction gates it (not because one account has one intent); the script says the book's 36 keys; the design states the guard and the two outcomes. | this file, `stripe-provision.sh`, the design | |
-Open, as both list: everything that needs Stripe test mode (both lists, with a corrected purchase under an open review and the `keep` outcome added this round), the two unreviewed system strings, the operator's door for a quantity-history gap, a listing of rejected webhook events; the site test establishes output equality per slot, not literal provenance (stated in its title's qualification). The sixteenth review reads this commit.
+Open, as both list: everything that needs Stripe test mode (both lists, with a corrected purchase under an open review and the `keep` outcome added this round), the two unreviewed system strings, the operator's door for a quantity-history gap, a listing of rejected webhook events; the site test establishes output equality per slot, not literal provenance (its title and comment say so from site `c12318d`). The sixteenth review reads this commit.
+
+## Review sixteen (reviewed `3527a066`; Opus accepted with fixes, Astra accepted with fixes)
+
+Reports: `OPUS-REVIEW-multi-currency-review16.md`, `CODEX-FINDINGS-multi-currency-review16.md` (in the tree from this commit). Both found the fifteenth round's findings closed, each with a test that fails without its fix, and both found the record's open-review clause with the `keep` outcome the right remedy; both found what `keep` left open: it was accepted for an intent that could never record (its bound session expired by the refusal), after which the pass retired that intent as an ordinary expiry, with no retirement record and the hand-made subscription neither recorded nor settled; and the outcome was recorded nowhere and absent from the replay comparison, so a replay asking for the other outcome answered success. Fixed, each with a test that fails without it; two mutants killed in a scratch copy (the keep check removed; the outcome dropped from the replay comparison). This commit also carries the operator's listing of rejected webhook events (both reviewers had named it missing since round eleven) and the provisioning script's step-2 notes aligned with the own-currency-option rule:
+
+| Finding | Disposition | Where | Reproduction |
+| --- | --- | --- | --- |
+| Opus P3-1: `keep` accepted for an intent that cannot record. | Fixed: the operator door accepts `keep` only when the intent stands, is bound, and its bound session reads back as its own, complete and carrying a subscription; otherwise 409, naming the state and saying to retire. | `billingoperator.go` `billingKeepRefused` | `TestKeepIsRefusedForAnIntentThatCannotRecord` (a hand-made subscription whose bound session the refusal expired: `keep` is 409, the review stays open; `retire` works and the audit names the outcome) |
+| Opus P3-2, Astra 1 (P3): the outcome was recorded nowhere, absent from the replay comparison, and absent from the audit. | Fixed: `billing_review_resolutions` gains an `outcome` column (added when missing; `retire` for the rows before it); the resolution writes it; a replay must repeat actor, note and outcome, else 409; the audit detail carries `outcome=`. The default is normalised to `retire` before anything is written. | `billingstore.go`, `billingoperator.go` | `TestACorrectedPurchaseWaitsForTheOperatorUnderItsReview` (the same resolution replayed with the other outcome is 409; the recorded outcome is `keep`); `TestKeepIsRefusedForAnIntentThatCannotRecord` (the audit) |
+| Opus claim 2: the later retirement check in reconcile was unreachable with a retirement. | Removed; the early check is the one. | `billingservice.go` | |
+| Opus claims 1 and 3, Astra claims: the site test's title claimed literal provenance; the design's `keep` sentence; the unknown-terms row's "known here"; the review-four row's lookup-history sentence; four "every resolution retires" comments. | Corrected: the site test's title and comment say output equality, not provenance (site `c12318d`); the design states the `keep` condition and the recorded outcome; the rows and comments say retire or keep. | this file, the design, `billingservice.go`, `billingstore.go`, site | |
+| Named missing since round eleven by both: a listing of the webhook events the console refused. | Built: `GET /api/billing/rejected` (operator only), newest first, at most 200, with the event's type, object, subscription and account where known. | `billingoperator.go` `handleAPIBillingRejected`, `server.go` | `TestRefusedPaidSessionReviewThroughTheWebhookAndItsResolution` (the settled events after a resolution are listed); `TestBillingOperatorDoorsNeedTheOperatorBit` (the member is refused) |
+| The provisioning script's step-2 notes said "no currency_options, ever". | Aligned with the rule the console and `--check` apply: never pass it; an expanded read showing the price's own currency at its own amount passes; any other entry is refused. | `stripe-provision.sh` | `internal/plans` (the script's text) |
+
+Open, as both list: everything that needs Stripe test mode (both lists, with `keep` against a corrected purchase, a hand-made subscription's `keep` refused, and `retire` against `keep` on one purchase added this round), the two unreviewed system strings, the operator's door for a quantity-history gap. The seventeenth review reads this commit.
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review16.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review16.md
new file mode 100644
index 000000000..f36255283
--- /dev/null
+++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review16.md
@@ -0,0 +1,142 @@
+# Sixteenth review of `feat/carloku-multi-currency-20261001` at `3527a066` (base `12e70d95`, delta from `abd1b76e`)
+
+**Verdict: accept with fixes.** Every finding from the fifteenth round is closed, and each has a test that fails without its fix. The record now refuses to run while a review is open, which makes the lifecycle invariant hold in the database. The new `keep` outcome brings two P3s:
+
+- `keep` is accepted for an intent that can never record. The pass then quietly retires that intent, and the subscription the operator meant to keep is never recorded and never settled.
+- The outcome the operator chose is written down nowhere.
+
+This round does not change how money is computed, sent to Stripe or stored.
+
+I read source with `git show` only, plus the site at `a7283e7`. I ran nothing and modified nothing, and my scratch copies are deleted.
+
+## The fifteenth review's findings
+
+| Finding | Closed? | Would its test pass without the fix? |
+|---|---|---|
+| **Opus P3-1 / Astra 1**: a corrected purchase recorded under an open review | **Closed.** The consuming `DELETE` now has the open-review clause (`billingstore.go:763-765`), and `consumeIntent && n != 1` gives `errBillingConflict`. The rollback takes the account row, the assignment, the entitlement queue, the event row and the quantity refresh with it. The resolution gained `keep` (`:963`); the API answers `retired`. | **No.** `TestACorrectedPurchaseWaitsForTheOperatorUnderItsReview` needs a 503 and nothing recorded; without the clause the event records with 204. If `keep` is ignored, the intent is retired: the API answers `retired:true` and the next event settles unrecorded, so the test fails. |
+| **Opus claim 1** (the design's "stands until resolved") | **Corrected.** Design `:152` now states the guard and both outcomes, and the code bears it out. | n/a |
+| **Opus claim 2** (the unknown-terms comment) | **Corrected** (`billingservice.go:849-854`). | n/a |
+| **Opus claim 3** (early vs later retirement check) | **Closed.** An unknown-terms event for a retired subscription after the resolution, with no replacement standing (`billingcurrency_test.go:2947-2958`). | **No.** Without the early check, `priceTermsFromMetadata` returns the raw error (503) before the later check at `:896` is reached. |
+| **Astra 2**: retired events retried while a replacement stood | **Closed.** The retirement check now runs for every subscription that is not the recorded one (`billingservice.go:801-807`). | **No.** In `TestARetiredSubscriptionSettlesBesideAReplacementIntent`, unknown terms return raw (503) and `incomplete` returns `errBillingPending` (503) under the old placement. |
+| **Astra 3**: the site's token check treated a comma as a left boundary | **Closed** (site `hack/token.mjs`: the left class is now `[^0-9.,]`). | **No.** The old regex accepts `250 MB` inside `1,250 MB`, so the new counterexample test fails. |
+| **Astra claims** | **Corrected:** the review-four mutant explanation, the "36 keys" line in `stripe-provision.sh:379`, and the lifecycle sentences. **Still contradicted:** the claim about literal provenance in the site test (claim 1 below). | n/a |
+
+**Where the reviewers disagreed.** Both rated the open-review record P3. They disagreed on the remedy, not the severity:
+
+- Opus wanted the resolution to report that the purchase had been recorded.
+- Astra wanted the record to refuse.
+
+The code shows Astra's remedy is the right one. With the clause in the record's own transaction, no ordering of the record, the review filing (including `ownBoundSession` from the pass, which files without the account lock) and the resolution can commit a record while the review is open, because SQLite serialises them.
+
+Opus's objection was that refusing leaves no way to accept a corrected purchase. The `keep` door answers it. Opus's own remedy, reporting after the fact, would have left entitlement granted before anyone looked.
+
+## Findings
+
+### P3-1. `keep` is accepted for an intent that can never record, and the pass then retires it with no record of the retirement
+
+- **Where:**
+ - `billingoperator.go:220-229` and `billingstore.go:963`: `keep` is accepted with no condition.
+ - `billingpass.go:210-212`: the pass's expiry branch.
+ - `billingservice.go:930-932`: reconcile needs the bound session to be `complete`.
+ - `:889-899`: the `sql.ErrNoRows` retry.
+- **Scenario:**
+ 1. A GBP intent is bound, and its session is still open.
+ 2. Someone makes a subscription by hand at the provider that names the intent, with band D. Reconcile's `refuse()` expires the bound session and files the review.
+ 3. The operator corrects the subscription's band at Stripe and resolves with `keep`, which is what the review now invites.
+ 4. The next event cannot record: the bound session is `expired`, not `complete`, so reconcile returns `errBillingPending` (503).
+ 5. On the next pass the session reads as expired and owned, and the review is resolved. `expireResolvedIntent` → `retireIntent` deletes the intent with outcome `checkout_expired` and writes no `billing_retired_intents` row.
+ 6. From then on the subscription's events find no intent and no retirement, and answer `sql.ErrNoRows` → 503 until Stripe stops retrying.
+- **Result:**
+ - The customer is billed by a live subscription with no entitlement.
+ - No review is open and no rejected-events row exists.
+ - The API answered `resolved:true, retired:false`.
+
+ Before `keep` existed, the only outcome was `retire`, which settles the same subscription visibly. The same pointless `keep` on an unbound intent (a review filed from the listing) merely re-files the review on the next pass, which is harmless.
+- **Smallest acceptable fix:** in the handler, for `keep`, read the intent's bound session by its bound id. Accept `keep` only if:
+ - `ownSession` holds,
+ - the status is `complete`, and
+ - a subscription is set.
+
+ Otherwise answer 409 ("keep needs a completed session; retire"). Add a test: a hand-made subscription, its session expired by `refuse()`, then `keep`, which must be refused.
+
+### P3-2. The outcome of a resolution is recorded nowhere
+
+- **Where:**
+ - `billingoperator.go:232`: the audit's detail is `req.Reason` only.
+ - `billing_review_resolutions` has no outcome column.
+ - The idempotent replay (`billingstore.go:930-942`) compares only the actor and the note.
+- **Scenario:** an operator resolves with `keep`. Later nothing in the audit or the resolution history shows that the intent was kept rather than retired. The only trace is a missing `billing_retired_intents` row, which looks the same as "nothing was there".
+
+ Also, a replay with the same actor and note but the other outcome answers 200 `resolved`. The `retired` field does reflect the real state, so this is honest but untraceable.
+- **Smallest acceptable fix:** put the outcome in the audit detail (for example `reason+" outcome="+outcome`). Preferably also store it in the resolution row and compare it on replay.
+
+## The added sections
+
+**UU. The record's open-review clause and `keep`.**
+
+- **No purchase records under an open review, by any path.** `applyBillingStateDated(consumeIntent=true)` from reconcile is the only production recorder of a first purchase, and its delete carries the clause.
+ - Renewals and recorded subscriptions have `consumeIntent=false`. The intent their metadata names was consumed at their own first record, so the clause cannot refuse them wrongly.
+ - The binder's fast path only returns state that is already recorded.
+ - The pass's complete branch and the success URL both reach the same reconcile.
+ - A replayed event id returns at the event-seen check.
+- **The refused corrected event is a 503.** The pass then re-reconciles after `keep` (`billingpass.go:204-208`), so recording does not depend on Stripe's retry window.
+- **`keep` cannot record anything wrong.** The next event still meets `ownBoundSession`, `intentSession`, price, currency, quantity and payment. A session paid in the wrong currency re-files the review: `fileBillingReviewTx` reopens it with `occurrences+1`.
+- **`keep` can leave an intent that should not stand only in P3-1.**
+- **`retire` is still the right default.** It is conservative, and it is what an older operator client gets.
+- **The API:**
+ - 400 for any other outcome, before the account lookup.
+ - 200 with `retired` for a checkout refusal.
+ - 500 if the read-back fails after a committed resolution; a retry with the same body then answers 200.
+ - `keep` on another reason is accepted and ignored.
+- **The audit omits the outcome** (P3-2).
+
+**VV. The lifecycle invariant, restated from the code.** An intent under an open refusal review stands until the operator resolves it. A retired intent never records, and its events settle with 204. The paths:
+
+| Path | Effect on an intent | Holds? |
+|---|---|---|
+| `beginBillingCheckout` (`billingstore.go:379`) | `INSERT OR IGNORE`; never replaces a standing intent | Yes |
+| `saveBillingCheckout`, the binder's bind `UPDATE` | Bind only when the session is unset or the same | Yes |
+| Click: previous or resumed intent; `refuseSession`; pass expiry → `retireIntent` (`:454-456`) | Provider proof plus the open-review clause | Yes |
+| `discardUnreadyIntent` | Only an intent with no session and `customer_ready=0`; such an intent can never carry a review | Yes |
+| `resolveIntentForDeletion` | Goes through the two rows above | Yes |
+| Record (`:763`) | Exact intent, open-review clause, `n==1` | Yes |
+| Resolution `retire` | The one retirement under review, with its record | Yes |
+| Resolution `keep` | Leaves the intent standing; after that it is an ordinary intent | Yes, but P3-1 |
+| `fileCheckoutRefusedReview` | Files only while the intent stands | Yes |
+| Retired checks (reconcile `:801`; binder `:1119`, and with a replacement standing `ownSession` refuses) | Settle with 204 | Yes |
+
+Nothing breaks the invariant. P3-1 is outside it: after a `keep` the intent is no longer under review, and its later retirement is an ordinary expiry. The operator's API comment "an intent is never gone at its resolution" holds: every delete of an intent is guarded or cannot meet a review.
+
+**WW. The retirement check before validation.** No legitimate first purchase is settled wrongly by it:
+
+- Intent ids are 24 random bytes and `UNIQUE`, so an id is never reused.
+- The lookup is keyed by the account the subscription itself names, so another account's retirement never matches.
+- A retired intent was never consumed, so its subscription was never recorded. A recorded subscription skips the check (`state.Subscription == id`), and so do its replays.
+- A subscription naming account B and an intent of account A falls through to the identity refusal.
+
+**Unchanged.** The delta touches no money computation, Stripe parameter, signature, trigger or page. Sections A–TT stand as round fifteen traced them. The GBP purchase from `?country=IN&currency=gbp` still has no conversion or re-pricing anywhere in its chain.
+
+The site's left boundary now excludes `,` and `.`. A figure at the end of a sentence ("… 10.") is refused by `(?![0-9.])`. That is a false refusal, the safe direction, and it was already there before this round.
+
+## Claims the code contradicts
+
+1. **Dispositions, review fifteen, "Open" line.** It says the site test's lack of literal provenance is "stated in its title's qualification". It is not stated:
+ - The title at site `test/pricing-pricebook.test.mjs:190` still reads "no amount anywhere is typed in the script".
+ - Its comment still says a literal "fails here, whatever other amount it happens to equal".
+
+ A literal equal to the slot's current value passes. Fix the title and comment, or the disposition.
+2. **`billingservice.go:889-899`.** The later retirement check, and its comment "No intent stands. A subscription naming an intent the operator retired … is settled", can no longer be reached with `retired=true`, because the early check at `:801` has already returned. Remove the check, or say that it is redundant.
+3. **Design `:152` and `billingstore.go:863-867`.** The design says `keep` is "for a purchase corrected at the provider to record". A kept intent whose bound session is not complete can never record (P3-1). The store comment's "or it expires as any other" is accurate but understates what follows: no retirement record, and a paid subscription left neither recorded nor settled.
+
+I found no contradiction in this round's three mutation claims. Each mutant is killed by the test traced above.
+
+## Verify in Stripe test mode before enabling
+
+All earlier lists stand. Add:
+
+1. **A refused subscription, corrected at the provider while the review is open.** Expect a 503 per delivery and nothing recorded. Then `keep`: confirm the next delivery records, or the pass does after Stripe has stopped retrying.
+2. **A hand-made subscription whose open session the console expired, then `keep`.** It should be refused after P3-1's fix. Before the fix, observe the pass retiring the intent and the subscription's events answering 503 until Stripe stops.
+3. **A retired intent's subscription with a replacement intent standing.** Re-deliver `customer.subscription.updated` with unknown terms and with `incomplete`: 204 each, and the replacement untouched.
+4. **`retire` versus `keep` on the same refused purchase.** Compare what Stripe still shows and what the account is granted.
+
+**Verdict: accept with fixes** (P3-1, P3-2 and the three claims).
diff --git a/implementation/carloku-paid-launch/stripe-provision.sh b/implementation/carloku-paid-launch/stripe-provision.sh
index 132a44bbc..f21fbd409 100755
--- a/implementation/carloku-paid-launch/stripe-provision.sh
+++ b/implementation/carloku-paid-launch/stripe-provision.sh
@@ -252,10 +252,12 @@ print_provision() {
-d "metadata[terms_version]=$TERMS"
done
- section "Step 2. Thirty-six prices (one per plan, band and currency; exclusive tax, yearly, licensed, per unit, no currency_options)"
+ section "Step 2. Thirty-six prices (one per plan, band and currency; exclusive tax, yearly, licensed, per unit, no currency option but the price's own)"
note "Prices are immutable. A mistake is fixed by archiving the price and making a new one under a new price book version."
- note "Each price is in one currency and no other: no currency_options, ever. The console creates every Checkout Session with"
- note "adaptive_pricing disabled and refuses a session in any currency but the price's."
+ note "Each price is in one currency and no other: never pass currency_options. The console and the --check below accept an"
+ note "expanded read that shows an entry under the price's own currency at its own amount (Stripe may answer so) and refuse"
+ note "any other entry. The console creates every Checkout Session with adaptive_pricing disabled and refuses a session in"
+ note "any currency but the price's."
for plan in "${PLANS[@]}"; do
for band in "${BANDS[@]}"; do
for currency in "${CURRENCIES[@]}"; do
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go
index b05a63e8e..31caa8236 100644
--- a/internal/console/billingcurrency_test.go
+++ b/internal/console/billingcurrency_test.go
@@ -2944,6 +2944,28 @@ func TestRefusedPaidSessionReviewThroughTheWebhookAndItsResolution(t *testing.T)
if code := f.webhookPost(t, fx, "evt_after_resolution_done", "checkout.session.completed", intent.SessionID); code != http.StatusNoContent {
t.Fatalf("the retired session's completion after the resolution: %d; want it settled with 204, not retried", code)
}
+ // What was turned away is listed for the operator (the door both
+ // reviews asked for): the events settled since the resolution, with
+ // their objects, newest first.
+ token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-rejected")
+ status, body := getJSON(t, fx.ts.URL+"/api/billing/rejected", token)
+ rejected, _ := body["rejected"].([]any)
+ if status != http.StatusOK || len(rejected) < 3 {
+ t.Fatalf("the rejected events listing: %d %v; want the settled events listed", status, body)
+ }
+ seen := map[string]bool{}
+ for _, r := range rejected {
+ row, _ := r.(map[string]any)
+ seen[row["id"].(string)] = true
+ if row["reason"] != "billing_identity_mismatch" || row["type"] == "" || row["object"] == "" {
+ t.Fatalf("a rejected event row: %v", row)
+ }
+ }
+ for _, id := range []string{"evt_after_resolution_0", "evt_after_resolution_1", "evt_after_resolution_done"} {
+ if !seen[id] {
+ t.Fatalf("rejected events listed %v; want %s among them", seen, id)
+ }
+ }
// A retired subscription whose terms were then edited to a version
// this build does not know: settled before that validation (review
// fifteen: the early retirement check, told from the later one).
@@ -3929,10 +3951,58 @@ func TestACorrectedPurchaseWaitsForTheOperatorUnderItsReview(t *testing.T) {
if _, err := fx.st.billingCheckoutIntent(acct.ID); err == nil {
t.Fatal("the intent stands after the record consumed it")
}
- // An outcome that is neither is refused.
+ // An outcome that is neither is refused; the same resolution replayed
+ // with the other outcome is refused too, and the outcome is in the
+ // resolution's record.
if status, _ := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", map[string]any{"account": acct.Sqid, "subscription": intent.ID, "reason": billingCheckoutRefusedReason, "occurrences": 1, "note": "x", "outcome": "shrug"}, token); status != http.StatusBadRequest {
t.Fatalf("an unknown outcome: %d; want 400", status)
}
+ if status, _ := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", map[string]any{"account": acct.Sqid, "subscription": intent.ID, "reason": billingCheckoutRefusedReason, "occurrences": 1, "note": "corrected at the provider", "outcome": "retire"}, token); status != http.StatusConflict {
+ t.Fatalf("the same resolution replayed with the other outcome: %d; want 409", status)
+ }
+ var recorded string
+ if err := fx.st.db.QueryRow(`SELECT outcome FROM billing_review_resolutions WHERE account_id=? AND subscription=? AND occurrence=1`, acct.ID, intent.ID).Scan(&recorded); err != nil || recorded != "keep" {
+ t.Fatalf("the resolution's recorded outcome: %q %v; want keep", recorded, err)
+ }
+}
+
+// keep is refused for an intent that could never record: a hand-made
+// subscription's review whose bound session the console expired. The
+// operator is told to retire. (Review sixteen, Opus P3-1.)
+func TestKeepIsRefusedForAnIntentThatCannotRecord(t *testing.T) {
+ fx, f, acct := newCurrencyFixture(t)
+ f.customerCountry = "GB"
+ if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil || dest == "" {
+ t.Fatalf("first click: dest %q err %v", dest, err)
+ }
+ intent, _ := fx.st.billingCheckoutIntent(acct.ID)
+ f.mu.Lock()
+ f.subscriptionPrice = currencyPriceID(plans.PlanPersonal, plans.BandD, plans.GBP)
+ f.mu.Unlock()
+ if code := f.webhookPost(t, fx, "evt_keep_hand_made", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent {
+ t.Fatalf("the hand-made subscription event: %d", code)
+ }
+ if f.sessionStatus != "expired" {
+ t.Fatalf("fixture: the bound session is %s; want it expired by the refusal", f.sessionStatus)
+ }
+ token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-keep")
+ status, body := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", map[string]any{"account": acct.Sqid, "subscription": intent.ID, "reason": billingCheckoutRefusedReason, "occurrences": 1, "note": "corrected", "outcome": "keep"}, token)
+ if status != http.StatusConflict {
+ t.Fatalf("keep on an intent whose session is expired: %d %v; want 409", status, body)
+ }
+ var open int
+ if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0`, acct.ID, intent.ID, billingCheckoutRefusedReason).Scan(&open); err != nil || open != 1 {
+ t.Fatalf("%d open reviews (%v); want the review still open", open, err)
+ }
+ // retire works, and the audit names the outcome.
+ status, body = postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", map[string]any{"account": acct.Sqid, "subscription": intent.ID, "reason": billingCheckoutRefusedReason, "occurrences": 1, "note": "refunded"}, token)
+ if status != http.StatusOK || body["retired"] != true {
+ t.Fatalf("retire: %d %v", status, body)
+ }
+ var detail string
+ if err := fx.st.db.QueryRow(`SELECT detail FROM audit WHERE action='billing-review-resolve' ORDER BY id DESC LIMIT 1`).Scan(&detail); err != nil || !strings.Contains(detail, "outcome=retire") {
+ t.Fatalf("the audit detail: %q %v; want the outcome in it", detail, err)
+ }
}
// A retired intent's subscription events are settled whether or not a
diff --git a/internal/console/billingoperator.go b/internal/console/billingoperator.go
index 31bd67cd7..37294af3c 100644
--- a/internal/console/billingoperator.go
+++ b/internal/console/billingoperator.go
@@ -1,10 +1,12 @@
package console
import (
+ "context"
"database/sql"
"errors"
"log"
"net/http"
+ "strconv"
"strings"
"time"
)
@@ -22,6 +24,7 @@ import (
//
// GET /api/billing/reviews open review holds
// POST /api/billing/reviews/resolve clear one hold, conditional on its occurrence count
+// GET /api/billing/rejected webhook events refused as identity mismatches, newest first
// GET /api/billing/house house accounts
// POST /api/billing/house set or clear the house marker
//
@@ -200,6 +203,84 @@ func (s *Server) handleAPIBillingReviews(w http.ResponseWriter, r *http.Request)
writeJSON(w, http.StatusOK, map[string]any{"reviews": out})
}
+// billingRejectedEventRow is one webhook event the console refused as an
+// identity mismatch, for the operator: what reached the console and was
+// recorded nowhere else (the intent-keyed reviews are what asks for a
+// decision; this is the record of what was turned away, within the
+// provider's own event window).
+type billingRejectedEventRow struct {
+ ID string `json:"id"`
+ Type string `json:"type"`
+ Reason string `json:"reason"`
+ Object string `json:"object"`
+ Subscription string `json:"subscription,omitempty"`
+ Account string `json:"account,omitempty"`
+ ReceivedAt int64 `json:"received_at"`
+}
+
+// billingKeepRefused says why a checkout refusal cannot be resolved with
+// keep: the intent is gone, unbound, or its bound session is not its own,
+// not complete, or carries no subscription. An empty answer means keep is
+// allowed.
+func (s *Server) billingKeepRefused(ctx context.Context, acct *Account, intentID string) (string, error) {
+ intent, err := s.store.billingCheckoutIntent(acct.ID)
+ if errors.Is(err, sql.ErrNoRows) || (err == nil && intent.ID != intentID) {
+ return "no such intent stands", nil
+ }
+ if err != nil {
+ return "", err
+ }
+ if intent.SessionID == "" {
+ return "the intent has no session", nil
+ }
+ session, err := s.billing.client.GetCheckout(ctx, intent.SessionID)
+ if err != nil {
+ return "", err
+ }
+ if !s.billing.ownSession(session, intent, acct) {
+ return "the session read back is not the intent's own", nil
+ }
+ if session.Status != "complete" || session.SubscriptionID == "" {
+ return "the session is " + session.Status + " with subscription " + strconv.Quote(session.SubscriptionID), nil
+ }
+ return "", nil
+}
+
+// handleAPIBillingRejected lists the webhook events the console refused,
+// newest first, at most 200.
+func (s *Server) handleAPIBillingRejected(w http.ResponseWriter, r *http.Request) {
+ if _, ok := s.billingOperator(w, r); !ok {
+ return
+ }
+ rows, err := s.store.db.Query(`SELECT id,event_type,reason,object_id,subscription_id,account_id,received_at FROM billing_rejected_events ORDER BY received_at DESC, id LIMIT 200`)
+ if err != nil {
+ log.Printf("console: api billing rejected: %v", err)
+ jsonError(w, http.StatusInternalServerError, "could not read rejected events. try again")
+ return
+ }
+ defer rows.Close()
+ out := make([]billingRejectedEventRow, 0)
+ for rows.Next() {
+ var x billingRejectedEventRow
+ var accountID int64
+ if err := rows.Scan(&x.ID, &x.Type, &x.Reason, &x.Object, &x.Subscription, &accountID, &x.ReceivedAt); err != nil {
+ jsonError(w, http.StatusInternalServerError, "could not read rejected events. try again")
+ return
+ }
+ if accountID != 0 {
+ if acct, err := s.store.AccountByID(accountID); err == nil && acct != nil {
+ x.Account = acct.Sqid
+ }
+ }
+ out = append(out, x)
+ }
+ if err := rows.Err(); err != nil {
+ jsonError(w, http.StatusInternalServerError, "could not read rejected events. try again")
+ return
+ }
+ writeJSON(w, http.StatusOK, map[string]any{"rejected": out})
+}
+
func (s *Server) handleAPIBillingReviewResolve(w http.ResponseWriter, r *http.Request) {
addr, ok := s.billingOperator(w, r)
if !ok {
@@ -226,10 +307,33 @@ func (s *Server) handleAPIBillingReviewResolve(w http.ResponseWriter, r *http.Re
jsonError(w, http.StatusNotFound, "no such billing review")
return
}
- err = s.store.resolveBillingReview(billingReviewResolution{AccountID: acct.ID, Subscription: req.Subscription, Reason: req.Reason, Occurrence: req.Occurrences, Actor: addr, Note: strings.TrimSpace(req.Note), Outcome: req.Outcome}, time.Now())
+ if req.Outcome == "keep" && req.Reason == billingCheckoutRefusedReason {
+ // Keeping an intent is for a purchase that can still record: the
+ // intent's bound session, read now, must be its own, complete and
+ // carrying a subscription. Any other kept intent could never
+ // record and would later expire with no retirement record, its
+ // subscription neither recorded nor settled.
+ if why, err := s.billingKeepRefused(r.Context(), acct, req.Subscription); err != nil {
+ log.Printf("console: api billing review keep: %v", err)
+ jsonError(w, http.StatusServiceUnavailable, "the intent's session could not be read. try again")
+ return
+ } else if why != "" {
+ jsonError(w, http.StatusConflict, "keep needs a completed session with a subscription ("+why+"); resolve with retire")
+ return
+ }
+ }
+ outcome := req.Outcome
+ if outcome == "" {
+ outcome = "retire"
+ }
+ err = s.store.resolveBillingReview(billingReviewResolution{AccountID: acct.ID, Subscription: req.Subscription, Reason: req.Reason, Occurrence: req.Occurrences, Actor: addr, Note: strings.TrimSpace(req.Note), Outcome: outcome}, time.Now())
switch {
case err == nil:
- s.audit(acct, "", addr, "billing-review-resolve", req.Reason)
+ detail := req.Reason
+ if req.Reason == billingCheckoutRefusedReason {
+ detail += " outcome=" + outcome
+ }
+ s.audit(acct, "", addr, "billing-review-resolve", detail)
out := map[string]any{"account": acct.Sqid, "subscription": req.Subscription, "reason": req.Reason, "resolved": true}
if req.Reason == billingCheckoutRefusedReason {
// What the resolution did about the intent the review was keyed
diff --git a/internal/console/billingoperator_test.go b/internal/console/billingoperator_test.go
index 7aea51e13..6a40c3f7d 100644
--- a/internal/console/billingoperator_test.go
+++ b/internal/console/billingoperator_test.go
@@ -53,7 +53,7 @@ func billingOperatorToken(t *testing.T, fx *consoleFixture, address, code string
func TestBillingOperatorDoorsAnswerOnlyOperators(t *testing.T) {
fx := newConsoleFixture(t)
member := cliToken(t, fx, "paul@keymail.dev", "code-billing-op-member")
- for _, u := range []string{"/api/billing/reviews", "/api/billing/house"} {
+ for _, u := range []string{"/api/billing/reviews", "/api/billing/house", "/api/billing/rejected"} {
if status := rawStatus(t, http.MethodGet, fx.ts.URL+u, nil, member); status != http.StatusNotFound {
t.Errorf("GET %s as a non-operator = %d, want 404", u, status)
}
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go
index e40410895..5582ef846 100644
--- a/internal/console/billingservice.go
+++ b/internal/console/billingservice.go
@@ -888,16 +888,9 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s
return errBillingIdentity
}
if pending == nil {
- // No intent stands. A subscription naming an intent the
- // operator retired (a refusal resolved) is settled for good:
- // refused, once, and every later event of it the same, never a
- // retry. Anything else is not settled (the provider tries
- // again).
- if retired, err := s.store.billingIntentRetired(acct.ID, sub.Metadata["checkout_intent"]); err != nil {
- return err
- } else if retired {
- return errBillingIdentity
- }
+ // No intent stands, and the subscription names no retired one
+ // (that was settled above): not settled, the provider tries
+ // again.
return sql.ErrNoRows
}
intent := *pending
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go
index e0dc54a8b..d2e040d82 100644
--- a/internal/console/billingstore.go
+++ b/internal/console/billingstore.go
@@ -224,6 +224,9 @@ func (s *Store) migrateBilling() error {
return err
}
}
+ if err := s.addColumnIfMissing("billing_review_resolutions", "outcome", "TEXT NOT NULL DEFAULT 'retire'"); err != nil {
+ return err
+ }
if err := s.retireEarlierBarrierTrigger(); err != nil {
return err
}
@@ -861,12 +864,24 @@ type billingReviewResolution struct {
// short technical reason code, never provider or customer data.
Actor, Note string
// Outcome, for a checkout refusal: "retire" (the default, the money is
- // settled and the intent is retired) or "keep" (the purchase was
- // corrected at the provider; the intent stands and the next event
- // records it, or it expires as any other).
+ // settled and the intent is retired, with its record) or "keep" (the
+ // purchase was corrected at the provider; the intent stands and the
+ // next event records it). The operator door accepts keep only for an
+ // intent whose bound session is complete with a subscription, since
+ // any other can never record and would later expire with no
+ // retirement record. The outcome is written with the resolution and
+ // is part of what a replay must repeat.
Outcome string
}
+// outcome is the resolution's outcome, "retire" when none was named.
+func (r billingReviewResolution) outcome() string {
+ if r.Outcome == "" {
+ return "retire"
+ }
+ return r.Outcome
+}
+
const (
billingReviewActorMax = 320
billingReviewNoteMax = 256
@@ -920,15 +935,17 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e
return errBillingReviewStale
}
if resolvedAt != 0 {
- var actor, note string
- err := tx.QueryRow(`SELECT actor,note FROM billing_review_resolutions WHERE account_id=? AND subscription=? AND reason=? AND occurrence=?`,
- r.AccountID, r.Subscription, r.Reason, r.Occurrence).Scan(&actor, &note)
+ var actor, note, outcome string
+ err := tx.QueryRow(`SELECT actor,note,outcome FROM billing_review_resolutions WHERE account_id=? AND subscription=? AND reason=? AND occurrence=?`,
+ r.AccountID, r.Subscription, r.Reason, r.Occurrence).Scan(&actor, &note, &outcome)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
return err
}
// No audit row means the row was resolved outside this method
// (before the history existed); that is not this resolution either.
- if err != nil || actor != r.Actor || note != r.Note {
+ // The same resolution is the same actor, note and outcome: a
+ // replay asking for the other outcome is refused, not granted.
+ if err != nil || actor != r.Actor || note != r.Note || outcome != r.outcome() {
return errBillingReviewResolved
}
return nil
@@ -947,20 +964,22 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e
if n != 1 {
return errBillingReviewStale
}
- if _, err := tx.Exec(`INSERT INTO billing_review_resolutions(account_id,subscription,reason,occurrence,last_seen_at,actor,note,resolved_at) VALUES(?,?,?,?,?,?,?,?)`,
- r.AccountID, r.Subscription, r.Reason, r.Occurrence, lastSeen, r.Actor, r.Note, at); err != nil {
+ if _, err := tx.Exec(`INSERT INTO billing_review_resolutions(account_id,subscription,reason,occurrence,last_seen_at,actor,note,resolved_at,outcome) VALUES(?,?,?,?,?,?,?,?,?)`,
+ r.AccountID, r.Subscription, r.Reason, r.Occurrence, lastSeen, r.Actor, r.Note, at, r.outcome()); err != nil {
return err
}
- // Resolving the review of a session paid in the wrong currency is the
- // operator saying the money is settled. The review is keyed by the
- // intent (its subscription field holds the intent id), and exactly
- // that intent, which was left standing so no second purchase opened
- // beside the refused one, is retired in the same transaction: its
+ // Resolving the review of a session paid in the wrong currency with
+ // the outcome retire is the operator saying the money is settled. The
+ // review is keyed by the intent (its subscription field holds the
+ // intent id), and exactly that intent, which was left standing so no
+ // second purchase opened beside the refused one, is retired in the
+ // same transaction (with keep it stands, for the corrected purchase
+ // to record): its
// quantity events are closed as retireIntent closes them, what it was
// is recorded, and the row goes, so the next pass finds nothing to
// bind and the review does not come back. An intent that is not that
// one (a later purchase's) is left alone.
- if r.Reason == billingCheckoutRefusedReason && r.Outcome != "keep" {
+ if r.Reason == billingCheckoutRefusedReason && r.outcome() == "retire" {
var sessionID, plan, band, currency, book string
err := tx.QueryRow(`SELECT session_id,plan,band,currency,price_book FROM billing_checkout_intents WHERE account_id=? AND id=?`, r.AccountID, r.Subscription).Scan(&sessionID, &plan, &band, &currency, &book)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
diff --git a/internal/console/server.go b/internal/console/server.go
index e9057011b..9c5d19e0b 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -1153,6 +1153,7 @@ func NewServer(cfg Config, st *Store) (*Server, error) {
s.mux.HandleFunc("/billing/webhook", s.handleBillingWebhook)
s.mux.HandleFunc("GET /api/billing/reviews", s.handleAPIBillingReviews)
s.mux.HandleFunc("POST /api/billing/reviews/resolve", s.handleAPIBillingReviewResolve)
+ s.mux.HandleFunc("GET /api/billing/rejected", s.handleAPIBillingRejected)
s.mux.HandleFunc("GET /api/billing/house", s.handleAPIBillingHouseList)
s.mux.HandleFunc("POST /api/billing/house", s.handleAPIBillingHouseSet)
s.mux.HandleFunc("POST /a/{sqid}/errors/level", s.handleAccountErrorLevel)