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