amadan
Review eighteen: the P2 left open and routed to the retirement protocol's design (revision 3); the bounded findings fixed
Both reviewers found that filing the review before the expiry lets an operator's retire delete an intent whose session is still payable. That is not fixed here by another reordering: the retirement protocol design (revision 3, after Astra rejected revisions 1 and 2) answers it, and the dispositions say the P2 is open at this commit. Fixed here: a retired session's completion beside a replacement intent is settled with its account (the binder's retirement check runs before any standing intent is read); a keep decided by another request during the gate's read is answered by the store's replay comparison; tests for keep with billing off, for the listing's tie-break within one second, and for the attribution; ownBoundSession's comment. Four mutants killed; console, plans and billingstripe pass in full (console 534 s). Reports of round eighteen and of both design reviews copied in.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8 files changed,
+605
−26
docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md+26 −2docs/superpowers/specs/CODEX-FINDINGS-intent-retirement-design-review2.md+144 −0docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review18.md+191 −0docs/superpowers/specs/DISPOSITIONS-multi-currency.md+19 −0docs/superpowers/specs/OPUS-REVIEW-multi-currency-review18.md+99 −0internal/console/billingcurrency_test.go+88 −0internal/console/billingoperator.go+18 −7internal/console/billingservice.go+20 −17
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 a9d473ad7..9f36566ee 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 |
| @@ -1,6 +1,6 @@ |
| -# The retirement of a refused checkout intent: a durable protocol (revision 2) |
| +# The retirement of a refused checkout intent: a durable protocol (revision 3) |
| -Status: design, revision 2, written 2026-10-02 on `feat/carloku-multi-currency-20261001`. Revision 1 (`8c72e88b`) was rejected by Astra's design review (`CODEX-FINDINGS-intent-retirement-design-review1.md`: four P2s, five P3s); this revision answers each finding in place. Nothing here is built. Billing is OFF. It replaces the ordering arguments of reviews thirteen to eighteen about when the bound session is expired relative to the review: both orders were shown wrong by an interleaving, and the fix is a protocol with recorded steps, not a third order. |
| +Status: design, revision 3, written 2026-10-02 on `feat/carloku-multi-currency-20261001`. Astra rejected revision 2 as well (`CODEX-FINDINGS-intent-retirement-design-review2.md`: five P2s, three P3s); the section "Revision 3" at the end answers each and narrows what is built first. Where that section and the text above it disagree, the section governs. Revision 2's status follows. Revision 1 (`8c72e88b`) was rejected by Astra's design review (`CODEX-FINDINGS-intent-retirement-design-review1.md`: four P2s, five P3s); this revision answers each finding in place. Nothing here is built. Billing is OFF. It replaces the ordering arguments of reviews thirteen to eighteen about when the bound session is expired relative to the review: both orders were shown wrong by an interleaving, and the fix is a protocol with recorded steps, not a third order. |
| ## The problem |
| @@ -63,3 +63,27 @@ At the pinned API version: a successful expiry prevents payment afterwards, incl |
| 1. Whether money may ever be accepted without the purchase recording (a one-off). This design says no: a paid wrong-currency session is refunded and `settled`, or corrected and `keep`. |
| 2. The route for a session whose identity the provider keeps contradicting after the console expired it: wait for the provider record to be corrected and re-read, or an exceptional disposition with its own evidence and audit. This design holds it with its review and offers no exception. |
| 3. The copy the buyer sees while their purchase is under review. |
| + |
| +## Revision 3: what review 2 changes |
| + |
| +**Built first (stage 1): `retire` and `keep` only.** `settled` and the adjudication of a dispute are not offered until Paul rules on decisions 1, 2 and 4. Until then an intent whose session is `complete` and cannot be kept, or whose evidence is disputed, stays held under its review: more held, never fewer. Stage 1 is the core both design reviews accept (the review persists before any remote work; an unproven expiry prevents `retire`; the pass owns the retries; a payment that wins the race is completion evidence; no resume URL under a review), with the changes below. |
| + |
| +1. **The obligations that caused the review** (review 2, finding 1). Expiring the bound session certifies nothing about a hand-made subscription that named the intent. The refusal records each subscription id that caused it (`billing_review_obligations`: account, intent, subscription), appended by reconcile's refusal each time it files. `retire` requires, besides the session's proven expiry, a provider read at resolution time showing every recorded obligation `canceled` or `incomplete_expired`; otherwise 409 naming them. An obligation discovered after the door's reads and before its transaction moves the review's generation (below), and the transaction refuses on the mismatch. |
| + |
| +2. **Generations, not observation times** (finding 3). `billing_reviews` gains `generation`, moved by every filing, obligation, fate, dispute and resolution of that review. An actor reads the generation before its provider work; its conditional write requires the generation unchanged, and otherwise the actor reads again. Incompatible terminal or identity evidence is never discarded for being older: it always sets the dispute. An expiry answer alone never proves `expired` (a successful replay under the fixed key carries no provenance): the fate is taken from a GET that the same actor initiates after the answer. |
| + |
| +3. **Disputes** (finding 4): held, with their review; no outcome is accepted while `fate_disputed_at` is set. How a person adjudicates contradictory provider evidence, and which evidence then governs, is decision 2 and is not built in stage 1. The first observation stays as recorded; nothing overwrites it. |
| + |
| +4. **Payment evidence is not write-once** (finding 6). The terminal status is; `session_payment_status` and `session_subscription` are refreshed by any later proof with the same terminal status. `keep` does not rest on stored payment evidence at all: the door reads the session and its subscription under the lock. |
| + |
| +5. **`keep` validates the subscription** (the fourth contradicted claim): before accepting `keep` the door runs reconcile's judgement of the subscription (price, quantity, terms, currency, customer) read-only; a session that is the purchase with a subscription that is not answers 409. The resolution row archives the evidence it rested on (`evidence`: the session id, status, payment status, subscription id and the generation). The ordinary expiry retirement (`retireIntent`) additionally refuses an intent that has a `keep` resolution (the first contradicted claim: after `keep` closes the review, an outstanding expiry proof could otherwise delete the intent). |
| + |
| +6. **The migration barrier comes first** (finding 5). At start, before it handles any refusal, the new build installs a trigger: a delete of an intent that has a refusal review is aborted unless the row's fate is `expired` and undisputed, or the intent is never-created, or the delete is the record's own consumption after a `keep`. An older binary's `retire` then fails loudly (its transaction aborts, the review stays open) instead of deleting a payable intent. The roll rule: older consoles are stopped before the new one starts; once the trigger exists an older binary is not rolled back to. |
| + |
| +7. **Replays and the audit** (findings 7, 8). The door's second read of the review before answering a gate refusal is in the code now (within one console); across consoles the store's exact comparison in the final transaction decides, and a console that loses the race re-reads and answers the replay. `billing_review_resolutions` is the atomic audit and carries the evidence; the general audit log is best-effort and may repeat on a replayed request, and is said to be so. For a webhook, every acknowledged refused-event state commits its evidence or dispute, its rejected-event row (with the account) and its seen-event row in one transaction; the same event delivered twice changes nothing the second time. |
| + |
| +8. **Referenced sessions** (the lost-create case) can be retired on proof, never kept: `keep` is for a bound intent. Decision 5 asks whether that should change. |
| + |
| +9. **Stripe test mode, added:** a pending payment after completion, and what cancellation does to it; collectible invoices and refunds after a subscription is canceled; whether a successful expiry replay is distinguishable from a first answer; that a GET after an expiry reflects it. |
| + |
| +**Decisions for Paul, complete list:** (1) may money ever be accepted without the purchase recording (the design says no); (2) how contradictory provider evidence is adjudicated, and whether a session whose identity stays contradicted has any exceptional disposition (the design holds it); (3) the copy the buyer sees while under review; (4) the settlement predicate: what must be true of the payment, invoices, refunds and subscription before a paid refused purchase may be retired as settled; (5) whether a referenced (never bound) session may be kept. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-intent-retirement-design-review2.md b/docs/superpowers/specs/CODEX-FINDINGS-intent-retirement-design-review2.md| new file mode 100644 |
| index 000000000..3d10bcc50 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-intent-retirement-design-review2.md |
| @@ -0,0 +1,144 @@ |
| +All references are at **`b4ff6b72`**. **D** means `docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md`. This was read-only source inspection; no modifications, tests, network commands or AWS commands. |
| + |
| +| First-review finding | Closed by revision 2? | |
| +|---|---| |
| +| **1. Unbound does not mean never created** | **Yes.** D:12–14 distinguishes the pinned reference from the transactional never-created exception and requires identity and terminal proof. | |
| +| **2. Contradictory evidence leaves the first fate actionable** | **Partly.** D:21 adds a blocking dispute and D:26 preserves retirement evidence. Resolving contradictory fates remains internally inconsistent; see Findings 3–4. | |
| +| **3. Completion is not payment; settlement omits future charges** | **Partly.** Payment status gates `keep`, and `settled` checks cancellation. Outstanding payments and the separate triggering subscription remain uncovered; Findings 1–2 and 6. | |
| +| **4. A session read back as nobody’s has no disposition** | **Partly.** D:14–15 correctly separates expiry authority from proof; D:29 explicitly holds uncorrected identity. Cached evidence and freshness remain unresolved, and decision 2 still needs Paul’s choice. | |
| +| **5. Locking misses stale work after `keep`** | **Partly.** The correct door lock is named. Wall-clock observation ordering is insufficient, and concurrent replay remains; Findings 3 and 7. | |
| +| **6. Expiry retries are not fairly bounded** | **Yes at design level.** D:23 specifies backoff, a separate budget and cursor, fixed key/target, and stopping conditions. It also acknowledges retained URLs can still win the payment race. | |
| +| **7. Migration and changed `retire` semantics** | **Partly.** Legacy evidence, historical outcomes, omitted outcomes and sequential replay are defined. The mixed-version window remains unsafe; Finding 5. | |
| +| **8. Durable webhook rejection and audit effects** | **Partly.** D:38 specifies the necessary atomic webhook writes for the unproven state. The audit contract is still missing; Finding 8. | |
| +| **9. Stripe assumptions omitted from verification** | **Partly.** The requested cases and session-object proof requirement are added. Freshness, delayed collection and cancellation still need explicit acceptance criteria, below. | |
| + |
| +| First-review contradicted claim | Closed? | |
| +|---|---| |
| +| **“The lock every writer takes”** | **Yes as a claim.** D:25 names the door’s lock, and D:43 acknowledges separate consoles. The replacement concurrency argument still has gaps. | |
| +| **“Only bound sessions are read/expired”** | **Yes.** D:12, :15 and :47 include validated referenced sessions. | |
| +| **“Complete means paid”** | **Yes in the fate definition.** D:14 separates completion from payment status. D:26’s “paid” response still misdescribes a completed, unpaid session. | |
| +| **“An expiry error proves expired”** | **Yes.** D:14 requires a session object obtained through GET after an error. | |
| +| **“Every actor can perform every step”** | **Yes.** D:25 makes local disposition operator-only. | |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — `retire` can clear the review while the separate subscription that caused it remains collectible.** |
| + |
| + **Where:** D:7, :26–27; `internal/console/billingservice.go:826–855`, :820–823. |
| + |
| + **Scenario:** A hand-made subscription names intent I but uses the wrong price. Its bound checkout S is still open. Reconciliation files the intent review, and the new protocol successfully expires S. The operator selects `retire`: S has the required expired, unpaid, subscription-free proof, so the transaction deletes I and clears its review. |
| + |
| + The hand-made subscription is **not S’s subscription**. Neither `retire` nor the proposed `settled` check requires its cancellation or financial disposition. It can remain active while its subsequent events are acknowledged through I’s retirement record. A replacement purchase can open beside it. |
| + |
| + This is an existing review-thirteen scenario, not a new provider anomaly. |
| + |
| + **Smallest acceptable change:** Persist the subscription IDs that caused the refusal, separately from the session’s subscription. Before releasing the hold, require a disposition for every known associated obligation. Define how concurrent discovery of another obligation invalidates the resolution’s evidence. Expiring S must never certify settlement of those subscriptions. |
| + |
| +2. **P2 — `settled` still permits retirement before payment obligations are settled.** |
| + |
| + **Where:** D:14, :27, :59, :63; `internal/console/billingservice.go:967–968`; `internal/billingstripe/client.go:536–539`. |
| + |
| + **Scenario:** A refused checkout reaches `complete` while a delayed payment remains pending. The operator cancels the subscription and submits `settled`. The stated gate passes: completion is proven and the subscription is canceled. The intent disappears and later events are treated as retired. The design does not require proof that the pending payment cannot subsequently succeed, that outstanding invoices cannot collect, or that a required refund actually succeeded. |
| + |
| + “The operator attests what was done” does not define what must have been done. Subscription cancellation is a separate fact from payment settlement. |
| + |
| + **Smallest acceptable change:** Define the settlement predicate for pending, successful, failed and refunded payments, including outstanding invoices and future collection. Specify the identifiers and evidence the attestation must name. Keep the review open while required settlement remains pending. Paul must approve this policy alongside decision 1. |
| + |
| + **Stripe verification:** D:59 should explicitly require the behavior of pending payments, collectible invoices and pending/failed refunds after subscription cancellation. The protocol must not assume cancellation alone settles them. |
| + |
| +3. **P2 — Observation timestamps can discard a real contradiction in favor of a cached answer.** |
| + |
| + **Where:** D:14, :21, :23, :29, :43; `internal/billingstripe/client.go:224–243`; `internal/console/billingstore.go:791–795`. |
| + |
| + **Scenario:** |
| + |
| + - An expiry succeeds with matching identity; its response is cached under the fixed key. |
| + - The provider’s session metadata later becomes contradictory. |
| + - Console A reads that contradiction and pauses before persisting it. |
| + - Console B repeats the expiry request, receives the old matching response, and records `expired` with its later observation time. |
| + - A resumes. D:43 rejects its observation because it is older than the recorded fate. |
| + - `retire` now accepts an undisputed expired fate without another provider read. |
| + |
| + The cached response describes an older provider state despite being received later. Clock skew between consoles creates another failure of the same ordering rule. Conversely, assigning timestamps at response completion can make work started before `keep` look newer than the resolution. |
| + |
| + **Smallest acceptable change:** Use database generations for review occurrence, evidence and dispute state. Capture the generation before provider work and compare it transactionally afterward. Distinguish response receipt time from evidence freshness. Do not silently discard incompatible terminal or identity evidence solely because its timestamp is older. |
| + |
| + A dispute-clearing proof must come from a new read initiated after the relevant dispute generation; a replayed expiry response cannot clear an identity dispute. |
| + |
| + **Stripe verification:** Successful expiry replays currently expose no replay provenance through `Session`; only errors preserve `Replayed`. Define which GET/listing consistency guarantees are required, and verify that the selected recovery read reflects corrected identity. The successful-expiry terminal-exclusivity test remains essential before treating cached expiry as terminal proof. |
| + |
| +4. **P2 — Dispute resolution does not define which fate becomes authoritative.** |
| + |
| + **Where:** D:21, :25–29, :47, :51; `internal/console/billingstore.go:457–459`, :766–768. |
| + |
| + **Scenario:** The immutable fate is `expired`. A later complete-session observation disputes it. The operator obtains fresh, fully matching `complete` evidence, cancels the subscription and requests `settled`. |
| + |
| + D:29 permits this resolution, but D:25–27 says its transaction must read a `complete` fate from the intent. D:21 forbids changing the stored `expired` fate. Following the ordinary predicate makes the dispute unresolvable; overriding it leaves undefined whether retirement archives the original expired fate or the accepted complete proof. Late-event classification then depends on that choice. |
| + |
| + The same ambiguity affects `keep`. Once its review closes, the supposedly untouched ordinary-expiry path checks only the open-review guard, not the resolved evidence. Consumption also deletes the intent containing the fate, leaving no specified durable destination for contradictory responses arriving afterward. |
| + |
| + **Smallest acceptable change:** Separate immutable observations from the evidence accepted by an operator resolution. Define the effective disposition evidence, gate the transaction against its exact dispute generation, and archive both the original observation and the resolving proof. Every retirement/consumption path must respect that decision. Specify how evidence survives consumption after `keep`, and where subsequent contradictions create an actionable hold. |
| + |
| + The original observation need not change; the design must explicitly distinguish it from the operator’s adjudicated result. |
| + |
| +5. **P2 — The migration barrier starts too late.** |
| + |
| + **Where:** D:55; `internal/console/billingoperator.go:357`; `internal/console/billingstore.go:982–1009`. |
| + |
| + **Scenario:** During a two-console rollout, the new console files the review and its expiry fails. No fate has yet been written. The old console accepts `retire`, deletes the intent and releases the hold while the URL remains payable. |
| + |
| + D:55 forbids the old binary only **after the first fate is written**. The unsafe operation occurs before that boundary and reproduces review eighteen’s P2 exactly. |
| + |
| + **Smallest acceptable change:** Require all older billing writers to be stopped and drained before the new protocol handles any refusal or resolution. Alternatively, install a database compatibility barrier before enabling the protocol. State the rollback procedure explicitly; an old writer must not remain authorized merely because fate is still empty. |
| + |
| +6. **P3 — A completed session’s payment status can become permanently stale.** |
| + |
| + **Where:** D:21, :23, :28, :37–38; `internal/console/billingservice.go:967–968`. |
| + |
| + **Scenario:** The first proof records `complete` with `payment_status=unpaid`. Payment later succeeds. The pass does nothing for a recorded complete fate, and later answers are recorded only if contradictory. `complete/paid` does not contradict the terminal status `complete`. |
| + |
| + If payment status belongs to the write-once evidence, `keep` remains unavailable even after correction and successful payment. If it may change, the design has not specified its writer, ordering or transactional comparison. |
| + |
| + **Smallest acceptable change:** Keep terminal fate observations immutable while defining how fresh payment/subscription evidence is appended or refreshed. State which evidence `keep` reads and how it is checked transactionally. Preserve polling or webhook recovery until the required payment state is known. |
| + |
| +7. **P3 — The preliminary replay check still fails across two consoles.** |
| + |
| + **Where:** D:25, :30, :43, :51; `internal/console/billingoperator.go:334–349`; `internal/console/billingstore.go:937–951`. |
| + |
| + **Scenario:** Two consoles receive identical `keep` requests and both read the occurrence as open. Console A passes its gate and commits. A webhook records the corrected purchase and consumes the intent. Console B then runs its gate, finds no intent and returns 409 instead of the committed 200. A delayed provider error can produce 503 instead. |
| + |
| + The newly specified account lock closes this interval within one runtime, but separate consoles have separate locks. This is Astra review eighteen’s concurrent-replay finding. |
| + |
| + **Smallest acceptable change:** Before returning a gate refusal or provider error, recheck the occurrence and route any committed resolution through the store’s exact actor/note/outcome comparison. Keep that comparison inside the final transaction as well. |
| + |
| + The **local fate read itself can be serialized correctly**: `internal/console/store.go:65` gives each store one connection, so a transaction excludes other operations through that handle. Separate connections require database conflict handling and whole-transaction retries. Helpers inside the transaction must use `tx`; querying through `s.db` would wait on the occupied connection. |
| + |
| + The door’s named lock is correct. Pass ownership/refusal work, recovery binding and expected-intent ownership handling currently run outside it (`billingpass.go:197`, :257; `billingservice.go:1100–1121`). New fate writes on those paths must therefore rely on the transactional conditions, not that mutex. |
| + |
| +8. **P3 — The atomic audit and replay-write contracts remain unspecified.** |
| + |
| + **Where:** D:25–30, :38, :55; `internal/console/billingstore.go:967–968`, :1325–1337; `internal/console/billingoperator.go:359–364`. |
| + |
| + **Scenario:** Resolution commits, then the process dies before `s.audit`. The resolution-history row exists but the general audit event does not. An identical retry returns through the success branch and calls `s.audit` again. Without a crash, two successful retries can produce additional audit writes despite D:30’s “without writing” promise. |
| + |
| + **Smallest acceptable change:** Name `billing_review_resolutions` as the atomic audit and specify the evidence it preserves. Declare the general audit’s delivery/replay semantics, or add durable delivery if it must be guaranteed. |
| + |
| + Also make D:38’s transactional rule apply explicitly to **every** acknowledged refused-event state: required evidence/dispute, rejected row and seen row commit together. Preserve previous rejection history across all three outcomes. Duplicate delivery must not increment a dispute occurrence merely because the same event was delivered twice. |
| + |
| +The main review-eighteen expiry failure is otherwise addressed: the review persists before remote work; an unproven expiry prevents `retire`; the pass owns retries; and payment winning the race produces completion evidence. Refusing every resume URL prevents further disclosure, but deliberately cannot revoke an already-held URL. Keeping the exact intent standing prevents a replacement until disposition. Those guarantees depend on fixing the evidence and migration gaps above. |
| + |
| +For the unbound lost-create case, the pinned reference and full proof now support `retire` or `settled` without pretending the session never existed. `keep` remains bound-only. If corrected referenced sessions are intended to support `keep`, define their conditional binding path explicitly: the table currently stops pass work once their fate is complete. |
| + |
| +Paul’s decisions 1 and 2 remain owner decisions, not approved policy. Before building, settle the pending-payment/refund requirements, whether referenced sessions may be kept, and whether permanently contradictory identity has an exceptional disposition. Buyer copy remains decision 3. |
| + |
| +**Claims the design makes that the code contradicts** |
| + |
| +- **“The pass’s expiry retirement … is untouched” — D:47.** `retireIntent` checks the open refusal review, but no fate or dispute (`billingstore.go:457–459`). After `keep` closes that review, an outstanding expiry proof can again reach ordinary deletion. The new protocol requires additional conditions here; preserving the current implementation is not sufficient. |
| + |
| +- **“A retired intent’s events settle and are listed” with account attribution — D:38, :47, :51.** A retired completion beside a replacement intent skips the binder’s retirement lookup because that lookup runs only when no intent exists (`billingservice.go:1129–1149`). It returns an untyped identity error, so `billinghandlers.go:323–328` records account zero. The same placement also prevents that branch from consulting the retired fate to identify a contradiction. Move the retired-session lookup ahead of comparison with the replacement intent. |
| + |
| +- **“Replays … without writing” — D:30.** That is true of the store’s resolution-history replay, not the complete HTTP path: `billingoperator.go:364` still writes the general audit after successful replays. The design must distinguish those guarantees. |
| + |
| +- **“The corrected purchase records” using “today’s gate” — D:28.** Today’s gate validates the session, not the subscription’s corrected price, quantity or terms (`billingoperator.go:238–251`). Those checks occur later in reconciliation (`billingservice.go:866–929`). A valid session with a still-wrong subscription can pass `keep` and immediately reopen the review. Either retain that explicitly as conditional behavior or require subscription validation before accepting `keep`. |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review18.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review18.md| new file mode 100644 |
| index 000000000..6d04c3f35 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review18.md |
| @@ -0,0 +1,191 @@ |
| +Platform references below are at **`68f6ee23`**; site references are at **`4026231`**. This was source-only: no changes, tests, network commands or AWS commands. Test counterfactuals are from inspection. |
| + |
| +| Seventeenth-review finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P3-1 / Astra 2:** exact `keep` replay fails after recording | **Closed for the reported sequential replay.** The resolved-occurrence check bypasses the provider gate (`internal/console/billingoperator.go:334`). Concurrent identical requests retain a smaller race, Finding 2 below. | **No.** The added replay after consumption at `billingcurrency_test.go:3967` would receive 409. | |
| +| **Opus P3-2 / Astra 3:** `keep` panics with billing disabled | **Code closed.** An unresolved `keep` receives 409 before dereferencing billing (`billingoperator.go:341`). A committed replay still reaches the store. | **Yes.** Contrary to the disposition, `TestKeepIsRefusedForAnIntentThatCannotRecord` never disables billing. Both fixtures supply a runtime (`billingcurrency_test.go:3989`, `:4017`). | |
| +| **Opus P3-3:** `keep` accepts a completed wrong-currency session | **Closed.** The gate now requires `intentSession` (`billingoperator.go:245`). | **No.** The wrong-currency fixture’s 409 assertion at `billingcurrency_test.go:4030` would receive 200. | |
| +| **Opus P3-4:** rejected listing lacks account attribution, tie ordering and sufficient coverage | **Partly closed.** Account attribution works on the typed retirement paths; ordering and limit are correct. A retired completion beside a replacement still loses its known account, Finding 3. Non-webhook refusals and retryable errors remain outside this listing. | Removing attribution fails `billingcurrency_test.go:2971`; removing the limit or reversing receipt-time ordering fails `:4186`. **Restoring the old tie ordering passes**: the bulk fixture gives every event a different second. | |
| +| **Astra 1, P2:** ordinary retirement between expiry and review filing | **The reported interval is closed at both sites.** Reviews precede expiry (`billingservice.go:321`, `:848`). **The requested protection against resolving an unfinished expiry is absent**, Finding 1. | **No** for reverting either ordering: that subtest’s retirement succeeds instead of conflicting (`billingcurrency_test.go:4114`). **Yes** with Finding 1 present: it exercises ordinary retirement, not operator resolution. | |
| +| **Astra 4:** site publishes stale generated HTML | **Closed for `build` and `check`.** Both render the selected export before checking and building (`site/package.json:6`, `:8`). | **No.** The script-order assertion fails without that change (`site/test/pricing-pricebook.test.mjs:168`). It checks command structure, not an actual stale-export deployment. | |
| +| **Astra 5:** fractional-cent floor renders malformed text | **Code closed.** `money()` rejects fractional minor units (`site/copy-review/pricing/build.mjs:42`). | **No** if the guard is removed: the source-text assertion fails (`site/test/pricing-pricebook.test.mjs:250`). The test does **not** actually render a fractional-floor fixture. | |
| + |
| +The prior claims are closed as follows: |
| + |
| +| Seventeenth-review claims | Assessment | |
| +|---|---| |
| +| **Opus 1; Astra’s unconditional-retirement comments** | The cited comments now mention `retire` and `keep`. The expiry-order descriptions remain inconsistent with the new implementation; detailed below. | |
| +| **Opus 2:** replay identity/comment/message omit outcome | Corrected in `billingstore.go:906` and `billingoperator.go:386`. | |
| +| **Opus 3:** `keep` accepts a session that cannot record | The reported wrong-currency exception is closed. `keep` still does not certify that the subscription has been corrected; subsequent reconciliation validates it. | |
| +| **Opus 4–5; Astra’s account-attribution claim** | Partly closed. The typed retirement errors carry accounts, but “otherwise … not resolvable” remains too broad. | |
| +| **Astra:** unknown historical terms were wrongly documented as an invoice review | Corrected: design line 143 distinguishes `ErrUnknownTermsVersion` and its retry from `ErrPriceTerms` and its review. | |
| +| **Astra:** existing resolution rows were not migration-tested | Closed by `TestAnOlderResolutionRowReadsAsRetire` (`billingcurrency_test.go:4201`). | |
| +| **Astra:** listing tests establish neither ordering nor bound | The new test establishes the 200-row bound and receipt-time direction, but not tie ordering, every intermediate position or account attribution on every bulk row. | |
| +| **Both: mutation evidence and test dependencies** | Historical mutation-run counts remain unverifiable from source. The current qualifications about equivalent mutations and text-only provisioning checks are substantially accurate. | |
| + |
| +**On the severity disagreement:** Astra’s P2 reading of the pre-review retirement race was right. The pass’s expiry branch does not take reconciliation’s account lock (`billingpass.go:209`). Before the review existed, the store could delete the intent. Opus’s reasoning about protection *under an open review* did not cover that interval. The shared `keep` findings were P3 in both reports. |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — Operator `retire` can release an intent while its bound session remains payable.** |
| + |
| + **Where:** `internal/console/billingservice.go:321`, `:848`; `internal/console/billingoperator.go:357`; `internal/console/billingstore.go:982`; `internal/console/billingpass.go:202`. |
| + |
| + **Scenario:** A hand-made subscription names the standing intent but uses another band’s price. Reconciliation reads the intent’s open bound session, files the review, then its expiry request fails. The webhook returns 503, but the review is already available to the operator. |
| + |
| + The operator submits the default `retire`. That path performs no provider check: it resolves the review, archives the intent and deletes it. A replacement checkout can now open while the original URL remains payable. Payment through that original URL is subsequently rejected as belonging to a retired intent; it cannot activate the purchase. |
| + |
| + A process death between review filing and expiry produces the same opportunity. Merely leaving the review open is insufficient protection against its resolution. |
| + |
| + Nor does every “next read” retry expiry: for this hand-made-subscription case, the session itself can still pass `ownSession` and `intentSession`. The background pass sees an ordinary open session and returns without expiring it (`billingpass.go:213`). A repeated subscription webhook retries the refusal, but the pass does not persistently own that unfinished work. |
| + |
| + **Smallest acceptable fix:** record an unfinished expiry obligation with the review. Keep ordinary retirement and first recording blocked, have the pass retry that obligation, and prevent `retire` from releasing the intent until the bound session is proved non-payable. A completed session must return to the operator’s settlement decision. |
| + |
| + The new hook test correctly exercises the ordinary-retirement decision after expiry. It is a stand-in for a competing pass, not a test of expiry failure, restart or operator resolution during the interval. |
| + |
| +2. **P3 — Concurrent identical `keep` requests can still disagree after one commits.** |
| + |
| + **Where:** `internal/console/billingoperator.go:334`, `:344`; `internal/console/billingoperator.go:228`. |
| + |
| + **Scenario:** Two identical operator requests both read the review while it is open. The first passes the provider gate and commits `keep`. A subscription event then records the corrected purchase and consumes the intent. The second request enters `billingKeepRefused`, finds no standing intent and returns 409, although its exact resolution is already committed. |
| + |
| + Similarly, a delayed provider failure can produce 503 after the identical decision has committed. The new preliminary read fixes sequential replay but is not atomic with the gate. |
| + |
| + **Smallest acceptable fix:** before returning a gate refusal or provider error, check whether that occurrence has since been resolved and let the store’s exact actor/note/outcome comparison decide the replay. Add a controlled concurrent-request test. |
| + |
| + The added sequential replay assertion passes with this race. |
| + |
| +3. **P3 — A retired session’s completion loses account attribution when a replacement intent stands.** |
| + |
| + **Where:** `internal/console/billingservice.go:1129`, `:1149`; `internal/console/billinghandlers.go:323`. |
| + |
| + **Scenario:** Retire intent A, open replacement B on the same account, then deliver A’s late completion. The binder successfully derives the account and verifies agreement between its metadata and client reference. Because B exists, it skips the retirement lookup—which currently runs only on `sql.ErrNoRows`. A fails ownership against B and returns bare `errBillingIdentity`. |
| + |
| + The event is correctly settled with 204 and cannot record, but its rejected row gets account ID zero. The same retired completion carries the account when no replacement stands. |
| + |
| + **Smallest acceptable fix:** perform the account-scoped retirement check for every non-recorded completion before judging it against a replacement intent, mirroring reconciliation. Return the typed account error for that retirement. Extend the existing late-completion fixture to inspect the resulting rejected row. |
| + |
| + The current attribution assertions run before the replacement opens (`billingcurrency_test.go:2971`); the later completion at `:3070` checks state preservation, not listing attribution. |
| + |
| +**Other requested paths** |
| + |
| +**A — GBP end to end.** I found no path that converts or records the ordinary book purchase in another currency. |
| + |
| +For one Personal member, `GET ?country=IN¤cy=gbp` selects band D and quotes **£7.70 annually**, with **29.75 awake hours, 0.7 GB storage and 0.7 GB transfer**. Country determines allowances; GBP determines money (`billingpage.go:258`, `:265`, `:395`). |
| + |
| +The form posts its country, currency, quote fields, signature, plan, consent and postal code. Unchanged signed fields proceed; changed effective pricing fields redirect to another quote (`billinghandlers.go:71`, `:102`). India requires a postal code (`billingservice.go:465`). |
| + |
| +Checkout computes GBP book terms, verifies the configured provider price’s currency and unit amount, and stores those terms on the intent (`billingservice.go:487`, `:526`, `:592`). `checkoutParams` pins the same terms; fixed-currency creation sends `currency=gbp` and disables Adaptive Pricing (`billingservice.go:221`; `internal/billingstripe/client.go:619`). |
| + |
| +Binding and reconciliation require ownership and matching purchase identity (`billingservice.go:1149`, `:919`, `:961`). The recording transaction preserves book/currency on the account and assignment (`billingstore.go:614`, `:740`). History reads both assignment tables (`billinginvoicestore.go:529`); `AllowanceForMonth` requires one book/currency across paid spans (`internal/plans/month.go:173`); version 2 calculations price from those terms (`billingcalculation.go:171`). |
| + |
| +Draft invoices, items, recovered objects and final lines use and verify the calculation’s currency; request hashes include it, with the deliberate legacy-EUR compatibility exception (`billinginvoice.go:859`, `:886`, `:901`, `:933`, `:1024`, `:1116`). The quantity worker checks the stored price and both subscription/item currencies, including the update response (`billingquantity.go:1014`, `:1105`). |
| + |
| +**B/C/M/O/Q/U/Z — Form, quote, cookies and held currency.** |
| + |
| +- Present currency fields must contain exactly one supported lowercase value. Empty, repeated, padded, uppercase and unsupported values receive 400 on either door, after preceding authentication/form gates (`billinghandlers.go:176`). |
| +- Missing currency means the country’s default on a book console and EUR on a legacy console. The effective-currency comparison prevents an omitted control from buying an unseen currency. |
| +- Forged agreeing `quoted_*` fields are insufficient. The signature key derives from the billing secret, not the disclosed CSRF/session token. It covers session token, account, book, country, band, currency, choice flag and member count (`billingpage.go:446`). |
| +- Another session, account, book, token or signed tuple cannot reuse the signature. An unchanged quote from an older tab remains usable; another tab’s cookies do not alter that form. |
| +- Plan is unsigned because every offered plan was priced on the page. Charged quantity follows membership and plan limits. Membership is compared under the billing lock and rechecked when the intent is created under the membership write lock (`billingservice.go:483`; `billingstore.go:353`). A change conflicts. Postal code affects tax, not the quoted base price. |
| +- The quote door checks session, CSRF and membership, then redirects to the fixed account billing path using encoded parameters. It creates no purchase and accepts no redirect destination (`billinghandlers.go:137`). |
| +- Present-empty URL parameters clear cookies; absent parameters permit fallback; invalid values set nothing. Consequently, an invalid visit can leave an older cookie available on a later parameter-absent visit. Explicit clearing does not (`billingcurrency.go:43`). |
| +- A prior purchaser is offered only the held currency, including legacy EUR. URL and cookie cannot override it; checkout independently refuses another currency (`billingpage.go:301`; `billingservice.go:579`). |
| + |
| +**D/L/R/W/Y/AA/DD–NN — Ownership and refusal routing.** |
| + |
| +`ownSession` checks mode, intent metadata, both account identifiers, known customer and bound session ID (`billingservice.go:272`). Create, replay, listing, resume and intent-driven binding paths apply it before accepting ownership proof. The expected-intent binder checks the expected account before deriving another account from the response (`:1100`). |
| + |
| +Two expiry paths deliberately use stronger existing evidence: the console’s previously validated **stored bound ID**. `ownBoundSession` and reconciliation’s refusal may expire that ID despite contradictory ownership metadata. They reject another-ID read responses, and their expiry responses authorize no retirement. This is the right distinction: an unowned ID learned only from creation or listing has no such authority and remains untouched at the provider. |
| + |
| +The webhook binder, which lacks an expected intent, cannot attribute contradictory account identifiers to a standing intent and files no ownership review. The bound-session readers can. The already-recorded fast path returns the recorded subscription ID without recording another purchase (`:1126`). |
| + |
| +Binder and reconciliation share `intentSession` for the completed session’s purchase identity. Their attributable mismatches converge on one intent-keyed open review. Review filing is transactional, refuses a gone intent and fills an empty reference without another occurrence (`billingstore.go:1026`). |
| + |
| +With the new ordering, expiry failure or completion during expiry leaves the review standing. Read/expiry failures return 503 through the webhook; successful identity refusals are recorded and acknowledged with 204. Concurrent filings add no occurrence while the review is open. Finding 1 concerns releasing that hold before expiry is finished. |
| + |
| +Correct first purchases, recorded renewals and late events for the currently recorded canceled subscription do not spuriously enter the first-purchase mismatch review. A wrong-intent subscription receives a bare identity refusal when it reaches that comparison. Unknown terms without an attributable bound intent remain retryable; incomplete status, invalid period dates and missing bindings can also return 503 before that comparison (`billingservice.go:874`, `:896`, `:909`, `:931`). |
| + |
| +**S/V/OO/RR–WW/XX/YY — Lifecycle invariant.** |
| + |
| +The database enforces: |
| + |
| +> An open refusal review prevents ordinary retirement and first-purchase recording from consuming its standing intent. `retire` archives and removes exactly that intent; `keep` resolves the hold while preserving purchase validation. Account-scoped retirement prevents later activation. |
| + |
| +Its provider-side extension—no payable session survives retirement—is still broken by Finding 1. |
| + |
| +| Path | Result | |
| +|---|---| |
| +| Checkout, previous/current resume, recovery listing and replay | Validate against the selected standing intent; conditional writes cannot resurrect a deleted one. | |
| +| Pass, success-return polling and account-deletion recovery | Use the same binding/reconciliation or guarded retirement paths. | |
| +| Stillborn cleanup | Deletes only an unbound intent with `customer_ready=0`, before session creation can proceed (`billingstore.go:498`). | |
| +| Binder, direct binding and webhook | Cannot consume an intent. Binding is conditional on the exact standing row (`billingservice.go:1168`). | |
| +| First-purchase recording | Requires consumption of the exact intent with no open refusal review in the recording transaction (`billingstore.go:766`). | |
| +| Recorded subscription updates and renewals | Do not require consuming the original intent again. | |
| +| Ordinary expiry retirement | Requires expired, unpaid, subscription-free ownership proof and no open review (`billingstore.go:444`). | |
| +| Operator `retire` | Archives/deletes only the named account/intent and closes only its pending checkout quantity events; Finding 1 remains. | |
| +| Operator `keep` | Reads the stored bound session and requires ownership, purchase identity, completion and a subscription. Refusal leaves the review open; provider failure is 503. Finding 2 affects concurrent replay. | |
| +| Late review filing | Files nothing after the exact intent has gone. | |
| +| Retired events | Cannot activate; subscription retirement is checked before purchase validation. Finding 3 concerns attribution, not recording. | |
| + |
| +A failed first-record consumption rolls back account state, assignment, event and entitlement writes; quantity refresh is not committed. The non-consuming wrappers have no production recording caller that bypasses reconciliation’s first-purchase check. |
| + |
| +Retirement lookup is account-scoped. Another account’s retirement cannot settle this account’s purchase. Intent IDs are randomly generated; there is no supported reuse operation. Already-recorded subscriptions bypass the first-purchase retirement test. |
| + |
| +Outcome migration defaults earlier rows to `retire`; omitted outcomes from old clients also mean `retire`. Actor, note and outcome define an exact replay. The resolution record and operator audit include the outcome (`billingstore.go:227`, `:948`, `:967`; `billingoperator.go:360`). |
| + |
| +**E/H/N/T — Barrier and rollout.** |
| + |
| +The trigger’s positive paid-span predicate matches `AllowanceForMonth`: active, non-Free, bounded by effective time, next assignment across both tables, paid-through time and month end. Revision order resolves same-second successors (`billingstore.go:208`; `internal/plans/month.go:146`). |
| + |
| +An active book assignment superseded in the same second contributes no span. A Free or inactive successor terminates the preceding span. Renewal and proven quantity changes affect the new assignment’s `effective_at`; `period_end` remains the subscription’s paid-through endpoint. The book-assignment insertion copies it from account state (`billingstore.go:627`, `:665`, `:740`). |
| + |
| +I found no valid-history example where the refined trigger permits an older version 1 over a book-priced span, or refuses this build’s legitimate legacy-only version 1. |
| + |
| +Migration creates the new trigger before transactionally checking its existence and dropping the old name. Missing replacement causes failure without dropping the old trigger; a second start is idempotent. Interruption can leave both temporarily, not neither (`billingstore.go:838`). An unmigrated database is migrated before serving. An older binary remains unsupported after book rows exist: this barrier protects calculation freezing, not all currency-unaware account writes. |
| + |
| +**F/G/J/K/MM/QQ — Figures, books, export and site.** |
| + |
| +The billing page divides `PriceFor`’s pooled allowance by its charged quantity. Unavailable Personal uses the fallback quote’s own quantity (`billingpage.go:408`, `:427`). Current figures are exact at every permitted quantity because each per-member band-scaled amount is integral in base units and representable to three decimals. `formatFigure(1,3)` truncates, but no current offered allowance requires that approximation. Band-A sentences are pinned verbatim (`billingcurrency_test.go:2628`). |
| + |
| +Book validation checks duplicate keys, supported currencies, known country codes, complete prices/rates/floors, checked rounding arithmetic and legacy EUR equality (`internal/plans/pricebook.go:149`, `:231`). Configuration requires the selected complete catalog and permits complete additional catalogs, rejecting shared provider IDs (`cmd/console/billing.go:134`). The provisioning verifier requires at least the selected book’s 36 keys and agrees with the console’s own-only currency-options rule (`stripe-provision.sh:426`, `:436`). |
| + |
| +The export contains commercial definitions, resources and country mappings, with no account data or credentials. **The committed site export is byte-for-byte equal to the platform export.** |
| + |
| +At the site commit, current prices, exact micro-unit rates, floors and band-scaled inclusions agree with that export. Country options supply both band and default currency. Both link parameters are always present; currency is populated only after explicit choice, country from the selection (`site/src/site.js:75`). |
| + |
| +A locale-only GBP display may therefore lead to a USD console default when no country or currency choice was sent. That is the owner’s stated suggestion policy; the console presents its own purchase quote. |
| + |
| +Wrong literals in the checked slots fail value checks. A literal equal to its slot’s current value passes, as the revised test acknowledges. The shared token checker rejects embedded numeric matches such as `10` in `10,000` and `250 MB` in `1,250 MB` (`site/hack/token.mjs:4`). |
| + |
| +**X/CC/BBB — Quantity history and database readers.** |
| + |
| +All three outcome-admission checks use `quantityOutcomesSQL`; startup replaces a differing guard body. No healing sweep remains. Summary updates preserve an existing gap and add clock-rollback gaps; they do not clear them (`billingquantityaudit.go:20`, `:61`; `billingmembership.go:462`). |
| + |
| +The rejected listing drains and closes rows before account lookups (`billingoperator.go:282`). I found no equivalent nested store query under open rows in the reviewed billing handlers or passes. Other billing listing helpers materialize slices before their callers perform further queries. |
| + |
| +The rejected endpoint requires an operator CLI identity, returns at most 200 rows ordered by receipt time and then insertion order, and exposes stable identifiers without provider payloads. Retryable failures are not rejected rows; non-webhook refusals are not automatically included. |
| + |
| +**Claims the code contradicts** |
| + |
| +- **“Expiry … is tried again at the next read.”** Dispositions line 336 and `billingservice.go:319` overstate this. An owned, otherwise valid open session in the hand-made-subscription case takes the pass’s no-op branch. There is no persisted expiry obligation. |
| +- **Expiry before review/resolution safety.** Design line 152 and `billingservice.go:831` still describe expiry before filing. The implementation now files first. The assurance that nothing can be paid after resolution is false under Finding 1. `ownBoundSession`’s comment also describes filing twice (`:295`, `:300`), although the code files once. |
| +- **Disabled-billing regression coverage.** Dispositions line 338 cites a test that never sets billing to nil. The fix is present; its claimed regression case is not. |
| +- **Other rejected identities have no resolvable account.** Dispositions line 325 and `billinghandlers.go:318` are too broad. Finding 3 has an established account and a matching retirement record. |
| +- **Complete listing coverage.** The bulk test checks 201 distinct timestamps, a 200-row result, first/last IDs and the first account (`billingcurrency_test.go:4176`). It does not pin same-second ordering or every returned account. The separate lifecycle test checks attribution only for its three named post-resolution events. |
| +- **Fractional-floor test “asserts the refusal.”** Dispositions line 342 overstates the site test. It checks the guard’s source text and current floors’ divisibility; it never supplies a fractional floor to the renderer. |
| +- **Review-three and review-four mutation claims:** the equivalent default-currency mutation is correctly qualified. The account-only delete mutation is redundant behind the exact-intent lookup in the same transaction. Successful trigger migration does not establish interruption behavior. Historical mutation counts cannot be confirmed from these files. |
| +- **The older-resolution test does establish its narrow claim.** It reopens a database containing a resolution row without `outcome`, checks the default `retire`, accepts its exact replay and rejects `keep`. Removing the migration fails it. It reconstructs that table shape, rather than opening an entire database produced by an older binary. |
| +- **The late-completion fixture now establishes non-recording.** It supplies the retired session’s ID and metadata while a distinct replacement stands, then checks account state and replacement binding (`billingcurrency_test.go:3065`). Recording the retired completion would fail those assertions. Its helper still discards the HTTP status and does not inspect attribution. |
| +- **Clock/test isolation:** the fake starts from wall-clock time (`billingservice_test.go:138`), and authentication/webhook tests use current time. I found no additional concrete failure caused by clock boundaries, test ordering, temporary-root selection or a shared cookie jar. No test was run. |
| + |
| +**What must be verified against Stripe test mode before enablement** |
| + |
| +1. **Catalog and checkout:** verify all 36 price identities, amounts, expanded own-currency options, tax settings and quantities. Complete purchases in USD/EUR/GBP, including IN/GBP and Community GBP bands B/D. Session, subscription and item currencies must agree; Adaptive Pricing must remain disabled. |
| +2. **Ownership and expiry:** exercise editable metadata contradictions and both webhook orders. Confirm expiry acts only on the stored bound ID, expired URLs cannot accept payment, and ordinary retirement responses retain every required proof field. |
| +3. **Failure lifecycle:** inject expiry failures, completion during expiry and restart after review filing. Attempt operator resolution during each interval. No payable session may outlive retirement or coexist with an unintended replacement. |
| +4. **Resolution and replay:** correct a subscription under review; recording must remain blocked until valid `keep`. Exercise expired/wrong-currency `keep`, disabled billing, sequential and concurrent identical replays, opposite outcomes, and late retired completions beside replacements. Verify audit and rejected-list attribution. |
| +5. **Invoices and recovery:** create, recover, finalize and pay overage invoices in every currency. Verify item/line currencies, exact allocation, floors, taxes, discount exclusion, house waivers and legacy EUR request compatibility. |
| +6. **Subscription lifecycle:** verify quantity fencing and proration, renewal dating, cancellation, delayed events, and customer-currency restrictions after an ended subscription with outstanding invoice items. |
| + |
| +**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 c37b50a9d..33200ce65 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md |
| @@ -345,3 +345,22 @@ Reports: `OPUS-REVIEW-multi-currency-review17.md`, `CODEX-FINDINGS-multi-currenc |
| | Astra claims on evidence: historical mutation counts cannot be confirmed from source; provisioning assertions read the script's text; the fake starts from wall-clock time. | Stated as they are: the mutation runs are the author's, named per round here; the provisioning check is exercised against the live catalog only in the test-mode rehearsal (approval E); no concrete ordering or clock failure was found by either reviewer or the author. | | | |
| Open, as both list: everything that needs Stripe test mode (Astra's six-part list, this round's additions: expiry failing after the review is filed, a restart between the two, `keep` against a corrected purchase at the provider), the two unreviewed system strings, the operator's door for a quantity-history gap, and the metered model's remaining code (READINESS on the launch branch). |
| + |
| +## Review eighteen (reviewed `68f6ee23`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-multi-currency-review18.md`, `CODEX-FINDINGS-multi-currency-review18.md`. Both found review seventeen's findings closed and the same new P2, opened by review seventeen's own fix. |
| + |
| +**The P2, not fixed by another reordering.** With the review filed before the expiry, an expiry that fails leaves the bound session open and payable under an open review; nothing retries it (the pass takes its no-op branch for an owned, purchase-matching open session), the resume path still offers the URL, and the operator's `retire` deletes the intent without reading the session: money can be taken after the operator recorded it settled (Opus P2-1, Astra 1). Reviews thirteen to eighteen have now shown each order of the two calls wrong. By the supervisor's ruling of 2026-10-02 this is answered by a protocol with recorded steps (the refusal authoritative; the session's fate proven from provider evidence and recorded; the operator's completion atomic; a paid session disposed of separately, never called expired), designed and reviewed before it is built: `2026-10-02-intent-retirement-protocol-design.md`. Astra rejected revision 1 (`CODEX-FINDINGS-intent-retirement-design-review1.md`, four P2s) and revision 2 (`CODEX-FINDINGS-intent-retirement-design-review2.md`, five P2s: the hand-made subscription's own obligations at `retire`; the settlement predicate for `settled`; observation times against cached answers; which fate a dispute's resolution makes authoritative; the migration barrier). The core both reviews accept (the review persists before remote work; an unproven expiry prevents `retire`; the pass owns the retries; a payment that wins the race is completion evidence; no resume URL under a review) is not built in this commit. **The P2 is open at this commit**, and the branch does not merge with it open. |
| + |
| +**Bounded findings, fixed in this commit:** |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra 3 (P3): a retired session's completion with a replacement intent standing was judged against the replacement and recorded with no account. | Fixed: the binder's account-scoped retirement check runs for every completion that does not record, before any standing intent is read, and returns the typed identity error. | `billingservice.go` `bindBillingCheckoutFor` | The late-completion fixture reads the rejected row's account; with the check back under "no intent stands" it fails. | |
| +| Astra 2 (P3): two identical `keep` requests could disagree (the second's gate found no standing intent after the first committed and the purchase recorded). | Fixed within one console: a gate refusal or provider failure is answered only after a second read of the review; an occurrence resolved meanwhile goes to the store's exact replay comparison. Two consoles still rest on the store's comparison alone at the transaction (stated; the protocol's design carries it). | `billingoperator.go` | `TestKeepWithBillingOffAndAKeepDecidedDuringTheGate` (the same resolution committed during the gate's read: 200; another note: the store's 409); without the second read it answers the gate's 409. | |
| +| Opus P3-1, Astra's claim: the nil-billing branch had no test, and the dispositions named a test that had no such case. | Added: `keep` with billing off answers 409 and the review stays open. The review-seventeen row's test name is corrected by this row. | `TestKeepWithBillingOffAndAKeepDecidedDuringTheGate` | With the branch removed the test fails (a nil dereference). | |
| +| Opus P3-2, Astra's claim: the lifecycle test's newest-first check could not fail (one second), and the tie-break was untested. | Added: two rows in one second whose ids sort against their arrival; the later arrival first. | `TestTheRejectedListingBreaksTiesNewestFirst` | With `ORDER BY received_at DESC, id` it fails. | |
| +| Opus claim 4, Astra's claim: the site test asserted the guard's source text, not the refusal. | Fixed on the site: the renderer is a module (`hack/money.mjs`), and the test calls it with a fractional amount and wants the throw. | site `01b62f7` | | |
| +| Claims (both): `ownBoundSession`'s comment described filing twice and ended mid-sentence; the review-seventeen row said a failed expiry "is tried again at the next read" (true for `ownBoundSession`, false for reconcile's refusal, whose session the pass sees as an ordinary open one). | The comment is corrected and says what is open; the row's claim is withdrawn here: nothing retries reconcile's failed expiry until the protocol is built. `refuse()`'s and `ownSession`'s comments on the order are left for the protocol's commit, which rewrites both sites. | `billingservice.go`, this file | | |
| + |
| +Open: the P2 above; everything needing Stripe test mode (both lists; Astra adds expiry failures, completion during expiry and a restart after the review is filed, with operator resolution attempted in each interval); the two unreviewed system strings; the operator's door for a quantity-history gap; the metered model's remaining code. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review18.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review18.md| new file mode 100644 |
| index 000000000..1b5ef9086 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review18.md |
| @@ -0,0 +1,99 @@ |
| +# Eighteenth review of `feat/carloku-multi-currency-20261001` at `68f6ee23` (base `12e70d95`, delta from `db130cfc`) |
| + |
| +**Verdict: accept with fixes.** One P2 remains. Putting the review ahead of the expiry closes Astra's P2, but it opens the reverse window: a `retire` resolution can now delete the intent while its bound session is still open and payable. Opus's seventeenth-round findings are all closed. The P3-2 fix (nil billing) has no test, although the dispositions name one. Money computation, Stripe parameters, the quote signature, the trigger and the pages are unchanged by this delta. |
| + |
| +I read source with `git show` only, plus the site at `4026231`. I ran nothing and modified nothing. |
| + |
| +## The seventeenth review's findings |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Astra 1 (P2)**: a pass could retire the intent between the refusal's expiry and the review being filed | **Closed.** Both sites now file the review first and expire second: `ownBoundSession` (`billingservice.go:321` then `:325`) and reconcile's `refuse()` (`:848` then `:852`). `retireIntent`'s open-review clause (`billingstore.go:453-457`) then refuses the pass. **The fix opens P2-1 below.** | **No.** With either site's old order, the `expireHook` retirement succeeds and returns nil, not `errBillingConflict`, and `check` fails. | |
| +| **Opus P3-1 / Astra 2**: an exact `keep` replay answered 409 (or 503) | **Closed.** The door reads the review first (`billingoperator.go:346-352`). For an occurrence already resolved it skips the gate, and the store's replay comparison decides. This cannot grant anything new, because a resolved occurrence cannot be resolved again. | **No.** Without the skip, the replay meets "no such intent stands" and answers 409. | |
| +| **Opus P3-2 / Astra 3**: `keep` with billing off dereferenced nil | **Closed in code** (`:353-355`, 409). | **No test exists.** `TestKeepIsRefusedForAnIntentThatCannotRecord` has no nil-billing case, so removing the check fails nothing. The dispositions' reproduction column is wrong (claim 1). | |
| +| **Opus P3-3**: `keep` accepted for a paid wrong-currency session | **Closed.** `intentSession` is now checked after `ownSession` (`:245-247`). | **No.** The fake renders a complete session with `sub_paid`, owned. Without the check the gate passes and the answer is 200 (or 404), not 409. | |
| +| **Opus P3-4 / Astra claim**: the rejected listing had no account, ordered ties oldest first, and its order and bound were untested | **Closed.** `billingIdentityOfAccount` is returned from both settle sites (`:822`, `:1137`) and unwrapped through `errors.Is`/`errors.As`. Every comparison in the package uses `errors.Is` (`billingDiagnostic` included), so the typed error breaks no caller. Ties are now ordered `rowid DESC`; `billing_rejected_events` is a rowid table. | **Account: no.** The `evt_after_resolution_*` rows cover both the reconcile and the binder sites. **Order:** the bound test (distinct seconds) fails if the order is ascending. The rowid tie-break is untested (claim 3). | |
| +| **Astra 4 (site)**: a stale page could be published | **Closed** at `4026231`. Both scripts run render, then check, then eleventy. | The order test parses `package.json`, so it fails if the order is reverted. | |
| +| **Astra 5 (site)**: a fractional floor rendered as `$1.20.5` | **Closed in code** (`build.mjs:42`). | **Would pass without the fix's behaviour.** The test asserts that the guard's source text exists; it never calls `money()` (claim 4). | |
| +| **Astra claim: an older resolutions table with rows** | **Added.** `TestAnOlderResolutionRowReadsAsRetire` drops the column, inserts a row, reopens the store, reads `retire`, accepts the exact replay and refuses `keep`. No trigger references `outcome`, so the drop is valid. | **No.** Without `addColumnIfMissing` the `SELECT outcome` errors; with a different default the value is wrong. | |
| +| **Comment and design claims** | Mostly corrected. The ownBoundSession and `refuse()` comments are now wrong in new ways (claim 2). | n/a | |
| + |
| +**Where the reviewers disagreed (round seventeen): Astra was right, and P2 was fair.** At `db130cfc`, reconcile held the account lock, but `billingPass` → `reconcilePendingIntent` takes none (`billingpass.go:122-141`, `:182-212`). So in the `refuse()` case: |
| +1. Reconcile expired the bound session. |
| +2. A concurrent pass read that session (expired, owned) and called `expireResolvedIntent`. The delete succeeded because no review existed yet. |
| +3. The later `fileCheckoutRefusedReview` found no intent and filed nothing. |
| + |
| +Subtest 1 of the new test is that real interleaving. Subtest 2 (`ownBoundSession` with `cus_other`) is only a stand-in. A concurrent reader of the same contradicted session would also see `cus_other` and retire nothing, and a crash there re-files at the next read. That race needs the provider to answer two reads differently. The reorder at that site is defensive, not a fix for a reachable race. |
| + |
| +## Findings |
| + |
| +### P2-1. A `retire` resolution can delete the intent while its bound session is still open and payable |
| + |
| +- **Where:** `billingservice.go:321-328` and `:848-855` (review, then an expiry that can fail). `billingoperator.go:357` → `billingstore.go:982-1001`: the `retire` branch deletes the intent without reading or expiring its session. |
| +- **Scenario:** |
| + 1. A hand-made subscription names a GBP intent whose buyer's session is still open. |
| + 2. Reconcile's `refuse()` files the review, then `ExpireCheckout` fails (a Stripe 5xx, or a timeout). The webhook answers 503. |
| + 3. The pass does not retry the expiry on this path. The session reads back owned and `intentSession`-true, so `reconcilePendingIntent`'s open branch does nothing (`billingpass.go:214-224`). Only Stripe's redelivery retries it, on an hours-long backoff. |
| + 4. The operator sees the review and resolves `retire` ("refunded"). The intent is deleted and a retirement is recorded. |
| + 5. The buyer, who still has the URL, pays. |
| + 6. The completion and the subscription's events settle as retired: they appear as `billing_rejected_events` rows only, with no review. |
| +- **Result:** money is taken after the operator has recorded that the money is settled, and no hold or review exists. |
| +- **The ownBoundSession path:** the same thing happens if the operator resolves between a failed expiry and the next pass. |
| +- **Why this is new:** before the reorder, every refusal review was filed only once the session was no longer payable (complete, expired, or after a successful expiry). The code's own comment at `:839-841` names this hazard: "a resolution would retire the intent with its own session still open". |
| +- **Smallest fix:** in the operator door, for `retire` of a checkout refusal on a bound intent with billing configured: |
| + - Read the bound session by its bound id. |
| + - If it reads open, expire it by that id (the same authority `ownBoundSession` uses). |
| + - If the read or the expiry fails, answer 503 and leave the review open. |
| + - With billing off, answer 409 unless the session is already known to be expired, or state the exception plainly. |
| + - Test: an expiry that fails after the review is filed, then `retire`, must be 503 with the intent standing; then a successful expiry followed by `retire` must be 200. |
| + |
| +### P3-1. The P3-2 fix (nil billing) is untested |
| + |
| +- **Where:** `billingoperator.go:353`. |
| +- **Scenario:** delete the `s.billing == nil` branch and every test still passes. |
| +- **Fix:** a case with `fx.srv.billing = nil`, posting `keep`, expecting 409 and the review still open. |
| + |
| +### P3-2. The webhook test's newest-first check cannot fail, and the rowid tie-break is untested |
| + |
| +- **Where:** `billingcurrency_test.go:2959`. |
| +- **Scenario:** the settled events are all received within one second (`received_at` is whole seconds), so `first < last` is false whatever the order. Reverting `rowid DESC` to `id` fails nothing; the bound test uses distinct seconds. |
| +- **Fix:** insert two rows in the same second whose ids sort opposite to their insertion order, and assert the later-inserted row comes first. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **Dispositions, review seventeen, the P3-2 row:** the reproduction column names `TestKeepIsRefusedForAnIntentThatCannotRecord`. That test has no billing-off case. |
| +2. **The comments at the two expiry sites:** |
| + - `billingservice.go:296-306` (`ownBoundSession`) now says the review is filed first, then the expiry, and then again "Then the intent-keyed review is filed…". It also ends with a stray "it.". |
| + - `refuse()` at `:827-833` still says the open session "is expired by its bound id first". |
| + - `:839-841` justifies not filing on an unread session by the very hazard P2-1 reopens. |
| + - The ownSession doc comment (`:269-271`) says the one session expired without passing `ownSession` is the one `ownBoundSession` handles. `refuse()` also expires the bound session without `ownSession`. |
| +3. **Dispositions, Astra 1 row: "an expiry that fails … is tried again at the next read."** That is true for `ownBoundSession`. For `refuse()`, the pass's next read sees an owned, purchase-matching open session and does nothing; only Stripe's redelivery retries. The same row and the P3-4 row say the tests check "newest first". In the webhook test that check is vacuous (P3-2). |
| +4. **Dispositions, Astra 5: "the test asserts the refusal".** It asserts that the guard's source text is present (site `test/pricing-pricebook.test.mjs`), not that `money()` throws. |
| + |
| +## Requested sections |
| + |
| +- **AAA (review, then expiry).** |
| + - Expiry fails: the review stands and the session stays payable. The `ownBoundSession` path retries at the next read; the `refuse()` path retries only on Stripe's redelivery. An operator `retire` in that window leaves the session open (P2-1). |
| + - The session completes during the expiry: the expiry errors and the review is already open. A legitimate completion through that session is then held by the record's open-review clause until `keep`. Its gate passes for an owned, correct, complete session, so this is visible and recoverable. |
| + - The process dies between filing and expiry: the review stands. The crash is safe unless the operator resolves first (P2-1). |
| + - The test's hook: for `refuse()` it is the interleaving that matters, because the pass takes no lock. For `ownBoundSession` it is a stand-in, as explained above. |
| +- **BBB.** No other `rows.Next()` loop in `internal/console` queries the store inside the loop. The rejected listing now reads to the end, then looks up accounts. The store runs with `SetMaxOpenConns(1)` (`store.go:65`). |
| +- **CCC.** The older-table test establishes what it claims. The bounded-listing test establishes the bound, descending order and the account on each row, but not ties (P3-2). |
| +- **XX–ZZ.** `keep` now also requires `intentSession`; 409 leaves the review open; a replay of a resolved occurrence goes to the store. The outcome migration is tested on existing rows. |
| +- **A–WW.** Unchanged by this delta: |
| + - The GBP purchase from `?country=IN¤cy=gbp` keeps its stored currency through checkout, record, allowance, calculation, invoice and quantity, with no conversion. |
| + - The signature, the trigger's span rule and the barrier migration, ownership on every path, and the record's open-review clause stand as rounds fifteen to seventeen traced them. |
| + - The rejected listing's account is correct even for an edited session or subscription. The retired check needs that account to have retired that 24-byte random intent id, so a row cannot name the wrong account. |
| +- **K, MM, QQ (site `4026231`).** The render, check, eleventy order now validates the artifact that gets published. `money()` refuses fractional minor units, and the committed floors are whole cents. The earlier slot and token checks stand. |
| + |
| +## Verify in Stripe test mode before enabling |
| + |
| +All earlier lists stand: catalog identities for the 36 prices; USD/EUR/GBP purchases including IN/GBP; event orderings; bound-session expiry; the review lifecycle; overage invoices; the quantity and subscription lifecycle. Add these: |
| + |
| +1. **A hand-made subscription naming an open intent, with the expiry made to fail.** Expect the review filed, a 503, and a redelivery that expires the session. After the fix, `retire` before the expiry must answer 503 and the session must stay unpayable once the door expires it. |
| +2. **The buyer completes the session while the console is expiring it.** The expiry should error, the review stand, and `keep` succeed; the next delivery records the purchase. |
| +3. **The process is killed between filing the review and the expiry**, with a pass running. The intent must stand under its review, and the redelivery must expire the session. |
| +4. **`keep` with billing off** answers 409. |
| +5. **Two rejected events in the same second** list newest first. |
| + |
| +**Verdict: accept with fixes:** P2-1, P3-1, P3-2, and claims 1–4. |
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go| index ceabc831c..f5a0845fa 100644 |
| --- a/internal/console/billingcurrency_test.go |
| +++ b/internal/console/billingcurrency_test.go |
| @@ -3068,6 +3068,13 @@ func TestRefusedPaidSessionReviewThroughTheWebhookAndItsResolution(t *testing.T) |
| }} |
| f.mu.Unlock() |
| send("evt_again") |
| + // Settled with the account named, though a replacement intent stands |
| + // (review eighteen, Astra 3: it was judged against the replacement and |
| + // recorded with no account). |
| + var rejectedAccount int64 |
| + if err := fx.st.db.QueryRow(`SELECT account_id FROM billing_rejected_events WHERE id='evt_again'`).Scan(&rejectedAccount); err != nil || rejectedAccount != acct.ID { |
| + t.Fatalf("the retired session's late completion beside a replacement: rejected row account %d (%v); want the account %d", rejectedAccount, err, acct.ID) |
| + } |
| if open, n := review(); open != 0 || n != 1 { |
| t.Fatalf("after resolution, a pass and the retired session's late completion: %d open, %d occurrences; want the review to stay resolved", open, n) |
| } |
| @@ -4237,3 +4244,84 @@ func TestAnOlderResolutionRowReadsAsRetire(t *testing.T) { |
| t.Fatalf("keep against the old retire: %v; want it refused as a different resolution", err) |
| } |
| } |
| + |
| +// Rows received in the same second come newest first too: the later |
| +// insertion first, whatever the ids' own order. (Review eighteen, Opus |
| +// P3-2: the lifecycle test's order check could not fail, and the tie-break |
| +// was untested.) |
| +func TestTheRejectedListingBreaksTiesNewestFirst(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + // The ids sort the other way from the order they arrive in. |
| + for _, id := range []string{"evt_tie_aaa", "evt_tie_zzz"} { |
| + if err := fx.st.rejectBillingEvent(id, "customer.subscription.updated", "sub_tie", "sub_tie", acct.ID, f.now); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-tie") |
| + status, body := getJSON(t, fx.ts.URL+"/api/billing/rejected", token) |
| + rejected, _ := body["rejected"].([]any) |
| + if status != http.StatusOK || len(rejected) != 2 { |
| + t.Fatalf("%d rows, status %d", len(rejected), status) |
| + } |
| + first, _ := rejected[0].(map[string]any) |
| + second, _ := rejected[1].(map[string]any) |
| + if first["id"] != "evt_tie_zzz" || second["id"] != "evt_tie_aaa" { |
| + t.Fatalf("order %v, %v; want the later arrival first", first["id"], second["id"]) |
| + } |
| +} |
| + |
| +// With billing switched off on a console that has an open refusal, keep is |
| +// refused (the intent's session cannot be read) and the review stays open; |
| +// and a keep that another request decided while the gate was reading is |
| +// answered by the store's replay comparison, not by the gate. (Review |
| +// eighteen: Opus P3-1, the nil-billing branch had no test; Astra 2, two |
| +// identical requests could disagree.) |
| +func TestKeepWithBillingOffAndAKeepDecidedDuringTheGate(t *testing.T) { |
| + fx, f, acct := newCurrencyFixture(t) |
| + f.customerCountry = "GB" |
| + if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil || dest == "" { |
| + t.Fatalf("first click: dest %q err %v", dest, err) |
| + } |
| + intent, _ := fx.st.billingCheckoutIntent(acct.ID) |
| + f.mu.Lock() |
| + f.subscriptionPrice = currencyPriceID(plans.PlanPersonal, plans.BandD, plans.GBP) |
| + f.mu.Unlock() |
| + if code := f.webhookPost(t, fx, "evt_keep_off", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent { |
| + t.Fatalf("the hand-made subscription event: %d", code) |
| + } |
| + token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-keep-off") |
| + keep := map[string]any{"account": acct.Sqid, "subscription": intent.ID, "reason": billingCheckoutRefusedReason, "occurrences": 1, "note": "corrected at the provider", "outcome": "keep"} |
| + open := func() int { |
| + 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.ID, billingCheckoutRefusedReason).Scan(&n); err != nil { |
| + t.Fatal(err) |
| + } |
| + return n |
| + } |
| + // Billing off. |
| + saved := fx.srv.billing |
| + fx.srv.billing = nil |
| + status, body := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", keep, token) |
| + fx.srv.billing = saved |
| + if status != http.StatusConflict || open() != 1 { |
| + t.Fatalf("keep with billing off: %d %v, %d open; want 409 and the review still open", status, body, open()) |
| + } |
| + // The gate's own read finds the session expired (the refusal expired |
| + // it), which refuses keep; but during that read the same resolution is |
| + // committed by another request. The answer is the replay's 200. |
| + f.mu.Lock() |
| + f.sessionAnswers = []func(*fakeBillingStripe){func(*fakeBillingStripe) { |
| + if err := fx.st.resolveBillingReview(billingReviewResolution{AccountID: acct.ID, Subscription: intent.ID, Reason: billingCheckoutRefusedReason, Occurrence: 1, Actor: "ops@keymail.dev", Note: "corrected at the provider", Outcome: "keep"}, f.now.Add(time.Minute)); err != nil { |
| + t.Errorf("the competing resolution: %v", err) |
| + } |
| + }} |
| + f.mu.Unlock() |
| + if status, body := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", keep, token); status != http.StatusOK { |
| + t.Fatalf("keep decided during the gate: %d %v; want the replay's 200", status, body) |
| + } |
| + // The same race with another note is the store's 409, not the gate's. |
| + other := map[string]any{"account": acct.Sqid, "subscription": intent.ID, "reason": billingCheckoutRefusedReason, "occurrences": 1, "note": "another note", "outcome": "keep"} |
| + if status, body := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", other, token); status != http.StatusConflict || !strings.Contains(fmt.Sprint(body["error"]), "already resolved") { |
| + t.Fatalf("a different note after the decision: %d %v; want the store's 409", status, body) |
| + } |
| +} |
diff --git a/internal/console/billingoperator.go b/internal/console/billingoperator.go| index 12966dff4..3c4ebc82c 100644 |
| --- a/internal/console/billingoperator.go |
| +++ b/internal/console/billingoperator.go |
| @@ -341,13 +341,24 @@ func (s *Server) handleAPIBillingReviewResolve(w http.ResponseWriter, r *http.Re |
| } else if s.billing == nil { |
| jsonError(w, http.StatusConflict, "billing is not configured on this console, so the intent's session cannot be read; resolve with retire") |
| return |
| - } else if why, err := s.billingKeepRefused(r.Context(), acct, req.Subscription); err != nil { |
| - log.Printf("console: api billing review keep: %v", err) |
| - jsonError(w, http.StatusServiceUnavailable, "the intent's session could not be read. try again") |
| - return |
| - } else if why != "" { |
| - jsonError(w, http.StatusConflict, "keep needs a completed session with a subscription ("+why+"); resolve with retire") |
| - return |
| + } else if why, gerr := s.billingKeepRefused(r.Context(), acct, req.Subscription); gerr != nil || why != "" { |
| + // The gate refused, or the provider failed. Another request may |
| + // have resolved this occurrence while the gate was reading (two |
| + // identical requests; another console): then the answer is the |
| + // store's exact replay comparison, not the gate's (review |
| + // eighteen, Astra 2). |
| + again, rerr := s.store.billingReview(acct.ID, req.Subscription, req.Reason) |
| + switch { |
| + case rerr == nil && again.ResolvedAt != 0 && again.Occurrences == req.Occurrences: |
| + // decided meanwhile: fall through to the store |
| + case gerr != nil: |
| + log.Printf("console: api billing review keep: %v", gerr) |
| + jsonError(w, http.StatusServiceUnavailable, "the intent's session could not be read. try again") |
| + return |
| + default: |
| + jsonError(w, http.StatusConflict, "keep needs a completed session with a subscription ("+why+"); resolve with retire") |
| + return |
| + } |
| } |
| } |
| outcome := req.Outcome |
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go| index 7517c1a50..129940ffd 100644 |
| --- a/internal/console/billingservice.go |
| +++ b/internal/console/billingservice.go |
| @@ -297,13 +297,15 @@ func (e *billingIdentityOfAccount) Unwrap() error { return errBillingIdentity } |
| // provider, so the expiry can make nothing but this intent's purchase |
| // unpayable), with the answer 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 or keeps it for a corrected purchase to record |
| -// it. A read that is not of the bound session (a listing, a create |
| -// response) files nothing: nothing was ever disclosed on it. |
| +// so the intent is retired on no such proof. The review is the one a |
| +// mismatched purchase files, with the session as its reference; the intent |
| +// stands, so no second purchase opens beside it, until the operator |
| +// resolves the review. An expiry that fails here leaves the session |
| +// payable under an open review, and nothing yet proves or retries it: |
| +// that is review eighteen's finding, answered by the retirement protocol |
| +// (2026-10-02-intent-retirement-protocol-design.md), not by this order. |
| +// 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 |
| @@ -1126,17 +1128,18 @@ func (s *Server) bindBillingCheckoutFor(ctx context.Context, sessionID string, e |
| if state.Subscription == session.SubscriptionID && state.Customer == session.CustomerID { |
| return session.SubscriptionID, nil |
| } |
| - intent, err := s.store.billingCheckoutIntent(acct.ID) |
| - if errors.Is(err, sql.ErrNoRows) { |
| - // No intent stands. The completion of a session whose intent the |
| - // operator retired (a refusal resolved) is settled, not retried: |
| - // nothing can ever be recorded from it. |
| - if retired, rerr := s.store.billingIntentRetired(acct.ID, session.Metadata["checkout_intent"]); rerr != nil { |
| - return "", rerr |
| - } else if retired { |
| - 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 |
| + // account, as reconcile's does (review eighteen, Astra 3: with a |
| + // replacement standing it was judged against the replacement and |
| + // recorded with no account). |
| + if retired, rerr := s.store.billingIntentRetired(acct.ID, session.Metadata["checkout_intent"]); rerr != nil { |
| + return "", rerr |
| + } else if retired { |
| + return "", &billingIdentityOfAccount{AccountID: acct.ID} |
| } |
| + intent, err := s.store.billingCheckoutIntent(acct.ID) |
| if err != nil { |
| return "", err |
| } |