test(nodes): update coverage after exec consolidation

This commit is contained in:
Peter Steinberger
2026-03-30 00:40:18 +01:00
parent 5dae663ea4
commit 2255e04b07
5 changed files with 44 additions and 381 deletions

View File

@@ -17,7 +17,6 @@ vi.mock("../media/image-ops.js", () => ({
let createOpenClawTools: typeof import("./openclaw-tools.js").createOpenClawTools;
const NODE_ID = "mac-1";
const BASE_RUN_INPUT = { action: "run", node: NODE_ID, command: ["echo", "hi"] } as const;
const JPG_PAYLOAD = {
format: "jpg",
base64: "aGVsbG8=",
@@ -138,43 +137,6 @@ function setupNodeInvokeMock(params: {
});
}
function createSystemRunPreparePayload(cwd: string | null) {
return {
payload: {
plan: {
argv: ["echo", "hi"],
cwd,
commandText: "echo hi",
agentId: null,
sessionKey: null,
},
},
};
}
function setupSystemRunGateway(params: {
onRunInvoke: (invokeParams: unknown) => GatewayMockResult | Promise<GatewayMockResult>;
onApprovalRequest?: (approvalParams: unknown) => GatewayMockResult | Promise<GatewayMockResult>;
prepareCwd?: string | null;
}) {
callGateway.mockImplementation(async ({ method, params: gatewayParams }: GatewayCall) => {
if (method === "node.list") {
return mockNodeList({ commands: ["system.run"] });
}
if (method === "node.invoke") {
const command = (gatewayParams as { command?: string } | undefined)?.command;
if (command === "system.run.prepare") {
return createSystemRunPreparePayload(params.prepareCwd ?? null);
}
return await params.onRunInvoke(gatewayParams);
}
if (method === "exec.approval.request" && params.onApprovalRequest) {
return await params.onApprovalRequest(gatewayParams);
}
return unexpectedGatewayMethod(method);
});
}
function setupPhotosLatestMock(params?: { remoteIp?: string }) {
setupNodeInvokeMock({
...(params?.remoteIp ? { remoteIp: params.remoteIp } : {}),
@@ -604,119 +566,6 @@ describe("nodes device_status and device_info", () => {
});
});
describe("nodes run", () => {
it("passes invoke and command timeouts", async () => {
setupSystemRunGateway({
prepareCwd: "/tmp",
onRunInvoke: (invokeParams) => {
expect(invokeParams).toMatchObject({
nodeId: NODE_ID,
command: "system.run",
timeoutMs: 45_000,
params: {
command: ["echo", "hi"],
cwd: "/tmp",
env: { FOO: "bar" },
timeoutMs: 12_000,
},
});
return {
payload: { stdout: "", stderr: "", exitCode: 0, success: true },
};
},
});
await executeNodes({
...BASE_RUN_INPUT,
cwd: "/tmp",
env: ["FOO=bar"],
commandTimeoutMs: 12_000,
invokeTimeoutMs: 45_000,
});
});
it("requests approval and retries with allow-once decision", async () => {
let invokeCalls = 0;
let approvalId: string | null = null;
setupSystemRunGateway({
onRunInvoke: (invokeParams) => {
invokeCalls += 1;
if (invokeCalls === 1) {
throw new Error("SYSTEM_RUN_DENIED: approval required");
}
expect(invokeParams).toMatchObject({
nodeId: NODE_ID,
command: "system.run",
params: {
command: ["echo", "hi"],
runId: approvalId,
approved: true,
approvalDecision: "allow-once",
},
});
return { payload: { stdout: "", stderr: "", exitCode: 0, success: true } };
},
onApprovalRequest: (approvalParams) => {
expect(approvalParams).toMatchObject({
id: expect.any(String),
systemRunPlan: expect.objectContaining({
argv: ["echo", "hi"],
commandText: "echo hi",
}),
nodeId: NODE_ID,
host: "node",
timeoutMs: 120_000,
});
approvalId =
typeof (approvalParams as { id?: unknown } | undefined)?.id === "string"
? ((approvalParams as { id: string }).id ?? null)
: null;
return { decision: "allow-once" };
},
});
await executeNodes(BASE_RUN_INPUT);
expect(invokeCalls).toBe(2);
});
it("fails with user denied when approval decision is deny", async () => {
setupSystemRunGateway({
onRunInvoke: () => {
throw new Error("SYSTEM_RUN_DENIED: approval required");
},
onApprovalRequest: () => {
return { decision: "deny" };
},
});
await expect(executeNodes(BASE_RUN_INPUT)).rejects.toThrow("exec denied: user denied");
});
it("fails closed for timeout and invalid approval decisions", async () => {
setupSystemRunGateway({
onRunInvoke: () => {
throw new Error("SYSTEM_RUN_DENIED: approval required");
},
onApprovalRequest: () => {
return {};
},
});
await expect(executeNodes(BASE_RUN_INPUT)).rejects.toThrow("exec denied: approval timed out");
setupSystemRunGateway({
onRunInvoke: () => {
throw new Error("SYSTEM_RUN_DENIED: approval required");
},
onApprovalRequest: () => {
return { decision: "allow-never" };
},
});
await expect(executeNodes(BASE_RUN_INPUT)).rejects.toThrow(
"exec denied: invalid approval decision",
);
});
});
describe("nodes invoke", () => {
it("allows metadata-only camera.list via generic invoke", async () => {
setupNodeInvokeMock({

View File

@@ -8,8 +8,6 @@ const gatewayMocks = vi.hoisted(() => ({
const nodeUtilsMocks = vi.hoisted(() => ({
resolveNodeId: vi.fn(async () => "node-1"),
resolveNode: vi.fn(async () => ({ nodeId: "node-1", remoteIp: "127.0.0.1" })),
listNodes: vi.fn(async () => [] as Array<{ nodeId: string; commands?: string[] }>),
resolveNodeIdFromList: vi.fn(() => "node-1"),
}));
const nodesCameraMocks = vi.hoisted(() => ({
@@ -48,8 +46,6 @@ vi.mock("./gateway.js", () => ({
vi.mock("./nodes-utils.js", () => ({
resolveNodeId: nodeUtilsMocks.resolveNodeId,
resolveNode: nodeUtilsMocks.resolveNode,
listNodes: nodeUtilsMocks.listNodes,
resolveNodeIdFromList: nodeUtilsMocks.resolveNodeIdFromList,
}));
vi.mock("../../cli/nodes-camera.js", () => ({
@@ -77,8 +73,6 @@ async function loadFreshNodesToolModuleForTest() {
vi.doMock("./nodes-utils.js", () => ({
resolveNodeId: nodeUtilsMocks.resolveNodeId,
resolveNode: nodeUtilsMocks.resolveNode,
listNodes: nodeUtilsMocks.listNodes,
resolveNodeIdFromList: nodeUtilsMocks.resolveNodeIdFromList,
}));
vi.doMock("../../cli/nodes-camera.js", () => ({
cameraTempPath: nodesCameraMocks.cameraTempPath,
@@ -148,50 +142,15 @@ describe("createNodesTool screen_record duration guardrails", () => {
);
});
it("omits rawCommand when preparing wrapped argv execution", async () => {
nodeUtilsMocks.listNodes.mockResolvedValue([
{
nodeId: "node-1",
commands: ["system.run"],
},
]);
gatewayMocks.callGatewayTool.mockImplementation(async (_method, _opts, payload) => {
if (payload?.command === "system.run.prepare") {
return {
payload: {
plan: {
argv: ["bash", "-lc", "echo hi"],
cwd: null,
commandText: 'bash -lc "echo hi"',
commandPreview: "echo hi",
agentId: null,
sessionKey: null,
},
},
};
}
if (payload?.command === "system.run") {
return { payload: { ok: true } };
}
throw new Error(`unexpected command: ${String(payload?.command)}`);
});
it("rejects the removed run action", async () => {
const tool = createNodesTool();
await tool.execute("call-1", {
action: "run",
node: "macbook",
command: ["bash", "-lc", "echo hi"],
});
const prepareCall = gatewayMocks.callGatewayTool.mock.calls.find(
(call) => call[2]?.command === "system.run.prepare",
)?.[2];
expect(prepareCall).toBeTruthy();
expect(prepareCall?.params).toMatchObject({
command: ["bash", "-lc", "echo hi"],
agentId: "main",
});
expect(prepareCall?.params).not.toHaveProperty("rawCommand");
await expect(
tool.execute("call-1", {
action: "run",
node: "macbook",
}),
).rejects.toThrow("Unknown action: run");
});
it("returns camera snaps via details.media.mediaUrls", async () => {
gatewayMocks.callGatewayTool.mockResolvedValue({ payload: { ok: true } });
@@ -385,4 +344,16 @@ describe("createNodesTool screen_record duration guardrails", () => {
{ scopes: ["operator.write"] },
);
});
it("blocks invokeCommand system.run so exec stays the only shell path", async () => {
const tool = createNodesTool();
await expect(
tool.execute("call-1", {
action: "invoke",
node: "macbook",
invokeCommand: "system.run",
}),
).rejects.toThrow('invokeCommand "system.run" is reserved for shell execution');
});
});

View File

@@ -125,7 +125,7 @@ describe("argv helpers", () => {
},
{
name: "help after -- terminator",
argv: ["node", "openclaw", "nodes", "run", "--", "git", "--help"],
argv: ["node", "openclaw", "nodes", "invoke", "--", "device.status", "--help"],
expected: false,
},
{

View File

@@ -1,7 +1,5 @@
import { Command } from "commander";
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import type { ExecApprovalsFile } from "../infra/exec-approvals.js";
import { buildSystemRunPreparePayload } from "../test-utils/system-run-prepare-payload.js";
import { createCliRuntimeCapture } from "./test-runtime-capture.js";
type NodeInvokeCall = {
@@ -15,17 +13,6 @@ type NodeInvokeCall = {
};
let lastNodeInvokeCall: NodeInvokeCall | null = null;
let lastApprovalRequestCall: { params?: Record<string, unknown> } | null = null;
let localExecApprovalsFile: ExecApprovalsFile = { version: 1, agents: {} };
let nodeExecApprovalsFile: ExecApprovalsFile = {
version: 1,
defaults: {
security: "allowlist",
ask: "on-miss",
askFallback: "deny",
},
agents: {},
};
const callGateway = vi.fn(async (opts: NodeInvokeCall) => {
if (opts.method === "node.list") {
@@ -44,16 +31,6 @@ const callGateway = vi.fn(async (opts: NodeInvokeCall) => {
}
if (opts.method === "node.invoke") {
lastNodeInvokeCall = opts;
const command = opts.params?.command;
if (command === "system.run.prepare") {
const params = (opts.params?.params ?? {}) as {
command?: unknown[];
rawCommand?: unknown;
cwd?: unknown;
agentId?: unknown;
};
return buildSystemRunPreparePayload(params);
}
return {
payload: {
stdout: "",
@@ -64,24 +41,12 @@ const callGateway = vi.fn(async (opts: NodeInvokeCall) => {
},
};
}
if (opts.method === "exec.approvals.node.get") {
return {
path: "/tmp/exec-approvals.json",
exists: true,
hash: "hash",
file: nodeExecApprovalsFile,
};
}
if (opts.method === "exec.approval.request") {
lastApprovalRequestCall = opts as { params?: Record<string, unknown> };
return { decision: "allow-once" };
}
return { ok: true };
});
const randomIdempotencyKey = vi.fn(() => "rk_test");
const { defaultRuntime, resetRuntimeCapture } = createCliRuntimeCapture();
const { runtimeErrors, defaultRuntime, resetRuntimeCapture } = createCliRuntimeCapture();
vi.mock("../gateway/call.js", () => ({
callGateway: (opts: unknown) => callGateway(opts as NodeInvokeCall),
@@ -93,20 +58,6 @@ vi.mock("../runtime.js", async (importOriginal) => ({
defaultRuntime,
}));
vi.mock("../config/config.js", () => ({
loadConfig: () => ({}),
}));
vi.mock("../infra/exec-approvals.js", async () => {
const actual = await vi.importActual<typeof import("../infra/exec-approvals.js")>(
"../infra/exec-approvals.js",
);
return {
...actual,
loadExecApprovals: () => localExecApprovalsFile,
};
});
describe("nodes-cli coverage", () => {
let registerNodesCli: (program: Command) => void;
let sharedProgram: Command;
@@ -119,8 +70,6 @@ describe("nodes-cli coverage", () => {
return last;
};
const getApprovalRequestCall = () => lastApprovalRequestCall;
const runNodesCommand = async (args: string[]) => {
await sharedProgram.parseAsync(args, { from: "user" });
return getNodeInvokeCall();
@@ -138,128 +87,23 @@ describe("nodes-cli coverage", () => {
callGateway.mockClear();
randomIdempotencyKey.mockClear();
lastNodeInvokeCall = null;
lastApprovalRequestCall = null;
localExecApprovalsFile = { version: 1, agents: {} };
nodeExecApprovalsFile = {
version: 1,
defaults: {
security: "allowlist",
ask: "on-miss",
askFallback: "deny",
},
agents: {},
};
});
it("invokes system.run with parsed params", async () => {
const invoke = await runNodesCommand([
"nodes",
"run",
"--node",
"mac-1",
"--cwd",
"/tmp",
"--env",
"FOO=bar",
"--command-timeout",
"1200",
"--needs-screen-recording",
"--invoke-timeout",
"5000",
"echo",
"hi",
]);
expect(invoke).toBeTruthy();
expect(invoke?.params?.idempotencyKey).toBe("rk_test");
expect(invoke?.params?.command).toBe("system.run");
expect(invoke?.params?.params).toEqual({
command: ["echo", "hi"],
rawCommand: "echo hi",
cwd: "/tmp",
env: { FOO: "bar" },
timeoutMs: 1200,
needsScreenRecording: true,
agentId: "main",
approved: true,
approvalDecision: "allow-once",
runId: expect.any(String),
});
expect(invoke?.params?.timeoutMs).toBe(5000);
const approval = getApprovalRequestCall();
expect(approval?.params?.["systemRunPlan"]).toEqual({
argv: ["echo", "hi"],
cwd: "/tmp",
commandText: "echo hi",
commandPreview: null,
agentId: "main",
sessionKey: null,
it("does not register the removed run wrapper", async () => {
await expect(
sharedProgram.parseAsync(["nodes", "run", "--node", "mac-1"], { from: "user" }),
).rejects.toMatchObject({
code: "commander.unknownCommand",
});
});
it("invokes system.run with raw command", async () => {
const invoke = await runNodesCommand([
"nodes",
"run",
"--agent",
"main",
"--node",
"mac-1",
"--raw",
"echo hi",
]);
expect(invoke).toBeTruthy();
expect(invoke?.params?.idempotencyKey).toBe("rk_test");
expect(invoke?.params?.command).toBe("system.run");
expect(invoke?.params?.params).toMatchObject({
command: ["/bin/sh", "-lc", "echo hi"],
rawCommand: '/bin/sh -lc "echo hi"',
agentId: "main",
approved: true,
approvalDecision: "allow-once",
runId: expect.any(String),
});
const approval = getApprovalRequestCall();
expect(approval?.params?.["systemRunPlan"]).toEqual({
argv: ["/bin/sh", "-lc", "echo hi"],
cwd: null,
commandText: '/bin/sh -lc "echo hi"',
commandPreview: "echo hi",
agentId: "main",
sessionKey: null,
});
});
it("inherits ask=off from local exec approvals when tools.exec.ask is unset", async () => {
localExecApprovalsFile = {
version: 1,
defaults: {
security: "allowlist",
ask: "off",
askFallback: "deny",
},
agents: {},
};
nodeExecApprovalsFile = {
version: 1,
defaults: {
security: "allowlist",
askFallback: "deny",
},
agents: {},
};
const invoke = await runNodesCommand(["nodes", "run", "--node", "mac-1", "echo", "hi"]);
expect(invoke).toBeTruthy();
expect(invoke?.params?.command).toBe("system.run");
expect(invoke?.params?.params).toMatchObject({
command: ["echo", "hi"],
approved: false,
});
expect(invoke?.params?.params).not.toHaveProperty("approvalDecision");
expect(getApprovalRequestCall()).toBeNull();
it("blocks system.run on nodes invoke", async () => {
await expect(
sharedProgram.parseAsync(["nodes", "invoke", "--node", "mac-1", "--command", "system.run"], {
from: "user",
}),
).rejects.toThrow("__exit__:1");
expect(runtimeErrors.at(-1)).toContain('command "system.run" is reserved for shell execution');
});
it("invokes system.notify with provided fields", async () => {

View File

@@ -1,19 +1,18 @@
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { DEFAULT_EXEC_APPROVAL_TIMEOUT_MS } from "../../infra/exec-approvals.js";
import { parseTimeoutMs } from "../nodes-run.js";
import { parseTimeoutMs } from "../parse-timeout.js";
/**
* Regression test for #12098:
* `openclaw nodes run` times out after 35s because the CLI transport timeout
* (35s default) is shorter than the exec approval timeout (120s). The
* exec.approval.request call must use a transport timeout at least as long
* as the approval timeout so the gateway has enough time to collect the
* user's decision.
* `exec.approval.request` times out after 35s when the CLI transport timeout
* is shorter than the exec approval timeout (120s). The transport timeout
* must be at least as long as the approval timeout so the gateway has enough
* time to collect the user's decision.
*
* The root cause: callGatewayCli reads opts.timeout for the transport timeout.
* Before the fix, nodes run called callGatewayCli("exec.approval.request", opts, ...)
* without overriding opts.timeout, so the 35s CLI default raced against the
* 120s approval wait on the gateway side. The CLI always lost.
* Before the fix, node exec flows called callGatewayCli("exec.approval.request",
* opts, ...) without overriding opts.timeout, so the 35s CLI default raced
* against the 120s approval wait on the gateway side. The CLI always lost.
*
* The fix: override the transport timeout for exec.approval.request to be at
* least approvalTimeoutMs + 10_000.
@@ -32,7 +31,7 @@ vi.mock("../progress.js", () => ({
withProgress: (_opts: unknown, fn: () => unknown) => fn(),
}));
describe("nodes run: approval transport timeout (#12098)", () => {
describe("exec approval transport timeout (#12098)", () => {
let callGatewayCli: typeof import("./rpc.js").callGatewayCli;
beforeAll(async () => {
@@ -100,7 +99,7 @@ describe("nodes run: approval transport timeout (#12098)", () => {
// parseTimeoutMs returns undefined for garbage input, ?? 0 ensures
// Math.max picks the approval floor instead of producing NaN
const transportTimeoutMs = Math.max(parseTimeoutMs("foo") ?? 0, approvalTimeoutMs + 10_000);
expect(transportTimeoutMs).toBe(130_000);
expect(transportTimeoutMs).toBe(approvalTimeoutMs + 10_000);
await callGatewayCli(
"exec.approval.request",
@@ -110,6 +109,6 @@ describe("nodes run: approval transport timeout (#12098)", () => {
);
const callOpts = callGatewaySpy.mock.calls[0][0];
expect(callOpts.timeoutMs).toBe(130_000);
expect(callOpts.timeoutMs).toBe(approvalTimeoutMs + 10_000);
});
});