Skip to content

Commit 70775f0

Browse files
fix: make removeClineFromStack() delegation-aware to prevent orphaned parent tasks (#11302)
* fix: make removeClineFromStack() delegation-aware to prevent orphaned parent tasks When a delegated child task is removed via removeClineFromStack() (e.g., Clear Task, navigate to history, start new task), the parent task was left orphaned in "delegated" status with a stale awaitingChildId. This made the parent unresumable without manual history repair. This fix captures parentTaskId and childTaskId before abort/dispose, then repairs the parent metadata (status -> active, clear awaitingChildId) when the popped task is a delegated child and awaitingChildId matches. Parent lookup + updateTaskHistory are wrapped in try/catch so failures are non-fatal (logged but do not block the pop). Closes #11301 * fix: add skipDelegationRepair opt-out to removeClineFromStack() for nested delegation --------- Co-authored-by: Roo Code <roomote@roocode.com>
1 parent bb86eb8 commit 70775f0

2 files changed

Lines changed: 319 additions & 2 deletions

File tree

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
// npx vitest run __tests__/removeClineFromStack-delegation.spec.ts
2+
3+
import { describe, it, expect, vi } from "vitest"
4+
import { ClineProvider } from "../core/webview/ClineProvider"
5+
6+
describe("ClineProvider.removeClineFromStack() delegation awareness", () => {
7+
/**
8+
* Helper to build a minimal mock provider with a single task on the stack.
9+
* The task's parentTaskId and taskId are configurable.
10+
*/
11+
function buildMockProvider(opts: {
12+
childTaskId: string
13+
parentTaskId?: string
14+
parentHistoryItem?: Record<string, any>
15+
getTaskWithIdError?: Error
16+
}) {
17+
const childTask = {
18+
taskId: opts.childTaskId,
19+
instanceId: "inst-1",
20+
parentTaskId: opts.parentTaskId,
21+
emit: vi.fn(),
22+
abortTask: vi.fn().mockResolvedValue(undefined),
23+
}
24+
25+
const updateTaskHistory = vi.fn().mockResolvedValue([])
26+
const getTaskWithId = opts.getTaskWithIdError
27+
? vi.fn().mockRejectedValue(opts.getTaskWithIdError)
28+
: vi.fn().mockImplementation(async (id: string) => {
29+
if (id === opts.parentTaskId && opts.parentHistoryItem) {
30+
return { historyItem: { ...opts.parentHistoryItem } }
31+
}
32+
throw new Error("Task not found")
33+
})
34+
35+
const provider = {
36+
clineStack: [childTask] as any[],
37+
taskEventListeners: new Map(),
38+
log: vi.fn(),
39+
getTaskWithId,
40+
updateTaskHistory,
41+
}
42+
43+
return { provider, childTask, updateTaskHistory, getTaskWithId }
44+
}
45+
46+
it("repairs parent metadata (delegated → active) when a delegated child is removed", async () => {
47+
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
48+
childTaskId: "child-1",
49+
parentTaskId: "parent-1",
50+
parentHistoryItem: {
51+
id: "parent-1",
52+
task: "Parent task",
53+
ts: 1000,
54+
number: 1,
55+
tokensIn: 0,
56+
tokensOut: 0,
57+
totalCost: 0,
58+
status: "delegated",
59+
awaitingChildId: "child-1",
60+
delegatedToId: "child-1",
61+
childIds: ["child-1"],
62+
},
63+
})
64+
65+
await (ClineProvider.prototype as any).removeClineFromStack.call(provider)
66+
67+
// Stack should be empty after pop
68+
expect(provider.clineStack).toHaveLength(0)
69+
70+
// Parent lookup should have been called
71+
expect(getTaskWithId).toHaveBeenCalledWith("parent-1")
72+
73+
// Parent metadata should be repaired
74+
expect(updateTaskHistory).toHaveBeenCalledTimes(1)
75+
const updatedParent = updateTaskHistory.mock.calls[0][0]
76+
expect(updatedParent).toEqual(
77+
expect.objectContaining({
78+
id: "parent-1",
79+
status: "active",
80+
awaitingChildId: undefined,
81+
}),
82+
)
83+
84+
// Log the repair
85+
expect(provider.log).toHaveBeenCalledWith(expect.stringContaining("Repaired parent parent-1 metadata"))
86+
})
87+
88+
it("does NOT modify parent metadata when the task has no parentTaskId (non-delegated)", async () => {
89+
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
90+
childTaskId: "standalone-1",
91+
// No parentTaskId — this is a top-level task
92+
})
93+
94+
await (ClineProvider.prototype as any).removeClineFromStack.call(provider)
95+
96+
// Stack should be empty
97+
expect(provider.clineStack).toHaveLength(0)
98+
99+
// No parent lookup or update should happen
100+
expect(getTaskWithId).not.toHaveBeenCalled()
101+
expect(updateTaskHistory).not.toHaveBeenCalled()
102+
})
103+
104+
it("does NOT modify parent metadata when awaitingChildId does not match the popped child", async () => {
105+
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
106+
childTaskId: "child-1",
107+
parentTaskId: "parent-1",
108+
parentHistoryItem: {
109+
id: "parent-1",
110+
task: "Parent task",
111+
ts: 1000,
112+
number: 1,
113+
tokensIn: 0,
114+
tokensOut: 0,
115+
totalCost: 0,
116+
status: "delegated",
117+
awaitingChildId: "child-OTHER", // different child
118+
delegatedToId: "child-OTHER",
119+
childIds: ["child-OTHER"],
120+
},
121+
})
122+
123+
await (ClineProvider.prototype as any).removeClineFromStack.call(provider)
124+
125+
// Parent was looked up but should NOT be updated
126+
expect(getTaskWithId).toHaveBeenCalledWith("parent-1")
127+
expect(updateTaskHistory).not.toHaveBeenCalled()
128+
})
129+
130+
it("does NOT modify parent metadata when parent status is not 'delegated'", async () => {
131+
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
132+
childTaskId: "child-1",
133+
parentTaskId: "parent-1",
134+
parentHistoryItem: {
135+
id: "parent-1",
136+
task: "Parent task",
137+
ts: 1000,
138+
number: 1,
139+
tokensIn: 0,
140+
tokensOut: 0,
141+
totalCost: 0,
142+
status: "completed", // already completed
143+
awaitingChildId: "child-1",
144+
childIds: ["child-1"],
145+
},
146+
})
147+
148+
await (ClineProvider.prototype as any).removeClineFromStack.call(provider)
149+
150+
expect(getTaskWithId).toHaveBeenCalledWith("parent-1")
151+
expect(updateTaskHistory).not.toHaveBeenCalled()
152+
})
153+
154+
it("catches and logs errors during parent metadata repair without blocking the pop", async () => {
155+
const { provider, childTask, updateTaskHistory, getTaskWithId } = buildMockProvider({
156+
childTaskId: "child-1",
157+
parentTaskId: "parent-1",
158+
getTaskWithIdError: new Error("Storage unavailable"),
159+
})
160+
161+
// Should NOT throw
162+
await (ClineProvider.prototype as any).removeClineFromStack.call(provider)
163+
164+
// Stack should still be empty (pop was not blocked)
165+
expect(provider.clineStack).toHaveLength(0)
166+
167+
// The abort should still have been called
168+
expect(childTask.abortTask).toHaveBeenCalledWith(true)
169+
170+
// Error should be logged as non-fatal
171+
expect(provider.log).toHaveBeenCalledWith(
172+
expect.stringContaining("Failed to repair parent metadata for parent-1 (non-fatal)"),
173+
)
174+
175+
// No update should have been attempted
176+
expect(updateTaskHistory).not.toHaveBeenCalled()
177+
})
178+
179+
it("handles empty stack gracefully", async () => {
180+
const provider = {
181+
clineStack: [] as any[],
182+
taskEventListeners: new Map(),
183+
log: vi.fn(),
184+
getTaskWithId: vi.fn(),
185+
updateTaskHistory: vi.fn(),
186+
}
187+
188+
// Should not throw
189+
await (ClineProvider.prototype as any).removeClineFromStack.call(provider)
190+
191+
expect(provider.clineStack).toHaveLength(0)
192+
expect(provider.getTaskWithId).not.toHaveBeenCalled()
193+
expect(provider.updateTaskHistory).not.toHaveBeenCalled()
194+
})
195+
196+
it("skips delegation repair when skipDelegationRepair option is true", async () => {
197+
const { provider, updateTaskHistory, getTaskWithId } = buildMockProvider({
198+
childTaskId: "child-1",
199+
parentTaskId: "parent-1",
200+
parentHistoryItem: {
201+
id: "parent-1",
202+
task: "Parent task",
203+
ts: 1000,
204+
number: 1,
205+
tokensIn: 0,
206+
tokensOut: 0,
207+
totalCost: 0,
208+
status: "delegated",
209+
awaitingChildId: "child-1",
210+
delegatedToId: "child-1",
211+
childIds: ["child-1"],
212+
},
213+
})
214+
215+
// Call with skipDelegationRepair: true (as delegateParentAndOpenChild would)
216+
await (ClineProvider.prototype as any).removeClineFromStack.call(provider, { skipDelegationRepair: true })
217+
218+
// Stack should be empty after pop
219+
expect(provider.clineStack).toHaveLength(0)
220+
221+
// Parent lookup should NOT have been called — repair was skipped entirely
222+
expect(getTaskWithId).not.toHaveBeenCalled()
223+
expect(updateTaskHistory).not.toHaveBeenCalled()
224+
})
225+
226+
it("does NOT reset grandparent during A→B→C nested delegation transition", async () => {
227+
// Scenario: A delegated to B, B is now delegating to C.
228+
// delegateParentAndOpenChild() pops B via removeClineFromStack({ skipDelegationRepair: true }).
229+
// Grandparent A should remain "delegated" — its metadata must not be repaired.
230+
const grandparentHistory = {
231+
id: "task-A",
232+
task: "Grandparent task",
233+
ts: 1000,
234+
number: 1,
235+
tokensIn: 0,
236+
tokensOut: 0,
237+
totalCost: 0,
238+
status: "delegated",
239+
awaitingChildId: "task-B",
240+
delegatedToId: "task-B",
241+
childIds: ["task-B"],
242+
}
243+
244+
const taskB = {
245+
taskId: "task-B",
246+
instanceId: "inst-B",
247+
parentTaskId: "task-A",
248+
emit: vi.fn(),
249+
abortTask: vi.fn().mockResolvedValue(undefined),
250+
}
251+
252+
const getTaskWithId = vi.fn().mockImplementation(async (id: string) => {
253+
if (id === "task-A") {
254+
return { historyItem: { ...grandparentHistory } }
255+
}
256+
throw new Error("Task not found")
257+
})
258+
const updateTaskHistory = vi.fn().mockResolvedValue([])
259+
260+
const provider = {
261+
clineStack: [taskB] as any[],
262+
taskEventListeners: new Map(),
263+
log: vi.fn(),
264+
getTaskWithId,
265+
updateTaskHistory,
266+
}
267+
268+
// Simulate what delegateParentAndOpenChild does: pop B with skipDelegationRepair
269+
await (ClineProvider.prototype as any).removeClineFromStack.call(provider, { skipDelegationRepair: true })
270+
271+
// B was popped
272+
expect(provider.clineStack).toHaveLength(0)
273+
274+
// Grandparent A should NOT have been looked up or modified
275+
expect(getTaskWithId).not.toHaveBeenCalled()
276+
expect(updateTaskHistory).not.toHaveBeenCalled()
277+
278+
// Grandparent A's metadata remains intact (delegated, awaitingChildId: task-B)
279+
// The caller (delegateParentAndOpenChild) will update A to point to C separately.
280+
})
281+
})

src/core/webview/ClineProvider.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ export class ClineProvider
456456

457457
// Removes and destroys the top Cline instance (the current finished task),
458458
// activating the previous one (resuming the parent task).
459-
async removeClineFromStack() {
459+
async removeClineFromStack(options?: { skipDelegationRepair?: boolean }) {
460460
if (this.clineStack.length === 0) {
461461
return
462462
}
@@ -465,6 +465,11 @@ export class ClineProvider
465465
let task = this.clineStack.pop()
466466

467467
if (task) {
468+
// Capture delegation metadata before abort/dispose, since abortTask(true)
469+
// is async and the task reference is cleared afterwards.
470+
const childTaskId = task.taskId
471+
const parentTaskId = task.parentTaskId
472+
468473
task.emit(RooCodeEventName.TaskUnfocused)
469474

470475
try {
@@ -488,6 +493,37 @@ export class ClineProvider
488493
// Make sure no reference kept, once promises end it will be
489494
// garbage collected.
490495
task = undefined
496+
497+
// Delegation-aware parent metadata repair:
498+
// If the popped task was a delegated child, repair the parent's metadata
499+
// so it transitions from "delegated" back to "active" and becomes resumable
500+
// from the task history list.
501+
// Skip when called from delegateParentAndOpenChild() during nested delegation
502+
// transitions (A→B→C), where the caller intentionally replaces the active
503+
// child and will update the parent to point at the new child.
504+
if (parentTaskId && childTaskId && !options?.skipDelegationRepair) {
505+
try {
506+
const { historyItem: parentHistory } = await this.getTaskWithId(parentTaskId)
507+
508+
if (parentHistory.status === "delegated" && parentHistory.awaitingChildId === childTaskId) {
509+
await this.updateTaskHistory({
510+
...parentHistory,
511+
status: "active",
512+
awaitingChildId: undefined,
513+
})
514+
this.log(
515+
`[ClineProvider#removeClineFromStack] Repaired parent ${parentTaskId} metadata: delegated → active (child ${childTaskId} removed)`,
516+
)
517+
}
518+
} catch (err) {
519+
// Non-fatal: log but do not block the pop operation.
520+
this.log(
521+
`[ClineProvider#removeClineFromStack] Failed to repair parent metadata for ${parentTaskId} (non-fatal): ${
522+
err instanceof Error ? err.message : String(err)
523+
}`,
524+
)
525+
}
526+
}
491527
}
492528
}
493529

@@ -3268,7 +3304,7 @@ export class ClineProvider
32683304
// This ensures we never have >1 tasks open at any time during delegation.
32693305
// Await abort completion to ensure clean disposal and prevent unhandled rejections.
32703306
try {
3271-
await this.removeClineFromStack()
3307+
await this.removeClineFromStack({ skipDelegationRepair: true })
32723308
} catch (error) {
32733309
this.log(
32743310
`[delegateParentAndOpenChild] Error during parent disposal (non-fatal): ${

0 commit comments

Comments
 (0)