Skip to content

Conversation

@seongwon030
Copy link

Motivation and Context

This PR fixes a compatibility issue with Zod v4 where optional schemas were incorrectly identified as required.
Specifically, isSchemaOptional in zod-compat.ts was failing to detect optional fields because it relied on internal properties that differ in Zod v4, causing optional tool arguments to be marked as required in the generated JSON Schema.

How Has This Been Tested?

I verified the fix by running the existing test suite (npm test), specifically the should support optional prompt arguments test case in src/server/mcp.test.ts, which now passes correctly.
I also manually verified that isSchemaOptional correctly returns true for z.string().optional() using Zod v4.

Breaking Changes

None. This is a bug fix that improves compatibility.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

- Prioritize `isOptional()` method check in isSchemaOptional to support both Zod v3 and v4 consistently.
- Add `typeName` check for Zod v4 internal definitions to correctly identify optional schemas.
- Fixes an issue where optional tool arguments were incorrectly marked as required in the generated JSON schema.
@seongwon030 seongwon030 requested a review from a team as a code owner December 4, 2025 09:08
@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 4, 2025

Open in StackBlitz

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/sdk@1232

commit: cdbeeaf

@mattzcarey
Copy link
Contributor

do you have a repro of this issue? All tests were passing before and it doesnt look like you changed any

@seongwon030
Copy link
Author

do you have a repro of this issue? All tests were passing before and it doesnt look like you changed any

Thanks for the review! After investigating, I found that PR #1199 already fixed this issue.
I'll close this PR. Sorry for the confusion.

@KKonstantinov
Copy link
Contributor

Closing this. In any case, thanks for your effort to raise it anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants