Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/coding-agent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 27 additions & 2 deletions packages/coding-agent/src/mcp/tool-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -261,7 +286,7 @@ export class MCPTool implements CustomTool<TSchema, MCPToolDetails> {
signal?: AbortSignal,
): Promise<CustomToolResult<MCPToolDetails>> {
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;

Expand Down Expand Up @@ -360,7 +385,7 @@ export class DeferredMCPTool implements CustomTool<TSchema, MCPToolDetails> {
signal?: AbortSignal,
): Promise<CustomToolResult<MCPToolDetails>> {
throwIfAborted(signal);
const args = normalizeToolArgs(params);
const args = omitUnusedOptionalArgs(normalizeToolArgs(params), this.tool.inputSchema);
const provider = this.#fallbackProvider;
const providerName = this.#fallbackProviderName;

Expand Down
89 changes: 89 additions & 0 deletions packages/coding-agent/test/mcp-tool-args.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown> | 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" },
},
},
]);
});
});
Loading