From 99bb5be5da8b4f2850ffbe5547bcbe7b741ea8a1 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Fri, 21 Aug 2026 09:13:00 +0200 Subject: [PATCH] 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. --- src/ClaudeDo.Localization/locales/de.json | 9 +- src/ClaudeDo.Localization/locales/en.json | 9 +- .../Islands/DetailsIslandViewModel.cs | 14 +- .../Islands/ListsIslandViewModel.cs | 8 +- .../Islands/TasksIslandViewModel.cs | 8 +- .../ViewModels/MissionControlViewModel.cs | 10 +- .../DetailsIslandErrorFeedbackTests.cs | 182 ++++++++++++++++++ .../ListsIslandErrorFeedbackTests.cs | 73 +++++++ .../MissionControlViewModelTests.cs | 26 +++ .../TasksIslandErrorFeedbackTests.cs | 149 ++++++++++++++ 10 files changed, 468 insertions(+), 20 deletions(-) create mode 100644 tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandErrorFeedbackTests.cs create mode 100644 tests/ClaudeDo.Ui.Tests/ViewModels/ListsIslandErrorFeedbackTests.cs create mode 100644 tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandErrorFeedbackTests.cs diff --git a/src/ClaudeDo.Localization/locales/de.json b/src/ClaudeDo.Localization/locales/de.json index 9d6a4eb6..6cc41b17 100644 --- a/src/ClaudeDo.Localization/locales/de.json +++ b/src/ClaudeDo.Localization/locales/de.json @@ -293,6 +293,9 @@ "retry": "Erneut versuchen", "retryTip": "Diese Sitzung erneut starten", "planningTitleSuffix": " (Planung)", + "enqueueAlreadyOpen": "Kann nicht in die Warteschlange gestellt werden — dieser Task hat eine offene interaktive Sitzung.", + "enqueueFailed": "In die Warteschlange stellen fehlgeschlagen: {0}", + "queueRefreshFailed": "Warteschlange konnte nicht aktualisiert werden: {0}", "question": { "title": "Claude fragt nach", "placeholder": "Antwort eingeben…", @@ -670,7 +673,7 @@ "planningBadge": { "active": "PLANUNG", "finalized": "GEPLANT" }, "taskRow": { "createdPrefix": "Erstellt {0}", "stepsText": "{0}/{1} Schritte" }, "queue": { "baseDirtyWarning": "Achtung: Das Repo dieser Liste hat uncommittete Änderungen ({0} geändert, {1} untracked) — ein neuer Worktree startet vom letzten Commit und enthält sie nicht." }, - "tasksIsland": { "completedHeader": "ABGESCHLOSSEN", "completedHeaderCount": "ABGESCHLOSSEN · {0}", "planningOpenFailed": "Planungssitzung konnte nicht geöffnet werden: {0}", "planningResumeFailed": "Planungssitzung konnte nicht fortgesetzt werden: {0}", "approveFailed": "Genehmigen & Mergen fehlgeschlagen: {0}", "cancelReviewFailed": "Prüfung abbrechen fehlgeschlagen: {0}", "sendToQueueFailed": "In die Warteschlange stellen fehlgeschlagen: {0}", "queuePlanBlockedInteractive": "Plan kann nicht in die Warteschlange gestellt werden — {0} hat eine offene interaktive Sitzung und muss zuerst geschlossen werden.", "moveRunningRejected": "Ein laufender Task kann nicht in eine andere Liste verschoben werden.", "moveWorktreeRejected": "Verschieben nicht möglich — dieser Task hat einen aktiven Worktree, der auf sein aktuelles Repo zeigt.", "moveRepoConfirm": "Unterschiedliche Repos — {0} → {1}. Task trotzdem verschieben?", "moveConfirmUnavailable": "Verschieben nicht möglich — der Bestätigungsdialog ist nicht verfügbar.", "quickClaudeNoWorkingDir": "Für diese Liste ist kein Arbeitsverzeichnis konfiguriert.", "quickClaudeDirMissing": "Arbeitsverzeichnis existiert nicht mehr: {0}" }, + "tasksIsland": { "completedHeader": "ABGESCHLOSSEN", "completedHeaderCount": "ABGESCHLOSSEN · {0}", "planningOpenFailed": "Planungssitzung konnte nicht geöffnet werden: {0}", "planningResumeFailed": "Planungssitzung konnte nicht fortgesetzt werden: {0}", "approveFailed": "Genehmigen & Mergen fehlgeschlagen: {0}", "cancelReviewFailed": "Prüfung abbrechen fehlgeschlagen: {0}", "sendToQueueFailed": "In die Warteschlange stellen fehlgeschlagen: {0}", "queuePlanBlockedInteractive": "Plan kann nicht in die Warteschlange gestellt werden — {0} hat eine offene interaktive Sitzung und muss zuerst geschlossen werden.", "moveRunningRejected": "Ein laufender Task kann nicht in eine andere Liste verschoben werden.", "moveWorktreeRejected": "Verschieben nicht möglich — dieser Task hat einen aktiven Worktree, der auf sein aktuelles Repo zeigt.", "moveRepoConfirm": "Unterschiedliche Repos — {0} → {1}. Task trotzdem verschieben?", "moveConfirmUnavailable": "Verschieben nicht möglich — der Bestätigungsdialog ist nicht verfügbar.", "quickClaudeNoWorkingDir": "Für diese Liste ist kein Arbeitsverzeichnis konfiguriert.", "quickClaudeDirMissing": "Arbeitsverzeichnis existiert nicht mehr: {0}", "setStatusFailed": "Status konnte nicht aktualisiert werden: {0}", "cancelFailed": "Abbrechen fehlgeschlagen: {0}", "rejectToQueueFailed": "Zurückweisen in die Warteschlange fehlgeschlagen: {0}", "rejectToIdleFailed": "Zurückweisen fehlgeschlagen: {0}" }, "diff": { "loadFailed": "Diff konnte nicht geladen werden: {0}", "noChanges": "Keine Änderungen anzuzeigen.", "unavailable": "Diff nicht mehr verfügbar — Commit-Bereich unvollständig." }, "planningDiff": { "hubError": "Kombinierte Vorschau konnte nicht erstellt werden (Hub-Fehler).", "conflict": "Kombinierte Vorschau nicht möglich: Teilaufgabe {0} steht im Konflikt mit einer früheren Teilaufgabe ({1} Dateien).", "buildFailed": "Kombinierte Vorschau konnte nicht erstellt werden: {0}" }, "merge": { "commitMessage": "chore: merge {0}", "progressMerging": "Wird zusammengeführt…", "progressVerifying": "Verify-Kommando der Liste läuft… ({0})", "workerOfflineBranches": "Worker offline — Branches können nicht aufgelistet werden.", "loadBranchesFailed": "Branches konnten nicht geladen werden: {0}", "merged": "Zusammengeführt.", "conflict": "Merge-Konflikt — Ziel-Branch wiederhergestellt. Manuell oder über Fortsetzen lösen, dann erneut versuchen.", "blocked": "Blockiert: {0}", "verifyFailed": "Merge ist gelandet, aber das Verify-Kommando der Liste ist fehlgeschlagen — die Aufgabe wurde nicht auf Erledigt gesetzt.", "untrackedCollision": "Merge abgelehnt — er würde eine unversionierte Datei im Ziel-Arbeitsverzeichnis überschreiben.", "unknownStatus": "Unbekannter Status: {0}", "mergeFailed": "Merge fehlgeschlagen: {0}" }, @@ -688,8 +691,8 @@ "worktreesTab": { "workerOffline": "Worker offline.", "removed": "{0} Worktree(s) entfernt.", "blocked": "Zwangsentfernung nicht möglich: {0} Aufgabe(n) laufen noch. Brich sie zuerst ab.", "removedFrom": "{0} Worktree(s) von {1} Aufgabe(n) entfernt.", "cleanupFailed": "Aufräumen fehlgeschlagen: {0}", "resetFailed": "Zurücksetzen fehlgeschlagen: {0}" }, "worktreesOverview": { "titleAll": "Worktrees", "titleList": "Worktrees — {0}", "listFallback": "Liste", "cleanupFailed": "Aufräumen fehlgeschlagen.", "cleanupFailedDetailed": "Aufräumen fehlgeschlagen: {0}", "removed": "{0} Worktree(s) entfernt.", "discardFailed": "Worktree konnte nicht verworfen werden.", "keepFailed": "Worktree konnte nicht behalten werden.", "cannotForceRunning": "Eine laufende Aufgabe kann nicht zwangsweise entfernt werden.", "forceRemoveFailed": "Zwangsentfernung fehlgeschlagen.", "forceRemoveFailedDetailed": "Zwangsentfernung fehlgeschlagen: {0}", "batchProgress": "Merge {0}/{1}…", "batchDone": "{0} gemergt, {1} zu lösen." }, "listSettings": { "untitled": "Unbenannt" }, - "detailsIsland": { "verifyFailed": "Merge ist erfolgt, aber das Verifikationskommando der Liste ist fehlgeschlagen — die Aufgabe wurde nicht auf 'Erledigt' gesetzt.", "untrackedCollision": "Merge abgelehnt — er würde eine unversionierte Datei im Ziel-Arbeitsverzeichnis überschreiben." }, - "lists": { "localSuffix": "{0} / lokal", "smartMyDay": "Mein Tag", "smartImportant": "Wichtig", "smartPlanned": "Geplant", "virtualQueue": "Warteschlange", "virtualRunning": "Läuft", "virtualReview": "Prüfung", "newList": "Neue Liste" }, + "detailsIsland": { "verifyFailed": "Merge ist erfolgt, aber das Verifikationskommando der Liste ist fehlgeschlagen — die Aufgabe wurde nicht auf 'Erledigt' gesetzt.", "untrackedCollision": "Merge abgelehnt — er würde eine unversionierte Datei im Ziel-Arbeitsverzeichnis überschreiben.", "stopOffline": "Worker offline — Task kann nicht gestoppt werden.", "stopFailed": "Stoppen fehlgeschlagen: {0}", "enqueueFailed": "In die Warteschlange stellen fehlgeschlagen: {0}", "dequeueFailed": "Aus der Warteschlange entfernen fehlgeschlagen: {0}", "resetAndRetryFailed": "Zurücksetzen & erneut versuchen fehlgeschlagen: {0}" }, + "lists": { "localSuffix": "{0} / lokal", "smartMyDay": "Mein Tag", "smartImportant": "Wichtig", "smartPlanned": "Geplant", "virtualQueue": "Warteschlange", "virtualRunning": "Läuft", "virtualReview": "Prüfung", "newList": "Neue Liste", "findingsNotFound": "Für diese Liste gibt es noch keine Findings.", "findingsOpenFailed": "Findings-Ordner konnte nicht geöffnet werden: {0}" }, "repoImport": { "loadFailed": "Gespeicherte Ordner konnten nicht geladen werden: {0}", "saveFailed": "Ordner konnten nicht gespeichert werden: {0}" } }, "ops": { diff --git a/src/ClaudeDo.Localization/locales/en.json b/src/ClaudeDo.Localization/locales/en.json index 35ce84f3..0b8e6a72 100644 --- a/src/ClaudeDo.Localization/locales/en.json +++ b/src/ClaudeDo.Localization/locales/en.json @@ -293,6 +293,9 @@ "retry": "Retry", "retryTip": "Try launching this session again", "planningTitleSuffix": " (Planning)", + "enqueueAlreadyOpen": "Can't queue — this task has an open interactive session.", + "enqueueFailed": "Queue failed: {0}", + "queueRefreshFailed": "Couldn't refresh the queue: {0}", "question": { "title": "Claude is asking", "placeholder": "Type your answer…", @@ -670,7 +673,7 @@ "planningBadge": { "active": "PLANNING", "finalized": "PLANNED" }, "taskRow": { "createdPrefix": "Created {0}", "stepsText": "{0}/{1} steps" }, "queue": { "baseDirtyWarning": "Heads up: this list's repo has uncommitted changes ({0} modified, {1} untracked) — a new worktree starts from the last commit and won't include them." }, - "tasksIsland": { "completedHeader": "COMPLETED", "completedHeaderCount": "COMPLETED · {0}", "planningOpenFailed": "Couldn't open planning session: {0}", "planningResumeFailed": "Couldn't resume planning session: {0}", "approveFailed": "Approve & merge failed: {0}", "cancelReviewFailed": "Cancel review failed: {0}", "sendToQueueFailed": "Send to queue failed: {0}", "queuePlanBlockedInteractive": "Can't queue the plan — {0} has an open interactive session and must be closed first.", "moveRunningRejected": "Can't move a running task to another list.", "moveWorktreeRejected": "Can't move — this task has an active worktree pointing at its current repo.", "moveRepoConfirm": "Different repos — {0} → {1}. Move the task anyway?", "moveConfirmUnavailable": "Can't move — the confirmation dialog isn't available.", "quickClaudeNoWorkingDir": "This list has no working directory configured.", "quickClaudeDirMissing": "Working directory no longer exists: {0}" }, + "tasksIsland": { "completedHeader": "COMPLETED", "completedHeaderCount": "COMPLETED · {0}", "planningOpenFailed": "Couldn't open planning session: {0}", "planningResumeFailed": "Couldn't resume planning session: {0}", "approveFailed": "Approve & merge failed: {0}", "cancelReviewFailed": "Cancel review failed: {0}", "sendToQueueFailed": "Send to queue failed: {0}", "queuePlanBlockedInteractive": "Can't queue the plan — {0} has an open interactive session and must be closed first.", "moveRunningRejected": "Can't move a running task to another list.", "moveWorktreeRejected": "Can't move — this task has an active worktree pointing at its current repo.", "moveRepoConfirm": "Different repos — {0} → {1}. Move the task anyway?", "moveConfirmUnavailable": "Can't move — the confirmation dialog isn't available.", "quickClaudeNoWorkingDir": "This list has no working directory configured.", "quickClaudeDirMissing": "Working directory no longer exists: {0}", "setStatusFailed": "Failed to update status: {0}", "cancelFailed": "Cancel failed: {0}", "rejectToQueueFailed": "Reject to queue failed: {0}", "rejectToIdleFailed": "Reject failed: {0}" }, "diff": { "loadFailed": "Failed to load diff: {0}", "noChanges": "No changes to show.", "unavailable": "Diff no longer available — commit range incomplete." }, "planningDiff": { "hubError": "Could not build combined preview (hub error).", "conflict": "Cannot build combined preview: subtask {0} conflicts with an earlier subtask ({1} files).", "buildFailed": "Could not build combined preview: {0}" }, "merge": { "commitMessage": "chore: merge {0}", "progressMerging": "Merging…", "progressVerifying": "Running the list's verify command… ({0})", "workerOfflineBranches": "Worker offline — cannot list branches.", "loadBranchesFailed": "Failed to load branches: {0}", "merged": "Merged.", "conflict": "Merge conflict — target branch restored. Resolve manually or via Continue, then retry.", "blocked": "Blocked: {0}", "verifyFailed": "Merge landed, but the list's verify command failed — the task was kept out of Done.", "untrackedCollision": "Merge refused — it would overwrite an untracked file in the target working directory.", "unknownStatus": "Unknown status: {0}", "mergeFailed": "Merge failed: {0}" }, @@ -688,8 +691,8 @@ "worktreesTab": { "workerOffline": "Worker offline.", "removed": "Removed {0} worktree(s).", "blocked": "Cannot force-remove: {0} task(s) still running. Cancel them first.", "removedFrom": "Removed {0} worktree(s) from {1} task(s).", "cleanupFailed": "Cleanup failed: {0}", "resetFailed": "Reset failed: {0}" }, "worktreesOverview": { "titleAll": "Worktrees", "titleList": "Worktrees — {0}", "listFallback": "list", "cleanupFailed": "Cleanup failed.", "cleanupFailedDetailed": "Cleanup failed: {0}", "removed": "Removed {0} worktree(s).", "discardFailed": "Failed to discard worktree.", "keepFailed": "Failed to keep worktree.", "cannotForceRunning": "Cannot force-remove a running task.", "forceRemoveFailed": "Force remove failed.", "forceRemoveFailedDetailed": "Force remove failed: {0}", "batchProgress": "Merging {0}/{1}…", "batchDone": "Merged {0}, {1} need resolution." }, "listSettings": { "untitled": "Untitled" }, - "detailsIsland": { "verifyFailed": "Merge landed, but the list's verify command failed — the task was kept out of Done.", "untrackedCollision": "Merge refused — it would overwrite an untracked file in the target working directory." }, - "lists": { "localSuffix": "{0} / local", "smartMyDay": "My Day", "smartImportant": "Important", "smartPlanned": "Planned", "virtualQueue": "Queue", "virtualRunning": "Running", "virtualReview": "Review", "newList": "New list" }, + "detailsIsland": { "verifyFailed": "Merge landed, but the list's verify command failed — the task was kept out of Done.", "untrackedCollision": "Merge refused — it would overwrite an untracked file in the target working directory.", "stopOffline": "Worker offline — can't stop the task.", "stopFailed": "Stop failed: {0}", "enqueueFailed": "Queue failed: {0}", "dequeueFailed": "Remove from queue failed: {0}", "resetAndRetryFailed": "Reset & retry failed: {0}" }, + "lists": { "localSuffix": "{0} / local", "smartMyDay": "My Day", "smartImportant": "Important", "smartPlanned": "Planned", "virtualQueue": "Queue", "virtualRunning": "Running", "virtualReview": "Review", "newList": "New list", "findingsNotFound": "No findings yet for this list.", "findingsOpenFailed": "Couldn't open findings folder: {0}" }, "repoImport": { "loadFailed": "Couldn't load remembered folders: {0}", "saveFailed": "Couldn't save folders: {0}" } }, "ops": { diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index f9f7b68e..1c07e0e4 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -1129,9 +1129,13 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable private async System.Threading.Tasks.Task StopAsync() { if (Task == null || !IsRunning) return; - if (!_worker.IsConnected) return; + if (!_worker.IsConnected) + { + ErrorReported?.Invoke(Loc.T("vm.detailsIsland.stopOffline")); + return; + } try { await _worker.CancelTaskAsync(Task.Id); } - catch { /* offline */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.detailsIsland.stopFailed", ex.Message)); } } [RelayCommand(CanExecute = nameof(CanEnqueue))] @@ -1144,7 +1148,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable AgentState = "queued"; ReportBaseDirty(baseDirty); } - catch { /* offline */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.detailsIsland.enqueueFailed", ex.Message)); } } private void ReportBaseDirty(BaseDirtyWarningDto? warning) @@ -1167,7 +1171,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable await _worker.SetTaskStatusAsync(Task.Id, ClaudeDo.Data.Models.TaskStatus.Idle); AgentState = "idle"; } - catch { /* offline */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.detailsIsland.dequeueFailed", ex.Message)); } } private bool CanDequeue() => @@ -1223,7 +1227,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable AgentState = "queued"; ReportBaseDirty(baseDirty); } - catch { /* offline */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.detailsIsland.resetAndRetryFailed", ex.Message)); } } // Reset & Retry discards the branch/uncommitted work and queues an autonomous run into the diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/ListsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/ListsIslandViewModel.cs index 7b55c19b..37dcb5ee 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/ListsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/ListsIslandViewModel.cs @@ -126,7 +126,11 @@ public sealed partial class ListsIslandViewModel : ViewModelBase, IDisposable var dir = row?.WorkingDir; if (string.IsNullOrWhiteSpace(dir)) return; var findings = System.IO.Path.Combine(dir, ".claudedo"); - if (!System.IO.Directory.Exists(findings)) return; + if (!System.IO.Directory.Exists(findings)) + { + ErrorReported?.Invoke(Loc.T("vm.lists.findingsNotFound")); + return; + } try { System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo @@ -135,7 +139,7 @@ public sealed partial class ListsIslandViewModel : ViewModelBase, IDisposable UseShellExecute = true, }); } - catch { /* best-effort */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.lists.findingsOpenFailed", ex.Message)); } } [RelayCommand] diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs index 1ebdc6b7..7c355ec5 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs @@ -1136,7 +1136,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable var baseDirty = await _worker.SetTaskStatusAsync(row.Id, status); ReportBaseDirty(baseDirty); } - catch { /* offline; broadcast won't fire */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.tasksIsland.setStatusFailed", ex.Message)); } } private void ReportBaseDirty(BaseDirtyWarningDto? warning) @@ -1227,7 +1227,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable { if (row is null || !row.IsRunning || _worker is null) return; try { await _worker.CancelTaskAsync(row.Id); } - catch { /* worker offline; the broadcast will reconcile when it returns */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.tasksIsland.cancelFailed", ex.Message)); } } // ── Review actions (visible when a task is WaitingForReview) ───────────── @@ -1247,7 +1247,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable if (!row.IsWaitingForReview || _worker is null) return; if (string.IsNullOrWhiteSpace(feedback)) return; try { await _worker.RejectReviewToQueueAsync(row.Id, feedback); } - catch { /* offline; broadcast reconciles on return */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.tasksIsland.rejectToQueueFailed", ex.Message)); } } [RelayCommand] @@ -1255,7 +1255,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable { if (row is null || !row.IsWaitingForReview || _worker is null) return; try { await _worker.RejectReviewToIdleAsync(row.Id); } - catch { /* offline; broadcast reconciles on return */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.tasksIsland.rejectToIdleFailed", ex.Message)); } } [RelayCommand] diff --git a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs index 4a7da83a..9621c2d5 100644 --- a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs @@ -119,7 +119,7 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable } OnPropertyChanged(nameof(HasQueued)); } - catch { /* best-effort queue refresh */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("missionControl.queueRefreshFailed", ex.Message)); } } // Drop-to-queue: a task dragged from the main app onto Mission Control gets queued. Goes @@ -130,7 +130,11 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable public async System.Threading.Tasks.Task EnqueueTaskAsync(string taskId) { if (string.IsNullOrEmpty(taskId)) return; - if (ConPtySessions.Any(s => s.TaskId == taskId)) return; + if (ConPtySessions.Any(s => s.TaskId == taskId)) + { + ErrorReported?.Invoke(Loc.T("missionControl.enqueueAlreadyOpen")); + return; + } try { var baseDirty = await _worker.SetTaskStatusAsync(taskId, ClaudeDo.Data.Models.TaskStatus.Queued); @@ -138,7 +142,7 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable ErrorReported?.Invoke(Loc.T( "vm.queue.baseDirtyWarning", baseDirty.ModifiedCount, baseDirty.UntrackedCount)); } - catch { /* best-effort enqueue */ } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("missionControl.enqueueFailed", ex.Message)); } await RefreshQueueAsync(); } diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandErrorFeedbackTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandErrorFeedbackTests.cs new file mode 100644 index 00000000..99fb3bdc --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandErrorFeedbackTests.cs @@ -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() + .UseSqlite($"Data Source={_dbPath}") + .Options; + return new ClaudeDoDbContext(opts); + } + + private sealed class TestDbFactory : IDbContextFactory + { + private readonly Func _create; + public TestDbFactory(Func 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> ListAsync(DateOnly day) => + Task.FromResult(new List()); + public Task AddAsync(DateOnly day, string text) => + Task.FromResult(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 SetTaskStatusAsync(string taskId, TaskStatus status) + { + if (ThrowOnSetTaskStatus is not null) throw ThrowOnSetTaskStatus; + return Task.FromResult(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); + } +} diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/ListsIslandErrorFeedbackTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/ListsIslandErrorFeedbackTests.cs new file mode 100644 index 00000000..95697b36 --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/ListsIslandErrorFeedbackTests.cs @@ -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() + .UseSqlite($"Data Source={_dbPath}") + .Options; + return new ClaudeDoDbContext(opts); + } + + private sealed class TestDbFactory : IDbContextFactory + { + private readonly Func _create; + public TestDbFactory(Func 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 { } + } + } +} diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs index fccf5ae8..16c54eb2 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs @@ -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 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 GetInteractiveLaunchSpecAsync(string taskId, CancellationToken ct = default) diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandErrorFeedbackTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandErrorFeedbackTests.cs new file mode 100644 index 00000000..3e740b77 --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandErrorFeedbackTests.cs @@ -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() + .UseSqlite($"Data Source={_dbPath}") + .Options; + return new ClaudeDoDbContext(opts); + } + + private sealed class TestDbFactory : IDbContextFactory + { + private readonly Func _create; + public TestDbFactory(Func 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 SetTaskStatusAsync(string taskId, TaskStatus status) + { + if (ThrowOnSetTaskStatus is not null) throw ThrowOnSetTaskStatus; + return Task.FromResult(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); + } +}