feat(dashboard): login with google and "last used" indicator #2746
Conversation
|
|
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 Google OAuth support across the webapp: new optional env vars for Google client ID/secret; a Google strategy registration helper and integration into the authenticator; new routes to initiate and handle Google OAuth (with a redirect cookie and MFA branching); model changes to find/create/link Google-authenticated users and logging of auth conflicts; Prisma enum and migration adding AuthenticationMethod.GOOGLE; UI and route updates to surface Google login and persist last-used auth method via a new last-auth-method cookie service; replaces several redirect header objects with Headers to append multiple Set-Cookie values; minor avatar img change adding referrerPolicy="no-referrer"; and adds the remix-auth-google dependency. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (7)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/services/**/*.server.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,css,scss}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-11-27T16:26:37.432ZApplied to files:
📚 Learning: 2025-11-27T16:26:58.661ZApplied to files:
🧬 Code graph analysis (1)apps/webapp/app/models/user.server.ts (1)
⏰ 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). (22)
🔇 Additional comments (12)
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
🧹 Nitpick comments (1)
apps/webapp/app/routes/login._index/route.tsx (1)
20-41: Consider extractingGoogleIconto a shared component.The inline
GoogleIconcomponent works correctly. However, since@trigger.dev/companyiconsis already used forGitHubLightIcon, consider adding the Google icon to that package for consistency and reusability across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/webapp/app/components/UserProfilePhoto.tsx(1 hunks)apps/webapp/app/env.server.ts(1 hunks)apps/webapp/app/models/user.server.ts(4 hunks)apps/webapp/app/routes/auth.github.callback.tsx(2 hunks)apps/webapp/app/routes/auth.google.callback.tsx(1 hunks)apps/webapp/app/routes/auth.google.ts(1 hunks)apps/webapp/app/routes/login._index/route.tsx(6 hunks)apps/webapp/app/routes/magic.tsx(2 hunks)apps/webapp/app/services/auth.server.ts(2 hunks)apps/webapp/app/services/googleAuth.server.ts(1 hunks)apps/webapp/app/services/lastAuthMethod.server.ts(1 hunks)apps/webapp/package.json(1 hunks)internal-packages/database/prisma/migrations/20251204143136_add_google_auth_method/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/auth.server.tsapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/models/user.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/login._index/route.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/UserProfilePhoto.tsxapps/webapp/app/routes/auth.google.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/auth.server.tsapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/models/user.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/login._index/route.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/UserProfilePhoto.tsxapps/webapp/app/routes/auth.google.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/auth.server.tsapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/models/user.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/login._index/route.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/UserProfilePhoto.tsxapps/webapp/app/routes/auth.google.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/auth.server.tsapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/models/user.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/login._index/route.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/UserProfilePhoto.tsxapps/webapp/app/routes/auth.google.ts
apps/webapp/app/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Separate testable services from configuration files; follow the pattern of
realtimeClient.server.ts(testable service) andrealtimeClientGlobal.server.ts(configuration) in the webapp
Files:
apps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/auth.server.tsapps/webapp/app/services/lastAuthMethod.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/auth.server.tsapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/models/user.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/login._index/route.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/UserProfilePhoto.tsxapps/webapp/app/routes/auth.google.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/auth.server.tsapps/webapp/package.jsonapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/models/user.server.tsapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/login._index/route.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/components/UserProfilePhoto.tsxapps/webapp/app/routes/auth.google.ts
🧠 Learnings (8)
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/services/**/*.server.{ts,tsx} : Separate testable services from configuration files; follow the pattern of `realtimeClient.server.ts` (testable service) and `realtimeClientGlobal.server.ts` (configuration) in the webapp
Applied to files:
apps/webapp/app/services/auth.server.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: The webapp at apps/webapp is a Remix 2.1 application using Node.js v20
Applied to files:
apps/webapp/package.json
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
apps/webapp/package.jsonapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/routes/auth.google.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/models/user.server.tsinternal-packages/database/prisma/migrations/20251204143136_add_google_auth_method/migration.sql
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Leverage the PostgreSQL database through the `trigger.dev/database` Prisma 6.14.0 client in the webapp for all data access patterns
Applied to files:
apps/webapp/app/models/user.server.ts
📚 Learning: 2025-09-02T11:27:36.336Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2463
File: apps/webapp/app/routes/_app.github.callback/route.tsx:33-44
Timestamp: 2025-09-02T11:27:36.336Z
Learning: In the GitHub App installation callback flow in apps/webapp/app/routes/_app.github.callback/route.tsx, the install session cookie is not cleared after use due to interface limitations with redirectWithSuccessMessage/redirectWithErrorMessage not supporting custom headers. The maintainer accepts this design as the 1-hour cookie expiration provides sufficient protection against replay attacks.
Applied to files:
apps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/routes/magic.tsx
📚 Learning: 2025-09-02T11:18:06.602Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2463
File: apps/webapp/app/services/gitHubSession.server.ts:31-36
Timestamp: 2025-09-02T11:18:06.602Z
Learning: In the GitHub App installation flow in apps/webapp/app/services/gitHubSession.server.ts, the redirectTo parameter stored in httpOnly session cookies is considered acceptable without additional validation by the maintainer, as the httpOnly cookie provides sufficient security for this use case.
Applied to files:
apps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/routes/magic.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/env.server.ts
🧬 Code graph analysis (8)
apps/webapp/app/services/googleAuth.server.ts (3)
apps/webapp/app/services/auth.server.ts (1)
authenticator(31-31)apps/webapp/app/env.server.ts (1)
env(1261-1261)apps/webapp/app/models/user.server.ts (1)
findOrCreateUser(38-50)
apps/webapp/app/services/auth.server.ts (4)
apps/webapp/app/env.server.ts (1)
env(1261-1261)apps/webapp/app/services/gitHubAuth.server.ts (1)
addGitHubStrategy(9-54)apps/webapp/app/services/googleAuth.server.ts (1)
addGoogleStrategy(9-54)apps/webapp/app/services/emailAuth.server.tsx (1)
addEmailLinkStrategy(48-50)
apps/webapp/app/routes/auth.google.callback.tsx (8)
apps/webapp/app/routes/auth.github.callback.tsx (1)
loader(11-60)apps/webapp/app/routes/auth.google.ts (2)
loader(4-4)redirectCookie(29-32)apps/webapp/app/routes/login._index/route.tsx (1)
loader(74-120)apps/webapp/app/routes/magic.tsx (1)
loader(10-57)apps/webapp/app/utils.ts (1)
sanitizeRedirectPath(13-42)apps/webapp/app/services/auth.server.ts (1)
authenticator(31-31)apps/webapp/app/models/message.server.ts (1)
redirectWithErrorMessage(201-218)apps/webapp/app/services/lastAuthMethod.server.ts (1)
setLastAuthMethodHeader(22-24)
apps/webapp/app/models/user.server.ts (1)
apps/webapp/app/utils/email.ts (1)
assertEmailAllowed(3-13)
apps/webapp/app/routes/auth.github.callback.tsx (1)
apps/webapp/app/services/lastAuthMethod.server.ts (1)
setLastAuthMethodHeader(22-24)
apps/webapp/app/routes/login._index/route.tsx (2)
apps/webapp/app/services/lastAuthMethod.server.ts (1)
getLastAuthMethod(13-20)apps/webapp/app/services/auth.server.ts (1)
isGoogleAuthSupported(31-31)
apps/webapp/app/routes/magic.tsx (1)
apps/webapp/app/services/lastAuthMethod.server.ts (1)
setLastAuthMethodHeader(22-24)
apps/webapp/app/routes/auth.google.ts (2)
apps/webapp/app/routes/auth.google.callback.tsx (1)
loader(11-60)apps/webapp/app/routes/login._index/route.tsx (1)
loader(74-120)
⏰ 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). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
apps/webapp/app/components/UserProfilePhoto.tsx (1)
25-25: LGTM! Good privacy practice.Adding
referrerPolicy="no-referrer"prevents the Referer header from being sent when fetching avatar images. This is particularly important for OAuth providers like Google, as it prevents leaking the user's current page URL to external servers when loading avatars.apps/webapp/app/env.server.ts (1)
97-98: LGTM! Consistent with existing OAuth pattern.The Google OAuth environment variables follow the same optional pattern as the GitHub credentials, allowing the feature to be conditionally enabled based on configuration.
internal-packages/database/prisma/migrations/20251204143136_add_google_auth_method/migration.sql (1)
1-2: LGTM! Correct enum extension.The migration correctly adds the GOOGLE value to the AuthenticationMethod enum. Note that PostgreSQL's
ALTER TYPE ... ADD VALUEcannot run inside a transaction, but since this is the only statement in the migration, it should execute safely.apps/webapp/app/routes/magic.tsx (1)
42-46: LGTM! Consistent header management pattern.The implementation correctly uses the Headers API to append multiple Set-Cookie headers (session and last-auth-method). This pattern is consistently applied in both the MFA redirect path and the final redirect path.
internal-packages/database/prisma/schema.prisma (1)
94-94: LGTM! Enum updated consistently.The GOOGLE authentication method is correctly added to the AuthenticationMethod enum, aligning with the database migration.
apps/webapp/app/services/auth.server.ts (1)
5-5: LGTM! Consistent with existing OAuth pattern.The Google authentication integration follows the same pattern as the existing GitHub authentication:
- Runtime check for credential availability
- Conditional strategy registration
- Exported support flag for UI conditional rendering
This maintains consistency across OAuth providers.
Also applies to: 17-19, 25-27, 31-31
apps/webapp/app/services/googleAuth.server.ts (1)
9-54: LGTM! Consistent OAuth strategy implementation.The Google authentication strategy follows the established pattern from the GitHub strategy:
- Email validation with clear error messages
- Proper user creation/lookup flow
- Post-authentication processing
- Error logging before re-throwing
The implementation correctly integrates with the existing authentication infrastructure.
apps/webapp/package.json (1)
194-194: No action needed—remix-auth-google v2.0.0 is the latest stable version with no known security vulnerabilities.Version 2.0.0 is the latest available release on npm, and no CVEs are reported for this package. The caret constraint (^2.0.0) is appropriate and will allow patch and minor version updates. Monitor related Remix auth libraries (e.g., authkit-remix) for vulnerabilities.
apps/webapp/app/routes/auth.github.callback.tsx (1)
45-49: LGTM!The refactoring to use
Headerswith multipleappendcalls for setting both the session and last-auth-method cookies is correct and follows the proper pattern for multipleSet-Cookieheaders in Remix.Also applies to: 55-59
apps/webapp/app/routes/login._index/route.tsx (2)
74-120: LGTM!The loader correctly fetches
lastAuthMethodfrom the request cookie and includes bothshowGoogleAuthandlastAuthMethodin both response paths. The data flow is consistent.
136-188: LGTM!The login UI correctly renders provider buttons conditionally and displays the "Last used" badge based on the
lastAuthMethodvalue. The form actions properly include theredirectToquery parameter.apps/webapp/app/routes/auth.google.callback.tsx (1)
1-60: LGTM!The Google OAuth callback follows the established pattern from the GitHub callback. The implementation correctly handles:
- Redirect cookie parsing and sanitization
- Authentication with the "google" strategy
- MFA flow detection and redirect
- Multiple
Set-Cookieheaders for session and last-auth-methodapps/webapp/app/services/lastAuthMethod.server.ts (1)
13-24: LGTM!The
getLastAuthMethodandsetLastAuthMethodHeaderfunctions are well-implemented with proper type safety and explicit validation of cookie values.apps/webapp/app/models/user.server.ts (4)
24-31: LGTM!The
FindOrCreateGoogletype follows the established pattern fromFindOrCreateGithuband correctly uses a type instead of an interface per coding guidelines. The union type is properly extended.
210-229: Inconsistent behavior with GitHub user linking.When linking an existing email user to Google auth, this code updates
authenticationMethodto"GOOGLE". However,findOrCreateGithubUser(lines 120-137) does not updateauthenticationMethodwhen linking. This inconsistency could cause confusion about the user's primary auth method.Is this intentional? If so, consider adding a comment explaining why the behavior differs. If not, align the behavior:
// Link existing email account to Google auth const user = await prisma.user.update({ where: { email, }, data: { - authenticationMethod: "GOOGLE", authenticationProfile: authProfile, authenticationExtraParams: authExtraParams, avatarUrl, authIdentifier, }, });
250-270: LGTM!The upsert logic for new Google users correctly sets up the user with Google authentication method and profile data. The pattern is consistent with the GitHub implementation.
183-188: All accessed fields exist onGoogleProfiletype and are correctly typed.The web search confirms that
GoogleProfilefromremix-auth-googleincludes all accessed fields:
_json.name: string ✓photos: array of objects withvalueproperty ✓displayName: string ✓id: string ✓The code safely accesses these fields. The
if (authenticationProfile.photos[0])guard on line 185 prevents null/undefined access issues.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/webapp/app/assets/logos/GoogleLogo.tsx (1)
1-22: LGTM! Consider adding accessibility attributes.The GoogleLogo component is well-implemented with proper SVG structure and styling. The named export follows the coding guidelines.
For improved accessibility, consider adding an
aria-labelor wrapping the SVG in a way that provides context for screen reader users, especially if this logo is used in interactive elements like buttons.Example enhancement:
-export function GoogleLogo({ className }: { className?: string }) { +export function GoogleLogo({ className, ariaLabel }: { className?: string; ariaLabel?: string }) { return ( - <svg className={className} viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg + className={className} + viewBox="0 0 24 24" + fill="none" + xmlns="http://www.w3.org/2000/svg" + aria-label={ariaLabel} + role={ariaLabel ? "img" : undefined} + > <pathapps/webapp/app/routes/login._index/route.tsx (3)
22-40: LastUsedBadge implementation looks solid; consider minor a11y / UX tweaksThe motion +
useReducedMotionusage is clean and respects reduced-motion preferences, and the visual treatment is clear.Two small optional refinements:
- To avoid screen readers announcing a detached “Last used” label without context, consider either:
- Marking the badge container as decorative:
aria-hidden="true", or- Wiring it via
aria-describedbyfrom the corresponding button, so it’s semantically tied to that control.- If you find the badge sometimes intercepts clicks near the right edge of the button, you could add
pointer-events-noneon the outer wrapper (and re-enable pointer events only on the button if needed).These are polish-level changes; current behavior is acceptable if you’re okay with the existing semantics.
118-181: Multi‑provider login UI and “last used” badge behavior look coherentThe new layout with stacked GitHub, Google, and Email options is consistent:
- Conditional rendering via
data.showGithubAuth/data.showGoogleAuthis straightforward.- Action URLs correctly preserve
redirectTowhere present.LastUsedBadgeis correctly scoped viadata.lastAuthMethod === "<provider>"and only displayed when the provider is visible.data.authError && <FormError>continues to behave as expected across loader branches.If this grows further (e.g., more providers), you might eventually want a small
<AuthProviderButton>abstraction to avoid duplicating the container +Form+Buttonpattern, but for the current three providers the duplication is manageable.
183-191: Addrelattribute to external links that usetarget="_blank"Both legal links open in a new tab via
target="_blank", but lack arelattribute. To avoid reverse‑tabnabbing and follow common security best practices, consider:- <TextLink href="https://trigger.dev/legal" target="_blank"> + <TextLink href="https://trigger.dev/legal" target="_blank" rel="noreferrer noopener"> … - <TextLink href="https://trigger.dev/legal/privacy" target="_blank"> + <TextLink href="https://trigger.dev/legal/privacy" target="_blank" rel="noreferrer noopener">Even though these are same‑origin links, using
rel="noopener noreferrer"on alltarget="_blank"links is a good default.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/assets/logos/GoogleLogo.tsx(1 hunks)apps/webapp/app/routes/login._index/route.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/routes/login._index/route.tsxapps/webapp/app/assets/logos/GoogleLogo.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/login._index/route.tsxapps/webapp/app/assets/logos/GoogleLogo.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/login._index/route.tsxapps/webapp/app/assets/logos/GoogleLogo.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/login._index/route.tsxapps/webapp/app/assets/logos/GoogleLogo.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/routes/login._index/route.tsxapps/webapp/app/assets/logos/GoogleLogo.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/routes/login._index/route.tsxapps/webapp/app/assets/logos/GoogleLogo.tsx
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/webapp/app/routes/login._index/route.tsx (1)
71-82: Loader wiring forlastAuthMethodand Google support looks correct; verify value domainThe loader changes cleanly plumb
lastAuthMethodandshowGoogleAuththrough both branches of the response, and the UI only branches on strict string matches ("github" | "google" | "email"), so unknown values gracefully result in no badge.One thing to double‑check: ensure
getLastAuthMethod(request)is constrained to a narrow union (e.g."github" | "google" | "email" | null) and safely handles missing/invalid cookies, ideally with validation at the helper level (e.g. zod narrowing). That keeps the loader anduseTypedLoaderDatatypes tight and prevents accidental propagation of arbitrary values.Also applies to: 103-108
…t in the db but their email doesn't
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/models/user.server.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/models/user.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/models/user.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/models/user.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/models/user.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/models/user.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/models/user.server.ts
🧠 Learnings (2)
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/models/user.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Leverage the PostgreSQL database through the `trigger.dev/database` Prisma 6.14.0 client in the webapp for all data access patterns
Applied to files:
apps/webapp/app/models/user.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/models/user.server.ts (1)
apps/webapp/app/utils/email.ts (1)
assertEmailAllowed(3-13)
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/webapp/app/models/user.server.ts (3)
3-3: LGTM! Type definitions follow existing patterns.The GoogleProfile import and FindOrCreateGoogle type definition are consistent with the existing GitHub authentication implementation.
Also applies to: 24-31
46-48: LGTM! Switch case properly integrated.The Google authentication case correctly delegates to the new handler function, maintaining consistency with existing authentication methods.
176-196: LGTM! Profile extraction and validation are correct.The email validation and profile data extraction follow the same pattern as the GitHub authentication implementation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/routes/login._index/route.tsx (1)
22-40: Consider adding an accessible label for screen readers.The
LastUsedBadgecomponent respects motion preferences (good!), but it lacks semantic information for assistive technologies. Screen reader users won't know which authentication method was last used.Consider adding an
aria-labelto the motion.div:<motion.div className="relative rounded border border-charcoal-700 bg-charcoal-800 px-2 py-1 text-center text-xxs font-medium uppercase text-blue-500" + aria-label="Last used authentication method" initial={shouldReduceMotion ? undefined : { opacity: 0, x: 4 }} animate={shouldReduceMotion ? undefined : { opacity: 1, x: 0 }} transition={shouldReduceMotion ? undefined : { duration: 0.8, ease: "easeOut" }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/routes/login._index/route.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/routes/login._index/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/login._index/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/login._index/route.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/login._index/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/routes/login._index/route.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/routes/login._index/route.tsx
🧬 Code graph analysis (1)
apps/webapp/app/routes/login._index/route.tsx (6)
apps/webapp/app/services/lastAuthMethod.server.ts (1)
getLastAuthMethod(14-21)apps/webapp/app/services/auth.server.ts (1)
isGoogleAuthSupported(31-31)apps/webapp/app/components/primitives/Headers.tsx (1)
Header1(32-50)apps/webapp/app/components/primitives/Paragraph.tsx (1)
Paragraph(88-107)apps/webapp/app/components/primitives/Buttons.tsx (1)
Button(296-329)apps/webapp/app/assets/logos/GoogleLogo.tsx (1)
GoogleLogo(1-22)
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/routes/login._index/route.tsx (4)
5-5: LGTM!The new imports correctly support Google authentication and the last-used indicator feature. The framer-motion library is appropriately used for the animated badge component.
Also applies to: 7-7, 15-16
71-71: LGTM!The loader correctly fetches the last-used authentication method and Google auth support flag, passing them consistently to the UI in both conditional branches.
Also applies to: 80-81, 106-107
127-179: Well-structured multi-provider authentication UI.The conditional rendering of
LastUsedBadgefor each authentication method is correctly implemented. Each provider is properly wrapped in a relative container, and the form actions correctly include the redirectTo parameter.
118-194: Clean and well-organized layout structure.The login page structure is logically organized with clear sections for the header, authentication options, error display, and legal notices. The terms and privacy links properly open in new tabs.
… with the same email address
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/models/user.server.ts (1)
14-23:AuthConflictErroris defined but never used.This error class is exported but never thrown anywhere in the file. The conflict case at lines 247-260 logs the error and returns
existingUserinstead of throwingAuthConflictError. Either:
- Throw this error in the conflict case so callers can handle it appropriately (e.g., show a user-friendly message), or
- Remove the unused class if silent handling is intentional.
If the intent is to surface this conflict to the user, consider:
logger.error( `Google auth conflict: Google ID ${authenticationProfile.id} belongs to user ${existingUser.id} but email ${email} is taken by user ${existingEmailUser.id}`, { email, existingEmailUserId: existingEmailUser.id, existingAuthUserId: existingUser.id, authIdentifier, } ); - return { - user: existingUser, - isNewUser: false, - }; + throw new AuthConflictError( + `Email ${email} is already associated with a different account`, + existingEmailUser.id, + existingUser.id + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/models/user.server.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/models/user.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/models/user.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/models/user.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/models/user.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/models/user.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/models/user.server.ts
🧠 Learnings (2)
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/models/user.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Leverage the PostgreSQL database through the `trigger.dev/database` Prisma 6.14.0 client in the webapp for all data access patterns
Applied to files:
apps/webapp/app/models/user.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/models/user.server.ts (1)
apps/webapp/app/utils/email.ts (1)
assertEmailAllowed(3-13)
⏰ 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). (24)
- GitHub Check: Cursor Bugbot
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/models/user.server.ts (4)
37-42: LGTM!The type definition follows the established pattern from
FindOrCreateGithuband usestypeinstead ofinterfaceper coding guidelines.
44-44: LGTM!Union type correctly extended to include
FindOrCreateGoogle.
59-61: LGTM!Correctly dispatches to the new
findOrCreateGoogleUserfunction.
243-261: Verify intended behavior when Google email conflicts with existing user.When a conflict is detected (Google auth belongs to user A, but the current Google email is already taken by user B), the code:
- Logs the conflict
- Returns user A (the Google-authenticated user)
- Does not update user A's email to match their current Google email
This means user A continues to have their old email in the system while authenticating with a different Google email. Is this the intended behavior? Consider whether:
- User A should be prompted to resolve the conflict
- The
AuthConflictErrorshould be thrown to allow the caller to handle this case (e.g., show an error page)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/models/user.server.ts (1)
189-315: Consider refactoring to reduce duplication with GitHub authentication.The
findOrCreateGoogleUserfunction has significant overlap withfindOrCreateGithubUser(lines 99-187). Common patterns include:
- Profile extraction (name, avatarUrl, displayName)
- Prisma.JsonObject casting
- Three-branch logic (link, conflict/update, upsert)
While acceptable for feature delivery, future refactoring could extract shared logic into helper functions to improve maintainability.
Example refactoring approach:
function extractOAuthProfile(profile: GitHubProfile | GoogleProfile, authenticationExtraParams: Record<string, unknown>) { const name = profile._json.name; const avatarUrl = profile.photos[0]?.value; const displayName = profile.displayName; const authProfile = profile as unknown as Prisma.JsonObject; const authExtraParams = authenticationExtraParams as unknown as Prisma.JsonObject; return { name, avatarUrl, displayName, authProfile, authExtraParams }; }This refactoring can be deferred to a future PR focused on code quality improvements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/models/user.server.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/models/user.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/models/user.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/models/user.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/models/user.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/models/user.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/models/user.server.ts
🧠 Learnings (2)
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/models/user.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Leverage the PostgreSQL database through the `trigger.dev/database` Prisma 6.14.0 client in the webapp for all data access patterns
Applied to files:
apps/webapp/app/models/user.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/models/user.server.ts (2)
apps/webapp/app/utils/email.ts (1)
assertEmailAllowed(3-13)apps/webapp/app/db.server.ts (1)
Prisma(99-99)
⏰ 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). (14)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
apps/webapp/app/models/user.server.ts (5)
3-3: LGTM!The GoogleProfile and logger imports are necessary for the new Google authentication flow and conflict logging.
Also applies to: 12-12
37-44: LGTM!The
FindOrCreateGoogletype and union extension follow the existing pattern for GitHub authentication and properly type the Google OAuth flow.
59-61: LGTM!The switch case correctly routes Google authentication requests to the new handler function.
245-261: Verify conflict handling aligns with product requirements.When a Google ID belongs to one user but the associated email belongs to a different user (e.g., user A changed their Google email to user B's email), the code logs the conflict and returns user A (the Google-authenticated user) without updating their email.
Behavior:
- User A successfully logs in with Google
- User A's email remains unchanged (not updated to the conflicting email)
- User A can continue using the app normally
- Conflict is logged for investigation
Consideration: Should users be prevented from logging in when this conflict occurs, or is silent logging with successful login acceptable? If users should be blocked, throw
AuthConflictErrorinstead (see previous comment).Confirm with product/design that this conflict resolution strategy meets requirements. If users should be notified or blocked, update the error handling accordingly.
189-315: Excellent implementation addressing past review concerns.The
findOrCreateGoogleUserfunction correctly handles the key scenarios:
- Linking email accounts (lines 223-241): Preserves the original
authenticationMethodwhen linking an existing email user to Google ✓- Conflict detection (lines 245-261): Detects when email and Google ID belong to different users and logs the conflict ✓
- Profile updates (lines 263-281): Updates all fields (email, displayName, name, avatarUrl, authenticationProfile, authenticationExtraParams) for returning users ✓
- Upsert completeness (lines 291-297): Includes
authenticationProfileandauthenticationExtraParamsin the update path ✓The control flow ensures email availability before upserting, preventing database constraint violations.
Note
Adds Google OAuth login and a cookie-based “last used” indicator on the login page, with supporting backend, routes, and schema updates.
remix-auth-googlevia newaddGoogleStrategyand enables whenAUTH_GOOGLE_CLIENT_ID/SECRETare set (services/googleAuth.server.ts,services/auth.server.ts).findOrCreateGoogleUserwith linking/upsert logic and conflict logging (models/user.server.ts).routes/auth.google*.tsx,routes/auth.github.callback.tsx,routes/magic.tsx,services/lastAuthMethod.server.ts).services/gitHubAuth.server.ts).routes/login._index/route.tsx).routes/auth.github.ts,routes/auth.google.ts).GoogleLogoSVG.referrerPolicy="no-referrer"on profile image.AUTH_GOOGLE_CLIENT_ID/AUTH_GOOGLE_CLIENT_SECRET(env.server.ts).AuthenticationMethodenum withGOOGLE(Prisma schema + migration).remix-auth-googleinpackage.json.Written by Cursor Bugbot for commit 9f84f97. This will update automatically on new commits. Configure here.