From 443f7035a2e5e31cab659de6b4927978eda5e3d8 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 3 May 2026 17:10:27 -0700 Subject: [PATCH] fix(plugins): filter unavailable optional tools --- CHANGELOG.md | 1 + src/plugins/tools.optional.test.ts | 68 +++++++++++++++++++++++ src/plugins/tools.ts | 86 +++++++++++++++++++++++++----- 3 files changed, 141 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fe66c3b5ee..7b55bc066d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ Docs: https://docs.openclaw.ai - Channels/WhatsApp: allow `@whiskeysockets/libsignal-node` in `onlyBuiltDependencies` so pnpm v9+ `blockExoticSubdeps` no longer rejects the baileys git-tarball subdep and silences all inbound agent replies. Fixes #76539. Thanks @ottodeng and @vincentkoc. - Gateway/systemd: preserve operator-added secrets in the Gateway env file across re-stage while clearing OpenClaw-managed keys (such as `OPENCLAW_GATEWAY_TOKEN`) so a fresh staging value is never shadowed by a stale env-file copy; operator secrets are also retained when the state-dir `.env` is empty. Fixes #76860. Thanks @hclsys. +- Plugin tools: keep auth-unavailable optional tools hidden even when another default tool from the same plugin is available and `tools.alsoAllow` names the optional tool. Thanks @vincentkoc. - Realtime transcription: report socket closes before provider readiness as closed-before-ready failures instead of mislabeling them as connection timeouts for OpenAI, xAI, and Deepgram streaming transcription. Thanks @vincentkoc. - OpenAI/Google Meet: fail realtime voice connection attempts when the socket closes before `session.updated`, avoiding stuck Meet joins waiting on a bridge that never became ready. Thanks @vincentkoc. - QA/cache: require the full `CACHE-OK ` marker before live cache probes stop retrying, so suffix-only prose cannot hide a broken probe response. Thanks @vincentkoc. diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index cdcb0656deb..55c2306b753 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -1144,6 +1144,74 @@ describe("resolvePluginTools optional tools", () => { expect(loadOpenClawPluginsMock).not.toHaveBeenCalled(); }); + it("does not materialize manifest-unavailable optional sibling tools under alsoAllow", () => { + const config = createContext().config; + installToolManifestSnapshot({ + config, + env: {}, + plugin: { + id: "multi", + origin: "bundled", + enabledByDefault: true, + channels: [], + providers: [], + providerAuthEnvVars: { + xai: ["XAI_API_KEY"], + }, + contracts: { + tools: ["other_tool", "optional_tool"], + }, + toolMetadata: { + optional_tool: { + optional: true, + authSignals: [{ provider: "xai" }], + }, + }, + }, + }); + const defaultFactory = vi.fn(() => makeTool("other_tool")); + const optionalFactory = vi.fn(() => makeTool("optional_tool")); + setActivePluginRegistry( + createToolRegistry([ + { + pluginId: "multi", + optional: false, + source: "/tmp/multi.js", + names: ["other_tool"], + declaredNames: ["other_tool"], + factory: defaultFactory, + }, + { + pluginId: "multi", + optional: true, + source: "/tmp/multi.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, ["other_tool"]); + expect(defaultFactory).toHaveBeenCalledTimes(1); + expect(optionalFactory).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 a8e89cb0e87..a14b42e025a 100644 --- a/src/plugins/tools.ts +++ b/src/plugins/tools.ts @@ -349,6 +349,40 @@ function listManifestToolNamesForAvailability(params: { return listManifestToolNamesForAllowlist(params); } +function isManifestToolNameAvailable(params: { + plugin: PluginManifestRecord; + toolName: string; + config: PluginLoadOptions["config"]; + env: NodeJS.ProcessEnv; + hasAuthForProvider?: (providerId: string) => boolean; +}): boolean { + return hasManifestToolAvailability({ + plugin: params.plugin, + toolNames: [params.toolName], + config: params.config, + env: params.env, + hasAuthForProvider: params.hasAuthForProvider, + }); +} + +function filterManifestToolNamesForAvailability(params: { + plugin: PluginManifestRecord; + toolNames: readonly string[]; + config: PluginLoadOptions["config"]; + env: NodeJS.ProcessEnv; + hasAuthForProvider?: (providerId: string) => boolean; +}): string[] { + return params.toolNames.filter((toolName) => + isManifestToolNameAvailable({ + plugin: params.plugin, + toolName, + config: params.config, + env: params.env, + hasAuthForProvider: params.hasAuthForProvider, + }), + ); +} + function resolvePluginToolRuntimePluginIds(params: { config: PluginLoadOptions["config"]; availabilityConfig?: PluginLoadOptions["config"]; @@ -546,21 +580,20 @@ function resolveCachedPluginTools(params: { continue; } const contractToolNames = plugin.contracts?.tools ?? []; - const availableToolNames = listManifestToolNamesForAvailability({ + const allowedToolNames = listManifestToolNamesForAvailability({ plugin, toolNames: contractToolNames, pluginId: plugin.id, allowlist: params.allowlist, }); - if ( - !hasManifestToolAvailability({ - plugin, - toolNames: availableToolNames, - config: params.availabilityConfig, - env: params.env, - hasAuthForProvider: params.hasAuthForProvider, - }) - ) { + const availableToolNames = filterManifestToolNamesForAvailability({ + plugin, + toolNames: allowedToolNames, + config: params.availabilityConfig, + env: params.env, + hasAuthForProvider: params.hasAuthForProvider, + }); + if (availableToolNames.length === 0) { continue; } if (params.existingNormalized.has(normalizeToolName(plugin.id))) { @@ -904,10 +937,25 @@ export function resolvePluginTools(params: { blockedPlugins.add(entry.pluginId); continue; } + const manifestPlugin = manifestPluginsById.get(entry.pluginId); const declaredNames = entry.names ?? []; + const availabilityNames = + declaredNames.length > 0 ? declaredNames : (entry.declaredNames ?? []); + const allowlistNames = manifestPlugin + ? filterManifestToolNamesForAvailability({ + plugin: manifestPlugin, + toolNames: availabilityNames, + config: params.context.runtimeConfig ?? context.config, + env, + hasAuthForProvider: params.hasAuthForProvider, + }) + : declaredNames; + if (manifestPlugin && availabilityNames.length > 0 && allowlistNames.length === 0) { + continue; + } if ( !pluginToolNamesMatchAllowlist({ - names: declaredNames, + names: allowlistNames, pluginId: entry.pluginId, optional: entry.optional, allowlist, @@ -936,15 +984,26 @@ export function resolvePluginTools(params: { continue; } const listRaw: unknown[] = Array.isArray(resolved) ? resolved : [resolved]; - const list = entry.optional + const availableList = manifestPlugin ? listRaw.filter((tool) => + isManifestToolNameAvailable({ + plugin: manifestPlugin, + toolName: readPluginToolName(tool), + config: params.context.runtimeConfig ?? context.config, + env, + hasAuthForProvider: params.hasAuthForProvider, + }), + ) + : listRaw; + const list = entry.optional + ? availableList.filter((tool) => isOptionalToolAllowed({ toolName: readPluginToolName(tool), pluginId: entry.pluginId, allowlist, }), ) - : listRaw; + : availableList; if (list.length === 0) { continue; } @@ -1003,7 +1062,6 @@ export function resolvePluginTools(params: { pluginId: entry.pluginId, optional: entry.optional, }); - const manifestPlugin = manifestPluginsById.get(entry.pluginId); if (manifestPlugin) { const capturedDescriptors = capturedDescriptorsByPluginId.get(entry.pluginId) ?? []; capturedDescriptors.push(