diff options
Diffstat (limited to 'packages/core/src/tools')
| -rw-r--r-- | packages/core/src/tools/edit.test.ts | 2 | ||||
| -rw-r--r-- | packages/core/src/tools/edit.ts | 16 | ||||
| -rw-r--r-- | packages/core/src/tools/read-file.test.ts | 2 | ||||
| -rw-r--r-- | packages/core/src/tools/read-file.ts | 1 | ||||
| -rw-r--r-- | packages/core/src/tools/read-many-files.test.ts | 3 | ||||
| -rw-r--r-- | packages/core/src/tools/read-many-files.ts | 1 | ||||
| -rw-r--r-- | packages/core/src/tools/write-file.test.ts | 71 | ||||
| -rw-r--r-- | packages/core/src/tools/write-file.ts | 8 |
8 files changed, 58 insertions, 46 deletions
diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index b2e31fdd..539ae3ef 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -36,6 +36,7 @@ import os from 'os'; import { ApprovalMode, Config } from '../config/config.js'; import { Content, Part, SchemaUnion } from '@google/genai'; import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; +import { StandardFileSystemService } from '../services/fileSystemService.js'; describe('EditTool', () => { let tool: EditTool; @@ -60,6 +61,7 @@ describe('EditTool', () => { getApprovalMode: vi.fn(), setApprovalMode: vi.fn(), getWorkspaceContext: () => createMockWorkspaceContext(rootDir), + getFileSystemService: () => new StandardFileSystemService(), getIdeClient: () => undefined, getIdeMode: () => false, // getGeminiConfig: () => ({ apiKey: 'test-api-key' }), // This was not a real Config method diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index 8d90dfe4..35e828b0 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -125,7 +125,9 @@ class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> { | undefined = undefined; try { - currentContent = fs.readFileSync(params.file_path, 'utf8'); + currentContent = await this.config + .getFileSystemService() + .readTextFile(params.file_path); // Normalize line endings to LF for consistent processing. currentContent = currentContent.replace(/\r\n/g, '\n'); fileExists = true; @@ -339,7 +341,9 @@ class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> { try { this.ensureParentDirectoriesExist(this.params.file_path); - fs.writeFileSync(this.params.file_path, editData.newContent, 'utf8'); + await this.config + .getFileSystemService() + .writeTextFile(this.params.file_path, editData.newContent); let displayResult: ToolResultDisplay; if (editData.isNewFile) { @@ -504,7 +508,9 @@ Expectation for required parameters: getFilePath: (params: EditToolParams) => params.file_path, getCurrentContent: async (params: EditToolParams): Promise<string> => { try { - return fs.readFileSync(params.file_path, 'utf8'); + return this.config + .getFileSystemService() + .readTextFile(params.file_path); } catch (err) { if (!isNodeError(err) || err.code !== 'ENOENT') throw err; return ''; @@ -512,7 +518,9 @@ Expectation for required parameters: }, getProposedContent: async (params: EditToolParams): Promise<string> => { try { - const currentContent = fs.readFileSync(params.file_path, 'utf8'); + const currentContent = await this.config + .getFileSystemService() + .readTextFile(params.file_path); return applyReplacement( currentContent, params.old_string, diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index 101b74a5..b7b323cf 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -13,6 +13,7 @@ import fs from 'fs'; import fsp from 'fs/promises'; import { Config } from '../config/config.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; +import { StandardFileSystemService } from '../services/fileSystemService.js'; import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; import { ToolInvocation, ToolResult } from './tools.js'; @@ -29,6 +30,7 @@ describe('ReadFileTool', () => { const mockConfigInstance = { getFileService: () => new FileDiscoveryService(tempRootDir), + getFileSystemService: () => new StandardFileSystemService(), getTargetDir: () => tempRootDir, getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir), } as unknown as Config; diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index f02db506..dde3cc0c 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -74,6 +74,7 @@ class ReadFileToolInvocation extends BaseToolInvocation< const result = await processSingleFileContent( this.params.absolute_path, this.config.getTargetDir(), + this.config.getFileSystemService(), this.params.offset, this.params.limit, ); diff --git a/packages/core/src/tools/read-many-files.test.ts b/packages/core/src/tools/read-many-files.test.ts index af5012cd..a57b3851 100644 --- a/packages/core/src/tools/read-many-files.test.ts +++ b/packages/core/src/tools/read-many-files.test.ts @@ -14,6 +14,7 @@ import fs from 'fs'; // Actual fs for setup import os from 'os'; import { Config } from '../config/config.js'; import { WorkspaceContext } from '../utils/workspaceContext.js'; +import { StandardFileSystemService } from '../services/fileSystemService.js'; vi.mock('mime-types', () => { const lookup = (filename: string) => { @@ -59,6 +60,7 @@ describe('ReadManyFilesTool', () => { const fileService = new FileDiscoveryService(tempRootDir); const mockConfig = { getFileService: () => fileService, + getFileSystemService: () => new StandardFileSystemService(), getFileFilteringOptions: () => ({ respectGitIgnore: true, @@ -456,6 +458,7 @@ describe('ReadManyFilesTool', () => { const fileService = new FileDiscoveryService(tempDir1); const mockConfig = { getFileService: () => fileService, + getFileSystemService: () => new StandardFileSystemService(), getFileFilteringOptions: () => ({ respectGitIgnore: true, respectGeminiIgnore: true, diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index aaf524c4..46aab23d 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -388,6 +388,7 @@ ${finalExclusionPatternsForDescription const fileReadResult = await processSingleFileContent( filePath, this.config.getTargetDir(), + this.config.getFileSystemService(), ); if (fileReadResult.error) { diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 2d877115..e5d5ece9 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -37,6 +37,7 @@ import { CorrectedEditResult, } from '../utils/editCorrector.js'; import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js'; +import { StandardFileSystemService } from '../services/fileSystemService.js'; const rootDir = path.resolve(os.tmpdir(), 'gemini-cli-test-root'); @@ -55,11 +56,13 @@ vi.mocked(ensureCorrectFileContent).mockImplementation( ); // Mock Config +const fsService = new StandardFileSystemService(); const mockConfigInternal = { getTargetDir: () => rootDir, getApprovalMode: vi.fn(() => ApprovalMode.DEFAULT), setApprovalMode: vi.fn(), getGeminiClient: vi.fn(), // Initialize as a plain mock function + getFileSystemService: () => fsService, getIdeClient: vi.fn(), getIdeMode: vi.fn(() => false), getWorkspaceContext: () => createMockWorkspaceContext(rootDir), @@ -316,10 +319,9 @@ describe('WriteFileTool', () => { fs.writeFileSync(filePath, 'content', { mode: 0o000 }); const readError = new Error('Permission denied'); - const originalReadFileSync = fs.readFileSync; - vi.spyOn(fs, 'readFileSync').mockImplementationOnce(() => { - throw readError; - }); + vi.spyOn(fsService, 'readTextFile').mockImplementationOnce(() => + Promise.reject(readError), + ); const result = await getCorrectedFileContent( mockConfig, @@ -328,7 +330,7 @@ describe('WriteFileTool', () => { abortSignal, ); - expect(fs.readFileSync).toHaveBeenCalledWith(filePath, 'utf8'); + expect(fsService.readTextFile).toHaveBeenCalledWith(filePath); expect(mockEnsureCorrectEdit).not.toHaveBeenCalled(); expect(mockEnsureCorrectFileContent).not.toHaveBeenCalled(); expect(result.correctedContent).toBe(proposedContent); @@ -339,7 +341,6 @@ describe('WriteFileTool', () => { code: undefined, }); - vi.spyOn(fs, 'readFileSync').mockImplementation(originalReadFileSync); fs.chmodSync(filePath, 0o600); }); }); @@ -353,16 +354,14 @@ describe('WriteFileTool', () => { fs.writeFileSync(filePath, 'original', { mode: 0o000 }); const readError = new Error('Simulated read error for confirmation'); - const originalReadFileSync = fs.readFileSync; - vi.spyOn(fs, 'readFileSync').mockImplementationOnce(() => { - throw readError; - }); + vi.spyOn(fsService, 'readTextFile').mockImplementationOnce(() => + Promise.reject(readError), + ); const invocation = tool.build(params); const confirmation = await invocation.shouldConfirmExecute(abortSignal); expect(confirmation).toBe(false); - vi.spyOn(fs, 'readFileSync').mockImplementation(originalReadFileSync); fs.chmodSync(filePath, 0o600); }); @@ -453,15 +452,14 @@ describe('WriteFileTool', () => { const params = { file_path: filePath, content: 'test content' }; fs.writeFileSync(filePath, 'original', { mode: 0o000 }); - const readError = new Error('Simulated read error for execute'); - const originalReadFileSync = fs.readFileSync; - vi.spyOn(fs, 'readFileSync').mockImplementationOnce(() => { - throw readError; + vi.spyOn(fsService, 'readTextFile').mockImplementationOnce(() => { + const readError = new Error('Simulated read error for execute'); + return Promise.reject(readError); }); const invocation = tool.build(params); const result = await invocation.execute(abortSignal); - expect(result.llmContent).toContain('Error checking existing file:'); + expect(result.llmContent).toContain('Error checking existing file'); expect(result.returnDisplay).toMatch( /Error checking existing file: Simulated read error for execute/, ); @@ -471,7 +469,6 @@ describe('WriteFileTool', () => { type: ToolErrorType.FILE_WRITE_FAILURE, }); - vi.spyOn(fs, 'readFileSync').mockImplementation(originalReadFileSync); fs.chmodSync(filePath, 0o600); }); @@ -504,7 +501,8 @@ describe('WriteFileTool', () => { /Successfully created and wrote to new file/, ); expect(fs.existsSync(filePath)).toBe(true); - expect(fs.readFileSync(filePath, 'utf8')).toBe(correctedContent); + const writtenContent = await fsService.readTextFile(filePath); + expect(writtenContent).toBe(correctedContent); const display = result.returnDisplay as FileDiff; expect(display.fileName).toBe('execute_new_corrected_file.txt'); expect(display.fileDiff).toMatch( @@ -563,7 +561,8 @@ describe('WriteFileTool', () => { abortSignal, ); expect(result.llmContent).toMatch(/Successfully overwrote file/); - expect(fs.readFileSync(filePath, 'utf8')).toBe(correctedProposedContent); + const writtenContent = await fsService.readTextFile(filePath); + expect(writtenContent).toBe(correctedProposedContent); const display = result.returnDisplay as FileDiff; expect(display.fileName).toBe('execute_existing_corrected_file.txt'); expect(display.fileDiff).toMatch( @@ -675,12 +674,11 @@ describe('WriteFileTool', () => { const filePath = path.join(rootDir, 'permission_denied_file.txt'); const content = 'test content'; - // Mock writeFileSync to throw EACCES error - const originalWriteFileSync = fs.writeFileSync; - vi.spyOn(fs, 'writeFileSync').mockImplementationOnce(() => { + // Mock FileSystemService writeTextFile to throw EACCES error + vi.spyOn(fsService, 'writeTextFile').mockImplementationOnce(() => { const error = new Error('Permission denied') as NodeJS.ErrnoException; error.code = 'EACCES'; - throw error; + return Promise.reject(error); }); const params = { file_path: filePath, content }; @@ -694,22 +692,19 @@ describe('WriteFileTool', () => { expect(result.returnDisplay).toContain( `Permission denied writing to file: ${filePath} (EACCES)`, ); - - vi.spyOn(fs, 'writeFileSync').mockImplementation(originalWriteFileSync); }); it('should return NO_SPACE_LEFT error when write fails with ENOSPC', async () => { const filePath = path.join(rootDir, 'no_space_file.txt'); const content = 'test content'; - // Mock writeFileSync to throw ENOSPC error - const originalWriteFileSync = fs.writeFileSync; - vi.spyOn(fs, 'writeFileSync').mockImplementationOnce(() => { + // Mock FileSystemService writeTextFile to throw ENOSPC error + vi.spyOn(fsService, 'writeTextFile').mockImplementationOnce(() => { const error = new Error( 'No space left on device', ) as NodeJS.ErrnoException; error.code = 'ENOSPC'; - throw error; + return Promise.reject(error); }); const params = { file_path: filePath, content }; @@ -723,8 +718,6 @@ describe('WriteFileTool', () => { expect(result.returnDisplay).toContain( `No space left on device: ${filePath} (ENOSPC)`, ); - - vi.spyOn(fs, 'writeFileSync').mockImplementation(originalWriteFileSync); }); it('should return TARGET_IS_DIRECTORY error when write fails with EISDIR', async () => { @@ -740,12 +733,11 @@ describe('WriteFileTool', () => { return originalExistsSync(path as string); }); - // Mock writeFileSync to throw EISDIR error - const originalWriteFileSync = fs.writeFileSync; - vi.spyOn(fs, 'writeFileSync').mockImplementationOnce(() => { + // Mock FileSystemService writeTextFile to throw EISDIR error + vi.spyOn(fsService, 'writeTextFile').mockImplementationOnce(() => { const error = new Error('Is a directory') as NodeJS.ErrnoException; error.code = 'EISDIR'; - throw error; + return Promise.reject(error); }); const params = { file_path: dirPath, content }; @@ -761,7 +753,6 @@ describe('WriteFileTool', () => { ); vi.spyOn(fs, 'existsSync').mockImplementation(originalExistsSync); - vi.spyOn(fs, 'writeFileSync').mockImplementation(originalWriteFileSync); }); it('should return FILE_WRITE_FAILURE for generic write errors', async () => { @@ -771,10 +762,10 @@ describe('WriteFileTool', () => { // Ensure fs.existsSync is not mocked for this test vi.restoreAllMocks(); - // Mock writeFileSync to throw generic error - vi.spyOn(fs, 'writeFileSync').mockImplementationOnce(() => { - throw new Error('Generic write error'); - }); + // Mock FileSystemService writeTextFile to throw generic error + vi.spyOn(fsService, 'writeTextFile').mockImplementationOnce(() => + Promise.reject(new Error('Generic write error')), + ); const params = { file_path: filePath, content }; const invocation = tool.build(params); diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index c889d6a3..57cc4646 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -80,7 +80,9 @@ export async function getCorrectedFileContent( let correctedContent = proposedContent; try { - originalContent = fs.readFileSync(filePath, 'utf8'); + originalContent = await config + .getFileSystemService() + .readTextFile(filePath); fileExists = true; // File exists and was read } catch (err) { if (isNodeError(err) && err.code === 'ENOENT') { @@ -261,7 +263,9 @@ class WriteFileToolInvocation extends BaseToolInvocation< fs.mkdirSync(dirName, { recursive: true }); } - fs.writeFileSync(file_path, fileContent, 'utf8'); + await this.config + .getFileSystemService() + .writeTextFile(file_path, fileContent); // Generate diff for display result const fileName = path.basename(file_path); |
