diff --git a/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs b/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs index fdb3058e..64e7308e 100644 --- a/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs @@ -52,13 +52,6 @@ public interface IWorkerClient : INotifyPropertyChanged /// total. event Action? OperationProgressEvent; - /// (taskId, phase, elapsedSeconds) — phase of an in-flight single-task merge - /// ("merging" | "verifying"). Fires while the MergeTask call itself is still pending, so the - /// waiting UI can show what it's blocked on; the verify phase re-fires every 30 s. A thin - /// forwarder over kept for existing consumers (elapsed - /// seconds riding in the generic "current" slot). - event Action? MergeProgressEvent; - event Action? PlanningMergeStartedEvent; event Action? PlanningSubtaskMergedEvent; /// (planningTaskId, subtaskId, conflictedFiles, externallyDriven). externallyDriven diff --git a/src/ClaudeDo.Ui/Services/WorkerClient.cs b/src/ClaudeDo.Ui/Services/WorkerClient.cs index a28a452e..14deb8ee 100644 --- a/src/ClaudeDo.Ui/Services/WorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/WorkerClient.cs @@ -67,7 +67,6 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC public event Action? UsageUpdatedEvent; public event Action? OperationProgressEvent; - public event Action? MergeProgressEvent; public event Action? PlanningMergeStartedEvent; public event Action? PlanningSubtaskMergedEvent; @@ -189,11 +188,7 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC _hub.On("OperationProgress", (opKey, phase, current, total) => { - Dispatcher.UIThread.Post(() => - { - OperationProgressEvent?.Invoke(opKey, phase, current, total); - MergeProgressEvent?.Invoke(opKey, phase, current); - }); + Dispatcher.UIThread.Post(() => OperationProgressEvent?.Invoke(opKey, phase, current, total)); }); _hub.On("PlanningMergeStarted", (planningTaskId, targetBranch) => diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index 77541452..63ae3f1c 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -1262,19 +1262,14 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable using var op = ApproveOp.Begin(Loc.T("ops.merge.merging")); // Subscribed only for the duration of the call: the worker broadcasts to every // client, and a VM left on this event would keep reacting to merges for other tasks. - void OnMergeProgress(string progressTaskId, string phase, int elapsedSeconds) - { - if (progressTaskId != taskId || phase != MergePhaseVerifying) return; - ApproveOp.Report(Loc.T("ops.merge.verifying", FormatElapsed(elapsedSeconds))); - } - // Separate event: MergeProgressEvent forwards elapsed-seconds-only (verifying), but the - // rebase phase needs the branch count too, which only rides the untrimmed OperationProgress. void OnOperationProgress(string opKey, string phase, int current, int total) { - if (opKey != taskId || phase != MergePhaseRebasing) return; - ApproveOp.Report(FormatRebasing(current, total)); + if (opKey != taskId) return; + if (phase == MergePhaseVerifying) + ApproveOp.Report(Loc.T("ops.merge.verifying", FormatElapsed(current))); + else if (phase == MergePhaseRebasing) + ApproveOp.Report(FormatRebasing(current, total)); } - _worker.MergeProgressEvent += OnMergeProgress; _worker.OperationProgressEvent += OnOperationProgress; try { @@ -1298,7 +1293,6 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable } finally { - _worker.MergeProgressEvent -= OnMergeProgress; _worker.OperationProgressEvent -= OnOperationProgress; } } diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs index 0f69ade4..bcb708e9 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs @@ -94,7 +94,6 @@ public sealed partial class MergeModalViewModel : ViewModelBase // Subscribed only for the duration of the call: the worker's broadcast reaches every // client, and a transient VM left on that event would outlive its window. ProgressMessage = Loc.T("vm.merge.progressMerging"); - _worker.MergeProgressEvent += OnMergeProgress; _worker.OperationProgressEvent += OnOperationProgress; try { @@ -148,34 +147,23 @@ public sealed partial class MergeModalViewModel : ViewModelBase } finally { - _worker.MergeProgressEvent -= OnMergeProgress; _worker.OperationProgressEvent -= OnOperationProgress; ProgressMessage = null; IsBusy = false; } } - private void OnMergeProgress(string taskId, string phase, int elapsedSeconds) - { - if (taskId != TaskId) return; - // Rebasing is handled by OnOperationProgress (needs the branch total, which this - // elapsed-seconds-only forwarder drops) -- don't overwrite it with the generic fallback. - if (phase == MergePhaseRebasing) return; - ProgressMessage = phase switch - { - MergePhaseVerifying => Loc.T("vm.merge.progressVerifying", FormatElapsed(elapsedSeconds)), - _ => Loc.T("vm.merge.progressMerging"), - }; - } - - // Separate event: MergeProgressEvent forwards elapsed-seconds-only (verifying), but the - // rebase phase needs the branch count too, which only rides the untrimmed OperationProgress. private void OnOperationProgress(string opKey, string phase, int current, int total) { - if (opKey != TaskId || phase != MergePhaseRebasing) return; - ProgressMessage = total > 0 - ? $"{Loc.T("ops.worker.rebasingAfterMerge")} ({current}/{total})" - : Loc.T("ops.worker.rebasingAfterMerge"); + if (opKey != TaskId) return; + ProgressMessage = phase switch + { + MergePhaseRebasing => total > 0 + ? $"{Loc.T("ops.worker.rebasingAfterMerge")} ({current}/{total})" + : Loc.T("ops.worker.rebasingAfterMerge"), + MergePhaseVerifying => Loc.T("vm.merge.progressVerifying", FormatElapsed(current)), + _ => Loc.T("vm.merge.progressMerging"), + }; } /// Mirrors TaskMergeService.PhaseVerifying — a hub payload token, not a display string. diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index d3c332c4..9d483be7 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -202,8 +202,7 @@ launch specs · worktrees · agents/settings/lists · reports/notes/prep · diag - `PrepLine` - `PrepFinished` - `OperationProgress` (generic `(opKey, phase, current, total)` channel; merge phases are its - first producer — `IWorkerClient.MergeProgressEvent` on the Ui side is a thin forwarder over it - for existing consumers, elapsed seconds riding in the `current` slot) + first producer) - `PlanningMergeStarted` - `PlanningSubtaskMerged` - `PlanningMergeConflict` diff --git a/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs b/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs index 21de02b8..4f22a459 100644 --- a/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs +++ b/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs @@ -35,7 +35,6 @@ public abstract class StubWorkerClient : IWorkerClient public event Action? RefineStartedEvent; public event Action? RefineFinishedEvent; public event Action? OperationProgressEvent; - public event Action? MergeProgressEvent; public event Action? PlanningMergeStartedEvent; public event Action? PlanningSubtaskMergedEvent; public event Action, bool>? PlanningMergeConflictEvent; @@ -64,7 +63,6 @@ public abstract class StubWorkerClient : IWorkerClient public void RaisePlanningMergeConflict(string planningTaskId, string subtaskId, IReadOnlyList files, bool externallyDriven) => PlanningMergeConflictEvent?.Invoke(planningTaskId, subtaskId, files, externallyDriven); public void RaiseOperationProgress(string opKey, string phase, int current, int total) => OperationProgressEvent?.Invoke(opKey, phase, current, total); - public void RaiseMergeProgress(string taskId, string phase, int elapsedSeconds) => MergeProgressEvent?.Invoke(taskId, phase, elapsedSeconds); public void RaisePlanningMergeStarted(string planningTaskId, string targetBranch) => PlanningMergeStartedEvent?.Invoke(planningTaskId, targetBranch); public void RaisePlanningMergeAborted(string planningTaskId, string? reason = null) => PlanningMergeAbortedEvent?.Invoke(planningTaskId, reason); public void RaisePlanningCompleted(string planningTaskId) => PlanningCompletedEvent?.Invoke(planningTaskId); diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandReviewActionsTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandReviewActionsTests.cs index a0800e40..f4dd2a42 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandReviewActionsTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandReviewActionsTests.cs @@ -170,14 +170,6 @@ public class DetailsIslandReviewActionsTests : IDisposable public override Task ApproveReviewAsync(string taskId, string targetBranch) => Gate.Task; } - private static int MergeProgressSubscriberCount(StubWorkerClient worker) - { - var field = typeof(StubWorkerClient).GetField("MergeProgressEvent", - System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); - var del = (Delegate?)field!.GetValue(worker); - return del?.GetInvocationList().Length ?? 0; - } - [Fact] public async Task ApproveReview_SetsIsRunning_AndLocksTheButton_WhileThePendingCallIsInFlight() { @@ -209,10 +201,10 @@ public class DetailsIslandReviewActionsTests : IDisposable var approve = vm.ApproveReviewCommand.ExecuteAsync(null); var merging = vm.ApproveOp.Label; - worker.RaiseMergeProgress("some-other-task", "verifying", 30); + worker.RaiseOperationProgress("some-other-task", "verifying", 30, 0); Assert.Equal(merging, vm.ApproveOp.Label); - worker.RaiseMergeProgress("task-approve-2", "verifying", 30); + worker.RaiseOperationProgress("task-approve-2", "verifying", 30, 0); Assert.NotEqual(merging, vm.ApproveOp.Label); worker.Gate.SetResult(new MergeResultDto("merged", new List(), null)); @@ -220,7 +212,7 @@ public class DetailsIslandReviewActionsTests : IDisposable } [Fact] - public async Task ApproveReview_UnsubscribesFromMergeProgress_AfterTheCallCompletes() + public async Task ApproveReview_UnsubscribesFromOperationProgress_AfterTheCallCompletes() { var worker = new BlockingApproveWorkerClient(); var vm = BuildVm(worker); @@ -228,12 +220,12 @@ public class DetailsIslandReviewActionsTests : IDisposable vm.Monitor.ApplyState(TaskStatus.WaitingForReview); var approve = vm.ApproveReviewCommand.ExecuteAsync(null); - Assert.Equal(1, MergeProgressSubscriberCount(worker)); + Assert.Equal(1, OperationProgressSubscriberCount(worker)); worker.Gate.SetResult(new MergeResultDto("merged", new List(), null)); await approve; - Assert.Equal(0, MergeProgressSubscriberCount(worker)); + Assert.Equal(0, OperationProgressSubscriberCount(worker)); } private static int OperationProgressSubscriberCount(StubWorkerClient worker) @@ -255,8 +247,7 @@ public class DetailsIslandReviewActionsTests : IDisposable var approve = vm.ApproveReviewCommand.ExecuteAsync(null); var merging = vm.ApproveOp.Label; - // The rebase phase needs the branch total, which only rides OperationProgress - // (MergeProgressEvent forwards elapsed-seconds only) — ignored for other tasks. + // The rebase phase needs the branch total — ignored for other tasks. worker.RaiseOperationProgress("some-other-task", "rebasing", 1, 3); Assert.Equal(merging, vm.ApproveOp.Label); diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MergeModalViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MergeModalViewModelTests.cs index bbc9479a..577f7e7f 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MergeModalViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MergeModalViewModelTests.cs @@ -79,7 +79,7 @@ public class MergeModalViewModelTests // The verify gate starts — a distinct phase, so the user sees why this is taking minutes. // (The elapsed time rides in a format argument, which the test localizer drops.) - worker.RaiseMergeProgress("task-1", "verifying", 90); + worker.RaiseOperationProgress("task-1", "verifying", 90, 0); Assert.NotEqual(merging, vm.ProgressMessage); worker.MergeGate.SetResult(new MergeResultDto("merged", new List(), null)); @@ -100,7 +100,7 @@ public class MergeModalViewModelTests var submit = vm.SubmitCommand.ExecuteAsync(null); var merging = vm.ProgressMessage; - worker.RaiseMergeProgress("some-other-task", "verifying", 42); + worker.RaiseOperationProgress("some-other-task", "verifying", 42, 0); Assert.Equal(merging, vm.ProgressMessage); @@ -115,7 +115,7 @@ public class MergeModalViewModelTests await vm.InitializeAsync("task-1", "do the thing"); await vm.SubmitCommand.ExecuteAsync(null); - worker.RaiseMergeProgress("task-1", "verifying", 10); + worker.RaiseOperationProgress("task-1", "verifying", 10, 0); Assert.Null(vm.ProgressMessage); } @@ -130,8 +130,7 @@ public class MergeModalViewModelTests var submit = vm.SubmitCommand.ExecuteAsync(null); var merging = vm.ProgressMessage; - // Needs the branch total, which only rides OperationProgress (MergeProgressEvent - // forwards elapsed-seconds only) -- and must not be clobbered by that other forwarder. + // Needs the branch total, which only OperationProgress carries. worker.RaiseOperationProgress("task-1", "rebasing", 1, 2); Assert.NotEqual(merging, vm.ProgressMessage); Assert.Contains("(1/2)", vm.ProgressMessage);