fix(task-lifecycle): prevent parent repair from winning race against child cancellation

Author: edelauna

src/__tests__/helpers/provider-stub.ts

@@ -3,15 +3,16 @@ import { ClineProvider } from "../../core/webview/ClineProvider"
 /**
  * Augments a plain stub object with the instance fields and bound methods that
  * ClineProvider methods read from `this` (runDelegationTransition,
- * delegationTransitionLocks, cancelledDelegationChildIds), so tests can call
- * private methods via `(ClineProvider.prototype as any).method.call(stub, …)`
+ * delegationTransitionLocks, cancelledDelegationChildIds, cancellingDelegationChildIds),
+ * so tests can call private methods via `(ClineProvider.prototype as any).method.call(stub, …)`
  * without instantiating a real ClineProvider.
  */
 export function makeProviderStub<T extends object>(stub: T): T {
 	const s = stub as any
 	const proto = ClineProvider.prototype as any
 	s.delegationTransitionLocks ??= new Map()
 	s.cancelledDelegationChildIds ??= new Set()
+	s.cancellingDelegationChildIds ??= new Set()
 	s.log ??= vi.fn()
 	s.taskHistoryStore ??= { get: () => undefined }
 	s.runDelegationTransition = proto.runDelegationTransition.bind(s)

src/core/webview/ClineProvider.ts

@@ -167,6 +167,14 @@ export class ClineProvider
 	private clineStack: Task[] = []
 	private delegationTransitionLocks?: Map<string, Promise<void>>
 	private cancelledDelegationChildIds = new Set<string>()
+	// Marks a child whose cancellation is currently in flight, from the moment cancelTask()
+	// is invoked until its "interrupted" status write lands (or the cancel path bails out).
+	// removeClineFromStack()'s delegation repair must not run against a stale "active" read
+	// while this is set — otherwise a concurrent navigation (e.g. showTaskWithId(parentTaskId)
+	// from the user clicking "back to parent" right after hitting Stop) can win the race
+	// against cancelTask()'s own runDelegationTransition call and repair the parent to
+	// "active" before "interrupted" is ever persisted, permanently severing the delegation link.
+	private cancellingDelegationChildIds = new Set<string>()
 	private codeIndexStatusSubscription?: vscode.Disposable
 	private codeIndexManager?: CodeIndexManager
 	private _workspaceTracker?: WorkspaceTracker // workSpaceTracker read-only for access outside this class
@@ -543,6 +551,19 @@ export class ClineProvider
 								return
 							}
 
+							// A cancellation for this child may be in flight (cancelTask() has
+							// marked it synchronously but its "interrupted" write hasn't landed
+							// yet, since both paths serialize on the same per-parent transition
+							// lock and this call won the race). Repairing here would clear
+							// awaitingChildId based on a stale "active" read and permanently
+							// sever the delegation link. Defer to cancelTask()'s own write instead.
+							if (this.cancellingDelegationChildIds.has(childTaskId)) {
+								this.log(
+									`[ClineProvider#removeClineFromStack] Skipping parent repair: cancellation for child ${childTaskId} is in flight`,
+								)
+								return
+							}
+
 							assertValidTransition(parentHistory.status, "active")
 							await this.updateTaskHistory({
 								...parentHistory,
@@ -3142,6 +3163,23 @@ export class ClineProvider
 
 		console.log(`[cancelTask] cancelling task ${task.taskId}.${task.instanceId}`)
 
+		// Mark this child as "cancellation in flight" synchronously, before any await, so a
+		// concurrent removeClineFromStack() (e.g. from the user navigating back to the parent
+		// right after clicking Stop) cannot win the race against this function's own
+		// runDelegationTransition call below and repair the parent from a stale "active" read
+		// before "interrupted" is persisted (see cancellingDelegationChildIds doc comment).
+		if (task.parentTaskId) {
+			this.cancellingDelegationChildIds.add(task.taskId)
+		}
+
+		try {
+			await this.cancelTaskInternal(task)
+		} finally {
+			this.cancellingDelegationChildIds.delete(task.taskId)
+		}
+	}
+
+	private async cancelTaskInternal(task: Task): Promise<void> {
 		let historyItem: HistoryItem | undefined
 		try {
 			const history = await this.getTaskWithId(task.taskId)

src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts

@@ -824,6 +824,68 @@ describe("ClineProvider flicker-free cancel", () => {
 		)
 	})
 
+	// Regression test for the race where a user clicks Stop on a freshly-delegated
+	// child and immediately navigates back to the parent (showTaskWithId), before
+	// cancelTask()'s own persistence of childHistory.status = "interrupted" has
+	// landed. Both cancelTask() and removeClineFromStack() serialize their parent
+	// writes through runDelegationTransition(parentTaskId, ...), but removeClineFromStack
+	// only skips its repair when taskHistoryStore.get(childTaskId)?.status === "interrupted".
+	// If removeClineFromStack's transition wins the race and runs while the store still
+	// reports "active" (the write from cancelTask() hasn't landed yet), it incorrectly
+	// repairs the parent to "active" and clears awaitingChildId, permanently severing
+	// the delegation link before the child ever gets a chance to report back.
+	it("removeClineFromStack does not repair parent when a cancellation for the child is in flight", async () => {
+		const parentHistory: HistoryItem = {
+			id: "parent-1",
+			number: 1,
+			task: "parent task",
+			ts: Date.now(),
+			tokensIn: 10,
+			tokensOut: 20,
+			totalCost: 0.001,
+			workspace: "/test/workspace",
+			status: "delegated",
+			awaitingChildId: "child-1",
+			delegatedToId: "child-1",
+		}
+
+		const childTask = {
+			taskId: "child-1",
+			instanceId: "inst-child",
+			parentTaskId: "parent-1",
+			emit: vi.fn(),
+			abortTask: vi.fn().mockResolvedValue(undefined),
+		}
+		;(provider as any).clineStack = [childTask]
+		;(provider as any).taskEventListeners = new Map()
+
+		// The store still reports "active" — cancelTask()'s write to "interrupted"
+		// has not landed yet. This is the pre-write window of the race.
+		vi.spyOn((provider as any).taskHistoryStore, "get").mockImplementation((id: unknown) =>
+			id === "child-1" ? { status: "active" } : undefined,
+		)
+
+		provider.getTaskWithId = vi.fn().mockImplementation((id) => {
+			if (id === "parent-1") return Promise.resolve({ historyItem: parentHistory })
+			throw new Error(`unexpected task lookup: ${id}`)
+		}) as any
+
+		const updateTaskHistorySpy = vi.spyOn(provider, "updateTaskHistory").mockResolvedValue([])
+
+		// Simulate cancelTask() having already synchronously marked this child as
+		// "being cancelled" before its own await chain reaches the history write.
+		;(provider as any).cancellingDelegationChildIds.add("child-1")
+
+		await (provider as any).removeClineFromStack()
+
+		// Parent must NOT be transitioned to active while the child's cancellation
+		// is still in flight — repairing here would clear awaitingChildId and
+		// permanently sever the delegation link before "interrupted" is persisted.
+		expect(updateTaskHistorySpy).not.toHaveBeenCalledWith(
+			expect.objectContaining({ id: "parent-1", status: "active" }),
+		)
+	})
+
 	afterAll(() => {
 		vi.restoreAllMocks()
 	})