From 45e23f80ddc66fda0df87c079822f2d8bf3c861d Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 8 Dec 2025 10:34:43 -0700 Subject: [PATCH] feat: add search_replace native tool for single-replacement operations Adds a new search_replace tool that performs a single search and replace operation on a file, requiring the old_string to uniquely identify the target text with 3-5 lines of context. Parameters: - file_path: Path to file (relative or absolute) - old_string: Text to find (must be unique in file) - new_string: Replacement text (must differ from old_string) --- packages/types/src/tool.ts | 1 + .../assistant-message/NativeToolCallParser.ts | 28 ++ .../presentAssistantMessage.ts | 13 + src/core/prompts/tools/native-tools/index.ts | 2 + .../tools/native-tools/search_replace.ts | 51 +++ src/core/tools/SearchReplaceTool.ts | 277 +++++++++++++ .../tools/__tests__/searchReplaceTool.spec.ts | 382 ++++++++++++++++++ src/shared/tools.ts | 7 +- 8 files changed, 760 insertions(+), 1 deletion(-) create mode 100644 src/core/prompts/tools/native-tools/search_replace.ts create mode 100644 src/core/tools/SearchReplaceTool.ts create mode 100644 src/core/tools/__tests__/searchReplaceTool.spec.ts diff --git a/packages/types/src/tool.ts b/packages/types/src/tool.ts index 69ef9ee4b75..fa200865124 100644 --- a/packages/types/src/tool.ts +++ b/packages/types/src/tool.ts @@ -20,6 +20,7 @@ export const toolNames = [ "write_to_file", "apply_diff", "search_and_replace", + "search_replace", "apply_patch", "search_files", "list_files", diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index f02bb8003bc..d9b810c7ca8 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -499,6 +499,20 @@ export class NativeToolCallParser { } break + case "search_replace": + if ( + partialArgs.file_path !== undefined || + partialArgs.old_string !== undefined || + partialArgs.new_string !== undefined + ) { + nativeArgs = { + file_path: partialArgs.file_path, + old_string: partialArgs.old_string, + new_string: partialArgs.new_string, + } + } + break + // Add other tools as needed default: break @@ -742,6 +756,20 @@ export class NativeToolCallParser { } break + case "search_replace": + if ( + args.file_path !== undefined && + args.old_string !== undefined && + args.new_string !== undefined + ) { + nativeArgs = { + file_path: args.file_path, + old_string: args.old_string, + new_string: args.new_string, + } as NativeArgsFor + } + break + default: break } diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 8089fdbdef4..7bbb692360f 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -18,6 +18,7 @@ import { shouldUseSingleFileRead, TOOL_PROTOCOL } from "@roo-code/types" import { writeToFileTool } from "../tools/WriteToFileTool" import { applyDiffTool } from "../tools/MultiApplyDiffTool" import { searchAndReplaceTool } from "../tools/SearchAndReplaceTool" +import { searchReplaceTool } from "../tools/SearchReplaceTool" import { applyPatchTool } from "../tools/ApplyPatchTool" import { listCodeDefinitionNamesTool } from "../tools/ListCodeDefinitionNamesTool" import { searchFilesTool } from "../tools/SearchFilesTool" @@ -385,6 +386,8 @@ export async function presentAssistantMessage(cline: Task) { }]` case "search_and_replace": return `[${block.name} for '${block.params.path}']` + case "search_replace": + return `[${block.name} for '${block.params.file_path}']` case "apply_patch": return `[${block.name}]` case "list_files": @@ -835,6 +838,16 @@ export async function presentAssistantMessage(cline: Task) { toolProtocol, }) break + case "search_replace": + await checkpointSaveAndMark(cline) + await searchReplaceTool.handle(cline, block as ToolUse<"search_replace">, { + askApproval, + handleError, + pushToolResult, + removeClosingTag, + toolProtocol, + }) + break case "apply_patch": await checkpointSaveAndMark(cline) await applyPatchTool.handle(cline, block as ToolUse<"apply_patch">, { diff --git a/src/core/prompts/tools/native-tools/index.ts b/src/core/prompts/tools/native-tools/index.ts index 1cb0baab837..0f6a0c4b8db 100644 --- a/src/core/prompts/tools/native-tools/index.ts +++ b/src/core/prompts/tools/native-tools/index.ts @@ -15,6 +15,7 @@ import newTask from "./new_task" import { createReadFileTool } from "./read_file" import runSlashCommand from "./run_slash_command" import searchAndReplace from "./search_and_replace" +import searchReplace from "./search_replace" import searchFiles from "./search_files" import switchMode from "./switch_mode" import updateTodoList from "./update_todo_list" @@ -47,6 +48,7 @@ export function getNativeTools(partialReadsEnabled: boolean = true): OpenAI.Chat createReadFileTool(partialReadsEnabled), runSlashCommand, searchAndReplace, + searchReplace, searchFiles, switchMode, updateTodoList, diff --git a/src/core/prompts/tools/native-tools/search_replace.ts b/src/core/prompts/tools/native-tools/search_replace.ts new file mode 100644 index 00000000000..cc3b0e5269e --- /dev/null +++ b/src/core/prompts/tools/native-tools/search_replace.ts @@ -0,0 +1,51 @@ +import type OpenAI from "openai" + +const SEARCH_REPLACE_DESCRIPTION = `Use this tool to propose a search and replace operation on an existing file. + +The tool will replace ONE occurrence of old_string with new_string in the specified file. + +CRITICAL REQUIREMENTS FOR USING THIS TOOL: + +1. UNIQUENESS: The old_string MUST uniquely identify the specific instance you want to change. This means: + - Include AT LEAST 3-5 lines of context BEFORE the change point + - Include AT LEAST 3-5 lines of context AFTER the change point + - Include all whitespace, indentation, and surrounding code exactly as it appears in the file + +2. SINGLE INSTANCE: This tool can only change ONE instance at a time. If you need to change multiple instances: + - Make separate calls to this tool for each instance + - Each call must uniquely identify its specific instance using extensive context + +3. VERIFICATION: Before using this tool: + - If multiple instances exist, gather enough context to uniquely identify each one + - Plan separate tool calls for each instance` + +const search_replace = { + type: "function", + function: { + name: "search_replace", + description: SEARCH_REPLACE_DESCRIPTION, + parameters: { + type: "object", + properties: { + file_path: { + type: "string", + description: + "The path to the file you want to search and replace in. You can use either a relative path in the workspace or an absolute path. If an absolute path is provided, it will be preserved as is.", + }, + old_string: { + type: "string", + description: + "The text to replace (must be unique within the file, and must match the file contents exactly, including all whitespace and indentation)", + }, + new_string: { + type: "string", + description: "The edited text to replace the old_string (must be different from the old_string)", + }, + }, + required: ["file_path", "old_string", "new_string"], + additionalProperties: false, + }, + }, +} satisfies OpenAI.Chat.ChatCompletionTool + +export default search_replace diff --git a/src/core/tools/SearchReplaceTool.ts b/src/core/tools/SearchReplaceTool.ts new file mode 100644 index 00000000000..b0150d04a17 --- /dev/null +++ b/src/core/tools/SearchReplaceTool.ts @@ -0,0 +1,277 @@ +import fs from "fs/promises" +import path from "path" + +import { getReadablePath } from "../../utils/path" +import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { Task } from "../task/Task" +import { formatResponse } from "../prompts/responses" +import { ClineSayTool } from "../../shared/ExtensionMessage" +import { RecordSource } from "../context-tracking/FileContextTrackerTypes" +import { fileExistsAtPath } from "../../utils/fs" +import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" +import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" +import { sanitizeUnifiedDiff, computeDiffStats } from "../diff/stats" +import { BaseTool, ToolCallbacks } from "./BaseTool" +import type { ToolUse } from "../../shared/tools" + +interface SearchReplaceParams { + file_path: string + old_string: string + new_string: string +} + +export class SearchReplaceTool extends BaseTool<"search_replace"> { + readonly name = "search_replace" as const + + parseLegacy(params: Partial>): SearchReplaceParams { + return { + file_path: params.file_path || "", + old_string: params.old_string || "", + new_string: params.new_string || "", + } + } + + async execute(params: SearchReplaceParams, task: Task, callbacks: ToolCallbacks): Promise { + const { file_path, old_string, new_string } = params + const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks + + try { + // Validate required parameters + if (!file_path) { + task.consecutiveMistakeCount++ + task.recordToolError("search_replace") + pushToolResult(await task.sayAndCreateMissingParamError("search_replace", "file_path")) + return + } + + if (!old_string) { + task.consecutiveMistakeCount++ + task.recordToolError("search_replace") + pushToolResult(await task.sayAndCreateMissingParamError("search_replace", "old_string")) + return + } + + if (new_string === undefined) { + task.consecutiveMistakeCount++ + task.recordToolError("search_replace") + pushToolResult(await task.sayAndCreateMissingParamError("search_replace", "new_string")) + return + } + + // Validate that old_string and new_string are different + if (old_string === new_string) { + task.consecutiveMistakeCount++ + task.recordToolError("search_replace") + pushToolResult( + formatResponse.toolError( + "The 'old_string' and 'new_string' parameters must be different.", + toolProtocol, + ), + ) + return + } + + // Determine relative path - file_path can be absolute or relative + let relPath: string + if (path.isAbsolute(file_path)) { + relPath = path.relative(task.cwd, file_path) + } else { + relPath = file_path + } + + const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + + if (!accessAllowed) { + await task.say("rooignore_error", relPath) + pushToolResult(formatResponse.rooIgnoreError(relPath, toolProtocol)) + return + } + + // Check if file is write-protected + const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath) || false + + const absolutePath = path.resolve(task.cwd, relPath) + + const fileExists = await fileExistsAtPath(absolutePath) + if (!fileExists) { + task.consecutiveMistakeCount++ + task.recordToolError("search_replace") + const errorMessage = `File not found: ${relPath}. Cannot perform search and replace on a non-existent file.` + await task.say("error", errorMessage) + pushToolResult(formatResponse.toolError(errorMessage, toolProtocol)) + return + } + + let fileContent: string + try { + fileContent = await fs.readFile(absolutePath, "utf8") + } catch (error) { + task.consecutiveMistakeCount++ + task.recordToolError("search_replace") + const errorMessage = `Failed to read file '${relPath}'. Please verify file permissions and try again.` + await task.say("error", errorMessage) + pushToolResult(formatResponse.toolError(errorMessage, toolProtocol)) + return + } + + // Check for exact match (literal string, not regex) + const matchCount = fileContent.split(old_string).length - 1 + + if (matchCount === 0) { + task.consecutiveMistakeCount++ + task.recordToolError("search_replace", "no_match") + pushToolResult( + formatResponse.toolError( + `No match found for the specified 'old_string'. Please ensure it matches the file contents exactly, including whitespace and indentation.`, + toolProtocol, + ), + ) + return + } + + if (matchCount > 1) { + task.consecutiveMistakeCount++ + task.recordToolError("search_replace", "multiple_matches") + pushToolResult( + formatResponse.toolError( + `Found ${matchCount} matches for the specified 'old_string'. This tool can only replace ONE occurrence at a time. Please provide more context (3-5 lines before and after) to uniquely identify the specific instance you want to change.`, + toolProtocol, + ), + ) + return + } + + // Apply the single replacement + const newContent = fileContent.replace(old_string, new_string) + + // Check if any changes were made + if (newContent === fileContent) { + pushToolResult(`No changes needed for '${relPath}'`) + return + } + + task.consecutiveMistakeCount = 0 + + // Initialize diff view + task.diffViewProvider.editType = "modify" + task.diffViewProvider.originalContent = fileContent + + // Generate and validate diff + const diff = formatResponse.createPrettyPatch(relPath, fileContent, newContent) + if (!diff) { + pushToolResult(`No changes needed for '${relPath}'`) + await task.diffViewProvider.reset() + return + } + + // Check if preventFocusDisruption experiment is enabled + const provider = task.providerRef.deref() + const state = await provider?.getState() + const diagnosticsEnabled = state?.diagnosticsEnabled ?? true + const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS + const isPreventFocusDisruptionEnabled = experiments.isEnabled( + state?.experiments ?? {}, + EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION, + ) + + const sanitizedDiff = sanitizeUnifiedDiff(diff) + const diffStats = computeDiffStats(sanitizedDiff) || undefined + const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) + + const sharedMessageProps: ClineSayTool = { + tool: "appliedDiff", + path: getReadablePath(task.cwd, relPath), + diff: sanitizedDiff, + isOutsideWorkspace, + } + + const completeMessage = JSON.stringify({ + ...sharedMessageProps, + content: sanitizedDiff, + isProtected: isWriteProtected, + diffStats, + } satisfies ClineSayTool) + + // Show diff view if focus disruption prevention is disabled + if (!isPreventFocusDisruptionEnabled) { + await task.diffViewProvider.open(relPath) + await task.diffViewProvider.update(newContent, true) + task.diffViewProvider.scrollToFirstDiff() + } + + const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected) + + if (!didApprove) { + // Revert changes if diff view was shown + if (!isPreventFocusDisruptionEnabled) { + await task.diffViewProvider.revertChanges() + } + pushToolResult("Changes were rejected by the user.") + await task.diffViewProvider.reset() + return + } + + // Save the changes + if (isPreventFocusDisruptionEnabled) { + // Direct file write without diff view or opening the file + await task.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs) + } else { + // Call saveChanges to update the DiffViewProvider properties + await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) + } + + // Track file edit operation + if (relPath) { + await task.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource) + } + + task.didEditFile = true + + // Get the formatted response message + const message = await task.diffViewProvider.pushToolWriteResult(task, task.cwd, false) + pushToolResult(message) + + // Record successful tool usage and cleanup + task.recordToolUsage("search_replace") + await task.diffViewProvider.reset() + + // Process any queued messages after file edit completes + task.processQueuedMessages() + } catch (error) { + await handleError("search and replace", error as Error) + await task.diffViewProvider.reset() + } + } + + override async handlePartial(task: Task, block: ToolUse<"search_replace">): Promise { + const filePath: string | undefined = block.params.file_path + const oldString: string | undefined = block.params.old_string + + let operationPreview: string | undefined + if (oldString) { + // Show a preview of what will be replaced + const preview = oldString.length > 50 ? oldString.substring(0, 50) + "..." : oldString + operationPreview = `replacing: "${preview}"` + } + + // Determine relative path for display + let relPath = filePath || "" + if (filePath && path.isAbsolute(filePath)) { + relPath = path.relative(task.cwd, filePath) + } + + const absolutePath = relPath ? path.resolve(task.cwd, relPath) : "" + const isOutsideWorkspace = absolutePath ? isPathOutsideWorkspace(absolutePath) : false + + const sharedMessageProps: ClineSayTool = { + tool: "appliedDiff", + path: getReadablePath(task.cwd, relPath), + diff: operationPreview, + isOutsideWorkspace, + } + + await task.ask("tool", JSON.stringify(sharedMessageProps), block.partial).catch(() => {}) + } +} + +export const searchReplaceTool = new SearchReplaceTool() diff --git a/src/core/tools/__tests__/searchReplaceTool.spec.ts b/src/core/tools/__tests__/searchReplaceTool.spec.ts new file mode 100644 index 00000000000..b9b1e453d4a --- /dev/null +++ b/src/core/tools/__tests__/searchReplaceTool.spec.ts @@ -0,0 +1,382 @@ +import * as path from "path" +import fs from "fs/promises" + +import type { MockedFunction } from "vitest" + +import { fileExistsAtPath } from "../../../utils/fs" +import { isPathOutsideWorkspace } from "../../../utils/pathUtils" +import { getReadablePath } from "../../../utils/path" +import { ToolUse, ToolResponse } from "../../../shared/tools" +import { searchReplaceTool } from "../SearchReplaceTool" + +vi.mock("fs/promises", () => ({ + default: { + readFile: vi.fn().mockResolvedValue(""), + }, +})) + +vi.mock("path", async () => { + const originalPath = await vi.importActual("path") + return { + ...originalPath, + resolve: vi.fn().mockImplementation((...args) => { + const separator = process.platform === "win32" ? "\\" : "/" + return args.join(separator) + }), + isAbsolute: vi.fn().mockReturnValue(false), + relative: vi.fn().mockImplementation((from, to) => to), + } +}) + +vi.mock("delay", () => ({ + default: vi.fn(), +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(true), +})) + +vi.mock("../../prompts/responses", () => ({ + formatResponse: { + toolError: vi.fn((msg) => `Error: ${msg}`), + rooIgnoreError: vi.fn((path) => `Access denied: ${path}`), + createPrettyPatch: vi.fn(() => "mock-diff"), + }, +})) + +vi.mock("../../../utils/pathUtils", () => ({ + isPathOutsideWorkspace: vi.fn().mockReturnValue(false), +})) + +vi.mock("../../../utils/path", () => ({ + getReadablePath: vi.fn().mockReturnValue("test/path.txt"), +})) + +vi.mock("../../diff/stats", () => ({ + sanitizeUnifiedDiff: vi.fn((diff) => diff), + computeDiffStats: vi.fn(() => ({ additions: 1, deletions: 1 })), +})) + +vi.mock("vscode", () => ({ + window: { + showWarningMessage: vi.fn().mockResolvedValue(undefined), + }, + env: { + openExternal: vi.fn(), + }, + Uri: { + parse: vi.fn(), + }, +})) + +describe("searchReplaceTool", () => { + // Test data + const testFilePath = "test/file.txt" + const absoluteFilePath = process.platform === "win32" ? "C:\\test\\file.txt" : "/test/file.txt" + const testFileContent = "Line 1\nLine 2\nLine 3\nLine 4" + const testOldString = "Line 2" + const testNewString = "Modified Line 2" + + // Mocked functions + const mockedFileExistsAtPath = fileExistsAtPath as MockedFunction + const mockedFsReadFile = fs.readFile as unknown as MockedFunction< + (path: string, encoding: string) => Promise + > + const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction + const mockedGetReadablePath = getReadablePath as MockedFunction + const mockedPathResolve = path.resolve as MockedFunction + const mockedPathIsAbsolute = path.isAbsolute as MockedFunction + + const mockCline: any = {} + let mockAskApproval: ReturnType + let mockHandleError: ReturnType + let mockPushToolResult: ReturnType + let mockRemoveClosingTag: ReturnType + let toolResult: ToolResponse | undefined + + beforeEach(() => { + vi.clearAllMocks() + + mockedPathResolve.mockReturnValue(absoluteFilePath) + mockedPathIsAbsolute.mockReturnValue(false) + mockedFileExistsAtPath.mockResolvedValue(true) + mockedFsReadFile.mockResolvedValue(testFileContent) + mockedIsPathOutsideWorkspace.mockReturnValue(false) + mockedGetReadablePath.mockReturnValue("test/path.txt") + + mockCline.cwd = "/" + mockCline.consecutiveMistakeCount = 0 + mockCline.didEditFile = false + mockCline.providerRef = { + deref: vi.fn().mockReturnValue({ + getState: vi.fn().mockResolvedValue({ + diagnosticsEnabled: true, + writeDelayMs: 1000, + experiments: {}, + }), + }), + } + mockCline.rooIgnoreController = { + validateAccess: vi.fn().mockReturnValue(true), + } + mockCline.rooProtectedController = { + isWriteProtected: vi.fn().mockReturnValue(false), + } + mockCline.diffViewProvider = { + editType: undefined, + isEditing: false, + originalContent: "", + open: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + reset: vi.fn().mockResolvedValue(undefined), + revertChanges: vi.fn().mockResolvedValue(undefined), + saveChanges: vi.fn().mockResolvedValue({ + newProblemsMessage: "", + userEdits: null, + finalContent: "final content", + }), + saveDirectly: vi.fn().mockResolvedValue(undefined), + scrollToFirstDiff: vi.fn(), + pushToolWriteResult: vi.fn().mockResolvedValue("Tool result message"), + } + mockCline.fileContextTracker = { + trackFileContext: vi.fn().mockResolvedValue(undefined), + } + mockCline.say = vi.fn().mockResolvedValue(undefined) + mockCline.ask = vi.fn().mockResolvedValue(undefined) + mockCline.recordToolError = vi.fn() + mockCline.recordToolUsage = vi.fn() + mockCline.processQueuedMessages = vi.fn() + mockCline.sayAndCreateMissingParamError = vi.fn().mockResolvedValue("Missing param error") + + mockAskApproval = vi.fn().mockResolvedValue(true) + mockHandleError = vi.fn().mockResolvedValue(undefined) + mockRemoveClosingTag = vi.fn((tag, content) => content) + + toolResult = undefined + }) + + /** + * Helper function to execute the search replace tool with different parameters + */ + async function executeSearchReplaceTool( + params: Partial = {}, + options: { + fileExists?: boolean + fileContent?: string + isPartial?: boolean + accessAllowed?: boolean + } = {}, + ): Promise { + const fileExists = options.fileExists ?? true + const fileContent = options.fileContent ?? testFileContent + const isPartial = options.isPartial ?? false + const accessAllowed = options.accessAllowed ?? true + + mockedFileExistsAtPath.mockResolvedValue(fileExists) + mockedFsReadFile.mockResolvedValue(fileContent) + mockCline.rooIgnoreController.validateAccess.mockReturnValue(accessAllowed) + + const toolUse: ToolUse = { + type: "tool_use", + name: "search_replace", + params: { + file_path: testFilePath, + old_string: testOldString, + new_string: testNewString, + ...params, + }, + partial: isPartial, + } + + mockPushToolResult = vi.fn((result: ToolResponse) => { + toolResult = result + }) + + await searchReplaceTool.handle(mockCline, toolUse as ToolUse<"search_replace">, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + removeClosingTag: mockRemoveClosingTag, + toolProtocol: "native", + }) + + return toolResult + } + + describe("parameter validation", () => { + it("returns error when file_path is missing", async () => { + const result = await executeSearchReplaceTool({ file_path: undefined }) + + expect(result).toBe("Missing param error") + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("search_replace") + }) + + it("returns error when old_string is missing", async () => { + const result = await executeSearchReplaceTool({ old_string: undefined }) + + expect(result).toBe("Missing param error") + expect(mockCline.consecutiveMistakeCount).toBe(1) + }) + + it("allows empty new_string for deletion", async () => { + // Empty new_string is valid - it means delete the old_string + await executeSearchReplaceTool( + { old_string: "Line 2", new_string: "" }, + { fileContent: "Line 1\nLine 2\nLine 3" }, + ) + + // Should proceed to approval (not error out) + expect(mockAskApproval).toHaveBeenCalled() + }) + + it("returns error when old_string equals new_string", async () => { + const result = await executeSearchReplaceTool({ + old_string: "same", + new_string: "same", + }) + + expect(result).toContain("Error:") + expect(mockCline.consecutiveMistakeCount).toBe(1) + }) + }) + + describe("file access", () => { + it("returns error when file does not exist", async () => { + const result = await executeSearchReplaceTool({}, { fileExists: false }) + + expect(result).toContain("Error:") + expect(result).toContain("File not found") + expect(mockCline.consecutiveMistakeCount).toBe(1) + }) + + it("returns error when access is denied", async () => { + const result = await executeSearchReplaceTool({}, { accessAllowed: false }) + + expect(result).toContain("Access denied") + }) + }) + + describe("search and replace logic", () => { + it("returns error when no match is found", async () => { + const result = await executeSearchReplaceTool( + { old_string: "NonExistent" }, + { fileContent: "Line 1\nLine 2\nLine 3" }, + ) + + expect(result).toContain("Error:") + expect(result).toContain("No match found") + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("search_replace", "no_match") + }) + + it("returns error when multiple matches are found", async () => { + const result = await executeSearchReplaceTool( + { old_string: "Line" }, + { fileContent: "Line 1\nLine 2\nLine 3" }, + ) + + expect(result).toContain("Error:") + expect(result).toContain("3 matches") + expect(mockCline.consecutiveMistakeCount).toBe(1) + expect(mockCline.recordToolError).toHaveBeenCalledWith("search_replace", "multiple_matches") + }) + + it("successfully replaces single unique match", async () => { + await executeSearchReplaceTool( + { + old_string: "Line 2", + new_string: "Modified Line 2", + }, + { fileContent: "Line 1\nLine 2\nLine 3" }, + ) + + expect(mockCline.consecutiveMistakeCount).toBe(0) + expect(mockCline.diffViewProvider.editType).toBe("modify") + expect(mockAskApproval).toHaveBeenCalled() + }) + }) + + describe("approval workflow", () => { + it("saves changes when user approves", async () => { + mockAskApproval.mockResolvedValue(true) + + await executeSearchReplaceTool() + + expect(mockCline.diffViewProvider.saveChanges).toHaveBeenCalled() + expect(mockCline.didEditFile).toBe(true) + expect(mockCline.recordToolUsage).toHaveBeenCalledWith("search_replace") + }) + + it("reverts changes when user rejects", async () => { + mockAskApproval.mockResolvedValue(false) + + const result = await executeSearchReplaceTool() + + expect(mockCline.diffViewProvider.revertChanges).toHaveBeenCalled() + expect(mockCline.diffViewProvider.saveChanges).not.toHaveBeenCalled() + expect(result).toContain("rejected") + }) + }) + + describe("partial block handling", () => { + it("handles partial block without errors", async () => { + await executeSearchReplaceTool({}, { isPartial: true }) + + expect(mockCline.ask).toHaveBeenCalled() + }) + }) + + describe("error handling", () => { + it("handles file read errors gracefully", async () => { + // Set up the rejection BEFORE executing + mockedFsReadFile.mockRejectedValueOnce(new Error("Read failed")) + + const toolUse: ToolUse = { + type: "tool_use", + name: "search_replace", + params: { + file_path: testFilePath, + old_string: testOldString, + new_string: testNewString, + }, + partial: false, + } + + let capturedResult: ToolResponse | undefined + const localPushToolResult = vi.fn((result: ToolResponse) => { + capturedResult = result + }) + + await searchReplaceTool.handle(mockCline, toolUse as ToolUse<"search_replace">, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: localPushToolResult, + removeClosingTag: mockRemoveClosingTag, + toolProtocol: "native", + }) + + expect(capturedResult).toContain("Error:") + expect(capturedResult).toContain("Failed to read file") + expect(mockCline.consecutiveMistakeCount).toBe(1) + }) + + it("handles general errors and resets diff view", async () => { + mockCline.diffViewProvider.open.mockRejectedValueOnce(new Error("General error")) + + await executeSearchReplaceTool() + + expect(mockHandleError).toHaveBeenCalledWith("search and replace", expect.any(Error)) + expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() + }) + }) + + describe("file tracking", () => { + it("tracks file context after successful edit", async () => { + await executeSearchReplaceTool() + + expect(mockCline.fileContextTracker.trackFileContext).toHaveBeenCalledWith(testFilePath, "roo_edited") + }) + }) +}) diff --git a/src/shared/tools.ts b/src/shared/tools.ts index 0961c829aa4..d9561428f89 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -72,6 +72,9 @@ export const toolParamNames = [ "files", // Native protocol parameter for read_file "operations", // search_and_replace parameter for multiple operations "patch", // apply_patch parameter + "file_path", // search_replace parameter + "old_string", // search_replace parameter + "new_string", // search_replace parameter ] as const export type ToolParamName = (typeof toolParamNames)[number] @@ -89,6 +92,7 @@ export type NativeToolArgs = { execute_command: { command: string; cwd?: string } apply_diff: { path: string; diff: string } search_and_replace: { path: string; operations: Array<{ search: string; replace: string }> } + search_replace: { file_path: string; old_string: string; new_string: string } apply_patch: { patch: string } ask_followup_question: { question: string @@ -244,6 +248,7 @@ export const TOOL_DISPLAY_NAMES: Record = { write_to_file: "write files", apply_diff: "apply changes", search_and_replace: "apply changes using search and replace", + search_replace: "apply single search and replace", apply_patch: "apply patches using codex format", search_files: "search files", list_files: "list files", @@ -275,7 +280,7 @@ export const TOOL_GROUPS: Record = { }, edit: { tools: ["apply_diff", "write_to_file", "generate_image"], - customTools: ["search_and_replace", "apply_patch"], + customTools: ["search_and_replace", "search_replace", "apply_patch"], }, browser: { tools: ["browser_action"],