carlos / platform Public

Merge commit '35356ad0' into feat/carloku-billing-operator-cli-20261002

claude 3 weeks ago 40ea61b1d157f35f000e70c5bcaed15ca2b9e497 parent 6d4c291parent 35356ad

14 files changed, +1089 −47

docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md +1 −1
docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review24.md +183 −0
docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review25.md +181 −0
docs/superpowers/specs/DISPOSITIONS-multi-currency.md +36 −3
docs/superpowers/specs/OPUS-REVIEW-multi-currency-review24.md +110 −0
docs/superpowers/specs/OPUS-REVIEW-multi-currency-review25.md +142 −0
internal/console/billingnamed_test.go +334 −1
internal/console/billingoperator.go +3 −1
internal/console/billingretirement.go +36 −5
internal/console/billingretirement_export_test.go +11 −0
internal/console/billingstore.go +14 −15
internal/console/deletion_test.go +12 −0
internal/console/ownership.go +24 −19
internal/console/store.go +2 −2

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 5cedfbf20..c9faefe37 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

@@ -125,7 +125,7 @@ Built in `internal/console/billingretirement.go` and the paths it touches, with

- **An unbound intent under a review that names no session** is still reconciled by the pass, which is how its session is found. Nothing records, retires or cleans up an intent under an open review on that path: the store's record, expiry and cleanup each refuse it. (Round twenty's build let the cleanup of an intent that never had a session delete it under its review; review twenty-one.)

- **Named subscriptions** (review twenty-one, answered as one rule). A subscription that names a standing intent is recorded from the first read, whether or not a refusal review exists. Nothing deletes an intent past one. The record of the purchase keeps what it consumed (`billing_consumed_intents`: the intent, its session, the subscription it recorded with) and in the same transaction makes every other subscription seen naming the intent a dispute; a subscription that names the intent afterwards is compared with that record and is a dispute. The expiry of the intent's session, and the cleanup of an intent that never had a session, do not delete an intent a subscription was seen naming: they file the refusal, with it attached, and the intent stands (an expired session has no subscription and an intent with no session never had a checkout, so that subscription was made outside checkout). `retire` is held while anything is attached, whether or not the intent still stands. What stays outside: a subscription first seen naming an intent that went with nothing seen naming it and no purchase (one that expired in the ordinary way; one that never had a session and was cleaned up) is a rejected event with its account and no hold, because nothing is kept of such an intent. What the rule costs: a subscription seen naming an intent holds that intent's expiry and cleanup whatever the subscription's own state, a dead one included, into a refusal stage one can only hold; and a second subscription at a purchase's record is a dispute stage one cannot resolve. If the provider ever makes a subscription before its checkout session completes, an honest buyer meets this and is held with no disposition; whether it does is the first check of the test-mode rehearsal, and a gate on enabling (review twenty-two).

- **Only a first purchase consumes an intent**: the record of a subscription that is not yet the account's recorded one, judged in the record's transaction. A later event of the account's recorded subscription deletes no intent, whatever its metadata names.

-- **A refusal filed late.** A refusal of a session filed for an intent that a purchase consumed meanwhile is kept as a dispute against what was consumed, in the filing's transaction. **Account retirement** refuses while a checkout refusal or dispute is open, and has a point of no return in the database (`commitBillingRetirement`), taken before anything outside is written: checkout evidence filed before it withdraws the pending retirement; evidence filed after it stays with the account, whose deletion finishes, and is listed and resolvable under the deleted account (review twenty-three). **A click** reads the intent's state again after each provider read and before it discloses a new link, and offers nothing under an open review (review twenty-two).

+- **A refusal filed late.** A refusal of a session filed for an intent that a purchase consumed meanwhile is kept as a dispute against what was consumed, in the filing's transaction. **Account retirement** refuses while a checkout refusal or dispute is open, and has a point of no return in the database (`commitBillingRetirement`), taken before anything outside is written: checkout evidence filed before it withdraws the pending retirement; evidence filed after it stays with the account, whose deletion finishes, and is listed under the deleted account, held with no stage-one disposition like every dispute (review twenty-three). An older binary has no commit step: during a roll its deletion reproduces the race, which the roll rule covers (older consoles are stopped before this build starts; a reservation from before the commit column is marked committed in the same transaction that adds the column, so no death or second console between the two leaves it pending). **A click** reads the intent's state again after each provider read and before it discloses a new link, and offers nothing under an open review (review twenty-two).

- **Binding.** A session id is written onto an intent in one place, which refuses a session that is already another purchase's (another intent's, a retired, kept or consumed one's) and one that is not the session the intent's open review names.

- **Creating an intent** re-reads the account's customer and the currency it is held to in its own transaction; a request built from an older reading is refused.

- **The refusals' turn** takes the longest-due first. **A listing** that fails on a later page records the pages it read. **An answer carrying another session's id** records nothing.

diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review24.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review24.md

new file mode 100644

index 000000000..e3675f2f5

--- /dev/null

+++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review24.md

@@ -0,0 +1,183 @@

+All platform references below are at **`e152df30`**; pricing-site references are at **`01b62f7`**. This was source-only review. Nothing was modified or tested, and no network commands were run.

+**Twenty-third review, finding by finding**

+| Item | Closed? | Would its test pass without its fix? |

+|---|---|---|

+| **Opus P3-1:** late refusal uses another key when the consumed intent has no session | **Code fixed** at `internal/console/billingstore.go:1269`: the fallback is now the intent ID. | **Yes, the existing tests would still pass if this particular line were reverted.** `billingnamed_test.go:938` tests subscription classification, while `:758` tests late refusal with a nonempty archived session. Neither exercises this fallback. |

+| **Opus P3-2:** deleted accounts’ reviews lose their account and resolution door | **Closed for the review listing and door.** `billingoperator.go:206` retains the account and marks `account_deleted`; `:343` finds deleted accounts. | **No.** `billingnamed_test.go:1066` requires deletion, the attributed listing row, and a held **409 rather than 404**. It does not prove that a dispute can be resolved. |

+| **Opus P3-3:** recovery’s hold filter has no test | **Closed.** `billingnamed_test.go:1144` exercises lost-create recovery, a wrong-currency session, expiry, and the resulting named-subscription hold. | **No.** Removing the recovery filter at `billingpass.go:297` makes the test’s nil-error assertion fail. |

+| **Opus claim 1:** unready cleanup is unconditional | **Corrected**, `billingservice.go:789`. Both the open-review and named-subscription exceptions are stated. | Not applicable: comment correction. |

+| **Opus claim 2:** the next retry is at least an hour away | **Mostly corrected**, but the replacement has an off-by-one description; see contradicted claims below. | No behavioral change was made. |

+| **Opus claim 3:** the dispositions cite a decision file as though it exists here | **Corrected:** the citation now identifies the launch branch, `DISPOSITIONS-multi-currency.md:500`. | Not applicable. |

+| **Opus claim 4:** no subscription before completion means no cost to an honest buyer | **Reported corrected elsewhere, not verifiable at this pinned commit.** `DISPOSITIONS-multi-currency.md:525` describes the launch-branch edit. The second-subscription limitation is stated in this branch’s retirement design. | Not applicable; the external edit cannot be certified from this tree. |

+| **Astra 1, P2:** evidence after reservation can leave the account externally retired and locally blocked | **The reported normal-path ordering is fixed, but the retirement boundary remains incomplete.** Failure cleanup can erase another console’s committed retirement: Finding 1. | **No** for removal of the new reservation, commit, or post-commit checks exercised at `billingnamed_test.go:965`. **Those tests still pass with Finding 1.** |

+| **Astra 2, P3:** one malformed session drops its otherwise decodable page | **Closed**, `internal/billingstripe/catalog.go:153`. Sessions are decoded separately and successful decodes accompany the error. | **No.** `catalog_test.go:333` requires both the preceding and following valid session alongside the decode error. |

+The gate failure was an obsolete assertion, not evidence that the new attribution rule was wrong. Before round twenty-two’s change, `TestABoundSessionReadBackAsNobodysIsReviewed` reached the unproven-expiry hold because the incorrectly attributed subscription had not been attached. That expectation matched the previous implementation, including its attribution defect. At `2618403f`, the subscription correctly attached to the account recorded for the intent, and `held()` checks that obligation first.

+The updated test at `billingcurrency_test.go:3512` correctly requires the `sub_paid` hold, then deliberately removes the attachment to isolate the unproven-expiry hold at `:3516`. Reverting the attribution fix would fail its first assertion. Keeping the old expectation would fail against the corrected implementation. **The test correction is closed; I did not run the current gate.**

+On the previous severity disagreement, **Astra’s P2 reading was right**. The old reservation check did not govern publication of the authoritative tombstone. Opus’s conclusion that the complete lifecycle boundary was closed at `2618403f` was too broad. The recovery diagnostic and defensive empty-session key remain appropriately P3.

+**Findings, most severe first**

+1. **P2 — A failed deletion request can remove another console’s committed retirement and reopen checkout during tombstone publication.**

+ **Where:** `internal/console/ownership.go:160`, `:162`, `:190`; `internal/console/billingstore.go:1559`, `:1640`. Checkout’s reservation check is at `billingstore.go:363`.

+ **Concrete scenario:**

+ 1. Two consoles share the database and both reserve deletion of an eligible account.

+ 2. Both enter `commitBillingRetirement` and read the reservation before either commits.

+ 3. Console A commits `committed_at` and starts the external tombstone write.

+ 4. Console B’s deferred read transaction cannot upgrade its stale WAL snapshot and returns a busy/snapshot error. Its transaction rolls back.

+ 5. B’s handler calls `releaseBillingRetirement`. Its unconditional `DELETE` removes **A’s committed reservation**.

+ 6. Before A finishes its external write, a checkout click creates a new intent and payable session: `beginBillingCheckout` now sees an undeleted account with no retirement reservation.

+ 7. A publishes the tombstone. Both `DeleteAccount` and `ReapAccount` refuse the new standing intent.

+ This requires no unusual Stripe behavior. One connection per `Store` at `store.go:65` does not serialize the read snapshots of two separate consoles.

+ A narrower variant needs only late dispute evidence after step 5: A publishes the tombstone, then local deletion refuses because the commit marker has disappeared. The new reap can eventually finish that dispute-only variant; it **cannot** finish the new-standing-intent variant.

+ The `errNoReleaseStore` cleanup at `ownership.go:190` has the same authority problem: one console’s knowledge that *it* made no external write does not establish that another console sharing the reservation made none.

+ **Smallest acceptable fix:** release only pending rows, with `committed_at=0`. Check for missing release storage before committing retirement so that this preflight failure never requires removing a committed row. Add a regression covering a losing request’s cleanup after another request commits, and require both the marker to survive and new checkout to remain blocked.

+ The new sequential test calls neither failing handler cleanup nor competing commits. Its passing assertions do not cover this ordering.

+2. **P3 — Rejected-event listings still omit a deleted account’s recorded identity.**

+ **Where:** `internal/console/billingoperator.go:312`, particularly `:314`.

+ **Concrete scenario:** a rejected event is recorded with an account ID. The account is subsequently deleted. `GET /api/billing/rejected` still returns the event, but `AccountByID` filters out the deleted account, so its `account` field disappears.

+ The separate review listing now handles this correctly. This is a smaller attribution gap, not a recurrence of the invisible-review P2.

+ **Smallest acceptable fix:** use the deleted-account-aware lookup for rejected rows too, preserving the recorded account sqid. Extend the listing regression with a soft-deleted account.

+**The retirement boundary and remaining lifecycle checks**

+The intended point of no return is sound **while its committed marker remains durable**:

+- Evidence first: `fileBillingReviewTx` withdraws the pending reservation in the evidence transaction (`billingstore.go:957`); commitment then refuses.

+- Commitment first: later evidence stays recorded; the committed retirement permits local deletion despite the newly opened checkout review.

+- Tombstone first, local process dies: `ReapAccount` accepts that the manifest already retired the account and preserves its billing records and reviews (`store.go:1366`).

+- Deleted account: the review retains its account and reaches the operator door.

+Finding 1 breaks the premise that the committed marker remains durable. Consequently, **I would not yet close review eighteen’s P2 as a whole**.

+“Resolvable” also needs precision. The deleted account’s review is now **listed and reachable**. A checkout dispute still has no stage-one disposition: `resolveBillingReview` returns a held conflict at `billingstore.go:1133`. The new test explicitly expects that result. The visibility fix does not implement settlement, and should not claim to.

+An older binary has no commitment step and can still publish a tombstone without following this protocol. The migration’s default `committed_at=0` does not teach it otherwise. The documented rollout rule—stop older consoles before starting this build and do not roll them back—remains necessary.

+The intent-level invariant supported by the inspected code is:

+> A recorded naming subscription survives consumption as accepted purchase identity or a dispute; expiry and cleanup preserve intents with recorded obligations or open refusals; operator retirement requires the recorded proof and absence of obligations in its own transaction.

+The complete mutation inventory is:

+| Door | Recorded evidence and subsequent observations |

+|---|---|

+| **Ordinary expiry — `billingstore.go:553`** | Pass, click and refusal handling require identity/proof and the exact intent. An observed naming subscription causes a refusal instead of deletion. Open review or dispute prevents deletion. |

+| **Unready cleanup — `billingstore.go:604`** | Pass, replacement checkout and deletion preparation require never-created state. A named subscription or open refusal prevents cleanup. |

+| **First-purchase consumption — `billingstore.go:890`** | Exact standing-intent consumption, billing state, assignments, event, entitlement work, consumption archive and disputes about other subscriptions commit together. Refusal rolls everything back. Renewals consume no intent. |

+| **Operator retirement — `billingstore.go:1208`** | Deletes exactly the reviewed intent, with its retirement archive, resolution evidence and quantity-event outcome in the same transaction. |

+| **Session binding — `billingstore.go:458`, `:461`** | Both writes are inside `bindIntentSession`; another purchase’s session or one conflicting with the review reference is refused. Rebinding the same valid session remains possible. |

+| **Account deletion/reap** | Soft deletion; no checkout-intent cascade. Billing records survive. Finding 1 remains at the external retirement boundary. |

+The two documented no-archive exceptions remain: ordinary expiry and never-created cleanup, each with **nothing previously observed naming the intent**. A subscription first observed after either can become only a rejected event. Consumption and protocol retirement retain archives and do not share that exception.

+**Other requested checks**

+| Scope | Source conclusion |

+|---|---|

+| **A: one GBP purchase** | With `?country=IN&currency=gbp`, the page selects India’s band and GBP (`billingpage.go:291`). A valid posted form, including the required postal information, passes effective-quote comparison (`billinghandlers.go:81`); checkout derives quantity and `PriceTerms` (`billingservice.go:494`). `CreateCheckout` sends GBP with Adaptive Pricing disabled (`billingstripe/client.go:589`). Binding and reconciliation verify ownership and purchase identity; state and assignments retain the book/currency (`billingstore.go:805`). History (`billinginvoicestore.go:524`), allowance (`plans/month.go:100`), calculation (`billingcalculation.go:119`), invoice requests/recovery/verification (`billinginvoice.go:856`, `:958`) and quantity updates (`billingquantity.go:1008`, `:1099`) preserve GBP. I found no conversion or silent EUR fallback in this trace. |

+| **B, M: one form** | Present currency must be exactly one supported value; empty, repeated, uppercase, padded or unsupported values return 400 on both doors (`billinghandlers.go:176`). An absent control supplies no explicit choice. The effective country-default currency must still match the signed quote. Changed inputs, erased choice flags and fabricated quote fields cannot authorize an unseen quote. A stale tab may buy its own still-valid quote; another tab’s cookies do not alter its posted purchase. Legacy no-book behavior remains EUR. |

+| **Quote redirect** | The quote door checks session, form token and membership, then constructs the account’s fixed billing path with encoded parameters (`billinghandlers.go:165`, `:197`). Posted values cannot choose another destination or initiate checkout. |

+| **Q, U, Z: signature** | The HMAC key derives from the server billing secret, not the disclosed CSRF value (`billingpage.go:446`). It covers session token, account, book, country, band, currency, choice flag and member count. Cross-session/account/book/token reuse fails. Plan is not signed because the page quotes the offered plans; quantity derives from the signed membership count and plan minimum. The count is compared again and validated under the membership write lock. Postal information affects tax/location, not the displayed pre-tax book amount. |

+| **C, O: cookies and held currency** | Present-empty clears; absent falls back; invalid changes no cookie (`billingcurrency.go:43`). Thus an invalid link can leave an older cookie available on a later parameter-absent visit; it does not silently authorize checkout. Held currency overrides URL/cookie suggestions and is offered alone (`billingpage.go:298`), including legacy EUR. Service and store enforce the hold after cancellation too. |

+| **E, N: allowance barrier** | The trigger at `billingstore.go:208` uses positive paid overlap ending at the next assignment, paid-through instant or month end, matching `AllowanceForMonth`. It orders both assignment tables by effective time and revision. Same-second supersession contributes no span; free/inactive rows do not count. Renewal and quantity writers preserve provider `period_end` as paid-through, distinct from effective time (`billingstore.go:842`). I found neither requested valid-history counterexample. |

+| **H, T: migration** | The refined trigger is present before `retireEarlierBarrierTrigger` verifies it and transactionally removes the old name (`billingstore.go:1001`). Missing new trigger leaves the old one; another current startup is idempotent. A prohibited earlier feature binary can recreate the wider trigger. Barriers prevent particular old writes; they do not make rollback generally supported. |

+| **F: inclusions** | The page divides `PriceFor`’s pooled allowance by charged quantity (`billingpage.go:395`). Disabled Personal uses its valid one-member quantity; teams above minimum use their charged quantity. Current factors give exact displayed shares at the tested precision. `billingcurrency_test.go:2554` checks larger quantities and pins band-A sentences. |

+| **G, J: book and export** | Validation covers dimensions, duplicates, rounding, overflow and legacy-EUR agreement (`pricebook.go:140`, `:231`). Catalog configuration requires complete sets and unique provider IDs (`cmd/console/billing.go:99`). Provisioning and console checks agree on own-only expanded `currency_options`. The export contains public prices, resources and country mappings, without credentials, customer data or Stripe catalog identities (`pricebook.go:714`). |

+| **K: pricing site** | The pinned export matches the platform export. Rates retain micro-unit precision; inclusion scaling and exactness checks derive from exported values (`copy-review/pricing/build.mjs:91`). Country options/defaults come from the book. Links always contain both parameters and carry currency only when chosen (`src/site.js:27`). Locale-only display suggestions intentionally are not transmitted as choices. The first-line draft marker and exact `PRICEBOOK_ALLOW_DRAFT=1` check agree with the checker. Approved numeric tokens are checked against the export. |

+| **MM, QQ: site tests** | Whole-token output checks cover the rendered slots across currencies (`test/pricing-pricebook.test.mjs:202`), and `hack/money.mjs` is the exercised formatter. A wrong literal fails; a literal exactly equal to the expected current value passes. An output-equality test cannot establish that an equal literal was not substituted, and the source acknowledges that limit. |

+| **X, AA, CC: quantity history** | `quantityOutcomesSQL` supplies the guard, startup detection and obligation checks (`billingquantityaudit.go:20`, `:42`, `:61`). The healing sweep is gone. Startup does not clear pre-history or clock-rollback gaps. |

+| **YY, ZZ, BBB, CCC: operator records** | Old omitted outcomes remain `retire`; exact replay compares actor, note and outcome. References and resolution evidence survive. Rejected events are operator-only, newest first, bounded to 200, and rows are drained before account lookups (`billingoperator.go:284`). Finding 2 remains for deleted-account attribution. The list is not a ledger of every interactive error or retry. |

+For **ownership and refusal paths**, `ownSession` checks mode, intent, both account identifiers, known customer and bound ID (`billingservice.go:273`). `intentSession` then checks purchase fields. Both create sites and listing recovery refuse foreign unbound sessions without expiring them. Expected-intent binding checks the expected account before deriving an account from provider metadata.

+There is an intentional exception for a **previously bound session** with contradictory metadata: the console files the review, then may expire its **own recorded bound ID**. This neither authorizes expiry of an ID supplied by the contradictory response nor accepts that response as retirement proof. Given a previously disclosed payable link, expiring the pinned ID is the justified reading of the earlier disagreement. An unowned create/listing result remains open at the provider because the console has no equivalent binding authority.

+Binder and reconciliation compare the first purchase consistently. Owned completed mismatches file the same intent-keyed review; repeated reads do not create repeated open occurrences. Reconcile attaches an offending subscription with the refusal before advancing expiry. Failure or process death leaves the review standing; a completion winning the race is classified from the subsequent read.

+A session that has not completed, or has completed without naming its subscription yet, causes retry rather than premature refusal. The subscription read has already been recorded as naming the intent. Therefore a hand-made subscription is not forgotten while that association remains unknown. Once the session names its own subscription, a different one is refused. Renewals and late events of the recorded subscription do not consume a newer intent.

+For **DDD–JJJJ**, the remaining stage-one behavior is conservative and explicit:

+- Fate is written once; contradictory identity or terminal evidence creates a durable dispute.

+- Resolution reads fate, attachments, protocol and dispute state transactionally. A stale caller cannot release newly recorded obligations.

+- `keep` requires the owned, matching, completed and acceptably paid bound session. It does **not** validate all subscription terms itself; reconciliation can reopen the refusal. That limitation is accurately stated in “Stage one as built.”

+- Other subscriptions seen after `keep` reopen the standing refusal or become archived-purchase disputes after consumption.

+- Protocol-zero reviews, disputes, and attached obligations remain held. A status such as `incomplete_expired` alone does not settle them.

+- Read, storage or expiry failures remain retryable; settled identity rejections are acknowledged only after required classification.

+- The delete barrier is recreated transactionally. It does not supply all protocol protections to an older binary.

+- The refusal turn has its own deadline, inherits caller cancellation, has a three-attempt budget and selects longest-due first. This avoids the reported fixed-order starvation, not arbitrary-workload completion guarantees.

+All inspected session/subscription reads—checkout/resume, recovery, binder, reconciliation, pass, operator keep/advance, quantity worker and invoice checks—use `readCheckout`, `listCheckouts` or `readSubscription` (`billingretirement.go:841`). Store failures return no usable object. Successfully decoded listing entries are recorded before the listing error reaches its caller.

+The exclusions remain meaningful: create/expiry responses do not establish fresh fate; pre-refusal session reads do not create a refused-purchase record; identity-less/open reads before any fate record no contradiction. Those exclusions keep a case held rather than authorize retirement. Wrong-request-ID answers record nothing and are refused by callers.

+The structural guard catches selector uses through receiver aliases, method values, method expressions and other receiver types with the classified method names. It also inventories exported client methods. Its wrapper-order check is **lexical**, not control-flow proof; a dead or conditional recording call could satisfy it. No such production bypass was found.

+**Test and mutation conclusions**

+The targeted source assertions support the reported fixes, with limits that matter here:

+- The four formerly surviving round-twenty mutations now have distinguishing cases: identity clauses changed independently; an incomplete additional subscription reopening after `keep`; the binder’s sole read recording the refusal’s fate; and a contradiction appearing only on the second resume read.

+- The attachment-failure regression now fails inside `fileCheckoutRefusal`, so splitting its review and attachment transactions would fail the test.

+- The fake’s separate second session and late retired-completion assertions distinguish incorrectly recording the retired purchase.

+- The historical-retirement helper represents pre-protocol state; it does not establish that current code can legitimately produce that state.

+- Review-three’s default-as-explicit-choice mutant remains behaviorally equivalent under the matching quote. Review-four’s provisioning tests inspect script text; they do not execute its jq or provider calls. Successful migration does not simulate every crash ordering. Removing an exactness assertion over already-exact data need not fail a test.

+- The eleven round-twenty-three mutation descriptions have corresponding distinguishing assertions for the changes they describe. “Late refusal not kept” is a broader mutation than reverting the empty-session fallback, which remains untested.

+- **Sequential store tests do not cover the losing-transaction cleanup in Finding 1.** SQLite’s single writer does not make separately read snapshots and subsequent handler cleanup equivalent to the tested sequence.

+- I found no additional shared-cookie-jar, temporary-root or calendar-dependent assertion defect. I cannot certify executed mutant counts or a green gate from source.

+For honest buyers, I found no additional provider-established regression relative to `b260ac8e`. Duplicate own events remain idempotent; ordinary renewals and cancellation do not become disputes. A subscription observed before completion is retained and retried. If abandonment leaves that subscription behind, or the provider replaces it with another ID, the conservative hold can become unresolvable in stage one. Those provider behaviors remain rehearsal gates, not facts established by the fake.

+**Claims the code contradicts**

+1. **A committed retirement is a point of no return.** `billingstore.go:1552` and the retirement design at `:128` overstate this while `releaseBillingRetirement` can delete a committed row. Finding 1 is the concrete counterexample.

+2. **Deleted-account reviews are “resolvable.”** They are attributable and reachable, but a dispute still returns 409 at `billingstore.go:1133`. The test at `billingnamed_test.go:1119` proves that refusal. Say “listed and reachable through the resolution door; existing stage-one holds still apply.”

+3. **The retry becomes hourly after three attempts.** `billingretirement.go:971` is off by one. The SQL at `:803` schedules 60, 300 and 900 seconds after the first three attempts; hourly scheduling starts after the fourth. “Within a few passes however many others” also promises more than a budget of three establishes.

+4. **A late refusal “files nothing.”** The comment at `billingstore.go:1216` contradicts the archive classification and consumed-purchase dispute writes at `:1244`.

+5. **Only `keep` reaches the provider.** `billingoperator.go:21` is stale: `retire` also calls `advanceRefusal` at `:380`.

+The claimed launch-branch enablement wording correction is not independently verifiable from the requested commit.

+**What must be verified against Stripe test mode before enablement**

+1. **Catalog identity:** exercise all 36 prices, exact amounts/currencies, tax behavior, metadata and expanded own-only `currency_options`. Confirm provisioning and console accept the same valid shapes and reject mismatches.

+2. **Checkout choices:** buy India plus explicit GBP, country defaults, cleared suggestions and held legacy EUR. Confirm the shown base amount, disabled Adaptive Pricing, session/subscription currency and stored purchase identity agree.

+3. **Event and payment ordering:** run declined-and-retried cards, abandoned 3-D Secure and delayed methods. Observe when subscriptions exist, when sessions name them, and whether one intent can receive multiple subscription IDs. This decides whether the conservative hold affects honest buyers.

+4. **Outstanding obligations:** inspect payments, pending collection, invoices and refunds for `incomplete_expired` and canceled subscriptions. Establish the evidence needed for the owner’s settlement decision; subscription status alone is insufficient.

+5. **Expiry versus completion:** race retained payment links against expiry, including timeouts and repeated idempotency keys. Verify a fresh GET distinguishes proven expiry from a winning completion and that an expired session cannot subsequently collect.

+6. **Identity and recovery:** verify create/GET/list/expiry identity fields, metadata edits, lost responses, original-key replay, pagination and key-retention expiry. Confirm session IDs are not reused.

+7. **GBP overages and quantities:** exercise invoice creation, lost-response recovery, item/final-line verification, payment and quantity/proration changes. Every amount and recovered request must remain GBP, with correctly dated allowances.

+8. **Legacy and ended purchases:** verify unchanged legacy EUR requests/hashes, renewal/cancellation, customer reuse in the held currency and historical invoice preservation. Establish which canceled-subscription fields can actually change.

+**OOOO:** acceptance still requires the retirement-release fix and its distinguishing regression. What remains is therefore not only rehearsal, decisions and owner approvals.

+**Verdict: reject.**

\ No newline at end of file

diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review25.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review25.md

new file mode 100644

index 000000000..1894e151b

--- /dev/null

+++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review25.md

@@ -0,0 +1,181 @@

+All platform references are at **`1f1368b0`**; pricing-site references are at **`01b62f7`**. This was source-only review. Nothing was modified or tested, and no network commands were run.

+**Twenty-fourth review, finding by finding**

+| Item | Closed? | Would its test pass without its fix? |

+|---|---|---|

+| **Opus P3-1:** handler’s commit call untested | **Closed.** `billingnamed_test.go:1228` drives the deletion handler and inspects the marker at the bucket’s first write. | **No.** Removing the commit or moving it after the tombstone fails the explicit assertion at `:1280`. The hook also files a real webhook and checks deletion and attribution. |

+| **Opus P3-2:** losing request releases another request’s committed reservation | **Closed in the handler’s code**, `ownership.go:176`, `billingstore.go:1620`. | **No** if the conditional release helper becomes unconditional. **Yes** if only the handler is changed back to calling unconditional release. That surviving mutant remains a coverage gap. |

+| **Opus P3-3:** pre-column reservations become pending | **Partially closed.** Successful migration preserves them, but interruption between its two writes defeats the conversion. Finding 1 below. | **No** if the backfill is removed. **Yes** with the crash window intact: `billingnamed_test.go:1324` never interrupts migration. |

+| **Opus P3-4:** detached function comment | **Closed**, `billingstore.go:1605`. | Not applicable: comment placement. |

+| **Opus claim 1:** deleted-account reviews are “resolvable” | **Corrected** in the governing design and changed comments: they are listed and held without a stage-one disposition. | The existing **409, not 404** assertion establishes visibility and the hold, not resolution. |

+| **Opus claim 2:** serialization also protects older binaries | **Correctly qualified** in the governing design: older binaries lack the commit step and must be stopped. | No mixed-binary test establishes the operational rollout. The referenced launch-branch `ENABLEMENT.md` is outside the pinned platform tree inspected here. |

+| **Astra 1, P2:** committed marker is removable by cleanup | **Closed in current handler paths**, with the call-site test limitation above. | Helper regression fails; handler-call regression still passes. |

+| **Astra 2, P3:** rejected listing loses a deleted account’s name | **Closed**, `billingoperator.go:316`. | **No.** The assertion at `billingnamed_test.go:1309` requires the deleted account’s sqid on the rejected event. |

+**Astra’s P2 severity was right for the shared cleanup defect.** Removing the marker could reopen checkout while another request published the tombstone, then prevent local deletion because a new payable intent existed. That is a broken retirement boundary, not merely a diagnostic problem. Neither unusual Stripe behavior nor a successful payment is needed.

+**Findings, most severe first**

+1. **P2 — Adding the commit column and preserving old reservations are not atomic.**

+ **Where:** `internal/console/billingretirement.go:184`, particularly `:190–198`.

+ The migration checks whether `committed_at` exists, executes `ALTER TABLE`, then executes the backfill through a separate `s.db.Exec`. Its transaction begins only afterwards.

+ **Concrete scenario:**

+ 1. An older binary retained a reservation following an ambiguous tombstone write.

+ 2. Migration commits the addition of `committed_at DEFAULT 0`.

+ 3. The process dies, or the backfill fails, before the next statement completes.

+ 4. Restart sees the column and permanently skips the backfill.

+ 5. Checkout evidence treats that old reservation as pending and deletes it through `cancelPendingRetirementTx` (`billingstore.go:1547`).

+ The database has now lost the barrier protecting an account whose external tombstone may already exist. Startup materialization does not guarantee rescue: an unreadable manifest is skipped (`materialize.go:68`), allowing the console to continue with its existing local account. A new checkout can then pass the reservation check at `billingstore.go:363`; later reaping refuses its standing intent.

+ This also admits a startup interleaving: another opener can observe the added column before the first opener backfills it.

+ **Smallest acceptable fix:** put the column-existence check, `ALTER TABLE`, and legacy-row backfill in one SQLite transaction. Preserve the distinction between legacy reservations and newly created pending reservations.

+ Add a regression that forces failure between addition and backfill, verifies rollback, then retries migration. A trigger that aborts the backfill can exercise this without killing a process. The successful-migration test at `billingnamed_test.go:1324` does not cover this boundary.

+2. **P3 — The failed-commit handler cleanup still has no regression that protects its choice of release function.**

+ **Where:** `internal/console/ownership.go:176`; `internal/console/billingnamed_test.go:1127`; dispositions `:545`.

+ Current production code is correct. However, changing that single handler call back to `releaseBillingRetirement` restores the previous P2 while the new tests remain green. The test calls `releasePendingBillingRetirement` directly at `billingnamed_test.go:1138`.

+ The surviving mutant is not exclusively reachable through a transient database error. The handler configurations support this ordering:

+ 1. Request A reserves and pauses.

+ 2. A no-bucket request B shares that pending reservation, then removes it during preflight cleanup.

+ 3. A’s commit returns ordinary `errBillingRetirement` because the reservation is missing.

+ 4. Before A cleans up, request C reserves and commits.

+ 5. A’s **mutated unconditional cleanup** removes C’s committed reservation.

+ The current conditional cleanup preserves C’s marker.

+ **Smallest acceptable fix:** add a deterministic handler regression around failed commit and cleanup, with another request’s committed reservation present when cleanup runs. Assert that the marker survives and checkout remains blocked. It need not run two processes or depend on a naturally occurring database error.

+ I would not waive this regression for the retirement boundary merely because the helper itself is tested.

+**Committed-marker durability and deletion, step by step**

+For reservations correctly marked committed, **all current handler cleanup paths preserve the marker**:

+| Step or failure | Evidence, competing requests and process death |

+|---|---|

+| Intent preparation — `ownership.go:148` | Preparation uses the ordinary guarded expiry/cleanup paths. A payable, reviewed or subscription-named intent remains and prevents reservation. A completed purchase records and then blocks retirement. |

+| Reservation — `:151` | `reserveBillingRetirement` checks obligations in its transaction. The reservation blocks new checkout and month freezing. Death leaves it blocking until a deletion retry or qualifying evidence withdraws it. |

+| No-bucket preflight — `:160` | Happens before commit. Cleanup removes only pending rows. It cannot erase another request’s committed row. |

+| Commit — `:172` | Rechecks the retirement conditions and requires a reservation. Evidence committed first withdraws the pending reservation; commit then refuses before external writes. Commit first makes later evidence belong to the retiring account. |

+| Failed commit — `:176` | Conditional cleanup cannot remove a committed row, including one committed after the failure but before cleanup. Its handler call remains insufficiently tested. |

+| Tombstone — `:198` | Every failure retains the committed reservation. Death before, during or after the write leaves checkout blocked. A later deletion retry can continue. |

+| Local deletion — `:211` | Post-commit checkout disputes do not prevent completion. The account row and billing evidence survive deletion. If the tombstone succeeded but local deletion failed, retry or materialization can finish. |

+A handler **can remove another request’s pending reservation**: reservations are account-scoped, not request-owned. That is safe here because the other request must subsequently prove the reservation still exists before committing. No handler now calls unconditional release.

+`ReapAccount` correctly treats an already-retired manifest as past the external boundary (`store.go:1370`). It still refuses an active subscription, standing intent, unsettled month or quantity obligation. A dispute remains attributable to the deleted account; it does not disappear into account deletion.

+The operator listing supplies the sqid and `account_deleted`; the resolution door finds the account and answers **409 held**. That provides identification and preserves the obligation. It provides no adjudication mechanism, as stage one expressly states.

+**Other requested source conclusions**

+- **GBP purchase, A–C, M–Q and U.** `GET ?country=IN&currency=gbp` renders band D in GBP; Personal is £7.70 annually. The signed form posts GBP as a choice. The checkout handler compares the effective purchase currency with the signed quote before starting checkout (`billinghandlers.go:71`). The intent, Checkout request, binding, subscription reconciliation, account state and assignments retain that book/currency identity. Monthly allowance and calculation retain GBP; invoice creation, recovery, line verification and quantity updates use the stored currency. I found no remaining path that converts this purchase or silently prices it in EUR.

+- **One form and signatures.** A changed country/currency or fabricated quote redirects before purchase. Present-empty, repeated, unsupported, padded or differently cased currency fields receive 400 on both POST doors. A missing field selects the country default, subject to quote comparison; legacy configuration remains EUR-only. The signature at `billingpage.go:446` uses a billing-secret-derived key and covers session token, account, book, country, band, currency, choice flag and member count. The disclosed form cannot sign another quote. Another tab does not invalidate an otherwise matching signed quote. Plan is omitted because all offered plans are priced on that page; quantity follows the signed membership count and plan minimum. Postal code affects location/tax validation, not the displayed exclusive-tax plan price.

+- **Quote endpoint and cookies.** The quote endpoint performs its gates and redirects to the account’s billing page using encoded parameters; it does not create a purchase. Cookies affect rendering, not checkout authority. Present-empty suppresses fallback and clears; absent falls back; invalid leaves the existing cookie unchanged. Consequently, an old choice can reappear on a later parameter-absent request after an invalid parameter, consistent with “invalid sets nothing.” A held currency overrides URL/cookie suggestions and is the sole option, including legacy EUR (`billingpage.go:304`).

+- **Ownership, D/L/R and subsequent refinements.** `ownSession` checks mode, intent, both account identifiers, pinned customer and bound session ID (`billingservice.go:273`). Create refusals and recovery cannot acquire authority from matching currency alone. Provider observations are recorded before the caller’s ownership decision; recording contradictory evidence is not permission to bind or charge. The deliberate expiry exception uses the console’s already-bound ID through `advanceRefusal`, not an ID supplied by conflicting metadata. Leaving an unbound, unowned session open is appropriate; expiring the console’s trusted bound ID is also appropriate.

+- **Binder and reconcile.** Their purchase checks agree on plan, band, terms, book and currency. A completed wrong-purchase session produces the intent-keyed refusal once while open. A matching subscription arriving before completion, or before the completed session names its subscription, retries rather than inventing a refusal (`billingservice.go:1022`, `:1033`). Named subscriptions are nevertheless recorded before that return. A different named subscription cannot subsequently vanish during consumption.

+- **Barrier, E/H/N/T.** The refined trigger at `billingstore.go:208` matches positive paid spans used by `AllowanceForMonth`: active, nonfree, clipped by month boundaries, paid-through and the next assignment across both tables. Same-second supersession uses revision order and can contribute zero. Free/inactive assignments truncate earlier spans without themselves counting. `period_end` means the provider’s paid-through endpoint, not supersession time. I found no valid-history counterexample permitting an older v1 freeze where this build uses book history, or refusing this build’s legitimate legacy-only v1. Trigger retirement checks that the new trigger exists before dropping the old one. A crash can leave both names temporarily, not neither; restart completes retirement. This is separate from Finding 1.

+- **Inclusions, validation and export, F/G/J.** Inclusions come from pooled allowances at charged quantity, with unavailable Personal handled separately. Current terms produce exact per-member figures across bands and quantities; band A matches the approved sentences. Book validation rejects malformed or duplicate keys, missing entries, inconsistent rounding/EUR values and overflowing arithmetic. Configuration requires the current catalog and whole optional historical catalogs, with distinct price IDs. Provisioning accepts at least the required book keys and the console’s own-only `currency_options` rule. The public export exposes commercial terms, not account or provider secrets.

+- **Pricing site, K.** At `01b62f7`, prices, exact overage rates, floors, country options/defaults and scaled inclusions derive from the export. Both link parameters are always present; currency is populated only when chosen (`src/site.js:77`). Locale-only GBP can therefore lead to a console USD default when no country or currency was chosen: that is the owner’s explicit suggestion-only rule. The first-line marker and exact `"1"` draft override are implemented (`build.mjs:34`, `:376`), with the check integrated into the ordinary build. Wrong-valued amounts in slots are checked across currencies; replacing a derived value with the **identical current literal** remains observationally indistinguishable, and the tests acknowledge that limit.

+**The lifecycle invariant at this commit**

+A reviewed intent remains standing until an allowed transactional disposition. A first purchase cannot consume it under an open refusal or dispute. Observed named subscriptions survive consumption and reviewed retirement; later incompatible evidence becomes a durable account dispute.

+The mutation points are:

+| Mutation | Paths reaching it and protection |

+|---|---|

+| Bind session — `billingstore.go:458`, `:461` | `saveBillingCheckout` and binder share `bindIntentSession`. Another purchase’s session and a session contradicting the review’s reference are refused. Rebinding the same session is allowed. |

+| Ordinary expiry deletion — `:553` | Click, pass, deletion preparation and refused-session cleanup. Exact proof is required; open reviews prevent deletion; observed named subscriptions cause a refusal instead. |

+| Unready cleanup — `:604` | Start, pass and account-deletion preparation. Requires no created session; open reviews prevent deletion; named subscriptions cause a refusal. |

+| First-purchase consumption — `:890` | Production reconciliation calls `applyBillingStateDated` with the first-purchase requirement (`billingservice.go:1142`). Exact intent consumption, account/assignment writes, event recording, entitlement work and other-subscription disputes commit together. Failure rolls them all back. Renewals do not consume a newer intent. |

+| Operator retirement — `:1208` | Resolves and archives exactly the reviewed account/intent, with its session/reference, customer and fate; closes only its pending checkout quantity events. Another standing intent is untouched. |

+| Account deletion/reap | Marks the account deleted rather than cascading away billing evidence. Existing intents block it. Finding 1 is the remaining migration hole in its reservation boundary. |

+`billing_consumed_intents` is written when an intent is actually consumed. Its historical subscription comparison prevents an honest old subscription replay from becoming a dispute after a later purchase. Intent IDs are generated identifiers; binding refuses reused archived session IDs.

+The two documented unarchived exceptions remain: ordinary expiry and never-created cleanup **with no subscription previously observed naming the intent**. A subscription first observed afterwards can become only a rejected event. Those are explicit limits, not evidence that was recorded and then erased.

+The three wrappers are the only direct session/subscription reads in the console: `billingretirement.go:857`, `:876`, `:894`. Start/resume, binder, reconcile, recovery, pass, operator gate, quantity worker and invoice preflight use them. Storage failure prevents delivery of the object to the caller. Partial listings preserve and record decodable observations before returning their error.

+The exclusions remain material:

+- Create/expiry answers can be idempotency replays; they generally do not prove current fate.

+- Session reads before refusal are not retained as refusal evidence; the observation causing filing is handed in again.

+- Identity-less/open observations before any fate do not establish one. Their earlier assertion is lost; a later valid expiry can become the first admissible proof.

+- Wrong-ID responses are not accepted as evidence about either session.

+- The account’s already-recorded subscription is excluded from named-intent classification.

+Those qualifications are necessary whenever describing “nothing observed is dropped.”

+`keep` still validates the session, not the subscription’s complete purchase identity. A wrong subscription can reopen the refusal on reconciliation; the governing design expressly accepts that behavior. Extra obligations, disputes and migrated reviews remain held. Old clients omitting outcome still request `retire`; they do not acquire authority over paid cases.

+The refusal pass has separate caller-cancellable time, a three-item budget and oldest-due ordering. Failed expiry preserves the review; payment winning expiry records completion. The quantity-history healing sweep is gone, and `quantityOutcomesSQL` supplies the guard, sweep and obligation check. Existing pre-history and clock-rollback holds are not silently cleared.

+I found no additional demonstrated regression for an honest buyer, relative to the named-subscription design, based on provider behavior established by this source. Whether failed authentication, delayed payment or provider replacement produces additional subscriptions remains a rehearsal question. Duplicate delivery alone does not create a second obligation.

+**Tests and claims the code contradicts**

+The four previously surviving round-twenty mutations now have discriminating assertions:

+| Mutation | Why the revised test fails without the fix |

+|---|---|

+| Ignore one identity clause | `billingretirement_test.go:942` varies clauses individually and requires the dispute. |

+| Do not reopen on attachment after `keep` | `billingretirement_test.go:815` uses an incomplete subscription, preventing reconcile’s later refusal from masking classification. |

+| Do not record the session that caused filing | `billingevidence_test.go:315` requires the completion evidence with exactly one provider request. |

+| Offer a session contradicted only by the second read | `billingevidence_test.go:294` checks no destination, pending status and the specific dispute. |

+The transaction-failure test exercises `fileCheckoutRefusal`, rather than merely demonstrating SQLite rollback. The listing tests distinguish arrival order from either ID sort direction. The revised fake’s distinct session IDs and late replay assertions materially test non-recording of the retired purchase.

+These establish their controlled sequences, not every two-console interleaving. The structural guard catches selectors through aliases, method values/expressions and other receiver types, and requires classification of new exported client methods. Its wrapper-order check is lexical, not a control-flow proof.

+The review-three/four qualifications now matter: provisioning tests inspect script text; deleting a divisibility assertion over unchanged exact data need not fail; equivalent or redundant guards can survive mutation. I cannot certify historical mutation runs from source. I found no additional concrete shared-cookie, fixture-order, temporary-root or wall-clock defect in the inspected tests.

+Remaining contradicted claims are:

+- **“Old reservations are marked committed when the column is added”**, dispositions `:540` and governing design `:128`: only if both autocommitted migration statements finish. Finding 1.

+- **The surviving handler mutant needs a transient database error**, dispositions `:545`: that misses a new commitment between an ordinary failed commit and cleanup. Finding 2.

+- **“Only this request’s own pending reservation”**, `ownership.go:158`: there is no request ownership. The conditional deletion can cancel another request’s pending reservation; the commit recheck makes that safe.

+- **A late filing “files nothing”**, `billingstore.go:1222`: after consumption, `fileCheckoutRefusal` can file the surviving dispute at `:1271`. The comment predates that protection.

+- **Hourly retry after three tries**, `billingretirement.go:987`: the SQL uses the previous counter. Delays are 1, 5, 15 minutes, then hourly after the fourth unsuccessful try.

+**What would close review eighteen’s P2**

+I would require the atomic migration fix and its interrupted-migration regression, plus the handler cleanup regression that kills the surviving call-site mutant. Then the code issues found in this review would be closed.

+The owner’s deferred dispositions, provider rehearsal and rollout restrictions are separate enablement conditions. Conservative cases intentionally held by stage one do not, by themselves, require inventing another code defect or blocking acceptance of that limited protocol.

+**What must be verified against Stripe test mode before enabling**

+1. **Catalog identities:** all 36 prices, amounts, currencies, recurrence, metadata and own-only `currency_options`. Establish that provisioning output passes the console’s actual validation.

+2. **GBP purchase:** India plus explicit GBP, country defaults and held legacy EUR. Establish matching Checkout, subscription and item currency/amount, with fixed-book Adaptive Pricing disabled.

+3. **Event ordering and failed payment:** subscription-before-completion, abandoned 3-D Secure, declined-card retry and delayed methods. Establish whether honest buyers encounter attached dead/replacement subscriptions and therefore require an additional approved disposition.

+4. **Expiry racing payment:** retained URLs, concurrent expiry, lost answers and idempotent replays, followed by GET. Establish that proven expiry is terminal and unpaid, while a winning payment becomes completion rather than false expiry.

+5. **Identity and corrections:** GET/list/expiry response fields, metadata changes and delayed reads after correction. Establish the identity and freshness assumptions used by terminal proof.

+6. **Lost-create recovery:** creation succeeds but its answer is lost; recover by the original key and customer listing. Establish that recovery finds the same purchase without creating another payable session.

+7. **Metered invoices:** GBP—and controls in USD/EUR—through floor, rounding, draft recovery, item recovery, finalization and payment. Establish exact stored-currency amounts throughout.

+8. **Quantity and renewal:** changes, prorations, delayed/lost answers and renewal boundaries. Establish the stored price/currency and dating assumptions used by allowance history.

+9. **Legacy replay:** original EUR request shapes and existing idempotency keys. Establish unchanged legacy behavior and refusal of unexpected converted identities.

+10. **Financial settlement:** inspect payments, PaymentIntents, invoices, refunds and future collection after `incomplete_expired` and cancellation. Establish what remains owed before any new disposition policy is approved.

+**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 0b35d7df1..659f3a0db 100644

--- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md

+++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md

@@ -517,8 +517,8 @@ The gate on `2618403f` **failed** (`gate-currency-2618403f.txt`, exit 2): one ex

| --- | --- | --- | --- |

-| Astra 1 (P2): a dispute committed after the retirement's reservation and before the account's tombstone was published did not stop the publication; the deletion then refused at its last step, leaving an account retired in the deployment's manifest and present here, under a dispute stage one cannot resolve. | Fixed by a point of no return in the database: `commitBillingRetirement`, taken after the reservation and before the first write outside, re-reads every condition in its own transaction and marks the reservation committed. Filing a checkout refusal or a checkout dispute withdraws a reservation that is not committed, in the evidence's own transaction. SQLite serialises the two: either the evidence withdraws first and the commit refuses (nothing outside was touched; the reservation is released), or the commit is first and the evidence lands on an account whose retirement finishes (the deletion and the reap pass a review that opened after the commit). | `billingstore.go` `commitBillingRetirement`, `cancelPendingRetirementTx`, `fileBillingReviewTx`, `checkBillingRetirement`; `ownership.go` the deletion handler, `DeleteAccount`; `store.go` `ReapAccount` | `TestARetirementIsCommittedInTheDatabaseAndEvidenceSerialisesAgainstIt` (three cases) |

-| Opus P3-2: evidence first seen after an account was deleted landed where no operator looked (the listing left the account blank; the door answered 404). | Fixed: the operator doors look accounts up whether or not they are deleted; the listing marks such a row `account_deleted`. | `billingoperator.go`; `billingstore.go` `accountEvenIfDeleted` | the same test, second case |

+| Astra 1 (P2): a dispute committed after the retirement's reservation and before the account's tombstone was published did not stop the publication; the deletion then refused at its last step, leaving an account retired in the deployment's manifest and present here, under a dispute stage one cannot resolve. | Fixed by a point of no return in the database: `commitBillingRetirement`, taken after the reservation and before the first write outside, re-reads every condition in its own transaction and marks the reservation committed. Filing a checkout refusal or a checkout dispute withdraws a reservation that is not committed, in the evidence's own transaction. SQLite serialises the two: either the evidence withdraws first and the commit refuses (nothing outside was touched; the reservation is released), or the commit is first and the evidence lands on an account whose retirement finishes (the deletion and the reap pass a review that opened after the commit), listed under the deleted account and held, like every dispute. SQLite serialises the two for binaries that have the commit step; an older binary during a roll does not, which the roll rule covers. | `billingstore.go` `commitBillingRetirement`, `cancelPendingRetirementTx`, `fileBillingReviewTx`, `checkBillingRetirement`; `ownership.go` the deletion handler, `DeleteAccount`; `store.go` `ReapAccount` | `TestARetirementIsCommittedInTheDatabaseAndEvidenceSerialisesAgainstIt` (three cases) |

+| Opus P3-2: evidence first seen after an account was deleted landed where no operator looked (the listing left the account blank; the door answered 404). | Fixed: the operator doors look accounts up whether or not they are deleted; the listing marks such a row `account_deleted`; the door finds it and holds it (every such review is a dispute). | `billingoperator.go`; `billingstore.go` `accountEvenIfDeleted` | the same test, second case |

| Astra 2 (P3): one session on a listing page that did not decode discarded the page's other sessions, one of which could be a contradiction of an archived fate. | Fixed: the client decodes each listed session on its own and returns the ones that decoded with the decode error. | `billingstripe/catalog.go` | `TestListCheckouts_AMalformedSessionKeepsTheOthersOnItsPage` |

| Opus P3-1: the late-refusal dispute used a third key for an intent consumed with no session. | Fixed: the intent id, as the record and the classification. | `billingstore.go` `fileCheckoutRefusal` | |

| Opus P3-3: the recovery site's filter of the hold it caused had no test. | Tested. | | `TestThePassDoesNotFailOnAHoldItsRecoveryCaused` |

@@ -526,5 +526,38 @@ The gate on `2618403f` **failed** (`gate-currency-2618403f.txt`, exit 2): one ex

Mutation checks, in a tree copy, eleven, all die: checkout evidence not withdrawing a pending retirement; the commit not re-reading the conditions, and not needing a reservation (each needed its own test case, added); the deletion refusing a review opened after the commit, and ignoring reviews whether or not committed; the listing leaving a deleted account blank; the door answering 404 for one; a committed reservation withdrawn; a session that does not decode dropping its page; the recovery's hold the pass's failure; the late refusal of a session not kept. Output: `currency-mutants-round23.txt` in the run directory's logs.

-Stated and not changed: the reap of an account the manifest already records as retired passes an open checkout review (the review stays listed and resolvable under the deleted account); the structural guard's wrapper check is lexical; no test runs two console processes (the serialisation is tested through the store's transactions in sequence, which is what SQLite's one writer makes of two consoles).

+Stated and not changed: the reap of an account the manifest already records as retired passes an open checkout review (the review stays listed under the deleted account, held); the structural guard's wrapper check is lexical; no test runs two console processes (the serialisation is tested through the store's transactions in sequence, which is what SQLite's one writer makes of two consoles).

+## Review twenty-four (reviewed `e152df30`; Opus accepted with fixes, Astra rejected)

+Reports: `OPUS-REVIEW-multi-currency-review24.md`, `CODEX-FINDINGS-multi-currency-review24.md`. Opus: the point of no return answers round twenty-three's P2 for this binary, no P1 or P2, four P3s and two claims, and again "I would count review eighteen's P2 as closed". Astra: one P2 at the same boundary, one P3; "I would not yet close review eighteen's P2 as a whole". Both are right about the one thing: the committed marker was not durable against a losing request's cleanup. **Review eighteen's P2 stays recorded as open.** The gate on `e152df30` passed (22:54Z, alone).

+| --- | --- | --- | --- |

+| Astra 1 (P2), Opus P3-2: a request whose commit failed on something transient released the reservation unconditionally, which removed another request's committed one; a checkout could then open before the tombstone was written, and the deletion refused after it. The no-release-store branch released the same way. | Fixed: a failed commit releases only a pending reservation (`releasePendingBillingRetirement`); the one definite pre-write refusal (no bucket at all) is checked before the commit and releases only a pending one too; after the commit nothing releases. `releaseBillingRetirement` has no handler caller left. | `ownership.go` the deletion handler; `billingstore.go` | `TestARetirementIsCommittedInTheDatabaseAndEvidenceSerialisesAgainstIt`, "a losing request's cleanup does not touch a committed retirement" (the marker survives; a checkout is still refused) |

+| Opus P3-1: the handler's own call of the commit was untested; removing it restored the race with green tests. | Tested through the handler with a hook on the bucket's first write: the retirement is committed at that moment, a dispute filed then (by the production classification) lands on the account, the deletion finishes, and the review is listed under the deleted account. | `ownership.go` | `TestTheDeletionHandlerCommitsBeforeTheTombstone` |

+| Opus P3-3: a reservation from before the commit column read as pending, so evidence could withdraw one an earlier build had kept after an ambiguous failure outside. | Fixed: when the column is added, existing rows are marked committed at their start time, once. | `billingretirement.go` `migrateBillingRetirement` | `TestAReservationFromBeforeTheCommitColumnIsCommitted` |

+| Opus P3-4: a doc comment detached by an insertion. | Fixed: the lookups moved below the release functions, and the release's comment says who calls it (no handler). | `billingstore.go` | |

+| Opus claims 1 and 2, Astra's "resolvable": a deleted account's review is listed and held with no stage-one disposition, not resolvable; the serialisation holds for binaries with the commit step, and an older binary during a roll reproduces the race. | Corrected everywhere the words appeared (the design, the dispositions, two comments); the roll rule names the older binary's deletion and reap. | design, comments, `ENABLEMENT.md` on the launch branch | |

+Mutation checks, in a tree copy, five. Four die: a losing request's cleanup removing a committed retirement (the store function); the handler writing the tombstone before the commit; the rejected listing leaving a deleted account blank; an old reservation reading as pending. One survives, the handler's failed commit calling the unconditional release: the only way the handler's commit fails with a committed row present is a transient database error, which no test injects (the store refuses a commit over an open review before any row is committed, and answers nil for one already committed). The store-level test covers the release the handler now calls. Output: `currency-mutants-round24.txt` in the run directory's logs.

+Stated and not changed: no test runs two console processes (the losing request's cleanup is driven directly); an older binary has no commit step (the roll rule); a deleted account's dispute is held with no stage-one disposition, like every dispute.

+## Review twenty-five (reviewed `1f1368b0`; Opus accepted with fixes, Astra rejected)

+Reports: `OPUS-REVIEW-multi-currency-review25.md`, `CODEX-FINDINGS-multi-currency-review25.md`. Opus: round twenty-four's finding closed on every cleanup path, no P1 or P2, three P3s and two claims; "for this binary, review eighteen's P2 is closed", with two additions wanted before saying so without qualification (a two-connection test; the unconditional release out of production code). Astra: one P2 and one P3; the committed marker durable on every cleanup path for reservations correctly marked; the P2 is the migration. **Review eighteen's P2 stays recorded as open until a round ends with both accepting.** The gate on `1f1368b0` passed (23:33Z, alone) and CI passed.

+| --- | --- | --- | --- |

+| Astra 1 (P2), Opus P3-1: the column's addition and the marking of old reservations were two statements outside a transaction; a death between them, or a second console starting in the window, left an older build's kept reservation reading as pending, which evidence could then withdraw. | Fixed: the check, the `ALTER` and the `UPDATE` are one transaction. | `billingretirement.go` `migrateBillingRetirement` | `TestAReservationFromBeforeTheCommitColumnIsCommitted`: a trigger aborts the marking, the addition rolls back with it, and the retried migration marks the row |

+| Opus P3-3, Astra 2 (P3): the handler's choice of the pending-only release had no regression; a handler could call the unconditional release again with green tests. | The unconditional release is no longer production code: it lives in a `_test.go` file, for building states. A handler cannot call it, so the choice cannot be unmade without adding it back. Astra's deterministic ordering (A's commit fails because B's preflight removed the shared pending row; C reserves and commits; A cleans up) is tested at the store with a second connection: A's cleanup is the pending-only release and C's commit survives. A handler-level pause between a failed commit and its cleanup has no seam, and none is added. | `billingretirement_export_test.go`; `billingnamed_test.go` | `TestTwoConsolesAtTheRetirementsPointOfNoReturn` (three orderings on two connections to one file) |

+| Opus RRRR: sequential tests are not two connections. | Two connections now: a second `Store` opened on the fixture's file plays the other console for evidence before the commit, evidence after it, and a losing commit's cleanup. | | the same test |

+| Opus claims 1 and 2: the `tombstoneAccount` comment said the caller releases on `errNoReleaseStore`; the design said old reservations are marked "when the column is added" without the crash window. | Corrected; the roll rule on the launch branch names the exception for a database that ran `e152df30` (none did: billing is off and it was never rolled). | `ownership.go`, design, `ENABLEMENT.md` | |

+Stated and not changed: the two-connection test is two handles in one process on one WAL file, which is what two consoles are to SQLite, and not two processes; a deleted account's dispute is held with no stage-one disposition; the roll rule covers an older binary's deletion and reap.

diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review24.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review24.md

new file mode 100644

index 000000000..045e60b34

--- /dev/null

+++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review24.md

@@ -0,0 +1,110 @@

+**Verdict: accept with fixes.** The point of no return answers Astra's P2 for this binary. I found no P1 or P2. Four P3s and two claims remain, and none of them gates the round. On that basis I would count review eighteen's P2 as closed at `e152df30`. What remains is decision 15, the test-mode rehearsal, the roll rule and the owner's approvals.

+This was a source-only review with `git show e152df30:`. I modified nothing and ran nothing. The delta `2618403f..e152df30` touches no pricing, form, quote, cookie, trigger, export or site code, so the conclusions on A–K, L–ZZZ2 and the site at `01b62f7` stand as last reported.

+## Round twenty-three, finding by finding

+| Finding | Closed? | Would its test pass without the fix? |

+|---|---|---|

+| **Astra 1 (P2):** a dispute committed between the reservation and the tombstone let the tombstone publish | **Closed for this binary.** `commitBillingRetirement` (`billingstore.go:1559-1583`) re-reads every condition and marks the row committed. The handler calls it before `tombstoneAccount` (`ownership.go:160`). `fileBillingReviewTx` (`billingstore.go:957-961`) withdraws an uncommitted reservation in the evidence's own transaction. That function is the only writer of `billing_reviews` (one `INSERT … ON CONFLICT`), so no refusal or dispute can bypass it. | **No** for the store. Case 1 fails without the withdrawal, and case 3 fails without the `committed` bypass in `DeleteAccount`. **But the handler's own call at `ownership.go:160` is untested** (P3-1 below). |

+| **Astra 2 (P3):** one malformed session discarded its page | **Closed.** `catalog.go:153-193` decodes each element on its own and returns the ones that decoded with `CodeDecodeError`. | **No.** The old code failed inside `c.do` and returned an empty `out`. |

+| **Opus P3-1:** the third key | **Closed** (`billingstore.go:1269`: `intent.ID`). | No test. The state is unreachable, because the record needs a bound session. This is acceptable as a defensive fix. |

+| **Opus P3-2:** a deleted account's review was invisible | **Closed for visibility.** The listing (`billingoperator.go:205-209`) and the door (`:343`) use `accountEvenIfDeleted`. `ReapAccount` and `DeleteAccount` only mark `deleted_at` and never remove the row, so the lookup always finds it. | **No.** Case 3 asserts `account_deleted` and a 409 rather than a 404. The "resolvable" wording is a claim the code contradicts (claim 1). |

+| **Opus P3-3:** the recovery site's filter was untested | **Closed.** `TestThePassDoesNotFailOnAHoldItsRecoveryCaused` reaches `billingpass.go:297-301` through a lost create, an attached subscription and a listing recovery in EUR. | **No.** Without the filter the pass returns `errBillingUnderReview`. |

+| **Opus claims 1–4** | Corrected (`billingservice.go:790-796`, `billingretirement.go:970-974`, the dispositions' "Where"). The `ENABLEMENT.md` change is on the launch branch, which I did not read. | n/a |

+**The gate failure on `2618403f`.** `TestABoundSessionReadBackAsNobodysIsReviewed` was right before round twenty-two's P3-2 and wrong after it.

+- That fix records `sub_paid`, which names the intent under the other account's name, against the intent. The first hold is therefore the attachment.

+- The test now asserts both holds: the attachment, then the unproven expiry after the obligation is deleted by hand. It is right now.

+- The hand deletion makes a state that no code path produces, but it isolates the second hold honestly.

+- It would fail without P3-2, because the first `heldRetire` wants `"sub_paid"`.

+**Severity, from the code at `2618403f`.** Astra was right.

+- The handler went reserve → `tombstoneAccount` → `DeleteAccount`, and only the last of these re-read the reviews.

+- A dispute committed in between therefore left the manifest retired and the account live, under a reservation that blocks checkout and a dispute stage one cannot resolve. That is a P2.

+- My round-twenty-three acceptance missed it.

+## NNNN: does the point of no return answer it?

+I enumerated the orderings, given SQLite's one writer. A deferred transaction that read before another writer committed fails its write upgrade with BUSY rather than going on.

+| Ordering | Result |

+|---|---|

+| Evidence, then reserve | Reserve refuses. |

+| Reserve, then evidence, then commit | The evidence withdraws the reservation. The commit finds nothing reserved and refuses. The handler releases; nothing outside was touched. ✓ |

+| Reserve, then commit, then evidence, then tombstone, then delete | The evidence lands on an account whose retirement is committed. `DeleteAccount` passes it (`committed=true`) and the review is listed with `account_deleted`. ✓ |

+| Evidence written by an older binary (no withdrawal) before the commit | The commit re-reads the reviews and refuses (case 2). ✓ |

+| Death between the commit and the tombstone | The committed reservation stays. Checkout stays blocked and later evidence lands visibly on a live account. A retry finishes, because reserve with `committed=true` skips the review clause. ✓ |

+| Another condition after the commit (intent, month, active subscription) | Blocked by the reservation: `beginBillingCheckout` at `:363`, `freezeBillingMonth` at `:1400`. A first purchase needs a standing intent. ✓ |

+| An **older binary** deleting during a roll | It has no commit step, so Astra's race is reproduced by that binary. Its `ReapAccount` (at `2618403f`) also refuses an open review, leaving the account "tombstoned but live" on that console until it is upgraded. This is covered only by the roll rule; state it there. |

+**Is evidence after the commit handled honestly?** Yes, with one correction. After the commit no intent stands, and none can be created. Every review that can open is therefore a `checkout_fate_disputed`:

+- a subscription naming a consumed or retired intent goes through `classifySubscriptionTx`;

+- a late session refusal goes through the consumed-intent branch at `:1256-1272`.

+Such a review is listed and the door finds it, but it answers 409 "held", exactly as on a live account. The test itself asserts that. So it is "listed and held with no disposition", not "resolvable" (claim 1).

+**`ReapAccount` passing `true` unconditionally** (`store.go:1370`) is the right call. The manifest is authoritative, and refusing the reap would leave a retired account's memberships live here. The review stays visible, and the reap still refuses an active subscription, an intent or an open month.

+## Findings

+### P3-1. The production commit call is untested

+- **Where:** `ownership.go:160-170`.

+- **Scenario:** delete the `commitBillingRetirement` call from the handler. Every test still passes:

+ - the new test drives `reserveBillingRetirement` and `commitBillingRetirement` directly;

+ - the handler tests in `deletion_test.go` and `controlplane_test.go` have no review opening between reserve and tombstone.

+- **Effect:** the handler would then publish the tombstone and refuse in `DeleteAccount`. That is Astra's P2 restored with green tests.

+- **Fix:** add a handler test in which a hook in `tombstoneAccount` (or the release store) files a dispute through the production classification. Assert either that the deletion finishes with the review listed, or, with the dispute filed before the commit, that nothing is written to the bucket.

+### P3-2. A failed commit releases a committed reservation that is not its own

+- **Where:** `ownership.go:161-163`, together with `releaseBillingRetirement` (`billingstore.go:1640`), which deletes regardless of `committed_at`.

+- **Scenario:**

+ 1. Two deletion requests for one account arrive from two tabs, or two consoles on one database.

+ 2. Request A commits and is writing the tombstone.

+ 3. Request B's `commitBillingRetirement` fails on a transient error (BUSY, `Begin`) and calls release, which deletes A's committed row.

+ 4. A dispute then opens.

+ 5. A's `DeleteAccount` sees `committed=false` and refuses after the tombstone. That is the P2 state again, and checkout is also unbarred in the meantime.

+- **Fix:** release only on `errBillingRetirement`, or make that branch `DELETE … AND committed_at=0`.

+### P3-3. Reservations from before the column read as pending

+- **Where:** `billingretirement.go:176` (the default is 0).

+- **Scenario:**

+ 1. An older binary's tombstone failed ambiguously and the reservation was kept, as intended.

+ 2. After the upgrade, a dispute withdraws that reservation as pending.

+ 3. The account can now open a checkout or freeze a month although its manifest may be tombstoned.

+ 4. The later reap refuses on the new intent or month, and the account is tombstoned but live.

+- **Fix:** when the column is added, set `committed_at=started_at` on existing rows (only in the migration that adds it), or state it in the roll rule.

+### P3-4. A doc comment is detached

+- **Where:** `billingstore.go:1605-1613`. `accountEvenIfDeleted` was inserted between `releaseBillingRetirement`'s comment and its function.

+- **Effect:** godoc gives the new lookup the release's comment, and the release has none. The release comment's "called only when … no release store is configured" also omits the new commit-failure caller.

+- **Fix:** move the lookups below the release and add the commit-failure case to its comment.

+## Claims the code contradicts

+1. **"Listed and resolvable under the deleted account"** appears in the design's "Stage one as built", the dispositions (Astra 1 and Opus P3-2), `checkBillingRetirement`'s comment (`billingstore.go:1505-1513`), `ReapAccount`'s comment (`store.go:1366-1369`) and the dispositions' "Stated and not changed". Every review that can open after the commit is a dispute, and the door answers 409 to every dispute. It should read "listed, and held with no stage-one disposition".

+2. **"SQLite serialises the two"** (dispositions) is true only of binaries that commit. An older binary has no commit step and reproduces the race. The roll rule should name it.

+## OOOO: anything required before acceptance?

+No code is required. P3-1 is the one I would most want before merge, because without it the fix for the round's only P2 is unguarded at its production site. The rest, and the two claims, are fixes to make, not gates. What remains:

+- decision 15, with the predicate corrected last round (every invoice void or draft, no succeeded or processing charge, `incomplete_expired`);

+- the test-mode rehearsal and its first-check gate;

+- the roll rule covering older binaries' deletion and reap;

+- the owner's approvals.

+## Verify in Stripe test mode before enabling

+Last round's six items are unchanged:

+1. Whether Checkout creates a Subscription before the session completes: with 3-D Secure abandoned, a declined card then another, and a delayed debit.

+2. What an `incomplete_expired` subscription leaves: its invoices, charges and PaymentIntents.

+3. SEPA or Bacs through a failed debit.

+4. The order of `customer.subscription.created` against `checkout.session.completed`.

+5. Whether a session's or a subscription's metadata can be edited after creation, and by whom.

+6. What can change on a canceled subscription.

+Plus Astra's ten-item list: the catalog identities, the GBP one-form purchase, expiry racing payment, idempotency, lost-create recovery, GBP overages and quantity, legacy EUR reuse.

+Nothing in this delta adds a provider dependency. The per-element listing decode rests on the response shape, which item 7 of that list (lost-create recovery) already exercises.

+**Verdict: accept with fixes** (P3-1 to P3-4 and the two claims; decision 15, the rehearsal gate and the roll rule before enabling).

diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review25.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review25.md

new file mode 100644

index 000000000..edb6d9a28

--- /dev/null

+++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review25.md

@@ -0,0 +1,142 @@

+**Verdict: accept with fixes.** I found no P1 or P2. Both reviewers' round-24 finding is closed: a losing request's cleanup can no longer remove a committed retirement on any of the handler's three cleanup paths. Three P3s and two stale claims remain, and none of them gates the round.

+On review eighteen's P2: nothing in this branch's code stands in the way of closing it for this binary. Below, under RRRR, I name the two small additions that would let me close it without qualification.

+This was a source-only review, using `git show 1f1368b0:` throughout. I modified nothing and ran nothing. The delta `e152df30..1f1368b0` touches only these files: `ownership.go`, `billingstore.go`, `billingretirement.go`, `billingoperator.go`, `store.go`, `billingnamed_test.go`, and the docs. It touches no pricing, form, quote, cookie, trigger, export or site code. My conclusions on A–K, L–ZZZ2, AAAA–OOOO and the site at `01b62f7` therefore stand as reported at `e152df30`.

+## Round twenty-four, finding by finding

+| Finding | Closed? | Would its test pass without the fix? |

+|---|---|---|

+| **Astra 1 (P2) / Opus P3-2:** a failed commit released a committed row | **Closed.** The handler's commit-failure branch (`ownership.go:176`) and the no-bucket branch (`:162`) both call `releasePendingBillingRetirement` (`billingstore.go:1620`, `AND committed_at=0`). Once the commit has happened, nothing releases. A grep finds no non-test caller of `releaseBillingRetirement`. | **The store test fails without the fix:** with an unconditional delete, the marker goes and `beginBillingCheckout` succeeds. **The handler's choice is untested.** Putting `releaseBillingRetirement` back at `:176` is the surviving mutant (see QQQQ). |

+| **Opus P3-1:** the handler's commit call was untested | **Closed.** `TestTheDeletionHandlerCommitsBeforeTheTombstone` hooks the first `Put` on the bucket. `TombstoneAccount` (`domains/mint.go:254`) writes only through `Put`. | **It fails without the fix.** Remove the commit and `committedAtWrite` is false. Move the commit after the tombstone and the same assertion fails. Drop the `committed` bypass and `DeleteAccount` refuses, so "the account survived" fails. The test also fails if the hook never runs. |

+| **Opus P3-3:** reservations from before the column read as pending | **Closed on a database that never ran `e152df30`**, apart from a crash window (P3-1 below). | **It fails without the fix:** `committed_at` stays 0 and the first assertion fails. |

+| **Opus P3-4:** a detached doc comment | **Closed.** The release functions now come before the lookups, and the comment names its callers. | n/a |

+| **Opus claims 1–2:** "resolvable"; "SQLite serialises" | **Corrected** in the design (`:128`), the dispositions, `checkBillingRetirement`, `ReapAccount` and the test comment. | n/a |

+| **Astra 2 (P3):** the rejected listing left a deleted account blank | **Closed** (`billingoperator.go:314`, `accountEvenIfDeleted`). | **It fails without the fix.** The last assertion of the handler test wants `account == sqid` after the deletion. |

+**Severity, from the code at `e152df30`: Astra's P2 was right.**

+- Within one console, a commit could not fail with a committed row present except on an I/O fault. `SetMaxOpenConns(1)` (`store.go:65`) serialises transactions, and `commitBillingRetirement` returns nil when it sees a committed row. In that case P3 would have been fair.

+- Across two consoles on one WAL database, the failure is deterministic, not a random fault:

+ 1. B's deferred transaction reads.

+ 2. A commits.

+ 3. B's write upgrade returns `SQLITE_BUSY_SNAPSHOT`, which `busy_timeout` does not retry.

+ 4. B's unconditional release deletes A's committed row.

+- At `e152df30`, the `errNoReleaseStore` branch also came after the commit. A console with no bucket deleted the committed row with no contention at all.

+- The protocol explicitly claims two consoles on one database, so the deletion handler reproduced the P2 state.

+**Is the committed marker durable now?** Yes, on every cleanup path the handler has:

+| Cleanup path | What happens now |

+|---|---|

+| A failed commit | Releases only a pending row (`:176`). |

+| No bucket | Checked before the commit (`:159-168`), and releases only a pending row. |

+| A tombstone that fails after the commit | Releases nothing (`:198-209`). |

+The only other writer that deletes from `billing_retirements` is `cancelPendingRetirementTx`, which is also pending-only.

+**Can any handler still release a reservation it cannot know is its own?** Yes, but only a pending one, and that is safe. Reservations share one row per account (`INSERT OR IGNORE`), so "its own" has no meaning for a pending row. If request B's cleanup removes request A's pending row:

+- A's commit then reads `!reserved` and refuses with `errBillingRetirement`.

+- It does so before anything outside the database is written.

+- The commit re-reads every condition in one transaction, so a row re-made by a third request is judged afresh.

+The only cost is liveness: A's user sees "Could not delete this account".

+## Findings

+### P3-1. The column migration is not atomic

+- **Where:** `billingretirement.go:184-197`. The `ALTER` and the `UPDATE` are two separate `Exec`s, outside the transaction that follows.

+- **Scenario 1 (crash):**

+ 1. The process dies after the `ALTER` and before the `UPDATE`.

+ 2. At the next start, `hasCommitted=1`, so the `UPDATE` never runs.

+ 3. An older build's kept reservation now reads as pending.

+ 4. Evidence withdraws it and checkout reopens on a possibly tombstoned account. That is exactly P3-3.

+- **Scenario 2 (two consoles starting together):** console B finishes its migration and serves a deletion's pending reservation in the window between A's `ALTER` and A's `UPDATE`. A's `UPDATE` then marks B's pending row committed. The window is microseconds.

+- **Scenario 3 (the `e152df30` gap):** a database that ran `e152df30` already has the column with no backfill, so the backfill never runs. Billing is off and `e152df30` was never rolled, so this only needs saying in the roll rule.

+- **Fix:** run the `pragma_table_info` check, the `ALTER` and the `UPDATE` in one transaction (SQLite DDL is transactional), and state the `e152df30` exception in the roll rule.

+### P3-2. The no-bucket preflight's release is untested

+- **Where:** `ownership.go:159-168`, with `TestDeletionRefusedWhenTheBucketIsUnreachable` (`deletion_test.go:118`).

+- **Scenario:** move the `Releases == nil` check back after the commit, or delete the release call. The test still passes, because it asserts only that the account survives.

+- **Effect:** the live account keeps a committed reservation. Checkout is blocked at `billingstore.go:363` for good, and no retry on that console can finish.

+- **Fix:** assert in that test that no reservation row remains, and that a checkout `beginBillingCheckout` call afterwards is not refused.

+### P3-3. The surviving mutant can be ruled out at compile time

+- **Where:** `billingstore.go:1605-1614`.

+- **Scenario:** `releaseBillingRetirement` stays in production code "for tests and for an operator's repair by hand". There is no operator door for that repair. AGENTS.md's API/CLI rule makes that a repair by direct database access, not a supported path. A future edit, or the round's surviving mutant, can call it from a handler with green tests.

+- **Fix:** move it into a `_test.go` file. The mutant then cannot compile.

+## Claims the code contradicts

+1. **`ownership.go:614-624`.** The comment on `tombstoneAccount` and `errNoReleaseStore` still says the caller tells this refusal apart "so … release the retirement reservation". The handler now checks for it before the commit. If `tombstoneAccount` ever returned it after the commit, the handler would keep the committed row, which is the right thing but the opposite of what the comment says.

+2. **The P3-3 disposition and the design (`:128`)** say "a reservation from before the commit column is marked committed when the column is added". That holds only on a database that never ran `e152df30`, and only if the process does not die between the two statements (P3-1).

+The QQQQ wording is addressed below.

+## PPPP: the deletion handler, step by step

+|---|---|---|---|

+| Preparation (`resolveIntentForDeletion`) | Recorded under the protocol as usual. The reservation then refuses on the open review or the intent. | Each runs its own preparation through the protocol. | Nothing is reserved and nothing outside is touched. |

+| Reserve (`:151`) | Lands after: `fileBillingReviewTx` withdraws the pending row in the same transaction, then the commit refuses. | Shares the row (`INSERT OR IGNORE`). | A pending row stays. Checkout (`:363`) and month freezes (`:1400`) are blocked until evidence withdraws it or a retry commits. The owner is not told why. This is the same as in older builds, not new. |

+| Commit (`:172`) | Evidence first: the commit refuses and nothing outside is written. Commit first: the evidence becomes a dispute that `DeleteAccount` and `ReapAccount` pass, listed as `account_deleted` and held with a 409. | The second request finds the row committed and gets nil, then tombstones (idempotent: `mint.go:245`). Its `DeleteAccount` may fail cosmetically after the first succeeds. | The committed row stays and the account is live. Checkout is blocked. Evidence lands as a dispute on a live account, visible and held. A retry finishes the deletion: reserve passes the review clause, the commit returns nil, then tombstone and delete. |

+| Tombstone (`:198`) | As after the commit. | Idempotent, with one compare-and-swap retry. | The manifest is retired and the row is committed. The next materialize runs `ReapAccount`, which passes reviews. After the commit no intent, open month or active subscription can arise, so the reap finishes. |

+In every state, a later retry or reap finishes the deletion. Nothing between the commit and the local delete reopens checkout.

+## QQQQ: the surviving mutant

+**True, with one correction of wording.**

+- Within one console, the commit cannot fail with a committed row present except on a database fault, for the serialisation reason above.

+- Across two consoles, "a transient database error" is the deterministic `SQLITE_BUSY_SNAPSHOT` of the exact race Astra described. It is not a random fault.

+**Acceptable**, because the handler's code is right by inspection and the store function it calls is tested. Either P3-3's move, or a test with two `*Store` handles opened on one WAL file, would kill the mutant. The two-handle test does not need two processes. Its order:

+1. B begins, then reads.

+2. A commits.

+3. B's update fails.

+4. Run the handler's cleanup.

+5. Assert the marker survives.

+## RRRR: what would make me say yes

+**For this binary, yes: review eighteen's P2 is closed.** No interleaving I can construct from the code deletes an intent while its session is payable or a subscription is outstanding. None releases a hold with money outstanding, or tombstones an account whose local deletion then refuses.

+To close it without qualification, I would want these two additions:

+- **Sequential tests are not two connections.** Add the two-handle WAL test above for the three orderings:

+ 1. reserve, then evidence, then commit;

+ 2. commit, then evidence, then delete;

+ 3. a losing commit's cleanup.

+- **The mutant is alive.** Make P3-3's move, so the unconditional release cannot be called from a handler.

+What stands outside the branch:

+- **The roll rule:** an older binary has no commit step and its release is unconditional, so older consoles must be stopped before this build starts and never rolled back.

+- **Stripe test-mode item 1:** whether a subscription can exist before its session completes. This decides how often honest buyers meet the hold, but not whether the hold is safe.

+- **Stage two:** a disposition for disputes. They are held by design.

+## OOOO: required before acceptance?

+No code is required. P3-1 to P3-3 and the two claims are fixes to make, not gates. What remains:

+- decision 15;

+- the test-mode rehearsal and its first-check gate;

+- the roll rule, now also naming the `e152df30` backfill exception;

+- the owner's approvals.

+## What to verify in Stripe test mode before enabling

+This delta adds no provider dependency. The list is the one from round twenty-four, unchanged:

+1. **Subscription before completion:** whether Checkout creates a Subscription before the session completes, under abandoned 3-D Secure, a declined card followed by another, and a delayed debit.

+2. **`incomplete_expired` remains:** the invoices, charges and PaymentIntents such a subscription leaves.

+3. **Failed debits:** SEPA and Bacs through a failed debit.

+4. **Event order:** `customer.subscription.created` against `checkout.session.completed`.

+5. **Metadata edits:** whether a session's or a subscription's metadata can be edited after creation, and by whom.

+6. **Canceled subscriptions:** what can still change on one.

+7. **Catalog identities:** all 36 prices with exact currencies, own-only `currency_options`, and Adaptive Pricing disabled.

+8. **The GBP purchase:** the one-form purchase at `?country=IN&currency=gbp` from end to end.

+9. **Expiry against payment:** expiry racing a payment, with idempotency keys and the fresh-read proof.

+10. **Lost-create recovery:** recovery by listing, including pagination and the per-element decode.

+11. **GBP overages and quantity:** invoices, their recovery, and proration.

+12. **Legacy EUR:** request hashes and customer reuse unchanged.

+**Verdict: accept with fixes.** The fixes are P3-1 to P3-3 and the two claims. Before enabling billing: decision 15, the rehearsal gate and the roll rule.

diff --git a/internal/console/billingnamed_test.go b/internal/console/billingnamed_test.go

index 4707e90ac..bb2f9c31d 100644

--- a/internal/console/billingnamed_test.go

+++ b/internal/console/billingnamed_test.go

@@ -6,11 +6,15 @@ import (

"errors"

"fmt"

"net/http"

+ "net/url"

"strings"

+ "sync"

"testing"

"time"

+ "github.com/carlosframework/platform/internal/domains"

"github.com/carlosframework/platform/internal/plans"

+ "github.com/carlosframework/platform/internal/release"

)

// Review twenty-one's counterexamples. One rule answers them: a

@@ -960,7 +964,7 @@ func TestADisputeAboutAConsumedIntentWithNoSessionIsKeyedByTheIntent(t *testing.

// anything outside it is written. Checkout evidence that lands before it

// withdraws the pending retirement, and the commit then refuses; evidence

// that lands after it stays with the account, which is deleted, and is

-// listed and resolvable under the deleted account. (Review twenty-three,

+// listed under the deleted account, held like every dispute. (Review twenty-three,

// Astra 1; Opus P3-2.)

func TestARetirementIsCommittedInTheDatabaseAndEvidenceSerialisesAgainstIt(t *testing.T) {

// On an account that can be deleted (the fixture's own is the ops

@@ -1120,6 +1124,30 @@ func TestARetirementIsCommittedInTheDatabaseAndEvidenceSerialisesAgainstIt(t *te

t.Fatalf("the door for the deleted account's dispute: %d %v; want it found and held, not 404", status, answer)

}

})

+ t.Run("a losing request's cleanup does not touch a committed retirement", func(t *testing.T) {

+ fx, f, acct, _ := ended(t)

+ // Request A reserves and commits and is writing the tombstone.

+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ // Request B's commit failed on something transient, and it cleans

+ // up: only a pending reservation goes.

+ if err := fx.st.releasePendingBillingRetirement(acct.ID); err != nil {

+ t.Fatal(err)

+ }

+ if r, c := reserved(t, fx, acct); !r || !c {

+ t.Fatalf("after the losing request's cleanup: reserved %v committed %v; want A's commit kept", r, c)

+ }

+ // And no checkout opens meanwhile.

+ state, _ := fx.st.billingAccount(acct.ID)

+ request := billingIntent{AccountID: acct.ID, Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, PriceBook: plans.CurrentPriceBook, Currency: "gbp", Band: "D", Price: currencyPriceID(plans.PlanPersonal, "D", plans.GBP), Quantity: 1, Customer: state.Customer, Country: "GB", Members: 1}

+ if _, err := fx.st.beginBillingCheckout(request, f.now.Add(3*time.Hour)); !errors.Is(err, errBillingConflict) {

+ t.Fatalf("a checkout under a committed retirement: %v; want it refused", err)

+ }

+ })

t.Run("a review of another kind withdraws nothing", func(t *testing.T) {

fx, f, acct, _ := ended(t)

if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {

@@ -1177,3 +1205,308 @@ func TestThePassDoesNotFailOnAHoldItsRecoveryCaused(t *testing.T) {

t.Fatalf("state %+v, session %s; want the refusal filed naming the recovered session, which was expired", st, f.sessionStatus)

}

+// hookedReleases is the deployment bucket with a hook on its first write:

+// what happens on another console while this one is between its commit

+// and its tombstone.

+type hookedReleases struct {

+ release.Store

+ once sync.Once

+ onPut func()

+func (h *hookedReleases) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) {

+ h.once.Do(h.onPut)

+ return h.Store.Put(ctx, key, body, ifMatch)

+// The deletion handler commits the retirement before it writes the

+// tombstone, and a dispute that lands between the two does not stop the

+// deletion: the account is deleted, the review is listed under it. (Review

+// twenty-four, Opus P3-1: the production call site of the point of no

+// return.)

+func TestTheDeletionHandlerCommitsBeforeTheTombstone(t *testing.T) {

+ fx, f, _ := newCurrencyFixture(t)

+ c := newBrowser(t)

+ signUp(t, fx, c, "code-delete-billing")

+ _, sqid, err := domains.MintAccount(context.Background(), fx.rel, "Testco", "paul@keymail.dev", time.Now())

+ if err != nil {

+ t.Fatal(err)

+ }

+ if n, err := MaterializeAccounts(context.Background(), fx.rel, fx.st); err != nil || n != 1 {

+ t.Fatalf("materialize = %d %v, want 1", n, err)

+ }

+ acct, _ := fx.st.AccountBySqid(sqid)

+ csrf := sessionToken(t, fx, c)

+ // The account buys, and the purchase ends.

+ f.customerCountry = "GB"

+ f.mu.Lock()

+ f.sqid = sqid

+ f.mu.Unlock()

+ 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

+ f.otherSubscriptions = map[string]bool{"sub_hand": true}

+ f.mu.Unlock()

+ if code := f.webhookPost(t, fx, "evt_own_testco", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent {

+ t.Fatalf("the purchase: %d", code)

+ }

+ f.mu.Lock()

+ f.subscriptionStatus = "canceled"

+ f.mu.Unlock()

+ if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now.Add(time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ // On the first write to the bucket: the retirement must already be

+ // committed, and a subscription made by hand naming the consumed

+ // intent is read by another console.

+ var committedAtWrite bool

+ fx.srv.cfg.Releases = &hookedReleases{Store: fx.rel, onPut: func() {

+ var at int64

+ if err := fx.st.db.QueryRow(`SELECT committed_at FROM billing_retirements WHERE account_id=?`, acct.ID).Scan(&at); err == nil && at != 0 {

+ committedAtWrite = true

+ }

+ if code := f.webhookPost(t, fx, "evt_hand_mid_deletion", "customer.subscription.created", "sub_hand"); code != http.StatusNoContent {

+ t.Errorf("the hand-made subscription during the deletion: %d", code)

+ }

+ }}

+ status, _, body := postGated(t, fx, c, fx.ts.URL+"/a/"+sqid+"/delete", url.Values{"csrf": {csrf}, "confirm": {"Testco"}})

+ if status != http.StatusSeeOther {

+ t.Fatalf("delete = %d:\n%s", status, body)

+ }

+ if !committedAtWrite {

+ t.Fatal("the tombstone was written before the retirement was committed")

+ }

+ if a, _ := fx.st.AccountBySqid(sqid); a != nil {

+ t.Fatal("the account survived its deletion")

+ }

+ if n := disputesOf(t, fx, acct, intent.SessionID, "%"); n != 1 {

+ t.Fatalf("%d disputes; want the evidence that landed mid-deletion recorded", n)

+ }

+ token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-mid-deletion")

+ _, listing := getJSON(t, fx.ts.URL+"/api/billing/reviews", token)

+ rows, _ := listing["reviews"].([]any)

+ found := false

+ for _, r := range rows {

+ row, _ := r.(map[string]any)

+ if row["reason"] == billingFateDisputedReason && row["account"] == sqid && row["account_deleted"] == true {

+ found = true

+ }

+ if !found {

+ t.Fatalf("the deleted account's dispute is not listed: %v", listing)

+ }

+ // A rejected event recorded with the account keeps the account's

+ // name in the operator's list after the deletion.

+ _, rejected := getJSON(t, fx.ts.URL+"/api/billing/rejected", token)

+ rows, _ = rejected["rejected"].([]any)

+ named := false

+ for _, r := range rows {

+ row, _ := r.(map[string]any)

+ if row["id"] == "evt_hand_mid_deletion" && row["account"] == sqid {

+ named = true

+ }

+ if !named {

+ t.Fatalf("the deleted account's rejected event is not listed under it: %v", rejected)

+ }

+// A reservation from before the commit column exists was kept by an

+// earlier build after an ambiguous failure outside, with the tombstone

+// possibly written: it is marked committed once, when the column is added,

+// so evidence does not withdraw it and a checkout does not open on a

+// tombstoned account. A reservation made after the column exists is

+// pending until its commit. (Review twenty-four, Opus P3-3.)

+func TestAReservationFromBeforeTheCommitColumnIsCommitted(t *testing.T) {

+ fx, f, acct := newCurrencyFixture(t)

+ for _, q := range []string{

+ `DROP TRIGGER IF EXISTS billing_refused_intent_delete_barrier`,

+ `ALTER TABLE billing_retirements DROP COLUMN committed_at`,

+ } {

+ if _, err := fx.st.db.Exec(q); err != nil {

+ t.Fatal(err)

+ }

+ if _, err := fx.st.db.Exec(`INSERT INTO billing_retirements(account_id,started_at) VALUES(?,?)`, acct.ID, f.now.Unix()); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.migrateBillingRetirement(); err != nil {

+ t.Fatal(err)

+ }

+ var committed int64

+ if err := fx.st.db.QueryRow(`SELECT committed_at FROM billing_retirements WHERE account_id=?`, acct.ID).Scan(&committed); err != nil || committed != f.now.Unix() {

+ t.Fatalf("the old reservation after the column was added: committed_at %d %v; want its start time", committed, err)

+ }

+ // The column and the marking are one transaction: a marking that

+ // fails (a trigger here; a death between the two statements in life)

+ // leaves no column behind to make the next start skip the marking.

+ fx2, f2, acct2 := newCurrencyFixture(t)

+ for _, q := range []string{

+ `DROP TRIGGER IF EXISTS billing_refused_intent_delete_barrier`,

+ `ALTER TABLE billing_retirements DROP COLUMN committed_at`,

+ `CREATE TRIGGER test_no_backfill BEFORE UPDATE ON billing_retirements BEGIN SELECT RAISE(ABORT, 'test_no_backfill'); END`,

+ } {

+ if _, err := fx2.st.db.Exec(q); err != nil {

+ t.Fatal(err)

+ }

+ if _, err := fx2.st.db.Exec(`INSERT INTO billing_retirements(account_id,started_at) VALUES(?,?)`, acct2.ID, f2.now.Unix()); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx2.st.migrateBillingRetirement(); err == nil || !strings.Contains(err.Error(), "test_no_backfill") {

+ t.Fatalf("the migration whose marking fails: %v; want it to fail", err)

+ }

+ var hasColumn int

+ if err := fx2.st.db.QueryRow(`SELECT count(*) FROM pragma_table_info('billing_retirements') WHERE name='committed_at'`).Scan(&hasColumn); err != nil || hasColumn != 0 {

+ t.Fatalf("after the failed marking the column exists (%d, %v); want the addition rolled back with it", hasColumn, err)

+ }

+ if _, err := fx2.st.db.Exec(`DROP TRIGGER test_no_backfill`); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx2.st.migrateBillingRetirement(); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx2.st.db.QueryRow(`SELECT committed_at FROM billing_retirements WHERE account_id=?`, acct2.ID).Scan(&committed); err != nil || committed != f2.now.Unix() {

+ t.Fatalf("the old reservation after the retried migration: committed_at %d %v; want its start time", committed, err)

+ }

+ // Run again, it changes nothing; and a new reservation is pending.

+ if err := fx.st.migrateBillingRetirement(); err != nil {

+ t.Fatal(err)

+ }

+ if _, err := fx.st.db.Exec(`INSERT INTO billing_retirements(account_id,started_at) VALUES(?,?)`, acct.ID+1, f.now.Unix()); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.migrateBillingRetirement(); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.db.QueryRow(`SELECT committed_at FROM billing_retirements WHERE account_id=?`, acct.ID+1).Scan(&committed); err != nil || committed != 0 {

+ t.Fatalf("a new reservation after a later start: committed_at %d %v; want pending", committed, err)

+ }

+// Two consoles are two connections to one database, not two steps of one.

+// A second store handle on the fixture's file plays the other console for

+// the three orderings at the retirement's point of no return: evidence

+// before the commit withdraws it; evidence after the commit lands on the

+// retiring account; a losing request's commit fails under the winner's

+// and its cleanup leaves the winner's committed row. (Review twenty-five,

+// Opus RRRR.)

+func TestTwoConsolesAtTheRetirementsPointOfNoReturn(t *testing.T) {

+ second := func(t *testing.T, fx *consoleFixture) *Store {

+ t.Helper()

+ var file string

+ if err := fx.st.db.QueryRow(`SELECT file FROM pragma_database_list WHERE name='main'`).Scan(&file); err != nil || file == "" {

+ t.Fatalf("the fixture's database file: %q %v", file, err)

+ }

+ other, err := Open(file)

+ if err != nil {

+ t.Fatal(err)

+ }

+ t.Cleanup(func() { other.Close() })

+ return other

+ }

+ ended := func(t *testing.T) (*consoleFixture, *fakeBillingStripe, *Account, billingIntent) {

+ t.Helper()

+ fx, f, ops := newCurrencyFixture(t)

+ if err := fx.st.CreateAccount(ops.ID+100, "cdef", "Other", "paul@keymail.dev", time.Now()); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.AddMember(ops.ID+100, "paul@keymail.dev", RoleOwner, time.Now()); err != nil && !strings.Contains(err.Error(), "already") {

+ t.Fatal(err)

+ }

+ acct, _ := fx.st.AccountBySqid("cdef")

+ f.customerCountry = "GB"

+ f.mu.Lock()

+ f.sqid = "cdef"

+ f.mu.Unlock()

+ 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

+ f.mu.Unlock()

+ if code := f.webhookPost(t, fx, "evt_own_two", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent {

+ t.Fatalf("the purchase: %d", code)

+ }

+ f.mu.Lock()

+ f.subscriptionStatus = "canceled"

+ f.mu.Unlock()

+ if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now.Add(time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ return fx, f, acct, intent

+ }

+ t.Run("evidence on the other console before the commit", func(t *testing.T) {

+ fx, f, acct, intent := ended(t)

+ other := second(t, fx)

+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ if err := other.noteSubscriptionSeen("cdef", intent.ID, "sub_hand", f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); !errors.Is(err, errBillingRetirement) {

+ t.Fatalf("the commit after the other console's evidence: %v; want it refused", err)

+ }

+ })

+ t.Run("evidence on the other console after the commit", func(t *testing.T) {

+ fx, f, acct, intent := ended(t)

+ other := second(t, fx)

+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ if err := other.noteSubscriptionSeen("cdef", intent.ID, "sub_hand", f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ if n := disputesOf(t, fx, acct, intent.SessionID, "%"); n != 1 {

+ t.Fatalf("%d disputes; want the other console's evidence recorded", n)

+ }

+ if err := fx.st.DeleteAccount(acct.ID, f.now.Add(3*time.Hour)); err != nil {

+ t.Fatalf("the deletion after the commit: %v", err)

+ }

+ })

+ t.Run("a losing commit on the other console and its cleanup", func(t *testing.T) {

+ fx, f, acct, _ := ended(t)

+ other := second(t, fx)

+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ // The other console's commit has read the reservation and is

+ // about to write; this console commits first; the other's write

+ // fails on its stale snapshot, and its cleanup runs.

+ tx, err := other.db.Begin()

+ if err != nil {

+ t.Fatal(err)

+ }

+ var reserved bool

+ if err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_retirements WHERE account_id=?)`, acct.ID).Scan(&reserved); err != nil || !reserved {

+ t.Fatalf("the other console's read: %v %v", reserved, err)

+ }

+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ _, werr := tx.Exec(`UPDATE billing_retirements SET committed_at=? WHERE account_id=?`, f.now.Add(2*time.Hour).Unix(), acct.ID)

+ tx.Rollback()

+ if werr == nil {

+ // Some SQLite builds let the stale writer through once the

+ // first writer is done; then the row is committed either

+ // way, and the cleanup below is what the test is about.

+ t.Log("the other console's write went through after the first commit")

+ }

+ if err := other.releasePendingBillingRetirement(acct.ID); err != nil {

+ t.Fatal(err)

+ }

+ var committed int64

+ if err := fx.st.db.QueryRow(`SELECT committed_at FROM billing_retirements WHERE account_id=?`, acct.ID).Scan(&committed); err != nil || committed == 0 {

+ t.Fatalf("after the losing console's cleanup: committed_at %d %v; want the winner's commit kept", committed, err)

+ }

+ })

diff --git a/internal/console/billingoperator.go b/internal/console/billingoperator.go

index 32c3fd741..cac6449fc 100644

--- a/internal/console/billingoperator.go

+++ b/internal/console/billingoperator.go

@@ -311,7 +311,9 @@ func (s *Server) handleAPIBillingRejected(w http.ResponseWriter, r *http.Request

}

for i, id := range accountIDs {

if id != 0 {

- if acct, err := s.store.AccountByID(id); err == nil && acct != nil {

+ // Whether or not the account is deleted since: the event was

+ // recorded with it.

+ if acct, _, err := s.store.accountEvenIfDeleted(id); err == nil && acct != nil {

out[i].Account = acct.Sqid

}

diff --git a/internal/console/billingretirement.go b/internal/console/billingretirement.go

index 3ff6aa1f2..9bac4901e 100644

--- a/internal/console/billingretirement.go

+++ b/internal/console/billingretirement.go

@@ -169,16 +169,47 @@ func (s *Store) migrateBillingRetirement() error {

// another customer's is seen as the contradiction it is.

{"billing_retired_intents", "customer", `TEXT NOT NULL DEFAULT ''`},

{"billing_retired_intents", "session_fate", `TEXT NOT NULL DEFAULT ''`},

- // An account's retirement is committed in the database before

- // anything outside it is written (commitBillingRetirement): until

- // then checkout evidence cancels it, and from then on evidence

- // lands on an account that is being retired and stays visible.

- {"billing_retirements", "committed_at", `INTEGER NOT NULL DEFAULT 0`},

} {

if err := s.addColumnIfMissing(c[0], c[1], c[2]); err != nil {

return err

}

+ // An account's retirement is committed in the database before anything

+ // outside it is written (commitBillingRetirement): until then checkout

+ // evidence cancels it, and from then on evidence lands on an account

+ // that is being retired and stays visible. A reservation from before

+ // the column exists was kept by an earlier build after an ambiguous

+ // failure outside, with the tombstone possibly written: it is a

+ // committed one, and is marked so once, when the column is added.

+ // The check, the column and the marking are one transaction (SQLite's

+ // DDL is transactional): a death between the column and the marking,

+ // or a second console starting in that window, cannot leave an old

+ // reservation reading as pending.

+ if err := func() error {

+ tx, err := s.db.Begin()

+ if err != nil {

+ return err

+ }

+ defer tx.Rollback()

+ var hasCommitted int

+ if err := tx.QueryRow(`SELECT count(*) FROM pragma_table_info('billing_retirements') WHERE name='committed_at'`).Scan(&hasCommitted); err != nil {

+ return err

+ }

+ if hasCommitted != 0 {

+ return nil

+ }

+ for _, stmt := range []string{

+ `ALTER TABLE billing_retirements ADD COLUMN committed_at INTEGER NOT NULL DEFAULT 0`,

+ `UPDATE billing_retirements SET committed_at=started_at WHERE committed_at=0`,

+ } {

+ if _, err := tx.Exec(stmt); err != nil {

+ return err

+ }

+ return tx.Commit()

+ }(); err != nil {

+ return err

+ }

tx, err := s.db.Begin()

if err != nil {

return err

diff --git a/internal/console/billingretirement_export_test.go b/internal/console/billingretirement_export_test.go

new file mode 100644

index 000000000..0bfdc2f95

--- /dev/null

+++ b/internal/console/billingretirement_export_test.go

@@ -0,0 +1,11 @@

+package console

+// releaseBillingRetirement removes a reservation, committed or not. It is

+// a test's tool for building a state: no handler may call it, because a

+// request can never know that a committed reservation is its own and not

+// another request's whose tombstone is being written, and in production

+// code it does not exist.

+func (s *Store) releaseBillingRetirement(accountID int64) error {

+ _, err := s.db.Exec(`DELETE FROM billing_retirements WHERE account_id=?`, accountID)

+ return err

diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go

index 5891150ad..52a6e113d 100644

--- a/internal/console/billingstore.go

+++ b/internal/console/billingstore.go

@@ -1508,9 +1508,9 @@ func advanceBillingMonthTx(tx *sql.Tx, x billingMonthRecord, state, invoiceID, i

// retirement is reserved and when it is committed; once committed (the

// point of no return, in this database, before anything outside it is

// written) a review that opens afterwards does not stop the deletion

-// finishing, and is listed and resolvable under the deleted account

-// instead (the operator doors look accounts up whether or not they are

-// deleted).

+// finishing, and is listed under the deleted account instead, held with no

+// stage-one disposition like every dispute (the operator doors look

+// accounts up whether or not they are deleted).

func checkBillingRetirement(tx *sql.Tx, accountID int64, committed bool) error {

if held, err := quantityObligationTx(tx, accountID); err != nil {

return err

@@ -1602,15 +1602,19 @@ func (s *Store) reserveBillingRetirement(accountID int64, now time.Time) error {

return tx.Commit()

}

-// releaseBillingRetirement removes a reservation. It is called only when the

-// retirement is abandoned before ANY external write has happened (for

-// example, no release store is configured, so tombstoneAccount refuses before

-// touching the bucket). An ambiguous failure AFTER a write may have begun

-// must keep the reservation, so that path never calls this: a retry can then

-// finish retirement, but no second checkout can open in the meantime.

+// releasePendingBillingRetirement releases a reservation that was not

+// committed. It is what a failed commit calls: the failure may be another

+// request's commit having won (two tabs, two consoles), whose committed

+// reservation is not this request's to release.

+func (s *Store) releasePendingBillingRetirement(accountID int64) error {

+ _, err := s.db.Exec(`DELETE FROM billing_retirements WHERE account_id=? AND committed_at=0`, accountID)

+ return err

// accountEvenIfDeleted and accountBySqidEvenIfDeleted are the operator

// doors' lookups: a billing review of a deleted account is still a person's

-// to see and resolve.

+// to see, and to hold (every review that can open after an account's

+// retirement is committed is a dispute, which stage one does not resolve).

func (s *Store) accountEvenIfDeleted(id int64) (*Account, bool, error) {

var a Account

var created, deleted int64

@@ -1637,11 +1641,6 @@ func (s *Store) accountBySqidEvenIfDeleted(sqid string) (*Account, bool, error)

return s.accountEvenIfDeleted(id)

}

-func (s *Store) releaseBillingRetirement(accountID int64) error {

- _, err := s.db.Exec(`DELETE FROM billing_retirements WHERE account_id=?`, accountID)

- return err

func (s *Store) readyBillingCustomer(intent billingIntent, customer string) error {

if customer == "" {

return errBillingIdentity

diff --git a/internal/console/deletion_test.go b/internal/console/deletion_test.go

index 471c9c1ee..affe72fed 100644

--- a/internal/console/deletion_test.go

+++ b/internal/console/deletion_test.go

@@ -16,6 +16,7 @@ import (

"github.com/carlosframework/platform/internal/domains"

"github.com/carlosframework/platform/internal/mail"

"github.com/carlosframework/platform/internal/objstore"

+ "github.com/carlosframework/platform/internal/plans"

"github.com/carlosframework/platform/internal/release"

)

@@ -128,6 +129,17 @@ func TestDeletionRefusedWhenTheBucketIsUnreachable(t *testing.T) {

if a, _ := fx.st.AccountBySqid(sqid); a == nil {

t.Fatal("the account was deleted locally even though the bucket could not be told")

}

+ // Nothing was committed and no reservation remains: the account goes

+ // on as it was, and can open a checkout.

+ a, _ := fx.st.AccountBySqid(sqid)

+ var reserved int

+ if err := fx.st.db.QueryRow(`SELECT count(*) FROM billing_retirements WHERE account_id=?`, a.ID).Scan(&reserved); err != nil || reserved != 0 {

+ t.Fatalf("%d reservation(s) left by a refused deletion (%v); want none", reserved, err)

+ }

+ request := billingIntent{AccountID: a.ID, Plan: plans.PlanPersonal, Terms: plans.CurrentTermsVersion, Band: "A", Price: "price_personal", Quantity: 1, Members: 1}

+ if _, err := fx.st.beginBillingCheckout(request, time.Now()); err != nil {

+ t.Fatalf("a checkout after the refused deletion: %v; want it allowed", err)

+ }

}

func readManifest(t *testing.T, s release.Store, sqid string) domains.AccountManifest {

diff --git a/internal/console/ownership.go b/internal/console/ownership.go

index 7fb896ac1..e5578aba0 100644

--- a/internal/console/ownership.go

+++ b/internal/console/ownership.go

@@ -154,12 +154,26 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) {

http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther)

return

}

+ // No bucket to write the tombstone to is known before the point of no

+ // return: nothing is committed, and only this request's own pending

+ // reservation is released (a committed one may be another request's).

+ if s.cfg.Releases == nil {

+ log.Printf("console: tombstone account %s: %v", acct.Sqid, errNoReleaseStore)

+ if rerr := s.store.releasePendingBillingRetirement(acct.ID); rerr != nil {

+ log.Printf("console: release retirement reservation %s: %v", acct.Sqid, rerr)

+ }

+ s.setFlash(w, "err", deletionAdvice(errNoReleaseStore))

+ http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther)

+ return

+ }

// The point of no return, in the database, before anything outside it:

// a checkout review that opened since the reservation (and withdrew

// it) refuses here, and the reservation it withdrew is not re-made.

if err := s.store.commitBillingRetirement(acct.ID, time.Now()); err != nil {

log.Printf("console: account retirement: %s", billingDiagnostic(err))

- if rerr := s.store.releaseBillingRetirement(acct.ID); rerr != nil {

+ // Only a pending reservation is released: the failure may be that

+ // another request committed first, and its retirement goes on.

+ if rerr := s.store.releasePendingBillingRetirement(acct.ID); rerr != nil {

log.Printf("console: release retirement reservation %s: %v", acct.Sqid, rerr)

}

s.setFlash(w, "err", deletionAdvice(err))

@@ -183,20 +197,11 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) {

// on its own, which is the bug this replaces.

if err := s.tombstoneAccount(r.Context(), acct); err != nil {

log.Printf("console: tombstone account %s: %v", acct.Sqid, err)

- if errors.Is(err, errNoReleaseStore) {

- // Definite pre-write refusal: nothing external was touched, so the

- // reservation must not persist and permanently bar this account

- // from ever opening a checkout once billing is on.

- if rerr := s.store.releaseBillingRetirement(acct.ID); rerr != nil {

- log.Printf("console: release retirement reservation %s: %v", acct.Sqid, rerr)

- }

- s.setFlash(w, "err", deletionAdvice(err))

- http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther)

- return

- }

- // Ambiguous failure after a bucket write may have begun: KEEP the

- // reservation so a retry can finish retirement, and no new checkout can

- // open in the meantime.

+ // A failure here is after the commit and may be after a bucket

+ // write began: the committed reservation is KEPT, so a retry can

+ // finish the retirement and no new checkout opens meanwhile. (The

+ // one definite pre-write refusal, no bucket at all, is checked

+ // before the commit.)

s.setFlash(w, "err", "Could not retire this account in the deployment bucket, so nothing "+

"was deleted — an account deleted only here would come back on the next restart. "+

"Try again once the bucket is reachable.")

@@ -612,10 +617,10 @@ func purgeAdvice(err error, app string) string {

// and says so: the alternative is a console-only deletion that undoes itself,

// which is worse than a refusal because it looks like it worked.

// errNoReleaseStore is returned by tombstoneAccount when no release store is

-// configured, so the caller can tell this DEFINITE pre-write refusal (nothing

-// external was touched) apart from an ambiguous failure after a bucket write

-// may have begun. The former is safe to unwind — release the retirement

-// reservation — while the latter must keep the reservation for recovery.

+// configured. The deletion handler checks for that before the retirement's

+// commit (and releases only its pending reservation then); after the

+// commit every failure of the tombstone, this one included, keeps the

+// committed reservation, because a bucket write may have begun.

var errNoReleaseStore = errors.New("console: no deployment release store is configured on this box")

func (s *Server) tombstoneAccount(ctx context.Context, acct *Account) error {

diff --git a/internal/console/store.go b/internal/console/store.go

index 288927f2b..f6c46a1f3 100644

--- a/internal/console/store.go

+++ b/internal/console/store.go

@@ -1365,8 +1365,8 @@ func (s *Store) ReapAccount(accountID int64, now time.Time) error {

defer tx.Rollback()

// The reap follows a retirement the deployment's manifest already

// records (made here after its commit, or on another console): an open

- // checkout review does not stop it, and stays listed and resolvable

- // under the deleted account.

+ // checkout review does not stop it, and stays listed under the deleted

+ // account, held.

if err := checkBillingRetirement(tx, accountID, true); err != nil {

return err

}