fix(mcp): tolerate output schema ref failures (#26614)
This commit is contained in:
@@ -6,6 +6,7 @@ import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"
|
|||||||
import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js"
|
import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js"
|
||||||
import {
|
import {
|
||||||
CallToolResultSchema,
|
CallToolResultSchema,
|
||||||
|
ToolSchema,
|
||||||
type Tool as MCPToolDef,
|
type Tool as MCPToolDef,
|
||||||
ToolListChangedNotificationSchema,
|
ToolListChangedNotificationSchema,
|
||||||
} from "@modelcontextprotocol/sdk/types.js"
|
} from "@modelcontextprotocol/sdk/types.js"
|
||||||
@@ -36,6 +37,15 @@ import { withStatics } from "@opencode-ai/core/schema"
|
|||||||
const log = Log.create({ service: "mcp" })
|
const log = Log.create({ service: "mcp" })
|
||||||
const DEFAULT_TIMEOUT = 30_000
|
const DEFAULT_TIMEOUT = 30_000
|
||||||
|
|
||||||
|
const TolerantToolSchema = ToolSchema.extend({
|
||||||
|
outputSchema: z.unknown().optional(),
|
||||||
|
})
|
||||||
|
|
||||||
|
const TolerantListToolsResultSchema = z.looseObject({
|
||||||
|
tools: z.array(TolerantToolSchema),
|
||||||
|
nextCursor: z.string().optional(),
|
||||||
|
})
|
||||||
|
|
||||||
export const Resource = Schema.Struct({
|
export const Resource = Schema.Struct({
|
||||||
name: Schema.String,
|
name: Schema.String,
|
||||||
uri: Schema.String,
|
uri: Schema.String,
|
||||||
@@ -119,6 +129,38 @@ function remoteURL(key: string, value: string) {
|
|||||||
log.warn("invalid remote mcp url", { key })
|
log.warn("invalid remote mcp url", { key })
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isOutputSchemaValidationError(error: Error) {
|
||||||
|
return /can't resolve reference|resolves to more than one schema|outputSchema|schema.*reference|reference.*schema/i.test(
|
||||||
|
error.message,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
function listTools(key: string, client: MCPClient, timeout: number) {
|
||||||
|
return Effect.tryPromise({
|
||||||
|
try: () => client.listTools(undefined, { timeout }),
|
||||||
|
catch: (err) => (err instanceof Error ? err : new Error(String(err))),
|
||||||
|
}).pipe(
|
||||||
|
Effect.map((result) => result.tools),
|
||||||
|
Effect.catch((error) => {
|
||||||
|
if (!isOutputSchemaValidationError(error)) return Effect.fail(error)
|
||||||
|
|
||||||
|
log.warn("failed to validate MCP tool output schemas, retrying without output schema validation", { key, error })
|
||||||
|
return Effect.tryPromise({
|
||||||
|
try: () => client.request({ method: "tools/list" }, TolerantListToolsResultSchema, { timeout }),
|
||||||
|
catch: (err) => (err instanceof Error ? err : new Error(String(err))),
|
||||||
|
}).pipe(
|
||||||
|
Effect.map((result) =>
|
||||||
|
result.tools.map((tool) => ({
|
||||||
|
name: tool.name,
|
||||||
|
description: tool.description,
|
||||||
|
inputSchema: tool.inputSchema,
|
||||||
|
})),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
// Convert MCP tool definition to AI SDK Tool type
|
// Convert MCP tool definition to AI SDK Tool type
|
||||||
function convertMcpTool(mcpTool: MCPToolDef, client: MCPClient, timeout?: number): Tool {
|
function convertMcpTool(mcpTool: MCPToolDef, client: MCPClient, timeout?: number): Tool {
|
||||||
const inputSchema = mcpTool.inputSchema
|
const inputSchema = mcpTool.inputSchema
|
||||||
@@ -151,11 +193,7 @@ function convertMcpTool(mcpTool: MCPToolDef, client: MCPClient, timeout?: number
|
|||||||
}
|
}
|
||||||
|
|
||||||
function defs(key: string, client: MCPClient, timeout?: number) {
|
function defs(key: string, client: MCPClient, timeout?: number) {
|
||||||
return Effect.tryPromise({
|
return listTools(key, client, timeout ?? DEFAULT_TIMEOUT).pipe(
|
||||||
try: () => withTimeout(client.listTools(), timeout ?? DEFAULT_TIMEOUT),
|
|
||||||
catch: (err) => (err instanceof Error ? err : new Error(String(err))),
|
|
||||||
}).pipe(
|
|
||||||
Effect.map((result) => result.tools),
|
|
||||||
Effect.catch((err) => {
|
Effect.catch((err) => {
|
||||||
log.error("failed to get tools from client", { key, error: err })
|
log.error("failed to get tools from client", { key, error: err })
|
||||||
return Effect.succeed(undefined)
|
return Effect.succeed(undefined)
|
||||||
|
|||||||
@@ -7,8 +7,9 @@ import type { MCP as MCPNS } from "../../src/mcp/index"
|
|||||||
|
|
||||||
// Per-client state for controlling mock behavior
|
// Per-client state for controlling mock behavior
|
||||||
interface MockClientState {
|
interface MockClientState {
|
||||||
tools: Array<{ name: string; description?: string; inputSchema: object }>
|
tools: Array<{ name: string; description?: string; inputSchema: object; outputSchema?: object }>
|
||||||
listToolsCalls: number
|
listToolsCalls: number
|
||||||
|
requestCalls: number
|
||||||
listToolsShouldFail: boolean
|
listToolsShouldFail: boolean
|
||||||
listToolsError: string
|
listToolsError: string
|
||||||
listPromptsShouldFail: boolean
|
listPromptsShouldFail: boolean
|
||||||
@@ -36,6 +37,7 @@ function getOrCreateClientState(name?: string): MockClientState {
|
|||||||
state = {
|
state = {
|
||||||
tools: [{ name: "test_tool", description: "A test tool", inputSchema: { type: "object", properties: {} } }],
|
tools: [{ name: "test_tool", description: "A test tool", inputSchema: { type: "object", properties: {} } }],
|
||||||
listToolsCalls: 0,
|
listToolsCalls: 0,
|
||||||
|
requestCalls: 0,
|
||||||
listToolsShouldFail: false,
|
listToolsShouldFail: false,
|
||||||
listToolsError: "listTools failed",
|
listToolsError: "listTools failed",
|
||||||
listPromptsShouldFail: false,
|
listPromptsShouldFail: false,
|
||||||
@@ -139,6 +141,12 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||||||
return { tools: this._state?.tools ?? [] }
|
return { tools: this._state?.tools ?? [] }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async request(request: { method: string }, schema: { parse: (value: unknown) => unknown }) {
|
||||||
|
if (this._state) this._state.requestCalls++
|
||||||
|
if (request.method === "tools/list") return schema.parse({ tools: this._state?.tools ?? [] })
|
||||||
|
throw new Error(`unsupported request: ${request.method}`)
|
||||||
|
}
|
||||||
|
|
||||||
async listPrompts() {
|
async listPrompts() {
|
||||||
if (this._state?.listPromptsShouldFail) {
|
if (this._state?.listPromptsShouldFail) {
|
||||||
throw new Error("listPrompts failed")
|
throw new Error("listPrompts failed")
|
||||||
@@ -205,6 +213,11 @@ function withInstance(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function statusName(status: Record<string, MCPNS.Status> | MCPNS.Status, server: string) {
|
||||||
|
if ("status" in status) return status.status
|
||||||
|
return status[server]?.status
|
||||||
|
}
|
||||||
|
|
||||||
// ========================================================================
|
// ========================================================================
|
||||||
// Test: tools() are cached after connect
|
// Test: tools() are cached after connect
|
||||||
// ========================================================================
|
// ========================================================================
|
||||||
@@ -433,6 +446,59 @@ test(
|
|||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
test(
|
||||||
|
"falls back when MCP output schema refs fail SDK tool discovery",
|
||||||
|
withInstance({}, (mcp) =>
|
||||||
|
Effect.gen(function* () {
|
||||||
|
lastCreatedClientName = "stitch-like-server"
|
||||||
|
const serverState = getOrCreateClientState("stitch-like-server")
|
||||||
|
serverState.listToolsShouldFail = true
|
||||||
|
serverState.listToolsError = "can't resolve reference #/$defs/ScreenInstance from id #"
|
||||||
|
serverState.tools = [
|
||||||
|
{
|
||||||
|
name: "render_screen",
|
||||||
|
description: "renders a screen",
|
||||||
|
inputSchema: { type: "object", properties: { prompt: { type: "string" } }, required: ["prompt"] },
|
||||||
|
outputSchema: { type: "object", properties: { screen: { $ref: "#/$defs/ScreenInstance" } } },
|
||||||
|
},
|
||||||
|
]
|
||||||
|
|
||||||
|
const addResult = yield* mcp.add("stitch-like-server", {
|
||||||
|
type: "local",
|
||||||
|
command: ["echo", "test"],
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(statusName(addResult.status, "stitch-like-server")).toBe("connected")
|
||||||
|
|
||||||
|
const tools = yield* mcp.tools()
|
||||||
|
expect(Object.keys(tools).some((key) => key.includes("render_screen"))).toBe(true)
|
||||||
|
expect(serverState.listToolsCalls).toBe(1)
|
||||||
|
expect(serverState.requestCalls).toBe(1)
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
test(
|
||||||
|
"does not fall back for non-schema MCP tool discovery errors",
|
||||||
|
withInstance({}, (mcp) =>
|
||||||
|
Effect.gen(function* () {
|
||||||
|
lastCreatedClientName = "broken-server"
|
||||||
|
const serverState = getOrCreateClientState("broken-server")
|
||||||
|
serverState.listToolsShouldFail = true
|
||||||
|
serverState.listToolsError = "transport closed"
|
||||||
|
|
||||||
|
const addResult = yield* mcp.add("broken-server", {
|
||||||
|
type: "local",
|
||||||
|
command: ["echo", "test"],
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(statusName(addResult.status, "broken-server")).toBe("failed")
|
||||||
|
expect(serverState.listToolsCalls).toBe(1)
|
||||||
|
expect(serverState.requestCalls).toBe(0)
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
// ========================================================================
|
// ========================================================================
|
||||||
// Test: disabled server via config
|
// Test: disabled server via config
|
||||||
// ========================================================================
|
// ========================================================================
|
||||||
|
|||||||
Reference in New Issue
Block a user