docs(worker): spec + plan for unifying the parent-task model
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,69 @@
|
|||||||
|
# Plan — Unify the parent-task model
|
||||||
|
|
||||||
|
Spec: `docs/superpowers/specs/2026-06-09-unify-parent-task-model-design.md`
|
||||||
|
|
||||||
|
Subagents: `sonnet`. Stage files explicitly by path (never `git add -A`). TDD.
|
||||||
|
Build with `-c Release` per project. Commit per task (Conventional Commits).
|
||||||
|
|
||||||
|
## Task 1 — Single parent-advance path
|
||||||
|
|
||||||
|
- Rename `TaskStateService.TryAdvanceImprovementParentAsync` → `TryAdvanceParentAsync`.
|
||||||
|
- Make it advance **any** `WaitingForChildren` parent → `WaitingForReview` when all
|
||||||
|
children are terminal, and advance a parent with **zero** children straight to
|
||||||
|
`WaitingForReview`.
|
||||||
|
- In `OnChildTerminalAsync`: drop the `TryCompleteParentAsync` call; keep
|
||||||
|
`_chain.OnChildFinishedAsync`; call the renamed advance method for all parents.
|
||||||
|
- Tests: extend `WaitingForChildrenLifecycleTests` — (a) improvement parent still
|
||||||
|
advances; (b) a `WaitingForChildren` parent whose children are a *sequential chain*
|
||||||
|
advances only after the last one is terminal; (c) zero-children parent advances.
|
||||||
|
|
||||||
|
## Task 2 — Delete `TryCompleteParentAsync`
|
||||||
|
|
||||||
|
- Remove `TaskRepository.TryCompleteParentAsync` (`TaskRepository.cs:477-502`) and
|
||||||
|
any remaining references.
|
||||||
|
- Update `src/ClaudeDo.Data/CLAUDE.md` (drop it from the TaskRepository helper list).
|
||||||
|
- Build Data + Worker; fix references.
|
||||||
|
|
||||||
|
## Task 3 — Planning finalize enters `WaitingForChildren`
|
||||||
|
|
||||||
|
- `TaskStateService.FinalizePlanningAsync`: in the same `ExecuteUpdateAsync`, set
|
||||||
|
`Status = WaitingForChildren` alongside `PlanningPhase = Finalized` /
|
||||||
|
`PlanningFinalizedAt`.
|
||||||
|
- Verify `PlanningSessionManager.FinalizeAsync` ordering: finalize (→ WaitingForChildren)
|
||||||
|
**before** `SetupChainAsync` enqueues child[0]. Adjust only if ordering is wrong.
|
||||||
|
- Tests: finalizing a planning parent with N children leaves it `WaitingForChildren`;
|
||||||
|
after the chain completes it is `WaitingForReview` (not `Done`); a planning parent
|
||||||
|
with zero finalized children lands in `WaitingForReview`.
|
||||||
|
|
||||||
|
## Task 4 — Approve merges the whole unit
|
||||||
|
|
||||||
|
- `WorkerHub.ApproveReview` (+ MCP `ReviewTask` approve): if the task has children,
|
||||||
|
invoke `PlanningMergeOrchestrator` (parent worktree if `Active` + each `Done` child
|
||||||
|
in order) then transition parent → `Done`; on child conflict keep `WaitingForReview`
|
||||||
|
and surface the conflicting child. If no children, keep the existing
|
||||||
|
`TaskMergeService.ApproveAndMergeAsync` path.
|
||||||
|
- Retire `WorkerHub.MergeAllPlanning` + its UI button/command and any now-dead
|
||||||
|
orchestrator entry that only `MergeAllPlanning` used (keep the orchestrator itself).
|
||||||
|
- Sync test fakes for `IWorkerClient`/`WorkerHub` if signatures changed.
|
||||||
|
- Tests: approving a parent with two `Done` children merges both then sets `Done`;
|
||||||
|
a conflicting second child keeps the parent in `WaitingForReview`.
|
||||||
|
|
||||||
|
## Task 5 — Cancellable `WaitingForChildren` parent
|
||||||
|
|
||||||
|
- Add `TaskStatus.WaitingForChildren` to the `CancelAsync` guard.
|
||||||
|
- Test: a parent in `WaitingForChildren` can be cancelled.
|
||||||
|
|
||||||
|
## Task 6 — Docs
|
||||||
|
|
||||||
|
- `src/ClaudeDo.Worker/CLAUDE.md`: add `WaitingForChildren` to the Status table +
|
||||||
|
transition diagram; document the unified parent flow and approve-merges-unit;
|
||||||
|
remove `MergeAllPlanning` from the Hub method list.
|
||||||
|
- `src/ClaudeDo.Data/CLAUDE.md`: add `WaitingForChildren` to the TaskEntity status list.
|
||||||
|
- Root `CLAUDE.md`: update the "Task status flow" convention line.
|
||||||
|
|
||||||
|
## Verify
|
||||||
|
|
||||||
|
- `dotnet test` for Worker.Tests + Data.Tests (`-c Release`).
|
||||||
|
- UI flows (planning finalize → review → approve-merge; improvement parent;
|
||||||
|
retired MergeAllPlanning button) are **visual-verification gaps** — flag for the
|
||||||
|
user to run the app; do not claim they work from tests alone.
|
||||||
@@ -0,0 +1,145 @@
|
|||||||
|
# Unify the parent-task model (planning · improvement · normal)
|
||||||
|
|
||||||
|
**Date:** 2026-06-09
|
||||||
|
**Status:** Approved-pending-implementation
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
ClaudeDo has three ways a task produces and waits on work, grown as separate
|
||||||
|
mechanisms that represent the *same shape* — "a task runs, may emit children,
|
||||||
|
and once it + its children are terminal it surfaces for review":
|
||||||
|
|
||||||
|
| | children authored | scheduling | parent flow today | merge of children |
|
||||||
|
|---|---|---|---|---|
|
||||||
|
| **Normal** | none | — | `Running → WaitingForReview → Done` | own worktree on approve |
|
||||||
|
| **Improvement** | autonomously *during* run (`suggest_improvement`) | parallel (no blockers) | `Running → WaitingForChildren → WaitingForReview → Done` | separate `MergeAllPlanning` |
|
||||||
|
| **Planning** | interactively *before* run (planning session) | sequential chain (`BlockedByTaskId`) | `Idle →(Active→Finalized)→ Done` (skips review) | separate `MergeAllPlanning` |
|
||||||
|
|
||||||
|
The incidental divergence we want to remove:
|
||||||
|
|
||||||
|
1. **Two "parent is waiting on children" representations** — improvement uses
|
||||||
|
`Status=WaitingForChildren`; planning uses `PlanningPhase=Finalized` with the
|
||||||
|
parent's `Status` jumping `Idle → Done`, never passing through the waiting/review
|
||||||
|
states at all.
|
||||||
|
2. **Two parent-advance methods** doing the same job —
|
||||||
|
`TaskRepository.TryCompleteParentAsync` (planning → `Done`, no review) vs
|
||||||
|
`TaskStateService.TryAdvanceImprovementParentAsync` (improvement → `WaitingForReview`).
|
||||||
|
3. **A separate merge action** — `MergeAllPlanning` / `PlanningMergeOrchestrator`
|
||||||
|
merges children, decoupled from the parent's `approve`. Approving a parent and
|
||||||
|
merging its unit are two clicks.
|
||||||
|
|
||||||
|
What is **genuinely unique and kept**: `PlanningPhase.Active` — the interactive,
|
||||||
|
human-in-the-loop authoring gate where children are drafted and cannot run until
|
||||||
|
finalize. Improvement has no equivalent. The two *authoring* entry points
|
||||||
|
(`PlanningMcpService.CreateChildTask` vs `TaskRunMcpService.SuggestImprovement`)
|
||||||
|
also stay distinct — they already share `CreateChildAsync`; unifying the authoring
|
||||||
|
UX is explicitly out of scope.
|
||||||
|
|
||||||
|
## Decisions (locked)
|
||||||
|
|
||||||
|
- **All parents get review.** A planning parent now surfaces in `WaitingForReview`
|
||||||
|
after its children finish, instead of auto-completing to `Done`.
|
||||||
|
- **Approve merges the whole unit.** Approving a parent merges the parent worktree
|
||||||
|
(if any) + all `Done` children in order, reusing `PlanningMergeOrchestrator`. The
|
||||||
|
standalone `MergeAllPlanning` button is retired (folded into approve).
|
||||||
|
- **Scope = state model + code paths.** Internal refactor; authoring UX and child
|
||||||
|
base-commit resolution are unchanged.
|
||||||
|
|
||||||
|
## Target model
|
||||||
|
|
||||||
|
**One parent-with-children lifecycle, used by every parent regardless of how its
|
||||||
|
children were authored:**
|
||||||
|
|
||||||
|
```
|
||||||
|
┌─ (no children) ──────────────┐
|
||||||
|
Idle → Queued → Running ──┤ ├→ WaitingForReview → Done
|
||||||
|
└─ (has/spawns children) ─┐ │ (approve =
|
||||||
|
│ │ merge unit)
|
||||||
|
WaitingForChildren ─┘ │
|
||||||
|
│ │
|
||||||
|
(all children terminal) ───────┘
|
||||||
|
```
|
||||||
|
|
||||||
|
Planning parent (never runs as an agent — it runs an interactive session):
|
||||||
|
|
||||||
|
```
|
||||||
|
Idle (PlanningPhase None)
|
||||||
|
→[StartPlanning] Idle (PlanningPhase Active) ← authoring gate (KEPT)
|
||||||
|
→[FinalizePlanning] WaitingForChildren (Finalized) ← children chain runs
|
||||||
|
→[all children terminal] WaitingForReview
|
||||||
|
→[approve] merge unit → Done
|
||||||
|
```
|
||||||
|
|
||||||
|
Children (planning **and** improvement) keep going straight to `Done` with no
|
||||||
|
individual review; they accumulate on their branches and merge as a unit when the
|
||||||
|
parent is approved.
|
||||||
|
|
||||||
|
### State machine after the change
|
||||||
|
|
||||||
|
- `WaitingForChildren` is the **single** "parent waiting on children" state, used by
|
||||||
|
both planning and improvement parents.
|
||||||
|
- `WaitingForReview` is reached by every parent before `Done`.
|
||||||
|
- `PlanningPhase`: `None | Active | Finalized` — unchanged; `Active` remains the
|
||||||
|
authoring gate, `Finalized` marks "was a planning parent" and is set together with
|
||||||
|
`Status=WaitingForChildren`.
|
||||||
|
|
||||||
|
## Code changes
|
||||||
|
|
||||||
|
1. **Single parent-advance path.** Rename
|
||||||
|
`TaskStateService.TryAdvanceImprovementParentAsync` →
|
||||||
|
`TryAdvanceParentAsync`; it already only checks `Status==WaitingForChildren` +
|
||||||
|
"all children terminal" → `WaitingForReview` (with the failed/cancelled
|
||||||
|
annotation on `Result`). It becomes the only path for both systems.
|
||||||
|
- Handle **zero children**: a finalized planning parent with no children must go
|
||||||
|
straight to `WaitingForReview` (today `TryComplete`/`TryAdvance` both `return`
|
||||||
|
on `Count == 0`).
|
||||||
|
|
||||||
|
2. **Delete `TaskRepository.TryCompleteParentAsync`** (`TaskRepository.cs:477`) and
|
||||||
|
its invocation in `TaskStateService.OnChildTerminalAsync`. Planning parents now
|
||||||
|
advance via `TryAdvanceParentAsync` to `WaitingForReview` instead of `Done`.
|
||||||
|
- Keep `_chain.OnChildFinishedAsync` (inter-child unblock — planning-only effect).
|
||||||
|
|
||||||
|
3. **`FinalizePlanningAsync`** (`TaskStateService.cs:289`) sets the parent
|
||||||
|
`Status = WaitingForChildren` in the same update that sets
|
||||||
|
`PlanningPhase = Finalized`. This happens before `SetupChainAsync` enqueues
|
||||||
|
child[0], so the parent is in `WaitingForChildren` before any child can finish.
|
||||||
|
|
||||||
|
4. **Approve merges the unit.** `WorkerHub.ApproveReview` (and the MCP
|
||||||
|
`ReviewTask` approve path): when the approved task has children, run
|
||||||
|
`PlanningMergeOrchestrator` (parent worktree if `Active` + each `Done` child in
|
||||||
|
order), then transition the parent to `Done`. On a child merge conflict, the
|
||||||
|
parent stays in `WaitingForReview` (mirrors current single-task approve-conflict
|
||||||
|
behavior). Retire the `MergeAllPlanning` Hub method + UI button.
|
||||||
|
|
||||||
|
5. **Allow cancelling a `WaitingForChildren` parent.** Add `WaitingForChildren` to
|
||||||
|
the `CancelAsync` guard so a parent waiting on children can be cancelled (today it
|
||||||
|
cannot — minor gap).
|
||||||
|
|
||||||
|
6. **Docs.** Fix the `WaitingForChildren`-missing drift in
|
||||||
|
`src/ClaudeDo.Data/CLAUDE.md` and `src/ClaudeDo.Worker/CLAUDE.md`, and update the
|
||||||
|
transition diagram + the root `CLAUDE.md` status-flow line to the unified model.
|
||||||
|
|
||||||
|
## Out of scope (unchanged)
|
||||||
|
|
||||||
|
- Authoring UX: planning session vs `suggest_improvement` stay as two distinct
|
||||||
|
entry points (both already call `CreateChildAsync`).
|
||||||
|
- `WorktreeManager.ResolveBaseCommitAsync` base-commit divergence (planning children
|
||||||
|
branch from list HEAD; improvement children from parent head) — left as-is.
|
||||||
|
- Sequential-vs-parallel scheduling — already shared infrastructure
|
||||||
|
(`BlockedByTaskId`); planning chains, improvement doesn't. No change.
|
||||||
|
|
||||||
|
## Risks / edge cases
|
||||||
|
|
||||||
|
- **Ordering on finalize** — parent must be `WaitingForChildren` before the first
|
||||||
|
child can reach terminal. Guaranteed by setting it inside `FinalizePlanningAsync`,
|
||||||
|
which runs before `SetupChainAsync`.
|
||||||
|
- **Zero-children planning parent** — must advance to `WaitingForReview`, not stick
|
||||||
|
in `WaitingForChildren`. Explicit branch in `TryAdvanceParentAsync` /
|
||||||
|
`FinalizePlanningAsync`.
|
||||||
|
- **Failed/cancelled children** — parent still advances to `WaitingForReview` with
|
||||||
|
the existing `⚠ Children: N failed, M cancelled` annotation; no wedge.
|
||||||
|
- **Approve-merge conflict** — keep parent in `WaitingForReview`; surface the
|
||||||
|
conflicting child like the current merge-conflict path.
|
||||||
|
- **Existing rows** — planning parents currently sitting at `Idle`+`Finalized` with
|
||||||
|
live children: behavior change is forward-only (new finalizes use the new flow);
|
||||||
|
no migration needed since `Status`/`PlanningPhase` columns already exist.
|
||||||
Reference in New Issue
Block a user