Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Dec 4, 2025

Summary

When using native tool protocol, if a model attempts to call a tool that doesn't exist, the extension would freeze in a loop instead of returning an error to the model.

Root Causes

  1. Partial block loop: During streaming, partial blocks of unknown tools would reach the default case in the switch statement, showing an error on every streaming chunk, creating an infinite loop
  2. Validation timing: validateToolUse was being called for partial blocks, causing repeated validation errors during streaming

Changes

src/core/tools/validateToolUse.ts

  • Add isValidToolName() function to explicitly check if a tool name is valid (either a known static tool or a dynamic MCP tool)
  • Validation now throws a specific "Unknown tool" error before checking mode permissions

src/core/assistant-message/presentAssistantMessage.ts

  • Wrap validateToolUse in if (!block.partial) - validation only runs for complete tool blocks, preventing repeated errors during streaming
  • Add if (block.partial) break in the default case to skip partial blocks of unknown tools

Testing

  • All 4494 existing tests pass
  • Added tests for unknown tool validation in validateToolUse.spec.ts

How to Test

  1. Configure a model that uses native tool protocol (e.g., Claude via Anthropic API)
  2. Ask the model to use a tool that doesn't exist (e.g., "use edit_file tool")
  3. Before: Extension would freeze showing error repeatedly
  4. After: Extension shows single error and sends tool_result back to model, allowing it to correct itself

…eeze

When a model called a tool that doesn't exist, the switch statement in
presentAssistantMessage had no default case to handle it. For native
protocol, this meant no tool_result was sent back to the API, causing
the extension to freeze waiting indefinitely.

Added:
- Default case in tool switch statement that:
  - Pushes an error tool_result back to the API
  - Increments consecutiveMistakeCount
  - Records the tool error
  - Shows error message to user
- Comprehensive test suite for unknown tool handling
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners December 4, 2025 18:01
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Dec 4, 2025
@roomote
Copy link
Contributor

roomote bot commented Dec 4, 2025

Rooviewer Clock   See task on Roo Cloud

Review Complete ✅

Re-reviewed commit 0292351 and found no new issues. The changes successfully internationalize the unknown tool error message as requested in the previous review.

Previous reviews

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

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Dec 4, 2025
daniel-lxs and others added 2 commits December 4, 2025 14:39
When using native tool protocol, if a model attempts to call a tool that doesn't exist (like 'edit_file'), the extension would freeze in a loop instead of returning an error to the model.

Root causes:
1. During streaming, partial blocks of unknown tools would reach the default case in the switch statement, showing an error on every streaming chunk
2. validateToolUse was being called for partial blocks, causing repeated validation errors

Changes:
- Add isValidToolName() function to validateToolUse.ts to explicitly check for unknown tools
- Only run tool validation for complete (non-partial) blocks in presentAssistantMessage
- Skip partial blocks in the default case to prevent error loops during streaming
- Fetch state early in tool_use case so mode/customModes are available throughout

This ensures that when an invalid tool is called, the extension waits for the complete tool call before showing a single error and sending a tool_result back to the model.
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 4, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Dec 4, 2025
@hannesrudolph hannesrudolph added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Dec 4, 2025
@mrubens mrubens merged commit 9f111e1 into main Dec 5, 2025
13 checks passed
@mrubens mrubens deleted the fix/native-tool-unknown-tool-freeze branch December 5, 2025 03:21
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Dec 5, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants