feat(organizations): unified sub-org membership directory (Phase 1) - #5516
feat(organizations): unified sub-org membership directory (Phase 1)#5516jrf0110 wants to merge 7 commits into
Conversation
Add a unified navigation hub for parent organizations to view and manage member roles across all direct child organizations, plus two independent bug fixes to existing membership code. - Add canManageMemberships to organizations.subOrganizations.people so the directory can gate management affordances per row without re-deriving RBAC client-side (UX hint only; every mutation still re-checks authorization server-side). - Add a MemberManagementDrawerStackProvider (built on the shared createDrawerStack primitive) that mounts the existing single-org OrganizationAdminMembers UI for any child org, launched from the directory. - Add guided "Add people to a child org" / "Remove people from a child org" wizards that sequentially orchestrate the existing invite/remove/deleteInvite mutations, with per-row eligibility checks and a shared results/retry UI. No new tRPC procedures. - Add usage telemetry (drawer opens, wizard runs, repeat runs, large selections, partial failures) to gate a possible Phase 2 bulk-write backend on real usage data. - Fix addUserToOrganization missing the lockOrganizationMembershipMutation advisory lock that its sibling functions already take. - Fix setChildMemberships's removal loop silently stripping a child org's owner/billing_manager membership when omitted from the request; only a client-side check prevented this before. Refs #5495
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| const execute = useCallback( | ||
| async (person: Person) => { | ||
| await inviteMutation.mutateAsync({ | ||
| organizationId: targetOrganizationId, |
There was a problem hiding this comment.
CRITICAL: invite rejects child organizations, so every add-wizard row fails
organizations.members.invite calls inviteUserToOrganization, which throws when the target has a parent_organization_id. The tRPC layer maps that to PRECONDITION_FAILED: "Child organizations manage membership through their parent organization." OrganizationAdminMembers already hides Invite for children for this reason.
This wizard always passes the selected child as organizationId, so every eligible row fails. Child assignment today goes through setChildMemberships on the parent (organizationId: parent, memberId, full childOrganizationIds) or addUserToOrganization on the child — not invite.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| )} | ||
|
|
||
| {(step === 'preview' || step === 'results') && targetOrganizationId && ( | ||
| <OrganizationAdminContextProvider organizationId={targetOrganizationId}> |
There was a problem hiding this comment.
WARNING: Child-scoped OrganizationAdminContextProvider treats parent owners/admins as member
OrganizationAdminContextProvider resolves role from the child's direct members list and defaults missing membership to 'member'. Parent owners/admins inherit manage access (canManageMemberships: true via getOrganizationsAccessRoles) without being child members — the case the people-router tests document.
getAvailableInviteRoles('member') is [], so preview shows "You don't have permission…" and Confirm stays disabled for the primary parent-manager user. withMembers already returns inherited callerRole; use that instead of members.find.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| )} | ||
|
|
||
| {step !== 'target' && targetOrganizationId && ( | ||
| <OrganizationAdminContextProvider organizationId={targetOrganizationId}> |
There was a problem hiding this comment.
WARNING: Child-scoped OrganizationAdminContextProvider treats parent owners/admins as member
Same role-resolution hole as the add wizard: parent owners/admins are usually not in the child members list, so useUserOrganizationRole() falls back to 'member'. Then canRemoveMember('member', …) is false for every row, removableCount is 0, and Confirm stays disabled.
withMembers already returns inherited callerRole for this child; the wizards should use that instead of wrapping with a provider that only sees direct membership.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return { | ||
| header: <h2 className="type-body font-medium">{entry.childOrganizationName}</h2>, | ||
| body: ( | ||
| <OrganizationAdminContextProvider organizationId={entry.childOrganizationId}> |
There was a problem hiding this comment.
WARNING: Manage-members drawer hides controls for parent owners who inherited access
Clickable directory badges are gated on inherited canManageMemberships, but this wrapper resolves role from the child's direct members list (OrganizationAdminContextProvider defaults missing membership to 'member'). OrganizationAdminMembers then hides role/remove/invite-revoke controls via useUserOrganizationRole() even though remove/deleteInvite would accept the parent owner's inherited manage role.
withMembers already returns effective callerRole; use that rather than re-deriving from the members array.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| onManage={() => | ||
| drawer.open({ | ||
| type: 'manage-members', | ||
| childOrganizationId: invitation.organizationId, |
There was a problem hiding this comment.
WARNING: Parent-org invitation badges open the child-org member drawer
invitations include the parent (isParent: true, name "Parent organization"). Memberships correctly exclude the parent, but invitation badges always pass invitation.organizationId as childOrganizationId. A pending parent invite therefore mounts OrganizationAdminMembers for the parent from the sub-org directory. Skip isParent invitations, or do not treat them as manage-members targets.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const [role, setRole] = useState<OrganizationRole>('member'); | ||
|
|
||
| const selectedPeople = useMemo( | ||
| () => people.filter(person => selected.has(person.identityKey)), |
There was a problem hiding this comment.
WARNING: Cross-page directory selections are silently dropped
The people table accumulates selectedIdentityKeys across pages and seeds this wizard with all of them. people is only the current directory page, so this filter drops off-page keys while Next ({selected.size} selected) still counts them. Preview and execute then run on a smaller set than the user selected. The remove wizard has the same gap: its target-change effect drops seeded keys that are not on the current page.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Previous Review Summaries (5 snapshots, latest commit 4b95c78)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 4b95c78)Status: 4 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Previous review (commit 9a4f2f7)Status: 4 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (1 file)
Previous review (commit c28b5f6)Status: 4 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (14 files)
Previous review (commit adda2ee)Status: 8 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (15 files)
Previous review (commit 142239e)Status: 6 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (28 files)
Reviewed by grok-4.6 · Input: 168.2K · Output: 17.5K · Cached: 949.9K Review guidance: REVIEW.md from base branch |
… tab - AddPeopleWizard now supports selecting multiple target sub-organizations in one run (checkbox multi-select instead of single radio target), operating on the full (person x target org) cross product with eligibility, sequential execution, and results attribution computed per pair. RemovePeopleWizard is unchanged (still single-target). - Add a new InvitePersonWizard, reachable from a selection-independent "Invite person..." button on the People tab, letting an admin invite a brand-new person directly into the parent org or any specific direct child org, gated by real per-org invite access (fails closed while access is still resolving). Calls the existing organizations.members.invite mutation; no new tRPC procedures. - Extract a shared WizardChrome step wrapper and thread invite-person runs through the existing wizard_run telemetry alongside add/remove. Addresses follow-up feedback on #5516.
| const execute = useCallback( | ||
| async ({ person, organization }: AddPersonToOrgRow) => { | ||
| await inviteMutation.mutateAsync({ | ||
| organizationId: organization.id, |
There was a problem hiding this comment.
CRITICAL: invite rejects child organizations, so every add-wizard row fails
organizations.members.invite calls inviteUserToOrganization, which throws when the target has a parent_organization_id. The tRPC layer maps that to PRECONDITION_FAILED: "Child organizations manage membership through their parent organization." This wizard now fans out one invite per (person, child org) pair, so every eligible row still fails — just more of them.
Child assignment today goes through setChildMemberships on the parent (organizationId: parent, memberId, full childOrganizationIds) or addUserToOrganization on the child — not invite.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| )} | ||
|
|
||
| {(step === 'preview' || step === 'results') && primaryTargetOrganizationId && ( | ||
| <OrganizationAdminContextProvider organizationId={primaryTargetOrganizationId}> |
There was a problem hiding this comment.
WARNING: Child-scoped OrganizationAdminContextProvider treats parent owners/admins as member
OrganizationAdminContextProvider resolves role from the child's direct members list and defaults missing membership to 'member'. Parent owners/admins inherit manage access (canManageMemberships: true) without being child members.
getAvailableInviteRoles('member') is [], so preview shows "You don't have permission…" and Confirm stays disabled for the primary parent-manager user. Multi-org selection makes this worse: the role picker uses only the first selected org, so one inherited-only target blocks the whole run.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
|
|
||
| inviteMutation.mutate( | ||
| { organizationId: targetOrganizationId, email: email.trim(), role }, |
There was a problem hiding this comment.
CRITICAL: invite rejects child organizations, so child-target invites always fail
handleInvite passes the selected org — including any child — to organizations.members.invite. inviteUserToOrganization throws when the target has a parent_organization_id, mapped to PRECONDITION_FAILED: "Child organizations manage membership through their parent organization."
Inviting into the parent works. Inviting into a child does not. A new person must be invited to the parent first, then assigned to a child via setChildMemberships / addUserToOrganization.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| onResult: (organizationId: string, availableRoles: OrganizationRole[]) => void; | ||
| }) { | ||
| return ( | ||
| <OrganizationAdminContextProvider organizationId={organizationId}> |
There was a problem hiding this comment.
WARNING: Access probes treat inherited parent owners/admins as member
Each OrgAccessProbe wraps OrganizationAdminContextProvider, which resolves role from the child's direct members list and defaults missing membership to 'member'. Parent owners/admins who inherit manage access are not child members, so getAvailableInviteRoles returns [] and every child is disabled with "You can't invite here".
The parent option still works (the viewer is a direct parent member). Combined with invite rejecting children, the child path is both hidden and broken.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
The invite mutation unconditionally rejects any organization that has a parent (organizations.ts:608), so both bulk wizards were broken for their primary use case: - AddPeopleWizard called organizations.members.invite once per (person, target org) pair, which always failed for every target since its targets are exclusively direct children. Switch to organizations.members.setChildMemberships, called once per selected person with their full desired child-org-id set (existing child memberships unioned with newly eligible targets), matching the existing ChildTeamsControl precedent. Drop the role picker, since setChildMemberships has no role parameter and always adds as 'member'. Add a 'not-parent-member' eligibility exclusion for directory people with no accepted parent membership, since that mutation requires one. - InvitePersonWizard offered any direct child as an invite target, which can never succeed for anyone regardless of role. Child organizations are now permanently, unconditionally disabled as direct-invite targets with an explanation distinct from the per-viewer access states, plus a defensive client-side guard before the mutation call. The explanation names the two-step workaround (invite into the parent, then use "Add people to sub-organizations" once they've joined). Neither fix required a new tRPC procedure. Test coverage now includes explicit negative assertions that the invite mutation is never called by AddPeopleWizard and never called with a child organization id by InvitePersonWizard, closing the gap that let this ship: every prior wizard test mocked the mutation hooks and never exercised the real server-side business rule.
- Role dropdowns rendered inside the shared drawer-stack primitive opened but were invisible/unclickable: Select/DropdownMenu portal content defaults to z-50, below the drawer's z-60/z-61+ layers. Add the z-[70] override (matching the existing ModelAccessPolicyEditor.tsx precedent) to MemberRoleDropdown's DropdownMenuContent and InvitePersonWizard's role SelectContent. - Simplify InvitePersonWizard from a 2-step (choose org, then email/role) to a single email/role step. Child organizations were already permanently unselectable there (they can never receive a direct invite), so the org-picker step offered exactly one real choice and was pure noise. The wizard now always targets the parent org directly, with a short informational note replacing the removed step's explanation of the two-step sub-org workaround. apps/web/src/components/organizations/sub-organizations/drawer/wizards/InvitePersonWizard.tsx shrinks from 490 to 205 lines.
Creates a parent org, two direct child orgs, and a roster of users covering the membership shapes the sub-organizations People tab, drawer, and bulk wizards need to be exercised against locally: - Parent owner with no direct child memberships (inherited access). - Parent admin who is also a direct child member (both paths visible). - Parent billing_manager (permission-boundary case). - Plain parent member (read-only). - A parent member who is also a child *owner* (elevated child role, exercises the setChildMemberships removal-loop role-filter guard). - A parent member already in one child (exercises the add-wizard's union-not-replace logic when adding them to a second child). - A child-only member with no parent membership (exercises the "must be a member of the parent organization first" exclusion). - A pending invitation into a child org. Usage: pnpm dev:seed app:sub-organizations
Every scrollable list across the three bulk-action wizards (select people, pick target sub-organizations, preview) capped its own height at a fixed max-h-96/max-h-80, leaving the rest of the drawer blank instead of using the available vertical space. WizardChrome now matches the drawer body's real height (h-full min-h-0) and each list uses flex-1 min-h-0 to fill whatever space remains below the fixed chrome (title, search input, buttons), scrolling internally only once its own content exceeds that space. WizardResultsList's outer wrapper duplicated WizardChrome's own flex-column/padding (it's always rendered inside WizardChrome) and its row list had no scroll constraint at all; converted the wrapper to a Fragment and gave the list the same flex-1 min-h-0 treatment for consistency with the other four lists.
setChildMemberships has no role parameter (always adds as member), which is why the role picker was dropped when the wizard was switched to it from invite (invite unconditionally rejects child-org targets). Restore role selection as a follow-up organizations.members.update call per newly added org, issued only when a non-member role is chosen. Both mutations already exist; no new tRPC procedures. The role list offered (via getAvailableInviteRoles, keyed off the viewer's PARENT role) is intentionally exactly the set the follow-up update call can satisfy for that viewer: a parent billing_manager only ever sees 'member' (matching setChildMemberships's own capability, so no follow-up call is ever attempted for them), while a parent owner/admin's inherited access to every direct child satisfies update's per-child authorization check for elevated roles. Also add a jsdom scrollIntoView polyfill (apps/web/src/tests/setup/ jsdomPolyfills.ts) — Radix Select calls it when an item is actually clicked (not just asserted present), which no test in this codebase had exercised until now, and jsdom doesn't implement it at all.
1 participant
Summary
Implements Phase 1 of #5495 — a unified navigation hub for parent organizations to view and manage member roles across all direct child organizations, plus two independent bug fixes to existing membership code (fixed regardless of the UI approach, per the linked design rationale).
Backend fixes (unrelated to each other, both closing real gaps)
addUserToOrganization(apps/web/src/lib/organizations/organizations.ts) was missing thelockOrganizationMembershipMutationadvisory lock that its siblingsremoveUserFromOrganization/addSsoUserToOrganizationalready take before writing — added it.setChildMemberships's removal loop (apps/web/src/routers/organizations/organization-members-router.ts) had no role filter, so it would silently strip a child org'sowner/billing_managermembership if that child id was simply omitted from the request; only a client-sidelockedcheckbox prevented this in practice. Added a server-side guard so elevated roles are never removed by this bulk-reconcile mutation. The existing test that asserted the old (unsafe) behavior was rewritten to assert the fixed contract, plus a companion test proving ordinarymember-role removals still work.Unified directory (Phase 1 UI)
canManageMemberships: booleanto every membership/invitation/parentMembershipentry inorganizations.subOrganizations.people, computed via the existing batchedgetOrganizationsAccessRoleshelper (one extra fixed-cost query, not per-row). This is a UX hint only — every mutation still runs its own independent authorization check.MemberManagementDrawerStackProvider(built on the sharedcreateDrawerStackprimitive, mirroring the existingOrganizationGroupsDrawerStackpattern) that mounts the existing, unmodifiedOrganizationAdminMemberssingle-org member card for any child org, launched by clicking a manageable badge in the directory.invite/remove/deleteInvitemutations (no new tRPC procedures). Sequential (not parallel) execution preserves the existing seat-capacity invariant ininvite.sub_org_directory.*PostHog events: drawer opens, wizard runs, repeat runs, large selections, partial failures) to gate a possible Phase 2 bulk-write backend on real usage data over time, rather than building it speculatively.Design process
This PRD was produced via an adversarial multi-agent design exploration (three independent architectures compared, an adversarial critique of the top-ranked design, then a synthesis into this phased plan) — full rationale in #5495.
Testing
setChildMembershipsrole-filter regression + companion test, wizard row-eligibility filtering, sequential-execution/partial-failure/retry state machine, telemetry repeat-run/threshold/failure-detection logic.canManageMembershipscoverage for inherited parent access vs. plain member vs. billing_manager inorganization-sub-organizations-router.test.ts.canManageMemberships, drawer context-nesting safety (child-scoped role resolves correctly, not an outer parent-scoped one), full wizard flow (select → preview → execute → results, including the skipped-row progress-counting fix).All new/changed tests pass; pre-existing, unrelated failures (
organization-kiloclaw-router.test.tstimezone flake,organization-bitbucket-router.test.tsmock issue) were confirmed to reproduce identically onmainand are out of scope for this PR.Non-goals (this PR)
membershipMatrix/reconcilebulk-write backend or spreadsheet-grid UI (evidence-gated Phase 2, tracked separately if telemetry justifies it).Closes #5495 (Phase 1 scope only).