[Story 2.1] Serialize delegateParentAndOpenChild with withLock #364

Part of #357 (Epic 2: Task Lifecycle Fixes).

Depends on: Nothing — can start immediately.

Fixes #208 (task logs disappear on Stop — root cause is the withLock race leaving an empty snapshot as the winning write).

Context

The delegation flow in ClineProvider.ts (line 3127) calls getTaskWithId (line 3228) then updateTaskHistory (line 3237) with no atomicity between the read and write. A concurrent caller can read stale state between those two operations — specifically the awaitingChildId and delegatedToId fields get lost if two delegations race. This is one of the three documented races currently patched with step ordering.

Note: taskHistoryStore.upsert() is already locked internally, but the read in getTaskWithId happens before the lock is acquired. The fix is to expand the lock boundary to include the read.

Developer Notes

In src/core/task-persistence/TaskHistoryStore.ts:

• Add a public atomicReadAndUpdate<T>(fn: () => Promise<T>): Promise<T> method that delegates to withLock. This keeps the raw lock private while exposing a purpose-built API for read-then-write sequences. Add /** @internal Do not hold this lock for longer than a single read-modify-write cycle. */ JSDoc.

In src/core/webview/ClineProvider.ts:

• Wrap steps 5–6 of delegateParentAndOpenChild (lines 3227–3243: getTaskWithId + updateTaskHistory for parent delegation metadata) in this.taskHistoryStore.atomicReadAndUpdate(async () => { ... }).
• child.start() (line 3247) must be called outside the lock — it eventually calls saveClineMessages → upsert() which tries to acquire the same lock, causing a deadlock.

Files: src/core/task-persistence/TaskHistoryStore.ts, src/core/webview/ClineProvider.ts

Tests (src/__tests__/delegation-concurrent.spec.ts):

• Mock TaskHistoryStore with real withLock but in-memory storage.
• Fire two concurrent delegateParentAndOpenChild calls; assert HistoryItem.status is never inconsistent.
• Use controlled promises inside the lock to deterministically interleave callers.
• Negative deadlock test — proves child.start() must stay outside the lock. Use real timers with a 100ms timeout (a deadlock is instant; 100ms is generous for slow CI runners):

it('deadlocks if child.start() is called inside withLock', async () => {
  const result = await Promise.race([
    store.withLock(async () => {
      await store.upsert(item) // re-acquires same lock → deadlock
      return 'completed'
    }),
    new Promise<string>(resolve => setTimeout(() => resolve('deadlocked'), 100)),
  ])
  expect(result).toBe('deadlocked')
})

Acceptance Criteria

• All existing delegation tests (history-resume-delegation.spec.ts, newTaskTool.spec.ts) pass unchanged.
• child.start() is always called outside the lock.
• Deadlock negative test passes and proves the constraint.