diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index 0a9cf3eb..75d6b97e 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -1290,7 +1290,15 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable 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)); + } _worker.MergeProgressEvent += OnMergeProgress; + _worker.OperationProgressEvent += OnOperationProgress; try { var hasChildren = Subtasks.Count > 0 || ChildOutcomes.Count > 0; @@ -1314,15 +1322,24 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable finally { _worker.MergeProgressEvent -= OnMergeProgress; + _worker.OperationProgressEvent -= OnOperationProgress; } } /// Mirrors TaskMergeService.PhaseVerifying — a hub payload token, not a display string. private const string MergePhaseVerifying = "verifying"; + /// Mirrors TaskMergeService.PhaseRebasing — a hub payload token, not a display string. + private const string MergePhaseRebasing = "rebasing"; + private static string FormatElapsed(int seconds) => TimeSpan.FromSeconds(Math.Max(0, seconds)).ToString(@"mm\:ss"); + private static string FormatRebasing(int current, int total) => + total > 0 + ? $"{Loc.T("ops.worker.rebasingAfterMerge")} ({current}/{total})" + : Loc.T("ops.worker.rebasingAfterMerge"); + // Force the diff to have been opened before a merge can happen — but only when // there is actually something to inspect (a childless sandbox run with no worktree // has no diff, so it approves straight through). diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs index 5af137ef..cf8e4c0b 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs @@ -95,6 +95,7 @@ public sealed partial class MergeModalViewModel : ViewModelBase // 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 { var result = await _worker.MergeTaskAsync( @@ -148,6 +149,7 @@ public sealed partial class MergeModalViewModel : ViewModelBase finally { _worker.MergeProgressEvent -= OnMergeProgress; + _worker.OperationProgressEvent -= OnOperationProgress; ProgressMessage = null; IsBusy = false; } @@ -156,6 +158,9 @@ public sealed partial class MergeModalViewModel : ViewModelBase 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)), @@ -163,9 +168,22 @@ public sealed partial class MergeModalViewModel : ViewModelBase }; } + // 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"); + } + /// Mirrors TaskMergeService.PhaseVerifying — a hub payload token, not a display string. private const string MergePhaseVerifying = "verifying"; + /// Mirrors TaskMergeService.PhaseRebasing — a hub payload token, not a display string. + private const string MergePhaseRebasing = "rebasing"; + private static string FormatElapsed(int seconds) => TimeSpan.FromSeconds(Math.Max(0, seconds)).ToString(@"mm\:ss"); diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index e43630b4..e37059b1 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -64,6 +64,7 @@ public sealed class TaskMergeService // Phase tokens for the OperationProgress broadcast — stable identifiers, localized by the UI. public const string PhaseMerging = "merging"; public const string PhaseVerifying = "verifying"; + public const string PhaseRebasing = "rebasing"; public const string PreviewClean = "clean"; public const string PreviewConflict = "conflict"; @@ -296,9 +297,15 @@ public sealed class TaskMergeService var tasks = await new TaskRepository(ctx).GetByListIdAsync(list.Id, ct); candidates = tasks.Where(t => t.Id != mergedTask.Id && t.Status == TaskStatus.WaitingForReview).ToList(); } + if (candidates.Count == 0) return; - foreach (var candidate in candidates) - await RebaseOneIfOverlappingAsync(candidate, targetBranch, newTargetTip, landedFiles, ct); + // Runs where the stalled "Merging…" phase would otherwise sit unchanged in the UI while + // this loop does real (if best-effort) work — see the correction note in the task spec. + for (var i = 0; i < candidates.Count; i++) + { + await _broadcaster.OperationProgress(mergedTask.Id, PhaseRebasing, i + 1, candidates.Count); + await RebaseOneIfOverlappingAsync(candidates[i], targetBranch, newTargetTip, landedFiles, ct); + } } private async Task RebaseOneIfOverlappingAsync( diff --git a/src/ClaudeDo.Worker/Worktrees/WorktreeMaintenanceService.cs b/src/ClaudeDo.Worker/Worktrees/WorktreeMaintenanceService.cs index a36aa4d1..4e89bd21 100644 --- a/src/ClaudeDo.Worker/Worktrees/WorktreeMaintenanceService.cs +++ b/src/ClaudeDo.Worker/Worktrees/WorktreeMaintenanceService.cs @@ -1,6 +1,7 @@ using ClaudeDo.Data; using ClaudeDo.Data.Git; using ClaudeDo.Data.Models; +using ClaudeDo.Worker.Hub; using Microsoft.EntityFrameworkCore; namespace ClaudeDo.Worker.Worktrees; @@ -11,20 +12,31 @@ public sealed class WorktreeMaintenanceService public sealed record ResetResult(int Removed, int TasksAffected, bool Blocked, int RunningTasks, IReadOnlyList RemovedTaskIds); public sealed record ForceRemoveResult(bool Removed, string? Reason, bool BranchDeleted); + // opKey/phase for the OperationProgress broadcast. Not task-bound, so opKey is a stable + // string rather than a task id — mirrors "startup-recovery"/"worktree-cleanup" in HubBroadcaster. + public const string OpKey = "worktree-maintenance"; + public const string PhaseMaintaining = "maintaining-worktrees"; + private readonly IDbContextFactory _dbFactory; private readonly GitService _git; private readonly ILogger _logger; + private readonly HubBroadcaster? _broadcaster; public WorktreeMaintenanceService( IDbContextFactory dbFactory, GitService git, - ILogger logger) + ILogger logger, + HubBroadcaster? broadcaster = null) { _dbFactory = dbFactory; _git = git; _logger = logger; + _broadcaster = broadcaster; } + private Task ReportProgress(int current, int total) => + _broadcaster?.OperationProgress(OpKey, PhaseMaintaining, current, total) ?? Task.CompletedTask; + public async Task CleanupFinishedAsync(string? listId = null, CancellationToken ct = default) { using var context = _dbFactory.CreateDbContext(); @@ -41,13 +53,14 @@ public sealed class WorktreeMaintenanceService int removed = 0; var removedTaskIds = new List(); - foreach (var row in rows) + for (var i = 0; i < rows.Count; i++) { - var (rowRemoved, _) = await TryRemoveAsync(row, force: false, ct); + await ReportProgress(i + 1, rows.Count); + var (rowRemoved, _) = await TryRemoveAsync(rows[i], force: false, ct); if (rowRemoved) { removed++; - removedTaskIds.Add(row.TaskId); + removedTaskIds.Add(rows[i].TaskId); } } return new CleanupResult(removed, removedTaskIds); @@ -70,13 +83,14 @@ public sealed class WorktreeMaintenanceService int removed = 0; var removedTaskIds = new List(); - foreach (var row in rows) + for (var i = 0; i < rows.Count; i++) { - var (rowRemoved, _) = await TryRemoveAsync(row, force: true, ct); + await ReportProgress(i + 1, rows.Count); + var (rowRemoved, _) = await TryRemoveAsync(rows[i], force: true, ct); if (rowRemoved) { removed++; - removedTaskIds.Add(row.TaskId); + removedTaskIds.Add(rows[i].TaskId); } } return new ResetResult(removed, rows.Count, Blocked: false, RunningTasks: 0, removedTaskIds); diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandReviewActionsTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandReviewActionsTests.cs index 7ee5cabc..de2317d3 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandReviewActionsTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandReviewActionsTests.cs @@ -255,6 +255,40 @@ public class DetailsIslandReviewActionsTests : IDisposable Assert.Equal(0, MergeProgressSubscriberCount(worker)); } + private static int OperationProgressSubscriberCount(StubWorkerClient worker) + { + var field = typeof(StubWorkerClient).GetField("OperationProgressEvent", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + var del = (Delegate?)field!.GetValue(worker); + return del?.GetInvocationList().Length ?? 0; + } + + [Fact] + public async Task ApproveReview_ShowsRebasePhase_WithBranchCount_InsteadOfStalledMerging() + { + var worker = new BlockingApproveWorkerClient(); + var vm = BuildVm(worker); + vm.Bind(new TaskRowViewModel { Id = "task-approve-rebase", Status = TaskStatus.WaitingForReview }); + vm.Monitor.ApplyState(TaskStatus.WaitingForReview); + + 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. + worker.RaiseOperationProgress("some-other-task", "rebasing", 1, 3); + Assert.Equal(merging, vm.ApproveOp.Label); + + worker.RaiseOperationProgress("task-approve-rebase", "rebasing", 2, 3); + Assert.NotEqual(merging, vm.ApproveOp.Label); + Assert.Contains("(2/3)", vm.ApproveOp.Label); + + worker.Gate.SetResult(new MergeResultDto("merged", new List(), null)); + await approve; + + Assert.Equal(0, OperationProgressSubscriberCount(worker)); + } + private sealed class BlockingSubmitWorkerClient : StubWorkerClient { public override bool IsConnected => true; diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MergeModalViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MergeModalViewModelTests.cs index 04450bd8..bbc9479a 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MergeModalViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MergeModalViewModelTests.cs @@ -119,4 +119,26 @@ public class MergeModalViewModelTests Assert.Null(vm.ProgressMessage); } + + [Fact] + public async Task Submit_shows_the_rebase_phase_with_branch_count_instead_of_stalled_merging() + { + var (vm, worker) = Build(); + await vm.InitializeAsync("task-1", "do the thing"); + worker.BlockMerge = true; + + 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. + worker.RaiseOperationProgress("task-1", "rebasing", 1, 2); + Assert.NotEqual(merging, vm.ProgressMessage); + Assert.Contains("(1/2)", vm.ProgressMessage); + + worker.MergeGate.SetResult(new MergeResultDto("merged", new List(), null)); + await submit; + + Assert.Null(vm.ProgressMessage); + } } diff --git a/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs index cf83cfcc..7db0c193 100644 --- a/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs @@ -1618,6 +1618,14 @@ public class TaskMergeServiceTests : IDisposable Assert.Equal(GitRepoFixture.RunGit(wtPathB, "rev-parse", "HEAD").Trim(), wtB.HeadCommit); Assert.Contains(proxy.Calls, c => c.Method == "WorktreeUpdated" && c.Args[0] is string s && s == taskB.Id); + + // The rebase phase must be visible on the wire while it runs -- otherwise the UI still + // shows the stalled "Merging…" phase for the whole (best-effort) rebase loop. + Assert.Contains(proxy.Calls, c => c.Method == "OperationProgress" + && c.Args[0] is string opKey && opKey == taskA.Id + && c.Args[1] is string phase && phase == TaskMergeService.PhaseRebasing + && c.Args[2] is int current && current == 1 + && c.Args[3] is int total && total == 1); } [Fact] diff --git a/tests/ClaudeDo.Worker.Tests/Services/WorktreeMaintenanceServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Services/WorktreeMaintenanceServiceTests.cs index 1d5f8f96..2cb1fb7e 100644 --- a/tests/ClaudeDo.Worker.Tests/Services/WorktreeMaintenanceServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Services/WorktreeMaintenanceServiceTests.cs @@ -1,6 +1,7 @@ using ClaudeDo.Data.Git; using ClaudeDo.Data.Models; using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.Hub; using ClaudeDo.Worker.Worktrees; using ClaudeDo.Worker.Tests.Infrastructure; using Microsoft.Extensions.Logging.Abstractions; @@ -477,4 +478,57 @@ public class WorktreeMaintenanceServiceTests : IDisposable var remaining = await new WorktreeRepository(checkCtx).GetAllAsync(); Assert.Empty(remaining); } + + [Fact] + public async Task ResetAll_Reports_OperationProgress_PerWorktree() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var repo = NewRepo(); + var git = new GitService(); + var db = NewDb(); + + var (list, t1) = MakeEntities(repo.RepoDir, status: ClaudeDo.Data.Models.TaskStatus.Done); + var t2 = MakeTaskForList(list.Id, ClaudeDo.Data.Models.TaskStatus.Idle); + + var wt1 = await CreateWorktreeAsync(git, repo.RepoDir, t1.Id); + var wt2 = await CreateWorktreeAsync(git, repo.RepoDir, t2.Id); + + using (var ctx = db.CreateContext()) + { + await new ListRepository(ctx).AddAsync(list); + var taskRepo = new TaskRepository(ctx); + await taskRepo.AddAsync(t1); + await taskRepo.AddAsync(t2); + var wtRepo = new WorktreeRepository(ctx); + await wtRepo.AddAsync(new WorktreeEntity + { + TaskId = t1.Id, Path = wt1, BranchName = $"test/{t1.Id}", + BaseCommit = repo.BaseCommit, State = WorktreeState.Active, CreatedAt = DateTime.UtcNow, + }); + await wtRepo.AddAsync(new WorktreeEntity + { + TaskId = t2.Id, Path = wt2, BranchName = $"test/{t2.Id}", + BaseCommit = repo.BaseCommit, State = WorktreeState.Kept, CreatedAt = DateTime.UtcNow, + }); + } + + var hub = new CapturingHubContext(); + var svc = new WorktreeMaintenanceService( + db.CreateFactory(), git, NullLogger.Instance, new HubBroadcaster(hub)); + + var result = await svc.ResetAllAsync(); + + Assert.Equal(2, result.Removed); + var progressCalls = hub.Proxy.Calls.Where(c => c.Method == "OperationProgress").ToList(); + Assert.Equal(2, progressCalls.Count); + Assert.All(progressCalls, c => + { + Assert.Equal(WorktreeMaintenanceService.OpKey, c.Args[0]); + Assert.Equal(WorktreeMaintenanceService.PhaseMaintaining, c.Args[1]); + Assert.Equal(2, c.Args[3]); + }); + Assert.Equal(1, progressCalls[0].Args[2]); + Assert.Equal(2, progressCalls[1].Args[2]); + } }