From e76faf6a18bcd25724649a57b07c410957510192 Mon Sep 17 00:00:00 2001 From: joeyzzeng Date: Wed, 18 Jun 2025 21:34:42 +0800 Subject: [PATCH 1/2] fix: skip validation if tool reports error --- src/server/mcp.test.ts | 66 ++++++++++++++++++++++++++++++++++++++++++ src/server/mcp.ts | 2 +- 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/server/mcp.test.ts b/src/server/mcp.test.ts index 7fb6bd55c..242f05297 100644 --- a/src/server/mcp.test.ts +++ b/src/server/mcp.test.ts @@ -1204,6 +1204,72 @@ describe("tool()", () => { ).rejects.toThrow(/Tool test has an output schema but no structured content was provided/); }); + /*** + * Test: Tool with Output Schema Must Provide Structured Content + */ + test("should not throw error when tool with outputSchema returns no structuredContent and isError is true", async () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + const client = new Client({ + name: "test client", + version: "1.0", + }); + + // Register a tool with outputSchema that returns only content without structuredContent + mcpServer.registerTool( + "test", + { + description: "Test tool with output schema but missing structured content", + inputSchema: { + input: z.string(), + }, + outputSchema: { + processedInput: z.string(), + resultType: z.string(), + }, + }, + async ({ input }) => ({ + // Only return content without structuredContent + content: [ + { + type: "text", + text: `Processed: ${input}`, + }, + ], + isError: true, + }) + ); + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + await Promise.all([ + client.connect(clientTransport), + mcpServer.server.connect(serverTransport), + ]); + + // Call the tool and expect it to not throw an error + await expect( + client.callTool({ + name: "test", + arguments: { + input: "hello", + }, + }), + ).resolves.toStrictEqual({ + content: [ + { + type: "text", + text: `Processed: hello`, + }, + ], + isError: true, + }); + }); + /*** * Test: Schema Validation Failure for Invalid Structured Content */ diff --git a/src/server/mcp.ts b/src/server/mcp.ts index 3d9673da7..9440708d9 100644 --- a/src/server/mcp.ts +++ b/src/server/mcp.ts @@ -200,7 +200,7 @@ export class McpServer { } } - if (tool.outputSchema) { + if (tool.outputSchema && (result.isError !== true)) { if (!result.structuredContent) { throw new McpError( ErrorCode.InvalidParams, From c20a47a79f38f617ffd9ef0df1106651891d9ea7 Mon Sep 17 00:00:00 2001 From: ihrpr Date: Tue, 24 Jun 2025 17:09:29 +0100 Subject: [PATCH 2/2] small fixes --- src/server/mcp.test.ts | 108 ++++++++++++++++++++--------------------- src/server/mcp.ts | 2 +- 2 files changed, 53 insertions(+), 57 deletions(-) diff --git a/src/server/mcp.test.ts b/src/server/mcp.test.ts index 242f05297..e09ab5117 100644 --- a/src/server/mcp.test.ts +++ b/src/server/mcp.test.ts @@ -1203,72 +1203,68 @@ describe("tool()", () => { }), ).rejects.toThrow(/Tool test has an output schema but no structured content was provided/); }); - - /*** + /*** * Test: Tool with Output Schema Must Provide Structured Content */ - test("should not throw error when tool with outputSchema returns no structuredContent and isError is true", async () => { - const mcpServer = new McpServer({ - name: "test server", - version: "1.0", - }); - - const client = new Client({ - name: "test client", - version: "1.0", - }); - - // Register a tool with outputSchema that returns only content without structuredContent - mcpServer.registerTool( - "test", - { - description: "Test tool with output schema but missing structured content", - inputSchema: { - input: z.string(), - }, - outputSchema: { - processedInput: z.string(), - resultType: z.string(), - }, + test("should skip outputSchema validation when isError is true", async () => { + const mcpServer = new McpServer({ + name: "test server", + version: "1.0", + }); + + const client = new Client({ + name: "test client", + version: "1.0", + }); + + mcpServer.registerTool( + "test", + { + description: "Test tool with output schema but missing structured content", + inputSchema: { + input: z.string(), }, - async ({ input }) => ({ - // Only return content without structuredContent - content: [ - { - type: "text", - text: `Processed: ${input}`, - }, - ], - isError: true, - }) - ); - - const [clientTransport, serverTransport] = - InMemoryTransport.createLinkedPair(); - - await Promise.all([ - client.connect(clientTransport), - mcpServer.server.connect(serverTransport), - ]); - - // Call the tool and expect it to not throw an error - await expect( - client.callTool({ - name: "test", - arguments: { - input: "hello", - }, - }), - ).resolves.toStrictEqual({ + outputSchema: { + processedInput: z.string(), + resultType: z.string(), + }, + }, + async ({ input }) => ({ content: [ { type: "text", - text: `Processed: hello`, + text: `Processed: ${input}`, }, ], isError: true, - }); + }) + ); + + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + + await Promise.all([ + client.connect(clientTransport), + mcpServer.server.connect(serverTransport), + ]); + + await expect( + client.callTool({ + name: "test", + arguments: { + input: "hello", + }, + }), + ).resolves.toStrictEqual({ + content: [ + { + type: "text", + text: `Processed: hello`, + }, + ], + isError: true, }); + }); /*** * Test: Schema Validation Failure for Invalid Structured Content diff --git a/src/server/mcp.ts b/src/server/mcp.ts index 9440708d9..67da78ffb 100644 --- a/src/server/mcp.ts +++ b/src/server/mcp.ts @@ -200,7 +200,7 @@ export class McpServer { } } - if (tool.outputSchema && (result.isError !== true)) { + if (tool.outputSchema && !result.isError) { if (!result.structuredContent) { throw new McpError( ErrorCode.InvalidParams,