feat(auth): trigger handleSignupOrganization on verifyEmail success (best-effort)#3765
Conversation
…best-effort) Wire AuthOrganizationService.handleSignupOrganization into verifyEmail after the emailVerified flag is persisted. Required for mailer-deferred org/grant provisioning (N2 signup path): when email verification is deferred, org setup must run at verify-time, not only at signup time. The call is idempotent (convergence guard: re-verify edge cases do not double-credit the grant). Wrapped in try/catch — org-setup failure must NEVER fail email verification (regression risk if wrapper is removed). Closes #3762 Promotes trawl_node trawl#1317 up to devkit. Part of infra plan 2026-06-01-trawl-promote-up-followups.md Task 4.
WalkthroughThe PR wires post-verification organization provisioning into the email verification flow. After successful email verification, ChangesEmail verification with org provisioning
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 docstrings
🧪 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 |
There was a problem hiding this comment.
Pull request overview
Wires AuthOrganizationService.handleSignupOrganization(user) into the verifyEmail controller flow so that org/grant provisioning happens at email verification time when mailer-deferred verification is in use. The call is wrapped in a best-effort try/catch so a provisioning error cannot fail email verification.
Changes:
- Mark
user.emailVerified = trueon the local object after persisting the verified flag, then invokehandleSignupOrganizationinsideverifyEmail. - Wrap the org-setup call in a
try/catchthat logs a warning on failure (idempotent + non-fatal). - Add a new unit test file with two cases: handler called with
emailVerified=true, and verifyEmail still succeeds when org provisioning throws.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| modules/auth/controllers/auth.controller.js | Adds local emailVerified=true mark plus best-effort handleSignupOrganization try/catch in verifyEmail. |
| modules/auth/tests/auth.verifyEmail.signup-org.unit.tests.js | New ESM unit tests covering happy path and best-effort failure path for the new wiring. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3765 +/- ##
===========================================
+ Coverage 59.43% 90.14% +30.71%
===========================================
Files 151 151
Lines 4970 4974 +4
Branches 1577 1577
===========================================
+ Hits 2954 4484 +1530
+ Misses 1491 385 -1106
+ Partials 525 105 -420
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/auth/controllers/auth.controller.js`:
- Around line 672-682: When handleSignupOrganization fails we should record a
reconciliation trigger so provisioning is retried later; catch the error in
AuthOrganizationService.handleSignupOrganization call (the existing try/catch)
and instead of only logging, set a durable flag on the User (e.g.,
user.needsOrgProvisioning = true) or enqueue a background job to retry
provisioning, then save the user; also update signin/token flow (where
autoSetCurrentOrganization is invoked) to detect verified users with no active
membership and either call AuthOrganizationService.handleSignupOrganization
again or enqueue the same retry job so provisioning occurs on next signin;
optionally implement a periodic background sweep that queries users with
needsOrgProvisioning to re-run handleSignupOrganization and clear the flag on
success.
🪄 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: 79038e53-9165-463a-8d8e-dfc5b2487c71
📒 Files selected for processing (2)
modules/auth/controllers/auth.controller.jsmodules/auth/tests/auth.verifyEmail.signup-org.unit.tests.js
| // Post-verification org setup — provision org/grant if not yet done (best-effort). | ||
| // handleSignupOrganization is idempotent: if the org already exists it converges | ||
| // without double-crediting. Failure must never block email verification success. | ||
| try { | ||
| await AuthOrganizationService.handleSignupOrganization(user); | ||
| } catch (orgErr) { | ||
| logger.warn('[auth.verifyEmail] org/grant provisioning failed (non-fatal)', { | ||
| userId: user.id, | ||
| error: orgErr?.message, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Best-effort swallow is correct, but consider a reconciliation path for permanent provisioning failures.
Wrapping handleSignupOrganization in try/catch and logging is the right call for the "must not block verification" requirement, and the service's idempotent convergence guard makes re-invocation safe. However, when provisioning fails here the error is only logged — a verified user can end up with no organization/grants and no automatic recovery, since signin/token only call autoSetCurrentOrganization (which sets an existing membership, never creates one). Consider a reconciliation trigger (e.g., re-attempt handleSignupOrganization on next signin when the user has verified email but no active membership, or a background sweep) so a transient failure here doesn't strand the account.
🤖 Prompt for 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.
In `@modules/auth/controllers/auth.controller.js` around lines 672 - 682, When
handleSignupOrganization fails we should record a reconciliation trigger so
provisioning is retried later; catch the error in
AuthOrganizationService.handleSignupOrganization call (the existing try/catch)
and instead of only logging, set a durable flag on the User (e.g.,
user.needsOrgProvisioning = true) or enqueue a background job to retry
provisioning, then save the user; also update signin/token flow (where
autoSetCurrentOrganization is invoked) to detect verified users with no active
membership and either call AuthOrganizationService.handleSignupOrganization
again or enqueue the same retry job so provisioning occurs on next signin;
optionally implement a periodic background sweep that queries users with
needsOrgProvisioning to re-run handleSignupOrganization and clear the flag on
success.
PierreBrisorgueil
dismissed
coderabbitai[bot]’s stale review
The finding requests a durable retry mechanism (needsOrgProvisioning flag + signin retry + background sweep). That is a valid product enhancement but is out of scope for this PR, which is a verbatim promote-up port of trawl's existing production behavior (best-effort try/catch, no retry). Trawl has run this pattern since #1251 without issue — handleSignupOrganization is idempotent so a manual retry or re-verify already recovers gracefully. A proper retry mechanism would require User schema changes + signin/token handler changes + a background cron and will be tracked as a separate issue if the team decides to add it.
Summary
Promote-up from trawl_node (trawl#1317 + alignment plan
2026-05-30-trawl-devkit-perfect-alignment.mdE.1 — Task 4 of2026-06-01-trawl-promote-up-followups.md).AuthOrganizationService.handleSignupOrganization(user)intoverifyEmailafter theemailVerifiedflag is persisted to DB and marked on the local objecttry/catch— org-setup failure must NEVER fail email verification (regression risk if wrapper is removed; second test proves it)user.emailVerified = truemarked on the local object before the call sohandleSignupOrganizationsees the correct flag stateWhy this is needed
When a downstream uses a real mailer (SMTP configured), org provisioning is deferred at signup time (the user hasn't verified yet). Without this call, users who go through the email-verification flow never get their org/grant provisioned. The
signuphandler already callshandleSignupOrganization, but that path short-circuits when mailer is active —verifyEmailis the completion point.handleSignupOrganizationis idempotent (convergence guard: re-verify edge cases do not double-credit the grant, and if the org already exists it converges cleanly).Safety
Best-effort
try/catch— an org-setup error (transient DB, re-verify edge case) logs awarnand lets email verification succeed. This is the identical pattern used in trawl production since trawl#1251.Files changed
modules/auth/controllers/auth.controller.js— 15 lines:user.emailVerified = truemark +handleSignupOrganizationtry/catch block inverifyEmailmodules/auth/tests/auth.verifyEmail.signup-org.unit.tests.js(new) — 2 unit tests: (1) handler called withemailVerified=trueuser; (2) org failure does NOT crashverifyEmailPre-push gate
/critical-review --via deepseek: OK — 0 critical/high/medium, 1 low (cosmetic log field, matches trawl reference verbatim). Verdict: proceed.Closes #3762
Summary by CodeRabbit
New Features
Tests