Keep a handoff grant revocable after somebody hides the coworker it points at

CHANGELOG.md

@@ -8,6 +8,17 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### Hiding a coworker no longer hides the grants pointing at it
+
+Hiding a coworker is a preference about your own roster — one row per person — and the grants saying
+which Bots may hand work to it are a deployment-wide fact an administrator set. The Handoff section
+joined the two, so hiding a coworker from your roster took every grant aimed at it off the screen:
+the switch was gone, no note said why, and the count above the list quietly dropped by one. Those
+grants were still in force, because a hop is decided by the grant and not by anybody's roster, so
+the coworker went on being asked while the only screen that could stop it had stopped listing it.
+A coworker you have hidden now appears in that list when a grant already points at it, marked as
+hidden from your roster, so it can be switched off. One you have hidden with nothing granted to it
+stays hidden.
 ### A tool call that failed no longer reads as one that worked
 
 The audit page draws a row it does not recognise as `Allowed`, which is right for the many rows that

app/src/components/agents/handoff-panel.tsx

@@ -15,6 +15,7 @@ import {
   ItemTitle,
 } from "@/components/ui/item";
 import { Switch } from "@/components/ui/switch";
+import { handoffRoster } from "@/lib/agents/handoff-roster";
 import { setHandoffGrantMutationOptions } from "@/lib/agents/mutations";
 import {
   agentHandoffQueryOptions,
@@ -38,29 +39,27 @@ export function HandoffPanel({ agentId }: { agentId: string }) {
   const queryClient = useQueryClient();
   const handoff = useQuery(agentHandoffQueryOptions(agentId));
   const agents = useQuery(agentListQueryOptions());
+  /*
+   * The roster this person has hidden, read so a grant pointing into it can still be taken away.
+   *
+   * Hiding is a per-person display preference and the grants are not filtered by it at all, so
+   * joining the grants against the visible roster alone dropped live grants off the only screen that
+   * manages them. See `handoffRoster`, which is where that join now happens.
+   */
+  const hiddenAgents = useQuery(agentListQueryOptions(true));
   const setGrant = useMutation(setHandoffGrantMutationOptions(queryClient));
 
   if (handoff.isPending || !handoff.data) return null;
   const { enabled, canGrant, reachable, grantable } = handoff.data;
 
-  /*
-   * A Bot may not be granted itself, and the server refuses it, so it is not offered here either.
-   * Hidden Bots are already absent from this list.
-   */
-  const others = (agents.data ?? []).filter(
-    (candidate) => candidate.id !== agentId,
-  );
-  const granted = others.filter((candidate) =>
-    reachable.includes(candidate.id),
-  ).length;
-  /*
-   * On a Bot that cannot be a grantee only the leftovers are shown: a stale grant may still be
-   * revoked — taking away is always allowed — but offering switches that can only bounce off the
-   * server's refusal is the thing the explanation item above replaces.
-   */
-  const candidates = grantable
-    ? others
-    : others.filter((candidate) => reachable.includes(candidate.id));
+  // A Bot may not be granted itself, and the server refuses it, so it is not offered here either.
+  const { candidates, granted, total } = handoffRoster({
+    agentId,
+    roster: agents.data ?? [],
+    hidden: hiddenAgents.data ?? [],
+    reachable,
+    grantable,
+  });
 
   // Nothing to say to somebody who cannot change it and has nothing to read.
   if (!canGrant && reachable.length === 0) return null;
@@ -72,9 +71,9 @@ export function HandoffPanel({ agentId }: { agentId: string }) {
           Bots it may ask
         </h2>
         {/* The current answer at a glance, so the list below is detail rather than homework. */}
-        {grantable && others.length > 0 ? (
+        {grantable && total > 0 ? (
           <span className="text-muted-foreground text-xs tabular-nums">
-            {granted} of {others.length}
+            {granted} of {total}
           </span>
         ) : null}
       </header>
@@ -121,7 +120,7 @@ export function HandoffPanel({ agentId }: { agentId: string }) {
         </p>
       ) : null}
 
-      {grantable && others.length === 0 ? (
+      {grantable && total === 0 ? (
         <Empty className="h-[180px] border border-dashed">
           <EmptyHeader>
             <EmptyTitle className="text-muted-foreground">
@@ -147,7 +146,16 @@ export function HandoffPanel({ agentId }: { agentId: string }) {
               </ItemMedia>
               <ItemContent>
                 <ItemTitle>{candidate.name}</ItemTitle>
-                <ItemDescription>{candidate.title}</ItemDescription>
+                {/*
+                 * Said on the row, because otherwise it is a coworker that is not on your roster
+                 * appearing in a list with no explanation. It is here only because this Bot may
+                 * already ask it, and that is the sentence a person needs to decide what to do.
+                 */}
+                <ItemDescription>
+                  {candidate.hidden
+                    ? `${candidate.title} · hidden from your roster`
+                    : candidate.title}
+                </ItemDescription>
               </ItemContent>
               <ItemActions>
                 <Switch

app/src/lib/agents/handoff-roster.ts

@@ -0,0 +1,88 @@
+/**
+ * Which coworkers the handoff panel draws a switch for, and what the count above them means.
+ *
+ * ITS OWN MODULE BECAUSE THE PANEL GOT THIS WRONG WHILE READING CORRECTLY. The panel joined the
+ * grants the server reports against the roster the browser holds, and those two are not filtered the
+ * same way. `reachable` is `botsReachableFrom`, a raw read of the `bot` grants on this coworker with
+ * no visibility filter of any kind. The roster is `GET /api/agents`, which drops every coworker the
+ * signed-in person has hidden — and hidden is a PER-PERSON display preference, one row per user in
+ * `agent_preferences`, not a fact about the coworker.
+ *
+ * So an administrator who tidied a coworker off their own roster stopped being shown its inbound
+ * grants. The switch was not disabled and no note appeared; the row was simply not drawn, and the
+ * `N of M` above it silently dropped by one. Nothing on any other screen manages these grants, so
+ * the grant could no longer be taken away at all.
+ *
+ * And it was still in force. The run loop asks `mayAddress`, which calls the same unfiltered
+ * `botsReachableFrom` (`server/src/index.ts:346-353`), so the hop kept working while the only
+ * surface that could stop it had quietly stopped listing it. A boundary you cannot see is one you
+ * cannot withdraw.
+ *
+ * The rule, in one place so the list and the count cannot disagree again:
+ *
+ *  - A coworker on your roster is offered as it always was.
+ *  - A coworker you have hidden is offered ONLY if it is already granted. Hiding is a preference
+ *    about clutter and this screen has no business undoing it, but "taking away is always allowed"
+ *    is what the panel promises, and a grant nobody can reach is the one thing it must not do.
+ *  - The count is over the rows actually drawn, so it answers the question somebody reading it is
+ *    asking: how many of these are on.
+ */
+
+/** The little a roster row has to carry for this to place it. */
+type Placeable = { id: string; hidden: boolean };
+
+export type HandoffRoster<T> = {
+  /** The rows to draw, in order: your roster first, then anything granted that you have hidden. */
+  candidates: T[];
+  /** How many of `candidates` are switched on. */
+  granted: number;
+  /** How many rows there are, which is what `granted` is out of. */
+  total: number;
+};
+
+export function handoffRoster<T extends Placeable>(input: {
+  /** The coworker whose panel this is. It may not be granted itself, so it is never a row. */
+  agentId: string;
+  /** `GET /api/agents`: everybody this person can see and has not hidden. */
+  roster: readonly T[];
+  /** `GET /api/agents?hidden=true`: the ones this person has hidden from that roster. */
+  hidden: readonly T[];
+  /** Bot ids this coworker may address today, exactly as the server reports them. */
+  reachable: readonly string[];
+  /**
+   * Whether this coworker can hold such a grant at all.
+   *
+   * False means every new grant would be refused, so only the leftovers are shown: a stale grant may
+   * still be revoked, and offering switches that can only bounce off the server is what the
+   * explanation above the list replaces.
+   */
+  grantable: boolean;
+}): HandoffRoster<T> {
+  const held = new Set(input.reachable);
+  const isSelf = (candidate: T) => candidate.id === input.agentId;
+
+  const offered = input.roster.filter(
+    (candidate) =>
+      !isSelf(candidate) && (input.grantable || held.has(candidate.id)),
+  );
+
+  /*
+   * Only the granted ones, and only those the roster did not already carry.
+   *
+   * The two lists are mutually exclusive per person — `list` filters on `hiddenAt` being null or not
+   * null — so the id check is belt and braces rather than a real case. It costs one Set and it stops
+   * a duplicate row with a duplicate React key if that ever stops being true.
+   */
+  const shown = new Set(offered.map((candidate) => candidate.id));
+  const strays = input.hidden.filter(
+    (candidate) =>
+      !isSelf(candidate) && held.has(candidate.id) && !shown.has(candidate.id),
+  );
+
+  const candidates = [...offered, ...strays];
+  return {
+    candidates,
+    granted: candidates.filter((candidate) => held.has(candidate.id)).length,
+    total: candidates.length,
+  };
+}

app/tests/handoff-roster.test.ts

@@ -0,0 +1,123 @@
+import { describe, expect, test } from "bun:test";
+import { handoffRoster } from "../src/lib/agents/handoff-roster";
+
+/**
+ * Which coworkers the handoff panel draws a switch for.
+ *
+ * The grants the server reports are not filtered by anybody's roster preferences, and the roster the
+ * browser holds is. Joining one against the other dropped live grants off the only screen that can
+ * take them away.
+ */
+
+const bot = (id: string, hidden = false) => ({
+  id,
+  hidden,
+  name: id,
+  title: `${id}'s job`,
+});
+
+describe("who the handoff panel offers a switch for", () => {
+  test("offers everybody else on the roster", () => {
+    const { candidates, granted, total } = handoffRoster({
+      agentId: "a",
+      roster: [bot("a"), bot("b"), bot("c")],
+      hidden: [],
+      reachable: ["b"],
+      grantable: true,
+    });
+
+    expect(candidates.map((candidate) => candidate.id)).toEqual(["b", "c"]);
+    expect(granted).toBe(1);
+    expect(total).toBe(2);
+  });
+
+  test("never offers a Bot itself, which the server refuses anyway", () => {
+    const { candidates } = handoffRoster({
+      agentId: "a",
+      roster: [bot("a")],
+      hidden: [],
+      reachable: [],
+      grantable: true,
+    });
+
+    expect(candidates).toEqual([]);
+  });
+
+  /*
+   * The bug. Hiding is one row per person in `agent_preferences`, and the grant is a deployment-wide
+   * fact an administrator set. Hide the grantee and the switch disappeared, while `mayAddress` went
+   * on letting the hop through.
+   */
+  test("still offers a granted coworker this person has hidden, so it can be revoked", () => {
+    const { candidates, granted, total } = handoffRoster({
+      agentId: "a",
+      roster: [bot("a"), bot("b")],
+      hidden: [bot("c", true)],
+      reachable: ["b", "c"],
+      grantable: true,
+    });
+
+    expect(candidates.map((candidate) => candidate.id)).toEqual(["b", "c"]);
+    expect(granted).toBe(2);
+    // And the count says two of two rather than one of one: the row is drawn, so it counts.
+    expect(total).toBe(2);
+  });
+
+  test("leaves a hidden coworker hidden when nothing was granted to it", () => {
+    // Hiding is a preference about clutter. This screen has no business undoing it for a coworker
+    // that has nothing to withdraw.
+    const { candidates, total } = handoffRoster({
+      agentId: "a",
+      roster: [bot("a"), bot("b")],
+      hidden: [bot("c", true)],
+      reachable: ["b"],
+      grantable: true,
+    });
+
+    expect(candidates.map((candidate) => candidate.id)).toEqual(["b"]);
+    expect(total).toBe(1);
+  });
+
+  test("shows only the leftovers on a coworker that cannot hold a grant", () => {
+    // Every new grant would be refused, so offering switches that can only bounce is noise. A stale
+    // grant is still shown, on the roster or off it, because taking away is always allowed.
+    const { candidates, granted } = handoffRoster({
+      agentId: "a",
+      roster: [bot("a"), bot("b"), bot("c")],
+      hidden: [bot("d", true), bot("e", true)],
+      reachable: ["b", "d"],
+      grantable: false,
+    });
+
+    expect(candidates.map((candidate) => candidate.id)).toEqual(["b", "d"]);
+    expect(granted).toBe(2);
+  });
+
+  test("draws one row for a coworker that somehow appears on both lists", () => {
+    // The two reads are mutually exclusive per person today. If that ever stops being true, a
+    // duplicate row is a duplicate React key, which is a worse failure than a missing note.
+    const { candidates, total } = handoffRoster({
+      agentId: "a",
+      roster: [bot("b")],
+      hidden: [bot("b", true)],
+      reachable: ["b"],
+      grantable: true,
+    });
+
+    expect(candidates.map((candidate) => candidate.id)).toEqual(["b"]);
+    expect(total).toBe(1);
+  });
+
+  test("counts nothing when this coworker may ask nobody", () => {
+    const { granted, total } = handoffRoster({
+      agentId: "a",
+      roster: [bot("a"), bot("b")],
+      hidden: [],
+      reachable: [],
+      grantable: true,
+    });
+
+    expect(granted).toBe(0);
+    expect(total).toBe(1);
+  });
+});