From 1954f45c193e3948e455cf2c4086f4cd8e43bf6a Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Tue, 9 Dec 2025 20:08:39 -0800 Subject: [PATCH] Limit search depth in path corrector (#14869) --- .../zed-integration/fileSystemService.test.ts | 16 --- .../src/zed-integration/fileSystemService.ts | 3 - .../core/src/services/fileSystemService.ts | 21 --- packages/core/src/utils/bfsFileSearch.test.ts | 94 ++++++++++++- packages/core/src/utils/bfsFileSearch.ts | 130 +++++++++++++----- packages/core/src/utils/pathCorrector.test.ts | 8 +- packages/core/src/utils/pathCorrector.ts | 17 ++- 7 files changed, 213 insertions(+), 76 deletions(-) diff --git a/packages/cli/src/zed-integration/fileSystemService.test.ts b/packages/cli/src/zed-integration/fileSystemService.test.ts index e274df0618..60462458d3 100644 --- a/packages/cli/src/zed-integration/fileSystemService.test.ts +++ b/packages/cli/src/zed-integration/fileSystemService.test.ts @@ -24,7 +24,6 @@ describe('AcpFileSystemService', () => { mockFallback = { readTextFile: vi.fn(), writeTextFile: vi.fn(), - findFiles: vi.fn(), }; }); @@ -113,19 +112,4 @@ describe('AcpFileSystemService', () => { verify(); }); }); - - it('should always use fallback for findFiles', () => { - service = new AcpFileSystemService( - mockClient, - 'session-1', - { readTextFile: true, writeTextFile: true }, - mockFallback, - ); - mockFallback.findFiles.mockReturnValue(['file1', 'file2']); - - const result = service.findFiles('pattern', ['/path']); - - expect(mockFallback.findFiles).toHaveBeenCalledWith('pattern', ['/path']); - expect(result).toEqual(['file1', 'file2']); - }); }); diff --git a/packages/cli/src/zed-integration/fileSystemService.ts b/packages/cli/src/zed-integration/fileSystemService.ts index 0d18fe3b81..a56593f8c3 100644 --- a/packages/cli/src/zed-integration/fileSystemService.ts +++ b/packages/cli/src/zed-integration/fileSystemService.ts @@ -44,7 +44,4 @@ export class AcpFileSystemService implements FileSystemService { sessionId: this.sessionId, }); } - findFiles(fileName: string, searchPaths: readonly string[]): string[] { - return this.fallback.findFiles(fileName, searchPaths); - } } diff --git a/packages/core/src/services/fileSystemService.ts b/packages/core/src/services/fileSystemService.ts index 67c9106113..946c227ab6 100644 --- a/packages/core/src/services/fileSystemService.ts +++ b/packages/core/src/services/fileSystemService.ts @@ -5,8 +5,6 @@ */ import fs from 'node:fs/promises'; -import * as path from 'node:path'; -import { globSync } from 'glob'; /** * Interface for file system operations that may be delegated to different implementations @@ -27,15 +25,6 @@ export interface FileSystemService { * @param content - The content to write */ writeTextFile(filePath: string, content: string): Promise; - - /** - * Finds files with a given name within specified search paths. - * - * @param fileName - The name of the file to find. - * @param searchPaths - An array of directory paths to search within. - * @returns An array of absolute paths to the found files. - */ - findFiles(fileName: string, searchPaths: readonly string[]): string[]; } /** @@ -49,14 +38,4 @@ export class StandardFileSystemService implements FileSystemService { async writeTextFile(filePath: string, content: string): Promise { await fs.writeFile(filePath, content, 'utf-8'); } - - findFiles(fileName: string, searchPaths: readonly string[]): string[] { - return searchPaths.flatMap((searchPath) => { - const pattern = path.posix.join(searchPath, '**', fileName); - return globSync(pattern, { - nodir: true, - absolute: true, - }); - }); - } } diff --git a/packages/core/src/utils/bfsFileSearch.test.ts b/packages/core/src/utils/bfsFileSearch.test.ts index 7fb2022b53..c194090fb6 100644 --- a/packages/core/src/utils/bfsFileSearch.test.ts +++ b/packages/core/src/utils/bfsFileSearch.test.ts @@ -8,7 +8,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as fsPromises from 'node:fs/promises'; import * as path from 'node:path'; import * as os from 'node:os'; -import { bfsFileSearch } from './bfsFileSearch.js'; +import { bfsFileSearch, bfsFileSearchSync } from './bfsFileSearch.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; describe('bfsFileSearch', () => { @@ -230,3 +230,95 @@ describe('bfsFileSearch', () => { expect(result.sort()).toEqual(expectedFiles.sort()); }); }); + +describe('bfsFileSearchSync', () => { + let testRootDir: string; + + async function createEmptyDir(...pathSegments: string[]) { + const fullPath = path.join(testRootDir, ...pathSegments); + await fsPromises.mkdir(fullPath, { recursive: true }); + return fullPath; + } + + async function createTestFile(content: string, ...pathSegments: string[]) { + const fullPath = path.join(testRootDir, ...pathSegments); + await fsPromises.mkdir(path.dirname(fullPath), { recursive: true }); + await fsPromises.writeFile(fullPath, content); + return fullPath; + } + + beforeEach(async () => { + testRootDir = await fsPromises.mkdtemp( + path.join(os.tmpdir(), 'bfs-file-search-sync-test-'), + ); + }); + + afterEach(async () => { + await fsPromises.rm(testRootDir, { recursive: true, force: true }); + }); + + it('should find a file in the root directory synchronously', async () => { + const targetFilePath = await createTestFile('content', 'target.txt'); + const result = bfsFileSearchSync(testRootDir, { fileName: 'target.txt' }); + expect(result).toEqual([targetFilePath]); + }); + + it('should find a file in a nested directory synchronously', async () => { + const targetFilePath = await createTestFile( + 'content', + 'a', + 'b', + 'target.txt', + ); + const result = bfsFileSearchSync(testRootDir, { fileName: 'target.txt' }); + expect(result).toEqual([targetFilePath]); + }); + + it('should respect the maxDirs limit and not find the file synchronously', async () => { + await createTestFile('content', 'a', 'b', 'c', 'target.txt'); + const result = bfsFileSearchSync(testRootDir, { + fileName: 'target.txt', + maxDirs: 3, + }); + expect(result).toEqual([]); + }); + + it('should ignore directories synchronously', async () => { + await createTestFile('content', 'ignored', 'target.txt'); + const targetFilePath = await createTestFile( + 'content', + 'not-ignored', + 'target.txt', + ); + const result = bfsFileSearchSync(testRootDir, { + fileName: 'target.txt', + ignoreDirs: ['ignored'], + }); + expect(result).toEqual([targetFilePath]); + }); + + it('should work with FileDiscoveryService synchronously', async () => { + const projectRoot = await createEmptyDir('project'); + await createEmptyDir('project', '.git'); + await createTestFile('node_modules/', 'project', '.gitignore'); + await createTestFile('content', 'project', 'node_modules', 'target.txt'); + const targetFilePath = await createTestFile( + 'content', + 'project', + 'not-ignored', + 'target.txt', + ); + + const fileService = new FileDiscoveryService(projectRoot); + const result = bfsFileSearchSync(projectRoot, { + fileName: 'target.txt', + fileService, + fileFilteringOptions: { + respectGitIgnore: true, + respectGeminiIgnore: true, + }, + }); + + expect(result).toEqual([targetFilePath]); + }); +}); diff --git a/packages/core/src/utils/bfsFileSearch.ts b/packages/core/src/utils/bfsFileSearch.ts index 36006335af..781e988d30 100644 --- a/packages/core/src/utils/bfsFileSearch.ts +++ b/packages/core/src/utils/bfsFileSearch.ts @@ -5,6 +5,7 @@ */ import * as fs from 'node:fs/promises'; +import * as fsSync from 'node:fs'; import * as path from 'node:path'; import type { FileDiscoveryService } from '../services/fileDiscoveryService.js'; import type { FileFilteringOptions } from '../config/constants.js'; @@ -37,13 +38,7 @@ export async function bfsFileSearch( rootDir: string, options: BfsFileSearchOptions, ): Promise { - const { - fileName, - ignoreDirs = [], - maxDirs = Infinity, - debug = false, - fileService, - } = options; + const { ignoreDirs = [], maxDirs = Infinity, debug = false } = options; const foundFiles: string[] = []; const queue: string[] = [rootDir]; const visited = new Set(); @@ -99,36 +94,109 @@ export async function bfsFileSearch( const results = await Promise.all(readPromises); for (const { currentDir, entries } of results) { - for (const entry of entries) { - const fullPath = path.join(currentDir, entry.name); - const isDirectory = entry.isDirectory(); - const isMatchingFile = entry.isFile() && entry.name === fileName; + processDirEntries( + currentDir, + entries, + options, + ignoreDirsSet, + queue, + foundFiles, + ); + } + } - if (!isDirectory && !isMatchingFile) { - continue; - } - if (isDirectory && ignoreDirsSet.has(entry.name)) { - continue; - } + return foundFiles; +} - if ( - fileService?.shouldIgnoreFile(fullPath, { - respectGitIgnore: options.fileFilteringOptions?.respectGitIgnore, - respectGeminiIgnore: - options.fileFilteringOptions?.respectGeminiIgnore, - }) - ) { - continue; - } +/** + * Performs a synchronous breadth-first search for a specific file within a directory structure. + * + * @param rootDir The directory to start the search from. + * @param options Configuration for the search. + * @returns An array of paths where the file was found. + */ +export function bfsFileSearchSync( + rootDir: string, + options: BfsFileSearchOptions, +): string[] { + const { ignoreDirs = [], maxDirs = Infinity, debug = false } = options; + const foundFiles: string[] = []; + const queue: string[] = [rootDir]; + const visited = new Set(); + let scannedDirCount = 0; + let queueHead = 0; - if (isDirectory) { - queue.push(fullPath); - } else { - foundFiles.push(fullPath); - } + const ignoreDirsSet = new Set(ignoreDirs); + + while (queueHead < queue.length && scannedDirCount < maxDirs) { + const currentDir = queue[queueHead]; + queueHead++; + + if (!visited.has(currentDir)) { + visited.add(currentDir); + scannedDirCount++; + + if (debug) { + logger.debug( + `Scanning Sync [${scannedDirCount}/${maxDirs}]: ${currentDir}`, + ); + } + + try { + const entries = fsSync.readdirSync(currentDir, { withFileTypes: true }); + processDirEntries( + currentDir, + entries, + options, + ignoreDirsSet, + queue, + foundFiles, + ); + } catch (error) { + const message = (error as Error)?.message ?? 'Unknown error'; + debugLogger.warn( + `[WARN] Skipping unreadable directory: ${currentDir} (${message})`, + ); } } } return foundFiles; } + +function processDirEntries( + currentDir: string, + entries: fsSync.Dirent[], + options: BfsFileSearchOptions, + ignoreDirsSet: Set, + queue: string[], + foundFiles: string[], +): void { + for (const entry of entries) { + const fullPath = path.join(currentDir, entry.name); + const isDirectory = entry.isDirectory(); + const isMatchingFile = entry.isFile() && entry.name === options.fileName; + + if (!isDirectory && !isMatchingFile) { + continue; + } + if (isDirectory && ignoreDirsSet.has(entry.name)) { + continue; + } + + if ( + options.fileService?.shouldIgnoreFile(fullPath, { + respectGitIgnore: options.fileFilteringOptions?.respectGitIgnore, + respectGeminiIgnore: options.fileFilteringOptions?.respectGeminiIgnore, + }) + ) { + continue; + } + + if (isDirectory) { + queue.push(fullPath); + } else { + foundFiles.push(fullPath); + } + } +} diff --git a/packages/core/src/utils/pathCorrector.test.ts b/packages/core/src/utils/pathCorrector.test.ts index 3516183695..a3ed038ddc 100644 --- a/packages/core/src/utils/pathCorrector.test.ts +++ b/packages/core/src/utils/pathCorrector.test.ts @@ -10,7 +10,7 @@ import * as path from 'node:path'; import * as os from 'node:os'; import type { Config } from '../config/config.js'; import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; -import { StandardFileSystemService } from '../services/fileSystemService.js'; +import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; import { correctPath } from './pathCorrector.js'; describe('pathCorrector', () => { @@ -30,7 +30,11 @@ describe('pathCorrector', () => { getTargetDir: () => rootDir, getWorkspaceContext: () => createMockWorkspaceContext(rootDir, [otherWorkspaceDir]), - getFileSystemService: () => new StandardFileSystemService(), + getFileService: () => new FileDiscoveryService(rootDir), + getFileFilteringOptions: () => ({ + respectGitIgnore: true, + respectGeminiIgnore: true, + }), } as unknown as Config; }); diff --git a/packages/core/src/utils/pathCorrector.ts b/packages/core/src/utils/pathCorrector.ts index 94576c43e5..d6538ff056 100644 --- a/packages/core/src/utils/pathCorrector.ts +++ b/packages/core/src/utils/pathCorrector.ts @@ -7,6 +7,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { Config } from '../config/config.js'; +import { bfsFileSearchSync } from './bfsFileSearch.js'; type SuccessfulPathCorrection = { success: true; @@ -41,9 +42,21 @@ export function correctPath( // If not found directly, search across all workspace directories for ambiguous matches. const workspaceContext = config.getWorkspaceContext(); - const fileSystem = config.getFileSystemService(); const searchPaths = workspaceContext.getDirectories(); - const foundFiles = fileSystem.findFiles(filePath, searchPaths); + const basename = path.basename(filePath); + const normalizedTarget = filePath.replace(/\\/g, '/'); + + // Normalize path for matching and check if it ends with the provided relative path + const foundFiles = searchPaths + .flatMap((searchPath) => + bfsFileSearchSync(searchPath, { + fileName: basename, + maxDirs: 50, // Capped to avoid deep hangs + fileService: config.getFileService(), + fileFilteringOptions: config.getFileFilteringOptions(), + }), + ) + .filter((f) => f.replace(/\\/g, '/').endsWith(normalizedTarget)); if (foundFiles.length === 0) { return {