feat: migrate SDK to use SetProjectMemberRole and RemoveProjectMember RPCs#1508
feat: migrate SDK to use SetProjectMemberRole and RemoveProjectMember RPCs#1508whoAbhishekSah wants to merge 7 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces client-side policy CRUD (listPolicies/deletePolicy/createPolicy/createPolicyForProject) with direct role-based mutations ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Manual Testing — PASSTested locally with client demo app on clean Frontier + SpiceDB.
All flows use |
whoAbhishekSah
requested review from
rohanchkrabrty and
rsbh
and removed request for
rohanchkrabrty
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/sdk/admin/views/organizations/details/projects/members/assign-role.tsx (1)
88-107:⚠️ Potential issue | 🔴 Critical
setProjectMemberRoleonly preserves the last role in this loop.This RPC replaces the member’s project role set; it does not append to it. Iterating
assignedRolesArrhere means each call overwrites the previous one, so only the last role survives whileonRoleUpdatestill reports every selected role locally.Suggested fix
const onSubmit = async (data: FormData) => { try { const assignedRolesArr = Array.from(data.roleIds); + if (assignedRolesArr.length !== 1) { + throw new Error("Project members can only have one direct project role"); + } - for (const roleId of assignedRolesArr) { - await setProjectMemberRole( - create(SetProjectMemberRoleRequestSchema, { - projectId, - principalId: user?.id || "", - principalType: "app/user", - roleId, - }), - ); - } + await setProjectMemberRole( + create(SetProjectMemberRoleRequestSchema, { + projectId, + principalId: user?.id || "", + principalType: "app/user", + roleId: assignedRolesArr[0], + }), + );web/sdk/react/views/projects/details/remove-project-member-dialog.tsx (1)
20-25:⚠️ Potential issue | 🟠 MajorMake
memberTyperequired instead of defaulting to'user'.This makes missing caller updates look valid.
web/sdk/react/views/projects/details/project-detail-page.tsx:255-260still opens the dialog withoutmemberType, so removing a team member will continue to sendapp/userand fail. Making the prop required forces the remaining call sites to pass the correct principal type.Suggested fix
export interface RemoveProjectMemberDialogProps { open: boolean; onOpenChange?: (value: boolean) => void; projectId: string; memberId: string; - memberType?: 'user' | 'group'; + memberType: 'user' | 'group'; } @@ export const RemoveProjectMemberDialog = ({ open, onOpenChange, projectId, memberId, - memberType = 'user' + memberType }: RemoveProjectMemberDialogProps) => {Also applies to: 32-33, 52-57
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 96b266ff-0062-46b1-bc94-f27ec9d10147
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
web/sdk/admin/views/organizations/details/projects/members/assign-role.tsxweb/sdk/admin/views/organizations/details/projects/members/remove-member.tsxweb/sdk/admin/views/organizations/details/projects/use-add-project-members.tsxweb/sdk/package.jsonweb/sdk/react/views/api-keys/details/manage-service-user-projects-dialog.tsxweb/sdk/react/views/api-keys/list/add-service-account-dialog.tsxweb/sdk/react/views/projects/details/project-member-columns.tsxweb/sdk/react/views/projects/details/project-members.tsxweb/sdk/react/views/projects/details/remove-project-member-dialog.tsx
web/sdk/react/views/api-keys/details/manage-service-user-projects-dialog.tsx
Show resolved
Hide resolved
whoAbhishekSah
force-pushed
the
feat/sdk-project-member-management
branch
from
6e6778f to
d873d1e
Compare
Coverage Report for CI Build 24121714139Coverage remained the same at 41.146%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
web/sdk/react/views/api-keys/details/manage-service-user-projects-dialog.tsx (1)
179-223:⚠️ Potential issue | 🟠 MajorAdd
ownerRoleIdto the callback dependencies.
onAccessChangereadsownerRoleIdon Lines 188 and 194, but Line 223 omits it. WhenlistRolesresolves after the dialog opens, the callback can keep the initial empty string and continue throwing “Project owner role not found” on grant attempts until another dependency changes.Suggested fix
- [serviceUserId, setProjectMemberRole, removeProjectMember] + [serviceUserId, ownerRoleId, setProjectMemberRole, removeProjectMember]
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9cfd1e26-ef92-4f05-9d62-76ad0532fd3c
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
web/sdk/admin/views/organizations/details/projects/members/assign-role.tsxweb/sdk/admin/views/organizations/details/projects/members/remove-member.tsxweb/sdk/admin/views/organizations/details/projects/use-add-project-members.tsxweb/sdk/package.jsonweb/sdk/react/views-new/projects/components/add-member-menu.tsxweb/sdk/react/views-new/projects/components/remove-member-dialog.tsxweb/sdk/react/views-new/projects/project-details-view.tsxweb/sdk/react/views/api-keys/details/manage-service-user-projects-dialog.tsxweb/sdk/react/views/api-keys/list/add-service-account-dialog.tsxweb/sdk/react/views/projects/details/project-member-columns.tsxweb/sdk/react/views/projects/details/project-members.tsxweb/sdk/react/views/projects/details/remove-project-member-dialog.tsx
✅ Files skipped from review due to trivial changes (3)
- web/sdk/package.json
- web/sdk/react/views/projects/details/project-member-columns.tsx
- web/sdk/react/views/api-keys/list/add-service-account-dialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/sdk/admin/views/organizations/details/projects/members/remove-member.tsx
| const onSubmit = async (data: FormData) => { | ||
| try { | ||
| const client = createClient(FrontierService, transport); | ||
| const policiesResp = await client.listPolicies( | ||
| create(ListPoliciesRequestSchema, { | ||
| projectId: projectId, | ||
| userId: user?.id, | ||
| }), | ||
| ); | ||
| const policies = policiesResp.policies || []; | ||
|
|
||
| const removedRolesPolicies = policies.filter( | ||
| (policy) => !(policy.roleId && data.roleIds.has(policy.roleId)), | ||
| ); | ||
| await Promise.all( | ||
| removedRolesPolicies.map((policy) => | ||
| deletePolicy( | ||
| create(DeletePolicyRequestSchema, { id: policy.id || "" }), | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| const resource = `app/project:${projectId}`; | ||
| const principal = `app/user:${user?.id}`; | ||
|
|
||
| const assignedRolesArr = Array.from(data.roleIds); | ||
| await Promise.all( | ||
| assignedRolesArr.map((roleId) => | ||
| createPolicy( | ||
| create(CreatePolicyRequestSchema, { | ||
| body: { | ||
| roleId, | ||
| resource, | ||
| principal, | ||
| }, | ||
| }), | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| for (const roleId of assignedRolesArr) { | ||
| await setProjectMemberRole( | ||
| create(SetProjectMemberRoleRequestSchema, { | ||
| projectId, | ||
| principalId: user?.id || "", | ||
| principalType: "app/user", | ||
| roleId, | ||
| }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find setProjectMemberRole definition and usage
rg "setProjectMemberRole" -t ts -t tsx -B 3 -A 3Repository: raystack/frontier
Length of output: 88
🏁 Script executed:
# Find SetProjectMemberRoleRequestSchema definition
rg "SetProjectMemberRoleRequestSchema" -t ts -t tsx -B 5 -A 10Repository: raystack/frontier
Length of output: 88
🏁 Script executed:
# Check the full assign-role.tsx file to understand form flow
find . -name "assign-role.tsx" -path "*/projects/members/*" | head -1 | xargs catRepository: raystack/frontier
Length of output: 5223
🏁 Script executed:
# Look for any role removal or reconciliation logic
rg "removeProjectMemberRole\|deleteProjectMemberRole\|clearRoles" -t ts -t tsx -B 2 -A 2Repository: raystack/frontier
Length of output: 88
🏁 Script executed:
# Find where SetProjectMemberRoleRequestSchema comes from
rg "SetProjectMemberRoleRequestSchema" -t js -t ts --max-count 5 | head -20Repository: raystack/frontier
Length of output: 2236
🏁 Script executed:
# Search for setProjectMemberRole in proto files
find . -name "*.proto" | xargs grep -l "setProjectMemberRole\|SetProjectMemberRole" 2>/dev/nullRepository: raystack/frontier
Length of output: 45
🏁 Script executed:
# Look for the backend implementation in Go (service files)
find . -name "*service*.go" -path "*/project/*" | head -5Repository: raystack/frontier
Length of output: 253
🏁 Script executed:
# Check if there's any API documentation or schema files
find . -name "*.proto" -o -name "*.openapi*" | head -10Repository: raystack/frontier
Length of output: 43
🏁 Script executed:
# Search for the project service implementation
find . -path "*/core/project/*.go" -name "*.go" | xargs ls -la | head -20Repository: raystack/frontier
Length of output: 1152
🏁 Script executed:
# Look for SetProjectMemberRole implementation in Go
rg "SetProjectMemberRole\|setProjectMemberRole" --type go -B 5 -A 15Repository: raystack/frontier
Length of output: 43
🏁 Script executed:
# Check if there's any role removal/deletion in the codebase
rg "RemoveProjectMember\|DeleteProjectMember\|RemoveRole.*Project\|RemoveProjectRole" --type go -B 3 -A 10Repository: raystack/frontier
Length of output: 43
🏁 Script executed:
# Look for any unset/delete operations on project members
rg "unsetProjectMemberRole\|UnsetProjectMemberRole" --type goRepository: raystack/frontier
Length of output: 43
🏁 Script executed:
# Read the project service.go to find SetProjectMemberRole implementation
cat -n ./core/project/service.go | grep -A 30 -i "setprojectmemberrole\|setmemberrole"Repository: raystack/frontier
Length of output: 1279
🏁 Script executed:
# Try broader search for member role methods
cat -n ./core/project/service.go | grep -i "member.*role\|role.*member"Repository: raystack/frontier
Length of output: 335
🏁 Script executed:
# Search for any method that modifies roles
grep -n "func.*Member\|func.*Role" ./core/project/service.go | head -20Repository: raystack/frontier
Length of output: 375
🏁 Script executed:
# Get more context of SetMemberRole to see the complete implementation
sed -n '362,405p' ./core/project/service.goRepository: raystack/frontier
Length of output: 1346
🏁 Script executed:
# Check if there are any tests for SetMemberRole that confirm the behavior
grep -A 20 "SetMemberRole\|setMemberRole" ./core/project/service_test.go | head -50Repository: raystack/frontier
Length of output: 1206
The multi-role selection UI is incompatible with the API's single-role replace semantics.
SetMemberRole deletes all existing policies and creates exactly one (see core/project/service.go:362-405). Since the form submits each selected role separately in a loop, only the last role persists—earlier selections are silently overwritten without error. If a user selects roles [A, B, C], only C will be assigned.
Either switch the dialog to single-select, or implement a proper multi-role endpoint that handles the full desired role set atomically.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| memberType?: 'user' | 'group'; | ||
| } | ||
|
|
||
| export const RemoveProjectMemberDialog = ({ | ||
| open, | ||
| onOpenChange, | ||
| projectId, | ||
| memberId | ||
| memberId, | ||
| memberType = 'user' |
There was a problem hiding this comment.
Make memberType required instead of defaulting to 'user'.
This dialog now supports both users and groups, but the primary caller in web/sdk/react/views/projects/details/project-detail-page.tsx still opens it without memberType. With the default at Lines 25-33, group removals silently take the 'app/user' branch at Lines 52-57 and fail against group memberships.
Also applies to: 52-57
… RPCs Replace policy-based member management (createPolicyForProject, listPolicies, deletePolicy, createPolicy) with the new atomic RPCs. React SDK: - project-members.tsx: addMember/addTeam use SetProjectMemberRole - project-member-columns.tsx: updateRole uses SetProjectMemberRole - remove-project-member-dialog.tsx: uses RemoveProjectMember - add-service-account-dialog.tsx: uses SetProjectMemberRole - manage-service-user-projects-dialog.tsx: uses both RPCs Admin SDK: - use-add-project-members.tsx: uses SetProjectMemberRole - remove-member.tsx: uses RemoveProjectMember - assign-role.tsx: uses SetProjectMemberRole Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ions Fetch project-scoped roles via listRoles and resolve the role UUID before calling SetProjectMemberRole. Consistent with the role change dropdown which already uses UUIDs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AddMemberDropdown is a separate child component that needs the roles array to resolve the viewer role UUID. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… RemoveProjectMember Update the design-revamped SDK views to use the new atomic RPCs: - add-member-menu.tsx: use SetProjectMemberRole with fetched role UUID - remove-member-dialog.tsx: use RemoveProjectMember - project-details-view.tsx: use SetProjectMemberRole for role change Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
whoAbhishekSah
force-pushed
the
feat/sdk-project-member-management
branch
from
c7c1ca0 to
cfeb60b
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
web/sdk/react/views/api-keys/list/add-service-account-dialog.tsx (2)
128-153:⚠️ Potential issue | 🟠 MajorValidate
ownerRoleIdbefore creating the service user.The validation at line 145 occurs after
createServiceUser(line 133). IfownerRoleIdis empty (e.g., roles query is slow or the role name doesn't match), the service user will be created but left orphaned without project access.Suggested fix: validate early
const onSubmit = useCallback( async (data: FormData) => { if (!orgId) return; + if (!ownerRoleId) { + toast.error('Something went wrong', { + description: 'Project owner role not found' + }); + return; + } try { const serviceUserResponse = await createServiceUser( create(CreateServiceUserRequestSchema, { orgId, body: create(ServiceUserRequestBodySchema, { title: data.title }) }) ); const serviceUserId = serviceUserResponse.serviceuser?.id; if (!serviceUserId) return; - if (!ownerRoleId) throw new Error('Project owner role not found'); await setProjectMemberRole(
201-209:⚠️ Potential issue | 🟠 MajorAdd
ownerRoleIdto the dependency array.The callback uses
ownerRoleId(lines 145, 151) but it's missing from the dependency array. This causes a stale closure: the callback captures the initial empty string even afterlistRolesresolves, leading to "Project owner role not found" errors.Suggested fix
[ orgId, + ownerRoleId, createServiceUser, setProjectMemberRole, createServiceUserToken, queryClient, transport, onCreated ]web/sdk/admin/views/organizations/details/projects/use-add-project-members.tsx (1)
68-89:⚠️ Potential issue | 🟡 MinorSilent failure when
viewerRoleIdis missing.The early return at line 70 silently exits without user feedback if
viewerRoleIdis empty (e.g., role not found or query still loading). Users clicking "Add" see no response.Consider adding a toast or ensuring the add button is disabled while roles are loading.
Suggested fix
const addMember = useCallback( async (userId: string) => { - if (!userId || !projectId || !viewerRoleId) return; + if (!userId || !projectId) return; + if (!viewerRoleId) { + toast.error("Unable to add member", { description: "Project viewer role not found" }); + return; + } try {
♻️ Duplicate comments (1)
web/sdk/react/views/api-keys/details/manage-service-user-projects-dialog.tsx (1)
179-224:⚠️ Potential issue | 🟠 Major
onAccessChangecloses over a staleownerRoleId.The callback uses
ownerRoleIdat lines 188 and 194, but it's missing from the dependency array at line 223. When the dialog opens andlistRolespopulates the role ID,onAccessChangestill holds the stale empty string from initial render, causing "Project owner role not found" errors.Suggested fix
- [serviceUserId, setProjectMemberRole, removeProjectMember] + [serviceUserId, ownerRoleId, setProjectMemberRole, removeProjectMember]
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3466a0ec-412a-4b4e-a218-712d1f8d0ba5
📒 Files selected for processing (11)
web/sdk/admin/views/organizations/details/projects/members/assign-role.tsxweb/sdk/admin/views/organizations/details/projects/members/remove-member.tsxweb/sdk/admin/views/organizations/details/projects/use-add-project-members.tsxweb/sdk/react/views-new/projects/components/add-member-menu.tsxweb/sdk/react/views-new/projects/components/remove-member-dialog.tsxweb/sdk/react/views-new/projects/project-details-view.tsxweb/sdk/react/views/api-keys/details/manage-service-user-projects-dialog.tsxweb/sdk/react/views/api-keys/list/add-service-account-dialog.tsxweb/sdk/react/views/projects/details/project-member-columns.tsxweb/sdk/react/views/projects/details/project-members.tsxweb/sdk/react/views/projects/details/remove-project-member-dialog.tsx
✅ Files skipped from review due to trivial changes (2)
- web/sdk/react/views-new/projects/components/remove-member-dialog.tsx
- web/sdk/react/views/projects/details/project-member-columns.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/sdk/react/views-new/projects/components/add-member-menu.tsx
- web/sdk/react/views/projects/details/project-members.tsx
- Fix hardcoded principalType in views-new remove-member-dialog: pass memberType through payload to support group removal - Add ownerRoleId to dependency arrays in manage-service-user-projects and add-service-account dialogs to prevent stale closures - Move ownerRoleId validation before service user creation to avoid orphaned service users Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/membership-package-tasks.md (1)
31-41: Consider adding language specifiers to fenced code blocks.Several code blocks use plain triple-backticks without a language identifier. While these are pseudo-code/call-chain diagrams rather than compilable code, adding a language hint (e.g.,
```textor```plaintext) would satisfy markdown linters and improve rendering consistency.Example fix for one block
-``` -Before: +```text +Before:Also applies to: 180-218, 222-234, 323-344, 400-432, 497-515, 586-600
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 89a2c7f1-7a7c-4a9b-9b88-c6324742f82d
📒 Files selected for processing (4)
docs/membership-package-tasks.mdweb/sdk/react/views-new/projects/components/remove-member-dialog.tsxweb/sdk/react/views/api-keys/details/manage-service-user-projects-dialog.tsxweb/sdk/react/views/api-keys/list/add-service-account-dialog.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/sdk/react/views-new/projects/components/remove-member-dialog.tsx
- web/sdk/react/views/api-keys/list/add-service-account-dialog.tsx
Both views and views-new now pass the member type (user/group) from the column action through to the remove dialog and into the RemoveProjectMember RPC call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
Replace policy-based project member management with the new atomic RPCs across React SDK (views + views-new) and Admin SDK.
Before: SDK manipulated policies directly (
createPolicyForProject,listPolicies → deletePolicy x N → createPolicy)After: SDK uses
SetProjectMemberRoleandRemoveProjectMemberInconsistency found and fixed
The "Add member" flow was passing role names (e.g.,
app_project_viewer) as theroleId, while the "Change role" dropdown was passing role UUIDs from the fetched roles list. The backendSetProjectMemberRoleexpects a UUID.Fix: All flows now fetch project-scoped roles via
listRolesand resolve the role UUID before calling the RPC.Files changed
React SDK (views):
project-members.tsxaddMember/addTeamuseSetProjectMemberRolewith role UUIDproject-member-columns.tsxupdateRoleusesSetProjectMemberRole, passesmemberTypefor removeproject-detail-page.tsxmemberType(user/group) through remove flowremove-project-member-dialog.tsxRemoveProjectMemberwith correctprincipalTypeReact SDK (views-new):
add-member-menu.tsxSetProjectMemberRolewith fetched viewer role UUIDproject-details-view.tsxSetProjectMemberRolefor role change, passesmemberTypefor removeremove-member-dialog.tsxRemoveProjectMemberwith correctprincipalTypefrom payloadService account flows:
add-service-account-dialog.tsxSetProjectMemberRolewith fetched owner role UUIDmanage-service-user-projects-dialog.tsxAdmin SDK:
use-add-project-members.tsxSetProjectMemberRolewith fetched viewer role UUIDremove-member.tsxRemoveProjectMemberassign-role.tsxSetProjectMemberRoleCloses #1462
Test plan
🤖 Generated with Claude Code