amadan
Multi-currency: the second review's findings
A database trigger refuses to freeze a version 1 calculation for a month a book-priced assignment begins in or before, so a build from before price books cannot price the legacy half of a mixed history in EUR (Astra P1). A bound session that reads back in another currency is expired and its intent retired, on the resume paths and by the background pass (Astra P2); one already paid is left for a person with a review filed once, not asked to expire on every pass (Opus P3-1). A price's own currency among its expanded currency options is accepted and any other refused (Opus P2-1). The console's book list is a seam so two books' catalogs are tested. Keys differing only in case are duplicates. The golden tail asserts exact bytes. The provisioning plan and script carry the 36 prices with book and currency metadata, refuse live mode on a draft book, and a test pins the script's table to the embedded book.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
16 files changed,
+837
−121
cmd/console/billing.go+13 −4cmd/console/billing_test.go+82 −3docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md+4 −4docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review2.md+120 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+29 −1docs/superpowers/specs/OPUS-REVIEW-multi-currency-review2.md+112 −0implementation/carloku-paid-launch/stripe-provision.sh+72 −43implementation/carloku-paid-launch/stripe-provisioning.md+58 −35internal/console/billingcurrency_test.go+136 −10internal/console/billingpass.go+11 −0internal/console/billingservice.go+94 −20internal/console/billingservice_test.go+17 −0internal/console/billingstore.go+15 −0internal/plans/pricebook.go+5 −1internal/plans/pricebook_test.go+5 −0internal/plans/provision_test.go+64 −0
diff --git a/cmd/console/billing.go b/cmd/console/billing.go| index a7afb9e90..c5f97932f 100644 |
| --- a/cmd/console/billing.go |
| +++ b/cmd/console/billing.go |
| @@ -11,6 +11,15 @@ import ( |
| var errBillingConfiguration = errors.New("console: billing_configuration_invalid") |
| +// The price books this build has. Variables so a test can configure a |
| +// console over books that are not embedded (a book that has been |
| +// superseded, say); production reads the embedded books. |
| +var ( |
| + priceBooks = plans.PriceBooks |
| + bookCurrencies = plans.BookCurrencies |
| + bookIsApproved = plans.BookIsApproved |
| +) |
| + |
| func billingConfig(getenv func(string) string) (*console.BillingConfig, error) { |
| mode := getenv("CONSOLE_BILLING_MODE") |
| if mode == "" || mode == "off" { |
| @@ -95,10 +104,10 @@ func billingConfig(getenv func(string) string) (*console.BillingConfig, error) { |
| // not been approved can be configured in test mode and never in live |
| // mode. |
| cfg.PriceBook = getenv("CONSOLE_BILLING_PRICE_BOOK") |
| - if _, err := plans.BookCurrencies(cfg.PriceBook); cfg.PriceBook == plans.LegacyPriceBook || err != nil { |
| + if _, err := bookCurrencies(cfg.PriceBook); cfg.PriceBook == plans.LegacyPriceBook || err != nil { |
| return nil, errBillingConfiguration |
| } |
| - if approved, err := plans.BookIsApproved(cfg.PriceBook); err != nil || (cfg.LiveMode && !approved) { |
| + if approved, err := bookIsApproved(cfg.PriceBook); err != nil || (cfg.LiveMode && !approved) { |
| return nil, errBillingConfiguration |
| } |
| // One provider price for every plan, band and currency the book sells |
| @@ -127,7 +136,7 @@ func billingConfig(getenv func(string) string) (*console.BillingConfig, error) { |
| sold := []plans.Currency{""} |
| if book != plans.LegacyPriceBook { |
| var err error |
| - if sold, err = plans.BookCurrencies(book); err != nil { |
| + if sold, err = bookCurrencies(book); err != nil { |
| return false |
| } |
| } |
| @@ -165,7 +174,7 @@ func billingConfig(getenv func(string) string) (*console.BillingConfig, error) { |
| used += len(keys) |
| return true |
| } |
| - for _, book := range plans.PriceBooks() { |
| + for _, book := range priceBooks() { |
| if !catalog(book, book == cfg.PriceBook) { |
| return nil, errBillingConfiguration |
| } |
diff --git a/cmd/console/billing_test.go b/cmd/console/billing_test.go| index fb6452822..e7eda319e 100644 |
| --- a/cmd/console/billing_test.go |
| +++ b/cmd/console/billing_test.go |
| @@ -97,6 +97,85 @@ func TestBillingConfigExplicitModeAndCompleteCatalog(t *testing.T) { |
| // The price book has to be named and known, and the price map has to hold |
| // exactly one price for every plan, band and currency of it, with the |
| // twelve keys from before books all present or all absent. |
| +// A subscription stays on the book it was bought under, so when a new |
| +// book is published the console must still know the old one's prices. |
| +// Two embedded books, through the test seam: the current one's catalog is |
| +// required, the other's is taken whole or not at all, and no price may |
| +// serve two books. |
| +func TestBillingConfigKeepsCatalogsOfOtherBooks(t *testing.T) { |
| + const old = "2026-10-01-draft1" |
| + const current = "2027-01-01-v2" |
| + defer func(a func() []string, b func(string) ([]plans.Currency, error), c func(string) (bool, error)) { |
| + priceBooks, bookCurrencies, bookIsApproved = a, b, c |
| + }(priceBooks, bookCurrencies, bookIsApproved) |
| + priceBooks = func() []string { return []string{old, current} } |
| + bookCurrencies = func(book string) ([]plans.Currency, error) { |
| + switch book { |
| + case old: |
| + return []plans.Currency{plans.USD, plans.EUR, plans.GBP}, nil |
| + case current: |
| + return []plans.Currency{plans.USD, plans.EUR}, nil |
| + } |
| + return nil, plans.ErrUnknownPriceBook |
| + } |
| + bookIsApproved = func(book string) (bool, error) { return book == current, nil } |
| + catalog := func(book string, currencies ...string) map[string]string { |
| + out := map[string]string{} |
| + for _, plan := range []string{"personal", "community", "business"} { |
| + for _, band := range []string{"A", "B", "C", "D"} { |
| + for _, c := range currencies { |
| + out[book+"/"+plan+"/"+band+"/"+c] = "price_" + book + "_" + plan + band + c |
| + } |
| + } |
| + } |
| + return out |
| + } |
| + run := func(mode string, maps ...map[string]string) error { |
| + env := billingEnvFixture(t) |
| + env["CONSOLE_BILLING_MODE"], env["CONSOLE_BILLING_PRICE_BOOK"] = mode, current |
| + if mode == "live" { |
| + env["CARLOS_STRIPE_SECRET_KEY"] = "rk_live_secret_must_not_escape" |
| + } |
| + prices := map[string]string{} |
| + for _, m := range maps { |
| + for k, v := range m { |
| + prices[k] = v |
| + } |
| + } |
| + raw, _ := json.Marshal(prices) |
| + env["CONSOLE_STRIPE_PRICES"] = string(raw) |
| + _, err := billingConfig(func(key string) string { return env[key] }) |
| + return err |
| + } |
| + if err := run("test", catalog(current, "usd", "eur")); err != nil { |
| + t.Fatalf("the current book alone: %v", err) |
| + } |
| + if err := run("test", catalog(current, "usd", "eur"), catalog(old, "usd", "eur", "gbp")); err != nil { |
| + t.Fatalf("the current book and the old one whole: %v", err) |
| + } |
| + if err := run("live", catalog(current, "usd", "eur"), catalog(old, "usd", "eur", "gbp")); err != nil { |
| + t.Fatalf("live mode on an approved current book: %v", err) |
| + } |
| + partial := catalog(old, "usd", "eur", "gbp") |
| + delete(partial, old+"/community/C/gbp") |
| + for name, maps := range map[string][]map[string]string{ |
| + "the old book alone": {catalog(old, "usd", "eur", "gbp")}, |
| + "the old book partly": {catalog(current, "usd", "eur"), partial}, |
| + "the current book partly": {catalog(current, "usd")}, |
| + "a price serving both books": {catalog(current, "usd", "eur"), func() map[string]string { |
| + m := catalog(old, "usd", "eur", "gbp") |
| + m[old+"/personal/A/usd"] = "price_" + current + "_personalAusd" |
| + return m |
| + }()}, |
| + "a key for a book this build lacks": {catalog(current, "usd", "eur"), catalog("2031-01-01-v9", "usd")}, |
| + "a currency the current book lacks": {catalog(current, "usd", "eur", "gbp")}, |
| + } { |
| + if err := run("test", maps...); !errors.Is(err, errBillingConfiguration) { |
| + t.Errorf("%s: %v, want a configuration error", name, err) |
| + } |
| + } |
| +} |
| + |
| func TestBillingConfigPriceBookAndCatalog(t *testing.T) { |
| config := func(edit func(env map[string]string, prices map[string]string)) error { |
| env := billingEnvFixture(t) |
| @@ -148,9 +227,9 @@ func TestBillingConfigPriceBookAndCatalog(t *testing.T) { |
| } |
| }, |
| // With no book named, the keys an empty book name would call for: |
| - // twelve of them, and with the legacy twelve as well. Nothing but |
| - // the explicit refusal of an empty book stands in the way of |
| - // these, in live mode too. |
| + // twelve of them, and with the legacy twelve as well. The count of |
| + // used keys refuses these two; the explicit refusal of an empty |
| + // book is what refuses a legacy-only map with no book (below). |
| "no book, and keys under an empty book name": func(env map[string]string, prices map[string]string) { |
| delete(env, "CONSOLE_BILLING_PRICE_BOOK") |
| for k := range prices { |
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 5d55e63f6..57c9f43a9 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 |
| @@ -140,17 +140,17 @@ func PublicPriceBook(book string) ([]byte, error) // the export below, byte-stab |
| `Quote`, `CalculateOverage`, `InvoiceOverageLines` and the `MicroEUR` types stay as they are, for legacy rows and for snapshots already written. The new functions give the same numbers for legacy terms. |
| -`Assignment` gains `Currency` and `PriceBook` (both `omitempty`, so an existing snapshot marshals byte for byte as before). `AllowanceForMonth` copies them to `MonthAllowance` and refuses, with `ErrAssignmentHistory`, a month whose paid spans disagree on either. |
| +`Assignment` gains `Currency` and `PriceBook` (both `omitempty`, so an existing snapshot marshals byte for byte as before). `AllowanceForMonth` copies them to `MonthAllowance` and refuses, with `ErrPriceTerms`, a month whose paid spans disagree on either or name terms this build does not have; the invoice engine files a review for such a month rather than retrying it quietly. |
| ## 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 and cannot price its months in EUR. |
| +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 version 1 calculation for any month a book-priced assignment of the account begins in or before. 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`): a new optional form field `currency`. Absent or empty: the country's default. Present and 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. |
| - **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. 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`, 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 `adaptive_pricing[enabled]=false`, and with `currency`, `price_book` in the session and subscription metadata. The session that comes back, and the subscription reconciled later, must be in the intent's currency; a subscription whose price is not the configured one for its stored (book, plan, band, currency) is an identity mismatch, as a wrong price is today. |
| -- **Configuration**: `CONSOLE_BILLING_PRICE_BOOK` names the book and is required. `CONSOLE_STRIPE_PRICES` holds 36 entries keyed `"<price_book>/<plan>/<band>/<currency>"`. The twelve legacy keys `"<terms>/<plan>/<band>"` may be given as well, all or none, and are used only to reconcile legacy subscriptions. Live mode with a draft book is refused. |
| +- **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. |
| - **Monthly calculation**: a month priced from a book is a version 2 snapshot: `currency`, `price_book`, and amounts under currency-neutral names. A version 1 snapshot is EUR, is read as it was written, and is never priced again. A stored calculation whose currency is not the month's assignment currency is refused. |
| - **Overage invoice**: the invoice and every invoice item are created in the calculation's currency and verified against it on every read. The currency is part of each request's parameters and so of its request hash; legacy EUR requests hash as they do today. |
| - **Quantity worker**: reads the price for the account's stored (book, plan, band, currency). |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review2.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review2.md| new file mode 100644 |
| index 000000000..7416ab0f4 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review2.md |
| @@ -0,0 +1,120 @@ |
| +All references below are at `a88a39e6`. This was a source-only review against `12e70d95`; no files were modified, tests run, or network commands used. The requested delta `a88a39e6..a88a39e6` is empty. The stored first-review reports reviewed `9d104f1d`. |
| + |
| +**First-review findings, individually** |
| + |
| +“Would fail” below is a conclusion from reading the test and implementation, not an executed mutation result. |
| + |
| +| First-review finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| Astra P1 / Opus P2-1: rejected checkout recovered and offered | **Original recovery hole closed; bound-session cleanup incomplete.** Creation, replay and listing validate currency. See finding 2 below. | Removing the recovery fixes would fail the new tests. Removing either resume check separately survives the existing fixed-response fixture. | |
| +| Astra P1: old console freezes book purchases as EUR | **Partially.** Pure book history is hidden, but mixed legacy/book history remains unsafe. See finding 1. | The test at `internal/console/billingcurrency_test.go:1303` would fail without the table split, but passes despite the remaining mixed-history defect. | |
| +| Astra P2: historical books cannot retain catalogs | **Implementation closed**, `cmd/console/billing.go:125`, `:168`. | **Yes.** No test supplies two nonlegacy books. See finding 3. | |
| +| Astra P2 / Opus P3-3: item currency and quantity-update response unchecked | **Closed**, `internal/console/billinginvoice.go:576`; `internal/console/billingquantity.go:1105`. | No. The separate item-currency and update-response cases would fail. | |
| +| Astra P2 / Opus P3-4: validation overflow, unknown countries, duplicate keys | **Closed**, `internal/plans/pricebook.go:151`, `:262`, `:302`. | No. The new validation cases independently exercise these defects. | |
| +| Opus P2-2: checkout after an existing purchase changes currency | **Closed on the current checkout path**, `internal/console/billingservice.go:414`, `:424`. | The canceled-subscription cases would fail without the currency guard. The active-subscription portion alone passes without the new service guard because the store already refuses it. | |
| +| Opus P2-3: mixed-price-terms month retries silently | **Closed**, `internal/plans/month.go:179`; `internal/console/billinginvoice.go:320`. | No. The test requires a durable review and no frozen month. | |
| +| Astra P3: legacy snapshot test shares changed nested types | **Closed for actual pre-change snapshots**, `internal/console/billingcurrency_test.go:1152`, `:1161`. | The literal golden would catch the former `omitempty` blind spot. The older reconstructed-type comparison remains weaker. | |
| +| Astra P3: three overage rates lack independent expectations | **Closed**, `internal/plans/pricebook_test.go:280`. | Substituting EUR rates for USD/GBP would fail the handwritten expectations. | |
| +| Opus P3-1: changing customer currency strands historical overages | **Prevented by the current checkout restriction.** Customer currency itself remains unchecked. | No dedicated customer-currency test; the canceled-purchase checkout tests cover the restriction. | |
| +| Opus P3-2: rollback requires coordinated configuration | **Still open as rollout work**, and the stronger rollback-safety claim is incorrect. | No rollout test. | |
| +| Opus P3-5 / Astra: free-plan allowance documentation | **Closed**, `internal/plans/pricebook.go:716`; design line 199. | No documentation regression test; export tests would pass with the former prose. | |
| +| Opus P3-6: subscription and item checks mask one another | **Closed**, `internal/console/billingcurrency_test.go:1362`. | Removing either check would fail its independently varied case. | |
| +| Opus P3-7: legacy non-EUR subscriptions are refused | **Accepted behavior, not removed.** `billingCurrencyAgrees` accepts legacy EUR or an absent reported currency, not USD/GBP. | `TestLegacySubscriptionStaysEURUnderAConsoleWithABook` would fail if non-EUR subscriptions were accepted. | |
| +| Astra claim: invoice recovery acknowledges unchecked objects | **Closed for recovered draft/line currency**, `internal/console/billinginvoice.go:940`, `:1024`. | No. The test inspects the operation’s acknowledgement, so later finalization checks cannot mask the omission. | |
| +| Astra claim: legacy checkout replay changes parameters | **Closed**, `internal/console/billingservice.go:190`; `internal/billingstripe/client.go:606`. | The legacy HTTP-form tests would fail without the compatibility behavior. | |
| +| Opus claim: invalid currency checked too late | **Closed**, `internal/console/billingservice.go:297`. | No. Invalid-country, invalid-plan and missing-postal cases distinguish the early check. | |
| +| Opus claim: empty-book guard is redundant | **Corrected.** The guard is necessary. | The new **legacy-only map with no book** case would fail without it. The `/personal/A/eur` cases now also fail later catalog validation and do not independently prove this guard. | |
| +| Opus claim: cookie applies beyond sign-in entry points | **Documentation corrected.** Any GET may set the suggestion. | No implementation fix was needed. | |
| +| Opus claim: new currency-less rows remain possible | **Accepted compatibility exception**, enforced at the command configuration boundary rather than the store. | Store/demo tests deliberately permit them; command configuration tests reject a missing book. | |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P1 — The assignment-table split does not make rollback safe for mixed history.** |
| + |
| + **Where:** `internal/console/billingstore.go:237`, `:665`, `:668`; `internal/console/billingcurrency_test.go:1303`. |
| + |
| + **Scenario:** A legacy EUR subscription ends on September 10. The account purchases a book-priced EUR subscription on September 12, which the current checkout permits. Roll back the console and its configuration before September is frozen. |
| + |
| + The older history query still finds the first subscription’s active and canceled assignments in `billing_assignments`. It therefore has a valid partial-month history—not an empty history. It cannot see the September 12 purchase in `billing_book_assignments`, so it freezes September using only the earlier paid days. With no excess on those days, it can waive the entire account-month despite substantial usage after September 12. |
| + |
| + Returning to this build does not repair that omission: terminal months return immediately (`internal/console/billinginvoice.go:327`). Even a nonterminal snapshot is checked against the assignments it contains, not for omitted later paid spans (`:509`). |
| + |
| + The rollback test uses a fresh account with exclusively book-priced history, so it misses this case. |
| + |
| + **Smallest acceptable fix:** Enforce a compatibility barrier that prevents older billing readers and writers from operating once book purchases exist. Alternatively, make the schema enforce that barrier. Add a legacy→book purchase within one month and prove the older queries cannot produce a billable partial history or freeze the month. |
| + |
| +2. **P2 — A bound checkout detected as invalid remains payable through its previously issued URL.** |
| + |
| + **Where:** `internal/console/billingservice.go:467`, `:514`; `internal/console/billingpass.go:187`; `internal/console/billingstore.go:374`. |
| + |
| + **Scenario:** A valid GBP session is created and its URL reaches the buyer. On a subsequent read, the provider reports EUR, conversion, Adaptive Pricing, or incompatible price terms. The resume path returns `errBillingIdentity` but never expires the session. The background pass likewise ignores an open bound session. The buyer still has the original hosted URL and can complete a purchase the console will subsequently refuse. |
| + |
| + The new test explicitly models a bound session changing currency, but checks only that the second click returns no destination (`internal/console/billingcurrency_test.go:335`). It does not require provider expiry. |
| + |
| + **Smallest acceptable fix:** For a session proven to belong to this intent, expire an invalid open bound session and retire it using the bound-session retirement path. `retireRefusedCheckout` currently rejects bound intents. Apply the same validation to background reads and test that the previously issued session becomes unpayable. |
| + |
| +3. **P3 — Historical-catalog support still lacks the regression test claimed for it.** |
| + |
| + **Where:** `cmd/console/billing_test.go:100`; `cmd/console/billing.go:168`. |
| + |
| + **Scenario:** Revert catalog validation to accepting only the selected book plus optional legacy keys. Every current configuration fixture still fits that restriction: none supplies two nonlegacy books. The suite therefore does not protect the fix needed when the next book is published. |
| + |
| + **Smallest acceptable fix:** Test catalog validation with two known nonlegacy books: both complete, one historical catalog partial, and one provider price reused across books. Use an injectable book catalog or another suitable test seam. |
| + |
| +4. **P3 — The surviving checkout mutations are not all redundant.** |
| + |
| + **Where:** `docs/superpowers/specs/DISPOSITIONS-multi-currency.md:45`; `internal/console/billingcurrency_test.go:335`; `internal/billingstripe/client.go:644`. |
| + |
| + **Scenario:** The first resume GET returns valid GBP; the second GET returns EUR. Removing the second validation at `internal/console/billingservice.go:514` now exposes the saved URL. The existing fake returns the same currency on both reads, masking this distinction. |
| + |
| + Separately, a created session with valid GBP, a valid Stripe URL, but another intent’s metadata passes `CreateCheckout`: that client checks response shape, URL and currency, **not ownership**. The service’s unrelated-session refusal is therefore reachable and necessary. |
| + |
| + **Smallest acceptable fix:** Add sequential-response fixtures and a same-currency/wrong-intent creation response. Retain the guards and correct the disposition’s redundancy claim. |
| + |
| +**Checks with no additional finding** |
| + |
| +The ordinary GBP purchase carries its choice through checkout intent, account state, assignments, monthly allowance, version 2 calculation, draft and item requests, invoice verification, finalization and payment. Reconciliation and the quantity worker check subscription and item currency; the quantity-update response now checks price identity and currency. |
| + |
| +Explicit unsupported currencies are refused. Checkout reads `PostForm`, so cookies and query parameters do not directly select purchase currency. The embedded defaults match USD internationally, EUR for the listed EU states and GBP for GB. Unknown countries default to USD in the currency API; checkout separately requires a known affordability band. |
| + |
| +Version 1 calculations retain their serialized field order and omit the new fields. Legacy draft/line hashes omit the appended currency parameter, while their HTTP requests still send the same EUR value as the base. Valid historical rows remain EUR; a legacy provider subscription reporting another currency is deliberately refused. |
| + |
| +`validateBook` now rejects the identified overflow, rounding, EUR-parity, unsupported-currency, duplicate-key and ambiguous-country cases. Runtime price and overage arithmetic uses checked integer operations, with the legacy EUR rounding and line allocation preserved. Currency does not affect resources, quantities, tax-code selection or entitlement publication. |
| + |
| +Command configuration rejects missing books, a selected draft book in live mode, incomplete catalogs and reused provider-price IDs. Migration now adds **four default-empty columns to existing tables**, plus two required nonempty columns in the new assignment table—not six default-empty additions. Startup runs migration before normal store use. |
| + |
| +The public export contains no customer, credential or provider identifiers. Its minor-unit values, free prices and `band_scaled` allowance rule are consistent with the implementation, including GBP Community B at 338 pence and D at 158 pence. |
| + |
| +**Claims the code contradicts** |
| + |
| +- **“Every session … passes one check” and invalid sessions are expired** — dispositions line 24 overstates the implementation. Finding 2 covers bound open sessions. Additionally, `bindBillingCheckout` returns early for an already-recorded subscription before checking session price terms or currency (`internal/console/billingservice.go:822`). |
| + |
| +- **Rollback “removes the mispricing”** — dispositions lines 25 and 40, and the design at line 147, omit mixed legacy/book history. The shared account and intent tables also remain writable by older binaries; separating assignment storage is not a general downgrade barrier. |
| + |
| +- **“Before the provider is asked”** — dispositions line 26 and the test comment at `internal/console/billingcurrency_test.go:1200` are inaccurate. Merchant, price, product, tax-registration and portal reads occur before the subscription guard. The test counts checkout creations, not all provider requests. |
| + |
| +- **The historical-catalog fix is covered by its named test** — dispositions line 28 is unsupported; see finding 3. |
| + |
| +- **The three post-fix mutations are redundant** — dispositions line 45 is incorrect; see finding 4. The earlier two intent-currency comparisons are redundant on the normal path under a validated unique-price catalog, but that does not independently establish the store invariant. |
| + |
| +- **The explicit-EUR golden-test tail proves identical bytes** — `internal/console/billingcurrency_test.go:1184` does not enforce its comment. Its fallback substring check already matches the calculation’s top-level `"currency":"eur"`. The real pre-change golden comparison remains useful and independent. |
| + |
| +- **Design details are stale:** line 143 names `ErrAssignmentHistory` instead of `ErrPriceTerms`; lines 153 and 210 describe only 36/48-entry catalogs despite historical-book support; line 206 still calls these “the six columns.” The blanket Adaptive Pricing statements also need the deliberate legacy-replay exception. |
| + |
| +The new fixtures use wall-clock authentication, checkout and quantity timing. I found no concrete calendar- or test-order-dependent failure in the reviewed changes. |
| + |
| +**What must be verified against Stripe test mode before enabling** |
| + |
| +1. **Catalog and fixed checkout:** Inspect all 36 prices at the pinned API version, including expanded `currency_options`, amounts, recurrence, tax behavior and metadata. Create, retrieve and complete sessions in all three currencies with differing billing countries. Confirm fixed displayed amounts, disabled Adaptive Pricing, no conversion, and matching subscription/item currencies. Deliberately mismatched price/session currencies must be refused. |
| + |
| +2. **Monthly overages:** Exercise all four meters in GBP and USD through draft, items, finalization and payment, including canceled subscriptions, floors and rounding boundaries. Confirm every currency, subtotal, tax result and final total against the frozen calculation. |
| + |
| +3. **Recovery:** Lose responses after checkout, draft, item, finalize and pay operations, both inside and beyond the replay window. Confirm recovery uses the original objects without duplicate charges. Include legacy checkout requests and version 1 invoice requests carrying their original keys and hashes. |
| + |
| +4. **Quantity changes and renewals:** Exercise both proration policies and renewal. Inspect actual prorated/upcoming invoices and their lines. The worker does not read those amounts or currencies (`internal/console/billingquantity.go:1173`). Run `TestSandboxQuantityFence` before enabling its switch. |
| + |
| +5. **Customer currency:** Inspect customer currency during initial purchase, cancellation, repurchase and outstanding historical invoices/items. `Customer` does not decode currency (`internal/billingstripe/customer.go:79`); local checks therefore do not establish customer-currency compatibility. |
| + |
| +6. **Legacy Adaptive Pricing and discounts:** Complete an old-style adaptive session and inspect its subscription currency. Exercise percentage and fixed-amount promotion codes across currencies, proving that incompatible discounts cannot alter the fixed-currency contract. |
| + |
| +**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 85d3b497f..b2c6fb97b 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -42,6 +42,34 @@ Reports: `OPUS-REVIEW-multi-currency-review1.md`, `CODEX-FINDINGS-multi-currency |
| | Opus, claim 6: the cookie is set on any GET, not only sign-in entry points. | Accepted; the design now says so. | | | |
| | Opus, claim 3: a console with no book writes rows with no currency. | Accepted: only tests and the demo run without a book; `cmd/console` requires one. | | | |
| -Mutation checks after these fixes, twenty-five in a scratch copy: twenty-two failed their tests. Three survived: the two checks of a bound session when it is resumed (each is the other's second fence in the same function, and one of them always refuses); and the refusal of a created session that is not this intent's and not expirable (unreachable through the client, which refuses such a session first). |
| +Mutation checks after these fixes, twenty-five in a scratch copy: twenty-two failed their tests. Three survived, and review two showed none was redundant: the two checks of a bound session when it is resumed are each the fence for a reading the other did not see (the fakes answered the same on both reads); the refusal of a created session that is not this intent's is reachable, because the client checks a session's currency and not whose it is. All three have cases now (review two, below). |
| The fixes are in the commit that adds this section and have not been read by a reviewer. |
| + |
| +## Review two (at a88a39e6; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-multi-currency-review2.md`, `CODEX-FINDINGS-multi-currency-review2.md`. Both found every first-round finding closed except where noted below, the GBP purchase carrying one currency end to end, and legacy bytes and hashes unchanged. |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra P1: the table split did not stop an older build on mixed history. A legacy subscription that ended on the 10th and a book one bought on the 12th: the older build finds the legacy half, prices the month from it alone in EUR, and can waive it. | Fixed, in the database. A trigger refuses to freeze a version 1 calculation for any month a book-priced assignment of the account begins in or before. The older build writes only version 1, so it cannot freeze such a month; this build never writes one for it. A legacy-only month before the purchase, and a legacy-only account, stay the older build's. | `billingstore.go` (`billing_months_no_legacy_calc_over_book_history`) | `TestOlderBuildCannotFreezeAMonthWithBookHistory` | |
| +| Astra P2: a bound session that reads back in another currency was refused but left open, so a buyer with its URL could still pay. | Fixed. On the resume paths and in the background pass, a bound session that is this intent's and not its purchase is expired and the intent retired (`refuseSession` handles bound and unbound intents). | `billingservice.go`, `billingpass.go`, `billingstore.go` `retireIntent` | `TestCheckoutNeverOffersASessionInAnotherCurrency` (two new cases, one with a provider that answers in pounds on the first read and euros on the second) | |
| +| Opus P3-1: a refused session that was already paid wedged the intent, was asked to expire on every pass, and nobody was told. | Fixed. A completed or paid session is not expired; a review (`billing_checkout_currency_refused`) is filed once, the intent is left as it is so no second purchase opens beside it, and nothing is asked of the provider again. | `billingservice.go` `refuseSession` | `TestRecoveredSessionMustBeInTheIntentsCurrency` (the two "untouched" cases, across two passes) | |
| +| Opus P2-1: `currency_options` may hold the price's own currency when expanded, and the check would then refuse every real price. | Fixed. An entry under the price's own currency, at its own amount and tax behaviour, is accepted; any other entry is refused. Which shape Stripe returns is test-mode item 1; either passes. | `billingservice.go` `priceCurrencyOptionsAreOwn` | `TestCheckoutRefusesAProviderPriceThatIsNotTheBooks` (own-only accepted; own plus another, another alone, own at another amount refused) | |
| +| Astra P3: the catalog of another book had no test. | Fixed through a seam: the console's book list, currencies and approval come from package variables a test can set. Two books, the old one whole or not at all, no price serving both. | `cmd/console/billing.go` | `TestBillingConfigKeepsCatalogsOfOtherBooks` | |
| +| Opus P3-2, Astra P3: the surviving mutations were not redundant. | Corrected above; each has a case: a created session carrying another intent's id; a provider answering differently on two reads. | tests | as above | |
| +| Opus P3-3: keys differing only in case loaded. | Fixed: keys are folded before the duplicate check. | `plans/pricebook.go` | `TestPriceBookValidation` | |
| +| Opus claim 1: the golden test's tail was vacuous. | Fixed: it asserts the exact bytes. | `billingcurrency_test.go` | `TestLegacyCalculationBytesAreUnchanged` | |
| +| Opus claim 2: a test comment misstated which check refuses a map keyed under an empty book. | Corrected. | `cmd/console/billing_test.go` | none | |
| +| Opus claim 5: a comment said background passes never POST. | Corrected: a pass may expire a session, which creates nothing. | `billingservice.go` | none | |
| +| Astra claim: `bindBillingCheckout` returns early for a subscription already recorded. | Accepted: a subscription already recorded was checked when it was; binding it again changes nothing. | | | |
| +| Astra claim: "before the provider is asked" overstated. | Corrected: the live-subscription refusal spares the session creation, not the reads before it. | this file, test comment | | |
| +| Opus P3-4: rollback leaves stale columns that fail closed; an older build on a month where a legacy subscription ended and a book one began. | The second case is the barrier above. The first is a roll note (below). | | | |
| +| Design text stale (six columns; `ErrAssignmentHistory`; 36 or 48 keys; Adaptive Pricing without the legacy exception). | Corrected. | design | | |
| + |
| +**Roll note.** The billing environment and the console binary change together: this build needs `CONSOLE_BILLING_PRICE_BOOK` and the 36 keys, and the build before it exits on them. Once any account has a book-priced row, no binary from before price books may run against the database: its reads of the account rows and intents would fail closed, and the database refuses its freezes over book history, but a legacy subscription it recorded for such an account would leave the account's `currency` and `price_book` columns stale and this build refusing the account for good. Billing is off everywhere today, so this is a note for the first roll. |
| + |
| +Mutation checks on these fixes, nine in a scratch copy: eight failed their tests (the barrier absent, a bound session not expired on either resume path or by the pass, a paid session expired and unreviewed, a second currency among the options, case-folded duplicates, another intent's session kept). One survived, redundant with the count of used keys: a partial catalog of another book is refused by that count whichever check is removed. |
| + |
| +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-review2.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review2.md| new file mode 100644 |
| index 000000000..0ddcda103 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review2.md |
| @@ -0,0 +1,112 @@ |
| +# Opus authority review: `feat/carloku-multi-currency-20261001` at `a88a39e6` (base `12e70d95`) |
| + |
| +This was a static read only. I ran no tests and changed nothing. The delta you named (`git diff a88a39e6 a88a39e6`) is empty. The first review was at `9d104f1d`, so I reviewed `9d104f1d..a88a39e6` as the delta. |
| + |
| +## First-review findings: closed? Would the test pass without the fix? |
| + |
| +| Finding | Closed? | Test passes without its fix? | |
| +|---|---|---| |
| +| Opus P2-1 / Astra P1: a session in the wrong currency is offered on the second click or by the pass | Closed. `intentSession` is checked at all five places a session is created, listed or read back. `refuseSession` expires such a session and retires the intent. | No. Cases 2 and 3 of `TestCheckoutNeverOffersASessionInAnotherCurrency` and the "retired" cases of `TestRecoveredSessionMustBeInTheIntentsCurrency` fail on the old code. Case 4 needs both checks at `:467`/`:514` removed (see I-2). | |
| +| Astra P1: an older build prices GBP history as EUR | Closed. Book-priced assignments go in `billing_book_assignments`. | No. `TestBookPricedHistoryIsInvisibleToAnOlderBuild` runs the old queries word for word. Rollback still has other hazards (P3-4). | |
| +| Opus P2-2: a change of currency is not refused | Closed (`billingservice.go:412-426`). | The currency half: no, it would fail. The live-subscription half: yes, it passes anyway, because `beginBillingCheckout` already refused it with no POST. The early refusal is an optimisation, which is fine. | |
| +| Opus P2-3: a month bought in two currencies is held quietly | Closed (`ErrPriceTerms` → `billing_invoice_price_terms`). | No. Without the fix the error is `ErrAssignmentHistory`, which is quiet, and the test asserts the review. | |
| +| Astra P2: historical books' catalogs refused | Closed in code (`cmd/console/billing.go` `catalog`). | Yes. Only one book is embedded, so a second book's catalog can't be tested. The test covers only the current book and the legacy twelve. | |
| +| Astra P2 / Opus P3-3: item currency at preflight; quantity update response unchecked | Closed (`billinginvoice.go:565-583`, `billingquantity.go:1105`). | No, for both `TestSubscriptionAndItemCurrencyAreEachChecked` and `TestQuantityWorkerDoesNotConfirmAnUpdateOnAnotherPrice`. | |
| +| Astra P2 / Opus P3-4: overflow, unknown country, duplicate keys | Closed. One case-folding gap remains (P3-3). | No. The 2^62 case wraps to 2800 under unchecked arithmetic, so it would be accepted. | |
| +| Astra P3: the golden test shared this build's types | Closed: the golden is now a literal. | No, but its last block is vacuous (I-1). | |
| +| Astra P3: overage rates checked by hand | Closed. I re-derived all 15 figures and the three rounded totals (731 / 609 / 548). | No. | |
| +| Astra claim: recovered draft and line not verified | Closed. | No, on both. The test checks that the operation is not acked clean. | |
| +| Astra claim: the legacy replay was a different request | Closed (`LegacyAdaptive`). | No. The old request is byte-identical to the base's (`LocalizeCurrency: true` at base `:353`/`:799`). | |
| +| Opus P3-5: export rule for the free plan | Closed (the design text and the code comment). | Not applicable. | |
| +| Opus P3-6: currency checks masked each other in the fakes | Closed (`itemCurrency` is separate). | No. | |
| +| Opus P3-1, P3-7, claims 3 and 6 | Accepted, and still accurate. | Not applicable. | |
| +| Opus P3-2: roll note | Still owed. | Not applicable. | |
| +| Opus claim 4: an unknown currency was reported after other faults | Closed: it is checked before the lock. | No. | |
| +| Opus claim 5: the empty-book refusal was called redundant | Corrected. | No, but only through the "no book, and only the twelve legacy keys" case (I-3). | |
| + |
| +A GBP purchase carries one currency end to end: |
| +- **Checkout:** chosen at `:331`, stored on the intent; the price is checked for currency, amount, no options and metadata at `:369-376`. |
| +- **Session:** asked for with `currency=gbp` and Adaptive Pricing off; checked by `FixedCurrency` at creation, replay, listing, read-back and bind. |
| +- **Reconcile:** checks the subscription's and its item's currency; the store refuses any change of book or currency (`billingstore.go:542`). The assignment goes to the book table. |
| +- **Month:** `AllowanceForMonth` gives one set of price terms. The calculation is version 2 in GBP. |
| +- **Invoice:** the draft and every line are sent in `r.calc.Currency` and verified on every read, including recovery. Preflight checks the subscription and its item. |
| +- **Quantity worker:** checks the read and the update response. |
| + |
| +Legacy version 1 bytes and request hashes are unchanged. I checked the base `DraftInvoiceParams` and `InvoiceItemParams` field order against the test's reconstructions. |
| + |
| +## Findings, most severe first |
| + |
| +**P2-1 (plausible; verify before enabling). The `currency_options` check may refuse every real price.** |
| +- **Where:** `internal/console/billingservice.go:375` (`len(price.CurrencyOptions) != 0`), with `GetPrice` expanding `currency_options` (`billingstripe/catalog.go:106`). |
| +- **Scenario:** as I recall Stripe's API, an expanded `currency_options` on a single-currency price returns a map that includes the price's own currency, e.g. `{"gbp":{"unit_amount":2200,…}}`. If so, every checkout returns `unavailable`. It fails closed, and the check predates this branch. But the design's "no currency options" invariant now rests on it, and no real price object has been seen. |
| +- **Smallest fix:** accept a map whose only key is `price.Currency`, with `unit_amount == price.UnitAmount` and tax behaviour unchanged. Refuse anything else. Settle it with test-mode item 1 first. |
| + |
| +**P3-1. A refused session that is already paid wedges the intent, re-POSTs expire on every pass, and files no review.** |
| +- **Where:** `billingservice.go:1056` and `:1026` → `refuseSession` (`:238`). |
| +- **Scenario:** a session completes in another currency, so the provider misbehaved. |
| + - The listing path calls `ExpireCheckout` on a `complete` session. Stripe answers 400. |
| + - The click returns `unavailable`. Every billing pass POSTs a failing expire again. |
| + - The intent can never be retired, so the account cannot check out again. |
| + - The only record is the webhook's rejected-event row, and only if `checkout.session.completed` arrives. |
| + - The dispositions say "left unbound for a person", but no person is told. |
| +- **Fix:** in `refuseSession`, when the session's status is `complete`, don't expire it. File `billing_checkout_currency_refused` once and return `errBillingIdentity`. |
| + |
| +**P3-2. The third surviving mutation is reachable, and its branch is untested.** |
| +- **Where:** `billingservice.go:551-556` (`!intentSession && !ownSession → errBillingUnavailable`). |
| +- **Why it's reachable:** the client checks only currency, Adaptive Pricing and conversion (`client.go:651`). A created session whose `livemode`, `client_reference_id` or `checkout_intent` disagrees passes the client and reaches this branch. If the branch is deleted, that session is saved and its URL is offered. |
| +- **Fix:** add a fake case where the created session has the wrong client reference or intent metadata. Correct the dispositions. |
| + |
| +**P3-3. Duplicate keys in the price book that differ only in case are not refused.** |
| +- **Where:** `internal/plans/pricebook.go:206`. |
| +- **Scenario:** `"awake_hour": 12000, "Awake_Hour": 1` both pass `rejectDuplicateKeys`. `encoding/json` matches struct fields case-insensitively, so the last one wins silently. Map keys are exact and are already caught by the length and sold-currency checks. |
| +- **Fix:** case-fold the keys in `seen`, or refuse any key that isn't exactly lower case. |
| + |
| +**P3-4. Rollback below this commit leaves stale columns, which fail closed, and can under-bill one month.** |
| +- **Where:** an older binary's `UPDATE billing_accounts` leaves `currency` and `price_book` untouched. `invoiceIdentityFromHistory` (`billinginvoice.go:500`) checks only the calculation's own assignments. |
| +- **Scenario 1:** a GBP subscription ends, the console is rolled back, and the old build records a new legacy EUR subscription. The row still says gbp and the book. |
| + - After rolling forward, `applyBillingStateDated` refuses it forever (`:542`), as do the quantity worker and checkout. |
| + - Nothing is mispriced, but renewals and cancellations of that subscription are never recorded. |
| +- **Scenario 2:** in a month where a legacy subscription ended and a book EUR one began, the old build freezes a partial legacy calculation. This build then invoices it, and the book portion is never billed. |
| +- **Fix:** write the roll note Opus P3-2 asked for: no binary rollback once a book row exists, and the environment and binary change together. Optionally, have this build refuse a `billing_accounts` row whose subscription has no book but whose columns name one. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **`TestLegacyCalculationBytesAreUnchanged`, the last block (`billingcurrency_test.go:1186-1197`):** the `else if !strings.Contains(raw, "currency":"eur")` can never fail, because the top-level `currency` is always `"eur"`. The "explicit eur" case asserts nothing. It also can't happen in this build: the legacy table always reads `''`. Delete it or make it a real assertion. |
| +2. **The comment at `cmd/console/billing_test.go:151` ("Nothing but the explicit refusal of an empty book stands in the way"):** false. `used != len(cfg.Prices)` refuses both "keys under an empty book name" cases with or without the guard. The guard is covered only by "no book, and only the twelve legacy keys". |
| +3. **Dispositions `:45` on the redundant mutations:** |
| + - The two bound-session checks (`:467`, `:514`) are redundant in a single process only. `:514` alone is the fence when another process binds the intent between the lookup and `beginBillingCheckout`. Keep both, and say so. |
| + - The third survivor is not unreachable (P3-2). |
| + - The dispositions' first-round redundancy argument (a different currency is always a different price id) is right under a validated configuration. |
| +4. **Dispositions `:24` ("left unbound for a person"):** see P3-1. |
| +5. **The comment at `billingservice.go:1017` ("background passes… never POST"):** they now POST an expire through `refuseSession`. It is not monetary, but the comment is wrong. |
| +6. **Design `:206` ("the six columns"):** it is now four columns plus `billing_book_assignments`. |
| +7. **Design rule 7 ("a new row with no currency is refused"):** still true only through `cmd/console`. `storedPriceTerms` accepts legacy terms for new rows, which is accepted as before. |
| + |
| +Checked and sound, with no finding: |
| +- **B (choosing a currency):** only `PostForm` chooses a currency; the cookie only suggests one. |
| +- **E (purchases differing only in currency):** bands, allowances, quantities, tax codes and entitlements don't depend on currency. |
| +- **F (the book's validation):** the rounding rule, the EUR-equals-terms check, supported currencies, ambiguous countries, and overflow in `PriceFor`, `OverageFor` and `overageLines` all hold. EUR is identical to the legacy arithmetic. |
| +- **G (configuration):** live mode with a draft book or no book is refused, as are shared price ids and incomplete catalogs. |
| +- **H (schema and stored bytes):** the added fields are `omitempty` and appended. The `Overage` and `Invoice` pointers marshal identically. Request hashes blank the currency for version 1. |
| +- **I (test timing):** the time-sensitive tests use fixed fixture times, and the map-ordered tests are independent of order. |
| +- **J (public export):** it is correct and discloses nothing beyond the book and the country bands. The free plan is zero-priced and not band-scaled, and the allowance rule matches `Quote`'s pooled floor. |
| + |
| +## Must be verified in Stripe test mode before enabling, and what each run should show |
| + |
| +1. **`GET /v1/prices/{id}?expand[]=currency_options`** on a single-currency price. Is `currency_options` empty, or does it hold only the price's own currency? This decides P2-1. |
| +2. **Checkout Session with `currency` and `adaptive_pricing[enabled]=false`,** subscription mode, for gbp, usd and eur, with buyers in IE, GB, US and one Adaptive-eligible country (JP). Expect `currency` equal to the price's, `adaptive_pricing.enabled=false`, `currency_conversion` null, and `amount_subtotal` equal to unit × quantity. |
| +3. **The same with `currency` different from the price's.** Expect a 400 and its error code, which shows `CodeCurrencyMismatch` is only a backstop. |
| +4. **The completed subscription.** Expect `currency` and `items[0].price.currency` equal to the choice, and `price_book` and `currency` in the metadata. |
| +5. **Customer currency lock:** |
| + - With a live GBP subscription, does a USD checkout fail, and at which call? |
| + - After a cancel, is USD allowed on the same customer? |
| + - Is a GBP draft invoice for the old month then still accepted? |
| +6. **Overage invoice in gbp and usd** (linked draft, items with tax codes, `pending_invoice_items_behavior=exclude`, finalize, pay) for UK and US customers with only the IE registrations. Expect `automatic_tax.status=complete`, every line in the currency, and the subtotal equal to the calculation. |
| +7. **Quantity increase on a GBP subscription under both proration policies.** The update response's `price.currency` should be gbp, and the proration items and invoice should be in GBP. |
| +8. **Lost-response replays.** Same key and parameters should return the same session. A legacy replay (`adaptive_pricing[enabled]=true`, no `currency`) should still be accepted under its original key. |
| +9. **`ExpireCheckout` on a `complete` session.** The error shape (P3-1). |
| +10. **Promotion codes.** An EUR `amount_off` code on a GBP session should be refused; the 50% code should apply. |
| +11. **One old-style Adaptive session for a US buyer.** The resulting subscription's currency (Opus P3-7). |
| +12. **A renewal of a GBP subscription.** The renewal invoice's currency, and that reconcile records it without an identity error. |
| + |
| +**Verdict: accept with fixes.** Before billing is enabled, settle P2-1 in test mode, fix P3-1 and P3-2, and correct claims 1–3. P3-3, P3-4 and the remaining claim corrections can follow. |
diff --git a/implementation/carloku-paid-launch/stripe-provision.sh b/implementation/carloku-paid-launch/stripe-provision.sh| index c2c742d09..eaf2dc868 100755 |
| --- a/implementation/carloku-paid-launch/stripe-provision.sh |
| +++ b/implementation/carloku-paid-launch/stripe-provision.sh |
| @@ -40,6 +40,13 @@ |
| set -euo pipefail |
| readonly TERMS="2026-09-07-v1" |
| +# The price book (internal/plans/pricebook.json). Prices are per book, per |
| +# currency, and the console keys CONSOLE_STRIPE_PRICES by it. A book whose |
| +# amounts are not approved for sale is a draft; the console refuses a draft |
| +# book in live mode, and so does this script. |
| +readonly BOOK="2026-10-01-draft1" |
| +readonly BOOK_STATUS="draft" |
| +readonly CURRENCIES=(usd eur gbp) |
| readonly API_VERSION="2025-02-24.acacia" |
| readonly DEFAULT_ACCOUNT="acct_1UD7LXI4Nrr6AEsO" |
| readonly DEFAULT_WEBHOOK_URL="https://console.carloku.com/billing/webhook" |
| @@ -54,12 +61,21 @@ readonly WEBHOOK_EVENTS=( |
| readonly PLANS=(personal community business) |
| readonly BANDS=(A B C D) |
| -# Unit amounts in euro cents. These mirror stripe-live-readiness.json |
| -# required_catalog and internal/plans/plans.json (annual base times band factor). |
| +# Unit amounts in minor units, per member per year, keyed plan/band/currency. |
| +# These mirror internal/plans/pricebook.json (annual_unit_minor): the annual |
| +# base times the band factor, rounded half up per member before quantity. |
| +# The EUR column equals internal/plans/plans.json. The table is checked |
| +# against the embedded book by TestStripeProvisionScriptMatchesThePriceBook. |
| declare -A CENTS=( |
| - [personal/A]=2400 [personal/B]=1800 [personal/C]=1200 [personal/D]=840 |
| - [community/A]=500 [community/B]=375 [community/C]=250 [community/D]=175 |
| - [business/A]=5000 [business/B]=3750 [business/C]=2500 [business/D]=1750 |
| + [personal/A/usd]=2800 [personal/B/usd]=2100 [personal/C/usd]=1400 [personal/D/usd]=980 |
| + [personal/A/eur]=2400 [personal/B/eur]=1800 [personal/C/eur]=1200 [personal/D/eur]=840 |
| + [personal/A/gbp]=2200 [personal/B/gbp]=1650 [personal/C/gbp]=1100 [personal/D/gbp]=770 |
| + [community/A/usd]=600 [community/B/usd]=450 [community/C/usd]=300 [community/D/usd]=210 |
| + [community/A/eur]=500 [community/B/eur]=375 [community/C/eur]=250 [community/D/eur]=175 |
| + [community/A/gbp]=450 [community/B/gbp]=338 [community/C/gbp]=225 [community/D/gbp]=158 |
| + [business/A/usd]=6000 [business/B/usd]=4500 [business/C/usd]=3000 [business/D/usd]=2100 |
| + [business/A/eur]=5000 [business/B/eur]=3750 [business/C/eur]=2500 [business/D/eur]=1750 |
| + [business/A/gbp]=4500 [business/B/gbp]=3375 [business/C/gbp]=2250 [business/D/gbp]=1575 |
| ) |
| declare -A TAX_CODE=( |
| [personal]=txcd_10102001 [community]=txcd_10102001 [business]=txcd_10102000 |
| @@ -131,6 +147,9 @@ if [[ "$action" == "check" && -n "$promo_code" ]]; then |
| [[ "$promo_code" =~ ^[A-Za-z0-9]{3,32}$ ]] || die "--promo-code must be 3 to 32 letters and digits" |
| fi |
| +if [[ "$mode" == "live" && "$BOOK_STATUS" != "approved" ]]; then |
| + die "price book $BOOK is a $BOOK_STATUS: its amounts are not approved for sale, and the console refuses a draft book in live mode. Live provisioning waits for a book version marked approved." |
| +fi |
| if [[ "$mode" == "live" ]]; then |
| MODE_WORDS=(--project-name carloku --live --stripe-version "$API_VERSION") |
| MODE_TEXT="--project-name carloku --live --stripe-version $API_VERSION" |
| @@ -203,7 +222,7 @@ header() { |
| printf 'set -o pipefail\n\n' |
| } |
| -lookup_key() { printf 'carloku-%s-%s-%s' "$TERMS" "$1" "$2"; } |
| +lookup_key() { printf 'carloku-%s-%s-%s-%s' "$BOOK" "$1" "$2" "$3"; } |
| print_provision() { |
| header |
| @@ -233,38 +252,47 @@ print_provision() { |
| -d "metadata[terms_version]=$TERMS" |
| done |
| - section "Step 2. Twelve prices (EUR, exclusive tax, yearly, licensed, per unit, no currency_options)" |
| - note "Prices are immutable. A mistake is fixed by archiving the price and making a new one under a new terms version." |
| + section "Step 2. Thirty-six prices (one per plan, band and currency; exclusive tax, yearly, licensed, per unit, no currency_options)" |
| + note "Prices are immutable. A mistake is fixed by archiving the price and making a new one under a new price book version." |
| + note "Each price is in one currency and no other: no currency_options, ever. The console creates every Checkout Session with" |
| + note "adaptive_pricing disabled and refuses a session in any currency but the price's." |
| for plan in "${PLANS[@]}"; do |
| for band in "${BANDS[@]}"; do |
| - capture "PRICE_ID_${plan^^}_${band}" price_ .id \ |
| - stripe post /v1/prices "${MODE_WORDS[@]}" \ |
| - --idempotency "carloku-price-$TERMS-$plan-$band" \ |
| - -d "RAW:\"product=\$PRODUCT_ID_${plan^^}\"" \ |
| - -d currency=eur \ |
| - -d "unit_amount=${CENTS[$plan/$band]}" \ |
| - -d billing_scheme=per_unit \ |
| - -d tax_behavior=exclusive \ |
| - -d "recurring[interval]=year" \ |
| - -d "recurring[interval_count]=1" \ |
| - -d "recurring[usage_type]=licensed" \ |
| - -d "lookup_key=$(lookup_key "$plan" "$band")" \ |
| - -d "metadata[terms_version]=$TERMS" \ |
| - -d "metadata[plan]=$plan" \ |
| - -d "metadata[band]=$band" |
| + for currency in "${CURRENCIES[@]}"; do |
| + capture "PRICE_ID_${plan^^}_${band}_${currency^^}" price_ .id \ |
| + stripe post /v1/prices "${MODE_WORDS[@]}" \ |
| + --idempotency "carloku-price-$BOOK-$plan-$band-$currency" \ |
| + -d "RAW:\"product=\$PRODUCT_ID_${plan^^}\"" \ |
| + -d "currency=$currency" \ |
| + -d "unit_amount=${CENTS[$plan/$band/$currency]}" \ |
| + -d billing_scheme=per_unit \ |
| + -d tax_behavior=exclusive \ |
| + -d "recurring[interval]=year" \ |
| + -d "recurring[interval_count]=1" \ |
| + -d "recurring[usage_type]=licensed" \ |
| + -d "lookup_key=$(lookup_key "$plan" "$band" "$currency")" \ |
| + -d "metadata[terms_version]=$TERMS" \ |
| + -d "metadata[price_book]=$BOOK" \ |
| + -d "metadata[currency]=$currency" \ |
| + -d "metadata[plan]=$plan" \ |
| + -d "metadata[band]=$band" |
| + done |
| done |
| done |
| note "Assemble the CONSOLE_STRIPE_PRICES value. The ids are not secret. Keep the printed JSON for the console environment." |
| local lk=() |
| for plan in "${PLANS[@]}"; do |
| for band in "${BANDS[@]}"; do |
| - lk+=(-d "lookup_keys[]=$(lookup_key "$plan" "$band")") |
| + for currency in "${CURRENCIES[@]}"; do |
| + lk+=(-d "lookup_keys[]=$(lookup_key "$plan" "$band" "$currency")") |
| + done |
| done |
| done |
| printf 'CONSOLE_STRIPE_PRICES="$(' |
| words stripe get /v1/prices "${MODE_WORDS[@]}" -d active=true -d limit=100 "${lk[@]}" |
| - printf ' \\\n | jq -c %s)"\n' "'[.data[] | {key: (\"$TERMS/\" + .metadata.plan + \"/\" + .metadata.band), value: .id}] | from_entries'" |
| - printf 'echo "$CONSOLE_STRIPE_PRICES" | jq -e "length == 12" >/dev/null || { echo "expected 12 prices" >&2; exit 1; }\necho "$CONSOLE_STRIPE_PRICES"\n\n' |
| + printf ' \\\n | jq -c %s)"\n' "'[.data[] | {key: (\"$BOOK/\" + .metadata.plan + \"/\" + .metadata.band + \"/\" + .metadata.currency), value: .id}] | from_entries'" |
| + printf 'echo "$CONSOLE_STRIPE_PRICES" | jq -e "length == 36" >/dev/null || { echo "expected 36 prices" >&2; exit 1; }\necho "$CONSOLE_STRIPE_PRICES"\n' |
| + printf 'echo "CONSOLE_BILLING_PRICE_BOOK=%s"\n\n' "$BOOK" |
| section "Step 3. Coupons and the one public promotion code" |
| note "Public 50% coupon, forever. Unlimited redemptions: no max_redemptions and no expiry on the promotion code." |
| @@ -350,41 +378,39 @@ print_check() { |
| note "Read-only. Set these first (ids are not secret):" |
| note " export CONSOLE_STRIPE_PRICES='{...12 keys...}' PORTAL_CONFIG_ID=bpc_..." |
| note " export HOUSE_COUPON_ID=carloku-house-100-forever PUBLIC_COUPON_ID=carloku-public-50-forever" |
| - printf 'LIVE=%s\nEXPECTED_ACCOUNT=%s\nTERMS=%s\n' "$LIVE_JSON" "$account" "$TERMS" |
| + printf 'LIVE=%s\nEXPECTED_ACCOUNT=%s\nTERMS=%s\nBOOK=%s\n' "$LIVE_JSON" "$account" "$TERMS" "$BOOK" |
| printf ': "${CONSOLE_STRIPE_PRICES:?set CONSOLE_STRIPE_PRICES}" "${PORTAL_CONFIG_ID:?set PORTAL_CONFIG_ID}" "${HOUSE_COUPON_ID:?set HOUSE_COUPON_ID}" "${PUBLIC_COUPON_ID:?set PUBLIC_COUPON_ID}"\n\n' |
| section "Check 1. Merchant identity (startBillingCheckout: the account must equal CONSOLE_STRIPE_ACCOUNT)" |
| 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 12 active recurring prices" |
| + section "Check 2. Counts: exactly 3 active products and 36 active recurring prices" |
| 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) == 12 and (.has_more == false)'" |
| + printf ' \\\n | jq -e %s\n\n' "'(.data | 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, eur, exclusive, year x1, licensed, per_unit, no transform_quantity," |
| - note "no currency_options, unit_amount equals the book, metadata terms_version/plan/band; product active, livemode, tax_code per plan." |
| + note "Console rules: active, livemode matches mode, the key's currency, exclusive, year x1, licensed, per_unit, no transform_quantity," |
| + note "no currency_options, unit_amount equals the book, metadata terms_version/price_book/currency/plan/band; product active, livemode, tax_code per plan." |
| local tmpl |
| tmpl=$( |
| cat <<'EOF' |
| -declare -A CENTS=( |
| - [personal/A]=2400 [personal/B]=1800 [personal/C]=1200 [personal/D]=840 |
| - [community/A]=500 [community/B]=375 [community/C]=250 [community/D]=175 |
| - [business/A]=5000 [business/B]=3750 [business/C]=2500 [business/D]=1750 |
| -) |
| +declare -A CENTS=(@CENTS@) |
| declare -A TAX=([personal]=txcd_10102001 [community]=txcd_10102001 [business]=txcd_10102000) |
| +[ "$(jq -r 'length' <<<"$CONSOLE_STRIPE_PRICES")" = 36 ] || { echo "CONSOLE_STRIPE_PRICES must have exactly the 36 keys of $BOOK" >&2; exit 1; } |
| for key in "${!CENTS[@]}"; do |
| - plan="${key%/*}"; band="${key#*/}" |
| - id="$(jq -er --arg k "$TERMS/$key" '.[$k]' <<<"$CONSOLE_STRIPE_PRICES")" |
| + plan="${key%%/*}"; rest="${key#*/}"; band="${rest%/*}"; currency="${rest#*/}" |
| + id="$(jq -er --arg k "$BOOK/$key" '.[$k]' <<<"$CONSOLE_STRIPE_PRICES")" |
| price="$(stripe get "/v1/prices/$id" @FLAGS@ -d 'expand[]=currency_options')" |
| - jq -e --argjson live "$LIVE" --arg plan "$plan" --arg band "$band" --arg terms "$TERMS" --argjson cents "${CENTS[$key]}" --arg id "$id" ' |
| - .id == $id and .active == true and .livemode == $live and .currency == "eur" |
| + jq -e --argjson live "$LIVE" --arg plan "$plan" --arg band "$band" --arg currency "$currency" --arg terms "$TERMS" --arg book "$BOOK" --argjson cents "${CENTS[$key]}" --arg id "$id" ' |
| + .id == $id and .active == true and .livemode == $live and .currency == $currency |
| and .tax_behavior == "exclusive" and .billing_scheme == "per_unit" and .transform_quantity == null |
| and .unit_amount == $cents and .recurring.interval == "year" and .recurring.interval_count == 1 |
| and .recurring.usage_type == "licensed" and ((.currency_options // {}) | length) == 0 |
| - and .metadata.terms_version == $terms and .metadata.plan == $plan and .metadata.band == $band' \ |
| + and .metadata.terms_version == $terms and .metadata.price_book == $book and .metadata.currency == $currency |
| + and .metadata.plan == $plan and .metadata.band == $band' \ |
| <<<"$price" >/dev/null || { echo "PRICE CHECK FAILED: $key $id" >&2; exit 1; } |
| product="$(jq -r .product <<<"$price")" |
| stripe get "/v1/products/$product" @FLAGS@ \ |
| @@ -394,9 +420,12 @@ for key in "${!CENTS[@]}"; do |
| >/dev/null || { echo "PRODUCT CHECK FAILED: $key $product" >&2; exit 1; } |
| echo "ok $key $id $product" |
| done |
| -echo "all 12 prices and their products verified" |
| +echo "all 36 prices and their products verified" |
| EOF |
| ) |
| + local cents="" |
| + for key in "${!CENTS[@]}"; do cents+=" [$key]=${CENTS[$key]}"; done |
| + tmpl="${tmpl//@CENTS@/$cents }" |
| printf '%s\n\n' "${tmpl//@FLAGS@/$MODE_TEXT}" |
| section "Check 4. Tax registrations (ActiveTaxRegistrations: IE standard and IE oss_union, active and started)" |
| @@ -483,7 +512,7 @@ print_rollback() { |
| section "Rollback 3. Deactivate the portal configuration" |
| printf 'stripe post "/v1/billing_portal/configurations/$PORTAL_CONFIG_ID" %s \\\n --idempotency "carloku-rollback-portal-$PORTAL_CONFIG_ID-deactivate" \\\n -d active=false\n\n' "$MODE_TEXT" |
| - section "Rollback 4. Archive the twelve prices, then the three products" |
| + section "Rollback 4. Archive the thirty-six prices, then the three products" |
| printf 'for id in $(jq -r %s <<<"$CONSOLE_STRIPE_PRICES"); do\n' "'.[]'" |
| printf ' stripe post "/v1/prices/$id" %s \\\n --idempotency "carloku-rollback-price-$id-archive" \\\n -d active=false >/dev/null && echo "archived $id"\ndone\n' "$MODE_TEXT" |
| printf 'for plan in personal community business; do\n' |
diff --git a/implementation/carloku-paid-launch/stripe-provisioning.md b/implementation/carloku-paid-launch/stripe-provisioning.md| index ad67c8fc8..2df1319dc 100644 |
| --- a/implementation/carloku-paid-launch/stripe-provisioning.md |
| +++ b/implementation/carloku-paid-launch/stripe-provisioning.md |
| @@ -45,7 +45,7 @@ The console fails closed. These are the checks, from |
| | --- | --- | --- | |
| | Key shape | `billingConfig` | `CARLOS_STRIPE_SECRET_KEY` starts with `rk_live_` in live mode (`rk_test_` in test mode). Anything else stops the console at startup. | |
| | Merchant | `startBillingCheckout` | `GET /v1/account` returns the id in `CONSOLE_STRIPE_ACCOUNT`. | |
| -| Price | `startBillingCheckout` | The price from `CONSOLE_STRIPE_PRICES` is active, live-mode matches, EUR, `tax_behavior` exclusive, `billing_scheme` per_unit, no `transform_quantity`, no `currency_options`, yearly with interval count 1, `usage_type` licensed, metadata `terms_version`, `plan` and `band` match, and `unit_amount` equals the plan book price per unit in cents. | |
| +| Price | `startBillingCheckout` | The price from `CONSOLE_STRIPE_PRICES` under the key `<price_book>/<plan>/<band>/<currency>` is active, live-mode matches, in exactly that currency, its `unit_amount` the book's per-member price, its metadata naming `price_book` and `currency` as well as `terms_version`, `plan` and `band`, `tax_behavior` exclusive, `billing_scheme` per_unit, no `transform_quantity`, no `currency_options`, yearly with interval count 1, `usage_type` licensed, metadata `terms_version`, `plan` and `band` match, and `unit_amount` equals the plan book price per unit in cents. | |
| | Product | `startBillingCheckout` | The price's product is active, live-mode matches, and its `tax_code` equals the code configured for the plan. | |
| | Tax registrations | `startBillingCheckout` | `GET /v1/tax/registrations?status=active` holds an active, already started `IE` registration with `country_options.ie.type` of `standard` and another of `oss_union`. | |
| | Portal | `verifyPortalConfiguration` | The configuration is active, live-mode matches, `subscription_update` is off, `subscription_cancel` is on with mode `at_period_end`, `payment_method_update` is on, `invoice_history` is on, and if `customer_update` is on every allowed field is one of `email`, `name`, `phone`, `tax_id`. | |
| @@ -95,46 +95,68 @@ prices and are not secret. |
| ## Step 2. Prices |
| -Twelve prices, exactly the `required_catalog` in `stripe-live-readiness.json`. |
| -Every price has these fixed fields: `currency=eur`, `tax_behavior=exclusive`, |
| -`billing_scheme=per_unit`, `recurring[interval]=year`, |
| +Thirty-six prices: one for each plan, band and currency of the price book |
| +(`internal/plans/pricebook.json`, `2026-10-01-draft1`; design in |
| +`docs/superpowers/specs/2026-10-01-multi-currency-price-book-design.md`). Every |
| +price has these fixed fields: `currency` one of `usd`, `eur`, `gbp`, |
| +`tax_behavior=exclusive`, `billing_scheme=per_unit`, `recurring[interval]=year`, |
| `recurring[interval_count]=1`, `recurring[usage_type]=licensed`. No |
| -`currency_options` on any price (it would switch off Adaptive Pricing, and the |
| -console refuses a price that has any). Never pass `payment_method_types` |
| -anywhere. |
| - |
| -The unit amount is per seat. Personal has quantity 1. Community and Business |
| -use the member count as the subscription quantity, with a floor of 10. |
| - |
| -| Plan | Band A | Band B | Band C | Band D | |
| -| --- | --- | --- | --- | --- | |
| -| personal | 2400 | 1800 | 1200 | 840 | |
| -| community | 500 | 375 | 250 | 175 | |
| -| business | 5000 | 3750 | 2500 | 1750 | |
| - |
| -These agree with `internal/plans/plans.json`: annual base (24, 5 and 50 euro) |
| -times the band factors 1.00, 0.75, 0.50 and 0.35. |
| - |
| -Each price carries `metadata[terms_version]=2026-09-07-v1`, `metadata[plan]` and |
| -`metadata[band]`; the console compares all three. |
| +`currency_options` on any price: a price is in one currency and no other, the |
| +console refuses a price that has any, and every Checkout Session is created |
| +with `adaptive_pricing[enabled]=false` in the price's currency. Never pass |
| +`payment_method_types` anywhere. |
| + |
| +The unit amount is per member per year, in the currency's minor unit. Personal |
| +has quantity 1. Community and Business use the member count as the |
| +subscription quantity, with a floor of 10. |
| + |
| +| Plan | Currency | Band A | Band B | Band C | Band D | |
| +| --- | --- | --- | --- | --- | --- | |
| +| personal | usd | 2800 | 2100 | 1400 | 980 | |
| +| personal | eur | 2400 | 1800 | 1200 | 840 | |
| +| personal | gbp | 2200 | 1650 | 1100 | 770 | |
| +| community | usd | 600 | 450 | 300 | 210 | |
| +| community | eur | 500 | 375 | 250 | 175 | |
| +| community | gbp | 450 | 338 | 225 | 158 | |
| +| business | usd | 6000 | 4500 | 3000 | 2100 | |
| +| business | eur | 5000 | 3750 | 2500 | 1750 | |
| +| business | gbp | 4500 | 3375 | 2250 | 1575 | |
| + |
| +These are the book's `annual_unit_minor`: the annual base (28/24/22, 6/5/4.50, |
| +60/50/45 in USD/EUR/GBP) times the band factors 1.00, 0.75, 0.50 and 0.35, |
| +rounded half up per member before any quantity (GBP Community band B is |
| +3.375, so 338; band D is 1.575, so 158). The EUR column equals |
| +`internal/plans/plans.json`. Paul confirmed the annual bases on 2026-10-01 for |
| +implementation, not for production: the book is a draft, the console refuses a |
| +draft book in live mode, and the script refuses `--mode live` on one. |
| +Approving the amounts is a new book version marked `approved`, and the script's |
| +`BOOK` and `BOOK_STATUS` change with it. |
| + |
| +Each price carries `metadata[terms_version]=2026-09-07-v1`, |
| +`metadata[price_book]=2026-10-01-draft1`, `metadata[currency]`, |
| +`metadata[plan]` and `metadata[band]`; the console compares all five, and the |
| +price's own `currency` and `unit_amount`, against the book before it sells. |
| Conventions: |
| -- Idempotency key: `carloku-price-2026-09-07-v1-<plan>-<band>`, for example |
| - `carloku-price-2026-09-07-v1-personal-A`. |
| -- `lookup_key`: `carloku-2026-09-07-v1-<plan>-<band>`. Stripe keeps a lookup key |
| - unique across the account, so it also stops a duplicate price being created |
| +- Idempotency key: `carloku-price-<book>-<plan>-<band>-<currency>`, for example |
| + `carloku-price-2026-10-01-draft1-personal-A-gbp`. |
| +- `lookup_key`: `carloku-<book>-<plan>-<band>-<currency>`. Stripe keeps a lookup |
| + key unique across the account, so it also stops a duplicate price being created |
| after the 24-hour idempotency window has passed. The console never reads |
| lookup keys. The script uses them to assemble `CONSOLE_STRIPE_PRICES`. |
| - Never pass `transfer_lookup_key`. A corrected price is made under a new terms |
| + Never pass `transfer_lookup_key`. A corrected price is made under a new book |
| version and so a new key. |
| - Prices are immutable. A mistake is fixed by archiving the price |
| (`active=false`) and creating a new one, and the new price id goes into |
| `CONSOLE_STRIPE_PRICES`. |
| -`CONSOLE_STRIPE_PRICES` is a JSON object with exactly 12 keys of the form |
| -`2026-09-07-v1/<plan>/<band>` whose values are distinct `price_...` ids. The |
| -script prints the `GET /v1/prices` query and the `jq` that builds it. |
| +`CONSOLE_STRIPE_PRICES` is a JSON object with exactly the 36 keys |
| +`<book>/<plan>/<band>/<currency>` whose values are distinct `price_...` ids |
| +(the console also accepts, beside them, the whole catalog of an earlier book |
| +or the twelve legacy keys `2026-09-07-v1/<plan>/<band>`, for subscriptions |
| +already bought under them). `CONSOLE_BILLING_PRICE_BOOK` names the book. The |
| +script prints the `GET /v1/prices` query and the `jq` that builds the JSON. |
| ## Step 3. Coupons and the promotion code |
| @@ -327,12 +349,13 @@ through the normal roll workflow. |
| | `CONSOLE_STRIPE_TAX_CODES` | `{"personal":"txcd_10102001","community":"txcd_10102001","business":"txcd_10102000"}` | no | |
| | `CARLOS_STRIPE_WEBHOOK_SECRET` | the `whsec_` secret from step 5 | yes | |
| | `CARLOS_STRIPE_PREVIOUS_WEBHOOK_SECRETS` | empty, or up to two earlier `whsec_` values, comma separated | yes | |
| -| `CONSOLE_STRIPE_PRICES` | JSON with the 12 keys `2026-09-07-v1/<plan>/<band>` | no | |
| +| `CONSOLE_STRIPE_PRICES` | JSON with the 36 keys `2026-10-01-draft1/<plan>/<band>/<currency>` | no | |
| +| `CONSOLE_BILLING_PRICE_BOOK` | `2026-10-01-draft1` (a draft: live mode refuses it until a book version is approved) | no | |
| Notes from `billingConfig`: `CONSOLE_STRIPE_TAX_CODES` must have exactly three |
| entries (`personal`, `community`, `business`) and must not be combined with the |
| older single `CONSOLE_STRIPE_TAX_CODE`, which is refused alongside it. The |
| -prices JSON is capped at 16 KiB, the tax JSON at 1 KiB. `CONSOLE_STRIPE_HOUSE_COUPON` |
| +prices JSON is capped at 64 KiB, the tax JSON at 1 KiB. `CONSOLE_STRIPE_HOUSE_COUPON` |
| is only consulted for house accounts, but set it, since a house checkout without it |
| is refused. |
| @@ -344,8 +367,8 @@ Each check asserts the same conditions the console asserts, so a green run means |
| skim. |
| 1. Account id, country `IE`, currency `eur`, charges enabled. |
| -2. Exactly 3 active products and 12 active prices, no second page. |
| -3. For each of the 12 prices: every field in the Price row of the checks table near the top, and |
| +2. Exactly 3 active products and 36 active prices, no second page. |
| +3. For each of the 36 prices: every field in the Price row of the checks table near the top, and |
| for its product: active, livemode, the plan's tax code and the plan metadata. |
| 4. The two tax registrations, `standard` and `oss_union`, active and started. This |
| reads `country_options.ie.type`. The readiness file records `type` at the |
| @@ -468,7 +491,7 @@ Text for Paul, per AGENTS.md "Production infrastructure needs scoped approval". |
| > |
| > - 3 products with tax codes `txcd_10102001` (Personal, Community) and `txcd_10102000` |
| > (Business). Names pending copy approval. |
| -> - 12 yearly EUR prices at the amounts in step 2, tax exclusive, no currency options. |
| +> - 36 yearly prices (12 each in USD, EUR and GBP) at the amounts in step 2, tax exclusive, no currency options. |
| > - 2 coupons (50% forever, 100% forever) and 1 public promotion code for the 50% coupon. |
| > The 100% coupon has no promotion code. |
| > - 1 locked Customer Portal configuration. |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index bd80699ff..601c314a7 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -223,9 +223,15 @@ func TestCheckoutRefusesAProviderPriceThatIsNotTheBooks(t *testing.T) { |
| "metadata with no currency": func(p map[string]any) { delete(p["metadata"].(map[string]string), "currency") }, |
| "metadata for another book": func(p map[string]any) { p["metadata"].(map[string]string)["price_book"] = "2031-01-01-v9" }, |
| "metadata for another band": func(p map[string]any) { p["metadata"].(map[string]string)["band"] = "B" }, |
| - "carrying currency options": func(p map[string]any) { |
| + "carrying another currency's option": func(p map[string]any) { |
| p["currency_options"] = map[string]any{"eur": map[string]any{"unit_amount": 2400}} |
| }, |
| + "carrying its own and another": func(p map[string]any) { |
| + p["currency_options"] = map[string]any{"gbp": map[string]any{"unit_amount": 2200}, "eur": map[string]any{"unit_amount": 2400}} |
| + }, |
| + "its own option at another amount": func(p map[string]any) { |
| + p["currency_options"] = map[string]any{"gbp": map[string]any{"unit_amount": 2201}} |
| + }, |
| "another price's id at this path": func(p map[string]any) { p["id"] = currencyPriceID(plans.PlanPersonal, plans.BandA, plans.EUR) }, |
| } { |
| t.Run(name, func(t *testing.T) { |
| @@ -240,6 +246,17 @@ func TestCheckoutRefusesAProviderPriceThatIsNotTheBooks(t *testing.T) { |
| } |
| }) |
| } |
| + // Stripe, asked to expand currency_options, is understood to answer |
| + // with the price's own currency among them. That entry, at the |
| + // price's own amount, is the price itself and is accepted. |
| + t.Run("its own currency as its only option", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + f.prices[id]["currency_options"] = map[string]any{"gbp": map[string]any{"unit_amount": 2200, "tax_behavior": "exclusive"}} |
| + if _, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil { |
| + t.Fatalf("a price whose only currency option is its own: %v", err) |
| + } |
| + }) |
| // A price configured under the wrong currency's key. |
| fx, f, acct := newCurrencyFixture(t) |
| f.customerCountry = "GB" |
| @@ -332,7 +349,35 @@ func TestCheckoutNeverOffersASessionInAnotherCurrency(t *testing.T) { |
| } |
| noIntent(t, fx, acct) |
| }) |
| - t.Run("a bound session that later reads back in another currency is not resumed", func(t *testing.T) { |
| + t.Run("a bound session that later reads back in another currency is expired, not resumed", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := click(fx, f, acct, 0); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + // The provider answers in pounds on the first read of the second |
| + // click and in euros on the next: the two checks on the resume |
| + // path are each the fence for a reading the other did not see. |
| + for _, reads := range [][]string{{"eur"}, {"gbp", "eur"}} { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := click(fx, f, acct, 0); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + f.mu.Lock() |
| + f.readCurrencies = reads |
| + f.mu.Unlock() |
| + if dest, err := click(fx, f, acct, time.Minute); dest != "" || !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("reads %v, second click: dest %q err %v, want no destination", reads, dest, err) |
| + } |
| + if f.sessionStatus != "expired" || f.expireCalls != 1 { |
| + t.Fatalf("reads %v: the session a buyer may still hold the URL of is %s (%d expiries)", reads, f.sessionStatus, f.expireCalls) |
| + } |
| + noIntent(t, fx, acct) |
| + } |
| + _ = fx |
| + }) |
| + t.Run("a bound session that reads back changed is expired by the background pass", func(t *testing.T) { |
| fx, f, acct := newCurrencyFixture(t) |
| f.customerCountry = "GB" |
| if dest, err := click(fx, f, acct, 0); err != nil || dest == "" { |
| @@ -341,8 +386,23 @@ func TestCheckoutNeverOffersASessionInAnotherCurrency(t *testing.T) { |
| f.mu.Lock() |
| f.sessionCurrency = "eur" |
| f.mu.Unlock() |
| - if dest, err := click(fx, f, acct, time.Minute); dest != "" || !errors.Is(err, errBillingIdentity) { |
| - t.Fatalf("second click: dest %q err %v, want no destination", dest, err) |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)) |
| + if f.sessionStatus != "expired" { |
| + t.Fatalf("the pass left the session %s", f.sessionStatus) |
| + } |
| + noIntent(t, fx, acct) |
| + }) |
| + t.Run("a created session that is not this intent's is not kept", func(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry, f.createdIntent = "GB", "0123456789abcdef0123456789abcdef0123456789abcdef" |
| + if dest, err := click(fx, f, acct, 0); dest != "" || err == nil { |
| + t.Fatalf("dest %q err %v, want no destination", dest, err) |
| + } |
| + if intent, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || intent.SessionID != "" { |
| + t.Fatalf("another intent's session was bound: %+v %v", intent, err) |
| + } |
| + if f.expireCalls != 0 { |
| + t.Fatal("another intent's session was expired on this intent's behalf") |
| } |
| }) |
| } |
| @@ -1139,6 +1199,19 @@ func TestRecoveredSessionMustBeInTheIntentsCurrency(t *testing.T) { |
| if state.Subscription != "" || ierr != nil || after.SessionID != "" || f.sessionStatus != "complete" { |
| t.Fatalf("state %+v, intent %+v (%v), session %s; want nothing recorded and the intent left unbound", state, after, ierr, f.sessionStatus) |
| } |
| + // A person is told, once, and the provider is not asked to |
| + // expire what it will not. |
| + if f.expireCalls != 0 { |
| + t.Fatalf("the pass asked the provider to expire a completed session %d time(s)", f.expireCalls) |
| + } |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(2*time.Minute)) |
| + var reviews 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(&reviews); err != nil || reviews != 1 { |
| + t.Fatalf("%d reviews filed (%v), want exactly one after two passes", reviews, err) |
| + } |
| + if f.expireCalls != 0 { |
| + t.Fatal("a later pass asked the provider to expire the completed session") |
| + } |
| } |
| }) |
| } |
| @@ -1181,8 +1254,11 @@ func TestLegacyCalculationBytesAreUnchanged(t *testing.T) { |
| if string(again) != legacyCalculationGolden { |
| t.Fatalf("read and written again it is\n%s", again) |
| } |
| - // With an explicit "eur" on the assignment, as a legacy row may be |
| - // written, the bytes are still these: one spelling is stored. |
| + // A legacy assignment is always read back from the store with no |
| + // currency (storedPriceTerms keeps one spelling), so these are the |
| + // bytes a legacy month gets from this build. An assignment handed in |
| + // with an explicit "eur", which the store never produces, marshals |
| + // with that field and nothing else different: the money is the same. |
| history := monthHistory(plans.LegacyPriceTerms(plans.CurrentTermsVersion), false) |
| history[0].Currency = plans.EUR |
| spelled, err := priceBillingCalculation(statement, history) |
| @@ -1190,10 +1266,9 @@ func TestLegacyCalculationBytesAreUnchanged(t *testing.T) { |
| t.Fatal(err) |
| } |
| spelled.TaxCode = "txcd_test" |
| - if raw, _ := json.Marshal(spelled); string(raw) == legacyCalculationGolden { |
| - t.Log("an assignment spelled with eur marshals as the golden") |
| - } else if !strings.Contains(string(raw), `"currency":"eur"`) { |
| - t.Fatalf("unexpected bytes for a legacy assignment spelled with eur: %s", raw) |
| + raw, _ := json.Marshal(spelled) |
| + if want := strings.Replace(legacyCalculationGolden, `"paid_through":"2027-08-10T00:00:00Z"}`, `"paid_through":"2027-08-10T00:00:00Z","currency":"eur"}`, 1); string(raw) != want { |
| + t.Fatalf("a legacy assignment spelled with eur marshals as\n%s\nwant\n%s", raw, want) |
| } |
| } |
| @@ -1356,6 +1431,57 @@ func TestBookPricedHistoryIsInvisibleToAnOlderBuild(t *testing.T) { |
| } |
| } |
| +// A build from before price books, run against this database, finds the |
| +// legacy half of a mixed history and would freeze the month from it alone, |
| +// in EUR. The database refuses that: no version 1 calculation may be |
| +// frozen for a month a book-priced assignment of the account begins in or |
| +// before. A legacy month before the book purchase is still the older |
| +// build's to freeze, and this build's own version 2 goes in. |
| +func TestOlderBuildCannotFreezeAMonthWithBookHistory(t *testing.T) { |
| + fx, _, acct := newCurrencyFixture(t) |
| + st := fx.st |
| + at := func(m, d int) time.Time { return time.Date(2026, time.Month(m), d, 0, 0, 0, 0, time.UTC) } |
| + legacy := billingState{AccountID: acct.ID, Customer: "cus_one", Subscription: "sub_legacy", Status: "active", Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, Band: plans.BandA, Quantity: 1, PeriodStart: at(7, 1).Unix(), PeriodEnd: at(7, 1).AddDate(1, 0, 0).Unix()} |
| + if err := st.applyBillingState(legacy, 0, "e1", at(7, 1)); err != nil { |
| + t.Fatal(err) |
| + } |
| + legacy.Status = "canceled" |
| + if err := st.applyBillingState(legacy, 1, "e2", at(9, 10)); err != nil { |
| + t.Fatal(err) |
| + } |
| + 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 := st.applyBillingState(book, 2, "e3", at(9, 12)); err != nil { |
| + t.Fatal(err) |
| + } |
| + // What the older build writes: a version 1 calculation, as its own |
| + // marshal lays it out. |
| + v1 := []byte(`{"statement_object":{"key":""},"v":1,"arithmetic":"base_units_floor_v1","currency":"eur","statement":{}}`) |
| + v2 := []byte(`{"statement_object":{"key":""},"v":2,"arithmetic":"base_units_floor_v1","currency":"eur","statement":{}}`) |
| + if _, err := st.freezeBillingMonth(acct.ID, "2026-08", v1); err != nil { |
| + t.Fatalf("August, before the book purchase, from the older build: %v", err) |
| + } |
| + if _, err := st.freezeBillingMonth(acct.ID, "2026-09", v1); err == nil || !strings.Contains(err.Error(), "billing_month_legacy_calculation_over_book_history") { |
| + t.Fatalf("September, with the legacy half only, from the older build: %v; want the database to refuse it", err) |
| + } |
| + if _, err := st.freezeBillingMonth(acct.ID, "2026-10", v1); err == nil { |
| + t.Fatal("October, from the older build, was frozen in EUR over a book-priced subscription") |
| + } |
| + if _, err := st.freezeBillingMonth(acct.ID, "2026-10", v2); err != nil { |
| + t.Fatalf("October, from this build: %v", err) |
| + } |
| + // The same month for an account with no book history is the older |
| + // build's as ever. |
| + other, _, otherAcct := newBillingFixture(t) |
| + l := legacy |
| + l.AccountID, l.Status = otherAcct.ID, "active" |
| + if err := other.st.applyBillingState(l, 0, "e9", at(7, 1)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := other.st.freezeBillingMonth(otherAcct.ID, "2026-09", v1); err != nil { |
| + t.Fatalf("a legacy-only account, from the older build: %v", err) |
| + } |
| +} |
| + |
| // The subscription and the item it is a subscription to each report a |
| // currency, and each is checked on its own: at reconcile, in the quantity |
| // worker, and before an overage invoice is written. |
diff --git a/internal/console/billingpass.go b/internal/console/billingpass.go| index 8e2d37a3e..f9e27fa02 100644 |
| --- a/internal/console/billingpass.go |
| +++ b/internal/console/billingpass.go |
| @@ -196,6 +196,17 @@ func (s *Server) reconcilePendingIntent(ctx context.Context, intent billingInten |
| // delete is scoped to this exact session id. |
| return s.expireResolvedIntent(intent, session, now) |
| default: |
| + // Open. If it is no longer the purchase the intent describes, |
| + // a buyer who kept its URL must not be able to pay through it. |
| + acct, err := s.store.AccountByID(intent.AccountID) |
| + if err != nil || acct == nil { |
| + return err |
| + } |
| + if session.ID == intent.SessionID && !b.intentSession(session, intent, acct) && b.ownSession(session, intent, acct) { |
| + if err := s.refuseSession(ctx, intent, session.ID, session, now); err != nil && !errors.Is(err, errBillingIdentity) { |
| + return err |
| + } |
| + } |
| return nil |
| } |
| } |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 21fe890fe..7a6b284f5 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -6,6 +6,7 @@ import ( |
| "crypto/sha256" |
| "database/sql" |
| "encoding/hex" |
| + "encoding/json" |
| "errors" |
| "fmt" |
| "strings" |
| @@ -166,6 +167,36 @@ func billingPriceKey(t plans.PriceTerms, plan, band string) string { |
| return t.PriceBook + "/" + plan + "/" + band + "/" + string(t.Currency) |
| } |
| +// priceCurrencyOptionsAreOwn reports whether a price has no currency option |
| +// but its own. A price here is in one currency: a second currency on it |
| +// would let a session be presented and charged in that one. Stripe, asked |
| +// to expand currency_options, is understood to answer with the price's own |
| +// currency among them, so an entry under the price's currency, saying the |
| +// price's own amount and tax behaviour, is the price itself and not an |
| +// option. Anything else is refused. Which of the two shapes Stripe really |
| +// returns for a single-currency price is on the test-mode list; either |
| +// passes here, and a second currency never does. |
| +func priceCurrencyOptionsAreOwn(price *billingstripe.Price) bool { |
| + if len(price.CurrencyOptions) == 0 { |
| + return true |
| + } |
| + if len(price.CurrencyOptions) != 1 { |
| + return false |
| + } |
| + raw, ok := price.CurrencyOptions[price.Currency] |
| + if !ok { |
| + return false |
| + } |
| + var own struct { |
| + UnitAmount *int64 `json:"unit_amount"` |
| + TaxBehavior string `json:"tax_behavior"` |
| + } |
| + if err := json.Unmarshal(raw, &own); err != nil || own.UnitAmount == nil || *own.UnitAmount != price.UnitAmount { |
| + return false |
| + } |
| + return own.TaxBehavior == "" || own.TaxBehavior == price.TaxBehavior |
| +} |
| + |
| // billingCurrencyAgrees reports whether a currency the provider reports for |
| // an object agrees with the purchase's price terms. For a purchase from a |
| // book the provider must say the currency, and it must be the purchase's. |
| @@ -225,17 +256,33 @@ func (b *billingRuntime) ownSession(session *billingstripe.Session, intent billi |
| return session != nil && session.ID != "" && session.LiveMode == b.cfg.LiveMode && session.Metadata["checkout_intent"] == intent.ID && session.ClientReferenceID == acct.Sqid |
| } |
| -// refuseSession deals with a session the provider created for intent that |
| -// is not the purchase the intent describes: in another currency, converted, |
| -// or with Adaptive Pricing on it. Such a session is never offered to the |
| -// buyer and never bound to the intent. It is expired at the provider, and |
| -// once the provider says it is expired the intent is retired, so the next |
| -// attempt starts clean under a new key. The attempt that met it fails. |
| +// refuseSession deals with a session that is this intent's and is not the |
| +// purchase the intent describes: in another currency, converted, or with |
| +// Adaptive Pricing on it. Such a session is never offered to the buyer and |
| +// never bound to the intent, and one already bound is not resumed. The |
| +// attempt that met it fails. |
| +// |
| +// An open one is expired at the provider, so its URL stops working for a |
| +// buyer who still has it, and once the provider says it is expired the |
| +// intent is retired, whether it was bound to the session or not: the next |
| +// attempt starts clean under a new key. |
| // |
| -// If the session cannot be shown expired (it was already completed, or the |
| -// provider will not say) the intent stays as it is and the answer is an |
| -// identity error: a person has to look at what was paid for. |
| -func (s *Server) refuseSession(ctx context.Context, intent billingIntent, sessionID string, now time.Time) error { |
| +// A completed one cannot be expired: somebody paid for it, in the wrong |
| +// currency, which the provider should not have let happen. Nothing is |
| +// recorded from it, the intent is left as it is so no second purchase is |
| +// opened beside it, a review is filed once for the account, and nothing |
| +// is asked of the provider again. A person has to look at what was paid |
| +// for. |
| +// |
| +// session is the session as last read, or nil when only its id is known |
| +// (the client refused it as it was created, so it is open). |
| +func (s *Server) refuseSession(ctx context.Context, intent billingIntent, sessionID string, session *billingstripe.Session, now time.Time) error { |
| + if session != nil && (session.Status == "complete" || session.PaymentStatus == "paid") { |
| + if err := s.store.fileBillingReviewOnce(intent.AccountID, session.SubscriptionID, billingCheckoutRefusedReason, now); err != nil { |
| + return err |
| + } |
| + return errBillingIdentity |
| + } |
| expired, err := s.billing.client.ExpireCheckout(ctx, sessionID, "carloku-expire-"+intent.ID) |
| if err != nil { |
| return err |
| @@ -243,12 +290,23 @@ func (s *Server) refuseSession(ctx context.Context, intent billingIntent, sessio |
| if expired.ID != sessionID || expired.Status != "expired" { |
| return errBillingIdentity |
| } |
| - if err := s.store.retireRefusedCheckout(intent, expired, now); err != nil && !errors.Is(err, errBillingConflict) { |
| + if intent.SessionID != "" { |
| + err = s.store.expireBillingCheckout(intent, expired, now) |
| + } else { |
| + err = s.store.retireRefusedCheckout(intent, expired, now) |
| + } |
| + if err != nil && !errors.Is(err, errBillingConflict) { |
| return err |
| } |
| return errBillingIdentity |
| } |
| +// billingCheckoutRefusedReason is the review filed when a checkout session |
| +// was completed and paid in a currency other than the one asked for. The |
| +// console records nothing from such a session; what was paid, and what to |
| +// do about it, is a person's question. |
| +const billingCheckoutRefusedReason = "billing_checkout_currency_refused" |
| + |
| // refusedByCurrency reports whether err is the provider client refusing a |
| // session it created because of its currency, and which session. |
| func refusedByCurrency(err error) (string, bool) { |
| @@ -372,7 +430,7 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| if err != nil || priceTerms != terms { |
| return "", errBillingUnavailable |
| } |
| - if len(price.CurrencyOptions) != 0 || price.BillingScheme != "per_unit" || (len(price.TransformQuantity) > 0 && string(price.TransformQuantity) != "null") || price.Metadata["plan"] != plan || price.Metadata["band"] != band || price.ID != priceID || !price.Active || price.LiveMode != b.cfg.LiveMode || price.Currency != string(terms.Money()) || price.TaxBehavior != "exclusive" || price.Recurring.Interval != "year" || price.Recurring.IntervalCount != 1 || price.Recurring.UsageType != "licensed" || price.UnitAmount != quote.UnitMinor { |
| + if !priceCurrencyOptionsAreOwn(price) || price.BillingScheme != "per_unit" || (len(price.TransformQuantity) > 0 && string(price.TransformQuantity) != "null") || price.Metadata["plan"] != plan || price.Metadata["band"] != band || price.ID != priceID || !price.Active || price.LiveMode != b.cfg.LiveMode || price.Currency != string(terms.Money()) || price.TaxBehavior != "exclusive" || price.Recurring.Interval != "year" || price.Recurring.IntervalCount != 1 || price.Recurring.UsageType != "licensed" || price.UnitAmount != quote.UnitMinor { |
| return "", errBillingUnavailable |
| } |
| product, err := b.client.GetProduct(ctx, price.ProductID) |
| @@ -464,7 +522,15 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| if err != nil { |
| return "", err |
| } |
| - if session.ID != previous.SessionID || !b.intentSession(session, previous, acct) { |
| + if session.ID != previous.SessionID { |
| + return "", errBillingIdentity |
| + } |
| + if !b.intentSession(session, previous, acct) { |
| + // Bound to this intent and no longer the purchase it describes: |
| + // a buyer who kept its URL must not be able to pay through it. |
| + if b.ownSession(session, previous, acct) { |
| + return "", s.refuseSession(ctx, previous, session.ID, session, now) |
| + } |
| return "", errBillingIdentity |
| } |
| if session.Status == "open" && (previous.Currency != request.Currency || previous.PriceBook != request.PriceBook || previous.Plan != request.Plan || previous.Band != request.Band || previous.Quantity != request.Quantity || previous.Country != request.Country || previous.Price != request.Price || previous.Coupon != request.Coupon || previous.LocDigest != request.LocDigest || previous.LiveMode != request.LiveMode || previous.Members != request.Members) { |
| @@ -511,7 +577,13 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| if err != nil { |
| return "", err |
| } |
| - if session.ID != intent.SessionID || !b.intentSession(session, intent, acct) { |
| + if session.ID != intent.SessionID { |
| + return "", errBillingIdentity |
| + } |
| + if !b.intentSession(session, intent, acct) { |
| + if b.ownSession(session, intent, acct) { |
| + return "", s.refuseSession(ctx, intent, session.ID, session, now) |
| + } |
| return "", errBillingIdentity |
| } |
| switch session.Status { |
| @@ -540,7 +612,7 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| } |
| session, err := b.client.CreateCheckout(ctx, s.checkoutParams(acct, intent), "carloku-checkout-"+intent.ID) |
| if id, refused := refusedByCurrency(err); refused { |
| - return "", s.refuseSession(ctx, intent, id, now) |
| + return "", s.refuseSession(ctx, intent, id, nil, now) |
| } |
| if err != nil { |
| return "", err |
| @@ -550,7 +622,7 @@ func (s *Server) startBillingCheckout(ctx context.Context, acct *Account, plan, |
| } |
| if !b.intentSession(session, intent, acct) { |
| if b.ownSession(session, intent, acct) { |
| - return "", s.refuseSession(ctx, intent, session.ID, now) |
| + return "", s.refuseSession(ctx, intent, session.ID, session, now) |
| } |
| return "", errBillingUnavailable |
| } |
| @@ -1014,16 +1086,18 @@ func (s *Server) recoverReadyIntentSession(ctx context.Context, acct *Account, i |
| // in-flight create to that same session, or creates the one session if the |
| // accepted create never reached Stripe. It never uses a new key, so it can |
| // never open a second purchase. Background passes pass allowCreate=false so |
| - // they never POST; they rely on the read-only listing below instead. |
| + // they never create; they rely on the read-only listing below instead |
| + // (the one write a pass may make is to expire a session that is this |
| + // intent's and not its purchase, which creates nothing). |
| if allowCreate && within() { |
| session, err := b.client.CreateCheckout(ctx, s.checkoutParams(acct, intent), "carloku-checkout-"+intent.ID) |
| if id, refused := refusedByCurrency(err); refused { |
| - return nil, s.refuseSession(ctx, intent, id, now) |
| + return nil, s.refuseSession(ctx, intent, id, nil, now) |
| } |
| if err == nil { |
| if !b.intentSession(session, intent, acct) { |
| if b.ownSession(session, intent, acct) { |
| - return nil, s.refuseSession(ctx, intent, session.ID, now) |
| + return nil, s.refuseSession(ctx, intent, session.ID, session, now) |
| } |
| return nil, errBillingIdentity |
| } |
| @@ -1053,7 +1127,7 @@ func (s *Server) recoverReadyIntentSession(ctx context.Context, acct *Account, i |
| // describes. One that is this intent's and is not that purchase |
| // is expired and the intent retired. |
| if !b.intentSession(&session, intent, acct) { |
| - return nil, s.refuseSession(ctx, intent, session.ID, now) |
| + return nil, s.refuseSession(ctx, intent, session.ID, &session, now) |
| } |
| return &session, nil |
| } |
diff --git a/internal/console/billingservice_test.go b/internal/console/billingservice_test.go| index 12c8f9438..00b40e8b1 100644 |
| --- a/internal/console/billingservice_test.go |
| +++ b/internal/console/billingservice_test.go |
| @@ -52,6 +52,14 @@ type fakeBillingStripe struct { |
| itemCurrency string |
| // failExpire makes that many expire requests fail without effect. |
| failExpire int |
| + // readCurrencies, when set, is what successive reads of the session |
| + // report, in order; the last value stays. A provider whose answers |
| + // change between two reads. |
| + readCurrencies []string |
| + // createdIntent, when set, is the checkout_intent the created session |
| + // carries instead of the one asked for: a session that is not this |
| + // intent's at all. |
| + createdIntent string |
| } |
| func (f *fakeBillingStripe) itemMoney() string { |
| @@ -126,6 +134,12 @@ func (f *fakeBillingStripe) session() map[string]any { |
| if f.sessionCurrency != "" { |
| currency = f.sessionCurrency |
| } |
| + if len(f.readCurrencies) > 0 { |
| + currency = f.readCurrencies[0] |
| + if len(f.readCurrencies) > 1 { |
| + f.readCurrencies = f.readCurrencies[1:] |
| + } |
| + } |
| if currency == "-" { // a session that reports no currency at all |
| currency = "" |
| } |
| @@ -185,6 +199,9 @@ func (f *fakeBillingStripe) serve(w http.ResponseWriter, r *http.Request) { |
| } |
| } |
| f.currency = r.PostForm.Get("currency") |
| + if f.createdIntent != "" { |
| + f.meta["checkout_intent"] = f.createdIntent |
| + } |
| if f.failCreate { |
| f.failCreate = false |
| w.WriteHeader(http.StatusInternalServerError) |
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go| index 17a5c2afc..410b2c364 100644 |
| --- a/internal/console/billingstore.go |
| +++ b/internal/console/billingstore.go |
| @@ -170,6 +170,21 @@ func (s *Store) migrateBilling() error { |
| )`, |
| `CREATE TRIGGER IF NOT EXISTS billing_review_resolutions_no_update BEFORE UPDATE ON billing_review_resolutions BEGIN SELECT RAISE(ABORT,'billing_review_resolution_immutable'); END`, |
| `CREATE TRIGGER IF NOT EXISTS billing_review_resolutions_no_delete BEFORE DELETE ON billing_review_resolutions BEGIN SELECT RAISE(ABORT,'billing_review_resolution_immutable'); END`, |
| + // The barrier against a build from before price books. Such a |
| + // build reads legacy assignments only, prices what it reads in EUR |
| + // and writes the month as a version 1 calculation. For a month in |
| + // which an account held a book-priced subscription, that is a |
| + // month priced without part of its history. The database itself |
| + // refuses it: no version 1 calculation may be frozen for a month |
| + // that any book-priced assignment of the account begins in or |
| + // before. This build never writes one for such a month (it prices |
| + // the month from the book, or refuses it as mixed), so the trigger |
| + // only ever stops the older build. |
| + `CREATE TRIGGER IF NOT EXISTS 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 := s.db.Exec(stmt); err != nil { |
| return err |
diff --git a/internal/plans/pricebook.go b/internal/plans/pricebook.go| index 6d13a96fc..6ae5212ae 100644 |
| --- a/internal/plans/pricebook.go |
| +++ b/internal/plans/pricebook.go |
| @@ -9,6 +9,7 @@ import ( |
| "io" |
| "math" |
| "sort" |
| + "strings" |
| ) |
| // Fixed prices in more than one currency |
| @@ -202,7 +203,10 @@ func rejectDuplicateKeys(data []byte) error { |
| if err != nil { |
| return err |
| } |
| - key, _ := keyTok.(string) |
| + // Folded: encoding/json matches a struct field without |
| + // regard to case, so "eur" and "EUR" would be one field, |
| + // the last one winning. |
| + key := strings.ToLower(keyTok.(string)) |
| if seen[key] { |
| return fmt.Errorf("the key %q appears twice in one object", key) |
| } |
diff --git a/internal/plans/pricebook_test.go b/internal/plans/pricebook_test.go| index d73720a8f..0aace1e62 100644 |
| --- a/internal/plans/pricebook_test.go |
| +++ b/internal/plans/pricebook_test.go |
| @@ -509,6 +509,11 @@ func TestPriceBookValidation(t *testing.T) { |
| return strings.Replace(s, `"personal": {"usd": 2800, "eur": 2400, "gbp": 2200}`, `"personal": {"usd": 2800, "eur": 2400, "gbp": 2200, "gbp": 2200}`, 1) |
| }, |
| "a band given twice": func(s string) string { return strings.Replace(s, `"D": 980}`, `"D": 980, "D": 980}`, 1) }, |
| + // encoding/json matches a struct field without regard to case, so |
| + // the second of these would win silently. |
| + "a field given twice in another case": func(s string) string { |
| + return strings.Replace(s, `"status": "draft"`, `"status": "draft", "Status": "approved"`, 1) |
| + }, |
| } { |
| edited := edit(string(priceBookJSON)) |
| if edited == string(priceBookJSON) { |
diff --git a/internal/plans/provision_test.go b/internal/plans/provision_test.go| new file mode 100644 |
| index 000000000..1e3423468 |
| --- /dev/null |
| +++ b/internal/plans/provision_test.go |
| @@ -0,0 +1,64 @@ |
| +package plans |
| + |
| +import ( |
| + "os" |
| + "regexp" |
| + "strconv" |
| + "testing" |
| +) |
| + |
| +// The Stripe provisioning script carries the book's prices as a table of |
| +// its own, because the operator pastes its output. That table is what would |
| +// be written to Stripe, so it has to be the book: every key, the same |
| +// amount, no key the book does not have, and the book's own version. |
| +func TestStripeProvisionScriptMatchesThePriceBook(t *testing.T) { |
| + raw, err := os.ReadFile("../../implementation/carloku-paid-launch/stripe-provision.sh") |
| + if err != nil { |
| + t.Skip("the provisioning script is not beside this package") |
| + } |
| + script := string(raw) |
| + 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) |
| + } |
| + status := regexp.MustCompile(`(?m)^readonly BOOK_STATUS="([^"]+)"`).FindStringSubmatch(script) |
| + approved, err := BookIsApproved(CurrentPriceBook) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if status == nil || (status[1] == BookApproved) != approved { |
| + t.Fatalf("the script says the book is %v; the embedded book says approved=%v", status, approved) |
| + } |
| + want := map[string]int64{} |
| + for _, plan := range paidPlans { |
| + qty := terms.Plans[plan].MinQuantity |
| + for _, band := range bandOrder { |
| + for _, c := range priceBooks[CurrentPriceBook].Currencies { |
| + p, err := PriceFor(PriceTerms{TermsVersion: CurrentTermsVersion, PriceBook: CurrentPriceBook, Currency: c}, plan, band, qty) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + want[plan+"/"+band+"/"+string(c)] = p.UnitMinor |
| + } |
| + } |
| + } |
| + // The first CENTS table is the script's own; the check template later |
| + // in the file is filled from it at run time. |
| + table := regexp.MustCompile(`(?s)declare -A CENTS=\((.*?)\)`).FindStringSubmatch(script) |
| + if table == nil { |
| + t.Fatal("no CENTS table in the script") |
| + } |
| + got := map[string]int64{} |
| + for _, m := range regexp.MustCompile(`\[([a-z]+/[A-D]/[a-z]{3})\]=([0-9]+)`).FindAllStringSubmatch(table[1], -1) { |
| + n, _ := strconv.ParseInt(m[2], 10, 64) |
| + got[m[1]] = n |
| + } |
| + if len(got) != len(want) { |
| + t.Fatalf("the script's table has %d prices, the book %d", len(got), len(want)) |
| + } |
| + for key, amount := range want { |
| + if got[key] != amount { |
| + t.Errorf("%s: the script says %d, the book %d", key, got[key], amount) |
| + } |
| + } |
| +} |