diff --git a/packages/core/src/mcp/client.ts b/packages/core/src/mcp/client.ts index 3db0d64ea01..c638ddf6eff 100644 --- a/packages/core/src/mcp/client.ts +++ b/packages/core/src/mcp/client.ts @@ -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, diff --git a/packages/core/test/mcp.test.ts b/packages/core/test/mcp.test.ts index 7a9ad20640b..0312ee7f367 100644 --- a/packages/core/test/mcp.test.ts +++ b/packages/core/test/mcp.test.ts @@ -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"]) }), ), )