Skip to content

Commit 593ff6e

Browse files
edelaunaedelauna
committed
fix: preserve explicit-resume ownership across restart
1 parent be2c3eb commit 593ff6e

8 files changed

Lines changed: 142 additions & 11 deletions

File tree

‎docs/architecture/task-lifecycle-persisted-ownership-model.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ All blocks use one vocabulary, established here so that dependent workstreams (P
2323
- **Lock-time ownership check.** A required new check on the parent record, specified under LIFE-BLK-P1-001. It runs in the parent file's merge. The gap report's dependency column for `LIFE-GAP-001` calls it the disk-authoritative ownership guard.
2424
- **Lock-time generation check.** A required new check on the child record, specified under LIFE-BLK-P1-012. It runs in the child file's merge. It is a separate check because the generation lives in the child file, and each file has its own lock. `mergeHistoryDelta` sees only one disk record per call, so one check cannot compare both files under their locks.
2525
- **Cached updater input.** Both `TaskHistoryStore.atomicReadAndUpdate` and `atomicUpdatePair` pass the updater the cached item, not the disk record. The updater result becomes a delta against the cache, and `mergeHistoryDelta` merges that delta into disk. So a reducer check inside an updater validates the cache, not disk.
26-
- **Lifecycle-owned fields.** The fields whose authoritative writer is the lifecycle layer: `status`, `rootTaskId`, `parentTaskId`, `delegatedToId`, `childIds`, `awaitingChildId`, `completedByChildId`, `completionResultSummary`, and `pendingAction`. The block-level ownership table is under LIFE-BLK-P1-002 below.
26+
- **Lifecycle-owned fields.** The fields whose authoritative writer is the lifecycle layer: `status`, `rootTaskId`, `parentTaskId`, `delegatedToId`, `childIds`, `awaitingChildId`, `completedByChildId`, `completionResultSummary`, `explicitResume`, and `pendingAction`. The block-level ownership table is under LIFE-BLK-P1-002 below.
2727
- **Attempt generation.** A monotonically increasing per-task token distinguishing one execution attempt of the same task ID from the next after interruption and resume. No such persisted token exists today; `PendingTaskAction.actionId` is pending-approval identity, not attempt identity (see LIFE-BLK-P1-012).
2828

2929
## LIFE-BLK-P1-001 — lock-time ownership check (GAP-001)
@@ -71,6 +71,7 @@ Inside each write, `writeTaskFile` runs `mergeHistoryDelta` under that file's ad
7171
| `delegatedToId`, `awaitingChildId` | `delegateTaskToChild`, `completeDelegatedChild`, `abandonDelegatedChild`, and the repair paths | No |
7272
| `childIds` | The same reducers. The merge unions concurrent additions. | No |
7373
| `completedByChildId`, `completionResultSummary` | `completeDelegatedChild` and reconciliation repair. Standalone completion writes `status` only. | No |
74+
| `explicitResume` | `resumeInterruptedTask` sets it on the explicit resume and no writer clears it. Startup reconciliation reads it in `reconcileDelegationStateCore`; the reducer model strips it because reconciliation sits outside the persisted reducer graph. | No |
7475
| `pendingAction` | `setPendingTaskAction` (`ClineProvider.ts:740`), the compare-and-clear settlement (`:752-757`), and the delegation and completion paths | No |
7576
| `id`, `number`, `task` | Creation | Creation only |
7677
| `ts` | Metadata. `taskMetadata` sets it from the last relevant message on every save (`taskMetadata.ts:69-73`). | Yes |

‎packages/types/src/history.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export const historyItemSchema = z.object({
4848
awaitingChildId: z.string().optional(), // Child currently awaited (set when delegated)
4949
completedByChildId: z.string().optional(), // Child that completed and resumed this parent
5050
completionResultSummary: z.string().optional(), // Summary from completed child
51+
explicitResume: z.boolean().optional(), // Set when the explicit user-resume path reactivated this task; startup recovery keeps the parent's awaiting link
5152
pendingAction: pendingTaskActionSchema.optional(),
5253
})
5354

‎scripts/check-task-lifecycle.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,17 @@ function invariantViolations(state: ModelState): string[] {
341341
}
342342

343343
function canonical(state: ModelState): string {
344-
return JSON.stringify(taskIds.map((id) => state[id] ?? null))
344+
return JSON.stringify(
345+
taskIds.map((id) => {
346+
const task = state[id]
347+
if (!task) return null
348+
// explicitResume only steers store-level startup recovery after a
349+
// restart, which this reducer model does not represent. No modeled
350+
// transition reads it, so it is not part of the canonical state.
351+
const { explicitResume: _explicitResume, ...modeled } = task
352+
return modeled
353+
}),
354+
)
345355
}
346356

347357
function formatCounterexample(message: string, trace: TraceStep[]): string {

‎src/core/task-persistence/TaskHistoryStore.ts‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -454,11 +454,14 @@ export class TaskHistoryStore {
454454
* - Parent `delegated` with no `awaitingChildId` → parent → `active` (invalid state)
455455
* - Parent `delegated`, child not found → parent → `active` (orphaned delegation)
456456
* - Parent `delegated`, child `completed` → parent → `active` (interrupted handoff)
457-
* - Parent `delegated`, child `active` → child → `interrupted`, parent → `active`
457+
* - Parent `delegated`, child `active` without `explicitResume` → child → `interrupted`, parent → `active`
458+
* - Parent `delegated`, child `active` with `explicitResume` → child → `interrupted`, parent stays `delegated`
458459
*
459460
* A parent awaiting an `interrupted` or `delegated` child is left as-is — the child is
460461
* resumable. An `active` child is treated as orphaned during startup recovery because
461-
* no live task session exists to own it.
462+
* no live task session exists to own it. An `active` child marked `explicitResume` was
463+
* reactivated through the user-resume path, so the restart only ends the lost session:
464+
* the parent keeps its awaiting link and the next linked resume succeeds.
462465
*/
463466
private async reconcileDelegationState(persistedActiveIds: ReadonlySet<string>): Promise<void> {
464467
return this.withLock(() => this.reconcileDelegationStateCore(persistedActiveIds))
@@ -524,10 +527,20 @@ export class TaskHistoryStore {
524527
// behind it. Mark it interrupted before releasing the parent's delegation
525528
// link so the normal resume/re-delegate flow can take over. This is an
526529
// administrative recovery, not a runtime delegation transition.
527-
await this.repairActiveDelegation(item, child)
528-
console.warn(
529-
`[TaskHistoryStore] Reconciled orphaned active child: child ${child.id} → interrupted, task ${item.id} → active`,
530-
)
530+
// An explicitly resumed child keeps the parent's awaiting link: the user
531+
// approved that pairing, so the restart only ends the lost session and
532+
// the next linked resume must still succeed.
533+
if (child.explicitResume) {
534+
await this.upsertCore({ ...child, status: "interrupted" }, { skipTransitionCheck: true })
535+
console.warn(
536+
`[TaskHistoryStore] Preserved explicit-resume ownership across restart: child ${child.id} → interrupted, task ${item.id} still awaits it`,
537+
)
538+
} else {
539+
await this.repairActiveDelegation(item, child)
540+
console.warn(
541+
`[TaskHistoryStore] Reconciled orphaned active child: child ${child.id} → interrupted, task ${item.id} → active`,
542+
)
543+
}
531544
repairsInThisPass++
532545
} else if (child.status === "completed") {
533546
await this.upsertCore(

‎src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,54 @@ describe("TaskHistoryStore real cross-host locking", () => {
263263
}
264264
})
265265

266+
it("preserves explicit-resume ownership across a restart", async () => {
267+
const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "resume-restart-ownership-"))
268+
const storeA = new TaskHistoryStore(storagePath)
269+
const parent = {
270+
...item("parent"),
271+
status: "delegated" as const,
272+
awaitingChildId: "child",
273+
delegatedToId: "child",
274+
childIds: ["child"],
275+
}
276+
const child = { ...item("child"), status: "interrupted" as const, parentTaskId: "parent", rootTaskId: "parent" }
277+
278+
try {
279+
await storeA.initialize()
280+
await storeA.upsert(parent)
281+
await storeA.upsert(child)
282+
283+
// The user accepts the resume prompt; the activated record persists.
284+
await storeA.resumeInterruptedTask("child", "parent")
285+
expect(storeA.get("child")).toMatchObject({ status: "active", explicitResume: true })
286+
} finally {
287+
storeA.dispose()
288+
}
289+
290+
// A restart replays startup recovery on the same storage. The resumed
291+
// child's session is gone, but the user-approved awaiting link survives so
292+
// the next linked resume still succeeds.
293+
const storeB = new TaskHistoryStore(storagePath)
294+
try {
295+
await storeB.initialize()
296+
expect(storeB.get("child")).toMatchObject({ status: "interrupted", explicitResume: true })
297+
expect(storeB.get("parent")).toMatchObject({
298+
status: "delegated",
299+
awaitingChildId: "child",
300+
delegatedToId: "child",
301+
})
302+
303+
await expect(storeB.resumeInterruptedTask("child", "parent")).resolves.toMatchObject({
304+
id: "child",
305+
status: "active",
306+
explicitResume: true,
307+
})
308+
} finally {
309+
storeB.dispose()
310+
await fs.rm(storagePath, { recursive: true, force: true })
311+
}
312+
})
313+
266314
it("reads the awaited child's disk record at lock time even when the cache is stale", async () => {
267315
const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "redelegation-stale-cache-"))
268316
const storeA = new TaskHistoryStore(storagePath)

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,50 @@ describe("TaskHistoryStore reconcileDelegationState", () => {
275275
expect(persistedParent.delegatedToId).toBeUndefined()
276276
})
277277

278+
it("preserves explicit-resume ownership across restart", async () => {
279+
const child = makeItem({
280+
id: "child-4-resumed",
281+
status: "active",
282+
parentTaskId: "parent-4-resumed",
283+
rootTaskId: "parent-4-resumed",
284+
explicitResume: true,
285+
})
286+
const parent = makeItem({
287+
id: "parent-4-resumed",
288+
status: "delegated",
289+
awaitingChildId: "child-4-resumed",
290+
delegatedToId: "child-4-resumed",
291+
childIds: ["child-4-resumed"],
292+
})
293+
await seedItems([parent, child])
294+
295+
await store.initialize()
296+
297+
// The lost session ends, but the user-approved awaiting link survives so
298+
// the next linked resume succeeds.
299+
expect(store.get("child-4-resumed")).toMatchObject({
300+
id: "child-4-resumed",
301+
status: "interrupted",
302+
explicitResume: true,
303+
})
304+
expect(store.get("parent-4-resumed")).toMatchObject({
305+
id: "parent-4-resumed",
306+
status: "delegated",
307+
awaitingChildId: "child-4-resumed",
308+
delegatedToId: "child-4-resumed",
309+
})
310+
311+
const tasksDir = path.join(tmpDir, "tasks")
312+
const persistedChild = JSON.parse(
313+
await fs.readFile(path.join(tasksDir, "child-4-resumed", "history_item.json"), "utf8"),
314+
) as HistoryItem
315+
const persistedParent = JSON.parse(
316+
await fs.readFile(path.join(tasksDir, "parent-4-resumed", "history_item.json"), "utf8"),
317+
) as HistoryItem
318+
expect(persistedChild).toMatchObject({ status: "interrupted", explicitResume: true })
319+
expect(persistedParent).toMatchObject({ status: "delegated", awaitingChildId: "child-4-resumed" })
320+
})
321+
278322
it("repairs a delegated child with an omitted status as implicit active", async () => {
279323
const child = makeItem({
280324
id: "child-implicit-active",

‎src/core/task-persistence/__tests__/taskLifecycle.spec.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,14 @@ describe("task lifecycle transitions", () => {
7979
expect(() => resumeInterruptedTask(item("child"))).toThrow(LifecycleTransitionError)
8080
})
8181

82+
it("marks an explicit resume so startup recovery can preserve ownership", () => {
83+
const child = item("child", { status: "interrupted", parentTaskId: "parent" })
84+
const rootTask = item("root-task", { status: "interrupted" })
85+
86+
expect(resumeInterruptedTask(child, "parent")).toMatchObject({ status: "active", explicitResume: true })
87+
expect(resumeInterruptedTask(rootTask)).toMatchObject({ status: "active", explicitResume: true })
88+
})
89+
8290
it("resumes an interrupted child only while its parent still awaits it", () => {
8391
const parent = item("parent", {
8492
status: "delegated",
@@ -88,7 +96,11 @@ describe("task lifecycle transitions", () => {
8896
})
8997
const child = item("child", { status: "interrupted", parentTaskId: "parent" })
9098

91-
expect(resumeInterruptedChild(parent, child)).toMatchObject({ status: "active", parentTaskId: "parent" })
99+
expect(resumeInterruptedChild(parent, child)).toMatchObject({
100+
status: "active",
101+
parentTaskId: "parent",
102+
explicitResume: true,
103+
})
92104
})
93105

94106
it("rejects the parent-locked resume after the parent re-delegates elsewhere", () => {

‎src/core/task-persistence/taskLifecycle.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ export function assertValidTransition(from: HistoryItemStatus | undefined, to: H
2727
/**
2828
* Reactivates a task only through the explicit user-resume path. This is kept
2929
* outside VALID_TASK_STATUS_TRANSITIONS so generic stale writes cannot revive
30-
* an interrupted task.
30+
* an interrupted task. The persisted explicitResume marker lets startup
31+
* recovery recognize a resumed child after a restart: the session is gone, but
32+
* the parent keeps its awaiting link so the next linked resume succeeds.
3133
*/
3234
export function resumeInterruptedTask(task: HistoryItem, expectedParentTaskId?: string): HistoryItem {
3335
if (task.status !== "interrupted") {
@@ -36,7 +38,7 @@ export function resumeInterruptedTask(task: HistoryItem, expectedParentTaskId?:
3638
if (task.parentTaskId !== expectedParentTaskId) {
3739
throw new LifecycleTransitionError(`Cannot resume task ${task.id}: parent linkage changed`)
3840
}
39-
return { ...task, status: "active" }
41+
return { ...task, status: "active", explicitResume: true }
4042
}
4143

4244
/**

0 commit comments

Comments
 (0)