scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-09-05 08:10:14 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: cache tool protocol to prevent race conditions during profile changes

Browse source
This commit is contained in:
daniel-lxs 2025-11-19 12:05:28 -05:00
parent 18c4d1ac41
commit a84656d097
No known key found for this signature in database
GPG key ID: 21C74479048B3AA6
7 changed files with 68 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/core/assistant-message/presentAssistantMessage.ts
View file

@ -39,7 +39,6 @@ import { codebaseSearchTool } from "../tools/CodebaseSearchTool"
import { experiments, EXPERIMENT_IDS } from "../../shared/experiments"
import { applyDiffTool as applyDiffToolClass } from "../tools/ApplyDiffTool"
import { isNativeProtocol } from "@roo-code/types"
import { resolveToolProtocol } from "../../utils/resolveToolProtocol"
/**
* Processes and presents assistant message content to the user interface.

96
src/core/task/Task.ts
View file

@ -27,6 +27,7 @@ import {
type ToolProgressStatus,
type HistoryItem,
type CreateTaskOptions,
type ToolProtocol,
RooCodeEventName,
TelemetryEventName,
TaskStatus,
@ -235,6 +236,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
private static lastGlobalApiRequestTime?: number
private autoApprovalHandler: AutoApprovalHandler
/**
* Cached tool protocol resolved from API configuration and model info.
* Updated whenever the API configuration changes to prevent race conditions.
* @private
*/
private _cachedToolProtocol: ToolProtocol
/**
* Reset the global API request timestamp. This should only be used for testing.
* @internal
@ -380,6 +388,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
this.api = buildApiHandler(apiConfiguration)
this.autoApprovalHandler = new AutoApprovalHandler()
// Initialize cached tool protocol to prevent race conditions
this._cachedToolProtocol = resolveToolProtocol(this.apiConfiguration, this.api.getModel().info)
this.urlContentFetcher = new UrlContentFetcher(provider.context)
this.browserSession = new BrowserSession(provider.context)
this.diffEnabled = enableDiff
@ -411,9 +422,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// Initialize the assistant message parser only 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)
const toolProtocol = resolveToolProtocol(this.apiConfiguration, this.api.getModel().info)
this.assistantMessageParser = toolProtocol !== "native" ? new AssistantMessageParser() : undefined
// Use cached protocol instead of resolving again
this.assistantMessageParser = this._cachedToolProtocol !== "native" ? new AssistantMessageParser() : undefined
this.messageQueueService = new MessageQueueService()
@ -619,6 +629,17 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
return this._taskMode
}
/**
* Get the cached tool protocol. This is synchronized with API configuration changes
* to prevent race conditions when switching profiles or models.
*
* @returns The current tool protocol (either "xml" or "native")
* @public
*/
public get toolProtocol(): ToolProtocol {
return this._cachedToolProtocol
}
static create(options: TaskOptions): [Task, Promise<void>] {
const instance = new Task({ ...options, startTask: false })
const { images, task, historyItem } = options
@ -1093,16 +1114,18 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
* @param newApiConfiguration - The new API configuration to use
*/
public async updateApiConfiguration(newApiConfiguration: ProviderSettings): Promise<void> {
// Determine the previous protocol before updating
const previousProtocol = this.apiConfiguration
? resolveToolProtocol(this.apiConfiguration, this.api.getModel().info)
: undefined
// Store previous protocol for comparison
const previousProtocol = this._cachedToolProtocol
this.apiConfiguration = newApiConfiguration
this.api = buildApiHandler(newApiConfiguration)
// Determine the new tool protocol
const newProtocol = resolveToolProtocol(this.apiConfiguration, this.api.getModel().info)
// Update cached tool protocol
this._cachedToolProtocol = resolveToolProtocol(this.apiConfiguration, this.api.getModel().info)
console.log(
`[Task#${this.taskId}.${this.instanceId}] API configuration updated, new tool protocol: ${this._cachedToolProtocol}`,
)
const newProtocol = this._cachedToolProtocol
const shouldUseXmlParser = newProtocol === "xml"
// Only make changes if the protocol actually changed
@ -1368,10 +1391,7 @@ export class Task extends EventEmitter<TaskEvents> 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))
return formatResponse.toolError(formatResponse.missingToolParameterError(paramName, this.toolProtocol))
}
// Lifecycle
@ -1513,9 +1533,7 @@ export class Task extends EventEmitter<TaskEvents> 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)
const useNative = isNativeProtocol(this.toolProtocol)
// Only convert tool blocks to text for XML protocol
// For native protocol, the API expects proper tool_use/tool_result structure
@ -1524,9 +1542,9 @@ export class Task extends EventEmitter<TaskEvents> 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 cached protocol
const params = block.input as Record<string, any>
const formattedText = formatToolInvocation(block.name, params, protocol)
const formattedText = formatToolInvocation(block.name, params, this.toolProtocol)
return {
type: "text",
@ -1903,10 +1921,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// the user hits max requests and denies resetting the count.
break
} else {
const modelInfo = this.api.getModel().info
const state = await this.providerRef.deref()?.getState()
const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo)
nextUserContent = [{ type: "text", text: formatResponse.noToolsUsed(toolProtocol) }]
nextUserContent = [{ type: "text", text: formatResponse.noToolsUsed(this.toolProtocol) }]
this.consecutiveMistakeCount++
}
}
@ -2137,9 +2152,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
await this.diffViewProvider.reset()
// Determine protocol once per API request to avoid repeated calls in the streaming loop
const streamProtocol = resolveToolProtocol(this.apiConfiguration, this.api.getModel().info)
const shouldUseXmlParser = streamProtocol === "xml"
// Use cached protocol to avoid race conditions during streaming
const shouldUseXmlParser = this.toolProtocol === "xml"
// Yields only if the first chunk is successful, otherwise will
// allow the user to retry the request (most likely due to rate
@ -2680,10 +2694,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
const didToolUse = this.assistantMessageContent.some((block) => block.type === "tool_use")
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) })
this.userMessageContent.push({
type: "text",
text: formatResponse.noToolsUsed(this.toolProtocol),
})
this.consecutiveMistakeCount++
}
@ -2707,11 +2721,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// apiConversationHistory at line 1876. Since the assistant failed to respond,
// 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
) {
if (isNativeProtocol(this.toolProtocol) && 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
@ -2720,7 +2730,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
}
// Check if we should auto-retry or prompt the user
// Reuse the state variable from above
const state = await this.providerRef.deref()?.getState()
if (state?.autoApprovalEnabled && state?.alwaysApproveResubmit) {
// Auto-retry with backoff - don't persist failure message when retrying
const errorMsg =
@ -2773,10 +2783,7 @@ export class Task extends EventEmitter<TaskEvents> 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))
) {
if (isNativeProtocol(this.toolProtocol)) {
await this.addToApiConversationHistory({
role: "user",
content: currentUserContent,
@ -2873,9 +2880,6 @@ export class Task extends EventEmitter<TaskEvents> 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)
return SYSTEM_PROMPT(
provider.context,
this.cwd,
@ -2902,7 +2906,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
newTaskRequireTodos: vscode.workspace
.getConfiguration(Package.name)
.get<boolean>("newTaskRequireTodos", false),
toolProtocol,
toolProtocol: this.toolProtocol,
},
undefined, // todoList
this.api.getModel().id,
@ -3113,8 +3117,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// 1. Tool protocol is set to NATIVE
// 2. Model supports native tools
const modelInfo = this.api.getModel().info
const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo)
const shouldIncludeTools = toolProtocol === TOOL_PROTOCOL.NATIVE && (modelInfo.supportsNativeTools ?? false)
const shouldIncludeTools =
this.toolProtocol === TOOL_PROTOCOL.NATIVE && (modelInfo.supportsNativeTools ?? false)
// Build complete tools array: native tools + dynamic MCP tools, filtered by mode restrictions
let allTools: OpenAI.Chat.ChatCompletionTool[] = []
@ -3154,7 +3158,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
mode: mode,
taskId: this.taskId,
// Include tools and tool protocol when using native protocol and model supports it
...(shouldIncludeTools ? { tools: allTools, tool_choice: "auto", toolProtocol } : {}),
...(shouldIncludeTools ? { tools: allTools, tool_choice: "auto", toolProtocol: this.toolProtocol } : {}),
}
// The provider accepts reasoning items alongside standard messages; cast to the expected parameter type.

11
src/core/tools/MultiApplyDiffTool.ts
View file

@ -17,7 +17,6 @@ import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { applyDiffTool as applyDiffToolClass } from "./ApplyDiffTool"
import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
import { isNativeProtocol } from "@roo-code/types"
import { resolveToolProtocol } from "../../utils/resolveToolProtocol"
interface DiffOperation {
path: string
@ -62,14 +61,13 @@ 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)
if (isNativeProtocol(toolProtocol)) {
if (isNativeProtocol(cline.toolProtocol)) {
return applyDiffToolClass.handle(cline, block as ToolUse<"apply_diff">, {
askApproval,
handleError,
pushToolResult,
removeClosingTag,
toolProtocol,
toolProtocol: cline.toolProtocol,
})
}
@ -89,7 +87,7 @@ export async function applyDiffTool(
handleError,
pushToolResult,
removeClosingTag,
toolProtocol,
toolProtocol: cline.toolProtocol,
})
}
}
@ -737,10 +735,9 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
}
// Check protocol for notice formatting
const toolProtocol = resolveToolProtocol(cline.apiConfiguration, cline.api.getModel().info)
const singleBlockNotice =
totalSearchBlocks === 1
? isNativeProtocol(toolProtocol)
? isNativeProtocol(cline.toolProtocol)
? "\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.",

4
src/core/tools/ReadFileTool.ts
View file

@ -15,7 +15,6 @@ import { readLines } from "../../integrations/misc/read-lines"
import { extractTextFromFile, addLineNumbers, getSupportedBinaryFormats } from "../../integrations/misc/extract-text"
import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter"
import { parseXml } from "../../utils/xml"
import { resolveToolProtocol } from "../../utils/resolveToolProtocol"
import {
DEFAULT_MAX_IMAGE_FILE_SIZE_MB,
DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB,
@ -109,8 +108,7 @@ 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)
const useNative = isNativeProtocol(protocol)
const useNative = isNativeProtocol(task.toolProtocol)
if (!fileEntries || fileEntries.length === 0) {
task.consecutiveMistakeCount++

5
src/core/tools/WriteToFileTool.ts
View file

@ -18,7 +18,6 @@ import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
import { BaseTool, ToolCallbacks } from "./BaseTool"
import type { ToolUse } from "../../shared/tools"
import { resolveToolProtocol } from "../../utils/resolveToolProtocol"
interface WriteToFileParams {
path: string
@ -110,8 +109,6 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
const actualLineCount = newContent.split("\n").length
const isNewFile = !fileExists
const diffStrategyEnabled = !!task.diffStrategy
const modelInfo = task.api.getModel().info
const toolProtocol = resolveToolProtocol(task.apiConfiguration, modelInfo)
await task.say(
"error",
@ -126,7 +123,7 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
actualLineCount,
isNewFile,
diffStrategyEnabled,
toolProtocol,
task.toolProtocol,
),
),
)

10
src/core/tools/__tests__/applyDiffTool.experiment.spec.ts
View file

@ -68,6 +68,10 @@ describe("applyDiffTool experiment routing", () => {
}),
},
processQueuedMessages: vi.fn(),
// Add toolProtocol getter that returns XML by default
get toolProtocol() {
return TOOL_PROTOCOL.XML
},
} as any
mockBlock = {
@ -170,6 +174,12 @@ describe("applyDiffTool experiment routing", () => {
},
})
// Update mockCline to return native protocol
Object.defineProperty(mockCline, "toolProtocol", {
get: () => TOOL_PROTOCOL.NATIVE,
configurable: true,
})
mockProvider.getState.mockResolvedValue({
experiments: {
[EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF]: true,

6
src/integrations/editor/DiffViewProvider.ts
View file

@ -13,7 +13,6 @@ import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
import { ClineSayTool } from "../../shared/ExtensionMessage"
import { Task } from "../../core/task/Task"
import { DEFAULT_WRITE_DELAY_MS, isNativeProtocol } from "@roo-code/types"
import { resolveToolProtocol } from "../../utils/resolveToolProtocol"
import { DecorationController } from "./DecorationController"
@ -326,9 +325,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)
const useNative = isNativeProtocol(toolProtocol)
// Check which protocol we're using (cached to avoid race conditions)
const useNative = isNativeProtocol(task.toolProtocol)
// Build notices array
const notices = [