Skip to content

Commit a655cd2

Browse files
cdcabreracdcabrera
committed
test: review updates
1 parent b14c4f7 commit a655cd2

File tree

3 files changed

+264
-17
lines changed

3 files changed

+264
-17
lines changed

‎src/__tests__/__snapshots__/server.tools.test.ts.snap‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,127 @@ exports[`logWarningsErrors should log warnings and errors, with warnings only 1`
192192
]
193193
`;
194194

195+
exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false with error: handler 1`] = `[Error: Error message]`;
196+
197+
exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false with error: send 1`] = `
198+
[
199+
[
200+
undefined,
201+
{
202+
"args": {
203+
"loremIpsum": 7,
204+
},
205+
"id": "id-1",
206+
"t": "invoke",
207+
"toolId": "loremIpsum",
208+
},
209+
],
210+
]
211+
`;
212+
213+
exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false with full error: handler 1`] = `[Error: Error message]`;
214+
215+
exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false with full error: send 1`] = `
216+
[
217+
[
218+
undefined,
219+
{
220+
"args": {
221+
"loremIpsum": 7,
222+
},
223+
"id": "id-1",
224+
"t": "invoke",
225+
"toolId": "loremIpsum",
226+
},
227+
],
228+
]
229+
`;
230+
231+
exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false: handler 1`] = `[Error: Tool invocation failed]`;
232+
233+
exports[`makeProxyCreators should attempt to invoke a creator then throw an error on child response, ok false: send 1`] = `
234+
[
235+
[
236+
undefined,
237+
{
238+
"args": {
239+
"loremIpsum": 7,
240+
},
241+
"id": "id-1",
242+
"t": "invoke",
243+
"toolId": "loremIpsum",
244+
},
245+
],
246+
]
247+
`;
248+
249+
exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per tool, basic 1`] = `
250+
{
251+
"debug": [],
252+
"output": [
253+
[
254+
"Lorem Ipsum",
255+
{
256+
"description": "Lorem ipsum dolor sit amet",
257+
"inputSchema": "isZod = true",
258+
},
259+
[Function],
260+
],
261+
],
262+
}
263+
`;
264+
265+
exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per tool, no tools 1`] = `
266+
{
267+
"debug": [],
268+
"output": [],
269+
}
270+
`;
271+
272+
exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per tool, null JSON input schema 1`] = `
273+
{
274+
"debug": [
275+
[
276+
"Tool "Lorem Ipsum" from unknown source failed strict JSON to Zod reconstruction.",
277+
"Using fallback best-effort schema. Review the tool's inputSchema and ensure it is a valid JSON or Zod schema.",
278+
"[ZOD_SCHEMA: defined: true]",
279+
],
280+
],
281+
"output": [
282+
[
283+
"Lorem Ipsum",
284+
{
285+
"description": "Lorem ipsum dolor sit amet",
286+
"inputSchema": "isZod = true",
287+
},
288+
[Function],
289+
],
290+
],
291+
}
292+
`;
293+
294+
exports[`makeProxyCreators should attempt to return proxy creators, a function wrapper per tool, undefined JSON input schema 1`] = `
295+
{
296+
"debug": [
297+
[
298+
"Tool "Lorem Ipsum" from unknown source failed strict JSON to Zod reconstruction.",
299+
"Using fallback best-effort schema. Review the tool's inputSchema and ensure it is a valid JSON or Zod schema.",
300+
"[ZOD_SCHEMA: defined: true]",
301+
],
302+
],
303+
"output": [
304+
[
305+
"Lorem Ipsum",
306+
{
307+
"description": "Lorem ipsum dolor sit amet",
308+
"inputSchema": "isZod = true",
309+
},
310+
[Function],
311+
],
312+
],
313+
}
314+
`;
315+
195316
exports[`spawnToolsHost attempt to spawn the Tools Host, with no pluginIsolation, node 24: spawn 1`] = `
196317
{
197318
"spawn": [

‎src/__tests__/server.tools.test.ts‎

Lines changed: 128 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
composeTools
1414
} from '../server.tools';
1515
import { awaitIpc, makeId, send } from '../server.toolsIpc';
16+
import { isZodSchema } from '../server.schema';
1617

1718
jest.mock('node:child_process', () => ({
1819
spawn: jest.fn()
@@ -33,12 +34,11 @@ jest.mock('../server.toolsIpc', () => ({
3334
awaitIpc: jest.fn(),
3435
makeId: jest.fn(() => 'id-1'),
3536
isHelloAck: jest.fn((msg: any) => msg?.t === 'hello:ack'),
37+
isInvokeResult: jest.fn((msg: any) => msg?.t === 'invoke:result'),
3638
isLoadAck: jest.fn((id: string) => (msg: any) => msg?.t === 'load:ack' && msg?.id === id),
3739
isManifestResult: jest.fn((id: string) => (msg: any) => msg?.t === 'manifest:result' && msg?.id === id)
3840
}));
3941

40-
const MockLog = log as jest.MockedObject<typeof log>;
41-
4242
describe('getBuiltInToolName', () => {
4343
it('should return built-in tool name', () => {
4444
const toolName = 'loremIpsum';
@@ -59,6 +59,8 @@ describe('computeFsReadAllowlist', () => {
5959
});
6060

6161
describe('logWarningsErrors', () => {
62+
const MockLog = jest.mocked(log);
63+
6264
beforeEach(() => {
6365
jest.clearAllMocks();
6466
});
@@ -199,10 +201,10 @@ describe('debugChild', () => {
199201
});
200202

201203
describe('spawnToolsHost', () => {
202-
const MockSpawn = spawn as unknown as jest.MockedFunction<typeof spawn>;
203-
const MockAwaitIpc = awaitIpc as unknown as jest.MockedFunction<typeof awaitIpc>;
204-
const MockSend = send as unknown as jest.MockedFunction<typeof send>;
205-
const MockMakeId = makeId as unknown as jest.MockedFunction<typeof makeId>;
204+
const MockSpawn = jest.mocked(spawn);
205+
const MockAwaitIpc = jest.mocked(awaitIpc);
206+
const MockSend = jest.mocked(send);
207+
const MockMakeId = jest.mocked(makeId);
206208

207209
beforeEach(() => {
208210
jest.clearAllMocks();
@@ -263,9 +265,126 @@ describe('spawnToolsHost', () => {
263265
});
264266

265267
describe('makeProxyCreators', () => {
266-
it('should exist', () => {
267-
// placeholder test
268-
expect(makeProxyCreators).toBeDefined();
268+
const MockAwaitIpc = jest.mocked(awaitIpc);
269+
const MockSend = jest.mocked(send);
270+
const MockMakeId = jest.mocked(makeId);
271+
const MockLog = jest.mocked(log);
272+
273+
beforeEach(() => {
274+
jest.clearAllMocks();
275+
});
276+
277+
it.each([
278+
{
279+
description: 'no tools',
280+
tools: []
281+
},
282+
{
283+
description: 'basic',
284+
tools: [
285+
{
286+
id: 'loremIpsum',
287+
name: 'Lorem Ipsum',
288+
description: 'Lorem ipsum dolor sit amet',
289+
inputSchema: {},
290+
source: ''
291+
}
292+
]
293+
},
294+
{
295+
description: 'null JSON input schema',
296+
tools: [
297+
{
298+
id: 'loremIpsum',
299+
name: 'Lorem Ipsum',
300+
description: 'Lorem ipsum dolor sit amet',
301+
inputSchema: null,
302+
source: ''
303+
}
304+
]
305+
},
306+
{
307+
description: 'undefined JSON input schema',
308+
tools: [
309+
{
310+
id: 'loremIpsum',
311+
name: 'Lorem Ipsum',
312+
description: 'Lorem ipsum dolor sit amet',
313+
inputSchema: undefined,
314+
source: ''
315+
}
316+
]
317+
}
318+
])('should attempt to return proxy creators, a function wrapper per tool, $description', ({ tools }) => {
319+
const proxies = makeProxyCreators({ tools } as any, { pluginHost: { invokeTimeoutMs: 10 } } as any);
320+
const output = proxies.map(proxy => {
321+
const [name, { description, inputSchema, ...rest }, handler] = proxy();
322+
323+
return [
324+
name,
325+
{ description, inputSchema: `isZod = ${isZodSchema(inputSchema)}`, ...rest },
326+
handler
327+
];
328+
});
329+
330+
expect({
331+
output,
332+
debug: MockLog.debug.mock.calls
333+
}).toMatchSnapshot();
334+
});
335+
336+
it.each([
337+
{
338+
description: 'ok false',
339+
response: {
340+
ok: false,
341+
result: { value: 7 }
342+
}
343+
},
344+
{
345+
description: 'ok false with error',
346+
response: {
347+
ok: false,
348+
result: { value: 7 },
349+
error: { message: 'Error message' }
350+
}
351+
},
352+
{
353+
description: 'ok false with full error',
354+
response: {
355+
ok: false,
356+
result: { value: 7 },
357+
error: { message: 'Error message', stack: 'line 1\nline 2', code: 'ERR_CODE', cause: { details: 'Details' } }
358+
}
359+
}
360+
])('should attempt to invoke a creator then throw an error on child response, $description', async ({ response }) => {
361+
const tools = [
362+
{
363+
id: 'loremIpsum',
364+
name: 'Lorem Ipsum',
365+
description: 'Lorem ipsum dolor sit amet',
366+
inputSchema: {},
367+
source: ''
368+
}
369+
];
370+
371+
MockMakeId.mockReturnValue('id-1' as any);
372+
MockAwaitIpc
373+
.mockResolvedValueOnce({ t: 'invoke:result', id: 'id-1', ...response } as any);
374+
375+
const proxies = makeProxyCreators({ tools } as any, { pluginHost: { invokeTimeoutMs: 10 } } as any);
376+
const [_name, _schema, handler]: any = proxies.map(proxy => {
377+
const [name, { description, inputSchema, ...rest }, handler] = proxy();
378+
379+
return [
380+
name,
381+
{ description, inputSchema: `isZod = ${isZodSchema(inputSchema)}`, ...rest },
382+
handler
383+
];
384+
})[0];
385+
386+
await expect(handler({ loremIpsum: 7 })).rejects.toMatchSnapshot('handler');
387+
expect(MockSend.mock.calls).toMatchSnapshot('send');
269388
});
270389
});
271390

‎src/server.tools.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,12 @@ const spawnToolsHost = async (
272272

273273
/**
274274
* Recreate parent-side tool creators that forward invocations to the Tools Host.
275-
* - Parent does not perform validation; the child validates with Zod at invoke time.
275+
* - Parent does not perform validation; the child validates with Zod at invocation.
276276
* - A minimal Zod inputSchema from the parent is required to trigger the MCP SDK parameter
277-
* validation for tool invocation. This schema is not used, it is a noop.
278-
* - Invocation errors from the child preserve `error.code` and `error.details` for UX.
277+
* validation.
278+
* - There is an unreachable defensive check in `makeProxyCreators` that ensures the Zod schema
279+
* always returns a value.
280+
* - Invocation errors from the child preserve `error.code` and `error.details` for debugging.
279281
*
280282
* @param {HostHandle} handle - Tools Host handle.
281283
* @param {GlobalOptions} options - Global options.
@@ -297,22 +299,27 @@ const makeProxyCreators = (
297299

298300
log.debug(
299301
`Tool "${name}" from ${tool.source || 'unknown source'} failed strict JSON to Zod reconstruction.`,
300-
`Review the tool's inputSchema and ensure it is a valid JSON or Zod schema.`
302+
`Using fallback best-effort schema. Review the tool's inputSchema and ensure it is a valid JSON or Zod schema.`,
303+
`[ZOD_SCHEMA: defined: ${Boolean(zodSchema)}]`
301304
);
302305
}
303306

307+
// Defensive check only. Currently, unreachable due to `jsonSchemaToZod`'s current return/response. Zod is integral
308+
// to the MCP SDK, in the unlikely event that the Zod schema is still unavailable, fallback again. All hail Zod!
304309
if (!zodSchema) {
310+
zodSchema = z.looseObject({});
311+
305312
log.error(
306313
`Tool "${name}" from ${tool.source || 'unknown source'} failed strict and best‑effort JSON to Zod reconstruction.`,
307-
`Falling back to permissive schema for SDK broadcast. Review the inputSchema.`
314+
`Falling back to permissive schema for SDK broadcast. Review the inputSchema.`,
315+
`[ZOD_SCHEMA: defined: ${Boolean(zodSchema)}]`
308316
);
309317
}
310318

311-
// Broadcast the tool's input schema towards clients/agents. Because Zod is integral to the MCP SDK,
312-
// in the unlikely event that the Zod schema is still unavailable, fallback again. All hail Zod!
319+
// Broadcast the tool's input schema towards clients/agents.
313320
const schema = {
314321
description: tool.description,
315-
inputSchema: zodSchema || z.looseObject({})
322+
inputSchema: zodSchema
316323
};
317324

318325
const handler = async (args: unknown) => {

0 commit comments

Comments
 (0)