From 87d9448404361ebb202c0fb97e157a785489aa7b Mon Sep 17 00:00:00 2001 From: Thilina Date: Sun, 8 Nov 2026 17:41:45 +0530 Subject: [PATCH] fix(server): validate non-object outputSchema roots instead of crashing (#1308) tools/call crashed for any outputSchema that is not an object schema (.optional(), .nullable(), .nullish(), z.union(...)): normalizeObjectSchema returns undefined for non-object roots, and validateToolOutput passed that undefined straight to safeParseAsync ("Cannot read properties of undefined (reading '_zod')"). The same registration also silently omitted the schema from tools/list, which disabled client-side success validation for the tool (clients only build validators from the listing). - validateToolOutput now falls back to parsing the schema directly when normalization fails, mirroring the input-validation path - registration warns when an output schema cannot be advertised, so the listing omission is visible instead of silent. It cannot be converted: the wire schema requires type "object" at the outputSchema root per the MCP specification, so an anyOf root would make spec-valid clients reject the listing itself - tests cover all four root shapes over the zod v3/v4 matrix, pin the listing omission as documented behaviour, and assert the warning --- .../fix-non-object-output-schema-roots.md | 6 ++ src/server/mcp.ts | 24 +++++-- src/shared/outputSchemaWarning.ts | 18 +++++ test/server/mcp.test.ts | 68 +++++++++++++++++++ 4 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 .changeset/fix-non-object-output-schema-roots.md create mode 100644 src/shared/outputSchemaWarning.ts diff --git a/.changeset/fix-non-object-output-schema-roots.md b/.changeset/fix-non-object-output-schema-roots.md new file mode 100644 index 0000000000..81b265d23b --- /dev/null +++ b/.changeset/fix-non-object-output-schema-roots.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/sdk': patch +--- + +Fix the `tools/call` crash for non-object `outputSchema` roots (`.optional()`, `.nullable()`, `.nullish()`, `z.union(...)`): output validation now parses the schema directly when it cannot be normalized to an object, mirroring the input-validation path. `tools/list` still cannot +advertise such schemas (the wire schema requires `type: "object"` at the root, per the MCP specification), but registration now warns about the omission instead of dropping it silently. diff --git a/src/server/mcp.ts b/src/server/mcp.ts index aa21af7bbf..ff75c42f8c 100644 --- a/src/server/mcp.ts +++ b/src/server/mcp.ts @@ -60,6 +60,7 @@ import { RequestHandlerExtra } from '../shared/protocol.js'; import { Transport } from '../shared/transport.js'; import { validateAndWarnToolName } from '../shared/toolNameValidation.js'; +import { issueNonObjectOutputSchemaWarning } from '../shared/outputSchemaWarning.js'; import { ExperimentalMcpServerTasks } from '../experimental/tasks/mcp-server.js'; import type { ToolTaskHandler } from '../experimental/tasks/interfaces.js'; import { ZodOptional } from 'zod'; @@ -373,9 +374,13 @@ export class McpServer { ); } - // if the tool has an output schema, validate structured content - const outputObj = normalizeObjectSchema(tool.outputSchema) as AnyObjectSchema; - const parseResult = await safeParseAsync(outputObj, result.structuredContent); + // if the tool has an output schema, validate structured content. + // Try to normalize to object schema first (for raw shapes and object schemas) + // If that fails, use the schema directly (for union/intersection/etc), mirroring + // the input-validation path — non-object roots must validate, not crash (#1308) + const outputObj = normalizeObjectSchema(tool.outputSchema); + const schemaToParse = outputObj ?? (tool.outputSchema as AnySchema); + const parseResult = await safeParseAsync(schemaToParse, result.structuredContent); if (!parseResult.success) { const error = 'error' in parseResult ? parseResult.error : 'Unknown error'; const errorMessage = getParseErrorMessage(error); @@ -950,11 +955,20 @@ export class McpServer { // Validate tool name according to SEP specification validateAndWarnToolName(name); + const normalizedInputSchema = getZodSchemaObject(inputSchema); + const normalizedOutputSchema = getZodSchemaObject(outputSchema); + + // Warn (rather than refuse) when the output schema cannot be advertised in + // tools/list: the call-time validation still applies, so registration proceeds. + if (normalizedOutputSchema && !normalizeObjectSchema(normalizedOutputSchema)) { + issueNonObjectOutputSchemaWarning(name); + } + const registeredTool: RegisteredTool = { title, description, - inputSchema: getZodSchemaObject(inputSchema), - outputSchema: getZodSchemaObject(outputSchema), + inputSchema: normalizedInputSchema, + outputSchema: normalizedOutputSchema, annotations, execution, _meta, diff --git a/src/shared/outputSchemaWarning.ts b/src/shared/outputSchemaWarning.ts new file mode 100644 index 0000000000..bd2ce8b809 --- /dev/null +++ b/src/shared/outputSchemaWarning.ts @@ -0,0 +1,18 @@ +/** + * Registration-time warning for non-object output schemas. + * + * The MCP specification requires `Tool.outputSchema` to be an object schema + * (type: "object" at its root), so a non-object root (union, optional, …) cannot be + * serialized into `tools/list` — clients would never see it and would not validate + * the tool's output. `tools/call` still validates against the schema itself. This + * warning makes the listing omission visible instead of silent (#1308). + */ + +export function issueNonObjectOutputSchemaWarning(name: string): void { + console.warn(`Tool outputSchema warning for "${name}":`); + console.warn( + ' - The outputSchema is not an object schema, so tools/list cannot advertise it (the MCP specification requires type "object" at the root).' + ); + console.warn(" - tools/call still validates results against it, but clients will not validate this tool's output."); + console.warn('Consider wrapping the schema in z.object(...) — e.g. merging an error envelope in as an optional `error` property.'); +} diff --git a/test/server/mcp.test.ts b/test/server/mcp.test.ts index 9a8db72117..b1460fef7e 100644 --- a/test/server/mcp.test.ts +++ b/test/server/mcp.test.ts @@ -1437,6 +1437,74 @@ describe.each(zodTestMatrix)('$zodVersionLabel', (entry: ZodMatrixEntry) => { expect(JSON.parse(textContent.text)).toEqual(result.structuredContent); }); + /*** + * Test: Non-object outputSchema roots validate instead of crashing (#1308) + * + * optional/nullable/nullish/union output schemas previously crashed every + * tools/call with "Cannot read properties of undefined (reading '_zod')" and + * were silently omitted from tools/list. The call must now validate (mirroring + * the input path's fallback), and the listing omission — unavoidable, since the + * wire schema requires type "object" at the outputSchema root — must be warned + * about at registration rather than silent. + */ + test('should validate output for non-object outputSchema roots and warn about the listing omission', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const mcpServer = new McpServer({ + name: 'test server', + version: '1.0' + }); + const client = new Client({ + name: 'test client', + version: '1.0' + }); + + const cases = [ + { name: 'optional-root', schema: z.object({ data: z.string() }).optional() }, + { name: 'nullable-root', schema: z.object({ data: z.string() }).nullable() }, + { name: 'nullish-root', schema: z.object({ data: z.string() }).nullish() }, + { + name: 'union-root', + schema: z.union([z.object({ data: z.string() }), z.object({ value: z.string() })]) + } + ]; + + for (const { name, schema } of cases) { + mcpServer.registerTool(name, { description: name, outputSchema: schema }, async () => ({ + structuredContent: { data: 'hello' }, + content: [] + })); + } + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); + + const listResult = await client.request({ method: 'tools/list' }, ListToolsResultSchema); + expect(listResult.tools).toHaveLength(cases.length); + // Pinned: the wire schema (Tool.outputSchema requires type "object" at the + // root) means these schemas cannot be advertised. The pin keeps the omission + // a documented behaviour; the registration warning keeps it non-silent. + for (const tool of listResult.tools) { + expect(tool.outputSchema).toBeUndefined(); + } + + // Every root that previously crashed the call now validates cleanly. + for (const { name } of cases) { + const result = await client.request({ method: 'tools/call', params: { name, arguments: {} } }, CallToolResultSchema); + expect(result.isError).toBeFalsy(); + expect(result.structuredContent).toEqual({ data: 'hello' }); + } + + // The listing omission is warned about once per tool at registration. + const warnings = warnSpy.mock.calls.map(call => call.join(' ')).join('\n'); + expect(warnings).toContain('outputSchema warning'); + expect(warnings).toContain('type "object"'); + for (const { name } of cases) { + expect(warnings).toContain(`"${name}"`); + } + warnSpy.mockRestore(); + }); + /*** * Test: Tool with Output Schema Must Provide Structured Content */