Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Jan 20, 2026

Closes #10848

Removes all legacy XML-based tool calling support with zero fallback.

  • Deletes XML protocol/types/parsers/formatters and XML-only tests/fixtures
  • Removes toolProtocol config plumbing and UI settings
  • Makes tool invocation native-only; fails fast if XML tool markup is encountered
  • Updates assistant message rendering to reject XML tool tags and invalid legacy tool blocks
  • No backwards compatibility needed as we made the switch about a month ago and are confident there are no straggler tasks that need the xml logic.

Validation:

  • turbo lint
  • turbo check-types
  • turbo test

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jan 20, 2026
@roomote
Copy link
Contributor

roomote bot commented Jan 20, 2026

Oroocle Clock   Follow along on Roo Cloud

Re-reviewed latest changes. 1 net new issue flagged; the existing Bedrock item is still unresolved.

  • Bedrock: native-only message conversion can emit toolUse/toolResult blocks even when no toolConfig is being sent (may error)
  • Providers: avoid always sending parallel_tool_calls: false; only include when explicitly set (can change wire semantics)
Previous reviews

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

@ellipsis-dev
Copy link
Contributor

ellipsis-dev bot commented Jan 20, 2026

⚠️ This PR is too big for Ellipsis, but support for larger PRs is coming soon. If you want us to prioritize this feature, let us know at help@ellipsis.dev


Generated with ❤️ by ellipsis.dev

…allParser

- Add list_files case to parseToolCall() switch with path/recursive args
- Add new_task case to parseToolCall() switch with mode/message/todos args
- Add list_files to NativeToolArgs type in tools.ts
- Both tools were in toolNames but lacked case handlers, causing the
  fail-fast check on line 852 to throw 'Invalid arguments for tool'
- Remove redundant pushToolResult after handleError in BaseTool.ts
  (handleError already emits tool_result via formatResponse.toolError)
- Fix containsXmlToolMarkup to exclude content inside markdown code
  fences, preventing false positives when users paste docs/examples
- Make 'wait for confirmation' guideline conditional on
  !isMultipleNativeToolCallsEnabled to avoid contradicting
  multiple-tools-per-message guidance
- Update tests to reflect intentional behavior changes
- Remove useNativeTools variable and inline metadata?.tools?.length checks
- Updated: lm-studio, lite-llm, unbound, cerebras, xai, native-ollama, qwen-code, deepinfra, vscode-lm, requesty, bedrock
- Remove dead extractToolSpecsFromMessages() method from bedrock
- Tools are always provided in Roo Code, no need for historical tool detection
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jan 20, 2026
…I providers

After PR #10841, tools are ALWAYS present (minimum 6 from ALWAYS_AVAILABLE_TOOLS)
and tool_choice is always set to 'auto' when tools are present. This eliminates
the need for defensive conditional checks.

Changes:
- Simplified 27 API provider files to use direct property assignments
- Updated 14 test files to reflect new behavior where tools are always present
- Changed patterns from conditional spreads to direct assignments:
  Before: ...(metadata?.tools?.length ? { tools: ... } : {})
  After: tools: convertTools(metadata?.tools ?? [])

All 859 provider tests pass.
}),
tools: this.convertToolsForOpenAI(metadata?.tools),
tool_choice: metadata?.tool_choice,
parallel_tool_calls: metadata?.parallelToolCalls ?? false,
Copy link
Contributor

Choose a reason for hiding this comment

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

A bunch of providers now always send parallel_tool_calls: false when metadata.parallelToolCalls is unset. This is a behavior change (and a few earlier threads explicitly preferred omitting the field unless requested), so it can break backends that treat presence of the field differently from absence. Consider only including parallel_tool_calls when metadata.parallelToolCalls !== undefined (like the earlier DeepSeek tweak) to avoid changing default wire semantics.

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

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

Status: PR [Needs Review]

Development

Successfully merging this pull request may close these issues.

Remove legacy XML tool calling support

4 participants