diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index 1ae130ecb42b..d08e1543e4c9 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -2138,7 +2138,6 @@ src/agents/tools/mobile-ui-tool.ts 5 src/agents/tools/model-config.helpers.ts 1 src/agents/tools/music-generate-tool.ts 2 src/agents/tools/nodes-tool.ts 3 -src/agents/tools/nodes-utils.ts 2 src/agents/tools/openclaw-delegate-tool.ts 1 src/agents/tools/pdf-native-providers.ts 2 src/agents/tools/pdf-tool.ts 3 @@ -2445,7 +2444,7 @@ src/cli/nodes-cli/register.pairing.ts 4 src/cli/nodes-cli/register.push.ts 1 src/cli/nodes-cli/register.screen.ts 1 src/cli/nodes-cli/register.status.ts 4 -src/cli/nodes-cli/rpc.ts 3 +src/cli/nodes-cli/rpc.ts 2 src/cli/one-shot-exit.ts 2 src/cli/output-file.runtime.ts 1 src/cli/pairing-cli.ts 1 diff --git a/extensions/canvas/src/cli.test.ts b/extensions/canvas/src/cli.test.ts index 62b84bcbd1a6..500ab6c89117 100644 --- a/extensions/canvas/src/cli.test.ts +++ b/extensions/canvas/src/cli.test.ts @@ -1,5 +1,19 @@ +import { + GatewayClientRequestError, + GatewayClientRequestTimeoutError, +} from "@openclaw/gateway-client"; import { Command } from "commander"; import { beforeEach, describe, expect, it, vi } from "vitest"; + +const gatewayMocks = vi.hoisted(() => ({ + callGatewayFromCli: vi.fn(), +})); + +vi.mock("openclaw/plugin-sdk/gateway-runtime", async (importOriginal) => ({ + ...(await importOriginal()), + callGatewayFromCli: gatewayMocks.callGatewayFromCli, +})); + import { createDefaultCanvasCliDependencies, registerNodesCanvasCommands, @@ -35,6 +49,7 @@ function createProgram(deps: CanvasCliDependencies) { describe("nodes canvas CLI", () => { beforeEach(() => { vi.clearAllMocks(); + gatewayMocks.callGatewayFromCli.mockReset(); }); it("registers only presenter commands", () => { @@ -180,4 +195,123 @@ describe("nodes canvas CLI", () => { ).rejects.toThrow(message); expect(deps.callGatewayCli).not.toHaveBeenCalled(); }); + + it("resolves and invokes a paired node when an older Gateway lacks node.list", async () => { + const { deps } = createDeps(); + deps.resolveNodeId = createDefaultCanvasCliDependencies().resolveNodeId; + gatewayMocks.callGatewayFromCli + .mockRejectedValueOnce( + new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list", + }), + ) + .mockResolvedValueOnce({ + pending: [], + paired: [{ nodeId: "legacy-node", displayName: "Legacy Node" }], + }); + + await createProgram(deps).parseAsync(["nodes", "canvas", "hide", "--node", "Legacy Node"], { + from: "user", + }); + + expect(gatewayMocks.callGatewayFromCli.mock.calls.map(([method]) => method)).toEqual([ + "node.list", + "node.pair.list", + ]); + expect(deps.callGatewayCli).toHaveBeenCalledWith( + "node.invoke", + expect.any(Object), + expect.objectContaining({ nodeId: "legacy-node", command: "canvas.hide" }), + expect.any(Object), + ); + }); + + it.each([ + { + label: "a local request timeout", + error: new GatewayClientRequestTimeoutError({ + method: "node.list", + timeoutMs: 80, + requestSent: true, + }), + }, + { + label: "an authorization rejection", + error: new GatewayClientRequestError({ + code: "FORBIDDEN", + message: "unknown method: node.list", + }), + }, + { + label: "an INVALID_REQUEST authentication failure", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unauthorized", + }), + }, + { + label: "a retryable unknown-method rejection", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list", + retryable: true, + }), + }, + { + label: "an unknown-method rejection for another method", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list.extra", + }), + }, + { + label: "malformed request retry metadata", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list", + retryAfterMs: -1, + }), + }, + { + label: "a network connection error", + error: Object.assign(new Error("connect ECONNREFUSED 127.0.0.1:18789"), { + code: "ECONNREFUSED", + }), + }, + { + label: "a closed Gateway transport", + error: new Error("gateway closed (1006): connection lost"), + }, + { + label: "a malformed request-error lookalike", + error: Object.assign(new Error("unknown method: node.list"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }), + }, + { + label: "a plain unknown-method error", + error: new Error("unknown method: node.list"), + }, + ])("preserves $label without resolving or invoking a stale node", async ({ error }) => { + const { deps, runtime } = createDeps(); + deps.resolveNodeId = createDefaultCanvasCliDependencies().resolveNodeId; + gatewayMocks.callGatewayFromCli.mockRejectedValueOnce(error).mockResolvedValueOnce({ + pending: [], + paired: [{ nodeId: "stale-node", displayName: "Stale Node" }], + }); + + await expect( + createProgram(deps).parseAsync(["nodes", "canvas", "hide", "--node", "Stale Node"], { + from: "user", + }), + ).rejects.toBe(error); + + expect(gatewayMocks.callGatewayFromCli.mock.calls.map(([method]) => method)).toEqual([ + "node.list", + ]); + expect(deps.callGatewayCli).not.toHaveBeenCalled(); + expect(runtime.log).not.toHaveBeenCalled(); + }); }); diff --git a/extensions/canvas/src/cli.ts b/extensions/canvas/src/cli.ts index 4f69c2a8b40d..111aecb4d7d8 100644 --- a/extensions/canvas/src/cli.ts +++ b/extensions/canvas/src/cli.ts @@ -7,6 +7,7 @@ import { runCommandWithRuntime, theme } from "openclaw/plugin-sdk/cli-runtime"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; import { callGatewayFromCli, + isGatewayClientRequestError, resolveNodeFromNodeList, type NodeMatchCandidate, } from "openclaw/plugin-sdk/gateway-runtime"; @@ -192,7 +193,15 @@ export function createDefaultCanvasCliDependencies(): CanvasCliDependencies { let raw: unknown; try { raw = await callGatewayCli("node.list", opts, {}); - } catch { + } catch (error) { + if ( + !isGatewayClientRequestError(error) || + error.gatewayCode !== "INVALID_REQUEST" || + error.retryable || + error.message !== "unknown method: node.list" + ) { + throw error; + } raw = await callGatewayCli("node.pair.list", opts, {}); } return resolveNodeFromNodeList(parseNodeCandidates(raw), query).nodeId; diff --git a/src/agents/tools/nodes-utils.test.ts b/src/agents/tools/nodes-utils.test.ts index 68d15559f420..1cbba008bae1 100644 --- a/src/agents/tools/nodes-utils.test.ts +++ b/src/agents/tools/nodes-utils.test.ts @@ -1,6 +1,8 @@ // Node utility tests cover node selection defaults and gateway fallback between // current and legacy node list methods. import { beforeEach, describe, expect, it, vi } from "vitest"; +import { GatewayProtocolRequestTimeoutError } from "../../../packages/gateway-client/src/protocol-request.js"; +import { GatewayClientRequestError } from "../../../packages/gateway-client/src/request-error.js"; const gatewayMocks = vi.hoisted(() => ({ callGatewayTool: vi.fn(), @@ -145,7 +147,12 @@ describe("listNodes", () => { // Old gateways only expose node.pair.list; newer authorization failures // must still surface instead of being hidden by fallback. gatewayMocks.callGatewayTool - .mockRejectedValueOnce(new Error("unknown method: node.list")) + .mockRejectedValueOnce( + new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list", + }), + ) .mockResolvedValueOnce({ pending: [], paired: [{ nodeId: "pair-1", displayName: "Pair 1", platform: "ios", remoteIp: "1.2.3.4" }], @@ -178,13 +185,88 @@ describe("listNodes", () => { ); }); - it("rethrows unexpected node.list failures without fallback", async () => { - gatewayMocks.callGatewayTool.mockRejectedValueOnce( - new Error("gateway closed (1008): unauthorized"), - ); + it.each([ + { + label: "a local request timeout", + error: new GatewayProtocolRequestTimeoutError({ + method: "node.list", + timeoutMs: 80, + requestSent: true, + }), + }, + { + label: "an authorization rejection", + error: new GatewayClientRequestError({ + code: "FORBIDDEN", + message: "unknown method: node.list", + }), + }, + { + label: "an INVALID_REQUEST authentication failure", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unauthorized", + }), + }, + { + label: "a retryable unknown-method rejection", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list", + retryable: true, + }), + }, + { + label: "an unknown-method rejection for another method", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list.extra", + }), + }, + { + label: "malformed request retry metadata", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list", + retryAfterMs: -1, + }), + }, + { + label: "an unsupported-method prose error", + error: new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "node.list is not implemented", + }), + }, + { + label: "a network connection error", + error: Object.assign(new Error("connect ECONNREFUSED 127.0.0.1:18789"), { + code: "ECONNREFUSED", + }), + }, + { + label: "a closed Gateway transport", + error: new Error("gateway closed (1008): unauthorized"), + }, + { + label: "a malformed request-error lookalike", + error: Object.assign(new Error("unknown method: node.list"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }), + }, + { + label: "a plain unknown-method error", + error: new Error("unknown method: node.list"), + }, + ])("rethrows $label without consulting paired nodes", async ({ error }) => { + gatewayMocks.callGatewayTool.mockRejectedValueOnce(error).mockResolvedValueOnce({ + pending: [], + paired: [{ nodeId: "stale-node", displayName: "Stale Node" }], + }); const signal = new AbortController().signal; - await expect(listNodes({}, signal)).rejects.toThrow("gateway closed (1008): unauthorized"); + await expect(listNodes({}, signal)).rejects.toBe(error); expect(gatewayMocks.callGatewayTool).toHaveBeenCalledTimes(1); expect(gatewayMocks.callGatewayTool).toHaveBeenCalledWith("node.list", {}, {}, { signal }); }); diff --git a/src/agents/tools/nodes-utils.ts b/src/agents/tools/nodes-utils.ts index 4c0fb94e65ef..f3169afa7dee 100644 --- a/src/agents/tools/nodes-utils.ts +++ b/src/agents/tools/nodes-utils.ts @@ -4,6 +4,7 @@ * Loads paired nodes from Gateway and resolves requested/default nodes with legacy pair-list fallback. */ import { normalizeOptionalLowercaseString } from "@openclaw/normalization-core/string-coerce"; +import { GatewayClientRequestError } from "../../../packages/gateway-client/src/request-error.js"; import { parseNodeList, parsePairingList } from "../../shared/node-list-parse.js"; import type { NodeListNode } from "../../shared/node-list-types.js"; import { resolveNodeFromNodeList, resolveNodeIdFromNodeList } from "../../shared/node-resolve.js"; @@ -19,50 +20,19 @@ type DefaultNodeSelectionOptions = { 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 = normalizeOptionalLowercaseString(messageFromError(error)) ?? ""; - 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, signal?: AbortSignal): Promise { try { const res = await callGatewayTool("node.list", opts, {}, { signal }); return parseNodeList(res); } catch (error) { - if (!shouldFallbackToPairList(error)) { + if ( + !(error instanceof GatewayClientRequestError) || + error.gatewayCode !== "INVALID_REQUEST" || + error.retryable || + error.message !== "unknown method: node.list" || + (error.retryAfterMs !== undefined && + (!Number.isInteger(error.retryAfterMs) || error.retryAfterMs < 0)) + ) { throw error; } // Older gateways only expose paired-node state; preserve node tools until node.list exists. diff --git a/src/cli/nodes-cli/rpc.test.ts b/src/cli/nodes-cli/rpc.test.ts new file mode 100644 index 000000000000..4734432cf31e --- /dev/null +++ b/src/cli/nodes-cli/rpc.test.ts @@ -0,0 +1,133 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { GatewayProtocolRequestTimeoutError } from "../../../packages/gateway-client/src/protocol-request.js"; +import { GatewayClientRequestError } from "../../../packages/gateway-client/src/request-error.js"; + +const gatewayMocks = vi.hoisted(() => ({ + callGatewayFromCliWithTransport: vi.fn(), +})); + +vi.mock("../gateway-rpc.js", () => ({ + callGatewayFromCliWithTransport: gatewayMocks.callGatewayFromCliWithTransport, +})); + +import { resolveCliNode, resolveNodeDiagnosticsId } from "./rpc.js"; + +function requestError(params: { + code?: string; + message?: string; + retryable?: boolean; + retryAfterMs?: number; +}) { + return new GatewayClientRequestError({ + code: params.code ?? "INVALID_REQUEST", + message: params.message ?? "unknown method: node.list", + ...params, + }); +} + +describe("node inventory resolution", () => { + beforeEach(() => { + gatewayMocks.callGatewayFromCliWithTransport.mockReset(); + }); + + it("uses paired records when an older Gateway rejects the exact node.list method", async () => { + gatewayMocks.callGatewayFromCliWithTransport + .mockRejectedValueOnce(requestError({})) + .mockResolvedValueOnce({ + pending: [], + paired: [{ nodeId: "legacy-node", displayName: "Legacy Node", platform: "ios" }], + }); + + await expect(resolveCliNode({}, "Legacy Node")).resolves.toMatchObject({ + nodeId: "legacy-node", + displayName: "Legacy Node", + }); + expect( + gatewayMocks.callGatewayFromCliWithTransport.mock.calls.map(([method]) => method), + ).toEqual(["node.list", "node.pair.list"]); + }); + + it.each([ + { + label: "a local request timeout", + error: new GatewayProtocolRequestTimeoutError({ + method: "node.list", + timeoutMs: 80, + requestSent: true, + }), + }, + { + label: "an authorization rejection", + error: requestError({ code: "UNAUTHORIZED", message: "operator authorization required" }), + }, + { + label: "an INVALID_REQUEST authentication failure", + error: requestError({ message: "invalid auth token" }), + }, + { + label: "a retryable unknown-method rejection", + error: requestError({ retryable: true }), + }, + { + label: "an unknown-method rejection for another method", + error: requestError({ message: "unknown method: node.list.extra" }), + }, + { + label: "malformed request retry metadata", + error: requestError({ retryAfterMs: -1 }), + }, + { + label: "an embedded unknown-method message", + error: requestError({ message: "request failed: unknown method: node.list" }), + }, + { + label: "a network connection error", + error: Object.assign(new Error("connect ECONNREFUSED 127.0.0.1:18789"), { + code: "ECONNREFUSED", + }), + }, + { + label: "a closed Gateway transport", + error: new Error("gateway closed (1006): connection lost"), + }, + { + label: "a malformed request-error lookalike", + error: Object.assign(new Error("unknown method: node.list"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }), + }, + { + label: "a plain unknown-method error", + error: new Error("unknown method: node.list"), + }, + ])("preserves $label without consulting stale paired nodes", async ({ error }) => { + gatewayMocks.callGatewayFromCliWithTransport + .mockRejectedValueOnce(error) + .mockResolvedValueOnce({ + pending: [], + paired: [{ nodeId: "stale-node", displayName: "Stale Node" }], + }); + + await expect(resolveCliNode({}, "Stale Node")).rejects.toBe(error); + expect( + gatewayMocks.callGatewayFromCliWithTransport.mock.calls.map(([method]) => method), + ).toEqual(["node.list"]); + }); + + it.each([ + requestError({ retryable: true }), + requestError({ message: "unknown method: node.list.extra" }), + Object.assign(new Error("unknown method: node.list"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }), + ])("keeps diagnostics on the same exact missing-method contract", async (error) => { + gatewayMocks.callGatewayFromCliWithTransport.mockRejectedValueOnce(error); + + await expect(resolveNodeDiagnosticsId({}, "stale-node")).rejects.toBe(error); + expect( + gatewayMocks.callGatewayFromCliWithTransport.mock.calls.map(([method]) => method), + ).toEqual(["node.list"]); + }); +}); diff --git a/src/cli/nodes-cli/rpc.ts b/src/cli/nodes-cli/rpc.ts index 26e2741ab8ff..fb0613643a11 100644 --- a/src/cli/nodes-cli/rpc.ts +++ b/src/cli/nodes-cli/rpc.ts @@ -7,6 +7,7 @@ import { } from "@openclaw/normalization-core/number-coercion"; import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import type { Command } from "commander"; +import { GatewayClientRequestError } from "../../../packages/gateway-client/src/request-error.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES, @@ -73,12 +74,17 @@ function isDiagnosticsAuthFallbackError(value: unknown): value is Error { return readMissingScopeError(value)?.missingScope === "operator.read"; } -function isUnknownGatewayMethodError(value: unknown, method: string): value is Error { +function isUnknownGatewayMethodError( + value: unknown, + method: string, +): value is GatewayClientRequestError { return ( - value instanceof Error && - value.name === "GatewayClientRequestError" && - (value as Error & { gatewayCode?: unknown }).gatewayCode === "INVALID_REQUEST" && - value.message.includes(`unknown method: ${method}`) + value instanceof GatewayClientRequestError && + value.gatewayCode === "INVALID_REQUEST" && + !value.retryable && + value.message === `unknown method: ${method}` && + (value.retryAfterMs === undefined || + (Number.isInteger(value.retryAfterMs) && value.retryAfterMs >= 0)) ); } @@ -296,7 +302,10 @@ export async function resolveCliNode(opts: NodesRpcOpts, query: string): Promise try { const res = await callNodesGatewayCli("node.list", opts, {}); nodes = parseNodeList(res); - } catch { + } catch (error) { + if (!isUnknownGatewayMethodError(error, "node.list")) { + throw error; + } const res = await callNodesGatewayCli("node.pair.list", opts, {}); const { paired } = parsePairingList(res); nodes = paired.map((n) => ({ diff --git a/src/cli/program.nodes-basic.e2e.test.ts b/src/cli/program.nodes-basic.e2e.test.ts index 6f8a8b7d6ea1..6fffc3d41b8b 100644 --- a/src/cli/program.nodes-basic.e2e.test.ts +++ b/src/cli/program.nodes-basic.e2e.test.ts @@ -1,6 +1,8 @@ // Program nodes basic e2e tests cover node command registration through the full CLI program. import { Command } from "commander"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { GatewayProtocolRequestTimeoutError } from "../../packages/gateway-client/src/protocol-request.js"; +import { GatewayClientRequestError } from "../../packages/gateway-client/src/request-error.js"; import { createIosNodeListResponse, formatRuntimeLogCallArg, @@ -687,9 +689,9 @@ describe("cli program (nodes basics)", () => { params?: { nodeId?: string }; }; if (opts.method === "node.list") { - throw Object.assign(new Error("unknown method: node.list"), { - name: "GatewayClientRequestError", - gatewayCode: "INVALID_REQUEST", + throw new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "unknown method: node.list", }); } if (opts.method === "node.pair.list") { @@ -877,4 +879,33 @@ describe("cli program (nodes basics)", () => { expect(invokeRequest?.clientName).toBe("cli"); expect(invokeRequest?.mode).toBe("cli"); }); + + it("reports the inventory timeout instead of invoking a stale paired node", async () => { + const timeout = new GatewayProtocolRequestTimeoutError({ + method: "node.list", + timeoutMs: 80, + requestSent: true, + }); + programGatewayCallMock.mockImplementation(async (...args: unknown[]) => { + const { method } = (args[0] ?? {}) as { method?: string }; + if (method === "node.list") { + throw timeout; + } + if (method === "node.pair.list") { + return { pending: [], paired: [{ nodeId: "stale-node", displayName: "Stale Node" }] }; + } + throw new GatewayClientRequestError({ + code: "UNAVAILABLE", + message: "node not connected", + }); + }); + + await expect( + runProgram(["nodes", "invoke", "--node", "Stale Node", "--command", "canvas.hide"]), + ).rejects.toThrow("exit"); + + expect(runtime.error).toHaveBeenCalledWith(expect.stringContaining(timeout.message)); + expect(gatewayRequests().map(({ method }) => method)).toEqual(["node.list"]); + expect(runtime.writeJson).not.toHaveBeenCalled(); + }); });