diff --git a/packages/cli/src/zed-integration/fileSystemService.ts b/packages/cli/src/zed-integration/fileSystemService.ts index a56593f8c3..0604c34153 100644 --- a/packages/cli/src/zed-integration/fileSystemService.ts +++ b/packages/cli/src/zed-integration/fileSystemService.ts @@ -44,4 +44,8 @@ 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 946c227ab6..8209925956 100644 --- a/packages/core/src/services/fileSystemService.ts +++ b/packages/core/src/services/fileSystemService.ts @@ -4,7 +4,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -import fs from 'node:fs/promises'; +import * as 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 @@ -25,6 +27,15 @@ 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[]; } /** @@ -38,4 +49,17 @@ 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[] { + const foundFiles: string[] = []; + for (const searchPath of searchPaths) { + const pattern = path.join(searchPath, '**', fileName); + const matches = globSync(pattern, { + nodir: true, + absolute: true, + }); + foundFiles.push(...matches); + } + return foundFiles; + } } diff --git a/packages/core/src/tools/smart-edit.test.ts b/packages/core/src/tools/smart-edit.test.ts index f1bd9f14c6..bb1b37002a 100644 --- a/packages/core/src/tools/smart-edit.test.ts +++ b/packages/core/src/tools/smart-edit.test.ts @@ -242,6 +242,78 @@ describe('SmartEditTool', () => { }); }); + describe('correctPath', () => { + it('should correct a relative path if it is unambiguous', () => { + const testFile = 'unique.txt'; + fs.writeFileSync(path.join(rootDir, testFile), 'content'); + + const params: EditToolParams = { + file_path: testFile, + instruction: 'An instruction', + old_string: 'old', + new_string: 'new', + }; + + const validationResult = (tool as any).correctPath(params); + + expect(validationResult).toBeNull(); + expect(params.file_path).toBe(path.join(rootDir, testFile)); + }); + + it('should correct a partial relative path if it is unambiguous', () => { + const subDir = path.join(rootDir, 'sub'); + fs.mkdirSync(subDir); + const testFile = 'file.txt'; + const partialPath = path.join('sub', testFile); + const fullPath = path.join(subDir, testFile); + fs.writeFileSync(fullPath, 'content'); + + const params: EditToolParams = { + file_path: partialPath, + instruction: 'An instruction', + old_string: 'old', + new_string: 'new', + }; + + const validationResult = (tool as any).correctPath(params); + + expect(validationResult).toBeNull(); + expect(params.file_path).toBe(fullPath); + }); + + it('should return an error for a relative path that does not exist', () => { + const params: EditToolParams = { + file_path: 'test.txt', + instruction: 'An instruction', + old_string: 'old', + new_string: 'new', + }; + const result = (tool as any).correctPath(params); + expect(result).toMatch(/File not found for 'test.txt'/); + }); + + it('should return an error for an ambiguous path', () => { + const subDir1 = path.join(rootDir, 'module1'); + const subDir2 = path.join(rootDir, 'module2'); + fs.mkdirSync(subDir1, { recursive: true }); + fs.mkdirSync(subDir2, { recursive: true }); + + const ambiguousFile = 'component.ts'; + fs.writeFileSync(path.join(subDir1, ambiguousFile), 'content 1'); + fs.writeFileSync(path.join(subDir2, ambiguousFile), 'content 2'); + + const params: EditToolParams = { + file_path: ambiguousFile, + instruction: 'An instruction', + old_string: 'old', + new_string: 'new', + }; + + const validationResult = (tool as any).correctPath(params); + expect(validationResult).toMatch(/ambiguous and matches multiple files/); + }); + }); + describe('validateToolParams', () => { it('should return null for valid params', () => { const params: EditToolParams = { @@ -253,15 +325,15 @@ describe('SmartEditTool', () => { expect(tool.validateToolParams(params)).toBeNull(); }); - it('should return error for relative path', () => { + it('should return an error if path is outside the workspace', () => { const params: EditToolParams = { - file_path: 'test.txt', + file_path: path.join(os.tmpdir(), 'outside.txt'), instruction: 'An instruction', old_string: 'old', new_string: 'new', }; expect(tool.validateToolParams(params)).toMatch( - /File path must be absolute/, + /must be within one of the workspace directories/, ); }); }); diff --git a/packages/core/src/tools/smart-edit.ts b/packages/core/src/tools/smart-edit.ts index 3f62301ead..4b7a8d4f88 100644 --- a/packages/core/src/tools/smart-edit.ts +++ b/packages/core/src/tools/smart-edit.ts @@ -755,6 +755,58 @@ A good instruction should concisely answer: ); } + /** + * Quickly checks if the file path can be resolved directly against the workspace root. + * @param filePath The relative file path to check. + * @returns The absolute path if the file exists, otherwise null. + */ + private findDirectPath(filePath: string): string | null { + const directPath = path.join(this.config.getTargetDir(), filePath); + return fs.existsSync(directPath) ? directPath : null; + } + + /** + * Searches for a file across all configured workspace directories. + * @param filePath The file path (can be partial) to search for. + * @returns A list of absolute paths for all matching files found. + */ + private findAmbiguousPaths(filePath: string): string[] { + const workspaceContext = this.config.getWorkspaceContext(); + const fileSystem = this.config.getFileSystemService(); + const searchPaths = workspaceContext.getDirectories(); + return fileSystem.findFiles(filePath, searchPaths); + } + + /** + * Attempts to correct a relative file path to an absolute path. + * This function modifies `params.file_path` in place if successful. + * @param params The tool parameters containing the file_path to correct. + * @returns An error message string if correction fails, otherwise null. + */ + private correctPath(params: EditToolParams): string | null { + const directPath = this.findDirectPath(params.file_path); + if (directPath) { + params.file_path = directPath; + return null; + } + + const foundFiles = this.findAmbiguousPaths(params.file_path); + + if (foundFiles.length === 0) { + return `File not found for '${params.file_path}' and path is not absolute.`; + } + + if (foundFiles.length > 1) { + return ( + `The file path '${params.file_path}' is too ambiguous and matches multiple files. ` + + `Please provide a more specific path. Matches: ${foundFiles.join(', ')}` + ); + } + + params.file_path = foundFiles[0]; + return null; + } + /** * Validates the parameters for the Edit tool * @param params Parameters to validate @@ -768,7 +820,9 @@ A good instruction should concisely answer: } if (!path.isAbsolute(params.file_path)) { - return `File path must be absolute: ${params.file_path}`; + // Attempt to auto-correct to an absolute path + const error = this.correctPath(params); + if (error) return error; } const workspaceContext = this.config.getWorkspaceContext(); @@ -776,7 +830,6 @@ A good instruction should concisely answer: const directories = workspaceContext.getDirectories(); return `File path must be within one of the workspace directories: ${directories.join(', ')}`; } - return null; }