From e59f6c2438e022a76540b402d41cc59819491927 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 6 Aug 2026 11:54:46 +0200 Subject: [PATCH] fix(worker): surface empty review ranges and blocked children over MCP preview_merge/preview_merge_set now report isEmpty (ahead==0, or HandlerBaseCommit==HandlerHeadCommit for a worktree-less list-handler task) instead of leaving an empty branch indistinguishable from a small one. preview_merge also stops throwing for worktree-less handler tasks, falling back to their fixed commit range. review_task's parent approve returns emptyChildren, naming the Done children whose review range contributed nothing before the unit merge lands. TaskRefDto/TaskDto now expose roadblockCount so a CLAUDEDO_BLOCKED child is identifiable over MCP, since it still reaches Done per the unified parent model. --- docs/explore-notes/external-mcp.md | 36 +++- docs/explore-notes/review-merge.md | 13 +- .../External/ExternalMcpService.cs | 172 ++++++++++++---- .../External/ExternalMcpServiceTests.cs | 187 ++++++++++++++++++ 4 files changed, 361 insertions(+), 47 deletions(-) diff --git a/docs/explore-notes/external-mcp.md b/docs/explore-notes/external-mcp.md index 38ace536..0ee949d7 100644 --- a/docs/explore-notes/external-mcp.md +++ b/docs/explore-notes/external-mcp.md @@ -1,8 +1,8 @@ # External MCP tool surface > **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative. -> Last verified against commit `86f962e` (2026-08-06). -> Drift check: `git log --oneline 86f962e..HEAD -- src/ClaudeDo.Worker/External` +> Last verified against commit `8247a74` (2026-08-06). +> Drift check: `git log --oneline 8247a74..HEAD -- src/ClaudeDo.Worker/External` > Stable structure only (no line numbers). See docs/explore-notes/README.md. Covers `src/ClaudeDo.Worker/External/` — the always-on MCP tools ClaudeDo exposes to general @@ -85,17 +85,35 @@ happens before the lean/full projection either way. `WaitingForReview` task. Approve is review+merge exactly like the hub's `ApproveReview`: unit merge for parents, worktree merge into optional `targetBranch` for childless tasks. Conflicts are reported in `ReviewTaskResult`. +- A parent's approve also returns `emptyChildren`: the `Done` children about to be unit-merged + whose own review range contributed nothing (computed the same way as `PreviewMerge`'s + `isEmpty`, before the merge starts so it reflects what's about to be approved). Surfaces a + child that reported `CLAUDEDO_BLOCKED` and committed no code — previously that child reached + `Done` and merged silently with `changedFileCount: 0`, indistinguishable from a small-but-real + change. `TaskRefDto.roadblockCount` (on every task-returning tool, stamped by `TaskRunner` from + `result.Blocks.Count`) is the MCP-visible signal for *why* a child is empty. **`PreviewMerge`** — non-destructive `git merge-tree --write-tree` mergeability check for one task's worktree branch against `targetBranch` (default: the repo's current branch). Returns -status / conflictFiles / changedFileCount / `behind`. Unlike `TaskMergeService.PreviewAsync`'s -silent *"unavailable"*, this **throws a clear error** when the task has no worktree, the -worktree isn't `Active`, or the list's working dir is missing. +status / conflictFiles / changedFileCount / `behind` / `isEmpty`. Unlike +`TaskMergeService.PreviewAsync`'s silent *"unavailable"*, this **throws a clear error** when the +task has neither an active worktree nor a handler commit range, or the list's working dir is +missing. +- `isEmpty` = the review range contributed nothing — zero files changed against the worktree's + base commit, or (for a worktree-less list-handler host task) `HandlerBaseCommit == + HandlerHeadCommit`. Distinguishes a genuinely empty branch from one that merely made a small + change (`changedFileCount: 0` alone reads as "tiny", not "nothing to review") — the gap that + let two blocked planning children reach `Done` with unmerged empty branches unnoticed. +- A worktree-less handler task has no separate branch to `merge-tree`-preview (its commits + already sit in `list.WorkingDir`) — `PreviewMergeCoreAsync` falls back to a synthetic `clean` + preview over its own `HandlerBaseCommit..HandlerHeadCommit` diff-stat instead of throwing + "has no worktree". -**`PreviewMergeSet`** — same preview for a batch, plus a file→tasks overlap report built from -each task's own diff-stat. ⚠️ That overlap report is a **same-file-name hint only** — it is -blind to cross-file collisions (e.g. the CS0103 case that motivated it). A task that fails to -preview gets `error` set and is excluded from the overlap instead of aborting the batch. +**`PreviewMergeSet`** — same preview for a batch (each entry also carries `isEmpty`), plus a +file→tasks overlap report built from each task's own diff-stat. ⚠️ That overlap report is a +**same-file-name hint only** — it is blind to cross-file collisions (e.g. the CS0103 case that +motivated it). A task that fails to preview gets `error` set and is excluded from the overlap +instead of aborting the batch. **`RevertMerge`** — undoes a previously merged task's merge commit on `targetBranch` via `git revert -m 1`. Always a **new commit**, never a reset/rewrite, because the target working diff --git a/docs/explore-notes/review-merge.md b/docs/explore-notes/review-merge.md index e4e0674e..4e18763a 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 commit `f6cb825` (2026-08-05). -> Drift check: `git log --oneline f6cb825..HEAD -- src/ClaudeDo.Worker/Lifecycle src/ClaudeDo.Worker/State src/ClaudeDo.Worker/Planning src/ClaudeDo.Ui/ViewModels/Conflicts` +> Last verified against commit `8247a74` (2026-08-06). +> Drift check: `git log --oneline 8247a74..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. Covers the review→merge path: `TaskStateService` review transitions, `TaskMergeService`, @@ -47,6 +47,15 @@ advance the parent — the parent stays in `WaitingForChildren` until every chil The UI surfaces blocked children on the parent's Session tab (`ChildOutcomes` + a "children need attention" band) so the roadblock is visible without forcing a transition. +A blocked planning/improvement child still goes straight to `Done` per the unified parent model +above — it committed nothing, but nothing prevents its (empty) branch from being unit-merged +like any other `Done` child once the parent is approved. The MCP surface has no UI equivalent of +`ChildOutcomes`, so `review_task`'s approve on a parent additionally returns `emptyChildren` (the +`Done` children whose review range is empty) and every task-returning tool exposes +`TaskRefDto.roadblockCount` → [external-mcp.md](external-mcp.md) → `ReviewTask`/`PreviewMerge`. +An empty branch is still mergeable by design (some tasks — e.g. an audit — legitimately produce +no diff); this is a visibility fix, not a merge gate. + ## Post-merge verify gate A list can set `ListConfigEntity.VerifyCommand` (List Settings modal → Verification). diff --git a/src/ClaudeDo.Worker/External/ExternalMcpService.cs b/src/ClaudeDo.Worker/External/ExternalMcpService.cs index 38e1891f..be46dcea 100644 --- a/src/ClaudeDo.Worker/External/ExternalMcpService.cs +++ b/src/ClaudeDo.Worker/External/ExternalMcpService.cs @@ -19,7 +19,10 @@ namespace ClaudeDo.Worker.External; public sealed record TaskListDto(string Id, string Name, string? WorkingDir); public sealed record DeleteTaskResult(bool Deleted, string Id); public sealed record CancelTaskResult(bool Cancelled, string Id); -public sealed record ReviewTaskResult(TaskRefDto Task, string? MergeStatus, IReadOnlyList MergeConflicts, string? MergeMessage, string? RepoPath = null); +// EmptyChildren is non-null only for a parent's approve (unit merge): the Done children whose +// review range (worktree ahead, or HandlerBaseCommit..HandlerHeadCommit for a worktree-less +// child) contributed nothing, so a reviewer sees them before approving instead of after. +public sealed record ReviewTaskResult(TaskRefDto Task, string? MergeStatus, IReadOnlyList MergeConflicts, string? MergeMessage, string? RepoPath = null, IReadOnlyList? EmptyChildren = null); public sealed record StatusValueDto(string Status, string Meaning); public sealed record RunTaskNowResult(bool Started, string TaskId); @@ -35,7 +38,12 @@ public sealed record TaskDto( DateTime? StartedAt, DateTime? FinishedAt, bool IsMyDay, - int SortOrder); + int SortOrder, + // Count of CLAUDEDO_BLOCKED roadblocks the run reported, stamped by TaskRunner on finish. + // A planning/improvement child reporting > 0 still goes straight to Done (see + // ClaudeDo.Worker/CLAUDE.md → Unified parent model) -- this is the only MCP-visible signal + // that it may have delivered nothing despite that Done status. + int RoadblockCount = 0); // Lean counterpart to TaskDto for writing/status-changing tools: echoes back what changed // without re-sending Description/Result, which the caller just sent or already has. @@ -45,7 +53,8 @@ public sealed record TaskRefDto( string Title, string Status, int SortOrder, - bool IsMyDay); + bool IsMyDay, + int RoadblockCount = 0); // tasks is populated when includeDescription=false (the default): lean references, no // Description/Result. tasksFull is populated when includeDescription=true: full tasks incl. @@ -71,11 +80,15 @@ public sealed record MergeContinuationResultDto( bool Merged, string TaskStatus, IReadOnlyList Conflicts, string? RepoPath, string? Message); +// IsEmpty = the review range contributed nothing (worktree ahead-of-base is empty, or a +// worktree-less handler task's HandlerBaseCommit == HandlerHeadCommit) -- distinguishable from +// a merge that is merely small, so an empty branch can't be misread as "changedFileCount: 0 +// means tiny" when it actually means "nothing to review". public sealed record MergePreviewToolDto( - string Status, IReadOnlyList ConflictFiles, int ChangedFileCount, int Behind); + string Status, IReadOnlyList ConflictFiles, int ChangedFileCount, int Behind, bool IsEmpty = false); public sealed record MergePreviewSetEntryDto( - string TaskId, string Status, IReadOnlyList ConflictFiles, int ChangedFileCount, int Behind, string? Error); + string TaskId, string Status, IReadOnlyList ConflictFiles, int ChangedFileCount, int Behind, string? Error, bool IsEmpty = false); public sealed record FileOverlapDto(string File, IReadOnlyList TaskIds); @@ -396,7 +409,10 @@ public sealed class ExternalMcpService "decision='reject_park' → Idle for manual editing (feedback ignored). " + "decision='cancel' → Cancelled. " + "Fails if the task is not currently WaitingForReview (except cancel, which also works while Running/Queued). " + - "The result's task field is a lean reference (id, listId, title, status, sortOrder, isMyDay), not the task's description.")] + "The result's task field is a lean reference (id, listId, title, status, sortOrder, isMyDay), not the task's description. " + + "emptyChildren (parent approve only) lists the Done children about to be unit-merged whose own review range " + + "contributed nothing (e.g. a child that reported CLAUDEDO_BLOCKED and committed no code) — check it before " + + "trusting that every child actually delivered something.")] public async Task ReviewTask( string taskId, string decision, @@ -412,6 +428,7 @@ public sealed class ExternalMcpService IReadOnlyList mergeConflicts = Array.Empty(); string? mergeMessage = null; string? repoPath = null; + IReadOnlyList? emptyChildren = null; if (decision.Trim().ToLowerInvariant() == "approve") { @@ -423,6 +440,10 @@ public sealed class ExternalMcpService 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. + emptyChildren = await GetEmptyDoneChildrenAsync(taskId, cancellationToken); await _planningMerge.StartAsync(taskId, targetBranch ?? "", cancellationToken); var parentDone = (await _tasks.GetByIdAsync(taskId, cancellationToken))!.Status == TaskStatus.Done; mergeStatus = parentDone ? TaskMergeService.StatusMerged : TaskMergeService.StatusConflict; @@ -478,7 +499,51 @@ public sealed class ExternalMcpService return new ReviewTaskResult( ToRefDto((await _tasks.GetByIdAsync(taskId, cancellationToken))!), - mergeStatus, mergeConflicts, mergeMessage, repoPath); + mergeStatus, mergeConflicts, mergeMessage, repoPath, emptyChildren); + } + + // Done children about to be unit-merged whose own review range contributed nothing: an + // active worktree with zero changed files against its base commit, or a worktree-less + // handler child whose HandlerBaseCommit == HandlerHeadCommit. A child with neither (never + // committed anything at all) also counts as empty. Best-effort per child -- a diff failure + // on one child must not block the caller from seeing the others or from approving. + private async Task> GetEmptyDoneChildrenAsync(string parentTaskId, CancellationToken ct) + { + await using var ctx = await _dbFactory.CreateDbContextAsync(ct); + var children = await ctx.Tasks + .AsNoTracking() + .Include(t => t.Worktree) + .Where(t => t.ParentTaskId == parentTaskId && t.Status == TaskStatus.Done) + .ToListAsync(ct); + + var empty = new List(); + foreach (var child in children) + { + if (await IsChildEmptyAsync(child, ct)) + empty.Add(ToRefDto(child)); + } + return empty; + } + + private async Task IsChildEmptyAsync(TaskEntity child, CancellationToken ct) + { + try + { + if (child.Worktree is not null) + { + if (!Directory.Exists(child.Worktree.Path)) return false; + var files = ParseDiffStatFileNames( + await _git.DiffStatAsync(child.Worktree.Path, child.Worktree.BaseCommit, "HEAD", ct)); + return files.Count == 0; + } + if (child.HandlerBaseCommit is { Length: > 0 } handlerBase && child.HandlerHeadCommit is { Length: > 0 } handlerHead) + return string.Equals(handlerBase, handlerHead, StringComparison.Ordinal); + return true; + } + catch + { + return false; + } } [McpServerTool, Description("Immediately run a task in the override execution slot (bypasses the agent queue). Returns { started: true, taskId } on success.")] @@ -800,15 +865,18 @@ public sealed class ExternalMcpService "IMPORTANT: a clean preview says nothing about whether the merged result compiles or passes tests — git " + "can merge two changes cleanly (e.g. one file deletes a symbol another file still references) and still " + "break the build. " + - "Throws a clear error if the task has no worktree, the worktree is not Active, or the list's working " + - "directory is missing from disk.")] + "isEmpty=true means the task's review range contributed nothing (no commits ahead of base, or — for a " + + "worktree-less list-handler host task — HandlerBaseCommit == HandlerHeadCommit); do not mistake a small " + + "changedFileCount for an empty one, check isEmpty instead. " + + "Throws a clear error if the task has neither an active worktree nor a handler commit range, or the " + + "list's working directory is missing from disk.")] public async Task PreviewMerge( string taskId, string? targetBranch = null, CancellationToken cancellationToken = default) { - var (preview, behind, _) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken); - return new MergePreviewToolDto(preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind); + var (preview, behind, _, isEmpty) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken); + return new MergePreviewToolDto(preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, isEmpty); } [McpServerTool, Description( @@ -820,7 +888,8 @@ public sealed class ExternalMcpService "which tasks touch it — passing a single taskId always yields an empty overlaps list. " + "IMPORTANT: file-name overlap is a HINT, not a guarantee of a real collision, and its absence is not a " + "guarantee of safety — two tasks touching different files entirely (e.g. one deletes a symbol, another " + - "still references it elsewhere) can still collide, and this tool will not flag that case.")] + "still references it elsewhere) can still collide, and this tool will not flag that case. " + + "isEmpty=true (per entry) means that task's review range contributed nothing — see preview_merge.")] public async Task PreviewMergeSet( IReadOnlyList taskIds, string? targetBranch = null, @@ -836,9 +905,9 @@ public sealed class ExternalMcpService { try { - var (preview, behind, changedFiles) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken); + var (preview, behind, changedFiles, isEmpty) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken); entries.Add(new MergePreviewSetEntryDto( - taskId, preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, null)); + taskId, preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, null, isEmpty)); filesByTask[taskId] = changedFiles; } catch (InvalidOperationException ex) @@ -861,32 +930,61 @@ public sealed class ExternalMcpService // Shared core for PreviewMerge/PreviewMergeSet: throws a clear InvalidOperationException instead of // TaskMergeService.PreviewAsync's silent "unavailable" status, and adds `behind` + the task's own - // changed-file list (via diff-stat, not the merge-tree preview) for overlap detection. - private async Task<(MergePreviewResult Preview, int Behind, IReadOnlyList ChangedFiles)> PreviewMergeCoreAsync( + // changed-file list (via diff-stat, not the merge-tree preview) for overlap detection, plus + // `isEmpty`. A worktree-less list-handler host task has no branch to merge-tree-preview at all + // (its commits already sit on the list's working dir) — falls back to the fixed + // HandlerBaseCommit..HandlerHeadCommit range, reporting a synthetic "clean" preview of that + // range's own diff-stat instead of throwing "has no worktree". + private async Task<(MergePreviewResult Preview, int Behind, IReadOnlyList ChangedFiles, bool IsEmpty)> PreviewMergeCoreAsync( string taskId, string? targetBranch, CancellationToken ct) { - var (_, list, wt) = await LoadWorktreeContextAsync(taskId, ct); - if (wt.State != WorktreeState.Active) - throw new InvalidOperationException( - $"Worktree state must be Active to preview a merge (current: {wt.State})."); - if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir)) - throw new InvalidOperationException("The list's working directory no longer exists."); + using var ctx = _dbFactory.CreateDbContext(); + var task = await new TaskRepository(ctx).GetByIdAsync(taskId, ct) + ?? throw new InvalidOperationException($"Task {taskId} not found."); + var list = await new ListRepository(ctx).GetByIdAsync(task.ListId, ct) + ?? throw new InvalidOperationException("List not found."); + var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(taskId, ct); - var preview = await _merge.PreviewAsync(taskId, targetBranch ?? "", ct); - if (preview.Status == TaskMergeService.PreviewUnavailable) - throw new InvalidOperationException( - "Merge preview unavailable for this task (worktree inactive or repo is not a git repository)."); + if (wt is not null) + { + if (wt.State != WorktreeState.Active) + throw new InvalidOperationException( + $"Worktree state must be Active to preview a merge (current: {wt.State})."); + if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir)) + throw new InvalidOperationException("The list's working directory no longer exists."); - var target = string.IsNullOrWhiteSpace(targetBranch) - ? await _git.GetCurrentBranchAsync(list.WorkingDir, ct) - : targetBranch; - var behind = await GitRevListCountAsync(list.WorkingDir, $"{wt.BranchName}..{target}", ct); + var preview = await _merge.PreviewAsync(taskId, targetBranch ?? "", ct); + if (preview.Status == TaskMergeService.PreviewUnavailable) + throw new InvalidOperationException( + "Merge preview unavailable for this task (worktree inactive or repo is not a git repository)."); - var changedFiles = Directory.Exists(wt.Path) - ? ParseDiffStatFileNames(await _git.DiffStatAsync(wt.Path, wt.BaseCommit, "HEAD", ct)) - : Array.Empty(); + var target = string.IsNullOrWhiteSpace(targetBranch) + ? await _git.GetCurrentBranchAsync(list.WorkingDir, ct) + : targetBranch; + var behind = await GitRevListCountAsync(list.WorkingDir, $"{wt.BranchName}..{target}", ct); - return (preview, behind, changedFiles); + var changedFiles = Directory.Exists(wt.Path) + ? ParseDiffStatFileNames(await _git.DiffStatAsync(wt.Path, wt.BaseCommit, "HEAD", ct)) + : Array.Empty(); + + return (preview, behind, changedFiles, changedFiles.Count == 0); + } + + if (task.HandlerBaseCommit is { Length: > 0 } handlerBase && task.HandlerHeadCommit is { Length: > 0 } handlerHead) + { + if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir)) + throw new InvalidOperationException("The list's working directory no longer exists."); + + var isEmpty = string.Equals(handlerBase, handlerHead, StringComparison.Ordinal); + var changedFiles = isEmpty + ? Array.Empty() + : ParseDiffStatFileNames(await _git.DiffStatAsync(list.WorkingDir, handlerBase, handlerHead, ct)); + + var preview = new MergePreviewResult(TaskMergeService.PreviewClean, Array.Empty(), changedFiles.Count); + return (preview, 0, changedFiles, isEmpty); + } + + throw new InvalidOperationException($"Task {taskId} has no worktree."); } [McpServerTool, Description( @@ -1165,7 +1263,8 @@ public sealed class ExternalMcpService t.StartedAt, t.FinishedAt, t.IsMyDay, - t.SortOrder); + t.SortOrder, + t.RoadblockCount); private static TaskRefDto ToRefDto(TaskEntity t) => new( t.Id, @@ -1173,7 +1272,8 @@ public sealed class ExternalMcpService t.Title, t.Status.ToString(), t.SortOrder, - t.IsMyDay); + t.IsMyDay, + t.RoadblockCount); } internal static class DailyPrepFilter diff --git a/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs b/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs index a80efb0b..3a16d24d 100644 --- a/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs @@ -367,6 +367,71 @@ public sealed class ExternalMcpServiceTests : IDisposable Assert.Equal(WorktreeState.Merged, verify.Worktrees.Single(w => w.TaskId == childId).State); } + [Fact] + public async Task ReviewTask_Approve_ParentWithChildren_ReportsEmptyChildByName() + { + // A blocked child (CLAUDEDO_BLOCKED, committed nothing) still gets an active worktree at + // its base commit -- TaskRunner creates one before the run starts -- so "empty" shows up + // as zero commits ahead of base, not as a missing worktree. + if (!GitAvailable) return; + var repo = new GitRepoFixture(); + _repos.Add(repo); + GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main"); + + var listId = Guid.NewGuid().ToString(); + var parentId = Guid.NewGuid().ToString(); + var normalChildId = Guid.NewGuid().ToString(); + var blockedChildId = Guid.NewGuid().ToString(); + using (var ctx = _db.CreateContext()) + { + ctx.Lists.Add(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow }); + ctx.Tasks.Add(new TaskEntity { Id = parentId, ListId = listId, Title = "plan", CreatedAt = DateTime.UtcNow, + Status = TaskStatus.WaitingForReview, PlanningPhase = PlanningPhase.Finalized }); + ctx.Tasks.Add(new TaskEntity { Id = normalChildId, ListId = listId, Title = "normal child", CreatedAt = DateTime.UtcNow, + ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 1 }); + ctx.Tasks.Add(new TaskEntity { Id = blockedChildId, ListId = listId, Title = "blocked child", CreatedAt = DateTime.UtcNow, + ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 2, RoadblockCount = 1 }); + + var normalWtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _worktreeCleanups.Add((repo.RepoDir, normalWtPath)); + var normalBranch = $"claudedo/{normalChildId[..8]}"; + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", normalBranch, normalWtPath, repo.BaseCommit); + File.WriteAllText(Path.Combine(normalWtPath, "child.txt"), "c\n"); + GitRepoFixture.RunGit(normalWtPath, "add", "child.txt"); + GitRepoFixture.RunGit(normalWtPath, "commit", "-m", "add child.txt"); + ctx.Worktrees.Add(new WorktreeEntity + { + TaskId = normalChildId, Path = normalWtPath, BranchName = normalBranch, + BaseCommit = repo.BaseCommit, + HeadCommit = GitRepoFixture.RunGit(normalWtPath, "rev-parse", "HEAD").Trim(), + State = WorktreeState.Active, CreatedAt = DateTime.UtcNow, + }); + + var blockedWtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _worktreeCleanups.Add((repo.RepoDir, blockedWtPath)); + var blockedBranch = $"claudedo/{blockedChildId[..8]}"; + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", blockedBranch, blockedWtPath, repo.BaseCommit); + ctx.Worktrees.Add(new WorktreeEntity + { + TaskId = blockedChildId, Path = blockedWtPath, BranchName = blockedBranch, + BaseCommit = repo.BaseCommit, + HeadCommit = repo.BaseCommit, + State = WorktreeState.Active, CreatedAt = DateTime.UtcNow, + }); + await ctx.SaveChangesAsync(); + } + + var sut = BuildSut(CreateQueue()); + var result = await sut.ReviewTask(parentId, "approve", null, "main", cancellationToken: CancellationToken.None); + + Assert.Equal(TaskMergeService.StatusMerged, result.MergeStatus); + Assert.Equal("Done", result.Task.Status); + Assert.NotNull(result.EmptyChildren); + var empty = Assert.Single(result.EmptyChildren!); + Assert.Equal(blockedChildId, empty.Id); + Assert.DoesNotContain(result.EmptyChildren!, c => c.Id == normalChildId); + } + [Fact] public async Task DeleteTask_RemovesTask() { @@ -1424,10 +1489,89 @@ public sealed class ExternalMcpServiceTests : IDisposable Assert.Empty(result.ConflictFiles); Assert.Equal(1, result.ChangedFileCount); Assert.Equal(0, result.Behind); + Assert.False(result.IsEmpty); Assert.Equal(headBefore, await git.RevParseHeadAsync(list.WorkingDir!, CancellationToken.None)); Assert.False(await git.HasChangesAsync(list.WorkingDir!, CancellationToken.None)); } + [Fact] + public async Task PreviewMerge_EmptyWorktree_ReturnsIsEmptyTrue() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + // Worktree created but nothing committed beyond base -- e.g. a child that reported + // CLAUDEDO_BLOCKED before writing any code. + var (task, list, _) = await SeedWorktreeAsync(TaskStatus.WaitingForReview); + var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir!, CancellationToken.None); + var sut = BuildSut(CreateQueue()); + + var result = await sut.PreviewMerge(task.Id, target, CancellationToken.None); + + Assert.Equal(TaskMergeService.PreviewClean, result.Status); + Assert.Equal(0, result.ChangedFileCount); + Assert.True(result.IsEmpty); + } + + [Fact] + public async Task PreviewMerge_WorktreeLessHandlerTask_EmptyRange_ReturnsIsEmptyTrue() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var repo = new GitRepoFixture(); + _repos.Add(repo); + var listId = Guid.NewGuid().ToString(); + await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow }); + var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview); + // Worktree-less list-handler host task whose handler range never actually committed + // anything: HandlerBaseCommit == HandlerHeadCommit, not "no worktree -> unknown". + using (var ctx = _db.CreateContext()) + { + var t = await ctx.Tasks.FindAsync(task.Id); + t!.HandlerBaseCommit = repo.BaseCommit; + t.HandlerHeadCommit = repo.BaseCommit; + await ctx.SaveChangesAsync(); + } + + var sut = BuildSut(CreateQueue()); + var result = await sut.PreviewMerge(task.Id, null, CancellationToken.None); + + Assert.Equal(TaskMergeService.PreviewClean, result.Status); + Assert.Equal(0, result.ChangedFileCount); + Assert.True(result.IsEmpty); + } + + [Fact] + public async Task PreviewMerge_WorktreeLessHandlerTask_NonEmptyRange_ReturnsIsEmptyFalse() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var repo = new GitRepoFixture(); + _repos.Add(repo); + var listId = Guid.NewGuid().ToString(); + await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow }); + var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview); + + File.WriteAllText(Path.Combine(repo.RepoDir, "handled.txt"), "content"); + GitRepoFixture.RunGit(repo.RepoDir, "add", "-A"); + GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "handled task merge"); + var headCommit = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim(); + + using (var ctx = _db.CreateContext()) + { + var t = await ctx.Tasks.FindAsync(task.Id); + t!.HandlerBaseCommit = repo.BaseCommit; + t.HandlerHeadCommit = headCommit; + await ctx.SaveChangesAsync(); + } + + var sut = BuildSut(CreateQueue()); + var result = await sut.PreviewMerge(task.Id, null, CancellationToken.None); + + Assert.Equal(TaskMergeService.PreviewClean, result.Status); + Assert.Equal(1, result.ChangedFileCount); + Assert.False(result.IsEmpty); + } + [Fact] public async Task PreviewMerge_ConflictingBranches_ReturnsConflictStatusAndDoesNotChangeRepo() { @@ -1546,6 +1690,49 @@ public sealed class ExternalMcpServiceTests : IDisposable Assert.Empty(result.Overlaps); } + [Fact] + public async Task PreviewMergeSet_MixedEmptyAndNonEmpty_ReportsIsEmptyPerEntry() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var repo = new GitRepoFixture(); + _repos.Add(repo); + GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main"); + + var listId = Guid.NewGuid().ToString(); + await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow }); + var emptyTask = await SeedTaskAsync(listId, "empty", TaskStatus.WaitingForReview); + var fullTask = await SeedTaskAsync(listId, "full", TaskStatus.WaitingForReview); + + var emptyBranch = $"claudedo/{emptyTask.Id[..8]}"; + var emptyWtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _worktreeCleanups.Add((repo.RepoDir, emptyWtPath)); + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", emptyBranch, emptyWtPath, repo.BaseCommit); + using (var ctx = _db.CreateContext()) + { + ctx.Worktrees.Add(new WorktreeEntity + { + TaskId = emptyTask.Id, Path = emptyWtPath, BranchName = emptyBranch, + BaseCommit = repo.BaseCommit, + HeadCommit = repo.BaseCommit, + State = WorktreeState.Active, CreatedAt = DateTime.UtcNow, + }); + await ctx.SaveChangesAsync(); + } + + await CreateActiveWorktreeAsync(repo, fullTask.Id, "full.txt", "content\n"); + + var sut = BuildSut(CreateQueue()); + var result = await sut.PreviewMergeSet([emptyTask.Id, fullTask.Id], "main", CancellationToken.None); + + var emptyEntry = result.Tasks.Single(t => t.TaskId == emptyTask.Id); + var fullEntry = result.Tasks.Single(t => t.TaskId == fullTask.Id); + Assert.Null(emptyEntry.Error); + Assert.True(emptyEntry.IsEmpty); + Assert.Null(fullEntry.Error); + Assert.False(fullEntry.IsEmpty); + } + // ── AddTask model override ──────────────────────────────────────────────── [Fact]