refactor: fix CI, use zod v4, and latest method in mcp#332
Conversation
registerToolregisterTool in MCP
|
The CI failure appears to be a regression in I've opened an issue about it in |
registerTool in MCPtool with registerTool in @eslint/mcp
tool with registerTool in @eslint/mcpzod v4, v10_config_lookup_from_file and latest method in mcp
This comment was marked as resolved.
This comment was marked as resolved.
zod v4, v10_config_lookup_from_file and latest method in mcpzod v4, v10 flag and latest method in mcp
zod v4, v10 flag and latest method in mcpzod v4, and latest method in mcp
| $schema: "http://json-schema.org/draft-07/schema#", | ||
| additionalProperties: false, |
There was a problem hiding this comment.
If I don't add these two lines, the tests fail, so I've added them.
However, I'm not sure whether this is considered a user-facing change.
There was a problem hiding this comment.
I think it's fine to re-add these lines. These were removed in commit 45842c0 to fix compatibility with version 1.22.0 of @modelcontextprotocol/sdk.
zod v4, and latest method in mcpzod v4, and latest method in mcp
…ed-mcpserver-tool
packages/mcp/package.json
Outdated
| }, | ||
| "dependencies": { | ||
| "@modelcontextprotocol/sdk": "^1.22.0", | ||
| "@modelcontextprotocol/sdk": "^1.23.0", |
There was a problem hiding this comment.
With @modelcontextprotocol/sdk 1.23.0, the build completes without issues. Anyway, with version 1.24.1 (latest at the time of writing), I'm getting a type error:
$ npm run build
> @eslint/mcp@0.2.0 build
> tsc
../../node_modules/@modelcontextprotocol/sdk/dist/esm/server/index.d.ts:1:25 - error TS7016: Could not find a declaration file for module 'express'. '.../rewrite/node_modules/express/index.js' implicitly has an 'any' type.
Try `npm i --save-dev @types/express` if it exists or add a new declaration (.d.ts) file containing `declare module 'express';`
1 import { Express } from 'express';
~~~~~~~~~
Found 1 error in ../../node_modules/@modelcontextprotocol/sdk/dist/esm/server/index.d.ts:1The error can be fixed by installing @types/express as the message suggests.
fasttime
left a comment
There was a problem hiding this comment.
Overall, my impression is that @modelcontextprotocol/sdk isn't very mature, considering that each of the last three minor releases (1.22.0, 1.23.0 and 1.24.0) seems to introduce a new incompatibility. If this trend continues, I'd suggest shrinking the version range for that package with a tilde.
There was a problem hiding this comment.
Pull request overview
This PR addresses CI failures caused by TypeScript type instantiation errors and updates dependencies to resolve version conflicts. The primary issue stemmed from @modelcontextprotocol/sdk v1.23.0 introducing Zod v4 support, which conflicted with the repository's use of both Zod v3 (in @eslint/mcp) and Zod v4 (used internally by knip). The PR upgrades to Zod v4 with v3 compatibility, updates the MCP SDK to the latest version, and incorporates formatting changes from a newer Prettier release.
Key changes:
- Upgraded
zodfrom v3.24.4 to v4.1.13 using thezod/v3compatibility import path - Updated
@modelcontextprotocol/sdkto v1.24.1 and migrated from deprecatedmcpServer.tool()tomcpServer.registerTool() - Updated
prettierto v3.7.3, resulting in formatting adjustments to TypeScript files
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/mcp/package.json | Updated dependencies: @modelcontextprotocol/sdk to v1.24.1, zod to v4.1.13, prettier to v3.7.3; added @types/express devDependency |
| packages/mcp/src/mcp-server.js | Changed zod import to use v3 compatibility layer and migrated to registerTool() API |
| packages/mcp/tests/mcp-server.test.js | Updated JSON schema assertion to include $schema and additionalProperties fields required by new SDK version |
| packages/core/tests/types/types.test.ts | Prettier formatting adjustments to TypeScript generic type declarations |
| packages/core/src/types.ts | Prettier formatting adjustments to TypeScript interface declarations |
| package.json | Updated prettier to v3.7.3 in root package.json |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fasttime
left a comment
There was a problem hiding this comment.
LGTM, thanks! I've verified that the MCP server behaves as expected after these changes.
Prerequisites checklist
What is the purpose of this pull request?
In this PR, I've made several updates, particularly for
@eslint/mcp.Unfortunately, some of the problems have recently arisen in the
rewriterepository, so this PR includes a bit of changes especially for@eslint/mcp.This PR includes the following fixes:
1. Upgrade
@modelcontextprotocol/sdktov1.23.0andzodtov4Two days ago,
@modelcontextprotocol/sdk@1.23.0was released.https://github.com/modelcontextprotocol/typescript-sdk/releases/tag/1.23.0
Shortly after that release, our CI began failing with the
TS2589: Type instantiation is excessively deep and possibly infiniteerror.I tried to resolve it myself but couldn't find a suitable solution, so I opened an issue in
@modelcontextprotocol/sdk: modelcontextprotocol/typescript-sdk#1180The issue was traced to the newly introduced Zod v4 support in
@modelcontextprotocol/sdk@1.23.0.This only affected the
eslint/rewriterepository because the dev-dependencyknipuses Zod v4 internally, while@eslint/mcpdepends on Zod v3, causing a version conflict. The SDK team provided a clear explanation here: modelcontextprotocol/typescript-sdk#1180 (comment)To avoid the conflict, I've bumped Zod to v4. Zod v4 offers v3 compatibility via
zod/v3, so no code changes were necessary.2. Replace deprecated
mcpServer.toolusage withmcpServer.registerToolPer the JSDoc comments in
@modelcontextprotocol/sdk,mcpServer.toolis deprecated, so I've replaced it with the recommendedmcpServer.registerTool.3. The recently released
prettier@3.7.1has caused formatting errors:Yesterday,
prettier@3.7.1was released and caused formatting errors, particularly in.tsfiles.I've updated it accordingly, which introduced some additional code diffs.
What changes did you make? (Give an overview)
In this PR, I've fixed CI and use
zodv4, and latest method inmcpRelated Issues
Ref: modelcontextprotocol/typescript-sdk#1180 (comment)
Is there anything you'd like reviewers to focus on?
The behavior of MCP has not changed, so I've marked the prefix as
refactor, though I'm not sure about that.