From b4b6225432434a16f2615b31cf5177b90f40dc9c Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 10 Sep 2026 13:21:58 +0200 Subject: [PATCH] fix(ui): surface worker connect/restart failures instead of no-opting WorkerConnectionModalViewModel's StartWorker/RerunInstaller swallowed missing-exe and Process.Start failures behind an empty catch and closed the dialog anyway, looking like success. Now both show an inline error and keep the dialog open; only a real launch closes it. IslandsShellViewModel.RestartWorkerAsync never checked whether the relaunched worker actually stayed up, and its status property was never bound in any view. RestartWorkerService now fails if the new process exits within a short survival window, and the outcome routes through the footer error/success strip instead of the dead RestartWorkerStatus. InstallArtifactLocator.Find() is now virtual and its two subclasses unsealed so tests can fake exe resolution without touching the real filesystem/registry. --- src/ClaudeDo.Localization/locales/de.json | 7 +- src/ClaudeDo.Localization/locales/en.json | 7 +- .../Services/InstallArtifactLocator.cs | 9 +- .../ViewModels/IslandsShellViewModel.cs | 25 +++--- .../Modals/WorkerConnectionModalViewModel.cs | 41 +++++++-- .../Modals/WorkerConnectionModalView.axaml | 8 +- ...IslandsShellViewModelRestartWorkerTests.cs | 41 +++++++++ .../WorkerConnectionModalViewModelTests.cs | 85 +++++++++++++++++++ 8 files changed, 194 insertions(+), 29 deletions(-) create mode 100644 tests/ClaudeDo.Ui.Tests/IslandsShellViewModelRestartWorkerTests.cs create mode 100644 tests/ClaudeDo.Ui.Tests/ViewModels/WorkerConnectionModalViewModelTests.cs diff --git a/src/ClaudeDo.Localization/locales/de.json b/src/ClaudeDo.Localization/locales/de.json index d40850a9..4d8d14eb 100644 --- a/src/ClaudeDo.Localization/locales/de.json +++ b/src/ClaudeDo.Localization/locales/de.json @@ -407,7 +407,10 @@ "body": "ClaudeDo kann den Hintergrund-Worker nicht erreichen. Normalerweise wird er bei der Anmeldung automatisch gestartet. Du kannst ihn jetzt starten oder neu installieren, falls das Problem bestehen bleibt.", "dismiss": "Ausblenden", "rerunInstaller": "Installer erneut ausführen", - "startWorker": "Worker starten" + "startWorker": "Worker starten", + "workerNotFound": "Worker-Programm nicht gefunden. Installation prüfen.", + "installerNotFound": "Installer nicht gefunden. Installation prüfen.", + "startFailed": "Start fehlgeschlagen: {0}" }, "listSettings": { "title": "LISTENEINSTELLUNGEN", @@ -777,7 +780,7 @@ }, "vm": { "connection": { "online": "Online", "connecting": "Verbinden…", "offline": "Offline" }, - "shell": { "restartingWorker": "Worker wird neu gestartet…", "unexpectedError": "Unerwarteter Fehler: {0}" }, + "shell": { "unexpectedError": "Unerwarteter Fehler: {0}" }, "mainWindow": { "closeConfirm": { "message": "{0} Mission-Control-Sitzung(en) laufen noch. Beim Beenden werden sie abgebrochen. Trotzdem schließen?" } }, "agentStatus": { "idle": "Leerlauf", "queued": "In Warteschlange", "running": "Läuft", "review": "Prüfung", "children": "Wartet auf Teilaufgaben", "done": "Fertig", "failed": "Fehlgeschlagen", "cancelled": "Abgebrochen" }, "taskStatus": { "idle": "Leerlauf", "queued": "In Warteschlange", "running": "Läuft", "waitingForReview": "Wartet auf Prüfung", "waitingForChildren": "Wartet auf Teilaufgaben", "done": "Fertig", "failed": "Fehlgeschlagen", "cancelled": "Abgebrochen", "parked": "Geparkt", "interactive": "Interaktiv", "waiting": "Wartet" }, diff --git a/src/ClaudeDo.Localization/locales/en.json b/src/ClaudeDo.Localization/locales/en.json index 3c27cca8..f02214d4 100644 --- a/src/ClaudeDo.Localization/locales/en.json +++ b/src/ClaudeDo.Localization/locales/en.json @@ -407,7 +407,10 @@ "body": "ClaudeDo can't reach the background worker. It is normally started automatically at logon. You can start it now, or reinstall if the problem persists.", "dismiss": "Dismiss", "rerunInstaller": "Rerun Installer", - "startWorker": "Start Worker" + "startWorker": "Start Worker", + "workerNotFound": "Worker executable not found. Check the installation.", + "installerNotFound": "Installer not found. Check the installation.", + "startFailed": "Start failed: {0}" }, "listSettings": { "title": "LIST SETTINGS", @@ -777,7 +780,7 @@ }, "vm": { "connection": { "online": "Online", "connecting": "Connecting…", "offline": "Offline" }, - "shell": { "restartingWorker": "Restarting worker…", "unexpectedError": "Unexpected error: {0}" }, + "shell": { "unexpectedError": "Unexpected error: {0}" }, "mainWindow": { "closeConfirm": { "message": "{0} Mission Control session(s) are still running. Closing the app will end them. Close anyway?" } }, "agentStatus": { "idle": "Idle", "queued": "Queued", "running": "Running", "review": "Review", "children": "Waiting for Subtasks", "done": "Done", "failed": "Failed", "cancelled": "Cancelled" }, "taskStatus": { "idle": "Idle", "queued": "Queued", "running": "Running", "waitingForReview": "Waiting for Review", "waitingForChildren": "Waiting for Subtasks", "done": "Done", "failed": "Failed", "cancelled": "Cancelled", "parked": "Parked", "interactive": "Interactive", "waiting": "Waiting" }, diff --git a/src/ClaudeDo.Ui/Services/InstallArtifactLocator.cs b/src/ClaudeDo.Ui/Services/InstallArtifactLocator.cs index 73d22a76..6a744001 100644 --- a/src/ClaudeDo.Ui/Services/InstallArtifactLocator.cs +++ b/src/ClaudeDo.Ui/Services/InstallArtifactLocator.cs @@ -1,9 +1,10 @@ namespace ClaudeDo.Ui.Services; -// Two named types because DI resolves them by type, not by key. -public sealed class InstallerLocator() : InstallArtifactLocator("uninstaller", "ClaudeDo.Installer.exe"); +// Two named types because DI resolves them by type, not by key. Not sealed: tests subclass +// these to fake Find() without touching the real filesystem/registry. +public class InstallerLocator() : InstallArtifactLocator("uninstaller", "ClaudeDo.Installer.exe"); -public sealed class WorkerLocator() : InstallArtifactLocator("worker", "ClaudeDo.Worker.exe"); +public class WorkerLocator() : InstallArtifactLocator("worker", "ClaudeDo.Worker.exe"); /// /// Locates an executable inside a ClaudeDo install: walk up from the running @@ -14,7 +15,7 @@ public abstract class InstallArtifactLocator(string subdir, string exeName) { private const string InstallJson = "install.json"; - public string? Find() + public virtual string? Find() => FindByWalkingUp(AppContext.BaseDirectory) ?? (OperatingSystem.IsWindows() ? FindByRegistry() : null); diff --git a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs index 864e2a33..1ead9b8e 100644 --- a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs @@ -631,20 +631,25 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable await _updateCheck.CheckNowAsync(CancellationToken.None); } - [ObservableProperty] private string? _restartWorkerStatus; + // How long to wait after launching the new worker process before deciding it survived + // (e.g. a port conflict kills it almost immediately — see #292 for the worker-side fix). + private const int RestartSurvivalCheckMs = 2000; + + // Test seam: overrides the real kill-and-relaunch body, so tests can assert the + // success/failure footer routing without spawning a real worker process. + internal Func? RestartWorkerOperation { get; set; } [RelayCommand] private async Task RestartWorkerAsync() { - RestartWorkerStatus = Loc.T("vm.shell.restartingWorker"); try { - await Task.Run(RestartWorkerService); - await FlashRestartStatusAsync("Worker restarted."); + await (RestartWorkerOperation ?? (() => Task.Run(RestartWorkerService)))(); + FlashFooterSuccess("Worker restarted."); } catch (Exception ex) { - await FlashRestartStatusAsync($"Restart failed: {ex.Message}"); + FlashFooterError($"Restart failed: {ex.Message} — check the worker log."); } } @@ -670,14 +675,10 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable catch { /* may have exited or be inaccessible */ } finally { p.Dispose(); } } - System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo(exe) { UseShellExecute = true }); - } - private async Task FlashRestartStatusAsync(string text) - { - RestartWorkerStatus = text; - await Task.Delay(3000); - if (RestartWorkerStatus == text) RestartWorkerStatus = null; + var started = System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo(exe) { UseShellExecute = true }); + if (started is not null && started.WaitForExit(RestartSurvivalCheckMs)) + throw new InvalidOperationException($"worker process exited immediately (code {started.ExitCode})"); } private bool _updatingClaudeCli; diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/WorkerConnectionModalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/WorkerConnectionModalViewModel.cs index 48e4b5f6..8dc9f545 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/WorkerConnectionModalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/WorkerConnectionModalViewModel.cs @@ -1,7 +1,9 @@ using System; using System.Diagnostics; using System.IO; +using ClaudeDo.Ui.Localization; using ClaudeDo.Ui.Services; +using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; namespace ClaudeDo.Ui.ViewModels.Modals; @@ -11,6 +13,9 @@ public sealed partial class WorkerConnectionModalViewModel : ViewModelBase private readonly WorkerLocator _workerLocator; private readonly InstallerLocator _installerLocator; + // Test seam: real Process.Start by default, swappable to simulate a blocked/failed launch. + internal Action ProcessStarter { get; set; } = psi => Process.Start(psi); + public WorkerConnectionModalViewModel(WorkerLocator workerLocator, InstallerLocator installerLocator) { _workerLocator = workerLocator; @@ -19,15 +24,29 @@ public sealed partial class WorkerConnectionModalViewModel : ViewModelBase public Action? CloseAction { get; set; } + [ObservableProperty] private string? _errorText; + [RelayCommand] private void Close() => CloseAction?.Invoke(); [RelayCommand] private void StartWorker() { var exe = _workerLocator.Find(); - if (exe is null) return; - try { Process.Start(new ProcessStartInfo(exe) { UseShellExecute = true }); } - catch { /* nothing useful to show */ } + if (exe is null) + { + ErrorText = Loc.T("modals.workerConnection.workerNotFound"); + return; + } + try + { + ProcessStarter(new ProcessStartInfo(exe) { UseShellExecute = true }); + } + catch (Exception ex) + { + ErrorText = Loc.T("modals.workerConnection.startFailed", ex.Message); + return; + } + ErrorText = null; CloseAction?.Invoke(); } @@ -35,18 +54,26 @@ public sealed partial class WorkerConnectionModalViewModel : ViewModelBase private void RerunInstaller() { var path = _installerLocator.Find(); - if (path is null) return; + if (path is null) + { + ErrorText = Loc.T("modals.workerConnection.installerNotFound"); + return; + } try { // See IslandsShellViewModel.UpdateNow: an inherited CWD inside the install dir // makes the installer block its own app\ rename. - Process.Start(new ProcessStartInfo(path) + ProcessStarter(new ProcessStartInfo(path) { UseShellExecute = true, WorkingDirectory = Path.GetTempPath(), }); - Environment.Exit(0); } - catch { /* nothing useful to show */ } + catch (Exception ex) + { + ErrorText = Loc.T("modals.workerConnection.startFailed", ex.Message); + return; + } + Environment.Exit(0); } } diff --git a/src/ClaudeDo.Ui/Views/Modals/WorkerConnectionModalView.axaml b/src/ClaudeDo.Ui/Views/Modals/WorkerConnectionModalView.axaml index c2cc24e2..c62fe4dc 100644 --- a/src/ClaudeDo.Ui/Views/Modals/WorkerConnectionModalView.axaml +++ b/src/ClaudeDo.Ui/Views/Modals/WorkerConnectionModalView.axaml @@ -16,10 +16,14 @@ - + - +