refactor(merge): single IMergeCoordinator replaces the 5 conflict seams

The RequestConflictResolution Func was declared on 5 VMs and hand-threaded shell->details->merge-section->diff->merge-modal. Replaced with a DI-singleton IMergeCoordinator (MergeCoordinator holder; shell wires its Handler at composition, breaking the shell<->island cycle). Invokers (MergeModal, DetailsIsland, WorktreesOverview) depend on the interface; the two pass-through VMs (DiffModal, MergeSection) drop the seam entirely. No behavior change; conflict-seam + batch tests rewired to assert via the coordinator.
This commit is contained in:
Mika Kuns
2026-06-22 17:18:57 +02:00
parent 3f9f047955
commit 5be4b5c5fb
15 changed files with 79 additions and 75 deletions

View File

@@ -116,6 +116,10 @@ sealed class Program
return new UpdateCheckService(releases, version);
});
// Conflict-merge coordinator: single seam the shell wires to its resolver entry.
sc.AddSingleton<MergeCoordinator>();
sc.AddSingleton<IMergeCoordinator>(sp => sp.GetRequiredService<MergeCoordinator>());
// ViewModels
sc.AddTransient<WorktreeModalViewModel>();
sc.AddTransient<Func<WorktreeModalViewModel>>(sp => () => sp.GetRequiredService<WorktreeModalViewModel>());
@@ -152,12 +156,14 @@ sealed class Program
sp.GetRequiredService<IDbContextFactory<ClaudeDoDbContext>>(),
sp.GetRequiredService<IWorkerClient>(),
sp,
sp.GetRequiredService<INotesApi>()));
sp.GetRequiredService<INotesApi>(),
sp.GetRequiredService<IMergeCoordinator>()));
sc.AddSingleton<IslandsShellViewModel>(sp =>
{
var shell = ActivatorUtilities.CreateInstance<IslandsShellViewModel>(sp);
shell.ConflictResolverFactory =
sp.GetRequiredService<Func<string, ClaudeDo.Ui.ViewModels.Conflicts.ConflictResolverViewModel>>();
sp.GetRequiredService<MergeCoordinator>().Handler = shell.RequestConflictResolutionAsync;
return shell;
});

View File

@@ -0,0 +1,29 @@
using System;
using System.Threading.Tasks;
namespace ClaudeDo.Ui.Services;
/// <summary>
/// Single entry point for handing a conflicting merge to the in-app 3-pane resolver.
/// Replaces the per-VM <c>RequestConflictResolution</c> Func seams that used to be
/// hand-threaded shell → details → merge-section → diff → merge-modal. The shell wires
/// <see cref="MergeCoordinator.Handler"/> once at composition; invokers depend only on
/// this interface (injected via DI).
/// </summary>
public interface IMergeCoordinator
{
Task ResolveConflictAsync(string taskId, string targetBranch);
}
/// <summary>
/// DI singleton holding the resolver entry. The holder breaks the shell↔island construction
/// cycle: islands depend on the interface, the shell sets <see cref="Handler"/> after it is built.
/// </summary>
public sealed class MergeCoordinator : IMergeCoordinator
{
/// Set once at composition to the shell's resolver entry. Null (headless/tests) ⇒ no-op.
public Func<string, string, Task>? Handler { get; set; }
public Task ResolveConflictAsync(string taskId, string targetBranch) =>
Handler?.Invoke(taskId, targetBranch) ?? Task.CompletedTask;
}

View File

@@ -52,6 +52,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
private readonly IWorkerClient _worker;
private readonly IServiceProvider _services;
private readonly INotesApi _notesApi;
private readonly IMergeCoordinator _merge;
// ── Section view models ───────────────────────────────────────────────────
public AgentSettingsSectionViewModel AgentSettings { get; }
@@ -343,13 +344,6 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
public bool HasReviewFeedback => !string.IsNullOrWhiteSpace(ReviewFeedback);
// Kept for backwards-compat surface — delegates to Merge.RequestConflictResolution
public Func<string, string, System.Threading.Tasks.Task>? RequestConflictResolution
{
get => Merge.RequestConflictResolution;
set => Merge.RequestConflictResolution = value;
}
private static string StatusToStateKey(ClaudeDo.Data.Models.TaskStatus status) => status switch
{
ClaudeDo.Data.Models.TaskStatus.Queued => "queued",
@@ -404,12 +398,14 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
IDbContextFactory<ClaudeDoDbContext> dbFactory,
IWorkerClient worker,
IServiceProvider services,
INotesApi notesApi)
INotesApi notesApi,
IMergeCoordinator merge)
{
_dbFactory = dbFactory;
_worker = worker;
_services = services;
_notesApi = notesApi;
_merge = merge;
AgentSettings = new AgentSettingsSectionViewModel(worker);
Merge = new MergeSectionViewModel(worker, services);
@@ -1151,20 +1147,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
var hasChildren = Subtasks.Count > 0 || ChildOutcomes.Count > 0;
var result = await _worker.ApproveReviewAsync(Task.Id, Merge.SelectedMergeTarget ?? "");
if (!hasChildren && result?.Status == "conflict")
{
if (Merge.RequestConflictResolution is not null)
{
await Merge.RequestConflictResolution(Task.Id, Merge.SelectedMergeTarget ?? "");
}
else
{
var (text, _, _) = MergePreviewPresenter.Describe(
new MergePreviewDto("conflict", result.ConflictFiles, 0));
Merge.MergePreviewText = text;
Merge.MergeIsClean = false;
Merge.MergeIsConflict = true;
}
}
await _merge.ResolveConflictAsync(Task.Id, Merge.SelectedMergeTarget ?? "");
}
catch (Exception ex)
{

View File

@@ -48,7 +48,6 @@ public sealed partial class MergeSectionViewModel : ViewModelBase
public Func<DiffModalViewModel, System.Threading.Tasks.Task>? ShowDiffModal { get; set; }
public Func<MergeModalViewModel, System.Threading.Tasks.Task>? ShowMergeModal { get; set; }
public Func<ViewModels.Planning.PlanningDiffViewModel, System.Threading.Tasks.Task>? ShowPlanningDiffModal { get; set; }
public Func<string, string, System.Threading.Tasks.Task>? RequestConflictResolution { get; set; }
public MergeSectionViewModel(IWorkerClient worker, IServiceProvider services)
{
@@ -156,7 +155,6 @@ public sealed partial class MergeSectionViewModel : ViewModelBase
TaskTitle = TaskTitle ?? "",
ShowMergeModal = ShowMergeModal,
ResolveMergeVm = () => _services.GetRequiredService<MergeModalViewModel>(),
RequestConflictResolution = RequestConflictResolution,
};
}
else if (CanDiffMergedRange)

View File

@@ -194,7 +194,6 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable
_ = Lists.RefreshCountsAsync();
return System.Threading.Tasks.Task.CompletedTask;
};
Details.RequestConflictResolution = RequestConflictResolutionAsync;
Worker.PropertyChanged += (_, e) =>
{
if (e.PropertyName is nameof(IWorkerClient.IsConnected) or nameof(IWorkerClient.IsReconnecting))

View File

@@ -72,7 +72,6 @@ public sealed partial class DiffModalViewModel : ViewModelBase
public string TaskTitle { get; init; } = "";
public Func<MergeModalViewModel, Task>? ShowMergeModal { get; set; }
public Func<MergeModalViewModel>? ResolveMergeVm { get; set; }
public Func<string, string, Task>? RequestConflictResolution { get; set; }
public ObservableCollection<DiffFileViewModel> Files { get; } = new();
@@ -100,7 +99,6 @@ public sealed partial class DiffModalViewModel : ViewModelBase
{
if (TaskId is null || ShowMergeModal is null || ResolveMergeVm is null) return;
var vm = ResolveMergeVm();
vm.RequestConflictResolution = RequestConflictResolution;
await vm.InitializeAsync(TaskId, TaskTitle);
await ShowMergeModal(vm);
// The diff is stale once the worktree merged away or a conflict opened the editor.

View File

@@ -9,6 +9,7 @@ namespace ClaudeDo.Ui.ViewModels.Modals;
public sealed partial class MergeModalViewModel : ViewModelBase
{
private readonly IWorkerClient _worker;
private readonly IMergeCoordinator _merge;
public string TaskId { get; set; } = "";
public string TaskTitle { get; set; } = "";
@@ -28,10 +29,6 @@ public sealed partial class MergeModalViewModel : ViewModelBase
public Action? CloseAction { get; set; }
/// Set by the caller to hand a conflicting merge off to the in-app 3-pane editor
/// instead of dead-ending on the conflict message.
public Func<string, string, Task>? RequestConflictResolution { get; set; }
/// True once a merge has succeeded — lets the caller (e.g. the diff window)
/// close itself after this modal closes.
public bool Merged { get; private set; }
@@ -39,9 +36,10 @@ public sealed partial class MergeModalViewModel : ViewModelBase
/// True once a conflict has been handed off to the resolver — also a cue to close the diff window.
public bool RoutedToResolver { get; private set; }
public MergeModalViewModel(IWorkerClient worker)
public MergeModalViewModel(IWorkerClient worker, IMergeCoordinator merge)
{
_worker = worker;
_merge = merge;
}
public async Task InitializeAsync(string taskId, string taskTitle)
@@ -103,21 +101,11 @@ public sealed partial class MergeModalViewModel : ViewModelBase
});
break;
case "conflict":
// Hand off to the in-app 3-pane merge editor when wired (MergeTask aborted
// cleanly, so the resolver re-starts the merge leaving conflicts in the tree).
if (RequestConflictResolution is not null)
{
var branch = SelectedBranch!;
RoutedToResolver = true;
CloseAction?.Invoke();
await RequestConflictResolution(TaskId, branch);
}
else
{
HasConflict = true;
ConflictFiles = result.ConflictFiles;
ErrorMessage = Loc.T("vm.merge.conflict");
}
// MergeTask aborted cleanly; hand the conflict to the in-app 3-pane editor,
// which re-starts the merge leaving conflicts in the tree.
RoutedToResolver = true;
CloseAction?.Invoke();
await _merge.ResolveConflictAsync(TaskId, SelectedBranch!);
break;
case "blocked":
ErrorMessage = Loc.T("vm.merge.blocked", result.ErrorMessage ?? "");

View File

@@ -62,6 +62,7 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase
{
private readonly IWorkerClient _worker;
private readonly Func<WorktreeModalViewModel> _diffVmFactory;
private readonly IMergeCoordinator _merge;
[ObservableProperty] private string? _listIdFilter;
[ObservableProperty] private string _title = "Worktrees";
@@ -79,9 +80,6 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase
public ObservableCollection<string> MergeTargets { get; } = new();
public ObservableCollection<WorktreeOverviewRowViewModel> ConflictRows { get; } = new();
/// Inert seam wired by the integrator to Layer C's resolver at merge time. (taskId, targetBranch)
public Func<string, string, Task>? RequestConflictResolution { get; set; }
public Action? CloseAction { get; set; }
public Action<WorktreeModalViewModel>? ShowDiffAction { get; set; }
public Action<string, string>? JumpToTaskAction { get; set; }
@@ -89,10 +87,11 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase
public Func<MergeModalViewModel>? ResolveMergeVm { get; set; }
public Func<MergeModalViewModel, Task>? ShowMergeAction { get; set; }
public WorktreesOverviewModalViewModel(IWorkerClient worker, Func<WorktreeModalViewModel> diffVmFactory)
public WorktreesOverviewModalViewModel(IWorkerClient worker, Func<WorktreeModalViewModel> diffVmFactory, IMergeCoordinator merge)
{
_worker = worker;
_diffVmFactory = diffVmFactory;
_merge = merge;
}
public void SelectRow(WorktreeOverviewRowViewModel row)
@@ -328,7 +327,7 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase
private void ResolveConflict(WorktreeOverviewRowViewModel? row)
{
if (row is null) return;
RequestConflictResolution?.Invoke(row.TaskId, SelectedTarget ?? "");
_ = _merge.ResolveConflictAsync(row.TaskId, SelectedTarget ?? "");
}
[RelayCommand]

View File

@@ -87,8 +87,6 @@ public sealed class WindowDialogService : IDialogService
var mergeDlg = new MergeModalView { DataContext = mergeVm };
await mergeDlg.ShowDialog(_owner);
};
vm.RequestConflictResolution = (taskId, target) =>
shell.RequestConflictResolutionAsync(taskId, target);
}
await dlg.ShowDialog(_owner);
}

View File

@@ -64,10 +64,10 @@ public class DetailsIslandConflictSeamTests : IDisposable
Task.FromResult<MergeResultDto?>(new MergeResultDto("conflict", new[] { "a.cs" }, null));
}
private DetailsIslandViewModel BuildVm(StubWorkerClient worker)
private DetailsIslandViewModel BuildVm(StubWorkerClient worker, IMergeCoordinator merge)
{
var factory = new TestDbFactory(NewContext);
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi());
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi(), merge);
}
[Fact]
@@ -75,19 +75,22 @@ public class DetailsIslandConflictSeamTests : IDisposable
{
const string taskId = "task-conflict-1";
var vm = BuildVm(new ConflictApproveWorkerClient());
vm.Bind(new TaskRowViewModel { Id = taskId, Status = TaskStatus.WaitingForReview });
vm.Merge.SelectedMergeTarget = "main";
string? capturedTaskId = null;
string? capturedTarget = null;
vm.Merge.RequestConflictResolution = (tid, target) =>
var coordinator = new MergeCoordinator
{
capturedTaskId = tid;
capturedTarget = target;
return Task.CompletedTask;
Handler = (tid, target) =>
{
capturedTaskId = tid;
capturedTarget = target;
return Task.CompletedTask;
},
};
var vm = BuildVm(new ConflictApproveWorkerClient(), coordinator);
vm.Bind(new TaskRowViewModel { Id = taskId, Status = TaskStatus.WaitingForReview });
vm.Merge.SelectedMergeTarget = "main";
await vm.ApproveReviewCommand.ExecuteAsync(null);
Assert.Equal(taskId, capturedTaskId);

View File

@@ -66,7 +66,7 @@ public class DetailsIslandPlanningTests : IDisposable
private DetailsIslandViewModel BuildVm(StubWorkerClient worker)
{
var factory = new TestDbFactory(NewContext);
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi());
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi(), new ClaudeDo.Ui.Services.MergeCoordinator());
}
// Connected worker whose review calls fail the way the hub does when the task

View File

@@ -51,7 +51,7 @@ public class DetailsIslandPrepModeTests : IDisposable
private DetailsIslandViewModel NewDetailsVm(StubWorkerClient stub)
{
var factory = new TestDbFactory(NewContext);
return new DetailsIslandViewModel(factory, stub, new NullServiceProvider(), new StubNotesApi());
return new DetailsIslandViewModel(factory, stub, new NullServiceProvider(), new StubNotesApi(), new ClaudeDo.Ui.Services.MergeCoordinator());
}
private sealed class NullServiceProvider : IServiceProvider

View File

@@ -69,7 +69,7 @@ public class DetailsIslandReviewActionsTests : IDisposable
private DetailsIslandViewModel BuildVm(StubWorkerClient worker)
{
var factory = new TestDbFactory(NewContext);
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi());
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi(), new ClaudeDo.Ui.Services.MergeCoordinator());
}
[Fact]

View File

@@ -57,7 +57,7 @@ public class DetailsIslandTabsTests : IDisposable
private DetailsIslandViewModel NewVm()
{
var factory = new TestDbFactory(NewContext);
return new DetailsIslandViewModel(factory, new DefaultStub(), new NullServiceProvider(), new StubNotesApi());
return new DetailsIslandViewModel(factory, new DefaultStub(), new NullServiceProvider(), new StubNotesApi(), new ClaudeDo.Ui.Services.MergeCoordinator());
}
[Fact]

View File

@@ -30,8 +30,9 @@ public class WorktreesOverviewBatchMergeTests
Assert.False(row.IsConflict);
}
private static WorktreesOverviewModalViewModel NewVm() =>
new(new ClaudeDo.Ui.Services.WorkerClient("http://127.0.0.1:1/hub"), () => null!);
private static WorktreesOverviewModalViewModel NewVm(IMergeCoordinator? merge = null) =>
new(new ClaudeDo.Ui.Services.WorkerClient("http://127.0.0.1:1/hub"), () => null!,
merge ?? new ClaudeDo.Ui.Services.MergeCoordinator());
private static MergeResultDto Merged() => new("merged", System.Array.Empty<string>(), null);
private static MergeResultDto Conflict() => new("conflict", new[] { "f.cs" }, null);
@@ -137,13 +138,15 @@ public class WorktreesOverviewBatchMergeTests
[Fact]
public void ResolveConflict_invokes_seam_with_task_and_target()
{
var vm = NewVm();
(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; },
};
var vm = NewVm(coordinator);
vm.SelectedTarget = "release";
var row = ActiveRow("x"); row.MergeOutcome = BatchMergeOutcome.Conflict;
(string Task, string Target)? captured = null;
vm.RequestConflictResolution = (taskId, target) => { captured = (taskId, target); return System.Threading.Tasks.Task.CompletedTask; };
vm.ResolveConflictCommand.Execute(row);
Assert.Equal(("x", "release"), captured);