Compare commits
3 commits
dev
...
kit/mcp-to
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ff6f032faa | ||
|
|
d3a69ad910 | ||
|
|
1c3950111a |
2 changed files with 286 additions and 5 deletions
|
|
@ -36,6 +36,24 @@ 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 = z.looseObject({
|
||||||
|
name: z.string(),
|
||||||
|
description: z.string().optional(),
|
||||||
|
inputSchema: z
|
||||||
|
.object({
|
||||||
|
type: z.literal("object"),
|
||||||
|
properties: z.record(z.string(), z.unknown()).optional(),
|
||||||
|
required: z.array(z.string()).optional(),
|
||||||
|
})
|
||||||
|
.catchall(z.unknown()),
|
||||||
|
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 +137,32 @@ function remoteURL(key: string, value: string) {
|
||||||
log.warn("invalid remote mcp url", { key })
|
log.warn("invalid remote mcp url", { key })
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isSchemaReferenceError(err: unknown) {
|
||||||
|
return err instanceof Error && /can't resolve reference|schema.*reference|reference.*schema/i.test(err.message)
|
||||||
|
}
|
||||||
|
|
||||||
|
const wrapAsError = (err: unknown) => (err instanceof Error ? err : new Error(String(err)))
|
||||||
|
|
||||||
|
function listToolsTolerant(key: string, client: MCPClient, timeout: number) {
|
||||||
|
return Effect.tryPromise({
|
||||||
|
try: () => client.listTools(undefined, { timeout }),
|
||||||
|
catch: wrapAsError,
|
||||||
|
}).pipe(
|
||||||
|
Effect.map((result) => result.tools),
|
||||||
|
Effect.catch((err) => {
|
||||||
|
if (!isSchemaReferenceError(err)) return Effect.fail(err)
|
||||||
|
log.warn("failed to validate MCP tool output schemas, retrying without output schema validation", {
|
||||||
|
key,
|
||||||
|
error: err,
|
||||||
|
})
|
||||||
|
return Effect.tryPromise({
|
||||||
|
try: () => client.request({ method: "tools/list" }, TolerantListToolsResultSchema, { timeout }),
|
||||||
|
catch: wrapAsError,
|
||||||
|
}).pipe(Effect.map((result) => result.tools as MCPToolDef[]))
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
// 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 +195,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 listToolsTolerant(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)
|
||||||
|
|
|
||||||
241
packages/opencode/test/mcp/output-schema-unresolved-refs.test.ts
Normal file
241
packages/opencode/test/mcp/output-schema-unresolved-refs.test.ts
Normal file
|
|
@ -0,0 +1,241 @@
|
||||||
|
// Reproducer for opencode issue #26529
|
||||||
|
//
|
||||||
|
// When an MCP server's `tools/list` response contains a tool whose
|
||||||
|
// `outputSchema` has an unresolved `$ref` (e.g. `#/$defs/ScreenInstance`),
|
||||||
|
// the MCP SDK's response validation throws on the entire `listTools()`
|
||||||
|
// call. opencode currently treats this as a fatal error and marks the
|
||||||
|
// whole server as `failed`, even though the server has other valid tools
|
||||||
|
// that should still be usable.
|
||||||
|
//
|
||||||
|
// Expected behavior: opencode should skip tools with malformed schemas
|
||||||
|
// and keep the server connected with its remaining valid tools.
|
||||||
|
|
||||||
|
import { test, expect, mock, beforeEach } from "bun:test"
|
||||||
|
import { InstanceRuntime } from "../../src/project/instance-runtime"
|
||||||
|
import { Effect } from "effect"
|
||||||
|
import type { MCP as MCPNS } from "../../src/mcp/index"
|
||||||
|
|
||||||
|
// --- Mock infrastructure (mirrors lifecycle.test.ts patterns) ---
|
||||||
|
|
||||||
|
interface MockClientState {
|
||||||
|
tools: Array<{ name: string; description?: string; inputSchema: object; outputSchema?: object }>
|
||||||
|
listToolsShouldFail: boolean
|
||||||
|
listToolsError: string
|
||||||
|
notificationHandlers: Map<unknown, (...args: any[]) => any>
|
||||||
|
closed: boolean
|
||||||
|
}
|
||||||
|
|
||||||
|
const clientStates = new Map<string, MockClientState>()
|
||||||
|
let lastCreatedClientName: string | undefined
|
||||||
|
|
||||||
|
function getOrCreateClientState(name?: string): MockClientState {
|
||||||
|
const key = name ?? "default"
|
||||||
|
let state = clientStates.get(key)
|
||||||
|
if (!state) {
|
||||||
|
state = {
|
||||||
|
tools: [],
|
||||||
|
listToolsShouldFail: false,
|
||||||
|
listToolsError: "listTools failed",
|
||||||
|
notificationHandlers: new Map(),
|
||||||
|
closed: false,
|
||||||
|
}
|
||||||
|
clientStates.set(key, state)
|
||||||
|
}
|
||||||
|
return state
|
||||||
|
}
|
||||||
|
|
||||||
|
class MockStdioTransport {
|
||||||
|
stderr: null = null
|
||||||
|
pid = 12345
|
||||||
|
// oxlint-disable-next-line no-useless-constructor
|
||||||
|
constructor(_opts: any) {}
|
||||||
|
async start() {}
|
||||||
|
async close() {}
|
||||||
|
}
|
||||||
|
|
||||||
|
class MockStreamableHTTP {
|
||||||
|
// oxlint-disable-next-line no-useless-constructor
|
||||||
|
constructor(_url: URL, _opts?: any) {}
|
||||||
|
async start() {}
|
||||||
|
async close() {}
|
||||||
|
async finishAuth() {}
|
||||||
|
}
|
||||||
|
|
||||||
|
class MockSSE {
|
||||||
|
// oxlint-disable-next-line no-useless-constructor
|
||||||
|
constructor(_url: URL, _opts?: any) {}
|
||||||
|
async start() {}
|
||||||
|
async close() {}
|
||||||
|
}
|
||||||
|
|
||||||
|
void mock.module("@modelcontextprotocol/sdk/client/stdio.js", () => ({
|
||||||
|
StdioClientTransport: MockStdioTransport,
|
||||||
|
}))
|
||||||
|
|
||||||
|
void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
|
||||||
|
StreamableHTTPClientTransport: MockStreamableHTTP,
|
||||||
|
}))
|
||||||
|
|
||||||
|
void mock.module("@modelcontextprotocol/sdk/client/sse.js", () => ({
|
||||||
|
SSEClientTransport: MockSSE,
|
||||||
|
}))
|
||||||
|
|
||||||
|
void mock.module("@modelcontextprotocol/sdk/client/auth.js", () => ({
|
||||||
|
UnauthorizedError: class extends Error {
|
||||||
|
constructor() {
|
||||||
|
super("Unauthorized")
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
||||||
|
Client: class MockClient {
|
||||||
|
_state!: MockClientState
|
||||||
|
transport: any
|
||||||
|
// oxlint-disable-next-line no-useless-constructor
|
||||||
|
constructor(_opts: any) {}
|
||||||
|
async connect(transport: { start: () => Promise<void> }) {
|
||||||
|
this.transport = transport
|
||||||
|
await transport.start()
|
||||||
|
this._state = getOrCreateClientState(lastCreatedClientName)
|
||||||
|
}
|
||||||
|
setNotificationHandler(schema: unknown, handler: (...args: any[]) => any) {
|
||||||
|
this._state?.notificationHandlers.set(schema, handler)
|
||||||
|
}
|
||||||
|
async listTools() {
|
||||||
|
if (this._state?.listToolsShouldFail) {
|
||||||
|
throw new Error(this._state.listToolsError)
|
||||||
|
}
|
||||||
|
return { tools: this._state?.tools ?? [] }
|
||||||
|
}
|
||||||
|
async request(req: { method: string }) {
|
||||||
|
// The fix retries via raw `request("tools/list", ...)` with a
|
||||||
|
// tolerant schema after the typed listTools() rejects on a bad
|
||||||
|
// outputSchema reference. The retry path bypasses the SDK's
|
||||||
|
// strict validator, so the mock returns the same tools list
|
||||||
|
// without the validation that originally threw.
|
||||||
|
if (req.method === "tools/list") return { tools: this._state?.tools ?? [] }
|
||||||
|
throw new Error(`unsupported request: ${req.method}`)
|
||||||
|
}
|
||||||
|
async listPrompts() {
|
||||||
|
return { prompts: [] }
|
||||||
|
}
|
||||||
|
async listResources() {
|
||||||
|
return { resources: [] }
|
||||||
|
}
|
||||||
|
async close() {
|
||||||
|
if (this._state) this._state.closed = true
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
clientStates.clear()
|
||||||
|
lastCreatedClientName = undefined
|
||||||
|
})
|
||||||
|
|
||||||
|
const { MCP } = await import("../../src/mcp/index")
|
||||||
|
const { Instance } = await import("../../src/project/instance")
|
||||||
|
const { WithInstance } = await import("../../src/project/with-instance")
|
||||||
|
const { tmpdir } = await import("../fixture/fixture")
|
||||||
|
|
||||||
|
function withInstance(
|
||||||
|
config: Record<string, unknown>,
|
||||||
|
fn: (mcp: MCPNS.Interface) => Effect.Effect<void, unknown, never>,
|
||||||
|
) {
|
||||||
|
return async () => {
|
||||||
|
await using tmp = await tmpdir({
|
||||||
|
init: async (dir) => {
|
||||||
|
await Bun.write(
|
||||||
|
`${dir}/opencode.json`,
|
||||||
|
JSON.stringify({
|
||||||
|
$schema: "https://opencode.ai/config.json",
|
||||||
|
mcp: config,
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
await WithInstance.provide({
|
||||||
|
directory: tmp.path,
|
||||||
|
fn: async () => {
|
||||||
|
await Effect.runPromise(MCP.Service.use(fn).pipe(Effect.provide(MCP.defaultLayer)))
|
||||||
|
await InstanceRuntime.disposeInstance(Instance.current)
|
||||||
|
},
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ========================================================================
|
||||||
|
// Reproducer: outputSchema with unresolved $ref fails the whole server
|
||||||
|
// ========================================================================
|
||||||
|
//
|
||||||
|
// In the real bug, the MCP SDK's response-validation layer attempts to
|
||||||
|
// resolve `$ref`s inside a tool's `outputSchema`. When a referenced
|
||||||
|
// definition is missing (e.g. `#/$defs/ScreenInstance`), validation
|
||||||
|
// throws something like:
|
||||||
|
//
|
||||||
|
// can't resolve reference #/$defs/ScreenInstance from id #
|
||||||
|
//
|
||||||
|
// `client.listTools()` therefore rejects, opencode's `defs()` catches
|
||||||
|
// the error and returns `undefined`, and `create()` then marks the whole
|
||||||
|
// MCP server as `failed` -- losing access to all other valid tools the
|
||||||
|
// server exposes.
|
||||||
|
//
|
||||||
|
// This test simulates the same failure path by making `listTools()`
|
||||||
|
// throw the same error, and asserts the server stays connected with its
|
||||||
|
// valid tool exposed.
|
||||||
|
|
||||||
|
test(
|
||||||
|
"tool with unresolved $ref in outputSchema does not fail the whole server",
|
||||||
|
withInstance(
|
||||||
|
{
|
||||||
|
"screen-server": {
|
||||||
|
type: "local",
|
||||||
|
command: ["echo", "test"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
(mcp) =>
|
||||||
|
Effect.gen(function* () {
|
||||||
|
lastCreatedClientName = "screen-server"
|
||||||
|
const serverState = getOrCreateClientState("screen-server")
|
||||||
|
|
||||||
|
// Simulate the SDK's validation throwing on the bad outputSchema.
|
||||||
|
// This is exactly what happens in the wild when one tool in
|
||||||
|
// tools/list has an `outputSchema` like:
|
||||||
|
// { $ref: "#/$defs/ScreenInstance" }
|
||||||
|
// with no `$defs` block to resolve against.
|
||||||
|
serverState.tools = [
|
||||||
|
{
|
||||||
|
name: "good_tool",
|
||||||
|
description: "valid tool that should still load",
|
||||||
|
inputSchema: { type: "object", properties: {} },
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "bad_tool",
|
||||||
|
description: "tool with unresolved outputSchema $ref",
|
||||||
|
inputSchema: { type: "object", properties: {} },
|
||||||
|
outputSchema: { $ref: "#/$defs/ScreenInstance" },
|
||||||
|
},
|
||||||
|
]
|
||||||
|
serverState.listToolsShouldFail = true
|
||||||
|
serverState.listToolsError = "can't resolve reference #/$defs/ScreenInstance from id #"
|
||||||
|
|
||||||
|
yield* mcp.add("screen-server", {
|
||||||
|
type: "local",
|
||||||
|
command: ["echo", "test"],
|
||||||
|
})
|
||||||
|
|
||||||
|
const status = yield* mcp.status()
|
||||||
|
|
||||||
|
// Expected: the server should remain connected because at least
|
||||||
|
// one tool (`good_tool`) has a valid schema.
|
||||||
|
expect(status["screen-server"]?.status).toBe("connected")
|
||||||
|
|
||||||
|
// Expected: the valid tool should be available even though one
|
||||||
|
// of the server's tools had a bad outputSchema.
|
||||||
|
const tools = yield* mcp.tools()
|
||||||
|
expect(Object.keys(tools).some((k) => k.includes("good_tool"))).toBe(true)
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
)
|
||||||
Loading…
Add table
Add a link
Reference in a new issue