fix(subscriptions): make cancel_at a real schedule signal and harden the plan-change state machine (#545) (#558)

`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


Claude-Session: https://claude.ai/code/session_01WiXe7s6snHsso7q9LE9gCq

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: guillermoscript

CLAUDE.md

@@ -105,6 +105,7 @@ Also supported (`products.payment_provider`): `paypal`, `lemonsqueezy`, `solana`
 
 **Key invariants:**
 - Transaction `status`: `pending`, `successful`, `failed`, `archived`, `canceled`, `refunded`
+- **`subscriptions.cancel_at_period_end` is the ONLY signal that a cancel is scheduled** (since `20260726120000`, issue #545). `cancel_at` is nullable and purely informational — the CHECK `subscriptions_cancel_at_requires_schedule` allows it to be non-NULL only while the flag is set, so clear both together (both reactivate actions do). It shipped as `NOT NULL DEFAULT now()` with no writer setting it, which made the Solana crank cancel every subscription at its first rollover. `subscription_status` is `active`/`canceled`/`expired`/`renewed`/`past_due`; `renewed` and `past_due` both count as LIVE (parallel-subscription guards, billing UI, plan change), and cancelling must never *improve* a status
 - **Access control lives in `entitlements`, not `enrollments`** (since migration `20260516150000`): `entitlements` (`user_id`, `course_id`, `tenant_id`, `source_type`, `source_id`, `status`, `expires_at`) is the polymorphic source of truth for product/subscription access. `enrollments.product_id`/`subscription_id` and their old CHECK constraint were dropped — `enrollments` is now a learning-progress record only (`user_id`, `course_id`, `status`, `tenant_id`, `enrollment_date`)
 - `enroll_user()` RPC loops through ALL courses for a product (a product can map to multiple courses via `product_courses`) and writes to `entitlements`
 - **Subscriptions grant access, not auto-enrollment** — students self-enroll via `/dashboard/student/browse` (`useEnrollment()` hook); `plan_courses` defines which courses a plan covers

app/[locale]/(public)/checkout/actions.ts

@@ -249,9 +249,33 @@ function mapPlanChangeError(raw: string): string {
     if (raw.includes('no_active_subscription')) return "You don't have an active subscription to change.";
     if (raw.includes('invalid_plan') || raw.includes('cross_tenant_plan')) return "Plan not found.";
     if (raw.includes('plan_deleted')) return "That plan is no longer available.";
+    if (raw.includes('upgrade_requires_payment')) {
+        return "This plan costs more than your current one. Contact your school to arrange the payment for the upgrade.";
+    }
+    // 23502 (not-null) / 23514 (check) on the subscriptions cancel columns. The
+    // #545 schema change should make these unreachable; before it, the
+    // reactivate branch of the RPC wrote NULL into a NOT NULL `cancel_at` and
+    // every switch back to a previously-held plan died here behind the generic
+    // message below, with no clue for the student or the logs.
+    if (raw.includes('23502') || raw.includes('23514') || raw.includes('cancel_at')) {
+        return "We couldn't switch your plan because of a problem on our side. Please contact your school.";
+    }
     return "Could not change your plan. Please try again.";
 }
 
+/**
+ * RPC errors raised BEFORE the function writes anything, where our DB is
+ * already in the state the caller wanted (or never had a subscription to
+ * change). Compensating a native provider swap on one of these actively breaks
+ * the pairing: on a double-click, call A swaps the provider to plan B and
+ * commits the DB; call B swaps to B again, gets `same_plan`, and a blind revert
+ * would put the provider back on plan A's price with `proration_behavior:
+ * 'none'` — DB on B, billing A, no error the student ever sees (#545).
+ */
+function isNoOpPlanChangeError(raw: string): boolean {
+    return raw.includes('same_plan') || raw.includes('no_active_subscription');
+}
+
 /**
  * Switch the student's current subscription to a different plan of the SAME
  * school + payment provider (supersession — never two live subs; issue #463).
@@ -360,8 +384,12 @@ export async function changePlan(newPlanId?: string) {
             });
             if (rpcError) {
                 // Compensate: put the provider subscription back on the old price
-                // so billing and our DB stay consistent.
-                if (currentPlan?.provider_price_id) {
+                // so billing and our DB stay consistent — but only when the RPC
+                // genuinely failed to apply the switch. `same_plan` /
+                // `no_active_subscription` mean our DB is already where the
+                // student wanted it (or never had a subscription to move), so a
+                // revert would DESYNC the provider rather than restore it.
+                if (currentPlan?.provider_price_id && !isNoOpPlanChangeError(rpcError.message)) {
                     try {
                         await provider.updateSubscription(current.provider_subscription_id, {
                             newProviderPriceId: currentPlan.provider_price_id,

app/[locale]/dashboard/student/billing/page.tsx

@@ -51,8 +51,20 @@ interface Subscription {
   cancel_at_period_end: boolean | null
   plan_id: number
   payment_provider: string
+  provider_subscription_id: string | null
 }
 
+/**
+ * Statuses whose subscription is the student's LIVE one — the row this page
+ * renders and offers cancel / switch controls for.
+ *
+ * `past_due` belongs here (#545). It used to be excluded, so a student mid
+ * dunning got the "no subscription" empty state: neither ChangePlanDialog nor
+ * ManageSubscription mounted and they could neither cancel nor switch while
+ * still being billed, even though both server actions accept the status.
+ */
+const LIVE_SUBSCRIPTION_STATUSES: SubscriptionStatus[] = ['active', 'renewed', 'past_due']
+
 interface PaymentRequest {
   request_id: number
   status: string
@@ -197,7 +209,7 @@ export default async function StudentBillingPage() {
   // ── 2. Subscriptions ────────────────────────────────────────────────────────
   const { data: rawSubs } = await supabase
     .from('subscriptions')
-    .select('subscription_id, subscription_status, current_period_end, end_date, cancel_at_period_end, plan_id, payment_provider')
+    .select('subscription_id, subscription_status, current_period_end, end_date, cancel_at_period_end, plan_id, payment_provider, provider_subscription_id')
     .eq('user_id', userId)
     .eq('tenant_id', tenantId)
     .order('created', { ascending: false })
@@ -235,9 +247,10 @@ export default async function StudentBillingPage() {
   }
 
   // Derived
-  const activeSubscription = subscriptions.find(
-    (s) => s.subscription_status === 'active' || s.subscription_status === 'renewed'
+  const activeSubscription = subscriptions.find((s) =>
+    LIVE_SUBSCRIPTION_STATUSES.includes(s.subscription_status)
   )
+  const isPastDue = activeSubscription?.subscription_status === 'past_due'
 
   // ── Plan-change (switch) support ────────────────────────────────────────────
   // Switchable when the provider does an in-place swap (Stripe/LS) OR is
@@ -250,14 +263,44 @@ export default async function StudentBillingPage() {
     !!activeCaps &&
     (activeCaps.supportsPlanChange || !activeCaps.supportsNativeSubscriptions)
 
+  // Does this subscription settle a price difference by itself? Only an
+  // in-place provider swap with proration does (Stripe / Lemon Squeezy, and
+  // only when we actually hold a provider subscription to swap). Mirrors
+  // `_settles_natively` in change_subscription_plan.
+  const settlesNatively =
+    !!activeCaps?.supportsPlanChange && !!activeSubscription?.provider_subscription_id
+
   let switchablePlans: SwitchablePlan[] = []
-  if (canSwitchPlan) {
-    const { data: rawPlans } = await supabase
-      .from('plans')
-      .select('plan_id, plan_name, price, payment_provider')
-      .eq('tenant_id', tenantId)
-      .is('deleted_at', null)
-    switchablePlans = (rawPlans ?? []) as SwitchablePlan[]
+  if (canSwitchPlan && activeSubscription) {
+    const [{ data: rawPlans }, { data: currentPlanRow }] = await Promise.all([
+      supabase
+        .from('plans')
+        .select('plan_id, plan_name, price, payment_provider')
+        .eq('tenant_id', tenantId)
+        .is('deleted_at', null),
+      supabase.from('plans').select('price').eq('plan_id', activeSubscription.plan_id).maybeSingle(),
+    ])
+    const allPlans = (rawPlans ?? []) as SwitchablePlan[]
+
+    // Price gate, server-side (#545). This list used to be every non-deleted
+    // tenant plan, so a student on a one-click FREE plan could pick the
+    // school's paid bank-transfer plan and walk away with its entitlements —
+    // no transaction, no payment_requests row, nothing for an admin to
+    // reconcile. Anything that does not settle natively may only move to a
+    // plan costing the same or less; the RPC refuses the rest with
+    // `upgrade_requires_payment` regardless of what the UI offers.
+    //
+    // The current plan is read WITHOUT the `deleted_at` filter on purpose: a
+    // school can soft-delete a plan students are still subscribed to, and the
+    // gate needs its real price. Students can read those rows — "Students can
+    // view active plans" (20260330200000) is permissive on `tenant_id` alone,
+    // so it ORs past the `deleted_at IS NULL` policy. If the row is genuinely
+    // gone the price reads 0 and the list collapses to free-only, which fails
+    // closed rather than opening the gate.
+    const currentPrice = Number(currentPlanRow?.price ?? 0)
+    switchablePlans = settlesNatively
+      ? allPlans
+      : allPlans.filter((p) => Number(p.price) <= currentPrice)
   }
 
   return (
@@ -298,13 +341,24 @@ export default async function StudentBillingPage() {
               </div>
             </CardHeader>
             <CardContent className="pt-0 space-y-2">
+              {/* Dunning notice (#545): a past_due subscription still bills and
+                  still grants access during the provider's retry window, so say
+                  so instead of rendering it as if nothing were wrong. */}
+              {isPastDue && (
+                <div className="flex items-start gap-2 rounded-md bg-amber-50 dark:bg-amber-900/20 border border-amber-200 dark:border-amber-800 px-3 py-2 text-xs text-amber-800 dark:text-amber-400">
+                  <IconAlertTriangle className="w-3.5 h-3.5 shrink-0 mt-0.5" />
+                  <span>{t('subscription.pastDueNotice')}</span>
+                </div>
+              )}
               {/* Renewal / expiry date */}
               {(activeSubscription.current_period_end ?? activeSubscription.end_date) && (
                 <div className="flex items-center justify-between text-sm">
                   <span className="text-muted-foreground">
                     {activeSubscription.cancel_at_period_end
                       ? t('subscription.cancelsOn')
-                      : t('subscription.renewsOn')}
+                      : isPastDue
+                        ? t('subscription.paymentDueSince')
+                        : t('subscription.renewsOn')}
                   </span>
                   <span className="font-medium tabular-nums">
                     {formatDate(activeSubscription.current_period_end ?? activeSubscription.end_date)}
@@ -340,7 +394,7 @@ export default async function StudentBillingPage() {
                     <ChangePlanDialog
                       currentPlanId={activeSubscription.plan_id}
                       currentProvider={activeProvider}
-                      isNativeSwap={!!activeCaps?.supportsPlanChange}
+                      isNativeSwap={settlesNatively}
                       plans={switchablePlans}
                     />
                   )}

app/actions/admin/subscriptions.ts

@@ -62,9 +62,17 @@ export async function cancelSubscription(
       }
     }
 
+    // A period-end cancel keeps whatever live status the row already had —
+    // cancelling must never IMPROVE it (#545). Writing 'active' unconditionally
+    // erased a `past_due` delinquency from billing health and the admin views
+    // the moment anyone scheduled a cancel. Only the legacy `renewed` status is
+    // normalized to 'active'; nothing writes it any more.
+    const liveStatus =
+      subscription.subscription_status === 'renewed' ? 'active' : subscription.subscription_status
+
     const updates: any = {
-      // Use 'canceled' (correct enum value) for immediate; keep 'active' for period-end
-      subscription_status: immediate ? 'canceled' : 'active',
+      // Use 'canceled' (correct enum value) for immediate; keep the live status for period-end
+      subscription_status: immediate ? 'canceled' : liveStatus,
       canceled_at: new Date().toISOString(),
     }
 
@@ -160,18 +168,16 @@ export async function reactivateSubscription(subscriptionId: number) {
       }
     }
 
-    // Reactivate the subscription. NB: `cancel_at` is NOT NULL in the schema —
-    // setting it to null here would fail the update (a latent bug that silently
-    // broke admin reactivate). Clearing `cancel_at_period_end` un-schedules the
-    // subscription-expiry crons, but the Solana auto-pull crank
-    // (lib/payments/solana-pull-decision.ts) reads `cancel_at` on its own, so we
-    // push it forward to the live period end instead of leaving a past value that
-    // would re-cancel the row. `canceled_at` is cleared.
+    // Reactivate the subscription. `cancel_at` is cleared WITH the flag: since
+    // #545 it is nullable and the CHECK
+    // `subscriptions_cancel_at_requires_schedule` rejects a cancel date that
+    // outlives its schedule, so a leftover date can no longer be read as a live
+    // cancel by the Solana auto-pull crank. `canceled_at` is cleared too.
     const { error: updateError } = await supabase
       .from('subscriptions')
       .update({
         cancel_at_period_end: false,
-        cancel_at: subscription.current_period_end || subscription.end_date,
+        cancel_at: null,
         canceled_at: null,
       })
       .eq('subscription_id', subscriptionId)

app/actions/subscriptions.ts

@@ -75,15 +75,26 @@ export async function cancelMySubscription(subscriptionId: number): Promise<Acti
       }
     }
 
-    // Schedule the cancel in our DB. Status stays 'active' so access continues to
-    // period end; the cron / webhook flips it to expired when the period lapses.
+    // Schedule the cancel in our DB. The status stays live so access continues
+    // to period end; the cron / webhook flips it to expired when the period
+    // lapses.
+    //
+    // Cancelling must never IMPROVE a status (#545). This used to write
+    // 'active' unconditionally, so a student mid-dunning who cancelled had
+    // their `past_due` row rewritten as healthy and the delinquency vanished
+    // from billing health and the admin views. Only the legacy `renewed`
+    // status is normalized — it means the same thing as `active` and nothing
+    // else writes it any more.
+    const nextStatus =
+      subscription.subscription_status === 'renewed' ? 'active' : subscription.subscription_status
+
     const { error: updateError } = await supabase
       .from('subscriptions')
       .update({
         cancel_at_period_end: true,
         cancel_at: subscription.current_period_end || subscription.end_date,
         canceled_at: new Date().toISOString(),
-        subscription_status: 'active',
+        subscription_status: nextStatus,
       })
       .eq('subscription_id', subscriptionId)
       .eq('tenant_id', tenantId)
@@ -155,17 +166,15 @@ export async function reactivateMySubscription(subscriptionId: number): Promise<
       }
     }
 
-    // NB: `cancel_at` is NOT NULL in the schema — never set it to null here or the
-    // update is rejected. Clearing `cancel_at_period_end` un-schedules the
-    // subscription-expiry crons, but the Solana auto-pull crank
-    // (lib/payments/solana-pull-decision.ts) reads `cancel_at` on its own, so we
-    // push it forward to the live period end instead of leaving a past value that
-    // could re-cancel the row. `canceled_at` is cleared.
+    // Un-schedule the cancel. `cancel_at` is cleared WITH the flag: since #545
+    // it is nullable and the CHECK `subscriptions_cancel_at_requires_schedule`
+    // rejects a cancel date that outlives its schedule, so a leftover date can
+    // no longer be read as a live cancel by the Solana auto-pull crank.
     const { error: updateError } = await supabase
       .from('subscriptions')
       .update({
         cancel_at_period_end: false,
-        cancel_at: subscription.current_period_end || subscription.end_date,
+        cancel_at: null,
         canceled_at: null,
       })
       .eq('subscription_id', subscriptionId)

components/student/change-plan-dialog.tsx

@@ -55,6 +55,12 @@ export function ChangePlanDialog({
 
   // Only plans on the same provider (a cross-provider switch needs a new
   // checkout), excluding the plan the student is already on.
+  //
+  // `plans` is ALREADY price-gated by the billing page (#545) — a subscription
+  // whose provider does not settle the difference itself is handed only
+  // same-or-cheaper plans, so an unpaid upgrade is never on offer. This filter
+  // is presentation only; `change_subscription_plan` enforces the price rule
+  // server-side no matter what reaches this list.
   const switchable = plans.filter(
     (p) => p.plan_id !== currentPlanId && (p.payment_provider ?? 'manual') === currentProvider,
   )

components/student/manage-subscription.tsx

@@ -77,6 +77,7 @@ export function ManageSubscription({
         variant="outline"
         size="sm"
         className="gap-1.5"
+        data-testid="resume-subscription-button"
         onClick={handleReactivate}
         disabled={pending}
       >
@@ -92,6 +93,7 @@ export function ManageSubscription({
         variant="outline"
         size="sm"
         className="gap-1.5 text-destructive"
+        data-testid="cancel-subscription-button"
         onClick={() => setOpen(true)}
       >
         <IconX className="h-3.5 w-3.5" />

docs/DATABASE_SCHEMA.md

@@ -525,8 +525,8 @@ Student subscriptions to a school's plans. Tenant-scoped.
 | `start_date` / `end_date` | TIMESTAMPTZ | |
 | `current_period_start` / `current_period_end` | TIMESTAMPTZ | |
 | `trial_start` / `trial_end` | TIMESTAMPTZ | |
-| `cancel_at` | TIMESTAMPTZ | NOT NULL — must be advanced on renewal, or reactivation breaks |
-| `cancel_at_period_end` | BOOLEAN | Student self-cancel flow |
+| `cancel_at` | TIMESTAMPTZ | NULLABLE. When the subscription terminates, or NULL when no cancel is scheduled — informational, never a signal. CHECK `subscriptions_cancel_at_requires_schedule`: non-NULL only while `cancel_at_period_end` (#545) |
+| `cancel_at_period_end` | BOOLEAN | NOT NULL DEFAULT false. **The single source of truth for "a cancel is scheduled"** — read by the Solana crank, the expiry crons and the billing UI. It used to be OR'd with `cancel_at <= now()`, and since `cancel_at` defaulted to `now()` on every INSERT, that canceled every `solana_subs` subscription at its first rollover (#545) |
 | `canceled_at` / `ended_at` | TIMESTAMPTZ | |
 | `superseded_by` | INTEGER | Set on plan change — points at the replacement subscription |
 | `payment_provider` | VARCHAR | |