From b6003e6e3da1a4e5fe94a44278915589888582d6 Mon Sep 17 00:00:00 2001
From: Sawyer <sawyer@dirtroad.dev>
Date: Fri, 18 Sep 2026 09:17:45 -0700
Subject: [PATCH] fix(web): a room can never be the selected bench
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

resolveSelection/isBenchMembership used to treat any non-raw-id tenant
name as a bench, but rooms are named child tenants too — once
selectTenant ran with a room id, the room satisfied the same check and
became the selected bench, so /new, "+ Create agent", and the Agents
page all operated inside the room.

A bench is now defined as a top-level tenant: GET /api/tenants/:id's
parentId, fetched once per membership and never guessed from the
tenant's name. BenchProvider exposes the resolved set as
benchMemberships, which the command palette's switch-workbench and the
global Routines aggregation now read instead of re-deriving their own
name-based filter.
---
 apps/web/src/api.ts                           | 28 ++++++++
 apps/web/src/bench-context-value.ts           |  8 ++-
 apps/web/src/bench-context.test.ts            | 70 +++++++++++-------
 apps/web/src/bench-context.tsx                | 72 +++++++++++++------
 apps/web/src/command-palette-provider.tsx     | 13 ++--
 apps/web/src/global-routines.ts               | 19 ++---
 .../src/pages/insights-page-render.test.tsx   |  1 +
 apps/web/src/pending-approvals.test.tsx       | 11 +++
 apps/web/src/query-client.ts                  |  4 ++
 9 files changed, 155 insertions(+), 71 deletions(-)

diff --git a/apps/web/src/api.ts b/apps/web/src/api.ts
index 3362dcf92..2fe9517af 100644
--- a/apps/web/src/api.ts
+++ b/apps/web/src/api.ts
@@ -7,6 +7,7 @@ import {
   ApprovalResponse,
   AssetWithOriginResponse,
   PrincipalSummary,
+  TenantResponse,
   UserProfile,
   WorkflowRunSummary,
   paginatedSchema,
@@ -22,6 +23,7 @@ import { pathToQueryKey } from "./query-client";
 export const ProfileSchema = UserProfile;
 export const PrincipalsSchema = paginatedSchema(PrincipalSummary);
 export const TenantApprovalsSchema = paginatedSchema(ApprovalResponse);
+export const TenantDetailSchema = TenantResponse;
 
 // `GET /api/tenants/:tenantId/assets` returns a bare array of
 // `AssetWithOriginResponse` rows (not the paginated envelope), so the schema
@@ -73,6 +75,7 @@ export const ArtifactCountsSchema = type({
 
 export type Profile = typeof UserProfile.infer;
 export type Principal = typeof PrincipalSummary.infer;
+export type TenantDetail = typeof TenantResponse.infer;
 export type WorkflowRun = typeof WorkflowRunSummary.infer;
 export type Approval = typeof ApprovalResponse.infer;
 export type AssetRow = typeof AssetWithOriginResponse.infer;
@@ -181,6 +184,31 @@ export function rejectApproval(
   );
 }
 
+/**
+ * One-shot fetch of `GET /api/tenants/:id` — the only place `parentId`
+ * comes from. A bench is a top-level tenant (`parentId === null`); a room
+ * is a named child tenant, so the raw-id/name heuristic can never tell them
+ * apart. `bench-context.tsx` fans this out per membership with
+ * `useQueries` to decide which memberships are benches.
+ */
+export async function fetchTenantDetail(tenantId: string): Promise<TenantDetail> {
+  const response = await fetch(`/api/tenants/${encodeURIComponent(tenantId)}`, {
+    headers: { accept: "application/json" },
+  });
+  if (!response.ok) {
+    throw new ApiQueryError(
+      `The server answered ${response.status}.`,
+      response.status,
+      `tenant ${tenantId}`,
+    );
+  }
+  const parsed = TenantDetailSchema(await response.json());
+  if (parsed instanceof type.errors) {
+    throw new ApiQueryError(`Unexpected tenant response shape: ${parsed.summary}`);
+  }
+  return parsed;
+}
+
 /**
  * Sandboxed HTML preview URL for a Library artifact — the same
  * path an `<iframe sandbox>` in the canvas or Library detail pane loads,
diff --git a/apps/web/src/bench-context-value.ts b/apps/web/src/bench-context-value.ts
index 10f16b43e..53ff8f923 100644
--- a/apps/web/src/bench-context-value.ts
+++ b/apps/web/src/bench-context-value.ts
@@ -9,10 +9,16 @@
 import type { APIQuery } from "@/lib/api-query";
 import { createContext } from "react";
 
-import type { PrincipalsPage } from "./api";
+import type { Principal, PrincipalsPage } from "./api";
 
 export type BenchState = {
   readonly memberships: APIQuery<PrincipalsPage>;
+  /** The subset of `memberships` this account may treat as a bench: a
+   * top-level tenant (`parentId === null`, per `GET /api/tenants/:id`).
+   * Empty while any membership's tenant detail is still loading — every
+   * consumer that used to filter `memberships` with `isBenchMembership`
+   * reads this instead, so a room can never sneak into a bench list. */
+  readonly benchMemberships: readonly Principal[];
   readonly selectedTenantId: string | null;
   readonly selectedPrincipalId: string | null;
   readonly selectTenant: (tenantId: string) => void;
diff --git a/apps/web/src/bench-context.test.ts b/apps/web/src/bench-context.test.ts
index 35ecaff39..0de020047 100644
--- a/apps/web/src/bench-context.test.ts
+++ b/apps/web/src/bench-context.test.ts
@@ -15,69 +15,85 @@ function membership(overrides: Partial<Principal> & { tenantId: string }): Princ
   };
 }
 
+/** `null` = a top-level tenant (a bench); a string = its parent's id (a
+ * room, or any other child tenant). Missing entries model a tenant whose
+ * detail hasn't loaded yet. */
+function parents(entries: Record<string, string | null>): ReadonlyMap<string, string | null> {
+  return new Map(Object.entries(entries));
+}
+
 describe("resolveSelection", () => {
-  test("a named tenant sorting first wins — no kinds lookup to skip it", () => {
+  test("a bench sorting first wins over a room, regardless of name", () => {
     const memberships = [
-      membership({ tenantId: "tnt_first", tenantName: "Myra" }),
-      membership({ tenantId: "tnt_bench", tenantName: "Launch Team" }),
+      membership({ tenantId: "tnt_bench", tenantName: "Growth Team" }),
+      membership({ tenantId: "tnt_room", tenantName: "Launch Planning" }),
     ];
+    const parentByTenantId = parents({ tnt_bench: null, tnt_room: "tnt_bench" });
 
-    const resolved = resolveSelection(memberships, null);
+    const resolved = resolveSelection(memberships, null, parentByTenantId);
 
-    expect(resolved?.tenantId).toBe("tnt_first");
+    expect(resolved?.tenantId).toBe("tnt_bench");
   });
 
-  test("a raw-id tenant sorting first is skipped in favor of the first named bench", () => {
+  test("a room sorting first is skipped in favor of the first bench", () => {
     const memberships = [
-      membership({
-        tenantId: "tnt_raw",
-        tenantName: "ins_71f5c0c9c30026859014ccd9df8b1",
-      }),
-      membership({ tenantId: "tnt_bench", tenantName: "Launch Team" }),
+      membership({ tenantId: "tnt_room", tenantName: "Launch Planning" }),
+      membership({ tenantId: "tnt_bench", tenantName: "Growth Team" }),
     ];
+    const parentByTenantId = parents({ tnt_room: "tnt_bench", tnt_bench: null });
 
-    const resolved = resolveSelection(memberships, null);
+    const resolved = resolveSelection(memberships, null, parentByTenantId);
 
     expect(resolved?.tenantId).toBe("tnt_bench");
   });
 
   test("a stored selection that still names a bench wins over the first membership", () => {
     const memberships = [
-      membership({ tenantId: "tnt_bench_a", tenantName: "A" }),
-      membership({ tenantId: "tnt_bench_b", tenantName: "B" }),
+      membership({ tenantId: "tnt_bench_a" }),
+      membership({ tenantId: "tnt_bench_b" }),
     ];
+    const parentByTenantId = parents({ tnt_bench_a: null, tnt_bench_b: null });
 
-    const resolved = resolveSelection(memberships, "tnt_bench_b");
+    const resolved = resolveSelection(memberships, "tnt_bench_b", parentByTenantId);
 
     expect(resolved?.tenantId).toBe("tnt_bench_b");
   });
 
-  test("a stored selection naming a raw-id tenant falls through to the first named bench", () => {
+  test("a stored selection naming a room falls through to the first bench — a room can never be selected even when it is the stored id", () => {
     const memberships = [
-      membership({
-        tenantId: "tnt_raw",
-        tenantName: "ins_71f5c0c9c30026859014ccd9df8b1",
-      }),
-      membership({ tenantId: "tnt_bench", tenantName: "Launch Team" }),
+      membership({ tenantId: "tnt_room" }),
+      membership({ tenantId: "tnt_bench" }),
     ];
+    const parentByTenantId = parents({ tnt_room: "tnt_bench", tnt_bench: null });
 
-    const resolved = resolveSelection(memberships, "tnt_raw");
+    const resolved = resolveSelection(memberships, "tnt_room", parentByTenantId);
 
     expect(resolved?.tenantId).toBe("tnt_bench");
   });
 
   test("a stored selection for a tenant no longer in memberships falls through", () => {
-    const memberships = [membership({ tenantId: "tnt_bench", tenantName: "Launch Team" })];
+    const memberships = [membership({ tenantId: "tnt_bench" })];
+    const parentByTenantId = parents({ tnt_bench: null });
 
-    const resolved = resolveSelection(memberships, "tnt_gone");
+    const resolved = resolveSelection(memberships, "tnt_gone", parentByTenantId);
 
     expect(resolved?.tenantId).toBe("tnt_bench");
   });
 
-  test("undefined when every membership is a raw-id tenant", () => {
-    const memberships = [membership({ tenantId: "tnt_raw", tenantName: "tnt_raw" })];
+  test("undefined when every membership is a room", () => {
+    const memberships = [membership({ tenantId: "tnt_room" })];
+    const parentByTenantId = parents({ tnt_room: "tnt_primary" });
+
+    const resolved = resolveSelection(memberships, null, parentByTenantId);
+
+    expect(resolved).toBeUndefined();
+  });
+
+  test("undefined while a tenant's parent hasn't loaded yet — never guessed as a bench", () => {
+    const memberships = [membership({ tenantId: "tnt_unknown" })];
+    const parentByTenantId = parents({});
 
-    const resolved = resolveSelection(memberships, null);
+    const resolved = resolveSelection(memberships, null, parentByTenantId);
 
     expect(resolved).toBeUndefined();
   });
diff --git a/apps/web/src/bench-context.tsx b/apps/web/src/bench-context.tsx
index aaff5c17f..3494844f2 100644
--- a/apps/web/src/bench-context.tsx
+++ b/apps/web/src/bench-context.tsx
@@ -4,12 +4,11 @@
 // bench (the chat page, the benches page, the header switcher) reads this
 // context instead of re-deriving "membership[0]" on its own.
 
-import { isRawIdentifier } from "@/bench";
-import { useQueryClient } from "@tanstack/react-query";
+import { useQueries, useQueryClient } from "@tanstack/react-query";
 import { useContext, useMemo, useState } from "react";
 import type { ReactNode } from "react";
 
-import { PrincipalsSchema, useAPIQuery } from "./api";
+import { fetchTenantDetail, PrincipalsSchema, useAPIQuery } from "./api";
 import type { Principal } from "./api";
 import { BenchContext } from "./bench-context-value";
 import type { BenchState } from "./bench-context-value";
@@ -37,33 +36,33 @@ function writeStoredTenantId(tenantId: string): void {
   }
 }
 
-/** The membership this context currently treats as selected: the stored
- * choice if it still names a bench the account belongs to, otherwise the
- * first named membership — the same personal-bench convention
- * `chat-page.tsx` used to apply inline, minus the raw-id tenancies that
- * same unfiltered "first membership" pick let default in.
- *
- * A bench is a membership with a human-assigned name: a tenant whose name
- * is a raw platform id never hosts the shell. The server-side kinds lookup
- * (`POST /api/workbench-tenancies/kinds`) is gone with chat's
- * `workbench_tenancy` table — child-tenant exclusion now lives in the
- * client-held workbench list (`needs-list.ts`'s `childTenantStore`), not in
- * this selector.
- *
- * True for a membership the shell may treat as a bench: named, never raw. */
-export function isBenchMembership(membership: Principal): boolean {
-  return !isRawIdentifier(membership.tenantName);
+/** True for a membership the shell may treat as a bench: a top-level
+ * tenant, i.e. one whose `GET /api/tenants/:id` reports `parentId: null`.
+ * Rooms are named child tenants (`needs-converge.ts`'s `POST /api/tenants
+ * { parentId }`), so a name-based heuristic can never tell a room from a
+ * bench — only the tenant's own parent can. `parentByTenantId` holds
+ * `undefined` for a tenant whose detail hasn't loaded yet, which this
+ * treats as "not (yet known to be) a bench" rather than guessing. */
+export function isBenchMembership(
+  membership: Principal,
+  parentByTenantId: ReadonlyMap<string, string | null>,
+): boolean {
+  return parentByTenantId.get(membership.tenantId) === null;
 }
 
+/** The membership this context currently treats as selected: the stored
+ * choice if it still names a bench the account belongs to, otherwise the
+ * first bench membership. */
 export function resolveSelection(
   memberships: readonly Principal[],
   stored: string | null,
+  parentByTenantId: ReadonlyMap<string, string | null>,
 ): Principal | undefined {
   const storedMatch = stored !== null ? memberships.find((m) => m.tenantId === stored) : undefined;
-  if (storedMatch !== undefined && isBenchMembership(storedMatch)) {
+  if (storedMatch !== undefined && isBenchMembership(storedMatch, parentByTenantId)) {
     return storedMatch;
   }
-  return memberships.find((m) => isBenchMembership(m));
+  return memberships.find((m) => isBenchMembership(m, parentByTenantId));
 }
 
 export function BenchProvider({ children }: { readonly children: ReactNode }) {
@@ -71,8 +70,34 @@ export function BenchProvider({ children }: { readonly children: ReactNode }) {
   const memberships = useAPIQuery("/api/me/principals", PrincipalsSchema);
   const [stored, setStored] = useState<string | null>(() => readStoredTenantId());
 
+  const membershipTenantIds =
+    memberships.kind === "ready" ? memberships.data.data.map((m) => m.tenantId) : [];
+  // One `GET /api/tenants/:id` per membership — the only place `parentId`
+  // comes from. `useQueries` fans a dynamic list of reads out over stable
+  // per-tenant cache entries, shared with any other reader of the same key.
+  const tenantDetailResults = useQueries({
+    queries: membershipTenantIds.map((tenantId) => ({
+      queryKey: tenantKeys.detail(tenantId),
+      queryFn: () => fetchTenantDetail(tenantId),
+      staleTime: 30_000,
+    })),
+  });
+  // A handful of memberships at most, so this is cheap to rebuild every
+  // render rather than chase a stable memo key across two parallel arrays.
+  const parentByTenantId = new Map<string, string | null>();
+  membershipTenantIds.forEach((tenantId, index) => {
+    const detail = tenantDetailResults[index]?.data;
+    if (detail !== undefined) parentByTenantId.set(tenantId, detail.parentId ?? null);
+  });
+
   const resolved =
-    memberships.kind === "ready" ? resolveSelection(memberships.data.data, stored) : undefined;
+    memberships.kind === "ready"
+      ? resolveSelection(memberships.data.data, stored, parentByTenantId)
+      : undefined;
+  const benchMemberships =
+    memberships.kind === "ready"
+      ? memberships.data.data.filter((m) => isBenchMembership(m, parentByTenantId))
+      : [];
 
   // The resolved bench is the stored one: written during render so no
   // consumer reads a selection the store disagrees with.
@@ -84,6 +109,7 @@ export function BenchProvider({ children }: { readonly children: ReactNode }) {
   const value = useMemo<BenchState>(
     () => ({
       memberships,
+      benchMemberships,
       selectedTenantId: resolved?.tenantId ?? null,
       selectedPrincipalId: resolved?.principalId ?? null,
       selectTenant: (tenantId: string) => {
@@ -102,7 +128,7 @@ export function BenchProvider({ children }: { readonly children: ReactNode }) {
         void queryClient.invalidateQueries({ queryKey: meKeys.principals });
       },
     }),
-    [memberships, resolved, stored, queryClient],
+    [memberships, benchMemberships, resolved, stored, queryClient],
   );
 
   return <BenchContext.Provider value={value}>{children}</BenchContext.Provider>;
diff --git a/apps/web/src/command-palette-provider.tsx b/apps/web/src/command-palette-provider.tsx
index 31bc3ddc9..241e1721e 100644
--- a/apps/web/src/command-palette-provider.tsx
+++ b/apps/web/src/command-palette-provider.tsx
@@ -31,7 +31,7 @@ import { WORKBENCH_NOT_FOUND_EVENT } from "./workbench-not-found-event";
 import { recentsStoreForBench } from "./command-palette-recents";
 import { NAV_ROUTES } from "./routes";
 import { ArtifactListPageSchema, useAPIQuery } from "./api";
-import { isBenchMembership, useBench } from "./bench-context";
+import { useBench } from "./bench-context";
 import { useCloseCanvas } from "./shell/canvas-availability";
 import { SKILLS_PATH_PREFIX } from "./path-ids";
 import { listScheduledWorkflows, runScheduledWorkflowNow, useTenantQuery } from "./routines-api";
@@ -69,7 +69,7 @@ export function CommandPaletteProvider({
   readonly navigate: Navigate;
   readonly children: ReactNode;
 }) {
-  const { memberships, selectedTenantId, selectTenant } = useBench();
+  const { benchMemberships, selectedTenantId, selectTenant } = useBench();
   const queryClient = useQueryClient();
   // Open state and query live in the shared store, not in this component:
   // Cmd+K and a context-menu item both open this surface from outside the
@@ -220,12 +220,9 @@ export function CommandPaletteProvider({
   // simplest honest thing a single command-palette entry can do without
   // reinventing a picker. Absent entirely for the common one-workbench
   // account, same principle the old dock used to hide itself by. Benches
-  // are the named memberships — the kinds lookup is gone, so a raw-id
-  // tenancy is the only thing filtered out here.
-  const workbenchMemberships =
-    memberships.kind === "ready"
-      ? memberships.data.data.filter((membership) => isBenchMembership(membership))
-      : [];
+  // are top-level memberships (`useBench`'s `benchMemberships`) — a room
+  // (a named child tenant) can never be cycled to.
+  const workbenchMemberships = benchMemberships;
   const nextWorkbench =
     workbenchMemberships.length > 1
       ? workbenchMemberships[
diff --git a/apps/web/src/global-routines.ts b/apps/web/src/global-routines.ts
index 654ce0897..0c56df77a 100644
--- a/apps/web/src/global-routines.ts
+++ b/apps/web/src/global-routines.ts
@@ -8,8 +8,7 @@ import { useQueries, useQueryClient } from "@tanstack/react-query";
 import { describeApiError } from "@/lib/api-query";
 import type { APIQuery } from "@/lib/api-query";
 
-import type { Principal } from "./api";
-import { isBenchMembership, useBench } from "./bench-context";
+import { useBench } from "./bench-context";
 import { WORKFLOWS_PATH_PREFIX } from "./path-ids";
 import { tenantKeys } from "./query-client";
 import {
@@ -35,17 +34,13 @@ function useMemberBenches(): {
   readonly kind: "loading" | "ready";
   readonly benches: readonly { tenantId: string; tenantName: string }[];
 } {
-  const { memberships } = useBench();
-  const allMemberships: readonly Principal[] =
-    memberships.kind === "ready" ? memberships.data.data : [];
-  // No kinds lookup: a bench is a named membership, and the Routines
-  // roster aggregates per bench. Raw-id tenancies never host routines.
+  const { memberships, benchMemberships } = useBench();
+  // The Routines roster aggregates per bench; `useBench`'s
+  // `benchMemberships` is already the top-level subset — a room (a named
+  // child tenant) never hosts routines here.
   const benches = useMemo(
-    () =>
-      allMemberships
-        .filter((m) => isBenchMembership(m))
-        .map((m) => ({ tenantId: m.tenantId, tenantName: m.tenantName })),
-    [allMemberships],
+    () => benchMemberships.map((m) => ({ tenantId: m.tenantId, tenantName: m.tenantName })),
+    [benchMemberships],
   );
   if (memberships.kind !== "ready") return { kind: "loading", benches: [] };
   return { kind: "ready", benches };
diff --git a/apps/web/src/pages/insights-page-render.test.tsx b/apps/web/src/pages/insights-page-render.test.tsx
index 69f5d7123..db05feb8f 100644
--- a/apps/web/src/pages/insights-page-render.test.tsx
+++ b/apps/web/src/pages/insights-page-render.test.tsx
@@ -41,6 +41,7 @@ const readyEmpty = <T,>(data: T): APIQuery<T> => ({ kind: "ready", data });
 
 const benchState: BenchState = {
   memberships: { kind: "ready", data: { data: [], nextCursor: null } },
+  benchMemberships: [],
   selectedTenantId: "tnt_bench_a",
   selectedPrincipalId: "prn_bench_a",
   selectTenant: () => {},
diff --git a/apps/web/src/pending-approvals.test.tsx b/apps/web/src/pending-approvals.test.tsx
index ce4624b36..d0bbe8258 100644
--- a/apps/web/src/pending-approvals.test.tsx
+++ b/apps/web/src/pending-approvals.test.tsx
@@ -37,6 +37,17 @@ const benchState: BenchState = {
       nextCursor: null,
     },
   },
+  benchMemberships: [
+    {
+      principalId: "prn_1",
+      tenantId: TENANT_ID,
+      tenantName: "Growth Team Bench",
+      tenantSlug: "growth-team-bench",
+      kind: "user",
+      status: "active",
+      roles: [],
+    },
+  ],
   selectedTenantId: TENANT_ID,
   selectedPrincipalId: "prn_1",
   selectTenant: () => undefined,
diff --git a/apps/web/src/query-client.ts b/apps/web/src/query-client.ts
index 32d1552e8..df8062f8f 100644
--- a/apps/web/src/query-client.ts
+++ b/apps/web/src/query-client.ts
@@ -77,6 +77,10 @@ export const meKeys = {
 /** Tenant-scoped keys — removed wholesale when the user leaves a bench. */
 export const tenantKeys = {
   all: (tenantId: string) => ["tenant", tenantId] as const,
+  /** `GET /api/tenants/:id` — the only source of `parentId`, so this is the
+   * key `bench-context.tsx` fans out per membership to tell a bench
+   * (`parentId === null`) apart from a room (a named child tenant). */
+  detail: (tenantId: string) => ["tenant", tenantId, "detail"] as const,
   pendingApprovals: (tenantId: string) => ["tenant", tenantId, "approvals"] as const,
   /** One agent-name read per run, shared by every approval that run raised
    * (see `pending-approvals.ts`). */