From 26d53d82b2d1451c4ab755023a2c9c1e19fd6f9a Mon Sep 17 00:00:00 2001 From: abate Date: Sun, 11 Oct 2026 16:06:09 +0200 Subject: [PATCH 1/5] test(restore): a quit joining an answered reload waits for the page's late save When the renderer answered the reload before the upgrade reached it, its save runs after main already went on to quit, and the window can close before the write lands. Main should wait for the page to report the save done, bounded, and the page should report it. --- test/dom-file-panel-unsaved-guard.test.js | 40 +++++++++++++++++- test/unsaved-guard.test.js | 49 ++++++++++++++++++++++- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index e1ad0d09f..398b4419b 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -43,7 +43,7 @@ function setup() { const dom = new JSDOM(INDEX_HTML, { url: 'http://localhost/', runScripts: 'outside-only', pretendToBeVisual: true }); const { window } = dom; const disk = new Map(); - const calls = { openFile: null, check: null, acks: [], answers: [], saves: [], confirms: [] }; + const calls = { openFile: null, check: null, acks: [], answers: [], saves: [], confirms: [], reasonDone: [] }; let editor = null; window.api = new Proxy({ @@ -52,6 +52,7 @@ function setup() { onUnsavedCheckReason: (cb) => { calls.reason = cb; }, unsavedCheckAck: (id) => { calls.acks.push({ id, at: calls.answers.length, dialog: !!window.document.getElementById('unsaved-edits-dialog') }); }, unsavedCheckResult: (id, proceed) => { calls.answers.push({ id, proceed }); }, + unsavedCheckReasonDone: (id) => { calls.reasonDone.push(id); }, watchFile: () => Promise.resolve({ ok: true }), unwatchFile: () => Promise.resolve({ ok: true }), readFileForPanel: (p) => Promise.resolve(disk.has(p) ? { ok: true, git: false, original: disk.get(p), current: disk.get(p) } : { ok: false, code: 'ENOENT', error: 'ENOENT' }), @@ -313,6 +314,43 @@ test('a quit that joins a reload check the page has already answered still write } finally { ctx.destroy(); } }); +test('the page tells main when the save a late upgrade asked for is done, and at once when it has none to do', async () => { + const ctx = setup(); + try { + let finish = null; + let fail = false; + ctx.window.flushStateForExit = () => new Promise((resolve, reject) => { finish = () => (fail ? reject(new Error('disk')) : resolve()); }); + ctx.calls.check(6, 'reload'); + await flush(); + ctx.calls.reason(6, 'quit'); + await flush(); + assert.deepEqual(ctx.calls.reasonDone, [], 'not before the save lands'); + finish(); + await flush(); + assert.deepEqual(ctx.calls.reasonDone, [6]); + + ctx.calls.check(7, 'reload'); + await flush(); + fail = true; + ctx.calls.reason(7, 'quit'); + await flush(); + finish(); + await flush(); + assert.deepEqual(ctx.calls.reasonDone, [6, 7], 'a failed save still lets the quit go'); + + ctx.calls.reason(99, 'quit'); + await flush(); + assert.deepEqual(ctx.calls.reasonDone, [6, 7, 99], 'an unknown check has nothing to wait for'); + + await dirtyTab(ctx, 's1', A); + ctx.calls.check(8, 'reload'); + await flush(); + ctx.calls.reason(8, 'quit'); + await flush(); + assert.deepEqual(ctx.calls.reasonDone, [6, 7, 99, 8], 'a check still asking saves before it answers, so main need not wait'); + } finally { ctx.destroy(); } +}); + test('a cancelled close does not flush', async () => { const ctx = setup(); try { diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index fbf65a17b..e27c1cf21 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -39,7 +39,8 @@ function setup({ timeoutMs = 1000, quit, closeFinishes = false } = {}) { const closeEvent = () => ({ prevented: false, preventDefault() { this.prevented = true; } }); const answer = (id, proceed) => ipcMain.emit('unsaved-check-result', {}, id, proceed); const ack = (id) => ipcMain.emit('unsaved-check-ack', {}, id); - return { win, wc, sent, timers, closeEvent, answer, ack, ipcMain, guard }; + const reasonDone = (id) => ipcMain.emit('unsaved-check-reason-done', {}, id); + return { win, wc, sent, timers, closeEvent, answer, ack, reasonDone, ipcMain, guard }; } const tick = () => new Promise((r) => setImmediate(r)); @@ -279,6 +280,7 @@ test('a quit that joins an unanswered reload check tells the renderer it is now assert.deepEqual(t.sent.map((m) => m.channel), ['unsaved-check', 'unsaved-check-reason'], 'one upgrade, no second question'); assert.deepEqual(t.sent[1].args, [id, 'quit']); t.answer(id, true); + t.reasonDone(id); await tick(); assert.equal(quits.length, 1); }); @@ -290,11 +292,56 @@ test('a reload check that a quit joined quits without reloading first', async () const id = t.sent[0].args[0]; t.guard.beforeQuit(t.closeEvent(), t.win); t.answer(id, true); + t.reasonDone(id); await tick(); assert.equal(t.wc.reloads, 0); assert.equal(quits.length, 1); }); +test('a quit that joins a reload check waits for the page to finish the save that upgrade asks for, but not for ever', async () => { + const quits = []; + const t = setup({ quit: () => quits.push(1) }); + t.wc.emit('will-prevent-unload', t.closeEvent()); + const id = t.sent[0].args[0]; + t.guard.beforeQuit(t.closeEvent(), t.win); + t.answer(id, true); + await tick(); + assert.equal(quits.length, 0, 'the page may have answered before it saw the upgrade, and is still saving'); + t.reasonDone(id); + await tick(); + assert.equal(quits.length, 1); + + const silent = setup({ quit: () => quits.push(2) }); + silent.wc.emit('will-prevent-unload', silent.closeEvent()); + const sid = silent.sent[0].args[0]; + silent.guard.beforeQuit(silent.closeEvent(), silent.win); + silent.answer(sid, true); + await tick(); + assert.deepEqual(quits, [1]); + const bound = silent.timers.filter((x) => !x.cleared && x.ms === 1000); + assert.equal(bound.length, 1, 'one bounded wait'); + bound[0].fn(); + await tick(); + assert.deepEqual(quits, [1, 2], 'a page that never says it is done does not hold the quit'); + + const declined = setup({ quit: () => quits.push(3) }); + declined.wc.emit('will-prevent-unload', declined.closeEvent()); + const did = declined.sent[0].args[0]; + declined.guard.beforeQuit(declined.closeEvent(), declined.win); + declined.answer(did, false); + await tick(); + assert.equal(declined.timers.filter((x) => !x.cleared && x.ms === 1000).length, 0, 'a declined check leaves no wait behind'); + + const hung = setup({ quit: () => quits.push(4) }); + hung.wc.emit('will-prevent-unload', hung.closeEvent()); + hung.guard.beforeQuit(hung.closeEvent(), hung.win); + const checkBound = hung.timers.filter((x) => !x.cleared && x.ms === 1000)[0]; + checkBound.fn(); + await tick(); + assert.deepEqual(quits, [1, 2, 4], 'a page that never answered the check is not waited for a second time'); + assert.equal(hung.timers.filter((x) => !x.cleared && x.ms === 1000).length, 0); +}); + test('a reload joining a quit check leaves it a quit', () => { const t = setup(); t.win.emit('close', t.closeEvent()); From 551f72149588321197305fda5c32e0cf29604479 Mon Sep 17 00:00:00 2001 From: abate Date: Sun, 11 Oct 2026 16:06:09 +0200 Subject: [PATCH 2/5] fix(restore): hold a quit that joined a reload until the page's late save is done The renderer answers every reason upgrade with unsaved-check-reason-done, after the save when it is writing for an already approved reload, at once otherwise; main waits for it, bounded by the check timeout. --- .ai/contexts/viewer-panel.md | 2 +- CHANGELOG.md | 1 + docs/session-restore.md | 6 ++++-- preload.js | 1 + public/file-panel.js | 5 ++++- unsaved-guard.js | 31 ++++++++++++++++++++++++++++++- 6 files changed, 41 insertions(+), 5 deletions(-) diff --git a/.ai/contexts/viewer-panel.md b/.ai/contexts/viewer-panel.md index 856c865e8..c2f57a8f1 100644 --- a/.ai/contexts/viewer-panel.md +++ b/.ai/contexts/viewer-panel.md @@ -164,7 +164,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; a reload it has already approved then writes the app state at once, and main skips the reload of a question a quit joined. 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. +- **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; a reload it has already approved then writes the app state at once. The renderer answers every upgrade with `unsaved-check-reason-done` (`id`): at once, unless it is writing for an approved reload, then once that write ends; main holds the joined quit until it, bounded by the check timeout, since the answer may have left before the upgrade arrived. Main skips the reload of a question a quit joined. 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. 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. diff --git a/CHANGELOG.md b/CHANGELOG.md index 862763f77..d17cf1f1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ### Fixed - The New Session dialog now opens with focus on the selected permission mode, and Enter starts the session with that mode. Cancelling returns focus to the + button that opened the dialog; starting moves it to the new terminal. (#583) - Tab and Shift+Tab now close the New session (+) menu and move on from its + button, instead of leaving the menu open with no way to close it from the keyboard. (#582) +- A quit that arrives just as a reload of the window is confirmed now waits for the open sessions to be saved before Switchboard closes, so the next start restores them. (#479) ## v0.0.95 — 2026-10-12 diff --git a/docs/session-restore.md b/docs/session-restore.md index fdf2f7bfe..b741a5412 100644 --- a/docs/session-restore.md +++ b/docs/session-restore.md @@ -57,8 +57,10 @@ not an exit and does neither. A quit or close that arrives while a reload's question is still open joins it, and main tells the renderer the question is now an exit (`unsaved-check-reason`), so the answer still waits for the write, and the page is not reloaded before the quit. If the page had already answered -the reload when the upgrade reaches it, it writes the set then, best effort as -for a logoff below. +the reload when the upgrade reaches it, it writes the set then, and main waits +for it to report the write done (`unsaved-check-reason-done`, bounded to +2.5 s) before quitting; the page sends that report at once when it has +nothing to write then. The write keeps the saved sessions that are not open yet: those the index has not reached, those offered by the Restore prompt and not answered, and those diff --git a/preload.js b/preload.js index 82d46c043..fb62fa74e 100644 --- a/preload.js +++ b/preload.js @@ -168,6 +168,7 @@ contextBridge.exposeInMainWorld('api', { }, unsavedCheckAck: (id) => ipcRenderer.send('unsaved-check-ack', id), unsavedCheckResult: (id, proceed) => ipcRenderer.send('unsaved-check-result', id, proceed), + unsavedCheckReasonDone: (id) => ipcRenderer.send('unsaved-check-reason-done', id), onExitFlush: (callback) => { ipcRenderer.on('exit-flush', () => callback()); }, diff --git a/public/file-panel.js b/public/file-panel.js index a91a9de00..d06604e9f 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -154,10 +154,13 @@ function initFilePanel() { const answeredReloads = new Set(); if (window.api.onUnsavedCheckReason) { window.api.onUnsavedCheckReason((id, reason) => { + const done = () => { if (window.api.unsavedCheckReasonDone) window.api.unsavedCheckReasonDone(id); }; if (unsavedCheckReasons.has(id)) unsavedCheckReasons.set(id, reason); else if (answeredReloads.delete(id) && typeof window.flushStateForExit === 'function') { - window.flushStateForExit().catch((err) => console.error('[exit-flush]', err)); + window.flushStateForExit().catch((err) => console.error('[exit-flush]', err)).finally(done); + return; } + done(); }); } window.api.onUnsavedCheck(async (id, reason) => { diff --git a/unsaved-guard.js b/unsaved-guard.js index 639ec7416..ccc1a9516 100644 --- a/unsaved-guard.js +++ b/unsaved-guard.js @@ -31,11 +31,40 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, logoffCan entry.finish(proceed === true); }); + const reasonWaits = new Map(); + ipcMain.on('unsaved-check-reason-done', (_event, id) => { + const done = reasonWaits.get(id); + if (done) done(); + }); + + function waitReasonDone(id) { + let release = null; + const done = new Promise((resolve) => { + const timer = setTimeoutFn(() => release(), timeoutMs); + release = () => { + reasonWaits.delete(id); + clearTimeoutFn(timer); + resolve(); + }; + reasonWaits.set(id, release); + }); + return { done, release }; + } + function askFull(win, reason) { if (inflight) { if (inflight.reason === 'reload') { inflight.reason = reason; - try { win.webContents.send('unsaved-check-reason', inflight.id, reason); } catch {} + const { id } = inflight; + const wait = waitReasonDone(id); + try { win.webContents.send('unsaved-check-reason', id, reason); } catch { wait.release(); } + inflight.asked = inflight.asked.then((result) => { + if (!result.proceed || result.unanswered) { + wait.release(); + return result; + } + return wait.done.then(() => result); + }); } return inflight.asked; } From a1c1d94d2df420e84eadd1849df68615f12a082f Mon Sep 17 00:00:00 2001 From: abate Date: Mon, 12 Oct 2026 09:14:25 +0200 Subject: [PATCH 3/5] test(restore): a quit held for the late save is joined, and released when the page dies A close or a quit that arrives after the page answered a reload that a quit joined, but before the page reported its late save, must join that quit instead of asking again. A page that crashes or is destroyed while the quit waits must release it at once rather than after the bound. --- test/unsaved-guard.test.js | 39 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index e27c1cf21..7650122e2 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -342,6 +342,45 @@ test('a quit that joins a reload check waits for the page to finish the save tha assert.equal(hung.timers.filter((x) => !x.cleared && x.ms === 1000).length, 0); }); +test('a close or quit arriving while a joined quit waits for the late save joins it instead of asking again', async () => { + const quits = []; + const t = setup({ quit: () => quits.push(1) }); + t.wc.emit('will-prevent-unload', t.closeEvent()); + const id = t.sent[0].args[0]; + t.guard.beforeQuit(t.closeEvent(), t.win); + t.answer(id, true); + await tick(); + const close = t.closeEvent(); + t.win.emit('close', close); + const install = t.guard.confirmQuit(t.win); + await tick(); + assert.equal(close.prevented, true); + assert.deepEqual(t.sent.map((m) => m.channel), ['unsaved-check', 'unsaved-check-reason'], 'no second question while the save is pending'); + assert.equal(quits.length, 0); + t.reasonDone(id); + await tick(); + assert.equal(quits.length, 1); + assert.equal(await install, true); + assert.equal(t.win.closes, 1, 'the held close goes through once the save is done'); +}); + +test('a page that dies while a joined quit waits for its late save releases the quit at once', async () => { + for (const event of ['render-process-gone', 'destroyed']) { + const quits = []; + const t = setup({ quit: () => quits.push(1) }); + t.wc.emit('will-prevent-unload', t.closeEvent()); + const id = t.sent[0].args[0]; + t.guard.beforeQuit(t.closeEvent(), t.win); + t.answer(id, true); + await tick(); + assert.equal(quits.length, 0); + t.wc.emit(event); + await tick(); + assert.equal(quits.length, 1, `released on ${event}`); + assert.equal(t.timers.filter((x) => !x.cleared && x.ms === 1000).length, 0, 'no bounded wait left running'); + } +}); + test('a reload joining a quit check leaves it a quit', () => { const t = setup(); t.win.emit('close', t.closeEvent()); From 767c5569b902233711a0738143d2dd1c27e4ee0d Mon Sep 17 00:00:00 2001 From: abate Date: Mon, 12 Oct 2026 09:15:42 +0200 Subject: [PATCH 4/5] fix(restore): keep a joined quit in flight until the late save is done The answer used to clear the check in flight while the joined quit still waited for the page's late save, so a close or quit in between started a second question. The joined check now stays in flight until that wait ends, and the wait ends at once when the renderer process is gone. --- .ai/contexts/viewer-panel.md | 2 +- docs/session-restore.md | 5 +++-- test/unsaved-guard.test.js | 14 ++++++++++++++ unsaved-guard.js | 22 ++++++++++++++++------ 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/.ai/contexts/viewer-panel.md b/.ai/contexts/viewer-panel.md index c2f57a8f1..c9ced07e5 100644 --- a/.ai/contexts/viewer-panel.md +++ b/.ai/contexts/viewer-panel.md @@ -164,7 +164,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; a reload it has already approved then writes the app state at once. The renderer answers every upgrade with `unsaved-check-reason-done` (`id`): at once, unless it is writing for an approved reload, then once that write ends; main holds the joined quit until it, bounded by the check timeout, since the answer may have left before the upgrade arrived. Main skips the reload of a question a quit joined. 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. +- **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; a reload it has already approved then writes the app state at once. The renderer answers every upgrade with `unsaved-check-reason-done` (`id`): at once, unless it is writing for an approved reload, then once that write ends; main holds the joined quit until it, bounded by the check timeout and released at once if the renderer process is gone (`render-process-gone`, `destroyed`), since the answer may have left before the upgrade arrived. Until then the joined question stays the one in flight, so a close, quit or install arriving meanwhile joins it instead of asking again. Main skips the reload of a question a quit joined. 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. 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. diff --git a/docs/session-restore.md b/docs/session-restore.md index b741a5412..f0b4c8a9e 100644 --- a/docs/session-restore.md +++ b/docs/session-restore.md @@ -59,8 +59,9 @@ now an exit (`unsaved-check-reason`), so the answer still waits for the write, and the page is not reloaded before the quit. If the page had already answered the reload when the upgrade reaches it, it writes the set then, and main waits for it to report the write done (`unsaved-check-reason-done`, bounded to -2.5 s) before quitting; the page sends that report at once when it has -nothing to write then. +2.5 s, and not at all once the page has crashed) before quitting; the page +sends that report at once when it has nothing to write then. A close or quit +that arrives during that wait joins it rather than asking again. The write keeps the saved sessions that are not open yet: those the index has not reached, those offered by the Restore prompt and not answered, and those diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index 7650122e2..239db9d03 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -364,6 +364,20 @@ test('a close or quit arriving while a joined quit waits for the late save joins assert.equal(t.win.closes, 1, 'the held close goes through once the save is done'); }); +test('a joined quit the page declined leaves nothing for a later close to join', async () => { + const t = setup(); + t.wc.emit('will-prevent-unload', t.closeEvent()); + t.guard.beforeQuit(t.closeEvent(), t.win); + t.answer(t.sent[0].args[0], false); + await tick(); + t.win.emit('close', t.closeEvent()); + const checks = t.sent.filter((m) => m.channel === 'unsaved-check'); + assert.equal(checks.length, 2, 'the later close is asked about'); + t.answer(checks[1].args[0], true); + await tick(); + assert.equal(t.win.closes, 1); +}); + test('a page that dies while a joined quit waits for its late save releases the quit at once', async () => { for (const event of ['render-process-gone', 'destroyed']) { const quits = []; diff --git a/unsaved-guard.js b/unsaved-guard.js index ccc1a9516..55c4d62ad 100644 --- a/unsaved-guard.js +++ b/unsaved-guard.js @@ -37,16 +37,21 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, logoffCan if (done) done(); }); - function waitReasonDone(id) { + function waitReasonDone(id, wc) { let release = null; const done = new Promise((resolve) => { const timer = setTimeoutFn(() => release(), timeoutMs); + const onGone = () => release(); release = () => { reasonWaits.delete(id); clearTimeoutFn(timer); + wc.removeListener('render-process-gone', onGone); + wc.removeListener('destroyed', onGone); resolve(); }; reasonWaits.set(id, release); + wc.on('render-process-gone', onGone); + wc.on('destroyed', onGone); }); return { done, release }; } @@ -54,16 +59,21 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, logoffCan function askFull(win, reason) { if (inflight) { if (inflight.reason === 'reload') { - inflight.reason = reason; - const { id } = inflight; - const wait = waitReasonDone(id); + const joined = inflight; + joined.reason = reason; + joined.held = true; + const { id } = joined; + const wait = waitReasonDone(id, win.webContents); try { win.webContents.send('unsaved-check-reason', id, reason); } catch { wait.release(); } - inflight.asked = inflight.asked.then((result) => { + joined.asked = joined.asked.then((result) => { if (!result.proceed || result.unanswered) { wait.release(); return result; } return wait.done.then(() => result); + }).then((result) => { + if (inflight === joined) inflight = null; + return result; }); } return inflight.asked; @@ -82,7 +92,7 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, logoffCan wc.removeListener('render-process-gone', onGone); wc.removeListener('destroyed', onGone); settled = true; - inflight = null; + if (inflight && inflight.id === id && !inflight.held) inflight = null; resolve({ proceed, unanswered }); }; entry.timer = setTimeoutFn(() => entry.finish(true, true), timeoutMs); From 80c87e3c4f8275352e2b030c5ee0a53fe61a0cad Mon Sep 17 00:00:00 2001 From: abate Date: Mon, 12 Oct 2026 15:35:26 +0200 Subject: [PATCH 5/5] test(restore): a reason report that arrives before the answer releases the joined quit on that answer The renderer reports the upgrade done before it answers when the question is still open; the joined quit must then go on the answer, with no bounded wait left running. --- test/unsaved-guard.test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index 239db9d03..b26822cb1 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -342,6 +342,21 @@ test('a quit that joins a reload check waits for the page to finish the save tha assert.equal(hung.timers.filter((x) => !x.cleared && x.ms === 1000).length, 0); }); +test('a page that reports the upgrade done before it answers lets the joined quit go on the answer', async () => { + const quits = []; + const t = setup({ quit: () => quits.push(1) }); + t.wc.emit('will-prevent-unload', t.closeEvent()); + const id = t.sent[0].args[0]; + t.guard.beforeQuit(t.closeEvent(), t.win); + t.reasonDone(id); + await tick(); + assert.equal(quits.length, 0, 'the question is still open'); + t.answer(id, true); + await tick(); + assert.equal(quits.length, 1, 'no wait for a report already received'); + assert.equal(t.timers.filter((x) => !x.cleared && x.ms === 1000).length, 0, 'no bounded wait left running'); +}); + test('a close or quit arriving while a joined quit waits for the late save joins it instead of asking again', async () => { const quits = []; const t = setup({ quit: () => quits.push(1) });