Repository navigation
feat(organizations): add admin role with owner-equivalent authority - #5007
Merged
Merged
Conversation
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryIncremental pass over Files Reviewed (3 files this pass)
Previous Review Summaries (2 snapshots, latest commit d00404e)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit d00404e)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental pass over Files Reviewed (4 files this pass)
Previous review (commit ff40951)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe latest fix commit (c563d9ea) leaves Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (4 files this pass)Incremental pass over
Reviewed by kimi-k3 · Input: 59.6K · Output: 11.1K · Cached: 792.6K Review guidance: REVIEW.md from base branch |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
jrf0110
force-pushed
the
admin-capabilities
branch
from
August 4, 2026 19:31
a04704b to
2cd2eaa
Compare
Introduces an `admin` organization role that holds the same authority as `owner`. There is no action an owner can take that an admin cannot. - Add `admin` to the canonical role union (`packages/db` schema-types and `@kilocode/app-shared/organizations`). The `role` column is plain `text` with a TS-level `$type`, so no migration is required. - Add shared `ORGANIZATION_MANAGE_ROLES` (owner, admin) and `ORGANIZATION_BILLING_ROLES` (owner, admin, billing_manager) constants and replace the literal role arrays that previously duplicated them. - Rename `organizationOwnerProcedure`/`organizationOwnerMutationProcedure` to `organizationAdminProcedure`/`organizationAdminMutationProcedure` and widen them to owner+admin, so every previously owner-gated org route (members, settings, groups, modes, SSO, usage recommendations) now accepts admins. - Include `admin` in role priority and in parent-organization inheritance. - Widen the matching frontend gates via `canManageOrganization` so admins are not shown a read-only UI for routes the API now permits, and surface the role in the label maps, invite dialog, and role dropdown. Adds admin-parity coverage to the members router tests and points the role array assertions at the shared constant so they cannot drift.
Admins match owners everywhere except owner management: an admin may not grant the owner role, nor act on an existing owner's membership. Both halves are enforced together on purpose. Blocking only the grant would let an admin demote or remove every owner while being unable to appoint a replacement, leaving the organization with no owner. Enforced in `assertOwnerAuthority` and applied to the role update, member removal, invite, and invite-revocation paths. Kilo staff are unaffected because `ensureOrganizationAccess` resolves them to `owner`. The role dropdown and invite dialog mirror the server rules: admins can assign admin, member, and billing_manager, and an owner's membership renders read-only for them.
`apps/mobile` shares `OrganizationRole` with `packages/db`, so adding `admin` broke its typecheck in two places. - `canManageSecurityAgent` took a hand-written narrow role union. It now takes `OrganizationRole` and delegates to `canManageOrganizationBilling`, which already encodes the same owner/billing_manager rule and includes admins. - Mobile's `ROLE_LABEL` is a `Record<OrgRole, string>`, so it needs an `admin` entry; the role is also added to the assignable options an owner sees.
Addresses PR review on the member card and stale role docs. `canManageMembers` was widened to admins, which also widened the remove and revoke-invitation gates. The server rejects both for admins when the target is an owner, so an admin saw enabled controls that always failed with FORBIDDEN. - Add `canManageOrganizationOwners` to the shared role module as the single statement of the owner-management rule, mirroring `assertOwnerAuthority`. - Gate `DeleteMemberButton` and `DeleteInvitationButton` on the target's role so admins are not offered actions against an owner or a pending owner invite. - Refresh the member-removal business-rule comment, which predated the role. - Correct the `ORGANIZATION_MANAGE_ROLES` docstring, which still claimed admin was an exact peer of owner, and a stale security-agent test name.
Completes the previous commit. `canRemoveMember` gained a target-role parameter but the member row's own `canDelete` still called it with three arguments, which broke the build, and two sibling gates on that row had the same owner-management gap. - Pass the target role at the row call site. - Gate the row's `canEditRole` and the invited-member `canDelete` on `canActOnMemberRole`, so admins are not shown edit mode or a revoke control for an owner or a pending owner invitation. - Leave `canEditLimit` ungated and say why: the server scopes `assertOwnerAuthority` to role changes, so an admin may still set an owner's daily usage limit.
jrf0110
force-pushed
the
admin-capabilities
branch
from
August 6, 2026 13:54
d00404e to
3ec6112
Compare
jeanduplessis
approved these changes
Aug 6, 2026
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
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.
What
Adds an
adminorganization role that matchesownereverywhere except owner management: an admin cannot grant the owner role, and cannot act on an existing owner's membership.Context on the original ask
The request was "Admins can do everything owners can but can't delete the org." Two findings changed the shape of this:
adminorg role. Roles wereowner | member | billing_manager, so this is a new role rather than a permission tweak.organizations.admin.delete, anadminProcedure). Owners are not getting self-serve deletion, so the "can't delete the org" carve-out is a no-op.The meaningful boundary landed elsewhere: owner management.
Permission model
adminownerBoth owner-management restrictions are enforced together on purpose. Blocking only the grant would let an admin demote or remove every owner while being unable to appoint a replacement, leaving the org with no owner.
Changes
Role definition
adminadded to the canonical union inpackages/db/src/schema-types.tsand@kilocode/app-shared/organizations.organization_memberships.roleis plaintext()with a TS-level$typeand no DB check constraint.Shared role sets (replacing literal arrays duplicated across ~25 files)
ORGANIZATION_MANAGE_ROLES=owner,adminORGANIZATION_BILLING_ROLES=owner,admin,billing_managerAuthorization
organizationOwnerProcedure/organizationOwnerMutationProcedurerenamed toorganizationAdminProcedure/organizationAdminMutationProcedureand widened to owner+admin. The rename forces every call site to be reviewed rather than silently changing semantics.assertOwnerAuthorityreserves owner management for owners, applied to the role-update, member-removal, invite, and invite-revocation paths. Kilo staff bypass it becauseensureOrganizationAccessresolves them toowner.adminadded to role priority and to parent-organization inheritance (kept as its own list so billing-scope changes can't silently widen child-org inheritance).Frontend
canManageOrganizationso admins aren't shown a read-only UI for routes the API now permits.adminsurfaced in role labels and the role-testing switcher.getRoleLabelalso renderingbilling_manageras "Member".Testing
apps/webtypecheck: clean (tsgo --noEmit, exit 0)oxlintoverapps/web/src,packages/app-shared/src,packages/db/src: 0 warnings, 0 errorsrouters/organizations,lib/organizations,components/organizations: 932 passed, 1 failedorganization-kiloclaw-router.test.ts"clears organization subscription destruction lifecycle" — a pre-existing timezone assertion (2026-04-12vs2026-04-11 19:00:00-05), confirmed failing on a clean stashed tree.packages/app-sharedvitest role tests: 5 passedadmin, invite anadmin, and remove a non-owner; admin is rejected when grantingowner, inviting anowner, changing an existing owner's role, or removing an owner. Existing assertions that billing managers and members are rejected still pass.Notes for review
mainafter chore(cost-insights): remove frontend pages, components, and tRPC routers #4993/chore(cost-insights): stop all writes to the cost_insight_* tables #4996 removed the cost-insights frontend and repository. Both rebase conflicts were modify/delete on files those PRs deleted; the deletions were accepted, so this branch no longer touches cost-insights.apps/mobilestill has local role string-unions withoutadmin; mobile does not yet render or assign the role.