amadan
Merge commit 'b260ac8e' into feat/carloku-billing-operator-cli-20261002
15 files changed,
+1656
−320
docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md+7 −4docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review20.md+222 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+43 −0docs/superpowers/specs/OPUS-REVIEW-multi-currency-review20.md+166 −0internal/console/billingcurrency_test.go+3 −3internal/console/billingevidence_test.go+650 −0internal/console/billinginvoice.go+1 −1internal/console/billingoperator.go+5 −9internal/console/billingpass.go+19 −17internal/console/billingquantity.go+1 −1internal/console/billingretirement.go+268 −121internal/console/billingretirement_test.go+131 −64internal/console/billingservice.go+101 −87internal/console/billingservice_test.go+6 −0internal/console/billingstore.go+33 −13
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 14f47b10e..7e96a4bd7 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 |
| @@ -98,7 +98,7 @@ Astra's third review (`CODEX-FINDINGS-intent-retirement-design-review3.md`) reje |
| - `keep`: the review was filed by this protocol; its only recorded obligation, if any, is the kept session's own subscription; nothing is disputed; the session is `complete` and paid; the subscription passes reconcile's judgement. A case with any other subscription attached is held (finding 1). |
| - Held, with no disposition in stage 1: a paid session that is not kept; any case with a hand-made subscription attached; any disputed case; **every review that was open before this build** (finding 3: its obligations were never recorded, and an empty table is not proof there were none). A migrated review is marked as such (`billing_reviews.protocol = 0`); a review this build files has `protocol = 1`. |
| -**Disputes and late evidence survive the intent** (finding 4). A contradiction is recorded against the session, not only on the intent row: `billing_checkout_disputes` (account, session id, the intent it belonged to, what contradicted what, first and last seen, occurrences), with an account-level review `billing_checkout_fate_disputed` whose subject is the session id. It is written by any actor that meets contradicting evidence for a session the console pinned, whether the intent stands, was retired, or was consumed by a `keep`; lookup is by the session id through the intent, the retirement record, or the `keep` resolution's archived evidence. The review holds the account's invoices like any other and is resolved only by a person; stage 1 defines no outcome for it beyond acknowledging it with a note after a fresh read (it never deletes, records or releases anything). A subscription that names a retired or consumed intent and was not known as an obligation when the review was resolved is filed the same way. The webhook writes the dispute, the rejected-event row and the seen-event row in one transaction; a duplicate delivery changes nothing. |
| +**Disputes and late evidence survive the intent** (finding 4). A contradiction is recorded against the session, not only on the intent row: `billing_checkout_disputes` (account, session id, the intent it belonged to, what contradicted what, first and last seen, occurrences), with an account-level review `billing_checkout_fate_disputed` whose subject is the session id. It is written by any actor that meets contradicting evidence for a session the console pinned, whether the intent stands, was retired, or was consumed by a `keep`; lookup is by the session id through the intent, the retirement record, or the `keep` resolution's archived evidence. The review holds the account's invoices like any other and is resolved only by a person; stage 1 defines no outcome for it at all: the resolution door refuses a dispute review (answering that it is held), so the hold stays until what adjudicates a dispute is decided. (Earlier text here said it could be acknowledged with a note after a fresh read; as first built, the generic resolution took the note, made no read and lifted the hold. Review twenty; it is now refused.) A subscription that names a retired or consumed intent and was not known as an obligation when the review was resolved is filed the same way. The webhook writes the dispute, the rejected-event row and the seen-event row in one transaction; a duplicate delivery changes nothing. |
| **The trigger's claim, narrowed** (review 3's first contradicted claim): the delete barrier stops an older binary from deleting an intent whose session is not proven expired. It does not make an older binary follow the protocol (it would not check obligations or the generation); the roll rule does: older consoles are stopped before this build starts and are not rolled back to. |
| @@ -110,15 +110,18 @@ Astra's third review (`CODEX-FINDINGS-intent-retirement-design-review3.md`) reje |
| ## Stage one as built |
| -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. |
| +Built in `internal/console/billingretirement.go` and the paths it touches, with tests in `billingretirement_test.go` and `billingevidence_test.go` for the sequences the reviews named (they are not one test per interleaving: no test runs two consoles, and a second actor is stood in for by a hook inside the fake provider or by an answer handed to the store by hand). Where the build differs from the text above, this is what it does and why. |
| -- **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. |
| +- **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; made structural after round twenty, which showed the round-nineteen build still chose, call site by call site, whether and against which intent to record). Every read of a checkout session or a subscription from the provider is made by one of three methods (`readCheckout`, `listCheckouts`, `readSubscription`), and each hands what it read to the store before its caller sees it; a test parses the package and fails on any other use of the client's three reads. The store finds the purchase by what the console itself recorded, never by what the answer claims: a session by its id, against the standing intent bound to it or referenced to it by its review, the retirement record, or the kept resolution's evidence (`purchaseOfSessionTx`); a subscription by the account and intent it names. It classifies in one transaction against the purchase as it is at that instant, standing, retired, or kept and consumed (`noteSessionSeen`, `noteSubscriptionSeen`), judging identity against the recorded account, intent and customer (the archives keep the customer), and records the session's fate, an attached subscription, or a dispute. A refusal and the subscription that caused it are one transaction (`fileCheckoutRefusal`); filing for an intent that went meanwhile classifies the subscription against the archive in that same transaction. What this does not cover, on purpose: the answers to a create and to an expiry (replays under an idempotency key, which prove nothing about the present; every decision after one is taken on a read or on the store's own checks of the answer), a read before any refusal review exists (there is no refused purchase for it to be evidence about; the reads that then file the refusal are handed in again once it exists), and an identity-less or still-open read before any fate is recorded (nothing is recorded to contradict; with no fate both outcomes are held). |
| +- **Where a refusal is filed that round nineteen's build answered bare** (review twenty, Opus P2-1): a subscription that names the bound standing intent and is not its completed session's own, on the purchase's own terms or not, files the refusal with itself attached whether or not a review existed; and one that names a standing intent never bound to a session files it with no session named. A completed session that names no subscription yet is a retry, as it is for the binder: it says nothing about which subscription is its own, and an honest purchase is not refused on it. |
| +- **A dispute is not resolvable.** The resolution door refuses a `billing_checkout_fate_disputed` review, so its hold on the account's invoices stays. A kept intent that is disputed is not offered to the buyer again and does not record. |
| +- **Events about a purchase that no longer stands are settled with the recorded account** (the binder: found by the session's id), in the rejected list, not retried for days; the one exception is the kept purchase's own completion delivered again, which is the account's recorded purchase. |
| - **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 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. |
| +- **The pass's refusal turn** runs under its own deadline whether the reconciliation finished, failed or ran out of time (both turns are children of the caller's context, so cancelling it stops either; production starts the pass, as it starts every console pass, on a context that is never cancelled, so in production a stop is the process ending mid-turn, which the protocol tolerates because each step is one transaction or is redone by the next pass), so slow reconciliation cannot leave a refused session payable pass after pass. |
| - **An unbound intent under a review that names no session** is still reconciled by the pass, which is how its session is found; nothing records or retires under the review on that path. |
| - **A retirement from before the protocol** (a retirement record with no fate) keeps its behaviour: its session's and its subscription's later events settle as rejected events. A subscription that names an intent retired on a proven expiry, and that session read back as anything but expired, are each a dispute for that session, once. |
| - **The reviews listing** shows each refusal's recorded fate, what is attached, and why `retire` and `keep` are or are not available. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review20.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review20.md| new file mode 100644 |
| index 000000000..4667c35ed |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review20.md |
| @@ -0,0 +1,222 @@ |
| +References are at **`43d326b5`**, except pricing-site references, which are at **`4026231`**. This was source-only: no modifications, tests, network commands or AWS commands. Test counterfactuals below are judgments from source inspection. |
| + |
| +**Nineteenth-review findings** |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1:** on-terms subscription omitted from obligations | **Closed for the reported sequence.** `noteSubscription` runs before reconciliation’s pending/subscription-mismatch answers. Capture across concurrent retirement remains incomplete; Finding 2 below. | **No** if that call is removed: `billingretirement_test.go:590` checks the attachment and the subsequent held `keep`. | |
| +| **Opus P3-1 / Astra 1, atomic filing:** refusal and attachment committed separately | **The original write boundary is fixed. The full invariant is not.** Both writes share `fileCheckoutRefusal`’s transaction, but its gone-intent branch drops the subscription. | **Yes for restoring separate commits.** The new test checks a separately constructed rollback and the helper’s successful result; it does not interrupt the helper. Removing the attachment entirely would fail. | |
| +| **Opus P3-2:** completion after proven-expiry retirement loses its dispute | **Closed when the response retains the expected account/intent identity.** Archived attribution still has gaps; Finding 3. | **No:** the added completion assertions at `billingretirement_test.go:261` fail without the binder’s retired-session evidence call. | |
| +| **Opus P3-3:** tie-break test admits `id DESC` | **Closed.** Arrival is `b,c,a`; expected output is `a,c,b` (`billingcurrency_test.go:4258`, `:4275`). | **No for either mutant.** Ascending ID gives `a,b,c`; descending gives `c,b,a`. | |
| +| **Opus P3-4:** disputed kept intent produces raw trigger error | **Closed as an error-classification defect.** Consumption explicitly requires `fate_disputed_at=0` (`billingstore.go:785`). | **No:** the third case of `TestEvidenceAfterAKeepSurvivesTheIntent` reaches recording and requires `errBillingConflict` (`billingretirement_test.go:839`). This conflict still produces webhook **503**, not settlement. | |
| +| **Opus P3-5:** tests overstate their assertions | **Partly closed.** The dispute test now checks a retirement that only the dispute prevents and actually attempts ordinary retirement. The reconciliation test is accurately renamed. The pass’s unasserted claim remains in its test comment. | Removing the relevant dispute protection fails the revised checks at `billingretirement_test.go:531`, `:548`. The pass test still does not establish its “not reconciled meanwhile” assertion. | |
| +| **Opus P3-6:** never-created shortcut precedes obligations; migrated resolved review can become protocol 1 | **Ordering fixed** at `billingretirement.go:195`. **Migration edge acknowledged, not fixed:** reopening still assigns protocol 1 (`billingstore.go:1097`). | No targeted never-created-plus-obligation test was added. Existing tests can pass with the old ordering. | |
| +| **Astra 1, remaining cases:** on-terms and structurally invalid subscriptions omitted | **Closed for those sequences**, with the concurrent disappearance gap in Finding 2. | **No** for omitting the on-terms attachment or restoring the two-item bare rejection: tests at `billingretirement_test.go:590`, `:635` distinguish them. | |
| +| **Astra 2:** keep accepts a changed subscription without recording it | **The specific changed-nonempty-subscription case is closed. Other keep-gate returns still discard evidence.** Finding 1. | **No** if the new successful-gate evidence call is removed. **Yes** with the early-return defects below: the test changes only the subscription ID. | |
| +| **Astra 3:** late/consumed/stale evidence disappears | **Partly closed.** The named terminal and subscription cases now work, and classification loads state transactionally. Several callers and archived identity checks remain outside that guarantee. | Removing the tested classifiers fails the new tests. Those tests still pass with Findings 1–3. | |
| +| **Astra 4:** reconciliation exhausts the refusal turn’s deadline | **Closed for deadline exhaustion.** Shutdown cancellation is broken; Finding 5. | **No** if refusals reuse the exhausted context. **Yes** with cancellation during the detached turn: the test only cancels before entry. | |
| +| **Astra 5:** pricing-site fractional-floor test checks source text | **Open at requested site `4026231`.** | **Yes** if the searched guard remains present but inert. Finding 7. | |
| + |
| +Both nineteenth reviewers were right about their respective tie-break mutants. Astra’s earlier unqualified “closed” was too broad; the three-ID fixture now closes both. |
| + |
| +Where nineteenth-review severities differed, **Astra’s P2 reading was correct**. The separate attachment commits could release a case with a collectible subscription, and discarded late evidence could prevent the required hold. Those are lifecycle failures, not merely diagnostic P3s. Both reviewers already agreed that the on-terms extra subscription was P2. |
| + |
| +**Nineteenth-review claims** |
| + |
| +| Claim | Assessment now | |
| +|---|---| |
| +| **Opus 1:** completion after expired retirement becomes a dispute | Implemented for unchanged attribution; still incomplete for contradictory identity. | |
| +| **Opus 2:** evidence survives kept-intent consumption | Implemented for the tested subscription/status cases; Finding 3 prevents the general claim. | |
| +| **Opus 3:** fate is written only from a read | The ordinary-retirement exception is now accurately stated in the retirement design at `:117`. | |
| +| **Opus 4:** only `retire` requests expiry | Corrected at `billingoperator.go:377`: `keep` can advance expiry too. | |
| +| **Opus 5:** dispute/pass/interleaving tests establish their descriptions | Partly corrected, as above; additional overstated test claims remain below. | |
| +| **Opus 6:** fractional-floor behavioral test exists at `4026231` | Attribution corrected: dispositions `:405` names later commit `01b62f7`. That does not fix the requested site snapshot. | |
| +| **Astra:** transactional, monotonic evidence makes acknowledgements safe | The individual classification transactions improve this substantially, but incomplete invocation and archived attribution still invalidate the overall claim. | |
| +| **Astra:** “one test per interleaving” overstates coverage | **Still open.** The design still says this at `:113`, despite dispositions `:406` saying it was corrected. | |
| +| **Astra:** `retire` comments describe settling paid purchases | **Still open:** `billingstore.go:885`, `:1008`; `billingservice.go:353`. Stage one does not authorize that disposition. | |
| +| **Astra:** dispute acknowledgement reads freshly and releases nothing | **Still false**, with an operational consequence in Finding 4. | |
| +| **Astra:** gate-failure reread is process-local | Revision 4 correctly supersedes that wording: it reads the shared database. | |
| +| **Astra:** historical mutation claims establish current coverage | The qualifications are mostly accurate; historical execution counts remain unverified, and the new atomicity test has a surviving mutation described below. | |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — Several provider reads still return before recording contradictory evidence, allowing retirement against superseded proof.** |
| + |
| + **Where:** `internal/console/billingoperator.go:251`; `internal/console/billingservice.go:648`, `:705`, `:997`; `internal/console/billingretirement.go:593`. |
| + |
| + **Concrete scenario:** |
| + |
| + 1. Refused intent I has recorded fate `expired`, no obligations and no dispute. |
| + 2. The operator asks to keep it. `advanceRefusal` skips the provider because a fate already exists. |
| + 3. The keep gate reads I’s pinned session as **complete and paid**, but with a wrong plan or another ownership contradiction. |
| + 4. The gate returns 409 at `billingoperator.go:252` or `:255`, **before** `noteSessionEvidence` at `:268`. |
| + 5. A later `retire` again skips the provider and accepts the old expired fate. The paid completion that should have prohibited retirement has vanished. |
| + |
| + This is sequential; no second console is required. |
| + |
| + Resume has another sequential counterexample: after `keep`, an owned session reads expired. The click calls ordinary retirement, which correctly conflicts with the recorded complete fate, but never records the contradiction. A later complete response can therefore record the purchase. Reconciliation’s matching-session read likewise returns pending or identity without recording its observation. |
| + |
| + **Smallest acceptable fix:** classify every successfully returned pinned-session observation using its known account/intent **before any semantic return**, including failed ownership, purchase, status and payment checks. Add tests through these actual doors, followed by attempted retirement or consumption. |
| + |
| +2. **P2 — The atomic refusal helper still drops its triggering subscription when another console disposes of the intent.** |
| + |
| + **Where:** `internal/console/billingservice.go:841`, `:898`; `internal/console/billingstore.go:1079`; `internal/console/billingretirement.go:449`. |
| + |
| + **Concrete interleaving:** |
| + |
| + 1. Console A reads subscription H naming I. No refusal exists yet, so `noteSubscription` correctly records nothing. |
| + 2. A reads the standing intent and its session, then pauses before `fileCheckoutRefusal`. |
| + 3. Console B files a session refusal, establishes expiry with no recorded obligations, and retires I. |
| + 4. A resumes. `fileCheckoutRefusal` sees that I no longer stands and returns **nil**, discarding H. |
| + 5. A’s session observation can agree with the archived expired fate, so it creates no dispute either. H’s event is acknowledged as rejected. |
| + |
| + The same omission applies if B keeps and consumes the purchase between A’s classification and filing. SQLite’s single writer makes each transaction atomic; it does not combine these separate transactions. |
| + |
| + **Smallest acceptable fix:** when filing finds the intent gone, classify the supplied subscription against the retirement/keep archive **inside that transaction**. Share the transactional classifier rather than returning success without retaining the evidence. |
| + |
| +3. **P2 — Late session evidence is attributed through mutable metadata or the replacement intent, and archives forget customer identity.** |
| + |
| + **Where:** `internal/console/billingservice.go:1155`, `:1186`, `:1198`, `:1239`; `internal/console/billingretirement.go:352`, `:384`, `:399`. |
| + |
| + **Concrete scenarios:** |
| + |
| + - Keep and consume I. Later its session reads complete with the same customer/subscription but edited `checkout_intent`. The binder’s recorded-subscription fast path returns before evidence classification. |
| + - After that purchase ends and a replacement intent J stands, I’s old session reads with another subscription. The binder hands the observation to the classifier using **J’s ID** at `:1239`. J’s pinned session differs, so the classifier silently ignores it. |
| + - With no replacement, an archived session reads with another customer but otherwise unchanged metadata, fate and subscription. Standing classification compares the customer; archived classification cannot, because neither archive lookup preserves it. `observeSession.Identity` deliberately excludes customer, leaving the contradiction unrecorded. |
| + |
| + A webhook’s lack of authority to bind or expire an unowned response does not prevent retaining a contradiction about a session already pinned in durable history. |
| + |
| + **Smallest acceptable fix:** locate the original purchase through the trusted session ID across standing intents, refusal references, retirements and kept-resolution evidence. Classify before fast returns. Preserve and compare the known customer identity in the archives. |
| + |
| +4. **P2 — Generic dispute resolution releases the invoice hold without the stage-one adjudication that is explicitly deferred.** |
| + |
| + **Where:** `internal/console/billingoperator.go:353`; `internal/console/billingstore.go:977`, `:992`; `internal/console/billinginvoicestore.go:165`. |
| + |
| + **Concrete scenario:** keep and record C, then observe another subscription naming its consumed intent. The new machinery correctly files `billing_checkout_fate_disputed`. An operator resolves that review through the generic API with a note. Only checkout-refusal reviews receive protocol-specific handling; the dispute gets no fresh provider read or held-state check. Its `resolved_at` is set, and the invoice hold disappears because `checkBillingHoldTx` checks only unresolved reviews. |
| + |
| + The durable dispute row remains, but it no longer holds invoicing. Repeating the same contradiction updates `last_seen_at` without reopening the review (`billingretirement.go:293`). |
| + |
| + **Smallest acceptable fix:** keep fate disputes holding throughout stage one. Either refuse their generic resolution or separate acknowledgement from the unresolved financial hold. Do not silently make acknowledgement an adjudication. |
| + |
| +5. **P3 — The refusal turn detaches from shutdown cancellation.** |
| + |
| + **Where:** `internal/console/billingpass.go:159`. |
| + |
| + **Concrete scenario:** reconciliation finishes and the refusal turn starts. The console’s parent context is then canceled. `context.WithoutCancel(ctx)` prevents that cancellation reaching the turn, so reads and expiry requests can continue for its 45-second deadline. If reconciliation’s deadline already expired, subsequent shutdown also cannot change that context’s error from `DeadlineExceeded` to `Canceled`. |
| + |
| + **Smallest acceptable fix:** give reconciliation and refusal work separate timeout children of the actual shutdown context. Test cancellation after refusal work starts and after reconciliation’s deadline expires. |
| + |
| +6. **P3 — The new atomicity test would pass with the original separate-commit defect.** |
| + |
| + **Where:** `internal/console/billingretirement_test.go:671`. |
| + |
| + **Concrete counterfactual:** change production `fileCheckoutRefusal` to commit the review and attachment separately. The test still passes: |
| + |
| + - Its rollback exercise manually starts another transaction and calls `fileBillingReviewTx`, not the production helper. |
| + - Its production-helper assertion checks only the final successful state. |
| + - Its post-retirement exercise calls `noteSubscription`, not `fileCheckoutRefusal`. |
| + |
| + The comment describes a trigger and another connection observing the filing. Neither exists in this test. |
| + |
| + **Smallest acceptable fix:** interrupt or fail the **production helper** after the review write and before attachment, then inspect durable state through another connection/reopen. Separately exercise filing after concurrent retirement/consumption. |
| + |
| +7. **P3 — The scoped pricing-site test still does not establish fractional-floor refusal.** |
| + |
| + **Where at site `4026231`:** `test/pricing-pricebook.test.mjs:246`; `copy-review/pricing/build.mjs:41`. |
| + |
| + **Concrete counterfactual:** retain the searched integer-guard text in a comment or unreachable branch. The test passes although fractional minor units no longer throw. |
| + |
| + **Smallest acceptable fix:** invoke the production formatter with fractional minor units and assert rejection, or render a malformed export fixture. The later commit cited by dispositions is outside this review’s site scope. |
| + |
| +**Claims, evidence coverage and remaining requested checks** |
| + |
| +The header’s invariant—`billingretirement.go:40`—is **not yet true**. The provider-read inventory is: |
| + |
| +| Path | Evidence handling and returns that precede it | |
| +|---|---| |
| +| Reconcile subscription reads | The first GET at `billingservice.go:806` is superseded without classification. Account/identity returns at `:815`, `:827`, `:830` precede classification of the second GET. Only a subscription different from the recorded one reaches `noteSubscription` at `:841`. | |
| +| Reconcile bound-session reads | Refusal’s read reaches evidence after atomic filing (`:898–904`). The matching-purchase read at `:997` has no evidence call before returns at `:1002`, `:1010`, `:1023`, `:1026`, or recording. | |
| +| Binder | Added calls cover several pending, retired, no-intent and standing branches. Provider-derived attribution, the recorded fast path and replacement-intent selection still lose evidence as in Finding 3. The pending branch also runs before the expected-intent ownership check. | |
| +| Pending pass | `billingpass.go:225` does not classify an owned response before dispatch. Complete triggers another read; expired goes to ordinary retirement; open can return at `:267`. An observation from the first read can disappear. | |
| +| Start/resume | Both reads at `billingservice.go:648`, `:705` omit classification on the owned path. Create-response success and early rejection paths do too. | |
| +| Recovery by create/listing | `billingservice.go:1455`, `:1477` can return or skip entries without classification. Completed mismatches reach it through `refuseSession`; other observations do not generally do so. | |
| +| Operator/keep gate | `advanceRefusal` records its GETs, but skips an existing fate. Keep records only after all guards pass; Finding 1. | |
| +| `advanceRefusal` | Matching-ID GETs reach classification at `billingretirement.go:610`, `:628`. Another returned ID is treated as a failed read. Expiry answers deliberately do not establish refusal fate; the subsequent GET does. | |
| +| `refuseSession` | Completed/paid refusal records evidence after filing. Open-session handling goes through expiry and ordinary retirement without classifying contradictory responses (`billingservice.go:382`). | |
| +| Quantity worker | `billingquantity.go:1010` reads the recorded subscription, with no `noteSubscription` before its returns. Ordinary reads concern an already-recorded purchase, but edited intent metadata naming another refused purchase is ignored. | |
| +| Invoice pass | `billinginvoice.go:558` reads the history-bound subscription, with no classification before returns at `:563`, `:570`, `:577`, `:581`, `:587`. Again, ordinary historical reads are legitimate; contradictory intent metadata is outside the asserted invariant. | |
| + |
| +Within their transactions, the classifiers now have useful, coherent behavior: |
| + |
| +- **Standing without a refusal:** both are no-ops. |
| +- **Standing with a refusal:** terminal owned evidence establishes fate; later incompatible fate/identity or changed nonempty subscription becomes a dispute. Identity/nonterminal observations before any fate are deliberately not retained. |
| +- **Subscription naming a standing refused intent:** another subscription attaches; a new attachment after `keep` reopens the review. The recorded session’s own subscription is excluded. |
| +- **Retired on proven expiry:** new subscriptions and incompatible session fate produce disputes when correctly attributed. |
| +- **Legacy retirement:** absent archived fate preserves historical rejection behavior. |
| +- **Kept and consumed:** other subscriptions and incompatible session fate are classified against archived session/subscription evidence, subject to Finding 3’s attribution and customer gaps. |
| + |
| +Thus neither “nothing observed is dropped” nor the comments’ full archived-identity promise is established. |
| + |
| +The local lifecycle protections remain sound **for evidence actually recorded**. First recording consumes the exact standing intent in the account/assignment/event/entitlement transaction; open refusal or disputed intent prevents consumption (`billingstore.go:785`). Failure rolls everything back. Renewals of an already-recorded subscription do not require another successful consumption. Ordinary expiry checks the exact intent/session, ownership, open review, fate and dispute. Stillborn cleanup requires an unready, unbound intent. Operator retirement archives and deletes exactly the reviewed intent and closes its matching checkout quantity events. Findings 1–4 are the remaining breaks in the stronger lifecycle guarantee. |
| + |
| +The delete barrier at `billingretirement.go:135` prevents an older delete of an unproven refused session, but does not enforce obligations or the entire protocol against old binaries. Stopping old consoles before migration and prohibiting rollback remain necessary. The operator door’s account lock, exact replay comparison, default `retire`, outcome audit, billing-off `keep` refusal, provider-failure 503 and held-state 409 behave as described. `keep` still checks the session rather than revalidating the subscription; the governing “Stage one as built” explicitly acknowledges that a subsequent reconciliation can reopen the review. |
| + |
| +The seven new retirement tests establish the following limits: |
| + |
| +| Test, in `billingretirement_test.go` | What it distinguishes | |
| +|---|---| |
| +| `TestAnOnTermsSubscriptionBesideTheSessionIsAttachedAndHoldsKeep` `:590` | Fails without on-terms evidence capture; no concurrent disposition. | |
| +| `TestAStructurallyInvalidSubscriptionNamingTheIntentIsReviewedAndAttached` `:635` | Fails with the two-item bare rejection. | |
| +| `TestTheRefusalAndItsSubscriptionAreOneTransaction` `:664` | Fails if attachment is omitted, **passes if the successful writes use separate commits**. | |
| +| `TestEvidenceAfterAKeepSurvivesTheIntent` `:730` | Distinguishes late extra subscription, changed/expired session, reopened review and disputed-record conflict in its specific fixtures. No replacement-intent or edited-attribution case. | |
| +| `TestTheKeepGatesReadIsRecordedAsEvidence` `:852` | Distinguishes changed subscription on an otherwise acceptable gate response. Does not cover any pre-classification rejection. | |
| +| `TestAContradictionReadBesideARecordedFateIsNotDropped` `:892` | Distinguishes transactional classification of a stale observation explicitly handed to the store. Does not prove callers hand it in. | |
| +| `TestThePassWorksADueRefusalThoughItsOwnTimeIsSpent` `:923` | Distinguishes exhausted deadline from preexisting cancellation. Does not cover cancellation during the turn. | |
| + |
| +The reworked proven-expiry test now genuinely checks late completion. The dispute test now attempts retirement. The renamed reconciliation test establishes refusal under a review that remains open, not successful concurrent retirement. The tie test distinguishes both ID orders. |
| + |
| +Earlier retirement tests still meaningfully establish their sequential hold/proof/barrier cases. However: |
| + |
| +- Payment-before-retire completes the fake **before** the operator request; it does not race payment inside expiry. |
| +- Protocol-zero coverage sets the marker manually; it does not test migration of that column. |
| +- The expiry hook invokes the store’s retirement with proof: a useful stand-in for the competing transaction, not two running consoles. |
| +- The outcome migration test really reopens an older table containing a row. The 200-row listing test checks count, endpoints and the first account, not every intermediate position/account. |
| +- Fixtures use `time.Now()` (`billingservice_test.go:147`); most schedule assertions use explicit offsets. I found no additional concrete shared-cookie, temporary-root or wall-clock failure. |
| + |
| +Several remaining prose claims need correction: |
| + |
| +- Design `:113` still says “one test per interleaving”; dispositions `:406` claims that wording was corrected. |
| +- The pass test’s “not reconciled meanwhile” remains at `billingretirement_test.go:389`. |
| +- The tie-test comment at `billingcurrency_test.go:4255` describes different ID orders from its correct fixture. |
| +- The raw-trigger-error fix does not settle disputed webhooks: `acknowledgeBillingRejection` only settles identity errors (`billinghandlers.go:315`); conflict remains 503. |
| +- The revised review-three/four qualifications correctly distinguish source-text provisioning checks, successful migration, redundant delete predicates and meaningful data mutations. Their historical mutation totals—and the new twenty-mutant total—are not independently established by source inspection. |
| + |
| +For the money, form, schema and export portions, I found no additional defect: |
| + |
| +- **GBP purchase:** `GET ?country=IN¤cy=gbp` quotes band D; Personal is £7.70 annually. With the required postal field, the posted effective choice and signed quote reach `startBillingCheckoutQuoted`, whose intent stores GBP/book terms (`billingservice.go:505`, `:610`). Checkout disables Adaptive Pricing for the book purchase. Binding/reconciliation validate purchase identity; account and book assignment retain those terms (`billingstore.go:756`). `billingInvoiceHistory`, `AllowanceForMonth`, V2 calculation, invoice requests/recovery and quantity-price selection retain GBP. I found no conversion or historical repricing path. |
| +- **One form and signature:** the server-secret-derived HMAC covers session token, account, book, country, band, currency, choice flag and member count (`billingpage.go:446`). Disclosed CSRF/quote fields cannot forge it. Another tuple/session/account/book/token cannot reuse it. Plan selects among already-priced rows; quantity follows membership and plan limits with the locked recheck; postal affects tax/location. Changed effective price redirects to a quote. Present invalid, empty or repeated currency is 400 on both doors. Omitted control uses the country default for book checkout and EUR for legacy checkout, subject to signed-quote agreement. |
| +- **Cookies and held currency:** present-empty clears, absent permits fallback, invalid writes nothing. An invalid visit can leave an old cookie available to a later parameter-absent visit; it cannot override the current posted form. A held purchase offers its stored currency alone, including pre-book EUR (`billingpage.go:301`, `billingservice.go:597`). The quote door only redirects to the authenticated account’s billing path. |
| +- **Ownership:** create/listing responses are not expired merely for naming an intent. The full ownership predicate governs acceptance. A contradictory bound session can be expired by the ID previously validated and stored; the response supplies no substitute authority or retirement proof. Leaving an unowned, never-established create/listing response untouched is correct. Evidence preservation is the separate defect above. |
| +- **Month barrier:** `billingstore.go:208` matches the positive paid span used at `plans/month.go:146`: active, non-Free, clipped by next assignment across both tables, paid-through and month end. Same-second supersession contributes no span. Renewal/quantity changes can move `effective_at`; `period_end` remains paid-through on assignment writes. I found neither requested valid-history counterexample. New-trigger existence is verified before transactional removal of the old name; interruption can leave both temporarily, not neither. Second startup is idempotent. |
| +- **Inclusions/configuration/export:** pooled allowances divided by charged quantity supply the displayed figures, including disabled Personal and teams above minimum. Current shares are exact at displayed precision; band A’s approved figures are pinned. Book validation and arithmetic checks, complete optional catalogs, unique price IDs, and own-only `currency_options` agree with provisioning. The export contains public commercial/resource/country definitions, not customer or credential data. |
| +- **History and operator listings:** `quantityOutcomesSQL` remains the shared admitted-outcome list; differing guards are replaced. No healing sweep clears pre-history or clock-rollback gaps. The rejected endpoint is operator-only, drains rows before account lookup, and returns at most 200 ordered by receipt then insertion. Some known-account bare identity errors still lose account attribution; retryable failures and non-webhook refusals are not automatically rows in this listing. |
| +- **Legacy retirement fixtures:** `retiredBeforeTheProtocol` reconstructs plausible old retirement effects. Its protocol marker remains 1, unlike a migrated old row, but the gone-intent paths tested do not consult it. The distinct replacement session and state/binding assertions establish non-recording of the old purchase; they do not authorize equivalent retirement under stage one. |
| + |
| +For **section K**, the site’s exported book at `4026231` is byte-identical to the platform export. Exact rates, annual prices, floors, band-scaled inclusions, country list and option defaults agree. The build checks scaling exactness and approved embedded figures. Draft output has the first-line marker; only `PRICEBOOK_ALLOW_DRAFT=1` admits it, and ordinary build/check regenerate before checking. |
| + |
| +`src/site.js:74` always carries both link parameters: currency only when explicitly chosen, country as selected. Locale is display-only. Consequently a locale suggestion can differ from the console’s subsequent default without sending a false choice. |
| + |
| +The slot tests check each slot against its own values across currencies/bands, and whole-token matching rejects embedded numeric matches. **A literal equal to the current correct exported value still passes:** those tests establish output equality, not provenance. A changed-export rebuild is needed to establish dependence on the export. Finding 7 is the remaining scoped site-test defect. |
| + |
| +**What must be verified against Stripe test mode before enablement** |
| + |
| +1. **Catalog and checkout:** all 36 price identities, amounts, currencies, quantities, tax settings and expanded own-currency options. Complete USD/EUR/GBP purchases, including IN/GBP and fractional-pound Community unit prices. Session, subscription, item and stored terms must agree; Adaptive Pricing must remain disabled. |
| +2. **Expiry and proof:** retained-URL payment racing expiry; concurrent/replayed keys, remembered errors, lost responses and expired idempotency retention. Successful expiry must prevent later payment, and subsequent GETs must supply the complete identity and terminal proof. An expiry response alone must not authorize refusal retirement. |
| +3. **Provider corrections and delayed payment:** metadata correction visibility, completed-but-unpaid sessions, `no_payment_required`, and session/subscription association behavior. Contradictory observations must remain durable through keep, retirement and consumption. |
| +4. **Refused obligations:** hand-made on-terms/wrong-price subscriptions, multiple items, additional subscriptions and both webhook orders. Expiring checkout must never settle another subscription; cancellation must not stand in for disposition of payments, collectible invoices or refunds. |
| +5. **Invoices and recovery:** create, recover, finalize and pay metered invoices in each currency. Verify rates, floors, line allocation, tax, waiver/discount handling and unchanged legacy EUR recovery/request identity. |
| +6. **Subscription lifecycle:** membership quantity changes, proration, renewal dates, cancellation, delayed events and customer-currency restrictions after an ended subscription. |
| + |
| +Separately, controlled local tests must exercise the actual transaction boundaries, two database connections/consoles, cancellation during refusal work, and every pre-classification return above. Stripe rehearsal cannot establish those database guarantees. |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md| index 313ebf98d..f0a982ef6 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -408,3 +408,46 @@ These are answered as one invariant, not as call sites patched one by one (the s |
| 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). |
| + |
| +## Review twenty (reviewed `43d326b5`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-multi-currency-review20.md`, `CODEX-FINDINGS-multi-currency-review20.md`. Both found the classification inside the store sound and the invariant still not true of the callers: round nineteen's build chose, call site by call site, whether to hand a read to the store and against which intent. Opus named one P2 and five P3s; Astra four P2s and three P3s. **Review eighteen's P2 stays recorded as open until a round accepts this code.** |
| + |
| +Where the severities differed, Astra's reading is taken: a read dropped at the keep gate let a later retire rest on a superseded expiry (a release), and a dispute resolved on a note lifted the invoice hold (a release); both are P2. |
| + |
| +The answer is one change of shape, not more call sites. No caller decides whether to record. Every read of a session or a subscription is made by one of three methods (`readCheckout`, `listCheckouts`, `readSubscription`) that hand it to the store before the caller sees it, and `TestEveryProviderReadGoesThroughTheRecordingMethods` parses the package and fails on any other use of the client's three reads. The store finds the purchase by the session's id in its own record (the standing intent, the review's reference, the retirement record, the kept resolution's evidence), never by the account or intent the answer names. |
| + |
| +| Finding | Disposition | Where | Counterexample test | |
| +| --- | --- | --- | --- | |
| +| Opus P2-1: a subscription on the purchase's own terms that names the bound intent, seen while its session is complete with its own subscription and no review exists, was a bare rejection, never attached; `keep` then passed beside it. The same for an intent never bound. | Fixed. A subscription that names the standing intent and is not its completed session's own files the refusal with itself attached (`refuse()`); one that names an unbound standing intent files it with no session named. | `billingservice.go` `reconcileBillingSubscription` | `TestAnOnTermsSubscriptionSeenBeforeAnyReviewIsAttachedAndHoldsKeep`, `TestASubscriptionNamingAnUnboundIntentIsReviewedAndAttached` | |
| +| Astra 1, Opus P3-1: reads returned before they were recorded: the keep gate's four refusals, the resume after a keep, reconcile's read of the matching session, the pending pass, recovery by listing. A session recorded expired and read complete at a refused keep left a later retire resting on the expiry. | Fixed by the shape: these are all `readCheckout` or `listCheckouts` now, recorded before any check. A kept intent that becomes disputed is not offered to the buyer again and does not record. | `billingretirement.go` `readCheckout`, `listCheckouts`; `billingoperator.go` `billingKeepRefused`; `billingservice.go` start path | `TestAReadTheKeepGateRefusesIsStillEvidence` (both the completion and the other customer's read), `TestAResumeThatContradictsAKeptFateIsADisputeAndNothingRecords` | |
| +| Astra 2, Opus claim 3: `fileCheckoutRefusal` returned nil for an intent that went between the caller's read and the filing, dropping the subscription. | Fixed: it classifies the subscription against the retirement record or the kept resolution in the same transaction. | `billingstore.go` `fileCheckoutRefusal`, `classifySubscriptionTx` | `TestTheRefusalAndItsSubscriptionAreOneTransaction`, second half | |
| +| Astra 3, Opus P3-2: late session evidence was attributed through the answer's own metadata, or against a replacement intent, and the archives kept no customer, so an edited or re-attributed answer about a kept or retired session recorded nothing. | Fixed: `purchaseOfSessionTx` finds the purchase by the session's id; identity is judged against the recorded account, intent and customer; the retirement record and the kept resolution carry the customer (`billing_retired_intents.customer`, `billing_review_resolutions.evidence_customer`). An answer that names another intent or account is a contradiction about this session. | `billingretirement.go` `noteSessionSeen` | `TestLateEvidenceIsAttributedByTheSessionTheConsoleRecorded` (three cases), `TestAContradictionReadBesideARecordedFateIsNotDropped` | |
| +| Astra 4, Opus claim 5: the generic resolution took a note for a `billing_checkout_fate_disputed` review, made no read, and lifted the invoice hold. | Fixed: a dispute review is not resolvable in stage one. The door answers 409 held and the hold stays. The design no longer says it can be acknowledged. | `billingstore.go` `resolveBillingReview` | `TestADisputeReviewIsNotResolvedByANote` | |
| +| Opus P3-3: after recording a dispute for an intent that is gone, the binder answered 503 for days; a subscription naming an ordinarily expired intent was retried until the provider gave up. | Fixed: an event about a purchase that no longer stands is settled with the recorded account (204, in the rejected list); the one exception is the kept purchase's own completion delivered again. A subscription with no intent standing is settled with its account. | `billingservice.go` `bindBillingCheckoutFor`, `reconcileBillingSubscription` | `TestEvidenceAfterAKeepSurvivesTheIntent` (the answers are asserted now), `TestLateEvidence...`, `TestASubscriptionNamingAnOrdinarilyExpiredIntentIsSettledWithItsAccount` | |
| +| Opus P3-4, Astra 6: the atomicity test tested SQLite's rollback, not the helper, and described a trigger it did not install. | Fixed: a trigger makes the attachment fail inside `fileCheckoutRefusal`; the test requires the helper to fail and leave no review. A mutant that commits the review before the attachment fails it. | `TestTheRefusalAndItsSubscriptionAreOneTransaction` | the test itself | |
| +| Astra 5, Opus P3-5: the refusals' turn was detached from the caller's cancellation, and "only the console stopping cancels it" described nothing production does. | Fixed: both turns are children of the caller's context, each with its own deadline. Stated: production starts every console pass on a context that is never cancelled, so in production a stop is the process ending, which each step tolerates. | `billingpass.go` `billingPass` | `TestThePassWorksADueRefusalThoughTheReconciliationsTimeIsSpent`: spent reconciliation deadline, cancelled before, cancelled inside the expiry | |
| +| Astra 7, Opus claim 8: the brief named site `4026231`, where the fractional-floor test still reads source text. | The behavioural test is at site `01b62f7` (review nineteen's row). Round twenty-one's brief names `01b62f7`. | site | | |
| +| Claims: design "hands in every subscription before any answer", "the keep gate hands in its own read", "nothing observed is dropped", "one test per interleaving"; comments saying `retire` settles money (`billingstore.go`, `refuseSession`); the garbled tie-break comment; the pass test's "not reconciled meanwhile". | Corrected: the design's "Stage one as built" states the invariant as built and what it does not cover; the comments say what stage one allows; the tie-break comment gives the three orders; the pass test's comment says what its last assertion establishes. | design, comments | | |
| + |
| +Added while answering, beyond what the reports asked: |
| + |
| +- The guard classifies every exported method of the provider client (`providerMethods` in `billingevidence_test.go`): a read of a session or a subscription has one recording wrapper or is marked unused, and anything else says why it is not such a read. `ListCheckouts` and `ListSubscriptions` return sessions and subscriptions, have no wrapper and are used nowhere in the console; a use of either fails the test. A method added to the client fails it until classified. |
| +- A completed session that names no subscription yet is a retry, not a refusal (`TestASubscriptionSeenBeforeItsSessionNamesOneIsRetriedNotRefused`): with the P2-1 fix a subscription that is not the session's own is refused, and an honest purchase's subscription read before the provider attached it to its session must not be. |
| +- A kept intent whose dispute is made by the click's own second read is not offered (`TestAClickWhoseSecondReadContradictsAKeptFateOffersNothing`). |
| + |
| +Mutation checks, in a tree copy (`git archive` of the tip plus the working files; nothing in the worktree was mutated), forty-three, each run against the lifecycle tests. Thirty-nine in the first batch, of which **four survived**, each because no test reached the line, not because the line was dead: |
| + |
| +| Survivor | Why it survived | Test added, which kills it | |
| +| --- | --- | --- | |
| +| identity ignores the intent the answer names | the only edited-answer test changed the intent and the account together | `TestAContradictionReadBesideARecordedFateIsNotDropped` now edits one identity clause at a time (intent, account in metadata, client reference, mode) | |
| +| an attachment after `keep` does not reopen the review | with the P2-1 fix reconcile's own refusal files the review again, so the classification's reopening was shadowed | `TestEvidenceAfterAKeepSurvivesTheIntent`, second case: the other subscription is still incomplete, reconcile answers pending before it judges it, and the read alone attaches and reopens | |
| +| the filing does not hand the session in again | every tested filing was followed by `advanceRefusal`, which reads again | `TestTheReadThatFilesARefusalRecordsItsFate`: the binder's one read files the refusal and records the completion, with one provider request | |
| +| a disputed kept intent is resumed at the second read | the first read always carried the dispute | `TestAClickWhoseSecondReadContradictsAKeptFateOffersNothing` | |
| + |
| +The other thirty-five died on the first run: the refusal filed without its subscription; the refusal and its subscription in two transactions; filing for a gone intent dropping the subscription; `readCheckout` and `readSubscription` not recording; a direct provider read in the keep gate; a subscription beside the complete session a bare rejection; an unbound intent named by a refused subscription filing nothing; a retired, and a kept, purchase not found by its session; identity ignoring the recorded customer; a dispute review resolved on a note; the binder judging an archived session by the account it names, against the standing intent, and retrying a not-complete one; a disputed kept intent resumed at the first read; a standing fate contradicted, an identity contradiction beside a recorded fate, a retired session read complete, a kept one read expired, a kept and a standing complete one naming another subscription, each not a dispute; a subscription naming a retired, and a kept and consumed, intent not a dispute; the record consuming a disputed kept intent; a two-item subscription a bare rejection; the refusals' turn on the reconciliation's context, and detached from the caller's; ties by id, both ways; the session's own subscription attached like any other; a subscription with no intent standing retried; the retirement record forgetting the customer; a disputed case retirable; the kept resolution forgetting its evidence. Four more were added with the tests above (the three other identity clauses; a complete session naming no subscription refusing the purchase) and die. The scripts are `retire-stage1/mutants3.sh` and `mutants3b.sh` in the run's scratch; their output is in the run directory's logs as `currency-mutants-round20.txt`. |
| + |
| +A mutant that dies shows a test reaches the line and asserts on its effect. It does not show the interleavings are covered: no test runs two console processes, and a fake provider is not the provider. |
| + |
| +Stated and not changed: a read made before any refusal review exists records nothing (there is no refused purchase for it to be evidence about; the read that leads to a filing is handed in again once the review exists); an identity-less or still-open read before any fate is recorded records nothing (with no fate both outcomes are held); the answers to a create and to an expiry are not evidence (replays under an idempotency key); the account's own recorded subscription is not classified, whatever intent its metadata names; a record refused because its kept intent is disputed is still a conflict the provider retries (503), as a record under an open review is, and stage one has nothing that ends it; a review resolved before the protocol and filed again becomes protocol 1 (unmerged builds only). |
| + |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review20.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review20.md| new file mode 100644 |
| index 000000000..a4c7308ca |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review20.md |
| @@ -0,0 +1,166 @@ |
| +# Twentieth review of `feat/carloku-multi-currency-20261001` at `43d326b5` (base `12e70d95`; delta from `59666bab`) |
| + |
| +**Verdict: accept with fixes.** The evidence invariant holds wherever a refusal review already exists. A store transaction now classifies every subscription that names a refused intent, every session read the binder makes, and the keep gate's read, and it does so against the purchase as it stands at that moment. The refusal and its subscription are written in one transaction. |
| + |
| +One P2 remains, in the same family as round nineteen's P2-1. A subscription that names the bound standing intent and is not its session's own subscription is still refused bare by reconcile when no review exists yet. The store's classification records nothing for a purchase with no refusal, so the subscription is never attached. A review filed later can then be kept beside it. I also found five P3s. |
| + |
| +The delta does not touch pricing, the page, the quote signature, the month trigger or the site. Those sections (A–Q, E/H/N, K) stand as rounds fifteen to nineteen traced them. Sources: `git show 43d326b5:` only, plus the site at `4026231`. I ran nothing and modified nothing. |
| + |
| +## Round nineteen, finding by finding |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1**: an on-terms subscription H beside the session was never attached | **Closed when the review exists before H arrives.** Reconcile calls `noteSubscription` before any answer (`billingservice.go:835-839`), and that attaches H (`billingretirement.go:449-466`). **Open when H arrives before any review**: see P2-1 below. | **No.** In `TestAnOnTermsSubscriptionBesideTheSessionIsAttachedAndHoldsKeep`, without the early `noteSubscription` the keep returns 200. | |
| +| **Opus P3-1**: the filing and the attachment were separate transactions | **Closed.** `fileCheckoutRefusal` (`billingstore.go:1072-1115`) does both in one transaction. Attachments are made only while the intent stands (`billingretirement.go:448`, `billingstore.go:1082`). | **Yes, for the atomicity half** (P3-4 below): the test passes if the filing is split into two transactions. **No** for the dispute-after-retirement half. | |
| +| **Opus P3-2**: a late completion after a retire on a proven expiry was settled with no dispute | **Closed.** The binder's retired branch hands the session in first (`billingservice.go:1207-1215`). | **No.** Without that call the late-completion dispute count is 0. | |
| +| **Opus P3-3**: the tie-break test could not fail for `id DESC` | **Closed.** The rows arrive as b, c, a. `rowid DESC` gives a, c, b; `id` gives a, b, c; `id DESC` gives c, b, a. | **No.** It fails for both id orders. | |
| +| **Opus P3-4**: a dispute after `keep` made the record abort on the barrier's raw error | **Closed.** The consumption now has `AND fate_disputed_at=0` (`billingstore.go:785`). | **No.** Subtest 3 would get the barrier's abort instead of `errBillingConflict`. | |
| +| **Opus P3-5**: tests that did not establish what they claimed | **Closed.** The dispute test now asserts "contradictory" on retire (the fate is expired with nothing attached, so only the dispute holds it) and calls `expireBillingCheckout`. The reconciliation test is renamed. The pass test's final zero-request pass now does establish "not reconciled meanwhile", after the fate is recorded. | **No** for the retire assertion and for the `expireBillingCheckout` call. | |
| +| **Opus P3-6**: `held` ordering | **Closed** by reordering (`billingretirement.go:195-200`). It has no test, and no code path can attach a subscription to a never-created intent. | n/a | |
| +| **Opus claims 1–6** | 1 and 2 are now true. 3 is stated; the exception is accurate because an ordinary retirement writes no `billing_retired_intents` row. 4 and 6 are corrected. 5 is only partly corrected (claim 7 below). | n/a | |
| +| **Astra 1 (P2)** | **Partly closed.** The two-transaction gap and the two-item subscription are closed. The on-terms subscription is closed only under an existing review (P2-1). | The two-item test fails without `refuse()` at `:914`. The atomicity test does not discriminate (P3-4). | |
| +| **Astra 2 (P2)**: the keep gate's fresh read was not recorded | **Closed** for reads that pass the gate's checks (`billingoperator.go:268-275`). Reads the gate refuses are still dropped (P3-1). | **No.** Without the call, `held("keep")` passes (fate complete, nothing attached) and the keep returns 200. | |
| +| **Astra 3 (P2)**: three paths that lost late evidence | **Closed for all three**: the late completion; a subscription naming a consumed intent that was kept (`billingretirement.go:477-487`); and the stale identity read, which the store now classifies against its own current state (`:350-355`). Residual paths are in P3-2. | **No.** `TestEvidenceAfterAKeepSurvivesTheIntent` and `TestAContradictionReadBesideARecordedFateIsNotDropped` each fail without their branch. | |
| +| **Astra 4 (P3)**: reconciliation could starve the refusals | **Closed** (`billingpass.go:161-165`). | **No.** Under the spent deadline the old loop broke before any refusal. | |
| +| **Astra 5 (P3)**: the site's fractional-floor test | **Open at `4026231`**, where `test/pricing-pricebook.test.mjs` still reads source. Closed at `01b62f7`. | n/a | |
| +| **Astra's claims** | The monotonicity claim and "one test per interleaving" are restated correctly. Still uncorrected: the dispute acknowledgement (claim 5 below) and the comments saying `retire` settles a paid purchase (`billingstore.go:1008-1009`; `billingservice.go` `refuseSession` doc, "resolving it retires the intent"). | n/a | |
| + |
| +**Where the reviewers disagreed, round nineteen:** |
| + |
| +- **The two-transaction filing (Opus P3-1, Astra P2):** Astra was right. A crash or another console between the two transactions released a hold with H outstanding, which is a P2 consequence. |
| +- **The keep gate's read (Astra 2, P2; Opus did not report it):** the read was real at `59666bab` and the release was real, so P2 is right. |
| +- **The completion after a proven expiry (Opus P3, Astra P2):** Opus was right. Nothing was released, and only the visibility was lost. |
| +- **The tie-break test:** both were right about different mutants, as the dispositions say. The three-id test is now right. |
| + |
| +## Findings |
| + |
| +### P2-1. A subscription naming the bound intent that is not its session's own is refused bare when no review exists yet, so it is never attached |
| + |
| +- **Where:** |
| + - `billingservice.go:1009`: `!own || session.SubscriptionID != id …` returns `errBillingIdentity`, which is checked before `intentSession` and before any filing. |
| + - `billingservice.go:872`: `refuse()` acts only for a bound intent. |
| + - `billingretirement.go:449`: `noteSubscription` returns nil when no review exists (`!st.Exists`). |
| +- **Scenario:** |
| + 1. Intent I is bound to session S. The buyer pays, creating subscription C. |
| + 2. Staff create H at Stripe by hand, on I's exact plan, band, terms, quantity and price, naming I. |
| + 3. H's `created`, `updated` and `invoice.paid` events all arrive while no review exists. `noteSubscription` records nothing. Reconcile reads S (complete, its subscription C) and answers bare identity: 204 and a rejected row each time. |
| + 4. C is then refused (for example, its quantity was edited at the provider). `refuse()` files the review with C attached. Staff correct C. |
| + 5. `keep` passes: the only obligation is C, which is the session's own. C records and I is consumed. |
| + 6. H bills for a year. Its next event becomes a dispute, but only then. The hold was released while the console had already seen H naming I. |
| +- **Without any review at all:** H is only ever rejected rows, because a consumed intent with no `keep` row classifies nothing. |
| +- **Unbound intent:** an off-terms H naming an unbound standing intent is bare too (`refuse()` needs `SessionID`). |
| +- **Smallest fix:** |
| + 1. At `:1009`, for an owned session whose subscription is not `id`, return `refuse()`. That files the review with H attached in `fileCheckoutRefusal`. |
| + 2. In `refuse()`, file for an unbound standing intent that the subscription names, with an empty reference. The pass then finds its session. |
| + 3. Test: deliver H while S is complete and no review exists, refuse C, then `keep` must answer 409 naming H. |
| + |
| +### P3-1. The keep gate drops every read it refuses |
| + |
| +- **Where:** `billingoperator.go:248-262`. The returns for "not the intent's own", "not the purchase", "not complete" and "unpaid" all come before `noteSessionEvidence` at `:268`. The door's `advanceRefusal` reads nothing once a fate is recorded (`billingretirement.go:585`). |
| +- **Scenario:** |
| + 1. The fate is recorded complete with C. |
| + 2. At the keep, the session reads as another account's, or reads open. The answer is 409 and no dispute is recorded. |
| + 3. A later keep, after the provider reads normally, passes. The contradiction the gate saw is gone. |
| +- **Fix:** hand the read in immediately after `GetCheckout` whenever `session.ID == intent.SessionID`, then run the checks. |
| + |
| +### P3-2. Identity-contradicted session reads on the webhook binder are still dropped |
| + |
| +- **Where:** `billingservice.go`: |
| + - `:1164`: the not-complete branch hands the read in only when the metadata account equals the client reference. |
| + - `:1190`: a bare return. |
| + - `:1236-1240`: when another intent stands, the read is handed in against the standing intent, not the intent the session names. |
| +- **Scenario A:** a session that was kept and consumed (or retired on a proven expiry) has its `account_sqid` metadata edited at Stripe. Its next event is refused at `:1190` with nothing recorded. |
| +- **Scenario B:** an account that cancelled a kept purchase and then started intent I2. A contradicting read of the old kept session is classified against I2's pin and dropped. |
| +- **Fix:** classify by session id. The store already knows the pinned session through the intent row, `billing_retired_intents.session_id` and `evidence_session`, so it does not need the response's metadata to find the purchase. |
| + |
| +### P3-3. The binder answers 503 for days after recording a dispute for a gone intent |
| + |
| +- **Where:** `billingservice.go:1219-1226` returns `sql.ErrNoRows`, and `:1170` returns `errBillingPending`. Neither is `errBillingIdentity`. |
| +- **Scenario:** in `TestEvidenceAfterAKeepSurvivesTheIntent` subtest 1: |
| + - The kept session reads `sub_changed`. The dispute is recorded, the webhook answers 503, and Stripe retries for three days. The event never reaches the rejected list. The test ignores the status code. |
| +- **Related:** an on-terms H naming an intent that expired the ordinary way gets `sql.ErrNoRows` at `:971`, and is retried until Stripe gives up, in no list anywhere (section ZZ). |
| +- **Fix:** after the evidence is recorded for an intent that does not stand, return `&billingIdentityOfAccount{acct.ID}`. |
| + |
| +### P3-4. `TestTheRefusalAndItsSubscriptionAreOneTransaction` does not test that |
| + |
| +- **Where:** `billingretirement_test.go:675-692`. It rolls back a hand-made transaction around `fileBillingReviewTx`, which tests SQLite rollback. It then calls `fileCheckoutRefusal` and checks the end state. Both pass if `fileCheckoutRefusal` is two transactions. |
| +- **Also:** the comment describes "a trigger on the reviews table" that the test does not install. The dispositions' twenty mutants do not include splitting the transaction. |
| +- **Fix:** create a temporary `BEFORE INSERT ON billing_review_obligations … RAISE(ABORT)` trigger. Assert that `fileCheckoutRefusal` errs and leaves no review row. |
| + |
| +### P3-5. The refusal turn's "only the console stopping cancels it" is vacuous |
| + |
| +- **Where:** `billingpass.go:161-165` and `cmd/console/main.go:351`, which passes `go srv.StartBillingPass(context.Background())`. |
| +- **What happens:** production never cancels the pass. A turn already running ignores cancellation (`WithoutCancel`). A context past its deadline keeps `DeadlineExceeded` even if its parent is later cancelled. |
| +- **Consequence:** harmless, because a stop is a process death and the protocol tolerates it. But the second half of the test exercises a path production never takes. |
| +- **Fix:** say so, or pass `serve`'s context. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **Design `:115`:** "reconcile hands in every subscription that is not the account's recorded one before any pending, identity or retired answer." |
| + - Three identity returns precede it (`billingservice.go:810-830`). |
| + - With no review the hand-in classifies nothing, and `:1009` decides from reconcile's own read (P2-1). |
| +2. **Design `:115`:** "the keep gate hands in its own read." Only reads that pass four checks are handed in (P3-1). |
| +3. **Design `:115`:** "Nothing observed is dropped." P2-1, P3-1 and P3-2 contradict it. In the same vein, `fileCheckoutRefusal` returns nil for an intent that no longer stands (`billingstore.go:1082`) instead of classifying the subscription as `noteSubscription` would. |
| +4. **Design `:121`:** "only the console stopping cancels it" (P3-5). |
| +5. **Design `:101`:** "acknowledging it with a note after a fresh read (it never … releases anything)." Resolving a `billing_checkout_fate_disputed` review does no read and lifts that review's invoice hold. This is Astra's round-nineteen claim, still unaddressed. |
| +6. **Comments:** |
| + - `billingstore.go:1008` ("retire is the operator saying the money is settled") and `refuseSession`'s doc ("resolving it retires the intent") describe something stage one forbids for a completed session. |
| + - The tie-break test's comment ("c, a, b reversed (b, a, c…") is garbled; its assertion is right. |
| +7. **Dispositions `:403`:** "the pass test's unasserted claim is withdrawn from this file." The comment is still at `billingretirement_test.go:389-391`. It is now true after the fate is recorded, not before. |
| +8. **Dispositions, Astra 5 row:** the commit this brief names (`4026231`) still has the source-text test. |
| + |
| +## Requested sections, the evidence invariant |
| + |
| +**MMM. Every path that reads a session or subscription from the provider, and whether the read reaches the store before an answer:** |
| + |
| +- **Reconcile:** yes for any subscription that names an intent with an existing refusal review. The exceptions: |
| + - An unknown account, an account mismatch on the second read, and a bad id, mode or customer (`:810-830`). These are nobody's purchase. |
| + - With no review, everything (P2-1). |
| + - In the record path, reads of the bound session that return before handing it in: `:1001` (not complete) is a retry; `:1009` and `:1018` are covered by P2-1 and by the pass. |
| +- **Binder:** see P3-2 for the misses. The fast path at `:1196` returns only for the account's own recorded purchase, so nothing is lost there. |
| +- **The pass:** |
| + - Refused intents go through `advanceRefusal`, which hands in both reads. |
| + - A pending intent goes through `ownBoundSession` and then the binder. |
| + - Recovery by listing binds only an owned session. |
| +- **The start and resume path:** `ownBoundSession` files first, then hands in the read (`:329`). |
| +- **The operator door:** `advanceRefusal`, then the keep gate (P3-1). |
| +- **`refuseSession`:** the completed branch hands the session in. The open branch goes through `expireBillingCheckout`'s proof checks. |
| +- **The quantity worker and the invoice pass:** they read only recorded subscriptions, never a refused purchase. |
| + |
| +**NNN. `noteSessionObservation` and `noteSubscription`, branch by branch:** |
| + |
| +- Standing with a fate: open, identity-less, the other fate, or another subscription are each a dispute. |
| +- Standing with no fate: an identity-less read records nothing. That is safe, because no fate means retire and keep are both held. |
| +- Retired on a proven expiry: anything but expired with identity is a dispute. A retirement from before the protocol, or an ordinary expiry, has no `expired` archive and records nothing. |
| +- Kept and standing: an attachment reopens the review. |
| +- Kept and consumed: compared with `evidence_*`. The columns are empty for keeps made by `59666bab`, which ran only on unmerged builds with billing off. |
| +- No refusal: nothing is recorded. |
| +- A legitimate late event cannot become a dispute. The recorded subscription is excluded at the call site, and kept or expired evidence that agrees is a no-op. |
| +- Wrong account: none. Every lookup is keyed by the account and intent and checks the pinned session. |
| +- Minor: the retired and kept branches do not compare `o.Customer`, because the archive holds no customer. |
| + |
| +**OOO.** Each classification and the resolution are single SQLite transactions, which are serializable. A deferred transaction that loses a race errs and is retried, never misclassified. Every interleaving I traced with a retire or keep commit lands in the dispute or reopen branch. The only crash gap left is a caller dying between its provider read and its hand-in: for webhooks the provider redelivers, and for the pass the next pass reads again. |
| + |
| +**PPP.** A subscription seen while a kept intent stands reopens the review, and a disputed kept intent is a conflict at the record. After consumption, only P3-2's metadata-edited reads can still vanish. |
| + |
| +**QQQ.** Reconciliation is capped at two minutes, then refusals get 45 seconds with a budget of three. A due check costs nothing. Neither can starve the other. On shutdown, see P3-5. |
| + |
| +**RRR.** |
| +- These fail without their code: on-terms, two-item, keep-gate, after-keep (all three subtests), stale-read, pass-deadline, and the late completion. |
| +- `OneTransaction` is the exception (P4). |
| +- No test runs two consoles, as the dispositions say. The stale-read test stands in for a second actor by building the observation by hand, which is a fair stand-in, because the classification under test is entirely in the store. |
| + |
| +**DDD** (review eighteen's P2): |
| +- **Closed:** every sequential, crash and two-console ordering once a review exists. |
| +- **Still open:** P2-1, a hold released beside a subscription the console saw before any review existed. |
| + |
| +## Verify against Stripe test mode before enabling |
| + |
| +Every earlier list stands. Add these: |
| + |
| +1. A subscription made by hand on the exact price, naming the intent, created after the session completes and before any review exists. After the P2-1 fix, its first event files the review with it attached, and `keep` answers 409. |
| +2. Editing a completed session's `account_sqid` metadata. Confirm whether Stripe allows it, and what the binder then records (P3-2). |
| +3. The webhook's answer to a `checkout.session.completed` for a consumed kept session whose subscription reads differently. After the P3-3 fix it should be 204 with a rejected row. |
| +4. Whether `client_reference_id` can change after creation. The binder's identity rests on it. |
| + |
| +**Verdict: accept with fixes.** Fix P2-1; then P3-1 to P3-5 and claims 1–8. |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index f86f2cf12..4920ee5a7 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -4252,9 +4252,9 @@ func TestAnOlderResolutionRowReadsAsRetire(t *testing.T) { |
| // was untested.) |
| func TestTheRejectedListingBreaksTiesNewestFirst(t *testing.T) { |
| fx, f, acct := newCurrencyFixture(t) |
| - // 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). |
| + // Three rows in one second, arriving b, c, a: neither ascending nor |
| + // descending by id. Newest first is the reverse of arrival, a, c, b; |
| + // ordering by id gives a, b, c and by id descending c, b, a. |
| 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 { |
diff --git a/internal/console/billingevidence_test.go b/internal/console/billingevidence_test.go| new file mode 100644 |
| index 000000000..8bb6cd64f |
| --- /dev/null |
| +++ b/internal/console/billingevidence_test.go |
| @@ -0,0 +1,650 @@ |
| +package console |
| + |
| +import ( |
| + "context" |
| + "errors" |
| + "fmt" |
| + "go/ast" |
| + "go/parser" |
| + "go/token" |
| + "net/http" |
| + "path/filepath" |
| + "reflect" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/billingstripe" |
| + "github.com/carlosframework/platform/internal/plans" |
| +) |
| + |
| +// The evidence invariant of the retirement protocol (billingretirement.go), |
| +// through the doors that read the provider: review twenty's |
| +// counterexamples, each driven through the production path that makes the |
| +// read, and followed by the disposition the lost evidence would have let |
| +// through. |
| + |
| +// keptRefusal opens a GBP purchase, has its session read back as another |
| +// customer's (the refusal), then corrected, completed and paid, and has an |
| +// operator keep it: the intent stands, its review resolved, its fate |
| +// recorded complete with sub_paid, nothing yet recorded as a purchase. |
| +func keptRefusal(t *testing.T, code string) (*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) |
| + 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 |
| +} |
| + |
| +// retiredOnAProvenExpiry opens a GBP purchase, has it refused, its expiry |
| +// proven and an operator retire it. |
| +func retiredOnAProvenExpiry(t *testing.T, code string) (*consoleFixture, *fakeBillingStripe, *Account, billingIntent, string) { |
| + t.Helper() |
| + fx, f, acct, intent := refusedByAnotherCustomer(t) |
| + f.mu.Lock() |
| + f.sessionCustomer = "" |
| + f.mu.Unlock() |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", code) |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "", "expired, never paid"); status != http.StatusOK { |
| + t.Fatalf("retire: %d %q", status, msg) |
| + } |
| + var fate, customer string |
| + if err := fx.st.db.QueryRow(`SELECT session_fate,customer FROM billing_retired_intents WHERE account_id=? AND intent_id=?`, acct.ID, intent.ID).Scan(&fate, &customer); err != nil || fate != sessionFateExpired || customer == "" { |
| + t.Fatalf("fixture: the retirement record: fate %q customer %q %v; want the proven expiry and the purchase's customer", fate, customer, err) |
| + } |
| + return fx, f, acct, intent, token |
| +} |
| + |
| +func disputesOf(t *testing.T, fx *consoleFixture, acct *Account, sessionID, 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, sessionID, like).Scan(&n); err != nil { |
| + t.Fatal(err) |
| + } |
| + return n |
| +} |
| + |
| +// rejectedFor is how many rejected webhook events are recorded with the |
| +// account: an event settled with its account reaches the operator's list |
| +// under it. |
| +func rejectedFor(t *testing.T, fx *consoleFixture, acct *Account, eventID string) int { |
| + t.Helper() |
| + var n int |
| + if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_rejected_events WHERE id=? AND account_id=?`, eventID, acct.ID).Scan(&n); err != nil { |
| + t.Fatal(err) |
| + } |
| + return n |
| +} |
| + |
| +// A subscription made by hand on the purchase's own terms, naming the |
| +// intent, seen while the intent's session is complete with its own |
| +// subscription and no review exists yet: it is not a bare rejection. The |
| +// refusal is filed with it attached, and a keep of the checkout's purchase |
| +// is held beside it. (Review twenty, Opus P2-1.) |
| +func TestAnOnTermsSubscriptionSeenBeforeAnyReviewIsAttachedAndHoldsKeep(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) |
| + // The buyer paid: the session is complete, and its own subscription |
| + // is sub_checkout. sub_paid is the one made by hand. |
| + f.mu.Lock() |
| + f.sessionStatus, f.sessionSubscription, f.subscriptionPrice = "complete", "sub_checkout", intent.Price |
| + f.otherSubscriptions = map[string]bool{"sub_checkout": true} |
| + f.mu.Unlock() |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Exists { |
| + t.Fatalf("fixture: a review already exists: %+v", st) |
| + } |
| + if code := f.webhookPost(t, fx, "evt_hand_before_review", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the hand-made subscription: %d; want it settled as refused", code) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || st.Protocol != 1 || len(st.Obligations) != 1 || st.Obligations[0] != "sub_paid" || st.Fate != sessionFateComplete || st.SessionSubscription != "sub_checkout" { |
| + t.Fatalf("state %+v %v; want the review open, sub_paid attached, the session recorded complete with sub_checkout", st, err) |
| + } |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-before-review") |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "keep", "paid"); status != http.StatusConflict || !strings.Contains(msg, "sub_paid") { |
| + 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 completed session that carries no subscription id yet says nothing |
| +// about which subscription is its own: the purchase's subscription, seen |
| +// then, is retried, not refused, and no review is filed on an honest |
| +// purchase. Once the session names it, it records. |
| +func TestASubscriptionSeenBeforeItsSessionNamesOneIsRetriedNotRefused(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.sessionStatus, f.sessionNoSubscription, f.subscriptionPrice = "complete", true, intent.Price |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_before_attached", "customer.subscription.created", "sub_paid"); code == http.StatusNoContent || code == http.StatusOK { |
| + t.Fatalf("the purchase's subscription before its session names one: %d; want it retried", code) |
| + } |
| + if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Exists { |
| + t.Fatalf("a review was filed on an honest purchase: %+v", st) |
| + } |
| + f.mu.Lock() |
| + f.sessionNoSubscription = false |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_attached", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the purchase's subscription once its session names it: %d", code) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "sub_paid" { |
| + t.Fatalf("recorded: %+v; want the purchase", state) |
| + } |
| +} |
| + |
| +// The same for an intent that was never bound to a session (its creation |
| +// answer was lost): a subscription that names it and is not the purchase |
| +// files the refusal with it attached and no session named. (Review |
| +// twenty, Opus P2-1, the unbound half.) |
| +func TestASubscriptionNamingAnUnboundIntentIsReviewedAndAttached(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + f.mu.Lock() |
| + f.failCreate = true |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err == nil { |
| + t.Fatalf("fixture: the checkout went through: %q", dest) |
| + } |
| + intent, err := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || intent.SessionID != "" { |
| + t.Fatalf("fixture: intent %+v %v; want it standing and unbound", intent, err) |
| + } |
| + f.mu.Lock() |
| + f.subscriptionPrice = currencyPriceID(plans.PlanPersonal, plans.BandD, plans.GBP) |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_hand_unbound", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the hand-made subscription: %d; want it settled as refused", code) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || st.Protocol != 1 || st.Reference != "" || len(st.Obligations) != 1 || st.Obligations[0] != "sub_paid" { |
| + t.Fatalf("state %+v %v; want the review open with sub_paid attached and no session named", st, err) |
| + } |
| + if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil { |
| + t.Fatal(err) |
| + } |
| + heldRetire(t, fx, acct, intent.ID, "paul@keymail.dev", "sub_paid", f.now.Add(time.Hour)) |
| +} |
| + |
| +// The keep gate's read is recorded whatever the gate answers: a session |
| +// recorded expired that reads complete and paid at the keep is a dispute, |
| +// though the keep is refused for another reason, and the retire that the |
| +// recorded expiry would have allowed is held. (Review twenty: Astra 1, |
| +// Opus P3-1.) |
| +func TestAReadTheKeepGateRefusesIsStillEvidence(t *testing.T) { |
| + fx, f, acct, intent := refusedByAnotherCustomer(t) |
| + 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 || len(st.Obligations) != 0 { |
| + t.Fatalf("fixture: %+v", st) |
| + } |
| + // At the keep the provider shows the session complete and paid. |
| + f.mu.Lock() |
| + f.sessionStatus, f.subscriptionPrice = "complete", intent.Price |
| + f.mu.Unlock() |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-gate-refuses") |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "keep", "paid"); status != http.StatusConflict { |
| + t.Fatalf("keep of a session recorded expired: %d %q; want 409", status, msg) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "read back complete after it was recorded expired"); n != 1 { |
| + t.Fatalf("%d disputes for the completion the gate read; want one", n) |
| + } |
| + // The provider shows it expired again. Without the dispute the |
| + // recorded expiry would allow the retire. |
| + f.mu.Lock() |
| + f.sessionStatus = "expired" |
| + f.mu.Unlock() |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "", "expired"); status != http.StatusConflict || !strings.Contains(msg, "contradictory") { |
| + t.Fatalf("retire after the gate read a completion: %d %q; want 409 held as contradictory", status, msg) |
| + } |
| + if standing, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || standing.ID != intent.ID { |
| + t.Fatalf("the intent: %+v %v; want it standing", standing, err) |
| + } |
| + // The same when the gate's read is of another customer's session. |
| + fx2, f2, acct2, intent2 := refusedByAnotherCustomer(t) |
| + f2.mu.Lock() |
| + f2.sessionCustomer = "" |
| + f2.mu.Unlock() |
| + if err := fx2.srv.advanceRefusal(context.Background(), intent2, acct2, f2.now.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + f2.mu.Lock() |
| + f2.sessionCustomer = "cus_other" |
| + f2.mu.Unlock() |
| + token2 := billingOperatorToken(t, fx2, "ops@keymail.dev", "code-billing-op-gate-refuses-2") |
| + if status, msg := resolveRefusal(t, fx2, token2, acct2, intent2, "keep", "paid"); status != http.StatusConflict { |
| + t.Fatalf("keep read back as another customer's: %d %q; want 409", status, msg) |
| + } |
| + if n := disputesOf(t, fx2, acct2, intent2.SessionID, "read back as another account's, customer's or intent's after it was recorded expired"); n != 1 { |
| + t.Fatalf("%d disputes for the other customer's read at the gate; want one", n) |
| + } |
| +} |
| + |
| +// After a keep, the buyer's click reads the session expired: the read is |
| +// a dispute, nothing is offered or retired, and a completion the provider |
| +// shows afterwards does not record. (Review twenty, Astra 1, the resume.) |
| +func TestAResumeThatContradictsAKeptFateIsADisputeAndNothingRecords(t *testing.T) { |
| + fx, f, acct, intent, _ := keptRefusal(t, "code-billing-op-resume") |
| + f.mu.Lock() |
| + f.sessionStatus = "expired" |
| + f.mu.Unlock() |
| + dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(time.Hour)) |
| + if dest != "" || !errors.Is(err, errBillingPending) { |
| + t.Fatalf("the click: dest %q err %v; want nothing offered, under review", dest, err) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "read back expired after it was recorded complete"); n != 1 { |
| + t.Fatalf("%d disputes for the expiry the click read; want one", n) |
| + } |
| + if standing, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || standing.ID != intent.ID { |
| + t.Fatalf("the intent: %+v %v; want it standing", standing, err) |
| + } |
| + // The provider shows the completion again: every check of the |
| + // reconcile passes, and the record is refused as a conflict. |
| + f.mu.Lock() |
| + f.sessionStatus = "complete" |
| + f.mu.Unlock() |
| + if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "evt_after_resume", f.now.Add(2*time.Hour)); !errors.Is(err, errBillingConflict) { |
| + t.Fatalf("the record of the disputed purchase: %v; want a conflict", err) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded a disputed purchase: %+v", state) |
| + } |
| + // And a click now is not offered the old link either. |
| + f.mu.Lock() |
| + f.sessionStatus = "open" |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(3*time.Hour)); dest != "" || err == nil { |
| + t.Fatalf("a click on the disputed purchase: dest %q err %v; want nothing offered", dest, err) |
| + } |
| +} |
| + |
| +// A disputed kept intent is not offered again even when the dispute is |
| +// made by the click's own second read: the first read of the kept session |
| +// agrees with the record, the second shows it open again, and the link is |
| +// not handed out on the strength of the first. |
| +func TestAClickWhoseSecondReadContradictsAKeptFateOffersNothing(t *testing.T) { |
| + fx, f, acct, intent, _ := keptRefusal(t, "code-billing-op-second-read") |
| + f.mu.Lock() |
| + f.sessionAnswers = []func(*fakeBillingStripe){ |
| + func(*fakeBillingStripe) {}, |
| + func(f *fakeBillingStripe) { f.sessionStatus = "open" }, |
| + } |
| + f.mu.Unlock() |
| + dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(time.Hour)) |
| + if dest != "" || !errors.Is(err, errBillingPending) { |
| + t.Fatalf("the click: dest %q err %v; want nothing offered, under review", dest, err) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "read back open after it was recorded complete"); n != 1 { |
| + t.Fatalf("%d disputes for the open read; want one", n) |
| + } |
| +} |
| + |
| +// The read that leads to a refusal is the first evidence of the session's |
| +// fate, though it was made before the review existed: handed in again at |
| +// the filing, it records the completion at once, without waiting for a |
| +// pass to read the session again. |
| +func TestTheReadThatFilesARefusalRecordsItsFate(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) |
| + // The session completes in another currency than it was asked for. |
| + f.mu.Lock() |
| + f.sessionStatus, f.sessionCurrency, f.subscriptionPrice = "complete", "eur", intent.Price |
| + f.mu.Unlock() |
| + requests := func() int { f.mu.Lock(); defer f.mu.Unlock(); return f.requests } |
| + before := requests() |
| + if code := f.webhookPost(t, fx, "evt_done_eur", "checkout.session.completed", intent.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the completion in another currency: %d; want it settled as refused", code) |
| + } |
| + if n := requests() - before; n != 1 { |
| + t.Fatalf("the binder made %d provider requests; want the one read", n) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, intent.ID) |
| + if err != nil || !st.Open || st.Fate != sessionFateComplete || st.SessionSubscription != "sub_paid" { |
| + t.Fatalf("state %+v %v; want the review open and the completion recorded from the read that filed it", st, err) |
| + } |
| +} |
| + |
| +// Late evidence about a purchase that no longer stands is found by the |
| +// session's id in the console's own record, whatever the answer says of |
| +// itself and whatever intent stands now, and its event is settled with |
| +// the recorded account. (Review twenty: Astra 3; Opus P3-2 and P3-3.) |
| +func TestLateEvidenceIsAttributedByTheSessionTheConsoleRecorded(t *testing.T) { |
| + t.Run("a kept session whose metadata was edited", func(t *testing.T) { |
| + fx, f, acct, intent, _ := keptRefusal(t, "code-billing-op-late-1") |
| + 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) |
| + } |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); err == nil { |
| + t.Fatal("fixture: the intent still stands after its purchase recorded") |
| + } |
| + f.mu.Lock() |
| + f.sessionMeta = map[string]string{"checkout_intent": "ci_edited", "account_sqid": "zzzz"} |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_kept_edited", "checkout.session.completed", intent.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the kept session read with edited metadata: %d; want it settled", code) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "read back as another account's, customer's or intent's after its purchase was kept%"); n != 1 { |
| + t.Fatalf("%d disputes; want one", n) |
| + } |
| + if n := rejectedFor(t, fx, acct, "evt_kept_edited"); n != 1 { |
| + t.Fatalf("%d rejected rows under the account; want the event settled with the recorded account", n) |
| + } |
| + // The unedited completion, delivered again, is the account's |
| + // recorded purchase and no dispute. |
| + f.mu.Lock() |
| + f.sessionMeta = nil |
| + f.mu.Unlock() |
| + before := disputesOf(t, fx, acct, intent.SessionID, "%") |
| + if code := f.webhookPost(t, fx, "evt_kept_again", "checkout.session.completed", intent.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the kept session's completion again: %d", code) |
| + } |
| + if after := disputesOf(t, fx, acct, intent.SessionID, "%"); after != before { |
| + t.Fatalf("the agreeing completion made %d disputes", after-before) |
| + } |
| + }) |
| + t.Run("a retired session read complete while a replacement intent stands", func(t *testing.T) { |
| + fx, f, acct, intent, _ := retiredOnAProvenExpiry(t, "code-billing-op-late-2") |
| + // A new purchase: another intent, bound to another session. |
| + f.mu.Lock() |
| + f.sessionID, f.sessionStatus = "cs_test_second", "open" |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(30*time.Minute)); err != nil || dest == "" { |
| + t.Fatalf("the replacement click: dest %q err %v", dest, err) |
| + } |
| + replacement, err := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || replacement.ID == intent.ID || replacement.SessionID != "cs_test_second" { |
| + t.Fatalf("fixture: replacement %+v %v", replacement, err) |
| + } |
| + // The old session now reads complete, and names the replacement |
| + // intent (the fake's metadata is the last checkout's). |
| + f.mu.Lock() |
| + f.sessionID, f.sessionStatus, f.subscriptionPrice = "", "complete", intent.Price |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_old_done", "checkout.session.completed", intent.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the old session's completion: %d; want it settled", code) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "%after its intent was retired on a proven expiry"); n != 1 { |
| + t.Fatalf("%d disputes of the retired session; want one", n) |
| + } |
| + if n := rejectedFor(t, fx, acct, "evt_old_done"); n != 1 { |
| + t.Fatalf("%d rejected rows under the account; want one", n) |
| + } |
| + st, err := fx.st.refusalState(acct.ID, replacement.ID) |
| + if err != nil || !st.Stands || st.Exists || st.Disputed != 0 { |
| + t.Fatalf("the replacement after the old session's event: %+v %v; want it standing, unreviewed, undisputed", st, err) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded from the retired session: %+v", state) |
| + } |
| + }) |
| + t.Run("a retired session read as another customer's", func(t *testing.T) { |
| + fx, f, acct, intent, _ := retiredOnAProvenExpiry(t, "code-billing-op-late-3") |
| + // Everything as it was archived (expired, unpaid, this account |
| + // and intent) but the customer. |
| + f.mu.Lock() |
| + f.sessionCustomer = "cus_other" |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_old_other_customer", "checkout.session.completed", intent.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the old session read as another customer's: %d; want it settled", code) |
| + } |
| + if n := disputesOf(t, fx, acct, intent.SessionID, "read back as another account's, customer's or intent's after its intent was retired on a proven expiry"); n != 1 { |
| + t.Fatalf("%d disputes; want one", n) |
| + } |
| + // Read as it was archived, it is no dispute. |
| + fx2, f2, acct2, intent2, _ := retiredOnAProvenExpiry(t, "code-billing-op-late-4") |
| + if code := f2.webhookPost(t, fx2, "evt_old_agreeing", "checkout.session.completed", intent2.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the old session read as archived: %d", code) |
| + } |
| + if n := disputesOf(t, fx2, acct2, intent2.SessionID, "%"); n != 0 { |
| + t.Fatalf("%d disputes for a read that agrees with the archive", n) |
| + } |
| + }) |
| +} |
| + |
| +// A subscription made by hand on the purchase's terms, naming an intent |
| +// that expired the ordinary way (no refusal, no retirement record), can |
| +// never record: its event is settled with its account, in the rejected |
| +// list, not retried until the provider gives up and seen nowhere. (Review |
| +// twenty, Opus P3-3, the related case.) |
| +func TestASubscriptionNamingAnOrdinarilyExpiredIntentIsSettledWithItsAccount(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.sessionStatus = "expired" |
| + f.mu.Unlock() |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)) |
| + if _, err := fx.st.billingCheckoutIntent(acct.ID); err == nil { |
| + t.Fatal("fixture: the intent stands after its session expired") |
| + } |
| + f.mu.Lock() |
| + f.subscriptionPrice = intent.Price |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_hand_after_expiry", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the hand-made subscription after the ordinary expiry: %d; want it settled", code) |
| + } |
| + if n := rejectedFor(t, fx, acct, "evt_hand_after_expiry"); n != 1 { |
| + t.Fatalf("%d rejected rows under the account; want one", n) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded: %+v", state) |
| + } |
| +} |
| + |
| +// A dispute has no resolution in stage one: the generic resolution door |
| +// refuses it, its review stays open, and the account stays held. (Review |
| +// twenty, Astra 4; Opus claim 5.) |
| +func TestADisputeReviewIsNotResolvedByANote(t *testing.T) { |
| + fx, f, acct, intent, token := retiredOnAProvenExpiry(t, "code-billing-op-dispute") |
| + f.mu.Lock() |
| + f.sessionStatus, f.subscriptionPrice = "complete", intent.Price |
| + f.mu.Unlock() |
| + _ = f.webhookPost(t, fx, "evt_late_done", "checkout.session.completed", intent.SessionID) |
| + open := func() bool { |
| + var n int |
| + if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0`, acct.ID, intent.SessionID, billingFateDisputedReason).Scan(&n); err != nil { |
| + t.Fatal(err) |
| + } |
| + return n == 1 |
| + } |
| + if !open() { |
| + t.Fatal("fixture: no open dispute review") |
| + } |
| + body := map[string]any{"account": acct.Sqid, "subscription": intent.SessionID, "reason": billingFateDisputedReason, "occurrences": 1, "note": "looked at it"} |
| + status, answer := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", body, token) |
| + if status != http.StatusConflict || !strings.Contains(fmt.Sprint(answer["error"]), "held") { |
| + t.Fatalf("resolving the dispute with a note: %d %v; want 409 held", status, answer) |
| + } |
| + if !open() { |
| + t.Fatal("the dispute review was resolved") |
| + } |
| + var resolutions int |
| + _ = fx.st.db.QueryRow(`SELECT count(*) FROM billing_review_resolutions WHERE account_id=? AND reason=?`, acct.ID, billingFateDisputedReason).Scan(&resolutions) |
| + if resolutions != 0 { |
| + t.Fatalf("%d resolutions recorded for the dispute", resolutions) |
| + } |
| +} |
| + |
| +// providerMethods classifies every exported method of the provider client |
| +// for the evidence invariant. A method that returns a checkout session or |
| +// a subscription read from the provider is "recorded by <wrapper>" (the |
| +// one function in billingretirement.go that may call it, and that hands |
| +// the read to the store before its caller sees it) or "unused" (no code |
| +// in this package may call it: it has no recording wrapper). Everything |
| +// else says why it is not such a read. A method added to the client fails |
| +// the test below until it is classified here. |
| +var providerMethods = map[string]string{ |
| + "GetCheckout": "recorded by readCheckout", |
| + "ListCheckoutsForCustomer": "recorded by listCheckouts", |
| + "GetSubscription": "recorded by readSubscription", |
| + "ListCheckouts": "unused", |
| + "ListSubscriptions": "unused", |
| + |
| + "CreateCheckout": "not a read: the answer to a create, replayed under its idempotency key; the session it returns is judged by ownSession and intentSession and bound, or refused, by the caller", |
| + "ExpireCheckout": "not a read: the answer to an expiry, replayed under its key and proving nothing about the present; the fate comes from the read after it, or from the store's own checks of the answer (retireIntent)", |
| + "UpdateQuantity": "a subscription item of the account's recorded subscription, not a session or a subscription naming an intent", |
| + "UpdateQuantityWith": "a subscription item of the account's recorded subscription, not a session or a subscription naming an intent", |
| + "FenceQuantityKey": "a kind, not an object", |
| + |
| + "MerchantAccount": "the merchant", "GetProduct": "catalogue", "GetPrice": "catalogue", "GetCoupon": "catalogue", |
| + "ActiveTaxRegistrations": "tax settings", "GetPortalConfiguration": "portal settings", "CreatePortal": "a portal session, not a checkout", |
| + "CreateCustomer": "a customer", "UpdateCustomerLocation": "a customer", "GetCustomer": "a customer", |
| + "CreateDraftInvoice": "an invoice", "GetInvoice": "an invoice", "ListInvoiceLines": "an invoice", "ListInvoicesForCustomer": "an invoice", |
| + "CreateInvoiceItem": "an invoice", "FinalizeInvoice": "an invoice", "PayInvoice": "an invoice", |
| + "String": "formatting", "GoString": "formatting", |
| +} |
| + |
| +// No code in this package reads a checkout session or a subscription from |
| +// the provider but the three methods that hand the read to the store |
| +// before their caller sees it. The invariant is the shape of the code, |
| +// not a list of call sites someone kept up to date: every method of the |
| +// client is classified, every use of a recorded read is inside its one |
| +// wrapper, and the wrapper's calls are, in order, the client's read and |
| +// the store's recording, with nothing returned between them but a failed |
| +// read. |
| +func TestEveryProviderReadGoesThroughTheRecordingMethods(t *testing.T) { |
| + client := reflect.TypeOf(&billingstripe.Client{}) |
| + for i := 0; i < client.NumMethod(); i++ { |
| + if _, classified := providerMethods[client.Method(i).Name]; !classified { |
| + t.Errorf("billingstripe.Client.%s is not classified in providerMethods: say whether it reads a session or a subscription, and if it does give it a recording wrapper", client.Method(i).Name) |
| + } |
| + } |
| + for name := range providerMethods { |
| + if _, exists := client.MethodByName(name); !exists { |
| + t.Errorf("providerMethods names %s, which the client does not have", name) |
| + } |
| + } |
| + wrapperOf := map[string]string{} |
| + for name, class := range providerMethods { |
| + if w, ok := strings.CutPrefix(class, "recorded by "); ok { |
| + wrapperOf[name] = w |
| + } else if class == "unused" { |
| + wrapperOf[name] = "" |
| + } |
| + } |
| + files, err := filepath.Glob("*.go") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + fset := token.NewFileSet() |
| + seen := map[string]int{} |
| + wrappers := map[string]*ast.FuncDecl{} |
| + for _, name := range files { |
| + if strings.HasSuffix(name, "_test.go") { |
| + continue |
| + } |
| + file, err := parser.ParseFile(fset, name, nil, 0) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, decl := range file.Decls { |
| + fn, _ := decl.(*ast.FuncDecl) |
| + where := "a package-level declaration" |
| + if fn != nil { |
| + where = fn.Name.Name |
| + if name == "billingretirement.go" { |
| + wrappers[fn.Name.Name] = fn |
| + } |
| + } |
| + // Any mention of the method's name as a selector: a call, a |
| + // method value, a method expression. |
| + ast.Inspect(decl, func(n ast.Node) bool { |
| + sel, ok := n.(*ast.SelectorExpr) |
| + if !ok { |
| + return true |
| + } |
| + wrapper, isRead := wrapperOf[sel.Sel.Name] |
| + if !isRead { |
| + return true |
| + } |
| + seen[sel.Sel.Name]++ |
| + switch { |
| + case wrapper == "": |
| + t.Errorf("%s: %s is used in %s; it returns provider objects and has no recording wrapper", fset.Position(sel.Pos()), sel.Sel.Name, where) |
| + case name != "billingretirement.go" || where != wrapper: |
| + t.Errorf("%s: %s is used in %s; a provider read is made only by %s (billingretirement.go), which records it first", fset.Position(sel.Pos()), sel.Sel.Name, where, wrapper) |
| + } |
| + return true |
| + }) |
| + } |
| + } |
| + for read, wrapper := range wrapperOf { |
| + if wrapper != "" && seen[read] != 1 { |
| + t.Errorf("%s is used %d times; want exactly once, in %s", read, seen[read], wrapper) |
| + } |
| + } |
| + // Each wrapper, statement by statement: the client's read; a return |
| + // of the failed read; then the store's recording before any return |
| + // that hands the read back. |
| + note := map[string]string{"readCheckout": "noteSessionSeen", "listCheckouts": "noteSessionSeen", "readSubscription": "noteSubscriptionSeen"} |
| + for wrapper, recording := range note { |
| + fn := wrappers[wrapper] |
| + if fn == nil { |
| + t.Errorf("%s is not in billingretirement.go", wrapper) |
| + continue |
| + } |
| + recorded := token.NoPos |
| + ast.Inspect(fn.Body, func(n ast.Node) bool { |
| + if sel, ok := n.(*ast.SelectorExpr); ok && sel.Sel.Name == recording && !recorded.IsValid() { |
| + recorded = sel.Pos() |
| + } |
| + return true |
| + }) |
| + if !recorded.IsValid() { |
| + t.Errorf("%s does not hand its read to the store (%s)", wrapper, recording) |
| + continue |
| + } |
| + ast.Inspect(fn.Body, func(n ast.Node) bool { |
| + ret, ok := n.(*ast.ReturnStmt) |
| + if !ok || len(ret.Results) != 2 { |
| + return true |
| + } |
| + if first, isIdent := ret.Results[0].(*ast.Ident); isIdent && first.Name == "nil" { |
| + return true // a failed read, or a read the store could not record: nothing handed back |
| + } |
| + if ret.Pos() < recorded { |
| + t.Errorf("%s: %s returns what it read before handing it to the store", fset.Position(ret.Pos()), wrapper) |
| + } |
| + return true |
| + }) |
| + } |
| +} |
diff --git a/internal/console/billinginvoice.go b/internal/console/billinginvoice.go| index 3e9282876..07f78a6b1 100644 |
| --- a/internal/console/billinginvoice.go |
| +++ b/internal/console/billinginvoice.go |
| @@ -555,7 +555,7 @@ func (r *billingInvoiceRun) preflight(ctx context.Context, id *billingInvoiceIde |
| if customer.AutomaticTax != "supported" && customer.AutomaticTax != "not_collecting" { |
| return errBillingUnavailable |
| } |
| - sub, err := r.b.client.GetSubscription(ctx, id.Subscription) |
| + sub, err := r.s.readSubscription(ctx, id.Subscription) |
| if err != nil { |
| return err |
| } |
diff --git a/internal/console/billingoperator.go b/internal/console/billingoperator.go| index 72876cf91..bc61ad5a2 100644 |
| --- a/internal/console/billingoperator.go |
| +++ b/internal/console/billingoperator.go |
| @@ -244,7 +244,7 @@ func (s *Server) billingKeepRefused(ctx context.Context, acct *Account, intentID |
| if intent.SessionID == "" { |
| return "the intent has no session", nil |
| } |
| - session, err := s.billing.client.GetCheckout(ctx, intent.SessionID) |
| + session, err := s.readCheckout(ctx, intent.SessionID) |
| if err != nil { |
| return "", err |
| } |
| @@ -260,14 +260,10 @@ 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 |
| - } |
| + // (This read was handed to the store when it was made, whatever the |
| + // checks above answered: a session that now contradicts what was |
| + // recorded is a dispute, and the resolution's transaction, which |
| + // reads the recorded state, refuses.) |
| if st, err := s.store.refusalState(acct.ID, intent.ID); err != nil { |
| return "", err |
| } else if st.Disputed != 0 { |
diff --git a/internal/console/billingpass.go b/internal/console/billingpass.go| index 5567bb635..e135f3b31 100644 |
| --- a/internal/console/billingpass.go |
| +++ b/internal/console/billingpass.go |
| @@ -31,11 +31,11 @@ func (s *Server) StartBillingPass(ctx context.Context) { |
| ticker := time.NewTicker(5 * time.Minute) |
| defer ticker.Stop() |
| for { |
| - bounded, cancel := context.WithTimeout(ctx, 2*time.Minute) |
| - if err := s.billingPass(bounded, time.Now().UTC()); err != nil { |
| + // billingPass bounds its own two turns (reconciliation, then the |
| + // refused intents) as children of this context. |
| + if err := s.billingPass(ctx, time.Now().UTC()); err != nil { |
| log.Printf("console: billing reconciliation: %s", billingDiagnostic(err)) |
| } |
| - cancel() |
| // Member additions next (billingquantity.go): they change the |
| // subscription quantity, which is history the invoice pass reads. |
| quantities, cancelQuantities := context.WithTimeout(ctx, 2*time.Minute) |
| @@ -121,11 +121,15 @@ func (s *Store) setBillingPassCursor(last int64) error { |
| // 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). |
| +// The two turns are each a child of ctx with a deadline of its own |
| +// (billingPassReconcileTimeout, billingPassRefusalTimeout): the refusals' |
| +// turn runs whether the reconciliation finished, failed or ran out of its |
| +// time, 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. Cancelling ctx stops both, a |
| +// turn already running included. (The console starts the pass with a |
| +// context it never cancels, cmd/console: a stop is the process ending, |
| +// which every step here tolerates.) |
| func (s *Server) billingPass(ctx context.Context, now time.Time) error { |
| if s.billing == nil { |
| return nil |
| @@ -153,16 +157,14 @@ func (s *Server) billingPass(ctx context.Context, now time.Time) error { |
| pending = append(pending, intent) |
| } |
| } |
| - if err := s.billingPassReconcile(ctx, pending, now); err != nil { |
| + reconcile, cancelReconcile := context.WithTimeout(ctx, billingPassReconcileTimeout) |
| + if err := s.billingPassReconcile(reconcile, 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() |
| - } |
| + cancelReconcile() |
| + turn, cancelTurn := context.WithTimeout(ctx, billingPassRefusalTimeout) |
| + failures = append(failures, s.billingPassRefusals(turn, refused, now)...) |
| + cancelTurn() |
| return errors.Join(failures...) |
| } |
| @@ -222,7 +224,7 @@ func (s *Server) billingPassReconcile(ctx context.Context, intents []billingInte |
| func (s *Server) reconcilePendingIntent(ctx context.Context, intent billingIntent, now time.Time) error { |
| b := s.billing |
| if intent.SessionID != "" { |
| - session, err := b.client.GetCheckout(ctx, intent.SessionID) |
| + session, err := s.readCheckout(ctx, intent.SessionID) |
| if err != nil { |
| return err |
| } |
diff --git a/internal/console/billingquantity.go b/internal/console/billingquantity.go| index 1ee18385e..15ce06056 100644 |
| --- a/internal/console/billingquantity.go |
| +++ b/internal/console/billingquantity.go |
| @@ -1007,7 +1007,7 @@ func (s *Server) raiseQuantity(ctx context.Context, acct *Account, behavior stri |
| // reconcile catches the console up). |
| func (s *Server) quantityProviderItem(ctx context.Context, acct *Account, state billingState) (billingstripe.SubscriptionItem, bool, error) { |
| b := s.billing |
| - sub, err := b.client.GetSubscription(ctx, state.Subscription) |
| + sub, err := s.readSubscription(ctx, state.Subscription) |
| if err != nil { |
| return billingstripe.SubscriptionItem{}, false, err |
| } |
diff --git a/internal/console/billingretirement.go b/internal/console/billingretirement.go| index 768dbf4de..bb00a089d 100644 |
| --- a/internal/console/billingretirement.go |
| +++ b/internal/console/billingretirement.go |
| @@ -37,18 +37,32 @@ import ( |
| // 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. |
| +// The evidence invariant. Every read of a checkout session or a |
| +// subscription from the provider goes through one of three methods here |
| +// (readCheckout, listCheckouts, readSubscription), and each hands what it |
| +// read to the store before the caller sees it. The store finds the |
| +// purchase by what the console itself recorded, never by what the answer |
| +// claims: a session by its id, against the standing intent bound or |
| +// referenced to it, the retirement record, or the kept resolution's |
| +// evidence; a subscription by the account and intent it names. It |
| +// classifies in one transaction against the purchase as it is at that |
| +// instant, standing, retired, or kept and consumed, and records the |
| +// session's fate, an attached subscription, or a dispute. No caller |
| +// decides whether to record, so no return of a caller's can come first; |
| +// a test parses this package and fails on any direct read of the client. |
| +// A refusal and the subscription that caused it are one transaction, and |
| +// filing for an intent that is already gone classifies the subscription |
| +// against the archive in that same transaction. Nothing read about a |
| +// refused purchase is dropped. |
| +// |
| +// What the invariant is not about: the answers to a create and to an |
| +// expiry (replays under an idempotency key, proving nothing about the |
| +// present: what follows one is decided on a read, or on the store's own |
| +// checks of the answer); a read made before any refusal review exists |
| +// (the read that leads to a filing is handed in again once the review |
| +// exists: fileRefusal); and a read without identity, or of a session |
| +// still open, before any fate is recorded (there is nothing recorded for |
| +// it to contradict, and with no fate both outcomes are held). |
| const ( |
| sessionFateExpired = "expired" |
| @@ -67,6 +81,10 @@ const ( |
| billingPassRefusalTimeout = 45 * time.Second |
| ) |
| +// billingPassReconcileTimeout bounds the reconciliation's turn in a pass. |
| +// A variable so a test can spend it. |
| +var billingPassReconcileTimeout = 2 * time.Minute |
| + |
| // errBillingUnderReview: the account's purchase is under a refusal review. |
| // Nothing is offered or opened until a person disposes of it. To a caller |
| // that knows only pending, it is pending. |
| @@ -104,6 +122,10 @@ func (s *Store) migrateBillingRetirement() error { |
| // compared with what was kept. |
| {"billing_review_resolutions", "evidence_session", `TEXT NOT NULL DEFAULT ''`}, |
| {"billing_review_resolutions", "evidence_subscription", `TEXT NOT NULL DEFAULT ''`}, |
| + {"billing_review_resolutions", "evidence_customer", `TEXT NOT NULL DEFAULT ''`}, |
| + // The retired intent's customer, so a late read of its session as |
| + // another customer's is seen as the contradiction it is. |
| + {"billing_retired_intents", "customer", `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 { |
| @@ -300,36 +322,84 @@ func disputeSessionTx(tx *sql.Tx, accountID int64, sessionID, intentID, detail s |
| return fileBillingReviewTx(tx, accountID, sessionID, billingFateDisputedReason, now) |
| } |
| -// 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 |
| +// sessionPurchase is the refused purchase a session belongs to, found by |
| +// the session's id in what the console recorded. |
| +type sessionPurchase struct { |
| + AccountID int64 |
| + IntentID string |
| + Kind string // standing | retired | kept |
| + Customer string // the purchase's customer, where recorded |
| + ArchivedFate string // retired: the fate it was retired on ("" before the protocol) |
| + KeptSubscription string // kept: the subscription it was kept with |
| +} |
| + |
| +// purchaseOfSessionTx finds the purchase a session id belongs to: the |
| +// standing intent bound to it, or never bound and referenced to it by its |
| +// refusal review; else the retirement record that names it; else the kept |
| +// resolution whose evidence names it. The id is the console's own (it |
| +// bound it, or validated it in full when it found it), so the lookup does |
| +// not depend on anything the provider's answer says. |
| +func purchaseOfSessionTx(tx *sql.Tx, sessionID string) (sessionPurchase, bool, error) { |
| + var p sessionPurchase |
| + err := tx.QueryRow(`SELECT account_id,id,customer FROM billing_checkout_intents WHERE session_id=?`, sessionID).Scan(&p.AccountID, &p.IntentID, &p.Customer) |
| + if err == nil { |
| + p.Kind = "standing" |
| + return p, true, nil |
| + } |
| + if !errors.Is(err, sql.ErrNoRows) { |
| + return p, false, err |
| + } |
| + err = tx.QueryRow(`SELECT i.account_id,i.id,i.customer FROM billing_checkout_intents i JOIN billing_reviews r |
| + ON r.account_id=i.account_id AND r.subscription=i.id AND r.reason=? WHERE i.session_id='' AND r.reference=?`, billingCheckoutRefusedReason, sessionID).Scan(&p.AccountID, &p.IntentID, &p.Customer) |
| + if err == nil { |
| + p.Kind = "standing" |
| + return p, true, nil |
| + } |
| + if !errors.Is(err, sql.ErrNoRows) { |
| + return p, false, err |
| + } |
| + err = tx.QueryRow(`SELECT account_id,intent_id,session_fate,customer FROM billing_retired_intents WHERE session_id=? ORDER BY retired_at DESC LIMIT 1`, sessionID).Scan(&p.AccountID, &p.IntentID, &p.ArchivedFate, &p.Customer) |
| + if err == nil { |
| + p.Kind = "retired" |
| + return p, true, nil |
| + } |
| + if !errors.Is(err, sql.ErrNoRows) { |
| + return p, false, err |
| + } |
| + err = tx.QueryRow(`SELECT account_id,subscription,evidence_subscription,evidence_customer FROM billing_review_resolutions |
| + WHERE reason=? AND outcome='keep' AND evidence_session=? ORDER BY resolved_at DESC LIMIT 1`, billingCheckoutRefusedReason, sessionID).Scan(&p.AccountID, &p.IntentID, &p.KeptSubscription, &p.Customer) |
| + if err == nil { |
| + p.Kind = "kept" |
| + return p, true, nil |
| + } |
| + if !errors.Is(err, sql.ErrNoRows) { |
| + return p, false, err |
| + } |
| + return p, false, nil |
| } |
| -// 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. |
| +// noteSessionSeen classifies and records one provider answer about a |
| +// session, in one transaction that finds the purchase by the session's id |
| +// and loads it as it is now. |
| +// |
| +// Identity is judged against what the console recorded, not against the |
| +// answer: this mode, the purchase's intent in the session's metadata, the |
| +// purchase's account in both places, and its customer where one is |
| +// recorded. An answer that names another intent or account is a |
| +// contradiction about this session, not an answer about another purchase. |
| // |
| -// 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 == "" { |
| +// The intent stands under a refusal review: with 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 refused purchase has this session; no |
| +// refusal review; a retirement from before the protocol) records nothing. |
| +func (s *Store) noteSessionSeen(liveMode bool, session *billingstripe.Session, now time.Time) error { |
| + if session == nil || session.ID == "" { |
| return nil |
| } |
| tx, err := s.db.Begin() |
| @@ -337,90 +407,115 @@ func (s *Store) noteSessionObservation(accountID int64, intentID string, o sessi |
| return err |
| } |
| defer tx.Rollback() |
| - st, err := refusalStateQ(tx, accountID, intentID) |
| - if err != nil { |
| + p, found, err := purchaseOfSessionTx(tx, session.ID) |
| + if err != nil || !found { |
| + return err |
| + } |
| + var sqid string |
| + if err := tx.QueryRow(`SELECT sqid FROM accounts WHERE id=?`, p.AccountID).Scan(&sqid); err != nil && !errors.Is(err, sql.ErrNoRows) { |
| return err |
| } |
| - read := o.Status |
| + identity := sqid != "" && session.LiveMode == liveMode && session.Metadata["checkout_intent"] == p.IntentID && |
| + session.ClientReferenceID == sqid && session.Metadata["account_sqid"] == sqid && (p.Customer == "" || session.CustomerID == p.Customer) |
| + fate := "" |
| + switch { |
| + case session.Status == "expired" && session.SubscriptionID == "" && session.PaymentStatus != "paid": |
| + fate = sessionFateExpired |
| + case session.Status == "complete": |
| + fate = sessionFateComplete |
| + } |
| + read := session.Status |
| if read == "" { |
| read = "with no status" |
| } |
| - if st.Stands { |
| - if !st.Exists || st.pinned() != o.SessionID { |
| + dispute := func(detail string) error { |
| + return disputeSessionTx(tx, p.AccountID, session.ID, p.IntentID, detail, now) |
| + } |
| + switch p.Kind { |
| + case "standing": |
| + st, err := refusalStateQ(tx, p.AccountID, p.IntentID) |
| + if err != nil { |
| + return err |
| + } |
| + if !st.Exists { |
| return nil |
| } |
| - identity := o.Identity && (st.Customer == "" || o.Customer == st.Customer) |
| switch { |
| case !identity: |
| if st.Fate == "" { |
| return nil |
| } |
| - 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 == "": |
| + err = dispute("read back as another account's, customer's or intent's after it was recorded " + st.Fate) |
| + case fate == "": |
| if st.Fate == "" { |
| return nil |
| } |
| - err = disputeSessionTx(tx, accountID, o.SessionID, intentID, "read back "+read+" after it was recorded "+st.Fate, now) |
| + err = dispute("read back " + read + " after it was recorded " + st.Fate) |
| 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: |
| + fate, max(now.Unix(), 1), session.PaymentStatus, session.SubscriptionID, p.AccountID, p.IntentID) |
| + case st.Fate != fate: |
| + err = dispute("read back " + fate + " after it was recorded " + st.Fate) |
| + case fate == sessionFateComplete && st.SessionSubscription != "" && session.SubscriptionID != "" && session.SubscriptionID != st.SessionSubscription: |
| + err = dispute("its subscription read back as " + session.SubscriptionID + " after it was recorded as " + st.SessionSubscription) |
| + case 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) |
| + session.PaymentStatus, session.SubscriptionID, p.AccountID, p.IntentID) |
| } |
| if err != nil { |
| return err |
| } |
| - return tx.Commit() |
| - } |
| - // 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 err == nil: |
| + case "retired": |
| // 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 { |
| + if p.ArchivedFate != sessionFateExpired { |
| return nil |
| } |
| - 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 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 |
| + if !identity { |
| + err = dispute("read back as another account's, customer's or intent's after its intent was retired on a proven expiry") |
| + } else if fate != sessionFateExpired { |
| + err = dispute("read back " + read + " after its intent was retired on a proven expiry") |
| } |
| if err != nil { |
| return err |
| } |
| + case "kept": |
| 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) |
| + case !identity: |
| + err = dispute("read back as another account's, customer's or intent's after its purchase was kept and recorded") |
| + case fate != sessionFateComplete: |
| + err = dispute("read back " + read + " after its purchase was kept as complete") |
| + case p.KeptSubscription != "" && session.SubscriptionID != "" && session.SubscriptionID != p.KeptSubscription: |
| + err = dispute("its subscription read back as " + session.SubscriptionID + " after its purchase was kept with " + p.KeptSubscription) |
| + } |
| + if err != nil { |
| + return err |
| } |
| } |
| + return tx.Commit() |
| +} |
| + |
| +// sessionArchived reports whether a session belongs to a purchase that no |
| +// longer stands (retired, or kept and recorded), and whose account: its |
| +// later events can never record anything and are settled, not retried. |
| +func (s *Store) sessionArchived(sessionID string) (accountID int64, archived bool, err error) { |
| + tx, err := s.db.Begin() |
| if err != nil { |
| - return err |
| + return 0, false, err |
| } |
| - return tx.Commit() |
| + defer tx.Rollback() |
| + p, found, err := purchaseOfSessionTx(tx, sessionID) |
| + if err != nil || !found || p.Kind == "standing" { |
| + return 0, false, err |
| + } |
| + return p.AccountID, true, nil |
| } |
| -// noteSubscription classifies and records a subscription that names an |
| -// intent, in one transaction that loads the purchase as it is now. |
| +// classifySubscriptionTx classifies and records a subscription that names |
| +// an intent of an account, against the purchase as it is in this |
| +// transaction. |
| // |
| // The intent stands with a refusal review, open or resolved: the |
| // subscription is attached to the case unless it is the pinned session's |
| @@ -432,15 +527,10 @@ func (s *Store) noteSessionObservation(accountID int64, intentID string, o sessi |
| // 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 { |
| +func classifySubscriptionTx(tx *sql.Tx, accountID int64, intentID, subscription string, now time.Time) error { |
| if intentID == "" || subscription == "" { |
| return nil |
| } |
| - tx, err := s.db.Begin() |
| - if err != nil { |
| - return err |
| - } |
| - defer tx.Rollback() |
| st, err := refusalStateQ(tx, accountID, intentID) |
| if err != nil { |
| return err |
| @@ -461,7 +551,7 @@ func (s *Store) noteSubscription(accountID int64, intentID, subscription string, |
| return err |
| } |
| } |
| - return tx.Commit() |
| + return nil |
| } |
| for _, known := range st.Obligations { |
| if known == subscription { |
| @@ -478,7 +568,7 @@ func (s *Store) noteSubscription(accountID int64, intentID, subscription string, |
| 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) |
| + return 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`, |
| @@ -489,11 +579,40 @@ func (s *Store) noteSubscription(accountID int64, intentID, subscription string, |
| 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) |
| + return disputeSessionTx(tx, accountID, keptSession, intentID, "subscription "+subscription+" names the intent whose purchase was kept and recorded with "+keptSubscription, now) |
| } |
| + return err |
| +} |
| + |
| +// noteSubscriptionSeen classifies and records one subscription read from |
| +// the provider, by the account and intent it names, in one transaction. |
| +// The account's own recorded subscription is not evidence about anything. |
| +func (s *Store) noteSubscriptionSeen(sqid, intentID, subscription string, now time.Time) error { |
| + if sqid == "" || intentID == "" || subscription == "" { |
| + return nil |
| + } |
| + tx, err := s.db.Begin() |
| if err != nil { |
| return err |
| } |
| + defer tx.Rollback() |
| + var accountID int64 |
| + if err := tx.QueryRow(`SELECT id FROM accounts WHERE sqid=?`, sqid).Scan(&accountID); err != nil { |
| + if errors.Is(err, sql.ErrNoRows) { |
| + return nil |
| + } |
| + return err |
| + } |
| + var recorded string |
| + if err := tx.QueryRow(`SELECT subscription FROM billing_accounts WHERE account_id=?`, accountID).Scan(&recorded); err != nil && !errors.Is(err, sql.ErrNoRows) { |
| + return err |
| + } |
| + if recorded == subscription { |
| + return nil |
| + } |
| + if err := classifySubscriptionTx(tx, accountID, intentID, subscription, now); err != nil { |
| + return err |
| + } |
| return tx.Commit() |
| } |
| @@ -545,30 +664,64 @@ func (b *billingRuntime) sessionProof(session *billingstripe.Session, intent bil |
| return "", true |
| } |
| -// 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 |
| +// readCheckout is the one way this package reads a checkout session from |
| +// the provider: what was read is handed to the store (noteSessionSeen) |
| +// before the caller sees it, and a read the store could not record is a |
| +// failed read. |
| +func (s *Server) readCheckout(ctx context.Context, id string) (*billingstripe.Session, error) { |
| + session, err := s.billing.client.GetCheckout(ctx, id) |
| + if err != nil { |
| + return nil, err |
| + } |
| + if err := s.store.noteSessionSeen(s.billing.cfg.LiveMode, session, time.Now()); err != nil { |
| + return nil, err |
| } |
| - return o |
| + return session, nil |
| } |
| -// 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 |
| +// listCheckouts is readCheckout for the customer-scoped listing recovery |
| +// uses: every session listed is handed to the store. |
| +func (s *Server) listCheckouts(ctx context.Context, customer string, from, to int64) ([]billingstripe.Session, error) { |
| + sessions, err := s.billing.client.ListCheckoutsForCustomer(ctx, customer, from, to) |
| + if err != nil { |
| + return nil, err |
| + } |
| + for i := range sessions { |
| + if err := s.store.noteSessionSeen(s.billing.cfg.LiveMode, &sessions[i], time.Now()); err != nil { |
| + return nil, err |
| + } |
| + } |
| + return sessions, nil |
| +} |
| + |
| +// readSubscription is the one way this package reads a subscription from |
| +// the provider: it is handed to the store (noteSubscriptionSeen), by the |
| +// account and intent it names, before the caller sees it. |
| +func (s *Server) readSubscription(ctx context.Context, id string) (*billingstripe.Subscription, error) { |
| + sub, err := s.billing.client.GetSubscription(ctx, id) |
| + if err != nil { |
| + return nil, err |
| } |
| - return s.store.noteSessionObservation(acct.ID, intentID, s.billing.observeSession(session, intentID, acct), now) |
| + if sub.ID == id && sub.LiveMode == s.billing.cfg.LiveMode { |
| + if err := s.store.noteSubscriptionSeen(sub.Metadata["account_sqid"], sub.Metadata["checkout_intent"], sub.ID, time.Now()); err != nil { |
| + return nil, err |
| + } |
| + } |
| + return sub, nil |
| +} |
| + |
| +// fileRefusal files the refusal review of an intent, with the subscription |
| +// that caused it when one did, and then hands the session that was read |
| +// to the store again: read before the review existed, it recorded |
| +// nothing, and it is the first evidence of the session's fate. |
| +func (s *Server) fileRefusal(intent billingIntent, sessionID, subscription string, session *billingstripe.Session, now time.Time) error { |
| + if err := s.store.fileCheckoutRefusal(intent, sessionID, subscription, now); err != nil { |
| + return err |
| + } |
| + if session != nil && session.ID == sessionID { |
| + return s.store.noteSessionSeen(s.billing.cfg.LiveMode, session, now) |
| + } |
| + return nil |
| } |
| // advanceRefusal works a refused intent toward a recorded fate: it reads |
| @@ -599,7 +752,7 @@ func (s *Server) advanceRefusal(ctx context.Context, intent billingIntent, acct |
| } |
| return err |
| } |
| - session, err := b.client.GetCheckout(ctx, pinned) |
| + session, err := s.readCheckout(ctx, pinned) |
| if err != nil { |
| return fail(err) |
| } |
| @@ -607,9 +760,6 @@ 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 nil |
| } |
| @@ -618,16 +768,13 @@ func (s *Server) advanceRefusal(ctx context.Context, intent billingIntent, acct |
| // one key; a session it never bound is not expired here. |
| if session.Status == "open" && intent.SessionID != "" { |
| _, expireErr := b.client.ExpireCheckout(ctx, pinned, "carloku-expire-"+intent.ID) |
| - again, err := b.client.GetCheckout(ctx, pinned) |
| + again, err := s.readCheckout(ctx, pinned) |
| if err != nil { |
| return fail(errors.Join(expireErr, err)) |
| } |
| 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 nil |
| } |
diff --git a/internal/console/billingretirement_test.go b/internal/console/billingretirement_test.go| index 92da2a042..06dc0a260 100644 |
| --- a/internal/console/billingretirement_test.go |
| +++ b/internal/console/billingretirement_test.go |
| @@ -387,8 +387,9 @@ func TestAPaymentBeforeTheRetireIsACompletionNotARetirement(t *testing.T) { |
| } |
| // The pass owns the retries: a refused intent whose expiry failed is tried |
| -// again on its schedule (one minute, then five), never before, and is not |
| -// reconciled as a pending purchase meanwhile. |
| +// again on its schedule (one minute, then five), never before; and once |
| +// its fate is recorded a pass asks the provider nothing more about it |
| +// (the last assertion: it is not reconciled as a pending purchase). |
| func TestThePassRetriesAFailedExpiryOnItsSchedule(t *testing.T) { |
| fx, f, acct, intent, _ := refusedByAHandMadeSubscription(t, 100) |
| calls := func() int { f.mu.Lock(); defer f.mu.Unlock(); return f.expireCalls } |
| @@ -654,36 +655,33 @@ func TestAStructurallyInvalidSubscriptionNamingTheIntentIsReviewedAndAttached(t |
| } |
| } |
| -// 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.) |
| +// The refusal and the subscription that caused it are one transaction of |
| +// the production helper: when the attachment cannot be written, no review |
| +// is left behind, so no reader (after a crash, or on another console |
| +// between two statements) finds the review without what is attached to |
| +// it. And when the intent went between the caller's read and the filing, |
| +// the subscription is not dropped: it is classified, in the same |
| +// transaction, against what the intent was disposed on. (Review nineteen: |
| +// Opus P3-1, Astra 1; review twenty: Opus P3-4, Astra 2 and 6.) |
| 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 { |
| + fx, f, acct, intent := refusedByAnotherCustomer(t) |
| + // A clean slate for the filing: no review yet. |
| + if _, err := fx.st.db.Exec(`DELETE FROM billing_reviews WHERE account_id=?`, acct.ID); err != nil { |
| t.Fatal(err) |
| } |
| - if err := fileBillingReviewTx(tx, acct.ID, intent.ID, billingCheckoutRefusedReason, f.now); err != nil { |
| + // The attachment fails after the review's row was written, inside |
| + // fileCheckoutRefusal's own transaction. |
| + if _, err := fx.st.db.Exec(`CREATE TRIGGER test_no_attachment BEFORE INSERT ON billing_review_obligations BEGIN SELECT RAISE(ABORT, 'test_no_attachment'); END`); err != nil { |
| t.Fatal(err) |
| } |
| - _ = tx.Rollback() // the crash between the review and the attachment, as two statements of one transaction |
| + if err := fx.st.fileCheckoutRefusal(intent, intent.SessionID, "sub_hand", f.now); err == nil || !strings.Contains(err.Error(), "test_no_attachment") { |
| + t.Fatalf("the filing whose attachment fails: %v; want it to fail", err) |
| + } |
| if st, _ := fx.st.refusalState(acct.ID, intent.ID); st.Exists { |
| - t.Fatalf("a review survives the rolled-back filing: %+v", st) |
| + t.Fatalf("a review survives the filing whose attachment failed: %+v", st) |
| + } |
| + if _, err := fx.st.db.Exec(`DROP TRIGGER test_no_attachment`); err != nil { |
| + t.Fatal(err) |
| } |
| if err := fx.st.fileCheckoutRefusal(intent, intent.SessionID, "sub_hand", f.now); err != nil { |
| t.Fatal(err) |
| @@ -692,31 +690,29 @@ func TestTheRefusalAndItsSubscriptionAreOneTransaction(t *testing.T) { |
| 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. |
| + // Another console retires the intent (a case with nothing attached, |
| + // its expiry proven) between this console's read and its filing. |
| 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) |
| + f.mu.Lock() |
| + f.sessionCustomer = "" |
| + f.mu.Unlock() |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-atomic") |
| + if status, msg := resolveRefusal(t, fx, token, acct, intent, "", "expired, never paid"); status != http.StatusOK { |
| + t.Fatalf("the other console's retire: %d %q", status, msg) |
| } |
| + // This console's filing arrives, twice. |
| for i := 0; i < 2; i++ { |
| - if err := fx.st.noteSubscription(acct.ID, intent.ID, "sub_hand", f.now.Add(time.Hour)); err != nil { |
| + if err := fx.st.fileCheckoutRefusal(intent, intent.SessionID, "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) |
| + _ = fx.st.db.QueryRow(`SELECT count(*) FROM billing_checkout_disputes WHERE account_id=? AND session_id=? AND detail LIKE 'subscription sub_hand names%'`, 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) |
| + t.Fatalf("%d attached, %d disputes; want nothing attached to the retired intent and the subscription a dispute, once", attached, disputes) |
| } |
| } |
| @@ -784,7 +780,9 @@ func TestEvidenceAfterAKeepSurvivesTheIntent(t *testing.T) { |
| f.mu.Lock() |
| f.sessionSubscription = "sub_changed" |
| f.mu.Unlock() |
| - _ = f.webhookPost(t, fx, "evt_kept_session_changed", "checkout.session.completed", intent.SessionID) |
| + if code := f.webhookPost(t, fx, "evt_kept_session_changed", "checkout.session.completed", intent.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the kept session's contradicting event: %d; want it settled, not retried for days", code) |
| + } |
| 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) |
| } |
| @@ -793,17 +791,28 @@ func TestEvidenceAfterAKeepSurvivesTheIntent(t *testing.T) { |
| 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 code := f.webhookPost(t, fx, "evt_kept_session_again", "checkout.session.completed", intent.SessionID); code != http.StatusNoContent { |
| + t.Fatalf("the kept session's contradicting event: %d; want it settled, not retried for days", code) |
| + } |
| 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) |
| + // The other subscription is still incomplete at the provider, so |
| + // the reconcile answers pending before it judges it: it is |
| + // attached, and the review reopened, by the read alone. |
| f.mu.Lock() |
| f.otherSubscriptions = map[string]bool{"sub_hand": true} |
| + f.subscriptionStatus = "incomplete" |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_between", "customer.subscription.created", "sub_hand"); code == http.StatusNoContent || code == http.StatusOK { |
| + t.Fatalf("an incomplete subscription: %d; want it retried", code) |
| + } |
| + f.mu.Lock() |
| + f.subscriptionStatus = "active" |
| 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) |
| @@ -884,17 +893,18 @@ func TestTheKeepGatesReadIsRecordedAsEvidence(t *testing.T) { |
| } |
| } |
| -// 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.) |
| +// An actor with a stale view cannot drop what it read: the classification |
| +// is the store's, in one transaction, and finds the purchase by the |
| +// session's id whatever the answer claims. A read that names another |
| +// customer, handed in after another actor recorded the expiry, is a |
| +// dispute, and the retire that would have passed is held. (Review |
| +// nineteen, Astra 3; review twenty, Astra 1 and 3.) |
| 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 |
| + // Actor A read the session as another customer's and is paused |
| + // holding that answer. 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) |
| + stale := &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}} |
| f.mu.Lock() |
| f.sessionCustomer = "" |
| f.mu.Unlock() |
| @@ -904,13 +914,49 @@ func TestAContradictionReadBesideARecordedFateIsNotDropped(t *testing.T) { |
| 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 { |
| + if err := fx.st.noteSessionSeen(false, 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) |
| } |
| + // The same, one clause of the identity at a time: an answer for this |
| + // session id that names another intent, another account in its |
| + // metadata, or another account as its client reference. Each is found |
| + // by the session's id and is a contradiction about this purchase, not |
| + // an answer about some other. |
| + for name, edit := range map[string]func(*billingstripe.Session){ |
| + "another intent": func(x *billingstripe.Session) { x.Metadata["checkout_intent"] = "ci_edited" }, |
| + "another account in metadata": func(x *billingstripe.Session) { x.Metadata["account_sqid"] = "zzz" }, |
| + "another account as reference": func(x *billingstripe.Session) { x.ClientReferenceID = "zzz" }, |
| + "the other mode": func(x *billingstripe.Session) { x.LiveMode = true }, |
| + "another intent and account": func(x *billingstripe.Session) { |
| + x.Metadata["checkout_intent"], x.Metadata["account_sqid"], x.ClientReferenceID = "ci_edited", "zzz", "zzz" |
| + }, |
| + } { |
| + fx2, f2, acct2, intent2 := refusedByAnotherCustomer(t) |
| + f2.mu.Lock() |
| + f2.sessionCustomer = "" |
| + f2.mu.Unlock() |
| + if err := fx2.srv.advanceRefusal(context.Background(), intent2, acct2, f2.now.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + edited := &billingstripe.Session{ID: intent2.SessionID, Status: "expired", LiveMode: false, ClientReferenceID: acct2.Sqid, CustomerID: intent2.Customer, Metadata: map[string]string{"checkout_intent": intent2.ID, "account_sqid": acct2.Sqid}} |
| + // Unedited, it agrees with the recorded expiry and is no dispute. |
| + if err := fx2.st.noteSessionSeen(false, edited, f2.now.Add(2*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st, _ := fx2.st.refusalState(acct2.ID, intent2.ID); st.Fate != sessionFateExpired || st.Disputed != 0 { |
| + t.Fatalf("%s: fixture: %+v; want the expiry recorded and the agreeing answer no dispute", name, st) |
| + } |
| + edit(edited) |
| + if err := fx2.st.noteSessionSeen(false, edited, f2.now.Add(3*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st, _ := fx2.st.refusalState(acct2.ID, intent2.ID); st.Disputed == 0 { |
| + t.Fatalf("%s: state %+v; want the answer recorded as a dispute of this session", name, st) |
| + } |
| + } |
| if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil { |
| t.Fatal(err) |
| } |
| @@ -918,27 +964,48 @@ func TestAContradictionReadBesideARecordedFateIsNotDropped(t *testing.T) { |
| } |
| // 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) { |
| +// time: with the reconciliation's deadline spent before it starts, a |
| +// refused intent whose retry is due is still worked. And the two turns are |
| +// children of the caller's context: cancelled, neither runs. (Review |
| +// nineteen, Astra 4; review twenty, Opus P3-5 and Astra 5.) |
| +func TestThePassWorksADueRefusalThoughTheReconciliationsTimeIsSpent(t *testing.T) { |
| + saved := billingPassReconcileTimeout |
| + billingPassReconcileTimeout = time.Nanosecond |
| + t.Cleanup(func() { billingPassReconcileTimeout = saved }) |
| 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)) |
| + _ = fx.srv.billingPass(context.Background(), 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) |
| + t.Fatalf("after a pass whose reconciliation had no time: 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 |
| + requests := f2.requests |
| _ = 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) |
| + if f2.requests != requests { |
| + t.Fatalf("a cancelled pass made %d provider requests", f2.requests-requests) |
| + } |
| + // Cancelled while the refusals' turn is at work (inside the expiry, |
| + // the reconciliation's deadline long spent): the turn stops there. |
| + // The read that would prove the fate is not made, and the next pass |
| + // that is due makes it. |
| + fx3, f3, acct3, intent3, _ := refusedByAHandMadeSubscription(t, 1) |
| + running, cancel := context.WithCancel(context.Background()) |
| + defer cancel() |
| + f3.mu.Lock() |
| + f3.expireHook = cancel |
| + f3.mu.Unlock() |
| + _ = fx3.srv.billingPass(running, f3.now.Add(2*time.Minute)) |
| + st3, err := fx3.st.refusalState(acct3.ID, intent3.ID) |
| + if err != nil || f3.sessionStatus != "expired" || st3.Fate != "" { |
| + t.Fatalf("after a pass cancelled inside the expiry: session %s, state %+v %v; want the expiry made and nothing read after the cancellation", f3.sessionStatus, st3, err) |
| + } |
| + _ = fx3.srv.billingPass(context.Background(), f3.now.Add(30*time.Minute)) |
| + if st3, err = fx3.st.refusalState(acct3.ID, intent3.ID); err != nil || st3.Fate != sessionFateExpired { |
| + t.Fatalf("after the next due pass: state %+v %v; want the expiry proven", st3, err) |
| } |
| } |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 0f0a5fa6c..632d355b4 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -321,15 +321,13 @@ func (s *Server) ownBoundSession(ctx context.Context, session *billingstripe.Ses |
| // under its review rather than retired with no record. An expiry that |
| // fails leaves the review standing too, and is requested again by the |
| // pass on its schedule and by the operator's retire. |
| - if err := s.store.fileCheckoutRefusedReview(intent, session.ID, now); err != nil { |
| - return false, err |
| - } |
| - // A recorded fate this read contradicts is a dispute; then the session |
| - // is worked toward a proven fate (advanceRefusal: its expiry requested |
| - // 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.ID, acct, session, now); err != nil { |
| + // Filed, and the read handed to the store again now that the review |
| + // exists (fileRefusal); then the session is worked toward a proven |
| + // fate (advanceRefusal: its expiry requested 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.fileRefusal(intent, session.ID, "", session, now); err != nil { |
| return false, err |
| } |
| if err := s.advanceRefusal(ctx, intent, acct, now); err != nil { |
| @@ -354,8 +352,10 @@ func (s *Server) ownBoundSession(ctx context.Context, session *billingstripe.Ses |
| // currency, which the provider should not have let happen. Nothing is |
| // recorded from it, the intent is left as it is so no second purchase is |
| // opened beside it, a review is filed for the account (once while it is |
| -// open; resolving it retires the intent, or keeps one whose session can |
| -// still record, after which a session still wrong files it again), and |
| +// open; it is resolved only as the retirement protocol allows, which in |
| +// stage one is to keep a purchase corrected at the provider, after which |
| +// a session still wrong files it again: nothing retires an intent whose |
| +// session was paid), and |
| // nothing is written at the provider again (later passes still read the |
| // session, and file nothing more). A person has to look at what was paid |
| // for. |
| @@ -365,17 +365,10 @@ func (s *Server) ownBoundSession(ctx context.Context, session *billingstripe.Ses |
| // own (ownSession): nothing here expires a session on its id alone. |
| func (s *Server) refuseSession(ctx context.Context, intent billingIntent, sessionID string, session *billingstripe.Session, now time.Time) error { |
| if session != nil && (session.Status == "complete" || session.PaymentStatus == "paid") { |
| - if err := s.store.fileCheckoutRefusedReview(intent, session.ID, now); err != nil { |
| - return err |
| - } |
| // What the completed session proves is recorded with the review: |
| // a session recorded complete is never retired as expired. |
| - if acct, err := s.store.AccountByID(intent.AccountID); err != nil { |
| + if err := s.fileRefusal(intent, session.ID, "", session, now); err != nil { |
| return err |
| - } else if acct != nil { |
| - if err := s.noteSessionEvidence(intent.ID, acct, session, now); err != nil { |
| - return err |
| - } |
| } |
| return errBillingIdentity |
| } |
| @@ -645,13 +638,21 @@ func (s *Server) startBillingCheckoutQuoted(ctx context.Context, acct *Account, |
| previous.SessionID, previous.URL, previous.ExpiresAt = session.ID, session.URL, session.ExpiresAt |
| } |
| if lookupErr == nil && previous.SessionID != "" { |
| - session, err := b.client.GetCheckout(ctx, previous.SessionID) |
| + session, err := s.readCheckout(ctx, previous.SessionID) |
| if err != nil { |
| return "", err |
| } |
| if session.ID != previous.SessionID { |
| return "", errBillingIdentity |
| } |
| + // The read was recorded (readCheckout). If it contradicted what |
| + // is recorded of a purchase that was kept, the intent is disputed |
| + // now: nothing is offered, expired or replaced on it. |
| + if st, err := s.store.refusalState(acct.ID, previous.ID); err != nil { |
| + return "", err |
| + } else if st.Disputed != 0 { |
| + return "", errBillingUnderReview |
| + } |
| if own, err := s.ownBoundSession(ctx, session, previous, acct, now); err != nil { |
| return "", err |
| } else if !own { |
| @@ -702,13 +703,18 @@ func (s *Server) startBillingCheckoutQuoted(ctx context.Context, acct *Account, |
| return "", err |
| } |
| if intent.SessionID != "" { |
| - session, err := b.client.GetCheckout(ctx, intent.SessionID) |
| + session, err := s.readCheckout(ctx, intent.SessionID) |
| if err != nil { |
| return "", err |
| } |
| if session.ID != intent.SessionID { |
| return "", errBillingIdentity |
| } |
| + if st, err := s.store.refusalState(acct.ID, intent.ID); err != nil { |
| + return "", err |
| + } else if st.Disputed != 0 { |
| + return "", errBillingUnderReview |
| + } |
| if own, err := s.ownBoundSession(ctx, session, intent, acct, now); err != nil { |
| return "", err |
| } else if !own { |
| @@ -803,7 +809,7 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| if b == nil { |
| return errBillingUnavailable |
| } |
| - sub, err := b.client.GetSubscription(ctx, id) |
| + sub, err := s.readSubscription(ctx, id) |
| if err != nil { |
| return err |
| } |
| @@ -819,7 +825,7 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| if err != nil { |
| return err |
| } |
| - sub, err = b.client.GetSubscription(ctx, id) |
| + sub, err = s.readSubscription(ctx, id) |
| if err != nil { |
| return err |
| } |
| @@ -829,19 +835,10 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| 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 |
| - } |
| - } |
| + // (The subscription was handed to the store when it was read, before |
| + // any answer here: readSubscription. Under a refusal review it is |
| + // attached to the case; naming an intent retired on a proven expiry, |
| + // or whose purchase was kept and recorded, it is a dispute.) |
| // 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 |
| @@ -869,6 +866,17 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| } |
| } |
| refuse := func() error { |
| + if pending != nil && pending.SessionID == "" && pending.ID == sub.Metadata["checkout_intent"] { |
| + // The standing intent the subscription names was never bound |
| + // to a session: the refusal is filed all the same, with the |
| + // subscription attached and no session named (the pass finds |
| + // the intent's session, if one was made), so the hold knows of |
| + // the subscription rather than a rejected row alone. |
| + if err := s.store.fileCheckoutRefusal(*pending, "", id, now); err != nil { |
| + return err |
| + } |
| + return errBillingIdentity |
| + } |
| if pending != nil && pending.SessionID != "" && pending.ID == sub.Metadata["checkout_intent"] { |
| // The bound session is read before the review names it. The |
| // review is filed, the subscription recorded as attached to |
| @@ -877,7 +885,7 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| // buyer's session waits, has its expiry requested by its |
| // bound id); the intent is released only by the operator, on |
| // what the store has recorded (billingretirement.go). |
| - session, err := b.client.GetCheckout(ctx, pending.SessionID) |
| + session, err := s.readCheckout(ctx, pending.SessionID) |
| if err != nil { |
| return err |
| } |
| @@ -895,10 +903,7 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| // 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.ID, acct, session, now); err != nil { |
| + if err := s.fileRefusal(*pending, pending.SessionID, id, session, now); err != nil { |
| return err |
| } |
| if err := s.advanceRefusal(ctx, *pending, acct, now); err != nil { |
| @@ -966,9 +971,13 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| } |
| if pending == nil { |
| // No intent stands, and the subscription names no retired one |
| - // (that was settled above): not settled, the provider tries |
| - // again. |
| - return sql.ErrNoRows |
| + // (that was settled above). Nothing can ever record from it (a |
| + // first purchase records only against its standing intent), |
| + // so it is settled with its account, in the rejected list, |
| + // rather than retried until the provider gives up and seen |
| + // nowhere. (If it named an intent whose purchase was kept and |
| + // recorded, the store recorded a dispute when it was read.) |
| + return &billingIdentityOfAccount{AccountID: acct.ID} |
| } |
| intent := *pending |
| if intent.ID != sub.Metadata["checkout_intent"] { |
| @@ -994,7 +1003,7 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| // this reconcile then succeeds. Never file it for review. |
| return errBillingPending |
| } |
| - session, err := b.client.GetCheckout(ctx, intent.SessionID) |
| + session, err := s.readCheckout(ctx, intent.SessionID) |
| if err != nil { |
| return err |
| } |
| @@ -1006,9 +1015,24 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| // session of this subscription and its customer. |
| if own, err := s.ownBoundSession(ctx, session, intent, acct, now); err != nil { |
| return err |
| - } else if !own || session.SubscriptionID != id || session.CustomerID != sub.CustomerID { |
| + } else if !own { |
| return errBillingIdentity |
| } |
| + if session.SubscriptionID == "" { |
| + // A completed session that names no subscription yet says |
| + // nothing about which one is its own. Retried: the binder |
| + // treats the same read as pending. |
| + return errBillingPending |
| + } |
| + if session.SubscriptionID != id || session.CustomerID != sub.CustomerID { |
| + // The intent's own session, complete, and this is not its |
| + // subscription: a subscription made outside checkout on the |
| + // purchase's own terms. It is not the purchase, and it is not |
| + // a bare rejection either: the refusal is filed with it |
| + // attached, so no keep of the checkout's purchase releases |
| + // the hold beside it (review twenty, Opus P2-1). |
| + return refuse() |
| + } |
| // And the purchase the intent describes: plan, band, terms, price |
| // terms, and for a book purchase its currency with no conversion |
| // on it. The intent's own session completed for anything else is |
| @@ -1017,7 +1041,7 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| // meets the same judgement and records nothing; the intent stands |
| // until the operator resolves the review. |
| if !b.intentSession(session, intent, acct) { |
| - if err := s.store.fileCheckoutRefusedReview(intent, session.ID, now); err != nil { |
| + if err := s.fileRefusal(intent, session.ID, id, session, now); err != nil { |
| return err |
| } |
| return errBillingIdentity |
| @@ -1148,23 +1172,20 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| if b == nil { |
| return "", errBillingUnavailable |
| } |
| - session, err := b.client.GetCheckout(ctx, sessionID) |
| + session, err := s.readCheckout(ctx, sessionID) |
| if err != nil { |
| 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. |
| + // (What this read shows was handed to the store when it was read: |
| + // readCheckout.) A session of a purchase that no longer stands |
| + // can never bind anything: its event is settled with its account, |
| + // not retried for days. |
| 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 |
| - } |
| + if accountID, archived, err := s.store.sessionArchived(sessionID); err != nil { |
| + return "", err |
| + } else if archived { |
| + return "", &billingIdentityOfAccount{AccountID: accountID} |
| } |
| } |
| return "", errBillingPending |
| @@ -1183,10 +1204,23 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| return "", errBillingIdentity |
| } |
| } |
| + // A completed session of a purchase that no longer stands (retired, or |
| + // kept and recorded) is settled with the account the console recorded |
| + // for it, whatever account its answer names now and whatever intent |
| + // stands now; the one thing it can still do is name the account's |
| + // recorded purchase again, below. (What it shows was recorded when it |
| + // was read.) |
| + archivedAccount, archived, err := s.store.sessionArchived(sessionID) |
| + if err != nil { |
| + return "", err |
| + } |
| acct, err := s.store.AccountBySqid(session.Metadata["account_sqid"]) |
| if err != nil { |
| return "", err |
| } |
| + if archived && (acct == nil || acct.ID != archivedAccount || acct.Sqid != session.ClientReferenceID) { |
| + return "", &billingIdentityOfAccount{AccountID: archivedAccount} |
| + } |
| if acct == nil || acct.Sqid != session.ClientReferenceID { |
| return "", errBillingIdentity |
| } |
| @@ -1198,6 +1232,9 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| if state.Subscription == session.SubscriptionID && state.Customer == session.CustomerID { |
| return session.SubscriptionID, nil |
| } |
| + if archived { |
| + return "", &billingIdentityOfAccount{AccountID: acct.ID} |
| + } |
| // The completion of a session whose intent the operator retired (a |
| // refusal resolved) is settled, not retried, whatever intent stands |
| // now: nothing can ever be recorded from it, and its refusal names the |
| @@ -1207,22 +1244,13 @@ 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 |
| - } |
| + // No intent stands and none was retired under that name: nothing |
| + // can be bound, now or later. Settled with its account. |
| + return "", &billingIdentityOfAccount{AccountID: acct.ID} |
| } |
| if err != nil { |
| return "", err |
| @@ -1234,19 +1262,8 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| // there on an owned, completed session is a review, not a silent |
| // refusal. |
| 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.ID, acct, session, time.Now()); err != nil { |
| - return "", err |
| - } |
| return "", errBillingIdentity |
| } |
| - // 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.ID, acct, session, time.Now()); err != nil { |
| - return "", err |
| - } |
| if !b.intentSession(session, intent, acct) { |
| // This intent's own session (its id, account, customer, mode), |
| // completed for another plan, band or terms, on other price terms |
| @@ -1258,10 +1275,7 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| // purchase opens beside it, until the operator resolves the review, |
| // which retires exactly that intent or keeps it for a purchase |
| // corrected at the provider to record. |
| - if err := s.store.fileCheckoutRefusedReview(intent, session.ID, time.Now()); err != nil { |
| - return "", err |
| - } |
| - if err := s.noteSessionEvidence(intent.ID, acct, session, time.Now()); err != nil { |
| + if err := s.fileRefusal(intent, session.ID, "", session, time.Now()); err != nil { |
| return "", err |
| } |
| return "", errBillingIdentity |
| @@ -1474,7 +1488,7 @@ func (s *Server) recoverReadyIntentSession(ctx context.Context, acct *Account, i |
| // Read-only proof: list this customer's checkouts over the intent's |
| // lifetime (bounded, never a global window). If the lost create actually |
| // reached Stripe, its session is listable and is bound without any POST. |
| - sessions, err := b.client.ListCheckoutsForCustomer(ctx, intent.Customer, intent.CreatedAt-60, now.Unix()) |
| + sessions, err := s.listCheckouts(ctx, intent.Customer, intent.CreatedAt-60, now.Unix()) |
| if err != nil { |
| return nil, err |
| } |
diff --git a/internal/console/billingservice_test.go b/internal/console/billingservice_test.go| index 65728c111..72606347e 100644 |
| --- a/internal/console/billingservice_test.go |
| +++ b/internal/console/billingservice_test.go |
| @@ -67,6 +67,9 @@ 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 |
| + // sessionNoSubscription makes a completed session carry no |
| + // subscription id at all: read before the provider attached one. |
| + sessionNoSubscription bool |
| // otherSubscriptions are further subscription ids the provider knows |
| // (sub_paid always exists), served on the same terms; and |
| // subscriptionSecondItem gives every subscription a second item. |
| @@ -171,6 +174,9 @@ func (f *fakeBillingStripe) session() map[string]any { |
| if f.sessionSubscription != "" { |
| sub = f.sessionSubscription |
| } |
| + if f.sessionNoSubscription { |
| + sub = "" |
| + } |
| } |
| if f.paymentStatus != "" { |
| payment = f.paymentStatus |
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go| index 715d9b401..a3780a9be 100644 |
| --- a/internal/console/billingstore.go |
| +++ b/internal/console/billingstore.go |
| @@ -882,8 +882,10 @@ type billingReviewResolution struct { |
| // the operator bit), the console's existing operator identity. Note is a |
| // short technical reason code, never provider or customer data. |
| Actor, Note string |
| - // Outcome, for a checkout refusal: "retire" (the default, the money is |
| - // settled and the intent is retired, with its record) or "keep" (the |
| + // Outcome, for a checkout refusal: "retire" (the default: the intent |
| + // is retired, with its record; in stage one only an intent whose |
| + // session is proven expired and unpaid, with nothing attached, so |
| + // there is no money to settle) or "keep" (the |
| // purchase was corrected at the provider; the intent stands and the |
| // next event records it). The operator door accepts keep only for an |
| // intent whose bound session is complete with a subscription, since |
| @@ -973,7 +975,15 @@ 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, evidenceSession, evidenceSubscription := "", "", "" |
| + // A dispute has no resolution in stage one: it is evidence the |
| + // provider contradicted itself about a session, or showed something |
| + // about a purchase that was already disposed of, and how such a case |
| + // is adjudicated is not decided. Resolving it as any other review |
| + // would lift the account's hold on a note alone. |
| + if r.Reason == billingFateDisputedReason { |
| + return &billingReviewHeld{Why: "a dispute about a checkout session has no resolution yet: how contradictory provider evidence is adjudicated is not decided"} |
| + } |
| + evidence, evidenceSession, evidenceSubscription, evidenceCustomer := "", "", "", "" |
| if r.Reason == billingCheckoutRefusedReason { |
| st, err := refusalStateQ(tx, r.AccountID, r.Subscription) |
| if err != nil { |
| @@ -984,7 +994,7 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e |
| } |
| evidence = st.evidence() |
| if st.Stands { |
| - evidenceSession, evidenceSubscription = st.pinned(), st.SessionSubscription |
| + evidenceSession, evidenceSubscription, evidenceCustomer = st.pinned(), st.SessionSubscription, st.Customer |
| } |
| } |
| // Never record a resolution earlier than the occurrence it resolves. |
| @@ -1001,12 +1011,14 @@ 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,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 { |
| + 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,evidence_customer) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?)`, |
| + r.AccountID, r.Subscription, r.Reason, r.Occurrence, lastSeen, r.Actor, r.Note, at, r.outcome(), evidence, evidenceSession, evidenceSubscription, evidenceCustomer); err != nil { |
| return err |
| } |
| - // Resolving the review of a session paid in the wrong currency with |
| - // the outcome retire is the operator saying the money is settled. The |
| + // Resolving a checkout refusal with the outcome retire releases an |
| + // intent whose session is proven expired and unpaid with nothing |
| + // attached (stage one; the gate above: a session that was paid is not |
| + // retired by anything yet). The |
| // review is keyed by the intent (its subscription field holds the |
| // intent id), and exactly that intent, which was left standing so no |
| // second purchase opened beside the refused one, is retired in the |
| @@ -1017,8 +1029,8 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e |
| // bind and the review does not come back. An intent that is not that |
| // one (a later purchase's) is left alone. |
| if r.Reason == billingCheckoutRefusedReason && r.outcome() == "retire" { |
| - var sessionID, plan, band, currency, book, fate string |
| - err := tx.QueryRow(`SELECT session_id,plan,band,currency,price_book,session_fate FROM billing_checkout_intents WHERE account_id=? AND id=?`, r.AccountID, r.Subscription).Scan(&sessionID, &plan, &band, ¤cy, &book, &fate) |
| + var sessionID, plan, band, currency, book, fate, customer string |
| + err := tx.QueryRow(`SELECT session_id,plan,band,currency,price_book,session_fate,customer FROM billing_checkout_intents WHERE account_id=? AND id=?`, r.AccountID, r.Subscription).Scan(&sessionID, &plan, &band, ¤cy, &book, &fate, &customer) |
| if err != nil && !errors.Is(err, sql.ErrNoRows) { |
| return err |
| } |
| @@ -1039,8 +1051,8 @@ func (s *Store) resolveBillingReview(r billingReviewResolution, now time.Time) e |
| if _, err := tx.Exec(`UPDATE billing_quantity_events SET processed_at=?,processing_outcome='checkout_refused_resolved',processing_reference=? WHERE account_id=? AND intent_id=? AND subscription='' AND processed_at=0`, at, reference, r.AccountID, r.Subscription); err != nil { |
| return err |
| } |
| - if _, err := tx.Exec(`INSERT INTO billing_retired_intents(account_id,intent_id,session_id,plan,band,currency,price_book,reason,actor,retired_at,session_fate) VALUES(?,?,?,?,?,?,?,?,?,?,?)`, |
| - r.AccountID, r.Subscription, sessionID, plan, band, currency, book, r.Reason, r.Actor, at, fate); err != nil { |
| + if _, err := tx.Exec(`INSERT INTO billing_retired_intents(account_id,intent_id,session_id,plan,band,currency,price_book,reason,actor,retired_at,session_fate,customer) VALUES(?,?,?,?,?,?,?,?,?,?,?,?)`, |
| + r.AccountID, r.Subscription, sessionID, plan, band, currency, book, r.Reason, r.Actor, at, fate, customer); err != nil { |
| return err |
| } |
| if _, err := tx.Exec(`DELETE FROM billing_checkout_intents WHERE account_id=? AND id=?`, r.AccountID, r.Subscription); err != nil { |
| @@ -1080,7 +1092,15 @@ func (s *Store) fileCheckoutRefusal(intent billingIntent, sessionID, subscriptio |
| return err |
| } |
| if !stands { |
| - return nil |
| + // The intent went (retired, or its purchase kept and recorded) |
| + // between the caller's read and this filing. The subscription it |
| + // was filing for is still evidence: classified here, in this |
| + // transaction, against what the intent was disposed on, so it is |
| + // a dispute and never nothing. |
| + if err := classifySubscriptionTx(tx, intent.AccountID, intent.ID, subscription, observed); err != nil { |
| + return err |
| + } |
| + return tx.Commit() |
| } |
| var open bool |
| if err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0)`, intent.AccountID, intent.ID, billingCheckoutRefusedReason).Scan(&open); err != nil { |