feat(platform): add multi-team agent access control#1399
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
This was referenced Apr 11, 2026
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR implements multi-team agent sharing and access control. It adds a new Convex mutation Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/convex/agents/mutations.ts`:
- Around line 111-146: The updateAgentSharing flow currently writes
client-supplied args.teamIds directly into agentBindings; instead, validate each
team id belongs to args.organizationId before persisting. For function logic
around getOrganizationMember / existing / ctx.db.insert / ctx.db.patch, load
each team record (e.g., findOne/get on "team" by _id) and confirm
team.organizationId === args.organizationId, collect any invalid ids and throw a
descriptive error if any are found, then use the validated list (and
primaryTeamId derived from it) when patching or inserting into agentBindings.
In `@services/platform/convex/documents/list_indexed_documents_for_agent.ts`:
- Around line 85-91: The current logic building agentTeamIdSet treats
args.agentTeamIds as truthy which makes an empty array disable the fallback to
args.agentTeamId; change the checks to test for a non-empty array (e.g.,
args.agentTeamIds && args.agentTeamIds.length > 0) before iterating so that when
agentTeamIds is [] the code falls back to using agentTeamId; update the same
pattern wherever agentTeamIds is checked (the other occurrences noted) to ensure
legacy team fallback works and ensure you still add args.agentTeamId to
agentTeamIdSet when agentTeamIds is absent or empty.
In `@services/platform/convex/lib/agent_chat/internal_actions.ts`:
- Around line 424-428: The teamIds assignment incorrectly treats an empty array
(agentConfig.agentTeamIds === []) as truthy and prevents falling back to
agentConfig.agentTeamId; update the logic in internal_actions.ts where teamIds
is set to first check that agentConfig.agentTeamIds is a non-empty array (e.g.,
agentConfig.agentTeamIds && agentConfig.agentTeamIds.length > 0) and only use it
when non-empty, otherwise fall back to agentConfig.agentTeamId (wrapping it into
an array) or undefined; ensure you modify the expression that currently reads
agentConfig.agentTeamIds ?? (agentConfig.agentTeamId ? [agentConfig.agentTeamId]
: undefined) to the non-empty-array check using the unique symbols
agentConfig.agentTeamIds, agentConfig.agentTeamId and teamIds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0eccfa4-88cd-46b6-8688-7d254889535b
📒 Files selected for processing (22)
services/platform/app/features/agents/hooks/mutations.tsservices/platform/convex/agent_tools/approval_shared.tsservices/platform/convex/agent_tools/human_input/mutations.tsservices/platform/convex/agent_tools/location/mutations.tsservices/platform/convex/agent_tools/rag/helpers/list_indexed_documents.tsservices/platform/convex/agent_tools/rag/rag_search_tool.tsservices/platform/convex/agents/__tests__/access.test.tsservices/platform/convex/agents/access.tsservices/platform/convex/agents/config.tsservices/platform/convex/agents/file_actions.tsservices/platform/convex/agents/internal_queries.tsservices/platform/convex/agents/mutations.tsservices/platform/convex/agents/queries.tsservices/platform/convex/agents/schema.tsservices/platform/convex/documents/get_agent_scoped_file_ids.tsservices/platform/convex/documents/internal_queries.tsservices/platform/convex/documents/list_indexed_documents_for_agent.tsservices/platform/convex/lib/agent_chat/internal_actions.tsservices/platform/convex/lib/agent_chat/start_agent_chat.tsservices/platform/convex/lib/agent_chat/types.tsservices/platform/convex/lib/agent_response/generate_response.tsservices/platform/convex/lib/agent_response/types.ts
Comment on lines
+111
to
+146
| const member = await getOrganizationMember(ctx, args.organizationId, { | ||
| userId: String(authUser._id), | ||
| email: authUser.email, | ||
| name: authUser.name, | ||
| }); | ||
|
|
||
| const role = member.role ?? 'member'; | ||
| if (role !== 'owner' && role !== 'admin') { | ||
| throw new Error('Only admins can update agent sharing'); | ||
| } | ||
|
|
||
| const existing = await ctx.db | ||
| .query('agentBindings') | ||
| .withIndex('by_org_agent', (q) => | ||
| q | ||
| .eq('organizationId', args.organizationId) | ||
| .eq('agentSlug', args.agentSlug), | ||
| ) | ||
| .first(); | ||
|
|
||
| const primaryTeamId = args.teamIds[0] ?? undefined; | ||
| const sharedWithTeamIds = | ||
| args.teamIds.length > 0 ? args.teamIds : undefined; | ||
|
|
||
| if (existing) { | ||
| await ctx.db.patch(existing._id, { | ||
| teamId: primaryTeamId, | ||
| sharedWithTeamIds, | ||
| }); | ||
| } else { | ||
| await ctx.db.insert('agentBindings', { | ||
| organizationId: args.organizationId, | ||
| agentSlug: args.agentSlug, | ||
| teamId: primaryTeamId, | ||
| sharedWithTeamIds, | ||
| }); |
There was a problem hiding this comment.
Validate each shared team against the organization before writing.
updateAgentSharing writes client-supplied teamIds straight into agentBindings without checking that those teams actually belong to args.organizationId. That allows stale or cross-org IDs to be persisted and makes the sharing state invalid. Please load each team first and reject any ID whose owning org does not match.
Based on learnings, addMember validates the target team belongs to the provided organization (findOne on team by _id, compare organizationId) and keeps this org-ownership validation for team-member mutations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/agents/mutations.ts` around lines 111 - 146, The
updateAgentSharing flow currently writes client-supplied args.teamIds directly
into agentBindings; instead, validate each team id belongs to
args.organizationId before persisting. For function logic around
getOrganizationMember / existing / ctx.db.insert / ctx.db.patch, load each team
record (e.g., findOne/get on "team" by _id) and confirm team.organizationId ===
args.organizationId, collect any invalid ids and throw a descriptive error if
any are found, then use the validated list (and primaryTeamId derived from it)
when patching or inserting into agentBindings.
Comment on lines
+85
to
+91
| // Build effective team set: prefer agentTeamIds, fall back to single agentTeamId | ||
| const agentTeamIdSet = new Set<string>(); | ||
| if (args.agentTeamIds) { | ||
| for (const id of args.agentTeamIds) agentTeamIdSet.add(id); | ||
| } else if (args.agentTeamId) { | ||
| agentTeamIdSet.add(args.agentTeamId); | ||
| } |
There was a problem hiding this comment.
Empty agentTeamIds currently disables legacy team fallback.
if (args.agentTeamIds) treats [] as truthy, so agentTeamId is ignored and team-scoped docs can be dropped unexpectedly.
Proposed fix
- if (args.agentTeamIds) {
+ if (args.agentTeamIds && args.agentTeamIds.length > 0) {
for (const id of args.agentTeamIds) agentTeamIdSet.add(id);
} else if (args.agentTeamId) {
agentTeamIdSet.add(args.agentTeamId);
}Also applies to: 94-94, 131-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/documents/list_indexed_documents_for_agent.ts`
around lines 85 - 91, The current logic building agentTeamIdSet treats
args.agentTeamIds as truthy which makes an empty array disable the fallback to
args.agentTeamId; change the checks to test for a non-empty array (e.g.,
args.agentTeamIds && args.agentTeamIds.length > 0) before iterating so that when
agentTeamIds is [] the code falls back to using agentTeamId; update the same
pattern wherever agentTeamIds is checked (the other occurrences noted) to ensure
legacy team fallback works and ensure you still add args.agentTeamId to
agentTeamIdSet when agentTeamIds is absent or empty.
Comment on lines
+424
to
+428
| teamIds: | ||
| agentConfig.agentTeamIds ?? | ||
| (agentConfig.agentTeamId | ||
| ? [agentConfig.agentTeamId] | ||
| : undefined), |
There was a problem hiding this comment.
Handle empty agentTeamIds before legacy fallback.
If agentTeamIds is [], the current expression still picks it and suppresses fallback to agentTeamId, which can unexpectedly drop team-scoped access.
Proposed fix
- teamIds:
- agentConfig.agentTeamIds ??
- (agentConfig.agentTeamId
- ? [agentConfig.agentTeamId]
- : undefined),
+ teamIds:
+ agentConfig.agentTeamIds && agentConfig.agentTeamIds.length > 0
+ ? agentConfig.agentTeamIds
+ : agentConfig.agentTeamId
+ ? [agentConfig.agentTeamId]
+ : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/lib/agent_chat/internal_actions.ts` around lines 424
- 428, The teamIds assignment incorrectly treats an empty array
(agentConfig.agentTeamIds === []) as truthy and prevents falling back to
agentConfig.agentTeamId; update the logic in internal_actions.ts where teamIds
is set to first check that agentConfig.agentTeamIds is a non-empty array (e.g.,
agentConfig.agentTeamIds && agentConfig.agentTeamIds.length > 0) and only use it
when non-empty, otherwise fall back to agentConfig.agentTeamId (wrapping it into
an array) or undefined; ensure you modify the expression that currently reads
agentConfig.agentTeamIds ?? (agentConfig.agentTeamId ? [agentConfig.agentTeamId]
: undefined) to the non-empty-array check using the unique symbols
agentConfig.agentTeamIds, agentConfig.agentTeamId and teamIds.
Add sharedWithTeamIds field to agentBindings schema for multi-team agent scoping. Agents can now be shared with multiple teams instead of just one. The access model: org-wide (no teams) or team-scoped (visible only to members of assigned teams). Admins always have full access. - Add access.ts helper with checkAgentAccess and getAgentTeamIds - Add updateAgentSharing mutation with admin-only enforcement - Add listBindingsByOrg query and internal listBindingsForOrg - Update document scoping (getAgentScopedFileIds, listIndexedDocumentsForAgent) to use agentTeamIds array instead of single agentTeamId - Thread agentTeamIds through agent chat/response pipeline and all agent tools (RAG, approval, human input, location) - Add useUpdateAgentSharing frontend mutation hook - Add comprehensive tests for access control logic
larryro
force-pushed
the
feat/issue-1343-agent-access
branch
from
6f46b77 to
f54f078
Compare
…d permissions Owning team members get canUse + canEdit, shared team members get canUse only. updateAgentSharing no longer overwrites teamId. Agent settings UI shows create-team guidance when no teams exist and a multi-select for sharing when an owning team is set.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sharedWithTeamIdsfield toagentBindingsschema enabling agents to be shared with multiple teams (backward-compatible: agents with no teams remain org-wide)access.tshelper module withcheckAgentAccessandgetAgentTeamIdsfor centralized access control logicupdateAgentSharingmutation (admin-only) andlistBindingsByOrgquery for managing and reading team assignmentsgetAgentScopedFileIds,listIndexedDocumentsForAgent) to use full team set instead of single team IDagentTeamIdsthrough the agent chat/response pipeline, budget checks, and all agent tools (RAG search, approval, human input, location)useUpdateAgentSharingfrontend mutation hookTest plan
bun run --filter @tale/platform lint)bun run --filter @tale/platform typecheck)sharedWithTeamIdscontinue to work as org-wideupdateAgentSharingmutation rejects non-admin callersCloses #1343
Summary by CodeRabbit
Release Notes