amadan
Multi-currency: the twelfth review's findings
The bound session read back as nobody's is expired by its own id before the review is filed, so it cannot be paid after the operator resolves (the answer retires nothing); the bound id alone identifies the console's session, so an edited intent metadata no longer wedges it; every identity refusal of a first purchase's subscription that names the bound intent is the intent-keyed review, price and currency included; tests tell reconcile's and the binder's reads from a plain ownership check; refuseSession loses its unused parameter. The twelfth review's reports; dispositions corrected and "Review twelve".
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
6 files changed,
+507
−60
docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review12.md+110 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+17 −3docs/superpowers/specs/OPUS-REVIEW-multi-currency-review12.md+123 −0internal/console/billingcurrency_test.go+178 −12internal/console/billingpass.go+2 −2internal/console/billingservice.go+77 −43
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review12.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review12.md| new file mode 100644 |
| index 000000000..29115cb61 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review12.md |
| @@ -0,0 +1,110 @@ |
| +| Eleventh-review item | Status at `781bb1c2` | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P3-1 — contradictory bound session leaves an intent stuck without review** | **Closed for the reported bound-session case.** `ownBoundSession` files a referenced, intent-keyed review at `internal/console/billingservice.go:289`. Unbound foreign create/listing responses remain refused without review or expiry. | **No.** `TestABoundSessionReadBackAsNobodysIsReviewed`, at `internal/console/billingcurrency_test.go:3329`, requires the review immediately after resume. | |
| +| **Opus P3-2 — subscription differs from its intent but receives no review** | **Partly closed.** The new review at `internal/console/billingservice.go:812` handles a self-consistent alternative purchase. Earlier price, currency and terms validation failures still bypass it. Finding 1 below. | **No for the implemented fix**, but the test at `internal/console/billingcurrency_test.go:3411` changes band metadata **and its matching price**. It does not cover changing the price alone. | |
| +| **Opus P3-3 — fake expiry always returns `cs_test_paid`** | **Code closed:** the fake returns the requested ID at `internal/console/billingservice_test.go:267`. | **Yes, for reverting only the ID fix.** Both cases in the cited expiry test use `cs_test_paid`. Retaining `expireMeta` while hardcoding that ID would leave those assertions passing. | |
| +| **Opus’s claims** | The review-ten disposition now acknowledges the unresolved-intent problem at `docs/superpowers/specs/DISPOSITIONS-multi-currency.md:231`. The fake’s response now agrees with the requested ID. Historical mutation executions remain unverifiable from source. | Prose corrections have no distinguishing behavioral test; the fake-ID coverage limitation is above. | |
| +| **Astra 1 — contradictory expiry metadata accepted as retirement proof** | **Closed.** `internal/console/billingstore.go:465` checks account metadata transactionally before committing retirement or processing quantity events. | **No.** The two service-path cases at `internal/console/billingcurrency_test.go:3452` and the store-proof mutation distinguish removal of this check. | |
| +| **Astra’s claims** | Review-four’s delete-predicate qualification and review-six’s mutation count are corrected at dispositions `:108` and `:156`. The already-recorded webhook exception is documented at `billingservice.go:242`. That ownership comment now needs another exception for `ownBoundSession`. | The account-only delete mutation remains equivalent under the enforced invariants. These prose corrections do not change that. | |
| + |
| +These are source counterfactuals, not executed mutation tests. All platform references below are at **`781bb1c2`**, reviewed against **`12e70d95`**, including the **`25487506..781bb1c2`** delta. Pricing-site references are at **`3f1610d`**. Nothing was modified; no tests or network commands were run. |
| + |
| +The pinned eleventh reports do **not** contain a numerical P2/P3 disagreement: both report P3 findings and accept with fixes. They disagree over whether contradictory expiry metadata is harmless. **Astra’s reading is correct:** the expiry request was authorized, but the contradictory answer formerly committed intent deletion and quantity-event closure. This supports P3, without demonstrating authority to expire another account’s session. |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P3 — Initial subscription validation still has paths that leave a paid purchase pending without an operator review.** |
| + |
| + **Where:** `internal/console/billingservice.go:776`, `:783`, `:786`, `:789`; the new review is later, at `:812`. |
| + |
| + **Concrete scenario:** A GBP Personal band-A checkout completes correctly. Before initial reconciliation, someone changes only the subscription item’s price to the configured GBP Personal band-D price, leaving its metadata, customer and checkout-intent ID unchanged. |
| + |
| + Reconciliation calculates the expected band-A identity, rejects the different price at `:786`, and returns before loading the standing intent or reaching the new review. The completion binder accepts the unchanged, correct session, but the subsequent reconciliation fails in the same place. Subscription-first and completion-first delivery therefore both leave the intent standing with no refusal review; the pass continues retrying. Webhook rejection records are not the operator’s review listing. |
| + |
| + The same omission applies to the earlier invalid-price-terms and currency checks. Nothing here demonstrates a conversion or incorrect local activation; the defect is failure to expose a paid, unresolved purchase to the operator. |
| + |
| + **Smallest acceptable fix:** Route initial-purchase price, terms, currency and quantity validation failures through the intent-keyed refusal path after establishing the standing intent and its bound session/subscription linkage. Preserve bare refusal for a subscription naming another intent. |
| + |
| + **Regression:** Keep the session and subscription metadata unchanged; change only `subscriptionPrice`. Assert one referenced review, no recorded subscription and the intent still standing, through both webhook orders and a pass. The existing test at `internal/console/billingcurrency_test.go:3428` changes both metadata band and price, so it avoids the failing guard. |
| + |
| +**Results on the other requested boundaries** |
| + |
| +- **A — GBP purchase end to end.** `GET ?country=IN¤cy=gbp` renders band D, with Personal at **£7.70**. The posted form supplies its country, GBP and signed quote; India also requires a postal code. `billingFormChoice` and `handleBillingCheckout` compare the effective purchase currency and quote before entering checkout (`internal/console/billinghandlers.go:71`, `:102`, `:117`). Checkout validates the configured GBP unit price and stores GBP/book on the intent (`internal/console/billingservice.go:454`, `:497`, `:559`). `CreateCheckout` sends GBP and disables Adaptive Pricing (`internal/billingstripe/client.go:622`). |
| + |
| + Binding and reconciliation require the purchase identity before recording it. Account and assignment writes preserve GBP/book (`internal/console/billingstore.go:599`, `:682`, `:725`). `billingInvoiceHistory` reads both assignment tables (`internal/console/billinginvoicestore.go:530`); `AllowanceForMonth` preserves the paid price terms; `priceBillingCalculation` produces a GBP version-2 snapshot (`internal/plans/month.go:173`; `internal/console/billingcalculation.go:174`). Invoice creation, items, recovery, verification and request hashes use that calculation’s currency (`internal/console/billinginvoice.go:859`, `:885`, `:964`, `:1024`, `:1116`). The quantity worker selects and confirms the stored purchase’s price/currency (`internal/console/billingquantity.go:1014`, `:1105`). I found no alternate-currency computation or historical repricing in this chain. |
| + |
| +- **B/M/Q/U — One form and quote integrity.** Changed country/currency, forged quote fields and a changed meaningful choice flag cannot buy an unrendered quote. A present currency must be exactly one sold value; empty, repeated, uppercase or padded values receive 400 on both doors after their preceding authentication checks (`internal/console/billinghandlers.go:176`). An absent currency field uses the country default on a book console and EUR on a legacy console, and still must satisfy the signed effective-currency comparison. |
| + |
| + The signature derives its key from the server’s billing secret and covers session token, account, book, country, band, currency, choice flag and member count (`internal/console/billingpage.go:446`). Disclosed form values cannot sign another tuple. An unchanged quote can be reused; another session, account, book or rotated token cannot reuse it. All selectable plans are displayed; quantity follows signed membership and plan minimum. Postal affects tax, which the page excludes from its pretax figures. Membership changes conflict at `billingservice.go:450`, with another count check under the membership lock at `billingstore.go:348`. |
| + |
| + Another tab’s cookie does not alter an existing form’s POST. The quote door checks session, CSRF and membership, then constructs an encoded, account-local billing redirect and performs no billing mutation (`billinghandlers.go:137`, `:197`). |
| + |
| +- **C/O — Cookies and held currencies.** Present-empty clears; absent falls back; invalid sets nothing (`internal/console/billingcurrency.go:43`). Consequently, an older cookie can reappear on a later parameter-absent GET after an **invalid** parameter; it cannot reappear after successful clearing. Cookies populate the page, never checkout’s POST decision. |
| + |
| + A held purchase currency overrides URL and cookie suggestions and is offered alone, including legacy EUR (`internal/console/billingpage.go:301`). Checkout refuses another currency even after cancellation (`internal/console/billingservice.go:546`). Without a choice or held currency, the quoted and purchased currency is the billing country’s default. |
| + |
| +- **D/L/R/W/Y/AA/DD/FF/HH — Ownership and binding.** `ownSession` checks mode, intent metadata, both account identifiers, the intent’s customer once assigned, and its bound session ID (`internal/console/billingservice.go:266`). Both create sites, recovery/listing, resume, the pass and binding enforce it before offering, expiring or recording a purchase. The expected-intent binder checks against the expected account before deriving an account from the response (`:997`). The two-account second-read case cannot bind or record B while processing A. |
| + |
| + `ownBoundSession` adds review filing for a contradictory read carrying the bound session ID and original intent ID. It is used on resume, the pass, initial reconcile and expected-intent binding. It does not turn an unowned create/listing response into a review. The webhook binder without an expected intent rejects conflicting ownership without attributing a review to an inferred victim; the pass can subsequently review the known bound session. Filing also checks that the exact intent still stands (`internal/console/billingstore.go:970`). |
| + |
| + No session naming two accounts is offered, bound, recorded or expired. Such an already-disclosed session can remain payable at Stripe until provider expiry or operator action; its review names it. **Leaving it untouched automatically is the right call under the stated ownership rule.** Expiring solely from the stored ID would introduce a separate authority rule for contradictory provider responses. |
| + |
| + The already-recorded-subscription webhook fast path remains an explicit exception: it returns the stored subscription identity without binding or recording another purchase (`billingservice.go:1023`). |
| + |
| +- **EE/II — Reconcile versus binder.** For session purchase-field mismatches, both use `intentSession` and the same intent-keyed review. Either event order produces one review and leaves the intent standing. Finding 1 is the remaining “none” case for subscription mismatches. |
| + |
| + The new subscription review does not misclassify ordinary membership changes: the quantity worker cannot update an unrecorded first subscription (`internal/console/billingquantity.go:918`, `:922`, `:952`). Once recorded, ordinary quantity changes and renewal bypass the initial-intent comparison (`internal/console/billingservice.go:801`). A canceled subscription’s late event either addresses the still-recorded subscription or fails the current-subscription/intent guards; it does not review a newer intent. Refusing a subscription naming another intent is correct. |
| + |
| +- **JJ — Expiry proof.** All provider-proof retirement paths converge on `retireIntent`; bound retirement additionally requires the exact session ID (`internal/console/billingstore.go:417`, `:430`, `:442`, `:465`). A contradictory account answer rolls back the preceding transactional delete. Operator resolution and discarding an intent that never reached session creation are separate retirement authorities. Both legacy and book checkout requests carry account metadata, so the new check does not require a field this code historically omitted. Real expiry-response preservation remains a test-mode check. |
| + |
| +- **E/H/N/T — Barrier and rollout.** The trigger matches the positive paid spans selected by `AllowanceForMonth`: active, non-Free, intersecting the month, ending at the earliest next assignment across either table, `period_end`, or month end (`internal/console/billingstore.go:208`; `internal/plans/month.go:146`). Same-second supersession gives the earlier revision zero duration. Free/inactive rows terminate predecessors but contribute no paid span themselves. Renewal and quantity dating move `effective_at`; the book writer’s `period_end` remains the subscription’s paid-through instant (`billingstore.go:618`, `:655`, `:725`). |
| + |
| + I found neither a valid history that lets an older version-1 freeze through over a contributing book span nor one where this build’s valid legacy-only version 1 is wrongly refused. New-trigger creation precedes old-trigger retirement. The retirement transaction checks the replacement exists before dropping the old trigger; failure preserves the old one, and another start is idempotent (`:797`). A crash can temporarily leave **both**, not neither. |
| + |
| + Startup supplies the new tables and legacy-default columns before serving. The barrier does not make an older binary understand book accounts or new quantity outcomes; the rollback prohibition remains necessary. |
| + |
| +- **S/V/X/Z/BB/CC/GG — Resolution, references and history holds.** Resolution selects, records and deletes exactly the reviewed intent and closes only its outstanding quantity events (`internal/console/billingstore.go:922`, `:940`, `:943`, `:947`). A newer intent survives. Retirement records the bound session ID or, for an unbound refusal, the review reference. Both the single reader and operator listing/API carry the reference; an open older review can acquire it without another occurrence. |
| + |
| + `quantityOutcomesSQL` is the shared admission list for the processing guard, startup validation and obligation check (`internal/console/billingquantityaudit.go:61`). Changed guard text causes replacement. No remaining production statement clears an existing history gap; pre-history and clock-rollback holds survive restart. |
| + |
| + The late-completion test now uses distinct session IDs and returns the retired session’s actual completed shape. Its assertion at `internal/console/billingcurrency_test.go:3005` would fail if that completion recorded billing state. It does not independently distinguish every guard along that path. |
| + |
| +- **F/G/J/K — Figures, books and pricing site.** Console inclusions divide `PriceFor`’s pooled allowance by charged quantity, including the unavailable Personal fallback (`internal/console/billingpage.go:395`). Current terms have exact per-member shares across every band and supported quantity; band A’s sentences are asserted verbatim (`internal/console/billingcurrency_test.go:2551`, `:2628`). |
| + |
| + Book validation covers complete prices, rounding, overflow, duplicate keys, positive rates/floors and legacy EUR equality (`internal/plans/pricebook.go:240`). Configuration requires the selected catalog, complete optional catalogs and unique provider IDs (`cmd/console/billing.go:124`). Provisioning verification accepts at least the selected book’s 36 keys and matches the console’s own-only currency-option rule. |
| + |
| + The public export contains commercial terms, not customer or credential information. The pinned site’s book is byte-identical to the platform export. Rates, scaled inclusions, exactness checks, approved numeric strings, country options and defaults derive from that export. The first-line marker and exact `PRICEBOOK_ALLOW_DRAFT=1` override are enforced by the normal build/check path. Links always contain both parameters, with currency populated only for an explicit choice. A locale-only suggestion can differ from the subsequent console default because it is deliberately not forwarded as a choice. |
| + |
| +**Claims the code contradicts** |
| + |
| +- **“Five mutants killed” is internally inconsistent.** Dispositions `:242` enumerates three killed mutations and two explicitly surviving service-check mutations. Correct it to **three killed, two survived**. That sentence also overstates agreement: the committed Astra eleventh report explicitly marked expiry-response ownership incomplete. |
| + |
| +- **The subscription-review disposition overstates closure.** Dispositions `:247` says subscription purchase mismatches now receive the review. The earlier validation returns described in Finding 1 remain outside it. |
| + |
| +- **The fake-ID regression does not establish its claimed coverage.** Dispositions `:249` cites the expiry test, but that test never expires `cs_test_second`. Add a successful expiry and retirement of that distinct session; retaining the default ID must then fail. |
| + |
| +- **The ownership comment needs the new review exception.** `internal/console/billingservice.go:241` says every reviewed session passes `ownSession`. `ownBoundSession` intentionally reviews one precisely when that predicate is false (`:289`). State that exception alongside the already-recorded webhook exception. |
| + |
| +- **The site’s euro-provenance test is vacuous.** At site commit `3f1610d`, `test/pricing-pricebook.test.mjs:162` removes the currency data attributes and searches the static USD page for euro amounts. There are none. It would pass with an incorrectly hardcoded USD/GBP amount. Compare generated values for all currencies against the export, or remove the misleading claim. |
| + |
| +- **Some historical assurance wording remains too broad.** Dispositions `:26` still says live-subscription refusal occurs before asking the provider; checkout performs provider reads first (`internal/console/billingservice.go:476`), then refuses at `:536`. It prevents session creation. |
| + |
| +The review-three equivalent default-as-choice mutation is correctly described at dispositions `:102`. Review-four’s delete qualification and review-six’s corrected count now agree with the code. Provisioning tests inspect shell text; the successful trigger-migration test does not prove failure ordering. Historical scratch executions cannot be established from these files. |
| + |
| +The fixtures still use wall-clock time for authentication, webhook freshness and relative session expiry (`internal/console/billingservice_test.go:130`). I found no new calendar-date, test-order, temporary-root or shared-cookie-jar defect in the reviewed changes. |
| + |
| +**What must be verified against Stripe test mode before enablement** |
| + |
| +1. **Catalog and checkout matrix:** Verify all 36 identities, expanded own-only `currency_options`, amounts and metadata. Complete USD/EUR/GBP purchases across countries, including IN with GBP and GBP Community B/D rounding. Confirm actual line price, quantity, presented currency, subscription/item currency and disabled Adaptive Pricing. |
| + |
| +2. **Independent provider mutations and event ordering:** Change session metadata separately from subscription metadata; change subscription price alone as well as price plus metadata. Deliver completion/subscription events in both orders and replay them. Correct purchases must record once; attributed mismatches must leave one referenced review and no activation. |
| + |
| +3. **Recovery and expiry:** Lose create responses, replay the original key and recover by customer listing. Verify expiry responses retain session ID, intent/account metadata, client reference, customer and mode. Completed sessions must not be retired as unpaid expiries. |
| + |
| +4. **Refusal resolution and late delivery:** Resolve a refused intent, open a distinctly identified replacement checkout, then deliver the retired session’s completion. The replacement must remain untouched and the old purchase unrecorded. Exercise contradictory bound ownership and confirm the operator can identify and settle the provider-side session. |
| + |
| +5. **Overages in each currency:** Create, recover, finalize and pay invoices; check invoice/item/line currency, exact subtotal allocation, floor boundaries, tax and house waivers. Replay legacy EUR request shapes and verify stored calculations are not repriced. |
| + |
| +6. **Quantity and lifecycle:** Exercise increases, proration, renewal and the quantity fence in each currency. Confirm price/currency acknowledgements and allowance dating. Establish customer currency restrictions after cancellation and with outstanding invoice items, including discount behavior. |
| + |
| +**Verdict: accept with fixes.** |
| \ 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 c08cfa4be..ef8a24f90 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -239,15 +239,29 @@ Still open after this round, as both reviewers list: everything that needs Strip |
| ## Review eleven (reviewed `25487506`; Opus accepted with fixes, Astra accepted with fixes) |
| -Reports: `OPUS-REVIEW-multi-currency-review11.md`, `CODEX-FINDINGS-multi-currency-review11.md` (in the tree from this commit, with the tenth's since `28dc2629`). Both found every tenth-round finding closed, each with a test that fails without its fix, and nothing in a money path; both accept with fixes for the second round running. The fixes, each with a test that fails without it; five mutants killed in a scratch copy (the bound-session review never filed; the subscription mismatch's review never filed; the store's account-metadata check on an expiry's answer removed; and, surviving by construction, the two service-level copies of that check, removed in favour of the one in the store): |
| +Reports: `OPUS-REVIEW-multi-currency-review11.md`, `CODEX-FINDINGS-multi-currency-review11.md` (in the tree from this commit, with the tenth's since `28dc2629`). Both found every tenth-round finding closed, each with a test that fails without its fix, and nothing in a money path; both accept with fixes for the second round running. The fixes, each with a test that fails without it; three mutants killed in a scratch copy (the bound-session review never filed; the subscription mismatch's review never filed; the store's account-metadata check on an expiry's answer removed) and two surviving by construction (the two service-level copies of that check, removed in favour of the one in the store). Astra's eleventh report marked the expiry-answer ownership incomplete; both accepted with fixes: |
| | Finding | Disposition | Where | Reproduction | |
| | --- | --- | --- | --- | |
| | Opus P3-1: the session an intent is bound to, read back as nobody's (its account metadata, client reference or customer changed at the provider), wedged the intent: every click and pass refused, nothing expired, no review, the account unable to buy or be deleted, and a payment through the URL the buyer held produced only a rejected-events row. | Fixed: `ownBoundSession`, used wherever the bound session is read (resume of the previous and the current intent, the pass, reconcile, the expected-intent binder), files the intent-keyed review with the session as its reference when the read is of the bound session (its id and the intent's id) and `ownSession` says no; the intent stands until the operator resolves the review, which retires it; the session is left as it is at the provider (the console acts on no session it cannot call its own; the review names it for the operator). A read that is not of the bound session (a listing, a create response) files nothing. The existing "bound session completed for another customer" case now expects that one review. | `billingservice.go` `ownBoundSession`, `billingpass.go` | `TestABoundSessionReadBackAsNobodysIsReviewed` (the read files one review naming the session; paid through the URL, a pass and both webhooks record nothing and add nothing; the resolution retires the intent and a new purchase opens); `TestOwnershipIsEnforcedOnEveryPath` | |
| -| Opus P3-2: a subscription that names the intent and is not its purchase (plan, band, terms, price terms or quantity changed at the provider between payment and recording) was refused with no review. | Fixed: once the intent is bound, reconcile files the same intent-keyed review with the bound session as reference, and records nothing; a subscription naming another intent is still a bare refusal. | `billingservice.go` `reconcileBillingSubscription` | `TestASubscriptionNotTheIntentsPurchaseIsReviewed` (the session unchanged through `sessionMeta`, the subscription's band and price another band's) | |
| +| Opus P3-2: a subscription that names the intent and is not its purchase (plan, band, terms, price terms or quantity changed at the provider between payment and recording) was refused with no review. | Fixed for plan, band, terms, price terms and quantity: once the intent is bound, reconcile files the same intent-keyed review with the bound session as reference, and records nothing; a subscription naming another intent is still a bare refusal. [Review twelve: the price and currency checks returned before the intent was read, so a price swapped alone was still a bare refusal; fixed in the twelfth-round commit.] | `billingservice.go` `reconcileBillingSubscription` | `TestASubscriptionNotTheIntentsPurchaseIsReviewed` (the session unchanged through `sessionMeta`, the subscription's band and price another band's) | |
| | Astra 1 (P3): an authorised expiry's answer that named another account in its metadata was accepted as proof and retired the intent. | Fixed in the store: `retireIntent` requires the answer's account metadata to name the account, beside the client reference, customer, intent, mode, status and payment it already required. The two service paths that ask for the expiry rely on that one check (two copies there were written and removed: the store's check made them indistinguishable). | `billingstore.go` `retireIntent` | `TestAnExpiryAnsweredForAnotherAccountRetiresNothing` (both paths: a changed currency, and a session read back in another currency; the intent and its session as they were, no replacement opened); `TestQuantityAuditExpiredCheckoutRequiresBoundProviderProof` (the new mutation) | |
| -| Opus P3-3: the fake's expiry answer always said `cs_test_paid`. | Fixed: it names the session that was asked for, and carries `expireMeta` when a test sets it. | `billingservice_test.go` | the expiry test above | |
| +| Opus P3-3: the fake's expiry answer always said `cs_test_paid`. | Fixed: it names the session that was asked for, and carries `expireMeta` when a test sets it. [Review twelve: the expiry test expired `cs_test_paid` alone, so the id fix was not pinned; the resolution test now expires and retires `cs_test_second`.] | `billingservice_test.go` | the resolution test's last step | |
| | Astra claims: review four's qualification about the delete predicate; review six's mutant count; `intentSession`'s "every session" and the already-recorded fast path. | Corrected in place; the fast path is named in the comment. | this file, `billingservice.go` | | |
| | Opus claims: the review-ten row for Astra 2 omitted the wedge. | Noted on that row, with the fix here. | this file | | |
| Still open, as both reviewers list: everything that needs Stripe test mode (both lists, with session metadata mutability and a subscription changed between payment and recording added this round), the two unreviewed system strings, the operator's door for a quantity-history gap, and a listing of rejected events for the operator (not built; the reviews above are what reaches the operator). The twelfth review reads this commit. |
| + |
| +## Review twelve (reviewed `781bb1c2`; Opus accepted with fixes, Astra accepted with fixes) |
| + |
| +Reports: `OPUS-REVIEW-multi-currency-review12.md`, `CODEX-FINDINGS-multi-currency-review12.md` (in the tree from this commit). Both found every eleventh-round finding closed, each with a test that fails without it, and nothing in a money path; both accept with fixes for the third round running. The two disagreed on one point, which the code settles below. Fixed, each with a test that fails without it; five mutants killed in a scratch copy (the bound session not expired; the intent-id clause restored; the price and currency checks back to a bare refusal; reconcile's and the binder's reads back to a plain ownership check): |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Opus P3-1: the bound session read back as nobody's was left open at the provider, so after the operator resolved the review the buyer could still pay through the URL, with nothing recorded or reviewed. Astra (HH) held that leaving it untouched was right under the ownership rule. | Fixed as Opus asked, and the code decides it: the console already expires a session by its bound id when the buyer changes a purchase parameter, on the authority of having created it (the idempotency key names the intent), not on the authority of the read; the id is unique to the account and mode at the provider, so the expiry can make nothing but this intent's purchase unpayable; and the answer is used for nothing (`retireIntent` judges every retirement, and a contradictory answer retires nothing). So `ownBoundSession` expires an open bound session first, then files the review; the intent stands until the operator resolves it. A read that is not of the bound session still files nothing and expires nothing. | `billingservice.go` `ownBoundSession` | `TestABoundSessionReadBackAsNobodysIsReviewed` (one expiry by the bound id, the session expired at the provider, the intent standing and bound; then resolution and a new purchase); the two-account resume case in `TestASessionNamingTwoAccountsIsNobodys` | |
| +| Opus P3-2: a bound session whose `checkout_intent` metadata was edited still wedged the intent in silence (the clause requiring the session's intent metadata to match). | Fixed: the bound id alone identifies the console's own session; the clause is gone. The two-account second-read cases now expect A's one review naming A's session, and none for B. | `billingservice.go` `ownBoundSession` | `TestBindersSecondReadIsJudgedAgainstTheIntent`, `TestOwnershipIsEnforcedOnEveryPath` ("another account's everything") | |
| +| Opus P3-3, Astra 1 (P3): a subscription naming the bound intent whose price or currency was changed at the provider was a bare refusal (those checks returned before the intent was read). | Fixed: for a first purchase the standing intent is read before the subscription is judged, and every identity refusal of a subscription that names that bound intent (its price terms, its price, its currency, its plan, band, terms or quantity) files the intent-keyed review with the bound session as reference; a subscription naming another intent, or with no intent standing, is refused as before. | `billingservice.go` `reconcileBillingSubscription` (`refuse`) | `TestASubscriptionRefusedForItsPriceAloneIsReviewed` (another band's price, another currency's price; subscription first, completion first, the pass) | |
| +| Opus P3-4: reconcile's and the expected-intent binder's uses of `ownBoundSession` were not told from a plain ownership check. | Fixed: a subscription event before any pass, with the session's customer alone changed, files through reconcile; a pass whose binder reads the session a second time and finds it another customer's files through the binder. | tests | `TestEachReaderOfTheBoundSessionFilesTheReview` | |
| +| Astra claims: the eleventh summary's mutant count; the subscription-review row overstated; the fake-id regression unpinned; the ownership comment; the site's euro test vacuous; a review-two sentence about when a live subscription is refused. Opus claims: the ownership comment; the `ownBoundSession` comment ("by hand"); `refuseSession`'s unused parameter; the row claim about reconcile and the binder. | Corrected in place: three killed, two surviving; the rows above; the resolution test expires and retires `cs_test_second`; the `intentSession` comment names its three exceptions; `refuseSession` lost the parameter; the site test (site `251c4b1`) checks every amount on the page in all three currencies against the book or an approved string. | this file, `billingservice.go`, site `test/pricing-pricebook.test.mjs` | | |
| + |
| +Still open, as both reviewers list: everything that needs Stripe test mode (both lists, with session metadata edits, resolve-then-pay, a price swapped between completion and recording, and the expire response's fields added this round), the two unreviewed system strings, the operator's door for a quantity-history gap, a listing of rejected webhook events for the operator. The thirteenth review reads this commit. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review12.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review12.md| new file mode 100644 |
| index 000000000..40ead1361 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review12.md |
| @@ -0,0 +1,123 @@ |
| +# Twelfth review of `feat/carloku-multi-currency-20261001` at `781bb1c2` (base `12e70d95`) |
| + |
| +**Verdict: accept with fixes.** All four round-eleven findings are closed, and each fix has a test that fails without it. Nothing in this round touches how money is computed, sent or stored. The remaining defects are all P3, and each needs someone with Stripe access to change an object at the provider. The two most important: |
| + |
| +- A session the console bound and later can no longer call its own is left payable after the operator's resolution (P3-1). |
| +- Reconcile still refuses without a review when the provider changes a subscription's price or currency (P3-3). |
| + |
| +## How I reviewed it |
| + |
| +I read the source only with `git show`, changed nothing and ran no tests. My scratch copies are deleted. |
| + |
| +The round's code delta (`25487506..781bb1c2`) touches: |
| + |
| +- `billingservice.go`: `ownBoundSession`, the new review in reconcile, and the new `acct` parameter on `refuseSession`. |
| +- `billingpass.go`. |
| +- `retireIntent` in `billingstore.go`. |
| +- Tests and the test fake. |
| + |
| +`internal/plans`, `cmd`, `billingstripe`, `billingpage.go`, `billinghandlers.go`, `billingcurrency.go`, the trigger code and the quantity code have not changed since `faf1bd25`. So sections A–C, E–K, M–Q, T–X and Z–DD stand as the tenth and eleventh reports traced them. I did not re-trace them, and I did not re-read the site at `3f1610d` (section K stands as those reports traced it). |
| + |
| +## Round-eleven findings |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P3-1**: a bound session read back as nobody's wedged the intent with no review | **Closed for the case reported.** `ownBoundSession` (`billingservice.go:289`) files the intent-keyed review on resume (`:592`, `:649`), the pass (`billingpass.go:195`), reconcile (`:847`) and the expected-intent binder (`:1005`). Two gaps remain: P3-1 and P3-2 below. | **No.** `TestABoundSessionReadBackAsNobodysIsReviewed` expects one review after the first read; the old code files none. The changed row in `TestOwnershipIsEnforcedOnEveryPath` also fails (it expects 1 review, the old code gives 0). Only the shared helper is tested this way, not each call site (P3-4). | |
| +| **Opus P3-2**: a subscription naming the intent but not matching its purchase was refused with no review | **Partly closed.** Plan, band, terms, price terms and quantity now file the review (`:812-826`). A changed price or currency still refuses with no review (P3-3). | **No.** `TestASubscriptionNotTheIntentsPurchaseIsReviewed` gets a bare refusal and no review without the fix. | |
| +| **Opus P3-3**: the fake's expiry answered every id as `cs_test_paid` | **Closed.** The fake answers with the id asked for, plus `expireMeta`. | Test infrastructure; nothing to fail. | |
| +| **Astra 1 (P3)**: an expiry answer naming another account in its metadata retired the intent | **Closed.** `retireIntent` (`billingstore.go:465`) now checks `metadata.account_sqid` inside the transaction, after the `DELETE`, so a mismatch rolls the delete back. Every retirement on proof goes through it: `refuseSession`, both resume expiries, and the pass's `expired` branch. | **No.** In both subtests of `TestAnExpiryAnsweredForAnotherAccountRetiresNothing` the old store retires the intent. "The buyer changes currency" would also open a second session (`checkoutCalls == 2`). The new `account_sqid` case in `TestQuantityAuditExpiredCheckoutRequiresBoundProviderProof` fails as well. | |
| +| **Claims** (both reviewers: review four's predicate sentence, review six's count, the fast-path comment; Opus: the review-ten row) | **Closed**, except that review eleven's own summary repeats the counting error (claim 2 below). | n/a | |
| + |
| +**Severity disagreement.** There was none: Opus did not report Astra's finding at all. Opus's FF section called it harmless, Astra called it P3, and the code supports both readings: |
| + |
| +- **Opus was right on consequence.** The answer only arrives after the console expired its own bound id. The store already required the answer to be expired, unpaid, with no subscription, and to carry this intent's id, client reference and customer. So retiring on it could never leave anything payable. |
| +- **Astra was right on consistency.** The answer was accepted as proof by a weaker test than the one the predicate defines. P3 is right, and the fix cost nothing. |
| + |
| +## Findings |
| + |
| +### P3-1. A bound session the console can no longer call its own stays payable, and resolving the review retires the intent anyway |
| + |
| +- **Where:** `billingservice.go:289-299` (`ownBoundSession`); `billingstore.go:937-949` (the resolution retires the intent whatever the session's state). |
| +- **Scenario:** |
| + 1. A GBP intent is bound to `cs_X`. At Stripe, someone changes `cs_X`'s `metadata.account_sqid` (or its client reference or customer). |
| + 2. The next click or pass files the review naming `cs_X`, which is correct, and leaves `cs_X` open. |
| + 3. The operator reads "review names the session", refunds or decides nothing was paid, and resolves. The intent is retired, and the account starts a new purchase. |
| + 4. Within the session's lifetime, the buyer pays through the URL they still have: |
| + - The completion webhook derives the account from the edited metadata and is refused, leaving only a `billing_rejected_events` row. |
| + - The subscription webhook reaches reconcile. With no intent standing, `billingCheckoutIntent` returns `sql.ErrNoRows`, which the handler answers 503, so Stripe retries for days. If the new intent stands, its id mismatches at `:809` and the event is refused outright. |
| + 5. The customer is charged and nothing is recorded or reviewed. |
| + |
| + The test pays before resolving, so it never covers this ordering. |
| +- **Is leaving it open the right call (HH)?** No. The id is the console's own: it was bound only after `intentSession` passed, and Stripe ids are unique within the account and mode. Expiring it can only make this intent's purchase unpayable; it cannot act on anyone else's. |
| + |
| + The expiry's answer would then still name another account, so `retireIntent` refuses it, the intent stands and the review stands. That is exactly the desired state. |
| + |
| + The alternative the code relies on is "the operator expires it by hand". There is no console or CLI door for that. Under AGENTS.md's "Operate via API and CLI" rule, that is a product gap, not a procedure. |
| +- **Smallest fix:** in `ownBoundSession`, when the read is of the bound id and its status is `open`, call `ExpireCheckout(intent.SessionID, "carloku-expire-"+intent.ID)` and use the answer for nothing. Then file the review as now. |
| +- **Test:** read the session as nobody's, expect one expiry and one review, resolve, then pay. Expect nothing payable and nothing recorded. |
| + |
| +### P3-2. A bound session whose `checkout_intent` was edited still wedges the intent in silence |
| + |
| +- **Where:** `billingservice.go:293`, the clause `session.Metadata["checkout_intent"] == intent.ID`. |
| +- **Scenario:** at Stripe, `cs_X`'s metadata is edited or replaced, so `checkout_intent` no longer names the intent. Then: |
| + - every click and pass returns `errBillingIdentity`; |
| + - no review is filed; |
| + - the account cannot buy; |
| + - `resolveIntentForDeletion` fails through `reconcilePendingIntent`, so the account cannot be deleted. |
| + |
| + This is round eleven's P3-1 in a narrower form. The test "the bound session answered with another account's everything" (`billingcurrency_test.go:~2817`) asserts no review for A, so it locks the gap in. |
| +- **Fix:** drop that clause. `session.ID == intent.SessionID` already identifies the console's own bound session, and the resume and pass paths check the id first. A review on A names A's own session and acts on nothing. |
| + |
| + Change that test to expect one review for A and none for B. |
| + |
| +### P3-3. A subscription naming the bound intent whose price or currency was changed at the provider is still a bare refusal |
| + |
| +- **Where:** `billingservice.go:786` (price) and `:789` (currency) return `errBillingIdentity` before the intent is looked up at `:809`. |
| +- **Scenario:** between payment and recording, someone at Stripe swaps the subscription item to the USD price, or to any price outside the catalog, while its metadata still says GBP. `billingCurrencyAgrees` fails, and the only trace is a rejected-events row. |
| + |
| + This is the provider-side form of ruling 2's own worry: the customer is charged in a currency that is not the purchase's, and no operator is told. Round eleven's suggested fix said "on any intent and subscription mismatch". The disposition narrowed it to the metadata fields. |
| +- **Fix:** in the first-purchase case (`state.Subscription != id`), read the standing intent before the price, currency and `PriceFor` checks. When `sub.Metadata["checkout_intent"] == intent.ID` and `intent.SessionID != ""`, file the same review on every `errBillingIdentity` outcome. |
| +- **Test:** the subscription's item price set to another currency's price; expect one review and nothing recorded. |
| + |
| +### P3-4. No test distinguishes the reconcile or expected-binder call sites of `ownBoundSession` |
| + |
| +- **Where:** `billingservice.go:847` and `:1005`. |
| +- **Scenario:** revert either site to plain `ownSession` and every test still passes: |
| + - In "the bound session completed for another customer", the pass's own call (`billingpass.go:195`) files the review first and returns. |
| + - In `TestABoundSessionReadBackAsNobodysIsReviewed`, `f.meta["account_sqid"]="zzz"` also changes the subscription's metadata, so reconcile exits at `AccountBySqid` (`:747`) before reading the session. |
| + |
| + The only effect is that a subscription event arriving before any pass would file nothing until the pass does. |
| +- **Fix:** add a subscription-event-first test that changes only `sessionCustomer`, with no pass, and expects one review. For the binder site, add a pass test where the binder's second read (`sessionAnswers`) differs from the first. |
| + |
| +## The added sections |
| + |
| +- **HH.** Where `ownBoundSession` files: |
| + - It files on reads of the bound id only: resume of the previous and the current intent, the pass, reconcile, and the expected-intent binder. |
| + - It files nothing on a listing, a create response, or the webhook binder with no intent. That is correct: the webhook binder derives the account from the response, so filing there would act on the provider's claim. |
| + - **Only the account's standing intent can get a review.** `fileCheckoutRefusedReview` checks that `(account_id, id)` exists. The table is keyed by account, and an intent's session id never changes once set. So a review lands only on the standing intent, bound to the session it names. A stale pass snapshot of a retired intent files nothing. |
| + - The pass's unbound path passes `&intent` with `SessionID == ""` after `saveBillingCheckout`, so it can never file. That is harmless. |
| + - Leaving the session open is the wrong call (P3-1). |
| +- **II.** Reconcile's new review: |
| + - **A legitimate first purchase cannot file it.** The checkout quantity is fixed (no adjustable quantity). Member growth is locked until the purchase is resolved, and the quantity worker acts only on recorded subscriptions. Renewals and late events of a canceled subscription reach either the `state.Subscription == id` branch, or an intent whose id differs, which is a bare refusal. |
| + - The bare refusal for a subscription naming another intent is right, provided every retirement leaves nothing payable. P3-1 is the one retirement that does not. |
| + - The review names `intent.SessionID` without reading it. A subscription made by hand names the bound session as its reference, which only holds the intent; acceptable. |
| +- **JJ.** Every retirement on proof goes through `retireIntent`: `expireBillingCheckout` from three sites and the pass, and `retireRefusedCheckout`. The resolution and `discardUnreadyIntent` retire without proof, by design. |
| + |
| + No legitimate answer lacks `account_sqid`: the base build already set it (`12e70d95` derives the binder's account from it), and Stripe's expire response is the full session object. Older-build intents carry it too, and billing is off everywhere. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **`billingservice.go:240-243`** says "every session the console expires, reviews, binds or records from passes ownSession first". `ownBoundSession` reviews exactly the sessions that fail `ownSession`, and reconcile's new review (`:823`) files without reading the session at all. |
| +2. **Dispositions, review eleven summary:** "five mutants killed … and, surviving by construction, the two service-level copies". That is three killed and two surviving: the same counting error review ten corrected for review six. |
| +3. **The review-eleven Opus P3-1 row** says `ownBoundSession` is used at reconcile and the expected-intent binder, under the round's "each with a test that fails without it". Neither site is distinguished by a test (P3-4). |
| +4. **The `ownBoundSession` comment** says the review names the session "for the operator to expire by hand". No operator door does this, and the resolution does not wait for it (P3-1). The design document (line 152) says nothing of the bound-session review or of the session being left open. |
| +5. **`refuseSession`'s new `acct` parameter (`:324`) is unused.** The service-level checks it was added for were removed. Drop it, or the signature implies a check that is not there. |
| + |
| +## Verify in Stripe test mode before enabling |
| + |
| +Both eleventh-round lists stand. Add: |
| + |
| +1. **Metadata edits.** Edit an open Checkout Session's metadata (`account_sqid`, `checkout_intent`), client reference and customer, through the API and the dashboard. This shows whether P3-1 and P3-2 can occur, and whether the console's expiry of its own bound id succeeds after such an edit. |
| +2. **Resolve, then pay.** Resolve the review with the session still open, then pay through the held URL. Today this shows a charge that is never recorded or reviewed, and 503 retries on the subscription event. |
| +3. **Price and currency swap.** Swap a subscription item to another currency's price between completion and reconcile. Stripe may refuse a cross-currency swap on a customer locked to a currency; if it allows it, today the console refuses with no review (P3-3). |
| +4. **The expire response.** Confirm it carries `metadata`, `client_reference_id` and `customer`, as `retireIntent` now requires all three. |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index 58967f852..d6cba499d 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -2815,8 +2815,19 @@ func TestOwnershipIsEnforcedOnEveryPath(t *testing.T) { |
| stateB, _ := fx.st.billingAccount(other.ID) |
| afterB, _ := fx.st.billingCheckoutIntent(other.ID) |
| stateA, _ := fx.st.billingAccount(acct.ID) |
| - if stateB.Subscription != "" || afterB.SessionID != "" || stateA.Subscription != "" || f.expireCalls != 0 || reviews(t, fx, acct) != 0 || reviews(t, fx, other) != 0 { |
| - t.Fatalf("B %+v / %+v, A %+v, %d expiries; want nothing bound, recorded, expired or reviewed for either account", stateB, afterB, stateA, f.expireCalls) |
| + // Nothing is bound or recorded for either account. A's bound |
| + // session, read back as B's in everything but its id, is a |
| + // contradiction at the provider: A's intent-keyed review names it |
| + // (and the session, complete, is not expired); B, whose intent |
| + // never had a session, gets no review (review twelve: before it, |
| + // A's intent was wedged in silence because the read's intent |
| + // metadata named B's). |
| + if stateB.Subscription != "" || afterB.SessionID != "" || stateA.Subscription != "" || f.expireCalls != 0 || reviews(t, fx, acct) != 1 || reviews(t, fx, other) != 0 { |
| + t.Fatalf("B %+v / %+v, A %+v, %d expiries, A reviews %d, B reviews %d; want nothing bound, recorded or expired, one review for A naming its session, none for B", stateB, afterB, stateA, f.expireCalls, reviews(t, fx, acct), reviews(t, fx, other)) |
| + } |
| + var reference string |
| + if err := fx.st.db.QueryRow(`SELECT reference FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=?`, acct.ID, intentA.ID, billingCheckoutRefusedReason).Scan(&reference); err != nil || reference != intentA.SessionID { |
| + t.Fatalf("A's review names %q (%v); want A's bound session", reference, err) |
| } |
| }) |
| t.Run("a read of the bound session answered with another", func(t *testing.T) { |
| @@ -3008,6 +3019,18 @@ func TestRefusedPaidSessionReviewThroughTheWebhookAndItsResolution(t *testing.T) |
| if after, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || after.ID != second.ID || after.SessionID != "cs_test_second" { |
| t.Fatalf("the standing intent after the late completion: %+v %v", after, err) |
| } |
| + // And the second session is expired and its intent retired under its |
| + // own id when the buyer changes currency: the expiry's answer names |
| + // cs_test_second, as the provider's would. |
| + f.mu.Lock() |
| + f.sessionAnswers, f.sessionMeta, f.sessionStatus, f.sessionCurrency, f.sessionID = nil, nil, "open", "", "cs_test_second" |
| + f.mu.Unlock() |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanCommunity, "GB", "", "usd", f.now.Add(35*time.Minute)); err != nil || dest == "" { |
| + t.Fatalf("a purchase in another currency after cs_test_second: dest %q err %v", dest, err) |
| + } |
| + if third, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || third.ID == second.ID || third.Currency != "usd" || f.expireCalls != 1 { |
| + t.Fatalf("after changing currency: intent %+v %v, %d expiries; want cs_test_second expired by its own id and a new USD intent", third, err, f.expireCalls) |
| + } |
| } |
| // A session that names two accounts (one in its client reference, another |
| @@ -3032,12 +3055,19 @@ func TestASessionNamingTwoAccountsIsNobodys(t *testing.T) { |
| if dest, err := click(fx, f, acct, time.Minute); dest != "" || !errors.Is(err, errBillingIdentity) { |
| t.Fatalf("the session named another account: dest %q err %v; want no destination", dest, err) |
| } |
| - if f.expireCalls != 0 || f.sessionStatus != "open" { |
| - t.Fatalf("%d expiries, session %s; want a session that is nobody's left alone", f.expireCalls, f.sessionStatus) |
| + // The bound session is the console's own to expire (review |
| + // twelve), and the contradiction is a review; the answer to the |
| + // expiry retired nothing. |
| + if f.expireCalls != 1 || f.sessionStatus != "expired" { |
| + t.Fatalf("%d expiries, session %s; want the bound session expired by its own id", f.expireCalls, f.sessionStatus) |
| } |
| if after, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || after.ID != before.ID || after.SessionID != before.SessionID { |
| t.Fatalf("intent %+v %v; want it as it was", after, err) |
| } |
| + 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, before.ID, billingCheckoutRefusedReason).Scan(&n); err != nil || n != 1 { |
| + t.Fatalf("%d reviews (%v); want the one review of the bound session", n, err) |
| + } |
| }) |
| t.Run("a listed session with another account in its metadata is not bound", func(t *testing.T) { |
| fx, f, acct := newCurrencyFixture(t) |
| @@ -3241,8 +3271,16 @@ func TestBindersSecondReadIsJudgedAgainstTheIntent(t *testing.T) { |
| if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_reviews WHERE reason=?`, billingCheckoutRefusedReason).Scan(&reviewsEither); err != nil { |
| t.Fatal(err) |
| } |
| - if stateB.Subscription != "" || afterB.SessionID != "" || stateA.Subscription != "" || f.expireCalls != 0 || reviewsEither != 0 { |
| - t.Fatalf("B %+v / %+v, A %+v, %d expiries, %d reviews; want nothing done for either account", stateB, afterB, stateA, f.expireCalls, reviewsEither) |
| + // Nothing is bound, recorded or expired for either account; A's bound |
| + // session, read back as B's, is A's one intent-keyed review (review |
| + // twelve), and B, whose intent never had a session, gets none. |
| + var reviewsA int |
| + var referenceA string |
| + if err := fx.st.db.QueryRow(`SELECT count(*), coalesce(max(reference),'') FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=?`, acct.ID, intentA.ID, billingCheckoutRefusedReason).Scan(&reviewsA, &referenceA); err != nil { |
| + t.Fatal(err) |
| + } |
| + if stateB.Subscription != "" || afterB.SessionID != "" || stateA.Subscription != "" || f.expireCalls != 0 || reviewsEither != 1 || reviewsA != 1 || referenceA != intentA.SessionID { |
| + t.Fatalf("B %+v / %+v, A %+v, %d expiries, %d reviews (%d A's, naming %q); want nothing done for either account but A's one review naming its session", stateB, afterB, stateA, f.expireCalls, reviewsEither, reviewsA, referenceA) |
| } |
| } |
| @@ -3355,11 +3393,18 @@ func TestABoundSessionReadBackAsNobodysIsReviewed(t *testing.T) { |
| if open, n, ref := review(); open != 1 || n != 1 || ref != intent.SessionID { |
| t.Fatalf("after the read: %d open, %d occurrences, reference %q; want one review naming the session", open, n, ref) |
| } |
| - if f.expireCalls != 0 || f.sessionStatus != "open" { |
| - t.Fatalf("%d expiries, session %s; want the session left as it is", f.expireCalls, f.sessionStatus) |
| + // The session was the console's own to expire (its bound id), and was: |
| + // a buyer holding its URL can no longer pay through it. The answer to |
| + // that expiry retired nothing (it names another account). |
| + if f.expireCalls != 1 || f.sessionStatus != "expired" { |
| + t.Fatalf("%d expiries, session %s; want the bound session expired by its own id", f.expireCalls, f.sessionStatus) |
| } |
| - // Paid through the URL the buyer held: the pass and both webhooks |
| - // record nothing and add nothing to the one review. |
| + if standing, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || standing.ID != intent.ID || standing.SessionID != intent.SessionID { |
| + t.Fatalf("intent %+v %v; want it standing, bound as it was", standing, err) |
| + } |
| + // Were the provider to complete it anyway (an expiry answered and not |
| + // honoured): the pass and both webhooks record nothing and add |
| + // nothing to the one review. |
| f.mu.Lock() |
| f.sessionStatus, f.subscriptionPrice = "complete", intent.Price |
| f.mu.Unlock() |
| @@ -3376,8 +3421,8 @@ func TestABoundSessionReadBackAsNobodysIsReviewed(t *testing.T) { |
| if open, n, _ := review(); open != 1 || n != 1 { |
| t.Fatalf("after a pass and both webhooks: %d open, %d occurrences; want the one review", open, n) |
| } |
| - if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" || f.expireCalls != 0 { |
| - t.Fatalf("state %+v, %d expiries; want nothing recorded or expired", state, f.expireCalls) |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" || f.expireCalls != 1 { |
| + t.Fatalf("state %+v, %d expiries; want nothing recorded and no second expiry", state, f.expireCalls) |
| } |
| if standing, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || standing.ID != intent.ID { |
| t.Fatalf("intent %+v %v; want it standing", standing, err) |
| @@ -3497,3 +3542,124 @@ func TestAnExpiryAnsweredForAnotherAccountRetiresNothing(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// A subscription that names the bound intent and is refused for its price |
| +// alone (the item's price swapped at the provider to another band's or |
| +// another currency's, metadata untouched) is the intent-keyed review, in |
| +// either event order and through the pass, never a bare refusal. (Review |
| +// twelve, both reviewers: the price and currency checks returned before |
| +// the intent was read.) |
| +func TestASubscriptionRefusedForItsPriceAloneIsReviewed(t *testing.T) { |
| + for name, price := range map[string]string{ |
| + "another band's price": currencyPriceID(plans.PlanPersonal, plans.BandD, plans.GBP), |
| + "another currency's price": currencyPriceID(plans.PlanPersonal, plans.BandA, plans.USD), |
| + } { |
| + for _, order := range []string{"subscription first", "completion first", "the pass"} { |
| + t.Run(name+", "+order, func(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, err := fx.st.billingCheckoutIntent(acct.ID) |
| + if err != nil || intent.SessionID == "" { |
| + t.Fatalf("fixture: intent %+v %v", intent, err) |
| + } |
| + f.mu.Lock() |
| + f.sessionStatus, f.subscriptionPrice = "complete", price |
| + if strings.Contains(name, "currency") { |
| + f.subscriptionCurrency, f.itemCurrency = "usd", "usd" |
| + } |
| + f.mu.Unlock() |
| + post := func(id, typ, object string) { |
| + t.Helper() |
| + if code := f.webhookPost(t, fx, id, typ, object); code != http.StatusNoContent { |
| + t.Fatalf("%s: %d", typ, code) |
| + } |
| + } |
| + switch order { |
| + case "subscription first": |
| + post("evt_price_sub", "customer.subscription.created", "sub_paid") |
| + post("evt_price_done", "checkout.session.completed", intent.SessionID) |
| + case "completion first": |
| + post("evt_price_done", "checkout.session.completed", intent.SessionID) |
| + post("evt_price_sub", "customer.subscription.created", "sub_paid") |
| + default: |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)) |
| + } |
| + var occurrences int64 |
| + var reference string |
| + if err := fx.st.db.QueryRow(`SELECT occurrences, reference FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0`, acct.ID, intent.ID, billingCheckoutRefusedReason).Scan(&occurrences, &reference); err != nil || occurrences != 1 || reference != intent.SessionID { |
| + t.Fatalf("review: %d occurrences, reference %q, %v; want one review keyed by the intent naming its session", occurrences, reference, err) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded: %+v", state) |
| + } |
| + if standing, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || standing.ID != intent.ID { |
| + t.Fatalf("intent %+v %v; want it standing", standing, err) |
| + } |
| + }) |
| + } |
| + } |
| +} |
| + |
| +// Reconcile's own read of the bound session, and the expected-intent |
| +// binder's second read, each file the review for a session read back as |
| +// nobody's: a subscription event before any pass, with the session's |
| +// customer alone changed; and a pass whose binder reads the session a |
| +// second time and finds it another customer's. (Review twelve, Opus |
| +// P3-4: neither site was told apart from a plain ownership check.) |
| +func TestEachReaderOfTheBoundSessionFilesTheReview(t *testing.T) { |
| + review := func(t *testing.T, fx *consoleFixture, acct *Account, intent billingIntent) (int, string) { |
| + t.Helper() |
| + var n int |
| + var reference string |
| + if err := fx.st.db.QueryRow(`SELECT count(*), coalesce(max(reference),'') FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=? AND resolved_at=0`, acct.ID, intent.ID, billingCheckoutRefusedReason).Scan(&n, &reference); err != nil { |
| + t.Fatal(err) |
| + } |
| + return n, reference |
| + } |
| + t.Run("reconcile, on a subscription event before any pass", func(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() |
| + // The subscription is the intent's purchase for the intent's |
| + // customer; the session alone reads back as another customer's. |
| + f.sessionStatus, f.subscriptionPrice, f.sessionCustomer = "complete", intent.Price, "cus_other" |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_recon_customer", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the subscription event: %d", code) |
| + } |
| + if n, ref := review(t, fx, acct, intent); n != 1 || ref != intent.SessionID { |
| + t.Fatalf("%d reviews, reference %q; want reconcile to file the one review naming the session", n, ref) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded: %+v", state) |
| + } |
| + }) |
| + t.Run("the binder's second read, through the pass", func(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.subscriptionPrice = "complete", intent.Price |
| + // The pass's own read is the intent's own; the binder's read after |
| + // it is another customer's. |
| + f.sessionAnswers = []func(*fakeBillingStripe){func(*fakeBillingStripe) {}, func(f *fakeBillingStripe) { f.sessionCustomer = "cus_other" }} |
| + f.mu.Unlock() |
| + _ = fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)) |
| + if n, ref := review(t, fx, acct, intent); n != 1 || ref != intent.SessionID { |
| + t.Fatalf("%d reviews, reference %q; want the binder's second read to file the one review", n, ref) |
| + } |
| + if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "" { |
| + t.Fatalf("recorded: %+v", state) |
| + } |
| + }) |
| +} |
diff --git a/internal/console/billingpass.go b/internal/console/billingpass.go| index 658d38dab..10eea8830 100644 |
| --- a/internal/console/billingpass.go |
| +++ b/internal/console/billingpass.go |
| @@ -192,7 +192,7 @@ func (s *Server) reconcilePendingIntent(ctx context.Context, intent billingInten |
| if err != nil || owner == nil { |
| return err |
| } |
| - if own, err := s.ownBoundSession(session, intent, owner, now); err != nil { |
| + if own, err := s.ownBoundSession(ctx, session, intent, owner, now); err != nil { |
| return err |
| } else if !own { |
| return errBillingIdentity |
| @@ -216,7 +216,7 @@ func (s *Server) reconcilePendingIntent(ctx context.Context, intent billingInten |
| return err |
| } |
| if session.ID == intent.SessionID && !b.intentSession(session, intent, acct) && b.ownSession(session, intent, acct) { |
| - if err := s.refuseSession(ctx, intent, session.ID, session, owner, now); err != nil && !errors.Is(err, errBillingIdentity) { |
| + if err := s.refuseSession(ctx, intent, session.ID, session, now); err != nil && !errors.Is(err, errBillingIdentity) { |
| return err |
| } |
| } |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 7d7552258..a00e4936b 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -238,10 +238,13 @@ func (s *Server) checkoutParams(acct *Account, intent billingIntent) billingstri |
| // conversion on it. Every session the console binds to an intent, offers |
| // to a buyer or records a purchase from passes through here, however it |
| // was come by: created, replayed, listed, or read back by its id; and |
| -// every session the console expires, reviews, binds or records from |
| -// passes ownSession first. The one read that passes neither is the |
| -// webhook binder's fast path for a subscription already recorded, which |
| -// returns the recorded id and records nothing. |
| +// every session the console expires on proof, binds or records from |
| +// passes ownSession first. The exceptions: the webhook binder's fast path |
| +// for a subscription already recorded, which returns the recorded id and |
| +// records nothing; a bound session that fails ownSession, which is |
| +// reviewed and expired for that very reason (ownBoundSession); and the |
| +// review reconcile files for a subscription that is not its intent's |
| +// purchase, which names the bound session without reading it. |
| func (b *billingRuntime) intentSession(session *billingstripe.Session, intent billingIntent, acct *Account) bool { |
| if !b.ownSession(session, intent, acct) { |
| return false |
| @@ -276,25 +279,36 @@ func (b *billingRuntime) ownSession(session *billingstripe.Session, intent billi |
| // ownBoundSession is ownSession for a read of the session an intent is |
| // bound to, with one more thing when the answer is no: the console created |
| // that session and judged it its own when it bound it, so a read of it |
| -// that names another account, customer or mode is a contradiction at the |
| -// provider that a person must see. The intent-keyed review is filed with |
| -// the session as its reference (the same review a mismatched purchase |
| -// files); the intent stands, so no second purchase opens beside a session |
| -// that may still be paid, until the operator resolves the review, which |
| -// retires it; and the session is left as it is at the provider, since the |
| -// console acts on no session it cannot call its own (the review names it |
| -// for the operator to expire by hand). A read that is not of the bound |
| -// session (a listing, a create response) files nothing: nothing was ever |
| -// disclosed on it. |
| -func (s *Server) ownBoundSession(session *billingstripe.Session, intent billingIntent, acct *Account, now time.Time) (bool, error) { |
| +// that names another account, customer, intent or mode is a contradiction |
| +// at the provider that a person must see. If the session is still open it |
| +// is expired first, by the id the console itself bound (unique to this |
| +// account and mode at the provider, so the expiry can make nothing but |
| +// this intent's purchase unpayable), and the answer is used for nothing: |
| +// what the provider says of a session it has contradicted proves nothing, |
| +// so the intent is retired on no such proof. Then the intent-keyed review |
| +// is filed with the session as its reference (the same review a |
| +// mismatched purchase files); the intent stands, so no second purchase |
| +// opens beside it, until the operator resolves the review, which retires |
| +// it. A read that is not of the bound session (a listing, a create |
| +// response) files nothing: nothing was ever disclosed on it. |
| +func (s *Server) ownBoundSession(ctx context.Context, session *billingstripe.Session, intent billingIntent, acct *Account, now time.Time) (bool, error) { |
| if s.billing.ownSession(session, intent, acct) { |
| return true, nil |
| } |
| - if session != nil && intent.SessionID != "" && session.ID == intent.SessionID && session.Metadata["checkout_intent"] == intent.ID { |
| - if err := s.store.fileCheckoutRefusedReview(intent, session.ID, now); err != nil { |
| + if session == nil || intent.SessionID == "" || session.ID != intent.SessionID { |
| + return false, nil |
| + } |
| + if session.Status == "open" { |
| + if _, err := s.billing.client.ExpireCheckout(ctx, intent.SessionID, "carloku-expire-"+intent.ID); err != nil { |
| + // Not expired (the provider refused, or the session completed |
| + // under the read): nothing is filed on this pass; the next |
| + // read finds the session as it is now and files then. |
| return false, err |
| } |
| } |
| + if err := s.store.fileCheckoutRefusedReview(intent, session.ID, now); err != nil { |
| + return false, err |
| + } |
| return false, nil |
| } |
| @@ -321,7 +335,7 @@ func (s *Server) ownBoundSession(session *billingstripe.Session, intent billingI |
| // session is the session as last read, or as the client refused it when |
| // it was created. The caller has established that it is this intent's |
| // 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, acct *Account, now time.Time) error { |
| +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 |
| @@ -589,7 +603,7 @@ func (s *Server) startBillingCheckoutQuoted(ctx context.Context, acct *Account, |
| if session.ID != previous.SessionID { |
| return "", errBillingIdentity |
| } |
| - if own, err := s.ownBoundSession(session, previous, acct, now); err != nil { |
| + if own, err := s.ownBoundSession(ctx, session, previous, acct, now); err != nil { |
| return "", err |
| } else if !own { |
| return "", errBillingIdentity |
| @@ -597,7 +611,7 @@ func (s *Server) startBillingCheckoutQuoted(ctx context.Context, acct *Account, |
| if !b.intentSession(session, previous, acct) { |
| // Bound to this intent and no longer the purchase it describes: |
| // a buyer who kept its URL must not be able to pay through it. |
| - return "", s.refuseSession(ctx, previous, session.ID, session, acct, now) |
| + return "", s.refuseSession(ctx, previous, session.ID, session, now) |
| } |
| if session.Status == "open" && (previous.Currency != request.Currency || previous.PriceBook != request.PriceBook || previous.Plan != request.Plan || previous.Band != request.Band || previous.Quantity != request.Quantity || previous.Country != request.Country || previous.Price != request.Price || previous.Coupon != request.Coupon || previous.LocDigest != request.LocDigest || previous.LiveMode != request.LiveMode || previous.Members != request.Members) { |
| // A bound parameter changed — including the currency, a corrected postal code, |
| @@ -646,13 +660,13 @@ func (s *Server) startBillingCheckoutQuoted(ctx context.Context, acct *Account, |
| if session.ID != intent.SessionID { |
| return "", errBillingIdentity |
| } |
| - if own, err := s.ownBoundSession(session, intent, acct, now); err != nil { |
| + if own, err := s.ownBoundSession(ctx, session, intent, acct, now); err != nil { |
| return "", err |
| } else if !own { |
| return "", errBillingIdentity |
| } |
| if !b.intentSession(session, intent, acct) { |
| - return "", s.refuseSession(ctx, intent, session.ID, session, acct, now) |
| + return "", s.refuseSession(ctx, intent, session.ID, session, now) |
| } |
| switch session.Status { |
| case "open": |
| @@ -686,7 +700,7 @@ func (s *Server) startBillingCheckoutQuoted(ctx context.Context, acct *Account, |
| if !b.ownSession(refusedSession, intent, acct) { |
| return "", errBillingUnavailable |
| } |
| - return "", s.refuseSession(ctx, intent, refusedSession.ID, refusedSession, acct, now) |
| + return "", s.refuseSession(ctx, intent, refusedSession.ID, refusedSession, now) |
| } |
| if err != nil { |
| return "", err |
| @@ -696,7 +710,7 @@ func (s *Server) startBillingCheckoutQuoted(ctx context.Context, acct *Account, |
| } |
| if !b.intentSession(session, intent, acct) { |
| if b.ownSession(session, intent, acct) { |
| - return "", s.refuseSession(ctx, intent, session.ID, session, acct, now) |
| + return "", s.refuseSession(ctx, intent, session.ID, session, now) |
| } |
| return "", errBillingUnavailable |
| } |
| @@ -766,6 +780,29 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| if sub.ID != id || sub.LiveMode != b.cfg.LiveMode || len(sub.Items) != 1 || sub.ItemsHasMore || sub.CustomerID == "" { |
| return errBillingIdentity |
| } |
| + // 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 |
| + // is then refused for anything (its price, its currency, its plan or |
| + // band, its quantity), is the intent-keyed review with that session as |
| + // reference, so a change made to it at the provider between payment |
| + // and recording reaches a person rather than a rejected-events row. |
| + var pending *billingIntent |
| + if state.Subscription != id { |
| + if intent, err := s.store.billingCheckoutIntent(acct.ID); err == nil { |
| + pending = &intent |
| + } else if !errors.Is(err, sql.ErrNoRows) { |
| + return err |
| + } |
| + } |
| + refuse := func() error { |
| + if pending != nil && pending.SessionID != "" && pending.ID == sub.Metadata["checkout_intent"] { |
| + if err := s.store.fileCheckoutRefusedReview(*pending, pending.SessionID, now); err != nil { |
| + return err |
| + } |
| + } |
| + return errBillingIdentity |
| + } |
| plan, terms, band := sub.Metadata["plan"], sub.Metadata["terms_version"], sub.Metadata["band"] |
| // The subscription says which book and currency it was bought under; |
| // one that says neither is from before books, which is EUR. Its price |
| @@ -778,16 +815,16 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| if errors.Is(err, plans.ErrUnknownTermsVersion) { |
| return err |
| } |
| - return errBillingIdentity |
| + return refuse() |
| } |
| if _, err := plans.PriceFor(priceTerms, plan, band, sub.Items[0].Quantity); err != nil { |
| return err |
| } |
| if plan == plans.PlanFree || sub.Items[0].PriceID == "" || sub.Items[0].PriceID != b.cfg.Prices[billingPriceKey(priceTerms, plan, band)] { |
| - return errBillingIdentity |
| + return refuse() |
| } |
| if !billingCurrencyAgrees(priceTerms, sub.Currency) || !billingCurrencyAgrees(priceTerms, sub.Items[0].PriceCurrency) { |
| - return errBillingIdentity |
| + return refuse() |
| } |
| switch sub.Status { |
| case "active", "trialing", "past_due", "canceled", "unpaid", "incomplete_expired": |
| @@ -802,10 +839,12 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| if state.Subscription != "" && state.Status != "canceled" && state.Status != "incomplete_expired" { |
| return errBillingIdentity |
| } |
| - intent, err := s.store.billingCheckoutIntent(acct.ID) |
| - if err != nil { |
| - return err |
| + if pending == nil { |
| + // No intent stands: nothing this subscription could be the |
| + // purchase of. Not settled (the provider tries again). |
| + return sql.ErrNoRows |
| } |
| + intent := *pending |
| if intent.ID != sub.Metadata["checkout_intent"] { |
| return errBillingIdentity |
| } |
| @@ -817,14 +856,9 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| // recorded from it. Once the intent is bound (its session was |
| // the intent's own when it was bound) a person is told through |
| // the same intent-keyed review a mismatched session files, with |
| - // that session as its reference; unbound, the completion event |
| - // binds or reviews the session first. |
| - if intent.SessionID != "" { |
| - if err := s.store.fileCheckoutRefusedReview(intent, intent.SessionID, now); err != nil { |
| - return err |
| - } |
| - } |
| - return errBillingIdentity |
| + // that session as its reference (refuse); unbound, the |
| + // completion event binds or reviews the session first. |
| + return refuse() |
| } |
| if intent.SessionID == "" { |
| // The intent matches this subscription, but its CreateCheckout |
| @@ -844,7 +878,7 @@ func (s *Server) reconcileBillingSubscription(ctx context.Context, id, eventID s |
| // The session read back must be the intent's own (its id, this |
| // account, the intent's customer, this mode), and must be the |
| // session of this subscription and its customer. |
| - if own, err := s.ownBoundSession(session, intent, acct, now); err != nil { |
| + if own, err := s.ownBoundSession(ctx, session, intent, acct, now); err != nil { |
| return err |
| } else if !own || session.SubscriptionID != id || session.CustomerID != sub.CustomerID { |
| return errBillingIdentity |
| @@ -1002,7 +1036,7 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| if owner == nil { |
| return "", errBillingIdentity |
| } |
| - if own, err := s.ownBoundSession(session, *expected, owner, time.Now()); err != nil { |
| + if own, err := s.ownBoundSession(ctx, session, *expected, owner, time.Now()); err != nil { |
| return "", err |
| } else if !own { |
| return "", errBillingIdentity |
| @@ -1242,12 +1276,12 @@ func (s *Server) recoverReadyIntentSession(ctx context.Context, acct *Account, i |
| if !b.ownSession(refusedSession, intent, acct) { |
| return nil, errBillingIdentity |
| } |
| - return nil, s.refuseSession(ctx, intent, refusedSession.ID, refusedSession, acct, now) |
| + return nil, s.refuseSession(ctx, intent, refusedSession.ID, refusedSession, now) |
| } |
| if err == nil { |
| if !b.intentSession(session, intent, acct) { |
| if b.ownSession(session, intent, acct) { |
| - return nil, s.refuseSession(ctx, intent, session.ID, session, acct, now) |
| + return nil, s.refuseSession(ctx, intent, session.ID, session, now) |
| } |
| return nil, errBillingIdentity |
| } |
| @@ -1281,7 +1315,7 @@ func (s *Server) recoverReadyIntentSession(ctx context.Context, acct *Account, i |
| // describes. One that is this intent's and is not that purchase |
| // is expired and the intent retired. |
| if !b.intentSession(&session, intent, acct) { |
| - return nil, s.refuseSession(ctx, intent, session.ID, &session, acct, now) |
| + return nil, s.refuseSession(ctx, intent, session.ID, &session, now) |
| } |
| return &session, nil |
| } |