Close two holes left by the update-install bypass

CLAUDE.md

@@ -181,10 +181,20 @@ an app that refuses to exit, which on Windows ends with the installer killing it
 lost anyway, and the prompt asking about it was on screen for a second. So the updater IPC handler
 calls `allowNextClose()` before it installs, and `rearmCloseGuard()` if nothing was launched
 (`quitAndInstall` returns whether it committed to quitting, which the macOS "no downloaded file"
-path does not). The question is asked one layer up instead, in `update-notification.tsx`, in front
-of the install rather than behind it. `confirmDiscard` is held in a ref there: it closes over the
-app state, so it is a new function on every broadcast from main, and naming it as a dependency
-would re-run the update-status effect several times a second during an interview.
+path does not).
+
+**A failed install reports nothing - it simply does not quit** - so `rearmCloseGuardIfStillRunning()`
+puts the guard back unless `before-quit` has fired by then. Without it one failed update disarms
+the guard for the rest of the session and the next close takes the interview with it, which is
+precisely what the guard exists to stop. The test is `quitting` rather than a window count,
+because re-arming a quit that *is* under way turns the guard into a veto of the close it just
+approved.
+
+The question is asked one layer up, in `update-notification.tsx`, in front of the install rather
+than behind it. It reads `hasHistory` over `appState.get()` on the click instead of through
+`useSaveHistoryGuard`: that hook subscribes to the app state, which during an interview is a new
+object several times a second, and this component would then re-render and re-arm its status
+effect on every ASR partial to answer a question it only asks when a button is pressed.
 
 ### Interview language
 

src/main/ipc/auto-updater.ts

@@ -1,7 +1,11 @@
 import { ipcMain } from 'electron';
 
 import { autoUpdaterService } from '../services/auto-updater.service.js';
-import { allowNextClose, rearmCloseGuard } from '../window-close-guard.js';
+import {
+  allowNextClose,
+  rearmCloseGuard,
+  rearmCloseGuardIfStillRunning,
+} from '../window-close-guard.js';
 
 export function registerAutoUpdaterHandlers(): void {
   ipcMain.handle('auto-updater:check-for-updates', async () => {
@@ -25,9 +29,16 @@ export function registerAutoUpdaterHandlers(): void {
     allowNextClose();
     try {
       const quitting = await autoUpdaterService.quitAndInstall();
-      // Nothing was launched, so the app is staying and the guard goes back on.
-      if (!quitting) rearmCloseGuard();
-      return { success: quitting };
+      if (!quitting) {
+        // Nothing was launched, so the app is staying and the guard goes back on.
+        rearmCloseGuard();
+        return { success: false };
+      }
+
+      // The install was started, but electron-updater says nothing when one fails - it just
+      // does not quit. So the guard is put back unless the quit really begins.
+      rearmCloseGuardIfStillRunning();
+      return { success: true };
     } catch (error) {
       console.error('[IPC] Failed to quit and install:', error);
       rearmCloseGuard();

src/main/window-close-guard.ts

@@ -76,6 +76,27 @@ export function rearmCloseGuard(): void {
   closeConfirmed = false;
 }
 
+// Long enough for `app.quit()` to have emitted `before-quit`, which happens in the tick after
+// the installer is spawned. Nothing waits on this, so being generous costs nothing.
+const QUIT_GRACE_MS = 5_000;
+
+/**
+ * Re-arm unless the quit actually started.
+ *
+ * `autoUpdater.quitAndInstall()` reports nothing back when the install fails - it simply does
+ * not quit - so this is the only way to notice that the app is still here. Left unchecked, one
+ * failed update would disarm the guard for the rest of the session and the next close would
+ * take the interview with it, silently, which is the whole thing this file exists to stop.
+ *
+ * `quitting` is the test rather than a window count, because re-arming a quit that *is* under
+ * way would turn the guard into a veto of the close it had just approved.
+ */
+export function rearmCloseGuardIfStillRunning(): void {
+  setTimeout(() => {
+    if (!quitting) closeConfirmed = false;
+  }, QUIT_GRACE_MS).unref();
+}
+
 export function registerCloseGuardHandlers(): void {
   app.on('before-quit', () => {
     quitting = true;

src/renderer/components/custom/update-notification.tsx

@@ -1,23 +1,37 @@
-import { useEffect, useRef } from 'react';
+import { useCallback, useEffect, useRef } from 'react';
 import { toast } from 'sonner';
 
 import { UpdateStatus, useAutoUpdater } from '@/hooks/use-auto-updater';
-import { useSaveHistoryGuard } from '@/hooks/use-save-history-guard';
+import { useSaveHistoryPrompt } from '@/hooks/use-save-history-guard';
+import { getElectron } from '@/lib/utils';
 
 export function UpdateNotification() {
   const { updateStatus, quitAndInstall } = useAutoUpdater();
-  const { confirmDiscard } = useSaveHistoryGuard();
   const lastStatusRef = useRef<UpdateStatus | null>(null);
   const downloadToastIdRef = useRef<string | number | null>(null);
 
-  // Held in a ref rather than listed as a dependency. `confirmDiscard` closes over the current
-  // app state, so it is a new function on every broadcast from main - several a second during
-  // an interview - and naming it in the effect below would re-run the whole status machine
-  // that often.
-  const confirmDiscardRef = useRef(confirmDiscard);
-  useEffect(() => {
-    confirmDiscardRef.current = confirmDiscard;
-  });
+  /**
+   * Installing takes the app down, so it loses the interview exactly as closing does - and
+   * unlike a close it cannot be vetoed once started, because the installer is launched before
+   * the quit is requested. So the question is asked here, in front of it.
+   *
+   * `hasHistory` is read from main on the click rather than through `useSaveHistoryGuard`.
+   * That hook subscribes to the app state, which during an interview is a new object several
+   * times a second, and this component would then re-render - and re-arm its status effect -
+   * on every ASR partial to answer a question it only asks when a button is pressed. One round
+   * trip on the click also reads the flag where it is derived rather than a broadcast behind.
+   */
+  const confirmThenInstall = useCallback(async () => {
+    const electron = getElectron();
+    const state = await electron?.appState.get();
+
+    if (state?.hasHistory) {
+      const proceed = await useSaveHistoryPrompt.getState().prompt('update');
+      if (!proceed) return;
+    }
+
+    await quitAndInstall();
+  }, [quitAndInstall]);
 
   useEffect(() => {
     if (!updateStatus) return;
@@ -76,14 +90,7 @@ export function UpdateNotification() {
             duration: Infinity,
             action: {
               label: isMac ? 'Open Installer' : 'Restart Now',
-              // Installing takes the app down, so it loses the interview exactly as closing
-              // does - and the install cannot be vetoed once started, because the installer is
-              // launched before the quit. So the question is asked here, in front of it.
-              onClick: () => {
-                void confirmDiscardRef.current('update').then((proceed) => {
-                  if (proceed) void quitAndInstall();
-                });
-              },
+              onClick: () => void confirmThenInstall(),
             },
           });
         }
@@ -97,7 +104,7 @@ export function UpdateNotification() {
         console.error('[UpdateNotification] Update error:', error);
         break;
     }
-  }, [updateStatus, quitAndInstall]);
+  }, [updateStatus, confirmThenInstall]);
 
   return null;
 }

test/save-history.test.mjs

@@ -7,7 +7,7 @@
  * writes rather than the array lengths, and that an export refuses the placeholder - which the
  * length check let through, producing a billed summary of "Transcripts will be here".
  */
-import { createChecker, loadMain, readSource } from './helpers.mjs';
+import { codeOnly, createChecker, loadMain, readSource } from './helpers.mjs';
 
 export async function run() {
   const { check, failures } = createChecker('save-history');
@@ -134,21 +134,33 @@ export async function run() {
   );
   check(
     'and re-arms it when nothing was launched',
-    updaterIpc.includes('if (!quitting) rearmCloseGuard();') &&
-      updaterIpc.includes('catch') &&
-      updaterIpc.split('rearmCloseGuard()').length === 3
+    updaterIpc.includes('rearmCloseGuard();') && updaterIpc.includes('return { success: false };')
+  );
+  // electron-updater says nothing when an install fails - it simply does not quit - so without
+  // this one failed update disarms the guard for the rest of the session and the next close
+  // takes the interview with it.
+  check(
+    'and re-arms it when the install was started but never quit',
+    updaterIpc.includes('rearmCloseGuardIfStillRunning();')
+  );
+  check(
+    'the delayed re-arm stands down for a quit that did start',
+    guard.includes('if (!quitting) closeConfirmed = false;')
   );
 
   const updateToast = readSource(
     new URL('../src/renderer/components/custom/update-notification.tsx', import.meta.url)
   );
   check(
     'the install asks about an unsaved interview first',
-    updateToast.includes("confirmDiscardRef.current('update')")
+    updateToast.includes("useSaveHistoryPrompt.getState().prompt('update')")
   );
+  check('and only installs when the answer is yes', updateToast.includes('if (!proceed) return;'));
+  // Reading it off the subscribed app state would re-render this component - and re-arm its
+  // status effect - on every ASR partial, to answer a question it only asks on a click.
   check(
-    'and only installs when the answer is yes',
-    updateToast.includes('if (proceed) void quitAndInstall();')
+    'and does not subscribe to the app state to find that out',
+    !codeOnly(updateToast).includes('useSaveHistoryGuard') && updateToast.includes('appState.get()')
   );
 
   return failures;