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
22 changes: 21 additions & 1 deletion packages/opencode/src/mcp/catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>,
arguments: cleanToolArguments(args, inputSchema),
},
CallToolResultSchema,
{
Expand All @@ -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<string, unknown> {
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<string, unknown> = {}
for (const [key, value] of Object.entries(args as Record<string, unknown>)) {
if (value === "" && !required.has(key) && key in properties) continue
cleaned[key] = value
}
return cleaned
}

export function fetch<T extends { name: string }>(
clientName: string,
client: Client,
Expand Down
68 changes: 68 additions & 0 deletions packages/opencode/test/mcp/catalog.test.ts
Original file line number Diff line number Diff line change
@@ -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" })
})
Loading