feat: move SetOrganizationMemberRole to membership package

[whoAbhishekSah]

Summary

Moves the existing SetOrganizationMemberRole RPC implementation from core/organization into core/membership. Also fixes the known leak where demoting an org owner to viewer left the org#owner direct relation in place.

What changes

New in core/membership/

• SetOrganizationMemberRole(ctx, orgID, principalID, principalType, roleID) — handles the full role-change flow
• Private helpers: validateMinOwnerConstraint, replacePolicy, replaceRelation
• Skips write when role is unchanged (existing optimization carried over)

Removed from core/organization/

• SetMemberRole
• validateSetMemberRoleRequest
• getUserOrgPolicies
• validateMinOwnerConstraint
• replaceUserOrgPolicies

Handler

• SetOrganizationMemberRole RPC handler now calls membershipService.SetOrganizationMemberRole with schema.UserPrincipal
• Error mapping updated to membership package error types (membership.ErrNotMember, membership.ErrInvalidOrgRole, membership.ErrLastOwnerRole)

Bug fix — relation cleanup

Previously, org.SetMemberRole replaced policies but never touched the explicit org#owner or org#member relations. So demoting an owner to viewer left the owner relation in place, continuing to grant owner permissions via SpiceDB.

Now replaceRelation deletes both owner and member relations for the principal before creating the new one matching the target role. Verified by the should succeed demoting owner to viewer with multiple owners test.

Edge cases handled

• Principal type must be app/user (serviceusers bound at creation, not changed via this RPC)
• Org does not exist or is disabled → error
• User does not exist or is disabled → error
• Role does not exist → error
• Role is not org-scoped → error
• User is not a member → ErrNotMember
• Role is unchanged → early return (no writes)
• Min-owner constraint — can't demote the last owner

Test plan

• 6 new unit tests in core/membership/ covering all edge cases above
• Handler tests updated to use membership service mock

Related

• Parent issue: #1478
• Prior PR (AddOrganizationMember): #1537

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview, Comment	Apr 16, 2026 4:12am

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2245ba2d-333b-413b-aa99-ccb4bf25fa8a

📥 Commits

Reviewing files that changed from the base of the PR and between bc494ac and 899deed.

📒 Files selected for processing (2)

• core/membership/service.go
• core/membership/service_test.go

📝 Walkthrough

Walkthrough

This PR migrates the organization member role-update functionality from core/organization/service.go to core/membership/service.go, introducing a new SetOrganizationMemberRole() method parameterized by principal identity and type. The old implementation is removed, interfaces are updated, mocks are regenerated, and the API handler is rewired to call the membership service instead of the organization service.

Changes

Cohort / File(s)	Summary
Core Membership Service core/membership/service.go, core/membership/service_test.go	Added SetOrganizationMemberRole() method with owner constraint validation, policy/relation replacement, and audit logging; includes comprehensive test coverage for role updates, last-owner prevention, and no-op handling.
Core Organization Service Removal core/organization/service.go, core/organization/service_test.go	Removed SetMemberRole() method and all supporting functions (validateSetMemberRoleRequest, getUserOrgPolicies, validateMinOwnerConstraint, replaceUserOrgPolicies); removed associated test suite and unused imports.
API Interface Updates internal/api/v1beta1connect/interfaces.go	Removed SetMemberRole() from OrganizationService interface; added SetOrganizationMemberRole() to MembershipService interface with principal-type parameterization.
API Mock Generation internal/api/v1beta1connect/mocks/membership_service.go, internal/api/v1beta1connect/mocks/organization_service.go	Generated new MembershipService mock with SetOrganizationMemberRole() method; removed SetMemberRole() from OrganizationService mock.
API Handler and Tests internal/api/v1beta1connect/organization.go, internal/api/v1beta1connect/organization_test.go	Updated handler to call membershipService.SetOrganizationMemberRole() instead of orgService.SetMemberRole(); remapped error handling to membership-scoped errors; updated test to use MembershipService mock with new principal-type parameter.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Issue #1459: Both implement the same server-side SetOrganizationMemberRole RPC with minimum-owner and atomic policy replacement behavior.

Possibly related PRs

• PR #1537: This PR directly extends the membership service foundation added in that PR by introducing role-update functionality to the new service.
• PR #1519: Both PRs modify audit logging for organization member role changes, wiring audit record creation for the same event.
• PR #1477: Complementary SDK/frontend changes that invoke the new SetOrganizationMemberRole RPC endpoint being implemented here.

Suggested reviewers

• rohilsurana
• rsbh

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

Tip

CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.

OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required.

[coveralls]

Coverage Report for CI Build 24444825715

Coverage decreased (-0.05%) to 41.679%

Details

• Coverage decreased (-0.05%) from the base build.
• Patch coverage: 28 uncovered changes across 2 files (100 of 128 lines covered, 78.13%).
• 4 coverage regressions across 1 file.

Uncovered Changes

File	Changed	Covered	%
core/membership/service.go	122	96	78.69%
internal/api/v1beta1connect/organization.go	6	4	66.67%

Coverage Regressions

4 previously-covered lines in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
core/organization/service.go	4	37.87%

---

Coverage Stats

Relevant Lines:	36616
Covered Lines:	15261
Line Coverage:	41.68%
Coverage Strength:	11.86 hits per line

---

💛 - Coveralls

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

core/membership/service_test.go (1)

276-428: Add a regression for relation-replacement failures.

This suite only covers the success path for deleting old relations and creating the new one. A case where RelationService.Delete or Create fails during SetOrganizationMemberRole would lock in the owner-demotion fix and catch the partial-update paths in this new flow.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7017a580-65f7-4314-aa5d-0fccb1f9afc1

📥 Commits

Reviewing files that changed from the base of the PR and between 08d63db and bc494ac.

📒 Files selected for processing (9)

• core/membership/service.go
• core/membership/service_test.go
• core/organization/service.go
• core/organization/service_test.go
• internal/api/v1beta1connect/interfaces.go
• internal/api/v1beta1connect/mocks/membership_service.go
• internal/api/v1beta1connect/mocks/organization_service.go
• internal/api/v1beta1connect/organization.go
• internal/api/v1beta1connect/organization_test.go

💤 Files with no reviewable changes (3)

• core/organization/service_test.go
• core/organization/service.go
• internal/api/v1beta1connect/mocks/organization_service.go