refactor(nodes): share default selection and tighten node.list fallback

src/agents/tools/browser-tool.ts

@@ -29,7 +29,12 @@ import { wrapExternalContent } from "../../security/external-content.js";
 import { BrowserToolSchema } from "./browser-tool.schema.js";
 import { type AnyAgentTool, imageResultFromFile, jsonResult, readStringParam } from "./common.js";
 import { callGatewayTool } from "./gateway.js";
-import { listNodes, resolveNodeIdFromList, type NodeListNode } from "./nodes-utils.js";
+import {
+  listNodes,
+  resolveNodeIdFromList,
+  selectDefaultNodeFromList,
+  type NodeListNode,
+} from "./nodes-utils.js";
 
 function wrapBrowserExternalJson(params: {
   kind: "snapshot" | "console" | "tabs";
@@ -143,10 +148,17 @@ async function resolveBrowserNodeTarget(params: {
     return { nodeId, label: node?.displayName ?? node?.remoteIp ?? nodeId };
   }
 
+  const selected = selectDefaultNodeFromList(browserNodes, {
+    preferLocalMac: false,
+    fallback: "none",
+  });
+
   if (params.target === "node") {
-    if (browserNodes.length === 1) {
-      const node = browserNodes[0];
-      return { nodeId: node.nodeId, label: node.displayName ?? node.remoteIp ?? node.nodeId };
+    if (selected) {
+      return {
+        nodeId: selected.nodeId,
+        label: selected.displayName ?? selected.remoteIp ?? selected.nodeId,
+      };
     }
     throw new Error(
       `Multiple browser-capable nodes connected (${browserNodes.length}). Set gateway.nodes.browser.node or pass node=<id>.`,
@@ -157,9 +169,11 @@ async function resolveBrowserNodeTarget(params: {
     return null;
   }
 
-  if (browserNodes.length === 1) {
-    const node = browserNodes[0];
-    return { nodeId: node.nodeId, label: node.displayName ?? node.remoteIp ?? node.nodeId };
+  if (selected) {
+    return {
+      nodeId: selected.nodeId,
+      label: selected.displayName ?? selected.remoteIp ?? selected.nodeId,
+    };
   }
   return null;
 }

src/agents/tools/nodes-utils.test.ts

@@ -1,6 +1,14 @@
-import { describe, expect, it } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const gatewayMocks = vi.hoisted(() => ({
+  callGatewayTool: vi.fn(),
+}));
+vi.mock("./gateway.js", () => ({
+  callGatewayTool: (...args: unknown[]) => gatewayMocks.callGatewayTool(...args),
+}));
+
 import type { NodeListNode } from "./nodes-utils.js";
-import { resolveNodeIdFromList } from "./nodes-utils.js";
+import { listNodes, resolveNodeIdFromList } from "./nodes-utils.js";
 
 function node(overrides: Partial<NodeListNode> & { nodeId: string }): NodeListNode {
   return {
@@ -11,14 +19,18 @@ function node(overrides: Partial<NodeListNode> & { nodeId: string }): NodeListNo
   };
 }
 
+beforeEach(() => {
+  gatewayMocks.callGatewayTool.mockReset();
+});
+
 describe("resolveNodeIdFromList defaults", () => {
-  it("falls back to first connected canvas-capable node when multiple non-Mac candidates exist", () => {
+  it("falls back to most recently connected node when multiple non-Mac candidates exist", () => {
     const nodes: NodeListNode[] = [
-      node({ nodeId: "ios-1", platform: "ios" }),
-      node({ nodeId: "android-1", platform: "android" }),
+      node({ nodeId: "ios-1", platform: "ios", connectedAtMs: 1 }),
+      node({ nodeId: "android-1", platform: "android", connectedAtMs: 2 }),
     ];
 
-    expect(resolveNodeIdFromList(nodes, undefined, true)).toBe("ios-1");
+    expect(resolveNodeIdFromList(nodes, undefined, true)).toBe("android-1");
   });
 
   it("preserves local Mac preference when exactly one local Mac candidate exists", () => {
@@ -29,4 +41,45 @@ describe("resolveNodeIdFromList defaults", () => {
 
     expect(resolveNodeIdFromList(nodes, undefined, true)).toBe("mac-1");
   });
+
+  it("uses stable nodeId ordering when connectedAtMs is unavailable", () => {
+    const nodes: NodeListNode[] = [
+      node({ nodeId: "z-node", platform: "ios", connectedAtMs: undefined }),
+      node({ nodeId: "a-node", platform: "android", connectedAtMs: undefined }),
+    ];
+
+    expect(resolveNodeIdFromList(nodes, undefined, true)).toBe("a-node");
+  });
+});
+
+describe("listNodes", () => {
+  it("falls back to node.pair.list only when node.list is unavailable", async () => {
+    gatewayMocks.callGatewayTool
+      .mockRejectedValueOnce(new Error("unknown method: node.list"))
+      .mockResolvedValueOnce({
+        pending: [],
+        paired: [{ nodeId: "pair-1", displayName: "Pair 1", platform: "ios", remoteIp: "1.2.3.4" }],
+      });
+
+    await expect(listNodes({})).resolves.toEqual([
+      {
+        nodeId: "pair-1",
+        displayName: "Pair 1",
+        platform: "ios",
+        remoteIp: "1.2.3.4",
+      },
+    ]);
+    expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith(1, "node.list", {}, {});
+    expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith(2, "node.pair.list", {}, {});
+  });
+
+  it("rethrows unexpected node.list failures without fallback", async () => {
+    gatewayMocks.callGatewayTool.mockRejectedValueOnce(
+      new Error("gateway closed (1008): unauthorized"),
+    );
+
+    await expect(listNodes({})).rejects.toThrow("gateway closed (1008): unauthorized");
+    expect(gatewayMocks.callGatewayTool).toHaveBeenCalledTimes(1);
+    expect(gatewayMocks.callGatewayTool).toHaveBeenCalledWith("node.list", {}, {});
+  });
 });

src/agents/tools/nodes-utils.ts

@@ -5,11 +5,60 @@ import { callGatewayTool, type GatewayCallOptions } from "./gateway.js";
 
 export type { NodeListNode };
 
+type DefaultNodeFallback = "none" | "first";
+
+type DefaultNodeSelectionOptions = {
+  capability?: string;
+  fallback?: DefaultNodeFallback;
+  preferLocalMac?: boolean;
+};
+
+function messageFromError(error: unknown): string {
+  if (error instanceof Error) {
+    return error.message;
+  }
+  if (typeof error === "string") {
+    return error;
+  }
+  if (
+    typeof error === "object" &&
+    error !== null &&
+    "message" in error &&
+    typeof (error as { message?: unknown }).message === "string"
+  ) {
+    return (error as { message: string }).message;
+  }
+  if (typeof error === "object" && error !== null) {
+    try {
+      return JSON.stringify(error);
+    } catch {
+      return "";
+    }
+  }
+  return "";
+}
+
+function shouldFallbackToPairList(error: unknown): boolean {
+  const message = messageFromError(error).toLowerCase();
+  if (!message.includes("node.list")) {
+    return false;
+  }
+  return (
+    message.includes("unknown method") ||
+    message.includes("method not found") ||
+    message.includes("not implemented") ||
+    message.includes("unsupported")
+  );
+}
+
 async function loadNodes(opts: GatewayCallOptions): Promise<NodeListNode[]> {
   try {
     const res = await callGatewayTool("node.list", opts, {});
     return parseNodeList(res);
-  } catch {
+  } catch (error) {
+    if (!shouldFallbackToPairList(error)) {
+      throw error;
+    }
     const res = await callGatewayTool("node.pair.list", opts, {});
     const { paired } = parsePairingList(res);
     return paired.map((n) => ({
@@ -21,34 +70,67 @@ async function loadNodes(opts: GatewayCallOptions): Promise<NodeListNode[]> {
   }
 }
 
-function pickDefaultNode(nodes: NodeListNode[]): NodeListNode | null {
-  const withCanvas = nodes.filter((n) =>
-    Array.isArray(n.caps) ? n.caps.includes("canvas") : true,
+function isLocalMacNode(node: NodeListNode): boolean {
+  return (
+    node.platform?.toLowerCase().startsWith("mac") === true &&
+    typeof node.nodeId === "string" &&
+    node.nodeId.startsWith("mac-")
   );
-  if (withCanvas.length === 0) {
+}
+
+function compareDefaultNodeOrder(a: NodeListNode, b: NodeListNode): number {
+  const aConnectedAt = Number.isFinite(a.connectedAtMs) ? (a.connectedAtMs ?? 0) : -1;
+  const bConnectedAt = Number.isFinite(b.connectedAtMs) ? (b.connectedAtMs ?? 0) : -1;
+  if (aConnectedAt !== bConnectedAt) {
+    return bConnectedAt - aConnectedAt;
+  }
+  return a.nodeId.localeCompare(b.nodeId);
+}
+
+export function selectDefaultNodeFromList(
+  nodes: NodeListNode[],
+  options: DefaultNodeSelectionOptions = {},
+): NodeListNode | null {
+  const capability = options.capability?.trim();
+  const withCapability = capability
+    ? nodes.filter((n) => (Array.isArray(n.caps) ? n.caps.includes(capability) : true))
+    : nodes;
+  if (withCapability.length === 0) {
     return null;
   }
 
-  const connected = withCanvas.filter((n) => n.connected);
-  const candidates = connected.length > 0 ? connected : withCanvas;
+  const connected = withCapability.filter((n) => n.connected);
+  const candidates = connected.length > 0 ? connected : withCapability;
   if (candidates.length === 1) {
     return candidates[0];
   }
 
-  const local = candidates.filter(
-    (n) =>
-      n.platform?.toLowerCase().startsWith("mac") &&
-      typeof n.nodeId === "string" &&
-      n.nodeId.startsWith("mac-"),
-  );
-  if (local.length === 1) {
-    return local[0];
+  const preferLocalMac = options.preferLocalMac ?? true;
+  if (preferLocalMac) {
+    const local = candidates.filter(isLocalMacNode);
+    if (local.length === 1) {
+      return local[0];
+    }
   }
 
+  const fallback = options.fallback ?? "none";
+  if (fallback === "none") {
+    return null;
+  }
+
+  const ordered = [...candidates].toSorted(compareDefaultNodeOrder);
   // Multiple candidates — pick the first connected canvas-capable node.
   // For A2UI and other canvas operations, any node works since multi-node
   // setups broadcast surfaces across devices.
-  return candidates[0] ?? null;
+  return ordered[0] ?? null;
+}
+
+function pickDefaultNode(nodes: NodeListNode[]): NodeListNode | null {
+  return selectDefaultNodeFromList(nodes, {
+    capability: "canvas",
+    fallback: "first",
+    preferLocalMac: true,
+  });
 }
 
 export async function listNodes(opts: GatewayCallOptions): Promise<NodeListNode[]> {