Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Nov 15, 2025

Overview

This PR implements dynamic tool protocol resolution that properly respects the precedence hierarchy: Support > Preference > Defaults.

Precedence Hierarchy

The tool protocol is now resolved in the following order:

  1. User Preference - Per-Profile (explicit profile toolProtocol setting) - highest priority
  2. User Preference - Global (VSCode setting when set to "native") - user override
  3. Model Default (defaultToolProtocol in ModelInfo) - model-specific preference
  4. Provider Default (configurable list, XML for others) - provider fallback
  5. XML Fallback (final fallback when nothing else is specified)
  6. Support Validation (if native chosen but model doesn't support it, fall back to XML)

Key Changes

Type Definitions

  • Added defaultToolProtocol field to ModelInfo type
  • Added toolProtocol field to ProviderSettings type
  • Created rooModelDefaults configuration object for Roo provider

Core Resolution Logic

  • Created resolveToolProtocol() utility function that centralizes all resolution logic
  • Handles support validation to prevent using native tools with unsupported models
  • Global VSCode setting only applies when explicitly set to "native" (non-default)

Integration Points Updated

  • Task.ts: Constructor, getSystemPrompt, attemptApiRequest, error messages (9 locations)
  • System prompt generation for preview
  • Tool result formatting
  • All tool execution points

Bug Fixes

  • Error messages now show correct tool usage guidelines (XML vs native) based on resolved protocol
  • Roo provider model defaults now properly merged in system prompt preview
  • All test mocks updated to include required apiConfiguration property

Testing

  • 33 comprehensive tests for resolveToolProtocol covering all scenarios
  • All 4,161 tests pass across all test suites
  • Updated snapshots for system prompt tests
  • Fixed test mocks for multiApplyDiffTool, readFileTool, and applyDiffTool.experiment

Configuration Example

Roo Model Defaults

export const rooModelDefaults: Record<string, Partial<ModelInfo>> = {
  "minimax/minimax-m2:free": {
    defaultToolProtocol: "native",
  },
}

Provider Defaults

Currently all providers default to XML (empty list in getProviderDefaultProtocol()). To add native-preferred providers:

function getProviderDefaultProtocol(provider: ProviderName): ToolProtocol {
  const nativeProviders: ProviderName[] = [] // Add providers here
  return nativeProviders.includes(provider) ? TOOL_PROTOCOL.NATIVE : TOOL_PROTOCOL.XML
}

What This Enables

  • Models can specify their preferred tool protocol via defaultToolProtocol
  • Users can override with global VSCode setting (when set to "native")
  • Per-profile settings (future) will take highest priority
  • Proper fallback chain ensures XML is used when native isn't supported
  • Error messages always show correct guidelines for the resolved protocol

Important

Introduces dynamic tool protocol resolution with a precedence hierarchy, updating core logic and tests to ensure correct protocol selection based on user, model, and provider settings.

  • Behavior:
    • Implements resolveToolProtocol() to determine tool protocol based on precedence: User Profile > Global Setting > Model Default > Provider Default > XML Fallback.
    • Ensures XML is used if native protocol is unsupported by the model.
  • Integration:
    • Updates Task.ts, presentAssistantMessage.ts, and generateSystemPrompt.ts to use resolveToolProtocol().
    • Modifies applyDiffTool() and ReadFileTool to handle protocol resolution.
  • Testing:
    • Adds resolveToolProtocol.spec.ts with 33 tests covering all precedence scenarios and edge cases.
    • Updates existing tests in applyDiffTool.experiment.spec.ts and multiApplyDiffTool.spec.ts to reflect new protocol logic.
  • Misc:
    • Adds defaultToolProtocol to ModelInfo and toolProtocol to ProviderSettings types.

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

@daniel-lxs daniel-lxs requested a review from mrubens as a code owner November 15, 2025 18:48
@daniel-lxs daniel-lxs requested review from cte and jr as code owners November 15, 2025 18:48
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. Enhancement New feature or request labels Nov 15, 2025
@roomote
Copy link
Contributor

roomote bot commented Nov 15, 2025

Rooviewer Clock   See task on Roo Cloud

Re-review completed. The previous issue has been resolved.

  • Support validation doesn't properly handle supportsNativeTools: undefined - the check should treat undefined as unsupported
Previous reviews

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

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Nov 15, 2025
MARKDOWN RULES
ALL responses MUST show ANY \`language construct\` OR filename reference as clickable, exactly as [\`filename OR language.declaration()\`](relative/file/path.ext:line); line is required for \`syntax\` and optional for filename links. This applies to ALL markdown responses and ALSO those in <attempt_completion>`
ALL responses MUST show ANY \`language construct\` OR filename reference as clickable, exactly as [\`filename OR language.declaration()\`](relative/file/path.ext:line); line is required for \`syntax\` and optional for filename links. This applies to ALL markdown responses and ALSO those in attempt_completion`
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears there's a typographical error in the inline code formatting in this line. The fragment "in attempt_completion" ends with a backtick that isn't balanced by a corresponding opening backtick. Should it be formatted as "attempt_completion`" or corrected in some other way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you ok buddy?

@roomote
Copy link
Contributor

roomote bot commented Nov 15, 2025

Fixaroo Clock   See task on Roo Cloud

Fixed the support validation issue. All local checks passed.

View commit | Revert commit

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 15, 2025
@mrubens mrubens merged commit 744f4bd into main Nov 15, 2025
13 checks passed
@mrubens mrubens deleted the feat/dynamic-tool-protocol-resolution branch November 15, 2025 19:35
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Nov 15, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Nov 15, 2025
mini2s added a commit to zgsm-ai/costrict that referenced this pull request Nov 17, 2025