From 7c17d430fc2704f738af13cb0db07af13a66553b Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Thu, 27 Nov 2025 10:51:20 -0500 Subject: [PATCH] Enable XML parser if there are any xml messages in the conversation history --- src/core/task/Task.ts | 27 +++- .../__tests__/toolProtocol-tracking.test.ts | 150 ++++++++++++++++++ 2 files changed, 172 insertions(+), 5 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 48669478e4..c6f93951e3 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -443,9 +443,10 @@ export class Task extends EventEmitter implements TaskLike { TelemetryService.instance.captureTaskCreated(this.taskId) } - // Initialize the assistant message parser only for XML protocol. + // Initialize the assistant message parser for XML protocol. // For native protocol, tool calls come as tool_call chunks, not XML. - // experiments is always provided via TaskOptions (defaults to experimentDefault in provider) + // The parser will be properly initialized after history is loaded (for resumed tasks) + // or based on current protocol (for new tasks). const modelInfo = this.api.getModel().info const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) this.assistantMessageParser = toolProtocol !== "native" ? new AssistantMessageParser() : undefined @@ -1172,15 +1173,29 @@ export class Task extends EventEmitter implements TaskLike { * * @param newApiConfiguration - The new API configuration to use */ + /** + * Checks if there are any XML protocol messages in the conversation history. + * This is used to determine if the XML parser should be activated for resumed tasks + * that may have been started with a different model/protocol. + * + * @returns true if any message in the conversation history was created with XML protocol + */ + public hasXmlMessagesInHistory(): boolean { + return this.apiConversationHistory.some((msg) => (msg as any).toolProtocol === "xml") + } + public updateApiConfiguration(newApiConfiguration: ProviderSettings): void { // Update the configuration and rebuild the API handler this.apiConfiguration = newApiConfiguration this.api = buildApiHandler(newApiConfiguration) - // Determine what the tool protocol should be + // Determine what the tool protocol should be based on current config const modelInfo = this.api.getModel().info const protocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - const shouldUseXmlParser = protocol === "xml" + + // Also check if there are any XML protocol messages in history + // This ensures we can parse XML tool calls from resumed conversations even if model changed + const shouldUseXmlParser = protocol === "xml" || this.hasXmlMessagesInHistory() // Ensure parser state matches protocol requirement const parserStateCorrect = @@ -2308,7 +2323,9 @@ export class Task extends EventEmitter implements TaskLike { const streamModelInfo = this.cachedStreamingModel.info const cachedModelId = this.cachedStreamingModel.id const streamProtocol = resolveToolProtocol(this.apiConfiguration, streamModelInfo) - const shouldUseXmlParser = streamProtocol === "xml" + // Activate XML parser if current protocol is XML OR if there are any XML protocol messages in history + // This ensures we can parse XML tool calls from resumed conversations even if model changed + const shouldUseXmlParser = streamProtocol === "xml" || this.hasXmlMessagesInHistory() // Yields only if the first chunk is successful, otherwise will // allow the user to retry the request (most likely due to rate diff --git a/src/core/task/__tests__/toolProtocol-tracking.test.ts b/src/core/task/__tests__/toolProtocol-tracking.test.ts index 8f87eb9e82..7cf13a03a3 100644 --- a/src/core/task/__tests__/toolProtocol-tracking.test.ts +++ b/src/core/task/__tests__/toolProtocol-tracking.test.ts @@ -454,4 +454,154 @@ describe("Task toolProtocol tracking", () => { expect(msg).toHaveProperty("content") } }) + + describe("XML parser activation based on history", () => { + it("should activate XML parser when history contains XML protocol messages even if current model uses native", async () => { + // Create a task instance with a model that supports native tools + const nativeApiConfiguration: ProviderSettings = { + apiProvider: "openai", + apiKey: "test-key", + toolProtocol: "native", + } as ProviderSettings + + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: nativeApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Mock the API to return a model that supports native tools + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: true, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "gpt-4o", + info: mockModelInfo, + }), + } + + // Initially with native protocol and no history, parser should NOT be active + // (We would need to call updateApiConfiguration to check this) + + // Populate history with XML protocol messages (simulating a resumed task) + task.apiConversationHistory = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + ts: Date.now() - 1000, + toolProtocol: "xml", + } as any, + { + role: "assistant", + content: [{ type: "text", text: "Hi there!" }], + ts: Date.now() - 500, + toolProtocol: "xml", + } as any, + ] + + // Call updateApiConfiguration which should activate the parser due to XML history + task.updateApiConfiguration(nativeApiConfiguration) + + // Parser should now be activated because history contains XML protocol messages + expect(task.assistantMessageParser).toBeDefined() + }) + + it("should NOT activate XML parser when history is empty and using native protocol", async () => { + // Create a task instance with a model that supports native tools + const nativeApiConfiguration: ProviderSettings = { + apiProvider: "openai", + apiKey: "test-key", + toolProtocol: "native", + } as ProviderSettings + + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: nativeApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Mock the API to return a model that supports native tools + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: true, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "gpt-4o", + info: mockModelInfo, + }), + } + + // Empty history with native protocol + task.apiConversationHistory = [] + + // Force recreate parser state based on configuration + task.assistantMessageParser = undefined + task.updateApiConfiguration(nativeApiConfiguration) + + // Parser should NOT be activated - no XML history and using native protocol + expect(task.assistantMessageParser).toBeUndefined() + }) + + it("should NOT activate XML parser when history only contains native protocol messages", async () => { + // Create a task instance with a model that supports native tools + const nativeApiConfiguration: ProviderSettings = { + apiProvider: "openai", + apiKey: "test-key", + toolProtocol: "native", + } as ProviderSettings + + const task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: nativeApiConfiguration, + task: "Test task", + startTask: false, + }) + + // Mock the API to return a model that supports native tools + const mockModelInfo: ModelInfo = { + contextWindow: 16000, + supportsPromptCache: true, + supportsNativeTools: true, + } + + task.api = { + getModel: vi.fn().mockReturnValue({ + id: "gpt-4o", + info: mockModelInfo, + }), + } + + // Populate history with native protocol messages only + task.apiConversationHistory = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + ts: Date.now() - 1000, + toolProtocol: "native", + } as any, + { + role: "assistant", + content: [{ type: "text", text: "Hi there!" }], + ts: Date.now() - 500, + toolProtocol: "native", + } as any, + ] + + // Force recreate parser state based on configuration + task.assistantMessageParser = undefined + task.updateApiConfiguration(nativeApiConfiguration) + + // Parser should NOT be activated - only native history and using native protocol + expect(task.assistantMessageParser).toBeUndefined() + }) + }) })