fix(billing): dispute reinstate credit + reconcile meter↔extras + balance guards (Batch 2) (#3620)

* fix(billing): dispute reinstate credit + reconcile meter↔extras + balance guards (Batch 2)

Five money-flow safety items from the Opus/DeepSeek dual audit:

Item 1 (DeepSeek HIGH): POST /api/admin/billing/dispute/credit/:orgId — idempotent
admin endpoint to restore extras balance after dispute.funds_reinstated. Uses new
creditCompensation() repo method (adjustment ledger kind). Updates RUNBOOKS #1 step 6
with concrete curl example.

Item 2 (Opus C1): _checkMeterExtrasMismatch() in reconcile service — per-org meter↔extras
divergence detection. Compares meterUsed overflow vs ledger debit sums for current period.
Emits billing.reconciliation.divergence with subType:meter_extras_mismatch when delta
exceeds tolerance (max(50, 0.5% × max(expected, actual))). Non-fatal; Batch 1 listener
catches the event.

Item 3 (Opus H1): Runaway negative balance detection in incrementMeter. After extras debit,
checks cachedBalance < -10×meterQuota — emits billing.extras.runaway_debit + logs error.
Check skipped for free plans (quota=0). Debit already applied (defensive only).

Item 4 (Opus H4): Org-existence guard in BillingExtraBalanceRepository.debit. On fresh-doc
path, validates Organization OR Subscription exists before creating an orphan balance doc.
Accepts Subscription as proof of org existence to support provisioning flows.

Item 5 (Opus H7): handleInvoicePaymentSucceeded early return on healthy subs (pastDueSince
null). Skips the updateIfEventNewer call entirely — eliminating ~95% of runValidators
overhead on normal payment webhooks. Past-due subs still update normally.

* fix(billing): address DeepSeek pre-push review findings (Batch 2 follow-up)

- [HIGH] advance invoice-family marker for healthy subs in
  handleInvoicePaymentSucceeded — always call updateIfEventNewer with {}
  fields to guard against stale replay of older invoice events
- [CASL] fix policy path to /api/admin/billing/dispute/credit/:orgId
  (exact-match requires :orgId param for correct resolution)
- [MEDIUM] remove mongoose from billing.admin.service — 24-char hex regex
  replaces mongoose.Types.ObjectId.isValid() (ERRORS.md architecture rule)
- [MEDIUM] remove mongoose.model() from billing.reconcile.service — delegate
  to repositories: add SubscriptionRepository.findPageForReconciliation and
  BillingExtraBalanceRepository.findLedgerByOrg; _checkMeterExtrasMismatch
  now uses lazy repo imports via BillingUsageRepository.findByWeek
- Tests: reconcile suite mocks repos directly; webhook test reflects new
  marker-advance behavior for healthy subs

* fix(billing): address Copilot review threads (Batch 2 follow-up 2)

- JSDoc + warn log: update route path to /dispute/credit/:orgId
- reason param: pass as memo to creditCompensation (persisted in ledger)
- debit() hot path: use .exists() instead of findOne+lean for doc check
- reconcile comment: clarify period boundary vs weekly meter scope
- Tests: update mocks from findOne to exists; add reason to creditCompensation assertion

* test(billing): cover dispatchWebhookEvent switch — 11 cases + default

Adds 12 unit tests for the replay-path switch in admin.service.js:
- 11 case-each tests verifying each Stripe event type routes through
  withIdempotency to the matching webhook service handler
- 1 test for the default case logging unhandled events and returning
  { skipped: true }

Closes the codecov/patch gap (lines 378-422 in admin.service.js were
0% covered).

* test(billing): cover adminDisputeCredit endpoint + adminListDeadLetters 422 path

Adds 6 integration tests:
- POST /api/admin/billing/dispute/credit/:orgId — admin 200, non-admin 403,
  invalid body 422, charge not found 404, unexpected error 500
- GET /api/admin/billing/dead-letters — invalid query 422 path

Closes the codecov/patch gap on:
- billing.admin.controller.js:282-315 (adminDisputeCredit body)
- billing.admin.controller.js:219 (adminListDeadLetters Zod 422 path)

Author: PierreBrisorgueil

modules/billing/RUNBOOKS.md

@@ -22,7 +22,24 @@ Operational runbooks for the billing module. Each runbook references real endpoi
    ```
 4. Gather evidence in Stripe Dashboard: usage records, signup email, ToS acceptance timestamp, IP logs.
 5. Submit evidence before day 7 via Stripe Dashboard → Dispute → Submit evidence.
-6. If dispute won (`charge.dispute.funds_reinstated` received): verify the extras credit was re-applied by re-checking the ledger via `GET /api/admin/billing/customer/:orgId`.
+6. If dispute won (`charge.dispute.funds_reinstated` received): restore the customer's extras balance via:
+   ```
+   POST /api/admin/billing/dispute/credit/:orgId
+   Body: {
+     "chargeId": "ch_xxx",
+     "amountCents": <dispute_amount_cents>,
+     "reason": "dispute won — funds reinstated on charge ch_xxx",
+     "refundRequestId": "<uuid>"
+   }
+   ```
+   Example curl:
+   ```bash
+   curl -X POST https://api.trawl.me/api/admin/billing/dispute/credit/<orgId> \
+     -H "Authorization: Bearer $ADMIN_JWT" \
+     -H "Content-Type: application/json" \
+     -d '{"chargeId":"ch_xxx","amountCents":2000,"reason":"dispute won — Stripe reinstated funds","refundRequestId":"<uuid>"}'
+   ```
+   Confirm credit applied: `GET /api/admin/billing/customer/:orgId` — check ledger for an `adjustment` entry with `refId: dispute-credit-<uuid>`.
 7. If dispute lost: extras balance debited by `charge.dispute.funds_withdrawn` is not refunded — log in incident tracker.
 
 ---

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

@@ -5,6 +5,7 @@ import responses from '../../../lib/helpers/responses.js';
 import getStripe from '../lib/stripe.js';
 import BillingAdminService from '../services/billing.admin.service.js';
 import { AdminDeadLettersQuery } from '../models/billing.subscription.schema.js';
+import logger from '../../../lib/services/logger.js';
 
 /**
  * @desc Admin endpoint to trigger a Stripe refund for a charge.
@@ -268,6 +269,53 @@ const adminCancelSubscription = async (req, res) => {
   }
 };
 
+/**
+ * @desc POST /api/admin/billing/dispute/credit/:orgId
+ *       Apply a manual extras-balance credit after a dispute is won (funds_reinstated).
+ *       Idempotent via refundRequestId — safe to call multiple times per dispute.
+ * @param {Object} req - Express request object (body validated as AdminDisputeCreditRequest)
+ * @param {Object} res - Express response object
+ * @returns {Promise<void>}
+ */
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js controller, not Qwik
+const adminDisputeCredit = async (req, res) => {
+  try {
+    const { chargeId, amountCents, reason, refundRequestId } = req.body;
+    const { orgId } = req.params;
+
+    const rawAdminId = req.user?._id;
+    if (!rawAdminId) {
+      return responses.error(res, 422, 'Unprocessable Entity', 'Failed to apply dispute credit')(
+        new Error('invalid argument: authenticated user id is missing'),
+      );
+    }
+    const adminUserId = String(rawAdminId);
+
+    const result = await BillingAdminService.creditDisputeReinstated(
+      chargeId,
+      amountCents,
+      reason,
+      refundRequestId,
+      orgId,
+      adminUserId,
+    );
+
+    logger.info('[billing.admin] adminDisputeCredit completed', {
+      orgId,
+      chargeId,
+      amountCents,
+      applied: result.applied,
+      adminUserId,
+    });
+
+    return responses.success(res, 'dispute credit applied')(result);
+  } catch (err) {
+    const status = err.status ?? 500;
+    const title = status === 404 ? 'Not Found' : status === 422 ? 'Unprocessable Entity' : 'Internal Server Error';
+    return responses.error(res, status, title, 'Failed to apply dispute credit')(err);
+  }
+};
+
 export default {
   adminRefundCharge,
   adminBumpPlan,
@@ -277,4 +325,5 @@ export default {
   adminListDeadLetters,
   adminPurgeDeadLetter,
   adminCancelSubscription,
+  adminDisputeCredit,
 };

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

@@ -127,11 +127,31 @@ const AdminDeadLettersQuery = z
   })
   .strict();
 
+/**
+ * Dispute credit request body schema.
+ * Used by POST /api/admin/billing/dispute/credit to apply a manual ledger credit
+ * when Stripe rules in our favor on a dispute (funds_reinstated).
+ *
+ * chargeId        — Stripe charge ID (ch_xxx) to correlate with the dispute.
+ * amountCents     — Amount to credit in cents (positive integer).
+ * reason          — Ops note for audit trail (3–200 chars).
+ * refundRequestId — UUID generated by the frontend per click for idempotency.
+ */
+const AdminDisputeCreditRequest = z
+  .object({
+    chargeId: z.string().regex(/^ch_/, 'chargeId must start with ch_'),
+    amountCents: z.number().int().positive('amountCents must be a positive integer'),
+    reason: z.string().min(3, 'reason must be at least 3 chars').max(200, 'reason must be at most 200 chars'),
+    refundRequestId: z.string().min(8, 'refundRequestId must be at least 8 characters'),
+  })
+  .strict();
+
 export {
   AdminRefundRequest,
   AdminBumpPlanRequest,
   AdminWebhookReplayRequest,
   AdminDeadLettersQuery,
+  AdminDisputeCreditRequest,
 };
 
 export default {
@@ -144,4 +164,5 @@ export default {
   AdminBumpPlanRequest,
   AdminWebhookReplayRequest,
   AdminDeadLettersQuery,
+  AdminDisputeCreditRequest,
 };

modules/billing/policies/billing.policy.js

@@ -27,6 +27,8 @@ export function billingSubjectRegistration({ registerPathSubject }) {
   registerPathSubject('/api/admin/billing/webhook/replay', 'BillingAdminOps');
   registerPathSubject('/api/admin/billing/dead-letters', 'BillingAdminOps');
   registerPathSubject('/api/admin/billing/cancel', 'BillingAdminOps');
+  // Dispute credit — manual ledger credit after funds_reinstated (Batch 2)
+  registerPathSubject('/api/admin/billing/dispute/credit/:orgId', 'BillingAdminOps');
   // Webhook is mounted in billing.preroute.js before body parsing and auth middleware,
   // so it bypasses policy.isAllowed entirely. Registered here for documentation completeness.
   registerPathSubject('/api/billing/webhook', 'BillingWebhook');

modules/billing/repositories/billing.extraBalance.repository.js

@@ -2,6 +2,7 @@
  * Module dependencies
  */
 import mongoose from 'mongoose';
+import AppError from '../../../lib/helpers/AppError.js';
 
 /**
  * Validate that orgId is a syntactically valid MongoDB ObjectId.
@@ -116,6 +117,27 @@ const debit = async (orgId, amount, refId) => {
   };
 
   // Step 1: ensure the document exists (atomic getOrCreate, no-op if already present).
+  // Guard: if no doc exists yet, validate that the organization exists before creating a
+  // dangling ExtraBalance doc for a ghost org (Opus H4). Accepts proof via either the
+  // Organization collection OR the Subscription collection — a valid Subscription is
+  // sufficient evidence that the org is real (provisioning sometimes writes Subscription
+  // before the full Organization doc, e.g. in CLI tools or integration tests).
+  const existing = await BillingExtraBalance().exists({ organization: orgId });
+  if (!existing) {
+    const { default: OrganizationRepository } = await import('../../organizations/repositories/organizations.repository.js');
+    const orgExists = await OrganizationRepository.exists({ _id: orgId });
+    if (!orgExists) {
+      const Subscription = mongoose.model('Subscription');
+      const subExists = await Subscription.exists({ organization: orgId });
+      if (!subExists) {
+        throw Object.assign(
+          new AppError(`Organization not found: ${orgId}`, { status: 404, code: 'ORGANIZATION_NOT_FOUND' }),
+          { organizationId: orgId },
+        );
+      }
+    }
+  }
+
   await BillingExtraBalance().findOneAndUpdate(
     { organization: orgId },
     { $setOnInsert: { organization: orgId, ledger: [], cachedBalance: 0, cachedBalanceAt: new Date() } },
@@ -140,6 +162,57 @@ const debit = async (orgId, amount, refId) => {
   return { doc: null, applied: false, reason: 'duplicate_step' };
 };
 
+/**
+ * @function creditCompensation
+ * @description Atomically push a positive 'adjustment' ledger entry for dispute/ops compensation.
+ *              Idempotent: if a ledger entry with the same refId already exists the update
+ *              is a no-op and applied=false is returned.
+ *              Uses a 2-step pattern aligned with creditPack:
+ *                Step 1 — ensure doc exists (getOrCreate, no-op on replay).
+ *                Step 2 — idempotency-guarded credit push.
+ *              Intended use: admin dispute reinstatement, manual ops corrections.
+ *
+ * @param {string} orgId - The organization ObjectId (string).
+ * @param {number} amount - Meter units to credit (must be > 0).
+ * @param {string} refId - Unique idempotency key (e.g. `dispute-credit-<refundRequestId>`).
+ * @param {string} [memo] - Optional human-readable memo stored in the ledger entry.
+ * @returns {Promise<{doc: Object|null, applied: boolean, reason?: string}>}
+ */
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js repository, not Qwik
+const creditCompensation = async (orgId, amount, refId, memo = '') => {
+  if (!isValidOrgId(orgId)) return { doc: null, applied: false };
+  if (!Number.isFinite(amount) || amount <= 0) throw new Error('invalid argument: amount must be a positive finite number');
+  if (typeof refId !== 'string' || refId.trim() === '') throw new Error('invalid argument: refId must be a non-empty string');
+
+  const entry = {
+    kind: 'adjustment',
+    amount,
+    refId,
+    at: new Date(),
+    ...(memo ? { memo } : {}),
+  };
+
+  // Step 1: ensure the document exists (atomic getOrCreate, no-op if already present).
+  await getOrCreate(orgId);
+
+  // Step 2: idempotency-guarded credit (no upsert — doc is guaranteed to exist after step 1).
+  const doc = await BillingExtraBalance().findOneAndUpdate(
+    {
+      organization: orgId,
+      'ledger.refId': { $ne: refId },
+    },
+    {
+      $push: { ledger: entry },
+      $inc: { cachedBalance: amount },
+      $set: { cachedBalanceAt: new Date() },
+    },
+    { returnDocument: 'after' },
+  );
+
+  if (doc) return { doc, applied: true };
+  return { doc: null, applied: false, reason: 'duplicate_refId' };
+};
+
 /**
  * @function addExpirationEntries
  * @description Sweep topup entries that have expired and push matching expiration
@@ -355,13 +428,32 @@ const findOrgsWithExpiringTopups = async (now) => {
   return orgIds;
 };
 
+/**
+ * Fetch the full ledger array for an org.
+ * Used by the reconcile service to compute actual extras debits in the current period.
+ * Returns null when no document exists yet (org has never had extras).
+ * @param {string} orgId - Organization ObjectId (string).
+ * @returns {Promise<Object[]|null>} Ledger entries array or null.
+ */
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js repository, not Qwik
+const findLedgerByOrg = async (orgId) => {
+  if (!isValidOrgId(orgId)) return null;
+  const doc = await BillingExtraBalance().findOne(
+    { organization: orgId },
+    { ledger: 1 },
+  ).lean();
+  return doc?.ledger ?? null;
+};
+
 export default {
   getOrCreate,
   creditPack,
+  creditCompensation,
   debit,
   addExpirationEntries,
   refundPartial,
   getBalance,
   listLedgerPage,
   findOrgsWithExpiringTopups,
+  findLedgerByOrg,
 };

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

@@ -310,6 +310,25 @@ const adminUpdatePlanOnly = (id, planId, adminUserId) => {
     .exec();
 };
 
+/**
+ * Fetch one page of subscriptions matching given statuses for reconciliation.
+ * Used by the billing reconcile service (replaces direct mongoose.model() access there).
+ * @param {string[]} statuses - Subscription statuses to include.
+ * @param {number} page - 0-based page index.
+ * @param {number} limit - Page size.
+ * @returns {Promise<Object[]>} Lean subscription documents.
+ */
+// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js repository, not Qwik
+const findPageForReconciliation = (statuses, page, limit) =>
+  Subscription.find(
+    { status: { $in: statuses }, stripeSubscriptionId: { $ne: null } },
+    { _id: 1, organization: 1, stripeSubscriptionId: 1, stripeCustomerId: 1, plan: 1, status: 1, currentPeriodStart: 1 },
+  )
+    .skip(page * limit)
+    .limit(limit)
+    .lean()
+    .exec();
+
 export default {
   list,
   create,
@@ -326,4 +345,5 @@ export default {
   markUnpaid,
   updateIfEventNewer,
   adminUpdatePlanOnly,
+  findPageForReconciliation,
 };

modules/billing/routes/billing.admin.routes.js

@@ -10,6 +10,7 @@ import {
   AdminRefundRequest,
   AdminBumpPlanRequest,
   AdminWebhookReplayRequest,
+  AdminDisputeCreditRequest,
 } from '../models/billing.subscription.schema.js';
 
 /**
@@ -59,4 +60,9 @@ export default (app) => {
     .route('/api/admin/billing/cancel/:orgId')
     .all(passport.authenticate('jwt', { session: false }), policy.isAllowed)
     .post(billingAdmin.adminCancelSubscription);
+
+  app
+    .route('/api/admin/billing/dispute/credit/:orgId')
+    .all(passport.authenticate('jwt', { session: false }), policy.isAllowed)
+    .post(model.isValid(AdminDisputeCreditRequest), billingAdmin.adminDisputeCredit);
 };