From e32fd13e84092058b418777a746f5191c42ac84f Mon Sep 17 00:00:00 2001 From: Aaron Gabriel Date: Mon, 27 Jul 2026 18:14:37 -0600 Subject: [PATCH 1/2] feat(billing): teardown module for account-delete's 24h undo window (cloud#226 A-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds workers/identity/src/billing-teardown.ts: deferBilling (reversible hold on delete-request), resumeBilling (release on undo, with a discriminated result for the un-cancelable case), teardownBilling (the real, sweep-time cancel-everything + delete-customer + NULL-ids teardown, including the cloud#64 orphan belt across active/trialing/ past_due/unpaid subscriptions). Inert — nothing calls these yet; A-3 wires them into the delete/undo routes and the hourly sweep. Two guarantees this PR establishes: no new charge can post after a deletion is requested, and no subscription survives a completed teardown. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01XLZtmuSs1RirWGMGyCB1QB --- package.json | 2 +- workers/identity/src/billing-teardown.ts | 191 +++++++++++ .../identity/test/billing-teardown.test.ts | 300 ++++++++++++++++++ 3 files changed, 492 insertions(+), 1 deletion(-) create mode 100644 workers/identity/src/billing-teardown.ts create mode 100644 workers/identity/test/billing-teardown.test.ts diff --git a/package.json b/package.json index f9a7ffa..f9505fe 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@openparachute/cloud", - "version": "0.0.8-rc.128", + "version": "0.0.8-rc.129", "private": true, "description": "Open Parachute PBC's Vault Cloud \u2014 one Durable Object per vault on Cloudflare, OAuth issuer + self-serve console (accounts + vault ownership).", "license": "AGPL-3.0", diff --git a/workers/identity/src/billing-teardown.ts b/workers/identity/src/billing-teardown.ts new file mode 100644 index 0000000..1f0f347 --- /dev/null +++ b/workers/identity/src/billing-teardown.ts @@ -0,0 +1,191 @@ +/** + * Billing teardown for the account-delete train (cloud#226's sibling; this PR + * is A-2 — inert until A-3 wires it into the delete/undo routes). + * + * Aaron's ruling: deleting an account severs auth immediately but gives a + * 24-HOUR UNDO WINDOW before anything with Stripe is touched. Three functions, + * each over the injected Stripe client ({@link BillingOverrides} in billing.ts + * — same seam, so tests inject a stub the same way billing-lifecycle.ts's + * tests do, no new mechanism): + * + * - {@link deferBilling} — the REVERSIBLE hold, run the instant deletion is + * requested: `cancel_at_period_end = true`. No new charge can post, and + * it's still undoable — including for a `trialing` subscription, where it + * means the trial cancels at its boundary instead of converting. + * - {@link resumeBilling} — undo, run if the user reactivates inside the + * window: flips the flag back. The one case it CANNOT release — the + * period boundary already passed and Stripe auto-canceled the + * subscription for real — has no un-cancel; the discriminated result says + * so plainly rather than throwing or lying about a restore. + * - {@link teardownBilling} — the REAL, irreversible teardown, run only at + * window expiry (the hourly sweep, A-3): cancel the stored subscription, + * sweep every OTHER live subscription under the customer (the cloud#64 + * orphan shape — a subscription the `users` row doesn't name would + * otherwise bill a ghost forever), delete the customer, then NULL the + * Stripe ids — that NULL is the converged marker the sweep keys on to + * stop retrying. + * + * TOLERANCE, everywhere: Stripe's "this doesn't exist" / "already in a + * terminal state" shape ({@link Stripe.errors.StripeInvalidRequestError} — a + * missing id, a 404, "you cannot update a canceled subscription") is success, + * not an error, for every operation here — cancel-of-canceled and + * delete-of-deleted both count as done. Any OTHER failure (auth, rate limit, + * a genuine outage) is real and must propagate so the caller — and the + * sweep's retry — sees it; teardownBilling is the one place that catches it + * itself, to answer with an unconverged result instead of throwing out of a + * cron tick. + */ +import Stripe from "stripe"; +import { getUserById } from "./users.ts"; + +/** + * The one error shape every function below treats as "already done, not a + * failure": Stripe's invalid-request class covers a missing/unknown id (404, + * `resource_missing`) AND an operation refused because the object is already + * in its terminal state (e.g. updating a subscription that's already fully + * canceled). Every Stripe call here passes exactly one id and no other + * user-controlled params, so an invalid-request error can only mean one of + * those two things — never a param validation bug we'd want surfaced. + */ +function isTolerableStripeError(err: unknown): boolean { + return err instanceof Stripe.errors.StripeInvalidRequestError; +} + +async function cancelSubscriptionTolerantly(stripe: Stripe, subscriptionId: string): Promise { + try { + await stripe.subscriptions.cancel(subscriptionId); + } catch (err) { + if (!isTolerableStripeError(err)) throw err; + } +} + +/** + * Set the reversible hold on a subscription: `cancel_at_period_end = true`. + * Run the instant an account-delete is requested (A-3). Guarantees no NEW + * charge posts after the request, while staying reversible up to the period + * boundary — including a `trialing` subscription, where this means the trial + * lapses at its own boundary instead of converting and charging. + * + * Tolerant of a missing id (nothing to defer — no subscription on file: not a + * Stripe call at all), a 404, or a subscription already canceled. Any other + * Stripe failure propagates — the caller needs to know the hold may not have + * landed on a subscription that's actually still live. + */ +export async function deferBilling(stripe: Stripe, subscriptionId: string | null): Promise { + if (!subscriptionId) return; + try { + await stripe.subscriptions.update(subscriptionId, { cancel_at_period_end: true }); + } catch (err) { + if (!isTolerableStripeError(err)) throw err; + } +} + +/** Why {@link resumeBilling} could not release the hold — see its doc comment. */ +export type ResumeBillingResult = { resumed: true } | { resumed: false; reason: "already_canceled" }; + +/** + * Release the hold {@link deferBilling} set: flip `cancel_at_period_end` back + * to `false`. Run if the account reactivates inside the 24h undo window. + * + * A missing id means there was never a hold to release (no subscription on + * file) — trivially resumed, no Stripe call needed. + * + * THE CASE THAT MATTERS: if the period boundary passed WHILE the undo window + * was still open, Stripe already auto-canceled the subscription for real — + * there is no un-cancel. That surfaces here as the same tolerable + * invalid-request shape {@link deferBilling} treats as success, but resuming + * is not a no-op success: it is a genuine "billing did not come back", and + * the caller must be able to tell the user that plainly. So this returns a + * discriminated result instead of throwing OR silently answering `resumed: + * true` — a "restored" customer whose subscription is actually gone is + * exactly the money bug this whole design exists to avoid. + */ +export async function resumeBilling(stripe: Stripe, subscriptionId: string | null): Promise { + if (!subscriptionId) return { resumed: true }; + try { + await stripe.subscriptions.update(subscriptionId, { cancel_at_period_end: false }); + return { resumed: true }; + } catch (err) { + if (!isTolerableStripeError(err)) throw err; + return { resumed: false, reason: "already_canceled" }; + } +} + +/** Live-in-Stripe's-eyes statuses the {@link teardownBilling} belt must not + * leave behind — `trialing` is load-bearing: a trialing subscription is a + * real subscription that bills at trial end, not merely a preview. */ +const LIVE_SUBSCRIPTION_STATUSES: ReadonlySet = new Set([ + "active", + "trialing", + "past_due", + "unpaid", +]); + +/** Whether {@link teardownBilling} reached the converged (NULL-ids) end + * state, or must be retried by the next sweep pass. */ +export type TeardownBillingResult = { converged: true } | { converged: false; error: string }; + +/** + * The real, irreversible teardown — run ONLY at window expiry (the hourly + * sweep, A-3), never at delete-request time. In order: + * + * 1. Cancel the STORED subscription id outright (tolerant of + * already-canceled/404) — whatever state it's in, it's going away. + * 2. THE BELT (cloud#64 orphan case): list every subscription under the + * customer and cancel every one still in {@link LIVE_SUBSCRIPTION_STATUSES} + * — this catches a live subscription the `users` row doesn't name. + * Without it, deleting an account can leave a subscription billing a + * ghost forever. Fetches `status: "all"` and filters client-side rather + * than trusting Stripe's default list scope, so nothing in that status + * set is silently excluded. + * 3. Delete the customer (tolerant of already-deleted). + * 4. On full success, NULL both Stripe ids on the user row — that NULL is + * the converged marker the sweep keys on to stop retrying. On ANY + * failure along the way, leave the ids untouched, log it, and return an + * unconverged result so the sweep retries the whole sequence next pass + * (steps 1-3 are each individually idempotent, so a retry from the top + * is always safe). + * + * Idempotent as a whole: a user row whose Stripe ids are already NULL (a + * prior run converged, or there was never anything to tear down) short- + * circuits with no Stripe calls at all. + */ +export async function teardownBilling(stripe: Stripe, db: D1Database, userId: string): Promise { + const user = await getUserById(db, userId); + if (!user || (user.stripeCustomerId === null && user.stripeSubscriptionId === null)) { + return { converged: true }; + } + + try { + const canceled = new Set(); + if (user.stripeSubscriptionId) { + await cancelSubscriptionTolerantly(stripe, user.stripeSubscriptionId); + canceled.add(user.stripeSubscriptionId); + } + + if (user.stripeCustomerId) { + const subs = await stripe.subscriptions.list({ customer: user.stripeCustomerId, status: "all" }); + for (const sub of subs.data) { + if (canceled.has(sub.id) || !LIVE_SUBSCRIPTION_STATUSES.has(sub.status)) continue; + await cancelSubscriptionTolerantly(stripe, sub.id); + canceled.add(sub.id); + } + + try { + await stripe.customers.del(user.stripeCustomerId); + } catch (err) { + if (!isTolerableStripeError(err)) throw err; + } + } + + await db + .prepare("UPDATE users SET stripe_customer_id = NULL, stripe_subscription_id = NULL WHERE id = ?") + .bind(userId) + .run(); + return { converged: true }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + console.error(`event=billing_teardown_failed user=${userId} error=${message}`); + return { converged: false, error: message }; + } +} diff --git a/workers/identity/test/billing-teardown.test.ts b/workers/identity/test/billing-teardown.test.ts new file mode 100644 index 0000000..a4727c1 --- /dev/null +++ b/workers/identity/test/billing-teardown.test.ts @@ -0,0 +1,300 @@ +/** + * billing-teardown.ts — the account-delete train's A-2 (cloud#226 sibling): + * deferBilling (the reversible hold), resumeBilling (undo), teardownBilling + * (the real, irreversible sweep-time teardown). No route calls this yet + * (A-3 wires it) — these tests exercise the three functions directly against + * an injected Stripe stub (the same seam billing-lifecycle.ts's tests use: + * a plain object satisfying the subset of the SDK these functions call, + * `as unknown as Stripe`), no network. + */ +import Stripe from "stripe"; +import { describe, expect, test } from "vitest"; +import { + type ResumeBillingResult, + type TeardownBillingResult, + deferBilling, + resumeBilling, + teardownBilling, +} from "../src/billing-teardown.ts"; +import { db, seedUser } from "./helpers.ts"; + +// --- the injected Stripe stub --------------------------------------------- + +/** Stripe's "this doesn't exist / already in a terminal state" shape — the + * ONE class every function under test treats as tolerable. */ +function tolerableError(message: string): InstanceType { + return new Stripe.errors.StripeInvalidRequestError({ message, statusCode: 404, code: "resource_missing" }); +} + +interface StripeStubOptions { + /** subscription ids whose cancel/update should behave as "already gone". */ + tolerableSubIds?: Set; + /** subscription ids whose cancel/update should throw a genuine (non-tolerable) failure. */ + hardFailSubIds?: Set; + /** subscriptions.list({customer}) canned response, keyed by customer id. */ + listByCustomer?: Record>; + /** customer ids whose .del() should behave as "already deleted". */ + tolerableDeleteCustomerIds?: Set; + /** customer ids whose .del() should throw a genuine (non-tolerable) failure. */ + hardFailDeleteCustomerIds?: Set; +} + +interface StripeStubCalls { + updated: Array<{ id: string; cancelAtPeriodEnd: boolean }>; + canceled: string[]; + listed: string[]; + deletedCustomers: string[]; +} + +function makeStripeStub(opts: StripeStubOptions = {}): { stub: Stripe; calls: StripeStubCalls } { + const calls: StripeStubCalls = { updated: [], canceled: [], listed: [], deletedCustomers: [] }; + const stub = { + subscriptions: { + update: async (id: string, params: { cancel_at_period_end?: boolean }) => { + calls.updated.push({ id, cancelAtPeriodEnd: params.cancel_at_period_end ?? false }); + if (opts.hardFailSubIds?.has(id)) throw new Error(`hard failure updating ${id}`); + if (opts.tolerableSubIds?.has(id)) throw tolerableError(`No such subscription: ${id}`); + return { id, status: "active" }; + }, + cancel: async (id: string) => { + calls.canceled.push(id); + if (opts.hardFailSubIds?.has(id)) throw new Error(`hard failure canceling ${id}`); + if (opts.tolerableSubIds?.has(id)) throw tolerableError(`No such subscription: ${id}`); + return { id, status: "canceled" }; + }, + list: async (params: { customer: string }) => { + calls.listed.push(params.customer); + return { object: "list", data: opts.listByCustomer?.[params.customer] ?? [], has_more: false, url: "/v1/subscriptions" }; + }, + }, + customers: { + del: async (id: string) => { + calls.deletedCustomers.push(id); + if (opts.hardFailDeleteCustomerIds?.has(id)) throw new Error(`hard failure deleting customer ${id}`); + if (opts.tolerableDeleteCustomerIds?.has(id)) throw tolerableError(`No such customer: ${id}`); + return { id, object: "customer", deleted: true }; + }, + }, + } as unknown as Stripe; + return { stub, calls }; +} + +async function seedBilledUser( + email: string, + opts: { customer?: string | null; subscription?: string | null } = {}, +): Promise<{ id: string }> { + const { id } = await seedUser(email); + await db() + .prepare("UPDATE users SET stripe_customer_id = ?, stripe_subscription_id = ? WHERE id = ?") + .bind(opts.customer ?? "cus_test_1", opts.subscription ?? "sub_test_1", id) + .run(); + return { id }; +} + +async function stripeIdsOf(userId: string): Promise<{ customer: string | null; subscription: string | null }> { + const row = await db() + .prepare("SELECT stripe_customer_id, stripe_subscription_id FROM users WHERE id = ?") + .bind(userId) + .first<{ stripe_customer_id: string | null; stripe_subscription_id: string | null }>(); + return { customer: row?.stripe_customer_id ?? null, subscription: row?.stripe_subscription_id ?? null }; +} + +// --- deferBilling — the reversible hold ----------------------------------- + +describe("deferBilling — sets the reversible hold", () => { + test("sets cancel_at_period_end = true on the stored subscription", async () => { + const { stub, calls } = makeStripeStub(); + await deferBilling(stub, "sub_defer_1"); + expect(calls.updated).toEqual([{ id: "sub_defer_1", cancelAtPeriodEnd: true }]); + }); + + test("a missing (null) subscription id is a no-op success — no Stripe call at all", async () => { + const { stub, calls } = makeStripeStub(); + await expect(deferBilling(stub, null)).resolves.toBeUndefined(); + expect(calls.updated).toEqual([]); + }); + + test("tolerant of a 404 / unknown subscription id", async () => { + const { stub } = makeStripeStub({ tolerableSubIds: new Set(["sub_gone"]) }); + await expect(deferBilling(stub, "sub_gone")).resolves.toBeUndefined(); + }); + + test("tolerant of an already-canceled subscription", async () => { + // Same tolerable error class Stripe uses for "you cannot update a + // canceled subscription" — deferBilling must not distinguish the two. + const { stub } = makeStripeStub({ tolerableSubIds: new Set(["sub_already_canceled"]) }); + await expect(deferBilling(stub, "sub_already_canceled")).resolves.toBeUndefined(); + }); + + test("a genuine Stripe failure propagates — the caller must know the hold may not have landed", async () => { + const { stub } = makeStripeStub({ hardFailSubIds: new Set(["sub_boom"]) }); + await expect(deferBilling(stub, "sub_boom")).rejects.toThrow("hard failure updating sub_boom"); + }); +}); + +// --- resumeBilling — release the hold ------------------------------------- + +describe("resumeBilling — releases the hold (or says plainly it couldn't)", () => { + test("flips cancel_at_period_end back to false → { resumed: true }", async () => { + const { stub, calls } = makeStripeStub(); + const result: ResumeBillingResult = await resumeBilling(stub, "sub_resume_1"); + expect(result).toEqual({ resumed: true }); + expect(calls.updated).toEqual([{ id: "sub_resume_1", cancelAtPeriodEnd: false }]); + }); + + test("a missing (null) subscription id: nothing to release → { resumed: true }, no Stripe call", async () => { + const { stub, calls } = makeStripeStub(); + await expect(resumeBilling(stub, null)).resolves.toEqual({ resumed: true }); + expect(calls.updated).toEqual([]); + }); + + test("THE MONEY CASE: the period boundary passed during the undo window — Stripe has no un-cancel. Must return the not-resumed result, never throw, never claim success", async () => { + const { stub } = makeStripeStub({ tolerableSubIds: new Set(["sub_boundary_passed"]) }); + const result = await resumeBilling(stub, "sub_boundary_passed"); + expect(result).toEqual({ resumed: false, reason: "already_canceled" }); + }); + + test("a genuine Stripe failure propagates (not silently swallowed into a resumed/not-resumed answer)", async () => { + const { stub } = makeStripeStub({ hardFailSubIds: new Set(["sub_boom"]) }); + await expect(resumeBilling(stub, "sub_boom")).rejects.toThrow("hard failure updating sub_boom"); + }); +}); + +// --- teardownBilling — the real, irreversible teardown -------------------- + +describe("teardownBilling — cancel everything, delete the customer, NULL the ids", () => { + test("happy path: cancels the stored subscription, deletes the customer, NULLs both ids", async () => { + const { id } = await seedBilledUser("teardown-happy@example.com", { customer: "cus_happy", subscription: "sub_happy" }); + const { stub, calls } = makeStripeStub({ listByCustomer: { cus_happy: [] } }); + + const result = await teardownBilling(stub, db(), id); + expect(result).toEqual({ converged: true }); + expect(calls.canceled).toEqual(["sub_happy"]); + expect(calls.deletedCustomers).toEqual(["cus_happy"]); + expect(await stripeIdsOf(id)).toEqual({ customer: null, subscription: null }); + }); + + test("THE MONEY TEST: a trialing subscription found only via the belt (not the stored id) is canceled — fails if the belt filters `active` only", async () => { + // The stored subscription id is DELIBERATELY something else (or absent), + // so the trialing subscription's cancellation can ONLY come from the + // belt's subscriptions.list() sweep — never from the direct stored-id + // cancel. This is the assertion that catches the specific bug: a belt + // that filters to `active` only would skip `sub_trialing` entirely, and + // `calls.canceled` would never contain it. + const { id } = await seedBilledUser("teardown-trialing@example.com", { customer: "cus_trialing", subscription: null }); + const { stub, calls } = makeStripeStub({ + listByCustomer: { cus_trialing: [{ id: "sub_trialing", status: "trialing" }] }, + }); + + const result = await teardownBilling(stub, db(), id); + expect(result).toEqual({ converged: true }); + expect(calls.canceled).toContain("sub_trialing"); // the money assertion + expect(calls.deletedCustomers).toEqual(["cus_trialing"]); + expect(await stripeIdsOf(id)).toEqual({ customer: null, subscription: null }); + }); + + test("THE ORPHAN BELT (cloud#64): stored id A + subscriptions.list returns A and an unrelated live B — both canceled, then the customer deleted, then both ids NULLed", async () => { + const { id } = await seedBilledUser("teardown-orphan@example.com", { customer: "cus_orphan", subscription: "sub_a" }); + const { stub, calls } = makeStripeStub({ + listByCustomer: { + cus_orphan: [ + { id: "sub_a", status: "active" }, + { id: "sub_b", status: "active" }, // the orphan the users row never named + ], + }, + }); + + const result = await teardownBilling(stub, db(), id); + expect(result).toEqual({ converged: true }); + // sub_a is canceled exactly once (the direct step + the belt de-dupe — + // not a redundant second Stripe call for the same id). + expect(calls.canceled.filter((s) => s === "sub_a")).toHaveLength(1); + expect(calls.canceled).toContain("sub_b"); + expect(calls.deletedCustomers).toEqual(["cus_orphan"]); + expect(await stripeIdsOf(id)).toEqual({ customer: null, subscription: null }); + }); + + test("belt statuses: past_due and unpaid are swept too; a subscription already canceled in Stripe is left alone (no redundant cancel)", async () => { + const { id } = await seedBilledUser("teardown-statuses@example.com", { customer: "cus_statuses", subscription: null }); + const { stub, calls } = makeStripeStub({ + listByCustomer: { + cus_statuses: [ + { id: "sub_pastdue", status: "past_due" }, + { id: "sub_unpaid", status: "unpaid" }, + { id: "sub_already_gone", status: "canceled" }, + { id: "sub_incomplete", status: "incomplete" }, + ], + }, + }); + + await teardownBilling(stub, db(), id); + expect(calls.canceled).toContain("sub_pastdue"); + expect(calls.canceled).toContain("sub_unpaid"); + expect(calls.canceled).not.toContain("sub_already_gone"); + expect(calls.canceled).not.toContain("sub_incomplete"); + }); + + test("PARTIAL FAILURE: customers.del throws → ids retained, an unconverged result is returned, no exception escapes", async () => { + const { id } = await seedBilledUser("teardown-partial@example.com", { customer: "cus_partial", subscription: "sub_partial" }); + const { stub } = makeStripeStub({ + listByCustomer: { cus_partial: [] }, + hardFailDeleteCustomerIds: new Set(["cus_partial"]), + }); + + let result: TeardownBillingResult | undefined; + await expect( + (async () => { + result = await teardownBilling(stub, db(), id); + })(), + ).resolves.toBeUndefined(); // never throws out of the sweep + expect(result?.converged).toBe(false); + expect((result as { converged: false; error: string }).error).toContain("hard failure deleting customer cus_partial"); + // The ids are NOT nulled — the sweep must retry the whole sequence next pass. + expect(await stripeIdsOf(id)).toEqual({ customer: "cus_partial", subscription: "sub_partial" }); + }); + + test("PARTIAL FAILURE mid-belt: canceling the orphan throws → unconverged, ids retained, customers.del never reached", async () => { + const { id } = await seedBilledUser("teardown-belt-fail@example.com", { customer: "cus_beltfail", subscription: "sub_stored" }); + const { stub, calls } = makeStripeStub({ + listByCustomer: { cus_beltfail: [{ id: "sub_orphan_boom", status: "active" }] }, + hardFailSubIds: new Set(["sub_orphan_boom"]), + }); + + const result = await teardownBilling(stub, db(), id); + expect(result.converged).toBe(false); + expect(calls.deletedCustomers).toEqual([]); // never reached + expect(await stripeIdsOf(id)).toEqual({ customer: "cus_beltfail", subscription: "sub_stored" }); + }); + + test("IDEMPOTENCE: running teardown twice reaches the same terminal state without throwing — the second pass makes no Stripe calls", async () => { + const { id } = await seedBilledUser("teardown-idempotent@example.com", { customer: "cus_idem", subscription: "sub_idem" }); + const { stub, calls } = makeStripeStub({ listByCustomer: { cus_idem: [] } }); + + const first = await teardownBilling(stub, db(), id); + expect(first).toEqual({ converged: true }); + expect(await stripeIdsOf(id)).toEqual({ customer: null, subscription: null }); + + const callsBeforeSecondRun = calls.canceled.length + calls.deletedCustomers.length + calls.listed.length; + const second = await teardownBilling(stub, db(), id); + expect(second).toEqual({ converged: true }); + // Already-converged (NULL ids) short-circuits before any Stripe call. + expect(calls.canceled.length + calls.deletedCustomers.length + calls.listed.length).toBe(callsBeforeSecondRun); + expect(await stripeIdsOf(id)).toEqual({ customer: null, subscription: null }); + }); + + test("a user who never had Stripe ids converges trivially with no Stripe calls", async () => { + const { id } = await seedUser("teardown-never-billed@example.com"); // fresh trial, no Stripe ids + const { stub, calls } = makeStripeStub(); + const result = await teardownBilling(stub, db(), id); + expect(result).toEqual({ converged: true }); + expect(calls.canceled).toEqual([]); + expect(calls.listed).toEqual([]); + expect(calls.deletedCustomers).toEqual([]); + }); + + test("a nonexistent user id converges trivially (nothing to tear down)", async () => { + const { stub } = makeStripeStub(); + const result = await teardownBilling(stub, db(), "no-such-user-id"); + expect(result).toEqual({ converged: true }); + }); +}); From 18e756eed7c1181c7f1697a939c9926a00e7eb40 Mon Sep 17 00:00:00 2001 From: Aaron Gabriel Date: Mon, 27 Jul 2026 18:30:33 -0600 Subject: [PATCH 2/2] fix(billing-teardown-tests): seedBilledUser coalesced null subscription ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `opts.subscription ?? "sub_test_1"` treated an explicit `null` (no stored subscription — the cloud#64 orphan shape) the same as omitted, silently substituting the stub default. Two existing tests intended to exercise `teardownBilling`'s `if (user.stripeSubscriptionId)` false branch never actually reached it. Switch to `=== undefined` so null and omitted are distinct, and add a dedicated test for the null-subscription-id + live-belt case, watched red on unfixed code and again with the belt broken before confirming green. Fold per PR #230 review (cloud#226 A-2). --- .../identity/test/billing-teardown.test.ts | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/workers/identity/test/billing-teardown.test.ts b/workers/identity/test/billing-teardown.test.ts index a4727c1..ff1b44e 100644 --- a/workers/identity/test/billing-teardown.test.ts +++ b/workers/identity/test/billing-teardown.test.ts @@ -84,9 +84,19 @@ async function seedBilledUser( opts: { customer?: string | null; subscription?: string | null } = {}, ): Promise<{ id: string }> { const { id } = await seedUser(email); + // `??` coalesces on `null` as well as `undefined`, so `opts.subscription ?? + // "sub_test_1"` would silently discard a caller's DELIBERATE `null` (no + // stored subscription) and substitute the stub default — the exact bug + // that let the "genuinely-null" test above pass on unfixed code without + // exercising the branch it names. `=== undefined` treats "omitted" and + // "explicitly null" as the two distinct things they are. await db() .prepare("UPDATE users SET stripe_customer_id = ?, stripe_subscription_id = ? WHERE id = ?") - .bind(opts.customer ?? "cus_test_1", opts.subscription ?? "sub_test_1", id) + .bind( + opts.customer === undefined ? "cus_test_1" : opts.customer, + opts.subscription === undefined ? "sub_test_1" : opts.subscription, + id, + ) .run(); return { id }; } @@ -214,6 +224,26 @@ describe("teardownBilling — cancel everything, delete the customer, NULL the i expect(await stripeIdsOf(id)).toEqual({ customer: null, subscription: null }); }); + test("a genuinely-null stored subscription id (the cloud#64 orphan shape: customer exists, stored sub id lost or never set) — the belt is the ONLY path to a live subscription, and teardown still cancels it, deletes the customer, and NULLs both ids", async () => { + const { id } = await seedBilledUser("teardown-null-subscription@example.com", { customer: "cus_nullsub", subscription: null }); + // Pin the seed itself: this must be a REAL null in the row, not the helper's + // stub default silently substituted for it — that substitution is exactly + // the bug this test exists to catch. + expect(await stripeIdsOf(id)).toEqual({ customer: "cus_nullsub", subscription: null }); + + const { stub, calls } = makeStripeStub({ + listByCustomer: { cus_nullsub: [{ id: "sub_only_in_belt", status: "active" }] }, + }); + + const result = await teardownBilling(stub, db(), id); + expect(result).toEqual({ converged: true }); + // Exact equality, not toContain: with a real (non-null) stripeSubscriptionId + // there would ALSO be a direct-step cancel call, which toContain wouldn't catch. + expect(calls.canceled).toEqual(["sub_only_in_belt"]); + expect(calls.deletedCustomers).toEqual(["cus_nullsub"]); + expect(await stripeIdsOf(id)).toEqual({ customer: null, subscription: null }); + }); + test("belt statuses: past_due and unpaid are swept too; a subscription already canceled in Stripe is left alone (no redundant cancel)", async () => { const { id } = await seedBilledUser("teardown-statuses@example.com", { customer: "cus_statuses", subscription: null }); const { stub, calls } = makeStripeStub({