Fix interrupted child redelegation

docs/architecture/task-lifecycle-model.md

@@ -46,18 +46,21 @@ TLA+/PlusCal or Quint with TLC becomes a better fit when the lifecycle needs tem
 | Task record and status    | `HistoryItem` persisted by `TaskHistoryStore`                                                                        |
 | `delegate(parent, child)` | `ClineProvider.delegateParentAndOpenChild`                                                                           |
 | `interrupt(child)`        | cancellation or eviction through `markDelegatedChildInterrupted`                                                     |
+| `resume(child)`           | accepted `resume_task` prompt through `TaskHistoryStore.resumeInterruptedTask`                                       |
 | `complete(child)`         | `ClineProvider.reopenParentFromDelegation`                                                                           |
 | `abandon(child)`          | `ClineProvider.abandonSubtask`                                                                                       |
 | Pending-action settlement | `TaskHistoryStore.clearPendingActionIfMatching` compare-and-clear in the rejected-delegation settlement path (#1714) |
 | Atomic event step         | `atomicReadAndUpdate`, `atomicUpdatePair`, and per-parent delegation transition lock                                 |
 | Event interleaving        | Competing completion, cancellation, abandonment, and new delegation calls                                            |
 
-The model has three fixed task slots, enough to cover competing siblings and a nested parent-child-grandchild chain. It explores every reachable interleaving through depth 12, deduplicating canonical states. Representative checks also exercise rejected operations that do not create a new state: a second concurrent delegation while the first child is active, stale completion after re-delegation, late completion after abandonment, completion after interruption, and nested completion. Named semantic landmarks require the graph to retain interrupted-child re-delegation and nested delegation even when the raw state total changes.
+The model has three fixed task slots, enough to cover competing siblings and a nested parent-child-grandchild chain. It explores every reachable interleaving through depth 13, deduplicating canonical states. Representative checks also exercise rejected operations that do not create a new state: a second concurrent delegation while the first child is active, stale completion after re-delegation, late completion after abandonment, completion after interruption, and nested completion. Named semantic landmarks require the graph to retain interrupted-child re-delegation, nested delegation, and an explicitly resumed interrupted child delegating to a grandchild even when the raw state total changes. The resume action calls the production reducer and carries a model-only provenance bit so the explorer can distinguish that path from a child that was never interrupted; generic persisted transitions still reject `interrupted → active`.
 
 Each task slot can also hold one of two pending `create_subtask` actions. A `stage` action mirrors `setPendingTaskAction` overwrite semantics, delegation clears the action its request carried, completion clears the child's action only when its event carries the matching action ID, and a `settle-rejected` action models the settlement that follows an authoritative delegation rejection (#1714). Production settles through the typed `LifecycleTransitionError` from the shared guards: the provider calls the disk-authoritative `TaskHistoryStore.clearPendingActionIfMatching` compare-and-clear under the per-file lock, then propagates the original rejection. Six named witnesses must remain reachable: settlement from an interrupted record after rejection, settlement through a successful active delegation, unrelated-action preservation during completion, stale-action protection where a settlement targeting one action ID leaves a replacement action intact, matching-ID completion clearing, and replacement-ID completion preservation. A mismatched pending-action request keeps its production behavior: the atomic update throws before any transition, and no settlement runs.
 
 Production completion also accepts a recovery-compatible `active` parent that still awaits the returning child, then clears the stale pointers. Normal model transitions never create that intermediate state, so it is covered by a focused reducer test rather than admitted as a generally valid reachable state.
 
+Explicit resume compares the caller's expected parent with the authoritative child backlink under the child-file lock. Abandonment writes child detachment before parent release, so a stale approval is rejected even in that partial-write window. Representative reducer scenarios cover stale linked and standalone approvals; a two-store provider test covers abandonment after the parent precheck. Rehydrated interrupted tasks omit lineage from message metadata saves, preventing their stale construction snapshots from restoring severed links after rejection. This closes abandonment-before-resume, not every cross-host ordering: parent-only redelegation, reverse-order stale abandonment, and other live-task writers remain within the ownership/generation gaps below.
+
 ## Shared-store concurrency model
 
 The same `pnpm lifecycle:model-check` command also runs a second bounded explorer over two `TaskHistoryStore` hosts. It imports the production `computeHistoryDelta` and `mergeHistoryDelta` functions, so its semantics match the store rather than assuming coherent caches or transactional pair writes:

scripts/check-task-lifecycle.ts

@@ -7,12 +7,16 @@ import {
 	completeDelegatedChild,
 	delegateTaskToChild,
 	interruptDelegatedChild,
+	resumeInterruptedTask,
 	settleRejectedCreateSubtaskAction,
 } from "../src/core/task-persistence/taskLifecycle"
 
 const taskIds = ["parent", "child-a", "child-b"] as const
 type TaskId = (typeof taskIds)[number]
-type ModelState = Record<TaskId, HistoryItem | undefined>
+interface ModelTask extends HistoryItem {
+	modelWasResumed?: boolean
+}
+type ModelState = Record<TaskId, ModelTask | undefined>
 
 interface Transition {
 	name: string
@@ -33,10 +37,10 @@ interface WitnessContext {
 	transition: Transition
 }
 
-const MAX_DEPTH = 12
+const MAX_DEPTH = 13
 const MAX_STATES = 10_000
 const actionIds = ["action-1", "action-2"] as const
-const expectedActions = ["delegate", "interrupt", "complete", "abandon", "stage", "settle-rejected"] as const
+const expectedActions = ["delegate", "interrupt", "resume", "complete", "abandon", "stage", "settle-rejected"] as const
 const semanticLandmarks = {
 	"interrupted-child-redelegation": (state: ModelState) =>
 		state.parent?.status === "delegated" &&
@@ -47,6 +51,12 @@ const semanticLandmarks = {
 		state.parent.awaitingChildId === "child-a" &&
 		state["child-a"]?.status === "delegated" &&
 		state["child-a"].awaitingChildId === "child-b",
+	"resumed-interrupted-child-nested-delegation": (state: ModelState) =>
+		state.parent?.status === "delegated" &&
+		state.parent.awaitingChildId === "child-a" &&
+		state["child-a"]?.status === "delegated" &&
+		state["child-a"].awaitingChildId === "child-b" &&
+		state["child-a"].modelWasResumed === true,
 } satisfies Record<string, (state: ModelState) => boolean>
 const semanticWitnesses = {
 	"interrupted-pending-delegation-settled": ({ prev, next, transition }: WitnessContext) =>
@@ -101,7 +111,7 @@ const semanticWitnesses = {
 	},
 } satisfies Record<string, (context: WitnessContext) => boolean>
 
-function task(id: TaskId, parentTaskId?: TaskId): HistoryItem {
+function task(id: TaskId, parentTaskId?: TaskId): ModelTask {
 	return {
 		id,
 		number: taskIds.indexOf(id),
@@ -132,7 +142,7 @@ function initialState(): ModelState {
 	return { parent: task("parent"), "child-a": undefined, "child-b": undefined }
 }
 
-function replace(state: ModelState, ...updates: HistoryItem[]): ModelState {
+function replace(state: ModelState, ...updates: ModelTask[]): ModelState {
 	const next = { ...state }
 	for (const update of updates) next[update.id as TaskId] = update
 	return next
@@ -176,6 +186,18 @@ function transitions(state: ModelState): Transition[] {
 				})
 			}
 		}
+
+		const lineageParent = parent.parentTaskId ? state[parent.parentTaskId as TaskId] : undefined
+		const resumeValid =
+			parent.status === "interrupted" &&
+			(!parent.parentTaskId ||
+				(lineageParent?.status === "delegated" && lineageParent.awaitingChildId === parent.id))
+		if (resumeValid) {
+			result.push({
+				name: `resume(${parentId})`,
+				next: replace(state, { ...resumeInterruptedTask(parent, parent.parentTaskId), modelWasResumed: true }),
+			})
+		}
 	}
 
 	for (const childId of taskIds) {
@@ -416,11 +438,19 @@ function runRepresentativeScenarios(): void {
 	assert.throws(() => delegateTaskToChild(delegated, "child-b", "active"), /not interrupted/)
 
 	const interruptedA = interruptDelegatedChild(delegated, childA)
+	const resumedA = resumeInterruptedTask(interruptedA, parent.id)
+	const resumedNested = delegateTaskToChild(resumedA, "child-b")
+	assert.equal(resumedNested.status, "delegated")
+	assert.equal(resumedNested.awaitingChildId, "child-b")
 	const redelegated = delegateTaskToChild(delegated, "child-b", interruptedA.status)
 	assert.throws(() => completeDelegatedChild(redelegated, interruptedA, "stale"), /not delegated to child/)
 
 	const abandoned = abandonDelegatedChild(delegated, interruptedA)
 	assert.throws(() => completeDelegatedChild(abandoned.parent, abandoned.child, "late"), /not delegated to child/)
+	// A resume approved against the old linkage cannot revive an abandoned child.
+	assert.throws(() => resumeInterruptedTask(abandoned.child, parent.id), /parent linkage changed/)
+	assert.throws(() => resumeInterruptedTask(interruptedA), /parent linkage changed/)
+	assert.equal(resumeInterruptedTask(abandoned.child).status, "active")
 
 	const childB = task("child-b", "child-a")
 	const nestedParent = delegateTaskToChild(childA, childB.id)

src/__tests__/ClineProvider.delegation.spec.ts

@@ -9,7 +9,9 @@ import type { HistoryItem } from "@roo-code/types"
 import { providerIdentifiers, RooCodeEventName } from "@roo-code/types"
 import { ClineProvider } from "../core/webview/ClineProvider"
 import { TaskScheduler } from "../core/task/TaskScheduler"
-import { LifecycleTransitionError } from "../core/task-persistence"
+import { LifecycleTransitionError, TaskHistoryStore } from "../core/task-persistence"
+import { abandonDelegatedChild } from "../core/task-persistence/taskLifecycle"
+import { makeProviderStub } from "./helpers/provider-stub"
 
 const parentHistoryItem: HistoryItem = {
 	id: "parent-1",
@@ -51,6 +53,92 @@ const makeParentTask = () =>
 		retrySaveApiConversationHistory: vi.fn(),
 	}) as any
 
+describe("ClineProvider.resumeInterruptedTask()", () => {
+	it("resumes a delegated child only while its parent still awaits it", async () => {
+		const resumed = { id: "child-1", status: "active" } as HistoryItem
+		const taskHistoryStore = {
+			invalidate: vi.fn().mockResolvedValue(undefined),
+			get: vi.fn().mockReturnValue({ id: "parent-1", status: "delegated", awaitingChildId: "child-1" }),
+			resumeInterruptedTask: vi.fn().mockResolvedValue(resumed),
+		}
+		const provider = makeProviderStub({ taskHistoryStore, isViewLaunched: false })
+
+		await ClineProvider.prototype.resumeInterruptedTask.call(provider, "child-1", "parent-1")
+
+		expect(taskHistoryStore.invalidate).toHaveBeenCalledWith("parent-1")
+		expect(taskHistoryStore.resumeInterruptedTask).toHaveBeenCalledWith("child-1", "parent-1")
+	})
+
+	it("rejects abandonment committed by another host after the parent precheck", async () => {
+		const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "resume-abandon-"))
+		const storeA = new TaskHistoryStore(storagePath)
+		const storeB = new TaskHistoryStore(storagePath)
+		const parent: HistoryItem = {
+			id: "parent",
+			number: 1,
+			ts: 1,
+			task: "Parent",
+			tokensIn: 0,
+			tokensOut: 0,
+			totalCost: 0,
+			status: "delegated",
+			awaitingChildId: "child",
+			delegatedToId: "child",
+			childIds: ["child"],
+		}
+		const child: HistoryItem = {
+			...parent,
+			id: "child",
+			status: "interrupted",
+			parentTaskId: "parent",
+			rootTaskId: "parent",
+			awaitingChildId: undefined,
+			delegatedToId: undefined,
+			childIds: [],
+		}
+		try {
+			await storeA.initialize()
+			await storeA.upsert(parent)
+			await storeA.upsert(child)
+			await storeB.initialize()
+			const resume = storeA.resumeInterruptedTask.bind(storeA)
+			vi.spyOn(storeA, "resumeInterruptedTask").mockImplementationOnce(async (id, expectedParent) => {
+				// This hook runs after the provider's parent check. Commit only the
+				// first half of abandonment: the parent still appears to await this child.
+				await storeB.atomicReadAndUpdate("child", (current) => abandonDelegatedChild(parent, current).child)
+				return resume(id, expectedParent)
+			})
+			const provider = makeProviderStub({ taskHistoryStore: storeA, isViewLaunched: false })
+			await expect(
+				ClineProvider.prototype.resumeInterruptedTask.call(provider, "child", "parent"),
+			).rejects.toThrow("parent linkage changed")
+			await storeB.invalidate("child")
+			expect(storeA.get("child")).toEqual(storeB.get("child"))
+			expect(storeB.get("child")).toMatchObject({ status: "interrupted" })
+			expect(storeB.get("child")?.parentTaskId).toBeUndefined()
+			expect(storeB.get("child")?.rootTaskId).toBeUndefined()
+		} finally {
+			storeA.dispose()
+			storeB.dispose()
+			await fs.rm(storagePath, { recursive: true, force: true })
+		}
+	})
+
+	it("rejects a stale interrupted child after its parent delegates elsewhere", async () => {
+		const taskHistoryStore = {
+			invalidate: vi.fn().mockResolvedValue(undefined),
+			get: vi.fn().mockReturnValue({ id: "parent-1", status: "delegated", awaitingChildId: "child-2" }),
+			resumeInterruptedTask: vi.fn(),
+		}
+		const provider = makeProviderStub({ taskHistoryStore, isViewLaunched: false })
+
+		await expect(
+			ClineProvider.prototype.resumeInterruptedTask.call(provider, "child-1", "parent-1"),
+		).rejects.toThrow("parent parent-1 no longer awaits it")
+		expect(taskHistoryStore.resumeInterruptedTask).not.toHaveBeenCalled()
+	})
+})
+
 describe("ClineProvider.delegateParentAndOpenChild()", () => {
 	it("rejects a stale restored action before delegation side effects", async () => {
 		const parentTask = makeParentTask()

src/__tests__/removeClineFromStack-delegation.spec.ts

@@ -4,6 +4,7 @@ import { describe, it, expect, vi, type MockedFunction } from "vitest"
 import { ClineProvider } from "../core/webview/ClineProvider"
 import { TaskRegistry } from "../core/task/TaskRegistry"
 import { PendingActionSettlementError, type Task } from "../core/task/Task"
+import { LifecycleTransitionError } from "../core/task-persistence/taskLifecycle"
 import { makeProviderStub } from "./helpers/provider-stub"
 
 type MockTask = Pick<Task, "taskId" | "instanceId"> &
@@ -178,35 +179,34 @@ describe("ClineProvider.removeClineFromStack() — pure lifecycle, no delegation
 })
 
 describe("ClineProvider failed history restoration cleanup", () => {
-	it("removes the failed task, its listeners, and its resources without saving stale history", async () => {
-		const cleanupListener = vi.fn()
-		const task = {
-			taskId: "failed-history-task",
-			instanceId: "inst-1",
-			emit: vi.fn(),
-			dispose: vi.fn().mockResolvedValue(undefined),
-		} as unknown as Task
-		const taskRegistry = new TaskRegistry()
-		taskRegistry.push(task)
-		const taskEventListeners = new Map([[task, [cleanupListener]]])
-		const provider = {
-			taskRegistry,
-			taskEventListeners,
-			log: vi.fn(),
-		} as unknown as ClineProvider
-
-		await privateClineProvider.cleanupFailedHistoryTask.call(
-			provider,
-			task,
-			new PendingActionSettlementError("settlement failed"),
-		)
-
-		expect(taskRegistry.getById(task.taskId)).toBeUndefined()
-		expect(taskRegistry.current).toBeUndefined()
-		expect(cleanupListener).toHaveBeenCalledOnce()
-		expect(taskEventListeners.has(task)).toBe(false)
-		expect(task.dispose).toHaveBeenCalledOnce()
-	})
+	it.each([new PendingActionSettlementError("settlement failed"), new LifecycleTransitionError("resume rejected")])(
+		"removes the failed task without saving stale history after %s",
+		async (error) => {
+			const cleanupListener = vi.fn()
+			const task = {
+				taskId: "failed-history-task",
+				instanceId: "inst-1",
+				emit: vi.fn(),
+				dispose: vi.fn().mockResolvedValue(undefined),
+			} as unknown as Task
+			const taskRegistry = new TaskRegistry()
+			taskRegistry.push(task)
+			const taskEventListeners = new Map([[task, [cleanupListener]]])
+			const provider = {
+				taskRegistry,
+				taskEventListeners,
+				log: vi.fn(),
+			} as unknown as ClineProvider
+
+			await privateClineProvider.cleanupFailedHistoryTask.call(provider, task, error)
+
+			expect(taskRegistry.getById(task.taskId)).toBeUndefined()
+			expect(taskRegistry.current).toBeUndefined()
+			expect(cleanupListener).toHaveBeenCalledOnce()
+			expect(taskEventListeners.has(task)).toBe(false)
+			expect(task.dispose).toHaveBeenCalledOnce()
+		},
+	)
 
 	it("keeps the task active after an unrelated history resume failure", async () => {
 		const cleanupListener = vi.fn()

src/core/task-persistence/TaskHistoryStore.ts

@@ -10,7 +10,12 @@
 import { LOCK_STALE_MS, withFileLock } from "../../utils/fileLock"
 import { safeWriteJson } from "../../utils/safeWriteJson"
 import { getStorageBasePath } from "../../utils/storage"
-import { assertValidTransition, settleRejectedCreateSubtaskAction, type HistoryItemStatus } from "./taskLifecycle"
+import {
+	assertValidTransition,
+	resumeInterruptedTask as resumeInterruptedTaskRecord,
+	settleRejectedCreateSubtaskAction,
+	type HistoryItemStatus,
+} from "./taskLifecycle"
 import { computeHistoryDelta, DeltaRejectedError, mergeHistoryDelta } from "./taskStoreConcurrency"
 
 export { assertValidTransition, type HistoryItemStatus } from "./taskLifecycle"
@@ -1073,6 +1078,47 @@
 		})
 	}
 
+	/**
+	 * Disk-authoritative transition used only after the user accepts a resume
+	 * prompt. Generic writes intentionally reject interrupted → active so stale
+	 * task snapshots cannot revive cancelled work.
+	 */
+	public async resumeInterruptedTask(taskId: string, expectedParentTaskId?: string): Promise<HistoryItem> {
+		return this.withLock(async () => {
+			const cached = this.cache.get(taskId)
+			if (!cached) {
+				throw new Error(`[TaskHistoryStore] resumeInterruptedTask: task ${taskId} not found in cache`)
+			}
+
+			const filePath = await this.getTaskFilePath(taskId)
+			let authoritative: HistoryItem = cached
+			await safeWriteJson(filePath, cached, {
+				merge: (existing) => {
+					const parsed = historyItemSchema.safeParse(existing)
+					if (!parsed.success || parsed.data.id !== taskId) {
+						this.cache.delete(taskId)
+						this.taskFileMtimes.delete(taskId)
+						throw new Error(
+							`[TaskHistoryStore] resumeInterruptedTask: task ${taskId} has no valid disk record`,
+						)
+					}
+
+					// Abandonment commits child detachment before releasing the parent.
+					// Compare the caller's linkage under this same child-file lock.
+					this.cache.set(taskId, existing as HistoryItem)
+					authoritative = resumeInterruptedTaskRecord(existing as HistoryItem, expectedParentTaskId)
+					return authoritative
+				},
+			})
+
+			this.cache.set(taskId, authoritative)
+			if (this.onWrite) {
+				await this.onWrite(this.getAll())
+			}
+			return authoritative
+		})
+	}
+
 	/**
 	 * Disk-authoritative compare-and-clear for a rejected `create_subtask`
 	 * pending action (#1714). The comparison runs inside the per-file