diff --git a/apps/vscode-e2e/src/fixtures/subtasks.ts b/apps/vscode-e2e/src/fixtures/subtasks.ts index 772eb3306a..721af81630 100644 --- a/apps/vscode-e2e/src/fixtures/subtasks.ts +++ b/apps/vscode-e2e/src/fixtures/subtasks.ts @@ -7,6 +7,8 @@ const SUBTASK_PARENT_MARKER = "SUBTASK_PARENT_CANCELLATION_SMOKE" const SUBTASK_CHILD_MARKER = "SUBTASK_CHILD_CALCULATOR_SMOKE" const SUBTASK_INTERRUPT_PARENT_MARKER = "SUBTASK_PARENT_INTERRUPT_RESUME" const SUBTASK_INTERRUPT_CHILD_MARKER = "SUBTASK_CHILD_INTERRUPT_RESUME" +export const SUBTASK_API_HANG_PARENT_MARKER = "SUBTASK_PARENT_API_HANG_INTERRUPT_RESUME" +export const SUBTASK_API_HANG_CHILD_MARKER = "SUBTASK_CHILD_API_HANG_INTERRUPT_RESUME" const SUBTASK_FAST_PARENT_MARKER = "SUBTASK_PARENT_IMMEDIATE_COMPLETION" const SUBTASK_FAST_CHILD_MARKER = "SUBTASK_CHILD_IMMEDIATE_COMPLETION" const SUBTASK_XPROFILE_PARENT_MARKER = "SUBTASK_PARENT_CROSS_PROFILE" @@ -24,6 +26,12 @@ export const SUBTASK_INTERRUPT_PARENT_PROMPT = `${SUBTASK_INTERRUPT_PARENT_MARKE export const SUBTASK_INTERRUPT_CHILD_FOLLOWUP_ANSWER = "9" export const SUBTASK_INTERRUPT_PARENT_RESULT = "Interrupted parent resumed" +const SUBTASK_API_HANG_CHILD_PROMPT = `${SUBTASK_API_HANG_CHILD_MARKER}: Complete with the exact result "Hung child completed".` +export const SUBTASK_API_HANG_PARENT_PROMPT = `${SUBTASK_API_HANG_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_API_HANG_CHILD_PROMPT}" Do not answer directly. When the subtask returns, complete with the exact result "API hang parent resumed".` +export const SUBTASK_API_HANG_RESUME_MESSAGE = "Continue after provider hang." +export const SUBTASK_API_HANG_CHILD_RESULT = "Hung child completed" +export const SUBTASK_API_HANG_PARENT_RESULT = "API hang parent resumed" + const SUBTASK_XPROFILE_SAME_CHILD_PROMPT = `${SUBTASK_XPROFILE_SAME_CHILD_MARKER}: Complete immediately with the exact result "Same-profile child completed".` const SUBTASK_XPROFILE_DIFFERENT_CHILD_PROMPT = `${SUBTASK_XPROFILE_DIFFERENT_CHILD_MARKER}: Complete immediately with the exact result "Different-profile child completed".` export const SUBTASK_XPROFILE_PARENT_PROMPT = `${SUBTASK_XPROFILE_PARENT_MARKER}: First use new_task to create a code-mode subtask with this exact message: "${SUBTASK_XPROFILE_SAME_CHILD_PROMPT}" After it returns, create an ask-mode subtask with the next instructions you receive.` @@ -31,6 +39,8 @@ 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 apiHangChildMatch = new RegExp(SUBTASK_API_HANG_CHILD_MARKER) + const requestContains = (req: ChatCompletionRequest, expected: string[]) => { const rawRequest = JSON.stringify(req) return expected.every((text) => rawRequest.includes(text)) @@ -167,6 +177,79 @@ export function addSubtaskFixtures(mock: InstanceType) { }, }) + mock.addFixture({ + match: { + userMessage: new RegExp(SUBTASK_API_HANG_PARENT_MARKER), + sequenceIndex: 0, + }, + response: { + toolCalls: [ + { + name: "new_task", + arguments: JSON.stringify({ + mode: "ask", + message: SUBTASK_API_HANG_CHILD_PROMPT, + }), + id: "call_api_hang_parent_new_task_001", + }, + ], + }, + }) + + mock.addFixture({ + match: { + userMessage: apiHangChildMatch, + sequenceIndex: 0, + }, + // Keep the first child response pending long enough for the e2e test to cancel an in-flight API request. + latency: 15_000, + response: { + toolCalls: [ + { + name: "attempt_completion", + arguments: JSON.stringify({ result: SUBTASK_API_HANG_CHILD_RESULT }), + id: "call_api_hang_child_completion_002", + }, + ], + }, + }) + + mock.addFixture({ + match: { + userMessage: apiHangChildMatch, + sequenceIndex: 1, + }, + response: { + toolCalls: [ + { + name: "attempt_completion", + arguments: JSON.stringify({ result: SUBTASK_API_HANG_CHILD_RESULT }), + id: "call_api_hang_child_completion_003", + }, + ], + }, + }) + + mock.addFixture({ + match: { + predicate: (req: ChatCompletionRequest) => + requestContains(req, [ + SUBTASK_API_HANG_PARENT_MARKER, + "call_api_hang_parent_new_task_001", + SUBTASK_API_HANG_CHILD_RESULT, + ]), + }, + response: { + toolCalls: [ + { + name: "attempt_completion", + arguments: JSON.stringify({ result: SUBTASK_API_HANG_PARENT_RESULT }), + id: "call_api_hang_parent_completion_004", + }, + ], + }, + }) + // Issue #457 sequence: a same-profile child returns first, then the resumed // parent delegates to a child whose mode uses a different API profile. mock.addFixture({ diff --git a/apps/vscode-e2e/src/suite/subtasks.test.ts b/apps/vscode-e2e/src/suite/subtasks.test.ts index 05b8178b1f..3a81b8b492 100644 --- a/apps/vscode-e2e/src/suite/subtasks.test.ts +++ b/apps/vscode-e2e/src/suite/subtasks.test.ts @@ -5,6 +5,12 @@ import { RooCodeEventName, type ClineMessage } from "@roo-code/types" import { setDefaultSuiteTimeout } from "./test-utils" import { sleep, waitFor, waitUntilCompleted } from "./utils" import { + SUBTASK_API_HANG_CHILD_RESULT, + SUBTASK_API_HANG_CHILD_MARKER, + SUBTASK_API_HANG_PARENT_MARKER, + SUBTASK_API_HANG_PARENT_PROMPT, + SUBTASK_API_HANG_PARENT_RESULT, + SUBTASK_API_HANG_RESUME_MESSAGE, SUBTASK_CHILD_FOLLOWUP_ANSWER, SUBTASK_FAST_PARENT_PROMPT, SUBTASK_INTERRUPT_CHILD_FOLLOWUP_ANSWER, @@ -17,6 +23,45 @@ import { SUBTASK_XPROFILE_SAME_CHILD_RESULT, } from "../fixtures/subtasks" +type AimockMessageContent = string | Array<{ type?: string; text?: string }> + +type AimockJournalEntry = { + body?: { + messages?: Array<{ + role?: string + content?: AimockMessageContent + }> + } +} + +const messageContentText = (content?: AimockMessageContent) => { + if (typeof content === "string") { + return content + } + + return content?.map((part) => part.text ?? "").join("") ?? "" +} + +const waitForAimockRequestContaining = async (expectedText: string, excludeText?: string) => { + const aimockUrl = process.env.AIMOCK_URL + assert.ok(aimockUrl, "AIMOCK_URL must be set for aimock journal assertions") + + await waitFor(async () => { + const response = await fetch(`${aimockUrl}/__aimock/journal`) + const entries = (await response.json()) as AimockJournalEntry[] + + return entries.some((entry) => { + const messages = entry.body?.messages + if (!messages) return false + const entryText = messages.map((m) => messageContentText(m.content)).join("") + if (excludeText && entryText.includes(excludeText)) return false + return messages.some( + (message) => message.role === "user" && messageContentText(message.content).includes(expectedText), + ) + }) + }) +} + suite("Roo Code Subtasks", function () { setDefaultSuiteTimeout(this) @@ -412,6 +457,113 @@ suite("Roo Code Subtasks", function () { } }) + // Issue #566: a child interrupted while its provider request is still pending + // must keep its parent link when manually resumed and completed. + test("API-hung interrupted child resumes and returns to parent", async () => { + const api = globalThis.api + const asks: Record = {} + const says: Record = {} + + const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => { + if (message.type === "ask") { + asks[taskId] = asks[taskId] || [] + asks[taskId].push(message) + } + 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_API_HANG_PARENT_PROMPT, + }) + + let childTaskId: string | undefined + await waitFor(() => { + const stack = api.getCurrentTaskStack() + const current = stack[stack.length - 1] + if (current && current !== parentTaskId) { + childTaskId = current + return true + } + return false + }) + + await waitForAimockRequestContaining(SUBTASK_API_HANG_CHILD_MARKER, SUBTASK_API_HANG_PARENT_MARKER) + + await api.cancelCurrentTask() + + await waitFor(() => api.getCurrentTaskStack().at(-1) === childTaskId) + await waitFor( + () => asks[childTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ?? false, + ) + + const interruptedChild = await api.getTaskHistoryItem(childTaskId!) + assert.strictEqual(interruptedChild?.status, "interrupted", "Child should be interrupted after manual stop") + assert.strictEqual( + interruptedChild?.parentTaskId, + parentTaskId, + "Interrupted child should retain its parent link before resume", + ) + + const completedParentTaskId = await waitUntilCompleted({ + api, + start: async () => { + await api.sendMessage(SUBTASK_API_HANG_RESUME_MESSAGE) + return parentTaskId + }, + }) + + assert.strictEqual( + completedParentTaskId, + parentTaskId, + "Parent task should complete after API-hung child resumes and reports back", + ) + assert.strictEqual( + says[childTaskId!] + ?.filter(({ say }) => say === "completion_result") + .map(({ text }) => text?.trim()) + .find((text) => text === SUBTASK_API_HANG_CHILD_RESULT), + SUBTASK_API_HANG_CHILD_RESULT, + "Child should complete with its expected result after resume", + ) + assert.strictEqual( + says[parentTaskId] + ?.filter(({ say }) => say === "completion_result") + .map(({ text }) => text?.trim()) + .find((text) => text === SUBTASK_API_HANG_PARENT_RESULT), + SUBTASK_API_HANG_PARENT_RESULT, + "Parent should resume and complete with its expected result", + ) + + const parent = await api.getTaskHistoryItem(parentTaskId) + assert.notStrictEqual(parent?.status, "delegated", "Parent history should not remain delegated") + assert.strictEqual(parent?.awaitingChildId, undefined, "Parent awaitingChildId should be cleared") + assert.strictEqual(parent?.completedByChildId, childTaskId, "Parent should record completed child") + + const child = await api.getTaskHistoryItem(childTaskId!) + assert.strictEqual(child?.status, "completed", "Child history should be completed") + assert.strictEqual(child?.parentTaskId, parentTaskId, "Completed child should still point to parent") + } finally { + api.off(RooCodeEventName.Message, messageHandler) + while (api.getCurrentTaskStack().length > 0) { + await api.clearCurrentTask() + } + await waitFor(() => api.getCurrentTaskStack().length === 0).catch(() => {}) + } + }) + test("same-profile child returns before a different-profile child", async () => { const api = globalThis.api const says: Record = {} diff --git a/src/__tests__/history-resume-delegation.spec.ts b/src/__tests__/history-resume-delegation.spec.ts index 0be2d28a3f..13560a7434 100644 --- a/src/__tests__/history-resume-delegation.spec.ts +++ b/src/__tests__/history-resume-delegation.spec.ts @@ -26,6 +26,15 @@ vi.mock("vscode", () => { return { window, workspace, env, Uri, commands, ExtensionMode, version } }) +// Mock TelemetryService (needed by attemptCompletionTool's emitTaskCompleted) +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureTaskCompleted: vi.fn(), + }, + }, +})) + // Mock persistence BEFORE importing provider vi.mock("../core/task-persistence/taskMessages", () => ({ readTaskMessages: vi.fn().mockResolvedValue([]), @@ -1135,4 +1144,140 @@ describe("History resume delegation - parent metadata transitions", () => { expect(capturedParentResult?.completedByChildId).toBe("c-handoff") }) }) + + describe("Issue #566 — manual stop/resume of a delegated subtask", () => { + it("reopens the parent after a subtask is cancelled mid-stream, resumed, and completes (interrupted → attempt_completion)", async () => { + // Step 1: simulate cancelTask()'s persisted transition — child "active" → "interrupted", + // parent stays "delegated" with awaitingChildId intact (ClineProvider.ts cancelTaskInternal). + const childItem: Record = { + id: "child-566", + status: "interrupted", + parentTaskId: "parent-566", + ts: 1, + task: "Child task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + mode: "code", + workspace: "/tmp", + } + const parentItem: Record = { + id: "parent-566", + status: "delegated", + delegatedToId: "child-566", + awaitingChildId: "child-566", + childIds: ["child-566"], + ts: 0, + task: "Parent task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + mode: "code", + workspace: "/tmp", + } + + // Step 2: user clicks "Resume" — the extension rehydrates the child from its persisted + // history item (createTaskWithHistoryItem passes historyItem.status through as + // initialStatus), so the resumed task instance still reports "interrupted". + let currentActiveId: string | undefined = "child-566" + const emitSpy = vi.fn() + const removeClineFromStack = vi.fn().mockImplementation(async () => { + currentActiveId = undefined + }) + const createTaskWithHistoryItem = vi.fn().mockImplementation(async (historyItem: any) => { + currentActiveId = historyItem.id + return { + taskId: historyItem.id, + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + } + }) + const getTaskWithId = vi.fn(async (id: string) => { + const item = id === "child-566" ? childItem : id === "parent-566" ? parentItem : undefined + if (!item) throw new Error("Task not found") + return { historyItem: item } + }) + const taskHistoryStore = { + atomicUpdatePair: vi.fn( + async ( + firstId: string, + secondId: string, + firstUpdater: (h: any) => any, + secondUpdater: (h: any) => any, + ) => { + Object.assign(childItem, firstUpdater(childItem)) + Object.assign(parentItem, secondUpdater(parentItem)) + return [] + }, + ), + get: vi.fn((id: string) => + id === "child-566" ? childItem : id === "parent-566" ? parentItem : undefined, + ), + } + + const provider = makeProviderStub({ + contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, + getTaskWithId, + emit: emitSpy, + getCurrentTask: vi.fn(() => (currentActiveId ? ({ taskId: currentActiveId } as any) : undefined)), + removeClineFromStack, + createTaskWithHistoryItem, + taskHistoryStore, + reopenParentFromDelegation: vi.fn(async (params: any) => { + // Intentional self-reference: the provider variable is initialized before this stub is invoked. + return await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, params) + }), + } as unknown as ClineProvider) + + vi.mocked(readTaskMessages).mockResolvedValue([]) + vi.mocked(readApiMessages).mockResolvedValue([]) + + // Step 3: the resumed subtask finishes its work and calls attempt_completion. + const { attemptCompletionTool } = await import("../core/tools/AttemptCompletionTool") + const resumedChildTask = { + taskId: "child-566", + parentTask: undefined, // live parent reference is gone after resume; only parentTaskId survives + parentTaskId: "parent-566", + historyItem: { parentTaskId: "parent-566" }, + providerRef: { deref: () => provider }, + say: vi.fn().mockResolvedValue(undefined), + emit: vi.fn(), + getTokenUsage: vi.fn(() => ({})), + toolUsage: {}, + clineMessages: [], + userMessageContent: [], + consecutiveMistakeCount: 0, + emitFinalTokenUsageUpdate: vi.fn(), + } as unknown as import("../core/task/Task").Task + + const block = { + type: "tool_use", + name: "attempt_completion", + params: { result: "Child finished after resume" }, + nativeArgs: { result: "Child finished after resume" }, + partial: false, + } as any + + await attemptCompletionTool.handle(resumedChildTask, block, { + askApproval: vi.fn(), + handleError: vi.fn(async (_action: string, err: Error) => { + throw err + }), + pushToolResult: vi.fn(), + askFinishSubTaskApproval: vi.fn(async () => true), + toolDescription: () => "desc", + } as any) + + // The parent must regain control — this is the exact behavior issue #566 reported as broken. + expect(currentActiveId).toBe("parent-566") + expect(childItem.status).toBe("completed") + expect(parentItem.status).toBe("active") + expect(parentItem.awaitingChildId).toBeUndefined() + + const eventNames = emitSpy.mock.calls.map((c: any[]) => c[0]) + expect(eventNames).toContain(RooCodeEventName.TaskDelegationCompleted) + expect(eventNames).toContain(RooCodeEventName.TaskDelegationResumed) + }) + }) })