From 8083b7d935395dde54a6634b6f4a5a2a3240cd1d Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 28 Jan 2026 05:04:51 +0000 Subject: [PATCH] fix(EXT-665): push tool_result BEFORE delegation to prevent loss Root cause: In NewTaskTool.execute(), pushToolResult was called AFTER delegateParentAndOpenChild(), but delegateParentAndOpenChild disposes the parent task. This meant the tool_result was lost, and flushPendingToolResultsToHistory generated a placeholder "interrupted" tool_result instead. Fix: Push the tool_result BEFORE calling delegateParentAndOpenChild. Since the child taskId is not yet known, use a generic message. The actual completion result is injected by reopenParentFromDelegation when the child completes. Also keep the defensive duplicate check in reopenParentFromDelegation for backward compatibility with any existing task histories. --- src/core/tools/NewTaskTool.ts | 13 ++++++++++--- src/core/tools/__tests__/newTaskTool.spec.ts | 16 ++++++++-------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/core/tools/NewTaskTool.ts b/src/core/tools/NewTaskTool.ts index f36d8e1e37..bc95d9edb6 100644 --- a/src/core/tools/NewTaskTool.ts +++ b/src/core/tools/NewTaskTool.ts @@ -109,16 +109,23 @@ export class NewTaskTool extends BaseTool<"new_task"> { return } + // IMPORTANT: Push the tool_result BEFORE delegation, because delegateParentAndOpenChild + // disposes the parent task. If we push after, the tool_result is lost and + // flushPendingToolResultsToHistory will generate a placeholder "interrupted" tool_result, + // causing duplicate tool_results when the child completes (EXT-665). + // + // The child taskId isn't known yet, so we use a generic message. The actual completion + // result will be injected by reopenParentFromDelegation when the child completes. + pushToolResult(`Delegating to subtask...`) + // Delegate parent and open child as sole active task - const child = await (provider as any).delegateParentAndOpenChild({ + await (provider as any).delegateParentAndOpenChild({ parentTaskId: task.taskId, message: unescapedMessage, initialTodos: todoItems, mode, }) - // Reflect delegation in tool result (no pause/unpause, no wait) - pushToolResult(`Delegated to child task ${child.taskId}`) return } catch (error) { await handleError("creating new task", error) diff --git a/src/core/tools/__tests__/newTaskTool.spec.ts b/src/core/tools/__tests__/newTaskTool.spec.ts index fc383c13ee..5bce727391 100644 --- a/src/core/tools/__tests__/newTaskTool.spec.ts +++ b/src/core/tools/__tests__/newTaskTool.spec.ts @@ -175,7 +175,7 @@ describe("newTaskTool", () => { ) // Verify side effects - expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Delegated to child task")) + expect(mockPushToolResult).toHaveBeenCalledWith("Delegating to subtask...") }) it("should not un-escape single escaped \@", async () => { @@ -280,7 +280,7 @@ describe("newTaskTool", () => { expect(mockStartSubtask).toHaveBeenCalledWith("Test message", [], "code") // Should complete successfully - expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Delegated to child task")) + expect(mockPushToolResult).toHaveBeenCalledWith("Delegating to subtask...") }) it("should work with todos parameter when provided", async () => { @@ -311,7 +311,7 @@ describe("newTaskTool", () => { "code", ) - expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Delegated to child task")) + expect(mockPushToolResult).toHaveBeenCalledWith("Delegating to subtask...") }) it("should error when mode parameter is missing", async () => { @@ -423,7 +423,7 @@ describe("newTaskTool", () => { expect(mockStartSubtask).toHaveBeenCalledWith("Test message", [], "code") // Should complete successfully - expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Delegated to child task")) + expect(mockPushToolResult).toHaveBeenCalledWith("Delegating to subtask...") }) it("should REQUIRE todos when VSCode setting is enabled", async () => { @@ -501,7 +501,7 @@ describe("newTaskTool", () => { ) // Should complete successfully - expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Delegated to child task")) + expect(mockPushToolResult).toHaveBeenCalledWith("Delegating to subtask...") }) it("should work with empty todos string when VSCode setting is enabled", async () => { @@ -536,7 +536,7 @@ describe("newTaskTool", () => { expect(mockStartSubtask).toHaveBeenCalledWith("Test message", [], "code") // Should complete successfully - expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Delegated to child task")) + expect(mockPushToolResult).toHaveBeenCalledWith("Delegating to subtask...") }) it("should check VSCode setting with Package.name configuration key", async () => { @@ -671,7 +671,7 @@ describe("newTaskTool delegation flow", () => { ) expect(pauseEvents.length).toBe(0) - // Assert: tool result reflects delegation - expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Delegated to child task child-1")) + // Assert: tool result reflects delegation (pushed BEFORE delegation, so no child ID yet) + expect(mockPushToolResult).toHaveBeenCalledWith("Delegating to subtask...") }) })