From 7a1412c4b47d23199ef911ea1379e43a013b6348 Mon Sep 17 00:00:00 2001 From: kigland Date: Sun, 31 May 2026 18:09:50 +0800 Subject: [PATCH 1/2] Return protocol errors for invalid tool args --- .changeset/late-validations-argue.md | 5 + packages/server/src/server/mcp.ts | 8 +- test/integration/test/standardSchema.test.ts | 127 ++++++------------- 3 files changed, 49 insertions(+), 91 deletions(-) create mode 100644 .changeset/late-validations-argue.md diff --git a/.changeset/late-validations-argue.md b/.changeset/late-validations-argue.md new file mode 100644 index 0000000000..c2707d1474 --- /dev/null +++ b/.changeset/late-validations-argue.md @@ -0,0 +1,5 @@ +--- +"@modelcontextprotocol/server": patch +--- + +Return protocol errors for invalid tool input arguments instead of wrapping them as tool execution errors. diff --git a/packages/server/src/server/mcp.ts b/packages/server/src/server/mcp.ts index fb45fd5db6..989cbddf5e 100644 --- a/packages/server/src/server/mcp.ts +++ b/packages/server/src/server/mcp.ts @@ -208,8 +208,12 @@ 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.code === ProtocolErrorCode.InvalidParams && error.message.startsWith('Input validation error:'))) + ) { + throw error; // Return protocol errors to the caller without wrapping in CallToolResult } return this.createToolError(error instanceof Error ? error.message : String(error)); } diff --git a/test/integration/test/standardSchema.test.ts b/test/integration/test/standardSchema.test.ts index ffc41ce4d8..2521371d13 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, 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,18 @@ describe('Standard Schema Support', () => { await Promise.all([client.connect(clientTransport), mcpServer.connect(serverTransport)]); } + async function expectToolInputValidationError(params: { name: string; arguments: Record }) { + try { + await client.request({ method: 'tools/call', params }); + } catch (error) { + expect(error).toMatchObject({ code: ProtocolErrorCode.InvalidParams }); + const message = error instanceof Error ? error.message : String(error); + expect(message).toContain('Input validation error'); + return message; + } + throw new Error('Expected tool input validation to reject with InvalidParams'); + } + describe('ArkType schemas', () => { describe('tool registration', () => { test('should register tool with ArkType input schema', async () => { @@ -130,14 +142,7 @@ 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'); + const errorText = await expectToolInputValidationError({ name: 'double', arguments: { value: 'not a number' } }); expect(errorText).toContain('value'); expect(errorText).toContain('number'); }); @@ -153,14 +158,7 @@ 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'); + const errorText = await expectToolInputValidationError({ name: 'calculate', arguments: { operation: 'divide' } }); expect(errorText).toMatch(/add|subtract|multiply/); }); @@ -173,14 +171,7 @@ 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'); + const errorText = await expectToolInputValidationError({ name: 'greet', arguments: { name: 'Alice' } }); expect(errorText).toContain('age'); }); }); @@ -273,14 +264,7 @@ 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'); + const errorText = await expectToolInputValidationError({ name: 'double', arguments: { value: 'not a number' } }); expect(errorText).toContain('number'); }); @@ -297,14 +281,7 @@ 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 expectToolInputValidationError({ name: 'calculate', arguments: { operation: 'divide' } }); }); test('should validate min/max constraints', async () => { @@ -328,13 +305,7 @@ 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 expectToolInputValidationError({ name: 'setPercentage', arguments: { percentage: 150 } }); }); }); }); @@ -420,11 +391,7 @@ 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 expectToolInputValidationError({ name: 'double', arguments: { count: 'not a number' } }); }); }); @@ -557,21 +524,15 @@ 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' - } + const errorText = await expectToolInputValidationError({ + 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 @@ -593,21 +554,15 @@ 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' - } + const errorText = await expectToolInputValidationError({ + 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 @@ -627,21 +582,15 @@ 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' - } + const errorText = await expectToolInputValidationError({ + 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'); }); From 287dfbc7663da41f3855b9771b05081f4d1acfcf Mon Sep 17 00:00:00 2001 From: kigland Date: Sun, 31 May 2026 18:51:52 +0800 Subject: [PATCH 2/2] test: expect invalid tool args protocol errors --- test/e2e/requirements.ts | 14 +-- test/e2e/scenarios/tools.test.ts | 34 ++++--- test/integration/test/server/mcp.test.ts | 110 ++++++++++------------- 3 files changed, 67 insertions(+), 91 deletions(-) diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index 092abb1a7c..dc51c43aef 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -2375,12 +2375,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', @@ -2390,12 +2385,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..72ee49677b 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(/invalid|validation/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(/invalid|validation|required/i) + }); expect(handlerCalls.n).toBe(1); }); @@ -1200,14 +1202,18 @@ verifies('typescript:mcpserver:tool:schema-variants', async ({ transport }: Test 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); + 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 92af09744c..b78c430079 100644 --- a/test/integration/test/server/mcp.test.ts +++ b/test/integration/test/server/mcp.test.ts @@ -1212,26 +1212,21 @@ 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 expect( + client.request({ + method: 'tools/call', + params: { name: 'test', - value: 'not a number' + arguments: { + name: 'test', + value: 'not a number' + } } - } + }) + ).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams, + message: expect.stringContaining('Input validation error: Invalid arguments for tool test') }); - - expect(result.isError).toBe(true); - expect(result.content).toEqual( - expect.arrayContaining([ - { - type: 'text', - text: expect.stringContaining('Input validation error: Invalid arguments for tool test') - } - ]) - ); }); /*** @@ -5149,23 +5144,18 @@ describe('Zod v4', () => { await server.connect(serverTransport); await client.connect(clientTransport); - const invalidTypeResult = await client.callTool({ - name: 'union-test', - arguments: { - type: 'a', - value: 123 - } + await expect( + client.callTool({ + name: 'union-test', + arguments: { + type: 'a', + value: 123 + } + }) + ).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams, + message: expect.stringContaining('Input validation error') }); - - expect(invalidTypeResult.isError).toBe(true); - expect(invalidTypeResult.content).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - type: 'text', - text: expect.stringContaining('Input validation error') - }) - ]) - ); }); }); @@ -6407,41 +6397,31 @@ describe('Zod v4', () => { await server.connect(serverTransport); await client.connect(clientTransport); - const invalidTypeResult = await client.callTool({ - name: 'union-test', - arguments: { - type: 'a', - value: 123 - } + await expect( + client.callTool({ + name: 'union-test', + arguments: { + type: 'a', + value: 123 + } + }) + ).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams, + message: expect.stringContaining('Input validation error') }); - expect(invalidTypeResult.isError).toBe(true); - expect(invalidTypeResult.content).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - type: 'text', - text: expect.stringContaining('Input validation error') - }) - ]) - ); - - const invalidDiscriminatorResult = await client.callTool({ - name: 'union-test', - arguments: { - type: 'c', - value: 'test' - } + await expect( + client.callTool({ + name: 'union-test', + arguments: { + type: 'c', + value: 'test' + } + }) + ).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams, + message: expect.stringContaining('Input validation error') }); - - expect(invalidDiscriminatorResult.isError).toBe(true); - expect(invalidDiscriminatorResult.content).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - type: 'text', - text: expect.stringContaining('Input validation error') - }) - ]) - ); }); });