fix(server): honor Zod v4 toJSONSchema output semantics to match raw structuredContent#2467
Open
chakshu-dhannawat wants to merge 2 commits into
Open
Conversation
zod-json-schema-compat.ts called zod v4's toJSONSchema with hardcoded options and no passthrough for the asymmetry between the schema used for validation and the raw structuredContent object the server actually ships (validation never replaces or serializes the tool's returned object). This caused three real defects: - z.date() anywhere in a tool schema throws during conversion, since v4 defaults unrepresentable types to 'throw'. That crashes the entire tools/list response, not just the one tool. - Output schema fields with .default() were listed in required even though the server never runs structuredContent through the schema's output transform, so a tool response correctly omitting a defaulted field got rejected by the client's own schema validation. - additionalProperties: false was set on output schemas the server doesn't actually enforce, so a tool response with extra fields also got rejected client-side. Fixed by always using io: 'input' semantics for the v4 branch (matching what the server actually ships, for both input and output schemas), adding unrepresentable: 'any' so an unsupported type degrades gracefully instead of crashing tools/list, and overriding z.date() specifically to the date-time string format JSON.stringify already produces on the wire. Added regression tests for all three cases, gated to the v4 matrix entry since the v3 vendored converter is a separate code path this doesn't touch.
🦋 Changeset detectedLatest commit: b8301bf The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2464
The bug
toJsonSchemaCompat's Zod v4 branch (src/server/zod-json-schema-compat.ts) calls zod v4'stoJSONSchemawith hardcoded options and no passthrough. That produces three real defects, all stemming from the same root cause: the server validates a tool'sstructuredContentagainst its zod schema, but ships the tool's raw object as-is, it never runs the object through the schema's output transform. So the schema advertised intools/listmust describe that raw shape, not the fully-resolved one.z.date()anywhere in a tool schema throws during conversion (zod v4 defaultsunrepresentableto"throw"), which crashes the entiretools/listresponse, not just the one tool..default()were listed inrequired, so a tool response correctly omitting a defaulted field gets rejected by the client's own schema validation.additionalProperties: falsewas set on output schemas the server doesn't actually enforce, so a response with extra fields also gets rejected client-side.I reproduced all three against a real in-memory client/server pair before touching anything:
Defect 2 in particular isn't just a schema-advertisement nitpick, it makes the SDK's own client reject a perfectly legitimate tool response.
The fix
In the v4 branch only (the v3 vendored converter is a separate code path, untouched):
io: 'input'regardless ofopts.pipeStrategy. Since the server ships the raw object for both input and output, 'input' semantics (defaulted fields optional, noadditionalProperties: false) is correct in both cases, fixing defects 2 and 3 together.unrepresentable: 'any'so a single unsupported type degrades to an unconstrained schema instead of crashing the wholetools/listcall.overridethat rewritesz.date()specifically to{ type: "string", format: "date-time" }, the actual wire formatJSON.stringifyproduces for aDate. Without this,unrepresentable: 'any'alone would silence the crash but lose the date-time format entirely (falls back to an empty/any schema).Testing
Added three regression tests to
test/server/mcp.test.ts, gated to the Zod v4 matrix entry, covering each defect end-to-end through a real client/server pair (not just the schema shape, but that a previously-rejected legitimate tool call now succeeds). Verified they fail against the pre-fix code and pass with the fix. Full suite (npx vitest run): 2721 tests passed, 76 files, no regressions.npm run lintclean. Added a changeset.