amadan
Multi-currency: the fourth review's findings
Ownership of a session includes the intent's customer and, once bound, the bound session's id; nothing is expired, offered, bound or recorded on a session that fails it. The checkout door compares the quote with what would be bought, the chosen currency or else the posted country's own, so no arrangement of the form's fields buys a price the page did not show; a currency field that is not exactly a currency sold is a 400 before the quote is looked at, on both doors, and the page says when its URL names one. A bound completion on other price terms, metadata included, files the one review, through the pass, the webhook or a direct binding. The barrier trigger spans an assignment to the next assignment's instant, its paid-through instant or the month's end, whichever is first, which is the month allowance's own rule; it has a new name and the old, wider one is dropped at migration, so a database an earlier build opened gets it. An account that bought before price books is offered its euro alone. The provisioning check counts the book's own prices and its test reads the generated checks. The design, the dispositions and the review reports record all of it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
13 files changed,
+708
−57
docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md+4 −4docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review4.md+168 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+29 −2docs/superpowers/specs/OPUS-REVIEW-multi-currency-review4.md+121 −0implementation/carloku-paid-launch/stripe-provision.sh+2 −2internal/console/billingcurrency_test.go+246 −1internal/console/billinghandlers.go+46 −18internal/console/billingpage.go+20 −5internal/console/billingservice.go+23 −12internal/console/billingservice_test.go+6 −0internal/console/billingstore.go+25 −13internal/console/templates/billing.html+1 −0internal/plans/provision_test.go+17 −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 41b0cec88..713e21771 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 |
| @@ -144,10 +144,10 @@ func PublicPriceBook(book string) ([]byte, error) // the export below, byte-stab |
| ## Console data contract |
| -New columns, both `TEXT NOT NULL DEFAULT ''`, on `billing_checkout_intents` and `billing_accounts`: `currency`, `price_book`. Empty in both means a legacy row. Assignments of a purchase priced from a book go in a new table, `billing_book_assignments`, with the same columns plus `currency` and `price_book` required non-empty; `billing_assignments` keeps only legacy assignments. The split is for rollback: a build from before books reads `billing_assignments` alone, so it finds no history for a book-priced purchase. For an account with both kinds of history it would find the legacy half, so a database trigger (`billing_months_no_legacy_calc_over_book_history`) refuses to freeze a calculation whose top-level `v` is not 2 (read with `json_extract`) for any month in which an active book-priced assignment of the account was paid for (began before the month ended, paid through past its start). A book assignment that never counted for a month, a purchase cancelled before its first reconciliation, say, is left out of that, so this build's own version 1 for a legacy-only month after it still goes in. The environment and the binary change together, and no binary from before books runs against a database with a book-priced row (roll note in the dispositions). |
| +New columns, both `TEXT NOT NULL DEFAULT ''`, on `billing_checkout_intents` and `billing_accounts`: `currency`, `price_book`. Empty in both means a legacy row. Assignments of a purchase priced from a book go in a new table, `billing_book_assignments`, with the same columns plus `currency` and `price_book` required non-empty; `billing_assignments` keeps only legacy assignments. The split is for rollback: a build from before books reads `billing_assignments` alone, so it finds no history for a book-priced purchase. For an account with both kinds of history it would find the legacy half, so a database trigger (`billing_months_no_legacy_calc_over_paid_book_history`; the earlier, wider `..._over_book_history` is dropped at migration) refuses to freeze a calculation whose top-level `v` is not 2 (read with `json_extract`) for any month into which an active book-priced assignment of the account was priced: its span, from its effective instant to the earliest of the next assignment's effective instant (by time then revision, over both tables), its paid-through instant and the month's end, has length inside the month, which is `AllowanceForMonth`'s own rule. A book assignment that never counted for a month (cancelled before its first reconciliation, or superseded in the same second) is left out, so this build's own version 1 for a legacy-only month after it still goes in. The environment and the binary change together, and no binary from before books runs against a database with a book-priced row (roll note in the dispositions). |
| -- **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 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 the buyer has not seen. 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 chosen currency that is not `usd`, `eur` or `gbp`: 400, nothing is created. 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. 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. |
| +- **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 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 the buyer has not seen. 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. |
| - **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. |
| @@ -210,7 +210,7 @@ The billing page (`billingpage.go`, built on the layout branch's template): `s.b |
| - `internal/console/billingpage.go`, `billinghandlers.go` and `templates/billing.html`: one form with the country and currency controls in the approved words, prices and per-member inclusions at the quoted country's band in the page's currency, the quoted-price rule and the quote door, the account's held currency offered alone. |
| - `cmd/console/billing.go`: `CONSOLE_BILLING_PRICE_BOOK` is required; the 36 prices of the configured book, beside which the twelve legacy keys (all or none) and the whole catalog of any other book this build has may stand; live mode refuses a draft book. |
| -With no book configured (`BillingConfig.PriceBook` empty, which only tests and the demo do) the console behaves as it did: EUR, the terms' own figures, rows with no currency. Every billing test from before this change passes in that mode, unedited except for call signatures and fakes that now report a currency. |
| +With no book configured (`BillingConfig.PriceBook` empty, which only the legacy-mode tests do; the review demo configures the current book) the console behaves as it did: EUR, the terms' own figures, rows with no currency. Every billing test from before this change passes in that mode, unedited except for call signatures and fakes that now report a currency. |
| `startBillingCheckout` gained a `currency` argument before `now`. The site (carloku-paid-pricing, `copy-review/pricing/build.mjs` and `src/site.js`) renders every amount from the export: the static page is the default currency at band A, the controls apply the currency and the country's band, and the signup links carry both. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review4.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review4.md| new file mode 100644 |
| index 000000000..1afece9fa |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review4.md |
| @@ -0,0 +1,168 @@ |
| +All platform references are at `8958392b`; pricing-site references are at `2fc6388`. Test outcomes below are deductions from source. Nothing was modified or executed beyond read-only inspection. |
| + |
| +**Third-review findings** |
| + |
| +| Opus finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| P2-1: country default and conflicting forms | **Closed for ordinary submissions; incomplete for crafted fields.** One form now redirects changed selections. See finding 2. | **No** for removing the redirect. **Yes** for passing the displayed default as explicit currency in the tested, untampered forms. | |
| +| P2-2: rounded site overage rates | **Closed.** Site `build.mjs:48` formats exact micro amounts. | **No.** Site `pricing-pricebook.test.mjs:99` checks exact examples and parses every displayed rate back. | |
| +| P3-1: returning buyer offered the wrong currency | **Closed for book purchases; still open for legacy EUR purchases.** See finding 7. | **No** for removing the held-GBP branch; the test at `billingcurrency_test.go:1916` fails. It does not cover legacy purchases. | |
| +| P3-2: empty country revives cookie | **Code closed**, `billingpage.go:215`, `billingcurrency.go:55`. | **Yes** for restoring only the page’s empty-country fallback. The named page tests supply a currency cookie, not a country cookie. Removing cookie deletion itself would fail. | |
| +| P3-3: trigger reads version by substring | **Closed on newly migrated databases.** Existing triggers remain unchanged; finding 5. | **No.** The nested-version case at `billingcurrency_test.go:2112` distinguishes JSON extraction from substring matching. | |
| +| P3-4: site draft-book gate | **Implemented, with an incorrectly permissive override.** See finding 10. | **No** for removing the build/check refusals or marker. **Yes** for accepting `"0"` as authorization: that case is absent. | |
| +| P3-5: smaller site/provisioning problems | **Mixed.** Country options, exact resource scaling and current figures are corrected. Provisioning still rejects larger catalogs globally; finding 9. | **Yes** for several individual reversions: removing the site’s consistency guards, restoring the duplicated country-default list, restoring provisioning’s skip, or reverting its verifier rules. Current fixtures do not distinguish these. | |
| + |
| +| Astra finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| 1: foreign session expired before ownership check | **Original wrong-intent case closed; ownership validation incomplete.** See finding 1. | **No** for removing the initial-create ownership guard. The test at `billingcurrency_test.go:2038` does not exercise the replay-create guard independently or vary customer identity. | |
| +| 2: visible controls disagree with checkout | **Ordinary interaction closed; crafted submissions remain unsafe against the stated quote rule.** Finding 2. | **No** for removing quote-before-purchase. Tests leave hidden fields intact. | |
| +| 3: trigger rejects legitimate legacy calculation | **Reported canceled-before-first-reconcile case closed; another legitimate history remains blocked.** Finding 4. | **No.** Restoring the previous broad predicate fails `billingcurrency_test.go:2125`. | |
| +| 4: bound wrong-currency completion gets no review | **Reported currency-only mismatch closed; matching wrong-currency metadata still bypasses review.** Finding 6. | **No** for removing the new review call. **Yes** for repeatedly incrementing one review row: the test counts rows, not occurrences. | |
| +| 5: unscaled billing inclusions | **Closed for the embedded terms.** `billingpage.go:394` divides the quoted pooled allowance by its charged quantity. | **No.** The rendered band-D assertions at `billingcurrency_test.go:2210` fail without the fix. | |
| +| 6: site overage rounding | **Closed.** Same as Opus P2-2. | **No.** | |
| +| 7: cleared site country revives console cookie | **Closed after JavaScript runs; static links still reproduce it.** Finding 8. | **No** for reverting scripted empty parameters. Static links are not checked. | |
| +| 8: first resume guard lacks independent coverage | **Closed.** `{"eur","gbp"}` at `billingcurrency_test.go:376` distinguishes the first read. | **No.** Without the first guard, the second valid response returns the URL. | |
| +| 9: provisioning rejects own-only currency options | **Closed for the reported response shape**, `stripe-provision.sh:414`. | **Yes.** `plans/provision_test.go:14` checks the price table and book identity, not the verification expression. | |
| + |
| +**Third-review claims and severity disagreements** |
| + |
| +- Opus’s country-default and locale-choice claims are corrected for normal scripted flows. Its claim about continued provider reads was right; the dispositions now correctly distinguish reads from writes. |
| +- Astra was right about the previous trigger, missing bound-completion review and first-resume test gap. These were behavioral or coverage differences, not merely different severity labels. |
| +- Astra’s P2 treatment of the mismatched checkout controls and inclusions was justified by the owner’s requirement that the console quote match the purchase. Stripe displaying a later price did not satisfy that requirement. |
| +- Astra’s claims about “six columns,” “36 or 48,” and the missing own-only exception are corrected in the principal design sections. |
| +- The already-recorded binding exception remains intentional: `billingservice.go:902` returns the known subscription without recording another purchase. |
| +- The broader provenance and mutation claims remain overstated, as detailed below. |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — Ownership checks still permit expiry or binding with a different customer identity.** |
| + |
| + **Where:** `internal/console/billingservice.go:241`, `:255`, `:620`, `:1115`; `internal/console/billingstore.go:431`. |
| + |
| + **Scenario:** A GBP intent belongs to customer `cus_A`. The provider returns an EUR session carrying the expected `checkout_intent` and `client_reference_id`, but `CustomerID=cus_B`. `ownSession` accepts it and `refuseSession` expires it. Only afterward does `retireIntent` reject the customer mismatch. Likewise, a correct-currency response with the wrong customer passes `intentSession` and can be saved before later reconciliation refuses it. |
| + |
| + This is an inconsistent-provider-response case, not a demonstrated buyer-controlled exploit. It is nevertheless the same validation boundary the previous finding addressed. |
| + |
| + **Smallest fix:** Before expiry or binding, require agreement with the intent’s stored customer and mode, the account metadata and client reference, and any already-bound session ID. Reuse that ownership predicate across both create sites, recovery and binding. Test customer-only and account-metadata-only mismatches. |
| + |
| +2. **P2 — Posted quote fields and the choice flag can authorize a currency the page did not show.** |
| + |
| + **Where:** `internal/console/billinghandlers.go:69`, `:79`, `:135`. |
| + |
| + **Scenario:** Load `?country=IN¤cy=gbp`, showing Personal at £7.70. Submit its unchanged country, currency and `quoted_*` fields, but erase `currency_chosen`. The comparison passes; `billingFormChoice` reports no choice; checkout receives an empty currency and buys India’s default USD at $9.80. Removing the `currency` control entirely produces the same result. |
| + |
| + Independently, an owner can fabricate matching `country`/`quoted_country` and `currency`/`quoted_currency` pairs without obtaining that quote. No server-issued evidence ties these declarations to a rendered page. Forging `"1"` preserves the posted currency; forging `""` can change it to the country default. |
| + |
| + **Smallest fix:** Resolve the effective purchase currency before comparing it with the quote. Require a currency control for book checkout. To enforce the requested rule against fabricated quotes, authenticate the quote’s account, country, currency and choice state with a signed value or stored quote ID. |
| + |
| +3. **P2 — Invalid explicit currencies can bypass refusal through the HTTP form handler.** |
| + |
| + **Where:** `internal/console/billinghandlers.go:70`, `:140`; `internal/console/billingpage.go:281`. |
| + |
| + **Scenario:** Change a valid form’s currency to `jpy`, leaving `quoted_currency=gbp`. Checkout redirects to `?currency=jpy` instead of returning 400. The page then drops that explicit value and displays a default. |
| + |
| + A crafted form with `currency=" "`, `quoted_currency=""`, `currency_chosen=1`, and matching Irish country fields goes further: trimming turns the invalid value into absence, and checkout creates an EUR purchase. |
| + |
| + **Smallest fix:** Validate the submitted currency before normalization, quote redirection or choice inference. Preserve the distinction between absence and an invalid nonempty value. Test invalid values with both matching and differing quote fields. |
| + |
| +4. **P2 — The refined trigger still rejects a legitimate version-1 calculation.** |
| + |
| + **Where:** `internal/console/billingstore.go:191`; `internal/plans/month.go:150`. |
| + |
| + **Scenario:** A legacy subscription supplies September’s paid span and ends September 10. A book subscription is recorded active on September 12, then canceled in the same second. Both assignments have the same stored `effective_at`, with increasing revisions and a future annual `period_end`. |
| + |
| + `AllowanceForMonth` ends the active assignment at the next assignment’s instant, finds a zero-length span and skips it. September remains legacy-only and produces version 1. The trigger nevertheless sees the active book row and rejects the freeze. |
| + |
| + The trigger’s predicate is a **superset**, not exactly the set priced by `AllowanceForMonth`. I found no corresponding too-narrow case for valid history: every priced book span satisfies its current conditions. |
| + |
| + **Smallest fix:** Compute overlap using the earlier of the next assignment’s effective instant, `period_end`, and month end, ordered consistently across both assignment tables. Require a positive span after month start. Add the same-second transition regression. |
| + |
| + `period_end` is the provider period boundary, copied by `applyBillingStateDated` at `billingstore.go:691`; renewal and quantity dating change `effective_at`, not that boundary. `billingInvoiceHistory:530` reads it as `PaidThrough`. It is not the assignment’s effective end. |
| + |
| +5. **P2 — Existing databases never receive the refined trigger.** |
| + |
| + **Where:** `internal/console/billingstore.go:189`. |
| + |
| + **Scenario:** Open a database previously migrated by `de81dd70`. Its trigger already exists, so `CREATE TRIGGER IF NOT EXISTS` preserves the substring check and broad history predicate. The canceled-purchase regression fixed in this commit still fails on that database. |
| + |
| + **Smallest fix:** Replace the trigger through an atomic migration. Test migration from the previous trigger definition, as well as a fresh database. |
| + |
| + A base-era database receives the new table, columns and trigger normally. The documented prohibition on running pre-book binaries after book purchases remains necessary; the trigger does not make arbitrary downgrades safe. |
| + |
| +6. **P2 — A bound completion whose metadata also changes currency still gets no account review.** |
| + |
| + **Where:** `internal/console/billingservice.go:912`; `internal/console/billingcurrency_test.go:470`. |
| + |
| + **Scenario:** A bound GBP checkout completes with both its actual currency and metadata currency changed to USD. Its intent and account identifiers still match. Binding returns at the price-terms comparison, before the review call at line 922. Repeated background passes leave it pending without the promised account review. |
| + |
| + The existing “provider says dollars throughout” test already constructs this case, but checks refusal and nonactivation only. |
| + |
| + **Smallest fix:** Establish ownership first, then route owned completed sessions with either a price-terms mismatch or fixed-currency mismatch through the common review path. |
| + |
| + For the narrower currency-only mismatch, repeated pass, webhook and direct-bind calls reach the same deduplicated open review. The success URL only renders the billing page. Resolving the review allows a later pass to reopen an occurrence; “once” currently means once while open. |
| + |
| +7. **P3 — Legacy EUR purchases do not receive the held-currency selector.** |
| + |
| + **Where:** `internal/console/billingpage.go:278`; `internal/console/billingservice.go:484`. |
| + |
| + **Scenario:** A canceled legacy EUR subscription visits `?country=GB¤cy=gbp`. Because its stored currency and book are empty, the held-currency branch is skipped. GBP and the other options are offered, but checkout rejects GBP against the legacy EUR purchase. |
| + |
| + **Smallest fix:** Derive held currency from validated `state.priceTerms().Money()` whenever an account has a previous purchase, including legacy rows. Add a canceled legacy-EUR page-and-submit case. |
| + |
| +8. **P3 — Static pricing links still revive old console choices.** |
| + |
| + **Where, site:** `copy-review/pricing/build.mjs:188`, `:341`; `src/site.js:71`. |
| + |
| + **Scenario:** JavaScript is disabled, delayed or fails to load. Pricing displays USD/band A, but its signup links omit both parameters. A browser carrying `carloku_country=IN` and `carloku_currency=gbp` reaches a console that reuses those choices. |
| + |
| + **Smallest fix:** Generate every pricing signup link with `currency=&country=` initially; JavaScript can replace their values. Assert the static links’ parameters. |
| + |
| +9. **P3 — Provisioning verification still rejects legitimate historical catalogs.** |
| + |
| + **Where:** `implementation/carloku-paid-launch/stripe-provision.sh:388`. |
| + |
| + **Scenario:** The configured catalog contains the current 36 prices plus 12 active legacy prices. The revised map-length check accepts it, but the preceding global Stripe-price count still demands exactly 36 and fails on 48. |
| + |
| + **Smallest fix:** Scope catalog counts to the selected book, or verify its required identities without rejecting unrelated historical prices. Cover the emitted verification logic, not just the price table. |
| + |
| +10. **P3 — `"0"` enables the site’s draft override.** |
| + |
| + **Where, site:** `copy-review/pricing/build.mjs:33`; `hack/check-pricebook.mjs:8`. |
| + |
| + **Scenario:** A build environment sets `PRICEBOOK_ALLOW_DRAFT=0` to disable draft publication. Both checks treat the nonempty string as permission and succeed. |
| + |
| + **Smallest fix:** Require `process.env.PRICEBOOK_ALLOW_DRAFT === "1"` in both places. Test unset, empty, `"0"`, `"false"` and `"1"`. |
| + |
| +The ordinary **IN/GBP purchase** remains consistent: the page posts GBP with matching quote fields; `startBillingCheckout:392` selects GBP and band D; `PriceFor` supplies 770 pence for Personal; provider price verification checks currency and amount; `CreateCheckout:621` disables Adaptive Pricing. Binding and reconciliation retain the intent’s terms, `applyBillingStateDated:648` stores them, and `billingInvoiceHistory:530` carries them into `AllowanceForMonth`. `priceBillingCalculation:174` writes version 2 in GBP. Drafts, items, recovery and invoice verification use that frozen currency, and `billingquantity.go:1016` and `:1104` check the stored price and currency. I found no additional conversion or historical repricing path. |
| + |
| +Other requested checks: |
| + |
| +- An unchanged stale tab uses its own posted fields; another tab’s cookies do not alter it. Normal changed selections redirect before purchase. |
| +- The quote endpoint checks session, form token and membership, then constructs an escaped account-local redirect. It performs no billing mutation or provider call. |
| +- Present-empty parameters clear cookies; absent parameters reuse them; invalid parameters leave existing cookies unchanged. Consequently an invalid visit can be followed by an absent-parameter visit that reuses the old choice. Checkout itself does not read these cookies. |
| +- Current inclusion figures are exact for every valid quantity: the embedded per-member band products are integral in base units and fit the displayed precision. Unavailable Personal uses quantity one; teams use the charged quantity, including above the minimum. Band-A sentences match the asserted approved text. `formatFigure(1,3)` truncates, but no current plan produces that share. |
| +- Book validation, checked arithmetic, historical catalog configuration, legacy snapshot bytes and request-hash compatibility show no additional defect. |
| +- The two committed public exports are byte-identical. They disclose commercial configuration, not customer data or provider credentials. The site’s current prices, rates, country options and scaled inclusions agree with that export. |
| + |
| +**Claims the code contradicts** |
| + |
| +- `DISPOSITIONS-multi-currency.md:79` says every finding has a test that fails without its fix. Provisioning verifier changes are not tested by `plans/provision_test.go:14`. Several site consistency guards can also be removed while the current valid fixture still passes. |
| +- The claimed empty-country mutation coverage at dispositions line 102 is not established by the named tests. Replacing only `billingpage.go`’s absent-only fallback with empty-value fallback would survive them. Add a GET containing `country=` **and an incoming country cookie**. |
| +- `TestBoundSessionCompletedInAnotherCurrencyIsReviewedOnce`, `billingcurrency_test.go:2059`, claims webhook coverage but never sends one. Its `count(*)` assertion also cannot detect repeated increments of `occurrences`. |
| +- The “equivalent by construction” choice-flag mutation at dispositions line 102 is equivalent for the tested genuine forms, not for all accepted requests. Finding 2 supplies a counterexample. |
| +- Dispositions line 85 says the trigger matches what `AllowanceForMonth` prices. Finding 4 disproves that. The upgrade path additionally retains the previous predicate. |
| +- Site `build.mjs:142` checks typed figures with substring matching. It does not establish exact numeric equality: a minimum of `1` would match approved text containing `10`. The country-option test at `pricing-currency.test.mjs:120` uses the same real defaults as the removed hardcoded rules, so it does not independently prove the source of those defaults. |
| +- `billingpage.go:393` names nonexistent `TestBillingInclusionsAreExactPerMember`; the actual test checks minimum and seven-times-minimum quantities. The current arithmetic supports all quantities, but the claimed exhaustive test does not exist. |
| +- Design line 213 still says the demo runs without a book; `billingflow_demo_test.go:77` now configures one. |
| +- I found no concrete date-dependent failure or test-order/cookie-jar contamination in the reviewed tests. They do use wall-clock session and fake-expiry timestamps. The browser fixture intentionally has no jar; the sign-in test intentionally carries one. These are not evidence of runtime verification. |
| + |
| +**What must be verified against Stripe test mode before enablement** |
| + |
| +1. **Catalog:** Read all 36 prices with the pinned API version and expanded `currency_options`. Confirm exact unit amounts, currency, own-only/empty options, metadata, annual recurrence and tax behavior. Repeat verification with historical catalogs present. |
| +2. **Checkout:** Complete USD, EUR and GBP purchases with matching and differing country defaults, including IN/GBP. Confirm subtotal, disabled Adaptive Pricing, absent conversion, and matching session, subscription and item currencies. An incompatible price/session currency must produce no payable checkout. |
| +3. **Overages:** Exercise all four meters, rounding boundaries and invoice floors in each currency through creation, finalization and payment. Compare lines and subtotal with the frozen calculation; include canceled subscriptions and tax results. |
| +4. **Recovery:** Lose responses after checkout creation, invoice creation, item creation, finalization and payment, inside and beyond replay windows. Establish that recovery retains original objects and parameters without duplicate charges. Include legacy checkout requests and version-1 invoice operations. |
| +5. **Quantity and renewal:** Run `TestSandboxQuantityFence`, both proration policies, and renewal in GBP and USD. Inspect actual proration and renewal invoices for the stored currency and expected amounts. |
| +6. **Customer and discount constraints:** Establish behavior with active/canceled subscriptions and outstanding historical invoices. Verify percentage and house discounts, rejection of incompatible fixed-currency discounts, completed-session expiry errors, and the resulting subscription currency of legacy Adaptive Pricing sessions. |
| + |
| +**Verdict: reject.** |
| \ 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 383dda39e..80ea3c2fa 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -76,13 +76,13 @@ The fixes are in the commit that adds this section and have not been read by a r |
| ## Review three (at de81dd70; Opus accepted with fixes, Astra rejected) |
| -Reports: `OPUS-REVIEW-multi-currency-review3.md`, `CODEX-FINDINGS-multi-currency-review3.md`. Both found the second review's findings closed and the GBP purchase carrying one currency end to end; Astra found the first resume guard not independently covered (below). The two disagreed on severity, not on the code: every finding of either is treated as a defect here, and each has a test that fails without its fix. |
| +Reports: `OPUS-REVIEW-multi-currency-review3.md`, `CODEX-FINDINGS-multi-currency-review3.md`. Both found the second review's findings closed and the GBP purchase carrying one currency end to end; Astra found the first resume guard not independently covered (below). The two disagreed on severity, not on the code: every finding of either is treated as a defect here, and each code change has a test that fails without it; the two changes to `stripe-provision.sh` had none until review four (its test now reads the generated checks). |
| | Finding | Disposition | Where | Reproduction | |
| | --- | --- | --- | --- | |
| | Astra 1 (P2): a currency refusal expired the session the provider answered with before checking whose it was. | Fixed. The client returns the refused session itself, not its id alone; the service expires it only when `ownSession` says it is this intent's, else refuses the attempt and touches nothing. | `billingstripe/client.go` (`Error.Session`), `billingservice.go` (both create sites) | `TestCurrencyRefusalExpiresOnlyTheIntentsOwnSession` (another intent's session in another currency: zero expiries, the intent kept) | |
| | Astra 4 (P2): a bound session that completed in another currency was refused in `bindBillingCheckout` with no review. | Fixed. The owned, completed, wrong-currency session files the same review once, through the pass, the webhook and a direct binding; nothing is expired, nothing recorded. | `billingservice.go` `bindBillingCheckout` | `TestBoundSessionCompletedInAnotherCurrencyIsReviewedOnce` | |
| -| Astra 3 (P2): the barrier trigger refused this build's own version 1 for a legacy-only month after a book purchase that was cancelled before its first reconciliation. | Fixed. The trigger counts only book assignments that were active and paid into the month (`status='active'`, not free, began before the month's end, paid through past its start), which is what `AllowanceForMonth` prices from. The claim at `billingstore.go` that this build never writes a refused version 1 is true again. | `billingstore.go` | `TestBarrierTriggerLetsThisBuildsLegacyMonthsThrough` (the cancelled purchase: September freezes; an active one: refused) | |
| +| Astra 3 (P2): the barrier trigger refused this build's own version 1 for a legacy-only month after a book purchase that was cancelled before its first reconciliation. | Fixed. The trigger counts only book assignments that were active and paid into the month (`status='active'`, not free, began before the month's end, paid through past its start), which review four showed was a superset of what `AllowanceForMonth` prices from (a same-second supersession) and which did not reach a database that already had the old trigger; both corrected in review four. | `billingstore.go` | `TestBarrierTriggerLetsThisBuildsLegacyMonthsThrough` (the cancelled purchase: September freezes; an active one: refused) | |
| | Opus P3-3: the trigger read the version by substring. | Fixed: `json_extract(... '$.v') IS NOT 2`, with `json_valid` guarding a calculation that is not JSON (nothing writes one; tests do). | `billingstore.go` | same test (a version 2 nested in a version 1 is refused; "not json" over book history is refused, before it is not) | |
| | Opus P2-1, Astra 2 (P2): the page always posted a currency, so the country's default never applied; the quote form and the checkout form could disagree. | Fixed by one form. The country and currency controls, the plans, the postal code and the consent are one form, which carries what the page priced (`quoted_country`, `quoted_currency`, `currency_chosen`). A post whose country or currency is not what the page priced is redirected to the page quoting it, so nothing is bought unseen; a currency shown as the country's default and not chosen reaches the door as no currency, and the country's own currency is bought in; a chosen one is the purchase's. The quote button posts the same form to `/billing/quote`, which only redirects. | `billing.html`, `billingpage.go`, `billinghandlers.go` | `TestBillingFormPostsWhatItShows` (the form as the page built it, submitted untouched and changed: Ireland with no choice buys in euro after one quote; a currency changed is shown first, then bought as a choice; the quote door; the old-way console; the held currency), `TestCheckoutFormCurrencyField`, `TestBillingPageCurrencyControls` (the `currency_chosen` flag per case) | |
| | Opus P2-1 (the site's half), Astra 7 (P3): the pricing page sent the locale's guess as `?currency=`, and omitted the country, so the console reused an old one. | Fixed on the site (`2fc6388`): the links always carry both parameters, the currency only when the visitor chose it, the country selected or empty; the console clears a cookie whose parameter is present and empty. | site `src/site.js`; `billingcurrency.go` | site `pricing-currency.test.mjs` ("a locale or country default is shown but never sent as a choice"); `TestCurrencySuggestionCookie`, `TestCountrySuggestionCookie` (present-empty clears) | |
| @@ -102,3 +102,30 @@ Not done: the design's "no step-up stands between a member and a price" is the q |
| Mutation checks on these fixes, twelve in a scratch copy, each run against the test named for it: eleven failed their tests (the ownership check before expiry removed; the review on a bound completion removed; the trigger counting cancelled purchases; the trigger reading the version by substring, once the nested statement was spelled `"v":2,` as a substring reads it; the quote-before-purchase redirect removed; the pricing page's cookie counted as no choice; the held currency not offered alone; an empty `?country=` falling back to the cookie; the inclusions priced at band A; an empty `?currency=` not clearing the cookie; the first resume guard removed, which `{"eur","gbp"}` now catches). One survived and is equivalent by construction: passing the shown currency to the door as a choice when it was a default. Under the quote rule the posted currency equals the shown one, which equals the country's default, so the door's outcome is the same either way; the flag keeps the door's contract honest (no currency means the country's) and changes no result. No claim is made that the flag is tested beyond what `TestBillingFormPostsWhatItShows` observes. |
| The fixes are in the commit that adds this section and have not been read by a reviewer. |
| + |
| +## 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 every third-review finding closed in the code and no path that stores or sends money in a currency other than the purchase's; the disputed trigger reading went Astra's way. Every finding of either is fixed below, each with a test that fails without it; eight mutants, one per fix, were killed in a scratch copy. |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra 1 (P2): `ownSession` did not compare the customer, so a session carrying the intent's id but another customer could be expired, or saved, before a later check refused it. | Fixed: `ownSession` also requires the intent's customer (once it has one) and, once bound, the bound session's id; `intentSession` starts from it; `bindBillingCheckout` checks the customer before anything. | `billingservice.go` | `TestCurrencyRefusalExpiresOnlyTheIntentsOwnSession` (another customer, in another currency and in the right one: zero expiries, nothing kept) | |
| +| Astra 2 (P2): posted quote fields and the choice flag could make the door buy a currency the page did not show (the flag erased on a chosen GBP page bought India's dollar). | Fixed: the handler compares the quote with what would be bought (the chosen currency, else the posted country's own), not with the posted field. A form whose fields disagree with each other, or has no currency control against a chosen quote, is answered with the quote for what it would buy. Fields forged to agree with each other buy exactly what they describe, which the buyer sees at the provider; a signed quote would add nothing to that, since the posted country and currency determine the price. | `billinghandlers.go` | `TestBillingFormPostsWhatItShows` ("a choice flag erased buys nothing the page did not show") | |
| +| Astra 3 (P2), Opus P3-5: an invalid currency reached the quote redirect instead of a 400, `" "` was trimmed into absence, and an unsold `?currency=` was silently replaced on the page. | Fixed: a present currency field must be exactly a currency sold (no trimming, no folding, not empty), else 400 before anything, on the checkout and the quote door; the page says when `?currency=` names something not sold and does not post the default as a choice. | `billinghandlers.go` `billingFormChoice`; `billingpage.go`; `billing.html` (`billing.currency.unknown`, unreviewed system copy, flagged) | `TestBillingFormPostsWhatItShows` ("a currency that is not sold is refused before anything") | |
| +| Astra 4 (P2): the trigger counted an active book row superseded in the same second, which `AllowanceForMonth` skips. | Fixed: the trigger's span ends at the earliest of the next assignment's effective instant (time, then revision, over both tables), `period_end` and the month's end, and must have length inside the month: `AllowanceForMonth`'s rule. | `billingstore.go` | `TestBarrierTriggerLetsThisBuildsLegacyMonthsThrough` ("superseded in the same second") | |
| +| Astra 5 (P2), Opus P3-1: a database opened by an earlier build kept the old trigger. | Fixed: the trigger has a new name and the old one is dropped first at migration. | `billingstore.go` | same test ("a database that carries the earlier, wider trigger": the old body installed, migration run, the new name alone remains, the legacy month freezes) | |
| +| Astra 6 (P2): a bound completion whose metadata changed with its currency returned before the review. | Fixed: an owned completed session on other price terms or in another currency files the review; the webhook path is now exercised and `occurrences` stays 1. | `billingservice.go` `bindBillingCheckout` | `TestBoundSessionCompletedInAnotherCurrencyIsReviewedOnce` (two passes, a binding, two webhooks: one review, one occurrence; dollars throughout, metadata included) | |
| +| Opus P3-2, Astra 7 (P3): a purchase from before price books was not held. | Fixed: any account with a subscription is held in `state.priceTerms().Money()`, legacy EUR included. | `billingpage.go` | `TestBillingFormPostsWhatItShows` ("bought before price books is offered its euro alone") | |
| +| Opus P3-4, Astra 8 (P3): the static site links carried neither parameter. | Fixed on the site (`4feb1cd`): every link carries `currency=&country=` statically; the script replaces the values. | site `build.mjs` | site `pricing-pricebook.test.mjs` | |
| +| Astra 9 (P3): the script's Check 2 demanded exactly 36 active prices in the account. | Fixed: it counts the book's own (metadata `price_book`); the plans test reads the generated checks. | `stripe-provision.sh`; `plans/provision_test.go` | `TestStripeProvisionScriptMatchesThePriceBook` | |
| +| Astra 10 (P3): `PRICEBOOK_ALLOW_DRAFT=0` allowed a draft. | Fixed on the site: exactly `"1"` in the build and the check; the preview is written before the refusal, as the comment said. | site | site test (unset, empty, `0`, `false`, `yes` refuse) | |
| +| Opus P3-6: the draft gate holds only through `npm run check`. | Stated, not changed: the runbook runs `check`; a `build` hook is a site-build decision for the morning. | | | |
| +| Opus P3-3: a resolved refused-paid review is reopened by the next pass while the intent stands. | Open: an operator's door that retires such an intent once the money is settled does not exist; the comment and the design say "once while open". | | | |
| +| Opus P3-7, Astra claims: test gaps (webhook path, `occurrences`, `ownSession` clauses, console divisibility check, the script's jq, the empty-country mutation claim, the token match on the site). | Fixed: the webhook and `occurrences` in the review test; the customer and account clauses; divisibility of every figure at the minimum in the console test; the script's checks read by the plans test; `?country=` with a country cookie in the page test; token matching in `build.mjs`. | tests | as named | |
| +| Claims corrected: the nonexistent test name at `billingpage.go`; "every finding has a test" (the script changes had none); "once" is once while open; the trigger "exactly the set"; the design's demo-without-a-book sentence; `build.mjs`'s "preview always written". | Corrected in this commit. | | | |
| + |
| +Mutation checks on these fixes, eight in a scratch copy, each run against its test: all eight failed their tests (the customer clause removed; the posted currency compared instead of the effective one; an invalid currency normalised instead of refused; the trigger ignoring the next assignment; the old trigger left in place; a legacy purchase not held; a terms mismatch on a bound completion not reviewed; the unsold-currency notice removed). |
| + |
| +Open after this round: the operator's door for a refused, paid session's intent (Opus P3-3); the site's `bands.p3` sentence; `billing.currency.unknown` and `billing.quote.unknown` are unreviewed system copy. |
| + |
| +The fixes are in the commit that adds this section and have not been read by a reviewer. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review4.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review4.md| new file mode 100644 |
| index 000000000..c393380c5 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review4.md |
| @@ -0,0 +1,121 @@ |
| +# Opus authority review, round four: `feat/carloku-multi-currency-20261001` at `8958392b` |
| + |
| +**Verdict: accept with fixes.** Every finding from the third review is closed in the code. The trigger dispute and the three code disputes went Astra's way. What remains are P3s: the trigger migration (P3-1) and the legacy held currency (P3-2) are the ones to fix before enabling. No path was found that charges or stores money in a currency other than the purchase's stored one. |
| + |
| +How I worked: I read source only, with `git show` at `8958392b` and, for the site, at `2fc6388`. I ran no tests and modified nothing; the snapshot I made under `~/.cache` has been deleted. The Bash sandbox failed to start in this worktree (bwrap could not find `.claude/settings.json`), so the read-only git commands ran outside it. `/sandbox` can fix that. |
| + |
| +## Third-review findings |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1 / Astra 2**: the page always posted a currency; the quote form and the checkout form could disagree | **Closed.** There is now one form, carrying `quoted_country`, `quoted_currency` and `currency_chosen`. A post that differs from the quote is redirected to the quote (`billinghandlers.go:69-72`). A default that was not chosen reaches the door as `""` (`:80-84`). | **No.** `TestBillingFormPostsWhatItShows`: without the redirect, Ireland buys USD; with the flag always empty, the IE-in-USD case buys EUR. The one surviving mutation (a default sent as a choice) gives the same result, as the dispositions say. | |
| +| **Opus P2-1, the site's half / Astra 7** | **Closed** with scripting on. Without scripting, see P3-4. | Site test "a locale or country default is shown but never sent as a choice" asserts it. | |
| +| **Opus P2-2 / Astra 6**: rounded overage rates | **Closed.** `rate()` in `build.mjs` is exact. | **No.** `pricing-pricebook.test.mjs:99-117` parses every rate back to the export's micro amount. | |
| +| **Opus P3-1**: returning buyer gets a misleading banner | **Closed only for book-priced accounts** (`billingpage.go:278`). Still open for legacy accounts (P3-2). | **No** for the held-GBP case. | |
| +| **Opus P3-2**: an empty `?country=` brought the cookie back | **Closed** (`billingpage.go:213-227`, `billingcurrency.go:50-61`). | I checked the code; I did not rerun that test. | |
| +| **Opus P3-3**: the trigger read the version by substring | **Closed** (`billingstore.go:189-195`, `json_valid` + `json_extract`). | **No.** The nested `"v":2,` case passes the old `NOT LIKE` and would be frozen. | |
| +| **Opus P3-4**: the site published a draft book | **Closed, with caveats** (P3-6). | Site test exists. | |
| +| **Opus P3-5**: smaller site and test items | **Closed**: `must()` checks the figures in strings; the country list and each country's default come from the book; `(f*bps)%10000` and `(f*min*bps)%10000` are checked; the provisioning test uses `t.Fatalf`; the check script accepts `-ge 36` keys and the own-only rule. | The `stripe-provision.sh` jq changes have **no test**. `TestStripeProvisionScriptMatchesThePriceBook` reads only the `CENTS` table, so both changes could be reverted and it would pass. | |
| +| **Astra 1**: refusal expired a session before checking whose it was | **Closed.** `Error.Session` is returned and `ownSession` is checked at both create sites (`billingservice.go:620`, `:1115`). | **No.** `TestCurrencyRefusalExpiresOnlyTheIntentsOwnSession` expects zero expiries. Only the intent-id branch of `ownSession` is exercised (P3-7). | |
| +| **Astra 3**: the trigger refused this build's own version 1 | **Closed.** The trigger now requires `status='active'`, not free, and overlap with the month. | **No.** The cancelled-before-reconcile September case fails under the old trigger. | |
| +| **Astra 4**: a bound completion in the wrong currency got no review | **Closed** (`billingservice.go:916-925`). The pass, the webhook (`billinghandlers.go:225`) and a direct bind all reach it. | **No.** Zero reviews without the fix. The test's comment says it covers the webhook; it does not call it. | |
| +| **Astra 5**: band A inclusions beside band D prices | **Closed** (`billingPlanRows`, `formatFigure`). | **No.** The band D strings fail. | |
| +| **Astra 8**: the first resume guard had no case of its own | **Closed.** With `{"eur","gbp"}` and the first guard removed, the second read passes `intentSession` and the URL is returned. | **No.** | |
| +| **Astra 9**: the verifier refused the own-only `currency_options` shape | **Closed.** The jq rule matches `priceCurrencyOptionsAreOwn`. | No test (see Opus P3-5). | |
| +| **Claims** ("nothing asked of the provider", "six columns", "36 or 48", `bands.p3`) | Corrected or recorded. | — | |
| + |
| +**Where the third review's two reviewers disagreed, the code at `de81dd70` sides with Astra:** |
| +- **The trigger.** Old line 183 counted any book assignment with `effective_at` before the month's end, so a cancelled purchase blocked a legitimate legacy September. Opus called the trigger sound; it was not. |
| +- **The bound completion.** `bindBillingCheckout` returned `errBillingIdentity` with no review. Opus marked its own P3-1 closed; it was open on that path. |
| +- **The inclusions.** They were plainly wrong figures, not a pre-existing quirk. |
| +- **Astra 1** needed an inconsistent provider response, as Astra said. It was real, but low likelihood. |
| + |
| +Astra's "reject" at `de81dd70` was justified. |
| + |
| +## Findings, most severe first |
| + |
| +**P3-1. The changed trigger never reaches a database that already has the old one.** |
| +- Where: `billingstore.go:189`, `CREATE TRIGGER IF NOT EXISTS`. |
| +- Scenario: any database opened by `a88a39e6` or `de81dd70` (dev, a demo, a test-mode console) keeps the substring trigger, which is too wide. That build's own legacy September after a cancelled book purchase is refused there for good. |
| +- Fix: `DROP TRIGGER IF EXISTS` and `CREATE` in one `BEGIN IMMEDIATE` (the pattern at `billingquantityaudit.go:38`), or a new trigger name. Add a test that opens a database carrying the old body. |
| + |
| +**P3-2. A legacy (pre-book, EUR) account is not offered its held currency.** |
| +- Where: `billingpage.go:278` requires `state.PriceBook != ""`, but `billingservice.go:483` refuses any purchase whose `Money()` is not `eur` for every account with `Plan != ""`. |
| +- Scenario: an account bought under legacy terms, then cancelled. A US owner sees USD as the default and posts; the door chooses `usd` and returns `errBillingConflict`, and the page shows "The account changed while you were on this page". |
| +- Fix: hold whenever `state.Plan != "" && state.Subscription != ""`, using `state.priceTerms().Money()`. Add a held-legacy case to the test. |
| + |
| +**P3-3. A refused, paid session's review comes back after every resolution.** |
| +- Where: `billingservice.go:922`, `:281`; `fileBillingReviewTx` sets `resolved_at=0` on conflict (`billingstore.go:731`). |
| +- Scenario: the operator resolves `billing_checkout_currency_refused`. The intent is still there, so the next pass binds again and reopens the review, holding the account indefinitely. Nothing lets an operator retire the intent. |
| +- Fix: an operator door that retires the intent once the refund is settled, or record on the intent that the review was filed. |
| + |
| +**P3-4. The site's signup links carry neither parameter when scripting is off.** |
| +- Where: site `build.mjs`, the card `href="https://console.carloku.com/?plan=${p}"` and the `start.cta.console` link. |
| +- Scenario: a visitor without scripting sees USD at band A. The console then quotes an old `carloku_country=IN` cookie at band D, or an old `carloku_currency=gbp` cookie as a choice. |
| +- Fix: build the static hrefs with `¤cy=&country=`. |
| + |
| +**P3-5. An unsold `?currency=` is silently replaced on the billing page.** |
| +- Where: `billingpage.go:281-286`. |
| +- Scenario: `?currency=jpy` (or a post that changed to `jpy`, which redirects there) shows the country's default with no notice. Clicking buys the default. Nothing is bought unseen, but rule 1's "never replaced by a default" holds only at the door. |
| +- Fix: a `CurrencyError` notice like `QuoteError`. |
| + |
| +**P3-6. The site's draft gate holds only through `npm run check`.** |
| +- Where: `package.json`; `build.mjs:30`. |
| +- Scenario: the committed `src/pricing/index.html` is already a draft (first line is the marker), and a plain `eleventy` / `npm run build` ships it. The runbook does run `check`, so the gate holds as long as the runbook is followed. |
| +- Fix: run `check-pricebook` in `build`, or in an eleventy `before` hook. |
| + |
| +**P3-7. Test gaps.** |
| +- `ownSession`'s `client_reference_id` and live-mode clauses are untested; it also does not compare the customer, which is acceptable because only this console writes `checkout_intent`. |
| +- The inclusions test uses quantities of min and 7×min only. The `(f*bps)%10000` check that would catch a future inexact figure exists on the site but not in the console. Add it. |
| +- The webhook path in `TestBoundSessionCompletedInAnotherCurrencyIsReviewedOnce` is not exercised. |
| +- The `stripe-provision.sh` jq rule is not exercised. |
| + |
| +**Section notes:** |
| +- **A. One GBP purchase from the form, end to end.** From the form's `?country=IN¤cy=gbp` (£7.70, `currency_chosen=1`), the post's quote check passes, `gbp` is posted, the book's band D GBP price is checked, and the session is created in `gbp` with Adaptive Pricing off. Bind, reconcile, the book assignment, `AllowanceForMonth`, the version 2 calculation, the engine and the quantity worker are unchanged since the third review and stay in `gbp`. |
| +- **B. The one-form rule and the quote door.** |
| + - The quote door's redirect is built from `acct.Sqid` and `url.Values`. It cannot leave the account's billing page and writes nothing. |
| + - The checkout door's redirect comes after step-up and consent. |
| + - A stale tab posts its own quote. A forged `currency_chosen=""` on a chosen currency buys the country's default; only the buyer can do that, and Stripe shows the amount. |
| + - A post with no `currency` field buys the default. |
| +- **D. Ownership on every path.** No path expires, binds or records a session that is not this account's. |
| +- **E. The trigger against `AllowanceForMonth`.** |
| + - Every book span that `AllowanceForMonth` counts satisfies the trigger (active, not free, `effective_at` before the month's end, `PaidThrough = period_end` after its start), so the trigger is never too narrow. |
| + - It is wider only for superseded active rows. This build cannot hit that, because a legacy assignment cannot follow a book one: reconcile requires the current intent. |
| + - `period_end` has one writer (`applyBillingStateDated`, from `sub.CurrentPeriodEnd`). |
| + - The one way through is back-dating: an assignment recorded after its month was frozen (`effective = sub.Created`). That needs an older binary to run after a book-priced intent exists, which the roll note forbids. |
| +- **F. Inclusions.** They are exact at every plan, band and quantity today. The unavailable Personal row is priced at quantity 1. Band A matches the approved sentences. |
| +- **H. Schema and rollout.** Fine, apart from P3-1. |
| +- **J. The export.** Unchanged; it discloses nothing beyond prices, bands and allowances. |
| +- **K. The pricing site.** |
| + - The export copy is byte-identical to the platform's. Bands come from the same `CountryBands()` the console uses. |
| + - A visitor whose browser locale is en-GB sees £22, while the console shows $28. That follows from ruling 9, but it is a visible difference. |
| + - `site.js` still hard-codes the `supported` currency list and assumes the first currency is the book's default. |
| + |
| +## Claims the code contradicts |
| + |
| +- `billingpage.go:393` cites `TestBillingInclusionsAreExactPerMember`, which does not exist. |
| +- `billingcurrency_test.go:2061` and the dispositions say the review-once test covers the webhook. It does not. |
| +- The dispositions say each third-review finding "has a test that fails without its fix". That is untrue for the `stripe-provision.sh` changes (Astra 9 and the 36-key change), and the Opus P3-1 fix does not cover legacy accounts. |
| +- `build.mjs` says "The preview is always written for review". The throw at line 30 comes before any write. |
| +- The design and `site.js` say the link carries both parameters "always". Not without scripting (P3-4). |
| +- The dispositions and design say a review is filed "once". It is reopened after every resolution (P3-3). |
| + |
| +## To run in Stripe test mode before enabling |
| + |
| +1. **Every one of the 36 prices**, with `?expand[]=currency_options`. Expect the expanded map to be empty or hold only the price's own currency, and the script's Check 3 and the console both to pass. |
| +2. **A session per currency** (`usd`, `eur`, `gbp`, with `adaptive_pricing[enabled]=false`), for buyers in IE, GB, US, IN and JP. Expect `currency` equal to the price's, no `currency_conversion`, and a subtotal of unit × quantity. |
| +3. **A session whose `currency` differs from the price's.** Expect a 400 at create; note the error code. |
| +4. **The completed subscription.** Expect `currency` and the item's price currency to be the choice, and `price_book` and `currency` in its metadata. |
| +5. **The customer's currency lock.** |
| + - A USD checkout while a GBP subscription is live: expect it refused; note at which call. |
| + - After cancelling: is USD allowed on the same customer? |
| + - Is a GBP overage invoice for an old month still accepted? |
| +6. **Overage invoices in GBP and USD** (draft, items, finalize, pay) with only the IE registrations. Expect automatic tax to complete and the subtotal to equal the calculation. |
| +7. **A GBP quantity change and a GBP renewal.** Expect proration lines and the renewal invoice in GBP. |
| +8. **Replays after a lost response.** |
| + - The same key and parameters: expect the same session. |
| + - A legacy request (Adaptive Pricing on, no currency) under its original key: expect it accepted. |
| +9. **`ExpireCheckout` on a completed session.** Note the error shape. |
| +10. **Promotion codes.** A EUR `amount_off` code on a GBP session: expect it refused. The 50% code and the house coupon: expect them applied. |
| + |
| +**Verdict: accept with fixes** (P3-1 and P3-2 before enabling; the rest can follow). |
diff --git a/implementation/carloku-paid-launch/stripe-provision.sh b/implementation/carloku-paid-launch/stripe-provision.sh| index d4e9db422..64e210649 100755 |
| --- a/implementation/carloku-paid-launch/stripe-provision.sh |
| +++ b/implementation/carloku-paid-launch/stripe-provision.sh |
| @@ -385,11 +385,11 @@ print_check() { |
| words stripe get /v1/account "${MODE_WORDS[@]}" |
| printf ' \\\n | jq -e --arg a "$EXPECTED_ACCOUNT" %s\n\n' "'.id == \$a and .country == \"IE\" and .default_currency == \"eur\" and .charges_enabled == true'" |
| - section "Check 2. Counts: exactly 3 active products and 36 active recurring prices" |
| + section "Check 2. Counts: exactly 3 active products, and exactly 36 active recurring prices of this book (other books' and legacy prices may stand beside them)" |
| words stripe get /v1/products "${MODE_WORDS[@]}" -d active=true -d limit=100 |
| printf ' \\\n | jq -e %s\n' "'(.data | length) == 3 and (.has_more == false)'" |
| words stripe get /v1/prices "${MODE_WORDS[@]}" -d active=true -d limit=100 |
| - printf ' \\\n | jq -e %s\n\n' "'(.data | length) == 36 and (.has_more == false)'" |
| + printf ' \\\n | jq -e --arg book "%s" %s\n\n' "$BOOK" "'([.data[] | select(.metadata.price_book == \$book)] | length) == 36 and (.has_more == false)'" |
| section "Check 3. Every price and its product (the GetPrice and GetProduct fields the console compares)" |
| note "Console rules: active, livemode matches mode, the key's currency, exclusive, year x1, licensed, per_unit, no transform_quantity," |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index f344ba167..7733902ab 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -2,10 +2,12 @@ package console |
| import ( |
| "context" |
| + "crypto/hmac" |
| "crypto/sha256" |
| "encoding/hex" |
| "encoding/json" |
| "errors" |
| + "fmt" |
| "io" |
| "net/http" |
| "net/http/httptest" |
| @@ -1676,6 +1678,25 @@ func TestBillingPageCurrencyControls(t *testing.T) { |
| } |
| }) |
| } |
| + // An empty ?country= quotes nothing, whatever country cookie came in; |
| + // an absent one quotes the cookie's. |
| + { |
| + c := signInPaul(t, fx) |
| + for _, tc := range []struct{ query, want string }{{"?country=", ""}, {"", "India"}} { |
| + req, _ := http.NewRequest(http.MethodGet, fx.u("/billing")+tc.query, nil) |
| + req.AddCookie(&http.Cookie{Name: countryCookie, Value: "IN"}) |
| + res, err := c.Do(req) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + body, _ := io.ReadAll(res.Body) |
| + res.Body.Close() |
| + cm := mainOf(t, string(body)) |
| + if quoted := strings.Contains(cm, "Prices for India"); quoted != (tc.want != "") || strings.Contains(cm, `<option value="IN" selected>`) != (tc.want != "") { |
| + t.Fatalf("%q with a country cookie: quoted India = %v, want %v:\n%s", tc.query, quoted, tc.want != "", cm) |
| + } |
| + } |
| + } |
| // Band D in pounds, per member: the rounding rule's figures. |
| m = fetch(t, "?country=IN¤cy=gbp", "") |
| for _, want := range []string{"<strong>£1.58</strong> per member a year", "£15.80 a year", "<strong>£15.75</strong> per member a year"} { |
| @@ -1913,6 +1934,82 @@ func TestBillingFormPostsWhatItShows(t *testing.T) { |
| t.Fatalf("intent %+v %v; want legacy terms", intent, err) |
| } |
| }) |
| + t.Run("a choice flag erased buys nothing the page did not show", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "IN" |
| + b := newBillingBrowser(t, fx) |
| + _, fields := b.page("?country=IN¤cy=gbp") |
| + fields.Set("currency_chosen", "") |
| + fields.Set("postal", "110001") |
| + // What would be bought is India's own dollar, which the page did |
| + // not price: a quote first. |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx.path("/billing")+"?country=IN¤cy=" { |
| + t.Fatalf("erased flag: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); err == nil { |
| + t.Fatal("an intent was opened at a price not shown") |
| + } |
| + // And a form with no currency control at all, against a page that |
| + // priced a choice. |
| + _, fields = b.page("?country=IN¤cy=gbp") |
| + fields.Del("currency") |
| + fields.Set("postal", "110001") |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || w.Header().Get("Location") != fx.path("/billing")+"?country=IN¤cy=" { |
| + t.Fatalf("no control: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + // Forged quote fields that agree with each other buy exactly what |
| + // they describe, which the buyer then sees at the provider: the |
| + // dollar at band D, not the pound the page showed. |
| + _, fields = b.page("?country=IN¤cy=gbp") |
| + fields.Set("currency", "usd") |
| + fields.Set("currency_chosen", "") |
| + fields.Set("quoted_currency", "usd") |
| + fields.Set("postal", "110001") |
| + if w := b.submit(fields); w.Code != http.StatusSeeOther || !strings.HasPrefix(w.Header().Get("Location"), "https://checkout.stripe.com/") { |
| + t.Fatalf("forged agreeing fields: %d %s", w.Code, w.Header().Get("Location")) |
| + } |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.Currency != "usd" || intent.Band != plans.BandD { |
| + t.Fatalf("intent %+v %v; want the dollar the fields described", intent, err) |
| + } |
| + }) |
| + t.Run("a currency that is not sold is refused before anything", func(t *testing.T) { |
| + fx, _, acct := newCurrencyFixture(t) |
| + b := newBillingBrowser(t, fx) |
| + for _, bad := range []string{"jpy", " ", "", "GBP", "gbp ", "eur\x00"} { |
| + _, fields := b.page("?country=IE¤cy=gbp") |
| + fields.Set("currency", bad) |
| + if w := b.submit(fields); w.Code != http.StatusBadRequest || strings.TrimSpace(w.Body.String()) != "currency_invalid" { |
| + t.Fatalf("currency %q: %d %q", bad, w.Code, w.Body.String()) |
| + } |
| + if w := b.post("/billing/quote", fields); w.Code != http.StatusBadRequest { |
| + t.Fatalf("quote with currency %q: %d", bad, w.Code) |
| + } |
| + } |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); err == nil { |
| + t.Fatal("an intent was opened for a currency not sold") |
| + } |
| + // On the page, a ?currency= that names something not sold is said, |
| + // and the default it prices in is not a choice. |
| + m, fields := b.page("?country=IE¤cy=jpy") |
| + if !strings.Contains(m, "That is not a currency we sell in.") || fields.Get("currency_chosen") != "" || fields.Get("currency") != "eur" { |
| + t.Fatalf("an unsold currency on the page: %v in:\n%s", fields, m) |
| + } |
| + }) |
| + t.Run("an account that bought before price books is offered its euro alone", func(t *testing.T) { |
| + fx, _, acct := newCurrencyFixture(t) |
| + at := time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC) |
| + legacy := billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_legacy", Status: "canceled", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, Band: plans.BandA, Quantity: 1, PeriodStart: at.Unix(), PeriodEnd: at.AddDate(1, 0, 0).Unix()} |
| + if err := fx.st.applyBillingState(legacy, 0, "e1", at); err != nil { |
| + t.Fatal(err) |
| + } |
| + b := newBillingBrowser(t, fx) |
| + for _, query := range []string{"", "?country=GB¤cy=gbp", "?country=US"} { |
| + m, fields := b.page(query) |
| + if strings.Count(m, `<option value="eur" selected>EUR</option>`) != 1 || strings.Contains(m, `<option value="gbp"`) || fields.Get("currency_chosen") != "1" || fields.Get("currency") != "eur" { |
| + t.Fatalf("%q: %v in:\n%s", query, fields, m) |
| + } |
| + } |
| + }) |
| t.Run("an account that bought in pounds is offered pounds alone", func(t *testing.T) { |
| fx, _, acct, _ := buyIn(t, "GB", "") |
| if _, err := fx.st.db.Exec(`UPDATE billing_accounts SET status='canceled' WHERE account_id=?`, acct.ID); err != nil { |
| @@ -2048,6 +2145,20 @@ func TestCurrencyRefusalExpiresOnlyTheIntentsOwnSession(t *testing.T) { |
| if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.SessionID != "" { |
| t.Fatalf("intent %+v %v; want it kept, unbound, for the next attempt", intent, err) |
| } |
| + // A session carrying the intent's id but another customer is not the |
| + // intent's either: not expired when it is in another currency, and |
| + // not kept when it is in the right one. |
| + for _, currency := range []string{"eur", ""} { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry, f.sessionCurrency, f.sessionCustomer = "GB", currency, "cus_other" |
| + dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now) |
| + if dest != "" || err == nil || f.expireCalls != 0 { |
| + t.Fatalf("another customer, session currency %q: dest %q err %v, %d expiries", currency, dest, err, f.expireCalls) |
| + } |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.SessionID != "" { |
| + t.Fatalf("another customer, session currency %q: intent %+v %v; want it kept, unbound", currency, intent, err) |
| + } |
| + } |
| // The same response for the intent's own session is refused and expired. |
| fx, f, acct = newCurrencyFixture(t) |
| f.customerCountry, f.sessionCurrency = "GB", "eur" |
| @@ -2086,8 +2197,26 @@ func TestBoundSessionCompletedInAnotherCurrencyIsReviewedOnce(t *testing.T) { |
| if _, err := fx.srv.bindBillingCheckout(context.Background(), intent.SessionID); !errors.Is(err, errBillingIdentity) { |
| t.Fatalf("binding the completed session directly: %v, want an identity refusal", err) |
| } |
| + // And through the webhook that announces the completion. |
| + for _, id := range []string{"evt_done_1", "evt_done_2"} { |
| + payload := fmt.Sprintf(`{"id":%q,"type":"checkout.session.completed","created":%d,"livemode":false,"data":{"object":{"id":%q}}}`, id, time.Now().Unix(), intent.SessionID) |
| + ts := time.Now().Unix() |
| + mac := hmac.New(sha256.New, []byte("whsec_test")) |
| + fmt.Fprintf(mac, "%d.%s", ts, payload) |
| + r := httptest.NewRequest(http.MethodPost, "/billing/webhook", strings.NewReader(payload)) |
| + r.Header.Set("Stripe-Signature", fmt.Sprintf("t=%d,v1=%s", ts, hex.EncodeToString(mac.Sum(nil)))) |
| + w := httptest.NewRecorder() |
| + fx.srv.ServeHTTP(w, r) |
| + if w.Code/100 != 2 { |
| + t.Fatalf("webhook %s: %d %s", id, w.Code, w.Body.String()) |
| + } |
| + } |
| if n := reviews(); n != 1 { |
| - t.Fatalf("%d reviews filed, want exactly one after two passes and a binding", n) |
| + t.Fatalf("%d reviews filed, want exactly one after two passes, a binding and two webhooks", n) |
| + } |
| + var occurrences int |
| + if err := fx.st.db.QueryRow(`SELECT occurrences FROM billing_reviews WHERE account_id=? AND reason=?`, acct.ID, billingCheckoutRefusedReason).Scan(&occurrences); err != nil || occurrences != 1 { |
| + t.Fatalf("the review was filed %d time(s) (%v), want once", occurrences, err) |
| } |
| state, _ := fx.st.billingAccount(acct.ID) |
| if state.Subscription != "" || f.expireCalls != 0 || f.sessionStatus != "complete" { |
| @@ -2096,6 +2225,31 @@ func TestBoundSessionCompletedInAnotherCurrencyIsReviewedOnce(t *testing.T) { |
| if after, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || after.ID != intent.ID { |
| t.Fatalf("intent %+v %v; want it left as it was so no second purchase opens", after, err) |
| } |
| + // The same when the metadata changed with the currency: the session is |
| + // still this intent's (its id, account, customer, plan and band), and |
| + // its price terms are not what was asked for. |
| + 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, err = fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + f.mu.Lock() |
| + f.sessionStatus, f.sessionCurrency, f.meta["currency"] = "complete", "usd", "usd" |
| + f.subscriptionPrice, f.subscriptionCurrency = currencyPriceID(plans.PlanPersonal, intent.Band, plans.USD), "usd" |
| + f.mu.Unlock() |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)) |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(2*time.Minute)) |
| + var n int |
| + if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_reviews WHERE account_id=? AND reason=? AND resolved_at=0`, acct.ID, billingCheckoutRefusedReason).Scan(&n); err != nil || n != 1 { |
| + t.Fatalf("dollars throughout, metadata included: %d reviews (%v), want one", n, err) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" || f.expireCalls != 0 { |
| + t.Fatalf("state %+v, %d expiries; want nothing recorded and nothing expired", state, f.expireCalls) |
| + } |
| } |
| // The barrier trigger stops the older build and only the older build: a |
| @@ -2143,6 +2297,68 @@ func TestBarrierTriggerLetsThisBuildsLegacyMonthsThrough(t *testing.T) { |
| t.Fatalf("this build's version 1 for a legacy-only September: %v", err) |
| } |
| }) |
| + t.Run("a book purchase superseded in the same second", func(t *testing.T) { |
| + fx, acct, _ := setup(t) |
| + book := billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_book", Status: "active", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, PriceBook: plans.CurrentPriceBook, Currency: "eur", Band: plans.BandA, Quantity: 1, PeriodStart: at(9, 12).Unix(), PeriodEnd: at(9, 12).AddDate(1, 0, 0).Unix()} |
| + if err := fx.st.applyBillingState(book, 2, "e3", at(9, 12)); err != nil { |
| + t.Fatal(err) |
| + } |
| + book.Status = "canceled" |
| + if err := fx.st.applyBillingState(book, 3, "e4", at(9, 12)); err != nil { |
| + t.Fatal(err) |
| + } |
| + rows, err := fx.st.billingInvoiceHistory(acct.ID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + history := make([]plans.Assignment, 0, len(rows)) |
| + for _, r := range rows { |
| + history = append(history, r.Assignment) |
| + } |
| + if m, err := plans.AllowanceForMonth("2026-09", history); err != nil || !m.PriceTerms().Legacy() { |
| + t.Fatalf("September's terms: %+v %v; want legacy: the active book row's span is empty", m, err) |
| + } |
| + if _, err := fx.st.freezeBillingMonth(acct.ID, "2026-09", v1); err != nil { |
| + t.Fatalf("this build's version 1 after a same-second supersession: %v", err) |
| + } |
| + }) |
| + t.Run("a database that carries the earlier, wider trigger", func(t *testing.T) { |
| + fx, acct, _ := setup(t) |
| + // What a database opened by an earlier build of this branch holds. |
| + for _, stmt := range []string{ |
| + `DROP TRIGGER IF EXISTS billing_months_no_legacy_calc_over_paid_book_history`, |
| + `CREATE TRIGGER billing_months_no_legacy_calc_over_book_history BEFORE INSERT ON billing_months |
| + WHEN CAST(NEW.calculation AS TEXT) NOT LIKE '%"v":2,%' |
| + AND EXISTS (SELECT 1 FROM billing_book_assignments b WHERE b.account_id = NEW.account_id |
| + AND b.effective_at < CAST(strftime('%s', NEW.month || '-01', '+1 month') AS INTEGER)) |
| + BEGIN SELECT RAISE(ABORT, 'billing_month_legacy_calculation_over_book_history'); END`, |
| + } { |
| + if _, err := fx.st.db.Exec(stmt); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + book := billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_book", Status: "canceled", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, PriceBook: plans.CurrentPriceBook, Currency: "eur", Band: plans.BandA, Quantity: 1, PeriodStart: at(9, 12).Unix(), PeriodEnd: at(9, 12).AddDate(1, 0, 0).Unix()} |
| + if err := fx.st.applyBillingState(book, 2, "e3", at(9, 12)); err != nil { |
| + t.Fatal(err) |
| + } |
| + // The earlier trigger read the version by substring, so the bytes |
| + // here carry no nested "v":2, as that build's own never did. |
| + plainV1 := []byte(`{"statement_object":{"key":""},"v":1,"arithmetic":"base_units_floor_v1","currency":"eur","statement":{}}`) |
| + if _, err := fx.st.freezeBillingMonth(acct.ID, "2026-09", plainV1); err == nil { |
| + t.Fatal("fixture: the earlier trigger let the legacy September through") |
| + } |
| + // This build's migration replaces it. |
| + if err := fx.st.migrateBilling(); err != nil { |
| + t.Fatal(err) |
| + } |
| + var names string |
| + if err := fx.st.db.QueryRow(`SELECT group_concat(name) FROM sqlite_master WHERE type='trigger' AND name LIKE 'billing_months_no_legacy_calc%'`).Scan(&names); err != nil || names != "billing_months_no_legacy_calc_over_paid_book_history" { |
| + t.Fatalf("triggers after migration: %q %v", names, err) |
| + } |
| + if _, err := fx.st.freezeBillingMonth(acct.ID, "2026-09", plainV1); err != nil { |
| + t.Fatalf("after migration, this build's version 1: %v", err) |
| + } |
| + }) |
| t.Run("an active book purchase, and a nested version 2", func(t *testing.T) { |
| fx, acct, _ := setup(t) |
| book := billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_book", Status: "active", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, PriceBook: plans.CurrentPriceBook, Currency: "eur", Band: plans.BandA, Quantity: 1, PeriodStart: at(9, 12).Unix(), PeriodEnd: at(9, 12).AddDate(1, 0, 0).Unix()} |
| @@ -2187,6 +2403,35 @@ func TestBillingInclusionsFollowTheQuotedBand(t *testing.T) { |
| } |
| } |
| } |
| + // Why the shares are exact at every quantity: each per-member figure |
| + // times each band factor is whole in base units, so the pooled figure |
| + // is the per-member share times the quantity exactly. |
| + for _, plan := range billingPaidPlans { |
| + def, err := plans.Lookup(plans.CurrentTermsVersion, plan) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, band := range []string{plans.BandA, plans.BandB, plans.BandC, plans.BandD} { |
| + q1, err := plans.Quote(plans.CurrentTermsVersion, plan, band, def.MinQuantity) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, f := range []struct { |
| + name string |
| + per, pooled int64 |
| + }{{"awake", def.ResourcesPerMember.AwakeSeconds, q1.Allowances.AwakeSeconds}, {"storage", def.ResourcesPerMember.StorageByteMonths, q1.Allowances.StorageByteMonths}, {"transfer", def.ResourcesPerMember.TransferBytes, q1.Allowances.TransferBytes}, {"writes", def.ResourcesPerMember.Writes, q1.Allowances.Writes}} { |
| + if f.pooled%def.MinQuantity != 0 { |
| + t.Errorf("%s band %s: %s pooled %d is not a whole multiple of %d members", plan, band, f.name, f.pooled, def.MinQuantity) |
| + } |
| + // The per-member share at the minimum must equal the share at |
| + // any other quantity: that holds exactly when figure x factor |
| + // is whole, which the quote at one member's worth shows. |
| + if share := f.pooled / def.MinQuantity; share*7 != 0 && (f.per*7*share*def.MinQuantity != f.per*7*f.pooled) { |
| + t.Errorf("%s band %s: %s share is not stable across quantities", plan, band, f.name) |
| + } |
| + } |
| + } |
| + } |
| for _, tc := range []struct { |
| num, den int64 |
| want string |
diff --git a/internal/console/billinghandlers.go b/internal/console/billinghandlers.go| index fd2e94466..83fe5ae64 100644 |
| --- a/internal/console/billinghandlers.go |
| +++ b/internal/console/billinghandlers.go |
| @@ -63,19 +63,33 @@ func (s *Server) handleBillingCheckout(w http.ResponseWriter, r *http.Request) { |
| http.Redirect(w, r, back+"?error=unavailable", http.StatusSeeOther) |
| return |
| } |
| - // The form's country and currency must be the ones the page priced |
| - // (the quoted_* fields it carried). Anything else is first shown as a |
| - // quote: nothing is bought at a price the buyer has not seen. |
| - country, currency, chosen := billingFormChoice(r.PostForm) |
| - if country != r.PostForm.Get("quoted_country") || currency != r.PostForm.Get("quoted_currency") { |
| + // A currency field that is present must name a currency sold, exactly |
| + // as the form's control spells it: anything else is a malformed |
| + // request, refused before the quote is looked at and before anything |
| + // is created, and no default stands in (rule 1). |
| + country, currency, chosen, ok := s.billingFormChoice(r.PostForm) |
| + if !ok { |
| + http.Error(w, "currency_invalid", http.StatusBadRequest) |
| + return |
| + } |
| + // What would be bought is the chosen currency or, with none chosen, |
| + // the posted country's own. It and the country must be what the page |
| + // priced (the quoted_* fields it carried); anything else is first |
| + // shown as a quote, so nothing is bought at a price the buyer has not |
| + // seen, whatever the form's fields say about themselves. |
| + effective := currency |
| + if !chosen { |
| + effective = "" |
| + if c, err := plans.CurrencyForCountry(s.billing.cfg.PriceBook, country); err == nil { |
| + effective = string(c) |
| + } |
| + } |
| + if country != r.PostForm.Get("quoted_country") || effective != r.PostForm.Get("quoted_currency") { |
| http.Redirect(w, r, back+billingQuoteQuery(country, currency, chosen), http.StatusSeeOther) |
| return |
| } |
| - // The currency is given to the door only when it is a choice (the |
| - // form says whether the one it shows was chosen or a default). With |
| - // none, the billing country decides. A choice that is not a currency |
| - // sold is a malformed request: nothing is created and no default |
| - // stands in. |
| + // The currency is given to the door only when it is a choice. With |
| + // none, the billing country decides. |
| posted := "" |
| if chosen { |
| posted = currency |
| @@ -123,23 +137,37 @@ func (s *Server) handleBillingQuote(w http.ResponseWriter, r *http.Request) { |
| if !ok { |
| return |
| } |
| - country, currency, chosen := billingFormChoice(r.PostForm) |
| + country, currency, chosen, ok := s.billingFormChoice(r.PostForm) |
| + if !ok { |
| + http.Error(w, "currency_invalid", http.StatusBadRequest) |
| + return |
| + } |
| http.Redirect(w, r, "/a/"+url.PathEscape(acct.Sqid)+"/billing"+billingQuoteQuery(country, currency, chosen), http.StatusSeeOther) |
| } |
| // billingFormChoice reads the billing form's country and currency, and |
| // whether the currency is a choice: the form said the one it showed was |
| // chosen (currency_chosen), or the buyer changed it from the one shown |
| -// (quoted_currency). A form with no currency control (a console selling |
| -// the old way) shows and chooses nothing. |
| -func billingFormChoice(form url.Values) (country, currency string, chosen bool) { |
| +// (quoted_currency). A currency field that is present must be a currency |
| +// sold, spelled as the control spells it (lower case, nothing else); any |
| +// other value, an empty one included, is not ok and nothing is read from |
| +// it. A form with no currency control at all (a console selling the old |
| +// way) shows and chooses nothing. |
| +func (s *Server) billingFormChoice(form url.Values) (country, currency string, chosen, ok bool) { |
| country = strings.ToUpper(strings.TrimSpace(form.Get("country"))) |
| - if _, has := form["currency"]; !has { |
| - return country, form.Get("quoted_currency"), false |
| + values, has := form["currency"] |
| + if !has { |
| + return country, form.Get("quoted_currency"), false, true |
| + } |
| + if len(values) != 1 { |
| + return "", "", false, false |
| + } |
| + currency, ok = s.soldCurrency(values[0]) |
| + if !ok { |
| + return "", "", false, false |
| } |
| - currency = strings.ToLower(strings.TrimSpace(form.Get("currency"))) |
| chosen = form.Get("currency_chosen") == "1" || currency != form.Get("quoted_currency") |
| - return country, currency, chosen |
| + return country, currency, chosen, true |
| } |
| // billingQuoteQuery is the billing page URL's query for a quote: the |
diff --git a/internal/console/billingpage.go b/internal/console/billingpage.go| index b9d662cbf..703bed9a0 100644 |
| --- a/internal/console/billingpage.go |
| +++ b/internal/console/billingpage.go |
| @@ -86,10 +86,13 @@ type billingPageView struct { |
| // ISO2, or "" with none; the form carries it so that a checkout for |
| // another country, or in another currency, is first shown as a quote. |
| // QuoteError is a ?country the page could not price (malformed or not |
| - // a known band). |
| + // a known band). CurrencyError is a ?currency= that names something |
| + // that is not a currency sold: the page says so and prices in the |
| + // default, which is not posted as a choice. |
| Quote *billingQuote |
| QuotedCountry string |
| QuoteError bool |
| + CurrencyError bool |
| // Error is the ?error= key (unavailable|pending|conflict), "" otherwise. |
| Error string |
| @@ -275,14 +278,24 @@ func (s *Server) pageCurrency(r *http.Request, v *billingPageView, country strin |
| } |
| sold := s.billingCurrencies() |
| chosen := "" |
| - if held, err := plans.ParseCurrency(state.Currency); err == nil && state.PriceBook != "" && state.Subscription != "" { |
| - chosen, v.CurrencyChosen, v.CurrencyHeld = string(held), true, true |
| + // An account that bought before buys in the currency it bought in, a |
| + // purchase from before price books included (EUR): the checkout door |
| + // refuses any other (startBillingCheckout), so no other is offered. |
| + if state.Subscription != "" && state.Plan != "" && len(sold) > 0 { |
| + chosen, v.CurrencyChosen, v.CurrencyHeld = string(state.priceTerms().Money()), true, true |
| sold = []string{chosen} |
| + } |
| + if v.CurrencyHeld { |
| + // Nothing in the URL or the cookie changes a held currency. |
| } else if values, ok := r.URL.Query()["currency"]; ok { |
| if len(values) == 1 && values[0] != "" { |
| if c, ok := s.soldCurrency(values[0]); ok { |
| chosen, v.CurrencyChosen = c, true |
| + } else { |
| + v.CurrencyError = true |
| } |
| + } else if len(values) != 1 { |
| + v.CurrencyError = true |
| } |
| } else if c := s.suggestedCurrency(r); c != "" { |
| chosen, v.CurrencyChosen = c, true |
| @@ -389,8 +402,10 @@ func billingPlanRows(terms plans.PriceTerms, band string, members int64) []billi |
| // The inclusions for one member at this band: the pooled allowance |
| // the quote grants at its quantity, per member. What the account |
| // is granted is the pooled figure; the page states its per-member |
| - // share, which is exact for every plan and band in the terms |
| - // (TestBillingInclusionsAreExactPerMember keeps it so). |
| + // share, which is exact for every plan, band and quantity in the |
| + // terms (TestBillingInclusionsFollowTheQuotedBand checks the |
| + // figures' divisibility, so a future figure that is not would fail |
| + // there). |
| row.Hours = formatFigure(price.Allowances.AwakeSeconds, price.Quantity*3600) |
| row.StorageGB = formatFigure(price.Allowances.StorageByteMonths, price.Quantity*1_000_000_000) |
| row.TransferGB = formatFigure(price.Allowances.TransferBytes, price.Quantity*1_000_000_000) |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 876cff0ff..fd3fb2880 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -239,7 +239,7 @@ func (s *Server) checkoutParams(acct *Account, intent billingIntent) billingstri |
| // to a buyer or records a purchase from passes through here, however it |
| // was come by: created, replayed, listed, or read back by its id. |
| func (b *billingRuntime) intentSession(session *billingstripe.Session, intent billingIntent, acct *Account) bool { |
| - if session == nil || session.ID == "" || session.LiveMode != b.cfg.LiveMode || session.Metadata["checkout_intent"] != intent.ID || session.ClientReferenceID != acct.Sqid { |
| + if !b.ownSession(session, intent, acct) { |
| return false |
| } |
| terms, err := priceTermsFromMetadata(session.Metadata) |
| @@ -250,10 +250,18 @@ func (b *billingRuntime) intentSession(session *billingstripe.Session, intent bi |
| } |
| // ownSession reports whether a session is this intent's at all: this |
| -// account's, in this mode, carrying the intent's id. Only such a session is |
| -// ever expired on the intent's behalf. |
| +// account's, in this mode, carrying the intent's id, for the intent's own |
| +// customer (once the intent has one), and, once the intent is bound, the |
| +// very session it is bound to. Only such a session is ever expired, |
| +// offered, bound or recorded on the intent's behalf. |
| func (b *billingRuntime) ownSession(session *billingstripe.Session, intent billingIntent, acct *Account) bool { |
| - return session != nil && session.ID != "" && session.LiveMode == b.cfg.LiveMode && session.Metadata["checkout_intent"] == intent.ID && session.ClientReferenceID == acct.Sqid |
| + if session == nil || session.ID == "" || session.LiveMode != b.cfg.LiveMode || session.Metadata["checkout_intent"] != intent.ID || session.ClientReferenceID != acct.Sqid { |
| + return false |
| + } |
| + if intent.Customer != "" && session.CustomerID != intent.Customer { |
| + return false |
| + } |
| + return intent.SessionID == "" || intent.SessionID == session.ID |
| } |
| // refuseSession deals with a session that is this intent's and is not the |
| @@ -909,16 +917,19 @@ func (s *Server) bindBillingCheckout(ctx context.Context, sessionID string) (str |
| if intent.ID != session.Metadata["checkout_intent"] || intent.Plan != session.Metadata["plan"] || intent.Terms != session.Metadata["terms_version"] || intent.Band != session.Metadata["band"] { |
| return "", errBillingIdentity |
| } |
| - sessionTerms, err := priceTermsFromMetadata(session.Metadata) |
| - if err != nil || sessionTerms != intent.priceTerms() { |
| + if intent.Customer != "" && session.CustomerID != intent.Customer { |
| return "", errBillingIdentity |
| } |
| - if !sessionTerms.Legacy() && !session.FixedCurrency(string(sessionTerms.Currency)) { |
| - // This intent's own session, completed in another currency than it |
| - // was asked for. Nothing is recorded from it, and a person is told |
| - // once, the same as when the background pass finds it unbound |
| - // (refuseSession); the intent stays as it is so no second purchase |
| - // opens beside it. |
| + if sessionTerms, err := priceTermsFromMetadata(session.Metadata); err != nil || sessionTerms != intent.priceTerms() || (!sessionTerms.Legacy() && !session.FixedCurrency(string(sessionTerms.Currency))) { |
| + // This intent's own session (its id, account, customer, plan and |
| + // band), completed on other price terms or in another currency |
| + // than it was asked for. Nothing is recorded from it, and a person |
| + // is told, the same as when the background pass finds it unbound |
| + // (refuseSession): one open review, however many passes, webhooks |
| + // or bindings meet it; the intent stays as it is so no second |
| + // purchase opens beside it. (A resolved review is reopened by the |
| + // next pass while the intent stands: retiring such an intent is an |
| + // operator's door that does not exist yet.) |
| if err := s.store.fileBillingReviewOnce(acct.ID, session.SubscriptionID, billingCheckoutRefusedReason, time.Now()); err != nil { |
| return "", err |
| } |
diff --git a/internal/console/billingservice_test.go b/internal/console/billingservice_test.go| index 00b40e8b1..2851af279 100644 |
| --- a/internal/console/billingservice_test.go |
| +++ b/internal/console/billingservice_test.go |
| @@ -43,6 +43,9 @@ type fakeBillingStripe struct { |
| // overrides what a session reports, for a provider that answers in |
| // another currency than it was asked for. |
| currency, sessionCurrency string |
| + // sessionCustomer, when set, is the customer the session names in |
| + // place of the one the intent was opened for. |
| + sessionCustomer string |
| // prices are extra price objects, by id, beside price_personal. |
| prices map[string]map[string]any |
| subscriptionPrice, subscriptionCurrency string |
| @@ -130,6 +133,9 @@ func (f *fakeBillingStripe) session() map[string]any { |
| if f.paymentStatus != "" { |
| payment = f.paymentStatus |
| } |
| + if f.sessionCustomer != "" { |
| + customer = f.sessionCustomer |
| + } |
| currency := f.money() |
| if f.sessionCurrency != "" { |
| currency = f.sessionCurrency |
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go| index 27ae45fac..1901f0722 100644 |
| --- a/internal/console/billingstore.go |
| +++ b/internal/console/billingstore.go |
| @@ -176,22 +176,34 @@ func (s *Store) migrateBilling() error { |
| // which an account held a paid, book-priced subscription, that is |
| // a month priced without part of its history. The database itself |
| // refuses it: no calculation whose top-level version is not 2 may |
| - // be frozen for a month in which a book-priced assignment of the |
| - // account was active and paid for (it began before the month |
| - // ended and was paid through past the month's start). A book |
| - // assignment that never counted for the month, a purchase |
| - // cancelled before it was first reconciled, say, is not history |
| - // the month was priced from (plans.AllowanceForMonth skips it), |
| - // and a legacy-only month after it is this build's own version 1. |
| - // The version is read with json_extract, not by substring: a |
| - // statement nested in a version 1 calculation may one day carry a |
| - // version 2 of its own. |
| - `CREATE TRIGGER IF NOT EXISTS billing_months_no_legacy_calc_over_book_history BEFORE INSERT ON billing_months |
| + // be frozen for a month into which a book-priced assignment of the |
| + // account was priced, which is exactly plans.AllowanceForMonth's |
| + // rule: the assignment is active and not free, and its span, from |
| + // its effective instant to the earliest of the next assignment's |
| + // effective instant (by time, then revision, over both assignment |
| + // tables), its paid-through instant and the month's end, has |
| + // length inside the month. A book assignment that never counted |
| + // (a purchase cancelled before it was first reconciled, or |
| + // superseded in the same second) is not history the month was |
| + // priced from, and a legacy-only month after it is this build's own |
| + // version 1. The version is read with json_extract, not by |
| + // substring. Earlier builds of this branch created a wider trigger |
| + // under the name billing_months_no_legacy_calc_over_book_history; |
| + // it is dropped first, so a database they opened gets this one. |
| + `DROP TRIGGER IF EXISTS billing_months_no_legacy_calc_over_book_history`, |
| + `CREATE TRIGGER IF NOT EXISTS billing_months_no_legacy_calc_over_paid_book_history BEFORE INSERT ON billing_months |
| WHEN (CASE WHEN json_valid(CAST(NEW.calculation AS TEXT)) THEN json_extract(CAST(NEW.calculation AS TEXT), '$.v') IS NOT 2 ELSE 1 END) |
| AND EXISTS (SELECT 1 FROM billing_book_assignments b WHERE b.account_id = NEW.account_id |
| AND b.status = 'active' AND b.plan <> 'free' |
| - AND b.effective_at < CAST(strftime('%s', NEW.month || '-01', '+1 month') AS INTEGER) |
| - AND b.period_end > CAST(strftime('%s', NEW.month || '-01') AS INTEGER)) |
| + AND MAX(b.effective_at, CAST(strftime('%s', NEW.month || '-01') AS INTEGER)) |
| + < MIN(b.period_end, |
| + CAST(strftime('%s', NEW.month || '-01', '+1 month') AS INTEGER), |
| + COALESCE((SELECT MIN(n.effective_at) FROM ( |
| + SELECT effective_at, revision FROM billing_assignments WHERE account_id = b.account_id |
| + UNION ALL |
| + SELECT effective_at, revision FROM billing_book_assignments WHERE account_id = b.account_id) n |
| + WHERE n.effective_at > b.effective_at OR (n.effective_at = b.effective_at AND n.revision > b.revision)), |
| + CAST(strftime('%s', NEW.month || '-01', '+1 month') AS INTEGER)))) |
| BEGIN SELECT RAISE(ABORT, 'billing_month_legacy_calculation_over_book_history'); END`, |
| } { |
| if _, err := s.db.Exec(stmt); err != nil { |
diff --git a/internal/console/templates/billing.html b/internal/console/templates/billing.html| index 0c5a027d2..95a37d982 100644 |
| --- a/internal/console/templates/billing.html |
| +++ b/internal/console/templates/billing.html |
| @@ -71,6 +71,7 @@ |
| <p class="small"><!-- country.explanation (currency review) -->Your billing country sets your pricing band. Changing currency does not change your inclusions.</p> |
| {{end}} |
| {{if .QuoteError}}<p class="small"><!-- billing.quote.unknown (not in strings.json) -->There are no prices for that country. Choose one from the list.</p>{{end}} |
| +{{if .CurrencyError}}<p class="small"><!-- billing.currency.unknown (not in strings.json) -->That is not a currency we sell in. Choose one from the list.</p>{{end}} |
| {{with .Quote}}<p class="small"><!-- billing.quote.band -->Prices for {{.Country}} are {{.Percent}}% of the base price, and so are the monthly inclusions.</p>{{end}} |
| <fieldset class="billing-plan-options" aria-labelledby="billing-choose-heading"> |
| {{range .Plans}} |
diff --git a/internal/plans/provision_test.go b/internal/plans/provision_test.go| index d8e9a1a2a..37f064210 100644 |
| --- a/internal/plans/provision_test.go |
| +++ b/internal/plans/provision_test.go |
| @@ -4,6 +4,7 @@ import ( |
| "os" |
| "regexp" |
| "strconv" |
| + "strings" |
| "testing" |
| ) |
| @@ -19,6 +20,22 @@ func TestStripeProvisionScriptMatchesThePriceBook(t *testing.T) { |
| t.Fatalf("the provisioning script is not beside this package: %v", err) |
| } |
| script := string(raw) |
| + // The verification the script prints applies the console's own rules: |
| + // the book's 36 prices are counted among whatever else is active, the |
| + // configured map may hold more than the 36 keys, and a price may carry |
| + // its own currency as its only currency option. |
| + for _, want := range []string{ |
| + `select(.metadata.price_book == \$book)] | length) == 36`, |
| + `-ge 36 ]`, |
| + `to_entries | all(.key == $currency and .value.unit_amount == $cents`, |
| + } { |
| + if !strings.Contains(script, want) { |
| + t.Errorf("the script's verification lacks %q", want) |
| + } |
| + } |
| + if strings.Contains(script, `(.data | length) == 36`) || strings.Contains(script, `((.currency_options // {}) | length) == 0`) { |
| + t.Error("the script still demands exactly 36 active prices, or no currency options at all") |
| + } |
| book := regexp.MustCompile(`(?m)^readonly BOOK="([^"]+)"`).FindStringSubmatch(script) |
| if book == nil || book[1] != CurrentPriceBook { |
| t.Fatalf("the script provisions book %v; the console sells from %s", book, CurrentPriceBook) |