From 8afb9c6cc074cc1762eca15d69867f8dd8a2e864 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 1 May 2026 11:20:55 +0200 Subject: [PATCH 1/2] =?UTF-8?q?refactor(billing):=20planDefinitions=20arra?= =?UTF-8?q?y=20shape=20=E2=80=94=20single=20source=20of=20truth?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- config/index.js | 35 +++++++- ...26-05-01-billing-plan-definitions-array.md | 68 +++++++++++++++ modules/billing/billing.init.js | 19 +++++ .../config/billing.development.config.js | 19 ++--- .../billing/services/billing.plan.service.js | 16 ++-- .../billing/tests/billing.init.unit.tests.js | 48 ++++++++++- .../billing.plan.ensureSeeded.unit.tests.js | 44 +++++----- .../normalizePlanDefinitions.unit.tests.js | 84 +++++++++++++++++++ 8 files changed, 293 insertions(+), 40 deletions(-) create mode 100644 docs/migrations/2026-05-01-billing-plan-definitions-array.md create mode 100644 scripts/tests/normalizePlanDefinitions.unit.tests.js diff --git a/config/index.js b/config/index.js index 1bdef8c49..4bcbb5077 100644 --- a/config/index.js +++ b/config/index.js @@ -199,6 +199,39 @@ 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. + * + * Safe when planDefinitions is missing/null — returns the input unchanged. + * + * @param {Array|Object|null|undefined} planDefinitions + * @returns {Array|null|undefined} Array form, or the original value if not an object/array. + */ +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]) => ({ planId, ...def })); + } + 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); + } +} + +export { deepMerge, assertSafeEnv, normalizePlanDefinitions }; export default config; diff --git a/docs/migrations/2026-05-01-billing-plan-definitions-array.md b/docs/migrations/2026-05-01-billing-plan-definitions-array.md new file mode 100644 index 000000000..ff008691b --- /dev/null +++ b/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. diff --git a/modules/billing/billing.init.js b/modules/billing/billing.init.js index 91873de07..31d5f9198 100644 --- a/modules/billing/billing.init.js +++ b/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) + } + } }; diff --git a/modules/billing/config/billing.development.config.js b/modules/billing/config/billing.development.config.js index 0ddba30a6..8311ed1f9 100644 --- a/modules/billing/config/billing.development.config.js +++ b/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. diff --git a/modules/billing/services/billing.plan.service.js b/modules/billing/services/billing.plan.service.js index b89518d7d..420640a80 100644 --- a/modules/billing/services/billing.plan.service.js +++ b/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, diff --git a/modules/billing/tests/billing.init.unit.tests.js b/modules/billing/tests/billing.init.unit.tests.js index e02f496c4..e92ded513 100644 --- a/modules/billing/tests/billing.init.unit.tests.js +++ b/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(); + }); }); diff --git a/modules/billing/tests/billing.plan.ensureSeeded.unit.tests.js b/modules/billing/tests/billing.plan.ensureSeeded.unit.tests.js index 5645b8d2a..24515b8e9 100644 --- a/modules/billing/tests/billing.plan.ensureSeeded.unit.tests.js +++ b/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')); diff --git a/scripts/tests/normalizePlanDefinitions.unit.tests.js b/scripts/tests/normalizePlanDefinitions.unit.tests.js new file mode 100644 index 000000000..a33d54d74 --- /dev/null +++ b/scripts/tests/normalizePlanDefinitions.unit.tests.js @@ -0,0 +1,84 @@ +/** + * Module dependencies. + */ +import { describe, test, expect, beforeEach, afterEach, jest } from '@jest/globals'; + +/** + * Unit tests for normalizePlanDefinitions (billing planDefinitions shim). + */ +describe('normalizePlanDefinitions unit tests:', () => { + let normalizePlanDefinitions; + let warnSpy; + + beforeEach(async () => { + jest.resetModules(); + warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + // Import the named export directly from config/index.js. + // config/index.js is a top-level ESM module with a top-level await — jest.resetModules() + // causes a fresh evaluation each time, which is fine for unit-testing the named export. + const mod = await import('../../config/index.js'); + normalizePlanDefinitions = mod.normalizePlanDefinitions; + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('returns null unchanged', () => { + expect(normalizePlanDefinitions(null)).toBeNull(); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('returns undefined unchanged', () => { + expect(normalizePlanDefinitions(undefined)).toBeUndefined(); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('returns array input as-is (no conversion, no warning)', () => { + const input = [ + { planId: 'free', meterQuota: 0, ratios: { default: 1 } }, + { planId: 'pro', meterQuota: 500000, ratios: { default: 1 } }, + ]; + const result = normalizePlanDefinitions(input); + expect(result).toBe(input); // same reference — no copy + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('converts legacy object shape to array form and emits deprecation warning', () => { + const legacy = { + free: { meterQuota: 0, ratios: { default: 1 } }, + starter: { meterQuota: 50000, ratios: { default: 1 } }, + pro: { meterQuota: 500000, ratios: { default: 2 } }, + }; + + const result = normalizePlanDefinitions(legacy); + + expect(Array.isArray(result)).toBe(true); + expect(result).toHaveLength(3); + expect(result[0]).toEqual({ planId: 'free', meterQuota: 0, ratios: { default: 1 } }); + expect(result[1]).toEqual({ planId: 'starter', meterQuota: 50000, ratios: { default: 1 } }); + expect(result[2]).toEqual({ planId: 'pro', meterQuota: 500000, ratios: { default: 2 } }); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('[billing] planDefinitions object shape is deprecated'), + ); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('2026-05-01-billing-plan-definitions-array.md'), + ); + }); + + test('planId is correctly extracted as its own field (not nested under planId key)', () => { + const legacy = { + enterprise: { meterQuota: 2000000, ratios: { default: 1 } }, + }; + + const [entry] = normalizePlanDefinitions(legacy); + + expect(entry.planId).toBe('enterprise'); + expect(entry.meterQuota).toBe(2000000); + // planId should not be doubled/nested + expect(entry).not.toHaveProperty('enterprise'); + }); +}); From 109da6ad6eba761048195713b0958cda719cdfb8 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 1 May 2026 11:33:09 +0200 Subject: [PATCH 2/2] 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 --- config/index.js | 10 ++++++--- .../normalizePlanDefinitions.unit.tests.js | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/config/index.js b/config/index.js index 4bcbb5077..7e3797cc8 100644 --- a/config/index.js +++ b/config/index.js @@ -205,11 +205,13 @@ const initGlobalConfig = async () => { * 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|null|undefined} Array form, or the original value if not an object/array. + * @returns {Array|Object|null|undefined} Array form when convertible; original value otherwise. */ const normalizePlanDefinitions = (planDefinitions) => { if (!planDefinitions) return planDefinitions; @@ -218,7 +220,7 @@ const normalizePlanDefinitions = (planDefinitions) => { 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]) => ({ planId, ...def })); + return Object.entries(planDefinitions).map(([planId, def]) => ({ ...(def ?? {}), planId })); } return planDefinitions; }; @@ -229,7 +231,9 @@ const config = await initGlobalConfig(); 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); + config.billing.plans = config.billing.planDefinitions + .map((p) => p.planId) + .filter((id) => typeof id === 'string' && id.length > 0); } } diff --git a/scripts/tests/normalizePlanDefinitions.unit.tests.js b/scripts/tests/normalizePlanDefinitions.unit.tests.js index a33d54d74..5b1b0d1a1 100644 --- a/scripts/tests/normalizePlanDefinitions.unit.tests.js +++ b/scripts/tests/normalizePlanDefinitions.unit.tests.js @@ -81,4 +81,26 @@ describe('normalizePlanDefinitions unit tests:', () => { // planId should not be doubled/nested expect(entry).not.toHaveProperty('enterprise'); }); + + test('object key remains authoritative when nested def.planId conflicts with it', () => { + const legacy = { + starter: { planId: 'wrong', meterQuota: 50000, ratios: { default: 1 } }, + }; + + const result = normalizePlanDefinitions(legacy); + + expect(Array.isArray(result)).toBe(true); + expect(result).toHaveLength(1); + const [entry] = result; + // The object key 'starter' must win over the nested def.planId 'wrong' + expect(entry.planId).toBe('starter'); + expect(entry.meterQuota).toBe(50000); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('[billing] planDefinitions object shape is deprecated'), + ); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('2026-05-01-billing-plan-definitions-array.md'), + ); + }); });