-
-
Notifications
You must be signed in to change notification settings - Fork 953
refactor(mcp): Remove Zod v3 compatibility workarounds #992
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
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request upgrades the Zod SDK integration by updating the MCP SDK dependency to version 1.24.0 and removing v3-specific compatibility workarounds. All tool and prompt files transition from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Areas for attention:
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a significant refactoring effort to eliminate legacy Zod v3 compatibility code. By upgrading the MCP SDK to a version that fully supports Zod v4, the project can now leverage a unified Zod implementation, leading to a more consistent, type-safe, and streamlined codebase. This change reduces technical debt and simplifies future development by removing unnecessary workarounds. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Deploying repomix with
|
| Latest commit: |
7a272d1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a5c248d8.repomix.pages.dev |
| Branch Preview URL: | https://refactor-remove-zod-v3-compa.repomix.pages.dev |
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.
Code Review
This is an excellent refactoring that successfully removes the Zod v3 compatibility workarounds. By upgrading the @modelcontextprotocol/sdk dependency, you've cleanly removed the @ts-nocheck comments, updated the Zod imports, and eliminated the .shape as any casts. The changes are applied consistently across all the relevant MCP files, making the codebase more maintainable and unifying the Zod version. Great work!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #992 +/- ##
=======================================
Coverage 90.38% 90.38%
=======================================
Files 110 110
Lines 7890 7890
Branches 1528 1528
=======================================
Hits 7131 7131
Misses 759 759 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🔍 Code Review by ClaudeOverall AssessmentThis PR successfully removes Zod v3 compatibility workarounds after the MCP SDK v1.24.0 update. The changes are clean, well-structured, and follow the project's coding guidelines. The refactoring reduces technical debt by eliminating 56 lines of compatibility code while maintaining functionality. ✅ Strengths
🎯 Code Quality & Best PracticesDetails
|
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mcp/tools/grepRepomixOutputTool.ts (1)
244-263: Context separator informatSearchResultsmay insert extra--blocksUnrelated to this PR’s Zod changes, but while here: the gap check
if (resultLines.length > 0 && start > Math.min(...addedLines) + 1) { resultLines.push('--'); }uses the minimum added line index. This can add a
--separator even when a new match’s context overlaps or directly abuts the most recent context block. Comparing to the maximum added index would better reflect “there is a gap after the last emitted line”:- if (resultLines.length > 0 && start > Math.min(...addedLines) + 1) { + const maxAdded = Math.max(...addedLines); + if (resultLines.length > 0 && start > maxAdded + 1) { resultLines.push('--'); }Not urgent, but worth tightening when you next touch this helper.
🧹 Nitpick comments (1)
src/mcp/prompts/packRemoteRepositoryPrompts.ts (1)
27-29: Stale comment about “compress” parameterMinor nit: the comment
// Convert compress string to booleanno longer matches the handler signature (nocompressargument). Consider removing or updating it to avoid confusion for future maintainers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
package.json(1 hunks)src/mcp/prompts/packRemoteRepositoryPrompts.ts(1 hunks)src/mcp/tools/attachPackedOutputTool.ts(2 hunks)src/mcp/tools/fileSystemReadDirectoryTool.ts(2 hunks)src/mcp/tools/fileSystemReadFileTool.ts(2 hunks)src/mcp/tools/grepRepomixOutputTool.ts(2 hunks)src/mcp/tools/mcpToolRuntime.ts(0 hunks)src/mcp/tools/packCodebaseTool.ts(2 hunks)src/mcp/tools/packRemoteRepositoryTool.ts(2 hunks)src/mcp/tools/readRepomixOutputTool.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/mcp/tools/mcpToolRuntime.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Test (macos-latest, 22.x)
- GitHub Check: Build and run (windows-latest, 24.x)
- GitHub Check: Build and run (macos-latest, 24.x)
- GitHub Check: Build and run (macos-latest, 22.x)
- GitHub Check: Build and run (windows-latest, 20.x)
- GitHub Check: Build and run (windows-latest, 25.x)
- GitHub Check: Build and run with Bun (windows-latest, latest)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Test (macos-latest, 25.x)
- GitHub Check: Test (windows-latest, 25.x)
- GitHub Check: Test coverage
- GitHub Check: Test with Bun (macos-latest, latest)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 24.x)
- GitHub Check: claude-review
- GitHub Check: Cloudflare Pages
🔇 Additional comments (10)
package.json (1)
75-81: MCP SDK version bump looks consistent with Zod v4 migrationUpdating
@modelcontextprotocol/sdkto^1.24.0is aligned with switching offzod/v3and using native Zod v4 schemas. Since this relies on external SDK behavior, just confirm against the 1.24.0 release notes thatregisterTool/promptnow officially accept Zod v4 schemas directly (which your PR description suggests).src/mcp/tools/fileSystemReadDirectoryTool.ts (1)
5-5: Direct Zod v4 schemas in tool registration look correctSwitching to
import { z } from 'zod'and wiringinputSchema: fileSystemReadDirectoryInputSchema/outputSchema: fileSystemReadDirectoryOutputSchemamatches the new MCP SDK expectations and keeps thez.infer<...>satisfiescheck meaningful. No functional change beyond using full schemas.Also applies to: 31-32
src/mcp/tools/fileSystemReadFileTool.ts (1)
5-5: Schema wiring aligned with Zod v4 + MCP SDKThe move to
import { z } from 'zod'and passingfileSystemReadFileInputSchema/fileSystemReadFileOutputSchemadirectly intoregisterToolis consistent with the directory tool and keeps thesatisfies z.infer<...>guard accurate. No issues from a typing or runtime perspective.Also applies to: 32-33
src/mcp/tools/grepRepomixOutputTool.ts (1)
4-4: Zod v4 integration and schema usage are consistentUsing
import { z } from 'zod'and passinggrepRepomixOutputInputSchema/grepRepomixOutputOutputSchemadirectly intoregisterToolmatches the new MCP SDK pattern and keeps the output object validated viasatisfies z.infer<...>. Looks good and behaviorally equivalent to the prior.shapeworkaround.Also applies to: 87-88
src/mcp/tools/packCodebaseTool.ts (1)
4-4: Pack-codebase tool now correctly exposes full Zod schemasThe change to
import { z } from 'zod'and to usepackCodebaseInputSchema/packCodebaseOutputSchemadirectly inregisterToolis consistent with the MCP SDK upgrade and with other tools in this PR. The runtime behavior and CLI orchestration remain unchanged.Also applies to: 67-68
src/mcp/tools/attachPackedOutputTool.ts (1)
5-5: Attach-packed-output tool Zod wiring matches new patternImporting
zfrom'zod'and passingattachPackedOutputInputSchema/attachPackedOutputOutputSchemastraight intoregisterToolaligns with the other MCP tools and the SDK’s Zod v4 support. The metrics computation and call toformatPackToolResponseare untouched.Also applies to: 258-259
src/mcp/tools/readRepomixOutputTool.ts (1)
4-4: Read-output tool correctly updated to Zod v4 schemasThe switch to
import { z } from 'zod'and usingreadRepomixOutputInputSchema/readRepomixOutputOutputSchemadirectly inregisterToolis consistent with the rest of the MCP tools. The return payload is still checked viasatisfies z.infer<...>, so the schema and implementation stay in sync.Also applies to: 43-44
src/mcp/prompts/packRemoteRepositoryPrompts.ts (1)
2-2: Prompt Zod import updated cleanly to v4Updating the prompt file to
import { z } from 'zod'keeps its argument schema consistent with the rest of the MCP layer and with the new SDK version; no behavior change.src/mcp/tools/packRemoteRepositoryTool.ts (2)
4-4: LGTM! Clean upgrade to standard Zod import.The transition from
zod/v3to the standardzodimport is correct for Zod v4. This aligns with the MCP SDK v1.24.0 upgrade that adds native Zod v4 support.
71-72: LGTM! Proper Zod v4 schema registration.Passing schemas directly to
inputSchemaandoutputSchemais the correct approach for Zod v4, eliminating the.shape as anyworkarounds needed for v3 compatibility. This improves type safety and aligns with the MCP SDK's updated expectations.
f9f29cf to
edd18cc
Compare
🔍 Code Review by ClaudeOverall AssessmentExcellent refactoring PR that successfully removes Zod v3 compatibility workarounds following the MCP SDK v1.24.0 upgrade. The changes are clean, consistent, and improve code maintainability by eliminating 68 lines of technical debt. ✅ Strengths
🎯 Code QualityDetailsThe refactoring maintains high code quality:
|
MCP SDK 1.24.0 now fully supports Zod v4, allowing us to remove the compatibility workarounds that were introduced in #923. Changes: - Update @modelcontextprotocol/sdk from ^1.22.0 to ^1.24.0 - Remove @ts-nocheck comments from all MCP tool and prompt files - Change imports from 'zod/v3' to 'zod' - Remove .shape as any casts and biome-ignore comments - Pass Zod schemas directly to registerTool() instead of .shape - Remove src/mcp/tsconfig.json (no longer needed without Zod v3)
edd18cc to
7a272d1
Compare
🔍 Code Review by ClaudeOverall AssessmentThis PR successfully removes Zod v3 compatibility workarounds after the MCP SDK v1.24.0 update. The changes are clean, well-structured, and follow the project's coding guidelines. The refactoring reduces technical debt by eliminating 68 lines of compatibility code while maintaining functionality. ✅ Strengths
🎯 Code Quality & Best PracticesDetails
|
Summary
This PR removes the Zod v3 compatibility workarounds that were introduced in #923 when we upgraded to Zod v4.
MCP SDK v1.24.0 now fully supports Zod v4 (see PR #1040), allowing us to unify Zod usage across the entire codebase.
Changes
Updated MCP SDK version:
@modelcontextprotocol/sdk:^1.22.0→^1.24.0Removed Zod v3 workarounds from MCP files:
@ts-nocheckcomments from all MCP tool and prompt filesimport { z } from 'zod/v3'toimport { z } from 'zod'.shape as anycasts and associatedbiome-ignorecommentsregisterTool()instead of.shapeRemoved MCP-specific tsconfig:
src/mcp/tsconfig.json(was only needed for Zod v3 type compatibility)Files Changed
package.json- MCP SDK version bumpsrc/mcp/tsconfig.json- Deletedsrc/mcp/tools/fileSystemReadDirectoryTool.tssrc/mcp/tools/fileSystemReadFileTool.tssrc/mcp/tools/packCodebaseTool.tssrc/mcp/tools/packRemoteRepositoryTool.tssrc/mcp/tools/readRepomixOutputTool.tssrc/mcp/tools/grepRepomixOutputTool.tssrc/mcp/tools/attachPackedOutputTool.tssrc/mcp/tools/mcpToolRuntime.tssrc/mcp/prompts/packRemoteRepositoryPrompts.tsResult
References
Checklist
npm run testnpm run lint