Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new LogoutPage component that calls logOut on mount and shows a loading state; changes welcome-redirect to send invalid-session users to /logout; replaces sign-in hook with a database user.create.after postSignUp flow that provisions new users; bumps version to v2.21.54. Changes
Sequence DiagramsequenceDiagram
participant Client
participant AuthLayer as Auth Layer / DB Hooks
participant External as External Services (Loops/Resend/Dub)
participant DB
Client->>AuthLayer: Sign Up completes (DB create)
AuthLayer->>DB: databaseHooks.user.create.after triggers postSignUp
AuthLayer->>External: Resolve account provider (by userId) and create contacts
AuthLayer->>External: Track Dub sign-up
par Parallel async tasks
AuthLayer->>DB: handlePendingPremiumInvite
AuthLayer->>DB: handleReferralOnSignUp
end
External-->>AuthLayer: Service responses / errors
DB-->>AuthLayer: Async task results
AuthLayer-->>Client: Provisioning finished (or errors logged)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
| if (!user) redirect("/login"); | ||
| // Session exists but user doesn't - invalid state, log out | ||
| if (!user) redirect("/logout"); | ||
| if (searchParams.force) redirect("/onboarding"); |
There was a problem hiding this comment.
searchParams.force is a string, so if (searchParams.force) treats "false" as truthy and redirects. Consider coercing to a boolean (e.g., compare to "true") before branching.
| if (searchParams.force) redirect("/onboarding"); | |
| if (searchParams.force === "true") redirect("/onboarding"); |
🚀 Reply to ask Macroscope to explain or update this suggestion.
👍 Helpful? React to give us feedback.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/utils/auth.ts (1)
163-174: Consider passing provider to postSignUp to avoid extra database query.The
loops()function fetches the account provider separately from the database. Since this information might be available in the calling context (the database hook receives the created user), consider passing the provider as a parameter topostSignUpto eliminate this extra query.This would require updating the database hook to fetch and pass the account provider:
// In the database hook (lines 116-117) const account = await prisma.account.findFirst({ where: { userId: user.id }, select: { provider: true }, }); await postSignUp({ id: user.id, email: user.email, name: user.name, image: user.image, provider: account?.provider, }).catch(...)Then update
postSignUpto acceptprovideras an optional parameter and use it directly instead of fetching it.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/utils/auth.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Use@/path aliases for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Follow consistent naming conventions using PascalCase for components
Centralize shared types in dedicated type filesImport specific lodash functions rather than entire lodash library to minimize bundle size (e.g.,
import groupBy from 'lodash/groupBy')
Files:
apps/web/utils/auth.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)
**/*.{ts,tsx}: For API GET requests to server, use theswrpackage
Useresult?.serverErrorwithtoastErrorfrom@/components/Toastfor error handling in async operations
**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls
**/*.{ts,tsx}: For early access feature flags, create hooks using the naming conventionuse[FeatureName]Enabledthat return a boolean fromuseFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming conventionuse[FeatureName]Variantthat define variant types, useuseFeatureFlagVariantKey()with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g.,inbox-cleaner,pricing-options-2)
Always define types for A/B test variant flags (e.g.,type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the!postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Useas constinstead of literal types and type annotations
Use eitherT[]orArray<T>consistently
Initialize each enum member value explicitly
Useexport typefor types
Use `impo...
Files:
apps/web/utils/auth.ts
**/{server,api,actions,utils}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/{server,api,actions,utils}/**/*.ts: UsecreateScopedLoggerfrom "@/utils/logger" for logging in backend code
Add thecreateScopedLoggerinstantiation at the top of the file with an appropriate scope name
Use.with()method to attach context variables only within specific functions, not on global loggers
For large functions with reused variables, usecreateScopedLogger().with()to attach context once and reuse the logger without passing variables repeatedly
Files:
apps/web/utils/auth.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)
Always import Prisma enums from
@/generated/prisma/enumsinstead of@/generated/prisma/clientto avoid Next.js bundling errors in client componentsImport Prisma using the project's centralized utility:
import prisma from '@/utils/prisma'
Files:
apps/web/utils/auth.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma'sselectoption. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. AllfindUnique/findFirstcalls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
AllfindManyqueries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g.,emailAccount: { id: emailAccountId }) to validate ownership
Files:
apps/web/utils/auth.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Usenext/imagepackage for images
For API GET requests to server, use theswrpackage with hooks likeuseSWRto fetch data
For text inputs, use theInputcomponent withregisterPropsfor form integration and error handling
Files:
apps/web/utils/auth.ts
**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Implement responsive design with Tailwind CSS using a mobile-first approach
Files:
apps/web/utils/auth.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useaccessKeyattribute on any HTML element
Don't setaria-hidden="true"on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like<marquee>or<blink>
Only use thescopeprop on<th>elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assigntabIndexto non-interactive HTML elements
Don't use positive integers fortabIndexproperty
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include atitleelement for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
AssigntabIndexto non-interactive HTML elements witharia-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include atypeattribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden)
Always include alangattribute on the html element
Always include atitleattribute for iframe elements
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...
Files:
apps/web/utils/auth.ts
!(pages/_document).{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't use the next/head module in pages/_document.js on Next.js projects
Files:
apps/web/utils/auth.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g.,import groupBy from 'lodash/groupBy')
Files:
apps/web/utils/auth.ts
🧠 Learnings (6)
📚 Learning: 2025-11-25T14:37:35.343Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-11-25T14:37:35.343Z
Learning: Applies to apps/web/hooks/use*.ts : Create dedicated hooks for specific data types (e.g., `useAccounts`, `useLabels`) to wrap `useSWR` for individual API endpoints
Applied to files:
apps/web/utils/auth.ts
📚 Learning: 2025-11-25T14:39:27.909Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:27.909Z
Learning: Applies to **/app/api/**/*.ts : Use `withAuth` middleware for user-level operations such as user settings, API keys, and referrals that use only `userId`
Applied to files:
apps/web/utils/auth.ts
📚 Learning: 2025-11-25T14:37:11.434Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-11-25T14:37:11.434Z
Learning: Applies to **/app/**/route.ts : Always wrap GET API route handlers with `withAuth` or `withEmailAccount` middleware for consistent error handling and authentication in Next.js App Router
Applied to files:
apps/web/utils/auth.ts
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account (reading/writing emails, rules, schedules, etc.) - provides `emailAccountId`, `userId`, and `email` in `request.auth`
Applied to files:
apps/web/utils/auth.ts
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `withAuth` middleware for user-level operations (user settings, API keys, referrals) - provides only `userId` in `request.auth`
Applied to files:
apps/web/utils/auth.ts
📚 Learning: 2025-11-25T14:37:30.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-11-25T14:37:30.660Z
Learning: Applies to apps/web/hooks/use*.ts : Create dedicated hooks for specific data types (e.g., `useAccounts`, `useLabels`) that wrap `useSWR`, handle the API endpoint URL, and return data, loading state, error state, and the `mutate` function
Applied to files:
apps/web/utils/auth.ts
🧬 Code graph analysis (1)
apps/web/utils/auth.ts (1)
apps/web/utils/error.ts (1)
captureException(36-48)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Review for correctness
🔇 Additional comments (4)
apps/web/utils/auth.ts (4)
6-6: LGTM!The import cleanup appropriately removes the unused
Usertype, as the database hook now receives the user object directly with the necessary properties.
114-128: Improved architecture for post-signup handling.The database hook approach is superior to the previous events.signIn pattern because it:
- Runs specifically after user creation rather than every sign-in
- Includes proper error handling that prevents signup failures if side effects fail
- Provides clear logging and exception tracking
The implementation correctly passes the necessary user properties and handles errors gracefully.
211-217: Excellent use of Promise.all for parallel execution.The parallel execution of post-signup operations (Loops, Resend, Dub, premium invites, and referrals) is well-designed. Each operation has independent error handling, ensuring that a failure in one doesn't block the others.
220-259: Robust implementation with comprehensive error handling.The updated
handlePendingPremiumInvitefunction demonstrates good practices:
- Comprehensive try/catch with detailed logging
- Clear info-level logging for the happy path
- Proper error context in
captureException- Safe array filtering that won't fail if email isn't in the list
The implementation correctly handles the premium invite flow without blocking user signup on failures.
| const loops = async () => { | ||
| const account = await prisma.account | ||
| .findFirst({ | ||
| where: { userId }, | ||
| select: { provider: true }, | ||
| }) | ||
| .catch((error) => { | ||
| logger.error("Error finding account", { | ||
| userId, | ||
| error, | ||
| }); | ||
|
|
||
| await createLoopsContact( | ||
| user.email, | ||
| user.name?.split(" ")?.[0], | ||
| account?.provider, | ||
| ).catch((error) => { | ||
| const alreadyExists = | ||
| error instanceof Error && error.message.includes("409"); | ||
| if (!alreadyExists) { | ||
| logger.error("Error creating Loops contact", { | ||
| email: user.email, | ||
| error, | ||
| }); | ||
| captureException(error, undefined, user.email); | ||
| } | ||
| captureException(error, undefined, email); | ||
| }); | ||
| }; | ||
|
|
||
| const resend = createResendContact({ email: user.email }).catch((error) => { | ||
| logger.error("Error creating Resend contact", { | ||
| email: user.email, | ||
| error, | ||
| }); | ||
| captureException(error, undefined, user.email); | ||
| await createLoopsContact( | ||
| email, | ||
| name?.split(" ")?.[0], | ||
| account?.provider, | ||
| ).catch((error) => { | ||
| const alreadyExists = | ||
| error instanceof Error && error.message.includes("409"); | ||
| if (!alreadyExists) { | ||
| logger.error("Error creating Loops contact", { | ||
| email, | ||
| error, | ||
| }); | ||
| captureException(error, undefined, email); | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Fragile error handling for duplicate contact detection.
The error check on lines 181-183 uses error.message.includes("409") to detect duplicate contacts, which is fragile and could break if the error message format changes.
Consider checking the HTTP status code directly if the error object provides it:
- const alreadyExists =
- error instanceof Error && error.message.includes("409");
+ const alreadyExists =
+ (error as any)?.response?.status === 409 ||
+ (error instanceof Error && error.message.includes("409"));Alternatively, check the Loops API client documentation to see if it throws a specific error type for conflicts.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/utils/auth.ts around lines 162 to 191, the duplicate-contact
detection is fragile because it checks error.message.includes("409"); instead,
inspect the error object for a concrete status/code or specific error type: if
the Loops client or HTTP client exposes error.status or error.response?.status,
check that === 409 (or use the client’s Conflict error class), or import and use
the Loops-specific error type/utility to detect conflicts; update the catch to
branch on that status/type and only log/capture non-conflict errors.
Fix post sign up handling by moving side effects to
betterAuthConfig.databaseHooks.user.create.afterand add/logoutpage redirecting to/loginMove post-signup side effects into a
user.create.afterhook and introduce a dedicatedpostSignUpfunction; add a/logoutclient page that logs out then redirects to/login; update the welcome redirect to send null-user sessions to/logout.📍Where to Start
Start with the
betterAuthConfigchanges andpostSignUpflow in auth.ts. Then review the logout flow in page.tsx and the redirect change in page.tsx.Macroscope summarized f10d305.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.