From b7310b9e90f4cf23fbe4f6648e4b64448fd6c4c8 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 26 Apr 2026 08:14:55 +0700 Subject: [PATCH] feat(desktop): simplify quit lifecycle with sync PID check and shutdown dialog Replace the async daemon status resolution during quit with a synchronous PID file check, avoiding races when the event loop is shutting down. Show a blocking dialog so users know the daemon is stopping. Add --kill-timeout to the CLI stop command for the desktop app to control SIGKILL wait time. --- packages/cli/src/commands/daemon/index.ts | 1 + .../cli/src/commands/daemon/local-daemon.ts | 6 +- packages/cli/src/commands/daemon/stop.ts | 23 +++-- packages/desktop/src/daemon/daemon-manager.ts | 25 ++++++ .../desktop/src/daemon/quit-lifecycle.test.ts | 85 ++++++++++++------- packages/desktop/src/daemon/quit-lifecycle.ts | 49 +++++------ packages/desktop/src/main.ts | 22 ++++- 7 files changed, 137 insertions(+), 74 deletions(-) diff --git a/packages/cli/src/commands/daemon/index.ts b/packages/cli/src/commands/daemon/index.ts index 79bcb59d6..74c0dffeb 100644 --- a/packages/cli/src/commands/daemon/index.ts +++ b/packages/cli/src/commands/daemon/index.ts @@ -27,6 +27,7 @@ export function createDaemonCommand(): Command { .option("--home ", "Paseo home directory (default: ~/.paseo)") .option("--timeout ", "Wait timeout before failing (default: 15)") .option("--force", "Send SIGKILL if graceful stop times out") + .option("--kill-timeout ", "Wait after SIGKILL before failing (default: 3)") .action(withOutput(runStopCommand)); addJsonOption(daemon.command("restart").description("Restart the local daemon")) diff --git a/packages/cli/src/commands/daemon/local-daemon.ts b/packages/cli/src/commands/daemon/local-daemon.ts index fc0a2410b..3174ceee8 100644 --- a/packages/cli/src/commands/daemon/local-daemon.ts +++ b/packages/cli/src/commands/daemon/local-daemon.ts @@ -43,6 +43,7 @@ export interface DetachedStartResult { export interface StopLocalDaemonOptions { home?: string; timeoutMs?: number; + killTimeoutMs?: number; force?: boolean; } @@ -64,11 +65,11 @@ type DetachedStartupResult = { exitedEarly: false } | ({ exitedEarly: true } & P const DETACHED_STARTUP_GRACE_MS = 1200; const PID_POLL_INTERVAL_MS = 100; -const KILL_TIMEOUT_MS = 3000; const DAEMON_LOG_FILENAME = "daemon.log"; const DAEMON_PID_FILENAME = "paseo.pid"; export const DEFAULT_STOP_TIMEOUT_MS = 15_000; +export const DEFAULT_KILL_TIMEOUT_MS = 3_000; const require = createRequire(import.meta.url); @@ -494,6 +495,7 @@ export async function stopLocalDaemon( options: StopLocalDaemonOptions = {}, ): Promise { const timeoutMs = options.timeoutMs ?? DEFAULT_STOP_TIMEOUT_MS; + const killTimeoutMs = options.killTimeoutMs ?? DEFAULT_KILL_TIMEOUT_MS; const state = resolveLocalDaemonState({ home: options.home }); if (!state.pidInfo || !state.running) { @@ -530,7 +532,7 @@ export async function stopLocalDaemon( if (!stopped && options.force) { forced = true; signalProcessGroupSafely(pid, "SIGKILL"); - stopped = await waitForPidExit(pid, KILL_TIMEOUT_MS); + stopped = await waitForPidExit(pid, killTimeoutMs); } if (!stopped) { diff --git a/packages/cli/src/commands/daemon/stop.ts b/packages/cli/src/commands/daemon/stop.ts index 2818a7662..c08cbea50 100644 --- a/packages/cli/src/commands/daemon/stop.ts +++ b/packages/cli/src/commands/daemon/stop.ts @@ -1,5 +1,9 @@ import type { Command } from "commander"; -import { stopLocalDaemon, DEFAULT_STOP_TIMEOUT_MS } from "./local-daemon.js"; +import { + stopLocalDaemon, + DEFAULT_STOP_TIMEOUT_MS, + DEFAULT_KILL_TIMEOUT_MS, +} from "./local-daemon.js"; import type { CommandOptions, SingleResult, @@ -30,17 +34,17 @@ const stopResultSchema: OutputSchema = { export type StopCommandResult = SingleResult; -function parseTimeoutMs(raw: unknown): number { +function parseSecondsOption(raw: unknown, fallbackMs: number, label: string): number { if (typeof raw !== "string" || raw.trim().length === 0) { - return DEFAULT_STOP_TIMEOUT_MS; + return fallbackMs; } const seconds = Number(raw); if (!Number.isFinite(seconds) || seconds <= 0) { const error: CommandError = { code: "INVALID_TIMEOUT", - message: `Invalid timeout value: ${raw}`, - details: "Timeout must be a positive number of seconds", + message: `Invalid ${label} value: ${raw}`, + details: `${label} must be a positive number of seconds`, }; throw error; } @@ -54,10 +58,15 @@ export async function runStopCommand( ): Promise { const home = typeof options.home === "string" ? options.home : undefined; const force = options.force === true; - const timeoutMs = parseTimeoutMs(options.timeout); + const timeoutMs = parseSecondsOption(options.timeout, DEFAULT_STOP_TIMEOUT_MS, "timeout"); + const killTimeoutMs = parseSecondsOption( + options.killTimeout, + DEFAULT_KILL_TIMEOUT_MS, + "kill-timeout", + ); try { - const result = await stopLocalDaemon({ home, force, timeoutMs }); + const result = await stopLocalDaemon({ home, force, timeoutMs, killTimeoutMs }); return { type: "single", data: { diff --git a/packages/desktop/src/daemon/daemon-manager.ts b/packages/desktop/src/daemon/daemon-manager.ts index 1e9c10748..c4da31f85 100644 --- a/packages/desktop/src/daemon/daemon-manager.ts +++ b/packages/desktop/src/daemon/daemon-manager.ts @@ -96,6 +96,31 @@ function logFilePath(): string { return path.join(getPaseoHome(), DAEMON_LOG_FILENAME); } +export function isDesktopManagedDaemonRunningSync(): boolean { + try { + const raw = readFileSync(path.join(getPaseoHome(), "paseo.pid"), "utf-8"); + const lock = JSON.parse(raw) as { pid?: unknown; desktopManaged?: unknown }; + if (lock.desktopManaged !== true) return false; + if (typeof lock.pid !== "number" || !Number.isInteger(lock.pid)) return false; + return isProcessRunning(lock.pid); + } catch { + return false; + } +} + +export async function stopDesktopDaemonViaCli(): Promise { + await runCliJsonCommand([ + "daemon", + "stop", + "--json", + "--timeout", + "5", + "--force", + "--kill-timeout", + "5", + ]); +} + function isProcessRunning(pid: number): boolean { try { process.kill(pid, 0); diff --git a/packages/desktop/src/daemon/quit-lifecycle.test.ts b/packages/desktop/src/daemon/quit-lifecycle.test.ts index 6254b2ae9..a0d856aa3 100644 --- a/packages/desktop/src/daemon/quit-lifecycle.test.ts +++ b/packages/desktop/src/daemon/quit-lifecycle.test.ts @@ -7,49 +7,72 @@ import { stopDesktopManagedDaemonOnQuitIfNeeded, } from "./quit-lifecycle"; +const SETTINGS_KEEP_RUNNING = DEFAULT_DESKTOP_SETTINGS; +const SETTINGS_STOP_ON_QUIT = { + ...DEFAULT_DESKTOP_SETTINGS, + daemon: { + ...DEFAULT_DESKTOP_SETTINGS.daemon, + keepRunningAfterQuit: false, + }, +}; + describe("quit-lifecycle", () => { - it("stops only when quit persistence disables keep-running and the daemon is desktop-managed", () => { - expect( - shouldStopDesktopManagedDaemonOnQuit({ - settings: { - ...DEFAULT_DESKTOP_SETTINGS, - daemon: { - ...DEFAULT_DESKTOP_SETTINGS.daemon, - keepRunningAfterQuit: false, - }, - }, - daemonStatus: { - status: "running", - desktopManaged: true, - }, - }), - ).toBe(true); + it("only stops when keepRunningAfterQuit is explicitly disabled", () => { + expect(shouldStopDesktopManagedDaemonOnQuit(SETTINGS_STOP_ON_QUIT)).toBe(true); + expect(shouldStopDesktopManagedDaemonOnQuit(SETTINGS_KEEP_RUNNING)).toBe(false); + }); + + it("short-circuits without inspecting the daemon when keep-running is on", async () => { + const isDesktopManagedDaemonRunning = vi.fn(() => true); + const stopDaemon = vi.fn(async () => undefined); + const showShutdownFeedback = vi.fn(); + + const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({ + settingsStore: { get: async () => SETTINGS_KEEP_RUNNING }, + isDesktopManagedDaemonRunning, + stopDaemon, + showShutdownFeedback, + }); + + expect(stopped).toBe(false); + expect(isDesktopManagedDaemonRunning).not.toHaveBeenCalled(); + expect(stopDaemon).not.toHaveBeenCalled(); + expect(showShutdownFeedback).not.toHaveBeenCalled(); }); it("does not stop a manually started daemon on quit", async () => { const stopDaemon = vi.fn(async () => undefined); + const showShutdownFeedback = vi.fn(); const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({ - settingsStore: { - get: async () => ({ - ...DEFAULT_DESKTOP_SETTINGS, - daemon: { - ...DEFAULT_DESKTOP_SETTINGS.daemon, - keepRunningAfterQuit: false, - }, - }), - patch: async () => DEFAULT_DESKTOP_SETTINGS, - migrateLegacyRendererSettings: async () => DEFAULT_DESKTOP_SETTINGS, - }, - resolveStatus: async () => ({ - status: "running", - desktopManaged: false, - }), + settingsStore: { get: async () => SETTINGS_STOP_ON_QUIT }, + isDesktopManagedDaemonRunning: () => false, stopDaemon, + showShutdownFeedback, }); expect(stopped).toBe(false); expect(stopDaemon).not.toHaveBeenCalled(); + expect(showShutdownFeedback).not.toHaveBeenCalled(); + }); + + it("shows feedback then stops a desktop-managed daemon", async () => { + const stopDaemon = vi.fn(async () => undefined); + const showShutdownFeedback = vi.fn(); + + const stopped = await stopDesktopManagedDaemonOnQuitIfNeeded({ + settingsStore: { get: async () => SETTINGS_STOP_ON_QUIT }, + isDesktopManagedDaemonRunning: () => true, + stopDaemon, + showShutdownFeedback, + }); + + expect(stopped).toBe(true); + expect(showShutdownFeedback).toHaveBeenCalledTimes(1); + expect(stopDaemon).toHaveBeenCalledTimes(1); + expect(showShutdownFeedback.mock.invocationCallOrder[0]).toBeLessThan( + stopDaemon.mock.invocationCallOrder[0], + ); }); it("gates quit until the stop decision completes, then lets the next quit pass through", async () => { diff --git a/packages/desktop/src/daemon/quit-lifecycle.ts b/packages/desktop/src/daemon/quit-lifecycle.ts index 21d4cc637..f2abdbfc8 100644 --- a/packages/desktop/src/daemon/quit-lifecycle.ts +++ b/packages/desktop/src/daemon/quit-lifecycle.ts @@ -6,11 +6,6 @@ interface QuitLifecycleSettings { }; } -interface QuitLifecycleStatus { - status: "starting" | "running" | "stopped" | "errored"; - desktopManaged: boolean; -} - interface BeforeQuitEvent { preventDefault(): void; } @@ -19,37 +14,31 @@ interface BeforeQuitApp { quit(): void; } -export function shouldStopDesktopManagedDaemonOnQuit({ - settings, - daemonStatus, -}: { - settings: QuitLifecycleSettings; - daemonStatus: QuitLifecycleStatus; -}): boolean { - return ( - settings.daemon.keepRunningAfterQuit === false && - daemonStatus.status === "running" && - daemonStatus.desktopManaged - ); +export interface StopOnQuitDeps { + settingsStore: Pick; + isDesktopManagedDaemonRunning: () => boolean; + stopDaemon: () => Promise; + showShutdownFeedback: () => void; } -export async function stopDesktopManagedDaemonOnQuitIfNeeded({ - settingsStore, - resolveStatus, - stopDaemon, -}: { - settingsStore: DesktopSettingsStore; - resolveStatus: () => Promise; - stopDaemon: () => Promise; -}): Promise { - const settings = await settingsStore.get(); - const daemonStatus = await resolveStatus(); +export function shouldStopDesktopManagedDaemonOnQuit(settings: QuitLifecycleSettings): boolean { + return settings.daemon.keepRunningAfterQuit === false; +} - if (!shouldStopDesktopManagedDaemonOnQuit({ settings, daemonStatus })) { +export async function stopDesktopManagedDaemonOnQuitIfNeeded( + deps: StopOnQuitDeps, +): Promise { + const settings = await deps.settingsStore.get(); + if (!shouldStopDesktopManagedDaemonOnQuit(settings)) { return false; } - await stopDaemon(); + if (!deps.isDesktopManagedDaemonRunning()) { + return false; + } + + deps.showShutdownFeedback(); + await deps.stopDaemon(); return true; } diff --git a/packages/desktop/src/main.ts b/packages/desktop/src/main.ts index d537c6fc2..fb72136be 100644 --- a/packages/desktop/src/main.ts +++ b/packages/desktop/src/main.ts @@ -9,7 +9,7 @@ import path from "node:path"; import { pathToFileURL } from "node:url"; import { existsSync } from "node:fs"; import { execFileSync } from "node:child_process"; -import { app, BrowserWindow, ipcMain, nativeImage, net, protocol } from "electron"; +import { app, BrowserWindow, dialog, ipcMain, nativeImage, net, protocol } from "electron"; import { registerDaemonManager } from "./daemon/daemon-manager.js"; import { parseCliPassthroughArgsFromArgv, @@ -35,7 +35,10 @@ import { registerOpenerHandlers } from "./features/opener.js"; import { setupApplicationMenu } from "./features/menu.js"; import { parseOpenProjectPathFromArgv } from "./open-project-routing.js"; import { getDesktopSettingsStore } from "./settings/desktop-settings-electron.js"; -import { resolveDesktopDaemonStatus, stopDesktopDaemon } from "./daemon/daemon-manager.js"; +import { + isDesktopManagedDaemonRunningSync, + stopDesktopDaemonViaCli, +} from "./daemon/daemon-manager.js"; import { createBeforeQuitHandler, stopDesktopManagedDaemonOnQuitIfNeeded, @@ -346,6 +349,16 @@ void bootstrap().catch((error) => { process.exit(1); }); +function showDaemonShutdownDialog(): void { + void dialog.showMessageBox({ + type: "info", + message: "Shutting down Paseo daemon…", + detail: "Paseo will quit once the local daemon has stopped.", + buttons: [], + noLink: true, + }); +} + app.on( "before-quit", createBeforeQuitHandler({ @@ -354,8 +367,9 @@ app.on( stopDesktopManagedDaemonIfNeeded: () => stopDesktopManagedDaemonOnQuitIfNeeded({ settingsStore: getDesktopSettingsStore(), - resolveStatus: resolveDesktopDaemonStatus, - stopDaemon: stopDesktopDaemon, + isDesktopManagedDaemonRunning: isDesktopManagedDaemonRunningSync, + stopDaemon: stopDesktopDaemonViaCli, + showShutdownFeedback: showDaemonShutdownDialog, }), onStopError: (error) => { log.error("[desktop daemon] failed to stop managed daemon on quit", error);