refactor(billing): planDefinitions array shape — single source of truth (#3560) (#3562)

* refactor(billing): planDefinitions array shape — single source of truth

Convert planDefinitions to array of self-contained { planId, ... } objects.
Derive billing.plans enum at boot in config/index.js with backward-compat shim
(deprecation warning). Add meterMode-gated boot validator on
Subscription.distinct('plan'). 5 new shim tests + 2 new validator tests +
5 ensureSeeded mocks updated. Migration doc added.

Closes #3560

* fix(billing): key-wins planId precedence in normalizePlanDefinitions; add regression test

- { ...def, planId } ensures the object key is always authoritative (was { planId, ...def })
- Guard null/undefined def with `?? {}` to prevent spread throwing
- Filter non-string/empty planIds when deriving config.billing.plans enum
- Update JSDoc @returns to accurately reflect passthrough behavior for non-object values
- Add regression test: object key beats nested def.planId on conflict

Author: PierreBrisorgueil

config/index.js

@@ -199,6 +199,43 @@ const initGlobalConfig = async () => {
   return conf;
 };
 
+/**
+ * Normalize billing.planDefinitions to the canonical array-of-objects shape.
+ *
+ * Accepts either:
+ *  - Array (new shape): returned as-is.
+ *  - Plain object (legacy shape): converted to array and a deprecation warning is emitted.
+ *    The object key is always authoritative as planId; any nested planId value is overridden.
+ *
+ * Safe when planDefinitions is missing/null — returns the input unchanged.
+ * Non-object, non-array values (e.g. strings) are returned unchanged.
+ *
+ * @param {Array|Object|null|undefined} planDefinitions
+ * @returns {Array|Object|null|undefined} Array form when convertible; original value otherwise.
+ */
+const normalizePlanDefinitions = (planDefinitions) => {
+  if (!planDefinitions) return planDefinitions;
+  if (Array.isArray(planDefinitions)) return planDefinitions;
+  if (typeof planDefinitions === 'object') {
+    console.warn(
+      '[billing] planDefinitions object shape is deprecated, switch to array — see docs/migrations/2026-05-01-billing-plan-definitions-array.md. Will be removed ~2026-07.',
+    );
+    return Object.entries(planDefinitions).map(([planId, def]) => ({ ...(def ?? {}), planId }));
+  }
+  return planDefinitions;
+};
+
 const config = await initGlobalConfig();
-export { deepMerge, assertSafeEnv };
+
+// Post-merge billing normalization: shim legacy planDefinitions + derive plans enum.
+if (config.billing?.planDefinitions != null) {
+  config.billing.planDefinitions = normalizePlanDefinitions(config.billing.planDefinitions);
+  if (Array.isArray(config.billing.planDefinitions)) {
+    config.billing.plans = config.billing.planDefinitions
+      .map((p) => p.planId)
+      .filter((id) => typeof id === 'string' && id.length > 0);
+  }
+}
+
+export { deepMerge, assertSafeEnv, normalizePlanDefinitions };
 export default config;

docs/migrations/2026-05-01-billing-plan-definitions-array.md

@@ -0,0 +1,68 @@
+# Migration: billing.planDefinitions array shape (2026-05-01)
+
+## Why
+
+During PR-N6 (#3555), the `enterprise` plan was present in `billing.plans` but missing from `billing.planDefinitions`. This silent divergence would have caused `ensureSeeded()` to skip seeding the enterprise plan, resulting in a `null` return from `getActivePlan('enterprise')` and a 503 for all metered enterprise users.
+
+The root cause: `plans` (enum array) and `planDefinitions` (definition map) were two separate declarations that had to be kept in sync manually. The fix is to make `planDefinitions` the single source of truth — `plans` is now derived at boot.
+
+## Before / After
+
+**Before (object-keyed — deprecated):**
+
+```js
+billing: {
+  plans: ['free', 'starter', 'pro', 'enterprise'],   // must match planDefinitions keys — drift risk
+  planDefinitions: {
+    free:       { meterQuota: 0,        ratios: { default: 1 } },
+    starter:    { meterQuota: 50000,    ratios: { default: 1 } },
+    pro:        { meterQuota: 500000,   ratios: { default: 1 } },
+    enterprise: { meterQuota: 2000000,  ratios: { default: 1 } },
+  },
+}
+```
+
+**After (array — canonical):**
+
+```js
+billing: {
+  // plans: derived at boot from planDefinitions — do NOT declare manually
+  planDefinitions: [
+    { planId: 'free',       meterQuota: 0,        ratios: { default: 1 } },
+    { planId: 'starter',    meterQuota: 50000,    ratios: { default: 1 } },
+    { planId: 'pro',        meterQuota: 500000,   ratios: { default: 1 } },
+    { planId: 'enterprise', meterQuota: 2000000,  ratios: { default: 1 } },
+  ],
+}
+```
+
+`config.billing.plans` is still available at runtime (derived by `config/index.js`) — `z.enum(config.billing.plans)` and all Zod/Mongoose schemas keep working unchanged.
+
+## Downstream migration steps
+
+For each downstream project that overrides `billing.planDefinitions` (e.g. `modules/billing/config/billing.{project}.config.js`):
+
+1. Pull the latest devkit: `npm run update-stack` (or merge the upstream devkit branch).
+2. Open `modules/billing/config/billing.{project}.config.js`.
+3. Convert `planDefinitions` from object to array form (see Before/After above). Remove the `plans: [...]` line — leaving it is harmless (the derivation in `config/index.js` runs after deepMerge and overwrites it), but remove it to avoid confusion.
+4. Run unit tests: `NODE_ENV={project} npm run test:unit -- billing`.
+5. Smoke test the plans endpoint: `curl https://api.{project}.example.com/api/billing/plans`.
+
+No database migration is required — the `BillingPlan` collection is unchanged.
+
+## Validation
+
+After migration, confirm:
+
+- Boot logs show **no** `[billing] planDefinitions object shape is deprecated` warning (shim did not trigger).
+- `NODE_ENV={project} npm run test:unit -- billing` passes.
+- `[billing] Subscription.plan value "..." not in planDefinitions` warning is **absent** in boot logs (no orphaned subscriptions).
+
+## Shim removal timeline
+
+The backward-compat shim in `config/index.js` converts legacy object-keyed `planDefinitions` to array form with a `console.warn`. It will be removed **~2026-07-01**, contingent on:
+
+- Zero deprecation warnings observed in any downstream prod log for ≥ 30 days.
+- All downstream projects confirmed migrated to array form.
+
+Track removal in a follow-up issue once all downstreams are migrated.

modules/billing/billing.init.js

@@ -1,6 +1,7 @@
 /**
  * Module dependencies
  */
+import mongoose from 'mongoose';
 import config from '../../config/index.js';
 import AnalyticsService from '../../lib/services/analytics.js';
 import billingEvents from './lib/events.js';
@@ -43,4 +44,22 @@ export default async (app) => {
     // Surfacing the crash here prevents a deploy from succeeding in a broken state.
     if (config?.billing?.meterMode) throw err;
   }
+
+  // Boot validator: warn on orphaned Subscription.plan values (meterMode only).
+  // Runs after ensureSeeded so the plan catalog is up to date.
+  // Never crashes boot — wrapped in try/catch.
+  if (config?.billing?.meterMode) {
+    try {
+      const Subscription = mongoose.model('Subscription');
+      const knownPlans = new Set(config.billing.plans ?? []);
+      const distinctPlans = await Subscription.distinct('plan');
+      for (const plan of distinctPlans) {
+        if (!knownPlans.has(plan)) {
+          console.warn(`[billing] Subscription.plan value "${plan}" not in planDefinitions — orphaned plan, may resolve quota=0`);
+        }
+      }
+    } catch (_err) {
+      // Validator failure must NOT crash boot (e.g. model not yet registered at early init)
+    }
+  }
 };

modules/billing/config/billing.development.config.js

@@ -6,8 +6,6 @@ const config = {
   },
   billing: {
     activated: true,
-    // Plans available for subscriptions — extend as needed
-    plans: ['free', 'starter', 'pro', 'enterprise'],
     // Quotas — downstream projects override these per plan:
     // quotas: {
     //   free:    { documents: { create: 10, export: 50 } },
@@ -49,15 +47,16 @@ const config = {
     /**
      * Plan definitions — DOWNSTREAM-OVERRIDE-REQUIRED for meter mode.
      * Used by BillingPlanService.ensureSeeded() at boot to upsert BillingPlan docs.
-     * Each entry: { meterQuota: units/week, ratios: { featureKey: multiplier } }.
-     * Plans listed here must also exist in billing.plans enum.
+     * Array of objects: { planId, meterQuota: units/week, ratios: { featureKey: multiplier } }.
+     * billing.plans enum is derived at boot from planDefinitions.map(p => p.planId) — do NOT
+     * declare billing.plans manually. This is the single source of truth for plan identifiers.
      */
-    planDefinitions: {
-      free: { meterQuota: 0, ratios: { default: 1 } },
-      starter: { meterQuota: 50000, ratios: { default: 1 } },
-      pro: { meterQuota: 500000, ratios: { default: 1 } },
-      enterprise: { meterQuota: 2000000, ratios: { default: 1 } },
-    },
+    planDefinitions: [
+      { planId: 'free', meterQuota: 0, ratios: { default: 1 } },
+      { planId: 'starter', meterQuota: 50000, ratios: { default: 1 } },
+      { planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
+      { planId: 'enterprise', meterQuota: 2000000, ratios: { default: 1 } },
+    ],
     /**
      * Meter unit parameters — downstream projects must override with their
      * actual unit economics before enabling meterMode in production.

modules/billing/services/billing.plan.service.js

@@ -165,9 +165,14 @@ const bumpVersionWithRetry = async (planId, fields, { maxAttempts = 3 } = {}) =>
 /**
  * @function ensureSeeded
  * @description Upsert BillingPlan docs from config.billing.planDefinitions.
- *              For each configured planId, ensures an active plan exists.
+ *              For each configured plan entry, ensures an active plan exists.
  *              No-op when meter mode is disabled. Idempotent on re-run.
  *
+ *              Accepts the canonical array-of-objects shape:
+ *              [{ planId, meterQuota, ratios }, ...].
+ *              The legacy object shape is normalized upstream in config/index.js
+ *              before this service is ever called.
+ *
  *              Race / E11000 safety: version is derived from the total count of
  *              existing docs for the planId (same strategy as bumpVersion). A
  *              try-catch on create absorbs E11000 from concurrent multi-pod
@@ -179,11 +184,12 @@ const bumpVersionWithRetry = async (planId, fields, { maxAttempts = 3 } = {}) =>
 const ensureSeeded = async () => {
   if (!config?.billing?.meterMode) return { seeded: 0, skipped: 0 };
 
-  const definitions = config?.billing?.planDefinitions ?? {};
+  const definitions = config?.billing?.planDefinitions ?? [];
   let seeded = 0;
   let skipped = 0;
 
-  for (const [planId, def] of Object.entries(definitions)) {
+  for (const def of definitions) {
+    const { planId, ...planDef } = def;
     const existing = await BillingPlanRepository.findActive(planId);
     if (existing) {
       skipped += 1;
@@ -199,8 +205,8 @@ const ensureSeeded = async () => {
       await BillingPlanRepository.create({
         planId,
         version,
-        meterQuota: def.meterQuota ?? 0,
-        ratios: def.ratios ?? { default: 1 },
+        meterQuota: planDef.meterQuota ?? 0,
+        ratios: planDef.ratios ?? { default: 1 },
         effectiveFrom: new Date(),
         effectiveUntil: null,
         active: true,

modules/billing/tests/billing.init.unit.tests.js

@@ -4,12 +4,14 @@
 import { jest, describe, test, beforeEach, afterEach, expect } from '@jest/globals';
 
 /**
- * Unit tests for billing.init ensureSeeded integration.
+ * Unit tests for billing.init — ensureSeeded integration and boot validator.
  */
-describe('billing.init ensureSeeded unit tests:', () => {
+describe('billing.init unit tests:', () => {
   let billingInit;
   let mockBillingPlanService;
   let mockConfig;
+  let mockDistinct;
+  let mockMongoose;
 
   const mockApp = {};
 
@@ -27,6 +29,11 @@ describe('billing.init ensureSeeded unit tests:', () => {
       ensureSeeded: jest.fn().mockResolvedValue({ seeded: 0, skipped: 0 }),
     };
 
+    mockDistinct = jest.fn().mockResolvedValue([]);
+    mockMongoose = {
+      model: jest.fn().mockReturnValue({ distinct: mockDistinct }),
+    };
+
     jest.unstable_mockModule('../../../config/index.js', () => ({
       default: mockConfig,
     }));
@@ -44,6 +51,10 @@ describe('billing.init ensureSeeded unit tests:', () => {
       default: { on: jest.fn(), emit: jest.fn() },
     }));
 
+    jest.unstable_mockModule('mongoose', () => ({
+      default: mockMongoose,
+    }));
+
     const mod = await import('../billing.init.js');
     billingInit = mod.default;
   });
@@ -81,4 +92,37 @@ describe('billing.init ensureSeeded unit tests:', () => {
       expect.stringContaining('seeded 2 plan(s)'),
     );
   });
+
+  test('boot validator warns on orphaned Subscription.plan values when meterMode=true', async () => {
+    mockConfig.billing.meterMode = true;
+    mockConfig.billing.plans = ['free', 'starter', 'pro'];
+
+    // Stub distinct to return a known plan + an orphaned plan
+    mockDistinct.mockResolvedValue(['free', 'legacy_plan']);
+
+    const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+
+    await billingInit(mockApp);
+
+    expect(mockDistinct).toHaveBeenCalledWith('plan');
+    expect(warnSpy).toHaveBeenCalledWith(
+      expect.stringContaining('"legacy_plan" not in planDefinitions'),
+    );
+    // Known plan 'free' must NOT trigger a warning
+    const warnings = warnSpy.mock.calls.map((c) => c[0]);
+    expect(warnings.some((w) => w.includes('"free"'))).toBe(false);
+  });
+
+  test('boot validator failure does not crash boot', async () => {
+    mockConfig.billing.meterMode = true;
+    mockConfig.billing.plans = ['free'];
+
+    // Subscription model throws (e.g. not yet registered at early init)
+    mockMongoose.model.mockImplementation(() => {
+      throw new Error('model not registered');
+    });
+
+    // Must resolve without throwing
+    await expect(billingInit(mockApp)).resolves.toBeUndefined();
+  });
 });

modules/billing/tests/billing.plan.ensureSeeded.unit.tests.js

@@ -17,7 +17,7 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
     mockConfig = {
       billing: {
         meterMode: true,
-        planDefinitions: {},
+        planDefinitions: [],
       },
     };
 
@@ -55,7 +55,7 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
     expect(mockBillingPlanRepository.create).not.toHaveBeenCalled();
   });
 
-  test('meterMode=true with empty planDefinitions returns zeroes', async () => {
+  test('meterMode=true with empty planDefinitions array returns zeroes', async () => {
     const result = await BillingPlanService.ensureSeeded();
 
     expect(result).toEqual({ seeded: 0, skipped: 0 });
@@ -64,11 +64,11 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
   });
 
   test('meterMode=true with 3 planDefinitions and none existing seeds 3 plans', async () => {
-    mockConfig.billing.planDefinitions = {
-      free: { meterQuota: 0, ratios: { default: 1 } },
-      starter: { meterQuota: 50000, ratios: { default: 1 } },
-      pro: { meterQuota: 500000, ratios: { default: 2 } },
-    };
+    mockConfig.billing.planDefinitions = [
+      { planId: 'free', meterQuota: 0, ratios: { default: 1 } },
+      { planId: 'starter', meterQuota: 50000, ratios: { default: 1 } },
+      { planId: 'pro', meterQuota: 500000, ratios: { default: 2 } },
+    ];
     mockBillingPlanRepository.findActive.mockResolvedValue(null);
     // count=0 → version 'v1' for each plan
     mockBillingPlanRepository.count.mockResolvedValue(0);
@@ -87,11 +87,11 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
   });
 
   test('meterMode=true with 3 planDefinitions and 1 existing seeds 2 and skips 1', async () => {
-    mockConfig.billing.planDefinitions = {
-      free: { meterQuota: 0, ratios: { default: 1 } },
-      starter: { meterQuota: 50000, ratios: { default: 1 } },
-      pro: { meterQuota: 500000, ratios: { default: 1 } },
-    };
+    mockConfig.billing.planDefinitions = [
+      { planId: 'free', meterQuota: 0, ratios: { default: 1 } },
+      { planId: 'starter', meterQuota: 50000, ratios: { default: 1 } },
+      { planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
+    ];
     mockBillingPlanRepository.findActive
       .mockResolvedValueOnce({ planId: 'free', version: 'v1' })
       .mockResolvedValueOnce(null)
@@ -114,9 +114,9 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
   });
 
   test('count > 0 yields next version string correctly', async () => {
-    mockConfig.billing.planDefinitions = {
-      pro: { meterQuota: 500000, ratios: { default: 1 } },
-    };
+    mockConfig.billing.planDefinitions = [
+      { planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
+    ];
     mockBillingPlanRepository.findActive.mockResolvedValue(null);
     // Existing inactive v1 is present (total count = 1)
     mockBillingPlanRepository.count.mockResolvedValue(1);
@@ -131,10 +131,10 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
   });
 
   test('E11000 on create is absorbed as a skip (concurrent pod race)', async () => {
-    mockConfig.billing.planDefinitions = {
-      free: { meterQuota: 0, ratios: { default: 1 } },
-      pro: { meterQuota: 500000, ratios: { default: 1 } },
-    };
+    mockConfig.billing.planDefinitions = [
+      { planId: 'free', meterQuota: 0, ratios: { default: 1 } },
+      { planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
+    ];
     mockBillingPlanRepository.findActive.mockResolvedValue(null);
     mockBillingPlanRepository.count.mockResolvedValue(0);
     // First create throws E11000; second succeeds
@@ -147,9 +147,9 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
   });
 
   test('non-E11000 error on create propagates and aborts', async () => {
-    mockConfig.billing.planDefinitions = {
-      pro: { meterQuota: 500000, ratios: { default: 1 } },
-    };
+    mockConfig.billing.planDefinitions = [
+      { planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
+    ];
     mockBillingPlanRepository.findActive.mockResolvedValue(null);
     mockBillingPlanRepository.count.mockResolvedValue(0);
     mockBillingPlanRepository.create.mockRejectedValue(new Error('DB connection lost'));