fix(core): skip malformed MCP tools instead of failing the server (#58350)

This commit is contained in:
Aiden Cline
2026-10-24 12:45:34 -05:00
committed by GitHub
Unverified
parent 99845391b7
commit 2dc8db4a9d
2 changed files with 80 additions and 6 deletions
+36 -1
View File
@@ -10,6 +10,7 @@ import {
StreamableHTTPClientTransport,
UnauthorizedError,
UnsupportedProtocolVersionError,
isJSONRPCResultResponse,
type CallToolResult as SdkCallToolResult,
type ElicitRequestFormParams,
type ElicitRequestParams,
@@ -61,6 +62,14 @@ const isTransient = (error: unknown) =>
export type { GetPromptResult, Prompt, ReadResourceResult, Resource, Tool }
export type ResourceTemplate = ResourceTemplateType
// The era codec is protected; it checks a tools/list result against the exact schema the SDK applies
// for the negotiated protocol era, so a tool is skipped only when the SDK itself would reject it.
class ToolCheckingClient extends Client {
checkToolsList(result: unknown) {
return this._wireCodec().validateResult("tools/list", result)
}
}
export class NeedsAuthError extends Schema.TaggedError<NeedsAuthError>()("MCP.NeedsAuthError", {
server: Schema.String,
message: Schema.String,
@@ -198,7 +207,7 @@ export const connect = Effect.fnUntraced(function* (
const context = yield* Effect.context()
const runFork = Effect.runForkWith(context)
const runPromise = Effect.runPromiseWith(context)
const client = new Client(clientInfo, {
const client = new ToolCheckingClient(clientInfo, {
capabilities: {
...(elicitation ? { elicitation: { form: {}, url: {} } } : {}),
// Legacy era only: roots are deprecated as of 2026-07-28 and modern servers cannot request them.
@@ -228,6 +237,32 @@ export const connect = Effect.fnUntraced(function* (
)
}
// The SDK validates tools/list as a whole, so one malformed tool would fail every tool on the server.
// The SDK runs a handler set on the transport before connect ahead of its own, on the same message,
// so tools it would reject are dropped there and it still caches the rest. A page that fails for
// another reason is left for the SDK to report.
const onmessage = transport.onmessage
transport.onmessage = (message, extra) => {
if (
isJSONRPCResultResponse(message) &&
Array.isArray(message.result.tools) &&
client.checkToolsList({ ...message.result, tools: [] }).ok
)
message.result.tools = message.result.tools.filter((tool: unknown) => {
const outcome = client.checkToolsList({ ...message.result, tools: [tool] })
if (outcome.ok) return true
runFork(
Effect.logWarning("skipping invalid MCP tool", {
server,
tool: typeof tool === "object" && tool !== null && "name" in tool ? tool.name : undefined,
error: outcome.reason === "invalid" ? outcome.message : outcome.reason,
}),
)
return false
})
onmessage?.(message, extra)
}
yield* Effect.tryPromise({
try: (signal) => client.connect(transport, { timeout: timeout?.startup ?? DEFAULT_STARTUP_TIMEOUT, signal }),
catch: (error) => error,
+44 -5
View File
@@ -98,6 +98,11 @@ function resourceServer(
listChanged?: boolean
emptyElicitation?: boolean
urlElicitation?: boolean
/**
* List a tool whose input schema is not an object, and one whose `required` is a string, which
* only the 2026-07-28 schema accepts, ahead of the valid tools.
*/
malformedTool?: boolean
/** Legacy servers give up on the URL elicitation after this many milliseconds. */
elicitationTimeout?: number
respond?: (request: Request) => Response | undefined | Promise<Response | undefined>
@@ -147,11 +152,23 @@ function resourceServer(
protocol.setRequestHandler("tools/list", () => {
state.toolLists += 1
return Promise.resolve({
tools: input.emptyElicitation
? [{ name: "empty-elicitation", inputSchema: { type: "object" as const, properties: {} } }]
: input.urlElicitation
? [{ name: "url-elicitation", inputSchema: { type: "object" as const, properties: {} } }]
: [{ name: "echo", inputSchema: { type: "object" as const, properties: {} } }],
tools: [
// Servers send raw JSON; the cast lets the fixture send a tool the schema rejects.
...(input.malformedTool
? [
{ name: "malformed", inputSchema: { type: "string" } } as unknown as McpClient.Tool,
{
name: "modern-only",
inputSchema: { type: "object", required: "text" },
} as unknown as McpClient.Tool,
]
: []),
...(input.emptyElicitation
? [{ name: "empty-elicitation", inputSchema: { type: "object" as const, properties: {} } }]
: input.urlElicitation
? [{ name: "url-elicitation", inputSchema: { type: "object" as const, properties: {} } }]
: [{ name: "echo", inputSchema: { type: "object" as const, properties: {} } }]),
],
})
})
if (input.emptyElicitation) {
@@ -1182,6 +1199,28 @@ describe.each([
),
)
})
test("skips tools the SDK rejects and keeps the rest of the server's tools", async () => {
await Effect.runPromise(
Effect.scoped(
Effect.gen(function* () {
const server = yield* resourceServer({ modern: era === "modern", resources: false, malformedTool: true })
const connection = yield* connect(
"malformed",
new ConfigMCP.Remote({ type: "remote", url: server.url, oauth: false, protocol }),
import.meta.dir,
)
// Only tools the SDK itself would reject for the negotiated era are skipped.
expect((yield* connection.tools()).map((tool) => tool.name)).toEqual(
era === "modern" ? ["modern-only", "echo"] : ["echo"],
)
yield* connection.callTool({ name: "echo", args: { text: "hi" } })
expect(server.state.toolCalls.at(-1)).toMatchObject({ name: "echo", arguments: { text: "hi" } })
}),
),
)
})
})
test("lists, reads, and reports MCP resource changes", async () => {