Skip to content

Commit fb833e7

Browse files
committed
feat(task-lifecycle): transition guard and startup delegation reconciliation
1 parent 0f1054e commit fb833e7

6 files changed

Lines changed: 409 additions & 21 deletions

File tree

src/__tests__/helpers/provider-stub.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export function makeProviderStub<T extends object>(stub: T): T {
1212
const proto = ClineProvider.prototype as any
1313
s.delegationTransitionLocks ??= new Map()
1414
s.cancelledDelegationChildIds ??= new Set()
15+
s.log ??= vi.fn()
1516
s.runDelegationTransition = proto.runDelegationTransition.bind(s)
1617
return s
1718
}

src/__tests__/history-resume-delegation.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ vi.mock("../core/task-persistence", () => ({
3333
readApiMessages: vi.fn().mockResolvedValue([]),
3434
saveApiMessages: vi.fn().mockResolvedValue(undefined),
3535
saveTaskMessages: vi.fn().mockResolvedValue(undefined),
36+
assertValidTransition: vi.fn(),
3637
}))
3738

3839
import { ClineProvider } from "../core/webview/ClineProvider"

src/core/task-persistence/TaskHistoryStore.ts

Lines changed: 123 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,28 @@ import { GlobalFileNames } from "../../shared/globalFileNames"
88
import { safeWriteJson } from "../../utils/safeWriteJson"
99
import { getStorageBasePath } from "../../utils/storage"
1010

11+
/** Valid status values for a task's HistoryItem. */
12+
export type HistoryItemStatus = "active" | "delegated" | "completed"
13+
14+
const VALID_TRANSITIONS: Record<HistoryItemStatus, HistoryItemStatus[]> = {
15+
active: ["delegated", "completed"],
16+
delegated: ["active"],
17+
completed: [],
18+
}
19+
20+
/**
21+
* Asserts that a task status transition is valid, throwing if not.
22+
*
23+
* @throws {Error} When the transition is not allowed by the state machine.
24+
*/
25+
export function assertValidTransition(from: HistoryItemStatus | undefined, to: HistoryItemStatus): void {
26+
const fromStatus: HistoryItemStatus = from ?? "active"
27+
const validTargets = VALID_TRANSITIONS[fromStatus]
28+
if (!validTargets.includes(to)) {
29+
throw new Error(`Invalid task status transition: ${fromStatus}${to}`)
30+
}
31+
}
32+
1133
/**
1234
* Index file format for fast startup reads.
1335
*/
@@ -88,10 +110,13 @@ export class TaskHistoryStore {
88110
// 2. Reconcile cache against actual task directories on disk
89111
await this.reconcile()
90112

91-
// 3. Start fs.watch for cross-instance reactivity
113+
// 3. Repair delegation inconsistencies left by a previous crash
114+
await this.reconcileDelegationState()
115+
116+
// 4. Start fs.watch for cross-instance reactivity
92117
this.startWatcher()
93118

94-
// 4. Start periodic reconciliation as a defensive fallback
119+
// 5. Start periodic reconciliation as a defensive fallback
95120
this.startPeriodicReconciliation()
96121
} finally {
97122
// Mark initialization as complete so callers awaiting `initialized` can proceed
@@ -158,30 +183,35 @@ export class TaskHistoryStore {
158183
* updates the in-memory Map, and schedules a debounced index write.
159184
*/
160185
async upsert(item: HistoryItem): Promise<HistoryItem[]> {
161-
return this.withLock(async () => {
162-
const existing = this.cache.get(item.id)
186+
return this.withLock(() => this.upsertCore(item))
187+
}
163188

164-
// Merge: preserve existing metadata unless explicitly overwritten
165-
const merged = existing ? { ...existing, ...item } : item
189+
/**
190+
* Core upsert logic — must only be called from within `withLock`.
191+
*/
192+
private async upsertCore(item: HistoryItem): Promise<HistoryItem[]> {
193+
const existing = this.cache.get(item.id)
166194

167-
// Write per-task file (source of truth)
168-
await this.writeTaskFile(merged)
195+
// Merge: preserve existing metadata unless explicitly overwritten
196+
const merged = existing ? { ...existing, ...item } : item
169197

170-
// Update in-memory cache
171-
this.cache.set(merged.id, merged)
198+
// Write per-task file (source of truth)
199+
await this.writeTaskFile(merged)
172200

173-
// Schedule debounced index write
174-
this.scheduleIndexWrite()
201+
// Update in-memory cache
202+
this.cache.set(merged.id, merged)
175203

176-
const all = this.getAll()
204+
// Schedule debounced index write
205+
this.scheduleIndexWrite()
177206

178-
// Call onWrite callback inside the lock for serialized write-through
179-
if (this.onWrite) {
180-
await this.onWrite(all)
181-
}
207+
const all = this.getAll()
182208

183-
return all
184-
})
209+
// Call onWrite callback inside the lock for serialized write-through
210+
if (this.onWrite) {
211+
await this.onWrite(all)
212+
}
213+
214+
return all
185215
}
186216

187217
/**
@@ -289,6 +319,80 @@ export class TaskHistoryStore {
289319
})
290320
}
291321

322+
/**
323+
* Repair delegation inconsistencies left by a crash mid-transition.
324+
*
325+
* Called once from `initialize()` after `reconcile()`. Runs inside `withLock` to
326+
* prevent interleaving with watcher-triggered reconcile() calls. Iterates until
327+
* convergence so that chained delegations (A→B→C) are fully resolved in one startup.
328+
*
329+
* Must NOT be called from within `withLock` — each upsertCore call holds the lock
330+
* for the duration of a single write, and the outer lock wraps the whole pass.
331+
*
332+
* Cases repaired per pass:
333+
* - Parent `delegated` with no `awaitingChildId` → parent → `active` (invalid state)
334+
* - Parent `delegated`, child not found → parent → `active` (orphaned delegation)
335+
* - Parent `delegated`, child `completed` → parent → `active` (interrupted handoff)
336+
*
337+
* A parent awaiting an `active` child is left as-is — the child is resumable.
338+
*/
339+
private async reconcileDelegationState(): Promise<void> {
340+
return this.withLock(async () => {
341+
let repairsInThisPass: number
342+
do {
343+
repairsInThisPass = 0
344+
// Rebuild the lookup map each pass so repairs from the previous pass
345+
// are visible when evaluating chained delegations.
346+
const byId = new Map(this.getAll().map((i) => [i.id, i]))
347+
348+
for (const [, item] of byId) {
349+
if (item.status !== "delegated") {
350+
continue
351+
}
352+
353+
if (!item.awaitingChildId) {
354+
await this.upsertCore({ ...item, status: "active", delegatedToId: undefined })
355+
console.warn(
356+
`[TaskHistoryStore] Reconciled invalid delegation: task ${item.id} → active (no awaitingChildId)`,
357+
)
358+
repairsInThisPass++
359+
continue
360+
}
361+
362+
const child = byId.get(item.awaitingChildId)
363+
364+
if (!child) {
365+
await this.upsertCore({
366+
...item,
367+
status: "active",
368+
awaitingChildId: undefined,
369+
delegatedToId: undefined,
370+
})
371+
console.warn(
372+
`[TaskHistoryStore] Reconciled orphaned delegation: task ${item.id} → active (child ${item.awaitingChildId} not found)`,
373+
)
374+
repairsInThisPass++
375+
} else if (child.status === "completed") {
376+
await this.upsertCore({
377+
...item,
378+
status: "active",
379+
awaitingChildId: undefined,
380+
delegatedToId: undefined,
381+
completedByChildId: child.id,
382+
completionResultSummary:
383+
child.completionResultSummary ?? "Task completed (recovered after interruption)",
384+
})
385+
console.warn(
386+
`[TaskHistoryStore] Reconciled interrupted handoff: task ${item.id} → active (child ${item.awaitingChildId} already completed)`,
387+
)
388+
repairsInThisPass++
389+
}
390+
// child.status === "active" or "delegated" → leave as-is this pass
391+
}
392+
} while (repairsInThisPass > 0)
393+
})
394+
}
395+
292396
// ────────────────────────────── Cache invalidation ──────────────────────────────
293397

294398
/**

0 commit comments

Comments
 (0)