mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 17:04:46 +00:00
fix(file-transfer): avoid eager invoke policy load
Signed-off-by: samzong <samzong.lu@gmail.com>
This commit is contained in:
committed by
Peter Steinberger
parent
dd457474b3
commit
c746ec5fc7
@@ -15,6 +15,7 @@ vi.mock("./src/tools/file-fetch-tool.js", rejectRuntimeImport("tools/file-fetch-
|
||||
vi.mock("./src/tools/dir-list-tool.js", rejectRuntimeImport("tools/dir-list-tool"));
|
||||
vi.mock("./src/tools/dir-fetch-tool.js", rejectRuntimeImport("tools/dir-fetch-tool"));
|
||||
vi.mock("./src/tools/file-write-tool.js", rejectRuntimeImport("tools/file-write-tool"));
|
||||
vi.mock("./src/shared/node-invoke-policy.js", rejectRuntimeImport("shared/node-invoke-policy"));
|
||||
|
||||
afterAll(() => {
|
||||
vi.doUnmock("./src/node-host/file-fetch.js");
|
||||
@@ -25,6 +26,7 @@ afterAll(() => {
|
||||
vi.doUnmock("./src/tools/dir-list-tool.js");
|
||||
vi.doUnmock("./src/tools/dir-fetch-tool.js");
|
||||
vi.doUnmock("./src/tools/file-write-tool.js");
|
||||
vi.doUnmock("./src/shared/node-invoke-policy.js");
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
@@ -45,6 +47,12 @@ describe("file-transfer plugin entry", () => {
|
||||
"file.write",
|
||||
]);
|
||||
expect(registerNodeInvokePolicy).toHaveBeenCalledTimes(1);
|
||||
expect(registerNodeInvokePolicy.mock.calls[0]?.[0].commands).toEqual([
|
||||
"file.fetch",
|
||||
"dir.list",
|
||||
"dir.fetch",
|
||||
"file.write",
|
||||
]);
|
||||
expect(registerTool.mock.calls.map(([tool]) => tool.name)).toEqual([
|
||||
"file_fetch",
|
||||
"dir_list",
|
||||
@@ -52,4 +60,33 @@ describe("file-transfer plugin entry", () => {
|
||||
"file_write",
|
||||
]);
|
||||
});
|
||||
|
||||
it("fails closed if the lazy policy module cannot load", async () => {
|
||||
const registerNodeInvokePolicy = vi.fn();
|
||||
const registerTool = vi.fn();
|
||||
const invokeNode = vi.fn();
|
||||
|
||||
pluginEntry.register({
|
||||
registerNodeInvokePolicy,
|
||||
registerTool,
|
||||
} as never);
|
||||
|
||||
const policy = registerNodeInvokePolicy.mock.calls[0]?.[0];
|
||||
await expect(
|
||||
policy.handle({
|
||||
nodeId: "node-1",
|
||||
command: "file.fetch",
|
||||
params: { path: "/tmp/a.txt" },
|
||||
config: {},
|
||||
pluginConfig: {},
|
||||
client: null,
|
||||
invokeNode,
|
||||
}),
|
||||
).resolves.toMatchObject({
|
||||
ok: false,
|
||||
code: "PLUGIN_POLICY_UNAVAILABLE",
|
||||
unavailable: true,
|
||||
});
|
||||
expect(invokeNode).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,7 +3,7 @@ import {
|
||||
type AnyAgentTool,
|
||||
type OpenClawPluginNodeHostCommand,
|
||||
} from "openclaw/plugin-sdk/plugin-entry";
|
||||
import { createFileTransferNodeInvokePolicy } from "./src/shared/node-invoke-policy.js";
|
||||
import { createLazyFileTransferNodeInvokePolicy } from "./src/shared/lazy-node-invoke-policy.js";
|
||||
import {
|
||||
DIR_FETCH_TOOL_DESCRIPTOR,
|
||||
DIR_LIST_TOOL_DESCRIPTOR,
|
||||
@@ -91,7 +91,7 @@ export default definePluginEntry({
|
||||
description: "Fetch, list, and write files on paired nodes via dedicated node commands.",
|
||||
nodeHostCommands: fileTransferNodeHostCommands,
|
||||
register(api) {
|
||||
api.registerNodeInvokePolicy(createFileTransferNodeInvokePolicy());
|
||||
api.registerNodeInvokePolicy(createLazyFileTransferNodeInvokePolicy());
|
||||
api.registerTool(
|
||||
createLazyTool(FILE_FETCH_TOOL_DESCRIPTOR, async () => {
|
||||
const { createFileFetchTool } = await import("./src/tools/file-fetch-tool.js");
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
import type {
|
||||
OpenClawPluginNodeInvokePolicy,
|
||||
OpenClawPluginNodeInvokePolicyContext,
|
||||
} from "openclaw/plugin-sdk/plugin-entry";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { createLazyFileTransferNodeInvokePolicy } from "./lazy-node-invoke-policy.js";
|
||||
|
||||
function createPolicyContext(
|
||||
overrides: Partial<OpenClawPluginNodeInvokePolicyContext> = {},
|
||||
): OpenClawPluginNodeInvokePolicyContext {
|
||||
return {
|
||||
nodeId: "node-1",
|
||||
command: "file.fetch",
|
||||
params: { path: "/tmp/a.txt" },
|
||||
config: {} as never,
|
||||
pluginConfig: {},
|
||||
node: {
|
||||
nodeId: "node-1",
|
||||
displayName: "Test Node",
|
||||
commands: ["file.fetch"],
|
||||
},
|
||||
client: null,
|
||||
invokeNode: vi.fn<OpenClawPluginNodeInvokePolicyContext["invokeNode"]>(async () => ({
|
||||
ok: true,
|
||||
payload: { ok: true },
|
||||
payloadJSON: null,
|
||||
})),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("lazy file-transfer node invoke policy", () => {
|
||||
it("exposes command metadata without loading the delegate", () => {
|
||||
const loadPolicy = vi.fn<() => Promise<OpenClawPluginNodeInvokePolicy>>();
|
||||
|
||||
const policy = createLazyFileTransferNodeInvokePolicy(loadPolicy);
|
||||
|
||||
expect(policy.commands).toEqual(["file.fetch", "dir.list", "dir.fetch", "file.write"]);
|
||||
expect(loadPolicy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("loads and caches the delegate on first handle", async () => {
|
||||
const invokeNode = vi.fn<OpenClawPluginNodeInvokePolicyContext["invokeNode"]>(async () => ({
|
||||
ok: true,
|
||||
payload: { ok: true },
|
||||
payloadJSON: null,
|
||||
}));
|
||||
const delegateHandle = vi.fn<OpenClawPluginNodeInvokePolicy["handle"]>(async (ctx) => {
|
||||
await ctx.invokeNode();
|
||||
return { ok: true, payload: { delegated: true } };
|
||||
});
|
||||
const loadPolicy = vi.fn<() => Promise<OpenClawPluginNodeInvokePolicy>>(async () => ({
|
||||
commands: ["file.fetch"],
|
||||
handle: delegateHandle,
|
||||
}));
|
||||
const policy = createLazyFileTransferNodeInvokePolicy(loadPolicy);
|
||||
|
||||
await expect(policy.handle(createPolicyContext({ invokeNode }))).resolves.toEqual({
|
||||
ok: true,
|
||||
payload: { delegated: true },
|
||||
});
|
||||
await expect(policy.handle(createPolicyContext({ invokeNode }))).resolves.toEqual({
|
||||
ok: true,
|
||||
payload: { delegated: true },
|
||||
});
|
||||
|
||||
expect(loadPolicy).toHaveBeenCalledTimes(1);
|
||||
expect(delegateHandle).toHaveBeenCalledTimes(2);
|
||||
expect(invokeNode).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("fails closed when the delegate cannot load", async () => {
|
||||
const invokeNode = vi.fn<OpenClawPluginNodeInvokePolicyContext["invokeNode"]>(async () => ({
|
||||
ok: true,
|
||||
payload: { ok: true },
|
||||
payloadJSON: null,
|
||||
}));
|
||||
const policy = createLazyFileTransferNodeInvokePolicy(async () => {
|
||||
throw new Error("load failed");
|
||||
});
|
||||
|
||||
await expect(policy.handle(createPolicyContext({ invokeNode }))).resolves.toMatchObject({
|
||||
ok: false,
|
||||
code: "PLUGIN_POLICY_UNAVAILABLE",
|
||||
unavailable: true,
|
||||
});
|
||||
expect(invokeNode).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not rewrite delegate failures as load failures", async () => {
|
||||
const delegateError = new Error("delegate failed");
|
||||
const policy = createLazyFileTransferNodeInvokePolicy(async () => ({
|
||||
commands: ["file.fetch"],
|
||||
handle: async () => {
|
||||
throw delegateError;
|
||||
},
|
||||
}));
|
||||
|
||||
await expect(policy.handle(createPolicyContext())).rejects.toBe(delegateError);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,35 @@
|
||||
import type { OpenClawPluginNodeInvokePolicy } from "openclaw/plugin-sdk/plugin-entry";
|
||||
import { FILE_TRANSFER_NODE_INVOKE_COMMANDS } from "./node-invoke-policy-commands.js";
|
||||
|
||||
type LoadFileTransferNodeInvokePolicy = () => Promise<OpenClawPluginNodeInvokePolicy>;
|
||||
|
||||
const loadFileTransferNodeInvokePolicy: LoadFileTransferNodeInvokePolicy = async () => {
|
||||
const { createFileTransferNodeInvokePolicy } = await import("./node-invoke-policy.js");
|
||||
return createFileTransferNodeInvokePolicy();
|
||||
};
|
||||
|
||||
export function createLazyFileTransferNodeInvokePolicy(
|
||||
loadPolicy: LoadFileTransferNodeInvokePolicy = loadFileTransferNodeInvokePolicy,
|
||||
): OpenClawPluginNodeInvokePolicy {
|
||||
let policyPromise: Promise<OpenClawPluginNodeInvokePolicy> | undefined;
|
||||
|
||||
return {
|
||||
commands: [...FILE_TRANSFER_NODE_INVOKE_COMMANDS],
|
||||
async handle(ctx) {
|
||||
let policy: OpenClawPluginNodeInvokePolicy;
|
||||
try {
|
||||
policyPromise ??= loadPolicy();
|
||||
policy = await policyPromise;
|
||||
} catch (error) {
|
||||
const message = error instanceof Error && error.message ? error.message : String(error);
|
||||
return {
|
||||
ok: false,
|
||||
code: "PLUGIN_POLICY_UNAVAILABLE",
|
||||
message: `file-transfer PLUGIN_POLICY_UNAVAILABLE: node.invoke policy unavailable: ${message}`,
|
||||
unavailable: true,
|
||||
};
|
||||
}
|
||||
return await policy.handle(ctx);
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,8 @@
|
||||
export const FILE_TRANSFER_NODE_INVOKE_COMMANDS = [
|
||||
"file.fetch",
|
||||
"dir.list",
|
||||
"dir.fetch",
|
||||
"file.write",
|
||||
] as const;
|
||||
|
||||
export type FileTransferNodeInvokeCommand = (typeof FILE_TRANSFER_NODE_INVOKE_COMMANDS)[number];
|
||||
@@ -5,6 +5,10 @@ import type {
|
||||
OpenClawPluginNodeInvokePolicyResult,
|
||||
} from "openclaw/plugin-sdk/plugin-entry";
|
||||
import { appendFileTransferAudit, type FileTransferAuditOp } from "./audit.js";
|
||||
import {
|
||||
FILE_TRANSFER_NODE_INVOKE_COMMANDS,
|
||||
type FileTransferNodeInvokeCommand,
|
||||
} from "./node-invoke-policy-commands.js";
|
||||
import { evaluateFilePolicy, persistAllowAlways, type FilePolicyKind } from "./policy.js";
|
||||
|
||||
const FILE_FETCH_DEFAULT_MAX_BYTES = 8 * 1024 * 1024;
|
||||
@@ -14,9 +18,7 @@ const DIR_FETCH_HARD_MAX_BYTES = 16 * 1024 * 1024;
|
||||
const DIR_FETCH_ARCHIVE_LIST_TIMEOUT_MS = 30_000;
|
||||
const DIR_FETCH_ARCHIVE_LIST_MAX_OUTPUT_BYTES = 32 * 1024 * 1024;
|
||||
|
||||
type FileTransferCommand = "file.fetch" | "dir.list" | "dir.fetch" | "file.write";
|
||||
|
||||
const COMMANDS: FileTransferCommand[] = ["file.fetch", "dir.list", "dir.fetch", "file.write"];
|
||||
type FileTransferCommand = FileTransferNodeInvokeCommand;
|
||||
|
||||
function asRecord(value: unknown): Record<string, unknown> {
|
||||
return value && typeof value === "object" && !Array.isArray(value)
|
||||
@@ -651,7 +653,7 @@ async function runDirFetchPreflight(input: {
|
||||
async function handleFileTransferInvoke(
|
||||
ctx: OpenClawPluginNodeInvokePolicyContext,
|
||||
): Promise<OpenClawPluginNodeInvokePolicyResult> {
|
||||
if (!COMMANDS.includes(ctx.command as FileTransferCommand)) {
|
||||
if (!FILE_TRANSFER_NODE_INVOKE_COMMANDS.includes(ctx.command as FileTransferCommand)) {
|
||||
return { ok: false, code: "UNSUPPORTED_COMMAND", message: "unsupported file-transfer command" };
|
||||
}
|
||||
const command = ctx.command as FileTransferCommand;
|
||||
@@ -837,7 +839,7 @@ async function handleFileTransferInvoke(
|
||||
|
||||
export function createFileTransferNodeInvokePolicy(): OpenClawPluginNodeInvokePolicy {
|
||||
return {
|
||||
commands: COMMANDS,
|
||||
commands: [...FILE_TRANSFER_NODE_INVOKE_COMMANDS],
|
||||
handle: handleFileTransferInvoke,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user