Skip to content
Draft
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
281 changes: 281 additions & 0 deletions src/__tests__/removeClineFromStack-delegation.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,281 @@
// npx vitest run __tests__/removeClineFromStack-delegation.spec.ts

import { describe, it, expect, vi } from "vitest"
import { ClineProvider } from "../core/webview/ClineProvider"

describe("ClineProvider.removeClineFromStack() delegation awareness", () => {
/**
* Helper to build a minimal mock provider with a single task on the stack.
* The task's parentTaskId and taskId are configurable.
*/
function buildMockProvider(opts: {
childTaskId: string
parentTaskId?: string
parentHistoryItem?: Record<string, any>
getTaskWithIdError?: Error
}) {
const childTask = {
taskId: opts.childTaskId,
instanceId: "inst-1",
parentTaskId: opts.parentTaskId,
emit: vi.fn(),
abortTask: vi.fn().mockResolvedValue(undefined),
}

const updateTaskHistory = vi.fn().mockResolvedValue([])
const getTaskWithId = opts.getTaskWithIdError
? vi.fn().mockRejectedValue(opts.getTaskWithIdError)
: vi.fn().mockImplementation(async (id: string) => {
if (id === opts.parentTaskId && opts.parentHistoryItem) {
return { historyItem: { ...opts.parentHistoryItem } }
}
throw new Error("Task not found")
})

const provider = {
clineStack: [childTask] as any[],
taskEventListeners: new Map(),
log: vi.fn(),
getTaskWithId,
updateTaskHistory,
}

return { provider, childTask, updateTaskHistory, getTaskWithId }
}

it("repairs parent metadata (delegated → active) when a delegated child is removed", async () => {
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
childTaskId: "child-1",
parentTaskId: "parent-1",
parentHistoryItem: {
id: "parent-1",
task: "Parent task",
ts: 1000,
number: 1,
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
status: "delegated",
awaitingChildId: "child-1",
delegatedToId: "child-1",
childIds: ["child-1"],
},
})

await (ClineProvider.prototype as any).removeClineFromStack.call(provider)

// Stack should be empty after pop
expect(provider.clineStack).toHaveLength(0)

// Parent lookup should have been called
expect(getTaskWithId).toHaveBeenCalledWith("parent-1")

// Parent metadata should be repaired
expect(updateTaskHistory).toHaveBeenCalledTimes(1)
const updatedParent = updateTaskHistory.mock.calls[0][0]
expect(updatedParent).toEqual(
expect.objectContaining({
id: "parent-1",
status: "active",
awaitingChildId: undefined,
}),
)

// Log the repair
expect(provider.log).toHaveBeenCalledWith(expect.stringContaining("Repaired parent parent-1 metadata"))
})

it("does NOT modify parent metadata when the task has no parentTaskId (non-delegated)", async () => {
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
childTaskId: "standalone-1",
// No parentTaskId — this is a top-level task
})

await (ClineProvider.prototype as any).removeClineFromStack.call(provider)

// Stack should be empty
expect(provider.clineStack).toHaveLength(0)

// No parent lookup or update should happen
expect(getTaskWithId).not.toHaveBeenCalled()
expect(updateTaskHistory).not.toHaveBeenCalled()
})

it("does NOT modify parent metadata when awaitingChildId does not match the popped child", async () => {
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
childTaskId: "child-1",
parentTaskId: "parent-1",
parentHistoryItem: {
id: "parent-1",
task: "Parent task",
ts: 1000,
number: 1,
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
status: "delegated",
awaitingChildId: "child-OTHER", // different child
delegatedToId: "child-OTHER",
childIds: ["child-OTHER"],
},
})

await (ClineProvider.prototype as any).removeClineFromStack.call(provider)

// Parent was looked up but should NOT be updated
expect(getTaskWithId).toHaveBeenCalledWith("parent-1")
expect(updateTaskHistory).not.toHaveBeenCalled()
})

it("does NOT modify parent metadata when parent status is not 'delegated'", async () => {
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
childTaskId: "child-1",
parentTaskId: "parent-1",
parentHistoryItem: {
id: "parent-1",
task: "Parent task",
ts: 1000,
number: 1,
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
status: "completed", // already completed
awaitingChildId: "child-1",
childIds: ["child-1"],
},
})

await (ClineProvider.prototype as any).removeClineFromStack.call(provider)

expect(getTaskWithId).toHaveBeenCalledWith("parent-1")
expect(updateTaskHistory).not.toHaveBeenCalled()
})

it("catches and logs errors during parent metadata repair without blocking the pop", async () => {
const { provider, childTask, updateTaskHistory, getTaskWithId } = buildMockProvider({
childTaskId: "child-1",
parentTaskId: "parent-1",
getTaskWithIdError: new Error("Storage unavailable"),
})

// Should NOT throw
await (ClineProvider.prototype as any).removeClineFromStack.call(provider)

// Stack should still be empty (pop was not blocked)
expect(provider.clineStack).toHaveLength(0)

// The abort should still have been called
expect(childTask.abortTask).toHaveBeenCalledWith(true)

// Error should be logged as non-fatal
expect(provider.log).toHaveBeenCalledWith(
expect.stringContaining("Failed to repair parent metadata for parent-1 (non-fatal)"),
)

// No update should have been attempted
expect(updateTaskHistory).not.toHaveBeenCalled()
})

it("handles empty stack gracefully", async () => {
const provider = {
clineStack: [] as any[],
taskEventListeners: new Map(),
log: vi.fn(),
getTaskWithId: vi.fn(),
updateTaskHistory: vi.fn(),
}

// Should not throw
await (ClineProvider.prototype as any).removeClineFromStack.call(provider)

expect(provider.clineStack).toHaveLength(0)
expect(provider.getTaskWithId).not.toHaveBeenCalled()
expect(provider.updateTaskHistory).not.toHaveBeenCalled()
})

it("skips delegation repair when skipDelegationRepair option is true", async () => {
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
childTaskId: "child-1",
parentTaskId: "parent-1",
parentHistoryItem: {
id: "parent-1",
task: "Parent task",
ts: 1000,
number: 1,
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
status: "delegated",
awaitingChildId: "child-1",
delegatedToId: "child-1",
childIds: ["child-1"],
},
})

// Call with skipDelegationRepair: true (as delegateParentAndOpenChild would)
await (ClineProvider.prototype as any).removeClineFromStack.call(provider, { skipDelegationRepair: true })

// Stack should be empty after pop
expect(provider.clineStack).toHaveLength(0)

// Parent lookup should NOT have been called — repair was skipped entirely
expect(getTaskWithId).not.toHaveBeenCalled()
expect(updateTaskHistory).not.toHaveBeenCalled()
})

it("does NOT reset grandparent during A→B→C nested delegation transition", async () => {
// Scenario: A delegated to B, B is now delegating to C.
// delegateParentAndOpenChild() pops B via removeClineFromStack({ skipDelegationRepair: true }).
// Grandparent A should remain "delegated" — its metadata must not be repaired.
const grandparentHistory = {
id: "task-A",
task: "Grandparent task",
ts: 1000,
number: 1,
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
status: "delegated",
awaitingChildId: "task-B",
delegatedToId: "task-B",
childIds: ["task-B"],
}

const taskB = {
taskId: "task-B",
instanceId: "inst-B",
parentTaskId: "task-A",
emit: vi.fn(),
abortTask: vi.fn().mockResolvedValue(undefined),
}

const getTaskWithId = vi.fn().mockImplementation(async (id: string) => {
if (id === "task-A") {
return { historyItem: { ...grandparentHistory } }
}
throw new Error("Task not found")
})
const updateTaskHistory = vi.fn().mockResolvedValue([])

const provider = {
clineStack: [taskB] as any[],
taskEventListeners: new Map(),
log: vi.fn(),
getTaskWithId,
updateTaskHistory,
}

// Simulate what delegateParentAndOpenChild does: pop B with skipDelegationRepair
await (ClineProvider.prototype as any).removeClineFromStack.call(provider, { skipDelegationRepair: true })

// B was popped
expect(provider.clineStack).toHaveLength(0)

// Grandparent A should NOT have been looked up or modified
expect(getTaskWithId).not.toHaveBeenCalled()
expect(updateTaskHistory).not.toHaveBeenCalled()

// Grandparent A's metadata remains intact (delegated, awaitingChildId: task-B)
// The caller (delegateParentAndOpenChild) will update A to point to C separately.
})
})
40 changes: 38 additions & 2 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ export class ClineProvider

// Removes and destroys the top Cline instance (the current finished task),
// activating the previous one (resuming the parent task).
async removeClineFromStack() {
async removeClineFromStack(options?: { skipDelegationRepair?: boolean }) {
if (this.clineStack.length === 0) {
return
}
Expand All @@ -462,6 +462,11 @@ export class ClineProvider
let task = this.clineStack.pop()

if (task) {
// Capture delegation metadata before abort/dispose, since abortTask(true)
// is async and the task reference is cleared afterwards.
const childTaskId = task.taskId
const parentTaskId = task.parentTaskId

task.emit(RooCodeEventName.TaskUnfocused)

try {
Expand All @@ -485,6 +490,37 @@ export class ClineProvider
// Make sure no reference kept, once promises end it will be
// garbage collected.
task = undefined

// Delegation-aware parent metadata repair:
// If the popped task was a delegated child, repair the parent's metadata
// so it transitions from "delegated" back to "active" and becomes resumable
// from the task history list.
// Skip when called from delegateParentAndOpenChild() during nested delegation
// transitions (A→B→C), where the caller intentionally replaces the active
// child and will update the parent to point at the new child.
if (parentTaskId && childTaskId && !options?.skipDelegationRepair) {
try {
const { historyItem: parentHistory } = await this.getTaskWithId(parentTaskId)

if (parentHistory.status === "delegated" && parentHistory.awaitingChildId === childTaskId) {
await this.updateTaskHistory({
...parentHistory,
status: "active",
awaitingChildId: undefined,
})
this.log(
`[ClineProvider#removeClineFromStack] Repaired parent ${parentTaskId} metadata: delegated → active (child ${childTaskId} removed)`,
)
}
} catch (err) {
// Non-fatal: log but do not block the pop operation.
this.log(
`[ClineProvider#removeClineFromStack] Failed to repair parent metadata for ${parentTaskId} (non-fatal): ${
err instanceof Error ? err.message : String(err)
}`,
)
}
}
}
}

Expand Down Expand Up @@ -3229,7 +3265,7 @@ export class ClineProvider
// This ensures we never have >1 tasks open at any time during delegation.
// Await abort completion to ensure clean disposal and prevent unhandled rejections.
try {
await this.removeClineFromStack()
await this.removeClineFromStack({ skipDelegationRepair: true })
} catch (error) {
this.log(
`[delegateParentAndOpenChild] Error during parent disposal (non-fatal): ${
Expand Down
Loading