From 643633c1e5fc26201bc31031fd03c6ed4fa0f676 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 31 May 2026 14:08:01 +0100 Subject: [PATCH] fix(plugins): scope tool callbacks during materialization --- src/plugins/tools.optional.test.ts | 153 +++++++++++++++++++++++++++++ src/plugins/tools.ts | 83 +++++++++++++++- 2 files changed, 234 insertions(+), 2 deletions(-) diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index 93582bf529e..f0e7daf6f7c 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -40,6 +40,8 @@ let resetPluginRuntimeStateForTest: typeof import("./runtime.js").resetPluginRun let setActivePluginRegistry: typeof import("./runtime.js").setActivePluginRegistry; let clearCurrentPluginMetadataSnapshot: typeof import("./current-plugin-metadata-snapshot.js").clearCurrentPluginMetadataSnapshot; let setCurrentPluginMetadataSnapshot: typeof import("./current-plugin-metadata-snapshot.js").setCurrentPluginMetadataSnapshot; +let getPluginRuntimeGatewayRequestScope: typeof import("./runtime/gateway-request-scope.js").getPluginRuntimeGatewayRequestScope; +let withPluginRuntimeGatewayRequestScope: typeof import("./runtime/gateway-request-scope.js").withPluginRuntimeGatewayRequestScope; function makeTool(name: string) { return { @@ -480,6 +482,8 @@ describe("resolvePluginTools optional tools", () => { resetPluginRuntimeStateForTest, setActivePluginRegistry, } = await import("./runtime.js")); + ({ getPluginRuntimeGatewayRequestScope, withPluginRuntimeGatewayRequestScope } = + await import("./runtime/gateway-request-scope.js")); ({ clearCurrentPluginMetadataSnapshot, setCurrentPluginMetadataSnapshot } = await import("./current-plugin-metadata-snapshot.js")); }); @@ -510,6 +514,155 @@ describe("resolvePluginTools optional tools", () => { vi.useRealTimers(); }); + it("runs plugin tool factories, prepare callbacks, and execute callbacks under the owning plugin scope", async () => { + const context = createContext(); + const observed: Array<{ + phase: "factory" | "prepare" | "execute"; + pluginId?: string; + pluginSource?: string; + }> = []; + + setRegistry( + ["multi", "optional-demo"].map((pluginId) => ({ + pluginId, + optional: false, + source: `/tmp/${pluginId}.js`, + names: [`${pluginId}_tool`], + factory: () => { + const scope = getPluginRuntimeGatewayRequestScope(); + observed.push({ + phase: "factory", + pluginId: scope?.pluginId, + pluginSource: scope?.pluginSource, + }); + return { + name: `${pluginId}_tool`, + description: `${pluginId} tool`, + parameters: { type: "object", properties: {} }, + prepareArguments(args: unknown) { + const prepareScope = getPluginRuntimeGatewayRequestScope(); + observed.push({ + phase: "prepare", + pluginId: prepareScope?.pluginId, + pluginSource: prepareScope?.pluginSource, + }); + return args; + }, + async execute() { + const executeScope = getPluginRuntimeGatewayRequestScope(); + observed.push({ + phase: "execute", + pluginId: executeScope?.pluginId, + pluginSource: executeScope?.pluginSource, + }); + return { content: [{ type: "text", text: pluginId }] }; + }, + }; + }, + })), + ); + + await withPluginRuntimeGatewayRequestScope( + { + pluginId: "outer", + pluginSource: "/tmp/outer.js", + isWebchatConnect: () => false, + }, + async () => { + const tools = resolvePluginTools(createResolveToolsParams({ context })); + expect(tools.map((tool) => tool.name)).toEqual(["multi_tool", "optional-demo_tool"]); + for (const tool of tools) { + await tool.execute(`call-${tool.name}`, tool.prepareArguments?.({}) ?? {}, undefined); + expect(getPluginRuntimeGatewayRequestScope()).toMatchObject({ + pluginId: "outer", + pluginSource: "/tmp/outer.js", + }); + } + }, + ); + + expect(getPluginRuntimeGatewayRequestScope()).toBeUndefined(); + expect(observed).toEqual([ + { phase: "factory", pluginId: "multi", pluginSource: "/tmp/multi.js" }, + { + phase: "factory", + pluginId: "optional-demo", + pluginSource: "/tmp/optional-demo.js", + }, + { phase: "prepare", pluginId: "multi", pluginSource: "/tmp/multi.js" }, + { phase: "execute", pluginId: "multi", pluginSource: "/tmp/multi.js" }, + { + phase: "prepare", + pluginId: "optional-demo", + pluginSource: "/tmp/optional-demo.js", + }, + { + phase: "execute", + pluginId: "optional-demo", + pluginSource: "/tmp/optional-demo.js", + }, + ]); + }); + + it("wraps every array tool callback and restores caller scope after errors", async () => { + const context = createContext(); + const observed: Array<{ name: string; pluginId?: string; pluginSource?: string }> = []; + setRegistry([ + { + pluginId: "multi", + optional: false, + source: "/tmp/multi.js", + names: ["array_first", "array_second"], + factory: () => + ["array_first", "array_second"].map((name) => ({ + name, + description: `${name} tool`, + parameters: { type: "object", properties: {} }, + prepareArguments() { + const scope = getPluginRuntimeGatewayRequestScope(); + observed.push({ name: `${name}:prepare`, pluginId: scope?.pluginId }); + if (name === "array_second") { + throw new Error("bad args"); + } + return {}; + }, + async execute() { + const scope = getPluginRuntimeGatewayRequestScope(); + observed.push({ + name, + pluginId: scope?.pluginId, + pluginSource: scope?.pluginSource, + }); + return { content: [{ type: "text", text: name }] }; + }, + })), + }, + ]); + + await withPluginRuntimeGatewayRequestScope( + { + pluginId: "outer", + pluginSource: "/tmp/outer.js", + isWebchatConnect: () => false, + }, + async () => { + const tools = resolvePluginTools(createResolveToolsParams({ context })); + await tools[0]?.execute("call-first", tools[0].prepareArguments?.({}) ?? {}, undefined); + expect(() => tools[1]?.prepareArguments?.({})).toThrow("bad args"); + expect(getPluginRuntimeGatewayRequestScope()).toMatchObject({ + pluginId: "outer", + pluginSource: "/tmp/outer.js", + }); + }, + ); + + expect(observed).toEqual([ + { name: "array_first:prepare", pluginId: "multi" }, + { name: "array_first", pluginId: "multi", pluginSource: "/tmp/multi.js" }, + { name: "array_second:prepare", pluginId: "multi" }, + ]); + }); + it("does not load plugin-owned tools whose manifest metadata has no available signal", () => { const config = createContext().config; installToolManifestSnapshot({ diff --git a/src/plugins/tools.ts b/src/plugins/tools.ts index b34112dead3..4bad1e23129 100644 --- a/src/plugins/tools.ts +++ b/src/plugins/tools.ts @@ -18,6 +18,7 @@ import type { PluginManifestRecord } from "./manifest-registry.js"; import { hasManifestToolAvailability } from "./manifest-tool-availability.js"; import type { PluginMetadataManifestView } from "./plugin-metadata-snapshot.types.js"; import type { PluginRegistry, PluginToolRegistration } from "./registry-types.js"; +import { withPluginRuntimePluginScope } from "./runtime/gateway-request-scope.js"; import { buildPluginRuntimeLoadOptions, resolvePluginRuntimeLoadContext, @@ -74,6 +75,7 @@ const PLUGIN_TOOL_FACTORY_WARN_FACTORY_MS = 1_000; const PLUGIN_TOOL_FACTORY_SUMMARY_LIMIT = 20; const pluginToolMeta = new WeakMap(); +const scopedPluginTools = new WeakMap>(); export function setPluginToolMeta(tool: AnyAgentTool, meta: PluginToolMeta): void { pluginToolMeta.set(tool, meta); @@ -90,6 +92,83 @@ export function copyPluginToolMeta(source: AnyAgentTool, target: AnyAgentTool): } } +function pluginToolScopeKey(entry: PluginToolRegistration): string { + return JSON.stringify([entry.pluginId, entry.source]); +} + +function runWithPluginToolScope(entry: PluginToolRegistration, run: () => T): T { + return withPluginRuntimePluginScope( + { + pluginId: entry.pluginId, + ...(entry.source ? { pluginSource: entry.source } : {}), + }, + run, + ); +} + +function isAgentTool(value: unknown): value is AnyAgentTool { + return ( + Boolean(value) && + typeof value === "object" && + !Array.isArray(value) && + typeof (value as { execute?: unknown }).execute === "function" + ); +} + +function wrapPluginToolCallbacks(entry: PluginToolRegistration, tool: AnyAgentTool): AnyAgentTool { + const key = pluginToolScopeKey(entry); + const scopedByKey = scopedPluginTools.get(tool); + const cached = scopedByKey?.get(key); + if (cached) { + return cached; + } + + const prepareArguments = tool.prepareArguments; + const wrapped: AnyAgentTool = { + ...tool, + ...(prepareArguments + ? { + prepareArguments(args) { + return runWithPluginToolScope(entry, () => + Reflect.apply(prepareArguments, tool, [args]), + ); + }, + } + : {}), + execute(toolCallId, params, signal, onUpdate) { + return runWithPluginToolScope( + entry, + () => + Reflect.apply(tool.execute, tool, [toolCallId, params, signal, onUpdate]) as ReturnType< + AnyAgentTool["execute"] + >, + ); + }, + }; + + copyPluginToolMeta(tool, wrapped); + const nextScopedByKey = scopedByKey ?? new Map(); + nextScopedByKey.set(key, wrapped); + scopedPluginTools.set(tool, nextScopedByKey); + return wrapped; +} + +function wrapPluginToolFactoryResult( + entry: PluginToolRegistration, + result: PluginToolFactoryResult, +): PluginToolFactoryResult { + if (Array.isArray(result)) { + return result.map((tool) => (isAgentTool(tool) ? wrapPluginToolCallbacks(entry, tool) : tool)); + } + return isAgentTool(result) ? wrapPluginToolCallbacks(entry, result) : result; +} + +function resolvePluginToolFactory(entry: PluginToolRegistration, ctx: OpenClawPluginToolContext) { + return runWithPluginToolScope(entry, () => + wrapPluginToolFactoryResult(entry, entry.factory(ctx)), + ); +} + /** * Builds a collision-proof key for plugin-owned tool metadata lookups. */ @@ -271,7 +350,7 @@ function resolvePluginToolFactoryEntry(params: { const factoryStartedAt = Date.now(); try { - resolved = params.entry.factory(params.ctx); + resolved = resolvePluginToolFactory(params.entry, params.ctx); } catch (err) { failed = true; params.logError(`plugin tool failed (${params.entry.pluginId}): ${String(err)}`); @@ -582,7 +661,7 @@ function createCachedDescriptorPluginTool(params: { const resolveCandidateTool = ( candidate: PluginToolRegistration, ): AnyAgentTool | undefined => { - const resolved = candidate.factory(params.ctx); + const resolved = resolvePluginToolFactory(candidate, params.ctx); const listRaw: unknown[] = Array.isArray(resolved) ? resolved : resolved ? [resolved] : []; for (const toolRaw of listRaw) { const malformedReason = describeMalformedPluginTool(toolRaw);