-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: zod v4 #869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: zod v4 #869
Conversation
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 |
@avivnakar / all Updated with main, resolved conflicts, reverted to use |
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? |
Is there any update here on if this is going to get merged soon? |
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. |
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 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 Here's the the simplest code to reproduce the issue: package.json Hope, this helps! |
There was a problem hiding this 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"], |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
src/client/index.test.ts
Outdated
/* eslint-disable @typescript-eslint/no-unused-expressions */ | ||
import { Client } from "./index.js"; | ||
import { z } from "zod"; | ||
import { z } from "zod/v4"; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
src/shared/auth.ts
Outdated
dpop_bound_access_tokens_required: z.boolean().optional(), | ||
}) | ||
.passthrough(); | ||
export const OAuthProtectedResourceMetadataSchema = z.object({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good catch: https://zod.dev/v4/changelog?id=deprecates-strict-and-passthrough
refresh_token: z.string().optional(), | ||
}) | ||
.strip(); | ||
export const OAuthTokensSchema = z.object({ |
There was a problem hiding this comment.
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.
Hi @serjrd ! Looks like {
"$schema": "https://json-schema.org/draft/2020-12/schema",
"type": "integer",
"exclusiveMinimum": 0,
"maximum": 9007199254740991,
} |
Another temporary fix (beyond previously mentioned "overrides" method), useful if your package wishes to use v4. {
"resolution": {
"@modelcontextprotocol/sdk>zod": "^3.23.8",
}
} |
Thanks for everyone's patience. This PR has been updated to:
Testing this in our own MCP server I am not seeing any memory leaks (as I was when importing from I think the deciding factor here is around the In the meantime, feel free and test using @felixweinberger I saw your PR from 5 days ago. Feel free and take the lead on this if you'd like to implement separately. |
Summary
import * as z from 'zod'
instead ofimport { z } from 'zod'
to prevent memory leaksMotivation 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)Breaking Changes
Users will need to use Zod 3.25.76+ or Zod 4+
Types of changes
Checklist
Additional context