Skip to content

Commit bf09458

Browse files
committed
fix(task): recover stale delegated children after restart
1 parent fe0956b commit bf09458

2 files changed

Lines changed: 116 additions & 12 deletions

File tree

src/core/task-persistence/TaskHistoryStore.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,11 +355,23 @@ export class TaskHistoryStore {
355355
* - Parent `delegated` with no `awaitingChildId` → parent → `active` (invalid state)
356356
* - Parent `delegated`, child not found → parent → `active` (orphaned delegation)
357357
* - Parent `delegated`, child `completed` → parent → `active` (interrupted handoff)
358+
* - Parent `delegated`, child `active` → child → `interrupted`, parent → `active`
358359
*
359-
* A parent awaiting an `active`, `interrupted`, or `delegated` child is left as-is — the child is resumable.
360+
* A parent awaiting an `interrupted` or `delegated` child is left as-is — the child is
361+
* resumable. An `active` child is treated as orphaned during startup recovery because
362+
* no live task session exists to own it.
360363
*/
361364
private async reconcileDelegationState(): Promise<void> {
362365
return this.withLock(async () => {
366+
// Only statuses loaded from persistence represent sessions that could have
367+
// been orphaned by a crash. A delegated parent repaired to active earlier in
368+
// this pass remains resumable and must not be mistaken for a second orphaned
369+
// child in a delegation chain.
370+
const persistedActiveIds = new Set(
371+
Array.from(this.cache.values())
372+
.filter((item) => item.status === "active")
373+
.map((item) => item.id),
374+
)
363375
let repairsInThisPass: number
364376
do {
365377
repairsInThisPass = 0
@@ -400,6 +412,25 @@ export class TaskHistoryStore {
400412
`[TaskHistoryStore] Reconciled orphaned delegation: task ${item.id} → active (child ${item.awaitingChildId} not found)`,
401413
)
402414
repairsInThisPass++
415+
} else if (child.status === "active" && persistedActiveIds.has(child.id)) {
416+
// An active child persisted across startup cannot have a live task session
417+
// behind it. Mark it interrupted before releasing the parent's delegation
418+
// link so the normal resume/re-delegate flow can take over. This is an
419+
// administrative recovery, not a runtime delegation transition.
420+
await this.upsertCore({ ...child, status: "interrupted" }, { skipTransitionCheck: true })
421+
await this.upsertCore(
422+
{
423+
...item,
424+
status: "active",
425+
awaitingChildId: undefined,
426+
delegatedToId: undefined,
427+
},
428+
{ skipTransitionCheck: true },
429+
)
430+
console.warn(
431+
`[TaskHistoryStore] Reconciled orphaned active child: child ${child.id} → interrupted, task ${item.id} → active`,
432+
)
433+
repairsInThisPass++
403434
} else if (child.status === "completed") {
404435
await this.upsertCore(
405436
{
@@ -418,7 +449,7 @@ export class TaskHistoryStore {
418449
)
419450
repairsInThisPass++
420451
}
421-
// child.status === "active", "interrupted", or "delegated" → leave as-is this pass
452+
// child.status === "interrupted" or "delegated" → leave as-is this pass
422453
}
423454
} while (repairsInThisPass > 0)
424455
})

src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts

Lines changed: 83 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -185,16 +185,59 @@ describe("TaskHistoryStore reconcileDelegationState", () => {
185185
expect(repaired?.completionResultSummary).toBe("Task completed (recovered after interruption)")
186186
})
187187

188-
it("leaves delegated parent alone when child is still active", async () => {
189-
const child = makeItem({ id: "child-4", status: "active" })
190-
const parent = makeItem({ id: "parent-4", status: "delegated", awaitingChildId: "child-4" })
188+
it("repairs a delegated parent with an active orphaned child", async () => {
189+
const child = makeItem({
190+
id: "child-4",
191+
status: "active",
192+
parentTaskId: "parent-4",
193+
rootTaskId: "parent-4",
194+
childIds: ["grandchild-4"],
195+
})
196+
const parent = makeItem({
197+
id: "parent-4",
198+
status: "delegated",
199+
awaitingChildId: "child-4",
200+
delegatedToId: "child-4",
201+
childIds: ["child-4"],
202+
})
191203
await seedItems([parent, child])
192204

193205
await store.initialize()
194206

195-
const unchanged = store.get("parent-4")
196-
expect(unchanged?.status).toBe("delegated")
197-
expect(unchanged?.awaitingChildId).toBe("child-4")
207+
const repairedParent = store.get("parent-4")
208+
const repairedChild = store.get("child-4")
209+
expect(repairedChild).toMatchObject({
210+
id: "child-4",
211+
status: "interrupted",
212+
parentTaskId: "parent-4",
213+
rootTaskId: "parent-4",
214+
childIds: ["grandchild-4"],
215+
})
216+
expect(repairedParent).toMatchObject({
217+
id: "parent-4",
218+
status: "active",
219+
childIds: ["child-4"],
220+
})
221+
expect(repairedParent?.awaitingChildId).toBeUndefined()
222+
expect(repairedParent?.delegatedToId).toBeUndefined()
223+
224+
const tasksDir = path.join(tmpDir, "tasks")
225+
const persistedChild = JSON.parse(
226+
await fs.readFile(path.join(tasksDir, "child-4", "history_item.json"), "utf8"),
227+
) as HistoryItem
228+
const persistedParent = JSON.parse(
229+
await fs.readFile(path.join(tasksDir, "parent-4", "history_item.json"), "utf8"),
230+
) as HistoryItem
231+
expect(persistedChild).toMatchObject({
232+
id: "child-4",
233+
status: "interrupted",
234+
parentTaskId: "parent-4",
235+
rootTaskId: "parent-4",
236+
childIds: ["grandchild-4"],
237+
})
238+
expect(persistedParent).toMatchObject({ id: "parent-4", status: "active" })
239+
expect(persistedParent.awaitingChildId).toBeUndefined()
240+
expect(persistedParent.delegatedToId).toBeUndefined()
198241
})
199242

200243
it("repairs invalid delegation: delegated parent with no awaitingChildId → active (clears delegatedToId and awaitingChildId)", async () => {
@@ -239,9 +282,9 @@ describe("TaskHistoryStore reconcileDelegationState", () => {
239282
expect(store.get("parent-b")?.status).toBe("active")
240283
})
241284

242-
it("handles chained delegation (A→B→C): repairs B first, then A sees B as active and is left delegated", async () => {
285+
it("handles chained delegation (A→B→C) until all orphaned links converge", async () => {
243286
// C doesn't exist (orphaned). B is delegated waiting for C → repaired to active.
244-
// A is delegated waiting for B → left delegated (B is now active, resumable by user).
287+
// A then sees B as an orphaned active child and is repaired as well.
245288
const parentA = makeItem({ id: "parent-a-chain", status: "delegated", awaitingChildId: "parent-b-chain" })
246289
const parentB = makeItem({
247290
id: "parent-b-chain",
@@ -254,9 +297,39 @@ describe("TaskHistoryStore reconcileDelegationState", () => {
254297

255298
// B is repaired: its child (C) was missing
256299
expect(store.get("parent-b-chain")?.status).toBe("active")
257-
// A stays delegated: its child (B) is now active, which is a valid state
300+
// A stays delegated: B was repaired from delegated to active and remains
301+
// resumable rather than being mistaken for an active orphan from disk.
258302
expect(store.get("parent-a-chain")?.status).toBe("delegated")
259303
expect(store.get("parent-a-chain")?.awaitingChildId).toBe("parent-b-chain")
304+
expect(store.get("parent-b-chain")?.status).toBe("active")
305+
expect(store.get("parent-b-chain")?.awaitingChildId).toBeUndefined()
306+
})
307+
308+
it("is idempotent when recovering an active child", async () => {
309+
const child = makeItem({ id: "child-active-idempotent", status: "active" })
310+
const parent = makeItem({
311+
id: "parent-active-idempotent",
312+
status: "delegated",
313+
awaitingChildId: child.id,
314+
delegatedToId: child.id,
315+
})
316+
await seedItems([parent, child])
317+
318+
await store.initialize()
319+
const afterFirstParent = { ...store.get(parent.id) }
320+
const afterFirstChild = { ...store.get(child.id) }
321+
322+
store.dispose()
323+
const store2 = new TaskHistoryStore(tmpDir)
324+
await store2.initialize()
325+
const afterSecondParent = { ...store2.get(parent.id) }
326+
const afterSecondChild = { ...store2.get(child.id) }
327+
store2.dispose()
328+
329+
expect(afterFirstParent).toMatchObject({ status: "active" })
330+
expect(afterSecondParent).toEqual(afterFirstParent)
331+
expect(afterFirstChild).toMatchObject({ status: "interrupted" })
332+
expect(afterSecondChild).toEqual(afterFirstChild)
260333
})
261334

262335
it("is idempotent: running initialize twice produces the same result", async () => {
@@ -407,7 +480,7 @@ describe("TaskHistoryStore upsert transition guard", () => {
407480

408481
it("rejects delegated → completed transition", async () => {
409482
// Must include a live active child so reconciliation doesn't repair the parent to active
410-
const child = makeItem({ id: "child-guard-2", status: "active" })
483+
const child = makeItem({ id: "child-guard-2", status: "interrupted" })
411484
const item = makeItem({ id: "task-guard-2", status: "delegated", awaitingChildId: "child-guard-2" })
412485
await seedItems([child, item])
413486
store.dispose()

0 commit comments

Comments
 (0)