feat(ClineProvider): guarding parent, child transitions

Author: edelauna

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/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts

@@ -389,4 +389,24 @@ describe("TaskHistoryStore upsert transition guard", () => {
 		// Status is preserved from the existing cache entry
 		expect(store.get("task-guard-4")?.status).toBe("completed")
 	})
+
+	it("atomicReadAndUpdate bypasses the upsertCore transition guard (skipTransitionCheck)", async () => {
+		// atomicReadAndUpdate passes skipTransitionCheck: true — the updater is
+		// responsible for calling assertValidTransition itself. This test confirms
+		// the path doesn't double-guard: a valid update through atomicReadAndUpdate
+		// succeeds even when the status change would normally be guarded.
+		const item = makeItem({ id: "task-atomic-guard", status: "active" })
+		await store.upsert(item)
+
+		// active → delegated via atomicReadAndUpdate — must succeed
+		await expect(
+			store.atomicReadAndUpdate("task-atomic-guard", (current) => ({
+				...current,
+				status: "delegated" as const,
+				awaitingChildId: "some-child",
+			})),
+		).resolves.toBeDefined()
+
+		expect(store.get("task-atomic-guard")?.status).toBe("delegated")
+	})
 })

src/core/task-persistence/__tests__/TaskHistoryStore.spec.ts

@@ -6,7 +6,7 @@ import * as os from "os"
 
 import type { HistoryItem } from "@roo-code/types"
 
-import { TaskHistoryStore } from "../TaskHistoryStore"
+import { TaskHistoryStore, assertValidTransition } from "../TaskHistoryStore"
 import { GlobalFileNames } from "../../../shared/globalFileNames"
 
 vi.mock("../../../utils/storage", () => ({
@@ -641,5 +641,54 @@ describe("TaskHistoryStore", () => {
 
 			storeWithCallback.dispose()
 		})
+
+		it("propagates throw from first updater — neither record is written", async () => {
+			await store.initialize()
+
+			const child = makeHistoryItem({ id: "child-updater-throw", status: "active" })
+			const parent = makeHistoryItem({ id: "parent-updater-throw", status: "delegated" })
+			await store.upsert(child)
+			await store.upsert(parent)
+
+			await expect(
+				store.atomicUpdatePair(
+					"child-updater-throw",
+					"parent-updater-throw",
+					(_c) => {
+						throw new Error("updater exploded")
+					},
+					(p) => p,
+				),
+			).rejects.toThrow("updater exploded")
+
+			// Neither record should have been modified
+			expect(store.get("child-updater-throw")?.status).toBe("active")
+			expect(store.get("parent-updater-throw")?.status).toBe("delegated")
+		})
+
+		it("transition guard in first updater: delegated → completed throws assertValidTransition", async () => {
+			await store.initialize()
+
+			const child = makeHistoryItem({ id: "child-guard-pair", status: "delegated" })
+			const parent = makeHistoryItem({ id: "parent-guard-pair", status: "active" })
+			await store.upsert(child)
+			await store.upsert(parent)
+
+			await expect(
+				store.atomicUpdatePair(
+					"child-guard-pair",
+					"parent-guard-pair",
+					(c) => {
+						assertValidTransition(c.status, "completed")
+						return { ...c, status: "completed" as const }
+					},
+					(p) => p,
+				),
+			).rejects.toThrow("Invalid task status transition: delegated → completed")
+
+			// Original statuses preserved
+			expect(store.get("child-guard-pair")?.status).toBe("delegated")
+			expect(store.get("parent-guard-pair")?.status).toBe("active")
+		})
 	})
 })

src/core/webview/ClineProvider.ts

@@ -2348,9 +2348,8 @@ export class ClineProvider
 		}
 
 		try {
-			const { isZooCodeAuthenticated, getCachedZooCodeUserInfo, getZooCodeBaseUrl } = await import(
-				"../../services/zoo-code-auth"
-			)
+			const { isZooCodeAuthenticated, getCachedZooCodeUserInfo, getZooCodeBaseUrl } =
+				await import("../../services/zoo-code-auth")
 			const userInfo = getCachedZooCodeUserInfo()
 			zooCodeState = {
 				zooCodeIsAuthenticated: await isZooCodeAuthenticated(),
@@ -3778,8 +3777,12 @@ export class ClineProvider
 			await this.taskHistoryStore.atomicUpdatePair(
 				childTaskId,
 				parentTaskId,
-				(child) => ({ ...child, status: "completed" as const }),
+				(child) => {
+					assertValidTransition(child.status, "completed")
+					return { ...child, status: "completed" as const }
+				},
 				(parent) => {
+					assertValidTransition(parent.status, "active")
 					const childIds = Array.from(new Set([...(parent.childIds ?? []), childTaskId]))
 					updatedHistory = {
 						...parent,
@@ -3803,7 +3806,8 @@ export class ClineProvider
 				}
 				if (updatedParent) {
 					await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedParent })
-				}			}
+				}
+			}
 
 			// 6) Emit TaskDelegationCompleted (provider-level)
 			try {