From 8f49ebb2483763a12d6da48365b51bf7fe48e986 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Tue, 9 Jun 2026 10:58:45 +0200 Subject: [PATCH] docs(worker): spec + plan for unifying the parent-task model Co-Authored-By: Claude Opus 4.8 (1M context) --- ...2026-06-09-unify-parent-task-model-plan.md | 69 +++++++++ ...26-06-09-unify-parent-task-model-design.md | 145 ++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-09-unify-parent-task-model-plan.md create mode 100644 docs/superpowers/specs/2026-06-09-unify-parent-task-model-design.md diff --git a/docs/superpowers/plans/2026-06-09-unify-parent-task-model-plan.md b/docs/superpowers/plans/2026-06-09-unify-parent-task-model-plan.md new file mode 100644 index 0000000..4cd24b9 --- /dev/null +++ b/docs/superpowers/plans/2026-06-09-unify-parent-task-model-plan.md @@ -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. diff --git a/docs/superpowers/specs/2026-06-09-unify-parent-task-model-design.md b/docs/superpowers/specs/2026-06-09-unify-parent-task-model-design.md new file mode 100644 index 0000000..7ca3199 --- /dev/null +++ b/docs/superpowers/specs/2026-06-09-unify-parent-task-model-design.md @@ -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.