diff --git a/src/plugins/loader.runtime-registry.test.ts b/src/plugins/loader.runtime-registry.test.ts index e0bb9c8f47d..0e8cc54f176 100644 --- a/src/plugins/loader.runtime-registry.test.ts +++ b/src/plugins/loader.runtime-registry.test.ts @@ -106,6 +106,57 @@ describe("getCompatibleActivePluginRegistry", () => { ).toBeUndefined(); }); + it("reuses an active full registry for compatible tool-discovery loads", () => { + const registry = createEmptyPluginRegistry(); + const loadOptions = { + config: { + plugins: { + allow: ["demo"], + load: { paths: ["/tmp/demo.js"] }, + }, + }, + workspaceDir: "/tmp/workspace-a", + }; + const { cacheKey } = __testing.resolvePluginLoadCacheContext(loadOptions); + setActivePluginRegistry(registry, cacheKey, "default"); + + expect( + __testing.getCompatibleActivePluginRegistry({ + ...loadOptions, + activate: false, + installBundledRuntimeDeps: false, + toolDiscovery: true, + }), + ).toBe(registry); + }); + + it("does not reuse a default-mode active registry for gateway-bindable tool discovery", () => { + const registry = createEmptyPluginRegistry(); + const loadOptions = { + config: { + plugins: { + allow: ["demo"], + load: { paths: ["/tmp/demo.js"] }, + }, + }, + workspaceDir: "/tmp/workspace-a", + }; + const { cacheKey } = __testing.resolvePluginLoadCacheContext(loadOptions); + setActivePluginRegistry(registry, cacheKey, "default"); + + expect( + __testing.getCompatibleActivePluginRegistry({ + ...loadOptions, + activate: false, + installBundledRuntimeDeps: false, + runtimeOptions: { + allowGatewaySubagentBinding: true, + }, + toolDiscovery: true, + }), + ).toBeUndefined(); + }); + it("does not embed activation secrets in the loader cache key", () => { const { cacheKey } = __testing.resolvePluginLoadCacheContext({ config: { diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index f8dfc6edec0..4b9dc843a0f 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -778,6 +778,32 @@ function pluginLoadOptionsMatchCacheKey( return resolvePluginLoadCacheContext(options).cacheKey === expectedCacheKey; } +function pluginToolDiscoveryOptionsMatchActiveCacheKey( + options: PluginLoadOptions, + expectedCacheKey: string, +): boolean { + if (options.toolDiscovery !== true) { + return false; + } + const fullRuntimeOptions = { + ...options, + toolDiscovery: undefined, + }; + if (pluginLoadOptionsMatchCacheKey(fullRuntimeOptions, expectedCacheKey)) { + return true; + } + if (options.activate !== false) { + return false; + } + return pluginLoadOptionsMatchCacheKey( + { + ...fullRuntimeOptions, + activate: true, + }, + expectedCacheKey, + ); +} + type PluginRegistrationPlan = { /** Public compatibility label passed to plugin register(api). */ mode: PluginRegistrationMode; @@ -1031,6 +1057,9 @@ function getCompatibleActivePluginRegistry( return activeRegistry; } } + if (pluginToolDiscoveryOptionsMatchActiveCacheKey(options, activeCacheKey)) { + return activeRegistry; + } if ( loadContext.runtimeSubagentMode === "default" && getActivePluginRuntimeSubagentMode() === "gateway-bindable" @@ -1045,6 +1074,9 @@ function getCompatibleActivePluginRegistry( if (matchesActiveCacheKey(gatewayBindableOptions)) { return activeRegistry; } + if (pluginToolDiscoveryOptionsMatchActiveCacheKey(gatewayBindableOptions, activeCacheKey)) { + return activeRegistry; + } if (!loadContext.shouldActivate) { const activatingGatewayBindableOptions = { ...options, diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index 8c3e79e991d..3b53a11eb8e 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -7,6 +7,7 @@ type MockRegistryToolEntry = { names?: string[]; optional: boolean; source: string; + names: string[]; factory: (ctx: unknown) => unknown; }; @@ -88,6 +89,7 @@ function setMultiToolRegistry() { pluginId: "multi", optional: false, source: "/tmp/multi.js", + names: ["message", "other_tool"], factory: () => [makeTool("message"), makeTool("other_tool")], }, ]); @@ -99,6 +101,7 @@ function createOptionalDemoEntry(): MockRegistryToolEntry { names: ["optional_tool"], optional: true, source: "/tmp/optional-demo.js", + names: ["optional_tool"], factory: () => makeTool("optional_tool"), }; } @@ -255,6 +258,41 @@ describe("resolvePluginTools optional tools", () => { expect(tools).toHaveLength(0); }); + it("does not invoke named optional tool factories without a matching allowlist", () => { + const factory = vi.fn(() => makeTool("optional_tool")); + setRegistry([ + { + pluginId: "optional-demo", + optional: true, + source: "/tmp/optional-demo.js", + names: ["optional_tool"], + factory, + }, + ]); + + expect(resolveOptionalDemoTools()).toHaveLength(0); + expect(resolveOptionalDemoTools(["other_tool"])).toHaveLength(0); + expect(factory).not.toHaveBeenCalled(); + }); + + it("invokes unnamed optional tool factories when a tool allowlist may match the result", () => { + const factory = vi.fn(() => makeTool("optional_tool")); + setRegistry([ + { + pluginId: "optional-demo", + optional: true, + source: "/tmp/optional-demo.js", + names: [], + factory, + }, + ]); + + const tools = resolveOptionalDemoTools(["optional_tool"]); + + expectResolvedToolNames(tools, ["optional_tool"]); + expect(factory).toHaveBeenCalledTimes(1); + }); + it.each([ { name: "allows optional tools by tool name", @@ -281,6 +319,7 @@ describe("resolvePluginTools optional tools", () => { pluginId: "message", optional: false, source: "/tmp/message.js", + names: ["optional_tool"], factory: () => makeTool("optional_tool"), }, ]); @@ -348,6 +387,7 @@ describe("resolvePluginTools optional tools", () => { pluginId: "schema-bug", optional: false, source: "/tmp/schema-bug.js", + names: ["broken_tool", "valid_tool"], factory: () => [createMalformedTool("broken_tool"), makeTool("valid_tool")], }, ]); @@ -447,6 +487,7 @@ describe("resolvePluginTools optional tools", () => { pluginId: "optional-demo", optional: true, source: "/tmp/optional-demo.js", + names: ["optional_tool"], factory: () => createMalformedTool("optional_tool"), }, ]); diff --git a/src/plugins/tools.ts b/src/plugins/tools.ts index acd07480ebd..55dff4b0a4c 100644 --- a/src/plugins/tools.ts +++ b/src/plugins/tools.ts @@ -81,6 +81,24 @@ function isOptionalToolAllowed(params: { return params.allowlist.has("group:plugins"); } +function isOptionalToolEntryPotentiallyAllowed(params: { + names: readonly string[]; + pluginId: string; + allowlist: Set; +}): boolean { + if (params.allowlist.size === 0) { + return false; + } + const pluginKey = normalizeToolName(params.pluginId); + if (params.allowlist.has(pluginKey) || params.allowlist.has("group:plugins")) { + return true; + } + if (params.names.length === 0) { + return true; + } + return params.names.some((name) => params.allowlist.has(normalizeToolName(name))); +} + function isRecord(value: unknown): value is Record { return Boolean(value && typeof value === "object" && !Array.isArray(value)); } @@ -270,6 +288,16 @@ export function resolvePluginTools(params: { continue; } const declaredNames = entry.names ?? []; + if ( + entry.optional && + !isOptionalToolEntryPotentiallyAllowed({ + names: declaredNames, + pluginId: entry.pluginId, + allowlist, + }) + ) { + continue; + } let resolved: AnyAgentTool | AnyAgentTool[] | null | undefined = null; let factoryFailed = false; const factoryStartedAt = Date.now();