| new file mode 100644 |
| index 000000000..c97c8f41c |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review27.md |
| @@ -0,0 +1,152 @@ |
| +**Every review-26 finding and claim is closed at `9b872e38`.** No new P1, P2 or P3 finding. Review eighteen’s P2 remains closed on this branch. |
| + |
| +All platform references below are at `9b872e38`; pricing-site references are at `01b62f7`. This was source-only inspection: no modifications, tests, network or AWS commands. Test counterfactuals are judgments from source, not reported mutation runs. |
| + |
| +| Review-26 item | Closed? | Evidence and counterfactual | |
| +|---|---|---| |
| +| **Astra P3 / Opus P3-3:** losing-writer test accepted a successful stale write | **Yes.** | `internal/console/billingnamed_test.go:1509` requires a non-nil error containing `(517)`. Removing the snapshot-establishing read at `:1496` permits the later update to succeed; the strengthened assertion then fails. An unrelated error no longer satisfies it. | |
| +| **Opus P3-2:** evidence-before-commit ordering did not assert withdrawal | **Yes.** | `billingnamed_test.go:1456` checks that no reservation remains immediately after the other connection records evidence. Removing `cancelPendingRetirementTx` from `billingstore.go:958` leaves one row and fails this assertion. The subsequent commit-refusal assertion alone would still pass. | |
| +| **Opus P3-1:** three inaccurate comments | **Yes.** | The corrections at `ownership.go:157`, `billingstore.go:1216` and `billingretirement.go:999` match the implementation. These are documentation corrections, not independently testable behavior changes. | |
| +| **Astra claim 1:** dispositions said Astra’s A/B/C ordering was tested | **Yes.** | `docs/superpowers/specs/DISPOSITIONS-multi-currency.md:558` now distinguishes the stale-snapshot ordering from the shared-reservation A/B/C ordering. They reach the same cleanup condition; they are not the same experiment. | |
| +| **Astra claim 2:** round-24 mutant required a transient database fault | **Yes.** | `DISPOSITIONS-multi-currency.md:545` now records the ordinary ordering: B removes the shared pending reservation, A’s commit refuses, C reserves and commits, then A cleans up. It also records the structural closure: unconditional release is test-only. | |
| +| **Astra claim 3:** pending reservation belonged to the request | **Yes.** | `ownership.go:157` now says the account’s pending reservation. `billingstore.go:1602` uses one account-keyed row and `INSERT OR IGNORE`; there is no request ownership. | |
| +| **Astra claim 4:** late filing after intent removal files nothing | **Yes.** | `billingstore.go:1216` describes archive classification. The gone-intent branch at `:1247` classifies subscriptions and preserves a refusal against a consumed purchase; `billingretirement.go:928` also resubmits the observed session for archive classification. | |
| +| **Astra claim 5:** hourly retry began after three attempts | **Yes.** | `billingretirement.go:999` now agrees with the old-counter expression at `:833`: delays are 1, 5, 15, then 60 minutes after each attempt from the fourth. | |
| +| **Astra claim 6:** no test runs two consoles | **Yes.** | Retirement design `:113` now says no test runs two console **processes**, and explicitly names the two connections to one database file. | |
| +| **Astra claim 7:** only one exception to fate-from-read | **Yes.** | Retirement design `:120` names ordinary expiry retirement and the conservative completion observation from a refused create response. These correspond to `billingstore.go:549` and `billingservice.go:366`. | |
| + |
| +The two strengthened tests therefore **would not pass without the behavior they now distinguish**. Neither depends on a sleep, scheduling luck, shared cookie jar or wall-clock boundary. Their database order is explicit. They establish two-connection transaction behavior, not actual process termination. |
| + |
| +**Effect on the previous conclusions** |
| + |
| +Nothing in this delta reverses either report’s substantive conclusion at `35356ad0`. Executable production code is unchanged. The delta closes the remaining test and documentation qualifications, so Astra’s previous **accept with fixes** becomes **accept**. |
| + |
| +Where severity differed, **Astra’s P2 reading was correct** for the retirement safety failures: |
| + |
| +- A non-atomic migration could leave an older, possibly externally committed retirement withdrawable. That could reopen checkout during account deletion. |
| +- Unconditional cleanup could remove another request’s committed reservation through an ordinary request ordering; a database fault was unnecessary. |
| + |
| +Those consequences cross the retirement safety boundary. Their rarity did not make them P3. Conversely, review 26’s missing assertions were P3 test weaknesses: the production protections already existed. |
| + |
| +For the earlier disagreement about expiry authority, expiring the console’s **persisted bound session ID** is the correct reading. Contradictory response metadata prevents proving a fate; it does not revoke the console’s previously established authority over that bound ID. An arbitrary unowned create or listing response supplies no equivalent authority. |
| + |
| +**Review eighteen’s P2 and the retirement boundary** |
| + |
| +**Closed, including the subsequent account-retirement and migration counterexamples. Nothing further on this branch is required to close it.** This conclusion assumes the documented rollout rule stopping older consoles; the schema does not make arbitrary older binaries safe. |
| + |
| +The original scenario is now directly covered by `TestRetireDoesNotReleaseAnIntentWhoseSessionIsStillPayable`, `billingretirement_test.go:153`: |
| + |
| +1. A subscription causes refusal while expiry fails. |
| +2. Operator retirement receives 503; the review and payable intent remain. |
| +3. After expiry succeeds, the attached subscription still causes 409. |
| +4. The operator listing exposes that remaining obligation. |
| + |
| +Restoring the old unconditional operator retirement fails this test. Removing only the operator’s advancement call also changes the expected 503 behavior; removing the obligation hold allows the later retirement and fails the 409 assertion. The separate proven-expiry test at `:216` checks the case that may actually retire. |
| + |
| +The invariant enforced by this implementation is: |
| + |
| +> An open refusal prevents ordinary consumption or removal of its intent. Observed subscriptions survive through an obligation or archive classification. Reviewed retirement requires the permitted terminal evidence and no outstanding obligation. Account retirement has a durable database commitment boundary; evidence arriving afterwards remains visible against the retiring or deleted account. |
| + |
| +The production exits are: |
| + |
| +| Exit or transition | Protection at this commit | |
| +|---|---| |
| +| **Bind/rebind session**, `billingstore.go:458`, `:461` | One binding helper checks existing purchase ownership and the review’s pinned reference. Same-session replay is allowed; another purchase’s session is refused. | |
| +| **Ordinary expiry**, `:553` | Validated expired, unpaid, subscription-free proof; exact intent/session; no open refusal or dispute. A previously observed named subscription causes a hold instead of deletion. | |
| +| **Unready cleanup**, `:604` | Exact unbound, customer-not-ready intent; no open refusal. Named subscriptions cause a hold. | |
| +| **First-purchase consumption**, `:890` | Exact standing intent, no open refusal or dispute. Consumption, archive, account/assignment changes, event record and queued work share the transaction. Additional named subscriptions become disputes. | |
| +| **Operator retirement**, `:1208` | The transaction first evaluates `refusalStateQ` and `held`, archives the exact intent and evidence, and closes only its pending checkout quantity events. | |
| +| **Account deletion/reap** | Account identity is soft-deleted; billing evidence survives. Commitment determines whether later evidence stops deletion or remains attached to the deleted account. | |
| + |
| +There is no new ordering in the delta where observed evidence becomes only a rejected event. The documented unarchived exceptions remain: ordinary expiry or never-created cleanup **before anything was observed naming the intent**. |
| + |
| +All checkout/session and subscription reads in production `internal/console` still enter through `readCheckout`, `listCheckouts` or `readSubscription` (`billingretirement.go:872`, `:891`, `:909`). This covers binder, reconciliation, pass, start/resume, recovery, operator keep, refusal advancement, invoice preflight and quantity work. Recording errors prevent callers receiving a successful read. Listing records decoded objects before returning a listing failure. |
| + |
| +Evidence classification deliberately precedes operational ownership checks. It uses recorded identity to preserve contradictory evidence; it does not authorize binding or recording the response as a purchase. Create and expiry responses retain their documented exceptions. |
| + |
| +`keep` remains conservative but limited: it validates the bound completed, paid session and the store’s obligations/disputes; it does **not** certify the subscription’s complete purchase identity. Subsequent reconciliation can reopen the refusal. Paid-but-not-kept, disputed, additional-subscription and pre-protocol cases remain held without a stage-one disposition. |
| + |
| +**Account deletion, migration and cleanup** |
| + |
| +The deletion handler still performs preparation → reservation → no-bucket preflight → commitment → tombstone → local deletion (`ownership.go:148`, `:151`, `:162`, `:174`, `:200`, `:213`). |
| + |
| +- Before commitment, evidence withdraws the pending reservation and prevents external publication. |
| +- A no-bucket request can withdraw the account’s shared pending reservation. Another request must then fail its commitment safely. |
| +- After commitment, neither evidence nor losing-request cleanup removes the reservation. Checkout stays blocked through an ambiguous tombstone failure or process death; a retry can finish deletion. |
| +- Evidence after commitment remains discoverable under the deleted account, including `account_deleted`; it is held, not silently resolved. |
| + |
| +Every production write to `billing_retirements` is accounted for: |
| + |
| +| Location | Operation | |
| +|---|---| |
| +| `billingretirement.go:203` | Backfill zero to `started_at`, only in the transaction introducing the column. | |
| +| `billingstore.go:1551` | Evidence deletes only `committed_at=0`. | |
| +| `billingstore.go:1581` | Commitment writes a positive timestamp after transactional reservation and condition checks. | |
| +| `billingstore.go:1602` | `INSERT OR IGNORE`; existing reservations are preserved. | |
| +| `billingstore.go:1613` | Cleanup deletes only `committed_at=0`. | |
| + |
| +**No production path withdraws a committed reservation.** The unconditional helper exists only at `billingretirement_export_test.go:8`. Reintroducing its call into production fails an ordinary build, although a same-package test build can still see the helper. |
| + |
| +The column check, `ALTER TABLE ADD COLUMN` and backfill share one transaction (`billingretirement.go:188`). SQLite’s transactional DDL means interruption before commit preserves neither addition nor backfill; after commit both persist. The aborting-backfill regression at `billingnamed_test.go:1347` distinguishes the former autocommit implementation. |
| + |
| +A second migration sees the committed column if its first schema read occurs afterwards. A stale read transaction can instead fail promotion and abort startup; that is a startup retry, not partial migration. A later start does not mark newly created pending reservations committed. The documented exception for a database already damaged by the earlier faulty migration still applies. |
| + |
| +**Confirmation of the unchanged pricing and purchase conclusions** |
| + |
| +For the requested one-member Personal purchase, India plus explicit GBP means band D, **£7.70 annually**, with **29.75 awake hours and 0.7 GB each of storage and transfer**. Country determines allowances independently of currency. |
| + |
| +The signed form context flows through `billingpage.go:291`, `:395`, `:446`; effective-choice comparison at `billinghandlers.go:71`; membership comparison and GBP intent creation at `billingservice.go:452`; ownership and purchase checks at `:249`, `:273`, `:1182`; reconciliation at `:819`; and the recording transaction at `billingstore.go:677`. |
| + |
| +History reads both assignment tables with their stored money terms (`billinginvoicestore.go:529`). `AllowanceForMonth` preserves those terms and refuses mixed paid spans (`internal/plans/month.go:173`). Calculation uses the assignment’s book and currency (`billingcalculation.go:174`). Invoice creation, recovery and line verification compare against that calculation’s currency; quantity work checks the stored price and both subscription/item currencies (`billinginvoice.go:859`, `:901`, `:1004`, `:1116`; `billingquantity.go:1010`, `:1105`). I found no new conversion, EUR fallback or historical repricing path. |
| + |
| +The unchanged form and cookie conclusions stand: |
| + |
| +- Present currency must be exactly one supported lowercase value. Empty, repeated, padded, uppercase or unknown values receive 400 after preceding authentication/form gates. |
| +- Missing currency is distinct from present-empty. The effective country default must match the signed quote before purchase. |
| +- Changed or fabricated quote fields redirect to the account-local quote. The quote endpoint creates no purchase and cannot construct an external redirect. |
| +- The server-keyed signature covers session token, account, book, country, band, currency, choice flag and member count. Disclosed form values do not permit forgery. Another tab does not revoke an unchanged genuine quote. |
| +- Membership determines quantity under the lock and transactional recheck. All offered plans are shown under the signed context; postal code affects checkout tax. |
| +- Present-empty query parameters clear cookies; absence permits fallback; invalid parameters do not overwrite. An invalid parameter can preserve an older cookie for a later absent-parameter request, but a cleared cookie is not resurrected. |
| +- Checkout reads the posted form, not cookies. Previously purchased currency is offered alone, including legacy EUR. |
| + |
| +The refined barrier at `billingstore.go:208` matches positive paid spans in `AllowanceForMonth`: active, non-free assignments, clipped by month boundaries, paid-through time and the next assignment across both tables. Same-second supersession contributes zero. Renewal and quantity assignments end the preceding span at their effective instant; `period_end` remains the paid-through boundary. I found no counterexample allowing an older version-1 freeze over a positively priced book span, or rejecting this build’s valid legacy-only month. |
| + |
| +The replacement-trigger check precedes dropping the old name (`:1007`). Interrupted migration can conservatively leave both; it does not remove the old trigger without its replacement. Successful restart is idempotent. |
| + |
| +Book validation, checked arithmetic, legacy snapshots, complete historical catalogs, current-book key counts and own-only `currency_options` retain the prior conclusions. Provisioning regression coverage remains partly source-text inspection, not execution of the emitted provider checks. |
| + |
| +**Section K: pricing repository** |
| + |
| +The site’s committed export is byte-for-byte equal to the platform export at these commits. |
| + |
| +`copy-review/pricing/build.mjs:47` renders exact micro rates; `:102` checks display precision; `:160` checks exact per-member shares. Countries and their default currencies come from the export (`:133`, `:208`). The first-line draft marker and exact `PRICEBOOK_ALLOW_DRAFT=1` exception agree with `hack/check-pricebook.mjs:7`. |
| + |
| +`src/site.js:75` always sends both parameters, sending currency only after an explicit choice and country only as selected. A locale suggestion may therefore differ from the console’s later default; that is the deliberate behavior required by ruling 9. |
| + |
| +The tests check all price slots across currencies and bands, exact rendered static values, and whole-token figures. Their stated limitation remains correct: **a literal identical to its slot’s current derived value passes an output-equality test** (`test/pricing-pricebook.test.mjs:202`). These tests establish correct output, not derivation under every possible future export. |
| + |
| +**Claims the code contradicts** |
| + |
| +No additional contradiction requiring a branch fix was found in this confirming delta. All seven review-26 claims are corrected. |
| + |
| +The existing qualifications remain material: two connections are not a process-death test; the provider-read guard is lexical rather than a general control-flow proof; provisioning checks partly inspect script text; and historical mutation-run counts cannot be certified from committed source. Review three’s economically equivalent mutation and review four’s qualified coverage should retain those qualifications. |
| + |
| +**What must be verified against Stripe test mode before enablement** |
| + |
| +| Run | Required evidence | |
| +|---|---| |
| +| **Payment lifecycle first:** successful card, decline/retry, abandoned 3-D Secure, delayed payments, SEPA/Bacs failure | Whether subscriptions exist before completion, survive failed attempts or change IDs; which honest flows enter stage-one holds. This remains the first protocol enablement gate. | |
| +| Read all 36 catalog prices, including expanded options | Exact amounts, currencies, intervals, tax behavior and metadata; own-only `currency_options` accepted. | |
| +| Complete USD/EUR/GBP purchases, including India + GBP and country defaults | Form, session, subscription and assignments agree; Adaptive Pricing is disabled. | |
| +| Legacy EUR and customers with a different provider default | Held currency remains usable; customer reuse does not silently convert or permanently strand checkout. | |
| +| Race expiry with payment; replay successful/failed keys and lose responses | Fresh reads distinguish expired from complete; cached responses do not substitute for reviewed-retirement proof; retained URLs cannot pay after proven expiry. | |
| +| Reorder and duplicate checkout/subscription events | One purchase records once; incomplete associations retry; retired completions cannot record replacements; evidence survives acknowledgement. | |
| +| Lose create responses and exercise paginated recovery | Recovery finds the original purchase, preserves partial-page evidence and opens no duplicate payable session. | |
| +| Edit provider identity and purchase metadata where permitted | Read shapes satisfy ownership assumptions; expiry targets only the persisted or fully validated session; corrected identity is observable. | |
| +| Quantity changes, renewal, cancellation and late old-subscription events | Stored-currency proration and verification; renewals consume no unrelated intent. | |
| +| Overage invoices around every currency’s floor, with interrupted creation/recovery | Exact rates, rounding, line totals, request hashes and recovery without conversion or duplicate invoices. | |
| +| Reviewed retire/keep, provider failures, restart and late evidence | Holds persist until permitted proof; exact replay works; attached, paid and disputed cases remain visible, including after account deletion. Cancellation and `incomplete_expired` must not be assumed to settle outstanding money. | |
| + |
| +**Merge condition:** this confirming review leaves **nothing to fix on the branch**. I accept the tip for the intended merge commit into `feat/carloku-paid-launch-20260916`. The merge still requires the other confirming reviewer’s acceptance and green CI on **`9b872e38`**; this source-only review cannot certify either. CI recorded for `35356ad0` does not establish the tip’s result. The documented decisions, approvals and provider rehearsal remain enablement conditions, not additional branch fixes. |
| + |
| +**Verdict: accept.** |
| \ No newline at end of file |