mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-30 03:33:34 +00:00
* 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 <steipete@gmail.com>
This commit is contained in:
@@ -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<unknown> => {
|
||||
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"], {
|
||||
|
||||
@@ -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<void>) {
|
||||
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);
|
||||
|
||||
@@ -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<OperatorScope[]> {
|
||||
): 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("<requestId>", "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);
|
||||
});
|
||||
}),
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user