From 60f5d67e37b447dae27963d3ed012bf6c33acc3c Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Fri, 19 Dec 2025 15:50:46 -0500 Subject: [PATCH 1/2] fix: disable strict mode for MCP tools to preserve optional parameters MCP tools (like Linear's MCP server) have optional parameters that were broken by OpenAI's strict mode which requires ALL properties to be in the required array. This change: - Checks if tool name starts with 'mcp--' prefix - Sets strict: false for MCP tools - Preserves original schema without forcing all properties to required Fixes ROO-241 --- src/api/providers/base-provider.ts | 32 +++++++++++++++++++----------- src/api/providers/openai-native.ts | 21 +++++++++++++------- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/api/providers/base-provider.ts b/src/api/providers/base-provider.ts index 84c8cf6fe97..cd45818d60f 100644 --- a/src/api/providers/base-provider.ts +++ b/src/api/providers/base-provider.ts @@ -28,18 +28,26 @@ export abstract class BaseProvider implements ApiHandler { return undefined } - return tools.map((tool) => - tool.type === "function" - ? { - ...tool, - function: { - ...tool.function, - strict: true, - parameters: this.convertToolSchemaForOpenAI(tool.function.parameters), - }, - } - : tool, - ) + return tools.map((tool) => { + if (tool.type !== "function") { + return tool + } + + // MCP tools use the 'mcp--' prefix - disable strict mode for them + // to preserve optional parameters from the MCP server schema + const isMcpTool = tool.function.name.startsWith("mcp--") + + return { + ...tool, + function: { + ...tool.function, + strict: !isMcpTool, + parameters: isMcpTool + ? tool.function.parameters + : this.convertToolSchemaForOpenAI(tool.function.parameters), + }, + } + }) } /** diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index 762b81fc83a..7b07adeec06 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -291,13 +291,20 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio ...(metadata?.tools && { tools: metadata.tools .filter((tool) => tool.type === "function") - .map((tool) => ({ - type: "function", - name: tool.function.name, - description: tool.function.description, - parameters: ensureAllRequired(tool.function.parameters), - strict: true, - })), + .map((tool) => { + // MCP tools use the 'mcp--' prefix - disable strict mode for them + // to preserve optional parameters from the MCP server schema + const isMcpTool = tool.function.name.startsWith("mcp--") + return { + type: "function", + name: tool.function.name, + description: tool.function.description, + parameters: isMcpTool + ? tool.function.parameters + : ensureAllRequired(tool.function.parameters), + strict: !isMcpTool, + } + }), }), ...(metadata?.tool_choice && { tool_choice: metadata.tool_choice }), } From 7b0985af5e22f47a4a1cca4cbf6426e27dcba4b0 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 22 Dec 2025 17:37:55 +0000 Subject: [PATCH 2/2] refactor: use isMcpTool helper instead of hardcoded mcp-- prefix --- src/api/providers/base-provider.ts | 7 +++--- src/api/providers/openai-native.ts | 9 ++++---- src/utils/__tests__/mcp-name.spec.ts | 32 +++++++++++++++++++++++++++- src/utils/mcp-name.ts | 10 +++++++++ 4 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/api/providers/base-provider.ts b/src/api/providers/base-provider.ts index cd45818d60f..64d99b3f0c1 100644 --- a/src/api/providers/base-provider.ts +++ b/src/api/providers/base-provider.ts @@ -5,6 +5,7 @@ import type { ModelInfo } from "@roo-code/types" import type { ApiHandler, ApiHandlerCreateMessageMetadata } from "../index" import { ApiStream } from "../transform/stream" import { countTokens } from "../../utils/countTokens" +import { isMcpTool } from "../../utils/mcp-name" /** * Base class for API providers that implements common functionality. @@ -35,14 +36,14 @@ export abstract class BaseProvider implements ApiHandler { // MCP tools use the 'mcp--' prefix - disable strict mode for them // to preserve optional parameters from the MCP server schema - const isMcpTool = tool.function.name.startsWith("mcp--") + const isMcp = isMcpTool(tool.function.name) return { ...tool, function: { ...tool.function, - strict: !isMcpTool, - parameters: isMcpTool + strict: !isMcp, + parameters: isMcp ? tool.function.parameters : this.convertToolSchemaForOpenAI(tool.function.parameters), }, diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index 7b07adeec06..8f9cc2297f8 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -24,6 +24,7 @@ import { getModelParams } from "../transform/model-params" import { BaseProvider } from "./base-provider" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata } from "../index" +import { isMcpTool } from "../../utils/mcp-name" export type OpenAiNativeModel = ReturnType @@ -294,15 +295,13 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio .map((tool) => { // MCP tools use the 'mcp--' prefix - disable strict mode for them // to preserve optional parameters from the MCP server schema - const isMcpTool = tool.function.name.startsWith("mcp--") + const isMcp = isMcpTool(tool.function.name) return { type: "function", name: tool.function.name, description: tool.function.description, - parameters: isMcpTool - ? tool.function.parameters - : ensureAllRequired(tool.function.parameters), - strict: !isMcpTool, + parameters: isMcp ? tool.function.parameters : ensureAllRequired(tool.function.parameters), + strict: !isMcp, } }), }), diff --git a/src/utils/__tests__/mcp-name.spec.ts b/src/utils/__tests__/mcp-name.spec.ts index b28c2e504c4..5511893f79e 100644 --- a/src/utils/__tests__/mcp-name.spec.ts +++ b/src/utils/__tests__/mcp-name.spec.ts @@ -1,4 +1,11 @@ -import { sanitizeMcpName, buildMcpToolName, parseMcpToolName, MCP_TOOL_SEPARATOR, MCP_TOOL_PREFIX } from "../mcp-name" +import { + sanitizeMcpName, + buildMcpToolName, + parseMcpToolName, + isMcpTool, + MCP_TOOL_SEPARATOR, + MCP_TOOL_PREFIX, +} from "../mcp-name" describe("mcp-name utilities", () => { describe("constants", () => { @@ -8,6 +15,29 @@ describe("mcp-name utilities", () => { }) }) + describe("isMcpTool", () => { + it("should return true for valid MCP tool names", () => { + expect(isMcpTool("mcp--server--tool")).toBe(true) + expect(isMcpTool("mcp--my_server--get_forecast")).toBe(true) + }) + + it("should return false for non-MCP tool names", () => { + expect(isMcpTool("server--tool")).toBe(false) + expect(isMcpTool("tool")).toBe(false) + expect(isMcpTool("read_file")).toBe(false) + expect(isMcpTool("")).toBe(false) + }) + + it("should return false for old underscore format", () => { + expect(isMcpTool("mcp_server_tool")).toBe(false) + }) + + it("should return false for partial prefix", () => { + expect(isMcpTool("mcp-server")).toBe(false) + expect(isMcpTool("mcp")).toBe(false) + }) + }) + describe("sanitizeMcpName", () => { it("should return underscore placeholder for empty input", () => { expect(sanitizeMcpName("")).toBe("_") diff --git a/src/utils/mcp-name.ts b/src/utils/mcp-name.ts index 55845d67ed7..c81d5e770f0 100644 --- a/src/utils/mcp-name.ts +++ b/src/utils/mcp-name.ts @@ -17,6 +17,16 @@ export const MCP_TOOL_SEPARATOR = "--" */ export const MCP_TOOL_PREFIX = "mcp" +/** + * Check if a tool name is an MCP tool (starts with the MCP prefix and separator). + * + * @param toolName - The tool name to check + * @returns true if the tool name starts with "mcp--", false otherwise + */ +export function isMcpTool(toolName: string): boolean { + return toolName.startsWith(`${MCP_TOOL_PREFIX}${MCP_TOOL_SEPARATOR}`) +} + /** * Sanitize a name to be safe for use in API function names. * This removes special characters and ensures the name starts correctly.