From dc73f138d22eb2b0bcdffdff3cfcb26c612003c3 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:13:50 +0200 Subject: [PATCH 1/8] 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"`. From 724ad90f88dd9a9e6220d0467bc19b1e1a4beb17 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:25:47 +0200 Subject: [PATCH 2/8] feat(git): add ff-only merge and allow a merged worktree without merge commit --- src/ClaudeDo.Data/Git/GitService.cs | 8 ++++++++ src/ClaudeDo.Data/Repositories/WorktreeRepository.cs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ClaudeDo.Data/Git/GitService.cs b/src/ClaudeDo.Data/Git/GitService.cs index bacd9855..13cd34a4 100644 --- a/src/ClaudeDo.Data/Git/GitService.cs +++ b/src/ClaudeDo.Data/Git/GitService.cs @@ -450,6 +450,14 @@ public sealed class GitService return (exitCode, stderr); } + /// 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); + } + public async Task MergeAbortAsync(string repoDir, CancellationToken ct = default) { var (exitCode, _, stderr) = await RunGitAsync(repoDir, ["merge", "--abort"], ct); diff --git a/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs b/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs index 2abf7c72..af6d4eef 100644 --- a/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs +++ b/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs @@ -36,7 +36,7 @@ public sealed class WorktreeRepository .ExecuteUpdateAsync(s => s.SetProperty(w => w.State, state), ct); } - public async Task SetMergedAsync(string taskId, string mergeCommit, CancellationToken ct = default) + public async Task SetMergedAsync(string taskId, string? mergeCommit, CancellationToken ct = default) { await _context.Worktrees .Where(w => w.TaskId == taskId) From 9da522b0c9d7ee1068ed7a9ad1ee3215f613f0c1 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:25:47 +0200 Subject: [PATCH 3/8] feat(worker): fast-forward finished children into the parent's unit branch Planning parents get their own worktree on Queue plan, children fork from the unit head and are absorbed via merge --ff-only before Done (ff failure -> Failed, interactive child approve -> absorb instead of merge to the target). --- .../Lifecycle/TaskMergeService.cs | 20 +- .../Planning/PlanningChainCoordinator.cs | 9 +- src/ClaudeDo.Worker/Program.cs | 1 + src/ClaudeDo.Worker/Runner/TaskRunner.cs | 15 +- .../Runner/UnitBranchService.cs | 122 +++++++ src/ClaudeDo.Worker/Runner/WorktreeManager.cs | 20 +- .../Infrastructure/TaskStateServiceBuilder.cs | 5 +- .../Runner/UnitBranchTests.cs | 314 ++++++++++++++++++ 8 files changed, 489 insertions(+), 17 deletions(-) create mode 100644 src/ClaudeDo.Worker/Runner/UnitBranchService.cs create mode 100644 tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index af3ba7bb..9d99bbbd 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -117,6 +117,7 @@ public sealed class TaskMergeService private readonly ITaskStateService _state; private readonly IVerifyCommandRunner _verify; private readonly ILogger _logger; + private readonly UnitBranchService? _unitBranch; public TaskMergeService( IDbContextFactory dbFactory, @@ -124,7 +125,8 @@ public sealed class TaskMergeService HubBroadcaster broadcaster, ITaskStateService state, IVerifyCommandRunner verify, - ILogger logger) + ILogger logger, + UnitBranchService? unitBranch = null) { _dbFactory = dbFactory; _git = git; @@ -132,6 +134,7 @@ public sealed class TaskMergeService _state = state; _verify = verify; _logger = logger; + _unitBranch = unitBranch; } private async Task<(TaskEntity Task, ListEntity List, WorktreeEntity? Worktree, string? VerifyCommand)> LoadMergeContextAsync( @@ -1010,6 +1013,21 @@ public sealed class TaskMergeService if (task.Status != TaskStatus.WaitingForReview) return Blocked("task is not waiting for review"); + // An interactive child of a unit lands in the unit branch, not on the target; the parent's + // own approve merges the unit. A failed fast-forward keeps the child in review. + 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"); + } + } + if (wt is not null && wt.State == WorktreeState.Kept) { // Kept means either RevertMergeAsync undid a previous merge, or the user manually diff --git a/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs b/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs index cf1f19f8..85bda745 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs @@ -1,5 +1,6 @@ using ClaudeDo.Data; using ClaudeDo.Data.Models; +using ClaudeDo.Worker.Runner; using ClaudeDo.Worker.State; using Microsoft.EntityFrameworkCore; using TaskStatus = ClaudeDo.Data.Models.TaskStatus; @@ -10,13 +11,16 @@ public sealed class PlanningChainCoordinator { private readonly IDbContextFactory _dbFactory; private readonly Func _state; + private readonly UnitBranchService? _unitBranch; public PlanningChainCoordinator( IDbContextFactory dbFactory, - Func state) + Func state, + UnitBranchService? unitBranch = null) { _dbFactory = dbFactory; _state = state; + _unitBranch = unitBranch; } // Sets up a sequential chain over a planning parent's children. @@ -86,6 +90,9 @@ public sealed class PlanningChainCoordinator if (phase != PlanningPhase.Finalized) throw new InvalidOperationException("Plan must be finalized before it can be queued."); + // The parent's worktree is the unit branch every child forks from and is absorbed into. + if (_unitBranch is not null) + await _unitBranch.EnsureUnitWorktreeAsync(parentTaskId, ct); return await SetupChainAsync(parentTaskId, enqueue: true, ct); } diff --git a/src/ClaudeDo.Worker/Program.cs b/src/ClaudeDo.Worker/Program.cs index 7e356985..74ee597a 100644 --- a/src/ClaudeDo.Worker/Program.cs +++ b/src/ClaudeDo.Worker/Program.cs @@ -112,6 +112,7 @@ builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); +builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); diff --git a/src/ClaudeDo.Worker/Runner/TaskRunner.cs b/src/ClaudeDo.Worker/Runner/TaskRunner.cs index ca14055e..ad8fbc77 100644 --- a/src/ClaudeDo.Worker/Runner/TaskRunner.cs +++ b/src/ClaudeDo.Worker/Runner/TaskRunner.cs @@ -28,6 +28,7 @@ public sealed class TaskRunner private readonly AttachmentStore _attachments; private readonly ISessionSkillSeeder _skillSeeder; private readonly ITranscriptUsageReader _usageReader; + private readonly UnitBranchService? _unitBranch; public TaskRunner( IClaudeProcess claude, @@ -41,7 +42,8 @@ public sealed class TaskRunner TaskRunTokenRegistry tokens, AttachmentStore attachments, ISessionSkillSeeder skillSeeder, - ITranscriptUsageReader usageReader) + ITranscriptUsageReader usageReader, + UnitBranchService? unitBranch = null) { _claude = claude; _dbFactory = dbFactory; @@ -55,6 +57,7 @@ public sealed class TaskRunner _attachments = attachments; _skillSeeder = skillSeeder; _usageReader = usageReader; + _unitBranch = unitBranch; } public async Task RunAsync(TaskEntity task, string slot, CancellationToken ct, bool alreadyClaimed = false) @@ -568,6 +571,16 @@ public sealed class TaskRunner } else { + // Land the child in its unit branch before Done unblocks the successor, which forks from that head. + 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; + } + } var transition = await _state.CompleteAsync(task.Id, finishedAt, reviewResult, CancellationToken.None); if (!transition.Ok) _logger.LogWarning("CompleteAsync rejected for task {TaskId}: {Reason}", task.Id, transition.Reason); diff --git a/src/ClaudeDo.Worker/Runner/UnitBranchService.cs b/src/ClaudeDo.Worker/Runner/UnitBranchService.cs new file mode 100644 index 00000000..98d66b32 --- /dev/null +++ b/src/ClaudeDo.Worker/Runner/UnitBranchService.cs @@ -0,0 +1,122 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Git; +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.Hub; +using Microsoft.EntityFrameworkCore; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; + +namespace ClaudeDo.Worker.Runner; + +/// Outcome of . Applicable = the child belongs to a unit branch. +public sealed record AbsorbResult(bool Applicable, string? Error) +{ + /// The child has no unit branch to land in (no parent worktree, or no own worktree). + public static readonly AbsorbResult NotApplicable = new(false, null); + /// The unit branch was fast-forwarded to the child. + public static readonly AbsorbResult Absorbed = new(true, null); + /// True when the fast-forward was impossible; says why. + public bool Failed => Error is not null; +} + +/// +/// Git side of the unit branch: the parent's worktree owns it, children are fast-forwarded into it. +/// Never writes task status — callers own the transitions, which keeps the DI graph acyclic. +/// +public sealed class UnitBranchService +{ + private readonly GitService _git; + private readonly WorktreeManager _worktrees; + private readonly IDbContextFactory _dbFactory; + private readonly ILogger _logger; + private readonly HubBroadcaster? _broadcaster; + + public UnitBranchService( + GitService git, WorktreeManager worktrees, IDbContextFactory dbFactory, + ILogger logger, HubBroadcaster? broadcaster = null) + { + _git = git; + _worktrees = worktrees; + _dbFactory = dbFactory; + _logger = logger; + _broadcaster = broadcaster; + } + + /// Gives a parent its own worktree (= unit branch) unless it already has an Active one or its list has no working dir. + public async Task EnsureUnitWorktreeAsync(string parentId, CancellationToken ct) + { + TaskEntity parent; + ListEntity list; + using (var ctx = _dbFactory.CreateDbContext()) + { + parent = await ctx.Tasks.AsNoTracking().FirstOrDefaultAsync(t => t.Id == parentId, ct) + ?? throw new KeyNotFoundException($"Task '{parentId}' not found."); + var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(parentId, ct); + if (wt is { State: WorktreeState.Active }) return; + list = await new ListRepository(ctx).GetByIdAsync(parent.ListId, ct) + ?? throw new InvalidOperationException("List not found."); + } + if (string.IsNullOrWhiteSpace(list.WorkingDir)) return; + + await _worktrees.CreateAsync(parent, list, ct); + if (_broadcaster is not null) await _broadcaster.WorktreeUpdated(parentId); + } + + /// + /// Fast-forwards the parent's unit branch to the child's branch, then marks the child row + /// Merged (no merge commit) and removes its worktree and branch. + /// + public async Task AbsorbChildAsync(string childId, CancellationToken ct) + { + WorktreeEntity? childWt, parentWt; + string? workingDir; + using (var ctx = _dbFactory.CreateDbContext()) + { + var child = await ctx.Tasks.AsNoTracking().Include(t => t.List) + .FirstOrDefaultAsync(t => t.Id == childId, ct); + if (child?.ParentTaskId is null) return AbsorbResult.NotApplicable; + var repo = new WorktreeRepository(ctx); + childWt = await repo.GetByTaskIdAsync(childId, ct); + parentWt = await repo.GetByTaskIdAsync(child.ParentTaskId, ct); + workingDir = child.List.WorkingDir; + } + if (childWt is not { State: WorktreeState.Active } || parentWt is not { State: WorktreeState.Active } + || string.IsNullOrWhiteSpace(workingDir)) + return AbsorbResult.NotApplicable; + + // ponytail: kein Merge-Fallback; einbauen, falls parallele Children ein echter Fall werden. + var (code, stderr) = await _git.MergeFastForwardOnlyAsync(parentWt.Path, childWt.BranchName, ct); + if (code != 0) + return new AbsorbResult(true, + $"Unit branch '{parentWt.BranchName}' cannot be fast-forwarded to '{childWt.BranchName}' — the child " + + $"did not start from the unit's current head (e.g. it was run in parallel via Run now). git: {stderr}"); + + var head = await _git.RevParseHeadAsync(parentWt.Path, ct); + var diffStat = await _git.DiffStatAsync(parentWt.Path, parentWt.BaseCommit, head, ct: ct); + using (var ctx = _dbFactory.CreateDbContext()) + { + var repo = new WorktreeRepository(ctx); + await repo.UpdateHeadAsync(parentWt.TaskId, head, diffStat, ct); + await repo.SetMergedAsync(childId, null, ct); + } + + // The work is in the unit branch now; a leftover folder or branch is only clutter. + try { await _git.WorktreeRemoveAsync(workingDir, childWt.Path, force: true, CancellationToken.None); } + catch (Exception ex) { _logger.LogWarning(ex, "Failed to remove absorbed worktree {Path}", childWt.Path); } + try { await _git.BranchDeleteAsync(workingDir, childWt.BranchName, force: true, CancellationToken.None); } + catch (Exception ex) { _logger.LogWarning(ex, "Failed to delete absorbed branch {Branch}", childWt.BranchName); } + + _logger.LogInformation("Absorbed {Child} into unit branch {Branch} at {Head}", childWt.BranchName, parentWt.BranchName, head); + if (_broadcaster is not null) + { + await _broadcaster.WorktreeUpdated(childId); + await _broadcaster.WorktreeUpdated(parentWt.TaskId); + } + return AbsorbResult.Absorbed; + } + + /// Legacy unit: a Done child whose worktree is still Active was never absorbed and needs the unit merge. + 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); +} diff --git a/src/ClaudeDo.Worker/Runner/WorktreeManager.cs b/src/ClaudeDo.Worker/Runner/WorktreeManager.cs index 7b6540a2..420e1c25 100644 --- a/src/ClaudeDo.Worker/Runner/WorktreeManager.cs +++ b/src/ClaudeDo.Worker/Runner/WorktreeManager.cs @@ -256,23 +256,19 @@ public sealed class WorktreeManager return TicketStatusMap.ParseBandelTicketId(parentRef); } - // Improvement children (parent is a non-planning task with its own worktree) branch - // from the parent's recorded HEAD so they build on the parent's not-yet-merged work. - // Planning children and standalone tasks base off list_config.base_branch, else the current HEAD. + /// + /// A child of a parent with an Active worktree starts from the unit branch's current head, so it + /// sees every sibling absorbed before it. Unit roots (and legacy children) base off + /// list_config.base_branch, else the current HEAD. + /// private async Task ResolveBaseCommitAsync(TaskEntity task, string workingDir, CancellationToken ct) { if (task.ParentTaskId is not null) { using var ctx = _dbFactory.CreateDbContext(); - var parent = await ctx.Tasks.AsNoTracking() - .FirstOrDefaultAsync(t => t.Id == task.ParentTaskId, ct); - if (parent is not null && parent.PlanningPhase == PlanningPhase.None) - { - var parentWt = await new WorktreeRepository(ctx).GetByTaskIdAsync(task.ParentTaskId, ct); - var parentHead = parentWt?.HeadCommit ?? parentWt?.BaseCommit; - if (parentHead is not null) - return parentHead; - } + var parentWt = await new WorktreeRepository(ctx).GetByTaskIdAsync(task.ParentTaskId, ct); + if (parentWt is { State: WorktreeState.Active }) + return await _git.RevParseAsync(workingDir, parentWt.BranchName, ct); } using var cfgCtx = _dbFactory.CreateDbContext(); var baseBranch = await cfgCtx.ListConfigs.AsNoTracking() diff --git a/tests/ClaudeDo.Worker.Tests/Infrastructure/TaskStateServiceBuilder.cs b/tests/ClaudeDo.Worker.Tests/Infrastructure/TaskStateServiceBuilder.cs index b247e391..36ae7265 100644 --- a/tests/ClaudeDo.Worker.Tests/Infrastructure/TaskStateServiceBuilder.cs +++ b/tests/ClaudeDo.Worker.Tests/Infrastructure/TaskStateServiceBuilder.cs @@ -26,7 +26,8 @@ public static class TaskStateServiceBuilder public static Built Build( IDbContextFactory dbFactory, Func? mergeState = null, - BaseDirtyChecker? baseDirtyChecker = null) + BaseDirtyChecker? baseDirtyChecker = null, + ClaudeDo.Worker.Runner.UnitBranchService? unitBranch = null) { var hub = new CapturingHubContext(); var broadcaster = new HubBroadcaster(hub); @@ -46,7 +47,7 @@ public static class TaskStateServiceBuilder NullLogger.Instance); TaskStateService? state = null; - var chain = new PlanningChainCoordinator(dbFactory, () => state!); + var chain = new PlanningChainCoordinator(dbFactory, () => state!, unitBranch); state = new TaskStateService( dbFactory, broadcaster, diff --git a/tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs new file mode 100644 index 00000000..a360478e --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs @@ -0,0 +1,314 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Git; +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.Config; +using ClaudeDo.Worker.Git; +using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Lifecycle; +using ClaudeDo.Worker.Planning; +using ClaudeDo.Worker.Runner; +using ClaudeDo.Worker.Tests.Infrastructure; +using ClaudeDo.Worker.Tests.Services; +using Microsoft.Extensions.Logging.Abstractions; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; + +namespace ClaudeDo.Worker.Tests.Runner; + +/// Unit branch (spec 2026-10-01, section 2): children fork from and fast-forward into the parent's worktree. Real git + SQLite. +public sealed class UnitBranchTests : IDisposable +{ + private const string Parent = "p"; + private const string Child1 = "c1"; + private const string Child2 = "c2"; + + private readonly DbFixture _db = new(); + private readonly GitRepoFixture _repo = new(); + private readonly string _tempDir = Path.Combine(Path.GetTempPath(), $"cd_unit_{Guid.NewGuid():N}"); + private readonly string _listName = $"ub{Guid.NewGuid():N}"[..12]; + private readonly WorkerConfig _cfg; + private readonly GitService _git = new(); + private readonly WorktreeManager _wt; + private readonly UnitBranchService _unit; + private readonly TaskStateServiceBuilder.Built _built; + private readonly HubBroadcaster _broadcaster = new(new CapturingHubContext()); + + public UnitBranchTests() + { + Directory.CreateDirectory(_tempDir); + GitRepoFixture.RunGit(_repo.RepoDir, "branch", "-m", "main"); + _cfg = new WorkerConfig { SandboxRoot = _tempDir, LogRoot = _tempDir, WorktreeRootStrategy = "sibling" }; + var factory = _db.CreateFactory(); + _wt = new WorktreeManager(_git, factory, _cfg, NullLogger.Instance); + _unit = new UnitBranchService(_git, _wt, factory, NullLogger.Instance); + _built = TaskStateServiceBuilder.Build(factory, unitBranch: _unit); + Seed(); + } + + public void Dispose() + { + try + { + foreach (var line in GitRepoFixture.RunGit(_repo.RepoDir, "worktree", "list", "--porcelain").Split('\n')) + if (line.StartsWith("worktree ") && !Path.GetFullPath(line[9..].Trim()).Equals(Path.GetFullPath(_repo.RepoDir), StringComparison.OrdinalIgnoreCase)) + try { GitRepoFixture.RunGit(_repo.RepoDir, "worktree", "remove", "--force", line[9..].Trim()); } catch { } + } + catch { } + try { Directory.Delete(Path.Combine(WorktreeRootResolver.ResolveSiblingRoot(_repo.RepoDir), ".claudedo-worktrees", _listName), true); } catch { } + _db.Dispose(); + _repo.Dispose(); + try { Directory.Delete(_tempDir, true); } catch { } + } + + private void Seed() + { + using var ctx = _db.CreateContext(); + ctx.Lists.Add(new ListEntity { Id = "l1", Name = _listName, WorkingDir = _repo.RepoDir, CreatedAt = DateTime.UtcNow }); + ctx.Tasks.Add(new TaskEntity { Id = Parent, ListId = "l1", Title = "Plan", Number = 1, CommitType = "feat", + Status = TaskStatus.WaitingForChildren, PlanningPhase = PlanningPhase.Finalized, CreatedAt = DateTime.UtcNow }); + ctx.Tasks.Add(new TaskEntity { Id = Child1, ListId = "l1", Title = "First", Number = 2, ParentTaskId = Parent, + SortOrder = 1, Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow }); + ctx.Tasks.Add(new TaskEntity { Id = Child2, ListId = "l1", Title = "Second", Number = 3, ParentTaskId = Parent, + SortOrder = 2, Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow.AddSeconds(1) }); + ctx.SaveChanges(); + } + + private async Task WorktreeOf(string taskId) + { + using var ctx = _db.CreateContext(); + return await new WorktreeRepository(ctx).GetByTaskIdAsync(taskId); + } + + private async Task TaskOf(string taskId) + { + using var ctx = _db.CreateContext(); + return (await new TaskRepository(ctx).GetByIdAsync(taskId))!; + } + + private async Task SetStatus(string taskId, TaskStatus status) + { + using var ctx = _db.CreateContext(); + var t = ctx.Tasks.Single(x => x.Id == taskId); + t.Status = status; + await ctx.SaveChangesAsync(); + } + + private ListEntity List() { using var ctx = _db.CreateContext(); return ctx.Lists.Single(); } + + private TaskRunner Runner(FakeClaudeProcess fake) => new(fake, _db.CreateFactory(), _broadcaster, _wt, + new ClaudeArgsBuilder(), _cfg, NullLogger.Instance, _built.State, new TaskRunTokenRegistry(), + new AttachmentStore(), new FakeSessionSkillSeeder(), new FakeTranscriptUsageReader(), _unit); + + private TaskMergeService MergeService() => new(_db.CreateFactory(), _git, _broadcaster, _built.State, + new FakeVerifyCommandRunner(), NullLogger.Instance, _unit); + + private static FakeClaudeProcess Writes(Action? before = null) => new((prompt, dir, _, _, _) => + { + before?.Invoke(dir); + File.WriteAllText(Path.Combine(dir, $"{Guid.NewGuid():N}.txt"), "x"); + return Task.FromResult(new RunResult { ExitCode = 0, ResultMarkdown = "done" }); + }); + + private async Task RunChild(string childId, FakeClaudeProcess fake) + { + await SetStatus(childId, TaskStatus.Running); + await Runner(fake).RunAsync(await TaskOf(childId), "slot-1", default, alreadyClaimed: true); + } + + /// Runs both children serially; child 1 writes a.txt, child 2 records whether it saw it and writes b.txt. + private async Task RunTwoSerialChildren() + { + await _unit.EnsureUnitWorktreeAsync(Parent, default); + await RunChild(Child1, new FakeClaudeProcess((_, dir, _, _, _) => + { + File.WriteAllText(Path.Combine(dir, "a.txt"), "a"); + return Task.FromResult(new RunResult { ExitCode = 0, ResultMarkdown = "done" }); + })); + var sawA = false; + await RunChild(Child2, new FakeClaudeProcess((_, dir, _, _, _) => + { + sawA = File.Exists(Path.Combine(dir, "a.txt")); + File.WriteAllText(Path.Combine(dir, "b.txt"), "b"); + return Task.FromResult(new RunResult { ExitCode = 0, ResultMarkdown = "done" }); + })); + return sawA; + } + + [Fact] + public async Task QueuePlan_gives_planning_parent_its_unit_worktree() + { + await _built.Chain.QueuePlanAsync(Parent); + + var wt = await WorktreeOf(Parent); + Assert.NotNull(wt); + Assert.Equal(WorktreeState.Active, wt!.State); + Assert.Equal(_repo.BaseCommit, wt.BaseCommit); + Assert.StartsWith("feature/1-plan", wt.BranchName); + Assert.Equal(TaskStatus.Queued, (await TaskOf(Child1)).Status); + + await _unit.EnsureUnitWorktreeAsync(Parent, default); + Assert.Equal(wt.Path, (await WorktreeOf(Parent))!.Path); + } + + [Fact] + public async Task Child_starts_from_current_unit_head() + { + await _unit.EnsureUnitWorktreeAsync(Parent, default); + var unit = (await WorktreeOf(Parent))!; + File.WriteAllText(Path.Combine(unit.Path, "unit.txt"), "u"); + GitRepoFixture.RunGit(unit.Path, "add", "-A"); + GitRepoFixture.RunGit(unit.Path, "commit", "-m", "unit work"); + var unitHead = GitRepoFixture.RunGit(_repo.RepoDir, "rev-parse", unit.BranchName).Trim(); + + var ctx = await _wt.CreateAsync(await TaskOf(Child1), List(), default); + + Assert.Equal(unitHead, ctx.BaseCommit); + Assert.NotEqual(_repo.BaseCommit, ctx.BaseCommit); + } + + [Fact] + public async Task Serial_children_are_fast_forwarded_and_child2_sees_child1() + { + var sawA = await RunTwoSerialChildren(); + + Assert.True(sawA); + Assert.Equal(TaskStatus.Done, (await TaskOf(Child1)).Status); + Assert.Equal(TaskStatus.Done, (await TaskOf(Child2)).Status); + Assert.Equal(TaskStatus.WaitingForReview, (await TaskOf(Parent)).Status); + + var unit = (await WorktreeOf(Parent))!; + Assert.True(File.Exists(Path.Combine(unit.Path, "a.txt"))); + Assert.True(File.Exists(Path.Combine(unit.Path, "b.txt"))); + Assert.Equal(GitRepoFixture.RunGit(_repo.RepoDir, "rev-parse", unit.BranchName).Trim(), unit.HeadCommit); + + var branches = await _git.ListLocalBranchesAsync(_repo.RepoDir); + foreach (var child in new[] { Child1, Child2 }) + { + var wt = (await WorktreeOf(child))!; + Assert.Equal(WorktreeState.Merged, wt.State); + Assert.Null(wt.MergeCommit); + Assert.False(Directory.Exists(wt.Path)); + Assert.DoesNotContain(wt.BranchName, branches); + } + // Nothing lands on main before the parent's approve. + Assert.Equal(_repo.BaseCommit, GitRepoFixture.RunGit(_repo.RepoDir, "rev-parse", "main").Trim()); + } + + [Fact] + public async Task Child_that_cannot_fast_forward_fails_and_leaves_unit_alone() + { + await _unit.EnsureUnitWorktreeAsync(Parent, default); + var unit = (await WorktreeOf(Parent))!; + string? movedHead = null; + + // The unit moves while the child runs -- what a parallel "Run now" sibling would do. + await RunChild(Child1, Writes(_ => + { + File.WriteAllText(Path.Combine(unit.Path, "sibling.txt"), "s"); + GitRepoFixture.RunGit(unit.Path, "add", "-A"); + GitRepoFixture.RunGit(unit.Path, "commit", "-m", "sibling"); + movedHead = GitRepoFixture.RunGit(unit.Path, "rev-parse", "HEAD").Trim(); + })); + + var child = await TaskOf(Child1); + Assert.Equal(TaskStatus.Failed, child.Status); + Assert.Equal(movedHead, GitRepoFixture.RunGit(_repo.RepoDir, "rev-parse", unit.BranchName).Trim()); + Assert.Contains("fast-forward", child.Result); + Assert.Equal(WorktreeState.Active, (await WorktreeOf(Child1))!.State); + } + + [Fact] + public async Task Approving_a_new_parent_merges_the_unit_branch_onto_main() + { + await RunTwoSerialChildren(); + using (var ctx = _db.CreateContext()) + Assert.False(await UnitBranchService.HasUnabsorbedChildrenAsync(ctx, Parent, default)); + + var result = await MergeService().ApproveAndMergeAsync(Parent, "main", default); + + Assert.Equal(TaskMergeService.StatusMerged, result.Status); + Assert.Equal(TaskStatus.Done, (await TaskOf(Parent)).Status); + Assert.NotNull(await _git.ShowFileAsync(_repo.RepoDir, "main", "a.txt")); + Assert.NotNull(await _git.ShowFileAsync(_repo.RepoDir, "main", "b.txt")); + Assert.Equal(WorktreeState.Merged, (await WorktreeOf(Parent))!.State); + } + + [Fact] + public async Task Approving_an_interactive_child_absorbs_it_instead_of_merging_to_main() + { + await _unit.EnsureUnitWorktreeAsync(Parent, default); + var child = await TaskOf(Child1); + var ctx = await _wt.CreateAsync(child, List(), default); + File.WriteAllText(Path.Combine(ctx.WorktreePath, "i.txt"), "i"); + await _wt.CommitIfChangedAsync(ctx, child, List(), default); + await SetStatus(Child1, TaskStatus.WaitingForReview); + + var result = await MergeService().ApproveAndMergeAsync(Child1, "", default); + + Assert.Equal(TaskMergeService.StatusMerged, result.Status); + Assert.Equal(TaskStatus.Done, (await TaskOf(Child1)).Status); + Assert.True(File.Exists(Path.Combine((await WorktreeOf(Parent))!.Path, "i.txt"))); + Assert.Null(await _git.ShowFileAsync(_repo.RepoDir, "main", "i.txt")); + } + + [Fact] + public async Task Legacy_unit_still_goes_through_the_orchestrator() + { + // Pre-rollout shape: the planning parent has no worktree, a Done child still owns an Active one. + var child = await TaskOf(Child1); + var ctx = await _wt.CreateAsync(child, List(), default); + File.WriteAllText(Path.Combine(ctx.WorktreePath, "legacy.txt"), "l"); + await _wt.CommitIfChangedAsync(ctx, child, List(), default); + await SetStatus(Child1, TaskStatus.Done); + await SetStatus(Child2, TaskStatus.Cancelled); + await SetStatus(Parent, TaskStatus.WaitingForReview); + + using (var db = _db.CreateContext()) + Assert.True(await UnitBranchService.HasUnabsorbedChildrenAsync(db, Parent, default)); + // The legacy child is not absorbed by a late approve call either: it has no parent worktree. + Assert.False((await _unit.AbsorbChildAsync(Child1, default)).Applicable); + + var factory = _db.CreateFactory(); + var orch = new PlanningMergeOrchestrator(factory, MergeService(), + new PlanningAggregator(factory, _git, NullLogger.Instance), + _broadcaster, _git, _built.State, NullLogger.Instance); + await SetStatus(Child2, TaskStatus.Done); // StartAsync requires every planning child Done + using (var db = _db.CreateContext()) + { + db.Worktrees.Add(new WorktreeEntity { TaskId = Child2, Path = Path.Combine(_tempDir, "gone"), + BranchName = "gone", BaseCommit = _repo.BaseCommit, State = WorktreeState.Merged, CreatedAt = DateTime.UtcNow }); + await db.SaveChangesAsync(); + } + + var result = await orch.StartAsync(Parent, "main", default); + + Assert.Equal(TaskMergeService.StatusMerged, result.Status); + Assert.NotNull(await _git.ShowFileAsync(_repo.RepoDir, "main", "legacy.txt")); + Assert.Equal(TaskStatus.Done, (await TaskOf(Parent)).Status); + } + + [Fact] + public async Task Combined_diff_of_a_unit_is_base_to_head_without_an_integration_branch() + { + await RunTwoSerialChildren(); + var factory = _db.CreateFactory(); + var aggregator = new PlanningAggregator(factory, _git, NullLogger.Instance); + + var combined = await aggregator.BuildIntegrationBranchAsync(Parent, "main", default); + + var ok = Assert.IsType(combined); + Assert.Equal((await WorktreeOf(Parent))!.BranchName, ok.Value.IntegrationBranch); + Assert.Contains("a.txt", ok.Value.UnifiedDiff); + Assert.Contains("b.txt", ok.Value.UnifiedDiff); + Assert.Equal("main", await _git.GetCurrentBranchAsync(_repo.RepoDir)); + Assert.DoesNotContain(await _git.ListLocalBranchesAsync(_repo.RepoDir), b => b.StartsWith("planning/")); + + var perChild = await aggregator.GetAggregatedDiffAsync(Parent, default); + var d1 = perChild.Single(d => d.SubtaskId == Child1).UnifiedDiff; + var d2 = perChild.Single(d => d.SubtaskId == Child2).UnifiedDiff; + Assert.Contains("a.txt", d1); + Assert.DoesNotContain("b.txt", d1); + Assert.Contains("b.txt", d2); + Assert.DoesNotContain("a.txt", d2); + } +} From 1f373a5a675770593be69e5d7fe048da0309c341 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:25:48 +0200 Subject: [PATCH 4/8] feat(worker): approve a unit parent like a single task, orchestrator only for legacy units --- .../External/ExternalMcpService.cs | 25 +++++++++++++------ src/ClaudeDo.Worker/Hub/WorkerHub.cs | 8 +++--- .../Planning/PlanningMergeOrchestrator.cs | 4 ++- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/ClaudeDo.Worker/External/ExternalMcpService.cs b/src/ClaudeDo.Worker/External/ExternalMcpService.cs index c2240667..569ebede 100644 --- a/src/ClaudeDo.Worker/External/ExternalMcpService.cs +++ b/src/ClaudeDo.Worker/External/ExternalMcpService.cs @@ -12,6 +12,7 @@ using ClaudeDo.Worker.Hub; using ClaudeDo.Worker.Lifecycle; using ClaudeDo.Worker.Planning; using ClaudeDo.Worker.Queue; +using ClaudeDo.Worker.Runner; using ClaudeDo.Worker.State; using ClaudeDo.Worker.Worktrees; using Microsoft.EntityFrameworkCore; @@ -678,16 +679,23 @@ public sealed class ExternalMcpService { // Approve is the single review+merge action — mirror the hub's ApproveReview // routing instead of only flipping the status (which left branches unmerged). - bool hasChildren; + bool hasChildren, legacyUnit; await using (var ctx = await _dbFactory.CreateDbContextAsync(cancellationToken)) - hasChildren = await ctx.Tasks.AnyAsync(t => t.ParentTaskId == taskId, cancellationToken); - - if (hasChildren) { - // Compute before the merge starts -- children are still Done with their own - // pre-merge worktree/commit range at this point, so "did it contribute anything" - // reflects the review range the reviewer is about to approve. + hasChildren = await ctx.Tasks.AnyAsync(t => t.ParentTaskId == taskId, cancellationToken); + legacyUnit = hasChildren && await UnitBranchService.HasUnabsorbedChildrenAsync(ctx, taskId, cancellationToken); + } + + // Compute before the merge starts -- children are still Done with their own + // pre-merge worktree/commit range at this point, so "did it contribute anything" + // reflects the review range the reviewer is about to approve. + if (hasChildren) emptyChildren = await GetEmptyDoneChildrenAsync(taskId, cancellationToken); + + // Absorbed children are already in the parent's unit branch: the parent merges like a + // single task. Only a legacy unit (a Done child still Active) takes the unit merge. + if (legacyUnit) + { // externallyDriven: true — this call came from an MCP session, not the UI's // Approve button. A unit-merge conflict must not auto-open the in-app resolver; // the driving session resolves it via continue_merge/abort_merge instead. @@ -796,6 +804,9 @@ public sealed class ExternalMcpService { if (child.Worktree is not null) { + // Absorbed into the unit branch (no merge commit): empty iff it never committed. + if (child.Worktree is { State: WorktreeState.Merged, MergeCommit: null }) + return child.Worktree.HeadCommit is null; if (!Directory.Exists(child.Worktree.Path)) return false; var files = ParseDiffStatFileNames( await _git.DiffStatAsync(child.Worktree.Path, child.Worktree.BaseCommit, "HEAD", ct: ct)); diff --git a/src/ClaudeDo.Worker/Hub/WorkerHub.cs b/src/ClaudeDo.Worker/Hub/WorkerHub.cs index 7a6374fb..54c319dd 100644 --- a/src/ClaudeDo.Worker/Hub/WorkerHub.cs +++ b/src/ClaudeDo.Worker/Hub/WorkerHub.cs @@ -697,11 +697,13 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub public Task ApproveReview(string taskId, string targetBranch, bool autostash = false) => HubGuard(async () => { - bool hasChildren; + // Children already absorbed into the parent's unit branch need no unit merge -- the parent + // merges like a single task. Only legacy units (a Done child still Active) take the orchestrator. + bool legacyUnit; await using (var ctx = await _dbFactory.CreateDbContextAsync(CancellationToken.None)) - hasChildren = await ctx.Tasks.AnyAsync(t => t.ParentTaskId == taskId, CancellationToken.None); + legacyUnit = await UnitBranchService.HasUnabsorbedChildrenAsync(ctx, taskId, CancellationToken.None); - if (hasChildren) + if (legacyUnit) { var unitResult = await _planningMergeOrchestrator.StartAsync(taskId, targetBranch ?? "", CancellationToken.None); if (unitResult.Status == TaskMergeService.StatusBlocked) diff --git a/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs b/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs index b661b47a..35a04efc 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs @@ -134,7 +134,9 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState throw new InvalidOperationException("working tree has uncommitted changes"); var idsToMerge = new List(); - if (!isPlanning && parentHasWorktree) + // A planning parent owns a worktree too since the unit branch -- a mixed unit (absorbed + // children in it, legacy children beside it) must land it before the legacy children. + if (parentHasWorktree) idsToMerge.Add(parentTaskId); idsToMerge.AddRange( children From 44d94caabcd133486cb97d8c6a719f598265f5a7 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:25:48 +0200 Subject: [PATCH 5/8] feat(planning): combined diff of a unit is base..head, no integration branch checkout --- .../Planning/PlanningAggregator.cs | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/ClaudeDo.Worker/Planning/PlanningAggregator.cs b/src/ClaudeDo.Worker/Planning/PlanningAggregator.cs index b9ab7f42..adeead3f 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningAggregator.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningAggregator.cs @@ -2,6 +2,7 @@ using ClaudeDo.Data; using ClaudeDo.Data.Git; using ClaudeDo.Data.Models; using Microsoft.EntityFrameworkCore; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; namespace ClaudeDo.Worker.Planning; @@ -44,32 +45,47 @@ public sealed class PlanningAggregator { using var ctx = _dbFactory.CreateDbContext(); var parent = await ctx.Tasks + .Include(t => t.List) .Include(t => t.Worktree) .Include(t => t.Children).ThenInclude(c => c.Worktree) .SingleOrDefaultAsync(t => t.Id == planningTaskId, ct); if (parent is null) return new List(); + // Once a child was absorbed, the parent's branch also carries the children's commits; its own + // share ends where the first child forked off. + var children = parent.Children.OrderBy(c => c.SortOrder).ToList(); + var parentOwnHead = children.Any(c => c.Worktree is { State: WorktreeState.Merged, MergeCommit: null }) + ? children.Select(c => c.Worktree?.BaseCommit).FirstOrDefault(b => b is not null) + : null; + var nodes = new List(); // An improvement parent carries its own code branch — fold it in first so the // combined diff matches what the tree-merge will produce. if (parent.PlanningPhase == PlanningPhase.None && parent.Worktree is { State: WorktreeState.Active }) nodes.Add(parent); - nodes.AddRange(parent.Children.OrderBy(c => c.SortOrder)); + nodes.AddRange(children); var result = new List(); foreach (var node in nodes) { if (node.Worktree is null) continue; var wt = node.Worktree; - var head = wt.HeadCommit ?? await _git.RevParseHeadAsync(wt.Path, ct); + var isOwnShare = node == parent && parentOwnHead is not null; + // An absorbed child's worktree is gone; its commits live on in the unit branch. + var isGone = !Directory.Exists(wt.Path) && wt.HeadCommit is not null && parent.List.WorkingDir is not null; + string head; string unified; try { - unified = await _git.GetBranchDiffAsync(wt.Path, wt.BaseCommit, ct: ct); + head = isOwnShare ? parentOwnHead! : wt.HeadCommit ?? await _git.RevParseHeadAsync(wt.Path, ct); + unified = isOwnShare || isGone + ? await _git.GetCommitRangeDiffAsync(parent.List.WorkingDir ?? wt.Path, wt.BaseCommit, head, ct: ct) + : await _git.GetBranchDiffAsync(wt.Path, wt.BaseCommit, ct: ct); } catch (Exception ex) { _logger.LogWarning(ex, "diff failed for node {Id}", node.Id); + head = wt.HeadCommit ?? wt.BaseCommit; unified = ""; } result.Add(new SubtaskDiff( @@ -83,6 +99,15 @@ public sealed class PlanningAggregator { var (planning, repoDir, childSubtasks) = await LoadPlanningContextAsync(planningTaskId, ct); + // A parent that owns the unit branch already is the integration: unitBase..unitHead, no + // checkout in the main checkout. Legacy units (a Done child never absorbed) keep the old path. + if (planning.Worktree is { State: WorktreeState.Active } unitWt + && !childSubtasks.Any(c => c.Status == TaskStatus.Done && c.Worktree is { State: WorktreeState.Active })) + { + var unitDiff = await _git.GetCommitRangeDiffAsync(repoDir, unitWt.BaseCommit, unitWt.BranchName, ct: ct); + return new CombinedDiffResult.Ok(new CombinedDiffSuccess(unitWt.BranchName, unitDiff)); + } + var integrationBranch = BuildIntegrationBranchName(planning); // Reset: checkout target first (so we're never ON the integration branch when deleting it), From 823368484e6d27d6abb714a7fc3d57132af8a254 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:26:17 +0200 Subject: [PATCH 6/8] docs(worker): document unit branch, child absorb and legacy approve routing --- docs/explore-notes/review-merge.md | 23 +++++++++++++++-------- src/ClaudeDo.Worker/CLAUDE.md | 20 +++++++++++++++++++- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/docs/explore-notes/review-merge.md b/docs/explore-notes/review-merge.md index 2351dfc1..f2c28f70 100644 --- a/docs/explore-notes/review-merge.md +++ b/docs/explore-notes/review-merge.md @@ -1,8 +1,8 @@ # Review, merge & conflict resolution > **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative. -> Last verified against `168ab1cd` (2026-08-26), which added the batch verify -> (`skipVerify` + `verify_merges`). +> Last verified against `44d94caa` (2026-10-01), which made the parent's worktree the unit +> branch (children fast-forwarded into it). > Planning mode renders per file and `DiffLinesView` is retired. > Drift check: `git log --oneline 20bce9b..HEAD -- src/ClaudeDo.Worker/Lifecycle src/ClaudeDo.Worker/State src/ClaudeDo.Worker/Planning src/ClaudeDo.Ui/ViewModels/Conflicts src/ClaudeDo.Worker/External` > Stable structure only (no line numbers). See docs/explore-notes/README.md. @@ -15,11 +15,17 @@ Covers the review→merge path: `TaskStateService` review transitions, `TaskMerg `ApproveReview` (hub) and `review_task` approve (MCP) are the **single** review+merge action. There is no separate "Merge all" entry. -- **Task with children** → drives `PlanningMergeOrchestrator`: merges the parent worktree if - `Active`, then each `Done` child in order, then sets the parent `Done`. A mid-merge conflict - pauses for `ContinuePlanningMerge` / `AbortPlanningMerge`. -- **Childless task** → `TaskMergeService.ApproveAndMergeAsync`. A conflict keeps the task in - `WaitingForReview`. +- **Legacy unit** (`UnitBranchService.HasUnabsorbedChildrenAsync`: a `Done` child whose worktree + is still `Active`, i.e. planned before the unit branch existed) → drives + `PlanningMergeOrchestrator`: merges the parent worktree if `Active` (planning parents too, for a + mixed unit), then each `Done` child in order, then sets the parent `Done`. A mid-merge conflict + pauses for `ContinuePlanningMerge` / `AbortPlanningMerge`. Children in `WaitingForReview` are + silently skipped there. +- **Everything else** — a childless task, or a parent whose children were absorbed into its unit + branch → `TaskMergeService.ApproveAndMergeAsync` on the task's own worktree. A conflict keeps + the task in `WaitingForReview`. +- **Child of a unit** (parent has an `Active` worktree) → `ApproveAndMergeAsync` absorbs it into + the unit branch (`UnitBranchService.AbsorbChildAsync`, ff-only) instead of merging to the target. - **No active worktree** (sandbox run) → straight to `Done`. Review transitions all live in `TaskStateService`: `SubmitForReviewAsync`, @@ -203,7 +209,8 @@ succeeds, written **only** by `WorktreeRepository.SetMergedAsync` (which atomica It is the only thing that makes `revert_merge` possible without heuristically searching `git log` — see [external-mcp.md](external-mcp.md) → `RevertMerge`. Null for any worktree -merged before the field existed. +merged before the field existed, and for a child absorbed into its unit branch (a fast-forward +has no merge commit) — such a child is reverted with its whole unit, never alone. ## Review gate in the UI diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index 7293329e..fa1735bf 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -96,6 +96,19 @@ nothing to merge; a worktree task's branch just stays Active for the worktrees o `… → WaitingForChildren → WaitingForReview → Done` via the single `TryAdvanceParentAsync`. Planning/improvement **children** go straight to `Done`; only the parent is reviewed. +**Unit branch.** The parent's own worktree is the unit branch (improvement parents have it from +their run, planning parents get it in `QueuePlanAsync` via `UnitBranchService.EnsureUnitWorktreeAsync`). +Children fork from its current head and `UnitBranchService.AbsorbChildAsync` fast-forwards it to +the child (`merge --ff-only` in the parent worktree) — from `TaskRunner.HandleSuccess` right before +`CompleteAsync`, and from `TaskMergeService.ApproveAndMergeAsync` for an interactive child in +review. The child row goes `Merged` with `MergeCommit` null (so `revert_merge` only works on the +whole unit), its worktree and branch are removed, the parent row's `HeadCommit`/`DiffStat` move +along. A child that can't fast-forward (e.g. started in parallel via Run now) is `Failed` — no +merge fallback; in the approve path it stays `WaitingForReview` with a `blocked` result. +`UnitBranchService` never writes task status (it would close a DI cycle through +`PlanningChainCoordinator`). ⚠️ `QueuePicker` counts `worktrees.state='merged'` as a satisfied +dependency, so an absorbed child satisfies a `dependsOn` edge although nothing reached `main` yet. + **Approve = merge the whole unit.** `ApproveReview` / `review_task` approve is the single review+merge action — there is no separate "Merge all". Details, plus the post-merge verify gate and the conflict resolver → [review-merge](../../docs/explore-notes/review-merge.md). @@ -108,6 +121,11 @@ gate and the conflict resolver → [review-merge](../../docs/explore-notes/revie 2. `PlanningChainCoordinator.SetupChainAsync(parent, enqueue: false)` establishes the blocked-by chain (child[i] → child[i-1]) but **leaves children `Idle`** — finalize never auto-queues. Queueing is a deliberate user action: `QueuePlanAsync` (hub `QueuePlanningSubtasksAsync`, the "Queue plan" button) calls `SetupChainAsync(parent, enqueue: true)`. 3. Once queued, the first child is woken automatically; successors unblock as their predecessor reaches a terminal state via `OnChildFinishedAsync`. +`QueuePlanAsync` first gives the parent its worktree (the unit branch, see Unified parent model); +the integration branch in `PlanningAggregator` (`planning/{slug}-integration`, `checkout -b` in the +main checkout) is only built for legacy units — a parent that owns the unit returns +`unitBase..unitBranch` as its combined diff. + A child that hits a roadblock (fails, or reports `CLAUDEDO_BLOCKED`) does **not** advance the parent — it stays in `WaitingForChildren` until every child is terminal, with blocked children surfaced on the parent's Session tab instead. @@ -130,7 +148,7 @@ Full flow, invariants, and model/effort/max-turns resolution (including the low- - **ClaudeProcess** — spawns `claude -p --output-format stream-json --verbose --permission-mode auto` (or whatever app settings specify). Prompt via stdin, NDJSON from stdout. CancellationToken kills the process tree. Resolves `_cfg.ClaudeBin` via `ExecutableResolver` (`ClaudeDo.Data`, shared with `ClaudeCliPreflight` and the Installer's checks) before spawning — a `.cmd`/`.bat` shim (e.g. an npm-installed `claude`) is launched through `cmd.exe /c` since `UseShellExecute = false` can't exec a shim directly; a resolved `.exe` starts exactly as before. Throws if nothing resolves. *(Not yet on `main` — see `Environment Checks` in `ClaudeDo.Installer/CLAUDE.md`.)* - **ClaudeArgsBuilder** — `--model`, `--effort`, `--max-turns`, `--append-system-prompt`, `--agents`, `--json-schema`, `--resume` - **StreamAnalyzer** — parses NDJSON; extracts session_id, token counts, turn counts, result text, structured output. Replaced MessageParser. -- **WorktreeManager** — worktrees on `/-` branches, same leaf as the folder (`NameFor`, slug capped at 40 chars, umlauts transliterated). Typ from `BranchTypeFor` (`feat`→`feature`, `fix/refactor/docs/test/chore` as-is, anything else `chore`). Nummer = ticket id (own `TicketRef` or the parent's), else the task number — in a company repo (`CompanyRepo`, origin matches `company_remote_pattern`) without a ticket no number at all. A taken folder/branch name gets an 8-char task-id suffix, so the self-heal never force-deletes a foreign branch. Base = parent head for children, else `list_config.base_branch`, else the checked-out HEAD. Commits with semantic messages, updates DB with head commit + diff stats +- **WorktreeManager** — worktrees on `/-` branches, same leaf as the folder (`NameFor`, slug capped at 40 chars, umlauts transliterated). Typ from `BranchTypeFor` (`feat`→`feature`, `fix/refactor/docs/test/chore` as-is, anything else `chore`). Nummer = ticket id (own `TicketRef` or the parent's), else the task number — in a company repo (`CompanyRepo`, origin matches `company_remote_pattern`) without a ticket no number at all. A taken folder/branch name gets an 8-char task-id suffix, so the self-heal never force-deletes a foreign branch. Base = `rev-parse ` for a child whose parent has an Active worktree, else `list_config.base_branch`, else the checked-out HEAD. Commits with semantic messages, updates DB with head commit + diff stats - **CommitMessageBuilder** — `{commitType}(slug): title\n\ndescription\n\nClaudeDo-Task: taskId`; `BuildMerge` is the merge-commit variant (`{commitType}(slug): merge title` + trailer). **Every merge caller passes a blank commit message on purpose** — `TaskMergeService` fills in `BuildMerge` from the task's commit type and its list's name, which is the only place that knows both. Don't reintroduce a caller-side literal. - **TaskResetService** — discards a failed task's worktree, resets the row to Idle, preserves run history - **AgentFileService** — manages `~/.claudeDo/agents/*.md`; list/refresh via SignalR From a353897345eded7a3ae8a13d7fb92ccfd899a3c8 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:27:22 +0200 Subject: [PATCH 7/8] docs(external): describe unit-branch routing in the review_task tool description --- src/ClaudeDo.Worker/External/ExternalMcpService.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ClaudeDo.Worker/External/ExternalMcpService.cs b/src/ClaudeDo.Worker/External/ExternalMcpService.cs index 569ebede..0b036471 100644 --- a/src/ClaudeDo.Worker/External/ExternalMcpService.cs +++ b/src/ClaudeDo.Worker/External/ExternalMcpService.cs @@ -628,8 +628,10 @@ public sealed class ExternalMcpService [McpServerTool, Description( "Act on a task that is WaitingForReview — the only way to approve, reject or retire a reviewed run. " + "'approve' is review+merge, exactly like the UI's Approve: a childless task merges its worktree into " + - "targetBranch then goes Done; a task with children drives the unit merge (parent worktree if active + each " + - "Done child in order); a task without an active worktree approves straight to Done. Fails if the task is " + + "targetBranch then goes Done; a parent whose children were fast-forwarded into its own worktree (the unit " + + "branch) merges that worktree the same way; a child of such a unit is absorbed into the unit branch instead " + + "of the target; an older unit with unabsorbed Done children drives the unit merge (parent worktree if active " + + "+ each Done child in order); a task without an active worktree approves straight to Done. Fails if the task is " + "not WaitingForReview (except 'cancel', which also works while Running/Queued). mergeStatus 'conflict' " + "means the merge stopped on conflicts, with the files listed. emptyChildren (parent approve only) lists " + "the Done children about to be unit-merged whose own review range contributed nothing (e.g. a child that " + From acd188bca9d596488153fc1b430f346a15f7d824 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:36:45 +0200 Subject: [PATCH 8/8] fix(worker): block approve of a planning parent with an unfinished child, surface git stderr on ff failure --- docs/explore-notes/review-merge.md | 3 ++- src/ClaudeDo.Worker/CLAUDE.md | 3 +++ .../Lifecycle/TaskMergeService.cs | 14 ++++++++++++++ .../Runner/UnitBranchService.cs | 5 +++-- .../Runner/UnitBranchTests.cs | 18 ++++++++++++++++++ 5 files changed, 40 insertions(+), 3 deletions(-) diff --git a/docs/explore-notes/review-merge.md b/docs/explore-notes/review-merge.md index f2c28f70..e8a47bd0 100644 --- a/docs/explore-notes/review-merge.md +++ b/docs/explore-notes/review-merge.md @@ -23,7 +23,8 @@ There is no separate "Merge all" entry. silently skipped there. - **Everything else** — a childless task, or a parent whose children were absorbed into its unit branch → `TaskMergeService.ApproveAndMergeAsync` on the task's own worktree. A conflict keeps - the task in `WaitingForReview`. + the task in `WaitingForReview`. A planning parent with any child not `Done` is `blocked` there + (`subtask X is not Done (status …)`, same as the orchestrator) — no half plan on the target. - **Child of a unit** (parent has an `Active` worktree) → `ApproveAndMergeAsync` absorbs it into the unit branch (`UnitBranchService.AbsorbChildAsync`, ff-only) instead of merging to the target. - **No active worktree** (sandbox run) → straight to `Done`. diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index fa1735bf..09c8cadf 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -105,6 +105,9 @@ review. The child row goes `Merged` with `MergeCommit` null (so `revert_merge` o whole unit), its worktree and branch are removed, the parent row's `HeadCommit`/`DiffStat` move along. A child that can't fast-forward (e.g. started in parallel via Run now) is `Failed` — no merge fallback; in the approve path it stays `WaitingForReview` with a `blocked` result. +A planning parent is only approvable once **every** child is `Done` — `ApproveAndMergeAsync` +blocks with `subtask X is not Done (status …)`, the orchestrator's rule, so a failed/cancelled child +never lands half a plan on the target (improvement parents have no such gate, as before). `UnitBranchService` never writes task status (it would close a DI cycle through `PlanningChainCoordinator`). ⚠️ `QueuePicker` counts `worktrees.state='merged'` as a satisfied dependency, so an absorbed child satisfies a `dependsOn` edge although nothing reached `main` yet. diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index 9d99bbbd..00481abb 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -1013,6 +1013,20 @@ public sealed class TaskMergeService if (task.Status != TaskStatus.WaitingForReview) return Blocked("task is not waiting for review"); + // A plan lands whole or not at all -- same rule (and message) as PlanningMergeOrchestrator.StartAsync, + // which a unit-branch parent no longer passes through. + if (task.PlanningPhase != PlanningPhase.None) + { + using var ctx = _dbFactory.CreateDbContext(); + var notDone = await ctx.Tasks.AsNoTracking() + .Where(c => c.ParentTaskId == taskId && c.Status != TaskStatus.Done) + .OrderBy(c => c.SortOrder) + .Select(c => new { c.Id, c.Status }) + .FirstOrDefaultAsync(ct); + if (notDone is not null) + return Blocked($"subtask {notDone.Id} is not Done (status {notDone.Status})"); + } + // An interactive child of a unit lands in the unit branch, not on the target; the parent's // own approve merges the unit. A failed fast-forward keeps the child in review. if (task.ParentTaskId is not null && _unitBranch is not null) diff --git a/src/ClaudeDo.Worker/Runner/UnitBranchService.cs b/src/ClaudeDo.Worker/Runner/UnitBranchService.cs index 98d66b32..dd4edb35 100644 --- a/src/ClaudeDo.Worker/Runner/UnitBranchService.cs +++ b/src/ClaudeDo.Worker/Runner/UnitBranchService.cs @@ -88,8 +88,9 @@ public sealed class UnitBranchService var (code, stderr) = await _git.MergeFastForwardOnlyAsync(parentWt.Path, childWt.BranchName, ct); if (code != 0) return new AbsorbResult(true, - $"Unit branch '{parentWt.BranchName}' cannot be fast-forwarded to '{childWt.BranchName}' — the child " + - $"did not start from the unit's current head (e.g. it was run in parallel via Run now). git: {stderr}"); + $"Unit branch '{parentWt.BranchName}' cannot be fast-forwarded to '{childWt.BranchName}': {stderr.Trim()} " + + "(usually the child did not start from the unit's current head, e.g. run in parallel via Run now, " + + "or the unit worktree has local changes)"); var head = await _git.RevParseHeadAsync(parentWt.Path, ct); var diffStat = await _git.DiffStatAsync(parentWt.Path, parentWt.BaseCommit, head, ct: ct); diff --git a/tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs index a360478e..9fd0af51 100644 --- a/tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Runner/UnitBranchTests.cs @@ -214,6 +214,7 @@ public sealed class UnitBranchTests : IDisposable Assert.Equal(TaskStatus.Failed, child.Status); Assert.Equal(movedHead, GitRepoFixture.RunGit(_repo.RepoDir, "rev-parse", unit.BranchName).Trim()); Assert.Contains("fast-forward", child.Result); + Assert.Contains("Not possible to fast-forward", child.Result); // git's own stderr is surfaced Assert.Equal(WorktreeState.Active, (await WorktreeOf(Child1))!.State); } @@ -233,6 +234,23 @@ public sealed class UnitBranchTests : IDisposable Assert.Equal(WorktreeState.Merged, (await WorktreeOf(Parent))!.State); } + [Fact] + public async Task Approving_a_planning_parent_with_an_unfinished_child_is_blocked() + { + await _unit.EnsureUnitWorktreeAsync(Parent, default); + await RunChild(Child1, Writes()); + await SetStatus(Child2, TaskStatus.Failed); + await SetStatus(Parent, TaskStatus.WaitingForReview); + + var result = await MergeService().ApproveAndMergeAsync(Parent, "main", default); + + Assert.Equal(TaskMergeService.StatusBlocked, result.Status); + Assert.Equal($"subtask {Child2} is not Done (status Failed)", result.ErrorMessage); + Assert.Equal(TaskStatus.WaitingForReview, (await TaskOf(Parent)).Status); + Assert.Equal(WorktreeState.Active, (await WorktreeOf(Parent))!.State); + Assert.Equal(_repo.BaseCommit, GitRepoFixture.RunGit(_repo.RepoDir, "rev-parse", "main").Trim()); + } + [Fact] public async Task Approving_an_interactive_child_absorbs_it_instead_of_merging_to_main() {