fix(core): don't fail MCP tool calls whose results break their output schema (#61254)

This commit is contained in:
Adam
2026-11-08 00:40:54 -06:00
committed by GitHub
Unverified
parent 6e96c5bbbc
commit 10bc5cbebb
2 changed files with 39 additions and 27 deletions
+10 -8
View File
@@ -419,9 +419,9 @@ export const connect = Effect.fnUntraced(function* (options: ConnectOptions) {
instructions: client.getInstructions()?.trim() || undefined,
// Tool lists skip the SDK response cache: it re-stringifies every list, and on the first tools/call
// re-parses it and compiles an output validator for every listed tool. Each call passes its own
// checked definition instead, so only that tool's outputSchema is compiled and modern Mcp-Param-*
// header mirroring still sees the schema. Tools are relisted on connect, list_changed and a stale
// header declaration; the SDK cache never relisted an expired list before a call either.
// checked definition instead, so modern Mcp-Param-* header mirroring still sees the input schema.
// Tools are relisted on connect, list_changed and a stale header declaration; the SDK cache never
// relisted an expired list before a call either.
tools: () =>
advertised("tools")
? list("list MCP tools", (signal) =>
@@ -522,11 +522,13 @@ export const connect = Effect.fnUntraced(function* (options: ConnectOptions) {
onprogress: () => {},
// Always passed, even for a tool we haven't listed, so the SDK never falls back to listing
// and caching the raw tools itself. Its types are narrower than what it reads (JSON values,
// a string `$schema`); ours keep the server's values.
toolDefinition: (listed.get(input.name) ?? {
name: input.name,
inputSchema: { type: "object" },
}) as CallToolRequestOptions["toolDefinition"],
// a string `$schema`); ours keep the server's values. Leaving out `outputSchema` skips the
// SDK's output check: servers return results that miss or break their own schema (Zoom
// answers a 204 with nothing), and failing the call would report a completed action as failed.
toolDefinition: {
...(listed.get(input.name) ?? { name: input.name, inputSchema: { type: "object" } }),
outputSchema: undefined,
} as CallToolRequestOptions["toolDefinition"],
},
),
catch: (error) => error,
+29 -19
View File
@@ -118,7 +118,10 @@ function resourceServer(
* malformed optional field of each kind, then one with every optional field valid.
*/
malformedTool?: boolean
/** Listed after the default tools. A call's `structured` argument comes back as structured content. */
/**
* Listed after the default tools. A call's `structured` argument comes back as structured content and
* its `text` argument as text content.
*/
tools?: ListToolsResult["tools"]
/** Legacy servers give up on the URL elicitation after this many milliseconds. */
elicitationTimeout?: number
@@ -269,9 +272,9 @@ function resourceServer(
progressToken: request.params._meta?.progressToken,
})
const structured = request.params.arguments?.structured
return Promise.resolve(
structured === undefined ? { content: [] } : { content: [], structuredContent: structured },
)
const text = request.params.arguments?.text
const content = typeof text === "string" ? [{ type: "text" as const, text }] : []
return Promise.resolve(structured === undefined ? { content } : { content, structuredContent: structured })
})
}
protocol.setRequestHandler("prompts/list", () => Promise.resolve({ prompts: [{ name: "greet" }] }))
@@ -1318,7 +1321,7 @@ describe.each([
)
})
test("validates structured content against the called tool's output schema", async () => {
test("returns results that miss or break the called tool's output schema", async () => {
await Effect.runPromise(
Effect.scoped(
Effect.gen(function* () {
@@ -1326,33 +1329,40 @@ describe.each([
modern: era === "modern",
resources: false,
tools: [
// Zoom's meeting_delete declares an empty object and returns nothing for the REST API's 204.
{ name: "delete", inputSchema: { type: "object" }, outputSchema: { type: "object", properties: {} } },
{
name: "typed",
name: "collaborators",
inputSchema: { type: "object" },
outputSchema: { type: "object", properties: { value: { type: "string" } }, required: ["value"] },
outputSchema: {
type: "object",
properties: { role: { type: "string", enum: ["owner", "editor"] } },
required: ["role"],
},
},
],
})
const connection = yield* connect(
"typed",
"lenient",
new ConfigMCP.Remote({ type: "remote", url: server.url, oauth: false, protocol }),
import.meta.dir,
)
yield* connection.tools()
expect(yield* connection.callTool({ name: "typed", args: { structured: { value: "ok" } } })).toMatchObject({
structured: { value: "ok" },
expect(yield* connection.callTool({ name: "delete", args: { text: "Meeting deleted." } })).toEqual({
isError: false,
structured: undefined,
content: [{ type: "text", text: "Meeting deleted." }],
})
expect(yield* connection.callTool({ name: "delete" })).toEqual({
isError: false,
structured: undefined,
content: [],
})
expect(
(yield* connection.callTool({ name: "typed", args: { structured: { value: 42 } } }).pipe(Effect.flip))
.message,
).toContain("Structured content does not match the tool's output schema")
expect((yield* connection.callTool({ name: "typed" }).pipe(Effect.flip)).message).toContain(
"has an output schema but did not return structured content",
)
expect(yield* connection.callTool({ name: "echo", args: { structured: { value: 42 } } })).toMatchObject({
structured: { value: 42 },
})
yield* connection.callTool({ name: "collaborators", args: { structured: { role: "commenter" } } }),
).toMatchObject({ isError: false, structured: { role: "commenter" } })
expect(server.state.toolCalls.map((call) => call.name)).toEqual(["delete", "delete", "collaborators"])
}),
),
)