diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1bff466483..57b94c41f6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed MCP tool calls forwarding empty optional placeholder arguments (`""` and `{}`) to `tools/call`; optional placeholders are now omitted while required fields and meaningful falsy values are preserved. ([#3302](https://github.com/can1357/oh-my-pi/issues/3302)) + ## [16.1.16] - 2026-06-23 ### Breaking Changes diff --git a/packages/coding-agent/src/mcp/tool-bridge.ts b/packages/coding-agent/src/mcp/tool-bridge.ts index 6d6e75db8d..3be6e3bca2 100644 --- a/packages/coding-agent/src/mcp/tool-bridge.ts +++ b/packages/coding-agent/src/mcp/tool-bridge.ts @@ -58,6 +58,31 @@ function normalizeToolArgs(value: unknown): MCPToolArgs { return value as MCPToolArgs; } +function isUnusedOptionalPlaceholder(value: unknown): boolean { + return ( + value === undefined || + value === "" || + (typeof value === "object" && value !== null && !Array.isArray(value) && Object.keys(value).length === 0) + ); +} + +function omitUnusedOptionalArgs(args: MCPToolArgs, inputSchema: MCPToolDefinition["inputSchema"]): MCPToolArgs { + const properties = inputSchema.properties; + if (!properties) return args; + + let cleaned: MCPToolArgs | undefined; + const required = new Set(inputSchema.required ?? []); + for (const [key, value] of Object.entries(args)) { + if (required.has(key) || !Object.hasOwn(properties, key) || !isUnusedOptionalPlaceholder(value)) { + continue; + } + cleaned ??= { ...args }; + delete cleaned[key]; + } + + return cleaned ?? args; +} + /** Details included in MCP tool results for rendering */ export interface MCPToolDetails { /** Server name */ @@ -261,7 +286,7 @@ export class MCPTool implements CustomTool { signal?: AbortSignal, ): Promise> { throwIfAborted(signal); - const args = normalizeToolArgs(params); + const args = omitUnusedOptionalArgs(normalizeToolArgs(params), this.tool.inputSchema); const provider = this.connection._source?.provider; const providerName = this.connection._source?.providerName; @@ -360,7 +385,7 @@ export class DeferredMCPTool implements CustomTool { signal?: AbortSignal, ): Promise> { throwIfAborted(signal); - const args = normalizeToolArgs(params); + const args = omitUnusedOptionalArgs(normalizeToolArgs(params), this.tool.inputSchema); const provider = this.#fallbackProvider; const providerName = this.#fallbackProviderName; diff --git a/packages/coding-agent/test/mcp-tool-args.test.ts b/packages/coding-agent/test/mcp-tool-args.test.ts new file mode 100644 index 0000000000..c7fa709204 --- /dev/null +++ b/packages/coding-agent/test/mcp-tool-args.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from "bun:test"; +import type { CustomToolContext } from "@oh-my-pi/pi-coding-agent/extensibility/custom-tools"; +import { DeferredMCPTool, MCPTool, type MCPToolDefinition } from "@oh-my-pi/pi-coding-agent/mcp"; +import type { MCPServerConnection } from "@oh-my-pi/pi-coding-agent/mcp/types"; +import { createMockConnection, createMockTransport } from "./mcp-test-utils"; + +type CapturedRequest = { + method: string; + params: Record | undefined; +}; + +const unusedContext = {} as CustomToolContext; + +function createSearchToolDefinition(): MCPToolDefinition { + return { + name: "search", + description: "Search symbols or file locations", + inputSchema: { + type: "object", + properties: { + symbol: { type: "string" }, + language: { type: "string" }, + file: { type: "string" }, + line: { type: "number" }, + column: { type: "number" }, + filters: { type: "object" }, + exact: { type: "boolean" }, + }, + required: ["symbol", "language"], + }, + }; +} + +function createCapturedConnection(calls: CapturedRequest[]): MCPServerConnection { + const transport = createMockTransport( + new Map([["tools/call", [{ content: [{ type: "text", text: "ok" }] }]]]), + (method, params) => calls.push({ method, params }), + ); + return createMockConnection({ tools: {} }, transport); +} + +describe("MCP tool arguments", () => { + it("omits optional empty placeholders before tools/call", async () => { + const calls: CapturedRequest[] = []; + const tool = new MCPTool(createCapturedConnection(calls), createSearchToolDefinition()); + + await tool.execute( + "call-1", + { symbol: "Foo", language: "", file: "", line: 0, filters: {}, exact: false }, + undefined, + unusedContext, + undefined, + ); + + expect(calls).toEqual([ + { + method: "tools/call", + params: { + name: "search", + arguments: { symbol: "Foo", language: "", line: 0, exact: false }, + }, + }, + ]); + }); + + it("omits optional empty placeholders for deferred MCP tools", async () => { + const calls: CapturedRequest[] = []; + const connection = createCapturedConnection(calls); + const tool = new DeferredMCPTool("intellij-index", createSearchToolDefinition(), async () => connection); + + await tool.execute( + "call-1", + { symbol: "Foo", language: "TypeScript", file: "", column: "", filters: {} }, + undefined, + unusedContext, + undefined, + ); + + expect(calls).toEqual([ + { + method: "tools/call", + params: { + name: "search", + arguments: { symbol: "Foo", language: "TypeScript" }, + }, + }, + ]); + }); +});