fix(billing): make school billing lifecycle reversible, bounded and consistently counted (#546) (#557)

Five independent defects in the school→platform billing lifecycle, none of
which had any test coverage.

1. Cancel was irreversible. PostgREST's ON CONFLICT DO UPDATE only writes the
   columns you name, so confirmManualPayment's upsert left a stale
   cancel_at_period_end = true: a school could cancel, change its mind, pay for
   a full year and still be dropped to free at the end of it, with no reminder
   and no grace (both cron phases filter on cancel_at_period_end = false).
   The confirm upsert and changePlan now clear the flag (and pass
   cancel_at_period_end: false to Stripe), and a new reactivateSubscription()
   action is wired into the billing page — there was no reactivate action in
   the UI at all.

2. An unpaid renewal request paused the expiry downgrade forever. Requests now
   carry expires_at (created + 14 days); a new cron phase sweeps lapsed ones to
   'expired' with an admin email, and the pause only honours open requests.
   Both duplicate guards moved off .single() — with two matching rows PostgREST
   returns PGRST116 / data: null, so the guards PASSED — and the renewal window
   guard is no longer bypassed by any non-active subscription status.

3. forceTenantPlanChange on a live Stripe subscriber billed the school for the
   comped plan: the next customer.subscription.updated read Stripe's real price
   as a downgrade, found the tenant over the real plan's limits (the reason it
   was comped) and repriced Stripe onto the comped plan. The override is now
   stamped on the subscription and applyPortalPlanChange ignores overridden
   tenants; clearTenantPlanOverride, a confirmed payment or an in-app plan
   change lift it.

4. The grace window collapsed to zero after a cron outage — graceEnd came from
   the subscription's period end, so one pass could send both "overdue" and
   "downgraded". It is now max(period_end, now) + GRACE_DAYS, and phase 3 skips
   tenants that entered grace in the same run.

5. Four definitions of "how many courses does this tenant have" collapsed to
   one: the billing page, the downgrade pre-flight and creation enforcement all
   route through countTenantUsage, and the hardcoded PLAN_LIMITS_FALLBACK map
   is gone in favour of platform_plans.

Tests: 411 → 447. New cron, action, TTL and count-parity suites; each new
assertion was verified to fail with the corresponding fix patched back out.


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

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: guillermoscript

app/[locale]/dashboard/admin/billing/billing-dashboard-client.tsx

@@ -19,7 +19,12 @@ import {
 } from '@/components/ui/alert-dialog'
 import { IconExternalLink, IconRefresh, IconCreditCard, IconPhoto, IconClock, IconLoader2 } from '@tabler/icons-react'
 import { useTranslations, useLocale } from 'next-intl'
-import { uploadPaymentProof, requestManualRenewal, cancelSubscription } from '@/app/actions/admin/billing'
+import {
+  uploadPaymentProof,
+  requestManualRenewal,
+  cancelSubscription,
+  reactivateSubscription,
+} from '@/app/actions/admin/billing'
 
 interface BillingDashboardClientProps {
   status: {
@@ -75,6 +80,7 @@ export function BillingDashboardClient({ status, paymentRequests }: BillingDashb
   const [uploadingFor, setUploadingFor] = useState<string | null>(null)
   const [cancelOpen, setCancelOpen] = useState(false)
   const [cancelLoading, setCancelLoading] = useState(false)
+  const [reactivateLoading, setReactivateLoading] = useState(false)
 
   const handleManageBilling = async () => {
     setPortalLoading(true)
@@ -134,6 +140,19 @@ export function BillingDashboardClient({ status, paymentRequests }: BillingDashb
     }
   }
 
+  const handleReactivate = async () => {
+    setReactivateLoading(true)
+    try {
+      await reactivateSubscription()
+      toast.success(t('reactivateSuccess'))
+      router.refresh()
+    } catch (e) {
+      toast.error(e instanceof Error ? e.message : t('reactivateError'))
+    } finally {
+      setReactivateLoading(false)
+    }
+  }
+
   const handleProofUpload = async (requestId: string, file: File) => {
     setUploadingFor(requestId)
     try {
@@ -181,6 +200,8 @@ export function BillingDashboardClient({ status, paymentRequests }: BillingDashb
         accessCutoffAt={status.accessCutoffAt}
         onManageClick={status.hasStripeCustomer ? handleManageBilling : undefined}
         onCancelClick={() => setCancelOpen(true)}
+        onReactivateClick={handleReactivate}
+        reactivateLoading={reactivateLoading}
       />
 
       {/* Manual Subscription Renewal Section */}

app/actions/admin/billing.ts

@@ -3,9 +3,14 @@
 import { createClient } from '@/lib/supabase/server'
 import { createAdminClient } from '@/lib/supabase/admin'
 import {getCurrentTenantId, getCurrentUserId } from '@/lib/supabase/tenant'
-import { checkPlanLimits, formatPlanLimitError } from '@/lib/billing/plan-limits'
+import { checkPlanLimits, countTenantUsage, formatPlanLimitError } from '@/lib/billing/plan-limits'
 import { classifyPlanChange } from '@/lib/billing/plan-change'
 import { reconcileAccessCutoff } from '@/lib/billing/access-cutoff'
+import {
+  OPEN_REQUEST_STATUSES,
+  isRequestOpen,
+  requestExpiresAt,
+} from '@/lib/billing/payment-request-ttl'
 import { revalidatePath } from 'next/cache'
 
 async function verifyAdminAccess() {
@@ -29,15 +34,43 @@ async function verifyAdminAccess() {
   return { userId, tenantId, supabase }
 }
 
+/**
+ * Is there an open (pending and not lapsed) platform payment request for this
+ * tenant? One helper for both duplicate guards so a renewal can never be
+ * created alongside a pending upgrade — the combination that used to disable
+ * both guards permanently, because each returned `PGRST116 / data: null` from
+ * `.single()` once two rows matched.
+ */
+async function hasOpenPaymentRequest(
+  adminClient: Awaited<ReturnType<typeof createAdminClient>>,
+  tenantId: string,
+): Promise<boolean> {
+  const { data } = await adminClient
+    .from('platform_payment_requests')
+    .select('request_id, status, expires_at')
+    .eq('tenant_id', tenantId)
+    .in('status', OPEN_REQUEST_STATUSES as unknown as string[])
+    .order('created_at', { ascending: false })
+    .limit(20)
+
+  return ((data as { status: string; expires_at: string | null }[] | null) || []).some((r) =>
+    isRequestOpen(r)
+  )
+}
+
 /**
  * Get current subscription status and usage statistics
  */
 export async function getSubscriptionStatus() {
   const { tenantId } = await verifyAdminAccess()
   const adminClient = await createAdminClient()
 
-  // Fetch tenant, subscription, plan, and usage in parallel
-  const [tenantResult, subscriptionResult, coursesCount, studentsCount] = await Promise.all([
+  // Fetch tenant, subscription, plan, and usage in parallel.
+  // Usage goes through countTenantUsage (#546 §5) so the number an admin reads
+  // here is the same number the downgrade pre-flight and course-creation
+  // enforcement compare against — it used to count archived courses that
+  // neither of those did.
+  const [tenantResult, subscriptionResult, usage] = await Promise.all([
     adminClient
       .from('tenants')
       .select('plan, billing_status, billing_period_end, billing_email, stripe_customer_id, access_cutoff_at')
@@ -48,16 +81,7 @@ export async function getSubscriptionStatus() {
       .select('*, platform_plans(*)')
       .eq('tenant_id', tenantId)
       .single(),
-    adminClient
-      .from('courses')
-      .select('*', { count: 'exact', head: true })
-      .eq('tenant_id', tenantId),
-    adminClient
-      .from('tenant_users')
-      .select('*', { count: 'exact', head: true })
-      .eq('tenant_id', tenantId)
-      .eq('role', 'student')
-      .eq('status', 'active'),
+    countTenantUsage(adminClient, tenantId),
   ])
 
   const tenant = tenantResult.data
@@ -102,11 +126,11 @@ export async function getSubscriptionStatus() {
     } : null,
     usage: {
       courses: {
-        current: coursesCount.count || 0,
+        current: usage.courses,
         limit: limits.max_courses,
       },
       students: {
-        current: studentsCount.count || 0,
+        current: usage.students,
         limit: limits.max_students,
       },
     },
@@ -182,15 +206,12 @@ export async function requestManualPlanUpgrade(planId: string, interval: 'monthl
     throw new Error(formatPlanLimitError(limitCheck) || 'Plan limits exceeded')
   }
 
-  // Check for existing pending request
-  const { data: existingRequest } = await adminClient
-    .from('platform_payment_requests')
-    .select('request_id')
-    .eq('tenant_id', tenantId)
-    .in('status', ['pending', 'instructions_sent', 'payment_received'])
-    .single()
-
-  if (existingRequest) {
+  // Check for an existing open request. Bounded list + array check, never
+  // `.single()` (#546 §2): with two or more matching rows `.single()` returns
+  // PGRST116 and `data: null`, so the guard PASSED and the table grew unbounded
+  // per tenant. Lapsed requests are ignored so a stale row cannot lock a school
+  // out of paying.
+  if (await hasOpenPaymentRequest(adminClient, tenantId)) {
     throw new Error('You already have a pending plan change request. Please wait for it to be processed.')
   }
 
@@ -207,6 +228,7 @@ export async function requestManualPlanUpgrade(planId: string, interval: 'monthl
       request_type: requestType,
       bank_reference: bankReference || null,
       notes: notes || null,
+      expires_at: requestExpiresAt(),
     })
     .select('request_id')
     .single()
@@ -334,6 +356,18 @@ export async function confirmManualPayment(requestId: string) {
       grace_period_end: null,
       // Reset the reminder stamp so the next cycle can remind again.
       renewal_reminder_sent_at: null,
+      // A confirmed payment is an un-cancel (#546 §1). PostgREST's ON CONFLICT
+      // DO UPDATE only touches the columns supplied here, so omitting these two
+      // left a stale `cancel_at_period_end = true` on the row: the school paid
+      // for a full period and was then silently dropped to free at the end of
+      // it by the cron's cancel phase, with no reminder and no grace window
+      // (phases 1 and 2 both filter on `cancel_at_period_end = false`).
+      cancel_at_period_end: false,
+      canceled_at: null,
+      // A real payment supersedes any super-admin comp (#546 §3) — this is one
+      // of the override's exits, so portal changes start syncing again.
+      plan_override_by: null,
+      plan_override_at: null,
       updated_at: now.toISOString(),
     }, { onConflict: 'tenant_id' })
 
@@ -503,6 +537,11 @@ export async function changePlan(planId: string, interval: 'monthly' | 'yearly'
   await ctx.stripe.subscriptions.update(ctx.subId, {
     items: [{ id: ctx.itemId, price: ctx.targetPriceId }],
     proration_behavior: 'create_prorations',
+    // Paying for a different plan un-cancels (#546 §1). `resolvePlatformPlanChange`
+    // accepts a still-`active` subscription, which a cancel-at-period-end sub is
+    // right up to its last day — without this the school changes plan, is billed,
+    // and is still dropped to free at period end.
+    cancel_at_period_end: false,
     metadata: {
       tenant_id: tenantId,
       plan_id: ctx.plan.plan_id,
@@ -517,7 +556,18 @@ export async function changePlan(planId: string, interval: 'monthly' | 'yearly'
   const now = new Date().toISOString()
   await adminClient
     .from('platform_subscriptions')
-    .update({ plan_id: ctx.plan.plan_id, interval, updated_at: now })
+    .update({
+      plan_id: ctx.plan.plan_id,
+      interval,
+      cancel_at_period_end: false,
+      canceled_at: null,
+      // An admin-initiated, Stripe-backed change supersedes a super-admin comp
+      // (#546 §3): Stripe and the DB now agree again, so the reconciler should
+      // resume applying portal changes for this tenant.
+      plan_override_by: null,
+      plan_override_at: null,
+      updated_at: now,
+    })
     .eq('tenant_id', tenantId)
   await adminClient
     .from('tenants')
@@ -593,6 +643,65 @@ export async function cancelSubscription() {
   return { success: true }
 }
 
+/**
+ * Undo a pending cancellation (#546 §1).
+ *
+ * Cancellation was one-way: the only writers of `cancel_at_period_end` were
+ * `cancelSubscription` (sets it) and the Stripe webhook (mirrors Stripe), the
+ * billing UI imported no reactivate action at all, and re-checkout is blocked
+ * while the subscription is still `active` — which a cancel-at-period-end
+ * subscription is until its last day. A school that changed its mind had no way
+ * back short of contacting support.
+ */
+export async function reactivateSubscription() {
+  const { tenantId } = await verifyAdminAccess()
+  const adminClient = await createAdminClient()
+
+  const { data: subscription } = await adminClient
+    .from('platform_subscriptions')
+    .select('stripe_subscription_id, payment_method, status, cancel_at_period_end, current_period_end')
+    .eq('tenant_id', tenantId)
+    .single()
+
+  if (!subscription) throw new Error('No subscription to reactivate')
+  if (!subscription.cancel_at_period_end) {
+    throw new Error('This subscription is not scheduled for cancellation')
+  }
+  // Once the period has lapsed the cron has already downgraded (or is about
+  // to); reactivating would revive a plan nobody is paying for. Those schools
+  // go through checkout / a renewal request instead.
+  if (subscription.status !== 'active') {
+    throw new Error('This subscription has already ended. Please start a new plan instead.')
+  }
+  if (
+    subscription.current_period_end &&
+    new Date(subscription.current_period_end) <= new Date()
+  ) {
+    throw new Error('Your billing period has already ended. Please start a new plan instead.')
+  }
+
+  if (subscription.payment_method === 'stripe' && subscription.stripe_subscription_id) {
+    // Stripe is authoritative for Stripe subs — clear it there first so a
+    // failure leaves both sides still cancelling rather than disagreeing.
+    const { getStripe } = await import('@/lib/stripe')
+    await getStripe().subscriptions.update(subscription.stripe_subscription_id, {
+      cancel_at_period_end: false,
+    })
+  }
+
+  await adminClient
+    .from('platform_subscriptions')
+    .update({
+      cancel_at_period_end: false,
+      canceled_at: null,
+      updated_at: new Date().toISOString(),
+    })
+    .eq('tenant_id', tenantId)
+
+  revalidatePath('/dashboard/admin/billing')
+  return { success: true }
+}
+
 /**
  * Upload payment proof for a platform payment request (school admin)
  */
@@ -681,21 +790,22 @@ export async function requestManualRenewal() {
   const now = new Date()
   const daysUntilEnd = Math.ceil((periodEnd.getTime() - now.getTime()) / (1000 * 60 * 60 * 24))
 
-  if (daysUntilEnd > 30 && subscription.status === 'active') {
+  // The window guard used to be bypassed entirely for any non-`active` status
+  // (#546 §2), so a canceled or long-lapsed subscription could mint the row
+  // that pauses the downgrade at any time. A school in grace (`past_due`) must
+  // still be able to pay — that is the whole point of the pause — so allow it
+  // explicitly rather than by falling through the guard.
+  const lapsed = daysUntilEnd <= 0 || subscription.status === 'past_due'
+  if (daysUntilEnd > 30 && !lapsed) {
     throw new Error('Renewal can only be requested within 30 days of period end')
   }
 
-  // Check for existing pending renewal request
-  const { data: existingRequest } = await adminClient
-    .from('platform_payment_requests')
-    .select('request_id')
-    .eq('tenant_id', tenantId)
-    .eq('request_type', 'renewal')
-    .in('status', ['pending', 'instructions_sent', 'payment_received'])
-    .single()
-
-  if (existingRequest) {
-    throw new Error('You already have a pending renewal request')
+  // One open request per tenant, whatever its type: the old guard filtered on
+  // `request_type = 'renewal'` while the upgrade guard did not, so creating a
+  // renewal alongside a pending upgrade was the ordinary way to get two rows —
+  // after which `.single()` returned PGRST116 and BOTH guards passed forever.
+  if (await hasOpenPaymentRequest(adminClient, tenantId)) {
+    throw new Error('You already have a pending payment request. Please wait for it to be processed.')
   }
 
   const plan = subscription.platform_plans as { plan_id: string; price_monthly: number; price_yearly: number }
@@ -712,6 +822,7 @@ export async function requestManualRenewal() {
       currency: 'usd',
       status: 'pending',
       request_type: 'renewal',
+      expires_at: requestExpiresAt(),
     })
     .select('request_id')
     .single()