diff --git a/packages/types/src/history.ts b/packages/types/src/history.ts index e5b6f5418f7..d97884d216e 100644 --- a/packages/types/src/history.ts +++ b/packages/types/src/history.ts @@ -19,6 +19,16 @@ export const historyItemSchema = z.object({ size: z.number().optional(), workspace: z.string().optional(), mode: z.string().optional(), + /** + * The tool protocol used by this task. Once a task uses tools with a specific + * protocol (XML or Native), it is permanently locked to that protocol. + * + * - "xml": Tool calls are parsed from XML text (no tool IDs) + * - "native": Tool calls come as tool_call chunks with IDs + * + * This ensures task resumption works correctly even when NTC settings change. + */ + toolProtocol: z.enum(["xml", "native"]).optional(), status: z.enum(["active", "completed", "delegated"]).optional(), delegatedToId: z.string().optional(), // Last child this parent delegated to childIds: z.array(z.string()).optional(), // All children spawned by this task diff --git a/src/api/providers/anthropic.ts b/src/api/providers/anthropic.ts index 38426d6295e..c17b4313ac1 100644 --- a/src/api/providers/anthropic.ts +++ b/src/api/providers/anthropic.ts @@ -73,8 +73,9 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa // Enable native tools by default using resolveToolProtocol (which checks model's defaultToolProtocol) // This matches OpenRouter's approach of always including tools when provided // Also exclude tools when tool_choice is "none" since that means "don't use tools" + // IMPORTANT: Use metadata.toolProtocol if provided (task's locked protocol) for consistency const model = this.getModel() - const toolProtocol = resolveToolProtocol(this.options, model.info) + const toolProtocol = resolveToolProtocol(this.options, model.info, metadata?.toolProtocol) const shouldIncludeNativeTools = metadata?.tools && metadata.tools.length > 0 && diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts index 4f4cc6184c7..5b8c29c337a 100644 --- a/src/api/providers/openrouter.ts +++ b/src/api/providers/openrouter.ts @@ -244,7 +244,8 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH } // Process reasoning_details when switching models to Gemini for native tool call compatibility - const toolProtocol = resolveToolProtocol(this.options, model.info) + // IMPORTANT: Use metadata.toolProtocol if provided (task's locked protocol) for consistency + const toolProtocol = resolveToolProtocol(this.options, model.info, metadata?.toolProtocol) const isNativeProtocol = toolProtocol === TOOL_PROTOCOL.NATIVE const isGemini = modelId.startsWith("google/gemini") diff --git a/src/api/providers/requesty.ts b/src/api/providers/requesty.ts index 43b32793183..b84a36bcc16 100644 --- a/src/api/providers/requesty.ts +++ b/src/api/providers/requesty.ts @@ -139,7 +139,8 @@ export class RequestyHandler extends BaseProvider implements SingleCompletionHan : undefined // Check if native tool protocol is enabled - const toolProtocol = resolveToolProtocol(this.options, info) + // IMPORTANT: Use metadata.toolProtocol if provided (task's locked protocol) for consistency + const toolProtocol = resolveToolProtocol(this.options, info, metadata?.toolProtocol) const useNativeTools = toolProtocol === TOOL_PROTOCOL.NATIVE const completionParams: RequestyChatCompletionParamsStreaming = { diff --git a/src/core/environment/getEnvironmentDetails.ts b/src/core/environment/getEnvironmentDetails.ts index 64cd93737fd..ebb6f18e48a 100644 --- a/src/core/environment/getEnvironmentDetails.ts +++ b/src/core/environment/getEnvironmentDetails.ts @@ -236,9 +236,12 @@ export async function getEnvironmentDetails(cline: Task, includeFileDetails: boo language: language ?? formatLanguage(vscode.env.language), }) - // Resolve and add tool protocol information + // Use the task's locked tool protocol for consistent environment details. + // This ensures the model sees the same tool format it was started with, + // even if user settings have changed. Fall back to resolving fresh if + // the task hasn't been fully initialized yet (shouldn't happen in practice). const modelInfo = cline.api.getModel().info - const toolProtocol = resolveToolProtocol(state?.apiConfiguration ?? {}, modelInfo) + const toolProtocol = resolveToolProtocol(state?.apiConfiguration ?? {}, modelInfo, cline.taskToolProtocol) details += `\n\n# Current Mode\n` details += `${currentMode}\n` diff --git a/src/core/task-persistence/taskMetadata.ts b/src/core/task-persistence/taskMetadata.ts index 25a548b6e2e..eb872a6f7e9 100644 --- a/src/core/task-persistence/taskMetadata.ts +++ b/src/core/task-persistence/taskMetadata.ts @@ -1,7 +1,7 @@ import NodeCache from "node-cache" import getFolderSize from "get-folder-size" -import type { ClineMessage, HistoryItem } from "@roo-code/types" +import type { ClineMessage, HistoryItem, ToolProtocol } from "@roo-code/types" import { combineApiRequests } from "../../shared/combineApiRequests" import { combineCommandSequences } from "../../shared/combineCommandSequences" @@ -23,6 +23,11 @@ export type TaskMetadataOptions = { mode?: string /** Initial status for the task (e.g., "active" for child tasks) */ initialStatus?: "active" | "delegated" | "completed" + /** + * The tool protocol locked to this task. Once set, the task will + * continue using this protocol even if user settings change. + */ + toolProtocol?: ToolProtocol } export async function taskMetadata({ @@ -35,6 +40,7 @@ export async function taskMetadata({ workspace, mode, initialStatus, + toolProtocol, }: TaskMetadataOptions) { const taskDir = await getTaskDirectoryPath(globalStoragePath, id) @@ -90,6 +96,8 @@ export async function taskMetadata({ // initialStatus is included when provided (e.g., "active" for child tasks) // to ensure the status is set from the very first save, avoiding race conditions // where attempt_completion might run before a separate status update. + // toolProtocol is persisted to ensure tasks resume with the correct protocol + // even if user settings have changed. const historyItem: HistoryItem = { id, rootTaskId, @@ -107,6 +115,7 @@ export async function taskMetadata({ size: taskDirSize, workspace, mode, + ...(toolProtocol && { toolProtocol }), ...(initialStatus && { status: initialStatus }), } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9161d3b462a..1bc19c15945 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -32,6 +32,7 @@ import { type HistoryItem, type CreateTaskOptions, type ModelInfo, + type ToolProtocol, RooCodeEventName, TelemetryEventName, TaskStatus, @@ -51,7 +52,7 @@ import { } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" import { CloudService, BridgeOrchestrator } from "@roo-code/cloud" -import { resolveToolProtocol } from "../../utils/resolveToolProtocol" +import { resolveToolProtocol, detectToolProtocolFromHistory } from "../../utils/resolveToolProtocol" // api import { ApiHandler, ApiHandlerCreateMessageMetadata, buildApiHandler } from "../../api" @@ -203,6 +204,30 @@ export class Task extends EventEmitter implements TaskLike { */ private _taskMode: string | undefined + /** + * The tool protocol locked to this task. Once set, the task will continue + * using this protocol even if user settings change. + * + * ## Why This Matters + * When NTC (Native Tool Calling) is enabled, XML parsing does NOT occur. + * If a task previously used XML tools, resuming it with NTC enabled would + * break because the tool calls in the history would not be parseable. + * + * ## Lifecycle + * + * ### For new tasks: + * 1. Set immediately in constructor via `resolveToolProtocol()` + * 2. Locked for the lifetime of the task + * + * ### For history items: + * 1. If `historyItem.toolProtocol` exists, use it + * 2. Otherwise, detect from API history via `detectToolProtocolFromHistory()` + * 3. If no tools in history, use `resolveToolProtocol()` from current settings + * + * @private + */ + private _taskToolProtocol: ToolProtocol | undefined + /** * Promise that resolves when the task mode has been initialized. * This ensures async mode initialization completes before the task is used. @@ -459,19 +484,29 @@ export class Task extends EventEmitter implements TaskLike { this._taskMode = historyItem.mode || defaultModeSlug this.taskModeReady = Promise.resolve() TelemetryService.instance.captureTaskRestarted(this.taskId) + + // For history items, use the persisted tool protocol if available. + // If not available (old tasks), it will be detected in resumeTaskFromHistory. + this._taskToolProtocol = historyItem.toolProtocol } else { // For new tasks, don't set the mode yet - wait for async initialization. this._taskMode = undefined this.taskModeReady = this.initializeTaskMode(provider) TelemetryService.instance.captureTaskCreated(this.taskId) + + // For new tasks, resolve and lock the tool protocol immediately. + // This ensures the task will continue using this protocol even if + // user settings change. + const modelInfo = this.api.getModel().info + this._taskToolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) } - // Initialize the assistant message parser only for XML protocol. + // Initialize the assistant message parser based on the locked tool protocol. // For native protocol, tool calls come as tool_call chunks, not XML. - // experiments is always provided via TaskOptions (defaults to experimentDefault in provider) - const modelInfo = this.api.getModel().info - const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - this.assistantMessageParser = toolProtocol !== "native" ? new AssistantMessageParser() : undefined + // For history items without a persisted protocol, we default to XML parser + // and will update it in resumeTaskFromHistory after detection. + const effectiveProtocol = this._taskToolProtocol || "xml" + this.assistantMessageParser = effectiveProtocol !== "native" ? new AssistantMessageParser() : undefined this.messageQueueService = new MessageQueueService() @@ -977,6 +1012,7 @@ export class Task extends EventEmitter implements TaskLike { workspace: this.cwd, mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode. initialStatus: this.initialStatus, + toolProtocol: this._taskToolProtocol, // Persist the locked tool protocol. }) // Emit token/tool usage updates using debounced function @@ -1286,9 +1322,9 @@ export class Task extends EventEmitter implements TaskLike { } /** - * Updates the API configuration and reinitializes the parser based on the new tool protocol. - * This should be called when switching between models/profiles with different tool protocols - * to prevent the parser from being left in an inconsistent state. + * Updates the API configuration but preserves the locked tool protocol. + * The task's tool protocol is locked at creation time and should NOT change + * even when switching between models/profiles with different settings. * * @param newApiConfiguration - The new API configuration to use */ @@ -1297,26 +1333,10 @@ export class Task extends EventEmitter implements TaskLike { this.apiConfiguration = newApiConfiguration this.api = buildApiHandler(newApiConfiguration) - // Determine what the tool protocol should be - const modelInfo = this.api.getModel().info - const protocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - const shouldUseXmlParser = protocol === "xml" - - // Ensure parser state matches protocol requirement - const parserStateCorrect = - (shouldUseXmlParser && this.assistantMessageParser) || (!shouldUseXmlParser && !this.assistantMessageParser) - - if (parserStateCorrect) { - return - } - - // Fix parser state - if (shouldUseXmlParser && !this.assistantMessageParser) { - this.assistantMessageParser = new AssistantMessageParser() - } else if (!shouldUseXmlParser && this.assistantMessageParser) { - this.assistantMessageParser.reset() - this.assistantMessageParser = undefined - } + // IMPORTANT: Do NOT change the parser based on the new configuration! + // The task's tool protocol is locked at creation time and must remain + // consistent throughout the task's lifetime to ensure history can be + // properly resumed. } public async submitUserMessage( @@ -1399,9 +1419,8 @@ export class Task extends EventEmitter implements TaskLike { const { contextTokens: prevContextTokens } = this.getTokenUsage() // Determine if we're using native tool protocol for proper message handling - const modelInfo = this.api.getModel().info - const protocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - const useNativeTools = isNativeProtocol(protocol) + // Use the task's locked protocol, NOT the current settings (fallback to xml if not set) + const useNativeTools = isNativeProtocol(this._taskToolProtocol ?? "xml") const { messages, @@ -1583,10 +1602,10 @@ export class Task extends EventEmitter implements TaskLike { relPath ? ` for '${relPath.toPosix()}'` : "" } without value for required parameter '${paramName}'. Retrying...`, ) - const modelInfo = this.api.getModel().info - const state = await this.providerRef.deref()?.getState() - const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - return formatResponse.toolError(formatResponse.missingToolParameterError(paramName, toolProtocol)) + // Use the task's locked protocol, NOT the current settings (fallback to xml if not set) + return formatResponse.toolError( + formatResponse.missingToolParameterError(paramName, this._taskToolProtocol ?? "xml"), + ) } // Lifecycle @@ -1702,6 +1721,31 @@ export class Task extends EventEmitter implements TaskLike { // the task first. this.apiConversationHistory = await this.getSavedApiConversationHistory() + // If we don't have a persisted tool protocol (old tasks before this feature), + // detect it from the API history. This ensures tasks that previously used + // XML tools will continue using XML even if NTC is now enabled. + if (!this._taskToolProtocol) { + const detectedProtocol = detectToolProtocolFromHistory(this.apiConversationHistory) + if (detectedProtocol) { + // Found tool calls in history - lock to that protocol + this._taskToolProtocol = detectedProtocol + } else { + // No tool calls in history yet - use current settings + const modelInfo = this.api.getModel().info + this._taskToolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) + } + + // Update parser state to match the detected/resolved protocol + const shouldUseXmlParser = this._taskToolProtocol === "xml" + if (shouldUseXmlParser && !this.assistantMessageParser) { + this.assistantMessageParser = new AssistantMessageParser() + } else if (!shouldUseXmlParser && this.assistantMessageParser) { + this.assistantMessageParser.reset() + this.assistantMessageParser = undefined + } + } else { + } + const lastClineMessage = this.clineMessages .slice() .reverse() @@ -1735,9 +1779,8 @@ export class Task extends EventEmitter implements TaskLike { // we need to replace all tool use blocks with a text block since the API disallows // conversations with tool uses and no tool schema. // For native protocol, we preserve tool_use and tool_result blocks as they're expected by the API. - const state = await this.providerRef.deref()?.getState() - const protocol = resolveToolProtocol(this.apiConfiguration, this.api.getModel().info) - const useNative = isNativeProtocol(protocol) + // IMPORTANT: Use the task's locked protocol, NOT the current settings! + const useNative = isNativeProtocol(this._taskToolProtocol) // Only convert tool blocks to text for XML protocol // For native protocol, the API expects proper tool_use/tool_result structure @@ -1746,9 +1789,9 @@ export class Task extends EventEmitter implements TaskLike { if (Array.isArray(message.content)) { const newContent = message.content.map((block) => { if (block.type === "tool_use") { - // Format tool invocation based on protocol + // Format tool invocation based on the task's locked protocol const params = block.input as Record - const formattedText = formatToolInvocation(block.name, params, protocol) + const formattedText = formatToolInvocation(block.name, params, this._taskToolProtocol) return { type: "text", @@ -2190,9 +2233,8 @@ export class Task extends EventEmitter implements TaskLike { // the user hits max requests and denies resetting the count. break } else { - const modelInfo = this.api.getModel().info - const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - nextUserContent = [{ type: "text", text: formatResponse.noToolsUsed(toolProtocol) }] + // Use the task's locked protocol, NOT the current settings (fallback to xml if not set) + nextUserContent = [{ type: "text", text: formatResponse.noToolsUsed(this._taskToolProtocol ?? "xml") }] this.consecutiveMistakeCount++ } } @@ -2434,7 +2476,14 @@ export class Task extends EventEmitter implements TaskLike { this.cachedStreamingModel = this.api.getModel() const streamModelInfo = this.cachedStreamingModel.info const cachedModelId = this.cachedStreamingModel.id - const streamProtocol = resolveToolProtocol(this.apiConfiguration, streamModelInfo) + // Use the task's locked protocol instead of resolving fresh. + // This ensures task resumption works correctly even if NTC settings changed. + // Fallback to resolving if somehow _taskToolProtocol is not set (should not happen). + const streamProtocol = resolveToolProtocol( + this.apiConfiguration, + streamModelInfo, + this._taskToolProtocol, + ) const shouldUseXmlParser = streamProtocol === "xml" // Yields only if the first chunk is successful, otherwise will @@ -3187,10 +3236,11 @@ export class Task extends EventEmitter implements TaskLike { ) if (!didToolUse) { - const modelInfo = this.api.getModel().info - const state = await this.providerRef.deref()?.getState() - const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - this.userMessageContent.push({ type: "text", text: formatResponse.noToolsUsed(toolProtocol) }) + // Use the task's locked protocol for consistent behavior + this.userMessageContent.push({ + type: "text", + text: formatResponse.noToolsUsed(this._taskToolProtocol ?? "xml"), + }) this.consecutiveMistakeCount++ } @@ -3217,10 +3267,8 @@ export class Task extends EventEmitter implements TaskLike { // we need to remove that message before retrying to avoid having two consecutive // user messages (which would cause tool_result validation errors). let state = await this.providerRef.deref()?.getState() - if ( - isNativeProtocol(resolveToolProtocol(this.apiConfiguration, this.api.getModel().info)) && - this.apiConversationHistory.length > 0 - ) { + // Use the task's locked protocol, NOT current settings + if (isNativeProtocol(this._taskToolProtocol ?? "xml") && this.apiConversationHistory.length > 0) { const lastMessage = this.apiConversationHistory[this.apiConversationHistory.length - 1] if (lastMessage.role === "user") { // Remove the last user message that we added earlier @@ -3280,10 +3328,8 @@ export class Task extends EventEmitter implements TaskLike { } else { // User declined to retry // For native protocol, re-add the user message we removed - // Reuse the state variable from above - if ( - isNativeProtocol(resolveToolProtocol(this.apiConfiguration, this.api.getModel().info)) - ) { + // Use the task's locked protocol, NOT current settings + if (isNativeProtocol(this._taskToolProtocol ?? "xml")) { await this.addToApiConversationHistory({ role: "user", content: currentUserContent, @@ -3380,8 +3426,14 @@ export class Task extends EventEmitter implements TaskLike { const canUseBrowserTool = modelSupportsBrowser && modeSupportsBrowser && (browserToolEnabled ?? true) - // Resolve the tool protocol based on profile, model, and provider settings - const toolProtocol = resolveToolProtocol(apiConfiguration ?? this.apiConfiguration, modelInfo) + // Use the task's locked protocol for system prompt consistency. + // This ensures the system prompt matches the protocol the task was started with, + // even if user settings have changed since then. + const toolProtocol = resolveToolProtocol( + apiConfiguration ?? this.apiConfiguration, + modelInfo, + this._taskToolProtocol, + ) return SYSTEM_PROMPT( provider.context, @@ -3451,8 +3503,8 @@ export class Task extends EventEmitter implements TaskLike { ) // Determine if we're using native tool protocol for proper message handling - const protocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - const useNativeTools = isNativeProtocol(protocol) + // Use the task's locked protocol, NOT the current settings + const useNativeTools = isNativeProtocol(this._taskToolProtocol ?? "xml") // Send condenseTaskContextStarted to show in-progress indicator await this.providerRef.deref()?.postMessageToWebview({ type: "condenseTaskContextStarted", text: this.taskId }) @@ -3595,9 +3647,8 @@ export class Task extends EventEmitter implements TaskLike { const currentProfileId = this.getCurrentProfileId(state) // Determine if we're using native tool protocol for proper message handling - const modelInfoForProtocol = this.api.getModel().info - const protocol = resolveToolProtocol(this.apiConfiguration, modelInfoForProtocol) - const useNativeTools = isNativeProtocol(protocol) + // Use the task's locked protocol, NOT the current settings + const useNativeTools = isNativeProtocol(this._taskToolProtocol ?? "xml") // Check if context management will likely run (threshold check) // This allows us to show an in-progress indicator to the user @@ -3722,11 +3773,13 @@ export class Task extends EventEmitter implements TaskLike { } // Determine if we should include native tools based on: - // 1. Tool protocol is set to NATIVE + // 1. Task's locked tool protocol is set to NATIVE // 2. Model supports native tools + // CRITICAL: Use the task's locked protocol to ensure tasks that started with XML + // tools continue using XML even if NTC settings have since changed. const modelInfo = this.api.getModel().info - const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo) - const shouldIncludeTools = toolProtocol === TOOL_PROTOCOL.NATIVE && (modelInfo.supportsNativeTools ?? false) + const taskProtocol = this._taskToolProtocol ?? "xml" + const shouldIncludeTools = taskProtocol === TOOL_PROTOCOL.NATIVE && (modelInfo.supportsNativeTools ?? false) // Build complete tools array: native tools + dynamic MCP tools, filtered by mode restrictions let allTools: OpenAI.Chat.ChatCompletionTool[] = [] @@ -3760,7 +3813,12 @@ export class Task extends EventEmitter implements TaskLike { suppressPreviousResponseId: this.skipPrevResponseIdOnce, // Include tools and tool protocol when using native protocol and model supports it ...(shouldIncludeTools - ? { tools: allTools, tool_choice: "auto", toolProtocol, parallelToolCalls: parallelToolCallsEnabled } + ? { + tools: allTools, + tool_choice: "auto", + toolProtocol: taskProtocol, + parallelToolCalls: parallelToolCallsEnabled, + } : {}), } @@ -4164,6 +4222,16 @@ export class Task extends EventEmitter implements TaskLike { return this.workspacePath } + /** + * Get the tool protocol locked to this task. + * Returns undefined only if the task hasn't been fully initialized yet. + * + * @see {@link _taskToolProtocol} for lifecycle details + */ + public get taskToolProtocol() { + return this._taskToolProtocol + } + /** * Provides convenient access to high-level message operations. * Uses lazy initialization - the MessageManager is only created when first accessed. diff --git a/src/core/tools/MultiApplyDiffTool.ts b/src/core/tools/MultiApplyDiffTool.ts index 7e076d27a9a..94cdb3fd492 100644 --- a/src/core/tools/MultiApplyDiffTool.ts +++ b/src/core/tools/MultiApplyDiffTool.ts @@ -62,7 +62,8 @@ export async function applyDiffTool( removeClosingTag: RemoveClosingTag, ) { // Check if native protocol is enabled - if so, always use single-file class-based tool - const toolProtocol = resolveToolProtocol(cline.apiConfiguration, cline.api.getModel().info) + // Use the task's locked protocol for consistency throughout the task lifetime + const toolProtocol = resolveToolProtocol(cline.apiConfiguration, cline.api.getModel().info, cline.taskToolProtocol) if (isNativeProtocol(toolProtocol)) { return applyDiffToolClass.handle(cline, block as ToolUse<"apply_diff">, { askApproval, @@ -736,11 +737,15 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""} } } - // Check protocol for notice formatting - const toolProtocol = resolveToolProtocol(cline.apiConfiguration, cline.api.getModel().info) + // Check protocol for notice formatting - reuse the task's locked protocol + const noticeProtocol = resolveToolProtocol( + cline.apiConfiguration, + cline.api.getModel().info, + cline.taskToolProtocol, + ) const singleBlockNotice = totalSearchBlocks === 1 - ? isNativeProtocol(toolProtocol) + ? isNativeProtocol(noticeProtocol) ? "\n" + JSON.stringify({ notice: "Making multiple related changes in a single apply_diff is more efficient. If other changes are needed in this file, please include them as additional SEARCH/REPLACE blocks.", diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts index 17282df7744..58f3cc495e1 100644 --- a/src/core/tools/ReadFileTool.ts +++ b/src/core/tools/ReadFileTool.ts @@ -110,7 +110,8 @@ export class ReadFileTool extends BaseTool<"read_file"> { const { handleError, pushToolResult, toolProtocol } = callbacks const fileEntries = params.files const modelInfo = task.api.getModel().info - const protocol = resolveToolProtocol(task.apiConfiguration, modelInfo) + // Use the task's locked protocol for consistent output formatting throughout the task + const protocol = resolveToolProtocol(task.apiConfiguration, modelInfo, task.taskToolProtocol) const useNative = isNativeProtocol(protocol) if (!fileEntries || fileEntries.length === 0) { diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index 983307d9198..81e4982124a 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -326,8 +326,8 @@ export class DiffViewProvider { await task.say("user_feedback_diff", JSON.stringify(say)) } - // Check which protocol we're using - const toolProtocol = resolveToolProtocol(task.apiConfiguration, task.api.getModel().info) + // Check which protocol we're using - use the task's locked protocol for consistency + const toolProtocol = resolveToolProtocol(task.apiConfiguration, task.api.getModel().info, task.taskToolProtocol) const useNative = isNativeProtocol(toolProtocol) // Build notices array diff --git a/src/utils/__tests__/resolveToolProtocol.spec.ts b/src/utils/__tests__/resolveToolProtocol.spec.ts index 783b282ca01..5fbc5344382 100644 --- a/src/utils/__tests__/resolveToolProtocol.spec.ts +++ b/src/utils/__tests__/resolveToolProtocol.spec.ts @@ -1,7 +1,8 @@ import { describe, it, expect } from "vitest" -import { resolveToolProtocol } from "../resolveToolProtocol" +import { resolveToolProtocol, detectToolProtocolFromHistory } from "../resolveToolProtocol" import { TOOL_PROTOCOL, openAiModelInfoSaneDefaults } from "@roo-code/types" import type { ProviderSettings, ModelInfo } from "@roo-code/types" +import type { Anthropic } from "@anthropic-ai/sdk" describe("resolveToolProtocol", () => { describe("Precedence Level 1: User Profile Setting", () => { @@ -218,6 +219,58 @@ describe("resolveToolProtocol", () => { }) }) + describe("Locked Protocol (Precedence Level 0)", () => { + it("should return lockedProtocol when provided, ignoring all other settings", () => { + const settings: ProviderSettings = { + toolProtocol: "xml", // User wants XML + apiProvider: "openai-native", + } + const modelInfo: ModelInfo = { + maxTokens: 4096, + contextWindow: 128000, + supportsPromptCache: false, + supportsNativeTools: true, + defaultToolProtocol: "xml", + } + // lockedProtocol overrides everything + const result = resolveToolProtocol(settings, modelInfo, "native") + expect(result).toBe(TOOL_PROTOCOL.NATIVE) + }) + + it("should return XML lockedProtocol even when model supports native", () => { + const settings: ProviderSettings = { + toolProtocol: "native", // User wants native + apiProvider: "anthropic", + } + const modelInfo: ModelInfo = { + maxTokens: 4096, + contextWindow: 128000, + supportsPromptCache: false, + supportsNativeTools: true, // Model supports native + defaultToolProtocol: "native", + } + // lockedProtocol forces XML + const result = resolveToolProtocol(settings, modelInfo, "xml") + expect(result).toBe(TOOL_PROTOCOL.XML) + }) + + it("should fall through to normal resolution when lockedProtocol is undefined", () => { + const settings: ProviderSettings = { + toolProtocol: "xml", + apiProvider: "anthropic", + } + const modelInfo: ModelInfo = { + maxTokens: 4096, + contextWindow: 128000, + supportsPromptCache: false, + supportsNativeTools: true, + } + // undefined lockedProtocol should use normal precedence + const result = resolveToolProtocol(settings, modelInfo, undefined) + expect(result).toBe(TOOL_PROTOCOL.XML) // User setting wins + }) + }) + describe("Edge Cases", () => { it("should handle missing provider name gracefully", () => { const settings: ProviderSettings = {} @@ -333,3 +386,223 @@ describe("resolveToolProtocol", () => { }) }) }) + +describe("detectToolProtocolFromHistory", () => { + // Helper type for API messages in tests + type ApiMessageForTest = Anthropic.MessageParam & { ts?: number } + + describe("Native Protocol Detection", () => { + it("should detect native protocol when tool_use block has an id", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Hello" }, + { + role: "assistant", + content: [ + { + type: "tool_use", + id: "toolu_01abc123", // Native protocol always has an ID + name: "read_file", + input: { path: "test.ts" }, + }, + ], + }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBe(TOOL_PROTOCOL.NATIVE) + }) + + it("should detect native protocol from the first tool_use block found", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "First message" }, + { role: "assistant", content: "Let me help you" }, + { role: "user", content: "Second message" }, + { + role: "assistant", + content: [ + { + type: "tool_use", + id: "toolu_first", + name: "read_file", + input: { path: "first.ts" }, + }, + ], + }, + { role: "user", content: "Third message" }, + { + role: "assistant", + content: [ + { + type: "tool_use", + id: "toolu_second", + name: "write_to_file", + input: { path: "second.ts", content: "test" }, + }, + ], + }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBe(TOOL_PROTOCOL.NATIVE) + }) + }) + + describe("XML Protocol Detection", () => { + it("should detect XML protocol when tool_use block has no id", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Hello" }, + { + role: "assistant", + content: [ + { + type: "tool_use", + // No id field - XML protocol tool calls never have an ID + name: "read_file", + input: { path: "test.ts" }, + } as Anthropic.ToolUseBlock, // Cast to bypass type check for missing id + ], + }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBe(TOOL_PROTOCOL.XML) + }) + + it("should detect XML protocol when id is empty string", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Hello" }, + { + role: "assistant", + content: [ + { + type: "tool_use", + id: "", // Empty string should be treated as no id + name: "read_file", + input: { path: "test.ts" }, + }, + ], + }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBe(TOOL_PROTOCOL.XML) + }) + }) + + describe("No Tool Calls", () => { + it("should return undefined when no messages", () => { + const messages: ApiMessageForTest[] = [] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBeUndefined() + }) + + it("should return undefined when only user messages", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Hello" }, + { role: "user", content: "How are you?" }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBeUndefined() + }) + + it("should return undefined when assistant messages have no tool_use", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Hello" }, + { role: "assistant", content: "Hi! How can I help?" }, + { role: "user", content: "What's the weather?" }, + { + role: "assistant", + content: [{ type: "text", text: "I don't have access to weather data." }], + }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBeUndefined() + }) + + it("should return undefined when content is string", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Hello" }, + { role: "assistant", content: "Hi there!" }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBeUndefined() + }) + }) + + describe("Mixed Content", () => { + it("should detect protocol from tool_use even with mixed content", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Read this file" }, + { + role: "assistant", + content: [ + { type: "text", text: "I'll read that file for you." }, + { + type: "tool_use", + id: "toolu_mixed", + name: "read_file", + input: { path: "test.ts" }, + }, + ], + }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBe(TOOL_PROTOCOL.NATIVE) + }) + + it("should skip user messages and only check assistant messages", () => { + const messages: ApiMessageForTest[] = [ + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "toolu_user", + content: "result", + }, + ], + }, + { + role: "assistant", + content: [ + { + type: "tool_use", + id: "toolu_assistant", + name: "write_to_file", + input: { path: "out.ts", content: "test" }, + }, + ], + }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBe(TOOL_PROTOCOL.NATIVE) + }) + }) + + describe("Edge Cases", () => { + it("should handle messages with empty content array", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Hello" }, + { role: "assistant", content: [] }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBeUndefined() + }) + + it("should handle messages with ts field (ApiMessage format)", () => { + const messages: ApiMessageForTest[] = [ + { role: "user", content: "Hello", ts: Date.now() }, + { + role: "assistant", + content: [ + { + type: "tool_use", + id: "toolu_with_ts", + name: "read_file", + input: { path: "test.ts" }, + }, + ], + ts: Date.now(), + }, + ] + const result = detectToolProtocolFromHistory(messages) + expect(result).toBe(TOOL_PROTOCOL.NATIVE) + }) + }) +}) diff --git a/src/utils/resolveToolProtocol.ts b/src/utils/resolveToolProtocol.ts index 35619546761..2f8ddea0c34 100644 --- a/src/utils/resolveToolProtocol.ts +++ b/src/utils/resolveToolProtocol.ts @@ -1,9 +1,20 @@ import { ToolProtocol, TOOL_PROTOCOL } from "@roo-code/types" import type { ProviderSettings, ModelInfo } from "@roo-code/types" +import type { Anthropic } from "@anthropic-ai/sdk" +import { findLast, findLastIndex } from "../shared/array" + +/** + * Represents an API message in the conversation history. + * This is a minimal type definition for the detection function. + */ +type ApiMessageForDetection = Anthropic.MessageParam & { + ts?: number +} /** * Resolve the effective tool protocol based on the precedence hierarchy: * + * 0. Locked Protocol (task-level lock, if provided - highest priority) * 1. User Preference - Per-Profile (explicit profile setting) * 2. Model Default (defaultToolProtocol in ModelInfo) * 3. Native Fallback (final fallback) @@ -12,9 +23,20 @@ import type { ProviderSettings, ModelInfo } from "@roo-code/types" * * @param providerSettings - The provider settings for the current profile * @param modelInfo - Optional model information containing capabilities + * @param lockedProtocol - Optional task-locked protocol that takes absolute precedence * @returns The resolved tool protocol (either "xml" or "native") */ -export function resolveToolProtocol(providerSettings: ProviderSettings, modelInfo?: ModelInfo): ToolProtocol { +export function resolveToolProtocol( + providerSettings: ProviderSettings, + modelInfo?: ModelInfo, + lockedProtocol?: ToolProtocol, +): ToolProtocol { + // 0. Locked Protocol - task-level lock takes absolute precedence + // This ensures tasks continue using their original protocol even if settings change + if (lockedProtocol) { + return lockedProtocol + } + // If model doesn't support native tools, return XML immediately // Treat undefined as unsupported (only allow native when explicitly true) if (modelInfo?.supportsNativeTools !== true) { @@ -34,3 +56,57 @@ export function resolveToolProtocol(providerSettings: ProviderSettings, modelInf // 3. Native Fallback return TOOL_PROTOCOL.NATIVE } + +/** + * Detect the tool protocol used in an existing conversation history. + * + * This function scans the API conversation history for tool_use blocks + * and determines which protocol was used based on their structure: + * + * - Native protocol: tool_use blocks ALWAYS have an `id` field + * - XML protocol: tool_use blocks NEVER have an `id` field + * + * This is critical for task resumption: if a task previously used tools + * with a specific protocol, we must continue using that protocol even + * if the user's NTC settings have changed. + * + * The function searches from the most recent message backwards to find + * the last tool call, which represents the task's current protocol state. + * + * @param messages - The API conversation history to scan + * @returns The detected protocol, or undefined if no tool calls were found + */ +export function detectToolProtocolFromHistory(messages: ApiMessageForDetection[]): ToolProtocol | undefined { + // Find the last assistant message that contains a tool_use block + const lastAssistantWithTool = findLast(messages, (message) => { + if (message.role !== "assistant") { + return false + } + const content = message.content + if (!Array.isArray(content)) { + return false + } + return content.some((block) => block.type === "tool_use") + }) + + if (!lastAssistantWithTool) { + return undefined + } + + // Find the last tool_use block in that message's content + const content = lastAssistantWithTool.content as Anthropic.ContentBlock[] + const lastToolUseIndex = findLastIndex(content, (block) => block.type === "tool_use") + + if (lastToolUseIndex === -1) { + return undefined + } + + const lastToolUse = content[lastToolUseIndex] + + // The presence or absence of `id` determines the protocol: + // - Native protocol tool calls ALWAYS have an ID (set when parsed from tool_call chunks) + // - XML protocol tool calls NEVER have an ID (parsed from XML text) + // This pattern is used in presentAssistantMessage.ts:497-500 + const hasId = "id" in lastToolUse && !!lastToolUse.id + return hasId ? TOOL_PROTOCOL.NATIVE : TOOL_PROTOCOL.XML +}