- Remove getToolDescription() method from MultiSearchReplaceDiffStrategy
- Remove getToolDescription() from DiffStrategy interface
- Remove unused ToolDescription type from shared/tools.ts
- Remove unused eslint-disable directive
- Update test mocks to remove getToolDescription references
- Remove getToolDescription tests from multi-search-replace.spec.ts
Native tools are now defined in src/core/prompts/tools/native-tools/ using
the OpenAI function format. The removed code was dead since XML-style tool
calling was replaced with native tool calling.
* chore: remove MULTI_FILE_APPLY_DIFF experiment
Remove the 'Enable concurrent file edits' experimental feature that
allowed editing multiple files in a single apply_diff call.
- Remove multiFileApplyDiff from experiment types and config
- Delete MultiFileSearchReplaceDiffStrategy class and tests
- Delete MultiApplyDiffTool wrapper and tests
- Remove experiment-specific code paths in Task.ts, generateSystemPrompt.ts, and presentAssistantMessage.ts
- Remove special handling in ExperimentalSettings.tsx
- Remove translations from all 18 locale files
The existing MultiSearchReplaceDiffStrategy continues to handle
multiple SEARCH/REPLACE blocks within a single file.
* fix: remove unused EXPERIMENT_IDS/experiments import from Task.ts
Addresses review feedback: removes the unused imports from
src/core/task/Task.ts that were left over after removing the
MULTI_FILE_APPLY_DIFF experiment routing code.
* feat: add BatchDiffApproval component for multi-file diff application
- Introduced a new component `BatchDiffApproval` to handle the approval of batch changes across multiple files.
- Integrated the `BatchDiffApproval` component into `ChatRow` to display batch diff requests.
- Updated experimental settings to include a toggle for multi-file apply diff functionality.
- Enhanced localization files to support new strings related to batch changes in multiple languages.
- Updated tests to cover the new multi-file apply diff feature.
* revert this
* fix: update applyDiff parameter type to accept string or DiffItem
* refactor: keep original file name for apply diff tool
* revert this
* Update src/core/webview/__tests__/ClineProvider.test.ts
* revert this
* fix: keep the original path if the experiment is disabled
* test: add dynamic strategy selection tests for MultiSearchReplaceDiffStrategy and MultiFileSearchReplaceDiffStrategy
* fix: mock applyDiffTool module and ensure legacy tool resolves successfully in tests
* remove this
* ellipsis suggestion
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
* refactor: mirror concurrent file reads
---------
Co-authored-by: Daniel Riccio <ricciodaniel98@gmail.com>
Co-authored-by: Daniel <57051444+daniel-lxs@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Adds a new test case that allows testing the multi-search-replace diff strategy directly from the command line by passing source and diff files as arguments.
This enables easier debugging and manual testing of the diff application functionality without needing to modify test code for each test case.
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
Co-authored-by: Eric Wheeler <roo-code@z.ewheeler.org>
Adds validation to ensure that `:start_line:` and `:end_line:`
markers do not appear in the REPLACE section of an apply_diff
operation. These markers are only valid within the SEARCH section.
This change prevents potential errors and confusion when users
might inadvertently include these markers in the replacement content.
New tests have been added to verify this validation.
Fixes: #4013
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
Co-authored-by: Eric Wheeler <roo-code@z.ewheeler.org>
* Improve tool descriptions to guide proper file editing tool selection
### Problem
LLMs were frequently choosing `write_to_file` for file edits instead of more appropriate tools like `apply_diff`, `insert_content`, or `search_and_replace`. This was likely because the tool descriptions made it seem like files needed to be reconstructed rather than edited.
### Changes Made
**write_to_file tool description:**
- Clarified primary use cases: **creating new files** or **complete rewrites when intentionally required**
- Changed opening from "write full content" to "write content" to reduce emphasis on full reconstruction
- Updated parameter description to be more specific about when complete content is needed
**apply_diff tool description:**
- Enhanced description to emphasize "targeted modifications" and "precise, surgical edits"
- Clarified that multiple search/replace operations can be performed in a single call
- Removed misleading "only a single operation allowed" restriction
- Better positioned as the preferred tool for making several targeted changes efficiently
### Impact
These changes should guide LLMs toward:
- Using `write_to_file` primarily for new files or intentional complete rewrites
- Using `apply_diff` for targeted edits and modifications to existing files
- Better understanding of when each tool is most appropriate
The updated descriptions maintain necessary technical details while providing clearer guidance on tool selection without being overly prescriptive.
* Update src/core/prompts/tools/write-to-file.ts
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
* Update src/core/prompts/tools/write-to-file.ts
Co-authored-by: Matt Rubens <mrubens@users.noreply.github.com>
* Update src/core/prompts/tools/write-to-file.ts
Co-authored-by: Matt Rubens <mrubens@users.noreply.github.com>
---------
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: Matt Rubens <mrubens@users.noreply.github.com>
Add animated dots to progress indicator based on content length
Optimize when progress updates are shown (every 10 characters)
Move searchBlockCount calculation inside conditional blocks
Skip unnecessary ask operations when toolProgressStatus is empty
* refactor: enhance insertGroups and insertContentTool for better handling of insertion operations
* refactor: simplify insert_content tool
- Remove operations-based implementation in favor of single line insertion
- Update parameters from operations to line and content
- Simplify insertion logic and error handling
- Update tool description and documentation
- Remove XML parsing for operations
- Clean up code and improve error messages
* refactor: remove insert_content experiment and related tests
* Remove the append_to_file tool
* Improvements to chat row and instructions
---------
Co-authored-by: Matt Rubens <mrubens@users.noreply.github.com>
* fix: clarify difference between workspace directory and terminal working directory
This commit addresses confusion between the VS Code workspace directory and
terminal working directory. Roo was not properly distinguishing between these
concepts, leading to issues when terminal commands changed directories.
- Renamed 'Current Working Directory' to 'Current Workspace Directory' throughout
- Added clearer notice when a command changes the working directory in a terminal
- Added explanation about the difference between workspace and working directories
- Updated all tool descriptions to reference 'workspace directory'
References: https://www.reddit.com/r/RooCode/s/6L19EvsFbF
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* test: update directory terminology in test files
Update terminology from 'working directory' to 'workspace directory' in tests
to reflect VSCode's concept of workspace vs working directory.
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
---------
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
Co-authored-by: Eric Wheeler <roo-code@z.ewheeler.org>
* refactor: Change read_file result format to XML
Modifies how the result of the `read_file` tool is presented
in the conversation history sent to the AI model.
Previously, the format was:
[read_file for 'path/to/file'] Result:
{content_or_error}
This commit changes the format to use XML tags for better
structure and potentially easier parsing by the model:
<file>
<path>path/to/file</path>
<content>
{content_or_error}
</content>
</file>
This change only affects the `read_file` tool result formatting
within the user context message constructed in `src/core/Cline.ts`.
Other tool result formats remain unchanged.
* fix: Update error message for better clarity in multi-search-replace strategy
Refines the error message returned when no sufficiently similar match is found during the multi-search-replace operation. The message now includes a clearer instruction to use the read_file tool for obtaining the latest file content before attempting to apply the diff again.
* refactor: Update readFileTool to return results in XML format
Modifies the `readFileTool` function to format the output as XML, enhancing the structure of the returned file content. This change aligns with previous updates to ensure consistent result formatting across tools.
* refactor: Simplify result handling for tool responses in Cline
This update refines the handling of tool responses in the `Cline` class by removing the XML formatting for `read_file` results and consolidating the logic for pushing results to the user message content. The changes ensure that all tool results are processed uniformly, improving code clarity and maintainability.
* test: Update read_file tests to validate XML formatted results
This commit modifies the assertions in the `read_file` tool tests to check for the expected XML format in the results. The changes ensure that the output structure aligns with recent updates to the tool's response formatting, enhancing test accuracy and reliability.
* test: Refactor assertions in read_file tests to use expected XML variable
This commit updates the `read_file` tool tests to utilize a predefined variable for the expected XML output, improving readability and maintainability of the test code. The changes ensure consistency in the expected results across multiple test cases.
* fix: Improve handling of escaped markers in apply_diff
unescapeMarkers() is supposed to fix escaped merge conflict markers so
that they match the code. However the function has a bug which requires
SEARCH and REPLACE strings in the markers to be replaced. This is not
part of merge conflict markers, so many valid cases were previously
missed.
* Revert test changes
* Add new test
---------
Co-authored-by: Matt Rubens <mrubens@users.noreply.github.com>
Currently all errors in apply_diff diff structure are assumed to be
related to merge conflicts. This is unfortunate because it is way more
likely that LLM simply made an error structuring the diff. The LLM is
then directed to fix escaping in the diff from which it doesn't recover.
It is safe to assume that missing or extra "<<<<<<< SEARCH" or ">>>>>>>
REPLACE" strings are likely bad diff structure, because while "<<<<<<<"
and ">>>>>>>" are common merge conflict markers, no tools add "SEARCH"
and "REPLACE" strings after them. This is likely output of the LLM
itself.
LLM not recovering has been observed in Claude 3.5 Sonnet and Claude 3.7
Sonnet models.
To address the issue, error message now mentions merge conflict markers
and their escaping only if it's clear that errors may come from this
source. In the rest of cases the error message repeats the expected diff
structure back to the LLM.
The multi-search-replace diff strategy previously did not correctly strip line numbers (`number | `) from the SEARCH block when the REPLACE block was empty. This occurred because the condition for stripping required both blocks to consistently have line numbers.
This prevented successful deletion operations when the SEARCH block content was copied from `read_file` output (which includes line numbers) and the REPLACE block was empty.
This commit updates the line number stripping condition in `applyDiff` to also trigger if the SEARCH block has line numbers and the REPLACE block is empty or contains only whitespace, resolving the bug.
Additionally, a new test case has been added to `multi-search-replace.test.ts` to specifically verify this deletion scenario. All tests now pass with this updated logic.
* fix: remove -p flag from test script to prevent git operation errors
The -p flag in npm-run-all was causing tests to run in parallel, which led to 'Cannot log after tests are done' errors with git operations. These errors don't appear when running test:extension alone.
The issue occurs because git-based tests create temporary directories and run async operations that can interfere with each other when executed in parallel. Running tests sequentially resolves this cleanly.
While it might increase total test time slightly, it ensures more reliable and consistent test results.
* refactor(terminal): improve mock streams and fix test issues
- Create shell-specific mock streams (bash, cmd, pwsh) with proper line ending handling
- Fix open handles in tests by properly managing timeouts
- Standardize stderr redirection across all shell implementations using stdio option
- Improve test reliability and output cleanliness
* fix(tests): add skipVerification option to PowerShell tests to debug Linux issues
* fix(tests): use explicit variable name in PowerShell test to fix Linux compatibility
* Refactor terminal tests to use purpose-based approach instead of command mapping
* Remove reference to non-existent test file
* fix: use printf instead of echo -e for more consistent behavior across platforms
* fix: use single quotes for PowerShell commands to preserve variables on Linux
* Update code-qa workflow to run tests on both Windows and Ubuntu
* fix: use platform-specific PowerShell command execution for Linux and Windows
* Fix toggleToolAlwaysAllow to handle path normalization for cross-platform compatibility
* Fix McpHub tests to handle normalized paths on Windows
* Suppress console.error messages in McpHub tests
* fix: make Bedrock ARN regex patterns Windows-compatible
Fixed an issue where AWS Bedrock tests were timing out on Windows but passing on Linux. The root cause was path separator handling in regex patterns used for model ID extraction from ARNs.
1. Updated model ID extraction regex to handle both forward slashes (Linux) and backslashes (Windows)
2. Modified ARN matching regex to be platform-independent
3. Ensured consistent region prefix handling for all supported regions
This change maintains functionality while ensuring cross-platform compatibility.
* fix: make WorkspaceTracker test cross-platform compatible
Fixed an issue where the WorkspaceTracker test 'should initialize with workspace files' was failing on Windows but passing on Linux. The problem was in the mock implementation of toRelativePath that only handled forward slashes.
- Updated the toRelativePath mock to use path.relative which properly handles platform-specific path separators
- Ensured all paths are converted to forward slashes for consistency in test assertions
- The fix maintains cross-platform compatibility while preserving the test's intent
* fix: make WorkspaceTracker tests cross-platform compatible
Fixed cross-platform compatibility issues in the WorkspaceTracker tests that were causing failures on Windows but passing on Linux:
1. Updated the toRelativePath mock implementation to:
- Use path.relative which properly handles platform-specific path separators
- Convert paths to forward slashes for consistency in test assertions
2. Enhanced the 'should not update file paths' test to be platform-agnostic by:
- Using more flexible assertions that don't depend on specific path formats
- Checking file path length and content rather than exact string matches
- Properly typed the test assertions to fix TypeScript errors
These changes preserve the test intent while ensuring they run successfully across different operating systems.
* fix: make McpHub tests cross-platform compatible
Fixed cross-platform compatibility issues in the McpHub tests that were causing failures on Windows but passing on Linux:
1. Made the toggleToolAlwaysAllow tests more platform-agnostic by:
- No longer relying on specific path formats which differ between Windows and Linux
- Using the last write call instead of searching for a specific path string
- Adding more robust assertions that verify structure instead of exact path matches
- Properly handling array existence checks
2. These tests would fail on Windows because paths are formatted with backslashes instead of
forward slashes, causing path equality checks to fail.
The changes maintain test intent while ensuring cross-platform compatibility.
* handle escaping of slash and quote
* fix: ensure consistent line endings in git fallback strategy
Fixed an issue where tests would fail on GitHub Windows runners but pass on local Windows machines due to line ending differences. The fix ensures consistent line ending handling by:
1. Normalizing CRLF to LF when reading files in the git fallback strategy
2. Disabling Git's automatic line ending conversion
3. Maintaining consistent line ending usage throughout text operations
* feat: run tests sequentially on Windows, parallel otherwise
Add tests that validate:
- Original content with unescaped markers
- Search content with escaped markers to match unescaped markers in original content
- Proper validation of escaped search, separator, and replace markers in diff content
- Successful application of diffs with escaped markers in search content
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
Prevents attempting to apply diffs where search and replace content are identical,
which would result in no changes being made. Instead, provide a helpful error
message explaining why the operation was rejected.
Fixes: #1350
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
