fix(subscriptions): make cancel_at a real schedule signal and harden the plan-change state machine (#545) - #558
Merged
Conversation
…the plan-change state machine (#545) `subscriptions.cancel_at` shipped as NOT NULL DEFAULT now() with no writer ever advancing it, so every row was born with a cancel date permanently in the past. Three bugs fell out of that: 1. The Solana crank ORed `cancel_at <= now()` into its cancel decision, so it cancelled EVERY subscription at its first rollover. 2. `change_subscription_plan` writes `cancel_at = NULL` on the reactivate branch, which the NOT NULL column rejected with 23502 — every A -> B -> A plan switch failed. 3. Nothing gated price: a student on a free plan could switch to a paid one, and a self-managed (non-Stripe/LS) subscriber could upgrade, with no payment taken. Schema (20260726120000): `cancel_at` becomes nullable with no default, `cancel_at_period_end` becomes NOT NULL DEFAULT false, and a CHECK (`subscriptions_cancel_at_requires_schedule`) pins the contract — a cancel date may exist only while the flag is set. `cancel_at_period_end` is now the ONLY signal that a cancel is scheduled; `cancel_at` is informational. `canceled_at`/`ended_at` lose their now() defaults too, and live rows are backfilled clean. handle_new_subscription (20260726120100): the parallel-subscription backstop now counts `renewed` as live, and a resubscribe clears the stale cancel fields instead of inheriting them. change_subscription_plan (20260726120200): takes a per-user advisory lock so a double-submit cannot build two subscriptions, writes `cancel_at = NULL` alongside the flag, refuses an upgrade that would not settle natively (`upgrade_requires_payment`), and carries a pending cancel across the swap instead of silently dropping or resurrecting it. Application side: - solana-pull-decision reads only `cancel_at_period_end`; a stale date no longer cancels a healthy subscription. - `renewed` added to BLOCKING_SUBSCRIPTION_STATUSES (it bills and grants access, so it must block a parallel checkout). - The `subscription.past_due` webhook branch now actually writes the status instead of dropping the event. (Refund handling is deliberately untouched.) - Cancelling never IMPROVES a status: a past_due student who cancels stays past_due rather than being rewritten as healthy. Reactivating clears `cancel_at` with the flag. - Billing UI treats past_due as live (it was falling through to the "no subscription" empty state mid-dunning), shows a dunning notice, and only offers same-or-cheaper plans when the provider cannot settle the swap. Tests: new `subscription-plan-change-rpc.spec.ts` drives the RPC against a real DB as an authenticated student (A -> B, A -> B -> A, cancel-then-switch, same_plan, cross-tenant, the price gate, the advisory lock), asserting the entitlement delta per course via `has_course_access`. plan-change.spec.ts previously CLAIMED that coverage while nothing exercised the RPC at all — which is how bugs 2 and 3 shipped; its header now points at the real spec. Each fix was proven load-bearing by restoring the old schema/function and watching the new tests go red (A -> B -> A reproduced the exact 23502). Unit: 427/427 (411 on master). Playwright plan-change + entitlement batch 19/19. Closes #545 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WiXe7s6snHsso7q9LE9gCq
guillermoscript
force-pushed
the
fix/subscription-state-machine-545
branch
from
July 26, 2026 14:52
5cdcfa4 to
f3e53fb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
subscriptions.cancel_atshipped asNOT NULL DEFAULT now()with no writer ever advancing it, so every subscription row was born with a cancel date permanently in the past. This makescancel_at_period_endthe single source of truth for "a cancel is scheduled", fixes the three bugs that fell out of the old contract, and hardens the rest of the subscription state machine.Closes #545
Why
Three hard bugs, all downstream of the same column:
solana-pull-decision.tsORedcancel_at <= now()into the cancel branch, andcancel_atwas alwaysnow()-at-insert.23502.change_subscription_plan's reactivate branch writescancel_at = NULLinto what was a NOT NULL column.Plus five smaller state bugs listed in the issue.
Schema —
20260726120000cancel_at→ nullable, no default.cancel_at_period_endis now the ONLY signal that a cancel is scheduled;cancel_atis purely informational.cancel_at_period_end→NOT NULL DEFAULT false.subscriptions_cancel_at_requires_schedule: a cancel date may exist only while the flag is set, so the two can never drift apart again.canceled_at/ended_atlose theirnow()defaults; still-live rows are backfilled clean.handle_new_subscription—20260726120100renewedas live.change_subscription_plan—20260726120200pg_advisory_xact_lockper (user, tenant) — a double-submit can no longer build two subscriptions.cancel_at = NULLalongside the flag (bug 2).upgrade_requires_payment(bug 3).Application
solana-pull-decisionreads onlycancel_at_period_end; a stale date no longer cancels a healthy subscription.renewedadded toBLOCKING_SUBSCRIPTION_STATUSES— it bills and grants access, so it must block a parallel checkout.subscription.past_duewebhook branch now actually writes the status instead of dropping the event. Refund handling is deliberately untouched (separate work).past_duestudent who cancels stayspast_duerather than being rewritten as healthy in billing health and the admin views. Reactivating clearscancel_attogether with the flag.past_dueas live — it was falling through to the "no subscription" empty state mid-dunning, so a student being charged had no cancel, no switch and no explanation. Adds a dunning notice, and only offers same-or-cheaper plans when the provider cannot settle the swap itself.How to QA
On a fresh
npm run db:reset(the three migrations apply from files):cancel_at IS NULL, cancel_at_period_end = false:alice@student.com/password123oncode-academy.lvh.me:3000→/en/dashboard/student/billing→ Change plan → switch, then switch back. Before this PR the second hop died with23502.past_duewith the service key and reload billing: the card renders badged "Past due" with the dunning notice, and both Change plan and Cancel are still offered (it used to show the empty state).change_subscription_planfor a more expensive plan on a manual/Solana subscription:upgrade_requires_payment, and no transaction is created.npx playwright test subscription-plan-change-rpc --workers=1.Screenshots / GIF
No new visual surface beyond the
past_duedunning notice, which is asserted directly inplan-change.spec.ts(badge + notice + both action buttons render for apast_duesubscription).Tests
tests/playwright/subscription-plan-change-rpc.spec.ts(11 tests) driveschange_subscription_planagainst a real DB as an authenticated student on an isolated QA user + 5 QA plans: A → B, A → B → A, cancel-then-switch,same_planwrites nothing, cross-tenant unreachable, free → paid refused, self-managed upgrade refused, same-price allowed, switch-down-to-free, the advisory lock under double-submit, and the cancel-state contract. Entitlement deltas are asserted per course and cross-checked against thehas_course_accessRPC.plan-change.spec.tspreviously claimed exactly this coverage in its header while nothing exercised the RPC at all — which is how bugs 2 and 3 shipped. Its header now points at the real spec and says so.tests/unit/subscription-state-machine.test.ts(9 tests) pins cancel/reactivate/guard behaviour;solana-pull-decision.test.tsfixtures rewritten (the old ones usedcancel_at: null, impossible under the shipped schema) and now include a legacy-poisoned row and three consecutive rollovers.23502from the issue.npm run test:unit427/427 (master baseline 411). Playwright plan-change + entitlement batch 19/19.Deliberately out of scope
expire-subscriptionscron still filterssubscription_status = 'active'. Addingpast_duethere would wrongly expire native subscriptions mid-dunning — that belongs with the dunning work.cancel_at_period_endtoggle still never syncs back to studentsubscriptionsrows (pre-existing).Pre-existing failures noticed while running the suite (not caused by this PR)
subscription-lapse.spec.tsasserts on course10005, which does not exist inseed.sqlat all (plan_coursesonly has(2001,2001)and(2001,2002)).entitlements-overlap.spec.tsdeletes alice's plan-2001 transaction — cascading her subscription — beforeparallel-subscription-guard.spec.tsruns, which then fails 3/5 with 400s instead of 409s. It passes 5/5 in isolation after a reseed.Checklist
npm run typecheckandnpm run test:unitpassnpm run buildpassestenant_idupgrade_requires_paymentand the constraint codes are mapped to user-facing copy inmapPlanChangeError)messages/en.jsonandmessages/es.jsonnpm run db:reset, ship rollback files, andlib/database.types.tswas regenerated🤖 Generated with Claude Code
https://claude.ai/code/session_01WiXe7s6snHsso7q9LE9gCq