refactor: centralize toolProtocol configuration checks (#9279)

* refactor: centralize toolProtocol configuration checks

- Created src/utils/toolProtocol.ts with getToolProtocolFromSettings() utility
- Replaced all direct vscode.workspace.getConfiguration() calls with centralized utility
- Updated 6 files to use the new utility function
- All tests pass and TypeScript compilation succeeds

* refactor: use isNativeProtocol function from types package
This commit is contained in:
Daniel 2025-11-14 20:24:40 -05:00 committed by GitHub
parent 64b1b62f5f
commit 0a305310e5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 34 additions and 40 deletions

View file

@ -38,8 +38,8 @@ import { Task } from "../task/Task"
import { codebaseSearchTool } from "../tools/CodebaseSearchTool"
import { experiments, EXPERIMENT_IDS } from "../../shared/experiments"
import { applyDiffTool as applyDiffToolClass } from "../tools/ApplyDiffTool"
import * as vscode from "vscode"
import { ToolProtocol, isNativeProtocol } from "@roo-code/types"
import { isNativeProtocol } from "@roo-code/types"
import { getToolProtocolFromSettings } from "../../utils/toolProtocol"
/**
* Processes and presents assistant message content to the user interface.
@ -279,10 +279,7 @@ export async function presentAssistantMessage(cline: Task) {
const pushToolResult = (content: ToolResponse) => {
// Check if we're using native tool protocol
const toolProtocol = vscode.workspace
.getConfiguration(Package.name)
.get<ToolProtocol>("toolProtocol", "xml")
const isNative = isNativeProtocol(toolProtocol)
const isNative = isNativeProtocol(getToolProtocolFromSettings())
// Get the tool call ID if this is a native tool call
const toolCallId = (block as any).id
@ -503,10 +500,7 @@ export async function presentAssistantMessage(cline: Task) {
await checkpointSaveAndMark(cline)
// Check if native protocol is enabled - if so, always use single-file class-based tool
const toolProtocol = vscode.workspace
.getConfiguration(Package.name)
.get<ToolProtocol>("toolProtocol", "xml")
if (isNativeProtocol(toolProtocol)) {
if (isNativeProtocol(getToolProtocolFromSettings())) {
await applyDiffToolClass.handle(cline, block as ToolUse<"apply_diff">, {
askApproval,
handleError,

View file

@ -3,9 +3,8 @@ import * as path from "path"
import * as diff from "diff"
import { RooIgnoreController, LOCK_TEXT_SYMBOL } from "../ignore/RooIgnoreController"
import { RooProtectedController } from "../protect/RooProtectedController"
import * as vscode from "vscode"
import { ToolProtocol, isNativeProtocol } from "@roo-code/types"
import { Package } from "../../shared/package"
import { getToolProtocolFromSettings } from "../../utils/toolProtocol"
export const formatResponse = {
toolDenied: () => `The user denied this operation.`,
@ -250,7 +249,6 @@ Always ensure you provide all required parameters for the tool you wish to use.`
* @returns The tool use instructions reminder text
*/
function getToolInstructionsReminder(protocol?: ToolProtocol): string {
const effectiveProtocol =
protocol ?? vscode.workspace.getConfiguration(Package.name).get<ToolProtocol>("toolProtocol", "xml")
const effectiveProtocol = protocol ?? getToolProtocolFromSettings()
return isNativeProtocol(effectiveProtocol) ? toolUseInstructionsReminderNative : toolUseInstructionsReminder
}

View file

@ -42,10 +42,10 @@ import {
MAX_CHECKPOINT_TIMEOUT_SECONDS,
MIN_CHECKPOINT_TIMEOUT_SECONDS,
TOOL_PROTOCOL,
ToolProtocol,
} from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"
import { CloudService, BridgeOrchestrator } from "@roo-code/cloud"
import { getToolProtocolFromSettings } from "../../utils/toolProtocol"
// api
import { ApiHandler, ApiHandlerCreateMessageMetadata, buildApiHandler } from "../../api"
@ -408,8 +408,7 @@ 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.
const toolProtocol = vscode.workspace.getConfiguration(Package.name).get<ToolProtocol>("toolProtocol", "xml")
this.assistantMessageParser = toolProtocol === "xml" ? new AssistantMessageParser() : undefined
this.assistantMessageParser = getToolProtocolFromSettings() === "xml" ? new AssistantMessageParser() : undefined
this.messageQueueService = new MessageQueueService()
@ -2416,16 +2415,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
const parsedBlocks = this.assistantMessageParser.getContentBlocks()
// Check if we're using native protocol
const toolProtocol = vscode.workspace
.getConfiguration(Package.name)
.get<ToolProtocol>("toolProtocol", "xml")
const isNative = isNativeProtocol(toolProtocol)
const isNative = isNativeProtocol(getToolProtocolFromSettings())
if (isNative) {
// For native protocol: Preserve tool_use blocks that were added via tool_call chunks
// These are added directly to assistantMessageContent and have an 'id' property
const nativeToolBlocks = this.assistantMessageContent.filter(
(block): block is ToolUse<any> => block.type === "tool_use" && (block as any).id !== undefined,
(block): block is ToolUse<any> =>
block.type === "tool_use" && (block as any).id !== undefined,
)
// Merge: parser blocks (text) + native tool blocks (tools with IDs)
this.assistantMessageContent = [...parsedBlocks, ...nativeToolBlocks]
@ -2580,12 +2577,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).
const toolProtocol = vscode.workspace
.getConfiguration(Package.name)
.get<ToolProtocol>("toolProtocol", "xml")
const isNativeProtocol = toolProtocol === TOOL_PROTOCOL.NATIVE
if (isNativeProtocol && this.apiConversationHistory.length > 0) {
if (isNativeProtocol(getToolProtocolFromSettings()) && 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
@ -2647,7 +2639,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
} else {
// User declined to retry
// For native protocol, re-add the user message we removed
if (isNativeProtocol) {
if (isNativeProtocol(getToolProtocolFromSettings())) {
await this.addToApiConversationHistory({
role: "user",
content: currentUserContent,
@ -2770,9 +2762,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
newTaskRequireTodos: vscode.workspace
.getConfiguration(Package.name)
.get<boolean>("newTaskRequireTodos", false),
toolProtocol: vscode.workspace
.getConfiguration(Package.name)
.get<ToolProtocol>("toolProtocol", "xml"),
toolProtocol: getToolProtocolFromSettings(),
},
undefined, // todoList
this.api.getModel().id,
@ -2982,7 +2972,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// Determine if we should include native tools based on:
// 1. Tool protocol is set to NATIVE
// 2. Model supports native tools
const toolProtocol = vscode.workspace.getConfiguration(Package.name).get<ToolProtocol>("toolProtocol", "xml")
const toolProtocol = getToolProtocolFromSettings()
const modelInfo = this.api.getModel().info
const shouldIncludeTools = toolProtocol === TOOL_PROTOCOL.NATIVE && (modelInfo.supportsNativeTools ?? false)

View file

@ -16,9 +16,8 @@ import { parseXmlForDiff } from "../../utils/xml"
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { applyDiffTool as applyDiffToolClass } from "./ApplyDiffTool"
import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
import * as vscode from "vscode"
import { ToolProtocol, isNativeProtocol } from "@roo-code/types"
import { Package } from "../../shared/package"
import { isNativeProtocol } from "@roo-code/types"
import { getToolProtocolFromSettings } from "../../utils/toolProtocol"
interface DiffOperation {
path: string
@ -63,8 +62,7 @@ export async function applyDiffTool(
removeClosingTag: RemoveClosingTag,
) {
// Check if native protocol is enabled - if so, always use single-file class-based tool
const toolProtocol = vscode.workspace.getConfiguration(Package.name).get<ToolProtocol>("toolProtocol", "xml")
if (isNativeProtocol(toolProtocol)) {
if (isNativeProtocol(getToolProtocolFromSettings())) {
return applyDiffToolClass.handle(cline, block as ToolUse<"apply_diff">, {
askApproval,
handleError,

View file

@ -7,8 +7,8 @@ import { experiments as experimentsModule, EXPERIMENT_IDS } from "../../shared/e
import { SYSTEM_PROMPT } from "../prompts/system"
import { MultiSearchReplaceDiffStrategy } from "../diff/strategies/multi-search-replace"
import { MultiFileSearchReplaceDiffStrategy } from "../diff/strategies/multi-file-search-replace"
import { ToolProtocol } from "@roo-code/types"
import { Package } from "../../shared/package"
import { getToolProtocolFromSettings } from "../../utils/toolProtocol"
import { ClineProvider } from "./ClineProvider"
@ -93,7 +93,7 @@ export const generateSystemPrompt = async (provider: ClineProvider, message: Web
newTaskRequireTodos: vscode.workspace
.getConfiguration(Package.name)
.get<boolean>("newTaskRequireTodos", false),
toolProtocol: vscode.workspace.getConfiguration(Package.name).get<ToolProtocol>("toolProtocol", "xml"),
toolProtocol: getToolProtocolFromSettings(),
},
)

14
src/utils/toolProtocol.ts Normal file
View file

@ -0,0 +1,14 @@
import * as vscode from "vscode"
import { ToolProtocol } from "@roo-code/types"
import { Package } from "../shared/package"
/**
* Get the tool protocol setting from VSCode configuration.
* This centralizes the logic for retrieving the toolProtocol setting,
* ensuring consistent behavior across the codebase.
*
* @returns The configured tool protocol, defaults to "xml" if not set
*/
export function getToolProtocolFromSettings(): ToolProtocol {
return vscode.workspace.getConfiguration(Package.name).get<ToolProtocol>("toolProtocol", "xml")
}