diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-validation-errors.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-validation-errors.spec.ts new file mode 100644 index 0000000000..af2d3f9acd --- /dev/null +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-validation-errors.spec.ts @@ -0,0 +1,89 @@ +// npx vitest run src/core/assistant-message/__tests__/presentAssistantMessage-validation-errors.spec.ts + +import { describe, it, expect, beforeEach, vi } from "vitest" +import { presentAssistantMessage } from "../presentAssistantMessage" + +// Mock validateToolUse to ensure we don't depend on its behavior for this test. +vi.mock("../../tools/validateToolUse", () => ({ + validateToolUse: vi.fn(), +})) + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureToolUsage: vi.fn(), + captureConsecutiveMistakeError: vi.fn(), + }, + }, +})) + +describe("presentAssistantMessage - validation/missing-param handling", () => { + let mockTask: any + + beforeEach(() => { + mockTask = { + taskId: "test-task-id", + instanceId: "test-instance", + abort: false, + presentAssistantMessageLocked: false, + presentAssistantMessageHasPendingUpdates: false, + currentStreamingContentIndex: 0, + assistantMessageContent: [], + userMessageContent: [], + didCompleteReadingStream: true, + userMessageContentReady: false, + didRejectTool: false, + didAlreadyUseTool: false, + diffEnabled: false, + consecutiveMistakeCount: 0, + didToolFailInCurrentTurn: false, + clineMessages: [], + api: { + getModel: () => ({ id: "test-model", info: {} }), + }, + browserSession: { + closeBrowser: vi.fn().mockResolvedValue(undefined), + }, + recordToolUsage: vi.fn(), + recordToolError: vi.fn(), + toolRepetitionDetector: { + check: vi.fn().mockReturnValue({ allowExecution: true }), + }, + providerRef: { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "orchestrator", + customModes: [], + }), + }), + }, + say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("missing param"), + } + }) + + it("treats missing codebase_search.path as missing param when read fileRegex is present (native protocol)", async () => { + const toolCallId = "tool_call_missing_path" + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: toolCallId, + name: "codebase_search", + params: { query: "x" }, + partial: false, + }, + ] + + // Make orchestrator mode config discoverable via shared/modes.getModeBySlug() (built-in DEFAULT_MODES) + await presentAssistantMessage(mockTask) + + expect(mockTask.sayAndCreateMissingParamError).toHaveBeenCalledWith("codebase_search", "path") + const toolResult = mockTask.userMessageContent.find( + (item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId, + ) + expect(toolResult).toBeDefined() + expect(toolResult.is_error).toBe(true) + expect(mockTask.userMessageContentReady).toBe(true) + }) +}) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 0367e71ce3..ae4d79971e 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -356,6 +356,10 @@ export async function presentAssistantMessage(cline: Task) { // Fetch state early so it's available for toolDescription and validation const state = await cline.providerRef.deref()?.getState() const { mode, customModes, experiments: stateExperiments } = state ?? {} + const activeModeConfig = getModeBySlug(mode ?? defaultModeSlug, customModes) + const readGroupEntry = activeModeConfig?.groups.find((g) => (Array.isArray(g) ? g[0] : g) === "read") + const readGroupOptions = Array.isArray(readGroupEntry) ? readGroupEntry[1] : undefined + const readFileRegex = readGroupOptions?.fileRegex const toolDescription = (): string => { switch (block.name) { @@ -720,6 +724,34 @@ export async function presentAssistantMessage(cline: Task) { ? { ...block.params, ...block.nativeArgs } : block.params + // In modes that restrict read access via fileRegex (e.g., Orchestrator), + // codebase_search must NOT run with a missing/empty path because that implies + // an unrestricted workspace search. + // + // Instead of throwing a FileRestrictionError (which is confusing here), treat + // it like a missing required parameter so the model can retry with a safe path. + if (block.name === "codebase_search" && typeof readFileRegex === "string" && readFileRegex.length > 0) { + const rawPath = (toolParamsForValidation as { path?: unknown }).path + if (typeof rawPath !== "string" || rawPath.trim().length === 0) { + cline.consecutiveMistakeCount++ + cline.didToolFailInCurrentTurn = true + const missingParamError = await cline.sayAndCreateMissingParamError("codebase_search", "path") + + if (toolProtocol === TOOL_PROTOCOL.NATIVE && toolCallId) { + cline.userMessageContent.push({ + type: "tool_result", + tool_use_id: toolCallId, + content: + typeof missingParamError === "string" ? missingParamError : "(missing parameter)", + is_error: true, + } as Anthropic.ToolResultBlockParam) + } else { + pushToolResult(missingParamError) + } + break + } + } + try { validateToolUse( block.name as ToolName,