* fix: Include nativeArgs in tool repetition detection
Fixes false positive 'stuck in a loop' error for native protocol tools
like read_file that store parameters in nativeArgs instead of params.
Previously, the ToolRepetitionDetector only compared the params object,
which was empty for native protocol tools. This caused all read_file
calls to appear identical, triggering false loop detection even when
reading different files.
Changes:
- Updated serializeToolUse() to include nativeArgs in comparison
- Added comprehensive tests for native protocol scenarios
- Maintains backward compatibility with XML protocol tools
Closes: Issue reported in Discord about read_file loop detection
* Try to use safe-stable-stringify in the tool repetition detector
---------
Co-authored-by: Matt Rubens <mrubens@users.noreply.github.com>
* fix: format tool responses for native protocol
- Add toolResultFormatting utilities for protocol detection
- ReadFileTool now builds both XML and native formats
- Native format returns clean, readable text without XML tags
- Legacy conversation history conversion is protocol-aware
- All tests passing (55 total)
* refactor: use isNativeProtocol from @roo-code/types
Remove duplicate implementation and import from types package instead
* refactor: centralize toolProtocol configuration checks
- Created src/utils/toolProtocol.ts with getToolProtocolFromSettings() utility
- Replaced all direct vscode.workspace.getConfiguration() calls with centralized utility
- Updated 6 files to use the new utility function
- All tests pass and TypeScript compilation succeeds
* refactor: use isNativeProtocol function from types package
When read_file encountered errors (e.g., file not found), it would call
handleError() which internally calls pushToolResult(), then continue to
call pushToolResult() again with the final XML. In native protocol mode,
this created two tool_result blocks with the same tool_call_id, causing
400 errors on subsequent API calls.
This fix replaces handleError() with task.say() for error notifications.
The agent still receives error details through the XML in the single
final pushToolResult() call.
This change works for both protocols:
- Native: Only one tool_result per tool_call_id (fixes duplicate issue)
- XML: Only one text block with complete XML (cleaner than before)
Agent visibility preserved: Errors are included in the XML response
sent to the agent via pushToolResult().
Tests: All 44 tests passing. Updated test to verify say() is called.
- Updated regex pattern to support optional dash prefix (e.g., "- [ ] Task")
- Added comprehensive test coverage for both formats
- Fixes issue where todo lists with dash prefixes were not being parsed correctly
Fixes#8054
Co-authored-by: Roo Code <roomote@roocode.com>
Fixes issue where isWriteProtected was incorrectly passed as the partial flag
to task.ask(), causing "Current ask promise was ignored (#1)" error.
Both tools now use the askApproval wrapper function which correctly handles
the parameter ordering, passing undefined for progressStatus and isWriteProtected
as the fourth parameter.
Fixes#7648
Co-authored-by: Roo Code <roomote@roocode.com>
* feat: optimize memory usage for image handling in webview
- Replace base64 image data with webview URIs to reduce memory footprint
- Add proper resource roots to webview for workspace file access
- Implement convertToWebviewUri method for safe file-to-URI conversion
- Update ImageViewer to handle both webview URIs and file paths separately
- Add image message type for proper image rendering in chat
- Improve error handling and display for failed image loads
- Add comprehensive tests for ImageViewer component
- Format display paths as relative for better readability
This change significantly reduces memory usage by avoiding base64 encoding
of images and instead using VSCode's webview URI system for direct file
access. Images are now loaded on-demand from disk rather than being held
in memory as base64 strings.
* fix: address PR review comments
- Use safeJsonParse instead of JSON.parse in ChatRow.tsx
- Add type definition for parsed image info
- Add more specific error types in ClineProvider.ts
- Add comprehensive JSDoc comments to ImageBlock.tsx
- Improve error handling and type safety
* fix: address MrUbens' review comments
- Remove hardcoded 'rc1' pattern in formatDisplayPath, use generic workspace detection
- Internationalize 'No image data' text using i18n system
* chore: remove useless comment
* chore(i18n): add image.noData to all locales to fix translation check
* test: update ImageViewer.spec to align with i18n key and flexible path formatting
- Modified ToolRepetitionDetector to skip repetition detection for browser_action scroll_down and scroll_up actions
- Added isBrowserScrollAction() helper method to identify scroll actions
- Added comprehensive tests for the new behavior
- Fixes issue where multiple scroll actions were incorrectly flagged as being stuck in a loop
Resolves: https://github.com/RooCodeInc/Roo-Code/discussions/7470
Co-authored-by: Roo Code <roomote@roocode.com>
* fix: change default showRooIgnoredFiles to false to hide ignored files
- Changed default value from true to false across all files
- Updated tests to reflect the new default behavior
- This prevents ignored files from appearing in environment details
Fixes#7368
* fix: update tests to match new showRooIgnoredFiles default
* fix: update test expectation to match new showRooIgnoredFiles default value
The PR changed the default value of showRooIgnoredFiles from true to false,
so the test needs to expect false instead of true when calling formatFilesList.
---------
Co-authored-by: Roo Code <roomote@roocode.com>
Co-authored-by: daniel-lxs <ricciodaniel98@gmail.com>
* fix: resolve newTaskRequireTodos setting not working correctly
- Use dynamic Package.name instead of hardcoded namespace values
- Show todos parameter as optional/required based on setting value
- Remove hardcoded new_task example from shared tool use section
- Update tests to use Package.name pattern
The setting now works correctly for both regular and nightly builds
without requiring hardcoded namespace values.
* test: update snapshots after removing hardcoded new_task example
The snapshots needed updating because the hardcoded new_task example
was removed from the shared tool use section
* fix: remove namespace prefix from newTaskRequireTodos setting
The setting name should not include the namespace prefix in package.json
as VSCode automatically adds the extension's namespace. This was preventing
the setting from appearing in the VSCode settings UI.
* fix: restore prefixed setting id roo-cline.newTaskRequireTodos for Settings UI visibility
* docs(newTaskTool): explain dynamic Package.name config namespace\n\ntest(newTaskTool): verify config uses Package.name variant (roo-code-nightly)
* refactor: simplify new-task prompt generation
- Replace complex template literals with two complete prompt constants
- Remove nested ternary operators for better readability
- Hide todos parameter completely when disabled (not shown as optional)
- Update tests to reflect new behavior
- Reduce code from 105 to 66 lines for better maintainability
* test: update snapshots after removing optional todos parameter
The todos parameter is now conditionally required based on the newTaskRequireTodos setting, so the snapshots needed to be updated to reflect the new tool documentation format.
---------
Co-authored-by: daniel-lxs <ricciodaniel98@gmail.com>
* feat: add optional todos parameter to new_task tool with experimental setting (#6329)
- Add optional todos parameter to new_task tool for hierarchical task planning
- Implement experimental setting to optionally require todos parameter
- Add clean state-based UI rendering to avoid spurious messages
- Export and reuse parseMarkdownChecklist function
- Add comprehensive test coverage for both optional and required modes
- Maintain full backward compatibility (todos optional by default)
* fix: update new_task tool example to include todos parameter
- Updated the example in tool-use.ts to show the todos parameter
- This prevents AI confusion about whether todos is a valid parameter
- The example now demonstrates the complete tool usage pattern
* fix: make new_task tool definition dynamic based on experimental setting
- Tool description now changes based on newTaskRequireTodos setting
- When disabled: shows todos as (optional)
- When enabled: shows todos as (required) with no mention of configuration
- Added tests to verify dynamic behavior
- Ensures AI models get unambiguous instructions based on current settings
* fix: add translations for newTaskRequireTodos experimental setting
- Added translations for all 17 supported languages
- Ensures consistent UI experience across all locales
* test: update snapshots for new_task tool example with todos parameter
- Updated 13 snapshot files to reflect the new tool-use example
- All tests now pass with the updated example format
* Update src/core/tools/newTaskTool.ts
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix: address PR review comments
- Replace any[] with TodoItem[] type in ExtensionStateContext.tsx for better type safety
- Remove redundant initialTodos parameter from startTask call in Task.ts (todos already set in constructor)
- Improve code clarity in newTaskTool.ts by checking provider reference early and reusing state
# Conflicts:
# src/core/task/Task.ts
# webview-ui/src/context/ExtensionStateContext.tsx
* fix: revert order of operations in startTask to fix integration test timeout
The change in order of operations (calling say() before postStateToWebview()) was causing the XML content file test to timeout. Reverting to the original order fixes the issue.
* fix: hide todos parameter from new_task tool prompt when experiment is disabled
- Modified getNewTaskDescription to completely omit todos parameter when experiment is off
- Updated tests to verify todos parameter is not shown at all when disabled
- Ensures tool prompt remains unchanged when experimental setting is disabled
- Maintains backward compatibility while providing cleaner prompt interface
* fix: update snapshots for new_task tool todos parameter
- Updated snapshots in add-custom-instructions.spec.ts
- Updated snapshots in system-prompt.spec.ts
- All tests now passing with the new todos parameter documentation
* feat: move newTaskRequireTodos from experimental to VSCode settings
- Added newTaskRequireTodos as a VSCode configuration property in src/package.json
- Added description in src/package.nls.json
- Updated newTaskTool.ts to read from VSCode configuration instead of experiments
- Removed NEW_TASK_REQUIRE_TODOS from experimental settings in src/shared/experiments.ts
- Removed newTaskRequireTodos from packages/types/src/experiment.ts
- Updated tests to use VSCode configuration mocking instead of experiments
- Removed references from experiments test file
- Maintains backward compatibility (defaults to false)
* fix: make new_task tool description dynamically reflect VSCode setting
- Updated new-task.ts to check args.settings instead of args.experiments
- Added newTaskRequireTodos to SystemPromptSettings interface
- Pass newTaskRequireTodos setting through Task.ts and generateSystemPrompt.ts
- Updated all related tests to use settings instead of experiments
- Fixed TypeScript errors in test files by adding newTaskRequireTodos property
This ensures the tool description correctly shows todos parameter as required/optional
based on the VSCode setting value, fixing the issue where Roo would try to use
new_task without the todos parameter when it was required.
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Merge Resolver <merge-resolver@roo-code.com>
Co-authored-by: Roo Code <roomote@roocode.com>
* fix: add type check before calling .match() on diffItem.content
Fixes#6905 - Error during diff application "v.content.match is not a function"
- Added type check to ensure diffItem.content is a string before calling .match()
- Added comprehensive tests for handling non-string content values
- Prevents runtime errors when content is null, undefined, or other non-string types
* fix: validate content type earlier in diff parsing
- Move type check to where content is first extracted from XML
- Remove redundant check since content is now guaranteed to be string
- Add test for early content validation
* chore: remove comments
* fix: add TelemetryService mock to multiApplyDiffTool tests
* fix: add captureDiffApplicationError to TelemetryService mock
---------
Co-authored-by: Roo Code <roomote@roocode.com>
Co-authored-by: daniel-lxs <ricciodaniel98@gmail.com>
Co-authored-by: Daniel <57051444+daniel-lxs@users.noreply.github.com>
