diff --git a/docs/explore-notes/review-merge.md b/docs/explore-notes/review-merge.md index 4e18763a..a73574e7 100644 --- a/docs/explore-notes/review-merge.md +++ b/docs/explore-notes/review-merge.md @@ -138,6 +138,23 @@ mid-merge conflicts **without re-starting the merge** and routes continue/abort `ContinuePlanningMerge` / `AbortPlanningMerge`, so a unit-merge conflict re-opens the editor per subtask via the `PlanningMergeConflict` broadcast. +**Except when an MCP session is driving the merge.** `PlanningMergeOrchestrator.StartAsync` takes +an `externallyDriven` bool (default `false`; `ExternalMcpService.review_task`'s parent-with-children +path passes `true`, since a running Claude session — not the UI — will resolve conflicts via +`continue_merge`/`abort_merge`). The flag rides along on the `PlanningMergeConflict` broadcast +(4th arg); `IslandsShellViewModel.OnPlanningMergeConflict` only auto-opens the resolver when it's +`false` — otherwise it shows a persistent, non-auto-dismissing banner +(`IsExternalMergeBannerVisible`) with a manual "Open resolver" button, cleared on +`PlanningMergeAborted`/`PlanningCompleted`. This exists because two parties (a human and the +driving session) could otherwise end up editing the same shared checkout at once. +`PlanningMergeOrchestrator.GetActiveExternalConflictsAsync` (hub: `GetActiveExternalPlanningMergeConflicts`) +re-derives this state by checking `GitService.IsMidMergeAsync` rather than trusting the in-memory +flag alone, so a UI restart mid-merge (or a stale entry left behind if something resolved the +repo outside the normal Continue/Abort path) can't show a phantom banner — the Ui calls it on +`ConnectionRestoredEvent`. The childless single-task conflict path +(`TaskMergeService.ApproveAndMergeAsync`) has no equivalent broadcast — it only sends the generic +`TaskUpdated` — so it never auto-opened the resolver and needed no change. + ### View Three **AvaloniaEdit** panes showing the whole file: MAIN/ours (read-only) | editable Result | diff --git a/src/ClaudeDo.Localization/locales/de.json b/src/ClaudeDo.Localization/locales/de.json index 1693ad07..3c73a3da 100644 --- a/src/ClaudeDo.Localization/locales/de.json +++ b/src/ClaudeDo.Localization/locales/de.json @@ -632,6 +632,10 @@ }, "staleWorker": { "message": "Der Worker läuft auf einem älteren Stand als der gemergte main-Branch dieses Repos — Neustart nötig, damit gemergte Änderungen wirken." + }, + "externalMerge": { + "banner": "Eine Claude-Session löst gerade einen Merge-Konflikt auf — bitte keine Dateien im Repository bearbeiten.", + "open": "Resolver öffnen" } }, "vm": { diff --git a/src/ClaudeDo.Localization/locales/en.json b/src/ClaudeDo.Localization/locales/en.json index 903ed98b..e2e0a6d2 100644 --- a/src/ClaudeDo.Localization/locales/en.json +++ b/src/ClaudeDo.Localization/locales/en.json @@ -632,6 +632,10 @@ }, "staleWorker": { "message": "The worker is running an older build than this repo's merged main — restart it so your merged changes take effect." + }, + "externalMerge": { + "banner": "A Claude session is resolving a merge conflict — don't edit files in the repository.", + "open": "Open resolver" } }, "vm": { diff --git a/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs b/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs index 0a848c81..a807b6b3 100644 --- a/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs @@ -35,7 +35,10 @@ public interface IWorkerClient : INotifyPropertyChanged event Action? PlanningMergeStartedEvent; event Action? PlanningSubtaskMergedEvent; - event Action>? PlanningMergeConflictEvent; + /// (planningTaskId, subtaskId, conflictedFiles, externallyDriven). externallyDriven + /// is true when an MCP session (not the UI) started the unit merge — the resolver must not + /// auto-open in that case. + event Action, bool>? PlanningMergeConflictEvent; event Action? PlanningMergeAbortedEvent; event Action? PlanningCompletedEvent; @@ -117,6 +120,9 @@ public interface IWorkerClient : INotifyPropertyChanged Task BuildPlanningIntegrationBranchAsync(string planningTaskId, string targetBranch); Task ContinuePlanningMergeAsync(string planningTaskId); Task AbortPlanningMergeAsync(string planningTaskId); + /// Unit merges currently paused on a conflict that an MCP session started. Called + /// on (re)connect to recover the "don't auto-open" banner state after a UI restart. + Task> GetActiveExternalPlanningMergeConflictsAsync(); Task QueuePlanningSubtasksAsync(string parentTaskId, CancellationToken ct = default); Task GetWeekReportAsync(DateOnly start, DateOnly end); Task GenerateWeekReportAsync(DateOnly start, DateOnly end); diff --git a/src/ClaudeDo.Ui/Services/WorkerClient.cs b/src/ClaudeDo.Ui/Services/WorkerClient.cs index f4201a7c..3523f584 100644 --- a/src/ClaudeDo.Ui/Services/WorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/WorkerClient.cs @@ -66,7 +66,7 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC public event Action? PlanningMergeStartedEvent; public event Action? PlanningSubtaskMergedEvent; - public event Action>? PlanningMergeConflictEvent; + public event Action, bool>? PlanningMergeConflictEvent; public event Action? PlanningMergeAbortedEvent; public event Action? PlanningCompletedEvent; @@ -182,9 +182,9 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC Dispatcher.UIThread.Post(() => PlanningSubtaskMergedEvent?.Invoke(planningTaskId, subtaskId)); }); - _hub.On>("PlanningMergeConflict", (planningTaskId, subtaskId, conflictedFiles) => + _hub.On, bool>("PlanningMergeConflict", (planningTaskId, subtaskId, conflictedFiles, externallyDriven) => { - Dispatcher.UIThread.Post(() => PlanningMergeConflictEvent?.Invoke(planningTaskId, subtaskId, conflictedFiles)); + Dispatcher.UIThread.Post(() => PlanningMergeConflictEvent?.Invoke(planningTaskId, subtaskId, conflictedFiles, externallyDriven)); }); _hub.On("PlanningMergeAborted", planningTaskId => @@ -581,6 +581,10 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC await _hub.InvokeAsync("AbortPlanningMerge", planningTaskId); } + public async Task> GetActiveExternalPlanningMergeConflictsAsync() + => await TryInvokeAsync>("GetActiveExternalPlanningMergeConflicts") + ?? []; + public async Task QueuePlanningSubtasksAsync(string parentTaskId, CancellationToken ct = default) { await _hub.InvokeAsync("QueuePlanningSubtasksAsync", parentTaskId, ct); @@ -695,6 +699,7 @@ public sealed record LaunchSpec( IReadOnlyDictionary Env); public sealed record ForceRemoveResultDto(bool Removed, string? Reason); +public sealed record PlanningMergeConflictStateDto(string PlanningTaskId, string SubtaskId); public sealed record PendingQuestionDto(string TaskId, string QuestionId, string Question); public sealed record WorkerBuildInfoDto(string? BuildSha); diff --git a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs index 9057ce1e..6886dc27 100644 --- a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs @@ -109,6 +109,13 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable // only — see RefreshStaleWorkerCheckAsync. [ObservableProperty] private bool _isStaleWorkerBannerVisible; + // planningTaskId -> subtaskId, for unit-merge conflicts an MCP session (not the UI) started. + // Kept as a dictionary so a later conflict on the same planning task updates in place instead + // of piling up duplicate entries. + private readonly Dictionary _externalMergeConflicts = new(); + [ObservableProperty] private bool _isExternalMergeBannerVisible; + + [ObservableProperty] private double _windowWidth = 1280; @@ -185,13 +192,59 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable _primeStatusTimer.Start(); } - private void OnPlanningMergeConflict(string planningTaskId, string subtaskId, IReadOnlyList conflictedFiles) + public void OnPlanningMergeConflict( + string planningTaskId, string subtaskId, IReadOnlyList conflictedFiles, bool externallyDriven) { // Already on UI thread (WorkerClient dispatches via Dispatcher.UIThread.Post). + if (externallyDriven) + { + // An MCP session (review_task/continue_merge) is driving this merge — it owns + // resolution. Auto-opening the resolver here raced with the session's own writes + // (two parties editing the same shared checkout at once); show a banner instead and + // leave the resolver reachable only via a deliberate click. + _externalMergeConflicts[planningTaskId] = subtaskId; + IsExternalMergeBannerVisible = true; + return; + } + // A unit-merge conflict resolves in the same in-app 3-way editor as a single-task merge. _ = OpenPlanningConflictAsync(planningTaskId, subtaskId); } + public void OnPlanningMergeAborted(string planningTaskId) => ClearExternalMergeConflict(planningTaskId); + public void OnPlanningMergeCompleted(string planningTaskId) => ClearExternalMergeConflict(planningTaskId); + + private void ClearExternalMergeConflict(string planningTaskId) + { + _externalMergeConflicts.Remove(planningTaskId); + IsExternalMergeBannerVisible = _externalMergeConflicts.Count > 0; + } + + /// Re-syncs the external-merge banner from the worker on (re)connect — the + /// one-shot PlanningMergeConflict broadcast isn't replayed after a UI restart, so this is + /// the recovery path. The worker checks MERGE_HEAD before reporting a conflict as active, + /// so a session that died mid-merge without cleaning up doesn't leave a stale banner up. + private async Task RefreshExternalMergeConflictsAsync() + { + if (Worker is null) return; + IReadOnlyList active; + try { active = await Worker.GetActiveExternalPlanningMergeConflictsAsync(); } + catch { return; } + + _externalMergeConflicts.Clear(); + foreach (var c in active) + _externalMergeConflicts[c.PlanningTaskId] = c.SubtaskId; + IsExternalMergeBannerVisible = _externalMergeConflicts.Count > 0; + } + + [RelayCommand] + private Task OpenExternalMergeConflictAsync() + { + if (_externalMergeConflicts.Count == 0) return Task.CompletedTask; + var (planningTaskId, subtaskId) = _externalMergeConflicts.First(); + return OpenPlanningConflictAsync(planningTaskId, subtaskId); + } + private async Task OpenPlanningConflictAsync(string planningTaskId, string subtaskId) { if (ConflictResolverFactory is null || Dialogs is null) return; @@ -298,6 +351,9 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable Worker.WorkerLogReceivedEvent += OnWorkerLogReceived; Worker.ConnectionRestoredEvent += () => _ = RefreshStaleWorkerCheckAsync(); Worker.PlanningMergeConflictEvent += OnPlanningMergeConflict; + Worker.PlanningMergeAbortedEvent += OnPlanningMergeAborted; + Worker.PlanningCompletedEvent += OnPlanningMergeCompleted; + Worker.ConnectionRestoredEvent += () => _ = RefreshExternalMergeConflictsAsync(); Worker.PrimeFired += OnPrimeFired; _clearTimer.Elapsed += (_, _) => { diff --git a/src/ClaudeDo.Ui/Views/MainWindow.axaml b/src/ClaudeDo.Ui/Views/MainWindow.axaml index 3331d241..a5dd9fee 100644 --- a/src/ClaudeDo.Ui/Views/MainWindow.axaml +++ b/src/ClaudeDo.Ui/Views/MainWindow.axaml @@ -281,5 +281,27 @@ + + + + + +