amadan
Retirement protocol design, revision 2: Astra's first review answered (proof defined, fates immutable with a dispute state, complete told from paid, the operator's lock and conditional writes, bounded retries, migration, the never-created case); design only
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+160
−54
docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md+38 −54docs/superpowers/specs/CODEX-FINDINGS-intent-retirement-design-review1.md+122 −0
CI passed — run details
diff --git a/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md b/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md| index 0fbcbfa6d..a9d473ad7 100644 |
| --- a/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md |
| +++ b/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md |
| @@ -1,81 +1,65 @@ |
| -# The retirement of a refused checkout intent: a durable protocol |
| +# The retirement of a refused checkout intent: a durable protocol (revision 2) |
| -Status: design, written 2026-10-02 on `feat/carloku-multi-currency-20261001` after review eighteen (Opus P2-1). Nothing here is built. Billing is OFF. It supersedes the ordering arguments of reviews thirteen to eighteen about when the bound session is expired relative to the review: both orders have been shown wrong by an interleaving, and the fix is not a third order but a protocol with recorded steps. |
| +Status: design, revision 2, written 2026-10-02 on `feat/carloku-multi-currency-20261001`. Revision 1 (`8c72e88b`) was rejected by Astra's design review (`CODEX-FINDINGS-intent-retirement-design-review1.md`: four P2s, five P3s); this revision answers each finding in place. Nothing here is built. Billing is OFF. It replaces the ordering arguments of reviews thirteen to eighteen about when the bound session is expired relative to the review: both orders were shown wrong by an interleaving, and the fix is a protocol with recorded steps, not a third order. |
| -## The problem, stated once |
| +## The problem |
| -An intent is bound to one Checkout session the console created. When the console learns the session is not the intent's purchase (another currency, another price, a hand-made subscription, a session read back as nobody's), the intent must stop being payable and a person must dispose of it. Three facts live in three places: |
| +An intent is bound to one Checkout session the console created. When the console learns the session is not the intent's purchase (another currency or price, a hand-made subscription, a session read back as nobody's), the intent must stop being payable and a person must dispose of it. Three facts live in three places: the **refusal** (a review row keyed by the intent), the **session's fate** at the provider (open, expired, complete), and the **intent's end** (retired or kept). Today only the first is recorded; the code reasons about the second from the order of two calls, one of which can fail, and the third is taken on no evidence of the second. |
| -- the **refusal**, a review row (`billing_checkout_currency_refused`, keyed by the intent, the session as its reference), which holds the account and under which nothing records and no ordinary retirement runs; |
| -- the **session's fate at the provider**: open and payable, expired, or complete (paid); |
| -- the **intent's end**: retired (gone, with a record), or kept for a corrected purchase. |
| +## Vocabulary |
| -Reviews thirteen to eighteen moved the provider call around the review's filing. Filing after a successful expiry (rounds thirteen to sixteen) let a pass retire the intent between the expiry and the filing with no record (Astra, round seventeen). Filing before the expiry (round seventeen) lets the operator retire an intent whose session is still open and payable when the expiry failed, so money is taken after the operator recorded it settled (Opus, round eighteen). Each order leaves one window because the session's fate is never recorded: the code reasons about it from the order of two calls, one of which can fail. |
| +- **Bound session**: the id in `billing_checkout_intents.session_id`, written when the console created the session and judged it its own. |
| +- **Referenced session**: for an intent that was never bound (the create response was lost; recovery found the session by listing), the id in the review's `reference`. The intent is identified with exactly that id from the filing on; a later binding to another id is a contradiction (below), never a replacement. |
| +- **Never created**: `session_id = ''`, `customer_ready = 0` and no review reference. Only this, checked inside the resolution's transaction, means no session exists for the intent. |
| +- **Proof of a fate**: a session object from the provider (an expiry answer, a GET, a listing entry; never an error's text or code) that names the exact session id, the console's mode, the intent's id in `checkout_intent`, the account in both `client_reference_id` and `account_sqid`, and the intent's customer once it has one; then `status = expired` with no subscription and `payment_status` not `paid` (fate `expired`), or `status = complete` (fate `complete`, recorded with its `payment_status` and subscription id). An answer that fails the identity clauses proves nothing and is a contradiction of identity (below). After an expiry request that answers with an error, the fate is taken from a GET, never from the error. |
| +- **Authority to request an expiry** is the bound or referenced id alone (the console's own id); **authority to prove a fate** is the full proof. The two are distinct: a session the console may expire can still read back as nobody's. |
| ## The protocol |
| -Three durable steps, each recorded in the console's store, each idempotent, each performable by any of the four actors (a buyer's click, the pass, a webhook, the operator), in this order and only this order: |
| +1. **Refusal recorded.** The review `billing_checkout_currency_refused` is filed, keyed by the intent, with the session id as reference. From this instant the intent is refused: it stands; nothing records on it (the record's `consumeIntent` clause); no ordinary retirement runs (`retireIntent`'s open-review clause); no resume path offers its session (new); every actor that meets it works toward step 2. |
| -1. **Refusal recorded.** The review is filed, keyed by the intent, naming the session. From this instant the intent is `refused`: it stands (no second purchase opens beside it), nothing records on it, no ordinary retirement touches it, the resume path offers its session to nobody, and every actor that meets it works toward step 2. This is today's review row; it is authoritative until step 3. |
| +2. **The session's fate proven and recorded.** `billing_checkout_intents` gains `session_fate` (`''` | `expired` | `complete`), `session_fate_at`, `session_payment_status`, `session_subscription`, `fate_disputed_at`, `expiry_next_try_at`, `expiry_tries`. A fate is written once, by a conditional transactional write against the exact intent id and session id, from a proof as defined; it never changes. A later proof that contradicts it (expired then complete, or any identity contradiction on the same id) sets `fate_disputed_at` and files `billing_checkout_fate_disputed` for the intent; a dispute blocks every disposition and every record until a person resolves it (below). The provider's truth is not decided by whichever writer came first: a disputed intent needs a fresh proof at resolution time. |
| -2. **The session's fate proven and recorded.** The intent row gains `session_fate` (`''` | `expired` | `complete`) and `session_fate_at`. It is written once, from provider evidence that names the bound session id and the account (the same proof `retireIntent` demands today: the session's id, mode, `checkout_intent` and account metadata; `expired` with no subscription and not paid; or `complete`). Evidence comes from: an expiry answer; a read of the session (the pass, a resume, a listing); the completion webhook; the subscription webhook's read. `expired` and `complete` are final; a later contradiction (an expired session read back as complete) is a provider contradiction and files its own review, as a session read back as nobody's does. |
| + Until a fate is recorded, the expiry is requested by the bound or referenced id under one fixed idempotency key (`carloku-expire-<intent id>`) by the reader that files the review, by the pass, and by the operator door on `retire`. The pass's retries are scheduled per intent (`expiry_next_try_at` with backoff 1, 5, 15, 60 minutes, capped) under their own budget and cursor, separate from the pass's other work, so slow expiries of the oldest intents cannot starve subscription reconciliation; they stop at a recorded fate or a resolution; an answer that arrives after that is validated conditionally and recorded only as a contradiction if it is one. A buyer who already holds the URL can still pay until the provider has expired the session: that payment is a `complete` fate and goes to the operator's completed-session disposition, never to a record. |
| - Until the fate is `expired` or `complete`, the console **requests the expiry** by the bound id under one fixed idempotency key (`carloku-expire-<intent id>`, as today), from every actor that meets the refused intent: the reader that files the review (the same call it makes now), every pass while the review is open and the fate is unproven (new: today the pass does nothing for an open session that is still the intent's purchase), and the operator door on a `retire` (new). One key means the provider expires once and answers every later request the same way; an answer that the session is complete is the proof `complete`. The request's failure is not a state: nothing is recorded for it, and the next actor asks again. The console never records `expired` from its own request having been sent; only from an answer or a read. |
| - |
| -3. **Local completion, atomic.** The operator's resolution ends the intent in one transaction, and the transaction refuses unless the fate on the row allows the outcome: |
| - - `retire`: the fate is `expired`, or the intent was never bound (no session). The intent is deleted, `billing_retired_intents` written, the quantity events closed, as today. With the fate unproven: the door first requests the expiry (step 2) and records the answer; a request that fails answers 503 and the review stays open; a fate still unproven after the answer (the answer named another session) answers 503. With the fate `complete`: 409, "the session was paid; settle or keep". |
| - - `settled` (new): the fate is `complete`. The operator attests that the money was handled at the provider by hand (refunded, or accepted as a one-off). The intent is retired exactly as `retire` retires it, with the outcome recorded as `settled`; the paid session's later events settle as a retired intent's do today. A paid session is never called expired. |
| - - `keep`: the fate is `complete`, the bound session reads back as the intent's own purchase with a subscription (today's gate), and the corrected purchase records afterwards; the review is closed and the intent stands. |
| - - Any outcome on a review already resolved: the exact replay rule, as today (200 for the same actor, note and outcome; 409 otherwise). |
| - |
| - The resolution transaction reads the fate from the row under the same write lock that the fate's writers take (the store's one connection), so a fate recorded after the operator looked and before the transaction ran is what the transaction sees: a completion that lands between the operator's read and their `retire` turns the `retire` into a 409, not a retirement of a paid intent. |
| +3. **Local completion, atomic and operator-only.** Only the operator's resolution ends the intent; the other actors advance evidence. The door takes the account's billing lock (`s.billing.locks.lock(acct.Sqid, "billing")`, held across the gate and the resolution and never re-entered), reads the review first and sends any already-resolved occurrence to the store's exact-replay comparison before touching the provider or the intent, for every outcome. The resolution's transaction then reads the fate and the dispute flag from the row and refuses unless they allow the outcome: |
| + - `retire`: fate `expired` and not disputed; or never created (the three-column check in the transaction). The intent is deleted; `billing_retired_intents` gains the fate, its time, its proof's session id, `payment_status` and the protocol version; the exact intent's pending checkout quantity events are closed; nothing else. With no fate: the door requests the expiry and takes the fate from the GET that follows; a request or read that fails answers 503 and the review stays open; a fate `complete` answers 409 "paid: settle or keep". |
| + - `settled`: fate `complete`, not disputed. The operator attests what was done at the provider about the money and the subscription the session created, if any: the door reads that subscription and requires it canceled (or no subscription), else 409 "cancel the subscription at the provider first, or keep". The intent is retired as `retire` retires it, with the outcome `settled` recorded; its later events settle as a retired intent's do. Accepting money without recording a purchase, as a one-off, is not a disposition this design offers (decision 1 below). |
| + - `keep`: fate `complete` with `payment_status` `paid` or `no_payment_required`, not disputed, and the bound session reading back as the intent's own purchase with a subscription (today's gate). The review closes; the intent stands; the corrected purchase records. |
| + - A dispute: no outcome is accepted until the door, at resolution time and under the lock, obtains a fresh proof of the same id whose fate matches the outcome asked for and whose identity clauses all hold; the dispute review is resolved in the same transaction with the proof recorded. An identity contradiction that the provider does not correct (the metadata stays another account's) has no disposition here; it is held with its review (decision 2 below). |
| + - Replays: the same actor, note and outcome on a resolved occurrence answers 200 without writing; anything else 409. An omitted outcome is `retire`. An outcome on a review of another reason is 400. With billing off: `retire` of a never-created intent is accepted; every other outcome needs the provider and answers 409. |
| ## Each actor, each state |
| -| Actor meets a refused intent whose fate is… | unproven | expired | complete | |
| -| --- | --- | --- | --- | |
| -| Buyer's click (resume) | the page says the purchase is being reviewed; no URL is offered; no new intent | same | same | |
| -| Pass | reads the session; records the fate if the read proves one; else requests the expiry and records the answer's fate | nothing (retirement is the operator's) | nothing | |
| -| Webhook (completion, subscription) | records `complete` from the session read; records no purchase (the review is open); answers 204 | a completion of an expired session: provider contradiction, review | records nothing; 204 | |
| -| Operator `retire` | requests the expiry; records the fate; retires on `expired`, 503 on failure | retires | 409: settle or keep | |
| -| Operator `settled` | 409 | 409 | retires with outcome `settled` | |
| -| Operator `keep` | 409 | 409 | today's gate (own purchase, subscription) | |
| - |
| -An unbound refused intent (the create response was lost and the listing found the session paid: the one case `fileCheckoutRefusedReview` has with no bound id) has its session only as the review's reference; the fate is proven from reads of that id, and `retire` requires it as for a bound one. |
| - |
| -## Two more things review eighteen named, settled by the same protocol |
| +| An actor meets a refused intent whose fate is | unproven | expired | complete | disputed | |
| +| --- | --- | --- | --- | --- | |
| +| Buyer's click (resume, start) | no URL; the page says the purchase is under review; no new intent | same | same | same | |
| +| Pass | GET; records the fate from a proof, else requests the expiry when `expiry_next_try_at` has passed and records what the GET after it proves | nothing | nothing | nothing | |
| +| Webhook (completion, subscription) | in one transaction: the fate or contradiction from the event's session read, the rejected-event row with the account, the seen-event row; then 204. A failed write is retried by redelivery | a completion of an expired session: dispute, in the same transaction, then 204 | nothing new; rejected-event row; 204 | rejected-event row; 204 | |
| +| Operator `retire` | requests the expiry, records the fate, retires on `expired` | retires | 409 | 409 until a fresh proof | |
| +| Operator `settled` | 409 | 409 | retires with `settled` after the subscription check | same | |
| +| Operator `keep` | 409 | 409 | today's gate plus `payment_status` | same | |
| -- **Concurrent identical resolutions** (Astra 2): two identical `keep` requests both read the review open; the first commits; the second's gate then finds no standing intent (the corrected purchase recorded meanwhile) and answers 409 for a resolution that is already committed. The door's gate and its resolution run under the account's billing lock (the lock every writer of the intent takes), and a gate refusal or a provider failure is answered only after a second read of the review: an occurrence resolved meanwhile goes to the store's exact replay comparison (200 for the same actor, note and outcome; 409 otherwise). A controlled two-request test establishes it. |
| -- **A retired session's completion with a replacement intent standing** (Astra 3): the binder judges the late completion against the replacement, fails ownership, and settles it with no account on its rejected row. The account-scoped retirement check runs for every completion that does not record, before any judgement against a standing intent, and returns the typed identity error that carries the account, as reconcile does; the late-completion fixture inspects the rejected row. |
| +Stale observations: a refusal filing or a fate write carries the instant of the read it came from, and the conditional write refuses an observation older than the review's last resolution or than the fate already recorded; a pass that paused across a `keep` files nothing when it resumes. Two consoles are correct through these database conditions, not through either's service lock. |
| ## What does not change |
| -- `ownSession` and `intentSession` as the one judgement of a session on every path. |
| -- Nothing records under an open review; the record's transaction consumes the exact standing intent (`consumeIntent`). |
| -- A retired intent's events settle (`billingIntentRetired`), and are listed for the operator (`GET /api/billing/rejected`). |
| -- The pass's expiry retirement of an intent with no review (`expireBillingCheckout` with proof) is untouched; it already refuses under an open review. |
| -- The provider is read only through the console's own bound id; no session is expired that the console did not bind. |
| +`ownSession` and `intentSession` as the one judgement; the record consumes the exact standing intent and refuses under an open review; a retired intent's events settle and are listed; the pass's expiry retirement of an intent with no review (`expireBillingCheckout`, with proof) is untouched. The console requests an expiry only by an id it bound or validated in full (creation, listing); that is today's authority and stays. |
| ## Interleavings this closes |
| -- Expiry fails after the review (Opus eighteen): the fate stays unproven; `retire` requests the expiry itself and retires only on the answer; the pass retries meanwhile; the click offers no URL. |
| -- A pass between an expiry and the filing (Astra seventeen): the review is step 1, filed first; nothing retires under it. |
| -- The operator looks, the buyer pays, the operator retires: the completion recorded the fate `complete` before the resolution's transaction, which answers 409. |
| -- A reader's expiry answer and a webhook's completion for the same session: whichever records first wins the row's fate; the other, if it contradicts, files the contradiction review and records nothing. |
| -- Two operators: the second's resolution is a replay or a 409, as today. |
| -- A retired intent's session paid afterwards (impossible once `retire` requires `expired`; a `settled` intent's later events settle as retired, by the operator's attestation). |
| +An expiry that fails after the review (Opus and Astra eighteen): no fate, so `retire` must obtain one; the pass retries on its schedule; no URL is offered. A pass between an expiry and the filing (Astra seventeen): the review is step 1. The operator looks, the buyer pays, the operator retires: the completion recorded `complete` (or a dispute) before the resolution's transaction, which refuses. Two identical resolutions: the replay comparison first, under the lock. A retired session's late completion: the retirement record carries the fate, so a completion after `expired` is a dispute on the record and a completion after `settled` is the expected late event; both are rejected-event rows with the account. Process death between any two steps: each step is a durable row; the next actor continues from it. |
| -## Schema and migration |
| +## Migration |
| -`billing_checkout_intents` gains `session_fate TEXT NOT NULL DEFAULT ''` and `session_fate_at INTEGER NOT NULL DEFAULT 0` (`addColumnIfMissing`). `billing_review_resolutions.outcome` gains the value `settled`; an older row reads `retire`. A refused intent from before this build has an unproven fate: its `retire` requests the expiry and proves it, as any other. |
| +New columns by `addColumnIfMissing`. Resolution rows gain `evidence` (`''` for rows from before this protocol, else the fate the resolution rested on); an old `retire` is never reinterpreted as `settled`, and its exact replay still answers 200. An open review from before this build has no fate; its `retire` obtains one. An older operator client that sends no outcome asks for `retire`, which now needs an `expired` fate: on a paid session it answers 409 and the person chooses `settled` or `keep`. The roll note forbids an older console binary after the first fate is written (the older binary's `retire` would skip the proof). |
| -## What must be verified in Stripe test mode (approval E) |
| +## Stripe test mode (approval E) |
| -1. Expiring an already expired session under the same idempotency key answers the expired session (a replay), and under a new key answers an error naming its status; the console records `expired` from either. |
| -2. Expiring a completed session answers an error naming its status; the console records `complete` from the read that follows, never from the error alone. |
| -3. A session completed between the console's read and its expiry request: the expiry fails, the completion webhook arrives, the fate is `complete`. |
| +At the pinned API version: a successful expiry prevents payment afterwards, including a payment racing with it and a retained URL; the proof fields are present on expiry, GET and listing answers, including after a metadata edit; the same key concurrently, a successful replay, a remembered error, a lost answer, and behaviour after the key's retention ends; `complete` against `payment_status` for delayed payment methods and `no_payment_required`; the lost-create, referenced-session recovery; refund and cancellation behaviour for `settled`. Crash boundaries, duplicate delivery, two consoles and transaction rollback are established by controlled local interleaving tests, not by the rehearsal. |
| -## Not decided here |
| +## Decisions for Paul |
| -- The copy the buyer sees when their purchase is under review (Paul's). |
| -- Whether the operator door should also let a person request the expiry without resolving (a `expire` action with no outcome). The protocol does not need it: `retire` requests it. |
| +1. Whether money may ever be accepted without the purchase recording (a one-off). This design says no: a paid wrong-currency session is refunded and `settled`, or corrected and `keep`. |
| +2. The route for a session whose identity the provider keeps contradicting after the console expired it: wait for the provider record to be corrected and re-read, or an exceptional disposition with its own evidence and audit. This design holds it with its review and offers no exception. |
| +3. The copy the buyer sees while their purchase is under review. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-intent-retirement-design-review1.md b/docs/superpowers/specs/CODEX-FINDINGS-intent-retirement-design-review1.md| new file mode 100644 |
| index 000000000..e6ddf0a00 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-intent-retirement-design-review1.md |
| @@ -0,0 +1,122 @@ |
| +References are at **`8c72e88b`**. **Design** below means `docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md`. This review used source inspection only; nothing was modified or tested, and no network or AWS commands were run. |
| + |
| +**Findings, most severe first** |
| + |
| +1. **P2 — “Never bound” permits retirement without proving that no session exists.** |
| + |
| + **Where:** Design:26, :44; `internal/console/billingstore.go:483`; `internal/console/billingservice.go:1335`. |
| + |
| + **Scenario:** Create succeeds but its response is lost. Recovery discovers the paid, wrong-currency session and files a review with its ID as `reference`; `session_id` remains empty. Line 26 permits `retire` because the intent was never bound, while line 44 requires terminal proof. Implementing the former retires a paid purchase without the new `settled` attestation. |
| + |
| + Empty `session_id` also does not prove that creation never happened. The existing stillborn-intent cleanup requires `customer_ready=0` for precisely that reason. |
| + |
| + **Smallest acceptable change:** Remove the unbound exception as written. Define three cases explicitly: bound session; unbound but identified session in the review; and provably never-created checkout. The last requires a transactional check of `session_id=''`, `customer_ready=0`, and no reviewed session reference. For the second, pin the reference and require the same ownership and terminal-state proof against that exact ID. Resolve disagreements between a later binding and the reference conservatively. |
| + |
| +2. **P2 — The first terminal observation remains actionable even after contradictory evidence arrives.** |
| + |
| + **Where:** Design:21, :26, :31, :64, :70; `internal/console/billingstore.go:177`, :812, :1005; `internal/console/billingservice.go:820`. |
| + |
| + **Scenario:** An expiry answer records `expired`. Before resolution, another actor reads the same session as `complete`. The design retains `expired` and files a separate contradiction review. Nevertheless, the stated `retire` precondition still passes. Line 31’s promise that the intervening completion produces 409 does not hold: line 21 prohibits changing the fate to `complete`. |
| + |
| + There is another gap after retirement. The fate and timestamp disappear with the intent; the retirement record currently preserves neither. A later completion reaches the ordinary “retired intent” acknowledgement path, which cannot distinguish a legitimate delayed event after `settled` from a contradiction of proven expiry. |
| + |
| + **Smallest acceptable change:** Keep terminal observations immutable, but add a durable disputed state or equivalent blocking condition. A contradiction must disable ordinary resolution and consumption in their transactions. Preserve the terminal evidence, session identity and protocol version with retirement. Late contradictory events must create an actionable hold before acknowledgement, rather than merely becoming rejected events. Define an explicit procedure for resolving disputed evidence; “first writer wins” cannot decide the provider’s truth. |
| + |
| +3. **P2 — `complete` is treated as proof of payment, while settlement does not cover future charges.** |
| + |
| + **Where:** Design:10, :21, :27–28; `internal/console/billingservice.go:967`; `internal/billingstripe/client.go:536`, :620; `internal/console/billingoperator.go:248`. |
| + |
| + **Scenario:** A session is complete but its payment remains pending. The design records `complete` and enables both `settled` and `keep`. Current reconciliation separately requires `payment_status` to be `paid` or `no_payment_required`; the proposed `keep` gate does not. Closing the review can therefore leave a purchase that still cannot record, with its payment outcome unresolved. |
| + |
| + Separately, an operator refunds the initial payment and selects `settled`, exactly as line 27 permits, but leaves the subscription active. Subsequent events are deliberately discarded as retired. Calling the original payment “accepted as a one-off” likewise does not define what happens to recurring charges. |
| + |
| + The hand-made-subscription case has the same separation: expiring the bound checkout proves nothing about money already collected through the subscription that triggered the review. |
| + |
| + **Smallest acceptable change:** Define checkout completion separately from payment settlement. Specify payment-status requirements and handling of pending payments. `settled` is an appropriate separate disposition, but its attestation must address the named subscription, outstanding payment obligations and future collection—not merely the initial payment. State what `retire` requires when the refusal concerns another paid subscription. |
| + |
| + Whether accepting money without recording the purchase is allowed, and what service that buys, are **owner decisions**. They should not be established implicitly by “accepted as a one-off.” |
| + |
| +4. **P2 — A session read back as nobody’s has no defined route to completion.** |
| + |
| + **Where:** Design:7, :21–28; `internal/console/billingservice.go:290`, :321; `internal/console/billingstore.go:445`, :474. |
| + |
| + **Scenario:** The stored bound session now carries conflicting account metadata. The console expires its trusted bound ID successfully, but every subsequent response retains that conflicting metadata. Strict ownership proof correctly refuses to record either fate. Consequently, `retire`, `settled` and `keep` are all unavailable indefinitely. |
| + |
| + Conversely, trusting an earlier cached expiry answer with the original metadata could permit retirement despite the later identity contradiction. The design does not say which evidence controls that case. |
| + |
| + **Smallest acceptable change:** Distinguish authority to request expiry from authority to prove fate. Enumerate the proof fields: exact session ID, provider mode, intent metadata, both account identifiers and known customer; then the terminal-state predicates. Explicitly state that identity contradictions cannot be repaired by replaying older matching evidence. Define the recovery procedure—provider identity correction followed by verification, or a separately approved exceptional disposition with its own evidence and audit. The choice belongs to the owner. |
| + |
| +5. **P3 — The locking description misses stale work that can reopen a completed `keep`.** |
| + |
| + **Where:** Design:31, :48; `internal/console/store.go:65`; `internal/console/billingpass.go:180`; `internal/console/billingservice.go:1100`, :1121; `internal/console/billingstore.go:1026`. |
| + |
| + **Scenario:** A pass reads a completed session with contradictory metadata and pauses before filing. Another actor files the review; the metadata is corrected; the operator performs `keep`. Before the corrected purchase records, the delayed pass files its stale observation. Because the intent still stands and the review is resolved, `fileCheckoutRefusedReview` opens another occurrence. This can happen within one console because the pass’s initial ownership/refusal work takes no billing lock; two consoles also have independent service locks. |
| + |
| + The narrower atomicity claim is sound **within one `Store`**: a transaction reserves its single connection, so another writer using that handle cannot alter the fate between its transactional read and commit. That does not serialize provider reads, work before the transaction, or another console’s connection. |
| + |
| + **Smallest acceptable change:** Name the door’s lock explicitly: `s.billing.locks.lock(acct.Sqid, "billing")`, covering the gate and resolution without recursively entering locking helpers. Inventory pass, expected-intent binding and recovery paths that currently operate outside it. Define transactional conditional writes against the exact intent, session/reference and review occurrence, including how stale observations after `keep` are discarded or freshly verified. Two-console correctness must rest on database conditions and retry handling, not the service mutex. Transactional helpers must use `tx`, not attempt a second store query through the occupied connection. |
| + |
| +6. **P3 — The retry work is not fairly bounded, and a fixed key does not make it so.** |
| + |
| + **Where:** Design:23, :38; `internal/console/billingpass.go:31`, :34, :61, :135; `internal/billingstripe/client.go:38`. |
| + |
| + **Scenario:** The oldest reviewed intents have fast successful GETs but expiry requests that repeatedly time out. Roughly eight 15-second expiry calls exhaust the pass’s two-minute budget. Every pass starts again with those same oldest intents, so later pending purchases and subscription reconciliation can remain unprocessed. Previously, otherwise-valid open sessions took the inexpensive no-op branch. |
| + |
| + **Smallest acceptable change:** Specify a bounded, fairly scheduled retirement-retry batch, with per-intent retry timing and an independent budget or cursor. Keep the fixed key and fixed target ID. Stop retries after terminal proof or resolution. Define requests already in flight at that point as harmless late work whose answers still undergo conditional validation. |
| + |
| + Refusing a resume URL prevents another disclosure; it does **not** revoke a URL the buyer already possesses. The design must explicitly permit payment to win until provider expiry is proven and route that result to the completed-session disposition. |
| + |
| +7. **P3 — Migration preserves the spelling `retire` but leaves its changed meaning undefined.** |
| + |
| + **Where:** Design:29, :70; `internal/console/billingstore.go:171`, :227, :878, :937, :971; `internal/console/billingoperator.go:353`. |
| + |
| + **Scenario:** An earlier build records `retire` for a paid session whose money the operator handled. After migration, the design describes `retire` as evidence of expiry, although this historical row proves no such thing. Reinterpreting it as `settled` would alter immutable history and break an identical old-client replay. |
| + |
| + An older client submitting no outcome against a newly reviewed paid session also needs an explicit answer: its former default can no longer authorize the same action. |
| + |
| + **Smallest acceptable change:** Preserve historical outcomes and exact replay identity, but distinguish legacy retirement from protocol-proven expiry, using a version or explicit absence of evidence. Never infer fate from an old outcome. Specify that omitted outcome remains `retire`; an already-committed matching legacy replay succeeds, while a new paid-session retirement conflicts and requires explicit `settled`. Run replay comparison before provider access or current-intent prerequisites for **all three outcomes**. Also define behavior with billing disabled and for outcomes submitted on unrelated review reasons. |
| + |
| +8. **P3 — The webhook table does not specify durable rejection records before 204.** |
| + |
| + **Where:** Design:39, :55; `internal/console/billinghandlers.go:251`, :314; `internal/console/billingstore.go:1325`; `internal/console/billingoperator.go:357`. |
| + |
| + **Scenario:** Following the table literally, a webhook finds an already-recorded `complete` fate, “records nothing,” and returns 204. Its event never reaches the rejected listing. Alternatively, marking the event seen before durably recording a required contradiction would make a crash and redelivery skip that contradiction. |
| + |
| + There are also two different audit mechanisms today: the resolution-history append is transactional; the general operator audit happens afterwards and repeats on successful HTTP replays. |
| + |
| + **Smallest acceptable change:** Specify the durable effects before acknowledgement: any required fate/contradiction record, plus the idempotent rejected-event and seen-event records, with account attribution wherever established. A failed durable write must remain retryable. Explicitly name the resolution-history row as the atomic audit, or include a durable audit-delivery mechanism if the general audit must have that guarantee. |
| + |
| + State that `retire` and `settled` close only pending checkout quantity events for the exact intent and archive that intent; `keep` does neither. Neither disposition should erase previous rejection history. |
| + |
| +9. **P3 — The Stripe verification list omits assumptions on which safety depends.** |
| + |
| + **Where:** Design:21, :23, :74–76; `internal/billingstripe/client.go:63`, :224, :296, :676. |
| + |
| + **Scenario:** An expiry request returns an error or an answer remembered under the shared key. Line 74 allows recording `expired` from an error, but the current client’s error contains no session/account ownership proof. Line 23 also assumes indefinite identical replay, without defining recovery when a remembered failure persists or the provider no longer retains the key. |
| + |
| + **Smallest acceptable change:** Require a validated session object for either terminal fate. After any expiry error, obtain that evidence through GET; status text or an error code alone cannot satisfy line 21. |
| + |
| + Expand the Stripe test-mode requirements, at the pinned API version, to cover: |
| + |
| + - Terminal exclusivity: successful expiry prevents subsequent payment, including payment already racing with expiry and retained checkout URLs. |
| + - Full proof fields on expiry, GET and listing responses, including after metadata edits. |
| + - Same-key concurrency, successful replays, remembered errors, lost responses, and behavior after key retention ends. |
| + - Completion versus `payment_status`, delayed payment methods and `no_payment_required`. |
| + - The lost-create, unbound-reference recovery path. |
| + - Refund/cancellation and future-collection behavior for the approved settlement policy. |
| + |
| + The design’s three existing checks are necessary but insufficient. Crash boundaries, duplicate delivery, competing consoles and transaction rollback additionally need controlled local interleaving tests; a Stripe rehearsal alone cannot establish them. |
| + |
| +**Claims the design makes that the code contradicts** |
| + |
| +- **“The lock every writer of the intent takes” — Design:48.** The pass can file refusal reviews, retire expired intents and save recovered bindings without taking that service lock (`billingpass.go:180`, :212, :257). Expected-intent ownership handling also precedes the binder’s lock (`billingservice.go:1100`, :1121). |
| + |
| +- **“The provider is read only through the console’s own bound id” and “no session is expired that the console did not bind” — Design:57.** Recovery lists customer sessions, and `refuseSession` expires validated unbound sessions from creation or listing (`billingservice.go:364`, :378, :1354, :1376`). The actual authority includes fully validated unbound sessions. |
| + |
| +- **“Complete (paid)” — Design:10.** Current reconciliation checks payment status separately (`billingservice.go:967`). Neither `Session.Status` nor today’s `keep` gate establishes payment by itself. |
| + |
| +- **“The console records expired from either [success or error]” — Design:74.** `ExpireCheckout` returns no session on an API error, and the sanitized error lacks the required identity evidence (`internal/billingstripe/client.go:224`, :681). |
| + |
| +- **“Each [step] performable by any of the four actors” — Design:17.** Local resolution is operator-only; the store verifies operator identity (`billingstore.go:918`). The other actors may advance evidence collection, not authorize the operator’s disposition. |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |