diff --git a/src/components/Messages/ToolMessage.tsx b/src/components/Messages/ToolMessage.tsx index f46c8f2c0..ba1f819b2 100644 --- a/src/components/Messages/ToolMessage.tsx +++ b/src/components/Messages/ToolMessage.tsx @@ -13,9 +13,9 @@ import type { BashToolResult, FileReadToolArgs, FileReadToolResult, + FileEditReplaceStringToolArgs, FileEditInsertToolArgs, FileEditInsertToolResult, - FileEditReplaceStringToolArgs, FileEditReplaceStringToolResult, FileEditReplaceLinesToolArgs, FileEditReplaceLinesToolResult, @@ -121,26 +121,26 @@ export const ToolMessage: React.FC = ({ message, className, wo ); } - if (isFileEditReplaceLinesTool(message.toolName, message.args)) { + if (isFileEditInsertTool(message.toolName, message.args)) { return (
); } - if (isFileEditInsertTool(message.toolName, message.args)) { + if (isFileEditReplaceLinesTool(message.toolName, message.args)) { return (
diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 01b14dc87..94113cae5 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -5,32 +5,21 @@ import * as os from "os"; import { createFileEditInsertTool } from "./file_edit_insert"; import type { FileEditInsertToolArgs, FileEditInsertToolResult } from "@/types/tools"; import type { ToolCallOptions } from "ai"; -import { TestTempDir, getTestDeps } from "./testHelpers"; import { createRuntime } from "@/runtime/runtimeFactory"; +import { getTestDeps } from "./testHelpers"; -// Mock ToolCallOptions for testing const mockToolCallOptions: ToolCallOptions = { toolCallId: "test-call-id", messages: [], }; -// Helper to create file_edit_insert tool with test configuration -// Returns both tool and disposable temp directory -function createTestFileEditInsertTool(options?: { cwd?: string }) { - const tempDir = new TestTempDir("test-file-edit-insert"); - const tool = createFileEditInsertTool({ +function createTestTool(cwd: string) { + return createFileEditInsertTool({ ...getTestDeps(), - cwd: options?.cwd ?? process.cwd(), - runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }), - runtimeTempDir: tempDir.path, + cwd, + runtime: createRuntime({ type: "local", srcBaseDir: cwd }), + runtimeTempDir: cwd, }); - - return { - tool, - [Symbol.dispose]() { - tempDir[Symbol.dispose](); - }, - }; } describe("file_edit_insert tool", () => { @@ -38,368 +27,100 @@ describe("file_edit_insert tool", () => { let testFilePath: string; beforeEach(async () => { - // Create a temporary directory for test files - testDir = await fs.mkdtemp(path.join(os.tmpdir(), "fileEditInsert-test-")); + testDir = await fs.mkdtemp(path.join(os.tmpdir(), "file-edit-insert-")); testFilePath = path.join(testDir, "test.txt"); + await fs.writeFile(testFilePath, "Line 1\nLine 3"); }); afterEach(async () => { - // Clean up test directory await fs.rm(testDir, { recursive: true, force: true }); }); - it("should insert content at the top of the file (line_offset = 0)", async () => { - // Setup - const initialContent = "line1\nline2\nline3"; - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 0, - content: "INSERTED", - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("INSERTED\nline1\nline2\nline3"); - }); - - it("should insert content after line 1 (line_offset = 1)", async () => { - // Setup - const initialContent = "line1\nline2\nline3"; - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 1, - content: "INSERTED", - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("line1\nINSERTED\nline2\nline3"); - }); - - it("should insert content after line 2 (line_offset = 2)", async () => { - // Setup - const initialContent = "line1\nline2\nline3"; - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 2, - content: "INSERTED", - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("line1\nline2\nINSERTED\nline3"); - }); - - it("should insert content at the end of the file (line_offset = line count)", async () => { - // Setup - const initialContent = "line1\nline2\nline3"; - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 3, - content: "INSERTED", - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("line1\nline2\nline3\nINSERTED"); - }); - - it("should insert multiline content", async () => { - // Setup - const initialContent = "line1\nline2"; - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 1, - content: "INSERTED1\nINSERTED2", - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("line1\nINSERTED1\nINSERTED2\nline2"); - }); - - it("should insert content into empty file", async () => { - // Setup - await fs.writeFile(testFilePath, ""); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 0, - content: "INSERTED", - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("INSERTED\n"); - }); - - it("should fail when file does not exist and create is not set", async () => { - // Setup - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "nonexistent.txt", // Use relative path - line_offset: 0, - content: "INSERTED", - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("File not found"); - expect(result.error).toContain("set create: true"); - } - }); - - it("should create file when create is true and file does not exist", async () => { - // Setup - const tool = createFileEditInsertTool({ - ...getTestDeps(), - cwd: testDir, - runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }), - runtimeTempDir: "/tmp", - }); + it("inserts content using before guard", async () => { + const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { - file_path: "newfile.txt", // Use relative path - line_offset: 0, - content: "INSERTED", - create: true, + file_path: path.relative(testDir, testFilePath), + content: "Line 2\n", + before: "Line 1\n", }; - // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - // Assert expect(result.success).toBe(true); - - const fileContent = await fs.readFile(path.join(testDir, "newfile.txt"), "utf-8"); - expect(fileContent).toBe("INSERTED\n"); + const updated = await fs.readFile(testFilePath, "utf-8"); + expect(updated).toBe("Line 1\nLine 2\nLine 3"); }); - it("should create parent directories when create is true", async () => { - // Setup - const tool = createFileEditInsertTool({ - ...getTestDeps(), - cwd: testDir, - runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }), - runtimeTempDir: "/tmp", - }); + it("inserts content using after guard", async () => { + const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { - file_path: "nested/dir/newfile.txt", // Use relative path - line_offset: 0, - content: "INSERTED", - create: true, + file_path: path.relative(testDir, testFilePath), + content: "Header\n", + after: "Line 1", }; - // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert expect(result.success).toBe(true); - - const fileContent = await fs.readFile(path.join(testDir, "nested/dir/newfile.txt"), "utf-8"); - expect(fileContent).toBe("INSERTED\n"); + expect(await fs.readFile(testFilePath, "utf-8")).toBe("Header\nLine 1\nLine 3"); }); - it("should work normally with create: true when file already exists", async () => { - // Setup - const initialContent = "line1\nline2"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditInsertTool({ - ...getTestDeps(), - cwd: testDir, - runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }), - runtimeTempDir: "/tmp", - }); + it("fails when guard matches multiple times", async () => { + await fs.writeFile(testFilePath, "repeat\nrepeat\nrepeat\n"); + const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 1, - content: "INSERTED", - create: true, + file_path: path.relative(testDir, testFilePath), + content: "middle\n", + before: "repeat\n", }; - // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("line1\nINSERTED\nline2"); - }); - - it("should fail when line_offset is negative", async () => { - // Setup - const initialContent = "line1\nline2"; - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: -1, - content: "INSERTED", - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain("must be non-negative"); + expect(result.error).toContain("multiple times"); } }); - it("should fail when line_offset exceeds file length", async () => { - // Setup - const initialContent = "line1\nline2"; - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; + it("fails when both before and after are provided", async () => { + const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 10, // File only has 2 lines - content: "INSERTED", + file_path: path.relative(testDir, testFilePath), + content: "oops", + before: "Line 1", + after: "Line 3", }; - // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain("beyond file length"); + expect(result.error).toContain("only one of before or after"); } }); - it("should handle content with trailing newline correctly (no double newlines)", async () => { - // This test verifies the fix for the terminal-bench "hello-world" bug - // where content with \n at the end was getting an extra newline added - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; + it("creates new file when create flag is provided", async () => { + const newFile = path.join(testDir, "new.txt"); + const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { - file_path: "newfile.txt", - line_offset: 0, - content: "Hello, world!\n", // Content already has trailing newline + file_path: path.relative(testDir, newFile), + content: "Hello world!\n", create: true, }; - // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert expect(result.success).toBe(true); - - const fileContent = await fs.readFile(path.join(testDir, "newfile.txt"), "utf-8"); - // Should NOT have double newline - the trailing \n in content should be preserved as-is - expect(fileContent).toBe("Hello, world!\n"); - expect(fileContent).not.toBe("Hello, world!\n\n"); + expect(await fs.readFile(newFile, "utf-8")).toBe("Hello world!\n"); }); - it("should handle multiline content with trailing newline", async () => { - // Setup - const initialContent = "line1\nline2"; - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; + it("fails when no guards are provided", async () => { + const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { - file_path: "test.txt", - line_offset: 1, - content: "INSERTED1\nINSERTED2\n", // Multiline with trailing newline + file_path: path.relative(testDir, testFilePath), + content: "noop", }; - // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - // Should respect the trailing newline in content - expect(updatedContent).toBe("line1\nINSERTED1\nINSERTED2\nline2"); - }); - - it("should preserve trailing newline when appending to file without trailing newline", async () => { - // Regression test for Codex feedback: when inserting at EOF with content ending in \n, - // the newline should be preserved even if the original file doesn't end with one - const initialContent = "line1\nline2"; // No trailing newline - await fs.writeFile(testFilePath, initialContent); - - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); - const tool = testEnv.tool; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", - line_offset: 2, // Append at end - content: "line3\n", // With trailing newline - }; - - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - - // Assert - expect(result.success).toBe(true); - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - // Should preserve the trailing newline from content even at EOF - expect(updatedContent).toBe("line1\nline2\nline3\n"); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Provide either a before or after guard"); + } }); }); diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 87d3e1a30..524a634ce 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -1,35 +1,63 @@ import { tool } from "ai"; -import type { FileEditInsertToolResult } from "@/types/tools"; +import type { FileEditInsertToolArgs, FileEditInsertToolResult } from "@/types/tools"; +import { EDIT_FAILED_NOTE_PREFIX, NOTE_READ_FILE_RETRY } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { validatePathInCwd, validateAndCorrectPath } from "./fileCommon"; -import { EDIT_FAILED_NOTE_PREFIX, NOTE_READ_FILE_RETRY } from "@/types/tools"; +import { generateDiff, validateAndCorrectPath, validatePathInCwd } from "./fileCommon"; import { executeFileEditOperation } from "./file_edit_operation"; -import { RuntimeError } from "@/runtime/Runtime"; import { fileExists } from "@/utils/runtime/fileExists"; import { writeFileString } from "@/utils/runtime/helpers"; +import { RuntimeError } from "@/runtime/Runtime"; + +const READ_AND_RETRY_NOTE = `${EDIT_FAILED_NOTE_PREFIX} ${NOTE_READ_FILE_RETRY}`; + +interface InsertOperationSuccess { + success: true; + newContent: string; + metadata: Record; +} + +interface InsertOperationFailure { + success: false; + error: string; + note?: string; +} + +interface InsertContentOptions { + before?: string; + after?: string; +} + +interface GuardResolutionSuccess { + success: true; + index: number; +} + +function guardFailure(error: string): InsertOperationFailure { + return { + success: false, + error, + note: READ_AND_RETRY_NOTE, + }; +} + +type GuardAnchors = Pick; -/** - * File edit insert tool factory for AI assistant - * Creates a tool that allows the AI to insert content at a specific line position - * @param config Required configuration including working directory - */ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) => { return tool({ description: TOOL_DEFINITIONS.file_edit_insert.description, inputSchema: TOOL_DEFINITIONS.file_edit_insert.schema, execute: async ( - { file_path, line_offset, content, create }, + { file_path, content, before, after, create }: FileEditInsertToolArgs, { abortSignal } ): Promise => { try { - // Validate and auto-correct redundant path prefix - const { correctedPath: validatedPath, warning: pathWarning } = validateAndCorrectPath( + const { correctedPath, warning: pathWarning } = validateAndCorrectPath( file_path, config.cwd, config.runtime ); - file_path = validatedPath; + file_path = correctedPath; const pathValidation = validatePathInCwd(file_path, config.cwd, config.runtime); if (pathValidation) { @@ -39,32 +67,20 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }; } - if (line_offset < 0) { - return { - success: false, - error: `line_offset must be non-negative (got ${line_offset})`, - note: `${EDIT_FAILED_NOTE_PREFIX} The line_offset must be >= 0.`, - }; - } - - // Use runtime's normalizePath method to resolve paths correctly for both local and SSH runtimes const resolvedPath = config.runtime.normalizePath(file_path, config.cwd); - - // Check if file exists using runtime const exists = await fileExists(config.runtime, resolvedPath, abortSignal); if (!exists) { if (!create) { return { success: false, - error: `File not found: ${file_path}. To create it, set create: true`, + error: `File not found: ${file_path}. Set create: true to create it.`, note: `${EDIT_FAILED_NOTE_PREFIX} File does not exist. Set create: true to create it, or check the file path.`, }; } - // Create empty file using runtime helper try { - await writeFileString(config.runtime, resolvedPath, "", abortSignal); + await writeFileString(config.runtime, resolvedPath, content, abortSignal); } catch (err) { if (err instanceof RuntimeError) { return { @@ -74,55 +90,25 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) } throw err; } + + const diff = generateDiff(resolvedPath, "", content); + return { + success: true, + diff, + ...(pathWarning && { warning: pathWarning }), + }; } - const result = await executeFileEditOperation({ + return executeFileEditOperation({ config, filePath: file_path, abortSignal, - operation: (originalContent) => { - const lines = originalContent.split("\n"); - - if (line_offset > lines.length) { - return { - success: false, - error: `line_offset ${line_offset} is beyond file length (${lines.length} lines)`, - note: `${EDIT_FAILED_NOTE_PREFIX} The file has ${lines.length} lines. ${NOTE_READ_FILE_RETRY}`, - }; - } - - // Handle newline behavior: - // - If content ends with \n and we're not at EOF, strip it (join will add it back) - // - If content ends with \n and we're at EOF, keep it (join won't add trailing newline) - // - If content doesn't end with \n, keep as-is (join will add newlines between lines) - const contentEndsWithNewline = content.endsWith("\n"); - const insertingAtEnd = line_offset === lines.length; - const shouldStripTrailingNewline = contentEndsWithNewline && !insertingAtEnd; - const normalizedContent = shouldStripTrailingNewline ? content.slice(0, -1) : content; - - const newLines = [ - ...lines.slice(0, line_offset), - normalizedContent, - ...lines.slice(line_offset), - ]; - const newContent = newLines.join("\n"); - - return { - success: true, - newContent, - metadata: {}, - }; - }, + operation: (originalContent) => + insertContent(originalContent, content, { + before, + after, + }), }); - - // Add path warning if present - if (pathWarning && result.success) { - return { - ...result, - warning: pathWarning, - }; - } - return result; } catch (error) { if (error && typeof error === "object" && "code" in error && error.code === "EACCES") { return { @@ -140,3 +126,86 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }, }); }; + +function insertContent( + originalContent: string, + contentToInsert: string, + options: InsertContentOptions +): InsertOperationSuccess | InsertOperationFailure { + const { before, after } = options; + + if (before !== undefined && after !== undefined) { + return guardFailure("Provide only one of before or after (not both)."); + } + + if (before === undefined && after === undefined) { + return guardFailure("Provide either a before or after guard to anchor the insertion point."); + } + + return insertWithGuards(originalContent, contentToInsert, { before, after }); +} + +function insertWithGuards( + originalContent: string, + contentToInsert: string, + anchors: GuardAnchors +): InsertOperationSuccess | InsertOperationFailure { + const anchorResult = resolveGuardAnchor(originalContent, anchors); + if (!anchorResult.success) { + return anchorResult; + } + + const newContent = + originalContent.slice(0, anchorResult.index) + + contentToInsert + + originalContent.slice(anchorResult.index); + + return { + success: true, + newContent, + metadata: {}, + }; +} + +function findUniqueSubstringIndex( + haystack: string, + needle: string, + label: "before" | "after" +): GuardResolutionSuccess | InsertOperationFailure { + const firstIndex = haystack.indexOf(needle); + if (firstIndex === -1) { + return guardFailure(`Guard mismatch: unable to find ${label} substring in the current file.`); + } + + const secondIndex = haystack.indexOf(needle, firstIndex + needle.length); + if (secondIndex !== -1) { + return guardFailure( + `Guard mismatch: ${label} substring matched multiple times. Provide a more specific string.` + ); + } + + return { success: true, index: firstIndex }; +} + +function resolveGuardAnchor( + originalContent: string, + { before, after }: GuardAnchors +): GuardResolutionSuccess | InsertOperationFailure { + if (before !== undefined) { + const beforeIndexResult = findUniqueSubstringIndex(originalContent, before, "before"); + if (!beforeIndexResult.success) { + return beforeIndexResult; + } + return { success: true, index: beforeIndexResult.index + before.length }; + } + + if (after !== undefined) { + const afterIndexResult = findUniqueSubstringIndex(originalContent, after, "after"); + if (!afterIndexResult.success) { + return afterIndexResult; + } + return { success: true, index: afterIndexResult.index }; + } + + return guardFailure("Unable to determine insertion point from guards."); +} diff --git a/src/types/tools.ts b/src/types/tools.ts index b025b9219..7a060b233 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -71,6 +71,19 @@ export interface FileEditErrorResult { note?: string; // Agent-only message (not displayed in UI) } +export interface FileEditInsertToolArgs { + file_path: string; + content: string; + /** When true, create the file if it doesn't exist */ + create?: boolean; + /** Optional substring that must appear immediately before the insertion point */ + before?: string; + /** Optional substring that must appear immediately after the insertion point */ + after?: string; +} + +export type FileEditInsertToolResult = FileEditDiffSuccessBase | FileEditErrorResult; + export interface FileEditReplaceStringToolArgs { file_path: string; old_string: string; @@ -113,15 +126,6 @@ export const FILE_EDIT_TOOL_NAMES = [ export type FileEditToolName = (typeof FILE_EDIT_TOOL_NAMES)[number]; -export interface FileEditInsertToolArgs { - file_path: string; - line_offset: number; - content: string; - create?: boolean; -} - -export type FileEditInsertToolResult = FileEditDiffSuccessBase | FileEditErrorResult; - /** * Prefix for file write denial error messages. * This consistent prefix helps both the UI and models detect when writes fail. diff --git a/src/utils/messages/toolOutputRedaction.ts b/src/utils/messages/toolOutputRedaction.ts index 8f2da0e7e..51ffb79e0 100644 --- a/src/utils/messages/toolOutputRedaction.ts +++ b/src/utils/messages/toolOutputRedaction.ts @@ -36,10 +36,7 @@ function rewrapJsonContainer(wrapped: boolean, value: unknown): unknown { return value; } -// Narrowing helpers for our tool result types -function isFileEditResult( - v: unknown -): v is FileEditReplaceStringToolResult | FileEditReplaceLinesToolResult { +function isFileEditInsertResult(v: unknown): v is FileEditInsertToolResult { return ( typeof v === "object" && v !== null && @@ -47,8 +44,10 @@ function isFileEditResult( typeof (v as { success: unknown }).success === "boolean" ); } - -function isFileEditInsertResult(v: unknown): v is FileEditInsertToolResult { +// Narrowing helpers for our tool result types +function isFileEditResult( + v: unknown +): v is FileEditReplaceStringToolResult | FileEditReplaceLinesToolResult { return ( typeof v === "object" && v !== null && @@ -91,7 +90,7 @@ function redactFileEditInsert(output: unknown): unknown { const unwrapped = unwrapJsonContainer(output); const val = unwrapped.value; - if (!isFileEditInsertResult(val)) return output; // unknown structure, leave as-is + if (!isFileEditInsertResult(val)) return output; if (val.success) { const compact: FileEditInsertToolResult = { diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 90746cd84..e63f7f221 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -17,6 +17,10 @@ import { TOOL_EDIT_WARNING } from "@/types/tools"; import { zodToJsonSchema } from "zod-to-json-schema"; +const FILE_EDIT_FILE_PATH = z + .string() + .describe("Path to the file to edit (absolute or relative to the current workspace)"); + interface ToolSchema { name: string; description: string; @@ -73,7 +77,7 @@ export const TOOL_DEFINITIONS = { "⚠️ CRITICAL: Always check tool results - edits WILL fail if old_string is not found or unique. Do not proceed with dependent operations (commits, pushes, builds) until confirming success.\n\n" + "Apply one or more edits to a file by replacing exact text matches. All edits are applied sequentially. Each old_string must be unique in the file unless replace_count > 1 or replace_count is -1.", schema: z.object({ - file_path: z.string().describe("The absolute path to the file to edit"), + file_path: FILE_EDIT_FILE_PATH, old_string: z .string() .describe( @@ -94,7 +98,7 @@ export const TOOL_DEFINITIONS = { "⚠️ CRITICAL: Always check tool results - edits WILL fail if line numbers are invalid or file content has changed. Do not proceed with dependent operations (commits, pushes, builds) until confirming success.\n\n" + "Replace a range of lines in a file. Use this for line-based edits when you know the exact line numbers to modify.", schema: z.object({ - file_path: z.string().describe("The absolute path to the file to edit"), + file_path: FILE_EDIT_FILE_PATH, start_line: z.number().int().min(1).describe("1-indexed start line (inclusive) to replace"), end_line: z.number().int().min(1).describe("1-indexed end line (inclusive) to replace"), new_lines: z @@ -110,21 +114,38 @@ export const TOOL_DEFINITIONS = { }, file_edit_insert: { description: - "Insert content at a specific line position in a file. Line offset is 1-indexed: 0 inserts at the top, 1 inserts after line 1, etc. " + - `IMPORTANT: Edits may fail if line_offset is invalid or file doesn't exist. ${TOOL_EDIT_WARNING}`, - schema: z.object({ - file_path: z.string().describe("The absolute path to the file to edit"), - line_offset: z - .number() - .int() - .min(0) - .describe("1-indexed line position (0 = insert at top, N = insert after line N)"), - content: z.string().describe("The content to insert"), - create: z - .boolean() - .optional() - .describe("If true, create the file if it doesn't exist (default: false)"), - }), + "Insert content into a file using substring guards. " + + "Provide exactly one of before or after to anchor the operation when editing an existing file. " + + "Set create: true to write a brand new file without guards. " + + `Optional before/after substrings must uniquely match surrounding content. ${TOOL_EDIT_WARNING}`, + schema: z + .object({ + file_path: FILE_EDIT_FILE_PATH, + content: z.string().describe("The content to insert"), + create: z.boolean().optional().describe("If true, create the file if it does not exist"), + before: z + .string() + .min(1) + .optional() + .describe("Optional substring that must appear immediately before the insertion point"), + after: z + .string() + .min(1) + .optional() + .describe("Optional substring that must appear immediately after the insertion point"), + }) + .refine( + (data) => data.create === true || data.before !== undefined || data.after !== undefined, + { + message: + "Provide before or after when editing existing files, or set create: true to write a new file.", + path: ["before"], + } + ) + .refine((data) => !(data.before !== undefined && data.after !== undefined), { + message: "Provide only one of before or after (not both).", + path: ["before"], + }), }, propose_plan: { description: diff --git a/src/utils/tools/tools.ts b/src/utils/tools/tools.ts index a48be74b1..b86f70412 100644 --- a/src/utils/tools/tools.ts +++ b/src/utils/tools/tools.ts @@ -65,11 +65,11 @@ export async function getToolsForModel( const runtimeTools: Record = { file_read: wrap(createFileReadTool(config)), file_edit_replace_string: wrap(createFileEditReplaceStringTool(config)), + file_edit_insert: wrap(createFileEditInsertTool(config)), // DISABLED: file_edit_replace_lines - causes models (particularly GPT-5-Codex) // to leave repository in broken state due to issues with concurrent file modifications - // and line number miscalculations. Use file_edit_replace_string or file_edit_insert instead. + // and line number miscalculations. Use file_edit_replace_string instead. // file_edit_replace_lines: wrap(createFileEditReplaceLinesTool(config)), - file_edit_insert: wrap(createFileEditInsertTool(config)), bash: wrap(createBashTool(config)), }; diff --git a/tests/ipcMain/runtimeFileEditing.test.ts b/tests/ipcMain/runtimeFileEditing.test.ts index 13f29fc01..e53ed497c 100644 --- a/tests/ipcMain/runtimeFileEditing.test.ts +++ b/tests/ipcMain/runtimeFileEditing.test.ts @@ -325,7 +325,7 @@ describeIntegration("Runtime File Editing Tools", () => { const insertEvents = await sendMessageAndWait( env, workspaceId, - `Use the file_edit_insert or file_edit_replace_string tool to insert "Line 2" between Line 1 and Line 3 in ${testFileName}.`, + `Use the file_edit_insert (preferred) or file_edit_replace_string tool to insert "Line 2" between Line 1 and Line 3 in ${testFileName}.`, HAIKU_MODEL, FILE_TOOLS_ONLY, streamTimeout @@ -336,7 +336,7 @@ describeIntegration("Runtime File Editing Tools", () => { expect(streamEnd).toBeDefined(); expect((streamEnd as any).error).toBeUndefined(); - // Verify a file_edit tool was called (either insert or replace_string) + // Verify file_edit_insert (or fallback file_edit_replace_string) tool was called const toolCalls = insertEvents.filter( (e) => "type" in e && e.type === "tool-call-start" );