amadan
Merge commit '05ccec07' into feat/carloku-billing-operator-cli-20261002
16 files changed,
+1566
−100
docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md+6 −1docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review21.md+236 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+36 −0docs/superpowers/specs/OPUS-REVIEW-multi-currency-review21.md+160 −0internal/billingstripe/catalog.go+9 −4internal/billingstripe/catalog_test.go+25 −0internal/console/billingcurrency_test.go+4 −2internal/console/billingnamed_test.go+675 −0internal/console/billingoperator.go+4 −2internal/console/billingpass.go+8 −2internal/console/billingretirement.go+195 −60internal/console/billingretirement_test.go+16 −2internal/console/billingservice.go+4 −9internal/console/billingservice_test.go+11 −0internal/console/billingstore.go+164 −17internal/console/billingstore_test.go+13 −1
CI passed — run details
diff --git a/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md b/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md| index 7e96a4bd7..391d8f1ed 100644 |
| --- a/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md |
| +++ b/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md |
| @@ -122,7 +122,12 @@ Built in `internal/console/billingretirement.go` and the paths it touches, with |
| - **`keep` does not re-run reconcile's judgement of the subscription.** The door requires the bound session to be the intent's own, the intent's purchase, complete, paid and carrying a subscription, and the store requires the recorded fate complete, nothing attached but the session's own subscription, and no dispute. A subscription that is still not the purchase is refused again by the next reconcile, which files the review's next occurrence: keep closes nothing for good. (The read-only judgement revision 3 proposed needs reconcile's checks factored out; not done.) |
| - **A stale observation after `keep`** (a pass that read a mismatch before the keep and files after it) reopens the review as a new occurrence while the intent still stands: one more hold, visible, never a silent record. A subscription other than the kept session's own that is seen while the kept intent still stands is attached and reopens the review the same way; seen after the purchase recorded and the intent was consumed, it is a dispute for the kept session. A dispute raised between the `keep` and the record stops the record as a conflict. |
| - **The pass's refusal turn** runs under its own deadline whether the reconciliation finished, failed or ran out of time (both turns are children of the caller's context, so cancelling it stops either; production starts the pass, as it starts every console pass, on a context that is never cancelled, so in production a stop is the process ending mid-turn, which the protocol tolerates because each step is one transaction or is redone by the next pass), so slow reconciliation cannot leave a refused session payable pass after pass. |
| -- **An unbound intent under a review that names no session** is still reconciled by the pass, which is how its session is found; nothing records or retires under the review on that path. |
| +- **An unbound intent under a review that names no session** is still reconciled by the pass, which is how its session is found. Nothing records, retires or cleans up an intent under an open review on that path: the store's record, expiry and cleanup each refuse it. (Round twenty's build let the cleanup of an intent that never had a session delete it under its review; review twenty-one.) |
| +- **Named subscriptions** (review twenty-one, answered as one rule). A subscription that names a standing intent is recorded from the first read, whether or not a refusal review exists. Nothing deletes an intent past one. The record of the purchase keeps what it consumed (`billing_consumed_intents`: the intent, its session, the subscription it recorded with) and in the same transaction makes every other subscription seen naming the intent a dispute; a subscription that names the intent afterwards is compared with that record and is a dispute. The expiry of the intent's session, and the cleanup of an intent that never had a session, do not delete an intent a subscription was seen naming: they file the refusal, with it attached, and the intent stands (an expired session has no subscription and an intent with no session never had a checkout, so that subscription was made outside checkout). `retire` is held while anything is attached, whether or not the intent still stands. What stays outside: a subscription first seen naming an intent that already expired in the ordinary way is a rejected event with its account and no hold, because nothing is kept of such an intent. |
| +- **Only a first purchase consumes an intent**: the record of a subscription that is not yet the account's recorded one, judged in the record's transaction. A later event of the account's recorded subscription deletes no intent, whatever its metadata names. |
| +- **Binding.** A session id is written onto an intent in one place, which refuses a session that is already another purchase's (another intent's, a retired, kept or consumed one's) and one that is not the session the intent's open review names. |
| +- **Creating an intent** re-reads the account's customer and the currency it is held to in its own transaction; a request built from an older reading is refused. |
| +- **The refusals' turn** takes the longest-due first. **A listing** that fails on a later page records the pages it read. **An answer carrying another session's id** records nothing. |
| - **A retirement from before the protocol** (a retirement record with no fate) keeps its behaviour: its session's and its subscription's later events settle as rejected events. A subscription that names an intent retired on a proven expiry, and that session read back as anything but expired, are each a dispute for that session, once. |
| - **The reviews listing** shows each refusal's recorded fate, what is attached, and why `retire` and `keep` are or are not available. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review21.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review21.md| new file mode 100644 |
| index 000000000..ee2e6b510 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review21.md |
| @@ -0,0 +1,236 @@ |
| +References below are at **`b260ac8e`**, except pricing-site references, which are at **`01b62f7`**. This was source-only review: no files changed, tests run, network commands or AWS commands. Test counterfactuals are judgments from source, not reproduced mutation results. |
| + |
| +**Twentieth review, finding by finding** |
| + |
| +| Finding | Closed? | Would its current test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1:** another subscription seen before any review is not attached | **Closed for the reported ordering:** the completed session already names its own subscription. `billingservice.go:1027` now files the refusal with the other subscription. The unbound mismatch also files it at `:869`. **The wider invariant remains open:** Findings 1–2 below. | **No.** `billingevidence_test.go:98` and `:165` require the attachment and held resolution. Neither tests the new counterexamples below. | |
| +| **Opus P3-1:** keep-gate refusals discard their reads | **Closed.** `billingoperator.go:247` uses the recording wrapper before its checks. | **No.** `billingevidence_test.go:199` requires a dispute and prevents retirement using the superseded expiry. | |
| +| **Opus P3-2:** edited identity/replacement intent defeats archived attribution | **Closed for archived retired/kept purchases.** Lookup uses the recorded session ID and customer (`billingretirement.go:342`, `:418`). | **No.** `billingevidence_test.go:344` independently checks edited attribution, a replacement intent and archived customer identity. | |
| +| **Opus P3-3:** archived events retry indefinitely | **Closed for the reported cases.** The binder settles against the archived account; an ordinary-expiry subscription with no standing intent also settles (`billingservice.go:1179`, `:1213`, `:972`). | **No.** The revised assertions require 204 and account attribution (`billingretirement_test.go:783`, `:794`; `billingevidence_test.go:445`). | |
| +| **Opus P3-4:** atomicity test does not exercise the helper’s transaction | **Closed.** An attachment-aborting trigger now interrupts `fileCheckoutRefusal` itself. | **No.** Separate commits leave the review behind and fail `billingretirement_test.go:674`. | |
| +| **Opus P3-5:** cancellation claim does not match the pass | **Closed.** Both turns are timeout children of the caller (`billingpass.go:160`). Production’s uncancelled background context is explicitly acknowledged. | **No.** `billingretirement_test.go:971` covers exhausted reconciliation time, cancellation before work and cancellation inside expiry. | |
| +| **Astra 1, P2:** reads discarded before classification | **The named successful-read paths are closed.** Both resume reads also stop on a dispute. Partial listing failure remains uncovered: Finding 4. | **No** for the reported mutations: `billingevidence_test.go:199`, `:255`, `:294` distinguish them. | |
| +| **Astra 2, P2:** filing after disappearance drops the subscription | **Closed after protocol retirement or kept consumption; incomplete after ordinary consumption.** See Finding 2. | **No** for the tested retirement case: `billingretirement_test.go:707` calls the production helper after retirement. **That test still passes with the ordinary-consumption hole.** | |
| +| **Astra 3, P2:** archived identity evidence disappears | **Closed for the reported cases.** The four session lookups and archived customer checks address them. | **No:** the late-evidence and individual-identity-clause tests distinguish the fixes. | |
| +| **Astra 4, P2:** a note resolves a dispute and releases invoicing | **Closed.** `billingstore.go:983` refuses dispute resolution in stage one. | **No.** `billingevidence_test.go:476` requires 409, an open review and no resolution row. | |
| +| **Astra 5, P3:** refusal work ignores caller cancellation | **Closed**, as above. | **No:** cancellation inside expiry distinguishes `WithoutCancel`. | |
| +| **Astra 6, P3:** atomicity test admits separate commits | **Closed**, as above. | **No:** the production-helper failure test distinguishes separate commits. | |
| +| **Astra 7, P3:** fractional-floor test reads source instead of exercising formatting | **Closed at the supplied site commit.** `test/pricing-pricebook.test.mjs:247` calls the renderer’s `moneyText`. | **No:** removing fractional-minor-unit refusal makes `assert.throws` fail. The separate build-wiring assertion remains textual. | |
| + |
| +**Astra’s P2 severity was right in both disputed cases.** Discarding the keep gate’s contradictory completion could authorize retirement using an obsolete expiry. Resolving a dispute on a note released an invoice hold. Those are release defects, not merely missing diagnostics. Their reported sequences are now closed with distinguishing tests. |
| + |
| +The eight Opus claims have these dispositions: |
| + |
| +| Claim | Assessment | |
| +|---|---| |
| +| **1. Every subscription is handed in before an answer** | The service call sites now use the wrapper. The universal wording still needs qualification: wrong returned ID/mode bypasses classification, and pre-review observations can disappear as in Finding 2. | |
| +| **2. The keep gate records its read** | **Closed:** recording precedes all four refusals. | |
| +| **3. Nothing observed is dropped; gone-intent filing classifies it** | **Not fully closed:** ordinary consumption and partial listing failures remain. | |
| +| **4. Only console shutdown cancels refusal work** | **Corrected:** caller cancellation now propagates, and production’s background-context limitation is stated. | |
| +| **5. Acknowledging a dispute releases nothing** | **Closed by removing that operation:** disputes are unresolvable in stage one. | |
| +| **6. Retirement settles paid money; garbled tie comment** | **Corrected:** retirement’s permitted scope and the three tie orders are now described accurately. | |
| +| **7. The pass-test assertion was withdrawn** | **Corrected:** its comment now describes the final, post-fate assertion actually made. | |
| +| **8. The scoped site still has the textual fractional-floor test** | **Closed by the supplied `01b62f7` scope and executable formatter test.** | |
| + |
| +Astra’s overlapping claims receive the same answers. “One test per interleaving” is explicitly withdrawn at design line 113. The disputed-record conflict still returns webhook 503; the dispositions now acknowledge that. |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — An unready intent can be deleted under its open refusal review, after which `retire` releases a hold with an attached subscription.** |
| + |
| + **Where:** `internal/console/billingstore.go:514`; `internal/console/billingretirement.go:159`, `:203`; `internal/console/billingpass.go:154`, `:272`. |
| + |
| + **Concrete scenario:** |
| + |
| + - Checkout persists intent I. Customer creation succeeds, but verification fails before `readyBillingCustomer`, leaving `customer_ready=0` and no bound session (`billingservice.go:1345`, `:1382`). |
| + - A subscription H made at the provider names I and has the wrong purchase terms. Reconciliation’s new unbound branch files the review and attaches H (`billingservice.go:869`). |
| + - The review has no session reference, so the pass includes I in ordinary pending reconciliation. After 25 hours, `discardUnreadyIntent` deletes it. That helper checks neither the review nor its obligations, and the delete barrier expressly exempts unready intents. |
| + - The operator requests `retire`. `refusalState.held` returns success immediately because the intent no longer stands, **before checking obligations or protocol**. |
| + - Resolution closes the review. It creates no retirement archive because there is no intent row (`billingstore.go:1033`). H remains outstanding. Its later reads match the existing obligation and do not reopen anything (`billingretirement.go:556`). |
| + |
| + Account-deletion preparation can invoke the same discard immediately (`billingservice.go:800`). |
| + |
| + **Smallest acceptable fix:** make unready-intent deletion refuse an open refusal inside its transaction. A missing intent must not bypass obligation and migrated-review holds. Add a production-pass → operator-resolution regression, plus the deletion-preparation entry. |
| + |
| + The new unbound test would **pass with this defect**: its fixture is customer-ready and it never exercises unready cleanup. |
| + |
| +2. **P2 — A subscription observed before the first refusal can disappear when an ordinary purchase consumes the intent.** |
| + |
| + **Where:** `internal/console/billingretirement.go:538`, `:572`; `internal/console/billingservice.go:1021`; `internal/console/billingstore.go:785`, `:1094`. |
| + |
| + **Concrete scenario:** |
| + |
| + - GBP intent I has session S and no review. A manually created, otherwise matching subscription H names I. |
| + - S reads `complete` but does not yet name a subscription. `readSubscription` reaches classification, but classification retains nothing without a review. Reconciliation returns pending at `billingservice.go:1021`. |
| + - S subsequently names its legitimate subscription C. C records and consumes I. |
| + - H retries. Ordinary consumption left no purchase archive. Classification finds neither a standing intent, retirement nor kept resolution, and retains nothing. Because C is active, reconciliation returns bare identity at `billingservice.go:969`; the webhook acknowledges it with a rejected row, without a hold for H. |
| + |
| + The same gap exists while S is open, while H is incomplete, or while a matching intent is unbound. |
| + |
| + There is also a two-console ordering: A reads wrong H and prepares its first refusal; B records C before A files. The new gone-intent branch calls classification, but ordinary consumption supplies no archive, so H still disappears. Its comment’s “a dispute and never nothing” is false. |
| + |
| + **Smallest acceptable fix:** retain candidate subscription observations without prematurely refusing an honest first subscription. Atomically compare those candidates with the session’s eventual subscription when recording, and preserve enough identity after ordinary consumption to classify a racing first refusal. |
| + |
| + The new no-subscription retry is correct for an honest purchase, but **can hide a subscription made by hand**. Its test at `billingevidence_test.go:134` covers only the honest subscription and would pass with this gap. |
| + |
| +3. **P2 — Held currency and customer identity are not rechecked in the transaction that creates an intent.** |
| + |
| + **Where:** `internal/console/billingservice.go:573`, `:590`, `:603`; `internal/console/billingstore.go:372`. |
| + |
| + **Concrete scenario:** |
| + |
| + - Console A reads a never-purchased account while preparing a signed GBP checkout, then pauses before `beginBillingCheckout`. |
| + - Console B completes an EUR purchase, records it, and reconciles its cancellation. |
| + - A resumes. The intent transaction sees a canceled subscription and permits another purchase. It rechecks house status and membership, but neither the newly held EUR currency nor A’s stale empty customer. |
| + - A creates a new customer and offers a payable GBP checkout (`billingservice.go:1348`). |
| + - That checkout can complete and pass binding against its new intent. Recording then fails because the account’s existing EUR customer cannot be replaced (`billingstore.go:713`). |
| + |
| + This permits payment in a currency the account now holds against, followed by a purchase that cannot record. The process-local billing lock does not exclude the other console. |
| + |
| + **Smallest acceptable fix:** enforce the current held currency and customer snapshot inside `beginBillingCheckout` before inserting the intent. Reject stale state and re-render the quote before creating another customer or session. |
| + |
| + Existing held-currency tests at `billingcurrency_test.go:1330` and legacy page coverage at `:2131` are sequential and **pass with the missing transactional guard**. |
| + |
| +4. **P3 — A successful listing page’s evidence is discarded when a later page fails.** |
| + |
| + **Where:** `internal/billingstripe/catalog.go:156`, `:161`, `:167`, `:173`; `internal/console/billingretirement.go:685`. |
| + |
| + **Concrete scenario:** recovery for a new unbound intent lists customer sessions over its bounded window. An archived refused session from shortly before that intent falls within the 60-second overlap. Page one shows a contradiction of its archived fate. Page two fails or violates pagination checks. |
| + |
| + The client returns `nil,error`, discarding the successfully decoded first page. The wrapper returns before recording any session. A later corrected provider response can erase the only observed contradiction. |
| + |
| + **Smallest acceptable fix:** record each successfully decoded page before fetching the next, or return partial results and record them before propagating the listing error. |
| + |
| + The new tests and structural guard **pass with this gap**; they do not exercise partial listing success. |
| + |
| +5. **P3 — Refusal retries remain unfair within their own bounded batch.** |
| + |
| + **Where:** `internal/console/billingretirement.go:632`, `:790`; `internal/console/billingpass.go:62`. |
| + |
| + This is an acknowledged existing limitation, not a new delta defect. |
| + |
| + **Concrete scenario:** 36 older, persistently unproven refusals have reached hourly retry intervals, distributed across the twelve five-minute pass slots. Each pass spends its three-attempt budget on that slot’s older cases. A later, continuously due refusal can remain beyond the budget indefinitely. |
| + |
| + Separate deadlines prevent reconciliation and refusal work from starving each other. They do not provide fairness among refusals. |
| + |
| + **Smallest acceptable fix:** persist a refusal cursor or select eligible work by a fair next-attempt ordering. Test more cases than the hourly service capacity; the current one-refusal schedule tests do not distinguish this defect. |
| + |
| +**Claims the code contradicts, and the remaining requested checks** |
| + |
| +The strongest lifecycle claim is still false. In particular: |
| + |
| +- Design line 125 says nothing retires under an unbound review without a session reference. Finding 1 disproves that. |
| +- `billingoperator.go:421` says an intent is never gone at resolution. Unready cleanup can remove it. |
| +- `billingstore.go:1099` says a late filing is “a dispute and never nothing.” Finding 2 disproves that after ordinary consumption. |
| +- The header’s universal evidence claim at `billingretirement.go:40` does not cover successful pages discarded inside the provider client. |
| +- The stated exclusion of create answers is not literal: an owned complete/paid refused **create response** reaches `fileRefusal` through `refuseSession`, then `noteSessionSeen`, without a GET (`billingservice.go:758`, `:367`; `billingretirement.go:721`). This conservatively records a completion, but contradicts the claimed provenance. Separate filing from observation on create responses, or accurately document the exception. |
| + |
| +The actual provider-read inventory is: |
| + |
| +| Path | Recording behavior at this commit | |
| +|---|---| |
| +| Reconcile’s initial and locked subscription reads | Both use `readSubscription` (`billingservice.go:812`, `:828`). | |
| +| Reconcile’s refusal and matching-session reads | Both use `readCheckout` (`:888`, `:1006`). | |
| +| Binder, including pending/archive/fast paths | Uses `readCheckout` before those decisions (`:1175`). | |
| +| Previous/current checkout resume | Both use `readCheckout` (`:641`, `:706`). | |
| +| Pending pass | Uses `readCheckout` (`billingpass.go:227`). | |
| +| Recovery listing | Uses `listCheckouts` (`billingservice.go:1491`), subject to Finding 4. | |
| +| Operator keep gate | Uses `readCheckout` before every refusal (`billingoperator.go:247`). | |
| +| `advanceRefusal` | Both GETs use `readCheckout` (`billingretirement.go:755`, `:771`). Expiry responses do not establish refusal fate. | |
| +| `refuseSession` | Its input was read through a wrapper or came from create. Completed refusal files and observes it; ordinary expiry uses the store’s proof checks. | |
| +| Quantity worker | Uses `readSubscription` (`billingquantity.go:1010`). | |
| +| Invoice pass | Uses `readSubscription` (`billinginvoice.go:558`). | |
| + |
| +I found no remaining direct use of the three provider read methods outside these wrappers in `internal/console`. A store-recording error makes the wrapper return no object. The returns preceding classification are provider errors, the partial-list failure above, and `readSubscription`’s wrong-ID/wrong-mode exclusion (`billingretirement.go:705`). Missing metadata and purchases without a refusal also produce deliberate classification no-ops. |
| + |
| +The structural test catches receiver aliases, method values and method expressions because it checks selector names. It also catches same-named reads through another type, and reflection requires every new exported `billingstripe.Client` method to be classified. A differently named read on another client type is outside that inventory. |
| + |
| +Its wrapper check is **lexical, not control-flow analysis** (`billingevidence_test.go:625`). A recording selector in an unreachable branch, or an ignored recording error, can satisfy it. Behavioral tests distinguish the named concrete mutants; the guard does not independently prove that every execution records before returning. |
| + |
| +For the store classification: |
| + |
| +- `purchaseOfSessionTx` searches bound standing intent, referenced unbound standing intent, retirement archive, then kept-resolution archive. Session identity is compared with the recorded account, intent and known customer. |
| +- Standing refused purchases acquire one terminal fate. Later incompatible fate, identity or nonempty subscription association produces a dispute. Identity-less/open observations before any fate are deliberately forgotten; they authorize neither outcome themselves. |
| +- Retirements with proven expiry classify contradictory late sessions/subscriptions as disputes. Pre-protocol retirements retain their historical behavior. |
| +- Kept standing purchases reopen on another attached subscription. A disputed kept intent cannot be consumed. After consumption, session evidence is compared against archived identity and subscription. |
| +- An ordinary purchase with no refusal has no corresponding archive after consumption. That distinction causes Finding 2. |
| +- Legitimate kept completion redelivery and the account’s recorded subscription are excluded from spurious disputes. Renewal of that recorded subscription does not require fresh intent consumption. |
| +- Subscription attribution uses the account and intent named in provider metadata. A subscription naming another account’s retired ID alone does not match that retirement: the lookup is scoped by both account and intent. |
| +- Standing lookup takes precedence over archives. Honest Stripe session-ID uniqueness makes that appropriate; the database does not independently establish that an ID can never be rebound under provider-contradictory identities. |
| + |
| +The original record transaction is otherwise sound: consuming the exact standing intent, account/assignment changes, event insertion, entitlement queue and quantity refresh commit together. An open refusal or disputed intent aborts first recording and rolls those effects back (`billingstore.go:785`). Operator retirement archives and deletes exactly the reviewed intent. **Unready cleanup is the exception that breaks the advertised lifecycle.** |
| + |
| +Refusal filing and subscription attachment are now one transaction. SQLite writer serialization prevents another transaction seeing half of that pair. Evidence and webhook acknowledgement are separate transactions, correctly ordered: a crash before acknowledgement causes redelivery. This does not repair evidence deliberately discarded before filing or attribution lost through ordinary consumption. |
| + |
| +Dispute reviews are now inaccessible to resolution throughout stage one: an operator can inspect them, but the resolution API returns 409 and leaves the invoice hold. That absence of a disposition is stated honestly. `keep` still validates the session rather than re-running subscription validation; the governing “Stage one as built” explicitly documents that subsequent reconciliation can reopen the refusal. |
| + |
| +The operator door preserves exact actor/note/outcome replay, including omitted outcome meaning `retire`. Provider failures produce 503; held cases produce 409. The resolution-history row is the transactional audit; the general audit remains best-effort and can repeat. Billing-off behavior preserves already committed replays and refuses provider-dependent keep. |
| + |
| +The delete trigger blocks an older binary’s deletion of an unproven bound/refused session. It does **not** enforce the entire protocol against old binaries, and its unready exemption is now unsafe as described above. Stopping older consoles before migration and prohibiting rollback remain necessary. |
| + |
| +For **money, forms, schema and the site**: |
| + |
| +| Scope | Result | |
| +|---|---| |
| +| **A: GBP end to end** | The ordinary `?country=IN¤cy=gbp` purchase quotes band D—Personal £7.70—and posts that effective choice through the signed quote. Intent, checkout, subscription, account and book assignment retain GBP/book terms. `billingInvoiceHistory`, `AllowanceForMonth`, V2 calculation, invoice requests/recovery and quantity-price checks preserve them. I found no conversion path. Finding 3 is the remaining held-currency race. | |
| +| **B/C/M/Q/U/Z: one form, cookies, signature** | Present currency must be exactly valid; empty, repeated or invalid values are refused on both authenticated doors. Missing currency uses the country default for a book console and EUR for legacy. Changed effective currency/country or invalid signature redirects to a quote. The server-secret-derived HMAC covers session token, account, book, country, band, currency, choice flag and member count (`billingpage.go:446`). A stale tab’s unchanged valid quote remains valid; another tuple/session/token cannot borrow its signature. | |
| +| **Unsigned inputs** | Plan selects among rows already priced on the page. Quantity comes from membership and plan limits, with the member snapshot checked again under the membership write lock. Postal code affects location/tax, not the displayed pre-tax book price. | |
| +| **Cookies and held currency** | Present-empty clears; absent falls back; invalid sets nothing. An invalid visit can leave an old cookie available on a later parameter-absent visit. Cookies do not override the posted form. Held currency, including legacy EUR, is offered alone. | |
| +| **Quote door** | Session, form token and membership precede a redirect built from encoded posted values to the account’s billing path. It performs no checkout/provider operation and supplies no arbitrary redirect destination. | |
| +| **D/L/R/FF/HH/KK: ownership** | `ownSession` checks mode, intent, both account identifiers, known customer and bound session ID. Create/listing answers that cannot establish ownership are not expired. Expiry of a contradicted **bound** session uses the console’s previously established ID; that is the right authority and does not substitute proof of fate. Failures leave the review standing; payment winning expiry becomes completion evidence. | |
| +| **E/H/N/T: month barrier and rollout** | The refined trigger matches positive active paid spans, clipped by the next assignment across both tables, `period_end` and month end. Same-second supersession contributes no span; free/inactive rows do not count. I found neither requested valid-history counterexample. The book writer copies verified subscription `period_end`; renewal/quantity dating changes `effective_at`, not paid-through meaning. | |
| +| **Trigger migration** | `retireEarlierBarrierTrigger` verifies the new trigger before transactionally dropping the old name (`billingstore.go:857`). Missing new trigger fails safely; repeat startup is idempotent. An interruption may leave both temporarily, not neither. An older branch build can recreate the old name, hence the rollout restriction. | |
| +| **F: inclusions** | Figures use pooled allowances at charged quantity, divided per member, including unavailable Personal and teams above minimum (`billingpage.go:395`). Current data produces exact displayed shares at supported quantities; tests constrain that property. Band A’s approved sentences are pinned literally. | |
| +| **G/J: book/configuration/export** | Validation covers catalog completeness, duplicates, currencies/defaults, positive rates/floors, arithmetic and overflow. Optional other-book catalogs must be complete; provider price IDs are unique. Provisioning accepts at least the selected 36 keys and the own-only `currency_options` shape. The public export contains commercial/resource/country definitions, not private customer or credential data. | |
| +| **K: pricing site** | The site’s book is byte-identical to the platform export at the supplied commits. Rates retain exact micro-unit values; band scaling checks divisibility; country options and defaults come from the book. Both link parameters are always present, with currency populated only for a choice and country only as selected. Locale remains a display suggestion. | |
| +| **K/MM/QQ: draft and amount tests** | First-line draft marking and exact `PRICEBOOK_ALLOW_DRAFT=1` checks are present. Slot tests validate rendered values across currencies/bands; whole-token matching is executable. A literal equal to its slot’s current correct value still passes—the test explicitly claims output equality, not provenance. A changed-export rebuild is needed to prove derivation. | |
| +| **X/CC: quantity history** | `quantityOutcomesSQL` is the shared admitted-outcome list; changed guard text is replaced. The healing sweep is gone, and startup does not clear pre-history or clock-rollback holds. | |
| +| **ZZ/BBB/CCC: rejected listing** | Operator-only, at most 200, receipt time then insertion order descending. Rows are exhausted/closed before account lookups. Some bare identity rejections still carry account zero; retryable failures are not rejected rows. The bound test checks its selected count/endpoints/account assertions, not every possible ordering. | |
| + |
| +The review-three/four mutation qualifications are now mostly accurate: the equivalent choice-flag mutation can survive; removing a divisibility assertion over already-exact data proves nothing; provisioning checks read script text; successful migration does not prove interrupted ordering; redundant deletion predicates need not have distinguishing tests. The historical scratch-run totals are not independently verifiable from pinned source. |
| + |
| +For the **forty-three named mutants**, the following groups account for the listed mutations. Each is predicted to fail the relevant current assertions; that does not establish the missing interleavings in Findings 1–4. |
| + |
| +| Named mutations | Distinguishing current coverage | |
| +|---|---| |
| +| Omit refusal attachment; split filing commits; drop filing after archived retirement | `TestTheRefusalAndItsSubscriptionAreOneTransaction` | |
| +| Stop recording in checkout/subscription wrappers; direct keep-gate read | Keep-gate evidence, subscription attachment and structural-guard tests | |
| +| Bare rejection beside a completed session; omit unbound refusal | `billingevidence_test.go:98`, `:165` | |
| +| Omit retired/kept session lookup; ignore archived customer; forget retirement customer | `TestLateEvidenceIsAttributedByTheSessionTheConsoleRecorded` | |
| +| Resolve dispute on a note | `TestADisputeReviewIsNotResolvedByANote` | |
| +| Binder trusts archived response account, compares replacement intent, or retries archived noncompletion | Late-evidence and after-keep tests now assert response and account attribution | |
| +| Resume disputed kept intent at first or second read | Separate resume tests at `billingevidence_test.go:255`, `:294` | |
| +| Drop standing-fate contradiction, identity contradiction, retired completion, kept expiry, changed kept/standing subscription; permit disputed retirement | Recorded-fate, late-completion, keep-gate and after-keep tests | |
| +| Drop subscriptions after retirement/kept consumption; consume disputed kept intent; forget kept evidence | Atomic-filing and `TestEvidenceAfterAKeepSurvivesTheIntent` | |
| +| Bare two-item rejection | `TestAStructurallyInvalidSubscriptionNamingTheIntentIsReviewedAndAttached` | |
| +| Reuse reconciliation deadline; detach caller cancellation | Three cases at `billingretirement_test.go:971` | |
| +| Sort ties by ID ascending or descending | Arrival `b,c,a`, expected `a,c,b` | |
| +| Attach the session’s own subscription as another obligation | On-terms/keep fixtures distinguish the extra obligation | |
| +| Retry subscription with no standing intent | `billingevidence_test.go:445` | |
| +| Ignore intent, metadata account, client reference or mode individually | Individual edits at `billingretirement_test.go:928` | |
| +| Do not reopen after kept attachment | The incomplete-subscription case prevents later refusal logic masking the defect (`:801`) | |
| +| Do not re-observe the read that files refusal | `billingevidence_test.go:315` requires fate after one provider request | |
| +| Refuse an honest subscription before its session names it | `billingevidence_test.go:134` | |
| + |
| +The four former survivors now have distinguishing assertions. The expiry hook remains a stand-in for a competing store transaction, not two running consoles. Payment-before-retire is not an actual payment/expiry race. The legacy-retirement helper reconstructs plausible old effects; its retained protocol marker does not govern the gone-intent paths tested. The second session ID and late replay assertions now distinguish recording the retired purchase. |
| + |
| +I found no additional concrete shared-cookie, temporary-root or wall-clock failure. None of these tests establishes fairness beyond the refusal budget or the two-console held-currency race. |
| + |
| +**What must be verified against Stripe test mode before enablement** |
| + |
| +1. **Catalog and checkout:** verify all 36 identities, exact amounts/currencies, annual licensed quantities, tax configuration and expanded currency options. Complete USD/EUR/GBP purchases, including IN/GBP and fractional-pound Community pricing. Session, subscription, item and stored terms must agree; Adaptive Pricing must stay disabled. |
| + |
| +2. **Expiry and proof:** race retained-URL payment with expiry; exercise concurrent/replayed keys, remembered failures, lost responses and expired key retention. Confirm expiry prevents subsequent payment and subsequent GETs provide the required identity and terminal proof. Completed payment must never become expiry retirement. |
| + |
| +3. **Delayed association and payment:** exercise subscription events before session completion and before its subscription ID appears, delayed payment and `no_payment_required`. Include manually created H followed by legitimate C. The honest purchase must retry safely while every additional obligation remains durable. |
| + |
| +4. **Corrections and contradictions:** verify metadata/customer identity corrections, GET/listing visibility, client-reference mutability and whether completed-session subscription association can change. Contradictions must survive retirement, keep, consumption and partial listing failure. |
| + |
| +5. **Refused obligations:** exercise wrong-price, on-terms, multiple-item and additional subscriptions in both webhook orders. Expiring checkout must not settle another subscription. Establish what cancellation means for pending payments, collectible invoices and refunds. |
| + |
| +6. **Invoices and subscription lifecycle:** create, recover, finalize and pay metered invoices in each currency; check rates, floors, discounts, tax and immutable recovery hashes. Exercise membership quantity changes, proration, renewal, cancellation, delayed events and legacy EUR recovery. Verify customer-currency restrictions after cancellation. |
| + |
| +Separately, controlled local tests must cover the exact transaction boundaries above, including unready cleanup under review, ordinary consumption racing first refusal, two consoles creating against changed held currency, partial listing success and refusal-batch fairness. Stripe rehearsal cannot establish those database guarantees. |
| + |
| +**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 f0a982ef6..0584fcc8b 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -451,3 +451,39 @@ A mutant that dies shows a test reaches the line and asserts on its effect. It d |
| Stated and not changed: a read made before any refusal review exists records nothing (there is no refused purchase for it to be evidence about; the read that leads to a filing is handed in again once the review exists); an identity-less or still-open read before any fate is recorded records nothing (with no fate both outcomes are held); the answers to a create and to an expiry are not evidence (replays under an idempotency key); the account's own recorded subscription is not classified, whatever intent its metadata names; a record refused because its kept intent is disputed is still a conflict the provider retries (503), as a record under an open review is, and stage one has nothing that ends it; a review resolved before the protocol and filed again becomes protocol 1 (unmerged builds only). |
| + |
| +## Review twenty-one (reviewed `b260ac8e`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-multi-currency-review21.md`, `CODEX-FINDINGS-multi-currency-review21.md`. Both found every round-twenty finding closed and the structural guard sound. Both found the same new P2, which round twenty's own fix opened: a refusal can be filed on an intent that never had a session, and the cleanup of such an intent ignored the review. Astra found two more P2s and two P3s; Opus three P3s. **Review eighteen's P2 stays recorded as open until a round accepts this code.** |
| + |
| +Four of the eight findings are one fault seen from four sides: a subscription that names an intent was durable only once a refusal review existed, and three different things could delete the intent first (its purchase's record, its session's expiry, the cleanup of an intent with no session). They are answered as one rule, stated in the header of `billingretirement.go` and the design's "Stage one as built": **a subscription that names a standing intent is recorded from the first read, and nothing deletes an intent past one.** |
| + |
| +| Finding | Disposition | Where | Counterexample test | |
| +| --- | --- | --- | --- | |
| +| Opus P2-1, Astra 1: an intent that never had a session, under a refusal review with a subscription attached, was deleted by the 25-hour cleanup and by the preparation of an account's deletion; `retire` then passed, because an intent that does not stand skipped every hold. | Fixed in three places, each sufficient for the scenario. The cleanup refuses an intent under an open review, in its own delete. `held` checks the protocol, a dispute and what is attached before it asks whether the intent stands. The delete barrier no longer exempts an unready intent whose review is open (it is dropped and made again at every start, in one transaction). | `billingstore.go` `discardUnreadyIntent`; `billingretirement.go` `held`, `migrateBillingRetirement` | `TestAnUnreadyIntentUnderReviewIsNotCleanedUp` (the pass at +26h, the deletion's preparation, the cleanup itself, then `retire` 409 naming the subscription), `TestAReviewWhoseIntentIsGoneIsStillHeldForWhatWasAttached`, `TestTheDeleteBarrierCoversAnUnreadyIntentUnderAnOpenReview` | |
| +| Astra 2: a subscription seen before any refusal (the session complete and naming none yet; the session open; the subscription incomplete; the intent unbound) was recorded nowhere, and vanished when the honest purchase consumed the intent. The two-console form: a refusal filed after another console recorded the purchase found no archive. | Fixed by the rule. The classification records a subscription naming a standing intent whether or not a review exists. The record of a purchase keeps what it consumed (`billing_consumed_intents`) and, in its own transaction, makes every other subscription seen naming the intent a dispute. A subscription that names the intent afterwards, or a refusal filed late, is compared with that record and is a dispute. The honest purchase alone makes none. | `billingretirement.go` `classifySubscriptionTx`; `billingstore.go` `applyBillingStateDated`, `fileCheckoutRefusal` | `TestASubscriptionSeenBeforeThePurchaseRecordsIsNotLostWithTheIntent` (three cases) | |
| +| Opus P3-2: a hand-made subscription naming a bound intent whose session is open was retried and never reviewed; if the session expired the intent was retired with no review. | Fixed by the rule, without refusing on an open session (Astra's caution: the purchase's own subscription can be read while its session still reads open). The subscription is recorded at the read. When the session expires, the expiry does not retire the intent: it files the refusal, naming the session, with the subscription attached; the fate is left for a read to prove. | `billingstore.go` `retireIntent`; `billingretirement.go` `holdForNamedSubscriptionsTx` | `TestAnExpiryDoesNotRetireAnIntentASubscriptionNames` (the pass and the click), `TestTheCleanupOfAnUnreadyIntentASubscriptionNamesFilesItsRefusal` | |
| +| Astra 3: the currency an account is held to, and its customer, were read by the caller and not again in the transaction that creates the intent; a second console recording and ending a purchase in between let a stale request create a second customer and a checkout whose purchase could not record. | Fixed: `beginBillingCheckout` compares the request's customer and money with the account's, in its transaction, and refuses a stale one. | `billingstore.go` `beginBillingCheckout` | `TestAnIntentIsNotCreatedFromAStaleReadingOfTheAccount`; `TestBillingStateAtomicEventAndRevision` | |
| +| Opus P3-1: a bind could move an intent under review onto another session, or onto a retired purchase's session. | Fixed: one store method writes a session id onto an intent, and refuses a session that is another purchase's (another intent's, a retired, kept or consumed one's) and one that is not the session the intent's open review names. The binder's own `UPDATE` is gone. | `billingstore.go` `bindIntentSession`; `billingretirement.go` `sessionBelongsElsewhereTx` | `TestASessionIsBoundOnlyWhereItBelongs` | |
| +| Opus P3-3: the store keyed on the id in the answer, not the id asked for. | Fixed: `readCheckout` records only an answer that carries the id it asked for. | `billingretirement.go` `readCheckout` | `TestAnAnswerAboutAnotherSessionIsNotRecorded` | |
| +| Astra 4: a listing's first page was discarded when a later page failed. | Fixed: the client returns the pages it read with the error, and the wrapper records them before the error is the caller's. | `billingstripe/catalog.go` `listCheckouts`; `billingretirement.go` `listCheckouts` | `TestAListingThatFailsLaterStillRecordsWhatItRead`; `TestListCheckouts_ALaterPagesFailureKeepsTheEarlierPages` | |
| +| Astra 5: the refusals' turn took intents in the order they stand, so older ones could use the budget for ever. | Fixed: the longest-due first. | `billingretirement.go` `billingPassRefusals` | `TestTheRefusalsTurnTakesTheLongestDueFirst` | |
| +| Claims: "nothing read about a refused purchase is dropped"; "never by what the answer claims"; "nothing retires under an unbound review"; "an intent is never gone at its resolution"; "a dispute and never nothing"; the create answer's exception; the review-twenty row on an unbound intent. | Corrected. The header and the design state the rule, what it covers and what is lost outside it, including the one exception for a create answer that shows a completion. An on-terms subscription naming an unbound intent is a retry, and is now recorded at the read. | header, design, comments | | |
| + |
| +Found while answering, by neither reviewer, and of the same family as review eighteen's P2: **a later event of an already recorded subscription deleted whatever standing intent its metadata named.** The record's delete of the intent ran for every state change, not only a first purchase; a subscription that had ended, with its metadata made to name a newer purchase's standing intent, deleted that intent while its session could still be paid. Fixed: only a first purchase consumes an intent, and whether a record is a first purchase is judged in the record's transaction from the account's own row (the subscription is not yet the recorded one), not from what the caller read (`billingstore.go` `applyBillingStateDated`; `TestALaterEventOfARecordedSubscriptionConsumesNoIntent`). |
| + |
| +Existing tests that changed, and why: `TestCheckoutAfterAPurchaseKeepsItsCurrency` (the fake reused one session id for two purchases, which the bind now refuses; the second checkout has its own id); `TestEvidenceAgainstARecordedFateIsADispute` (an expiry proof for an intent a subscription names now files the review again; the dispute clause is tested with that taken away by hand); `TestBillingStateAtomicEventAndRevision` and `TestBillingIntentPinsCustomerAndCoupon` (hand-built requests now name the customer the account has, as the start path does). |
| + |
| +Mutation checks, in a tree copy, twenty-seven, each run against the lifecycle tests and the client's listing tests; all die. The cleanup deleting an unready intent under an open review; the barrier exempting every unready intent; an intent that is gone skipping every hold; a subscription naming an intent with no review not recorded; the record making no dispute of the others seen; the record keeping nothing of what it consumed; a subscription naming a consumed intent being nothing; the purchase's own subscription disputed at its record; an expiry retiring an intent a subscription names; the cleanup deleting one; the refusal an expiry files naming no session; a bind taking a session that is another purchase's, one other than the review names, and a consumed purchase's; an intent created from a stale customer, and in a currency the account is not held to; an answer carrying another id recorded; a failed listing recording nothing; the client dropping the pages before a failure; the refusals' turn taking them as they stand; the hold filing again under an open review; the pass reporting an intent left under review as a failure; the expiry that files a review also writing the fate; the retirement proof's identity not judged; an attachment after keep not reopening the review; what is attached not holding a retire; a later event of a recorded subscription consuming the intent it names. Two needed a second run: one mutant did not compile as first written, and one (the proof's identity) is killed by tests outside the first run's selection (`TestAnExpiryAnsweredForAnotherAccountRetiresNothing`). Output: `currency-mutants-round21.txt` in the run directory's logs. |
| + |
| +The full `internal/console`, `internal/plans` and `internal/billingstripe` packages passed on the tree before the last change (console 417 s, `currency-round21-run1.txt`). A second run after "only a first purchase consumes an intent" failed one test, `TestMembershipPaidButUnreconciledPersonalNeverGainsSeat` (`currency-round21-run2.txt`): the first form of that fix took the caller's word for what a first purchase is, and that test records a first purchase through a helper that does not say. The fix now judges it from the account's row in the transaction, which is also the better rule. The gate on the commit is the run that counts. |
| + |
| +Stated and not changed: |
| + |
| +- A subscription first seen naming an intent that already expired in the ordinary way, with nothing seen naming it before, is a rejected event with its account and no hold. Nothing is kept of such an intent. |
| +- A subscription seen naming an intent holds that intent's expiry even when the subscription is dead (`incomplete_expired`). The buyer then waits on a person. Whether the provider ever makes a subscription before its checkout session completes decides how often an honest buyer can meet this; it is on the test-mode list. |
| +- A dispute made at a record holds the account's invoices and has no resolution in stage one, like every dispute. |
| +- The refusals' budget is still three a pass; fairness is by due time, and there is no cursor. |
| +- The structural guard's check that a wrapper records before it returns is lexical, as Astra says. The behavioural tests and the mutants are what show each wrapper records. |
| +- No test runs two console processes. The two-console findings are tested through the store's transactions, one after another. |
| + |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review21.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review21.md| new file mode 100644 |
| index 000000000..acfbc2cd7 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review21.md |
| @@ -0,0 +1,160 @@ |
| +# Twenty-first review of `feat/carloku-multi-currency-20261001` at `b260ac8e` |
| + |
| +Base is `12e70d95`; the delta reviewed is `43d326b5..b260ac8e`. I read sources only with `git show b260ac8e:`, plus the pricing site at `01b62f7`. I modified nothing and ran nothing. |
| + |
| +**Verdict: accept with fixes.** The new shape works. Every read of a session or subscription in `internal/console` goes through `readCheckout`, `listCheckouts` or `readSubscription`. The store classifies each read by the session id it recorded, and the guard test enforces this. All of round twenty's findings are closed. |
| + |
| +One new P2 remains, and this round opened it. A refusal can now be filed against an unbound intent, but a stillborn intent is still deleted with no regard to its review. After that, `held("retire")` waves the retire through with the subscription still attached. |
| + |
| +## Round twenty, finding by finding |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1**: on-terms H beside a complete session, with no review yet | **Closed.** Reconcile now calls `refuse()` when the session's subscription is not `id` (`billingservice.go:1029-1035`). It retries when the session names none yet (`:1022`). | **No.** In `TestAnOnTermsSubscriptionSeenBeforeAnyReviewIsAttachedAndHoldsKeep`, the old bare answer leaves `st.Exists` false. | |
| +| Opus P2-1, unbound half | **Closed for off-terms H** (`:869-879`). On-terms H naming an unbound intent is still a retry, which is right for a lost create. **But the new branch opens P2-1 below.** | **No** (`TestASubscriptionNamingAnUnboundIntentIsReviewedAndAttached`). | |
| +| **Opus P3-1**: the keep gate dropped the reads it refused | **Closed.** The gate reads through `readCheckout` (`billingoperator.go:247`). | **No.** Both halves of `TestAReadTheKeepGateRefusesIsStillEvidence` count disputes. | |
| +| **Opus P3-2**: binder attribution by metadata or by the replacement intent | **Closed.** `purchaseOfSessionTx` finds the purchase by session id (`billingretirement.go:342-385`), and the archives now keep the customer. | **No.** Each of the three subtests of `TestLateEvidenceIsAttributedByTheSessionTheConsoleRecorded` fails. | |
| +| **Opus P3-3**: the binder answered 503 for a gone purchase | **Closed** (`billingservice.go:1182-1187`, `:1217-1222`, `:1235-1236`, `:1250-1254`; reconcile `:980`). | **No.** The status codes are asserted now. | |
| +| **Opus P3-4**: the atomicity test | **Closed.** An abort trigger on the obligations insert runs inside the production helper. | **No.** A two-commit mutant leaves the review row behind. | |
| +| **Opus P3-5**: the pass's cancellation | **Closed.** Both turns are children of `ctx` (`billingpass.go:160-167`), and the design now states that production never cancels it. | **No.** Case 3 (`expireHook = cancel`) fails under `WithoutCancel`. | |
| +| **Opus claims 1–8** | 1–4 restated; 5 fixed (dispute not resolvable); 6 comments fixed; 7 fixed; 8 brief now names `01b62f7`. The new header has residual overstatements (see Claims). | n/a | |
| +| **Astra 1 (P2)**: reads returned before they were recorded | **Closed by the shape.** | **No** (keep-gate, resume and second-read tests). | |
| +| **Astra 2 (P2)**: filing for a gone intent dropped H | **Closed.** `fileCheckoutRefusal` calls `classifySubscriptionTx` in the same transaction (`billingstore.go:1094-1103`). | **No.** The second half of `TestTheRefusalAndItsSubscriptionAreOneTransaction` counts one dispute. | |
| +| **Astra 3 (P2)**: attribution and customer | **Closed**, as for Opus P3-2. | **No.** | |
| +| **Astra 4 (P2)**: a dispute resolved on a note | **Closed** (`billingstore.go:983-985`). | **No.** `TestADisputeReviewIsNotResolvedByANote` would get 200. | |
| +| **Astra 5 (P3)**: pass cancellation | **Closed**, as for Opus P3-5. | **No.** | |
| +| **Astra 6 (P3)**: atomicity test | **Closed**, as for Opus P3-4. | **No.** | |
| +| **Astra 7 (P3)**: the site's fractional floor | **Closed at `01b62f7`.** The test calls `moneyText('$', 120.5, 'USD')` and asserts that it throws. A residual source-text assertion remains (line 255), see Claims. | **No** for the behavioural assertion. | |
| +| **Astra's claims** | "One test per interleaving" is withdrawn. The `retire` comments are corrected. The dispute acknowledgement is now refused. | n/a | |
| + |
| +**Where the severities differed, the dispositions took Astra's reading, and that was right in both cases:** |
| +- **The keep gate's dropped read.** At `43d326b5`: |
| + 1. The fate is recorded `expired`. |
| + 2. At the keep, the gate reads the session complete and paid, then answers 409 for another reason before recording anything. |
| + 3. A later `retire` skips the provider, because a fate already exists, and passes on the stale expiry. |
| + |
| + That retires a paid session, which is a release, so P2 is right. My P3 was wrong. |
| +- **The dispute resolved on a note.** The resolution lifted the invoice hold with no read, which is a release, so P2 is right. It is closed, and its test fails without the fix. |
| + |
| +## Findings |
| + |
| +### P2-1. A stillborn intent under a refusal review, with a subscription attached, is deleted, and then `retire` releases the case |
| + |
| +This is new in this round. `refuse()` can now file against an unbound intent. |
| + |
| +- **Where:** |
| + - The new branch: `billingservice.go:869-879` files `fileCheckoutRefusal(*pending, "", id)` for any unbound standing intent, ready or not. |
| + - The pass: `billingpass.go:150-155` sends an unbound intent whose review names no session (`pinned()==""`) to `pending`, not `refused`. `reconcilePendingIntent` (`:288-297`) then calls `discardUnreadyIntent` after 25 hours. |
| + - `resolveIntentForDeletion` (`billingservice.go:799-801`) discards it unconditionally. |
| + - `discardUnreadyIntent` (`billingstore.go:514`) deletes with no review clause. The barrier trigger exempts `session_id='' AND customer_ready=0` (`billingretirement.go:157`). |
| + - `held()` returns `""` for `retire` on any intent that does not stand, before it looks at obligations, protocol or dispute (`billingretirement.go:194-199`). |
| +- **Scenario:** |
| + 1. Intent I is created. `ensureBillingCustomer` fails partway, so the customer exists at Stripe but `customer_ready=0` and I is unbound. |
| + 2. Staff make H by hand for that customer, naming I, off-terms. |
| + 3. H's event reaches reconcile. `refuse()` files the review with H attached. So far this is correct. |
| + 4. More than 25 hours after creation, the pass discards I. The trigger allows it. |
| + 5. The operator's `retire` passes `held()` (I does not stand), resolves the review, and lifts the hold. |
| + 6. H's later events go through `classifySubscriptionTx`, which finds H among the obligations and records nothing. Reconcile finds no intent standing and settles the event as a rejected row. |
| + 7. H bills at Stripe while the console's record says the case was resolved. |
| +- **Smallest fix:** |
| + 1. In `held()`, check `Protocol`, `Disputed` and `Obligations` before the `!Stands` shortcut. An intent that is gone with obligations is held. |
| + 2. Give `discardUnreadyIntent` an `AND NOT EXISTS (… billing_reviews … reason=refused)` clause, and narrow the trigger's stillborn exemption the same way. |
| + 3. Test: an unready intent, H refused, a pass at +26h, then `retire` must answer 409 naming H and the intent must still stand. |
| + |
| +### P3-1. A bind can move an intent under review onto a different session, or onto an archived one |
| + |
| +- **Where:** |
| + - The binder's bind is `UPDATE … WHERE session_id='' OR session_id=?` (`billingservice.go:1284`). |
| + - `saveBillingCheckout` (`billingstore.go:411`). |
| + - Neither checks an open review whose reference is another session, or a session already in `billing_retired_intents` or in a keep's evidence. |
| + - `purchaseOfSessionTx` tries the standing intent first (`billingretirement.go:344`). |
| +- **Scenario A:** |
| + 1. Unbound I has an open review referencing S1, which is complete and paid in the wrong currency. |
| + 2. A webhook arrives for a hand-made S2 that names I and is on terms. `ownSession` passes because I is unbound, and the bind succeeds. |
| + 3. `pinned()` is now S2. The keep gate reads S2 (complete, paid) while the fate recorded from S1 is complete. `keep` passes and C2 records. |
| + 4. S1's subscription C1 is the recorded `SessionSubscription`, which becomes the keep's `evidence_subscription`. Its later events classify as the kept one, with no dispute. |
| +- **Scenario B:** a retired session whose `checkout_intent` metadata was edited to the replacement intent is found by `listCheckouts`. The dispute is recorded, but the session is then saved onto the replacement. From then on, standing-first lookup classifies it against the replacement, and the retired purchase's evidence stops being recorded. The replacement is also wedged: the binder answers "archived" on every pass. |
| +- **Fix:** refuse to bind (errBillingIdentity) when an open refusal review on the intent references a different session, or when `purchaseOfSessionTx` already finds the session as retired or kept. |
| + |
| +### P3-2. A hand-made subscription naming a bound intent whose session is open or expired is a retry, never a refusal |
| + |
| +- **Where:** `billingservice.go:1011-1013`, `if session.Status != "complete" { return errBillingPending }`. |
| +- **Why it matters:** an open or expired session has no subscription, so H naming I is provably not the checkout's own. It is retried for the provider's three days. |
| +- **What follows:** |
| + - If the session expires, the ordinary expiry retires I with no review. H then settles as a rejected row (which is the deliberate `TestASubscriptionNamingAnOrdinarilyExpiredIntentIsSettledWithItsAccount`), with no hold. |
| + - If the buyer later pays and the purchase records, H is a bare rejection with account 0, because `state.Subscription` is active (`:975-977`). |
| + - It is visible, but only in the rejected list. |
| +- **Fix:** treat an `open` or `expired` session named by a subscription as `refuse()`. The open-review clause then stops the ordinary expiry. |
| + |
| +### P3-3. `noteSessionSeen` keys on the answer's id, not on the id the console asked for |
| + |
| +- **Where:** `readCheckout` hands `session` in whole (`billingretirement.go:671-680`). The store looks up `session.ID`. |
| +- **Consequence:** an answer to GET S1 that carries S2 is classified against S2's purchase before the caller treats it as a failed read. The header says "the lookup does not depend on anything the provider's answer says"; it does depend on the id. |
| +- **Fix:** pass the requested id. Record nothing, and fail the read, when the two ids differ. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **Header (`billingretirement.go:40-65`) and design `:115`:** "Nothing read about a refused purchase is dropped." P2-1 and P3-1 drop H and S1 respectively. The design's "never by what the answer claims" is contradicted by P3-3. |
| +2. **`billingpass.go:143-147` and design:** "One whose session is not known … is still reconciled … Nothing records or retires under the review on that path either." `discardUnreadyIntent` removes it on that path (P2-1). |
| +3. **Dispositions, review twenty, the Opus P2-1 row:** "one that names an unbound standing intent files it with no session named." That holds only for off-terms H. On-terms H against an unbound intent is a retry, which is the correct choice, but it should be stated. |
| +4. **Site `test/pricing-pricebook.test.mjs:255`:** this assertion still reads source text (`src.includes('const money = … moneyText …')`). The wiring it checks is not exercised behaviourally. That is acceptable only because the slot tests compare the rendered output. |
| + |
| +The mutation table's four survivors and the tests added for them check out from the source: each named test reaches the line it is meant to kill. I cannot confirm the thirty-nine first-run results or the count of forty-three. |
| + |
| +## Sections |
| + |
| +**Pricing sections (A–Q, K/MM/QQ):** the delta changes no pricing, form, quote-signature, cookie, trigger, export or site code. The only change in these files is the `readSubscription` call at `billinginvoice.go:558` and `billingquantity.go:1010`. I checked that from the diff and did not re-trace the unchanged code this round. Rounds 15–20, by both reviewers, found no defect there. The GBP path and the ownership predicate are unchanged. |
| + |
| +**SSS, the guard:** |
| +- **What it catches:** any selector use of the five session- and subscription-read methods outside their wrappers, including method values, interface calls, aliases and other types. That is complete for reads through `billingstripe.Client`. It does not catch reflection or raw HTTP, which nothing here uses. |
| +- **What it cannot see:** that `readSubscription` records only when the id and mode match. That is safe, because callers refuse both. |
| +- **The exclusions:** |
| + - **Create answers:** nothing is lost. Every decision is taken on a later read, or on `retireIntent`'s own checks of the answer. |
| + - **Expiry answers:** nothing is lost. `advanceRefusal` re-reads, and `refuseSession` retires only when no review is open. |
| + - **Reads before any review:** what is lost is P3-2, plus H read while S is open. Most of that is recovered once a review exists, because H's later `invoice.paid` events then classify and attach it. |
| + - **Identity-less or open reads before any fate:** nothing is lost. With no fate, both outcomes are held. |
| +- **Can a caller see an unrecorded read?** No. A failed recording returns `nil, err`. |
| + |
| +**TTT, `purchaseOfSessionTx` and `noteSessionSeen`:** |
| +- **Lookup order** (standing, then referenced-unbound, then retired, then kept) is right, except for the rebinding in P3-1. |
| +- **Honest late events** produce no dispute: |
| + - The kept purchase's own completion redelivered: identity holds, and its subscription is the kept one. |
| + - A renewal: the subscription is excluded as the account's recorded one. |
| + - An ordinary purchase's session: it has no refusal row. |
| +- **Wrong account:** a dispute is always filed on the account the purchase recorded. |
| + |
| +**UUU:** `classifySubscriptionTx` is correct. The recorded subscription is excluded. A standing intent with no review records nothing, and reconcile's `refuse()` then decides. The gone-intent filing classifies against the archive. |
| + |
| +**VVV, the three new `refuse()` sites:** |
| +- **Can a legitimate purchase be refused at any of them?** No: |
| + - The subscription is read before the session names it: that is a retry (`:1022`). |
| + - The subscription event arrives before the session completes: that is a retry (`:1011`). |
| + - The legitimate purchase always has exactly one item. |
| +- **The unbound intent:** |
| + - Ready: listed and bound by the pass. |
| + - Not ready: discarded. That is P2-1. |
| + |
| +**WWW:** both the archived answers and the fast path between them are right. The start path stops a disputed kept intent after each read (`:651`, `:713`). |
| + |
| +**XXX:** a dispute is reachable by no operator path. It is honestly stated as held with no disposition. |
| + |
| +**YYY:** both turns are children of the caller's context, and the three test cases establish what they name. |
| + |
| +**ZZZ2:** each new test drives the production door it names and fails without its code, as tabled above. No test runs two consoles, which is stated. |
| + |
| +**The no-subscription retry:** it could hide a hand-made H only if the provider showed a completed subscription-mode session with no subscription for longer than its retry window. Verify that this never happens; it is item 3 below. |
| + |
| +**DDD and review eighteen's P2:** open only through P2-1. I found no other actor or interleaving that deletes or consumes an intent while its session is payable, or releases a hold with a subscription known. |
| + |
| +## Verify against Stripe test mode before enabling |
| + |
| +Every earlier list stands. Add these: |
| + |
| +1. A hand-made subscription on the exact price, naming a bound intent, created **after** the session completes and **before** any review exists. Its first event must file the review with it attached, and `keep` must answer 409. |
| +2. The same, while the session is still **open**. Today this is a retry (P3-2). |
| +3. Whether a subscription-mode session can ever read `complete` with no `subscription`, and for how long. |
| +4. Whether checkout session metadata can be edited after creation (P3-1 B; the identity clauses). |
| +5. Whether `client_reference_id` can be edited after creation. |
| +6. A GET on a checkout session id never answers another session's object (P3-3). |
| + |
| +**Verdict: accept with fixes.** Fix P2-1 (`held()` and `discardUnreadyIntent` with the trigger); then P3-1 to P3-3 and claims 1–3. |
diff --git a/internal/billingstripe/catalog.go b/internal/billingstripe/catalog.go| index 2e21a08e5..6978681d8 100644 |
| --- a/internal/billingstripe/catalog.go |
| +++ b/internal/billingstripe/catalog.go |
| @@ -153,24 +153,29 @@ func (c *Client) listCheckouts(ctx context.Context, customerID string, createdAf |
| Data []Session `json:"data"` |
| HasMore bool `json:"has_more"` |
| } |
| + // A listing that fails after its first page returns, with the |
| + // error, the sessions of the pages it did read: each of those was |
| + // a provider answer, and a caller that records what it reads must |
| + // not lose them to a later page's failure. A caller that wants a |
| + // whole listing checks the error, as before. |
| if err := c.do(ctx, http.MethodGet, "/v1/checkout/sessions", q, "", &page); err != nil { |
| - return nil, err |
| + return out, err |
| } |
| for _, session := range page.Data { |
| if !validID(session.ID) || seen[session.ID] { |
| - return nil, &Error{Code: CodeBrokenPagination} |
| + return out, &Error{Code: CodeBrokenPagination} |
| } |
| seen[session.ID] = true |
| out = append(out, session) |
| } |
| if len(out) > 10000 { |
| - return nil, &Error{Code: CodeTooManyRecords} |
| + return out, &Error{Code: CodeTooManyRecords} |
| } |
| if !page.HasMore { |
| return out, nil |
| } |
| if len(page.Data) == 0 { |
| - return nil, &Error{Code: CodeBrokenPagination} |
| + return out, &Error{Code: CodeBrokenPagination} |
| } |
| after = page.Data[len(page.Data)-1].ID |
| } |
diff --git a/internal/billingstripe/catalog_test.go b/internal/billingstripe/catalog_test.go| index b2edf4fe5..7c041ab07 100644 |
| --- a/internal/billingstripe/catalog_test.go |
| +++ b/internal/billingstripe/catalog_test.go |
| @@ -302,6 +302,31 @@ func TestListCheckouts_UsesStartingAfterOnSubsequentPage(t *testing.T) { |
| } |
| } |
| +// A listing that fails on a later page returns the sessions of the pages |
| +// it read together with the error: each was a provider answer, and a |
| +// caller that records what it reads must not lose them. |
| +func TestListCheckouts_ALaterPagesFailureKeepsTheEarlierPages(t *testing.T) { |
| + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + if r.URL.Query().Get("starting_after") == "" { |
| + _ = json.NewEncoder(w).Encode(map[string]any{ |
| + "data": []map[string]any{{"id": "cs_1", "livemode": true}, {"id": "cs_2", "livemode": true}}, |
| + "has_more": true, |
| + }) |
| + return |
| + } |
| + w.WriteHeader(http.StatusInternalServerError) |
| + })) |
| + defer srv.Close() |
| + c := testClient(t, srv.URL) |
| + out, err := c.ListCheckouts(context.Background(), 0, 100) |
| + if err == nil { |
| + t.Fatal("a failed second page: no error") |
| + } |
| + if len(out) != 2 || out[0].ID != "cs_1" || out[1].ID != "cs_2" { |
| + t.Fatalf("the pages read before the failure: %+v", out) |
| + } |
| +} |
| + |
| func TestListCheckouts_RejectsInvalidBoundsWithoutHTTPCall(t *testing.T) { |
| calls := 0 |
| srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls++ })) |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index 4920ee5a7..e8f4d54c2 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -1367,9 +1367,11 @@ func TestCheckoutAfterAPurchaseKeepsItsCurrency(t *testing.T) { |
| t.Fatal("an intent was opened in another currency") |
| } |
| // In pounds again, it goes ahead (and from the United States, with the |
| - // currency chosen). |
| + // currency chosen). A new checkout is a new session: the first one's |
| + // id is the console's record of the purchase it made, and is bound to |
| + // nothing else. |
| f.mu.Lock() |
| - f.sessionStatus = "open" |
| + f.sessionStatus, f.sessionID = "open", "cs_test_again" |
| f.mu.Unlock() |
| if dest, err := fx.srv.startBillingCheckout(ctx, acct, plans.PlanPersonal, "US", "94107", "gbp", f.now.Add(4*time.Minute)); err != nil || dest == "" { |
| t.Fatalf("in pounds again: dest %q err %v", dest, err) |
diff --git a/internal/console/billingnamed_test.go b/internal/console/billingnamed_test.go| new file mode 100644 |
| index 000000000..3e21e25a0 |
| --- /dev/null |
| +++ b/internal/console/billingnamed_test.go |
| @@ -0,0 +1,675 @@ |
| +package console |
| + |
| +import ( |
| + "context" |
| + "database/sql" |
| + "errors" |
| + "fmt" |
| + "net/http" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/plans" |
| +) |
| + |
| +// Review twenty-one's counterexamples. One rule answers them: a |
| +// subscription that names an intent is recorded from the first read, |
| +// whether or not a refusal review exists yet, and an intent is never |
| +// deleted (its session's expiry, the cleanup of one that never had a |
| +// session, the record of its purchase) while one is unaccounted for. |
| + |
| +// unreadyIntent leaves the account an intent that never reached the |
| +// provider's checkout: its customer was made and failed verification, so |
| +// it has no session and is not ready. The fake's subscription then names |
| +// that intent on the given price. |
| +func unreadyIntent(t *testing.T, price func(billingIntent) string) (*consoleFixture, *fakeBillingStripe, *Account, billingIntent) { |
| + t.Helper() |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "FR" // the provider's customer is in France |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err == nil { |
| + t.Fatalf("fixture: the checkout went through: %q", dest) |
| + } |
| + intent, err := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || intent.SessionID != "" || intent.CustomerReady { |
| + t.Fatalf("fixture: intent %+v %v; want it standing, unbound and not ready", intent, err) |
| + } |
| + f.mu.Lock() |
| + f.meta = map[string]string{"account_sqid": acct.Sqid, "checkout_intent": intent.ID, "plan": intent.Plan, "band": intent.Band, "terms_version": intent.Terms, "price_book": intent.PriceBook, "currency": intent.Currency} |
| + f.subscriptionPrice, f.subscriptionCurrency = price(intent), intent.Currency |
| + f.mu.Unlock() |
| + return fx, f, acct, intent |
| +} |
| + |
| +func standing(t *testing.T, fx *consoleFixture, acct *Account, intent billingIntent) { |
| + t.Helper() |
| + if got, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || got.ID != intent.ID { |
| + t.Fatalf("the intent: %+v %v; want it standing", got, err) |
| + } |
| +} |
| + |
| +// An intent that never had a session, under a refusal review with a |
| +// subscription attached, is not cleaned up: not by the pass after its |
| +// twenty-five hours, not by the preparation of an account's deletion. |
| +// And a retire of the review is held for the subscription. (Review |
| +// twenty-one: Opus P2-1, Astra 1.) |
| +func TestAnUnreadyIntentUnderReviewIsNotCleanedUp(t *testing.T) { |
| + fx, f, acct, intent := unreadyIntent(t, func(billingIntent) string { return currencyPriceID(plans.PlanPersonal, plans.BandD, plans.GBP) }) |
| + if code := f.webhookPost(t, fx, "evt_hand_unready", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the hand-made subscription: %d; want it settled as refused", code) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || len(st.Obligations) != 1 || st.Obligations[0] != "sub_paid" { |
| + t.Fatalf("fixture: %+v %v", st, err) |
| + } |
| + if err := fx.srv.billingPass(context.Background(), f.now.Add(26*time.Hour)); err != nil { |
| + t.Fatalf("the pass a day later: %v; want it to leave the intent and report nothing", err) |
| + } |
| + standing(t, fx, acct, intent) |
| + if err := fx.srv.resolveIntentForDeletion(context.Background(), acct, f.now.Add(27*time.Hour)); err != nil { |
| + t.Fatalf("the preparation of the account's deletion: %v", err) |
| + } |
| + standing(t, fx, acct, intent) |
| + if deleted, err := fx.st.discardUnreadyIntent(intent, f.now.Add(28*time.Hour)); deleted || err != nil { |
| + t.Fatalf("the cleanup itself: deleted %v err %v; want it refused quietly", deleted, err) |
| + } |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-unready") |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "", "nothing was ever paid"); status != http.StatusConflict || !strings.Contains(msg, "sub_paid") { |
| + t.Fatalf("retire: %d %q; want 409 held, naming the subscription", status, msg) |
| + } |
| + standing(t, fx, acct, intent) |
| + if open, _ := fx.st.refusalReviewOpen(acct.ID, intent.ID); !open { |
| + t.Fatal("the review was resolved") |
| + } |
| +} |
| + |
| +// What is attached, the protocol and a dispute hold a retire whether or |
| +// not the intent still stands: a review whose intent is gone is not |
| +// released past them. |
| +func TestAReviewWhoseIntentIsGoneIsStillHeldForWhatWasAttached(t *testing.T) { |
| + for name, tc := range map[string]struct { |
| + st refusalState |
| + want string |
| + }{ |
| + "a subscription attached": {refusalState{Exists: true, Open: true, Protocol: 1, Obligations: []string{"sub_hand"}}, "sub_hand"}, |
| + "opened before the protocol": {refusalState{Exists: true, Open: true, Protocol: 0}, "before the retirement protocol"}, |
| + "nothing attached": {refusalState{Exists: true, Open: true, Protocol: 1}, ""}, |
| + } { |
| + got := tc.st.held("retire") |
| + if (tc.want == "") != (got == "") || !strings.Contains(got, tc.want) { |
| + t.Errorf("%s: retire held %q; want %q", name, got, tc.want) |
| + } |
| + if keep := tc.st.held("keep"); keep != "no such intent stands" { |
| + t.Errorf("%s: keep held %q", name, keep) |
| + } |
| + } |
| + // Through the door: the intent removed by hand under its review (as |
| + // the delete barrier now stops the cleanup doing). |
| + fx, f, acct, intent := unreadyIntent(t, func(billingIntent) string { return currencyPriceID(plans.PlanPersonal, plans.BandD, plans.GBP) }) |
| + _ = f.webhookPost(t, fx, "evt_hand_gone", "customer.subscription.created", "sub_paid") |
| + for _, q := range []string{`DROP TRIGGER billing_refused_intent_delete_barrier`, `DELETE FROM billing_checkout_intents WHERE account_id=` + fmt.Sprint(acct.ID)} { |
| + if _, err := fx.st.db.Exec(q); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-gone") |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "", "gone"); status != http.StatusConflict || !strings.Contains(msg, "sub_paid") { |
| + t.Fatalf("retire of a review whose intent is gone: %d %q; want 409 held, naming the subscription", status, msg) |
| + } |
| +} |
| + |
| +// The delete barrier itself: an unready intent under an open refusal |
| +// review is not deleted by any statement, and one whose review is |
| +// resolved is. |
| +func TestTheDeleteBarrierCoversAnUnreadyIntentUnderAnOpenReview(t *testing.T) { |
| + fx, f, acct, intent := unreadyIntent(t, func(billingIntent) string { return currencyPriceID(plans.PlanPersonal, plans.BandD, plans.GBP) }) |
| + _ = f.webhookPost(t, fx, "evt_hand_barrier", "customer.subscription.created", "sub_paid") |
| + if _, err := fx.st.db.Exec(`DELETE FROM billing_checkout_intents WHERE account_id=? AND id=?`, acct.ID, intent.ID); err == nil || !strings.Contains(err.Error(), "billing_refused_intent_delete_barrier") { |
| + t.Fatalf("a bare delete under the open review: %v; want the barrier", err) |
| + } |
| + if _, err := fx.st.db.Exec(`UPDATE billing_reviews SET resolved_at=1 WHERE account_id=? AND subscription=?`, acct.ID, intent.ID); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.st.db.Exec(`DELETE FROM billing_checkout_intents WHERE account_id=? AND id=?`, acct.ID, intent.ID); err != nil { |
| + t.Fatalf("a delete once the review is resolved: %v", err) |
| + } |
| + // The barrier is made again at every start, so an earlier build's |
| + // text of it is replaced. |
| + if _, err := fx.st.db.Exec(`DROP TRIGGER billing_refused_intent_delete_barrier`); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.st.db.Exec(`CREATE TRIGGER billing_refused_intent_delete_barrier BEFORE DELETE ON billing_checkout_intents WHEN 0 BEGIN SELECT 1; END`); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := fx.st.migrateBillingRetirement(); err != nil { |
| + t.Fatal(err) |
| + } |
| + var text string |
| + if err := fx.st.db.QueryRow(`SELECT sql FROM sqlite_master WHERE type='trigger' AND name='billing_refused_intent_delete_barrier'`).Scan(&text); err != nil || !strings.Contains(text, "RAISE(ABORT") || !strings.Contains(text, "resolved_at=0") { |
| + t.Fatalf("the barrier after a start: %v\n%s", err, text) |
| + } |
| +} |
| + |
| +// A subscription on the purchase's own terms that names an intent which |
| +// never had a session is retried (the purchase's own could look like |
| +// that while its session is unknown), and is recorded from that first |
| +// read: the cleanup a day later does not delete the intent, it files the |
| +// refusal with the subscription attached. |
| +func TestTheCleanupOfAnUnreadyIntentASubscriptionNamesFilesItsRefusal(t *testing.T) { |
| + fx, f, acct, intent := unreadyIntent(t, func(i billingIntent) string { return i.Price }) |
| + if code := f.webhookPost(t, fx, "evt_on_terms_unready", "customer.subscription.created", "sub_paid"); code == http.StatusNoContent || code == http.StatusOK { |
| + t.Fatalf("the on-terms subscription: %d; want it retried", code) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || st.Exists || len(st.Obligations) != 1 || st.Obligations[0] != "sub_paid" { |
| + t.Fatalf("state %+v %v; want no review yet and the subscription recorded", st, err) |
| + } |
| + if err := fx.srv.billingPass(context.Background(), f.now.Add(26*time.Hour)); err != nil { |
| + t.Fatalf("the pass a day later: %v", err) |
| + } |
| + standing(t, fx, acct, intent) |
| + st, err = fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || st.Protocol != 1 || st.Reference != "" || len(st.Obligations) != 1 { |
| + t.Fatalf("state %+v %v; want the refusal filed by the cleanup, with the subscription attached", st, err) |
| + } |
| + if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil { |
| + t.Fatal(err) |
| + } |
| + heldRetire(t, fx, acct, intent.ID, "paul@keymail.dev", "sub_paid", f.now.Add(27*time.Hour)) |
| + // With nothing naming it, the same cleanup deletes the intent. |
| + fx2, f2, acct2, _ := unreadyIntent(t, func(i billingIntent) string { return i.Price }) |
| + if err := fx2.srv.billingPass(context.Background(), f2.now.Add(26*time.Hour)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx2.st.billingCheckoutIntent(acct2.ID); !errors.Is(err, sql.ErrNoRows) { |
| + t.Fatalf("an unready intent nothing names, a day later: %v; want it gone", err) |
| + } |
| +} |
| + |
| +// A subscription made by hand that names a bound intent while its session |
| +// is still open is retried, and recorded. When the session then expires |
| +// unpaid the intent is not retired: an expired session has no |
| +// subscription, so that one is not the checkout's own. The refusal is |
| +// filed with it attached, the pass proves the expiry, and a retire is |
| +// held. (Review twenty-one, Opus P3-2.) |
| +func TestAnExpiryDoesNotRetireAnIntentASubscriptionNames(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + intent, _ := fx.st.billingCheckoutIntent(acct.ID) |
| + f.mu.Lock() |
| + f.subscriptionPrice = intent.Price |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_hand_open", "customer.subscription.created", "sub_paid"); code == http.StatusNoContent || code == http.StatusOK { |
| + t.Fatalf("the subscription while the session is open: %d; want it retried", code) |
| + } |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Exists || len(st.Obligations) != 1 { |
| + t.Fatalf("state %+v; want no review and the subscription recorded", st) |
| + } |
| + f.mu.Lock() |
| + f.sessionStatus = "expired" |
| + f.mu.Unlock() |
| + if err := fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)); err != nil { |
| + t.Fatalf("the pass that meets the expiry: %v", err) |
| + } |
| + standing(t, fx, acct, intent) |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || st.Protocol != 1 || st.Reference != intent.SessionID || len(st.Obligations) != 1 || st.Fate != "" { |
| + t.Fatalf("state %+v %v; want the refusal filed, naming the session, its fate left for a read", st, err) |
| + } |
| + // The next pass reads the session and proves the expiry; retire is |
| + // held all the same, for the subscription. |
| + if err := fx.srv.billingPass(context.Background(), f.now.Add(10*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Fate != sessionFateExpired { |
| + t.Fatalf("state %+v; want the expiry proven by the pass", st) |
| + } |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-expiry-named") |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "", "expired"); status != http.StatusConflict || !strings.Contains(msg, "sub_paid") { |
| + t.Fatalf("retire: %d %q; want 409 held, naming the subscription", status, msg) |
| + } |
| + // The same click path: the buyer returns to the expired session. |
| + fx2, f2, acct2 := newCurrencyFixture(t) |
| + f2.customerCountry = "GB" |
| + _, _ = fx2.srv.startBillingCheckout(context.Background(), acct2, plans.PlanPersonal, "GB", "", "", f2.now) |
| + intent2, _ := fx2.st.billingCheckoutIntent(acct2.ID) |
| + f2.mu.Lock() |
| + f2.subscriptionPrice = intent2.Price |
| + f2.mu.Unlock() |
| + _ = f2.webhookPost(t, fx2, "evt_hand_open_2", "customer.subscription.created", "sub_paid") |
| + f2.mu.Lock() |
| + f2.sessionStatus = "expired" |
| + f2.mu.Unlock() |
| + if dest, err := fx2.srv.startBillingCheckout(context.Background(), acct2, plans.PlanPersonal, "GB", "", "", f2.now.Add(time.Minute)); dest != "" || !errors.Is(err, errBillingPending) { |
| + t.Fatalf("the click that meets the expiry: dest %q err %v; want nothing offered, under review", dest, err) |
| + } |
| + standing(t, fx2, acct2, intent2) |
| + if open, _ := fx2.st.refusalReviewOpen(acct2.ID, intent2.ID); !open { |
| + t.Fatal("the click's expiry filed no review") |
| + } |
| +} |
| + |
| +// A subscription made by hand that names an intent, seen before any |
| +// refusal exists, does not vanish when the honest purchase records and |
| +// consumes the intent: it is a dispute, made in the record's own |
| +// transaction, and the account is held. Seen for the first time after |
| +// the record, it is a dispute too. The honest purchase alone is not. |
| +// (Review twenty-one, Astra 2.) |
| +func TestASubscriptionSeenBeforeThePurchaseRecordsIsNotLostWithTheIntent(t *testing.T) { |
| + bought := func(t *testing.T) (*consoleFixture, *fakeBillingStripe, *Account, billingIntent) { |
| + t.Helper() |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + intent, _ := fx.st.billingCheckoutIntent(acct.ID) |
| + f.mu.Lock() |
| + f.subscriptionPrice = intent.Price |
| + f.otherSubscriptions = map[string]bool{"sub_hand": true} |
| + f.mu.Unlock() |
| + return fx, f, acct, intent |
| + } |
| + disputeReviews := func(fx *consoleFixture, acct *Account) int { |
| + var n int |
| + _ = fx.st.db.QueryRow(`SELECT count(*) FROM billing_reviews WHERE account_id=? AND reason=? AND resolved_at=0`, acct.ID, billingFateDisputedReason).Scan(&n) |
| + return n |
| + } |
| + t.Run("seen while the session named no subscription", func(t *testing.T) { |
| + fx, f, acct, intent := bought(t) |
| + // The session is complete and names no subscription yet: the |
| + // hand-made one is retried, and recorded. |
| + f.mu.Lock() |
| + f.sessionStatus, f.sessionNoSubscription = "complete", true |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_hand_early", "customer.subscription.created", "sub_hand"); code == http.StatusNoContent || code == http.StatusOK { |
| + t.Fatalf("the hand-made subscription: %d; want it retried", code) |
| + } |
| + // The session names its own, which records. |
| + f.mu.Lock() |
| + f.sessionNoSubscription = false |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_own", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the purchase's own subscription: %d", code) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "sub_paid" { |
| + t.Fatalf("recorded: %+v; want the purchase", state) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "subscription sub_hand named the intent before its purchase recorded with sub_paid"); n != 1 { |
| + t.Fatalf("%d disputes for the hand-made subscription; want one, made with the record", n) |
| + } |
| + if disputeReviews(fx, acct) != 1 { |
| + t.Fatal("the account is not held") |
| + } |
| + // Its retry is settled, and changes nothing. |
| + if code := f.webhookPost(t, fx, "evt_hand_retry", "customer.subscription.created", "sub_hand"); code != http.StatusNoContent { |
| + t.Fatalf("the hand-made subscription's retry: %d; want it settled", code) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "%"); n != 1 { |
| + t.Fatalf("%d disputes after the retry; want one", n) |
| + } |
| + }) |
| + t.Run("first seen after the record", func(t *testing.T) { |
| + fx, f, acct, intent := bought(t) |
| + f.mu.Lock() |
| + f.sessionStatus = "complete" |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_own", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the purchase: %d", code) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "%"); n != 0 || disputeReviews(fx, acct) != 0 { |
| + t.Fatalf("an honest purchase alone made %d disputes", n) |
| + } |
| + var session, subscription string |
| + if err := fx.st.db.QueryRow(`SELECT session_id,subscription FROM billing_consumed_intents WHERE account_id=? AND intent_id=?`, acct.ID, intent.ID).Scan(&session, &subscription); err != nil || session != intent.SessionID || subscription != "sub_paid" { |
| + t.Fatalf("what the record kept of the intent: %q %q %v", session, subscription, err) |
| + } |
| + // The purchase's own later events are no dispute. |
| + if code := f.webhookPost(t, fx, "evt_own_again", "customer.subscription.updated", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the purchase's later event: %d", code) |
| + } |
| + for _, id := range []string{"evt_hand_late", "evt_hand_late_again"} { |
| + if code := f.webhookPost(t, fx, id, "customer.subscription.created", "sub_hand"); code != http.StatusNoContent { |
| + t.Fatalf("the hand-made subscription after the record: %d; want it settled", code) |
| + } |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "subscription sub_hand names the intent whose purchase recorded with sub_paid"); n != 1 { |
| + t.Fatalf("%d disputes; want one", n) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "%"); n != 1 || disputeReviews(fx, acct) != 1 { |
| + t.Fatalf("%d disputes in all, %d reviews; want one of each", n, disputeReviews(fx, acct)) |
| + } |
| + }) |
| + t.Run("a refusal filed after another console recorded the purchase", func(t *testing.T) { |
| + fx, f, acct, intent := bought(t) |
| + f.mu.Lock() |
| + f.sessionStatus = "complete" |
| + f.mu.Unlock() |
| + _ = f.webhookPost(t, fx, "evt_own", "customer.subscription.created", "sub_paid") |
| + // This console read the intent before that, and files now. |
| + if err := fx.st.fileCheckoutRefusal(intent, intent.SessionID, "sub_hand", f.now.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "subscription sub_hand names the intent whose purchase recorded%"); n != 1 { |
| + t.Fatalf("%d disputes; want the late filing's subscription kept as one", n) |
| + } |
| + }) |
| +} |
| + |
| +// Only a first purchase consumes an intent. A later event of the |
| +// account's recorded subscription, its metadata edited to name the |
| +// standing intent of a newer purchase, deletes nothing: that intent's |
| +// session may still be payable. |
| +func TestALaterEventOfARecordedSubscriptionConsumesNoIntent(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + first, _ := fx.st.billingCheckoutIntent(acct.ID) |
| + f.mu.Lock() |
| + f.sessionStatus, f.subscriptionPrice = "complete", first.Price |
| + f.mu.Unlock() |
| + if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + f.mu.Lock() |
| + f.subscriptionStatus = "canceled" |
| + f.mu.Unlock() |
| + if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now.Add(2*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + // The subscription's metadata, as the first purchase made it. |
| + f.mu.Lock() |
| + original := map[string]string{} |
| + for k, v := range f.meta { |
| + original[k] = v |
| + } |
| + f.sessionStatus, f.sessionID = "open", "cs_test_next" |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(3*time.Minute)); err != nil || dest == "" { |
| + t.Fatalf("the next purchase's click: dest %q err %v", dest, err) |
| + } |
| + next, _ := fx.st.billingCheckoutIntent(acct.ID) |
| + if next.ID == first.ID { |
| + t.Fatalf("fixture: next %+v", next) |
| + } |
| + // The old subscription's next event names the new intent (the fake's |
| + // metadata is now the new checkout's), with a period that has moved. |
| + f.mu.Lock() |
| + f.now = f.now.Add(time.Hour) |
| + f.mu.Unlock() |
| + _ = fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "evt_old_names_new", f.now) |
| + standing(t, fx, acct, next) |
| + // And with its own metadata it changes nothing about the new intent |
| + // either. |
| + f.mu.Lock() |
| + f.meta = original |
| + f.now = f.now.Add(time.Hour) |
| + f.mu.Unlock() |
| + _ = fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "evt_old_names_old", f.now) |
| + standing(t, fx, acct, next) |
| + var archived int |
| + _ = fx.st.db.QueryRow(`SELECT count(*) FROM billing_consumed_intents WHERE account_id=? AND intent_id=?`, acct.ID, next.ID).Scan(&archived) |
| + if archived != 0 { |
| + t.Fatal("the new intent was recorded as consumed") |
| + } |
| +} |
| + |
| +// A session id is written onto an intent in one place, which refuses a |
| +// session that is already another purchase's and one that is not the |
| +// session the intent's open review names. (Review twenty-one, Opus P3-1.) |
| +func TestASessionIsBoundOnlyWhereItBelongs(t *testing.T) { |
| + // A retired purchase's session is not bound onto the replacement. |
| + fx, f, acct, intent, _ := retiredOnAProvenExpiry(t, "code-billing-op-bind") |
| + f.mu.Lock() |
| + f.sessionID, f.sessionStatus = "cs_test_second", "open" |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(30*time.Minute)); err != nil || dest == "" { |
| + t.Fatalf("the replacement click: dest %q err %v", dest, err) |
| + } |
| + replacement, _ := fx.st.billingCheckoutIntent(acct.ID) |
| + if _, err := fx.st.db.Exec(`UPDATE billing_checkout_intents SET session_id='',url='' WHERE account_id=?`, acct.ID); err != nil { |
| + t.Fatal(err) |
| + } |
| + replacement.SessionID = "" |
| + expires := f.now.Add(2 * time.Hour).Unix() |
| + if err := fx.st.bindIntentSession(replacement, intent.SessionID, nil, expires); !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("binding the retired purchase's session onto the replacement: %v; want an identity refusal", err) |
| + } |
| + if err := fx.st.saveBillingCheckout(replacement, intent.SessionID, "https://checkout.stripe.com/c/x", expires); !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("saving the retired purchase's session onto the replacement: %v; want an identity refusal", err) |
| + } |
| + if got, _ := fx.st.billingCheckoutIntent(acct.ID); got.SessionID != "" { |
| + t.Fatalf("the replacement is bound to %q", got.SessionID) |
| + } |
| + if err := fx.st.bindIntentSession(replacement, "cs_test_second", nil, expires); err != nil { |
| + t.Fatalf("binding its own session: %v", err) |
| + } |
| + // The same binding again is the same binding. |
| + replacement.SessionID = "cs_test_second" |
| + if err := fx.st.bindIntentSession(replacement, "cs_test_second", nil, expires); err != nil { |
| + t.Fatalf("binding its own session again: %v", err) |
| + } |
| + |
| + // Nor is the session of a purchase that recorded and ended bound onto |
| + // the next purchase's intent. |
| + fx3, f3, acct3 := newCurrencyFixture(t) |
| + f3.customerCountry = "GB" |
| + if dest, err := fx3.srv.startBillingCheckout(context.Background(), acct3, plans.PlanPersonal, "GB", "", "", f3.now); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + first, _ := fx3.st.billingCheckoutIntent(acct3.ID) |
| + f3.mu.Lock() |
| + f3.sessionStatus, f3.subscriptionPrice = "complete", first.Price |
| + f3.mu.Unlock() |
| + if err := fx3.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f3.now.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + f3.mu.Lock() |
| + f3.subscriptionStatus = "canceled" |
| + f3.mu.Unlock() |
| + if err := fx3.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f3.now.Add(2*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + f3.mu.Lock() |
| + f3.sessionStatus, f3.sessionID = "open", "cs_test_next" |
| + f3.mu.Unlock() |
| + if dest, err := fx3.srv.startBillingCheckout(context.Background(), acct3, plans.PlanPersonal, "GB", "", "", f3.now.Add(3*time.Minute)); err != nil || dest == "" { |
| + t.Fatalf("the next purchase's click: dest %q err %v", dest, err) |
| + } |
| + next, _ := fx3.st.billingCheckoutIntent(acct3.ID) |
| + if next.ID == first.ID || next.SessionID != "cs_test_next" { |
| + t.Fatalf("fixture: next %+v", next) |
| + } |
| + if _, err := fx3.st.db.Exec(`UPDATE billing_checkout_intents SET session_id='',url='' WHERE account_id=?`, acct3.ID); err != nil { |
| + t.Fatal(err) |
| + } |
| + next.SessionID = "" |
| + if err := fx3.st.bindIntentSession(next, first.SessionID, nil, f3.now.Add(2*time.Hour).Unix()); !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("binding a recorded purchase's session onto the next intent: %v; want an identity refusal", err) |
| + } |
| + |
| + // An unbound intent whose open review names one session is not bound |
| + // to another. |
| + fx2, f2, acct2 := newCurrencyFixture(t) |
| + f2.customerCountry = "GB" |
| + f2.mu.Lock() |
| + f2.failCreate = true |
| + f2.mu.Unlock() |
| + _, _ = fx2.srv.startBillingCheckout(context.Background(), acct2, plans.PlanPersonal, "GB", "", "", f2.now) |
| + unbound, err := fx2.st.billingCheckoutIntent(acct2.ID) |
| + if err != nil || unbound.SessionID != "" { |
| + t.Fatalf("fixture: %+v %v", unbound, err) |
| + } |
| + if err := fx2.st.fileCheckoutRefusal(unbound, "cs_test_one", "", f2.now); err != nil { |
| + t.Fatal(err) |
| + } |
| + expires2 := f2.now.Add(2 * time.Hour).Unix() |
| + if err := fx2.st.bindIntentSession(unbound, "cs_test_two", nil, expires2); !errors.Is(err, errBillingIdentity) { |
| + t.Fatalf("binding another session than the review names: %v; want an identity refusal", err) |
| + } |
| + // Through the binder: a completed session on the intent's own terms |
| + // that is not the one under review binds nothing and records nothing. |
| + f2.mu.Lock() |
| + f2.sessionID, f2.sessionStatus, f2.subscriptionPrice = "cs_test_two", "complete", unbound.Price |
| + f2.mu.Unlock() |
| + if code := f2.webhookPost(t, fx2, "evt_other_session", "checkout.session.completed", "cs_test_two"); code != http.StatusNoContent { |
| + t.Fatalf("the other session's completion: %d; want it settled as refused", code) |
| + } |
| + if got, _ := fx2.st.billingCheckoutIntent(acct2.ID); got.SessionID != "" { |
| + t.Fatalf("the intent under review was bound to %q", got.SessionID) |
| + } |
| + if state, _ := fx2.st.billingAccount(acct2.ID); state.Subscription != "" { |
| + t.Fatalf("recorded: %+v", state) |
| + } |
| + if err := fx2.st.bindIntentSession(unbound, "cs_test_one", nil, expires2); err != nil { |
| + t.Fatalf("binding the session the review names: %v", err) |
| + } |
| +} |
| + |
| +// The customer and the currency an account is held to are read again in |
| +// the transaction that creates an intent: a request built from a reading |
| +// older than a purchase another console recorded and ended is refused. |
| +// (Review twenty-one, Astra 3.) |
| +func TestAnIntentIsNotCreatedFromAStaleReadingOfTheAccount(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + intent, _ := fx.st.billingCheckoutIntent(acct.ID) |
| + // Another console records the purchase and its cancellation. |
| + f.mu.Lock() |
| + f.sessionStatus, f.subscriptionPrice = "complete", intent.Price |
| + f.mu.Unlock() |
| + if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + f.mu.Lock() |
| + f.subscriptionStatus = "canceled" |
| + f.mu.Unlock() |
| + if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now.Add(2*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + state, _ := fx.st.billingAccount(acct.ID) |
| + if state.Status != "canceled" || state.Customer == "" || state.Currency != "gbp" { |
| + t.Fatalf("fixture: %+v", state) |
| + } |
| + request := func(customer string, c plans.Currency) billingIntent { |
| + terms := bookTerms(c) |
| + return billingIntent{AccountID: acct.ID, Plan: intent.Plan, Terms: intent.Terms, PriceBook: terms.PriceBook, Currency: string(terms.Currency), Band: intent.Band, Price: currencyPriceID(intent.Plan, intent.Band, c), |
| + Quantity: intent.Quantity, Customer: customer, Country: intent.Country, LocDigest: intent.LocDigest, LiveMode: intent.LiveMode, Members: intent.Members} |
| + } |
| + // This console read the account before any of that: no customer. |
| + if _, err := fx.st.beginBillingCheckout(request("", plans.GBP), f.now.Add(3*time.Minute)); !errors.Is(err, errBillingConflict) { |
| + t.Fatalf("a request that names no customer, for an account that has one now: %v; want a conflict", err) |
| + } |
| + // Or read its customer and not the currency it is now held to. |
| + if _, err := fx.st.beginBillingCheckout(request(state.Customer, plans.USD), f.now.Add(3*time.Minute)); !errors.Is(err, errBillingConflict) { |
| + t.Fatalf("a request in dollars, for an account held to pounds: %v; want a conflict", err) |
| + } |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); !errors.Is(err, sql.ErrNoRows) { |
| + t.Fatalf("an intent was created from a stale reading: %v", err) |
| + } |
| + if _, err := fx.st.beginBillingCheckout(request(state.Customer, plans.GBP), f.now.Add(3*time.Minute)); err != nil { |
| + t.Fatalf("a request from the account as it is: %v", err) |
| + } |
| +} |
| + |
| +// An answer that carries another session's id is not a read of the |
| +// session that was asked for, and nothing is recorded from it about |
| +// either. (Review twenty-one, Opus P3-3.) |
| +func TestAnAnswerAboutAnotherSessionIsNotRecorded(t *testing.T) { |
| + fx, f, acct, intent, _ := retiredOnAProvenExpiry(t, "code-billing-op-other-id") |
| + // Asked for some other session, the provider answers with the retired |
| + // one, complete. |
| + f.mu.Lock() |
| + f.sessionStatus = "complete" |
| + f.mu.Unlock() |
| + session, err := fx.srv.readCheckout(context.Background(), "cs_test_asked_for") |
| + if err != nil || session.ID != intent.SessionID { |
| + t.Fatalf("the read: %+v %v", session, err) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "%"); n != 0 { |
| + t.Fatalf("%d disputes recorded from an answer to a read of another session", n) |
| + } |
| + // Asked for by its own id, the same answer is the contradiction it is. |
| + if _, err := fx.srv.readCheckout(context.Background(), intent.SessionID); err != nil { |
| + t.Fatal(err) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "%"); n != 1 { |
| + t.Fatalf("%d disputes from the read of the session itself; want one", n) |
| + } |
| +} |
| + |
| +// A listing whose later page fails still records the pages it read. |
| +// (Review twenty-one, Astra 4.) |
| +func TestAListingThatFailsLaterStillRecordsWhatItRead(t *testing.T) { |
| + fx, f, acct, intent, _ := retiredOnAProvenExpiry(t, "code-billing-op-listing") |
| + f.mu.Lock() |
| + f.sessionStatus, f.listSecondPageFails = "complete", true |
| + f.mu.Unlock() |
| + sessions, err := fx.srv.listCheckouts(context.Background(), "cus_paid", f.now.Add(-time.Hour).Unix(), f.now.Add(time.Hour).Unix()) |
| + if err == nil || sessions != nil { |
| + t.Fatalf("the listing: %v %v; want its failure, and no sessions handed on", sessions, err) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "read back complete after its intent was retired on a proven expiry"); n != 1 { |
| + t.Fatalf("%d disputes from the page that was read; want one", n) |
| + } |
| +} |
| + |
| +// The refusals' turn takes the longest-due first, so one that stays due |
| +// is reached however many others there are. (Review twenty-one, Astra 5.) |
| +func TestTheRefusalsTurnTakesTheLongestDueFirst(t *testing.T) { |
| + fx, f, _ := newCurrencyFixture(t) |
| + now := f.now |
| + var refused []billingIntent |
| + // Five refused intents on five accounts, each bound to a session of |
| + // its own that the fake answers with another id (a failed read, which |
| + // counts a try). The fifth has been due far longer than the rest and |
| + // stands last. |
| + for i := 1; i <= 5; i++ { |
| + id := int64(7000 + i) |
| + if err := fx.st.CreateAccount(id, fmt.Sprintf("fair%d", i), "Fair", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + due := now.Add(-time.Duration(i*10) * time.Second).Unix() |
| + if i == 5 { |
| + due = now.Add(-2 * time.Hour).Unix() |
| + } |
| + intent := billingIntent{AccountID: id, ID: fmt.Sprintf("ci_fair_%d", i), SessionID: fmt.Sprintf("cs_fair_%d", i)} |
| + for _, q := range []struct { |
| + sql string |
| + args []any |
| + }{ |
| + {`INSERT INTO billing_checkout_intents(account_id,id,plan,terms,band,price,quantity,created_at,session_id,customer_ready,expiry_next_try_at) VALUES(?,?,'personal',?,'A','price_x',1,?,?,1,?)`, |
| + []any{id, intent.ID, plans.CurrentTermsVersion, now.Unix(), intent.SessionID, due}}, |
| + {`INSERT INTO billing_reviews(account_id,subscription,reason,created_at,last_seen_at,occurrences,protocol,reference) VALUES(?,?,?,?,?,1,1,?)`, |
| + []any{id, intent.ID, billingCheckoutRefusedReason, now.Unix(), now.Unix(), intent.SessionID}}, |
| + } { |
| + if _, err := fx.st.db.Exec(q.sql, q.args...); err != nil { |
| + t.Fatalf("%s: %v", q.sql, err) |
| + } |
| + } |
| + refused = append(refused, intent) |
| + } |
| + _ = fx.srv.billingPassRefusals(context.Background(), refused, now) |
| + tries := map[string]int64{} |
| + for _, intent := range refused { |
| + st, err := fx.st.refusalState(intent.AccountID, intent.ID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + tries[intent.ID] = st.Tries |
| + } |
| + want := map[string]int64{"ci_fair_1": 0, "ci_fair_2": 0, "ci_fair_3": 1, "ci_fair_4": 1, "ci_fair_5": 1} |
| + for id, n := range want { |
| + if tries[id] != n { |
| + t.Fatalf("tries %v; want the three longest due worked (%v)", tries, want) |
| + } |
| + } |
| +} |
diff --git a/internal/console/billingoperator.go b/internal/console/billingoperator.go| index bc61ad5a2..91cf42b07 100644 |
| --- a/internal/console/billingoperator.go |
| +++ b/internal/console/billingoperator.go |
| @@ -417,8 +417,10 @@ func (s *Server) handleAPIBillingReviewResolve(w http.ResponseWriter, r *http.Re |
| if req.Reason == billingCheckoutRefusedReason { |
| // What the resolution did about the intent the review was keyed |
| // by: retired it (the default), or kept it standing for the |
| - // corrected purchase to record. Nothing records under an open |
| - // review, so an intent is never gone at its resolution. |
| + // corrected purchase to record. Nothing records, expires or |
| + // cleans up an intent under an open review, so an intent is |
| + // not gone at its resolution; if one ever is, nothing was |
| + // retired, and the answer says so (retired false). |
| retired, rerr := s.store.billingIntentRetired(acct.ID, req.Subscription) |
| if rerr != nil { |
| jsonError(w, http.StatusInternalServerError, "the retirement could not be read back") |
diff --git a/internal/console/billingpass.go b/internal/console/billingpass.go| index e135f3b31..521c3b224 100644 |
| --- a/internal/console/billingpass.go |
| +++ b/internal/console/billingpass.go |
| @@ -144,7 +144,9 @@ func (s *Server) billingPass(ctx context.Context, now time.Time) error { |
| // whose session is not known yet, an unbound intent under a review |
| // that names none, is still reconciled: that is how its session is |
| // found. Nothing records or retires under the review on that path |
| - // either.) These reads are the store's alone. |
| + // either: the record, the expiry and the cleanup of an intent that |
| + // never had a session each refuse an intent under an open review, in |
| + // the store.) These reads are the store's alone. |
| var pending, refused []billingIntent |
| for _, intent := range intents { |
| st, err := s.store.refusalState(intent.AccountID, intent.ID) |
| @@ -313,7 +315,11 @@ func (s *Server) reconcilePendingIntent(ctx context.Context, intent billingInten |
| // expireResolvedIntent deletes an intent whose session Stripe reports expired. |
| func (s *Server) expireResolvedIntent(intent billingIntent, session *billingstripe.Session, now time.Time) error { |
| - if err := s.store.expireBillingCheckout(intent, session, now); err != nil && !errors.Is(err, errBillingConflict) { |
| + // A conflict is an intent that is no longer this one; under review is |
| + // an intent the expiry did not retire because a subscription was seen |
| + // naming it (the store filed its refusal and it stands). Neither is a |
| + // failure of the pass. |
| + if err := s.store.expireBillingCheckout(intent, session, now); err != nil && !errors.Is(err, errBillingConflict) && !errors.Is(err, errBillingUnderReview) { |
| return err |
| } |
| return nil |
diff --git a/internal/console/billingretirement.go b/internal/console/billingretirement.go| index bb00a089d..92d447854 100644 |
| --- a/internal/console/billingretirement.go |
| +++ b/internal/console/billingretirement.go |
| @@ -42,27 +42,56 @@ import ( |
| // (readCheckout, listCheckouts, readSubscription), and each hands what it |
| // read to the store before the caller sees it. The store finds the |
| // purchase by what the console itself recorded, never by what the answer |
| -// claims: a session by its id, against the standing intent bound or |
| -// referenced to it, the retirement record, or the kept resolution's |
| -// evidence; a subscription by the account and intent it names. It |
| -// classifies in one transaction against the purchase as it is at that |
| -// instant, standing, retired, or kept and consumed, and records the |
| -// session's fate, an attached subscription, or a dispute. No caller |
| -// decides whether to record, so no return of a caller's can come first; |
| -// a test parses this package and fails on any direct read of the client. |
| -// A refusal and the subscription that caused it are one transaction, and |
| -// filing for an intent that is already gone classifies the subscription |
| -// against the archive in that same transaction. Nothing read about a |
| -// refused purchase is dropped. |
| +// claims: a session by the id that was asked for (an answer carrying |
| +// another id is a failed read and records nothing), against the standing |
| +// intent bound or referenced to it, the retirement record, or the kept |
| +// resolution's evidence; a subscription by the account and intent it |
| +// names. It classifies in one transaction against the purchase as it is |
| +// at that instant, standing, retired, kept, or consumed by a purchase, |
| +// and records the session's fate, a subscription seen naming the intent, |
| +// or a dispute. No caller decides whether to record, so no return of a |
| +// caller's can come first; a test parses this package and fails on any |
| +// direct read of the client. A refusal and the subscription that caused |
| +// it are one transaction, and filing for an intent that is already gone |
| +// classifies the subscription against the archive in that same |
| +// transaction. |
| +// |
| +// Named subscriptions. A subscription that names a standing intent is |
| +// recorded from the first read, whether or not a refusal review exists |
| +// (billing_review_obligations). Nothing deletes an intent past one: |
| +// |
| +// - the record of the purchase keeps what it consumed (the intent, its |
| +// session, the subscription it recorded with) and, in the same |
| +// transaction, makes every other subscription seen naming the intent |
| +// a dispute; |
| +// - the expiry of the intent's session, and the cleanup of an intent |
| +// that never had a session, file the refusal instead, with what was |
| +// seen attached, and the intent stands under it; |
| +// - the operator's retire is held while anything is attached, whether |
| +// or not the intent still stands. |
| +// |
| +// A session id is written onto an intent in one place (bindIntentSession), |
| +// which refuses a session that is another purchase's and one that is not |
| +// the session the intent's open review names. |
| // |
| -// What the invariant is not about: the answers to a create and to an |
| -// expiry (replays under an idempotency key, proving nothing about the |
| -// present: what follows one is decided on a read, or on the store's own |
| -// checks of the answer); a read made before any refusal review exists |
| -// (the read that leads to a filing is handed in again once the review |
| -// exists: fileRefusal); and a read without identity, or of a session |
| -// still open, before any fate is recorded (there is nothing recorded for |
| -// it to contradict, and with no fate both outcomes are held). |
| +// What the invariant is not about, and what is lost by each: |
| +// |
| +// - The answers to a create and to an expiry: replays under an |
| +// idempotency key, proving nothing about the present. What follows |
| +// one is decided on a read, or on the store's own checks of the |
| +// answer. One exception, which only ever holds: a create answer that |
| +// shows its session completed or paid is recorded as a completion |
| +// when the refusal it causes is filed (refuseSession). |
| +// - A session read before any refusal review exists: there is no |
| +// refused purchase for it to be evidence about. The read that leads |
| +// to a filing is handed in again once the review exists (fileRefusal). |
| +// - A read without identity, or of a session still open, before any |
| +// fate is recorded: nothing is recorded for it to contradict, and |
| +// with no fate both outcomes are held. |
| +// - A subscription that names an intent which expired in the ordinary |
| +// way with nothing seen naming it: nothing is kept of such an intent, |
| +// and the subscription is a rejected event with its account, in the |
| +// operator's list, and no hold. |
| const ( |
| sessionFateExpired = "expired" |
| @@ -132,9 +161,15 @@ func (s *Store) migrateBillingRetirement() error { |
| return err |
| } |
| } |
| + tx, err := s.db.Begin() |
| + if err != nil { |
| + return err |
| + } |
| + defer tx.Rollback() |
| for _, stmt := range []string{ |
| - // The subscriptions that caused or were attached to a refusal, |
| - // beside the session's own. |
| + // The subscriptions seen naming an intent while it stood: from |
| + // the first read, whether or not a refusal review exists yet. |
| + // Under a review they are what is attached to the case. |
| `CREATE TABLE IF NOT EXISTS billing_review_obligations ( |
| account_id INTEGER NOT NULL, intent_id TEXT NOT NULL, subscription TEXT NOT NULL, first_seen_at INTEGER NOT NULL, |
| PRIMARY KEY(account_id, intent_id, subscription) |
| @@ -147,16 +182,30 @@ func (s *Store) migrateBillingRetirement() error { |
| first_seen_at INTEGER NOT NULL, last_seen_at INTEGER NOT NULL, |
| PRIMARY KEY(account_id, session_id, detail) |
| )`, |
| + // What an ordinary purchase consumed: the intent, its session and |
| + // the subscription it recorded with, so a subscription that names |
| + // the intent afterwards is seen for what it is. |
| + `CREATE TABLE IF NOT EXISTS billing_consumed_intents ( |
| + account_id INTEGER NOT NULL, intent_id TEXT NOT NULL, session_id TEXT NOT NULL, subscription TEXT NOT NULL, |
| + customer TEXT NOT NULL, consumed_at INTEGER NOT NULL, |
| + PRIMARY KEY(account_id, intent_id) |
| + )`, |
| // The barrier: an intent that has a refusal review is deleted only |
| - // when no session was ever created for it, when its session is |
| - // proven expired and undisputed, or by the record of a purchase |
| - // the operator kept. It stops an older binary's retire from |
| - // deleting an intent whose session is not proven expired; it does |
| - // not make an older binary follow the protocol (the roll rule |
| - // does: older consoles are stopped before this build starts). |
| - `CREATE TRIGGER IF NOT EXISTS billing_refused_intent_delete_barrier BEFORE DELETE ON billing_checkout_intents |
| + // when no session was ever created for it and its review is not |
| + // open, when its session is proven expired and undisputed, or by |
| + // the record of a purchase the operator kept. It stops an older |
| + // binary's retire from deleting an intent whose session is not |
| + // proven expired; it does not make an older binary follow the |
| + // protocol (the roll rule does: older consoles are stopped before |
| + // this build starts). It is dropped and made again at every |
| + // start, in the one transaction these statements share, so a |
| + // database that carries an earlier build's text of it gets this |
| + // one and is never without one. |
| + `DROP TRIGGER IF EXISTS billing_refused_intent_delete_barrier`, |
| + `CREATE TRIGGER billing_refused_intent_delete_barrier BEFORE DELETE ON billing_checkout_intents |
| WHEN EXISTS (SELECT 1 FROM billing_reviews r WHERE r.account_id=OLD.account_id AND r.subscription=OLD.id AND r.reason='` + billingCheckoutRefusedReason + `') |
| - AND NOT (OLD.session_id='' AND OLD.customer_ready=0) |
| + AND NOT (OLD.session_id='' AND OLD.customer_ready=0 AND NOT EXISTS ( |
| + SELECT 1 FROM billing_reviews r WHERE r.account_id=OLD.account_id AND r.subscription=OLD.id AND r.reason='` + billingCheckoutRefusedReason + `' AND r.resolved_at=0)) |
| AND NOT (OLD.session_fate='` + sessionFateExpired + `' AND OLD.fate_disputed_at=0) |
| AND NOT (OLD.session_fate='` + sessionFateComplete + `' AND OLD.fate_disputed_at=0 AND EXISTS ( |
| SELECT 1 FROM billing_reviews r JOIN billing_review_resolutions x |
| @@ -165,11 +214,11 @@ func (s *Store) migrateBillingRetirement() error { |
| AND r.resolved_at<>0 AND x.outcome='keep')) |
| BEGIN SELECT RAISE(ABORT, 'billing_refused_intent_delete_barrier'); END`, |
| } { |
| - if _, err := s.db.Exec(stmt); err != nil { |
| + if _, err := tx.Exec(stmt); err != nil { |
| return err |
| } |
| } |
| - return nil |
| + return tx.Commit() |
| } |
| // refusalState is what the store holds about a refused intent. |
| @@ -200,23 +249,25 @@ func (st refusalState) pinned() string { |
| // held says why an outcome is not available for the refused intent in |
| // stage one, or "" when it is. |
| func (st refusalState) held(outcome string) string { |
| - if !st.Stands { |
| - if outcome == "keep" { |
| - return "no such intent stands" |
| - } |
| - return "" |
| + if !st.Stands && outcome == "keep" { |
| + return "no such intent stands" |
| } |
| + // These hold whether or not the intent still stands: an intent that |
| + // is gone leaves its review, and what was attached to it, behind. |
| if st.Protocol == 0 { |
| return "the review was opened before the retirement protocol, so what was attached to the purchase was never recorded" |
| } |
| if st.Disputed != 0 { |
| return "the provider's evidence about the session is contradictory (see the " + billingFateDisputedReason + " review)" |
| } |
| + if outcome == "retire" && len(st.Obligations) > 0 { |
| + return "a subscription is attached to the purchase (" + strings.Join(st.Obligations, ", ") + "), and canceling a subscription is not settling it" |
| + } |
| + if !st.Stands { |
| + return "" |
| + } |
| switch outcome { |
| case "retire": |
| - if len(st.Obligations) > 0 { |
| - return "a subscription is attached to the purchase (" + strings.Join(st.Obligations, ", ") + "), and canceling a subscription is not settling it" |
| - } |
| if st.SessionID == "" && !st.CustomerReady && st.Reference == "" { |
| return "" |
| } |
| @@ -523,10 +574,11 @@ func (s *Store) sessionArchived(sessionID string) (accountID int64, archived boo |
| // kept, standing for its record), it files the review again, so the |
| // record waits on a person who has seen it. The intent was retired on a |
| // proven expiry: nothing was attached to it then, so the subscription is |
| -// a dispute. Its purchase was kept and recorded: any subscription but the |
| -// kept one is a dispute. Anything else (no refusal review; a retirement |
| -// from before the protocol; a subscription attached while the intent |
| -// stood) records nothing. |
| +// a dispute. Its purchase was kept and recorded, or recorded in the |
| +// ordinary way: any subscription but the one it recorded with is a |
| +// dispute. Anything else (a retirement from before the protocol; a |
| +// subscription already recorded while the intent stood; an intent that |
| +// expired in the ordinary way, of which nothing is kept) records nothing. |
| func classifySubscriptionTx(tx *sql.Tx, accountID int64, intentID, subscription string, now time.Time) error { |
| if intentID == "" || subscription == "" { |
| return nil |
| @@ -536,14 +588,14 @@ func classifySubscriptionTx(tx *sql.Tx, accountID int64, intentID, subscription |
| return err |
| } |
| if st.Stands { |
| - if !st.Exists || subscription == st.SessionSubscription { |
| + if subscription == st.SessionSubscription { |
| return nil |
| } |
| attached, err := attachSubscriptionTx(tx, accountID, intentID, subscription, now) |
| if err != nil { |
| return err |
| } |
| - if attached && !st.Open { |
| + if attached && st.Exists && !st.Open { |
| if err := fileBillingReviewTx(tx, accountID, intentID, billingCheckoutRefusedReason, now); err != nil { |
| return err |
| } |
| @@ -573,17 +625,78 @@ func classifySubscriptionTx(tx *sql.Tx, accountID int64, intentID, subscription |
| var keptSession, keptSubscription string |
| err = tx.QueryRow(`SELECT evidence_session,evidence_subscription FROM billing_review_resolutions WHERE account_id=? AND subscription=? AND reason=? AND outcome='keep' ORDER BY occurrence DESC LIMIT 1`, |
| accountID, intentID, billingCheckoutRefusedReason).Scan(&keptSession, &keptSubscription) |
| - if errors.Is(err, sql.ErrNoRows) || (err == nil && (keptSession == "" || subscription == keptSubscription)) { |
| + if err == nil && keptSession != "" { |
| + if subscription == keptSubscription { |
| + return nil |
| + } |
| + return disputeSessionTx(tx, accountID, keptSession, intentID, "subscription "+subscription+" names the intent whose purchase was kept and recorded with "+keptSubscription, now) |
| + } |
| + if err != nil && !errors.Is(err, sql.ErrNoRows) { |
| + return err |
| + } |
| + // An ordinary purchase consumed the intent: any subscription that |
| + // names it but the one it recorded with is a dispute. |
| + var usedSession, usedSubscription string |
| + err = tx.QueryRow(`SELECT session_id,subscription FROM billing_consumed_intents WHERE account_id=? AND intent_id=?`, accountID, intentID).Scan(&usedSession, &usedSubscription) |
| + if errors.Is(err, sql.ErrNoRows) || (err == nil && (usedSession == "" || subscription == usedSubscription)) { |
| return nil |
| } |
| if err != nil { |
| return err |
| } |
| - return disputeSessionTx(tx, accountID, keptSession, intentID, "subscription "+subscription+" names the intent whose purchase was kept and recorded with "+keptSubscription, now) |
| + return disputeSessionTx(tx, accountID, usedSession, intentID, "subscription "+subscription+" names the intent whose purchase recorded with "+usedSubscription, now) |
| } |
| return err |
| } |
| +// holdForNamedSubscriptionsTx stops the deletion of an intent that a |
| +// subscription was seen naming: where any is recorded, it files the |
| +// intent's refusal review and reports true, and the caller commits that |
| +// and deletes nothing. (Under a review that is already open it files |
| +// nothing and reports false: the deletion's own open-review clause |
| +// refuses.) sessionID is the session the |
| +// deletion was about, for the review's reference. A subscription that |
| +// names an intent whose session expired, or that never had a session, is |
| +// not the checkout's own; what it has collected or will is a person's to |
| +// look at, and the intent stands under its review until one has. |
| +func holdForNamedSubscriptionsTx(tx *sql.Tx, accountID int64, intentID, sessionID string, now time.Time) (bool, error) { |
| + st, err := refusalStateQ(tx, accountID, intentID) |
| + // Under an open review there is nothing to file: the deletion's own |
| + // clause refuses it, as it does whatever is attached. |
| + if err != nil || !st.Stands || st.Open || len(st.Obligations) == 0 { |
| + return false, err |
| + } |
| + if err := fileBillingReviewTx(tx, accountID, intentID, billingCheckoutRefusedReason, now); err != nil { |
| + return false, err |
| + } |
| + if _, err := tx.Exec(`UPDATE billing_reviews SET protocol=1 WHERE account_id=? AND subscription=? AND reason=?`, accountID, intentID, billingCheckoutRefusedReason); err != nil { |
| + return false, err |
| + } |
| + if sessionID != "" { |
| + if _, err := tx.Exec(`UPDATE billing_reviews SET reference=? WHERE account_id=? AND subscription=? AND reason=? AND reference=''`, sessionID, accountID, intentID, billingCheckoutRefusedReason); err != nil { |
| + return false, err |
| + } |
| + } |
| + return true, nil |
| +} |
| + |
| +// sessionBelongsElsewhereTx reports whether a session id is already the |
| +// console's record of another purchase: another standing intent's, a |
| +// retired intent's, a kept one's, or one an ordinary purchase consumed. |
| +// Such a session is never bound to an intent. |
| +func sessionBelongsElsewhereTx(tx *sql.Tx, sessionID string, accountID int64, intentID string) (bool, error) { |
| + p, found, err := purchaseOfSessionTx(tx, sessionID) |
| + if err != nil { |
| + return false, err |
| + } |
| + if found && (p.Kind != "standing" || p.AccountID != accountID || p.IntentID != intentID) { |
| + return true, nil |
| + } |
| + var used bool |
| + err = tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_consumed_intents WHERE session_id=?)`, sessionID).Scan(&used) |
| + return used, err |
| +} |
| + |
| // noteSubscriptionSeen classifies and records one subscription read from |
| // the provider, by the account and intent it names, in one transaction. |
| // The account's own recorded subscription is not evidence about anything. |
| @@ -673,8 +786,14 @@ func (s *Server) readCheckout(ctx context.Context, id string) (*billingstripe.Se |
| if err != nil { |
| return nil, err |
| } |
| - if err := s.store.noteSessionSeen(s.billing.cfg.LiveMode, session, time.Now()); err != nil { |
| - return nil, err |
| + // An answer that carries another session's id is not a read of the |
| + // session that was asked for, and is not evidence about the one it |
| + // names either: every caller treats it as a failed read, and nothing |
| + // is recorded from it. |
| + if session.ID == id { |
| + if err := s.store.noteSessionSeen(s.billing.cfg.LiveMode, session, time.Now()); err != nil { |
| + return nil, err |
| + } |
| } |
| return session, nil |
| } |
| @@ -682,15 +801,17 @@ func (s *Server) readCheckout(ctx context.Context, id string) (*billingstripe.Se |
| // listCheckouts is readCheckout for the customer-scoped listing recovery |
| // uses: every session listed is handed to the store. |
| func (s *Server) listCheckouts(ctx context.Context, customer string, from, to int64) ([]billingstripe.Session, error) { |
| - sessions, err := s.billing.client.ListCheckoutsForCustomer(ctx, customer, from, to) |
| - if err != nil { |
| - return nil, err |
| - } |
| + // A listing that fails on a later page still returns the pages it |
| + // read: they are recorded before the failure is the caller's. |
| + sessions, listErr := s.billing.client.ListCheckoutsForCustomer(ctx, customer, from, to) |
| for i := range sessions { |
| if err := s.store.noteSessionSeen(s.billing.cfg.LiveMode, &sessions[i], time.Now()); err != nil { |
| return nil, err |
| } |
| } |
| + if listErr != nil { |
| + return nil, listErr |
| + } |
| return sessions, nil |
| } |
| @@ -785,15 +906,20 @@ func (s *Server) advanceRefusal(ctx context.Context, intent billingIntent, acct |
| return s.store.noteExpiryTry(acct.ID, intent.ID, now) |
| } |
| -// billingPassRefusals works a bounded number of refused intents, in the |
| -// order they stand, after the pass's other work. |
| +// billingPassRefusals works a bounded number of refused intents after |
| +// the pass's other work: of those whose try is due, the ones that have |
| +// been due longest. A refusal that was just tried is due again an hour |
| +// later at the earliest, so one that stays due is reached within a few |
| +// passes however many others there are; taken in the order they stand, |
| +// the same old ones could use the budget for ever. |
| func (s *Server) billingPassRefusals(ctx context.Context, refused []billingIntent, now time.Time) []error { |
| var failures []error |
| - worked := 0 |
| + type due struct { |
| + intent billingIntent |
| + since int64 |
| + } |
| + var queue []due |
| for _, intent := range refused { |
| - if ctx.Err() != nil || worked >= billingPassRefusalBudget { |
| - break |
| - } |
| st, err := s.store.refusalState(intent.AccountID, intent.ID) |
| if err != nil { |
| failures = append(failures, err) |
| @@ -802,6 +928,15 @@ func (s *Server) billingPassRefusals(ctx context.Context, refused []billingInten |
| if !st.Stands || !st.Exists || st.Fate != "" || st.Disputed != 0 || st.pinned() == "" || now.Unix() < st.NextTry { |
| continue |
| } |
| + queue = append(queue, due{intent, st.NextTry}) |
| + } |
| + sort.SliceStable(queue, func(i, j int) bool { return queue[i].since < queue[j].since }) |
| + worked := 0 |
| + for _, d := range queue { |
| + intent := d.intent |
| + if ctx.Err() != nil || worked >= billingPassRefusalBudget { |
| + break |
| + } |
| acct, err := s.store.AccountByID(intent.AccountID) |
| if err != nil || acct == nil { |
| if err != nil { |
diff --git a/internal/console/billingretirement_test.go b/internal/console/billingretirement_test.go| index 06dc0a260..1e4dd9696 100644 |
| --- a/internal/console/billingretirement_test.go |
| +++ b/internal/console/billingretirement_test.go |
| @@ -540,11 +540,25 @@ func TestEvidenceAgainstARecordedFateIsADispute(t *testing.T) { |
| t.Fatalf("the disputed intent: %+v %v; want it standing", standing, err) |
| } |
| // The ordinary expiry retirement does not take it either, review or |
| - // no review: with the review closed by hand (as nothing in the |
| - // console does), an expiry proof still retires nothing. |
| + // no review. With the review closed by hand (as nothing in the |
| + // console does), an expiry proof files it again, because a |
| + // subscription was seen naming the intent (the completion's own); and |
| + // with that taken away by hand too, the dispute alone still stops it. |
| if _, err := fx.st.db.Exec(`UPDATE billing_reviews SET resolved_at=? WHERE account_id=? AND subscription=? AND reason=?`, f.now.Unix(), acct.ID, intent.ID, billingCheckoutRefusedReason); err != nil { |
| t.Fatal(err) |
| } |
| + early := &billingstripe.Session{ID: intent.SessionID, Status: "expired", LiveMode: false, ClientReferenceID: acct.Sqid, CustomerID: "cus_paid", Metadata: map[string]string{"checkout_intent": intent.ID, "account_sqid": acct.Sqid}} |
| + if err := fx.st.expireBillingCheckout(standing, early, f.now.Add(30*time.Minute)); !errors.Is(err, errBillingUnderReview) { |
| + t.Fatalf("an expiry proof for an intent a subscription names: %v; want it left under review", err) |
| + } |
| + if open, _ := fx.st.refusalReviewOpen(acct.ID, intent.ID); !open { |
| + t.Fatal("the review was not filed again by the expiry") |
| + } |
| + for _, q := range []string{`UPDATE billing_reviews SET resolved_at=1 WHERE account_id=? AND subscription=?`, `DELETE FROM billing_review_obligations WHERE account_id=? AND intent_id=?`} { |
| + if _, err := fx.st.db.Exec(q, acct.ID, intent.ID); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| proof := &billingstripe.Session{ID: intent.SessionID, Status: "expired", LiveMode: false, ClientReferenceID: acct.Sqid, CustomerID: "cus_paid", Metadata: map[string]string{"checkout_intent": intent.ID, "account_sqid": acct.Sqid}} |
| if err := fx.st.expireBillingCheckout(standing, proof, f.now.Add(time.Hour)); !errors.Is(err, errBillingConflict) { |
| t.Fatalf("an expiry proof for a disputed intent: %v; want a conflict", err) |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 632d355b4..6d457819b 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -1280,17 +1280,12 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| } |
| return "", errBillingIdentity |
| } |
| - r, err := s.store.db.Exec(`UPDATE billing_checkout_intents SET session_id=?,expires_at=? WHERE account_id=? AND id=? AND (session_id='' OR session_id=?)`, session.ID, session.ExpiresAt, acct.ID, intent.ID, session.ID) |
| - if err != nil { |
| - return "", err |
| - } |
| - n, err := r.RowsAffected() |
| - if err != nil { |
| + // Bound by the store, which refuses a session that is already another |
| + // purchase's and one that is not the session the intent's open review |
| + // names (bindIntentSession). |
| + if err := s.store.bindIntentSession(intent, session.ID, nil, session.ExpiresAt); err != nil { |
| return "", err |
| } |
| - if n != 1 { |
| - return "", errBillingConflict |
| - } |
| return session.SubscriptionID, nil |
| } |
diff --git a/internal/console/billingservice_test.go b/internal/console/billingservice_test.go| index 72606347e..f0bca5ae7 100644 |
| --- a/internal/console/billingservice_test.go |
| +++ b/internal/console/billingservice_test.go |
| @@ -70,6 +70,9 @@ type fakeBillingStripe struct { |
| // sessionNoSubscription makes a completed session carry no |
| // subscription id at all: read before the provider attached one. |
| sessionNoSubscription bool |
| + // listSecondPageFails makes the session listing answer one page that |
| + // says there is more, and fail the next. |
| + listSecondPageFails bool |
| // otherSubscriptions are further subscription ids the provider knows |
| // (sub_paid always exists), served on the same terms; and |
| // subscriptionSecondItem gives every subscription a second item. |
| @@ -254,6 +257,14 @@ func (f *fakeBillingStripe) serve(w http.ResponseWriter, r *http.Request) { |
| } |
| out = map[string]any{"data": rows, "has_more": false} |
| case r.URL.Path == "/v1/checkout/sessions" && r.Method == http.MethodGet: |
| + if f.listSecondPageFails { |
| + if r.URL.Query().Get("starting_after") != "" { |
| + w.WriteHeader(http.StatusInternalServerError) |
| + return |
| + } |
| + out = map[string]any{"data": []any{f.session()}, "has_more": true} |
| + break |
| + } |
| out = map[string]any{"data": []any{f.session()}, "has_more": false} |
| case r.URL.Path == "/v1/checkout/sessions" && r.Method == http.MethodPost: |
| _ = r.ParseForm() |
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go| index a3780a9be..8f4aaf36c 100644 |
| --- a/internal/console/billingstore.go |
| +++ b/internal/console/billingstore.go |
| @@ -383,6 +383,19 @@ func (s *Store) beginBillingCheckout(want billingIntent, now time.Time) (billing |
| if current.House != (want.Coupon != "") { |
| return billingIntent{}, errBillingConflict |
| } |
| + // So were the account's customer and the currency it is held to. A |
| + // purchase recorded and ended by another console since then leaves a |
| + // customer this request does not name and a currency it may not be |
| + // in: an intent made from the stale reading would create a second |
| + // customer and offer a checkout whose purchase could never record. |
| + // The request is refused, and the page is quoted again from what the |
| + // account now is. |
| + if current.Customer != want.Customer { |
| + return billingIntent{}, errBillingConflict |
| + } |
| + if current.Plan != "" && current.priceTerms().Money() != want.priceTerms().Money() { |
| + return billingIntent{}, errBillingConflict |
| + } |
| if err := checkIntentMembershipTx(tx, want); err != nil { |
| return billingIntent{}, err |
| } |
| @@ -408,8 +421,46 @@ func (s *Store) saveBillingCheckout(intent billingIntent, sessionID, dest string |
| if sessionID == "" || dest == "" || expires <= intent.CreatedAt { |
| return errors.New("billing_session_invalid") |
| } |
| - r, err := s.db.Exec(`UPDATE billing_checkout_intents SET session_id=?,url=?,expires_at=? |
| - WHERE account_id=? AND id=? AND (session_id='' OR session_id=?)`, sessionID, dest, expires, intent.AccountID, intent.ID, sessionID) |
| + return s.bindIntentSession(intent, sessionID, &dest, expires) |
| +} |
| + |
| +// bindIntentSession binds an intent to its session: the one place a |
| +// session id is written onto an intent. It refuses (an identity refusal) |
| +// a session that is already the console's record of another purchase |
| +// (another intent's, a retired or kept one's, one a purchase consumed), |
| +// and a session other than the one the intent's open refusal review |
| +// names: a review pins the session it was filed about, and what is known |
| +// of that session is not carried onto another. dest is nil when the |
| +// binder has no URL to save (a session bound from its completion). |
| +func (s *Store) bindIntentSession(intent billingIntent, sessionID string, dest *string, expires int64) error { |
| + if sessionID == "" { |
| + return errors.New("billing_session_invalid") |
| + } |
| + tx, err := s.db.Begin() |
| + if err != nil { |
| + return err |
| + } |
| + defer tx.Rollback() |
| + if elsewhere, err := sessionBelongsElsewhereTx(tx, sessionID, intent.AccountID, intent.ID); err != nil { |
| + return err |
| + } else if elsewhere { |
| + return errBillingIdentity |
| + } |
| + st, err := refusalStateQ(tx, intent.AccountID, intent.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if st.Open && st.Reference != "" && st.Reference != sessionID { |
| + return errBillingIdentity |
| + } |
| + var r sql.Result |
| + if dest != nil { |
| + r, err = tx.Exec(`UPDATE billing_checkout_intents SET session_id=?,url=?,expires_at=? |
| + WHERE account_id=? AND id=? AND (session_id='' OR session_id=?)`, sessionID, *dest, expires, intent.AccountID, intent.ID, sessionID) |
| + } else { |
| + r, err = tx.Exec(`UPDATE billing_checkout_intents SET session_id=?,expires_at=? |
| + WHERE account_id=? AND id=? AND (session_id='' OR session_id=?)`, sessionID, expires, intent.AccountID, intent.ID, sessionID) |
| + } |
| if err != nil { |
| return err |
| } |
| @@ -420,7 +471,7 @@ func (s *Store) saveBillingCheckout(intent billingIntent, sessionID, dest string |
| if n != 1 { |
| return errBillingConflict |
| } |
| - return nil |
| + return tx.Commit() |
| } |
| func (s *Store) expireBillingCheckout(intent billingIntent, proof *billingstripe.Session, now time.Time) error { |
| @@ -456,6 +507,36 @@ func (s *Store) retireIntent(intent billingIntent, proof *billingstripe.Session, |
| return err |
| } |
| defer tx.Rollback() |
| + // The proof is judged first: an answer that names another account or |
| + // customer is nobody's, and nothing below is done on it. |
| + var sqid string |
| + if err := tx.QueryRow(`SELECT sqid FROM accounts WHERE id=?`, intent.AccountID).Scan(&sqid); err != nil { |
| + return err |
| + } |
| + if proof.ClientReferenceID != sqid || proof.Metadata["account_sqid"] != sqid || (intent.Customer != "" && proof.CustomerID != intent.Customer) { |
| + return errBillingIdentity |
| + } |
| + // An intent a subscription was seen naming is not retired on its |
| + // session's expiry: an expired session has no subscription, so that |
| + // one was made outside the checkout, and expiring the session says |
| + // nothing about what it has collected or will. The refusal is filed |
| + // here (naming the session; its fate is left for a read to prove) and |
| + // the intent stands under it. Only when the row is still this |
| + // intent's with this session. |
| + var same bool |
| + if err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_checkout_intents WHERE account_id=? AND id=? AND session_id=? AND live_mode=?)`, intent.AccountID, intent.ID, intent.SessionID, intent.LiveMode).Scan(&same); err != nil { |
| + return err |
| + } |
| + if same { |
| + if held, err := holdForNamedSubscriptionsTx(tx, intent.AccountID, intent.ID, proof.ID, now); err != nil { |
| + return err |
| + } else if held { |
| + if err := tx.Commit(); err != nil { |
| + return err |
| + } |
| + return errBillingUnderReview |
| + } |
| + } |
| // An intent under an open review stands until the operator resolves |
| // it, whatever its session's state: the review may name a session the |
| // console itself expired (ownBoundSession, reconcile's refusal), and |
| @@ -464,7 +545,7 @@ func (s *Store) retireIntent(intent billingIntent, proof *billingstripe.Session, |
| // first, where none is recorded (the delete barrier reads it), and an |
| // intent whose session is recorded complete, or whose evidence is |
| // disputed, is never retired on an expiry proof: that contradiction is |
| - // a person's. All of it rolls back with the identity check below. |
| + // a person's. The fate rolls back with the delete when it does not go. |
| if _, err := tx.Exec(`UPDATE billing_checkout_intents SET session_fate=?,session_fate_at=? WHERE account_id=? AND id=? AND session_id=? AND session_fate=''`, |
| sessionFateExpired, max(now.Unix(), 1), intent.AccountID, intent.ID, intent.SessionID); err != nil { |
| return err |
| @@ -483,13 +564,6 @@ func (s *Store) retireIntent(intent billingIntent, proof *billingstripe.Session, |
| if n != 1 { |
| return errBillingConflict |
| } |
| - var sqid string |
| - if err := tx.QueryRow(`SELECT sqid FROM accounts WHERE id=?`, intent.AccountID).Scan(&sqid); err != nil { |
| - return err |
| - } |
| - if proof.ClientReferenceID != sqid || proof.Metadata["account_sqid"] != sqid || (intent.Customer != "" && proof.CustomerID != intent.Customer) { |
| - return errBillingIdentity |
| - } |
| if _, err := tx.Exec(`UPDATE billing_quantity_events SET processed_at=?,processing_outcome='checkout_expired',processing_reference=? WHERE account_id=? AND intent_id=? AND subscription='' AND processed_at=0`, now.Unix(), proof.ID, intent.AccountID, intent.ID); err != nil { |
| return err |
| } |
| @@ -511,7 +585,25 @@ func (s *Store) discardUnreadyIntent(intent billingIntent, now time.Time) (bool, |
| return false, err |
| } |
| defer tx.Rollback() |
| - r, err := tx.Exec(`DELETE FROM billing_checkout_intents WHERE account_id=? AND id=? AND session_id='' AND customer_ready=0`, intent.AccountID, intent.ID) |
| + // Not one a subscription was seen naming: no session was ever made |
| + // for this intent, so that subscription was made outside checkout. |
| + // The refusal is filed here and the intent stands under it. And not |
| + // one under an open refusal review, whoever filed it: only the |
| + // operator's resolution ends a review. |
| + var unready bool |
| + if err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_checkout_intents WHERE account_id=? AND id=? AND session_id='' AND customer_ready=0)`, intent.AccountID, intent.ID).Scan(&unready); err != nil { |
| + return false, err |
| + } |
| + if unready { |
| + if held, err := holdForNamedSubscriptionsTx(tx, intent.AccountID, intent.ID, "", now); err != nil { |
| + return false, err |
| + } else if held { |
| + return false, tx.Commit() |
| + } |
| + } |
| + r, err := tx.Exec(`DELETE FROM billing_checkout_intents WHERE account_id=? AND id=? AND session_id='' AND customer_ready=0 |
| + AND NOT EXISTS (SELECT 1 FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0)`, |
| + intent.AccountID, intent.ID, intent.AccountID, intent.ID, billingCheckoutRefusedReason) |
| if err != nil { |
| return false, err |
| } |
| @@ -773,7 +865,16 @@ func (s *Store) applyBillingStateDated(next billingState, expectedRevision int64 |
| return err |
| } |
| } |
| - if next.IntentID != "" { |
| + if next.IntentID != "" && current.Subscription != next.Subscription { |
| + // Only a first purchase consumes an intent: the record of a |
| + // subscription that is not yet the account's recorded one, judged |
| + // here from the account's row as this transaction reads it. A |
| + // later event of the recorded subscription names the intent it |
| + // was bought on, which went with its first record; if its |
| + // metadata has been made to name another (a newer purchase's |
| + // standing intent, after this subscription ended), that intent |
| + // is not this subscription's to delete. |
| + // |
| // The intent is consumed only if it stands and is not under an open |
| // refusal review: a purchase corrected at the provider while its |
| // review is open waits for the operator (who resolves the review |
| @@ -782,17 +883,61 @@ func (s *Store) applyBillingStateDated(next billingState, expectedRevision int64 |
| // Nor one whose evidence is disputed: a purchase kept and then |
| // contradicted waits for a person too, as a conflict here and not |
| // as the delete barrier's raw abort. |
| + var usedSession, usedCustomer string |
| + if err := tx.QueryRow(`SELECT session_id,customer FROM billing_checkout_intents WHERE account_id=? AND id=?`, next.AccountID, next.IntentID).Scan(&usedSession, &usedCustomer); err != nil && !errors.Is(err, sql.ErrNoRows) { |
| + return err |
| + } |
| r, err := tx.Exec(`DELETE FROM billing_checkout_intents WHERE account_id=? AND id=? AND fate_disputed_at=0 |
| AND NOT EXISTS (SELECT 1 FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0)`, |
| next.AccountID, next.IntentID, next.AccountID, next.IntentID, billingCheckoutRefusedReason) |
| if err != nil { |
| return err |
| } |
| - if n, err := r.RowsAffected(); err != nil { |
| + n, err := r.RowsAffected() |
| + if err != nil { |
| return err |
| - } else if consumeIntent && n != 1 { |
| + } |
| + if consumeIntent && n != 1 { |
| return errBillingConflict |
| } |
| + if n == 1 { |
| + // What was consumed is kept, with the subscription it recorded |
| + // with: a subscription that names this intent afterwards is |
| + // compared with it (classifySubscriptionTx). And every other |
| + // subscription already seen naming the intent while it stood |
| + // is a dispute now, in this transaction: the purchase records, |
| + // and the account is held until a person has looked at what |
| + // else was made in its name. |
| + if _, err := tx.Exec(`INSERT OR REPLACE INTO billing_consumed_intents(account_id,intent_id,session_id,subscription,customer,consumed_at) VALUES(?,?,?,?,?,?)`, |
| + next.AccountID, next.IntentID, usedSession, next.Subscription, usedCustomer, effective.Unix()); err != nil { |
| + return err |
| + } |
| + rows, err := tx.Query(`SELECT subscription FROM billing_review_obligations WHERE account_id=? AND intent_id=? AND subscription<>? ORDER BY subscription`, next.AccountID, next.IntentID, next.Subscription) |
| + if err != nil { |
| + return err |
| + } |
| + var others []string |
| + for rows.Next() { |
| + var other string |
| + if err := rows.Scan(&other); err != nil { |
| + rows.Close() |
| + return err |
| + } |
| + others = append(others, other) |
| + } |
| + if err := rows.Close(); err != nil { |
| + return err |
| + } |
| + for _, other := range others { |
| + key := usedSession |
| + if key == "" { |
| + key = next.IntentID |
| + } |
| + if err := disputeSessionTx(tx, next.AccountID, key, next.IntentID, "subscription "+other+" named the intent before its purchase recorded with "+next.Subscription, effective); err != nil { |
| + return err |
| + } |
| + } |
| + } |
| } |
| if err := refreshQuantityChangeTx(tx, next.AccountID, queuedAt); err != nil { |
| return err |
| @@ -1095,8 +1240,10 @@ func (s *Store) fileCheckoutRefusal(intent billingIntent, sessionID, subscriptio |
| // The intent went (retired, or its purchase kept and recorded) |
| // between the caller's read and this filing. The subscription it |
| // was filing for is still evidence: classified here, in this |
| - // transaction, against what the intent was disposed on, so it is |
| - // a dispute and never nothing. |
| + // transaction, against what the intent was disposed on (retired, |
| + // kept, or consumed by a purchase), where it is a dispute. Of an |
| + // intent that expired in the ordinary way nothing is kept, and |
| + // the subscription is then the rejected event alone. |
| if err := classifySubscriptionTx(tx, intent.AccountID, intent.ID, subscription, observed); err != nil { |
| return err |
| } |
diff --git a/internal/console/billingstore_test.go b/internal/console/billingstore_test.go| index 305b65b31..b9b26d96f 100644 |
| --- a/internal/console/billingstore_test.go |
| +++ b/internal/console/billingstore_test.go |
| @@ -136,6 +136,18 @@ func TestBillingStateAtomicEventAndRevision(t *testing.T) { |
| if err := s.applyBillingState(x, 2, "evt_three", now); !errors.Is(err, errBillingConflict) { |
| t.Fatalf("changed customer: %v", err) |
| } |
| + // The next purchase names the customer the account now has, as the |
| + // start path does (it reads the account first); one that does not is |
| + // a stale request and is refused. |
| + if _, err := s.beginBillingCheckout(want, now); !errors.Is(err, errBillingConflict) { |
| + t.Fatalf("checkout after cancellation, from a reading older than the purchase: %v; want a conflict", err) |
| + } |
| + // (This fixture recorded its purchase without naming the intent, so |
| + // the first intent still stands; a real record consumes it.) |
| + if _, err := s.db.Exec(`DELETE FROM billing_checkout_intents WHERE account_id=?`, want.AccountID); err != nil { |
| + t.Fatal(err) |
| + } |
| + want.Customer = got.Customer |
| if _, err := s.beginBillingCheckout(want, now); err != nil { |
| t.Fatalf("checkout after cancellation: %v", err) |
| } |
| @@ -212,7 +224,7 @@ func TestBillingIntentPinsCustomerAndCoupon(t *testing.T) { |
| want.Customer, want.Coupon = "cus_original", "coupon_house" |
| // A coupon is only ever attached for a house account, and the intent |
| // transaction now checks the marker agrees with the coupon. |
| - if _, err := s.db.Exec(`INSERT INTO billing_accounts(account_id,house) VALUES(?,1) ON CONFLICT(account_id) DO UPDATE SET house=1`, want.AccountID); err != nil { |
| + if _, err := s.db.Exec(`INSERT INTO billing_accounts(account_id,house,customer) VALUES(?,1,'cus_original') ON CONFLICT(account_id) DO UPDATE SET house=1,customer='cus_original'`, want.AccountID); err != nil { |
| t.Fatal(err) |
| } |
| first, err := s.beginBillingCheckout(want, now) |