fix(ui): stop app crash when approving review after Merge all
The Details island review commands (Approve/Reject/Park/Cancel) invoked the hub without catching exceptions. After "Merge all" folds the parent out of WaitingForReview, pressing Approve made the hub throw a HubException, which escaped the generated AsyncRelayCommand as an unobserved async-void exception and crashed the app. Wrap the calls in try/catch like the Tasks island does; the TaskUpdated broadcast reconciles the UI. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -1267,7 +1267,11 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase
|
|||||||
private async System.Threading.Tasks.Task ApproveReviewAsync()
|
private async System.Threading.Tasks.Task ApproveReviewAsync()
|
||||||
{
|
{
|
||||||
if (Task is null || !_worker.IsConnected) return;
|
if (Task is null || !_worker.IsConnected) return;
|
||||||
await _worker.ApproveReviewAsync(Task.Id);
|
// The hub rejects (HubException) if the task is no longer WaitingForReview
|
||||||
|
// — e.g. after "Merge all" folded the parent. Swallow it; the TaskUpdated
|
||||||
|
// broadcast reconciles the UI. An unhandled command exception would crash.
|
||||||
|
try { await _worker.ApproveReviewAsync(Task.Id); }
|
||||||
|
catch { /* stale review action; broadcast reconciles */ }
|
||||||
}
|
}
|
||||||
|
|
||||||
[RelayCommand]
|
[RelayCommand]
|
||||||
@@ -1276,7 +1280,8 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase
|
|||||||
if (Task is null || !_worker.IsConnected) return;
|
if (Task is null || !_worker.IsConnected) return;
|
||||||
var feedback = ReviewFeedback;
|
var feedback = ReviewFeedback;
|
||||||
if (string.IsNullOrWhiteSpace(feedback)) return;
|
if (string.IsNullOrWhiteSpace(feedback)) return;
|
||||||
await _worker.RejectReviewToQueueAsync(Task.Id, feedback);
|
try { await _worker.RejectReviewToQueueAsync(Task.Id, feedback); }
|
||||||
|
catch { /* stale review action; broadcast reconciles */ return; }
|
||||||
ReviewFeedback = "";
|
ReviewFeedback = "";
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1284,14 +1289,16 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase
|
|||||||
private async System.Threading.Tasks.Task ParkReviewAsync()
|
private async System.Threading.Tasks.Task ParkReviewAsync()
|
||||||
{
|
{
|
||||||
if (Task is null || !_worker.IsConnected) return;
|
if (Task is null || !_worker.IsConnected) return;
|
||||||
await _worker.RejectReviewToIdleAsync(Task.Id);
|
try { await _worker.RejectReviewToIdleAsync(Task.Id); }
|
||||||
|
catch { /* stale review action; broadcast reconciles */ }
|
||||||
}
|
}
|
||||||
|
|
||||||
[RelayCommand]
|
[RelayCommand]
|
||||||
private async System.Threading.Tasks.Task CancelReviewAsync()
|
private async System.Threading.Tasks.Task CancelReviewAsync()
|
||||||
{
|
{
|
||||||
if (Task is null || !_worker.IsConnected) return;
|
if (Task is null || !_worker.IsConnected) return;
|
||||||
await _worker.CancelReviewAsync(Task.Id);
|
try { await _worker.CancelReviewAsync(Task.Id); }
|
||||||
|
catch { /* stale review action; broadcast reconciles */ }
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Diff meter parser ───────────────────────────────────────────────────────
|
// ── Diff meter parser ───────────────────────────────────────────────────────
|
||||||
|
|||||||
@@ -63,12 +63,21 @@ public class DetailsIslandPlanningTests : IDisposable
|
|||||||
public Task DeleteAsync(string id) => Task.CompletedTask;
|
public Task DeleteAsync(string id) => Task.CompletedTask;
|
||||||
}
|
}
|
||||||
|
|
||||||
private DetailsIslandViewModel BuildVm(FakeWorkerClient worker)
|
private DetailsIslandViewModel BuildVm(StubWorkerClient worker)
|
||||||
{
|
{
|
||||||
var factory = new TestDbFactory(NewContext);
|
var factory = new TestDbFactory(NewContext);
|
||||||
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi());
|
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Connected worker whose review calls fail the way the hub does when the task
|
||||||
|
// is no longer WaitingForReview (e.g. after "Merge all" folded the parent).
|
||||||
|
private sealed class ThrowingReviewWorkerClient : StubWorkerClient
|
||||||
|
{
|
||||||
|
public override bool IsConnected => true;
|
||||||
|
public override Task ApproveReviewAsync(string taskId) =>
|
||||||
|
Task.FromException(new InvalidOperationException("Task is not waiting for review; cannot approve."));
|
||||||
|
}
|
||||||
|
|
||||||
private static SubtaskRowViewModel MakeSubtask(TaskStatus status, WorktreeState wt = WorktreeState.Active) =>
|
private static SubtaskRowViewModel MakeSubtask(TaskStatus status, WorktreeState wt = WorktreeState.Active) =>
|
||||||
new() { Id = Guid.NewGuid().ToString(), Title = "t", Status = status, WorktreeState = wt };
|
new() { Id = Guid.NewGuid().ToString(), Title = "t", Status = status, WorktreeState = wt };
|
||||||
|
|
||||||
@@ -135,6 +144,21 @@ public class DetailsIslandPlanningTests : IDisposable
|
|||||||
vm.MergeAllDisabledReason.Contains("discarded", StringComparison.OrdinalIgnoreCase));
|
vm.MergeAllDisabledReason.Contains("discarded", StringComparison.OrdinalIgnoreCase));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── Review-action resilience: a failing hub call must not crash the app ───
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ApproveReview_WhenWorkerThrows_DoesNotPropagate()
|
||||||
|
{
|
||||||
|
var vm = BuildVm(new ThrowingReviewWorkerClient());
|
||||||
|
vm.Bind(new TaskRowViewModel { Id = "p", Status = TaskStatus.WaitingForReview });
|
||||||
|
|
||||||
|
// Before the fix this surfaced the HubException as an unobserved
|
||||||
|
// async-void exception from the command, crashing the process.
|
||||||
|
var ex = await Record.ExceptionAsync(() => vm.ApproveReviewCommand.ExecuteAsync(null));
|
||||||
|
|
||||||
|
Assert.Null(ex);
|
||||||
|
}
|
||||||
|
|
||||||
// ── Branch-load test exercising the VM via Bind ──────────────────────────
|
// ── Branch-load test exercising the VM via Bind ──────────────────────────
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user