fix: add invited members to agent's team using stored team_id#302
Conversation
Pass the agent's team_id through the invite form as a hidden field instead of re-querying the agent on submit. The team_id is already available from the parent route's loader data. Closes #301 Co-authored-by: Heinrich Wendel <heiwen@users.noreply.github.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAgent Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (Console UI)"
participant Route as "Route Action\n(_members/route.tsx)"
participant AuthSvc as "AuthService\n(better-auth)"
participant AuthAPI as "Auth API\n(authClient.organization)"
User->>Route: Submit invite form (email, role, hidden teamId)
Route->>AuthSvc: authService.inviteMember(email, role, teamId)
AuthSvc->>AuthAPI: organization.inviteMember({ email, role, resend: true, ...(teamId && { teamId }) })
AuthAPI-->>AuthSvc: 200 OK / invite result
AuthSvc-->>Route: result / success
Route-->>User: render success / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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/console/app/routes/_shell.agent.$agentSlug.settings.members/route.tsx (1)
15-21: Consider addingteamIdto the invite schema for defense-in-depth validation.The
teamIdis read directly fromformDataand bypasses Zod validation (theinviteSchemaonly validatesrole). While the auth server should validate team membership permissions, adding format validation here provides an extra layer of protection against malformed input.💡 Optional: extend inviteSchema
In
invite-schema.ts:export const inviteSchema = z.object({ email: z.email("Enter a valid email address"), role: z.enum(["member", "admin"]), + teamId: z.string().uuid().optional(), });Then in the route:
- const teamId = formData.get("teamId"); try { await authService.inviteMember( submission.value.email, submission.value.role, - typeof teamId === "string" ? teamId : undefined, + submission.value.teamId, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/console/app/routes/_shell.agent`.$agentSlug.settings.members/route.tsx around lines 15 - 21, The route currently reads teamId from formData and passes it to authService.inviteMember without Zod validation; update the invite schema (inviteSchema) to include teamId (e.g., a string().optional() or string().nullable() with any format constraints you want), validate the incoming submission against the extended inviteSchema so submission.value contains teamId, and then pass the validated submission.value.teamId into authService.inviteMember instead of reading formData.get("teamId"); ensure any existing callers and types that use inviteSchema are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/console/app/routes/_shell.agent`.$agentSlug.settings.members/route.tsx:
- Around line 15-21: The route currently reads teamId from formData and passes
it to authService.inviteMember without Zod validation; update the invite schema
(inviteSchema) to include teamId (e.g., a string().optional() or
string().nullable() with any format constraints you want), validate the incoming
submission against the extended inviteSchema so submission.value contains
teamId, and then pass the validated submission.value.teamId into
authService.inviteMember instead of reading formData.get("teamId"); ensure any
existing callers and types that use inviteSchema are updated accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8efcf4bc-b2be-4922-9ef8-31a151799593
📒 Files selected for processing (7)
apps/api/prisma/schema.prismaapps/console/app/lib/auth/better-auth.tsapps/console/app/lib/auth/dummy-auth.tsapps/console/app/lib/auth/types.tsapps/console/app/routes/_shell.agent.$agentSlug.settings.members/route.tsxapps/console/app/routes/_shell.agent.$agentSlug.settings/members.tsxapps/console/app/routes/_shell.agent.$agentSlug.settings/route.tsx
|
@turisanapo for review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03d8b9bbdf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@claude what do you think about the latest comments? |
|
Claude finished @heiwen's task in 54s —— View job Assessment of review comments
Here's my take on each comment: 1. CodeRabbit — Add Agree, worth doing. Right now 2. Codex P1 — Invitation list not scoped to agent team Valid observation, but out of scope for this PR. The However, this PR's goal is narrowly to pass 3. Codex P2 — Re-inviting doesn't update existing invitations Low risk, but worth noting. This is a Better Auth behavior question — whether Summary
The CodeRabbit suggestion is the only one I'd act on in this PR. The Codex comments are valid architectural observations but they're not regressions introduced here — they're pre-existing limitations that deserve their own issues. Let me know if you'd like me to implement the schema change. |
|
@claude for (1), how do we handle that in other forms? I think (2) is worth fixing. let's be a good citizen and add resend for (3). |
|
Claude finished @heiwen's task in 2m 10s —— View job Addressing review feedback
Changes made1. Schema validation for 2. Scope invitations to agent team (P1) — Three changes:
3. Resend support (P2) — Added View job | |
- Add teamId to inviteSchema for Zod validation instead of raw formData - Add teamId to OrgInvitation type and preserve it from Better Auth response - Filter displayed invitations by agent's team_id - Add resend: true to inviteMember to update existing pending invitations Co-authored-by: Heinrich Wendel <heiwen@users.noreply.github.com>
|
@claude you are scoping to the teamId in the frontend. Are you sure that is the right / secure way to do things? |
|
Claude finished @heiwen's task in 1m 2s —— View job Security review: frontend vs backend scoping of invitations
Good catch — you're right to question this. There are two separate concerns here: 1. Invitation display filtering is client-side only ( const invitations = allInvitations.filter((i) => i.teamId === agent.team_id);This is purely cosmetic. 2. The <input type="hidden" name="teamId" value={agent.team_id} />A user can trivially change this to any Since this is a client-side SPA (clientLoader / clientAction), there's no server-side route handler where you could independently verify the teamId. But that's exactly why you shouldn't trust the hidden field. Instead: Recommended approach: Derive the
Something like: // In clientAction — read teamId from the parent route's agent data
// instead of from formData
import { unstable_useRoute } from "react-router";
// Or pass it via route context / match dataThe exact mechanism depends on whether React Router's TL;DR: The frontend filtering is fine for UX, but the |
|
@claude got it, we're fine then from a security perspective. But I guess this additional map that you introduced here is unnessecary and the filtering itself should already happen in the client loader of the agent settings page? |
|
Claude finished @heiwen's task in 2m 10s —— View job Simplifying invitation handling
Changes
|
… route Remove redundant .map() in getOrganization() — the cast to OrgInvitation[] is sufficient since the type already includes teamId. Move team-scoped filtering from MembersSettings component up to the settings route component where both loader data and agent context are available. Co-authored-by: Heinrich Wendel <heiwen@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@turisanapo for final review |
Pass the agent's
team_idthrough the invite form as a hidden field instead of re-querying the agent on submit. Theteam_idis already available from the parent route's loader data.Closes #301
Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes