carlos / platform Public

Review twenty-three answered: a point of no return for an account's retirement, deleted accounts' reviews kept in sight, and one test's expectation brought up to date

Round twenty-three: Opus accept with fixes (no P1 or P2; "I would count
review eighteen's P2 as closed"), Astra reject (one P2, one P3). The
gate on 2618403f failed on one existing test whose expectation round
twenty-two's own fix (Opus P3-2) made out of date; it asserts both holds
now.

- commitBillingRetirement: the retirement's point of no return, in the
  database, after the reservation and before the first write outside.
  Filing a checkout refusal or dispute withdraws a reservation that is
  not committed, in the evidence's transaction; the commit re-reads every
  condition and refuses if the reservation went. Evidence after the
  commit stays with the account, whose deletion and reap finish (Astra 1).
- The operator doors look accounts up whether or not they are deleted;
  the listing marks a deleted account's review (Opus P3-2).
- The listing client decodes each session on its own and returns the
  ones that decoded with the decode error (Astra 2).
- One key for a late refusal of an intent consumed with no session
  (P3-1); the recovery's hold tested (P3-3); two comments and the
  dispositions' pointer corrected.

Eleven mutants in a tree copy; two needed their own test case. Review
eighteen's P2 stays recorded as open until a round ends with both
accepting.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev ef8d9440619fdcac442139299244a76c868a5b40 parent 2618403
12 files changed, +443 −37
  • docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md +1 −1
  • docs/superpowers/specs/DISPOSITIONS-multi-currency.md +21 −1
  • internal/billingstripe/catalog.go +26 −14
  • internal/billingstripe/catalog_test.go +21 −0
  • internal/console/billingcurrency_test.go +10 −3
  • internal/console/billingnamed_test.go +224 −0
  • internal/console/billingoperator.go +8 −5
  • internal/console/billingretirement.go +8 −2
  • internal/console/billingservice.go +3 −2
  • internal/console/billingstore.go +99 −7
  • internal/console/ownership.go +17 −1
  • internal/console/store.go +5 −1

CI cancelled — superseded by a newer push — 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 e0ac7bd07..5cedfbf20 100644
--- a/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md
+++ b/docs/superpowers/specs/2026-10-02-intent-retirement-protocol-design.md
@@ -125,7 +125,7 @@ Built in `internal/console/billingretirement.go` and the paths it touches, with
- **An unbound intent under a review that names no session** is still reconciled by the pass, which is how its session is found. Nothing records, retires or cleans up an intent under an open review on that path: the store's record, expiry and cleanup each refuse it. (Round twenty's build let the cleanup of an intent that never had a session delete it under its review; review twenty-one.)
- **Named subscriptions** (review twenty-one, answered as one rule). A subscription that names a standing intent is recorded from the first read, whether or not a refusal review exists. Nothing deletes an intent past one. The record of the purchase keeps what it consumed (`billing_consumed_intents`: the intent, its session, the subscription it recorded with) and in the same transaction makes every other subscription seen naming the intent a dispute; a subscription that names the intent afterwards is compared with that record and is a dispute. The expiry of the intent's session, and the cleanup of an intent that never had a session, do not delete an intent a subscription was seen naming: they file the refusal, with it attached, and the intent stands (an expired session has no subscription and an intent with no session never had a checkout, so that subscription was made outside checkout). `retire` is held while anything is attached, whether or not the intent still stands. What stays outside: a subscription first seen naming an intent that went with nothing seen naming it and no purchase (one that expired in the ordinary way; one that never had a session and was cleaned up) is a rejected event with its account and no hold, because nothing is kept of such an intent. What the rule costs: a subscription seen naming an intent holds that intent's expiry and cleanup whatever the subscription's own state, a dead one included, into a refusal stage one can only hold; and a second subscription at a purchase's record is a dispute stage one cannot resolve. If the provider ever makes a subscription before its checkout session completes, an honest buyer meets this and is held with no disposition; whether it does is the first check of the test-mode rehearsal, and a gate on enabling (review twenty-two).
- **Only a first purchase consumes an intent**: the record of a subscription that is not yet the account's recorded one, judged in the record's transaction. A later event of the account's recorded subscription deletes no intent, whatever its metadata names.
-- **A refusal filed late.** A refusal of a session filed for an intent that a purchase consumed meanwhile is kept as a dispute against what was consumed, in the filing's transaction. **Account retirement** refuses while a checkout refusal or dispute is open. **A click** reads the intent's state again after each provider read and before it discloses a new link, and offers nothing under an open review (review twenty-two).
+- **A refusal filed late.** A refusal of a session filed for an intent that a purchase consumed meanwhile is kept as a dispute against what was consumed, in the filing's transaction. **Account retirement** refuses while a checkout refusal or dispute is open, and has a point of no return in the database (`commitBillingRetirement`), taken before anything outside is written: checkout evidence filed before it withdraws the pending retirement; evidence filed after it stays with the account, whose deletion finishes, and is listed and resolvable under the deleted account (review twenty-three). **A click** reads the intent's state again after each provider read and before it discloses a new link, and offers nothing under an open review (review twenty-two).
- **Binding.** A session id is written onto an intent in one place, which refuses a session that is already another purchase's (another intent's, a retired, kept or consumed one's) and one that is not the session the intent's open review names.
- **Creating an intent** re-reads the account's customer and the currency it is held to in its own transaction; a request built from an older reading is refused.
- **The refusals' turn** takes the longest-due first. **A listing** that fails on a later page records the pages it read. **An answer carrying another session's id** records nothing.
diff --git a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md
index fa6ff3b7f..0b35d7df1 100644
--- a/docs/superpowers/specs/DISPOSITIONS-multi-currency.md
+++ b/docs/superpowers/specs/DISPOSITIONS-multi-currency.md
@@ -497,7 +497,7 @@ Reports: `OPUS-REVIEW-multi-currency-review22.md`, `CODEX-FINDINGS-multi-currenc
| Astra 2 (P2): a console that judged a session not the purchase and filed its refusal after another console recorded the purchase lost the refusal: the gone-intent branch had no subscription to classify and the session's purchase was not in any archive it searched. | Fixed: a refusal of a session filed for an intent that a purchase consumed is kept as a dispute against what was consumed, in the filing's transaction, whatever subscription it carried. | `billingstore.go` `fileCheckoutRefusal` | `TestARefusalOfASessionFiledAfterItsPurchaseRecordedIsKept` |
| Astra 3 (P2): a click that passed its first open-review check could return a pay link for an intent whose refusal another console filed while the click's read was in flight; the post-read checks looked at the dispute only, and a newly made session's link was disclosed with no check at all. | Fixed: after each read the state is read again and an open review or a dispute stops the click; a new session's link is disclosed only if the intent is not under review then (it stays bound, for the pass to expire). | `billingservice.go` start path | `TestAClickOffersNothingWhenAReviewOpensWhileItReads` (first read, second read, new session) |
| Astra 4 (P3): a page that repeated a session was refused before the repeat, and what followed it, was returned, so a second answer about an archived session was lost. | Fixed: the client returns everything the page decoded with the pagination error; the wrapper records it all. | `billingstripe/catalog.go`, `billingretirement.go` `listCheckouts` | `TestABrokenListingStillRecordsBothAnswers`; `TestListCheckouts_ABrokenPageStillReturnsWhatItSaid` |
-| Opus P2-1: an honest buyer whose provider left a dead subscription naming the intent (an abandoned authentication; a declined card then another) is held with no stage-one disposition; the dispositions said "waits on a person", and no person can act. | **Not changed in code; made a gate and a decision.** The code does not read an attached subscription's status, and whether `incomplete_expired` is nothing owed is a ruling (the test-mode list already asks what it leaves outstanding). Whether the provider ever makes a subscription before its session completes is the first check of the test-mode rehearsal; if it does, that ruling is needed before billing is enabled. The dispositions and the header say so. | `DISPOSITIONS`, header, `DECISIONS-FOR-PAUL.md` | |
+| Opus P2-1: an honest buyer whose provider left a dead subscription naming the intent (an abandoned authentication; a declined card then another) is held with no stage-one disposition; the dispositions said "waits on a person", and no person can act. | **Not changed in code; made a gate and a decision.** The code does not read an attached subscription's status, and whether `incomplete_expired` is nothing owed is a ruling (the test-mode list already asks what it leaves outstanding). Whether the provider ever makes a subscription before its session completes is the first check of the test-mode rehearsal; if it does, that ruling is needed before billing is enabled. The dispositions and the header say so. | `DISPOSITIONS`, header, `DECISIONS-FOR-PAUL.md` on the launch branch (decision 15; `ENABLEMENT.md` section 6 step 2) | |
| Opus P3-1: the rule's stated exception (an intent that expired in the ordinary way) was not the only one: an unready intent cleaned up with nothing naming it leaves nothing either. | Stated, in the header and the design. | header, design | |
| Opus P3-2: a subscription was classified only under the account its own metadata named; naming A's intent under B, or no account, it was recorded against nothing. | Fixed: the account is the one the console recorded the intent for (standing, consumed, retired, or resolved), by the intent id alone; the metadata's account is used only for an intent the console has no record of. | `billingretirement.go` `noteSubscriptionSeen`, `accountOfIntentTx` | `TestASubscriptionIsRecordedUnderTheIntentsOwnAccount` |
| Opus P3-3: the pass reported a hold it caused as a failure, once. | Fixed at both sites (`refuseSession` through the pass; recovery). | `billingpass.go` | `TestThePassDoesNotFailOnAHoldItCaused` |
@@ -508,3 +508,23 @@ Mutation checks, in a tree copy, nine, each run against the round's tests and th
Stated and not changed: everything under review twenty-one's "Stated and not changed", with the second bullet as corrected above; the structural guard's wrapper check is lexical; no test runs two console processes.
+
+## Review twenty-three (reviewed `2618403f`; Opus accepted with fixes, Astra rejected)
+
+Reports: `OPUS-REVIEW-multi-currency-review23.md`, `CODEX-FINDINGS-multi-currency-review23.md`. Opus: every round-twenty-two finding closed or honestly stated, no P1 or P2, three P3s and four claims; "on that basis I would count review eighteen's P2 as closed at `2618403f`". Astra: one P2 and one P3. **Review eighteen's P2 stays recorded as open until a round ends with both accepting**; Opus's sentence is noted and is one reviewer's.
+
+The gate on `2618403f` **failed** (`gate-currency-2618403f.txt`, exit 2): one existing test, `TestABoundSessionReadBackAsNobodysIsReviewed`, expected a retire held for an unproven expiry and got it held for an attached subscription. That is round twenty-two's Opus P3-2 at work (a subscription naming the intent under another account's name is now recorded against the intent), and the test's expectation was out of date; it now asserts both holds, the attachment first and the unproven expiry once the attachment is taken away by hand. The test was not in the subset run before the commit; the full package run is part of the round's checks again.
+
+| Finding | Disposition | Where | Counterexample test |
+| --- | --- | --- | --- |
+| Astra 1 (P2): a dispute committed after the retirement's reservation and before the account's tombstone was published did not stop the publication; the deletion then refused at its last step, leaving an account retired in the deployment's manifest and present here, under a dispute stage one cannot resolve. | Fixed by a point of no return in the database: `commitBillingRetirement`, taken after the reservation and before the first write outside, re-reads every condition in its own transaction and marks the reservation committed. Filing a checkout refusal or a checkout dispute withdraws a reservation that is not committed, in the evidence's own transaction. SQLite serialises the two: either the evidence withdraws first and the commit refuses (nothing outside was touched; the reservation is released), or the commit is first and the evidence lands on an account whose retirement finishes (the deletion and the reap pass a review that opened after the commit). | `billingstore.go` `commitBillingRetirement`, `cancelPendingRetirementTx`, `fileBillingReviewTx`, `checkBillingRetirement`; `ownership.go` the deletion handler, `DeleteAccount`; `store.go` `ReapAccount` | `TestARetirementIsCommittedInTheDatabaseAndEvidenceSerialisesAgainstIt` (three cases) |
+| Opus P3-2: evidence first seen after an account was deleted landed where no operator looked (the listing left the account blank; the door answered 404). | Fixed: the operator doors look accounts up whether or not they are deleted; the listing marks such a row `account_deleted`. | `billingoperator.go`; `billingstore.go` `accountEvenIfDeleted` | the same test, second case |
+| Astra 2 (P3): one session on a listing page that did not decode discarded the page's other sessions, one of which could be a contradiction of an archived fate. | Fixed: the client decodes each listed session on its own and returns the ones that decoded with the decode error. | `billingstripe/catalog.go` | `TestListCheckouts_AMalformedSessionKeepsTheOthersOnItsPage` |
+| Opus P3-1: the late-refusal dispute used a third key for an intent consumed with no session. | Fixed: the intent id, as the record and the classification. | `billingstore.go` `fileCheckoutRefusal` | |
+| Opus P3-3: the recovery site's filter of the hold it caused had no test. | Tested. | | `TestThePassDoesNotFailOnAHoldItsRecoveryCaused` |
+| Opus claims: "discarded unconditionally" (`resolveIntentForDeletion`); "an hour later at the earliest"; the dispositions' "Where" naming a file not in this branch; the enablement plan's "costs an honest buyer nothing". | The two comments corrected; the "Where" reads `DECISIONS-FOR-PAUL.md` on the launch branch; the plan's sentence is qualified there (two subscriptions naming one intent by any means, not only a subscription before completion). | | |
+
+Mutation checks, in a tree copy, eleven, all die: checkout evidence not withdrawing a pending retirement; the commit not re-reading the conditions, and not needing a reservation (each needed its own test case, added); the deletion refusing a review opened after the commit, and ignoring reviews whether or not committed; the listing leaving a deleted account blank; the door answering 404 for one; a committed reservation withdrawn; a session that does not decode dropping its page; the recovery's hold the pass's failure; the late refusal of a session not kept. Output: `currency-mutants-round23.txt` in the run directory's logs.
+
+Stated and not changed: the reap of an account the manifest already records as retired passes an open checkout review (the review stays listed and resolvable under the deleted account); the structural guard's wrapper check is lexical; no test runs two console processes (the serialisation is tested through the store's transactions in sequence, which is what SQLite's one writer makes of two consoles).
+
diff --git a/internal/billingstripe/catalog.go b/internal/billingstripe/catalog.go
index e28742630..159b68334 100644
--- a/internal/billingstripe/catalog.go
+++ b/internal/billingstripe/catalog.go
@@ -150,8 +150,8 @@ func (c *Client) listCheckouts(ctx context.Context, customerID string, createdAf
q.Set("starting_after", after)
}
var page struct {
- Data []Session `json:"data"`
- HasMore bool `json:"has_more"`
+ Data []json.RawMessage `json:"data"`
+ HasMore bool `json:"has_more"`
}
// A listing that fails after its first page returns, with the
// error, the sessions of the pages it did read: each of those was
@@ -161,24 +161,36 @@ func (c *Client) listCheckouts(ctx context.Context, customerID string, createdAf
if err := c.do(ctx, http.MethodGet, "/v1/checkout/sessions", q, "", &page); err != nil {
return out, err
}
- // A page that repeats a session, or carries one with no usable
- // id, is broken pagination and the listing is refused. What the
- // page did say is still returned with that error: a session seen
- // twice is two answers about it, and the second may differ.
- broken := false
- for _, session := range page.Data {
+ // Each session on the page is decoded on its own: one that does
+ // not decode fails the listing, and the ones that did decode are
+ // still returned with that error. A page that repeats a session,
+ // or carries one with no usable id, is broken pagination and the
+ // listing is refused the same way, with what the page did say: a
+ // session seen twice is two answers about it, and the second may
+ // differ.
+ var pageErr error
+ lastID := ""
+ for _, raw := range page.Data {
+ var session Session
+ if err := json.Unmarshal(raw, &session); err != nil {
+ pageErr = &Error{Code: CodeDecodeError, HTTPStatus: http.StatusOK}
+ continue
+ }
if !validID(session.ID) {
- broken = true
+ if pageErr == nil {
+ pageErr = &Error{Code: CodeBrokenPagination}
+ }
continue
}
- if seen[session.ID] {
- broken = true
+ if seen[session.ID] && pageErr == nil {
+ pageErr = &Error{Code: CodeBrokenPagination}
}
seen[session.ID] = true
out = append(out, session)
+ lastID = session.ID
}
- if broken {
- return out, &Error{Code: CodeBrokenPagination}
+ if pageErr != nil {
+ return out, pageErr
}
if len(out) > 10000 {
return out, &Error{Code: CodeTooManyRecords}
@@ -189,7 +201,7 @@ func (c *Client) listCheckouts(ctx context.Context, customerID string, createdAf
if len(page.Data) == 0 {
return out, &Error{Code: CodeBrokenPagination}
}
- after = page.Data[len(page.Data)-1].ID
+ after = lastID
}
}
diff --git a/internal/billingstripe/catalog_test.go b/internal/billingstripe/catalog_test.go
index 443faddc6..c59efbeac 100644
--- a/internal/billingstripe/catalog_test.go
+++ b/internal/billingstripe/catalog_test.go
@@ -328,6 +328,27 @@ func TestListCheckouts_ALaterPagesFailureKeepsTheEarlierPages(t *testing.T) {
}
}
+// A page with a session that does not decode fails the listing, and the
+// sessions on it that did decode are still returned with the error.
+func TestListCheckouts_AMalformedSessionKeepsTheOthersOnItsPage(t *testing.T) {
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ _ = json.NewEncoder(w).Encode(map[string]any{
+ "data": []map[string]any{{"id": "cs_1", "status": "complete", "livemode": true}, {"id": "cs_bad"}, {"id": "cs_2", "status": "open", "livemode": true}},
+ "has_more": false,
+ })
+ }))
+ defer srv.Close()
+ c := testClient(t, srv.URL)
+ out, err := c.ListCheckouts(context.Background(), 0, 100)
+ var listErr *Error
+ if !errors.As(err, &listErr) || listErr.Code != CodeDecodeError {
+ t.Fatalf("a session that does not decode: %v; want a decode error", err)
+ }
+ if len(out) != 2 || out[0].ID != "cs_1" || out[0].Status != "complete" || out[1].ID != "cs_2" {
+ t.Fatalf("the sessions that did decode: %+v; want the ones before and after the malformed one", out)
+ }
+}
+
// A page that repeats a session is broken pagination, and what it said is
// still returned with the error: the repeat, and what follows it.
func TestListCheckouts_ABrokenPageStillReturnsWhatItSaid(t *testing.T) {
diff --git a/internal/console/billingcurrency_test.go b/internal/console/billingcurrency_test.go
index e8f4d54c2..c35475960 100644
--- a/internal/console/billingcurrency_test.go
+++ b/internal/console/billingcurrency_test.go
@@ -3500,12 +3500,19 @@ func TestABoundSessionReadBackAsNobodysIsReviewed(t *testing.T) {
if standing, err := fx.st.billingCheckoutIntent(acct.ID); err != nil || standing.ID != intent.ID {
t.Fatalf("intent %+v %v; want it standing", standing, err)
}
- // The operator cannot retire it: a session that reads back as another
- // account's proves nothing about itself, so its fate is unproven and
- // the intent is held; and the buyer's next click is offered nothing.
+ // The operator cannot retire it. The subscription that named the
+ // intent (under the other account's name) is recorded against the
+ // intent and holds the retire first; and a session that reads back as
+ // another account's proves nothing about itself, so with that taken
+ // away by hand the fate is unproven and the intent is held for that.
+ // The buyer's next click is offered nothing.
if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil {
t.Fatal(err)
}
+ heldRetire(t, fx, acct, intent.ID, "paul@keymail.dev", "sub_paid", f.now.Add(10*time.Minute))
+ if _, err := fx.st.db.Exec(`DELETE FROM billing_review_obligations WHERE account_id=? AND intent_id=?`, acct.ID, intent.ID); err != nil {
+ t.Fatal(err)
+ }
heldRetire(t, fx, acct, intent.ID, "paul@keymail.dev", "not yet proven expired", f.now.Add(10*time.Minute))
if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now.Add(20*time.Minute)); dest != "" || !errors.Is(err, errBillingUnderReview) {
t.Fatalf("a click under the review: dest %q err %v; want nothing offered", dest, err)
diff --git a/internal/console/billingnamed_test.go b/internal/console/billingnamed_test.go
index 2aefc3a13..4707e90ac 100644
--- a/internal/console/billingnamed_test.go
+++ b/internal/console/billingnamed_test.go
@@ -953,3 +953,227 @@ func TestADisputeAboutAConsumedIntentWithNoSessionIsKeyedByTheIntent(t *testing.
t.Fatalf("%d disputes after the purchase's own subscription was seen", n)
}
}
+
+// Review twenty-three's counterexamples.
+
+// An account's retirement has a point of no return in the database, before
+// anything outside it is written. Checkout evidence that lands before it
+// withdraws the pending retirement, and the commit then refuses; evidence
+// that lands after it stays with the account, which is deleted, and is
+// listed and resolvable under the deleted account. (Review twenty-three,
+// Astra 1; Opus P3-2.)
+func TestARetirementIsCommittedInTheDatabaseAndEvidenceSerialisesAgainstIt(t *testing.T) {
+ // On an account that can be deleted (the fixture's own is the ops
+ // account): a second account, with the fixture's member, that buys
+ // and whose purchase ends.
+ ended := func(t *testing.T) (*consoleFixture, *fakeBillingStripe, *Account, billingIntent) {
+ t.Helper()
+ fx, f, ops := newCurrencyFixture(t)
+ if err := fx.st.CreateAccount(ops.ID+100, "cdef", "Other", "paul@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.AddMember(ops.ID+100, "paul@keymail.dev", RoleOwner, time.Now()); err != nil && !strings.Contains(err.Error(), "already") {
+ t.Fatal(err)
+ }
+ acct, _ := fx.st.AccountBySqid("cdef")
+ f.customerCountry = "GB"
+ f.mu.Lock()
+ f.sqid = "cdef"
+ f.mu.Unlock()
+ if dest, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err != nil || dest == "" {
+ t.Fatalf("first click: dest %q err %v", dest, err)
+ }
+ intent, _ := fx.st.billingCheckoutIntent(acct.ID)
+ f.mu.Lock()
+ f.sessionStatus, f.subscriptionPrice = "complete", intent.Price
+ f.mu.Unlock()
+ if code := f.webhookPost(t, fx, "evt_own_other", "customer.subscription.created", "sub_paid"); code != http.StatusNoContent {
+ t.Fatalf("the purchase: %d", code)
+ }
+ f.mu.Lock()
+ f.subscriptionStatus = "canceled"
+ f.mu.Unlock()
+ if err := fx.srv.reconcileBillingSubscription(context.Background(), "sub_paid", "", f.now.Add(time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ if state, _ := fx.st.billingAccount(acct.ID); state.Subscription != "sub_paid" || state.Status != "canceled" {
+ t.Fatalf("fixture: %+v", state)
+ }
+ return fx, f, acct, intent
+ }
+ reserved := func(t *testing.T, fx *consoleFixture, acct *Account) (reserved, committed bool) {
+ t.Helper()
+ var at int64
+ err := fx.st.db.QueryRow(`SELECT committed_at FROM billing_retirements WHERE account_id=?`, acct.ID).Scan(&at)
+ if errors.Is(err, sql.ErrNoRows) {
+ return false, false
+ }
+ if err != nil {
+ t.Fatal(err)
+ }
+ return true, at != 0
+ }
+ t.Run("evidence before the commit withdraws the retirement", func(t *testing.T) {
+ fx, f, acct, _ := ended(t)
+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ // Another console reads a subscription made by hand naming the
+ // consumed intent, through the production classification.
+ f.mu.Lock()
+ f.otherSubscriptions = map[string]bool{"sub_hand": true}
+ f.mu.Unlock()
+ if code := f.webhookPost(t, fx, "evt_hand_while_reserved", "customer.subscription.created", "sub_hand"); code != http.StatusNoContent {
+ t.Fatalf("the hand-made subscription: %d", code)
+ }
+ if r, _ := reserved(t, fx, acct); r {
+ t.Fatal("the pending retirement was not withdrawn by the evidence")
+ }
+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); !errors.Is(err, errBillingRetirement) {
+ t.Fatalf("the commit after the evidence: %v; want it refused", err)
+ }
+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(3*time.Hour)); !errors.Is(err, errBillingRetirement) {
+ t.Fatalf("a new reservation under the open dispute: %v; want it refused", err)
+ }
+ if err := fx.st.DeleteAccount(acct.ID, f.now.Add(3*time.Hour)); !errors.Is(err, errBillingRetirement) {
+ t.Fatalf("the deletion under the open dispute: %v; want it refused", err)
+ }
+ })
+ t.Run("the commit re-reads every condition and needs a reservation", func(t *testing.T) {
+ fx, f, acct, intent := ended(t)
+ // Never reserved: nothing to commit.
+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); !errors.Is(err, errBillingRetirement) {
+ t.Fatalf("a commit with no reservation: %v; want it refused", err)
+ }
+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ // A review written without withdrawing the reservation (as an
+ // older binary would write it): the commit reads it and refuses.
+ if _, err := fx.st.db.Exec(`INSERT INTO billing_reviews(account_id,subscription,reason,created_at,last_seen_at,occurrences,protocol) VALUES(?,?,?,?,?,1,1)`, acct.ID, intent.SessionID, billingFateDisputedReason, f.now.Unix(), f.now.Unix()); err != nil {
+ t.Fatal(err)
+ }
+ if r, _ := reserved(t, fx, acct); !r {
+ t.Fatal("fixture: the reservation went")
+ }
+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(3*time.Hour)); !errors.Is(err, errBillingRetirement) {
+ t.Fatalf("the commit over an open review: %v; want it refused", err)
+ }
+ if _, c := reserved(t, fx, acct); c {
+ t.Fatal("the retirement was committed over an open review")
+ }
+ })
+ t.Run("evidence after the commit stays with the deleted account", func(t *testing.T) {
+ fx, f, acct, intent := ended(t)
+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ if r, c := reserved(t, fx, acct); !r || !c {
+ t.Fatalf("after the commit: reserved %v committed %v", r, c)
+ }
+ f.mu.Lock()
+ f.otherSubscriptions = map[string]bool{"sub_hand": true}
+ f.mu.Unlock()
+ if code := f.webhookPost(t, fx, "evt_hand_after_commit", "customer.subscription.created", "sub_hand"); code != http.StatusNoContent {
+ t.Fatalf("the hand-made subscription: %d", code)
+ }
+ if n := disputesOf(t, fx, acct, intent.SessionID, "%"); n != 1 {
+ t.Fatalf("%d disputes; want the evidence recorded", n)
+ }
+ if r, c := reserved(t, fx, acct); !r || !c {
+ t.Fatalf("after the evidence: reserved %v committed %v; want the committed retirement kept", r, c)
+ }
+ // The commit again (a retry after an ambiguous failure outside)
+ // passes, and so does the deletion.
+ if err := fx.st.commitBillingRetirement(acct.ID, f.now.Add(3*time.Hour)); err != nil {
+ t.Fatalf("the commit again: %v", err)
+ }
+ if err := fx.st.DeleteAccount(acct.ID, f.now.Add(3*time.Hour)); err != nil {
+ t.Fatalf("the deletion after the commit: %v", err)
+ }
+ if a, _ := fx.st.AccountByID(acct.ID); a != nil {
+ t.Fatal("the account was not deleted")
+ }
+ // The operator still sees the review, under the deleted account,
+ // and the door finds it (and holds it, as every dispute).
+ token := billingOperatorToken(t, fx, "ops@keymail.dev", "code-billing-op-deleted")
+ status, listing := getJSON(t, fx.ts.URL+"/api/billing/reviews", token)
+ rows, _ := listing["reviews"].([]any)
+ found := false
+ for _, r := range rows {
+ row, _ := r.(map[string]any)
+ if row["reason"] == billingFateDisputedReason && row["subscription"] == intent.SessionID {
+ found = true
+ if row["account"] != acct.Sqid || row["account_deleted"] != true {
+ t.Fatalf("the deleted account's review as listed: %v", row)
+ }
+ }
+ }
+ if status != http.StatusOK || !found {
+ t.Fatalf("the deleted account's dispute is not listed: %d %v", status, listing)
+ }
+ body := map[string]any{"account": acct.Sqid, "subscription": intent.SessionID, "reason": billingFateDisputedReason, "occurrences": 1, "note": "looked"}
+ if status, answer := postJSON(t, fx.ts.URL+"/api/billing/reviews/resolve", body, token); status != http.StatusConflict || !strings.Contains(fmt.Sprint(answer["error"]), "held") {
+ t.Fatalf("the door for the deleted account's dispute: %d %v; want it found and held, not 404", status, answer)
+ }
+ })
+ t.Run("a review of another kind withdraws nothing", func(t *testing.T) {
+ fx, f, acct, _ := ended(t)
+ if err := fx.st.reserveBillingRetirement(acct.ID, f.now.Add(2*time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ tx, _ := fx.st.db.Begin()
+ if err := fileBillingReviewTx(tx, acct.ID, "sub_paid", "billing_location_changed", f.now.Add(2*time.Hour)); err != nil {
+ t.Fatal(err)
+ }
+ if err := tx.Commit(); err != nil {
+ t.Fatal(err)
+ }
+ if r, _ := reserved(t, fx, acct); !r {
+ t.Fatal("a review that does not stop a retirement withdrew the reservation")
+ }
+ })
+}
+
+// The recovery of a lost session that refuses it, and leaves the intent
+// under review because a subscription names it, is not the pass's failure
+// either. (Review twenty-three, Opus P3-3.)
+func TestThePassDoesNotFailOnAHoldItsRecoveryCaused(t *testing.T) {
+ fx, f, acct := newCurrencyFixture(t)
+ f.customerCountry = "GB"
+ // The create's answer is lost: the intent is ready and unbound.
+ f.mu.Lock()
+ f.failCreate = true
+ f.mu.Unlock()
+ if _, err := fx.srv.startBillingCheckout(context.Background(), acct, plans.PlanPersonal, "GB", "", "", f.now); err == nil {
+ t.Fatal("fixture: the checkout went through")
+ }
+ intent, err := fx.st.billingCheckoutIntent(acct.ID)
+ if err != nil || intent.SessionID != "" || !intent.CustomerReady {
+ t.Fatalf("fixture: %+v %v; want a ready, unbound intent", intent, err)
+ }
+ // A subscription names it (on terms: a retry, and recorded).
+ f.mu.Lock()
+ f.subscriptionPrice = intent.Price
+ f.mu.Unlock()
+ _ = f.webhookPost(t, fx, "evt_hand_unbound_pass", "customer.subscription.created", "sub_paid")
+ if st, _ := fx.st.refusalState(acct.ID, intent.ID); len(st.Obligations) != 1 {
+ t.Fatalf("fixture: %+v", st)
+ }
+ // The pass recovers the session by listing; it reads in another
+ // currency than the intent's, so it is refused and expired, and the
+ // expiry files the refusal instead of retiring the intent.
+ f.mu.Lock()
+ f.sessionCurrency = "eur"
+ f.mu.Unlock()
+ if err := fx.srv.billingPass(context.Background(), f.now.Add(time.Minute)); err != nil {
+ t.Fatalf("the pass: %v; want no failure", err)
+ }
+ standing(t, fx, acct, intent)
+ if st, _ := fx.st.refusalState(acct.ID, intent.ID); !st.Open || st.Reference == "" || f.sessionStatus != "expired" {
+ t.Fatalf("state %+v, session %s; want the refusal filed naming the recovered session, which was expired", st, f.sessionStatus)
+ }
+}
diff --git a/internal/console/billingoperator.go b/internal/console/billingoperator.go
index 91cf42b07..58c509561 100644
--- a/internal/console/billingoperator.go
+++ b/internal/console/billingoperator.go
@@ -182,6 +182,9 @@ type billingReviewRow struct {
LastSeenAt int64 `json:"last_seen_at"`
// Reference: the refused session's id for a currency refusal, else "".
Reference string `json:"reference,omitempty"`
+ // AccountDeleted: the review is a deleted account's (evidence that
+ // landed after its retirement was committed). Still a person's to see.
+ AccountDeleted bool `json:"account_deleted,omitempty"`
// Disposition, for a checkout refusal: the session's recorded fate,
// what is attached, and why retire or keep is held (empty: available).
Disposition *refusalDisposition `json:"disposition,omitempty"`
@@ -199,11 +202,11 @@ func (s *Server) handleAPIBillingReviews(w http.ResponseWriter, r *http.Request)
}
out := make([]billingReviewRow, 0, len(reviews))
for _, x := range reviews {
- sqid := ""
- if acct, err := s.store.AccountByID(x.AccountID); err == nil && acct != nil {
- sqid = acct.Sqid
+ sqid, deleted := "", false
+ if acct, gone, err := s.store.accountEvenIfDeleted(x.AccountID); err == nil && acct != nil {
+ sqid, deleted = acct.Sqid, gone
}
- row := billingReviewRow{Account: sqid, Subscription: x.Subscription, Reason: x.Reason, Occurrences: x.Occurrences, CreatedAt: x.CreatedAt, LastSeenAt: x.LastSeenAt, Reference: x.Reference}
+ row := billingReviewRow{Account: sqid, Subscription: x.Subscription, Reason: x.Reason, Occurrences: x.Occurrences, CreatedAt: x.CreatedAt, LastSeenAt: x.LastSeenAt, Reference: x.Reference, AccountDeleted: deleted}
if x.Reason == billingCheckoutRefusedReason {
if st, err := s.store.refusalState(x.AccountID, x.Subscription); err == nil {
row.Disposition = st.disposition()
@@ -337,7 +340,7 @@ func (s *Server) handleAPIBillingReviewResolve(w http.ResponseWriter, r *http.Re
jsonError(w, http.StatusBadRequest, "outcome must be retire or keep")
return
}
- acct, err := s.store.AccountBySqid(req.Account)
+ acct, _, err := s.store.accountBySqidEvenIfDeleted(req.Account)
if err != nil || acct == nil {
jsonError(w, http.StatusNotFound, "no such billing review")
return
diff --git a/internal/console/billingretirement.go b/internal/console/billingretirement.go
index edfff3821..3ff6aa1f2 100644
--- a/internal/console/billingretirement.go
+++ b/internal/console/billingretirement.go
@@ -169,6 +169,11 @@ func (s *Store) migrateBillingRetirement() error {
// another customer's is seen as the contradiction it is.
{"billing_retired_intents", "customer", `TEXT NOT NULL DEFAULT ''`},
{"billing_retired_intents", "session_fate", `TEXT NOT NULL DEFAULT ''`},
+ // An account's retirement is committed in the database before
+ // anything outside it is written (commitBillingRetirement): until
+ // then checkout evidence cancels it, and from then on evidence
+ // lands on an account that is being retired and stays visible.
+ {"billing_retirements", "committed_at", `INTEGER NOT NULL DEFAULT 0`},
} {
if err := s.addColumnIfMissing(c[0], c[1], c[2]); err != nil {
return err
@@ -962,8 +967,9 @@ func (s *Server) advanceRefusal(ctx context.Context, intent billingIntent, acct
// billingPassRefusals works a bounded number of refused intents after
// the pass's other work: of those whose try is due, the ones that have
-// been due longest. A refusal that was just tried is due again an hour
-// later at the earliest, so one that stays due is reached within a few
+// been due longest. A refusal that was just tried is due again a minute
+// later at the earliest, and an hour later once it has been tried three
+// times (noteExpiryTry), so one that stays due is reached within a few
// passes however many others there are; taken in the order they stand,
// the same old ones could use the budget for ever.
func (s *Server) billingPassRefusals(ctx context.Context, refused []billingIntent, now time.Time) []error {
diff --git a/internal/console/billingservice.go b/internal/console/billingservice.go
index 670103862..5247b5e63 100644
--- a/internal/console/billingservice.go
+++ b/internal/console/billingservice.go
@@ -788,8 +788,9 @@ func (s *Server) startBillingCheckoutQuoted(ctx context.Context, acct *Account,
// resolveIntentForDeletion clears a checkout intent that must not block an
// account deletion. A stillborn intent (no session, customer not ready) proves
-// no session was ever created and is discarded unconditionally — the operator
-// is deleting the account, so a never-started attempt must not wedge it. Every
+// no session was ever created and is discarded, unless a subscription was
+// seen naming it or a refusal review is open on it (discardUnreadyIntent
+// refuses both, and the deletion then waits on a person). Every
// other resting state is handed to the ordinary pending-intent path: an
// expired session is retired, a completed purchase is bound and activated (and
// then legitimately blocks the deletion), and a lost response is recovered.
diff --git a/internal/console/billingstore.go b/internal/console/billingstore.go
index 09e703d57..5891150ad 100644
--- a/internal/console/billingstore.go
+++ b/internal/console/billingstore.go
@@ -954,6 +954,11 @@ func (s *Store) applyBillingStateDated(next billingState, expectedRevision int64
// recorded.
func fileBillingReviewTx(tx *sql.Tx, accountID int64, subscription, reason string, observed time.Time) error {
at := observed.Unix()
+ if reason == billingCheckoutRefusedReason || reason == billingFateDisputedReason {
+ if err := cancelPendingRetirementTx(tx, accountID); err != nil {
+ return err
+ }
+ }
_, err := tx.Exec(`INSERT INTO billing_reviews(account_id,subscription,reason,created_at,last_seen_at,occurrences) VALUES(?,?,?,?,?,1)
ON CONFLICT(account_id,subscription,reason) DO UPDATE SET resolved_at=0, occurrences=occurrences+1, last_seen_at=MAX(last_seen_at,excluded.last_seen_at)`,
accountID, subscription, reason, at, at)
@@ -1261,7 +1266,7 @@ func (s *Store) fileCheckoutRefusal(intent billingIntent, sessionID, subscriptio
}
if err == nil {
if usedSession == "" {
- usedSession = sessionID
+ usedSession = intent.ID // the key the record and the classification use
}
if err := disputeSessionTx(tx, intent.AccountID, usedSession, intent.ID, "a refusal of session "+sessionID+" was filed after the purchase recorded with "+usedSubscription, observed); err != nil {
return err
@@ -1499,9 +1504,14 @@ func advanceBillingMonthTx(tx *sql.Tx, x billingMonthRecord, state, invoiceID, i
// An account is not retired while a checkout refusal or a checkout
// dispute is open on it: each is a subscription or a session at the
-// provider that a person has not disposed of, and once the account is gone
-// its reviews are listed under no account and resolved through no door.
-func checkBillingRetirement(tx *sql.Tx, accountID int64) error {
+// provider that a person has not disposed of. That clause is read when the
+// retirement is reserved and when it is committed; once committed (the
+// point of no return, in this database, before anything outside it is
+// written) a review that opens afterwards does not stop the deletion
+// finishing, and is listed and resolvable under the deleted account
+// instead (the operator doors look accounts up whether or not they are
+// deleted).
+func checkBillingRetirement(tx *sql.Tx, accountID int64, committed bool) error {
if held, err := quantityObligationTx(tx, accountID); err != nil {
return err
} else if held {
@@ -1511,8 +1521,8 @@ func checkBillingRetirement(tx *sql.Tx, accountID int64) error {
err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_accounts WHERE account_id=? AND subscription<>'' AND status NOT IN ('canceled','incomplete_expired'))
OR EXISTS(SELECT 1 FROM billing_checkout_intents WHERE account_id=?)
OR EXISTS(SELECT 1 FROM billing_months WHERE account_id=? AND state NOT IN ('complete','waived'))
- OR EXISTS(SELECT 1 FROM billing_reviews WHERE account_id=? AND reason IN (?,?) AND resolved_at=0)`,
- accountID, accountID, accountID, accountID, billingCheckoutRefusedReason, billingFateDisputedReason).Scan(&blocked)
+ OR (NOT ? AND EXISTS(SELECT 1 FROM billing_reviews WHERE account_id=? AND reason IN (?,?) AND resolved_at=0))`,
+ accountID, accountID, accountID, committed, accountID, billingCheckoutRefusedReason, billingFateDisputedReason).Scan(&blocked)
if err != nil {
return err
}
@@ -1522,6 +1532,55 @@ func checkBillingRetirement(tx *sql.Tx, accountID int64) error {
return nil
}
+// retirementCommitted reports whether the account's retirement has passed
+// its point of no return.
+func retirementCommitted(tx *sql.Tx, accountID int64) (bool, error) {
+ var committed bool
+ err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_retirements WHERE account_id=? AND committed_at<>0)`, accountID).Scan(&committed)
+ return committed, err
+}
+
+// cancelPendingRetirementTx withdraws a reservation that has not been
+// committed: checkout evidence that lands while a deletion is being
+// prepared stops it, in the evidence's own transaction. A committed
+// retirement is not withdrawn; the evidence stays with the account.
+func cancelPendingRetirementTx(tx *sql.Tx, accountID int64) error {
+ _, err := tx.Exec(`DELETE FROM billing_retirements WHERE account_id=? AND committed_at=0`, accountID)
+ return err
+}
+
+// commitBillingRetirement is the retirement's point of no return, taken
+// after the reservation and before the first write outside this database.
+// It re-reads every condition in its own transaction, so a checkout review
+// that opened since the reservation (and withdrew it) refuses it here, and
+// nothing outside has been touched. A retirement already committed (a
+// retry after an ambiguous failure outside) passes whatever has opened
+// since.
+func (s *Store) commitBillingRetirement(accountID int64, now time.Time) error {
+ tx, err := s.db.Begin()
+ if err != nil {
+ return err
+ }
+ defer tx.Rollback()
+ var reserved, committed bool
+ if err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_retirements WHERE account_id=?), EXISTS(SELECT 1 FROM billing_retirements WHERE account_id=? AND committed_at<>0)`, accountID, accountID).Scan(&reserved, &committed); err != nil {
+ return err
+ }
+ if committed {
+ return nil
+ }
+ if !reserved {
+ return errBillingRetirement
+ }
+ if err := checkBillingRetirement(tx, accountID, false); err != nil {
+ return err
+ }
+ if _, err := tx.Exec(`UPDATE billing_retirements SET committed_at=? WHERE account_id=?`, max(now.Unix(), 1), accountID); err != nil {
+ return err
+ }
+ return tx.Commit()
+}
+
// Reserve before the bucket tombstone. Keep the reservation on an ambiguous
// write failure: a retry may finish retirement, but cannot open a new checkout.
func (s *Store) reserveBillingRetirement(accountID int64, now time.Time) error {
@@ -1530,7 +1589,11 @@ func (s *Store) reserveBillingRetirement(accountID int64, now time.Time) error {
return err
}
defer tx.Rollback()
- if err := checkBillingRetirement(tx, accountID); err != nil {
+ committed, err := retirementCommitted(tx, accountID)
+ if err != nil {
+ return err
+ }
+ if err := checkBillingRetirement(tx, accountID, committed); err != nil {
return err
}
if _, err := tx.Exec(`INSERT OR IGNORE INTO billing_retirements(account_id,started_at) VALUES(?,?)`, accountID, now.Unix()); err != nil {
@@ -1545,6 +1608,35 @@ func (s *Store) reserveBillingRetirement(accountID int64, now time.Time) error {
// touching the bucket). An ambiguous failure AFTER a write may have begun
// must keep the reservation, so that path never calls this: a retry can then
// finish retirement, but no second checkout can open in the meantime.
+// accountEvenIfDeleted and accountBySqidEvenIfDeleted are the operator
+// doors' lookups: a billing review of a deleted account is still a person's
+// to see and resolve.
+func (s *Store) accountEvenIfDeleted(id int64) (*Account, bool, error) {
+ var a Account
+ var created, deleted int64
+ err := s.db.QueryRow(`SELECT id, sqid, name, created_at, deleted_at FROM accounts WHERE id = ?`, id).Scan(&a.ID, &a.Sqid, &a.Name, &created, &deleted)
+ if errors.Is(err, sql.ErrNoRows) {
+ return nil, false, nil
+ }
+ if err != nil {
+ return nil, false, err
+ }
+ a.CreatedAt = time.Unix(created, 0).UTC()
+ return &a, deleted != 0, nil
+}
+
+func (s *Store) accountBySqidEvenIfDeleted(sqid string) (*Account, bool, error) {
+ var id int64
+ err := s.db.QueryRow(`SELECT id FROM accounts WHERE sqid = ?`, sqid).Scan(&id)
+ if errors.Is(err, sql.ErrNoRows) {
+ return nil, false, nil
+ }
+ if err != nil {
+ return nil, false, err
+ }
+ return s.accountEvenIfDeleted(id)
+}
+
func (s *Store) releaseBillingRetirement(accountID int64) error {
_, err := s.db.Exec(`DELETE FROM billing_retirements WHERE account_id=?`, accountID)
return err
diff --git a/internal/console/ownership.go b/internal/console/ownership.go
index 599ce6a6f..7fb896ac1 100644
--- a/internal/console/ownership.go
+++ b/internal/console/ownership.go
@@ -154,6 +154,18 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther)
return
}
+ // The point of no return, in the database, before anything outside it:
+ // a checkout review that opened since the reservation (and withdrew
+ // it) refuses here, and the reservation it withdrew is not re-made.
+ if err := s.store.commitBillingRetirement(acct.ID, time.Now()); err != nil {
+ log.Printf("console: account retirement: %s", billingDiagnostic(err))
+ if rerr := s.store.releaseBillingRetirement(acct.ID); rerr != nil {
+ log.Printf("console: release retirement reservation %s: %v", acct.Sqid, rerr)
+ }
+ s.setFlash(w, "err", deletionAdvice(err))
+ http.Redirect(w, r, "/a/"+acct.Sqid+"/settings", http.StatusSeeOther)
+ return
+ }
// The bucket first, and the deletion is abandoned if it will not take.
//
// This console's rows are not where account identity lives — every console
@@ -728,7 +740,11 @@ func (s *Store) DeleteAccount(accountID int64, now time.Time) error {
return err
}
defer tx.Rollback()
- if err := checkBillingRetirement(tx, accountID); err != nil {
+ committed, err := retirementCommitted(tx, accountID)
+ if err != nil {
+ return err
+ }
+ if err := checkBillingRetirement(tx, accountID, committed); err != nil {
return err
}
var apps int
diff --git a/internal/console/store.go b/internal/console/store.go
index 3e66345b0..288927f2b 100644
--- a/internal/console/store.go
+++ b/internal/console/store.go
@@ -1363,7 +1363,11 @@ func (s *Store) ReapAccount(accountID int64, now time.Time) error {
return err
}
defer tx.Rollback()
- if err := checkBillingRetirement(tx, accountID); err != nil {
+ // The reap follows a retirement the deployment's manifest already
+ // records (made here after its commit, or on another console): an open
+ // checkout review does not stop it, and stays listed and resolvable
+ // under the deleted account.
+ if err := checkBillingRetirement(tx, accountID, true); err != nil {
return err
}
for _, stmt := range []string{