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.
This commit is contained in:
@@ -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<string> 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; }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<WorktreeOverviewRowViewModel> Rows { get; } = new();
|
||||
public ObservableCollection<WorktreesGroupViewModel> Groups { get; } = new();
|
||||
public ObservableCollection<string> 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,
|
||||
@@ -167,9 +183,6 @@ 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
|
||||
@@ -199,23 +212,19 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase
|
||||
}
|
||||
await LoadMergeTargetsAsync();
|
||||
}
|
||||
finally
|
||||
{
|
||||
IsBusy = false;
|
||||
}
|
||||
}
|
||||
|
||||
[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,16 +437,14 @@ 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)
|
||||
{
|
||||
ct.ThrowIfCancellationRequested();
|
||||
row.MergeOutcome = BatchMergeOutcome.Merging;
|
||||
BatchProgress = Loc.T("vm.worktreesOverview.batchProgress", ++done, selected.Count);
|
||||
BatchMergeStatus.Report(Loc.T("ops.worktrees.batchMerging", ++done, selected.Count));
|
||||
|
||||
MergeResultDto result;
|
||||
try
|
||||
@@ -476,12 +485,10 @@ public sealed partial class WorktreesOverviewModalViewModel : ViewModelBase
|
||||
break;
|
||||
}
|
||||
}
|
||||
// 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);
|
||||
}
|
||||
finally
|
||||
{
|
||||
IsMerging = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -257,7 +257,9 @@
|
||||
<StackPanel Orientation="Horizontal" Spacing="8">
|
||||
<Button Classes="btn" Content="{loc:Tr settings.cancel}" Command="{Binding Worktrees.CancelResetConfirmCommand}"/>
|
||||
<Button Content="{loc:Tr settings.worktrees.removeAll}" Classes="danger"
|
||||
Command="{Binding Worktrees.ConfirmResetAllCommand}"/>
|
||||
Command="{Binding Worktrees.ConfirmResetAllCommand}"
|
||||
IsEnabled="{Binding !Worktrees.ResetStatus.IsRunning}"/>
|
||||
<ctl:OperationIndicator Status="{Binding Worktrees.ResetStatus}"/>
|
||||
</StackPanel>
|
||||
</StackPanel>
|
||||
</Border>
|
||||
|
||||
@@ -104,8 +104,10 @@
|
||||
BorderThickness="0,0,0,1"
|
||||
Padding="12,8">
|
||||
<StackPanel Orientation="Horizontal" Spacing="8">
|
||||
<Button Classes="btn" Content="{loc:Tr modals.worktreesOverview.refresh}" Command="{Binding RefreshCommand}" IsEnabled="{Binding !IsBusy}"/>
|
||||
<Button Classes="btn" Content="{loc:Tr modals.worktreesOverview.cleanupFinished}" Command="{Binding CleanupFinishedCommand}" IsEnabled="{Binding !IsBusy}"/>
|
||||
<Button Classes="btn" Content="{loc:Tr modals.worktreesOverview.refresh}" Command="{Binding RefreshCommand}" IsEnabled="{Binding !RefreshStatus.IsRunning}"/>
|
||||
<ctl:OperationIndicator Status="{Binding RefreshStatus}"/>
|
||||
<Button Classes="btn" Content="{loc:Tr modals.worktreesOverview.cleanupFinished}" Command="{Binding CleanupFinishedCommand}" IsEnabled="{Binding !CleanupStatus.IsRunning}"/>
|
||||
<ctl:OperationIndicator Status="{Binding CleanupStatus}"/>
|
||||
<!-- Batch merge is per-list only: a single target branch is meaningless across repos. -->
|
||||
<StackPanel Orientation="Horizontal" Spacing="8" IsVisible="{Binding !IsGlobal}">
|
||||
<Button Classes="btn" Content="{loc:Tr modals.worktreesOverview.selectAll}" Command="{Binding ToggleSelectAllCommand}"/>
|
||||
@@ -117,11 +119,15 @@
|
||||
<Button Classes="btn accent"
|
||||
Content="{loc:Tr modals.worktreesOverview.mergeAll}"
|
||||
Command="{Binding MergeAllCommand}"/>
|
||||
<ctl:OperationIndicator Status="{Binding BatchMergeStatus}"/>
|
||||
<TextBlock Text="{Binding SelectedCount, StringFormat='{}{0} selected'}"
|
||||
VerticalAlignment="Center" Foreground="{DynamicResource TextDimBrush}"/>
|
||||
<!-- Post-run summary only ("N merged, M conflicts"); the live i/n ticker lives in the
|
||||
OperationIndicator's label above so the two don't show the same count twice. -->
|
||||
<TextBlock Text="{Binding BatchProgress}" VerticalAlignment="Center" Margin="8,0,0,0"
|
||||
Foreground="{DynamicResource TextDimBrush}"/>
|
||||
</StackPanel>
|
||||
<ctl:OperationIndicator Status="{Binding ForceRemoveStatus}"/>
|
||||
<TextBlock Text="{Binding StatusMessage}" VerticalAlignment="Center" Margin="8,0,0,0"
|
||||
Foreground="{DynamicResource TextDimBrush}"/>
|
||||
</StackPanel>
|
||||
|
||||
@@ -98,7 +98,7 @@ public class WorktreesOverviewBatchMergeTests
|
||||
Assert.Equal(BatchMergeOutcome.Blocked, a.MergeOutcome);
|
||||
Assert.Equal(BatchMergeOutcome.Failed, b.MergeOutcome);
|
||||
Assert.Empty(vm.ConflictRows);
|
||||
Assert.False(vm.IsMerging);
|
||||
Assert.False(vm.BatchMergeStatus.IsRunning);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -164,7 +164,40 @@ public class WorktreesOverviewBatchMergeTests
|
||||
Assert.False(vm.MergeAllCommand.CanExecute(null));
|
||||
vm.SelectedTarget = "main";
|
||||
Assert.True(vm.MergeAllCommand.CanExecute(null));
|
||||
vm.IsMerging = true;
|
||||
using (vm.BatchMergeStatus.Begin("merging"))
|
||||
{
|
||||
Assert.False(vm.MergeAllCommand.CanExecute(null));
|
||||
}
|
||||
Assert.True(vm.MergeAllCommand.CanExecute(null));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void MergeAll_CanExecuteChanged_fires_when_batch_status_changes()
|
||||
{
|
||||
var vm = NewVm();
|
||||
var a = ActiveRow("a"); a.IsChecked = true;
|
||||
vm.AddRowForTest(a);
|
||||
vm.SelectedTarget = "main";
|
||||
|
||||
var raised = 0;
|
||||
vm.MergeAllCommand.CanExecuteChanged += (_, _) => raised++;
|
||||
|
||||
using (vm.BatchMergeStatus.Begin("merging")) { }
|
||||
|
||||
Assert.Equal(2, raised);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ForceRemove_canExecute_is_false_while_a_removal_is_in_flight()
|
||||
{
|
||||
var vm = NewVm();
|
||||
var row = ActiveRow("a");
|
||||
|
||||
Assert.True(vm.ForceRemoveCommand.CanExecute(row));
|
||||
using (vm.ForceRemoveStatus.Begin("removing"))
|
||||
{
|
||||
Assert.False(vm.ForceRemoveCommand.CanExecute(row));
|
||||
}
|
||||
Assert.True(vm.ForceRemoveCommand.CanExecute(row));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,6 +20,8 @@ public class WorktreesOverviewModalErrorTests
|
||||
private sealed class ThrowingWorker : StubWorkerClient
|
||||
{
|
||||
public string ExceptionMessage { get; init; } = "worktree is locked by another process";
|
||||
public override Task<List<WorktreeOverviewDto>> GetWorktreesOverviewAsync(string? listId) =>
|
||||
throw new Exception(ExceptionMessage);
|
||||
public override Task<WorktreeCleanupDto?> CleanupFinishedWorktreesAsync(string? listId = null) =>
|
||||
throw new Exception(ExceptionMessage);
|
||||
public override Task<ForceRemoveResultDto?> ForceRemoveWorktreeAsync(string taskId) =>
|
||||
@@ -39,7 +41,7 @@ public class WorktreesOverviewModalErrorTests
|
||||
|
||||
Assert.NotNull(vm.StatusMessage);
|
||||
Assert.Contains(worker.ExceptionMessage, vm.StatusMessage);
|
||||
Assert.False(vm.IsBusy);
|
||||
Assert.False(vm.CleanupStatus.IsRunning);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -61,5 +63,17 @@ public class WorktreesOverviewModalErrorTests
|
||||
Assert.NotNull(vm.StatusMessage);
|
||||
Assert.Contains(worker.ExceptionMessage, vm.StatusMessage);
|
||||
Assert.Contains(row, vm.Rows);
|
||||
Assert.False(vm.ForceRemoveStatus.IsRunning);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Refresh_WhenWorkerThrows_ResetsRefreshStatus()
|
||||
{
|
||||
var worker = new ThrowingWorker();
|
||||
var vm = NewVm(worker);
|
||||
|
||||
await Assert.ThrowsAsync<Exception>(() => vm.RefreshCommand.ExecuteAsync(null));
|
||||
|
||||
Assert.False(vm.RefreshStatus.IsRunning);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -117,4 +117,29 @@ public class WorktreesOverviewReconcileTickTests
|
||||
Assert.False(vm.Rows.Single(r => r.TaskId == "d").IsChecked);
|
||||
Assert.Equal(1, vm.SelectedCount);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("refresh")]
|
||||
[InlineData("cleanup")]
|
||||
[InlineData("forceRemove")]
|
||||
[InlineData("batchMerge")]
|
||||
public async Task Tick_skips_the_reload_while_any_action_status_is_running(string which)
|
||||
{
|
||||
var (vm, worker) = await BuildLoadedAsync();
|
||||
var status = which switch
|
||||
{
|
||||
"refresh" => vm.RefreshStatus,
|
||||
"cleanup" => vm.CleanupStatus,
|
||||
"forceRemove" => vm.ForceRemoveStatus,
|
||||
"batchMerge" => vm.BatchMergeStatus,
|
||||
_ => throw new ArgumentOutOfRangeException(nameof(which)),
|
||||
};
|
||||
|
||||
using var op = status.Begin("running");
|
||||
worker.Worktrees.Add(Wt("d"));
|
||||
|
||||
await vm.ReconcileTickAsync();
|
||||
|
||||
Assert.DoesNotContain(vm.Rows, r => r.TaskId == "d");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,7 +47,7 @@ public class WorktreesSettingsTabViewModelTests
|
||||
await vm.ConfirmResetAllCommand.ExecuteAsync(null);
|
||||
|
||||
Assert.Contains(worker.ExceptionMessage, vm.StatusMessage);
|
||||
Assert.False(vm.IsBusy);
|
||||
Assert.False(vm.ResetStatus.IsRunning);
|
||||
Assert.False(vm.ShowResetConfirm);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user