Skip to content

Commit 688b282

Browse files
test(compat): add e2e raw-shape tools/call test; drop vestigial warn-spy; cover normalizeRawShapeSchema passthrough/undefined
1 parent 0152b26 commit 688b282

File tree

5 files changed

+148
-14
lines changed

5 files changed

+148
-14
lines changed

.converge/round-0/findings.json

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
[
2+
{
3+
"bug_id": "r0-001",
4+
"severity": "nit",
5+
"file_path": "packages/server/src/server/mcp.ts",
6+
"start_line": 879,
7+
"end_line": 891,
8+
"pr_comment": "The raw-shape overload requires `InputArgs extends ZodRawShape` but allows `OutputArgs extends ZodRawShape | StandardSchemaWithJSON`. The reverse mix (wrapped `inputSchema` + raw `outputSchema`) matches no overload: overload 1 rejects raw `outputSchema`, overload 2 rejects wrapped `inputSchema`. v1 code was always all-raw so this doesn't break any v1 caller, but the asymmetry is surprising.",
9+
"reasoning": "Only matters if someone writes `registerTool('x', {inputSchema: z.object({a: z.string()}), outputSchema: {b: z.number()}}, cb)` — a hybrid that v1 didn't support either. Low impact; could add a third overload `<InputArgs extends StandardSchemaWithJSON, OutputArgs extends ZodRawShape>` for symmetry, or document that raw-shape applies all-or-nothing.",
10+
"status": "open"
11+
},
12+
{
13+
"bug_id": "r0-002",
14+
"severity": "nit",
15+
"file_path": "packages/server/src/server/mcp.ts",
16+
"start_line": 43,
17+
"end_line": 43,
18+
"pr_comment": "`import type * as z from 'zod/v4'` adds `z.ZodType` to the public `.d.mts` surface via `ZodRawShape = Record<string, z.ZodType>`. This is intentional (raw-shape is documented Zod-only and `@deprecated`), and server already depends on zod, but it's a new direct type-surface coupling worth noting in case the intent was to keep server's primary types StandardSchema-only.",
19+
"reasoning": "The directive flagged this as a watch-for. It's correctly scoped to the deprecated path; mentioning here so it's on record as a deliberate choice rather than a regression.",
20+
"status": "open"
21+
},
22+
{
23+
"bug_id": "r0-003",
24+
"severity": "nit",
25+
"file_path": "packages/core/src/util/standardSchema.ts",
26+
"start_line": 173,
27+
"end_line": 175,
28+
"pr_comment": "`z.object(schema) as StandardSchemaWithJSON` — the cast is needed because `ZodObject` doesn't structurally narrow to the interface here, but it relies on zod ≥4.2.0 actually providing `~standard.jsonSchema`. On this branch alone the catalog still has `^4.0` (B1/#1895 bumps to ^4.2.0). With zod 4.0.x this cast would type-check but `standardSchemaToJsonSchema(wrapped)` could fail at runtime.",
29+
"reasoning": "B1's fallback covers this once both land; on C4 standalone it's a hidden version assumption. Nit because the integration validates with zod 4.2+, and B1 is the right place for the catalog bump.",
30+
"status": "open"
31+
},
32+
{
33+
"bug_id": "r0-004",
34+
"severity": "nit",
35+
"file_path": "packages/server/test/server/mcp.compat.test.ts",
36+
"start_line": 7,
37+
"end_line": 24,
38+
"pr_comment": "Tests verify the schema is wrapped (via `isStandardSchema(tools['a'].inputSchema)`) but never call the tool to confirm the callback actually receives `{x: number}` typed args at runtime. Consider one end-to-end test that connects via InMemoryTransport and calls the tool.",
39+
"reasoning": "Current tests prove normalization happened but not that the callback signature works end-to-end. The wrapping is the load-bearing change; the callback typing is asserted only at compile time via the overload.",
40+
"status": "open"
41+
},
42+
{
43+
"bug_id": "r0-005",
44+
"severity": "nit",
45+
"file_path": "packages/server/test/server/mcp.compat.test.ts",
46+
"start_line": 8,
47+
"end_line": 23,
48+
"pr_comment": "`vi.spyOn(console, 'warn')` + `expect(warn).not.toHaveBeenCalled()` are vestiges of the removed `deprecate()` runtime warns. Harmless and they document the no-warn intent, but the spy setup could be dropped for clarity since there's no warn path left in the code under test.",
49+
"reasoning": "Policy is JSDoc-only `@deprecated`, no runtime warn. The assertions still pass but test code that guards against a removed mechanism can confuse future readers.",
50+
"status": "open"
51+
},
52+
{
53+
"bug_id": "r0-006",
54+
"severity": "nit",
55+
"file_path": "packages/core/test/util/standardSchema.test.ts",
56+
"start_line": 21,
57+
"end_line": 26,
58+
"pr_comment": "`normalizeRawShapeSchema` tests cover the wrap path but not the passthrough (`normalizeRawShapeSchema(z.object({a: z.string()}))` returns the same instance) or `undefined → undefined`.",
59+
"reasoning": "Minor coverage gap; both branches are trivial but documenting them keeps the contract explicit.",
60+
"status": "open"
61+
},
62+
{
63+
"bug_id": "r0-007",
64+
"severity": "nit",
65+
"file_path": "packages/server/src/server/mcp.ts",
66+
"start_line": 1113,
67+
"end_line": 1131,
68+
"pr_comment": "`ZodRawShape`/`InferRawShape`/`LegacyToolCallback`/`LegacyPromptCallback` are also defined by C2 (#1900) for the variadic `.tool()/.prompt()` shims. On each PR alone that's fine; merging both into d1-base/integration produces duplicate exported type declarations in mcp.ts.",
69+
"reasoning": "Integration concern, not a defect of this PR. Parent's d1-base merge handles the dedup (`c2efc634` already resolved a C2/C4 overlap once). Noting so it's not re-discovered.",
70+
"status": "open"
71+
}
72+
]

.converge/round-0/sha

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
0152b266fd4ab51de1144b77dd7bed9d1bae79f0

.converge/state.json

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"round": 0,
3+
"converged": true,
4+
"open_red": 0,
5+
"open_nit": 7,
6+
"lastSha": "0152b266",
7+
"base": "9ed62fe7d8be34df88a71d52f1e17a6a767138fd",
8+
"depth": "light",
9+
"verify_cmd": "pnpm typecheck:all && pnpm lint:all && pnpm build:all && pnpm docs:check && pnpm --filter @modelcontextprotocol/core test && pnpm --filter @modelcontextprotocol/server test",
10+
"preflight": {
11+
"pass": true,
12+
"typecheck": "pass",
13+
"lint": "pass",
14+
"build": "pass",
15+
"docs": "pass",
16+
"core_tests": "494/494",
17+
"server_tests": "59/59"
18+
}
19+
}

packages/core/test/util/standardSchema.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ describe('normalizeRawShapeSchema', () => {
2424
expect(wrapped).toBeDefined();
2525
expect(standardSchemaToJsonSchema(wrapped!, 'input').type).toBe('object');
2626
});
27+
test('passes through an already-wrapped Standard Schema unchanged', () => {
28+
const schema = z.object({ a: z.string() });
29+
expect(normalizeRawShapeSchema(schema)).toBe(schema);
30+
});
31+
test('returns undefined for undefined input', () => {
32+
expect(normalizeRawShapeSchema(undefined)).toBeUndefined();
33+
});
2734
});
2835

2936
describe('standardSchemaToJsonSchema', () => {

packages/server/test/server/mcp.compat.test.ts

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import { isStandardSchema } from '@modelcontextprotocol/core';
1+
import type { JSONRPCMessage } from '@modelcontextprotocol/core';
2+
import { InMemoryTransport, isStandardSchema, LATEST_PROTOCOL_VERSION } from '@modelcontextprotocol/core';
23
import { describe, expect, it, vi } from 'vitest';
34
import * as z from 'zod/v4';
45
import { McpServer } from '../../src/index.js';
56

67
describe('registerTool/registerPrompt accept raw Zod shape (auto-wrapped)', () => {
7-
it('registerTool accepts a raw shape for inputSchema, auto-wraps, and does not warn', () => {
8-
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
8+
it('registerTool accepts a raw shape for inputSchema and auto-wraps it', () => {
99
const server = new McpServer({ name: 't', version: '1.0.0' });
1010

1111
server.registerTool('a', { inputSchema: { x: z.number() } }, async ({ x }) => ({
@@ -19,9 +19,6 @@ describe('registerTool/registerPrompt accept raw Zod shape (auto-wrapped)', () =
1919
expect(Object.keys(tools)).toEqual(['a', 'b']);
2020
// raw shape was wrapped into a Standard Schema (z.object)
2121
expect(isStandardSchema(tools['a']?.inputSchema)).toBe(true);
22-
23-
expect(warn).not.toHaveBeenCalled();
24-
warn.mockRestore();
2522
});
2623

2724
it('registerTool accepts a raw shape for outputSchema and auto-wraps it', () => {
@@ -36,20 +33,18 @@ describe('registerTool/registerPrompt accept raw Zod shape (auto-wrapped)', () =
3633
expect(isStandardSchema(tools['out']?.outputSchema)).toBe(true);
3734
});
3835

39-
it('registerTool with z.object() inputSchema also works without warning', () => {
40-
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
36+
it('registerTool with z.object() inputSchema also works (passthrough, no auto-wrap)', () => {
4137
const server = new McpServer({ name: 't', version: '1.0.0' });
4238

4339
server.registerTool('c', { inputSchema: z.object({ x: z.number() }) }, async ({ x }) => ({
4440
content: [{ type: 'text' as const, text: String(x) }]
4541
}));
4642

47-
expect(warn).not.toHaveBeenCalled();
48-
warn.mockRestore();
43+
const tools = (server as unknown as { _registeredTools: Record<string, { inputSchema?: unknown }> })._registeredTools;
44+
expect(isStandardSchema(tools['c']?.inputSchema)).toBe(true);
4945
});
5046

51-
it('registerPrompt accepts a raw shape for argsSchema and does not warn', () => {
52-
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
47+
it('registerPrompt accepts a raw shape for argsSchema', () => {
5348
const server = new McpServer({ name: 't', version: '1.0.0' });
5449

5550
server.registerPrompt('p', { argsSchema: { topic: z.string() } }, async ({ topic }) => ({
@@ -59,8 +54,48 @@ describe('registerTool/registerPrompt accept raw Zod shape (auto-wrapped)', () =
5954
const prompts = (server as unknown as { _registeredPrompts: Record<string, { argsSchema?: unknown }> })._registeredPrompts;
6055
expect(Object.keys(prompts)).toContain('p');
6156
expect(isStandardSchema(prompts['p']?.argsSchema)).toBe(true);
57+
});
58+
59+
it('callback receives validated, typed args end-to-end via tools/call', async () => {
60+
const server = new McpServer({ name: 't', version: '1.0.0' });
61+
62+
let received: { x: number } | undefined;
63+
server.registerTool('echo', { inputSchema: { x: z.number() } }, async args => {
64+
received = args;
65+
return { content: [{ type: 'text' as const, text: String(args.x) }] };
66+
});
67+
68+
const [client, srv] = InMemoryTransport.createLinkedPair();
69+
await server.connect(srv);
70+
await client.start();
71+
72+
const responses: JSONRPCMessage[] = [];
73+
client.onmessage = m => responses.push(m);
74+
75+
await client.send({
76+
jsonrpc: '2.0',
77+
id: 1,
78+
method: 'initialize',
79+
params: {
80+
protocolVersion: LATEST_PROTOCOL_VERSION,
81+
capabilities: {},
82+
clientInfo: { name: 'c', version: '1.0.0' }
83+
}
84+
} as JSONRPCMessage);
85+
await client.send({ jsonrpc: '2.0', method: 'notifications/initialized' } as JSONRPCMessage);
86+
await client.send({
87+
jsonrpc: '2.0',
88+
id: 2,
89+
method: 'tools/call',
90+
params: { name: 'echo', arguments: { x: 7 } }
91+
} as JSONRPCMessage);
92+
93+
await vi.waitFor(() => expect(responses.some(r => 'id' in r && r.id === 2)).toBe(true));
94+
95+
expect(received).toEqual({ x: 7 });
96+
const result = responses.find(r => 'id' in r && r.id === 2) as { result?: { content: Array<{ text: string }> } };
97+
expect(result.result?.content[0]?.text).toBe('7');
6298

63-
expect(warn).not.toHaveBeenCalled();
64-
warn.mockRestore();
99+
await server.close();
65100
});
66101
});

0 commit comments

Comments
 (0)