Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions src/__tests__/cancel-cascade.spec.ts
Original file line number Diff line number Diff line change
@@ -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<string, "active" | "interrupted" | "completed">
children: Array<ReturnType<typeof makeChildDouble>>
}) {
const store = new Map<string, HistoryItem>()
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<void> {
const proto = ClineProvider.prototype as unknown as {
interruptLiveChildren: (this: object, id: string) => Promise<void>
}
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<typeof vi.fn> }).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<typeof vi.fn> }).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<typeof vi.fn> }).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<typeof vi.fn> }).updateTaskHistory
expect(update).not.toHaveBeenCalled()
})
})
60 changes: 60 additions & 0 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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
}
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, HistoryItem[]>()
childrenMap.set("root", [child])

const node = buildSubtree(root, childrenMap, new Set<string>())

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<string, HistoryItem[]>()
childrenMap.set("root", [child])
childrenMap.set("child", [grandchild])

const node = buildSubtree(root, childrenMap, new Set<string>())

// 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<string, HistoryItem[]>(), new Set<string>(), 4)

expect(node.depth).toBe(5)
})
})

describe("countAllSubtasks", () => {
Expand Down
2 changes: 2 additions & 0 deletions webview-ui/src/components/history/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand Down
8 changes: 7 additions & 1 deletion webview-ui/src/components/history/useGroupedTasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, HistoryItem[]>,
expandedIds: Set<string>,
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,
}
}

Expand Down
Loading