docs(planning): plan for unit branch part 2 (children fast-forward)
This commit is contained in:
mika kuns
@@ -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 <parentWt.BranchName>` 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
|
||||
/// <summary><c>git merge --ff-only</c>: moves the checked-out branch to <paramref name="sourceRef"/> or fails without touching it.</summary>
|
||||
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 <wt> commit`), then `CreateAsync(child)` → `ctx.BaseCommit == rev-parse <parentBranch>`. 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<AbsorbResult> AbsorbChildAsync(string childId, CancellationToken ct);
|
||||
|
||||
/// Legacy routing: a Done child whose worktree is still Active was never absorbed.
|
||||
public static Task<bool> 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<string>(), 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 <BaseCommit> <BranchName>)`, 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"`.
|
||||
Reference in New Issue
Block a user