Conversation
Zod v4への対応を完了しました。
主な変更:
- package.json: Zod v3.25.76 → v4.1.12にアップグレード
- ZodError.errors → ZodError.issuesに変更(Zod v4の破壊的変更)
- configSchemaでネストされたオブジェクトの外側の.default({})を削除
(Zod v4では外側のdefaultが内側のdefaultを上書きするため)
- configLoadでtokenCountとoutput.gitのマージ処理を追加
- テストの期待値を修正(空オブジェクトのパースが失敗するように)
MCP SDK互換性の対応:
- MCP SDKはZod v3を使用しているため、src/mcp配下でzod/v3をインポート
- src/mcp/tsconfig.jsonを作成し、型チェックを緩和(noImplicitAny: false)
- TypeScriptの制限(importされたファイルはexcludeできない)により、
MCPツール・プロンプトファイルに@ts-nocheckを追加
- repomixOutputStyleSchemaの代わりにz.enum()を直接使用
- biome-ignoreコメントを追加してas any使用箇所のlintエラーを抑制
すべてのテスト(800テスト)とlintチェックが成功しています。
|
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. WalkthroughUpgraded Zod from major version 3 to version 4, with MCP tools maintaining v3 compatibility through a compatibility layer. Restructured configuration schema defaults and fixed error handling to use Zod v4's Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main Code (Zod v4)
participant Config as Config Schema
participant MCP as MCP Tools (Zod v3 compat)
participant Runtime as Runtime Execution
Main->>Config: Parse configuration with Zod v4
Config->>Config: Merge input, output.git, tokenCount
Config-->>Main: Return merged config
Main->>Runtime: Initialize application
Runtime->>MCP: Register MCP tools
MCP->>MCP: Import from zod/v3 layer
MCP->>MCP: Cast schemas to any
MCP-->>Runtime: Tool registration complete
Runtime->>Runtime: Validate inputs using Zod v4
Note over Runtime: error.issues (v4 format)
Runtime-->>Main: Validation result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 updates the Zod library to its latest major version (v4.1.12), incorporating necessary code adjustments for breaking changes while ensuring continued compatibility with the Model Context Protocol (MCP) SDK by maintaining a dual-version strategy for Zod. The update improves schema validation and error handling across the main codebase, with a clear path for future unification once the MCP SDK supports Zod v4. 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
|
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades Zod from v3.25.76 to v4.1.12 and addresses breaking changes in Zod v4 while maintaining backward compatibility for MCP SDK which requires Zod v3.
Key Changes
- Updated Zod API usage: changed
ZodError.errorstoZodError.issuesthroughout error handling - Modified config schema to remove outer
.default({})wrappers which now override inner field defaults in Zod v4 - Implemented dual Zod version strategy using
zod/v3subpath imports for MCP files
Reviewed Changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated Zod dependency from ^3.25.76 to ^4.1.12 |
| src/shared/errorHandle.ts | Changed ZodError.errors to ZodError.issues for Zod v4 compatibility |
| src/config/configSchema.ts | Removed outer .default({}) from nested objects to preserve inner field defaults |
| src/config/configLoad.ts | Added explicit merging logic for git and tokenCount nested objects |
| tests/config/configSchema.test.ts | Updated test to expect empty config to throw validation error |
| src/mcp/tsconfig.json | Created separate TypeScript config for MCP files with relaxed compiler options |
| src/mcp/tools/*.ts | Added @ts-nocheck, switched to zod/v3 imports, and added type assertions for MCP SDK compatibility |
| src/mcp/prompts/packRemoteRepositoryPrompts.ts | Added @ts-nocheck and switched to zod/v3 import |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #923 +/- ##
==========================================
+ Coverage 74.60% 75.32% +0.71%
==========================================
Files 111 111
Lines 7782 7793 +11
Branches 1450 1454 +4
==========================================
+ Hits 5806 5870 +64
+ Misses 1976 1923 -53 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This is an excellent and thorough pull request for upgrading Zod to v4. The detailed summary of breaking changes and the dual-version strategy for maintaining compatibility with the MCP SDK is well-documented and clearly explained. The code changes are consistent with the plan, addressing schema updates, error handling, and configuration merging correctly. The addition of a separate tsconfig.json for the MCP directory is a smart solution for improving the developer experience during this transition period. I have one high-severity suggestion regarding a potential side-effect in the configuration merging logic, which could lead to hard-to-debug issues. Other than that, this is a high-quality contribution.
Pull Request Review: Zod v4 UpgradeSummaryThis PR successfully upgrades Zod from v3.25.76 to v4.1.12 while maintaining MCP SDK compatibility through a dual-version strategy. The approach is well-documented and the changes are thorough. All tests pass (800+) and the implementation addresses all Zod v4 breaking changes. ✅ Strengths
|
🔍 Code Quality Review - MCP SDK Compatibility StrategyFiles: src/mcp/tools/*.ts, src/mcp/tsconfig.json Current Approach
|
🔍 Code Quality Review - TypeScript ConfigurationFile: src/mcp/tsconfig.json
|
🧪 Test Coverage & SecurityCurrent Coverage: ✅ Excellent
🔒 Security Review: ✅ No concerns
📋 Recommended Additional Tests
🎯 Performance: ✅ No concerns
|
🎭 Premortem Analysis: Potential Failure ScenariosScenario 1: Config Loading Fails with Partial ConfigsRisk: Medium Mitigation:
Scenario 2: MCP SDK Updates Breaking RuntimeRisk: Low (but high impact) Mitigation:
Scenario 3: Type Errors Slip Through Due to @ts-nocheckRisk: Medium - HIGHEST PRIORITY CONCERN Failure Mode:
Mitigation:
|
🎯 Recommendations SummaryHigh Priority 🔴
Medium Priority 🟡
Low Priority 🟢
|
✅ Final Verdict: APPROVE with suggestions ✨This is a solid PR that handles a complex upgrade situation pragmatically. The dual Zod version strategy is a reasonable temporary solution given the MCP SDK constraint. The code changes correctly address all Zod v4 breaking changes, and the extensive test suite provides confidence. Main Concerns (addressable with follow-up PRs):
However, these do not block this upgrade. The current implementation is functional and well-documented. What I really liked:
Great work on the thorough documentation and testing! 🎉 The upgrade path is well-thought-out, and once MCP SDK adds Zod v4 support, the cleanup will be straightforward. This unblocks the project to use Zod v4 features going forward. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
src/mcp/tools/fileSystemReadDirectoryTool.ts (2)
1-1: Zod v3 bridge: keep @ts-nocheck temporary and track removalThe zod/v3 import and file-wide @ts-nocheck look fine for MCP SDK 1.20.x. Please add a TODO linking the SDK issue and plan to remove both once MCP supports Zod 4. Prefer narrower suppressions if feasible.
Also applies to: 6-6
32-35: AddtoMcpSchemahelper to centralize Zod v3 schema conversion.The repeated
.shape as anypattern occurs 14 times across the 7 modified tool files. Add this helper tosrc/mcp/tools/mcpToolRuntime.ts:export const toMcpSchema = <T extends { shape: unknown }>(schema: T) => schema.shape as any;Then replace all occurrences in:
attachPackedOutputTool.ts(lines 260, 262)fileSystemReadDirectoryTool.ts(lines 33, 35)fileSystemReadFileTool.ts(lines 34, 36)grepRepomixOutputTool.ts(lines 89, 91)packCodebaseTool.ts(lines 69, 71)packRemoteRepositoryTool.ts(lines 73, 75)readRepomixOutputTool.ts(lines 45, 47)This removes 14 redundant
biome-ignorecomments, centralizes the Zod v3 compatibility concern, and improves maintainability. Since all affected files already import frommcpToolRuntime.ts, no new dependencies are introduced.src/mcp/tools/fileSystemReadFileTool.ts (2)
1-1: Zod v3 compatibility acknowledged; add cleanup TODOSame note as other MCP tools: keep a TODO to drop @ts-nocheck and zod/v3 when MCP SDK upgrades.
Also applies to: 6-6
33-36: Consolidate MCP schema casts; minor perf nit on double stat
- Use a shared toMcpSchema helper instead of
.shape as anyto reduce duplication (see sibling comment).- You call
fs.stat(filePath)twice (Lines 65 and 73). Cache the first result to getsizeand avoid the second call.- const stats = await fs.stat(filePath); + const stats = await fs.stat(filePath); if (stats.isDirectory()) { /* ... */ } - const fileStats = await fs.stat(filePath); + const fileStats = statssrc/mcp/tools/attachPackedOutputTool.ts (1)
259-262: Replace.shape as anywith a helperApply the same
toMcpSchemahelper here for input/output schema wiring to de-duplicate casts and lint suppressions.src/config/configSchema.ts (1)
120-125: Validate tokenCount.encoding against known encodingsCasting to
TiktokenEncodinghides invalid values. Prefer a guarded schema:const encodings = ['o200k_base','gpt2','cl100k_base','p50k_base','p50k_edit'] as const; const encodingSchema = z.enum(encodings); ... tokenCount: z.object({ encoding: encodingSchema.default('o200k_base'), })Or use
z.string().refine(isValidEncoding, 'Unsupported encoding').src/mcp/tools/packCodebaseTool.ts (3)
1-1: Zod v3 + @ts-nocheck accepted for MCP areaSame guidance: keep a TODO to remove once MCP supports Zod 4; prefer scoped suppressions when possible.
Also applies to: 5-5
43-45: Deduplicate style enum across toolsThe style enum is duplicated here and in packRemoteRepositoryTool. Export a shared const (no Zod dependency) to avoid zod v3/v4 mixing:
// src/config/outputStyles.ts export const repomixOutputStyles = ['xml','markdown','json','plain'] as const; export type RepomixOutputStyle = typeof repomixOutputStyles[number];Then:
- style: z.enum(['xml','markdown','json','plain']).default('xml') + import { repomixOutputStyles } from '../../config/outputStyles.js'; + style: z.enum(repomixOutputStyles).default('xml')
69-71: Use helper for MCP schema castsReplace
.shape as anywithtoMcpSchema(...)as suggested elsewhere.src/mcp/tools/readRepomixOutputTool.ts (2)
1-1: MCP Zod v3 bridge noted; add cleanup TODOConsistent with other tools; add TODO to remove @ts-nocheck/zod/v3 when MCP SDK upgrades.
Also applies to: 5-5
45-47: Schema cast helper; minor UX tweak for returned line numbers
- Swap
.shape as anyfor atoMcpSchemahelper.- When clamping
endLineto file length, the response currently returns the originalendLine(possibly > totalLines). Prefer returning the actual applied range:- startLine: startLine || actualStartLine, - endLine: endLine || actualEndLine, + startLine: actualStartLine, + endLine: actualEndLine,This reflects what was read.
src/mcp/tools/packRemoteRepositoryTool.ts (3)
1-1: OK to use zod/v3 here; keep removal path trackedSame note: add TODO for deprecation of @ts-nocheck and zod/v3 when MCP updates.
Also applies to: 5-5
47-49: Share style enum with packCodebaseToolUse a shared
repomixOutputStylesconst to avoid duplication and cross-version zod coupling (see earlier comment).
73-75: Apply MCP schema cast helperReplace
.shape as anywithtoMcpSchema(...)for consistency.
📜 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 (16)
package.json(1 hunks)src/config/configLoad.ts(2 hunks)src/config/configSchema.ts(2 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(1 hunks)src/mcp/tools/packCodebaseTool.ts(4 hunks)src/mcp/tools/packRemoteRepositoryTool.ts(4 hunks)src/mcp/tools/readRepomixOutputTool.ts(2 hunks)src/mcp/tsconfig.json(1 hunks)src/shared/errorHandle.ts(1 hunks)tests/config/configSchema.test.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/config/configSchema.test.ts (1)
src/config/configSchema.ts (1)
repomixConfigDefaultSchema(76-126)
src/mcp/tools/packCodebaseTool.ts (1)
src/config/configSchema.ts (1)
defaultFilePathMap(9-14)
src/mcp/tools/packRemoteRepositoryTool.ts (1)
src/config/configSchema.ts (1)
defaultFilePathMap(9-14)
⏰ 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). (7)
- GitHub Check: Lint GitHub Actions
- GitHub Check: Test with Bun (windows-latest, latest)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (ubuntu-latest, 20.x)
- GitHub Check: Test (windows-latest, 24.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: claude-review
🔇 Additional comments (16)
src/mcp/tools/mcpToolRuntime.ts (1)
1-1: Temporary compatibility layer noted.The
@ts-nocheckdirective is part of the temporary Zod v3 compatibility strategy for MCP SDK. While this file doesn't directly import Zod, the directive may be needed for type consistency when imported by other MCP files.As noted in the PR objectives, remember to remove this directive when the MCP SDK upgrades to Zod v4.
tests/config/configSchema.test.ts (1)
142-145: LGTM! Test correctly validates stricter schema.The test now correctly expects
repomixConfigDefaultSchemato reject an empty configuration object. This aligns with the schema changes that removed outer.default({})wrappers, requiring explicit field definitions instead.src/mcp/prompts/packRemoteRepositoryPrompts.ts (1)
1-3: LGTM! Correct implementation of Zod v3 compatibility.The file correctly switches to
zod/v3import and adds@ts-nocheckto maintain compatibility with MCP SDK's Zod v3 requirement. This is part of the temporary compatibility layer until the MCP SDK upgrades.tsconfig.json (1)
22-22: LGTM! Correctly excludes MCP directory.Excluding
src/mcp/**from the main TypeScript configuration is the correct approach, as the MCP directory has its owntsconfig.jsonwith permissive settings for Zod v3 compatibility.src/mcp/tsconfig.json (1)
1-12: Acceptable as temporary technical debt for MCP SDK compatibility.This permissive TypeScript configuration (with
strict: falseandnoImplicitAny: false) is necessary for the Zod v3 compatibility layer. However, it reduces type safety for the MCP directory.As noted in the PR objectives, ensure this configuration is removed when the MCP SDK upgrades to Zod v4. Consider tracking this technical debt:
- Remove this file when MCP SDK supports Zod v4
- Remove
@ts-nocheckdirectives from MCP files- Remove
zod/v3imports and switch back tozod- Re-enable strict type checking for the MCP directory
You may want to create a tracking issue for this cleanup work.
src/shared/errorHandle.ts (1)
111-118: LGTM! Correctly implements Zod v4 breaking change.The change from
error.errorstoerror.issuesis required for Zod v4 compatibility. According to the Zod v4 documentation, the error property was renamed fromerrorstoissueswith new standardized issue formats.Based on library documentation.
src/mcp/tools/grepRepomixOutputTool.ts (3)
1-1: LGTM! Part of Zod v3 compatibility layer.The
@ts-nocheckdirective is correctly applied for Zod v3 compatibility with the MCP SDK.
5-5: LGTM! Correct use of Zod v3 import.The import path
zod/v3correctly maintains compatibility with the MCP SDK's Zod v3 requirement while Zod v4 is installed.
88-91: LGTM! Necessary type coercion for MCP SDK compatibility.Casting the schema shapes to
anyis required for compatibility between Zod v3 (used by MCP SDK) and the tool registration API. The biome-ignore comments clearly document this intentional type bypass.The use of
satisfieson lines 167 and 176 maintains internal type safety for the response objects.package.json (1)
105-105: Zod version 4.1.12 is current and secure.Verification confirms that 4.1.12 is the latest available version on npm. The only known security advisory for Zod affects versions ≤ 3.22.2 and is not applicable to 4.1.12. The major version upgrade is current and introduces no exposed vulnerabilities.
src/mcp/tools/attachPackedOutputTool.ts (1)
1-1: Consistent with MCP Zod v3 bridge; document removal pathLooks good. Please add a TODO pointing to the MCP SDK issue to remove @ts-nocheck and zod/v3 later.
Also applies to: 6-6
src/config/configSchema.ts (2)
157-165: defaultConfig via parse is solidParsing a shaped object to realize nested defaults is correct with Zod 4 and keeps types consistent. LGTM.
77-110: Verify intentional alignment difference between config and MCP tool schemasThe review comment's observation is verified as accurate. Configuration schema (
src/config/configSchema.ts:96) definestopFilesLengthwith.min(0).default(5), while all three MCP tools define it with.min(1).default(10):
src/mcp/tools/packCodebaseTool.ts,packRemoteRepositoryTool.ts,attachPackedOutputTool.tsall declare.min(1).optional().default(10)The difference in min constraints is meaningful: the main config treats
0as "disable summary" (checked insrc/cli/cliReport.ts:16), while MCP tools enforce a minimum of1. The default discrepancy (5 vs 10) is also real but the design intent is unclear—whether this reflects intentional context-specific defaults or an oversight. Consider documenting the rationale or aligning both to a consistent default for user predictability.src/config/configLoad.ts (1)
201-205: TokenCount merge looks correctMerges base, file, and CLI in the expected precedence. LGTM.
src/mcp/tools/packCodebaseTool.ts (1)
91-91: Indexing defaultFilePathMap is safe with enum; good
styleis constrained by enum; casting to the map’s key type is acceptable here. LGTM.src/mcp/tools/packRemoteRepositoryTool.ts (1)
88-88: defaultFilePathMap index is fineEnum-constrained
styleplus a type cast is acceptable; LGTM.
Addresses AI reviewer feedback on PR #923: **Critical fixes:** - Use structuredClone(defaultConfig) to prevent mutation of global defaults - Move filePath assignment to merge-time instead of mutating baseConfig - This prevents side effects in long-running processes and test suites **Documentation improvements:** - Add comment explaining Zod v4 nested object initialization pattern - Expand @ts-nocheck comments in MCP files explaining type incompatibility - Clarify that the issue extends beyond .shape to handler function signatures **Test coverage:** - Add test for nested git config merging (Zod v4 specific behavior) - Add test to verify defaultConfig immutability - Add test for tokenCount config merging All 803 tests passing, all lint checks passing. Fixes identified by @gemini-code-assist, @coderabbitai, and @claude.
Deploying repomix with
|
| Latest commit: |
da90167
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9f64aa35.repomix.pages.dev |
| Branch Preview URL: | https://chore-zod4.repomix.pages.dev |
Code Review: Zod v4 UpgradeThis is an excellent and well-executed upgrade to Zod v4! The PR demonstrates thorough understanding of the breaking changes and implements a pragmatic dual-version strategy for MCP SDK compatibility. ✅ Strengths1. Excellent Breaking Change Handling The PR correctly addresses all Zod v4 breaking changes:
2. Critical Fix: Prevented defaultConfig Mutation The use of structuredClone(defaultConfig) (src/config/configLoad.ts:158) is essential and prevents side effects in long-running processes (MCP server) and test suites. The corresponding test validates this behavior. 3. Pragmatic MCP SDK Compatibility Strategy The dual Zod version approach is well-justified:
4. Comprehensive Test Coverage All 803 tests passing with new tests for:
|
Replaced structuredClone with a more efficient immutable merge pattern for config object merging. The new implementation: - Removes unnecessary deep cloning of the entire defaultConfig - Uses spread operators to create new objects at each level - Wraps the output merge in an IIFE for clearer conditional logic - Sets filePath conditionally on the merged output object This improves performance by avoiding deep cloning overhead while maintaining immutability. The mergedOutput is mutated locally within the IIFE scope, which is safe as it's a newly created object. All 803 tests pass and lint checks are clean.
Added a test to verify that Zod v4 provides helpful error messages when required fields are missing from the config schema. The test ensures that the error message includes "expected object" to help users understand what went wrong. This addresses the suggestion from Claude bot to improve test coverage for error message quality and helps catch regressions in error messaging.
PR Review: Zod v4 UpgradeSummaryThis is a well-executed upgrade to Zod v4 that carefully handles breaking changes while maintaining backward compatibility with the MCP SDK. The implementation demonstrates excellent attention to detail with comprehensive testing and documentation. ✅ Strengths
🔍 Key ObservationsMCP TypeScript ConfigurationLocation: src/mcp/tsconfig.json The tsconfig with strict: false is pragmatic but relaxes type safety. Consider if @ts-nocheck comments alone would suffice once stable. Impact: Low - Current approach is well-documented as temporary. Schema Type Casting PatternLocation: src/mcp/tools/ The "as any" casts for MCP SDK compatibility create a type safety gap. When migrating to Zod v4, add type tests to verify schema compatibility. Impact: Low - Well-documented technical debt with clear resolution path. Config Initialization PatternLocation: src/config/configSchema.ts:157-167 The explicit nested object initialization is clear but requires manual maintenance. Consider adding tests to verify defaultConfig contains all expected fields. Impact: Low - Nice-to-have for future maintainability. 🎯 Premortem AnalysisPotential Failure ScenariosHigh PriorityType Mismatches in MCP Tool Handlers
Missing Nested Config Defaults
Medium PriorityMCP SDK Update Side Effects
Config Merge Order
Low PriorityMemory in Long-Running Processes
🔐 Security & Performance✅ Security: No concerns - changes are internal schema updates 🎨 Code QualityStrengths:
✨ RecommendationsBefore Merge
Post-Merge
Future WorkWhen MCP SDK supports Zod v4:
🎉 Final VerdictLGTM with confidence! 🚀 This is a high-quality upgrade that:
Risk Level: Low Great work! The iterative improvements based on AI reviewer feedback show excellent attention to detail. 👏 |
…ration Replaced zod-to-json-schema with Zod v4's native toJSONSchema() method for generating JSON Schema from Zod schemas. This eliminates the need for an external dependency and leverages Zod v4's new built-in feature. Changes: - Updated generateSchema.ts to use z.toJSONSchema() with draft-7 target - Removed direct dependency on zod-to-json-schema (still exists via MCP SDK) - Simplified the schema generation process The generated schema remains compatible with the existing format and all tooling that depends on it.
Code Review: Zod v4 UpgradeSummaryThis is an excellent and well-executed upgrade that handles the Zod v3 to v4 migration thoughtfully. The PR demonstrates strong engineering practices with comprehensive testing, clear documentation, and a pragmatic approach to the MCP SDK compatibility challenge. Strengths
Code Quality ObservationsPositive Patterns
Potential Issues
Security AnalysisNo security concerns. No changes to security validation logic, and Zod v4 includes security patches. PerformanceImprovements: Native JSON Schema generation is faster, immutable merge pattern avoids overhead. Premortem Analysis: Potential Failure ScenariosScenario 1: MCP SDK Releases Zod v4 Support Scenario 2: Nested Object Schema Growth Scenario 3: Type Errors Hidden by @ts-nocheck Scenario 4: Zod v4 Breaking Changes Scenario 5: Complex Nested Config Overrides RecommendationsCritical (Before Merge): None - the PR is ready to merge Important (Post-Merge):
Nice to Have: Test CoverageExcellent - 803 tests passing with specific tests for Zod v4 behavior changes, immutability, and error messages. Final VerdictAPPROVE This PR demonstrates exceptional engineering quality with thorough analysis, pragmatic workarounds, comprehensive testing, clear documentation, and performance improvements. The identified issues are minor and mostly relate to future maintenance. The code is production-ready. Great work on this migration! |
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
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)
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)
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)
Summary
This PR upgrades Zod from v3.25.76 to v4.1.12 and addresses all breaking changes introduced by Zod v4.
Main Changes
Fixed Zod v4 breaking changes in main codebase:
ZodError.errorstoZodError.issuesin error handling.default({})from nested object schemas to preserve inner field defaultstokenCountandoutput.gitin config loadingMaintained MCP SDK compatibility (Zod v3):
zod@^3.23.8import { z } from 'zod/v3'for runtime compatibility@ts-nocheckto MCP files due to type-level incompatibilitysrc/mcp/tsconfig.jsonfor improved IDE experienceDual Zod version strategy:
src/): Zod v4src/mcp/): Zod v3 (viazod/v3subpath)Technical Details
Zod v4 Breaking Changes Addressed:
ZodError.errors→ZodError.issues(src/shared/errorHandle.ts)Nested
.default({})behavior change (src/config/configSchema.ts).default({})overrides inner field defaultsinput,output,output.git,ignore,security,tokenCountencoding: z.string().default('o200k_base')) are preservedConfig merging logic (src/config/configLoad.ts)
tokenCountandoutput.gitobjects.default({})MCP SDK Compatibility:
excludedoesn't work for files imported by other TypeScript filessrc/mcp/mcpAction.tsimports MCP tool files, preventing exclusion from type checking@ts-nocheckis necessary due to type-level incompatibility between Zod v3 and v4@ts-expect-errorwould require extensive handler function refactoringTesting
Future Work
When MCP SDK supports Zod v4:
zod/v3imports fromsrc/mcp/files@ts-nocheckcommentsimport { z } from 'zod'throughout the codebaseTrack MCP SDK Zod v4 support: https://github.com/modelcontextprotocol/typescript-sdk/issues
Checklist
npm run testnpm run lint