fix(subscription): make the trial-expiry auto-downgrade actually fire (#228)

getUserPlan()'s auto-downgrade was guarded on `subscription_status !== 'active'`,
but that column DEFAULTs to 'active' and is only ever changed by Stripe webhook
events. For trial users who never touch Stripe — the entire population it's meant
to catch — the condition was always false, so the downgrade never ran and every
signup kept Pro free forever.

Re-key the guard on the real signals:
  - trial expired (!trial_active), AND
  - stripe_subscription_id IS NULL (never paid), AND
  - plan_id === trial_plan (still on the plan the trial granted), AND
  - plan_name !== 'free'

The plan_id === trial_plan clause is load-bearing: it protects comped / hand-
granted plans (e.g. a manually-set enterprise plan, where plan_id !== trial_plan)
from being silently downgraded. Grandfathered accounts (trial_started IS NULL)
never enter the block at all, so the ~home cohort is untouched. Added a comment
documenting the subscription_status-default trap so it isn't reintroduced.

Enforcement stays forward-only/lazy — the downgrade happens in the resolver on a
user's next request; no mass update here.

Downstream (deviceSocket.checkDeviceAccess, traced, unchanged): a genuinely-
expired free-tier trial now resolves to free and its device-limit block correctly
caps it to 1 device; grandfathered home (2 devices) and paid users are not
blocked. NOTE: the separate "Trial Expired" screen branch there is a pre-existing
dead condition (it needs trial_started set AND plan_name='free' at once, but the
downgrade clears trial_started) — left as-is per scope; flagged for follow-up.

Tests (new trial-expiry.test.js — there was none, which is how this shipped):
lapsed trial downgrades; comped enterprise (plan_id!=trial_plan) not downgraded;
grandfathered home (trial_started NULL) not downgraded; paid user not downgraded;
in-window trial not downgraded; plus a regression pinning that subscription_status
='active' no longer shields a lapsed trial. Suite 563/563.

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

server/middleware/subscription.js

@@ -28,8 +28,21 @@ function getUserPlan(userId) {
     user.trial_days_left = Math.max(0, Math.ceil((trialEnd - now) / 86400));
     user.trial_end = trialEnd;
 
-    // Auto-downgrade if trial expired and no paid subscription
-    if (!user.trial_active && user.subscription_status !== 'active' && user.plan_name !== 'free') {
+    // Auto-downgrade an EXPIRED trial to free. Keyed on "no real paid subscription"
+    // (stripe_subscription_id IS NULL) plus "still on the plan the trial granted"
+    // (plan_id === trial_plan) — deliberately NOT on subscription_status.
+    //
+    // TRAP — do not reintroduce a subscription_status guard here: that column DEFAULTs to
+    // 'active' and is only ever changed by Stripe webhook events. A `subscription_status !==
+    // 'active'` check is therefore ALWAYS false for trial users who never touch Stripe — the
+    // entire population this is meant to catch — so the downgrade never fired and every signup
+    // kept Pro free forever.
+    //
+    // The `plan_id === user.trial_plan` clause is load-bearing: it protects comped / hand-
+    // granted plans (e.g. an enterprise plan set manually, where plan_id !== trial_plan) from
+    // being silently downgraded. Grandfathered users (trial_started IS NULL) never reach this
+    // block at all.
+    if (!user.trial_active && !user.stripe_subscription_id && user.plan_id === user.trial_plan && user.plan_name !== 'free') {
       db.prepare("UPDATE users SET plan_id = 'free', trial_started = NULL WHERE id = ?").run(userId);
       // Re-fetch with free plan
       return getUserPlan(userId);

server/test/trial-expiry.test.js

@@ -0,0 +1,84 @@
+'use strict';
+
+// Trial-expiry guard in getUserPlan() (middleware/subscription.js). There was NO test on this
+// path before — which is exactly how the bug shipped: the auto-downgrade was guarded on
+// `subscription_status !== 'active'`, but that column DEFAULTs to 'active' and only Stripe
+// webhooks change it, so for trial users (who never touch Stripe) it was always false and the
+// downgrade never fired → Pro-for-free forever.
+//
+// In-process (billing-unit.test.js convention): seed a user, call getUserPlan directly, assert
+// both the returned plan and the DB side-effect (plan_id / trial_started).
+
+const os = require('node:os');
+const path = require('node:path');
+const crypto = require('node:crypto');
+process.env.DATA_DIR = path.join(os.tmpdir(), 'st-trial-' + crypto.randomBytes(4).toString('hex'));
+process.env.SELF_HOSTED = 'true';
+process.env.NODE_ENV = 'test';
+
+const { test, before } = require('node:test');
+const assert = require('node:assert/strict');
+const { db } = require('../db/database');
+const { getUserPlan } = require('../middleware/subscription');
+
+const DAY = 86400;
+const now = () => Math.floor(Date.now() / 1000);
+const uid = (p) => p + '-' + crypto.randomBytes(4).toString('hex');
+
+function mkUser({ plan_id = 'pro', trial_plan = 'pro', trial_started = null, stripe_sub = null, subscription_status = 'active' }) {
+  const id = uid('u');
+  db.prepare(`INSERT INTO users (id, email, plan_id, trial_plan, trial_started, stripe_subscription_id, subscription_status)
+              VALUES (?, ?, ?, ?, ?, ?, ?)`)
+    .run(id, id + '@t.local', plan_id, trial_plan, trial_started, stripe_sub, subscription_status);
+  return id;
+}
+const rowOf = (id) => db.prepare('SELECT plan_id, trial_started FROM users WHERE id = ?').get(id);
+
+before(() => {
+  // 'home' is a prod-only plan (not in the schema seed); add it so the grandfathered case joins.
+  db.prepare("INSERT OR IGNORE INTO plans (id, name, display_name, max_devices) VALUES ('home', 'home', 'Home', 2)").run();
+});
+
+test('lapsed pro trial, no sub, plan_id=trial_plan -> downgraded to free', () => {
+  const id = mkUser({ plan_id: 'pro', trial_plan: 'pro', trial_started: now() - 15 * DAY });
+  const plan = getUserPlan(id);
+  assert.equal(plan.plan_name, 'free', 'resolver returns the free plan');
+  assert.equal(rowOf(id).plan_id, 'free', 'persisted as free');
+  assert.equal(rowOf(id).trial_started, null, 'trial_started cleared');
+});
+
+test('comped enterprise (plan_id != trial_plan), no sub -> NOT downgraded', () => {
+  // enterprise granted by hand: plan_id='enterprise' but the trial had granted 'pro'.
+  // The plan_id===trial_plan clause must protect it.
+  const id = mkUser({ plan_id: 'enterprise', trial_plan: 'pro', trial_started: now() - 15 * DAY });
+  getUserPlan(id);
+  assert.equal(rowOf(id).plan_id, 'enterprise', 'comped plan not silently downgraded');
+});
+
+test('grandfathered home user (trial_started NULL) -> NOT downgraded', () => {
+  const id = mkUser({ plan_id: 'home', trial_plan: 'pro', trial_started: null });
+  const plan = getUserPlan(id);
+  assert.equal(plan.plan_id, 'home');
+  assert.equal(rowOf(id).plan_id, 'home', 'grandfathered account untouched');
+});
+
+test('paid user (stripe_subscription_id present) -> NOT downgraded', () => {
+  const id = mkUser({ plan_id: 'pro', trial_plan: 'pro', trial_started: now() - 15 * DAY, stripe_sub: 'sub_123' });
+  getUserPlan(id);
+  assert.equal(rowOf(id).plan_id, 'pro', 'paying customer untouched');
+});
+
+test('trial still within window -> NOT downgraded', () => {
+  const id = mkUser({ plan_id: 'pro', trial_plan: 'pro', trial_started: now() - 3 * DAY });
+  const plan = getUserPlan(id);
+  assert.equal(plan.trial_active, true);
+  assert.equal(rowOf(id).plan_id, 'pro');
+});
+
+// Regression pinning the exact bug: subscription_status='active' (the column default) must NOT
+// shield a lapsed free-tier trial from downgrade. This is the assertion the old guard failed.
+test('subscription_status=active default does NOT block downgrade of a lapsed trial', () => {
+  const id = mkUser({ plan_id: 'pro', trial_plan: 'pro', trial_started: now() - 20 * DAY, subscription_status: 'active' });
+  getUserPlan(id);
+  assert.equal(rowOf(id).plan_id, 'free', 'the default active status no longer protects a lapsed trial');
+});