diff --git a/apps/vscode-e2e/src/suite/markdown-lists.test.ts b/apps/vscode-e2e/src/suite/markdown-lists.test.ts index 9b5c1bd8457..154d83bc93e 100644 --- a/apps/vscode-e2e/src/suite/markdown-lists.test.ts +++ b/apps/vscode-e2e/src/suite/markdown-lists.test.ts @@ -20,7 +20,12 @@ suite("Markdown List Rendering", function () { }) const taskId = await api.startNewTask({ - configuration: { mode: "ask", alwaysAllowModeSwitch: true, autoApprovalEnabled: true }, + configuration: { + mode: "ask", + alwaysAllowModeSwitch: true, + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }, text: "Please show me an example of an unordered list with the following items: Apple, Banana, Orange", }) @@ -57,7 +62,12 @@ suite("Markdown List Rendering", function () { }) const taskId = await api.startNewTask({ - configuration: { mode: "ask", alwaysAllowModeSwitch: true, autoApprovalEnabled: true }, + configuration: { + mode: "ask", + alwaysAllowModeSwitch: true, + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }, text: "Please show me a numbered list with three steps: First step, Second step, Third step", }) @@ -94,7 +104,12 @@ suite("Markdown List Rendering", function () { }) const taskId = await api.startNewTask({ - configuration: { mode: "ask", alwaysAllowModeSwitch: true, autoApprovalEnabled: true }, + configuration: { + mode: "ask", + alwaysAllowModeSwitch: true, + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }, text: "Please create a nested list with 'Main item' having two sub-items: 'Sub-item A' and 'Sub-item B'", }) @@ -146,7 +161,12 @@ suite("Markdown List Rendering", function () { }) const taskId = await api.startNewTask({ - configuration: { mode: "ask", alwaysAllowModeSwitch: true, autoApprovalEnabled: true }, + configuration: { + mode: "ask", + alwaysAllowModeSwitch: true, + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }, text: "Please create a list that has both numbered items and bullet points, mixing ordered and unordered lists", }) diff --git a/apps/vscode-e2e/src/suite/modes.test.ts b/apps/vscode-e2e/src/suite/modes.test.ts index 7982f3cf22b..2ae19cacecb 100644 --- a/apps/vscode-e2e/src/suite/modes.test.ts +++ b/apps/vscode-e2e/src/suite/modes.test.ts @@ -14,7 +14,12 @@ suite("Roo Code Modes", function () { globalThis.api.on(RooCodeEventName.TaskModeSwitched, (_taskId, mode) => modes.push(mode)) const switchModesTaskId = await globalThis.api.startNewTask({ - configuration: { mode: "code", alwaysAllowModeSwitch: true, autoApprovalEnabled: true }, + configuration: { + mode: "code", + alwaysAllowModeSwitch: true, + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }, text: "For each of `architect`, `ask`, and `debug` use the `switch_mode` tool to switch to that mode.", }) diff --git a/apps/vscode-e2e/src/suite/subtasks.test.ts b/apps/vscode-e2e/src/suite/subtasks.test.ts index e3e3457520c..5b67636ae7b 100644 --- a/apps/vscode-e2e/src/suite/subtasks.test.ts +++ b/apps/vscode-e2e/src/suite/subtasks.test.ts @@ -26,6 +26,7 @@ suite.skip("Roo Code Subtasks", () => { alwaysAllowModeSwitch: true, alwaysAllowSubtasks: true, autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, enableCheckpoints: false, }, text: diff --git a/apps/vscode-e2e/src/suite/task.test.ts b/apps/vscode-e2e/src/suite/task.test.ts index 10e4e4f9a62..58a6c4ba05e 100644 --- a/apps/vscode-e2e/src/suite/task.test.ts +++ b/apps/vscode-e2e/src/suite/task.test.ts @@ -20,7 +20,12 @@ suite("Roo Code Task", function () { }) const taskId = await api.startNewTask({ - configuration: { mode: "ask", alwaysAllowModeSwitch: true, autoApprovalEnabled: true }, + configuration: { + mode: "ask", + alwaysAllowModeSwitch: true, + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }, text: "Hello world, what is your name? Respond with 'My name is ...'", }) diff --git a/packages/types/src/followup.ts b/packages/types/src/followup.ts index 1a5424cd11e..2d246f1f1fc 100644 --- a/packages/types/src/followup.ts +++ b/packages/types/src/followup.ts @@ -10,6 +10,8 @@ export interface FollowUpData { question?: string /** Array of suggested answers that the user can select */ suggest?: Array + /** If true, hide the "Roo has a question" header in the UI */ + hideHeader?: boolean } /** @@ -36,6 +38,7 @@ export const suggestionItemSchema = z.object({ export const followUpDataSchema = z.object({ question: z.string().optional(), suggest: z.array(suggestionItemSchema).optional(), + hideHeader: z.boolean().optional(), }) export type FollowUpDataType = z.infer diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap index ee8a50e9933..5c181f5ac2a 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap index 44287486326..5655e242202 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-disabled.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-disabled.snap index 48b39d001f6..84e0273572e 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-disabled.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-disabled.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-enabled.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-enabled.snap index acc36d1ffd8..61b96f6a00a 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-enabled.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-enabled.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/partial-reads-enabled.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/partial-reads-enabled.snap index ac93623fda2..694b71ec78b 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/partial-reads-enabled.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/partial-reads-enabled.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap index ee8a50e9933..5c181f5ac2a 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap index 8edc23260ea..bd3a58acf88 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-false.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-false.snap index ee8a50e9933..5c181f5ac2a 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-false.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-false.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-true.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-true.snap index 54df428abd3..2d597433dcc 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-true.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-true.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-undefined.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-undefined.snap index ee8a50e9933..5c181f5ac2a 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-undefined.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-diff-enabled-undefined.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-different-viewport-size.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-different-viewport-size.snap index ee8a50e9933..5c181f5ac2a 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-different-viewport-size.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-different-viewport-size.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap index acc36d1ffd8..61b96f6a00a 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap index ee8a50e9933..5c181f5ac2a 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap @@ -10,7 +10,7 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/prompts/sections/__tests__/tool-use.spec.ts b/src/core/prompts/sections/__tests__/tool-use.spec.ts index c8e3a9b5d06..920727a1f65 100644 --- a/src/core/prompts/sections/__tests__/tool-use.spec.ts +++ b/src/core/prompts/sections/__tests__/tool-use.spec.ts @@ -3,11 +3,10 @@ import { TOOL_PROTOCOL } from "@roo-code/types" describe("getSharedToolUseSection", () => { describe("XML protocol", () => { - it("should include one tool per message requirement", () => { + it("should include step-by-step tool usage guidance", () => { const section = getSharedToolUseSection(TOOL_PROTOCOL.XML) - expect(section).toContain("You must use exactly one tool per message") - expect(section).toContain("every assistant message must include a tool call") + expect(section).toContain("You use tools step-by-step to accomplish a given task") }) it("should include XML formatting instructions", () => { @@ -19,28 +18,23 @@ describe("getSharedToolUseSection", () => { }) describe("native protocol", () => { - it("should include one tool per message requirement when experiment is disabled", () => { + it("should include single tool guidance when experiment is disabled", () => { // No experiment flags passed (default: disabled) const section = getSharedToolUseSection(TOOL_PROTOCOL.NATIVE) - expect(section).toContain("You must use exactly one tool call per assistant response") - expect(section).toContain("Do not call zero tools or more than one tool") + expect(section).toContain("When using tools, use exactly one tool call per assistant response") }) - it("should include one tool per message requirement when experiment is explicitly disabled", () => { + it("should include single tool guidance when experiment is explicitly disabled", () => { const section = getSharedToolUseSection(TOOL_PROTOCOL.NATIVE, { multipleNativeToolCalls: false }) - expect(section).toContain("You must use exactly one tool call per assistant response") - expect(section).toContain("Do not call zero tools or more than one tool") + expect(section).toContain("When using tools, use exactly one tool call per assistant response") }) - it("should NOT include one tool per message requirement when experiment is enabled", () => { + it("should include multiple tools guidance when experiment is enabled", () => { const section = getSharedToolUseSection(TOOL_PROTOCOL.NATIVE, { multipleNativeToolCalls: true }) - expect(section).not.toContain("You must use exactly one tool per message") - expect(section).not.toContain("every assistant message must include a tool call") - expect(section).toContain("You must call at least one tool per assistant response") - expect(section).toContain("Prefer calling as many tools as are reasonably needed") + expect(section).toContain("When using tools, prefer calling as many tools as are reasonably needed") }) it("should include native tool-calling instructions", () => { @@ -63,7 +57,7 @@ describe("getSharedToolUseSection", () => { const section = getSharedToolUseSection() expect(section).toContain("XML-style tags") - expect(section).toContain("You must use exactly one tool per message") + expect(section).toContain("You use tools step-by-step to accomplish a given task") }) }) }) diff --git a/src/core/prompts/sections/tool-use.ts b/src/core/prompts/sections/tool-use.ts index c3f5e221b80..aa0c75103ff 100644 --- a/src/core/prompts/sections/tool-use.ts +++ b/src/core/prompts/sections/tool-use.ts @@ -14,8 +14,8 @@ export function getSharedToolUseSection( ) const toolUseGuidance = isMultipleNativeToolCallsEnabled - ? " You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster." - : " You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response." + ? " When using tools, prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster." + : " When using tools, use exactly one tool call per assistant response." return `==== @@ -28,7 +28,7 @@ You have access to a set of tools that are executed upon the user's approval. Us TOOL USE -You have access to a set of tools that are executed upon the user's approval. You must use exactly one tool per message, and every assistant message must include a tool call. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. +You have access to a set of tools that are executed upon the user's approval. You use tools step-by-step to accomplish a given task, with each tool use informed by the result of the previous tool use. # Tool Use Formatting diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 33e8245ccc6..620bb853c67 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -33,6 +33,7 @@ import { type CreateTaskOptions, type ModelInfo, type ToolProtocol, + type FollowUpData, RooCodeEventName, TelemetryEventName, TaskStatus, @@ -3304,21 +3305,35 @@ export class Task extends EventEmitter implements TaskLike { ) if (!didToolUse) { - // Increment consecutive no-tool-use counter - this.consecutiveNoToolUseCount++ - - // Only show error and count toward mistake limit after 2 consecutive failures - if (this.consecutiveNoToolUseCount >= 2) { - await this.say("error", "MODEL_NO_TOOLS_USED") - // Only count toward mistake limit after second consecutive failure - this.consecutiveMistakeCount++ - } + // Reset counter when we get a response (even without tools) + this.consecutiveNoToolUseCount = 0 - // Use the task's locked protocol for consistent behavior - this.userMessageContent.push({ - type: "text", - text: formatResponse.noToolsUsed(this._taskToolProtocol ?? "xml"), - }) + const state = await this.providerRef.deref()?.getState() + + if (state?.autoApprovalEnabled && state?.alwaysAllowFollowupQuestions) { + // Auto-approval enabled: tell model to use a tool and continue + this.userMessageContent.push({ + type: "text", + text: formatResponse.noToolsUsed(this._taskToolProtocol ?? "xml"), + }) + } else { + // Auto-approval disabled: present message to user, wait for response + // Use the assistant's text content as the message (hide "has a question" header) + const followUpData: FollowUpData = { question: assistantMessage, hideHeader: true } + const { text, images } = await this.ask("followup", JSON.stringify(followUpData), false) + await this.say("user_feedback", text ?? "", images) + + // formatResponse.toolResult can return string or array, handle both + const toolResult = formatResponse.toolResult(`\n${text}\n`, images) + if (typeof toolResult === "string") { + this.userMessageContent.push({ + type: "text", + text: toolResult, + }) + } else { + this.userMessageContent.push(...toolResult) + } + } } else { // Reset counter when tools are used successfully this.consecutiveNoToolUseCount = 0 diff --git a/src/core/task/__tests__/no-tool-followup.spec.ts b/src/core/task/__tests__/no-tool-followup.spec.ts new file mode 100644 index 00000000000..48055d1d785 --- /dev/null +++ b/src/core/task/__tests__/no-tool-followup.spec.ts @@ -0,0 +1,478 @@ +// Test for treating messages without tools as follow-up questions +// npx vitest core/task/__tests__/no-tool-followup.spec.ts + +import * as os from "os" +import * as path from "path" +import { Task } from "../Task" +import { ClineProvider } from "../../webview/ClineProvider" +import { ContextProxy } from "../../config/ContextProxy" +import type { ProviderSettings } from "@roo-code/types" +import * as vscode from "vscode" +import { TelemetryService } from "@roo-code/telemetry" + +// Mock delay +vi.mock("delay", () => ({ + __esModule: true, + default: vi.fn().mockResolvedValue(undefined), +})) + +// Mock file system +vi.mock("fs/promises", async (importOriginal) => { + const actual = (await importOriginal()) as Record + return { + ...actual, + mkdir: vi.fn().mockResolvedValue(undefined), + writeFile: vi.fn().mockResolvedValue(undefined), + readFile: vi.fn().mockResolvedValue("[]"), + unlink: vi.fn().mockResolvedValue(undefined), + rmdir: vi.fn().mockResolvedValue(undefined), + } +}) + +// Mock p-wait-for +vi.mock("p-wait-for", () => ({ + default: vi.fn().mockImplementation(async () => Promise.resolve()), +})) + +// Mock vscode +vi.mock("vscode", () => ({ + workspace: { + workspaceFolders: [{ uri: { fsPath: "/mock/workspace" } }], + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(() => ({ dispose: vi.fn() })), + onDidDelete: vi.fn(() => ({ dispose: vi.fn() })), + onDidChange: vi.fn(() => ({ dispose: vi.fn() })), + dispose: vi.fn(), + })), + fs: { stat: vi.fn().mockResolvedValue({ type: 1 }) }, + onDidSaveTextDocument: vi.fn(() => ({ dispose: vi.fn() })), + getConfiguration: vi.fn(() => ({ get: (key: string, defaultValue: any) => defaultValue })), + }, + env: { uriScheme: "vscode", language: "en" }, + window: { + createTextEditorDecorationType: vi.fn().mockReturnValue({ dispose: vi.fn() }), + visibleTextEditors: [], + tabGroups: { all: [], close: vi.fn(), onDidChangeTabs: vi.fn(() => ({ dispose: vi.fn() })) }, + }, + EventEmitter: vi.fn().mockImplementation(() => ({ event: vi.fn(), fire: vi.fn() })), + Disposable: { from: vi.fn() }, +})) + +// Mock environment details +vi.mock("../../environment/getEnvironmentDetails", () => ({ + getEnvironmentDetails: vi.fn().mockResolvedValue(""), +})) + +// Mock RooIgnoreController +vi.mock("../../ignore/RooIgnoreController") + +// Mock condense +vi.mock("../../condense", async (importOriginal) => { + const actual = (await importOriginal()) as any + return { + ...actual, + summarizeConversation: vi.fn().mockResolvedValue({ + messages: [{ role: "user", content: [{ type: "text", text: "continued" }], ts: Date.now() }], + summary: "summary", + cost: 0, + newContextTokens: 1, + }), + } +}) + +// Mock storage utilities +vi.mock("../../../utils/storage", () => ({ + getTaskDirectoryPath: vi + .fn() + .mockImplementation((globalStoragePath, taskId) => Promise.resolve(`${globalStoragePath}/tasks/${taskId}`)), + getSettingsDirectoryPath: vi + .fn() + .mockImplementation((globalStoragePath) => Promise.resolve(`${globalStoragePath}/settings`)), +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(false), +})) + +describe("No-Tool Messages as Follow-up Questions", () => { + let mockProvider: any + let mockApiConfig: ProviderSettings + let mockContext: vscode.ExtensionContext + + beforeEach(() => { + if (!TelemetryService.hasInstance()) { + TelemetryService.createInstance([]) + } + + const storageUri = { fsPath: path.join(os.tmpdir(), "test-storage") } + + mockContext = { + globalState: { + get: vi.fn().mockReturnValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + globalStorageUri: storageUri, + workspaceState: { + get: vi.fn().mockReturnValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + secrets: { + get: vi.fn().mockResolvedValue(undefined), + store: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + }, + extensionUri: { fsPath: "/mock/extension" }, + extension: { packageJSON: { version: "1.0.0" } }, + } as unknown as vscode.ExtensionContext + + const mockOutput = { + appendLine: vi.fn(), + append: vi.fn(), + clear: vi.fn(), + show: vi.fn(), + hide: vi.fn(), + dispose: vi.fn(), + } + + mockProvider = new ClineProvider(mockContext, mockOutput as any, "sidebar", new ContextProxy(mockContext)) + mockProvider.postMessageToWebview = vi.fn().mockResolvedValue(undefined) + mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) + + mockApiConfig = { + apiProvider: "anthropic", + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-key", + } + }) + + describe("Auto-approval enabled with alwaysAllowFollowupQuestions", () => { + it("should auto-continue with noToolsUsed message when both settings are enabled", async () => { + // Setup state with both auto-approval and follow-up questions enabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Mock the API to return a message without tools + const mockStream = { + async *[Symbol.asyncIterator]() { + yield { type: "text", text: "I need more information" } + }, + } + + vi.spyOn(task.api, "createMessage").mockReturnValue(mockStream as any) + + // Add initial conversation history + task.apiConversationHistory = [ + { + role: "user", + content: [{ type: "text", text: "Do something" }], + ts: Date.now(), + }, + ] + + // Set up spy on say to verify no error is shown + const saySpy = vi.spyOn(task, "say") + + // Set up spy on ask to verify it's NOT called (auto-continue) + const askSpy = vi.spyOn(task, "ask") + + // Trigger the recursivelyMakeClineRequests with the assistant's no-tool message + task.assistantMessageContent = [{ type: "text", content: "I need more information", partial: false }] + task.userMessageContentReady = true + + // Simulate what happens in recursivelyMakeClineRequests when didToolUse is false + const didToolUse = false + + if (!didToolUse) { + task.consecutiveNoToolUseCount = 0 + + const state = await mockProvider.getState() + + if (state?.autoApprovalEnabled && state?.alwaysAllowFollowupQuestions) { + // This should be the path taken - auto-continue + task.userMessageContent.push({ + type: "text", + text: "[ERROR] You did not use a tool...", + }) + } + } + + // Verify that ask was NOT called (no follow-up question shown) + expect(askSpy).not.toHaveBeenCalledWith("followup", expect.any(String), false) + + // Verify that the noToolsUsed message was added to userMessageContent + expect(task.userMessageContent).toHaveLength(1) + expect(task.userMessageContent[0]).toHaveProperty("type", "text") + expect((task.userMessageContent[0] as any).text).toContain("did not use a tool") + + // Verify no error was shown to the user + expect(saySpy).not.toHaveBeenCalledWith("error", "MODEL_NO_TOOLS_USED") + }) + }) + + describe("Auto-approval disabled or alwaysAllowFollowupQuestions disabled", () => { + it("should present as follow-up question with hideHeader when autoApprovalEnabled is false", async () => { + // Setup state with auto-approval disabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: false, + alwaysAllowFollowupQuestions: true, + }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Set up spy on ask to verify it IS called with followup + const askSpy = vi.spyOn(task, "ask").mockResolvedValue({ + response: "messageResponse", + text: "User's answer", + images: [], + }) + + // Set up spy on say to verify user_feedback is shown + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) + + // Simulate what happens in recursivelyMakeClineRequests when didToolUse is false + const assistantMessage = "What color would you like?" + const didToolUse = false + + if (!didToolUse) { + task.consecutiveNoToolUseCount = 0 + + const state = await mockProvider.getState() + + if (state?.autoApprovalEnabled && state?.alwaysAllowFollowupQuestions) { + // Should NOT take this path + } else { + // Should take this path - present as follow-up question with hideHeader + const followUpData = { question: assistantMessage, hideHeader: true } + const { text, images } = await task.ask("followup", JSON.stringify(followUpData), false) + await task.say("user_feedback", text ?? "", images) + + const toolResult = `\n${text}\n` + task.userMessageContent.push({ + type: "text", + text: toolResult, + }) + } + } + + // Verify that ask WAS called with followup and hideHeader + expect(askSpy).toHaveBeenCalledWith( + "followup", + JSON.stringify({ question: assistantMessage, hideHeader: true }), + false, + ) + + // Verify that say WAS called with user_feedback + expect(saySpy).toHaveBeenCalledWith("user_feedback", "User's answer", []) + + // Verify that the user's response was added to userMessageContent + expect(task.userMessageContent).toHaveLength(1) + expect(task.userMessageContent[0]).toHaveProperty("type", "text") + expect((task.userMessageContent[0] as any).text).toContain("User's answer") + }) + + it("should present as follow-up question with hideHeader when alwaysAllowFollowupQuestions is false", async () => { + // Setup state with alwaysAllowFollowupQuestions disabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: false, + }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Set up spy on ask to verify it IS called with followup + const askSpy = vi.spyOn(task, "ask").mockResolvedValue({ + response: "messageResponse", + text: "User's answer", + images: [], + }) + + // Set up spy on say to verify user_feedback is shown + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) + + // Simulate what happens in recursivelyMakeClineRequests when didToolUse is false + const assistantMessage = "What's your preference?" + const didToolUse = false + + if (!didToolUse) { + task.consecutiveNoToolUseCount = 0 + + const state = await mockProvider.getState() + + if (state?.autoApprovalEnabled && state?.alwaysAllowFollowupQuestions) { + // Should NOT take this path + } else { + // Should take this path - present as follow-up question with hideHeader + const followUpData = { question: assistantMessage, hideHeader: true } + const { text, images } = await task.ask("followup", JSON.stringify(followUpData), false) + await task.say("user_feedback", text ?? "", images) + + const toolResult = `\n${text}\n` + task.userMessageContent.push({ + type: "text", + text: toolResult, + }) + } + } + + // Verify that ask WAS called with followup and hideHeader + expect(askSpy).toHaveBeenCalledWith( + "followup", + JSON.stringify({ question: assistantMessage, hideHeader: true }), + false, + ) + + // Verify that say WAS called with user_feedback + expect(saySpy).toHaveBeenCalledWith("user_feedback", "User's answer", []) + }) + + it("should handle images in follow-up question response", async () => { + // Setup state with auto-approval disabled + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: false, + alwaysAllowFollowupQuestions: true, + }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + const testImages = ["data:image/png;base64,test1", "data:image/png;base64,test2"] + + // Set up spy on ask to return response with images + const askSpy = vi.spyOn(task, "ask").mockResolvedValue({ + response: "messageResponse", + text: "Here are the images", + images: testImages, + }) + + // Set up spy on say + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined) + + // Simulate what happens in recursivelyMakeClineRequests when didToolUse is false + const assistantMessage = "Can you show me some screenshots?" + const didToolUse = false + + if (!didToolUse) { + task.consecutiveNoToolUseCount = 0 + + const state = await mockProvider.getState() + + if (state?.autoApprovalEnabled && state?.alwaysAllowFollowupQuestions) { + // Should NOT take this path + } else { + // Should take this path - present as follow-up question with hideHeader + const followUpData = { question: assistantMessage, hideHeader: true } + const { text, images } = await task.ask("followup", JSON.stringify(followUpData), false) + await task.say("user_feedback", text ?? "", images) + + // formatResponse.toolResult can return string or array when images present + const toolResult = `\n${text}\n` + // Simplified - in real code formatResponse.toolResult handles image blocks + task.userMessageContent.push({ + type: "text", + text: toolResult, + }) + } + } + + // Verify that ask WAS called with hideHeader + expect(askSpy).toHaveBeenCalledWith( + "followup", + JSON.stringify({ question: assistantMessage, hideHeader: true }), + false, + ) + + // Verify that say WAS called with user_feedback including images + expect(saySpy).toHaveBeenCalledWith("user_feedback", "Here are the images", testImages) + }) + }) + + describe("Counter behavior", () => { + it("should reset consecutiveNoToolUseCount when no tools are used", async () => { + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Set initial counter + task.consecutiveNoToolUseCount = 2 + + // Simulate no tool use scenario + const didToolUse = false + + if (!didToolUse) { + task.consecutiveNoToolUseCount = 0 // This is the new behavior + } + + // Verify counter was reset + expect(task.consecutiveNoToolUseCount).toBe(0) + }) + + it("should not increment consecutiveMistakeCount for no-tool responses", async () => { + mockProvider.getState = vi.fn().mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowFollowupQuestions: true, + }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + const initialMistakeCount = task.consecutiveMistakeCount + + // Simulate what happens in recursivelyMakeClineRequests when didToolUse is false + const didToolUse = false + + if (!didToolUse) { + task.consecutiveNoToolUseCount = 0 + + const state = await mockProvider.getState() + + if (state?.autoApprovalEnabled && state?.alwaysAllowFollowupQuestions) { + // Auto-continue - should NOT increment consecutiveMistakeCount + task.userMessageContent.push({ + type: "text", + text: "[ERROR] You did not use a tool...", + }) + } + } + + // Verify mistake count was NOT incremented + expect(task.consecutiveMistakeCount).toBe(initialMistakeCount) + }) + }) +}) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index ef2231b26d1..e4922bdadc6 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -353,6 +353,12 @@ export const ChatRowContent = ({ ), ] case "followup": + // Parse followup data to check for hideHeader flag + const followUpDataForIcon = message.partial ? null : safeJsonParse(message.text) + + if (followUpDataForIcon?.hideHeader) { + return [null, null] // No header + } return [ , {t("chat:questions.hasQuestion")}, @@ -1614,9 +1620,12 @@ export const ChatRowContent = ({ )}
- + {/* Only show markdown if hideHeader is false - when true, the text was already shown in a previous message */} + {!followUpData?.hideHeader && ( + + )}