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/controllers/billing.controller.js

@@ -133,8 +133,8 @@ const getUsage = async (req, res) => {
 // biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js controller, not Qwik
 const extrasCheckout = async (req, res) => {
   try {
-    const { packId, successUrl, cancelUrl } = req.body;
-    const result = await BillingService.createExtrasCheckout(req.organization, packId, successUrl, cancelUrl);
+    const { packId, successUrl, cancelUrl, intentId } = req.body;
+    const result = await BillingService.createExtrasCheckout(req.organization, packId, successUrl, cancelUrl, intentId);
     responses.success(res, 'extras checkout session created')(result);
   } catch (err) {
     const status = err.message?.startsWith('Invalid') || err.message?.includes('not found') ? 422 : 502;

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

@@ -73,6 +73,11 @@ const ExtrasCheckoutRequest = z
     packId: z.string().trim().min(1, 'packId is required'),
     successUrl: z.string().url('successUrl must be a valid URL'),
     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.
+    // 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

@@ -2,7 +2,10 @@
  * Module dependencies
  */
 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
@@ -78,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

@@ -1,12 +1,12 @@
 /**
  * Module dependencies
  */
-import { randomBytes } from 'node:crypto';
 import config from '../../../config/index.js';
 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.
@@ -147,6 +147,28 @@ const createCheckout = async (organization, priceId, successUrl, cancelUrl) => {
 
   const subscription = await _ensureStripeCustomer(stripe, organization);
 
+  // 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 liveSubs = await stripe.subscriptions.list({
+      customer: subscription.stripeCustomerId,
+      status: 'all',
+      limit: 10,
+    });
+    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;
+      err.code = 'subscription_already_active';
+      err.portalUrl = portalUrl;
+      throw err;
+    }
+  }
+
   const checkoutParams = {
     customer: subscription.stripeCustomerId,
     mode: 'subscription',
@@ -184,10 +206,15 @@ const createCheckout = async (organization, priceId, successUrl, cancelUrl) => {
  * @param {String} packId - Pack identifier (must exist in config.billing.packs)
  * @param {String} successUrl - URL to redirect on payment success
  * @param {String} cancelUrl - URL to redirect on cancel
+ * @param {String} [intentId] - Optional stable caller-provided intent ID for idempotency.
+ *        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
-const createExtrasCheckout = async (organization, packId, successUrl, cancelUrl) => {
+const createExtrasCheckout = async (organization, packId, successUrl, cancelUrl, intentId) => {
   const stripe = getStripe();
   if (!stripe) throw new Error('Stripe is not configured');
 
@@ -206,9 +233,14 @@ const createExtrasCheckout = async (organization, packId, successUrl, cancelUrl)
 
   const subscription = await _ensureStripeCustomer(stripe, organization);
 
-  // Per-intent idempotency key: timestamp + crypto-random suffix reduces collision risk under concurrent clicks.
-  // Full deduplication would require a caller-provided stable intent id — deferred to a future improvement.
-  const idempotencyKey = `extras_checkout_${String(organization._id)}_${packId}_${Date.now()}_${randomBytes(4).toString('hex')}`;
+  // Idempotency key strategy:
+  // - If intentId is provided by the caller (e.g. UUID generated on button click): fully stable key.
+  // - Otherwise: minute-bucketed key — prevents double-click within the same minute window.
+  //   This is strictly better than Date.now()+random (which disabled idempotency entirely).
+  const orgId = String(organization._id);
+  const idempotencyKey = intentId
+    ? `extras_checkout_${orgId}_${packId}_${intentId}`
+    : `extras_checkout_${orgId}_${packId}_${Math.floor(Date.now() / 60000)}`;
 
   const extrasCheckoutParams = {
     customer: subscription.stripeCustomerId,
@@ -217,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,
       },
     },
   };