From c6c4cd7dcae4f82af311039a6f3acb456470d927 Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Wed, 24 Jun 2026 23:23:16 -0500 Subject: [PATCH] fix(opencode): validate deferred MCP calls --- packages/opencode/src/mcp/tool-search.ts | 14 +++++++- packages/opencode/src/session/tools.ts | 6 ++-- .../opencode/test/mcp/tool-search.test.ts | 36 +++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/packages/opencode/src/mcp/tool-search.ts b/packages/opencode/src/mcp/tool-search.ts index 465dfc3dc8..644c1a19d1 100644 --- a/packages/opencode/src/mcp/tool-search.ts +++ b/packages/opencode/src/mcp/tool-search.ts @@ -1,19 +1,24 @@ import { jsonSchema, tool, type JSONSchema7, type Tool, type ToolExecutionOptions } from "ai" import fuzzysort from "fuzzysort" import { Token } from "@opencode-ai/core/util/token" +import { AjvJsonSchemaValidator } from "@modelcontextprotocol/sdk/validation/ajv" +import type { JsonSchemaType, JsonSchemaValidator } from "@modelcontextprotocol/sdk/validation" // Match Hermes defaults. OpenClaw independently uses the same maximum. const DEFAULT_SEARCH_LIMIT = 5 const MAX_SEARCH_LIMIT = 20 const MAX_SEARCH_DESCRIPTION = 400 const SEARCH_THRESHOLD_TOKENS = 15_000 +const CONTROL_NAMES = ["mcp_search", "mcp_describe", "mcp_call"] const controls = new WeakSet() +const validator = new AjvJsonSchemaValidator() type Entry = { id: string description: string parameters: string schema: JSONSchema7 + validate: JsonSchemaValidator> tool: Tool } @@ -29,6 +34,10 @@ export function isControl(item: Tool) { return controls.has(item) } +export function collides(tools: Record) { + return CONTROL_NAMES.some((name) => tools[name]) +} + export function create(input: { tools: Record schemas: Record @@ -46,6 +55,7 @@ export function create(input: { description: item.description ?? "", parameters: Object.keys(schema.properties ?? {}).join(" "), schema, + validate: validator.getValidator>(schema as JsonSchemaType), tool: item, }, ] as const @@ -139,7 +149,9 @@ export function create(input: { async execute(args: { id: string; args?: Record }, options: ToolExecutionOptions) { const entry = resolve(entries, args.id) if (!entry.tool.execute) throw new Error(`MCP tool "${entry.id}" is not executable`) - return entry.tool.execute(args.args ?? {}, options) + const result = entry.validate(args.args ?? {}) + if (!result.valid) throw new Error(`Invalid arguments for MCP tool "${entry.id}": ${result.errorMessage}`) + return entry.tool.execute(result.data, options) }, }), } diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index 9be48725ac..b6916c7ded 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -502,6 +502,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { Object.assign(tools, mcpTools) return tools } + if (McpToolSearch.collides(tools)) { + Object.assign(tools, mcpTools) + return tools + } const schemas = Object.fromEntries(Object.keys(searchable).map((key) => [key, mcpSchemas[key]])) const controls = McpToolSearch.create({ @@ -509,8 +513,6 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { schemas, transformSchema: (schema) => ProviderTransform.schema(input.model, schema), }) - const collision = Object.keys(controls).find((key) => tools[key]) - if (collision) throw new Error(`Tool name reserved for MCP tool search: ${collision}`) Object.assign(tools, controls) return tools }) diff --git a/packages/opencode/test/mcp/tool-search.test.ts b/packages/opencode/test/mcp/tool-search.test.ts index 7d902840a9..7a14c84226 100644 --- a/packages/opencode/test/mcp/tool-search.test.ts +++ b/packages/opencode/test/mcp/tool-search.test.ts @@ -60,6 +60,11 @@ describe("MCP tool search", () => { expect(McpToolSearch.shouldUse(tools, schemas)).toBe(true) }) + test("detects control-name collisions", () => { + expect(McpToolSearch.collides({ mcp_call: target("mcp_call", "Plugin tool") })).toBe(true) + expect(McpToolSearch.collides({ other: target("other", "Plugin tool") })).toBe(false) + }) + test("exposes only the three stable control tools", async () => { expect(Object.keys(await catalog())).toEqual(["mcp_search", "mcp_describe", "mcp_call"]) }) @@ -120,6 +125,37 @@ describe("MCP tool search", () => { expect(result.output).toBe('{"title":"Cache bug"}') }) + test("validates hidden target arguments before execution", async () => { + let calls = 0 + const tools = McpToolSearch.create({ + tools: { + create_issue: tool({ + inputSchema: jsonSchema({}), + async execute() { + calls++ + return { output: "called" } + }, + }), + }, + schemas: { + create_issue: { + type: "object", + properties: { title: { type: "string" } }, + required: ["title"], + additionalProperties: false, + }, + }, + transformSchema: (schema) => schema, + }) + expect( + tools.mcp_call!.execute?.( + { id: "create_issue", args: {} }, + { toolCallId: "call", messages: [], abortSignal: new AbortController().signal }, + ), + ).rejects.toThrow('Invalid arguments for MCP tool "create_issue"') + expect(calls).toBe(0) + }) + test("suggests but does not execute inexact tool names", async () => { const tools = await McpToolSearch.create({ tools: {