fix: User onboarding and organization setup logic #1215
Conversation
WalkthroughIntroduces transactional user creation and org initialization in the Drizzle auth adapter, centralizes onboarding organization resolution and naming, adds a new "download" onboarding step, and guards organization membership insertion when accepting invites. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AuthAdapter as Auth Adapter
participant DB as Database
participant Stripe as Stripe API
User->>AuthAdapter: Create user
activate AuthAdapter
AuthAdapter->>DB: Begin transaction
AuthAdapter->>DB: Insert user (use generated userId)
AuthAdapter->>DB: Check organizationInvites
alt No pending invite
AuthAdapter->>DB: Insert organization
AuthAdapter->>DB: Insert organizationMembers (owner)
AuthAdapter->>DB: Update user active/default org IDs
else Invite exists
AuthAdapter->>DB: Link user to invited org
end
AuthAdapter->>Stripe: Update / fetch subscription metadata
alt Subscription present
AuthAdapter->>DB: Update user Stripe fields
end
AuthAdapter->>DB: Commit transaction
AuthAdapter->>DB: Query user by id
AuthAdapter-->>User: Return created user
deactivate AuthAdapter
sequenceDiagram
actor User
participant Onboarding as Onboarding Flow
participant DB as Database
participant S3 as S3
User->>Onboarding: Submit onboarding data
Onboarding->>Onboarding: Trim firstName/lastName
Onboarding->>DB: Update user name fields
Onboarding->>DB: Resolve organization (update existing or create new)
Onboarding->>DB: Ensure organizationMembers owner entry
Onboarding->>DB: Update user's active/default org IDs
alt icon provided
Onboarding->>S3: Upload icon using finalOrganizationId
Onboarding->>DB: Update org icon reference
end
Onboarding-->>User: Return { organizationId: finalOrganizationId }
sequenceDiagram
actor User
participant InviteAPI as Invite Accept API
participant DB as Database
User->>InviteAPI: Accept invite
InviteAPI->>DB: Validate invite and email
InviteAPI->>DB: Query organizationMembers for existing membership
alt membership exists
Note right of DB: Skip insert
else no membership
InviteAPI->>DB: Insert organizationMembers
end
InviteAPI->>DB: Update user (defaultOrgId, onboardingSteps, activeOrganizationId, stripe fields)
InviteAPI->>DB: Delete invite
InviteAPI-->>User: Success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/database/auth/drizzle-adapter.ts (1)
329-345: Verification token can be “used” with the wrong identifier.You select by token only, then delete by token+identifier. If identifier doesn’t match, delete is a no‑op but you still return the row. Select by both fields up front.
Apply this diff:
- const rows = await db - .select() - .from(verificationTokens) - .where(eq(verificationTokens.token, token)) - .limit(1); + const rows = await db + .select() + .from(verificationTokens) + .where( + and( + eq(verificationTokens.token, token), + eq(verificationTokens.identifier, identifier), + ), + ) + .limit(1); const row = rows[0]; if (!row) return null; await db .delete(verificationTokens) .where( and( eq(verificationTokens.token, token), eq(verificationTokens.identifier, identifier), ), ); return row;packages/web-backend/src/Users/UsersOnboarding.ts (2)
164-176: Allowing SVG uploads can enable XSS if rendered unsanitized.Unless you sanitize SVGs at upload or render time, restrict to safe raster formats (png/jpg/webp).
Suggested change:
- ["image/svg+xml", "svg"], + // Consider removing SVG or sanitize it server-side before use
283-304: Skip path doesn’t create an organization if missing; update does nothing.When
existingOrgis absent, you still attempt an update. Create a new org and set it active/default; otherwise rename existing only when orgSetup wasn’t already completed.Apply this diff:
- if (!existingOrg || !user.onboardingSteps?.organizationSetup) { - await tx - .update(Db.organizations) - .set({ - name: orgName, - }) - .where( - Dz.eq( - Db.organizations.id, - currentUser.activeOrganizationId, - ), - ); - } + if (!existingOrg) { + const newOrgId = Organisation.OrganisationId.make(nanoId()); + await tx.insert(Db.organizations).values({ + id: newOrgId, + ownerId: currentUser.id, + name: orgName, + }); + await tx.insert(Db.organizationMembers).values({ + id: nanoId(), + organizationId: newOrgId, + userId: currentUser.id, + role: "owner", + }); + await tx + .update(Db.users) + .set({ + activeOrganizationId: newOrgId, + defaultOrgId: newOrgId, + }) + .where(Dz.eq(Db.users.id, currentUser.id)); + } else if (!user.onboardingSteps?.organizationSetup) { + await tx + .update(Db.organizations) + .set({ name: orgName }) + .where(Dz.eq(Db.organizations.id, existingOrg.id)); + }
🧹 Nitpick comments (5)
packages/database/auth/drizzle-adapter.ts (3)
199-205: Fix branding: accounts.id shouldn’t be a User.UserId; brand userId instead.Use a plain nanoId for accounts.id, and brand the userId consistently.
Apply this diff:
- id: User.UserId.make(nanoId()), - userId: account.userId, + id: nanoId(), + userId: User.UserId.make(account.userId),
20-20: Avoidanyand rely on Adapter/Domain types.Replace
anywith proper NextAuth/Domain types (e.g., AdapterUser, AdapterAccount, AdapterSession, VerificationToken), or omit annotations to inherit from Adapter interface.Example:
import type { AdapterUser, AdapterAccount, AdapterSession, VerificationToken, } from "next-auth/adapters"; // async createUser(userData: AdapterUser) { ... } // async linkAccount(account: AdapterAccount) { ... } // async updateSession(session: AdapterSession) { ... } // async createVerificationToken(verificationToken: VerificationToken) { ... } // async unlinkAccount({ providerAccountId, provider }: AdapterAccount) { ... } // async useVerificationToken(args: VerificationToken) { ... }As per coding guidelines
Also applies to: 198-198, 214-214, 268-268, 285-285, 328-328
118-129: Pick the newest subscription explicitly.Stripe usually returns newest first, but it’s safer to select by max
createdto avoid relying on implicit ordering.Example:
const mostRecentSubscription = subscriptions.data .slice() .sort((a, b) => b.created - a.created)[0];packages/web-backend/src/Users/UsersOnboarding.ts (2)
87-91: Ensure organizationName is non-empty after trim.An empty string can slip through and be persisted. Validate and reject or provide a safe fallback.
Apply this diff:
- const organizationName = - data.organizationName.trim() || data.organizationName; + const organizationName = data.organizationName.trim(); + if (organizationName.length === 0) { + throw new Error("Organization name cannot be empty"); + }
94-138: Restrict renaming to org owners.When an org already exists, you rename it without confirming the user’s role. Guard updates so only owners can rename.
Example insertion before the update:
const [membership] = await tx .select({ role: Db.organizationMembers.role }) .from(Db.organizationMembers) .where( Dz.and( Dz.eq(Db.organizationMembers.organizationId, resolvedOrgId), Dz.eq(Db.organizationMembers.userId, currentUser.id), ), ) .limit(1); if (membership?.role !== "owner") { throw new Error("Only owners can rename the organization"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/database/auth/drizzle-adapter.ts(2 hunks)packages/web-backend/src/Users/UsersOnboarding.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
packages/database/auth/drizzle-adapter.tspackages/web-backend/src/Users/UsersOnboarding.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/database/auth/drizzle-adapter.tspackages/web-backend/src/Users/UsersOnboarding.ts
🧬 Code graph analysis (2)
packages/database/auth/drizzle-adapter.ts (4)
packages/web-api-contract-effect/src/index.ts (1)
User(55-62)packages/database/helpers.ts (1)
nanoId(6-9)packages/database/schema.ts (4)
users(58-117)organizationInvites(229-254)organizations(170-202)organizationMembers(205-227)packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)
packages/web-backend/src/Users/UsersOnboarding.ts (2)
packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)packages/database/helpers.ts (1)
nanoId(6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
| await tx.insert(users).values({ | ||
| id: userId, | ||
| email: userData.email, | ||
| emailVerified: userData.emailVerified, | ||
| name: userData.name, | ||
| image: userData.image, | ||
| activeOrganizationId: Organisation.OrganisationId.make(""), | ||
| }); |
There was a problem hiding this comment.
Don’t insert an empty activeOrganizationId; leave null until set.
Writing OrganisationId.make("") creates an invalid ID and risks downstream logic branching on a non-null/empty value. Omit the field on insert (or set null) and update it later in the transaction.
Apply this diff:
await tx.insert(users).values({
id: userId,
email: userData.email,
emailVerified: userData.emailVerified,
name: userData.name,
image: userData.image,
- activeOrganizationId: Organisation.OrganisationId.make(""),
+ // leave null; will be set below if/when an org is created
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await tx.insert(users).values({ | |
| id: userId, | |
| email: userData.email, | |
| emailVerified: userData.emailVerified, | |
| name: userData.name, | |
| image: userData.image, | |
| activeOrganizationId: Organisation.OrganisationId.make(""), | |
| }); | |
| await tx.insert(users).values({ | |
| id: userId, | |
| email: userData.email, | |
| emailVerified: userData.emailVerified, | |
| name: userData.name, | |
| image: userData.image, | |
| // leave null; will be set below if/when an org is created | |
| }); |
🤖 Prompt for AI Agents
In packages/database/auth/drizzle-adapter.ts around lines 23 to 30, the insert
sets activeOrganizationId to Organisation.OrganisationId.make("") which creates
an invalid non-null value; remove this field from the insert (or set it
explicitly to null) so the DB stores NULL until the real organization id is
assigned, and then update activeOrganizationId later in the same transaction
when the proper OrganisationId is available.
| const [pendingInvite] = await tx | ||
| .select({ id: organizationInvites.id }) | ||
| .from(organizationInvites) | ||
| .where( | ||
| and( | ||
| eq(organizationInvites.invitedEmail, userData.email), | ||
| eq(organizationInvites.status, "pending"), | ||
| ), | ||
| ) | ||
| .limit(1); | ||
|
|
||
| if (!pendingInvite) { | ||
| const organizationId = Organisation.OrganisationId.make(nanoId()); | ||
|
|
||
| await tx.insert(organizations).values({ | ||
| id: organizationId, | ||
| ownerId: userId, | ||
| name: "My Organization", | ||
| }); | ||
|
|
||
| await tx.insert(organizationMembers).values({ | ||
| id: nanoId(), | ||
| organizationId, | ||
| userId, | ||
| role: "owner", | ||
| }); | ||
|
|
||
| await tx | ||
| .update(users) | ||
| .set({ | ||
| activeOrganizationId: organizationId, | ||
| defaultOrgId: organizationId, | ||
| }) | ||
| .where(eq(users.id, userId)); | ||
| } |
There was a problem hiding this comment.
Users with pending invites still end up without an organization.
This reintroduces the original problem for invited users: no active/default org is set, so Instant Mode still breaks. Create a personal org regardless of pending invites (keep invite acceptance separate), or at least set a default/active org to ensure immediate usability. Also consider ignoring expired invites.
Apply this diff to always create a personal org at sign‑up:
- const [pendingInvite] = await tx
- .select({ id: organizationInvites.id })
- .from(organizationInvites)
- .where(
- and(
- eq(organizationInvites.invitedEmail, userData.email),
- eq(organizationInvites.status, "pending"),
- ),
- )
- .limit(1);
-
- if (!pendingInvite) {
- const organizationId = Organisation.OrganisationId.make(nanoId());
-
- await tx.insert(organizations).values({
- id: organizationId,
- ownerId: userId,
- name: "My Organization",
- });
-
- await tx.insert(organizationMembers).values({
- id: nanoId(),
- organizationId,
- userId,
- role: "owner",
- });
-
- await tx
- .update(users)
- .set({
- activeOrganizationId: organizationId,
- defaultOrgId: organizationId,
- })
- .where(eq(users.id, userId));
- }
+ // Always create a personal org so the new account is usable immediately.
+ const organizationId = Organisation.OrganisationId.make(nanoId());
+
+ await tx.insert(organizations).values({
+ id: organizationId,
+ ownerId: userId,
+ name: "My Organization",
+ });
+
+ await tx.insert(organizationMembers).values({
+ id: nanoId(),
+ organizationId,
+ userId,
+ role: "owner",
+ });
+
+ await tx
+ .update(users)
+ .set({
+ activeOrganizationId: organizationId,
+ defaultOrgId: organizationId,
+ })
+ .where(eq(users.id, userId));Optional: when checking invites, also filter out expired ones: add a condition like and(eq(status,"pending"), Dz.gt(expiresAt, new Date())).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [pendingInvite] = await tx | |
| .select({ id: organizationInvites.id }) | |
| .from(organizationInvites) | |
| .where( | |
| and( | |
| eq(organizationInvites.invitedEmail, userData.email), | |
| eq(organizationInvites.status, "pending"), | |
| ), | |
| ) | |
| .limit(1); | |
| if (!pendingInvite) { | |
| const organizationId = Organisation.OrganisationId.make(nanoId()); | |
| await tx.insert(organizations).values({ | |
| id: organizationId, | |
| ownerId: userId, | |
| name: "My Organization", | |
| }); | |
| await tx.insert(organizationMembers).values({ | |
| id: nanoId(), | |
| organizationId, | |
| userId, | |
| role: "owner", | |
| }); | |
| await tx | |
| .update(users) | |
| .set({ | |
| activeOrganizationId: organizationId, | |
| defaultOrgId: organizationId, | |
| }) | |
| .where(eq(users.id, userId)); | |
| } | |
| // Always create a personal org so the new account is usable immediately. | |
| const organizationId = Organisation.OrganisationId.make(nanoId()); | |
| await tx.insert(organizations).values({ | |
| id: organizationId, | |
| ownerId: userId, | |
| name: "My Organization", | |
| }); | |
| await tx.insert(organizationMembers).values({ | |
| id: nanoId(), | |
| organizationId, | |
| userId, | |
| role: "owner", | |
| }); | |
| await tx | |
| .update(users) | |
| .set({ | |
| activeOrganizationId: organizationId, | |
| defaultOrgId: organizationId, | |
| }) | |
| .where(eq(users.id, userId)); |
🤖 Prompt for AI Agents
In packages/database/auth/drizzle-adapter.ts around lines 32 to 66, the current
logic skips creating a personal organization when a pending invite exists so
invited users end up without an active/default org; always create a personal
organization at sign‑up (generate id with
Organisation.OrganisationId.make(nanoId()), insert into organizations, insert
owner membership, and update users.activeOrganizationId and users.defaultOrgId)
regardless of pendingInvite so Instant Mode works immediately, and keep invite
acceptance logic separate; optionally when querying invites add a check to
ignore expired invites (e.g., include expiresAt > new Date()).
| .where(Dz.eq(Db.organizations.id, finalOrganizationId)), | ||
| ); |
There was a problem hiding this comment.
Don’t persist pre‑signed URLs; they expire. Store the object key and derive URLs on demand.
Saving a signed URL will break later. Persist the key (e.g., fileKey) and generate signed URLs at read time.
Example:
- Store
{ iconKey: fileKey }(schema field), and return/generate signed URL to clients when needed.
🤖 Prompt for AI Agents
In packages/web-backend/src/Users/UsersOnboarding.ts around lines 190–191, the
code is persisting a pre-signed URL (which will expire); change the persistence
to save the storage object key (e.g., iconKey or fileKey) instead of the signed
URL, update the DB schema field to hold that key, and remove any code that
writes the signed URL to the record; then generate and return a fresh pre-signed
URL on read/response by calling the storage SDK with the stored key (or provide
a helper that derives signed URLs on demand).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/database/auth/drizzle-adapter.ts (2)
34-41: Don’t insert an empty activeOrganizationId; leave null/omit until set.Setting
activeOrganizationIdtoOrganisation.OrganisationId.make("")writes an invalid non‑null value and breaks logic that checks non‑null. Omit the field at insert, then set it after creating the org.Apply:
await tx.insert(users).values({ id: userId, email: userData.email, emailVerified: userData.emailVerified, name: userData.name, image: userData.image, - activeOrganizationId: Organisation.OrganisationId.make(""), + // leave null; set after org creation below });
43-46: Always create a personal org at sign‑up (even with pending invites).Early return on pending invites leaves users without an org (and with empty
activeOrganizationId), reintroducing the Instant Mode blocker this PR aims to fix. Create a personal org unconditionally; invites can be accepted later to switch orgs.- if (pendingInvite) { - return; - } - - const organizationId = Organisation.OrganisationId.make(nanoId()); + const organizationId = Organisation.OrganisationId.make(nanoId()); await tx.insert(organizations).values({ id: organizationId, ownerId: userId, name: "My Organization", }); await tx.insert(organizationMembers).values({ id: nanoId(), organizationId, userId, role: "owner", }); await tx .update(users) .set({ activeOrganizationId: organizationId, defaultOrgId: organizationId, }) .where(eq(users.id, userId));Optionally, when later accepting an invite, set
activeOrganizationId/defaultOrgIdto the invited org. This keeps new accounts usable immediately.Also applies to: 47-69
🧹 Nitpick comments (3)
apps/web/app/api/invite/accept/route.ts (1)
69-74: Onboarding steps/default org update: good; add input validation and invite status checks.
- Good: merging prior steps and setting defaultOrgId.
- Add body validation for
inviteIdand check invitestatus === "pending"and not expired (expiresAt > NOW()), else return 400/410.Example tweak:
- const { inviteId } = await request.json(); + const body = await request.json().catch(() => null); + const inviteId = body?.inviteId; + if (!inviteId || typeof inviteId !== "string") { + return NextResponse.json({ error: "Invalid inviteId" }, { status: 400 }); + } ... - const [invite] = await db() + const [invite] = await db() .select() .from(organizationInvites) - .where(eq(organizationInvites.id, inviteId)); + .where( + and( + eq(organizationInvites.id, inviteId), + eq(organizationInvites.status, "pending"), + ), + );Additionally, if the repo uses read replicas, force primary for this flow (
db().$primary) or run in a transaction to avoid read‑after‑write inconsistencies. [Based on learnings]Also applies to: 81-83
packages/database/auth/drizzle-adapter.ts (2)
21-21: Avoidanyfor userData; use NextAuth types or a narrowed interface.Improve type safety per repo guidelines.
- async createUser(userData: any) { + async createUser(userData: { + email: string; + emailVerified?: Date | null; + name?: string | null; + image?: string | null; + }) {
71-77: Force primary reads after transactional writes (or keep withintx).Post‑transaction selects may hit a replica and miss fresh data if replicas are configured. Use
db.$primary.select()for the immediate reads, or move reads into the same transaction.- const rows = await db + const rows = await db.$primary .select() .from(users) .where(eq(users.id, userId)) .limit(1); ... - const [updatedRow] = await db + const [updatedRow] = await db.$primary .select() .from(users) .where(eq(users.id, row.id)) .limit(1);Based on learnings
Also applies to: 134-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(org)/onboarding/[...steps]/page.tsx(1 hunks)apps/web/app/api/invite/accept/route.ts(2 hunks)packages/database/auth/drizzle-adapter.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/web/app/(org)/onboarding/[...steps]/page.tsxpackages/database/auth/drizzle-adapter.tsapps/web/app/api/invite/accept/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/onboarding/[...steps]/page.tsxpackages/database/auth/drizzle-adapter.tsapps/web/app/api/invite/accept/route.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/onboarding/[...steps]/page.tsxapps/web/app/api/invite/accept/route.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components
Files:
apps/web/app/(org)/onboarding/[...steps]/page.tsxapps/web/app/api/invite/accept/route.ts
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))
Files:
apps/web/app/(org)/onboarding/[...steps]/page.tsxapps/web/app/api/invite/accept/route.ts
apps/web/app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/app/api/**/*.{ts,tsx}: Prefer Server Actions for API surface; when routes are necessary, implement under app/api and export only the handler from apiToHandler(ApiLive)
Construct API routes with @effect/platform HttpApi/HttpApiBuilder, declare contracts with Schema, and only export the handler
Use HttpAuthMiddleware for required auth and provideOptionalAuth for guests; avoid duplicating session lookups
Map domain errors to transport with HttpApiError.* and keep translation exhaustive (catchTags/tapErrorCause)
Inside HttpApiBuilder.group, acquire services with Effect.gen and provide dependencies via Layer.provide instead of manual provideService
Files:
apps/web/app/api/invite/accept/route.ts
🧬 Code graph analysis (2)
packages/database/auth/drizzle-adapter.ts (2)
packages/database/schema.ts (4)
organizationInvites(229-254)users(58-117)organizations(170-202)organizationMembers(205-227)packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)
apps/web/app/api/invite/accept/route.ts (3)
packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (2)
organizationMembers(205-227)users(58-117)packages/database/helpers.ts (1)
nanoId(6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
apps/web/app/(org)/onboarding/[...steps]/page.tsx (1)
33-34: Download step rendering looks good.
| params: Promise<{ | ||
| steps: "welcome" | "organization-setup" | "custom-domain" | "invite-team"; | ||
| steps: | ||
| | "welcome" | ||
| | "organization-setup" | ||
| | "custom-domain" | ||
| | "invite-team" | ||
| | "download"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix params typing and step extraction (string[] vs Promise/union).
Next.js passes params synchronously; catch‑all [...steps] yields string[]. Current type (Promise<{ steps: union }>), then indexing steps[0], is wrong and loses type safety.
Apply:
-export default async function OnboardingStepPage({
- params,
-}: {
- params: Promise<{
- steps:
- | "welcome"
- | "organization-setup"
- | "custom-domain"
- | "invite-team"
- | "download";
- }>;
-}) {
- const step = (await params).steps[0];
+type OnboardingStep =
+ | "welcome"
+ | "organization-setup"
+ | "custom-domain"
+ | "invite-team"
+ | "download";
+
+export default async function OnboardingStepPage({
+ params,
+}: {
+ params: { steps: string[] };
+}) {
+ const step = (params.steps?.[0] ?? "welcome") as OnboardingStep;Also applies to: 20-20
🤖 Prompt for AI Agents
In apps/web/app/(org)/onboarding/[...steps]/page.tsx around lines 11-17 (and
also line 20), the params typing is incorrect: Next.js provides params
synchronously and the catch-all "[...steps]" yields string[] (or undefined), not
Promise or a union; change the function signature to accept params: { steps?:
string[] } (or the Next.js PageProps equivalent), then extract the current step
via a safe read like const step = params.steps?.[0] ?? "welcome" and narrow it
to a StepType union (e.g., type Step = "welcome" | "organization-setup" |
"custom-domain" | "invite-team" | "download"; const stepTyped =
(["welcome","organization-setup","custom-domain","invite-team","download"] as
const).includes(step as any) ? step as Step : "welcome";) so you preserve type
safety and handle missing/unknown values; update the usage at line 20
accordingly to use the new stepTyped variable.
| const [existingMembership] = await db() | ||
| .select({ id: organizationMembers.id }) | ||
| .from(organizationMembers) | ||
| .where( | ||
| and( | ||
| eq(organizationMembers.organizationId, invite.organizationId), | ||
| eq(organizationMembers.userId, user.id), | ||
| ), | ||
| ) | ||
| .limit(1); | ||
|
|
||
| if (!existingMembership) { | ||
| await db().insert(organizationMembers).values({ | ||
| id: nanoId(), | ||
| organizationId: invite.organizationId, | ||
| userId: user.id, | ||
| role: invite.role, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Eliminate TOCTOU duplicate membership risk; make insert idempotent and atomic.
The check‑then‑insert can race and create duplicate rows. Wrap accept flow in a DB transaction and enforce a unique constraint on (organizationId, userId) with an upsert.
- Schema: make (organizationId,userId) unique.
Outside this file (packages/database/schema.ts):
- userIdOrganizationIdIndex: index("user_id_organization_id_idx").on(
+ userIdOrganizationIdIndex: uniqueIndex("user_id_organization_id_uidx").on(
table.userId,
table.organizationId,
),- Route: use upsert to be idempotent (and optionally update role), or at least run inside a transaction.
Within this file:
- const [existingMembership] = await db()
- .select({ id: organizationMembers.id })
- .from(organizationMembers)
- .where(
- and(
- eq(organizationMembers.organizationId, invite.organizationId),
- eq(organizationMembers.userId, user.id),
- ),
- )
- .limit(1);
-
- if (!existingMembership) {
- await db().insert(organizationMembers).values({
- id: nanoId(),
- organizationId: invite.organizationId,
- userId: user.id,
- role: invite.role,
- });
- }
+ // within a transaction `tx`, or use db().$primary if not transacting
+ await db()
+ .insert(organizationMembers)
+ .values({
+ id: nanoId(),
+ organizationId: invite.organizationId,
+ userId: user.id,
+ role: invite.role,
+ })
+ .onDuplicateKeyUpdate({ set: { role: invite.role } });Also consider running the entire accept path in db().transaction(async tx => { ... }) and using tx for all reads/writes to keep it atomic. [Based on learnings]
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/api/invite/accept/route.ts around lines 49 to 67, the current
check-then-insert for organization membership can race and create duplicates;
change the flow to run inside a database transaction and perform an idempotent
upsert against a schema-enforced unique constraint on (organizationId, userId).
Update the DB schema (packages/database/schema.ts) to add a unique constraint on
(organizationId, userId), wrap the accept handler in db().transaction(async tx
=> { ... }) and use tx for all selects/inserts/updates, and replace the
conditional insert with an upsert (on conflict / merge) that either does nothing
or updates the role as needed so the operation is atomic and safe from TOCTOU
races.
1 participant
This PR fixes an issue where after creating an account, the user would not have an organization, preventing them from being able to use Instant Mode right away (would flag an error).
Also makes sure that if a user is being invited, it doesn't create a second org, only joins the invited one, and skips unnecessary onboarding steps.
Summary by CodeRabbit