From 0a076bc0fca22e2a71bbc600b6f8bed7ee235408 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Apr 2026 13:22:23 +0100 Subject: [PATCH] fix: isolate malformed plugin tools --- CHANGELOG.md | 1 + docs/plugins/building-plugins.md | 1 + src/agents/anthropic-transport-stream.test.ts | 41 ++++++++++++++++ src/agents/anthropic-transport-stream.ts | 28 +++++++---- src/plugins/tools.optional.test.ts | 49 +++++++++++++++++++ src/plugins/tools.ts | 47 ++++++++++++++++-- 6 files changed, 154 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c826d288895..8f395a1b80a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Docs: https://docs.openclaw.ai - Agents/tools: scope tool-loop detection history to the active run when available, so scheduled heartbeat cycles no longer inherit stale repeated-call counts from previous runs. Fixes #40144. Thanks @mattbrown319. - Control UI: show loading, reload, and retry states when a lazy dashboard panel cannot load after an upgrade, so the Logs tab no longer appears blank on stale browser bundles. Fixes #72450. Thanks @sobergou. - Gateway/plugins: start the Gateway in degraded mode when a single plugin entry has invalid schema config, and let `openclaw doctor --fix` quarantine that plugin config instead of crash-looping every channel. Fixes #62976 and #70371. Thanks @Doraemon-Claw and @pksidekyk. +- Agents/plugins: skip malformed plugin tools with missing schema objects and report plugin diagnostics, so one broken tool no longer crashes Anthropic agent runs. Fixes #69423. Thanks @jmnickels. - Agents/reasoning: recover fully wrapped unclosed `` replies that would otherwise sanitize to empty text while keeping strict stripping for closed reasoning blocks and unclosed tails after visible text. Fixes #37696; supersedes #51915. Thanks @druide67 and @okuyam2y. - Control UI/Gateway: bind WebChat handshakes to their active socket and reject post-close server registrations, so aborted connects no longer leave zombie clients or misleading duplicate WebSocket connection logs. Fixes #72753. Thanks @LumenFromTheFuture. - Agents/fallback: split ambiguous provider failures into `empty_response`, `no_error_details`, and `unclassified`, and add flat fallback-step fields to structured fallback logs so primary-model failures stay visible when later fallbacks also fail. Fixes #71922; refs #71744. Thanks @andyk-ms and @nikolaykazakovvs-ux. diff --git a/docs/plugins/building-plugins.md b/docs/plugins/building-plugins.md index ff9d4914d02..7424ca3c6d2 100644 --- a/docs/plugins/building-plugins.md +++ b/docs/plugins/building-plugins.md @@ -243,6 +243,7 @@ Users enable optional tools in config: ``` - Tool names must not clash with core tools (conflicts are skipped) +- Tools with malformed registration objects, including missing `parameters`, are skipped and reported in plugin diagnostics instead of breaking agent runs - Use `optional: true` for tools with side effects or extra binary requirements - Users can enable all tools from a plugin by adding the plugin id to `tools.allow` diff --git a/src/agents/anthropic-transport-stream.test.ts b/src/agents/anthropic-transport-stream.test.ts index e0d1fc9ab90..ea1e7b7d74e 100644 --- a/src/agents/anthropic-transport-stream.test.ts +++ b/src/agents/anthropic-transport-stream.test.ts @@ -309,6 +309,47 @@ describe("anthropic transport stream", () => { ); }); + it("skips malformed tools when building Anthropic payloads", async () => { + await runTransportStream( + makeAnthropicTransportModel(), + { + messages: [{ role: "user", content: "hello" }], + tools: [ + { + name: "bad_plugin_tool", + description: "missing schema", + execute: async () => ({ content: [{ type: "text", text: "bad" }] }), + }, + { + name: "good_plugin_tool", + description: "valid schema", + parameters: { + type: "object", + properties: { + query: { type: "string" }, + }, + required: ["query"], + }, + }, + ], + } as unknown as AnthropicStreamContext, + { + apiKey: "sk-ant-api", + } as AnthropicStreamOptions, + ); + + expect(latestAnthropicRequest().payload.tools).toEqual([ + expect.objectContaining({ + name: "good_plugin_tool", + input_schema: expect.objectContaining({ + properties: { + query: { type: "string" }, + }, + }), + }), + ]); + }); + it("coerces replayed malformed tool-call args to an object for Anthropic payloads", async () => { const model = makeAnthropicTransportModel({ requestTransport: { diff --git a/src/agents/anthropic-transport-stream.ts b/src/agents/anthropic-transport-stream.ts index 0613fe03d1d..81e58698204 100644 --- a/src/agents/anthropic-transport-stream.ts +++ b/src/agents/anthropic-transport-stream.ts @@ -395,17 +395,25 @@ function convertAnthropicTools(tools: Context["tools"], isOAuthToken: boolean) { if (!tools) { return []; } - return tools.map((tool) => { - const parameters = tool.parameters as Record; - return { - name: isOAuthToken ? toClaudeCodeName(tool.name) : tool.name, - description: tool.description, - input_schema: { - type: "object", - properties: parameters.properties || {}, - required: parameters.required || [], + return tools.flatMap((tool) => { + const parameters = + tool.parameters && typeof tool.parameters === "object" && !Array.isArray(tool.parameters) + ? (tool.parameters as Record) + : undefined; + if (!parameters) { + return []; + } + return [ + { + name: isOAuthToken ? toClaudeCodeName(tool.name) : tool.name, + description: tool.description, + input_schema: { + type: "object", + properties: parameters.properties || {}, + required: parameters.required || [], + }, }, - }; + ]; }); } diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index 85defd862c6..0b3fa715a99 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -98,6 +98,17 @@ function createOptionalDemoEntry(): MockRegistryToolEntry { }; } +function createMalformedTool(name: string) { + return { + name, + description: `${name} tool`, + inputSchema: { type: "object", properties: {} }, + async execute() { + return { content: [{ type: "text", text: "bad" }] }; + }, + }; +} + function resolveWithConflictingCoreName(options?: { suppressNameConflicts?: boolean }) { return resolvePluginTools( createResolveToolsParams({ @@ -310,6 +321,44 @@ describe("resolvePluginTools optional tools", () => { expectLoaderCall(expectedLoaderCall); }); + it("skips malformed plugin tools while keeping valid sibling tools", () => { + const registry = setRegistry([ + { + pluginId: "schema-bug", + optional: false, + source: "/tmp/schema-bug.js", + factory: () => [createMalformedTool("broken_tool"), makeTool("valid_tool")], + }, + ]); + + const tools = resolvePluginTools(createResolveToolsParams()); + + expectResolvedToolNames(tools, ["valid_tool"]); + expectSingleDiagnosticMessage( + registry.diagnostics, + "plugin tool is malformed (schema-bug): broken_tool missing parameters object", + ); + }); + + it("skips allowlisted optional malformed plugin tools", () => { + const registry = setRegistry([ + { + pluginId: "optional-demo", + optional: true, + source: "/tmp/optional-demo.js", + factory: () => createMalformedTool("optional_tool"), + }, + ]); + + const tools = resolveOptionalDemoTools(["optional_tool"]); + + expect(tools).toHaveLength(0); + expectSingleDiagnosticMessage( + registry.diagnostics, + "plugin tool is malformed (optional-demo): optional_tool missing parameters object", + ); + }); + it.each([ { name: "loads plugin tools from the auto-enabled config snapshot", diff --git a/src/plugins/tools.ts b/src/plugins/tools.ts index bd17225cf70..e774c3fef9f 100644 --- a/src/plugins/tools.ts +++ b/src/plugins/tools.ts @@ -58,6 +58,34 @@ function isOptionalToolAllowed(params: { return params.allowlist.has("group:plugins"); } +function isRecord(value: unknown): value is Record { + return Boolean(value && typeof value === "object" && !Array.isArray(value)); +} + +function readPluginToolName(tool: unknown): string { + if (!isRecord(tool)) { + return ""; + } + return typeof tool.name === "string" ? tool.name.trim() : ""; +} + +function describeMalformedPluginTool(tool: unknown): string | undefined { + if (!isRecord(tool)) { + return "tool must be an object"; + } + const name = readPluginToolName(tool); + if (!name) { + return "missing non-empty name"; + } + if (typeof tool.execute !== "function") { + return `${name} missing execute function`; + } + if (!isRecord(tool.parameters)) { + return `${name} missing parameters object`; + } + return undefined; +} + function resolvePluginToolRegistry(params: { loadOptions: PluginLoadOptions; allowGatewaySubagentBinding?: boolean; @@ -146,11 +174,11 @@ export function resolvePluginTools(params: { } continue; } - const listRaw = Array.isArray(resolved) ? resolved : [resolved]; + const listRaw: unknown[] = Array.isArray(resolved) ? resolved : [resolved]; const list = entry.optional ? listRaw.filter((tool) => isOptionalToolAllowed({ - toolName: tool.name, + toolName: readPluginToolName(tool), pluginId: entry.pluginId, allowlist, }), @@ -160,7 +188,20 @@ export function resolvePluginTools(params: { continue; } const nameSet = new Set(); - for (const tool of list) { + for (const toolRaw of list) { + const malformedReason = describeMalformedPluginTool(toolRaw); + if (malformedReason) { + const message = `plugin tool is malformed (${entry.pluginId}): ${malformedReason}`; + context.logger.error(message); + registry.diagnostics.push({ + level: "error", + pluginId: entry.pluginId, + source: entry.source, + message, + }); + continue; + } + const tool = toolRaw as AnyAgentTool; if (nameSet.has(tool.name) || existing.has(tool.name)) { const message = `plugin tool name conflict (${entry.pluginId}): ${tool.name}`; if (!params.suppressNameConflicts) {