From e0f1b29cfb59671c9c0aff74b06c844f693c8e8d Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 5 Jan 2026 08:13:35 +0000 Subject: [PATCH 1/3] feat: restore tool protocol setting for user override This restores the ability for users to override the tool call protocol in the Advanced Settings. This is useful for OpenAI-compatible models (like Qwen3 and Kimi2) that do not handle native tool calling well. Changes: - Updated resolveToolProtocol.ts to respect user toolProtocol preference - Re-added Tool Call Protocol dropdown in Advanced Settings (ApiOptions.tsx) - Updated tests to reflect new behavior Fixes #10459 --- .../__tests__/resolveToolProtocol.spec.ts | 110 +++++++++++++----- src/utils/resolveToolProtocol.ts | 23 ++-- .../src/components/settings/ApiOptions.tsx | 27 +++++ 3 files changed, 119 insertions(+), 41 deletions(-) diff --git a/src/utils/__tests__/resolveToolProtocol.spec.ts b/src/utils/__tests__/resolveToolProtocol.spec.ts index 513a7eaa35e..e85480ced23 100644 --- a/src/utils/__tests__/resolveToolProtocol.spec.ts +++ b/src/utils/__tests__/resolveToolProtocol.spec.ts @@ -6,20 +6,19 @@ import type { Anthropic } from "@anthropic-ai/sdk" describe("resolveToolProtocol", () => { /** - * XML Protocol Deprecation: - * - * XML tool protocol has been fully deprecated. All models now use Native - * tool calling. User preferences and model defaults are ignored. + * Tool Protocol Resolution: * * Precedence: - * 1. Locked Protocol (for resumed tasks that used XML) - * 2. Native (always, for all new tasks) + * 1. Locked Protocol (for resumed tasks - highest priority) + * 2. User/Profile Preference (providerSettings.toolProtocol) - allows users to force XML + * for models that don't handle native tool calling well (e.g., Qwen3, Kimi2) + * 3. Native (default for all new tasks without explicit preference) */ describe("Locked Protocol (Precedence Level 0 - Highest Priority)", () => { - it("should return lockedProtocol when provided", () => { + it("should return lockedProtocol when provided, ignoring user preference", () => { const settings: ProviderSettings = { - toolProtocol: "xml", // Ignored + toolProtocol: "xml", // User preference - overridden by locked protocol apiProvider: "openai-native", } // lockedProtocol overrides everything @@ -29,7 +28,7 @@ describe("resolveToolProtocol", () => { it("should return XML lockedProtocol for resumed tasks that used XML", () => { const settings: ProviderSettings = { - toolProtocol: "native", // Ignored + toolProtocol: "native", // User preference - overridden by locked protocol apiProvider: "anthropic", } // lockedProtocol forces XML for backward compatibility @@ -37,36 +36,58 @@ describe("resolveToolProtocol", () => { expect(result).toBe(TOOL_PROTOCOL.XML) }) - it("should fall through to Native when lockedProtocol is undefined", () => { + it("should fall through to user preference when lockedProtocol is undefined", () => { const settings: ProviderSettings = { - toolProtocol: "xml", // Ignored + toolProtocol: "xml", // User preference takes effect apiProvider: "anthropic", } - // undefined lockedProtocol should return native + // undefined lockedProtocol should use user preference const result = resolveToolProtocol(settings, undefined, undefined) - expect(result).toBe(TOOL_PROTOCOL.NATIVE) + expect(result).toBe(TOOL_PROTOCOL.XML) }) }) - describe("Native Protocol Always Used For New Tasks", () => { - it("should always use native for new tasks", () => { + describe("User/Profile Preference (Precedence Level 1)", () => { + it("should respect user preference for XML protocol", () => { const settings: ProviderSettings = { - apiProvider: "anthropic", + toolProtocol: "xml", + apiProvider: "openai", + } + const result = resolveToolProtocol(settings) + expect(result).toBe(TOOL_PROTOCOL.XML) + }) + + it("should respect user preference for native protocol", () => { + const settings: ProviderSettings = { + toolProtocol: "native", + apiProvider: "openai", } const result = resolveToolProtocol(settings) expect(result).toBe(TOOL_PROTOCOL.NATIVE) }) - it("should use native even when user preference is XML (user prefs ignored)", () => { + it("should allow XML for OpenAI-compatible models when user prefers XML", () => { + // This is the key scenario for models like Qwen3, Kimi2 that + // don't handle native tool calling well const settings: ProviderSettings = { - toolProtocol: "xml", // User wants XML - ignored - apiProvider: "openai-native", + toolProtocol: "xml", + apiProvider: "openai", + } + const result = resolveToolProtocol(settings, openAiModelInfoSaneDefaults) + expect(result).toBe(TOOL_PROTOCOL.XML) + }) + }) + + describe("Default Protocol (Precedence Level 2 - No User Preference)", () => { + it("should default to native for new tasks without user preference", () => { + const settings: ProviderSettings = { + apiProvider: "anthropic", } const result = resolveToolProtocol(settings) expect(result).toBe(TOOL_PROTOCOL.NATIVE) }) - it("should use native for OpenAI compatible provider", () => { + it("should default to native for OpenAI compatible provider without preference", () => { const settings: ProviderSettings = { apiProvider: "openai", } @@ -79,7 +100,7 @@ describe("resolveToolProtocol", () => { it("should handle missing provider name gracefully", () => { const settings: ProviderSettings = {} const result = resolveToolProtocol(settings) - expect(result).toBe(TOOL_PROTOCOL.NATIVE) // Always native now + expect(result).toBe(TOOL_PROTOCOL.NATIVE) // Default to native }) it("should handle undefined model info gracefully", () => { @@ -87,18 +108,26 @@ describe("resolveToolProtocol", () => { apiProvider: "openai-native", } const result = resolveToolProtocol(settings, undefined) - expect(result).toBe(TOOL_PROTOCOL.NATIVE) // Always native now + expect(result).toBe(TOOL_PROTOCOL.NATIVE) // Default to native }) it("should handle empty settings", () => { const settings: ProviderSettings = {} const result = resolveToolProtocol(settings) - expect(result).toBe(TOOL_PROTOCOL.NATIVE) // Always native now + expect(result).toBe(TOOL_PROTOCOL.NATIVE) // Default to native + }) + + it("should respect user XML preference even with empty provider", () => { + const settings: ProviderSettings = { + toolProtocol: "xml", + } + const result = resolveToolProtocol(settings) + expect(result).toBe(TOOL_PROTOCOL.XML) }) }) describe("Real-world Scenarios", () => { - it("should use Native for OpenAI models", () => { + it("should default to Native for OpenAI models", () => { const settings: ProviderSettings = { apiProvider: "openai-native", } @@ -112,7 +141,7 @@ describe("resolveToolProtocol", () => { expect(result).toBe(TOOL_PROTOCOL.NATIVE) }) - it("should use Native for Claude models", () => { + it("should default to Native for Claude models", () => { const settings: ProviderSettings = { apiProvider: "anthropic", } @@ -134,21 +163,40 @@ describe("resolveToolProtocol", () => { const result = resolveToolProtocol(settings, undefined, "xml") expect(result).toBe(TOOL_PROTOCOL.XML) }) + + it("should allow user to force XML for OpenAI-compatible models (Qwen3, Kimi2 use case)", () => { + // This is the primary use case from issue #10459 + // Users with Qwen3 or Kimi2 models need XML protocol + const settings: ProviderSettings = { + toolProtocol: "xml", + apiProvider: "openai", + } + const result = resolveToolProtocol(settings, openAiModelInfoSaneDefaults) + expect(result).toBe(TOOL_PROTOCOL.XML) + }) }) - describe("Backward Compatibility - User Preferences Ignored", () => { - it("should ignore user preference for XML", () => { + describe("Backward Compatibility - User Preferences Now Respected", () => { + it("should respect user preference for XML when set", () => { const settings: ProviderSettings = { - toolProtocol: "xml", // User explicitly wants XML - ignored + toolProtocol: "xml", apiProvider: "openai-native", } const result = resolveToolProtocol(settings) - expect(result).toBe(TOOL_PROTOCOL.NATIVE) // Native is always used + expect(result).toBe(TOOL_PROTOCOL.XML) + }) + + it("should respect user preference for native when set", () => { + const settings: ProviderSettings = { + toolProtocol: "native", + apiProvider: "anthropic", + } + const result = resolveToolProtocol(settings) + expect(result).toBe(TOOL_PROTOCOL.NATIVE) }) - it("should return native regardless of user preference", () => { + it("should default to native when no preference set", () => { const settings: ProviderSettings = { - toolProtocol: "native", // User preference - ignored but happens to match apiProvider: "anthropic", } const result = resolveToolProtocol(settings) diff --git a/src/utils/resolveToolProtocol.ts b/src/utils/resolveToolProtocol.ts index 92041fbeaf2..88f53e850d0 100644 --- a/src/utils/resolveToolProtocol.ts +++ b/src/utils/resolveToolProtocol.ts @@ -14,22 +14,19 @@ type ApiMessageForDetection = Anthropic.MessageParam & { /** * Resolve the effective tool protocol. * - * **Deprecation Note (XML Protocol):** - * XML tool protocol has been deprecated. All models now use Native tool calling. - * User/profile preferences (`providerSettings.toolProtocol`) and model defaults - * (`modelInfo.defaultToolProtocol`) are ignored. - * * Precedence: * 1. Locked Protocol (task-level lock for resumed tasks - highest priority) - * 2. Native (always, for all new tasks) + * 2. User/Profile Preference (providerSettings.toolProtocol) - allows forcing XML for models + * that don't handle native tool calling well (e.g., some OpenAI-compatible models like Qwen3, Kimi2) + * 3. Native (default for all new tasks without explicit preference) * - * @param _providerSettings - The provider settings (toolProtocol field is ignored) + * @param providerSettings - The provider settings (toolProtocol field used for user preference) * @param _modelInfo - Unused, kept for API compatibility * @param lockedProtocol - Optional task-locked protocol that takes absolute precedence * @returns The resolved tool protocol (either "xml" or "native") */ export function resolveToolProtocol( - _providerSettings: ProviderSettings, + providerSettings: ProviderSettings, _modelInfo?: unknown, lockedProtocol?: ToolProtocol, ): ToolProtocol { @@ -39,8 +36,14 @@ export function resolveToolProtocol( return lockedProtocol } - // 2. Always return Native protocol for new tasks - // All models now support native tools; XML is deprecated + // 2. User/Profile Preference - allow users to explicitly force XML protocol + // This is useful for models that don't handle native tool calling well + // (e.g., some OpenAI-compatible models like Qwen3, Kimi2) + if (providerSettings.toolProtocol) { + return providerSettings.toolProtocol + } + + // 3. Default to Native protocol for new tasks return TOOL_PROTOCOL.NATIVE } diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index 8e2c117e7c0..2e2b8ab2603 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -877,6 +877,33 @@ const ApiOptions = ({ } onChange={(value) => setApiConfigurationField("consecutiveMistakeLimit", value)} /> +
+ + +
+ {t("settings:toolProtocol.description")} +
+
{selectedProvider === "openrouter" && openRouterModelProviders && Object.keys(openRouterModelProviders).length > 0 && ( From c418a830f0267ef34785ff0db3e76f402af1d252 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 6 Jan 2026 12:27:09 +0000 Subject: [PATCH 2/3] fix: increase clearCache test timeout to 60s --- .../src/custom-tools/__tests__/custom-tool-registry.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/custom-tools/__tests__/custom-tool-registry.spec.ts b/packages/core/src/custom-tools/__tests__/custom-tool-registry.spec.ts index 967ae2e8df7..8ba3d024eb0 100644 --- a/packages/core/src/custom-tools/__tests__/custom-tool-registry.spec.ts +++ b/packages/core/src/custom-tools/__tests__/custom-tool-registry.spec.ts @@ -281,7 +281,7 @@ describe("CustomToolRegistry", () => { const result = await registry.loadFromDirectory(TEST_FIXTURES_DIR) expect(result.loaded).toContain("cached") - }, 30000) + }, 60000) }) describe.sequential("loadFromDirectories", () => { From d30b3b4523646360028e173847090caa71d4de7d Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 6 Jan 2026 12:40:13 +0000 Subject: [PATCH 3/3] fix: add null check for providerSettings and update tests for user preference behavior --- .../providers/__tests__/anthropic-vertex.spec.ts | 15 ++++++--------- src/api/providers/__tests__/anthropic.spec.ts | 15 ++++++--------- src/utils/resolveToolProtocol.ts | 2 +- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/api/providers/__tests__/anthropic-vertex.spec.ts b/src/api/providers/__tests__/anthropic-vertex.spec.ts index 6890e4178b1..0644ffc5479 100644 --- a/src/api/providers/__tests__/anthropic-vertex.spec.ts +++ b/src/api/providers/__tests__/anthropic-vertex.spec.ts @@ -1200,8 +1200,9 @@ describe("VertexHandler", () => { ) }) - it("should include tools even when toolProtocol is set to xml (user preference now ignored)", async () => { - // XML protocol deprecation: user preference is now ignored when model supports native tools + it("should NOT include tools when toolProtocol is set to xml (user preference is now respected)", async () => { + // User preference is now respected: when XML protocol is selected, tools are NOT sent to the API + // (tools are handled via XML in the system prompt instead) handler = new AnthropicVertexHandler({ apiModelId: "claude-3-5-sonnet-v2@20241022", vertexProjectId: "test-project", @@ -1242,14 +1243,10 @@ describe("VertexHandler", () => { // Just consume } - // Native is forced when supportsNativeTools===true, so tools should still be included + // User preference for XML is now respected, so tools should NOT be included expect(mockCreate).toHaveBeenCalledWith( - expect.objectContaining({ - tools: expect.arrayContaining([ - expect.objectContaining({ - name: "get_weather", - }), - ]), + expect.not.objectContaining({ + tools: expect.anything(), }), undefined, ) diff --git a/src/api/providers/__tests__/anthropic.spec.ts b/src/api/providers/__tests__/anthropic.spec.ts index 3fa5baf81be..d9fba62f511 100644 --- a/src/api/providers/__tests__/anthropic.spec.ts +++ b/src/api/providers/__tests__/anthropic.spec.ts @@ -451,8 +451,9 @@ describe("AnthropicHandler", () => { ) }) - it("should include tools even when toolProtocol is set to xml (user preference now ignored)", async () => { - // XML protocol deprecation: user preference is now ignored when model supports native tools + it("should NOT include tools when toolProtocol is set to xml (user preference is now respected)", async () => { + // User preference is now respected: when XML protocol is selected, tools are NOT sent to the API + // (tools are handled via XML in the system prompt instead) const xmlHandler = new AnthropicHandler({ ...mockOptions, toolProtocol: "xml", @@ -468,14 +469,10 @@ describe("AnthropicHandler", () => { // Just consume } - // Native is forced when supportsNativeTools===true, so tools should still be included + // User preference for XML is now respected, so tools should NOT be included expect(mockCreate).toHaveBeenCalledWith( - expect.objectContaining({ - tools: expect.arrayContaining([ - expect.objectContaining({ - name: "get_weather", - }), - ]), + expect.not.objectContaining({ + tools: expect.anything(), }), expect.anything(), ) diff --git a/src/utils/resolveToolProtocol.ts b/src/utils/resolveToolProtocol.ts index 88f53e850d0..9bc3b124fd7 100644 --- a/src/utils/resolveToolProtocol.ts +++ b/src/utils/resolveToolProtocol.ts @@ -39,7 +39,7 @@ export function resolveToolProtocol( // 2. User/Profile Preference - allow users to explicitly force XML protocol // This is useful for models that don't handle native tool calling well // (e.g., some OpenAI-compatible models like Qwen3, Kimi2) - if (providerSettings.toolProtocol) { + if (providerSettings?.toolProtocol) { return providerSettings.toolProtocol }