Skip to content

Conversation

myftija
Copy link
Member

@myftija myftija commented Sep 25, 2025

Fixes some scoping issues with team invites.

Fixes some scoping issues with team invites.
Copy link

changeset-bot bot commented Sep 25, 2025

⚠️ No Changeset found

Latest commit: 02d3e02

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Invite-related server methods were updated to enforce user-scoped operations and include email where needed: acceptInvite and declineInvite now accept a user object ({id, email}); resendInvite now requires an additional userId; revokeInvite now uses orgSlug and verifies membership by organization slug. invite-accept loader now requires authentication before token fetch. Several route modules changed to pass the new parameters (user or userId/orgSlug) and switched some imports to type-only for Remix Action/Loader types.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description consists of a single sentence and does not follow the required template, missing the issue reference, checklist, testing steps, changelog summary, and screenshots sections. Please update the description to include “Closes #”, complete the checklist, detail the testing steps taken, add a concise changelog entry, and include any relevant screenshots as specified in the repository template.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely summarizes the primary change by indicating a fix to organization invite scoping within the webapp and follows the conventional commit format, making it immediately understandable to reviewers.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-invite-scopes

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61b7219 and 02d3e02.

📒 Files selected for processing (3)
  • apps/webapp/app/models/member.server.ts (5 hunks)
  • apps/webapp/app/routes/invite-accept.tsx (1 hunks)
  • apps/webapp/app/routes/invites.tsx (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/webapp/app/routes/invite-accept.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • apps/webapp/app/routes/invites.tsx
  • apps/webapp/app/models/member.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

  • apps/webapp/app/routes/invites.tsx
  • apps/webapp/app/models/member.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json

Files:

  • apps/webapp/app/routes/invites.tsx
  • apps/webapp/app/models/member.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly

Files:

  • apps/webapp/app/models/member.server.ts
apps/webapp/app/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead

Files:

  • apps/webapp/app/models/member.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/routes/invites.tsx (1)
apps/webapp/app/services/session.server.ts (1)
  • requireUser (39-62)
apps/webapp/app/models/member.server.ts (1)
apps/webapp/app/presenters/OrganizationsPresenter.server.ts (1)
  • userId (126-168)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/webapp/app/models/member.server.ts (1)

213-232: Keep the decline flow inside the transaction (and reuse the fetched invite).

We’re inside prisma.$transaction, but both the delete and the follow-up read still hit the global client. That executes outside the transaction context, so the unit of work you intended to keep atomic can interleave with other writes. This was already raised earlier and is still unresolved.

Please stick to the transactional client and reuse the invite you fetch for the email check:

-    const declinedInvite = await prisma.orgMemberInvite.delete({
-      where: {
-        id: inviteId,
-        email: user.email,
-      },
-      include: {
-        organization: true,
-      },
-    });
-
-    //2. check for other invites
-    const remainingInvites = await prisma.orgMemberInvite.findMany({
+    const invite = await tx.orgMemberInvite.findUnique({
+      where: { id: inviteId },
+      include: { organization: true },
+    });
+    if (!invite || invite.email !== user.email) {
+      throw new Error("Invite not found or you are not the recipient");
+    }
+
+    await tx.orgMemberInvite.delete({ where: { id: invite.id } });
+
+    //2. check for other invites
+    const remainingInvites = await tx.orgMemberInvite.findMany({
       where: {
         email: user.email,
       },
     });
 
-    return { remainingInvites, organization: declinedInvite.organization };
+    return { remainingInvites, organization: invite.organization };
apps/webapp/app/routes/invites.tsx (2)

3-3: Type-only import alignment looks good

Consistent with our Remix typing practices and keeps the runtime bundle clean.


39-68: Passing the full user context resolves the invite scope issues

Switching to requireUser and handing downstream helpers the { id, email } payload is exactly what those model updates expect, so accept/decline now scope correctly to the signed-in member.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/webapp/app/models/member.server.ts (1)

262-276: Blocker: invalid Prisma update where; add explicit access check then update

update(where: …) must be unique; adding inviterId in where isn’t supported unless it’s part of a unique constraint (and id already is unique). Do a two‑step access check and then update by id.

Replace the function body:

 export async function resendInvite({ inviteId, userId }: { inviteId: string; userId: string }) {
-  return await prisma.orgMemberInvite.update({
-    where: {
-      id: inviteId,
-      inviterId: userId,
-    },
-    data: {
-      updatedAt: new Date(),
-    },
-    include: {
-      inviter: true,
-      organization: true,
-    },
-  });
+  const invite = await prisma.orgMemberInvite.findFirst({
+    where: {
+      id: inviteId,
+      // Require requester to be a member; adjust policy if only original inviter can resend:
+      organization: { members: { some: { userId } } },
+    },
+    include: { inviter: true, organization: true },
+  });
+  if (!invite) throw new Error("Invite not found or you don't have access");
+  return await prisma.orgMemberInvite.update({
+    where: { id: invite.id },
+    data: { updatedAt: new Date() },
+    include: { inviter: true, organization: true },
+  });
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e22c321 and 61b7219.

📒 Files selected for processing (4)
  • apps/webapp/app/models/member.server.ts (6 hunks)
  • apps/webapp/app/routes/invite-accept.tsx (1 hunks)
  • apps/webapp/app/routes/invite-resend.tsx (2 hunks)
  • apps/webapp/app/routes/invite-revoke.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • apps/webapp/app/routes/invite-resend.tsx
  • apps/webapp/app/routes/invite-revoke.tsx
  • apps/webapp/app/routes/invite-accept.tsx
  • apps/webapp/app/models/member.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

  • apps/webapp/app/routes/invite-resend.tsx
  • apps/webapp/app/routes/invite-revoke.tsx
  • apps/webapp/app/routes/invite-accept.tsx
  • apps/webapp/app/models/member.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json

Files:

  • apps/webapp/app/routes/invite-resend.tsx
  • apps/webapp/app/routes/invite-revoke.tsx
  • apps/webapp/app/routes/invite-accept.tsx
  • apps/webapp/app/models/member.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly

Files:

  • apps/webapp/app/models/member.server.ts
apps/webapp/app/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead

Files:

  • apps/webapp/app/models/member.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/routes/invite-accept.tsx (2)
apps/webapp/app/models/message.server.ts (1)
  • redirectWithSuccessMessage (162-179)
apps/webapp/app/models/member.server.ts (1)
  • getInviteFromToken (123-140)
apps/webapp/app/models/member.server.ts (1)
apps/webapp/app/presenters/OrganizationsPresenter.server.ts (1)
  • userId (126-168)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
apps/webapp/app/routes/invite-accept.tsx (1)

21-25: Good: require auth before fetching the invite

Early redirect for unauthenticated users avoids leaking invite details.

apps/webapp/app/routes/invite-resend.tsx (2)

2-2: Type-only import LGTM

Type-only ActionFunction import is correct.


26-29: Use env via env.server.ts (don’t import from process)

Per project guidelines for apps/webapp/app routes, access environment variables via env from app/env.server.ts, not Node’s process env shim.

Change the import and usage:

// replace
import { env } from "process";
// with
import { env } from "~/env.server";

[ suggest_essential_refactor ]

apps/webapp/app/routes/invite-revoke.tsx (2)

2-2: Type-only import LGTM

Consistent with other routes.


25-29: Param rename to orgSlug is correct

Mapping submission.value.slug to orgSlug matches the updated server API.

apps/webapp/app/models/member.server.ts (3)

205-212: Remove membership filter from remainingInvites in accept flow

After accepting, remainingInvites should be filtered by email; requiring membership would often return zero and is unnecessary.

-        organization:
-          members:
-            some:
-              userId,
+        // No membership gating here; the recipient may not be a member yet

[ suggest_recommended_refactor ]


248-254: Remove membership filter from decline remainingInvites

Same reasoning as accept: filter by email; no membership gating needed.

-        organization:
-          members:
-            some:
-              userId,
+        // No membership constraint here

[ suggest_recommended_refactor ]


305-310: Nit: unreachable null-check after delete

prisma.delete throws if the record doesn’t exist. The if (!invite) check is unreachable once you adopt the findFirst + delete flow above. Remove it.
[ suggest_optional_refactor ]

@myftija myftija changed the title fix: org invite scoping fix(webapp): org invite scoping Sep 25, 2025
@myftija myftija merged commit a3cea13 into main Sep 25, 2025
31 checks passed
@myftija myftija deleted the fix-invite-scopes branch September 25, 2025 17:17
samejr pushed a commit that referenced this pull request Oct 4, 2025
* fix: org invite scoping

Fixes some scoping issues with team invites.

* Fix invite flow changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants