fix(ui): silent no-ops raise ErrorReported instead of swallowing failures (UX-Audit #1)

Stop/Enqueue/Dequeue/Reset&Retry (DetailsIslandViewModel), status/cancel/reject commands (TasksIslandViewModel), Mission Control's drag-enqueue and queue refresh, and "Open findings folder" (ListsIslandViewModel) used to catch {} or silently return on a blocked precondition. They now report through the existing ErrorReported -> FlashFooterError path, with new en/de locale keys and Ui.Tests covering each converted command.
2026-08-21 09:13:00 +02:00
parent fdd0e1e56a
commit 99bb5be5da
10 changed files with 468 additions and 20 deletions
@@ -0,0 +1,182 @@
+using ClaudeDo.Data;
+using ClaudeDo.Data.Models;
+using ClaudeDo.Localization;
+using ClaudeDo.Ui.Localization;
+using ClaudeDo.Ui.Services;
+using ClaudeDo.Ui.ViewModels.Islands;
+using Microsoft.EntityFrameworkCore;
+using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
+
+namespace ClaudeDo.Ui.Tests.ViewModels;
+
+// UX-Audit #1: Stop/Enqueue/Dequeue/Reset&Retry used to swallow worker failures with a
+// bare `catch { }` — nothing surfaced in the footer strip. These now raise ErrorReported.
+public class DetailsIslandErrorFeedbackTests : IDisposable
+{
+    private readonly string _dbPath;
+
+    public DetailsIslandErrorFeedbackTests()
+    {
+        _dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_details_error_feedback_test_{Guid.NewGuid():N}.db");
+        using var ctx = NewContext();
+        ctx.Database.EnsureCreated();
+
+        // Loc is a process-wide ambient singleton other tests also mutate — pin it to
+        // the real locale data so this test's assertions don't depend on run order.
+        var dir = AppContext.BaseDirectory;
+        while (dir is not null && !Directory.Exists(Path.Combine(dir, "src", "ClaudeDo.Localization", "locales")))
+            dir = Path.GetDirectoryName(dir);
+        Loc.Current = new Localizer(
+            LocaleStore.Load(Path.Combine(dir!, "src", "ClaudeDo.Localization", "locales")), "en");
+    }
+
+    public void Dispose()
+    {
+        try { File.Delete(_dbPath); } catch { }
+        try { File.Delete(_dbPath + "-wal"); } catch { }
+        try { File.Delete(_dbPath + "-shm"); } catch { }
+    }
+
+    private ClaudeDoDbContext NewContext()
+    {
+        var opts = new DbContextOptionsBuilder<ClaudeDoDbContext>()
+            .UseSqlite($"Data Source={_dbPath}")
+            .Options;
+        return new ClaudeDoDbContext(opts);
+    }
+
+    private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
+    {
+        private readonly Func<ClaudeDoDbContext> _create;
+        public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
+        public ClaudeDoDbContext CreateDbContext() => _create();
+    }
+
+    private sealed class NullServiceProvider : IServiceProvider
+    {
+        public object? GetService(Type serviceType) => null;
+    }
+
+    private sealed class StubNotesApi : ClaudeDo.Ui.Services.Interfaces.INotesApi
+    {
+        public Task<List<DailyNoteDto>> ListAsync(DateOnly day) =>
+            Task.FromResult(new List<DailyNoteDto>());
+        public Task<DailyNoteDto?> AddAsync(DateOnly day, string text) =>
+            Task.FromResult<DailyNoteDto?>(null);
+        public Task UpdateAsync(string id, string text) => Task.CompletedTask;
+        public Task DeleteAsync(string id) => Task.CompletedTask;
+    }
+
+    private sealed class ThrowingWorkerClient : StubWorkerClient
+    {
+        public bool Connected { get; set; } = true;
+        public override bool IsConnected => Connected;
+        public Exception? ThrowOnCancelTask;
+        public Exception? ThrowOnSetTaskStatus;
+
+        public override Task CancelTaskAsync(string taskId)
+        {
+            if (ThrowOnCancelTask is not null) throw ThrowOnCancelTask;
+            return Task.CompletedTask;
+        }
+
+        public override Task<BaseDirtyWarningDto?> SetTaskStatusAsync(string taskId, TaskStatus status)
+        {
+            if (ThrowOnSetTaskStatus is not null) throw ThrowOnSetTaskStatus;
+            return Task.FromResult<BaseDirtyWarningDto?>(null);
+        }
+    }
+
+    private DetailsIslandViewModel BuildVm(StubWorkerClient worker)
+    {
+        var factory = new TestDbFactory(NewContext);
+        return new DetailsIslandViewModel(
+            factory, worker, new NullServiceProvider(), new StubNotesApi(), new ClaudeDo.Ui.Services.MergeCoordinator());
+    }
+
+    [Fact]
+    public async Task Stop_WhenWorkerOffline_ReportsError()
+    {
+        var worker = new ThrowingWorkerClient { Connected = false };
+        var vm = BuildVm(worker);
+        vm.Bind(new TaskRowViewModel { Id = "task-stop-offline", Status = TaskStatus.Running });
+        vm.Monitor.ApplyState(TaskStatus.Running);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        await vm.StopCommand.ExecuteAsync(null);
+
+        Assert.NotNull(reportedError);
+        Assert.NotEqual(string.Empty, reportedError);
+    }
+
+    [Fact]
+    public async Task Stop_WhenWorkerThrows_ReportsError()
+    {
+        var worker = new ThrowingWorkerClient { ThrowOnCancelTask = new Exception("cancel slot busy") };
+        var vm = BuildVm(worker);
+        vm.Bind(new TaskRowViewModel { Id = "task-stop-throw", Status = TaskStatus.Running });
+        vm.Monitor.ApplyState(TaskStatus.Running);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        await vm.StopCommand.ExecuteAsync(null);
+
+        Assert.NotNull(reportedError);
+        Assert.Contains("cancel slot busy", reportedError);
+    }
+
+    [Fact]
+    public async Task Enqueue_WhenWorkerThrows_ReportsError()
+    {
+        var worker = new ThrowingWorkerClient { ThrowOnSetTaskStatus = new Exception("queue offline") };
+        var vm = BuildVm(worker);
+        vm.Bind(new TaskRowViewModel { Id = "task-enqueue-throw", Status = TaskStatus.Idle });
+        vm.Monitor.ApplyState(TaskStatus.Idle);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        await vm.EnqueueCommand.ExecuteAsync(null);
+
+        Assert.NotNull(reportedError);
+        Assert.Contains("queue offline", reportedError);
+    }
+
+    [Fact]
+    public async Task Dequeue_WhenWorkerThrows_ReportsError()
+    {
+        var worker = new ThrowingWorkerClient { ThrowOnSetTaskStatus = new Exception("dequeue offline") };
+        var vm = BuildVm(worker);
+        vm.Bind(new TaskRowViewModel { Id = "task-dequeue-throw", Status = TaskStatus.Queued });
+        vm.Monitor.ApplyState(TaskStatus.Queued);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        await vm.DequeueCommand.ExecuteAsync(null);
+
+        Assert.NotNull(reportedError);
+        Assert.Contains("dequeue offline", reportedError);
+    }
+
+    [Fact]
+    public async Task ResetAndRetry_WhenWorkerThrows_ReportsError()
+    {
+        var worker = new ThrowingWorkerClient { ThrowOnSetTaskStatus = new Exception("reset offline") };
+        var vm = BuildVm(worker);
+        vm.Bind(new TaskRowViewModel { Id = "task-reset-throw", Status = TaskStatus.Failed });
+        vm.Monitor.ApplyState(TaskStatus.Failed);
+        vm.ConfirmAsync = _ => Task.FromResult(true);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        await vm.ResetAndRetryCommand.ExecuteAsync(null);
+
+        Assert.NotNull(reportedError);
+        Assert.Contains("reset offline", reportedError);
+    }
+}
@@ -0,0 +1,73 @@
+using ClaudeDo.Data;
+using ClaudeDo.Localization;
+using ClaudeDo.Ui.Localization;
+using ClaudeDo.Ui.ViewModels.Islands;
+using Microsoft.EntityFrameworkCore;
+
+namespace ClaudeDo.Ui.Tests.ViewModels;
+
+// UX-Audit #1: "Open findings folder" used to do nothing when the list's repo has no
+// .claudedo/ folder yet — now it raises ErrorReported instead of a silent no-op.
+public class ListsIslandErrorFeedbackTests : IDisposable
+{
+    private readonly string _dbPath;
+
+    public ListsIslandErrorFeedbackTests()
+    {
+        _dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_lists_error_feedback_test_{Guid.NewGuid():N}.db");
+        using var ctx = NewContext();
+        ctx.Database.EnsureCreated();
+
+        var dir = AppContext.BaseDirectory;
+        while (dir is not null && !Directory.Exists(Path.Combine(dir, "src", "ClaudeDo.Localization", "locales")))
+            dir = Path.GetDirectoryName(dir);
+        Loc.Current = new Localizer(
+            LocaleStore.Load(Path.Combine(dir!, "src", "ClaudeDo.Localization", "locales")), "en");
+    }
+
+    public void Dispose()
+    {
+        try { File.Delete(_dbPath); } catch { }
+        try { File.Delete(_dbPath + "-wal"); } catch { }
+        try { File.Delete(_dbPath + "-shm"); } catch { }
+    }
+
+    private ClaudeDoDbContext NewContext()
+    {
+        var opts = new DbContextOptionsBuilder<ClaudeDoDbContext>()
+            .UseSqlite($"Data Source={_dbPath}")
+            .Options;
+        return new ClaudeDoDbContext(opts);
+    }
+
+    private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
+    {
+        private readonly Func<ClaudeDoDbContext> _create;
+        public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
+        public ClaudeDoDbContext CreateDbContext() => _create();
+    }
+
+    [Fact]
+    public void OpenFindings_WhenClaudedoFolderMissing_RaisesErrorReported()
+    {
+        var vm = new ListsIslandViewModel(new TestDbFactory(NewContext));
+        var workingDir = Path.Combine(Path.GetTempPath(), $"claudedo_findings_missing_{Guid.NewGuid():N}");
+        Directory.CreateDirectory(workingDir);
+        try
+        {
+            var row = new ListNavItemViewModel { Id = "L1", Kind = ListKind.User, WorkingDir = workingDir };
+
+            string? reportedError = null;
+            vm.ErrorReported += msg => reportedError = msg;
+
+            vm.OpenFindingsCommand.Execute(row);
+
+            Assert.NotNull(reportedError);
+            Assert.NotEqual(string.Empty, reportedError);
+        }
+        finally
+        {
+            try { Directory.Delete(workingDir, recursive: true); } catch { }
+        }
+    }
+}
@@ -278,14 +278,40 @@ public class MissionControlViewModelTests : IDisposable
        using var vm = BuildVm(worker);
        await vm.OpenConPtySessionAsync("t1");

+        string? error = null;
+        vm.ErrorReported += msg => error = msg;
+
        await vm.EnqueueTaskAsync("t1");

        Assert.Empty(worker.QueuedTaskIds);
+        Assert.NotNull(error);
        await using var verify = NewContext();
        var entity = await verify.Tasks.FirstAsync(t => t.Id == "t1");
        Assert.Equal(TaskStatus.Idle, entity.Status);
    }

+    private sealed class ThrowingSetStatusWorkerClient : StubWorkerClient
+    {
+        public Exception Error { get; init; } = new Exception("enqueue offline");
+        public override Task<BaseDirtyWarningDto?> SetTaskStatusAsync(string taskId, TaskStatus status) =>
+            throw Error;
+    }
+
+    [Fact]
+    public async Task EnqueueTaskAsync_WhenWorkerThrows_RaisesErrorReported()
+    {
+        var worker = new ThrowingSetStatusWorkerClient();
+        using var vm = BuildVm(worker);
+
+        string? error = null;
+        vm.ErrorReported += msg => error = msg;
+
+        await vm.EnqueueTaskAsync("t1");
+
+        Assert.NotNull(error);
+        Assert.Contains("enqueue offline", error);
+    }
+
    private sealed class ThrowingLaunchSpecWorker : StubWorkerClient
    {
        public override Task<LaunchSpec> GetInteractiveLaunchSpecAsync(string taskId, CancellationToken ct = default)
@@ -0,0 +1,149 @@
+using ClaudeDo.Data;
+using ClaudeDo.Data.Models;
+using ClaudeDo.Localization;
+using ClaudeDo.Ui.Localization;
+using ClaudeDo.Ui.Services;
+using ClaudeDo.Ui.ViewModels.Islands;
+using Microsoft.EntityFrameworkCore;
+using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
+
+namespace ClaudeDo.Ui.Tests.ViewModels;
+
+// UX-Audit #1: SetStatusOnRow/CancelRunningTask/RejectReviewToQueue/RejectReviewToIdle used
+// to swallow worker failures with a bare `catch { }` — nothing surfaced in the footer strip.
+// These now raise ErrorReported.
+public class TasksIslandErrorFeedbackTests : IDisposable
+{
+    private readonly string _dbPath;
+
+    public TasksIslandErrorFeedbackTests()
+    {
+        _dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_tasksisland_error_feedback_test_{Guid.NewGuid():N}.db");
+        using var ctx = NewContext();
+        ctx.Database.EnsureCreated();
+
+        var dir = AppContext.BaseDirectory;
+        while (dir is not null && !Directory.Exists(Path.Combine(dir, "src", "ClaudeDo.Localization", "locales")))
+            dir = Path.GetDirectoryName(dir);
+        Loc.Current = new Localizer(
+            LocaleStore.Load(Path.Combine(dir!, "src", "ClaudeDo.Localization", "locales")), "en");
+    }
+
+    public void Dispose()
+    {
+        try { File.Delete(_dbPath); } catch { }
+        try { File.Delete(_dbPath + "-wal"); } catch { }
+        try { File.Delete(_dbPath + "-shm"); } catch { }
+    }
+
+    private ClaudeDoDbContext NewContext()
+    {
+        var opts = new DbContextOptionsBuilder<ClaudeDoDbContext>()
+            .UseSqlite($"Data Source={_dbPath}")
+            .Options;
+        return new ClaudeDoDbContext(opts);
+    }
+
+    private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
+    {
+        private readonly Func<ClaudeDoDbContext> _create;
+        public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
+        public ClaudeDoDbContext CreateDbContext() => _create();
+    }
+
+    private sealed class ThrowingWorkerClient : StubWorkerClient
+    {
+        public Exception? ThrowOnSetTaskStatus;
+        public Exception? ThrowOnCancelTask;
+        public Exception? ThrowOnRejectToQueue;
+        public Exception? ThrowOnRejectToIdle;
+
+        public override Task<BaseDirtyWarningDto?> SetTaskStatusAsync(string taskId, TaskStatus status)
+        {
+            if (ThrowOnSetTaskStatus is not null) throw ThrowOnSetTaskStatus;
+            return Task.FromResult<BaseDirtyWarningDto?>(null);
+        }
+
+        public override Task CancelTaskAsync(string taskId)
+        {
+            if (ThrowOnCancelTask is not null) throw ThrowOnCancelTask;
+            return Task.CompletedTask;
+        }
+
+        public override Task RejectReviewToQueueAsync(string taskId, string feedback)
+        {
+            if (ThrowOnRejectToQueue is not null) throw ThrowOnRejectToQueue;
+            return Task.CompletedTask;
+        }
+
+        public override Task RejectReviewToIdleAsync(string taskId)
+        {
+            if (ThrowOnRejectToIdle is not null) throw ThrowOnRejectToIdle;
+            return Task.CompletedTask;
+        }
+    }
+
+    [Fact]
+    public async Task SetStatusOnRow_WhenWorkerThrows_RaisesErrorReported()
+    {
+        var worker = new ThrowingWorkerClient { ThrowOnSetTaskStatus = new Exception("status update offline") };
+        var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        var row = new TaskRowViewModel { Id = "task-status-1", Status = TaskStatus.Idle };
+        await vm.SetStatusOnRowAsync(row, TaskStatus.Queued);
+
+        Assert.NotNull(reportedError);
+        Assert.Contains("status update offline", reportedError);
+    }
+
+    [Fact]
+    public async Task CancelRunningTask_WhenWorkerThrows_RaisesErrorReported()
+    {
+        var worker = new ThrowingWorkerClient { ThrowOnCancelTask = new Exception("cancel offline") };
+        var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        var row = new TaskRowViewModel { Id = "task-cancel-1", Status = TaskStatus.Running };
+        await vm.CancelRunningTaskCommand.ExecuteAsync(row);
+
+        Assert.NotNull(reportedError);
+        Assert.Contains("cancel offline", reportedError);
+    }
+
+    [Fact]
+    public async Task RejectReviewToQueue_WhenWorkerThrows_RaisesErrorReported()
+    {
+        var worker = new ThrowingWorkerClient { ThrowOnRejectToQueue = new Exception("reject-to-queue offline") };
+        var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        var row = new TaskRowViewModel { Id = "task-reject-queue-1", Status = TaskStatus.WaitingForReview };
+        await vm.RejectReviewToQueueAsync(row, "needs another pass");
+
+        Assert.NotNull(reportedError);
+        Assert.Contains("reject-to-queue offline", reportedError);
+    }
+
+    [Fact]
+    public async Task RejectReviewToIdle_WhenWorkerThrows_RaisesErrorReported()
+    {
+        var worker = new ThrowingWorkerClient { ThrowOnRejectToIdle = new Exception("reject-to-idle offline") };
+        var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker);
+
+        string? reportedError = null;
+        vm.ErrorReported += msg => reportedError = msg;
+
+        var row = new TaskRowViewModel { Id = "task-reject-idle-1", Status = TaskStatus.WaitingForReview };
+        await vm.RejectReviewToIdleCommand.ExecuteAsync(row);
+
+        Assert.NotNull(reportedError);
+        Assert.Contains("reject-to-idle offline", reportedError);
+    }
+}