fix(billing): V8.1 post-merge hardening (#3629)

* fix(billing): v8.1 post-merge hardening

F1: wrap syncOrganizationPlan in try/catch in handleInvoicePaymentSucceeded —
    DB blip no longer dead-letters handler; emits billing.organization.sync_failed
F2: prepend '~' to synthetic event IDs in bumpEventMarkers — same-second lex
    tiebreaker now sorts synthetic bumps after Stripe evt_* IDs
F3: validatePlan warns on unrecognized non-empty planId before returning null
F4: add 'unpaid' to RECONCILE_STATUSES so reconciler covers unpaid subs
F5: add V8-C2b — canceled subscription in meter mode must be fail-closed

* fix(billing): include error stacks + cover syncOrganizationPlan failure path

Add stack: err?.stack to both logger.error calls in the syncOrganizationPlan
catch block to match the convention used throughout billing.webhook.service.js.
Add unit test covering the dunning recovery setPlan failure path: asserts
non-fatal behaviour, logger.error call, and billing.organization.sync_failed emit.

* test(billing): cover evtErr inner catch + validatePlan warn branch for codecov/patch

Add two more unit tests to close remaining uncovered lines in the V8.1 patch:
- sync_failed listener throws: asserts inner evtErr catch is non-fatal and logs correctly
- validatePlan warns on unrecognized non-empty planId (e.g. Stripe product ID)

Author: PierreBrisorgueil

modules/billing/lib/billing.markerBump.js

@@ -13,6 +13,6 @@ export const bumpEventMarkers = (family, source) => {
   const fieldPrefix = family === 'invoice' ? 'lastInvoiceEvent' : 'lastSubscriptionEvent';
   return {
     [`${fieldPrefix}CreatedAt`]: Math.floor(ms / 1000),
-    [`${fieldPrefix}Id`]: `${source}-${ms}`,
+    [`${fieldPrefix}Id`]: `~${source}-${ms}`,
   };
 };

modules/billing/services/billing.reconcile.service.js

@@ -15,7 +15,7 @@ const RECONCILE_PAGE_SIZE = 100;
 /**
  * Statuses reconciled against Stripe.
  */
-const RECONCILE_STATUSES = ['active', 'past_due', 'trialing'];
+const RECONCILE_STATUSES = ['active', 'past_due', 'trialing', 'unpaid'];
 
 /**
  * Valid plan names from config.

modules/billing/services/billing.webhook.service.js

@@ -42,7 +42,11 @@ const validPlans = new Set(config.billing?.plans || ['free', 'starter', 'pro', '
  * @param {string} plan - The plan name to validate.
  * @returns {string|null} The plan name if valid, null otherwise.
  */
-const validatePlan = (plan) => (validPlans.has(plan) ? plan : null);
+const validatePlan = (plan) => {
+  if (validPlans.has(plan)) return plan;
+  if (plan) logger.warn('[billing.webhook] validatePlan: unrecognized planId', { raw: plan, validPlans: [...validPlans] });
+  return null;
+};
 
 /**
  * Plan rank lookup — higher index means higher-tier plan.
@@ -601,7 +605,23 @@ const handleInvoicePaymentSucceeded = async (invoice, event) => {
   }
 
   if (isPastDue && resolvedPlan) {
-    await syncOrganizationPlan(organizationId, resolvedPlan);
+    try {
+      await syncOrganizationPlan(organizationId, resolvedPlan);
+    } catch (syncErr) {
+      logger.error('[billing.webhook] syncOrganizationPlan failed (non-fatal)', {
+        organizationId,
+        error: syncErr?.message ?? String(syncErr),
+        stack: syncErr?.stack,
+      });
+      try {
+        billingEvents.emit('billing.organization.sync_failed', { organizationId, source: 'dunning_recovery' });
+      } catch (evtErr) {
+        logger.error('[billing.webhook] billing.organization.sync_failed listener error (non-fatal)', {
+          error: evtErr?.message ?? String(evtErr),
+          stack: evtErr?.stack,
+        });
+      }
+    }
   }
 };
 

modules/billing/tests/billing.admin.service.unit.tests.js

@@ -449,7 +449,7 @@ describe('BillingAdminService unit tests:', () => {
 
       expect(lastSubscriptionEventCreatedAt).toBeGreaterThanOrEqual(before);
       expect(lastSubscriptionEventCreatedAt).toBeLessThanOrEqual(after);
-      expect(lastSubscriptionEventId).toMatch(/^admin-cancel-\d+$/);
+      expect(lastSubscriptionEventId).toMatch(/^~admin-cancel-\d+$/);
     });
   });
 

modules/billing/tests/billing.quota.unit.tests.js

@@ -621,5 +621,29 @@ describe('requireQuota middleware:', () => {
       // Fail-closed branch must short-circuit before the meter check
       expect(mockBillingUsageService.getMeter).not.toHaveBeenCalled();
     });
+
+    test('V8-C2b: canceled subscription in meter mode → 402 METER_EXHAUSTED with free quota (not paid)', async () => {
+      // Org has status='canceled' (subscription ended). Paid plan must NOT bleed through.
+      // Fail-closed branch routes to free plan, same as incomplete.
+      mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'pro', status: 'canceled' });
+      mockBillingUsageService.getMeter.mockResolvedValue(null);
+      mockBillingExtraBalanceRepository.getBalance.mockResolvedValue(0);
+      mockBillingPlanService.getActivePlan.mockReturnValue({ meterQuota: 0, version: 'v1' });
+
+      await requireQuota('scraps', 'create')(req, res, next);
+
+      expect(next).not.toHaveBeenCalled();
+      expect(res.status).toHaveBeenCalledWith(402);
+      const payload = res.json.mock.calls[0][0];
+      const errData = JSON.parse(payload.error);
+      expect(errData.type).toBe('METER_EXHAUSTED');
+      // Free plan quota (0), not the pro paid quota
+      expect(errData.meterQuota).toBe(0);
+      // getActivePlan called with the free/default plan id, not 'pro'
+      expect(mockBillingPlanService.getActivePlan).toHaveBeenCalledWith('free');
+      expect(mockBillingPlanService.getActivePlan).not.toHaveBeenCalledWith('pro');
+      // Fail-closed branch must short-circuit before the meter check
+      expect(mockBillingUsageService.getMeter).not.toHaveBeenCalled();
+    });
   });
 });

modules/billing/tests/billing.subscription.repository.unit.tests.js

@@ -409,7 +409,7 @@ describe('BillingSubscriptionRepository unit tests:', () => {
 
       expect(lastSubscriptionEventCreatedAt).toBeGreaterThanOrEqual(before);
       expect(lastSubscriptionEventCreatedAt).toBeLessThanOrEqual(after);
-      expect(lastSubscriptionEventId).toMatch(/^admin-bump-\d+$/);
+      expect(lastSubscriptionEventId).toMatch(/^~admin-bump-\d+$/);
     });
   });
 
@@ -433,7 +433,7 @@ describe('BillingSubscriptionRepository unit tests:', () => {
 
       expect(lastSubscriptionEventCreatedAt).toBeGreaterThanOrEqual(before);
       expect(lastSubscriptionEventCreatedAt).toBeLessThanOrEqual(after);
-      expect(lastSubscriptionEventId).toMatch(/^dunning-\d+$/);
+      expect(lastSubscriptionEventId).toMatch(/^~dunning-\d+$/);
     });
   });
 });

modules/billing/tests/billing.webhook.subscription.unit.tests.js

@@ -600,6 +600,96 @@ describe('Billing webhook subscription unit tests:', () => {
         'invoice',
       );
     });
+
+    // V8.1 — syncOrganizationPlan failure path coverage
+    test('V8.1 — syncOrganizationPlan failure: non-fatal, logs error + emits sync_failed', async () => {
+      const existing = {
+        _id: subId,
+        organization: orgId,
+        pastDueSince: new Date('2026-04-01'),
+        status: 'unpaid',
+        plan: 'free',
+      };
+      mockSubscriptionRepository.findByStripeSubscriptionId.mockResolvedValue(existing);
+      mockStripe.subscriptions.retrieve.mockResolvedValue({
+        items: { data: [{ price: { metadata: { planId: 'pro' } } }] },
+      });
+      mockOrganizationRepository.setPlan.mockRejectedValue(new Error('DB write failed'));
+
+      // The logger mock is registered in beforeEach via jest.unstable_mockModule.
+      // Import it here (after BillingWebhookService) to get the same mocked instance
+      // that the service module captured at load time.
+      const { default: mockLogger } = await import('../../../lib/services/logger.js');
+
+      await expect(
+        BillingWebhookService.handleInvoicePaymentSucceeded({ subscription: 'sub_456' }, makeEvent()),
+      ).resolves.not.toThrow();
+
+      expect(mockLogger.error).toHaveBeenCalledWith(
+        '[billing.webhook] syncOrganizationPlan failed (non-fatal)',
+        expect.objectContaining({ organizationId: orgId }),
+      );
+      expect(mockEvents.emit).toHaveBeenCalledWith(
+        'billing.organization.sync_failed',
+        expect.objectContaining({ organizationId: orgId, source: 'dunning_recovery' }),
+      );
+    });
+
+    test('V8.1 — sync_failed listener throws: inner evtErr catch is non-fatal', async () => {
+      const existing = {
+        _id: subId,
+        organization: orgId,
+        pastDueSince: new Date('2026-04-01'),
+        status: 'unpaid',
+        plan: 'free',
+      };
+      mockSubscriptionRepository.findByStripeSubscriptionId.mockResolvedValue(existing);
+      mockStripe.subscriptions.retrieve.mockResolvedValue({
+        items: { data: [{ price: { metadata: { planId: 'pro' } } }] },
+      });
+      // Make syncOrganizationPlan throw so we enter the syncErr catch
+      mockOrganizationRepository.setPlan.mockRejectedValue(new Error('DB write failed'));
+      // Make billingEvents.emit throw so we enter the inner evtErr catch
+      mockEvents.emit.mockImplementation(() => { throw new Error('listener crash'); });
+
+      const { default: mockLogger } = await import('../../../lib/services/logger.js');
+
+      await expect(
+        BillingWebhookService.handleInvoicePaymentSucceeded({ subscription: 'sub_456' }, makeEvent()),
+      ).resolves.not.toThrow();
+
+      expect(mockLogger.error).toHaveBeenCalledWith(
+        '[billing.webhook] billing.organization.sync_failed listener error (non-fatal)',
+        expect.objectContaining({ error: 'listener crash' }),
+      );
+    });
+
+    test('V8.1 — validatePlan warns on unrecognized non-empty planId (falls back to free)', async () => {
+      const existing = {
+        _id: subId,
+        organization: orgId,
+        pastDueSince: new Date('2026-04-01'),
+        status: 'past_due',
+        plan: 'free',
+      };
+      mockSubscriptionRepository.findByStripeSubscriptionId.mockResolvedValue(existing);
+      // Return an unrecognized planId (e.g. a Stripe product ID instead of a plan slug)
+      mockStripe.subscriptions.retrieve.mockResolvedValue({
+        items: { data: [{ price: { metadata: { planId: 'prod_unknownXYZ' } } }] },
+      });
+
+      const { default: mockLogger } = await import('../../../lib/services/logger.js');
+
+      await expect(
+        BillingWebhookService.handleInvoicePaymentSucceeded({ subscription: 'sub_456' }, makeEvent()),
+      ).resolves.not.toThrow();
+
+      // validatePlan should have logged a warning for the unrecognized plan
+      expect(mockLogger.warn).toHaveBeenCalledWith(
+        '[billing.webhook] validatePlan: unrecognized planId',
+        expect.objectContaining({ raw: 'prod_unknownXYZ' }),
+      );
+    });
   });
 
   describe('handleInvoicePaymentFailed', () => {