fix: resolve race condition in new_task delegation that loses parent task history (#11331)

* fix: resolve race condition in new_task delegation that loses parent task history

When delegateParentAndOpenChild creates a child task via createTask(), the
Task constructor fires startTask() as a fire-and-forget async call. The child
immediately begins its task loop and eventually calls saveClineMessages() →
updateTaskHistory(), which reads globalState, modifies it, and writes back.

Meanwhile, delegateParentAndOpenChild persists the parent's delegation
metadata (status: 'delegated', delegatedToId, awaitingChildId, childIds) via
a separate updateTaskHistory() call AFTER createTask() returns.

These two concurrent read-modify-write operations on globalState race: the
last writer wins, overwriting the other's changes. When the child's write
lands last, the parent's delegation fields are lost, making the parent task
unresumable when the child finishes.

Fix: create the child task with startTask: false, persist the parent's
delegation metadata first, then manually call child.start(). This ensures
the parent metadata is safely in globalState before the child begins writing.

* docs: clarify Task.start() only handles new tasks, not history resume

Author: daniel-lxs

packages/types/src/task.ts

@@ -95,6 +95,9 @@ export interface CreateTaskOptions {
 	initialTodos?: TodoItem[]
 	/** Initial status for the task's history item (e.g., "active" for child tasks) */
 	initialStatus?: "active" | "delegated" | "completed"
+	/** Whether to start the task loop immediately (default: true).
+	 *  When false, the caller must invoke `task.start()` manually. */
+	startTask?: boolean
 }
 
 export enum TaskStatus {

src/__tests__/provider-delegation.spec.ts

@@ -9,9 +9,10 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
 		const providerEmit = vi.fn()
 		const parentTask = { taskId: "parent-1", emit: vi.fn() } as any
 
+		const childStart = vi.fn()
 		const updateTaskHistory = vi.fn()
 		const removeClineFromStack = vi.fn().mockResolvedValue(undefined)
-		const createTask = vi.fn().mockResolvedValue({ taskId: "child-1" })
+		const createTask = vi.fn().mockResolvedValue({ taskId: "child-1", start: childStart })
 		const handleModeSwitch = vi.fn().mockResolvedValue(undefined)
 		const getTaskWithId = vi.fn().mockImplementation(async (id: string) => {
 			if (id === "parent-1") {
@@ -62,10 +63,11 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
 
 		// Invariant: parent closed before child creation
 		expect(removeClineFromStack).toHaveBeenCalledTimes(1)
-		// Child task is created with initialStatus: "active" to avoid race conditions
+		// Child task is created with startTask: false and initialStatus: "active"
 		expect(createTask).toHaveBeenCalledWith("Do something", undefined, parentTask, {
 			initialTodos: [],
 			initialStatus: "active",
+			startTask: false,
 		})
 
 		// Metadata persistence - parent gets "delegated" status (child status is set at creation via initialStatus)
@@ -83,10 +85,61 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
 			}),
 		)
 
+		// child.start() must be called AFTER parent metadata is persisted
+		expect(childStart).toHaveBeenCalledTimes(1)
+
 		// Event emission (provider-level)
 		expect(providerEmit).toHaveBeenCalledWith(RooCodeEventName.TaskDelegated, "parent-1", "child-1")
 
 		// Mode switch
 		expect(handleModeSwitch).toHaveBeenCalledWith("code")
 	})
+
+	it("calls child.start() only after parent metadata is persisted (no race condition)", async () => {
+		const callOrder: string[] = []
+
+		const parentTask = { taskId: "parent-1", emit: vi.fn() } as any
+		const childStart = vi.fn(() => callOrder.push("child.start"))
+
+		const updateTaskHistory = vi.fn(async () => {
+			callOrder.push("updateTaskHistory")
+		})
+		const removeClineFromStack = vi.fn().mockResolvedValue(undefined)
+		const createTask = vi.fn(async () => {
+			callOrder.push("createTask")
+			return { taskId: "child-1", start: childStart }
+		})
+		const handleModeSwitch = vi.fn().mockResolvedValue(undefined)
+		const getTaskWithId = vi.fn().mockResolvedValue({
+			historyItem: {
+				id: "parent-1",
+				task: "Parent",
+				tokensIn: 0,
+				tokensOut: 0,
+				totalCost: 0,
+				childIds: [],
+			},
+		})
+
+		const provider = {
+			emit: vi.fn(),
+			getCurrentTask: vi.fn(() => parentTask),
+			removeClineFromStack,
+			createTask,
+			getTaskWithId,
+			updateTaskHistory,
+			handleModeSwitch,
+			log: vi.fn(),
+		} as unknown as ClineProvider
+
+		await (ClineProvider.prototype as any).delegateParentAndOpenChild.call(provider, {
+			parentTaskId: "parent-1",
+			message: "Do something",
+			initialTodos: [],
+			mode: "code",
+		})
+
+		// Verify ordering: createTask → updateTaskHistory → child.start
+		expect(callOrder).toEqual(["createTask", "updateTaskHistory", "child.start"])
+	})
 })

src/core/task/Task.ts

@@ -394,6 +394,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 	didAlreadyUseTool = false
 	didToolFailInCurrentTurn = false
 	didCompleteReadingStream = false
+	private _started = false
 	// No streaming parser is required.
 	assistantMessageParser?: undefined
 	private providerProfileChangeListener?: (config: { name: string; provider?: string }) => void
@@ -598,6 +599,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		onCreated?.(this)
 
 		if (startTask) {
+			this._started = true
 			if (task || images) {
 				this.startTask(task, images)
 			} else if (historyItem) {
@@ -1935,6 +1937,30 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		}
 	}
 
+	/**
+	 * Manually start a **new** task when it was created with `startTask: false`.
+	 *
+	 * This fires `startTask` as a background async operation for the
+	 * `task/images` code-path only.  It does **not** handle the
+	 * `historyItem` resume path (use the constructor with `startTask: true`
+	 * for that).  The primary use-case is in the delegation flow where the
+	 * parent's metadata must be persisted to globalState **before** the
+	 * child task begins writing its own history (avoiding a read-modify-write
+	 * race on globalState).
+	 */
+	public start(): void {
+		if (this._started) {
+			return
+		}
+		this._started = true
+
+		const { task, images } = this.metadata
+
+		if (task || images) {
+			this.startTask(task ?? undefined, images ?? undefined)
+		}
+	}
+
 	private async startTask(task?: string, images?: string[]): Promise<void> {
 		try {
 			if (this.enableBridge) {

src/core/task/__tests__/Task.spec.ts

@@ -1820,6 +1820,49 @@ describe("Cline", () => {
 			})
 		})
 	})
+
+	describe("start()", () => {
+		it("should be a no-op if the task was already started in the constructor", () => {
+			const task = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "test task",
+				startTask: false,
+			})
+
+			// Manually trigger start
+			const startTaskSpy = vi.spyOn(task as any, "startTask").mockImplementation(async () => {})
+			task.start()
+
+			expect(startTaskSpy).toHaveBeenCalledTimes(1)
+
+			// Calling start() again should be a no-op
+			task.start()
+			expect(startTaskSpy).toHaveBeenCalledTimes(1)
+		})
+
+		it("should not call startTask if already started via constructor", () => {
+			// Create a task that starts immediately (startTask defaults to true)
+			// but mock startTask to prevent actual execution
+			const startTaskSpy = vi.spyOn(Task.prototype as any, "startTask").mockImplementation(async () => {})
+
+			const task = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "test task",
+				startTask: true,
+			})
+
+			// startTask was called by the constructor
+			expect(startTaskSpy).toHaveBeenCalledTimes(1)
+
+			// Calling start() should be a no-op since _started is already true
+			task.start()
+			expect(startTaskSpy).toHaveBeenCalledTimes(1)
+
+			startTaskSpy.mockRestore()
+		})
+	})
 })
 
 describe("Queued message processing after condense", () => {

src/core/webview/ClineProvider.ts

@@ -3305,12 +3305,20 @@ export class ClineProvider
 		// Pass initialStatus: "active" to ensure the child task's historyItem is created
 		// with status from the start, avoiding race conditions where the task might
 		// call attempt_completion before status is persisted separately.
+		//
+		// Pass startTask: false to prevent the child from beginning its task loop
+		// (and writing to globalState via saveClineMessages → updateTaskHistory)
+		// before we persist the parent's delegation metadata in step 5.
+		// Without this, the child's fire-and-forget startTask() races with step 5,
+		// and the last writer to globalState overwrites the other's changes—
+		// causing the parent's delegation fields to be lost.
 		const child = await this.createTask(message, undefined, parent as any, {
 			initialTodos,
 			initialStatus: "active",
+			startTask: false,
 		})
 
-		// 5) Persist parent delegation metadata
+		// 5) Persist parent delegation metadata BEFORE the child starts writing.
 		try {
 			const { historyItem } = await this.getTaskWithId(parentTaskId)
 			const childIds = Array.from(new Set([...(historyItem.childIds ?? []), child.taskId]))
@@ -3330,7 +3338,10 @@ export class ClineProvider
 			)
 		}
 
-		// 6) Emit TaskDelegated (provider-level)
+		// 6) Start the child task now that parent metadata is safely persisted.
+		child.start()
+
+		// 7) Emit TaskDelegated (provider-level)
 		try {
 			this.emit(RooCodeEventName.TaskDelegated, parentTaskId, child.taskId)
 		} catch {