fix: cache plugin tool factories by context

This commit is contained in:
Peter Steinberger
2026-05-02 12:19:23 +01:00
parent 335f870cd2
commit 40f2bf3950
4 changed files with 319 additions and 14 deletions

View File

@@ -26,6 +26,7 @@ vi.mock("../config/plugin-auto-enable.js", () => ({
let resolvePluginTools: typeof import("./tools.js").resolvePluginTools;
let buildPluginToolMetadataKey: typeof import("./tools.js").buildPluginToolMetadataKey;
let resetPluginToolFactoryCache: typeof import("./tools.js").resetPluginToolFactoryCache;
let pinActivePluginChannelRegistry: typeof import("./runtime.js").pinActivePluginChannelRegistry;
let resetPluginRuntimeStateForTest: typeof import("./runtime.js").resetPluginRuntimeStateForTest;
let setActivePluginRegistry: typeof import("./runtime.js").setActivePluginRegistry;
@@ -58,6 +59,7 @@ function createContext() {
}
function createResolveToolsParams(params?: {
context?: ReturnType<typeof createContext> & Record<string, unknown>;
toolAllowlist?: readonly string[];
existingToolNames?: Set<string>;
env?: NodeJS.ProcessEnv;
@@ -65,7 +67,7 @@ function createResolveToolsParams(params?: {
allowGatewaySubagentBinding?: boolean;
}) {
return {
context: createContext() as never,
context: (params?.context ?? createContext()) as never,
...(params?.toolAllowlist ? { toolAllowlist: [...params.toolAllowlist] } : {}),
...(params?.existingToolNames ? { existingToolNames: params.existingToolNames } : {}),
...(params?.env ? { env: params.env } : {}),
@@ -360,7 +362,8 @@ function expectConflictingCoreNameResolution(params: {
describe("resolvePluginTools optional tools", () => {
beforeAll(async () => {
({ buildPluginToolMetadataKey, resolvePluginTools } = await import("./tools.js"));
({ buildPluginToolMetadataKey, resetPluginToolFactoryCache, resolvePluginTools } =
await import("./tools.js"));
({ pinActivePluginChannelRegistry, resetPluginRuntimeStateForTest, setActivePluginRegistry } =
await import("./runtime.js"));
({ clearCurrentPluginMetadataSnapshot, setCurrentPluginMetadataSnapshot } =
@@ -380,11 +383,13 @@ describe("resolvePluginTools optional tools", () => {
}));
resetPluginRuntimeStateForTest?.();
clearCurrentPluginMetadataSnapshot?.();
resetPluginToolFactoryCache?.();
});
afterEach(() => {
resetPluginRuntimeStateForTest?.();
clearCurrentPluginMetadataSnapshot?.();
resetPluginToolFactoryCache?.();
setLoggerOverride(null);
loggingState.rawConsole = null;
resetLogger();
@@ -812,6 +817,163 @@ describe("resolvePluginTools optional tools", () => {
expect(warnSpy).not.toHaveBeenCalled();
});
it("caches plugin tool factory results for equivalent request context", () => {
const factory = vi.fn(() => makeTool("cached_tool"));
setRegistry([
{
pluginId: "cache-test",
optional: false,
source: "/tmp/cache-test.js",
names: ["cached_tool"],
factory,
},
]);
const first = resolvePluginTools(createResolveToolsParams({ context: createContext() }));
const second = resolvePluginTools(createResolveToolsParams({ context: createContext() }));
expectResolvedToolNames(first, ["cached_tool"]);
expectResolvedToolNames(second, ["cached_tool"]);
expect(factory).toHaveBeenCalledTimes(1);
expect(second[0]).toBe(first[0]);
});
it("does not reuse plugin tool factory results across sandbox context changes", () => {
const factory = vi.fn((rawCtx: unknown) => {
const ctx = rawCtx as { sandboxed?: boolean };
return ctx.sandboxed ? null : makeTool("sandbox_sensitive_tool");
});
setRegistry([
{
pluginId: "sandbox-sensitive",
optional: false,
source: "/tmp/sandbox-sensitive.js",
names: ["sandbox_sensitive_tool"],
factory,
},
]);
const hostTools = resolvePluginTools(
createResolveToolsParams({
context: { ...createContext(), sandboxed: false },
}),
);
const sandboxedTools = resolvePluginTools(
createResolveToolsParams({
context: { ...createContext(), sandboxed: true },
}),
);
expectResolvedToolNames(hostTools, ["sandbox_sensitive_tool"]);
expect(sandboxedTools).toEqual([]);
expect(factory).toHaveBeenCalledTimes(2);
});
it("does not reuse plugin tool factory results across runtime config changes", () => {
const firstRuntimeConfig = {
...createContext().config,
plugins: { ...createContext().config.plugins, allow: ["runtime_sensitive_tool"] },
};
const secondRuntimeConfig = {
...createContext().config,
plugins: { ...createContext().config.plugins, allow: ["runtime_sensitive_next_tool"] },
};
const factory = vi.fn((rawCtx: unknown) => {
const ctx = rawCtx as { runtimeConfig?: { plugins?: { allow?: string[] } } };
return makeTool(ctx.runtimeConfig?.plugins?.allow?.[0] ?? "runtime_missing_tool");
});
setRegistry([
{
pluginId: "runtime-sensitive",
optional: false,
source: "/tmp/runtime-sensitive.js",
names: ["runtime_sensitive_tool", "runtime_sensitive_next_tool"],
factory,
},
]);
const first = resolvePluginTools(
createResolveToolsParams({
context: { ...createContext(), runtimeConfig: firstRuntimeConfig as never },
}),
);
const second = resolvePluginTools(
createResolveToolsParams({
context: { ...createContext(), runtimeConfig: secondRuntimeConfig as never },
}),
);
expectResolvedToolNames(first, ["runtime_sensitive_tool"]);
expectResolvedToolNames(second, ["runtime_sensitive_next_tool"]);
expect(factory).toHaveBeenCalledTimes(2);
});
it("reuses plugin tool factory results when only runtime config getter identity changes", () => {
const runtimeConfig = {
...createContext().config,
plugins: { ...createContext().config.plugins, allow: ["getter_sensitive_tool"] },
};
const factory = vi.fn((rawCtx: unknown) => {
const ctx = rawCtx as { getRuntimeConfig?: () => { plugins?: { allow?: string[] } } };
return makeTool(ctx.getRuntimeConfig?.()?.plugins?.allow?.[0] ?? "getter_missing_tool");
});
setRegistry([
{
pluginId: "getter-sensitive",
optional: false,
source: "/tmp/getter-sensitive.js",
names: ["getter_sensitive_tool"],
factory,
},
]);
const context = createContext();
const first = resolvePluginTools(
createResolveToolsParams({
context: { ...context, getRuntimeConfig: () => runtimeConfig as never },
}),
);
const second = resolvePluginTools(
createResolveToolsParams({
context: { ...context, getRuntimeConfig: () => runtimeConfig as never },
}),
);
expectResolvedToolNames(first, ["getter_sensitive_tool"]);
expectResolvedToolNames(second, ["getter_sensitive_tool"]);
expect(factory).toHaveBeenCalledTimes(1);
});
it("reads live runtime config once per plugin tool resolution for cache keys", () => {
const runtimeConfig = createContext().config;
const getRuntimeConfig = vi.fn(() => runtimeConfig);
setRegistry([
{
pluginId: "getter-a",
optional: false,
source: "/tmp/getter-a.js",
names: ["getter_a_tool"],
factory: () => makeTool("getter_a_tool"),
},
{
pluginId: "getter-b",
optional: false,
source: "/tmp/getter-b.js",
names: ["getter_b_tool"],
factory: () => makeTool("getter_b_tool"),
},
]);
const tools = resolvePluginTools(
createResolveToolsParams({
context: { ...createContext(), getRuntimeConfig: getRuntimeConfig as never },
}),
);
expectResolvedToolNames(tools, ["getter_a_tool", "getter_b_tool"]);
expect(getRuntimeConfig).toHaveBeenCalledTimes(1);
});
it("skips factory-returned tools outside the manifest tool contract", () => {
const registry = setRegistry([
{

View File

@@ -1,5 +1,6 @@
import { normalizeToolName } from "../agents/tool-policy.js";
import type { AnyAgentTool } from "../agents/tools/common.js";
import { resolveRuntimeConfigCacheKey } from "../config/runtime-snapshot.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
import { applyTestPluginDefaults, normalizePluginsConfig } from "./config-state.js";
import { resolveRuntimePluginRegistry, type PluginLoadOptions } from "./loader.js";
@@ -19,7 +20,7 @@ import {
resolvePluginRuntimeLoadContext,
} from "./runtime/load-context.js";
import { findUndeclaredPluginToolNames } from "./tool-contracts.js";
import type { OpenClawPluginToolContext } from "./types.js";
import type { OpenClawPluginToolContext, OpenClawPluginToolFactory } from "./types.js";
export type PluginToolMeta = {
pluginId: string;
@@ -42,9 +43,106 @@ const log = createSubsystemLogger("plugins/tools");
const PLUGIN_TOOL_FACTORY_WARN_TOTAL_MS = 5_000;
const PLUGIN_TOOL_FACTORY_WARN_FACTORY_MS = 1_000;
const PLUGIN_TOOL_FACTORY_SUMMARY_LIMIT = 20;
const PLUGIN_TOOL_FACTORY_CACHE_LIMIT_PER_FACTORY = 64;
type PluginToolFactoryResult = AnyAgentTool | AnyAgentTool[] | null | undefined;
let pluginToolFactoryCache = new WeakMap<
OpenClawPluginToolFactory,
Map<string, PluginToolFactoryResult>
>();
let pluginToolFactoryCacheObjectIds = new WeakMap<object, number>();
let nextPluginToolFactoryCacheObjectId = 1;
const pluginToolMeta = new WeakMap<AnyAgentTool, PluginToolMeta>();
export function resetPluginToolFactoryCache(): void {
pluginToolFactoryCache = new WeakMap();
pluginToolFactoryCacheObjectIds = new WeakMap();
nextPluginToolFactoryCacheObjectId = 1;
}
function getPluginToolFactoryCacheObjectId(value: object | null | undefined): number | null {
if (!value) {
return null;
}
const existing = pluginToolFactoryCacheObjectIds.get(value);
if (existing !== undefined) {
return existing;
}
const next = nextPluginToolFactoryCacheObjectId++;
pluginToolFactoryCacheObjectIds.set(value, next);
return next;
}
function getPluginToolFactoryConfigCacheKey(
value: PluginLoadOptions["config"] | null | undefined,
): string | number | null {
if (!value) {
return null;
}
try {
return resolveRuntimeConfigCacheKey(value);
} catch {
return getPluginToolFactoryCacheObjectId(value);
}
}
function buildPluginToolFactoryCacheKey(params: {
ctx: OpenClawPluginToolContext;
currentRuntimeConfig?: PluginLoadOptions["config"] | null;
}): string {
const { ctx } = params;
return JSON.stringify({
config: getPluginToolFactoryConfigCacheKey(ctx.config),
runtimeConfig: getPluginToolFactoryConfigCacheKey(ctx.runtimeConfig),
currentRuntimeConfig: getPluginToolFactoryConfigCacheKey(params.currentRuntimeConfig),
fsPolicy: ctx.fsPolicy ?? null,
workspaceDir: ctx.workspaceDir ?? null,
agentDir: ctx.agentDir ?? null,
agentId: ctx.agentId ?? null,
sessionKey: ctx.sessionKey ?? null,
sessionId: ctx.sessionId ?? null,
browser: ctx.browser ?? null,
messageChannel: ctx.messageChannel ?? null,
agentAccountId: ctx.agentAccountId ?? null,
deliveryContext: ctx.deliveryContext ?? null,
requesterSenderId: ctx.requesterSenderId ?? null,
senderIsOwner: ctx.senderIsOwner ?? null,
sandboxed: ctx.sandboxed ?? null,
});
}
function readCachedPluginToolFactoryResult(params: {
factory: OpenClawPluginToolFactory;
cacheKey: string;
}): { hit: boolean; result: PluginToolFactoryResult } {
const cache = pluginToolFactoryCache.get(params.factory);
if (!cache || !cache.has(params.cacheKey)) {
return { hit: false, result: undefined };
}
return { hit: true, result: cache.get(params.cacheKey) };
}
function writeCachedPluginToolFactoryResult(params: {
factory: OpenClawPluginToolFactory;
cacheKey: string;
result: PluginToolFactoryResult;
}): void {
let cache = pluginToolFactoryCache.get(params.factory);
if (!cache) {
cache = new Map();
pluginToolFactoryCache.set(params.factory, cache);
}
if (!cache.has(params.cacheKey) && cache.size >= PLUGIN_TOOL_FACTORY_CACHE_LIMIT_PER_FACTORY) {
const oldestKey = cache.keys().next().value;
if (oldestKey !== undefined) {
cache.delete(oldestKey);
}
}
cache.set(params.cacheKey, params.result);
}
export function setPluginToolMeta(tool: AnyAgentTool, meta: PluginToolMeta): void {
pluginToolMeta.set(tool, meta);
}
@@ -394,6 +492,15 @@ export function resolvePluginTools(params: {
const blockedPlugins = new Set<string>();
const factoryTimingStartedAt = Date.now();
const factoryTimings: PluginToolFactoryTiming[] = [];
let currentRuntimeConfigForFactoryCache: PluginLoadOptions["config"] | null | undefined =
params.context.runtimeConfig;
if (currentRuntimeConfigForFactoryCache === undefined && params.context.getRuntimeConfig) {
try {
currentRuntimeConfigForFactoryCache = params.context.getRuntimeConfig();
} catch {
currentRuntimeConfigForFactoryCache = null;
}
}
for (const entry of registry.tools) {
if (!scopedPluginIds.has(entry.pluginId)) {
@@ -428,26 +535,55 @@ export function resolvePluginTools(params: {
) {
continue;
}
let resolved: AnyAgentTool | AnyAgentTool[] | null | undefined = null;
let resolved: PluginToolFactoryResult = null;
let factoryFailed = false;
const factoryStartedAt = Date.now();
try {
resolved = entry.factory(params.context);
} catch (err) {
factoryFailed = true;
context.logger.error(`plugin tool failed (${entry.pluginId}): ${String(err)}`);
} finally {
const factoryEndedAt = Date.now();
const result = describePluginToolFactoryResult(resolved, factoryFailed);
const factoryCacheKey = buildPluginToolFactoryCacheKey({
ctx: params.context,
currentRuntimeConfig: currentRuntimeConfigForFactoryCache,
});
const cached = readCachedPluginToolFactoryResult({
factory: entry.factory,
cacheKey: factoryCacheKey,
});
if (cached.hit) {
resolved = cached.result;
const result = describePluginToolFactoryResult(resolved, false);
factoryTimings.push({
pluginId: entry.pluginId,
names: declaredNames,
durationMs: toElapsedMs(factoryEndedAt - factoryStartedAt),
elapsedMs: toElapsedMs(factoryEndedAt - factoryTimingStartedAt),
durationMs: 0,
elapsedMs: toElapsedMs(Date.now() - factoryTimingStartedAt),
result: result.result,
resultCount: result.resultCount,
optional: entry.optional,
});
} else {
try {
resolved = entry.factory(params.context);
} catch (err) {
factoryFailed = true;
context.logger.error(`plugin tool failed (${entry.pluginId}): ${String(err)}`);
} finally {
const factoryEndedAt = Date.now();
const result = describePluginToolFactoryResult(resolved, factoryFailed);
factoryTimings.push({
pluginId: entry.pluginId,
names: declaredNames,
durationMs: toElapsedMs(factoryEndedAt - factoryStartedAt),
elapsedMs: toElapsedMs(factoryEndedAt - factoryTimingStartedAt),
result: result.result,
resultCount: result.resultCount,
optional: entry.optional,
});
if (!factoryFailed) {
writeCachedPluginToolFactoryResult({
factory: entry.factory,
cacheKey: factoryCacheKey,
result: resolved,
});
}
}
}
if (factoryFailed) {
continue;