From 2cef66f69e302f0c22fa82dfb85b299922305f24 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Sun, 16 Aug 2026 17:31:07 +0800 Subject: [PATCH] feat(task): cancel cascade interrupts live children + surface depth in history tree --- src/__tests__/cancel-cascade.spec.ts | 128 ++++++++++++++++++ src/core/webview/ClineProvider.ts | 60 ++++++++ .../history/__tests__/useGroupedTasks.spec.ts | 39 ++++++ webview-ui/src/components/history/types.ts | 2 + .../src/components/history/useGroupedTasks.ts | 8 +- 5 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/cancel-cascade.spec.ts diff --git a/src/__tests__/cancel-cascade.spec.ts b/src/__tests__/cancel-cascade.spec.ts new file mode 100644 index 0000000000..0b471958ed --- /dev/null +++ b/src/__tests__/cancel-cascade.spec.ts @@ -0,0 +1,128 @@ +// npx vitest run __tests__/cancel-cascade.spec.ts + +import { describe, it, expect, vi } from "vitest" +import type { HistoryItem } from "@roo-code/types" +import { ClineProvider } from "../core/webview/ClineProvider" +import { TaskRegistry } from "../core/task/TaskRegistry" +import type { Task } from "../core/task/Task" + +/** + * Minimal live-child double carrying only the fields interruptLiveChildren touches. + * `abortTask` is a real mock so we can assert it was invoked (and that its inlineSubtask + * phase marker would be cleared by the abort path). + */ +function makeChildDouble(taskId: string, opts: { abort?: boolean; abandoned?: boolean } = {}) { + const abortTask = vi.fn().mockResolvedValue(undefined) + return { + taskId, + abort: opts.abort ?? false, + abandoned: opts.abandoned ?? false, + inlineSubtask: undefined as { message: string; todos: unknown[] } | undefined, + abortTask, + } +} + +function makeProvider(opts: { + parentChildIds?: string[] + childStatuses?: Record + children: Array> +}) { + const store = new Map() + store.set("parent-1", { + id: "parent-1", + task: "Parent", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + childIds: opts.parentChildIds ?? [], + } as unknown as HistoryItem) + for (const [id, status] of Object.entries(opts.childStatuses ?? {})) { + store.set(id, { + id, + task: `Child ${id}`, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + status, + } as unknown as HistoryItem) + } + + const registry = new TaskRegistry() + for (const child of opts.children) { + registry.push(child as unknown as Task) + } + + const updateTaskHistory = vi.fn().mockResolvedValue([]) + + return { + taskHistoryStore: { get: (id: string) => store.get(id) }, + taskRegistry: registry, + updateTaskHistory, + log: vi.fn(), + } +} + +async function callInterruptLiveChildren(provider: object, parentTaskId: string): Promise { + const proto = ClineProvider.prototype as unknown as { + interruptLiveChildren: (this: object, id: string) => Promise + } + await proto.interruptLiveChildren.call(provider, parentTaskId) +} + +describe("ClineProvider.cancel cascade — interruptLiveChildren", () => { + it("aborts live children and marks them interrupted", async () => { + const child = makeChildDouble("child-1") + const provider = makeProvider({ + parentChildIds: ["child-1"], + childStatuses: { "child-1": "active" }, + children: [child], + }) + + await callInterruptLiveChildren(provider, "parent-1") + + expect(child.abortTask).toHaveBeenCalledTimes(1) + const update = (provider as { updateTaskHistory: ReturnType }).updateTaskHistory + expect(update).toHaveBeenCalledWith(expect.objectContaining({ id: "child-1", status: "interrupted" })) + }) + + it("skips children already in a terminal state (never overwrites completed/interrupted)", async () => { + const done = makeChildDouble("child-done") + const interrupted = makeChildDouble("child-interrupted") + const provider = makeProvider({ + parentChildIds: ["child-done", "child-interrupted"], + childStatuses: { "child-done": "completed", "child-interrupted": "interrupted" }, + children: [done, interrupted], + }) + + await callInterruptLiveChildren(provider, "parent-1") + + expect(done.abortTask).not.toHaveBeenCalled() + expect(interrupted.abortTask).not.toHaveBeenCalled() + const update = (provider as { updateTaskHistory: ReturnType }).updateTaskHistory + expect(update).not.toHaveBeenCalled() + }) + + it("skips children that are not live in the registry (already aborted/abandoned or evicted)", async () => { + const abandoned = makeChildDouble("child-abandoned", { abandoned: true }) + const provider = makeProvider({ + parentChildIds: ["child-abandoned", "child-evicted"], // child-evicted has no registry entry + childStatuses: { "child-abandoned": "active", "child-evicted": "active" }, + children: [abandoned], + }) + + await callInterruptLiveChildren(provider, "parent-1") + + expect(abandoned.abortTask).not.toHaveBeenCalled() + const update = (provider as { updateTaskHistory: ReturnType }).updateTaskHistory + expect(update).not.toHaveBeenCalled() + }) + + it("is a no-op when the parent has no children", async () => { + const provider = makeProvider({ parentChildIds: [], childStatuses: {}, children: [] }) + + await callInterruptLiveChildren(provider, "parent-1") + + const update = (provider as { updateTaskHistory: ReturnType }).updateTaskHistory + expect(update).not.toHaveBeenCalled() + }) +}) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 038cd57710..bb74660d82 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -691,6 +691,58 @@ export class ClineProvider } } + /** + * Cancel cascade: interrupt every LIVE child of the given parent task. + * + * When a task is cancelled, any children it spawned that are still running in the + * registry would otherwise keep streaming as orphans. This aborts each live child + * instance (which also clears its inlineSubtask phase marker) and marks its persisted + * status "interrupted" so the user can resume it later. The parent's delegation link is + * left intact — this mirrors markDelegatedChildInterrupted() on the parent side. + * + * No-op when the task has no live children (the common single-open case, where the child + * is itself the current task and is handled by cancelTaskInternal's own interruption path). + */ + private async interruptLiveChildren(parentTaskId: string): Promise { + const parentHistory = this.taskHistoryStore.get(parentTaskId) + const childIds = parentHistory?.childIds ?? [] + + for (const childId of childIds) { + // Skip children already in a terminal state — never overwrite completed/interrupted. + const existingStatus = this.taskHistoryStore.get(childId)?.status + if (existingStatus === "interrupted" || existingStatus === "completed") { + continue + } + + // Only cascade to children that are actually running (not already aborted/abandoned). + const liveChild = this.taskRegistry.getById(childId) + if (!liveChild || liveChild.abort || liveChild.abandoned) { + continue + } + + this.log(`[interruptLiveChildren] Cancelling parent ${parentTaskId} — interrupting live child ${childId}`) + try { + // Abort the live instance (clears inlineSubtask, stops its stream). Fire-and-forget is + // acceptable: abortTask settles asynchronously and cancelTaskInternal awaits the parent's + // own abort promise before persisting "interrupted". + void liveChild.abortTask().catch((err) => { + this.log( + `[interruptLiveChildren] Failed to abort child ${childId}: ${err instanceof Error ? err.message : String(err)}`, + ) + }) + + const childHistory = this.taskHistoryStore.get(childId) + if (childHistory && childHistory.status !== "interrupted") { + await this.updateTaskHistory({ ...childHistory, status: "interrupted" }) + } + } catch (err) { + this.log( + `[interruptLiveChildren] Failed to interrupt child ${childId}: ${err instanceof Error ? err.message : String(err)}`, + ) + } + } + } + getTaskStackSize(): number { return this.taskRegistry.length } @@ -3455,6 +3507,14 @@ export class ClineProvider // before we persist "interrupted", so our write is always the last one. await abortPromise.catch(() => {}) + // Cancel cascade: interrupt any live children of this task so they don't keep streaming + // as orphans. No-op in the common single-open case (the child is the current task itself). + void this.interruptLiveChildren(task.taskId).catch((err) => { + this.log( + `[cancelTask] Failed to interrupt live children of ${task.taskId}: ${err instanceof Error ? err.message : String(err)}`, + ) + }) + // Defensive safeguard: if current instance already changed, skip rehydrate const current = this.getCurrentTask() if (current && current.instanceId !== originalInstanceId) { diff --git a/webview-ui/src/components/history/__tests__/useGroupedTasks.spec.ts b/webview-ui/src/components/history/__tests__/useGroupedTasks.spec.ts index 8873695c62..7efbbc04e0 100644 --- a/webview-ui/src/components/history/__tests__/useGroupedTasks.spec.ts +++ b/webview-ui/src/components/history/__tests__/useGroupedTasks.spec.ts @@ -558,6 +558,45 @@ describe("buildSubtree", () => { expect(node.children[0].children[0].isExpanded).toBe(true) // grandchild expanded expect(node.children[0].children[0].children[0].isExpanded).toBe(false) // great-grandchild not expanded }) + + it("derives depth from the persisted depth field when present", () => { + const root = createMockTask({ id: "root", task: "Root", depth: 0 }) + const child = createMockTask({ id: "child", task: "Child", parentTaskId: "root", depth: 1 }) + + const childrenMap = new Map() + childrenMap.set("root", [child]) + + const node = buildSubtree(root, childrenMap, new Set()) + + expect(node.depth).toBe(0) + expect(node.children[0].depth).toBe(1) + }) + + it("falls back to tree position (parentDepth + 1) for legacy items without depth", () => { + const root = createMockTask({ id: "root", task: "Root" }) // no depth field + const child = createMockTask({ id: "child", task: "Child", parentTaskId: "root" }) + const grandchild = createMockTask({ id: "grandchild", task: "Grandchild", parentTaskId: "child" }) + + const childrenMap = new Map() + childrenMap.set("root", [child]) + childrenMap.set("child", [grandchild]) + + const node = buildSubtree(root, childrenMap, new Set()) + + // Isolated root resolves to depth 0; descendants derive from tree position. + expect(node.depth).toBe(0) + expect(node.children[0].depth).toBe(1) + expect(node.children[0].children[0].depth).toBe(2) + }) + + it("honors an explicit parentDepth argument over the default -1", () => { + const child = createMockTask({ id: "child", task: "Child" }) // no depth field + + // Called with parentDepth=4 (as if nested under a depth-4 node) → resolves to 5. + const node = buildSubtree(child, new Map(), new Set(), 4) + + expect(node.depth).toBe(5) + }) }) describe("countAllSubtasks", () => { diff --git a/webview-ui/src/components/history/types.ts b/webview-ui/src/components/history/types.ts index 0de5e43081..e67bbefaf0 100644 --- a/webview-ui/src/components/history/types.ts +++ b/webview-ui/src/components/history/types.ts @@ -20,6 +20,8 @@ export interface SubtaskTreeNode { children: SubtaskTreeNode[] /** Whether this node's children are expanded in the UI */ isExpanded: boolean + /** Nesting depth of this node (root = 0). From persisted `depth` when present, else parentDepth + 1. */ + depth?: number } /** diff --git a/webview-ui/src/components/history/useGroupedTasks.ts b/webview-ui/src/components/history/useGroupedTasks.ts index d3f3d4e953..8e89079b38 100644 --- a/webview-ui/src/components/history/useGroupedTasks.ts +++ b/webview-ui/src/components/history/useGroupedTasks.ts @@ -9,19 +9,25 @@ import type { DisplayHistoryItem, SubtaskTreeNode, TaskGroup, GroupedTasksResult * @param task - The task to build a tree node for * @param childrenMap - Map of parentId → direct children * @param expandedIds - Set of task IDs whose children are currently expanded + * @param parentDepth - Depth of the parent node; fallback when `task.depth` is unset (legacy data). Defaults to -1 so an isolated root resolves to depth 0. * @returns A SubtaskTreeNode with recursively built children sorted by ts (newest first) */ export function buildSubtree( task: HistoryItem, childrenMap: Map, expandedIds: Set, + parentDepth = -1, ): SubtaskTreeNode { const directChildren = (childrenMap.get(task.id) || []).slice().sort((a, b) => b.ts - a.ts) + // Prefer the persisted depth; fall back to tree position for legacy items. + const nodeDepth = task.depth ?? parentDepth + 1 + return { item: task as DisplayHistoryItem, - children: directChildren.map((child) => buildSubtree(child, childrenMap, expandedIds)), + children: directChildren.map((child) => buildSubtree(child, childrenMap, expandedIds, nodeDepth)), isExpanded: expandedIds.has(task.id), + depth: nodeDepth, } }