amadan
Review twenty-three's two reports in the tree
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+273
−0
docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review23.md+170 −0docs/superpowers/specs/OPUS-REVIEW-multi-currency-review23.md+103 −0
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review23.md b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review23.md| new file mode 100644 |
| index 000000000..2092779b0 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-multi-currency-review23.md |
| @@ -0,0 +1,170 @@ |
| +All platform references below are at **`2618403f`**; pricing-site references are at **`01b62f7`**. This was source-only review against `12e70d95`, including the delta from `05ccec07`. Nothing was modified or tested, and no network commands were run. Mutation counterfactuals below are judgments from source. |
| + |
| +**Twenty-second review, finding by finding** |
| + |
| +| Finding | Closed? | Would its test pass without its fix? | |
| +|---|---|---| |
| +| **Opus P2-1:** a dead attached subscription can leave an honest buyer held indefinitely | **Addressed honestly as a conservative stage-one limitation and enablement gate, not eliminated.** `billingretirement.go:97` and dispositions `:484`, `:500` explicitly say there is no stage-one disposition. | **Yes: behavior is unchanged.** There is no new behavioral regression proving a status-based release, nor should one be claimed. | |
| +| **Opus P3-1:** unready cleanup is another no-archive exception | **Closed as a documentation finding.** Both ordinary expiry and unready cleanup are named at `billingretirement.go:91` and retirement design `:126`. | Not applicable: the accepted response documents the exception. | |
| +| **Opus P3-2:** subscription attribution follows incorrect or missing account metadata | **Closed.** `accountOfIntentTx`, `billingretirement.go:765`, finds the console’s recorded owner before classification. | **No.** `billingnamed_test.go:888` requires attachment for both missing and incorrect account metadata. | |
| +| **Opus P3-3:** the pass reports a hold it caused as a failure | **Closed at both sites**, `billingpass.go:267` and `:297`. | **No with the whole fix removed.** The named test at `billingnamed_test.go:912` exercises the bound-session branch. Removing only the recovery filter at `billingpass.go:297` is not distinguished by that test. | |
| +| **Opus P3-4:** inconsistent dispute key for consumption without a session | **Closed.** `billingretirement.go:660` uses the intent ID, matching consumption. | **No.** `billingnamed_test.go:938` distinguishes it. This remains a defensive store case: production reconciliation requires a bound session. | |
| +| **Opus claim 1:** only ordinary expiry lacks an archive | **Corrected**, as above. | Not applicable. | |
| +| **Opus claim 2:** the stranded buyer “waits on a person” | **Corrected:** the buyer is held with no available stage-one disposition. | Not applicable. | |
| +| **Opus claim 3:** the ended-subscription regression establishes ordinary provider behavior | **Corrected.** Dispositions `:473` calls it defensive and identifies the fake’s period change. | The regression fails without the consumption fix, but does not establish that Stripe produces its setup. | |
| +| **Astra 1, P2:** retirement ignores open checkout refusals/disputes | **Closed for an already-open case at reservation. The complete retirement boundary remains incomplete:** Finding 1 below. `billingstore.go:1514` adds the transactional check. | **No.** `billingnamed_test.go:711` fails without it. It passes with the remaining post-reservation race. | |
| +| **Astra 2, P2:** a session-only refusal filed after consumption disappears | **Closed.** `billingstore.go:1256` preserves the refusal against the consumed purchase. | **No.** `billingnamed_test.go:758` requires the dispute and open review. It directly exercises delayed filing, rather than orchestrating two consoles. | |
| +| **Astra 3, P2:** a click discloses a link despite freshly observed review | **Closed for the reported cases.** Both post-read checks and the newly saved link check include `Open`: `billingservice.go:653`, `:715`, `:781`. | **No.** The three cases at `billingnamed_test.go:791` distinguish the checks independently. The first changes country and asserts zero expiries, avoiding the previous masking by the second check. | |
| +| **Astra 4, P3:** duplicate-page failure discards decoded observations | **Closed for duplicate pagination. A decoding-error variant remains:** Finding 2 below. | **No for the reported duplicate case.** `billingnamed_test.go:868` and `billingstripe/catalog_test.go:333` distinguish that fix; both pass with Finding 2. | |
| + |
| +For **Opus P2-1**, the disposition is sufficient for conservative **stage one**, provided it remains an enablement gate. Reading an attached subscription’s status is unnecessary merely to continue holding it. Automatically exempting `incomplete_expired` would introduce a settlement policy that stage one deliberately withholds. Astra’s test-mode item 8 means exactly that status alone cannot establish the disposition of payments, invoices, pending collection or refunds. If rehearsal demonstrates the problematic honest-buyer sequence, enabling requires the owner’s settlement decision and an implementation checking its evidence—not just a status exception. |
| + |
| +On severity, Astra’s P2 reading was right for losing outstanding obligations, retiring their account, or offering payment after observing an open refusal. Opus’s conclusion that the deletion/consumption checks closed the whole lifecycle family was too broad. The pass’s one-time diagnostic failure and the unreachable empty-session archive case are appropriately P3. |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — A dispute recorded after retirement reservation does not stop the authoritative account tombstone.** |
| + |
| + **Where:** `internal/console/billingstore.go:1527`, `:1539`; `internal/console/ownership.go:151`, `:172`, `:194`; `internal/console/billingretirement.go:653`, `:737`. |
| + |
| + **Concrete scenario:** |
| + |
| + - Legitimate subscription C records, consuming intent I. C later ends. |
| + - Account deletion passes `reserveBillingRetirement`; its transaction commits. |
| + - Before the bucket write, another console reads subscription H naming archived I. Classification **commits** a checkout dispute. |
| + - The deletion request still calls `tombstoneAccount`. |
| + - Only afterwards does `DeleteAccount` rerun `checkBillingRetirement` and refuse. |
| + |
| + This is not the acknowledged boundary between receiving provider data and recording it: the dispute has already committed before external retirement. |
| + |
| + The external effect is real. `internal/domains/mint.go:248` clears the manifest’s owner and sets `DeletedAt`; `:254` publishes it. `internal/domains/reconcile.go:601` subsequently removes the registry account. |
| + |
| + The local account and evidence remain because `DeleteAccount` refuses. However, the authoritative manifest says the account is retired. The reservation remains, retries now fail the review check, and new checkout is blocked by `billingstore.go:363`. Stage one cannot resolve the dispute that prevents completion. |
| + |
| + **Smallest acceptable fix:** coordinate authoritative retirement authorization with evidence recording across consoles. A pending retirement must be cancellable by evidence committed before finalization, or the final check and publication must have a shared serialization boundary. Another standalone preflight check merely narrows the race. Preserve evidence and operator access when retirement has already progressed. |
| + |
| + Add a regression that pauses **after successful reservation and before tombstoning**, records H through the production classification path, then resumes deletion. The current test at `billingnamed_test.go:711` checks only the opposite ordering and passes with this defect. |
| + |
| +2. **P3 — A malformed session discards valid sessions already decoded from the same listing page.** |
| + |
| + **Where:** `internal/billingstripe/catalog.go:152`, `:161`; `internal/billingstripe/client.go:240`, `:351`; recording wrapper at `internal/console/billingretirement.go:858`. |
| + |
| + **Concrete scenario:** |
| + |
| + - A session S was retired on proven expiry. |
| + - A listing response is valid JSON containing a fully valid S, now `complete`, followed by `{"id":"cs_bad"}` without `livemode`. |
| + - JSON decoding successfully populates S before `Session.UnmarshalJSON` rejects the second object. |
| + - `c.do` returns `CodeDecodeError`. |
| + - `listCheckouts` returns only `out`, containing previous pages, and discards the successfully decoded S from this page. |
| + - The recording wrapper never receives the contradictory completion, so no dispute is filed. |
| + |
| + The new loop correctly preserves duplicates **after successful page decoding**. It does not handle this earlier return. |
| + |
| + **Smallest acceptable fix:** decode the page envelope into raw session elements, validate elements independently, and return fully validated observations alongside the page’s error. Preserve valid elements before and after a malformed element; never classify a partially decoded object. Recovery must still fail. |
| + |
| + Add client and wrapper cases containing a contradictory archived session and a malformed neighboring element. The new duplicate-page tests pass without this additional protection. |
| + |
| +**Claims the code contradicts, and the remaining requested checks** |
| + |
| +The broad account-retirement and listing-evidence claims still need qualification because of Findings 1 and 2. Other concrete prose discrepancies remain: |
| + |
| +- `billingstore.go:1217` says a late gone-intent refusal files nothing; the new branch at `:1256` deliberately files a dispute after consumption. The binder’s “only while that intent stands” comment at `billingservice.go:1284` needs the same qualification. |
| +- `billingnamed_test.go:763` describes a session read before consumption. The test calls `purchased` first and reads the session at `:766` afterwards. It establishes delayed-filing handling, not that complete read/record interleaving. |
| +- `billingservice.go:790` calls unready deletion preparation unconditional. The production cleanup respects reviews and recorded named subscriptions. |
| +- `billingretirement.go:965` says retries are at least an hour apart and reach every case within “a few passes however many.” The schedule at `:797` begins at one, five and fifteen minutes; the three-attempt budget cannot promise a fixed small number of passes for an arbitrary backlog. |
| +- Retirement design, “Stage one as built,” `:116` still broadly says a subscription naming an unbound intent files a review. An on-terms one retries at `billingservice.go:1009`, with its obligation retained. |
| +- That design’s create-answer exclusion at `:115` still omits the completed-refused-create exception documented at `billingretirement.go:82`. Its subscription-attribution description also needs to reflect the recorded intent owner taking precedence over account metadata. |
| + |
| +The following conclusions cover the requested pricing, ownership, migration and lifecycle checks. |
| + |
| +| Area | Source conclusion | |
| +|---|---| |
| +| **A: GBP purchase end to end** | India selects band D independently of GBP. Personal renders £7.70 annually. The handler compares the effective posted choice with the authenticated quote (`billinghandlers.go:71`); checkout pins GBP/book (`billingservice.go:603`) and sends fixed-currency parameters (`:221`). Account and assignment storage preserve them (`billingstore.go:681`, `:848`). Monthly allowance rejects mixed price terms (`plans/month.go:167`); calculation produces GBP version 2 (`billingcalculation.go:174`). Invoice creation, recovery and verification retain GBP (`billinginvoice.go:516`, `:569`, `:901`, `:964`, `:1004`, `:1116`), as does the quantity worker (`billingquantity.go:1017`, `:1105`). I found no conversion or silent EUR substitution in this trace. | |
| +| **B, M: one form and effective choice** | Present currency must be exactly one supported value. Empty, repeated, uppercase and unsupported fields fail (`billinghandlers.go:176`). Absent currency supplies no explicit choice; against a book console the country default must still match the signed quote. Changing country/currency, erasing the choice flag or fabricating quote fields cannot authorize an unseen quote. A stale tab can submit its own still-valid quote; another tab’s cookies do not change its posted purchase. | |
| +| **Quote door** | Session, form token and membership precede a redirect constructed to the account’s billing page. Posted values are URL-encoded (`billinghandlers.go:165`, `:197`). This door creates no checkout and cannot choose an external redirect destination. | |
| +| **Q, U, Z: signature** | The key derives from the server’s billing secret; the disclosed CSRF token is message material, not the key (`billingpage.go:442`). The signature covers token, account, book, country, band, currency, choice flag and member count. Another session/account/book/token invalidates it. The page quotes all offered plans; charged quantity derives from membership and plan minimum. Postal information affects location/tax rather than the quoted pre-tax book price. Membership is checked again before charging and transactionally when the intent is created. | |
| +| **C, O: cookies and held currency** | Present-empty clears and suppresses fallback; absent falls back; invalid changes no cookie (`billingcurrency.go:43`). Consequently, an invalid parameter can leave an old cookie available on a later parameter-absent visit. That is the documented rule, not an implicit checkout choice. Held currency is offered alone; legacy purchases hold EUR. Service and transactional intent creation enforce it even after cancellation. | |
| +| **E, N: assignment-span trigger** | `billingstore.go:208` uses positive overlap ending at the earliest of the next assignment, paid-through limit and month end, matching `AllowanceForMonth` (`plans/month.go:141`). It considers both assignment tables and same-second revision ordering. Same-second supersession contributes zero span; inactive/free rows contribute no paid book span. Renewal and quantity writers preserve provider `period_end` as the paid-through limit, distinct from the revision’s effective date (`billingstore.go:845`). I found neither requested valid-history counterexample. | |
| +| **H, T: migration and rollback** | The refined trigger exists before `retireEarlierBarrierTrigger` verifies it and drops the old name transactionally (`billingstore.go:1002`). Missing new trigger leaves the old one; repeated startup is idempotent. An older prohibited build can recreate the old trigger name. The rollout prohibition remains necessary: these barriers do not make an old binary understand book history or the complete retirement protocol. | |
| +| **F: inclusions** | The page divides `PriceFor`’s pooled allowance by charged quantity. Unavailable Personal uses a valid Personal quantity for its disabled row. Current terms yield exact displayed shares, including teams above minimum; the tests check all factors and band-A wording (`billingcurrency_test.go:2554`). Removing an exactness assertion over already-exact data need not fail; changing the data to violate it does. | |
| +| **G, J: book, catalogs and export** | Validation checks strict shape, complete dimensions, duplicate keys, rounding, overflow and legacy-EUR agreement (`pricebook.go:149`). Configuration accepts the required catalog plus whole optional catalogs, with unique provider IDs (`cmd/console/billing.go:106`). Provisioning and console agree on own-only expanded `currency_options`. The export contains public commercial/resource/country data, not credentials or provider catalog identities (`pricebook.go:727`). | |
| +| **K: pricing site** | At `01b62f7`, rates retain exact micro amounts; inclusions use exported factors and exactness checks (`copy-review/pricing/build.mjs:47`, `:113`, `:156`). Draft rendering/checking uses the first-line marker and exact `"1"` override. Country choices/defaults come from the book. Links always carry both parameters, with currency only when chosen (`src/site.js:40`, `:75`, `:89`). A locale-only suggestion may differ from the console’s later country default because the suggestion is intentionally not transmitted as a choice. | |
| +| **MM, QQ: site tests** | Slot values are checked across plans, bands and currencies; rates are parsed back to exported micro amounts. Whole-token checks cover approved numeric literals. A hardcoded value exactly equal to the expected current value still passes an output-equality test; the test explicitly acknowledges that limitation (`pricing-pricebook.test.mjs:202`). `hack/money.mjs` supplies the production formatter exercised by the fractional-minor test. | |
| +| **D, L, R, Y, DD–NN: session ownership** | `ownSession` checks mode, intent, both account identifiers, known customer and bound ID (`billingservice.go:273`). Purchase checks follow ownership. Expected-intent binding checks its recorded account before deriving an account from the response (`:1204`). Create/list recovery cannot authorize another account’s session. | |
| +| **Bound-ID expiry exception** | A previously bound session with contradictory identity is reviewed first, then its **recorded bound ID** may be expired (`billingservice.go:310`; `billingretirement.go:914`). That is distinct from accepting its contradictory response as proof. The response cannot retire the intent. This is the justified exception to refusing expiry of an unowned create/listing result; leaving a disclosed payable bound URL open solely because metadata changed would be the worse authority choice. | |
| +| **Reconcile versus binder** | A first purchase must satisfy matching session purchase identity and its own subscription association (`billingservice.go:997`, `:1038`, `:1054`). Complete-with-no-subscription and pending payment retry. Renewals do not undergo first-purchase intent consumption. Refused matching-intent subscriptions are retained before rejection. Unknown terms outside the bound-first-purchase case remain retryable; an open review/disputed record can also continue returning conflict/503 under the explicitly conservative policy. | |
| +| **X, AA, CC: quantity history** | `quantityOutcomesSQL` supplies the processing guard, startup detection and obligation check (`billingquantityaudit.go:20`, `:42`, `:61`). The healing sweep is gone; startup does not clear pre-history or clock-rollback gaps. | |
| +| **Z, BB, YY, ZZ, BBB, CCC: operator records** | References and resolution outcomes are retained. Historical omitted outcomes remain `retire`; exact replay compares actor, note and outcome. The rejected listing is operator-only, newest first and bounded to 200. It drains rows before account lookups (`billingoperator.go:290`). It is not a universal ledger of interactive errors or retrying events. The added migration and ordering tests establish their fixtures, not every concurrent execution. | |
| + |
| +The lifecycle invariant supported by the code is: **durably classified named subscriptions survive intent consumption as accepted-purchase identity or disputes; expiry and cleanup preserve an intent with recorded obligations or an open refusal; operator retirement checks the recorded proof and obligations transactionally.** It is not yet a complete invariant across authoritative account retirement or all decoded listing evidence. |
| + |
| +There are four production intent-delete statements and one binding helper: |
| + |
| +| Door | Effect on previously recorded evidence and later observations | |
| +|---|---| |
| +| **First-purchase record — `billingstore.go:890`** | Exact-intent deletion, account/assignment changes, event, entitlement queue, consumption archive and disputes for other subscriptions commit together. Failure rolls everything back. Later subscription observations compare with that archive. Same-subscription renewal/cancellation consumes no intent. | |
| +| **Ordinary expiry — `billingstore.go:553`** | Reached through pass, click and refused-session expiry, including unbound refused creation. Proof is checked first. Known naming subscriptions cause a refusal and preserve the intent; open review/dispute blocks deletion. | |
| +| **Unready cleanup — `billingstore.go:604`** | Reached through pass, replacement click and account-deletion preparation. `customer_ready=0` plus no session distinguishes never-created checkout. Recorded subscriptions or open review prevent cleanup. | |
| +| **Operator retire — `billingstore.go:1203`** | Resolves and archives exactly the reviewed intent, with proof, outcome and quantity-event disposition, in one transaction. Protocol-zero reviews, disputes and attached subscriptions remain held. | |
| +| **Session binding — `billingstore.go:427`** | Both binding callers use this helper. It refuses another purchase’s session and a session conflicting with an open review reference. Same-session rebinding and lost-answer recovery remain possible. | |
| +| **Account retirement** | No intent cascade exists; local soft deletion and reaping check billing holds. The reservation-to-tombstone interval remains defective as Finding 1 describes. | |
| +| **Webhook settlement** | Required classification precedes acknowledgement. Evidence and rejected/seen-event recording use separate transactions; a crash between them leaves the event retryable. Archived identity paths retain attribution. Neither acknowledged rejection nor event deduplication independently resolves a review. | |
| + |
| +For observations committed before expiry or cleanup, the transaction ordering retains them. If deletion wins first, the explicitly documented exception applies: **ordinary expiry or unready cleanup with no previously recorded naming subscription leaves no archive**, and a subsequently observed subscription can become only a rejected event. Consumption and operator retirement have archives and do not share that exception. |
| + |
| +`billing_consumed_intents` is written when an intent is actually consumed. Generated intent IDs make ordinary replacement of an existing archive unnecessary; deliberate historical ID reuse is not a supported lifecycle. The account’s recorded subscription and the archive’s accepted subscription exclude ordinary renewal, cancellation and redelivery from becoming disputes. The empty-session fallback is now consistent. |
| + |
| +For **stage-one retirement**: |
| + |
| +- Fate is written once; contradictory terminal/identity evidence creates a durable dispute. |
| +- Review protocol, attachments, disputes and fate are checked inside the resolution transaction. No caller’s stale generation authorizes release. |
| +- `keep` requires the bound, owned, matching, completed and acceptably paid session. It does not fully validate the subscription’s terms; subsequent reconciliation can reopen the refusal, as the governing “as built” section states. |
| +- A kept intent receiving another subscription is held again; consumption disputes additional recorded subscriptions. Late session refusals now survive ordinary consumption. |
| +- A dispute has no stage-one resolution. An old client omitting outcome cannot bypass that rule. |
| +- Expiry failure or process death after filing leaves the review authoritative. `advanceRefusal` retries the pinned bound session and obtains proof through reads; a winning completion remains held. |
| +- The refusal turn has its own 45-second deadline, three-attempt budget and longest-due ordering. Both turns inherit caller cancellation. This avoids the reported fixed-order starvation, without promising bounded completion for an arbitrary workload. |
| +- Trigger recreation is transactional. A failed startup retains the previous committed trigger. The trigger alone does not make an older binary obey attachment and account-retirement rules. |
| + |
| +For **MMM–SSS**, the inspected production checkout/subscription reads use `readCheckout`, `listCheckouts` and `readSubscription`: start/resume, recovery, binder, reconciliation, pass, keep gate, refusal advancement, quantity worker and invoice checks. Store failure returns no usable object. The meaningful pre-classification returns are provider errors, requested-ID/mode mismatches and the explicitly excluded unidentifiable/no-refusal states; Finding 2 shows provider decoding errors can conceal successfully decoded neighboring observations. |
| + |
| +The structural test at `billingevidence_test.go:544` catches selector uses through aliases, method values, method expressions, interfaces and same-named methods on another type. Reflection also requires classification of every exported provider-client method. Its recording-order check is lexical, not control-flow proof; an unreachable recording call could satisfy it. A differently named read on an unrelated client remains outside that inventory. |
| + |
| +For **I, P, LLL, RRR, ZZZ2 and JJJJ**, the source supports the targeted counterfactual tests, with these limits: |
| + |
| +- The four previously surviving round-20 mutations now have distinguishing cases: individual identity fields, an incomplete additional subscription reopening after `keep`, the refusal’s sole read recording fate, and contradiction only on the second resume read. |
| +- The refusal-attachment rollback test injects failure inside the production helper, rather than merely testing an unrelated SQLite transaction. |
| +- The fake’s distinct second session and retired-completion replay assertions distinguish recording a retired purchase. |
| +- The historical-retirement helper recreates material pre-protocol state; it is not permission for current code to produce it. |
| +- Review-three/four qualifications matter: provisioning assertions inspect shell text; successful migration does not demonstrate every crash ordering; account-only deletion can be redundant behind an exact-intent lookup in the same transaction; removing an exactness assertion over exact data is not a distinguishing mutation. |
| +- The new late-filing test and other store-sequenced tests establish specific boundaries, not two concurrently running consoles. I cannot certify the claimed executed mutation counts or exact transformations from their descriptions. |
| +- I found no new shared-cookie-jar, temporary-root or calendar-dependent assertion defect. Fixture timestamps and provider hooks do not establish real provider ordering. |
| + |
| +For **BBBB and LLLL**, I found no additional regression relative to `b260ac8e` in an honest-provider sequence established without the disputed early-subscription/replacement assumptions: |
| + |
| +| Sequence | Result | |
| +|---|---| |
| +| Own subscription arrives before the session names it | Retained; reconciliation retries, then records once association/payment checks pass. | |
| +| Duplicate delivery | Idempotent attachment/event handling; no second subscription obligation. | |
| +| Payment retries on the same session and subscription | Pending until acceptable payment and identity; then ordinary recording. | |
| +| Abandonment leaves a named subscription and the session expires | Held without a stage-one disposition, even if the subscription later reads `incomplete_expired`. | |
| +| Provider replaces one subscription with another for the intent | The extra recorded ID becomes a dispute when the accepted purchase records. | |
| +| Renewal/cancellation after recording | Does not consume a newer intent or dispute the purchase merely for being a later event. | |
| + |
| +Whether the problematic honest sequences occur, and what their financial consequences are, remains a provider rehearsal and settlement-policy question. |
| + |
| +**What must be verified against Stripe test mode before enablement** |
| + |
| +1. **All 36 catalog identities:** exact integer amounts, currencies, metadata, tax behavior and expanded own-only `currency_options`. Console and provisioning checks must accept the same legitimate shapes and reject mutations. |
| +2. **One-form purchases:** India plus explicit GBP, country-default choices, cleared suggestions and held legacy EUR. Confirm displayed base price, checkout currency, disabled Adaptive Pricing, subscription/item currency and stored identity. |
| +3. **Payment/event ordering:** subscription before completion, completion before subscription attachment, duplicate delivery, declined/retried cards, abandoned 3-D Secure and delayed payment methods. Establish how many subscription IDs Stripe creates and when. |
| +4. **Financial obligations:** determine what `incomplete_expired` and cancellation leave outstanding—payments, collectible invoices, pending collection and refunds. No status alone is settlement. Apply the owner’s ruling before enabling any release that depends on it. |
| +5. **Expiry versus payment:** retained links, expiry failure/timeouts, payment racing expiry and subsequent GET visibility. Proven expiry must prevent further payment; winning completion must remain accounted for. |
| +6. **Identity and idempotency:** GET/list/create/expiry fields, metadata correction, concurrent identical keys, lost responses, remembered errors and key-retention expiry. Confirm session IDs are not recycled. |
| +7. **Lost-create recovery:** original-key replay and customer-scoped pagination, including failures. Recovery must offer no link from a failed listing while retaining attributable evidence. |
| +8. **GBP overages and quantity:** creation, lost-response recovery, item/final-line verification, finalization/payment and quantity/proration updates must preserve GBP and correct allowance dating. |
| +9. **Legacy/customer reuse:** unchanged legacy EUR request shapes and hashes, renewal/cancellation, subsequent purchases in the held currency, and unchanged historical invoices. |
| +10. **Ended subscriptions:** establish whether fields other than metadata can change after cancellation, separating the defensive regression fixture from actual provider behavior. |
| + |
| +**MMMM:** acceptance still requires the two code fixes above. What remains is therefore **not only** the listed decisions, rehearsal and owner approvals. |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review23.md b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review23.md| new file mode 100644 |
| index 000000000..51fc0e456 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-multi-currency-review23.md |
| @@ -0,0 +1,103 @@ |
| +**Verdict: accept with fixes.** Every round-twenty-two finding is closed or honestly stated, and each new test fails without its fix. I found no P1 or P2. What remains is three P3s and three claims to correct; none needs a code change before the round is accepted. On that basis I would count review eighteen's P2 as closed at `2618403f`. |
| + |
| +This was source-only, read with `git show 2618403f:`. I modified nothing and ran nothing. The delta (`05ccec07..2618403f`) touches no pricing, form, quote, cookie, allowance-trigger, export or site code, so last round's conclusions on A–K, L–ZZZ2 and the site at `01b62f7` stand. |
| + |
| +## Round twenty-two, finding by finding |
| + |
| +| Finding | Closed? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Astra 1 (P2)**: an account could be retired under an open checkout dispute or refusal | **Closed.** `checkBillingRetirement` (`billingstore.go:1504-1521`) now refuses on an open `checkout_refused` or `checkout_fate_disputed` review. It runs in both the reservation (`:1533`) and `DeleteAccount` (`ownership.go:731`). | **No.** In `TestAnAccountIsNotRetiredUnderAnOpenCheckoutDisputeOrRefusal` (`billingnamed_test.go:711`), the old query would reserve in both held cases. The test also has a control. | |
| +| **Astra 2 (P2)**: a session refusal filed after another console consumed the intent was lost | **Closed.** The gone-intent branch now files a dispute against `billing_consumed_intents` (`billingstore.go:1256-1270`). | **No.** Without the fix the filing classifies an empty subscription, `purchaseOfSessionTx` never searches consumed purchases, and the count is 0. The test drives `fileRefusal` directly, not two consoles, as the dispositions say. | |
| +| **Astra 3 (P2)**: a click could return a pay link after a review opened mid-read | **Closed.** `st.Open \|\| st.Disputed` is checked after the first read (`billingservice.go:653`), after the second (`:717`), and before a new link is disclosed (`:780-785`). An undisclosed new session is still expired by the pass: `pinned()` prefers `intent.SessionID` (`billingretirement.go:255`). | **No.** First read: without the check the click goes on to expire the session, and the test asserts no expiry was requested and gets the wrong error. Second read and a new session: the URL is returned. | |
| +| **Astra 4 (P3)**: a broken listing page dropped the repeat and what followed it | **Closed** (`catalog.go:164-182`). Entries with no usable id are skipped, since nothing can be recorded without an id. On error the wrapper still hands the caller nothing. | **No**, for both the client test and `TestABrokenListingStillRecordsBothAnswers`. | |
| +| **Opus P2-1**: the dead subscription | Answered as a gate and a decision; see the next section. | No test, correctly: nothing changed in code. | |
| +| **Opus P3-1**: the unstated cleanup exception | **Closed.** It is now named in the header (`billingretirement.go:91-95`) and in the design. | n/a | |
| +| **Opus P3-2**: a subscription classified under its metadata's account | **Closed.** `accountOfIntentTx` (`billingretirement.go:762-782`) finds the account by intent id: standing, then consumed, then retired, then a resolution. The resolution's `subscription` column holds the intent id for refusals, as at `:639`. `readSubscription` records before reconcile's `AccountBySqid` can return. | **No.** Under both "no account" and "zzzz" the old code returned nil, and nothing was attached. | |
| +| **Opus P3-3**: the pass reported a hold it caused as a failure | **Closed at both sites** (`billingpass.go:267`, `:297-301`). | **No** for the `refuseSession` site. The recovery site has no test (P3-3 below). | |
| +| **Opus P3-4**: two keys for a dispute about a consumed intent with no session | **Closed in the classifier** (`billingretirement.go:660-664`), but the new filing uses a third key (P3-1 below). | **No.** | |
| +| **Opus claims 1–3** | All corrected: the exception is named twice, "held with no disposition" is at `DISPOSITIONS:484`, and the found defect is described as defensive. | n/a | |
| + |
| +**Where the reviewers disagreed on severity, Astra's reading was right on all three P2s:** |
| +- **Astra 1** released a case with an active hand-made subscription at the provider into an account no door reaches. |
| +- **Astra 3** offered a payable session that could not record, which takes money into a hold. |
| +- **Astra 2** is a P2 under the owner's "nothing observed is dropped" rule, though not on money: the purchase recorded on terms the other console verified. |
| + |
| +**My P2-1:** Astra listed it as residual, and it is a conditional P2. It needs provider behaviour that is not yet verified. |
| + |
| +## Opus P2-1 as a gate rather than code |
| + |
| +This is honest and sufficient for stage one: |
| +- Billing is off. |
| +- `ENABLEMENT.md` §6 step 2 (on the launch branch) makes "does Checkout ever create a Subscription before its session completes" the first check of the rehearsal, before anything else. |
| +- It says that if the answer is yes, decision 15 is needed and code must be built. |
| +- Reading an attached subscription's status in code now would be wrong. |
| + |
| +**Why reading status now would be wrong.** That is exactly what Astra's item 8 forbids: `incomplete_expired` or `canceled` is not settlement from status alone. The release would have to rest on a fresh read of the subscription's invoices and payments. |
| + |
| +**A correction to decision 15's proposal.** It proposes "a fresh read of the subscription and of its invoices showing none paid". That is too weak against item 8, because an invoice that is `open`, or whose payment is still `processing` (SEPA or Bacs), can still collect. The predicate should be: every invoice `void` (or `draft`), no succeeded or processing charge or PaymentIntent, and the subscription `incomplete_expired`. Anything else stays held. |
| + |
| +## Findings |
| + |
| +### P3-1. The late-refusal dispute uses a third key for an intent consumed with no session |
| +- **Where:** `billingstore.go:1263-1265` sets `usedSession = sessionID`. The record and the classifier (`billingretirement.go:660-664`) use the intent id. |
| +- **Scenario:** an intent is consumed with no session; no path does this today. A late session refusal then opens a dispute keyed by the session, and a later subscription opens a second one keyed by the intent. That is the same split P3-4 closed. |
| +- **Fix:** use `intent.ID`. |
| + |
| +### P3-2. Evidence first seen after an account is deleted lands where no operator looks |
| +- **Where:** `accountOfIntentTx` and `classifySubscriptionTx` do not check `accounts.deleted_at`. |
| +- **Scenario:** |
| + 1. An account's purchase ended and the account was retired; the gate now passes only with no open checkout review. |
| + 2. A hand-made H naming the consumed intent is read afterwards. |
| + 3. A dispute is filed on the deleted account. Astra noted at `05ccec07` that the review listing excludes deleted accounts, and the resolution door answers 404. |
| + 4. Reconcile then answers a bare identity refusal. |
| +- **Effect:** the row is durable but invisible. This is the only remaining door by which seen evidence ends as nothing but a rejected event, beyond the two stated exceptions. |
| +- **Fix:** list reviews of deleted accounts in the operator listing, or state the limit beside the two exceptions. |
| + |
| +### P3-3. The recovery site's filter of the hold it caused has no test |
| +- **Where:** `billingpass.go:297-301`. |
| +- **Effect:** removing that filter survives `TestThePassDoesNotFailOnAHoldItCaused`, which reaches only `:267`. The dispositions' "all nine die" covers one site. |
| +- **Fix:** add a case where recovery's refusal leaves a hold. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **`billingservice.go:790-793`** says a stillborn intent "is discarded unconditionally". `discardUnreadyIntent` refuses under an open review or a named subscription. Astra raised this at `05ccec07` and it was not answered. |
| +2. **`billingretirement.go:965-966`** says a retried refusal is due again "an hour later at the earliest". The schedule (`:799`) is 60 s, 300 s, 900 s, then 3600 s. Also Astra's, also unanswered. |
| +3. **The dispositions' "Where" for Opus P2-1** cites `DECISIONS-FOR-PAUL.md`, which is not in `2618403f`. It is on the launch branch, as decision 15. |
| +4. **`ENABLEMENT.md` §6.2** says "if no, the rule costs an honest buyer nothing". That holds only if the provider also never names one intent from two subscriptions by any other means (test-mode items 5 and 6 below). Qualify it. |
| + |
| +## KKKK, LLLL and MMMM |
| + |
| +**KKKK, every door out of the protocol's sight:** |
| + |
| +| Door | Can seen evidence still end as nothing but a rejected event or a retired account? | |
| +|---|---| |
| +| Account retirement (reservation and `DeleteAccount`) | No longer while anything seen is open. After deletion, only through P3-2. | |
| +| The deletion's preparation | No: `discardUnreadyIntent` refuses under a review or an attachment. | |
| +| The operator's retire | No: it is held with anything attached; a later subscription is a dispute against the retirement archive. | |
| +| The record | No: the others seen become disputes; a late refusal is now one too. | |
| +| The click | No: it re-checks after each read and before disclosing a link. | |
| +| Recovery | No: partial and broken pages are recorded. | |
| +| The webhook's settled answers | No: each comes after `readSubscription` has recorded by intent. | |
| + |
| +The stated exceptions remain: an intent that expired in the ordinary way, and an unready intent cleaned up, each with nothing seen naming it. |
| + |
| +**LLLL, honest buyers.** Nothing in this delta is worse than at `b260ac8e` in any sequence the provider is known to produce: |
| +- The new retirement gate, the late-refusal dispute and the click's checks fire only on a review or a genuine disagreement. |
| +- A genuine disagreement needs a hand-made subscription, session metadata edited between reads, or two builds disagreeing during a roll. The roll rule covers the last. |
| + |
| +**MMMM.** Nothing in code is required before this round is accepted. The P3s and claims above are fixes to make, not gates. What remains is: |
| +- decision 15, with the corrected predicate; |
| +- the test-mode rehearsal and its gate; |
| +- the owner's approvals the dispositions list. |
| + |
| +## To verify against Stripe test mode before enabling |
| + |
| +Every earlier list stands. In order: |
| +1. **The gate:** in subscription mode, does Checkout create a Subscription before the session completes, and with what metadata? Run it with 3-D Secure abandoned, a declined card then another on the same session, and a delayed debit. |
| +2. **An abandoned `incomplete_expired` subscription:** what state are its invoices in, and does any charge or PaymentIntent exist? This is the input to decision 15. |
| +3. **SEPA or Bacs:** the session's status and payment status, and the subscription's, through a failed debit. |
| +4. **Event order:** `customer.subscription.created` against `checkout.session.completed`, and whether the session can read `open` once its subscription exists. |
| +5. **Metadata:** can a Checkout Session's or a Subscription's metadata be edited after creation, and by whom? |
| +6. **A canceled subscription:** can anything but its metadata change? |
| + |
| +**Verdict: accept with fixes** (P3-1 to P3-3 and the four claims; the gate and decision 15 before enabling). |