Skip to content

Commit 1b22286

Browse files
fix(mcp): answer malformed request params as -32602 Invalid params naming the param (#29205)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9KACvtSQYX3ZKqWRb83L4mE)_ Fixes #29135 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:MCP 请求参数格式不对(如 `prompts/get` 不带 name、`tools/call` 传 `{}`)时,服务器现在回 JSON-RPC -32602「参数无效」,并用一句英文点名哪个参数错了(`params.name is required`),不再回 -32603「内部错误」加一段 Zod 原始 JSON。 - 为什么改:-32603 告诉调用方「服务器坏了」,AI 代理会当成可重试错误反复重发一个永远不会成功的请求;开发者看到的是一堆 Zod 内部结构,而不是一句话。 - 风险与回滚:同样的请求照样被拒,只是错误码和文字变了;工具自身的参数校验(如 `validate_expression` 的 site 枚举)和处理函数内部抛出的错误都不受影响。回滚即回退本 PR。 - 先看:`packages/mcp/src/request-params.ts`(唯一的接缝,读 SDK 未公开的请求处理表,缺失时构造即报错)、`packages/mcp/src/mcp-server-runtime.ts` 两处建服务器的调用点、`packages/mcp/src/request-params.test.ts`。 - 两分钟验证:`pnpm --filter @objectstack/mcp exec vitest run src/request-params.test.ts`,预期 7 个测试全绿。 - 需要你做的:无,席位落地。 ## What changes - New `packages/mcp/src/request-params.ts` with `createPlatformMcpServer(...)`, now the only place the platform builds an MCP `McpServer`. Both transports use it: the long-lived (stdio) server in the `MCPServerRuntime` constructor and the per-request Streamable HTTP server in `handleHttpRequest`. - It wraps every request dispatcher the SDK's low-level `Server` holds, the ones the SDK installs in its own constructor (`initialize`, `ping`) and every later one (`tools/call`, `prompts/get`, `resources/read`, …). A ZodError thrown **synchronously** by a dispatcher is the method's request-schema parse (the SDK parses before it calls the async handler), and it is rethrown as `McpError(ErrorCode.InvalidParams, sentence)`. A ZodError raised inside a handler arrives as a rejected promise and keeps its own answer. - Sentence: one plain-English clause per issue, joined with `; `: `params.name is required` (the value is absent), `params.name must be a string` (wrong type), `must be one of …` (enum or literal), `has unrecognized key(s): …`. Any other issue falls back to the shared `formatZodIssue` from `@objectstack/spec/shared`. `McpError` adds its own `MCP error -32602:` prefix. - No new error code: -32602 is the JSON-RPC standard one the SDK already uses for its own invalid-params answers. No `packages/spec/**` edit, no lockfile change, no new gate. ## Why The SDK's `Protocol.setRequestHandler` (measured on the installed `@modelcontextprotocol/sdk` 1.32.1, and unchanged in the latest 1.32.2) installs a closure that parses the request with the method's schema and rethrows the ZodError unchanged. A ZodError has no numeric `code`, so the protocol answers `-32603 Internal error` with the error's message, which is the serialised issue array. The SDK offers no public hook between that parse and the error answer, so the seam reads the protocol's `_requestHandlers` map. That is an SDK-internal field, so the seam checks it is there: if a later SDK renames it, server construction throws an error that names this function, and the -32603 answer does not come back silently. Measured on base `0d6d4061` (in-process, same calls as the card): | Request | Base | Head | |:---|:---|:---| | `prompts/get` `{}` (HTTP) | -32603, Zod issue array | -32602 `params.name is required` | | `tools/call` `{}` (HTTP) | -32603, Zod issue array | -32602 `params.name is required` | | `prompts/get` `{"name":42}` (HTTP) | -32603, Zod issue array | -32602 `params.name must be a string` | | `tools/call` `{}` (stdio, long-lived server) | -32603, Zod issue array | -32602 `params.name is required` | | control: `validate_expression` with `site: "nope"` | tool result `isError`, `MCP error -32602: Input validation error: …` | unchanged | Clause-② measured: the same requests are refused on base and head (no request accepted on one side is refused on the other), so `no`. Only the error class and the message change. ## Tests `packages/mcp/src/request-params.test.ts` (new, 7 cases): - HTTP: `prompts/get` with no name, `tools/call` with `{}`, `prompts/get` with `name: 42`, and `initialize` with no `protocolVersion` (a handler the SDK installs in its own constructor). Each asserts `error.code === -32602` and the full message. - stdio: `tools/call` with `{}` on the long-lived server over a real `StdioServerTransport`. - Control: `validate_expression` with an unknown `site` keeps its tool-result answer. - Negative pin: a ZodError thrown inside a handler keeps `-32603`. No existing test was edited. ## Acceptance notes - `resources/read` with malformed params: neither server in these tests registers resources, so it answers -32601 Method not found there. The seam is method-agnostic (it wraps the map, not a method list), and the `initialize` pin proves that a handler installed outside the platform's own registration is covered too. - Upstream: the SDK could map the request-schema ZodError to InvalidParams itself; when it does, this seam can go. That is noted in the report's open questions, not filed. ## Verification All readings at head `7bff29ad` unless noted. <details><summary>Ablations (both restored, `git diff HEAD` empty, blob == HEAD)</summary> - Mapping removed (`answerMalformedParamsAsInvalidParams(server);` deleted through `scripts/ablation-replace.mjs`, anchor 1 → 0, at 1d7f90e): 5 failed, 2 passed. Each failure is `expected -32603 to be -32602`. The control and the inner-ZodError pin stay green. - Discriminator widened (the guard also maps a ZodError that a handler rejects with): only `a ZodError thrown inside a handler keeps the -32603 answer` fails, `expected -32602 to be -32603`. 6 passed. </details> <details><summary>Suites</summary> - `pnpm --filter @objectstack/mcp test`: Test Files 54 passed (54), Tests 651 passed (651). - `pnpm --filter @objectstack/mcp typecheck` (tsc + check:test-typecheck): exit 0. - `@objectstack/runtime` MCP domain tests (`src/domains/mcp-{federation-bridge,merged-skill-read,option-fields-read,package-object-discovery}.test.ts`, after building `@objectstack/runtime^...`): Test Files 4 passed (4), Tests 162 passed (162). - eslint over `packages/mcp` (`--no-inline-config`): exit 0. </details> <details><summary>Gates (dispatch-gates --commands, 59 derived)</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `7bff29ad`: 59 commands derived; 4 changed paths, 302 changed lines. - Exit 0 for 57 of them as first run. - `node scripts/check-plugin-teardown-shape.mjs --self-test`: exit 3 on the first run (its positive-control commit was outside the shallow clone). Exit 0 after `git fetch --unshallow origin main` (48 cases pass). - `pnpm check:dual-build-cjs-loads`: exit 3, PREREQUISITE NOT MET (it needs every package's `dist/`; only the `@objectstack/runtime` closure was built here). NOT MEASURED locally; CI's Lint & Repo Gates runs it. - `--ran` reconciliation: `59 derived, 58 run, 1 NOT-MEASURED, 0 UNRUN`. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) · https://claude.ai/code/session_01ADHkmT3dPW9V7vSk6VKtuN --- _Generated by [Claude Code](https://claude.ai/code/session_01ADHkmT3dPW9V7vSk6VKtuN)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 761fef4 commit 1b22286

4 files changed

Lines changed: 300 additions & 2 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/mcp': patch
3+
---
4+
5+
fix(mcp): a request with malformed params answers JSON-RPC -32602 Invalid params naming the param, not -32603 Internal error with a raw Zod issue list
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
An MCP request whose `params` fail the method's request schema (`prompts/get` with no `name`, `tools/call` with `{}`, `prompts/get` with `"name": 42`) used to answer `-32603 Internal error` whose message was the serialised Zod issue array: the client was told the server failed, and an agent that retries on Internal error kept retrying a request that could never succeed. It now answers `-32602 Invalid params` with one sentence per problem, such as `params.name is required` or `params.name must be a string`. The same requests stay refused; only the error class and the message change. Both transports (Streamable HTTP and stdio) build their server through one seam, so the answer is the same on either and for every method, including the ones the SDK installs itself (`initialize`). A tool's own argument check (for example `validate_expression` with an unknown `site`) keeps its answer, and an error raised inside a handler keeps its own code.

‎packages/mcp/src/mcp-server-runtime.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
metadataPartialListingSentence,
2727
} from './metadata-completeness.js';
2828
import { DEFAULT_SERVER_VERSION } from './package-version.js';
29+
import { createPlatformMcpServer } from './request-params.js';
2930
import { protocolStdout } from './protocol-stdout.js';
3031
import { renderSkillMarkdown, type RenderSkillOptions } from './skill-md.js';
3132
import {
@@ -1223,7 +1224,9 @@ export class MCPServerRuntime {
12231224
...config,
12241225
};
12251226

1226-
this.mcpServer = new McpServer(
1227+
// [#29135] Built through the one seam both transports share, so malformed
1228+
// params answer -32602 Invalid params rather than -32603 (see request-params.ts).
1229+
this.mcpServer = createPlatformMcpServer(
12271230
{
12281231
name: this.config.name,
12291232
version: this.config.version,
@@ -1830,7 +1833,8 @@ export class MCPServerRuntime {
18301833
: undefined;
18311834

18321835
// Fresh, isolated server per request (stateless).
1833-
const server = new McpServer(
1836+
// [#29135] The same seam as the long-lived server: malformed params answer -32602.
1837+
const server = createPlatformMcpServer(
18341838
{ name: this.config.name, version: this.config.version },
18351839
{
18361840
// [#8034] `tools` is DERIVED, exactly as on the long-lived server:
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, afterEach } from 'vitest';
4+
import { PassThrough } from 'node:stream';
5+
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js';
6+
import { WebStandardStreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js';
7+
import { GetPromptRequestSchema } from '@modelcontextprotocol/sdk/types.js';
8+
import { z } from 'zod';
9+
10+
import { MCPServerRuntime } from './mcp-server-runtime.js';
11+
import { createPlatformMcpServer } from './request-params.js';
12+
import type { McpDataBridge } from './mcp-http-tools.js';
13+
import type { McpSkillBridge } from './skill-prompts.js';
14+
15+
/**
16+
* A request whose params fail the method's request schema is the CALLER's
17+
* mistake: JSON-RPC -32602 Invalid params with a sentence naming the param,
18+
* never -32603 Internal error carrying the serialised Zod issue list. Driven
19+
* over both real transports, because both build their server through the one
20+
* seam in `request-params.ts`.
21+
*/
22+
23+
const INVALID_PARAMS = -32602;
24+
const INTERNAL_ERROR = -32603;
25+
26+
function makeBridge(): McpDataBridge & McpSkillBridge {
27+
return {
28+
async listObjects() {
29+
return [{ name: 'crm_opportunity', label: 'Opportunity', fieldCount: 1 }];
30+
},
31+
async describeObject(name: string) {
32+
return name === 'crm_opportunity'
33+
? { name, label: 'Opportunity', fields: [{ name: 'amount', type: 'currency', label: 'Amount', required: false }] }
34+
: null;
35+
},
36+
async query() { return { records: [] }; },
37+
async get() { return null; },
38+
async create() { return {}; },
39+
async update() { return {}; },
40+
async remove() { return {}; },
41+
async listSkills() { return []; },
42+
};
43+
}
44+
45+
async function overHttp(method: string, params: unknown): Promise<any> {
46+
const runtime = new MCPServerRuntime({ name: 'request-params', version: '0.0.0' });
47+
const body = { jsonrpc: '2.0', id: 1, method, params };
48+
const res = await runtime.handleHttpRequest(
49+
new Request('http://localhost/api/v1/mcp', {
50+
method: 'POST',
51+
headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' },
52+
body: JSON.stringify(body),
53+
}),
54+
{ bridge: makeBridge(), parsedBody: body } as never,
55+
);
56+
return res.json();
57+
}
58+
59+
/** The refusal shape every malformed request must get: the code, the named param, no JSON dump. */
60+
function expectInvalidParams(frame: any, sentence: string): void {
61+
expect(frame.error, JSON.stringify(frame)).toBeDefined();
62+
expect(frame.error.code).toBe(INVALID_PARAMS);
63+
expect(frame.error.message).toBe(`MCP error -32602: ${sentence}`);
64+
}
65+
66+
describe('malformed request params answer -32602 Invalid params (HTTP transport)', () => {
67+
it('prompts/get with no name', async () => {
68+
expectInvalidParams(await overHttp('prompts/get', {}), 'params.name is required');
69+
});
70+
71+
it('tools/call with {}', async () => {
72+
expectInvalidParams(await overHttp('tools/call', {}), 'params.name is required');
73+
});
74+
75+
it('prompts/get with a non-string name', async () => {
76+
expectInvalidParams(await overHttp('prompts/get', { name: 42 }), 'params.name must be a string');
77+
});
78+
79+
it('initialize, a handler the SDK installs in its own constructor, takes the same answer', async () => {
80+
const frame = await overHttp('initialize', { capabilities: {}, clientInfo: { name: 't', version: '1' } });
81+
expectInvalidParams(frame, 'params.protocolVersion is required');
82+
});
83+
84+
it("control: a tool's own argument check keeps its answer", async () => {
85+
const frame = await overHttp('tools/call', {
86+
name: 'validate_expression',
87+
arguments: { objectName: 'crm_opportunity', expression: 'record.amount > 1', site: 'nope' },
88+
});
89+
expect(frame.error).toBeUndefined();
90+
expect(frame.result.isError).toBe(true);
91+
expect(frame.result.content[0].text).toMatch(
92+
/^MCP error -32602: Input validation error: Invalid arguments for tool validate_expression: /,
93+
);
94+
});
95+
});
96+
97+
describe('malformed request params answer -32602 Invalid params (stdio transport)', () => {
98+
const closers: Array<() => Promise<void>> = [];
99+
afterEach(async () => {
100+
for (const close of closers.splice(0)) await close();
101+
});
102+
103+
it('tools/call with {} on the long-lived server', async () => {
104+
const runtime = new MCPServerRuntime({ name: 'request-params', version: '0.0.0' });
105+
runtime.bridgeDataTools(makeBridge());
106+
const stdin = new PassThrough();
107+
const stdout = new PassThrough();
108+
const transport = new StdioServerTransport(stdin, stdout);
109+
await runtime.server.connect(transport);
110+
closers.push(() => transport.close().catch(() => {}));
111+
112+
const frame = await new Promise<any>((resolve) => {
113+
stdout.on('data', (chunk) => {
114+
for (const line of String(chunk).split('\n')) {
115+
if (line.trim()) resolve(JSON.parse(line));
116+
}
117+
});
118+
stdin.write(`${JSON.stringify({ jsonrpc: '2.0', id: 7, method: 'tools/call', params: {} })}\n`);
119+
});
120+
expect(frame.id).toBe(7);
121+
expectInvalidParams(frame, 'params.name is required');
122+
});
123+
});
124+
125+
describe('only the request-schema parse is mapped', () => {
126+
it('a ZodError thrown inside a handler keeps the -32603 answer', async () => {
127+
const server = createPlatformMcpServer({ name: 'request-params', version: '0.0.0' }, { capabilities: { prompts: {} } });
128+
server.server.setRequestHandler(GetPromptRequestSchema, async () => {
129+
z.object({ inner: z.string() }).parse({});
130+
return { messages: [] };
131+
});
132+
const transport = new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: undefined, enableJsonResponse: true });
133+
await server.connect(transport);
134+
const body = { jsonrpc: '2.0', id: 1, method: 'prompts/get', params: { name: 'any' } };
135+
try {
136+
const res = await transport.handleRequest(
137+
new Request('http://localhost/mcp', {
138+
method: 'POST',
139+
headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' },
140+
body: JSON.stringify(body),
141+
}),
142+
{ parsedBody: body },
143+
);
144+
const frame = (await res.json()) as { error: { code: number } };
145+
expect(frame.error.code).toBe(INTERNAL_ERROR);
146+
} finally {
147+
await server.close();
148+
}
149+
});
150+
});

‎packages/mcp/src/request-params.ts‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
4+
import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js';
5+
import { formatZodIssue } from '@objectstack/spec/shared';
6+
import { z } from 'zod';
7+
8+
/**
9+
* The one place the platform builds an MCP {@link McpServer}: both transports
10+
* (the long-lived stdio server and the per-request Streamable HTTP server) go
11+
* through here, so a malformed request gets the same answer on either.
12+
*
13+
* What it adds over `new McpServer(...)`: a request whose `params` fail the
14+
* method's request schema is answered as JSON-RPC `-32602 Invalid params` with
15+
* a sentence naming the param (`params.name is required`), instead of
16+
* `-32603 Internal error` carrying the serialised Zod issue list.
17+
*
18+
* Why the SDK answers -32603 (measured on `@modelcontextprotocol/sdk` 1.32.1,
19+
* and unchanged in 1.32.2): `Protocol.setRequestHandler` installs a closure
20+
* that parses the request with the method's schema and rethrows the ZodError
21+
* as-is. A ZodError has no numeric `code`, so the protocol's error branch
22+
* falls back to `InternalError` with the error's message — the JSON dump. The
23+
* client is told the server failed when its own input was wrong, and an agent
24+
* that treats Internal error as retryable retries a request that can never
25+
* succeed.
26+
*/
27+
export function createPlatformMcpServer(
28+
...args: ConstructorParameters<typeof McpServer>
29+
): McpServer {
30+
const server = new McpServer(...args);
31+
answerMalformedParamsAsInvalidParams(server);
32+
return server;
33+
}
34+
35+
type RequestDispatcher = (request: unknown, extra: unknown) => Promise<unknown>;
36+
37+
/**
38+
* Wraps every request dispatcher the low-level `Server` holds — those the SDK
39+
* installed in its constructor (`initialize`, `ping`, …) and every one
40+
* registered later (`tools/call`, `prompts/get`, `resources/read`, …).
41+
*
42+
* The SDK keeps them in the protocol's `_requestHandlers` map, which it does
43+
* not expose; there is no public hook between the request-schema parse and
44+
* the error answer. The map is read once here, and its absence is loud: a
45+
* later SDK that renames it fails server construction with this function's
46+
* name, never silently goes back to -32603.
47+
*
48+
* Only a parse failure is mapped. The installed dispatcher parses
49+
* SYNCHRONOUSLY and only then calls the async handler, so a ZodError thrown
50+
* synchronously is the request-schema parse and nothing else; a ZodError from
51+
* inside a handler arrives as a rejected promise and keeps its own answer.
52+
*/
53+
function answerMalformedParamsAsInvalidParams(server: McpServer): void {
54+
const handlers = (server.server as unknown as { _requestHandlers?: unknown })._requestHandlers;
55+
if (!(handlers instanceof Map)) {
56+
throw new Error(
57+
'answerMalformedParamsAsInvalidParams: the MCP SDK no longer keeps its request handlers where this '
58+
+ 'server reads them, so malformed params would answer -32603 Internal error instead of -32602 '
59+
+ 'Invalid params. Re-read the SDK Protocol request path and move this seam.',
60+
);
61+
}
62+
const map = handlers as Map<string, RequestDispatcher>;
63+
64+
const guard = (dispatch: RequestDispatcher): RequestDispatcher => (request, extra) => {
65+
try {
66+
return dispatch(request, extra);
67+
} catch (error) {
68+
if (isZodError(error)) {
69+
throw new McpError(ErrorCode.InvalidParams, invalidParamsSentence(error.issues, request));
70+
}
71+
throw error;
72+
}
73+
};
74+
75+
for (const [method, dispatch] of map) map.set(method, guard(dispatch));
76+
const set = map.set.bind(map);
77+
map.set = (method, dispatch) => set(method, guard(dispatch));
78+
}
79+
80+
interface ZodIssueLike {
81+
code: string;
82+
path: PropertyKey[];
83+
message: string;
84+
expected?: unknown;
85+
values?: unknown[];
86+
keys?: string[];
87+
}
88+
89+
/**
90+
* The SDK parses with zod's core (`$ZodError`, not classic `ZodError`), so the
91+
* check is zod's own structural `instanceof` on the core class, which holds
92+
* across both builds.
93+
*/
94+
function isZodError(error: unknown): error is { issues: ZodIssueLike[] } {
95+
return error instanceof z.core.$ZodError;
96+
}
97+
98+
/**
99+
* One plain-English sentence per issue, joined with `; ` (`McpError` itself
100+
* prefixes `MCP error -32602:`): `params.name is
101+
* required`, `params.name must be a string`. The path is the request's own
102+
* (`params.…`), so it names exactly what the caller sent. An issue outside the
103+
* two shapes above falls back to the platform's shared Zod issue formatter.
104+
*/
105+
export function invalidParamsSentence(issues: readonly ZodIssueLike[], request: unknown): string {
106+
const sentences = issues.map((issue) => {
107+
const path = issue.path.length > 0 ? issue.path.map(String).join('.') : 'params';
108+
if (issue.code === 'invalid_type') {
109+
if (valueAt(request, issue.path) === undefined) return `${path} is required`;
110+
if (typeof issue.expected === 'string') return `${path} must be ${article(issue.expected)}`;
111+
}
112+
if (issue.code === 'invalid_value' && Array.isArray(issue.values) && issue.values.length > 0) {
113+
return `${path} must be one of ${issue.values.map((v) => JSON.stringify(v)).join(', ')}`;
114+
}
115+
if (issue.code === 'unrecognized_keys' && Array.isArray(issue.keys)) {
116+
return `${path} has unrecognized key(s): ${issue.keys.join(', ')}`;
117+
}
118+
return formatZodIssue(issue as Parameters<typeof formatZodIssue>[0]);
119+
});
120+
return sentences.join('; ');
121+
}
122+
123+
function valueAt(root: unknown, path: readonly PropertyKey[]): unknown {
124+
let node: unknown = root;
125+
for (const key of path) {
126+
if (typeof node !== 'object' || node === null) return undefined;
127+
node = (node as Record<PropertyKey, unknown>)[key];
128+
}
129+
return node;
130+
}
131+
132+
function article(type: string): string {
133+
return /^[aeiou]/.test(type) ? `an ${type}` : `a ${type}`;
134+
}

0 commit comments

Comments
 (0)