Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Feb 7, 2026

Summary

Renames the search_and_replace tool to edit with a new flat parameter structure (file_path, old_string, new_string, replace_all), replacing the old batch operations array. Keeps search_and_replace as a backward-compatible alias so models like MiniMax that reference it in includedTools continue to work without changes.

Changes

  • New edit tool definition (src/core/prompts/tools/native-tools/edit.ts) with file_path, old_string, new_string, replace_all parameters
  • New EditTool execution handler (src/core/tools/EditTool.ts) with replace_all support and uniqueness checking
  • Alias system: Added search_and_replace: "edit" to TOOL_ALIASES — models with includedTools: ["search_and_replace"] (e.g., MiniMax) get the edit tool renamed as search_and_replace
  • Parser updates: NativeToolCallParser.ts handles both "edit" and "search_and_replace" tool names with the new parameter structure
  • Execution routing: presentAssistantMessage.ts routes both tool names to EditTool
  • Alias resolution in validation: isToolAllowedForMode now resolves aliases when checking customTools, ensuring search_and_replace in includedTools correctly maps to edit
  • Backward compat wrapper: SearchAndReplaceTool.ts reduced to a 2-line re-export from EditTool
  • Types: Added "edit" to toolNames, "replace_all" to toolParamNames, updated NativeToolArgs
  • Tests: 16 new EditTool tests covering basic replacement, replace_all, uniqueness errors, no-match errors, approval flow

Test Results

  • 7 targeted test files: 105/105 passing
  • Full src/ suite: 341 passed (29 pre-existing failures from missing @ai-sdk/amazon-bedrock — unrelated)
  • packages/types: 170/170 passing

Important

Renames search_and_replace tool to edit, updates related components and tests, and maintains backward compatibility with an alias.

  • Behavior:
    • Renames search_and_replace tool to edit with new parameters (file_path, old_string, new_string, replace_all).
    • Keeps search_and_replace as an alias for backward compatibility.
  • Tool Implementation:
    • Adds EditTool in EditTool.ts with support for replace_all and uniqueness checking.
    • Removes search_and_replace.ts, re-exporting EditTool as SearchAndReplaceTool for compatibility.
  • UI and Parsing:
    • Updates NativeToolCallParser.ts and presentAssistantMessage.ts to handle both tool names.
    • Unifies chat UI for edit-family tools in ChatRow.tsx.
  • Testing:
    • Adds tests for EditTool in editTool.spec.ts.
    • Updates existing tests to reflect tool renaming and aliasing.
  • Miscellaneous:
    • Updates TOOL_ALIASES in tools.ts to map search_and_replace to edit.
    • Adjusts validateToolUse.ts to resolve tool aliases during validation.

This description was created by Ellipsis for e1a149c. You can customize this summary. It will automatically update as commits are pushed.


Additional updates in latest commits

  • Unified chat UI treatment for edit-family tool events so searchAndReplace, search_and_replace, search_replace, edit, edit_file, apply_patch, and related edit-result events render through the same appliedDiff branch.
  • Fixed apply_patch partial preview payloads to emit a deterministic non-empty path, eliminating blank initial approval rows and aligning early UX with apply_diff.
  • Added regression tests for apply_patch partial path extraction and unified chat diff-actions rendering.

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. Enhancement New feature or request labels Feb 7, 2026
@roomote
Copy link
Contributor

roomote bot commented Feb 7, 2026

Rooviewer Clock   Follow task

No new issues in latest commit. One pre-existing low-severity finding remains open.

  • Delete orphaned src/core/prompts/tools/native-tools/search_and_replace.ts (old parameter schema, no longer imported)
  • containsXmlToolMarkup false-positive risk: "edit" in the tool-name list matches <editor>, <editable>, etc. after lowercasing -- fixed with hasTagReference boundary checking
  • Unified edit-family branch in ChatRow.tsx drops the onJumpToFile prop that newFileCreated previously passed to CodeAccordian -- fixed with onJumpToCreatedFile memo
  • EditTool.ts: String.prototype.replace() interprets special $-patterns ($$, $&, $`, $') in the replacement string, which can silently corrupt edits containing dollar signs (e.g. JS template literals with $${...})
Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@hannesrudolph hannesrudolph force-pushed the feat/rename-search-and-replace-to-edit branch from 01335c6 to 0a1377c Compare February 8, 2026 00:32
@hannesrudolph hannesrudolph force-pushed the feat/rename-search-and-replace-to-edit branch from 52ad6f3 to 285a161 Compare February 8, 2026 03:44
@hannesrudolph hannesrudolph changed the title feat: rename search_and_replace tool to edit with new parameter structure feat: rename search_and_replace tool to edit and unify edit-family UI Feb 9, 2026
switch (tool.tool as string) {
case "editedExistingFile":
case "appliedDiff":
case "newFileCreated":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low severity: merging newFileCreated into the unified branch drops the onJumpToFile prop that the old dedicated case passed to CodeAccordian. That prop rendered an external-link icon in the accordian header, letting users click to open the newly created file. Without it, CodeAccordian falls back to showing an expand/collapse chevron instead. The other edit-family tools didn't have this prop, so it's only a regression for new-file-creation messages.

Fix it with Roo Code or mention @roomote and request a fix.

…ture

- Add new 'edit' tool with file_path, old_string, new_string, replace_all params
- Keep search_and_replace as backward-compatible alias via TOOL_ALIASES
- Create EditTool execution handler with replace_all and uniqueness checking
- Update NativeToolCallParser for both edit and search_and_replace names
- Update presentAssistantMessage routing for both tool names
- Add alias resolution to isToolAllowedForMode for customTools validation
- Reduce SearchAndReplaceTool.ts to re-export wrapper from EditTool
- Add 16 new EditTool tests, update searchAndReplaceTool tests
- MiniMax includedTools: ['search_and_replace'] continues to work via alias
- route edit/search/patch-related tool events through the appliedDiff chat UI branch

- ensure apply_patch partial rows emit a deterministic non-empty path

- add apply_patch partial regression tests

- expand ChatRow diff-actions tests for unified behavior
@hannesrudolph hannesrudolph force-pushed the feat/rename-search-and-replace-to-edit branch from 09a6b8a to 793be28 Compare February 10, 2026 00:24
Comment on lines +135 to +143
let newContent: string
if (replaceAll) {
// Replace all occurrences
const searchPattern = new RegExp(escapeRegExp(normalizedOld), "g")
newContent = fileContent.replace(searchPattern, normalizedNew)
} else {
// Replace single occurrence (already verified uniqueness above)
newContent = fileContent.replace(normalizedOld, normalizedNew)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low severity: both the replaceAll path (line 139) and the single-replace path (line 142) pass normalizedNew directly as the replacement string to String.prototype.replace(). JavaScript interprets special $-patterns in replacement strings: $$ becomes a single $, $& inserts the matched text, $` and $' insert surrounding text. This means edits whose new_string contains these patterns are silently corrupted. The most likely real-world case is $$ in JS template literals (e.g. `$${amount}` loses one $). Using a function replacer ((pattern, () => normalizedNew)) avoids special-pattern interpretation entirely.

Suggested change
let newContent: string
if (replaceAll) {
// Replace all occurrences
const searchPattern = new RegExp(escapeRegExp(normalizedOld), "g")
newContent = fileContent.replace(searchPattern, normalizedNew)
} else {
// Replace single occurrence (already verified uniqueness above)
newContent = fileContent.replace(normalizedOld, normalizedNew)
}
let newContent: string
if (replaceAll) {
// Replace all occurrences
const searchPattern = new RegExp(escapeRegExp(normalizedOld), "g")
newContent = fileContent.replace(searchPattern, () => normalizedNew)
} else {
// Replace single occurrence (already verified uniqueness above)
newContent = fileContent.replace(normalizedOld, () => normalizedNew)
}

Fix it with Roo Code or mention @roomote and request a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant