fix(billing): sentinel backfill + trialing guard + intentId + structured logger

C1 — add SENTINEL_PENDING constant + isUnresolved() helper; PI backfill resolver in
handleChargeRefunded recovers real cs_* session id from PaymentIntent metadata when
charge.metadata has __pending__; defensive guard in refundPartial rejects residual sentinels.

H1 — createCheckout live-sub check uses status:'all' + find for ['active','trialing']
(was status:'active', silently missed trialing subscriptions).

H2 — ExtrasCheckoutRequest Zod schema adds intentId z.string().max(180); createExtrasCheckout
accepts intentId; idempotency key is stable intentId-derived or minute-bucketed fallback.

H3 — all 7 console.error/warn sites in billing.webhook.service replaced with structured
logger.error/warn calls including { error, stack } context objects.

Tests: 44 billing suites, 657 tests all green. billing.usage.endpoint mock extended to
cover billing.extra.service import side-effect (logger instantiation in test context).

Author: PierreBrisorgueil

modules/billing/lib/billing.constants.js

@@ -3,6 +3,17 @@
  */
 import config from '../../../config/index.js';
 
+/**
+ * Sentinel value written to stripeSessionId during extras checkout session creation.
+ * Stripe forbids a session from self-referencing its own id at creation time, so this
+ * placeholder is used in both session.metadata and payment_intent_data.metadata.
+ * After checkout.session.completed fires, the real cs_* id is patched onto the
+ * PaymentIntent metadata — but Stripe Charge metadata is a one-time snapshot copied
+ * at charge creation, so charge.metadata.stripeSessionId may still carry this sentinel
+ * on refunds. Treat it as "unresolved" and trigger the PI backfill path.
+ */
+export const SENTINEL_PENDING = '__pending__';
+
 export const DEFAULT_METER_RUN_BASE = 1;
 export const DEFAULT_CRON_JITTER_MAX_MS = 60_000;
 export const DEFAULT_PLAN_CHANGE_PRESERVE_USAGE = true;

modules/billing/models/billing.subscription.schema.js

@@ -80,7 +80,9 @@ const ExtrasCheckoutRequest = z
     cancelUrl: z.string().url('cancelUrl must be a valid URL'),
     // Caller-provided stable intent ID for idempotency (prevents double-click double-charge).
     // When absent, a soft-stable minute-bucketed key is used as fallback.
-    intentId: z.string().min(1).optional(),
+    // Max 180 chars: Stripe idempotency key limit is 255; the prefix
+    // `extras_checkout_{orgId}_{packId}_` consumes ~64 chars, leaving ~10 chars margin.
+    intentId: z.string().min(1).max(180).optional(),
   })
   .strict();
 

modules/billing/services/billing.extra.service.js

@@ -5,6 +5,7 @@ import config from '../../../config/index.js';
 import logger from '../../../lib/services/logger.js';
 import billingEvents from '../lib/events.js';
 import BillingExtraBalanceRepository from '../repositories/billing.extraBalance.repository.js';
+import { SENTINEL_PENDING } from '../lib/billing.constants.js';
 
 /**
  * @function creditPack
@@ -80,22 +81,53 @@ const expireOldEntries = (orgId) =>
  *              original units were already consumed — this is the correct economic reflection
  *              (the debt persists until the next creditPack replenishes it).
  *
+ *              Defensive sentinel guard: if called with stripeSessionId === SENTINEL_PENDING,
+ *              it means the upstream caller failed to resolve the real session ID before calling.
+ *              Immediately return sentinel_unresolved to prevent a ledger lookup against a
+ *              placeholder value that will never match any topup entry.
+ *
  * @param {string} orgId - The organization ObjectId (string).
  * @param {string} stripeSessionId - Stripe session ID of the original purchase (to find the pack).
  * @param {number} amountRefundedCents - Amount refunded in cents (integer).
  * @param {string|undefined} [packId] - Optional pack identifier from Stripe metadata for exact correlation.
  * @param {string|undefined} [stripeRefundId] - Stripe refund object ID (e.g. rf_xxx). When present, used as
  *        primary idempotency key. When absent (legacy callers), falls back to session+amount+topupId composite.
  * @returns {Promise<{doc: Object|null, applied: boolean, refundUnits: number, reason?: string}>}
- *          `reason` is present only when `applied === false`: 'meter_mode_disabled' | 'invalid_org' |
- *          'pack_not_found' | 'ambiguous_pack_match'.
+ *          `reason` is present only when `applied === false`: 'meter_mode_disabled' | 'sentinel_unresolved' |
+ *          'invalid_org' | 'pack_not_found' | 'ambiguous_pack_match'.
  */
 // biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
 const refundPartial = async (orgId, stripeSessionId, amountRefundedCents, packId, stripeRefundId) => {
   if (!config?.billing?.meterMode) {
     return { doc: null, applied: false, reason: 'meter_mode_disabled', refundUnits: 0 };
   }
 
+  // Defensive sentinel guard — belt-and-suspenders in case the upstream caller missed
+  // the isUnresolved() check in handleChargeRefunded. A sentinel stripeSessionId can
+  // never match any topup ledger entry, so skip immediately rather than silently no-op.
+  if (stripeSessionId === SENTINEL_PENDING) {
+    logger.error('[billing.extra] refundPartial called with sentinel stripeSessionId — upstream missed isUnresolved() check', {
+      orgId,
+      amountRefundedCents,
+      packId,
+      stripeRefundId,
+    });
+    try {
+      billingEvents.emit('billing.refund.unresolved', {
+        reason: 'sentinel_unresolved',
+        orgId,
+        amountRefundedCents,
+        stripeRefundId,
+      });
+    } catch (evtErr) {
+      logger.error('[billing.extra] billing.refund.unresolved listener error (non-fatal)', {
+        error: evtErr?.message ?? String(evtErr),
+        stack: evtErr?.stack,
+      });
+    }
+    return { doc: null, applied: false, reason: 'sentinel_unresolved', refundUnits: 0 };
+  }
+
   // Find the topup ledger entry for this session to identify the pack
   const doc = await BillingExtraBalanceRepository.getOrCreate(orgId);
   if (!doc) return { doc: null, applied: false, reason: 'invalid_org', refundUnits: 0 };

modules/billing/services/billing.service.js

@@ -6,6 +6,7 @@ import getStripe from '../lib/stripe.js';
 import BillingPlansService from './billing.plans.service.js';
 import SubscriptionRepository from '../repositories/billing.subscription.repository.js';
 import { isDuplicateKeyError } from '../lib/billing.errors.js';
+import { SENTINEL_PENDING } from '../lib/billing.constants.js';
 
 /**
  * Validate that a redirect URL is safe for the current environment.
@@ -148,14 +149,17 @@ const createCheckout = async (organization, priceId, successUrl, cancelUrl) => {
 
   // Server-side active subscription guard — closes race window between local-DB check and
   // stripe.checkout.sessions.create (webhook may have arrived mid-flight).
+  // Fetches status:'all' and filters locally to cover both 'active' AND 'trialing' in one call
+  // (Stripe's status filter accepts only a single string, not an array).
   // +1 Stripe API call cost; acceptable given infrequent checkout entry point.
   if (subscription.stripeCustomerId) {
-    const liveActiveSubs = await stripe.subscriptions.list({
+    const liveSubs = await stripe.subscriptions.list({
       customer: subscription.stripeCustomerId,
-      status: 'active',
-      limit: 1,
+      status: 'all',
+      limit: 10,
     });
-    if (liveActiveSubs.data.length > 0) {
+    const blockingSub = liveSubs.data.find((s) => ['active', 'trialing'].includes(s.status));
+    if (blockingSub) {
       const portalUrl = await createPortalSession(organization);
       const err = new Error('Subscription already active');
       err.statusCode = 409;
@@ -206,6 +210,7 @@ const createCheckout = async (organization, priceId, successUrl, cancelUrl) => {
  *        When provided: key = `extras_checkout_{orgId}_{packId}_{intentId}` (fully stable).
  *        When absent: key bucketed to the minute — prevents instant double-click but not
  *        cross-minute replay. Callers should pass a UUID generated on button click.
+ *        Max 180 chars (Stripe idempotency key limit is 255; prefix consumes ~64 chars).
  * @returns {Promise<{url: String}>} Object with the Checkout session URL
  */
 // biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
@@ -244,17 +249,17 @@ const createExtrasCheckout = async (organization, packId, successUrl, cancelUrl,
     success_url: successUrl,
     cancel_url: cancelUrl,
     metadata: {
-      organizationId: String(organization._id),
+      organizationId: orgId,
       packId,
       kind: 'extras',
-      stripeSessionId: '__pending__',
+      stripeSessionId: SENTINEL_PENDING,
     },
     payment_intent_data: {
       metadata: {
-        organizationId: String(organization._id),
+        organizationId: orgId,
         packId,
         kind: 'extras',
-        stripeSessionId: '__pending__',
+        stripeSessionId: SENTINEL_PENDING,
       },
     },
   };

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

@@ -12,6 +12,19 @@ import OrganizationRepository from '../../organizations/repositories/organizatio
 import BillingExtraService from './billing.extra.service.js';
 import BillingResetService from './billing.reset.service.js';
 import billingEvents from '../lib/events.js';
+import { SENTINEL_PENDING } from '../lib/billing.constants.js';
+
+/**
+ * Treats a stripeSessionId as "unresolved" when absent, empty, or still the
+ * creation-time sentinel placeholder written before Stripe assigned a real cs_* id.
+ * Stripe Charge metadata is a one-time snapshot copied at charge creation — later
+ * PaymentIntent metadata patches do NOT propagate back, so charge.metadata.stripeSessionId
+ * may permanently carry SENTINEL_PENDING for any charge created during the brief window
+ * between session.create and checkout.session.completed.
+ * @param {string|undefined} id
+ * @returns {boolean}
+ */
+const isUnresolved = (id) => !id || id === SENTINEL_PENDING;
 
 /**
  * Maximum number of handler execution attempts before an event is dead-lettered.
@@ -112,14 +125,14 @@ const withIdempotency = async (event, handler) => {
     }
 
     // Rollback claim so Stripe can retry on a fresh delivery.
-    try {
-      await ProcessedStripeEventRepository.deleteByEventId(eventId);
-    } catch (rollbackErr) {
+    // Swallow rollback errors so the original handler error is always propagated.
+    await ProcessedStripeEventRepository.deleteByEventId(event.id).catch((rollbackErr) => {
       logger.error('[billing.webhook] rollback deleteByEventId failed — event may be stuck', {
-        eventId,
-        error: rollbackErr?.message ?? rollbackErr,
+        eventId: event.id,
+        error: rollbackErr?.message ?? String(rollbackErr),
+        stack: rollbackErr?.stack,
       });
-    }
+    });
     throw err;
   }
 };
@@ -185,6 +198,11 @@ const handleCheckoutCompleted = async (session) => {
  * @description Handle checkout.session.completed for mode='payment' — credit extras pack.
  *       Extracts organizationId, packId, kind from session metadata.
  *       Skips silently if payment_status !== 'paid', kind !== 'extras', or metadata is incomplete.
+ *       Backfills PaymentIntent metadata with the real cs_* session ID so that
+ *       charge.refunded events can correlate the charge back to the correct ledger entry.
+ *       At session creation time stripeSessionId is set to SENTINEL_PENDING (Stripe forbids
+ *       self-reference). Charge.metadata is a one-time snapshot, so the PI patch is best-effort;
+ *       handleChargeRefunded has a backfill resolver as a secondary defence.
  * @param {Object} session - Stripe checkout session object (mode='payment')
  * @returns {Promise<void>}
  */
@@ -203,7 +221,7 @@ const handleCheckoutPaymentCompleted = async (session) => {
 
   // Backfill PaymentIntent metadata with the real session ID so that charge.refunded
   // events can correlate the charge back to this ledger entry.
-  // At session.create time stripeSessionId was set to '__pending__' (Stripe forbids
+  // At session.create time stripeSessionId was set to SENTINEL_PENDING (Stripe forbids
   // self-reference). Propagating the real cs_* ID here ensures charge.metadata carries
   // it when a refund is issued later.
   if (paymentIntentId) {
@@ -215,12 +233,16 @@ const handleCheckoutPaymentCompleted = async (session) => {
             organizationId,
             packId,
             kind: 'extras',
-            stripeSessionId,  // real cs_* ID
+            stripeSessionId,  // real cs_* ID (replaces SENTINEL_PENDING)
           },
         });
       } catch (err) {
-        // Log but don't fail — refund correlation may need fallback path
-        console.warn('[billing.webhook] PaymentIntent metadata update failed:', err.message);
+        // Log but don't fail — refund correlation may use the backfill resolver path
+        logger.warn('[billing.webhook] PaymentIntent metadata update failed', {
+          paymentIntentId,
+          error: err?.message ?? String(err),
+          stack: err?.stack,
+        });
       }
     }
   }
@@ -289,7 +311,10 @@ const handleSubscriptionUpdated = async (subscription, event) => {
         });
       } catch (evtErr) {
         // Listener errors must not disrupt webhook processing — log for traceability
-        console.error('[billing.webhook] plan.changed listener error (non-fatal):', evtErr?.message ?? evtErr);
+        logger.error('[billing.webhook] plan.changed listener error (non-fatal)', {
+          error: evtErr?.message ?? String(evtErr),
+          stack: evtErr?.stack,
+        });
       }
 
       // Plan switch mid-cycle = refresh the active week snapshot to the new plan.
@@ -307,10 +332,10 @@ const handleSubscriptionUpdated = async (subscription, event) => {
         await BillingResetService.forceRotateForPlanChange(organizationId, { preserveUsage: true });
       } catch (err) {
         planChangeResetTriggered = false;
-        console.error(
-          '[billing.webhook] forceRotateForPlanChange failed, falling back to resetWeek:',
-          err?.message ?? err,
-        );
+        logger.error('[billing.webhook] forceRotateForPlanChange failed, falling back to resetWeek', {
+          error: err?.message ?? String(err),
+          stack: err?.stack,
+        });
       }
     }
   }
@@ -328,7 +353,10 @@ const handleSubscriptionUpdated = async (subscription, event) => {
       await BillingResetService.resetWeek(organizationId, newPeriodStart);
     } catch (err) {
       // Log for monitoring — not thrown so webhook processing continues
-      console.error('[billing.webhook] resetWeek failed (non-fatal):', err?.message ?? err);
+      logger.error('[billing.webhook] resetWeek failed (non-fatal)', {
+        error: err?.message ?? String(err),
+        stack: err?.stack,
+      });
     }
   }
 };
@@ -360,7 +388,10 @@ const handleSubscriptionDeleted = async (subscription, event) => {
     await BillingResetService.forceRotateForPlanChange(organizationId, { preserveUsage: false });
   } catch (err) {
     // Log for monitoring — not thrown so webhook processing continues
-    console.error('[billing.webhook] forceRotateForPlanChange on cancel failed (non-fatal):', err?.message ?? err);
+    logger.error('[billing.webhook] forceRotateForPlanChange on cancel failed (non-fatal)', {
+      error: err?.message ?? String(err),
+      stack: err?.stack,
+    });
   }
 };
 
@@ -398,7 +429,10 @@ const handleInvoicePaymentFailed = async (invoice, event) => {
     billingEvents.emit('payment.failed', { organizationId });
   } catch (evtErr) {
     // Listener errors must not disrupt webhook processing — log for traceability
-    console.error('[billing.webhook] payment.failed listener error (non-fatal):', evtErr?.message ?? evtErr);
+    logger.error('[billing.webhook] payment.failed listener error (non-fatal)', {
+      error: evtErr?.message ?? String(evtErr),
+      stack: evtErr?.stack,
+    });
   }
 };
 
@@ -452,6 +486,12 @@ const handleInvoicePaymentSucceeded = async (invoice, event) => {
  *       Each refund's rf_ id is used as the idempotency key, making webhook replay safe.
  *       Individual entries are silently skipped when: metadata is incomplete, refund amount
  *       is absent/zero, or the refund object has no id.
+ *
+ *       SENTINEL handling: at session.create time stripeSessionId is set to SENTINEL_PENDING
+ *       ('__pending__'). Stripe Charge metadata is a one-time snapshot — even though
+ *       checkout.session.completed patches the PaymentIntent with the real cs_* id,
+ *       charge.metadata.stripeSessionId may permanently carry SENTINEL_PENDING.
+ *       Both absent AND sentinel values trigger the PI backfill resolver path.
  * @param {Object} charge - Stripe charge object
  * @returns {Promise<void>}
  */
@@ -466,8 +506,10 @@ const handleChargeRefunded = async (charge) => {
 
   if (!organizationId || !mongoose.Types.ObjectId.isValid(organizationId)) return;
 
-  // Backfill resolver: if stripeSessionId is missing, fetch the PaymentIntent to find it.
-  if (!stripeSessionId && paymentIntentId) {
+  // Backfill resolver: if stripeSessionId is missing OR carries SENTINEL_PENDING,
+  // fetch the PaymentIntent to find the real session ID patched by checkout.session.completed.
+  // isUnresolved() treats both falsy values and the sentinel string as "unresolved".
+  if (isUnresolved(stripeSessionId) && paymentIntentId) {
     const stripe = getStripe();
     if (stripe) {
       try {
@@ -482,12 +524,17 @@ const handleChargeRefunded = async (charge) => {
           }
         }
       } catch (err) {
-        logger.error('[billing] refund PI fetch failed', { chargeId: charge.id, paymentIntentId, error: err?.message });
+        logger.error('[billing] refund PI fetch failed', {
+          chargeId: charge.id,
+          paymentIntentId,
+          error: err?.message ?? String(err),
+          stack: err?.stack,
+        });
       }
     }
   }
 
-  if (!stripeSessionId) {
+  if (isUnresolved(stripeSessionId)) {
     // Could not resolve stripeSessionId from charge or PaymentIntent metadata.
     // Emit alert event and log for manual reconciliation.
     const refundAmount = charge.refunds?.data?.reduce((sum, r) => sum + (r.amount ?? 0), 0) ?? 0;
@@ -499,7 +546,10 @@ const handleChargeRefunded = async (charge) => {
     try {
       billingEvents.emit('billing.refund.unresolved', { chargeId: charge.id, paymentIntentId, refundAmount });
     } catch (evtErr) {
-      console.error('[billing.webhook] billing.refund.unresolved listener error (non-fatal):', evtErr?.message ?? evtErr);
+      logger.error('[billing.webhook] billing.refund.unresolved listener error (non-fatal)', {
+        error: evtErr?.message ?? String(evtErr),
+        stack: evtErr?.stack,
+      });
     }
     return;
   }

modules/billing/tests/billing.checkout.unit.tests.js

@@ -37,7 +37,7 @@ describe('Billing service unit tests:', () => {
         },
       },
       subscriptions: {
-        // Server-side active-sub guard (Item 9): returns no active subs by default
+        // Default: no live active/trialing subs — checkout proceeds
         list: jest.fn().mockResolvedValue({ data: [] }),
       },
     };