fix(smart_read): guard zod-v4 ZodError and require non-empty path#167
Open
nilhemdot wants to merge 1 commit into
Open
fix(smart_read): guard zod-v4 ZodError and require non-empty path#167nilhemdot wants to merge 1 commit into
nilhemdot wants to merge 1 commit into
Conversation
validateToolArgs read `error.errors`, which zod v4 removed in favor of `error.issues`, so any failed validation crashed with "Cannot read properties of undefined (reading 'map')". Read `error.issues ?? error.errors ?? []` so error formatting works on both zod v3 and v4. Also guard smart_read against a missing/blank `path` (e.g. the wrong key `file_path`) with a clear message instead of an opaque downstream error. Rewrite docs/TESTING_INSTRUCTIONS.md for the WSL2 native port: correct `path` argument, daemon/invoke-mcp.js invocation, WSL paths, the dedup-based Read interception model, and a regression test for the crash. Bump version to 5.0.2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary by CodeRabbitRelease Notes
WalkthroughThis PR delivers v5.0.2 with two bug fixes for argument validation and Zod error handling, updates npm build scripts to include dashboard asset copying, and extensively revises testing documentation to reflect the new WSL2/Linux daemon-based workflow with caching, diff, and truncation metadata. Changesv5.0.2 Release — Validation Fixes and Build Updates
Testing Documentation — v5.0.1 WSL2/Linux Workflow
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Commit Message Format IssueYour commit messages don't follow the Conventional Commits specification. Required Format:Valid Types:
Examples:Breaking Changes:Add Please amend your commit messages to follow this format. Learn more: Conventional Commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/TESTING_INSTRUCTIONS.md`:
- Line 134: Update the two references of the package version string "v5.0.1" to
"v5.0.2" in the document (the occurrences noted in the diff), ensuring both
instances are changed (including the line containing "**Fix**: Fixed in v5.0.1
(`error.issues ?? error.errors ?? []`). Rebuild" and the second occurrence
around line 142) so the docs match the package version.
- Line 1: Update the document title "Testing Instructions for smart_read Caching
(v5.0.1)" and the other in-text occurrence of "v5.0.1" to "v5.0.2" so the file
matches the PR objective "Bumped package version to 5.0.2"; locate the exact
title string and any standalone "v5.0.1" tokens in the document and replace them
with "v5.0.2".
In `@src/tools/file-operations/smart-read.ts`:
- Around line 107-111: The validation in smart_read currently only rejects
non-strings and empty strings but allows whitespace-only paths; update the check
around the filePath variable in src/tools/file-operations/smart-read.ts so that
after confirming typeof filePath === 'string' you also treat strings with only
whitespace as invalid (e.g., use filePath.trim().length === 0 or a /^\s*$/ test)
and throw the same error; ensure you don't call trim() before the typeof check
so you avoid calling string methods on non-strings and keep the error message
thrown by the existing smart_read validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6aae9847-ce7b-4d61-bc66-5556550150f7
📒 Files selected for processing (5)
CHANGELOG.mddocs/TESTING_INSTRUCTIONS.mdpackage.jsonsrc/tools/file-operations/smart-read.tssrc/validation/validator.ts
| @@ -1,161 +1,181 @@ | |||
| # Testing Instructions for smart_read Caching (v2.4.0) | |||
| # Testing Instructions for smart_read Caching (v5.0.1) | |||
There was a problem hiding this comment.
Version mismatch: document references v5.0.1 but package is v5.0.2.
The title and line 8 reference v5.0.1, but the PR objectives state "Bumped package version to 5.0.2." Update both instances to v5.0.2 for consistency.
📝 Proposed fix
-# Testing Instructions for smart_read Caching (v5.0.1)
+# Testing Instructions for smart_read Caching (v5.0.2)-1. ✅ Package built/installed at v5.0.1 (`smart_read` tool present).
+1. ✅ Package built/installed at v5.0.2 (`smart_read` tool present).Also applies to: 8-8
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/TESTING_INSTRUCTIONS.md` at line 1, Update the document title "Testing
Instructions for smart_read Caching (v5.0.1)" and the other in-text occurrence
of "v5.0.1" to "v5.0.2" so the file matches the PR objective "Bumped package
version to 5.0.2"; locate the exact title string and any standalone "v5.0.1"
tokens in the document and replace them with "v5.0.2".
| ### Issue: "Cannot read properties of undefined (reading 'map')" | ||
| **Cause**: Old build — `validator.ts` read `error.errors`, removed in zod v4 | ||
| (the global package bundles zod 4.x), so it was `undefined.map`. | ||
| **Fix**: Fixed in v5.0.1 (`error.issues ?? error.errors ?? []`). Rebuild |
There was a problem hiding this comment.
Update version references to v5.0.2.
Lines 134 and 142 reference v5.0.1 but should match the package version v5.0.2.
📝 Proposed fix
-**Fix**: Fixed in v5.0.1 (`error.issues ?? error.errors ?? []`). Rebuild
+**Fix**: Fixed in v5.0.2 (`error.issues ?? error.errors ?? []`). Rebuild-**Fix**: Use `path`. v5.0.1 also guards with a clear
+**Fix**: Use `path`. v5.0.2 also guards with a clearAlso applies to: 142-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/TESTING_INSTRUCTIONS.md` at line 134, Update the two references of the
package version string "v5.0.1" to "v5.0.2" in the document (the occurrences
noted in the diff), ensuring both instances are changed (including the line
containing "**Fix**: Fixed in v5.0.1 (`error.issues ?? error.errors ?? []`).
Rebuild" and the second occurrence around line 142) so the docs match the
package version.
Comment on lines
+107
to
+111
| if (typeof filePath !== 'string' || filePath.length === 0) { | ||
| throw new Error( | ||
| 'smart_read requires a non-empty "path" argument (received: ' + | ||
| `${JSON.stringify(filePath)})` | ||
| ); |
There was a problem hiding this comment.
Handle whitespace-only path as invalid input.
Line 107 currently accepts values like " ", so blank-ish input still falls through to a downstream file error instead of the explicit validation error.
Proposed fix
- if (typeof filePath !== 'string' || filePath.length === 0) {
+ if (typeof filePath !== 'string' || filePath.trim().length === 0) {
throw new Error(
'smart_read requires a non-empty "path" argument (received: ' +
`${JSON.stringify(filePath)})`
);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tools/file-operations/smart-read.ts` around lines 107 - 111, The
validation in smart_read currently only rejects non-strings and empty strings
but allows whitespace-only paths; update the check around the filePath variable
in src/tools/file-operations/smart-read.ts so that after confirming typeof
filePath === 'string' you also treat strings with only whitespace as invalid
(e.g., use filePath.trim().length === 0 or a /^\s*$/ test) and throw the same
error; ensure you don't call trim() before the typeof check so you avoid calling
string methods on non-strings and keep the error message thrown by the existing
smart_read validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
smart_readcrashed withCannot read properties of undefined (reading 'map')on any validation failure.Root cause:
validateToolArgsreadserror.errors, which zod v4 removed in favor oferror.issues. The published package bundles zod 4.x, soerror.errorsisundefined→undefined.map(...). Any bad argument (e.g. calling with the wrong keyfile_pathinstead ofpath) triggers it.Changes
src/validation/validator.ts— readerror.issues ?? error.errors ?? [], so error formatting works on both zod v3 and v4 and can never crash on a malformedZodError.src/tools/file-operations/smart-read.ts— guard a missing/blankpathwith a clearsmart_read requires a non-empty "path" argumentmessage instead of an opaque downstream error.docs/TESTING_INSTRUCTIONS.md— rewritten for the WSL2/Linux native port: correctpathargument, daemon/invoke-mcp.jsinvocation, WSL paths (dispatcher.log,~/.token-optimizer-cache/cache.db), the dedup-based Read interception model, and a regression test for the.mapcrash.Verification
End-to-end through the daemon socket:
{file_path: ...}(bad arg) →Validation failed for tool "smart_read": - path: Invalid input: expected string, received undefined— no.mapcrash.{path: ...}(correct) → works,fromCache: true.🤖 Generated with Claude Code