From 503a5a9e1ee760ee3b650bc7826516f7c8a26a11 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 6 Jul 2026 17:50:49 +0200 Subject: [PATCH] Fix desktop-managed daemon restart from settings (#1915) * fix(settings): restart managed desktop daemon fully * fix(settings): respect paused desktop daemon management * test(settings): cover restart routing through public helper * fix(settings): wait for daemon restart disconnect * fix(settings): handle restart review edge cases --- .../screens/settings/daemon-restart.test.ts | 131 ++++++++++++++++++ .../src/screens/settings/daemon-restart.ts | 57 ++++++++ .../app/src/screens/settings/host-page.tsx | 97 +++++++++---- 3 files changed, 256 insertions(+), 29 deletions(-) create mode 100644 packages/app/src/screens/settings/daemon-restart.test.ts create mode 100644 packages/app/src/screens/settings/daemon-restart.ts diff --git a/packages/app/src/screens/settings/daemon-restart.test.ts b/packages/app/src/screens/settings/daemon-restart.test.ts new file mode 100644 index 000000000..51b6c4d12 --- /dev/null +++ b/packages/app/src/screens/settings/daemon-restart.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from "vitest"; +import type { DesktopDaemonStatus } from "@/desktop/daemon/desktop-daemon"; +import { restartDaemonFromSettings, type SettingsDaemonRestartDeps } from "./daemon-restart"; + +const runningDesktopDaemonStatus: DesktopDaemonStatus = { + serverId: "local-desktop", + status: "running", + listen: null, + hostname: null, + pid: 123, + home: "/tmp/paseo", + version: "1.0.0", + desktopManaged: true, + error: null, +}; + +const desktopSettings = { + daemon: { + manageBuiltInDaemon: true, + }, +}; + +function makeDeps(overrides?: { + isElectron?: boolean; + desktopDaemonStatus?: DesktopDaemonStatus; + desktopSettings?: typeof desktopSettings; + desktopSettingsError?: Error; + restartDesktopDaemon?: () => Promise; + restartServer?: (reason: string) => Promise; +}) { + const calls: string[] = []; + const deps: SettingsDaemonRestartDeps = { + getIsElectron: () => overrides?.isElectron ?? true, + getDesktopDaemonStatus: async () => { + calls.push("desktop-status"); + return overrides?.desktopDaemonStatus ?? runningDesktopDaemonStatus; + }, + getDesktopSettings: async () => { + calls.push("desktop-settings"); + if (overrides?.desktopSettingsError) { + throw overrides.desktopSettingsError; + } + return overrides?.desktopSettings ?? desktopSettings; + }, + restartDesktopDaemon: + overrides?.restartDesktopDaemon ?? + (async () => { + calls.push("desktop-restart"); + return runningDesktopDaemonStatus; + }), + restartServer: + overrides?.restartServer ?? + (async (reason) => { + calls.push(`rpc-restart:${reason}`); + }), + }; + return { calls, deps }; +} + +describe("restartDaemonFromSettings", () => { + it("restarts the local desktop-managed daemon through the desktop bridge", async () => { + const { calls, deps } = makeDeps(); + + await restartDaemonFromSettings(" local-desktop ", "settings_daemon_restart_local", deps); + + expect(calls).toEqual(["desktop-status", "desktop-settings", "desktop-restart"]); + }); + + it("restarts remote hosts over the daemon RPC without reading desktop settings", async () => { + const { calls, deps } = makeDeps({ + desktopSettingsError: new Error("Unreadable desktop settings."), + }); + + await restartDaemonFromSettings("remote-host", "settings_daemon_restart_remote", deps); + + expect(calls).toEqual(["desktop-status", "rpc-restart:settings_daemon_restart_remote"]); + }); + + it("keeps manually managed local daemons on the RPC path without reading desktop settings", async () => { + const { calls, deps } = makeDeps({ + desktopDaemonStatus: { ...runningDesktopDaemonStatus, desktopManaged: false }, + desktopSettingsError: new Error("Unreadable desktop settings."), + }); + + await restartDaemonFromSettings("local-desktop", "settings_daemon_restart_local", deps); + + expect(calls).toEqual(["desktop-status", "rpc-restart:settings_daemon_restart_local"]); + }); + + it("keeps the RPC path when built-in daemon management is disabled", async () => { + const { calls, deps } = makeDeps({ + desktopSettings: { + ...desktopSettings, + daemon: { ...desktopSettings.daemon, manageBuiltInDaemon: false }, + }, + }); + + await restartDaemonFromSettings("local-desktop", "settings_daemon_restart_local", deps); + + expect(calls).toEqual([ + "desktop-status", + "desktop-settings", + "rpc-restart:settings_daemon_restart_local", + ]); + }); + + it("uses the RPC path outside Electron without reading desktop daemon status or settings", async () => { + const { calls, deps } = makeDeps({ + isElectron: false, + }); + + await restartDaemonFromSettings("local-desktop", "settings_daemon_restart_local", deps); + + expect(calls).toEqual(["rpc-restart:settings_daemon_restart_local"]); + }); + + it("surfaces desktop restart failures without falling back to worker recycle", async () => { + const { calls, deps } = makeDeps({ + restartDesktopDaemon: async () => { + calls.push("desktop-restart"); + throw new Error("Desktop restart failed."); + }, + }); + + await expect( + restartDaemonFromSettings("local-desktop", "settings_daemon_restart_local", deps), + ).rejects.toThrow("Desktop restart failed."); + + expect(calls).toEqual(["desktop-status", "desktop-settings", "desktop-restart"]); + }); +}); diff --git a/packages/app/src/screens/settings/daemon-restart.ts b/packages/app/src/screens/settings/daemon-restart.ts new file mode 100644 index 000000000..61cce091e --- /dev/null +++ b/packages/app/src/screens/settings/daemon-restart.ts @@ -0,0 +1,57 @@ +import type { DesktopDaemonStatus } from "@/desktop/daemon/desktop-daemon"; + +interface DesktopDaemonRestartStatus { + desktopManaged: boolean; + serverId: string; +} + +interface DesktopDaemonRestartSettings { + daemon: { + manageBuiltInDaemon: boolean; + }; +} + +export interface SettingsDaemonRestartDeps { + getIsElectron: () => boolean; + getDesktopDaemonStatus: () => Promise; + getDesktopSettings: () => Promise; + restartDesktopDaemon: () => Promise; + restartServer: (reason: string) => Promise; +} + +async function isLocalDesktopManagedDaemon( + hostServerId: string, + deps: SettingsDaemonRestartDeps, +): Promise { + if (!deps.getIsElectron()) { + return false; + } + + const desktopDaemonStatus = await deps.getDesktopDaemonStatus(); + if (!desktopDaemonStatus.desktopManaged) { + return false; + } + + const normalizedHostServerId = hostServerId.trim(); + const normalizedDesktopServerId = desktopDaemonStatus.serverId.trim(); + + if (normalizedHostServerId.length === 0 || normalizedHostServerId !== normalizedDesktopServerId) { + return false; + } + + const desktopSettings = await deps.getDesktopSettings(); + return desktopSettings.daemon.manageBuiltInDaemon; +} + +export async function restartDaemonFromSettings( + hostServerId: string, + reason: string, + deps: SettingsDaemonRestartDeps, +): Promise { + if (await isLocalDesktopManagedDaemon(hostServerId, deps)) { + await deps.restartDesktopDaemon(); + return; + } + + await deps.restartServer(reason); +} diff --git a/packages/app/src/screens/settings/host-page.tsx b/packages/app/src/screens/settings/host-page.tsx index ca4ddc7a3..9fdb7fbcd 100644 --- a/packages/app/src/screens/settings/host-page.tsx +++ b/packages/app/src/screens/settings/host-page.tsx @@ -30,10 +30,16 @@ import { ProfileDraft, TerminalProfileEditModal, } from "@/screens/settings/terminal-profile-edit-modal"; -import { startDesktopDaemon, stopDesktopDaemon } from "@/desktop/daemon/desktop-daemon"; +import { getIsElectron } from "@/constants/platform"; +import { + getDesktopDaemonStatus, + restartDesktopDaemon, + startDesktopDaemon, + stopDesktopDaemon, +} from "@/desktop/daemon/desktop-daemon"; import { LocalDaemonSection } from "@/desktop/components/desktop-updates-section"; import { useDaemonStatus } from "@/desktop/hooks/use-daemon-status"; -import { useDesktopSettings } from "@/desktop/settings/desktop-settings"; +import { loadDesktopSettings, useDesktopSettings } from "@/desktop/settings/desktop-settings"; import { PairDeviceModal } from "@/desktop/components/pair-device-modal"; import { useDaemonConfig } from "@/hooks/use-daemon-config"; import { useIsLocalDaemon } from "@/hooks/use-is-local-daemon"; @@ -62,6 +68,7 @@ import { ICON_SIZE } from "@/styles/theme"; import type { Theme } from "@/styles/theme"; import { getProviderIcon } from "@/components/provider-icons"; import { BrowserToolsOptInCard } from "./browser-tools-card"; +import { restartDaemonFromSettings } from "./daemon-restart"; const ThemedArrowUp = withUnistyles(ArrowUp); const ThemedArrowDown = withUnistyles(ArrowDown); @@ -620,23 +627,56 @@ function RestartDaemonCard({ host }: { host: HostProfile }) { [], ); - const waitForDaemonRestart = useCallback(async () => { - const disconnectTimeoutMs = 7000; - const reconnectTimeoutMs = 30000; - if (isHostConnected()) { - await waitForCondition(() => !isHostConnected(), disconnectTimeoutMs); - } - const reconnected = await waitForCondition(() => isHostConnected(), reconnectTimeoutMs); - if (isMountedRef.current) { - setIsRestarting(false); - if (!reconnected) { + const waitForDaemonRestart = useCallback( + async (restartRequest: Promise) => { + const disconnectTimeoutMs = 30000; + const reconnectTimeoutMs = 30000; + const requestFailureDisconnectGraceMs = 2000; + const disconnectedPromise = isHostConnected() + ? waitForCondition(() => !isHostConnected(), disconnectTimeoutMs) + : Promise.resolve(true); + const restartResult = await restartRequest.then( + () => ({ status: "accepted" as const }), + async (error) => ({ + status: "rejected" as const, + error, + disconnectedAfterFailure: await waitForCondition( + () => !isHostConnected(), + requestFailureDisconnectGraceMs, + 100, + ), + }), + ); + if (!isMountedRef.current) return; + + if (restartResult.status === "rejected" && !restartResult.disconnectedAfterFailure) { + console.error(`[HostPage] Failed to restart daemon ${host.label}`, restartResult.error); + setIsRestarting(false); Alert.alert( - t("settings.host.daemon.restart.unableToReconnectTitle"), - t("settings.host.daemon.restart.unableToReconnectMessage", { name: host.label }), + t("settings.host.daemon.restart.requestFailedTitle"), + t("settings.host.daemon.restart.requestFailedMessage"), ); + return; } - } - }, [host.label, isHostConnected, t, waitForCondition]); + + const disconnected = + restartResult.status === "rejected" + ? restartResult.disconnectedAfterFailure + : await disconnectedPromise; + const reconnected = + disconnected && (await waitForCondition(() => isHostConnected(), reconnectTimeoutMs)); + if (isMountedRef.current) { + setIsRestarting(false); + if (!reconnected) { + Alert.alert( + t("settings.host.daemon.restart.unableToReconnectTitle"), + t("settings.host.daemon.restart.unableToReconnectMessage", { name: host.label }), + ); + } + } + }, + [host.label, isHostConnected, t, waitForCondition], + ); const handleRestart = useCallback(() => { if (!daemonClient) { @@ -664,18 +704,18 @@ function RestartDaemonCard({ host }: { host: HostProfile }) { .then((confirmed) => { if (!confirmed) return; setIsRestarting(true); - void daemonClient - .restartServer(`settings_daemon_restart_${host.serverId}`) - .catch((error) => { - console.error(`[HostPage] Failed to restart daemon ${host.label}`, error); - if (!isMountedRef.current) return; - setIsRestarting(false); - Alert.alert( - t("settings.host.daemon.restart.requestFailedTitle"), - t("settings.host.daemon.restart.requestFailedMessage"), - ); - }); - void waitForDaemonRestart(); + const restartRequest = restartDaemonFromSettings( + host.serverId, + `settings_daemon_restart_${host.serverId}`, + { + getIsElectron, + getDesktopDaemonStatus, + getDesktopSettings: loadDesktopSettings, + restartDesktopDaemon, + restartServer: (reason) => daemonClient.restartServer(reason), + }, + ); + void waitForDaemonRestart(restartRequest); return; }) .catch((error) => { @@ -715,7 +755,6 @@ function RestartDaemonCard({ host }: { host: HostProfile }) { ); } - function UpdateDaemonCard({ host }: { host: HostProfile }) { const { t } = useTranslation(); const { theme } = useUnistyles();