From 8bfcea33d042b987f6025fc43156c40769ab4bd9 Mon Sep 17 00:00:00 2001 From: root Date: Thu, 13 Nov 2025 19:40:15 +0000 Subject: [PATCH 1/8] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20remove=20file=5F?= =?UTF-8?q?edit=5Finsert=20tool?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file_edit_insert tool led to more issues than benefits (unreliable line offset calculations and edge cases). Standardize on file_edit_replace_string as the primary file editing tool. Changes: - Remove file_edit_insert tool and all references - Remove from tool definitions and type exports - Update UI components to only support replace_string and replace_lines - Update tests and tool policies - Update integration tests to use file_edit_replace_string All 987 unit tests pass; typecheck is clean. --- src/components/Messages/ToolMessage.tsx | 20 - src/components/tools/FileEditToolCall.tsx | 14 +- src/services/tools/file_edit_insert.test.ts | 405 -------------------- src/services/tools/file_edit_insert.ts | 142 ------- src/types/tools.ts | 18 +- src/utils/main/tokenizer.ts | 1 - src/utils/messages/toolOutputRedaction.ts | 29 -- src/utils/tools/toolDefinitions.ts | 20 - src/utils/tools/toolPolicy.test.ts | 15 +- src/utils/tools/tools.ts | 4 +- tests/ipcMain/runtimeFileEditing.test.ts | 9 +- 11 files changed, 11 insertions(+), 666 deletions(-) delete mode 100644 src/services/tools/file_edit_insert.test.ts delete mode 100644 src/services/tools/file_edit_insert.ts diff --git a/src/components/Messages/ToolMessage.tsx b/src/components/Messages/ToolMessage.tsx index f46c8f2c0..192cd5622 100644 --- a/src/components/Messages/ToolMessage.tsx +++ b/src/components/Messages/ToolMessage.tsx @@ -13,8 +13,6 @@ import type { BashToolResult, FileReadToolArgs, FileReadToolResult, - FileEditInsertToolArgs, - FileEditInsertToolResult, FileEditReplaceStringToolArgs, FileEditReplaceStringToolResult, FileEditReplaceLinesToolArgs, @@ -61,11 +59,6 @@ function isFileEditReplaceLinesTool( return TOOL_DEFINITIONS.file_edit_replace_lines.schema.safeParse(args).success; } -function isFileEditInsertTool(toolName: string, args: unknown): args is FileEditInsertToolArgs { - if (toolName !== "file_edit_insert") return false; - return TOOL_DEFINITIONS.file_edit_insert.schema.safeParse(args).success; -} - function isProposePlanTool(toolName: string, args: unknown): args is ProposePlanToolArgs { if (toolName !== "propose_plan") return false; return TOOL_DEFINITIONS.propose_plan.schema.safeParse(args).success; @@ -134,19 +127,6 @@ export const ToolMessage: React.FC = ({ message, className, wo ); } - if (isFileEditInsertTool(message.toolName, message.args)) { - return ( -
- -
- ); - } - if (isProposePlanTool(message.toolName, message.args)) { return (
diff --git a/src/components/tools/FileEditToolCall.tsx b/src/components/tools/FileEditToolCall.tsx index bb40ee70e..be846bdb0 100644 --- a/src/components/tools/FileEditToolCall.tsx +++ b/src/components/tools/FileEditToolCall.tsx @@ -1,8 +1,6 @@ import React from "react"; import { parsePatch } from "diff"; import type { - FileEditInsertToolArgs, - FileEditInsertToolResult, FileEditReplaceStringToolArgs, FileEditReplaceStringToolResult, FileEditReplaceLinesToolArgs, @@ -24,18 +22,12 @@ import { TooltipWrapper, Tooltip } from "../Tooltip"; import { DiffContainer, DiffRenderer, SelectableDiffRenderer } from "../shared/DiffRenderer"; import { KebabMenu, type KebabMenuItem } from "../KebabMenu"; -type FileEditOperationArgs = - | FileEditReplaceStringToolArgs - | FileEditReplaceLinesToolArgs - | FileEditInsertToolArgs; +type FileEditOperationArgs = FileEditReplaceStringToolArgs | FileEditReplaceLinesToolArgs; -type FileEditToolResult = - | FileEditReplaceStringToolResult - | FileEditReplaceLinesToolResult - | FileEditInsertToolResult; +type FileEditToolResult = FileEditReplaceStringToolResult | FileEditReplaceLinesToolResult; interface FileEditToolCallProps { - toolName: "file_edit_replace_string" | "file_edit_replace_lines" | "file_edit_insert"; + toolName: "file_edit_replace_string" | "file_edit_replace_lines"; args: FileEditOperationArgs; result?: FileEditToolResult; status?: ToolStatus; diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts deleted file mode 100644 index 01b14dc87..000000000 --- a/src/services/tools/file_edit_insert.test.ts +++ /dev/null @@ -1,405 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach } from "bun:test"; -import * as fs from "fs/promises"; -import * as path from "path"; -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"; - -// 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({ - ...getTestDeps(), - cwd: options?.cwd ?? process.cwd(), - runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }), - runtimeTempDir: tempDir.path, - }); - - return { - tool, - [Symbol.dispose]() { - tempDir[Symbol.dispose](); - }, - }; -} - -describe("file_edit_insert tool", () => { - let testDir: string; - let testFilePath: string; - - beforeEach(async () => { - // Create a temporary directory for test files - testDir = await fs.mkdtemp(path.join(os.tmpdir(), "fileEditInsert-test-")); - testFilePath = path.join(testDir, "test.txt"); - }); - - 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", - }); - const args: FileEditInsertToolArgs = { - file_path: "newfile.txt", // Use relative path - line_offset: 0, - content: "INSERTED", - 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"); - expect(fileContent).toBe("INSERTED\n"); - }); - - 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", - }); - const args: FileEditInsertToolArgs = { - file_path: "nested/dir/newfile.txt", // Use relative path - line_offset: 0, - content: "INSERTED", - 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, "nested/dir/newfile.txt"), "utf-8"); - expect(fileContent).toBe("INSERTED\n"); - }); - - 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", - }); - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 1, - content: "INSERTED", - create: true, - }; - - // 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"); - } - }); - - 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; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", // Use relative path - line_offset: 10, // File only has 2 lines - 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("beyond file length"); - } - }); - - 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; - const args: FileEditInsertToolArgs = { - file_path: "newfile.txt", - line_offset: 0, - content: "Hello, world!\n", // Content already has trailing newline - 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"); - }); - - 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; - const args: FileEditInsertToolArgs = { - file_path: "test.txt", - line_offset: 1, - content: "INSERTED1\nINSERTED2\n", // Multiline 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 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"); - }); -}); diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts deleted file mode 100644 index 87d3e1a30..000000000 --- a/src/services/tools/file_edit_insert.ts +++ /dev/null @@ -1,142 +0,0 @@ -import { tool } from "ai"; -import type { FileEditInsertToolResult } 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 { executeFileEditOperation } from "./file_edit_operation"; -import { RuntimeError } from "@/runtime/Runtime"; -import { fileExists } from "@/utils/runtime/fileExists"; -import { writeFileString } from "@/utils/runtime/helpers"; - -/** - * 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 }, - { abortSignal } - ): Promise => { - try { - // Validate and auto-correct redundant path prefix - const { correctedPath: validatedPath, warning: pathWarning } = validateAndCorrectPath( - file_path, - config.cwd, - config.runtime - ); - file_path = validatedPath; - - const pathValidation = validatePathInCwd(file_path, config.cwd, config.runtime); - if (pathValidation) { - return { - success: false, - error: pathValidation.error, - }; - } - - 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`, - 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); - } catch (err) { - if (err instanceof RuntimeError) { - return { - success: false, - error: err.message, - }; - } - throw err; - } - } - - const result = await 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: {}, - }; - }, - }); - - // 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 { - success: false, - error: `Permission denied: ${file_path}`, - }; - } - - const message = error instanceof Error ? error.message : String(error); - return { - success: false, - error: `Failed to insert content: ${message}`, - }; - } - }, - }); -}; diff --git a/src/types/tools.ts b/src/types/tools.ts index b025b9219..7a323f599 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -102,26 +102,15 @@ export type FileEditReplaceLinesToolResult = export type FileEditSharedToolResult = | FileEditReplaceStringToolResult - | FileEditReplaceLinesToolResult - | FileEditInsertToolResult; + | FileEditReplaceLinesToolResult; export const FILE_EDIT_TOOL_NAMES = [ "file_edit_replace_string", "file_edit_replace_lines", - "file_edit_insert", ] as const; 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. @@ -148,10 +137,7 @@ export const NOTE_READ_FILE_AGAIN_RETRY = "Read the file again and retry."; export const TOOL_EDIT_WARNING = "Always check the tool result before proceeding with other operations."; -export type FileEditToolArgs = - | FileEditReplaceStringToolArgs - | FileEditReplaceLinesToolArgs - | FileEditInsertToolArgs; +export type FileEditToolArgs = FileEditReplaceStringToolArgs | FileEditReplaceLinesToolArgs; // Propose Plan Tool Types export interface ProposePlanToolArgs { diff --git a/src/utils/main/tokenizer.ts b/src/utils/main/tokenizer.ts index d34c35700..b9d26a153 100644 --- a/src/utils/main/tokenizer.ts +++ b/src/utils/main/tokenizer.ts @@ -191,7 +191,6 @@ export async function getToolDefinitionTokens( file_read: 45, file_edit_replace_string: 70, file_edit_replace_lines: 80, - file_edit_insert: 50, web_search: 50, google_search: 50, }; diff --git a/src/utils/messages/toolOutputRedaction.ts b/src/utils/messages/toolOutputRedaction.ts index 8f2da0e7e..b37d0eae0 100644 --- a/src/utils/messages/toolOutputRedaction.ts +++ b/src/utils/messages/toolOutputRedaction.ts @@ -11,7 +11,6 @@ */ import type { - FileEditInsertToolResult, FileEditReplaceStringToolResult, FileEditReplaceLinesToolResult, } from "@/types/tools"; @@ -48,15 +47,6 @@ function isFileEditResult( ); } -function isFileEditInsertResult(v: unknown): v is FileEditInsertToolResult { - return ( - typeof v === "object" && - v !== null && - "success" in v && - typeof (v as { success: unknown }).success === "boolean" - ); -} - // Redactors per tool - unified for both string and line replace function redactFileEditReplace(output: unknown): unknown { const unwrapped = unwrapJsonContainer(output); @@ -87,31 +77,12 @@ function redactFileEditReplace(output: unknown): unknown { return output; } -function redactFileEditInsert(output: unknown): unknown { - const unwrapped = unwrapJsonContainer(output); - const val = unwrapped.value; - - if (!isFileEditInsertResult(val)) return output; // unknown structure, leave as-is - - if (val.success) { - const compact: FileEditInsertToolResult = { - success: true, - diff: "[diff omitted in context - call file_read on the target file if needed]", - }; - return rewrapJsonContainer(unwrapped.wrapped, compact); - } - - return output; -} - // Public API - registry entrypoint. Add new tools here as needed. export function redactToolOutput(toolName: string, output: unknown): unknown { switch (toolName) { case "file_edit_replace_string": case "file_edit_replace_lines": return redactFileEditReplace(output); - case "file_edit_insert": - return redactFileEditInsert(output); default: return output; } diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 90746cd84..f567dacf8 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -13,7 +13,6 @@ import { BASH_MAX_TOTAL_BYTES, STATUS_MESSAGE_MAX_LENGTH, } from "@/constants/toolLimits"; -import { TOOL_EDIT_WARNING } from "@/types/tools"; import { zodToJsonSchema } from "zod-to-json-schema"; @@ -108,24 +107,6 @@ 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)"), - }), - }, propose_plan: { description: "Propose a plan before taking action. The plan should be complete but minimal - cover what needs to be decided or understood, nothing more. Use this tool to get approval before proceeding with implementation.", @@ -251,7 +232,6 @@ export function getAvailableTools(modelString: string): string[] { "file_read", "file_edit_replace_string", // "file_edit_replace_lines", // DISABLED: causes models to break repo state - "file_edit_insert", "propose_plan", "todo_write", "todo_read", diff --git a/src/utils/tools/toolPolicy.test.ts b/src/utils/tools/toolPolicy.test.ts index 4782e9d4a..8214601d6 100644 --- a/src/utils/tools/toolPolicy.test.ts +++ b/src/utils/tools/toolPolicy.test.ts @@ -24,11 +24,6 @@ const mockTools = { inputSchema: z.object({ path: z.string(), start_line: z.number() }), execute: () => Promise.resolve({ success: true }), }), - file_edit_insert: tool({ - description: "Insert content in files", - inputSchema: z.object({ path: z.string() }), - execute: () => Promise.resolve({ success: true }), - }), web_search: tool({ description: "Search the web", inputSchema: z.object({ query: z.string() }), @@ -58,7 +53,6 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.file_edit_replace_string).toBeDefined(); expect(result.file_edit_replace_lines).toBeDefined(); - expect(result.file_edit_insert).toBeDefined(); expect(result.web_search).toBeDefined(); }); @@ -74,7 +68,6 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.file_edit_replace_string).toBeDefined(); expect(result.file_edit_replace_lines).toBeDefined(); - expect(result.file_edit_insert).toBeDefined(); }); }); @@ -85,7 +78,6 @@ describe("applyToolPolicy", () => { expect(result.file_edit_replace_string).toBeUndefined(); expect(result.file_edit_replace_lines).toBeUndefined(); - expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); expect(result.web_search).toBeDefined(); @@ -105,7 +97,6 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeUndefined(); expect(result.file_edit_replace_string).toBeUndefined(); expect(result.file_edit_replace_lines).toBeUndefined(); - expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.web_search).toBeDefined(); }); @@ -123,7 +114,6 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeUndefined(); expect(result.file_edit_replace_string).toBeUndefined(); expect(result.file_edit_replace_lines).toBeUndefined(); - expect(result.file_edit_insert).toBeUndefined(); expect(result.web_search).toBeUndefined(); }); @@ -136,7 +126,6 @@ describe("applyToolPolicy", () => { expect(result.file_edit_replace_string).toBeDefined(); expect(result.file_edit_replace_lines).toBeUndefined(); - expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); expect(result.web_search).toBeDefined(); @@ -172,7 +161,6 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.file_edit_replace_string).toBeDefined(); expect(result.file_edit_replace_lines).toBeDefined(); - expect(result.file_edit_insert).toBeDefined(); }); test("disables all except bash and file_read", () => { @@ -218,14 +206,13 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeUndefined(); expect(result.file_edit_replace_string).toBeUndefined(); expect(result.file_edit_replace_lines).toBeUndefined(); - expect(result.file_edit_insert).toBeUndefined(); expect(result.web_search).toBeUndefined(); }); test("requires tool with regex pattern", () => { const policy: ToolPolicy = [{ regex_match: "file_.*", action: "require" }]; - // This should throw because multiple tools match (file_read, file_edit_replace_string, file_edit_replace_lines, file_edit_insert) + // This should throw because multiple tools match (file_read, file_edit_replace_string, file_edit_replace_lines) expect(() => applyToolPolicy(mockTools, policy)).toThrow(/Multiple tools marked as required/); }); diff --git a/src/utils/tools/tools.ts b/src/utils/tools/tools.ts index a48be74b1..68e3b31d7 100644 --- a/src/utils/tools/tools.ts +++ b/src/utils/tools/tools.ts @@ -3,7 +3,6 @@ import { createFileReadTool } from "@/services/tools/file_read"; import { createBashTool } from "@/services/tools/bash"; import { createFileEditReplaceStringTool } from "@/services/tools/file_edit_replace_string"; // DISABLED: import { createFileEditReplaceLinesTool } from "@/services/tools/file_edit_replace_lines"; -import { createFileEditInsertTool } from "@/services/tools/file_edit_insert"; import { createProposePlanTool } from "@/services/tools/propose_plan"; import { createTodoWriteTool, createTodoReadTool } from "@/services/tools/todo"; import { createStatusSetTool } from "@/services/tools/status_set"; @@ -67,9 +66,8 @@ export async function getToolsForModel( file_edit_replace_string: wrap(createFileEditReplaceStringTool(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..73ccaf6a5 100644 --- a/tests/ipcMain/runtimeFileEditing.test.ts +++ b/tests/ipcMain/runtimeFileEditing.test.ts @@ -1,7 +1,7 @@ /** * Integration tests for file editing tools across Local and SSH runtimes * - * Tests file_read, file_edit_replace_string, and file_edit_insert tools + * Tests file_read and file_edit_replace_string tools * using real IPC handlers on both LocalRuntime and SSHRuntime. * * Uses toolPolicy to restrict AI to only file tools (prevents bash circumvention). @@ -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_replace_string tool to insert "Line 2" between Line 1 and Line 3 in ${testFileName}.`, HAIKU_MODEL, FILE_TOOLS_ONLY, streamTimeout @@ -336,13 +336,12 @@ 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_replace_string tool was called const toolCalls = insertEvents.filter( (e) => "type" in e && e.type === "tool-call-start" ); const editCall = toolCalls.find( - (e: any) => - e.toolName === "file_edit_insert" || e.toolName === "file_edit_replace_string" + (e: any) => e.toolName === "file_edit_replace_string" ); expect(editCall).toBeDefined(); From 2505f6d6d49d22e47a7b6eeb89abf53d183fa4de Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 14 Nov 2025 02:21:00 +0000 Subject: [PATCH 2/8] feat: add guarded file_edit_insert tool --- src/components/Messages/ToolMessage.tsx | 20 ++ src/components/tools/FileEditToolCall.tsx | 14 +- src/services/tools/file_edit_insert.test.ts | 113 ++++++++ src/services/tools/file_edit_insert.ts | 275 ++++++++++++++++++++ src/types/tools.ts | 26 +- src/utils/main/tokenizer.ts | 1 + src/utils/messages/toolOutputRedaction.ts | 28 ++ src/utils/tools/toolDefinitions.ts | 41 +++ src/utils/tools/toolPolicy.test.ts | 15 +- src/utils/tools/tools.ts | 2 + tests/ipcMain/runtimeFileEditing.test.ts | 9 +- 11 files changed, 534 insertions(+), 10 deletions(-) create mode 100644 src/services/tools/file_edit_insert.test.ts create mode 100644 src/services/tools/file_edit_insert.ts diff --git a/src/components/Messages/ToolMessage.tsx b/src/components/Messages/ToolMessage.tsx index 192cd5622..ba1f819b2 100644 --- a/src/components/Messages/ToolMessage.tsx +++ b/src/components/Messages/ToolMessage.tsx @@ -14,6 +14,8 @@ import type { FileReadToolArgs, FileReadToolResult, FileEditReplaceStringToolArgs, + FileEditInsertToolArgs, + FileEditInsertToolResult, FileEditReplaceStringToolResult, FileEditReplaceLinesToolArgs, FileEditReplaceLinesToolResult, @@ -59,6 +61,11 @@ function isFileEditReplaceLinesTool( return TOOL_DEFINITIONS.file_edit_replace_lines.schema.safeParse(args).success; } +function isFileEditInsertTool(toolName: string, args: unknown): args is FileEditInsertToolArgs { + if (toolName !== "file_edit_insert") return false; + return TOOL_DEFINITIONS.file_edit_insert.schema.safeParse(args).success; +} + function isProposePlanTool(toolName: string, args: unknown): args is ProposePlanToolArgs { if (toolName !== "propose_plan") return false; return TOOL_DEFINITIONS.propose_plan.schema.safeParse(args).success; @@ -114,6 +121,19 @@ export const ToolMessage: React.FC = ({ message, className, wo ); } + if (isFileEditInsertTool(message.toolName, message.args)) { + return ( +
+ +
+ ); + } + if (isFileEditReplaceLinesTool(message.toolName, message.args)) { return (
diff --git a/src/components/tools/FileEditToolCall.tsx b/src/components/tools/FileEditToolCall.tsx index be846bdb0..bb40ee70e 100644 --- a/src/components/tools/FileEditToolCall.tsx +++ b/src/components/tools/FileEditToolCall.tsx @@ -1,6 +1,8 @@ import React from "react"; import { parsePatch } from "diff"; import type { + FileEditInsertToolArgs, + FileEditInsertToolResult, FileEditReplaceStringToolArgs, FileEditReplaceStringToolResult, FileEditReplaceLinesToolArgs, @@ -22,12 +24,18 @@ import { TooltipWrapper, Tooltip } from "../Tooltip"; import { DiffContainer, DiffRenderer, SelectableDiffRenderer } from "../shared/DiffRenderer"; import { KebabMenu, type KebabMenuItem } from "../KebabMenu"; -type FileEditOperationArgs = FileEditReplaceStringToolArgs | FileEditReplaceLinesToolArgs; +type FileEditOperationArgs = + | FileEditReplaceStringToolArgs + | FileEditReplaceLinesToolArgs + | FileEditInsertToolArgs; -type FileEditToolResult = FileEditReplaceStringToolResult | FileEditReplaceLinesToolResult; +type FileEditToolResult = + | FileEditReplaceStringToolResult + | FileEditReplaceLinesToolResult + | FileEditInsertToolResult; interface FileEditToolCallProps { - toolName: "file_edit_replace_string" | "file_edit_replace_lines"; + toolName: "file_edit_replace_string" | "file_edit_replace_lines" | "file_edit_insert"; args: FileEditOperationArgs; result?: FileEditToolResult; status?: ToolStatus; diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts new file mode 100644 index 000000000..86868dabd --- /dev/null +++ b/src/services/tools/file_edit_insert.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect, beforeEach, afterEach } from "bun:test"; +import * as fs from "fs/promises"; +import * as path from "path"; +import * as os from "os"; +import { createFileEditInsertTool } from "./file_edit_insert"; +import type { FileEditInsertToolArgs, FileEditInsertToolResult } from "@/types/tools"; +import type { ToolCallOptions } from "ai"; +import { createRuntime } from "@/runtime/runtimeFactory"; +import { getTestDeps } from "./testHelpers"; + +const mockToolCallOptions: ToolCallOptions = { + toolCallId: "test-call-id", + messages: [], +}; + +function createTestTool(cwd: string) { + return createFileEditInsertTool({ + ...getTestDeps(), + cwd, + runtime: createRuntime({ type: "local", srcBaseDir: cwd }), + runtimeTempDir: cwd, + }); +} + +describe("file_edit_insert tool", () => { + let testDir: string; + let testFilePath: string; + + beforeEach(async () => { + 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 () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it("inserts content using before guard", async () => { + const tool = createTestTool(testDir); + const args: FileEditInsertToolArgs = { + file_path: path.relative(testDir, testFilePath), + content: "Line 2\n", + before: "Line 1\n", + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + + expect(result.success).toBe(true); + const updated = await fs.readFile(testFilePath, "utf-8"); + expect(updated).toBe("Line 1\nLine 2\nLine 3"); + }); + + it("inserts content using after guard", async () => { + const tool = createTestTool(testDir); + const args: FileEditInsertToolArgs = { + file_path: path.relative(testDir, testFilePath), + content: "Header\n", + after: "Line 1", + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + expect(result.success).toBe(true); + expect(await fs.readFile(testFilePath, "utf-8")).toBe("Header\nLine 1\nLine 3"); + }); + + 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: path.relative(testDir, testFilePath), + content: "middle\n", + before: "repeat\n", + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("multiple times"); + } + }); + + it("fails when before/after disagree", async () => { + const tool = createTestTool(testDir); + const args: FileEditInsertToolArgs = { + file_path: path.relative(testDir, testFilePath), + content: "oops", + before: "Line 1", + after: "Line 3", + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Guard mismatch"); + } + }); + + it("creates new file when create flag and line_offset provided", async () => { + const newFile = path.join(testDir, "new.txt"); + const tool = createTestTool(testDir); + const args: FileEditInsertToolArgs = { + file_path: path.relative(testDir, newFile), + content: "Hello world!\n", + line_offset: 0, + create: true, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + expect(result.success).toBe(true); + expect(await fs.readFile(newFile, "utf-8")).toBe("Hello world!\n"); + }); +}); diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts new file mode 100644 index 000000000..4b9627ed2 --- /dev/null +++ b/src/services/tools/file_edit_insert.ts @@ -0,0 +1,275 @@ +import { tool } from "ai"; +import { RuntimeError } from "@/runtime/Runtime"; +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 { validateAndCorrectPath, validatePathInCwd } from "./fileCommon"; +import { executeFileEditOperation } from "./file_edit_operation"; +import { fileExists } from "@/utils/runtime/fileExists"; +import { writeFileString } from "@/utils/runtime/helpers"; + +const READ_AND_RETRY_NOTE = `${EDIT_FAILED_NOTE_PREFIX} ${NOTE_READ_FILE_RETRY}`; + +type InsertOperationSuccess = { + success: true; + newContent: string; + metadata: Record; +}; + +type InsertOperationFailure = { + success: false; + error: string; + note?: string; +}; + +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, content, line_offset, create, before, after }: FileEditInsertToolArgs, + { abortSignal } + ): Promise => { + try { + const { correctedPath } = validateAndCorrectPath(file_path, config.cwd, config.runtime); + file_path = correctedPath; + + const pathValidation = validatePathInCwd(file_path, config.cwd, config.runtime); + if (pathValidation) { + return { + success: false, + error: pathValidation.error, + }; + } + + if (line_offset !== undefined && 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.`, + }; + } + + const resolvedPath = config.runtime.normalizePath(file_path, config.cwd); + const exists = await fileExists(config.runtime, resolvedPath, abortSignal); + + if (!exists) { + if (!create) { + return { + success: false, + 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.`, + }; + } + + try { + await writeFileString(config.runtime, resolvedPath, "", abortSignal); + } catch (err) { + if (err instanceof RuntimeError) { + return { + success: false, + error: err.message, + }; + } + throw err; + } + } + + return executeFileEditOperation({ + config, + filePath: file_path, + abortSignal, + operation: (originalContent) => + insertContent(originalContent, content, { + before, + after, + lineOffset: line_offset, + }), + }); + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "EACCES") { + return { + success: false, + error: `Permission denied: ${file_path}`, + }; + } + + const message = error instanceof Error ? error.message : String(error); + return { + success: false, + error: `Failed to insert content: ${message}`, + }; + } + }, + }); +}; + +function insertContent( + originalContent: string, + contentToInsert: string, + { + before, + after, + lineOffset, + }: { before?: string; after?: string; lineOffset?: number } +): InsertOperationSuccess | InsertOperationFailure { + if (before !== undefined || after !== undefined) { + return insertWithGuards(originalContent, contentToInsert, { before, after }); + } + + if (lineOffset === undefined) { + return { + success: false, + error: "line_offset must be provided when before/after guards are omitted.", + note: READ_AND_RETRY_NOTE, + }; + } + + return insertWithLineOffset(originalContent, contentToInsert, lineOffset); +} + +function insertWithGuards( + originalContent: string, + contentToInsert: string, + { + before, + after, + }: { before?: string; after?: string } +): InsertOperationSuccess | InsertOperationFailure { + let anchorIndex: number | undefined; + + if (before !== undefined) { + const beforeIndexResult = findUniqueSubstringIndex(originalContent, before, "before"); + if (!beforeIndexResult.success) { + return beforeIndexResult; + } + anchorIndex = beforeIndexResult.index + before.length; + } + + if (after !== undefined) { + const afterIndexResult = findUniqueSubstringIndex(originalContent, after, "after"); + if (!afterIndexResult.success) { + return afterIndexResult; + } + + if (anchorIndex === undefined) { + anchorIndex = afterIndexResult.index; + } else if (anchorIndex !== afterIndexResult.index) { + return { + success: false, + error: + "Guard mismatch: before and after substrings do not point to the same insertion point.", + note: READ_AND_RETRY_NOTE, + }; + } + } + + if (anchorIndex === undefined) { + return { + success: false, + error: "Unable to determine insertion point from guards.", + note: READ_AND_RETRY_NOTE, + }; + } + + const newContent = + originalContent.slice(0, anchorIndex) + + contentToInsert + + originalContent.slice(anchorIndex); + + return { + success: true, + newContent, + metadata: {}, + }; +} + +function findUniqueSubstringIndex( + haystack: string, + needle: string, + label: "before" | "after" +): InsertOperationFailure | { success: true; index: number } { + const firstIndex = haystack.indexOf(needle); + if (firstIndex === -1) { + return { + success: false, + error: `Guard mismatch: unable to find ${label} substring in the current file.`, + note: READ_AND_RETRY_NOTE, + }; + } + + const secondIndex = haystack.indexOf(needle, firstIndex + needle.length); + if (secondIndex !== -1) { + return { + success: false, + error: `Guard mismatch: ${label} substring matched multiple times. Provide a more specific string.`, + note: READ_AND_RETRY_NOTE, + }; + } + + return { success: true, index: firstIndex }; +} + +function insertWithLineOffset( + originalContent: string, + contentToInsert: string, + lineOffset: number +): InsertOperationSuccess | InsertOperationFailure { + const lines = originalContent.split("\n"); + if (lineOffset > lines.length) { + return { + success: false, + error: `line_offset ${lineOffset} is beyond file length (${lines.length} lines)`, + note: `${EDIT_FAILED_NOTE_PREFIX} The file has ${lines.length} lines. ${NOTE_READ_FILE_RETRY}`, + }; + } + + const insertionIndex = getIndexForLineOffset(originalContent, lineOffset); + if (insertionIndex === null) { + return { + success: false, + error: `Unable to compute insertion point for line_offset ${lineOffset}.`, + note: READ_AND_RETRY_NOTE, + }; + } + + const newContent = + originalContent.slice(0, insertionIndex) + + contentToInsert + + originalContent.slice(insertionIndex); + + return { + success: true, + newContent, + metadata: {}, + }; +} + +function getIndexForLineOffset(content: string, lineOffset: number): number | null { + if (lineOffset === 0) { + return 0; + } + + let linesAdvanced = 0; + let cursor = 0; + + while (linesAdvanced < lineOffset) { + const nextNewline = content.indexOf("\n", cursor); + if (nextNewline === -1) { + if (linesAdvanced + 1 === lineOffset) { + return content.length; + } + return null; + } + + linesAdvanced += 1; + cursor = nextNewline + 1; + + if (linesAdvanced === lineOffset) { + return cursor; + } + } + + return null; +} diff --git a/src/types/tools.ts b/src/types/tools.ts index 7a323f599..9448e9b18 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -71,6 +71,23 @@ export interface FileEditErrorResult { note?: string; // Agent-only message (not displayed in UI) } +export interface FileEditInsertToolArgs { + file_path: string; + content: string; + /** + * Optional line offset fallback when guards are not provided. + * 1-indexed: 0 inserts at the top, 1 after line 1, etc. + */ + line_offset?: number; + 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; @@ -102,11 +119,13 @@ export type FileEditReplaceLinesToolResult = export type FileEditSharedToolResult = | FileEditReplaceStringToolResult - | FileEditReplaceLinesToolResult; + | FileEditReplaceLinesToolResult + | FileEditInsertToolResult; export const FILE_EDIT_TOOL_NAMES = [ "file_edit_replace_string", "file_edit_replace_lines", + "file_edit_insert", ] as const; export type FileEditToolName = (typeof FILE_EDIT_TOOL_NAMES)[number]; @@ -137,7 +156,10 @@ export const NOTE_READ_FILE_AGAIN_RETRY = "Read the file again and retry."; export const TOOL_EDIT_WARNING = "Always check the tool result before proceeding with other operations."; -export type FileEditToolArgs = FileEditReplaceStringToolArgs | FileEditReplaceLinesToolArgs; +export type FileEditToolArgs = + | FileEditReplaceStringToolArgs + | FileEditReplaceLinesToolArgs + | FileEditInsertToolArgs; // Propose Plan Tool Types export interface ProposePlanToolArgs { diff --git a/src/utils/main/tokenizer.ts b/src/utils/main/tokenizer.ts index b9d26a153..d34c35700 100644 --- a/src/utils/main/tokenizer.ts +++ b/src/utils/main/tokenizer.ts @@ -191,6 +191,7 @@ export async function getToolDefinitionTokens( file_read: 45, file_edit_replace_string: 70, file_edit_replace_lines: 80, + file_edit_insert: 50, web_search: 50, google_search: 50, }; diff --git a/src/utils/messages/toolOutputRedaction.ts b/src/utils/messages/toolOutputRedaction.ts index b37d0eae0..51ffb79e0 100644 --- a/src/utils/messages/toolOutputRedaction.ts +++ b/src/utils/messages/toolOutputRedaction.ts @@ -11,6 +11,7 @@ */ import type { + FileEditInsertToolResult, FileEditReplaceStringToolResult, FileEditReplaceLinesToolResult, } from "@/types/tools"; @@ -35,6 +36,14 @@ function rewrapJsonContainer(wrapped: boolean, value: unknown): unknown { return value; } +function isFileEditInsertResult(v: unknown): v is FileEditInsertToolResult { + return ( + typeof v === "object" && + v !== null && + "success" in v && + typeof (v as { success: unknown }).success === "boolean" + ); +} // Narrowing helpers for our tool result types function isFileEditResult( v: unknown @@ -77,12 +86,31 @@ function redactFileEditReplace(output: unknown): unknown { return output; } +function redactFileEditInsert(output: unknown): unknown { + const unwrapped = unwrapJsonContainer(output); + const val = unwrapped.value; + + if (!isFileEditInsertResult(val)) return output; + + if (val.success) { + const compact: FileEditInsertToolResult = { + success: true, + diff: "[diff omitted in context - call file_read on the target file if needed]", + }; + return rewrapJsonContainer(unwrapped.wrapped, compact); + } + + return output; +} + // Public API - registry entrypoint. Add new tools here as needed. export function redactToolOutput(toolName: string, output: unknown): unknown { switch (toolName) { case "file_edit_replace_string": case "file_edit_replace_lines": return redactFileEditReplace(output); + case "file_edit_insert": + return redactFileEditInsert(output); default: return output; } diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index f567dacf8..7b4219bce 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -13,6 +13,7 @@ import { BASH_MAX_TOTAL_BYTES, STATUS_MESSAGE_MAX_LENGTH, } from "@/constants/toolLimits"; +import { TOOL_EDIT_WARNING } from "@/types/tools"; import { zodToJsonSchema } from "zod-to-json-schema"; @@ -107,6 +108,45 @@ export const TOOL_DEFINITIONS = { ), }), }, + file_edit_insert: { + description: + "Insert content into a file using either a line offset or substring guards. " + + "Provide at least one of before/after/line_offset so the operation is anchored. " + + `Optional before/after substrings must uniquely match surrounding content. ${TOOL_EDIT_WARNING}`, + schema: z + .object({ + file_path: z.string().describe("The absolute path to the file to edit"), + content: z.string().describe("The content to insert"), + line_offset: z + .number() + .int() + .min(0) + .optional() + .describe("Optional 1-indexed line position (0 = insert at top, 1 inserts after line 1, etc.)"), + create: z + .boolean() + .optional() + .describe("If true, create the file if it doesn't exist (default: false)"), + 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.line_offset !== undefined || data.before !== undefined || data.after !== undefined, + { + message: "Provide at least one of line_offset, before, or after to anchor the insertion point.", + path: ["line_offset"], + } + ), + }, propose_plan: { description: "Propose a plan before taking action. The plan should be complete but minimal - cover what needs to be decided or understood, nothing more. Use this tool to get approval before proceeding with implementation.", @@ -232,6 +272,7 @@ export function getAvailableTools(modelString: string): string[] { "file_read", "file_edit_replace_string", // "file_edit_replace_lines", // DISABLED: causes models to break repo state + "file_edit_insert", "propose_plan", "todo_write", "todo_read", diff --git a/src/utils/tools/toolPolicy.test.ts b/src/utils/tools/toolPolicy.test.ts index 8214601d6..4782e9d4a 100644 --- a/src/utils/tools/toolPolicy.test.ts +++ b/src/utils/tools/toolPolicy.test.ts @@ -24,6 +24,11 @@ const mockTools = { inputSchema: z.object({ path: z.string(), start_line: z.number() }), execute: () => Promise.resolve({ success: true }), }), + file_edit_insert: tool({ + description: "Insert content in files", + inputSchema: z.object({ path: z.string() }), + execute: () => Promise.resolve({ success: true }), + }), web_search: tool({ description: "Search the web", inputSchema: z.object({ query: z.string() }), @@ -53,6 +58,7 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.file_edit_replace_string).toBeDefined(); expect(result.file_edit_replace_lines).toBeDefined(); + expect(result.file_edit_insert).toBeDefined(); expect(result.web_search).toBeDefined(); }); @@ -68,6 +74,7 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.file_edit_replace_string).toBeDefined(); expect(result.file_edit_replace_lines).toBeDefined(); + expect(result.file_edit_insert).toBeDefined(); }); }); @@ -78,6 +85,7 @@ describe("applyToolPolicy", () => { expect(result.file_edit_replace_string).toBeUndefined(); expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); expect(result.web_search).toBeDefined(); @@ -97,6 +105,7 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeUndefined(); expect(result.file_edit_replace_string).toBeUndefined(); expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.web_search).toBeDefined(); }); @@ -114,6 +123,7 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeUndefined(); expect(result.file_edit_replace_string).toBeUndefined(); expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.web_search).toBeUndefined(); }); @@ -126,6 +136,7 @@ describe("applyToolPolicy", () => { expect(result.file_edit_replace_string).toBeDefined(); expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); expect(result.web_search).toBeDefined(); @@ -161,6 +172,7 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.file_edit_replace_string).toBeDefined(); expect(result.file_edit_replace_lines).toBeDefined(); + expect(result.file_edit_insert).toBeDefined(); }); test("disables all except bash and file_read", () => { @@ -206,13 +218,14 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeUndefined(); expect(result.file_edit_replace_string).toBeUndefined(); expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.web_search).toBeUndefined(); }); test("requires tool with regex pattern", () => { const policy: ToolPolicy = [{ regex_match: "file_.*", action: "require" }]; - // This should throw because multiple tools match (file_read, file_edit_replace_string, file_edit_replace_lines) + // This should throw because multiple tools match (file_read, file_edit_replace_string, file_edit_replace_lines, file_edit_insert) expect(() => applyToolPolicy(mockTools, policy)).toThrow(/Multiple tools marked as required/); }); diff --git a/src/utils/tools/tools.ts b/src/utils/tools/tools.ts index 68e3b31d7..b86f70412 100644 --- a/src/utils/tools/tools.ts +++ b/src/utils/tools/tools.ts @@ -3,6 +3,7 @@ import { createFileReadTool } from "@/services/tools/file_read"; import { createBashTool } from "@/services/tools/bash"; import { createFileEditReplaceStringTool } from "@/services/tools/file_edit_replace_string"; // DISABLED: import { createFileEditReplaceLinesTool } from "@/services/tools/file_edit_replace_lines"; +import { createFileEditInsertTool } from "@/services/tools/file_edit_insert"; import { createProposePlanTool } from "@/services/tools/propose_plan"; import { createTodoWriteTool, createTodoReadTool } from "@/services/tools/todo"; import { createStatusSetTool } from "@/services/tools/status_set"; @@ -64,6 +65,7 @@ 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 instead. diff --git a/tests/ipcMain/runtimeFileEditing.test.ts b/tests/ipcMain/runtimeFileEditing.test.ts index 73ccaf6a5..e53ed497c 100644 --- a/tests/ipcMain/runtimeFileEditing.test.ts +++ b/tests/ipcMain/runtimeFileEditing.test.ts @@ -1,7 +1,7 @@ /** * Integration tests for file editing tools across Local and SSH runtimes * - * Tests file_read and file_edit_replace_string tools + * Tests file_read, file_edit_replace_string, and file_edit_insert tools * using real IPC handlers on both LocalRuntime and SSHRuntime. * * Uses toolPolicy to restrict AI to only file tools (prevents bash circumvention). @@ -325,7 +325,7 @@ describeIntegration("Runtime File Editing Tools", () => { const insertEvents = await sendMessageAndWait( env, workspaceId, - `Use the 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,12 +336,13 @@ describeIntegration("Runtime File Editing Tools", () => { expect(streamEnd).toBeDefined(); expect((streamEnd as any).error).toBeUndefined(); - // Verify file_edit_replace_string tool was called + // 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" ); const editCall = toolCalls.find( - (e: any) => e.toolName === "file_edit_replace_string" + (e: any) => + e.toolName === "file_edit_insert" || e.toolName === "file_edit_replace_string" ); expect(editCall).toBeDefined(); From c5396c15142a29e6579231270389425ebf2b218e Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 14 Nov 2025 02:24:53 +0000 Subject: [PATCH 3/8] =?UTF-8?q?=F0=9F=A4=96=20fix:=20satisfy=20lint=20for?= =?UTF-8?q?=20file=5Fedit=5Finsert?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated with `mux` --- src/services/tools/file_edit_insert.ts | 23 +++++++---------------- src/utils/tools/toolDefinitions.ts | 7 +++++-- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 4b9627ed2..10bbd309c 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -11,17 +11,17 @@ import { writeFileString } from "@/utils/runtime/helpers"; const READ_AND_RETRY_NOTE = `${EDIT_FAILED_NOTE_PREFIX} ${NOTE_READ_FILE_RETRY}`; -type InsertOperationSuccess = { +interface InsertOperationSuccess { success: true; newContent: string; metadata: Record; -}; +} -type InsertOperationFailure = { +interface InsertOperationFailure { success: false; error: string; note?: string; -}; +} export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) => { return tool({ @@ -108,11 +108,7 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) function insertContent( originalContent: string, contentToInsert: string, - { - before, - after, - lineOffset, - }: { before?: string; after?: string; lineOffset?: number } + { before, after, lineOffset }: { before?: string; after?: string; lineOffset?: number } ): InsertOperationSuccess | InsertOperationFailure { if (before !== undefined || after !== undefined) { return insertWithGuards(originalContent, contentToInsert, { before, after }); @@ -132,10 +128,7 @@ function insertContent( function insertWithGuards( originalContent: string, contentToInsert: string, - { - before, - after, - }: { before?: string; after?: string } + { before, after }: { before?: string; after?: string } ): InsertOperationSuccess | InsertOperationFailure { let anchorIndex: number | undefined; @@ -174,9 +167,7 @@ function insertWithGuards( } const newContent = - originalContent.slice(0, anchorIndex) + - contentToInsert + - originalContent.slice(anchorIndex); + originalContent.slice(0, anchorIndex) + contentToInsert + originalContent.slice(anchorIndex); return { success: true, diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 7b4219bce..e386d5f3f 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -122,7 +122,9 @@ export const TOOL_DEFINITIONS = { .int() .min(0) .optional() - .describe("Optional 1-indexed line position (0 = insert at top, 1 inserts after line 1, etc.)"), + .describe( + "Optional 1-indexed line position (0 = insert at top, 1 inserts after line 1, etc.)" + ), create: z .boolean() .optional() @@ -142,7 +144,8 @@ export const TOOL_DEFINITIONS = { (data) => data.line_offset !== undefined || data.before !== undefined || data.after !== undefined, { - message: "Provide at least one of line_offset, before, or after to anchor the insertion point.", + message: + "Provide at least one of line_offset, before, or after to anchor the insertion point.", path: ["line_offset"], } ), From 102e5b385cdf86d99d667ada015e7258079212aa Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 14 Nov 2025 02:42:00 +0000 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=A4=96=20fix:=20enforce=20single=20gu?= =?UTF-8?q?ard=20for=20file=5Fedit=5Finsert?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated with --- src/services/tools/file_edit_insert.test.ts | 4 +- src/services/tools/file_edit_insert.ts | 145 +++++++++++--------- src/utils/tools/toolDefinitions.ts | 7 +- 3 files changed, 91 insertions(+), 65 deletions(-) diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 86868dabd..7da3956e4 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -80,7 +80,7 @@ describe("file_edit_insert tool", () => { } }); - it("fails when before/after disagree", async () => { + it("fails when both before and after are provided", async () => { const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { file_path: path.relative(testDir, testFilePath), @@ -92,7 +92,7 @@ describe("file_edit_insert tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain("Guard mismatch"); + expect(result.error).toContain("only one of before or after"); } }); diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 10bbd309c..145ebb532 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -23,6 +23,27 @@ interface InsertOperationFailure { note?: string; } +interface InsertContentOptions { + before?: string; + after?: string; + lineOffset?: number; +} + +interface GuardResolutionSuccess { + success: true; + index: number; +} + +function guardFailure(error: string): InsertOperationFailure { + return { + success: false, + error, + note: READ_AND_RETRY_NOTE, + }; +} + +type GuardAnchors = Pick; + export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) => { return tool({ description: TOOL_DEFINITIONS.file_edit_insert.description, @@ -108,18 +129,20 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) function insertContent( originalContent: string, contentToInsert: string, - { before, after, lineOffset }: { before?: string; after?: string; lineOffset?: number } + options: InsertContentOptions ): InsertOperationSuccess | InsertOperationFailure { + const { before, after, lineOffset } = options; + + if (before !== undefined && after !== undefined) { + return guardFailure("Provide only one of before or after (not both)."); + } + if (before !== undefined || after !== undefined) { return insertWithGuards(originalContent, contentToInsert, { before, after }); } if (lineOffset === undefined) { - return { - success: false, - error: "line_offset must be provided when before/after guards are omitted.", - note: READ_AND_RETRY_NOTE, - }; + return guardFailure("line_offset must be provided when before/after guards are omitted."); } return insertWithLineOffset(originalContent, contentToInsert, lineOffset); @@ -128,46 +151,17 @@ function insertContent( function insertWithGuards( originalContent: string, contentToInsert: string, - { before, after }: { before?: string; after?: string } + anchors: GuardAnchors ): InsertOperationSuccess | InsertOperationFailure { - let anchorIndex: number | undefined; - - if (before !== undefined) { - const beforeIndexResult = findUniqueSubstringIndex(originalContent, before, "before"); - if (!beforeIndexResult.success) { - return beforeIndexResult; - } - anchorIndex = beforeIndexResult.index + before.length; - } - - if (after !== undefined) { - const afterIndexResult = findUniqueSubstringIndex(originalContent, after, "after"); - if (!afterIndexResult.success) { - return afterIndexResult; - } - - if (anchorIndex === undefined) { - anchorIndex = afterIndexResult.index; - } else if (anchorIndex !== afterIndexResult.index) { - return { - success: false, - error: - "Guard mismatch: before and after substrings do not point to the same insertion point.", - note: READ_AND_RETRY_NOTE, - }; - } - } - - if (anchorIndex === undefined) { - return { - success: false, - error: "Unable to determine insertion point from guards.", - note: READ_AND_RETRY_NOTE, - }; + const anchorResult = resolveGuardAnchor(originalContent, anchors); + if (!anchorResult.success) { + return anchorResult; } const newContent = - originalContent.slice(0, anchorIndex) + contentToInsert + originalContent.slice(anchorIndex); + originalContent.slice(0, anchorResult.index) + + contentToInsert + + originalContent.slice(anchorResult.index); return { success: true, @@ -180,49 +174,62 @@ function findUniqueSubstringIndex( haystack: string, needle: string, label: "before" | "after" -): InsertOperationFailure | { success: true; index: number } { +): GuardResolutionSuccess | InsertOperationFailure { const firstIndex = haystack.indexOf(needle); if (firstIndex === -1) { - return { - success: false, - error: `Guard mismatch: unable to find ${label} substring in the current file.`, - note: READ_AND_RETRY_NOTE, - }; + 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 { - success: false, - error: `Guard mismatch: ${label} substring matched multiple times. Provide a more specific string.`, - note: READ_AND_RETRY_NOTE, - }; + 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."); +} + function insertWithLineOffset( originalContent: string, contentToInsert: string, lineOffset: number ): InsertOperationSuccess | InsertOperationFailure { - const lines = originalContent.split("\n"); - if (lineOffset > lines.length) { + const lineCount = countLines(originalContent); + if (lineOffset > lineCount) { return { success: false, - error: `line_offset ${lineOffset} is beyond file length (${lines.length} lines)`, - note: `${EDIT_FAILED_NOTE_PREFIX} The file has ${lines.length} lines. ${NOTE_READ_FILE_RETRY}`, + error: `line_offset ${lineOffset} is beyond file length (${lineCount} lines)`, + note: `${EDIT_FAILED_NOTE_PREFIX} The file has ${lineCount} lines. ${NOTE_READ_FILE_RETRY}`, }; } const insertionIndex = getIndexForLineOffset(originalContent, lineOffset); if (insertionIndex === null) { - return { - success: false, - error: `Unable to compute insertion point for line_offset ${lineOffset}.`, - note: READ_AND_RETRY_NOTE, - }; + return guardFailure(`Unable to compute insertion point for line_offset ${lineOffset}.`); } const newContent = @@ -264,3 +271,17 @@ function getIndexForLineOffset(content: string, lineOffset: number): number | nu return null; } + +function countLines(content: string): number { + if (content.length === 0) { + return 1; + } + + let count = 1; + for (const char of content) { + if (char === "\n") { + count += 1; + } + } + return count; +} diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index e386d5f3f..ab510c024 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -112,6 +112,7 @@ export const TOOL_DEFINITIONS = { description: "Insert content into a file using either a line offset or substring guards. " + "Provide at least one of before/after/line_offset so the operation is anchored. " + + "When using guards, supply exactly one of before or after (not both). " + `Optional before/after substrings must uniquely match surrounding content. ${TOOL_EDIT_WARNING}`, schema: z .object({ @@ -148,7 +149,11 @@ export const TOOL_DEFINITIONS = { "Provide at least one of line_offset, before, or after to anchor the insertion point.", path: ["line_offset"], } - ), + ) + .refine((data) => !(data.before !== undefined && data.after !== undefined), { + message: "Provide only one of before or after (not both).", + path: ["before"], + }), }, propose_plan: { description: From 67fe9a3001290b263932bff0d330d3b97c009e5f Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 14 Nov 2025 16:44:04 +0000 Subject: [PATCH 5/8] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20remove=20line=5F?= =?UTF-8?q?offset=20from=20insert=20tool?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated with --- src/services/tools/file_edit_insert.test.ts | 16 +-- src/services/tools/file_edit_insert.ts | 125 +------------------- src/types/tools.ts | 6 - src/utils/tools/toolDefinitions.ts | 30 +---- 4 files changed, 19 insertions(+), 158 deletions(-) diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 7da3956e4..28772fc41 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -96,18 +96,18 @@ describe("file_edit_insert tool", () => { } }); - it("creates new file when create flag and line_offset provided", async () => { - const newFile = path.join(testDir, "new.txt"); + it("fails when no guards are provided", async () => { const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { - file_path: path.relative(testDir, newFile), - content: "Hello world!\n", - line_offset: 0, - create: true, + file_path: path.relative(testDir, testFilePath), + content: "noop", }; const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; - expect(result.success).toBe(true); - expect(await fs.readFile(newFile, "utf-8")).toBe("Hello world!\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 145ebb532..21b92d8c6 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -1,13 +1,10 @@ import { tool } from "ai"; -import { RuntimeError } from "@/runtime/Runtime"; 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 { validateAndCorrectPath, validatePathInCwd } from "./fileCommon"; import { executeFileEditOperation } from "./file_edit_operation"; -import { fileExists } from "@/utils/runtime/fileExists"; -import { writeFileString } from "@/utils/runtime/helpers"; const READ_AND_RETRY_NOTE = `${EDIT_FAILED_NOTE_PREFIX} ${NOTE_READ_FILE_RETRY}`; @@ -26,7 +23,6 @@ interface InsertOperationFailure { interface InsertContentOptions { before?: string; after?: string; - lineOffset?: number; } interface GuardResolutionSuccess { @@ -49,7 +45,7 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) description: TOOL_DEFINITIONS.file_edit_insert.description, inputSchema: TOOL_DEFINITIONS.file_edit_insert.schema, execute: async ( - { file_path, content, line_offset, create, before, after }: FileEditInsertToolArgs, + { file_path, content, before, after }: FileEditInsertToolArgs, { abortSignal } ): Promise => { try { @@ -64,39 +60,6 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }; } - if (line_offset !== undefined && 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.`, - }; - } - - const resolvedPath = config.runtime.normalizePath(file_path, config.cwd); - const exists = await fileExists(config.runtime, resolvedPath, abortSignal); - - if (!exists) { - if (!create) { - return { - success: false, - 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.`, - }; - } - - try { - await writeFileString(config.runtime, resolvedPath, "", abortSignal); - } catch (err) { - if (err instanceof RuntimeError) { - return { - success: false, - error: err.message, - }; - } - throw err; - } - } - return executeFileEditOperation({ config, filePath: file_path, @@ -105,7 +68,6 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) insertContent(originalContent, content, { before, after, - lineOffset: line_offset, }), }); } catch (error) { @@ -131,21 +93,17 @@ function insertContent( contentToInsert: string, options: InsertContentOptions ): InsertOperationSuccess | InsertOperationFailure { - const { before, after, lineOffset } = options; + 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 insertWithGuards(originalContent, contentToInsert, { before, after }); + if (before === undefined && after === undefined) { + return guardFailure("Provide either a before or after guard to anchor the insertion point."); } - if (lineOffset === undefined) { - return guardFailure("line_offset must be provided when before/after guards are omitted."); - } - - return insertWithLineOffset(originalContent, contentToInsert, lineOffset); + return insertWithGuards(originalContent, contentToInsert, { before, after }); } function insertWithGuards( @@ -212,76 +170,3 @@ function resolveGuardAnchor( return guardFailure("Unable to determine insertion point from guards."); } - -function insertWithLineOffset( - originalContent: string, - contentToInsert: string, - lineOffset: number -): InsertOperationSuccess | InsertOperationFailure { - const lineCount = countLines(originalContent); - if (lineOffset > lineCount) { - return { - success: false, - error: `line_offset ${lineOffset} is beyond file length (${lineCount} lines)`, - note: `${EDIT_FAILED_NOTE_PREFIX} The file has ${lineCount} lines. ${NOTE_READ_FILE_RETRY}`, - }; - } - - const insertionIndex = getIndexForLineOffset(originalContent, lineOffset); - if (insertionIndex === null) { - return guardFailure(`Unable to compute insertion point for line_offset ${lineOffset}.`); - } - - const newContent = - originalContent.slice(0, insertionIndex) + - contentToInsert + - originalContent.slice(insertionIndex); - - return { - success: true, - newContent, - metadata: {}, - }; -} - -function getIndexForLineOffset(content: string, lineOffset: number): number | null { - if (lineOffset === 0) { - return 0; - } - - let linesAdvanced = 0; - let cursor = 0; - - while (linesAdvanced < lineOffset) { - const nextNewline = content.indexOf("\n", cursor); - if (nextNewline === -1) { - if (linesAdvanced + 1 === lineOffset) { - return content.length; - } - return null; - } - - linesAdvanced += 1; - cursor = nextNewline + 1; - - if (linesAdvanced === lineOffset) { - return cursor; - } - } - - return null; -} - -function countLines(content: string): number { - if (content.length === 0) { - return 1; - } - - let count = 1; - for (const char of content) { - if (char === "\n") { - count += 1; - } - } - return count; -} diff --git a/src/types/tools.ts b/src/types/tools.ts index 9448e9b18..633d151f7 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -74,12 +74,6 @@ export interface FileEditErrorResult { export interface FileEditInsertToolArgs { file_path: string; content: string; - /** - * Optional line offset fallback when guards are not provided. - * 1-indexed: 0 inserts at the top, 1 after line 1, etc. - */ - line_offset?: number; - create?: boolean; /** Optional substring that must appear immediately before the insertion point */ before?: string; /** Optional substring that must appear immediately after the insertion point */ diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index ab510c024..49ccbb26b 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -110,26 +110,13 @@ export const TOOL_DEFINITIONS = { }, file_edit_insert: { description: - "Insert content into a file using either a line offset or substring guards. " + - "Provide at least one of before/after/line_offset so the operation is anchored. " + - "When using guards, supply exactly one of before or after (not both). " + + "Insert content into a file using substring guards. " + + "Provide exactly one of before or after to anchor the operation. " + `Optional before/after substrings must uniquely match surrounding content. ${TOOL_EDIT_WARNING}`, schema: z .object({ file_path: z.string().describe("The absolute path to the file to edit"), content: z.string().describe("The content to insert"), - line_offset: z - .number() - .int() - .min(0) - .optional() - .describe( - "Optional 1-indexed line position (0 = insert at top, 1 inserts after line 1, etc.)" - ), - create: z - .boolean() - .optional() - .describe("If true, create the file if it doesn't exist (default: false)"), before: z .string() .min(1) @@ -141,15 +128,10 @@ export const TOOL_DEFINITIONS = { .optional() .describe("Optional substring that must appear immediately after the insertion point"), }) - .refine( - (data) => - data.line_offset !== undefined || data.before !== undefined || data.after !== undefined, - { - message: - "Provide at least one of line_offset, before, or after to anchor the insertion point.", - path: ["line_offset"], - } - ) + .refine((data) => data.before !== undefined || data.after !== undefined, { + message: "Provide either before or after to anchor the insertion point.", + path: ["before"], + }) .refine((data) => !(data.before !== undefined && data.after !== undefined), { message: "Provide only one of before or after (not both).", path: ["before"], From 9c44752903ec268e81c5da52537a66db974ec784 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 14 Nov 2025 17:00:35 +0000 Subject: [PATCH 6/8] =?UTF-8?q?=F0=9F=A4=96=20fix:=20reuse=20file=20path?= =?UTF-8?q?=20schema=20for=20edit=20tools?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated with --- src/utils/tools/toolDefinitions.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 49ccbb26b..1799e1603 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 @@ -115,7 +119,7 @@ export const TOOL_DEFINITIONS = { `Optional before/after substrings must uniquely match surrounding content. ${TOOL_EDIT_WARNING}`, schema: z .object({ - file_path: z.string().describe("The absolute path to the file to edit"), + file_path: FILE_EDIT_FILE_PATH, content: z.string().describe("The content to insert"), before: z .string() From 009b9015ee538fec30a60ca3f43779c3c0627f1a Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 14 Nov 2025 17:19:48 +0000 Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=A4=96=20chore:=20format=20file=5Fedi?= =?UTF-8?q?t=5Finsert=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated with --- src/services/tools/file_edit_insert.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 28772fc41..7bcf2b299 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -109,5 +109,4 @@ describe("file_edit_insert tool", () => { expect(result.error).toContain("Provide either a before or after guard"); } }); - }); From d9622c3117e02441a6fdd0d754fe3f7f88282d59 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 14 Nov 2025 17:40:38 +0000 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=A4=96=20fix:=20allow=20file=5Fedit?= =?UTF-8?q?=5Finsert=20to=20create=20files?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated with --- src/services/tools/file_edit_insert.test.ts | 14 +++++++ src/services/tools/file_edit_insert.ts | 45 +++++++++++++++++++-- src/types/tools.ts | 2 + src/utils/tools/toolDefinitions.ts | 16 +++++--- 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 7bcf2b299..94113cae5 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -96,6 +96,20 @@ describe("file_edit_insert 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: path.relative(testDir, newFile), + content: "Hello world!\n", + create: true, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditInsertToolResult; + expect(result.success).toBe(true); + expect(await fs.readFile(newFile, "utf-8")).toBe("Hello world!\n"); + }); + it("fails when no guards are provided", async () => { const tool = createTestTool(testDir); const args: FileEditInsertToolArgs = { diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 21b92d8c6..524a634ce 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -3,8 +3,11 @@ import type { FileEditInsertToolArgs, FileEditInsertToolResult } from "@/types/t 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 { validateAndCorrectPath, validatePathInCwd } from "./fileCommon"; +import { generateDiff, validateAndCorrectPath, validatePathInCwd } from "./fileCommon"; import { executeFileEditOperation } from "./file_edit_operation"; +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}`; @@ -45,11 +48,15 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) description: TOOL_DEFINITIONS.file_edit_insert.description, inputSchema: TOOL_DEFINITIONS.file_edit_insert.schema, execute: async ( - { file_path, content, before, after }: FileEditInsertToolArgs, + { file_path, content, before, after, create }: FileEditInsertToolArgs, { abortSignal } ): Promise => { try { - const { correctedPath } = validateAndCorrectPath(file_path, config.cwd, config.runtime); + const { correctedPath, warning: pathWarning } = validateAndCorrectPath( + file_path, + config.cwd, + config.runtime + ); file_path = correctedPath; const pathValidation = validatePathInCwd(file_path, config.cwd, config.runtime); @@ -60,6 +67,38 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }; } + const resolvedPath = config.runtime.normalizePath(file_path, config.cwd); + const exists = await fileExists(config.runtime, resolvedPath, abortSignal); + + if (!exists) { + if (!create) { + return { + success: false, + 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.`, + }; + } + + try { + await writeFileString(config.runtime, resolvedPath, content, abortSignal); + } catch (err) { + if (err instanceof RuntimeError) { + return { + success: false, + error: err.message, + }; + } + throw err; + } + + const diff = generateDiff(resolvedPath, "", content); + return { + success: true, + diff, + ...(pathWarning && { warning: pathWarning }), + }; + } + return executeFileEditOperation({ config, filePath: file_path, diff --git a/src/types/tools.ts b/src/types/tools.ts index 633d151f7..7a060b233 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -74,6 +74,8 @@ export interface FileEditErrorResult { 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 */ diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 1799e1603..e63f7f221 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -115,12 +115,14 @@ export const TOOL_DEFINITIONS = { file_edit_insert: { description: "Insert content into a file using substring guards. " + - "Provide exactly one of before or after to anchor the operation. " + + "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) @@ -132,10 +134,14 @@ export const TOOL_DEFINITIONS = { .optional() .describe("Optional substring that must appear immediately after the insertion point"), }) - .refine((data) => data.before !== undefined || data.after !== undefined, { - message: "Provide either before or after to anchor the insertion point.", - path: ["before"], - }) + .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"],