feat(task-lifecycle): task status transition guard and startup delegation reconciliation

[edelauna]

Related GitHub Issue

Closes: #366 (part of #357 — Epic 2: Task Lifecycle Fixes)

Description

Implements Story 2.3: a status transition guard enforced at the store write boundary, plus a startup reconciliation loop to detect and repair HistoryItem status corruption.

Transition guard (assertValidTransition)

Adds assertValidTransition(from, to) in TaskHistoryStore.ts that enforces the valid state machine:

• active → delegated | completed
• delegated → active
• completed → [] (terminal)

The guard is enforced at two levels:

1. upsertCore write boundary — every single-item write through upsert or atomicReadAndUpdate validates the transition before touching disk. Legacy items whose status is undefined are normalised to "active" before comparison so an idempotent re-write doesn't throw.
2. atomicUpdatePair write boundary — the two-item atomic write (child active → completed, parent delegated/active → active) validates both updater outputs before either file is written, so an invalid transition in either updater aborts the entire pair atomically.

Explicit assertValidTransition calls are also placed inside the updater lambdas passed to atomicUpdatePair in reopenParentFromDelegation and inside the atomicReadAndUpdate updater in delegateParentAndOpenChild, cancelTask, and removeClineFromStack — providing a clear guard at the call site as well as at the store boundary.

Partial-write resilience

reopenParentFromDelegation writes the child record first (child → completed) and then the parent (parent → active) within atomicUpdatePair. If the extension crashes after the child write but before the parent write, startup reconciliation detects the still-delegated parent with a completed child and repairs it. To ensure the repair copies the real result rather than a generic fallback, the child updater now also persists completionResultSummary onto the child record.

Startup reconciliation (reconcileDelegationState)

Called from TaskHistoryStore.initialize() after reconcile(). Repairs three crash-induced inconsistencies:

• Parent delegated with no awaitingChildId → active
• Parent delegated, child not found (orphaned) → active
• Parent delegated, child already completed (interrupted handoff) → active, copying the child's completionResultSummary

Implementation details:

• Runs inside withLock via the private upsertCore helper (avoids re-entrant deadlock)
• Convergence loop (do { } while (repairsInThisPass > 0)) so chained delegations (A→B→C) are resolved in one startup
• O(1) child lookup via pre-built Map
• All three repair paths clear both awaitingChildId and delegatedToId
• Uses skipTransitionCheck: true on repair writes since these are administrative corrections, not runtime state-machine transitions

Test Procedure

pnpm --filter zoo-code test core/task-persistence/__tests__/TaskHistoryStore
pnpm --filter zoo-code test core/task-persistence/__tests__/TaskHistoryStore.reconciliation
pnpm --filter zoo-code test __tests__/history-resume-delegation
pnpm --filter zoo-code test __tests__/provider-delegation
pnpm --filter zoo-code test __tests__/nested-delegation-resume

93 tests across the five affected suites pass. TaskHistoryStore.reconciliation.spec.ts covers the transition guard (valid/invalid transitions, legacy undefined status, undefined → delegated), all reconciliation scenarios (orphaned delegation, interrupted handoff, fallback summary, active-child left alone, multiple parents, chained delegation, idempotency, logging), and the atomicReadAndUpdate store-level guard. TaskHistoryStore.spec.ts covers the atomicUpdatePair store-level guard (invalid transition rejected even without an explicit updater assertion).

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Documentation Impact: No documentation updates required — internal persistence layer change with no user-visible behaviour change in the happy path.
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Summary by CodeRabbit

• New Features
  • Added an API method to fetch a task history item by task ID.
  • Delegation-related events are now surfaced from the API layer for consistent handling.
• Bug Fixes
  • Improved delegation/resume behavior for interrupted and nested handoffs, with more reliable parent/child history updates.
  • Added stronger startup/migration repair to correct inconsistent delegation state.
  • Enforced valid task-history status transitions during persistence and hardened overlapping delegation rollback behavior.
• Tests
  • Added an end-to-end test covering delegated child completion persisting parent/child history state.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds task-status transition validation, repairs delegated task state during startup, exposes task-history lookup through the API, and updates delegation flows, tests, and mocks to align with the new persistence behavior.

Changes

Task lifecycle fixes

Layer / File(s)	Summary
Status transition contract src/core/task-persistence/TaskHistoryStore.ts, src/core/task-persistence/index.ts, src/core/task-persistence/__tests__/TaskHistoryStore.spec.ts, src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts	Adds HistoryItemStatus and assertValidTransition, re-exports the helper, and adds direct and store-level coverage for valid and invalid status transitions.
Startup reconciliation and persistence src/core/task-persistence/TaskHistoryStore.ts, src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts	Runs delegation reconciliation during initialization and migration, routes writes through the guarded core path, validates paired updates before disk writes, and adds repair and rejection coverage.
Delegation guards in ClineProvider src/core/webview/ClineProvider.ts, src/__tests__/history-resume-delegation.spec.ts, src/__tests__/nested-delegation-resume.spec.ts, src/__tests__/provider-delegation.spec.ts, src/__tests__/removeClineFromStack-delegation.spec.ts	Adds transition checks around delegation repair, cancellation, delegation persistence, and reopen flows, and updates delegation-path assertions and rollback test behavior.
Task history API and e2e verification packages/types/src/api.ts, src/extension/api.ts, apps/vscode-e2e/src/suite/subtasks.test.ts	Exposes task-history lookup and delegation lifecycle events through the API and verifies persisted parent/child history after delegated completion in e2e.
Test doubles and module mocks src/__tests__/helpers/provider-stub.ts, src/core/task/__tests__/Task.persistence.spec.ts, src/core/task/__tests__/Task.throttle.test.ts, src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts, src/core/webview/__tests__/checkpointRestoreHandler.spec.ts, src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts, src/core/webview/__tests__/webviewMessageHandler.delete.spec.ts, src/core/webview/__tests__/webviewMessageHandler.edit.spec.ts	Adds a default log mock to the provider stub and converts several task-persistence mocks to async partial mocks that spread the real module exports first.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

• [Story 2.3] Task status transition guard and startup reconciliation #366: Directly matches the transition guard, startup reconciliation, and delegation flow updates implemented here.
• fix(task-lifecycle): preserve parent-child link when delegated subtask is interrupted #560: Shares the ClineProvider delegation and history transition paths updated in this PR.
• feat(task-lifecycle): introduce explicit TaskStatus state machine with "interrupted" history state #559: Covers the same task-history status and delegation lifecycle changes.

Possibly related PRs

• Zoo-Code-Org/Zoo-Code#510: Related to the parent reopen flow in ClineProvider, which this PR now guards with explicit transition checks.
• Zoo-Code-Org/Zoo-Code#691: Related through TaskHistoryStore.atomicReadAndUpdate and delegation persistence paths.
• Zoo-Code-Org/Zoo-Code#725: Related through the atomic paired reopen/completion path now validated here.

Suggested labels: awaiting-review

Suggested reviewers: JamesRobert20, hannesrudolph, taltas, navedmerchant, p12tic

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The PR adds unrelated public API surface for getTaskHistoryItem and delegation event re-emission beyond the task-lifecycle fix.	Move the API/type changes and event plumbing into a separate PR unless they are required to satisfy the linked issue.
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: task status transition guards plus startup delegation reconciliation.
Description check	✅ Passed	The PR description covers the required issue link, implementation summary, test procedure, and checklist, with only non-critical sections omitted.
Linked Issues check	✅ Passed	The changes implement the Story 2.3 requirements: guarded status transitions, atomic write validation, and startup reconciliation for delegated parents.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue/366

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 85.93750% with 9 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/extension/api.ts	37.50%	5 Missing ⚠️
src/core/task-persistence/TaskHistoryStore.ts	93.33%	0 Missing and 3 partials ⚠️
src/core/webview/ClineProvider.ts	90.00%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/webview/ClineProvider.ts (1)

537-541: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Clear delegatedToId whenever the parent returns to active.

Line 537, Line 3214, and Line 3789 all repair/resume the parent back to active, but they leave the old delegatedToId behind. That persists contradictory history (status: "active" with a stale delegated-child pointer), and the new startup reconciliation only repairs parents that are still delegated, so this inconsistency can survive reloads.

Suggested fix

 await this.updateTaskHistory({
 	...parentHistory,
 	status: "active",
+	delegatedToId: undefined,
 	awaitingChildId: undefined,
 })

 await this.updateTaskHistory({
 	...parentHistory,
 	status: "active",
+	delegatedToId: undefined,
 	awaitingChildId: undefined,
 })

 updatedHistory = {
 	...parent,
 	status: "active" as const,
 	completedByChildId: childTaskId,
 	completionResultSummary,
+	delegatedToId: undefined,
 	awaitingChildId: undefined,
 	childIds,
 }

Also applies to: 3214-3218, 3788-3796

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/webview/ClineProvider.ts` around lines 537 - 541, The parent task
history is being set back to active in ClineProvider without clearing the stale
delegated child pointer, which leaves contradictory state behind. Update every
parent-recovery path in ClineProvider that restores status to "active" to also
clear delegatedToId alongside awaitingChildId, using the same updateTaskHistory
flow in the affected resume/repair branches. Make sure the reconciliation and
resume paths all normalize the parent record the same way so no old delegated
child reference survives reloads.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/core/task-persistence/TaskHistoryStore.ts`:
- Around line 113-119: `TaskHistoryStore.initialize()` currently repairs
delegation state before `ClineProvider.migrateFromGlobalState(...)`, so migrated
orphaned delegated parents can remain unrepaired until a later restart. Update
the startup flow in `ClineProvider` to either run migration before calling
`taskHistoryStore.initialize()`, or invoke the same delegation-repair path again
immediately after migration; use `TaskHistoryStore.initialize()` and
`reconcileDelegationState()` as the key points to adjust.

---

Outside diff comments:
In `@src/core/webview/ClineProvider.ts`:
- Around line 537-541: The parent task history is being set back to active in
ClineProvider without clearing the stale delegated child pointer, which leaves
contradictory state behind. Update every parent-recovery path in ClineProvider
that restores status to "active" to also clear delegatedToId alongside
awaitingChildId, using the same updateTaskHistory flow in the affected
resume/repair branches. Make sure the reconciliation and resume paths all
normalize the parent record the same way so no old delegated child reference
survives reloads.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9368591b-ae36-4d09-92e5-63b1f43ba982

📥 Commits

Reviewing files that changed from the base of the PR and between f845f2a and 2b9199e.

📒 Files selected for processing (16)

• src/__tests__/helpers/provider-stub.ts
• src/__tests__/history-resume-delegation.spec.ts
• src/__tests__/nested-delegation-resume.spec.ts
• src/__tests__/provider-delegation.spec.ts
• src/core/task-persistence/TaskHistoryStore.ts
• src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts
• src/core/task-persistence/__tests__/TaskHistoryStore.spec.ts
• src/core/task-persistence/index.ts
• src/core/task/__tests__/Task.persistence.spec.ts
• src/core/task/__tests__/Task.throttle.test.ts
• src/core/webview/ClineProvider.ts
• src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts
• src/core/webview/__tests__/checkpointRestoreHandler.spec.ts
• src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts
• src/core/webview/__tests__/webviewMessageHandler.delete.spec.ts
• src/core/webview/__tests__/webviewMessageHandler.edit.spec.ts

[navedmerchant]

is active to active a valid transition? should we add that?

[edelauna]

It's not really a state change - it's guarded against before being called - so active -> active would be unexpected, if it happens better it throws so we can see why it's being called, to investigate as there shouldn't be any self loops. The other call sites check if this is actually a state change, or just a metadata update.

[coderabbitai]

🧹 Nitpick comments (1)

apps/vscode-e2e/src/suite/subtasks.test.ts (1)

240-246: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor: duplicated cleanup blocks could be a loop.

Two sequential if (length > 0) { clearCurrentTask() } blocks only handle up to two leftover stack entries. A while loop would be more robust without added complexity.

♻️ Optional cleanup loop

-			if (api.getCurrentTaskStack().length > 0) {
-				await api.clearCurrentTask()
-			}
-			if (api.getCurrentTaskStack().length > 0) {
-				await api.clearCurrentTask()
-			}
+			while (api.getCurrentTaskStack().length > 0) {
+				await api.clearCurrentTask()
+			}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/vscode-e2e/src/suite/subtasks.test.ts` around lines 240 - 246, The
cleanup in subTasks test uses two repeated api.getCurrentTaskStack() checks that
only remove up to two leftover entries; replace the duplicated blocks with a
single loop so all remaining tasks are cleared. Update the logic in
subTasks.test.ts around api.clearCurrentTask and api.getCurrentTaskStack() to
keep clearing until the stack is empty, then keep the existing waitFor
assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/vscode-e2e/src/suite/subtasks.test.ts`:
- Around line 240-246: The cleanup in subTasks test uses two repeated
api.getCurrentTaskStack() checks that only remove up to two leftover entries;
replace the duplicated blocks with a single loop so all remaining tasks are
cleared. Update the logic in subTasks.test.ts around api.clearCurrentTask and
api.getCurrentTaskStack() to keep clearing until the stack is empty, then keep
the existing waitFor assertion.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a21468eb-b2ed-4e42-b61a-c113798238a8

📥 Commits

Reviewing files that changed from the base of the PR and between e15f726 and f2e599e.

📒 Files selected for processing (3)

• apps/vscode-e2e/src/suite/subtasks.test.ts
• packages/types/src/api.ts
• src/extension/api.ts