amadan
Review nineteen answered as one evidence invariant: every provider observation about a refused purchase is classified and recorded in one store transaction, and a refusal and its subscription are one transaction
Both reviewers found stage one's core sound and the same family of gaps: evidence recorded at some call sites and returned past at others, and a refusal filed in one transaction with its subscription attached in another. Patched call by call that would recur; the answer is one invariant (billingretirement.go's header; the design's "Stage one as built"): anything the provider shows about a refused purchase, its pinned session or a subscription naming its intent, goes through one of two store transactions (noteSessionObservation, noteSubscription) that load the purchase as it stands at that instant, standing, retired, or kept and consumed, and record it as the session's fate, an attached subscription or a dispute; nothing is dropped. A caller decides nothing from its own read and records before it answers: reconcile hands in every subscription that is not the account's recorded one before any pending, identity or retired answer, and routes a structurally invalid subscription naming the bound intent through the refusal; the binder hands the session in before its not-complete, retired and no-intent answers; the keep gate hands in its own read. fileCheckoutRefusal files the review and attaches the subscription in one transaction, and an attachment is made only while the intent stands. A subscription seen after a keep while the intent stands reopens the review; after the record it is a dispute compared with the resolution's archived session and subscription. The record refuses a disputed kept intent as a conflict. The pass's refusal turn runs under its own deadline. The tie-break test uses three ids, so it fails for id in either direction. Seven counterexample tests added, four reworked; the full console, plans and billingstripe run passes. Review eighteen's P2 stays recorded as open until a round accepts this.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
12 files changed,
+1208
−223
docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md+7 −5docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review19.md+175 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+24 −0docs/superpowers/specs/OPUS-REVIEW-multi-currency-review19.md+178 −0internal/console/billingcurrency_test.go+22 −12internal/console/billingoperator.go+17 −4internal/console/billingpass.go+46 −22internal/console/billingretirement.go+225 −138internal/console/billingretirement_test.go+413 −13internal/console/billingservice.go+64 −23internal/console/billingservice_test.go+13 −2internal/console/billingstore.go+24 −4
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 5964bc481..14f47b10e 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 |
| @@ -112,13 +112,15 @@ Astra's third review (`CODEX-FINDINGS-intent-retirement-design-review3.md`) reje |
| Built in `internal/console/billingretirement.go` and the paths it touches, with one test per interleaving in `billingretirement_test.go`. Where the build differs from the text above, this is what it does and why. |
| -- **No generation counter.** Revision 3 put a generation on the review, to be compared across an actor's provider work. Stage one needs none, because every fact a resolution reads is monotonic and is read inside the resolution's own transaction: a fate is written once and never rewritten; an attached subscription is only ever added; a dispute is only ever set; the review's protocol mark does not change while it is open. A fate, an attached subscription or a dispute recorded after the operator looked and before the transaction ran is therefore what the transaction sees. Evidence that contradicts a recorded fate is never discarded for arriving later or earlier: it is a dispute. |
| -- **A fate comes from a read, never from an expiry answer.** `advanceRefusal` reads the pinned session, requests the expiry of a bound session still open, and reads again; only a read whose identity clauses all hold is recorded. A payment that beats the expiry shows as `complete` in that second read. |
| -- **Webhooks: evidence before acknowledgement, idempotently, not one transaction.** The binder and reconcile record what they prove (a fate, an attached subscription, a dispute) in their own transactions, each of which changes nothing when repeated, and only then is the event recorded as rejected and seen. A crash between the two leaves the event unseen, the provider delivers it again, and the evidence writes are no-ops. A dispute is keyed by its session and its wording, so the same contradiction delivered twice files one review once. |
| +- **The evidence invariant** (added after round nineteen, which showed the first build of stage one recorded evidence at some call sites and returned early at others, and filed a refusal and its subscription in two transactions). Anything the provider shows about a refused purchase is classified and recorded by one store transaction that loads the purchase as it is at that instant, standing, retired, or kept and consumed: `noteSessionObservation` for an answer about its pinned session, `noteSubscription` for a subscription that names its intent. A caller decides nothing from a read of its own and records before it answers or acknowledges what carried the evidence: reconcile hands in every subscription that is not the account's recorded one before any pending, identity or retired answer; the binder hands in the session before its not-complete, retired and no-intent answers; the keep gate hands in its own read. A refusal and the subscription that caused it are one transaction (`fileCheckoutRefusal`). Nothing observed is dropped: it becomes the session's fate, an attached subscription, or a dispute. A crash or another console between any two of these transactions leaves each one whole, and the next delivery or pass meets the same classification. |
| +- **No generation counter.** Revision 3 put a generation on the review, to be compared across an actor's provider work. Stage one needs none, and not because a caller's read is current (it may not be): every fact a resolution reads is monotonic and is read inside the resolution's own transaction, and every classification of evidence is made inside the transaction that records it. A fate is written once and never rewritten; an attached subscription is only ever added, and only while the intent stands; a dispute is only ever set; the review's protocol mark does not change while it is open. Evidence that contradicts a recorded fate is never discarded for arriving later or earlier, or for the caller having read the purchase before it changed: it is a dispute. |
| +- **A fate comes from a read, never from an expiry answer** (one exception, inside one transaction: the ordinary expiry retirement of an intent with no review writes `expired` from the proof it was handed, which may be an expiry answer, immediately before it deletes the row; the delete barrier reads it and nothing else can). `advanceRefusal` reads the pinned session, requests the expiry of a bound session still open, and reads again; only a read whose identity clauses all hold is recorded. A payment that beats the expiry shows as `complete` in that second read. |
| +- **Webhooks: evidence before acknowledgement, idempotently, not one transaction.** The binder and reconcile hand what they read to the store's classification before any answer (see the evidence invariant), each transaction changing nothing when repeated, and only then is the event recorded as rejected and seen. A crash between the two leaves the event unseen, the provider delivers it again, and the evidence writes are no-ops. A dispute is keyed by its session and its wording, so the same contradiction delivered twice files one review once. |
| - **`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 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 (only the console stopping cancels it), 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. |
| -- **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 is filed as a dispute for that session, once. |
| +- **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. |
| What stage one leaves held, with no disposition: a paid session that is not kept; any case a subscription made outside checkout is attached to; any disputed case; every review opened before this build. Decisions 1 to 5 govern those. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review19.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review19.md| new file mode 100644 |
| index 000000000..8a4f2e203 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review19.md |
| @@ -0,0 +1,175 @@ |
| +Platform references below are at **`59666bab`**; pricing-site references are at **`4026231`**. This was source-only: no modifications, tests, network commands or AWS commands. Test counterfactuals are judgments from source inspection. |
| + |
| +| Eighteenth-review finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1 / Astra 1:** retirement while the session remains payable | **The sequential reproduction is closed; the full obligation-preservation requirement is not.** Retirement now requires recorded proof, and failed expiry is retried. Findings 1–3 below identify remaining release and evidence-loss paths. | **No** for restoring the old operator behavior: `billingretirement_test.go:152` expects 503 while expiry fails and a continuing hold afterwards. **Yes** with the new races below: that test never pauses between evidence writes. | |
| +| **Opus P3-1:** nil-billing protection lacked a test | **Closed.** `billingcurrency_test.go:4305` actually disables billing; `billingoperator.go:382` returns 409. | **No.** Removing the protection reaches a nil dereference. | |
| +| **Opus P3-2:** same-second rejected-event ordering was untested | **Closed.** `billingcurrency_test.go:4249` inserts IDs whose lexical order opposes arrival order. | **No.** Replacing `rowid DESC` with `id` fails the explicit order assertion at `:4265`. | |
| +| **Astra 2:** concurrent identical `keep` can answer a gate failure | **Closed for the reported interleaving.** The door rereads resolution state after a gate failure and lets the store compare the replay (`billingoperator.go:351`, `:386`). That database reread also sees another console’s commit. | **No.** The resolution committed during the fake’s gate read at `billingcurrency_test.go:4316` would receive 409 instead of 200. | |
| +| **Astra 3:** retired completion beside a replacement loses account attribution | **Closed.** The account-scoped retirement lookup now precedes standing-intent lookup (`billingservice.go:1180`). | **No.** Restoring the lookup beneath “no intent stands” fails the rejected-row account assertion at `billingcurrency_test.go:3077`. | |
| + |
| +| Eighteenth-review claim | Assessment at the requested commits | |
| +|---|---| |
| +| **Opus claim 1 / Astra:** claimed billing-off coverage did not exist | Corrected by the new test above and the qualification in `DISPOSITIONS-multi-currency.md:361`. | |
| +| **Opus claim 2 / Astra:** contradictory expiry-order comments | The duplicate filing description and `refuse()` ordering are corrected. Other lifecycle comments remain stale, listed below. | |
| +| **Opus claim 3 / Astra:** every next read retries expiry; listing tests establish order | Scheduled refusal retries now exist. The distinguishing tie test now exists. The old lifecycle test’s timestamp comparison remains weak, but no longer supplies the only ordering coverage. | |
| +| **Opus claim 4 / Astra:** fractional-floor test asserts actual refusal | **Still false at site `4026231`.** It searches source text. Dispositions cite a later fix at `01b62f7`; that is outside the requested site commit. | |
| +| **Astra:** other rejected identities have no resolvable account | Still too broad. Several bare identity refusals already know the account; only typed retirement errors carry it into the rejected listing. | |
| +| **Astra:** complete listing coverage | Bound, receipt-time direction, specific account attribution and now tie order are tested. The bulk test still does not inspect every intermediate position/account. | |
| +| **Astra:** older resolution rows were migration-tested | Correct. The test reopens a database containing a row without `outcome`, checks default `retire`, and checks replay identity (`billingcurrency_test.go:4210`). | |
| +| **Astra:** late-completion fixture establishes non-recording | Correct for **legacy retirement**. It uses a distinct replacement session and checks both account state and replacement binding (`billingcurrency_test.go:3034`, `:3083`). Recording the retired purchase would fail it. | |
| +| **Astra:** mutation evidence and clock isolation | Historical mutation totals cannot be established from source. No additional concrete shared-cookie, temporary-root or ordering failure was found. Some fixtures use wall-clock time; no tests were run. | |
| + |
| +| Third design-review P2 | Closed? | Test counterfactual | |
| +|---|---|---| |
| +| **Keep with another subscription attached** | **Partly.** Persisted other subscriptions block `keep` (`billingretirement.go:190`), but their capture is incomplete, and the fresh gate can disagree with the stored subscription. Findings 1–2. | `TestKeepIsHeldWhileAnotherSubscriptionIsAttached` fails without the persisted-obligation check. It passes with both identified gaps. | |
| +| **Cancellation standing in for settlement under retire** | **Closed for recorded obligations.** Any attached subscription blocks retirement; cancellation is never treated as settlement (`billingretirement.go:176`). Incomplete capture still undermines the broader guarantee. | The hand-made-subscription test fails if that hold is removed. It does not exercise cancellation or interrupted attachment recording. | |
| +| **Migrated reviews with no recorded obligations** | **Closed for standing intents.** Migration defaults `protocol=0`; filing an already-open review preserves it; resolution refuses it (`billingretirement.go:81`, `:165`; `billingstore.go:1070`). | The hold test fails without the protocol guard. It manually sets `protocol=0`, so it does **not** independently test migration of that column. | |
| +| **Late contradictions after the intent is gone** | **Not closed.** Some evidence never reaches the surviving dispute machinery. Finding 3. | Existing tests pass with this defect. They test late subscription evidence after retirement and contradictory completion while the intent still stands—not the missing paths. | |
| + |
| +Both eighteenth reports classified the payable-retirement defect as **P2**; their disagreement was the verdict. The code supports treating it as merge-blocking. On the earlier severity dispute, Astra’s reasoning was correct: the pass’s expiry branch does not take reconciliation’s account lock, so the interval before review filing was real. |
| + |
| +On expiring contradictory metadata, the current distinction is sound: a previously validated **stored bound session ID** supplies authority to request expiry. A newly returned, unowned create/listing ID does not. Expiry authority and proof sufficient to retire must remain separate. |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — Subscription obligations are neither captured atomically with refusal nor captured on every attributable refusal.** |
| + |
| + **Where:** `internal/console/billingservice.go:884`, `:892`, `:829`, `:996`; `internal/console/billingretirement.go:260`. |
| + |
| + **Scenario:** A hand-made subscription H names intent I and has the wrong price. Console A files I’s review at `:884`, then pauses before adding H at `:892`. Console B resolves the review: it expires the bound session, records an expired fate, sees no obligation and retires I. |
| + |
| + When A resumes, `addRefusalObligation` inserts H against the now-resolved review. It creates no dispute or renewed hold. The webhook can then acknowledge the refusal. H remains collectible although the case has been released. |
| + |
| + A process death between those writes creates the same opportunity. Redelivery is not a sufficient repair: once the late attachment exists, `noteLateSubscription` treats it as already known. |
| + |
| + There are also paths requiring no race. A subscription naming the correct account and intent but containing two items returns at `:829`, before obligation capture. A subscription whose fields match the intent but whose ID differs from the bound session’s subscription returns at `:996`. Neither attaches the refused subscription. A subsequent `keep` can therefore release the checkout’s purchase beside that other subscription. |
| + |
| + **Smallest acceptable fix:** commit the attributable refusal and its triggering subscription together. Route structurally invalid but attributable subscriptions through that operation too. If the intent has already been resolved or consumed, record the newly discovered obligation as a surviving dispute rather than merely inserting an attachment. |
| + |
| + The existing tests would pass with these defects: every fixture finishes attachment recording before attempting resolution. |
| + |
| +2. **P2 — The fresh `keep` read can contradict the stored subscription without recording a dispute.** |
| + |
| + **Where:** `internal/console/billingoperator.go:247–263`; `internal/console/billingretirement.go:512`, `:190`. |
| + |
| + **Scenario:** The recorded complete fate names subscription C, and C is the case’s recorded obligation. At resolution, the provider returns the same owned, purchase-matching session as complete and paid, but now names subscription D. |
| + |
| + `advanceRefusal` skips work because a fate already exists. `billingKeepRefused` accepts the fresh session but never records its evidence. The transaction consequently compares obligations against **stored C**, not freshly observed D, and accepts `keep`. The contradiction is lost; C’s obligation has not been disposed of by accepting D. |
| + |
| + This is precisely the changed-nonempty-subscription case that revision 4 says must become a dispute. |
| + |
| + **Smallest acceptable fix:** persist the fresh gate read through the proof/dispute recorder before resolving. Require the subscription accepted by the gate to agree with the recorded complete evidence used by the transaction. A disagreement must leave the review open. |
| + |
| + `TestKeepIsHeldWhileAnotherSubscriptionIsAttached` does not cover this: its stored and freshly read checkout subscription agree. |
| + |
| +3. **P2 — Contradictory evidence can disappear after retirement/consumption, or through a stale preliminary state read.** |
| + |
| + **Where:** `internal/console/billingservice.go:1171`, `:1180`; `internal/console/billingretirement.go:418`, `:469–490`, `:327–350`. |
| + |
| + Three concrete paths remain: |
| + |
| + - **Completion after proven expiry retirement:** retire I on an expired fate, then deliver `checkout.session.completed` for its session. The binder returns the typed retirement rejection at `billingservice.go:1183` before recording session evidence. The event is acknowledged and listed, but no dispute is filed. The late-subscription test does not exercise this completion-only delivery. |
| + - **New subscription after kept purchase consumption:** keep and record C, consuming I. Later H names I. `noteLateSubscription` only recognizes retirement records, not consumed `keep` resolutions. Reconciliation can reject H without creating the required account dispute. |
| + - **Concurrent contradictory identity:** A reads refusal state with no fate at `billingretirement.go:469`. B records an expired fate. A then handles an identity contradiction using its stale state and returns at `:482`, recording nothing. B can retire despite contradictory evidence already observed by A. |
| + |
| + Archived handling is also narrower than its comment: it does not generally compare a consumed purchase’s fresh subscription/identity against the archived `keep` evidence. |
| + |
| + **Smallest acceptable fix:** use one transactional evidence operation that loads the standing or archived purchase context and classifies the observation inside that transaction. Invoke it before retirement/recorded fast returns, and for late subscriptions naming consumed intents. Preserve identity, nonterminal and changed-subscription contradictions, not only opposite terminal statuses. |
| + |
| + Existing tests would pass without these missing paths. `TestEvidenceAgainstARecordedFateIsADispute` keeps the intent standing throughout; `TestRetireOnAProvenExpiryWithNothingAttached` delivers a subscription event, not a late session completion. |
| + |
| +4. **P3 — Unrelated reconciliation can indefinitely prevent scheduled refusal expiry work.** |
| + |
| + **Where:** `internal/console/billingpass.go:34`, `:141`, `:172`, `:188`; `internal/console/billingretirement.go:556`. |
| + |
| + **Scenario:** Ordinary pending intents or subscription reads repeatedly consume the reconciliation pass’s two-minute context. The pass returns before its final refusal phase. A refused session whose expiry failed remains payable, even though its retry time has passed on every run. |
| + |
| + The three-refusal budget prevents refusals from starving other work; it does not protect refusal work from starvation. Per-intent backoff cannot help work that is never reached. |
| + |
| + **Smallest acceptable fix:** give refusal advancement an independently bounded scheduling turn or reserved time budget. Add a controlled test where earlier reconciliation exhausts its budget and a due refusal still receives a later turn. |
| + |
| + The current schedule test contains one refusal and no competing slow work, so it passes with this defect. |
| + |
| +5. **P3 — The scoped pricing-site test does not establish fractional-floor refusal.** |
| + |
| + **Where at site `4026231`:** `test/pricing-pricebook.test.mjs:246`; `copy-review/pricing/build.mjs:41`. |
| + |
| + **Scenario:** retain the searched guard text but make it inert. The test passes although fractional minor units no longer throw. Removing the guard text entirely fails, which is weaker than testing its behavior. |
| + |
| + **Smallest acceptable fix:** invoke the formatter with fractional minor units and assert the exception. The later site fix cited by dispositions is not present at the requested commit. |
| + |
| +**Claims the code contradicts, and the limits of the verified behavior** |
| + |
| +- **“Every fact … is monotonic,” therefore no generation counter is needed** (`2026-10-02-intent-retirement-protocol-design.md:115`). The persisted terminal fate, attachments and dispute flag mostly have the described monotonic behavior. That does not make separate refusal/attachment commits atomic, or make a state read outside the evidence transaction current. Findings 1–3 require transactional evidence handling; adding a counter alone would not repair omitted evidence. |
| + |
| +- **Evidence is recorded before webhook acknowledgement** (design `:117`) and **late contradictions survive the intent** (`:101`). Those claims exceed the actual early-return paths. Evidence writes being idempotent is insufficient when replay can take another path that never performs them. |
| + |
| +- **“One test per interleaving”** (design `:113`; dispositions `:384`). The eight retirement tests exercise useful state transitions, but none orchestrates two consoles, a crash between refusal and attachment, or evidence arriving after kept-intent consumption. |
| + |
| + | Retirement test | What it establishes; counterfactual limit | |
| + |---|---| |
| + | `TestRetireDoesNotReleaseAnIntentWhoseSessionIsStillPayable` (`:152`) | Meaningfully fails with old retirement behavior; verifies failed expiry, subscription hold, no resume URL and listing. No interrupted attachment write. | |
| + | `TestRetireOnAProvenExpiryWithNothingAttached` (`:215`) | Proof, archival evidence, exact replay and late subscription dispute. Removing those behaviors fails it. No late completion test. | |
| + | `TestAPaymentBeforeTheRetireIsACompletionNotARetirement` (`:287`) | Completion hold, unpaid-keep refusal, successful keep and protection from ordinary retirement. The fake completes **before** the operator request at `:308`; it does not test completion during expiry. | |
| + | `TestThePassRetriesAFailedExpiryOnItsSchedule` (`:367`) | Initial retry timing and cessation after fate recording. No budget, timeout or fairness coverage. | |
| + | `TestAReviewFromBeforeTheProtocolIsHeld` (`:404`) | Protocol-zero hold, including the store gate. It sets the marker manually, so the migration itself could be broken without this test detecting it. | |
| + | `TestTheDeleteBarrierHoldsARefusedIntent` (`:433`) | Actual SQL rejection, transaction rollback and permission after proven expiry. Removing the trigger fails it. | |
| + | `TestEvidenceAgainstARecordedFateIsADispute` (`:470`) | Standing-intent terminal contradiction and deduplication. It ignores webhook response codes; its final “ordinary expiry retirement” check only selects the dispute flag, rather than attempting retirement. | |
| + | `TestKeepIsHeldWhileAnotherSubscriptionIsAttached` (`:522`) | The persisted-other-subscription predicate. No concurrent discovery or changed gate subscription. | |
| + |
| +- **`retire` means the operator settled a paid wrong-currency purchase** (`billingstore.go:1002`). Stage one expressly refuses that disposition. The completed-refusal comment at `billingservice.go:353` likewise still describes retirement that stage one no longer permits. |
| + |
| +- **Dispute acknowledgement follows a fresh read and releases nothing** (design `:101`). Generic resolution of `billing_checkout_fate_disputed` performs no provider read: special handling applies only to `billingCheckoutRefusedReason` (`billingoperator.go:340`; `billingstore.go:974`). Resolving the dispute also removes that review’s invoice hold (`billinginvoicestore.go:167`). The documented acknowledgement/adjudication boundary needs to match that behavior. |
| + |
| +- **Review-eighteen replay qualification is process-local** (dispositions `:360`). It is not: the second read queries the shared database. Revision 4 correctly overrides that wording. |
| + |
| +- **Review-three/review-four mutation claims.** Historical counts remain unverified. The later qualifications correctly distinguish successful trigger migration from interruption testing, source-text provisioning assertions from execution, and the redundant account-only delete mutation from the preceding exact-intent lookup. The earlier inclusion-mutation claim is explicitly qualified later; it should not be presented as fresh evidence from this review. |
| + |
| +The ordinary currency and quote paths remain consistent: |
| + |
| +- **GBP purchase, end to end:** `GET ?country=IN¤cy=gbp` quotes band D in GBP—Personal £7.70 annually, with the band’s allowances. The submitted form is compared with its signed effective quote (`billinghandlers.go:71`). Checkout stores GBP/book terms and validates the configured price; creation disables Adaptive Pricing. Binding and reconciliation verify ownership and purchase identity (`billingservice.go:249`, `:911`, `:964`). The recording transaction writes those terms to the account and book assignment (`billingstore.go:756`). History, `AllowanceForMonth`, version-2 calculation, invoice requests/recovery and quantity updates retain that currency. I found no conversion or historical repricing path. |
| + |
| +- **One form, signature and cookies:** the server-keyed HMAC covers token, account, book, country, band, currency, choice flag and membership (`billingpage.go:446`). Another session/account/book/token or changed tuple cannot reuse it. Plan is unsigned because the page prices all offered plans; charged quantity follows signed membership and plan limits, with a membership-locked recheck. Postal affects tax/location, not the displayed base quote. Invalid, empty or repeated present currency fields receive 400 on both doors (`billinghandlers.go:176`). Omitted currency uses the country default on book checkout and EUR on legacy checkout; a mismatch with the displayed quote redirects. The quote door only redirects to the authenticated account’s billing path. |
| + |
| + Empty URL parameters clear cookies; absence permits fallback; invalid parameters write nothing. An invalid visit can therefore leave an older cookie available on a later parameter-absent visit, but cookies cannot override the current posted form. Held purchases offer only their stored currency, including legacy EUR (`billingpage.go:301`; `billingservice.go:597`). |
| + |
| +- **Ownership and lifecycle:** create, recovery, listing and expected-intent paths validate ownership before accepting a session. An unowned create/listing result is left open at the provider; declining to expire an ID never established as the console’s is correct. A contradictory **bound** session can be expired by its stored ID, but that expiry response alone does not release its refusal. |
| + |
| + First recording consumes the exact standing intent in the account-state transaction and refuses an open checkout review (`billingstore.go:782`); failure rolls back assignments, event and entitlement writes. Recorded renewals do not need to consume it again. Ordinary expiry retirement checks ownership and open-review state; stillborn cleanup is restricted to unready, unbound intents. Operator retirement archives and deletes only the named intent. These local protections hold, but Findings 1–3 prevent the stronger claim that every outstanding obligation and contradiction preserves a durable hold. |
| + |
| +- **Retirement protocol mechanics:** `sessionProof` requires the pinned ID, mode, intent, both account identifiers and known customer (`billingretirement.go:448`). Expired proof excludes paid sessions and subscriptions; complete is distinct from paid. `advanceRefusal` obtains proof from GET, including a GET after attempted expiry. Failed expiry schedules another attempt; completion becomes complete fate and cannot be retired as expired. Only the stored bound ID is expired here. |
| + |
| + The operator takes the account billing lock; provider failure produces 503, held state 409, and exact replay bypasses new provider requirements. Billing-off `keep` is refused; stored proof can still support a store-side retirement. The delete trigger protects payable refused intents but does not enforce the entire protocol against an older binary. Stopping older consoles before migration and forbidding rollback remain necessary. |
| + |
| +- **Legacy test helper:** `retiredBeforeTheProtocol` reconstructs plausible historical retirement effects—resolved review, empty evidence/fate, exact retirement record, closed checkout events and removed intent. It leaves the fixture’s protocol marker at 1 rather than a migrated 0, but those legacy replay paths do not consult it. Its two callers establish behavior after historical retirement, not authorization to retire those cases under stage one. |
| + |
| +- **Monthly barrier and rollout:** the refined trigger matches the positive paid span used by `AllowanceForMonth`: active, non-Free, clipped by the next assignment across both tables, paid-through time and month boundaries (`billingstore.go:208`; `month.go:146`). Same-second supersession contributes no span; Free/inactive successors terminate the preceding span. Renewal and quantity changes can move `effective_at`; `period_end` remains paid-through. I found no requested valid-history counterexample in either direction. |
| + |
| + Replacement-trigger existence is checked before dropping the old trigger inside a transaction (`billingstore.go:854`). Missing replacement fails safely; restart is idempotent; interruption can leave both temporarily, not neither. This does not make currency-unaware older binaries generally safe against book history. |
| + |
| +- **Inclusions, books and export:** inclusions divide pooled allowance by charged quantity, including the unavailable Personal fallback. Current permitted shares are exact in the displayed precision; band-A strings are pinned. Book validation, checked arithmetic, complete catalogs, unique provider IDs and own-only `currency_options` agree with the provisioning checks. The public export contains commercial definitions and country/resource mappings, not customer or credential data. |
| + |
| +- **Quantity history and operator listings:** `quantityOutcomesSQL` remains the common admission list, differing guards are replaced, and no healing sweep remains to erase existing history gaps. The rejected listing requires an operator, drains rows before account lookups, and bounds results to 200 with descending receipt/insertion order (`billingoperator.go:268`). Retryable failures and non-webhook refusals are not automatically rejected-list rows. |
| + |
| +- **Pricing site, section K:** its export matches the platform export. Annual prices, exact rates, floors, band-scaled inclusions, country list and option defaults derive from it. Draft marking and the exact `PRICEBOOK_ALLOW_DRAFT=1` gate are present; build/check render before validation. Both link parameters are always present, currency only when explicitly chosen and country as selected. Locale-only display can differ from the console’s later default because no choice was transmitted; that follows the owner’s rule. |
| + |
| + Wrong literals in checked slots fail value checks across currencies. A literal equal to the current exported value passes: the test establishes output equality, not provenance. Whole-token checks correctly reject embedded numeric matches. |
| + |
| +**What must be verified against Stripe test mode before enablement** |
| + |
| +1. **Catalog and checkout:** validate all 36 identities, amounts, quantities, tax settings and expanded own-currency options. Complete USD/EUR/GBP purchases, including IN/GBP and fractional-pound Community unit prices. Session, subscription, items and stored terms must agree; Adaptive Pricing must remain disabled. |
| + |
| +2. **Expiry and proof:** race payment against expiry with a retained URL; exercise concurrent/replayed keys, remembered errors, lost responses and key-retention expiry. Establish that successful expiry prevents later payment and that subsequent GETs supply the required identity and terminal fields. An expiry response alone must not establish refusal retirement. |
| + |
| +3. **Provider corrections and delayed payment:** verify visibility after metadata correction, complete-but-unpaid behavior, `no_payment_required`, and whether a completed session’s subscription association can change. Conflicting observations must produce durable disputes, including after retirement or consumption. |
| + |
| +4. **Refused purchases:** exercise hand-made subscriptions, multiple items, additional subscriptions and both webhook orders. Failed expiry must retain the intent and review; paid or otherwise financially unresolved cases must remain held. Cancellation must not be treated as proof that payments, invoices or refunds are settled. |
| + |
| +5. **Invoices and recovery:** create, recover, finalize and pay overage invoices in each currency. Verify exact line allocation, floors, taxes, discount exclusion, waivers and legacy EUR request compatibility. |
| + |
| +6. **Subscription lifecycle:** verify membership quantity changes and proration, renewal dating, cancellation, delayed events and customer-currency restrictions after an ended subscription. |
| + |
| +Separately, controlled local tests must cover the identified write boundaries, two consoles, completion during expiry, timeout starvation and late evidence after `keep` consumption. Stripe rehearsal cannot establish those database interleavings. |
| + |
| +**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 55d21a253..313ebf98d 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -384,3 +384,27 @@ What changed in behaviour: |
| Tests (`billingretirement_test.go`), each named for its interleaving: `TestRetireDoesNotReleaseAnIntentWhoseSessionIsStillPayable` (review eighteen's scenario: 503 while the expiry fails, held afterwards for the attached subscription, no URL offered, the listing's disposition); `TestRetireOnAProvenExpiryWithNothingAttached` (held while the session reads as another customer's; retired on proof; evidence and audit; exact replay with no provider read; a late subscription is one dispute however often delivered); `TestAPaymentBeforeTheRetireIsACompletionNotARetirement` (the payment recorded as the fate; retire refused; keep refused while unpaid; keep; an expiry proof after keep retires nothing; the purchase records); `TestThePassRetriesAFailedExpiryOnItsSchedule`; `TestAReviewFromBeforeTheProtocolIsHeld`; `TestTheDeleteBarrierHoldsARefusedIntent`; `TestEvidenceAgainstARecordedFateIsADispute`; `TestKeepIsHeldWhileAnotherSubscriptionIsAttached`. Eleven existing tests were reworked: those that retired a paid, contradicted or hand-made case now assert the hold; two that are about what follows a retirement (`TestRefusedPaidSessionReviewThroughTheWebhookAndItsResolution`, `TestARetiredSubscriptionSettlesBesideAReplacementIntent`) assert the hold and then continue from a retirement as a build from before the protocol left it (`retiredBeforeTheProtocol`), which a database can carry; three migration tests drop the barrier with the columns it names. |
| Not built, stated: `settled`; the adjudication of a dispute; `keep` for a never-bound session; the read-only subscription judgement before `keep`; a per-pass cursor across more refusals than the budget (the per-intent schedule spreads them); the buyer's copy under review (the existing pending answer is shown). |
| + |
| +## Review nineteen (reviewed `59666bab`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-multi-currency-review19.md`, `CODEX-FINDINGS-multi-currency-review19.md`. Both found the sequential form of review eighteen's defect closed (a retire now needs a provider read that proves the session expired, and the store's transaction enforces it) and the same family of gaps left: evidence was recorded at some call sites and not at others, and a refusal and its subscription were two transactions. Opus named one P2 and six P3s; Astra three P2s and two P3s. **Review eighteen's P2 stays recorded as open until a round accepts this code.** |
| + |
| +These are answered as one invariant, not as call sites patched one by one (the supervisor's instruction, and the lesson of rounds thirteen to eighteen): anything the provider shows about a refused purchase is classified and recorded by one store transaction that loads the purchase as it stands at that instant (standing, retired, or kept and consumed); a caller decides nothing from its own read and records before it answers; a refusal and its subscription are one transaction; nothing observed is dropped. The design's "Stage one as built" states it. |
| + |
| +| Finding | Disposition | Where | Counterexample test | |
| +| --- | --- | --- | --- | |
| +| Opus P2-1, Astra 1 (second half): a subscription on the purchase's own terms naming the refused intent was never attached (reconcile answered pending or identity first), so `keep` released the review beside it; a subscription with two items returned before anything was captured. | Fixed: reconcile hands every subscription that is not the account's recorded one to `noteSubscription` before any answer; under a refusal review it is attached unless it is the pinned session's own recorded subscription. A subscription with more or fewer than one item that names the bound standing intent goes through the refusal, not a bare rejection. | `billingservice.go` `reconcileBillingSubscription`; `billingretirement.go` `noteSubscription` | `TestAnOnTermsSubscriptionBesideTheSessionIsAttachedAndHoldsKeep`; `TestAStructurallyInvalidSubscriptionNamingTheIntentIsReviewedAndAttached` | |
| +| Astra 1 (first half), Opus P3-1 (both scenarios): the review and the attachment were two transactions (a crash, or another console resolving between them, found the review with nothing attached); and an attachment inserted for an already retired intent made every later delivery read as already known. | Fixed: `fileCheckoutRefusal` files the review and attaches the subscription in one transaction. An attachment is made only while the intent stands; for an intent retired on a proven expiry the subscription is a dispute, and for one kept and consumed any subscription but the kept one is a dispute. | `billingstore.go` `fileCheckoutRefusal`; `noteSubscription` | `TestTheRefusalAndItsSubscriptionAreOneTransaction` (a rolled-back filing leaves no review; a subscription seen after the retirement attaches nothing and is one dispute however often seen) | |
| +| Astra 2: the keep gate's fresh read could name another subscription than the one recorded with the completion, and the transaction compared against the stored one. | Fixed: the gate's read goes through `noteSessionObservation` before the resolution; another subscription on the complete session is a dispute, the gate refuses, and the store's transaction refuses on the dispute by itself. | `billingoperator.go` `billingKeepRefused` | `TestTheKeepGatesReadIsRecordedAsEvidence` | |
| +| Astra 3, Opus P3-2, Opus claim 1 and 2: a completion after a retirement on a proven expiry was settled with no dispute; a subscription naming an intent consumed after `keep` was a rejected row only; an identity contradiction read by an actor whose view of the purchase was stale was dropped. | Fixed: the binder hands the session in before its retired, no-intent and not-complete answers; `noteSessionObservation` classifies against the retirement record or the keep's archived evidence (the resolution row now carries the session and subscription it rested on); the classification no longer takes a state the caller read. | `billingservice.go` `bindBillingCheckoutFor`; `billingretirement.go` `noteSessionObservation`; `billingstore.go` (evidence columns) | `TestRetireOnAProvenExpiryWithNothingAttached` (the late completion: one dispute, however often delivered); `TestEvidenceAfterAKeepSurvivesTheIntent` (three cases); `TestAContradictionReadBesideARecordedFateIsNotDropped` | |
| +| Opus P3-4: a dispute after `keep` made the record abort on the delete barrier's raw error, retried for ever. | Fixed: the record's consumption names the dispute; a disputed kept intent is a conflict, and waits for a person. | `billingstore.go` `applyBillingStateDated` | `TestEvidenceAfterAKeepSurvivesTheIntent`, third case | |
| +| Astra 4: reconciliation that used up the pass's time left a due refusal unworked pass after pass. | Fixed: the refusals' turn runs under its own deadline whatever became of the reconciliation's; only the console stopping cancels it. | `billingpass.go` `billingPass` | `TestThePassWorksADueRefusalThoughItsOwnTimeIsSpent` | |
| +| Opus P3-3 against Astra's "closed" on the tie-break test. **Both were right about the code and they tested different mutants**: the test inserted `evt_tie_aaa` then `evt_tie_zzz` and wanted `zzz` first, which fails for `ORDER BY ..., id` ascending (Astra's counterfactual) and passes for `id DESC` (Opus's). | Fixed: three ids arriving in an order that is neither ascending nor descending (`b`, `c`, `a`); only arrival order gives `a`, `c`, `b`. | `TestTheRejectedListingBreaksTiesNewestFirst` | fails for `id` and for `id DESC` | |
| +| Opus P3-5: the dispute test's `keep` 409 and "ordinary expiry retirement" check could not fail; the pass test's "not reconciled meanwhile" was not asserted; `TestAResolutionDuringAReconciliation...` no longer exercised its title. | Fixed: the dispute test asserts the retire is held for the contradiction (its fate is expired with nothing attached, so only the dispute holds it) and calls `expireBillingCheckout`; the resolution test is renamed for what it now establishes (`TestAHeldResolutionDuringAReconciliationLeavesNothingRecorded`). The pass test's unasserted claim is withdrawn from this file. | tests | | |
| +| Opus P3-6: `held("retire")` took the never-created shortcut before asking what is attached. | Fixed: what is attached is asked first. The second edge (a resolved review from before the protocol filed again becomes protocol 1) exists only on unmerged builds and is stated. | `billingretirement.go` `held` | | |
| +| Astra 5, Opus claim 6: the site's fractional-floor test at `4026231` reads source text. | Fixed at site `01b62f7` (the renderer is a module the test calls); the review-eighteen row said `4026231` and is corrected by this row. | site | | |
| +| Claims: "one test per interleaving" (none orchestrates two consoles); "the door requests the expiry for retire" (it does for keep too); "a fate is written from a read" (the ordinary retirement writes it from its proof inside its own transaction). | Corrected: the design says what the tests are (state transitions and stand-ins for a second actor through the store or a provider hook; no test runs two console processes); the door's comment; the exception stated. | design, `billingoperator.go` | | |
| + |
| +Mutation checks, in a tree copy, twenty: the refusal filed without its subscription; reconcile answering before the subscription is evidence; an attachment after keep not reopening the review; a subscription naming a retired, and a kept and consumed, intent not a dispute; a retired session read back complete, a kept one read back expired, and a kept one naming another subscription not a dispute; an identity contradiction beside a recorded fate dropped; the keep gate's read not recorded; the record consuming a disputed kept intent; a two-item subscription a bare rejection; the refusals' turn dying with the pass's deadline; a cancelled pass still working refusals; the binder's retired and pending answers dropping what they read; ties by id in either direction; a standing complete session with another subscription; the session's own subscription attached like any other. All twenty fail the tests named above (two needed their tests corrected first: the record of a disputed kept intent was not reached, and a kept, consumed session naming another subscription had no case). |
| + |
| +Open and stated: the protocol holds and does not dispose (decisions 1 to 5); `keep` does not re-run reconcile's judgement of the subscription before accepting; no test runs two console processes against one database; a per-pass cursor across more refusals than the budget; everything that needs Stripe test mode (both lists, with Astra's additions for expiry races, provider corrections and refused purchases). |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review19.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review19.md| new file mode 100644 |
| index 000000000..cedc6076d |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review19.md |
| @@ -0,0 +1,178 @@ |
| +# Nineteenth review of `feat/carloku-multi-currency-20261001` at `59666bab` (base `12e70d95`; delta from `68f6ee23`) |
| + |
| +**Verdict: accept with fixes.** Stage one closes review eighteen's P2. A `retire` now needs a provider GET that proves the session expired, unpaid, owned and with nothing attached, and the store's transaction enforces this. However, one P2 remains in the protocol's coverage: |
| + |
| +- A subscription that names the refused intent, carries the purchase's own terms, and is not the session's own subscription is never recorded as attached. |
| +- So `keep` can release the review while that subscription keeps collecting. |
| + |
| +This is design review three's P2-1, closed only for subscriptions whose terms differ from the purchase. I found six P3s. |
| + |
| +The delta does not touch pricing, the page, the quote signature, the month trigger or the site. Those sections stand as rounds fifteen to eighteen traced them. |
| + |
| +Sources: `git show` only, plus the site at `4026231` (and `01b62f7` for one disposition). I ran nothing and modified nothing. |
| + |
| +## Review eighteen's findings |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1 / Astra 1 (P2)**: `retire` deleted the intent while its session was payable | **Closed.** The door requests the expiry and answers 503 while the expiry cannot be proven (`billingoperator.go:357-372`). `held("retire")` requires `Fate == expired` (`billingretirement.go:179-185`), read inside the resolution's transaction (`billingstore.go:972-982`). A hand-made subscription's case is held on its obligation (`:176`). | **No.** `TestRetireDoesNotReleaseAnIntentWhoseSessionIsStillPayable` gets 200 at `68f6ee23`. Without the door's `advanceRefusal` its first retire is 409, not 503. Without the obligations clause the second retire is 200. Without the start-path check the click returns the URL. | |
| +| **Opus P3-1** (nil billing untested) | **Closed.** | **No.** `TestKeepWithBillingOffAndAKeepDecidedDuringTheGate` dereferences nil without the branch. | |
| +| **Opus P3-2** (the tie-break was untested) | **Not closed.** `billingcurrency_test.go:4252` inserts `evt_tie_aaa` and then `evt_tie_zzz`, so the ids sort the *same* way as arrival. | **Yes, for `ORDER BY received_at DESC, id DESC`**: that order also puts `zzz` first. It fails only for ascending `id`. See P3-3. | |
| +| **Opus claims 1–4** | 1 is corrected in the dispositions. 2 is corrected: both sites now use `advanceRefusal`, and the `ownSession` doc names it. 3 is withdrawn, and the pass now retries. 4 is closed at site **`01b62f7`**: the test calls `moneyText('$', 120.5, 'USD')` and asserts that it throws. That fix is not at `4026231`, the commit the brief names. | n/a | |
| +| **Astra 2 (P3)**: concurrent identical `keep` | **Closed.** On one console the door now holds `locks.lock(acct.Sqid,"billing")`, so a second request waits and then meets `decided()`. Across consoles, the second read plus the store's exact comparison decide. | **No.** Without the post-gate `decided()` the test's hooked resolution answers the gate's 409. | |
| +| **Astra 3 (P3)**: a retired completion beside a replacement lost its account | **Closed.** The retired check runs before any standing intent is read (`billingservice.go:1180`). | **No.** `evt_again`'s rejected row would carry account 0 (`billingcurrency_test.go:3076`). | |
| +| **Astra claims** | The expiry-order comments are rewritten. The "next read" retry is now true through the pass. The fractional-floor claim is closed at `01b62f7`. | n/a | |
| + |
| +**Where they disagreed (round eighteen):** only on the verdict, not on any severity. Both reported the same P2, and Astra's **reject** was the right reading. The code at `68f6ee23` let a recorded "refunded" resolution coexist with a payable URL, and both orders of review and expiry had been shown wrong, which a fix with fixes could not answer. |
| + |
| +## Design review three's four P2s |
| + |
| +| P2 | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| 1. `keep` with another subscription attached | **Closed for a recorded obligation** (`billingretirement.go:190-193`). **Open for an obligation that is never recorded**: see P2-1 below. | **No.** In `TestKeepIsHeldWhileAnotherSubscriptionIsAttached`, without the loop the door's gate passes (own, paid, `sub_checkout`), the store keeps, and the answer is 200. | |
| +| 2. Cancellation standing in for settlement under `retire` | **Closed.** Any obligation holds `retire` (`:176`). | **No.** In `TestAHandMadeSubscriptionExpiresTheOpenBoundSession` the fate is expired, so without the clause the retire is 200. | |
| +| 3. Migrated reviews with no recorded obligations | **Closed for reviews open at upgrade.** The column defaults to 0 (`:81`), and `protocol=1` is set only when a review is newly filed or reopened (`billingstore.go:1078-1084`). | **No**, but the test sets `protocol=0` by hand rather than migrating a table without the column. A *resolved* review from before the protocol that is filed again becomes protocol 1 without its earlier obligations (P3-6). | |
| +| 4. Late contradictions after the intent is gone | **Partly closed.** A subscription naming an intent retired on a proven expiry becomes one dispute (`noteLateSubscription`, tested). These cases are not: a late **completion** of that session (P3-2), and any subscription naming a **consumed** (kept) intent (P2-1). | The late-subscription test fails without `noteLateSubscription`. Nothing tests the other two cases. | |
| + |
| +## Findings |
| + |
| +### P2-1. A subscription on the purchase's own terms that names the refused intent is never attached, so `keep` releases the review beside it |
| + |
| +- **Where:** `billingservice.go:988-997`. |
| + - The match branch answers `errBillingPending` while the bound session is open (`:988`). |
| + - It answers a bare `errBillingIdentity` when `session.SubscriptionID != id` (`:996`). |
| + - Neither path calls `addRefusalObligation`; only `refuse()` does (`:892`). |
| + - After consumption, `noteLateSubscription` is reached only for *retired* intents (`:857`). |
| +- **Scenario:** |
| + 1. Intent I's bound session S reads back as another customer's, and its expiry fails. The review is open. |
| + 2. Staff, trying to unstick the purchase, create subscription H at Stripe with I's exact plan, band, terms, quantity and price, naming I. |
| + 3. H's event finds S open and gets 503 (retried); nothing is attached. |
| + 4. The buyer pays S, creating C. The fate is `complete`, and the obligations are empty. |
| + 5. The operator keeps. C records and I is consumed. |
| + 6. H's redeliveries meet `state.Subscription=C`, are answered bare `errBillingIdentity` and 204, and land in the rejected list only. H bills the customer a second time with no review or dispute. |
| +- **The same happens** if H arrives after S completes but before C records. |
| +- **Smallest fix:** |
| + - In reconcile, for a subscription whose `checkout_intent` is the standing intent and which has a refusal review (open or kept), and which is not the pinned session's recorded `session_subscription`: attach it, in the filing's transaction, before any pending or identity answer. Under a resolved `keep`, reopen the review. |
| + - For a subscription naming an intent consumed after `keep`, file the dispute as `noteLateSubscription` does, keyed by the keep resolution's evidence. |
| + - Test: an on-terms H delivered while S is open, then S completes, then `keep` must be 409 naming H. |
| + |
| +### P3-1. The obligation is a separate transaction from the filing and is conditioned on no live state |
| + |
| +- **Where:** |
| + - `billingservice.go:886-894`: the filing, then `addRefusalObligation`. |
| + - `billingretirement.go:260-262`: the insert requires only that *a* review row exists. |
| + - `:407-412`: `noteLateSubscription` returns silently once an obligation row exists. |
| +- **Scenario A (crash):** |
| + 1. `refuse()` commits the review and dies before the obligation. |
| + 2. After restart the operator's `retire` proves the expiry and passes with no obligations. |
| + 3. H's redelivery becomes a dispute, so the hold was released and then re-raised. |
| +- **Scenario B (two consoles):** |
| + 1. Console B's `retire` commits between console A's filing and A's obligation insert. |
| + 2. A then inserts the obligation for the retired intent, because the review row still exists. |
| + 3. Every later H event finds `known > 0` and files nothing. That is silent. |
| +- **Fix:** |
| + - Attach inside `fileCheckoutRefusedReview`'s transaction, conditioned on the intent standing and the review open. |
| + - In `noteLateSubscription`, treat an obligation with `first_seen_at > retired_at` as unknown. |
| + |
| +### P3-2. A late completion of a session retired on a proven expiry is settled with no dispute |
| + |
| +- **Where:** |
| + - `billingservice.go:1180-1184` returns before any evidence is noted. |
| + - `noteSessionProof`'s branches for an intent that no longer stands (`billingretirement.go:327-354`) have no production caller that passes a retired or consumed intent. |
| +- **Scenario:** Stripe answers expired, the operator retires, and the session then completes. The completion event is a rejected row only. It is caught only if the subscription's own event reaches `noteLateSubscription`. |
| +- **Fix:** in the binder's retired branch, compare with the retirement record and call `disputeSessionTx`, then test it. Otherwise delete the unreachable branches and the design's claim. |
| + |
| +### P3-3. The tie-break test cannot fail for `id DESC` |
| + |
| +- **Where:** `billingcurrency_test.go:4252`. The comment says "the ids sort the other way from the order they arrive in", which is false. |
| +- **Fix:** insert `evt_tie_zzz` first, then `evt_tie_aaa`, and expect `aaa` first. |
| + |
| +### P3-4. A dispute after `keep` makes the record abort on the trigger's raw error, retried forever |
| + |
| +- **Where:** the consume `DELETE` (`billingstore.go:782-784`) does not name `fate_disputed_at`. The barrier (`billingretirement.go:118`) raises `ABORT`. |
| +- **Scenario:** |
| + 1. `keep` is resolved. |
| + 2. A later read contradicts the fate, which sets `fate_disputed_at`. |
| + 3. Every subscription event and every pass returns a raw SQLite error (503), until Stripe gives up and from then on in the pass. |
| +- **Fix:** add `AND fate_disputed_at=0` to the consume clause, and answer a clean hold the webhook settles. |
| + |
| +### P3-5. Tests that do not establish what they claim |
| + |
| +- `TestEvidenceAgainstARecordedFateIsADispute`: |
| + - The `keep` 409 (`:504`) would be 409 without the dispute, because the fate is `expired`, not `complete`. |
| + - "The ordinary expiry retirement does not take it" (`:513`) only reads the column. It never calls `expireBillingCheckout`. |
| +- `TestThePassRetriesAFailedExpiryOnItsSchedule`: "not reconciled as a pending purchase meanwhile" is not asserted. Removing the pass's skip changes no count it checks. |
| +- `TestAResolutionDuringAReconciliationLeavesNothingRecorded` (`:3815`): the resolution is now always held, so no resolution commits during a reconciliation. The test's title is no longer exercised. |
| + |
| +### P3-6. Ordering and migration edges in `held`, both theoretical |
| + |
| +- `held("retire")` takes the never-created shortcut (`:173`) before the obligations check. Put the obligations check first. |
| +- A resolved review from before the protocol that is filed again gets `protocol=1` with its earlier attachments unrecorded. This exists only on unmerged builds, where billing is off. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **Design, "Interleavings this closes":** "a completion after `expired` is a dispute on the record". The binder settles it (P3-2). |
| +2. **Design, revision 4:** "A subscription that names a retired **or consumed** intent … is filed the same way." Only retired intents are handled (P2-1). |
| +3. **Design, "a fate is written … from a read":** `retireIntent` writes `expired` from the proof it is handed, which can be an expiry answer (`billingstore.go:468`). It is only observable inside that transaction and by the trigger, so state the exception. |
| +4. **`billingoperator.go:363-366`:** "for retire, its expiry is requested here". `advanceRefusal` runs for `keep` too, and expires an open session. That is harmless, because the pass would do the same. |
| +5. **Dispositions, stage one:** the pass test's "not reconciled meanwhile" and the dispute test's "held whatever outcome" (P3-5). Review eighteen's tie-break row (P3-3). |
| +6. **Dispositions, review eighteen:** Opus claim 4 is fixed at site `01b62f7`, not at `4026231`. |
| + |
| +## Requested sections, stage one |
| + |
| +- **DDD.** The `retire` deletion is now gated on a GET proof of expiry, nothing attached, protocol 1 and no dispute, read in the resolution's transaction. Record consumption is gated on the open review and, through the barrier, on a `keep` with fate complete. `retireIntent` refuses fate complete or disputed. |
| + - **Process death:** each step is a row, except filing plus obligation (P3-1). |
| + - **Duplicate webhooks:** they are no-ops (`INSERT OR IGNORE`, conditional fate). |
| + - **Two consoles:** the store decides, except P3-1. |
| + - **Late or remembered provider answers:** a fate comes only from a GET. |
| + - **Older binary:** the barrier aborts its unproven retire, and a review it files keeps `protocol=0`, so it is held. |
| + - **What is still released:** a case with an unattached on-terms subscription (P2-1). |
| +- **EEE.** `sessionProof` (`:448-460`) checks the exact pinned id, mode, intent, both account fields and the customer. Expired requires no subscription and not paid. Complete is any `complete`. |
| + - A fate is written once, with the conditional `session_fate=''`. |
| + - A contradiction (another fate, another subscription id, or reading back as nobody's or open after a fate) is a dispute keyed by session and wording. |
| + - Dropped, by design: a contradicted identity before any fate (the review already exists). |
| + - Late evidence for a gone intent: see P3-2. |
| +- **FFF.** The facts the resolution reads are monotonic: fate set once, obligations only added, dispute only set, protocol changed only on a fresh filing. Retire and keep cannot pass a held case except through P2-1 and P3-1. Exact replay is checked before `held` (`billingstore.go:958-970`). |
| +- **GGG.** The door: |
| + - It takes the account's billing lock only when billing is on and the occurrence is undecided. It is not re-entered (`advanceRefusal` and `billingKeepRefused` take no lock). |
| + - It answers 503 only when provider work fails, and 409 `held: …` from the store. |
| + - With billing off: `retire` of a never-created intent is accepted, everything else is 409, and an exact replay is still accepted. |
| +- **HHH.** The pass schedule: |
| + - It retries at 1, 5 and 15 minutes, then hourly, with three refusals per pass, after all other work. A due check consumes no budget. |
| + - Every path that does not record a fate schedules the next try. |
| + - It expires only `pinned`, and only when the intent is bound. |
| + - A failed expiry leads to a retry. A completed session leads to fate `complete`, held. |
| + - An identity-contradicted session is read hourly forever, because it can never get a fate. That is cheap but unbounded; say so or cap it. |
| + - Refusals run last, so a pass whose subscription batch exhausts its context starves them. |
| +- **III.** Under an open review the start path returns `errBillingUnderReview` before any recovery, resume or new intent (`billingservice.go:619-626`). A pass's `ownBoundSession` filing between that check and `return intent.URL` (`:722`) can still hand out one URL; the pass then expires the session. An unbound intent whose review names no session is reconciled to find it. It is never retired or recorded under the review. |
| +- **JJJ.** The barrier stops every delete of an intent with any refusal review, except: |
| + - a never-created intent; |
| + - fate expired and undisputed; |
| + - fate complete and undisputed under a current `keep`. |
| + |
| + Against the code's deletes: |
| + - `discardUnreadyIntent`, `retireIntent` and the resolution pass. |
| + - Consumption fails noisily on a dispute (P3-4). |
| + |
| + It does not stop updates to the fate or deletion of review rows; none exist in the code. No migration rebuilds or drops a referenced column. Three tests drop the barrier first. |
| +- **KKK.** `retiredBeforeTheProtocol` writes what `68f6ee23`'s retire wrote: resolution `retire` with no evidence, a retirement with no fate, quantity events closed, the intent deleted. It differs in one respect: the review row keeps `protocol=1`, which no reader consults for a gone intent. Both tests still establish legacy settlement and attribution. Neither covers the stage-one equivalent for a completion (P3-2). |
| +- **LLL.** |
| + - These fail without their code: payable-retire, proven-expiry, payment-before-retire, delete barrier, keep-held, and the parts of the schedule they assert. |
| + - Vacuous parts are in P3-5. |
| + - Before-protocol tests a hand-set column rather than a migration. |
| + |
| +## Verify in Stripe test mode before enabling |
| + |
| +All earlier lists stand. Add the following: |
| + |
| +1. Expiry under the fixed key while the buyer pays. The GET after it must show `complete` and never `expired`. |
| +2. A GET immediately after a successful expiry must show `expired`. |
| +3. A replay of the key after its retention period ends. |
| +4. Proof fields on GET after a metadata edit and after a correction. |
| +5. A subscription created by hand on the exact price naming the intent, both while the session is open and after it completes. After the P2-1 fix, `keep` must be 409. |
| +6. `complete` with `payment_status=unpaid` (delayed methods): `keep` is 409 until the payment is paid. |
| +7. A completion and a subscription arriving after a retire on proven expiry: one dispute each. |
| +8. A kill between the filing and the obligation. |
| +9. Two consoles resolving and reconciling at once. |
| + |
| +**Verdict: accept with fixes:** P2-1, P3-1 to P3-6 and claims 1–6. |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index bb1b954a9..f86f2cf12 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -13,6 +13,7 @@ import ( |
| "net/http" |
| "net/http/httptest" |
| "net/url" |
| + "reflect" |
| "regexp" |
| "strings" |
| "testing" |
| @@ -3808,11 +3809,14 @@ func TestAHandMadeSubscriptionExpiresTheOpenBoundSession(t *testing.T) { |
| } |
| // A reconciliation paused between its checks and its record (the provider |
| -// being read) while the operator resolves the intent's review records |
| -// nothing: the record and the consumption of the exact standing intent are |
| -// one transaction, and an intent retired under it fails the record. |
| -// (Review fourteen, Astra 1, P2.) |
| -func TestAResolutionDuringAReconciliationLeavesNothingRecorded(t *testing.T) { |
| +// being read) while the operator tries to resolve the intent's review |
| +// records nothing. When this test was written (review fourteen, Astra 1) |
| +// the resolution retired the intent and the record's own transaction had |
| +// to refuse; under the retirement protocol the resolution of this case |
| +// is itself held, so no resolution commits during the reconciliation, and |
| +// what the test now establishes is that the record is refused under the |
| +// review that stays open. |
| +func TestAHeldResolutionDuringAReconciliationLeavesNothingRecorded(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 == "" { |
| @@ -4248,8 +4252,11 @@ func TestAnOlderResolutionRowReadsAsRetire(t *testing.T) { |
| // was untested.) |
| func TestTheRejectedListingBreaksTiesNewestFirst(t *testing.T) { |
| fx, f, acct := newCurrencyFixture(t) |
| - // The ids sort the other way from the order they arrive in. |
| - for _, id := range []string{"evt_tie_aaa", "evt_tie_zzz"} { |
| + // Three rows in one second, arriving in an order that is neither |
| + // ascending nor descending by id: only arrival order gives c, a, b |
| + // reversed (b, a, c... newest first: the last to arrive first). |
| + arrival := []string{"evt_tie_b", "evt_tie_c", "evt_tie_a"} |
| + for _, id := range arrival { |
| if err := fx.st.rejectBillingEvent(id, "customer.subscription.updated", "sub_tie", "sub_tie", acct.ID, f.now); err != nil { |
| t.Fatal(err) |
| } |
| @@ -4257,13 +4264,16 @@ func TestTheRejectedListingBreaksTiesNewestFirst(t *testing.T) { |
| token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-tie") |
| status, body := getJSON(t, fx.ts.URL+"/api/billing/rejected", token) |
| rejected, _ := body["rejected"].([]any) |
| - if status != http.StatusOK || len(rejected) != 2 { |
| + if status != http.StatusOK || len(rejected) != 3 { |
| t.Fatalf("%d rows, status %d", len(rejected), status) |
| } |
| - first, _ := rejected[0].(map[string]any) |
| - second, _ := rejected[1].(map[string]any) |
| - if first["id"] != "evt_tie_zzz" || second["id"] != "evt_tie_aaa" { |
| - t.Fatalf("order %v, %v; want the later arrival first", first["id"], second["id"]) |
| + var got []string |
| + for _, r := range rejected { |
| + got = append(got, r.(map[string]any)["id"].(string)) |
| + } |
| + // By id ascending it would be a, b, c; by id descending c, b, a. |
| + if want := []string{"evt_tie_a", "evt_tie_c", "evt_tie_b"}; !reflect.DeepEqual(got, want) { |
| + t.Fatalf("order %v; want %v, the later arrival first", got, want) |
| } |
| } |
diff --git a/internal/console/billingoperator.go b/internal/console/billingoperator.go| index 3c25b1e05..72876cf91 100644 |
| --- a/internal/console/billingoperator.go |
| +++ b/internal/console/billingoperator.go |
| @@ -260,6 +260,19 @@ func (s *Server) billingKeepRefused(ctx context.Context, acct *Account, intentID |
| if session.PaymentStatus != "paid" && session.PaymentStatus != "no_payment_required" { |
| return "the session's payment is " + strconv.Quote(session.PaymentStatus), nil |
| } |
| + // This read is evidence like any other: recorded through the store's |
| + // one classification before the resolution runs, so a session that |
| + // now names another subscription than the one recorded is a dispute, |
| + // and the resolution's transaction, which reads the recorded state, |
| + // refuses. |
| + if err := s.noteSessionEvidence(intent.ID, acct, session, time.Now()); err != nil { |
| + return "", err |
| + } |
| + if st, err := s.store.refusalState(acct.ID, intent.ID); err != nil { |
| + return "", err |
| + } else if st.Disputed != 0 { |
| + return "what the provider now shows of the session contradicts what was recorded", nil |
| + } |
| return "", nil |
| } |
| @@ -361,10 +374,10 @@ func (s *Server) handleAPIBillingReviewResolve(w http.ResponseWriter, r *http.Re |
| return |
| } |
| if ierr == nil && intent.ID == req.Subscription { |
| - // Work the session toward a proven fate now: for |
| - // retire, its expiry is requested here if it is still |
| - // open, and only what the provider then shows is |
| - // recorded. |
| + // Work the session toward a proven fate now, whatever |
| + // the outcome asked for: its expiry is requested here |
| + // if it is still open (as the pass would), and only |
| + // what the provider then shows is recorded. |
| if aerr := s.advanceRefusal(r.Context(), intent, acct, time.Now()); aerr != nil && !decided() { |
| log.Printf("console: api billing review resolve: %s", billingDiagnostic(aerr)) |
| jsonError(w, http.StatusServiceUnavailable, "the session's state at the provider could not be established. the review stays open. try again") |
diff --git a/internal/console/billingpass.go b/internal/console/billingpass.go| index e80ee6823..5567bb635 100644 |
| --- a/internal/console/billingpass.go |
| +++ b/internal/console/billingpass.go |
| @@ -118,39 +118,65 @@ func (s *Store) setBillingPassCursor(last int64) error { |
| } |
| // billingPass processes pending purchases first, then a bounded, fairly |
| -// ordered batch of stored subscriptions. It never creates a payment. |
| +// ordered batch of stored subscriptions, then, in a turn of their own, the |
| +// refused intents whose expiry is due. It never creates a payment. |
| +// |
| +// The refusals' turn does not depend on the rest: it runs whether the |
| +// reconciliation finished, failed or ran out of its time, under its own |
| +// short deadline, so slow reconciliation cannot leave a refused session |
| +// payable pass after pass (and a slow expiry, bounded by its budget and |
| +// its own deadline, cannot starve reconciliation). |
| func (s *Server) billingPass(ctx context.Context, now time.Time) error { |
| if s.billing == nil { |
| return nil |
| } |
| var failures []error |
| - |
| - // 1. Pending checkout intents first: a purchase whose response was lost |
| - // must be bound before any background subscription poll can time the |
| - // pass out. |
| intents, err := s.store.billingPendingCheckouts() |
| if err != nil { |
| failures = append(failures, err) |
| } |
| // An intent under a refusal review is not reconciled as a pending |
| - // purchase: nothing binds, records or retires under the review. It is |
| - // worked toward a proven fate last, within its own budget |
| - // (billingPassRefusals). |
| - var refused []billingIntent |
| + // purchase: nothing binds, records or retires under the review. (One |
| + // 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. |
| + var pending, refused []billingIntent |
| for _, intent := range intents { |
| - if ctx.Err() != nil { |
| - return errors.Join(append(failures, ctx.Err())...) |
| - } |
| - // (One 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.) |
| - if st, err := s.store.refusalState(intent.AccountID, intent.ID); err != nil { |
| + st, err := s.store.refusalState(intent.AccountID, intent.ID) |
| + switch { |
| + case err != nil: |
| failures = append(failures, err) |
| - continue |
| - } else if st.Open && st.pinned() != "" { |
| + case st.Open && st.pinned() != "": |
| refused = append(refused, intent) |
| - continue |
| + default: |
| + pending = append(pending, intent) |
| + } |
| + } |
| + if err := s.billingPassReconcile(ctx, pending, now); err != nil { |
| + failures = append(failures, err) |
| + } |
| + // The caller's cancellation (the console stopping) stops this too; its |
| + // deadline having passed does not. |
| + if !errors.Is(ctx.Err(), context.Canceled) { |
| + turn, cancel := context.WithTimeout(context.WithoutCancel(ctx), billingPassRefusalTimeout) |
| + failures = append(failures, s.billingPassRefusals(turn, refused, now)...) |
| + cancel() |
| + } |
| + return errors.Join(failures...) |
| +} |
| + |
| +// billingPassReconcile is the pass's reconciliation: pending checkout |
| +// intents, then a bounded batch of stored subscriptions. |
| +func (s *Server) billingPassReconcile(ctx context.Context, intents []billingIntent, now time.Time) error { |
| + var failures []error |
| + |
| + // 1. Pending checkout intents first: a purchase whose response was lost |
| + // must be bound before any background subscription poll can time the |
| + // pass out. |
| + for _, intent := range intents { |
| + if ctx.Err() != nil { |
| + return errors.Join(append(failures, ctx.Err())...) |
| } |
| if err := s.reconcilePendingIntent(ctx, intent, now); err != nil { |
| failures = append(failures, err) |
| @@ -185,8 +211,6 @@ func (s *Server) billingPass(ctx context.Context, now time.Time) error { |
| if err := s.store.setBillingPassCursor(last); err != nil { |
| failures = append(failures, err) |
| } |
| - // 3. Refused intents, last and bounded. |
| - failures = append(failures, s.billingPassRefusals(ctx, refused, now)...) |
| return errors.Join(failures...) |
| } |
diff --git a/internal/console/billingretirement.go b/internal/console/billingretirement.go| index 2f5e0975b..768dbf4de 100644 |
| --- a/internal/console/billingretirement.go |
| +++ b/internal/console/billingretirement.go |
| @@ -36,6 +36,19 @@ import ( |
| // kept, a case a subscription made outside checkout is attached to, a |
| // case the provider contradicted itself about, and every review opened |
| // before this protocol. What disposes of those is not decided. |
| +// |
| +// The evidence invariant. Anything the provider shows about a refused |
| +// purchase (its pinned session, or any subscription naming its intent) is |
| +// classified and recorded by one store transaction that loads the |
| +// purchase as it is at that instant: standing, retired, or kept and |
| +// consumed (noteSessionObservation, noteSubscription). The caller decides |
| +// nothing from a read of its own, and records before it answers or |
| +// acknowledges what carried the evidence. A refusal and the subscription |
| +// that caused it are one transaction (fileCheckoutRefusal). Nothing |
| +// observed is dropped: it becomes the session's fate, an attached |
| +// subscription, or a dispute. A crash or another console between any two |
| +// of these transactions leaves each one whole, and the next delivery or |
| +// pass meets the same classification. |
| const ( |
| sessionFateExpired = "expired" |
| @@ -49,6 +62,9 @@ const ( |
| // on, after everything else: slow expiries of old refusals cannot |
| // starve reconciliation. |
| billingPassRefusalBudget = 3 |
| + // billingPassRefusalTimeout is the refusals' own turn in a pass, |
| + // whatever became of the reconciliation's time. |
| + billingPassRefusalTimeout = 45 * time.Second |
| ) |
| // errBillingUnderReview: the account's purchase is under a refusal review. |
| @@ -83,6 +99,11 @@ func (s *Store) migrateBillingRetirement() error { |
| // resolution from before this protocol, which is never |
| // reinterpreted. |
| {"billing_review_resolutions", "evidence", `TEXT NOT NULL DEFAULT ''`}, |
| + // The session and subscription a resolution rested on, so |
| + // evidence that arrives after a kept intent is consumed can be |
| + // compared with what was kept. |
| + {"billing_review_resolutions", "evidence_session", `TEXT NOT NULL DEFAULT ''`}, |
| + {"billing_review_resolutions", "evidence_subscription", `TEXT NOT NULL DEFAULT ''`}, |
| {"billing_retired_intents", "session_fate", `TEXT NOT NULL DEFAULT ''`}, |
| } { |
| if err := s.addColumnIfMissing(c[0], c[1], c[2]); err != nil { |
| @@ -135,6 +156,7 @@ type refusalState struct { |
| Protocol int64 |
| Reference string // the review's session |
| SessionID string // the intent's bound session |
| + Customer string // the intent's customer, once it has one |
| CustomerReady bool |
| Fate string |
| FateAt, Disputed int64 |
| @@ -170,12 +192,12 @@ func (st refusalState) held(outcome string) string { |
| } |
| switch outcome { |
| case "retire": |
| - if st.SessionID == "" && !st.CustomerReady && st.Reference == "" { |
| - return "" |
| - } |
| 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 "" |
| + } |
| switch st.Fate { |
| case sessionFateExpired: |
| return "" |
| @@ -212,9 +234,9 @@ type billingQueryer interface { |
| func refusalStateQ(q billingQueryer, accountID int64, intentID string) (refusalState, error) { |
| var st refusalState |
| - err := q.QueryRow(`SELECT session_id,customer_ready,session_fate,session_fate_at,fate_disputed_at,session_payment_status,session_subscription,expiry_next_try_at,expiry_tries |
| + err := q.QueryRow(`SELECT session_id,customer,customer_ready,session_fate,session_fate_at,fate_disputed_at,session_payment_status,session_subscription,expiry_next_try_at,expiry_tries |
| FROM billing_checkout_intents WHERE account_id=? AND id=?`, accountID, intentID). |
| - Scan(&st.SessionID, &st.CustomerReady, &st.Fate, &st.FateAt, &st.Disputed, &st.PaymentStatus, &st.SessionSubscription, &st.NextTry, &st.Tries) |
| + Scan(&st.SessionID, &st.Customer, &st.CustomerReady, &st.Fate, &st.FateAt, &st.Disputed, &st.PaymentStatus, &st.SessionSubscription, &st.NextTry, &st.Tries) |
| switch { |
| case errors.Is(err, sql.ErrNoRows): |
| case err != nil: |
| @@ -251,18 +273,6 @@ func (s *Store) refusalState(accountID int64, intentID string) (refusalState, er |
| return refusalStateQ(s.db, accountID, intentID) |
| } |
| -// addRefusalObligation records a subscription attached to a refused |
| -// intent, while that intent's refusal review exists. |
| -func (s *Store) addRefusalObligation(accountID int64, intentID, subscription string, now time.Time) error { |
| - if intentID == "" || subscription == "" { |
| - return nil |
| - } |
| - _, err := s.db.Exec(`INSERT OR IGNORE INTO billing_review_obligations(account_id,intent_id,subscription,first_seen_at) |
| - SELECT ?,?,?,? WHERE EXISTS (SELECT 1 FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=?)`, |
| - accountID, intentID, subscription, now.Unix(), accountID, intentID, billingCheckoutRefusedReason) |
| - return err |
| -} |
| - |
| // disputeSessionTx records one contradiction about a pinned session. The |
| // first time it is seen it files the account's dispute review and marks |
| // the intent, if it still stands; seen again, it changes nothing but the |
| @@ -290,29 +300,36 @@ func disputeSessionTx(tx *sql.Tx, accountID int64, sessionID, intentID, detail s |
| return fileBillingReviewTx(tx, accountID, sessionID, billingFateDisputedReason, now) |
| } |
| -// disputeSession is disputeSessionTx in its own transaction. |
| -func (s *Store) disputeSession(accountID int64, sessionID, intentID, detail string, now time.Time) error { |
| - tx, err := s.db.Begin() |
| - if err != nil { |
| - return err |
| - } |
| - defer tx.Rollback() |
| - if err := disputeSessionTx(tx, accountID, sessionID, intentID, detail, now); err != nil { |
| - return err |
| - } |
| - return tx.Commit() |
| +// sessionObservation is one provider answer about a session, as the |
| +// caller read it. Identity says the answer names this mode, the intent and |
| +// the account in both places; the session's id and its customer are |
| +// compared in the store, against the purchase as it stands there. |
| +type sessionObservation struct { |
| + SessionID string |
| + Identity bool |
| + Customer string |
| + Fate string // expired (unpaid, no subscription), complete, or "" for anything else |
| + Status string |
| + PaymentStatus string |
| + Subscription string |
| } |
| -// noteSessionProof records what a provider answer proves about the session |
| -// a refused intent is pinned to. A fate is written once and never changed: |
| -// a later proof of the other fate, or of another subscription on a |
| -// complete session, is a dispute. Proof about an intent that no longer |
| -// stands is compared with what its retirement or its keep rested on, and a |
| -// contradiction of that is a dispute too. Nothing is recorded for an |
| -// intent with no refusal review, or for a session that is not the pinned |
| -// one. |
| -func (s *Store) noteSessionProof(accountID int64, intentID, sessionID, fate, payment, subscription string, now time.Time) error { |
| - if intentID == "" || sessionID == "" || (fate != sessionFateExpired && fate != sessionFateComplete) { |
| +// noteSessionObservation classifies and records one provider answer about |
| +// the session a refused purchase is pinned to, in one transaction that |
| +// loads the purchase as it is now. |
| +// |
| +// The intent stands, under a refusal review, pinned to this session: with |
| +// full identity, a terminal status is recorded as the fate if none is |
| +// recorded, and is a dispute if it contradicts the one that is (the other |
| +// fate; another subscription on a complete session); an answer without |
| +// identity, or one that reads open, is a dispute once a fate is recorded |
| +// and nothing before. The intent was retired on a proven expiry: any |
| +// answer but an expired one with identity is a dispute. Its purchase was |
| +// kept and recorded: any answer but a complete one with identity and the |
| +// kept subscription is a dispute. Anything else (no refusal; another |
| +// session; a retirement from before the protocol) records nothing. |
| +func (s *Store) noteSessionObservation(accountID int64, intentID string, o sessionObservation, now time.Time) error { |
| + if intentID == "" || o.SessionID == "" { |
| return nil |
| } |
| tx, err := s.db.Begin() |
| @@ -324,77 +341,98 @@ func (s *Store) noteSessionProof(accountID int64, intentID, sessionID, fate, pay |
| if err != nil { |
| return err |
| } |
| - if !st.Stands { |
| - // Retired: the archived fate. Consumed after a keep: the kept |
| - // resolution's evidence names the session as complete. |
| - var archivedSession, archivedFate string |
| - err := tx.QueryRow(`SELECT session_id,session_fate FROM billing_retired_intents WHERE account_id=? AND intent_id=?`, accountID, intentID).Scan(&archivedSession, &archivedFate) |
| + read := o.Status |
| + if read == "" { |
| + read = "with no status" |
| + } |
| + if st.Stands { |
| + if !st.Exists || st.pinned() != o.SessionID { |
| + return nil |
| + } |
| + identity := o.Identity && (st.Customer == "" || o.Customer == st.Customer) |
| switch { |
| - case err == nil: |
| - if archivedSession == sessionID && archivedFate == sessionFateExpired && fate == sessionFateComplete { |
| - if err := disputeSessionTx(tx, accountID, sessionID, intentID, "read back complete after its intent was retired on a proven expiry", now); err != nil { |
| - return err |
| - } |
| + case !identity: |
| + if st.Fate == "" { |
| + return nil |
| } |
| - case errors.Is(err, sql.ErrNoRows): |
| - if st.Exists && st.Reference == sessionID && fate == sessionFateExpired { |
| - var kept int |
| - if err := tx.QueryRow(`SELECT count(*) FROM billing_review_resolutions WHERE account_id=? AND subscription=? AND reason=? AND outcome='keep'`, accountID, intentID, billingCheckoutRefusedReason).Scan(&kept); err != nil { |
| - return err |
| - } |
| - if kept > 0 { |
| - if err := disputeSessionTx(tx, accountID, sessionID, intentID, "read back expired after its purchase was kept as complete", now); err != nil { |
| - return err |
| - } |
| - } |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "read back as another account's, customer's or intent's after it was recorded "+st.Fate, now) |
| + case o.Fate == "": |
| + if st.Fate == "" { |
| + return nil |
| } |
| - default: |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "read back "+read+" after it was recorded "+st.Fate, now) |
| + case st.Fate == "": |
| + _, err = tx.Exec(`UPDATE billing_checkout_intents SET session_fate=?,session_fate_at=?,session_payment_status=?,session_subscription=? WHERE account_id=? AND id=? AND session_fate=''`, |
| + o.Fate, max(now.Unix(), 1), o.PaymentStatus, o.Subscription, accountID, intentID) |
| + case st.Fate != o.Fate: |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "read back "+o.Fate+" after it was recorded "+st.Fate, now) |
| + case o.Fate == sessionFateComplete && st.SessionSubscription != "" && o.Subscription != "" && o.Subscription != st.SessionSubscription: |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "its subscription read back as "+o.Subscription+" after it was recorded as "+st.SessionSubscription, now) |
| + case o.Fate == sessionFateComplete: |
| + // The terminal status stands; what was paid and for which |
| + // subscription may become known later. |
| + _, err = tx.Exec(`UPDATE billing_checkout_intents SET session_payment_status=?,session_subscription=CASE WHEN session_subscription='' THEN ? ELSE session_subscription END WHERE account_id=? AND id=?`, |
| + o.PaymentStatus, o.Subscription, accountID, intentID) |
| + } |
| + if err != nil { |
| return err |
| } |
| return tx.Commit() |
| } |
| - if !st.Exists || st.pinned() != sessionID { |
| - return nil |
| - } |
| + // The intent no longer stands: what it was disposed on. |
| + var archivedSession, archivedFate string |
| + err = tx.QueryRow(`SELECT session_id,session_fate FROM billing_retired_intents WHERE account_id=? AND intent_id=?`, accountID, intentID).Scan(&archivedSession, &archivedFate) |
| switch { |
| - case st.Fate == "": |
| - if _, err := tx.Exec(`UPDATE billing_checkout_intents SET session_fate=?,session_fate_at=?,session_payment_status=?,session_subscription=? WHERE account_id=? AND id=? AND session_fate=''`, |
| - fate, max(now.Unix(), 1), payment, subscription, accountID, intentID); err != nil { |
| - return err |
| + case err == nil: |
| + // A retirement from before the protocol recorded no fate: its |
| + // session was a paid one, and what follows it settles as it did. |
| + if archivedFate != sessionFateExpired || archivedSession != o.SessionID { |
| + return nil |
| } |
| - case st.Fate != fate: |
| - if err := disputeSessionTx(tx, accountID, sessionID, intentID, "read back "+fate+" after it was recorded "+st.Fate, now); err != nil { |
| - return err |
| + if !o.Identity { |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "read back as another account's or intent's after its intent was retired on a proven expiry", now) |
| + } else if o.Fate != sessionFateExpired { |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "read back "+read+" after its intent was retired on a proven expiry", now) |
| } |
| - case fate == sessionFateComplete && st.SessionSubscription != "" && subscription != "" && subscription != st.SessionSubscription: |
| - if err := disputeSessionTx(tx, accountID, sessionID, intentID, "its subscription read back as "+subscription+" after it was recorded as "+st.SessionSubscription, now); err != nil { |
| - return err |
| + case errors.Is(err, sql.ErrNoRows): |
| + 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 == "" || keptSession != o.SessionID)) { |
| + return nil |
| } |
| - case fate == sessionFateComplete: |
| - // The terminal status stands; what was paid and for which |
| - // subscription may become known later. |
| - if _, err := tx.Exec(`UPDATE billing_checkout_intents SET session_payment_status=?,session_subscription=CASE WHEN session_subscription='' THEN ? ELSE session_subscription END WHERE account_id=? AND id=?`, |
| - payment, subscription, accountID, intentID); err != nil { |
| + if err != nil { |
| return err |
| } |
| + switch { |
| + case !o.Identity: |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "read back as another account's or intent's after its purchase was kept and recorded", now) |
| + case o.Fate != sessionFateComplete: |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "read back "+read+" after its purchase was kept as complete", now) |
| + case keptSubscription != "" && o.Subscription != "" && o.Subscription != keptSubscription: |
| + err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "its subscription read back as "+o.Subscription+" after its purchase was kept with "+keptSubscription, now) |
| + } |
| + } |
| + if err != nil { |
| + return err |
| } |
| return tx.Commit() |
| } |
| -// noteExpiryTry schedules the next attempt to prove a refused intent's |
| -// fate: 1, 5, 15 minutes, then hourly. |
| -func (s *Store) noteExpiryTry(accountID int64, intentID string, now time.Time) error { |
| - _, err := s.db.Exec(`UPDATE billing_checkout_intents SET expiry_tries=expiry_tries+1, |
| - expiry_next_try_at=?+CASE expiry_tries WHEN 0 THEN 60 WHEN 1 THEN 300 WHEN 2 THEN 900 ELSE 3600 END |
| - WHERE account_id=? AND id=?`, now.Unix(), accountID, intentID) |
| - return err |
| -} |
| - |
| -// noteLateSubscription records a subscription that names an intent which |
| -// no longer stands and was not known as attached to it when it was |
| -// disposed of: a person must see it, and it must not vanish as a settled |
| -// event because the intent is gone. |
| -func (s *Store) noteLateSubscription(accountID int64, intentID, subscription string, now time.Time) error { |
| +// noteSubscription classifies and records a subscription that names an |
| +// intent, in one transaction that loads the purchase as it is now. |
| +// |
| +// The intent stands with a refusal review, open or resolved: the |
| +// subscription is attached to the case unless it is the pinned session's |
| +// own, recorded one; attached to a review that was resolved (the intent |
| +// 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. |
| +func (s *Store) noteSubscription(accountID int64, intentID, subscription string, now time.Time) error { |
| if intentID == "" || subscription == "" { |
| return nil |
| } |
| @@ -403,34 +441,82 @@ func (s *Store) noteLateSubscription(accountID int64, intentID, subscription str |
| return err |
| } |
| defer tx.Rollback() |
| - var known int |
| - if err := tx.QueryRow(`SELECT count(*) FROM billing_review_obligations WHERE account_id=? AND intent_id=? AND subscription=?`, accountID, intentID, subscription).Scan(&known); err != nil { |
| + st, err := refusalStateQ(tx, accountID, intentID) |
| + if err != nil { |
| return err |
| } |
| - if known > 0 { |
| - return nil |
| - } |
| - // Only for an intent retired on a proven expiry, which had no |
| - // subscription at all. A retirement from before this protocol (no |
| - // fate recorded) was of a paid session, whose own subscription's |
| - // later events are expected and settle as they did. |
| - var sessionID, fate string |
| - err = tx.QueryRow(`SELECT session_id,session_fate FROM billing_retired_intents WHERE account_id=? AND intent_id=?`, accountID, intentID).Scan(&sessionID, &fate) |
| - if errors.Is(err, sql.ErrNoRows) || (err == nil && fate != sessionFateExpired) { |
| - return nil |
| + if st.Stands { |
| + if !st.Exists || subscription == st.SessionSubscription { |
| + return nil |
| + } |
| + attached, err := attachSubscriptionTx(tx, accountID, intentID, subscription, now) |
| + if err != nil { |
| + return err |
| + } |
| + if attached && !st.Open { |
| + if err := fileBillingReviewTx(tx, accountID, intentID, billingCheckoutRefusedReason, now); err != nil { |
| + return err |
| + } |
| + if _, err := tx.Exec(`UPDATE billing_reviews SET protocol=1 WHERE account_id=? AND subscription=? AND reason=?`, accountID, intentID, billingCheckoutRefusedReason); err != nil { |
| + return err |
| + } |
| + } |
| + return tx.Commit() |
| } |
| - if err != nil { |
| - return err |
| + for _, known := range st.Obligations { |
| + if known == subscription { |
| + return nil |
| + } |
| } |
| - if sessionID == "" { |
| - sessionID = intentID |
| + var archivedSession, archivedFate string |
| + err = tx.QueryRow(`SELECT session_id,session_fate FROM billing_retired_intents WHERE account_id=? AND intent_id=?`, accountID, intentID).Scan(&archivedSession, &archivedFate) |
| + switch { |
| + case err == nil: |
| + if archivedFate != sessionFateExpired { |
| + return nil |
| + } |
| + if archivedSession == "" { |
| + archivedSession = intentID |
| + } |
| + err = disputeSessionTx(tx, accountID, archivedSession, intentID, "subscription "+subscription+" names the intent retired on a proven expiry, and nothing was attached to it then", now) |
| + case errors.Is(err, sql.ErrNoRows): |
| + 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)) { |
| + return nil |
| + } |
| + if err != nil { |
| + return err |
| + } |
| + err = disputeSessionTx(tx, accountID, keptSession, intentID, "subscription "+subscription+" names the intent whose purchase was kept and recorded with "+keptSubscription, now) |
| } |
| - if err := disputeSessionTx(tx, accountID, sessionID, intentID, "subscription "+subscription+" names the retired intent and was not attached to it when it was retired", now); err != nil { |
| + if err != nil { |
| return err |
| } |
| return tx.Commit() |
| } |
| +// attachSubscriptionTx records a subscription as attached to a refused |
| +// intent and reports whether it was not attached before. |
| +func attachSubscriptionTx(tx *sql.Tx, accountID int64, intentID, subscription string, now time.Time) (bool, error) { |
| + res, err := tx.Exec(`INSERT OR IGNORE INTO billing_review_obligations(account_id,intent_id,subscription,first_seen_at) VALUES(?,?,?,?)`, accountID, intentID, subscription, now.Unix()) |
| + if err != nil { |
| + return false, err |
| + } |
| + n, err := res.RowsAffected() |
| + return n == 1, err |
| +} |
| + |
| +// noteExpiryTry schedules the next attempt to prove a refused intent's |
| +// fate: 1, 5, 15 minutes, then hourly. |
| +func (s *Store) noteExpiryTry(accountID int64, intentID string, now time.Time) error { |
| + _, err := s.db.Exec(`UPDATE billing_checkout_intents SET expiry_tries=expiry_tries+1, |
| + expiry_next_try_at=?+CASE expiry_tries WHEN 0 THEN 60 WHEN 1 THEN 300 WHEN 2 THEN 900 ELSE 3600 END |
| + WHERE account_id=? AND id=?`, now.Unix(), accountID, intentID) |
| + return err |
| +} |
| + |
| // refusalReviewOpen reports whether the intent is under an open refusal |
| // review. |
| func (s *Store) refusalReviewOpen(accountID int64, intentID string) (bool, error) { |
| @@ -459,35 +545,30 @@ func (b *billingRuntime) sessionProof(session *billingstripe.Session, intent bil |
| return "", true |
| } |
| -// noteSessionEvidence records what a session just read proves about a |
| -// refused intent, and disputes a recorded fate the read contradicts in |
| -// identity. It writes nothing for an intent with no refusal review. |
| -func (s *Server) noteSessionEvidence(intent billingIntent, acct *Account, session *billingstripe.Session, now time.Time) error { |
| - if session == nil { |
| - return nil |
| - } |
| - st, err := s.store.refusalState(acct.ID, intent.ID) |
| - if err != nil || !st.Exists { |
| - return err |
| - } |
| - pinned := st.pinned() |
| - if pinned == "" || session.ID != pinned { |
| - return nil |
| - } |
| - fate, identity := s.billing.sessionProof(session, intent, pinned, acct) |
| - if !identity { |
| - if st.Fate != "" { |
| - return s.store.disputeSession(acct.ID, pinned, intent.ID, "read back as another account's, customer's or intent's after it was recorded "+st.Fate, now) |
| - } |
| - return nil |
| +// observeSession turns a provider answer into the observation the store |
| +// classifies. Identity here is the mode, the intent's id and the account |
| +// in both places; the store adds the session's id and the customer. |
| +func (b *billingRuntime) observeSession(session *billingstripe.Session, intentID string, acct *Account) sessionObservation { |
| + o := sessionObservation{SessionID: session.ID, Customer: session.CustomerID, Status: session.Status, PaymentStatus: session.PaymentStatus, Subscription: session.SubscriptionID, |
| + Identity: session.LiveMode == b.cfg.LiveMode && session.Metadata["checkout_intent"] == intentID && session.ClientReferenceID == acct.Sqid && session.Metadata["account_sqid"] == acct.Sqid} |
| + switch { |
| + case session.Status == "expired" && session.SubscriptionID == "" && session.PaymentStatus != "paid": |
| + o.Fate = sessionFateExpired |
| + case session.Status == "complete": |
| + o.Fate = sessionFateComplete |
| } |
| - if fate == "" { |
| - if st.Fate != "" { |
| - return s.store.disputeSession(acct.ID, pinned, intent.ID, "read back open after it was recorded "+st.Fate, now) |
| - } |
| + return o |
| +} |
| + |
| +// noteSessionEvidence hands a session just read to the store's one |
| +// classification (noteSessionObservation), for the intent named. It reads |
| +// nothing first and decides nothing: a caller with a stale view of the |
| +// purchase cannot drop what it saw. |
| +func (s *Server) noteSessionEvidence(intentID string, acct *Account, session *billingstripe.Session, now time.Time) error { |
| + if session == nil || session.ID == "" || intentID == "" || acct == nil { |
| return nil |
| } |
| - return s.store.noteSessionProof(acct.ID, intent.ID, pinned, fate, session.PaymentStatus, session.SubscriptionID, now) |
| + return s.store.noteSessionObservation(acct.ID, intentID, s.billing.observeSession(session, intentID, acct), now) |
| } |
| // advanceRefusal works a refused intent toward a recorded fate: it reads |
| @@ -526,8 +607,11 @@ func (s *Server) advanceRefusal(ctx context.Context, intent billingIntent, acct |
| // Answered with another session: the read failed. |
| return fail(errBillingPending) |
| } |
| + if err := s.noteSessionEvidence(intent.ID, acct, session, now); err != nil { |
| + return err |
| + } |
| if fate, _ := b.sessionProof(session, intent, pinned, acct); fate != "" { |
| - return s.store.noteSessionProof(acct.ID, intent.ID, pinned, fate, session.PaymentStatus, session.SubscriptionID, now) |
| + return nil |
| } |
| // Open (or anything else not terminal). The expiry is the console's to |
| // request for a session it bound, by that id and under the intent's |
| @@ -541,8 +625,11 @@ func (s *Server) advanceRefusal(ctx context.Context, intent billingIntent, acct |
| if again.ID != pinned { |
| return fail(errors.Join(expireErr, errBillingPending)) |
| } |
| + if err := s.noteSessionEvidence(intent.ID, acct, again, now); err != nil { |
| + return errors.Join(expireErr, err) |
| + } |
| if fate, _ := b.sessionProof(again, intent, pinned, acct); fate != "" { |
| - return s.store.noteSessionProof(acct.ID, intent.ID, pinned, fate, again.PaymentStatus, again.SubscriptionID, now) |
| + return nil |
| } |
| if expireErr != nil { |
| return fail(expireErr) |
diff --git a/internal/console/billingretirement_test.go b/internal/console/billingretirement_test.go| index 4b5c8088a..92da2a042 100644 |
| --- a/internal/console/billingretirement_test.go |
| +++ b/internal/console/billingretirement_test.go |
| @@ -4,6 +4,7 @@ import ( |
| "context" |
| "database/sql" |
| "errors" |
| + "fmt" |
| "net/http" |
| "strings" |
| "testing" |
| @@ -257,6 +258,22 @@ func TestRetireOnAProvenExpiryWithNothingAttached(t *testing.T) { |
| if status, _ := resolveRefusal(t, fx, token, acct, intent, "", "another note"); status != http.StatusConflict { |
| t.Fatalf("a replay with another note: %d; want 409", status) |
| } |
| + // The retired session's completion, delivered afterwards (the provider |
| + // said expired and then completed it): settled as an event, and a |
| + // dispute for that session, recorded before the event is settled, |
| + // once however often it is delivered. |
| + f.mu.Lock() |
| + f.sessionStatus, f.subscriptionPrice = "complete", intent.Price |
| + f.mu.Unlock() |
| + for _, id := range []string{"evt_late_done", "evt_late_done", "evt_late_done_again"} { |
| + if code := f.webhookPost(t, fx, id, "checkout.session.completed", intent.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the retired session's late completion: %d; want it settled", code) |
| + } |
| + } |
| + var completions int |
| + if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_checkout_disputes WHERE account_id=? AND session_id=? AND detail LIKE 'read back complete%'`, acct.ID, intent.SessionID).Scan(&completions); err != nil || completions != 1 { |
| + t.Fatalf("%d disputes for the late completion (%v); want one", completions, err) |
| + } |
| // A subscription that names the retired intent afterwards is news |
| // (nothing was attached when it was retired): settled as an event, and |
| // a person is told, once however often it is delivered. |
| @@ -268,12 +285,20 @@ func TestRetireOnAProvenExpiryWithNothingAttached(t *testing.T) { |
| t.Fatalf("late subscription event %d: %d; want it settled", i, code) |
| } |
| } |
| - var disputes, occurrences int |
| - if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_checkout_disputes WHERE account_id=? AND session_id=?`, acct.ID, intent.SessionID).Scan(&disputes); err != nil || disputes != 1 { |
| - t.Fatalf("%d disputes (%v); want one", disputes, err) |
| + var disputes, occurrences, attached int |
| + if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_checkout_disputes WHERE account_id=? AND session_id=? AND detail LIKE 'subscription sub_paid names%'`, acct.ID, intent.SessionID).Scan(&disputes); err != nil || disputes != 1 { |
| + t.Fatalf("%d disputes for the late subscription (%v); want one", disputes, err) |
| } |
| - if err := fx.st.db.QueryRow(`SELECT occurrences FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0`, acct.ID, intent.SessionID, billingFateDisputedReason).Scan(&occurrences); err != nil || occurrences != 1 { |
| - t.Fatalf("the dispute review: %d occurrences (%v); want one open review filed once", occurrences, err) |
| + // Two distinct contradictions (the completion, the subscription), each |
| + // filed once on the one open review for the session. |
| + if err := fx.st.db.QueryRow(`SELECT occurrences FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0`, acct.ID, intent.SessionID, billingFateDisputedReason).Scan(&occurrences); err != nil || occurrences != 2 { |
| + t.Fatalf("the dispute review: %d occurrences (%v); want two, one per contradiction", occurrences, err) |
| + } |
| + // And nothing is attached to an intent that no longer stands: an |
| + // attachment after the retirement would make a later delivery read |
| + // as already known. |
| + if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_review_obligations WHERE account_id=? AND intent_id=?`, acct.ID, intent.ID).Scan(&attached); err != nil || attached != 0 { |
| + t.Fatalf("%d subscriptions attached to the retired intent (%v); want none", attached, err) |
| } |
| if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| t.Fatalf("recorded from a retired intent's subscription: %+v", state) |
| @@ -500,19 +525,32 @@ func TestEvidenceAgainstARecordedFateIsADispute(t *testing.T) { |
| if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| t.Fatalf("recorded from a disputed session: %+v", state) |
| } |
| + // Its fate is expired with nothing attached, so but for the dispute a |
| + // retire would go through: it is the dispute that holds it. |
| token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-dispute") |
| - for _, outcome := range []string{"", "keep"} { |
| - if status, msg := resolveRefusal(t, fx, token, acct, intent, outcome, "x"); status != http.StatusConflict { |
| - t.Fatalf("outcome %q on a disputed intent: %d %q; want 409", outcome, status, msg) |
| - } |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "", "x"); status != http.StatusConflict || !strings.Contains(msg, "contradictory") { |
| + t.Fatalf("retire of a disputed intent: %d %q; want 409 for the contradiction", status, msg) |
| } |
| - if _, err := fx.st.billingCheckoutIntent(acct.ID); err != nil { |
| - t.Fatalf("the disputed intent: %v; want it standing", err) |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "keep", "x"); status != http.StatusConflict { |
| + t.Fatalf("keep of a disputed intent: %d %q; want 409", status, msg) |
| + } |
| + standing, err := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || standing.ID != intent.ID { |
| + t.Fatalf("the disputed intent: %+v %v; want it standing", standing, err) |
| } |
| - // The ordinary expiry retirement does not take it either. |
| - if err := fx.st.db.QueryRow(`SELECT 1 FROM billing_checkout_intents WHERE account_id=? AND fate_disputed_at<>0`, acct.ID).Scan(new(int)); err != nil { |
| + // 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. |
| + 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) |
| } |
| + 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) |
| + } |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); err != nil { |
| + t.Fatalf("the disputed intent after the expiry proof: %v; want it standing", err) |
| + } |
| } |
| // Keep is for the plain case too: a completed session with a subscription |
| @@ -542,3 +580,365 @@ func TestKeepIsHeldWhileAnotherSubscriptionIsAttached(t *testing.T) { |
| t.Fatalf("retire of the same: %d %q; want 409", status, msg) |
| } |
| } |
| + |
| +// A subscription that names the refused intent on the purchase's own |
| +// terms, made outside checkout while the bound session is still open, is |
| +// attached to the case the moment it is seen, though reconcile answers it |
| +// pending; and when the buyer then pays the session, keep is held for it: |
| +// keeping the checkout's purchase disposes of nothing about that other |
| +// subscription. (Review nineteen, Opus P2-1; Astra 1.) |
| +func TestAnOnTermsSubscriptionBesideTheSessionIsAttachedAndHoldsKeep(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.sessionCustomer, f.failExpire = "cus_other", 100 |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(time.Minute)); dest != "" || err == nil { |
| + t.Fatalf("the contradicted read: dest %q err %v", dest, err) |
| + } |
| + // H: the intent's own plan, band, terms, quantity and price, naming |
| + // the intent, made by hand. The session is still open, so reconcile |
| + // answers pending (the provider retries); H is attached all the same. |
| + f.mu.Lock() |
| + f.sessionCustomer, f.subscriptionPrice = "", intent.Price |
| + f.otherSubscriptions = map[string]bool{"sub_hand": true} |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_on_terms", "customer.subscription.created", "sub_hand"); code == http.StatusNoContent || code == http.StatusOK { |
| + t.Fatalf("the on-terms subscription while the session is open: %d; want it retried, not settled", code) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || len(st.Obligations) != 1 || st.Obligations[0] != "sub_hand" { |
| + t.Fatalf("state %+v %v; want sub_hand attached", st, err) |
| + } |
| + // The buyer pays the session; its own subscription is sub_paid. |
| + f.mu.Lock() |
| + f.sessionStatus, f.failExpire = "complete", 0 |
| + f.mu.Unlock() |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-on-terms") |
| + status, msg := resolveRefusal(t, fx, token, acct, intent, "keep", "paid") |
| + if status != http.StatusConflict || !strings.Contains(msg, "sub_hand") { |
| + t.Fatalf("keep beside the hand-made subscription: %d %q; want 409 held, naming it", status, msg) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded: %+v", state) |
| + } |
| +} |
| + |
| +// A subscription with two items that names the bound standing intent is |
| +// not a purchase, and is not a bare rejection either: it is the refusal |
| +// review, with the subscription attached, filed in one transaction. |
| +// (Review nineteen, Astra 1: it returned before anything was captured.) |
| +func TestAStructurallyInvalidSubscriptionNamingTheIntentIsReviewedAndAttached(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, f.subscriptionSecondItem = intent.Price, true |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_two_items", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the two-item subscription: %d; want it settled as refused", code) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || st.Protocol != 1 || len(st.Obligations) != 1 || st.Obligations[0] != "sub_paid" { |
| + t.Fatalf("state %+v %v; want the review open with the subscription attached", st, err) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded from a two-item subscription: %+v", state) |
| + } |
| +} |
| + |
| +// The refusal and the subscription that caused it are one transaction: no |
| +// reader, on this console or another, after a crash or between two |
| +// statements, finds the review without what is attached to it. And an |
| +// attachment is made only while the intent stands: for an intent already |
| +// retired the same subscription is a dispute, never a row that would make |
| +// its next delivery read as already known. (Review nineteen: Opus P3-1, |
| +// both scenarios; Astra 1.) |
| +func TestTheRefusalAndItsSubscriptionAreOneTransaction(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) |
| + // Every statement of the filing is observed from another connection's |
| + // point of view by a trigger on the reviews table: at the instant the |
| + // review row appears, is the attachment already in the same |
| + // transaction's view? SQLite gives one writer, so the question is |
| + // whether both are written before the commit: a rollback after the |
| + // review's insert must take the review with it. |
| + tx, err := fx.st.db.Begin() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := fileBillingReviewTx(tx, acct.ID, intent.ID, billingCheckoutRefusedReason, f.now); err != nil { |
| + t.Fatal(err) |
| + } |
| + _ = tx.Rollback() // the crash between the review and the attachment, as two statements of one transaction |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Exists { |
| + t.Fatalf("a review survives the rolled-back filing: %+v", st) |
| + } |
| + if err := fx.st.fileCheckoutRefusal(intent, intent.SessionID, "sub_hand", f.now); err != nil { |
| + t.Fatal(err) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || st.Protocol != 1 || len(st.Obligations) != 1 || st.Obligations[0] != "sub_hand" { |
| + t.Fatalf("state %+v %v; want the review and the attachment together", st, err) |
| + } |
| + // The intent gone (retired on a proven expiry, as the store records |
| + // one): the same subscription seen now attaches nothing and is a |
| + // dispute. |
| + if _, err := fx.st.db.Exec(`DELETE FROM billing_review_obligations WHERE account_id=?`, acct.ID); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := fx.st.noteSessionObservation(acct.ID, intent.ID, sessionObservation{SessionID: intent.SessionID, Identity: true, Customer: intent.Customer, Fate: sessionFateExpired, Status: "expired", PaymentStatus: "unpaid"}, f.now); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := fx.st.resolveBillingReview(billingReviewResolution{AccountID: acct.ID, Subscription: intent.ID, Reason: billingCheckoutRefusedReason, Occurrence: 1, Actor: "paul@keymail.dev", Note: "expired, never paid"}, f.now.Add(time.Minute)); err != nil { |
| + t.Fatalf("retire on the recorded expiry: %v", err) |
| + } |
| + for i := 0; i < 2; i++ { |
| + if err := fx.st.noteSubscription(acct.ID, intent.ID, "sub_hand", f.now.Add(time.Hour)); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + var attached, disputes int |
| + _ = fx.st.db.QueryRow(`SELECT count(*) FROM billing_review_obligations WHERE account_id=? AND intent_id=?`, acct.ID, intent.ID).Scan(&attached) |
| + _ = fx.st.db.QueryRow(`SELECT count(*) FROM billing_checkout_disputes WHERE account_id=? AND session_id=?`, acct.ID, intent.SessionID).Scan(&disputes) |
| + if attached != 0 || disputes != 1 { |
| + t.Fatalf("%d attached, %d disputes; want nothing attached to a retired intent and one dispute", attached, disputes) |
| + } |
| +} |
| + |
| +// What the provider shows after a purchase was kept and recorded is still |
| +// evidence: another subscription naming the consumed intent, and the kept |
| +// session read back as anything but what was kept, are disputes for that |
| +// session, recorded before the event is answered. And a dispute raised |
| +// between the keep and the record stops the record as a conflict, not as |
| +// the delete barrier's raw error. (Review nineteen: Astra 3; Opus P2-1's |
| +// second half and P3-4.) |
| +func TestEvidenceAfterAKeepSurvivesTheIntent(t *testing.T) { |
| + kept := func(t *testing.T) (*consoleFixture, *fakeBillingStripe, *Account, billingIntent, string) { |
| + 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.sessionCustomer, f.failExpire = "cus_other", 100 |
| + f.mu.Unlock() |
| + _, _ = fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(time.Minute)) |
| + f.mu.Lock() |
| + f.sessionCustomer, f.sessionStatus, f.subscriptionPrice, f.failExpire = "", "complete", intent.Price, 0 |
| + f.mu.Unlock() |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-after-keep") |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "keep", "paid through the original link"); status != http.StatusOK { |
| + t.Fatalf("keep: %d %q", status, msg) |
| + } |
| + return fx, f, acct, intent, token |
| + } |
| + disputes := func(t *testing.T, fx *consoleFixture, acct *Account, intent billingIntent, like string) int { |
| + t.Helper() |
| + var n int |
| + if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_checkout_disputes WHERE account_id=? AND session_id=? AND detail LIKE ?`, acct.ID, intent.SessionID, like).Scan(&n); err != nil { |
| + t.Fatal(err) |
| + } |
| + return n |
| + } |
| + t.Run("another subscription after the record", func(t *testing.T) { |
| + fx, f, acct, intent, _ := kept(t) |
| + if code := f.webhookPost(t, fx, "evt_kept_sub", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the kept purchase's subscription: %d", code) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "sub_paid" { |
| + t.Fatalf("fixture: %+v", state) |
| + } |
| + f.mu.Lock() |
| + f.otherSubscriptions = map[string]bool{"sub_hand": true} |
| + f.mu.Unlock() |
| + for _, id := range []string{"evt_after_keep", "evt_after_keep_again"} { |
| + if code := f.webhookPost(t, fx, id, "customer.subscription.created", "sub_hand"); code != http.StatusNoContent { |
| + t.Fatalf("another subscription naming the consumed intent: %d; want it settled", code) |
| + } |
| + } |
| + if n := disputes(t, fx, acct, intent, "subscription sub_hand names%"); n != 1 { |
| + t.Fatalf("%d disputes for the other subscription; want one", n) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "sub_paid" { |
| + t.Fatalf("the recorded purchase changed: %+v", state) |
| + } |
| + // The kept session read back complete with another subscription |
| + // than the one it was kept with: a dispute. |
| + f.mu.Lock() |
| + f.sessionSubscription = "sub_changed" |
| + f.mu.Unlock() |
| + _ = f.webhookPost(t, fx, "evt_kept_session_changed", "checkout.session.completed", intent.SessionID) |
| + if n := disputes(t, fx, acct, intent, "its subscription read back as sub_changed after its purchase was kept with sub_paid"); n != 1 { |
| + t.Fatalf("%d disputes for the kept session naming another subscription; want one", n) |
| + } |
| + // And read back expired, by its completion event delivered again: |
| + // a dispute too. |
| + f.mu.Lock() |
| + f.sessionStatus, f.sessionSubscription = "expired", "" |
| + f.mu.Unlock() |
| + _ = f.webhookPost(t, fx, "evt_kept_session_again", "checkout.session.completed", intent.SessionID) |
| + if n := disputes(t, fx, acct, intent, "read back expired after its purchase was kept%"); n != 1 { |
| + t.Fatalf("%d disputes for the kept session read back expired; want one", n) |
| + } |
| + }) |
| + t.Run("another subscription between the keep and the record reopens the review", func(t *testing.T) { |
| + fx, f, acct, intent, token := kept(t) |
| + f.mu.Lock() |
| + f.otherSubscriptions = map[string]bool{"sub_hand": true} |
| + f.mu.Unlock() |
| + _ = f.webhookPost(t, fx, "evt_between", "customer.subscription.created", "sub_hand") |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || len(st.Obligations) != 1 || st.Obligations[0] != "sub_hand" { |
| + t.Fatalf("state %+v %v; want the review open again with sub_hand attached", st, err) |
| + } |
| + // The kept purchase's own subscription does not record under it. |
| + _ = f.webhookPost(t, fx, "evt_between_own", "customer.subscription.created", "sub_paid") |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded under the reopened review: %+v", state) |
| + } |
| + body := map[string]any{"account": acct.Sqid, "subscription": intent.ID, "reason": billingCheckoutRefusedReason, "occurrences": 2, "note": "paid", "outcome": "keep"} |
| + if status, answer := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", body, token); status != http.StatusConflict || !strings.Contains(fmt.Sprint(answer["error"]), "sub_hand") { |
| + t.Fatalf("keep of the reopened review: %d %v; want 409 held, naming sub_hand", status, answer) |
| + } |
| + }) |
| + t.Run("a dispute between the keep and the record is a conflict", func(t *testing.T) { |
| + fx, f, acct, intent, _ := kept(t) |
| + // The kept session now names another subscription. |
| + f.mu.Lock() |
| + f.sessionSubscription = "sub_changed" |
| + f.mu.Unlock() |
| + _ = f.webhookPost(t, fx, "evt_changed_done", "checkout.session.completed", intent.SessionID) |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Disputed == 0 { |
| + t.Fatalf("state %+v; want the kept intent disputed", st) |
| + } |
| + // The provider shows the first subscription again, so every check |
| + // of the reconcile passes and it reaches its record: refused there |
| + // as a conflict (the consumption names the dispute), which is the |
| + // same answer a record under an open review gets, and not the |
| + // delete barrier's raw abort. |
| + f.mu.Lock() |
| + f.sessionSubscription = "" |
| + f.mu.Unlock() |
| + err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "evt_changed_sub", f.now.Add(time.Hour)) |
| + if !errors.Is(err, errBillingConflict) { |
| + t.Fatalf("the record of a disputed kept purchase: %v; want a conflict", err) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded a disputed purchase: %+v", state) |
| + } |
| + }) |
| +} |
| + |
| +// The keep gate's own read is evidence: a session that now names another |
| +// subscription than the one recorded with its completion is a dispute, |
| +// and the keep is refused. (Review nineteen, Astra 2.) |
| +func TestTheKeepGatesReadIsRecordedAsEvidence(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.sessionCustomer, f.failExpire = "cus_other", 100 |
| + f.mu.Unlock() |
| + _, _ = fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(time.Minute)) |
| + f.mu.Lock() |
| + f.sessionCustomer, f.sessionStatus, f.subscriptionPrice, f.failExpire = "", "complete", intent.Price, 0 |
| + f.mu.Unlock() |
| + // The completion is recorded with subscription sub_paid. |
| + if err := fx.srv.advanceRefusal(context.Background(), intent, acct, f.now.Add(2*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Fate != sessionFateComplete || st.SessionSubscription != "sub_paid" { |
| + t.Fatalf("fixture: %+v", st) |
| + } |
| + // At the keep, the same session names another subscription. |
| + f.mu.Lock() |
| + f.sessionSubscription = "sub_other" |
| + f.mu.Unlock() |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-gate-read") |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "keep", "paid"); status != http.StatusConflict { |
| + t.Fatalf("keep when the session names another subscription than recorded: %d %q; want 409", status, msg) |
| + } |
| + st, _ := fx.st.refusalState(acct.ID, intent.ID) |
| + if st.Disputed == 0 || !st.Open || st.SessionSubscription != "sub_paid" { |
| + t.Fatalf("state %+v; want the dispute recorded, the review open, the first subscription kept on record", st) |
| + } |
| +} |
| + |
| +// A reader with a stale view cannot drop what it saw: the classification |
| +// is the store's, in one transaction, whatever the caller last knew. A |
| +// read that names another customer, arriving after another actor recorded |
| +// the expiry, is a dispute, and the retire that would have passed is held. |
| +// (Review nineteen, Astra 3, the third path.) |
| +func TestAContradictionReadBesideARecordedFateIsNotDropped(t *testing.T) { |
| + fx, f, acct, intent := refusedByAnotherCustomer(t) |
| + // Actor A read the session as another customer's (the fixture), and is |
| + // paused holding that read. Actor B, after the provider's record was |
| + // corrected, records the expiry. |
| + stale := fx.srv.billing.observeSession(&billingstripe.Session{ID: intent.SessionID, Status: "expired", LiveMode: false, ClientReferenceID: acct.Sqid, CustomerID: "cus_other", Metadata: map[string]string{"checkout_intent": intent.ID, "account_sqid": acct.Sqid}}, intent.ID, acct) |
| + f.mu.Lock() |
| + f.sessionCustomer = "" |
| + f.mu.Unlock() |
| + if err := fx.srv.advanceRefusal(context.Background(), intent, acct, f.now.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Fate != sessionFateExpired || st.Disputed != 0 { |
| + t.Fatalf("fixture: %+v", st) |
| + } |
| + // A resumes and hands in what it read. |
| + if err := fx.st.noteSessionObservation(acct.ID, intent.ID, stale, f.now.Add(2*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Disputed == 0 { |
| + t.Fatalf("state %+v; want the contradiction recorded as a dispute", st) |
| + } |
| + if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil { |
| + t.Fatal(err) |
| + } |
| + heldRetire(t, fx, acct, intent.ID, "paul@keymail.dev", "contradictory", f.now.Add(time.Hour)) |
| +} |
| + |
| +// The refusals' turn in a pass does not depend on the reconciliation's |
| +// time: a pass whose deadline has already passed still works a refused |
| +// intent whose retry is due. (Review nineteen, Astra 4.) |
| +func TestThePassWorksADueRefusalThoughItsOwnTimeIsSpent(t *testing.T) { |
| + fx, f, acct, intent, _ := refusedByAHandMadeSubscription(t, 1) |
| + if f.sessionStatus != "open" { |
| + t.Fatalf("fixture: session %s", f.sessionStatus) |
| + } |
| + spent, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Minute)) |
| + defer cancel() |
| + _ = fx.srv.billingPass(spent, f.now.Add(2*time.Minute)) |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || f.sessionStatus != "expired" || st.Fate != sessionFateExpired { |
| + t.Fatalf("after a pass with no time left: session %s, state %+v %v; want the due expiry made and proven", f.sessionStatus, st, err) |
| + } |
| + // The console stopping does stop it. |
| + fx2, f2, _, _, _ := refusedByAHandMadeSubscription(t, 1) |
| + stopped, stop := context.WithCancel(context.Background()) |
| + stop() |
| + calls := f2.expireCalls |
| + _ = fx2.srv.billingPass(stopped, f2.now.Add(2*time.Minute)) |
| + if f2.expireCalls != calls { |
| + t.Fatalf("a cancelled pass made %d expiry requests", f2.expireCalls-calls) |
| + } |
| +} |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 543a3e536..0f0a5fa6c 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -329,7 +329,7 @@ func (s *Server) ownBoundSession(ctx context.Context, session *billingstripe.Ses |
| // if it is open, and what the read after it proves recorded). A read |
| // that names another account proves nothing, so the fate of such a |
| // session stays unproven and the intent held. |
| - if err := s.noteSessionEvidence(intent, acct, session, now); err != nil { |
| + if err := s.noteSessionEvidence(intent.ID, acct, session, now); err != nil { |
| return false, err |
| } |
| if err := s.advanceRefusal(ctx, intent, acct, now); err != nil { |
| @@ -373,7 +373,7 @@ func (s *Server) refuseSession(ctx context.Context, intent billingIntent, sessio |
| if acct, err := s.store.AccountByID(intent.AccountID); err != nil { |
| return err |
| } else if acct != nil { |
| - if err := s.noteSessionEvidence(intent, acct, session, now); err != nil { |
| + if err := s.noteSessionEvidence(intent.ID, acct, session, now); err != nil { |
| return err |
| } |
| } |
| @@ -826,9 +826,22 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| if sub.Metadata["account_sqid"] != acct.Sqid { |
| return errBillingIdentity |
| } |
| - if sub.ID != id || sub.LiveMode != b.cfg.LiveMode || len(sub.Items) != 1 || sub.ItemsHasMore || sub.CustomerID == "" { |
| + if sub.ID != id || sub.LiveMode != b.cfg.LiveMode || sub.CustomerID == "" { |
| return errBillingIdentity |
| } |
| + // Evidence first (billingretirement.go, the evidence invariant): a |
| + // subscription that is not the one the account has recorded and that |
| + // names an intent is handed to the store's one classification before |
| + // any answer below, pending and identity answers included. Under a |
| + // refusal review it is attached to the case; naming an intent that |
| + // was retired on a proven expiry, or whose purchase was kept and |
| + // recorded, it is a dispute. For a purchase with no refusal this |
| + // records nothing. |
| + if state.Subscription != id { |
| + if err := s.store.noteSubscription(acct.ID, sub.Metadata["checkout_intent"], id, now); err != nil { |
| + return err |
| + } |
| + } |
| // For a first purchase (no subscription recorded yet, or a dead one), |
| // the standing intent is read before the subscription is judged: a |
| // subscription that names that intent and is bound to a session, and |
| @@ -850,13 +863,8 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| if retired, err := s.store.billingIntentRetired(acct.ID, sub.Metadata["checkout_intent"]); err != nil { |
| return err |
| } else if retired { |
| - // A subscription naming an intent that was retired on a proven |
| - // expiry is news: nothing was attached to it then. A person |
| - // is told (a dispute for the retired session), and the event |
| - // is settled as before. |
| - if err := s.store.noteLateSubscription(acct.ID, sub.Metadata["checkout_intent"], id, now); err != nil { |
| - return err |
| - } |
| + // (What this subscription is evidence of was recorded above: |
| + // a dispute, for an intent retired on a proven expiry.) |
| return &billingIdentityOfAccount{AccountID: acct.ID} |
| } |
| } |
| @@ -881,18 +889,16 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| } |
| // The review first: with it filed, nothing retires the intent |
| // but the operator's resolution. |
| - if err := s.store.fileCheckoutRefusedReview(*pending, pending.SessionID, now); err != nil { |
| - return err |
| - } |
| - // The subscription that caused the refusal is recorded with |
| - // it: expiring the bound session says nothing about what this |
| - // subscription has collected or will, and a case with a |
| - // subscription attached is not retired in stage one. Then the |
| - // session is worked toward a proven fate. |
| - if err := s.store.addRefusalObligation(acct.ID, pending.ID, id, now); err != nil { |
| + // The review and the subscription that caused it are one |
| + // transaction: expiring the bound session says nothing about |
| + // what this subscription has collected or will, a case with a |
| + // subscription attached is not retired in stage one, and no |
| + // crash or other console can find the review without it. |
| + // Then the session is worked toward a proven fate. |
| + if err := s.store.fileCheckoutRefusal(*pending, pending.SessionID, id, now); err != nil { |
| return err |
| } |
| - if err := s.noteSessionEvidence(*pending, acct, session, now); err != nil { |
| + if err := s.noteSessionEvidence(pending.ID, acct, session, now); err != nil { |
| return err |
| } |
| if err := s.advanceRefusal(ctx, *pending, acct, now); err != nil { |
| @@ -901,6 +907,13 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| } |
| return errBillingIdentity |
| } |
| + // One item, as every purchase the console makes has. A subscription |
| + // with more, or none, is not a purchase; naming the bound standing |
| + // intent, it is the same review as any other refusal (and attached to |
| + // it), never a bare rejection the hold never hears of. |
| + if len(sub.Items) != 1 || sub.ItemsHasMore { |
| + return refuse() |
| + } |
| plan, terms, band := sub.Metadata["plan"], sub.Metadata["terms_version"], sub.Metadata["band"] |
| // The subscription says which book and currency it was bought under; |
| // one that says neither is from before books, which is EUR. Its price |
| @@ -1140,6 +1153,20 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| return "", err |
| } |
| if session.ID != sessionID || session.Status != "complete" || session.LiveMode != b.cfg.LiveMode || session.SubscriptionID == "" { |
| + // Evidence first: a session read back as anything but complete is |
| + // still what the provider shows of it. For a refused purchase |
| + // whose completion was recorded, or one kept and recorded, or one |
| + // retired, the store's classification says what that is; for an |
| + // ordinary pending purchase it records nothing. |
| + if session.ID == sessionID { |
| + if acct, aerr := s.store.AccountBySqid(session.Metadata["account_sqid"]); aerr != nil { |
| + return "", aerr |
| + } else if acct != nil && acct.Sqid == session.ClientReferenceID { |
| + if err := s.noteSessionEvidence(session.Metadata["checkout_intent"], acct, session, time.Now()); err != nil { |
| + return "", err |
| + } |
| + } |
| + } |
| return "", errBillingPending |
| } |
| if expected != nil { |
| @@ -1180,9 +1207,23 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| if retired, rerr := s.store.billingIntentRetired(acct.ID, session.Metadata["checkout_intent"]); rerr != nil { |
| return "", rerr |
| } else if retired { |
| + // Evidence first: the session of an intent retired on a proven |
| + // expiry that now reads complete (or as nobody's) is a dispute |
| + // for that session, recorded before the event is settled. |
| + if err := s.noteSessionEvidence(session.Metadata["checkout_intent"], acct, session, time.Now()); err != nil { |
| + return "", err |
| + } |
| return "", &billingIdentityOfAccount{AccountID: acct.ID} |
| } |
| intent, err := s.store.billingCheckoutIntent(acct.ID) |
| + if errors.Is(err, sql.ErrNoRows) { |
| + // No intent stands and none was retired: the session of a |
| + // purchase that was kept and recorded, read back as anything but |
| + // what was kept, is a dispute too. |
| + if nerr := s.noteSessionEvidence(session.Metadata["checkout_intent"], acct, session, time.Now()); nerr != nil { |
| + return "", nerr |
| + } |
| + } |
| if err != nil { |
| return "", err |
| } |
| @@ -1195,7 +1236,7 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| if !b.ownSession(session, intent, acct) { |
| // Read back as nobody's: if it is the session a refused intent is |
| // pinned to and a fate is recorded for it, that is a dispute. |
| - if err := s.noteSessionEvidence(intent, acct, session, time.Now()); err != nil { |
| + if err := s.noteSessionEvidence(intent.ID, acct, session, time.Now()); err != nil { |
| return "", err |
| } |
| return "", errBillingIdentity |
| @@ -1203,7 +1244,7 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| // What this read proves about a refused intent's session is recorded |
| // (a completion under an open review, for one): it writes nothing for |
| // an intent with no refusal review. |
| - if err := s.noteSessionEvidence(intent, acct, session, time.Now()); err != nil { |
| + if err := s.noteSessionEvidence(intent.ID, acct, session, time.Now()); err != nil { |
| return "", err |
| } |
| if !b.intentSession(session, intent, acct) { |
| @@ -1220,7 +1261,7 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| if err := s.store.fileCheckoutRefusedReview(intent, session.ID, time.Now()); err != nil { |
| return "", err |
| } |
| - if err := s.noteSessionEvidence(intent, acct, session, time.Now()); err != nil { |
| + if err := s.noteSessionEvidence(intent.ID, acct, session, time.Now()); err != nil { |
| return "", err |
| } |
| return "", errBillingIdentity |
diff --git a/internal/console/billingservice_test.go b/internal/console/billingservice_test.go| index e70ca6acc..65728c111 100644 |
| --- a/internal/console/billingservice_test.go |
| +++ b/internal/console/billingservice_test.go |
| @@ -67,6 +67,11 @@ type fakeBillingStripe struct { |
| // subscription id (sub_paid otherwise): a session whose subscription |
| // is another one than a hand-made subscription the test delivers. |
| sessionSubscription string |
| + // 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. |
| + otherSubscriptions map[string]bool |
| + subscriptionSecondItem bool |
| // expireHook, when set, runs inside the expiry, once, after the fake |
| // has marked the session expired and before it answers: what happens |
| // between the provider's expiry and the console's next step. |
| @@ -306,8 +311,14 @@ func (f *fakeBillingStripe) serve(w http.ResponseWriter, r *http.Request) { |
| } else { |
| out = map[string]any{"id": "coupon_house", "valid": true, "livemode": false, "percent_off": 100, "duration": "forever"} |
| } |
| - case r.URL.Path == "/v1/subscriptions/sub_paid": |
| - out = map[string]any{"id": "sub_paid", "customer": "cus_paid", "status": f.subscriptionStatus, "livemode": false, "created": f.now.Unix(), "current_period_start": f.now.Unix(), "current_period_end": f.now.AddDate(1, 0, 0).Unix(), "metadata": f.meta, "currency": f.subscriptionMoney(), "items": map[string]any{"data": []map[string]any{{"id": "si_paid", "price": map[string]string{"id": f.subscriptionPriceID(), "currency": f.itemMoney()}, "quantity": f.subscriptionQty()}}, "has_more": false}} |
| + case r.URL.Path == "/v1/subscriptions/sub_paid" || (strings.HasPrefix(r.URL.Path, "/v1/subscriptions/") && f.otherSubscriptions[strings.TrimPrefix(r.URL.Path, "/v1/subscriptions/")]): |
| + // sub_paid, or another subscription a test has said exists: the |
| + // same purchase terms under that id. |
| + items := []map[string]any{{"id": "si_paid", "price": map[string]string{"id": f.subscriptionPriceID(), "currency": f.itemMoney()}, "quantity": f.subscriptionQty()}} |
| + if f.subscriptionSecondItem { |
| + items = append(items, map[string]any{"id": "si_extra", "price": map[string]string{"id": f.subscriptionPriceID(), "currency": f.itemMoney()}, "quantity": 1}) |
| + } |
| + out = map[string]any{"id": strings.TrimPrefix(r.URL.Path, "/v1/subscriptions/"), "customer": "cus_paid", "status": f.subscriptionStatus, "livemode": false, "created": f.now.Unix(), "current_period_start": f.now.Unix(), "current_period_end": f.now.AddDate(1, 0, 0).Unix(), "metadata": f.meta, "currency": f.subscriptionMoney(), "items": map[string]any{"data": items, "has_more": false}} |
| default: |
| w.WriteHeader(http.StatusNotFound) |
| return |
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go| index ad7c0ebce..715d9b401 100644 |
| --- a/internal/console/billingstore.go |
| +++ b/internal/console/billingstore.go |
| @@ -779,7 +779,10 @@ func (s *Store) applyBillingStateDated(next billingState, expectedRevision int64 |
| // review is open waits for the operator (who resolves the review |
| // keeping the intent, or retires it); nothing records under a |
| // review. |
| - r, err := tx.Exec(`DELETE FROM billing_checkout_intents WHERE account_id=? AND id=? |
| + // 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. |
| + 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 { |
| @@ -970,7 +973,7 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e |
| // intent allows (billingretirement.go): read here, in the transaction |
| // that resolves it, so a fate, a dispute or an attached subscription |
| // recorded since the operator looked is what decides. |
| - evidence := "" |
| + evidence, evidenceSession, evidenceSubscription := "", "", "" |
| if r.Reason == billingCheckoutRefusedReason { |
| st, err := refusalStateQ(tx, r.AccountID, r.Subscription) |
| if err != nil { |
| @@ -980,6 +983,9 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e |
| return &billingReviewHeld{Why: why} |
| } |
| evidence = st.evidence() |
| + if st.Stands { |
| + evidenceSession, evidenceSubscription = st.pinned(), st.SessionSubscription |
| + } |
| } |
| // Never record a resolution earlier than the occurrence it resolves. |
| at := max(now.Unix(), lastSeen) |
| @@ -995,8 +1001,8 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e |
| if n != 1 { |
| return errBillingReviewStale |
| } |
| - if _, err := tx.Exec(`INSERT INTO billing_review_resolutions(account_id,subscription,reason,occurrence,last_seen_at,actor,note,resolved_at,outcome,evidence) VALUES(?,?,?,?,?,?,?,?,?,?)`, |
| - r.AccountID, r.Subscription, r.Reason, r.Occurrence, lastSeen, r.Actor, r.Note, at, r.outcome(), evidence); err != nil { |
| + if _, err := tx.Exec(`INSERT INTO billing_review_resolutions(account_id,subscription,reason,occurrence,last_seen_at,actor,note,resolved_at,outcome,evidence,evidence_session,evidence_subscription) VALUES(?,?,?,?,?,?,?,?,?,?,?,?)`, |
| + r.AccountID, r.Subscription, r.Reason, r.Occurrence, lastSeen, r.Actor, r.Note, at, r.outcome(), evidence, evidenceSession, evidenceSubscription); err != nil { |
| return err |
| } |
| // Resolving the review of a session paid in the wrong currency with |
| @@ -1055,6 +1061,15 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e |
| // binding that read the session before the resolution and files after it |
| // files nothing. |
| func (s *Store) fileCheckoutRefusedReview(intent billingIntent, sessionID string, observed time.Time) error { |
| + return s.fileCheckoutRefusal(intent, sessionID, "", observed) |
| +} |
| + |
| +// fileCheckoutRefusal is fileCheckoutRefusedReview with the subscription |
| +// that caused the refusal, when one did: the review and the attachment are |
| +// one transaction, so the review never exists without what is attached to |
| +// it (a crash, or another console resolving in between, cannot find one |
| +// without the other). |
| +func (s *Store) fileCheckoutRefusal(intent billingIntent, sessionID, subscription string, observed time.Time) error { |
| tx, err := s.db.Begin() |
| if err != nil { |
| return err |
| @@ -1091,6 +1106,11 @@ func (s *Store) fileCheckoutRefusedReview(intent billingIntent, sessionID string |
| return err |
| } |
| } |
| + if subscription != "" { |
| + if _, err := attachSubscriptionTx(tx, intent.AccountID, intent.ID, subscription, observed); err != nil { |
| + return err |
| + } |
| + } |
| return tx.Commit() |
| } |