From e6bb877f55caf1ea0d8b0a09b76122b251ffe31d Mon Sep 17 00:00:00 2001 From: Serhii Vecherenko Date: Tue, 18 Aug 2026 09:30:48 -0700 Subject: [PATCH] fix(main): manage main window close lifecycle - defer app quit until the main window closes - test supervisor disposal without restart - add close lifecycle coverage --- src/main/main.ts | 27 ++++--- src/main/supervisor/SupervisorClient.test.ts | 27 ++++++- src/main/window/mainWindowClose.test.ts | 74 ++++++++++++++++++++ src/main/window/mainWindowClose.ts | 38 ++++++++++ 4 files changed, 154 insertions(+), 12 deletions(-) create mode 100644 src/main/window/mainWindowClose.test.ts create mode 100644 src/main/window/mainWindowClose.ts diff --git a/src/main/main.ts b/src/main/main.ts index b7b0a5499..17f3a4461 100644 --- a/src/main/main.ts +++ b/src/main/main.ts @@ -63,6 +63,7 @@ import { showQuickComposerWindow, } from "./window/createQuickComposerWindow"; import { showAndFocusWindow } from "./window/showAndFocusWindow"; +import { createMainWindowCloseLifecycle } from "./window/mainWindowClose"; import { createTray, type TrayHandle } from "./tray"; import { readKeybindingsFile } from "./keybindingsFile"; import { QuickComposerShortcutManager } from "./quickComposerShortcut"; @@ -359,14 +360,6 @@ function updateCrossagentRoutingOverride( mainWindow?.webContents.send(IPC_EVENT_CHANNELS.sharedSettingsChanged, next); } -function handleMainWindowClose(event: Electron.Event): void { - if (isQuitting) return; - if (!mainWindow || mainWindow.isDestroyed()) return; - if (!isCloseToTrayEnabled()) return; - event.preventDefault(); - mainWindow.hide(); -} - function quickComposerWindowFor(event: Electron.IpcMainInvokeEvent): BrowserWindow | null { const window = BrowserWindow.fromWebContents(event.sender); return window && window === quickComposerWindow && !window.isDestroyed() ? window : null; @@ -501,17 +494,29 @@ function forwardAgentStatusEventToQuickComposer(event: SupervisorEvent): void { function createMainAppWindow(showOnReady = true): BrowserWindow { const windowChrome = resolveWindowChromeOptions(); - const window = createMainWindow({ + let window: BrowserWindow; + const closeLifecycle = createMainWindowCloseLifecycle({ + isQuitting: () => isQuitting, + closeToTrayEnabled: isCloseToTrayEnabled, + hide: () => window.hide(), + markQuitting: () => { + isQuitting = true; + }, + quit: () => app.quit(), + }); + window = createMainWindow({ ...commonAppWindowOptions(), windowChromeHeight: WINDOW_CHROME_HEIGHT, appearance: windowChrome.appearance, sidebarTranslucency: windowChrome.sidebarTranslucency, showOnReady, onClosed: () => { - if (mainWindow === window) mainWindow = null; + const wasMainWindow = mainWindow === window; + if (wasMainWindow) mainWindow = null; mainRendererReady = false; + closeLifecycle.handleClosed(); }, - onClose: handleMainWindowClose, + onClose: (event) => closeLifecycle.handleClose(event), onRendererProcessGone: (details, intent) => { mainRendererReady = false; captureRendererProcessGone(details, "renderer", intent); diff --git a/src/main/supervisor/SupervisorClient.test.ts b/src/main/supervisor/SupervisorClient.test.ts index ada061225..99afb2b2e 100644 --- a/src/main/supervisor/SupervisorClient.test.ts +++ b/src/main/supervisor/SupervisorClient.test.ts @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { SupervisorEvent } from "@/shared/ipc"; const forkMock = vi.hoisted(() => vi.fn<(...args: unknown[]) => unknown>()); +const terminateChildProcessTreeMock = vi.hoisted(() => vi.fn<() => void>()); vi.mock("node:child_process", async (importOriginal) => { const actual = await importOriginal(); @@ -10,7 +11,7 @@ vi.mock("node:child_process", async (importOriginal) => { }); vi.mock("@/shared/processTree", () => ({ - terminateChildProcessTree: vi.fn<() => void>(), + terminateChildProcessTree: terminateChildProcessTreeMock, })); import { SupervisorClient, type SupervisorClientOptions } from "./SupervisorClient"; @@ -69,6 +70,7 @@ function captureSentId(child: FakeChild): () => string { describe("SupervisorClient.call", () => { beforeEach(() => { forkMock.mockReset(); + terminateChildProcessTreeMock.mockReset(); }); afterEach(() => { @@ -178,3 +180,26 @@ describe("SupervisorClient.call", () => { await vi.advanceTimersByTimeAsync(10 * 60 * 1000 + 1); }); }); + +describe("SupervisorClient lifecycle", () => { + beforeEach(() => { + forkMock.mockReset(); + terminateChildProcessTreeMock.mockReset(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("terminates the supervisor tree without restarting it when disposed", async () => { + vi.useFakeTimers(); + const { client, child } = makeClient(); + + client.dispose(); + child.emit("exit", 1); + await vi.advanceTimersByTimeAsync(1_000); + + expect(terminateChildProcessTreeMock).toHaveBeenCalledExactlyOnceWith(child); + expect(forkMock).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/main/window/mainWindowClose.test.ts b/src/main/window/mainWindowClose.test.ts new file mode 100644 index 000000000..4efe19721 --- /dev/null +++ b/src/main/window/mainWindowClose.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it, vi } from "vitest"; +import { createMainWindowCloseLifecycle } from "./mainWindowClose"; + +function createHarness(overrides: { isQuitting?: boolean; closeToTrayEnabled?: boolean } = {}) { + const event = { preventDefault: vi.fn<() => void>() }; + const options = { + isQuitting: vi.fn<() => boolean>(() => overrides.isQuitting ?? false), + closeToTrayEnabled: vi.fn<() => boolean>(() => overrides.closeToTrayEnabled ?? false), + hide: vi.fn<() => void>(), + markQuitting: vi.fn<() => void>(), + quit: vi.fn<() => void>(), + }; + const lifecycle = createMainWindowCloseLifecycle(options); + return { event, options, lifecycle }; +} + +describe("createMainWindowCloseLifecycle", () => { + it("starts an orderly app quit only after the main window has closed", () => { + const { event, options, lifecycle } = createHarness(); + + lifecycle.handleClose(event); + + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(options.hide).not.toHaveBeenCalled(); + expect(options.markQuitting).toHaveBeenCalledOnce(); + expect(options.quit).not.toHaveBeenCalled(); + + lifecycle.handleClosed(); + + expect(options.quit).toHaveBeenCalledOnce(); + lifecycle.handleClosed(); + expect(options.quit).toHaveBeenCalledOnce(); + }); + + it("keeps the app running and hides the window when close to tray is enabled", () => { + const { event, options, lifecycle } = createHarness({ closeToTrayEnabled: true }); + + lifecycle.handleClose(event); + + expect(event.preventDefault).toHaveBeenCalledOnce(); + expect(options.hide).toHaveBeenCalledOnce(); + expect(options.markQuitting).not.toHaveBeenCalled(); + lifecycle.handleClosed(); + expect(options.quit).not.toHaveBeenCalled(); + }); + + it("does not interfere with a quit already in progress", () => { + const { event, options, lifecycle } = createHarness({ isQuitting: true }); + + lifecycle.handleClose(event); + lifecycle.handleClosed(); + + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(options.closeToTrayEnabled).not.toHaveBeenCalled(); + expect(options.hide).not.toHaveBeenCalled(); + expect(options.markQuitting).not.toHaveBeenCalled(); + expect(options.quit).not.toHaveBeenCalled(); + }); + + it("does not consume another window's armed close", () => { + const first = createHarness(); + const second = createHarness(); + + first.lifecycle.handleClose(first.event); + second.lifecycle.handleClosed(); + + expect(first.options.quit).not.toHaveBeenCalled(); + expect(second.options.quit).not.toHaveBeenCalled(); + + first.lifecycle.handleClosed(); + + expect(first.options.quit).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/main/window/mainWindowClose.ts b/src/main/window/mainWindowClose.ts new file mode 100644 index 000000000..b2051721c --- /dev/null +++ b/src/main/window/mainWindowClose.ts @@ -0,0 +1,38 @@ +export interface MainWindowCloseLifecycleOptions { + isQuitting(): boolean; + closeToTrayEnabled(): boolean; + hide(): void; + markQuitting(): void; + quit(): void; +} + +export interface MainWindowCloseLifecycle { + handleClose(event: Pick): void; + handleClosed(): void; +} + +export function createMainWindowCloseLifecycle( + options: MainWindowCloseLifecycleOptions, +): MainWindowCloseLifecycle { + let quitAfterClose = false; + + return { + handleClose(event) { + if (options.isQuitting()) return; + + if (options.closeToTrayEnabled()) { + event.preventDefault(); + options.hide(); + return; + } + + options.markQuitting(); + quitAfterClose = true; + }, + handleClosed() { + if (!quitAfterClose) return; + quitAfterClose = false; + options.quit(); + }, + }; +}