refactor(billing): slim Subscription + delegate Stripe API + V5 P1 fixes (#1 #2 #3) (#3588)

* refactor(billing): slim BillingSubscription schema (delegate periodEnd to Stripe API)

Drop currentPeriodEnd and cancelAtPeriodEnd from Mongoose model and Zod schema.
Add stripeEventId field (tiebreaker for same-second event ordering, V5 P1 #2).
Migration 20260503000200 unsets the removed fields from existing documents.

* refactor(billing): fetchSubscriptionDetails helper + getSubscription merge Stripe details

New fetchSubscriptionDetails() fetches currentPeriodEnd, cancelAtPeriodEnd, status,
nextRenewalDate from Stripe API on-demand (+50ms on GET /subscription, 0ms on quota gate).
getSubscription() merges cached local fields with live Stripe details for UI consumers.
Falls back gracefully when Stripe is not configured or subscription is free.

* refactor(billing): unified webhook handlers — drop period fields, pass eventId

handleSubscriptionUpdated: drop currentPeriodEnd/cancelAtPeriodEnd from $set fields.
All 4 handlers now call updateIfEventNewer(id, created, eventId, fields).
handleInvoicePaymentSucceeded: add event param + use updateIfEventNewer (V5 P1 #1).
handleSubscriptionDeleted: force meter rotate to free on cancel (V5 P1 #3).

* fix(billing): event tiebreaker via eventId for same-second deliveries (V5 P1 #2)

updateIfEventNewer now takes eventId as 3rd param. Same-second events are ordered
by lex string comparison of evt_ IDs, preventing the 2nd event from being silently
skipped when both events have the same Unix second timestamp.
Stores stripeEventId alongside stripeEventCreatedAt in the document.

* refactor(billing): drop billing.refund.service — inline Stripe refund in admin controller

billing.refund.service.js had a single function (refundCharge) used only by the admin
controller. Inline it directly to eliminate an unnecessary service indirection layer.
Idempotency key pattern and argument validation are preserved unchanged.

* test(billing): update tests for PR2 — new signatures, V5 P1 coverage, inline refund

- subscription.schema: drop cancelAtPeriodEnd/currentPeriodEnd tests, add stripeEventId
- subscription.repository: updateIfEventNewer now 4-arg; add same-second tiebreaker test
- webhook.integration: add eventId arg to all updateIfEventNewer assertions; V5 P1 #3 tests
- webhook.subscription: updateIfEventNewer 4-arg; handleInvoicePaymentSucceeded with event
- billing.service: updateIfEventNewer 4-arg; add forceRotateForPlanChange mock
- billing.checkout: fetchSubscriptionDetails tests + updated getSubscription (Stripe merge)
- billing.admin.integration: rewritten for inline Stripe (no refund service mock)
- billing.refund.service: rewritten as admin controller refund tests

* fix(billing): payment_succeeded markers + admin refund key + reason guard

- webhook: always advance stripeEventCreatedAt/stripeEventId on
  invoice.payment_succeeded, even when pastDueSince is already null;
  prevents stale out-of-order replays from being applied against a
  markers state that was never advanced on routine invoices
- admin: replace fixed idempotency key with randomUUID suffix so
  multiple partial refunds on the same charge are not collapsed within
  Stripe's 24 h idempotency window
- reason validation already enforced upstream via AdminRefundRequest
  Zod schema (z.enum); no controller change required

Author: PierreBrisorgueil

modules/billing/controllers/billing.admin.controller.js

@@ -1,19 +1,49 @@
 /**
  * Module dependencies
  */
+import { randomUUID } from 'node:crypto';
+
 import responses from '../../../lib/helpers/responses.js';
-import BillingRefundService from '../services/billing.refund.service.js';
+import getStripe from '../lib/stripe.js';
 
 /**
- * @desc Admin endpoint to trigger a Stripe refund for a charge
+ * @desc Admin endpoint to trigger a Stripe refund for a charge.
+ *       Initiates the Stripe refund; actual ledger debit happens via the
+ *       `charge.refunded` webhook (single source of truth).
  * @param {Object} req - Express request object
  * @param {Object} res - Express response object
  * @returns {Promise<void>}
  */
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js controller, not Qwik
 const adminRefundCharge = async (req, res) => {
   try {
     const { chargeId, amountCents, reason } = req.body;
-    const refund = await BillingRefundService.refundCharge(chargeId, amountCents, { reason });
+
+    if (typeof chargeId !== 'string' || chargeId.trim() === '') {
+      return responses.error(res, 422, 'Unprocessable Entity', 'Failed to refund charge')(
+        new Error('invalid argument: chargeId must be a non-empty string'),
+      );
+    }
+    if (amountCents !== undefined) {
+      if (!Number.isInteger(amountCents) || amountCents <= 0) {
+        return responses.error(res, 422, 'Unprocessable Entity', 'Failed to refund charge')(
+          new Error('invalid argument: amountCents must be a positive integer'),
+        );
+      }
+    }
+
+    const stripe = getStripe();
+    if (!stripe) {
+      return responses.error(res, 502, 'Bad Gateway', 'Failed to refund charge')(
+        new Error('Stripe is not configured'),
+      );
+    }
+
+    const idempotencyKey = `refund_${chargeId}_${amountCents ?? 'full'}_${randomUUID()}`;
+    const params = { charge: chargeId, reason: reason || 'requested_by_customer' };
+    if (amountCents !== undefined) params.amount = amountCents;
+
+    const refund = await stripe.refunds.create(params, { idempotencyKey });
     responses.success(res, 'billing refund created')(refund);
   } catch (err) {
     const status = err.message?.startsWith('invalid argument') ? 422 : 502;

modules/billing/controllers/billing.webhook.controller.js

@@ -54,7 +54,7 @@ const handleWebhook = async (req, res) => {
         break;
       case 'invoice.payment_succeeded':
         await BillingWebhookService.withIdempotency(event, (e) =>
-          BillingWebhookService.handleInvoicePaymentSucceeded(e.data.object),
+          BillingWebhookService.handleInvoicePaymentSucceeded(e.data.object, e),
         );
         break;
       case 'charge.refunded':

modules/billing/migrations/20260503000200-slim-subscription-drop-period-fields.js

@@ -0,0 +1,31 @@
+/**
+ * Migration: Slim BillingSubscription — drop currentPeriodEnd and cancelAtPeriodEnd.
+ * These fields are now fetched on-demand from the Stripe API via fetchSubscriptionDetails().
+ * stripeEventId is added for same-second event tiebreaker ordering (V5 P1 #2) — no migration
+ * needed as it is optional and populated at the next webhook delivery.
+ *
+ * Safe to run multiple times (idempotent).
+ */
+export async function up() {
+  const { default: mongoose } = await import('mongoose');
+  const db = mongoose.connection.db;
+
+  await db.collection('subscriptions').updateMany(
+    {},
+    {
+      $unset: {
+        currentPeriodEnd: '',
+        cancelAtPeriodEnd: '',
+      },
+    },
+  );
+
+  console.info('[migration] slim-subscription-drop-period-fields: dropped currentPeriodEnd + cancelAtPeriodEnd');
+}
+
+/**
+ * Down: no-op — dropped fields are re-populated by incoming webhooks.
+ */
+export function down() {
+  console.warn('[migration] slim-subscription-drop-period-fields DOWN: no-op; fields will be re-populated by webhooks');
+}

modules/billing/models/billing.subscription.model.mongoose.js

@@ -37,13 +37,6 @@ const SubscriptionMongoose = new Schema(
       enum: config.billing.statuses,
       default: 'active',
     },
-    currentPeriodEnd: {
-      type: Date,
-    },
-    cancelAtPeriodEnd: {
-      type: Boolean,
-      default: false,
-    },
 
     // ── Meter fields (sparse — backward-compatible additions) ────────────────
 
@@ -87,6 +80,14 @@ const SubscriptionMongoose = new Schema(
       type: Number,
       default: null,
     },
+    /**
+     * Stripe event.id of the last processed subscription event.
+     * Tiebreaker for same-second events (lex string ordering on evt_ IDs).
+     */
+    stripeEventId: {
+      type: String,
+      default: null,
+    },
   },
   {
     timestamps: true,

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

@@ -19,20 +19,19 @@ const baseShape = {
   organization: z.string().trim().regex(objectIdRegex, 'organization must be a valid ObjectId'),
   stripeCustomerId: optionalStripeId,
   stripeSubscriptionId: optionalStripeId,
-  currentPeriodEnd: z.coerce.date().nullable().optional(),
   // ── Meter fields (optional — backward-compatible) ────────────────────────
   planVersion: z.string().trim().optional(),
   currentPeriodStart: z.coerce.date().nullable().optional(),
   pastDueSince: z.coerce.date().nullable().optional(),
   lastResetAt: z.coerce.date().nullable().optional(),
   stripeEventCreatedAt: z.number().int().nullable().optional(),
+  stripeEventId: z.string().nullable().optional(),
 };
 
 const Subscription = z.object({
   ...baseShape,
   plan: z.enum(config.billing.plans).default('free'),
   status: z.enum(config.billing.statuses).default('active'),
-  cancelAtPeriodEnd: z.boolean().default(false),
 });
 
 /**
@@ -42,7 +41,6 @@ const SubscriptionCore = z.object({
   ...baseShape,
   plan: z.enum(config.billing.plans),
   status: z.enum(config.billing.statuses),
-  cancelAtPeriodEnd: z.boolean(),
 });
 
 const SubscriptionUpdate = SubscriptionCore.partial();

modules/billing/repositories/billing.subscription.repository.js

@@ -216,21 +216,36 @@ const markUnpaid = (id, threshold) => {
  * @description Atomically update a subscription only when the incoming Stripe event is newer
  *              than the last processed event. Prevents out-of-order webhook delivery from
  *              overwriting more-recent state.
+ *              Same-second events are ordered by lex-string comparison of event IDs (evt_ prefix
+ *              makes these globally monotonic within a second) — V5 P1 #2 tiebreaker.
  *              Returns null when the guard prevents the write (stale event).
  * @param {string} id - The subscription ObjectId (string).
- * @param {number} eventCreatedAt - Stripe event.created Unix timestamp.
+ * @param {number} eventCreatedAt - Stripe event.created Unix timestamp (seconds).
+ * @param {string} eventId - Stripe event.id (e.g. evt_xxx) — tiebreaker for same-second delivery.
  * @param {Object} fields - Fields to $set on the document.
  * @returns {Promise<Object|null>} Updated doc, or null if id invalid or event is stale.
  */
 // biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js repository, not Qwik
-const updateIfEventNewer = (id, eventCreatedAt, fields) => {
+const updateIfEventNewer = (id, eventCreatedAt, eventId, fields) => {
   if (!id || !mongoose.Types.ObjectId.isValid(id)) return null;
   return Subscription.findOneAndUpdate(
     {
       _id: id,
-      $or: [{ stripeEventCreatedAt: null }, { stripeEventCreatedAt: { $lt: eventCreatedAt } }],
+      $or: [
+        { stripeEventCreatedAt: { $exists: false } },
+        { stripeEventCreatedAt: null },
+        { stripeEventCreatedAt: { $lt: eventCreatedAt } },
+        {
+          stripeEventCreatedAt: eventCreatedAt,
+          $or: [
+            { stripeEventId: { $exists: false } },
+            { stripeEventId: null },
+            { stripeEventId: { $lt: eventId } },
+          ],
+        },
+      ],
     },
-    { $set: { ...fields, stripeEventCreatedAt: eventCreatedAt } },
+    { $set: { ...fields, stripeEventCreatedAt: eventCreatedAt, stripeEventId: eventId } },
     { returnDocument: 'after', runValidators: true },
   )
     .populate(defaultPopulate)

modules/billing/services/billing.refund.service.js

@@ -244,15 +244,49 @@ const createExtrasCheckout = async (organization, packId, successUrl, cancelUrl)
 };
 
 /**
- * @desc Get subscription for the given organization
+ * @desc Fetch live subscription details from Stripe API on-demand.
+ *       Used by UI routes that need currentPeriodEnd, cancelAtPeriodEnd etc.
+ *       +50ms latency is acceptable on infrequent "My Subscription" page loads.
+ *       Not cached — callers can layer a short TTL if needed in the future.
+ * @param {String|null} stripeSubscriptionId - Stripe subscription ID (sub_xxx)
+ * @returns {Promise<Object|null>} Live fields or null when id is absent / Stripe not configured
+ */
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
+const fetchSubscriptionDetails = async (stripeSubscriptionId) => {
+  if (!stripeSubscriptionId) return null;
+  const stripe = getStripe();
+  if (!stripe) return null;
+  const stripeSub = await stripe.subscriptions.retrieve(stripeSubscriptionId);
+  const cancelAt = stripeSub.cancel_at ? new Date(stripeSub.cancel_at * 1000) : null;
+  return {
+    currentPeriodStart: new Date(stripeSub.current_period_start * 1000),
+    currentPeriodEnd: new Date(stripeSub.current_period_end * 1000),
+    cancelAtPeriodEnd: stripeSub.cancel_at_period_end,
+    status: stripeSub.status,
+    nextRenewalDate: cancelAt ?? new Date(stripeSub.current_period_end * 1000),
+  };
+};
+
+/**
+ * @desc Get subscription for the given organization.
+ *       Merges cached local fields with live Stripe details for UI consumers.
+ *       Falls back gracefully when Stripe is not configured or the subscription is free.
  * @param {String} organizationId - The organization ID
- * @returns {Promise<Object|null>} The subscription document or null
+ * @returns {Promise<Object|null>} Merged subscription object or null
  */
-const getSubscription = async (organizationId) => SubscriptionRepository.findByOrganization(organizationId);
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
+const getSubscription = async (organizationId) => {
+  const sub = await SubscriptionRepository.findByOrganization(organizationId);
+  if (!sub) return null;
+  const details = await fetchSubscriptionDetails(sub.stripeSubscriptionId).catch(() => null);
+  if (!details) return sub;
+  return { ...sub.toJSON?.() ?? sub, ...details };
+};
 
 export default {
   createCheckout,
   createExtrasCheckout,
   createPortalSession,
+  fetchSubscriptionDetails,
   getSubscription,
 };

modules/billing/services/billing.service.js

@@ -212,12 +212,10 @@ const handleSubscriptionUpdated = async (subscription, event) => {
   const fields = {
     plan: newPlan,
     status: subscription.status,
-    currentPeriodEnd: new Date(subscription.current_period_end * 1000),
-    cancelAtPeriodEnd: subscription.cancel_at_period_end,
   };
   if (newPeriodStart) fields.currentPeriodStart = newPeriodStart;
 
-  const updated = await SubscriptionRepository.updateIfEventNewer(String(existing._id), event.created, fields);
+  const updated = await SubscriptionRepository.updateIfEventNewer(String(existing._id), event.created, event.id, fields);
   if (!updated) {
     logger.info('[billing.webhook] skipped stale event', { eventId: event.id, type: event.type });
     return;
@@ -305,7 +303,7 @@ const handleSubscriptionDeleted = async (subscription, event) => {
   const existing = await SubscriptionRepository.findByStripeSubscriptionId(subscription.id);
   if (!existing) return;
 
-  const updated = await SubscriptionRepository.updateIfEventNewer(String(existing._id), event.created, {
+  const updated = await SubscriptionRepository.updateIfEventNewer(String(existing._id), event.created, event.id, {
     plan: 'free',
     status: 'canceled',
   });
@@ -314,7 +312,16 @@ const handleSubscriptionDeleted = async (subscription, event) => {
     return;
   }
 
-  await syncOrganizationPlan(existing.organization?._id || existing.organization, 'free');
+  const organizationId = String(existing.organization?._id || existing.organization);
+  await syncOrganizationPlan(organizationId, 'free');
+
+  // Force reset meter to free quota — canceled sub must not retain paid-plan snapshot units.
+  try {
+    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);
+  }
 };
 
 /**
@@ -340,7 +347,7 @@ const handleInvoicePaymentFailed = async (invoice, event) => {
     fields.pastDueSince = new Date();
   }
 
-  const updated = await SubscriptionRepository.updateIfEventNewer(String(existing._id), event.created, fields);
+  const updated = await SubscriptionRepository.updateIfEventNewer(String(existing._id), event.created, event.id, fields);
   if (!updated) {
     logger.info('[billing.webhook] skipped stale event', { eventId: event.id, type: event.type });
     return;
@@ -359,24 +366,33 @@ const handleInvoicePaymentFailed = async (invoice, event) => {
  * @description Handle invoice.payment_succeeded event — clear degraded mode (pastDueSince).
  *       When a past-due invoice is finally paid, remove the pastDueSince marker so
  *       the subscription exits degraded mode on next request.
+ *       Uses updateIfEventNewer to guard against out-of-order webhook delivery (V5 P1 #1).
  * @param {Object} invoice - Stripe invoice object
+ * @param {Object} event - Full Stripe event (with event.created and event.id for ordering)
  * @returns {Promise<void>}
  */
 // biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
-const handleInvoicePaymentSucceeded = async (invoice) => {
+const handleInvoicePaymentSucceeded = async (invoice, event) => {
   const { subscription: stripeSubscriptionId } = invoice;
   if (!stripeSubscriptionId) return;
 
   const existing = await SubscriptionRepository.findByStripeSubscriptionId(stripeSubscriptionId);
   if (!existing) return;
 
-  // Only clear if currently past_due (avoid unnecessary writes on routine invoices)
-  if (existing.pastDueSince !== null && existing.pastDueSince !== undefined) {
-    await SubscriptionRepository.update({
-      _id: existing._id,
-      pastDueSince: null,
-      status: 'active',
-    });
+  // Always advance the event markers (stripeEventCreatedAt + stripeEventId) so out-of-order
+  // replays are correctly rejected even when the sub is already active (pastDueSince == null).
+  const fields = (existing.pastDueSince !== null && existing.pastDueSince !== undefined)
+    ? { pastDueSince: null, status: 'active' }
+    : {};
+
+  const updated = await SubscriptionRepository.updateIfEventNewer(
+    String(existing._id),
+    event.created,
+    event.id,
+    fields,
+  );
+  if (!updated) {
+    logger.info('[billing.webhook] skipped stale event', { eventId: event.id, type: event.type });
   }
 };