From a5131d3db5f15c6e4ef701a60ff437ae4e488978 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 12 Oct 2025 11:40:14 -0500 Subject: [PATCH 1/8] =?UTF-8?q?feat:=20=F0=9F=A4=96=20refactor=20file=20ed?= =?UTF-8?q?it=20tools=20to=20add=20line=20replacements?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/AGENTS.md | 47 ++-- scripts/wait_pr_checks.sh | 2 + src/components/Messages/ToolMessage.tsx | 42 ++- src/components/TitleBar.tsx | 38 ++- src/components/tools/FileEditToolCall.tsx | 21 +- src/services/streamManager.test.ts | 8 +- src/services/tools/file_edit_insert.ts | 120 +++------ .../tools/file_edit_operation.test.ts | 24 ++ src/services/tools/file_edit_operation.ts | 113 +++++++++ src/services/tools/file_edit_replace.ts | 174 ------------- src/services/tools/file_edit_replace_lines.ts | 88 +++++++ ...st.ts => file_edit_replace_string.test.ts} | 239 +++++------------- .../tools/file_edit_replace_string.ts | 89 +++++++ src/types/tools.ts | 90 +++++-- src/utils/main/tokenizer.ts | 3 +- src/utils/messages/toolOutputRedaction.ts | 51 +++- src/utils/tools/toolDefinitions.ts | 34 ++- src/utils/tools/toolPolicy.test.ts | 43 +++- src/utils/tools/tools.ts | 6 +- 19 files changed, 709 insertions(+), 523 deletions(-) create mode 100644 src/services/tools/file_edit_operation.test.ts create mode 100644 src/services/tools/file_edit_operation.ts delete mode 100644 src/services/tools/file_edit_replace.ts create mode 100644 src/services/tools/file_edit_replace_lines.ts rename src/services/tools/{file_edit_replace.test.ts => file_edit_replace_string.test.ts} (56%) create mode 100644 src/services/tools/file_edit_replace_string.ts diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 2d935dae7b..ead56e3459 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -200,31 +200,34 @@ This project uses **Make** as the primary build orchestrator. See `Makefile` for const config = env.config.loadConfigOrDefault(); config.projects.set(projectPath, { path: projectPath, workspaces: [] }); env.config.saveConfig(config); - - // ❌ BAD - Directly accessing services - const history = await env.historyService.getHistory(workspaceId); - await env.historyService.appendToHistory(workspaceId, message); ``` - **Correct approach (DO THIS):** +// ❌ BAD - Directly accessing services +const history = await env.historyService.getHistory(workspaceId); +await env.historyService.appendToHistory(workspaceId, message); - ```typescript - // ✅ GOOD - Use IPC to save config - await env.mockIpcRenderer.invoke(IPC_CHANNELS.CONFIG_SAVE, { - projects: Array.from(projectsConfig.projects.entries()), - }); - - // ✅ GOOD - Use IPC to interact with services - await env.mockIpcRenderer.invoke(IPC_CHANNELS.HISTORY_GET, workspaceId); - await env.mockIpcRenderer.invoke(IPC_CHANNELS.WORKSPACE_CREATE, projectPath, branchName); - ``` +```` + +**Correct approach (DO THIS):** + +```typescript +// ✅ GOOD - Use IPC to save config +await env.mockIpcRenderer.invoke(IPC_CHANNELS.CONFIG_SAVE, { + projects: Array.from(projectsConfig.projects.entries()), +}); - **Acceptable exceptions:** - - Reading context (like `env.config.loadConfigOrDefault()`) to prepare IPC call parameters - - Verifying filesystem state (like checking if files exist) after IPC operations complete - - Loading existing data to avoid expensive API calls in test setup +// ✅ GOOD - Use IPC to interact with services +await env.mockIpcRenderer.invoke(IPC_CHANNELS.HISTORY_GET, workspaceId); +await env.mockIpcRenderer.invoke(IPC_CHANNELS.WORKSPACE_CREATE, projectPath, branchName); +```` - If IPC is hard to test, fix the test infrastructure or IPC layer, don't work around it by bypassing IPC. +**Acceptable exceptions:** + +- Reading context (like `env.config.loadConfigOrDefault()`) to prepare IPC call parameters +- Verifying filesystem state (like checking if files exist) after IPC operations complete +- Loading existing data to avoid expensive API calls in test setup + +If IPC is hard to test, fix the test infrastructure or IPC layer, don't work around it by bypassing IPC. ## Command Palette (Cmd+Shift+P) @@ -486,3 +489,7 @@ should go through `log.debug()`. - If you're fixing via simplifcation, a new test case is generally not necessary. - If fixing through additional complexity, add a test case if an existing convenient harness exists. - Otherwise if creating complexity, propose a new test harness to contain the new tests. + +## Mode: Exec + +If a user requests `wait_pr_checks`, treat it as a directive to keep running that process and address failures continuously. Do not return to the user until the checks succeed or you encounter a blocker you cannot resolve alone. This mode signals that the user expects persistent execution without further prompting. If static checks fail remotely, reproduce the error locally with `make static-check` before responding. If formatting issues are flagged, run `make fmt` to fix them before retrying CI. diff --git a/scripts/wait_pr_checks.sh b/scripts/wait_pr_checks.sh index 795d3400a3..7a3021e1b0 100755 --- a/scripts/wait_pr_checks.sh +++ b/scripts/wait_pr_checks.sh @@ -83,6 +83,7 @@ while true; do if ! ./scripts/check_pr_reviews.sh "$PR_NUMBER" >/dev/null 2>&1; then echo "" echo "❌ Unresolved review comments found!" + echo " 👉 Tip: run ./scripts/check_pr_reviews.sh "$PR_NUMBER" to list them." ./scripts/check_pr_reviews.sh "$PR_NUMBER" exit 1 fi @@ -103,6 +104,7 @@ while true; do else echo "" echo "❌ Please resolve Codex comments before merging." + echo " 👉 Tip: use ./scripts/resolve_codex_comment.sh "$PR_NUMBER" to apply Codex suggestions from the CLI." exit 1 fi elif [ "$MERGE_STATE" = "BLOCKED" ]; then diff --git a/src/components/Messages/ToolMessage.tsx b/src/components/Messages/ToolMessage.tsx index 926712bf08..47a8bf5a8d 100644 --- a/src/components/Messages/ToolMessage.tsx +++ b/src/components/Messages/ToolMessage.tsx @@ -11,10 +11,12 @@ import type { BashToolResult, FileReadToolArgs, FileReadToolResult, - FileEditReplaceToolArgs, FileEditInsertToolArgs, - FileEditReplaceToolResult, FileEditInsertToolResult, + FileEditReplaceLinesToolArgs, + FileEditReplaceLinesToolResult, + FileEditReplaceStringToolArgs, + FileEditReplaceStringToolResult, ProposePlanToolArgs, ProposePlanToolResult, } from "@/types/tools"; @@ -37,9 +39,20 @@ function isFileReadTool(toolName: string, args: unknown): args is FileReadToolAr return TOOL_DEFINITIONS.file_read.schema.safeParse(args).success; } -function isFileEditReplaceTool(toolName: string, args: unknown): args is FileEditReplaceToolArgs { - if (toolName !== "file_edit_replace") return false; - return TOOL_DEFINITIONS.file_edit_replace.schema.safeParse(args).success; +function isFileEditReplaceStringTool( + toolName: string, + args: unknown +): args is FileEditReplaceStringToolArgs { + if (toolName !== "file_edit_replace_string") return false; + return TOOL_DEFINITIONS.file_edit_replace_string.schema.safeParse(args).success; +} + +function isFileEditReplaceLinesTool( + toolName: string, + args: unknown +): args is FileEditReplaceLinesToolArgs { + if (toolName !== "file_edit_replace_lines") return false; + return TOOL_DEFINITIONS.file_edit_replace_lines.schema.safeParse(args).success; } function isFileEditInsertTool(toolName: string, args: unknown): args is FileEditInsertToolArgs { @@ -79,13 +92,26 @@ export const ToolMessage: React.FC = ({ message, className, wo ); } - if (isFileEditReplaceTool(message.toolName, message.args)) { + if (isFileEditReplaceStringTool(message.toolName, message.args)) { + return ( +
+ +
+ ); + } + + if (isFileEditReplaceLinesTool(message.toolName, message.args)) { return (
diff --git a/src/components/TitleBar.tsx b/src/components/TitleBar.tsx index e2f4dc391b..31bc75057f 100644 --- a/src/components/TitleBar.tsx +++ b/src/components/TitleBar.tsx @@ -30,6 +30,20 @@ const BuildInfo = styled.div` cursor: default; `; +interface VersionMetadata { + buildTime: string; + git_describe?: unknown; +} + +function hasBuildInfo(value: unknown): value is VersionMetadata { + if (typeof value !== "object" || value === null) { + return false; + } + + const candidate = value as Record; + return typeof candidate.buildTime === "string"; +} + function formatUSDate(isoDate: string): string { const date = new Date(isoDate); const month = String(date.getUTCMonth() + 1).padStart(2, "0"); @@ -51,13 +65,31 @@ function formatExtendedTimestamp(isoDate: string): string { }); } +function parseBuildInfo(version: unknown) { + if (hasBuildInfo(version)) { + const { buildTime, git_describe } = version; + const gitDescribe = typeof git_describe === "string" ? git_describe : undefined; + + return { + buildDate: formatUSDate(buildTime), + extendedTimestamp: formatExtendedTimestamp(buildTime), + gitDescribe, + }; + } + + return { + buildDate: "unknown", + extendedTimestamp: "Unknown build time", + gitDescribe: undefined, + }; +} + export function TitleBar() { - const buildDate = formatUSDate(VERSION.buildTime); - const extendedTimestamp = formatExtendedTimestamp(VERSION.buildTime); + const { buildDate, extendedTimestamp, gitDescribe } = parseBuildInfo(VERSION satisfies unknown); return ( - cmux {VERSION.git_describe} + cmux {gitDescribe ?? "(dev)"} {buildDate} Built at {extendedTimestamp} diff --git a/src/components/tools/FileEditToolCall.tsx b/src/components/tools/FileEditToolCall.tsx index 73cc831ff3..a1beb2c31a 100644 --- a/src/components/tools/FileEditToolCall.tsx +++ b/src/components/tools/FileEditToolCall.tsx @@ -2,10 +2,12 @@ import React from "react"; import styled from "@emotion/styled"; import { parsePatch } from "diff"; import type { - FileEditReplaceToolArgs, - FileEditReplaceToolResult, FileEditInsertToolArgs, FileEditInsertToolResult, + FileEditReplaceLinesToolArgs, + FileEditReplaceLinesToolResult, + FileEditReplaceStringToolArgs, + FileEditReplaceStringToolResult, } from "@/types/tools"; import { ToolContainer, @@ -177,11 +179,18 @@ const ButtonGroup = styled.div` margin-right: 8px; `; -type FileEditToolArgs = FileEditReplaceToolArgs | FileEditInsertToolArgs; -type FileEditToolResult = FileEditReplaceToolResult | FileEditInsertToolResult; +type FileEditToolArgs = + | FileEditReplaceStringToolArgs + | FileEditReplaceLinesToolArgs + | FileEditInsertToolArgs; + +type FileEditToolResult = + | FileEditReplaceStringToolResult + | FileEditReplaceLinesToolResult + | FileEditInsertToolResult; interface FileEditToolCallProps { - toolName: "file_edit_replace" | "file_edit_insert"; + toolName: "file_edit_replace_string" | "file_edit_replace_lines" | "file_edit_insert"; args: FileEditToolArgs; result?: FileEditToolResult; status?: ToolStatus; @@ -262,7 +271,7 @@ export const FileEditToolCall: React.FC = ({ const [showRaw, setShowRaw] = React.useState(false); const [copied, setCopied] = React.useState(false); - const filePath = args.file_path; + const filePath = "file_path" in args ? args.file_path : undefined; const handleCopyPatch = async () => { if (result && result.success && result.diff) { diff --git a/src/services/streamManager.test.ts b/src/services/streamManager.test.ts index abef27314c..687db0affc 100644 --- a/src/services/streamManager.test.ts +++ b/src/services/streamManager.test.ts @@ -344,14 +344,14 @@ describe("StreamManager - Unavailable Tool Handling", () => { yield { type: "tool-call", toolCallId: "test-call-1", - toolName: "file_edit_replace", + toolName: "file_edit_replace_string", input: { file_path: "/test", edits: [] }, }; // SDK emits tool-error when tool execution fails yield { type: "tool-error", toolCallId: "test-call-1", - toolName: "file_edit_replace", + toolName: "file_edit_replace_string", error: "Tool not found", }; })(), @@ -382,11 +382,11 @@ describe("StreamManager - Unavailable Tool Handling", () => { expect(events.length).toBeGreaterThanOrEqual(2); expect(events[0]).toMatchObject({ type: "tool-call-start", - toolName: "file_edit_replace", + toolName: "file_edit_replace_string", }); expect(events[1]).toMatchObject({ type: "tool-call-end", - toolName: "file_edit_replace", + toolName: "file_edit_replace_string", }); // Verify error result diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 6bc8502bf3..525f77f17e 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -1,16 +1,11 @@ import { tool } from "ai"; import * as fs from "fs/promises"; import * as path from "path"; -import writeFileAtomic from "write-file-atomic"; import type { FileEditInsertToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { - generateDiff, - validatePathInCwd, - validateFileSize, - WRITE_DENIED_PREFIX, -} from "./fileCommon"; +import { validatePathInCwd, WRITE_DENIED_PREFIX } from "./fileCommon"; +import { executeFileEditOperation } from "./file_edit_operation"; /** * File edit insert tool factory for AI assistant @@ -28,7 +23,6 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) create, }): Promise => { try { - // Validate that the path is within the working directory const pathValidation = validatePathInCwd(file_path, config.cwd); if (pathValidation) { return { @@ -37,99 +31,67 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }; } - // Resolve path (but expect absolute paths) + if (line_offset < 0) { + return { + success: false, + error: `${WRITE_DENIED_PREFIX} line_offset must be non-negative (got ${line_offset})`, + }; + } + const resolvedPath = path.isAbsolute(file_path) ? file_path : path.resolve(config.cwd, file_path); - // Check if file exists - let originalContent = ""; + let fileExists = await fs + .stat(resolvedPath) + .then((stats) => stats.isFile()) + .catch(() => false); - try { - const stats = await fs.stat(resolvedPath); - if (!stats.isFile()) { + if (!fileExists) { + if (!create) { return { success: false, - error: `${WRITE_DENIED_PREFIX} Path exists but is not a file: ${resolvedPath}`, + error: `${WRITE_DENIED_PREFIX} File not found: ${file_path}. To create it, set create: true`, }; } - // Validate file size - const sizeValidation = validateFileSize(stats); - if (sizeValidation) { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} ${sizeValidation.error}`, - }; - } + const parentDir = path.dirname(resolvedPath); + await fs.mkdir(parentDir, { recursive: true }); + await fs.writeFile(resolvedPath, ""); + fileExists = true; + } + + return executeFileEditOperation({ + config, + filePath: file_path, + operation: (originalContent) => { + const lines = originalContent.split("\n"); - // Read file content - originalContent = await fs.readFile(resolvedPath, { encoding: "utf-8" }); - } catch (error) { - // If file doesn't exist and create is not true, return error with suggestion - if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") { - if (!create) { + if (line_offset > lines.length) { return { success: false, - error: `${WRITE_DENIED_PREFIX} File not found: ${file_path}. To create it, set create: true`, + error: `line_offset ${line_offset} is beyond file length (${lines.length} lines)`, }; } - // File doesn't exist but create is true, so we'll create it - // Ensure parent directory exists - const parentDir = path.dirname(resolvedPath); - await fs.mkdir(parentDir, { recursive: true }); - originalContent = ""; - } else { - // Re-throw other errors - throw error; - } - } - const lines = originalContent.split("\n"); + const newLines = [...lines.slice(0, line_offset), content, ...lines.slice(line_offset)]; + const newContent = newLines.join("\n"); - // Validate line_offset - if (line_offset < 0) { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} line_offset must be non-negative (got ${line_offset})`, - }; - } - - if (line_offset > lines.length) { + return { + success: true, + newContent, + metadata: {}, + }; + }, + }); + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "EACCES") { return { success: false, - error: `${WRITE_DENIED_PREFIX} line_offset ${line_offset} is beyond file length (${lines.length} lines)`, + error: `${WRITE_DENIED_PREFIX} Permission denied: ${file_path}`, }; } - // Insert content at specified line - // line_offset = 0: insert at top (before line 1) - // line_offset = N: insert after line N - const newLines = [...lines.slice(0, line_offset), content, ...lines.slice(line_offset)]; - const newContent = newLines.join("\n"); - - // Write the modified content back to file atomically - await writeFileAtomic(resolvedPath, newContent, { encoding: "utf-8" }); - - // Generate diff - const diff = generateDiff(resolvedPath, originalContent, newContent); - - return { - success: true, - diff, - }; - } catch (error) { - // Handle specific errors - if (error && typeof error === "object" && "code" in error) { - if (error.code === "EACCES") { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} Permission denied: ${file_path}`, - }; - } - } - - // Generic error const message = error instanceof Error ? error.message : String(error); return { success: false, diff --git a/src/services/tools/file_edit_operation.test.ts b/src/services/tools/file_edit_operation.test.ts new file mode 100644 index 0000000000..81ac2f2900 --- /dev/null +++ b/src/services/tools/file_edit_operation.test.ts @@ -0,0 +1,24 @@ +import { describe, it, expect } from "bun:test"; +import { executeFileEditOperation } from "./file_edit_operation"; +import { WRITE_DENIED_PREFIX } from "./fileCommon"; + +const TEST_CWD = "/tmp"; + +function createConfig() { + return { cwd: TEST_CWD }; +} + +describe("executeFileEditOperation", () => { + it("should return error when path validation fails", async () => { + const result = await executeFileEditOperation({ + config: createConfig(), + filePath: "../../etc/passwd", + operation: () => ({ success: true, newContent: "", metadata: {} }), + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.startsWith(WRITE_DENIED_PREFIX)).toBe(true); + } + }); +}); diff --git a/src/services/tools/file_edit_operation.ts b/src/services/tools/file_edit_operation.ts new file mode 100644 index 0000000000..0a87c922e9 --- /dev/null +++ b/src/services/tools/file_edit_operation.ts @@ -0,0 +1,113 @@ +import * as fs from "fs/promises"; +import * as path from "path"; +import writeFileAtomic from "write-file-atomic"; +import type { FileEditDiffSuccessBase, FileEditErrorResult } from "@/types/tools"; +import type { ToolConfiguration } from "@/utils/tools/tools"; +import { + generateDiff, + validateFileSize, + validatePathInCwd, + WRITE_DENIED_PREFIX, +} from "./fileCommon"; + +type FileEditOperationResult = + | { + success: true; + newContent: string; + metadata: TMetadata; + } + | { + success: false; + error: string; + }; + +interface ExecuteFileEditOperationOptions { + config: ToolConfiguration; + filePath: string; + operation: ( + originalContent: string + ) => FileEditOperationResult | Promise>; +} + +/** + * Shared execution pipeline for file edit tools. + * Handles validation, file IO, diff generation, and common error handling. + */ +export async function executeFileEditOperation({ + config, + filePath, + operation, +}: ExecuteFileEditOperationOptions): Promise< + FileEditErrorResult | (FileEditDiffSuccessBase & TMetadata) +> { + try { + const pathValidation = validatePathInCwd(filePath, config.cwd); + if (pathValidation) { + return { + success: false, + error: `${WRITE_DENIED_PREFIX} ${pathValidation.error}`, + }; + } + + const resolvedPath = path.isAbsolute(filePath) ? filePath : path.resolve(config.cwd, filePath); + + const stats = await fs.stat(resolvedPath); + if (!stats.isFile()) { + return { + success: false, + error: `${WRITE_DENIED_PREFIX} Path exists but is not a file: ${resolvedPath}`, + }; + } + + const sizeValidation = validateFileSize(stats); + if (sizeValidation) { + return { + success: false, + error: `${WRITE_DENIED_PREFIX} ${sizeValidation.error}`, + }; + } + + const originalContent = await fs.readFile(resolvedPath, { encoding: "utf-8" }); + + const operationResult = await Promise.resolve(operation(originalContent)); + if (!operationResult.success) { + return { + success: false, + error: `${WRITE_DENIED_PREFIX} ${operationResult.error}`, + }; + } + + await writeFileAtomic(resolvedPath, operationResult.newContent, { encoding: "utf-8" }); + + const diff = generateDiff(resolvedPath, originalContent, operationResult.newContent); + + return { + success: true, + diff, + ...operationResult.metadata, + }; + } catch (error) { + if (error && typeof error === "object" && "code" in error) { + const nodeError = error as { code?: string }; + if (nodeError.code === "ENOENT") { + return { + success: false, + error: `${WRITE_DENIED_PREFIX} File not found: ${filePath}`, + }; + } + + if (nodeError.code === "EACCES") { + return { + success: false, + error: `${WRITE_DENIED_PREFIX} Permission denied: ${filePath}`, + }; + } + } + + const message = error instanceof Error ? error.message : String(error); + return { + success: false, + error: `${WRITE_DENIED_PREFIX} Failed to edit file: ${message}`, + }; + } +} diff --git a/src/services/tools/file_edit_replace.ts b/src/services/tools/file_edit_replace.ts deleted file mode 100644 index 31198c2a5f..0000000000 --- a/src/services/tools/file_edit_replace.ts +++ /dev/null @@ -1,174 +0,0 @@ -import { tool } from "ai"; -import * as fs from "fs/promises"; -import * as path from "path"; -import writeFileAtomic from "write-file-atomic"; -import type { FileEditReplaceToolResult } from "@/types/tools"; -import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; -import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { - generateDiff, - validatePathInCwd, - validateFileSize, - WRITE_DENIED_PREFIX, -} from "./fileCommon"; - -/** - * File edit replace tool factory for AI assistant - * Creates a tool that allows the AI to apply multiple edits to a file - * @param config Required configuration including working directory - */ -export const createFileEditReplaceTool: ToolFactory = (config: ToolConfiguration) => { - return tool({ - description: TOOL_DEFINITIONS.file_edit_replace.description, - inputSchema: TOOL_DEFINITIONS.file_edit_replace.schema, - execute: async ( - { file_path, edits }, - { abortSignal: _abortSignal } - ): Promise => { - // Note: abortSignal available but not used - file operations are fast and complete quickly - try { - // Validate that the path is within the working directory - const pathValidation = validatePathInCwd(file_path, config.cwd); - if (pathValidation) { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} ${pathValidation.error}`, - }; - } - - // Resolve path (but expect absolute paths) - const resolvedPath = path.isAbsolute(file_path) - ? file_path - : path.resolve(config.cwd, file_path); - - // Check if file exists - const stats = await fs.stat(resolvedPath); - if (!stats.isFile()) { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} Path exists but is not a file: ${resolvedPath}`, - }; - } - - // Validate file size - const sizeValidation = validateFileSize(stats); - if (sizeValidation) { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} ${sizeValidation.error}`, - }; - } - - // Read file content - const originalContent = await fs.readFile(resolvedPath, { encoding: "utf-8" }); - let content = originalContent; - - // Apply each edit sequentially - // THE KEY INSIGHT: MultiEdit is a state machine where each edit operates on the - // output of the previous edit, not the original file. - // - // For example, if the file contains "foo bar baz": - // Edit 1: "foo" -> "FOO" transforms content to "FOO bar baz" - // Edit 2: "bar" -> "BAR" operates on "FOO bar baz", resulting in "FOO BAR baz" - // Edit 3: "foo" -> "qux" would FAIL because "foo" no longer exists in the current state - // - // If ANY edit fails, the entire operation is rolled back - the file remains unchanged - // because we only write to disk after all edits succeed. - let editsApplied = 0; - for (let i = 0; i < edits.length; i++) { - const edit = edits[i]; - const replaceCount = edit.replace_count ?? 1; // Default to 1 - - // Validate old_string exists in content - if (!content.includes(edit.old_string)) { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} Edit ${i + 1}: old_string not found in file. The text to replace must exist exactly as written in the file.`, - }; - } - - // Count occurrences - const parts = content.split(edit.old_string); - const occurrences = parts.length - 1; - - // Check for uniqueness if replace_count is 1 - if (replaceCount === 1 && occurrences > 1) { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} Edit ${i + 1}: old_string appears ${occurrences} times in the file. Either expand the context to make it unique or set replace_count to ${occurrences} or -1.`, - }; - } - - // Validate replace_count doesn't exceed occurrences (unless -1) - if (replaceCount > occurrences && replaceCount !== -1) { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} Edit ${i + 1}: replace_count is ${replaceCount} but old_string only appears ${occurrences} time(s) in the file.`, - }; - } - - // Apply the edit - if (replaceCount === -1) { - // Replace all occurrences - content = parts.join(edit.new_string); - editsApplied += occurrences; - } else { - // Replace the specified number of occurrences - let replacedCount = 0; - let currentContent = content; - - for (let j = 0; j < replaceCount; j++) { - const index = currentContent.indexOf(edit.old_string); - if (index !== -1) { - currentContent = - currentContent.substring(0, index) + - edit.new_string + - currentContent.substring(index + edit.old_string.length); - replacedCount++; - } else { - break; - } - } - - content = currentContent; - editsApplied += replacedCount; - } - } - - // Write the modified content back to file atomically - await writeFileAtomic(resolvedPath, content, { encoding: "utf-8" }); - - // Generate diff - const diff = generateDiff(resolvedPath, originalContent, content); - - return { - success: true, - edits_applied: editsApplied, - diff, - }; - } catch (error) { - // Handle specific errors - if (error && typeof error === "object" && "code" in error) { - if (error.code === "ENOENT") { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} File not found: ${file_path}`, - }; - } else if (error.code === "EACCES") { - return { - success: false, - error: `${WRITE_DENIED_PREFIX} Permission denied: ${file_path}`, - }; - } - } - - // Generic error - const message = error instanceof Error ? error.message : String(error); - return { - success: false, - error: `${WRITE_DENIED_PREFIX} Failed to edit file: ${message}`, - }; - } - }, - }); -}; diff --git a/src/services/tools/file_edit_replace_lines.ts b/src/services/tools/file_edit_replace_lines.ts new file mode 100644 index 0000000000..a44dc5d48b --- /dev/null +++ b/src/services/tools/file_edit_replace_lines.ts @@ -0,0 +1,88 @@ +import { tool } from "ai"; +import type { FileEditReplaceLinesToolArgs, FileEditReplaceLinesToolResult } from "@/types/tools"; +import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; +import { executeFileEditOperation } from "./file_edit_operation"; + +/** + * File edit replace (lines) tool factory for AI assistant + * Applies line-range replacements sequentially with optional content validation. + */ +export const createFileEditReplaceLinesTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.file_edit_replace_lines.description, + inputSchema: TOOL_DEFINITIONS.file_edit_replace_lines.schema, + execute: async ( + args: FileEditReplaceLinesToolArgs + ): Promise => { + return executeFileEditOperation({ + config, + filePath: args.file_path, + operation: (originalContent) => { + let lines = originalContent.split("\n"); + let linesReplaced = 0; + let totalDelta = 0; + + for (let i = 0; i < args.edits.length; i++) { + const edit = args.edits[i]; + const startIndex = edit.start_line - 1; + const endIndex = edit.end_line - 1; + + if (edit.start_line <= 0) { + return { + success: false, + error: `Edit ${i + 1}: start_line must be >= 1 (received ${edit.start_line}).`, + }; + } + + if (edit.end_line < edit.start_line) { + return { + success: false, + error: `Edit ${i + 1}: end_line must be >= start_line (received start ${edit.start_line}, end ${edit.end_line}).`, + }; + } + + if (startIndex >= lines.length) { + return { + success: false, + error: `Edit ${i + 1}: start_line ${edit.start_line} exceeds current file length (${lines.length}).`, + }; + } + + const clampedEndIndex = Math.min(endIndex, lines.length - 1); + const currentRange = lines.slice(startIndex, clampedEndIndex + 1); + + if (edit.expected_lines && !arraysEqual(currentRange, edit.expected_lines)) { + return { + success: false, + error: `Edit ${i + 1}: expected_lines validation failed. Current lines [${currentRange.join("\n")}] differ from expected [${edit.expected_lines.join("\n")}].`, + }; + } + + const before = lines.slice(0, startIndex); + const after = lines.slice(clampedEndIndex + 1); + lines = [...before, ...edit.new_lines, ...after]; + + linesReplaced += currentRange.length; + totalDelta += edit.new_lines.length - currentRange.length; + } + + return { + success: true, + newContent: lines.join("\n"), + metadata: { + edits_applied: args.edits.length, + lines_replaced: linesReplaced, + line_delta: totalDelta, + }, + }; + }, + }); + }, + }); +}; + +function arraysEqual(a: string[], b: string[]): boolean { + if (a.length !== b.length) return false; + return a.every((value, index) => value === b[index]); +} diff --git a/src/services/tools/file_edit_replace.test.ts b/src/services/tools/file_edit_replace_string.test.ts similarity index 56% rename from src/services/tools/file_edit_replace.test.ts rename to src/services/tools/file_edit_replace_string.test.ts index 3ab3cb19be..25d8422b8d 100644 --- a/src/services/tools/file_edit_replace.test.ts +++ b/src/services/tools/file_edit_replace_string.test.ts @@ -2,8 +2,8 @@ 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 { createFileEditReplaceTool } from "./file_edit_replace"; -import type { FileEditReplaceToolArgs, FileEditReplaceToolResult } from "@/types/tools"; +import { createFileEditReplaceStringTool } from "./file_edit_replace_string"; +import type { FileEditReplaceStringToolArgs, FileEditReplaceStringToolResult } from "@/types/tools"; import type { ToolCallOptions } from "ai"; // Mock ToolCallOptions for testing @@ -22,15 +22,15 @@ const readFile = async (filePath: string): Promise => { }; const executeReplace = async ( - tool: ReturnType, + tool: ReturnType, filePath: string, - edits: FileEditReplaceToolArgs["edits"] -): Promise => { - const args: FileEditReplaceToolArgs = { file_path: filePath, edits }; - return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + edits: FileEditReplaceStringToolArgs["edits"] +): Promise => { + const args: FileEditReplaceStringToolArgs = { file_path: filePath, edits }; + return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceStringToolResult; }; -describe("file_edit_replace tool", () => { +describe("file_edit_replace_string tool", () => { let testDir: string; let testFilePath: string; @@ -47,7 +47,7 @@ describe("file_edit_replace tool", () => { it("should apply a single edit successfully", async () => { await setupFile(testFilePath, "Hello world\nThis is a test\nGoodbye world"); - const tool = createFileEditReplaceTool({ cwd: testDir }); + const tool = createFileEditReplaceStringTool({ cwd: testDir }); const result = await executeReplace(tool, testFilePath, [ { old_string: "Hello world", new_string: "Hello universe" }, @@ -63,7 +63,7 @@ describe("file_edit_replace tool", () => { it("should apply multiple edits sequentially", async () => { await setupFile(testFilePath, "foo bar baz"); - const tool = createFileEditReplaceTool({ cwd: testDir }); + const tool = createFileEditReplaceStringTool({ cwd: testDir }); const result = await executeReplace(tool, testFilePath, [ { old_string: "foo", new_string: "FOO" }, @@ -80,67 +80,42 @@ describe("file_edit_replace tool", () => { }); it("should rollback if later edit fails (first edit breaks second edit search)", async () => { - // Setup - This test demonstrates that multi-edit is a state machine: - // each edit operates on the OUTPUT of the previous edit, not the original file. - // If any edit fails, the entire operation is rolled back (file unchanged). const initialContent = "foo bar baz"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, edits: [ - { - old_string: "foo", - new_string: "FOO", - }, - { - // This edit will FAIL because "foo" was already replaced by the first edit - // The second edit operates on "FOO bar baz", not "foo bar baz" - old_string: "foo", - new_string: "qux", - }, + { old_string: "foo", new_string: "FOO" }, + { old_string: "foo", new_string: "qux" }, ], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert - The operation should fail expect(result.success).toBe(false); if (!result.success) { expect(result.error).toContain("Edit 2"); expect(result.error).toContain("old_string not found"); } - // Critical assertion: File should remain UNCHANGED because edit failed - // The atomic write should not have occurred const finalContent = await fs.readFile(testFilePath, "utf-8"); expect(finalContent).toBe(initialContent); - expect(finalContent).toBe("foo bar baz"); // Still the original content }); it("should replace all occurrences when replace_count is -1", async () => { - // Setup const initialContent = "cat dog cat bird cat"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "cat", - new_string: "mouse", - replace_count: -1, - }, - ], + edits: [{ old_string: "cat", new_string: "mouse", replace_count: -1 }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(3); @@ -151,26 +126,17 @@ describe("file_edit_replace tool", () => { }); it("should replace unique occurrence when replace_count defaults to 1", async () => { - // Setup const initialContent = "cat dog bird"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "cat", - new_string: "mouse", - // replace_count omitted, defaults to 1 - }, - ], + edits: [{ old_string: "cat", new_string: "mouse" }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(1); @@ -181,89 +147,60 @@ describe("file_edit_replace tool", () => { }); it("should fail when old_string is not found", async () => { - // Setup const initialContent = "Hello world"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "nonexistent", - new_string: "replacement", - }, - ], + edits: [{ old_string: "nonexistent", new_string: "replacement" }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(false); if (!result.success) { expect(result.error).toContain("old_string not found"); } - // File should remain unchanged const unchangedContent = await fs.readFile(testFilePath, "utf-8"); expect(unchangedContent).toBe(initialContent); }); it("should fail when old_string appears multiple times with replace_count of 1", async () => { - // Setup const initialContent = "cat dog cat bird cat"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "cat", - new_string: "mouse", - replace_count: 1, // Explicitly set to 1 - }, - ], + edits: [{ old_string: "cat", new_string: "mouse", replace_count: 1 }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(false); if (!result.success) { expect(result.error).toContain("appears 3 times"); - expect(result.error).toContain("expand the context to make it unique"); expect(result.error).toContain("replace_count to 3 or -1"); } - // File should remain unchanged const unchangedContent = await fs.readFile(testFilePath, "utf-8"); expect(unchangedContent).toBe(initialContent); }); it("should replace exactly N occurrences when replace_count is N", async () => { - // Setup const initialContent = "cat dog cat bird cat"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "cat", - new_string: "mouse", - replace_count: 2, // Replace first 2 occurrences - }, - ], + edits: [{ old_string: "cat", new_string: "mouse", replace_count: 2 }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(2); @@ -274,56 +211,38 @@ describe("file_edit_replace tool", () => { }); it("should fail when replace_count exceeds actual occurrences", async () => { - // Setup const initialContent = "cat dog bird"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "cat", - new_string: "mouse", - replace_count: 5, // Only 1 occurrence exists - }, - ], + edits: [{ old_string: "cat", new_string: "mouse", replace_count: 5 }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(false); if (!result.success) { expect(result.error).toContain("replace_count is 5"); expect(result.error).toContain("only appears 1 time(s)"); } - // File should remain unchanged const unchangedContent = await fs.readFile(testFilePath, "utf-8"); expect(unchangedContent).toBe(initialContent); }); it("should fail when file does not exist", async () => { - // Setup const nonExistentPath = path.join(testDir, "nonexistent.txt"); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: nonExistentPath, - edits: [ - { - old_string: "foo", - new_string: "bar", - }, - ], + edits: [{ old_string: "foo", new_string: "bar" }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, nonExistentPath, args.edits); - // Assert expect(result.success).toBe(false); if (!result.success) { expect(result.error).toContain("File not found"); @@ -331,25 +250,17 @@ describe("file_edit_replace tool", () => { }); it("should handle multiline edits", async () => { - // Setup const initialContent = "line1\nline2\nline3\nline4"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "line2\nline3", - new_string: "REPLACED", - }, - ], + edits: [{ old_string: "line2\nline3", new_string: "REPLACED" }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(1); @@ -360,25 +271,17 @@ describe("file_edit_replace tool", () => { }); it("should handle empty string replacement", async () => { - // Setup const initialContent = "Hello [DELETE_ME] world"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "[DELETE_ME] ", - new_string: "", - }, - ], + edits: [{ old_string: "[DELETE_ME] ", new_string: "" }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(1); @@ -389,29 +292,20 @@ describe("file_edit_replace tool", () => { }); it("should handle edits that depend on previous edits", async () => { - // Setup const initialContent = "step1"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, edits: [ - { - old_string: "step1", - new_string: "step2", - }, - { - old_string: "step2", - new_string: "step3", - }, + { old_string: "step1", new_string: "step2" }, + { old_string: "step2", new_string: "step3" }, ], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(true); if (result.success) { expect(result.edits_applied).toBe(2); @@ -422,25 +316,17 @@ describe("file_edit_replace tool", () => { }); it("should return unified diff with context of 3", async () => { - // Setup - create a file with multiple lines const initialContent = "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9"; await fs.writeFile(testFilePath, initialContent); - const tool = createFileEditReplaceTool({ cwd: testDir }); - const args: FileEditReplaceToolArgs = { + const tool = createFileEditReplaceStringTool({ cwd: testDir }); + const args: FileEditReplaceStringToolArgs = { file_path: testFilePath, - edits: [ - { - old_string: "line5", - new_string: "LINE5_MODIFIED", - }, - ], + edits: [{ old_string: "line5", new_string: "LINE5_MODIFIED" }], }; - // Execute - const result = (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; + const result = await executeReplace(tool, testFilePath, args.edits); - // Assert expect(result.success).toBe(true); if (result.success) { expect(result.diff).toBeDefined(); @@ -448,17 +334,12 @@ describe("file_edit_replace tool", () => { expect(result.diff).toContain("+++ " + testFilePath); expect(result.diff).toContain("-line5"); expect(result.diff).toContain("+LINE5_MODIFIED"); - - // Verify context of 3 - should include 3 lines before and after expect(result.diff).toContain("line2"); expect(result.diff).toContain("line3"); expect(result.diff).toContain("line4"); expect(result.diff).toContain("line6"); expect(result.diff).toContain("line7"); expect(result.diff).toContain("line8"); - - // Lines outside the context should not be in the diff (line1, line9) - // Note: This depends on the exact diff format, so we verify at least the key parts are present } }); }); diff --git a/src/services/tools/file_edit_replace_string.ts b/src/services/tools/file_edit_replace_string.ts new file mode 100644 index 0000000000..2caedc5d55 --- /dev/null +++ b/src/services/tools/file_edit_replace_string.ts @@ -0,0 +1,89 @@ +import { tool } from "ai"; +import type { FileEditReplaceStringToolArgs, FileEditReplaceStringToolResult } from "@/types/tools"; +import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; +import { executeFileEditOperation } from "./file_edit_operation"; + +/** + * File edit replace (string) tool factory for AI assistant + * Applies multiple text replacement edits sequentially against file content. + */ +export const createFileEditReplaceStringTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.file_edit_replace_string.description, + inputSchema: TOOL_DEFINITIONS.file_edit_replace_string.schema, + execute: async ( + args: FileEditReplaceStringToolArgs + ): Promise => { + return executeFileEditOperation({ + config, + filePath: args.file_path, + operation: (originalContent) => { + let content = originalContent; + let editsApplied = 0; + + for (let i = 0; i < args.edits.length; i++) { + const edit = args.edits[i]; + const replaceCount = edit.replace_count ?? 1; + + if (!content.includes(edit.old_string)) { + return { + success: false, + error: `Edit ${i + 1}: old_string not found in file. The text to replace must exist exactly as written in the file.`, + }; + } + + const parts = content.split(edit.old_string); + const occurrences = parts.length - 1; + + if (replaceCount === 1 && occurrences > 1) { + return { + success: false, + error: `Edit ${i + 1}: old_string appears ${occurrences} times in the file. Either expand the context to make it unique or set replace_count to ${occurrences} or -1.`, + }; + } + + if (replaceCount > occurrences && replaceCount !== -1) { + return { + success: false, + error: `Edit ${i + 1}: replace_count is ${replaceCount} but old_string only appears ${occurrences} time(s) in the file.`, + }; + } + + if (replaceCount === -1) { + content = parts.join(edit.new_string); + editsApplied += occurrences; + } else { + let replacedCount = 0; + let currentContent = content; + + for (let j = 0; j < replaceCount; j++) { + const index = currentContent.indexOf(edit.old_string); + if (index === -1) { + break; + } + + currentContent = + currentContent.substring(0, index) + + edit.new_string + + currentContent.substring(index + edit.old_string.length); + replacedCount++; + } + + content = currentContent; + editsApplied += replacedCount; + } + } + + return { + success: true, + newContent: content, + metadata: { + edits_applied: editsApplied, + }, + }; + }, + }); + }, + }); +}; diff --git a/src/types/tools.ts b/src/types/tools.ts index 11ae41c81d..0b12555e15 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -48,28 +48,67 @@ export type FileReadToolResult = error: string; }; -// File Edit Replace Tool Types -export interface FileEditReplaceEdit { +// File Edit Replace (String) Tool Types +export interface FileEditReplaceStringEdit { old_string: string; new_string: string; replace_count?: number; // Default: 1, -1 means replace all } -export interface FileEditReplaceToolArgs { +export interface FileEditReplaceStringToolArgs { file_path: string; - edits: FileEditReplaceEdit[]; + edits: FileEditReplaceStringEdit[]; } -export type FileEditReplaceToolResult = - | { - success: true; +export interface FileEditDiffSuccessBase { + success: true; + diff: string; +} + +export interface FileEditErrorResult { + success: false; + error: string; +} + +export type FileEditReplaceStringToolResult = + | (FileEditDiffSuccessBase & { edits_applied: number; - diff: string; - } - | { - success: false; - error: string; - }; + }) + | FileEditErrorResult; + +// File Edit Replace (Lines) Tool Types +export interface FileEditReplaceLinesEdit { + start_line: number; // 1-indexed inclusive start line + end_line: number; // 1-indexed inclusive end line + new_lines: string[]; // Replacement lines (empty array deletes the range) + expected_lines?: string[]; // Optional safety check of current lines prior to replacement +} + +export interface FileEditReplaceLinesToolArgs { + file_path: string; + edits: FileEditReplaceLinesEdit[]; +} + +export type FileEditReplaceLinesToolResult = + | (FileEditDiffSuccessBase & { + edits_applied: number; + lines_replaced: number; + line_delta: number; + }) + | FileEditErrorResult; + +export type FileEditSharedToolResult = + | FileEditReplaceStringToolResult + | 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]; // File Edit Insert Tool Types export interface FileEditInsertToolArgs { @@ -79,15 +118,22 @@ export interface FileEditInsertToolArgs { create?: boolean; // If true, create the file if it doesn't exist (default: false) } -export type FileEditInsertToolResult = - | { - success: true; - diff: string; - } - | { - success: false; - error: string; - }; +export type FileEditInsertToolResult = FileEditDiffSuccessBase | FileEditErrorResult; + +export type FileEditToolArgs = + | FileEditReplaceStringToolArgs + | FileEditReplaceLinesToolArgs + | FileEditInsertToolArgs; + +export interface FileEditToolMessage { + toolName: FileEditToolName; + args: FileEditToolArgs; + result?: FileEditSharedToolResult; +} + +export function isFileEditToolName(value: string): value is FileEditToolName { + return (FILE_EDIT_TOOL_NAMES as readonly string[]).includes(value); +} // Propose Plan Tool Types export interface ProposePlanToolArgs { diff --git a/src/utils/main/tokenizer.ts b/src/utils/main/tokenizer.ts index 180c28dbdb..5422eb3476 100644 --- a/src/utils/main/tokenizer.ts +++ b/src/utils/main/tokenizer.ts @@ -155,7 +155,8 @@ export function getToolDefinitionTokens(toolName: string, modelString: string): const fallbackSizes: Record = { bash: 65, file_read: 45, - file_edit_replace: 70, + file_edit_replace_string: 70, + file_edit_replace_lines: 65, 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 4c428845eb..f2dbaa3a45 100644 --- a/src/utils/messages/toolOutputRedaction.ts +++ b/src/utils/messages/toolOutputRedaction.ts @@ -10,7 +10,11 @@ * - Type safety: if a tool's result type changes, these redactors should fail type-checks. */ -import type { FileEditInsertToolResult, FileEditReplaceToolResult } from "@/types/tools"; +import type { + FileEditInsertToolResult, + FileEditReplaceLinesToolResult, + FileEditReplaceStringToolResult, +} from "@/types/tools"; // Tool-output from AI SDK is often wrapped like: { type: 'json', value: } // Keep this helper local so all redactors handle both wrapped and plain objects consistently. @@ -33,7 +37,16 @@ function rewrapJsonContainer(wrapped: boolean, value: unknown): unknown { } // Narrowing helpers for our tool result types -function isFileEditReplaceResult(v: unknown): v is FileEditReplaceToolResult { +function isFileEditReplaceStringResult(v: unknown): v is FileEditReplaceStringToolResult { + return ( + typeof v === "object" && + v !== null && + "success" in v && + typeof (v as { success: unknown }).success === "boolean" + ); +} + +function isFileEditReplaceLinesResult(v: unknown): v is FileEditReplaceLinesToolResult { return ( typeof v === "object" && v !== null && @@ -52,18 +65,16 @@ function isFileEditInsertResult(v: unknown): v is FileEditInsertToolResult { } // Redactors per tool -function redactFileEditReplace(output: unknown): unknown { +function redactFileEditReplaceString(output: unknown): unknown { const unwrapped = unwrapJsonContainer(output); const val = unwrapped.value; - if (!isFileEditReplaceResult(val)) return output; // unknown structure, leave as-is + if (!isFileEditReplaceStringResult(val)) return output; // unknown structure, leave as-is if (val.success) { - // Keep metadata but compact the diff to avoid huge context windows - const compact: FileEditReplaceToolResult = { + const compact: FileEditReplaceStringToolResult = { success: true, edits_applied: val.edits_applied, - // Replace the unified diff with a sentinel that tells the model how to fetch details if needed diff: "[diff omitted in context - call file_read on the target file if needed]", }; return rewrapJsonContainer(unwrapped.wrapped, compact); @@ -73,6 +84,26 @@ function redactFileEditReplace(output: unknown): unknown { return output; } +function redactFileEditReplaceLines(output: unknown): unknown { + const unwrapped = unwrapJsonContainer(output); + const val = unwrapped.value; + + if (!isFileEditReplaceLinesResult(val)) return output; + + if (val.success) { + const compact: FileEditReplaceLinesToolResult = { + success: true, + edits_applied: val.edits_applied, + lines_replaced: val.lines_replaced, + line_delta: val.line_delta, + diff: "[diff omitted in context - call file_read on the target file if needed]", + }; + return rewrapJsonContainer(unwrapped.wrapped, compact); + } + + return output; +} + function redactFileEditInsert(output: unknown): unknown { const unwrapped = unwrapJsonContainer(output); const val = unwrapped.value; @@ -93,8 +124,10 @@ function redactFileEditInsert(output: unknown): unknown { // Public API - registry entrypoint. Add new tools here as needed. export function redactToolOutput(toolName: string, output: unknown): unknown { switch (toolName) { - case "file_edit_replace": - return redactFileEditReplace(output); + case "file_edit_replace_string": + return redactFileEditReplaceString(output); + case "file_edit_replace_lines": + return redactFileEditReplaceLines(output); case "file_edit_insert": return redactFileEditInsert(output); default: diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 58d236c0f2..2199a2bb1f 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -66,7 +66,7 @@ export const TOOL_DEFINITIONS = { .describe("Number of lines to return from offset (optional, returns all if not specified)"), }), }, - file_edit_replace: { + file_edit_replace_string: { description: "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({ @@ -93,6 +93,35 @@ export const TOOL_DEFINITIONS = { .describe("Array of edits to apply sequentially"), }), }, + file_edit_replace_lines: { + description: + "Apply one or more edits to a file by replacing lines in a specific range. All edits are applied sequentially and line numbers refer to the file state after previous edits.", + schema: z.object({ + file_path: z.string().describe("The absolute path to the file to edit"), + edits: z + .array( + z.object({ + 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 + .array(z.string()) + .describe("Replacement lines. Provide an empty array to delete the specified range."), + expected_lines: z + .array(z.string()) + .optional() + .describe( + "Optional safety check. When provided, the current lines in the specified range must match exactly." + ), + }) + ) + .min(1) + .describe("Array of line replacement edits to apply sequentially"), + }), + }, 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.", @@ -174,7 +203,8 @@ export function getAvailableTools(modelString: string): string[] { const baseTools = [ "bash", "file_read", - "file_edit_replace", + "file_edit_replace_string", + "file_edit_replace_lines", "file_edit_insert", "propose_plan", "compact_summary", diff --git a/src/utils/tools/toolPolicy.test.ts b/src/utils/tools/toolPolicy.test.ts index 56c3a99ff9..f7ab014832 100644 --- a/src/utils/tools/toolPolicy.test.ts +++ b/src/utils/tools/toolPolicy.test.ts @@ -14,8 +14,13 @@ const mockTools = { inputSchema: z.object({ path: z.string() }), execute: () => Promise.resolve({ content: "test" }), }), - file_edit_replace: tool({ - description: "Replace content in files", + file_edit_replace_string: tool({ + description: "Replace content in files (string)", + inputSchema: z.object({ path: z.string() }), + execute: () => Promise.resolve({ success: true }), + }), + file_edit_replace_lines: tool({ + description: "Replace content in files (lines)", inputSchema: z.object({ path: z.string() }), execute: () => Promise.resolve({ success: true }), }), @@ -51,7 +56,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeUndefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace).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(); }); @@ -66,7 +72,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeUndefined(); expect(result.web_search).toBeUndefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace).toBeDefined(); + expect(result.file_edit_replace_string).toBeDefined(); + expect(result.file_edit_replace_lines).toBeDefined(); expect(result.file_edit_insert).toBeDefined(); }); }); @@ -76,7 +83,8 @@ describe("applyToolPolicy", () => { const policy: ToolPolicy = [{ regex_match: "file_edit_.*", action: "disable" }]; const result = applyToolPolicy(mockTools, policy); - expect(result.file_edit_replace).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.file_read).toBeDefined(); @@ -95,7 +103,8 @@ describe("applyToolPolicy", () => { const result = applyToolPolicy(mockTools, policy); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace).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(); @@ -112,19 +121,21 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace).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("disables file_edit_.* then enables file_edit_replace", () => { + test("disables file_edit_.* then enables file_edit_replace_string", () => { const policy: ToolPolicy = [ { regex_match: "file_edit_.*", action: "disable" }, - { regex_match: "file_edit_replace", action: "enable" }, + { regex_match: "file_edit_replace_string", action: "enable" }, ]; const result = applyToolPolicy(mockTools, policy); - expect(result.file_edit_replace).toBeDefined(); + 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(); @@ -149,7 +160,8 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.bash).toBeDefined(); - expect(result.file_edit_replace).toBeUndefined(); + expect(result.file_edit_replace_string).toBeUndefined(); + expect(result.file_edit_replace_lines).toBeUndefined(); expect(result.file_edit_insert).toBeUndefined(); }); @@ -158,7 +170,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace).toBeDefined(); + expect(result.file_edit_replace_string).toBeDefined(); + expect(result.file_edit_replace_lines).toBeDefined(); expect(result.file_edit_insert).toBeDefined(); }); @@ -172,7 +185,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace).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(); }); @@ -202,7 +216,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(Object.keys(result)).toHaveLength(1); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace).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(); }); diff --git a/src/utils/tools/tools.ts b/src/utils/tools/tools.ts index c026e16ef4..b290bdb5c0 100644 --- a/src/utils/tools/tools.ts +++ b/src/utils/tools/tools.ts @@ -4,7 +4,8 @@ import { openai } from "@ai-sdk/openai"; import { google } from "@ai-sdk/google"; import { createFileReadTool } from "@/services/tools/file_read"; import { createBashTool } from "@/services/tools/bash"; -import { createFileEditReplaceTool } from "@/services/tools/file_edit_replace"; +import { createFileEditReplaceStringTool } from "@/services/tools/file_edit_replace_string"; +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 { createCompactSummaryTool } from "@/services/tools/compact_summary"; @@ -43,7 +44,8 @@ export function getToolsForModel( const baseTools: Record = { // Use snake_case for tool names to match what seems to be the convention. file_read: createFileReadTool(config), - file_edit_replace: createFileEditReplaceTool(config), + file_edit_replace_string: createFileEditReplaceStringTool(config), + file_edit_replace_lines: createFileEditReplaceLinesTool(config), file_edit_insert: createFileEditInsertTool(config), bash: createBashTool(config), propose_plan: createProposePlanTool(config), From 0f1eb570dbcb587316bb7b87b531f33eb5ead5fb Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 12 Oct 2025 13:31:00 -0500 Subject: [PATCH 2/8] =?UTF-8?q?feat:=20=F0=9F=A4=96=20simplify=20file=20ed?= =?UTF-8?q?it=20tools=20to=20single=20edits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/services/streamManager.test.ts | 2 +- src/services/tools/file_edit_replace_lines.ts | 80 ++++---- .../tools/file_edit_replace_string.test.ts | 193 +++++++++--------- .../tools/file_edit_replace_string.ts | 89 ++++---- src/types/tools.ts | 16 +- src/utils/tools/toolDefinitions.ts | 76 +++---- 6 files changed, 216 insertions(+), 240 deletions(-) diff --git a/src/services/streamManager.test.ts b/src/services/streamManager.test.ts index 687db0affc..7d197e34e0 100644 --- a/src/services/streamManager.test.ts +++ b/src/services/streamManager.test.ts @@ -345,7 +345,7 @@ describe("StreamManager - Unavailable Tool Handling", () => { type: "tool-call", toolCallId: "test-call-1", toolName: "file_edit_replace_string", - input: { file_path: "/test", edits: [] }, + input: { file_path: "/test", old_string: "foo", new_string: "bar" }, }; // SDK emits tool-error when tool execution fails yield { diff --git a/src/services/tools/file_edit_replace_lines.ts b/src/services/tools/file_edit_replace_lines.ts index a44dc5d48b..714d7e5aa5 100644 --- a/src/services/tools/file_edit_replace_lines.ts +++ b/src/services/tools/file_edit_replace_lines.ts @@ -6,7 +6,7 @@ import { executeFileEditOperation } from "./file_edit_operation"; /** * File edit replace (lines) tool factory for AI assistant - * Applies line-range replacements sequentially with optional content validation. + * Applies a single line-range replacement with optional content validation. */ export const createFileEditReplaceLinesTool: ToolFactory = (config: ToolConfiguration) => { return tool({ @@ -19,59 +19,53 @@ export const createFileEditReplaceLinesTool: ToolFactory = (config: ToolConfigur config, filePath: args.file_path, operation: (originalContent) => { - let lines = originalContent.split("\n"); - let linesReplaced = 0; - let totalDelta = 0; + const startIndex = args.start_line - 1; + const endIndex = args.end_line - 1; - for (let i = 0; i < args.edits.length; i++) { - const edit = args.edits[i]; - const startIndex = edit.start_line - 1; - const endIndex = edit.end_line - 1; - - if (edit.start_line <= 0) { - return { - success: false, - error: `Edit ${i + 1}: start_line must be >= 1 (received ${edit.start_line}).`, - }; - } - - if (edit.end_line < edit.start_line) { - return { - success: false, - error: `Edit ${i + 1}: end_line must be >= start_line (received start ${edit.start_line}, end ${edit.end_line}).`, - }; - } + if (args.start_line <= 0) { + return { + success: false, + error: `start_line must be >= 1 (received ${args.start_line}).`, + }; + } - if (startIndex >= lines.length) { - return { - success: false, - error: `Edit ${i + 1}: start_line ${edit.start_line} exceeds current file length (${lines.length}).`, - }; - } + if (args.end_line < args.start_line) { + return { + success: false, + error: `end_line must be >= start_line (received start ${args.start_line}, end ${args.end_line}).`, + }; + } - const clampedEndIndex = Math.min(endIndex, lines.length - 1); - const currentRange = lines.slice(startIndex, clampedEndIndex + 1); + const lines = originalContent.split("\n"); - if (edit.expected_lines && !arraysEqual(currentRange, edit.expected_lines)) { - return { - success: false, - error: `Edit ${i + 1}: expected_lines validation failed. Current lines [${currentRange.join("\n")}] differ from expected [${edit.expected_lines.join("\n")}].`, - }; - } + if (startIndex >= lines.length) { + return { + success: false, + error: `start_line ${args.start_line} exceeds current file length (${lines.length}).`, + }; + } - const before = lines.slice(0, startIndex); - const after = lines.slice(clampedEndIndex + 1); - lines = [...before, ...edit.new_lines, ...after]; + const clampedEndIndex = Math.min(endIndex, lines.length - 1); + const currentRange = lines.slice(startIndex, clampedEndIndex + 1); - linesReplaced += currentRange.length; - totalDelta += edit.new_lines.length - currentRange.length; + if (args.expected_lines && !arraysEqual(currentRange, args.expected_lines)) { + return { + success: false, + error: `expected_lines validation failed. Current lines [${currentRange.join("\n")}] differ from expected [${args.expected_lines.join("\n")}].`, + }; } + const before = lines.slice(0, startIndex); + const after = lines.slice(clampedEndIndex + 1); + const updatedLines = [...before, ...args.new_lines, ...after]; + const linesReplaced = currentRange.length; + const totalDelta = args.new_lines.length - currentRange.length; + return { success: true, - newContent: lines.join("\n"), + newContent: updatedLines.join("\n"), metadata: { - edits_applied: args.edits.length, + edits_applied: 1, lines_replaced: linesReplaced, line_delta: totalDelta, }, diff --git a/src/services/tools/file_edit_replace_string.test.ts b/src/services/tools/file_edit_replace_string.test.ts index 25d8422b8d..018944b186 100644 --- a/src/services/tools/file_edit_replace_string.test.ts +++ b/src/services/tools/file_edit_replace_string.test.ts @@ -24,9 +24,9 @@ const readFile = async (filePath: string): Promise => { const executeReplace = async ( tool: ReturnType, filePath: string, - edits: FileEditReplaceStringToolArgs["edits"] + edit: Pick ): Promise => { - const args: FileEditReplaceStringToolArgs = { file_path: filePath, edits }; + const args: FileEditReplaceStringToolArgs = { file_path: filePath, ...edit }; return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceStringToolResult; }; @@ -49,9 +49,10 @@ describe("file_edit_replace_string tool", () => { await setupFile(testFilePath, "Hello world\nThis is a test\nGoodbye world"); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, [ - { old_string: "Hello world", new_string: "Hello universe" }, - ]); + const result = await executeReplace(tool, testFilePath, { + old_string: "Hello world", + new_string: "Hello universe", + }); expect(result.success).toBe(true); if (result.success) { @@ -65,38 +66,51 @@ describe("file_edit_replace_string tool", () => { await setupFile(testFilePath, "foo bar baz"); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, [ - { old_string: "foo", new_string: "FOO" }, - { old_string: "bar", new_string: "BAR" }, - { old_string: "baz", new_string: "BAZ" }, - ]); + const firstResult = await executeReplace(tool, testFilePath, { + old_string: "foo", + new_string: "FOO", + }); - expect(result.success).toBe(true); - if (result.success) { - expect(result.edits_applied).toBe(3); + expect(firstResult.success).toBe(true); + if (firstResult.success) { + expect(firstResult.edits_applied).toBe(1); + } + + const secondResult = await executeReplace(tool, testFilePath, { + old_string: "bar", + new_string: "BAR", + }); + + expect(secondResult.success).toBe(true); + if (secondResult.success) { + expect(secondResult.edits_applied).toBe(1); + } + + const thirdResult = await executeReplace(tool, testFilePath, { + old_string: "baz", + new_string: "BAZ", + }); + + expect(thirdResult.success).toBe(true); + if (thirdResult.success) { + expect(thirdResult.edits_applied).toBe(1); } expect(await readFile(testFilePath)).toBe("FOO BAR BAZ"); }); - it("should rollback if later edit fails (first edit breaks second edit search)", async () => { + it("should fail when old_string is missing", async () => { const initialContent = "foo bar baz"; await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [ - { old_string: "foo", new_string: "FOO" }, - { old_string: "foo", new_string: "qux" }, - ], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "missing", + new_string: "qux", + }); expect(result.success).toBe(false); if (!result.success) { - expect(result.error).toContain("Edit 2"); expect(result.error).toContain("old_string not found"); } @@ -109,12 +123,11 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "cat", new_string: "mouse", replace_count: -1 }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "cat", + new_string: "mouse", + replace_count: -1, + }); expect(result.success).toBe(true); if (result.success) { @@ -130,12 +143,10 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "cat", new_string: "mouse" }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "cat", + new_string: "mouse", + }); expect(result.success).toBe(true); if (result.success) { @@ -151,12 +162,10 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "nonexistent", new_string: "replacement" }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "nonexistent", + new_string: "replacement", + }); expect(result.success).toBe(false); if (!result.success) { @@ -172,12 +181,11 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "cat", new_string: "mouse", replace_count: 1 }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "cat", + new_string: "mouse", + replace_count: 1, + }); expect(result.success).toBe(false); if (!result.success) { @@ -194,12 +202,11 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "cat", new_string: "mouse", replace_count: 2 }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "cat", + new_string: "mouse", + replace_count: 2, + }); expect(result.success).toBe(true); if (result.success) { @@ -215,12 +222,11 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "cat", new_string: "mouse", replace_count: 5 }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "cat", + new_string: "mouse", + replace_count: 5, + }); expect(result.success).toBe(false); if (!result.success) { @@ -236,12 +242,10 @@ describe("file_edit_replace_string tool", () => { const nonExistentPath = path.join(testDir, "nonexistent.txt"); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: nonExistentPath, - edits: [{ old_string: "foo", new_string: "bar" }], - }; - - const result = await executeReplace(tool, nonExistentPath, args.edits); + const result = await executeReplace(tool, nonExistentPath, { + old_string: "foo", + new_string: "bar", + }); expect(result.success).toBe(false); if (!result.success) { @@ -254,12 +258,10 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "line2\nline3", new_string: "REPLACED" }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "line2\nline3", + new_string: "REPLACED", + }); expect(result.success).toBe(true); if (result.success) { @@ -275,12 +277,10 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "[DELETE_ME] ", new_string: "" }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "[DELETE_ME] ", + new_string: "", + }); expect(result.success).toBe(true); if (result.success) { @@ -296,19 +296,24 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [ - { old_string: "step1", new_string: "step2" }, - { old_string: "step2", new_string: "step3" }, - ], - }; + const firstResult = await executeReplace(tool, testFilePath, { + old_string: "step1", + new_string: "step2", + }); + + expect(firstResult.success).toBe(true); + if (firstResult.success) { + expect(firstResult.edits_applied).toBe(1); + } - const result = await executeReplace(tool, testFilePath, args.edits); + const secondResult = await executeReplace(tool, testFilePath, { + old_string: "step2", + new_string: "step3", + }); - expect(result.success).toBe(true); - if (result.success) { - expect(result.edits_applied).toBe(2); + expect(secondResult.success).toBe(true); + if (secondResult.success) { + expect(secondResult.edits_applied).toBe(1); } const updatedContent = await fs.readFile(testFilePath, "utf-8"); @@ -320,12 +325,10 @@ describe("file_edit_replace_string tool", () => { await fs.writeFile(testFilePath, initialContent); const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const args: FileEditReplaceStringToolArgs = { - file_path: testFilePath, - edits: [{ old_string: "line5", new_string: "LINE5_MODIFIED" }], - }; - - const result = await executeReplace(tool, testFilePath, args.edits); + const result = await executeReplace(tool, testFilePath, { + old_string: "line5", + new_string: "LINE5_MODIFIED", + }); expect(result.success).toBe(true); if (result.success) { diff --git a/src/services/tools/file_edit_replace_string.ts b/src/services/tools/file_edit_replace_string.ts index 2caedc5d55..c3015cca15 100644 --- a/src/services/tools/file_edit_replace_string.ts +++ b/src/services/tools/file_edit_replace_string.ts @@ -6,7 +6,7 @@ import { executeFileEditOperation } from "./file_edit_operation"; /** * File edit replace (string) tool factory for AI assistant - * Applies multiple text replacement edits sequentially against file content. + * Applies a single text replacement against file content. */ export const createFileEditReplaceStringTool: ToolFactory = (config: ToolConfiguration) => { return tool({ @@ -19,65 +19,62 @@ export const createFileEditReplaceStringTool: ToolFactory = (config: ToolConfigu config, filePath: args.file_path, operation: (originalContent) => { - let content = originalContent; - let editsApplied = 0; + const replaceCount = args.replace_count ?? 1; - for (let i = 0; i < args.edits.length; i++) { - const edit = args.edits[i]; - const replaceCount = edit.replace_count ?? 1; - - if (!content.includes(edit.old_string)) { - return { - success: false, - error: `Edit ${i + 1}: old_string not found in file. The text to replace must exist exactly as written in the file.`, - }; - } + if (!originalContent.includes(args.old_string)) { + return { + success: false, + error: "old_string not found in file. The text to replace must exist exactly as written in the file.", + }; + } - const parts = content.split(edit.old_string); - const occurrences = parts.length - 1; + const parts = originalContent.split(args.old_string); + const occurrences = parts.length - 1; - if (replaceCount === 1 && occurrences > 1) { - return { - success: false, - error: `Edit ${i + 1}: old_string appears ${occurrences} times in the file. Either expand the context to make it unique or set replace_count to ${occurrences} or -1.`, - }; - } + if (replaceCount === 1 && occurrences > 1) { + return { + success: false, + error: `old_string appears ${occurrences} times in the file. Either expand the context to make it unique or set replace_count to ${occurrences} or -1.`, + }; + } - if (replaceCount > occurrences && replaceCount !== -1) { - return { - success: false, - error: `Edit ${i + 1}: replace_count is ${replaceCount} but old_string only appears ${occurrences} time(s) in the file.`, - }; - } + if (replaceCount > occurrences && replaceCount !== -1) { + return { + success: false, + error: `replace_count is ${replaceCount} but old_string only appears ${occurrences} time(s) in the file.`, + }; + } - if (replaceCount === -1) { - content = parts.join(edit.new_string); - editsApplied += occurrences; - } else { - let replacedCount = 0; - let currentContent = content; + let newContent: string; + let editsApplied: number; - for (let j = 0; j < replaceCount; j++) { - const index = currentContent.indexOf(edit.old_string); - if (index === -1) { - break; - } + if (replaceCount === -1) { + newContent = parts.join(args.new_string); + editsApplied = occurrences; + } else { + let replacedCount = 0; + let currentContent = originalContent; - currentContent = - currentContent.substring(0, index) + - edit.new_string + - currentContent.substring(index + edit.old_string.length); - replacedCount++; + for (let i = 0; i < replaceCount; i++) { + const index = currentContent.indexOf(args.old_string); + if (index === -1) { + break; } - content = currentContent; - editsApplied += replacedCount; + currentContent = + currentContent.substring(0, index) + + args.new_string + + currentContent.substring(index + args.old_string.length); + replacedCount++; } + + newContent = currentContent; + editsApplied = replacedCount; } return { success: true, - newContent: content, + newContent, metadata: { edits_applied: editsApplied, }, diff --git a/src/types/tools.ts b/src/types/tools.ts index 0b12555e15..394b38269b 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -49,17 +49,13 @@ export type FileReadToolResult = }; // File Edit Replace (String) Tool Types -export interface FileEditReplaceStringEdit { +export interface FileEditReplaceStringToolArgs { + file_path: string; old_string: string; new_string: string; replace_count?: number; // Default: 1, -1 means replace all } -export interface FileEditReplaceStringToolArgs { - file_path: string; - edits: FileEditReplaceStringEdit[]; -} - export interface FileEditDiffSuccessBase { success: true; diff: string; @@ -77,18 +73,14 @@ export type FileEditReplaceStringToolResult = | FileEditErrorResult; // File Edit Replace (Lines) Tool Types -export interface FileEditReplaceLinesEdit { +export interface FileEditReplaceLinesToolArgs { + file_path: string; start_line: number; // 1-indexed inclusive start line end_line: number; // 1-indexed inclusive end line new_lines: string[]; // Replacement lines (empty array deletes the range) expected_lines?: string[]; // Optional safety check of current lines prior to replacement } -export interface FileEditReplaceLinesToolArgs { - file_path: string; - edits: FileEditReplaceLinesEdit[]; -} - export type FileEditReplaceLinesToolResult = | (FileEditDiffSuccessBase & { edits_applied: number; diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 2199a2bb1f..608cffc297 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -68,58 +68,48 @@ export const TOOL_DEFINITIONS = { }, file_edit_replace_string: { description: - "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.", + "Replace text in a file by providing the exact string to find and the replacement. The old_string must exist exactly as written in the file.", schema: z.object({ file_path: z.string().describe("The absolute path to the file to edit"), - edits: z - .array( - z.object({ - old_string: z - .string() - .describe( - "The exact text to replace (must be unique in file if replace_count is 1). Include enough context (indentation, surrounding lines) to make it unique." - ), - new_string: z.string().describe("The replacement text"), - replace_count: z - .number() - .int() - .optional() - .describe( - "Number of occurrences to replace (default: 1). Use -1 to replace all occurrences. If 1, old_string must be unique in the file." - ), - }) - ) - .min(1) - .describe("Array of edits to apply sequentially"), + old_string: z + .string() + .describe( + "The exact text to replace. Include enough context (indentation, surrounding lines) to make it unique." + ), + new_string: z.string().describe("The replacement text"), + replace_count: z + .number() + .int() + .optional() + .describe( + "Number of occurrences to replace (default: 1). Use -1 to replace all occurrences. If 1, old_string must be unique in the file." + ), }), }, file_edit_replace_lines: { description: - "Apply one or more edits to a file by replacing lines in a specific range. All edits are applied sequentially and line numbers refer to the file state after previous edits.", + "Replace a range of lines in a file. Line numbers refer to the state of the file before the edit.", schema: z.object({ file_path: z.string().describe("The absolute path to the file to edit"), - edits: z - .array( - z.object({ - 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 - .array(z.string()) - .describe("Replacement lines. Provide an empty array to delete the specified range."), - expected_lines: z - .array(z.string()) - .optional() - .describe( - "Optional safety check. When provided, the current lines in the specified range must match exactly." - ), - }) - ) + start_line: z + .number() + .int() + .min(1) + .describe("1-indexed start line (inclusive) to replace"), + end_line: z + .number() + .int() .min(1) - .describe("Array of line replacement edits to apply sequentially"), + .describe("1-indexed end line (inclusive) to replace"), + new_lines: z + .array(z.string()) + .describe("Replacement lines. Provide an empty array to delete the specified range."), + expected_lines: z + .array(z.string()) + .optional() + .describe( + "Optional safety check. When provided, the current lines in the specified range must match exactly." + ), }), }, file_edit_insert: { From 945cea1185b217128113aed20de852be05b1a005 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 12 Oct 2025 14:00:14 -0500 Subject: [PATCH 3/8] =?UTF-8?q?=F0=9F=A4=96=20Unify=20file=20edit=20tools?= =?UTF-8?q?=20into=20single=20file=5Fedit=5Freplace?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidates file_edit_replace_string and file_edit_replace_lines into a single file_edit_replace tool using discriminated unions. Changes: - New unified tool with mode: 'string' | 'lines' discriminant - Updated type system with FileEditReplaceToolArgs union type - Simplified frontend with single tool rendering path - Updated all tool definitions, registries, and utilities - Removed legacy tool implementations and tests Benefits: - Simpler API (1 tool instead of 2) - Easier to extend with new modes - Reduced code duplication (~327 line net reduction) All tests passing (TypeScript, unit tests, tool policy tests). _Generated with `cmux`_ --- src/components/Messages/ToolMessage.tsx | 42 +-- src/components/tools/FileEditToolCall.tsx | 20 +- src/services/streamManager.test.ts | 8 +- src/services/tools/file_edit_replace.test.ts | 106 ++++++ src/services/tools/file_edit_replace.ts | 182 +++++++++ src/services/tools/file_edit_replace_lines.ts | 82 ----- .../tools/file_edit_replace_string.test.ts | 348 ------------------ .../tools/file_edit_replace_string.ts | 86 ----- src/types/tools.ts | 61 ++- src/utils/main/tokenizer.ts | 3 +- src/utils/messages/toolOutputRedaction.ts | 46 +-- src/utils/tools/toolDefinitions.ts | 83 ++--- src/utils/tools/toolPolicy.test.ts | 54 ++- src/utils/tools/tools.ts | 6 +- 14 files changed, 400 insertions(+), 727 deletions(-) create mode 100644 src/services/tools/file_edit_replace.test.ts create mode 100644 src/services/tools/file_edit_replace.ts delete mode 100644 src/services/tools/file_edit_replace_lines.ts delete mode 100644 src/services/tools/file_edit_replace_string.test.ts delete mode 100644 src/services/tools/file_edit_replace_string.ts diff --git a/src/components/Messages/ToolMessage.tsx b/src/components/Messages/ToolMessage.tsx index 47a8bf5a8d..49fc6fd62e 100644 --- a/src/components/Messages/ToolMessage.tsx +++ b/src/components/Messages/ToolMessage.tsx @@ -13,10 +13,8 @@ import type { FileReadToolResult, FileEditInsertToolArgs, FileEditInsertToolResult, - FileEditReplaceLinesToolArgs, - FileEditReplaceLinesToolResult, - FileEditReplaceStringToolArgs, - FileEditReplaceStringToolResult, + FileEditReplaceToolArgs, + FileEditReplaceToolResult, ProposePlanToolArgs, ProposePlanToolResult, } from "@/types/tools"; @@ -39,20 +37,9 @@ function isFileReadTool(toolName: string, args: unknown): args is FileReadToolAr return TOOL_DEFINITIONS.file_read.schema.safeParse(args).success; } -function isFileEditReplaceStringTool( - toolName: string, - args: unknown -): args is FileEditReplaceStringToolArgs { - if (toolName !== "file_edit_replace_string") return false; - return TOOL_DEFINITIONS.file_edit_replace_string.schema.safeParse(args).success; -} - -function isFileEditReplaceLinesTool( - toolName: string, - args: unknown -): args is FileEditReplaceLinesToolArgs { - if (toolName !== "file_edit_replace_lines") return false; - return TOOL_DEFINITIONS.file_edit_replace_lines.schema.safeParse(args).success; +function isFileEditReplaceTool(toolName: string, args: unknown): args is FileEditReplaceToolArgs { + if (toolName !== "file_edit_replace") return false; + return TOOL_DEFINITIONS.file_edit_replace.schema.safeParse(args).success; } function isFileEditInsertTool(toolName: string, args: unknown): args is FileEditInsertToolArgs { @@ -92,26 +79,13 @@ export const ToolMessage: React.FC = ({ message, className, wo ); } - if (isFileEditReplaceStringTool(message.toolName, message.args)) { - return ( -
- -
- ); - } - - if (isFileEditReplaceLinesTool(message.toolName, message.args)) { + if (isFileEditReplaceTool(message.toolName, message.args)) { return (
diff --git a/src/components/tools/FileEditToolCall.tsx b/src/components/tools/FileEditToolCall.tsx index a1beb2c31a..67b43114c9 100644 --- a/src/components/tools/FileEditToolCall.tsx +++ b/src/components/tools/FileEditToolCall.tsx @@ -4,10 +4,8 @@ import { parsePatch } from "diff"; import type { FileEditInsertToolArgs, FileEditInsertToolResult, - FileEditReplaceLinesToolArgs, - FileEditReplaceLinesToolResult, - FileEditReplaceStringToolArgs, - FileEditReplaceStringToolResult, + FileEditReplaceToolArgs, + FileEditReplaceToolResult, } from "@/types/tools"; import { ToolContainer, @@ -179,19 +177,13 @@ const ButtonGroup = styled.div` margin-right: 8px; `; -type FileEditToolArgs = - | FileEditReplaceStringToolArgs - | FileEditReplaceLinesToolArgs - | FileEditInsertToolArgs; +type FileEditOperationArgs = FileEditReplaceToolArgs | FileEditInsertToolArgs; -type FileEditToolResult = - | FileEditReplaceStringToolResult - | FileEditReplaceLinesToolResult - | FileEditInsertToolResult; +type FileEditToolResult = FileEditReplaceToolResult | FileEditInsertToolResult; interface FileEditToolCallProps { - toolName: "file_edit_replace_string" | "file_edit_replace_lines" | "file_edit_insert"; - args: FileEditToolArgs; + toolName: "file_edit_replace" | "file_edit_insert"; + args: FileEditOperationArgs; result?: FileEditToolResult; status?: ToolStatus; } diff --git a/src/services/streamManager.test.ts b/src/services/streamManager.test.ts index 7d197e34e0..20c8dd186e 100644 --- a/src/services/streamManager.test.ts +++ b/src/services/streamManager.test.ts @@ -344,14 +344,14 @@ describe("StreamManager - Unavailable Tool Handling", () => { yield { type: "tool-call", toolCallId: "test-call-1", - toolName: "file_edit_replace_string", + toolName: "file_edit_replace", input: { file_path: "/test", old_string: "foo", new_string: "bar" }, }; // SDK emits tool-error when tool execution fails yield { type: "tool-error", toolCallId: "test-call-1", - toolName: "file_edit_replace_string", + toolName: "file_edit_replace", error: "Tool not found", }; })(), @@ -382,11 +382,11 @@ describe("StreamManager - Unavailable Tool Handling", () => { expect(events.length).toBeGreaterThanOrEqual(2); expect(events[0]).toMatchObject({ type: "tool-call-start", - toolName: "file_edit_replace_string", + toolName: "file_edit_replace", }); expect(events[1]).toMatchObject({ type: "tool-call-end", - toolName: "file_edit_replace_string", + toolName: "file_edit_replace", }); // Verify error result diff --git a/src/services/tools/file_edit_replace.test.ts b/src/services/tools/file_edit_replace.test.ts new file mode 100644 index 0000000000..a7a02c2369 --- /dev/null +++ b/src/services/tools/file_edit_replace.test.ts @@ -0,0 +1,106 @@ +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 { createFileEditReplaceTool } from "./file_edit_replace"; +import type { + FileEditReplaceLinesPayload, + FileEditReplaceStringPayload, + FileEditReplaceToolArgs, + FileEditReplaceToolResult, +} from "@/types/tools"; +import type { ToolCallOptions } from "ai"; + +// Mock ToolCallOptions for testing +const mockToolCallOptions: ToolCallOptions = { + toolCallId: "test-call-id", + messages: [], +}; + +// Test helpers +const setupFile = async (filePath: string, content: string): Promise => { + await fs.writeFile(filePath, content); +}; + +const readFile = async (filePath: string): Promise => { + return await fs.readFile(filePath, "utf-8"); +}; + +const executeReplace = async ( + tool: ReturnType, + args: FileEditReplaceToolArgs +): Promise => { + return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; +}; + +describe("file_edit_replace tool (string mode)", () => { + let testDir: string; + let testFilePath: string; + + beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), "fileEditReplace-test-")); + testFilePath = path.join(testDir, "test.txt"); + }); + + afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it("should apply a single edit successfully", async () => { + await setupFile(testFilePath, "Hello world\nThis is a test\nGoodbye world"); + const tool = createFileEditReplaceTool({ cwd: testDir }); + + const payload: FileEditReplaceStringPayload = { + mode: "string", + file_path: testFilePath, + old_string: "Hello world", + new_string: "Hello universe", + }; + + const result = await executeReplace(tool, payload); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.edits_applied).toBe(1); + } + + expect(await readFile(testFilePath)).toBe("Hello universe\nThis is a test\nGoodbye world"); + }); +}); + +describe("file_edit_replace tool (lines mode)", () => { + let testDir: string; + let testFilePath: string; + + beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), "fileEditReplace-test-")); + testFilePath = path.join(testDir, "test.txt"); + }); + + afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it("should replace a line range successfully", async () => { + await setupFile(testFilePath, "line1\nline2\nline3\nline4"); + const tool = createFileEditReplaceTool({ cwd: testDir }); + + const payload: FileEditReplaceLinesPayload = { + mode: "lines", + file_path: testFilePath, + start_line: 2, + end_line: 3, + new_lines: ["LINE2", "LINE3"], + }; + + const result = await executeReplace(tool, payload); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.lines_replaced).toBe(2); + expect(result.line_delta).toBe(0); + } + + expect(await readFile(testFilePath)).toBe("line1\nLINE2\nLINE3\nline4"); + }); +}); diff --git a/src/services/tools/file_edit_replace.ts b/src/services/tools/file_edit_replace.ts new file mode 100644 index 0000000000..0030b9c724 --- /dev/null +++ b/src/services/tools/file_edit_replace.ts @@ -0,0 +1,182 @@ +import { tool } from "ai"; +import type { + FileEditReplaceToolArgs, + FileEditReplaceToolResult, + FileEditReplaceLinesPayload, + FileEditReplaceStringPayload, +} from "@/types/tools"; +import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; +import { executeFileEditOperation } from "./file_edit_operation"; + +interface OperationMetadata { + edits_applied: number; + lines_replaced?: number; + line_delta?: number; +} + +interface OperationResult { + success: true; + newContent: string; + metadata: OperationMetadata; +} + +/** + * File edit replace tool factory for AI assistant. + * Supports string replacements and line range replacements using a discriminated union payload. + */ +export const createFileEditReplaceTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.file_edit_replace.description, + inputSchema: TOOL_DEFINITIONS.file_edit_replace.schema, + execute: async (args: FileEditReplaceToolArgs): Promise => { + return executeFileEditOperation({ + config, + filePath: args.file_path, + operation: (originalContent) => { + if (args.mode === "string") { + return handleStringReplace(args, originalContent); + } + + if (args.mode === "lines") { + return handleLineReplace(args, originalContent); + } + + return { + success: false, + error: `Unsupported mode: ${(args as { mode?: string }).mode ?? ""}`, + }; + }, + }); + }, + }); +}; + +function handleStringReplace( + args: FileEditReplaceStringPayload, + originalContent: string +): OperationResult | { success: false; error: string } { + const replaceCount = args.replace_count ?? 1; + + if (!originalContent.includes(args.old_string)) { + return { + success: false, + error: + "old_string not found in file. The text to replace must exist exactly as written in the file.", + }; + } + + const parts = originalContent.split(args.old_string); + const occurrences = parts.length - 1; + + if (replaceCount === 1 && occurrences > 1) { + return { + success: false, + error: `old_string appears ${occurrences} times in the file. Either expand the context to make it unique or set replace_count to ${occurrences} or -1.`, + }; + } + + if (replaceCount > occurrences && replaceCount !== -1) { + return { + success: false, + error: `replace_count is ${replaceCount} but old_string only appears ${occurrences} time(s) in the file.`, + }; + } + + let newContent: string; + let editsApplied: number; + + if (replaceCount === -1) { + newContent = parts.join(args.new_string); + editsApplied = occurrences; + } else { + let replacedCount = 0; + let currentContent = originalContent; + + for (let i = 0; i < replaceCount; i++) { + const index = currentContent.indexOf(args.old_string); + if (index === -1) { + break; + } + + currentContent = + currentContent.substring(0, index) + + args.new_string + + currentContent.substring(index + args.old_string.length); + replacedCount++; + } + + newContent = currentContent; + editsApplied = replacedCount; + } + + return { + success: true, + newContent, + metadata: { + edits_applied: editsApplied, + }, + }; +} + +function handleLineReplace( + args: FileEditReplaceLinesPayload, + originalContent: string +): OperationResult | { success: false; error: string } { + const startIndex = args.start_line - 1; + const endIndex = args.end_line - 1; + + if (args.start_line <= 0) { + return { + success: false, + error: `start_line must be >= 1 (received ${args.start_line}).`, + }; + } + + if (args.end_line < args.start_line) { + return { + success: false, + error: `end_line must be >= start_line (received start ${args.start_line}, end ${args.end_line}).`, + }; + } + + const lines = originalContent.split("\n"); + + if (startIndex >= lines.length) { + return { + success: false, + error: `start_line ${args.start_line} exceeds current file length (${lines.length}).`, + }; + } + + const clampedEndIndex = Math.min(endIndex, lines.length - 1); + const currentRange = lines.slice(startIndex, clampedEndIndex + 1); + + if (args.expected_lines && !arraysEqual(currentRange, args.expected_lines)) { + return { + success: false, + error: `expected_lines validation failed. Current lines [${currentRange.join("\n")}] differ from expected [${args.expected_lines.join("\n")}].`, + }; + } + + const before = lines.slice(0, startIndex); + const after = lines.slice(clampedEndIndex + 1); + const updatedLines = [...before, ...args.new_lines, ...after]; + const linesReplaced = currentRange.length; + const totalDelta = args.new_lines.length - currentRange.length; + + return { + success: true, + newContent: updatedLines.join("\n"), + metadata: { + edits_applied: 1, + lines_replaced: linesReplaced, + line_delta: totalDelta, + }, + }; +} + +function arraysEqual(a: string[], b: string[]): boolean { + if (a.length !== b.length) return false; + return a.every((value, index) => value === b[index]); +} diff --git a/src/services/tools/file_edit_replace_lines.ts b/src/services/tools/file_edit_replace_lines.ts deleted file mode 100644 index 714d7e5aa5..0000000000 --- a/src/services/tools/file_edit_replace_lines.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { tool } from "ai"; -import type { FileEditReplaceLinesToolArgs, FileEditReplaceLinesToolResult } from "@/types/tools"; -import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; -import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { executeFileEditOperation } from "./file_edit_operation"; - -/** - * File edit replace (lines) tool factory for AI assistant - * Applies a single line-range replacement with optional content validation. - */ -export const createFileEditReplaceLinesTool: ToolFactory = (config: ToolConfiguration) => { - return tool({ - description: TOOL_DEFINITIONS.file_edit_replace_lines.description, - inputSchema: TOOL_DEFINITIONS.file_edit_replace_lines.schema, - execute: async ( - args: FileEditReplaceLinesToolArgs - ): Promise => { - return executeFileEditOperation({ - config, - filePath: args.file_path, - operation: (originalContent) => { - const startIndex = args.start_line - 1; - const endIndex = args.end_line - 1; - - if (args.start_line <= 0) { - return { - success: false, - error: `start_line must be >= 1 (received ${args.start_line}).`, - }; - } - - if (args.end_line < args.start_line) { - return { - success: false, - error: `end_line must be >= start_line (received start ${args.start_line}, end ${args.end_line}).`, - }; - } - - const lines = originalContent.split("\n"); - - if (startIndex >= lines.length) { - return { - success: false, - error: `start_line ${args.start_line} exceeds current file length (${lines.length}).`, - }; - } - - const clampedEndIndex = Math.min(endIndex, lines.length - 1); - const currentRange = lines.slice(startIndex, clampedEndIndex + 1); - - if (args.expected_lines && !arraysEqual(currentRange, args.expected_lines)) { - return { - success: false, - error: `expected_lines validation failed. Current lines [${currentRange.join("\n")}] differ from expected [${args.expected_lines.join("\n")}].`, - }; - } - - const before = lines.slice(0, startIndex); - const after = lines.slice(clampedEndIndex + 1); - const updatedLines = [...before, ...args.new_lines, ...after]; - const linesReplaced = currentRange.length; - const totalDelta = args.new_lines.length - currentRange.length; - - return { - success: true, - newContent: updatedLines.join("\n"), - metadata: { - edits_applied: 1, - lines_replaced: linesReplaced, - line_delta: totalDelta, - }, - }; - }, - }); - }, - }); -}; - -function arraysEqual(a: string[], b: string[]): boolean { - if (a.length !== b.length) return false; - return a.every((value, index) => value === b[index]); -} diff --git a/src/services/tools/file_edit_replace_string.test.ts b/src/services/tools/file_edit_replace_string.test.ts deleted file mode 100644 index 018944b186..0000000000 --- a/src/services/tools/file_edit_replace_string.test.ts +++ /dev/null @@ -1,348 +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 { createFileEditReplaceStringTool } from "./file_edit_replace_string"; -import type { FileEditReplaceStringToolArgs, FileEditReplaceStringToolResult } from "@/types/tools"; -import type { ToolCallOptions } from "ai"; - -// Mock ToolCallOptions for testing -const mockToolCallOptions: ToolCallOptions = { - toolCallId: "test-call-id", - messages: [], -}; - -// Test helpers -const setupFile = async (filePath: string, content: string): Promise => { - await fs.writeFile(filePath, content); -}; - -const readFile = async (filePath: string): Promise => { - return await fs.readFile(filePath, "utf-8"); -}; - -const executeReplace = async ( - tool: ReturnType, - filePath: string, - edit: Pick -): Promise => { - const args: FileEditReplaceStringToolArgs = { file_path: filePath, ...edit }; - return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceStringToolResult; -}; - -describe("file_edit_replace_string tool", () => { - let testDir: string; - let testFilePath: string; - - beforeEach(async () => { - // Create a temporary directory for test files - testDir = await fs.mkdtemp(path.join(os.tmpdir(), "fileEditReplace-test-")); - testFilePath = path.join(testDir, "test.txt"); - }); - - afterEach(async () => { - // Clean up test directory - await fs.rm(testDir, { recursive: true, force: true }); - }); - - it("should apply a single edit successfully", async () => { - await setupFile(testFilePath, "Hello world\nThis is a test\nGoodbye world"); - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - - const result = await executeReplace(tool, testFilePath, { - old_string: "Hello world", - new_string: "Hello universe", - }); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.edits_applied).toBe(1); - } - - expect(await readFile(testFilePath)).toBe("Hello universe\nThis is a test\nGoodbye world"); - }); - - it("should apply multiple edits sequentially", async () => { - await setupFile(testFilePath, "foo bar baz"); - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - - const firstResult = await executeReplace(tool, testFilePath, { - old_string: "foo", - new_string: "FOO", - }); - - expect(firstResult.success).toBe(true); - if (firstResult.success) { - expect(firstResult.edits_applied).toBe(1); - } - - const secondResult = await executeReplace(tool, testFilePath, { - old_string: "bar", - new_string: "BAR", - }); - - expect(secondResult.success).toBe(true); - if (secondResult.success) { - expect(secondResult.edits_applied).toBe(1); - } - - const thirdResult = await executeReplace(tool, testFilePath, { - old_string: "baz", - new_string: "BAZ", - }); - - expect(thirdResult.success).toBe(true); - if (thirdResult.success) { - expect(thirdResult.edits_applied).toBe(1); - } - - expect(await readFile(testFilePath)).toBe("FOO BAR BAZ"); - }); - - it("should fail when old_string is missing", async () => { - const initialContent = "foo bar baz"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "missing", - new_string: "qux", - }); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("old_string not found"); - } - - const finalContent = await fs.readFile(testFilePath, "utf-8"); - expect(finalContent).toBe(initialContent); - }); - - it("should replace all occurrences when replace_count is -1", async () => { - const initialContent = "cat dog cat bird cat"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "cat", - new_string: "mouse", - replace_count: -1, - }); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.edits_applied).toBe(3); - } - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("mouse dog mouse bird mouse"); - }); - - it("should replace unique occurrence when replace_count defaults to 1", async () => { - const initialContent = "cat dog bird"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "cat", - new_string: "mouse", - }); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.edits_applied).toBe(1); - } - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("mouse dog bird"); - }); - - it("should fail when old_string is not found", async () => { - const initialContent = "Hello world"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "nonexistent", - new_string: "replacement", - }); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("old_string not found"); - } - - const unchangedContent = await fs.readFile(testFilePath, "utf-8"); - expect(unchangedContent).toBe(initialContent); - }); - - it("should fail when old_string appears multiple times with replace_count of 1", async () => { - const initialContent = "cat dog cat bird cat"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "cat", - new_string: "mouse", - replace_count: 1, - }); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("appears 3 times"); - expect(result.error).toContain("replace_count to 3 or -1"); - } - - const unchangedContent = await fs.readFile(testFilePath, "utf-8"); - expect(unchangedContent).toBe(initialContent); - }); - - it("should replace exactly N occurrences when replace_count is N", async () => { - const initialContent = "cat dog cat bird cat"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "cat", - new_string: "mouse", - replace_count: 2, - }); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.edits_applied).toBe(2); - } - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("mouse dog mouse bird cat"); - }); - - it("should fail when replace_count exceeds actual occurrences", async () => { - const initialContent = "cat dog bird"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "cat", - new_string: "mouse", - replace_count: 5, - }); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("replace_count is 5"); - expect(result.error).toContain("only appears 1 time(s)"); - } - - const unchangedContent = await fs.readFile(testFilePath, "utf-8"); - expect(unchangedContent).toBe(initialContent); - }); - - it("should fail when file does not exist", async () => { - const nonExistentPath = path.join(testDir, "nonexistent.txt"); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, nonExistentPath, { - old_string: "foo", - new_string: "bar", - }); - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("File not found"); - } - }); - - it("should handle multiline edits", async () => { - const initialContent = "line1\nline2\nline3\nline4"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "line2\nline3", - new_string: "REPLACED", - }); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.edits_applied).toBe(1); - } - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("line1\nREPLACED\nline4"); - }); - - it("should handle empty string replacement", async () => { - const initialContent = "Hello [DELETE_ME] world"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "[DELETE_ME] ", - new_string: "", - }); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.edits_applied).toBe(1); - } - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("Hello world"); - }); - - it("should handle edits that depend on previous edits", async () => { - const initialContent = "step1"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const firstResult = await executeReplace(tool, testFilePath, { - old_string: "step1", - new_string: "step2", - }); - - expect(firstResult.success).toBe(true); - if (firstResult.success) { - expect(firstResult.edits_applied).toBe(1); - } - - const secondResult = await executeReplace(tool, testFilePath, { - old_string: "step2", - new_string: "step3", - }); - - expect(secondResult.success).toBe(true); - if (secondResult.success) { - expect(secondResult.edits_applied).toBe(1); - } - - const updatedContent = await fs.readFile(testFilePath, "utf-8"); - expect(updatedContent).toBe("step3"); - }); - - it("should return unified diff with context of 3", async () => { - const initialContent = "line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9"; - await fs.writeFile(testFilePath, initialContent); - - const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const result = await executeReplace(tool, testFilePath, { - old_string: "line5", - new_string: "LINE5_MODIFIED", - }); - - expect(result.success).toBe(true); - if (result.success) { - expect(result.diff).toBeDefined(); - expect(result.diff).toContain("--- " + testFilePath); - expect(result.diff).toContain("+++ " + testFilePath); - expect(result.diff).toContain("-line5"); - expect(result.diff).toContain("+LINE5_MODIFIED"); - expect(result.diff).toContain("line2"); - expect(result.diff).toContain("line3"); - expect(result.diff).toContain("line4"); - expect(result.diff).toContain("line6"); - expect(result.diff).toContain("line7"); - expect(result.diff).toContain("line8"); - } - }); -}); diff --git a/src/services/tools/file_edit_replace_string.ts b/src/services/tools/file_edit_replace_string.ts deleted file mode 100644 index c3015cca15..0000000000 --- a/src/services/tools/file_edit_replace_string.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { tool } from "ai"; -import type { FileEditReplaceStringToolArgs, FileEditReplaceStringToolResult } from "@/types/tools"; -import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; -import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { executeFileEditOperation } from "./file_edit_operation"; - -/** - * File edit replace (string) tool factory for AI assistant - * Applies a single text replacement against file content. - */ -export const createFileEditReplaceStringTool: ToolFactory = (config: ToolConfiguration) => { - return tool({ - description: TOOL_DEFINITIONS.file_edit_replace_string.description, - inputSchema: TOOL_DEFINITIONS.file_edit_replace_string.schema, - execute: async ( - args: FileEditReplaceStringToolArgs - ): Promise => { - return executeFileEditOperation({ - config, - filePath: args.file_path, - operation: (originalContent) => { - const replaceCount = args.replace_count ?? 1; - - if (!originalContent.includes(args.old_string)) { - return { - success: false, - error: "old_string not found in file. The text to replace must exist exactly as written in the file.", - }; - } - - const parts = originalContent.split(args.old_string); - const occurrences = parts.length - 1; - - if (replaceCount === 1 && occurrences > 1) { - return { - success: false, - error: `old_string appears ${occurrences} times in the file. Either expand the context to make it unique or set replace_count to ${occurrences} or -1.`, - }; - } - - if (replaceCount > occurrences && replaceCount !== -1) { - return { - success: false, - error: `replace_count is ${replaceCount} but old_string only appears ${occurrences} time(s) in the file.`, - }; - } - - let newContent: string; - let editsApplied: number; - - if (replaceCount === -1) { - newContent = parts.join(args.new_string); - editsApplied = occurrences; - } else { - let replacedCount = 0; - let currentContent = originalContent; - - for (let i = 0; i < replaceCount; i++) { - const index = currentContent.indexOf(args.old_string); - if (index === -1) { - break; - } - - currentContent = - currentContent.substring(0, index) + - args.new_string + - currentContent.substring(index + args.old_string.length); - replacedCount++; - } - - newContent = currentContent; - editsApplied = replacedCount; - } - - return { - success: true, - newContent, - metadata: { - edits_applied: editsApplied, - }, - }; - }, - }); - }, - }); -}; diff --git a/src/types/tools.ts b/src/types/tools.ts index 394b38269b..da260386ba 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -48,14 +48,6 @@ export type FileReadToolResult = error: string; }; -// File Edit Replace (String) Tool Types -export interface FileEditReplaceStringToolArgs { - file_path: string; - old_string: string; - new_string: string; - replace_count?: number; // Default: 1, -1 means replace all -} - export interface FileEditDiffSuccessBase { success: true; diff: string; @@ -66,56 +58,49 @@ export interface FileEditErrorResult { error: string; } -export type FileEditReplaceStringToolResult = - | (FileEditDiffSuccessBase & { - edits_applied: number; - }) - | FileEditErrorResult; +export interface FileEditReplaceStringPayload { + mode: "string"; + file_path: string; + old_string: string; + new_string: string; + replace_count?: number; +} -// File Edit Replace (Lines) Tool Types -export interface FileEditReplaceLinesToolArgs { +export interface FileEditReplaceLinesPayload { + mode: "lines"; file_path: string; - start_line: number; // 1-indexed inclusive start line - end_line: number; // 1-indexed inclusive end line - new_lines: string[]; // Replacement lines (empty array deletes the range) - expected_lines?: string[]; // Optional safety check of current lines prior to replacement + start_line: number; + end_line: number; + new_lines: string[]; + expected_lines?: string[]; } -export type FileEditReplaceLinesToolResult = +export type FileEditReplaceToolArgs = FileEditReplaceStringPayload | FileEditReplaceLinesPayload; + +export type FileEditReplaceToolResult = | (FileEditDiffSuccessBase & { edits_applied: number; - lines_replaced: number; - line_delta: number; + lines_replaced?: number; + line_delta?: number; }) | FileEditErrorResult; -export type FileEditSharedToolResult = - | FileEditReplaceStringToolResult - | FileEditReplaceLinesToolResult - | FileEditInsertToolResult; +export type FileEditSharedToolResult = FileEditReplaceToolResult | FileEditInsertToolResult; -export const FILE_EDIT_TOOL_NAMES = [ - "file_edit_replace_string", - "file_edit_replace_lines", - "file_edit_insert", -] as const; +export const FILE_EDIT_TOOL_NAMES = ["file_edit_replace", "file_edit_insert"] as const; export type FileEditToolName = (typeof FILE_EDIT_TOOL_NAMES)[number]; -// File Edit Insert Tool Types export interface FileEditInsertToolArgs { file_path: string; - line_offset: number; // 1-indexed line position (0 = insert at top, N = insert after line N) + line_offset: number; content: string; - create?: boolean; // If true, create the file if it doesn't exist (default: false) + create?: boolean; } export type FileEditInsertToolResult = FileEditDiffSuccessBase | FileEditErrorResult; -export type FileEditToolArgs = - | FileEditReplaceStringToolArgs - | FileEditReplaceLinesToolArgs - | FileEditInsertToolArgs; +export type FileEditToolArgs = FileEditReplaceToolArgs | FileEditInsertToolArgs; export interface FileEditToolMessage { toolName: FileEditToolName; diff --git a/src/utils/main/tokenizer.ts b/src/utils/main/tokenizer.ts index 5422eb3476..180c28dbdb 100644 --- a/src/utils/main/tokenizer.ts +++ b/src/utils/main/tokenizer.ts @@ -155,8 +155,7 @@ export function getToolDefinitionTokens(toolName: string, modelString: string): const fallbackSizes: Record = { bash: 65, file_read: 45, - file_edit_replace_string: 70, - file_edit_replace_lines: 65, + file_edit_replace: 70, 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 f2dbaa3a45..709f22b487 100644 --- a/src/utils/messages/toolOutputRedaction.ts +++ b/src/utils/messages/toolOutputRedaction.ts @@ -12,8 +12,7 @@ import type { FileEditInsertToolResult, - FileEditReplaceLinesToolResult, - FileEditReplaceStringToolResult, + FileEditReplaceToolResult, } from "@/types/tools"; // Tool-output from AI SDK is often wrapped like: { type: 'json', value: } @@ -37,16 +36,7 @@ function rewrapJsonContainer(wrapped: boolean, value: unknown): unknown { } // Narrowing helpers for our tool result types -function isFileEditReplaceStringResult(v: unknown): v is FileEditReplaceStringToolResult { - return ( - typeof v === "object" && - v !== null && - "success" in v && - typeof (v as { success: unknown }).success === "boolean" - ); -} - -function isFileEditReplaceLinesResult(v: unknown): v is FileEditReplaceLinesToolResult { +function isFileEditReplaceResult(v: unknown): v is FileEditReplaceToolResult { return ( typeof v === "object" && v !== null && @@ -65,14 +55,14 @@ function isFileEditInsertResult(v: unknown): v is FileEditInsertToolResult { } // Redactors per tool -function redactFileEditReplaceString(output: unknown): unknown { +function redactFileEditReplace(output: unknown): unknown { const unwrapped = unwrapJsonContainer(output); const val = unwrapped.value; - if (!isFileEditReplaceStringResult(val)) return output; // unknown structure, leave as-is + if (!isFileEditReplaceResult(val)) return output; // unknown structure, leave as-is if (val.success) { - const compact: FileEditReplaceStringToolResult = { + const compact: FileEditReplaceToolResult = { success: true, edits_applied: val.edits_applied, diff: "[diff omitted in context - call file_read on the target file if needed]", @@ -84,26 +74,6 @@ function redactFileEditReplaceString(output: unknown): unknown { return output; } -function redactFileEditReplaceLines(output: unknown): unknown { - const unwrapped = unwrapJsonContainer(output); - const val = unwrapped.value; - - if (!isFileEditReplaceLinesResult(val)) return output; - - if (val.success) { - const compact: FileEditReplaceLinesToolResult = { - success: true, - edits_applied: val.edits_applied, - lines_replaced: val.lines_replaced, - line_delta: val.line_delta, - diff: "[diff omitted in context - call file_read on the target file if needed]", - }; - return rewrapJsonContainer(unwrapped.wrapped, compact); - } - - return output; -} - function redactFileEditInsert(output: unknown): unknown { const unwrapped = unwrapJsonContainer(output); const val = unwrapped.value; @@ -124,10 +94,8 @@ function redactFileEditInsert(output: unknown): unknown { // 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": - return redactFileEditReplaceString(output); - case "file_edit_replace_lines": - return redactFileEditReplaceLines(output); + case "file_edit_replace": + return redactFileEditReplace(output); case "file_edit_insert": return redactFileEditInsert(output); default: diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 608cffc297..47f386f300 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -66,51 +66,43 @@ export const TOOL_DEFINITIONS = { .describe("Number of lines to return from offset (optional, returns all if not specified)"), }), }, - file_edit_replace_string: { + file_edit_replace: { description: - "Replace text in a file by providing the exact string to find and the replacement. The old_string must exist exactly as written in the file.", - schema: z.object({ - file_path: z.string().describe("The absolute path to the file to edit"), - old_string: z - .string() - .describe( - "The exact text to replace. Include enough context (indentation, surrounding lines) to make it unique." - ), - new_string: z.string().describe("The replacement text"), - replace_count: z - .number() - .int() - .optional() - .describe( - "Number of occurrences to replace (default: 1). Use -1 to replace all occurrences. If 1, old_string must be unique in the file." - ), - }), - }, - file_edit_replace_lines: { - description: - "Replace a range of lines in a file. Line numbers refer to the state of the file before the edit.", - schema: z.object({ - file_path: z.string().describe("The absolute path to the file to edit"), - 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 - .array(z.string()) - .describe("Replacement lines. Provide an empty array to delete the specified range."), - expected_lines: z - .array(z.string()) - .optional() - .describe( - "Optional safety check. When provided, the current lines in the specified range must match exactly." - ), - }), + "Replace content in a file using either exact string matching or line ranges. Choose mode='string' for text replacements or mode='lines' for line-range updates.", + schema: z.discriminatedUnion("mode", [ + z.object({ + mode: z.literal("string"), + file_path: z.string().describe("The absolute path to the file to edit"), + old_string: z + .string() + .describe( + "The exact text to replace. Include enough context (indentation, surrounding lines) to make it unique." + ), + new_string: z.string().describe("The replacement text"), + replace_count: z + .number() + .int() + .optional() + .describe( + "Number of occurrences to replace (default: 1). Use -1 to replace all occurrences. If 1, old_string must be unique in the file." + ), + }), + z.object({ + mode: z.literal("lines"), + file_path: z.string().describe("The absolute path to the file to edit"), + 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 + .array(z.string()) + .describe("Replacement lines. Provide an empty array to delete the specified range."), + expected_lines: z + .array(z.string()) + .optional() + .describe( + "Optional safety check. When provided, the current lines in the specified range must match exactly." + ), + }), + ]), }, file_edit_insert: { description: @@ -193,8 +185,7 @@ export function getAvailableTools(modelString: string): string[] { const baseTools = [ "bash", "file_read", - "file_edit_replace_string", - "file_edit_replace_lines", + "file_edit_replace", "file_edit_insert", "propose_plan", "compact_summary", diff --git a/src/utils/tools/toolPolicy.test.ts b/src/utils/tools/toolPolicy.test.ts index f7ab014832..2979482794 100644 --- a/src/utils/tools/toolPolicy.test.ts +++ b/src/utils/tools/toolPolicy.test.ts @@ -14,14 +14,9 @@ const mockTools = { inputSchema: z.object({ path: z.string() }), execute: () => Promise.resolve({ content: "test" }), }), - file_edit_replace_string: tool({ - description: "Replace content in files (string)", - inputSchema: z.object({ path: z.string() }), - execute: () => Promise.resolve({ success: true }), - }), - file_edit_replace_lines: tool({ - description: "Replace content in files (lines)", - inputSchema: z.object({ path: z.string() }), + file_edit_replace: tool({ + description: "Replace content in files", + inputSchema: z.object({ path: z.string(), mode: z.string() }), execute: () => Promise.resolve({ success: true }), }), file_edit_insert: tool({ @@ -56,8 +51,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeUndefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace_string).toBeDefined(); - expect(result.file_edit_replace_lines).toBeDefined(); + expect(result.file_edit_replace).toBeDefined(); + expect(result.file_edit_replace).toBeDefined(); expect(result.file_edit_insert).toBeDefined(); expect(result.web_search).toBeDefined(); }); @@ -72,8 +67,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeUndefined(); expect(result.web_search).toBeUndefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace_string).toBeDefined(); - expect(result.file_edit_replace_lines).toBeDefined(); + expect(result.file_edit_replace).toBeDefined(); + expect(result.file_edit_replace).toBeDefined(); expect(result.file_edit_insert).toBeDefined(); }); }); @@ -83,8 +78,8 @@ describe("applyToolPolicy", () => { const policy: ToolPolicy = [{ regex_match: "file_edit_.*", action: "disable" }]; const result = applyToolPolicy(mockTools, policy); - expect(result.file_edit_replace_string).toBeUndefined(); - expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_replace).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); @@ -103,8 +98,8 @@ describe("applyToolPolicy", () => { const result = applyToolPolicy(mockTools, policy); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace_string).toBeUndefined(); - expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_replace).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.web_search).toBeDefined(); @@ -121,21 +116,20 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace_string).toBeUndefined(); - expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_replace).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.file_edit_insert).toBeUndefined(); expect(result.web_search).toBeUndefined(); }); - test("disables file_edit_.* then enables file_edit_replace_string", () => { + test("disables file_edit_.* then enables file_edit_replace", () => { const policy: ToolPolicy = [ { regex_match: "file_edit_.*", action: "disable" }, - { regex_match: "file_edit_replace_string", action: "enable" }, + { regex_match: "file_edit_replace", action: "enable" }, ]; const result = applyToolPolicy(mockTools, policy); - expect(result.file_edit_replace_string).toBeDefined(); - expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_replace).toBeDefined(); expect(result.file_edit_insert).toBeUndefined(); expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); @@ -160,8 +154,8 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.bash).toBeDefined(); - expect(result.file_edit_replace_string).toBeUndefined(); - expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_replace).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.file_edit_insert).toBeUndefined(); }); @@ -170,8 +164,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace_string).toBeDefined(); - expect(result.file_edit_replace_lines).toBeDefined(); + expect(result.file_edit_replace).toBeDefined(); + expect(result.file_edit_replace).toBeDefined(); expect(result.file_edit_insert).toBeDefined(); }); @@ -185,8 +179,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace_string).toBeUndefined(); - expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_replace).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.file_edit_insert).toBeUndefined(); expect(result.web_search).toBeUndefined(); }); @@ -216,8 +210,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(Object.keys(result)).toHaveLength(1); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace_string).toBeUndefined(); - expect(result.file_edit_replace_lines).toBeUndefined(); + expect(result.file_edit_replace).toBeUndefined(); + expect(result.file_edit_insert).toBeUndefined(); expect(result.file_edit_insert).toBeUndefined(); expect(result.web_search).toBeUndefined(); }); diff --git a/src/utils/tools/tools.ts b/src/utils/tools/tools.ts index b290bdb5c0..c026e16ef4 100644 --- a/src/utils/tools/tools.ts +++ b/src/utils/tools/tools.ts @@ -4,8 +4,7 @@ import { openai } from "@ai-sdk/openai"; import { google } from "@ai-sdk/google"; import { createFileReadTool } from "@/services/tools/file_read"; import { createBashTool } from "@/services/tools/bash"; -import { createFileEditReplaceStringTool } from "@/services/tools/file_edit_replace_string"; -import { createFileEditReplaceLinesTool } from "@/services/tools/file_edit_replace_lines"; +import { createFileEditReplaceTool } from "@/services/tools/file_edit_replace"; import { createFileEditInsertTool } from "@/services/tools/file_edit_insert"; import { createProposePlanTool } from "@/services/tools/propose_plan"; import { createCompactSummaryTool } from "@/services/tools/compact_summary"; @@ -44,8 +43,7 @@ export function getToolsForModel( const baseTools: Record = { // Use snake_case for tool names to match what seems to be the convention. file_read: createFileReadTool(config), - file_edit_replace_string: createFileEditReplaceStringTool(config), - file_edit_replace_lines: createFileEditReplaceLinesTool(config), + file_edit_replace: createFileEditReplaceTool(config), file_edit_insert: createFileEditInsertTool(config), bash: createBashTool(config), propose_plan: createProposePlanTool(config), From bff483863743d5d9672166e1854e7e4b5b92d45a Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 12 Oct 2025 14:03:28 -0500 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=A4=96=20Fix=20formatting=20in=20tool?= =?UTF-8?q?OutputRedaction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/messages/toolOutputRedaction.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/utils/messages/toolOutputRedaction.ts b/src/utils/messages/toolOutputRedaction.ts index 709f22b487..e2ee9785f2 100644 --- a/src/utils/messages/toolOutputRedaction.ts +++ b/src/utils/messages/toolOutputRedaction.ts @@ -10,10 +10,7 @@ * - Type safety: if a tool's result type changes, these redactors should fail type-checks. */ -import type { - FileEditInsertToolResult, - FileEditReplaceToolResult, -} from "@/types/tools"; +import type { FileEditInsertToolResult, FileEditReplaceToolResult } from "@/types/tools"; // Tool-output from AI SDK is often wrapped like: { type: 'json', value: } // Keep this helper local so all redactors handle both wrapped and plain objects consistently. From feca0e7732f1860c5cc66e8994e3b3380aa46523 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 12 Oct 2025 14:04:17 -0500 Subject: [PATCH 5/8] =?UTF-8?q?=F0=9F=A4=96=20Add=20dirty=20state=20and=20?= =?UTF-8?q?sync=20validation=20to=20wait=5Fpr=5Fchecks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevents checking PR status when: - Working directory has uncommitted changes - Local branch doesn't have upstream configured - Local branch is out of sync with remote (ahead/behind/diverged) Provides clear error messages with suggested remediation steps. _Generated with `cmux`_ --- scripts/wait_pr_checks.sh | 55 ++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/scripts/wait_pr_checks.sh b/scripts/wait_pr_checks.sh index 7a3021e1b0..2b485d635b 100755 --- a/scripts/wait_pr_checks.sh +++ b/scripts/wait_pr_checks.sh @@ -10,15 +10,58 @@ if [ $# -eq 0 ]; then fi PR_NUMBER=$1 -echo "⏳ Waiting for PR #$PR_NUMBER checks to complete..." -# Warn if there are unstaged changes -if ! git diff --quiet 2>/dev/null; then - echo "⚠️ Warning: You have unstaged changes that are not pushed!" - echo " Run 'git status' to see what changes are pending." - echo "" +# Check for dirty working tree +if ! git diff-index --quiet HEAD --; then + echo "❌ Error: You have uncommitted changes in your working directory." >&2 + echo "" >&2 + git status --short >&2 + echo "" >&2 + echo "Please commit or stash your changes before checking PR status." >&2 + exit 1 +fi + +# Get current branch name +CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD) + +# Get remote tracking branch +REMOTE_BRANCH=$(git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null || echo "") + +if [[ -z "$REMOTE_BRANCH" ]]; then + echo "❌ Error: Current branch '$CURRENT_BRANCH' has no upstream branch." >&2 + echo "Set an upstream with: git push -u origin $CURRENT_BRANCH" >&2 + exit 1 fi +# Check if local and remote are in sync +LOCAL_HASH=$(git rev-parse HEAD) +REMOTE_HASH=$(git rev-parse "$REMOTE_BRANCH") + +if [[ "$LOCAL_HASH" != "$REMOTE_HASH" ]]; then + echo "❌ Error: Local branch is not in sync with remote." >&2 + echo "" >&2 + echo "Local: $LOCAL_HASH" >&2 + echo "Remote: $REMOTE_HASH" >&2 + echo "" >&2 + + # Check if we're ahead, behind, or diverged + if git merge-base --is-ancestor "$REMOTE_HASH" HEAD 2>/dev/null; then + AHEAD=$(git rev-list --count "$REMOTE_BRANCH"..HEAD) + echo "Your branch is $AHEAD commit(s) ahead of '$REMOTE_BRANCH'." >&2 + echo "Push your changes with: git push" >&2 + elif git merge-base --is-ancestor HEAD "$REMOTE_HASH" 2>/dev/null; then + BEHIND=$(git rev-list --count HEAD.."$REMOTE_BRANCH") + echo "Your branch is $BEHIND commit(s) behind '$REMOTE_BRANCH'." >&2 + echo "Pull the latest changes with: git pull" >&2 + else + echo "Your branch has diverged from '$REMOTE_BRANCH'." >&2 + echo "You may need to rebase or merge." >&2 + fi + + exit 1 +fi + +echo "⏳ Waiting for PR #$PR_NUMBER checks to complete..." echo "" while true; do From baa5c05a3156801784ef7ad18da92aeeac825af2 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 12 Oct 2025 14:07:05 -0500 Subject: [PATCH 6/8] =?UTF-8?q?=F0=9F=A4=96=20Fix=20shell=20formatting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scripts/wait_pr_checks.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/wait_pr_checks.sh b/scripts/wait_pr_checks.sh index 2b485d635b..189261841a 100755 --- a/scripts/wait_pr_checks.sh +++ b/scripts/wait_pr_checks.sh @@ -43,7 +43,7 @@ if [[ "$LOCAL_HASH" != "$REMOTE_HASH" ]]; then echo "Local: $LOCAL_HASH" >&2 echo "Remote: $REMOTE_HASH" >&2 echo "" >&2 - + # Check if we're ahead, behind, or diverged if git merge-base --is-ancestor "$REMOTE_HASH" HEAD 2>/dev/null; then AHEAD=$(git rev-list --count "$REMOTE_BRANCH"..HEAD) @@ -57,7 +57,7 @@ if [[ "$LOCAL_HASH" != "$REMOTE_HASH" ]]; then echo "Your branch has diverged from '$REMOTE_BRANCH'." >&2 echo "You may need to rebase or merge." >&2 fi - + exit 1 fi From 826e3542ccf64b9310f9af58adfabdadea4574a4 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 12 Oct 2025 14:31:41 -0500 Subject: [PATCH 7/8] =?UTF-8?q?=F0=9F=A4=96=20Split=20file=5Fedit=5Freplac?= =?UTF-8?q?e=20into=20two=20separate=20tools?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of using a discriminated union (which AI providers don't support), create two separate tools that share implementation: - file_edit_replace_string: String-based text replacement - file_edit_replace_lines: Line range replacement - file_edit_replace_shared.ts: Shared helper functions Benefits: - Simple object schemas that AI providers accept - Code reuse through shared helpers - Clearer tool separation in registry - Type safety maintained throughout Updated: - Tool definitions with two separate simple schemas - Tool registry to register both tools - UI components to handle both tool names - Types to separate payload/result types - Tests to cover both tools independently --- src/components/Messages/ToolMessage.tsx | 42 +++++++-- src/components/tools/FileEditToolCall.tsx | 18 ++-- src/services/tools/file_edit_replace.test.ts | 46 +++++----- src/services/tools/file_edit_replace_lines.ts | 51 +++++++++++ ...replace.ts => file_edit_replace_shared.ts} | 85 +++++++++---------- .../tools/file_edit_replace_string.ts | 37 ++++++++ src/types/tools.ts | 36 +++++--- src/utils/main/tokenizer.ts | 3 +- src/utils/messages/toolOutputRedaction.ts | 31 +++++-- src/utils/tools/toolDefinitions.ts | 75 ++++++++-------- src/utils/tools/toolPolicy.test.ts | 56 ++++++------ src/utils/tools/tools.ts | 6 +- 12 files changed, 323 insertions(+), 163 deletions(-) create mode 100644 src/services/tools/file_edit_replace_lines.ts rename src/services/tools/{file_edit_replace.ts => file_edit_replace_shared.ts} (67%) create mode 100644 src/services/tools/file_edit_replace_string.ts diff --git a/src/components/Messages/ToolMessage.tsx b/src/components/Messages/ToolMessage.tsx index 49fc6fd62e..c6db6f5607 100644 --- a/src/components/Messages/ToolMessage.tsx +++ b/src/components/Messages/ToolMessage.tsx @@ -13,8 +13,10 @@ import type { FileReadToolResult, FileEditInsertToolArgs, FileEditInsertToolResult, - FileEditReplaceToolArgs, - FileEditReplaceToolResult, + FileEditReplaceStringToolArgs, + FileEditReplaceStringToolResult, + FileEditReplaceLinesToolArgs, + FileEditReplaceLinesToolResult, ProposePlanToolArgs, ProposePlanToolResult, } from "@/types/tools"; @@ -37,9 +39,20 @@ function isFileReadTool(toolName: string, args: unknown): args is FileReadToolAr return TOOL_DEFINITIONS.file_read.schema.safeParse(args).success; } -function isFileEditReplaceTool(toolName: string, args: unknown): args is FileEditReplaceToolArgs { - if (toolName !== "file_edit_replace") return false; - return TOOL_DEFINITIONS.file_edit_replace.schema.safeParse(args).success; +function isFileEditReplaceStringTool( + toolName: string, + args: unknown +): args is FileEditReplaceStringToolArgs { + if (toolName !== "file_edit_replace_string") return false; + return TOOL_DEFINITIONS.file_edit_replace_string.schema.safeParse(args).success; +} + +function isFileEditReplaceLinesTool( + toolName: string, + args: unknown +): args is FileEditReplaceLinesToolArgs { + if (toolName !== "file_edit_replace_lines") return false; + return TOOL_DEFINITIONS.file_edit_replace_lines.schema.safeParse(args).success; } function isFileEditInsertTool(toolName: string, args: unknown): args is FileEditInsertToolArgs { @@ -79,13 +92,26 @@ export const ToolMessage: React.FC = ({ message, className, wo ); } - if (isFileEditReplaceTool(message.toolName, message.args)) { + if (isFileEditReplaceStringTool(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 67b43114c9..e816c45bb8 100644 --- a/src/components/tools/FileEditToolCall.tsx +++ b/src/components/tools/FileEditToolCall.tsx @@ -4,8 +4,10 @@ import { parsePatch } from "diff"; import type { FileEditInsertToolArgs, FileEditInsertToolResult, - FileEditReplaceToolArgs, - FileEditReplaceToolResult, + FileEditReplaceStringToolArgs, + FileEditReplaceStringToolResult, + FileEditReplaceLinesToolArgs, + FileEditReplaceLinesToolResult, } from "@/types/tools"; import { ToolContainer, @@ -177,12 +179,18 @@ const ButtonGroup = styled.div` margin-right: 8px; `; -type FileEditOperationArgs = FileEditReplaceToolArgs | FileEditInsertToolArgs; +type FileEditOperationArgs = + | FileEditReplaceStringToolArgs + | FileEditReplaceLinesToolArgs + | FileEditInsertToolArgs; -type FileEditToolResult = FileEditReplaceToolResult | FileEditInsertToolResult; +type FileEditToolResult = + | FileEditReplaceStringToolResult + | FileEditReplaceLinesToolResult + | FileEditInsertToolResult; interface FileEditToolCallProps { - toolName: "file_edit_replace" | "file_edit_insert"; + 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_replace.test.ts b/src/services/tools/file_edit_replace.test.ts index a7a02c2369..e8e672bfe8 100644 --- a/src/services/tools/file_edit_replace.test.ts +++ b/src/services/tools/file_edit_replace.test.ts @@ -2,12 +2,13 @@ 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 { createFileEditReplaceTool } from "./file_edit_replace"; +import { createFileEditReplaceStringTool } from "./file_edit_replace_string"; +import { createFileEditReplaceLinesTool } from "./file_edit_replace_lines"; import type { - FileEditReplaceLinesPayload, - FileEditReplaceStringPayload, - FileEditReplaceToolArgs, - FileEditReplaceToolResult, + FileEditReplaceStringToolArgs, + FileEditReplaceStringToolResult, + FileEditReplaceLinesToolArgs, + FileEditReplaceLinesToolResult, } from "@/types/tools"; import type { ToolCallOptions } from "ai"; @@ -26,14 +27,21 @@ const readFile = async (filePath: string): Promise => { return await fs.readFile(filePath, "utf-8"); }; -const executeReplace = async ( - tool: ReturnType, - args: FileEditReplaceToolArgs -): Promise => { - return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceToolResult; +const executeStringReplace = async ( + tool: ReturnType, + args: FileEditReplaceStringToolArgs +): Promise => { + return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceStringToolResult; }; -describe("file_edit_replace tool (string mode)", () => { +const executeLinesReplace = async ( + tool: ReturnType, + args: FileEditReplaceLinesToolArgs +): Promise => { + return (await tool.execute!(args, mockToolCallOptions)) as FileEditReplaceLinesToolResult; +}; + +describe("file_edit_replace_string tool", () => { let testDir: string; let testFilePath: string; @@ -48,16 +56,15 @@ describe("file_edit_replace tool (string mode)", () => { it("should apply a single edit successfully", async () => { await setupFile(testFilePath, "Hello world\nThis is a test\nGoodbye world"); - const tool = createFileEditReplaceTool({ cwd: testDir }); + const tool = createFileEditReplaceStringTool({ cwd: testDir }); - const payload: FileEditReplaceStringPayload = { - mode: "string", + const payload: FileEditReplaceStringToolArgs = { file_path: testFilePath, old_string: "Hello world", new_string: "Hello universe", }; - const result = await executeReplace(tool, payload); + const result = await executeStringReplace(tool, payload); expect(result.success).toBe(true); if (result.success) { @@ -68,7 +75,7 @@ describe("file_edit_replace tool (string mode)", () => { }); }); -describe("file_edit_replace tool (lines mode)", () => { +describe("file_edit_replace_lines tool", () => { let testDir: string; let testFilePath: string; @@ -83,17 +90,16 @@ describe("file_edit_replace tool (lines mode)", () => { it("should replace a line range successfully", async () => { await setupFile(testFilePath, "line1\nline2\nline3\nline4"); - const tool = createFileEditReplaceTool({ cwd: testDir }); + const tool = createFileEditReplaceLinesTool({ cwd: testDir }); - const payload: FileEditReplaceLinesPayload = { - mode: "lines", + const payload: FileEditReplaceLinesToolArgs = { file_path: testFilePath, start_line: 2, end_line: 3, new_lines: ["LINE2", "LINE3"], }; - const result = await executeReplace(tool, payload); + const result = await executeLinesReplace(tool, payload); expect(result.success).toBe(true); if (result.success) { diff --git a/src/services/tools/file_edit_replace_lines.ts b/src/services/tools/file_edit_replace_lines.ts new file mode 100644 index 0000000000..30be0a586d --- /dev/null +++ b/src/services/tools/file_edit_replace_lines.ts @@ -0,0 +1,51 @@ +import { tool } from "ai"; +import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; +import { executeFileEditOperation } from "./file_edit_operation"; +import { handleLineReplace, type LineReplaceArgs } from "./file_edit_replace_shared"; + +export interface FileEditReplaceLinesResult { + success: true; + diff: string; + edits_applied: number; + lines_replaced: number; + line_delta: number; +} + +export interface FileEditReplaceLinesError { + success: false; + error: string; +} + +export type FileEditReplaceLinesToolResult = FileEditReplaceLinesResult | FileEditReplaceLinesError; + +/** + * Line-based file edit replace tool factory + */ +export const createFileEditReplaceLinesTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.file_edit_replace_lines.description, + inputSchema: TOOL_DEFINITIONS.file_edit_replace_lines.schema, + execute: async (args: LineReplaceArgs): Promise => { + const result = await executeFileEditOperation({ + config, + filePath: args.file_path, + operation: (originalContent) => handleLineReplace(args, originalContent), + }); + + // handleLineReplace always returns lines_replaced and line_delta, + // so we can safely assert this meets FileEditReplaceLinesToolResult + if (result.success) { + return { + success: true, + diff: result.diff, + edits_applied: result.edits_applied, + lines_replaced: result.lines_replaced!, + line_delta: result.line_delta!, + }; + } + + return result; + }, + }); +}; diff --git a/src/services/tools/file_edit_replace.ts b/src/services/tools/file_edit_replace_shared.ts similarity index 67% rename from src/services/tools/file_edit_replace.ts rename to src/services/tools/file_edit_replace_shared.ts index 0030b9c724..0e9c80037a 100644 --- a/src/services/tools/file_edit_replace.ts +++ b/src/services/tools/file_edit_replace_shared.ts @@ -1,13 +1,9 @@ -import { tool } from "ai"; -import type { - FileEditReplaceToolArgs, - FileEditReplaceToolResult, - FileEditReplaceLinesPayload, - FileEditReplaceStringPayload, -} from "@/types/tools"; -import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; -import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { executeFileEditOperation } from "./file_edit_operation"; +/** + * Shared implementation for file edit replace tools + * + * These helpers are used by both string-based and line-based replace tools, + * providing the core logic while keeping the tool definitions simple for AI providers. + */ interface OperationMetadata { edits_applied: number; @@ -15,47 +11,41 @@ interface OperationMetadata { line_delta?: number; } -interface OperationResult { +export interface OperationResult { success: true; newContent: string; metadata: OperationMetadata; } +export interface OperationError { + success: false; + error: string; +} + +export type OperationOutcome = OperationResult | OperationError; + +export interface StringReplaceArgs { + file_path: string; + old_string: string; + new_string: string; + replace_count?: number; +} + +export interface LineReplaceArgs { + file_path: string; + start_line: number; + end_line: number; + new_lines: string[]; + expected_lines?: string[]; +} + /** - * File edit replace tool factory for AI assistant. - * Supports string replacements and line range replacements using a discriminated union payload. + * Handle string-based replacement */ -export const createFileEditReplaceTool: ToolFactory = (config: ToolConfiguration) => { - return tool({ - description: TOOL_DEFINITIONS.file_edit_replace.description, - inputSchema: TOOL_DEFINITIONS.file_edit_replace.schema, - execute: async (args: FileEditReplaceToolArgs): Promise => { - return executeFileEditOperation({ - config, - filePath: args.file_path, - operation: (originalContent) => { - if (args.mode === "string") { - return handleStringReplace(args, originalContent); - } - - if (args.mode === "lines") { - return handleLineReplace(args, originalContent); - } - - return { - success: false, - error: `Unsupported mode: ${(args as { mode?: string }).mode ?? ""}`, - }; - }, - }); - }, - }); -}; - -function handleStringReplace( - args: FileEditReplaceStringPayload, +export function handleStringReplace( + args: StringReplaceArgs, originalContent: string -): OperationResult | { success: false; error: string } { +): OperationOutcome { const replaceCount = args.replace_count ?? 1; if (!originalContent.includes(args.old_string)) { @@ -119,10 +109,13 @@ function handleStringReplace( }; } -function handleLineReplace( - args: FileEditReplaceLinesPayload, +/** + * Handle line-range replacement + */ +export function handleLineReplace( + args: LineReplaceArgs, originalContent: string -): OperationResult | { success: false; error: string } { +): OperationOutcome { const startIndex = args.start_line - 1; const endIndex = args.end_line - 1; diff --git a/src/services/tools/file_edit_replace_string.ts b/src/services/tools/file_edit_replace_string.ts new file mode 100644 index 0000000000..8c255a0687 --- /dev/null +++ b/src/services/tools/file_edit_replace_string.ts @@ -0,0 +1,37 @@ +import { tool } from "ai"; +import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; +import { executeFileEditOperation } from "./file_edit_operation"; +import { handleStringReplace, type StringReplaceArgs } from "./file_edit_replace_shared"; + +export interface FileEditReplaceStringResult { + success: true; + diff: string; + edits_applied: number; +} + +export interface FileEditReplaceStringError { + success: false; + error: string; +} + +export type FileEditReplaceStringToolResult = + | FileEditReplaceStringResult + | FileEditReplaceStringError; + +/** + * String-based file edit replace tool factory + */ +export const createFileEditReplaceStringTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.file_edit_replace_string.description, + inputSchema: TOOL_DEFINITIONS.file_edit_replace_string.schema, + execute: async (args: StringReplaceArgs): Promise => { + return executeFileEditOperation({ + config, + filePath: args.file_path, + operation: (originalContent) => handleStringReplace(args, originalContent), + }); + }, + }); +}; diff --git a/src/types/tools.ts b/src/types/tools.ts index da260386ba..2e7adb20cd 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -58,16 +58,20 @@ export interface FileEditErrorResult { error: string; } -export interface FileEditReplaceStringPayload { - mode: "string"; +export interface FileEditReplaceStringToolArgs { file_path: string; old_string: string; new_string: string; replace_count?: number; } -export interface FileEditReplaceLinesPayload { - mode: "lines"; +export type FileEditReplaceStringToolResult = + | (FileEditDiffSuccessBase & { + edits_applied: number; + }) + | FileEditErrorResult; + +export interface FileEditReplaceLinesToolArgs { file_path: string; start_line: number; end_line: number; @@ -75,19 +79,24 @@ export interface FileEditReplaceLinesPayload { expected_lines?: string[]; } -export type FileEditReplaceToolArgs = FileEditReplaceStringPayload | FileEditReplaceLinesPayload; - -export type FileEditReplaceToolResult = +export type FileEditReplaceLinesToolResult = | (FileEditDiffSuccessBase & { edits_applied: number; - lines_replaced?: number; - line_delta?: number; + lines_replaced: number; + line_delta: number; }) | FileEditErrorResult; -export type FileEditSharedToolResult = FileEditReplaceToolResult | FileEditInsertToolResult; +export type FileEditSharedToolResult = + | FileEditReplaceStringToolResult + | FileEditReplaceLinesToolResult + | FileEditInsertToolResult; -export const FILE_EDIT_TOOL_NAMES = ["file_edit_replace", "file_edit_insert"] as const; +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]; @@ -100,7 +109,10 @@ export interface FileEditInsertToolArgs { export type FileEditInsertToolResult = FileEditDiffSuccessBase | FileEditErrorResult; -export type FileEditToolArgs = FileEditReplaceToolArgs | FileEditInsertToolArgs; +export type FileEditToolArgs = + | FileEditReplaceStringToolArgs + | FileEditReplaceLinesToolArgs + | FileEditInsertToolArgs; export interface FileEditToolMessage { toolName: FileEditToolName; diff --git a/src/utils/main/tokenizer.ts b/src/utils/main/tokenizer.ts index 180c28dbdb..4c9f3c78aa 100644 --- a/src/utils/main/tokenizer.ts +++ b/src/utils/main/tokenizer.ts @@ -155,7 +155,8 @@ export function getToolDefinitionTokens(toolName: string, modelString: string): const fallbackSizes: Record = { bash: 65, file_read: 45, - file_edit_replace: 70, + 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 e2ee9785f2..8f2da0e7e9 100644 --- a/src/utils/messages/toolOutputRedaction.ts +++ b/src/utils/messages/toolOutputRedaction.ts @@ -10,7 +10,11 @@ * - Type safety: if a tool's result type changes, these redactors should fail type-checks. */ -import type { FileEditInsertToolResult, FileEditReplaceToolResult } from "@/types/tools"; +import type { + FileEditInsertToolResult, + FileEditReplaceStringToolResult, + FileEditReplaceLinesToolResult, +} from "@/types/tools"; // Tool-output from AI SDK is often wrapped like: { type: 'json', value: } // Keep this helper local so all redactors handle both wrapped and plain objects consistently. @@ -33,7 +37,9 @@ function rewrapJsonContainer(wrapped: boolean, value: unknown): unknown { } // Narrowing helpers for our tool result types -function isFileEditReplaceResult(v: unknown): v is FileEditReplaceToolResult { +function isFileEditResult( + v: unknown +): v is FileEditReplaceStringToolResult | FileEditReplaceLinesToolResult { return ( typeof v === "object" && v !== null && @@ -51,19 +57,29 @@ function isFileEditInsertResult(v: unknown): v is FileEditInsertToolResult { ); } -// Redactors per tool +// Redactors per tool - unified for both string and line replace function redactFileEditReplace(output: unknown): unknown { const unwrapped = unwrapJsonContainer(output); const val = unwrapped.value; - if (!isFileEditReplaceResult(val)) return output; // unknown structure, leave as-is + if (!isFileEditResult(val)) return output; // unknown structure, leave as-is - if (val.success) { - const compact: FileEditReplaceToolResult = { + if (val.success && "edits_applied" in val) { + // Build base compact result + const compact: Record = { success: true, edits_applied: val.edits_applied, diff: "[diff omitted in context - call file_read on the target file if needed]", }; + + // Preserve line metadata for line-based edits (only if present) + if ("lines_replaced" in val) { + compact.lines_replaced = val.lines_replaced; + } + if ("line_delta" in val) { + compact.line_delta = val.line_delta; + } + return rewrapJsonContainer(unwrapped.wrapped, compact); } @@ -91,7 +107,8 @@ function redactFileEditInsert(output: unknown): unknown { // Public API - registry entrypoint. Add new tools here as needed. export function redactToolOutput(toolName: string, output: unknown): unknown { switch (toolName) { - case "file_edit_replace": + case "file_edit_replace_string": + case "file_edit_replace_lines": return redactFileEditReplace(output); case "file_edit_insert": return redactFileEditInsert(output); diff --git a/src/utils/tools/toolDefinitions.ts b/src/utils/tools/toolDefinitions.ts index 47f386f300..9e6eecfe23 100644 --- a/src/utils/tools/toolDefinitions.ts +++ b/src/utils/tools/toolDefinitions.ts @@ -66,43 +66,43 @@ export const TOOL_DEFINITIONS = { .describe("Number of lines to return from offset (optional, returns all if not specified)"), }), }, - file_edit_replace: { + file_edit_replace_string: { description: - "Replace content in a file using either exact string matching or line ranges. Choose mode='string' for text replacements or mode='lines' for line-range updates.", - schema: z.discriminatedUnion("mode", [ - z.object({ - mode: z.literal("string"), - file_path: z.string().describe("The absolute path to the file to edit"), - old_string: z - .string() - .describe( - "The exact text to replace. Include enough context (indentation, surrounding lines) to make it unique." - ), - new_string: z.string().describe("The replacement text"), - replace_count: z - .number() - .int() - .optional() - .describe( - "Number of occurrences to replace (default: 1). Use -1 to replace all occurrences. If 1, old_string must be unique in the file." - ), - }), - z.object({ - mode: z.literal("lines"), - file_path: z.string().describe("The absolute path to the file to edit"), - 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 - .array(z.string()) - .describe("Replacement lines. Provide an empty array to delete the specified range."), - expected_lines: z - .array(z.string()) - .optional() - .describe( - "Optional safety check. When provided, the current lines in the specified range must match exactly." - ), - }), - ]), + "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"), + old_string: z + .string() + .describe( + "The exact text to replace (must be unique in file if replace_count is 1). Include enough context (indentation, surrounding lines) to make it unique." + ), + new_string: z.string().describe("The replacement text"), + replace_count: z + .number() + .int() + .optional() + .describe( + "Number of occurrences to replace (default: 1). Use -1 to replace all occurrences. If 1, old_string must be unique in the file." + ), + }), + }, + file_edit_replace_lines: { + description: + "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"), + 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 + .array(z.string()) + .describe("Replacement lines. Provide an empty array to delete the specified range."), + expected_lines: z + .array(z.string()) + .optional() + .describe( + "Optional safety check. When provided, the current lines in the specified range must match exactly." + ), + }), }, file_edit_insert: { description: @@ -185,7 +185,8 @@ export function getAvailableTools(modelString: string): string[] { const baseTools = [ "bash", "file_read", - "file_edit_replace", + "file_edit_replace_string", + "file_edit_replace_lines", "file_edit_insert", "propose_plan", "compact_summary", diff --git a/src/utils/tools/toolPolicy.test.ts b/src/utils/tools/toolPolicy.test.ts index 2979482794..4782e9d4a4 100644 --- a/src/utils/tools/toolPolicy.test.ts +++ b/src/utils/tools/toolPolicy.test.ts @@ -14,9 +14,14 @@ const mockTools = { inputSchema: z.object({ path: z.string() }), execute: () => Promise.resolve({ content: "test" }), }), - file_edit_replace: tool({ - description: "Replace content in files", - inputSchema: z.object({ path: z.string(), mode: z.string() }), + file_edit_replace_string: tool({ + description: "Replace content in files using string matching", + inputSchema: z.object({ path: z.string(), old_string: z.string() }), + execute: () => Promise.resolve({ success: true }), + }), + file_edit_replace_lines: tool({ + description: "Replace content in files using line ranges", + inputSchema: z.object({ path: z.string(), start_line: z.number() }), execute: () => Promise.resolve({ success: true }), }), file_edit_insert: tool({ @@ -51,8 +56,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeUndefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace).toBeDefined(); - expect(result.file_edit_replace).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(); }); @@ -67,8 +72,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeUndefined(); expect(result.web_search).toBeUndefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace).toBeDefined(); - expect(result.file_edit_replace).toBeDefined(); + expect(result.file_edit_replace_string).toBeDefined(); + expect(result.file_edit_replace_lines).toBeDefined(); expect(result.file_edit_insert).toBeDefined(); }); }); @@ -78,8 +83,8 @@ describe("applyToolPolicy", () => { const policy: ToolPolicy = [{ regex_match: "file_edit_.*", action: "disable" }]; const result = applyToolPolicy(mockTools, policy); - expect(result.file_edit_replace).toBeUndefined(); - expect(result.file_edit_insert).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.file_read).toBeDefined(); @@ -98,8 +103,8 @@ describe("applyToolPolicy", () => { const result = applyToolPolicy(mockTools, policy); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace).toBeUndefined(); - expect(result.file_edit_insert).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(); @@ -116,20 +121,21 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace).toBeUndefined(); - expect(result.file_edit_insert).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("disables file_edit_.* then enables file_edit_replace", () => { + test("disables file_edit_.* then enables file_edit_replace_string", () => { const policy: ToolPolicy = [ { regex_match: "file_edit_.*", action: "disable" }, - { regex_match: "file_edit_replace", action: "enable" }, + { regex_match: "file_edit_replace_string", action: "enable" }, ]; const result = applyToolPolicy(mockTools, policy); - expect(result.file_edit_replace).toBeDefined(); + 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(); @@ -154,8 +160,8 @@ describe("applyToolPolicy", () => { expect(result.file_read).toBeDefined(); expect(result.bash).toBeDefined(); - expect(result.file_edit_replace).toBeUndefined(); - expect(result.file_edit_insert).toBeUndefined(); + expect(result.file_edit_replace_string).toBeUndefined(); + expect(result.file_edit_replace_lines).toBeUndefined(); expect(result.file_edit_insert).toBeUndefined(); }); @@ -164,8 +170,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace).toBeDefined(); - expect(result.file_edit_replace).toBeDefined(); + expect(result.file_edit_replace_string).toBeDefined(); + expect(result.file_edit_replace_lines).toBeDefined(); expect(result.file_edit_insert).toBeDefined(); }); @@ -179,8 +185,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(result.file_read).toBeDefined(); - expect(result.file_edit_replace).toBeUndefined(); - expect(result.file_edit_insert).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(); }); @@ -210,8 +216,8 @@ describe("applyToolPolicy", () => { expect(result.bash).toBeDefined(); expect(Object.keys(result)).toHaveLength(1); expect(result.file_read).toBeUndefined(); - expect(result.file_edit_replace).toBeUndefined(); - expect(result.file_edit_insert).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(); }); @@ -219,7 +225,7 @@ describe("applyToolPolicy", () => { 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, file_edit_insert) + // 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 c026e16ef4..b290bdb5c0 100644 --- a/src/utils/tools/tools.ts +++ b/src/utils/tools/tools.ts @@ -4,7 +4,8 @@ import { openai } from "@ai-sdk/openai"; import { google } from "@ai-sdk/google"; import { createFileReadTool } from "@/services/tools/file_read"; import { createBashTool } from "@/services/tools/bash"; -import { createFileEditReplaceTool } from "@/services/tools/file_edit_replace"; +import { createFileEditReplaceStringTool } from "@/services/tools/file_edit_replace_string"; +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 { createCompactSummaryTool } from "@/services/tools/compact_summary"; @@ -43,7 +44,8 @@ export function getToolsForModel( const baseTools: Record = { // Use snake_case for tool names to match what seems to be the convention. file_read: createFileReadTool(config), - file_edit_replace: createFileEditReplaceTool(config), + file_edit_replace_string: createFileEditReplaceStringTool(config), + file_edit_replace_lines: createFileEditReplaceLinesTool(config), file_edit_insert: createFileEditInsertTool(config), bash: createBashTool(config), propose_plan: createProposePlanTool(config), From 023d3974be5eb3d8b37dadd72035412c39a59f26 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 12 Oct 2025 14:36:52 -0500 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=A4=96=20Fix=20integration=20test=20t?= =?UTF-8?q?o=20check=20for=20both=20file=20edit=20tools?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update test to look for either file_edit_replace_string or file_edit_replace_lines instead of the old unified tool name. --- tests/ipcMain/sendMessage.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/ipcMain/sendMessage.test.ts b/tests/ipcMain/sendMessage.test.ts index 30fd22bd5f..aafa011e22 100644 --- a/tests/ipcMain/sendMessage.test.ts +++ b/tests/ipcMain/sendMessage.test.ts @@ -1333,10 +1333,11 @@ These are general instructions that apply to all modes. e !== null && "type" in e && (e as any).type === "tool-call-end" && - (e as any).toolName === "file_edit_replace" + ((e as any).toolName === "file_edit_replace_string" || + (e as any).toolName === "file_edit_replace_lines") ) as any[]; - // Find the last successful file_edit_replace event (model may retry) + // Find the last successful file_edit_replace_* event (model may retry) const successfulEdits = allFileEditEvents.filter((e) => { const result = e?.result; const payload = result && result.value ? result.value : result;