Browse Source

fix: serialize taskHistory writes and fix delegation status overwrite race (#11335)

Add a promise-chain mutex (withTaskHistoryLock) to serialize all
read-modify-write operations on taskHistory, preventing concurrent
interleaving from silently dropping entries.

Reorder reopenParentFromDelegation to close the child instance
before marking it completed, so the abort path's stale 'active'
status write no longer overwrites the 'completed' state.

Covered by new tests: RPD-04/05/06, UTH-02/04, and a full mutex
concurrency suite.
Hannes Rudolph 3 days ago
parent
commit
115d6c5fce

+ 244 - 1
src/__tests__/history-resume-delegation.spec.ts

@@ -387,6 +387,7 @@ describe("History resume delegation - parent metadata transitions", () => {
 
 	it("reopenParentFromDelegation emits events in correct order: TaskDelegationCompleted → TaskDelegationResumed", async () => {
 		const emitSpy = vi.fn()
+		const updateTaskHistory = vi.fn().mockResolvedValue([])
 
 		const provider = {
 			contextProxy: { globalStorageUri: { fsPath: "/tmp" } },
@@ -411,7 +412,7 @@ describe("History resume delegation - parent metadata transitions", () => {
 				overwriteClineMessages: vi.fn().mockResolvedValue(undefined),
 				overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined),
 			}),
-			updateTaskHistory: vi.fn().mockResolvedValue([]),
+			updateTaskHistory,
 		} as unknown as ClineProvider
 
 		vi.mocked(readTaskMessages).mockResolvedValue([])
@@ -433,6 +434,92 @@ describe("History resume delegation - parent metadata transitions", () => {
 		const resumedIdx = emitSpy.mock.calls.findIndex((c) => c[0] === RooCodeEventName.TaskDelegationResumed)
 		expect(completedIdx).toBeGreaterThanOrEqual(0)
 		expect(resumedIdx).toBeGreaterThan(completedIdx)
+
+		// RPD-05: verify parent metadata persistence happens before TaskDelegationCompleted emit
+		const parentUpdateCallIdx = updateTaskHistory.mock.calls.findIndex((call) => {
+			const item = call[0] as { id?: string; status?: string } | undefined
+			return item?.id === "p3" && item.status === "active"
+		})
+		expect(parentUpdateCallIdx).toBeGreaterThanOrEqual(0)
+
+		const parentUpdateCallOrder = updateTaskHistory.mock.invocationCallOrder[parentUpdateCallIdx]
+		const completedEmitCallOrder = emitSpy.mock.invocationCallOrder[completedIdx]
+		expect(parentUpdateCallOrder).toBeLessThan(completedEmitCallOrder)
+	})
+
+	it("reopenParentFromDelegation continues when overwrite operations fail and still resumes/emits (RPD-06)", async () => {
+		const emitSpy = vi.fn()
+		const parentInstance = {
+			resumeAfterDelegation: vi.fn().mockResolvedValue(undefined),
+			overwriteClineMessages: vi.fn().mockRejectedValue(new Error("ui overwrite failed")),
+			overwriteApiConversationHistory: vi.fn().mockRejectedValue(new Error("api overwrite failed")),
+		}
+
+		const provider = {
+			contextProxy: { globalStorageUri: { fsPath: "/tmp" } },
+			getTaskWithId: vi.fn().mockImplementation(async (id: string) => {
+				if (id === "parent-rpd06") {
+					return {
+						historyItem: {
+							id: "parent-rpd06",
+							status: "delegated",
+							awaitingChildId: "child-rpd06",
+							childIds: ["child-rpd06"],
+							ts: 800,
+							task: "Parent RPD-06",
+							tokensIn: 0,
+							tokensOut: 0,
+							totalCost: 0,
+						},
+					}
+				}
+
+				return {
+					historyItem: {
+						id: "child-rpd06",
+						status: "active",
+						ts: 801,
+						task: "Child RPD-06",
+						tokensIn: 0,
+						tokensOut: 0,
+						totalCost: 0,
+					},
+				}
+			}),
+			emit: emitSpy,
+			getCurrentTask: vi.fn(() => ({ taskId: "child-rpd06" })),
+			removeClineFromStack: vi.fn().mockResolvedValue(undefined),
+			createTaskWithHistoryItem: vi.fn().mockResolvedValue(parentInstance),
+			updateTaskHistory: vi.fn().mockResolvedValue([]),
+		} as unknown as ClineProvider
+
+		vi.mocked(readTaskMessages).mockResolvedValue([])
+		vi.mocked(readApiMessages).mockResolvedValue([])
+
+		await expect(
+			(ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, {
+				parentTaskId: "parent-rpd06",
+				childTaskId: "child-rpd06",
+				completionResultSummary: "Subtask finished despite overwrite failures",
+			}),
+		).resolves.toBeUndefined()
+
+		expect(parentInstance.overwriteClineMessages).toHaveBeenCalledTimes(1)
+		expect(parentInstance.overwriteApiConversationHistory).toHaveBeenCalledTimes(1)
+		expect(parentInstance.resumeAfterDelegation).toHaveBeenCalledTimes(1)
+
+		expect(emitSpy).toHaveBeenCalledWith(
+			RooCodeEventName.TaskDelegationCompleted,
+			"parent-rpd06",
+			"child-rpd06",
+			"Subtask finished despite overwrite failures",
+		)
+		expect(emitSpy).toHaveBeenCalledWith(RooCodeEventName.TaskDelegationResumed, "parent-rpd06", "child-rpd06")
+
+		const completedIdx = emitSpy.mock.calls.findIndex((c) => c[0] === RooCodeEventName.TaskDelegationCompleted)
+		const resumedIdx = emitSpy.mock.calls.findIndex((c) => c[0] === RooCodeEventName.TaskDelegationResumed)
+		expect(completedIdx).toBeGreaterThanOrEqual(0)
+		expect(resumedIdx).toBeGreaterThan(completedIdx)
 	})
 
 	it("reopenParentFromDelegation does NOT emit TaskPaused or TaskUnpaused (new flow only)", async () => {
@@ -480,6 +567,162 @@ describe("History resume delegation - parent metadata transitions", () => {
 		expect(eventNames).not.toContain(RooCodeEventName.TaskSpawned)
 	})
 
+	it("reopenParentFromDelegation skips child close when current task differs and still reopens parent (RPD-02)", async () => {
+		const parentInstance = {
+			resumeAfterDelegation: vi.fn().mockResolvedValue(undefined),
+			overwriteClineMessages: vi.fn().mockResolvedValue(undefined),
+			overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined),
+		}
+
+		const updateTaskHistory = vi.fn().mockResolvedValue([])
+		const removeClineFromStack = vi.fn().mockResolvedValue(undefined)
+		const createTaskWithHistoryItem = vi.fn().mockResolvedValue(parentInstance)
+
+		const provider = {
+			contextProxy: { globalStorageUri: { fsPath: "/tmp" } },
+			getTaskWithId: vi.fn().mockImplementation(async (id: string) => {
+				if (id === "parent-rpd02") {
+					return {
+						historyItem: {
+							id: "parent-rpd02",
+							status: "delegated",
+							awaitingChildId: "child-rpd02",
+							childIds: ["child-rpd02"],
+							ts: 600,
+							task: "Parent RPD-02",
+							tokensIn: 0,
+							tokensOut: 0,
+							totalCost: 0,
+						},
+					}
+				}
+				return {
+					historyItem: {
+						id: "child-rpd02",
+						status: "active",
+						ts: 601,
+						task: "Child RPD-02",
+						tokensIn: 0,
+						tokensOut: 0,
+						totalCost: 0,
+					},
+				}
+			}),
+			emit: vi.fn(),
+			getCurrentTask: vi.fn(() => ({ taskId: "different-open-task" })),
+			removeClineFromStack,
+			createTaskWithHistoryItem,
+			updateTaskHistory,
+		} as unknown as ClineProvider
+
+		vi.mocked(readTaskMessages).mockResolvedValue([])
+		vi.mocked(readApiMessages).mockResolvedValue([])
+
+		await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, {
+			parentTaskId: "parent-rpd02",
+			childTaskId: "child-rpd02",
+			completionResultSummary: "Child done without being current",
+		})
+
+		expect(removeClineFromStack).not.toHaveBeenCalled()
+		expect(updateTaskHistory).toHaveBeenCalledWith(
+			expect.objectContaining({
+				id: "child-rpd02",
+				status: "completed",
+			}),
+		)
+		expect(createTaskWithHistoryItem).toHaveBeenCalledWith(
+			expect.objectContaining({
+				id: "parent-rpd02",
+				status: "active",
+				completedByChildId: "child-rpd02",
+			}),
+			{ startTask: false },
+		)
+		expect(parentInstance.resumeAfterDelegation).toHaveBeenCalledTimes(1)
+	})
+
+	it("reopenParentFromDelegation logs child status persistence failure and continues reopen flow (RPD-04)", async () => {
+		const logSpy = vi.fn()
+		const emitSpy = vi.fn()
+		const parentInstance = {
+			resumeAfterDelegation: vi.fn().mockResolvedValue(undefined),
+			overwriteClineMessages: vi.fn().mockResolvedValue(undefined),
+			overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined),
+		}
+
+		const updateTaskHistory = vi.fn().mockImplementation(async (historyItem: { id?: string }) => {
+			if (historyItem.id === "child-rpd04") {
+				throw new Error("child status persist failed")
+			}
+			return []
+		})
+
+		const provider = {
+			contextProxy: { globalStorageUri: { fsPath: "/tmp" } },
+			getTaskWithId: vi.fn().mockImplementation(async (id: string) => {
+				if (id === "parent-rpd04") {
+					return {
+						historyItem: {
+							id: "parent-rpd04",
+							status: "delegated",
+							awaitingChildId: "child-rpd04",
+							childIds: ["child-rpd04"],
+							ts: 700,
+							task: "Parent RPD-04",
+							tokensIn: 0,
+							tokensOut: 0,
+							totalCost: 0,
+						},
+					}
+				}
+				return {
+					historyItem: {
+						id: "child-rpd04",
+						status: "active",
+						ts: 701,
+						task: "Child RPD-04",
+						tokensIn: 0,
+						tokensOut: 0,
+						totalCost: 0,
+					},
+				}
+			}),
+			emit: emitSpy,
+			log: logSpy,
+			getCurrentTask: vi.fn(() => ({ taskId: "child-rpd04" })),
+			removeClineFromStack: vi.fn().mockResolvedValue(undefined),
+			createTaskWithHistoryItem: vi.fn().mockResolvedValue(parentInstance),
+			updateTaskHistory,
+		} as unknown as ClineProvider
+
+		vi.mocked(readTaskMessages).mockResolvedValue([])
+		vi.mocked(readApiMessages).mockResolvedValue([])
+
+		await expect(
+			(ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, {
+				parentTaskId: "parent-rpd04",
+				childTaskId: "child-rpd04",
+				completionResultSummary: "Child completion with persistence failure",
+			}),
+		).resolves.toBeUndefined()
+
+		expect(logSpy).toHaveBeenCalledWith(
+			expect.stringContaining(
+				"[reopenParentFromDelegation] Failed to persist child completed status for child-rpd04:",
+			),
+		)
+		expect(updateTaskHistory).toHaveBeenCalledWith(
+			expect.objectContaining({
+				id: "parent-rpd04",
+				status: "active",
+				completedByChildId: "child-rpd04",
+			}),
+		)
+		expect(parentInstance.resumeAfterDelegation).toHaveBeenCalledTimes(1)
+		expect(emitSpy).toHaveBeenCalledWith(RooCodeEventName.TaskDelegationResumed, "parent-rpd04", "child-rpd04")
+	})
+
 	it("handles empty history gracefully when injecting synthetic messages", async () => {
 		const provider = {
 			contextProxy: { globalStorageUri: { fsPath: "/tmp" } },

+ 67 - 41
src/core/webview/ClineProvider.ts

@@ -150,6 +150,7 @@ export class ClineProvider
 	private currentWorkspacePath: string | undefined
 
 	private recentTasksCache?: string[]
+	private taskHistoryWriteLock: Promise<void> = Promise.resolve()
 	private pendingOperations: Map<string, PendingEditOperation> = new Map()
 	private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds
 
@@ -1800,10 +1801,12 @@ export class ClineProvider
 			}
 
 			// Delete all tasks from state in one batch
-			const taskHistory = this.getGlobalState("taskHistory") ?? []
-			const updatedTaskHistory = taskHistory.filter((task) => !allIdsToDelete.includes(task.id))
-			await this.updateGlobalState("taskHistory", updatedTaskHistory)
-			this.recentTasksCache = undefined
+			await this.withTaskHistoryLock(async () => {
+				const taskHistory = this.getGlobalState("taskHistory") ?? []
+				const updatedTaskHistory = taskHistory.filter((task) => !allIdsToDelete.includes(task.id))
+				await this.updateGlobalState("taskHistory", updatedTaskHistory)
+				this.recentTasksCache = undefined
+			})
 
 			// Delete associated shadow repositories or branches and task directories
 			const globalStorageDir = this.contextProxy.globalStorageUri.fsPath
@@ -1844,10 +1847,12 @@ export class ClineProvider
 	}
 
 	async deleteTaskFromState(id: string) {
-		const taskHistory = this.getGlobalState("taskHistory") ?? []
-		const updatedTaskHistory = taskHistory.filter((task) => task.id !== id)
-		await this.updateGlobalState("taskHistory", updatedTaskHistory)
-		this.recentTasksCache = undefined
+		await this.withTaskHistoryLock(async () => {
+			const taskHistory = this.getGlobalState("taskHistory") ?? []
+			const updatedTaskHistory = taskHistory.filter((task) => task.id !== id)
+			await this.updateGlobalState("taskHistory", updatedTaskHistory)
+			this.recentTasksCache = undefined
+		})
 		await this.postStateToWebview()
 	}
 
@@ -2514,6 +2519,19 @@ export class ClineProvider
 		}
 	}
 
+	/**
+	 * Serializes all read-modify-write operations on taskHistory to prevent
+	 * concurrent interleaving that can cause entries to vanish.
+	 */
+	private withTaskHistoryLock<T>(fn: () => Promise<T>): Promise<T> {
+		const result = this.taskHistoryWriteLock.then(fn, fn) // run even if previous write errored
+		this.taskHistoryWriteLock = result.then(
+			() => {},
+			() => {},
+		) // swallow for chain continuity
+		return result
+	}
+
 	/**
 	 * Updates a task in the task history and optionally broadcasts the updated history to the webview.
 	 * @param item The history item to update or add
@@ -2521,34 +2539,36 @@ export class ClineProvider
 	 * @returns The updated task history array
 	 */
 	async updateTaskHistory(item: HistoryItem, options: { broadcast?: boolean } = {}): Promise<HistoryItem[]> {
-		const { broadcast = true } = options
-		const history = (this.getGlobalState("taskHistory") as HistoryItem[] | undefined) || []
-		const existingItemIndex = history.findIndex((h) => h.id === item.id)
-		const wasExisting = existingItemIndex !== -1
-
-		if (wasExisting) {
-			// Preserve existing metadata (e.g., delegation fields) unless explicitly overwritten.
-			// This prevents loss of status/awaitingChildId/delegatedToId when tasks are reopened,
-			// terminated, or when routine message persistence occurs.
-			history[existingItemIndex] = {
-				...history[existingItemIndex],
-				...item,
+		return this.withTaskHistoryLock(async () => {
+			const { broadcast = true } = options
+			const history = (this.getGlobalState("taskHistory") as HistoryItem[] | undefined) || []
+			const existingItemIndex = history.findIndex((h) => h.id === item.id)
+			const wasExisting = existingItemIndex !== -1
+
+			if (wasExisting) {
+				// Preserve existing metadata (e.g., delegation fields) unless explicitly overwritten.
+				// This prevents loss of status/awaitingChildId/delegatedToId when tasks are reopened,
+				// terminated, or when routine message persistence occurs.
+				history[existingItemIndex] = {
+					...history[existingItemIndex],
+					...item,
+				}
+			} else {
+				history.push(item)
 			}
-		} else {
-			history.push(item)
-		}
 
-		await this.updateGlobalState("taskHistory", history)
-		this.recentTasksCache = undefined
+			await this.updateGlobalState("taskHistory", history)
+			this.recentTasksCache = undefined
 
-		// Broadcast the updated history to the webview if requested.
-		// Prefer per-item updates to avoid repeatedly cloning/sending the full history.
-		if (broadcast && this.isViewLaunched) {
-			const updatedItem = wasExisting ? history[existingItemIndex] : item
-			await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem })
-		}
+			// Broadcast the updated history to the webview if requested.
+			// Prefer per-item updates to avoid repeatedly cloning/sending the full history.
+			if (broadcast && this.isViewLaunched) {
+				const updatedItem = wasExisting ? history[existingItemIndex] : item
+				await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem })
+			}
 
-		return history
+			return history
+		})
 	}
 
 	/**
@@ -3421,7 +3441,19 @@ export class ClineProvider
 
 		await saveApiMessages({ messages: parentApiMessages as any, taskId: parentTaskId, globalStoragePath })
 
-		// 3) Update child metadata to "completed" status
+		// 3) Close child instance if still open (single-open-task invariant).
+		//    This MUST happen BEFORE updating the child's status to "completed" because
+		//    removeClineFromStack() → abortTask(true) → saveClineMessages() writes
+		//    the historyItem with initialStatus (typically "active"), which would
+		//    overwrite a "completed" status set earlier.
+		const current = this.getCurrentTask()
+		if (current?.taskId === childTaskId) {
+			await this.removeClineFromStack()
+		}
+
+		// 4) Update child metadata to "completed" status.
+		//    This runs after the abort so it overwrites the stale "active" status
+		//    that saveClineMessages() may have written during step 3.
 		try {
 			const { historyItem: childHistory } = await this.getTaskWithId(childTaskId)
 			await this.updateTaskHistory({
@@ -3436,7 +3468,7 @@ export class ClineProvider
 			)
 		}
 
-		// 4) Update parent metadata and persist BEFORE emitting completion event
+		// 5) Update parent metadata and persist BEFORE emitting completion event
 		const childIds = Array.from(new Set([...(historyItem.childIds ?? []), childTaskId]))
 		const updatedHistory: typeof historyItem = {
 			...historyItem,
@@ -3448,19 +3480,13 @@ export class ClineProvider
 		}
 		await this.updateTaskHistory(updatedHistory)
 
-		// 5) Emit TaskDelegationCompleted (provider-level)
+		// 6) Emit TaskDelegationCompleted (provider-level)
 		try {
 			this.emit(RooCodeEventName.TaskDelegationCompleted, parentTaskId, childTaskId, completionResultSummary)
 		} catch {
 			// non-fatal
 		}
 
-		// 6) Close child instance if still open (single-open-task invariant)
-		const current = this.getCurrentTask()
-		if (current?.taskId === childTaskId) {
-			await this.removeClineFromStack()
-		}
-
 		// 7) Reopen the parent from history as the sole active task (restores saved mode)
 		//    IMPORTANT: startTask=false to suppress resume-from-history ask scheduling
 		const parentInstance = await this.createTaskWithHistoryItem(updatedHistory, { startTask: false })

+ 161 - 0
src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts

@@ -420,6 +420,74 @@ describe("ClineProvider Task History Synchronization", () => {
 			expect(taskHistoryItemUpdatedCalls.length).toBe(0)
 		})
 
+		it("preserves delegated metadata on partial update unless explicitly overwritten (UTH-02)", async () => {
+			await provider.resolveWebviewView(mockWebviewView)
+			provider.isViewLaunched = true
+
+			const initial = createHistoryItem({
+				id: "task-delegated-metadata",
+				task: "Delegated task",
+				status: "delegated",
+				delegatedToId: "child-1",
+				awaitingChildId: "child-1",
+				childIds: ["child-1"],
+			})
+
+			await provider.updateTaskHistory(initial, { broadcast: false })
+
+			// Partial update intentionally omits delegated metadata fields.
+			const partialUpdate: HistoryItem = {
+				...createHistoryItem({ id: "task-delegated-metadata", task: "Delegated task (updated)" }),
+				status: "active",
+			}
+
+			const updatedHistory = await provider.updateTaskHistory(partialUpdate, { broadcast: false })
+			const updatedItem = updatedHistory.find((item) => item.id === "task-delegated-metadata")
+
+			expect(updatedItem).toBeDefined()
+			expect(updatedItem?.status).toBe("active")
+			expect(updatedItem?.delegatedToId).toBe("child-1")
+			expect(updatedItem?.awaitingChildId).toBe("child-1")
+			expect(updatedItem?.childIds).toEqual(["child-1"])
+		})
+
+		it("invalidates recentTasksCache on updateTaskHistory (UTH-04)", async () => {
+			const workspace = provider.cwd
+			const tsBase = Date.now()
+
+			await provider.updateTaskHistory(
+				createHistoryItem({
+					id: "cache-seed",
+					task: "Cache seed",
+					workspace,
+					ts: tsBase,
+				}),
+				{ broadcast: false },
+			)
+
+			const initialRecent = provider.getRecentTasks()
+			expect(initialRecent).toContain("cache-seed")
+
+			// Prime cache and verify internal cache is set.
+			expect((provider as unknown as { recentTasksCache?: string[] }).recentTasksCache).toEqual(initialRecent)
+
+			await provider.updateTaskHistory(
+				createHistoryItem({
+					id: "cache-new",
+					task: "Cache new",
+					workspace,
+					ts: tsBase + 1,
+				}),
+				{ broadcast: false },
+			)
+
+			// Direct assertion for invalidation side-effect.
+			expect((provider as unknown as { recentTasksCache?: string[] }).recentTasksCache).toBeUndefined()
+
+			const recomputedRecent = provider.getRecentTasks()
+			expect(recomputedRecent).toContain("cache-new")
+		})
+
 		it("updates existing task in history", async () => {
 			await provider.resolveWebviewView(mockWebviewView)
 			provider.isViewLaunched = true
@@ -597,4 +665,97 @@ describe("ClineProvider Task History Synchronization", () => {
 			expect(state.taskHistory.some((item: HistoryItem) => item.workspace === "/different/workspace")).toBe(true)
 		})
 	})
+
+	describe("taskHistory write lock (mutex)", () => {
+		it("serializes concurrent updateTaskHistory calls so no entries are lost", async () => {
+			await provider.resolveWebviewView(mockWebviewView)
+
+			// Fire 5 concurrent updateTaskHistory calls
+			const items = Array.from({ length: 5 }, (_, i) =>
+				createHistoryItem({ id: `concurrent-${i}`, task: `Task ${i}` }),
+			)
+
+			await Promise.all(items.map((item) => provider.updateTaskHistory(item, { broadcast: false })))
+
+			// All 5 entries must survive
+			const history = (provider as any).contextProxy.getGlobalState("taskHistory") as HistoryItem[]
+			const ids = history.map((h: HistoryItem) => h.id)
+			for (const item of items) {
+				expect(ids).toContain(item.id)
+			}
+			expect(history.length).toBe(5)
+		})
+
+		it("serializes concurrent update and deleteTaskFromState so they don't corrupt each other", async () => {
+			await provider.resolveWebviewView(mockWebviewView)
+
+			// Seed with two items
+			const keep = createHistoryItem({ id: "keep-me", task: "Keep" })
+			const remove = createHistoryItem({ id: "remove-me", task: "Remove" })
+			await provider.updateTaskHistory(keep, { broadcast: false })
+			await provider.updateTaskHistory(remove, { broadcast: false })
+
+			// Concurrently: add a new item AND delete "remove-me"
+			const newItem = createHistoryItem({ id: "new-item", task: "New" })
+			await Promise.all([
+				provider.updateTaskHistory(newItem, { broadcast: false }),
+				provider.deleteTaskFromState("remove-me"),
+			])
+
+			const history = (provider as any).contextProxy.getGlobalState("taskHistory") as HistoryItem[]
+			const ids = history.map((h: HistoryItem) => h.id)
+			expect(ids).toContain("keep-me")
+			expect(ids).toContain("new-item")
+			expect(ids).not.toContain("remove-me")
+		})
+
+		it("does not block subsequent writes when a previous write errors", async () => {
+			await provider.resolveWebviewView(mockWebviewView)
+
+			// Temporarily make updateGlobalState throw
+			const origUpdateGlobalState = (provider as any).updateGlobalState.bind(provider)
+			let callCount = 0
+			;(provider as any).updateGlobalState = vi.fn().mockImplementation((...args: unknown[]) => {
+				callCount++
+				if (callCount === 1) {
+					return Promise.reject(new Error("simulated write failure"))
+				}
+				return origUpdateGlobalState(...args)
+			})
+
+			// First call should fail
+			const item1 = createHistoryItem({ id: "fail-item", task: "Fail" })
+			await expect(provider.updateTaskHistory(item1, { broadcast: false })).rejects.toThrow(
+				"simulated write failure",
+			)
+
+			// Second call should still succeed (lock not stuck)
+			const item2 = createHistoryItem({ id: "ok-item", task: "OK" })
+			const result = await provider.updateTaskHistory(item2, { broadcast: false })
+			expect(result.some((h) => h.id === "ok-item")).toBe(true)
+		})
+
+		it("serializes concurrent updates to the same item preserving the last write", async () => {
+			await provider.resolveWebviewView(mockWebviewView)
+
+			const base = createHistoryItem({ id: "race-item", task: "Original" })
+			await provider.updateTaskHistory(base, { broadcast: false })
+
+			// Fire two concurrent updates to the same item
+			await Promise.all([
+				provider.updateTaskHistory(createHistoryItem({ id: "race-item", task: "Original", tokensIn: 111 }), {
+					broadcast: false,
+				}),
+				provider.updateTaskHistory(createHistoryItem({ id: "race-item", task: "Original", tokensIn: 222 }), {
+					broadcast: false,
+				}),
+			])
+
+			const history = (provider as any).contextProxy.getGlobalState("taskHistory") as HistoryItem[]
+			const item = history.find((h: HistoryItem) => h.id === "race-item")
+			expect(item).toBeDefined()
+			// The second write (tokensIn: 222) should be the last one since writes are serialized
+			expect(item!.tokensIn).toBe(222)
+		})
+	})
 })