From efd901a93002b1cbd677354f0a8e7c0c5a7d0586 Mon Sep 17 00:00:00 2001 From: Konstantin Konstantinov Date: Fri, 24 Oct 2025 21:40:04 +0300 Subject: [PATCH 1/3] Non-existing tool, disabled tool and tool inputSchema validation return CallToolResult instead of a protocol-level error --- src/server/mcp.test.ts | 96 ++++++++++++++++++++++++++---------------- src/server/mcp.ts | 82 ++++++++++++++++++------------------ 2 files changed, 101 insertions(+), 77 deletions(-) diff --git a/src/server/mcp.test.ts b/src/server/mcp.test.ts index 4bb42d7fc..c48c1fec2 100644 --- a/src/server/mcp.test.ts +++ b/src/server/mcp.test.ts @@ -897,37 +897,53 @@ describe('tool()', () => { await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); - await expect( - client.request( - { - method: 'tools/call', - params: { + const result = await client.request( + { + method: 'tools/call', + params: { + name: 'test', + arguments: { name: 'test', - arguments: { - name: 'test', - value: 'not a number' - } + value: 'not a number' } - }, - CallToolResultSchema - ) - ).rejects.toThrow(/Invalid arguments/); + } + }, + CallToolResultSchema + ); - await expect( - client.request( + expect(result.isError).toBe(true); + expect(result.content).toEqual( + expect.arrayContaining([ { - method: 'tools/call', - params: { - name: 'test (new api)', - arguments: { - name: 'test', - value: 'not a number' - } + type: 'text', + text: expect.stringContaining('Invalid arguments') + } + ]) + ); + + const result2 = await client.request( + { + method: 'tools/call', + params: { + name: 'test (new api)', + arguments: { + name: 'test', + value: 'not a number' } - }, - CallToolResultSchema - ) - ).rejects.toThrow(/Invalid arguments/); + } + }, + CallToolResultSchema + ); + + expect(result2.isError).toBe(true); + expect(result2.content).toEqual( + expect.arrayContaining([ + { + type: 'text', + text: expect.stringContaining('Invalid arguments') + } + ]) + ); }); /*** @@ -1518,17 +1534,25 @@ describe('tool()', () => { await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); - await expect( - client.request( + const result = await client.request( + { + method: 'tools/call', + params: { + name: 'nonexistent-tool' + } + }, + CallToolResultSchema + ); + + expect(result.isError).toBe(true); + expect(result.content).toEqual( + expect.arrayContaining([ { - method: 'tools/call', - params: { - name: 'nonexistent-tool' - } - }, - CallToolResultSchema - ) - ).rejects.toThrow(/Tool nonexistent-tool not found/); + type: 'text', + text: expect.stringContaining('Tool nonexistent-tool not found') + } + ]) + ); }); /*** diff --git a/src/server/mcp.ts b/src/server/mcp.ts index 765ba864f..d85015416 100644 --- a/src/server/mcp.ts +++ b/src/server/mcp.ts @@ -126,55 +126,37 @@ export class McpServer { this.server.setRequestHandler(CallToolRequestSchema, async (request, extra): Promise => { const tool = this._registeredTools[request.params.name]; - if (!tool) { - throw new McpError(ErrorCode.InvalidParams, `Tool ${request.params.name} not found`); - } - - if (!tool.enabled) { - throw new McpError(ErrorCode.InvalidParams, `Tool ${request.params.name} disabled`); - } let result: CallToolResult; - if (tool.inputSchema) { - const parseResult = await tool.inputSchema.safeParseAsync(request.params.arguments); - if (!parseResult.success) { - throw new McpError( - ErrorCode.InvalidParams, - `Invalid arguments for tool ${request.params.name}: ${parseResult.error.message}` - ); + try { + if (!tool) { + throw new McpError(ErrorCode.InvalidParams, `Tool ${request.params.name} not found`); } - const args = parseResult.data; - const cb = tool.callback as ToolCallback; - try { - result = await Promise.resolve(cb(args, extra)); - } catch (error) { - result = { - content: [ - { - type: 'text', - text: error instanceof Error ? error.message : String(error) - } - ], - isError: true - }; + if (!tool.enabled) { + throw new McpError(ErrorCode.InvalidParams, `Tool ${request.params.name} disabled`); } - } else { - const cb = tool.callback as ToolCallback; - try { + + if (tool.inputSchema) { + const cb = tool.callback as ToolCallback; + const parseResult = await tool.inputSchema.safeParseAsync(request.params.arguments); + if (!parseResult.success) { + throw new McpError( + ErrorCode.InvalidParams, + `Invalid arguments for tool ${request.params.name}: ${parseResult.error.message}` + ); + } + + const args = parseResult.data; + + result = await Promise.resolve(cb(args, extra)); + } else { + const cb = tool.callback as ToolCallback; result = await Promise.resolve(cb(extra)); - } catch (error) { - result = { - content: [ - { - type: 'text', - text: error instanceof Error ? error.message : String(error) - } - ], - isError: true - }; } + } catch (error) { + return this.createToolError(error instanceof Error ? error.message : String(error)); } if (tool.outputSchema && !result.isError) { @@ -201,6 +183,24 @@ export class McpServer { this._toolHandlersInitialized = true; } + /** + * Creates a tool error result. + * + * @param errorMessage - The error message. + * @returns The tool error result. + */ + private createToolError(errorMessage: string): CallToolResult { + return { + content: [ + { + type: 'text', + text: errorMessage + } + ], + isError: true + }; + } + private _completionHandlerInitialized = false; private setCompletionRequestHandler() { From f02ff1af6da97412b21be48cf909b83101e8564b Mon Sep 17 00:00:00 2001 From: Konstantin Konstantinov Date: Tue, 28 Oct 2025 15:10:58 +0200 Subject: [PATCH 2/3] outputSchema fix - return CallToolResult on structuredContent not returned or structuredContent validation error --- package-lock.json | 10 +--------- src/server/mcp.test.ts | 37 +++++++++++++++++++++++++++---------- src/server/mcp.ts | 36 +++++++++++++++++++----------------- 3 files changed, 47 insertions(+), 36 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0f614d70e..2736131cd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -52,19 +52,11 @@ "node": ">=18" }, "peerDependencies": { - "@cfworker/json-schema": "^4.1.1", - "ajv": "^8.17.1", - "ajv-formats": "^3.0.1" + "@cfworker/json-schema": "^4.1.1" }, "peerDependenciesMeta": { "@cfworker/json-schema": { "optional": true - }, - "ajv": { - "optional": true - }, - "ajv-formats": { - "optional": true } } }, diff --git a/src/server/mcp.test.ts b/src/server/mcp.test.ts index 2d514081c..11d338cd6 100644 --- a/src/server/mcp.test.ts +++ b/src/server/mcp.test.ts @@ -947,7 +947,7 @@ describe('tool()', () => { expect.arrayContaining([ { type: 'text', - text: expect.stringContaining('Invalid arguments') + text: expect.stringContaining('Input validation error: Invalid arguments for tool test') } ]) ); @@ -971,7 +971,7 @@ describe('tool()', () => { expect.arrayContaining([ { type: 'text', - text: expect.stringContaining('Invalid arguments') + text: expect.stringContaining('Input validation error: Invalid arguments for tool test (new api)') } ]) ); @@ -1168,14 +1168,23 @@ describe('tool()', () => { await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); // Call the tool and expect it to throw an error - await expect( - client.callTool({ + const result = + await client.callTool({ name: 'test', arguments: { input: 'hello' } - }) - ).rejects.toThrow(/Tool test has an output schema but no structured content was provided/); + }); + + expect(result.isError).toBe(true); + expect(result.content).toEqual( + expect.arrayContaining([ + { + type: 'text', + text: expect.stringContaining('Output validation error: Tool test has an output schema but no structured content was provided') + } + ]) + ); }); /*** * Test: Tool with Output Schema Must Provide Structured Content @@ -1290,14 +1299,22 @@ describe('tool()', () => { await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); // Call the tool and expect it to throw a server-side validation error - await expect( - client.callTool({ + const result = await client.callTool({ name: 'test', arguments: { input: 'hello' } - }) - ).rejects.toThrow(/Invalid structured content for tool test/); + }); + + expect(result.isError).toBe(true); + expect(result.content).toEqual( + expect.arrayContaining([ + { + type: 'text', + text: expect.stringContaining('Output validation error: Invalid structured content for tool test') + } + ]) + ); }); /*** diff --git a/src/server/mcp.ts b/src/server/mcp.ts index d85015416..09639190c 100644 --- a/src/server/mcp.ts +++ b/src/server/mcp.ts @@ -144,7 +144,7 @@ export class McpServer { if (!parseResult.success) { throw new McpError( ErrorCode.InvalidParams, - `Invalid arguments for tool ${request.params.name}: ${parseResult.error.message}` + `Input validation error: Invalid arguments for tool ${request.params.name}: ${parseResult.error.message}` ); } @@ -155,27 +155,29 @@ export class McpServer { const cb = tool.callback as ToolCallback; result = await Promise.resolve(cb(extra)); } + + if (tool.outputSchema && !result.isError) { + if (!result.structuredContent) { + throw new McpError( + ErrorCode.InvalidParams, + `Output validation error: Tool ${request.params.name} has an output schema but no structured content was provided` + ); + } + + // if the tool has an output schema, validate structured content + const parseResult = await tool.outputSchema.safeParseAsync(result.structuredContent); + if (!parseResult.success) { + throw new McpError( + ErrorCode.InvalidParams, + `Output validation error: Invalid structured content for tool ${request.params.name}: ${parseResult.error.message}` + ); + } + } } catch (error) { return this.createToolError(error instanceof Error ? error.message : String(error)); } - if (tool.outputSchema && !result.isError) { - if (!result.structuredContent) { - throw new McpError( - ErrorCode.InvalidParams, - `Tool ${request.params.name} has an output schema but no structured content was provided` - ); - } - // if the tool has an output schema, validate structured content - const parseResult = await tool.outputSchema.safeParseAsync(result.structuredContent); - if (!parseResult.success) { - throw new McpError( - ErrorCode.InvalidParams, - `Invalid structured content for tool ${request.params.name}: ${parseResult.error.message}` - ); - } - } return result; }); From 98a69ba761ba630b9292881a7435a4b67458b709 Mon Sep 17 00:00:00 2001 From: Konstantin Konstantinov Date: Tue, 28 Oct 2025 15:15:14 +0200 Subject: [PATCH 3/3] prettier fix --- src/server/mcp.test.ts | 27 ++++++++++++++------------- src/server/mcp.ts | 4 +--- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/server/mcp.test.ts b/src/server/mcp.test.ts index 11d338cd6..f3669fa64 100644 --- a/src/server/mcp.test.ts +++ b/src/server/mcp.test.ts @@ -1168,20 +1168,21 @@ describe('tool()', () => { await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); // Call the tool and expect it to throw an error - const result = - await client.callTool({ - name: 'test', - arguments: { - input: 'hello' - } - }); + const result = await client.callTool({ + name: 'test', + arguments: { + input: 'hello' + } + }); expect(result.isError).toBe(true); expect(result.content).toEqual( expect.arrayContaining([ { type: 'text', - text: expect.stringContaining('Output validation error: Tool test has an output schema but no structured content was provided') + text: expect.stringContaining( + 'Output validation error: Tool test has an output schema but no structured content was provided' + ) } ]) ); @@ -1300,11 +1301,11 @@ describe('tool()', () => { // Call the tool and expect it to throw a server-side validation error const result = await client.callTool({ - name: 'test', - arguments: { - input: 'hello' - } - }); + name: 'test', + arguments: { + input: 'hello' + } + }); expect(result.isError).toBe(true); expect(result.content).toEqual( diff --git a/src/server/mcp.ts b/src/server/mcp.ts index 09639190c..fb93bd326 100644 --- a/src/server/mcp.ts +++ b/src/server/mcp.ts @@ -163,7 +163,7 @@ export class McpServer { `Output validation error: Tool ${request.params.name} has an output schema but no structured content was provided` ); } - + // if the tool has an output schema, validate structured content const parseResult = await tool.outputSchema.safeParseAsync(result.structuredContent); if (!parseResult.success) { @@ -177,8 +177,6 @@ export class McpServer { return this.createToolError(error instanceof Error ? error.message : String(error)); } - - return result; });