diff --git a/src/__tests__/history-resume-delegation.spec.ts b/src/__tests__/history-resume-delegation.spec.ts index a78c41b7c06..fae79bc2912 100644 --- a/src/__tests__/history-resume-delegation.spec.ts +++ b/src/__tests__/history-resume-delegation.spec.ts @@ -29,15 +29,19 @@ vi.mock("vscode", () => { vi.mock("../core/task-persistence/taskMessages", () => ({ readTaskMessages: vi.fn().mockResolvedValue([]), })) -vi.mock("../core/task-persistence", () => ({ - readApiMessages: vi.fn().mockResolvedValue([]), - saveApiMessages: vi.fn().mockResolvedValue(undefined), - saveTaskMessages: vi.fn().mockResolvedValue(undefined), -})) +vi.mock("../core/task-persistence", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + readRooMessages: vi.fn().mockResolvedValue([]), + saveRooMessages: vi.fn().mockResolvedValue(undefined), + saveTaskMessages: vi.fn().mockResolvedValue(undefined), + } +}) import { ClineProvider } from "../core/webview/ClineProvider" import { readTaskMessages } from "../core/task-persistence/taskMessages" -import { readApiMessages, saveApiMessages, saveTaskMessages } from "../core/task-persistence" +import { readRooMessages, saveRooMessages, saveTaskMessages } from "../core/task-persistence" describe("History resume delegation - parent metadata transitions", () => { beforeEach(() => { @@ -83,7 +87,7 @@ describe("History resume delegation - parent metadata transitions", () => { // Mock persistence reads to return empty arrays vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { parentTaskId: "parent-1", @@ -152,7 +156,7 @@ describe("History resume delegation - parent metadata transitions", () => { const existingApiMessages = [{ role: "user", content: [{ type: "text", text: "Old request" }], ts: 50 }] vi.mocked(readTaskMessages).mockResolvedValue(existingUiMessages as any) - vi.mocked(readApiMessages).mockResolvedValue(existingApiMessages as any) + vi.mocked(readRooMessages).mockResolvedValue(existingApiMessages as any) await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { parentTaskId: "p1", @@ -176,7 +180,7 @@ describe("History resume delegation - parent metadata transitions", () => { ) // Verify API history injection (user role message) - expect(saveApiMessages).toHaveBeenCalledWith( + expect(saveRooMessages).toHaveBeenCalledWith( expect.objectContaining({ messages: expect.arrayContaining([ expect.objectContaining({ @@ -198,11 +202,11 @@ describe("History resume delegation - parent metadata transitions", () => { const uiCall = vi.mocked(saveTaskMessages).mock.calls[0][0] expect(uiCall.messages).toHaveLength(2) // 1 original + 1 injected - const apiCall = vi.mocked(saveApiMessages).mock.calls[0][0] + const apiCall = vi.mocked(saveRooMessages).mock.calls[0][0] expect(apiCall.messages).toHaveLength(2) // 1 original + 1 injected }) - it("reopenParentFromDelegation injects tool_result when new_task tool_use exists in API history", async () => { + it("reopenParentFromDelegation injects tool-result message when new_task tool_use exists in API history", async () => { const provider = { contextProxy: { globalStorageUri: { fsPath: "/storage" } }, getTaskWithId: vi.fn().mockResolvedValue({ @@ -249,7 +253,7 @@ describe("History resume delegation - parent metadata transitions", () => { ] vi.mocked(readTaskMessages).mockResolvedValue(existingUiMessages as any) - vi.mocked(readApiMessages).mockResolvedValue(existingApiMessages as any) + vi.mocked(readRooMessages).mockResolvedValue(existingApiMessages as any) await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { parentTaskId: "p-tool", @@ -257,17 +261,19 @@ describe("History resume delegation - parent metadata transitions", () => { completionResultSummary: "Subtask completed via tool_result", }) - // Verify API history injection uses tool_result (not text fallback) - expect(saveApiMessages).toHaveBeenCalledWith( + // Verify API history injection uses tool-result (not text fallback) + expect(saveRooMessages).toHaveBeenCalledWith( expect.objectContaining({ messages: expect.arrayContaining([ expect.objectContaining({ - role: "user", + role: "tool", content: expect.arrayContaining([ expect.objectContaining({ - type: "tool_result", - tool_use_id: "toolu_abc123", - content: expect.stringContaining("Subtask c-tool completed"), + type: "tool-result", + toolCallId: "toolu_abc123", + output: expect.objectContaining({ + value: expect.stringContaining("Subtask c-tool completed"), + }), }), ]), }), @@ -277,15 +283,15 @@ describe("History resume delegation - parent metadata transitions", () => { }), ) - // Verify total message count: 2 original + 1 injected user message with tool_result - const apiCall = vi.mocked(saveApiMessages).mock.calls[0][0] + // Verify total message count: 2 original + 1 injected tool message with tool-result + const apiCall = vi.mocked(saveRooMessages).mock.calls[0][0] expect(apiCall.messages).toHaveLength(3) - // Verify the injected message is a user message with tool_result type - const injectedMsg = apiCall.messages[2] - expect(injectedMsg.role).toBe("user") - expect((injectedMsg.content[0] as any).type).toBe("tool_result") - expect((injectedMsg.content[0] as any).tool_use_id).toBe("toolu_abc123") + // Verify the injected message is a tool message with tool-result type + const injectedMsg = apiCall.messages[2] as any + expect(injectedMsg.role).toBe("tool") + expect((injectedMsg.content[0] as any).type).toBe("tool-result") + expect((injectedMsg.content[0] as any).toolCallId).toBe("toolu_abc123") }) it("reopenParentFromDelegation injects plain text when no new_task tool_use exists in API history", async () => { @@ -321,7 +327,7 @@ describe("History resume delegation - parent metadata transitions", () => { const existingApiMessages = [{ role: "user", content: [{ type: "text", text: "Create a subtask" }], ts: 40 }] vi.mocked(readTaskMessages).mockResolvedValue(existingUiMessages as any) - vi.mocked(readApiMessages).mockResolvedValue(existingApiMessages as any) + vi.mocked(readRooMessages).mockResolvedValue(existingApiMessages as any) await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { parentTaskId: "p-no-tool", @@ -329,10 +335,10 @@ describe("History resume delegation - parent metadata transitions", () => { completionResultSummary: "Subtask completed without tool_use", }) - const apiCall = vi.mocked(saveApiMessages).mock.calls[0][0] + const apiCall = vi.mocked(saveRooMessages).mock.calls[0][0] // Should append a user text note expect(apiCall.messages).toHaveLength(2) - const injected = apiCall.messages[1] + const injected = apiCall.messages[1] as any expect(injected.role).toBe("user") expect((injected.content[0] as any).type).toBe("text") expect((injected.content[0] as any).text).toContain("Subtask c-no-tool completed") @@ -372,7 +378,7 @@ describe("History resume delegation - parent metadata transitions", () => { } as unknown as ClineProvider vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { parentTaskId: "parent-2", @@ -416,7 +422,7 @@ describe("History resume delegation - parent metadata transitions", () => { } as unknown as ClineProvider vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { parentTaskId: "p3", @@ -494,7 +500,7 @@ describe("History resume delegation - parent metadata transitions", () => { } as unknown as ClineProvider vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) await expect( (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { @@ -552,7 +558,7 @@ describe("History resume delegation - parent metadata transitions", () => { } as unknown as ClineProvider vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { parentTaskId: "p4", @@ -616,7 +622,7 @@ describe("History resume delegation - parent metadata transitions", () => { } as unknown as ClineProvider vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { parentTaskId: "parent-rpd02", @@ -697,7 +703,7 @@ describe("History resume delegation - parent metadata transitions", () => { } as unknown as ClineProvider vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) await expect( (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { @@ -752,7 +758,7 @@ describe("History resume delegation - parent metadata transitions", () => { // Mock read failures or empty returns vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) await expect( (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { @@ -774,7 +780,7 @@ describe("History resume delegation - parent metadata transitions", () => { }), ) - expect(saveApiMessages).toHaveBeenCalledWith( + expect(saveRooMessages).toHaveBeenCalledWith( expect.objectContaining({ messages: [ expect.objectContaining({ diff --git a/src/__tests__/nested-delegation-resume.spec.ts b/src/__tests__/nested-delegation-resume.spec.ts index 5dbafc949cb..93181b28640 100644 --- a/src/__tests__/nested-delegation-resume.spec.ts +++ b/src/__tests__/nested-delegation-resume.spec.ts @@ -43,17 +43,21 @@ vi.mock("vscode", () => { vi.mock("../core/task-persistence/taskMessages", () => ({ readTaskMessages: vi.fn().mockResolvedValue([]), })) -vi.mock("../core/task-persistence", () => ({ - readApiMessages: vi.fn().mockResolvedValue([]), - saveApiMessages: vi.fn().mockResolvedValue(undefined), - saveTaskMessages: vi.fn().mockResolvedValue(undefined), -})) +vi.mock("../core/task-persistence", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + readRooMessages: vi.fn().mockResolvedValue([]), + saveRooMessages: vi.fn().mockResolvedValue(undefined), + saveTaskMessages: vi.fn().mockResolvedValue(undefined), + } +}) import { attemptCompletionTool } from "../core/tools/AttemptCompletionTool" import { ClineProvider } from "../core/webview/ClineProvider" import type { Task } from "../core/task/Task" import { readTaskMessages } from "../core/task-persistence/taskMessages" -import { readApiMessages, saveApiMessages, saveTaskMessages } from "../core/task-persistence" +import { readRooMessages, saveRooMessages, saveTaskMessages } from "../core/task-persistence" describe("Nested delegation resume (A → B → C)", () => { beforeEach(() => { @@ -164,7 +168,7 @@ describe("Nested delegation resume (A → B → C)", () => { // Empty histories for simplicity vi.mocked(readTaskMessages).mockResolvedValue([]) - vi.mocked(readApiMessages).mockResolvedValue([]) + vi.mocked(readRooMessages).mockResolvedValue([]) // Step 1: C completes -> should reopen B automatically const clineC = { @@ -174,6 +178,7 @@ describe("Nested delegation resume (A → B → C)", () => { historyItem: { parentTaskId: "B" }, providerRef: { deref: () => provider }, say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked", text: undefined, images: undefined }), emit: vi.fn(), getTokenUsage: vi.fn(() => ({})), toolUsage: {}, @@ -221,6 +226,7 @@ describe("Nested delegation resume (A → B → C)", () => { historyItem: { parentTaskId: "A" }, providerRef: { deref: () => provider }, say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked", text: undefined, images: undefined }), emit: vi.fn(), getTokenUsage: vi.fn(() => ({})), toolUsage: {}, diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 1538cd7bcca..0f42d6c5fb9 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -98,11 +98,17 @@ import { Task } from "../task/Task" import { webviewMessageHandler } from "./webviewMessageHandler" import type { ClineMessage, TodoItem } from "@roo-code/types" import { - readApiMessages, readRooMessages, - saveApiMessages, + saveRooMessages, saveTaskMessages, type RooMessage, + isRooAssistantMessage, + isRooToolMessage, + isAnyToolCallBlock, + isAnyToolResultBlock, + getToolCallId, + getToolCallName, + getToolResultCallId, } from "../task-persistence" import { readTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" @@ -3411,12 +3417,12 @@ export class ClineProvider parentClineMessages = [] } - let parentApiMessages: any[] = [] + let parentApiMessages: RooMessage[] = [] try { - parentApiMessages = (await readApiMessages({ + parentApiMessages = await readRooMessages({ taskId: parentTaskId, globalStoragePath, - })) as any[] + }) } catch { parentApiMessages = [] } @@ -3437,14 +3443,15 @@ export class ClineProvider parentClineMessages.push(subtaskUiMessage) await saveTaskMessages({ messages: parentClineMessages, taskId: parentTaskId, globalStoragePath }) - // Find the tool_use_id from the last assistant message's new_task tool_use + // Find the tool call ID from the last assistant message's new_task tool call let toolUseId: string | undefined for (let i = parentApiMessages.length - 1; i >= 0; i--) { const msg = parentApiMessages[i] - if (msg.role === "assistant" && Array.isArray(msg.content)) { + if (isRooAssistantMessage(msg) && Array.isArray(msg.content)) { for (const block of msg.content) { - if (block.type === "tool_use" && block.name === "new_task") { - toolUseId = block.id + const typedBlock = block as unknown as { type: string } + if (isAnyToolCallBlock(typedBlock) && getToolCallName(typedBlock) === "new_task") { + toolUseId = getToolCallId(typedBlock) break } } @@ -3452,51 +3459,63 @@ export class ClineProvider } } - // Preferred: if the parent history contains the native tool_use for new_task, - // inject a matching tool_result for the Anthropic message contract: - // user → assistant (tool_use) → user (tool_result) + // Preferred: if the parent history contains a new_task tool call, + // inject a matching tool result for the model message contract. if (toolUseId) { - // Check if the last message is already a user message with a tool_result for this tool_use_id + // Check if the last message already contains a tool result for this tool call ID // (in case this is a retry or the history was already updated) const lastMsg = parentApiMessages[parentApiMessages.length - 1] let alreadyHasToolResult = false - if (lastMsg?.role === "user" && Array.isArray(lastMsg.content)) { + if (lastMsg && "role" in lastMsg && Array.isArray(lastMsg.content)) { for (const block of lastMsg.content) { - if (block.type === "tool_result" && block.tool_use_id === toolUseId) { - // Update the existing tool_result content - block.content = `Subtask ${childTaskId} completed.\n\nResult:\n${completionResultSummary}` + const typedBlock = block as unknown as { type: string } + if (isAnyToolResultBlock(typedBlock) && getToolResultCallId(typedBlock) === toolUseId) { + const updatedText = `Subtask ${childTaskId} completed.\n\nResult:\n${completionResultSummary}` + if (typedBlock.type === "tool-result") { + ;(typedBlock as { output: { type: "text"; value: string } }).output = { + type: "text", + value: updatedText, + } + } else { + ;(typedBlock as { content: string }).content = updatedText + } alreadyHasToolResult = true break } } } - // If no existing tool_result found, create a NEW user message with the tool_result + // If no existing tool result found, create a NEW tool message with the tool result if (!alreadyHasToolResult) { parentApiMessages.push({ - role: "user", + role: "tool", content: [ { - type: "tool_result" as const, - tool_use_id: toolUseId, - content: `Subtask ${childTaskId} completed.\n\nResult:\n${completionResultSummary}`, + type: "tool-result" as const, + toolCallId: toolUseId, + toolName: "new_task", + output: { + type: "text" as const, + value: `Subtask ${childTaskId} completed.\n\nResult:\n${completionResultSummary}`, + }, }, ], ts, }) } - // Validate the newly injected tool_result against the preceding assistant message. - // This ensures the tool_result's tool_use_id matches a tool_use in the immediately - // preceding assistant message (Anthropic API requirement). + // Validate the newly injected/updated tool result against the preceding assistant message. const lastMessage = parentApiMessages[parentApiMessages.length - 1] - if (lastMessage?.role === "user") { + if ( + lastMessage && + (isRooToolMessage(lastMessage) || ("role" in lastMessage && lastMessage.role === "user")) + ) { const validatedMessage = validateAndFixToolResultIds(lastMessage, parentApiMessages.slice(0, -1)) - parentApiMessages[parentApiMessages.length - 1] = validatedMessage + parentApiMessages[parentApiMessages.length - 1] = validatedMessage as RooMessage } } else { - // If there is no corresponding tool_use in the parent API history, we cannot emit a - // tool_result. Fall back to a plain user text note so the parent can still resume. + // If there is no corresponding tool call in the parent API history, we cannot emit a + // tool result. Fall back to a plain user text note so the parent can still resume. parentApiMessages.push({ role: "user", content: [ @@ -3509,7 +3528,14 @@ export class ClineProvider }) } - await saveApiMessages({ messages: parentApiMessages as any, taskId: parentTaskId, globalStoragePath }) + const savedApiMessages = await saveRooMessages({ + messages: parentApiMessages, + taskId: parentTaskId, + globalStoragePath, + }) + if (savedApiMessages === false) { + this.log(`[reopenParentFromDelegation] Failed to save API messages for parent ${parentTaskId}`) + } // 3) Close child instance if still open (single-open-task invariant). // This MUST happen BEFORE updating the child's status to "completed" because diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index a071799881e..f8667de5c97 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -213,6 +213,7 @@ vi.mock("../../task/Task", () => ({ Task: vi.fn().mockImplementation((options: any) => ({ api: undefined, abortTask: vi.fn(), + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), handleWebviewAskResponse: vi.fn(), clineMessages: [], apiConversationHistory: [], @@ -347,6 +348,7 @@ describe("ClineProvider", () => { const task: any = { api: undefined, abortTask: vi.fn(), + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), handleWebviewAskResponse: vi.fn(), clineMessages: [], apiConversationHistory: [], @@ -3869,4 +3871,117 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { vi.mocked(fsUtils.fileExistsAtPath).mockRestore() }) }) + + describe("reopenParentFromDelegation", () => { + it("reads/writes Roo messages and appends a tool result for new_task tool-call", async () => { + const parentTaskId = "parent-task" + const childTaskId = "child-task" + const completionResultSummary = "child completed work" + + vi.spyOn(provider, "getTaskWithId").mockImplementation(async (taskId: string) => { + if (taskId === parentTaskId) { + return { + historyItem: { id: parentTaskId, childIds: [] }, + } as any + } + return { + historyItem: { id: childTaskId, status: "active" }, + } as any + }) + + vi.spyOn(provider, "getCurrentTask").mockReturnValue(undefined as any) + vi.spyOn(provider, "updateTaskHistory").mockResolvedValue(undefined as any) + + const taskMessages = await import("../../task-persistence/taskMessages") + vi.spyOn(taskMessages, "readTaskMessages").mockResolvedValue([]) + vi.spyOn(taskMessages, "saveTaskMessages").mockResolvedValue(undefined) + + const persistence = await import("../../task-persistence") + vi.spyOn(persistence, "readRooMessages").mockResolvedValue([ + { + role: "assistant", + content: [{ type: "tool-call", toolCallId: "call_new_task_1", toolName: "new_task", input: {} }], + ts: 1, + }, + { role: "user", content: [{ type: "text", text: "continuation" }], ts: 2 }, + ] as any) + const saveRooMessagesSpy = vi.spyOn(persistence, "saveRooMessages").mockResolvedValue(true) + + await provider.reopenParentFromDelegation({ parentTaskId, childTaskId, completionResultSummary }) + + expect(persistence.readRooMessages).toHaveBeenCalledWith({ + taskId: parentTaskId, + globalStoragePath: "/test/storage/path", + }) + expect(saveRooMessagesSpy).toHaveBeenCalledTimes(1) + + const savedPayload = saveRooMessagesSpy.mock.calls[0][0] + expect(savedPayload.taskId).toBe(parentTaskId) + expect(savedPayload.globalStoragePath).toBe("/test/storage/path") + expect(savedPayload.messages).toHaveLength(3) + const last = savedPayload.messages[2] as any + expect(last.role).toBe("tool") + expect(last.content[0]).toMatchObject({ + type: "tool-result", + toolCallId: "call_new_task_1", + toolName: "new_task", + }) + expect(last.content[0].output.value).toContain(completionResultSummary) + }) + + it("updates existing trailing tool result instead of appending a duplicate", async () => { + const parentTaskId = "parent-task" + const childTaskId = "child-task" + const completionResultSummary = "updated child summary" + + vi.spyOn(provider, "getTaskWithId").mockImplementation(async (taskId: string) => { + if (taskId === parentTaskId) { + return { + historyItem: { id: parentTaskId, childIds: [] }, + } as any + } + return { + historyItem: { id: childTaskId, status: "active" }, + } as any + }) + + vi.spyOn(provider, "getCurrentTask").mockReturnValue(undefined as any) + vi.spyOn(provider, "updateTaskHistory").mockResolvedValue(undefined as any) + + const taskMessages = await import("../../task-persistence/taskMessages") + vi.spyOn(taskMessages, "readTaskMessages").mockResolvedValue([]) + vi.spyOn(taskMessages, "saveTaskMessages").mockResolvedValue(undefined) + + const persistence = await import("../../task-persistence") + vi.spyOn(persistence, "readRooMessages").mockResolvedValue([ + { + role: "assistant", + content: [{ type: "tool-call", toolCallId: "call_new_task_2", toolName: "new_task", input: {} }], + ts: 1, + }, + { + role: "tool", + content: [ + { + type: "tool-result", + toolCallId: "call_new_task_2", + toolName: "new_task", + output: { type: "text", value: "old summary" }, + }, + ], + ts: 2, + }, + ] as any) + const saveRooMessagesSpy = vi.spyOn(persistence, "saveRooMessages").mockResolvedValue(true) + + await provider.reopenParentFromDelegation({ parentTaskId, childTaskId, completionResultSummary }) + + const savedPayload = saveRooMessagesSpy.mock.calls[0][0] + expect(savedPayload.messages).toHaveLength(2) + const last = savedPayload.messages[1] as any + expect(last.role).toBe("tool") + expect(last.content[0].toolCallId).toBe("call_new_task_2") + expect(last.content[0].output.value).toContain(completionResultSummary) + }) + }) })