fix(claude-do): merge Modify/Delete- und Rename-Konflikte im Merge sichtbar machen
ClaudeDo-Task: 059bcf98-34d5-4ca0-822d-317980fadf8d
This commit is contained in:
@@ -376,6 +376,32 @@ public sealed class GitService
|
|||||||
throw new InvalidOperationException($"git add '{path}' failed (exit {exitCode}): {stderr}");
|
throw new InvalidOperationException($"git add '{path}' failed (exit {exitCode}): {stderr}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Stages <paramref name="path"/> as deleted (`git rm -f`), removing it from both the index
|
||||||
|
/// and, if present, the working tree. Used to resolve a merge conflict to "keep the
|
||||||
|
/// deletion" — unlike <see cref="AddPathAsync"/>, which would stage an empty file as a
|
||||||
|
/// tracked, zero-byte file rather than actually removing it.
|
||||||
|
/// </summary>
|
||||||
|
public async Task RemovePathAsync(string repoDir, string path, CancellationToken ct = default)
|
||||||
|
{
|
||||||
|
var (exitCode, _, stderr) = await RunGitAsync(repoDir, ["rm", "-f", "--", path], ct);
|
||||||
|
if (exitCode != 0)
|
||||||
|
throw new InvalidOperationException($"git rm '{path}' failed (exit {exitCode}): {stderr}");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Content of <paramref name="path"/> at a merge stage (1=base, 2=ours, 3=theirs) via
|
||||||
|
/// `git show :<stage>:<path>`. Null when that stage doesn't exist for the path —
|
||||||
|
/// the conventional way to tell a modify/delete or rename/delete conflict apart from an
|
||||||
|
/// ordinary content conflict (git never writes `<<<<<<<` markers for those,
|
||||||
|
/// it just leaves whichever side wasn't deleted in the working tree).
|
||||||
|
/// </summary>
|
||||||
|
public async Task<string?> ShowConflictStageAsync(string repoDir, int stage, string path, CancellationToken ct = default)
|
||||||
|
{
|
||||||
|
var (exitCode, stdout, _) = await RunGitAsync(repoDir, ["show", $":{stage}:{path}"], ct, trimOutput: false);
|
||||||
|
return exitCode == 0 ? stdout : null;
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Non-destructive mergeability probe via `git merge-tree --write-tree`. Writes only
|
/// Non-destructive mergeability probe via `git merge-tree --write-tree`. Writes only
|
||||||
/// loose objects — the working tree, index, and refs are left untouched.
|
/// loose objects — the working tree, index, and refs are left untouched.
|
||||||
|
|||||||
@@ -545,20 +545,48 @@ public sealed class TaskMergeService
|
|||||||
var oldTargetTip = await _git.RevParseHeadAsync(list.WorkingDir, ct);
|
var oldTargetTip = await _git.RevParseHeadAsync(list.WorkingDir, ct);
|
||||||
|
|
||||||
// Validate BEFORE staging: `git add` marks a conflicted path resolved regardless of
|
// Validate BEFORE staging: `git add` marks a conflicted path resolved regardless of
|
||||||
// its content, so an unresolved file with markers still in it would otherwise get
|
// its content, so an unresolved file needs a positively-checked resolution, not just
|
||||||
// staged (and committed) as-is. Check text content for markers first; binary files
|
// "whatever happens to be on disk". For an ordinary content conflict, git always
|
||||||
// can't carry markers, so they're left to the post-stage index check below.
|
// starts the file with markers, so their absence reliably means someone resolved it
|
||||||
|
// (in-app or in an external editor). A modify/delete or rename/delete conflict never
|
||||||
|
// gets markers at all (GetConflictDocumentsAsync) — the same marker-less state also
|
||||||
|
// describes a file nobody has touched — so those are additionally checked against
|
||||||
|
// what git itself left on disk by default (whichever side wasn't deleted); still
|
||||||
|
// matching that default means still unresolved.
|
||||||
var unresolved = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
|
var unresolved = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
|
||||||
var stillConflicted = new List<string>();
|
var stillConflicted = new List<string>();
|
||||||
|
var toAdd = new List<string>();
|
||||||
|
var toRemove = new List<string>();
|
||||||
foreach (var path in unresolved)
|
foreach (var path in unresolved)
|
||||||
{
|
{
|
||||||
var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
|
var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
|
||||||
string text;
|
string? text = null;
|
||||||
try { text = await File.ReadAllTextAsync(full, ct); }
|
if (File.Exists(full))
|
||||||
catch { continue; }
|
{
|
||||||
|
try { text = await File.ReadAllTextAsync(full, ct); }
|
||||||
|
catch { /* treated as unresolved below */ }
|
||||||
|
}
|
||||||
|
|
||||||
if (!LooksBinary(text) && ConflictMarkerParser.HasConflicts(text))
|
if (text is not null && (LooksBinary(text) || ConflictMarkerParser.HasConflicts(text)))
|
||||||
|
{
|
||||||
stillConflicted.Add(path);
|
stillConflicted.Add(path);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
var oursStage = await _git.ShowConflictStageAsync(list.WorkingDir, 2, path, ct);
|
||||||
|
var theirsStage = await _git.ShowConflictStageAsync(list.WorkingDir, 3, path, ct);
|
||||||
|
var isModifyDeleteStyle = oursStage is null || theirsStage is null;
|
||||||
|
if (isModifyDeleteStyle &&
|
||||||
|
NormalizeLineEndings(text ?? "") == NormalizeLineEndings(oursStage ?? theirsStage ?? ""))
|
||||||
|
{
|
||||||
|
stillConflicted.Add(path);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (text is null || text.Length == 0)
|
||||||
|
toRemove.Add(path);
|
||||||
|
else
|
||||||
|
toAdd.Add(path);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (stillConflicted.Count > 0)
|
if (stillConflicted.Count > 0)
|
||||||
@@ -575,8 +603,10 @@ public sealed class TaskMergeService
|
|||||||
// Stage exactly the resolved conflict paths — never `git add -A`, which would sweep
|
// Stage exactly the resolved conflict paths — never `git add -A`, which would sweep
|
||||||
// untracked/unrelated changes left by other sessions into this merge commit (the
|
// untracked/unrelated changes left by other sessions into this merge commit (the
|
||||||
// target working dir is shared).
|
// target working dir is shared).
|
||||||
foreach (var path in unresolved)
|
foreach (var path in toAdd)
|
||||||
await _git.AddPathAsync(list.WorkingDir, path, ct);
|
await _git.AddPathAsync(list.WorkingDir, path, ct);
|
||||||
|
foreach (var path in toRemove)
|
||||||
|
await _git.RemovePathAsync(list.WorkingDir, path, ct);
|
||||||
|
|
||||||
var remaining = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
|
var remaining = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
|
||||||
if (remaining.Count > 0)
|
if (remaining.Count > 0)
|
||||||
@@ -713,6 +743,15 @@ public sealed class TaskMergeService
|
|||||||
/// <summary>
|
/// <summary>
|
||||||
/// Reads each conflicted working-tree file and parses its conflict markers into line-level
|
/// Reads each conflicted working-tree file and parses its conflict markers into line-level
|
||||||
/// segments (with the diff3 merge base when present). Binary files are flagged and skipped.
|
/// segments (with the diff3 merge base when present). Binary files are flagged and skipped.
|
||||||
|
/// A path with no text markers on disk is either an ordinary content conflict that's already
|
||||||
|
/// been resolved (git keeps both index stages until the path is staged, whether or not the
|
||||||
|
/// working tree still looks conflicted — reads as a single stable segment, 0 hunks left), or
|
||||||
|
/// a modify/delete or rename/delete conflict — git never writes markers for those at all, it
|
||||||
|
/// just leaves whichever side wasn't deleted sitting in the working tree, so the same
|
||||||
|
/// "no markers" state also describes an untouched file. The index tells the two apart: a
|
||||||
|
/// modify/delete-style conflict only ever populates ONE of the ours/theirs stages. Those are
|
||||||
|
/// synthesized into a single whole-file conflict block straight from the stages (empty side
|
||||||
|
/// = that side deleted the path) so the resolver still shows a real choice.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public async Task<ConflictDocuments> GetConflictDocumentsAsync(string taskId, CancellationToken ct)
|
public async Task<ConflictDocuments> GetConflictDocumentsAsync(string taskId, CancellationToken ct)
|
||||||
{
|
{
|
||||||
@@ -725,21 +764,53 @@ public sealed class TaskMergeService
|
|||||||
foreach (var path in files)
|
foreach (var path in files)
|
||||||
{
|
{
|
||||||
var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
|
var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
|
||||||
string text;
|
string? diskText = null;
|
||||||
try { text = await File.ReadAllTextAsync(full, ct); }
|
if (File.Exists(full))
|
||||||
catch { text = ""; }
|
{
|
||||||
|
try { diskText = await File.ReadAllTextAsync(full, ct); }
|
||||||
|
catch { /* unreadable — fall through to the index stages below */ }
|
||||||
|
}
|
||||||
|
|
||||||
if (LooksBinary(text))
|
if (diskText is not null && LooksBinary(diskText))
|
||||||
{
|
{
|
||||||
result.Add(new ConflictDocumentContent(path, true, Array.Empty<MergeSegment>()));
|
result.Add(new ConflictDocumentContent(path, true, Array.Empty<MergeSegment>()));
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
result.Add(new ConflictDocumentContent(path, false, ConflictMarkerParser.Parse(text)));
|
if (diskText is not null && ConflictMarkerParser.HasConflicts(diskText))
|
||||||
|
{
|
||||||
|
result.Add(new ConflictDocumentContent(path, false, ConflictMarkerParser.Parse(diskText)));
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
var ours = await _git.ShowConflictStageAsync(list.WorkingDir, 2, path, ct);
|
||||||
|
var theirs = await _git.ShowConflictStageAsync(list.WorkingDir, 3, path, ct);
|
||||||
|
if (ours is not null && theirs is not null)
|
||||||
|
{
|
||||||
|
// Ordinary two-sided conflict, already resolved (no markers left) but not yet staged.
|
||||||
|
result.Add(new ConflictDocumentContent(path, false,
|
||||||
|
new[] { MergeSegment.Stable(diskText ?? "") }));
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (LooksBinary(ours ?? "") || LooksBinary(theirs ?? ""))
|
||||||
|
{
|
||||||
|
result.Add(new ConflictDocumentContent(path, true, Array.Empty<MergeSegment>()));
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
var @base = await _git.ShowConflictStageAsync(list.WorkingDir, 1, path, ct);
|
||||||
|
result.Add(new ConflictDocumentContent(
|
||||||
|
path, false, new[] { MergeSegment.Conflict(ours ?? "", @base, theirs ?? "") }));
|
||||||
}
|
}
|
||||||
return new ConflictDocuments(taskId, result);
|
return new ConflictDocuments(taskId, result);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Working-tree checkouts can go through autocrlf while `git show :stage:path` never does —
|
||||||
|
// normalize before comparing the two or an untouched file reads as "resolved" on a machine
|
||||||
|
// with autocrlf enabled.
|
||||||
|
private static string NormalizeLineEndings(string text) => text.Replace("\r\n", "\n").Replace('\r', '\n');
|
||||||
|
|
||||||
// A NUL byte in the head of the file is the conventional binary sniff.
|
// A NUL byte in the head of the file is the conventional binary sniff.
|
||||||
private static bool LooksBinary(string text)
|
private static bool LooksBinary(string text)
|
||||||
{
|
{
|
||||||
@@ -755,6 +826,15 @@ public sealed class TaskMergeService
|
|||||||
if (string.IsNullOrWhiteSpace(list.WorkingDir))
|
if (string.IsNullOrWhiteSpace(list.WorkingDir))
|
||||||
throw new InvalidOperationException("list has no working directory");
|
throw new InvalidOperationException("list has no working directory");
|
||||||
|
|
||||||
|
if (content.Length == 0)
|
||||||
|
{
|
||||||
|
// An empty resolution for a whole-file conflict (modify/delete, rename/delete) means
|
||||||
|
// "keep the deletion" — `git add` on an empty file would instead stage it as a
|
||||||
|
// tracked, zero-byte file, which is not what accepting the deleted side means.
|
||||||
|
await _git.RemovePathAsync(list.WorkingDir, path, ct);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
|
var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
|
||||||
await File.WriteAllTextAsync(full, content, ct);
|
await File.WriteAllTextAsync(full, content, ct);
|
||||||
await _git.AddPathAsync(list.WorkingDir, path, ct);
|
await _git.AddPathAsync(list.WorkingDir, path, ct);
|
||||||
|
|||||||
@@ -1109,6 +1109,63 @@ public class TaskMergeServiceTests : IDisposable
|
|||||||
Assert.Contains(file.Segments, s => !s.IsConflict && s.Text.Contains("line1"));
|
Assert.Contains(file.Segments, s => !s.IsConflict && s.Text.Contains("line1"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// git never writes <<<<<<< markers for a modify/delete conflict -- it just leaves the
|
||||||
|
// modifying side's content sitting in the working tree, which used to read as an
|
||||||
|
// already-resolved stable file (no conflict at all) to both GetConflictDocumentsAsync and
|
||||||
|
// ContinueMergeAsync's marker scan, so `continue_merge` would silently keep the modification.
|
||||||
|
[Fact]
|
||||||
|
public async Task ModifyDeleteConflict_IsSynthesizedAndCannotBeSilentlyStaged()
|
||||||
|
{
|
||||||
|
if (!GitRepoFixture.IsGitAvailable()) return;
|
||||||
|
|
||||||
|
var db = NewDb();
|
||||||
|
var repo = NewRepo();
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
|
||||||
|
|
||||||
|
var wtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
|
||||||
|
_wtCleanups.Add((repo.RepoDir, wtPath));
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/modify-delete", wtPath, repo.BaseCommit);
|
||||||
|
File.WriteAllText(Path.Combine(wtPath, "README.md"), "# modified by branch\n");
|
||||||
|
GitRepoFixture.RunGit(wtPath, "commit", "-am", "branch modifies README");
|
||||||
|
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "rm", "README.md");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main deletes README");
|
||||||
|
|
||||||
|
var (_, task) = await SeedListAndTask(db, workingDir: repo.RepoDir, status: TaskStatus.Done);
|
||||||
|
await SeedWorktree(db, task.Id, wtPath, "claudedo/modify-delete", repo.BaseCommit);
|
||||||
|
|
||||||
|
var (svc, _) = BuildService(db);
|
||||||
|
var merge = await svc.MergeAsync(task.Id, "main", removeWorktree: false, "msg",
|
||||||
|
leaveConflictsInTree: true, CancellationToken.None);
|
||||||
|
Assert.Equal(TaskMergeService.StatusConflict, merge.Status);
|
||||||
|
|
||||||
|
// No <<<<<<< markers anywhere -- git left the branch's modification as-is.
|
||||||
|
Assert.DoesNotContain("<<<<<<<", File.ReadAllText(Path.Combine(repo.RepoDir, "README.md")));
|
||||||
|
|
||||||
|
var docs = await svc.GetConflictDocumentsAsync(task.Id, CancellationToken.None);
|
||||||
|
var file = Assert.Single(docs.Files);
|
||||||
|
Assert.EndsWith("README.md", file.Path.Replace('\\', '/'));
|
||||||
|
Assert.False(file.IsBinary);
|
||||||
|
var conflict = Assert.Single(file.Segments.Where(s => s.IsConflict).ToList());
|
||||||
|
Assert.Equal("", conflict.Ours); // ours (main) deleted it
|
||||||
|
Assert.Contains("modified by branch", conflict.Theirs); // theirs (branch) modified it
|
||||||
|
|
||||||
|
// Nobody resolved it — continue must refuse rather than silently keeping the modification.
|
||||||
|
var headBefore = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim();
|
||||||
|
var refused = await svc.ContinueMergeAsync(task.Id, CancellationToken.None);
|
||||||
|
Assert.Equal(TaskMergeService.StatusConflict, refused.Status);
|
||||||
|
Assert.Contains("README.md", refused.ConflictFiles);
|
||||||
|
Assert.Equal(headBefore, GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim());
|
||||||
|
Assert.True(await new GitService().IsMidMergeAsync(repo.RepoDir));
|
||||||
|
|
||||||
|
// Explicitly resolving to the deleted (ours) side must `git rm`, not stage an empty file.
|
||||||
|
await svc.WriteResolutionAsync(task.Id, "README.md", "", CancellationToken.None);
|
||||||
|
var result = await svc.ContinueMergeAsync(task.Id, CancellationToken.None);
|
||||||
|
Assert.Equal(TaskMergeService.StatusMerged, result.Status);
|
||||||
|
Assert.False(File.Exists(Path.Combine(repo.RepoDir, "README.md")));
|
||||||
|
Assert.False(await new GitService().IsMidMergeAsync(repo.RepoDir));
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task ApproveAndMergeAsync_NoWorktree_MarksDone()
|
public async Task ApproveAndMergeAsync_NoWorktree_MarksDone()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user