feat(task-lifecycle): task status transition guard and startup delegation reconciliation

src/__tests__/helpers/provider-stub.ts

@@ -12,6 +12,7 @@ export function makeProviderStub<T extends object>(stub: T): T {
 	const proto = ClineProvider.prototype as any
 	s.delegationTransitionLocks ??= new Map()
 	s.cancelledDelegationChildIds ??= new Set()
+	s.log ??= vi.fn()
 	s.runDelegationTransition = proto.runDelegationTransition.bind(s)
 	return s
 }

src/__tests__/history-resume-delegation.spec.ts

@@ -30,11 +30,15 @@ vi.mock("vscode", () => {
 vi.mock("../core/task-persistence/taskMessages", () => ({
 	readTaskMessages: vi.fn().mockResolvedValue([]),
 }))
-vi.mock("../core/task-persistence", () => ({
-	readApiMessages: vi.fn().mockResolvedValue([]),
-	saveApiMessages: vi.fn().mockResolvedValue(undefined),
-	saveTaskMessages: vi.fn().mockResolvedValue(undefined),
-}))
+vi.mock("../core/task-persistence", async (importOriginal) => {
+	const real = await importOriginal<typeof import("../core/task-persistence")>()
+	return {
+		...real,
+		readApiMessages: vi.fn().mockResolvedValue([]),
+		saveApiMessages: vi.fn().mockResolvedValue(undefined),
+		saveTaskMessages: vi.fn().mockResolvedValue(undefined),
+	}
+})
 
 import { ClineProvider } from "../core/webview/ClineProvider"
 import { readTaskMessages } from "../core/task-persistence/taskMessages"
@@ -130,9 +134,11 @@ describe("History resume delegation - parent metadata transitions", () => {
 		expect(firstId).toBe("child-1")
 		expect(secondId).toBe("parent-1")
 
-		// Verify child updater produces completed status
+		// Verify child updater produces completed status and persists completionResultSummary
+		// so startup reconciliation has the real result if the parent write fails.
 		const updatedChild = firstUpdater({ id: "child-1", status: "active" } as HistoryItem)
 		expect(updatedChild.status).toBe("completed")
+		expect(updatedChild.completionResultSummary).toBe("Child done")
 
 		// Verify parent updater produces active status with correct fields
 		const updatedParent = secondUpdater(parentHistoryItem as HistoryItem)
@@ -142,6 +148,7 @@ describe("History resume delegation - parent metadata transitions", () => {
 			completedByChildId: "child-1",
 			completionResultSummary: "Child done",
 			awaitingChildId: undefined,
+			delegatedToId: undefined,
 			childIds: ["child-1"],
 		})
 

src/__tests__/nested-delegation-resume.spec.ts

@@ -44,7 +44,8 @@ vi.mock("vscode", () => {
 vi.mock("../core/task-persistence/taskMessages", () => ({
 	readTaskMessages: vi.fn().mockResolvedValue([]),
 }))
-vi.mock("../core/task-persistence", () => ({
+vi.mock("../core/task-persistence", async (importOriginal) => ({
+	...(await importOriginal<typeof import("../core/task-persistence")>()),
 	readApiMessages: vi.fn().mockResolvedValue([]),
 	saveApiMessages: vi.fn().mockResolvedValue(undefined),
 	saveTaskMessages: vi.fn().mockResolvedValue(undefined),

src/__tests__/provider-delegation.spec.ts

@@ -226,11 +226,20 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
 			atomicReadAndUpdate: vi.fn().mockRejectedValue(persistError),
 		})
 
+		const child = { taskId: "child-1", start: childStart }
+		// Before createTask: getCurrentTask returns parent (used by step 3 close).
+		// After createTask: returns child so the rollback guard passes and the child is popped.
+		const getCurrentTask = vi.fn().mockReturnValue(parentTask)
+		const createTask = vi.fn().mockImplementation(async () => {
+			getCurrentTask.mockReturnValue(child)
+			return child
+		})
+
 		const provider = {
 			emit: vi.fn(),
-			getCurrentTask: vi.fn(() => parentTask),
+			getCurrentTask,
 			removeClineFromStack,
-			createTask: vi.fn().mockResolvedValue({ taskId: "child-1", start: childStart }),
+			createTask,
 			getTaskWithId,
 			handleModeSwitch: vi.fn().mockResolvedValue(undefined),
 			deleteTaskWithId,

src/__tests__/removeClineFromStack-delegation.spec.ts

@@ -79,6 +79,7 @@ describe("ClineProvider.removeClineFromStack() delegation awareness", () => {
 				id: "parent-1",
 				status: "active",
 				awaitingChildId: undefined,
+				delegatedToId: undefined,
 			}),
 		)
 

src/core/task-persistence/TaskHistoryStore.ts

@@ -8,6 +8,28 @@ import { GlobalFileNames } from "../../shared/globalFileNames"
 import { safeWriteJson } from "../../utils/safeWriteJson"
 import { getStorageBasePath } from "../../utils/storage"
 
+/** Valid status values for a task's HistoryItem. */
+export type HistoryItemStatus = NonNullable<HistoryItem["status"]>
+
+const VALID_TRANSITIONS: Record<HistoryItemStatus, HistoryItemStatus[]> = {
+	active: ["delegated", "completed"],
+	delegated: ["active"],
+	completed: [],
+}
+
+/**
+ * Asserts that a task status transition is valid, throwing if not.
+ *
+ * @throws {Error} When the transition is not allowed by the state machine.
+ */
+export function assertValidTransition(from: HistoryItemStatus | undefined, to: HistoryItemStatus): void {
+	const fromStatus: HistoryItemStatus = from ?? "active"
+	const validTargets = VALID_TRANSITIONS[fromStatus]
+	if (!validTargets.includes(to)) {
+		throw new Error(`Invalid task status transition: ${fromStatus} → ${to}`)
+	}
+}
+
 /**
  * Index file format for fast startup reads.
  */
@@ -88,10 +110,13 @@ export class TaskHistoryStore {
 			// 2. Reconcile cache against actual task directories on disk
 			await this.reconcile()
 
-			// 3. Start fs.watch for cross-instance reactivity
+			// 3. Repair delegation inconsistencies left by a previous crash
+			await this.reconcileDelegationState()
+
+			// 4. Start fs.watch for cross-instance reactivity
 			this.startWatcher()
 
-			// 4. Start periodic reconciliation as a defensive fallback
+			// 5. Start periodic reconciliation as a defensive fallback
 			this.startPeriodicReconciliation()
 		} finally {
 			// Mark initialization as complete so callers awaiting `initialized` can proceed
@@ -158,16 +183,42 @@ export class TaskHistoryStore {
 	 * updates the in-memory Map, and schedules a debounced index write.
 	 */
 	async upsert(item: HistoryItem): Promise<HistoryItem[]> {
-		return this.withLock(() => this._upsertUnlocked(item))
+		return this.withLock(() => this.upsertCore(item))
 	}
 
 	/**
-	 * Upsert body executed without acquiring the lock.
-	 * Must only be called from within a `withLock` callback.
+	 * Core upsert logic — must only be called from within `withLock`.
+	 *
+	 * Enforces state-machine transition rules when `item.status` changes.
+	 * Pass `skipTransitionCheck: true` only for administrative repairs (reconciliation,
+	 * migration) that need to write corrected state outside the normal task lifecycle.
 	 */
-	private async _upsertUnlocked(item: HistoryItem): Promise<HistoryItem[]> {
-		await this.upsertCore(item)
+	private async upsertCore(
+		item: HistoryItem,
+		options: { skipTransitionCheck?: boolean } = {},
+	): Promise<HistoryItem[]> {
+		const existing = this.cache.get(item.id)
 
+		// Enforce transition validity at the write boundary so that any caller
+		// (including fire-and-forget saves) cannot silently stomp a terminal status.
+		// Skip when there is no existing record — first insert has no prior state to transition from.
+		// Normalize existing.status (undefined = legacy "active") before comparing so that writing
+		// status: "active" onto a legacy item without a status field is not treated as a transition.
+		if (!options.skipTransitionCheck && existing && item.status !== undefined) {
+			const normalizedExisting: HistoryItemStatus = existing.status ?? "active"
+			if (item.status !== normalizedExisting) {
+				assertValidTransition(existing.status, item.status)
+			}
+		}
+
+		// Merge: preserve existing metadata unless explicitly overwritten
+		const merged = existing ? { ...existing, ...item } : item
+
+		// Write per-task file (source of truth)
+		await this.writeTaskFile(merged)
+
+		// Update in-memory cache
+		this.cache.set(merged.id, merged)
 		// Schedule debounced index write
 		this.scheduleIndexWrite()
 
@@ -286,6 +337,92 @@ export class TaskHistoryStore {
 		})
 	}
 
+	/**
+	 * Repair delegation inconsistencies left by a crash mid-transition.
+	 *
+	 * Called once from `initialize()` after `reconcile()`. Runs inside `withLock` to
+	 * prevent interleaving with watcher-triggered reconcile() calls. Iterates until
+	 * convergence so that one-level chained delegations visible at startup are resolved.
+	 *
+	 * Must NOT be called from within `withLock` — `withLock` is non-reentrant (promise
+	 * chain); calling `upsert` (which acquires the lock) from inside would deadlock.
+	 * `upsertCore` is called directly here instead, bypassing transition validation via
+	 * `skipTransitionCheck: true` because these writes are administrative repairs, not
+	 * runtime state-machine transitions.
+	 *
+	 * Cases repaired per pass:
+	 * - Parent `delegated` with no `awaitingChildId` → parent → `active` (invalid state)
+	 * - Parent `delegated`, child not found → parent → `active` (orphaned delegation)
+	 * - Parent `delegated`, child `completed` → parent → `active` (interrupted handoff)
+	 *
+	 * A parent awaiting an `active` child is left as-is — the child is resumable.
+	 */
+	private async reconcileDelegationState(): Promise<void> {
+		return this.withLock(async () => {
+			let repairsInThisPass: number
+			do {
+				repairsInThisPass = 0
+				// Rebuild the lookup map each pass so repairs from the previous pass
+				// are visible when evaluating chained delegations.
+				const byId = new Map(Array.from(this.cache.values()).map((i) => [i.id, i]))
+
+				for (const [, item] of byId) {
+					if (item.status !== "delegated") {
+						continue
+					}
+
+					if (!item.awaitingChildId) {
+						await this.upsertCore(
+							{ ...item, status: "active", awaitingChildId: undefined, delegatedToId: undefined },
+							{ skipTransitionCheck: true },
+						)
+						console.warn(
+							`[TaskHistoryStore] Reconciled invalid delegation: task ${item.id} → active (no awaitingChildId)`,
+						)
+						repairsInThisPass++
+						continue
+					}
+
+					const child = byId.get(item.awaitingChildId)
+
+					if (!child) {
+						await this.upsertCore(
+							{
+								...item,
+								status: "active",
+								awaitingChildId: undefined,
+								delegatedToId: undefined,
+							},
+							{ skipTransitionCheck: true },
+						)
+						console.warn(
+							`[TaskHistoryStore] Reconciled orphaned delegation: task ${item.id} → active (child ${item.awaitingChildId} not found)`,
+						)
+						repairsInThisPass++
+					} else if (child.status === "completed") {
+						await this.upsertCore(
+							{
+								...item,
+								status: "active",
+								awaitingChildId: undefined,
+								delegatedToId: undefined,
+								completedByChildId: child.id,
+								completionResultSummary:
+									child.completionResultSummary ?? "Task completed (recovered after interruption)",
+							},
+							{ skipTransitionCheck: true },
+						)
+						console.warn(
+							`[TaskHistoryStore] Reconciled interrupted handoff: task ${item.id} → active (child ${item.awaitingChildId} already completed)`,
+						)
+						repairsInThisPass++
+					}
+					// child.status === "active" or "delegated" → leave as-is this pass
+				}
+			} while (repairsInThisPass > 0)
+		})
+	}
+
 	// ────────────────────────────── Cache invalidation ──────────────────────────────
 
 	/**
@@ -354,6 +491,10 @@ export class TaskHistoryStore {
 
 		// Write the index
 		await this.writeIndex()
+
+		// Repair any delegation inconsistencies introduced by the migrated entries.
+		// reconcileDelegationState() is idempotent so running it again is safe.
+		await this.reconcileDelegationState()
 	}
 
 	// ────────────────────────────── Private: Index management ──────────────────────────────
@@ -552,7 +693,7 @@ export class TaskHistoryStore {
 					`[TaskHistoryStore] atomicReadAndUpdate: updater changed task id from ${taskId} to ${updated.id}`,
 				)
 			}
-			return this._upsertUnlocked(updated)
+			return this.upsertCore(updated)
 		})
 	}
 
@@ -590,6 +731,19 @@ export class TaskHistoryStore {
 				)
 			}
 
+			// Validate status transitions before any disk write — mirrors upsertCore guard.
+			for (const [existing, updated] of [
+				[first, updatedFirst],
+				[second, updatedSecond],
+			] as const) {
+				if (updated.status !== undefined) {
+					const normalizedExisting: HistoryItemStatus = existing.status ?? "active"
+					if (updated.status !== normalizedExisting) {
+						assertValidTransition(existing.status, updated.status)
+					}
+				}
+			}
+
 			// Merge with existing cache entries before writing, mirroring upsertCore.
 			const mergedFirst = { ...first, ...updatedFirst }
 			const mergedSecond = { ...second, ...updatedSecond }
@@ -612,17 +766,6 @@ export class TaskHistoryStore {
 		})
 	}
 
-	/**
-	 * Write a single item to disk and update the cache without triggering onWrite
-	 * or scheduling an index write. Must only be called from within a withLock callback.
-	 */
-	private async upsertCore(item: HistoryItem): Promise<void> {
-		const existing = this.cache.get(item.id)
-		const merged = existing ? { ...existing, ...item } : item
-		await this.writeTaskFile(merged)
-		this.cache.set(merged.id, merged)
-	}
-
 	// ────────────────────────────── Private: Write lock ──────────────────────────────
 
 	/**