From 7641e9638816f4d1a031417a27c96ba4aa4adc9d Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Thu, 28 May 2026 17:15:46 +0800 Subject: [PATCH] fix: return protocol errors for invalid tool args --- packages/server/src/server/mcp.ts | 7 +- .../server/test/server/mcp.compat.test.ts | 44 +++- test/integration/test/server/mcp.test.ts | 123 +++++----- test/integration/test/standardSchema.test.ts | 230 +++++++++--------- 4 files changed, 219 insertions(+), 185 deletions(-) diff --git a/packages/server/src/server/mcp.ts b/packages/server/src/server/mcp.ts index fb45fd5db6..8ecb25f21d 100644 --- a/packages/server/src/server/mcp.ts +++ b/packages/server/src/server/mcp.ts @@ -208,8 +208,11 @@ export class McpServer { await this.validateToolOutput(tool, result, request.params.name); return result; } catch (error) { - if (error instanceof ProtocolError && error.code === ProtocolErrorCode.UrlElicitationRequired) { - throw error; // Return the error to the caller without wrapping in CallToolResult + if ( + error instanceof ProtocolError && + (error.code === ProtocolErrorCode.UrlElicitationRequired || error.message.startsWith('Input validation error:')) + ) { + throw error; } return this.createToolError(error instanceof Error ? error.message : String(error)); } diff --git a/packages/server/test/server/mcp.compat.test.ts b/packages/server/test/server/mcp.compat.test.ts index 322b615353..5b764f72d7 100644 --- a/packages/server/test/server/mcp.compat.test.ts +++ b/packages/server/test/server/mcp.compat.test.ts @@ -1,5 +1,5 @@ import type { JSONRPCMessage } from '@modelcontextprotocol/core'; -import { InMemoryTransport, isStandardSchema, LATEST_PROTOCOL_VERSION } from '@modelcontextprotocol/core'; +import { InMemoryTransport, isStandardSchema, LATEST_PROTOCOL_VERSION, ProtocolErrorCode } from '@modelcontextprotocol/core'; import { describe, expect, expectTypeOf, it, vi } from 'vitest'; import * as z from 'zod/v4'; import { McpServer } from '../../src/index.js'; @@ -119,6 +119,48 @@ describe('registerTool/registerPrompt accept raw Zod shape (auto-wrapped)', () = await server.close(); }); + + it('returns a protocol error for invalid tool arguments', async () => { + const server = new McpServer({ name: 't', version: '1.0.0' }); + + server.registerTool('echo', { inputSchema: { x: z.number() } }, async ({ x }) => ({ + content: [{ type: 'text' as const, text: String(x) }] + })); + + const [client, srv] = InMemoryTransport.createLinkedPair(); + await server.connect(srv); + await client.start(); + + const responses: JSONRPCMessage[] = []; + client.onmessage = m => responses.push(m); + + await client.send({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { + protocolVersion: LATEST_PROTOCOL_VERSION, + capabilities: {}, + clientInfo: { name: 'c', version: '1.0.0' } + } + } as JSONRPCMessage); + await client.send({ jsonrpc: '2.0', method: 'notifications/initialized' } as JSONRPCMessage); + await client.send({ + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'echo', arguments: { x: 'nope' } } + } as JSONRPCMessage); + + await vi.waitFor(() => expect(responses.some(r => 'id' in r && r.id === 2)).toBe(true)); + + const response = responses.find(r => 'id' in r && r.id === 2) as { error?: { code: number; message: string }; result?: unknown }; + expect(response.result).toBeUndefined(); + expect(response.error?.code).toBe(ProtocolErrorCode.InvalidParams); + expect(response.error?.message).toContain('Invalid arguments for tool echo'); + + await server.close(); + }); }); describe('InferRawShape', () => { diff --git a/test/integration/test/server/mcp.test.ts b/test/integration/test/server/mcp.test.ts index 92af09744c..c9e7dce109 100644 --- a/test/integration/test/server/mcp.test.ts +++ b/test/integration/test/server/mcp.test.ts @@ -4,6 +4,7 @@ import { getDisplayName, InMemoryTaskStore, InMemoryTransport, + ProtocolError, ProtocolErrorCode, UriTemplate, UrlElicitationRequiredError @@ -28,6 +29,26 @@ function createLatch() { }; } +async function expectInvalidToolArguments(request: Promise, expectedMessages: Array) { + let error: unknown; + try { + await request; + } catch (error_) { + error = error_; + } + + expect(error).toBeInstanceOf(ProtocolError); + const protocolError = error as ProtocolError; + expect(protocolError.code).toBe(ProtocolErrorCode.InvalidParams); + for (const expected of expectedMessages) { + if (typeof expected === 'string') { + expect(protocolError.message).toContain(expected); + } else { + expect(protocolError.message).toMatch(expected); + } + } +} + describe('Zod v4', () => { describe('McpServer', () => { /*** @@ -1212,25 +1233,18 @@ describe('Zod v4', () => { await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); - const result = await client.request({ - method: 'tools/call', - params: { - name: 'test', - arguments: { + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'test', - value: 'not a number' - } - } - }); - - expect(result.isError).toBe(true); - expect(result.content).toEqual( - expect.arrayContaining([ - { - type: 'text', - text: expect.stringContaining('Input validation error: Invalid arguments for tool test') + arguments: { + name: 'test', + value: 'not a number' + } } - ]) + }), + ['Input validation error: Invalid arguments for tool test'] ); }); @@ -5149,22 +5163,15 @@ describe('Zod v4', () => { await server.connect(serverTransport); await client.connect(clientTransport); - const invalidTypeResult = await client.callTool({ - name: 'union-test', - arguments: { - type: 'a', - value: 123 - } - }); - - expect(invalidTypeResult.isError).toBe(true); - expect(invalidTypeResult.content).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - type: 'text', - text: expect.stringContaining('Input validation error') - }) - ]) + await expectInvalidToolArguments( + client.callTool({ + name: 'union-test', + arguments: { + type: 'a', + value: 123 + } + }), + ['Input validation error'] ); }); }); @@ -6407,40 +6414,26 @@ describe('Zod v4', () => { await server.connect(serverTransport); await client.connect(clientTransport); - const invalidTypeResult = await client.callTool({ - name: 'union-test', - arguments: { - type: 'a', - value: 123 - } - }); - - expect(invalidTypeResult.isError).toBe(true); - expect(invalidTypeResult.content).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - type: 'text', - text: expect.stringContaining('Input validation error') - }) - ]) + await expectInvalidToolArguments( + client.callTool({ + name: 'union-test', + arguments: { + type: 'a', + value: 123 + } + }), + ['Input validation error'] ); - const invalidDiscriminatorResult = await client.callTool({ - name: 'union-test', - arguments: { - type: 'c', - value: 'test' - } - }); - - expect(invalidDiscriminatorResult.isError).toBe(true); - expect(invalidDiscriminatorResult.content).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - type: 'text', - text: expect.stringContaining('Input validation error') - }) - ]) + await expectInvalidToolArguments( + client.callTool({ + name: 'union-test', + arguments: { + type: 'c', + value: 'test' + } + }), + ['Input validation error'] ); }); }); diff --git a/test/integration/test/standardSchema.test.ts b/test/integration/test/standardSchema.test.ts index 67f16c5fa7..e8b4481cb2 100644 --- a/test/integration/test/standardSchema.test.ts +++ b/test/integration/test/standardSchema.test.ts @@ -5,7 +5,7 @@ import { Client } from '@modelcontextprotocol/client'; import type { TextContent } from '@modelcontextprotocol/core'; -import { AjvJsonSchemaValidator, fromJsonSchema, InMemoryTransport } from '@modelcontextprotocol/core'; +import { AjvJsonSchemaValidator, fromJsonSchema, InMemoryTransport, ProtocolError, ProtocolErrorCode } from '@modelcontextprotocol/core'; import { completable, fromJsonSchema as serverFromJsonSchema, McpServer } from '@modelcontextprotocol/server'; import { toStandardJsonSchema } from '@valibot/to-json-schema'; import { type } from 'arktype'; @@ -33,6 +33,26 @@ describe('Standard Schema Support', () => { await Promise.all([client.connect(clientTransport), mcpServer.connect(serverTransport)]); } + async function expectInvalidToolArguments(request: Promise, expectedMessages: Array) { + let error: unknown; + try { + await request; + } catch (error_) { + error = error_; + } + + expect(error).toBeInstanceOf(ProtocolError); + const protocolError = error as ProtocolError; + expect(protocolError.code).toBe(ProtocolErrorCode.InvalidParams); + for (const expected of expectedMessages) { + if (typeof expected === 'string') { + expect(protocolError.message).toContain(expected); + } else { + expect(protocolError.message).toMatch(expected); + } + } + } + describe('ArkType schemas', () => { describe('tool registration', () => { test('should register tool with ArkType input schema', async () => { @@ -130,16 +150,13 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { name: 'double', arguments: { value: 'not a number' } } - }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); - expect(errorText).toContain('value'); - expect(errorText).toContain('number'); + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'double', arguments: { value: 'not a number' } } + }), + ['Input validation error', 'value', 'number'] + ); }); test('should return validation error for invalid enum value', async () => { @@ -153,15 +170,13 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { name: 'calculate', arguments: { operation: 'divide' } } - }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); - expect(errorText).toMatch(/add|subtract|multiply/); + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'calculate', arguments: { operation: 'divide' } } + }), + ['Input validation error', /add|subtract|multiply/] + ); }); test('should return validation error for missing required field', async () => { @@ -173,15 +188,13 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { name: 'greet', arguments: { name: 'Alice' } } - }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); - expect(errorText).toContain('age'); + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'greet', arguments: { name: 'Alice' } } + }), + ['Input validation error', 'age'] + ); }); }); }); @@ -273,15 +286,13 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { name: 'double', arguments: { value: 'not a number' } } - }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); - expect(errorText).toContain('number'); + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'double', arguments: { value: 'not a number' } } + }), + ['Input validation error', 'number'] + ); }); test('should return validation error for invalid picklist value', async () => { @@ -297,14 +308,13 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { name: 'calculate', arguments: { operation: 'divide' } } - }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'calculate', arguments: { operation: 'divide' } } + }), + ['Input validation error'] + ); }); test('should validate min/max constraints', async () => { @@ -328,13 +338,13 @@ describe('Standard Schema Support', () => { expect(validResult.isError).toBeFalsy(); // Invalid value (too high) - const invalidResult = await client.request({ - method: 'tools/call', - params: { name: 'setPercentage', arguments: { percentage: 150 } } - }); - expect(invalidResult.isError).toBe(true); - const errorText = (invalidResult.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'setPercentage', arguments: { percentage: 150 } } + }), + ['Input validation error'] + ); }); }); }); @@ -420,11 +430,10 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ method: 'tools/call', params: { name: 'double', arguments: { count: 'not a number' } } }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); + await expectInvalidToolArguments( + client.request({ method: 'tools/call', params: { name: 'double', arguments: { count: 'not a number' } } }), + ['Input validation error'] + ); }); }); @@ -456,13 +465,13 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { name: 'double-default', arguments: { count: 'not a number' } } - }); - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'double-default', arguments: { count: 'not a number' } } + }), + ['Input validation error'] + ); }); }); @@ -595,25 +604,20 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { - name: 'test', - arguments: { - email: 123, - age: 'not a number', - status: 'unknown' + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { + name: 'test', + arguments: { + email: 123, + age: 'not a number', + status: 'unknown' + } } - } - }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - - // Check that error mentions the specific issues - expect(errorText).toContain('Input validation error'); - // ArkType should mention type mismatches - expect(errorText).toMatch(/email|age|status/i); + }), + ['Input validation error', /email|age|status/i] + ); }); test('Valibot should provide descriptive error messages', async () => { @@ -631,25 +635,20 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { - name: 'test', - arguments: { - email: 123, - age: 'not a number', - status: 'unknown' + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { + name: 'test', + arguments: { + email: 123, + age: 'not a number', + status: 'unknown' + } } - } - }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - - // Check that error mentions the specific issues - expect(errorText).toContain('Input validation error'); - // Valibot should provide "Invalid type" messages - expect(errorText).toContain('Invalid type'); + }), + ['Input validation error', 'Invalid type'] + ); }); test('Zod should provide descriptive error messages', async () => { @@ -665,23 +664,20 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ - method: 'tools/call', - params: { - name: 'test', - arguments: { - email: 123, - age: 'not a number', - status: 'unknown' + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { + name: 'test', + arguments: { + email: 123, + age: 'not a number', + status: 'unknown' + } } - } - }); - - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - - // Check that error mentions the specific issues - expect(errorText).toContain('Input validation error'); + }), + ['Input validation error'] + ); }); });