mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
fix(agents): keep node agent tools on dedicated surfaces (#125434)
* fix(agents): block generic node tool reentry * fix(agents): enforce node tool ownership for typed actions * fix(agents): clarify dedicated node tool policy errors
This commit is contained in:
@@ -15,17 +15,11 @@ import {
|
||||
readToolStringParam,
|
||||
} from "./common.js";
|
||||
import type { GatewayCallOptions } from "./gateway.js";
|
||||
import { callGatewayTool } from "./gateway.js";
|
||||
import { callNodesToolNodeInvoke } from "./nodes-tool-invoke.js";
|
||||
import { POLICY_REDIRECT_INVOKE_COMMANDS } from "./nodes-tool-media.js";
|
||||
import { resolveAgentNodeId } from "./nodes-utils.js";
|
||||
|
||||
const BLOCKED_INVOKE_COMMANDS = new Set(["system.run", "system.run.prepare"]);
|
||||
const DEDICATED_TOOL_INVOKE_COMMANDS = new Map([
|
||||
["computer.act", "computer"],
|
||||
["mobile.ui.observe", "mobile_ui"],
|
||||
["mobile.ui.act", "mobile_ui"],
|
||||
]);
|
||||
|
||||
const NODE_READ_ACTION_COMMANDS = {
|
||||
camera_list: "camera.list",
|
||||
notifications_list: "notifications.list",
|
||||
@@ -189,12 +183,6 @@ export async function executeNodeCommandAction(params: {
|
||||
`invokeCommand "${invokeCommand}" is reserved for shell execution; use exec with host=node instead`,
|
||||
);
|
||||
}
|
||||
const dedicatedTool = DEDICATED_TOOL_INVOKE_COMMANDS.get(invokeCommandNormalized);
|
||||
if (dedicatedTool) {
|
||||
throw new Error(
|
||||
`invokeCommand "${invokeCommand}" cannot be invoked through the generic nodes surface; use the dedicated ${dedicatedTool} tool`,
|
||||
);
|
||||
}
|
||||
const dedicatedAction = params.mediaInvokeActions[invokeCommandNormalized];
|
||||
// Policy-redirect commands (file-transfer) ALWAYS reroute to their
|
||||
// dedicated tool. The dedicated tool runs gatekeep() + path policy
|
||||
@@ -228,14 +216,18 @@ export async function executeNodeCommandAction(params: {
|
||||
}
|
||||
}
|
||||
const invokeTimeoutMs = readPositiveIntegerParam(params.input, "invokeTimeoutMs");
|
||||
const raw = await callGatewayTool("node.invoke", params.gatewayOpts, {
|
||||
nodeId,
|
||||
command: invokeCommand,
|
||||
params: invokeParams,
|
||||
timeoutMs: invokeTimeoutMs,
|
||||
idempotencyKey: crypto.randomUUID(),
|
||||
...(params.agentSessionKey ? { sessionKey: params.agentSessionKey } : {}),
|
||||
});
|
||||
const raw = await callNodesToolNodeInvoke(
|
||||
params.gatewayOpts,
|
||||
{
|
||||
nodeId,
|
||||
command: invokeCommand,
|
||||
params: invokeParams,
|
||||
timeoutMs: invokeTimeoutMs,
|
||||
idempotencyKey: crypto.randomUUID(),
|
||||
...(params.agentSessionKey ? { sessionKey: params.agentSessionKey } : {}),
|
||||
},
|
||||
{ rawInvoke: true },
|
||||
);
|
||||
return jsonResult(raw ?? {});
|
||||
}
|
||||
}
|
||||
@@ -249,7 +241,7 @@ async function invokeNodeCommandPayload(params: {
|
||||
commandParams?: Record<string, unknown>;
|
||||
}): Promise<unknown> {
|
||||
const nodeId = await resolveAgentNodeId(params.gatewayOpts, params.node);
|
||||
const raw = await callGatewayTool<{ payload: unknown }>("node.invoke", params.gatewayOpts, {
|
||||
const raw = await callNodesToolNodeInvoke<{ payload: unknown }>(params.gatewayOpts, {
|
||||
nodeId,
|
||||
command: params.command,
|
||||
params: params.commandParams ?? {},
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
callGatewayTool: vi.fn(),
|
||||
listConnectedNodePluginTools: vi.fn<
|
||||
() => Array<{ nodeId: string; descriptor: { name: string; command?: string } }>
|
||||
>(() => []),
|
||||
resolveAgentNodeId: vi.fn(async () => "node-1"),
|
||||
}));
|
||||
|
||||
vi.mock("./gateway.js", () => ({ callGatewayTool: mocks.callGatewayTool }));
|
||||
vi.mock("./nodes-utils.js", () => ({ resolveAgentNodeId: mocks.resolveAgentNodeId }));
|
||||
vi.mock("../../gateway/node-plugin-tool-snapshot.js", () => ({
|
||||
listConnectedNodePluginTools: mocks.listConnectedNodePluginTools,
|
||||
}));
|
||||
|
||||
const { executeNodeCommandAction } = await import("./nodes-tool-commands.js");
|
||||
|
||||
async function execute(action: "device_status" | "invoke", input: Record<string, unknown>) {
|
||||
return executeNodeCommandAction({
|
||||
action,
|
||||
input: { node: "macbook", ...input },
|
||||
gatewayOpts: {},
|
||||
mediaInvokeActions: {},
|
||||
});
|
||||
}
|
||||
|
||||
async function invoke(command: string) {
|
||||
return execute("invoke", { invokeCommand: command, invokeParamsJson: "{}" });
|
||||
}
|
||||
|
||||
describe("generic node invoke policy", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mocks.listConnectedNodePluginTools.mockReturnValue([]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "node-hosted MCP",
|
||||
command: "mcp.tools.call.v1",
|
||||
descriptors: [
|
||||
{
|
||||
nodeId: "node-1",
|
||||
descriptor: { name: "docs_search", command: "mcp.tools.call.v1" },
|
||||
},
|
||||
],
|
||||
rawToolName: "docs_search",
|
||||
},
|
||||
{
|
||||
name: "node-published plugin tool",
|
||||
command: "remote.secret",
|
||||
descriptors: [
|
||||
{ nodeId: "node-1", descriptor: { name: "remote_secret", command: "remote.secret" } },
|
||||
],
|
||||
rawToolName: "remote_secret",
|
||||
},
|
||||
])("blocks raw $name commands in favor of policy-filtered tools", async (testCase) => {
|
||||
mocks.listConnectedNodePluginTools.mockReturnValue(testCase.descriptors);
|
||||
|
||||
const error = await invoke(testCase.command).then(
|
||||
() => undefined,
|
||||
(cause: unknown) => cause,
|
||||
);
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
expect(String(error)).toContain(
|
||||
"use the matching dedicated agent tool if available; otherwise this command is disabled by tool policy",
|
||||
);
|
||||
expect(String(error)).not.toContain(testCase.rawToolName);
|
||||
expect(mocks.callGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows ordinary generic commands when another node publishes the same command", async () => {
|
||||
mocks.listConnectedNodePluginTools.mockReturnValue([
|
||||
{ nodeId: "node-2", descriptor: { name: "remote_status", command: "device.status" } },
|
||||
]);
|
||||
mocks.callGatewayTool.mockResolvedValue({ payload: { ok: true } });
|
||||
|
||||
await invoke("device.status");
|
||||
|
||||
expect(mocks.callGatewayTool).toHaveBeenCalledWith(
|
||||
"node.invoke",
|
||||
{},
|
||||
expect.objectContaining({ nodeId: "node-1", command: "device.status" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks a typed action when the selected node publishes its command as an agent tool", async () => {
|
||||
mocks.listConnectedNodePluginTools.mockReturnValue([
|
||||
{ nodeId: "node-1", descriptor: { name: "remote_status", command: "device.status" } },
|
||||
]);
|
||||
|
||||
await expect(execute("device_status", {})).rejects.toThrow(
|
||||
"use the matching dedicated agent tool if available; otherwise this command is disabled by tool policy",
|
||||
);
|
||||
expect(mocks.callGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows a typed action when only another node publishes its command", async () => {
|
||||
mocks.listConnectedNodePluginTools.mockReturnValue([
|
||||
{ nodeId: "node-2", descriptor: { name: "remote_status", command: "device.status" } },
|
||||
]);
|
||||
mocks.callGatewayTool.mockResolvedValue({ payload: { ok: true } });
|
||||
|
||||
await execute("device_status", {});
|
||||
|
||||
expect(mocks.callGatewayTool).toHaveBeenCalledWith(
|
||||
"node.invoke",
|
||||
{},
|
||||
expect.objectContaining({ nodeId: "node-1", command: "device.status" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,45 @@
|
||||
import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce";
|
||||
import { listConnectedNodePluginTools } from "../../gateway/node-plugin-tool-snapshot.js";
|
||||
import { NODE_MCP_TOOLS_CALL_COMMAND } from "../../infra/node-commands.js";
|
||||
import type { GatewayCallOptions } from "./gateway.js";
|
||||
import { callGatewayTool } from "./gateway.js";
|
||||
|
||||
const DEDICATED_TOOL_INVOKE_COMMANDS = new Map([
|
||||
["computer.act", "computer"],
|
||||
["mobile.ui.observe", "mobile_ui"],
|
||||
["mobile.ui.act", "mobile_ui"],
|
||||
]);
|
||||
|
||||
export async function callNodesToolNodeInvoke<T = Record<string, unknown>>(
|
||||
gatewayOpts: GatewayCallOptions,
|
||||
params: {
|
||||
nodeId: string;
|
||||
command: string;
|
||||
params?: unknown;
|
||||
timeoutMs?: number;
|
||||
idempotencyKey: string;
|
||||
sessionKey?: string;
|
||||
},
|
||||
options?: { rawInvoke?: boolean },
|
||||
): Promise<T> {
|
||||
const command = normalizeLowercaseStringOrEmpty(params.command);
|
||||
// Node-published agent tools own their model policy. Every Nodes action must
|
||||
// stay out of commands omitted from this agent's materialized tool set.
|
||||
const dedicatedTool = DEDICATED_TOOL_INVOKE_COMMANDS.get(command);
|
||||
const nodePublishedTool = listConnectedNodePluginTools().some(
|
||||
(entry) =>
|
||||
entry.nodeId === params.nodeId &&
|
||||
normalizeLowercaseStringOrEmpty(entry.descriptor.command) === command,
|
||||
);
|
||||
if (dedicatedTool || command === NODE_MCP_TOOLS_CALL_COMMAND || nodePublishedTool) {
|
||||
const guidance = dedicatedTool
|
||||
? `use the dedicated ${dedicatedTool} tool if available; otherwise this command is disabled by tool policy`
|
||||
: "use the matching dedicated agent tool if available; otherwise this command is disabled by tool policy";
|
||||
throw new Error(
|
||||
options?.rawInvoke
|
||||
? `invokeCommand "${params.command}" cannot be invoked through the generic nodes surface; ${guidance}`
|
||||
: `node command "${params.command}" cannot be invoked through the Nodes tool; ${guidance}`,
|
||||
);
|
||||
}
|
||||
return await callGatewayTool<T>("node.invoke", gatewayOpts, params);
|
||||
}
|
||||
@@ -40,7 +40,7 @@ import {
|
||||
readPositiveIntegerParam,
|
||||
} from "./common.js";
|
||||
import type { GatewayCallOptions } from "./gateway.js";
|
||||
import { callGatewayTool } from "./gateway.js";
|
||||
import { callNodesToolNodeInvoke } from "./nodes-tool-invoke.js";
|
||||
import { resolveAgentNode, resolveAgentNodeId } from "./nodes-utils.js";
|
||||
|
||||
export const MEDIA_INVOKE_ACTIONS = {
|
||||
@@ -185,7 +185,7 @@ async function executeCameraSnap({
|
||||
const details: Array<Record<string, unknown>> = [];
|
||||
|
||||
for (const target of targets) {
|
||||
const raw = await callGatewayTool<{ payload: unknown }>("node.invoke", gatewayOpts, {
|
||||
const raw = await callNodesToolNodeInvoke<{ payload: unknown }>(gatewayOpts, {
|
||||
nodeId,
|
||||
command: "camera.snap",
|
||||
params: {
|
||||
@@ -256,7 +256,7 @@ async function executePhotosLatest({
|
||||
max: 1,
|
||||
message: "quality must be between 0 and 1",
|
||||
}) ?? DEFAULT_PHOTOS_QUALITY;
|
||||
const raw = await callGatewayTool<{ payload: unknown }>("node.invoke", gatewayOpts, {
|
||||
const raw = await callNodesToolNodeInvoke<{ payload: unknown }>(gatewayOpts, {
|
||||
nodeId,
|
||||
command: "photos.latest",
|
||||
params: {
|
||||
@@ -349,7 +349,7 @@ async function executeCameraClip({
|
||||
? params.deviceId.trim()
|
||||
: undefined;
|
||||
const timeouts = resolveRecordingTimeouts({ input: params, gatewayOpts, durationMs });
|
||||
const raw = await callGatewayTool<{ payload: unknown }>("node.invoke", timeouts.gatewayOpts, {
|
||||
const raw = await callNodesToolNodeInvoke<{ payload: unknown }>(timeouts.gatewayOpts, {
|
||||
nodeId,
|
||||
command: "camera.clip",
|
||||
params: {
|
||||
@@ -399,7 +399,7 @@ async function executeScreenRecord({
|
||||
const screenIndex = readNonNegativeIntegerParam(params, "screenIndex") ?? 0;
|
||||
const includeAudio = typeof params.includeAudio === "boolean" ? params.includeAudio : true;
|
||||
const timeouts = resolveRecordingTimeouts({ input: params, gatewayOpts, durationMs });
|
||||
const raw = await callGatewayTool<{ payload: unknown }>("node.invoke", timeouts.gatewayOpts, {
|
||||
const raw = await callNodesToolNodeInvoke<{ payload: unknown }>(timeouts.gatewayOpts, {
|
||||
nodeId,
|
||||
command: "screen.record",
|
||||
params: {
|
||||
@@ -442,7 +442,7 @@ async function executeScreenSnapshot({
|
||||
// The node owns the encoding choice, so ask for the one the caller's filename
|
||||
// already promises instead of letting the default contradict it.
|
||||
const requestedFormat = outPath ? screenSnapshotFormatForPath(outPath) : undefined;
|
||||
const raw = await callGatewayTool<{ payload: unknown }>("node.invoke", gatewayOpts, {
|
||||
const raw = await callNodesToolNodeInvoke<{ payload: unknown }>(gatewayOpts, {
|
||||
nodeId,
|
||||
command: "screen.snapshot",
|
||||
params: { screenIndex, maxWidth, format: requestedFormat },
|
||||
|
||||
@@ -23,6 +23,7 @@ import { type AnyAgentTool, jsonResult, readToolStringParam } from "./common.js"
|
||||
import { gatewayCallOptionSchemaProperties } from "./gateway-schema.js";
|
||||
import { callGatewayTool, readGatewayCallOptions } from "./gateway.js";
|
||||
import { executeNodeCommandAction, type NodeCommandAction } from "./nodes-tool-commands.js";
|
||||
import { callNodesToolNodeInvoke } from "./nodes-tool-invoke.js";
|
||||
import { executeNodeMediaAction, MEDIA_INVOKE_ACTIONS } from "./nodes-tool-media.js";
|
||||
import { resolveAgentNodeId } from "./nodes-utils.js";
|
||||
|
||||
@@ -243,7 +244,7 @@ export function createNodesTool(options?: {
|
||||
throw new Error("title or body required");
|
||||
}
|
||||
const nodeId = await resolveAgentNodeId(gatewayOpts, node);
|
||||
await callGatewayTool("node.invoke", gatewayOpts, {
|
||||
await callNodesToolNodeInvoke(gatewayOpts, {
|
||||
nodeId,
|
||||
command: "system.notify",
|
||||
params: {
|
||||
|
||||
Reference in New Issue
Block a user