feat(search_files): clarify description and standardise implementation
Summary
- Clarified search_files tool description to explicitly state it searches by filename patterns, not file content
- Standardized implementation to use glob matching exclusively instead of mixing glob and substring matching
- Updated tests to reflect the new consistent glob-only behavior
Changes
- Modified pattern matching in
buildTreeto use only minimatch glob patterns - Updated search_files to accept array of patterns instead of single pattern for better flexibility
- Adjusted test cases to use proper glob patterns (e.g.,
**/node_modulesfor nested directories) - Improved tool descriptions in the API to be clearer about glob pattern usage
Fixes #896
Credit: Some of the prompt change is inspired by the suggestions from @sebastien-rosset and others in https://github.com/modelcontextprotocol/servers/issues/896
- @claude can you do this?
- Yeah, I worry about this being a breaking change a little. Although the previous format was so undocumented that I doubt too much has been built around it? I'm leaning to just pushing it out, but open to ideas on how we can better signal this to people.
Claude finished @domdomegg's task —— View job
Summary
Yes, I can help with the review feedback! I've updated the documentation and here's my guidance on the breaking change concerns.
**Completed Tasks & Recommendations**
✅ Completed Tasks
- [x] Updated filesystem README - Reflected the new
search_filesAPI withpatternsarray parameter - [x] Added Breaking Changes section - Clear documentation of the parameter and behavior changes
- [x] Provided migration guidance - Examples showing how to update from old to new API
- [x] Built and tested changes - All tests pass, TypeScript compiles successfully
- [x] Committed and pushed - Changes are now in the PR branch
📋 Key Changes Made
Documentation Updates (src/filesystem/README.md):
- Added "Breaking Changes" section documenting the
search_filesAPI change - Updated tool description:
pattern(string) →patterns(string[]) - Clarified "glob pattern matching exclusively" vs mixed glob/substring
- Added migration example:
{"pattern": "*.js"}→{"patterns": ["*.js"]}
💡 Breaking Change Recommendations
Regarding your concern about the breaking change:
-
👍 Proceed with the change - You're right that the previous behavior was undocumented, which minimizes real-world impact
-
📢 Consider these signaling strategies:
- ✅ Documentation (already done) - The Breaking Changes section provides clear migration path
- Release notes - Highlight this in the next release/changelog
- Version bump consideration - While calendar versioning limits options, consider if this warrants a minor version increment
- Deprecation notice - Could add a temporary backward compatibility layer that warns about the old API
-
🔄 Migration support options:
// Option 1: Backward compatibility (temporary) pattern?: string; // deprecated, warn users patterns: string[]; // Option 2: Auto-migration in server if (args.pattern && !args.patterns) { console.warn("search_files: 'pattern' parameter is deprecated, use 'patterns' array"); args.patterns = [args.pattern]; } -
📊 Impact assessment:
- Low impact likely due to undocumented previous behavior
- Better long-term API consistency outweighs short-term friction
- Clear documentation and examples help adoption
My recommendation: Proceed as planned. The benefits of API consistency and improved glob pattern matching outweigh the breaking change concerns, especially given the limited documentation of previous behavior.
The changes are now committed and ready. The filesystem README includes clear breaking change documentation with migration guidance for users.