diff --git a/apps/vscode-e2e/src/fixtures/subtasks.ts b/apps/vscode-e2e/src/fixtures/subtasks.ts index 30d3852b0a..b9d44eb387 100644 --- a/apps/vscode-e2e/src/fixtures/subtasks.ts +++ b/apps/vscode-e2e/src/fixtures/subtasks.ts @@ -14,6 +14,8 @@ const SUBTASK_FAST_CHILD_MARKER = "SUBTASK_CHILD_IMMEDIATE_COMPLETION" const SUBTASK_XPROFILE_PARENT_MARKER = "SUBTASK_PARENT_CROSS_PROFILE" const SUBTASK_XPROFILE_SAME_CHILD_MARKER = "SUBTASK_CHILD_SAME_PROFILE" const SUBTASK_XPROFILE_DIFFERENT_CHILD_MARKER = "SUBTASK_CHILD_DIFFERENT_PROFILE" +export const SUBTASK_QUEUED_INPUT_PARENT_MARKER = "SUBTASK_PARENT_QUEUED_INPUT" +export const SUBTASK_QUEUED_INPUT_CHILD_MARKER = "SUBTASK_CHILD_QUEUED_INPUT" const SUBTASK_CHILD_PROMPT = `${SUBTASK_CHILD_MARKER}: Ask the user exactly this follow-up question: What is the square root of 81? After the user answers, complete with only the answer.` export const SUBTASK_PARENT_PROMPT = `${SUBTASK_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_CHILD_PROMPT}" Do not answer directly.` @@ -54,6 +56,14 @@ export const SUBTASK_XPROFILE_SAME_CHILD_RESULT = "Same-profile child completed" export const SUBTASK_XPROFILE_DIFFERENT_CHILD_RESULT = "Different-profile child completed" export const SUBTASK_XPROFILE_PARENT_RESULT = "Sequential cross-profile parent resumed" +const SUBTASK_QUEUED_INPUT_INITIAL_RESULT = "Child completed before queued input" +export const SUBTASK_QUEUED_INPUT_MESSAGE = "Use the queued instruction before completing." +export const SUBTASK_QUEUED_INPUT_CHILD_RESULT = "Child processed queued input" +export const SUBTASK_QUEUED_INPUT_PARENT_RESULT = "Parent resumed after queued input" +const SUBTASK_QUEUED_INPUT_CHILD_PROMPT = `${SUBTASK_QUEUED_INPUT_CHILD_MARKER}: Complete immediately with the exact result "${SUBTASK_QUEUED_INPUT_INITIAL_RESULT}".` +export const SUBTASK_QUEUED_INPUT_PARENT_PROMPT = `${SUBTASK_QUEUED_INPUT_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_QUEUED_INPUT_CHILD_PROMPT}" Do not answer directly. When the subtask returns, complete with the exact result "${SUBTASK_QUEUED_INPUT_PARENT_RESULT}".` +export const SUBTASK_QUEUED_INPUT_RESPONSE_LATENCY_MS = 2_000 + // Scheduler regression tests — exercises TaskScheduler + run() dispatch post-CodeRabbit fix. // Separate markers to avoid collisions with the other subtask fixtures. const SCHED_STANDALONE_MARKER = "SCHED_STANDALONE_INTERRUPT_RESUME" @@ -122,6 +132,81 @@ const completionAfterAnswer = (followupId: string, completionId: string) => ({ }) export function addSubtaskFixtures(mock: InstanceType) { + mock.addFixture({ + match: { + userMessage: new RegExp(SUBTASK_QUEUED_INPUT_PARENT_MARKER), + sequenceIndex: 0, + }, + response: { + toolCalls: [ + { + name: "new_task", + arguments: JSON.stringify({ + mode: "ask", + message: SUBTASK_QUEUED_INPUT_CHILD_PROMPT, + }), + id: "call_queued_input_parent_new_task_001", + }, + ], + }, + }) + + mock.addFixture({ + match: { + predicate: (req: ChatCompletionRequest) => + lastUserMessageContains(req, SUBTASK_QUEUED_INPUT_CHILD_MARKER) && + !requestContains(req, [SUBTASK_QUEUED_INPUT_PARENT_MARKER]) && + !requestContains(req, [SUBTASK_QUEUED_INPUT_MESSAGE]), + }, + streamingProfile: { ttft: SUBTASK_QUEUED_INPUT_RESPONSE_LATENCY_MS }, + response: { + toolCalls: [ + { + name: "attempt_completion", + arguments: JSON.stringify({ result: SUBTASK_QUEUED_INPUT_INITIAL_RESULT }), + id: "call_queued_input_child_initial_completion_002", + }, + ], + }, + }) + + mock.addFixture({ + match: { + predicate: (req: ChatCompletionRequest) => + requestContains(req, [SUBTASK_QUEUED_INPUT_CHILD_MARKER, SUBTASK_QUEUED_INPUT_MESSAGE]) && + !requestContains(req, [SUBTASK_QUEUED_INPUT_PARENT_MARKER]), + }, + response: { + toolCalls: [ + { + name: "attempt_completion", + arguments: JSON.stringify({ result: SUBTASK_QUEUED_INPUT_CHILD_RESULT }), + id: "call_queued_input_child_revised_completion_003", + }, + ], + }, + }) + + mock.addFixture({ + match: { + predicate: (req: ChatCompletionRequest) => + requestContains(req, [ + SUBTASK_QUEUED_INPUT_PARENT_MARKER, + SUBTASK_RESULT_INJECTION, + SUBTASK_QUEUED_INPUT_CHILD_RESULT, + ]), + }, + response: { + toolCalls: [ + { + name: "attempt_completion", + arguments: JSON.stringify({ result: SUBTASK_QUEUED_INPUT_PARENT_RESULT }), + id: "call_queued_input_parent_completion_004", + }, + ], + }, + }) + mock.addFixture({ match: { userMessage: new RegExp(SUBTASK_FAST_PARENT_MARKER), diff --git a/apps/vscode-e2e/src/suite/subtasks.test.ts b/apps/vscode-e2e/src/suite/subtasks.test.ts index 02d3dfe487..15abdaf3c8 100644 --- a/apps/vscode-e2e/src/suite/subtasks.test.ts +++ b/apps/vscode-e2e/src/suite/subtasks.test.ts @@ -25,6 +25,11 @@ import { SUBTASK_INTERRUPT_PARENT_PROMPT, SUBTASK_INTERRUPT_PARENT_RESULT, SUBTASK_PARENT_PROMPT, + SUBTASK_QUEUED_INPUT_CHILD_MARKER, + SUBTASK_QUEUED_INPUT_CHILD_RESULT, + SUBTASK_QUEUED_INPUT_MESSAGE, + SUBTASK_QUEUED_INPUT_PARENT_PROMPT, + SUBTASK_QUEUED_INPUT_PARENT_RESULT, SUBTASK_XPROFILE_DIFFERENT_CHILD_RESULT, SUBTASK_XPROFILE_PARENT_PROMPT, SUBTASK_XPROFILE_PARENT_RESULT, @@ -174,6 +179,73 @@ suite("Roo Code Subtasks", function () { } }) + test("queued input interrupts child completion before the parent resumes", async () => { + const api = globalThis.api + const says: Record = {} + + const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => { + if (message.type === "say" && message.partial === false) { + says[taskId] = says[taskId] || [] + says[taskId].push(message) + } + } + + api.on(RooCodeEventName.Message, messageHandler) + + try { + const parentTaskId = await api.startNewTask({ + configuration: { + mode: "ask", + alwaysAllowModeSwitch: true, + alwaysAllowSubtasks: true, + autoApprovalEnabled: true, + enableCheckpoints: false, + }, + text: SUBTASK_QUEUED_INPUT_PARENT_PROMPT, + }) + + let childTaskId: string | undefined + await waitFor(() => { + const current = api.getCurrentTaskStack().at(-1) + if (current && current !== parentTaskId) { + childTaskId = current + return true + } + return false + }) + + await waitForAimockRequestContaining(SUBTASK_QUEUED_INPUT_CHILD_MARKER) + + const completedParentTaskId = await waitUntilCompleted({ + api, + start: async () => { + await api.sendMessage(SUBTASK_QUEUED_INPUT_MESSAGE) + return parentTaskId + }, + }) + + assert.strictEqual(completedParentTaskId, parentTaskId) + assert.ok( + says[childTaskId!]?.some( + ({ say, text }) => + say === "completion_result" && text?.trim() === SUBTASK_QUEUED_INPUT_CHILD_RESULT, + ), + "Child should process the queued instruction before returning to its parent", + ) + assert.strictEqual( + says[parentTaskId]?.find(({ say }) => say === "completion_result")?.text?.trim(), + SUBTASK_QUEUED_INPUT_PARENT_RESULT, + "Parent should resume only after the child processes the queued instruction", + ) + } finally { + api.off(RooCodeEventName.Message, messageHandler) + while (api.getCurrentTaskStack().length > 0) { + await api.clearCurrentTask() + } + await waitFor(() => api.getCurrentTaskStack().length === 0).catch(() => {}) + } + }) + // Smoke: child completing normally must resume the parent task. test("child task returns to parent after normal completion", async () => { const api = globalThis.api diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 4be087394e..00820ab17a 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1362,16 +1362,7 @@ export class Task extends EventEmitter implements TaskLike { const message = this.messageQueueService.dequeueMessage() if (message) { - // Check if this is a tool approval ask that needs to be handled. - if (type === "tool" || type === "command" || type === "use_mcp_server") { - // For tool approvals, we need to approve first, then send - // the message if there's text/images. - this.handleWebviewAskResponse("yesButtonClicked", message.text, message.images) - } else { - // For other ask types (like followup or command_output), fulfill the ask - // directly. - this.handleWebviewAskResponse("messageResponse", message.text, message.images) - } + this.handleWebviewAskResponse("messageResponse", message.text, message.images) } } @@ -1388,13 +1379,7 @@ export class Task extends EventEmitter implements TaskLike { if (shouldDrainQueuedMessageForAsk && !this.messageQueueService.isEmpty()) { const message = this.messageQueueService.dequeueMessage() if (message) { - // If this is a tool approval ask, we need to approve first (yesButtonClicked) - // and include any queued text/images. - if (type === "tool" || type === "command" || type === "use_mcp_server") { - this.handleWebviewAskResponse("yesButtonClicked", message.text, message.images) - } else { - this.handleWebviewAskResponse("messageResponse", message.text, message.images) - } + this.handleWebviewAskResponse("messageResponse", message.text, message.images) } } diff --git a/src/core/task/__tests__/ask-queued-message-drain.spec.ts b/src/core/task/__tests__/ask-queued-message-drain.spec.ts index 06f577881e..db605ea938 100644 --- a/src/core/task/__tests__/ask-queued-message-drain.spec.ts +++ b/src/core/task/__tests__/ask-queued-message-drain.spec.ts @@ -1,35 +1,75 @@ import { Task } from "../Task" +import { MessageQueueService } from "../../message-queue/MessageQueueService" // Keep this test focused: if a queued message arrives while Task.ask() is blocked, // it should be consumed and used to fulfill the ask. +const buildTask = () => { + const task = Object.create(Task.prototype) as Task + + Object.assign(task, { + abort: false, + clineMessages: [], + askResponse: undefined, + askResponseText: undefined, + askResponseImages: undefined, + lastMessageTs: undefined, + messageQueueService: new MessageQueueService(), + addToClineMessages: vi.fn(async () => {}), + saveClineMessages: vi.fn(async () => {}), + updateClineMessage: vi.fn(async () => {}), + cancelAutoApprovalTimeout: vi.fn(() => {}), + checkpointSave: vi.fn(async () => {}), + emit: vi.fn(), + providerRef: { deref: () => undefined }, + }) + + return task +} + describe("Task.ask queued message drain", () => { + it.each(["tool", "command", "use_mcp_server"] as const)( + "treats queued input as feedback instead of approving a %s ask", + async (askType) => { + const task = buildTask() + task.messageQueueService.addMessage("change direction", ["queued-image.png"]) + + const result = await task.ask(askType, "pending approval", false) + + expect(result).toEqual({ + response: "messageResponse", + text: "change direction", + images: ["queued-image.png"], + }) + expect(task.messageQueueService.isEmpty()).toBe(true) + }, + ) + + it.each(["tool", "command", "use_mcp_server"] as const)( + "treats input queued while blocked as feedback instead of approving a %s ask", + async (askType) => { + const task = buildTask() + const askPromise = task.ask(askType, "pending approval", false) + + // Let ask() observe an empty queue and enter its pWaitFor loop before + // simulating input that arrives while the approval is already blocked. + await new Promise((resolve) => setTimeout(resolve, 0)) + task.messageQueueService.addMessage("change direction") + + await expect(askPromise).resolves.toMatchObject({ + response: "messageResponse", + text: "change direction", + }) + }, + ) + it("consumes queued message while blocked on followup ask", async () => { - const task = Object.create(Task.prototype) as Task - ;(task as any).abort = false - ;(task as any).clineMessages = [] - ;(task as any).askResponse = undefined - ;(task as any).askResponseText = undefined - ;(task as any).askResponseImages = undefined - ;(task as any).lastMessageTs = undefined - - // Message queue service exists in constructor; for unit test we can attach a real one. - const { MessageQueueService } = await import("../../message-queue/MessageQueueService") - ;(task as any).messageQueueService = new MessageQueueService() - - // Minimal stubs used by ask() - ;(task as any).addToClineMessages = vi.fn(async () => {}) - ;(task as any).saveClineMessages = vi.fn(async () => {}) - ;(task as any).updateClineMessage = vi.fn(async () => {}) - ;(task as any).cancelAutoApprovalTimeout = vi.fn(() => {}) - ;(task as any).checkpointSave = vi.fn(async () => {}) - ;(task as any).emit = vi.fn() - ;(task as any).providerRef = { deref: () => undefined } + const task = buildTask() const askPromise = task.ask("followup", "Q?", false) // Simulate webview queuing the user's selection text while the ask is pending. - ;(task as any).messageQueueService.addMessage("picked answer") + task.messageQueueService.addMessage("picked answer") const result = await askPromise expect(result.response).toBe("messageResponse") @@ -37,26 +77,10 @@ describe("Task.ask queued message drain", () => { }) it("does not consume queued messages for command_output asks", async () => { - const task = Object.create(Task.prototype) as Task - ;(task as any).abort = false - ;(task as any).clineMessages = [] - ;(task as any).askResponse = undefined - ;(task as any).askResponseText = undefined - ;(task as any).askResponseImages = undefined - ;(task as any).lastMessageTs = undefined - - const { MessageQueueService } = await import("../../message-queue/MessageQueueService") - ;(task as any).messageQueueService = new MessageQueueService() - ;(task as any).addToClineMessages = vi.fn(async () => {}) - ;(task as any).saveClineMessages = vi.fn(async () => {}) - ;(task as any).updateClineMessage = vi.fn(async () => {}) - ;(task as any).cancelAutoApprovalTimeout = vi.fn(() => {}) - ;(task as any).checkpointSave = vi.fn(async () => {}) - ;(task as any).emit = vi.fn() - ;(task as any).providerRef = { deref: () => undefined } + const task = buildTask() const askPromise = task.ask("command_output", "command is still running...", false) - ;(task as any).messageQueueService.addMessage("1+1=?") + task.messageQueueService.addMessage("1+1=?") setTimeout(() => { task.approveAsk() @@ -66,7 +90,7 @@ describe("Task.ask queued message drain", () => { expect(result.response).toBe("yesButtonClicked") expect(result.text).toBeUndefined() - expect((task as any).messageQueueService.isEmpty()).toBe(false) - expect((task as any).messageQueueService.messages[0]?.text).toBe("1+1=?") + expect(task.messageQueueService.isEmpty()).toBe(false) + expect(task.messageQueueService.messages[0]?.text).toBe("1+1=?") }) }) diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index f405adc8df..40a42020f0 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -834,11 +834,6 @@ "count": 19 } }, - "core/task/__tests__/ask-queued-message-drain.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 32 - } - }, "core/task/__tests__/flushPendingToolResultsToHistory.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 14