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
6 changes: 4 additions & 2 deletions src/core/tools/NewTaskTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,10 @@ export class NewTaskTool extends BaseTool<"new_task"> {
return
}

// Delegate parent and open child as sole active task
const child = await (provider as any).delegateParentAndOpenChild({
// Delegate parent and open child as sole active task.
// `provider` is already narrowed to ClineProvider above (WeakRef deref + null check),
// so this is a typed method call — no cast needed.
const child = await provider.delegateParentAndOpenChild({
parentTaskId: task.taskId,
message: unescapedMessage,
initialTodos: todoItems,
Expand Down
72 changes: 72 additions & 0 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,68 @@ export class ClineProvider
}
}

/**
* Re-establish a parent→child delegation link when an interrupted child is resumed.
*
* The interrupt path (cancelTask / evictCurrentTask) preserves the link only while the
* parent is still "delegated". After a crash or resume cycle the parent can be left
* "active" with no awaitingChildId, so AttemptCompletionTool refuses to route the child's
* completion back (it requires parent.awaitingChildId === this child). Restoring the link
* here — the common funnel for every resume path — lets a resumed child report back.
*
* Safe by construction:
* - only called for children whose delegation was NOT intentionally severed
* (cancelledDelegationChildIds),
* - never clobbers a live delegation to a different child,
* - and only transitions an "active" parent to "delegated" (the sole legal path).
* Non-fatal: any failure is logged and the resume proceeds without the link.
*/
private async reestablishDelegationLinkOnResume(childTaskId: string, parentTaskId: string): Promise<void> {
// A child whose delegation was intentionally severed (abandonSubtask) or that failed a
// cancel must NOT be reattached — its parentTaskId may still point at the old parent.
if (this.cancelledDelegationChildIds.has(childTaskId)) {
return
}

try {
await this.taskHistoryStore.atomicReadAndUpdate(parentTaskId, (parent) => {
// Already linked to this child — nothing to do.
if (parent.status === "delegated" && parent.awaitingChildId === childTaskId) {
return parent
}

// Never clobber a live delegation to a different child.
if (parent.awaitingChildId && parent.awaitingChildId !== childTaskId) {
const otherStatus = this.taskHistoryStore.get(parent.awaitingChildId)?.status
if (otherStatus === "active" || otherStatus === "delegated") {
return parent
}
}

// Only an "active" parent can legally become "delegated"; any other status is left untouched.
if (parent.status !== "active") {
return parent
}

const childIds = Array.from(new Set([...(parent.childIds ?? []), childTaskId]))
this.log(
`[reestablishDelegationLinkOnResume] Restored link: parent ${parentTaskId} → child ${childTaskId}`,
)
return {
...parent,
status: "delegated" as const,
awaitingChildId: childTaskId,
delegatedToId: childTaskId,
childIds,
}
})
} catch (err) {
this.log(
`[reestablishDelegationLinkOnResume] Failed to restore link for parent ${parentTaskId} → child ${childTaskId}: ${err instanceof Error ? err.message : String(err)}`,
)
}
}

/**
* Cancel cascade: interrupt every LIVE child of the given parent task.
*
Expand Down Expand Up @@ -1186,6 +1248,16 @@ export class ClineProvider
await this.evictCurrentTask()
}

// Re-establish the parent→child delegation link when resuming an interrupted child.
// The interrupt path preserves the link only while the parent is still "delegated";
// after a crash or resume cycle the parent can be left "active" with no awaitingChildId,
// which would prevent the resumed child's completion from routing back (AttemptCompletionTool
// requires parent.awaitingChildId === this child). Restoring it here — before the child Task
// instance starts writing its own history — lets a resumed child report back to its parent.
if (historyItem.parentTaskId && historyItem.status === "interrupted") {
await this.reestablishDelegationLinkOnResume(historyItem.id, historyItem.parentTaskId)
}

// If the history item has a saved mode, restore it and its associated API configuration.
if (historyItem.mode) {
// Validate that the mode still exists
Expand Down
152 changes: 152 additions & 0 deletions src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,158 @@ describe("ClineProvider flicker-free cancel", () => {
expect(provider.getTaskWithId).not.toHaveBeenCalled()
})

describe("reestablishDelegationLinkOnResume (Problem A: link lost across interrupt/resume)", () => {
// The outer afterEach calls provider.dispose(), which needs the REAL store. We swap in a
// minimal fake per test and restore the original here so dispose() sees it.
let originalStore: unknown
beforeEach(() => {
originalStore = provider["taskHistoryStore"]
})
afterEach(() => {
setStore(originalStore)
})

// taskHistoryStore is `public readonly`, so a direct assignment fails type-checking.
// Object.assign sidesteps the compile-time readonly check (it's an own data property at
// runtime) so we can swap in a minimal fake; the real store is restored before dispose().
function setStore(store: unknown): void {
Object.assign(provider, { taskHistoryStore: store })
}

// Minimal Map-backed store mirroring the real TaskHistoryStore's atomicReadAndUpdate/get
// semantics closely enough to exercise the provider's restore logic in isolation.
function makeFakeStore(items: Record<string, HistoryItem>) {
const cache = new Map(Object.entries(items))
return {
get: (id: string) => cache.get(id),
// provider.dispose() calls this; no-op for the fake.
dispose: () => {},
atomicReadAndUpdate: async (taskId: string, updater: (current: HistoryItem) => HistoryItem) => {
const current = cache.get(taskId)
if (!current) throw new Error(`not found in cache: ${taskId}`)
const snapshot = structuredClone(current)
const updated = updater(snapshot)
if (updated.id !== taskId) throw new Error("updater changed task id")
cache.set(taskId, updated)
return [updated]
},
}
}

it("restores the parent link when an interrupted child is resumed and the parent was demoted to active", async () => {
// The exact Problem A state: parent left "active" with no awaitingChildId after a
// crash/resume cycle, while the child (interrupted) still points at it.
const store = makeFakeStore({
["parent-1"]: {
id: "parent-1",
number: 1,
task: "parent task",
ts: Date.now(),
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
workspace: "/test/workspace",
status: "active",
} as HistoryItem,
})
setStore(store)

await provider["reestablishDelegationLinkOnResume"]("child-1", "parent-1")

const parent = store.get("parent-1")!
expect(parent.status).toBe("delegated")
expect(parent.awaitingChildId).toBe("child-1")
expect(parent.delegatedToId).toBe("child-1")
expect(parent.childIds).toContain("child-1")
})

it("is a no-op when the parent is already delegated to this child", async () => {
const store = makeFakeStore({
["parent-1"]: {
id: "parent-1",
number: 1,
task: "parent task",
ts: Date.now(),
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
workspace: "/test/workspace",
status: "delegated",
awaitingChildId: "child-1",
delegatedToId: "child-1",
} as HistoryItem,
})
setStore(store)

await provider["reestablishDelegationLinkOnResume"]("child-1", "parent-1")

const parent = store.get("parent-1")!
expect(parent.status).toBe("delegated")
expect(parent.awaitingChildId).toBe("child-1")
})

it("never clobbers a live delegation to a different child", async () => {
const store = makeFakeStore({
["parent-1"]: {
id: "parent-1",
number: 1,
task: "parent task",
ts: Date.now(),
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
workspace: "/test/workspace",
status: "delegated",
awaitingChildId: "other-child",
delegatedToId: "other-child",
} as HistoryItem,
["other-child"]: {
id: "other-child",
number: 3,
task: "other child",
ts: Date.now(),
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
workspace: "/test/workspace",
status: "active",
} as HistoryItem,
})
setStore(store)

await provider["reestablishDelegationLinkOnResume"]("child-1", "parent-1")

const parent = store.get("parent-1")!
// Still awaiting the live other-child — child-1 must not have displaced it.
expect(parent.awaitingChildId).toBe("other-child")
})

it("does not reattach a child whose delegation was intentionally severed", async () => {
const store = makeFakeStore({
["parent-1"]: {
id: "parent-1",
number: 1,
task: "parent task",
ts: Date.now(),
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
workspace: "/test/workspace",
status: "active",
} as HistoryItem,
})
setStore(store)
// abandonSubtask / a failed cancel adds the child to this fail-closed set.
provider["cancelledDelegationChildIds"].add("child-1")

await provider["reestablishDelegationLinkOnResume"]("child-1", "parent-1")

const parent = store.get("parent-1")!
expect(parent.status).toBe("active")
expect(parent.awaitingChildId).toBeUndefined()
})
})

afterAll(() => {
vi.restoreAllMocks()
})
Expand Down
5 changes: 0 additions & 5 deletions src/eslint-suppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -904,11 +904,6 @@
"count": 1
}
},
"core/tools/NewTaskTool.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 1
}
},
"core/tools/ReadFileTool.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 4
Expand Down
Loading