From f3425f8957181d5c8468b8ee5c2ba2fa76fff04c Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 26 Jul 2026 19:48:48 +0200 Subject: [PATCH] fix(electron): re-open a window when a second launch finds none On macOS `window-all-closed` deliberately keeps the process alive while the 'closed' handler clears App.mainWindow, so the single-instance guard could hand a launch to a process with no window: focusExistingWindow(null) did nothing and the new process quit, leaving the user with nothing on screen. The second-instance handler now re-creates the window when none is live, via the same path the macOS dock 'activate' event uses. onActivate's body moved to a public App.ensureMainWindow() so both entry points share it. Reported by Codex review on #1272. Co-Authored-By: Claude Opus 5 --- apps/electron-backend-e2e/src/settings.e2e.ts | 36 +++++++++++++++++ apps/electron-backend/src/app/app.ts | 19 +++++++-- .../src/app/services/single-instance.spec.ts | 39 +++++++++++++++++-- .../src/app/services/single-instance.ts | 21 +++++++++- apps/electron-backend/src/main.ts | 11 +++++- 5 files changed, 116 insertions(+), 10 deletions(-) diff --git a/apps/electron-backend-e2e/src/settings.e2e.ts b/apps/electron-backend-e2e/src/settings.e2e.ts index 0525a0ceb..c22dc406c 100644 --- a/apps/electron-backend-e2e/src/settings.e2e.ts +++ b/apps/electron-backend-e2e/src/settings.e2e.ts @@ -168,6 +168,42 @@ test.describe('Electron Settings', () => { await closeElectronApp(relaunch); }); + test('@settings @electron re-opens a window when a second launch arrives with none open', async ({ + dataDir, + }) => { + // Only macOS keeps the process alive after its last window closes; + // elsewhere `window-all-closed` quits and the next launch is a plain + // cold start. + test.skip( + process.platform !== 'darwin', + 'macOS-only windowless-process behaviour' + ); + const runningApp = await launchElectronApp(dataDir); + + try { + await runningApp.electronApp.evaluate(({ BrowserWindow }) => { + for (const window of BrowserWindow.getAllWindows()) { + window.close(); + } + }); + await expect + .poll(() => runningApp.electronApp.windows().length) + .toBe(0); + + const recreatedWindow = + runningApp.electronApp.waitForEvent('window'); + const competing = await launchCompetingElectronInstance(dataDir); + + expect(competing.timedOut).toBe(false); + expect(competing.exitCode).toBe(0); + // Without this the guard would quit the launch into nothing and + // leave the user staring at no window at all. + await expect((await recreatedWindow).locator('body')).toBeVisible(); + } finally { + await closeElectronApp(runningApp); + } + }); + test('@settings @persistence @electron persists changed desktop settings across app restart', async ({ dataDir, }) => { diff --git a/apps/electron-backend/src/app/app.ts b/apps/electron-backend/src/app/app.ts index 7aef4a9ae..1f1113d90 100644 --- a/apps/electron-backend/src/app/app.ts +++ b/apps/electron-backend/src/app/app.ts @@ -268,9 +268,16 @@ export default class App { } } - private static onActivate() { - // On macOS it's common to re-create a window in the app when the - // dock icon is clicked and there are no other windows open. + /** + * Brings the app back to a windowed state, re-creating the main window if + * it is gone. + * + * On macOS closing the last window deliberately keeps the process alive + * (`onWindowAllClosed`), so this is the recovery path for both the dock + * `activate` event and a second launch that the single-instance guard + * hands over to this process. + */ + static ensureMainWindow() { if (App.mainWindow === null) { App.onReady(); } @@ -279,6 +286,12 @@ export default class App { } } + private static onActivate() { + // On macOS it's common to re-create a window in the app when the + // dock icon is clicked and there are no other windows open. + App.ensureMainWindow(); + } + private static handleRendererNavigation( event: Electron.Event, url: string diff --git a/apps/electron-backend/src/app/services/single-instance.spec.ts b/apps/electron-backend/src/app/services/single-instance.spec.ts index 0a66ad5f8..6baec2e37 100644 --- a/apps/electron-backend/src/app/services/single-instance.spec.ts +++ b/apps/electron-backend/src/app/services/single-instance.spec.ts @@ -59,7 +59,9 @@ describe('acquireSingleInstanceLock', () => { it('continues startup and registers the focus handler when the lock is free', () => { const app = createApp(true); - expect(acquireSingleInstanceLock(app, () => null, {})).toBe(true); + expect( + acquireSingleInstanceLock(app, () => null, jest.fn(), {}) + ).toBe(true); expect(app.quit).not.toHaveBeenCalled(); expect(app.on).toHaveBeenCalledWith( 'second-instance', @@ -70,7 +72,9 @@ describe('acquireSingleInstanceLock', () => { it('quits and stops startup when another instance owns the profile', () => { const app = createApp(false); - expect(acquireSingleInstanceLock(app, () => null, {})).toBe(false); + expect( + acquireSingleInstanceLock(app, () => null, jest.fn(), {}) + ).toBe(false); expect(app.quit).toHaveBeenCalledTimes(1); expect(app.on).not.toHaveBeenCalled(); }); @@ -82,20 +86,47 @@ describe('acquireSingleInstanceLock', () => { isVisible: jest.fn().mockReturnValue(false), }); - acquireSingleInstanceLock(app, () => window, {}); + const createMainWindow = jest.fn(); + acquireSingleInstanceLock(app, () => window, createMainWindow, {}); const [, handler] = app.on.mock.calls[0]; (handler as () => void)(); expect(window.restore).toHaveBeenCalledTimes(1); expect(window.show).toHaveBeenCalledTimes(1); expect(window.focus).toHaveBeenCalledTimes(1); + expect(createMainWindow).not.toHaveBeenCalled(); + }); + + // macOS keeps the process alive after the last window closes, so a second + // launch must rebuild a window instead of quitting into nothing. + it.each([ + ['no window exists', null], + ['the window was destroyed', 'destroyed'], + ])('re-creates the main window when %s', (_label, windowState) => { + const app = createApp(true); + const window = + windowState === 'destroyed' + ? createWindow({ + isDestroyed: jest.fn().mockReturnValue(true), + }) + : null; + const createMainWindow = jest.fn(); + + acquireSingleInstanceLock(app, () => window, createMainWindow, {}); + const [, handler] = app.on.mock.calls[0]; + (handler as () => void)(); + + expect(createMainWindow).toHaveBeenCalledTimes(1); + if (window) { + expect(window.focus).not.toHaveBeenCalled(); + } }); it('skips the lock entirely when multiple instances are allowed', () => { const app = createApp(false); expect( - acquireSingleInstanceLock(app, () => null, { + acquireSingleInstanceLock(app, () => null, jest.fn(), { [ALLOW_MULTIPLE_INSTANCES_ENV]: '1', }) ).toBe(true); diff --git a/apps/electron-backend/src/app/services/single-instance.ts b/apps/electron-backend/src/app/services/single-instance.ts index 1c2922108..8186ed2ce 100644 --- a/apps/electron-backend/src/app/services/single-instance.ts +++ b/apps/electron-backend/src/app/services/single-instance.ts @@ -67,15 +67,27 @@ export function focusExistingWindow( window.focus(); } +/** True when there is no live window left to bring forward. */ +function needsNewWindow( + window: SingleInstanceWindow | null | undefined +): boolean { + return !window || window.isDestroyed(); +} + /** * Acquires the single instance lock. * + * @param createMainWindow re-creates the main window when the lock owner has + * none left. On macOS closing the last window keeps the process alive, so + * without this a second launch would quit silently and leave the user with no + * window at all. * @returns `true` when this process owns the profile and startup may continue, * `false` when another instance already owns it and this one is quitting. */ export function acquireSingleInstanceLock( app: SingleInstanceApp, getMainWindow: () => SingleInstanceWindow | null | undefined, + createMainWindow: () => void, env: NodeJS.ProcessEnv = process.env ): boolean { if (allowsMultipleInstances(env)) { @@ -88,7 +100,14 @@ export function acquireSingleInstanceLock( } app.on('second-instance', () => { - focusExistingWindow(getMainWindow()); + const mainWindow = getMainWindow(); + + if (needsNewWindow(mainWindow)) { + createMainWindow(); + return; + } + + focusExistingWindow(mainWindow); }); return true; diff --git a/apps/electron-backend/src/main.ts b/apps/electron-backend/src/main.ts index 120c4ce62..9b91f7b34 100644 --- a/apps/electron-backend/src/main.ts +++ b/apps/electron-backend/src/main.ts @@ -197,8 +197,15 @@ runEmbeddedMpvRuntimeDiagnosticOrContinue(process.argv, () => { // A second instance sharing this userData directory cannot take the // Chromium storage lock, so its renderer silently loses every settings - // write. Focus the window that already owns the profile instead. - if (!acquireSingleInstanceLock(app, () => App.mainWindow)) { + // write. Focus the window that already owns the profile instead — or + // re-create it, since on macOS the process outlives its last window. + if ( + !acquireSingleInstanceLock( + app, + () => App.mainWindow, + () => App.ensureMainWindow() + ) + ) { return; }