Skip to content

fix(mobile): delegate organization member management by role and seats - #10726

Open
iscekic wants to merge 3 commits into
mainfrom
igor/parity-org-fixes
Open

iscekic wants to merge 3 commits into
mainfrom
igor/parity-org-fixes

Conversation

@iscekic

@iscekic iscekic commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Three organization member-management gaps on mobile:

  • Admins can manage members. Mobile showed member and invitation controls to owners only, while the server and web allow admins. Controls now follow canManageOrganization; rows whose target is an owner stay owner-only, and the role menu offers the same roles as web's getAvailableRoles.
  • Admin is an invitable role. invitableRoles now matches web's getAvailableInviteRoles (owners: owner, admin, member, billing manager; admins: admin, member, billing manager; billing managers: member).
  • Seat limit enforced on the server. The invite procedure did not check seats; only the web dialog did. assertInviteSeatCapacity now runs inside the invite transaction under an organization-scoped advisory lock, so two concurrent invites cannot take the last seat. It returns PRECONDITION_FAILED with a stable message. Billing managers take no seat; organizations without require_seats or on enterprise are unlimited. The rule moved to @kilocode/app-shared/organizations so the web dialog and mobile use one implementation.
  • Mobile seat warning. The invite sheet shows the web's seats-full warning and disables Send for seat-consuming roles; the server message maps to the same copy if a race slips through.

Behaviour change: an organization that requires seats and has none free can no longer invite seat-consuming members from any client. The web dialog already blocked this, so web users see no change.

Verification

  • Mobile typecheck, lint, check:unused, check:i18n, check:plurals, format check; web typecheck and lint.
  • Server: organization-members-router.test.ts against Postgres, including seats full (rejected, no invitation row), billing manager allowed when full, and unlimited plans. Web InviteMemberDialog and mobile member/invite suites pass.
  • Android emulator against a local stack with fake accounts: an admin changed a member's role and revoked an invite and saw no controls on the owner row; an owner invited a member as admin; with all seats used the sheet showed the warning with Send disabled for member and enabled for billing manager; a direct tRPC invite returned 412 PRECONDITION_FAILED with the new message.

Mobile showed member-management actions to owners only, offered the wrong
invite roles, and let an invitation through when the organization had no
free seat. Web already models all three.

- Member and invitation rows now open for owners and admins; a row whose
  target holds the owner role stays owner-only, mirroring the server's
  owner-authority check and the web role dropdown.
- The role menu and the invite sheet offer exactly the web's role lists
  for the caller.
- The invite procedure refuses a seat-consuming invitation when the plan
  enforces seats and none is free, under an advisory lock so concurrent
  invites cannot both take the last seat. A billing manager takes no seat.
- The invite sheet shows the web's seat warning with used/total and
  disables Send; a race is explained through the new server message.
- The seat rule lives once, in app-shared, and the web dialog reads it
  from there.
@iscekic iscekic self-assigned this Nov 12, 2026
@iscekic

iscekic commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android emulator proof (local stack, fake org): admin role menu without Owner, owner invite roles, seats-full warning for member vs billing manager.

04-admin-role-options

08-owner-invite-roles

11-owner-seats-full-member

12-owner-seats-full-billing

// Refuse before writing anything when the organization has no free
// seat: the advisory lock it takes makes two concurrent invites
// serialize, so neither can take the last seat twice.
await assertInviteSeatCapacity(tx, organizationId, role);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: Seat check runs before the duplicate-invite checks, so a duplicate on a full organization reports the wrong reason

assertInviteSeatCapacity runs before inviteUserToOrganization, and the seat count includes pending invitations. On an organization at its seat limit, an address that is already a member or already has a pending invitation therefore fails the seat check first and surfaces as PRECONDITION_FAILED / ORGANIZATION_INVITE_SEAT_LIMIT_REACHED instead of the specific CONFLICT (This user is already a member of this organization / This email already has a pending invitation). Mobile maps the seat message to organization.inviteMember.noSeatsAvailable, so the user is told to ask for seats when the real answer is that the person already has one. Running the existing pending-invitation/member validation before the seat gate (or letting those errors win) keeps the message accurate.

* only a plain member, because the invite mutation needs a billing role for a
* plain member and organization-management authority for anything above it.
*/
export function invitableRoles(role: OrganizationRole | undefined): OrganizationRole[] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: The role lists do not fully mirror web for Kilo staff

The doc comment says these lists match web's getAvailableInviteRoles / getAvailableRoles, but both web helpers take isKiloAdmin and return every role when isKiloAdmin || currentUserRole === 'owner' (web InviteMemberDialog.tsx:79, MemberRoleDropdown.tsx:53). Here an admin membership always stops at ['admin', 'member', 'billing_manager'], so a Kilo staff member on mobile is not offered owner for an invite or a role change, and the canManageOrganizationOwners(role) gate in members-screen.tsx hides actions on owner rows/owner invitations for them, while web shows them. Either thread the staff flag into these helpers (and the screen gates) or narrow the parity claim.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bot: Agreed on the wording. Kilo staff overrides (isKiloAdmin) stay web-only on purpose: staff administer organizations from the web admin tools, not the mobile app. The next push narrows the comment to say the lists mirror web for organization roles only.

@kilo-code-bot

kilo-code-bot Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental commit only updates the mobile sheet test's organization mock (no new defects), and the two earlier seat/role-parity suggestions remain present at current HEAD.

Assumptions: [INFERENCE] The seat gate's ordering consequence (finding 1) is inferred from the router calling assertInviteSeatCapacity before inviteUserToOrganization while the seat count includes pending invitations; no live request was executed. Finding 2 is only about the accuracy of the parity doc comment now that Kilo-staff elevation is confirmed intentional and web-only.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 2
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/web/src/routers/organizations/organization-members-router.ts 613 assertInviteSeatCapacity runs before inviteUserToOrganization, and the seat count includes pending invitations, so on a full organization a duplicate member/pending-invite email surfaces as PRECONDITION_FAILED (mapped to "no seats available") instead of the specific CONFLICT.
apps/mobile/src/components/organization/organization-role-options.ts 23 The doc comment says these lists mirror web's getAvailableInviteRoles / getAvailableRoles, but both web helpers return every role when `isKiloAdmin
Files Reviewed (3 files)
  • apps/mobile/src/components/organization/member-limit-sheet.mounted.test.tsx (incremental commit) - 0 issues
  • apps/web/src/routers/organizations/organization-members-router.ts - 1 suggestion (carried, re-verified at HEAD)
  • apps/mobile/src/components/organization/organization-role-options.ts - 1 suggestion (carried, re-verified at HEAD)

Fix these issues in Kilo Cloud

Previous Review Summary (commit e4f180a)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit e4f180a)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The seat-capacity enforcement and role delegation are sound, but the new seat gate runs before duplicate-invite validation and the mobile role lists drop web's Kilo-staff elevation, so two low-risk parity/UX gaps remain.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 2
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/web/src/routers/organizations/organization-members-router.ts 613 assertInviteSeatCapacity runs before inviteUserToOrganization, so on a full organization a duplicate member/pending-invite email returns the seat-limit PRECONDITION_FAILED instead of the specific CONFLICT.
apps/mobile/src/components/organization/organization-role-options.ts 23 Role lists claim web parity but omit web's isKiloAdmin branch, so Kilo staff with an admin membership are not offered owner in mobile (server behavior depends on support-access mode).
Files Reviewed (107 files)
  • apps/mobile/src/components/organization/invite-member-roles.test.ts (deleted)
  • apps/mobile/src/components/organization/invite-member-roles.ts (deleted)
  • apps/mobile/src/components/organization/invite-member-sheet.mounted.test.tsx
  • apps/mobile/src/components/organization/invite-member-sheet.tsx - 1 suggestion
  • apps/mobile/src/components/organization/invite-member-validators.test.ts
  • apps/mobile/src/components/organization/invited-member-row.tsx
  • apps/mobile/src/components/organization/member-row.mounted.test.tsx
  • apps/mobile/src/components/organization/member-row.tsx
  • apps/mobile/src/components/organization/members-screen.tsx
  • apps/mobile/src/components/organization/organization-role-options.test.ts
  • apps/mobile/src/components/organization/organization-role-options.ts - 1 suggestion
  • apps/mobile/src/lib/server-error-copy.ts
  • apps/web/src/components/organizations/members/InviteMemberDialog.test.ts
  • apps/web/src/components/organizations/members/InviteMemberDialog.tsx
  • apps/web/src/lib/organizations/organization-seats.ts
  • apps/web/src/routers/organizations/organization-members-router.test.ts
  • apps/web/src/routers/organizations/organization-members-router.ts - 1 suggestion
  • packages/app-shared/src/organizations/index.ts
  • packages/app-shared/src/organizations/seat-capacity.test.ts
  • packages/app-shared/src/organizations/seat-capacity.ts
  • apps/mobile/src/i18n/locales/*.json (87 catalogs; new keys and placeholder sets verified)

Reviewed by deepseek-flash-latest · Input: 58.8K · Output: 8.1K · Cached: 1.2M

Review guidance: REVIEW.md from base branch main

The sheet suite stubs `useOrgBoundary` with an organization that carries only
an id. The invite sheet now reads the organization's plan, `require_seats` and
seat count to decide whether Send is available, so the stub pointed the seat
calculation at `undefined`.

Give that suite's organization the fields the sheet reads, with seat
enforcement off so the surfaces keep testing their own states.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant