mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
fix(orchestrator): treat missing codebase_search path as missing-param
This commit is contained in:
parent
521cb33bf1
commit
ec8e2c0937
2 changed files with 121 additions and 0 deletions
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue