fix(webapp): match org invite emails case-insensitively#3849
Conversation
|
|
Need an answer fast? Review this PR in Change Stack to ask focused questions about the PR or a changed range. WalkthroughThis PR makes organization invite email handling case-insensitive. The invite form now trims and lowercases submitted emails before validation. The invite acceptance route compares emails case-insensitively. Prisma queries in getUsersInvites, acceptInvite, and declineInvite use equals + mode: "insensitive" for email matching. A changelog documents that invite emails are normalized on creation and lookups now match case-insensitively. 🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/webapp/app/models/member.server.ts (1)
87-142: 💤 Low valueConsider defensive email normalization in
inviteMembers.The function currently relies on the caller to lowercase emails (which happens in the form schema validation at route.tsx:152). For robustness, consider lowercasing the emails at the function boundary:
emails: emails.map(e => e.trim().toLowerCase()),This would ensure consistent behavior even if the function is called from other contexts in the future.
🛡️ Proposed defensive normalization
export async function inviteMembers({ slug, emails, userId, rbacRoleId, }: { slug: string; emails: string[]; userId: string; rbacRoleId?: string | null; }) { + // Normalize emails defensively at the function boundary + const normalizedEmails = emails.map(e => e.trim().toLowerCase()); + const org = await prisma.organization.findFirst({ where: { slug, members: { some: { userId } } }, }); if (!org) { throw new Error("User does not have access to this organization"); } - const invites = [...new Set(emails)].map( + const invites = [...new Set(normalizedEmails)].map( (email) => ({ email, token: tokenGenerator(), organizationId: org.id, inviterId: userId, role: "MEMBER", rbacRoleId: rbacRoleId ?? null, } satisfies Prisma.OrgMemberInviteCreateManyInput) ); await prisma.orgMemberInvite.createMany({ data: invites, }); return await prisma.orgMemberInvite.findMany({ where: { organizationId: org.id, inviterId: userId, email: { - in: emails, + in: normalizedEmails, }, }, include: { organization: true, inviter: true, }, }); }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: df7c6ff1-5be8-43d4-b1a7-3885c1397349
📒 Files selected for processing (4)
.server-changes/invite-email-case-insensitive.mdapps/webapp/app/models/member.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsxapps/webapp/app/routes/invite-accept.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). (11)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: typecheck / typecheck
- 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/invite-accept.tsxapps/webapp/app/models/member.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/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/invite-accept.tsxapps/webapp/app/models/member.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/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/invite-accept.tsxapps/webapp/app/models/member.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/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/invite-accept.tsxapps/webapp/app/models/member.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/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/invite-accept.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/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/invite-accept.tsxapps/webapp/app/models/member.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/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/models/member.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/models/member.server.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3545
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.playground.$agentParam/route.tsx:59-59
Timestamp: 2026-05-14T14:56:06.298Z
Learning: In triggerdotdev/trigger.dev, `trigger.dev/core/v3` is a known codebase-wide import convention violation across `apps/webapp`. The correct approach is to use narrow, dedicated subpath imports. Specifically:
- `generateJWT` should be imported from `trigger.dev/core/v3/jwt`
- `MachinePresetName` should be imported from `trigger.dev/core/v3/schemas`
Do not flag individual `trigger.dev/core/v3` occurrences in the webapp as isolated bugs; the full migration is tracked as a follow-up refactor.
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/app/models/member.server.ts:230-245
Timestamp: 2026-05-04T19:14:41.044Z
Learning: In `apps/webapp/app/models/member.server.ts`, the `acceptInvite` function intentionally logs (but does not surface) `rbac.setUserRole` failures. This is safe in v1 because: (1) "Member" is the only plan-gated role and is hidden in v1, and (2) for the exposed roles (Owner/Admin/Developer), any `setUserRole` failure causes the runtime to fall through to the legacy `OrgMember.role → Developer` mapping. Compensating cleanup should be added when the Member role is re-enabled alongside env-tier route wiring (tracked in TRI-9018/9019/9020).
📚 Learning: 2026-05-04T19:14:41.044Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/app/models/member.server.ts:230-245
Timestamp: 2026-05-04T19:14:41.044Z
Learning: In `apps/webapp/app/models/member.server.ts`, the `acceptInvite` function intentionally logs (but does not surface) `rbac.setUserRole` failures. This is safe in v1 because: (1) "Member" is the only plan-gated role and is hidden in v1, and (2) for the exposed roles (Owner/Admin/Developer), any `setUserRole` failure causes the runtime to fall through to the legacy `OrgMember.role → Developer` mapping. Compensating cleanup should be added when the Member role is re-enabled alongside env-tier route wiring (tracked in TRI-9018/9019/9020).
Applied to files:
.server-changes/invite-email-case-insensitive.mdapps/webapp/app/models/member.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx
📚 Learning: 2026-03-26T10:02:25.354Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3254
File: apps/webapp/app/services/platformNotifications.server.ts:363-385
Timestamp: 2026-03-26T10:02:25.354Z
Learning: In `triggerdotdev/trigger.dev`, the `getNextCliNotification` fallback in `apps/webapp/app/services/platformNotifications.server.ts` intentionally uses `prisma.orgMember.findFirst` (single org) when no `projectRef` is provided. This is acceptable for v1 because the CLI (`dev` and `login` commands) always passes `projectRef` in normal usage, making the fallback a rare edge case. Do not flag the single-org fallback as a multi-org correctness bug in this file.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2025-09-03T14:35:52.384Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2464
File: apps/webapp/app/utils/pathBuilder.ts:144-146
Timestamp: 2025-09-03T14:35:52.384Z
Learning: In the trigger.dev codebase, organization slugs are safe for URL query parameters and don't require URL encoding, as confirmed by the maintainer in apps/webapp/app/utils/pathBuilder.ts.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx
📚 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 {packages/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webapp
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx
🪛 LanguageTool
.server-changes/invite-email-case-insensitive.md
[style] ~6-~6: The noun “invitation” is usually used instead of ‘invite’ in formal writing.
Context: ...mails case-insensitively. Previously an invite created with different casing than the ...
(AN_INVITE)
🔇 Additional comments (8)
.server-changes/invite-email-case-insensitive.md (1)
1-12: LGTM!apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx (1)
152-152: LGTM!apps/webapp/app/models/member.server.ts (5)
159-159: LGTM!
183-183: LGTM!
218-218: LGTM!
262-262: LGTM!
272-272: LGTM!apps/webapp/app/routes/invite-accept.tsx (1)
37-37: LGTM!
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/webapp/app/models/member.server.ts (2)
113-130: ⚖️ Poor tradeoffDefensively normalize emails in
inviteMembersto prevent case-variant duplicates.The function currently relies on the caller to lowercase emails (done at the form layer per stack context), but
inviteMembersis an exported function that could be called directly from other routes or services. If invoked with mixed-case emails like['user@example.com', 'User@example.com']:
new Set(emails)deduplicates case-sensitively, keeping both variants- If the database unique constraint is case-sensitive (standard Postgres),
skipDuplicates: trueonly prevents exact matches, so both variants would be inserted- Users would see duplicate invites until the cleanup in
acceptInviteremoves them🛡️ Proposed defensive normalization
const invites = [...new Set(emails)].map( - (email) => + (email) => { + const normalizedEmail = email.trim().toLowerCase(); + return ( ({ - email, + email: normalizedEmail, token: tokenGenerator(), organizationId: org.id, inviterId: userId, role: "MEMBER", rbacRoleId: rbacRoleId ?? null, - } satisfies Prisma.OrgMemberInviteCreateManyInput) + } satisfies Prisma.OrgMemberInviteCreateManyInput); + } );Also normalize the lookup to match:
return await prisma.orgMemberInvite.findMany({ where: { organizationId: org.id, email: { - in: emails, + in: emails.map(e => e.trim().toLowerCase()), }, },
217-224: 💤 Low valueClarify that this step cleans up additional case-variant duplicates.
The comment is accurate but could be more explicit. The
deleteon line 182 already removed the invite being accepted; thisdeleteManyspecifically targets other case-variant duplicates within the same org (e.g., if invites were sent to bothuser@example.comandUser@example.combefore normalization).📝 Suggested comment revision
- // 4. Consume any case-variant duplicate invites for this org (rows - // created before invite emails were lowercased) + // 4. Clean up any additional case-variant duplicate invites for this org + // (e.g., both 'user@example.com' and 'User@example.com' rows created + // before normalization; the accepted invite was already deleted above) await tx.orgMemberInvite.deleteMany({
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c1b40dbe-e9d7-4c80-9230-52f69c19020a
📒 Files selected for processing (2)
.server-changes/invite-email-case-insensitive.mdapps/webapp/app/models/member.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). (12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: audit
- GitHub Check: Analyze (javascript-typescript)
🧰 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 insteadImport from
@trigger.dev/sdkwhen writing Trigger.dev tasks. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob
Files:
apps/webapp/app/models/member.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/member.server.ts
**/*.{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/models/member.server.ts
**/*.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/models/member.server.ts
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/models/member.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/models/member.server.ts
**/*.{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/models/member.server.ts
🧠 Learnings (19)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/app/models/member.server.ts:230-245
Timestamp: 2026-05-04T19:14:41.044Z
Learning: In `apps/webapp/app/models/member.server.ts`, the `acceptInvite` function intentionally logs (but does not surface) `rbac.setUserRole` failures. This is safe in v1 because: (1) "Member" is the only plan-gated role and is hidden in v1, and (2) for the exposed roles (Owner/Admin/Developer), any `setUserRole` failure causes the runtime to fall through to the legacy `OrgMember.role → Developer` mapping. Compensating cleanup should be added when the Member role is re-enabled alongside env-tier route wiring (tracked in TRI-9018/9019/9020).
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3254
File: apps/webapp/app/services/platformNotifications.server.ts:363-385
Timestamp: 2026-03-26T10:02:25.354Z
Learning: In `triggerdotdev/trigger.dev`, the `getNextCliNotification` fallback in `apps/webapp/app/services/platformNotifications.server.ts` intentionally uses `prisma.orgMember.findFirst` (single org) when no `projectRef` is provided. This is acceptable for v1 because the CLI (`dev` and `login` commands) always passes `projectRef` in normal usage, making the fallback a rare edge case. Do not flag the single-org fallback as a multi-org correctness bug in this file.
📚 Learning: 2026-05-04T19:14:41.044Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/app/models/member.server.ts:230-245
Timestamp: 2026-05-04T19:14:41.044Z
Learning: In `apps/webapp/app/models/member.server.ts`, the `acceptInvite` function intentionally logs (but does not surface) `rbac.setUserRole` failures. This is safe in v1 because: (1) "Member" is the only plan-gated role and is hidden in v1, and (2) for the exposed roles (Owner/Admin/Developer), any `setUserRole` failure causes the runtime to fall through to the legacy `OrgMember.role → Developer` mapping. Compensating cleanup should be added when the Member role is re-enabled alongside env-tier route wiring (tracked in TRI-9018/9019/9020).
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2026-03-26T10:02:25.354Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3254
File: apps/webapp/app/services/platformNotifications.server.ts:363-385
Timestamp: 2026-03-26T10:02:25.354Z
Learning: In `triggerdotdev/trigger.dev`, the `getNextCliNotification` fallback in `apps/webapp/app/services/platformNotifications.server.ts` intentionally uses `prisma.orgMember.findFirst` (single org) when no `projectRef` is provided. This is acceptable for v1 because the CLI (`dev` and `login` commands) always passes `projectRef` in normal usage, making the fallback a rare edge case. Do not flag the single-org fallback as a multi-org correctness bug in this file.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2026-04-13T21:44:00.032Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:24-67
Timestamp: 2026-04-13T21:44:00.032Z
Learning: In `apps/webapp/app/services/taskIdentifierRegistry.server.ts`, the sequential upsert/updateMany/findMany writes in `syncTaskIdentifiers` are intentionally NOT wrapped in a Prisma transaction. This function runs only during deployment-change events (low-concurrency path), and any partial `isInLatestDeployment` state is acceptable because it self-corrects on the next deployment. Do not flag this as a missing-transaction/atomicity issue in future reviews.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2026-05-09T08:07:47.468Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/app/routes/api.v1.sessions.ts:49-55
Timestamp: 2026-05-09T08:07:47.468Z
Learning: In triggerdotdev/trigger.dev, the `GET /api/v1/sessions` route (`apps/webapp/app/routes/api.v1.sessions.ts`) has a known deferred security concern: when multiple `filter[taskIdentifier]` values are requested under a per-task-scoped JWT (`read:tasks:<id>`), `anyResource` OR semantics grant access but the repository then lists sessions for ALL requested task IDs, leaking data beyond the JWT's permitted scope. The fix (either a multi-task-filter → require `read:sessions` collection-scope guard at the `apiBuilder` level, or intersecting the filter with JWT-permitted task IDs before the repository call) requires surfacing permitted-task-IDs from `RbacAbility`, and is tracked for a separate PR as part of the broader `anyResource` semantics work.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2026-03-13T13:42:25.092Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3213
File: apps/webapp/app/routes/admin.llm-models.new.tsx:65-91
Timestamp: 2026-03-13T13:42:25.092Z
Learning: In `apps/webapp/app/routes/admin.llm-models.new.tsx`, sequential Prisma writes for model/tier creation are intentionally not wrapped in a transaction. The form is admin-only with low concurrency risk, and the blast radius is considered minimal for admin tooling.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2026-05-04T19:14:44.097Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/test/auth-api.e2e.full.test.ts:205-227
Timestamp: 2026-05-04T19:14:44.097Z
Learning: In triggerdotdev/trigger.dev's e2e auth test suite (`apps/webapp/test/auth-api.e2e.full.test.ts` and related `*.e2e.full.test.ts` files), loose negative assertions like `expect(res.status).not.toBe(200)` are intentional. External infrastructure (e.g. ClickHouse) is unreachable in the e2e test environment, so a 5xx from the route handler after auth passes is an expected and acceptable outcome. Tightening these to a specific set like `[401, 403, 404]` would incorrectly exclude valid 5xx results. Do not flag these as issues during review.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2026-05-08T09:27:50.797Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3529
File: packages/cli-v3/src/executions/taskRunProcess.ts:216-220
Timestamp: 2026-05-08T09:27:50.797Z
Learning: In triggerdotdev/trigger.dev (`packages/cli-v3/src/executions/taskRunProcess.ts`), stale `_currentExecution` / `_isPreparedForNextAttempt` after an error-path rejection (e.g. `#rejectPendingAttempts`) is benign: `#gracefullyTerminate` immediately calls `kill()`, which synchronously sets `_isBeingKilled = true`, and the `isHealthy` getter returns `false` whenever `isBeingKilled` is true — preventing any caller from reusing the process. Both known callers (`dev-run-controller.ts` ~lines 516-567 and `execution.ts` ~lines 538-591) also handle the error and discard the process instance. Do not flag missing cleanup of these fields on error paths in this class.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2026-05-15T08:05:57.683Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3625
File: apps/webapp/app/services/taskMetadataCache.server.ts:270-291
Timestamp: 2026-05-15T08:05:57.683Z
Learning: In the triggerdotdev/trigger.dev codebase, `populateByCurrentWorker()` in `apps/webapp/app/services/taskMetadataCache.server.ts` intentionally logs and swallows Redis errors rather than rethrowing. The design rationale: rethrowing would propagate into `ChangeCurrentDeploymentService.call` and break deploy promotion when Redis is briefly unavailable; the 24h `TASK_META_CACHE_CURRENT_ENV_TTL_SECONDS` TTL acts as the self-healing window for cache drift, and next-promotion overwrites the env key sooner in practice. A compensating DEL on failure is also not a win because if Redis is unreachable the DEL fails identically, and Lua scripts are atomic so a partial write is impossible. Do not flag this log+swallow pattern as a bug in future reviews.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 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/models/member.server.ts
📚 Learning: 2026-04-07T14:12:59.018Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3331
File: apps/webapp/app/runEngine/concerns/batchPayloads.server.ts:112-136
Timestamp: 2026-04-07T14:12:59.018Z
Learning: In `apps/webapp/app/runEngine/concerns/batchPayloads.server.ts`, the `pRetry` call wrapping `uploadPacketToObjectStore` intentionally retries **all** error types (no `shouldRetry` filter / `AbortError` guards). The maintainer explicitly prefers over-retrying to under-retrying because multiple heterogeneous object store backends are supported and it is impractical to enumerate all permanent error signatures. Do not flag this as an issue in future reviews.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 Learning: 2026-04-16T14:21:17.695Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/components/logs/LogsTaskFilter.tsx:135-163
Timestamp: 2026-04-16T14:21:17.695Z
Learning: In `triggerdotdev/trigger.dev` PR `#3368`, the `TaskIdentifier` table has a `@unique([runtimeEnvironmentId, slug])` DB constraint, guaranteeing one row per (environment, slug). In components like `apps/webapp/app/components/logs/LogsTaskFilter.tsx` and `apps/webapp/app/components/runs/v3/RunFilters.tsx`, using `key={item.slug}` for SelectItem list items is correct and unique. Do NOT flag `key={item.slug}` as potentially non-unique — the old duplicate-(slug, triggerSource) issue only existed with the legacy `DISTINCT` query, which this registry replaces.
Applied to files:
apps/webapp/app/models/member.server.ts
📚 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/models/member.server.ts
📚 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/models/member.server.ts
📚 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/models/member.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/models/member.server.ts
📚 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/models/member.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/models/member.server.ts
📚 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/invite-email-case-insensitive.md
🪛 LanguageTool
.server-changes/invite-email-case-insensitive.md
[style] ~14-~14: The noun “invitation” is usually used instead of ‘invite’ in formal writing.
Context: ...e invite rows still work. Accepting an invite now also consumes any case-variant dupl...
(AN_INVITE)
🔇 Additional comments (4)
.server-changes/invite-email-case-insensitive.md (1)
1-18: LGTM!apps/webapp/app/models/member.server.ts (3)
158-171: LGTM!
180-239: LGTM!
261-289: LGTM!
| // Re-inviting an already-invited email is treated as a resend: skip the | ||
| // conflicting insert and return the existing invite below. | ||
| await prisma.orgMemberInvite.createMany({ | ||
| data: invites, | ||
| skipDuplicates: true, | ||
| }); |
There was a problem hiding this comment.
🚩 skipDuplicates silently preserves original invite's rbacRoleId on re-invite
When User B re-invites an email that User A already invited with a different RBAC role, skipDuplicates: true at line 129 silently drops User B's insert (including their rbacRoleId). The findMany then returns User A's invite with User A's rbacRoleId. The route handler's success message says "1 member invited" without indicating the role wasn't applied. The invitee would ultimately join with User A's originally-assigned role, not User B's chosen role. This is a consequence of the resend-as-skip design — not a data corruption issue, but could be surprising if role selection was important to User B.
Was this helpful? React with 👍 or 👎 to provide feedback.
Org member invites couldn't be accepted when the invite email's casing didn't match the invitee's account email (for example,
User@example.comvsuser@example.com). The accept route compared emails strictly, and the pending-invite lookups used exact matching, so the invite never appeared for the invitee in the first place.