fix(settings): harden close guard against stale quit intent and updater installs

Review-round fixes for the unsaved-settings close guard:

- A pending quit intent is never downgraded by a repeated close click
  while the dialog is open, and choosing Stay now explicitly cancels the
  intercepted intent in the main process (WINDOW:CANCEL_CLOSE), so a
  later close cannot replay a stale quit. Renderer navigation or a crash
  also drops the unanswered intent. (Greptile P1)
- Irreversible teardown (MPV/VLC sessions, embedded MPV, DB worker)
  moves from 'before-quit' to 'will-quit': the guard can cancel a quit
  mid-flight, and 'before-quit' has already fired by then — the old
  placement left a cancelled quit with playback and database work
  destroyed. (Codex P1)
- The self-updater's quitAndInstall() stands the guard down for exactly
  one close (allowNextClose): on macOS it closes windows before
  'before-quit', so an armed guard would strand the requested install as
  a plain window close. (Codex P2)
- The EPG source e2e now saves the staged form edit before closing the
  app; leaving it dirty made the new guard block the close, leaking the
  Electron process and timing out the Playwright worker teardown on CI.
  closeElectronApp additionally escalates to SIGKILL when SIGTERM is
  refused, so a future dirty-form slip cannot wedge the suite again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

apps/electron-backend-e2e/src/electron-test-fixtures.ts

@@ -581,6 +581,25 @@ export async function closeElectronApp(
             closePromise.catch(() => undefined),
             electronAppKillWaitMs
         );
+
+        // SIGTERM asks Electron for a graceful quit, which the app can
+        // legitimately refuse — the unsaved-settings close guard cancels the
+        // quit while it waits for an answer. A process that survives here
+        // would outlive the test, hold its data dir, and time out the worker
+        // teardown, so escalate to SIGKILL.
+        if (
+            childProcess.exitCode === null &&
+            childProcess.signalCode === null
+        ) {
+            console.warn(
+                'Electron app survived SIGTERM; escalating to SIGKILL'
+            );
+            childProcess.kill('SIGKILL');
+            await waitForPromiseWithTimeout(
+                closePromise.catch(() => undefined),
+                electronAppKillWaitMs
+            );
+        }
     } catch (error) {
         console.warn('Failed to close Electron app cleanly:', error);
     }

apps/electron-backend-e2e/src/epg.e2e.ts

@@ -110,6 +110,11 @@ test.describe('Electron EPG', () => {
             await expect(app.mainWindow.locator('.epg-source-row')).toHaveCount(
                 0
             );
+            // Adding and removing the source row leaves the settings form
+            // dirty; save so the unsaved-changes close guard has nothing to
+            // ask about when the app closes below — an unanswered dialog
+            // would block the close and leak the Electron process.
+            await saveSettings(app.mainWindow);
 
             await app.mainWindow
                 .getByRole('button', { name: 'Clear EPG data' })

apps/electron-backend/src/app/api/main.preload.ts

@@ -82,6 +82,7 @@ const WINDOW_GET_STATE = 'WINDOW:GET_STATE';
 const WINDOW_STATE_CHANGED = 'WINDOW:STATE_CHANGED';
 const WINDOW_SET_CLOSE_GUARD = 'WINDOW:SET_CLOSE_GUARD';
 const WINDOW_CONFIRM_CLOSE = 'WINDOW:CONFIRM_CLOSE';
+const WINDOW_CANCEL_CLOSE = 'WINDOW:CANCEL_CLOSE';
 const WINDOW_CLOSE_REQUESTED = 'WINDOW:CLOSE_REQUESTED';
 
 const dbSaveContentProgressListeners = new Set<
@@ -424,6 +425,7 @@ const electronApi: ElectronBridgeApi = {
     setWindowCloseGuard: (active: boolean) =>
         ipcRenderer.invoke(WINDOW_SET_CLOSE_GUARD, active),
     confirmWindowClose: () => ipcRenderer.invoke(WINDOW_CONFIRM_CLOSE),
+    cancelWindowClose: () => ipcRenderer.invoke(WINDOW_CANCEL_CLOSE),
     onWindowCloseRequested: (callback: () => void) => {
         const handler = () => callback();
         ipcRenderer.on(WINDOW_CLOSE_REQUESTED, handler);

apps/electron-backend/src/app/services/app-update.service.spec.ts

@@ -82,6 +82,7 @@ function createService(
         isPackaged?: boolean;
         platform?: NodeJS.Platform;
         env?: NodeJS.ProcessEnv;
+        prepareQuit?: () => void;
     } = {}
 ) {
     const updater = new FakeUpdater();
@@ -93,6 +94,7 @@ function createService(
         },
         getMainWindow: () => win,
         platform: overrides.platform ?? 'darwin',
+        prepareQuit: overrides.prepareQuit,
         processEnv: overrides.env ?? {},
         releaseFetcher: overrides.fetcher,
         updater,
@@ -449,4 +451,33 @@ describe('AppUpdateService', () => {
         );
         expect(updater.quitAndInstall).toHaveBeenCalledTimes(1);
     });
+
+    it('stands the close guard down before quitAndInstall', () => {
+        // quitAndInstall closes the windows before 'before-quit' (macOS), so
+        // an armed unsaved-settings guard would intercept that close and
+        // strand the requested install. prepareQuit must run first.
+        const calls: string[] = [];
+        const { service, updater } = createService({
+            prepareQuit: () => calls.push('prepareQuit'),
+        });
+        updater.quitAndInstall.mockImplementation(() => {
+            calls.push('quitAndInstall');
+        });
+        service.handleUpdateAvailable({ version: '0.23.0' });
+        service.handleUpdateDownloaded({ version: '0.23.0' });
+
+        service.installUpdate();
+
+        expect(calls).toEqual(['prepareQuit', 'quitAndInstall']);
+    });
+
+    it('does not stand the close guard down when nothing is installable', () => {
+        const prepareQuit = jest.fn();
+        const { service, updater } = createService({ prepareQuit });
+
+        service.installUpdate();
+
+        expect(prepareQuit).not.toHaveBeenCalled();
+        expect(updater.quitAndInstall).not.toHaveBeenCalled();
+    });
 });

apps/electron-backend/src/app/services/app-update.service.ts

@@ -110,6 +110,13 @@ export interface AppUpdateServiceOptions {
     platform?: NodeJS.Platform;
     processEnv?: NodeJS.ProcessEnv;
     releaseFetcher?: ReleaseFetcher;
+    /**
+     * Runs right before `quitAndInstall()`. Lets the unsaved-settings close
+     * guard stand down for the updater-driven window close, which on macOS
+     * happens before `before-quit` and would otherwise be intercepted as a
+     * plain close — abandoning the requested install.
+     */
+    prepareQuit?: () => void;
 }
 
 function isSelfUpdateSupported(
@@ -401,6 +408,7 @@ export class AppUpdateService {
             this.status.status ===
                 ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded
         ) {
+            this.options.prepareQuit?.();
             this.updater.quitAndInstall();
         }
 

apps/electron-backend/src/app/services/window-close-guard.service.spec.ts

@@ -18,6 +18,7 @@ jest.mock('electron', () => ({
 
 import { ipcMain } from 'electron';
 import {
+    WINDOW_CANCEL_CLOSE,
     WINDOW_CLOSE_REQUESTED,
     WINDOW_CONFIRM_CLOSE,
     WINDOW_SET_CLOSE_GUARD,
@@ -174,12 +175,13 @@ describe('WindowCloseGuard', () => {
         expect(win.close).not.toHaveBeenCalled();
     });
 
-    it('forgets an aborted quit before the next close attempt', () => {
+    it('forgets a cancelled quit before the next close attempt', () => {
         const { app, guard, win } = createArmedGuard();
 
-        // Quit intercepted, user stays: the quit is aborted.
+        // Quit intercepted, user stays: the renderer cancels the request.
         app.fireBeforeQuit();
         win.fireClose();
+        guard.cancelClose();
 
         // A later plain window close must not resurrect the quit.
         win.fireClose();
@@ -189,6 +191,45 @@ describe('WindowCloseGuard', () => {
         expect(app.quit).not.toHaveBeenCalled();
     });
 
+    it('keeps a pending quit when close is clicked again before the user decides', () => {
+        const { app, guard, win } = createArmedGuard();
+
+        // Cmd+Q intercepted; the dialog is open. An impatient second click
+        // on the window's close button must not downgrade the quit — saving
+        // would then close only the window and leave the app running.
+        app.fireBeforeQuit();
+        win.fireClose();
+        win.fireClose();
+
+        guard.confirmClose();
+
+        expect(app.quit).toHaveBeenCalledTimes(1);
+        expect(win.close).not.toHaveBeenCalled();
+    });
+
+    it('escalates a pending close to a quit when the user quits mid-decision', () => {
+        const { app, guard, win } = createArmedGuard();
+
+        win.fireClose();
+        app.fireBeforeQuit();
+        win.fireClose();
+
+        guard.confirmClose();
+
+        expect(app.quit).toHaveBeenCalledTimes(1);
+        expect(win.close).not.toHaveBeenCalled();
+    });
+
+    it('lets exactly one close through after allowNextClose', () => {
+        const { guard, win } = createArmedGuard();
+
+        guard.allowNextClose();
+
+        expect(win.fireClose()).toBe(false);
+        // The bypass is consumed: the guard is back for the next attempt.
+        expect(win.fireClose()).toBe(true);
+    });
+
     it('confirms as a plain close when nothing was intercepted', () => {
         const { app, guard, win } = createArmedGuard();
 
@@ -206,6 +247,22 @@ describe('WindowCloseGuard', () => {
         expect(win.fireClose()).toBe(false);
     });
 
+    it('drops an unanswered quit intent when the renderer navigates away', () => {
+        const { app, guard, win } = createArmedGuard();
+
+        app.fireBeforeQuit();
+        win.fireClose();
+        win.fireWebContentsEvent('did-navigate');
+
+        // A fresh interception cycle must not inherit the stale quit.
+        guard.setGuardActive(true);
+        win.fireClose();
+        guard.confirmClose();
+
+        expect(win.close).toHaveBeenCalledTimes(1);
+        expect(app.quit).not.toHaveBeenCalled();
+    });
+
     it('disarms when the render process is gone', () => {
         const { win } = createArmedGuard();
 
@@ -285,6 +342,8 @@ describe('bootstrapWindowCloseGuard', () => {
         getIpcHandler(WINDOW_SET_CLOSE_GUARD)({}, true);
         expect(win.fireClose()).toBe(true);
 
+        getIpcHandler(WINDOW_CANCEL_CLOSE)({});
+
         getIpcHandler(WINDOW_CONFIRM_CLOSE)({});
         expect(win.close).toHaveBeenCalledTimes(1);
     });

apps/electron-backend/src/app/services/window-close-guard.service.ts

@@ -16,6 +16,7 @@
 
 import { app, ipcMain } from 'electron';
 import {
+    WINDOW_CANCEL_CLOSE,
     WINDOW_CLOSE_REQUESTED,
     WINDOW_CONFIRM_CLOSE,
     WINDOW_SET_CLOSE_GUARD,
@@ -86,12 +87,15 @@ export class WindowCloseGuard {
         // A full navigation (reload included) discards the renderer state the
         // guard was protecting; a gone renderer can no longer answer the
         // close request. Either way an armed guard would make the window
-        // unclosable.
+        // unclosable, and a pending intent from an unanswered request would
+        // leak into the next interception cycle.
         win.webContents.on('did-navigate', () => {
             this.guardActive = false;
+            this.pendingIntent = null;
         });
         win.webContents.on('render-process-gone', () => {
             this.guardActive = false;
+            this.pendingIntent = null;
         });
     }
 
@@ -99,6 +103,17 @@ export class WindowCloseGuard {
         this.guardActive = active;
     }
 
+    /**
+     * Lets exactly one close through even while the guard is armed. Used for
+     * the self-updater's `quitAndInstall()`: on macOS it closes the windows
+     * *before* `before-quit` fires, so an intercepted close would downgrade
+     * the install into a plain window close and abandon the update. The user
+     * explicitly asked to install, so that close must win.
+     */
+    allowNextClose(): void {
+        this.bypassClose = true;
+    }
+
     /**
      * Renderer verdict: safe to leave (settings saved or discarded). Replays
      * the intercepted intent with the guard bypassed for exactly one close.
@@ -121,6 +136,15 @@ export class WindowCloseGuard {
         }
     }
 
+    /**
+     * Renderer verdict: the user stays. Clears the pending intent so a later
+     * close attempt starts fresh — without this, choosing Stay on a quit and
+     * clicking the window's close button minutes later would quit the app.
+     */
+    cancelClose(): void {
+        this.pendingIntent = null;
+    }
+
     private handleClose(
         event: { preventDefault(): void },
         win: CloseGuardWindow
@@ -131,7 +155,12 @@ export class WindowCloseGuard {
         const wasQuit = this.quitInProgress;
         this.quitInProgress = false;
 
-        if (!this.guardActive || this.bypassClose) {
+        if (this.bypassClose) {
+            this.bypassClose = false;
+            return;
+        }
+
+        if (!this.guardActive) {
             return;
         }
 
@@ -140,7 +169,15 @@ export class WindowCloseGuard {
         }
 
         event.preventDefault();
-        this.pendingIntent = wasQuit ? 'quit' : 'close';
+        // A quit is never downgraded while a decision is pending: the
+        // renderer drops repeated requests while its dialog is open, so a
+        // second close click must not overwrite an intercepted Cmd+Q —
+        // saving would then close only the window and leave the app running.
+        if (wasQuit) {
+            this.pendingIntent = 'quit';
+        } else {
+            this.pendingIntent = this.pendingIntent ?? 'close';
+        }
         win.webContents.send(WINDOW_CLOSE_REQUESTED);
     }
 }
@@ -170,6 +207,9 @@ export function bootstrapWindowCloseGuard(
     ipcMain.handle(WINDOW_CONFIRM_CLOSE, () => {
         guard.confirmClose();
     });
+    ipcMain.handle(WINDOW_CANCEL_CLOSE, () => {
+        guard.cancelClose();
+    });
 
     return guard;
 }

apps/electron-backend/src/main.ts

@@ -135,20 +135,24 @@ export default class Main {
             trace('startup', 'bootstrap-events:start');
         }
 
+        const windowCloseGuard = bootstrapWindowCloseGuard((listener) =>
+            App.onMainWindowCreated(listener)
+        );
         const appUpdateService = new AppUpdateService({
             app,
             appVersion: environment.version,
             getMainWindow: () => App.mainWindow,
             updater: () => autoUpdater,
+            // quitAndInstall() closes the windows before 'before-quit' fires
+            // (macOS), so without this an armed close guard would intercept
+            // the install's window close and strand the update.
+            prepareQuit: () => windowCloseGuard.allowNextClose(),
         });
         AppUpdateEvents.bootstrapAppUpdateEvents(appUpdateService);
 
         registerStaticHeaderShims();
         ElectronEvents.bootstrapElectronEvents();
         WindowEvents.bootstrapWindowEvents();
-        bootstrapWindowCloseGuard((listener) =>
-            App.onMainWindowCreated(listener)
-        );
         EmbeddedMpvEvents.bootstrapEmbeddedMpvEvents();
         PlaylistEvents.bootstrapPlaylistEvents();
         PlaylistOpenEvents.bootstrapPlaylistOpenEvents();
@@ -264,7 +268,13 @@ runEmbeddedMpvRuntimeDiagnosticOrContinue(process.argv, () => {
         await Main.bootstrapAppEvents();
     });
 
-    app.on('before-quit', () => {
+    // 'will-quit', not 'before-quit': the unsaved-settings close guard can
+    // cancel a quit mid-flight by preventing the window close, and
+    // 'before-quit' has already fired by then — tearing sessions and the DB
+    // worker down there would leave a cancelled quit with the app open but
+    // playback and database work destroyed. 'will-quit' only fires once
+    // every window close was allowed through.
+    app.on('will-quit', () => {
         shutdownEmbeddedMpv();
         shutdownMpvSession();
         shutdownVlcSession();

apps/web/src/app/settings/settings-unload-guard.service.spec.ts

@@ -23,6 +23,7 @@ describe('SettingsUnloadGuardService', () => {
     let electronStub: {
         setWindowCloseGuard: jest.Mock;
         confirmWindowClose: jest.Mock;
+        cancelWindowClose: jest.Mock;
         onWindowCloseRequested: jest.Mock;
     };
 
@@ -47,6 +48,7 @@ describe('SettingsUnloadGuardService', () => {
         electronStub = {
             setWindowCloseGuard: jest.fn().mockResolvedValue(undefined),
             confirmWindowClose: jest.fn().mockResolvedValue(undefined),
+            cancelWindowClose: jest.fn().mockResolvedValue(undefined),
             onWindowCloseRequested: jest.fn((callback: () => void) => {
                 closeRequestCallback = callback;
                 return unsubscribeCloseRequests;
@@ -174,6 +176,10 @@ describe('SettingsUnloadGuardService', () => {
             await flushAsyncWork();
 
             expect(electronStub.confirmWindowClose).not.toHaveBeenCalled();
+            // Staying abandons the request in the main process too, so the
+            // remembered close-vs-quit intent cannot leak into a later
+            // attempt.
+            expect(electronStub.cancelWindowClose).toHaveBeenCalledTimes(1);
         });
 
         it('ignores repeated close requests while a dialog is open', async () => {
@@ -228,6 +234,9 @@ describe('SettingsUnloadGuardService', () => {
             await flushAsyncWork();
 
             expect(reloadPage).not.toHaveBeenCalled();
+            // A reload was never remembered by the main process, so there
+            // is nothing to cancel there.
+            expect(electronStub.cancelWindowClose).not.toHaveBeenCalled();
         });
     });
 });

apps/web/src/app/settings/settings-unload-guard.service.ts

@@ -119,6 +119,12 @@ export class SettingsUnloadGuardService implements OnDestroy {
 
         try {
             if (!(await host.confirmClose())) {
+                if (intent === 'close') {
+                    // Staying must clear the intent the main process
+                    // remembered, or a later close attempt would replay a
+                    // stale quit.
+                    void window.electron?.cancelWindowClose?.();
+                }
                 return;
             }
 

apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts

@@ -244,6 +244,7 @@ export function createElectronStub(): typeof window.electron {
         onWindowCloseRequested: jest.fn(() => jest.fn()),
         setWindowCloseGuard: jest.fn().mockResolvedValue(undefined),
         confirmWindowClose: jest.fn().mockResolvedValue(undefined),
+        cancelWindowClose: jest.fn().mockResolvedValue(undefined),
         openInMpv: jest.fn(),
         openInVlc: jest.fn(),
         platform: 'linux',

libs/shared/interfaces/src/lib/electron-api.interface.ts

@@ -629,6 +629,11 @@ export interface ElectronBridgeApi {
      * original intent (window close or app quit) with the guard bypassed.
      */
     confirmWindowClose: () => Promise<void>;
+    /**
+     * Abandons a close the guard intercepted (the user stays), so the
+     * remembered close-vs-quit intent cannot leak into a later attempt.
+     */
+    cancelWindowClose: () => Promise<void>;
     onWindowCloseRequested: (callback: () => void) => () => void;
     fetchPlaylistByUrl: (
         url: string,

libs/shared/interfaces/src/lib/ipc-commands.ts

@@ -118,4 +118,5 @@ export const WINDOW_STATE_CHANGED = 'WINDOW:STATE_CHANGED';
 // main process and handed to the renderer for a save/discard/stay decision
 export const WINDOW_SET_CLOSE_GUARD = 'WINDOW:SET_CLOSE_GUARD';
 export const WINDOW_CONFIRM_CLOSE = 'WINDOW:CONFIRM_CLOSE';
+export const WINDOW_CANCEL_CLOSE = 'WINDOW:CANCEL_CLOSE';
 export const WINDOW_CLOSE_REQUESTED = 'WINDOW:CLOSE_REQUESTED';