chore: sync gemini-cli v0.1.19

This commit is contained in:
tanzhenxin
2025-08-18 19:55:46 +08:00
244 changed files with 19407 additions and 5030 deletions

View File

@@ -0,0 +1,129 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, expect, it } from 'vitest';
import { getDiffStat } from './diffOptions.js';
describe('getDiffStat', () => {
const fileName = 'test.txt';
it('should return 0 for all stats when there are no changes', () => {
const oldStr = 'line1\nline2\n';
const aiStr = 'line1\nline2\n';
const userStr = 'line1\nline2\n';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 0,
ai_removed_lines: 0,
user_added_lines: 0,
user_removed_lines: 0,
});
});
it('should correctly report AI additions', () => {
const oldStr = 'line1\nline2\n';
const aiStr = 'line1\nline2\nline3\n';
const userStr = 'line1\nline2\nline3\n';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 1,
ai_removed_lines: 0,
user_added_lines: 0,
user_removed_lines: 0,
});
});
it('should correctly report AI removals', () => {
const oldStr = 'line1\nline2\nline3\n';
const aiStr = 'line1\nline3\n';
const userStr = 'line1\nline3\n';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 0,
ai_removed_lines: 1,
user_added_lines: 0,
user_removed_lines: 0,
});
});
it('should correctly report AI modifications', () => {
const oldStr = 'line1\nline2\nline3\n';
const aiStr = 'line1\nline_two\nline3\n';
const userStr = 'line1\nline_two\nline3\n';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 1,
ai_removed_lines: 1,
user_added_lines: 0,
user_removed_lines: 0,
});
});
it('should correctly report user additions', () => {
const oldStr = 'line1\nline2\n';
const aiStr = 'line1\nline2\nline3\n';
const userStr = 'line1\nline2\nline3\nline4\n';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 1,
ai_removed_lines: 0,
user_added_lines: 1,
user_removed_lines: 0,
});
});
it('should correctly report user removals', () => {
const oldStr = 'line1\nline2\n';
const aiStr = 'line1\nline2\nline3\n';
const userStr = 'line1\nline2\n';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 1,
ai_removed_lines: 0,
user_added_lines: 0,
user_removed_lines: 1,
});
});
it('should correctly report user modifications', () => {
const oldStr = 'line1\nline2\n';
const aiStr = 'line1\nline2\nline3\n';
const userStr = 'line1\nline2\nline_three\n';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 1,
ai_removed_lines: 0,
user_added_lines: 1,
user_removed_lines: 1,
});
});
it('should handle complex changes from both AI and user', () => {
const oldStr = 'line1\nline2\nline3\nline4\n';
const aiStr = 'line_one\nline2\nline_three\nline4\n';
const userStr = 'line_one\nline_two\nline_three\nline4\nline5\n';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 2,
ai_removed_lines: 2,
user_added_lines: 2,
user_removed_lines: 1,
});
});
it('should report a single line modification as one addition and one removal', () => {
const oldStr = 'hello world';
const aiStr = 'hello universe';
const userStr = 'hello universe';
const diffStat = getDiffStat(fileName, oldStr, aiStr, userStr);
expect(diffStat).toEqual({
ai_added_lines: 1,
ai_removed_lines: 1,
user_added_lines: 0,
user_removed_lines: 0,
});
});
});

View File

@@ -5,8 +5,61 @@
*/
import * as Diff from 'diff';
import { DiffStat } from './tools.js';
export const DEFAULT_DIFF_OPTIONS: Diff.PatchOptions = {
context: 3,
ignoreWhitespace: true,
};
export function getDiffStat(
fileName: string,
oldStr: string,
aiStr: string,
userStr: string,
): DiffStat {
const countLines = (patch: Diff.ParsedDiff) => {
let added = 0;
let removed = 0;
patch.hunks.forEach((hunk: Diff.Hunk) => {
hunk.lines.forEach((line: string) => {
if (line.startsWith('+')) {
added++;
} else if (line.startsWith('-')) {
removed++;
}
});
});
return { added, removed };
};
const patch = Diff.structuredPatch(
fileName,
fileName,
oldStr,
aiStr,
'Current',
'Proposed',
DEFAULT_DIFF_OPTIONS,
);
const { added: aiAddedLines, removed: aiRemovedLines } = countLines(patch);
const userPatch = Diff.structuredPatch(
fileName,
fileName,
aiStr,
userStr,
'Proposed',
'User',
DEFAULT_DIFF_OPTIONS,
);
const { added: userAddedLines, removed: userRemovedLines } =
countLines(userPatch);
return {
ai_added_lines: aiAddedLines,
ai_removed_lines: aiRemovedLines,
user_added_lines: userAddedLines,
user_removed_lines: userRemovedLines,
};
}

View File

@@ -10,6 +10,8 @@ const mockEnsureCorrectEdit = vi.hoisted(() => vi.fn());
const mockGenerateJson = vi.hoisted(() => vi.fn());
const mockOpenDiff = vi.hoisted(() => vi.fn());
import { IDEConnectionStatus } from '../ide/ide-client.js';
vi.mock('../utils/editCorrector.js', () => ({
ensureCorrectEdit: mockEnsureCorrectEdit,
}));
@@ -25,8 +27,8 @@ vi.mock('../utils/editor.js', () => ({
}));
import { describe, it, expect, beforeEach, afterEach, vi, Mock } from 'vitest';
import { EditTool, EditToolParams } from './edit.js';
import { FileDiff } from './tools.js';
import { applyReplacement, EditTool, EditToolParams } from './edit.js';
import { FileDiff, ToolConfirmationOutcome } from './tools.js';
import { ToolErrorType } from './tool-error.js';
import path from 'path';
import fs from 'fs';
@@ -58,6 +60,9 @@ describe('EditTool', () => {
getApprovalMode: vi.fn(),
setApprovalMode: vi.fn(),
getWorkspaceContext: () => createMockWorkspaceContext(rootDir),
getIdeClient: () => undefined,
getIdeMode: () => false,
getIdeModeFeature: () => false,
// getGeminiConfig: () => ({ apiKey: 'test-api-key' }), // This was not a real Config method
// Add other properties/methods of Config if EditTool uses them
// Minimal other methods to satisfy Config type if needed by EditTool constructor or other direct uses:
@@ -150,45 +155,30 @@ describe('EditTool', () => {
fs.rmSync(tempDir, { recursive: true, force: true });
});
describe('_applyReplacement', () => {
// Access private method for testing
// Note: `tool` is initialized in `beforeEach` of the parent describe block
describe('applyReplacement', () => {
it('should return newString if isNewFile is true', () => {
expect((tool as any)._applyReplacement(null, 'old', 'new', true)).toBe(
'new',
);
expect(
(tool as any)._applyReplacement('existing', 'old', 'new', true),
).toBe('new');
expect(applyReplacement(null, 'old', 'new', true)).toBe('new');
expect(applyReplacement('existing', 'old', 'new', true)).toBe('new');
});
it('should return newString if currentContent is null and oldString is empty (defensive)', () => {
expect((tool as any)._applyReplacement(null, '', 'new', false)).toBe(
'new',
);
expect(applyReplacement(null, '', 'new', false)).toBe('new');
});
it('should return empty string if currentContent is null and oldString is not empty (defensive)', () => {
expect((tool as any)._applyReplacement(null, 'old', 'new', false)).toBe(
'',
);
expect(applyReplacement(null, 'old', 'new', false)).toBe('');
});
it('should replace oldString with newString in currentContent', () => {
expect(
(tool as any)._applyReplacement(
'hello old world old',
'old',
'new',
false,
),
).toBe('hello new world new');
expect(applyReplacement('hello old world old', 'old', 'new', false)).toBe(
'hello new world new',
);
});
it('should return currentContent if oldString is empty and not a new file', () => {
expect(
(tool as any)._applyReplacement('hello world', '', 'new', false),
).toBe('hello world');
expect(applyReplacement('hello world', '', 'new', false)).toBe(
'hello world',
);
});
});
@@ -234,15 +224,13 @@ describe('EditTool', () => {
filePath = path.join(rootDir, testFile);
});
it('should return false if params are invalid', async () => {
it('should throw an error if params are invalid', async () => {
const params: EditToolParams = {
file_path: 'relative.txt',
old_string: 'old',
new_string: 'new',
};
expect(
await tool.shouldConfirmExecute(params, new AbortController().signal),
).toBe(false);
expect(() => tool.build(params)).toThrow();
});
it('should request confirmation for valid edit', async () => {
@@ -254,8 +242,8 @@ describe('EditTool', () => {
};
// ensureCorrectEdit will be called by shouldConfirmExecute
mockEnsureCorrectEdit.mockResolvedValueOnce({ params, occurrences: 1 });
const confirmation = await tool.shouldConfirmExecute(
params,
const invocation = tool.build(params);
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
);
expect(confirmation).toEqual(
@@ -275,9 +263,11 @@ describe('EditTool', () => {
new_string: 'new',
};
mockEnsureCorrectEdit.mockResolvedValueOnce({ params, occurrences: 0 });
expect(
await tool.shouldConfirmExecute(params, new AbortController().signal),
).toBe(false);
const invocation = tool.build(params);
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
);
expect(confirmation).toBe(false);
});
it('should return false if multiple occurrences of old_string are found (ensureCorrectEdit returns > 1)', async () => {
@@ -288,9 +278,11 @@ describe('EditTool', () => {
new_string: 'new',
};
mockEnsureCorrectEdit.mockResolvedValueOnce({ params, occurrences: 2 });
expect(
await tool.shouldConfirmExecute(params, new AbortController().signal),
).toBe(false);
const invocation = tool.build(params);
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
);
expect(confirmation).toBe(false);
});
it('should request confirmation for creating a new file (empty old_string)', async () => {
@@ -305,8 +297,8 @@ describe('EditTool', () => {
// as shouldConfirmExecute handles this for diff generation.
// If it is called, it should return 0 occurrences for a new file.
mockEnsureCorrectEdit.mockResolvedValueOnce({ params, occurrences: 0 });
const confirmation = await tool.shouldConfirmExecute(
params,
const invocation = tool.build(params);
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
);
expect(confirmation).toEqual(
@@ -353,9 +345,8 @@ describe('EditTool', () => {
};
},
);
const confirmation = (await tool.shouldConfirmExecute(
params,
const invocation = tool.build(params);
const confirmation = (await invocation.shouldConfirmExecute(
new AbortController().signal,
)) as FileDiff;
@@ -403,15 +394,13 @@ describe('EditTool', () => {
});
});
it('should return error if params are invalid', async () => {
it('should throw error if params are invalid', async () => {
const params: EditToolParams = {
file_path: 'relative.txt',
old_string: 'old',
new_string: 'new',
};
const result = await tool.execute(params, new AbortController().signal);
expect(result.llmContent).toMatch(/Error: Invalid parameters provided/);
expect(result.returnDisplay).toMatch(/Error: File path must be absolute/);
expect(() => tool.build(params)).toThrow(/File path must be absolute/);
});
it('should edit an existing file and return diff with fileName', async () => {
@@ -428,12 +417,8 @@ describe('EditTool', () => {
// ensureCorrectEdit is NOT called by calculateEdit, only by shouldConfirmExecute
// So, the default mockEnsureCorrectEdit should correctly return 1 occurrence for 'old' in initialContent
// Simulate confirmation by setting shouldAlwaysEdit
(tool as any).shouldAlwaysEdit = true;
const result = await tool.execute(params, new AbortController().signal);
(tool as any).shouldAlwaysEdit = false; // Reset for other tests
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(/Successfully modified file/);
expect(fs.readFileSync(filePath, 'utf8')).toBe(newContent);
@@ -456,7 +441,8 @@ describe('EditTool', () => {
(mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
ApprovalMode.AUTO_EDIT,
);
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(/Created new file/);
expect(fs.existsSync(newFilePath)).toBe(true);
@@ -472,7 +458,8 @@ describe('EditTool', () => {
new_string: 'replacement',
};
// The default mockEnsureCorrectEdit will return 0 occurrences for 'nonexistent'
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(
/0 occurrences found for old_string in/,
);
@@ -489,7 +476,8 @@ describe('EditTool', () => {
new_string: 'new',
};
// The default mockEnsureCorrectEdit will return 2 occurrences for 'old'
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(
/Expected 1 occurrence but found 2 for old_string in file/,
);
@@ -507,12 +495,8 @@ describe('EditTool', () => {
expected_replacements: 3,
};
// Simulate confirmation by setting shouldAlwaysEdit
(tool as any).shouldAlwaysEdit = true;
const result = await tool.execute(params, new AbortController().signal);
(tool as any).shouldAlwaysEdit = false; // Reset for other tests
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(/Successfully modified file/);
expect(fs.readFileSync(filePath, 'utf8')).toBe(
@@ -532,7 +516,8 @@ describe('EditTool', () => {
new_string: 'new',
expected_replacements: 3, // Expecting 3 but only 2 exist
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(
/Expected 3 occurrences but found 2 for old_string in file/,
);
@@ -548,7 +533,8 @@ describe('EditTool', () => {
old_string: '',
new_string: 'new content',
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(/File already exists, cannot create/);
expect(result.returnDisplay).toMatch(
/Attempted to create a file that already exists/,
@@ -568,7 +554,8 @@ describe('EditTool', () => {
(mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
ApprovalMode.AUTO_EDIT,
);
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(
/User modified the `new_string` content/,
@@ -588,7 +575,8 @@ describe('EditTool', () => {
(mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
ApprovalMode.AUTO_EDIT,
);
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).not.toMatch(
/User modified the `new_string` content/,
@@ -607,7 +595,8 @@ describe('EditTool', () => {
(mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
ApprovalMode.AUTO_EDIT,
);
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).not.toMatch(
/User modified the `new_string` content/,
@@ -622,7 +611,8 @@ describe('EditTool', () => {
old_string: 'identical',
new_string: 'identical',
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toMatch(/No changes to apply/);
expect(result.returnDisplay).toMatch(/No changes to apply/);
});
@@ -642,7 +632,8 @@ describe('EditTool', () => {
old_string: 'any',
new_string: 'new',
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.error?.type).toBe(ToolErrorType.FILE_NOT_FOUND);
});
@@ -653,7 +644,8 @@ describe('EditTool', () => {
old_string: '',
new_string: 'new content',
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.error?.type).toBe(
ToolErrorType.ATTEMPT_TO_CREATE_EXISTING_FILE,
);
@@ -666,7 +658,8 @@ describe('EditTool', () => {
old_string: 'not-found',
new_string: 'new',
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.error?.type).toBe(ToolErrorType.EDIT_NO_OCCURRENCE_FOUND);
});
@@ -678,7 +671,8 @@ describe('EditTool', () => {
new_string: 'new',
expected_replacements: 3,
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.error?.type).toBe(
ToolErrorType.EDIT_EXPECTED_OCCURRENCE_MISMATCH,
);
@@ -691,18 +685,18 @@ describe('EditTool', () => {
old_string: 'content',
new_string: 'content',
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.error?.type).toBe(ToolErrorType.EDIT_NO_CHANGE);
});
it('should return INVALID_PARAMETERS error for relative path', async () => {
it('should throw INVALID_PARAMETERS error for relative path', async () => {
const params: EditToolParams = {
file_path: 'relative/path.txt',
old_string: 'a',
new_string: 'b',
};
const result = await tool.execute(params, new AbortController().signal);
expect(result.error?.type).toBe(ToolErrorType.INVALID_TOOL_PARAMS);
expect(() => tool.build(params)).toThrow();
});
it('should return FILE_WRITE_FAILURE on write error', async () => {
@@ -715,7 +709,8 @@ describe('EditTool', () => {
old_string: 'content',
new_string: 'new content',
};
const result = await tool.execute(params, new AbortController().signal);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE);
});
});
@@ -728,8 +723,9 @@ describe('EditTool', () => {
old_string: 'identical_string',
new_string: 'identical_string',
};
const invocation = tool.build(params);
// shortenPath will be called internally, resulting in just the file name
expect(tool.getDescription(params)).toBe(
expect(invocation.getDescription()).toBe(
`No file changes to ${testFileName}`,
);
});
@@ -741,9 +737,10 @@ describe('EditTool', () => {
old_string: 'this is the old string value',
new_string: 'this is the new string value',
};
const invocation = tool.build(params);
// shortenPath will be called internally, resulting in just the file name
// The snippets are truncated at 30 chars + '...'
expect(tool.getDescription(params)).toBe(
expect(invocation.getDescription()).toBe(
`${testFileName}: this is the old string value => this is the new string value`,
);
});
@@ -755,7 +752,8 @@ describe('EditTool', () => {
old_string: 'old',
new_string: 'new',
};
expect(tool.getDescription(params)).toBe(`${testFileName}: old => new`);
const invocation = tool.build(params);
expect(invocation.getDescription()).toBe(`${testFileName}: old => new`);
});
it('should truncate long strings in the description', () => {
@@ -767,7 +765,8 @@ describe('EditTool', () => {
new_string:
'this is a very long new string that will also be truncated',
};
expect(tool.getDescription(params)).toBe(
const invocation = tool.build(params);
expect(invocation.getDescription()).toBe(
`${testFileName}: this is a very long old string... => this is a very long new string...`,
);
});
@@ -796,4 +795,57 @@ describe('EditTool', () => {
expect(error).toContain(rootDir);
});
});
describe('IDE mode', () => {
const testFile = 'edit_me.txt';
let filePath: string;
let ideClient: any;
beforeEach(() => {
filePath = path.join(rootDir, testFile);
ideClient = {
openDiff: vi.fn(),
getConnectionStatus: vi.fn().mockReturnValue({
status: IDEConnectionStatus.Connected,
}),
};
(mockConfig as any).getIdeMode = () => true;
(mockConfig as any).getIdeModeFeature = () => true;
(mockConfig as any).getIdeClient = () => ideClient;
});
it('should call ideClient.openDiff and update params on confirmation', async () => {
const initialContent = 'some old content here';
const newContent = 'some new content here';
const modifiedContent = 'some modified content here';
fs.writeFileSync(filePath, initialContent);
const params: EditToolParams = {
file_path: filePath,
old_string: 'old',
new_string: 'new',
};
mockEnsureCorrectEdit.mockResolvedValueOnce({
params: { ...params, old_string: 'old', new_string: 'new' },
occurrences: 1,
});
ideClient.openDiff.mockResolvedValueOnce({
status: 'accepted',
content: modifiedContent,
});
const invocation = tool.build(params);
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
);
expect(ideClient.openDiff).toHaveBeenCalledWith(filePath, newContent);
if (confirmation && 'onConfirm' in confirmation) {
await confirmation.onConfirm(ToolConfirmationOutcome.ProceedOnce);
}
expect(params.old_string).toBe(initialContent);
expect(params.new_string).toBe(modifiedContent);
});
});
});

View File

@@ -8,25 +8,46 @@ import * as fs from 'fs';
import * as path from 'path';
import * as Diff from 'diff';
import {
BaseTool,
BaseDeclarativeTool,
Icon,
ToolCallConfirmationDetails,
ToolConfirmationOutcome,
ToolEditConfirmationDetails,
ToolInvocation,
ToolLocation,
ToolResult,
ToolResultDisplay,
} 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';
import { isNodeError } from '../utils/errors.js';
import { Config, ApprovalMode } from '../config/config.js';
import { ensureCorrectEdit } from '../utils/editCorrector.js';
import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js';
import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js';
import { ReadFileTool } from './read-file.js';
import { ModifiableTool, ModifyContext } from './modifiable-tool.js';
import { ModifiableDeclarativeTool, ModifyContext } from './modifiable-tool.js';
import { IDEConnectionStatus } from '../ide/ide-client.js';
export function applyReplacement(
currentContent: string | null,
oldString: string,
newString: string,
isNewFile: boolean,
): string {
if (isNewFile) {
return newString;
}
if (currentContent === null) {
// Should not happen if not a new file, but defensively return empty or newString if oldString is also empty
return oldString === '' ? newString : '';
}
// If oldString is empty and it's not a new file, do not modify the content.
if (oldString === '' && !isNewFile) {
return currentContent;
}
return currentContent.replaceAll(oldString, newString);
}
/**
* Parameters for the Edit tool
@@ -57,6 +78,11 @@ export interface EditToolParams {
* Whether the edit was modified manually by the user.
*/
modified_by_user?: boolean;
/**
* Initially proposed string.
*/
ai_proposed_string?: string;
}
interface CalculatedEdit {
@@ -67,112 +93,14 @@ interface CalculatedEdit {
isNewFile: boolean;
}
/**
* Implementation of the Edit tool logic
*/
export class EditTool
extends BaseTool<EditToolParams, ToolResult>
implements ModifiableTool<EditToolParams>
{
static readonly Name = 'replace';
class EditToolInvocation implements ToolInvocation<EditToolParams, ToolResult> {
constructor(
private readonly config: Config,
public params: EditToolParams,
) {}
constructor(private readonly config: Config) {
super(
EditTool.Name,
'Edit',
`Replaces text within a file. By default, replaces a single occurrence, but can replace multiple occurrences when \`expected_replacements\` is specified. This tool requires providing significant context around the change to ensure precise targeting. Always use the ${ReadFileTool.Name} tool to examine the file's current content before attempting a text replacement.
The user has the ability to modify the \`new_string\` content. If modified, this will be stated in the response.
Expectation for required parameters:
1. \`file_path\` MUST be an absolute path; otherwise an error will be thrown.
2. \`old_string\` MUST be the exact literal text to replace (including all whitespace, indentation, newlines, and surrounding code etc.).
3. \`new_string\` MUST be the exact literal text to replace \`old_string\` with (also including all whitespace, indentation, newlines, and surrounding code etc.). Ensure the resulting code is correct and idiomatic.
4. NEVER escape \`old_string\` or \`new_string\`, that would break the exact literal text requirement.
**Important:** If ANY of the above are not satisfied, the tool will fail. CRITICAL for \`old_string\`: Must uniquely identify the single instance to change. Include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. If this string matches multiple locations, or does not match exactly, the tool will fail.
**Multiple replacements:** Set \`expected_replacements\` to the number of occurrences you want to replace. The tool will replace ALL occurrences that match \`old_string\` exactly. Ensure the number of replacements matches your expectation.`,
Icon.Pencil,
{
properties: {
file_path: {
description:
"The absolute path to the file to modify. Must start with '/'.",
type: Type.STRING,
},
old_string: {
description:
'The exact literal text to replace, preferably unescaped. For single replacements (default), include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. For multiple replacements, specify expected_replacements parameter. If this string is not the exact literal text (i.e. you escaped it) or does not match exactly, the tool will fail.',
type: Type.STRING,
},
new_string: {
description:
'The exact literal text to replace `old_string` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.',
type: Type.STRING,
},
expected_replacements: {
type: Type.NUMBER,
description:
'Number of replacements expected. Defaults to 1 if not specified. Use when you want to replace multiple occurrences.',
minimum: 1,
},
},
required: ['file_path', 'old_string', 'new_string'],
type: Type.OBJECT,
},
);
}
/**
* Validates the parameters for the Edit tool
* @param params Parameters to validate
* @returns Error message string or null if valid
*/
validateToolParams(params: EditToolParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
if (errors) {
return errors;
}
if (!path.isAbsolute(params.file_path)) {
return `File path must be absolute: ${params.file_path}`;
}
const workspaceContext = this.config.getWorkspaceContext();
if (!workspaceContext.isPathWithinWorkspace(params.file_path)) {
const directories = workspaceContext.getDirectories();
return `File path must be within one of the workspace directories: ${directories.join(', ')}`;
}
return null;
}
/**
* Determines any file locations affected by the tool execution
* @param params Parameters for the tool execution
* @returns A list of such paths
*/
toolLocations(params: EditToolParams): ToolLocation[] {
return [{ path: params.file_path }];
}
private _applyReplacement(
currentContent: string | null,
oldString: string,
newString: string,
isNewFile: boolean,
): string {
if (isNewFile) {
return newString;
}
if (currentContent === null) {
// Should not happen if not a new file, but defensively return empty or newString if oldString is also empty
return oldString === '' ? newString : '';
}
// If oldString is empty and it's not a new file, do not modify the content.
if (oldString === '' && !isNewFile) {
return currentContent;
}
return currentContent.replaceAll(oldString, newString);
toolLocations(): ToolLocation[] {
return [{ path: this.params.file_path }];
}
/**
@@ -270,7 +198,7 @@ Expectation for required parameters:
};
}
const newContent = this._applyReplacement(
const newContent = applyReplacement(
currentContent,
finalOldString,
finalNewString,
@@ -291,23 +219,15 @@ Expectation for required parameters:
* It needs to calculate the diff to show the user.
*/
async shouldConfirmExecute(
params: EditToolParams,
abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails | false> {
if (this.config.getApprovalMode() === ApprovalMode.AUTO_EDIT) {
return false;
}
const validationError = this.validateToolParams(params);
if (validationError) {
console.error(
`[EditTool Wrapper] Attempted confirmation with invalid parameters: ${validationError}`,
);
return false;
}
let editData: CalculatedEdit;
try {
editData = await this.calculateEdit(params, abortSignal);
editData = await this.calculateEdit(this.params, abortSignal);
} catch (error) {
const errorMsg = error instanceof Error ? error.message : String(error);
console.log(`Error preparing edit: ${errorMsg}`);
@@ -319,7 +239,7 @@ Expectation for required parameters:
return false;
}
const fileName = path.basename(params.file_path);
const fileName = path.basename(this.params.file_path);
const fileDiff = Diff.createPatch(
fileName,
editData.currentContent ?? '',
@@ -328,10 +248,19 @@ Expectation for required parameters:
'Proposed',
DEFAULT_DIFF_OPTIONS,
);
const ideClient = this.config.getIdeClient();
const ideConfirmation =
this.config.getIdeModeFeature() &&
this.config.getIdeMode() &&
ideClient?.getConnectionStatus().status === IDEConnectionStatus.Connected
? ideClient.openDiff(this.params.file_path, editData.newContent)
: undefined;
const confirmationDetails: ToolEditConfirmationDetails = {
type: 'edit',
title: `Confirm Edit: ${shortenPath(makeRelative(params.file_path, this.config.getTargetDir()))}`,
title: `Confirm Edit: ${shortenPath(makeRelative(this.params.file_path, this.config.getTargetDir()))}`,
fileName,
filePath: this.params.file_path,
fileDiff,
originalContent: editData.currentContent,
newContent: editData.newContent,
@@ -339,31 +268,39 @@ Expectation for required parameters:
if (outcome === ToolConfirmationOutcome.ProceedAlways) {
this.config.setApprovalMode(ApprovalMode.AUTO_EDIT);
}
if (ideConfirmation) {
const result = await ideConfirmation;
if (result.status === 'accepted' && result.content) {
// TODO(chrstn): See https://github.com/google-gemini/gemini-cli/pull/5618#discussion_r2255413084
// for info on a possible race condition where the file is modified on disk while being edited.
this.params.old_string = editData.currentContent ?? '';
this.params.new_string = result.content;
}
}
},
ideConfirmation,
};
return confirmationDetails;
}
getDescription(params: EditToolParams): string {
if (!params.file_path || !params.old_string || !params.new_string) {
return `Model did not provide valid parameters for edit tool`;
}
getDescription(): string {
const relativePath = makeRelative(
params.file_path,
this.params.file_path,
this.config.getTargetDir(),
);
if (params.old_string === '') {
if (this.params.old_string === '') {
return `Create ${shortenPath(relativePath)}`;
}
const oldStringSnippet =
params.old_string.split('\n')[0].substring(0, 30) +
(params.old_string.length > 30 ? '...' : '');
this.params.old_string.split('\n')[0].substring(0, 30) +
(this.params.old_string.length > 30 ? '...' : '');
const newStringSnippet =
params.new_string.split('\n')[0].substring(0, 30) +
(params.new_string.length > 30 ? '...' : '');
this.params.new_string.split('\n')[0].substring(0, 30) +
(this.params.new_string.length > 30 ? '...' : '');
if (params.old_string === params.new_string) {
if (this.params.old_string === this.params.new_string) {
return `No file changes to ${shortenPath(relativePath)}`;
}
return `${shortenPath(relativePath)}: ${oldStringSnippet} => ${newStringSnippet}`;
@@ -374,25 +311,10 @@ Expectation for required parameters:
* @param params Parameters for the edit operation
* @returns Result of the edit operation
*/
async execute(
params: EditToolParams,
signal: AbortSignal,
): Promise<ToolResult> {
const validationError = this.validateToolParams(params);
if (validationError) {
return {
llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`,
returnDisplay: `Error: ${validationError}`,
error: {
message: validationError,
type: ToolErrorType.INVALID_TOOL_PARAMS,
},
};
}
async execute(signal: AbortSignal): Promise<ToolResult> {
let editData: CalculatedEdit;
try {
editData = await this.calculateEdit(params, signal);
editData = await this.calculateEdit(this.params, signal);
} catch (error) {
const errorMsg = error instanceof Error ? error.message : String(error);
return {
@@ -417,16 +339,16 @@ Expectation for required parameters:
}
try {
this.ensureParentDirectoriesExist(params.file_path);
fs.writeFileSync(params.file_path, editData.newContent, 'utf8');
this.ensureParentDirectoriesExist(this.params.file_path);
fs.writeFileSync(this.params.file_path, editData.newContent, 'utf8');
let displayResult: ToolResultDisplay;
if (editData.isNewFile) {
displayResult = `Created ${shortenPath(makeRelative(params.file_path, this.config.getTargetDir()))}`;
displayResult = `Created ${shortenPath(makeRelative(this.params.file_path, this.config.getTargetDir()))}`;
} else {
// Generate diff for display, even though core logic doesn't technically need it
// The CLI wrapper will use this part of the ToolResult
const fileName = path.basename(params.file_path);
const fileName = path.basename(this.params.file_path);
const fileDiff = Diff.createPatch(
fileName,
editData.currentContent ?? '', // Should not be null here if not isNewFile
@@ -435,22 +357,31 @@ Expectation for required parameters:
'Proposed',
DEFAULT_DIFF_OPTIONS,
);
const originallyProposedContent =
this.params.ai_proposed_string || this.params.new_string;
const diffStat = getDiffStat(
fileName,
editData.currentContent ?? '',
originallyProposedContent,
this.params.new_string,
);
displayResult = {
fileDiff,
fileName,
originalContent: editData.currentContent,
newContent: editData.newContent,
diffStat,
};
}
const llmSuccessMessageParts = [
editData.isNewFile
? `Created new file: ${params.file_path} with provided content.`
: `Successfully modified file: ${params.file_path} (${editData.occurrences} replacements).`,
? `Created new file: ${this.params.file_path} with provided content.`
: `Successfully modified file: ${this.params.file_path} (${editData.occurrences} replacements).`,
];
if (params.modified_by_user) {
if (this.params.modified_by_user) {
llmSuccessMessageParts.push(
`User modified the \`new_string\` content to be: ${params.new_string}.`,
`User modified the \`new_string\` content to be: ${this.params.new_string}.`,
);
}
@@ -480,6 +411,94 @@ Expectation for required parameters:
fs.mkdirSync(dirName, { recursive: true });
}
}
}
/**
* Implementation of the Edit tool logic
*/
export class EditTool
extends BaseDeclarativeTool<EditToolParams, ToolResult>
implements ModifiableDeclarativeTool<EditToolParams>
{
static readonly Name = 'replace';
constructor(private readonly config: Config) {
super(
EditTool.Name,
'Edit',
`Replaces text within a file. By default, replaces a single occurrence, but can replace multiple occurrences when \`expected_replacements\` is specified. This tool requires providing significant context around the change to ensure precise targeting. Always use the ${ReadFileTool.Name} tool to examine the file's current content before attempting a text replacement.
The user has the ability to modify the \`new_string\` content. If modified, this will be stated in the response.
Expectation for required parameters:
1. \`file_path\` MUST be an absolute path; otherwise an error will be thrown.
2. \`old_string\` MUST be the exact literal text to replace (including all whitespace, indentation, newlines, and surrounding code etc.).
3. \`new_string\` MUST be the exact literal text to replace \`old_string\` with (also including all whitespace, indentation, newlines, and surrounding code etc.). Ensure the resulting code is correct and idiomatic.
4. NEVER escape \`old_string\` or \`new_string\`, that would break the exact literal text requirement.
**Important:** If ANY of the above are not satisfied, the tool will fail. CRITICAL for \`old_string\`: Must uniquely identify the single instance to change. Include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. If this string matches multiple locations, or does not match exactly, the tool will fail.
**Multiple replacements:** Set \`expected_replacements\` to the number of occurrences you want to replace. The tool will replace ALL occurrences that match \`old_string\` exactly. Ensure the number of replacements matches your expectation.`,
Icon.Pencil,
{
properties: {
file_path: {
description:
"The absolute path to the file to modify. Must start with '/'.",
type: 'string',
},
old_string: {
description:
'The exact literal text to replace, preferably unescaped. For single replacements (default), include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. For multiple replacements, specify expected_replacements parameter. If this string is not the exact literal text (i.e. you escaped it) or does not match exactly, the tool will fail.',
type: 'string',
},
new_string: {
description:
'The exact literal text to replace `old_string` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.',
type: 'string',
},
expected_replacements: {
type: 'number',
description:
'Number of replacements expected. Defaults to 1 if not specified. Use when you want to replace multiple occurrences.',
minimum: 1,
},
},
required: ['file_path', 'old_string', 'new_string'],
type: 'object',
},
);
}
/**
* Validates the parameters for the Edit tool
* @param params Parameters to validate
* @returns Error message string or null if valid
*/
validateToolParams(params: EditToolParams): string | null {
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}
if (!path.isAbsolute(params.file_path)) {
return `File path must be absolute: ${params.file_path}`;
}
const workspaceContext = this.config.getWorkspaceContext();
if (!workspaceContext.isPathWithinWorkspace(params.file_path)) {
const directories = workspaceContext.getDirectories();
return `File path must be within one of the workspace directories: ${directories.join(', ')}`;
}
return null;
}
protected createInvocation(
params: EditToolParams,
): ToolInvocation<EditToolParams, ToolResult> {
return new EditToolInvocation(this.config, params);
}
getModifyContext(_: AbortSignal): ModifyContext<EditToolParams> {
return {
@@ -495,7 +514,7 @@ Expectation for required parameters:
getProposedContent: async (params: EditToolParams): Promise<string> => {
try {
const currentContent = fs.readFileSync(params.file_path, 'utf8');
return this._applyReplacement(
return applyReplacement(
currentContent,
params.old_string,
params.new_string,
@@ -510,12 +529,16 @@ Expectation for required parameters:
oldContent: string,
modifiedProposedContent: string,
originalParams: EditToolParams,
): EditToolParams => ({
...originalParams,
old_string: oldContent,
new_string: modifiedProposedContent,
modified_by_user: true,
}),
): EditToolParams => {
const content = originalParams.new_string;
return {
...originalParams,
ai_proposed_string: content,
old_string: oldContent,
new_string: modifiedProposedContent,
modified_by_user: true,
};
},
};
}
}

View File

@@ -64,7 +64,8 @@ describe('GlobTool', () => {
describe('execute', () => {
it('should find files matching a simple pattern in the root', async () => {
const params: GlobToolParams = { pattern: '*.txt' };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 2 file(s)');
expect(result.llmContent).toContain(path.join(tempRootDir, 'fileA.txt'));
expect(result.llmContent).toContain(path.join(tempRootDir, 'FileB.TXT'));
@@ -73,7 +74,8 @@ describe('GlobTool', () => {
it('should find files case-sensitively when case_sensitive is true', async () => {
const params: GlobToolParams = { pattern: '*.txt', case_sensitive: true };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 1 file(s)');
expect(result.llmContent).toContain(path.join(tempRootDir, 'fileA.txt'));
expect(result.llmContent).not.toContain(
@@ -83,7 +85,8 @@ describe('GlobTool', () => {
it('should find files case-insensitively by default (pattern: *.TXT)', async () => {
const params: GlobToolParams = { pattern: '*.TXT' };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 2 file(s)');
expect(result.llmContent).toContain(path.join(tempRootDir, 'fileA.txt'));
expect(result.llmContent).toContain(path.join(tempRootDir, 'FileB.TXT'));
@@ -94,7 +97,8 @@ describe('GlobTool', () => {
pattern: '*.TXT',
case_sensitive: false,
};
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 2 file(s)');
expect(result.llmContent).toContain(path.join(tempRootDir, 'fileA.txt'));
expect(result.llmContent).toContain(path.join(tempRootDir, 'FileB.TXT'));
@@ -102,7 +106,8 @@ describe('GlobTool', () => {
it('should find files using a pattern that includes a subdirectory', async () => {
const params: GlobToolParams = { pattern: 'sub/*.md' };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 2 file(s)');
expect(result.llmContent).toContain(
path.join(tempRootDir, 'sub', 'fileC.md'),
@@ -114,7 +119,8 @@ describe('GlobTool', () => {
it('should find files in a specified relative path (relative to rootDir)', async () => {
const params: GlobToolParams = { pattern: '*.md', path: 'sub' };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 2 file(s)');
expect(result.llmContent).toContain(
path.join(tempRootDir, 'sub', 'fileC.md'),
@@ -126,7 +132,8 @@ describe('GlobTool', () => {
it('should find files using a deep globstar pattern (e.g., **/*.log)', async () => {
const params: GlobToolParams = { pattern: '**/*.log' };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 1 file(s)');
expect(result.llmContent).toContain(
path.join(tempRootDir, 'sub', 'deep', 'fileE.log'),
@@ -135,7 +142,8 @@ describe('GlobTool', () => {
it('should return "No files found" message when pattern matches nothing', async () => {
const params: GlobToolParams = { pattern: '*.nonexistent' };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'No files found matching pattern "*.nonexistent"',
);
@@ -144,7 +152,8 @@ describe('GlobTool', () => {
it('should correctly sort files by modification time (newest first)', async () => {
const params: GlobToolParams = { pattern: '*.sortme' };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
const llmContent = partListUnionToString(result.llmContent);
expect(llmContent).toContain('Found 2 file(s)');
@@ -242,8 +251,8 @@ describe('GlobTool', () => {
// Let's try to go further up.
const paramsOutside: GlobToolParams = {
pattern: '*.txt',
path: '../../../../../../../../../../tmp',
}; // Definitely outside
path: '../../../../../../../../../../tmp', // Definitely outside
};
expect(specificGlobTool.validateToolParams(paramsOutside)).toContain(
'resolves outside the allowed workspace directories',
);
@@ -290,7 +299,8 @@ describe('GlobTool', () => {
it('should work with paths in workspace subdirectories', async () => {
const params: GlobToolParams = { pattern: '*.md', path: 'sub' };
const result = await globTool.execute(params, abortSignal);
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 2 file(s)');
expect(result.llmContent).toContain('fileC.md');

View File

@@ -8,8 +8,13 @@ import fs from 'fs';
import path from 'path';
import { glob } from 'glob';
import { SchemaValidator } from '../utils/schemaValidator.js';
import { BaseTool, Icon, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import {
BaseDeclarativeTool,
BaseToolInvocation,
Icon,
ToolInvocation,
ToolResult,
} from './tools.js';
import { shortenPath, makeRelative } from '../utils/paths.js';
import { Config } from '../config/config.js';
@@ -74,10 +79,168 @@ export interface GlobToolParams {
respect_git_ignore?: boolean;
}
class GlobToolInvocation extends BaseToolInvocation<
GlobToolParams,
ToolResult
> {
constructor(
private config: Config,
params: GlobToolParams,
) {
super(params);
}
getDescription(): string {
let description = `'${this.params.pattern}'`;
if (this.params.path) {
const searchDir = path.resolve(
this.config.getTargetDir(),
this.params.path || '.',
);
const relativePath = makeRelative(searchDir, this.config.getTargetDir());
description += ` within ${shortenPath(relativePath)}`;
}
return description;
}
async execute(signal: AbortSignal): Promise<ToolResult> {
try {
const workspaceContext = this.config.getWorkspaceContext();
const workspaceDirectories = workspaceContext.getDirectories();
// If a specific path is provided, resolve it and check if it's within workspace
let searchDirectories: readonly string[];
if (this.params.path) {
const searchDirAbsolute = path.resolve(
this.config.getTargetDir(),
this.params.path,
);
if (!workspaceContext.isPathWithinWorkspace(searchDirAbsolute)) {
return {
llmContent: `Error: Path "${this.params.path}" is not within any workspace directory`,
returnDisplay: `Path is not within workspace`,
};
}
searchDirectories = [searchDirAbsolute];
} else {
// Search across all workspace directories
searchDirectories = workspaceDirectories;
}
// Get centralized file discovery service
const respectGitIgnore =
this.params.respect_git_ignore ??
this.config.getFileFilteringRespectGitIgnore();
const fileDiscovery = this.config.getFileService();
// Collect entries from all search directories
let allEntries: GlobPath[] = [];
for (const searchDir of searchDirectories) {
const entries = (await glob(this.params.pattern, {
cwd: searchDir,
withFileTypes: true,
nodir: true,
stat: true,
nocase: !this.params.case_sensitive,
dot: true,
ignore: ['**/node_modules/**', '**/.git/**'],
follow: false,
signal,
})) as GlobPath[];
allEntries = allEntries.concat(entries);
}
const entries = allEntries;
// Apply git-aware filtering if enabled and in git repository
let filteredEntries = entries;
let gitIgnoredCount = 0;
if (respectGitIgnore) {
const relativePaths = entries.map((p) =>
path.relative(this.config.getTargetDir(), p.fullpath()),
);
const filteredRelativePaths = fileDiscovery.filterFiles(relativePaths, {
respectGitIgnore,
});
const filteredAbsolutePaths = new Set(
filteredRelativePaths.map((p) =>
path.resolve(this.config.getTargetDir(), p),
),
);
filteredEntries = entries.filter((entry) =>
filteredAbsolutePaths.has(entry.fullpath()),
);
gitIgnoredCount = entries.length - filteredEntries.length;
}
if (!filteredEntries || filteredEntries.length === 0) {
let message = `No files found matching pattern "${this.params.pattern}"`;
if (searchDirectories.length === 1) {
message += ` within ${searchDirectories[0]}`;
} else {
message += ` within ${searchDirectories.length} workspace directories`;
}
if (gitIgnoredCount > 0) {
message += ` (${gitIgnoredCount} files were git-ignored)`;
}
return {
llmContent: message,
returnDisplay: `No files found`,
};
}
// Set filtering such that we first show the most recent files
const oneDayInMs = 24 * 60 * 60 * 1000;
const nowTimestamp = new Date().getTime();
// Sort the filtered entries using the new helper function
const sortedEntries = sortFileEntries(
filteredEntries,
nowTimestamp,
oneDayInMs,
);
const sortedAbsolutePaths = sortedEntries.map((entry) =>
entry.fullpath(),
);
const fileListDescription = sortedAbsolutePaths.join('\n');
const fileCount = sortedAbsolutePaths.length;
let resultMessage = `Found ${fileCount} file(s) matching "${this.params.pattern}"`;
if (searchDirectories.length === 1) {
resultMessage += ` within ${searchDirectories[0]}`;
} else {
resultMessage += ` across ${searchDirectories.length} workspace directories`;
}
if (gitIgnoredCount > 0) {
resultMessage += ` (${gitIgnoredCount} additional files were git-ignored)`;
}
resultMessage += `, sorted by modification time (newest first):\n${fileListDescription}`;
return {
llmContent: resultMessage,
returnDisplay: `Found ${fileCount} matching file(s)`,
};
} catch (error) {
const errorMessage =
error instanceof Error ? error.message : String(error);
console.error(`GlobLogic execute Error: ${errorMessage}`, error);
return {
llmContent: `Error during glob search operation: ${errorMessage}`,
returnDisplay: `Error: An unexpected error occurred.`,
};
}
}
}
/**
* Implementation of the Glob tool logic
*/
export class GlobTool extends BaseTool<GlobToolParams, ToolResult> {
export class GlobTool extends BaseDeclarativeTool<GlobToolParams, ToolResult> {
static readonly Name = 'glob';
constructor(private config: Config) {
@@ -91,26 +254,26 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> {
pattern: {
description:
"The glob pattern to match against (e.g., '**/*.py', 'docs/*.md').",
type: Type.STRING,
type: 'string',
},
path: {
description:
'Optional: The absolute path to the directory to search within. If omitted, searches the root directory.',
type: Type.STRING,
type: 'string',
},
case_sensitive: {
description:
'Optional: Whether the search should be case-sensitive. Defaults to false.',
type: Type.BOOLEAN,
type: 'boolean',
},
respect_git_ignore: {
description:
'Optional: Whether to respect .gitignore patterns when finding files. Only available in git repositories. Defaults to true.',
type: Type.BOOLEAN,
type: 'boolean',
},
},
required: ['pattern'],
type: Type.OBJECT,
type: 'object',
},
);
}
@@ -119,7 +282,10 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> {
* Validates the parameters for the tool.
*/
validateToolParams(params: GlobToolParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}
@@ -158,166 +324,9 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> {
return null;
}
/**
* Gets a description of the glob operation.
*/
getDescription(params: GlobToolParams): string {
let description = `'${params.pattern}'`;
if (params.path) {
const searchDir = path.resolve(
this.config.getTargetDir(),
params.path || '.',
);
const relativePath = makeRelative(searchDir, this.config.getTargetDir());
description += ` within ${shortenPath(relativePath)}`;
}
return description;
}
/**
* Executes the glob search with the given parameters
*/
async execute(
protected createInvocation(
params: GlobToolParams,
signal: AbortSignal,
): Promise<ToolResult> {
const validationError = this.validateToolParams(params);
if (validationError) {
return {
llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`,
returnDisplay: validationError,
};
}
try {
const workspaceContext = this.config.getWorkspaceContext();
const workspaceDirectories = workspaceContext.getDirectories();
// If a specific path is provided, resolve it and check if it's within workspace
let searchDirectories: readonly string[];
if (params.path) {
const searchDirAbsolute = path.resolve(
this.config.getTargetDir(),
params.path,
);
if (!workspaceContext.isPathWithinWorkspace(searchDirAbsolute)) {
return {
llmContent: `Error: Path "${params.path}" is not within any workspace directory`,
returnDisplay: `Path is not within workspace`,
};
}
searchDirectories = [searchDirAbsolute];
} else {
// Search across all workspace directories
searchDirectories = workspaceDirectories;
}
// Get centralized file discovery service
const respectGitIgnore =
params.respect_git_ignore ??
this.config.getFileFilteringRespectGitIgnore();
const fileDiscovery = this.config.getFileService();
// Collect entries from all search directories
let allEntries: GlobPath[] = [];
for (const searchDir of searchDirectories) {
const entries = (await glob(params.pattern, {
cwd: searchDir,
withFileTypes: true,
nodir: true,
stat: true,
nocase: !params.case_sensitive,
dot: true,
ignore: ['**/node_modules/**', '**/.git/**'],
follow: false,
signal,
})) as GlobPath[];
allEntries = allEntries.concat(entries);
}
const entries = allEntries;
// Apply git-aware filtering if enabled and in git repository
let filteredEntries = entries;
let gitIgnoredCount = 0;
if (respectGitIgnore) {
const relativePaths = entries.map((p) =>
path.relative(this.config.getTargetDir(), p.fullpath()),
);
const filteredRelativePaths = fileDiscovery.filterFiles(relativePaths, {
respectGitIgnore,
});
const filteredAbsolutePaths = new Set(
filteredRelativePaths.map((p) =>
path.resolve(this.config.getTargetDir(), p),
),
);
filteredEntries = entries.filter((entry) =>
filteredAbsolutePaths.has(entry.fullpath()),
);
gitIgnoredCount = entries.length - filteredEntries.length;
}
if (!filteredEntries || filteredEntries.length === 0) {
let message = `No files found matching pattern "${params.pattern}"`;
if (searchDirectories.length === 1) {
message += ` within ${searchDirectories[0]}`;
} else {
message += ` within ${searchDirectories.length} workspace directories`;
}
if (gitIgnoredCount > 0) {
message += ` (${gitIgnoredCount} files were git-ignored)`;
}
return {
llmContent: message,
returnDisplay: `No files found`,
};
}
// Set filtering such that we first show the most recent files
const oneDayInMs = 24 * 60 * 60 * 1000;
const nowTimestamp = new Date().getTime();
// Sort the filtered entries using the new helper function
const sortedEntries = sortFileEntries(
filteredEntries,
nowTimestamp,
oneDayInMs,
);
const sortedAbsolutePaths = sortedEntries.map((entry) =>
entry.fullpath(),
);
const fileListDescription = sortedAbsolutePaths.join('\n');
const fileCount = sortedAbsolutePaths.length;
let resultMessage = `Found ${fileCount} file(s) matching "${params.pattern}"`;
if (searchDirectories.length === 1) {
resultMessage += ` within ${searchDirectories[0]}`;
} else {
resultMessage += ` across ${searchDirectories.length} workspace directories`;
}
if (gitIgnoredCount > 0) {
resultMessage += ` (${gitIgnoredCount} additional files were git-ignored)`;
}
resultMessage += `, sorted by modification time (newest first):\n${fileListDescription}`;
return {
llmContent: resultMessage,
returnDisplay: `Found ${fileCount} matching file(s)`,
};
} catch (error) {
const errorMessage =
error instanceof Error ? error.message : String(error);
console.error(`GlobLogic execute Error: ${errorMessage}`, error);
return {
llmContent: `Error during glob search operation: ${errorMessage}`,
returnDisplay: `Error: An unexpected error occurred.`,
};
}
): ToolInvocation<GlobToolParams, ToolResult> {
return new GlobToolInvocation(this.config, params);
}
}

View File

@@ -126,7 +126,8 @@ describe('GrepTool', () => {
describe('execute', () => {
it('should find matches for a simple pattern in all files', async () => {
const params: GrepToolParams = { pattern: 'world' };
const result = await grepTool.execute(params, abortSignal);
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'Found 3 matches for pattern "world" in the workspace directory',
);
@@ -142,7 +143,8 @@ describe('GrepTool', () => {
it('should find matches in a specific path', async () => {
const params: GrepToolParams = { pattern: 'world', path: 'sub' };
const result = await grepTool.execute(params, abortSignal);
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'Found 1 match for pattern "world" in path "sub"',
);
@@ -153,7 +155,8 @@ describe('GrepTool', () => {
it('should find matches with an include glob', async () => {
const params: GrepToolParams = { pattern: 'hello', include: '*.js' };
const result = await grepTool.execute(params, abortSignal);
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'Found 1 match for pattern "hello" in the workspace directory (filter: "*.js"):',
);
@@ -174,7 +177,8 @@ describe('GrepTool', () => {
path: 'sub',
include: '*.js',
};
const result = await grepTool.execute(params, abortSignal);
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'Found 1 match for pattern "hello" in path "sub" (filter: "*.js")',
);
@@ -185,7 +189,8 @@ describe('GrepTool', () => {
it('should return "No matches found" when pattern does not exist', async () => {
const params: GrepToolParams = { pattern: 'nonexistentpattern' };
const result = await grepTool.execute(params, abortSignal);
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'No matches found for pattern "nonexistentpattern" in the workspace directory.',
);
@@ -194,7 +199,8 @@ describe('GrepTool', () => {
it('should handle regex special characters correctly', async () => {
const params: GrepToolParams = { pattern: 'foo.*bar' }; // Matches 'const foo = "bar";'
const result = await grepTool.execute(params, abortSignal);
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'Found 1 match for pattern "foo.*bar" in the workspace directory:',
);
@@ -204,7 +210,8 @@ describe('GrepTool', () => {
it('should be case-insensitive by default (JS fallback)', async () => {
const params: GrepToolParams = { pattern: 'HELLO' };
const result = await grepTool.execute(params, abortSignal);
const invocation = grepTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
'Found 2 matches for pattern "HELLO" in the workspace directory:',
);
@@ -216,14 +223,10 @@ describe('GrepTool', () => {
);
});
it('should return an error if params are invalid', async () => {
it('should throw an error if params are invalid', async () => {
const params = { path: '.' } as unknown as GrepToolParams; // Invalid: pattern missing
const result = await grepTool.execute(params, abortSignal);
expect(result.llmContent).toBe(
"Error: Invalid parameters provided. Reason: params must have required property 'pattern'",
);
expect(result.returnDisplay).toBe(
"Model provided invalid parameters. Error: params must have required property 'pattern'",
expect(() => grepTool.build(params)).toThrow(
/params must have required property 'pattern'/,
);
});
@@ -242,7 +245,8 @@ describe('GrepTool', () => {
// Search for 'world' which exists in both the regular file and the ignored file
const params: GrepToolParams = { pattern: 'world' };
const result = await grepToolWithIgnore.execute(params, abortSignal);
const invocation = grepToolWithIgnore.build(params);
const result = await invocation.execute(abortSignal);
// Should only find matches in the non-ignored files (3 matches)
expect(result.llmContent).toContain(
@@ -294,7 +298,8 @@ describe('GrepTool', () => {
const multiDirGrepTool = new GrepTool(multiDirConfig);
const params: GrepToolParams = { pattern: 'world' };
const result = await multiDirGrepTool.execute(params, abortSignal);
const invocation = multiDirGrepTool.build(params);
const result = await invocation.execute(abortSignal);
// Should find matches in both directories
expect(result.llmContent).toContain(
@@ -350,7 +355,8 @@ describe('GrepTool', () => {
// Search only in the 'sub' directory of the first workspace
const params: GrepToolParams = { pattern: 'world', path: 'sub' };
const result = await multiDirGrepTool.execute(params, abortSignal);
const invocation = multiDirGrepTool.build(params);
const result = await invocation.execute(abortSignal);
// Should only find matches in the specified sub directory
expect(result.llmContent).toContain(
@@ -370,7 +376,8 @@ describe('GrepTool', () => {
describe('getDescription', () => {
it('should generate correct description with pattern only', () => {
const params: GrepToolParams = { pattern: 'testPattern' };
expect(grepTool.getDescription(params)).toBe("'testPattern'");
const invocation = grepTool.build(params);
expect(invocation.getDescription()).toBe("'testPattern'");
});
it('should generate correct description with pattern and include', () => {
@@ -378,19 +385,21 @@ describe('GrepTool', () => {
pattern: 'testPattern',
include: '*.ts',
};
expect(grepTool.getDescription(params)).toBe("'testPattern' in *.ts");
const invocation = grepTool.build(params);
expect(invocation.getDescription()).toBe("'testPattern' in *.ts");
});
it('should generate correct description with pattern and path', () => {
it('should generate correct description with pattern and path', async () => {
const dirPath = path.join(tempRootDir, 'src', 'app');
await fs.mkdir(dirPath, { recursive: true });
const params: GrepToolParams = {
pattern: 'testPattern',
path: path.join('src', 'app'),
};
const invocation = grepTool.build(params);
// The path will be relative to the tempRootDir, so we check for containment.
expect(grepTool.getDescription(params)).toContain("'testPattern' within");
expect(grepTool.getDescription(params)).toContain(
path.join('src', 'app'),
);
expect(invocation.getDescription()).toContain("'testPattern' within");
expect(invocation.getDescription()).toContain(path.join('src', 'app'));
});
it('should indicate searching across all workspace directories when no path specified', () => {
@@ -403,28 +412,31 @@ describe('GrepTool', () => {
const multiDirGrepTool = new GrepTool(multiDirConfig);
const params: GrepToolParams = { pattern: 'testPattern' };
expect(multiDirGrepTool.getDescription(params)).toBe(
const invocation = multiDirGrepTool.build(params);
expect(invocation.getDescription()).toBe(
"'testPattern' across all workspace directories",
);
});
it('should generate correct description with pattern, include, and path', () => {
it('should generate correct description with pattern, include, and path', async () => {
const dirPath = path.join(tempRootDir, 'src', 'app');
await fs.mkdir(dirPath, { recursive: true });
const params: GrepToolParams = {
pattern: 'testPattern',
include: '*.ts',
path: path.join('src', 'app'),
};
expect(grepTool.getDescription(params)).toContain(
const invocation = grepTool.build(params);
expect(invocation.getDescription()).toContain(
"'testPattern' in *.ts within",
);
expect(grepTool.getDescription(params)).toContain(
path.join('src', 'app'),
);
expect(invocation.getDescription()).toContain(path.join('src', 'app'));
});
it('should use ./ for root path in description', () => {
const params: GrepToolParams = { pattern: 'testPattern', path: '.' };
expect(grepTool.getDescription(params)).toBe("'testPattern' within ./");
const invocation = grepTool.build(params);
expect(invocation.getDescription()).toBe("'testPattern' within ./");
});
});
});

View File

@@ -9,9 +9,14 @@ import fsPromises from 'fs/promises';
import path from 'path';
import { EOL } from 'os';
import { spawn } from 'child_process';
import { globIterate } from 'glob';
import { BaseTool, Icon, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import { globStream } from 'glob';
import {
BaseDeclarativeTool,
BaseToolInvocation,
Icon,
ToolInvocation,
ToolResult,
} from './tools.js';
import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js';
@@ -49,46 +54,17 @@ interface GrepMatch {
line: string;
}
// --- GrepLogic Class ---
/**
* Implementation of the Grep tool logic (moved from CLI)
*/
export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
static readonly Name = 'search_file_content'; // Keep static name
constructor(private readonly config: Config) {
super(
GrepTool.Name,
'SearchText',
'Searches for a regular expression pattern within the content of files in a specified directory (or current working directory). Can filter files by a glob pattern. Returns the lines containing matches, along with their file paths and line numbers.',
Icon.Regex,
{
properties: {
pattern: {
description:
"The regular expression (regex) pattern to search for within file contents (e.g., 'function\\s+myFunction', 'import\\s+\\{.*\\}\\s+from\\s+.*').",
type: Type.STRING,
},
path: {
description:
'Optional: The absolute path to the directory to search within. If omitted, searches the current working directory.',
type: Type.STRING,
},
include: {
description:
"Optional: A glob pattern to filter which files are searched (e.g., '*.js', '*.{ts,tsx}', 'src/**'). If omitted, searches all files (respecting potential global ignores).",
type: Type.STRING,
},
},
required: ['pattern'],
type: Type.OBJECT,
},
);
class GrepToolInvocation extends BaseToolInvocation<
GrepToolParams,
ToolResult
> {
constructor(
private readonly config: Config,
params: GrepToolParams,
) {
super(params);
}
// --- Validation Methods ---
/**
* Checks if a path is within the root directory and resolves it.
* @param relativePath Path relative to the root directory (or undefined for root).
@@ -130,58 +106,11 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
return targetPath;
}
/**
* Validates the parameters for the tool
* @param params Parameters to validate
* @returns An error message string if invalid, null otherwise
*/
validateToolParams(params: GrepToolParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
if (errors) {
return errors;
}
try {
new RegExp(params.pattern);
} catch (error) {
return `Invalid regular expression pattern provided: ${params.pattern}. Error: ${getErrorMessage(error)}`;
}
// Only validate path if one is provided
if (params.path) {
try {
this.resolveAndValidatePath(params.path);
} catch (error) {
return getErrorMessage(error);
}
}
return null; // Parameters are valid
}
// --- Core Execution ---
/**
* Executes the grep search with the given parameters
* @param params Parameters for the grep search
* @returns Result of the grep search
*/
async execute(
params: GrepToolParams,
signal: AbortSignal,
): Promise<ToolResult> {
const validationError = this.validateToolParams(params);
if (validationError) {
return {
llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`,
returnDisplay: `Model provided invalid parameters. Error: ${validationError}`,
};
}
async execute(signal: AbortSignal): Promise<ToolResult> {
try {
const workspaceContext = this.config.getWorkspaceContext();
const searchDirAbs = this.resolveAndValidatePath(params.path);
const searchDirDisplay = params.path || '.';
const searchDirAbs = this.resolveAndValidatePath(this.params.path);
const searchDirDisplay = this.params.path || '.';
// Determine which directories to search
let searchDirectories: readonly string[];
@@ -197,9 +126,9 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
let allMatches: GrepMatch[] = [];
for (const searchDir of searchDirectories) {
const matches = await this.performGrepSearch({
pattern: params.pattern,
pattern: this.params.pattern,
path: searchDir,
include: params.include,
include: this.params.include,
signal,
});
@@ -226,7 +155,7 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
}
if (allMatches.length === 0) {
const noMatchMsg = `No matches found for pattern "${params.pattern}" ${searchLocationDescription}${params.include ? ` (filter: "${params.include}")` : ''}.`;
const noMatchMsg = `No matches found for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}.`;
return { llmContent: noMatchMsg, returnDisplay: `No matches found` };
}
@@ -247,7 +176,7 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
const matchCount = allMatches.length;
const matchTerm = matchCount === 1 ? 'match' : 'matches';
let llmContent = `Found ${matchCount} ${matchTerm} for pattern "${params.pattern}" ${searchLocationDescription}${params.include ? ` (filter: "${params.include}")` : ''}:
let llmContent = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}:
---
`;
@@ -274,8 +203,6 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
}
}
// --- Grep Implementation Logic ---
/**
* Checks if a command is available in the system's PATH.
* @param {string} command The command name (e.g., 'git', 'grep').
@@ -353,17 +280,20 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
* @param params Parameters for the grep operation
* @returns A string describing the grep
*/
getDescription(params: GrepToolParams): string {
let description = `'${params.pattern}'`;
if (params.include) {
description += ` in ${params.include}`;
getDescription(): string {
let description = `'${this.params.pattern}'`;
if (this.params.include) {
description += ` in ${this.params.include}`;
}
if (params.path) {
if (this.params.path) {
const resolvedPath = path.resolve(
this.config.getTargetDir(),
params.path,
this.params.path,
);
if (resolvedPath === this.config.getTargetDir() || params.path === '.') {
if (
resolvedPath === this.config.getTargetDir() ||
this.params.path === '.'
) {
description += ` within ./`;
} else {
const relativePath = makeRelative(
@@ -445,7 +375,9 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
return this.parseGrepOutput(output, absolutePath);
} catch (gitError: unknown) {
console.debug(
`GrepLogic: git grep failed: ${getErrorMessage(gitError)}. Falling back...`,
`GrepLogic: git grep failed: ${getErrorMessage(
gitError,
)}. Falling back...`,
);
}
}
@@ -525,7 +457,9 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
return this.parseGrepOutput(output, absolutePath);
} catch (grepError: unknown) {
console.debug(
`GrepLogic: System grep failed: ${getErrorMessage(grepError)}. Falling back...`,
`GrepLogic: System grep failed: ${getErrorMessage(
grepError,
)}. Falling back...`,
);
}
}
@@ -550,7 +484,7 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
...fileDiscovery.getGeminiIgnorePatterns(),
]; // Use glob patterns for ignores here
const filesIterator = globIterate(globPattern, {
const filesIterator = globStream(globPattern, {
cwd: absolutePath,
dot: true,
ignore: ignorePatterns,
@@ -582,7 +516,9 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
// Ignore errors like permission denied or file gone during read
if (!isNodeError(readError) || readError.code !== 'ENOENT') {
console.debug(
`GrepLogic: Could not read/process ${fileAbsolutePath}: ${getErrorMessage(readError)}`,
`GrepLogic: Could not read/process ${fileAbsolutePath}: ${getErrorMessage(
readError,
)}`,
);
}
}
@@ -591,9 +527,129 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
return allMatches;
} catch (error: unknown) {
console.error(
`GrepLogic: Error in performGrepSearch (Strategy: ${strategyUsed}): ${getErrorMessage(error)}`,
`GrepLogic: Error in performGrepSearch (Strategy: ${strategyUsed}): ${getErrorMessage(
error,
)}`,
);
throw error; // Re-throw
}
}
}
// --- GrepLogic Class ---
/**
* Implementation of the Grep tool logic (moved from CLI)
*/
export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
static readonly Name = 'search_file_content'; // Keep static name
constructor(private readonly config: Config) {
super(
GrepTool.Name,
'SearchText',
'Searches for a regular expression pattern within the content of files in a specified directory (or current working directory). Can filter files by a glob pattern. Returns the lines containing matches, along with their file paths and line numbers.',
Icon.Regex,
{
properties: {
pattern: {
description:
"The regular expression (regex) pattern to search for within file contents (e.g., 'function\\s+myFunction', 'import\\s+\\{.*\\}\\s+from\\s+.*').",
type: 'string',
},
path: {
description:
'Optional: The absolute path to the directory to search within. If omitted, searches the current working directory.',
type: 'string',
},
include: {
description:
"Optional: A glob pattern to filter which files are searched (e.g., '*.js', '*.{ts,tsx}', 'src/**'). If omitted, searches all files (respecting potential global ignores).",
type: 'string',
},
},
required: ['pattern'],
type: 'object',
},
);
}
/**
* Checks if a path is within the root directory and resolves it.
* @param relativePath Path relative to the root directory (or undefined for root).
* @returns The absolute path if valid and exists, or null if no path specified (to search all directories).
* @throws {Error} If path is outside root, doesn't exist, or isn't a directory.
*/
private resolveAndValidatePath(relativePath?: string): string | null {
// If no path specified, return null to indicate searching all workspace directories
if (!relativePath) {
return null;
}
const targetPath = path.resolve(this.config.getTargetDir(), relativePath);
// Security Check: Ensure the resolved path is within workspace boundaries
const workspaceContext = this.config.getWorkspaceContext();
if (!workspaceContext.isPathWithinWorkspace(targetPath)) {
const directories = workspaceContext.getDirectories();
throw new Error(
`Path validation failed: Attempted path "${relativePath}" resolves outside the allowed workspace directories: ${directories.join(', ')}`,
);
}
// Check existence and type after resolving
try {
const stats = fs.statSync(targetPath);
if (!stats.isDirectory()) {
throw new Error(`Path is not a directory: ${targetPath}`);
}
} catch (error: unknown) {
if (isNodeError(error) && error.code !== 'ENOENT') {
throw new Error(`Path does not exist: ${targetPath}`);
}
throw new Error(
`Failed to access path stats for ${targetPath}: ${error}`,
);
}
return targetPath;
}
/**
* Validates the parameters for the tool
* @param params Parameters to validate
* @returns An error message string if invalid, null otherwise
*/
validateToolParams(params: GrepToolParams): string | null {
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}
try {
new RegExp(params.pattern);
} catch (error) {
return `Invalid regular expression pattern provided: ${params.pattern}. Error: ${getErrorMessage(error)}`;
}
// Only validate path if one is provided
if (params.path) {
try {
this.resolveAndValidatePath(params.path);
} catch (error) {
return getErrorMessage(error);
}
}
return null; // Parameters are valid
}
protected createInvocation(
params: GrepToolParams,
): ToolInvocation<GrepToolParams, ToolResult> {
return new GrepToolInvocation(this.config, params);
}
}

View File

@@ -7,7 +7,6 @@
import fs from 'fs';
import path from 'path';
import { BaseTool, Icon, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { Config, DEFAULT_FILE_FILTERING_OPTIONS } from '../config/config.js';
@@ -82,35 +81,35 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> {
path: {
description:
'The absolute path to the directory to list (must be absolute, not relative)',
type: Type.STRING,
type: 'string',
},
ignore: {
description: 'List of glob patterns to ignore',
items: {
type: Type.STRING,
type: 'string',
},
type: Type.ARRAY,
type: 'array',
},
file_filtering_options: {
description:
'Optional: Whether to respect ignore patterns from .gitignore or .geminiignore',
type: Type.OBJECT,
type: 'object',
properties: {
respect_git_ignore: {
description:
'Optional: Whether to respect .gitignore patterns when listing files. Only available in git repositories. Defaults to true.',
type: Type.BOOLEAN,
type: 'boolean',
},
respect_gemini_ignore: {
description:
'Optional: Whether to respect .geminiignore patterns when listing files. Defaults to true.',
type: Type.BOOLEAN,
type: 'boolean',
},
},
},
},
required: ['path'],
type: Type.OBJECT,
type: 'object',
},
);
}
@@ -121,7 +120,10 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> {
* @returns An error message string if invalid, null otherwise
*/
validateToolParams(params: LSToolParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}

View File

@@ -12,6 +12,8 @@ import {
isEnabled,
discoverTools,
discoverPrompts,
hasValidTypes,
connectToMcpServer,
} from './mcp-client.js';
import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
import * as SdkClientStdioLib from '@modelcontextprotocol/sdk/client/stdio.js';
@@ -22,6 +24,8 @@ import { AuthProviderType } from '../config/config.js';
import { PromptRegistry } from '../prompts/prompt-registry.js';
import { DiscoveredMCPTool } from './mcp-tool.js';
import { WorkspaceContext } from '../utils/workspaceContext.js';
import { pathToFileURL } from 'node:url';
vi.mock('@modelcontextprotocol/sdk/client/stdio.js');
vi.mock('@modelcontextprotocol/sdk/client/index.js');
@@ -97,6 +101,232 @@ describe('mcp-client', () => {
`Error discovering tool: 'invalid tool name' from MCP server 'test-server': ${testError.message}`,
);
});
it('should skip tools if a parameter is missing a type', async () => {
const mockedClient = {} as unknown as ClientLib.Client;
const consoleWarnSpy = vi
.spyOn(console, 'warn')
.mockImplementation(() => {});
vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
tool: () =>
Promise.resolve({
functionDeclarations: [
{
name: 'validTool',
parametersJsonSchema: {
type: 'object',
properties: {
param1: { type: 'string' },
},
},
},
{
name: 'invalidTool',
parametersJsonSchema: {
type: 'object',
properties: {
param1: { description: 'a param with no type' },
},
},
},
],
}),
} as unknown as GenAiLib.CallableTool);
const tools = await discoverTools('test-server', {}, mockedClient);
expect(tools.length).toBe(1);
expect(vi.mocked(DiscoveredMCPTool).mock.calls[0][2]).toBe('validTool');
expect(consoleWarnSpy).toHaveBeenCalledOnce();
expect(consoleWarnSpy).toHaveBeenCalledWith(
`Skipping tool 'invalidTool' from MCP server 'test-server' because it has ` +
`missing types in its parameter schema. Please file an issue with the owner of the MCP server.`,
);
consoleWarnSpy.mockRestore();
});
it('should skip tools if a nested parameter is missing a type', async () => {
const mockedClient = {} as unknown as ClientLib.Client;
const consoleWarnSpy = vi
.spyOn(console, 'warn')
.mockImplementation(() => {});
vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
tool: () =>
Promise.resolve({
functionDeclarations: [
{
name: 'invalidTool',
parametersJsonSchema: {
type: 'object',
properties: {
param1: {
type: 'object',
properties: {
nestedParam: {
description: 'a nested param with no type',
},
},
},
},
},
},
],
}),
} as unknown as GenAiLib.CallableTool);
const tools = await discoverTools('test-server', {}, mockedClient);
expect(tools.length).toBe(0);
expect(consoleWarnSpy).toHaveBeenCalledOnce();
expect(consoleWarnSpy).toHaveBeenCalledWith(
`Skipping tool 'invalidTool' from MCP server 'test-server' because it has ` +
`missing types in its parameter schema. Please file an issue with the owner of the MCP server.`,
);
consoleWarnSpy.mockRestore();
});
it('should skip tool if an array item is missing a type', async () => {
const mockedClient = {} as unknown as ClientLib.Client;
const consoleWarnSpy = vi
.spyOn(console, 'warn')
.mockImplementation(() => {});
vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
tool: () =>
Promise.resolve({
functionDeclarations: [
{
name: 'invalidTool',
parametersJsonSchema: {
type: 'object',
properties: {
param1: {
type: 'array',
items: {
description: 'an array item with no type',
},
},
},
},
},
],
}),
} as unknown as GenAiLib.CallableTool);
const tools = await discoverTools('test-server', {}, mockedClient);
expect(tools.length).toBe(0);
expect(consoleWarnSpy).toHaveBeenCalledOnce();
expect(consoleWarnSpy).toHaveBeenCalledWith(
`Skipping tool 'invalidTool' from MCP server 'test-server' because it has ` +
`missing types in its parameter schema. Please file an issue with the owner of the MCP server.`,
);
consoleWarnSpy.mockRestore();
});
it('should discover tool with no properties in schema', async () => {
const mockedClient = {} as unknown as ClientLib.Client;
const consoleWarnSpy = vi
.spyOn(console, 'warn')
.mockImplementation(() => {});
vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
tool: () =>
Promise.resolve({
functionDeclarations: [
{
name: 'validTool',
parametersJsonSchema: {
type: 'object',
},
},
],
}),
} as unknown as GenAiLib.CallableTool);
const tools = await discoverTools('test-server', {}, mockedClient);
expect(tools.length).toBe(1);
expect(vi.mocked(DiscoveredMCPTool).mock.calls[0][2]).toBe('validTool');
expect(consoleWarnSpy).not.toHaveBeenCalled();
consoleWarnSpy.mockRestore();
});
it('should discover tool with empty properties object in schema', async () => {
const mockedClient = {} as unknown as ClientLib.Client;
const consoleWarnSpy = vi
.spyOn(console, 'warn')
.mockImplementation(() => {});
vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
tool: () =>
Promise.resolve({
functionDeclarations: [
{
name: 'validTool',
parametersJsonSchema: {
type: 'object',
properties: {},
},
},
],
}),
} as unknown as GenAiLib.CallableTool);
const tools = await discoverTools('test-server', {}, mockedClient);
expect(tools.length).toBe(1);
expect(vi.mocked(DiscoveredMCPTool).mock.calls[0][2]).toBe('validTool');
expect(consoleWarnSpy).not.toHaveBeenCalled();
consoleWarnSpy.mockRestore();
});
});
describe('connectToMcpServer', () => {
it('should register a roots/list handler', async () => {
const mockedClient = {
registerCapabilities: vi.fn(),
setRequestHandler: vi.fn(),
callTool: vi.fn(),
connect: vi.fn(),
};
vi.mocked(ClientLib.Client).mockReturnValue(
mockedClient as unknown as ClientLib.Client,
);
vi.spyOn(SdkClientStdioLib, 'StdioClientTransport').mockReturnValue(
{} as SdkClientStdioLib.StdioClientTransport,
);
const mockWorkspaceContext = {
getDirectories: vi
.fn()
.mockReturnValue(['/test/dir', '/another/project']),
} as unknown as WorkspaceContext;
await connectToMcpServer(
'test-server',
{
command: 'test-command',
},
false,
mockWorkspaceContext,
);
expect(mockedClient.registerCapabilities).toHaveBeenCalledWith({
roots: {},
});
expect(mockedClient.setRequestHandler).toHaveBeenCalledOnce();
const handler = mockedClient.setRequestHandler.mock.calls[0][1];
const roots = await handler();
expect(roots).toEqual({
roots: [
{
uri: pathToFileURL('/test/dir').toString(),
name: 'dir',
},
{
uri: pathToFileURL('/another/project').toString(),
name: 'project',
},
],
});
});
});
describe('discoverPrompts', () => {
@@ -309,7 +539,9 @@ describe('mcp-client', () => {
});
it('should connect via command', async () => {
const mockedTransport = vi.mocked(SdkClientStdioLib.StdioClientTransport);
const mockedTransport = vi
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
.mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
await createTransport(
'test-server',
@@ -336,7 +568,7 @@ describe('mcp-client', () => {
const transport = await createTransport(
'test-server',
{
httpUrl: 'http://test-server',
httpUrl: 'http://test.googleapis.com',
authProviderType: AuthProviderType.GOOGLE_CREDENTIALS,
oauth: {
scopes: ['scope1'],
@@ -355,7 +587,7 @@ describe('mcp-client', () => {
const transport = await createTransport(
'test-server',
{
url: 'http://test-server',
url: 'http://test.googleapis.com',
authProviderType: AuthProviderType.GOOGLE_CREDENTIALS,
oauth: {
scopes: ['scope1'],
@@ -383,7 +615,7 @@ describe('mcp-client', () => {
false,
),
).rejects.toThrow(
'No URL configured for Google Credentials MCP server',
'URL must be provided in the config for Google Credentials provider',
);
});
});
@@ -433,4 +665,163 @@ describe('mcp-client', () => {
);
});
});
describe('hasValidTypes', () => {
it('should return true for a valid schema with anyOf', () => {
const schema = {
anyOf: [{ type: 'string' }, { type: 'number' }],
};
expect(hasValidTypes(schema)).toBe(true);
});
it('should return false for an invalid schema with anyOf', () => {
const schema = {
anyOf: [{ type: 'string' }, { description: 'no type' }],
};
expect(hasValidTypes(schema)).toBe(false);
});
it('should return true for a valid schema with allOf', () => {
const schema = {
allOf: [
{ type: 'string' },
{ type: 'object', properties: { foo: { type: 'string' } } },
],
};
expect(hasValidTypes(schema)).toBe(true);
});
it('should return false for an invalid schema with allOf', () => {
const schema = {
allOf: [{ type: 'string' }, { description: 'no type' }],
};
expect(hasValidTypes(schema)).toBe(false);
});
it('should return true for a valid schema with oneOf', () => {
const schema = {
oneOf: [{ type: 'string' }, { type: 'number' }],
};
expect(hasValidTypes(schema)).toBe(true);
});
it('should return false for an invalid schema with oneOf', () => {
const schema = {
oneOf: [{ type: 'string' }, { description: 'no type' }],
};
expect(hasValidTypes(schema)).toBe(false);
});
it('should return true for a valid schema with nested subschemas', () => {
const schema = {
anyOf: [
{ type: 'string' },
{
allOf: [
{ type: 'object', properties: { a: { type: 'string' } } },
{ type: 'object', properties: { b: { type: 'number' } } },
],
},
],
};
expect(hasValidTypes(schema)).toBe(true);
});
it('should return false for an invalid schema with nested subschemas', () => {
const schema = {
anyOf: [
{ type: 'string' },
{
allOf: [
{ type: 'object', properties: { a: { type: 'string' } } },
{ description: 'no type' },
],
},
],
};
expect(hasValidTypes(schema)).toBe(false);
});
it('should return true for a schema with a type and subschemas', () => {
const schema = {
type: 'string',
anyOf: [{ minLength: 1 }, { maxLength: 5 }],
};
expect(hasValidTypes(schema)).toBe(true);
});
it('should return false for a schema with no type and no subschemas', () => {
const schema = {
description: 'a schema with no type',
};
expect(hasValidTypes(schema)).toBe(false);
});
it('should return true for a valid schema', () => {
const schema = {
type: 'object',
properties: {
param1: { type: 'string' },
},
};
expect(hasValidTypes(schema)).toBe(true);
});
it('should return false if a parameter is missing a type', () => {
const schema = {
type: 'object',
properties: {
param1: { description: 'a param with no type' },
},
};
expect(hasValidTypes(schema)).toBe(false);
});
it('should return false if a nested parameter is missing a type', () => {
const schema = {
type: 'object',
properties: {
param1: {
type: 'object',
properties: {
nestedParam: {
description: 'a nested param with no type',
},
},
},
},
};
expect(hasValidTypes(schema)).toBe(false);
});
it('should return false if an array item is missing a type', () => {
const schema = {
type: 'object',
properties: {
param1: {
type: 'array',
items: {
description: 'an array item with no type',
},
},
},
};
expect(hasValidTypes(schema)).toBe(false);
});
it('should return true for a schema with no properties', () => {
const schema = {
type: 'object',
};
expect(hasValidTypes(schema)).toBe(true);
});
it('should return true for a schema with an empty properties object', () => {
const schema = {
type: 'object',
properties: {},
};
expect(hasValidTypes(schema)).toBe(true);
});
});
});

View File

@@ -20,6 +20,7 @@ import {
ListPromptsResultSchema,
GetPromptResult,
GetPromptResultSchema,
ListRootsRequestSchema,
} from '@modelcontextprotocol/sdk/types.js';
import { parse } from 'shell-quote';
import { AuthProviderType, MCPServerConfig } from '../config/config.js';
@@ -33,6 +34,9 @@ import { MCPOAuthProvider } from '../mcp/oauth-provider.js';
import { OAuthUtils } from '../mcp/oauth-utils.js';
import { MCPOAuthTokenStorage } from '../mcp/oauth-token-storage.js';
import { getErrorMessage } from '../utils/errors.js';
import { basename } from 'node:path';
import { pathToFileURL } from 'node:url';
import { WorkspaceContext } from '../utils/workspaceContext.js';
export const MCP_DEFAULT_TIMEOUT_MSEC = 10 * 60 * 1000; // default to 10 minutes
@@ -306,6 +310,7 @@ export async function discoverMcpTools(
toolRegistry: ToolRegistry,
promptRegistry: PromptRegistry,
debugMode: boolean,
workspaceContext: WorkspaceContext,
): Promise<void> {
mcpDiscoveryState = MCPDiscoveryState.IN_PROGRESS;
try {
@@ -319,6 +324,7 @@ export async function discoverMcpTools(
toolRegistry,
promptRegistry,
debugMode,
workspaceContext,
),
);
await Promise.all(discoveryPromises);
@@ -363,6 +369,7 @@ export async function connectAndDiscover(
toolRegistry: ToolRegistry,
promptRegistry: PromptRegistry,
debugMode: boolean,
workspaceContext: WorkspaceContext,
): Promise<void> {
updateMCPServerStatus(mcpServerName, MCPServerStatus.CONNECTING);
@@ -372,6 +379,7 @@ export async function connectAndDiscover(
mcpServerName,
mcpServerConfig,
debugMode,
workspaceContext,
);
mcpClient.onerror = (error) => {
@@ -416,6 +424,65 @@ export async function connectAndDiscover(
}
}
/**
* Recursively validates that a JSON schema and all its nested properties and
* items have a `type` defined.
*
* @param schema The JSON schema to validate.
* @returns `true` if the schema is valid, `false` otherwise.
*
* @visiblefortesting
*/
export function hasValidTypes(schema: unknown): boolean {
if (typeof schema !== 'object' || schema === null) {
// Not a schema object we can validate, or not a schema at all.
// Treat as valid as it has no properties to be invalid.
return true;
}
const s = schema as Record<string, unknown>;
if (!s.type) {
// These keywords contain an array of schemas that should be validated.
//
// If no top level type was given, then they must each have a type.
let hasSubSchema = false;
const schemaArrayKeywords = ['anyOf', 'allOf', 'oneOf'];
for (const keyword of schemaArrayKeywords) {
const subSchemas = s[keyword];
if (Array.isArray(subSchemas)) {
hasSubSchema = true;
for (const subSchema of subSchemas) {
if (!hasValidTypes(subSchema)) {
return false;
}
}
}
}
// If the node itself is missing a type and had no subschemas, then it isn't valid.
if (!hasSubSchema) return false;
}
if (s.type === 'object' && s.properties) {
if (typeof s.properties === 'object' && s.properties !== null) {
for (const prop of Object.values(s.properties)) {
if (!hasValidTypes(prop)) {
return false;
}
}
}
}
if (s.type === 'array' && s.items) {
if (!hasValidTypes(s.items)) {
return false;
}
}
return true;
}
/**
* Discovers and sanitizes tools from a connected MCP client.
* It retrieves function declarations from the client, filters out disabled tools,
@@ -448,6 +515,15 @@ export async function discoverTools(
continue;
}
if (!hasValidTypes(funcDecl.parametersJsonSchema)) {
console.warn(
`Skipping tool '${funcDecl.name}' from MCP server '${mcpServerName}' ` +
`because it has missing types in its parameter schema. Please file an ` +
`issue with the owner of the MCP server.`,
);
continue;
}
discoveredTools.push(
new DiscoveredMCPTool(
mcpCallableTool,
@@ -587,12 +663,30 @@ export async function connectToMcpServer(
mcpServerName: string,
mcpServerConfig: MCPServerConfig,
debugMode: boolean,
workspaceContext: WorkspaceContext,
): Promise<Client> {
const mcpClient = new Client({
name: 'qwen-code-mcp-client',
version: '0.0.1',
});
mcpClient.registerCapabilities({
roots: {},
});
mcpClient.setRequestHandler(ListRootsRequestSchema, async () => {
const roots = [];
for (const dir of workspaceContext.getDirectories()) {
roots.push({
uri: pathToFileURL(dir).toString(),
name: basename(dir),
});
}
return {
roots,
};
});
// patch Client.callTool to use request timeout as genai McpCallTool.callTool does not do it
// TODO: remove this hack once GenAI SDK does callTool with request options
if ('callTool' in mcpClient) {

View File

@@ -12,13 +12,7 @@ import {
ToolMcpConfirmationDetails,
Icon,
} from './tools.js';
import {
CallableTool,
Part,
FunctionCall,
FunctionDeclaration,
Type,
} from '@google/genai';
import { CallableTool, Part, FunctionCall } from '@google/genai';
type ToolParams = Record<string, unknown>;
@@ -64,7 +58,7 @@ export class DiscoveredMCPTool extends BaseTool<ToolParams, ToolResult> {
readonly serverName: string,
readonly serverToolName: string,
description: string,
readonly parameterSchemaJson: unknown,
readonly parameterSchema: unknown,
readonly timeout?: number,
readonly trust?: boolean,
nameOverride?: string,
@@ -74,7 +68,7 @@ export class DiscoveredMCPTool extends BaseTool<ToolParams, ToolResult> {
`${serverToolName} (${serverName} MCP Server)`,
description,
Icon.Hammer,
{ type: Type.OBJECT }, // this is a dummy Schema for MCP, will be not be used to construct the FunctionDeclaration
parameterSchema,
true, // isOutputMarkdown
false, // canUpdateOutput
);
@@ -86,25 +80,13 @@ export class DiscoveredMCPTool extends BaseTool<ToolParams, ToolResult> {
this.serverName,
this.serverToolName,
this.description,
this.parameterSchemaJson,
this.parameterSchema,
this.timeout,
this.trust,
`${this.serverName}__${this.serverToolName}`,
);
}
/**
* Overrides the base schema to use parametersJsonSchema when building
* FunctionDeclaration
*/
override get schema(): FunctionDeclaration {
return {
name: this.name,
description: this.description,
parametersJsonSchema: this.parameterSchemaJson,
};
}
async shouldConfirmExecute(
_params: ToolParams,
_abortSignal: AbortSignal,

View File

@@ -199,7 +199,17 @@ describe('MemoryTool', () => {
);
expect(memoryTool.schema).toBeDefined();
expect(memoryTool.schema.name).toBe('save_memory');
expect(memoryTool.schema.parameters?.properties?.fact).toBeDefined();
expect(memoryTool.schema.parametersJsonSchema).toStrictEqual({
type: 'object',
properties: {
fact: {
type: 'string',
description:
'The specific fact or piece of information to remember. Should be a clear, self-contained statement.',
},
},
required: ['fact'],
});
});
it('should call performAddMemoryEntry with correct parameters and return success', async () => {

View File

@@ -11,24 +11,24 @@ import {
ToolConfirmationOutcome,
Icon,
} from './tools.js';
import { FunctionDeclaration, Type } from '@google/genai';
import { FunctionDeclaration } from '@google/genai';
import * as fs from 'fs/promises';
import * as path from 'path';
import { homedir } from 'os';
import * as Diff from 'diff';
import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js';
import { tildeifyPath } from '../utils/paths.js';
import { ModifiableTool, ModifyContext } from './modifiable-tool.js';
import { ModifiableDeclarativeTool, ModifyContext } from './modifiable-tool.js';
const memoryToolSchemaData: FunctionDeclaration = {
name: 'save_memory',
description:
'Saves a specific piece of information or fact to your long-term memory. Use this when the user explicitly asks you to remember something, or when they state a clear, concise fact that seems important to retain for future interactions.',
parameters: {
type: Type.OBJECT,
parametersJsonSchema: {
type: 'object',
properties: {
fact: {
type: Type.STRING,
type: 'string',
description:
'The specific fact or piece of information to remember. Should be a clear, self-contained statement.',
},
@@ -112,7 +112,7 @@ function ensureNewlineSeparation(currentContent: string): string {
export class MemoryTool
extends BaseTool<SaveMemoryParams, ToolResult>
implements ModifiableTool<SaveMemoryParams>
implements ModifiableDeclarativeTool<SaveMemoryParams>
{
private static readonly allowlist: Set<string> = new Set();
@@ -123,7 +123,7 @@ export class MemoryTool
'Save Memory',
memoryToolDescription,
Icon.LightBulb,
memoryToolSchemaData.parameters as Record<string, unknown>,
memoryToolSchemaData.parametersJsonSchema as Record<string, unknown>,
);
}
@@ -220,6 +220,7 @@ export class MemoryTool
type: 'edit',
title: `Confirm Memory Save: ${tildeifyPath(memoryFilePath)}`,
fileName: memoryFilePath,
filePath: memoryFilePath,
fileDiff,
originalContent: currentContent,
newContent,

View File

@@ -8,8 +8,8 @@ import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
import {
modifyWithEditor,
ModifyContext,
ModifiableTool,
isModifiableTool,
ModifiableDeclarativeTool,
isModifiableDeclarativeTool,
} from './modifiable-tool.js';
import { EditorType } from '../utils/editor.js';
import fs from 'fs';
@@ -338,16 +338,16 @@ describe('isModifiableTool', () => {
const mockTool = {
name: 'test-tool',
getModifyContext: vi.fn(),
} as unknown as ModifiableTool<TestParams>;
} as unknown as ModifiableDeclarativeTool<TestParams>;
expect(isModifiableTool(mockTool)).toBe(true);
expect(isModifiableDeclarativeTool(mockTool)).toBe(true);
});
it('should return false for objects without getModifyContext method', () => {
const mockTool = {
name: 'test-tool',
} as unknown as ModifiableTool<TestParams>;
} as unknown as ModifiableDeclarativeTool<TestParams>;
expect(isModifiableTool(mockTool)).toBe(false);
expect(isModifiableDeclarativeTool(mockTool)).toBe(false);
});
});

View File

@@ -11,13 +11,14 @@ import fs from 'fs';
import * as Diff from 'diff';
import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js';
import { isNodeError } from '../utils/errors.js';
import { Tool } from './tools.js';
import { AnyDeclarativeTool, DeclarativeTool, ToolResult } from './tools.js';
/**
* A tool that supports a modify operation.
* A declarative tool that supports a modify operation.
*/
export interface ModifiableTool<ToolParams> extends Tool<ToolParams> {
getModifyContext(abortSignal: AbortSignal): ModifyContext<ToolParams>;
export interface ModifiableDeclarativeTool<TParams extends object>
extends DeclarativeTool<TParams, ToolResult> {
getModifyContext(abortSignal: AbortSignal): ModifyContext<TParams>;
}
export interface ModifyContext<ToolParams> {
@@ -39,9 +40,12 @@ export interface ModifyResult<ToolParams> {
updatedDiff: string;
}
export function isModifiableTool<TParams>(
tool: Tool<TParams>,
): tool is ModifiableTool<TParams> {
/**
* Type guard to check if a declarative tool is modifiable.
*/
export function isModifiableDeclarativeTool(
tool: AnyDeclarativeTool,
): tool is ModifiableDeclarativeTool<object> {
return 'getModifyContext' in tool;
}

View File

@@ -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';
@@ -13,6 +14,7 @@ import fsp from 'fs/promises';
import { Config } from '../config/config.js';
import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
import { ToolInvocation, ToolResult } from './tools.js';
describe('ReadFileTool', () => {
let tempRootDir: string;
@@ -40,112 +42,137 @@ describe('ReadFileTool', () => {
}
});
describe('validateToolParams', () => {
it('should return null for valid params (absolute path within root)', () => {
describe('build', () => {
it('should return an invocation for valid params (absolute path within root)', () => {
const params: ReadFileToolParams = {
absolute_path: path.join(tempRootDir, 'test.txt'),
};
expect(tool.validateToolParams(params)).toBeNull();
const result = tool.build(params);
expect(typeof result).not.toBe('string');
});
it('should return null 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',
};
expect(tool.validateToolParams(params)).toBeNull();
});
it('should return error for relative path', () => {
const params: ReadFileToolParams = { absolute_path: 'test.txt' };
expect(tool.validateToolParams(params)).toBe(
`File path must be absolute, but was relative: test.txt. You must provide an absolute path.`,
expect(() => tool.build(params)).toThrow(
'File path must be absolute, but was relative: relative/path.txt. You must provide an absolute path.',
);
});
it('should return error for path outside root', () => {
const outsidePath = path.resolve(os.tmpdir(), 'outside-root.txt');
const params: ReadFileToolParams = { absolute_path: outsidePath };
const error = tool.validateToolParams(params);
expect(error).toContain(
'File path must be within one of the workspace directories',
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/,
);
});
it('should return 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.validateToolParams(params)).toBe(
expect(() => tool.build(params)).toThrow(
'Offset must be a non-negative number',
);
});
it('should return 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.validateToolParams(paramsZero)).toBe(
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,
};
expect(tool.validateToolParams(paramsNegative)).toBe(
'Limit must be a positive number',
);
});
it('should return error for schema validation failure (e.g. missing path)', () => {
const params = { offset: 0 } as unknown as ReadFileToolParams;
expect(tool.validateToolParams(params)).toBe(
`params must have required property 'absolute_path'`,
);
});
});
describe('getDescription', () => {
it('should return a shortened, relative path', () => {
const filePath = path.join(tempRootDir, 'sub', 'dir', 'file.txt');
const params: ReadFileToolParams = { absolute_path: filePath };
expect(tool.getDescription(params)).toBe(
path.join('sub', 'dir', 'file.txt'),
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'),
};
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 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');
});
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 };
expect(tool.getDescription(params)).toBe('.');
const invocation = tool.build(params);
expect(typeof invocation).not.toBe('string');
expect(
(
invocation as ToolInvocation<ReadFileToolParams, ToolResult>
).getDescription(),
).toBe('.');
});
});
describe('execute', () => {
it('should return validation error if params are invalid', async () => {
const params: ReadFileToolParams = {
absolute_path: 'relative/path.txt',
};
expect(await tool.execute(params, abortSignal)).toEqual({
llmContent:
'Error: Invalid parameters provided. Reason: File path must be absolute, but was relative: relative/path.txt. You must provide an absolute path.',
returnDisplay:
'File path must be absolute, but was relative: relative/path.txt. You must provide an absolute path.',
});
});
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 tool.execute(params, abortSignal)).toEqual({
llmContent: `File not found: ${filePath}`,
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,
},
});
});
@@ -154,59 +181,191 @@ describe('ReadFileTool', () => {
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 tool.execute(params, abortSignal)).toEqual({
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 };
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 tool.execute(params, abortSignal)).toEqual({
llmContent: {
inlineData: {
mimeType: 'image/png',
data: pngContent.toString('base64'),
},
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,
},
returnDisplay: `Read image file: image.png`,
});
});
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.';
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
>;
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 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
>;
expect(await tool.execute(params, 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.llmContent).toContain(
'IMPORTANT: The file content has been truncated',
);
expect(result.llmContent).toContain('--- FILE CONTENT (truncated) ---');
expect(result.returnDisplay).toContain('some lines were shortened');
});
it('should pass offset and limit to read a slice of a text file', async () => {
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 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');
});
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
>;
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');
});
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
>;
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 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 result = await invocation.execute(abortSignal);
expect(result.llmContent).toBe(svgContent);
expect(result.returnDisplay).toBe('Read SVG as text: image.svg');
});
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
>;
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 support offset and limit for text files', async () => {
const filePath = path.join(tempRootDir, 'paginated.txt');
const fileContent = Array.from(
{ length: 20 },
(_, i) => `Line ${i + 1}`,
).join('\n');
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 = {
@@ -214,16 +373,24 @@ describe('ReadFileTool', () => {
offset: 5, // Start from line 6
limit: 3,
};
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
expect(await tool.execute(params, abortSignal)).toEqual({
llmContent: [
'[File content truncated: showing lines 6-8 of 20 total lines. Use offset/limit parameters to view more.]',
'Line 6',
'Line 7',
'Line 8',
].join('\n'),
returnDisplay: 'Read lines 6-8 of 20 from paginated.txt',
});
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', () => {
@@ -234,66 +401,37 @@ describe('ReadFileTool', () => {
);
});
it('should return error if path is ignored by a .geminiignore pattern', async () => {
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(await tool.execute(params, abortSignal)).toEqual({
llmContent: `Error: Invalid parameters provided. Reason: ${expectedError}`,
returnDisplay: expectedError,
});
expect(() => tool.build(params)).toThrow(expectedError);
});
it('should return error if path is in an ignored directory', async () => {
it('should throw error if file 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');
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: filePath,
absolute_path: ignoredFilePath,
};
const expectedError = `File path '${filePath}' is ignored by .geminiignore pattern(s).`;
expect(await tool.execute(params, abortSignal)).toEqual({
llmContent: `Error: Invalid parameters provided. Reason: ${expectedError}`,
returnDisplay: expectedError,
});
const expectedError = `File path '${ignoredFilePath}' is ignored by .geminiignore pattern(s).`;
expect(() => tool.build(params)).toThrow(expectedError);
});
});
});
describe('workspace boundary validation', () => {
it('should validate paths are within workspace root', () => {
const params: ReadFileToolParams = {
absolute_path: path.join(tempRootDir, 'file.txt'),
};
expect(tool.validateToolParams(params)).toBeNull();
});
it('should reject paths outside workspace root', () => {
const params: ReadFileToolParams = {
absolute_path: '/etc/passwd',
};
const error = tool.validateToolParams(params);
expect(error).toContain(
'File path must be within one of the workspace directories',
);
expect(error).toContain(tempRootDir);
});
it('should provide clear error message with workspace directories', () => {
const outsidePath = path.join(os.tmpdir(), 'outside-workspace.txt');
const params: ReadFileToolParams = {
absolute_path: outsidePath,
};
const error = tool.validateToolParams(params);
expect(error).toContain(
'File path must be within one of the workspace directories',
);
expect(error).toContain(tempRootDir);
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');
});
});
});
});

View File

@@ -7,8 +7,16 @@
import path from 'path';
import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { BaseTool, Icon, ToolLocation, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import {
BaseDeclarativeTool,
BaseToolInvocation,
Icon,
ToolInvocation,
ToolLocation,
ToolResult,
} from './tools.js';
import { ToolErrorType } from './tool-error.js';
import { PartUnion } from '@google/genai';
import {
processSingleFileContent,
getSpecificMimeType,
@@ -39,44 +47,162 @@ export interface ReadFileToolParams {
limit?: number;
}
class ReadFileToolInvocation extends BaseToolInvocation<
ReadFileToolParams,
ToolResult
> {
constructor(
private config: Config,
params: ReadFileToolParams,
) {
super(params);
}
getDescription(): string {
const relativePath = makeRelative(
this.params.absolute_path,
this.config.getTargetDir(),
);
return shortenPath(relativePath);
}
override toolLocations(): ToolLocation[] {
return [{ path: this.params.absolute_path, line: this.params.offset }];
}
async execute(): Promise<ToolResult> {
const result = await processSingleFileContent(
this.params.absolute_path,
this.config.getTargetDir(),
this.params.offset,
this.params.limit,
);
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,
returnDisplay: result.returnDisplay || 'Error reading file',
error: {
message: result.error,
type: errorType,
},
};
}
let llmContent: PartUnion;
if (result.isTruncated) {
const [start, end] = result.linesShown!;
const total = result.originalLineCount!;
const nextOffset = this.params.offset
? this.params.offset + end - start + 1
: end;
llmContent = `
IMPORTANT: The file content has been truncated.
Status: Showing lines ${start}-${end} of ${total} 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: ${nextOffset}.
--- FILE CONTENT (truncated) ---
${result.llmContent}`;
} else {
llmContent = result.llmContent || '';
}
const lines =
typeof result.llmContent === 'string'
? result.llmContent.split('\n').length
: undefined;
const mimetype = getSpecificMimeType(this.params.absolute_path);
recordFileOperationMetric(
this.config,
FileOperation.READ,
lines,
mimetype,
path.extname(this.params.absolute_path),
);
return {
llmContent,
returnDisplay: result.returnDisplay || '',
};
}
}
/**
* Implementation of the ReadFile tool logic
*/
export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> {
export class ReadFileTool extends BaseDeclarativeTool<
ReadFileToolParams,
ToolResult
> {
static readonly Name: string = 'read_file';
constructor(private config: Config) {
super(
ReadFileTool.Name,
'ReadFile',
'Reads and returns the content of a specified file from the local filesystem. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), and PDF files. For text files, it can read specific line ranges.',
`Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), and PDF files. For text files, it can read specific line ranges.`,
Icon.FileSearch,
{
properties: {
absolute_path: {
description:
"The absolute path to the file to read (e.g., '/home/user/project/file.txt'). Relative paths are not supported. You must provide an absolute path.",
type: Type.STRING,
type: 'string',
},
offset: {
description:
"Optional: For text files, the 0-based line number to start reading from. Requires 'limit' to be set. Use for paginating through large files.",
type: Type.NUMBER,
type: 'number',
},
limit: {
description:
"Optional: For text files, maximum number of lines to read. Use with 'offset' to paginate through large files. If omitted, reads the entire file (if feasible, up to a default limit).",
type: Type.NUMBER,
type: 'number',
},
},
required: ['absolute_path'],
type: Type.OBJECT,
type: 'object',
},
);
}
validateToolParams(params: ReadFileToolParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
protected validateToolParams(params: ReadFileToolParams): string | null {
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}
@@ -106,67 +232,9 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> {
return null;
}
getDescription(params: ReadFileToolParams): string {
if (
!params ||
typeof params.absolute_path !== 'string' ||
params.absolute_path.trim() === ''
) {
return `Path unavailable`;
}
const relativePath = makeRelative(
params.absolute_path,
this.config.getTargetDir(),
);
return shortenPath(relativePath);
}
toolLocations(params: ReadFileToolParams): ToolLocation[] {
return [{ path: params.absolute_path, line: params.offset }];
}
async execute(
protected createInvocation(
params: ReadFileToolParams,
_signal: AbortSignal,
): Promise<ToolResult> {
const validationError = this.validateToolParams(params);
if (validationError) {
return {
llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`,
returnDisplay: validationError,
};
}
const result = await processSingleFileContent(
params.absolute_path,
this.config.getTargetDir(),
params.offset,
params.limit,
);
if (result.error) {
return {
llmContent: result.error, // The detailed error for LLM
returnDisplay: result.returnDisplay || 'Error reading file', // User-friendly error
};
}
const lines =
typeof result.llmContent === 'string'
? result.llmContent.split('\n').length
: undefined;
const mimetype = getSpecificMimeType(params.absolute_path);
recordFileOperationMetric(
this.config,
FileOperation.READ,
lines,
mimetype,
path.extname(params.absolute_path),
);
return {
llmContent: result.llmContent || '',
returnDisplay: result.returnDisplay || '',
};
): ToolInvocation<ReadFileToolParams, ToolResult> {
return new ReadFileToolInvocation(this.config, params);
}
}

View File

@@ -476,6 +476,34 @@ describe('ReadManyFilesTool', () => {
fs.rmSync(tempDir1, { recursive: true, force: true });
fs.rmSync(tempDir2, { recursive: true, force: true });
});
it('should add a warning for truncated files', async () => {
createFile('file1.txt', 'Content1');
// Create a file that will be "truncated" by making it long
const longContent = Array.from({ length: 2500 }, (_, i) => `L${i}`).join(
'\n',
);
createFile('large-file.txt', longContent);
const params = { paths: ['*.txt'] };
const result = await tool.execute(params, new AbortController().signal);
const content = result.llmContent as string[];
const normalFileContent = content.find((c) => c.includes('file1.txt'));
const truncatedFileContent = content.find((c) =>
c.includes('large-file.txt'),
);
expect(normalFileContent).not.toContain(
'[WARNING: This file was truncated.',
);
expect(truncatedFileContent).toContain(
"[WARNING: This file was truncated. To view the full content, use the 'read_file' tool on this specific file.]",
);
// Check that the actual content is still there but truncated
expect(truncatedFileContent).toContain('L200');
expect(truncatedFileContent).not.toContain('L2400');
});
});
describe('Batch Processing', () => {
@@ -495,7 +523,7 @@ describe('ReadManyFilesTool', () => {
fs.writeFileSync(fullPath, content);
};
it('should process files in parallel for performance', async () => {
it('should process files in parallel', async () => {
// Mock detectFileType to add artificial delay to simulate I/O
const detectFileTypeSpy = vi.spyOn(
await import('../utils/fileUtils.js'),
@@ -506,31 +534,21 @@ describe('ReadManyFilesTool', () => {
const fileCount = 4;
const files = createMultipleFiles(fileCount, 'Batch test');
// Mock with 100ms delay per file to simulate I/O operations
// Mock with 10ms delay per file to simulate I/O operations
detectFileTypeSpy.mockImplementation(async (_filePath: string) => {
await new Promise((resolve) => setTimeout(resolve, 100));
await new Promise((resolve) => setTimeout(resolve, 10));
return 'text';
});
const startTime = Date.now();
const params = { paths: files };
const result = await tool.execute(params, new AbortController().signal);
const endTime = Date.now();
const processingTime = endTime - startTime;
console.log(
`Processing time: ${processingTime}ms for ${fileCount} files`,
);
// Verify parallel processing performance improvement
// Parallel processing should complete in ~100ms (single file time)
// Sequential would take ~400ms (4 files × 100ms each)
expect(processingTime).toBeLessThan(200); // Should PASS with parallel implementation
// Verify all files were processed
const content = result.llmContent as string[];
expect(content).toHaveLength(fileCount);
for (let i = 0; i < fileCount; i++) {
expect(content.join('')).toContain(`Batch test ${i}`);
}
// Cleanup mock
detectFileTypeSpy.mockRestore();

View File

@@ -16,7 +16,7 @@ import {
DEFAULT_ENCODING,
getSpecificMimeType,
} from '../utils/fileUtils.js';
import { PartListUnion, Schema, Type } from '@google/genai';
import { PartListUnion } from '@google/genai';
import { Config, DEFAULT_FILE_FILTERING_OPTIONS } from '../config/config.js';
import {
recordFileOperationMetric,
@@ -150,47 +150,47 @@ export class ReadManyFilesTool extends BaseTool<
static readonly Name: string = 'read_many_files';
constructor(private config: Config) {
const parameterSchema: Schema = {
type: Type.OBJECT,
const parameterSchema = {
type: 'object',
properties: {
paths: {
type: Type.ARRAY,
type: 'array',
items: {
type: Type.STRING,
minLength: '1',
type: 'string',
minLength: 1,
},
minItems: '1',
minItems: 1,
description:
"Required. An array of glob patterns or paths relative to the tool's target directory. Examples: ['src/**/*.ts'], ['README.md', 'docs/']",
},
include: {
type: Type.ARRAY,
type: 'array',
items: {
type: Type.STRING,
minLength: '1',
type: 'string',
minLength: 1,
},
description:
'Optional. Additional glob patterns to include. These are merged with `paths`. Example: ["*.test.ts"] to specifically add test files if they were broadly excluded.',
default: [],
},
exclude: {
type: Type.ARRAY,
type: 'array',
items: {
type: Type.STRING,
minLength: '1',
type: 'string',
minLength: 1,
},
description:
'Optional. Glob patterns for files/directories to exclude. Added to default excludes if useDefaultExcludes is true. Example: ["**/*.log", "temp/"]',
default: [],
},
recursive: {
type: Type.BOOLEAN,
type: 'boolean',
description:
'Optional. Whether to search recursively (primarily controlled by `**` in glob patterns). Defaults to true.',
default: true,
},
useDefaultExcludes: {
type: Type.BOOLEAN,
type: 'boolean',
description:
'Optional. Whether to apply a list of default exclusion patterns (e.g., node_modules, .git, binary files). Defaults to true.',
default: true,
@@ -198,17 +198,17 @@ export class ReadManyFilesTool extends BaseTool<
file_filtering_options: {
description:
'Whether to respect ignore patterns from .gitignore or .geminiignore',
type: Type.OBJECT,
type: 'object',
properties: {
respect_git_ignore: {
description:
'Optional: Whether to respect .gitignore patterns when listing files. Only available in git repositories. Defaults to true.',
type: Type.BOOLEAN,
type: 'boolean',
},
respect_gemini_ignore: {
description:
'Optional: Whether to respect .geminiignore patterns when listing files. Defaults to true.',
type: Type.BOOLEAN,
type: 'boolean',
},
},
},
@@ -235,7 +235,10 @@ Use this tool when the user's query implies needing the content of several files
}
validateParams(params: ReadManyFilesParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}
@@ -524,11 +527,15 @@ Use this tool when the user's query implies needing the content of several files
'{filePath}',
filePath,
);
contentParts.push(
`${separator}\n\n${fileReadResult.llmContent}\n\n`,
);
let fileContentForLlm = '';
if (fileReadResult.isTruncated) {
fileContentForLlm += `[WARNING: This file was truncated. To view the full content, use the 'read_file' tool on this specific file.]\n\n`;
}
fileContentForLlm += fileReadResult.llmContent;
contentParts.push(`${separator}\n\n${fileContentForLlm}\n\n`);
} else {
contentParts.push(fileReadResult.llmContent); // This is a Part for image/pdf
// This is a Part for image/pdf, which we don't add the separator to.
contentParts.push(fileReadResult.llmContent);
}
processedFilesRelativePaths.push(relativePathForDisplay);

View File

@@ -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,
@@ -208,6 +209,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 },

View File

@@ -17,7 +17,7 @@ import {
ToolConfirmationOutcome,
Icon,
} from './tools.js';
import { Type } from '@google/genai';
import { ToolErrorType } from './tool-error.js';
import { SchemaValidator } from '../utils/schemaValidator.js';
import { getErrorMessage } from '../utils/errors.js';
import { summarizeToolOutput } from '../utils/summarizer.js';
@@ -63,19 +63,19 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> {
Process Group PGID: Process group started or \`(none)\``,
Icon.Terminal,
{
type: Type.OBJECT,
type: 'object',
properties: {
command: {
type: Type.STRING,
type: 'string',
description: 'Exact bash command to execute as `bash -c <command>`',
},
description: {
type: Type.STRING,
type: 'string',
description:
'Brief description of the command for the user. Be specific and concise. Ideally a single sentence. Can be up to 3 sentences for clarity. No line breaks.',
},
directory: {
type: Type.STRING,
type: 'string',
description:
'(OPTIONAL) Directory to run the command in, if not the project root directory. Must be relative to the project root directory and must already exist.',
},
@@ -112,7 +112,10 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> {
}
return commandCheck.reason;
}
const errors = SchemaValidator.validate(this.schema.parameters, params);
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}
@@ -186,8 +189,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,
},
};
}

View File

@@ -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',

View File

@@ -15,22 +15,12 @@ import {
Mocked,
} from 'vitest';
import { Config, ConfigParameters, ApprovalMode } from '../config/config.js';
import {
ToolRegistry,
DiscoveredTool,
sanitizeParameters,
} from './tool-registry.js';
import { ToolRegistry, DiscoveredTool } from './tool-registry.js';
import { DiscoveredMCPTool } from './mcp-tool.js';
import { BaseTool, Icon, ToolResult } from './tools.js';
import {
FunctionDeclaration,
CallableTool,
mcpToTool,
Type,
Schema,
} from '@google/genai';
import { FunctionDeclaration, CallableTool, mcpToTool } from '@google/genai';
import { spawn } from 'node:child_process';
import fs from 'node:fs';
import { MockTool } from '../test-utils/tools.js';
vi.mock('node:fs');
@@ -106,28 +96,6 @@ const createMockCallableTool = (
callTool: vi.fn(),
});
class MockTool extends BaseTool<{ param: string }, ToolResult> {
constructor(
name = 'mock-tool',
displayName = 'A mock tool',
description = 'A mock tool description',
) {
super(name, displayName, description, Icon.Hammer, {
type: Type.OBJECT,
properties: {
param: { type: Type.STRING },
},
required: ['param'],
});
}
async execute(params: { param: string }): Promise<ToolResult> {
return {
llmContent: `Executed with ${params.param}`,
returnDisplay: `Executed with ${params.param}`,
};
}
}
const baseConfigParams: ConfigParameters = {
cwd: '/tmp',
model: 'test-model',
@@ -275,18 +243,18 @@ describe('ToolRegistry', () => {
});
describe('discoverTools', () => {
it('should sanitize tool parameters during discovery from command', async () => {
it('should will preserve tool parametersJsonSchema during discovery from command', async () => {
const discoveryCommand = 'my-discovery-command';
mockConfigGetToolDiscoveryCommand.mockReturnValue(discoveryCommand);
const unsanitizedToolDeclaration: FunctionDeclaration = {
name: 'tool-with-bad-format',
description: 'A tool with an invalid format property',
parameters: {
type: Type.OBJECT,
parametersJsonSchema: {
type: 'object',
properties: {
some_string: {
type: Type.STRING,
type: 'string',
format: 'uuid', // This is an unsupported format
},
},
@@ -329,11 +297,39 @@ describe('ToolRegistry', () => {
expect(discoveredTool).toBeDefined();
const registeredParams = (discoveredTool as DiscoveredTool).schema
.parameters as Schema;
expect(registeredParams.properties?.['some_string']).toBeDefined();
expect(registeredParams.properties?.['some_string']).toHaveProperty(
'format',
.parametersJsonSchema;
expect(registeredParams).toStrictEqual({
type: 'object',
properties: {
some_string: {
type: 'string',
format: 'uuid',
},
},
});
});
it('should discover tools using MCP servers defined in getMcpServers', async () => {
mockConfigGetToolDiscoveryCommand.mockReturnValue(undefined);
vi.spyOn(config, 'getMcpServerCommand').mockReturnValue(undefined);
const mcpServerConfigVal = {
'my-mcp-server': {
command: 'mcp-server-cmd',
args: ['--port', '1234'],
trust: true,
},
};
vi.spyOn(config, 'getMcpServers').mockReturnValue(mcpServerConfigVal);
await toolRegistry.discoverAllTools();
expect(mockDiscoverMcpTools).toHaveBeenCalledWith(
mcpServerConfigVal,
undefined,
toolRegistry,
config.getPromptRegistry(),
false,
expect.any(Object),
);
});
@@ -357,214 +353,8 @@ describe('ToolRegistry', () => {
toolRegistry,
config.getPromptRegistry(),
false,
);
});
it('should discover tools using MCP servers defined in getMcpServers', async () => {
mockConfigGetToolDiscoveryCommand.mockReturnValue(undefined);
vi.spyOn(config, 'getMcpServerCommand').mockReturnValue(undefined);
const mcpServerConfigVal = {
'my-mcp-server': {
command: 'mcp-server-cmd',
args: ['--port', '1234'],
trust: true,
},
};
vi.spyOn(config, 'getMcpServers').mockReturnValue(mcpServerConfigVal);
await toolRegistry.discoverAllTools();
expect(mockDiscoverMcpTools).toHaveBeenCalledWith(
mcpServerConfigVal,
undefined,
toolRegistry,
config.getPromptRegistry(),
false,
expect.any(Object),
);
});
});
});
describe('sanitizeParameters', () => {
it('should remove default when anyOf is present', () => {
const schema: Schema = {
anyOf: [{ type: Type.STRING }, { type: Type.NUMBER }],
default: 'hello',
};
sanitizeParameters(schema);
expect(schema.default).toBeUndefined();
});
it('should recursively sanitize items in anyOf', () => {
const schema: Schema = {
anyOf: [
{
anyOf: [{ type: Type.STRING }],
default: 'world',
},
{ type: Type.NUMBER },
],
};
sanitizeParameters(schema);
expect(schema.anyOf![0].default).toBeUndefined();
});
it('should recursively sanitize items in items', () => {
const schema: Schema = {
items: {
anyOf: [{ type: Type.STRING }],
default: 'world',
},
};
sanitizeParameters(schema);
expect(schema.items!.default).toBeUndefined();
});
it('should recursively sanitize items in properties', () => {
const schema: Schema = {
properties: {
prop1: {
anyOf: [{ type: Type.STRING }],
default: 'world',
},
},
};
sanitizeParameters(schema);
expect(schema.properties!.prop1.default).toBeUndefined();
});
it('should handle complex nested schemas', () => {
const schema: Schema = {
properties: {
prop1: {
items: {
anyOf: [{ type: Type.STRING }],
default: 'world',
},
},
prop2: {
anyOf: [
{
properties: {
nestedProp: {
anyOf: [{ type: Type.NUMBER }],
default: 123,
},
},
},
],
},
},
};
sanitizeParameters(schema);
expect(schema.properties!.prop1.items!.default).toBeUndefined();
const nestedProp =
schema.properties!.prop2.anyOf![0].properties!.nestedProp;
expect(nestedProp?.default).toBeUndefined();
});
it('should remove unsupported format from a simple string property', () => {
const schema: Schema = {
type: Type.OBJECT,
properties: {
name: { type: Type.STRING },
id: { type: Type.STRING, format: 'uuid' },
},
};
sanitizeParameters(schema);
expect(schema.properties?.['id']).toHaveProperty('format', undefined);
expect(schema.properties?.['name']).not.toHaveProperty('format');
});
it('should NOT remove supported format values', () => {
const schema: Schema = {
type: Type.OBJECT,
properties: {
date: { type: Type.STRING, format: 'date-time' },
role: {
type: Type.STRING,
format: 'enum',
enum: ['admin', 'user'],
},
},
};
const originalSchema = JSON.parse(JSON.stringify(schema));
sanitizeParameters(schema);
expect(schema).toEqual(originalSchema);
});
it('should handle arrays of objects', () => {
const schema: Schema = {
type: Type.OBJECT,
properties: {
items: {
type: Type.ARRAY,
items: {
type: Type.OBJECT,
properties: {
itemId: { type: Type.STRING, format: 'uuid' },
},
},
},
},
};
sanitizeParameters(schema);
expect(
(schema.properties?.['items']?.items as Schema)?.properties?.['itemId'],
).toHaveProperty('format', undefined);
});
it('should handle schemas with no properties to sanitize', () => {
const schema: Schema = {
type: Type.OBJECT,
properties: {
count: { type: Type.NUMBER },
isActive: { type: Type.BOOLEAN },
},
};
const originalSchema = JSON.parse(JSON.stringify(schema));
sanitizeParameters(schema);
expect(schema).toEqual(originalSchema);
});
it('should not crash on an empty or undefined schema', () => {
expect(() => sanitizeParameters({})).not.toThrow();
expect(() => sanitizeParameters(undefined)).not.toThrow();
});
it('should handle complex nested schemas with cycles', () => {
const userNode: any = {
type: Type.OBJECT,
properties: {
id: { type: Type.STRING, format: 'uuid' },
name: { type: Type.STRING },
manager: {
type: Type.OBJECT,
properties: {
id: { type: Type.STRING, format: 'uuid' },
},
},
},
};
userNode.properties.reports = {
type: Type.ARRAY,
items: userNode,
};
const schema: Schema = {
type: Type.OBJECT,
properties: {
ceo: userNode,
},
};
expect(() => sanitizeParameters(schema)).not.toThrow();
expect(schema.properties?.['ceo']?.properties?.['id']).toHaveProperty(
'format',
undefined,
);
expect(
schema.properties?.['ceo']?.properties?.['manager']?.properties?.['id'],
).toHaveProperty('format', undefined);
});
});

View File

@@ -4,8 +4,8 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { FunctionDeclaration, Schema, Type } from '@google/genai';
import { Tool, ToolResult, BaseTool, Icon } from './tools.js';
import { FunctionDeclaration } from '@google/genai';
import { AnyDeclarativeTool, Icon, ToolResult, BaseTool } from './tools.js';
import { Config } from '../config/config.js';
import { spawn } from 'node:child_process';
import { StringDecoder } from 'node:string_decoder';
@@ -125,7 +125,7 @@ Signal: Signal number or \`(none)\` if no signal was received.
}
export class ToolRegistry {
private tools: Map<string, Tool> = new Map();
private tools: Map<string, AnyDeclarativeTool> = new Map();
private config: Config;
constructor(config: Config) {
@@ -136,7 +136,7 @@ export class ToolRegistry {
* Registers a tool definition.
* @param tool - The tool object containing schema and execution logic.
*/
registerTool(tool: Tool): void {
registerTool(tool: AnyDeclarativeTool): void {
if (this.tools.has(tool.name)) {
if (tool instanceof DiscoveredMCPTool) {
tool = tool.asFullyQualifiedTool();
@@ -178,6 +178,7 @@ export class ToolRegistry {
this,
this.config.getPromptRegistry(),
this.config.getDebugMode(),
this.config.getWorkspaceContext(),
);
}
@@ -199,6 +200,7 @@ export class ToolRegistry {
this,
this.config.getPromptRegistry(),
this.config.getDebugMode(),
this.config.getWorkspaceContext(),
);
}
@@ -225,6 +227,7 @@ export class ToolRegistry {
this,
this.config.getPromptRegistry(),
this.config.getDebugMode(),
this.config.getWorkspaceContext(),
);
}
}
@@ -328,14 +331,12 @@ export class ToolRegistry {
console.warn('Discovered a tool with no name. Skipping.');
continue;
}
// Sanitize the parameters before registering the tool.
const parameters =
func.parameters &&
typeof func.parameters === 'object' &&
!Array.isArray(func.parameters)
? (func.parameters as Schema)
func.parametersJsonSchema &&
typeof func.parametersJsonSchema === 'object' &&
!Array.isArray(func.parametersJsonSchema)
? func.parametersJsonSchema
: {};
sanitizeParameters(parameters);
this.registerTool(
new DiscoveredTool(
this.config,
@@ -365,10 +366,26 @@ export class ToolRegistry {
return declarations;
}
/**
* Retrieves a filtered list of tool schemas based on a list of tool names.
* @param toolNames - An array of tool names to include.
* @returns An array of FunctionDeclarations for the specified tools.
*/
getFunctionDeclarationsFiltered(toolNames: string[]): FunctionDeclaration[] {
const declarations: FunctionDeclaration[] = [];
for (const name of toolNames) {
const tool = this.tools.get(name);
if (tool) {
declarations.push(tool.schema);
}
}
return declarations;
}
/**
* Returns an array of all registered and discovered tool instances.
*/
getAllTools(): Tool[] {
getAllTools(): AnyDeclarativeTool[] {
return Array.from(this.tools.values()).sort((a, b) =>
a.displayName.localeCompare(b.displayName),
);
@@ -377,8 +394,8 @@ export class ToolRegistry {
/**
* Returns an array of tools registered from a specific MCP server.
*/
getToolsByServer(serverName: string): Tool[] {
const serverTools: Tool[] = [];
getToolsByServer(serverName: string): AnyDeclarativeTool[] {
const serverTools: AnyDeclarativeTool[] = [];
for (const tool of this.tools.values()) {
if ((tool as DiscoveredMCPTool)?.serverName === serverName) {
serverTools.push(tool);
@@ -390,79 +407,7 @@ export class ToolRegistry {
/**
* Get the definition of a specific tool.
*/
getTool(name: string): Tool | undefined {
getTool(name: string): AnyDeclarativeTool | undefined {
return this.tools.get(name);
}
}
/**
* Sanitizes a schema object in-place to ensure compatibility with the Gemini API.
*
* NOTE: This function mutates the passed schema object.
*
* It performs the following actions:
* - Removes the `default` property when `anyOf` is present.
* - Removes unsupported `format` values from string properties, keeping only 'enum' and 'date-time'.
* - Recursively sanitizes nested schemas within `anyOf`, `items`, and `properties`.
* - Handles circular references within the schema to prevent infinite loops.
*
* @param schema The schema object to sanitize. It will be modified directly.
*/
export function sanitizeParameters(schema?: Schema) {
_sanitizeParameters(schema, new Set<Schema>());
}
/**
* Internal recursive implementation for sanitizeParameters.
* @param schema The schema object to sanitize.
* @param visited A set used to track visited schema objects during recursion.
*/
function _sanitizeParameters(schema: Schema | undefined, visited: Set<Schema>) {
if (!schema || visited.has(schema)) {
return;
}
visited.add(schema);
if (schema.anyOf) {
// Vertex AI gets confused if both anyOf and default are set.
schema.default = undefined;
for (const item of schema.anyOf) {
if (typeof item !== 'boolean') {
_sanitizeParameters(item, visited);
}
}
}
if (schema.items && typeof schema.items !== 'boolean') {
_sanitizeParameters(schema.items, visited);
}
if (schema.properties) {
for (const item of Object.values(schema.properties)) {
if (typeof item !== 'boolean') {
_sanitizeParameters(item, visited);
}
}
}
// Handle enum values - Gemini API only allows enum for STRING type
if (schema.enum && Array.isArray(schema.enum)) {
if (schema.type !== Type.STRING) {
// If enum is present but type is not STRING, convert type to STRING
schema.type = Type.STRING;
}
// Filter out null and undefined values, then convert remaining values to strings for Gemini API compatibility
schema.enum = schema.enum
.filter((value: unknown) => value !== null && value !== undefined)
.map((value: unknown) => String(value));
}
// Vertex AI only supports 'enum' and 'date-time' for STRING format.
if (schema.type === Type.STRING) {
if (
schema.format &&
schema.format !== 'enum' &&
schema.format !== 'date-time'
) {
schema.format = undefined;
}
}
}

View File

@@ -0,0 +1,125 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import { hasCycleInSchema } from './tools.js'; // Added getStringifiedResultForDisplay
describe('hasCycleInSchema', () => {
it('should detect a simple direct cycle', () => {
const schema = {
properties: {
data: {
$ref: '#/properties/data',
},
},
};
expect(hasCycleInSchema(schema)).toBe(true);
});
it('should detect a cycle from object properties referencing parent properties', () => {
const schema = {
type: 'object',
properties: {
data: {
type: 'object',
properties: {
child: { $ref: '#/properties/data' },
},
},
},
};
expect(hasCycleInSchema(schema)).toBe(true);
});
it('should detect a cycle from array items referencing parent properties', () => {
const schema = {
type: 'object',
properties: {
data: {
type: 'array',
items: {
type: 'object',
properties: {
child: { $ref: '#/properties/data/items' },
},
},
},
},
};
expect(hasCycleInSchema(schema)).toBe(true);
});
it('should detect a cycle between sibling properties', () => {
const schema = {
type: 'object',
properties: {
a: {
type: 'object',
properties: {
child: { $ref: '#/properties/b' },
},
},
b: {
type: 'object',
properties: {
child: { $ref: '#/properties/a' },
},
},
},
};
expect(hasCycleInSchema(schema)).toBe(true);
});
it('should not detect a cycle in a valid schema', () => {
const schema = {
type: 'object',
properties: {
name: { type: 'string' },
address: { $ref: '#/definitions/address' },
},
definitions: {
address: {
type: 'object',
properties: {
street: { type: 'string' },
city: { type: 'string' },
},
},
},
};
expect(hasCycleInSchema(schema)).toBe(false);
});
it('should handle non-cyclic sibling refs', () => {
const schema = {
properties: {
a: { $ref: '#/definitions/stringDef' },
b: { $ref: '#/definitions/stringDef' },
},
definitions: {
stringDef: { type: 'string' },
},
};
expect(hasCycleInSchema(schema)).toBe(false);
});
it('should handle nested but not cyclic refs', () => {
const schema = {
properties: {
a: { $ref: '#/definitions/defA' },
},
definitions: {
defA: { properties: { b: { $ref: '#/definitions/defB' } } },
defB: { type: 'string' },
},
};
expect(hasCycleInSchema(schema)).toBe(false);
});
it('should return false for an empty schema', () => {
expect(hasCycleInSchema({})).toBe(false);
});
});

View File

@@ -4,105 +4,276 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { FunctionDeclaration, PartListUnion, Schema } from '@google/genai';
import { FunctionDeclaration, PartListUnion } from '@google/genai';
import { ToolErrorType } from './tool-error.js';
import { DiffUpdateResult } from '../ide/ideContext.js';
/**
* Interface representing the base Tool functionality
* Represents a validated and ready-to-execute tool call.
* An instance of this is created by a `ToolBuilder`.
*/
export interface Tool<
TParams = unknown,
TResult extends ToolResult = ToolResult,
export interface ToolInvocation<
TParams extends object,
TResult extends ToolResult,
> {
/**
* The internal name of the tool (used for API calls)
* The validated parameters for this specific invocation.
*/
name: string;
params: TParams;
/**
* The user-friendly display name of the tool
* Gets a pre-execution description of the tool operation.
* @returns A markdown string describing what the tool will do.
*/
displayName: string;
getDescription(): string;
/**
* Description of what the tool does
* Determines what file system paths the tool will affect.
* @returns A list of such paths.
*/
description: string;
toolLocations(): ToolLocation[];
/**
* The icon to display when interacting via ACP
*/
icon: Icon;
/**
* Function declaration schema from @google/genai
*/
schema: FunctionDeclaration;
/**
* Whether the tool's output should be rendered as markdown
*/
isOutputMarkdown: boolean;
/**
* Whether the tool supports live (streaming) output
*/
canUpdateOutput: boolean;
/**
* Validates the parameters for the tool
* Should be called from both `shouldConfirmExecute` and `execute`
* `shouldConfirmExecute` should return false immediately if invalid
* @param params Parameters to validate
* @returns An error message string if invalid, null otherwise
*/
validateToolParams(params: TParams): string | null;
/**
* Gets a pre-execution description of the tool operation
* @param params Parameters for the tool execution
* @returns A markdown string describing what the tool will do
* Optional for backward compatibility
*/
getDescription(params: TParams): string;
/**
* Determines what file system paths the tool will affect
* @param params Parameters for the tool execution
* @returns A list of such paths
*/
toolLocations(params: TParams): ToolLocation[];
/**
* Determines if the tool should prompt for confirmation before execution
* @param params Parameters for the tool execution
* @returns Whether execute should be confirmed.
* Determines if the tool should prompt for confirmation before execution.
* @returns Confirmation details or false if no confirmation is needed.
*/
shouldConfirmExecute(
params: TParams,
abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails | false>;
/**
* Executes the tool with the given parameters
* @param params Parameters for the tool execution
* @returns Result of the tool execution
* Executes the tool with the validated parameters.
* @param signal AbortSignal for tool cancellation.
* @param updateOutput Optional callback to stream output.
* @returns Result of the tool execution.
*/
execute(
params: TParams,
signal: AbortSignal,
updateOutput?: (output: string) => void,
): Promise<TResult>;
}
/**
* A convenience base class for ToolInvocation.
*/
export abstract class BaseToolInvocation<
TParams extends object,
TResult extends ToolResult,
> implements ToolInvocation<TParams, TResult>
{
constructor(readonly params: TParams) {}
abstract getDescription(): string;
toolLocations(): ToolLocation[] {
return [];
}
shouldConfirmExecute(
_abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails | false> {
return Promise.resolve(false);
}
abstract execute(
signal: AbortSignal,
updateOutput?: (output: string) => void,
): Promise<TResult>;
}
/**
* A type alias for a tool invocation where the specific parameter and result types are not known.
*/
export type AnyToolInvocation = ToolInvocation<object, ToolResult>;
/**
* An adapter that wraps the legacy `Tool` interface to make it compatible
* with the new `ToolInvocation` pattern.
*/
export class LegacyToolInvocation<
TParams extends object,
TResult extends ToolResult,
> implements ToolInvocation<TParams, TResult>
{
constructor(
private readonly legacyTool: BaseTool<TParams, TResult>,
readonly params: TParams,
) {}
getDescription(): string {
return this.legacyTool.getDescription(this.params);
}
toolLocations(): ToolLocation[] {
return this.legacyTool.toolLocations(this.params);
}
shouldConfirmExecute(
abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails | false> {
return this.legacyTool.shouldConfirmExecute(this.params, abortSignal);
}
execute(
signal: AbortSignal,
updateOutput?: (output: string) => void,
): Promise<TResult> {
return this.legacyTool.execute(this.params, signal, updateOutput);
}
}
/**
* Interface for a tool builder that validates parameters and creates invocations.
*/
export interface ToolBuilder<
TParams extends object,
TResult extends ToolResult,
> {
/**
* The internal name of the tool (used for API calls).
*/
name: string;
/**
* The user-friendly display name of the tool.
*/
displayName: string;
/**
* Description of what the tool does.
*/
description: string;
/**
* The icon to display when interacting via ACP.
*/
icon: Icon;
/**
* Function declaration schema from @google/genai.
*/
schema: FunctionDeclaration;
/**
* Whether the tool's output should be rendered as markdown.
*/
isOutputMarkdown: boolean;
/**
* Whether the tool supports live (streaming) output.
*/
canUpdateOutput: boolean;
/**
* Validates raw parameters and builds a ready-to-execute invocation.
* @param params The raw, untrusted parameters from the model.
* @returns A valid `ToolInvocation` if successful. Throws an error if validation fails.
*/
build(params: TParams): ToolInvocation<TParams, TResult>;
}
/**
* New base class for tools that separates validation from execution.
* New tools should extend this class.
*/
export abstract class DeclarativeTool<
TParams extends object,
TResult extends ToolResult,
> implements ToolBuilder<TParams, TResult>
{
constructor(
readonly name: string,
readonly displayName: string,
readonly description: string,
readonly icon: Icon,
readonly parameterSchema: unknown,
readonly isOutputMarkdown: boolean = true,
readonly canUpdateOutput: boolean = false,
) {}
get schema(): FunctionDeclaration {
return {
name: this.name,
description: this.description,
parametersJsonSchema: this.parameterSchema,
};
}
/**
* Validates the raw tool parameters.
* Subclasses should override this to add custom validation logic
* beyond the JSON schema check.
* @param params The raw parameters from the model.
* @returns An error message string if invalid, null otherwise.
*/
protected validateToolParams(_params: TParams): string | null {
// Base implementation can be extended by subclasses.
return null;
}
/**
* The core of the new pattern. It validates parameters and, if successful,
* returns a `ToolInvocation` object that encapsulates the logic for the
* specific, validated call.
* @param params The raw, untrusted parameters from the model.
* @returns A `ToolInvocation` instance.
*/
abstract build(params: TParams): ToolInvocation<TParams, TResult>;
/**
* A convenience method that builds and executes the tool in one step.
* Throws an error if validation fails.
* @param params The raw, untrusted parameters from the model.
* @param signal AbortSignal for tool cancellation.
* @param updateOutput Optional callback to stream output.
* @returns The result of the tool execution.
*/
async buildAndExecute(
params: TParams,
signal: AbortSignal,
updateOutput?: (output: string) => void,
): Promise<TResult> {
const invocation = this.build(params);
return invocation.execute(signal, updateOutput);
}
}
/**
* New base class for declarative tools that separates validation from execution.
* New tools should extend this class, which provides a `build` method that
* validates parameters before deferring to a `createInvocation` method for
* the final `ToolInvocation` object instantiation.
*/
export abstract class BaseDeclarativeTool<
TParams extends object,
TResult extends ToolResult,
> extends DeclarativeTool<TParams, TResult> {
build(params: TParams): ToolInvocation<TParams, TResult> {
const validationError = this.validateToolParams(params);
if (validationError) {
throw new Error(validationError);
}
return this.createInvocation(params);
}
protected abstract createInvocation(
params: TParams,
): ToolInvocation<TParams, TResult>;
}
/**
* A type alias for a declarative tool where the specific parameter and result types are not known.
*/
export type AnyDeclarativeTool = DeclarativeTool<object, ToolResult>;
/**
* Base implementation for tools with common functionality
* @deprecated Use `DeclarativeTool` for new tools.
*/
export abstract class BaseTool<
TParams = unknown,
TParams extends object,
TResult extends ToolResult = ToolResult,
> implements Tool<TParams, TResult>
{
> extends DeclarativeTool<TParams, TResult> {
/**
* Creates a new instance of BaseTool
* @param name Internal name of the tool (used for API calls)
@@ -110,27 +281,34 @@ export abstract class BaseTool<
* @param description Description of what the tool does
* @param isOutputMarkdown Whether the tool's output should be rendered as markdown
* @param canUpdateOutput Whether the tool supports live (streaming) output
* @param parameterSchema Open API 3.0 Schema defining the parameters
* @param parameterSchema JSON Schema defining the parameters
*/
constructor(
readonly name: string,
readonly displayName: string,
readonly description: string,
readonly icon: Icon,
readonly parameterSchema: Schema,
readonly parameterSchema: unknown,
readonly isOutputMarkdown: boolean = true,
readonly canUpdateOutput: boolean = false,
) {}
) {
super(
name,
displayName,
description,
icon,
parameterSchema,
isOutputMarkdown,
canUpdateOutput,
);
}
/**
* Function declaration schema computed from name, description, and parameterSchema
*/
get schema(): FunctionDeclaration {
return {
name: this.name,
description: this.description,
parameters: this.parameterSchema,
};
build(params: TParams): ToolInvocation<TParams, TResult> {
const validationError = this.validateToolParams(params);
if (validationError) {
throw new Error(validationError);
}
return new LegacyToolInvocation(this, params);
}
/**
@@ -228,6 +406,91 @@ export interface ToolResult {
};
}
/**
* Detects cycles in a JSON schemas due to `$ref`s.
* @param schema The root of the JSON schema.
* @returns `true` if a cycle is detected, `false` otherwise.
*/
export function hasCycleInSchema(schema: object): boolean {
function resolveRef(ref: string): object | null {
if (!ref.startsWith('#/')) {
return null;
}
const path = ref.substring(2).split('/');
let current: unknown = schema;
for (const segment of path) {
if (
typeof current !== 'object' ||
current === null ||
!Object.prototype.hasOwnProperty.call(current, segment)
) {
return null;
}
current = (current as Record<string, unknown>)[segment];
}
return current as object;
}
function traverse(
node: unknown,
visitedRefs: Set<string>,
pathRefs: Set<string>,
): boolean {
if (typeof node !== 'object' || node === null) {
return false;
}
if (Array.isArray(node)) {
for (const item of node) {
if (traverse(item, visitedRefs, pathRefs)) {
return true;
}
}
return false;
}
if ('$ref' in node && typeof node.$ref === 'string') {
const ref = node.$ref;
if (ref === '#/' || pathRefs.has(ref)) {
// A ref to just '#/' is always a cycle.
return true; // Cycle detected!
}
if (visitedRefs.has(ref)) {
return false; // Bail early, we have checked this ref before.
}
const resolvedNode = resolveRef(ref);
if (resolvedNode) {
// Add it to both visited and the current path
visitedRefs.add(ref);
pathRefs.add(ref);
const hasCycle = traverse(resolvedNode, visitedRefs, pathRefs);
pathRefs.delete(ref); // Backtrack, leaving it in visited
return hasCycle;
}
}
// Crawl all the properties of node
for (const key in node) {
if (Object.prototype.hasOwnProperty.call(node, key)) {
if (
traverse(
(node as Record<string, unknown>)[key],
visitedRefs,
pathRefs,
)
) {
return true;
}
}
}
return false;
}
return traverse(schema, new Set<string>(), new Set<string>());
}
export type ToolResultDisplay = string | FileDiff;
export interface FileDiff {
@@ -235,6 +498,14 @@ export interface FileDiff {
fileName: string;
originalContent: string | null;
newContent: string;
diffStat?: DiffStat;
}
export interface DiffStat {
ai_removed_lines: number;
ai_added_lines: number;
user_added_lines: number;
user_removed_lines: number;
}
export interface ToolEditConfirmationDetails {
@@ -245,10 +516,12 @@ export interface ToolEditConfirmationDetails {
payload?: ToolConfirmationPayload,
) => Promise<void>;
fileName: string;
filePath: string;
fileDiff: string;
originalContent: string | null;
newContent: string;
isModifying?: boolean;
ideConfirmation?: Promise<DiffUpdateResult>;
}
export interface ToolConfirmationPayload {

View File

@@ -12,7 +12,6 @@ import {
ToolConfirmationOutcome,
Icon,
} from './tools.js';
import { Type } from '@google/genai';
import { Config, ApprovalMode } from '../config/config.js';
import { getResponseText } from '../utils/generateContentResponseUtilities.js';
import { fetchWithTimeout } from '../utils/fetch.js';
@@ -52,15 +51,15 @@ export class WebFetchTool extends BaseTool<WebFetchToolParams, ToolResult> {
properties: {
url: {
description: 'The URL to fetch content from',
type: Type.STRING,
type: 'string',
},
prompt: {
description: 'The prompt to run on the fetched content',
type: Type.STRING,
type: 'string',
},
},
required: ['url', 'prompt'],
type: Type.OBJECT,
type: 'object',
},
);
const proxy = config.getProxy();
@@ -127,7 +126,10 @@ ${textContent}
}
validateParams(params: WebFetchToolParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}

View File

@@ -75,7 +75,10 @@ export class WebSearchTool extends BaseTool<
* @returns An error message string if validation fails, null if valid
*/
validateParams(params: WebSearchToolParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}

View File

@@ -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,
@@ -55,6 +56,9 @@ const mockConfigInternal = {
getApprovalMode: vi.fn(() => ApprovalMode.DEFAULT),
setApprovalMode: vi.fn(),
getGeminiClient: vi.fn(), // Initialize as a plain mock function
getIdeClient: vi.fn(),
getIdeMode: vi.fn(() => false),
getIdeModeFeature: vi.fn(() => false),
getWorkspaceContext: () => createMockWorkspaceContext(rootDir),
getApiKey: () => 'test-key',
getModel: () => 'test-model',
@@ -110,6 +114,14 @@ describe('WriteFileTool', () => {
mockConfigInternal.getGeminiClient.mockReturnValue(
mockGeminiClientInstance,
);
mockConfigInternal.getIdeClient.mockReturnValue({
openDiff: vi.fn(),
closeDiff: vi.fn(),
getIdeContext: vi.fn(),
subscribeToIdeContext: vi.fn(),
isCodeTrackerEnabled: vi.fn(),
getTrackedCode: vi.fn(),
});
tool = new WriteFileTool(mockConfig);
@@ -453,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.returnDisplay).toContain(
'Error: File path must be within one of the workspace directories',
expect(result.llmContent).toContain(
'Could not write file due to invalid parameters:',
);
expect(result.returnDisplay).toContain(
'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 () => {
@@ -479,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);
@@ -500,7 +526,11 @@ describe('WriteFileTool', () => {
params,
abortSignal,
);
if (typeof confirmDetails === 'object' && confirmDetails.onConfirm) {
if (
typeof confirmDetails === 'object' &&
'onConfirm' in confirmDetails &&
confirmDetails.onConfirm
) {
await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce);
}
@@ -554,7 +584,11 @@ describe('WriteFileTool', () => {
params,
abortSignal,
);
if (typeof confirmDetails === 'object' && confirmDetails.onConfirm) {
if (
typeof confirmDetails === 'object' &&
'onConfirm' in confirmDetails &&
confirmDetails.onConfirm
) {
await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce);
}
@@ -595,7 +629,11 @@ describe('WriteFileTool', () => {
params,
abortSignal,
);
if (typeof confirmDetails === 'object' && confirmDetails.onConfirm) {
if (
typeof confirmDetails === 'object' &&
'onConfirm' in confirmDetails &&
confirmDetails.onConfirm
) {
await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce);
}
@@ -686,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');
});
});
});

View File

@@ -17,7 +17,7 @@ import {
ToolCallConfirmationDetails,
Icon,
} from './tools.js';
import { Type } from '@google/genai';
import { ToolErrorType } from './tool-error.js';
import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js';
@@ -25,13 +25,14 @@ import {
ensureCorrectEdit,
ensureCorrectFileContent,
} from '../utils/editCorrector.js';
import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js';
import { ModifiableTool, ModifyContext } from './modifiable-tool.js';
import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js';
import { ModifiableDeclarativeTool, ModifyContext } from './modifiable-tool.js';
import { getSpecificMimeType } from '../utils/fileUtils.js';
import {
recordFileOperationMetric,
FileOperation,
} from '../telemetry/metrics.js';
import { IDEConnectionStatus } from '../ide/ide-client.js';
/**
* Parameters for the WriteFile tool
@@ -51,6 +52,11 @@ export interface WriteFileToolParams {
* Whether the proposed content was modified by the user.
*/
modified_by_user?: boolean;
/**
* Initially proposed content.
*/
ai_proposed_content?: string;
}
interface GetCorrectedFileContentResult {
@@ -65,7 +71,7 @@ interface GetCorrectedFileContentResult {
*/
export class WriteFileTool
extends BaseTool<WriteFileToolParams, ToolResult>
implements ModifiableTool<WriteFileToolParams>
implements ModifiableDeclarativeTool<WriteFileToolParams>
{
static readonly Name: string = 'write_file';
@@ -82,21 +88,24 @@ export class WriteFileTool
file_path: {
description:
"The absolute path to the file to write to (e.g., '/home/user/project/file.txt'). Relative paths are not supported.",
type: Type.STRING,
type: 'string',
},
content: {
description: 'The content to write to the file.',
type: Type.STRING,
type: 'string',
},
},
required: ['file_path', 'content'],
type: Type.OBJECT,
type: 'object',
},
);
}
validateToolParams(params: WriteFileToolParams): string | null {
const errors = SchemaValidator.validate(this.schema.parameters, params);
const errors = SchemaValidator.validate(
this.schema.parametersJsonSchema,
params,
);
if (errors) {
return errors;
}
@@ -184,10 +193,19 @@ export class WriteFileTool
DEFAULT_DIFF_OPTIONS,
);
const ideClient = this.config.getIdeClient();
const ideConfirmation =
this.config.getIdeModeFeature() &&
this.config.getIdeMode() &&
ideClient.getConnectionStatus().status === IDEConnectionStatus.Connected
? ideClient.openDiff(params.file_path, correctedContent)
: undefined;
const confirmationDetails: ToolEditConfirmationDetails = {
type: 'edit',
title: `Confirm Write: ${shortenPath(relativePath)}`,
fileName,
filePath: params.file_path,
fileDiff,
originalContent,
newContent: correctedContent,
@@ -195,7 +213,15 @@ export class WriteFileTool
if (outcome === ToolConfirmationOutcome.ProceedAlways) {
this.config.setApprovalMode(ApprovalMode.AUTO_EDIT);
}
if (ideConfirmation) {
const result = await ideConfirmation;
if (result.status === 'accepted' && result.content) {
params.content = result.content;
}
}
},
ideConfirmation,
};
return confirmationDetails;
}
@@ -207,8 +233,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,
},
};
}
@@ -220,10 +250,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,
},
};
}
@@ -265,6 +301,15 @@ export class WriteFileTool
DEFAULT_DIFF_OPTIONS,
);
const originallyProposedContent =
params.ai_proposed_content || params.content;
const diffStat = getDiffStat(
fileName,
currentContentForDiff,
originallyProposedContent,
params.content,
);
const llmSuccessMessageParts = [
isNewFile
? `Successfully created and wrote to new file: ${params.file_path}.`
@@ -281,6 +326,7 @@ export class WriteFileTool
fileName,
originalContent: correctedContentResult.originalContent,
newContent: correctedContentResult.correctedContent,
diffStat,
};
const lines = fileContent.split('\n').length;
@@ -293,6 +339,7 @@ export class WriteFileTool
lines,
mimetype,
extension,
diffStat,
);
} else {
recordFileOperationMetric(
@@ -301,6 +348,7 @@ export class WriteFileTool
lines,
mimetype,
extension,
diffStat,
);
}
@@ -309,10 +357,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,
},
};
}
}
@@ -400,11 +481,15 @@ export class WriteFileTool
_oldContent: string,
modifiedProposedContent: string,
originalParams: WriteFileToolParams,
) => ({
...originalParams,
content: modifiedProposedContent,
modified_by_user: true,
}),
) => {
const content = originalParams.content;
return {
...originalParams,
ai_proposed_content: content,
content: modifiedProposedContent,
modified_by_user: true,
};
},
};
}
}