From baadd74b6bb515f4a20aadf10ccadbec156e4f28 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 4 May 2026 00:40:53 +0100 Subject: [PATCH] fix(plugins): narrow optional tool cold loads --- CHANGELOG.md | 2 +- docs/plugins/building-plugins.md | 8 + extensions/diffs/openclaw.plugin.json | 5 + extensions/llm-task/openclaw.plugin.json | 5 + extensions/lobster/openclaw.plugin.json | 5 + src/plugins/manifest-registry.test.ts | 2 + src/plugins/manifest-tool-availability.ts | 6 +- src/plugins/manifest.ts | 59 +++++-- src/plugins/tools.optional.test.ts | 206 ++++++++++++++++++++-- src/plugins/tools.ts | 73 +++++--- 10 files changed, 313 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02bc7dd9dd4..267849c5c54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,7 +72,7 @@ Docs: https://docs.openclaw.ai - Doctor/plugins: reset stale `plugins.slots.memory` and `plugins.slots.contextEngine` references during `doctor --fix`, so cleanup of missing plugin config does not leave unrecoverable slot owners behind. Fixes #76550 and #76551. Thanks @vincentkoc. - Docs/WhatsApp: merge the duplicate top-level `web` objects in the gateway channel config example so copy-pasted WhatsApp config keeps both `web.whatsapp` and reconnect settings. Fixes #76619. Thanks @WadydX. - Plugins/Anthropic: expose Claude thinking profiles from the bundled provider-policy artifact so non-runtime callers keep Opus 4.7 `adaptive`, `xhigh`, and `max` instead of downgrading to `high`. Fixes #76779. Thanks @tomascupr and @iAbhi001. -- Plugins/tools: honor `tools.alsoAllow` as an optional plugin tool discovery hint without treating its internal allow-all default as permission to load every optional plugin tool. Fixes #76616. +- Plugins/tools: honor `tools.alsoAllow` as an optional plugin tool discovery hint without treating its internal allow-all default as permission to load every manifest-marked optional plugin tool. Fixes #76616. - Discord/native commands: skip slash-command registration and cleanup REST calls when `channels.discord.commands.native=false`, letting low-power gateways start without waiting on disabled native-command lifecycle requests. Fixes #76202. Thanks @vincentkoc. - CLI/plugins: reject unowned command roots such as `openclaw foo` before managed proxy startup and full plugin CLI runtime loading while preserving manifest-owned and CLI-metadata-owned plugin commands. Fixes #75287. Thanks @neilofneils404. - CLI/message: skip local configured-channel plugin preload for explicit gateway-owned message actions, letting normalized CLI delivery delegate to the gateway without initializing channel runtime in the short-lived CLI process. Fixes #75477. diff --git a/docs/plugins/building-plugins.md b/docs/plugins/building-plugins.md index 32ff2b4252e..0cd7b646795 100644 --- a/docs/plugins/building-plugins.md +++ b/docs/plugins/building-plugins.md @@ -252,6 +252,11 @@ plugin manifest: { "contracts": { "tools": ["my_tool", "workflow_tool"] + }, + "toolMetadata": { + "workflow_tool": { + "optional": true + } } } ``` @@ -260,6 +265,9 @@ OpenClaw captures and caches the validated descriptor from the registered tool, so plugins do not duplicate `description` or schema data in the manifest. The manifest contract only declares ownership and discovery; execution still calls the live registered tool implementation. +Set `toolMetadata..optional: true` for tools registered with +`api.registerTool(..., { optional: true })` so OpenClaw can avoid loading that +plugin runtime until the tool is explicitly allowlisted. Users enable optional tools in config: diff --git a/extensions/diffs/openclaw.plugin.json b/extensions/diffs/openclaw.plugin.json index 5ead0d91509..f02ecf07d74 100644 --- a/extensions/diffs/openclaw.plugin.json +++ b/extensions/diffs/openclaw.plugin.json @@ -8,6 +8,11 @@ "contracts": { "tools": ["diffs"] }, + "toolMetadata": { + "diffs": { + "optional": true + } + }, "skills": ["./skills"], "uiHints": { "viewerBaseUrl": { diff --git a/extensions/llm-task/openclaw.plugin.json b/extensions/llm-task/openclaw.plugin.json index d9418559b4a..45e0dc09d57 100644 --- a/extensions/llm-task/openclaw.plugin.json +++ b/extensions/llm-task/openclaw.plugin.json @@ -8,6 +8,11 @@ "contracts": { "tools": ["llm-task"] }, + "toolMetadata": { + "llm-task": { + "optional": true + } + }, "configSchema": { "type": "object", "additionalProperties": false, diff --git a/extensions/lobster/openclaw.plugin.json b/extensions/lobster/openclaw.plugin.json index 4f53ac619e3..44a13d6b01e 100644 --- a/extensions/lobster/openclaw.plugin.json +++ b/extensions/lobster/openclaw.plugin.json @@ -8,6 +8,11 @@ "contracts": { "tools": ["lobster"] }, + "toolMetadata": { + "lobster": { + "optional": true + } + }, "configSchema": { "type": "object", "additionalProperties": false, diff --git a/src/plugins/manifest-registry.test.ts b/src/plugins/manifest-registry.test.ts index ef0519da8ff..50206e909e4 100644 --- a/src/plugins/manifest-registry.test.ts +++ b/src/plugins/manifest-registry.test.ts @@ -1382,6 +1382,7 @@ describe("loadPluginManifestRegistry", () => { }, toolMetadata: { image_generate: { + optional: true, authSignals: [ { provider: "openai-codex", @@ -1450,6 +1451,7 @@ describe("loadPluginManifestRegistry", () => { }); expect(registry.plugins[0]?.toolMetadata).toEqual({ image_generate: { + optional: true, authSignals: [ { provider: "openai-codex", diff --git a/src/plugins/manifest-tool-availability.ts b/src/plugins/manifest-tool-availability.ts index 96e45561589..d48bac3cf63 100644 --- a/src/plugins/manifest-tool-availability.ts +++ b/src/plugins/manifest-tool-availability.ts @@ -215,6 +215,10 @@ function toolMetadataPasses(params: { env: NodeJS.ProcessEnv; hasAuthForProvider?: (providerId: string) => boolean; }): boolean { + const authSignals = listToolAuthSignals(params.metadata); + if (!params.metadata.configSignals?.length && authSignals.length === 0) { + return true; + } if ( params.metadata.configSignals?.some((signal) => manifestConfigSignalPasses({ @@ -226,7 +230,7 @@ function toolMetadataPasses(params: { ) { return true; } - for (const signal of listToolAuthSignals(params.metadata)) { + for (const signal of authSignals) { if ( !manifestProviderBaseUrlGuardPasses({ config: params.config, diff --git a/src/plugins/manifest.ts b/src/plugins/manifest.ts index 6992b6863f2..b64f715a9e7 100644 --- a/src/plugins/manifest.ts +++ b/src/plugins/manifest.ts @@ -458,7 +458,9 @@ export type PluginManifestCapabilityProviderMetadata = { configSignals?: PluginManifestCapabilityProviderConfigSignal[]; }; -export type PluginManifestToolMetadata = PluginManifestCapabilityProviderMetadata; +export type PluginManifestToolMetadata = PluginManifestCapabilityProviderMetadata & { + optional?: boolean; +}; export type PluginManifestProviderAuthChoice = { /** Provider id owned by this manifest entry. */ @@ -715,6 +717,22 @@ function normalizeCapabilityProviderConfigSignals( return signals.length > 0 ? signals : undefined; } +function normalizeCapabilityProviderMetadataEntry( + rawMetadata: Record, +): PluginManifestCapabilityProviderMetadata | undefined { + const aliases = normalizeTrimmedStringList(rawMetadata.aliases); + const authProviders = normalizeTrimmedStringList(rawMetadata.authProviders); + const authSignals = normalizeCapabilityProviderAuthSignals(rawMetadata.authSignals); + const configSignals = normalizeCapabilityProviderConfigSignals(rawMetadata.configSignals); + const metadata = { + ...(aliases.length > 0 ? { aliases } : {}), + ...(authProviders.length > 0 ? { authProviders } : {}), + ...(authSignals ? { authSignals } : {}), + ...(configSignals ? { configSignals } : {}), + } satisfies PluginManifestCapabilityProviderMetadata; + return Object.keys(metadata).length > 0 ? metadata : undefined; +} + function normalizeCapabilityProviderMetadata( value: unknown, ): Record | undefined { @@ -727,23 +745,38 @@ function normalizeCapabilityProviderMetadata( if (!providerId || isBlockedObjectKey(providerId) || !isRecord(rawMetadata)) { continue; } - const aliases = normalizeTrimmedStringList(rawMetadata.aliases); - const authProviders = normalizeTrimmedStringList(rawMetadata.authProviders); - const authSignals = normalizeCapabilityProviderAuthSignals(rawMetadata.authSignals); - const configSignals = normalizeCapabilityProviderConfigSignals(rawMetadata.configSignals); - const metadata = { - ...(aliases.length > 0 ? { aliases } : {}), - ...(authProviders.length > 0 ? { authProviders } : {}), - ...(authSignals ? { authSignals } : {}), - ...(configSignals ? { configSignals } : {}), - } satisfies PluginManifestCapabilityProviderMetadata; - if (Object.keys(metadata).length > 0) { + const metadata = normalizeCapabilityProviderMetadataEntry(rawMetadata); + if (metadata) { normalized[providerId] = metadata; } } return Object.keys(normalized).length > 0 ? normalized : undefined; } +function normalizePluginToolMetadata( + value: unknown, +): Record | undefined { + if (!isRecord(value)) { + return undefined; + } + const normalized: Record = Object.create(null); + for (const [rawToolName, rawMetadata] of Object.entries(value)) { + const toolName = normalizeOptionalString(rawToolName) ?? ""; + if (!toolName || isBlockedObjectKey(toolName) || !isRecord(rawMetadata)) { + continue; + } + const providerMetadata = normalizeCapabilityProviderMetadataEntry(rawMetadata); + const metadata = { + ...providerMetadata, + ...(rawMetadata.optional === true ? { optional: true } : {}), + } satisfies PluginManifestToolMetadata; + if (Object.keys(metadata).length > 0) { + normalized[toolName] = metadata; + } + } + return Object.keys(normalized).length > 0 ? normalized : undefined; +} + function normalizeManifestContracts(value: unknown): PluginManifestContracts | undefined { if (!isRecord(value)) { return undefined; @@ -1596,7 +1629,7 @@ export function loadPluginManifest( const musicGenerationProviderMetadata = normalizeCapabilityProviderMetadata( raw.musicGenerationProviderMetadata, ); - const toolMetadata = normalizeCapabilityProviderMetadata(raw.toolMetadata); + const toolMetadata = normalizePluginToolMetadata(raw.toolMetadata); const configContracts = normalizeManifestConfigContracts(raw.configContracts); const channelConfigs = normalizeChannelConfigs(raw.channelConfigs); diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index a8ebf3182cd..cdcb0656deb 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -112,6 +112,13 @@ function setRegistry(entries: MockRegistryToolEntry[]) { contracts: { tools: entry.declaredNames ?? entry.names, }, + ...(entry.optional + ? { + toolMetadata: Object.fromEntries( + (entry.declaredNames ?? entry.names).map((name) => [name, { optional: true }]), + ), + } + : {}), })) .filter((plugin) => plugin.contracts.tools.length > 0), }); @@ -363,6 +370,14 @@ function expectLoaderCall(overrides: Record) { expect(loadOpenClawPluginsMock).not.toHaveBeenCalled(); } +function expectLoaderSelectedOnlyPluginIds(expectedPluginIds: readonly string[]) { + const selectedPluginIds = loadOpenClawPluginsMock.mock.calls.map( + ([params]) => (params as { onlyPluginIds?: string[] }).onlyPluginIds, + ); + expect(selectedPluginIds.length).toBeGreaterThan(0); + expect(selectedPluginIds).toEqual(selectedPluginIds.map(() => expectedPluginIds)); +} + function expectSingleDiagnosticMessage( diagnostics: Array<{ message: string }>, messageFragment: string, @@ -500,13 +515,7 @@ describe("resolvePluginTools optional tools", () => { ); expectResolvedToolNames(tools, ["optional_tool"]); - expect(loadOpenClawPluginsMock).toHaveBeenCalledWith( - expect.objectContaining({ - activate: false, - onlyPluginIds: ["optional-demo"], - toolDiscovery: true, - }), - ); + expectLoaderSelectedOnlyPluginIds(["optional-demo"]); }); it("auto-loads cold registry for path-based config-origin plugins without pre-warming (#76598)", () => { @@ -550,13 +559,7 @@ describe("resolvePluginTools optional tools", () => { ); expectResolvedToolNames(tools, ["optional_tool"]); - expect(loadOpenClawPluginsMock).toHaveBeenCalledWith( - expect.objectContaining({ - activate: false, - onlyPluginIds: ["optional-demo"], - toolDiscovery: true, - }), - ); + expectLoaderSelectedOnlyPluginIds(["optional-demo"]); }); it("does not reuse a partial active registry for wildcard-selected plugin tools", () => { @@ -593,6 +596,11 @@ describe("resolvePluginTools optional tools", () => { contracts: { tools: ["optional_tool"], }, + toolMetadata: { + optional_tool: { + optional: true, + }, + }, }, ], }); @@ -966,6 +974,176 @@ describe("resolvePluginTools optional tools", () => { expectResolvedToolNames(tools, ["other_tool", "optional_tool"]); }); + it("cold-loads default plugin tools when alsoAllow opts into optional tools", () => { + const context = createContext(); + const config = context.config; + const defaultEntry: MockRegistryToolEntry = { + pluginId: "multi", + optional: false, + source: "/tmp/multi.js", + names: ["other_tool"], + declaredNames: ["other_tool"], + factory: () => makeTool("other_tool"), + }; + loadOpenClawPluginsMock.mockReturnValue( + createToolRegistry([defaultEntry, createOptionalDemoEntry()]), + ); + installToolManifestSnapshots({ + config, + plugins: [ + { + id: "multi", + origin: "bundled", + enabledByDefault: true, + channels: [], + providers: [], + contracts: { + tools: ["other_tool"], + }, + }, + { + id: "optional-demo", + origin: "bundled", + enabledByDefault: true, + channels: [], + providers: [], + contracts: { + tools: ["optional_tool"], + }, + }, + ], + }); + + const tools = resolvePluginTools( + createResolveToolsParams({ + context, + toolAllowlist: [DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY, "optional_tool"], + }), + ); + + expectResolvedToolNames(tools, ["other_tool", "optional_tool"]); + expectLoaderSelectedOnlyPluginIds(["multi", "optional-demo"]); + }); + + it("does not cold-load unrelated manifest-optional plugins when alsoAllow opts into one optional tool", () => { + const context = createContext(); + const config = context.config; + const explicitOptionalEntry = createOptionalDemoEntry(); + loadOpenClawPluginsMock.mockReturnValue(createToolRegistry([explicitOptionalEntry])); + installToolManifestSnapshots({ + config, + plugins: [ + { + id: "optional-demo", + origin: "bundled", + enabledByDefault: true, + channels: [], + providers: [], + contracts: { + tools: ["optional_tool"], + }, + toolMetadata: { + optional_tool: { + optional: true, + }, + }, + }, + { + id: "unrelated-optional", + origin: "bundled", + enabledByDefault: true, + channels: [], + providers: [], + contracts: { + tools: ["unrelated_optional_tool"], + }, + toolMetadata: { + unrelated_optional_tool: { + optional: true, + }, + }, + }, + ], + }); + + const tools = resolvePluginTools( + createResolveToolsParams({ + context, + toolAllowlist: [DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY, "optional_tool"], + }), + ); + + expectResolvedToolNames(tools, ["optional_tool"]); + expectLoaderSelectedOnlyPluginIds(["optional-demo"]); + }); + + it("does not materialize manifest-unavailable default tools from warm registries under alsoAllow", () => { + const config = createContext().config; + installToolManifestSnapshots({ + config, + env: {}, + plugins: [ + createXaiToolManifest(), + { + id: "optional-demo", + origin: "bundled", + enabledByDefault: true, + channels: [], + providers: [], + contracts: { + tools: ["optional_tool"], + }, + toolMetadata: { + optional_tool: { + optional: true, + }, + }, + }, + ], + }); + const unavailableFactory = vi.fn(() => makeTool("x_search")); + const optionalFactory = vi.fn(() => makeTool("optional_tool")); + setActivePluginRegistry( + createToolRegistry([ + { + pluginId: "xai", + optional: false, + source: "/tmp/xai.js", + names: ["x_search"], + declaredNames: ["x_search"], + factory: unavailableFactory, + }, + { + pluginId: "optional-demo", + optional: true, + source: "/tmp/optional-demo.js", + names: ["optional_tool"], + declaredNames: ["optional_tool"], + factory: optionalFactory, + }, + ]) as never, + "test-tool-registry", + "gateway-bindable", + "/tmp", + ); + + const tools = resolvePluginTools( + createResolveToolsParams({ + context: { + ...createContext(), + config, + }, + env: {}, + toolAllowlist: [DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY, "optional_tool"], + }), + ); + + expectResolvedToolNames(tools, ["optional_tool"]); + expect(optionalFactory).toHaveBeenCalledTimes(1); + expect(unavailableFactory).not.toHaveBeenCalled(); + expect(loadOpenClawPluginsMock).not.toHaveBeenCalled(); + }); + it("rejects plugin id collisions with core tool names", () => { const registry = setRegistry([ { diff --git a/src/plugins/tools.ts b/src/plugins/tools.ts index bd634b6d7c8..a8e89cb0e87 100644 --- a/src/plugins/tools.ts +++ b/src/plugins/tools.ts @@ -90,6 +90,10 @@ function allowlistIncludesDefaultPluginTools(allowlist: Set): boolean { return allowlist.size === 0 || allowlist.has(DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY); } +function isManifestToolOptional(plugin: PluginManifestRecord, toolName: string): boolean { + return plugin.toolMetadata?.[toolName]?.optional === true; +} + function isOptionalToolAllowed(params: { toolName: string; pluginId: string; @@ -299,43 +303,50 @@ function pluginToolNamesMatchAllowlist(params: { return isOptionalToolEntryPotentiallyAllowed(params); } -function manifestToolContractMatchesAllowlist(params: { - toolNames: readonly string[]; - pluginId: string; - allowlist: Set; -}): boolean { - if (params.toolNames.length === 0) { - return false; - } - if (allowlistIncludesDefaultPluginTools(params.allowlist)) { - return true; - } - if (params.allowlist.has("*") || params.allowlist.has("group:plugins")) { - return true; - } - const pluginKey = normalizeToolName(params.pluginId); - if (params.allowlist.has(pluginKey)) { - return true; - } - return params.toolNames.some((name) => params.allowlist.has(normalizeToolName(name))); -} - -function listManifestToolNamesForAvailability(params: { +function listManifestToolNamesForAllowlist(params: { + plugin: PluginManifestRecord; toolNames: readonly string[]; pluginId: string; allowlist: Set; }): string[] { - if ( - allowlistIncludesDefaultPluginTools(params.allowlist) || - params.allowlist.has("*") || - params.allowlist.has("group:plugins") - ) { + if (params.toolNames.length === 0) { + return []; + } + if (params.allowlist.has("*") || params.allowlist.has("group:plugins")) { return [...params.toolNames]; } - if (params.allowlist.has(normalizeToolName(params.pluginId))) { + const pluginKey = normalizeToolName(params.pluginId); + if (params.allowlist.has(pluginKey)) { return [...params.toolNames]; } - return params.toolNames.filter((name) => params.allowlist.has(normalizeToolName(name))); + const matchedToolNames = params.toolNames.filter((name) => + params.allowlist.has(normalizeToolName(name)), + ); + if (!allowlistIncludesDefaultPluginTools(params.allowlist)) { + return matchedToolNames; + } + const defaultToolNames = params.toolNames.filter( + (name) => !isManifestToolOptional(params.plugin, name), + ); + return [...new Set([...defaultToolNames, ...matchedToolNames])]; +} + +function manifestToolContractMatchesAllowlist(params: { + plugin: PluginManifestRecord; + toolNames: readonly string[]; + pluginId: string; + allowlist: Set; +}): boolean { + return listManifestToolNamesForAllowlist(params).length > 0; +} + +function listManifestToolNamesForAvailability(params: { + plugin: PluginManifestRecord; + toolNames: readonly string[]; + pluginId: string; + allowlist: Set; +}): string[] { + return listManifestToolNamesForAllowlist(params); } function resolvePluginToolRuntimePluginIds(params: { @@ -376,6 +387,7 @@ function resolvePluginToolRuntimePluginIds(params: { const toolNames = plugin.contracts?.tools ?? []; if ( manifestToolContractMatchesAllowlist({ + plugin, toolNames, pluginId: plugin.id, allowlist, @@ -384,6 +396,7 @@ function resolvePluginToolRuntimePluginIds(params: { plugin, toolNames: listManifestToolNamesForAvailability({ toolNames, + plugin, pluginId: plugin.id, allowlist, }), @@ -534,6 +547,7 @@ function resolveCachedPluginTools(params: { } const contractToolNames = plugin.contracts?.tools ?? []; const availableToolNames = listManifestToolNamesForAvailability({ + plugin, toolNames: contractToolNames, pluginId: plugin.id, allowlist: params.allowlist, @@ -1011,6 +1025,7 @@ export function resolvePluginTools(params: { continue; } const availableToolNames = listManifestToolNamesForAvailability({ + plugin: manifestPlugin, toolNames: manifestPlugin.contracts?.tools ?? [], pluginId, allowlist,