From deb0ffdcdf92cf825e83d892946580a99f57c421 Mon Sep 17 00:00:00 2001 From: mushuiyu886 Date: Sat, 27 Jun 2026 07:40:29 +0800 Subject: [PATCH] fix #94040: [Bug]: nodes approve failed: GatewayClientRequestError: unknown requestId (#94452) * fix(nodes): explain unknown approval request ids * fix(nodes): keep stale request handling CI-clean * fix(nodes): point stale approve hint at pending command * fix(nodes): explain stale approval request ids * fix(nodes): make stale approval guidance reliable * fix(nodes): preserve stale approval error context --------- Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com> Co-authored-by: Peter Steinberger --- src/cli/nodes-cli.coverage.test.ts | 78 +++++++++++++++++++- src/cli/nodes-cli/cli-utils.ts | 15 +++- src/cli/nodes-cli/register.pairing.ts | 97 +++++++++++++++++++------ src/cli/nodes-cli/register.status.ts | 12 +-- src/cli/program.nodes-basic.e2e.test.ts | 2 +- 5 files changed, 168 insertions(+), 36 deletions(-) diff --git a/src/cli/nodes-cli.coverage.test.ts b/src/cli/nodes-cli.coverage.test.ts index 99df4b3b8d6..1800b727049 100644 --- a/src/cli/nodes-cli.coverage.test.ts +++ b/src/cli/nodes-cli.coverage.test.ts @@ -15,7 +15,7 @@ type NodeInvokeCall = { let lastNodeInvokeCall: NodeInvokeCall | null = null; -const callGateway = vi.fn(async (opts: NodeInvokeCall) => { +const callGateway = vi.fn(async (opts: NodeInvokeCall): Promise => { if (opts.method === "node.list") { return { nodes: [ @@ -123,6 +123,82 @@ describe("nodes-cli coverage", () => { }); }); + it("explains unknown nodes approve request ids with the current pending requests", async () => { + callGateway.mockResolvedValueOnce({ + pending: [{ requestId: "current-request", nodeId: "n1", ts: Date.now() }], + paired: [], + }); + + await expect( + sharedProgram.parseAsync( + [ + "nodes", + "approve", + "stale-request", + "--url", + "wss://gateway.example.test", + "--token", + "secret-token", + ], + { from: "user" }, + ), + ).rejects.toThrow("__exit__:1"); + + const output = runtimeErrors.join("\n"); + expect(output).toContain("Unknown node pairing requestId: stale-request"); + expect(output).toContain("Pending requestIds: current-request"); + expect(output).toContain("openclaw nodes pending"); + expect(output).toContain("Reuse the same connection options when rerunning: --url, --token."); + expect(output).not.toContain("gateway.example.test"); + expect(output).not.toContain("secret-token"); + expect(output).not.toContain("nodes approve failed: Error:"); + expect(output).not.toContain("GatewayClientRequestError: unknown requestId"); + expect(callGateway.mock.calls.map(([call]) => call.method)).toEqual(["node.pair.list"]); + }); + + it("explains when a nodes approve request disappears after the preflight", async () => { + callGateway + .mockResolvedValueOnce({ + pending: [{ requestId: "expired-request", nodeId: "n1", ts: Date.now() }], + paired: [], + }) + .mockRejectedValueOnce( + Object.assign(new Error("unknown requestId"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }), + ); + + await expect( + sharedProgram.parseAsync(["nodes", "approve", "expired-request"], { from: "user" }), + ).rejects.toThrow("__exit__:1"); + + const output = runtimeErrors.join("\n"); + expect(output).toContain("Unknown node pairing requestId: expired-request"); + expect(output).not.toContain("No pending node pairing requests are currently visible."); + expect(output).not.toContain("Pending requestIds:"); + expect(output).toContain("openclaw nodes pending"); + expect(output).not.toContain("GatewayClientRequestError: unknown requestId"); + expect(callGateway.mock.calls.map(([call]) => call.method)).toEqual([ + "node.pair.list", + "node.pair.approve", + ]); + }); + + it("still approves when the pairing preflight is unavailable", async () => { + callGateway + .mockRejectedValueOnce(new Error("pairing list unavailable")) + .mockResolvedValueOnce({ approved: true }); + + await sharedProgram.parseAsync(["nodes", "approve", "request-1"], { from: "user" }); + + expect(callGateway.mock.calls.map(([call]) => call.method)).toEqual([ + "node.pair.list", + "node.pair.approve", + ]); + expect(defaultRuntime.writeJson).toHaveBeenCalledWith({ approved: true }); + }); + it("blocks system.run on nodes invoke", async () => { await expect( sharedProgram.parseAsync(["nodes", "invoke", "--node", "mac-1", "--command", "system.run"], { diff --git a/src/cli/nodes-cli/cli-utils.ts b/src/cli/nodes-cli/cli-utils.ts index 4f510641907..3f2cd6d617c 100644 --- a/src/cli/nodes-cli/cli-utils.ts +++ b/src/cli/nodes-cli/cli-utils.ts @@ -1,8 +1,11 @@ // Node CLI runtime helpers: terminal theme adaptation and standard error handling. +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { isRich, theme } from "../../../packages/terminal-core/src/theme.js"; +import { formatErrorMessage } from "../../infra/errors.js"; import { defaultRuntime } from "../../runtime.js"; import { runCommandWithRuntime } from "../cli-utils.js"; import { unauthorizedHintForMessage } from "./rpc.js"; +import type { NodesRpcOpts } from "./types.js"; /** Return color helpers that degrade to plain text in non-rich terminals. */ export function getNodesTheme() { @@ -18,10 +21,20 @@ export function getNodesTheme() { }; } +export function formatConnectionFlagReminder(opts: NodesRpcOpts): string | null { + const flags = [ + normalizeOptionalString(opts.url) ? "--url" : null, + normalizeOptionalString(opts.token) ? "--token" : null, + ].filter((flag) => flag !== null); + return flags.length > 0 + ? `Reuse the same connection option${flags.length === 1 ? "" : "s"} when rerunning: ${flags.join(", ")}.` + : null; +} + /** Run a node CLI action with standard failure text and authorization hints. */ export function runNodesCommand(label: string, action: () => Promise) { return runCommandWithRuntime(defaultRuntime, action, (err) => { - const message = String(err); + const message = formatErrorMessage(err); const { error, warn } = getNodesTheme(); defaultRuntime.error(error(`nodes ${label} failed: ${message}`)); const hint = unauthorizedHintForMessage(message); diff --git a/src/cli/nodes-cli/register.pairing.ts b/src/cli/nodes-cli/register.pairing.ts index e5a3a19a659..472ae9e4365 100644 --- a/src/cli/nodes-cli/register.pairing.ts +++ b/src/cli/nodes-cli/register.pairing.ts @@ -6,7 +6,7 @@ import type { OperatorScope } from "../../gateway/method-scopes.js"; import { resolveNodePairApprovalScopes } from "../../infra/node-pairing-authz.js"; import { defaultRuntime } from "../../runtime.js"; import { formatCliCommand } from "../command-format.js"; -import { getNodesTheme, runNodesCommand } from "./cli-utils.js"; +import { formatConnectionFlagReminder, getNodesTheme, runNodesCommand } from "./cli-utils.js"; import { parsePairingList } from "./format.js"; import { renderPendingPairingRequestsTable } from "./pairing-render.js"; import { @@ -44,7 +44,8 @@ function normalizeNodePairApproveScopes(scopes: unknown): OperatorScope[] { async function resolveApproveScopesForRequest( opts: NodesRpcOpts, requestId: string, -): Promise { +): Promise<{ scopes: OperatorScope[] }> { + let pending: PendingRequest[]; try { const result = await callNodePairApprovalGatewayCli( "node.pair.list", @@ -52,17 +53,58 @@ async function resolveApproveScopesForRequest( {}, { scopes: DEFAULT_NODE_PAIR_APPROVE_SCOPES }, ); - const { pending } = parsePairingList(result); - const request = pending.find((candidate: PendingRequest) => candidate.requestId === requestId); - const scopes = normalizeNodePairApproveScopes(request?.requiredApproveScopes); - if (scopes.length > DEFAULT_NODE_PAIR_APPROVE_SCOPES.length) { - return scopes; - } - // Older pending requests only list requested commands; derive approval scopes from them. - return resolveNodePairApprovalScopes(request?.commands) as OperatorScope[]; + pending = parsePairingList(result).pending; } catch { - return [...DEFAULT_NODE_PAIR_APPROVE_SCOPES]; + return { scopes: [...DEFAULT_NODE_PAIR_APPROVE_SCOPES] }; } + const pendingRequestIds = pending + .map((request) => request.requestId) + .filter((id): id is string => typeof id === "string" && id.length > 0); + const request = pending.find((candidate) => candidate.requestId === requestId); + if (!request) { + throw new Error(buildUnknownNodePairRequestIdMessage(requestId, opts, pendingRequestIds)); + } + const declaredScopes = normalizeNodePairApproveScopes(request.requiredApproveScopes); + if (declaredScopes.length > DEFAULT_NODE_PAIR_APPROVE_SCOPES.length) { + return { scopes: declaredScopes }; + } + // Older pending requests only list requested commands; derive approval scopes from them. + return { + scopes: resolveNodePairApprovalScopes(request.commands) as OperatorScope[], + }; +} + +function isUnknownNodePairRequestIdError( + error: unknown, +): error is Error & { gatewayCode: "INVALID_REQUEST" } { + const requestError = error as (Error & { gatewayCode?: unknown }) | undefined; + return ( + requestError instanceof Error && + requestError.name === "GatewayClientRequestError" && + requestError.gatewayCode === "INVALID_REQUEST" && + requestError.message === "unknown requestId" + ); +} + +function buildUnknownNodePairRequestIdMessage( + requestId: string, + opts: NodesRpcOpts, + pendingRequestIds?: string[], +): string { + const lines = [`Unknown node pairing requestId: ${requestId}`]; + if (pendingRequestIds !== undefined) { + if (pendingRequestIds.length > 0) { + lines.push(`Pending requestIds: ${pendingRequestIds.join(", ")}`); + } else { + lines.push("No pending node pairing requests are currently visible."); + } + } + lines.push(`Run ${formatCliCommand("openclaw nodes pending")} to inspect current requests.`); + const connectionReminder = formatConnectionFlagReminder(opts); + if (connectionReminder) { + lines.push(connectionReminder); + } + return lines.join("\n"); } /** Register node pairing management commands. */ @@ -106,17 +148,28 @@ export function registerNodesPairingCommands(nodes: Command) { .argument("", "Pending request id") .action(async (requestId: string, opts: NodesRpcOpts) => { await runNodesCommand("approve", async () => { - const scopes = await resolveApproveScopesForRequest(opts, requestId); - const result = await callNodePairApprovalGatewayCli( - "node.pair.approve", - opts, - { - requestId, - }, - { - scopes, - }, - ); + const { scopes } = await resolveApproveScopesForRequest(opts, requestId); + let result: unknown; + try { + result = await callNodePairApprovalGatewayCli( + "node.pair.approve", + opts, + { + requestId, + }, + { + scopes, + }, + ); + } catch (error) { + if (!isUnknownNodePairRequestIdError(error)) { + throw error; + } + // Reuse the gateway error so generic formatting does not append its raw cause. + error.name = "Error"; + error.message = buildUnknownNodePairRequestIdMessage(requestId, opts); + throw error; + } defaultRuntime.writeJson(result); }); }), diff --git a/src/cli/nodes-cli/register.status.ts b/src/cli/nodes-cli/register.status.ts index 1c7b0ff53f3..e895c9ac2ac 100644 --- a/src/cli/nodes-cli/register.status.ts +++ b/src/cli/nodes-cli/register.status.ts @@ -14,7 +14,7 @@ import { shortenHomeInString } from "../../utils.js"; import { formatCliCommand } from "../command-format.js"; import { parseDurationMs } from "../parse-duration.js"; import { quoteCliArg } from "../quote-cli-arg.js"; -import { getNodesTheme, runNodesCommand } from "./cli-utils.js"; +import { formatConnectionFlagReminder, getNodesTheme, runNodesCommand } from "./cli-utils.js"; import { formatPermissions, parseNodeList, parsePairingList } from "./format.js"; import { renderPendingPairingRequestsTable } from "./pairing-render.js"; import { @@ -146,16 +146,6 @@ function formatPendingApprovalCommand(raw: unknown, opts: NodesRpcOpts): string return formatCliCommand(args.map(quoteCliArg).join(" ")); } -function formatConnectionFlagReminder(opts: NodesRpcOpts): string | null { - const flags = [ - normalizeOptionalString(opts.url) ? "--url" : null, - normalizeOptionalString(opts.token) ? "--token" : null, - ].filter((flag) => flag !== null); - return flags.length > 0 - ? `Reuse the same ${flags.join("/")} option${flags.length === 1 ? "" : "s"} when rerunning.` - : null; -} - function parseSinceMs(raw: unknown, label: string): number | undefined { if (raw === undefined || raw === null) { return undefined; diff --git a/src/cli/program.nodes-basic.e2e.test.ts b/src/cli/program.nodes-basic.e2e.test.ts index 42ef52f772e..b849a17311e 100644 --- a/src/cli/program.nodes-basic.e2e.test.ts +++ b/src/cli/program.nodes-basic.e2e.test.ts @@ -562,7 +562,7 @@ describe("cli program (nodes basics)", () => { const output = getRuntimeOutput(); expect(output).toContain("openclaw nodes approve request-reapproval --timeout 3000"); - expect(output).toContain("Reuse the same --url/--token options when rerunning."); + expect(output).toContain("Reuse the same connection options when rerunning: --url, --token."); expect(output).not.toContain("gateway-user"); expect(output).not.toContain("url-secret"); expect(output).not.toContain("gateway.example");