mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-10 22:21:22 -07:00
feat(core): generalize path correction for use across tools (#10612)
This commit is contained in:
@@ -274,77 +274,6 @@ describe('SmartEditTool', () => {
|
||||
expect(result.occurrences).toBe(1);
|
||||
});
|
||||
});
|
||||
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', () => {
|
||||
|
||||
@@ -38,6 +38,7 @@ import { logSmartEditStrategy } from '../telemetry/loggers.js';
|
||||
import { SmartEditCorrectionEvent } from '../telemetry/types.js';
|
||||
import { logSmartEditCorrectionEvent } from '../telemetry/loggers.js';
|
||||
|
||||
import { correctPath } from '../utils/pathCorrector.js';
|
||||
interface ReplacementContext {
|
||||
params: EditToolParams;
|
||||
currentContent: string;
|
||||
@@ -876,58 +877,6 @@ 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
|
||||
@@ -940,11 +889,17 @@ A good instruction should concisely answer:
|
||||
return "The 'file_path' parameter must be non-empty.";
|
||||
}
|
||||
|
||||
if (!path.isAbsolute(params.file_path)) {
|
||||
let filePath = params.file_path;
|
||||
if (!path.isAbsolute(filePath)) {
|
||||
// Attempt to auto-correct to an absolute path
|
||||
const error = this.correctPath(params);
|
||||
if (error) return error;
|
||||
const result = correctPath(filePath, this.config);
|
||||
if (result.success) {
|
||||
filePath = result.correctedPath;
|
||||
} else {
|
||||
return result.error;
|
||||
}
|
||||
}
|
||||
params.file_path = filePath;
|
||||
|
||||
const workspaceContext = this.config.getWorkspaceContext();
|
||||
if (!workspaceContext.isPathWithinWorkspace(params.file_path)) {
|
||||
|
||||
101
packages/core/src/utils/pathCorrector.test.ts
Normal file
101
packages/core/src/utils/pathCorrector.test.ts
Normal file
@@ -0,0 +1,101 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import * as fs from 'node:fs';
|
||||
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 { correctPath } from './pathCorrector.js';
|
||||
|
||||
describe('pathCorrector', () => {
|
||||
let tempDir: string;
|
||||
let rootDir: string;
|
||||
let otherWorkspaceDir: string;
|
||||
let mockConfig: Config;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'path-corrector-test-'));
|
||||
rootDir = path.join(tempDir, 'root');
|
||||
otherWorkspaceDir = path.join(tempDir, 'other');
|
||||
fs.mkdirSync(rootDir, { recursive: true });
|
||||
fs.mkdirSync(otherWorkspaceDir, { recursive: true });
|
||||
|
||||
mockConfig = {
|
||||
getTargetDir: () => rootDir,
|
||||
getWorkspaceContext: () =>
|
||||
createMockWorkspaceContext(rootDir, [otherWorkspaceDir]),
|
||||
getFileSystemService: () => new StandardFileSystemService(),
|
||||
} as unknown as Config;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should correct a relative path if it is unambiguous in the target dir', () => {
|
||||
const testFile = 'unique.txt';
|
||||
fs.writeFileSync(path.join(rootDir, testFile), 'content');
|
||||
|
||||
const result = correctPath(testFile, mockConfig);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success) {
|
||||
expect(result.correctedPath).toBe(path.join(rootDir, testFile));
|
||||
}
|
||||
});
|
||||
|
||||
it('should correct a partial relative path if it is unambiguous in another workspace dir', () => {
|
||||
const subDir = path.join(otherWorkspaceDir, 'sub');
|
||||
fs.mkdirSync(subDir);
|
||||
const testFile = 'file.txt';
|
||||
const fullPath = path.join(subDir, testFile);
|
||||
fs.writeFileSync(fullPath, 'content');
|
||||
|
||||
const result = correctPath(testFile, mockConfig);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success) {
|
||||
expect(result.correctedPath).toBe(fullPath);
|
||||
}
|
||||
});
|
||||
|
||||
it('should return an error for a relative path that does not exist', () => {
|
||||
const result = correctPath('nonexistent.txt', mockConfig);
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
expect(result.error).toMatch(
|
||||
/File not found for 'nonexistent.txt' and path is not absolute./,
|
||||
);
|
||||
} else {
|
||||
expect.fail('Expected path correction to fail.');
|
||||
}
|
||||
});
|
||||
|
||||
it('should return an error for an ambiguous path', () => {
|
||||
const ambiguousFile = 'component.ts';
|
||||
const subDir1 = path.join(rootDir, 'module1');
|
||||
const subDir2 = path.join(otherWorkspaceDir, 'module2');
|
||||
fs.mkdirSync(subDir1, { recursive: true });
|
||||
fs.mkdirSync(subDir2, { recursive: true });
|
||||
fs.writeFileSync(path.join(subDir1, ambiguousFile), 'content 1');
|
||||
fs.writeFileSync(path.join(subDir2, ambiguousFile), 'content 2');
|
||||
|
||||
const result = correctPath(ambiguousFile, mockConfig);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
expect(result.error).toMatch(
|
||||
/The file path 'component.ts' is ambiguous and matches multiple files./,
|
||||
);
|
||||
} else {
|
||||
expect.fail('Expected path correction to fail.');
|
||||
}
|
||||
});
|
||||
});
|
||||
63
packages/core/src/utils/pathCorrector.ts
Normal file
63
packages/core/src/utils/pathCorrector.ts
Normal file
@@ -0,0 +1,63 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import type { Config } from '../config/config.js';
|
||||
|
||||
type SuccessfulPathCorrection = {
|
||||
success: true;
|
||||
correctedPath: string;
|
||||
};
|
||||
|
||||
type FailedPathCorrection = {
|
||||
success: false;
|
||||
error: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Attempts to correct a relative or ambiguous file path to a single, absolute path
|
||||
* within the workspace.
|
||||
*
|
||||
* @param filePath The file path to correct.
|
||||
* @param config The application configuration.
|
||||
* @returns A `PathCorrectionResult` object with either a `correctedPath` or an `error`.
|
||||
*/
|
||||
export type PathCorrectionResult =
|
||||
| SuccessfulPathCorrection
|
||||
| FailedPathCorrection;
|
||||
export function correctPath(
|
||||
filePath: string,
|
||||
config: Config,
|
||||
): PathCorrectionResult {
|
||||
// Check for direct path relative to the primary target directory.
|
||||
const directPath = path.join(config.getTargetDir(), filePath);
|
||||
if (fs.existsSync(directPath)) {
|
||||
return { success: true, correctedPath: directPath };
|
||||
}
|
||||
|
||||
// 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);
|
||||
|
||||
if (foundFiles.length === 0) {
|
||||
return {
|
||||
success: false,
|
||||
error: `File not found for '${filePath}' and path is not absolute.`,
|
||||
};
|
||||
}
|
||||
|
||||
if (foundFiles.length > 1) {
|
||||
return {
|
||||
success: false,
|
||||
error: `The file path '${filePath}' is ambiguous and matches multiple files. Please provide a more specific path. Matches: ${foundFiles.join(', ')}`,
|
||||
};
|
||||
}
|
||||
|
||||
return { success: true, correctedPath: foundFiles[0] };
|
||||
}
|
||||
Reference in New Issue
Block a user