chore(web): collapse App.tsx's six session refs into useSessionRef (#2129)

Step 1 of phase 2 of the App.tsx decomposition, landed first and alone
because it is the seam the remaining clusters are extracted onto.

App.tsx kept six "latest value" refs so long-lived callbacks could read a
current value without listing it as a dependency and being re-created on
every change: `activeServerNameRef`, `activeServerIdRef`, `serversRef`,
`inspectorClientRef`, `pendingStepUpRef` and `pendingReauthRef`, plus the
three passive effects that synced them. `useSessionRef` replaces all nine
with one hook returning one stable ref onto a `SessionSnapshot`.

This is not a store: nothing renders off it, nothing subscribes to it, and
mutating it never schedules a render. It is not the prop-to-state sync
AGENTS.md forbids either — that rule is about `setState` in an effect, and
a ref write produces no render at all. It is exactly the pattern the file
already used, written once instead of six times.

What it buys the later steps is a *stable* dependency: a hook taking
`sessionRef` does not drag the other clusters into its argument list, so
`useOAuthRecovery`, `useConnectionLifecycle` and `useServerCommands` can
be extracted one PR at a time.

Behavior is unchanged:

- The snapshot is seeded from the first render's values, matching what
  each `useRef(value)` seeded.
- `activeServerName` keeps its one non-uniform rule — it holds the last
  server that *was* active and is never cleared, because a transport
  crash clears `activeServerId` before the failure toast is raised. It is
  derived from `servers`/`activeServerId` rather than taking the
  `activeServer` memo, which is the same value by definition and is what
  lets the hook be called ahead of that memo.
- The two pending-* syncs already ran ahead of the only effect that reads
  them, so merging them changes no effect's view. The other four synced
  below every effect that reads them, so hoisting that sync changes none
  either.
- The sync effect is deliberately un-gated rather than carrying a
  dependency list: re-assigning unchanged values costs nothing and
  removes a place for a newly-added field to be forgotten.

`sessionRef` is declared ahead of its first reader because a hook return
is not provably stable to `react-hooks/exhaustive-deps` — consumers must
list it, and a later declaration would put every one of them in the
temporal dead zone. Listing a ref that never changes identity re-runs
nothing.

The inline `pendingStepUp` state shape is named `PendingStepUp` and moved
beside `StepUpSource` in `utils/stepUp.ts`, so the snapshot can name it
without importing from the component that owns the state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MZdpUwhHf943FoprcLVzEm
Signed-off-by: cliffhall <cliff@futurescale.com>

clients/web/src/hooks/useSessionRef.test.tsx

@@ -0,0 +1,190 @@
+import { describe, it, expect } from "vitest";
+import type { InspectorClient } from "@inspector/core/mcp/index.js";
+import type { ServerEntry } from "@inspector/core/mcp/types.js";
+import { renderWithMantine } from "../test/renderWithMantine";
+import type { PendingReauth } from "../utils/pendingReauth";
+import type { PendingStepUp } from "../utils/stepUp";
+import {
+  useSessionRef,
+  type SessionRef,
+  type SessionValues,
+} from "./useSessionRef";
+
+const entry = (id: string, name = id): ServerEntry => ({
+  id,
+  name,
+  config: { type: "stdio", command: "node" },
+  connection: { status: "disconnected" },
+});
+
+/** A stand-in for the live client — the hook only ever stores the reference. */
+const client = (tag: string) => ({ tag }) as unknown as InspectorClient;
+
+const stepUp = (serverId: string): PendingStepUp => ({
+  serverId,
+  challenge: { reason: "unauthorized" },
+  authorizationUrl: new URL("https://as.example/authorize"),
+  source: "tool",
+});
+
+const reauth = (serverId: string): PendingReauth => ({
+  serverId,
+  challenge: { reason: "unauthorized" },
+  authorizationUrl: new URL("https://as.example/authorize"),
+  authKind: "reauth",
+  source: "tool",
+});
+
+const values = (over: Partial<SessionValues> = {}): SessionValues => ({
+  activeServerId: undefined,
+  servers: [],
+  inspectorClient: null,
+  pendingStepUp: null,
+  pendingReauth: null,
+  ...over,
+});
+
+interface Harness {
+  ref: () => SessionRef;
+  /** Re-render with a new set of session values, as App.tsx does each render. */
+  update: (next: SessionValues) => void;
+}
+
+function harness(initial: SessionValues): Harness {
+  let latest: SessionRef | undefined;
+  function Probe({ v }: { v: SessionValues }) {
+    latest = useSessionRef(v);
+    return null;
+  }
+  const { rerender } = renderWithMantine(<Probe v={initial} />);
+  return {
+    ref: () => {
+      if (!latest) throw new Error("hook did not render");
+      return latest;
+    },
+    update: (next) => rerender(<Probe v={next} />),
+  };
+}
+
+describe("useSessionRef", () => {
+  it("seeds the snapshot from the first render's values", () => {
+    const servers = [entry("a")];
+    const c = client("first");
+    const h = harness(
+      values({ activeServerId: "a", servers, inspectorClient: c }),
+    );
+    const snapshot = h.ref().current;
+    expect(snapshot.activeServerId).toBe("a");
+    expect(snapshot.servers).toBe(servers);
+    expect(snapshot.inspectorClient).toBe(c);
+    expect(snapshot.pendingStepUp).toBeNull();
+    expect(snapshot.pendingReauth).toBeNull();
+  });
+
+  it("keeps one stable ref identity across renders", () => {
+    const h = harness(values());
+    const first = h.ref();
+    h.update(values({ activeServerId: "a", servers: [entry("a")] }));
+    expect(h.ref()).toBe(first);
+  });
+
+  it("mirrors every field on a later render", () => {
+    const h = harness(values());
+    const servers = [entry("a"), entry("b")];
+    const c = client("later");
+    const up = stepUp("a");
+    const re = reauth("b");
+    h.update(
+      values({
+        activeServerId: "b",
+        servers,
+        inspectorClient: c,
+        pendingStepUp: up,
+        pendingReauth: re,
+      }),
+    );
+    expect(h.ref().current).toMatchObject({
+      activeServerId: "b",
+      servers,
+      inspectorClient: c,
+      pendingStepUp: up,
+      pendingReauth: re,
+    });
+  });
+
+  it("clears a value that becomes null or undefined", () => {
+    const h = harness(
+      values({
+        activeServerId: "a",
+        servers: [entry("a")],
+        inspectorClient: client("x"),
+        pendingStepUp: stepUp("a"),
+        pendingReauth: reauth("a"),
+      }),
+    );
+    h.update(values());
+    expect(h.ref().current).toMatchObject({
+      activeServerId: undefined,
+      servers: [],
+      inspectorClient: null,
+      pendingStepUp: null,
+      pendingReauth: null,
+    });
+  });
+
+  describe("activeServerName", () => {
+    it("stays undefined when no server was ever active", () => {
+      // It is derived in the sync effect rather than seeded, so it is blank
+      // until a render actually resolves an active entry.
+      const h = harness(values({ servers: [entry("a", "Alpha")] }));
+      expect(h.ref().current.activeServerName).toBeUndefined();
+    });
+
+    it("is populated by the mount's own sync effect", () => {
+      const h = harness(
+        values({ activeServerId: "a", servers: [entry("a", "Alpha")] }),
+      );
+      expect(h.ref().current.activeServerName).toBe("Alpha");
+    });
+
+    it("tracks the active server's name", () => {
+      const h = harness(values());
+      h.update(values({ activeServerId: "a", servers: [entry("a", "Alpha")] }));
+      expect(h.ref().current.activeServerName).toBe("Alpha");
+      h.update(
+        values({
+          activeServerId: "b",
+          servers: [entry("a", "Alpha"), entry("b", "Beta")],
+        }),
+      );
+      expect(h.ref().current.activeServerName).toBe("Beta");
+    });
+
+    it("keeps the last active name when the active server goes away", () => {
+      // The case the stickiness exists for: a transport crash clears
+      // `activeServerId` before the failure toast is raised, and the name is
+      // the only surviving handle on which server died.
+      const h = harness(values());
+      h.update(values({ activeServerId: "a", servers: [entry("a", "Alpha")] }));
+      h.update(values({ servers: [entry("a", "Alpha")] }));
+      expect(h.ref().current.activeServerName).toBe("Alpha");
+      expect(h.ref().current.activeServerId).toBeUndefined();
+    });
+
+    it("keeps the last active name when the id names no entry", () => {
+      const h = harness(values());
+      h.update(values({ activeServerId: "a", servers: [entry("a", "Alpha")] }));
+      h.update(values({ activeServerId: "gone", servers: [] }));
+      expect(h.ref().current.activeServerName).toBe("Alpha");
+    });
+
+    it("follows a rename of the active server", () => {
+      const h = harness(values());
+      h.update(values({ activeServerId: "a", servers: [entry("a", "Alpha")] }));
+      h.update(
+        values({ activeServerId: "a", servers: [entry("a", "Renamed")] }),
+      );
+      expect(h.ref().current.activeServerName).toBe("Renamed");
+    });
+  });
+});

clients/web/src/hooks/useSessionRef.ts

@@ -0,0 +1,80 @@
+import { useEffect, useRef } from "react";
+import type { RefObject } from "react";
+import type { InspectorClient } from "@inspector/core/mcp/index.js";
+import type { ServerEntry } from "@inspector/core/mcp/types.js";
+import type { PendingReauth } from "../utils/pendingReauth";
+import type { PendingStepUp } from "../utils/stepUp";
+
+/**
+ * The session values App.tsx's long-lived callbacks read *currently* rather
+ * than as of the render that created them.
+ *
+ * `activeServerName` is the one field with a rule of its own — see the hook.
+ */
+export interface SessionSnapshot {
+  /** Name of the last server that was active; see `useSessionRef`. */
+  activeServerName: string | undefined;
+  activeServerId: string | undefined;
+  servers: ServerEntry[];
+  /**
+   * The live client. Mirrored because a settings write that outlives a
+   * disconnect/reconnect to the *same* server would otherwise push its value
+   * into the instance captured when it was issued — which has since been
+   * destroyed — while the replacement client, built from the stale list entry,
+   * keeps the value the write replaced (#2089).
+   */
+  inspectorClient: InspectorClient | null;
+  pendingStepUp: PendingStepUp | null;
+  pendingReauth: PendingReauth | null;
+}
+
+/** What the caller supplies each render; `activeServerName` is derived. */
+export type SessionValues = Omit<SessionSnapshot, "activeServerName">;
+
+/** A stable handle onto the latest {@link SessionSnapshot}. */
+export type SessionRef = RefObject<SessionSnapshot>;
+
+/**
+ * Mirrors the session values into a single stable ref, so a callback can read
+ * the current value without listing it as a dependency and being re-created on
+ * every change.
+ *
+ * This is the standard "latest ref" pattern, written once instead of once per
+ * value — App.tsx previously kept six such refs and three sync effects. It is
+ * **not** a store: nothing renders off it, nothing subscribes to it, and
+ * mutating it never schedules a render. It is also not the prop→state sync
+ * that AGENTS.md forbids, which is about `setState` in an effect; a ref write
+ * produces no render at all, which is the whole point.
+ *
+ * Because the ref identity never changes, a hook that takes it takes a
+ * *stable* dependency — which is what lets App.tsx's coupled clusters be
+ * extracted one at a time without each one dragging the others into its
+ * argument list.
+ *
+ * The single non-uniform field is `activeServerName`: it holds the name of the
+ * last server that *was* active and is never cleared. A transport crash
+ * dispatches `disconnect`, which clears `activeServerId` — and therefore the
+ * active entry — before the failure toast is raised, so the sticky name is the
+ * only surviving handle on which server just died.
+ */
+export function useSessionRef(values: SessionValues): SessionRef {
+  const ref = useRef<SessionSnapshot>({
+    activeServerName: undefined,
+    ...values,
+  });
+  // Deliberately un-gated: this is a passive mirror, so re-assigning the same
+  // values on a render that changed none of them costs nothing and removes the
+  // dependency list as a place for a newly-added field to be forgotten.
+  useEffect(() => {
+    const activeServer = values.servers.find(
+      (s) => s.id === values.activeServerId,
+    );
+    if (activeServer) ref.current.activeServerName = activeServer.name;
+    ref.current.activeServerId = values.activeServerId;
+    ref.current.servers = values.servers;
+    ref.current.inspectorClient = values.inspectorClient;
+    ref.current.pendingStepUp = values.pendingStepUp;
+    ref.current.pendingReauth = values.pendingReauth;
+  });
+  return ref;
+}