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.
This commit is contained in:
@@ -27,6 +27,7 @@ export function createDaemonCommand(): Command {
|
||||
.option("--home <path>", "Paseo home directory (default: ~/.paseo)")
|
||||
.option("--timeout <seconds>", "Wait timeout before failing (default: 15)")
|
||||
.option("--force", "Send SIGKILL if graceful stop times out")
|
||||
.option("--kill-timeout <seconds>", "Wait after SIGKILL before failing (default: 3)")
|
||||
.action(withOutput(runStopCommand));
|
||||
|
||||
addJsonOption(daemon.command("restart").description("Restart the local daemon"))
|
||||
|
||||
@@ -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<StopLocalDaemonResult> {
|
||||
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) {
|
||||
|
||||
@@ -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<StopResult> = {
|
||||
|
||||
export type StopCommandResult = SingleResult<StopResult>;
|
||||
|
||||
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<StopCommandResult> {
|
||||
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: {
|
||||
|
||||
@@ -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<void> {
|
||||
await runCliJsonCommand([
|
||||
"daemon",
|
||||
"stop",
|
||||
"--json",
|
||||
"--timeout",
|
||||
"5",
|
||||
"--force",
|
||||
"--kill-timeout",
|
||||
"5",
|
||||
]);
|
||||
}
|
||||
|
||||
function isProcessRunning(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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<DesktopSettingsStore, "get">;
|
||||
isDesktopManagedDaemonRunning: () => boolean;
|
||||
stopDaemon: () => Promise<unknown>;
|
||||
showShutdownFeedback: () => void;
|
||||
}
|
||||
|
||||
export async function stopDesktopManagedDaemonOnQuitIfNeeded({
|
||||
settingsStore,
|
||||
resolveStatus,
|
||||
stopDaemon,
|
||||
}: {
|
||||
settingsStore: DesktopSettingsStore;
|
||||
resolveStatus: () => Promise<QuitLifecycleStatus>;
|
||||
stopDaemon: () => Promise<unknown>;
|
||||
}): Promise<boolean> {
|
||||
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<boolean> {
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user