mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-10 22:41:14 +00:00
feat: add search_replace native tool for single-replacement operations (#9918)
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)
This commit is contained in:
parent
efbf427631
commit
fba8508b10
8 changed files with 760 additions and 1 deletions
|
|
@ -20,6 +20,7 @@ export const toolNames = [
|
|||
"write_to_file",
|
||||
"apply_diff",
|
||||
"search_and_replace",
|
||||
"search_replace",
|
||||
"apply_patch",
|
||||
"search_files",
|
||||
"list_files",
|
||||
|
|
|
|||
|
|
@ -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<TName>
|
||||
}
|
||||
break
|
||||
|
||||
default:
|
||||
break
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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">, {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
51
src/core/prompts/tools/native-tools/search_replace.ts
Normal file
51
src/core/prompts/tools/native-tools/search_replace.ts
Normal file
|
|
@ -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
|
||||
277
src/core/tools/SearchReplaceTool.ts
Normal file
277
src/core/tools/SearchReplaceTool.ts
Normal file
|
|
@ -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<Record<string, string>>): 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<void> {
|
||||
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<void> {
|
||||
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()
|
||||
382
src/core/tools/__tests__/searchReplaceTool.spec.ts
Normal file
382
src/core/tools/__tests__/searchReplaceTool.spec.ts
Normal file
|
|
@ -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<typeof fileExistsAtPath>
|
||||
const mockedFsReadFile = fs.readFile as unknown as MockedFunction<
|
||||
(path: string, encoding: string) => Promise<string>
|
||||
>
|
||||
const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction<typeof isPathOutsideWorkspace>
|
||||
const mockedGetReadablePath = getReadablePath as MockedFunction<typeof getReadablePath>
|
||||
const mockedPathResolve = path.resolve as MockedFunction<typeof path.resolve>
|
||||
const mockedPathIsAbsolute = path.isAbsolute as MockedFunction<typeof path.isAbsolute>
|
||||
|
||||
const mockCline: any = {}
|
||||
let mockAskApproval: ReturnType<typeof vi.fn>
|
||||
let mockHandleError: ReturnType<typeof vi.fn>
|
||||
let mockPushToolResult: ReturnType<typeof vi.fn>
|
||||
let mockRemoveClosingTag: ReturnType<typeof vi.fn>
|
||||
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<ToolUse["params"]> = {},
|
||||
options: {
|
||||
fileExists?: boolean
|
||||
fileContent?: string
|
||||
isPartial?: boolean
|
||||
accessAllowed?: boolean
|
||||
} = {},
|
||||
): Promise<ToolResponse | undefined> {
|
||||
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")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -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<ToolName, string> = {
|
|||
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<ToolGroup, ToolGroupConfig> = {
|
|||
},
|
||||
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"],
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue