feat(organizations): always-create workspace + suggestedJoin hint (drop onboarding limbo)#3680
Merged
PierreBrisorgueil merged 9 commits intoMay 19, 2026
Merged
Conversation
…op autoCreate footgun
Spec D5 — every signup provisions an active workspace. autoCreate config flag is a
deprecated no-op; no path returns organizationSetupRequired or pendingJoin.
- handleSignupOrganization: delete autoCreate:false footgun branch entirely; all enabled
paths call createOrganizationForUser returning {organization, membership}.
- Domain-match branch: drop createJoinRequest+pendingJoin; return suggestedJoin:
{orgId,orgName} name-only hint alongside the newly created workspace.
- Use A1 isPublicDomain for the public-domain gate (hardcoded list + config overrides).
- Naming: domainMatching+corporate domain → domain-based name; all other cases
(domainMatching off, public domain) → "{firstName}'s organization".
- signupGrant: credited inside createOrganizationForUser, once per real new org,
never double-credited. Preserved exactly from pre-A2.
- Auth controller: no change needed — uses ||false/||null fallbacks on removed keys.
- Test updates (required 0-failure): emailVerification, domainJoin E2E, auth signup
integration + auth E2E updated to assert new always-create contract.
- New unit test: organizations.service.signup.unit.tests.js — all config combos,
suggestedJoin shape (name-only), signupGrant credited once, createJoinRequest never.
…lation - I1: remove unused MembershipService import and dead sanitizeOrg helper (both orphaned by A2); file is now lint-clean (no-unused-vars: 0) - I2: replace clearAllMocks() with resetAllMocks() + explicit defaults in top-level beforeEach; prevents mockReturnValue leaking across describe blocks - m1: hoist triple `orgConfig.domainMatching && !domainIsPublic` into const isCorporateDomain; used at all 3 call sites, behavior identical - m2: reword misleading e2e comment on suggestedOrganization:null to state actual controller contract and reference A2b for wiring
…canonical normalizeEmailDomain replace extractDomain(user.email) with normalizeEmailDomain (A1) at the two signup sites in handleSignupOrganization: the repository list query (read) and the domain passed to createOrganizationForUser (write). both now use the single canonical lowercased/trimmed/null-safe path from organizations.domain.js. guard isCorporateDomain against null domain (malformed email → normalizeEmailDomain returns null → treated as non-corporate, falls through to personal workspace). extractDomain kept as exported public API for non-signup callers; not modified. tests (A3): mixed-case write normalization, case-insensitive match, subdomain non-match (eu.acme.com ≠ acme.com), public-domain gate with mixed-case email.
…main guard Remove extractDomain (def + JSDoc + export entry) — fully superseded by normalizeEmailDomain since A3; 0 callers in repo. Drop stale "kept for non-signup callers" comment (was factually wrong). Simplify domainIsPublic to `isPublicDomain(domain) || publicDomains.includes(domain ?? '')` — normalizeEmailDomain guarantees a lowercased string or null, so the redundant ?. chain is gone. Fix stale RED label in A3 test to describe the canonical-normalization contract it actually asserts.
…1 / A4)
Add convergence guard in handleSignupOrganization: if the user already holds
an active membership (partial-failure retry scenario), return the existing
org+membership without calling createOrganizationForUser — preventing duplicate
workspaces and double-crediting signupGrant on retried signups.
Uses MembershipRepository.findOne({ userId, status: ACTIVE }) — same canonical
read as autoSetCurrentOrganization. Guard fires after the email-verification
early-return and before Case 1/Case 2 always-create paths. signupGrant stays
once-only because it lives inside createOrganizationForUser, which is skipped.
…rif ordering test
Controller now serializes `suggestedJoin: { orgId, orgName } | null`
from orgResult into the signup response body alongside the existing keys.
`suggestedOrganization` kept as always-null with deprecation comment
(additive — breaking removal deferred to next release).
TDD: two integration tests (shape assert + null path), auth 1579 / orgs 1469 green.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3680 +/- ##
==========================================
+ Coverage 89.54% 89.60% +0.06%
==========================================
Files 138 139 +1
Lines 4733 4734 +1
Branches 1472 1481 +9
==========================================
+ Hits 4238 4242 +4
+ Misses 388 385 -3
Partials 107 107
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:
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis PR refactors signup organization provisioning to implement "Spec D5 — always-create." Every successful signup now provisions a workspace for the user, regardless of ChangesAlways-Create Signup Organization Provisioning
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthController
participant HandleSignup
participant OrgRepository
participant MembershipRepository
participant BillingService
User->>AuthController: POST /signup
AuthController->>HandleSignup: handleSignupOrganization(user)
HandleSignup->>MembershipRepository: findOne(userId, ACTIVE)
alt Existing Active Membership
MembershipRepository-->>HandleSignup: membership found
HandleSignup-->>AuthController: return existing org+membership
else New Signup
MembershipRepository-->>HandleSignup: no membership
HandleSignup->>HandleSignup: normalizeEmailDomain(email)
HandleSignup->>HandleSignup: isPublicDomain(domain)
HandleSignup->>OrgRepository: findByDomain(domain)
alt Corporate domain + org exists
OrgRepository-->>HandleSignup: org found
HandleSignup->>HandleSignup: return suggestedJoin hint
else Create new org
HandleSignup->>OrgRepository: create organization
HandleSignup->>MembershipRepository: create membership
HandleSignup->>BillingService: grantOnSignup
end
HandleSignup-->>AuthController: return org+membership+suggestedJoin
end
AuthController-->>User: signup response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the organizations/signup flow to eliminate the “onboarding limbo” path by always provisioning a workspace on signup, adds a sanitized suggestedJoin hint for corporate-domain matches, and centralizes email-domain normalization/public-domain detection.
Changes:
- Make
handleSignupOrganizationalways create an active organization + membership (with an idempotent convergence guard for retries) and return an optionalsuggestedJoinhint. - Add
organizations.domain.jsas the canonical domain normalization + public-domain source of truth. - Update auth controller + integration/E2E/unit tests to reflect the new always-create +
suggestedJoincontract.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/organizations/tests/organizations.service.signup.unit.tests.js | Adds unit coverage for always-create behavior, suggestedJoin shape, normalization, and idempotent convergence. |
| modules/organizations/tests/organizations.emailVerification.unit.tests.js | Updates expectations to reflect removal of organizationSetupRequired/pendingJoin keys from service return. |
| modules/organizations/tests/organizations.domainJoin.e2e.tests.js | Updates E2E expectations: no pending join requests; member gets their own workspace. |
| modules/organizations/tests/organizations.domain.unit.tests.js | Adds unit tests for normalizeEmailDomain and isPublicDomain. |
| modules/organizations/services/organizations.service.js | Implements always-create signup provisioning, suggestedJoin hinting, domain normalization usage, and retry-safe convergence guard. |
| modules/organizations/services/organizations.domain.js | Introduces domain parsing/normalization + public-domain detection helpers. |
| modules/auth/tests/auth.signup.organization.integration.tests.js | Updates integration expectations for always-create + suggestedJoin, and deprecates autoCreate:false behavior. |
| modules/auth/tests/auth.e2e.tests.js | Updates auth E2E to validate no pending join requests and active membership on new org. |
| modules/auth/controllers/auth.controller.js | Serializes suggestedJoin into the signup HTTP response (keeps deprecated suggestedOrganization). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
modules/auth/controllers/auth.controller.js (2)
135-152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the project response envelope helper instead of raw
res.status().json().The signup controller still constructs a raw JSON response directly. This breaks the repo’s standardized API envelope convention and should be routed through
responses.success(...).Based on learnings: "Enforce project-wide API envelope: all backend endpoints should wrap responses in the { type, message, data } envelope via responses.success(res, message)(data). Do not flag or accept raw res.status(200).json(...) responses in modules/auth/controllers/auth.controller.js, as they break consistency."
🤖 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 135 - 152, Replace the raw res.status(200).cookie(...).json(...) return with the project's response envelope helper: set the cookie first via res.cookie('TOKEN', token, tokenCookieOptions) and then call responses.success(res, 'Sign up')(...) passing the exact payload object (user, tokenExpiresIn, organization, joined, pendingJoin, abilities, organizationSetupRequired, emailVerificationRequired, suggestedOrganization, suggestedJoin, type: 'success', message: 'Sign up') so the endpoint uses the standardized {type,message,data} envelope; locate this change around the return in the signup handler in modules/auth/controllers/auth.controller.js where token, tokenCookieOptions and orgResult are used.
141-146:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDrop legacy
pendingJoin/organizationSetupRequiredfrom signup payload.At Line 143 and Line 145, the response still emits deprecated fields that D5 says should no longer be returned. Keeping them prolongs mixed contracts and client ambiguity.
💡 Suggested fix
.json({ user, tokenExpiresIn: Date.now() + config.jwt.expiresIn * 1000, organization: orgResult.organization || null, joined: orgResult.joined || false, - pendingJoin: orgResult.pendingJoin || false, abilities: orgResult.abilities || [], - organizationSetupRequired: orgResult.organizationSetupRequired || false, emailVerificationRequired: orgResult.emailVerificationRequired || false, // deprecated: always null since always-create (A2); superseded by suggestedJoin — remove next release suggestedOrganization: orgResult.suggestedOrganization || null, suggestedJoin: orgResult.suggestedJoin || null,🤖 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 141 - 146, Remove the deprecated fields pendingJoin and organizationSetupRequired from the signup/response payload in auth.controller.js — locate the object that builds the response (the block that sets organization: orgResult.organization, joined: orgResult.joined, pendingJoin: orgResult.pendingJoin, abilities: orgResult.abilities, organizationSetupRequired: orgResult.organizationSetupRequired, emailVerificationRequired: orgResult.emailVerificationRequired) and delete the pendingJoin and organizationSetupRequired properties so only current fields (e.g., organization, joined, abilities, emailVerificationRequired) are returned; run/update any tests or client expectations that referenced pendingJoin or organizationSetupRequired.
🤖 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/organizations/services/organizations.domain.js`:
- Around line 20-23: The domain-extraction logic currently uses indexOf('@')
which accepts malformed emails with multiple '@' (e.g., "a@b@acme.com"); update
the normalization to first verify there's exactly one '@' (e.g., ensure
email.indexOf('@') === email.lastIndexOf('@') and index !== -1) before slicing,
then trim/lowercase and validate domain as before (use the existing variables
email, at, domain) so multiple-`@` inputs return null.
In `@modules/organizations/services/organizations.service.js`:
- Around line 158-162: The helper function buildResult is missing a JSDoc
header; add a JSDoc block above the const buildResult declaration documenting
the parameters and return value: include `@param` {Object} organization, `@param`
{Object} membership, and mention that the function uses the outer-scope user and
returns a Promise resolving to an object with organization, membership, and
abilities (the result of serializeAbilities(await policy.defineAbilityFor(user,
membership))) — reference buildResult, policy.defineAbilityFor,
serializeAbilities and user in the description.
In `@modules/organizations/tests/organizations.domainJoin.e2e.tests.js`:
- Around line 142-144: The test currently uses permissive falsy assertions for
removed response fields; replace the toBeFalsy() checks with explicit undefined
assertions so the contract failure is caught — i.e., change
expect(result.body.pendingJoin).toBeFalsy() and
expect(result.body.joined).toBeFalsy() to
expect(result.body.pendingJoin).toBeUndefined() and
expect(result.body.joined).toBeUndefined(), referencing the result.body object
and the pendingJoin / joined properties in the domain join e2e test.
- Around line 155-159: Add explicit assertions for the new suggestedJoin
payload: after the existing
expect(result.body.suggestedOrganization).toBeNull(); add checks that
result.body.suggestedJoin is defined and has the expected "name-only" hint
pointing to the owner's org (e.g.
expect(result.body.suggestedJoin).toBeDefined();
expect(result.body.suggestedJoin.type).toBe('name-only');
expect(result.body.suggestedJoin.name).toBe(ownerOrg.name) or compare against
the expected owner name used in this test). Ensure you reference
result.body.suggestedJoin and the test's owner/org fixture (ownerOrg or
expectedOwnerName) so the assertion verifies both presence and correct value.
---
Outside diff comments:
In `@modules/auth/controllers/auth.controller.js`:
- Around line 135-152: Replace the raw res.status(200).cookie(...).json(...)
return with the project's response envelope helper: set the cookie first via
res.cookie('TOKEN', token, tokenCookieOptions) and then call
responses.success(res, 'Sign up')(...) passing the exact payload object (user,
tokenExpiresIn, organization, joined, pendingJoin, abilities,
organizationSetupRequired, emailVerificationRequired, suggestedOrganization,
suggestedJoin, type: 'success', message: 'Sign up') so the endpoint uses the
standardized {type,message,data} envelope; locate this change around the return
in the signup handler in modules/auth/controllers/auth.controller.js where
token, tokenCookieOptions and orgResult are used.
- Around line 141-146: Remove the deprecated fields pendingJoin and
organizationSetupRequired from the signup/response payload in auth.controller.js
— locate the object that builds the response (the block that sets organization:
orgResult.organization, joined: orgResult.joined, pendingJoin:
orgResult.pendingJoin, abilities: orgResult.abilities,
organizationSetupRequired: orgResult.organizationSetupRequired,
emailVerificationRequired: orgResult.emailVerificationRequired) and delete the
pendingJoin and organizationSetupRequired properties so only current fields
(e.g., organization, joined, abilities, emailVerificationRequired) are returned;
run/update any tests or client expectations that referenced pendingJoin or
organizationSetupRequired.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f5925ef7-8b87-4b20-90ae-1cc22a064ed7
📒 Files selected for processing (9)
modules/auth/controllers/auth.controller.jsmodules/auth/tests/auth.e2e.tests.jsmodules/auth/tests/auth.signup.organization.integration.tests.jsmodules/organizations/services/organizations.domain.jsmodules/organizations/services/organizations.service.jsmodules/organizations/tests/organizations.domain.unit.tests.jsmodules/organizations/tests/organizations.domainJoin.e2e.tests.jsmodules/organizations/tests/organizations.emailVerification.unit.tests.jsmodules/organizations/tests/organizations.service.signup.unit.tests.js
- domain: reject multiple-@ malformed emails in normalizeEmailDomain - service: JSDoc for buildResult @param/@returns (CR+Copilot) - service: update @returns to reflect null org/membership on mailer path - e2e: add explicit suggestedJoin assertion on domain-match signup path - e2e: fix stale A2b comment (now shipped in this PR) - unit: remove stale "RED" test-driven header comment - unit: cover multiple-@ rejection in domain tests
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.
What
PR (a) of the Devkit onboarding/billing/UI-decoupling effort (PR(b) #4170 + PR(c) #4175 already merged on Vue). Removes corporate-domain onboarding limbo on the Node side.
autoCreatefootgun —config.organizations.autoCreateis now a documented deprecated no-op; every signup always provisions an active workspace regardless of config/domain.organizationSetupRequired/pendingJoinare never returned by the signup path.suggestedJoinhint — corporate non-public domain matching an existing org → returns{ orgId, orgName }(sanitized, no membership/size/domain leak) alongside the user's own always-created workspace. Soft signal, never a gate.organizations.domain.js(normalizeEmailDomain+isPublicDomain); single source of truth; exact-match (no subdomain recursion:user@eu.acme.com≠acme.com).signupGrant).suggestedJoinserialized in the signup HTTP response (auth.controller.js) — the PR(a)↔PR(d) contract for the onboarding banner. DeprecatedsuggestedOrganizationkept one release with a removal comment.createOrganizationForUser, never on the idempotent converge path.Verification
npm run lint: 0 errors. Full suite: 99 suites / 1459 tests, 0 failures.feat-billing-plan-signup-grantandfix-checkout-502carry identical changes that removeBillingSignupGrantService/the grant call fromcreateOrganizationForUser(grant relocation). Neither has an open PR. Conflict is textual + semantic (grant relocation, not data-model). Decision: PR(a) merges first; whoever rebases those branches must re-verify the signupGrant "exactly-once" invariant against the relocated grant. Documented for the rebaser.8 commits (
6f7f0f8d→7132647f).Summary by CodeRabbit
Release Notes
New Features
suggestedJoinhint during signup to recommend existing organizations matching user's email domain.Bug Fixes
Deprecations
suggestedOrganizationfield is deprecated in signup responses.