diff options
Diffstat (limited to 'packages/core/src')
| -rw-r--r-- | packages/core/src/tools/read-file.test.ts | 525 | ||||
| -rw-r--r-- | packages/core/src/tools/read-file.ts | 41 | ||||
| -rw-r--r-- | packages/core/src/tools/shell.test.ts | 37 | ||||
| -rw-r--r-- | packages/core/src/tools/shell.ts | 7 | ||||
| -rw-r--r-- | packages/core/src/tools/tool-error.ts | 4 | ||||
| -rw-r--r-- | packages/core/src/tools/write-file.test.ts | 135 | ||||
| -rw-r--r-- | packages/core/src/tools/write-file.ts | 58 | ||||
| -rw-r--r-- | packages/core/src/utils/fileUtils.ts | 10 |
8 files changed, 597 insertions, 220 deletions
diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index 8c11afab..101b74a5 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -6,6 +6,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { ReadFileTool, ReadFileToolParams } from './read-file.js'; +import { ToolErrorType } from './tool-error.js'; import path from 'path'; import os from 'os'; import fs from 'fs'; @@ -47,276 +48,390 @@ describe('ReadFileTool', () => { absolute_path: path.join(tempRootDir, 'test.txt'), }; const result = tool.build(params); - expect(result).not.toBeTypeOf('string'); - expect(typeof result).toBe('object'); - expect( - (result as ToolInvocation<ReadFileToolParams, ToolResult>).params, - ).toEqual(params); + expect(typeof result).not.toBe('string'); }); - it('should return an invocation for valid params with offset and limit', () => { + it('should throw error if file path is relative', () => { const params: ReadFileToolParams = { - absolute_path: path.join(tempRootDir, 'test.txt'), - offset: 0, - limit: 10, + absolute_path: 'relative/path.txt', }; - const result = tool.build(params); - expect(result).not.toBeTypeOf('string'); - }); - - it('should throw error for relative path', () => { - const params: ReadFileToolParams = { absolute_path: 'test.txt' }; expect(() => tool.build(params)).toThrow( - `File path must be absolute, but was relative: test.txt. You must provide an absolute path.`, + 'File path must be absolute, but was relative: relative/path.txt. You must provide an absolute path.', ); }); - it('should throw error for path outside root', () => { - const outsidePath = path.resolve(os.tmpdir(), 'outside-root.txt'); - const params: ReadFileToolParams = { absolute_path: outsidePath }; + it('should throw error if path is outside root', () => { + const params: ReadFileToolParams = { + absolute_path: '/outside/root.txt', + }; expect(() => tool.build(params)).toThrow( - 'File path must be within one of the workspace directories', + /File path must be within one of the workspace directories/, ); }); - it('should throw error for negative offset', () => { + it('should throw error if offset is negative', () => { const params: ReadFileToolParams = { absolute_path: path.join(tempRootDir, 'test.txt'), offset: -1, - limit: 10, }; expect(() => tool.build(params)).toThrow( 'Offset must be a non-negative number', ); }); - it('should throw error for non-positive limit', () => { - const paramsZero: ReadFileToolParams = { + it('should throw error if limit is zero or negative', () => { + const params: ReadFileToolParams = { absolute_path: path.join(tempRootDir, 'test.txt'), - offset: 0, limit: 0, }; - expect(() => tool.build(paramsZero)).toThrow( + expect(() => tool.build(params)).toThrow( 'Limit must be a positive number', ); - const paramsNegative: ReadFileToolParams = { - absolute_path: path.join(tempRootDir, 'test.txt'), - offset: 0, - limit: -5, + }); + }); + + describe('getDescription', () => { + it('should return relative path without limit/offset', () => { + const subDir = path.join(tempRootDir, 'sub', 'dir'); + const params: ReadFileToolParams = { + absolute_path: path.join(subDir, 'file.txt'), }; - expect(() => tool.build(paramsNegative)).toThrow( - 'Limit must be a positive number', - ); + const invocation = tool.build(params); + expect(typeof invocation).not.toBe('string'); + expect( + ( + invocation as ToolInvocation<ReadFileToolParams, ToolResult> + ).getDescription(), + ).toBe(path.join('sub', 'dir', 'file.txt')); }); - it('should throw error for schema validation failure (e.g. missing path)', () => { - const params = { offset: 0 } as unknown as ReadFileToolParams; - expect(() => tool.build(params)).toThrow( - `params must have required property 'absolute_path'`, + it('should return shortened path when file path is deep', () => { + const deepPath = path.join( + tempRootDir, + 'very', + 'deep', + 'directory', + 'structure', + 'that', + 'exceeds', + 'the', + 'normal', + 'limit', + 'file.txt', ); + const params: ReadFileToolParams = { absolute_path: deepPath }; + const invocation = tool.build(params); + expect(typeof invocation).not.toBe('string'); + const desc = ( + invocation as ToolInvocation<ReadFileToolParams, ToolResult> + ).getDescription(); + expect(desc).toContain('...'); + expect(desc).toContain('file.txt'); }); - }); - describe('ToolInvocation', () => { - describe('getDescription', () => { - it('should return a shortened, relative path', () => { - const filePath = path.join(tempRootDir, 'sub', 'dir', 'file.txt'); - const params: ReadFileToolParams = { absolute_path: filePath }; - const invocation = tool.build(params); - expect(typeof invocation).not.toBe('string'); - expect( - ( - invocation as ToolInvocation<ReadFileToolParams, ToolResult> - ).getDescription(), - ).toBe(path.join('sub', 'dir', 'file.txt')); - }); + it('should handle non-normalized file paths correctly', () => { + const subDir = path.join(tempRootDir, 'sub', 'dir'); + const params: ReadFileToolParams = { + absolute_path: path.join(subDir, '..', 'dir', 'file.txt'), + }; + const invocation = tool.build(params); + expect(typeof invocation).not.toBe('string'); + expect( + ( + invocation as ToolInvocation<ReadFileToolParams, ToolResult> + ).getDescription(), + ).toBe(path.join('sub', 'dir', 'file.txt')); + }); - it('should return . if path is the root directory', () => { - const params: ReadFileToolParams = { absolute_path: tempRootDir }; - const invocation = tool.build(params); - expect(typeof invocation).not.toBe('string'); - expect( - ( - invocation as ToolInvocation<ReadFileToolParams, ToolResult> - ).getDescription(), - ).toBe('.'); - }); + it('should return . if path is the root directory', () => { + const params: ReadFileToolParams = { absolute_path: tempRootDir }; + const invocation = tool.build(params); + expect(typeof invocation).not.toBe('string'); + expect( + ( + invocation as ToolInvocation<ReadFileToolParams, ToolResult> + ).getDescription(), + ).toBe('.'); }); + }); - describe('execute', () => { - it('should return error if file does not exist', async () => { - const filePath = path.join(tempRootDir, 'nonexistent.txt'); - const params: ReadFileToolParams = { absolute_path: filePath }; - const invocation = tool.build(params) as ToolInvocation< - ReadFileToolParams, - ToolResult - >; + describe('execute', () => { + it('should return error if file does not exist', async () => { + const filePath = path.join(tempRootDir, 'nonexistent.txt'); + const params: ReadFileToolParams = { absolute_path: filePath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - expect(await invocation.execute(abortSignal)).toEqual({ - llmContent: `File not found: ${filePath}`, - returnDisplay: 'File not found.', - }); + const result = await invocation.execute(abortSignal); + expect(result).toEqual({ + llmContent: + 'Could not read file because no file was found at the specified path.', + returnDisplay: 'File not found.', + error: { + message: `File not found: ${filePath}`, + type: ToolErrorType.FILE_NOT_FOUND, + }, }); + }); - it('should return success result for a text file', async () => { - const filePath = path.join(tempRootDir, 'textfile.txt'); - const fileContent = 'This is a test file.'; - await fsp.writeFile(filePath, fileContent, 'utf-8'); - const params: ReadFileToolParams = { absolute_path: filePath }; - const invocation = tool.build(params) as ToolInvocation< - ReadFileToolParams, - ToolResult - >; + it('should return success result for a text file', async () => { + const filePath = path.join(tempRootDir, 'textfile.txt'); + const fileContent = 'This is a test file.'; + await fsp.writeFile(filePath, fileContent, 'utf-8'); + const params: ReadFileToolParams = { absolute_path: filePath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - expect(await invocation.execute(abortSignal)).toEqual({ - llmContent: fileContent, - returnDisplay: '', - }); + expect(await invocation.execute(abortSignal)).toEqual({ + llmContent: fileContent, + returnDisplay: '', }); + }); - it('should return success result for an image file', async () => { - // A minimal 1x1 transparent PNG file. - const pngContent = Buffer.from([ - 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, - 1, 0, 0, 0, 1, 8, 6, 0, 0, 0, 31, 21, 196, 137, 0, 0, 0, 10, 73, 68, - 65, 84, 120, 156, 99, 0, 1, 0, 0, 5, 0, 1, 13, 10, 45, 180, 0, 0, 0, - 0, 73, 69, 78, 68, 174, 66, 96, 130, - ]); - const filePath = path.join(tempRootDir, 'image.png'); - await fsp.writeFile(filePath, pngContent); - const params: ReadFileToolParams = { absolute_path: filePath }; - const invocation = tool.build(params) as ToolInvocation< - ReadFileToolParams, - ToolResult - >; + it('should return error if path is a directory', async () => { + const dirPath = path.join(tempRootDir, 'directory'); + await fsp.mkdir(dirPath); + const params: ReadFileToolParams = { absolute_path: dirPath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - expect(await invocation.execute(abortSignal)).toEqual({ - llmContent: { - inlineData: { - mimeType: 'image/png', - data: pngContent.toString('base64'), - }, - }, - returnDisplay: `Read image file: image.png`, - }); + const result = await invocation.execute(abortSignal); + expect(result).toEqual({ + llmContent: + 'Could not read file because the provided path is a directory, not a file.', + returnDisplay: 'Path is a directory.', + error: { + message: `Path is a directory, not a file: ${dirPath}`, + type: ToolErrorType.INVALID_TOOL_PARAMS, + }, }); + }); - it('should treat a non-image file with image extension as an image', async () => { - const filePath = path.join(tempRootDir, 'fake-image.png'); - const fileContent = 'This is not a real png.'; - await fsp.writeFile(filePath, fileContent, 'utf-8'); - const params: ReadFileToolParams = { absolute_path: filePath }; - const invocation = tool.build(params) as ToolInvocation< - ReadFileToolParams, - ToolResult - >; + it('should return error for a file that is too large', async () => { + const filePath = path.join(tempRootDir, 'largefile.txt'); + // 21MB of content exceeds 20MB limit + const largeContent = 'x'.repeat(21 * 1024 * 1024); + await fsp.writeFile(filePath, largeContent, 'utf-8'); + const params: ReadFileToolParams = { absolute_path: filePath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - expect(await invocation.execute(abortSignal)).toEqual({ - llmContent: { - inlineData: { - mimeType: 'image/png', - data: Buffer.from(fileContent).toString('base64'), - }, - }, - returnDisplay: `Read image file: fake-image.png`, - }); - }); + const result = await invocation.execute(abortSignal); + expect(result).toHaveProperty('error'); + expect(result.error?.type).toBe(ToolErrorType.FILE_TOO_LARGE); + expect(result.error?.message).toContain( + 'File size exceeds the 20MB limit', + ); + }); - it('should return a structured message when a slice of a text file is read', async () => { - const filePath = path.join(tempRootDir, 'paginated.txt'); - const fileContent = Array.from( - { length: 20 }, - (_, i) => `Line ${i + 1}`, - ).join('\n'); - await fsp.writeFile(filePath, fileContent, 'utf-8'); + it('should handle text file with lines exceeding maximum length', async () => { + const filePath = path.join(tempRootDir, 'longlines.txt'); + const longLine = 'a'.repeat(2500); // Exceeds MAX_LINE_LENGTH_TEXT_FILE (2000) + const fileContent = `Short line\n${longLine}\nAnother short line`; + await fsp.writeFile(filePath, fileContent, 'utf-8'); + const params: ReadFileToolParams = { absolute_path: filePath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - const params: ReadFileToolParams = { - absolute_path: filePath, - offset: 5, // Start from line 6 - limit: 3, - }; - const invocation = tool.build(params) as ToolInvocation< - ReadFileToolParams, - ToolResult - >; + const result = await invocation.execute(abortSignal); + expect(result.llmContent).toContain( + 'IMPORTANT: The file content has been truncated', + ); + expect(result.llmContent).toContain('--- FILE CONTENT (truncated) ---'); + expect(result.returnDisplay).toContain('some lines were shortened'); + }); - const result = await invocation.execute(abortSignal); + it('should handle image file and return appropriate content', async () => { + const imagePath = path.join(tempRootDir, 'image.png'); + // Minimal PNG header + const pngHeader = Buffer.from([ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, + ]); + await fsp.writeFile(imagePath, pngHeader); + const params: ReadFileToolParams = { absolute_path: imagePath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - const expectedLlmContent = ` -IMPORTANT: The file content has been truncated. -Status: Showing lines 6-8 of 20 total lines. -Action: To read more of the file, you can use the 'offset' and 'limit' parameters in a subsequent 'read_file' call. For example, to read the next section of the file, use offset: 8. + const result = await invocation.execute(abortSignal); + expect(result.llmContent).toEqual({ + inlineData: { + data: pngHeader.toString('base64'), + mimeType: 'image/png', + }, + }); + expect(result.returnDisplay).toBe('Read image file: image.png'); + }); ---- FILE CONTENT (truncated) --- -Line 6 -Line 7 -Line 8`; + it('should handle PDF file and return appropriate content', async () => { + const pdfPath = path.join(tempRootDir, 'document.pdf'); + // Minimal PDF header + const pdfHeader = Buffer.from('%PDF-1.4'); + await fsp.writeFile(pdfPath, pdfHeader); + const params: ReadFileToolParams = { absolute_path: pdfPath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - expect(result.llmContent).toEqual(expectedLlmContent); - expect(result.returnDisplay).toBe( - 'Read lines 6-8 of 20 from paginated.txt', - ); + const result = await invocation.execute(abortSignal); + expect(result.llmContent).toEqual({ + inlineData: { + data: pdfHeader.toString('base64'), + mimeType: 'application/pdf', + }, }); + expect(result.returnDisplay).toBe('Read pdf file: document.pdf'); + }); - describe('with .geminiignore', () => { - beforeEach(async () => { - await fsp.writeFile( - path.join(tempRootDir, '.geminiignore'), - ['foo.*', 'ignored/'].join('\n'), - ); - }); + it('should handle binary file and skip content', async () => { + const binPath = path.join(tempRootDir, 'binary.bin'); + // Binary data with null bytes + const binaryData = Buffer.from([0x00, 0xff, 0x00, 0xff]); + await fsp.writeFile(binPath, binaryData); + const params: ReadFileToolParams = { absolute_path: binPath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - it('should throw error if path is ignored by a .geminiignore pattern', async () => { - const ignoredFilePath = path.join(tempRootDir, 'foo.bar'); - await fsp.writeFile(ignoredFilePath, 'content', 'utf-8'); - const params: ReadFileToolParams = { - absolute_path: ignoredFilePath, - }; - const expectedError = `File path '${ignoredFilePath}' is ignored by .geminiignore pattern(s).`; - expect(() => tool.build(params)).toThrow(expectedError); - }); + const result = await invocation.execute(abortSignal); + expect(result.llmContent).toBe( + 'Cannot display content of binary file: binary.bin', + ); + expect(result.returnDisplay).toBe('Skipped binary file: binary.bin'); + }); - it('should throw error if path is in an ignored directory', async () => { - const ignoredDirPath = path.join(tempRootDir, 'ignored'); - await fsp.mkdir(ignoredDirPath); - const filePath = path.join(ignoredDirPath, 'somefile.txt'); - await fsp.writeFile(filePath, 'content', 'utf-8'); + it('should handle SVG file as text', async () => { + const svgPath = path.join(tempRootDir, 'image.svg'); + const svgContent = '<svg><circle cx="50" cy="50" r="40"/></svg>'; + await fsp.writeFile(svgPath, svgContent, 'utf-8'); + const params: ReadFileToolParams = { absolute_path: svgPath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - const params: ReadFileToolParams = { - absolute_path: filePath, - }; - const expectedError = `File path '${filePath}' is ignored by .geminiignore pattern(s).`; - expect(() => tool.build(params)).toThrow(expectedError); - }); - }); + const result = await invocation.execute(abortSignal); + expect(result.llmContent).toBe(svgContent); + expect(result.returnDisplay).toBe('Read SVG as text: image.svg'); }); - }); - describe('workspace boundary validation', () => { - it('should validate paths are within workspace root', () => { - const params: ReadFileToolParams = { - absolute_path: path.join(tempRootDir, 'file.txt'), - }; - expect(() => tool.build(params)).not.toThrow(); - }); + it('should handle large SVG file', async () => { + const svgPath = path.join(tempRootDir, 'large.svg'); + // Create SVG content larger than 1MB + const largeContent = '<svg>' + 'x'.repeat(1024 * 1024 + 1) + '</svg>'; + await fsp.writeFile(svgPath, largeContent, 'utf-8'); + const params: ReadFileToolParams = { absolute_path: svgPath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; - it('should reject paths outside workspace root', () => { - const params: ReadFileToolParams = { - absolute_path: '/etc/passwd', - }; - expect(() => tool.build(params)).toThrow( - 'File path must be within one of the workspace directories', + const result = await invocation.execute(abortSignal); + expect(result.llmContent).toBe( + 'Cannot display content of SVG file larger than 1MB: large.svg', ); + expect(result.returnDisplay).toBe( + 'Skipped large SVG file (>1MB): large.svg', + ); + }); + + it('should handle empty file', async () => { + const emptyPath = path.join(tempRootDir, 'empty.txt'); + await fsp.writeFile(emptyPath, '', 'utf-8'); + const params: ReadFileToolParams = { absolute_path: emptyPath }; + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; + + const result = await invocation.execute(abortSignal); + expect(result.llmContent).toBe(''); + expect(result.returnDisplay).toBe(''); }); - it('should provide clear error message with workspace directories', () => { - const outsidePath = path.join(os.tmpdir(), 'outside-workspace.txt'); + it('should support offset and limit for text files', async () => { + const filePath = path.join(tempRootDir, 'paginated.txt'); + const lines = Array.from({ length: 20 }, (_, i) => `Line ${i + 1}`); + const fileContent = lines.join('\n'); + await fsp.writeFile(filePath, fileContent, 'utf-8'); + const params: ReadFileToolParams = { - absolute_path: outsidePath, + absolute_path: filePath, + offset: 5, // Start from line 6 + limit: 3, }; - expect(() => tool.build(params)).toThrow( - 'File path must be within one of the workspace directories', + const invocation = tool.build(params) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; + + const result = await invocation.execute(abortSignal); + expect(result.llmContent).toContain( + 'IMPORTANT: The file content has been truncated', ); + expect(result.llmContent).toContain( + 'Status: Showing lines 6-8 of 20 total lines', + ); + expect(result.llmContent).toContain('Line 6'); + expect(result.llmContent).toContain('Line 7'); + expect(result.llmContent).toContain('Line 8'); + expect(result.returnDisplay).toBe( + 'Read lines 6-8 of 20 from paginated.txt', + ); + }); + + describe('with .geminiignore', () => { + beforeEach(async () => { + await fsp.writeFile( + path.join(tempRootDir, '.geminiignore'), + ['foo.*', 'ignored/'].join('\n'), + ); + }); + + it('should throw error if path is ignored by a .geminiignore pattern', async () => { + const ignoredFilePath = path.join(tempRootDir, 'foo.bar'); + await fsp.writeFile(ignoredFilePath, 'content', 'utf-8'); + const params: ReadFileToolParams = { + absolute_path: ignoredFilePath, + }; + const expectedError = `File path '${ignoredFilePath}' is ignored by .geminiignore pattern(s).`; + expect(() => tool.build(params)).toThrow(expectedError); + }); + + it('should throw error if file is in an ignored directory', async () => { + const ignoredDirPath = path.join(tempRootDir, 'ignored'); + await fsp.mkdir(ignoredDirPath, { recursive: true }); + const ignoredFilePath = path.join(ignoredDirPath, 'file.txt'); + await fsp.writeFile(ignoredFilePath, 'content', 'utf-8'); + const params: ReadFileToolParams = { + absolute_path: ignoredFilePath, + }; + const expectedError = `File path '${ignoredFilePath}' is ignored by .geminiignore pattern(s).`; + expect(() => tool.build(params)).toThrow(expectedError); + }); + + it('should allow reading non-ignored files', async () => { + const allowedFilePath = path.join(tempRootDir, 'allowed.txt'); + await fsp.writeFile(allowedFilePath, 'content', 'utf-8'); + const params: ReadFileToolParams = { + absolute_path: allowedFilePath, + }; + const invocation = tool.build(params); + expect(typeof invocation).not.toBe('string'); + }); }); }); }); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 4c1d044c..2ab50282 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -15,6 +15,7 @@ import { ToolLocation, ToolResult, } from './tools.js'; +import { ToolErrorType } from './tool-error.js'; import { PartUnion, Type } from '@google/genai'; import { processSingleFileContent, @@ -78,9 +79,45 @@ class ReadFileToolInvocation extends BaseToolInvocation< ); if (result.error) { + // Map error messages to ToolErrorType + let errorType: ToolErrorType; + let llmContent: string; + + // Check error message patterns to determine error type + if ( + result.error.includes('File not found') || + result.error.includes('does not exist') || + result.error.includes('ENOENT') + ) { + errorType = ToolErrorType.FILE_NOT_FOUND; + llmContent = + 'Could not read file because no file was found at the specified path.'; + } else if ( + result.error.includes('is a directory') || + result.error.includes('EISDIR') + ) { + errorType = ToolErrorType.INVALID_TOOL_PARAMS; + llmContent = + 'Could not read file because the provided path is a directory, not a file.'; + } else if ( + result.error.includes('too large') || + result.error.includes('File size exceeds') + ) { + errorType = ToolErrorType.FILE_TOO_LARGE; + llmContent = `Could not read file. ${result.error}`; + } else { + // Other read errors map to READ_CONTENT_FAILURE + errorType = ToolErrorType.READ_CONTENT_FAILURE; + llmContent = `Could not read file. ${result.error}`; + } + return { - llmContent: result.error, // The detailed error for LLM - returnDisplay: result.returnDisplay || 'Error reading file', // User-friendly error + llmContent, + returnDisplay: result.returnDisplay || 'Error reading file', + error: { + message: result.error, + type: errorType, + }, }; } diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 7f237e3d..55720af5 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -25,6 +25,7 @@ vi.mock('../utils/summarizer.js'); import { isCommandAllowed } from '../utils/shell-utils.js'; import { ShellTool } from './shell.js'; +import { ToolErrorType } from './tool-error.js'; import { type Config } from '../config/config.js'; import { type ShellExecutionResult, @@ -203,6 +204,42 @@ describe('ShellTool', () => { expect(result.llmContent).not.toContain('pgrep'); }); + it('should return error with error property for invalid parameters', async () => { + const result = await shellTool.execute( + { command: '' }, // Empty command is invalid + mockAbortSignal, + ); + + expect(result.llmContent).toContain( + 'Could not execute command due to invalid parameters:', + ); + expect(result.returnDisplay).toBe('Command cannot be empty.'); + expect(result.error).toEqual({ + message: 'Command cannot be empty.', + type: ToolErrorType.INVALID_TOOL_PARAMS, + }); + }); + + it('should return error with error property for invalid directory', async () => { + vi.mocked(fs.existsSync).mockReturnValue(false); + const result = await shellTool.execute( + { command: 'ls', directory: 'nonexistent' }, + mockAbortSignal, + ); + + expect(result.llmContent).toContain( + 'Could not execute command due to invalid parameters:', + ); + expect(result.returnDisplay).toBe( + "Directory 'nonexistent' is not a registered workspace directory.", + ); + expect(result.error).toEqual({ + message: + "Directory 'nonexistent' is not a registered workspace directory.", + type: ToolErrorType.INVALID_TOOL_PARAMS, + }); + }); + it('should summarize output when configured', async () => { (mockConfig.getSummarizeToolOutputConfig as Mock).mockReturnValue({ [shellTool.name]: { tokenBudget: 1000 }, diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 96423af1..6106c0cd 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -17,6 +17,7 @@ import { ToolConfirmationOutcome, Icon, } from './tools.js'; +import { ToolErrorType } from './tool-error.js'; import { Type } from '@google/genai'; import { SchemaValidator } from '../utils/schemaValidator.js'; import { getErrorMessage } from '../utils/errors.js'; @@ -186,8 +187,12 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> { }); if (validationError) { return { - llmContent: validationError, + llmContent: `Could not execute command due to invalid parameters: ${validationError}`, returnDisplay: validationError, + error: { + message: validationError, + type: ToolErrorType.INVALID_TOOL_PARAMS, + }, }; } diff --git a/packages/core/src/tools/tool-error.ts b/packages/core/src/tools/tool-error.ts index 38caa1da..2702de02 100644 --- a/packages/core/src/tools/tool-error.ts +++ b/packages/core/src/tools/tool-error.ts @@ -19,6 +19,10 @@ export enum ToolErrorType { FILE_WRITE_FAILURE = 'file_write_failure', READ_CONTENT_FAILURE = 'read_content_failure', ATTEMPT_TO_CREATE_EXISTING_FILE = 'attempt_to_create_existing_file', + FILE_TOO_LARGE = 'file_too_large', + PERMISSION_DENIED = 'permission_denied', + NO_SPACE_LEFT = 'no_space_left', + TARGET_IS_DIRECTORY = 'target_is_directory', // Edit-specific Errors EDIT_PREPARATION_FAILURE = 'edit_preparation_failure', diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 563579bb..1967b99b 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -14,6 +14,7 @@ import { type Mocked, } from 'vitest'; import { WriteFileTool, WriteFileToolParams } from './write-file.js'; +import { ToolErrorType } from './tool-error.js'; import { FileDiff, ToolConfirmationOutcome, @@ -464,18 +465,27 @@ describe('WriteFileTool', () => { it('should return error if params are invalid (relative path)', async () => { const params = { file_path: 'relative.txt', content: 'test' }; const result = await tool.execute(params, abortSignal); - expect(result.llmContent).toMatch(/Error: Invalid parameters provided/); - expect(result.returnDisplay).toMatch(/Error: File path must be absolute/); + expect(result.llmContent).toContain( + 'Could not write file due to invalid parameters:', + ); + expect(result.returnDisplay).toMatch(/File path must be absolute/); + expect(result.error).toEqual({ + message: 'File path must be absolute: relative.txt', + type: ToolErrorType.INVALID_TOOL_PARAMS, + }); }); it('should return error if params are invalid (path outside root)', async () => { const outsidePath = path.resolve(tempDir, 'outside-root.txt'); const params = { file_path: outsidePath, content: 'test' }; const result = await tool.execute(params, abortSignal); - expect(result.llmContent).toMatch(/Error: Invalid parameters provided/); + expect(result.llmContent).toContain( + 'Could not write file due to invalid parameters:', + ); expect(result.returnDisplay).toContain( - 'Error: File path must be within one of the workspace directories', + 'File path must be within one of the workspace directories', ); + expect(result.error?.type).toBe(ToolErrorType.INVALID_TOOL_PARAMS); }); it('should return error if _getCorrectedFileContent returns an error during execute', async () => { @@ -490,10 +500,15 @@ describe('WriteFileTool', () => { }); const result = await tool.execute(params, abortSignal); - expect(result.llmContent).toMatch(/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/, ); + expect(result.error).toEqual({ + message: + 'Error checking existing file: Simulated read error for execute', + type: ToolErrorType.FILE_WRITE_FAILURE, + }); vi.spyOn(fs, 'readFileSync').mockImplementation(originalReadFileSync); fs.chmodSync(filePath, 0o600); @@ -709,4 +724,114 @@ describe('WriteFileTool', () => { expect(error).toContain(rootDir); }); }); + + describe('specific error types for write failures', () => { + const abortSignal = new AbortController().signal; + + it('should return PERMISSION_DENIED error when write fails with EACCES', async () => { + 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(() => { + const error = new Error('Permission denied') as NodeJS.ErrnoException; + error.code = 'EACCES'; + throw error; + }); + + const params = { file_path: filePath, content }; + const result = await tool.execute(params, abortSignal); + + expect(result.error?.type).toBe(ToolErrorType.PERMISSION_DENIED); + expect(result.llmContent).toContain( + `Permission denied writing to file: ${filePath} (EACCES)`, + ); + expect(result.returnDisplay).toContain('Permission denied'); + + 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(() => { + const error = new Error( + 'No space left on device', + ) as NodeJS.ErrnoException; + error.code = 'ENOSPC'; + throw error; + }); + + const params = { file_path: filePath, content }; + const result = await tool.execute(params, abortSignal); + + expect(result.error?.type).toBe(ToolErrorType.NO_SPACE_LEFT); + expect(result.llmContent).toContain( + `No space left on device: ${filePath} (ENOSPC)`, + ); + expect(result.returnDisplay).toContain('No space left'); + + vi.spyOn(fs, 'writeFileSync').mockImplementation(originalWriteFileSync); + }); + + it('should return TARGET_IS_DIRECTORY error when write fails with EISDIR', async () => { + const dirPath = path.join(rootDir, 'test_directory'); + const content = 'test content'; + + // Mock fs.existsSync to return false to bypass validation + const originalExistsSync = fs.existsSync; + vi.spyOn(fs, 'existsSync').mockImplementation((path) => { + if (path === dirPath) { + return false; // Pretend directory doesn't exist to bypass validation + } + return originalExistsSync(path as string); + }); + + // Mock writeFileSync to throw EISDIR error + const originalWriteFileSync = fs.writeFileSync; + vi.spyOn(fs, 'writeFileSync').mockImplementationOnce(() => { + const error = new Error('Is a directory') as NodeJS.ErrnoException; + error.code = 'EISDIR'; + throw error; + }); + + const params = { file_path: dirPath, content }; + const result = await tool.execute(params, abortSignal); + + expect(result.error?.type).toBe(ToolErrorType.TARGET_IS_DIRECTORY); + expect(result.llmContent).toContain( + `Target is a directory, not a file: ${dirPath} (EISDIR)`, + ); + expect(result.returnDisplay).toContain('Target is a directory'); + + vi.spyOn(fs, 'existsSync').mockImplementation(originalExistsSync); + vi.spyOn(fs, 'writeFileSync').mockImplementation(originalWriteFileSync); + }); + + it('should return FILE_WRITE_FAILURE for generic write errors', async () => { + const filePath = path.join(rootDir, 'generic_error_file.txt'); + const content = 'test content'; + + // 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'); + }); + + const params = { file_path: filePath, content }; + const result = await tool.execute(params, abortSignal); + + expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE); + expect(result.llmContent).toContain( + 'Error writing to file: Generic write error', + ); + expect(result.returnDisplay).toContain('Generic write error'); + }); + }); }); diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 4a9f8d35..b018d653 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -17,6 +17,7 @@ import { ToolCallConfirmationDetails, Icon, } from './tools.js'; +import { ToolErrorType } from './tool-error.js'; import { Type } from '@google/genai'; import { SchemaValidator } from '../utils/schemaValidator.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; @@ -230,8 +231,12 @@ export class WriteFileTool const validationError = this.validateToolParams(params); if (validationError) { return { - llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`, - returnDisplay: `Error: ${validationError}`, + llmContent: `Could not write file due to invalid parameters: ${validationError}`, + returnDisplay: validationError, + error: { + message: validationError, + type: ToolErrorType.INVALID_TOOL_PARAMS, + }, }; } @@ -243,10 +248,16 @@ export class WriteFileTool if (correctedContentResult.error) { const errDetails = correctedContentResult.error; - const errorMsg = `Error checking existing file: ${errDetails.message}`; + const errorMsg = errDetails.code + ? `Error checking existing file '${params.file_path}': ${errDetails.message} (${errDetails.code})` + : `Error checking existing file: ${errDetails.message}`; return { - llmContent: `Error checking existing file ${params.file_path}: ${errDetails.message}`, + llmContent: errorMsg, returnDisplay: errorMsg, + error: { + message: errorMsg, + type: ToolErrorType.FILE_WRITE_FAILURE, + }, }; } @@ -344,10 +355,43 @@ export class WriteFileTool returnDisplay: displayResult, }; } catch (error) { - const errorMsg = `Error writing to file: ${error instanceof Error ? error.message : String(error)}`; + // Capture detailed error information for debugging + let errorMsg: string; + let errorType = ToolErrorType.FILE_WRITE_FAILURE; + + if (isNodeError(error)) { + // Handle specific Node.js errors with their error codes + errorMsg = `Error writing to file '${params.file_path}': ${error.message} (${error.code})`; + + // Log specific error types for better debugging + if (error.code === 'EACCES') { + errorMsg = `Permission denied writing to file: ${params.file_path} (${error.code})`; + errorType = ToolErrorType.PERMISSION_DENIED; + } else if (error.code === 'ENOSPC') { + errorMsg = `No space left on device: ${params.file_path} (${error.code})`; + errorType = ToolErrorType.NO_SPACE_LEFT; + } else if (error.code === 'EISDIR') { + errorMsg = `Target is a directory, not a file: ${params.file_path} (${error.code})`; + errorType = ToolErrorType.TARGET_IS_DIRECTORY; + } + + // Include stack trace in debug mode for better troubleshooting + if (this.config.getDebugMode() && error.stack) { + console.error('Write file error stack:', error.stack); + } + } else if (error instanceof Error) { + errorMsg = `Error writing to file: ${error.message}`; + } else { + errorMsg = `Error writing to file: ${String(error)}`; + } + return { - llmContent: `Error writing to file ${params.file_path}: ${errorMsg}`, - returnDisplay: `Error: ${errorMsg}`, + llmContent: errorMsg, + returnDisplay: errorMsg, + error: { + message: errorMsg, + type: errorType, + }, }; } } diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index 30ab69c6..92186e4f 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -195,10 +195,18 @@ export async function detectFileType( return 'text'; } +export enum FileErrorType { + FILE_NOT_FOUND = 'FILE_NOT_FOUND', + IS_DIRECTORY = 'IS_DIRECTORY', + FILE_TOO_LARGE = 'FILE_TOO_LARGE', + READ_ERROR = 'READ_ERROR', +} + export interface ProcessedFileReadResult { llmContent: PartUnion; // string for text, Part for image/pdf/unreadable binary returnDisplay: string; error?: string; // Optional error message for the LLM if file processing failed + errorType?: FileErrorType; // Structured error type using enum isTruncated?: boolean; // For text files, indicates if content was truncated originalLineCount?: number; // For text files linesShown?: [number, number]; // For text files [startLine, endLine] (1-based for display) @@ -225,6 +233,7 @@ export async function processSingleFileContent( llmContent: '', returnDisplay: 'File not found.', error: `File not found: ${filePath}`, + errorType: FileErrorType.FILE_NOT_FOUND, }; } const stats = await fs.promises.stat(filePath); @@ -233,6 +242,7 @@ export async function processSingleFileContent( llmContent: '', returnDisplay: 'Path is a directory.', error: `Path is a directory, not a file: ${filePath}`, + errorType: FileErrorType.IS_DIRECTORY, }; } |