chore: audit polish — migration test scoping, dead fallback, OAuth guard test, failure counters

[PierreBrisorgueil]

Summary

• What changed:
  • Integration test scopes the signup-grant backfill migration to its own seeded fixture orgs via a find()-filter intersection spy — the real migration logic (plan filter, grantOnSignup delegation, idempotency) is untouched, only the test's blast radius on the shared organizations collection is.
  • Removed the unreachable result.plan || 'free' fallback in organizations.crud.service.js::create (plan: 'free' is set unconditionally above) and aligned the organization.created emit to the literal 'free' used at the sibling call site (organizations.service.js::createOrganizationForUser).
  • Added a real-passport regression test for isEnabledOAuthProvider — registers a trivial strategy via passport.use() so a future passport upgrade renaming/removing the private _strategy API fails loudly in CI instead of silently 404-ing OAuth logins (kept in its own file: unstable_mockModule('passport', ...) factories are sticky per test file).
  • Migration completion log splits the failed counter into planNotFound / guardStripped / error buckets by pre-classifying with the same read-only, config-static plan lookup grantOnSignup uses internally, before calling it — never short-circuits the real call, only sharpens the log.
• Why: batches 4 small audit findings (M3, L2, L3, L4) into one polish PR — test-quality and observability, no behavior change.
• Related issues: Closes 🔧 Audit polish: migration test scoping, dead fallback, OAuth guard test, failure counter #3954

Scope

• Module(s) impacted: billing (migration + integration test), organizations (service + unit test), auth (new unit test)
• Cross-module impact: none
• Risk level: low

Validation

• npm run lint
• npm test
• Manual checks done (if applicable) — full suite 2649/2649 green (incl. integration), coverage unchanged

Guardrails check

• No secrets or credentials introduced (.env*, secrets/**, keys, tokens)
• No risky rename/move of core stack paths
• Changes remain merge-friendly for downstream projects
• Tests added or updated when behavior changed

Notes for reviewers

• Security considerations: none — test scoping + dead-code removal + observability only, no auth/billing logic changed
• Mergeability considerations: none, additive test file + small diffs
• Follow-up tasks (optional): none

https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup

Summary by CodeRabbit

• Bug Fixes

  • Organization creation now consistently records the Free plan, improving downstream billing and account setup.
  • Billing credit backfills now distinguish missing plans, unavailable signup grants, and processing errors in completion results.

• Reliability

  • Improved validation around OAuth strategy handling and billing credit recovery.
  • Migration checks are more isolated and reliable, reducing the risk of affecting unrelated organization records.

[coderabbitai]

Warning

Review limit reached

@PierreBrisorgueil, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6294d874-39d2-4f3d-9535-fb4f69b496d3

📥 Commits

Reviewing files that changed from the base of the PR and between 2a641b6 and 59fccaf.

📒 Files selected for processing (1)

• modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js

Walkthrough

Adds real-passport OAuth regression coverage, classifies signup-grant migration failures, scopes migration integration tests to fixtures, and makes organization creation events emit a constant free plan identifier.

Changes

OAuth passport guard

Layer / File(s)	Summary
Real passport OAuth regression test modules/auth/tests/auth.oauthCall.realPassportGuard.unit.tests.js, modules/auth/tests/fixtures/auth-controller.mock-setup.js	The test setup can load real passport, register a strategy, stub authentication, and verify stateless middleware delegation without calling next().

Signup grant backfill diagnostics

Layer / File(s)	Summary
Backfill failure classification modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js	Null grant results are classified as missing plans, stripped guards, or runtime errors, with separate completion counters.
Scoped migration integration coverage modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js	Migration runs are constrained to seeded organization IDs while retaining crediting, idempotency, and skip assertions.

Organization creation event

Layer / File(s)	Summary
Free plan event payload modules/organizations/services/organizations.crud.service.js, modules/organizations/tests/organizations.crud.grant.unit.tests.js	The creation event always emits planId: 'free', and the obsolete fallback-specific test is removed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• pierreb-devkit/Node#3947: Related OAuth oauthCall coverage and real-passport strategy validation.
• pierreb-devkit/Node#3957: Related changes to organization creation event emission.

Suggested labels: Tests, Refactor

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly reflects the main changes: migration test scoping, OAuth guard coverage, fallback removal, and migration logging counters.
Description check	✅ Passed	The PR description matches the required template and includes summary, scope, validation, guardrails, and notes.
Linked Issues check	✅ Passed	The changes address all four #3954 objectives: test scoping, dead fallback removal, real Passport coverage, and split migration failure logging.
Out of Scope Changes check	✅ Passed	The diff stays within the listed billing, organizations, and auth cleanup work and shows no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/3954-audit-polish

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.71%. Comparing base (2edb8e4) to head (59fccaf).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3960   +/-   ##
=======================================
  Coverage   92.71%   92.71%           
=======================================
  Files         169      169           
  Lines        5573     5573           
  Branches     1792     1791    -1     
=======================================
  Hits         5167     5167           
  Misses        326      326           
  Partials       80       80           

Flag	Coverage Δ
integration	61.00% <0.00%> (ø)
unit	74.44% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2edb8e4...59fccaf. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[PierreBrisorgueil]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js`:
- Around line 47-69: Update runScopedMigration’s mocked organizations collection
to return a Proxy around the real collection instead of a duck-typed object.
Intercept find to merge the fixture _id restriction with the migration filter,
while delegating all other property accesses and method calls to the real
collection with the correct collection receiver.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: baf80db2-1104-4038-b34a-e0cdda3296d3

📥 Commits

Reviewing files that changed from the base of the PR and between 2edb8e4 and 2a641b6.

📒 Files selected for processing (6)

• modules/auth/tests/auth.oauthCall.realPassportGuard.unit.tests.js
• modules/auth/tests/fixtures/auth-controller.mock-setup.js
• modules/billing/migrations/20260707100000-backfill-missing-signup-grant-credits.js
• modules/billing/tests/billing.migration.backfill-signup-grant.integration.tests.js
• modules/organizations/services/organizations.crud.service.js
• modules/organizations/tests/organizations.crud.grant.unit.tests.js

💤 Files with no reviewable changes (1)

• modules/organizations/tests/organizations.crud.grant.unit.tests.js