diff --git a/packages/opencode/src/mcp/catalog.ts b/packages/opencode/src/mcp/catalog.ts index 0cd2238edffe..1a5ef4e2b3a9 100644 --- a/packages/opencode/src/mcp/catalog.ts +++ b/packages/opencode/src/mcp/catalog.ts @@ -54,7 +54,7 @@ export function convertTool(mcpTool: MCPToolDef, client: Client, timeout?: numbe const result = await client.callTool( { name: mcpTool.name, - arguments: (args || {}) as Record, + arguments: cleanToolArguments(args, inputSchema), }, CallToolResultSchema, { @@ -81,6 +81,26 @@ export function convertTool(mcpTool: MCPToolDef, client: Client, timeout?: numbe }) } +// Some providers (notably OpenAI/GPT) fill omitted optional string parameters +// with empty strings. MCP servers treat "" as an explicit value rather than +// "unset", which breaks tools whose optionals carry distinct semantics +// (e.g. Slack search cursor/after/before returning no results). Drop the +// empty-string values for optional declared properties so the field is omitted +// from the tools/call request, matching the Anthropic path. See #33341. +export function cleanToolArguments(args: unknown, schema: JSONSchema7): Record { + if (!args || typeof args !== "object" || Array.isArray(args)) return {} + const required = new Set( + Array.isArray(schema.required) ? schema.required.filter((item): item is string => typeof item === "string") : [], + ) + const properties = schema.properties ?? {} + const cleaned: Record = {} + for (const [key, value] of Object.entries(args as Record)) { + if (value === "" && !required.has(key) && key in properties) continue + cleaned[key] = value + } + return cleaned +} + export function fetch( clientName: string, client: Client, diff --git a/packages/opencode/test/mcp/catalog.test.ts b/packages/opencode/test/mcp/catalog.test.ts new file mode 100644 index 000000000000..b74caf253806 --- /dev/null +++ b/packages/opencode/test/mcp/catalog.test.ts @@ -0,0 +1,68 @@ +import { expect, test } from "bun:test" +import type { JSONSchema7 } from "ai" +import { McpCatalog } from "../../src/mcp/catalog" + +// Mirrors Slack's official MCP search tools (see #33341): `query` is required, +// the rest are optional string fields. The OpenAI/GPT path fills the omitted +// optionals with "", which Slack treats as an explicit value. +const schema: JSONSchema7 = { + type: "object", + properties: { + query: { type: "string" }, + cursor: { type: "string" }, + after: { type: "string" }, + before: { type: "string" }, + context_channel_id: { type: "string" }, + }, + required: ["query"], +} + +const clean = McpCatalog.cleanToolArguments + +test("drops empty-string optional string args, keeps required", () => { + expect(clean({ query: "match", cursor: "" }, schema)).toEqual({ query: "match" }) +}) + +test("drops every omitted optional that the OpenAI path fills as empty string", () => { + expect( + clean( + { query: "match", cursor: "", after: "", before: "", context_channel_id: "" }, + schema, + ), + ).toEqual({ query: "match" }) +}) + +test("keeps non-empty optional string args", () => { + expect(clean({ query: "match", cursor: "page-2" }, schema)).toEqual({ + query: "match", + cursor: "page-2", + }) +}) + +test("keeps empty string for required fields", () => { + expect(clean({ query: "", cursor: "" }, schema)).toEqual({ query: "" }) +}) + +test("does not drop falsy non-string values for optionals", () => { + expect(clean({ query: "match", cursor: 0 }, schema)).toEqual({ query: "match", cursor: 0 }) +}) + +test("leaves unknown keys untouched", () => { + expect(clean({ query: "match", unknown: "" }, schema)).toEqual({ query: "match", unknown: "" }) +}) + +test("normalizes non-object args to an empty object", () => { + expect(clean(null, schema)).toEqual({}) + expect(clean(undefined, schema)).toEqual({}) + expect(clean("query", schema)).toEqual({}) + expect(clean([], schema)).toEqual({}) +}) + +test("treats args with no required list as all-optional", () => { + const allOptional: JSONSchema7 = { + type: "object", + properties: { cursor: { type: "string" } }, + } + expect(clean({ cursor: "" }, allOptional)).toEqual({}) + expect(clean({ cursor: "x" }, allOptional)).toEqual({ cursor: "x" }) +})