fix(worker): validate conflict markers before staging in ContinueMergeAsync
git add -A resolves the index U-stage regardless of file content, so a conflicted file whose markers were never removed still got staged and committed as-is. Check previously-conflicted files for leftover <<<<<<< markers before staging; refuse with a conflict result if any remain, leaving the repo mid-merge so Abort still cleans up. Adds a regression test that leaves one conflict unresolved and asserts ContinueMergeAsync refuses (no commit) and AbortMergeAsync still restores a clean tree.
This commit is contained in:
@@ -54,7 +54,6 @@ Alle 9 Review-Tasks (5 Refactorings, 4 Bugfixes) sind umgesetzt und gemerged; De
|
|||||||
- Cancel eines `WaitingForChildren`-Parents kaskadiert nicht auf laufende/queued Kinder (verwaiste Worktree-Commits).
|
- Cancel eines `WaitingForChildren`-Parents kaskadiert nicht auf laufende/queued Kinder (verwaiste Worktree-Commits).
|
||||||
- Ketten-Kaskade stoppt an einem `Idle`-Mittelglied (`OnChildFinishedAsync` prüft `CancelAsync`-Ergebnis nicht) → Rest bleibt `Queued+blocked`.
|
- Ketten-Kaskade stoppt an einem `Idle`-Mittelglied (`OnChildFinishedAsync` prüft `CancelAsync`-Ergebnis nicht) → Rest bleibt `Queued+blocked`.
|
||||||
- Delete des *letzten* nicht-terminalen Kindes triggert kein `TryAdvanceParentAsync` → Parent kann in `WaitingForChildren` hängen (FK `SET NULL` rettet nur die Blocked-Kette).
|
- Delete des *letzten* nicht-terminalen Kindes triggert kein `TryAdvanceParentAsync` → Parent kann in `WaitingForChildren` hängen (FK `SET NULL` rettet nur die Blocked-Kette).
|
||||||
- `ContinueMergeAsync` staged per `git add -A` vor dem Konflikt-Check (Marker im Index, Abort danach ggf. unsauber).
|
|
||||||
- `HasChangesAsync` zählt untracked Files → blockiert Merges unnötig (`--untracked-files=no`).
|
- `HasChangesAsync` zählt untracked Files → blockiert Merges unnötig (`--untracked-files=no`).
|
||||||
- `UnifiedDiffParser`: Pfade mit Leerzeichen / git-gequotete Pfade aus `diff --git` falsch geparst.
|
- `UnifiedDiffParser`: Pfade mit Leerzeichen / git-gequotete Pfade aus `diff --git` falsch geparst.
|
||||||
- Kleinkram: MergePreview-Race bei schnellem Target-Wechsel, CTS-Dispose-Leak in Debounce-Saves, `Environment.CurrentDirectory`-Fallback im Konflikt-Dialog, Doppel-Continue-Fenster im Orchestrator.
|
- Kleinkram: MergePreview-Race bei schnellem Target-Wechsel, CTS-Dispose-Leak in Debounce-Saves, `Environment.CurrentDirectory`-Fallback im Konflikt-Dialog, Doppel-Continue-Fenster im Orchestrator.
|
||||||
|
|||||||
@@ -204,6 +204,26 @@ public sealed class TaskMergeService
|
|||||||
if (!await _git.IsMidMergeAsync(list.WorkingDir, ct))
|
if (!await _git.IsMidMergeAsync(list.WorkingDir, ct))
|
||||||
return Blocked("repo is not mid-merge");
|
return Blocked("repo is not mid-merge");
|
||||||
|
|
||||||
|
// 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
|
||||||
|
// staged (and committed) as-is. Check text content for markers first; binary files
|
||||||
|
// can't carry markers, so they're left to the post-stage index check below.
|
||||||
|
var unresolved = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
|
||||||
|
var stillConflicted = new List<string>();
|
||||||
|
foreach (var path in unresolved)
|
||||||
|
{
|
||||||
|
var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
|
||||||
|
string text;
|
||||||
|
try { text = await File.ReadAllTextAsync(full, ct); }
|
||||||
|
catch { continue; }
|
||||||
|
|
||||||
|
if (!LooksBinary(text) && ConflictMarkerParser.HasConflicts(text))
|
||||||
|
stillConflicted.Add(path);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (stillConflicted.Count > 0)
|
||||||
|
return new MergeResult(StatusConflict, stillConflicted, "conflicts not fully resolved");
|
||||||
|
|
||||||
await _git.AddAllAsync(list.WorkingDir, ct);
|
await _git.AddAllAsync(list.WorkingDir, ct);
|
||||||
|
|
||||||
var remaining = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
|
var remaining = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
|
||||||
|
|||||||
@@ -410,6 +410,52 @@ public class TaskMergeServiceTests : IDisposable
|
|||||||
Assert.Equal(WorktreeState.Merged, wt.State);
|
Assert.Equal(WorktreeState.Merged, wt.State);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ContinueMergeAsync_UnresolvedConflictMarkersLeft_RefusesAndDoesNotCommit()
|
||||||
|
{
|
||||||
|
if (!GitRepoFixture.IsGitAvailable()) return;
|
||||||
|
|
||||||
|
var db = NewDb();
|
||||||
|
var repo = NewRepo();
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
|
||||||
|
|
||||||
|
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "# main change\n");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-am", "main change");
|
||||||
|
|
||||||
|
var wtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
|
||||||
|
_wtCleanups.Add((repo.RepoDir, wtPath));
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/t4", wtPath, repo.BaseCommit);
|
||||||
|
File.WriteAllText(Path.Combine(wtPath, "README.md"), "# branch change\n");
|
||||||
|
GitRepoFixture.RunGit(wtPath, "commit", "-am", "branch change");
|
||||||
|
|
||||||
|
var (_, task) = await SeedListAndTask(db, workingDir: repo.RepoDir, status: TaskStatus.Done);
|
||||||
|
await SeedWorktree(db, task.Id, wtPath, "claudedo/t4", repo.BaseCommit);
|
||||||
|
|
||||||
|
var (svc, _) = BuildService(db);
|
||||||
|
|
||||||
|
var first = await svc.MergeAsync(task.Id, "main", false, "msg",
|
||||||
|
leaveConflictsInTree: true, CancellationToken.None);
|
||||||
|
Assert.Equal(TaskMergeService.StatusConflict, first.Status);
|
||||||
|
var headBefore = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim();
|
||||||
|
|
||||||
|
// User never resolves the conflict — README.md still contains "<<<<<<<" markers.
|
||||||
|
var result = await svc.ContinueMergeAsync(task.Id, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.StatusConflict, result.Status);
|
||||||
|
Assert.Contains("README.md", result.ConflictFiles);
|
||||||
|
Assert.Contains("<<<<<<<", File.ReadAllText(Path.Combine(repo.RepoDir, "README.md")));
|
||||||
|
|
||||||
|
// No commit must have happened, and the repo must still be mid-merge so Abort can clean it up.
|
||||||
|
Assert.Equal(headBefore, GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim());
|
||||||
|
Assert.True(await new GitService().IsMidMergeAsync(repo.RepoDir));
|
||||||
|
|
||||||
|
var abort = await svc.AbortMergeAsync(task.Id, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.StatusAborted, abort.Status);
|
||||||
|
Assert.False(await new GitService().IsMidMergeAsync(repo.RepoDir));
|
||||||
|
Assert.True(string.IsNullOrWhiteSpace(GitRepoFixture.RunGit(repo.RepoDir, "status", "--porcelain")));
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task AbortMergeAsync_AfterConflict_RestoresCleanStateAndLeavesWorktreeActive()
|
public async Task AbortMergeAsync_AfterConflict_RestoresCleanStateAndLeavesWorktreeActive()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user