From aa68560b78fa23bffe87fa41965a0daea32f81ad Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Thu, 27 Nov 2025 10:46:58 -0500 Subject: [PATCH] Track tool protocol in the conversation history --- src/core/task-persistence/apiMessages.ts | 2 + src/core/task/Task.ts | 8 +- .../__tests__/toolProtocol-tracking.test.ts | 457 ++++++++++++++++++ 3 files changed, 465 insertions(+), 2 deletions(-) create mode 100644 src/core/task/__tests__/toolProtocol-tracking.test.ts diff --git a/src/core/task-persistence/apiMessages.ts b/src/core/task-persistence/apiMessages.ts index 2628568135..50fd3bfcab 100644 --- a/src/core/task-persistence/apiMessages.ts +++ b/src/core/task-persistence/apiMessages.ts @@ -20,6 +20,8 @@ export type ApiMessage = Anthropic.MessageParam & { text?: string // For OpenRouter reasoning_details array format (used by Gemini 3, etc.) reasoning_details?: any[] + // Tool protocol used when this message was created ("xml" or "native") + toolProtocol?: "xml" | "native" } export async function readApiMessages({ diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 04062797b5..48669478e4 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -687,6 +687,10 @@ export class Task extends EventEmitter implements TaskLike { getReasoningDetails?: () => any[] | undefined } + // Track the tool protocol used for this message + const modelInfo = this.api.getModel().info + const currentToolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) + if (message.role === "assistant") { const responseId = handler.getResponseId?.() const reasoningData = handler.getEncryptedContent?.() @@ -766,9 +770,9 @@ export class Task extends EventEmitter implements TaskLike { } } - this.apiConversationHistory.push(messageWithTs) + this.apiConversationHistory.push({ ...messageWithTs, toolProtocol: currentToolProtocol }) } else { - const messageWithTs = { ...message, ts: Date.now() } + const messageWithTs = { ...message, ts: Date.now(), toolProtocol: currentToolProtocol } this.apiConversationHistory.push(messageWithTs) } diff --git a/src/core/task/__tests__/toolProtocol-tracking.test.ts b/src/core/task/__tests__/toolProtocol-tracking.test.ts new file mode 100644 index 0000000000..8f87eb9e82 --- /dev/null +++ b/src/core/task/__tests__/toolProtocol-tracking.test.ts @@ -0,0 +1,457 @@ +import { describe, it, expect, vi, beforeEach, beforeAll } from "vitest" +import type { ClineProvider } from "../../webview/ClineProvider" +import type { ProviderSettings, ModelInfo } from "@roo-code/types" + +// Mock vscode module before importing Task +vi.mock("vscode", () => ({ + workspace: { + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(), + onDidChange: vi.fn(), + onDidDelete: vi.fn(), + dispose: vi.fn(), + })), + getConfiguration: vi.fn(() => ({ + get: vi.fn(() => true), + })), + openTextDocument: vi.fn(), + applyEdit: vi.fn(), + }, + RelativePattern: vi.fn((base, pattern) => ({ base, pattern })), + window: { + createOutputChannel: vi.fn(() => ({ + appendLine: vi.fn(), + dispose: vi.fn(), + })), + createTextEditorDecorationType: vi.fn(() => ({ + dispose: vi.fn(), + })), + showTextDocument: vi.fn(), + activeTextEditor: undefined, + }, + Uri: { + file: vi.fn((path) => ({ fsPath: path })), + parse: vi.fn((str) => ({ toString: () => str })), + }, + Range: vi.fn(), + Position: vi.fn(), + WorkspaceEdit: vi.fn(() => ({ + replace: vi.fn(), + insert: vi.fn(), + delete: vi.fn(), + })), + ViewColumn: { + One: 1, + Two: 2, + Three: 3, + }, +})) + +// Mock other dependencies +vi.mock("../../services/mcp/McpServerManager", () => ({ + McpServerManager: { + getInstance: vi.fn().mockResolvedValue(null), + }, +})) + +vi.mock("../../integrations/terminal/TerminalRegistry", () => ({ + TerminalRegistry: { + releaseTerminalsForTask: vi.fn(), + }, +})) + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureTaskCreated: vi.fn(), + captureTaskRestarted: vi.fn(), + captureConversationMessage: vi.fn(), + captureLlmCompletion: vi.fn(), + captureConsecutiveMistakeError: vi.fn(), + }, + }, +})) + +describe("Task toolProtocol tracking", () => { + let mockProvider: Partial + let mockApiConfiguration: ProviderSettings + let Task: any + + beforeAll(async () => { + // Import Task after mocks are set up + const taskModule = await import("../Task") + Task = taskModule.Task + }) + + beforeEach(() => { + // Mock provider with necessary methods + mockProvider = { + postStateToWebview: vi.fn().mockResolvedValue(undefined), + getState: vi.fn().mockResolvedValue({ + mode: "code", + experiments: {}, + }), + context: { + globalStorageUri: { fsPath: "/test/storage" }, + extensionPath: "/test/extension", + } as any, + log: vi.fn(), + updateTaskHistory: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + } + + mockApiConfiguration = { + apiProvider: "anthropic", + apiKey: "test-key", + } as ProviderSettings + }) + + it("should store toolProtocol 'xml' for user messages when using XML protocol", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Avoid disk writes in this test + ;(task as any).saveApiConversationHistory = vi.fn().mockResolvedValue(undefined) + + // Mock the API to return a model that uses XML protocol + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: false, // XML protocol + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + await (task as any).addToApiConversationHistory({ + role: "user", + content: [{ type: "text", text: "Hello" }], + }) + + expect(task.apiConversationHistory).toHaveLength(1) + const stored = task.apiConversationHistory[0] as any + + expect(stored.role).toBe("user") + expect(stored.toolProtocol).toBe("xml") + expect(stored.ts).toBeDefined() + }) + + it("should store toolProtocol 'xml' for assistant messages when using XML protocol", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Avoid disk writes in this test + ;(task as any).saveApiConversationHistory = vi.fn().mockResolvedValue(undefined) + + // Mock the API to return a model that uses XML protocol + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: false, // XML protocol + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + await (task as any).addToApiConversationHistory({ + role: "assistant", + content: [{ type: "text", text: "Here is my response." }], + }) + + expect(task.apiConversationHistory).toHaveLength(1) + const stored = task.apiConversationHistory[0] as any + + expect(stored.role).toBe("assistant") + expect(stored.toolProtocol).toBe("xml") + expect(stored.ts).toBeDefined() + }) + + it("should store toolProtocol 'native' when model supports native tools and configured for native", async () => { + // Create a task instance with native tool configuration + const nativeApiConfiguration: ProviderSettings = { + apiProvider: "openai", + apiKey: "test-key", + toolProtocol: "native", // Explicitly set to native + } as ProviderSettings + + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: nativeApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Avoid disk writes in this test + ;(task as any).saveApiConversationHistory = vi.fn().mockResolvedValue(undefined) + + // Update task's apiConfiguration + task.apiConfiguration = nativeApiConfiguration + + // Mock the API to return a model that supports native tools + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: true, // Native protocol supported + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "gpt-4o", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + await (task as any).addToApiConversationHistory({ + role: "user", + content: [{ type: "text", text: "Hello" }], + }) + + expect(task.apiConversationHistory).toHaveLength(1) + const stored = task.apiConversationHistory[0] as any + + expect(stored.role).toBe("user") + expect(stored.toolProtocol).toBe("native") + expect(stored.ts).toBeDefined() + }) + + it("should preserve toolProtocol on assistant messages with reasoning", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Avoid disk writes in this test + ;(task as any).saveApiConversationHistory = vi.fn().mockResolvedValue(undefined) + + // Mock the API to return a model that uses XML protocol + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: false, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + const reasoningText = "Let me think about this..." + + await (task as any).addToApiConversationHistory( + { + role: "assistant", + content: [{ type: "text", text: "Here is my response." }], + }, + reasoningText, + ) + + expect(task.apiConversationHistory).toHaveLength(1) + const stored = task.apiConversationHistory[0] as any + + expect(stored.role).toBe("assistant") + expect(stored.toolProtocol).toBe("xml") + expect(stored.ts).toBeDefined() + // Verify reasoning was also stored + expect(Array.isArray(stored.content)).toBe(true) + expect(stored.content[0].type).toBe("reasoning") + }) + + it("should include toolProtocol in both user and assistant messages in a conversation", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Avoid disk writes in this test + ;(task as any).saveApiConversationHistory = vi.fn().mockResolvedValue(undefined) + + // Mock the API to return a model that uses XML protocol + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: false, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + // Add user message + await (task as any).addToApiConversationHistory({ + role: "user", + content: [{ type: "text", text: "Hello" }], + }) + + // Add assistant message + await (task as any).addToApiConversationHistory({ + role: "assistant", + content: [{ type: "text", text: "Hi there!" }], + }) + + // Add another user message + await (task as any).addToApiConversationHistory({ + role: "user", + content: [{ type: "text", text: "Help me with a task" }], + }) + + expect(task.apiConversationHistory).toHaveLength(3) + + // All messages should have toolProtocol set + for (const msg of task.apiConversationHistory) { + expect((msg as any).toolProtocol).toBe("xml") + expect((msg as any).ts).toBeDefined() + } + }) + + it("should handle toolProtocol when apiConfiguration.toolProtocol is explicitly set to xml", async () => { + // Create a task instance with explicit XML configuration + const xmlApiConfiguration: ProviderSettings = { + apiProvider: "anthropic", + apiKey: "test-key", + toolProtocol: "xml", // Explicitly set to xml + } as ProviderSettings + + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: xmlApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Avoid disk writes in this test + ;(task as any).saveApiConversationHistory = vi.fn().mockResolvedValue(undefined) + + // Update task's apiConfiguration + task.apiConfiguration = xmlApiConfiguration + + // Mock the API to return a model (even if it supports native tools, config overrides) + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: true, // Model supports native, but config says xml + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Mock the API conversation history + task.apiConversationHistory = [] + + await (task as any).addToApiConversationHistory({ + role: "user", + content: [{ type: "text", text: "Hello" }], + }) + + expect(task.apiConversationHistory).toHaveLength(1) + const stored = task.apiConversationHistory[0] as any + + expect(stored.role).toBe("user") + // The explicit toolProtocol config should be respected + expect(stored.toolProtocol).toBe("xml") + }) + + it("should NOT include toolProtocol in cleaned conversation history sent to API", async () => { + // Create a task instance + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Avoid disk writes in this test + ;(task as any).saveApiConversationHistory = vi.fn().mockResolvedValue(undefined) + + // Mock the API to return a model that uses XML protocol + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: false, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "test-model", + info: mockModelInfo, + }), + } + + // Manually populate the API conversation history with toolProtocol field + task.apiConversationHistory = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + ts: Date.now(), + toolProtocol: "xml", + } as any, + { + role: "assistant", + content: [{ type: "text", text: "Hi there!" }], + ts: Date.now(), + toolProtocol: "xml", + } as any, + ] + + // Call buildCleanConversationHistory to get what would be sent to API + const cleanHistory = (task as any).buildCleanConversationHistory(task.apiConversationHistory) + + expect(cleanHistory).toHaveLength(2) + + // Verify toolProtocol is NOT in the cleaned messages + for (const msg of cleanHistory) { + expect(msg).not.toHaveProperty("toolProtocol") + expect(msg).not.toHaveProperty("ts") + // Should only have role and content + expect(msg).toHaveProperty("role") + expect(msg).toHaveProperty("content") + } + }) +})