feat(sso): SAML/OIDC single sign-on#3911
Conversation
|
WalkthroughThis PR introduces end-to-end SSO support across the monorepo. A new 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b89037b9-5f6d-4e99-af68-98db08cc3089
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
.changeset/sso-plugin-contract.md.server-changes/accounts-webhook-passthrough.md.server-changes/sso-plugin-plumbing.mdapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/env.server.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/models/user.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/routes/auth.sso.callback.tsxapps/webapp/app/routes/auth.sso.tsapps/webapp/app/routes/login._index/route.tsxapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/login.mfa/route.tsxapps/webapp/app/routes/login.sso/route.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/routes/webhooks.v1.accounts.tsapps/webapp/app/services/auth.server.tsapps/webapp/app/services/authUser.tsapps/webapp/app/services/emailAuth.server.tsxapps/webapp/app/services/gitHubAuth.server.tsapps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/services/session.server.tsapps/webapp/app/services/sso.server.tsapps/webapp/app/services/ssoAuth.server.tsapps/webapp/app/services/ssoAutoDiscovery.server.tsapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/services/ssoSessionRevalidation.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/v3/accountsWebhookWorker.server.tsapps/webapp/app/v3/featureFlags.tsapps/webapp/package.jsoninternal-packages/database/prisma/migrations/20260527130000_add_sso_authentication_method/migration.sqlinternal-packages/database/prisma/schema.prismainternal-packages/sso/package.jsoninternal-packages/sso/src/fallback.tsinternal-packages/sso/src/index.tsinternal-packages/sso/src/loader.test.tsinternal-packages/sso/tsconfig.jsoninternal-packages/sso/vitest.config.tspackages/plugins/package.jsonpackages/plugins/src/index.tspackages/plugins/src/sso.ts
✅ Files skipped from review due to trivial changes (5)
- internal-packages/database/prisma/migrations/20260527130000_add_sso_authentication_method/migration.sql
- apps/webapp/app/services/authUser.ts
- packages/plugins/package.json
- .server-changes/accounts-webhook-passthrough.md
- .changeset/sso-plugin-contract.md
🚧 Files skipped from review as they are similar to previous changes (35)
- apps/webapp/app/utils/pathBuilder.ts
- internal-packages/sso/package.json
- apps/webapp/app/routes/webhooks.v1.accounts.ts
- apps/webapp/app/v3/accountsWebhookWorker.server.ts
- apps/webapp/app/v3/featureFlags.ts
- apps/webapp/app/services/sso.server.ts
- apps/webapp/app/services/emailAuth.server.tsx
- apps/webapp/app/services/session.server.ts
- internal-packages/sso/vitest.config.ts
- packages/plugins/src/index.ts
- internal-packages/database/prisma/schema.prisma
- apps/webapp/app/services/auth.server.ts
- apps/webapp/app/routes/magic.tsx
- apps/webapp/app/services/ssoAutoDiscovery.server.ts
- apps/webapp/app/services/gitHubAuth.server.ts
- apps/webapp/app/env.server.ts
- apps/webapp/app/routes/login.mfa/route.tsx
- internal-packages/sso/tsconfig.json
- apps/webapp/app/routes/login._index/route.tsx
- apps/webapp/app/services/lastAuthMethod.server.ts
- internal-packages/sso/src/loader.test.ts
- apps/webapp/app/models/user.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsx
- apps/webapp/app/routes/login.magic/route.tsx
- internal-packages/sso/src/fallback.ts
- apps/webapp/app/services/googleAuth.server.ts
- packages/plugins/src/sso.ts
- apps/webapp/app/routes/auth.sso.callback.tsx
- apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx
- internal-packages/sso/src/index.ts
- apps/webapp/app/routes/auth.github.callback.tsx
- apps/webapp/app/services/ssoSessionRevalidation.server.ts
- apps/webapp/app/services/ssoAuth.server.ts
- apps/webapp/app/routes/login.sso/route.tsx
- apps/webapp/app/routes/auth.google.callback.tsx
📜 Review details
⏰ 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). (40)
- GitHub Check: internal / 🧪 Unit Tests: Internal (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: internal / 🧪 Unit Tests: Internal (8, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (7, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (12, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (3, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (6, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (9, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (10, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: internal / 🧪 Unit Tests: Internal (2, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: internal / 🧪 Unit Tests: Internal (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: internal / 🧪 Unit Tests: Internal (5, 12)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: typecheck / typecheck
- GitHub Check: audit
- GitHub Check: 🛡️ E2E Auth Tests (full)
- GitHub Check: Build and publish previews
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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 insteadImport from
@trigger.dev/sdkwhen writing Trigger.dev tasks. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob
Files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/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/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
**/*.{ts,tsx,js,jsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when circular dependencies cannot be resolved, code splitting is needed for performance, or the module must be loaded conditionally at runtime
Import subpaths only frompackages/core(@trigger.dev/core), never import from the root
Files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
**/*.{js,ts,tsx,jsx,css,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier for code formatting and run
pnpm run formatbefore committing
Files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/package.jsonapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.ts
apps/webapp/**/*.server.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.ts: Never userequest.signalfor detecting client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.tsinstead, which is wired directly to Expressres.on('close')and fires reliably
Access environment variables viaenvexport fromapp/env.server.ts. Never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries.findUniquehas an implicit DataLoader that batches concurrent calls and has active bugs even in Prisma 6.x (uppercase UUIDs returning null, composite key SQL correctness issues, 5-10x worse performance).findFirstis never batched and avoids this entire class of issues
Files:
apps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/models/orgMember.server.ts
🧠 Learnings (16)
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-05-08T21:00:20.973Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3538
File: apps/webapp/app/components/primitives/Resizable.tsx:60-78
Timestamp: 2026-05-08T21:00:20.973Z
Learning: In the triggerdotdev/trigger.dev codebase, treat Zod as a boundary validation tool (API handlers, request/response validation, and storage/DB read/write validation), not as inline render-time validation inside React components/primitive UI code. For render-time guards, prefer small manual type-narrowing checks (e.g., a short predicate like ~10–20 lines) over importing Zod into UI primitives, to avoid per-render schema-parse overhead and unnecessary abstraction. Use the manual guard approach unless you truly need schema validation at a boundary; only then introduce Zod.
Applied to files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-04-27T16:46:03.861Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3456
File: apps/webapp/package.json:152-152
Timestamp: 2026-04-27T16:46:03.861Z
Learning: In `apps/webapp/package.json`, treat the `effect` npm package as an intentional runtime dependency (not unused/misplaced) for the Schedule + Fiber-based metadata update logic. This should apply when reviewing `apps/webapp` code paths used by `apps/webapp/app/utils/updateMetadata.server.ts` (and closely related modules) that use Effect APIs such as `Duration.divide`, `STM.cond`, namespace exports for `Effect`/`Schedule`/`Duration`/`Fiber`, and the `Fiber.RuntimeFiber` type.
Applied to files:
apps/webapp/package.json
📚 Learning: 2026-03-26T09:02:07.973Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3274
File: apps/webapp/app/services/runsReplicationService.server.ts:922-924
Timestamp: 2026-03-26T09:02:07.973Z
Learning: When parsing Trigger.dev task run annotations in server-side services, keep `TaskRun.annotations` strictly conforming to the `RunAnnotations` schema from `trigger.dev/core/v3`. If the code already uses `RunAnnotations.safeParse` (e.g., in a `#parseAnnotations` helper), treat that as intentional/necessary for atomic, schema-accurate annotation handling. Do not recommend relaxing the annotation payload schema or using a permissive “passthrough” parse path, since the annotations are expected to be written atomically in one operation and should not contain partial/legacy payloads that would require a looser parser.
Applied to files:
apps/webapp/app/services/ssoRateLimiter.server.ts
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/models/orgMember.server.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
apps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/routes/auth.sso.tsapps/webapp/app/models/orgMember.server.ts
📚 Learning: 2026-04-02T19:18:26.255Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions/route.tsx:179-189
Timestamp: 2026-04-02T19:18:26.255Z
Learning: In this repo’s route components that render the Inspector `ResizablePanelGroup` panels, it’s acceptable to pass `collapsed={!isShowingInspector}` together with a no-op `onCollapseChange={() => {}}` when panel visibility is intentionally controlled only by route parameters (e.g., `*Param` search/route params) rather than user drag/collapse interactions. Do not flag an empty/no-op `onCollapseChange` as “missing wiring” in these cases; only flag it when collapse state is expected to change based on user interaction.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-05-12T21:04:00.184Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx:40-42
Timestamp: 2026-05-12T21:04:00.184Z
Learning: In triggerdotdev/trigger.dev route loader implementations (Remix `route.tsx` files under `apps/webapp/app/routes/**`), follow the existing convention for missing/unauthorized environment lookups: when `findEnvironmentBySlug` (or the equivalent env resolver) returns a falsy value, handle it by throwing `new Error("Environment not found")` rather than returning a `404` `Response` (i.e., do not flag this as “missing 404 response”). Changing the error-to-404 convention is a cross-cutting refactor and should be left out of individual PRs unless the PR explicitly addresses that broader migration.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
📚 Learning: 2026-05-14T14:54:39.095Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3545
File: .server-changes/agent-view-sessions.md:10-10
Timestamp: 2026-05-14T14:54:39.095Z
Learning: In the `trigger.dev` repository, do not flag inconsistent dot vs slash notation in route/path strings inside `.server-changes/*.md` files. These markdown files are consumed verbatim into the changelog, so the mixed notation (e.g., `resources.orgs.../runs.$runParam/...`) is intentional and should be preserved as-is.
Applied to files:
.server-changes/sso-plugin-plumbing.md
🪛 LanguageTool
.server-changes/sso-plugin-plumbing.md
[grammar] ~6-~6: Ensure spelling is correct
Context: ...in loader (@trigger.dev/sso) into the webapp: SSO auth method, hasSso flag, `SsoStr...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (2)
apps/webapp/app/models/orgMember.server.ts (1)
31-46: This still has the previously-raisedfindFirst()→create()race.Two concurrent JIT/invite requests can both miss the lookup and one will still throw on the unique constraint instead of returning
{ created: false }.apps/webapp/app/services/ssoRateLimiter.server.ts (1)
52-53:retryAftercan still go negative here.The earlier comment about clamping this value to
>= 0still applies to both limit paths.Also applies to: 60-61
| // Issue all three writes in parallel — they touch the same | ||
| // OrgSsoConfig row but only update disjoint columns, so there | ||
| // is no contention. A failure on any leaves the others applied; | ||
| // surface the first error string back to the form. | ||
| const [enforced, jit, jitRole] = await Promise.all([ | ||
| ssoController.setEnforced({ | ||
| organizationId: orgId, | ||
| enforced: parsed.data.enforced, | ||
| }), | ||
| ssoController.setJitProvisioningEnabled({ | ||
| organizationId: orgId, | ||
| enabled: parsed.data.jitEnabled, | ||
| }), | ||
| ssoController.setJitDefaultRole({ organizationId: orgId, roleId: jitRoleId }), | ||
| ]); | ||
| const failed = [enforced, jit, jitRole].find((r) => r.isErr()); | ||
| if (failed && failed.isErr()) { | ||
| return new Response(`Error: ${failed.error}`, { status: 400 }); |
There was a problem hiding this comment.
Make the settings save atomic.
The UI presents this as one Save action, but the implementation can persist any subset of the three changes before returning 400. That means enforcement/JIT behavior may already have changed even though the user sees a failed save. This needs a single controller-level update or transactional write so the form is all-or-nothing.
| // SSO auto-discovery: if the signed-in user's domain requires SSO, the | ||
| // current session was established via a non-SSO method — block the | ||
| // onboarding action and route them through the SSO flow instead. | ||
| const sessionUser = await prisma.user.findFirst({ | ||
| where: { id: userId }, | ||
| select: { email: true }, | ||
| }); | ||
| if (sessionUser?.email) { | ||
| const ssoRedirect = await ssoRedirectForEmail(sessionUser.email, "oauth_blocked"); | ||
| if (ssoRedirect) { | ||
| return redirect(ssoRedirect); | ||
| } | ||
| } |
There was a problem hiding this comment.
Preserve the Vercel onboarding return state before redirecting into SSO.
This redirect drops the active onboarding context (code, configurationId, next, and the selected org step), because ssoRedirectForEmail() only returns /login/sso?... with email and reason. Users who hit the SSO gate mid-install can’t resume this flow after authenticating, and a single-use Vercel code may be lost entirely. Please thread a redirectTo/resume URL through this branch before handing off to SSO.
Vendor-neutral plugin contract plus the host wiring that consumes it. With no SSO plugin installed, everything degrades to a no-op fallback, so OSS deployments are unaffected. - Plugin contract (@trigger.dev/plugins) + lazy loader/fallback in internal-packages/sso: status, portal-link, enforce/JIT config, route-decision, begin/complete authorization, identity resolution, JIT evaluation, and periodic session validation. All methods return neverthrow Results; the fallback is fail-open. - Login: 'Sign in with SSO' entry + dedicated /login/sso flow and /auth/sso(.callback) routes, plus auto-discovery from magic-link/OAuth. - Org settings -> SSO page: plan-tier upsell, connection status, verified-domain list, enforcement + JIT provisioning + default-role configuration, and an admin-portal link dialog. - AuthUser carries an optional signed 'sso' marker; SSO-established sessions are periodically re-validated against the identity provider on a single-flight, throttled, fail-open basis and logged out only on an explicit invalid result. - SSO_ENABLED gate (default off) so the feature ships dark until its backing plugin is available; SSO_SESSION_REVALIDATION_INTERVAL_SECONDS controls the cadence.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 87f34a29-8b5e-40c6-bca8-09a7230163e4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
.server-changes/accounts-webhook-passthrough.md.server-changes/sso-plugin-plumbing.md.server-changes/sso-session-expired-logout-ux.mdapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/entry.client.tsxapps/webapp/app/env.server.tsapps/webapp/app/hooks/useEventSource.tsxapps/webapp/app/models/orgMember.server.tsapps/webapp/app/models/user.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.github.callback.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/routes/auth.sso.callback.tsxapps/webapp/app/routes/auth.sso.tsapps/webapp/app/routes/login._index/route.tsxapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/login.mfa/route.tsxapps/webapp/app/routes/login.sso/route.tsxapps/webapp/app/routes/logout.tsxapps/webapp/app/routes/magic.tsxapps/webapp/app/routes/resources.session-check.tsapps/webapp/app/routes/vercel.onboarding.tsxapps/webapp/app/routes/webhooks.v1.accounts.tsapps/webapp/app/services/auth.server.tsapps/webapp/app/services/authUser.tsapps/webapp/app/services/emailAuth.server.tsxapps/webapp/app/services/gitHubAuth.server.tsapps/webapp/app/services/googleAuth.server.tsapps/webapp/app/services/lastAuthMethod.server.tsapps/webapp/app/services/session.server.tsapps/webapp/app/services/sso.server.tsapps/webapp/app/services/ssoAuth.server.tsapps/webapp/app/services/ssoAutoDiscovery.server.tsapps/webapp/app/services/ssoRateLimiter.server.tsapps/webapp/app/services/ssoSessionRevalidation.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/utils/ssoSession.tsapps/webapp/app/utils/ssoSessionGuard.tsapps/webapp/app/v3/accountsWebhookWorker.server.tsapps/webapp/app/v3/featureFlags.tsapps/webapp/package.jsoninternal-packages/database/prisma/migrations/20260527130000_add_sso_authentication_method/migration.sqlinternal-packages/database/prisma/schema.prismainternal-packages/sso/package.jsoninternal-packages/sso/src/fallback.tsinternal-packages/sso/src/index.tsinternal-packages/sso/src/loader.test.tsinternal-packages/sso/tsconfig.jsoninternal-packages/sso/vitest.config.ts
✅ Files skipped from review due to trivial changes (2)
- internal-packages/database/prisma/migrations/20260527130000_add_sso_authentication_method/migration.sql
- .server-changes/sso-session-expired-logout-ux.md
🚧 Files skipped from review as they are similar to previous changes (41)
- apps/webapp/app/routes/resources.session-check.ts
- internal-packages/sso/tsconfig.json
- apps/webapp/app/utils/pathBuilder.ts
- internal-packages/sso/vitest.config.ts
- .server-changes/accounts-webhook-passthrough.md
- apps/webapp/app/services/emailAuth.server.tsx
- apps/webapp/app/services/gitHubAuth.server.ts
- apps/webapp/app/routes/vercel.onboarding.tsx
- apps/webapp/package.json
- internal-packages/sso/package.json
- apps/webapp/app/services/auth.server.ts
- apps/webapp/app/services/authUser.ts
- apps/webapp/app/utils/ssoSession.ts
- apps/webapp/app/v3/featureFlags.ts
- apps/webapp/app/services/session.server.ts
- apps/webapp/app/routes/login.mfa/route.tsx
- apps/webapp/app/hooks/useEventSource.tsx
- apps/webapp/app/routes/magic.tsx
- internal-packages/database/prisma/schema.prisma
- apps/webapp/app/routes/webhooks.v1.accounts.ts
- apps/webapp/app/routes/logout.tsx
- apps/webapp/app/services/lastAuthMethod.server.ts
- apps/webapp/app/services/googleAuth.server.ts
- apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx
- apps/webapp/app/services/ssoRateLimiter.server.ts
- apps/webapp/app/routes/login._index/route.tsx
- apps/webapp/app/utils/ssoSessionGuard.ts
- apps/webapp/app/routes/auth.github.callback.tsx
- apps/webapp/app/services/ssoAuth.server.ts
- internal-packages/sso/src/index.ts
- internal-packages/sso/src/loader.test.ts
- apps/webapp/app/models/user.server.ts
- internal-packages/sso/src/fallback.ts
- apps/webapp/app/services/sso.server.ts
- apps/webapp/app/routes/auth.sso.ts
- apps/webapp/app/routes/login.sso/route.tsx
- apps/webapp/app/v3/accountsWebhookWorker.server.ts
- apps/webapp/app/models/orgMember.server.ts
- apps/webapp/app/services/ssoAutoDiscovery.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx
- apps/webapp/app/services/ssoSessionRevalidation.server.ts
📜 Review details
⏰ 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). (38)
- GitHub Check: internal / 🧪 Unit Tests: Internal (7, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (5, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (9, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (8, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (3, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (4, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (10, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (11, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (12, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (1, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (2, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal (6, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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 insteadImport from
@trigger.dev/sdkwhen writing Trigger.dev tasks. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob
Files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.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/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
**/*.{ts,tsx,js,jsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when circular dependencies cannot be resolved, code splitting is needed for performance, or the module must be loaded conditionally at runtime
Import subpaths only frompackages/core(@trigger.dev/core), never import from the root
Files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
**/*.{js,ts,tsx,jsx,css,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier for code formatting and run
pnpm run formatbefore committing
Files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/env.server.ts
apps/webapp/**/*.server.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.ts: Never userequest.signalfor detecting client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.tsinstead, which is wired directly to Expressres.on('close')and fires reliably
Access environment variables viaenvexport fromapp/env.server.ts. Never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries.findUniquehas an implicit DataLoader that batches concurrent calls and has active bugs even in Prisma 6.x (uppercase UUIDs returning null, composite key SQL correctness issues, 5-10x worse performance).findFirstis never batched and avoids this entire class of issues
Files:
apps/webapp/app/env.server.ts
🧠 Learnings (17)
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.
Applied to files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-05-08T21:00:20.973Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3538
File: apps/webapp/app/components/primitives/Resizable.tsx:60-78
Timestamp: 2026-05-08T21:00:20.973Z
Learning: In the triggerdotdev/trigger.dev codebase, treat Zod as a boundary validation tool (API handlers, request/response validation, and storage/DB read/write validation), not as inline render-time validation inside React components/primitive UI code. For render-time guards, prefer small manual type-narrowing checks (e.g., a short predicate like ~10–20 lines) over importing Zod into UI primitives, to avoid per-render schema-parse overhead and unnecessary abstraction. Use the manual guard approach unless you truly need schema validation at a boundary; only then introduce Zod.
Applied to files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/app/entry.client.tsxapps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/env.server.tsapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/auth.google.callback.tsxapps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsxapps/webapp/app/routes/auth.sso.callback.tsx
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-05-20T17:21:18.543Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3678
File: apps/webapp/app/entry.server.tsx:0-0
Timestamp: 2026-05-20T17:21:18.543Z
Learning: In env.server.ts (Zod env schema), any environment variable you plan to access via the typed `env` export (e.g., `env.SENTRY_DSN`) must be explicitly declared in the schema. For `SENTRY_DSN`, include `SENTRY_DSN: z.string().optional()`; otherwise switching from `process.env.SENTRY_DSN` to `env.SENTRY_DSN` will fail TypeScript typechecking.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-06-01T11:37:08.569Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3754
File: apps/webapp/app/env.server.ts:1104-1129
Timestamp: 2026-06-01T11:37:08.569Z
Learning: In apps/*/app/env.server.ts, any new background/periodic worker feature flag should hard-default to "0" (explicit opt-in) rather than inheriting from a parent flag (e.g., avoid defaulting to process.env.TRIGGER_MOLLIFIER_ENABLED ?? "0"). Inheriting can cause the new worker to auto-start on upgrade for deployments that already enabled the parent flag, turning on unexpected background load without an explicit rollout. Each worker component must require its own dedicated env var and default it explicitly to "0" (e.g., TRIGGER_MOLLIFIER_STALE_SWEEP_ENABLED defaults to "0" unless explicitly set to enable that worker).
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-04-02T19:18:26.255Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions/route.tsx:179-189
Timestamp: 2026-04-02T19:18:26.255Z
Learning: In this repo’s route components that render the Inspector `ResizablePanelGroup` panels, it’s acceptable to pass `collapsed={!isShowingInspector}` together with a no-op `onCollapseChange={() => {}}` when panel visibility is intentionally controlled only by route parameters (e.g., `*Param` search/route params) rather than user drag/collapse interactions. Do not flag an empty/no-op `onCollapseChange` as “missing wiring” in these cases; only flag it when collapse state is expected to change based on user interaction.
Applied to files:
apps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsx
📚 Learning: 2026-05-12T21:04:00.184Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.sessions._index/route.tsx:40-42
Timestamp: 2026-05-12T21:04:00.184Z
Learning: In triggerdotdev/trigger.dev route loader implementations (Remix `route.tsx` files under `apps/webapp/app/routes/**`), follow the existing convention for missing/unauthorized environment lookups: when `findEnvironmentBySlug` (or the equivalent env resolver) returns a falsy value, handle it by throwing `new Error("Environment not found")` rather than returning a `404` `Response` (i.e., do not flag this as “missing 404 response”). Changing the error-to-404 convention is a cross-cutting refactor and should be left out of individual PRs unless the PR explicitly addresses that broader migration.
Applied to files:
apps/webapp/app/routes/login.magic/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsx
📚 Learning: 2026-05-14T14:54:39.095Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3545
File: .server-changes/agent-view-sessions.md:10-10
Timestamp: 2026-05-14T14:54:39.095Z
Learning: In the `trigger.dev` repository, do not flag inconsistent dot vs slash notation in route/path strings inside `.server-changes/*.md` files. These markdown files are consumed verbatim into the changelog, so the mixed notation (e.g., `resources.orgs.../runs.$runParam/...`) is intentional and should be preserved as-is.
Applied to files:
.server-changes/sso-plugin-plumbing.md
🪛 LanguageTool
.server-changes/sso-plugin-plumbing.md
[grammar] ~6-~6: Ensure spelling is correct
Context: ...in loader (@trigger.dev/sso) into the webapp: SSO auth method, hasSso flag, `SsoStr...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (6)
.server-changes/sso-plugin-plumbing.md (1)
1-8: LGTM!apps/webapp/app/entry.client.tsx (1)
6-9: LGTM!apps/webapp/app/routes/auth.sso.callback.tsx (1)
1-110: LGTM!apps/webapp/app/routes/auth.google.callback.tsx (1)
10-39: LGTM!apps/webapp/app/routes/login.magic/route.tsx (1)
33-33: LGTM!Also applies to: 136-187
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsx (1)
12-12: LGTM!Also applies to: 15-18, 27-28, 33-33, 43-43
| SSO_SESSION_REVALIDATION_INTERVAL_SECONDS: z.coerce.number().int().default(300), | ||
| // Hard timeout (ms) on the re-validation round-trip. If the SSO plugin | ||
| // doesn't answer within this window the check fails OPEN (session kept) | ||
| // and emits a `sso.revalidation.timeout` warn log — alert on an | ||
| // elevated rate of those to catch a slow/unhealthy SSO dependency. | ||
| SSO_SESSION_REVALIDATION_TIMEOUT_MS: z.coerce.number().int().default(2000), |
There was a problem hiding this comment.
Add lower-bound validation to SSO revalidation numeric env vars.
SSO_SESSION_REVALIDATION_INTERVAL_SECONDS and SSO_SESSION_REVALIDATION_TIMEOUT_MS accept 0/negative values today. That can cause pathological revalidation behavior (request-amplification or immediate timeout churn) under misconfiguration. Enforce strictly positive bounds in the schema.
Suggested diff
- SSO_SESSION_REVALIDATION_INTERVAL_SECONDS: z.coerce.number().int().default(300),
+ SSO_SESSION_REVALIDATION_INTERVAL_SECONDS: z.coerce.number().int().positive().default(300),
...
- SSO_SESSION_REVALIDATION_TIMEOUT_MS: z.coerce.number().int().default(2000),
+ SSO_SESSION_REVALIDATION_TIMEOUT_MS: z.coerce.number().int().positive().default(2000),📝 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.
| SSO_SESSION_REVALIDATION_INTERVAL_SECONDS: z.coerce.number().int().default(300), | |
| // Hard timeout (ms) on the re-validation round-trip. If the SSO plugin | |
| // doesn't answer within this window the check fails OPEN (session kept) | |
| // and emits a `sso.revalidation.timeout` warn log — alert on an | |
| // elevated rate of those to catch a slow/unhealthy SSO dependency. | |
| SSO_SESSION_REVALIDATION_TIMEOUT_MS: z.coerce.number().int().default(2000), | |
| SSO_SESSION_REVALIDATION_INTERVAL_SECONDS: z.coerce.number().int().positive().default(300), | |
| // Hard timeout (ms) on the re-validation round-trip. If the SSO plugin | |
| // doesn't answer within this window the check fails OPEN (session kept) | |
| // and emits a `sso.revalidation.timeout` warn log — alert on an | |
| // elevated rate of those to catch a slow/unhealthy SSO dependency. | |
| SSO_SESSION_REVALIDATION_TIMEOUT_MS: z.coerce.number().int().positive().default(2000), |
| auth = await authenticator.authenticate("email-link", request); | ||
| } catch (thrown) { | ||
| if (thrown instanceof Response) throw thrown; | ||
| const ssoRedirect = ssoRedirectFromAuthError(thrown); | ||
| if (ssoRedirect) { | ||
| return redirect(ssoRedirect); | ||
| } | ||
| return redirect("/login/magic"); |
There was a problem hiding this comment.
🟡 Magic-link verification loses auth error message after removing failureRedirect
In magic.tsx, the old code passed failureRedirect: "/login/magic" to authenticator.authenticate(). remix-auth's Strategy.failure() flashes the error message onto the session via session.flash(sessionErrorKey, { message }) before throwing the redirect Response. The new code calls authenticate without failureRedirect, so Strategy.failure() throws an AuthorizationError directly — bypassing the session-flash step. The catch block at line 27 redirects to /login/magic without setting auth:error on the session. The login.magic loader reads session.get("auth:error") (apps/webapp/app/routes/login.magic/route.tsx:63) and finds nothing, so expired-link / invalid-link errors are silently swallowed. Users clicking expired magic links see the email form again with no indication of what went wrong.
Prompt for agents
In apps/webapp/app/routes/magic.tsx, the authenticate call was changed from using failureRedirect (which flashes the error onto the session) to a bare try/catch that silently redirects. The catch block for non-SSO, non-Response errors at line 33 should set the auth error on the session before redirecting, mirroring what remix-auth's failureRedirect did internally. Use getSession/commitSession from sessionStorage.server to flash the auth:error key with the thrown error's message, then redirect to /login/magic with the Set-Cookie header. This preserves the error display for expired/invalid magic links while still allowing the SsoRequiredError branch to redirect to the SSO flow.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export async function ssoRedirectForEmail( | ||
| email: string, | ||
| reason: "domain_policy" | "oauth_blocked" | ||
| ): Promise<string | null> { | ||
| const normalised = email.toLowerCase().trim(); | ||
| if (!normalised) return null; | ||
|
|
||
| // Fail-open covers both shapes of failure: a returned `Err` (handled | ||
| // below) and a thrown/rejected promise (e.g. the plugin throwing before | ||
| // it can build its ResultAsync). Either way the original login flow | ||
| // proceeds rather than being blocked by an SSO dependency error. | ||
| let decision: Awaited<ReturnType<typeof ssoController.decideRouteForEmail>>; | ||
| try { | ||
| decision = await ssoController.decideRouteForEmail(normalised); | ||
| } catch (error) { | ||
| logger.warn("SSO auto-discovery fail-open (threw)", { error, email: normalised }); | ||
| return null; | ||
| } | ||
| if (decision.isErr()) { | ||
| logger.warn("SSO auto-discovery fail-open", { reason: decision.error, email: normalised }); | ||
| return null; | ||
| } | ||
| if (decision.value.kind !== "sso_required") return null; | ||
|
|
||
| return `/login/sso?email=${encodeURIComponent(normalised)}&reason=${reason}`; |
There was a problem hiding this comment.
🟡 SSO auto-discovery redirects discard the user's original redirectTo destination
When ssoRedirectForEmail() fires (magic-link send, magic-link click, GitHub/Google OAuth callbacks), it returns /login/sso?email=…&reason=… with no redirectTo parameter. The SSO login form (login.sso/route.tsx:72) defaults redirectTo to "/", and passes that to auth.sso.ts:71 → beginAuthorization. After completing SSO, the user lands on / instead of the page they originally requested.
The redirect chain: user visits /dashboard/foo → redirected to /login?redirectTo=/dashboard/foo → clicks magic link/GitHub/Google → SSO gate fires → redirect to /login/sso (no redirectTo) → SSO completes → user lands on /.
ssoRedirectForEmail at apps/webapp/app/services/ssoAutoDiscovery.server.ts:38 builds the URL without a redirectTo parameter, and none of the callers pass the user's intended destination to the function.
Prompt for agents
The ssoRedirectForEmail function in apps/webapp/app/services/ssoAutoDiscovery.server.ts needs an optional redirectTo parameter that gets appended to the returned URL. The callers (magic link send action at login.magic/route.tsx:184, magic link verify at emailAuth.server.tsx:37, GitHub verify at gitHubAuth.server.ts:33, Google verify at googleAuth.server.ts:33) should pass through the user's redirectTo when available. For the OAuth callbacks this is available from the redirect cookie; for the magic link send action it's in the redirect session; for the verify callbacks the redirectTo would need to be threaded through or read from the session. The simplest fix is to add redirectTo as an optional third parameter to ssoRedirectForEmail and append it as &redirectTo=... to the returned URL when present.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export async function loader({ request }: LoaderFunctionArgs) { | ||
| const url = new URL(request.url); | ||
| const reason = parseReason(url.searchParams.get("reason")); | ||
| const email = url.searchParams.get("email") ?? ""; | ||
| const errorCode = url.searchParams.get("error"); | ||
| const redirectTo = url.searchParams.get("redirectTo") ?? "/"; | ||
|
|
||
| return typedjson({ | ||
| reason, | ||
| email, | ||
| redirectTo, | ||
| errorMessage: errorCode ? (ERROR_MESSAGES[errorCode] ?? "We couldn't complete sign-in. Try again.") : null, | ||
| }); |
There was a problem hiding this comment.
🚩 /login/sso missing from NON_NAVIGABLE_EXACT — inconsistency with other login paths
The NON_NAVIGABLE_EXACT set at apps/webapp/app/utils.ts:12 includes /login, /login/magic, /login/mfa but not /login/sso. This means sanitizeRedirectPath treats /login/sso as a valid post-login destination, so a crafted ?redirectTo=/login/sso would send an already-authenticated user to the SSO form after login. Additionally, the login.sso route loader (apps/webapp/app/routes/login.sso/route.tsx:67) does not call authenticator.isAuthenticated(request, { successRedirect: '/' }) like login.magic does, so authenticated users see the SSO form rather than being redirected away. Both are minor inconsistencies with the established pattern but require a manually crafted URL to trigger.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
No description provided.