From dc73f138d22eb2b0bcdffdff3cfcb26c612003c3 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:13:50 +0200 Subject: [PATCH] docs(planning): plan for unit branch part 2 (children fast-forward) --- .../2026-10-01-unit-branch-2-children.md | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-01-unit-branch-2-children.md diff --git a/docs/superpowers/plans/2026-10-01-unit-branch-2-children.md b/docs/superpowers/plans/2026-10-01-unit-branch-2-children.md new file mode 100644 index 00000000..409be2e2 --- /dev/null +++ b/docs/superpowers/plans/2026-10-01-unit-branch-2-children.md @@ -0,0 +1,184 @@ +# Unit-Branch Teil 2: Children per fast-forward — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Children of a unit start from the unit branch head and are fast-forwarded into it as soon as they finish; a parent is approved like a single task. + +**Architecture:** New `UnitBranchService` (Worker/Runner) owns the git side: it creates the planning parent's worktree on "Queue plan" and absorbs a finished child (`merge --ff-only` in the parent worktree, child worktree removed, row `Merged`, `MergeCommit` null). It never touches task status — `TaskRunner` (`FailAsync` on a ff failure) and `TaskMergeService` (approve of an interactive child) own the transitions, so `TaskStateService`/`PlanningChainCoordinator` stay git-free and the DI graph stays acyclic. Legacy units (a `Done` child with an `Active` worktree) keep going through `PlanningMergeOrchestrator`. + +**Tech Stack:** .NET 8, EF Core SQLite, git CLI, xUnit (real git + real SQLite in Worker.Tests). + +Spec: `docs/superpowers/specs/2026-10-01-unit-branch-draft-pr-design.md`, section 2. Private behaviour only — company mode (push/PR) is part 3. + +## File map + +| File | Change | +|---|---| +| `src/ClaudeDo.Data/Git/GitService.cs` | + `MergeFastForwardOnlyAsync` | +| `src/ClaudeDo.Data/Repositories/WorktreeRepository.cs` | `SetMergedAsync(…, string? mergeCommit)` | +| `src/ClaudeDo.Worker/Runner/WorktreeManager.cs` | `ResolveBaseCommitAsync`: child → `rev-parse ` for any parent with an Active worktree | +| `src/ClaudeDo.Worker/Runner/UnitBranchService.cs` | **new** — `EnsureUnitWorktreeAsync`, `AbsorbChildAsync`, `HasUnabsorbedChildrenAsync` | +| `src/ClaudeDo.Worker/Program.cs` | register `UnitBranchService` (+ `Share<>` for the external container is not needed — the MCP side only uses the static helper) | +| `src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs` | `QueuePlanAsync` calls `EnsureUnitWorktreeAsync` | +| `src/ClaudeDo.Worker/Runner/TaskRunner.cs` | `HandleSuccess`: absorb before `CompleteAsync`, ff failure → `MarkFailed` | +| `src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs` | `ApproveAndMergeAsync`: child of a unit worktree → absorb instead of merge | +| `src/ClaudeDo.Worker/Hub/WorkerHub.cs`, `src/ClaudeDo.Worker/External/ExternalMcpService.cs` | approve routing: orchestrator only for unabsorbed (legacy) children | +| `src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs` | also merges a *planning* parent's own Active worktree (mixed legacy unit) | +| `src/ClaudeDo.Worker/Planning/PlanningAggregator.cs` | combined diff = `unitBase..unitBranch` without `checkout -b` when the parent owns the unit; per-child diff of absorbed children from the commit range | +| `tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs` | **new** — all unit-branch tests | +| `src/ClaudeDo.Worker/CLAUDE.md`, `docs/explore-notes/review-merge.md` | docs | + +--- + +### Task 1: Git primitive + nullable MergeCommit + +**Files:** Modify `src/ClaudeDo.Data/Git/GitService.cs` (after `MergeNoFfAsync`), `src/ClaudeDo.Data/Repositories/WorktreeRepository.cs:39` + +- [ ] **Step 1: Add the method** + +```csharp +/// git merge --ff-only: moves the checked-out branch to or fails without touching it. +public async Task<(int ExitCode, string Stderr)> MergeFastForwardOnlyAsync( + string worktreePath, string sourceRef, CancellationToken ct = default) +{ + var (exitCode, _, stderr) = await RunGitAsync(worktreePath, ["merge", "--ff-only", sourceRef], ct); + return (exitCode, stderr); +} +``` + +- [ ] **Step 2:** `SetMergedAsync(string taskId, string? mergeCommit, …)` — an absorbed child has no merge commit. +- [ ] **Step 3:** Covered by the Task 3 tests (no standalone test for a one-line git wrapper). + +### Task 2: Child base = unit head + +**Files:** Modify `src/ClaudeDo.Worker/Runner/WorktreeManager.cs` (`ResolveBaseCommitAsync`) + +- [ ] **Step 1: Failing test** (`UnitBranchTests.Child_starts_from_unit_head`): planning parent (`PlanningPhase.Finalized`) with a worktree, commit a file in the parent worktree (`git -C commit`), then `CreateAsync(child)` → `ctx.BaseCommit == rev-parse `. Fails today because planning children fall through to `HEAD` of the main checkout. +- [ ] **Step 2: Implement** + +```csharp +if (task.ParentTaskId is not null) +{ + using var ctx = _dbFactory.CreateDbContext(); + var parentWt = await new WorktreeRepository(ctx).GetByTaskIdAsync(task.ParentTaskId, ct); + if (parentWt is { State: WorktreeState.Active }) + return await _git.RevParseAsync(workingDir, parentWt.BranchName, ct); +} +``` + +- [ ] **Step 3:** run `--filter "FullyQualifiedName~UnitBranchTests|FullyQualifiedName~WorktreeManagerTests"`, commit. + +### Task 3: UnitBranchService + +**Files:** Create `src/ClaudeDo.Worker/Runner/UnitBranchService.cs`, register in `Program.cs` next to `WorktreeManager`. + +```csharp +public sealed record AbsorbResult(bool Applicable, string? Error) +{ + public static readonly AbsorbResult NotApplicable = new(false, null); + public static readonly AbsorbResult Absorbed = new(true, null); + public bool Failed => Error is not null; +} + +public sealed class UnitBranchService +{ + // ctor: GitService, WorktreeManager, IDbContextFactory, HubBroadcaster?, ILogger + + /// Planning parent → own worktree = unit branch; no-op if it already has an Active one or the list has no git working dir. + public async Task EnsureUnitWorktreeAsync(string parentId, CancellationToken ct); + + /// ff the parent worktree to the child branch; child row Merged (MergeCommit null), child worktree + branch removed. + /// NotApplicable when the child or its parent has no Active worktree. Never changes task status. + public async Task AbsorbChildAsync(string childId, CancellationToken ct); + + /// Legacy routing: a Done child whose worktree is still Active was never absorbed. + public static Task HasUnabsorbedChildrenAsync(ClaudeDoDbContext ctx, string parentId, CancellationToken ct) => + ctx.Tasks.AnyAsync(t => t.ParentTaskId == parentId && t.Status == TaskStatus.Done + && t.Worktree != null && t.Worktree.State == WorktreeState.Active, ct); +} +``` + +Absorb order: ff → parent `UpdateHeadAsync(head, diffstat base..head)` → child `SetMergedAsync(null)` → best-effort `worktree remove --force` + `branch -D` → `WorktreeUpdated` for both. ff failure returns `Error` with a clear message and a `ponytail:` comment (no merge fallback). + +- [ ] Tests (real git): two serial children absorbed, child 2 sees child 1's file, parent branch head == child 2 head, child rows `Merged` with null `MergeCommit`, child branches gone; a child forked from the old head after the unit moved → `Failed`, unit branch unchanged. +- [ ] Commit. + +### Task 4: Queue plan creates the parent worktree + +**Files:** `PlanningChainCoordinator.cs` (optional ctor param `UnitBranchService? unitBranch = null`), `TaskStateServiceBuilder.cs` (optional pass-through). + +```csharp +if (_unitBranch is not null) await _unitBranch.EnsureUnitWorktreeAsync(parentTaskId, ct); +return await SetupChainAsync(parentTaskId, enqueue: true, ct); +``` + +- [ ] Test: finalized planning parent + 1 child in a git list → after `QueuePlanAsync` the parent has an Active worktree row whose base = repo HEAD; a second call keeps the same row. +- [ ] Commit. + +### Task 5: TaskRunner absorbs before CompleteAsync + +**Files:** `TaskRunner.cs` (optional trailing ctor param `UnitBranchService? unitBranch = null`; in the child branch of `HandleSuccess`): + +```csharp +if (_unitBranch is not null) +{ + var absorb = await _unitBranch.AbsorbChildAsync(task.Id, CancellationToken.None); + if (absorb.Failed) + { + await MarkFailed(task.Id, task.Number, task.Title, slot, absorb.Error, result.TurnCount); + return; + } +} +``` + +- [ ] Test: TaskRunner with `FakeClaudeProcess` writing a file into its working dir, two queued children of a unit → both Done, unit head has both files; a child whose base was overtaken → `Failed` with the ff message. +- [ ] Commit. + +### Task 6: Interactive child approve absorbs + +**Files:** `TaskMergeService.cs` (optional ctor param `UnitBranchService? unitBranch = null`); in `ApproveAndMergeAsync` after the `WaitingForReview` guard: + +```csharp +if (task.ParentTaskId is not null && _unitBranch is not null) +{ + var absorb = await _unitBranch.AbsorbChildAsync(taskId, ct); + if (absorb.Failed) return Blocked(absorb.Error!); + if (absorb.Applicable) + { + var approved = await _state.ApproveReviewAsync(taskId, ct); + return approved.Ok ? new MergeResult(StatusMerged, Array.Empty(), null) : Blocked(approved.Reason ?? "approve failed"); + } +} +``` + +A blocked approve keeps the child `WaitingForReview` (the reviewer can reject/re-run) — the `Failed` rule applies to autonomous runs. + +- [ ] Test: child in `WaitingForReview` with a commit → approve → child Done, unit branch contains the file, main untouched. +- [ ] Commit. + +### Task 7: Approve routing + +**Files:** `WorkerHub.ApproveReview`, `ExternalMcpService.ReviewTask`, `PlanningMergeOrchestrator.StartAsync`. + +- Replace `hasChildren` with `UnitBranchService.HasUnabsorbedChildrenAsync` for the orchestrator branch (MCP still computes `emptyChildren` whenever children exist; absorbed children count as empty when their row has no `HeadCommit`). +- Orchestrator: `if (parentHasWorktree) idsToMerge.Add(parentTaskId)` (drop `!isPlanning`) so a mixed legacy unit still lands the unit branch. +- [ ] Tests: approve of a new parent (all children absorbed) merges the unit branch onto `main` and the parent goes Done; a legacy parent (Done child with Active worktree, no parent worktree) still goes through the orchestrator and lands the child. +- [ ] Commit. + +### Task 8: Combined diff without the integration branch + +**Files:** `PlanningAggregator.cs`. + +- `BuildIntegrationBranchAsync`: parent with Active worktree and no unabsorbed child → `Ok(parentWt.BranchName, git diff )`, no checkout in the main checkout. Otherwise legacy path. +- `GetAggregatedDiffAsync`: a node whose worktree directory is gone but has `HeadCommit` → `GetCommitRangeDiffAsync(repoDir, BaseCommit, HeadCommit)`; an improvement parent's own node is limited to `BaseCommit..firstChild.BaseCommit` once children were forked from it (else it would repeat the absorbed children). +- [ ] Test: unit with an absorbed child → combined diff contains the child's file, main checkout stays on `main`, no `planning/*-integration` branch exists. +- [ ] Commit. + +### Task 9: Docs + +`src/ClaudeDo.Worker/CLAUDE.md` (Planning Flow, Unified parent model, WorktreeManager base rule), `docs/explore-notes/review-merge.md` (Approve section, MergeCommit/revert note, Unit-Branch section). Commit. + +## Verification + +`dotnet build ClaudeDo.slnx -c Release`, then +`dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release --no-build --filter "FullyQualifiedName~UnitBranch|FullyQualifiedName~WorktreeManager|FullyQualifiedName~PlanningMerge|FullyQualifiedName~PlanningAggregator|FullyQualifiedName~StandaloneChildren|FullyQualifiedName~TaskMergeService|FullyQualifiedName~ReviewTask"`.