From 5ef13fd54b1b1eefd24e7f5bd8227c5ac6774c60 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 --- .changeset/tool-argument-protocol-errors.md | 5 + packages/server/src/server/mcp.ts | 7 +- .../server/test/server/mcp.compat.test.ts | 44 ++- test/e2e/requirements.ts | 19 +- test/e2e/scenarios/tools.test.ts | 36 +-- test/integration/test/server/mcp.test.ts | 131 +++++---- test/integration/test/standardSchema.test.ts | 252 ++++++++++-------- 7 files changed, 286 insertions(+), 208 deletions(-) create mode 100644 .changeset/tool-argument-protocol-errors.md diff --git a/.changeset/tool-argument-protocol-errors.md b/.changeset/tool-argument-protocol-errors.md new file mode 100644 index 0000000000..f20c6c6bcb --- /dev/null +++ b/.changeset/tool-argument-protocol-errors.md @@ -0,0 +1,5 @@ +--- +"@modelcontextprotocol/server": patch +--- + +Return protocol errors for invalid tool arguments. diff --git a/packages/server/src/server/mcp.ts b/packages/server/src/server/mcp.ts index 40ec8bb1eb..30aaf5c124 100644 --- a/packages/server/src/server/mcp.ts +++ b/packages/server/src/server/mcp.ts @@ -153,8 +153,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/e2e/requirements.ts b/test/e2e/requirements.ts index ea471a21fc..1716a45611 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -452,8 +452,7 @@ export const REQUIREMENTS: Record = { }, 'mcpserver:tool:input-validation': { source: 'sdk', - behavior: - "Arguments that fail the tool's input validation produce a tool execution error (isError true with the validation failure described in content) without invoking the function." + behavior: "Arguments that fail the tool's input validation are rejected with JSON-RPC -32602 without invoking the function." }, 'mcpserver:tool:naming-validation': { source: 'sdk', @@ -467,7 +466,7 @@ export const REQUIREMENTS: Record = { 'typescript:mcpserver:tool:schema-variants': { source: 'sdk', behavior: - 'inputSchema accepts Zod union, intersection, nested-object, preprocess, transform, and pipe schemas; validation/coercion runs before the handler.' + 'inputSchema accepts Zod union, intersection, nested-object, preprocess, transform, and pipe schemas; validation/coercion runs before the handler, and invalid arguments reject with JSON-RPC -32602.' }, 'client:call-tool:compat-result-schema': { source: 'sdk', @@ -2109,12 +2108,7 @@ export const REQUIREMENTS: Record = { 'standardschema:tool:invalid-args-rejected': { source: 'sdk', behavior: - 'tools/call arguments that fail the registered Standard Schema validation are rejected with JSON-RPC -32602 (Input validation error) and the tool handler is not invoked.', - knownFailures: [ - { - note: "McpServer's tools/call handler catches the input-validation ProtocolError (-32602) and returns it as an isError result, so callTool() resolves instead of rejecting; the handler is still not invoked." - } - ] + 'tools/call arguments that fail the registered Standard Schema validation are rejected with JSON-RPC -32602 (Input validation error) and the tool handler is not invoked.' }, 'validators:from-json-schema:tool-roundtrip': { source: 'sdk', @@ -2124,12 +2118,7 @@ export const REQUIREMENTS: Record = { 'validators:from-json-schema:invalid-args-rejected': { source: 'sdk', behavior: - 'tools/call arguments violating the JSON Schema wrapped by fromJsonSchema() are rejected with JSON-RPC -32602 and the handler is not invoked.', - knownFailures: [ - { - note: "McpServer's tools/call handler catches the input-validation ProtocolError (-32602) and returns it as an isError result, so callTool() resolves instead of rejecting; the handler is still not invoked." - } - ] + 'tools/call arguments violating the JSON Schema wrapped by fromJsonSchema() are rejected with JSON-RPC -32602 and the handler is not invoked.' }, 'validators:custom-validator:override': { source: 'sdk', diff --git a/test/e2e/scenarios/tools.test.ts b/test/e2e/scenarios/tools.test.ts index 408712f23a..4b0abddca0 100644 --- a/test/e2e/scenarios/tools.test.ts +++ b/test/e2e/scenarios/tools.test.ts @@ -1064,14 +1064,16 @@ verifies('mcpserver:tool:input-validation', async ({ transport }: TestArgs) => { expect(ok.isError).toBeFalsy(); expect(handlerCalls.n).toBe(1); - const wrongType = await client.callTool({ name: 'typed', arguments: { prompt: 123 } }); - expect(wrongType.isError).toBe(true); - expect(wrongType.content).toEqual([{ type: 'text', text: expect.stringMatching(/invalid|validation/i) }]); + await expect(client.callTool({ name: 'typed', arguments: { prompt: 123 } })).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams, + message: expect.stringMatching(/input validation error/i) + }); expect(handlerCalls.n).toBe(1); - const missing = await client.callTool({ name: 'typed', arguments: {} }); - expect(missing.isError).toBe(true); - expect(missing.content).toEqual([{ type: 'text', text: expect.stringMatching(/invalid|validation|required/i) }]); + await expect(client.callTool({ name: 'typed', arguments: {} })).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams, + message: expect.stringMatching(/input validation error/i) + }); expect(handlerCalls.n).toBe(1); }); @@ -1199,15 +1201,19 @@ verifies('typescript:mcpserver:tool:schema-variants', async ({ transport }: Test const coerced = await client.callTool({ name: 'zod-coerce', arguments: { n: ' 7 ' } }); expect(coerced.content).toEqual([{ type: 'text', text: 'coerced:7' }]); - // Rejections — proves parse() actually runs for each shape. - const unionRejected = await client.callTool({ name: 'zod-union', arguments: { kind: 'a', a: 123 } }); - expect(unionRejected.isError).toBe(true); - const intersectionRejected = await client.callTool({ name: 'zod-intersection', arguments: { left: 'L' } }); - expect(intersectionRejected.isError).toBe(true); - const nestedRejected = await client.callTool({ name: 'zod-nested', arguments: { outer: { inner: { value: 'x' } } } }); - expect(nestedRejected.isError).toBe(true); - const coerceRejected = await client.callTool({ name: 'zod-coerce', arguments: { n: '-3' } }); - expect(coerceRejected.isError).toBe(true); + // Rejections prove parse() actually runs for each shape. + await expect(client.callTool({ name: 'zod-union', arguments: { kind: 'a', a: 123 } })).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams + }); + await expect(client.callTool({ name: 'zod-intersection', arguments: { left: 'L' } })).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams + }); + await expect(client.callTool({ name: 'zod-nested', arguments: { outer: { inner: { value: 'x' } } } })).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams + }); + await expect(client.callTool({ name: 'zod-coerce', arguments: { n: '-3' } })).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams + }); }); verifies('typescript:mcpserver:tool:extra', async ({ transport }: TestArgs) => { diff --git a/test/integration/test/server/mcp.test.ts b/test/integration/test/server/mcp.test.ts index 8c844b11cb..88cf1dae62 100644 --- a/test/integration/test/server/mcp.test.ts +++ b/test/integration/test/server/mcp.test.ts @@ -1,10 +1,37 @@ import { Client } from '@modelcontextprotocol/client'; import type { Notification, TextContent } from '@modelcontextprotocol/core'; -import { getDisplayName, InMemoryTransport, ProtocolErrorCode, UriTemplate, UrlElicitationRequiredError } from '@modelcontextprotocol/core'; +import { + getDisplayName, + InMemoryTransport, + ProtocolError, + ProtocolErrorCode, + UriTemplate, + UrlElicitationRequiredError +} from '@modelcontextprotocol/core'; import { completable, McpServer, ResourceTemplate } from '@modelcontextprotocol/server'; import { afterEach, beforeEach, describe, expect, test } from 'vitest'; import * as z from 'zod/v4'; +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', () => { /*** @@ -1189,25 +1216,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'] ); }); @@ -4986,22 +5006,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'] ); }); }); @@ -6244,40 +6257,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 ffc41ce4d8..aa10584373 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 { InMemoryTransport } from '@modelcontextprotocol/core'; +import { 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,48 @@ describe('Standard Schema Support', () => { await connectClientAndServer(); - const result = await client.request({ method: 'tools/call', params: { name: 'double', arguments: { count: 'not a number' } } }); + await expectInvalidToolArguments( + client.request({ method: 'tools/call', params: { name: 'double', arguments: { count: 'not a number' } } }), + ['Input validation error'] + ); + }); + }); + + describe('fromJsonSchema with default validator (server wrapper)', () => { + test('should use runtime-appropriate default validator when none is provided', async () => { + const inputSchema = serverFromJsonSchema<{ name: string }>({ + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + }); + + mcpServer.registerTool('greet-default', { inputSchema }, async ({ name }) => ({ + content: [{ type: 'text', text: `Hello, ${name}!` }] + })); - expect(result.isError).toBe(true); - const errorText = (result.content[0] as TextContent).text; - expect(errorText).toContain('Input validation error'); + await connectClientAndServer(); + + const result = await client.request({ method: 'tools/call', params: { name: 'greet-default', arguments: { name: 'World' } } }); + expect((result.content[0] as TextContent).text).toBe('Hello, World!'); + }); + + test('should reject invalid input with default validator', async () => { + const inputSchema = serverFromJsonSchema({ type: 'object', properties: { count: { type: 'number' } }, required: ['count'] }); + + mcpServer.registerTool('double-default', { inputSchema }, async args => { + const { count } = args as { count: number }; + return { content: [{ type: 'text', text: `${count * 2}` }] }; + }); + + await connectClientAndServer(); + + await expectInvalidToolArguments( + client.request({ + method: 'tools/call', + params: { name: 'double-default', arguments: { count: 'not a number' } } + }), + ['Input validation error'] + ); }); }); @@ -557,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 () => { @@ -593,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 () => { @@ -627,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'] + ); }); });