Limit search depth in path corrector (#14869)

This commit is contained in:
Tommaso Sciortino
2025-12-09 20:08:39 -08:00
committed by GitHub
parent ee6556cbd2
commit 1954f45c19
7 changed files with 213 additions and 76 deletions

View File

@@ -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']);
});
});

View File

@@ -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);
}
}

View File

@@ -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<void>;
/**
* 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<void> {
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,
});
});
}
}

View File

@@ -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]);
});
});

View File

@@ -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<string[]> {
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<string>();
@@ -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<string>();
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<string>,
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);
}
}
}

View File

@@ -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;
});

View File

@@ -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 {