fix(webapp): keep custom roles in the team role picker

The previous commit narrowed the picker with `isAtOrBelow`, which only
answers "yes" for roles that have a position on the system-role ladder.
Org-defined custom roles have no position, so they were filtered out for
everyone, and a viewer holding a custom role — no position either — was
offered nothing at all, leaving them with a dropdown containing only the
role each member already had. Neither was asked for.

Narrow subtractively instead: drop a role only where the ladder places it
strictly above the viewer, which is the only case picking it would always
be rejected. A new `isAbove` expresses that, and it is false whenever
either side is off the ladder, so custom roles stay offerable exactly as
before and an off-ladder or roleless viewer is not narrowed at all —
being unable to manage members is worse than being offered a role the
server may refuse. Plan-locked roles still render as "Name (upgrade)":
the offerable set and `assignableRoleIds` remain separate.

`offerableRoleIds` is no longer the invite flow's rule, so the invite
loader goes back to applying `isAtOrBelow` itself — ladder-only, then
intersected with the plan — which is the set it computed before this
branch. Its action was already doing its own check and is untouched.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: claude

.server-changes/team-role-picker-only-assignable-roles.md

@@ -3,4 +3,4 @@ area: webapp
 type: improvement
 ---
 
-The Team page's role dropdown now only lists roles you are actually allowed to assign, instead of showing higher roles that were rejected when you picked them. Roles that need a plan upgrade still appear, with a link to upgrade.
+The Team page's role dropdown no longer lists roles above your own, which were rejected when you picked them. Every other role you could pick before is still there, and roles that need a plan upgrade still appear with a link to upgrade.

apps/webapp/app/presenters/TeamPresenter.server.ts

@@ -42,23 +42,25 @@ export class TeamPresenter extends BasePresenter {
         organizationId
       ),
       // The viewer's own role, plus the system-role ladder it sits on —
-      // together these say how high this viewer is allowed to assign.
+      // together these say which roles sit above this viewer.
       rbac.getUserRole({ userId, organizationId }),
       rbac.systemRoles(organizationId),
     ]);
 
-    // Roles this viewer is allowed to hand out: at or below their own level
-    // on the system-role ladder. Deliberately NOT intersected with
-    // `assignableRoleIds` — the two answer different questions and the Team
-    // page renders them differently. A role above the viewer's level is left
-    // out of the picker altogether, while a role that is merely plan-locked
-    // still needs to appear with an upgrade link. Merging them would offer a
-    // viewer "Owner (upgrade)", inviting them to pay for something their own
-    // role still would not let them assign.
+    // Roles to offer this viewer in the picker: the catalogue minus whatever
+    // the system-role ladder puts strictly above their own role, which is the
+    // only part picking would always be rejected for. Roles the ladder can't
+    // place — org-defined custom roles, and every role when the viewer's own
+    // role is itself custom or missing — stay in, so custom roles keep
+    // behaving as they always have rather than vanishing from the picker.
     //
-    // Off-ladder roles (custom org roles, and any role held by a viewer who
-    // is themselves on a custom role) are refused, the same way the invite
-    // flow refuses them.
+    // Deliberately NOT intersected with `assignableRoleIds` — the two answer
+    // different questions and the Team page renders them differently. A role
+    // above the viewer's level is left out of the picker altogether, while a
+    // role that is merely plan-locked still needs to appear with an upgrade
+    // link. Merging them would offer a viewer "Owner (upgrade)", inviting
+    // them to pay for something their own role still would not let them
+    // assign.
     const offerableRoleIds = computeOfferableRoleIds(roles, systemRoles, viewerRole?.id ?? null);
 
     const memberRoles = result.members.map((m) => ({

apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx

@@ -77,18 +77,28 @@ export const loader = dashboardLoader(
       throw new Response("Not Found", { status: 404 });
     }
 
+    // Inviter's own role drives the "below their level" filter on the
+    // dropdown. Plus assignable role IDs already encode the org's plan
+    // tier — the intersection is what we offer.
+    const [inviterRole, assignableRoleIds, systemRoles] = await Promise.all([
+      rbac.getUserRole({ userId, organizationId }),
+      rbac.getAssignableRoleIds(organizationId),
+      rbac.systemRoles(organizationId),
+    ]);
+
     // Build the dropdown's offerable set server-side: roles that are
-    // (a) at or below the inviter's own level AND (b) assignable on the
-    // current plan. The client just renders these — it doesn't need to know
-    // about the system-role catalogue or the ladder.
-    //
-    // The presenter already applies the ladder; intersecting with the plan is
-    // right here, because the invite dropdown has no upgrade affordance and a
-    // plan-locked role is simply not offered. The Team page keeps the two
-    // sets apart instead, since it still shows plan-locked roles as an
-    // upgrade link — so it uses the presenter's `offerableRoleIds` unmerged.
-    const assignableSet = new Set(result.assignableRoleIds);
-    const offerableRoleIds = result.offerableRoleIds.filter((id) => assignableSet.has(id));
+    // (a) assignable on the current plan AND (b) at or below the
+    // inviter's own level. The client just renders these — it doesn't
+    // need to know about the system-role catalogue or the ladder.
+    const assignableSet = new Set(assignableRoleIds);
+    const offerableRoleIds = systemRoles
+      ? result.roles
+          .filter(
+            (r) =>
+              assignableSet.has(r.id) && isAtOrBelow(systemRoles, inviterRole?.id ?? null, r.id)
+          )
+          .map((r) => r.id)
+      : [];
 
     // Buying seats is a billing operation: surface whether this user can, so
     // the purchase modal disables its trigger (the team action enforces it).

apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx

@@ -697,8 +697,10 @@ function LeaveRemoveButton({
 // UI-affordance layer only.
 //
 // Two different sets narrow the list, and they must stay separate:
-//   offerableRoleIds — roles the viewer's own role lets them assign.
-//                      Anything else is left out of the list entirely.
+//   offerableRoleIds — the catalogue minus the roles that sit above the
+//                      viewer on the system-role ladder. Those are left out
+//                      of the list entirely; custom roles, which aren't on
+//                      the ladder, are always in it.
 //   assignableRoleIds — roles the org's plan allows. A role that is
 //                      offerable but not plan-assignable still shows,
 //                      as "Name (upgrade)" linking to billing.
@@ -721,13 +723,12 @@ function RolePicker({
   const fetcher = useFetcher<{ ok: boolean; error?: string } | { ok: true }>();
   const assignable = new Set(assignableRoleIds);
   const offerable = new Set(offerableRoleIds);
-  // The member's current role stays in the list even when the viewer could
-  // not assign it, so the controlled `value` below still resolves to a row
-  // and the dropdown shows the role the member actually holds.
+  // The member's current role stays in the list even when it sits above the
+  // viewer, so the controlled `value` below still resolves to a row and the
+  // dropdown shows the role the member actually holds.
   const visibleRoles = roles.filter((r) => offerable.has(r.id) || r.id === currentRoleId);
-  // With no RBAC plugin installed the loader returns no roles, and a viewer
-  // with nothing to offer would get an empty dropdown — render nothing
-  // rather than a dead control.
+  // With no RBAC plugin installed the loader returns no roles at all —
+  // render nothing rather than an empty dropdown.
   if (visibleRoles.length === 0) return null;
 
   const isSubmitting = fetcher.state === "submitting";
@@ -748,8 +749,8 @@ function RolePicker({
       text={(v) => visibleRoles.find((r) => r.id === v)?.name ?? "No role"}
       setValue={(next) => {
         if (typeof next !== "string" || next === (currentRoleId ?? "")) return;
-        // The member's current role is listed even when it isn't offerable,
-        // so re-check before submitting.
+        // The member's current role is listed even when it sits above the
+        // viewer, so re-check before submitting.
         if (!offerable.has(next)) return;
         // Upgrade-link rows have a value too (Ariakit needs one to
         // make the row interactive — without it the Link inside

apps/webapp/app/utils/inviteRoleLadder.ts

@@ -1,8 +1,13 @@
-// A user can only assign a role at or below their own — on invite, and on the
-// Team page's role picker. The systemRoles array is in canonical order
-// (highest authority first), so array index drives the ladder. Custom roles
-// aren't in the table and are refused. Dependency-free so the rule can be
-// unit-tested directly.
+// The system roles form a ladder: the systemRoles array is in canonical order
+// (highest authority first), so array index gives each role a level. Roles the
+// org defined itself are not in that array and have no level at all.
+//
+// Two callers read the ladder, and they treat a role with no level
+// differently. The invite flow (`isAtOrBelow`) requires a level on both sides
+// and refuses anything else. The Team page's role picker (`offerableRoleIds`)
+// only removes what the ladder positively places above the viewer, so custom
+// roles stay offerable. Dependency-free so both rules can be unit-tested
+// directly.
 
 export type LadderRole = { id: string };
 
@@ -34,20 +39,49 @@ export function isAtOrBelow(
 }
 
 /**
- * The subset of `roles` that a user holding `viewerRoleId` may assign, by the
- * ladder above. Knows nothing about plan gating: a role the org's plan does
- * not allow is still returned, so a caller that wants to surface it as an
- * upgrade affordance can. Callers that have no upgrade affordance intersect
- * with their plan-assignable set themselves.
+ * Whether the ladder places `candidateRoleId` strictly above `viewerRoleId`.
+ * Only ever true when both roles have a level: a custom role on either side
+ * is not comparable, so it is never "above", and nothing is above a viewer
+ * whose own role has no level.
+ */
+function isAbove(
+  roles: ReadonlyArray<LadderRole>,
+  viewerRoleId: string | null,
+  candidateRoleId: string
+): boolean {
+  if (!viewerRoleId) return false;
+  const level = buildRoleLevel(roles);
+  const viewer = level[viewerRoleId];
+  const candidate = level[candidateRoleId];
+  if (viewer === undefined || candidate === undefined) return false;
+  return candidate > viewer;
+}
+
+/**
+ * The subset of `roles` to offer a user holding `viewerRoleId` in a role
+ * picker. The narrowing is subtractive: a role is dropped only where the
+ * ladder puts it strictly above the viewer — the case where picking it would
+ * always be rejected. Everything the ladder can't place stays offerable:
+ *
+ * - org-defined custom roles, which have no level, are always offered;
+ * - a viewer whose own role has no level (a custom role, or no role at all)
+ *   is offered the whole catalogue, since narrowing to nothing would take
+ *   away their ability to manage members entirely — a worse outcome than
+ *   offering a role the server may go on to refuse.
+ *
+ * Knows nothing about plan gating: a role the org's plan does not allow is
+ * still returned, so a caller that wants to surface it as an upgrade
+ * affordance can. Callers with no upgrade affordance intersect with their
+ * plan-assignable set themselves.
  *
- * `systemRoles` is null when no RBAC plugin is installed — there is no ladder
- * to check against, so nothing is offerable.
+ * `systemRoles` is null when no RBAC plugin is installed — with no ladder
+ * there is nothing to narrow by, so `roles` comes back as-is.
  */
 export function offerableRoleIds(
   roles: ReadonlyArray<LadderRole>,
   systemRoles: ReadonlyArray<LadderRole> | null,
   viewerRoleId: string | null
 ): string[] {
-  if (!systemRoles) return [];
-  return roles.filter((r) => isAtOrBelow(systemRoles, viewerRoleId, r.id)).map((r) => r.id);
+  if (!systemRoles) return roles.map((r) => r.id);
+  return roles.filter((r) => !isAbove(systemRoles, viewerRoleId, r.id)).map((r) => r.id);
 }

apps/webapp/test/inviteRoleLadder.test.ts

@@ -32,19 +32,33 @@ describe("isAtOrBelow", () => {
   });
 });
 
-// Property under test: the picker/dropdown set is the ladder alone. Plan
-// gating is a separate concern the caller layers on, so a plan-locked role
-// must still come back here — the Team page renders it as an upgrade link.
+// Property under test: the picker set is the catalogue minus the roles the
+// ladder puts strictly above the viewer. Nothing else is removed — roles with
+// no ladder position (org-defined custom roles) stay offerable, and so does
+// the whole catalogue when the viewer's own role has no position either.
+// Plan gating is a separate concern the caller layers on, so a plan-locked
+// role must still come back here — the Team page renders it as an upgrade link.
 describe("offerableRoleIds", () => {
   const catalogue = [{ id: "owner" }, { id: "admin" }, { id: "member" }, { id: "custom-1" }];
 
   it("offers the viewer's own level and below", () => {
-    expect(offerableRoleIds(catalogue, roles, "admin")).toEqual(["admin", "member"]);
-    expect(offerableRoleIds(catalogue, roles, "member")).toEqual(["member"]);
+    expect(offerableRoleIds(catalogue, roles, "admin")).toEqual(["admin", "member", "custom-1"]);
+    expect(offerableRoleIds(catalogue, roles, "member")).toEqual(["member", "custom-1"]);
   });
 
   it("leaves roles above the viewer out entirely", () => {
     expect(offerableRoleIds(catalogue, roles, "admin")).not.toContain("owner");
+    expect(offerableRoleIds(catalogue, roles, "member")).not.toContain("owner");
+    expect(offerableRoleIds(catalogue, roles, "member")).not.toContain("admin");
+  });
+
+  it("offers the whole catalogue to a viewer at the top of the ladder", () => {
+    expect(offerableRoleIds(catalogue, roles, "owner")).toEqual([
+      "owner",
+      "admin",
+      "member",
+      "custom-1",
+    ]);
   });
 
   it("does not filter on plan gating — a plan-locked role is still offerable", () => {
@@ -53,12 +67,35 @@ describe("offerableRoleIds", () => {
     expect(offerableRoleIds(catalogue, roles, "owner")).toContain("owner");
   });
 
-  it("drops custom roles, which are not on the ladder", () => {
-    expect(offerableRoleIds(catalogue, roles, "owner")).not.toContain("custom-1");
+  it("keeps custom roles, which the ladder can't place above anyone", () => {
+    expect(offerableRoleIds(catalogue, roles, "owner")).toContain("custom-1");
+    expect(offerableRoleIds(catalogue, roles, "admin")).toContain("custom-1");
+    expect(offerableRoleIds(catalogue, roles, "member")).toContain("custom-1");
+  });
+
+  it("does not narrow at all for a viewer holding a custom role", () => {
+    // The ladder can't say what is above a role it doesn't list, so leave the
+    // picker as it was rather than emptying it and stranding the viewer.
+    expect(offerableRoleIds(catalogue, roles, "custom-1")).toEqual([
+      "owner",
+      "admin",
+      "member",
+      "custom-1",
+    ]);
   });
 
-  it("offers nothing to a roleless viewer or with no ladder at all", () => {
-    expect(offerableRoleIds(catalogue, roles, null)).toEqual([]);
-    expect(offerableRoleIds(catalogue, null, "owner")).toEqual([]);
+  it("does not narrow at all for a roleless viewer or with no ladder", () => {
+    expect(offerableRoleIds(catalogue, roles, null)).toEqual([
+      "owner",
+      "admin",
+      "member",
+      "custom-1",
+    ]);
+    expect(offerableRoleIds(catalogue, null, "owner")).toEqual([
+      "owner",
+      "admin",
+      "member",
+      "custom-1",
+    ]);
   });
 });