diff --git a/src/ClaudeDo.Worker/External/ExternalMcpService.cs b/src/ClaudeDo.Worker/External/ExternalMcpService.cs index da457ba2..69fa00f6 100644 --- a/src/ClaudeDo.Worker/External/ExternalMcpService.cs +++ b/src/ClaudeDo.Worker/External/ExternalMcpService.cs @@ -19,7 +19,7 @@ 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(TaskDto Task, string? MergeStatus, IReadOnlyList MergeConflicts, string? MergeMessage); +public sealed record ReviewTaskResult(TaskDto Task, string? MergeStatus, IReadOnlyList MergeConflicts, string? MergeMessage, string? RepoPath = null); public sealed record StatusValueDto(string Status, string Meaning); public sealed record TaskDto( @@ -44,7 +44,8 @@ public sealed record TaskDiffDto( string Content, IReadOnlyList Files, bool Truncated, int TotalBytes); public sealed record MergeTaskResultDto( - bool Merged, string? MergeCommit, IReadOnlyList Conflicts); + bool Merged, string? MergeCommit, IReadOnlyList Conflicts, + bool ConflictsInTree = false, string? RepoPath = null); public sealed record WorktreeListItemDto( string? TaskId, string Path, string Branch, @@ -300,7 +301,10 @@ public sealed class ExternalMcpService "decision='approve' → review+merge, exactly like the UI's Approve: a childless task merges its worktree into " + "targetBranch (default: the repo's current branch) 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. " + - "mergeStatus 'conflict' means the merge stopped on conflicts (files listed) — resolve them in the ClaudeDo UI. " + + "mergeStatus 'conflict' means the merge stopped on conflicts (files listed) — by default the merge is cleanly " + + "aborted and you resolve in the ClaudeDo UI; pass leaveConflictsInTree=true to instead leave the conflict " + + "markers in the working tree (repoPath in the result) so you can resolve them and call continue_merge, " + + "or abort_merge to cancel. " + "decision='reject_rerun' → Queued and re-runs, resuming the agent's session with your feedback as the next turn (feedback is required). " + "decision='reject_park' → Idle for manual editing (feedback ignored). " + "decision='cancel' → Cancelled. " + @@ -310,14 +314,16 @@ public sealed class ExternalMcpService string decision, string? feedback = null, string? targetBranch = null, + bool leaveConflictsInTree = false, CancellationToken cancellationToken = default) { - _ = await _tasks.GetByIdAsync(taskId, cancellationToken) + var task = await _tasks.GetByIdAsync(taskId, cancellationToken) ?? throw new InvalidOperationException($"Task {taskId} not found."); string? mergeStatus = null; IReadOnlyList mergeConflicts = Array.Empty(); string? mergeMessage = null; + string? repoPath = null; if (decision.Trim().ToLowerInvariant() == "approve") { @@ -333,18 +339,38 @@ public sealed class ExternalMcpService var parentDone = (await _tasks.GetByIdAsync(taskId, cancellationToken))!.Status == TaskStatus.Done; mergeStatus = parentDone ? TaskMergeService.StatusMerged : TaskMergeService.StatusConflict; if (!parentDone) - mergeMessage = "unit merge paused on a conflict — resolve and continue it in the ClaudeDo UI"; + { + var list = await _lists.GetByIdAsync(task.ListId, cancellationToken); + repoPath = list?.WorkingDir; + mergeMessage = "unit merge paused on a conflict — markers left in the working tree; " + + "resolve them then call continue_merge with the parent task id, or abort_merge to cancel"; + } } else { - var r = await _merge.ApproveAndMergeAsync(taskId, targetBranch ?? "", cancellationToken); + var r = await _merge.ApproveAndMergeAsync(taskId, targetBranch ?? "", leaveConflictsInTree, cancellationToken); if (r.Status == TaskMergeService.StatusBlocked) throw new InvalidOperationException(r.ErrorMessage ?? "approve failed"); mergeStatus = r.Status; mergeConflicts = r.ConflictFiles; - mergeMessage = r.Status == TaskMergeService.StatusConflict - ? "merge conflict — the task stays WaitingForReview; resolve it in the ClaudeDo UI" - : r.ErrorMessage; + if (r.Status == TaskMergeService.StatusConflict) + { + if (leaveConflictsInTree) + { + var list = await _lists.GetByIdAsync(task.ListId, cancellationToken); + repoPath = list?.WorkingDir; + mergeMessage = "merge conflict — markers left in the working tree; " + + "resolve them then call continue_merge, or abort_merge to cancel"; + } + else + { + mergeMessage = "merge conflict — the task stays WaitingForReview; resolve it in the ClaudeDo UI"; + } + } + else + { + mergeMessage = r.ErrorMessage; + } } } else @@ -364,7 +390,7 @@ public sealed class ExternalMcpService return new ReviewTaskResult( ToDto((await _tasks.GetByIdAsync(taskId, cancellationToken))!), - mergeStatus, mergeConflicts, mergeMessage); + mergeStatus, mergeConflicts, mergeMessage, repoPath); } [McpServerTool, Description("Immediately run a task in the override execution slot (bypasses the agent queue).")] @@ -484,13 +510,17 @@ public sealed class ExternalMcpService "dryRun=true: validates preconditions only, does not perform the merge; merged=false in the result means 'not actually merged'. " + "allowWaitingForReview=true: also allows merging a task in WaitingForReview (default false, which only allows Done). " + "On success: merged=true, mergeCommit contains the new merge commit SHA. " + - "On conflict: the merge is cleanly aborted (no half-merged state left); merged=false and conflicts lists the affected files.")] + "On conflict: by default the merge is cleanly aborted (no half-merged state left); merged=false and conflicts lists the affected files. " + + "leaveConflictsInTree=true: on conflict the merge is NOT aborted — conflict markers are left in the working " + + "tree at repoPath (conflictsInTree=true in the result) so you can resolve them there and call continue_merge, " + + "or abort_merge to cancel.")] public async Task MergeTask( string taskId, string targetBranch = "main", bool noFf = true, bool dryRun = false, bool allowWaitingForReview = false, + bool leaveConflictsInTree = false, CancellationToken cancellationToken = default) { var task = await _tasks.GetByIdAsync(taskId, cancellationToken) @@ -517,7 +547,7 @@ public sealed class ExternalMcpService var commitMessage = $"Merge task branch for: {task.Title}"; var result = await _merge.MergeAsync( - taskId, targetBranch, removeWorktree: false, commitMessage, cancellationToken); + taskId, targetBranch, removeWorktree: false, commitMessage, leaveConflictsInTree, cancellationToken); if (result.Status == TaskMergeService.StatusMerged) { @@ -532,7 +562,10 @@ public sealed class ExternalMcpService } if (result.Status == TaskMergeService.StatusConflict) - return new MergeTaskResultDto(false, null, result.ConflictFiles); + return leaveConflictsInTree + ? new MergeTaskResultDto(false, null, result.ConflictFiles, + ConflictsInTree: true, RepoPath: list?.WorkingDir) + : new MergeTaskResultDto(false, null, result.ConflictFiles); throw new InvalidOperationException(result.ErrorMessage ?? $"Merge blocked: {result.Status}"); } diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index 45bf152a..2c1a8574 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -342,7 +342,11 @@ public sealed class TaskMergeService return new MergePreviewResult(PreviewClean, Array.Empty(), count); } - public async Task ApproveAndMergeAsync(string taskId, string targetBranch, CancellationToken ct) + public Task ApproveAndMergeAsync(string taskId, string targetBranch, CancellationToken ct) + => ApproveAndMergeAsync(taskId, targetBranch, leaveConflictsInTree: false, ct); + + public async Task ApproveAndMergeAsync( + string taskId, string targetBranch, bool leaveConflictsInTree, CancellationToken ct) { var (task, list, wt) = await LoadMergeContextAsync(taskId, ct); @@ -367,7 +371,7 @@ public sealed class TaskMergeService // MergeAsync transitions the task WaitingForReview -> Done on a successful merge. // Remove the worktree on approve (matching the unit-merge path) so merged // worktrees don't pile up; the merge commit on the target branch is the record. - return await MergeAsync(taskId, target, removeWorktree: true, $"Merge {wt.BranchName}", ct); + return await MergeAsync(taskId, target, removeWorktree: true, $"Merge {wt.BranchName}", leaveConflictsInTree, ct); } private static MergeResult Blocked(string reason) => diff --git a/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs b/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs index 1d88205e..6127d9cc 100644 --- a/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs @@ -216,7 +216,7 @@ public sealed class ExternalMcpServiceTests : IDisposable var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview); var sut = BuildSut(CreateQueue()); - var result = await sut.ReviewTask(task.Id, "approve", null, null, CancellationToken.None); + var result = await sut.ReviewTask(task.Id, "approve", null, null, cancellationToken: CancellationToken.None); Assert.Equal("Done", result.Task.Status); Assert.Equal(TaskMergeService.StatusMerged, result.MergeStatus); @@ -230,7 +230,7 @@ public sealed class ExternalMcpServiceTests : IDisposable var sut = BuildSut(CreateQueue()); await Assert.ThrowsAsync(() => - sut.ReviewTask(task.Id, "reject_rerun", null, null, CancellationToken.None)); + sut.ReviewTask(task.Id, "reject_rerun", null, null, cancellationToken: CancellationToken.None)); } [Fact] @@ -240,7 +240,7 @@ public sealed class ExternalMcpServiceTests : IDisposable var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview); var sut = BuildSut(CreateQueue()); - var result = await sut.ReviewTask(task.Id, "reject_rerun", "fix it", null, CancellationToken.None); + var result = await sut.ReviewTask(task.Id, "reject_rerun", "fix it", null, cancellationToken: CancellationToken.None); Assert.Equal("Queued", result.Task.Status); var loaded = await new TaskRepository(_db.CreateContext()).GetByIdAsync(task.Id); @@ -255,7 +255,7 @@ public sealed class ExternalMcpServiceTests : IDisposable var sut = BuildSut(CreateQueue()); await Assert.ThrowsAsync(() => - sut.ReviewTask(task.Id, "bogus", null, null, CancellationToken.None)); + sut.ReviewTask(task.Id, "bogus", null, null, cancellationToken: CancellationToken.None)); } [Fact] @@ -275,7 +275,7 @@ public sealed class ExternalMcpServiceTests : IDisposable } var sut = BuildSut(CreateQueue()); - var result = await sut.ReviewTask(task.Id, "approve", null, null, CancellationToken.None); + var result = await sut.ReviewTask(task.Id, "approve", null, null, cancellationToken: CancellationToken.None); Assert.Equal(TaskMergeService.StatusMerged, result.MergeStatus); Assert.Equal("Done", result.Task.Status); @@ -321,7 +321,7 @@ public sealed class ExternalMcpServiceTests : IDisposable } var sut = BuildSut(CreateQueue()); - var result = await sut.ReviewTask(parentId, "approve", null, "main", CancellationToken.None); + var result = await sut.ReviewTask(parentId, "approve", null, "main", cancellationToken: CancellationToken.None); Assert.Equal(TaskMergeService.StatusMerged, result.MergeStatus); Assert.Equal("Done", result.Task.Status); @@ -610,7 +610,7 @@ public sealed class ExternalMcpServiceTests : IDisposable var sut = BuildSut(CreateQueue()); var ex = await Assert.ThrowsAsync( - () => sut.MergeTask(task.Id, "main", true, false, false, CancellationToken.None)); + () => sut.MergeTask(task.Id, "main", true, false, false, cancellationToken: CancellationToken.None)); Assert.Contains("Done", ex.Message); } @@ -785,7 +785,7 @@ public sealed class ExternalMcpServiceTests : IDisposable var sut = BuildSut(CreateQueue()); var ex = await Assert.ThrowsAsync( - () => sut.MergeTask(task.Id, "main", true, false, false, CancellationToken.None)); + () => sut.MergeTask(task.Id, "main", true, false, false, cancellationToken: CancellationToken.None)); Assert.Contains("Done", ex.Message); } @@ -797,7 +797,7 @@ public sealed class ExternalMcpServiceTests : IDisposable var (task, _, _) = await SeedWorktreeAsync(TaskStatus.WaitingForReview); var sut = BuildSut(CreateQueue()); - var result = await sut.MergeTask(task.Id, "main", true, dryRun: true, allowWaitingForReview: true, CancellationToken.None); + var result = await sut.MergeTask(task.Id, "main", true, dryRun: true, allowWaitingForReview: true, cancellationToken: CancellationToken.None); Assert.False(result.Merged); Assert.Null(result.MergeCommit); @@ -817,13 +817,114 @@ public sealed class ExternalMcpServiceTests : IDisposable var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir, CancellationToken.None); var sut = BuildSut(CreateQueue()); - var result = await sut.MergeTask(task.Id, target, true, dryRun: false, allowWaitingForReview: true, CancellationToken.None); + var result = await sut.MergeTask(task.Id, target, true, dryRun: false, allowWaitingForReview: true, cancellationToken: CancellationToken.None); Assert.True(result.Merged); var reloaded = await new TaskRepository(_db.CreateContext()).GetByIdAsync(task.Id); Assert.Equal(TaskStatus.Done, reloaded!.Status); } + // ── leaveConflictsInTree ────────────────────────────────────────────────── + + private async Task<(TaskEntity task, ListEntity list, WorktreeContext wt)> SeedConflictingWorktreeAsync( + TaskStatus status = TaskStatus.WaitingForReview) + { + var (task, list, wt) = await SeedWorktreeAsync(status); + + File.WriteAllText(Path.Combine(wt.WorktreePath, "README.md"), "# from worktree\n"); + GitRepoFixture.RunGit(wt.WorktreePath, "add", "README.md"); + GitRepoFixture.RunGit(wt.WorktreePath, "commit", "-m", "worktree edit"); + + File.WriteAllText(Path.Combine(list.WorkingDir!, "README.md"), "# from main\n"); + GitRepoFixture.RunGit(list.WorkingDir!, "add", "README.md"); + GitRepoFixture.RunGit(list.WorkingDir!, "commit", "-m", "main edit"); + + return (task, list, wt); + } + + [Fact] + public async Task MergeTask_LeaveConflictsInTree_LeavesMarkersAndKeepsRepoMidMerge() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, list, _) = await SeedConflictingWorktreeAsync(TaskStatus.WaitingForReview); + var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir!); + var sut = BuildSut(CreateQueue()); + + var result = await sut.MergeTask(task.Id, target, true, dryRun: false, + allowWaitingForReview: true, leaveConflictsInTree: true, CancellationToken.None); + + Assert.False(result.Merged); + Assert.True(result.ConflictsInTree); + Assert.Equal(list.WorkingDir, result.RepoPath); + Assert.Contains("README.md", result.Conflicts); + Assert.Contains("<<<<<<<", File.ReadAllText(Path.Combine(list.WorkingDir!, "README.md"))); + Assert.True(await new GitService().IsMidMergeAsync(list.WorkingDir!)); + var reloaded = await new TaskRepository(_db.CreateContext()).GetByIdAsync(task.Id); + Assert.Equal(TaskStatus.WaitingForReview, reloaded!.Status); + + GitRepoFixture.RunGit(list.WorkingDir!, "merge", "--abort"); + } + + [Fact] + public async Task MergeTask_ConflictWithoutLeaveFlag_AbortsCleanly() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, list, _) = await SeedConflictingWorktreeAsync(TaskStatus.WaitingForReview); + var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir!); + var sut = BuildSut(CreateQueue()); + + var result = await sut.MergeTask(task.Id, target, true, dryRun: false, + allowWaitingForReview: true, leaveConflictsInTree: false, CancellationToken.None); + + Assert.False(result.Merged); + Assert.False(result.ConflictsInTree); + Assert.Null(result.RepoPath); + Assert.Contains("README.md", result.Conflicts); + Assert.DoesNotContain("<<<<<<<", File.ReadAllText(Path.Combine(list.WorkingDir!, "README.md"))); + Assert.False(await new GitService().IsMidMergeAsync(list.WorkingDir!)); + } + + [Fact] + public async Task ReviewTask_Approve_LeaveConflictsInTree_LeavesMarkersAndReportsRepoPath() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, list, _) = await SeedConflictingWorktreeAsync(TaskStatus.WaitingForReview); + var sut = BuildSut(CreateQueue()); + + var result = await sut.ReviewTask(task.Id, "approve", null, null, + leaveConflictsInTree: true, CancellationToken.None); + + Assert.Equal(TaskMergeService.StatusConflict, result.MergeStatus); + Assert.Equal(list.WorkingDir, result.RepoPath); + Assert.Contains("continue_merge", result.MergeMessage); + Assert.Contains("<<<<<<<", File.ReadAllText(Path.Combine(list.WorkingDir!, "README.md"))); + Assert.True(await new GitService().IsMidMergeAsync(list.WorkingDir!)); + Assert.Equal("WaitingForReview", result.Task.Status); + + GitRepoFixture.RunGit(list.WorkingDir!, "merge", "--abort"); + } + + [Fact] + public async Task ReviewTask_Approve_ConflictWithoutLeaveFlag_KeepsOldBehaviour() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, list, _) = await SeedConflictingWorktreeAsync(TaskStatus.WaitingForReview); + var sut = BuildSut(CreateQueue()); + + var result = await sut.ReviewTask(task.Id, "approve", null, null, cancellationToken: CancellationToken.None); + + Assert.Equal(TaskMergeService.StatusConflict, result.MergeStatus); + Assert.Null(result.RepoPath); + Assert.Contains("ClaudeDo UI", result.MergeMessage); + Assert.DoesNotContain("<<<<<<<", File.ReadAllText(Path.Combine(list.WorkingDir!, "README.md"))); + Assert.False(await new GitService().IsMidMergeAsync(list.WorkingDir!)); + Assert.Equal("WaitingForReview", result.Task.Status); + } + // ── AddTask model override ──────────────────────────────────────────────── [Fact]