From a78cc526a6aef41648b858d44565df60733db906 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 6 Aug 2026 14:29:15 +0200 Subject: [PATCH] fix(ui): surface worker-offline failures when deleting a task DeleteTaskAsync had no IsConnected guard, unlike every other worker-dependent command in DetailsIslandViewModel, so an offline delete was a silent no-op. WorkerClient.DeleteTaskAsync also only caught HubException, letting the InvalidOperationException thrown by an inactive hub connection escape into the unobserved command task and vanish. Gate DeleteTaskCommand behind CanDeleteTask (Task != null && IsConnected), re-evaluate it on connection-state changes, widen WorkerClient to catch the connection-inactive case too, and wrap the ViewModel's call in try/catch as a second line of defense against a race between the guard and the call. --- src/ClaudeDo.Ui/Services/WorkerClient.cs | 5 +++ .../Islands/DetailsIslandViewModel.cs | 20 +++++++++- .../DetailsIslandDeleteTaskTests.cs | 37 ++++++++++++++++++- 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/ClaudeDo.Ui/Services/WorkerClient.cs b/src/ClaudeDo.Ui/Services/WorkerClient.cs index 2e089c9f..1fd45b7e 100644 --- a/src/ClaudeDo.Ui/Services/WorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/WorkerClient.cs @@ -338,6 +338,11 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC { return (false, ex.Message); } + catch (InvalidOperationException) + { + // Hub connection is not active (worker offline / not yet connected). + return (false, "Worker is offline. Reconnect and try again."); + } } public async Task WakeQueueAsync() diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index 3b155974..ba9680d7 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -355,6 +355,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable ContinueCommand.NotifyCanExecuteChanged(); SendRoadblockReplyCommand.NotifyCanExecuteChanged(); CancelReviewCommand.NotifyCanExecuteChanged(); + DeleteTaskCommand.NotifyCanExecuteChanged(); } }; _worker.PropertyChanged += _workerPropertyChangedHandler; @@ -963,7 +964,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable await repo.UpdateAsync(entity); } - [RelayCommand] + [RelayCommand(CanExecute = nameof(CanDeleteTask))] private async System.Threading.Tasks.Task DeleteTaskAsync() { if (Task == null) return; @@ -976,7 +977,20 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable // Routed through the worker (mirrors the MCP delete_task tool) so a deleted child // correctly advances a WaitingForChildren parent — a direct-repo delete from here used // to bypass TaskStateService.TryAdvanceParentAsync and could wedge the parent forever. - var (deleted, error) = await _worker.DeleteTaskAsync(row.Id); + bool deleted; + string? error; + try + { + (deleted, error) = await _worker.DeleteTaskAsync(row.Id); + } + catch (Exception ex) + { + // Belt and braces: the connection can drop between the CanExecute check and + // this call, so a stray throw here must surface, not vanish silently. + if (ShowErrorAsync != null) + await ShowErrorAsync(ex.Message); + return; + } if (!deleted) { if (ShowErrorAsync != null) @@ -988,6 +1002,8 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable CloseDetail?.Invoke(); } + private bool CanDeleteTask() => Task != null && _worker.IsConnected; + [RelayCommand] private async System.Threading.Tasks.Task CommitSubtaskEditAsync(SubtaskRowViewModel? row) { diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandDeleteTaskTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandDeleteTaskTests.cs index 3036b27a..185fedb1 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandDeleteTaskTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandDeleteTaskTests.cs @@ -57,12 +57,16 @@ public class DetailsIslandDeleteTaskTests : IDisposable private sealed class RecordingWorkerClient : StubWorkerClient { - public override bool IsConnected => true; + public override bool IsConnected { get; } = true; public (bool Ok, string? Error) Result { get; set; } = (true, null); public string? DeletedTaskId { get; private set; } + public Exception? ThrowOnDelete { get; set; } + + public RecordingWorkerClient(bool isConnected = true) => IsConnected = isConnected; public override Task<(bool Ok, string? Error)> DeleteTaskAsync(string taskId) { + if (ThrowOnDelete != null) throw ThrowOnDelete; DeletedTaskId = taskId; return Task.FromResult(Result); } @@ -114,4 +118,35 @@ public class DetailsIslandDeleteTaskTests : IDisposable Assert.False(deleteFromListCalled); Assert.False(closed); } + + [Fact] + public void DeleteTask_WhenWorkerOffline_CommandIsDisabled() + { + var worker = new RecordingWorkerClient(isConnected: false); + var vm = BuildVm(worker); + vm.Bind(new TaskRowViewModel { Id = "task-del-3", Status = TaskStatus.Idle }); + + Assert.False(vm.DeleteTaskCommand.CanExecute(null)); + } + + [Fact] + public async Task DeleteTask_WhenWorkerThrowsInvalidOperationException_SurfacesErrorAndKeepsDetailOpen() + { + var worker = new RecordingWorkerClient { ThrowOnDelete = new InvalidOperationException("Connection is not active.") }; + var vm = BuildVm(worker); + vm.Bind(new TaskRowViewModel { Id = "task-del-4", Status = TaskStatus.Idle }); + + var deleteFromListCalled = false; + vm.DeleteFromList = _ => { deleteFromListCalled = true; return Task.CompletedTask; }; + var closed = false; + vm.CloseDetail = () => closed = true; + string? reportedError = null; + vm.ShowErrorAsync = msg => { reportedError = msg; return Task.CompletedTask; }; + + await vm.DeleteTaskCommand.ExecuteAsync(null); + + Assert.Equal("Connection is not active.", reportedError); + Assert.False(deleteFromListCalled); + Assert.False(closed); + } }