diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 78d2afc166f..2e8b791b349 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -5,11 +5,14 @@ import { Anthropic } from "@anthropic-ai/sdk" import type { ToolName, ClineAsk, ToolProgressStatus } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" +import { t } from "../../i18n" + import { defaultModeSlug, getModeBySlug } from "../../shared/modes" import type { ToolParamName, ToolResponse, ToolUse, McpToolUse } from "../../shared/tools" -import { Package } from "../../shared/package" -import { t } from "../../i18n" +import { experiments, EXPERIMENT_IDS } from "../../shared/experiments" + import { AskIgnoredError } from "../task/AskIgnoredError" +import { Task } from "../task/Task" import { fetchInstructionsTool } from "../tools/FetchInstructionsTool" import { listFilesTool } from "../tools/ListFilesTool" @@ -30,17 +33,14 @@ import { askFollowupQuestionTool } from "../tools/AskFollowupQuestionTool" import { switchModeTool } from "../tools/SwitchModeTool" import { attemptCompletionTool, AttemptCompletionCallbacks } from "../tools/AttemptCompletionTool" import { newTaskTool } from "../tools/NewTaskTool" - import { updateTodoListTool } from "../tools/UpdateTodoListTool" import { runSlashCommandTool } from "../tools/RunSlashCommandTool" import { generateImageTool } from "../tools/GenerateImageTool" - -import { formatResponse } from "../prompts/responses" +import { applyDiffTool as applyDiffToolClass } from "../tools/ApplyDiffTool" import { validateToolUse } from "../tools/validateToolUse" -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 { formatResponse } from "../prompts/responses" /** * Processes and presents assistant message content to the user interface. @@ -353,7 +353,7 @@ export async function presentAssistantMessage(cline: Task) { case "tool_use": { // Fetch state early so it's available for toolDescription and validation const state = await cline.providerRef.deref()?.getState() - const { mode, customModes, experiments: stateExperiments, apiConfiguration } = state ?? {} + const { mode, customModes, experiments: stateExperiments } = state ?? {} const toolDescription = (): string => { switch (block.name) { @@ -731,6 +731,7 @@ export async function presentAssistantMessage(cline: Task) { // This prevents the stream from being interrupted with "Response interrupted by tool use result" // which would cause the extension to appear to hang const errorContent = formatResponse.toolError(error.message, toolProtocol) + if (toolProtocol === TOOL_PROTOCOL.NATIVE && toolCallId) { // For native protocol, push tool_result directly without setting didAlreadyUseTool cline.userMessageContent.push({ @@ -743,6 +744,7 @@ export async function presentAssistantMessage(cline: Task) { // For XML protocol, use the standard pushToolResult pushToolResult(errorContent) } + break } } diff --git a/src/core/environment/__tests__/getEnvironmentDetails.spec.ts b/src/core/environment/__tests__/getEnvironmentDetails.spec.ts index ef6d0513d47..65b447ff162 100644 --- a/src/core/environment/__tests__/getEnvironmentDetails.spec.ts +++ b/src/core/environment/__tests__/getEnvironmentDetails.spec.ts @@ -6,7 +6,8 @@ import type { Mock } from "vitest" import { getEnvironmentDetails } from "../getEnvironmentDetails" import { EXPERIMENT_IDS, experiments } from "../../../shared/experiments" -import { defaultModeSlug, getFullModeDetails, getModeBySlug, isToolAllowedForMode } from "../../../shared/modes" +import { getFullModeDetails } from "../../../shared/modes" +import { isToolAllowedForMode } from "../../tools/validateToolUse" import { getApiMetrics } from "../../../shared/getApiMetrics" import { listFiles } from "../../../services/glob/list-files" import { TerminalRegistry } from "../../../integrations/terminal/TerminalRegistry" @@ -51,6 +52,7 @@ vi.mock("../../../integrations/terminal/Terminal") vi.mock("../../../utils/path") vi.mock("../../../utils/git") vi.mock("../../prompts/responses") +vi.mock("../../tools/validateToolUse") describe("getEnvironmentDetails", () => { const mockCwd = "/test/path" diff --git a/src/core/environment/getEnvironmentDetails.ts b/src/core/environment/getEnvironmentDetails.ts index 9b01285ab8e..64cd93737fd 100644 --- a/src/core/environment/getEnvironmentDetails.ts +++ b/src/core/environment/getEnvironmentDetails.ts @@ -11,7 +11,7 @@ import { DEFAULT_TERMINAL_OUTPUT_CHARACTER_LIMIT } from "@roo-code/types" import { resolveToolProtocol } from "../../utils/resolveToolProtocol" import { EXPERIMENT_IDS, experiments as Experiments } from "../../shared/experiments" import { formatLanguage } from "../../shared/language" -import { defaultModeSlug, getFullModeDetails, getModeBySlug, isToolAllowedForMode } from "../../shared/modes" +import { defaultModeSlug, getFullModeDetails } from "../../shared/modes" import { getApiMetrics } from "../../shared/getApiMetrics" import { listFiles } from "../../services/glob/list-files" import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry" diff --git a/src/core/prompts/tools/filter-tools-for-mode.ts b/src/core/prompts/tools/filter-tools-for-mode.ts index 3c1b2e3676d..f296c1b5c50 100644 --- a/src/core/prompts/tools/filter-tools-for-mode.ts +++ b/src/core/prompts/tools/filter-tools-for-mode.ts @@ -1,10 +1,11 @@ import type OpenAI from "openai" import type { ModeConfig, ToolName, ToolGroup, ModelInfo } from "@roo-code/types" -import { getModeBySlug, getToolsForMode, isToolAllowedForMode } from "../../../shared/modes" +import { getModeBySlug, getToolsForMode } from "../../../shared/modes" import { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS, TOOL_ALIASES } from "../../../shared/tools" import { defaultModeSlug } from "../../../shared/modes" import type { CodeIndexManager } from "../../../services/code-index/manager" import type { McpHub } from "../../../services/mcp/McpHub" +import { isToolAllowedForMode } from "../../../core/tools/validateToolUse" /** * Reverse lookup map - maps alias name to canonical tool name. diff --git a/src/core/prompts/tools/index.ts b/src/core/prompts/tools/index.ts index e0993348ccc..c9a69efae83 100644 --- a/src/core/prompts/tools/index.ts +++ b/src/core/prompts/tools/index.ts @@ -1,15 +1,19 @@ import type { ToolName, ModeConfig } from "@roo-code/types" +import { shouldUseSingleFileRead } from "@roo-code/types" import { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS, DiffStrategy } from "../../../shared/tools" +import { Mode, getModeConfig, getGroupName } from "../../../shared/modes" + +import { isToolAllowedForMode } from "../../tools/validateToolUse" + import { McpHub } from "../../../services/mcp/McpHub" -import { Mode, getModeConfig, isToolAllowedForMode, getGroupName } from "../../../shared/modes" +import { CodeIndexManager } from "../../../services/code-index/manager" import { ToolArgs } from "./types" import { getExecuteCommandDescription } from "./execute-command" import { getReadFileDescription } from "./read-file" import { getSimpleReadFileDescription } from "./simple-read-file" import { getFetchInstructionsDescription } from "./fetch-instructions" -import { shouldUseSingleFileRead } from "@roo-code/types" import { getWriteToFileDescription } from "./write-to-file" import { getSearchFilesDescription } from "./search-files" import { getListFilesDescription } from "./list-files" @@ -24,7 +28,6 @@ import { getCodebaseSearchDescription } from "./codebase-search" import { getUpdateTodoListDescription } from "./update-todo-list" import { getRunSlashCommandDescription } from "./run-slash-command" import { getGenerateImageDescription } from "./generate-image" -import { CodeIndexManager } from "../../../services/code-index/manager" // Map of tool names to their description functions const toolDescriptionMap: Record string | undefined> = { diff --git a/src/core/task/__tests__/native-tools-filtering.spec.ts b/src/core/task/__tests__/native-tools-filtering.spec.ts index a32bd2cd0ce..761fe6e1ec2 100644 --- a/src/core/task/__tests__/native-tools-filtering.spec.ts +++ b/src/core/task/__tests__/native-tools-filtering.spec.ts @@ -1,4 +1,3 @@ -import { describe, it, expect, beforeEach, vi } from "vitest" import type { ModeConfig } from "@roo-code/types" describe("Native Tools Filtering by Mode", () => { @@ -23,7 +22,7 @@ describe("Native Tools Filtering by Mode", () => { } // Import the functions we need to test - const { isToolAllowedForMode } = await import("../../../shared/modes") + const { isToolAllowedForMode } = await import("../../tools/validateToolUse") const { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS } = await import("../../../shared/tools") // Test architect mode - should NOT have edit tools @@ -95,7 +94,7 @@ describe("Native Tools Filtering by Mode", () => { groups: ["read"] as const, } - const { isToolAllowedForMode } = await import("../../../shared/modes") + const { isToolAllowedForMode } = await import("../../tools/validateToolUse") // Mode with MCP group should allow use_mcp_tool expect(isToolAllowedForMode("use_mcp_tool", "test-mode-with-mcp", [modeWithMcp])).toBe(true) @@ -112,7 +111,7 @@ describe("Native Tools Filtering by Mode", () => { groups: [] as const, // No groups at all } - const { isToolAllowedForMode } = await import("../../../shared/modes") + const { isToolAllowedForMode } = await import("../../tools/validateToolUse") const { ALWAYS_AVAILABLE_TOOLS } = await import("../../../shared/tools") // Always-available tools should work even with no groups diff --git a/src/core/tools/__tests__/validateToolUse.spec.ts b/src/core/tools/__tests__/validateToolUse.spec.ts index 91f32c1bf81..87aa1594208 100644 --- a/src/core/tools/__tests__/validateToolUse.spec.ts +++ b/src/core/tools/__tests__/validateToolUse.spec.ts @@ -2,10 +2,10 @@ import type { ModeConfig } from "@roo-code/types" -import { isToolAllowedForMode, modes } from "../../../shared/modes" +import { modes } from "../../../shared/modes" import { TOOL_GROUPS } from "../../../shared/tools" -import { validateToolUse } from "../validateToolUse" +import { validateToolUse, isToolAllowedForMode } from "../validateToolUse" const codeMode = modes.find((m) => m.slug === "code")?.slug || "code" const architectMode = modes.find((m) => m.slug === "architect")?.slug || "architect" diff --git a/src/core/tools/validateToolUse.ts b/src/core/tools/validateToolUse.ts index a4ddf2ab77f..d0570337414 100644 --- a/src/core/tools/validateToolUse.ts +++ b/src/core/tools/validateToolUse.ts @@ -1,7 +1,9 @@ -import type { ToolName, ModeConfig } from "@roo-code/types" +import type { ToolName, ModeConfig, ExperimentId, GroupOptions, GroupEntry } from "@roo-code/types" import { toolNames as validToolNames } from "@roo-code/types" -import { Mode, isToolAllowedForMode } from "../../shared/modes" +import { type Mode, FileRestrictionError, getModeBySlug, getGroupName } from "../../shared/modes" +import { EXPERIMENT_IDS } from "../../shared/experiments" +import { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS } from "../../shared/tools" /** * Checks if a tool name is a valid, known tool. @@ -14,7 +16,7 @@ export function isValidToolName(toolName: string): toolName is ToolName { return true } - // Check if it's a dynamic MCP tool (mcp_serverName_toolName format) + // Check if it's a dynamic MCP tool (mcp_serverName_toolName format). if (toolName.startsWith("mcp_")) { return true } @@ -54,3 +56,142 @@ export function validateToolUse( throw new Error(`Tool "${toolName}" is not allowed in ${mode} mode.`) } } + +const EDIT_OPERATION_PARAMS = ["diff", "content", "operations", "search", "replace", "args", "line"] as const + +function getGroupOptions(group: GroupEntry): GroupOptions | undefined { + return Array.isArray(group) ? group[1] : undefined +} + +function doesFileMatchRegex(filePath: string, pattern: string): boolean { + try { + const regex = new RegExp(pattern) + return regex.test(filePath) + } catch (error) { + console.error(`Invalid regex pattern: ${pattern}`, error) + return false + } +} + +export function isToolAllowedForMode( + tool: string, + modeSlug: string, + customModes: ModeConfig[], + toolRequirements?: Record, + toolParams?: Record, // All tool parameters + experiments?: Record, + includedTools?: string[], // Opt-in tools explicitly included (e.g., from modelInfo) +): boolean { + // Always allow these tools + if (ALWAYS_AVAILABLE_TOOLS.includes(tool as any)) { + return true + } + + // Check if this is a dynamic MCP tool (mcp_serverName_toolName) + // These should be allowed if the mcp group is allowed for the mode + const isDynamicMcpTool = tool.startsWith("mcp_") + + if (experiments && Object.values(EXPERIMENT_IDS).includes(tool as ExperimentId)) { + if (!experiments[tool]) { + return false + } + } + + // Check tool requirements if any exist + if (toolRequirements && typeof toolRequirements === "object") { + if (tool in toolRequirements && !toolRequirements[tool]) { + return false + } + } else if (toolRequirements === false) { + // If toolRequirements is a boolean false, all tools are disabled + return false + } + + const mode = getModeBySlug(modeSlug, customModes) + + if (!mode) { + return false + } + + // Check if tool is in any of the mode's groups and respects any group options + for (const group of mode.groups) { + const groupName = getGroupName(group) + const options = getGroupOptions(group) + + const groupConfig = TOOL_GROUPS[groupName] + + // Check if this is a dynamic MCP tool and the mcp group is allowed + if (isDynamicMcpTool && groupName === "mcp") { + // Dynamic MCP tools are allowed if the mcp group is in the mode's groups + return true + } + + // Check if the tool is in the group's regular tools + const isRegularTool = groupConfig.tools.includes(tool) + + // Check if the tool is a custom tool that has been explicitly included + const isCustomTool = groupConfig.customTools?.includes(tool) && includedTools?.includes(tool) + + // If the tool isn't in regular tools and isn't an included custom tool, continue to next group + if (!isRegularTool && !isCustomTool) { + continue + } + + // If there are no options, allow the tool + if (!options) { + return true + } + + // For the edit group, check file regex if specified + if (groupName === "edit" && options.fileRegex) { + const filePath = toolParams?.path + // Check if this is an actual edit operation (not just path-only for streaming) + const isEditOperation = EDIT_OPERATION_PARAMS.some((param) => toolParams?.[param]) + + // Handle single file path validation + if (filePath && isEditOperation && !doesFileMatchRegex(filePath, options.fileRegex)) { + throw new FileRestrictionError(mode.name, options.fileRegex, options.description, filePath, tool) + } + + // Handle XML args parameter (used by MULTI_FILE_APPLY_DIFF experiment) + if (toolParams?.args && typeof toolParams.args === "string") { + // Extract file paths from XML args with improved validation + try { + const filePathMatches = toolParams.args.match(/([^<]+)<\/path>/g) + if (filePathMatches) { + for (const match of filePathMatches) { + // More robust path extraction with validation + const pathMatch = match.match(/([^<]+)<\/path>/) + if (pathMatch && pathMatch[1]) { + const extractedPath = pathMatch[1].trim() + // Validate that the path is not empty and doesn't contain invalid characters + if (extractedPath && !extractedPath.includes("<") && !extractedPath.includes(">")) { + if (!doesFileMatchRegex(extractedPath, options.fileRegex)) { + throw new FileRestrictionError( + mode.name, + options.fileRegex, + options.description, + extractedPath, + tool, + ) + } + } + } + } + } + } catch (error) { + // Re-throw FileRestrictionError as it's an expected validation error + if (error instanceof FileRestrictionError) { + throw error + } + // If XML parsing fails, log the error but don't block the operation + console.warn(`Failed to parse XML args for file restriction validation: ${error}`) + } + } + } + + return true + } + + return false +} diff --git a/src/shared/__tests__/modes.spec.ts b/src/shared/__tests__/modes.spec.ts index 4e5251eca73..a00abde7879 100644 --- a/src/shared/__tests__/modes.spec.ts +++ b/src/shared/__tests__/modes.spec.ts @@ -9,7 +9,8 @@ vi.mock("../../core/prompts/sections/custom-instructions", () => ({ addCustomInstructions: vi.fn().mockResolvedValue("Combined instructions"), })) -import { isToolAllowedForMode, FileRestrictionError, getFullModeDetails, modes, getModeSelection } from "../modes" +import { FileRestrictionError, getFullModeDetails, modes, getModeSelection } from "../modes" +import { isToolAllowedForMode } from "../../core/tools/validateToolUse" import { addCustomInstructions } from "../../core/prompts/sections/custom-instructions" describe("isToolAllowedForMode", () => { diff --git a/src/shared/modes.ts b/src/shared/modes.ts index ed7c5b4d581..a94aa47ed0b 100644 --- a/src/shared/modes.ts +++ b/src/shared/modes.ts @@ -1,11 +1,9 @@ import * as vscode from "vscode" import { - type GroupOptions, type GroupEntry, type ModeConfig, type CustomModePrompts, - type ExperimentId, type ToolGroup, type PromptComponent, DEFAULT_MODES, @@ -13,7 +11,6 @@ import { import { addCustomInstructions } from "../core/prompts/sections/custom-instructions" -import { EXPERIMENT_IDS } from "./experiments" import { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS } from "./tools" export type Mode = string @@ -27,22 +24,6 @@ export function getGroupName(group: GroupEntry): ToolGroup { return group[0] } -// Helper to get group options if they exist -function getGroupOptions(group: GroupEntry): GroupOptions | undefined { - return Array.isArray(group) ? group[1] : undefined -} - -// Helper to check if a file path matches a regex pattern -export function doesFileMatchRegex(filePath: string, pattern: string): boolean { - try { - const regex = new RegExp(pattern) - return regex.test(filePath) - } catch (error) { - console.error(`Invalid regex pattern: ${pattern}`, error) - return false - } -} - // Helper to get all tools for a mode export function getToolsForMode(groups: readonly GroupEntry[]): string[] { const tools = new Set() @@ -150,9 +131,6 @@ export function getModeSelection(mode: string, promptComponent?: PromptComponent } } -// Edit operation parameters that indicate an actual edit operation -const EDIT_OPERATION_PARAMS = ["diff", "content", "operations", "search", "replace", "args", "line"] as const - // Custom error class for file restrictions export class FileRestrictionError extends Error { constructor(mode: string, pattern: string, description: string | undefined, filePath: string, tool?: string) { @@ -164,127 +142,6 @@ export class FileRestrictionError extends Error { } } -export function isToolAllowedForMode( - tool: string, - modeSlug: string, - customModes: ModeConfig[], - toolRequirements?: Record, - toolParams?: Record, // All tool parameters - experiments?: Record, - includedTools?: string[], // Opt-in tools explicitly included (e.g., from modelInfo) -): boolean { - // Always allow these tools - if (ALWAYS_AVAILABLE_TOOLS.includes(tool as any)) { - return true - } - - // Check if this is a dynamic MCP tool (mcp_serverName_toolName) - // These should be allowed if the mcp group is allowed for the mode - const isDynamicMcpTool = tool.startsWith("mcp_") - if (experiments && Object.values(EXPERIMENT_IDS).includes(tool as ExperimentId)) { - if (!experiments[tool]) { - return false - } - } - - // Check tool requirements if any exist - if (toolRequirements && typeof toolRequirements === "object") { - if (tool in toolRequirements && !toolRequirements[tool]) { - return false - } - } else if (toolRequirements === false) { - // If toolRequirements is a boolean false, all tools are disabled - return false - } - - const mode = getModeBySlug(modeSlug, customModes) - if (!mode) { - return false - } - - // Check if tool is in any of the mode's groups and respects any group options - for (const group of mode.groups) { - const groupName = getGroupName(group) - const options = getGroupOptions(group) - - const groupConfig = TOOL_GROUPS[groupName] - - // Check if this is a dynamic MCP tool and the mcp group is allowed - if (isDynamicMcpTool && groupName === "mcp") { - // Dynamic MCP tools are allowed if the mcp group is in the mode's groups - return true - } - - // Check if the tool is in the group's regular tools - const isRegularTool = groupConfig.tools.includes(tool) - - // Check if the tool is a custom tool that has been explicitly included - const isCustomTool = groupConfig.customTools?.includes(tool) && includedTools?.includes(tool) - - // If the tool isn't in regular tools and isn't an included custom tool, continue to next group - if (!isRegularTool && !isCustomTool) { - continue - } - - // If there are no options, allow the tool - if (!options) { - return true - } - - // For the edit group, check file regex if specified - if (groupName === "edit" && options.fileRegex) { - const filePath = toolParams?.path - // Check if this is an actual edit operation (not just path-only for streaming) - const isEditOperation = EDIT_OPERATION_PARAMS.some((param) => toolParams?.[param]) - - // Handle single file path validation - if (filePath && isEditOperation && !doesFileMatchRegex(filePath, options.fileRegex)) { - throw new FileRestrictionError(mode.name, options.fileRegex, options.description, filePath, tool) - } - - // Handle XML args parameter (used by MULTI_FILE_APPLY_DIFF experiment) - if (toolParams?.args && typeof toolParams.args === "string") { - // Extract file paths from XML args with improved validation - try { - const filePathMatches = toolParams.args.match(/([^<]+)<\/path>/g) - if (filePathMatches) { - for (const match of filePathMatches) { - // More robust path extraction with validation - const pathMatch = match.match(/([^<]+)<\/path>/) - if (pathMatch && pathMatch[1]) { - const extractedPath = pathMatch[1].trim() - // Validate that the path is not empty and doesn't contain invalid characters - if (extractedPath && !extractedPath.includes("<") && !extractedPath.includes(">")) { - if (!doesFileMatchRegex(extractedPath, options.fileRegex)) { - throw new FileRestrictionError( - mode.name, - options.fileRegex, - options.description, - extractedPath, - tool, - ) - } - } - } - } - } - } catch (error) { - // Re-throw FileRestrictionError as it's an expected validation error - if (error instanceof FileRestrictionError) { - throw error - } - // If XML parsing fails, log the error but don't block the operation - console.warn(`Failed to parse XML args for file restriction validation: ${error}`) - } - } - } - - return true - } - - return false -} - // Create the mode-specific default prompts export const defaultPrompts: Readonly = Object.freeze( Object.fromEntries(