mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
feat: treat messages without tools as follow-up questions (ROO-400)
- Add hideHeader flag to FollowUpData interface to optionally hide 'Roo has a question' header - Modify no-tool handling in Task.ts to present as follow-up instead of error - Update ChatRow.tsx to conditionally hide header when hideHeader is true - Relax strict tool-per-message requirement in system prompt - Remove error treatment (MODEL_NO_TOOLS_USED) for no-tool responses - Remove consecutiveMistakeCount increment for no-tool scenarios - Add comprehensive test coverage
This commit is contained in:
daniel-lxs
parent
861139ca24
commit
0da4d352e1
19 changed files with 546 additions and 47 deletions
|
|
@ -10,6 +10,8 @@ export interface FollowUpData {
|
|||
question?: string
|
||||
/** Array of suggested answers that the user can select */
|
||||
suggest?: Array<SuggestionItem>
|
||||
/** 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<typeof followUpDataSchema>
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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<TaskEvents> implements TaskLike {
|
|||
)
|
||||
|
||||
if (!didToolUse) {
|
||||
// Increment consecutive no-tool-use counter
|
||||
this.consecutiveNoToolUseCount++
|
||||
// Reset counter when we get a response (even without tools)
|
||||
this.consecutiveNoToolUseCount = 0
|
||||
|
||||
// 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++
|
||||
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(`<answer>\n${text}\n</answer>`, images)
|
||||
if (typeof toolResult === "string") {
|
||||
this.userMessageContent.push({
|
||||
type: "text",
|
||||
text: toolResult,
|
||||
})
|
||||
} else {
|
||||
this.userMessageContent.push(...toolResult)
|
||||
}
|
||||
}
|
||||
|
||||
// Use the task's locked protocol for consistent behavior
|
||||
this.userMessageContent.push({
|
||||
type: "text",
|
||||
text: formatResponse.noToolsUsed(this._taskToolProtocol ?? "xml"),
|
||||
})
|
||||
} else {
|
||||
// Reset counter when tools are used successfully
|
||||
this.consecutiveNoToolUseCount = 0
|
||||
|
|
|
|||
478
src/core/task/__tests__/no-tool-followup.spec.ts
Normal file
478
src/core/task/__tests__/no-tool-followup.spec.ts
Normal file
|
|
@ -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<string, any>
|
||||
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 = `<answer>\n${text}\n</answer>`
|
||||
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 = `<answer>\n${text}\n</answer>`
|
||||
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 = `<answer>\n${text}\n</answer>`
|
||||
// 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)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -353,6 +353,12 @@ export const ChatRowContent = ({
|
|||
),
|
||||
]
|
||||
case "followup":
|
||||
// Parse followup data to check for hideHeader flag
|
||||
const followUpDataForIcon = message.partial ? null : safeJsonParse<FollowUpData>(message.text)
|
||||
|
||||
if (followUpDataForIcon?.hideHeader) {
|
||||
return [null, null] // No header
|
||||
}
|
||||
return [
|
||||
<MessageCircleQuestionMark className="w-4 shrink-0" aria-label="Question icon" />,
|
||||
<span style={{ color: normalColor, fontWeight: "bold" }}>{t("chat:questions.hasQuestion")}</span>,
|
||||
|
|
@ -1614,9 +1620,12 @@ export const ChatRowContent = ({
|
|||
</div>
|
||||
)}
|
||||
<div className="flex flex-col gap-2 ml-6">
|
||||
<Markdown
|
||||
markdown={message.partial === true ? message?.text : followUpData?.question}
|
||||
/>
|
||||
{/* Only show markdown if hideHeader is false - when true, the text was already shown in a previous message */}
|
||||
{!followUpData?.hideHeader && (
|
||||
<Markdown
|
||||
markdown={message.partial === true ? message?.text : followUpData?.question}
|
||||
/>
|
||||
)}
|
||||
<FollowUpSuggest
|
||||
suggestions={followUpData?.suggest}
|
||||
onSuggestionClick={onSuggestionClick}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue