diff --git a/src/ClaudeDo.Data/Git/ConflictMarkerParser.cs b/src/ClaudeDo.Data/Git/ConflictMarkerParser.cs index 6238acb6..11f6180c 100644 --- a/src/ClaudeDo.Data/Git/ConflictMarkerParser.cs +++ b/src/ClaudeDo.Data/Git/ConflictMarkerParser.cs @@ -25,10 +25,13 @@ public sealed record MergeSegment /// "Theirs" side (the incoming branch) when is true. public string Theirs { get; init; } = ""; - public static MergeSegment Stable(string text) => new() { Text = text }; + /// 1-based line number in the file where this segment starts (the marker line for a conflict). + public int StartLine { get; init; } = 1; - public static MergeSegment Conflict(string ours, string? @base, string theirs) => - new() { IsConflict = true, Ours = ours, Base = @base, Theirs = theirs }; + public static MergeSegment Stable(string text, int startLine = 1) => new() { Text = text, StartLine = startLine }; + + public static MergeSegment Conflict(string ours, string? @base, string theirs, int startLine = 1) => + new() { IsConflict = true, Ours = ours, Base = @base, Theirs = theirs, StartLine = startLine }; } /// @@ -49,22 +52,25 @@ public static class ConflictMarkerParser var segments = new List(); var lines = SplitKeepLineEndings(fileText); var stable = new StringBuilder(); + var stableStartLine = 1; var i = 0; while (i < lines.Count) { if (!IsMarker(lines[i], OursMarker)) { + if (stable.Length == 0) stableStartLine = i + 1; stable.Append(lines[i++]); continue; } if (stable.Length > 0) { - segments.Add(MergeSegment.Stable(stable.ToString())); + segments.Add(MergeSegment.Stable(stable.ToString(), stableStartLine)); stable.Clear(); } + var conflictStartLine = i + 1; i++; // consume "<<<<<<<" var ours = new StringBuilder(); while (i < lines.Count && !IsMarker(lines[i], BaseMarker) && !IsMarker(lines[i], SepMarker)) @@ -88,11 +94,11 @@ public static class ConflictMarkerParser if (i < lines.Count && IsMarker(lines[i], TheirsMarker)) i++; // consume ">>>>>>>" - segments.Add(MergeSegment.Conflict(ours.ToString(), @base, theirs.ToString())); + segments.Add(MergeSegment.Conflict(ours.ToString(), @base, theirs.ToString(), conflictStartLine)); } if (stable.Length > 0) - segments.Add(MergeSegment.Stable(stable.ToString())); + segments.Add(MergeSegment.Stable(stable.ToString(), stableStartLine)); return segments; } diff --git a/src/ClaudeDo.Worker/External/ExternalMcpService.cs b/src/ClaudeDo.Worker/External/ExternalMcpService.cs index 23fc6319..d92f268f 100644 --- a/src/ClaudeDo.Worker/External/ExternalMcpService.cs +++ b/src/ClaudeDo.Worker/External/ExternalMcpService.cs @@ -88,6 +88,18 @@ public sealed record MergeContinuationResultDto( bool Merged, string TaskStatus, IReadOnlyList Conflicts, string? RepoPath, string? Message); +public sealed record ConflictHunkDto( + int Index, int StartLine, string Ours, string? Base, string Theirs); + +public sealed record ConflictFileHunksDto( + string Path, bool IsBinary, IReadOnlyList Hunks); + +public sealed record GetMergeConflictsResultDto( + IReadOnlyList Files, int RemainingHunks, string ConflictStyleNote = McpToolDocs.Diff3Note); + +public sealed record ResolveConflictHunkResultDto( + bool Resolved, int RemainingHunks, string ConflictStyleNote = McpToolDocs.Diff3Note); + // 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 @@ -927,6 +939,107 @@ public sealed class ExternalMcpService return ToRefDto((await _tasks.GetByIdAsync(taskId, cancellationToken))!); } + [McpServerTool, Description( + "List every conflict hunk left in a paused merge (started via merge_task/review_task with " + + "leaveConflictsInTree=true), so you can resolve them with resolve_conflict_hunk instead of parsing marker " + + "text yourself. Each hunk carries the file's path, its index within that file (what resolve_conflict_hunk " + + "expects back), and its startLine. remainingHunks is the total across every file — call continue_merge once " + + "it reaches 0, or abort_merge to cancel. Throws if the task has no in-progress merge." + " " + McpToolDocs.Diff3Note)] + public async Task GetMergeConflicts(string taskId, CancellationToken cancellationToken = default) + { + _ = await _tasks.GetByIdAsync(taskId, cancellationToken) + ?? throw new InvalidOperationException($"Task {taskId} not found."); + + var docs = await _merge.GetConflictDocumentsAsync(taskId, cancellationToken); + var files = docs.Files.Select(ToConflictFileHunksDto).ToList(); + var remaining = files.Sum(f => f.Hunks.Count); + return new GetMergeConflictsResultDto(files, remaining); + } + + [McpServerTool, Description( + "Resolve exactly one conflict hunk from get_merge_conflicts, identified by file path and its index within " + + "that file. Writes only that hunk — every other hunk (in this file or another) is left exactly as it was, " + + "so call get_merge_conflicts again afterward: resolved hunks disappear and every remaining hunk's index " + + "shifts down to stay zero-based. This never decides FOR you which side wins — resolution must be 'ours', " + + "'theirs', 'base' (only valid when that hunk has a diff3 base), or literal replacement text (used exactly " + + "as given, including any trailing newline the surrounding file needs). remainingHunks is the total across " + + "every file; call continue_merge once it reaches 0. Throws if the file/index don't match a current hunk." + + " " + McpToolDocs.Diff3Note)] + public async Task ResolveConflictHunk( + string taskId, + [Description("File path exactly as returned by get_merge_conflicts, relative to the repo root.")] + string file, + [Description("Zero-based hunk index within that file, as returned by get_merge_conflicts.")] + int index, + [Description("'ours', 'theirs', 'base', or literal text to use for this hunk.")] + string resolution, + CancellationToken cancellationToken = default) + { + if (string.IsNullOrEmpty(resolution)) + throw new InvalidOperationException("resolution is required."); + + var docs = await _merge.GetConflictDocumentsAsync(taskId, cancellationToken); + var doc = docs.Files.FirstOrDefault(f => string.Equals(f.Path, file, StringComparison.Ordinal)) + ?? throw new InvalidOperationException($"File '{file}' has no conflict for task {taskId}."); + if (doc.IsBinary) + throw new InvalidOperationException($"File '{file}' is binary; resolve it directly in the working tree."); + + var hunkCount = doc.Segments.Count(s => s.IsConflict); + if (index < 0 || index >= hunkCount) + throw new InvalidOperationException( + $"Hunk index {index} is out of range for '{file}' ({hunkCount} hunk(s) remaining)."); + + var counter = -1; + var composed = ConflictMarkerParser.Compose(doc.Segments, seg => + { + counter++; + if (counter != index) return ReconstructConflictMarkers(seg); + + var choice = resolution.Trim().ToLowerInvariant(); + return choice switch + { + "ours" => seg.Ours, + "theirs" => seg.Theirs, + "base" => seg.Base ?? throw new InvalidOperationException( + $"Hunk {index} in '{file}' has no diff3 base to resolve to."), + _ => resolution, + }; + }); + + await _merge.WriteConflictFileAsync(taskId, file, composed, cancellationToken); + + var refreshed = await _merge.GetConflictDocumentsAsync(taskId, cancellationToken); + var remaining = refreshed.Files.Sum(f => f.Segments.Count(s => s.IsConflict)); + return new ResolveConflictHunkResultDto(true, remaining); + } + + private static ConflictFileHunksDto ToConflictFileHunksDto(ConflictDocumentContent f) + { + var hunks = new List(); + foreach (var seg in f.Segments) + { + if (!seg.IsConflict) continue; + hunks.Add(new ConflictHunkDto(hunks.Count, seg.StartLine, seg.Ours, seg.Base, seg.Theirs)); + } + return new ConflictFileHunksDto(f.Path, f.IsBinary, hunks); + } + + // Re-renders a still-unresolved hunk as valid (unlabeled) conflict markers so it round-trips through + // ConflictMarkerParser.Parse again untouched, and git still refuses continue_merge while it stands. The + // original marker labels (e.g. "<<<<<<< HEAD") aren't recoverable from a parsed MergeSegment and aren't + // needed for either check -- ConflictMarkerParser.IsMarker and ContinueMergeAsync's rescan only match the + // marker prefixes. + private static string ReconstructConflictMarkers(MergeSegment seg) + { + var sb = new StringBuilder(); + sb.Append("<<<<<<<\n").Append(seg.Ours); + if (seg.Base is not null) + sb.Append("|||||||\n").Append(seg.Base); + sb.Append("=======\n").Append(seg.Theirs); + sb.Append(">>>>>>>\n"); + return sb.ToString(); + } + [McpServerTool, Description( "Check whether a task would merge cleanly before touching anything — `git merge-tree --write-tree`, so the " + "working tree, index and HEAD are untouched. status is 'clean' or 'conflict' (conflictFiles lists where git " + diff --git a/src/ClaudeDo.Worker/External/McpToolDocs.cs b/src/ClaudeDo.Worker/External/McpToolDocs.cs index 46b814af..6d6c7d12 100644 --- a/src/ClaudeDo.Worker/External/McpToolDocs.cs +++ b/src/ClaudeDo.Worker/External/McpToolDocs.cs @@ -25,4 +25,15 @@ internal static class McpToolDocs /// Mutations that refuse to touch a task while its agent is running. public const string NotWhileRunning = " Refused while the task is Running — cancel it first."; + + /// + /// Explains this project's diff3 conflict-marker layout so a caller grepping only for + /// "<<<<<<</=======/>>>>>>>" doesn't mistake the "|||||||" base section for one of the two sides. + /// No leading space (unlike the other consts here) since it also doubles as a DTO field default value. + /// + public const string Diff3Note = + "Conflicts are parsed in git's diff3 style: each hunk has an 'ours' side (the target branch) and a " + + "'theirs' side (the branch being merged in), plus an optional 'base' — the common-ancestor text for that " + + "hunk, shown between a third '|||||||' marker and the '=======' separator. base is null when git recorded " + + "none for that hunk."; } diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index e98c1c05..c5a02f4c 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -488,6 +488,24 @@ public sealed class TaskMergeService await _git.AddPathAsync(list.WorkingDir, path, ct); } + /// + /// Writes a conflicted file's content without staging it. Unlike this must + /// NOT run `git add` — that marks the path resolved in git's index regardless of its content, so a partially + /// resolved hunk (markers still present elsewhere in the file) would drop out of + /// 's conflicted-file list and 's + /// pre-stage marker scan, letting a still-conflicted file slip into a commit. Staging happens once, for + /// everything, inside . + /// + public async Task WriteConflictFileAsync(string taskId, string path, string content, CancellationToken ct) + { + var (_, list, _, _) = await LoadMergeContextAsync(taskId, ct); + if (string.IsNullOrWhiteSpace(list.WorkingDir)) + throw new InvalidOperationException("list has no working directory"); + + var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar)); + await File.WriteAllTextAsync(full, content, ct); + } + public async Task GetTargetsAsync(string taskId, CancellationToken ct) { var (_, list, _, _) = await LoadMergeContextAsync(taskId, ct); diff --git a/tests/ClaudeDo.Data.Tests/ConflictMarkerParserTests.cs b/tests/ClaudeDo.Data.Tests/ConflictMarkerParserTests.cs index 149a8888..35437fe6 100644 --- a/tests/ClaudeDo.Data.Tests/ConflictMarkerParserTests.cs +++ b/tests/ClaudeDo.Data.Tests/ConflictMarkerParserTests.cs @@ -68,6 +68,20 @@ public class ConflictMarkerParserTests Assert.Equal("Y\n", segments[1].Theirs); } + [Fact] + public void Segments_ExposeStartLine() + { + const string text = + "line1\nline2\n<<<<<<< HEAD\nours\n=======\ntheirs\n>>>>>>> branch\nline3\n"; + + var segments = ConflictMarkerParser.Parse(text); + + Assert.Equal(3, segments.Count); + Assert.Equal(1, segments[0].StartLine); // "line1\nline2\n" + Assert.Equal(3, segments[1].StartLine); // the "<<<<<<<" line + Assert.Equal(8, segments[2].StartLine); // "line3\n" + } + [Fact] public void MultipleConflicts_AreEachCaptured() { diff --git a/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs b/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs index 4f7ef325..b3d529dd 100644 --- a/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs @@ -1124,6 +1124,49 @@ public sealed class ExternalMcpServiceTests : IDisposable return (task, list, wt); } + // Three edits spread far enough apart in a 20-line file that git's 3-way merge keeps them as three + // separate, non-adjacent hunks instead of folding them into one. + private static readonly int[] MultiHunkEditLines = [3, 9, 15]; + + private async Task<(TaskEntity task, ListEntity list, WorktreeContext wt)> SeedMultiHunkConflictingWorktreeAsync() + { + var repo = new GitRepoFixture(); + _repos.Add(repo); + + var baseLines = Enumerable.Range(1, 20).Select(n => $"line{n}").ToArray(); + File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), string.Join('\n', baseLines) + "\n"); + GitRepoFixture.RunGit(repo.RepoDir, "add", "README.md"); + GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "multi-line base"); + + var listId = Guid.NewGuid().ToString(); + var list = new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow }; + await _lists.AddAsync(list); + var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview); + + var cfg = new WorkerConfig { WorktreeRootStrategy = "sibling" }; + var mgr = new WorktreeManager(new GitService(), _db.CreateFactory(), cfg, NullLogger.Instance); + var wt = await mgr.CreateAsync(task, list, CancellationToken.None); + _worktreeCleanups.Add((repo.RepoDir, wt.WorktreePath)); + + WriteMultiHunkVariant(wt.WorktreePath, baseLines, "WT"); + GitRepoFixture.RunGit(wt.WorktreePath, "add", "README.md"); + GitRepoFixture.RunGit(wt.WorktreePath, "commit", "-m", "worktree edit"); + + WriteMultiHunkVariant(list.WorkingDir!, baseLines, "MAIN"); + GitRepoFixture.RunGit(list.WorkingDir!, "add", "README.md"); + GitRepoFixture.RunGit(list.WorkingDir!, "commit", "-m", "main edit"); + + return (task, list, wt); + } + + private static void WriteMultiHunkVariant(string dir, string[] baseLines, string tag) + { + var lines = (string[])baseLines.Clone(); + foreach (var lineNum in MultiHunkEditLines) + lines[lineNum - 1] = $"{tag}-{lineNum}"; + File.WriteAllText(Path.Combine(dir, "README.md"), string.Join('\n', lines) + "\n"); + } + [Fact] public async Task MergeTask_LeaveConflictsInTree_LeavesMarkersAndKeepsRepoMidMerge() { @@ -1296,6 +1339,130 @@ public sealed class ExternalMcpServiceTests : IDisposable Assert.Contains("mid-merge", ex.Message); } + // ── get_merge_conflicts / resolve_conflict_hunk ───────────────────────────── + + [Fact] + public async Task GetMergeConflicts_ReportsHunksWithStartLineAndDiff3Base() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, list, _) = await SeedConflictingWorktreeAsync(TaskStatus.WaitingForReview); + var sut = BuildSut(CreateQueue()); + await sut.ReviewTask(task.Id, "approve", null, null, leaveConflictsInTree: true, CancellationToken.None); + + var result = await sut.GetMergeConflicts(task.Id, CancellationToken.None); + + Assert.Equal(1, result.RemainingHunks); + var file = Assert.Single(result.Files); + Assert.Equal("README.md", file.Path); + Assert.False(file.IsBinary); + var hunk = Assert.Single(file.Hunks); + Assert.Equal(0, hunk.Index); + Assert.True(hunk.StartLine >= 1); + Assert.Equal("# from main", hunk.Ours.TrimEnd('\r', '\n')); + Assert.Equal("# from worktree", hunk.Theirs.TrimEnd('\r', '\n')); + Assert.NotEmpty(result.ConflictStyleNote); + + GitRepoFixture.RunGit(list.WorkingDir!, "merge", "--abort"); + } + + [Fact] + public async Task GetMergeConflicts_NoMergeInProgress_ReturnsEmpty() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, _, _) = await SeedWorktreeAsync(TaskStatus.WaitingForReview); + var sut = BuildSut(CreateQueue()); + + var result = await sut.GetMergeConflicts(task.Id, CancellationToken.None); + + Assert.Equal(0, result.RemainingHunks); + Assert.Empty(result.Files); + } + + [Fact] + public async Task ResolveConflictHunk_OursTheirsAndText_ResolveEachHunkAndTrackRemainingCount() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, list, _) = await SeedMultiHunkConflictingWorktreeAsync(); + var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir!); + var sut = BuildSut(CreateQueue()); + + var merge = await sut.MergeTask(task.Id, target, true, dryRun: false, + allowWaitingForReview: true, leaveConflictsInTree: true, CancellationToken.None); + Assert.True(merge.ConflictsInTree); + + var before = await sut.GetMergeConflicts(task.Id, CancellationToken.None); + Assert.Equal(3, before.RemainingHunks); + Assert.Equal(3, Assert.Single(before.Files).Hunks.Count); + + // Resolve hunk 0 three times in a row: each resolution removes a hunk from the front, so the + // next still-open hunk always slides into index 0. + var r1 = await sut.ResolveConflictHunk(task.Id, "README.md", 0, "ours", CancellationToken.None); + Assert.Equal(2, r1.RemainingHunks); + + var r2 = await sut.ResolveConflictHunk(task.Id, "README.md", 0, "theirs", CancellationToken.None); + Assert.Equal(1, r2.RemainingHunks); + + var r3 = await sut.ResolveConflictHunk(task.Id, "README.md", 0, "CUSTOM-15\n", CancellationToken.None); + Assert.Equal(0, r3.RemainingHunks); + + var finalConflicts = await sut.GetMergeConflicts(task.Id, CancellationToken.None); + Assert.Equal(0, finalConflicts.RemainingHunks); + + var continueResult = await sut.ContinueMerge(task.Id, CancellationToken.None); + Assert.True(continueResult.Merged); + Assert.Equal("Done", continueResult.TaskStatus); + + var mergedText = File.ReadAllText(Path.Combine(list.WorkingDir!, "README.md")); + Assert.Contains("MAIN-3", mergedText); // hunk 1 resolved "ours" (the target branch) + Assert.Contains("WT-9", mergedText); // hunk 2 resolved "theirs" (the incoming branch) + Assert.Contains("CUSTOM-15", mergedText); // hunk 3 resolved to literal text + Assert.DoesNotContain("<<<<<<<", mergedText); + } + + [Fact] + public async Task ResolveConflictHunk_LeavesOtherHunksUntouchedUntilContinueMerge() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, list, _) = await SeedMultiHunkConflictingWorktreeAsync(); + var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir!); + var sut = BuildSut(CreateQueue()); + await sut.MergeTask(task.Id, target, true, dryRun: false, + allowWaitingForReview: true, leaveConflictsInTree: true, CancellationToken.None); + + // Resolve only one of the three hunks. + await sut.ResolveConflictHunk(task.Id, "README.md", 0, "ours", CancellationToken.None); + + // continue_merge must still refuse: writing one hunk must not have staged the file as resolved. + var result = await sut.ContinueMerge(task.Id, CancellationToken.None); + + Assert.False(result.Merged); + Assert.Contains("README.md", result.Conflicts); + Assert.True(await new GitService().IsMidMergeAsync(list.WorkingDir!)); + + GitRepoFixture.RunGit(list.WorkingDir!, "merge", "--abort"); + } + + [Fact] + public async Task ResolveConflictHunk_UnknownFileOrOutOfRangeIndex_Throws() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var (task, list, _) = await SeedConflictingWorktreeAsync(TaskStatus.WaitingForReview); + var sut = BuildSut(CreateQueue()); + await sut.ReviewTask(task.Id, "approve", null, null, leaveConflictsInTree: true, CancellationToken.None); + + await Assert.ThrowsAsync( + () => sut.ResolveConflictHunk(task.Id, "not-a-real-file.md", 0, "ours", CancellationToken.None)); + await Assert.ThrowsAsync( + () => sut.ResolveConflictHunk(task.Id, "README.md", 5, "ours", CancellationToken.None)); + + GitRepoFixture.RunGit(list.WorkingDir!, "merge", "--abort"); + } + // ── RevertMerge ──────────────────────────────────────────────────────────── [Fact]