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.
This commit is contained in:
@@ -338,6 +338,11 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC
|
|||||||
{
|
{
|
||||||
return (false, ex.Message);
|
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()
|
public async Task WakeQueueAsync()
|
||||||
|
|||||||
@@ -355,6 +355,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
|
|||||||
ContinueCommand.NotifyCanExecuteChanged();
|
ContinueCommand.NotifyCanExecuteChanged();
|
||||||
SendRoadblockReplyCommand.NotifyCanExecuteChanged();
|
SendRoadblockReplyCommand.NotifyCanExecuteChanged();
|
||||||
CancelReviewCommand.NotifyCanExecuteChanged();
|
CancelReviewCommand.NotifyCanExecuteChanged();
|
||||||
|
DeleteTaskCommand.NotifyCanExecuteChanged();
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
_worker.PropertyChanged += _workerPropertyChangedHandler;
|
_worker.PropertyChanged += _workerPropertyChangedHandler;
|
||||||
@@ -963,7 +964,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
|
|||||||
await repo.UpdateAsync(entity);
|
await repo.UpdateAsync(entity);
|
||||||
}
|
}
|
||||||
|
|
||||||
[RelayCommand]
|
[RelayCommand(CanExecute = nameof(CanDeleteTask))]
|
||||||
private async System.Threading.Tasks.Task DeleteTaskAsync()
|
private async System.Threading.Tasks.Task DeleteTaskAsync()
|
||||||
{
|
{
|
||||||
if (Task == null) return;
|
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
|
// 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
|
// correctly advances a WaitingForChildren parent — a direct-repo delete from here used
|
||||||
// to bypass TaskStateService.TryAdvanceParentAsync and could wedge the parent forever.
|
// 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 (!deleted)
|
||||||
{
|
{
|
||||||
if (ShowErrorAsync != null)
|
if (ShowErrorAsync != null)
|
||||||
@@ -988,6 +1002,8 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
|
|||||||
CloseDetail?.Invoke();
|
CloseDetail?.Invoke();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private bool CanDeleteTask() => Task != null && _worker.IsConnected;
|
||||||
|
|
||||||
[RelayCommand]
|
[RelayCommand]
|
||||||
private async System.Threading.Tasks.Task CommitSubtaskEditAsync(SubtaskRowViewModel? row)
|
private async System.Threading.Tasks.Task CommitSubtaskEditAsync(SubtaskRowViewModel? row)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -57,12 +57,16 @@ public class DetailsIslandDeleteTaskTests : IDisposable
|
|||||||
|
|
||||||
private sealed class RecordingWorkerClient : StubWorkerClient
|
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 (bool Ok, string? Error) Result { get; set; } = (true, null);
|
||||||
public string? DeletedTaskId { get; private set; }
|
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)
|
public override Task<(bool Ok, string? Error)> DeleteTaskAsync(string taskId)
|
||||||
{
|
{
|
||||||
|
if (ThrowOnDelete != null) throw ThrowOnDelete;
|
||||||
DeletedTaskId = taskId;
|
DeletedTaskId = taskId;
|
||||||
return Task.FromResult(Result);
|
return Task.FromResult(Result);
|
||||||
}
|
}
|
||||||
@@ -114,4 +118,35 @@ public class DetailsIslandDeleteTaskTests : IDisposable
|
|||||||
Assert.False(deleteFromListCalled);
|
Assert.False(deleteFromListCalled);
|
||||||
Assert.False(closed);
|
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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user