You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
markFailedAttempt: single atomic aggregation pipeline op — closes TOCTOU where 2 concurrent crons both win the $inc phase then both flip status: failed, double-emitting billing.extras_debit.exhausted
incrementMeterWithOutbox: try/catch + E11000 fetch for outbox create crash recovery — closes free compute leak where meter incremented but outbox row missing
Zero-cost attribute(): now writes idempotency key even with units=0 — closes double-charge replay when later retry has non-zero costs
forceRotateForPlanChange: planChangeResetTriggered set BEFORE try — closes double-rotate via fallback resetWeek on throw
E11000 retry: legacy consumedHistoryIds filter added symmetrically to retry branch
BillingMeterOutbox: timestamps:true, drop manual createdAt
billing.policy.js: drop stale ESLint no-unused-vars disable
B — DRY + lib extraction (3 🟠 + 5 🟡 + 3 🔵)
billing.webhook.controller: all 3 raw res.status().json() → responses.error() canonical envelope
billing.controller: direct repo call → BillingExtraService.getOrgBalanceContext() (layering fix)
_ensureStripeCustomer: extracted private helper deduplicating ~50 lines of find-or-create logic between createCheckout + createExtrasCheckout
billing.processedStripeEvent.schema.js: NEW — Zod schema parity with Mongoose model
lib/billing.isoWeek.js: NEW — currentWeekKey() + isoWeekKey(date) single source (was duplicated in usage.service + reset.service with DST divergence risk)
findAllDueForReset deprecated: removed from repo + test mocks
lib/billing.cron-utils.js: NEW — applyJitter(maxMs) using crypto.randomInt, applied to all 3 crons
billing.plans.controller: unit tests added (success + Stripe down paths)
lib/billing.errors.js: NEW — isDuplicateKeyError(err) single source (was 3 inline duplicates)
lib/billing.constants.js: NEW — all shared getters/constants, kills dynamic imports in hot path
C — Config knobs surfacing (NEW)
All magic numbers extracted to config.billing.* with safe defaults. Backward-compat: all defaults preserve current behavior. runBaseUnits kept as deprecated alias.
@PierreBrisorgueil has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 41 minutes and 38 seconds before requesting another review.
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.
⌛ How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.
We recommend that you space out your commits to avoid hitting the rate limit.
🚦 How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
❌ Patch coverage is 90.84967% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.20%. Comparing base (fcc7183) to head (d18eda7). ⚠️ Report is 1 commits behind head on master.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR successfully implements several core hardening requirements, such as atomic aggregation for failed attempts and idempotency keys for zero-cost attributions. However, the current implementation contains a high-severity logic flaw in the usage overflow detection: users on zero-unit plans (like 'Free' or 'Pay-as-you-go') will bypass billing for their usage because the system incorrectly skips extra calculations when the quota is zero.
Furthermore, the PR has been flagged by Codacy as 'not up to standards,' primarily due to a high volume of code clones and linting issues. Specifically, the initialization logic across multiple cron scripts is identical and should be centralized. There are also several false-positive linter warnings related to Qwik that need to be suppressed to clean up the CI output.
While the extraction of shared utilities (ISO weeks, jitter) is a step in the right direction, addressing the overflow logic and the systemic duplication is necessary before merging.
About this PR
There is significant duplication in the boot sequence of the billing cron scripts. Consider centralizing the environment setup, dynamic imports, and jitter application into a shared 'cronBoot' utility in 'modules/billing/lib/'. This will reduce boilerplate and make the boot logic easier to maintain.
Several Node.js service and test files are triggering false-positive 'useQwikValidLexicalScope' warnings. These should be suppressed using the biome-ignore comment to prevent noise in the quality reports.
1 comment outside of the diffmodules/billing/services/billing.usage.service.js
line 125 🔴 HIGH RISK
Remove the effectiveQuota > 0 condition from the overflow detection logic. For plans with zero included units, every unit consumed should be treated as an overflow. The current code results in 0 extrasConsumed for these users, effectively giving them free usage.
Test suggestions
Concurrent markFailedAttempt calls emit exactly one exhausted event
Outbox creation handles E11000 by returning the existing row instead of failing
Zero-cost attribution writes the idempotency key and blocks later non-zero replays for that step
Boot fails when legacy consumedHistoryIds are detected in meterMode
Handle partial refunds correctly by identifying the latest refund delta via timestamp
Config overrides for jitter and retry intervals are correctly resolved through constants
ISO week key handles year rollover and DST transitions correctly
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Hardens the billing module’s metering/outbox flows and surfaces configuration knobs, while reducing duplication and aligning controllers/services with the project’s layering conventions.
Changes:
Improves outbox failure accounting + adds E11000 recovery for outbox create after meter increment.
Extracts shared billing utilities (ISO week, jitter, duplicate-key detection, config getters) and deprecates/removes old reset query paths.
Expands/updates unit + integration test coverage around plan-change rotation, refunds, outbox retries, and config knobs.
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 3 comments.
This cron connects to Mongo without calling mongooseService.loadModels() before importing repositories/services. Because the repositories resolve Mongoose models at module scope, this can crash with MissingSchemaError in standalone cron execution. Call mongooseService.loadModels() before connect(), and only import Billing repositories/services after models are loaded.
This cron connects to Mongo without calling mongooseService.loadModels() before importing repositories (and Org repository). In a standalone CronJob, importing repositories that do mongoose.model(...) at module scope can throw MissingSchemaError if models weren’t loaded. Call mongooseService.loadModels() before connect(), and only import repositories/services after models are loaded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Comprehensive billing module hardening bundling 3 logical groupings on overlapping surfaces. Goal: module the simplest, cleanest, most configurable possible.
Source: 4-agent audit 2026-05-02 found 22 backend findings (1 🔴 + 7 🟠 + 8 🟡 + 4 🔵 + 2 ⚪).
Sections
A — Atomicity hardening (🔴 + 7 issues)
markFailedAttempt: single atomic aggregation pipeline op — closes TOCTOU where 2 concurrent crons both win the$incphase then both flipstatus: failed, double-emittingbilling.extras_debit.exhaustedincrementMeterWithOutbox: try/catch + E11000 fetch for outbox create crash recovery — closes free compute leak where meter incremented but outbox row missingattribute(): now writes idempotency key even withunits=0— closes double-charge replay when later retry has non-zero costsforceRotateForPlanChange:planChangeResetTriggeredset BEFORE try — closes double-rotate via fallbackresetWeekon throwconsumedHistoryIdsfilter added symmetrically to retry branchunitsFromCosts: nullratioVersion+ non-null costs → hard-fail in meterMode, warn otherwiseBillingMeterOutbox:timestamps:true, drop manualcreatedAtbilling.policy.js: drop stale ESLintno-unused-varsdisableB — DRY + lib extraction (3 🟠 + 5 🟡 + 3 🔵)
billing.webhook.controller: all 3 rawres.status().json()→responses.error()canonical envelopebilling.controller: direct repo call →BillingExtraService.getOrgBalanceContext()(layering fix)_ensureStripeCustomer: extracted private helper deduplicating ~50 lines of find-or-create logic betweencreateCheckout+createExtrasCheckoutbilling.processedStripeEvent.schema.js: NEW — Zod schema parity with Mongoose modellib/billing.isoWeek.js: NEW —currentWeekKey()+isoWeekKey(date)single source (was duplicated in usage.service + reset.service with DST divergence risk)findAllDueForResetdeprecated: removed from repo + test mockslib/billing.cron-utils.js: NEW —applyJitter(maxMs)usingcrypto.randomInt, applied to all 3 cronsbilling.plans.controller: unit tests added (success + Stripe down paths)lib/billing.errors.js: NEW —isDuplicateKeyError(err)single source (was 3 inline duplicates)lib/billing.constants.js: NEW — all shared getters/constants, kills dynamic imports in hot pathC — Config knobs surfacing (NEW)
All magic numbers extracted to
config.billing.*with safe defaults. Backward-compat: all defaults preserve current behavior.runBaseUnitskept as deprecated alias.billing.meter.{runBase, fallbackPlanId, maxUnitsPerOperation}billing.outbox.{maxRetryAttempts, retryIntervalSec}billing.crons.jitterMaxMsbilling.planChange.preserveUsageDefaultbilling.alerts.thresholdPercentsbilling.events.extrasExhaustedREADME documents each knob with downstream override examples.
Boot migration guard (B.7)
Dual-read
consumedHistoryIdsremoved. Boot now assertscountDocuments({consumedHistoryIds: {$exists: true}}) === 0whenmeterMode: true— aborts deploy if unmigrated docs remain.Test plan
billing.lifecycle.integration.tests.js)billing.lifecycle.integration.tests.js)billing.webhook.subscription.unit.tests.js)billing.config-knobs.unit.tests.js)