fix: make delegation reopen flow Roo v2-native (#11418)

This commit is contained in:
Daniel 2026-02-11 16:38:57 -05:00 committed by GitHub
parent b759b92f01
commit b51af98278
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 226 additions and 73 deletions

View file

@ -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<typeof import("../core/task-persistence")>()
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({

View file

@ -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<typeof import("../core/task-persistence")>()
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: {},

View file

@ -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

View file

@ -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)
})
})
})