feat(invitations): referral substrate — referredBy + invitation.accepted event (P8a)#3824
Merged
Merged
Conversation
…ted (#3814) P8a of the invitations↔org decouple epic: wire the referral substrate (schema + event only, NO credit-grant logic). - E14: add server-only `referredBy` (ObjectId ref User, default:null) to the User Mongoose model. Deliberately NOT in the Zod schema — the signup POST path replaces req.body with the full Zod output, so any field in the schema would become client-writable; omitting it makes Zod strip it from every client parse (signup + PUT). Documented inline. - E22: new InvitationsService.accept(invite, userId) — the shared accept seam invoked by the eligibility `finalize` closure on BOTH the token two-phase path AND the OAuth-by-email path. Finalizes the invite, stamps referredBy = invite.invitedBy server-side via UserService.updateById (raw, bypasses the client whitelist), and emits `invitation.accepted`. Best-effort side-effects (a write failure or listener throw never breaks signup). invitedBy may be null (admin-created invite): skip the redundant null write, still emit with invitedBy:null. auth.controller stays import-free (relays the closure unchanged; the name stays `finalize`). - billing.init: no-op `invitation.accepted` listener (+ TODO(#5)) proving the fire-and-forget seam end-to-end (billing → invitations consumer). - events.js: finalize the documented payload { invitationId, email, invitedBy, acceptedUserId }. E20: verified `referredBy` is absent from config.whitelists.users.update + updateAdmin (removeSensitive strips it); added negative tests on PUT /api/users and the admin update path. End-to-end tests: token + OAuth invited signups set referredBy from the inviter (a client-supplied referredBy in the body is ignored); the event fires with the documented payload. Tests: full unit suite green (1899); invitations + users integration green.
…+ read-exposure note Review follow-ups (Approved-with-minors, no behavior change): - events.js: clarify the emit-site try/catch only catches SYNCHRONOUS listener throws; an async listener rejection escapes as unhandledRejection. Warn the future #5 credit-grant listener to self-guard (or switch emit to Promise.allSettled). Mirror as a TODO(#5) at the billing no-op listener site. - invitations.service.js: add invitationId to the best-effort referredBy-write warn so a lost-attribution event is self-contained for tracing. - users.schema.js: note that reading/exposing referredBy (e.g. P8b view) must go via a response projection / read whitelist, never by adding it to this Zod schema (also the signup-POST write surface — would reopen the client-writable hole).
|
Warning Review limit reached
More reviews will be available in 49 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR implements a referral substrate for invitations: a server-only ChangesReferral Substrate (referredBy + invitation.accepted)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3824 +/- ##
==========================================
+ Coverage 91.80% 91.82% +0.02%
==========================================
Files 159 159
Lines 5187 5201 +14
Branches 1652 1655 +3
==========================================
+ Hits 4762 4776 +14
Misses 337 337
Partials 88 88
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds the backend “referral substrate” for the invitations decouple effort (epic #3808 / P8a): a server-only referredBy link on users plus an invitation.accepted event emitted when an invite is successfully consumed, with billing wired as a no-op listener for the future credit-grant feature (#5).
Changes:
- Add
referredByto the User Mongoose model (server-set only; intentionally omitted from Zod write surface) and add negative integration coverage to ensure it cannot be client/admin-written. - Introduce
InvitationsService.accept(invite, userId)as the shared “finalize + referral wiring + event emit” seam used by both token and OAuth invite-accept paths. - Document
invitation.acceptedevent semantics and wire a no-op consumer inbilling.init.js, with unit/integration tests validating the seam and payload.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/users/tests/user.admin.integration.tests.js | Adds an admin-path negative test proving referredBy is not persisted via admin user update. |
| modules/users/tests/user.account.integration.tests.js | Adds self-update negative test proving referredBy is not client-writable via PUT /api/users. |
| modules/users/models/users.schema.js | Documents intentional omission of referredBy from Zod write schemas to keep it server-only. |
| modules/users/models/users.model.mongoose.js | Adds referredBy field to User schema (ObjectId ref User, default null). |
| modules/invitations/tests/invitations.service.unit.tests.js | Adds unit coverage for InvitationsService.accept (finalize + referredBy stamping + event payload + best-effort behavior). |
| modules/invitations/tests/invitations.integration.tests.js | Adds end-to-end tests verifying referredBy persistence for token + OAuth flows and event emission payload. |
| modules/invitations/tests/invitations.init.unit.tests.js | Updates init unit tests to assert finalize closure routes through accept(invite, userId). |
| modules/invitations/services/invitations.service.js | Implements accept() seam: finalize invite, best-effort stamp referredBy, emit invitation.accepted. |
| modules/invitations/lib/events.js | Documents invitation.accepted payload and clarifies sync vs async listener error behavior. |
| modules/invitations/invitations.init.js | Changes finalize closure to call InvitationsService.accept(invite, userId) instead of finalize(id, userId). |
| modules/billing/tests/billing.init.unit.tests.js | Adds unit test asserting billing wires a no-op invitation.accepted listener. |
| modules/billing/billing.init.js | Wires a no-op invitation.accepted listener (TODOs for #5). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/billing.init.js`:
- Around line 56-60: Add a JSDoc header for the new invitation.accepted listener
callback that documents the callback signature and return type: place a JSDoc
block immediately above the invitationEvents.on('invitation.accepted', (payload)
=> { ... }) listener describing `@param` {Object} payload (include common fields
used such as payload.invitedBy and note that it may be null) and `@returns`
{void}; keep the description concise and follow the project's JSDoc style for JS
files.
In `@modules/invitations/invitations.init.js`:
- Around line 90-93: The inline closure assigned to finalize should be extracted
into a named function with a JSDoc header so params/returns are explicit; create
a function like finalizeInvite(userId) that calls
InvitationsService.accept(invite, userId), add a JSDoc block documenting `@param`
{string|number} userId and `@returns` {Promise<...>} (matching
InvitationsService.accept), and replace the closure in the returned object
(finalize: finalizeInvite) to satisfy the JSDoc requirement.
In `@modules/invitations/services/invitations.service.js`:
- Around line 235-267: The current flow performs side-effects even when
finalize(invite.id, userId) returns null; change the logic in the accept flow to
only proceed with referral write (UserService.updateById) and the emit of
invitationEvents.emit('invitation.accepted', ...) when the finalize result is
truthy (non-null). Specifically, after calling finalize(...) check the returned
result and return early if it's null; only then attempt the referredBy update
and wrap the emit in the existing try/catch so no side-effects occur for
non-finalized invites, keeping existing logger.warn handling for failures.
🪄 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: fd4e1d3b-c74e-406c-9810-d184371e7614
📒 Files selected for processing (12)
modules/billing/billing.init.jsmodules/billing/tests/billing.init.unit.tests.jsmodules/invitations/invitations.init.jsmodules/invitations/lib/events.jsmodules/invitations/services/invitations.service.jsmodules/invitations/tests/invitations.init.unit.tests.jsmodules/invitations/tests/invitations.integration.tests.jsmodules/invitations/tests/invitations.service.unit.tests.jsmodules/users/models/users.model.mongoose.jsmodules/users/models/users.schema.jsmodules/users/tests/user.account.integration.tests.jsmodules/users/tests/user.admin.integration.tests.js
…+ add JSDoc - Accept() now returns null immediately when finalize() returns null (duplicate-accept / revoked / missing invite), preventing spurious referredBy writes and invitation.accepted emits for non-finalized invites. - Add JSDoc to billing.init invitation.accepted listener callback. - Extract finalize closure in invitations.init into named finalizeInvite() with JSDoc header. - Add unit test: finalize() null → no side-effects, return null.
This was referenced Jun 11, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Phase 8a — invitations ↔ org decouple (epic #3808)
The referral substrate for the credit offer (#5): schema + event only, zero credit-grant logic.
What lands
referredByon the user (ObjectId, ref User, default null) — the inviter's id, set server-side on invite-accept. Server-set ONLY, never client-writable (verified at 3 layers): omitted from the ZodUser/UserUpdateschemas (so the signup-POSTmodel.isValidstrip drops it from the body), absent from bothusers.update/updateAdminwhitelists (soremoveSensitivestrips it fromPUT /api/users+ admin update), written only via the rawUserService.updateByIdserver path. (Deliberately NOT in Zod — adding it there would make it client-writable on signup; the server path bypasses Zod, so it isn't needed there.)finalizeclosure now routes throughInvitationsService.accept(invite, userId): finalizes the invite (P3 behavior) → setsuser.referredBy = invite.invitedBy(best-effort, logged on failure — signup success is the invariant) → emitsinvitation.accepted. Fires on BOTH the token signup path AND the OAuth path (checkOAuthUserProfile) — OAuth-invited users are credited too.auth.controller.jsstill imports no invitation code.invitation.acceptedevent{ invitationId, email, invitedBy, acceptedUserId }(emit try/catch-wrapped against sync listener throws).billing.init.jswires a no-op listener (+TODO(#5)).E20 — referredBy blocked on all user-update paths
Verified absent from the update whitelists; negative tests assert a client-supplied
referredByis ignored on signup POST,PUT /api/users, and admin update (raw-doc reads).Verification
invitedByhandling, event payload + listener-throw crash-safety, billing no-op spy, 3 E20 negatives — all read the raw doc).TODO(#5)s).Forward notes (for #5, not this PR)
events.js).referredByindex to be added with adapting to front #5's "who did I refer" query (so the index shape matches the access pattern).Out of scope (filed separately)
A pre-existing hole surfaced during the E20 audit:
emailIS in the update whitelist and changing it does NOT resetemailVerified→ a verified user can swap to an unverified email keepingemailVerified:true, which OAuthlinkProviderByEmailtrusts. Being filed as its own issue/ERRORS.md entry.Closes #3814. Part of #3808.
Summary by CodeRabbit
New Features
Security