(window): close a window whose page stopped answering

.ai/contexts/viewer-panel.md

@@ -165,7 +165,7 @@ Each of these reads and clears `pendingTouchedOpen` before `destroyCurrentTab`,
 Nothing ends the window with unsaved file-panel edits without the user saying so. The scope is the Touched editor in the slot (a tab with an `absolutePath`) and every `touchedStashes` entry, in every session of `filePanelState`; the Memory and Work Files panels, MCP diff tabs and Changes buffers are not covered.
 
 - **Main** (`unsaved-guard.js`, one guard, `attach(win)` per window, `beforeQuit(event, win)` for the app). `main.js`'s `before-quit` handler calls `beforeQuit` first: it prevents the quit, asks the renderer, and calls `app.quit()` again on a yes, so the cleanup that follows (PTYs killed, MCP servers, watchers) runs only once the quit is confirmed and a Cancel leaves the app intact. This covers every `app.quit()` caller (☰ Quit, the last window closing). `updater-install` asks first (`confirmQuit`), because electron-updater's `quitAndInstall()` starts the installer before it calls `app.quit()`: a Cancel must leave the installer unstarted, and a yes pre-approves the quit that follows. A Windows `session-end` approves the quit, so logoff and shutdown never wait on the dialog; `query-session-end` approves it for 60 s (`LOGOFF_CANCEL_MS`), so a logoff that is cancelled does not skip the question on every later quit. A window close, a quit and an install share one question while it is open; one that joins a `'reload'` question sends `unsaved-check-reason` (`id`, `'quit'`), and the renderer acts on the reason it holds when the user answers. The window's own `close` event is held the same way for a plain window close. The question is `unsaved-check` (`id`, `'quit'` or `'reload'`); the renderer's `unsaved-check-result` (`id`, `proceed`) approves it. A `will-prevent-unload` (the renderer's `beforeunload` veto, which a reload hits) asks with `'reload'`; on a yes the next unload is allowed once (`allowNextUnload`, answered with `preventDefault()`, which in Electron means "unload anyway") and `webContents.reload()` is called. Every close asks the renderer, even with nothing dirty; a main-side dirty flag would save that round trip and is not built.
-- **Bounded.** The renderer acknowledges (`unsaved-check-ack`) as soon as it receives the check, before any dialog. The 2.5 s bound (`DEFAULT_TIMEOUT_MS`) covers only send to ack: no ack answers yes, so a hung renderer never keeps the app from quitting. After the ack the guard waits for the answer without a limit, so a slow user or a slow save loses nothing, and answers yes only if the renderer process is gone (`render-process-gone`, `destroyed`) or a send fails. A late answer is ignored.
+- **Bounded.** The renderer acknowledges (`unsaved-check-ack`) as soon as it receives the check, before any dialog. The 2.5 s bound (`DEFAULT_TIMEOUT_MS`) covers only send to ack: no ack answers yes, so a hung renderer never keeps the app from quitting. The window is then closed the usual way (`win.close()`, or `app.quit()` for a quit or an update install), so a page that is only slow (still starting, or busy) closes as before, without the question, once it runs its `beforeunload`. A hung page never runs it, and that close would never finish: the guard destroys such a window when Electron reports it `unresponsive`, or when it is closed again, the guard's own `close()` or `app.quit()` aside. Electron reports a page `unresponsive` about 5 s after a close it has not completed, and cannot tell a hung page from a busy one: a page still busy about 5 s after that close, about 7.5 s after the click counting the 2.5 s bound, is destroyed, and its pending work is lost (the debounced working-set save among it). Only a page that recovers before then closes normally; a stall of 8–10 s that starts before the click (#526) does not. `e2e/hung-window.spec.js` measures it: the app exits about 7.5 s after the close it schedules. A page reported unresponsive earlier, and neither `responsive` nor acknowledging a check since, is destroyed when that close reaches it: right after `win.close()` for a window close, and for a quit or an update install only once the quit closes the window (its `close` event), so a quit still held by another `before-quit` handler (the ActivityWatch flush) is not cut short. A check the page acknowledged never destroys the window, however long the user takes. After the ack the guard waits for the answer without a limit, so a slow user or a slow save loses nothing, and answers yes only if the renderer process is gone (`render-process-gone`, `destroyed`) or a send fails. A late answer is ignored.
 - **Renderer** (`file-panel.js`). `askAboutUnsavedEdits()` lists the dirty tabs (`collectUnsavedFileTabs`) in the `#unsaved-edits-dialog` dialog, built on the add-project dialog's classes (already in the frameless no-drag list). Save writes every dirty entry: the editor in the slot through `handleChangesSave`, a stashed one through `saveFileForPanel(filePath, content, savedContent, {git, version})`, removed from its session once written. A save that fails (including a disk that moved) keeps the dialog open with the reason, and nothing is answered. Discard answers yes. Cancel and Escape answer no. A second request while the dialog is open gets the same dialog's answer.
 - **`beforeunload`.** The renderer vetoes an unload while an entry is dirty, unless `unloadApproved`, set for 10 s after the user answered yes. That is what stops a reload from the keyboard or devtools from slipping past, and what keeps the veto from asking a second time after an approved close.
 - **Closing a session** does not drop its file panel: `destroySession` leaves `filePanelState` alone, so the tab and the stashes, dirty or not, are still there when the session is opened again, and the quit check still sees them. Deleting a session leaves its state in memory too, so its edits are asked about at quit rather than lost.

CHANGELOG.md

@@ -9,6 +9,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc
 - Switching to a hidden terminal tab no longer disposes and recreates its graphics context every time, which blocked the window for several seconds. The three most recently shown terminals keep theirs, and a context that has to be recreated is made once the tab is shown. (#526)
 - In a sandboxed session, a repository or a `.claude` folder below an Additional Directory that is a symbolic link now has its Git config, hooks and Claude settings read-only, like one below a plain directory. Before, they were writable from inside the sandbox. Such a directory holding a `.claude` that is itself a symbolic link, or a repository Git cannot read, now refuses to start the session (status 125), as a plain directory already did, instead of starting it unprotected. (#538)
 - A double click on a session in the sidebar opens it once, instead of sometimes starting two terminals that resume the same conversation. (#539)
+- Closing the window or quitting no longer leaves Switchboard running when its window has stopped responding: the window closes as before, and once the system reports it not responding, about 5 seconds after the close, or once you close it again, it is closed anyway. A window still busy at that point is closed too, and its last unsaved changes to the list of open sessions are lost; one that recovers sooner closes normally. (#540)
 
 ## v0.0.92 — 2026-10-10
 

docs/e2e.md

@@ -49,6 +49,7 @@ here.
 | The shell opened with no tab fills the panel and is not a sidebar row | `panel.spec.js` | the `.shell-only` layout, and `buildProjectsFromCache` skipping the panel shell |
 | Changes on a project with no git work tree says so, with no git output | `panel.spec.js` | the not-a-repository note instead of git's raw output |
 | Refresh beside Stop redraws a plain terminal and restores its fitted size | `terminal-refresh.spec.js` | visible header geometry, one refresh IPC, actual PTY size events, and final fitted size |
+| Closing a window whose page hangs still ends the app | `hung-window.spec.js` | the 2.5 s bound on an unacknowledged check, the window destroyed once Electron reports the page unresponsive, and exit code 0 |
 
 The issue's fifth journey expected the Changes control to disappear. #310 made
 it unconditional, so the journey checks what the panel says instead (see
@@ -94,7 +95,9 @@ the OS temporary directory locally, outside the app directory.
     `taskkill /T /F` on that pid, which kills the process and its descendants.
 
   Then it deletes the temporary `HOME`. If the app exits within the 10 s, no
-  kill is sent.
+  kill is sent. A journey may end the app itself (`hung-window.spec.js` does);
+  the fixture then skips the trace, the screenshot and the close, and only
+  deletes the temporary `HOME`.
 
 ## Proving a journey can fail
 

e2e/fixtures.js

@@ -102,9 +102,8 @@ function killTree(proc) {
   }
 }
 
-async function closeApp(app) {
-  const proc = app.process();
-  await bounded(app.close(), CLOSE_TIMEOUT_MS);
+async function closeApp(app, proc) {
+  if (!hasExited(proc)) await bounded(Promise.resolve().then(() => app.close()), CLOSE_TIMEOUT_MS);
   if (await waitForExit(proc, CLOSE_TIMEOUT_MS)) return;
   killTree(proc);
   await waitForExit(proc, CLOSE_TIMEOUT_MS);
@@ -126,6 +125,7 @@ const test = base.extend({
 
   launch: async ({ env }, use, testInfo) => {
     let app = null;
+    let proc = null;
     let page = null;
     const launch = async () => {
       app = await electron.launch({
@@ -134,6 +134,7 @@ const test = base.extend({
         cwd: APP_DIR,
         env: { ...env, ELECTRON_IS_DEV: '0' },
       });
+      proc = app.process();
       await app.context().tracing.start({ screenshots: true, snapshots: true });
       page = await app.firstWindow();
       await page.locator('.project-new-btn').first().waitFor();
@@ -145,10 +146,12 @@ const test = base.extend({
       if (app) {
         const failed = testInfo.status !== testInfo.expectedStatus;
         try {
-          if (failed && page) await bounded(page.screenshot({ path: testInfo.outputPath('failure.png'), timeout: CLOSE_TIMEOUT_MS }), CLOSE_TIMEOUT_MS);
-          await bounded(app.context().tracing.stop(failed ? { path: testInfo.outputPath('trace.zip') } : undefined), CLOSE_TIMEOUT_MS * 3);
+          if (!hasExited(proc)) {
+            if (failed && page) await bounded(page.screenshot({ path: testInfo.outputPath('failure.png'), timeout: CLOSE_TIMEOUT_MS }), CLOSE_TIMEOUT_MS);
+            await bounded(app.context().tracing.stop(failed ? { path: testInfo.outputPath('trace.zip') } : undefined), CLOSE_TIMEOUT_MS * 3);
+          }
         } finally {
-          await closeApp(app);
+          await closeApp(app, proc);
         }
       }
     }

e2e/hung-window.spec.js

@@ -0,0 +1,17 @@
+'use strict';
+
+const { test, expect, makeRepo } = require('./fixtures');
+
+const EXIT_TIMEOUT_MS = 60_000;
+
+test('closing a window whose page hangs still ends the app', async ({ home, env, launch }) => {
+  test.setTimeout(150_000);
+  makeRepo(home, env, { 'readme.txt': 'fixture\n' });
+  const { app, page } = await launch();
+  const proc = app.process();
+  await app.evaluate(({ BrowserWindow }) => { setTimeout(() => BrowserWindow.getAllWindows()[0].close(), 2000); });
+  await page.evaluate(() => { setTimeout(() => { for (;;) Date.now(); }, 500); });
+  await expect.poll(() => proc.exitCode !== null || proc.signalCode !== null, { timeout: EXIT_TIMEOUT_MS }).toBe(true);
+  expect(proc.signalCode).toBe(null);
+  expect(proc.exitCode).toBe(0);
+});

test/unsaved-guard.test.js

@@ -9,7 +9,7 @@ const { EventEmitter } = require('node:events');
 
 const { createUnsavedGuard, LOGOFF_CANCEL_MS } = require('../unsaved-guard');
 
-function setup({ timeoutMs = 1000, quit } = {}) {
+function setup({ timeoutMs = 1000, quit, closeFinishes = false } = {}) {
   const ipcMain = new EventEmitter();
   const timers = [];
   const setTimeoutFn = (fn, ms) => { const t = { fn, ms, cleared: false }; timers.push(t); return t; };
@@ -24,7 +24,15 @@ function setup({ timeoutMs = 1000, quit } = {}) {
     reloads: 0,
   });
   const win = new EventEmitter();
-  Object.assign(win, { webContents: wc, isDestroyed: () => false, close: () => { win.closes += 1; }, closes: 0 });
+  Object.assign(win, {
+    webContents: wc,
+    isDestroyed: () => win.destroys > 0 || win.closed,
+    close: () => { win.closes += 1; if (closeFinishes) win.closed = true; },
+    destroy: () => { win.destroys += 1; },
+    closes: 0,
+    destroys: 0,
+    closed: false,
+  });
   const guard = createUnsavedGuard({ ipcMain, timeoutMs, setTimeoutFn, clearTimeoutFn, quit });
   guard.attach(win);
 
@@ -299,3 +307,133 @@ test('a logoff that never ends the session stops approving quits after a while;
   assert.ok(s.timers.filter((x) => x.ms === LOGOFF_CANCEL_MS).every((x) => x.cleared));
   assert.equal(s.guard.beforeQuit(s.closeEvent(), s.win), false);
 });
+
+async function closeUnanswered(t) {
+  t.win.emit('close', t.closeEvent());
+  t.timers.filter((x) => x.ms === 2500 && !x.cleared).pop().fn();
+  await tick();
+}
+
+test('a page that never acknowledges is closed the usual way, and destroyed once reported unresponsive', async () => {
+  const t = setup({ timeoutMs: 2500 });
+  await closeUnanswered(t);
+  assert.equal(t.win.closes, 1, 'a page that is only slow still closes the usual way');
+  assert.equal(t.win.destroys, 0);
+  assert.equal(t.timers.filter((x) => !x.cleared && x.ms !== 2500).length, 0, 'no fixed delay destroys a busy page');
+  t.win.emit('unresponsive');
+  assert.equal(t.win.destroys, 1, 'a hung page never runs its beforeunload, so its close would never finish');
+});
+
+test('a busy page that answers again is not destroyed', async () => {
+  const t = setup({ timeoutMs: 2500, closeFinishes: true });
+  await closeUnanswered(t);
+  t.win.emit('responsive');
+  t.win.emit('unresponsive');
+  assert.equal(t.win.destroys, 0, 'the close finished, nothing left to force');
+});
+
+test('a page already reported unresponsive is destroyed when its unanswered close does not finish', async () => {
+  const t = setup({ timeoutMs: 2500 });
+  t.win.emit('unresponsive');
+  assert.equal(t.win.destroys, 0, 'unresponsive alone never destroys');
+  await closeUnanswered(t);
+  assert.equal(t.win.destroys, 1);
+
+  const recovered = setup({ timeoutMs: 2500 });
+  recovered.win.emit('unresponsive');
+  recovered.win.emit('responsive');
+  await closeUnanswered(recovered);
+  assert.equal(recovered.win.destroys, 0, 'a page that answered again is given its close');
+});
+
+test('closing again a window whose unanswered close did not finish destroys it', async () => {
+  const t = setup({ timeoutMs: 2500 });
+  await closeUnanswered(t);
+  const ours = t.closeEvent();
+  t.win.emit('close', ours);
+  assert.equal(t.win.destroys, 0, 'the close the guard itself started is not a second one');
+  const again = t.closeEvent();
+  t.win.emit('close', again);
+  assert.equal(t.win.destroys, 1);
+});
+
+test('a quit the page never acknowledges goes ahead, and the window is destroyed once reported unresponsive', async () => {
+  let quits = 0;
+  const t = setup({ timeoutMs: 2500, quit: () => { quits += 1; } });
+  assert.equal(t.guard.beforeQuit(t.closeEvent(), t.win), true);
+  t.timers.find((x) => x.ms === 2500).fn();
+  await tick();
+  assert.equal(quits, 1);
+  assert.equal(t.win.destroys, 0);
+  t.win.emit('close', t.closeEvent());
+  assert.equal(t.win.destroys, 0, 'app.quit() closes the window the usual way');
+  t.win.emit('unresponsive');
+  assert.equal(t.win.destroys, 1, 'a hung page holds that close');
+});
+
+test('an answered check never destroys the window, even while the user takes time to answer', async () => {
+  const t = setup({ timeoutMs: 2500 });
+  t.win.emit('close', t.closeEvent());
+  t.ack(t.sent[0].args[0]);
+  t.win.emit('unresponsive');
+  t.answer(t.sent[0].args[0], true);
+  await tick();
+  t.win.emit('unresponsive');
+  t.win.emit('close', t.closeEvent());
+  t.win.emit('close', t.closeEvent());
+  assert.equal(t.win.closes, 1);
+  assert.equal(t.win.destroys, 0);
+});
+
+test('a page that acknowledges a check is no longer counted unresponsive', async () => {
+  const t = setup({ timeoutMs: 2500 });
+  t.win.emit('unresponsive');
+  t.win.emit('close', t.closeEvent());
+  t.ack(t.sent[0].args[0]);
+  t.answer(t.sent[0].args[0], false);
+  await tick();
+  await closeUnanswered(t);
+  assert.equal(t.win.closes, 1);
+  assert.equal(t.win.destroys, 0, 'a busy page is given its close');
+});
+
+test('an updater install the page never acknowledges is approved, and the window destroyed once reported unresponsive', async () => {
+  const t = setup({ timeoutMs: 2500 });
+  const confirmed = t.guard.confirmQuit(t.win);
+  t.timers.find((x) => x.ms === 2500).fn();
+  assert.equal(await confirmed, true);
+  assert.equal(t.win.destroys, 0, 'quitAndInstall still closes the window the usual way');
+  t.win.emit('unresponsive');
+  assert.equal(t.win.destroys, 0, 'not before the quit reaches the window');
+  t.win.emit('close', t.closeEvent());
+  assert.equal(t.win.destroys, 1);
+  assert.equal(t.guard.beforeQuit(t.closeEvent(), t.win), false, 'the quit that follows is approved');
+});
+
+test('a quit held by another before-quit handler does not destroy the window before it closes it', async () => {
+  let quits = 0;
+  const t = setup({ timeoutMs: 2500, quit: () => { quits += 1; } });
+  t.win.emit('unresponsive');
+  assert.equal(t.guard.beforeQuit(t.closeEvent(), t.win), true);
+  t.timers.find((x) => x.ms === 2500).fn();
+  await tick();
+  assert.equal(quits, 1);
+  assert.equal(t.win.destroys, 0, 'the quit may still be held, by the ActivityWatch flush');
+  t.win.emit('unresponsive');
+  assert.equal(t.win.destroys, 0);
+  const e = t.closeEvent();
+  t.win.emit('close', e);
+  assert.equal(e.prevented, true);
+  assert.equal(t.win.destroys, 1, 'the quit closing a window already reported unresponsive destroys it');
+
+  const recovered = setup({ timeoutMs: 2500, quit: () => {} });
+  recovered.win.emit('unresponsive');
+  recovered.guard.beforeQuit(recovered.closeEvent(), recovered.win);
+  recovered.timers.find((x) => x.ms === 2500).fn();
+  await tick();
+  recovered.win.emit('responsive');
+  recovered.win.emit('close', recovered.closeEvent());
+  assert.equal(recovered.win.destroys, 0, 'a page that answers again is given its close');
+  recovered.win.emit('unresponsive');
+  assert.equal(recovered.win.destroys, 1);
+});