fix(merge): pass the stash consent to the conflict resolver so it opens on a dirty target

The resolver restarts the merge via StartConflictMerge, which had no autostash.
After a stash-retry approve reported 'conflict', the restart hit the same dirty
target, got 'dirty_target' and the resolver silently never opened. Thread the
consent through the hub, client, coordinator and resolver; surface a refusal in
the footer instead of dropping it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
mika kuns
2026-10-02 17:25:52 +02:00
co-authored by Claude Sonnet 5.5
parent ff7197c410
commit feb0afa33a
16 changed files with 173 additions and 20 deletions
@@ -1,6 +1,7 @@
using System;
using System.Threading.Tasks;
using ClaudeDo.Data.Models;
using ClaudeDo.Ui.Services;
using ClaudeDo.Ui.ViewModels;
using Xunit;
@@ -124,4 +125,26 @@ public class IslandsShellViewModelExternalMergeTests
// click must degrade gracefully rather than throw, same as the existing UI-driven path.
await vm.OpenExternalMergeConflictCommand.ExecuteAsync(null);
}
private sealed class DirtyTargetWorker : StubWorkerClient
{
public override Task<MergeResultDto> StartConflictMergeAsync(string taskId, string targetBranch, bool autostash = false) =>
Task.FromResult(new MergeResultDto("dirty_target", Array.Empty<string>(), "target working tree has uncommitted changes"));
}
[Fact]
public async Task RequestConflictResolution_ResolverRefused_ShowsWhyInTheFooter()
{
// Regression: a refusal (dirty target, ...) left the resolver closed with no explanation anywhere.
var vm = new IslandsShellViewModel
{
Dialogs = new RecordingDialogService(),
ConflictResolverFactory = id => new ClaudeDo.Ui.ViewModels.Conflicts.ConflictResolverViewModel(new DirtyTargetWorker(), id),
};
await vm.RequestConflictResolutionAsync("t1", "main");
Assert.Equal(WorkerLogLevel.Error, vm.WorkerLogLevel);
Assert.Contains("uncommitted changes", vm.WorkerLogText);
}
}
+1 -1
View File
@@ -127,7 +127,7 @@ public abstract class StubWorkerClient : IWorkerClient
public virtual Task RejectReviewToQueueAsync(string taskId, string feedback) => Task.CompletedTask;
public virtual Task RejectReviewToIdleAsync(string taskId) => Task.CompletedTask;
public virtual Task CancelReviewAsync(string taskId) => Task.CompletedTask;
public virtual Task<MergeResultDto> StartConflictMergeAsync(string taskId, string targetBranch) => Task.FromResult(new MergeResultDto("conflict", System.Array.Empty<string>(), null));
public virtual Task<MergeResultDto> StartConflictMergeAsync(string taskId, string targetBranch, bool autostash = false) => Task.FromResult(new MergeResultDto("conflict", System.Array.Empty<string>(), null));
public virtual Task<MergeConflictDocumentsDto> GetMergeConflictDocumentsAsync(string taskId) => Task.FromResult(new MergeConflictDocumentsDto(taskId, System.Array.Empty<ConflictDocumentDto>()));
public virtual Task WriteConflictResolutionAsync(string taskId, string path, string resolvedContent) => Task.CompletedTask;
public virtual Task<MergeResultDto> ContinueConflictMergeAsync(string taskId) => Task.FromResult(new MergeResultDto("merged", System.Array.Empty<string>(), null));
@@ -30,9 +30,14 @@ public class ConflictResolverViewModelTests
public string? ContinuedPlanningParent;
public string? AbortedPlanningParent;
public override Task<MergeResultDto> StartConflictMergeAsync(string taskId, string targetBranch)
/// <summary>Models a target working tree with uncommitted changes: refused unless the caller consented to the stash.</summary>
public bool DirtyTarget;
public override Task<MergeResultDto> StartConflictMergeAsync(string taskId, string targetBranch, bool autostash = false)
{
StartCalled = true;
if (DirtyTarget && !autostash)
return Task.FromResult(new MergeResultDto("dirty_target", System.Array.Empty<string>(), "target working tree has uncommitted changes"));
return Task.FromResult(new MergeResultDto("conflict", new[] { "README.md" }, null));
}
@@ -63,6 +68,19 @@ public class ConflictResolverViewModelTests
public override Task AbortConflictMergeAsync(string taskId) { Aborted = true; return Task.CompletedTask; }
}
[Fact]
public async Task OpenAsync_DirtyTarget_OpensOnlyWithStashConsent()
{
// Regression: a dirty target refused the resolver's own merge restart, so it never opened.
var refused = new ConflictResolverViewModel(new FakeWorker { DirtyTarget = true }, "task-1");
Assert.False(await refused.OpenAsync("main"));
Assert.Equal("target working tree has uncommitted changes", refused.Error);
var consented = new ConflictResolverViewModel(new FakeWorker { DirtyTarget = true }, "task-1");
Assert.True(await consented.OpenAsync("main", autostash: true));
Assert.Null(consented.Error);
}
[Fact]
public async Task OpenAsync_LoadsConflicts_AndBlocksContinueUntilResolved()
{
@@ -60,7 +60,7 @@ public class DetailsIslandConflictSeamTests : IDisposable
string? capturedTarget = null;
var coordinator = new MergeCoordinator
{
Handler = (tid, target) =>
Handler = (tid, target, _) =>
{
capturedTaskId = tid;
capturedTarget = target;
@@ -77,4 +77,35 @@ public class DetailsIslandConflictSeamTests : IDisposable
Assert.Equal(taskId, capturedTaskId);
Assert.Equal("main", capturedTarget);
}
/// <summary>Models the worker: a dirty target refuses the merge unless the caller consented to the stash.</summary>
private sealed class DirtyTargetConflictWorkerClient : StubWorkerClient
{
public override bool IsConnected => true;
public override Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch, bool autostash = false) =>
Task.FromResult<MergeResultDto?>(autostash
? new MergeResultDto("conflict", new[] { "a.cs" }, null)
: new MergeResultDto("dirty_target", Array.Empty<string>(), "target working tree has uncommitted changes"));
}
[Fact]
public async Task ApproveReview_StashRetryEndsInConflict_HandsTheStashConsentToTheResolver()
{
// Regression: the resolver restarts the merge itself; without the consent it hit the same
// dirty target, got "dirty_target" and silently never opened.
bool? capturedAutostash = null;
var coordinator = new MergeCoordinator
{
Handler = (_, _, autostash) => { capturedAutostash = autostash; return Task.CompletedTask; },
};
var vm = BuildVm(new DirtyTargetConflictWorkerClient(), coordinator);
vm.Dialogs = new RecordingDialogService { ConfirmResult = true };
vm.Bind(new TaskRowViewModel { Id = "task-dirty-1", Status = TaskStatus.WaitingForReview });
vm.Merge.SelectedMergeTarget = "main";
await vm.ApproveReviewCommand.ExecuteAsync(null);
Assert.True(capturedAutostash);
}
}
@@ -26,6 +26,9 @@ public class MergeModalViewModelTests
/// Result for a call with autostash=false; the autostash retry always merges.
public MergeResultDto CleanResult { get; set; } = new("merged", new List<string>(), null);
/// Result for the autostash retry.
public MergeResultDto StashedResult { get; set; } = new("merged", new List<string>(), null);
public override Task<MergeTargetsDto?> GetMergeTargetsAsync(string taskId) =>
Task.FromResult(Targets);
@@ -36,7 +39,7 @@ public class MergeModalViewModelTests
CapturedAutostash = autostash;
return BlockMerge
? MergeGate.Task
: Task.FromResult(autostash ? new MergeResultDto("merged", new List<string>(), null) : CleanResult);
: Task.FromResult(autostash ? StashedResult : CleanResult);
}
}
@@ -189,4 +192,26 @@ public class MergeModalViewModelTests
Assert.Null(vm.ProgressMessage);
}
[Fact]
public async Task Stash_retry_ending_in_conflict_hands_the_stash_consent_to_the_resolver()
{
// Regression: the resolver restarts the merge itself and needs the same consent, or the
// dirty target refuses it and the resolver never opens.
var worker = new Worker
{
CleanResult = new("dirty_target", new List<string>(), "uncommitted changes"),
StashedResult = new("conflict", new List<string> { "a.cs" }, null),
};
bool? capturedAutostash = null;
var vm = new MergeModalViewModel(worker, new MergeCoordinator
{
Handler = (_, _, autostash) => { capturedAutostash = autostash; return Task.CompletedTask; },
});
await vm.InitializeAsync("task-1", "do the thing");
await vm.StashAndMergeCommand.ExecuteAsync(null);
Assert.True(capturedAutostash);
}
}
@@ -141,7 +141,7 @@ public class WorktreesOverviewBatchMergeTests
(string Task, string Target)? captured = null;
var coordinator = new ClaudeDo.Ui.Services.MergeCoordinator
{
Handler = (taskId, target) => { captured = (taskId, target); return System.Threading.Tasks.Task.CompletedTask; },
Handler = (taskId, target, _) => { captured = (taskId, target); return System.Threading.Tasks.Task.CompletedTask; },
};
var vm = NewVm(coordinator);
vm.SelectedTarget = "release";
@@ -1567,6 +1567,50 @@ public class TaskMergeServiceTests : IDisposable
GitRepoFixture.RunGit(repo.RepoDir, "merge", "--abort");
}
[Fact]
public async Task MergeAsync_LeaveConflicts_DirtyTarget_NeedsAutostashAndAbortRestoresTheChanges()
{
// The resolver restarts the merge via leaveConflictsInTree; on a dirty target that only
// works with the caller's stash consent, and git hands the stash back on abort.
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");
File.WriteAllText(Path.Combine(repo.RepoDir, "other.txt"), "tracked\n");
GitRepoFixture.RunGit(repo.RepoDir, "add", "-A");
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "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/t1", 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.WaitingForReview);
await SeedWorktree(db, task.Id, wtPath, "claudedo/t1", repo.BaseCommit);
File.WriteAllText(Path.Combine(repo.RepoDir, "other.txt"), "dirty\n");
var (svc, _) = BuildService(db);
var refused = await svc.MergeAsync(task.Id, "main", false, "msg", leaveConflictsInTree: true, CancellationToken.None);
Assert.Equal(TaskMergeService.StatusDirtyTarget, refused.Status);
var result = await svc.MergeAsync(
task.Id, "main", false, "msg", leaveConflictsInTree: true, CancellationToken.None, autostash: true);
Assert.Equal(TaskMergeService.StatusConflict, result.Status);
Assert.Contains("README.md", result.ConflictFiles);
Assert.True(await new GitService().IsMidMergeAsync(repo.RepoDir));
await svc.AbortMergeAsync(task.Id, CancellationToken.None);
Assert.False(await new GitService().IsMidMergeAsync(repo.RepoDir));
Assert.Equal("dirty\n", File.ReadAllText(Path.Combine(repo.RepoDir, "other.txt")).Replace("\r\n", "\n"));
}
[Fact]
public async Task WriteResolutionAsync_ThenContinue_CompletesMerge()
{