feat(search_files): clarify description and standardise implementation

Previously description was confusing/ambigious about whether this was doing text content matching or filename matching. This clarifies it's by filename.

Also the implementation for globs was confusing - it would sometimes use glob and sometimes substring match. This makes it all globs.

Fixes #896

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Adam Jones
2025-08-23 06:51:19 +00:00
parent fd886fac9c
commit c4450cea6c
4 changed files with 32 additions and 35 deletions

View File

@@ -19,16 +19,10 @@ async function buildTreeForTesting(currentPath: string, rootPath: string, exclud
for (const entry of entries) {
const relativePath = path.relative(rootPath, path.join(currentPath, entry.name));
const shouldExclude = excludePatterns.some(pattern => {
if (pattern.includes('*')) {
return minimatch(relativePath, pattern, {dot: true});
}
// For files: match exact name or as part of path
// For directories: match as directory path
return minimatch(relativePath, pattern, {dot: true}) ||
minimatch(relativePath, `**/${pattern}`, {dot: true}) ||
minimatch(relativePath, `**/${pattern}/**`, {dot: true});
});
// Use glob matching exclusively for consistency
const shouldExclude = excludePatterns.some(pattern =>
minimatch(relativePath, pattern, {dot: true})
);
if (shouldExclude)
continue;
@@ -74,7 +68,7 @@ describe('buildTree exclude patterns', () => {
});
it('should exclude files matching simple patterns', async () => {
// Test the current implementation - this will fail until the bug is fixed
// With strict glob matching, '.env' only matches exactly '.env'
const tree = await buildTreeForTesting(testDir, testDir, ['.env']);
const fileNames = tree.map(entry => entry.name);
@@ -85,6 +79,7 @@ describe('buildTree exclude patterns', () => {
});
it('should exclude directories matching simple patterns', async () => {
// With strict glob matching, 'node_modules' only matches top-level
const tree = await buildTreeForTesting(testDir, testDir, ['node_modules']);
const dirNames = tree.map(entry => entry.name);
@@ -93,8 +88,9 @@ describe('buildTree exclude patterns', () => {
expect(dirNames).toContain('.git');
});
it('should exclude nested directories with same pattern', async () => {
const tree = await buildTreeForTesting(testDir, testDir, ['node_modules']);
it('should exclude nested directories with glob pattern', async () => {
// Use '**/node_modules' to match at any level
const tree = await buildTreeForTesting(testDir, testDir, ['**/node_modules']);
// Find the nested directory
const nestedDir = tree.find(entry => entry.name === 'nested');
@@ -124,7 +120,8 @@ describe('buildTree exclude patterns', () => {
});
it('should work with multiple exclude patterns', async () => {
const tree = await buildTreeForTesting(testDir, testDir, ['node_modules', '.env', '.git']);
// Mix of exact matches and glob patterns
const tree = await buildTreeForTesting(testDir, testDir, ['**/node_modules', '.env', '.git']);
const entryNames = tree.map(entry => entry.name);
expect(entryNames).not.toContain('node_modules');

View File

@@ -316,7 +316,7 @@ describe('Lib Functions', () => {
const result = await searchFilesWithValidation(
testDir,
'*test*',
['*test*'],
allowedDirs,
{ excludePatterns: ['*.log', 'node_modules'] }
);
@@ -346,7 +346,7 @@ describe('Lib Functions', () => {
const result = await searchFilesWithValidation(
testDir,
'*test*',
['*test*'],
allowedDirs,
{}
);
@@ -370,7 +370,7 @@ describe('Lib Functions', () => {
const result = await searchFilesWithValidation(
testDir,
'*test*',
['*test*'],
allowedDirs,
{ excludePatterns: ['*.backup'] }
);

View File

@@ -132,7 +132,7 @@ const MoveFileArgsSchema = z.object({
const SearchFilesArgsSchema = z.object({
path: z.string(),
pattern: z.string(),
patterns: z.array(z.string()),
excludePatterns: z.array(z.string()).optional().default([])
});
@@ -261,6 +261,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
"Get a recursive tree view of files and directories as a JSON structure. " +
"Each entry includes 'name', 'type' (file/directory), and 'children' for directories. " +
"Files have no children array, while directories always have a children array (which may be empty). " +
"Supports excluding paths using glob patterns (e.g., ['node_modules', '**/*.log', '.git']). " +
"The output is formatted with 2-space indentation for readability. Only works within allowed directories.",
inputSchema: zodToJsonSchema(DirectoryTreeArgsSchema) as ToolInput,
},
@@ -276,9 +277,10 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
{
name: "search_files",
description:
"Recursively search for files and directories matching a pattern. " +
"The patterns should be glob-style patterns that match paths relative to the working directory. " +
"Use pattern like '*.ext' to match files in current directory, and '**/*.ext' to match files in all subdirectories. " +
"Recursively search for files and directories matching glob patterns. " +
"The patterns should be glob-style patterns that match paths relative to the search path. " +
"Use patterns like ['*.ext'] to match files in current directory, and ['**/*.ext'] to match files in all subdirectories. " +
"Multiple patterns can be provided to match different file types, e.g., ['**/*.js', '**/*.ts']. " +
"Returns full paths to all matching items. Great for finding files when you don't know their exact location. " +
"Only searches within allowed directories.",
inputSchema: zodToJsonSchema(SearchFilesArgsSchema) as ToolInput,
@@ -539,16 +541,10 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
for (const entry of entries) {
const relativePath = path.relative(rootPath, path.join(currentPath, entry.name));
const shouldExclude = excludePatterns.some(pattern => {
if (pattern.includes('*')) {
return minimatch(relativePath, pattern, {dot: true});
}
// For files: match exact name or as part of path
// For directories: match as directory path
return minimatch(relativePath, pattern, {dot: true}) ||
minimatch(relativePath, `**/${pattern}`, {dot: true}) ||
minimatch(relativePath, `**/${pattern}/**`, {dot: true});
});
// Use glob matching exclusively for consistency
const shouldExclude = excludePatterns.some(pattern =>
minimatch(relativePath, pattern, {dot: true})
);
if (shouldExclude)
continue;
@@ -596,7 +592,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
throw new Error(`Invalid arguments for search_files: ${parsed.error}`);
}
const validPath = await validatePath(parsed.data.path);
const results = await searchFilesWithValidation(validPath, parsed.data.pattern, allowedDirectories, { excludePatterns: parsed.data.excludePatterns });
const results = await searchFilesWithValidation(validPath, parsed.data.patterns, allowedDirectories, { excludePatterns: parsed.data.excludePatterns });
return {
content: [{ type: "text", text: results.length > 0 ? results.join("\n") : "No matches found" }],
};

View File

@@ -350,7 +350,7 @@ export async function headFile(filePath: string, numLines: number): Promise<stri
export async function searchFilesWithValidation(
rootPath: string,
pattern: string,
patterns: string[],
allowedDirectories: string[],
options: SearchOptions = {}
): Promise<string[]> {
@@ -373,8 +373,12 @@ export async function searchFilesWithValidation(
if (shouldExclude) continue;
// Use glob matching for the search pattern
if (minimatch(relativePath, pattern, { dot: true })) {
// Check if the path matches any of the patterns
const matches = patterns.some(pattern =>
minimatch(relativePath, pattern, { dot: true })
);
if (matches) {
results.push(fullPath);
}