From a0d5db0db088c114c8f3d0a0c22c8820508d3fcb Mon Sep 17 00:00:00 2001 From: Mika Kuns Date: Wed, 12 Aug 2026 09:39:10 +0200 Subject: [PATCH] feat(ui): separate OperationStatus per worktree action (refresh/cleanup/reset/force-remove/batch-merge) Replaces the shared IsBusy/IsMerging flags in WorktreesOverviewModalViewModel and the reset flow in WorktreesSettingsTabViewModel with dedicated OperationStatus instances shown via OperationIndicator, so a running Refresh no longer blocks the Cleanup indicator. ForceRemove gains a CanExecute guard against re-entrancy while it's running, and the reconcile tick's busy guard now checks all four action statuses instead of the old IsBusy||IsMerging pair. --- .../Settings/WorktreesSettingsTabViewModel.cs | 6 +- .../Modals/WorktreesOverviewModalViewModel.cs | 187 +++++++++--------- .../Views/Modals/SettingsModalView.axaml | 4 +- .../Modals/WorktreesOverviewModalView.axaml | 10 +- .../WorktreesOverviewBatchMergeTests.cs | 39 +++- .../WorktreesOverviewModalErrorTests.cs | 16 +- .../WorktreesOverviewReconcileTickTests.cs | 25 +++ .../WorktreesSettingsTabViewModelTests.cs | 2 +- 8 files changed, 189 insertions(+), 100 deletions(-) diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/Settings/WorktreesSettingsTabViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/Settings/WorktreesSettingsTabViewModel.cs index dc856723..6c3ebbe1 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/Settings/WorktreesSettingsTabViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/Settings/WorktreesSettingsTabViewModel.cs @@ -19,6 +19,8 @@ public sealed partial class WorktreesSettingsTabViewModel : ViewModelBase [ObservableProperty] private string _statusMessage = ""; [ObservableProperty] private bool _isBusy; + public OperationStatus ResetStatus { get; } = new(); + public IReadOnlyList WorktreeStrategies { get; } = new[] { "sibling", "central" }; public WorktreesSettingsTabViewModel(IWorkerClient worker) => _worker = worker; @@ -56,7 +58,8 @@ public sealed partial class WorktreesSettingsTabViewModel : ViewModelBase [RelayCommand] private async Task ConfirmResetAll() { - ShowResetConfirm = false; IsBusy = true; StatusMessage = ""; + ShowResetConfirm = false; StatusMessage = ""; + using var op = ResetStatus.Begin(Loc.T("ops.worktrees.resetting")); try { var r = await _worker.ResetAllWorktreesAsync(); @@ -65,6 +68,5 @@ public sealed partial class WorktreesSettingsTabViewModel : ViewModelBase else StatusMessage = Loc.T("vm.worktreesTab.removedFrom", r.Removed, r.TasksAffected); } catch (Exception ex) { StatusMessage = Loc.T("vm.worktreesTab.resetFailed", ex.Message); } - finally { IsBusy = false; } } } diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/WorktreesOverviewModalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/WorktreesOverviewModalViewModel.cs index 9d949859..2bd4f519 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/WorktreesOverviewModalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/WorktreesOverviewModalViewModel.cs @@ -75,14 +75,20 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase [ObservableProperty] private string? _listIdFilter; [ObservableProperty] private string _title = "Worktrees"; [ObservableProperty] private bool _isGlobal; - [ObservableProperty] private bool _isBusy; [ObservableProperty] private string? _statusMessage; [ObservableProperty] private WorktreeOverviewRowViewModel? _selectedRow; [ObservableProperty][NotifyCanExecuteChangedFor(nameof(MergeAllCommand))] private string? _selectedTarget; [ObservableProperty][NotifyCanExecuteChangedFor(nameof(MergeAllCommand))] private int _selectedCount; - [ObservableProperty][NotifyCanExecuteChangedFor(nameof(MergeAllCommand))] private bool _isMerging; [ObservableProperty] private string? _batchProgress; + // Separate per-action status: a running Refresh must not block the Cleanup indicator (and + // vice versa). OperationStatus is a nested ObservableObject, so [NotifyCanExecuteChangedFor] + // can't observe it directly — IsRunning changes are wired to NotifyCanExecuteChanged() below. + public OperationStatus RefreshStatus { get; } = new(); + public OperationStatus CleanupStatus { get; } = new(); + public OperationStatus ForceRemoveStatus { get; } = new(); + public OperationStatus BatchMergeStatus { get; } = new(); + public ObservableCollection Rows { get; } = new(); public ObservableCollection Groups { get; } = new(); public ObservableCollection MergeTargets { get; } = new(); @@ -100,6 +106,16 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase _worker = worker; _diffVmFactory = diffVmFactory; _merge = merge; + + BatchMergeStatus.PropertyChanged += (_, e) => + { + if (e.PropertyName == nameof(OperationStatus.IsRunning)) MergeAllCommand.NotifyCanExecuteChanged(); + }; + ForceRemoveStatus.PropertyChanged += (_, e) => + { + if (e.PropertyName == nameof(OperationStatus.IsRunning)) ForceRemoveCommand.NotifyCanExecuteChanged(); + }; + // Phase 3 reconcile tick: this overlay is long-lived (stays open while the user reviews // worktrees), so refresh it on the same cadence as TasksIslandViewModel's tick instead of // leaving it frozen at the moment it was opened. Skipped while a load or batch merge is @@ -119,7 +135,7 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase // finished batch's) across the reload, keyed by task id; genuinely new rows come up unticked. internal async Task ReconcileTickAsync() { - if (IsBusy || IsMerging) return; + if (RefreshStatus.IsRunning || CleanupStatus.IsRunning || ForceRemoveStatus.IsRunning || BatchMergeStatus.IsRunning) return; // Hold the row INSTANCES, not a value snapshot, and read them back only after the reload: // a tick the user lands during the await happens before LoadAsync clears the collection, @@ -168,54 +184,47 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase public async Task LoadAsync(CancellationToken ct = default) { - IsBusy = true; - try - { - var dtos = await _worker.GetWorktreesOverviewAsync(ListIdFilter); - var ordered = dtos - .OrderBy(d => d.State == WorktreeState.Active ? 0 : 1) - .ThenByDescending(d => d.CreatedAt) - .Select(Map) - .ToList(); + var dtos = await _worker.GetWorktreesOverviewAsync(ListIdFilter); + var ordered = dtos + .OrderBy(d => d.State == WorktreeState.Active ? 0 : 1) + .ThenByDescending(d => d.CreatedAt) + .Select(Map) + .ToList(); - Rows.Clear(); - Groups.Clear(); - ConflictRows.Clear(); - SelectedCount = 0; - BatchProgress = null; - if (IsGlobal) - { - foreach (var grp in ordered.GroupBy(r => (r.ListId, r.ListName)) - .OrderBy(g => g.Key.ListName, StringComparer.OrdinalIgnoreCase)) - { - var group = new WorktreesGroupViewModel { ListId = grp.Key.ListId, ListName = grp.Key.ListName }; - foreach (var row in grp) { HookRow(row); group.Rows.Add(row); } - Groups.Add(group); - } - } - else - { - foreach (var row in ordered) { HookRow(row); Rows.Add(row); } - } - await LoadMergeTargetsAsync(); - } - finally + Rows.Clear(); + Groups.Clear(); + ConflictRows.Clear(); + SelectedCount = 0; + BatchProgress = null; + if (IsGlobal) { - IsBusy = false; + foreach (var grp in ordered.GroupBy(r => (r.ListId, r.ListName)) + .OrderBy(g => g.Key.ListName, StringComparer.OrdinalIgnoreCase)) + { + var group = new WorktreesGroupViewModel { ListId = grp.Key.ListId, ListName = grp.Key.ListName }; + foreach (var row in grp) { HookRow(row); group.Rows.Add(row); } + Groups.Add(group); + } } + else + { + foreach (var row in ordered) { HookRow(row); Rows.Add(row); } + } + await LoadMergeTargetsAsync(); } [RelayCommand] - private Task Refresh() + private async Task Refresh() { StatusMessage = null; - return LoadAsync(); + using var op = RefreshStatus.Begin(Loc.T("ops.worktrees.refreshing")); + await LoadAsync(); } [RelayCommand] private async Task CleanupFinished() { - IsBusy = true; + using var op = CleanupStatus.Begin(Loc.T("ops.worktrees.cleaningUp")); try { var result = await _worker.CleanupFinishedWorktreesAsync(ListIdFilter); @@ -223,7 +232,6 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase await LoadAsync(); } catch (Exception ex) { StatusMessage = Loc.T("vm.worktreesOverview.cleanupFailedDetailed", ex.Message); } - finally { IsBusy = false; } } [RelayCommand] @@ -296,13 +304,16 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase else StatusMessage = err ?? Loc.T("vm.worktreesOverview.keepFailed"); } - [RelayCommand] + private bool CanForceRemove(WorktreeOverviewRowViewModel? row) => !ForceRemoveStatus.IsRunning; + + [RelayCommand(CanExecute = nameof(CanForceRemove))] private async Task ForceRemove(WorktreeOverviewRowViewModel? row) { if (row is null) return; if (row.IsRunning) { StatusMessage = Loc.T("vm.worktreesOverview.cannotForceRunning"); return; } if (ConfirmAction is not null && !await ConfirmAction($"Force remove worktree for '{row.TaskTitle}'? This deletes the directory and branch.")) return; + using var op = ForceRemoveStatus.Begin(Loc.T("ops.worktrees.forceRemoving")); ForceRemoveResultDto? result; try { @@ -396,7 +407,7 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase catch { MergeTargets.Clear(); SelectedTarget = null; } } - private bool CanMergeAll() => !IsMerging && SelectedCount > 0 && !string.IsNullOrWhiteSpace(SelectedTarget); + private bool CanMergeAll() => !BatchMergeStatus.IsRunning && SelectedCount > 0 && !string.IsNullOrWhiteSpace(SelectedTarget); [RelayCommand(CanExecute = nameof(CanMergeAll))] private Task MergeAll() => MergeSelectedAsync(_worker.MergeTaskAsync); @@ -426,62 +437,58 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase var selected = AllRows.Where(r => r.IsChecked && r.IsActive).ToList(); if (selected.Count == 0) return; - IsMerging = true; ConflictRows.Clear(); var done = 0; - try + using var op = BatchMergeStatus.Begin(Loc.T("ops.worktrees.batchMerging", done, selected.Count)); + foreach (var row in selected) { - foreach (var row in selected) + ct.ThrowIfCancellationRequested(); + row.MergeOutcome = BatchMergeOutcome.Merging; + BatchMergeStatus.Report(Loc.T("ops.worktrees.batchMerging", ++done, selected.Count)); + + MergeResultDto result; + try { - ct.ThrowIfCancellationRequested(); - row.MergeOutcome = BatchMergeOutcome.Merging; - BatchProgress = Loc.T("vm.worktreesOverview.batchProgress", ++done, selected.Count); + // Blank message: the worker builds the conventional default per task. + result = await mergeFn(row.TaskId, target!, false, ""); + } + catch + { + row.MergeOutcome = BatchMergeOutcome.Failed; + continue; + } - MergeResultDto result; - try - { - // Blank message: the worker builds the conventional default per task. - result = await mergeFn(row.TaskId, target!, false, ""); - } - catch - { + switch (result.Status) + { + case "merged": + row.MergeOutcome = BatchMergeOutcome.Merged; + row.State = WorktreeState.Merged; + row.IsChecked = false; + break; + case "conflict": + row.MergeOutcome = BatchMergeOutcome.Conflict; + ConflictRows.Add(row); + break; + case "blocked": + row.MergeOutcome = BatchMergeOutcome.Blocked; + break; + case "verify_failed": + // The merge landed (so the worktree really is merged), but the list's + // verify command failed and the task was kept out of Done. Reporting + // this as a plain Failed would claim the merge didn't happen. + row.MergeOutcome = BatchMergeOutcome.VerifyFailed; + row.State = WorktreeState.Merged; + row.IsChecked = false; + break; + default: row.MergeOutcome = BatchMergeOutcome.Failed; - continue; - } - - switch (result.Status) - { - case "merged": - row.MergeOutcome = BatchMergeOutcome.Merged; - row.State = WorktreeState.Merged; - row.IsChecked = false; - break; - case "conflict": - row.MergeOutcome = BatchMergeOutcome.Conflict; - ConflictRows.Add(row); - break; - case "blocked": - row.MergeOutcome = BatchMergeOutcome.Blocked; - break; - case "verify_failed": - // The merge landed (so the worktree really is merged), but the list's - // verify command failed and the task was kept out of Done. Reporting - // this as a plain Failed would claim the merge didn't happen. - row.MergeOutcome = BatchMergeOutcome.VerifyFailed; - row.State = WorktreeState.Merged; - row.IsChecked = false; - break; - default: - row.MergeOutcome = BatchMergeOutcome.Failed; - break; - } + break; } - BatchProgress = Loc.T("vm.worktreesOverview.batchDone", - selected.Count(r => r.MergeOutcome == BatchMergeOutcome.Merged), ConflictRows.Count); - } - finally - { - IsMerging = false; } + // Batch-Merge OperationStatus.ShowIndicator (and its live "i/n" label) hides once this + // method returns and `op` disposes — BatchProgress is the post-run summary that survives + // after the indicator disappears, not a duplicate of the live ticker. + BatchProgress = Loc.T("vm.worktreesOverview.batchDone", + selected.Count(r => r.MergeOutcome == BatchMergeOutcome.Merged), ConflictRows.Count); } } diff --git a/src/ClaudeDo.Ui/Views/Modals/SettingsModalView.axaml b/src/ClaudeDo.Ui/Views/Modals/SettingsModalView.axaml index dd8975ce..952907db 100644 --- a/src/ClaudeDo.Ui/Views/Modals/SettingsModalView.axaml +++ b/src/ClaudeDo.Ui/Views/Modals/SettingsModalView.axaml @@ -257,7 +257,9 @@