From c746ec5fc721583bcb28dbb868aa0ce05c8c25c2 Mon Sep 17 00:00:00 2001 From: samzong Date: Fri, 15 May 2026 23:44:56 +0800 Subject: [PATCH] fix(file-transfer): avoid eager invoke policy load Signed-off-by: samzong --- extensions/file-transfer/index.test.ts | 37 +++++++ extensions/file-transfer/index.ts | 4 +- .../shared/lazy-node-invoke-policy.test.ts | 101 ++++++++++++++++++ .../src/shared/lazy-node-invoke-policy.ts | 35 ++++++ .../src/shared/node-invoke-policy-commands.ts | 8 ++ .../src/shared/node-invoke-policy.ts | 12 ++- 6 files changed, 190 insertions(+), 7 deletions(-) create mode 100644 extensions/file-transfer/src/shared/lazy-node-invoke-policy.test.ts create mode 100644 extensions/file-transfer/src/shared/lazy-node-invoke-policy.ts create mode 100644 extensions/file-transfer/src/shared/node-invoke-policy-commands.ts diff --git a/extensions/file-transfer/index.test.ts b/extensions/file-transfer/index.test.ts index cd0d7c5c252..e1bbbcc7fe5 100644 --- a/extensions/file-transfer/index.test.ts +++ b/extensions/file-transfer/index.test.ts @@ -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(); + }); }); diff --git a/extensions/file-transfer/index.ts b/extensions/file-transfer/index.ts index a14a6fa11b0..abd2b368bf9 100644 --- a/extensions/file-transfer/index.ts +++ b/extensions/file-transfer/index.ts @@ -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"); diff --git a/extensions/file-transfer/src/shared/lazy-node-invoke-policy.test.ts b/extensions/file-transfer/src/shared/lazy-node-invoke-policy.test.ts new file mode 100644 index 00000000000..7b4349299ae --- /dev/null +++ b/extensions/file-transfer/src/shared/lazy-node-invoke-policy.test.ts @@ -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 { + 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(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>(); + + 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(async () => ({ + ok: true, + payload: { ok: true }, + payloadJSON: null, + })); + const delegateHandle = vi.fn(async (ctx) => { + await ctx.invokeNode(); + return { ok: true, payload: { delegated: true } }; + }); + const loadPolicy = vi.fn<() => Promise>(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(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); + }); +}); diff --git a/extensions/file-transfer/src/shared/lazy-node-invoke-policy.ts b/extensions/file-transfer/src/shared/lazy-node-invoke-policy.ts new file mode 100644 index 00000000000..248e4067870 --- /dev/null +++ b/extensions/file-transfer/src/shared/lazy-node-invoke-policy.ts @@ -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; + +const loadFileTransferNodeInvokePolicy: LoadFileTransferNodeInvokePolicy = async () => { + const { createFileTransferNodeInvokePolicy } = await import("./node-invoke-policy.js"); + return createFileTransferNodeInvokePolicy(); +}; + +export function createLazyFileTransferNodeInvokePolicy( + loadPolicy: LoadFileTransferNodeInvokePolicy = loadFileTransferNodeInvokePolicy, +): OpenClawPluginNodeInvokePolicy { + let policyPromise: Promise | 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); + }, + }; +} diff --git a/extensions/file-transfer/src/shared/node-invoke-policy-commands.ts b/extensions/file-transfer/src/shared/node-invoke-policy-commands.ts new file mode 100644 index 00000000000..1579038819a --- /dev/null +++ b/extensions/file-transfer/src/shared/node-invoke-policy-commands.ts @@ -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]; diff --git a/extensions/file-transfer/src/shared/node-invoke-policy.ts b/extensions/file-transfer/src/shared/node-invoke-policy.ts index a2017cb06d7..fe535ecec5d 100644 --- a/extensions/file-transfer/src/shared/node-invoke-policy.ts +++ b/extensions/file-transfer/src/shared/node-invoke-policy.ts @@ -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 { return value && typeof value === "object" && !Array.isArray(value) @@ -651,7 +653,7 @@ async function runDirFetchPreflight(input: { async function handleFileTransferInvoke( ctx: OpenClawPluginNodeInvokePolicyContext, ): Promise { - 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, }; }