diff --git a/src/agents/tools/browser-tool.ts b/src/agents/tools/browser-tool.ts index b99adb4bfff..03138c3d54e 100644 --- a/src/agents/tools/browser-tool.ts +++ b/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=.`, @@ -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; } diff --git a/src/agents/tools/nodes-utils.test.ts b/src/agents/tools/nodes-utils.test.ts index 3fef8619dd7..971ee4c91ad 100644 --- a/src/agents/tools/nodes-utils.test.ts +++ b/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 & { nodeId: string }): NodeListNode { return { @@ -11,14 +19,18 @@ function node(overrides: Partial & { 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", {}, {}); + }); }); diff --git a/src/agents/tools/nodes-utils.ts b/src/agents/tools/nodes-utils.ts index 8dbda644ef6..e4d6e4280ae 100644 --- a/src/agents/tools/nodes-utils.ts +++ b/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 { 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 { } } -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 {