From 0dd73adce432a44929c20d7f0c27b97414676a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=A7=A0=F0=9F=8C=B8On=20Gaia?= Date: Tue, 8 Sep 2026 00:13:33 -0400 Subject: [PATCH 1/2] fix(window): make a backgrounded window reachable again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Keep them alive in the background" hides the window instead of closing it, which is the point — the agents keep running. Getting back to it is where it breaks down, in two ways. Nothing owns a second launch. Clicking the icon, or running the binary again, starts a whole second process. It reads the same state file, restores every persisted session, and spawns a duplicate agent for each one — on top of the PTYs the first instance is still holding. The hidden window is not raised, because nothing is listening for the launch. On Linux, where there is no dock to click, that launch is the only gesture a user has, so the app answers "come back" by cloning itself over its own running terminals. `show()` is also not enough on its own. It is a no-op on a minimized window, and does nothing for a window that is visible but buried behind another app. Both states reach the same handlers. - Take the single-instance lock in packaged builds. A second launch now arrives at the running instance as `second-instance` and restores its window instead of starting a process. Dev runs skip the lock, so `npm run dev` still starts while an installed build is running. - Add `restoreWindow()`: show if hidden, restore if minimized, focus either way, no-op if the window is missing or destroyed. Typed structurally so it is testable without an Electron runtime. - Route all three call sites through it — `activate` (the existing dock handler), the new `second-instance`, and the `before-quit` prompt, whose comment already said a quit must not prompt where the user cannot see it. 7 unit tests over the state matrix. Full suite passes. --- electron/main.ts | 100 +++++++++++++++++++------------- electron/window-restore.test.ts | 77 ++++++++++++++++++++++++ electron/window-restore.ts | 38 ++++++++++++ 3 files changed, 175 insertions(+), 40 deletions(-) create mode 100644 electron/window-restore.test.ts create mode 100644 electron/window-restore.ts diff --git a/electron/main.ts b/electron/main.ts index cbeeba52..014e99dd 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -1,5 +1,6 @@ import { app, autoUpdater, BrowserWindow, Menu, ipcMain, session, shell } from 'electron'; import { buildMenuTemplate } from './menu-template.js'; +import { restoreWindow } from './window-restore.js'; import path from 'path'; import fs from 'fs'; import { fileURLToPath } from 'url'; @@ -226,41 +227,61 @@ function createWindow() { }); } -app.whenReady().then(async () => { - // Grant microphone and clipboard access (deny camera/video) - session.defaultSession.setPermissionRequestHandler( - (_webContents, permission, callback, details) => { - if (permission === 'clipboard-read' || permission === 'clipboard-sanitized-write') { - return callback(true); - } - if (permission === 'media') { - const types = (details as { mediaTypes?: string[] }).mediaTypes ?? []; - return callback(types.every((t) => t === 'audio')); - } - callback(false); - }, - ); +// One running copy per profile. "Keep them alive in the background" hides the +// window instead of closing it, so a user who launches the app again is asking +// for the window they already have — but without the lock a second process +// starts, restores every persisted session from the same state file, and spawns +// a duplicate agent for each one on top of the PTYs the hidden instance is still +// holding. The hidden window has no way back either: nothing is listening for +// the launch. Taking the lock turns a second launch into "show the window". +// +// Dev runs skip the lock deliberately, so `npm run dev` still starts while an +// installed build is running. +const isPrimaryInstance = !app.isPackaged || app.requestSingleInstanceLock(); - // electron-updater stages the install, then quits through `app.quit()`. - // Vetoing that quit below would leave the update staged with the app still - // running, so let it through — the window's own close prompt still asks about - // running terminals, and `autoInstallOnAppQuit` re-applies the update on the - // next quit if the user backs out. Both platform paths announce the relaunch - // on Electron's own updater immediately before quitting (the AppImage updater - // emits it by hand, Squirrel natively), so this is set only while a quit is - // genuinely in flight — unlike a flag set when the install is *requested*, - // which sticks for the whole session on the many paths where - // `quitAndInstall()` returns without quitting. - autoUpdater.on('before-quit-for-update', () => { - quittingForUpdate = true; - }); +if (!isPrimaryInstance) { + app.quit(); +} else { + // A second launch (icon, CLI, file manager) reaches the instance that owns + // the lock as this event instead of starting a process of its own. + app.on('second-instance', () => restoreWindow(mainWindow)); - // Listening before the window exists: a renderer cannot spawn a Claude - // agent that misses its hooks. Failure falls back to PTY heuristics. - await startAgentHookRuntime(() => mainWindow); - setupApplicationMenu(); - createWindow(); -}); + app.whenReady().then(async () => { + // Grant microphone and clipboard access (deny camera/video) + session.defaultSession.setPermissionRequestHandler( + (_webContents, permission, callback, details) => { + if (permission === 'clipboard-read' || permission === 'clipboard-sanitized-write') { + return callback(true); + } + if (permission === 'media') { + const types = (details as { mediaTypes?: string[] }).mediaTypes ?? []; + return callback(types.every((t) => t === 'audio')); + } + callback(false); + }, + ); + + // electron-updater stages the install, then quits through `app.quit()`. + // Vetoing that quit below would leave the update staged with the app still + // running, so let it through — the window's own close prompt still asks about + // running terminals, and `autoInstallOnAppQuit` re-applies the update on the + // next quit if the user backs out. Both platform paths announce the relaunch + // on Electron's own updater immediately before quitting (the AppImage updater + // emits it by hand, Squirrel natively), so this is set only while a quit is + // genuinely in flight — unlike a flag set when the install is *requested*, + // which sticks for the whole session on the many paths where + // `quitAndInstall()` returns without quitting. + autoUpdater.on('before-quit-for-update', () => { + quittingForUpdate = true; + }); + + // Listening before the window exists: a renderer cannot spawn a Claude + // agent that misses its hooks. Failure falls back to PTY heuristics. + await startAgentHookRuntime(() => mainWindow); + setupApplicationMenu(); + createWindow(); + }); +} // A quit reaches `before-quit` *before* any window `close` event, so tearing // down agents here destroyed the very terminals the close dialog was about to @@ -274,9 +295,9 @@ app.whenReady().then(async () => { app.on('before-quit', (event) => { if (!mainWindow || mainWindow.isDestroyed() || quittingForUpdate) return; event.preventDefault(); - // The confirmation is a sheet on this window, and show() also focuses — a - // quit from the menu while the app sits hidden must not prompt invisibly. - mainWindow.show(); + // The confirmation is a sheet on this window — a quit from the menu while the + // app sits hidden or minimized must not prompt somewhere the user cannot see. + restoreWindow(mainWindow); mainWindow.close(); }); @@ -292,10 +313,9 @@ app.on('will-quit', () => { }); // "Keep them alive in the background" hides the window; without this the dock -// icon is a dead end and the only way back is attempting to quit. -app.on('activate', () => { - mainWindow?.show(); -}); +// icon is a dead end and the only way back is attempting to quit. `show()` alone +// left a minimized or buried window where it was — see restoreWindow. +app.on('activate', () => restoreWindow(mainWindow)); app.on('window-all-closed', () => { app.quit(); diff --git a/electron/window-restore.test.ts b/electron/window-restore.test.ts new file mode 100644 index 00000000..45a528d6 --- /dev/null +++ b/electron/window-restore.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it, vi } from 'vitest'; +import { restoreWindow, type RestorableWindow } from './window-restore.js'; + +interface FakeWindow extends RestorableWindow { + calls: string[]; +} + +function fakeWindow( + state: { destroyed?: boolean; visible?: boolean; minimized?: boolean } = {}, +): FakeWindow { + const calls: string[] = []; + return { + calls, + isDestroyed: () => state.destroyed ?? false, + isVisible: () => state.visible ?? true, + isMinimized: () => state.minimized ?? false, + show: () => void calls.push('show'), + restore: () => void calls.push('restore'), + focus: () => void calls.push('focus'), + }; +} + +describe('restoreWindow', () => { + // The case the whole function exists for: "Keep them alive in the background" + // hides the window, and without `show()` there is no way back to it at all. + it('shows a hidden window and focuses it', () => { + const win = fakeWindow({ visible: false }); + restoreWindow(win); + expect(win.calls).toEqual(['show', 'focus']); + }); + + // `show()` is a no-op on a minimized window, so a handler that only called + // `show()` would leave the user's click doing nothing at all. + it('restores a minimized window and focuses it', () => { + const win = fakeWindow({ minimized: true }); + restoreWindow(win); + expect(win.calls).toEqual(['restore', 'focus']); + }); + + // Minimized windows report themselves as not visible on some platforms; + // both branches have to run or one of the two platforms is left broken. + it('handles a window that is both hidden and minimized', () => { + const win = fakeWindow({ visible: false, minimized: true }); + restoreWindow(win); + expect(win.calls).toEqual(['show', 'restore', 'focus']); + }); + + // Visible but buried behind another app: nothing to show or restore, but the + // user asked for this window, so it still has to come forward. + it('focuses a window that is already visible', () => { + const win = fakeWindow(); + restoreWindow(win); + expect(win.calls).toEqual(['focus']); + }); + + // The window is nulled on `closed`, but these events can arrive in the gap + // before that fires, and calling into a destroyed window throws. + it('is a no-op for a destroyed window', () => { + const win = fakeWindow({ destroyed: true, visible: false, minimized: true }); + expect(() => restoreWindow(win)).not.toThrow(); + expect(win.calls).toEqual([]); + }); + + it('is a no-op for a missing window', () => { + expect(() => restoreWindow(null)).not.toThrow(); + expect(() => restoreWindow(undefined)).not.toThrow(); + }); + + // Guard clauses must not swallow the calls they guard: a regression that made + // `isDestroyed()` throw would otherwise look like a passing no-op test. + it('asks whether the window is destroyed before touching it', () => { + const isDestroyed = vi.fn(() => false); + const win = { ...fakeWindow(), isDestroyed }; + restoreWindow(win); + expect(isDestroyed).toHaveBeenCalled(); + }); +}); diff --git a/electron/window-restore.ts b/electron/window-restore.ts new file mode 100644 index 00000000..6325dcab --- /dev/null +++ b/electron/window-restore.ts @@ -0,0 +1,38 @@ +// Bringing the main window back from wherever the user left it. +// +// "Keep them alive in the background" hides the window rather than closing it, +// which is the whole point — the agents keep running. But a hidden window is +// only useful if there is a way back to it, and `show()` alone is not that way: +// a window the user minimized is still "visible" to Electron, so `show()` is a +// no-op on it, and a window that is visible but buried behind other apps needs +// `focus()` to come forward. Each entry point (dock click, second launch, tray) +// can hit any of those three states, so they all route through one function +// that handles all three rather than each guessing. +// +// Typed structurally instead of against `BrowserWindow` so the behaviour can be +// tested without an Electron runtime. `BrowserWindow` satisfies this shape. +export interface RestorableWindow { + isDestroyed(): boolean; + isVisible(): boolean; + isMinimized(): boolean; + show(): void; + restore(): void; + focus(): void; +} + +/** + * Bring `win` back into view, whatever state it is in: hidden, minimized, + * behind another app, or any combination. + * + * A no-op for a missing or destroyed window — the window is set to null on + * `closed`, but the events that call this can arrive in the gap before that + * fires, and calling into a destroyed window throws. + */ +export function restoreWindow(win: RestorableWindow | null | undefined): void { + if (!win || win.isDestroyed()) return; + // Order matters: a minimized window reports `isVisible() === false` on some + // platforms and `true` on others, so ask both questions and act on each. + if (!win.isVisible()) win.show(); + if (win.isMinimized()) win.restore(); + win.focus(); +} From 64123b3670d962aed4e528e4d96eec01c6a29db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=A7=A0=F0=9F=8C=B8On=20Gaia?= Date: Tue, 8 Sep 2026 15:01:06 -0400 Subject: [PATCH 2/2] fix(window): take the single-instance lock before resolving the environment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. fixEnv() spawns an interactive login shell at module scope, and the lock was requested 150 lines later — so the second, doomed instance paid for a full shell startup before finding out it should quit, directly on the icon-relaunch path the lock exists to make instant. The lock decision now happens before that call, and fixEnv() moves into the primary branch: only the primary spawns PTYs, so only the primary needs the resolved environment. Measured on a packaged build (3 runs each, headless Linux, nvm + conda in the rc file): the second instance went from 7.86s to 1.25s, with the login-shell probe no longer appearing at all while it runs. Also from review: - Drop the claim that show() is a no-op on a minimized window. Electron's docs do not say that either way, and the test file's comment told a different story from the source comment. The function calls both and is correct regardless; only the narration was over-specified. - Note the Wayland caveat on the visible-but-buried case: a client generally cannot raise itself there, and Electron's own docs say focus() may flash the icon instead. The doc comment stated that case as solved. - Drop 'tray' from the list of entry points. There is no Electron Tray in this repo. - Drop the test that only asserted isDestroyed() was called; the no-op test already covers it. --- electron/main.ts | 42 ++++++++++++++++++++------------- electron/window-restore.test.ts | 19 ++++----------- electron/window-restore.ts | 21 ++++++++++------- 3 files changed, 44 insertions(+), 38 deletions(-) diff --git a/electron/main.ts b/electron/main.ts index 014e99dd..9947bae1 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -81,7 +81,24 @@ function fixEnv(): void { } } -fixEnv(); +// One running copy per profile, and the lock is taken here rather than beside the +// window wiring because Electron's guidance is to take it as early as possible and +// this file gives that guidance teeth: fixEnv() above spawns an interactive login +// shell, which on a normal rc file (nvm, conda, compinit) costs on the order of half +// a second. A second launch is going to quit — spending that first would put the +// delay squarely on the icon-relaunch path the lock exists to make instant. +// +// Dev runs skip the lock deliberately, so `npm run dev` still starts while an +// installed build is running. +const isPrimaryInstance = !app.isPackaged || app.requestSingleInstanceLock(); + +if (!isPrimaryInstance) { + app.quit(); +} else { + // Only the primary instance ever spawns a PTY, so it is the only one that needs + // the resolved login-shell environment. + fixEnv(); +} // Blink evicts the oldest WebGL context past 16 per renderer process, and every // mounted terminal pane holds one — hidden task/tab terminals included. Past 16 @@ -227,21 +244,14 @@ function createWindow() { }); } -// One running copy per profile. "Keep them alive in the background" hides the -// window instead of closing it, so a user who launches the app again is asking -// for the window they already have — but without the lock a second process -// starts, restores every persisted session from the same state file, and spawns -// a duplicate agent for each one on top of the PTYs the hidden instance is still -// holding. The hidden window has no way back either: nothing is listening for -// the launch. Taking the lock turns a second launch into "show the window". -// -// Dev runs skip the lock deliberately, so `npm run dev` still starts while an -// installed build is running. -const isPrimaryInstance = !app.isPackaged || app.requestSingleInstanceLock(); - -if (!isPrimaryInstance) { - app.quit(); -} else { +// Why the lock matters here: "Keep them alive in the background" hides the window +// instead of closing it, so a user who launches the app again is asking for the +// window they already have. Without the lock a second process starts, restores +// every persisted session from the same state file, and spawns a duplicate agent +// for each one — on top of the PTYs the hidden instance is still holding. The +// hidden window has no way back either, because nothing is listening for the +// launch. With the lock, a second launch becomes "show the window". +if (isPrimaryInstance) { // A second launch (icon, CLI, file manager) reaches the instance that owns // the lock as this event instead of starting a process of its own. app.on('second-instance', () => restoreWindow(mainWindow)); diff --git a/electron/window-restore.test.ts b/electron/window-restore.test.ts index 45a528d6..1f3f5775 100644 --- a/electron/window-restore.test.ts +++ b/electron/window-restore.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { restoreWindow, type RestorableWindow } from './window-restore.js'; interface FakeWindow extends RestorableWindow { @@ -29,16 +29,16 @@ describe('restoreWindow', () => { expect(win.calls).toEqual(['show', 'focus']); }); - // `show()` is a no-op on a minimized window, so a handler that only called - // `show()` would leave the user's click doing nothing at all. + // A handler that only called `show()` would leave a minimized window where it + // was: `restore()` is the call that un-minimizes. it('restores a minimized window and focuses it', () => { const win = fakeWindow({ minimized: true }); restoreWindow(win); expect(win.calls).toEqual(['restore', 'focus']); }); - // Minimized windows report themselves as not visible on some platforms; - // both branches have to run or one of the two platforms is left broken. + // The two states are not exclusive, and the function must not treat them as + // such — a window can be hidden and minimized at the same time. it('handles a window that is both hidden and minimized', () => { const win = fakeWindow({ visible: false, minimized: true }); restoreWindow(win); @@ -65,13 +65,4 @@ describe('restoreWindow', () => { expect(() => restoreWindow(null)).not.toThrow(); expect(() => restoreWindow(undefined)).not.toThrow(); }); - - // Guard clauses must not swallow the calls they guard: a regression that made - // `isDestroyed()` throw would otherwise look like a passing no-op test. - it('asks whether the window is destroyed before touching it', () => { - const isDestroyed = vi.fn(() => false); - const win = { ...fakeWindow(), isDestroyed }; - restoreWindow(win); - expect(isDestroyed).toHaveBeenCalled(); - }); }); diff --git a/electron/window-restore.ts b/electron/window-restore.ts index 6325dcab..b0277f98 100644 --- a/electron/window-restore.ts +++ b/electron/window-restore.ts @@ -2,12 +2,16 @@ // // "Keep them alive in the background" hides the window rather than closing it, // which is the whole point — the agents keep running. But a hidden window is -// only useful if there is a way back to it, and `show()` alone is not that way: -// a window the user minimized is still "visible" to Electron, so `show()` is a -// no-op on it, and a window that is visible but buried behind other apps needs -// `focus()` to come forward. Each entry point (dock click, second launch, tray) -// can hit any of those three states, so they all route through one function -// that handles all three rather than each guessing. +// only useful if there is a way back to it, and the entry points that ask for +// it (a dock click, a second launch) can arrive with the window hidden, +// minimized, or merely behind another app. Each of those needs a different call, +// so they all route through one function that makes all three rather than each +// caller guessing which one applies. +// +// Wayland caveat: a client generally cannot raise itself there, and Electron's +// own docs say `focus()` on Wayland "may show a notification or flash the app +// icon" instead. So the visible-but-buried case can end at an icon flash rather +// than a raise, depending on the compositor. Hidden and minimized are unaffected. // // Typed structurally instead of against `BrowserWindow` so the behaviour can be // tested without an Electron runtime. `BrowserWindow` satisfies this shape. @@ -30,8 +34,9 @@ export interface RestorableWindow { */ export function restoreWindow(win: RestorableWindow | null | undefined): void { if (!win || win.isDestroyed()) return; - // Order matters: a minimized window reports `isVisible() === false` on some - // platforms and `true` on others, so ask both questions and act on each. + // Both questions get asked, and each answer gets acted on independently: the + // two states are not exclusive, and how a minimized window reports its + // visibility is not something to depend on. if (!win.isVisible()) win.show(); if (win.isMinimized()) win.restore(); win.focus();