diff --git a/src/main/index.ts b/src/main/index.ts index 99dc44509f5..5bd19ac61bf 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -27,6 +27,10 @@ import { createMainWindow } from './window/createMainWindow' import { CodexAccountService } from './codex-accounts/service' let mainWindow: BrowserWindow | null = null +/** Whether a manual app.quit() (Cmd+Q, etc.) is in progress. Shared with the + * window close handler so it can tell the renderer to skip the running-process + * confirmation dialog and proceed directly to buffer capture + close. */ +let isQuitting = false let store: Store | null = null let stats: StatsCollector | null = null let claudeUsage: ClaudeUsageStore | null = null @@ -75,7 +79,12 @@ function openMainWindow(): BrowserWindow { throw new Error('Codex account service must be initialized before opening the main window') } - const window = createMainWindow(store) + const window = createMainWindow(store, { + getIsQuitting: () => isQuitting, + onQuitAborted: () => { + isQuitting = false + } + }) registerCoreHandlers( store, runtime, @@ -169,12 +178,21 @@ app.whenReady().then(async () => { }) app.on('before-quit', () => { + isQuitting = true + // Why: PTY cleanup is deferred to will-quit so the renderer has a chance to + // capture terminal scrollback buffers before PTY exit events race in and + // unmount TerminalPane components (removing their capture callbacks). + // The window close handler passes isQuitting to the renderer so it skips the + // child-process confirmation dialog and proceeds directly to buffer capture. + rateLimits?.stop() +}) + +app.on('will-quit', () => { // Why: stats.flush() must run before killAllPty() so it can read the // live agent state and emit synthetic agent_stop events for agents that // are still running. killAllPty() does not call runtime.onPtyExit(), // so without this ordering, running agents would produce orphaned // agent_start events with no matching stops. - rateLimits?.stop() stats?.flush() killAllPty() void closeAllWatchers() @@ -187,7 +205,12 @@ app.on('before-quit', () => { }) app.on('window-all-closed', () => { - if (process.platform !== 'darwin') { + // Why: on macOS, closing all windows normally keeps the app alive (dock + // stays active). But when a quit is in progress (Cmd+Q), the window close + // handler defers to the renderer for buffer capture, which cancels the + // original quit sequence. Re-trigger quit here so the app actually exits + // instead of requiring a second Cmd+Q. + if (process.platform !== 'darwin' || isQuitting) { app.quit() } }) diff --git a/src/main/window/createMainWindow.test.ts b/src/main/window/createMainWindow.test.ts index 58f12c0e7ce..811b68983be 100644 --- a/src/main/window/createMainWindow.test.ts +++ b/src/main/window/createMainWindow.test.ts @@ -318,8 +318,24 @@ describe('createMainWindow', () => { const isDarwin = process.platform === 'darwin' for (const input of [ - { type: 'keyDown', code: 'KeyJ', key: 'j', meta: isDarwin, control: !isDarwin, alt: false, shift: !isDarwin }, - { type: 'keyDown', code: 'KeyJ', key: '', meta: isDarwin, control: !isDarwin, alt: false, shift: !isDarwin } + { + type: 'keyDown', + code: 'KeyJ', + key: 'j', + meta: isDarwin, + control: !isDarwin, + alt: false, + shift: !isDarwin + }, + { + type: 'keyDown', + code: 'KeyJ', + key: '', + meta: isDarwin, + control: !isDarwin, + alt: false, + shift: !isDarwin + } ]) { const preventDefault = vi.fn() windowHandlers['before-input-event']({ preventDefault } as never, input as never) @@ -377,4 +393,47 @@ describe('createMainWindow', () => { expect(webContents.openDevTools).toHaveBeenCalledWith({ mode: 'undocked' }) expect(webContents.closeDevTools).not.toHaveBeenCalled() }) + + it('clears the quit latch when the renderer prevents unload', () => { + const windowHandlers: Record void> = {} + const webContents = { + on: vi.fn((event, handler) => { + windowHandlers[event] = handler + }), + setZoomLevel: vi.fn(), + setBackgroundThrottling: vi.fn(), + invalidate: vi.fn(), + setWindowOpenHandler: vi.fn(), + send: vi.fn() + } + const browserWindowInstance = { + webContents, + on: vi.fn((event, handler) => { + windowHandlers[event] = handler + }), + isDestroyed: vi.fn(() => false), + isMaximized: vi.fn(() => true), + isFullScreen: vi.fn(() => false), + getSize: vi.fn(() => [1200, 800]), + setSize: vi.fn(), + maximize: vi.fn(), + show: vi.fn(), + loadFile: vi.fn(), + loadURL: vi.fn() + } + const onQuitAborted = vi.fn() + browserWindowMock.mockImplementation(function () { + return browserWindowInstance + }) + + createMainWindow(null, { getIsQuitting: () => true, onQuitAborted }) + + const preventDefault = vi.fn() + windowHandlers.close({ preventDefault } as never) + expect(preventDefault).toHaveBeenCalledTimes(1) + expect(webContents.send).toHaveBeenCalledWith('window:close-requested', { isQuitting: true }) + + windowHandlers['will-prevent-unload']() + expect(onQuitAborted).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index cc1d8b374d1..6d7c604f13c 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -45,7 +45,22 @@ function syncTrafficLightPosition(win: BrowserWindow, zoomFactor: number): void win.setWindowButtonPosition({ x: TRAFFIC_LIGHT_X, y }) } -export function createMainWindow(store: Store | null): BrowserWindow { +type CreateMainWindowOptions = { + /** Returns true when a manual app.quit() (Cmd+Q) is in progress. The close + * handler sends this to the renderer so it can skip the running-process + * confirmation dialog and proceed directly to buffer capture + close. */ + getIsQuitting?: () => boolean + /** Notifies the caller when the renderer vetoes unload. Why: a prevented + * beforeunload cancels the in-flight app.quit(), so the app-level quit + * latch must be cleared or later window closes will be misclassified as + * quit attempts. */ + onQuitAborted?: () => void +} + +export function createMainWindow( + store: Store | null, + opts?: CreateMainWindowOptions +): BrowserWindow { const mainWindow = new BrowserWindow({ width: 1200, height: 800, @@ -258,7 +273,12 @@ export function createMainWindow(store: Store | null): BrowserWindow { return } e.preventDefault() - mainWindow.webContents.send('window:close-requested') + mainWindow.webContents.send('window:close-requested', { + isQuitting: opts?.getIsQuitting?.() ?? false + }) + }) + mainWindow.webContents.on('will-prevent-unload', () => { + opts?.onQuitAborted?.() }) const onConfirmClose = (): void => { diff --git a/src/preload/api-types.d.ts b/src/preload/api-types.d.ts index c5e84b43b35..c3af3683ea3 100644 --- a/src/preload/api-types.d.ts +++ b/src/preload/api-types.d.ts @@ -376,7 +376,7 @@ export type PreloadApi = { setZoomLevel: (level: number) => void syncTrafficLights: (zoomFactor: number) => void onFullscreenChanged: (callback: (isFullScreen: boolean) => void) => () => void - onWindowCloseRequested: (callback: () => void) => () => void + onWindowCloseRequested: (callback: (data: { isQuitting: boolean }) => void) => () => void confirmWindowClose: () => void } runtime: { diff --git a/src/preload/index.ts b/src/preload/index.ts index 8d646a6de2b..69456eb6cec 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -672,9 +672,12 @@ const api = { }, /** Fired by the main process when the user tries to close the window * (X button, Cmd+Q, etc.). Renderer should show a confirmation dialog - * if terminals are still running, then call confirmWindowClose(). */ - onWindowCloseRequested: (callback: () => void): (() => void) => { - const listener = () => callback() + * if terminals are still running, then call confirmWindowClose(). + * When isQuitting is true, the close was initiated by app.quit() (Cmd+Q) + * and the renderer should skip the running-process dialog. */ + onWindowCloseRequested: (callback: (data: { isQuitting: boolean }) => void): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent, data: { isQuitting: boolean }) => + callback(data ?? { isQuitting: false }) ipcRenderer.on('window:close-requested', listener) return () => ipcRenderer.removeListener('window:close-requested', listener) }, diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 826360ab69e..5b3c9aeae8d 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -252,7 +252,18 @@ function App(): React.JSX.Element { // On shutdown, capture terminal scrollback buffers and flush to disk. // Runs synchronously in beforeunload: capture → Zustand set → sendSync → flush. useEffect(() => { + // Why: beforeunload fires twice during a manual quit — once from the + // synthetic dispatch in the onWindowCloseRequested handler (captures + // good data while TerminalPanes are still mounted), and again from the + // native window close triggered by confirmWindowClose(). Between these + // two firings, PTY exit events can arrive and unmount TerminalPanes, + // emptying shutdownBufferCaptures. The guard prevents the second call + // from overwriting the good session data with an empty snapshot. + let shutdownBuffersCaptured = false const captureAndFlush = (): void => { + if (shutdownBuffersCaptured) { + return + } if (!useAppStore.getState().workspaceSessionReady) { return } @@ -265,17 +276,16 @@ function App(): React.JSX.Element { } const state = useAppStore.getState() window.api.session.setSync(buildWorkspaceSessionPayload(state)) + shutdownBuffersCaptured = true } window.addEventListener('beforeunload', captureAndFlush) return () => window.removeEventListener('beforeunload', captureAndFlush) }, []) // Periodically capture terminal scrollback buffers and persist to disk. - // Why: the normal shutdown path races with PTY exit events — before-quit - // kills all PTYs, whose async exit handlers close tabs and unmount - // TerminalPane components (removing their capture callbacks) before the - // renderer's beforeunload can run. Periodic saves ensure scrollback is - // available on restart even when the shutdown capture is lost. + // Why: the shutdown path captures buffers in beforeunload, but periodic + // saves provide a safety net so scrollback is available on restart even + // if an unexpected exit (crash, force-kill) bypasses normal shutdown. useEffect(() => { const PERIODIC_SAVE_INTERVAL_MS = 3 * 60_000 const timer = window.setInterval(() => { diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 884b2c59db7..e2782ebf8f6 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -608,18 +608,24 @@ export default function Terminal(): React.JSX.Element | null { // Listen for main-process window close requests. When any terminal has a // child process running (not just an idle shell), show a confirmation dialog. useEffect(() => { - return window.api.ui.onWindowCloseRequested(() => { + return window.api.ui.onWindowCloseRequested(({ isQuitting }) => { if (isUpdaterQuitAndInstallInProgress()) { window.api.ui.confirmWindowClose() return } - // Why: before-quit (main process) kills all PTYs before this handler - // runs. The async PTY exit events close tabs and unmount TerminalPane - // components, removing their buffer capture callbacks. Dispatching - // beforeunload here — while components are still mounted — captures - // scrollback buffers before the race can discard them. This mirrors - // the auto-updater path (preload/index.ts) which does the same thing. + // Why: capture terminal scrollback buffers while TerminalPane components + // are still mounted. Dispatching beforeunload triggers the App.tsx + // captureAndFlush handler which serializes each pane's xterm buffer + // and writes the session to disk via synchronous IPC. window.dispatchEvent(new Event('beforeunload')) + // Why: during a quit (Cmd+Q), PTYs are still alive (cleanup is deferred + // to will-quit so buffers can be captured first). Skip the child-process + // confirmation dialog and proceed directly — the user's intent to quit + // is unambiguous. + if (isQuitting) { + window.api.ui.confirmWindowClose() + return + } const state = useAppStore.getState() const allPtyIds = Object.values(state.ptyIdsByTabId).flat() if (allPtyIds.length === 0) {