Skip to content

Conversation

dclark27
Copy link

@dclark27 dclark27 commented Aug 12, 2025

Summary

  • Upgrade to zod v4
  • Replaces zod-to-json-schema with zod v4 toJsonSchema
  • Adjust usage to be in best practice with zod v4 migration guide
  • Adjust imports to use import * as z from 'zod' instead of import { z } from 'zod' to prevent memory leaks

Motivation and Context

This has been a blocking issue for my team and many others. When versions mismatch across dependencies with older Zod versions, memory leaks become present and can slow or stall development.

This also will help unblock downstream dependencies, such as with vercel's AI SDK and mcp-tools.

#494
vercel/ai#7160
vercel/mcp-handler#93

How Has This Been Tested?

  • npm test → 37 suites / 755 tests / 0 failures (macOS, Node ≥18)
  • Published and running on Socotra's MCP server which hosts dozens of tools and completions

Breaking Changes

Users will need to use Zod 3.25.76+ or Zod 4+

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

Additional context

@dclark27 dclark27 requested review from a team, domdomegg and ochafik August 12, 2025 17:43
@dclark27 dclark27 changed the title Zod v4 feat: zod v4 Aug 12, 2025
@ihrpr
Copy link
Contributor

ihrpr commented Aug 13, 2025

Thanks for working on this. We are preparing for Typescript V2, this potentially can be included there

@ihrpr ihrpr added this to the v2 milestone Aug 13, 2025
@dclark27
Copy link
Author

Thanks for working on this. We are preparing for Typescript V2, this potentially can be included there

Let me know if I can help! @ihrpr

@dclark27
Copy link
Author

dclark27 commented Aug 19, 2025

@avivnakar / all

Updated with main, resolved conflicts, reverted to use default over prefault. I've been running since this PR went live by publishing behind our org and it's been lovely!

@ochafik
Copy link
Contributor

ochafik commented Sep 8, 2025

Hi @dclark27, thanks for your work on this!

I wonder if the v3 & v4 compatibility can be tested (and maybe documented)? Maybe adding v3 as a dev dependency and have some tests explicitly importing the /v3 paths?

@Scalahansolo
Copy link

Is there any update here on if this is going to get merged soon?

@dclark27
Copy link
Author

@ochafik Looking for a little guidance. I think this will likely need to be redone given the merge conflicts now, happy to do it too. Are you still wanting to wait for #792?

If not, I will tackle Sunday and make sure I work out v3 backwards compatibility. Thanks for everyone's patience!

@ardabeyazoglu
Copy link

I actually merged your pull request with the latest upstream, and another pull for jest->vitest migration in my fork and it works fine. Until the main repo has stable v4 support, i will keep using it. Of course, it only works on my codebase, there might be issues I haven't encountered.

@serjrd
Copy link

serjrd commented Oct 16, 2025

Not sure if the problem I stumbled upon belongs here, but maybe it will save some of you some time.

I was using zod4 with "@modelcontextprotocol/sdk": "npm:@socotra/modelcontextprotocol-sdk" and everything was fine during the development process when I served my model with LMStudio.

But as soon as I tried serving the model with llama.cpp server I got integer parameters truncation to a single digit. Tried different models - same result.

server.registerTool('test',
  {
    title: 'Get test record by ID',
    description: 'Get test record by ID',
    inputSchema: { testId: z.number().int().positive().describe('The ID of the test record') }
  },
  ({ testId }) => {
    return {
      content: [{ type: 'text', text: 'You requested test record ID: ' + testId }]
    }
  }
)

Leads to:

{
  type: 'reasoning',
  text: "The user wants to call the function test with testId: 654321. We'll need to use the function."
}
{
  type: 'tool-call',
  toolCallId: 'ADR3LRcFhzirsDqSvlfKPxlYmYLh3fKB',
  toolName: 'test',
  input: { testId: 6 },
  providerExecuted: undefined,
  providerMetadata: undefined,
  dynamic: true
}

Again: same code, same model, but served with LMStudio - no issues. The tool gets called with proper argument 654321.

If I declare 'testId' to be a string - both LMStudio and llama.cpp work fine.

When I switch to "zod": "3" and "@modelcontextprotocol/sdk": "^1.20.0", llama.cpp server works fine.

Here's the the simplest code to reproduce the issue:

package.json
test_llm.js
test_mcp.js

Hope, this helps!

@felixweinberger felixweinberger mentioned this pull request Oct 16, 2025
9 tasks
@felixweinberger felixweinberger linked an issue Oct 16, 2025 that may be closed by this pull request
Copy link
Contributor

@felixweinberger felixweinberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dclark27 thank you for working on this; keen to help get this merged asap, apologies for the time it took us to come back to this.

jest.config.js Outdated
"/node_modules/(?!eventsource)/"
],
testPathIgnorePatterns: ["/node_modules/", "/dist/"],
modulePathIgnorePatterns: ["<rootDir>/dist"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this? This seems like an unintentional change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this in as I was seeing issues running the auth test suite. Adding this prevent the mocks from duplicating and was able to get my tests back running again. We can remove if perhaps it's just my local setup.

/* eslint-disable @typescript-eslint/no-unused-expressions */
import { Client } from "./index.js";
import { z } from "zod";
import { z } from "zod/v4";
Copy link
Contributor

@felixweinberger felixweinberger Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found conflicting guidance on this import. https://zod.dev/library-authors?id=how-to-support-zod-4 suggests this import should never be used - it should be zod/v4/core.

On the other hand I'm not sure this repo counts as a "Library" as intended by these docs; currently zod isn't a peer dependency, so I'm not sure we need to support both v3 and v4 simultaneously in the code - we just need to ensure that clients using v3 are still compatible with servers using v4 and vice versa.

Ultimately because we have a server-client model in MCP and the two sides communicate via JSONRPC generated & parsed by Zod, I believe it doesn't actually matter if the sides use different versions of Zod. Just like a TypeScript based client might use Zod but a Python server would use Pydantic for validation. If true, my preference would be to just stick with import { z } from "zod"; and leave it at that without dealing with multiple versions unnecessarily.

Maybe that's something @colinhacks could help answer or suggest how we gain confidence on this? In general JSON generated by v3 should normally be parseable by v4 and vice versa I assume, I'm not sure what if anything would break over the wire.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I've run some tests on this as I was trying to migrate a big project that uses modelcontextprotocol/typescript-sdk to Zod 4 in a big monorepo and I've had some issues with npm hosting Zod.
It seems that the issue with libraries doing import { z } from "zod" in this setup is that zod will use either Zod 3 if package.json of the host app contains "zod": "^3.25.76" and Zod 4 if the host is using "zod": "^4.0.0".

Not sure of the impact of peerDependencies vs devDependencies in the context of this project, but not specifying the import (zod/v3, zod/v4 or zod/v4/core) seems to be able to have some impacts in some contexts.

Copy link
Contributor

@felixweinberger felixweinberger Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mmorainville-fountain that's great input - in that case I don't see a downside to using the zod/v4 import explicitly.

After writing this review + speaking with @ochafik offline I also realized there's a bit more to worry about than just JSONRPC transport layer between clients and server.

The quick start shows how SDK users would define their input and output schema in terms of zod and they might be using zod v3 to pass into say registerTools. If we internally then use zod v4 within the SDK, I'm not sure that's directly compatible.

I'll see if I can figure out a way to do some testing to find out - given how highly requested this is I wouldn't want this to depend on v2 ideally, so we'd need something fully backwards compatible. Maybe moving to a peerDependency is the right move.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just spotted this comment from @dclark27 #869 (comment) exploring this with a potential regression test, will take a look at this.

dpop_bound_access_tokens_required: z.boolean().optional(),
})
.passthrough();
export const OAuthProtectedResourceMetadataSchema = z.object({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refresh_token: z.string().optional(),
})
.strip();
export const OAuthTokensSchema = z.object({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't strict() before - it used strip() which happens on object by default now AFAIU.

@ochafik
Copy link
Contributor

ochafik commented Oct 16, 2025

I was using zod4 with "@modelcontextprotocol/sdk": "npm:@socotra/modelcontextprotocol-sdk" and everything was fine during the development process when I served my model with LMStudio.

But as soon as I tried serving the model with llama.cpp server I got integer parameters truncation to a single digit. Tried different models - same result.
...
inputSchema: { testId: z.number().int().positive().describe('The ID of the test record') }

Hi @serjrd ! Looks like z.number().int() changed in v4: it now adds in a maximum constraint (of Number.MAX_SAFE_INTEGER), which must be triggering a bug in the llama.cpp schema->grammar conversion, feel free to file a bug on llama.cpp and tag me in there. In the meanwhile, maybe just drop the .int() and mention the id is an integer in the description itself.

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "type": "integer",
  "exclusiveMinimum": 0,
  "maximum": 9007199254740991,
}

@felixweinberger felixweinberger self-assigned this Oct 16, 2025
@felixweinberger felixweinberger added dependencies Pull requests that update a dependency file P0 Broken core functionality, security issues, critical missing feature needs maintainer action Potentially serious issue - needs proactive fix and maintainer attention labels Oct 16, 2025
@dvanmali
Copy link

Another temporary fix (beyond previously mentioned "overrides" method), useful if your package wishes to use v4.

{
  "resolution": {
    "@modelcontextprotocol/sdk>zod": "^3.23.8",
  }
}

@dclark27 dclark27 requested review from a team as code owners October 20, 2025 21:50
@dclark27
Copy link
Author

dclark27 commented Oct 20, 2025

Thanks for everyone's patience. This PR has been updated to:

  • Implement feedback from comments on this PR (thank you!)
  • Resolved merge conflicts
  • Fix formatting
  • Changed how we import from zod to use the import * as z from 'zod'

Testing this in our own MCP server I am not seeing any memory leaks (as I was when importing from import { z } from 'zod') and it matches the guidance from Zod's migration guide.

I think the deciding factor here is around the z.toJsonSchema. Because zod-to-json-schema is no longer maintained, we need to either write our own version of it, use an aliased package that other people have forked and published, or save this PR for a v2 when we are ok with breaking changes.

In the meantime, feel free and test using npm i @socotra/modelcontextprotocol-sdk

@felixweinberger I saw your PR from 5 days ago. Feel free and take the lead on this if you'd like to implement separately.

@ihrpr ihrpr modified the milestones: v2, HPR Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file needs maintainer action Potentially serious issue - needs proactive fix and maintainer attention P0 Broken core functionality, security issues, critical missing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ListTools request handler fails to generate inputSchema (jsonSchema) MCP SDK v1.17.5 Incompatible with Zod v4 - Breaking Changes