From d5f1011d5fdc6ab0bcb441a0e4d8900263206a86 Mon Sep 17 00:00:00 2001 From: cte Date: Mon, 15 Dec 2025 01:39:32 -0800 Subject: [PATCH 1/5] Use `@roo-code/core` --- pnpm-lock.yaml | 6 +- .../assistant-message/NativeToolCallParser.ts | 28 +- .../presentAssistantMessage.ts | 64 +++- .../__tests__/getEnvironmentDetails.spec.ts | 4 +- src/core/environment/getEnvironmentDetails.ts | 2 +- src/core/prompts/system.ts | 28 +- .../prompts/tools/filter-tools-for-mode.ts | 3 +- src/core/prompts/tools/index.ts | 9 +- src/core/task/Task.ts | 8 + src/core/task/__tests__/build-tools.spec.ts | 307 ++++++++++++++++++ .../__tests__/native-tools-filtering.spec.ts | 7 +- src/core/task/build-tools.ts | 29 +- .../tools/__tests__/validateToolUse.spec.ts | 4 +- src/core/tools/validateToolUse.ts | 158 ++++++++- src/esbuild.mjs | 4 +- src/package.json | 2 +- src/shared/__tests__/modes.spec.ts | 3 +- src/shared/modes.ts | 143 -------- .../__tests__/autoImportSettings.spec.ts | 34 +- 19 files changed, 641 insertions(+), 202 deletions(-) create mode 100644 src/core/task/__tests__/build-tools.spec.ts diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5591f160d79..b5bcdcfbbd2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -739,6 +739,9 @@ importers: diff-match-patch: specifier: ^1.0.5 version: 1.0.5 + esbuild: + specifier: '>=0.25.0' + version: 0.25.9 exceljs: specifier: ^4.4.0 version: 4.4.0 @@ -974,9 +977,6 @@ importers: '@vscode/vsce': specifier: 3.3.2 version: 3.3.2 - esbuild: - specifier: '>=0.25.0' - version: 0.25.9 execa: specifier: ^9.5.2 version: 9.5.3 diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 250afdc3890..73642b9cbe9 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -1,13 +1,16 @@ +import { parseJSON } from "partial-json" + import { type ToolName, toolNames, type FileEntry } from "@roo-code/types" +import { customToolRegistry } from "@roo-code/core" + import { type ToolUse, type McpToolUse, type ToolParamName, - toolParamNames, type NativeToolArgs, + toolParamNames, } from "../../shared/tools" import { resolveToolAlias } from "../prompts/tools/filter-tools-for-mode" -import { parseJSON } from "partial-json" import type { ApiStreamToolCallStartChunk, ApiStreamToolCallDeltaChunk, @@ -556,8 +559,11 @@ export class NativeToolCallParser { name: TName arguments: string }): ToolUse | McpToolUse | null { + // console.log(`parseToolCall -> ${JSON.stringify(toolCall, null, 2)}`) + // Check if this is a dynamic MCP tool (mcp--serverName--toolName) const mcpPrefix = MCP_TOOL_PREFIX + MCP_TOOL_SEPARATOR + if (typeof toolCall.name === "string" && toolCall.name.startsWith(mcpPrefix)) { return this.parseDynamicMcpTool(toolCall) } @@ -565,8 +571,8 @@ export class NativeToolCallParser { // Resolve tool alias to canonical name (e.g., "edit_file" -> "apply_diff", "temp_edit_file" -> "search_and_replace") const resolvedName = resolveToolAlias(toolCall.name as string) as TName - // Validate tool name (after alias resolution) - if (!toolNames.includes(resolvedName as ToolName)) { + // Validate tool name (after alias resolution). + if (!toolNames.includes(resolvedName as ToolName) && !customToolRegistry.has(resolvedName)) { console.error(`Invalid tool name: ${toolCall.name} (resolved: ${resolvedName})`) console.error(`Valid tool names:`, toolNames) return null @@ -574,7 +580,7 @@ export class NativeToolCallParser { try { // Parse the arguments JSON string - const args = JSON.parse(toolCall.arguments) + const args = toolCall.arguments === "" ? {} : JSON.parse(toolCall.arguments) // Build legacy params object for backward compatibility with XML protocol and UI. // Native execution path uses nativeArgs instead, which has proper typing. @@ -589,7 +595,7 @@ export class NativeToolCallParser { } // Validate parameter name - if (!toolParamNames.includes(key as ToolParamName)) { + if (!toolParamNames.includes(key as ToolParamName) && !customToolRegistry.has(resolvedName)) { console.warn(`Unknown parameter '${key}' for tool '${resolvedName}'`) console.warn(`Valid param names:`, toolParamNames) continue @@ -786,6 +792,14 @@ export class NativeToolCallParser { break default: + if (customToolRegistry.has(resolvedName)) { + nativeArgs = args as NativeArgsFor + } else { + console.error(`Unhandled tool: ${resolvedName}`) + console.error(`Valid tool names:`, toolNames) + console.error(`Custom tool names:`, customToolRegistry.list()) + } + break } @@ -802,6 +816,8 @@ export class NativeToolCallParser { result.originalName = toolCall.name } + console.log(`parseToolCall -> result: ${JSON.stringify(result, null, 2)}`) + return result } catch (error) { console.error( diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 78d2afc166f..eac9decdfdf 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -4,12 +4,16 @@ import { Anthropic } from "@anthropic-ai/sdk" import type { ToolName, ClineAsk, ToolProgressStatus } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" +import { customToolRegistry } from "@roo-code/core" + +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 +34,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 +354,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 +732,8 @@ 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) + console.error(`[presentAssistantMessage] errorContent: ${errorContent}`) + if (toolProtocol === TOOL_PROTOCOL.NATIVE && toolCallId) { // For native protocol, push tool_result directly without setting didAlreadyUseTool cline.userMessageContent.push({ @@ -743,6 +746,7 @@ export async function presentAssistantMessage(cline: Task) { // For XML protocol, use the standard pushToolResult pushToolResult(errorContent) } + break } } @@ -1043,9 +1047,8 @@ export async function presentAssistantMessage(cline: Task) { }) break default: { - // Handle unknown/invalid tool names + // Handle unknown/invalid tool names OR custom tools // This is critical for native protocol where every tool_use MUST have a tool_result - // Note: This case should rarely be reached since validateToolUse now checks for unknown tools // CRITICAL: Don't process partial blocks for unknown tools - just let them stream in. // If we try to show errors for partial blocks, we'd show the error on every streaming chunk, @@ -1054,6 +1057,45 @@ export async function presentAssistantMessage(cline: Task) { break } + const customTool = customToolRegistry.get(block.name) + + if (customTool) { + try { + console.log(`executing customTool -> ${JSON.stringify(customTool, null, 2)}`) + let customToolArgs + + if (customTool.parameters) { + try { + customToolArgs = customTool.parameters.parse(block.nativeArgs || block.params || {}) + console.log(`customToolArgs -> ${JSON.stringify(customToolArgs, null, 2)}`) + } catch (parseParamsError) { + const message = `Custom tool "${block.name}" argument validation failed: ${parseParamsError.message}` + console.error(message) + cline.consecutiveMistakeCount++ + await cline.say("error", message) + pushToolResult(formatResponse.toolError(message, toolProtocol)) + break + } + } + + console.log(`${customTool.name}.execute() -> ${JSON.stringify(customToolArgs, null, 2)}`) + + const result = await customTool.execute(customToolArgs, { + mode: mode ?? defaultModeSlug, + task: cline, + }) + + pushToolResult(result) + cline.consecutiveMistakeCount = 0 + } catch (executionError: any) { + cline.consecutiveMistakeCount++ + await handleError(`executing custom tool "${block.name}"`, executionError) + } + + break + } + + // Not a custom tool - handle as unknown tool error const errorMessage = `Unknown tool "${block.name}". This tool does not exist. Please use one of the available tools.` cline.consecutiveMistakeCount++ cline.recordToolError(block.name as ToolName, errorMessage) 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/system.ts b/src/core/prompts/system.ts index 75b28bbb213..fe2d98504ce 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -1,9 +1,15 @@ import * as vscode from "vscode" import * as os from "os" -import type { ModeConfig, PromptComponent, CustomModePrompts, TodoItem } from "@roo-code/types" - -import type { SystemPromptSettings } from "./types" +import { + type ModeConfig, + type PromptComponent, + type CustomModePrompts, + type TodoItem, + getEffectiveProtocol, + isNativeProtocol, +} from "@roo-code/types" +import { customToolRegistry, formatXml } from "@roo-code/core" import { Mode, modes, defaultModeSlug, getModeBySlug, getGroupName, getModeSelection } from "../../shared/modes" import { DiffStrategy } from "../../shared/tools" @@ -15,8 +21,8 @@ import { CodeIndexManager } from "../../services/code-index/manager" import { PromptVariables, loadSystemPromptFile } from "./sections/custom-system-prompt" +import type { SystemPromptSettings } from "./types" import { getToolDescriptionsForMode } from "./tools" -import { getEffectiveProtocol, isNativeProtocol } from "@roo-code/types" import { getRulesSection, getSystemInfoSection, @@ -98,7 +104,7 @@ async function generatePrompt( ]) // Build tools catalog section only for XML protocol - const toolsCatalog = isNativeProtocol(effectiveProtocol) + const builtInToolsCatalog = isNativeProtocol(effectiveProtocol) ? "" : `\n\n${getToolDescriptionsForMode( mode, @@ -116,6 +122,18 @@ async function generatePrompt( modelId, )}` + let customToolsSection = "" + + if (!isNativeProtocol(effectiveProtocol)) { + const customTools = customToolRegistry.getAllSerialized() + + if (customTools.length > 0) { + customToolsSection = `\n\n${formatXml(customTools)}` + } + } + + const toolsCatalog = builtInToolsCatalog + customToolsSection + const basePrompt = `${roleDefinition} ${markdownFormattingSection()} 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/Task.ts b/src/core/task/Task.ts index b515e608554..60bd40780de 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2568,6 +2568,10 @@ export class Task extends EventEmitter implements TaskLike { // Get the index and replace partial with final if (toolUseIndex !== undefined) { + console.log( + `[tool_call_partial] setting tool use at index ${toolUseIndex} with finalToolUse: ${JSON.stringify(finalToolUse, null, 2)}`, + ) + this.assistantMessageContent[toolUseIndex] = finalToolUse } @@ -2727,6 +2731,10 @@ export class Task extends EventEmitter implements TaskLike { // Get the index and replace partial with final if (toolUseIndex !== undefined) { + console.log( + `[tool_call_end] setting tool use at index ${toolUseIndex} with finalToolUse: ${JSON.stringify(finalToolUse, null, 2)}`, + ) + this.assistantMessageContent[toolUseIndex] = finalToolUse } diff --git a/src/core/task/__tests__/build-tools.spec.ts b/src/core/task/__tests__/build-tools.spec.ts new file mode 100644 index 00000000000..15065dc01e6 --- /dev/null +++ b/src/core/task/__tests__/build-tools.spec.ts @@ -0,0 +1,307 @@ +import type OpenAI from "openai" + +import type { SerializedCustomToolDefinition } from "@roo-code/types" + +type FunctionTool = Extract + +// Helper function to get tool name. +const getToolName = (tool: OpenAI.Chat.ChatCompletionTool): string | undefined => { + if (tool.type === "function") { + return (tool as FunctionTool).function.name + } + + return undefined +} + +// Helper function to find a function tool by name. +const findFunctionTool = (tools: OpenAI.Chat.ChatCompletionTool[], name: string): FunctionTool | undefined => { + return tools.find((t) => t.type === "function" && (t as FunctionTool).function.name === name) as + | FunctionTool + | undefined +} + +// Create mock registry that can be controlled per test. +const mockCustomToolRegistry = { + loadFromDirectoryIfStale: vi.fn().mockResolvedValue({ loaded: [], failed: [] }), + getAllSerialized: vi.fn().mockReturnValue([]), + clear: vi.fn(), + has: vi.fn().mockReturnValue(false), +} + +vi.mock("vscode", () => ({ + EventEmitter: vi.fn().mockImplementation(() => ({ + event: vi.fn(), + fire: vi.fn(), + dispose: vi.fn(), + })), + workspace: { + getConfiguration: vi.fn().mockReturnValue({ + get: vi.fn(), + }), + }, +})) + +// Mock the CodeIndexManager dynamic import. +vi.mock("../../services/code-index/manager", () => ({ + CodeIndexManager: { + getInstance: vi.fn().mockReturnValue({ + isFeatureEnabled: vi.fn().mockReturnValue(false), + }), + }, +})) + +vi.mock("@roo-code/core", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + customToolRegistry: mockCustomToolRegistry, + } +}) + +vi.mock("../prompts/tools/native-tools", () => ({ + getNativeTools: vi.fn().mockReturnValue([ + { + type: "function", + function: { + name: "read_file", + description: "Read files", + parameters: {}, + }, + }, + ]), + getMcpServerTools: vi.fn().mockReturnValue([]), +})) + +vi.mock("../prompts/tools/filter-tools-for-mode", () => ({ + filterNativeToolsForMode: vi.fn((tools) => tools), + filterMcpToolsForMode: vi.fn((tools) => tools), +})) + +describe("buildNativeToolsArray", () => { + const mockProvider = { + getMcpHub: vi.fn().mockReturnValue(undefined), + context: { + globalStorageUri: { fsPath: "/test/storage" }, + subscriptions: [], + }, + } + + beforeEach(() => { + vi.clearAllMocks() + vi.resetModules() + mockCustomToolRegistry.getAllSerialized.mockReturnValue([]) + }) + + describe("custom tools injection", () => { + it("should include custom tools in the returned array when registry has tools", async () => { + const { buildNativeToolsArray } = await import("../build-tools") + + const customTools: SerializedCustomToolDefinition[] = [ + { + name: "my_custom_tool", + description: "A custom tool for testing", + }, + ] + + mockCustomToolRegistry.getAllSerialized.mockReturnValue(customTools) + + const result = await buildNativeToolsArray({ + provider: mockProvider as any, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: undefined, + apiConfiguration: undefined, + maxReadFileLine: -1, + browserToolEnabled: true, + modelInfo: undefined, + diffEnabled: false, + }) + + // Should include both native tools and custom tools. + expect(result.length).toBeGreaterThan(0) + + const customToolResult = findFunctionTool(result, "my_custom_tool") + + expect(customToolResult).toBeDefined() + expect(customToolResult?.type).toBe("function") + expect(customToolResult?.function.description).toBe("A custom tool for testing") + }) + + it("should include custom tools with parameters", async () => { + const { buildNativeToolsArray } = await import("../build-tools") + + // getAllSerialized returns SerializedCustomToolDefinition with JSON Schema parameters + const customTools: SerializedCustomToolDefinition[] = [ + { + name: "parameterized_tool", + description: "A tool with parameters", + parameters: { + type: "object", + properties: { + input: { + type: "string", + description: "The input value", + }, + count: { + type: "number", + description: "Optional count", + }, + }, + required: ["input"], + additionalProperties: false, + }, + }, + ] + + mockCustomToolRegistry.getAllSerialized.mockReturnValue(customTools) + + const result = await buildNativeToolsArray({ + provider: mockProvider as any, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: undefined, + apiConfiguration: undefined, + maxReadFileLine: -1, + browserToolEnabled: true, + modelInfo: undefined, + diffEnabled: false, + }) + + const paramTool = findFunctionTool(result, "parameterized_tool") + + expect(paramTool).toBeDefined() + expect(paramTool?.function.parameters).toMatchObject({ + type: "object", + properties: { + input: { + type: "string", + description: "The input value", + }, + count: { + type: "number", + description: "Optional count", + }, + }, + required: ["input"], + additionalProperties: false, + }) + }) + + it("should return only native tools when registry is empty", async () => { + const { buildNativeToolsArray } = await import("../build-tools") + + const result = await buildNativeToolsArray({ + provider: mockProvider as any, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: undefined, + apiConfiguration: undefined, + maxReadFileLine: -1, + browserToolEnabled: true, + modelInfo: undefined, + diffEnabled: false, + }) + + // Should still return native tools. + expect(result.length).toBeGreaterThan(0) + + // Should not contain any custom tools (just the mocked native tools). + const nativeToolNames = result.map(getToolName) + expect(nativeToolNames).toContain("read_file") + }) + + it("should return only native tools when registry returns empty array", async () => { + const { buildNativeToolsArray } = await import("../build-tools") + + mockCustomToolRegistry.getAllSerialized.mockReturnValue([]) + + const result = await buildNativeToolsArray({ + provider: mockProvider as any, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: undefined, + apiConfiguration: undefined, + maxReadFileLine: -1, + browserToolEnabled: true, + modelInfo: undefined, + diffEnabled: false, + }) + + // Should return native tools without error. + expect(result.length).toBeGreaterThan(0) + }) + + it("should include multiple custom tools", async () => { + const { buildNativeToolsArray } = await import("../build-tools") + + const customTools: SerializedCustomToolDefinition[] = [ + { + name: "tool_one", + description: "First custom tool", + }, + { + name: "tool_two", + description: "Second custom tool", + }, + { + name: "tool_three", + description: "Third custom tool", + }, + ] + + mockCustomToolRegistry.getAllSerialized.mockReturnValue(customTools) + + const result = await buildNativeToolsArray({ + provider: mockProvider as any, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: undefined, + apiConfiguration: undefined, + maxReadFileLine: -1, + browserToolEnabled: true, + modelInfo: undefined, + diffEnabled: false, + }) + + // Should find all three custom tools. + expect(findFunctionTool(result, "tool_one")).toBeDefined() + expect(findFunctionTool(result, "tool_two")).toBeDefined() + expect(findFunctionTool(result, "tool_three")).toBeDefined() + }) + + it("should append custom tools after native and MCP tools", async () => { + const { buildNativeToolsArray } = await import("../build-tools") + + const customTools: SerializedCustomToolDefinition[] = [ + { + name: "custom_tool", + description: "A custom tool", + }, + ] + + mockCustomToolRegistry.getAllSerialized.mockReturnValue(customTools) + + const result = await buildNativeToolsArray({ + provider: mockProvider as any, + cwd: "/test/path", + mode: "code", + customModes: undefined, + experiments: undefined, + apiConfiguration: undefined, + maxReadFileLine: -1, + browserToolEnabled: true, + modelInfo: undefined, + diffEnabled: false, + }) + + // Custom tools should be at the end of the array. + const lastTool = result[result.length - 1] as FunctionTool + expect(lastTool.function.name).toBe("custom_tool") + }) + }) +}) 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/task/build-tools.ts b/src/core/task/build-tools.ts index 575b31580e6..b7861be3270 100644 --- a/src/core/task/build-tools.ts +++ b/src/core/task/build-tools.ts @@ -1,6 +1,12 @@ +import path from "path" + import type OpenAI from "openai" + import type { ProviderSettings, ModeConfig, ModelInfo } from "@roo-code/types" +import { customToolRegistry, formatNative } from "@roo-code/core" + import type { ClineProvider } from "../webview/ClineProvider" + import { getNativeTools, getMcpServerTools } from "../prompts/tools/native-tools" import { filterNativeToolsForMode, filterMcpToolsForMode } from "../prompts/tools/filter-tools-for-mode" @@ -40,11 +46,11 @@ export async function buildNativeToolsArray(options: BuildToolsOptions): Promise const mcpHub = provider.getMcpHub() - // Get CodeIndexManager for feature checking + // Get CodeIndexManager for feature checking. const { CodeIndexManager } = await import("../../services/code-index/manager") const codeIndexManager = CodeIndexManager.getInstance(provider.context, cwd) - // Build settings object for tool filtering + // Build settings object for tool filtering. const filterSettings = { todoListEnabled: apiConfiguration?.todoListEnabled ?? true, browserToolEnabled: browserToolEnabled ?? true, @@ -52,13 +58,13 @@ export async function buildNativeToolsArray(options: BuildToolsOptions): Promise diffEnabled, } - // Determine if partial reads are enabled based on maxReadFileLine setting + // Determine if partial reads are enabled based on maxReadFileLine setting. const partialReadsEnabled = maxReadFileLine !== -1 - // Build native tools with dynamic read_file tool based on partialReadsEnabled + // Build native tools with dynamic read_file tool based on partialReadsEnabled. const nativeTools = getNativeTools(partialReadsEnabled) - // Filter native tools based on mode restrictions + // Filter native tools based on mode restrictions. const filteredNativeTools = filterNativeToolsForMode( nativeTools, mode, @@ -69,9 +75,18 @@ export async function buildNativeToolsArray(options: BuildToolsOptions): Promise mcpHub, ) - // Filter MCP tools based on mode restrictions + // Filter MCP tools based on mode restrictions. const mcpTools = getMcpServerTools(mcpHub) const filteredMcpTools = filterMcpToolsForMode(mcpTools, mode, customModes, experiments) - return [...filteredNativeTools, ...filteredMcpTools] + // Add custom tools if they are available. + await customToolRegistry.loadFromDirectoryIfStale(path.join(cwd, ".roo", "tools")) + const customTools = customToolRegistry.getAllSerialized() + let nativeCustomTools: OpenAI.Chat.ChatCompletionFunctionTool[] = [] + + if (customTools.length > 0) { + nativeCustomTools = customTools.map(formatNative) + } + + return [...filteredNativeTools, ...filteredMcpTools, ...nativeCustomTools] } 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..de814f0b3c9 100644 --- a/src/core/tools/validateToolUse.ts +++ b/src/core/tools/validateToolUse.ts @@ -1,7 +1,10 @@ -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 { customToolRegistry } from "@roo-code/core" -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 +17,11 @@ export function isValidToolName(toolName: string): toolName is ToolName { return true } - // Check if it's a dynamic MCP tool (mcp_serverName_toolName format) + if (customToolRegistry.has(toolName)) { + return true + } + + // Check if it's a dynamic MCP tool (mcp_serverName_toolName format). if (toolName.startsWith("mcp_")) { return true } @@ -54,3 +61,148 @@ 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 + } + + // For now, allow all custom tools in any mode. + // As a follow-up we should expand the custom tool definition to include mode restrictions. + if (customToolRegistry.has(tool)) { + 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/esbuild.mjs b/src/esbuild.mjs index f99b077e9f9..68298eb3de4 100644 --- a/src/esbuild.mjs +++ b/src/esbuild.mjs @@ -15,7 +15,7 @@ async function main() { const production = process.argv.includes("--production") const watch = process.argv.includes("--watch") const minify = production - const sourcemap = true // Always generate source maps for error handling + const sourcemap = true // Always generate source maps for error handling. /** * @type {import('esbuild').BuildOptions} @@ -100,7 +100,7 @@ async function main() { plugins, entryPoints: ["extension.ts"], outfile: "dist/extension.js", - external: ["vscode"], + external: ["vscode", "esbuild"], } /** diff --git a/src/package.json b/src/package.json index eb27df9a977..e5884238d2b 100644 --- a/src/package.json +++ b/src/package.json @@ -446,6 +446,7 @@ "@roo-code/telemetry": "workspace:^", "@roo-code/types": "workspace:^", "@vscode/codicons": "^0.0.36", + "esbuild": "^0.25.0", "async-mutex": "^0.5.0", "axios": "^1.12.0", "cheerio": "^1.0.0", @@ -535,7 +536,6 @@ "@types/vscode": "^1.84.0", "@vscode/test-electron": "^2.5.2", "@vscode/vsce": "3.3.2", - "esbuild": "^0.25.0", "execa": "^9.5.2", "glob": "^11.1.0", "mkdirp": "^3.0.1", 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( diff --git a/src/utils/__tests__/autoImportSettings.spec.ts b/src/utils/__tests__/autoImportSettings.spec.ts index be0d769670f..f3911571d10 100644 --- a/src/utils/__tests__/autoImportSettings.spec.ts +++ b/src/utils/__tests__/autoImportSettings.spec.ts @@ -17,15 +17,33 @@ vi.mock("fs/promises", () => ({ readFile: vi.fn(), })) -vi.mock("path", () => ({ - join: vi.fn((...args: string[]) => args.join("/")), - isAbsolute: vi.fn((p: string) => p.startsWith("/")), - basename: vi.fn((p: string) => p.split("/").pop() || ""), -})) +vi.mock("path", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + default: { + ...actual, + join: vi.fn((...args: string[]) => args.join("/")), + isAbsolute: vi.fn((p: string) => p.startsWith("/")), + basename: vi.fn((p: string) => p.split("/").pop() || ""), + }, + join: vi.fn((...args: string[]) => args.join("/")), + isAbsolute: vi.fn((p: string) => p.startsWith("/")), + basename: vi.fn((p: string) => p.split("/").pop() || ""), + } +}) -vi.mock("os", () => ({ - homedir: vi.fn(() => "/home/user"), -})) +vi.mock("os", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + default: { + ...actual, + homedir: vi.fn(() => "/home/user"), + }, + homedir: vi.fn(() => "/home/user"), + } +}) vi.mock("../fs", () => ({ fileExistsAtPath: vi.fn(), From d4f19383a273f3c981d398b59461c43eaffd623a Mon Sep 17 00:00:00 2001 From: cte Date: Mon, 15 Dec 2025 02:06:57 -0800 Subject: [PATCH 2/5] Revert this --- src/core/task/Task.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 60bd40780de..b515e608554 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2568,10 +2568,6 @@ export class Task extends EventEmitter implements TaskLike { // Get the index and replace partial with final if (toolUseIndex !== undefined) { - console.log( - `[tool_call_partial] setting tool use at index ${toolUseIndex} with finalToolUse: ${JSON.stringify(finalToolUse, null, 2)}`, - ) - this.assistantMessageContent[toolUseIndex] = finalToolUse } @@ -2731,10 +2727,6 @@ export class Task extends EventEmitter implements TaskLike { // Get the index and replace partial with final if (toolUseIndex !== undefined) { - console.log( - `[tool_call_end] setting tool use at index ${toolUseIndex} with finalToolUse: ${JSON.stringify(finalToolUse, null, 2)}`, - ) - this.assistantMessageContent[toolUseIndex] = finalToolUse } From 889f8e8fbc2c8f9b62a17e622462544a3416cc59 Mon Sep 17 00:00:00 2001 From: cte Date: Mon, 15 Dec 2025 02:08:01 -0800 Subject: [PATCH 3/5] There's not a ton of value for this test --- src/core/task/__tests__/build-tools.spec.ts | 307 -------------------- 1 file changed, 307 deletions(-) delete mode 100644 src/core/task/__tests__/build-tools.spec.ts diff --git a/src/core/task/__tests__/build-tools.spec.ts b/src/core/task/__tests__/build-tools.spec.ts deleted file mode 100644 index 15065dc01e6..00000000000 --- a/src/core/task/__tests__/build-tools.spec.ts +++ /dev/null @@ -1,307 +0,0 @@ -import type OpenAI from "openai" - -import type { SerializedCustomToolDefinition } from "@roo-code/types" - -type FunctionTool = Extract - -// Helper function to get tool name. -const getToolName = (tool: OpenAI.Chat.ChatCompletionTool): string | undefined => { - if (tool.type === "function") { - return (tool as FunctionTool).function.name - } - - return undefined -} - -// Helper function to find a function tool by name. -const findFunctionTool = (tools: OpenAI.Chat.ChatCompletionTool[], name: string): FunctionTool | undefined => { - return tools.find((t) => t.type === "function" && (t as FunctionTool).function.name === name) as - | FunctionTool - | undefined -} - -// Create mock registry that can be controlled per test. -const mockCustomToolRegistry = { - loadFromDirectoryIfStale: vi.fn().mockResolvedValue({ loaded: [], failed: [] }), - getAllSerialized: vi.fn().mockReturnValue([]), - clear: vi.fn(), - has: vi.fn().mockReturnValue(false), -} - -vi.mock("vscode", () => ({ - EventEmitter: vi.fn().mockImplementation(() => ({ - event: vi.fn(), - fire: vi.fn(), - dispose: vi.fn(), - })), - workspace: { - getConfiguration: vi.fn().mockReturnValue({ - get: vi.fn(), - }), - }, -})) - -// Mock the CodeIndexManager dynamic import. -vi.mock("../../services/code-index/manager", () => ({ - CodeIndexManager: { - getInstance: vi.fn().mockReturnValue({ - isFeatureEnabled: vi.fn().mockReturnValue(false), - }), - }, -})) - -vi.mock("@roo-code/core", async (importOriginal) => { - const actual = await importOriginal() - return { - ...actual, - customToolRegistry: mockCustomToolRegistry, - } -}) - -vi.mock("../prompts/tools/native-tools", () => ({ - getNativeTools: vi.fn().mockReturnValue([ - { - type: "function", - function: { - name: "read_file", - description: "Read files", - parameters: {}, - }, - }, - ]), - getMcpServerTools: vi.fn().mockReturnValue([]), -})) - -vi.mock("../prompts/tools/filter-tools-for-mode", () => ({ - filterNativeToolsForMode: vi.fn((tools) => tools), - filterMcpToolsForMode: vi.fn((tools) => tools), -})) - -describe("buildNativeToolsArray", () => { - const mockProvider = { - getMcpHub: vi.fn().mockReturnValue(undefined), - context: { - globalStorageUri: { fsPath: "/test/storage" }, - subscriptions: [], - }, - } - - beforeEach(() => { - vi.clearAllMocks() - vi.resetModules() - mockCustomToolRegistry.getAllSerialized.mockReturnValue([]) - }) - - describe("custom tools injection", () => { - it("should include custom tools in the returned array when registry has tools", async () => { - const { buildNativeToolsArray } = await import("../build-tools") - - const customTools: SerializedCustomToolDefinition[] = [ - { - name: "my_custom_tool", - description: "A custom tool for testing", - }, - ] - - mockCustomToolRegistry.getAllSerialized.mockReturnValue(customTools) - - const result = await buildNativeToolsArray({ - provider: mockProvider as any, - cwd: "/test/path", - mode: "code", - customModes: undefined, - experiments: undefined, - apiConfiguration: undefined, - maxReadFileLine: -1, - browserToolEnabled: true, - modelInfo: undefined, - diffEnabled: false, - }) - - // Should include both native tools and custom tools. - expect(result.length).toBeGreaterThan(0) - - const customToolResult = findFunctionTool(result, "my_custom_tool") - - expect(customToolResult).toBeDefined() - expect(customToolResult?.type).toBe("function") - expect(customToolResult?.function.description).toBe("A custom tool for testing") - }) - - it("should include custom tools with parameters", async () => { - const { buildNativeToolsArray } = await import("../build-tools") - - // getAllSerialized returns SerializedCustomToolDefinition with JSON Schema parameters - const customTools: SerializedCustomToolDefinition[] = [ - { - name: "parameterized_tool", - description: "A tool with parameters", - parameters: { - type: "object", - properties: { - input: { - type: "string", - description: "The input value", - }, - count: { - type: "number", - description: "Optional count", - }, - }, - required: ["input"], - additionalProperties: false, - }, - }, - ] - - mockCustomToolRegistry.getAllSerialized.mockReturnValue(customTools) - - const result = await buildNativeToolsArray({ - provider: mockProvider as any, - cwd: "/test/path", - mode: "code", - customModes: undefined, - experiments: undefined, - apiConfiguration: undefined, - maxReadFileLine: -1, - browserToolEnabled: true, - modelInfo: undefined, - diffEnabled: false, - }) - - const paramTool = findFunctionTool(result, "parameterized_tool") - - expect(paramTool).toBeDefined() - expect(paramTool?.function.parameters).toMatchObject({ - type: "object", - properties: { - input: { - type: "string", - description: "The input value", - }, - count: { - type: "number", - description: "Optional count", - }, - }, - required: ["input"], - additionalProperties: false, - }) - }) - - it("should return only native tools when registry is empty", async () => { - const { buildNativeToolsArray } = await import("../build-tools") - - const result = await buildNativeToolsArray({ - provider: mockProvider as any, - cwd: "/test/path", - mode: "code", - customModes: undefined, - experiments: undefined, - apiConfiguration: undefined, - maxReadFileLine: -1, - browserToolEnabled: true, - modelInfo: undefined, - diffEnabled: false, - }) - - // Should still return native tools. - expect(result.length).toBeGreaterThan(0) - - // Should not contain any custom tools (just the mocked native tools). - const nativeToolNames = result.map(getToolName) - expect(nativeToolNames).toContain("read_file") - }) - - it("should return only native tools when registry returns empty array", async () => { - const { buildNativeToolsArray } = await import("../build-tools") - - mockCustomToolRegistry.getAllSerialized.mockReturnValue([]) - - const result = await buildNativeToolsArray({ - provider: mockProvider as any, - cwd: "/test/path", - mode: "code", - customModes: undefined, - experiments: undefined, - apiConfiguration: undefined, - maxReadFileLine: -1, - browserToolEnabled: true, - modelInfo: undefined, - diffEnabled: false, - }) - - // Should return native tools without error. - expect(result.length).toBeGreaterThan(0) - }) - - it("should include multiple custom tools", async () => { - const { buildNativeToolsArray } = await import("../build-tools") - - const customTools: SerializedCustomToolDefinition[] = [ - { - name: "tool_one", - description: "First custom tool", - }, - { - name: "tool_two", - description: "Second custom tool", - }, - { - name: "tool_three", - description: "Third custom tool", - }, - ] - - mockCustomToolRegistry.getAllSerialized.mockReturnValue(customTools) - - const result = await buildNativeToolsArray({ - provider: mockProvider as any, - cwd: "/test/path", - mode: "code", - customModes: undefined, - experiments: undefined, - apiConfiguration: undefined, - maxReadFileLine: -1, - browserToolEnabled: true, - modelInfo: undefined, - diffEnabled: false, - }) - - // Should find all three custom tools. - expect(findFunctionTool(result, "tool_one")).toBeDefined() - expect(findFunctionTool(result, "tool_two")).toBeDefined() - expect(findFunctionTool(result, "tool_three")).toBeDefined() - }) - - it("should append custom tools after native and MCP tools", async () => { - const { buildNativeToolsArray } = await import("../build-tools") - - const customTools: SerializedCustomToolDefinition[] = [ - { - name: "custom_tool", - description: "A custom tool", - }, - ] - - mockCustomToolRegistry.getAllSerialized.mockReturnValue(customTools) - - const result = await buildNativeToolsArray({ - provider: mockProvider as any, - cwd: "/test/path", - mode: "code", - customModes: undefined, - experiments: undefined, - apiConfiguration: undefined, - maxReadFileLine: -1, - browserToolEnabled: true, - modelInfo: undefined, - diffEnabled: false, - }) - - // Custom tools should be at the end of the array. - const lastTool = result[result.length - 1] as FunctionTool - expect(lastTool.function.name).toBe("custom_tool") - }) - }) -}) From d8ec1eefb6396b198035009c88f7ca47abc56bc7 Mon Sep 17 00:00:00 2001 From: cte Date: Mon, 15 Dec 2025 02:08:54 -0800 Subject: [PATCH 4/5] Remove debugging --- src/core/assistant-message/NativeToolCallParser.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 73642b9cbe9..bc56943a07f 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -559,8 +559,6 @@ export class NativeToolCallParser { name: TName arguments: string }): ToolUse | McpToolUse | null { - // console.log(`parseToolCall -> ${JSON.stringify(toolCall, null, 2)}`) - // Check if this is a dynamic MCP tool (mcp--serverName--toolName) const mcpPrefix = MCP_TOOL_PREFIX + MCP_TOOL_SEPARATOR @@ -816,8 +814,6 @@ export class NativeToolCallParser { result.originalName = toolCall.name } - console.log(`parseToolCall -> result: ${JSON.stringify(result, null, 2)}`) - return result } catch (error) { console.error( From 4b8f7e76f4a282deb24bb76fb5f650be70997c51 Mon Sep 17 00:00:00 2001 From: cte Date: Mon, 15 Dec 2025 02:09:46 -0800 Subject: [PATCH 5/5] Remove debugging --- src/core/assistant-message/NativeToolCallParser.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index bc56943a07f..98dfd654660 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -794,8 +794,6 @@ export class NativeToolCallParser { nativeArgs = args as NativeArgsFor } else { console.error(`Unhandled tool: ${resolvedName}`) - console.error(`Valid tool names:`, toolNames) - console.error(`Custom tool names:`, customToolRegistry.list()) } break