From dead7ae4cc1bb279774330ec26e61d49b217630d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 20:34:06 +0200 Subject: [PATCH 1/3] (file-panel): ask before quit, close or reload drops unsaved file edits Unsaved edits in the file panel were lost silently when the app quit, the window closed or the page reloaded. Main now holds the window close and a blocked unload until the renderer answers, with a bounded wait so a hung renderer cannot prevent quitting. The renderer lists the dirty tabs (shown, kept aside, any session) in a Save / Discard / Cancel dialog and vetoes beforeunload while a tab is dirty. Closes #373 --- .ai/contexts/viewer-panel.md | 10 + CHANGELOG.md | 3 + main.js | 4 + preload.js | 4 + public/file-panel.js | 141 ++++++++++++++ public/style.css | 10 + public/viewer-panel.js | 4 + test/dom-file-panel-unsaved-guard.test.js | 223 ++++++++++++++++++++++ test/unsaved-guard.test.js | 136 +++++++++++++ unsaved-guard.js | 74 +++++++ 10 files changed, 609 insertions(+) create mode 100644 test/dom-file-panel-unsaved-guard.test.js create mode 100644 test/unsaved-guard.test.js create mode 100644 unsaved-guard.js diff --git a/.ai/contexts/viewer-panel.md b/.ai/contexts/viewer-panel.md index 158361b4..52246105 100644 --- a/.ai/contexts/viewer-panel.md +++ b/.ai/contexts/viewer-panel.md @@ -163,6 +163,16 @@ The bar (`#file-panel-held`) has `role="status"`, so a screen reader announces i It is shown with `open(…, restore)` and re-read at once, like any return to the viewer: a write made while it was held — the diff the user just accepted, for instance — raises "changed on disk", and `_agreedBase` has not moved, so a save against that write is refused by main and asks. The bar is not shown over a diff: leaving an unanswered diff would leave the CLI waiting on it. +### Unsaved edits on quit, reload and close + +Nothing ends the window with a dirty file tab without the user saying so. The scope is the file tabs of the file panel (the shown one and `heldFileTabs`, 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). The window's `close` event — which an app quit (`app.quit()`, the ☰ menu's Quit, the last window closing) goes through on every platform — is prevented, and main sends `unsaved-check` (`id`, `'quit'`) to the renderer. The renderer's `unsaved-check-result` (`id`, `proceed`) approves the close (`approved`, then `win.close()` again, which passes) or leaves the window open. A `will-prevent-unload` — the renderer's `beforeunload` veto, which is what a reload hits — asks with `'reload'` and calls `webContents.reload()` on a yes. During an approved close, `will-prevent-unload` is let through with `preventDefault()`, which in Electron means "unload anyway". +- **Bounded.** No answer within `DEFAULT_TIMEOUT_MS` (2.5 s), a crashed or destroyed renderer, or a failed send all answer yes: a hung renderer never keeps the app from quitting. A late answer is ignored. The edits of a renderer that is merely slow can be lost this way; that is the price of never deadlocking. +- **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 tab: the shown one through the viewer's own save (`ViewerPanel.saveNow()`, with its stale-disk confirm), a tab kept aside through `saveFileForPanel` with its snapshot's `agreedBase`. 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 a file tab 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 tabs, 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. + ### A document not yet in the editor `open()` puts its document in the editor only once the CodeMirror bundle has loaded. Until then the panel holds it as `_pendingContent`, and that is the buffer: `_isDirty` and `snapshot()` read it, so a restored tab replaced again before its editor exists is held with its edits, whichever route replaces it. diff --git a/CHANGELOG.md b/CHANGELOG.md index 97dae2a5..25491665 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased +### Fixed +- Quitting, closing the window or reloading while a file in the file panel has unsaved edits now asks first, in any session, kept-aside tabs included: Save writes them (a file that changed on disk is not overwritten), Discard drops them, Cancel stays. If Switchboard does not answer within a few seconds, it closes anyway. (#373) + ## v0.0.86 — 2026-10-01 ### New diff --git a/main.js b/main.js index 795ca5e8..1e4de5ab 100644 --- a/main.js +++ b/main.js @@ -39,6 +39,8 @@ const { state: TRACE, trace, codePoints, controlOffset, busyDecision, progressDe const { classifyTitleActivity } = require('./classify-title-activity'); const { windowFrameOptions, applicationMenuTemplate, zoomKey, nextZoomLevel, menuPopupPoint } = require('./window-frame'); const { createWhatsNew } = require('./changelog'); +const { createUnsavedGuard } = require('./unsaved-guard'); +const unsavedGuard = createUnsavedGuard({ ipcMain }); const { cleanEnv } = require('./clean-env'); try { require('electron-reloader')(module, { watchRenderer: true }); } catch {}; @@ -355,6 +357,8 @@ function createWindow() { `); }); + unsavedGuard.attach(mainWindow); + // Prevent Cmd+R / Ctrl+Shift+R from reloading the page (Chromium built-in). // Ctrl+R alone on macOS is NOT a reload shortcut and must pass through to xterm // for reverse-i-search. diff --git a/preload.js b/preload.js index c2dd3d0d..f58775cd 100644 --- a/preload.js +++ b/preload.js @@ -142,6 +142,10 @@ contextBridge.exposeInMainWorld('api', { onIndexingFinished: (callback) => { ipcRenderer.on('indexing-finished', () => callback()); }, + onUnsavedCheck: (callback) => { + ipcRenderer.on('unsaved-check', (_event, id, reason) => callback(id, reason)); + }, + unsavedCheckResult: (id, proceed) => ipcRenderer.send('unsaved-check-result', id, proceed), onFullScreenChanged: (callback) => { ipcRenderer.on('full-screen-changed', (_event, isFullScreen) => callback(isFullScreen)); }, diff --git a/public/file-panel.js b/public/file-panel.js index bd30ae51..e4da6a92 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -138,6 +138,19 @@ function initFilePanel() { onDetachedSave: dropSavedHeldTabs, }); + window.addEventListener('beforeunload', (event) => { + if (unloadApproved || !collectUnsavedFileTabs().length) return; + event.preventDefault(); + event.returnValue = false; + }); + if (window.api.onUnsavedCheck) { + window.api.onUnsavedCheck(async (id) => { + let proceed = true; + try { proceed = await askAboutUnsavedEdits(); } catch (err) { console.error('[unsaved-check]', err); } + window.api.unsavedCheckResult(id, proceed); + }); + } + // ── Diff-specific UI ── const diffContainer = document.createElement('div'); diffContainer.id = 'file-panel-diff'; @@ -557,6 +570,134 @@ function takeHeldFileTab(state, filePath) { return tab; } +// see .ai/contexts/viewer-panel.md ("Unsaved edits on quit, reload and close") +let unloadApproved = false; +let unsavedPrompt = null; + +function collectUnsavedFileTabs() { + if (!fpViewerPanel) return []; + const found = new Set(); + for (const state of filePanelState.values()) { + const tabs = []; + if (state.currentTab && state.currentTab.type === 'file') tabs.push(state.currentTab); + if (state.heldFileTabs) tabs.push(...state.heldFileTabs.values()); + for (const tab of tabs) if (fileTabHasUnsavedEdits(tab)) found.add(tab); + } + return [...found]; +} + +async function saveUnsavedFileTab(tab) { + if (fpViewerOwner === tab) { + await fpViewerPanel.saveNow(); + return fileTabHasUnsavedEdits(tab) ? 'not saved: it changed on disk, or could not be written' : null; + } + const saved = tab.viewerState; + const result = await window.api.saveFileForPanel(tab.filePath, saved.content, saved.agreedBase); + if (result && result.ok !== false) { + tab.viewerState = { ...saved, agreedBase: saved.content, lastSeenDisk: saved.content }; + return null; + } + if (result && result.reason === 'stale') return 'not saved: it changed on disk since you opened it'; + return `not saved: ${(result && result.error) || 'unknown error'}`; +} + +function showUnsavedEditsDialog(tabs) { + return new Promise((resolve) => { + const overlay = document.createElement('div'); + overlay.className = 'add-project-overlay'; + const dialog = document.createElement('div'); + dialog.className = 'add-project-dialog'; + dialog.id = 'unsaved-edits-dialog'; + dialog.setAttribute('role', 'alertdialog'); + + const title = document.createElement('h3'); + title.textContent = 'Unsaved file edits'; + dialog.appendChild(title); + + const hint = document.createElement('div'); + hint.className = 'add-project-hint'; + hint.textContent = 'These files have edits that are not saved. Discarding loses them.'; + dialog.appendChild(hint); + + const labels = heldTabLabels(tabs); + const list = document.createElement('ul'); + tabs.forEach((tab, i) => { + const li = document.createElement('li'); + li.textContent = labels[i]; + li.title = tab.filePath; + list.appendChild(li); + }); + dialog.appendChild(list); + + const errorEl = document.createElement('div'); + errorEl.className = 'add-project-error'; + dialog.appendChild(errorEl); + + const actions = document.createElement('div'); + actions.className = 'add-project-actions'; + const makeBtn = (id, cls, text) => { + const btn = document.createElement('button'); + btn.id = id; + btn.className = cls; + btn.textContent = text; + actions.appendChild(btn); + return btn; + }; + const cancelBtn = makeBtn('unsaved-cancel', 'add-project-cancel-btn', 'Cancel'); + const discardBtn = makeBtn('unsaved-discard', 'add-project-cancel-btn', 'Discard'); + const saveBtn = makeBtn('unsaved-save', 'add-project-add-btn', tabs.length > 1 ? 'Save all' : 'Save'); + dialog.appendChild(actions); + overlay.appendChild(dialog); + document.body.appendChild(overlay); + saveBtn.focus(); + + function finish(proceed) { + overlay.remove(); + document.removeEventListener('keydown', onKey); + resolve(proceed); + } + function onKey(e) { + if (e.key === 'Escape') finish(false); + } + document.addEventListener('keydown', onKey); + + cancelBtn.onclick = () => finish(false); + discardBtn.onclick = () => finish(true); + saveBtn.onclick = async () => { + for (const btn of [cancelBtn, discardBtn, saveBtn]) btn.disabled = true; + const failures = []; + for (let i = 0; i < tabs.length; i++) { + if (!fileTabHasUnsavedEdits(tabs[i])) continue; + let reason; + try { reason = await saveUnsavedFileTab(tabs[i]); } catch (err) { reason = `not saved: ${(err && err.message) || 'unknown error'}`; } + if (reason) failures.push(`${labels[i]} ${reason}`); + } + if (!failures.length) { + finish(true); + return; + } + errorEl.textContent = failures.join('. '); + errorEl.style.display = 'block'; + for (const btn of [cancelBtn, discardBtn, saveBtn]) btn.disabled = false; + }; + }); +} + +function askAboutUnsavedEdits() { + if (unsavedPrompt) return unsavedPrompt; + const tabs = collectUnsavedFileTabs(); + if (!tabs.length) return Promise.resolve(true); + unsavedPrompt = showUnsavedEditsDialog(tabs).then((proceed) => { + unsavedPrompt = null; + if (proceed) { + unloadApproved = true; + setTimeout(() => { unloadApproved = false; }, 10000); + } + return proceed; + }); + return unsavedPrompt; +} + function endCurrentTab(sessionId, state) { const held = state.heldFileTabs ? [...state.heldFileTabs.values()].at(-1) : null; if (held) { diff --git a/public/style.css b/public/style.css index e2a811a2..92f7adad 100644 --- a/public/style.css +++ b/public/style.css @@ -5115,3 +5115,13 @@ body.window-frameless :is(#terminal-header-status, #terminal-header-id, #termina body.window-frameless :is(.new-session-popover, .terminal-context-menu, .new-session-overlay, .add-project-overlay, .whats-new-overlay, .jsonl-screenshot-fullscreen, #update-toast, .restore-toast) { -webkit-app-region: no-drag; } + +#unsaved-edits-dialog ul { + margin: 0 0 4px 0; + padding-left: 18px; + color: #b0b0c4; + font-size: 13px; + font-family: 'SF Mono', 'Fira Code', monospace; + max-height: 160px; + overflow-y: auto; +} diff --git a/public/viewer-panel.js b/public/viewer-panel.js index be953f9f..0bf059aa 100644 --- a/public/viewer-panel.js +++ b/public/viewer-panel.js @@ -451,6 +451,10 @@ class ViewerPanel { this.toolbar.setWrapMode(this.wrapMode); } + saveNow() { + return this._save(); + } + // see .ai/contexts/viewer-panel.md ("Saving over a file that moved") async _save() { if (!this.opts.onSave || !this.filePath || this._pendingContent !== null) return; diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js new file mode 100644 index 00000000..4c05d0b6 --- /dev/null +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -0,0 +1,223 @@ +'use strict'; + +// Quitting, reloading or closing a session with unsaved file edits asks first (#373). + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { JSDOM } = require('jsdom'); + +const PUBLIC_DIR = path.join(__dirname, '..', 'public'); + +const INDEX_HTML = ` + + + +
+ + +`; + +function fakeEditor(initial) { + let doc = initial || ''; + return { + state: { doc: { toString: () => doc, get length() { return doc.length; } } }, + dispatch(tr) { + if (tr && tr.changes) { + const { from, to, insert } = tr.changes; + doc = doc.slice(0, from) + insert + doc.slice(to); + } + }, + type(text) { doc += text; }, + destroy() {}, + }; +} + +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, answers: [], saves: [], confirms: [] }; + let editor = null; + + window.api = new Proxy({ + onMcpOpenFile: (cb) => { calls.openFile = cb; }, + onUnsavedCheck: (cb) => { calls.check = cb; }, + unsavedCheckResult: (id, proceed) => { calls.answers.push({ id, proceed }); }, + watchFile: () => Promise.resolve({ ok: true }), + unwatchFile: () => Promise.resolve({ ok: true }), + readFileForPanel: (p) => Promise.resolve(disk.has(p) ? { ok: true, content: disk.get(p) } : { ok: false, code: 'ENOENT', error: 'ENOENT' }), + saveFileForPanel: (p, content, expected) => { + calls.saves.push({ path: p, content, expected }); + if (disk.get(p) !== expected) return Promise.resolve({ ok: false, reason: 'stale', error: 'stale', disk: disk.get(p) }); + disk.set(p, content); + return Promise.resolve({ ok: true }); + }, + }, { + get(target, prop) { + if (prop in target) return target[prop]; + if (typeof prop === 'string' && prop.startsWith('on')) return () => {}; + return () => Promise.resolve({ ok: true }); + }, + }); + window.confirm = (msg) => { calls.confirms.push(msg); return false; }; + window.createEditableViewer = (parent, content) => { editor = fakeEditor(content); return editor; }; + window.createPlanEditor = () => { editor = fakeEditor(''); return editor; }; + Object.defineProperty(window, 'activeSessionId', { value: null, writable: true, configurable: true }); + + const realCreate = window.document.createElement.bind(window.document); + window.document.createElement = function (tag, ...args) { + const el = realCreate(tag, ...args); + if (String(tag).toLowerCase() === 'script') Promise.resolve().then(() => el.onload && el.onload()); + return el; + }; + + for (const f of ['viewer-toolbar.js', 'viewer-panel.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'header-controls.js', 'file-panel.js']) { + vm.runInContext(fs.readFileSync(path.join(PUBLIC_DIR, f), 'utf8'), dom.getInternalVMContext(), { filename: path.join(PUBLIC_DIR, f) }); + } + window.initFilePanel(); + return { window, disk, calls, editor: () => editor, destroy: () => window.close() }; +} + +const flush = () => new Promise((r) => setTimeout(r, 5)); +const A = '/repo/a.md'; +const B = '/repo/b.md'; +const C = '/repo/c.md'; + +async function dirtyTab(ctx, sessionId, file, text = 'mine') { + ctx.disk.set(file, 'x0\n'); + ctx.window.switchPanel(sessionId); + ctx.calls.openFile(sessionId, { filePath: file, content: 'x0\n' }); + await flush(); + ctx.editor().type(text); +} + +const dialog = (ctx) => ctx.window.document.getElementById('unsaved-edits-dialog'); +const button = (ctx, id) => ctx.window.document.getElementById(id); + +function beforeUnload(ctx) { + const event = new ctx.window.Event('beforeunload', { cancelable: true }); + ctx.window.dispatchEvent(event); + return event; +} + +test('with nothing unsaved the check is answered at once and no dialog shows', async () => { + const ctx = setup(); + try { + ctx.disk.set(A, 'x0\n'); + ctx.window.switchPanel('s1'); + ctx.calls.openFile('s1', { filePath: A, content: 'x0\n' }); + await flush(); + ctx.calls.check(1, 'quit'); + await flush(); + assert.deepEqual(ctx.calls.answers, [{ id: 1, proceed: true }]); + assert.equal(dialog(ctx), null); + assert.equal(beforeUnload(ctx).defaultPrevented, false); + } finally { ctx.destroy(); } +}); + +test('a dirty file tab holds the answer behind a dialog naming the file; Cancel answers no', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A); + ctx.calls.check(7, 'quit'); + await flush(); + assert.ok(dialog(ctx), 'the dialog is shown'); + assert.match(dialog(ctx).textContent, /a\.md/); + assert.deepEqual(ctx.calls.answers, []); + button(ctx, 'unsaved-cancel').click(); + await flush(); + assert.deepEqual(ctx.calls.answers, [{ id: 7, proceed: false }]); + assert.equal(dialog(ctx), null); + assert.equal(ctx.disk.get(A), 'x0\n'); + } finally { ctx.destroy(); } +}); + +test('Discard answers yes, writes nothing, and lets the window unload', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A); + assert.equal(beforeUnload(ctx).defaultPrevented, true, 'a dirty tab blocks an unload nobody approved'); + ctx.calls.check(2, 'reload'); + await flush(); + button(ctx, 'unsaved-discard').click(); + await flush(); + assert.deepEqual(ctx.calls.answers, [{ id: 2, proceed: true }]); + assert.deepEqual(ctx.calls.saves, []); + assert.equal(beforeUnload(ctx).defaultPrevented, false, 'the approved unload goes through'); + } finally { ctx.destroy(); } +}); + +test('Save writes the edits against the agreed base, then answers yes', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A); + ctx.calls.check(3, 'quit'); + await flush(); + button(ctx, 'unsaved-save').click(); + await flush(); + assert.equal(ctx.disk.get(A), 'x0\nmine'); + assert.deepEqual(ctx.calls.answers, [{ id: 3, proceed: true }]); + } finally { ctx.destroy(); } +}); + +test('a save the disk refuses keeps the dialog open and answers nothing', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A); + ctx.disk.set(A, 'written by someone else\n'); + ctx.calls.check(4, 'quit'); + await flush(); + button(ctx, 'unsaved-save').click(); + await flush(); + assert.ok(dialog(ctx), 'the dialog stays'); + assert.match(dialog(ctx).textContent, /changed on disk/); + assert.equal(ctx.disk.get(A), 'written by someone else\n'); + assert.deepEqual(ctx.calls.answers, []); + button(ctx, 'unsaved-cancel').click(); + await flush(); + assert.deepEqual(ctx.calls.answers, [{ id: 4, proceed: false }]); + } finally { ctx.destroy(); } +}); + +test('a dirty tab of another session and a tab kept aside both count, and Save writes all of them', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A, 'one'); + ctx.disk.set(B, 'x0\n'); + ctx.calls.openFile('s1', { filePath: B, content: 'x0\n' }); + await flush(); + ctx.window.switchPanel('s2'); + ctx.disk.set(C, 'x0\n'); + ctx.calls.openFile('s2', { filePath: C, content: 'x0\n' }); + await flush(); + ctx.editor().type('three'); + + ctx.calls.check(5, 'quit'); + await flush(); + const text = dialog(ctx).textContent; + assert.match(text, /a\.md/); + assert.match(text, /c\.md/); + assert.doesNotMatch(text, /b\.md/, 'a clean tab is not listed'); + button(ctx, 'unsaved-save').click(); + await flush(); + assert.equal(ctx.disk.get(A), 'x0\none'); + assert.equal(ctx.disk.get(C), 'x0\nthree'); + assert.deepEqual(ctx.calls.answers, [{ id: 5, proceed: true }]); + } finally { ctx.destroy(); } +}); + +test('beforeunload blocks while a tab is dirty and lets go once it is saved', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A); + assert.equal(beforeUnload(ctx).defaultPrevented, true); + ctx.window.document.getElementById('file-panel-viewer').dispatchEvent(new ctx.window.CustomEvent('cm-save')); + await flush(); + assert.equal(beforeUnload(ctx).defaultPrevented, false); + } finally { ctx.destroy(); } +}); diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js new file mode 100644 index 00000000..2c05efb7 --- /dev/null +++ b/test/unsaved-guard.test.js @@ -0,0 +1,136 @@ +'use strict'; + +// Main side of the unsaved-edits handshake (#373): a window close or a blocked +// unload waits for the renderer's answer, and never waits for ever. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { EventEmitter } = require('node:events'); + +const { createUnsavedGuard } = require('../unsaved-guard'); + +function setup({ timeoutMs = 1000 } = {}) { + const ipcMain = new EventEmitter(); + const timers = []; + const setTimeoutFn = (fn, ms) => { const t = { fn, ms, cleared: false }; timers.push(t); return t; }; + const clearTimeoutFn = (t) => { t.cleared = true; }; + const sent = []; + const wc = new EventEmitter(); + Object.assign(wc, { + send: (channel, ...args) => sent.push({ channel, args }), + isDestroyed: () => false, + isCrashed: () => false, + reload: () => { wc.reloads += 1; }, + reloads: 0, + }); + const win = new EventEmitter(); + Object.assign(win, { webContents: wc, isDestroyed: () => false, close: () => { win.closes += 1; }, closes: 0 }); + const guard = createUnsavedGuard({ ipcMain, timeoutMs, setTimeoutFn, clearTimeoutFn }); + guard.attach(win); + + const closeEvent = () => ({ prevented: false, preventDefault() { this.prevented = true; } }); + const answer = (id, proceed) => ipcMain.emit('unsaved-check-result', {}, id, proceed); + return { win, wc, sent, timers, closeEvent, answer, ipcMain }; +} + +const tick = () => new Promise((r) => setImmediate(r)); + +test('a close is held while the renderer is asked, and goes through on yes', async () => { + const t = setup(); + const e = t.closeEvent(); + t.win.emit('close', e); + assert.equal(e.prevented, true); + assert.equal(t.sent.length, 1); + assert.equal(t.sent[0].channel, 'unsaved-check'); + assert.equal(t.sent[0].args[1], 'quit'); + assert.equal(t.win.closes, 0); + + t.answer(t.sent[0].args[0], true); + await tick(); + assert.equal(t.win.closes, 1); + + const again = t.closeEvent(); + t.win.emit('close', again); + assert.equal(again.prevented, false, 'the approved close is not asked about again'); +}); + +test('a no keeps the window open and a later close asks again', async () => { + const t = setup(); + t.win.emit('close', t.closeEvent()); + t.answer(t.sent[0].args[0], false); + await tick(); + assert.equal(t.win.closes, 0); + + const e = t.closeEvent(); + t.win.emit('close', e); + assert.equal(e.prevented, true); + assert.equal(t.sent.length, 2); +}); + +test('a second close while the question is open asks nothing more', () => { + const t = setup(); + t.win.emit('close', t.closeEvent()); + const e = t.closeEvent(); + t.win.emit('close', e); + assert.equal(e.prevented, true); + assert.equal(t.sent.length, 1); +}); + +test('a renderer that never answers is given up on after the bound', async () => { + const t = setup({ timeoutMs: 2500 }); + t.win.emit('close', t.closeEvent()); + assert.equal(t.timers.length, 1); + assert.equal(t.timers[0].ms, 2500); + t.timers[0].fn(); + await tick(); + assert.equal(t.win.closes, 1); +}); + +test('an answer cancels its timer and a late answer is ignored', async () => { + const t = setup(); + t.win.emit('close', t.closeEvent()); + const id = t.sent[0].args[0]; + t.answer(id, false); + assert.equal(t.timers[0].cleared, true); + await tick(); + t.answer(id, true); + await tick(); + assert.equal(t.win.closes, 0); +}); + +test('a crashed renderer is not asked', async () => { + const t = setup(); + t.wc.isCrashed = () => true; + const e = t.closeEvent(); + t.win.emit('close', e); + await tick(); + assert.equal(t.sent.length, 0); + assert.equal(t.win.closes, 1); +}); + +test('a blocked unload asks the renderer and reloads on yes only', async () => { + const t = setup(); + const e = t.closeEvent(); + t.wc.emit('will-prevent-unload', e); + assert.equal(e.prevented, false, 'the page keeps its own veto until the user answers'); + assert.equal(t.sent[0].args[1], 'reload'); + t.answer(t.sent[0].args[0], false); + await tick(); + assert.equal(t.wc.reloads, 0); + + t.wc.emit('will-prevent-unload', t.closeEvent()); + t.answer(t.sent[1].args[0], true); + await tick(); + assert.equal(t.wc.reloads, 1); +}); + +test('a blocked unload during an approved close is let through', () => { + const t = setup(); + t.win.emit('close', t.closeEvent()); + t.answer(t.sent[0].args[0], true); + return tick().then(() => { + const e = t.closeEvent(); + t.wc.emit('will-prevent-unload', e); + assert.equal(e.prevented, true, 'preventDefault on will-prevent-unload lets the unload proceed'); + }); +}); diff --git a/unsaved-guard.js b/unsaved-guard.js new file mode 100644 index 00000000..dce3b43b --- /dev/null +++ b/unsaved-guard.js @@ -0,0 +1,74 @@ +'use strict'; + +// see .ai/contexts/viewer-panel.md ("Unsaved edits on quit, reload and close") + +const DEFAULT_TIMEOUT_MS = 2500; + +function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeoutFn = setTimeout, clearTimeoutFn = clearTimeout }) { + const pending = new Map(); + let nextId = 1; + + ipcMain.on('unsaved-check-result', (_event, id, proceed) => { + const entry = pending.get(id); + if (!entry) return; + pending.delete(id); + clearTimeoutFn(entry.timer); + entry.resolve(proceed === true); + }); + + function ask(win, reason) { + const wc = win.webContents; + if (win.isDestroyed() || !wc || wc.isDestroyed() || wc.isCrashed()) return Promise.resolve(true); + return new Promise((resolve) => { + const id = nextId++; + const timer = setTimeoutFn(() => { + pending.delete(id); + resolve(true); + }, timeoutMs); + pending.set(id, { resolve, timer }); + try { + wc.send('unsaved-check', id, reason); + } catch { + pending.delete(id); + clearTimeoutFn(timer); + resolve(true); + } + }); + } + + function attach(win) { + let approved = false; + let closing = false; + let reloading = false; + + win.on('close', (event) => { + if (approved) return; + event.preventDefault(); + if (closing) return; + closing = true; + ask(win, 'quit').then((proceed) => { + closing = false; + if (!proceed) return; + approved = true; + if (!win.isDestroyed()) win.close(); + }); + }); + + win.webContents.on('will-prevent-unload', (event) => { + if (approved) { + event.preventDefault(); + return; + } + if (reloading) return; + reloading = true; + ask(win, 'reload').then((proceed) => { + reloading = false; + if (proceed && !win.isDestroyed() && !win.webContents.isDestroyed()) win.webContents.reload(); + }); + }); + } + + return { attach }; +} + +module.exports = { createUnsavedGuard, DEFAULT_TIMEOUT_MS }; From b8380ef5a0a37dbd0844b67321abe79d63889c14 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 22:26:36 +0200 Subject: [PATCH 2/3] (file-panel): keep waiting for the user's answer and clean up only after quit is confirmed The renderer now acknowledges an unsaved-edits check on receipt; the bound covers only send to ack, so a slow user, a slow save or a stale-disk confirm no longer counts as yes. The check moves into before-quit, ahead of the PTY, MCP and watcher cleanup, so Cancel leaves the app intact. An approved reload allows exactly one unload, so a timeout yes cannot loop. Refs #373 --- .ai/contexts/viewer-panel.md | 4 +- main.js | 3 +- preload.js | 1 + public/file-panel.js | 1 + test/dom-file-panel-unsaved-guard.test.js | 15 +++- test/unsaved-guard.test.js | 87 ++++++++++++++++++++++- unsaved-guard.js | 62 ++++++++++++---- 7 files changed, 151 insertions(+), 22 deletions(-) diff --git a/.ai/contexts/viewer-panel.md b/.ai/contexts/viewer-panel.md index 52246105..2f7c4f61 100644 --- a/.ai/contexts/viewer-panel.md +++ b/.ai/contexts/viewer-panel.md @@ -167,8 +167,8 @@ It is shown with `open(…, restore)` and re-read at once, like any return to th Nothing ends the window with a dirty file tab without the user saying so. The scope is the file tabs of the file panel (the shown one and `heldFileTabs`, 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). The window's `close` event — which an app quit (`app.quit()`, the ☰ menu's Quit, the last window closing) goes through on every platform — is prevented, and main sends `unsaved-check` (`id`, `'quit'`) to the renderer. The renderer's `unsaved-check-result` (`id`, `proceed`) approves the close (`approved`, then `win.close()` again, which passes) or leaves the window open. A `will-prevent-unload` — the renderer's `beforeunload` veto, which is what a reload hits — asks with `'reload'` and calls `webContents.reload()` on a yes. During an approved close, `will-prevent-unload` is let through with `preventDefault()`, which in Electron means "unload anyway". -- **Bounded.** No answer within `DEFAULT_TIMEOUT_MS` (2.5 s), a crashed or destroyed renderer, or a failed send all answer yes: a hung renderer never keeps the app from quitting. A late answer is ignored. The edits of a renderer that is merely slow can be lost this way; that is the price of never deadlocking. +- **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). `autoUpdater.quitAndInstall()` is not shown to go through `before-quit` on every platform and is not covered. 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. - **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 tab: the shown one through the viewer's own save (`ViewerPanel.saveNow()`, with its stale-disk confirm), a tab kept aside through `saveFileForPanel` with its snapshot's `agreedBase`. 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 a file tab 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 tabs, 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. diff --git a/main.js b/main.js index 1e4de5ab..7977997c 100644 --- a/main.js +++ b/main.js @@ -40,7 +40,7 @@ const { classifyTitleActivity } = require('./classify-title-activity'); const { windowFrameOptions, applicationMenuTemplate, zoomKey, nextZoomLevel, menuPopupPoint } = require('./window-frame'); const { createWhatsNew } = require('./changelog'); const { createUnsavedGuard } = require('./unsaved-guard'); -const unsavedGuard = createUnsavedGuard({ ipcMain }); +const unsavedGuard = createUnsavedGuard({ ipcMain, quit: () => app.quit() }); const { cleanEnv } = require('./clean-env'); try { require('electron-reloader')(module, { watchRenderer: true }); } catch {}; @@ -3153,6 +3153,7 @@ app.on('window-all-closed', () => { // see .ai/contexts/activitywatch.md ("Quitting") app.on('before-quit', (event) => { + if (unsavedGuard.beforeQuit(event, mainWindow)) return; if (!activityFlushedForQuit && activityReporter.hasPendingWork) { event.preventDefault(); activityFlushedForQuit = true; diff --git a/preload.js b/preload.js index f58775cd..b6264322 100644 --- a/preload.js +++ b/preload.js @@ -145,6 +145,7 @@ contextBridge.exposeInMainWorld('api', { onUnsavedCheck: (callback) => { ipcRenderer.on('unsaved-check', (_event, id, reason) => callback(id, reason)); }, + unsavedCheckAck: (id) => ipcRenderer.send('unsaved-check-ack', id), unsavedCheckResult: (id, proceed) => ipcRenderer.send('unsaved-check-result', id, proceed), onFullScreenChanged: (callback) => { ipcRenderer.on('full-screen-changed', (_event, isFullScreen) => callback(isFullScreen)); diff --git a/public/file-panel.js b/public/file-panel.js index e4da6a92..f240624c 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -145,6 +145,7 @@ function initFilePanel() { }); if (window.api.onUnsavedCheck) { window.api.onUnsavedCheck(async (id) => { + window.api.unsavedCheckAck(id); let proceed = true; try { proceed = await askAboutUnsavedEdits(); } catch (err) { console.error('[unsaved-check]', err); } window.api.unsavedCheckResult(id, proceed); diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index 4c05d0b6..ff1bb086 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -41,12 +41,13 @@ 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, answers: [], saves: [], confirms: [] }; + const calls = { openFile: null, check: null, acks: [], answers: [], saves: [], confirms: [] }; let editor = null; window.api = new Proxy({ onMcpOpenFile: (cb) => { calls.openFile = cb; }, onUnsavedCheck: (cb) => { calls.check = 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 }); }, watchFile: () => Promise.resolve({ ok: true }), unwatchFile: () => Promise.resolve({ ok: true }), @@ -221,3 +222,15 @@ test('beforeunload blocks while a tab is dirty and lets go once it is saved', as assert.equal(beforeUnload(ctx).defaultPrevented, false); } finally { ctx.destroy(); } }); + +test('the check is acknowledged on receipt, before any dialog is shown', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A); + ctx.calls.check(9, 'quit'); + assert.deepEqual(ctx.calls.acks, [{ id: 9, at: 0, dialog: false }]); + await flush(); + button(ctx, 'unsaved-cancel').click(); + await flush(); + } finally { ctx.destroy(); } +}); diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index 2c05efb7..0ab39d21 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -9,7 +9,7 @@ const { EventEmitter } = require('node:events'); const { createUnsavedGuard } = require('../unsaved-guard'); -function setup({ timeoutMs = 1000 } = {}) { +function setup({ timeoutMs = 1000, quit } = {}) { const ipcMain = new EventEmitter(); const timers = []; const setTimeoutFn = (fn, ms) => { const t = { fn, ms, cleared: false }; timers.push(t); return t; }; @@ -25,12 +25,13 @@ function setup({ timeoutMs = 1000 } = {}) { }); const win = new EventEmitter(); Object.assign(win, { webContents: wc, isDestroyed: () => false, close: () => { win.closes += 1; }, closes: 0 }); - const guard = createUnsavedGuard({ ipcMain, timeoutMs, setTimeoutFn, clearTimeoutFn }); + const guard = createUnsavedGuard({ ipcMain, timeoutMs, setTimeoutFn, clearTimeoutFn, quit }); guard.attach(win); const closeEvent = () => ({ prevented: false, preventDefault() { this.prevented = true; } }); const answer = (id, proceed) => ipcMain.emit('unsaved-check-result', {}, id, proceed); - return { win, wc, sent, timers, closeEvent, answer, ipcMain }; + const ack = (id) => ipcMain.emit('unsaved-check-ack', {}, id); + return { win, wc, sent, timers, closeEvent, answer, ack, ipcMain, guard }; } const tick = () => new Promise((r) => setImmediate(r)); @@ -134,3 +135,83 @@ test('a blocked unload during an approved close is let through', () => { assert.equal(e.prevented, true, 'preventDefault on will-prevent-unload lets the unload proceed'); }); }); + +test('an acknowledged check waits for a slow human, past the bound', async () => { + const t = setup({ timeoutMs: 2500 }); + t.win.emit('close', t.closeEvent()); + const id = t.sent[0].args[0]; + t.ack(id); + assert.equal(t.timers[0].cleared, true, 'the ack ends the bound'); + await tick(); + assert.equal(t.win.closes, 0, 'still waiting for the user'); + t.answer(id, true); + await tick(); + assert.equal(t.win.closes, 1); +}); + +test('an acknowledged check gives up when the renderer process goes away', async () => { + const t = setup(); + t.win.emit('close', t.closeEvent()); + t.ack(t.sent[0].args[0]); + t.wc.emit('render-process-gone'); + await tick(); + assert.equal(t.win.closes, 1); +}); + +test('a reload approved by timeout reloads once, not in a loop', async () => { + const t = setup(); + t.wc.emit('will-prevent-unload', t.closeEvent()); + t.timers[0].fn(); + await tick(); + assert.equal(t.wc.reloads, 1); + const e = t.closeEvent(); + t.wc.emit('will-prevent-unload', e); + assert.equal(e.prevented, true, 'the reloaded page is let through'); + assert.equal(t.sent.length, 1, 'and is not asked again'); + t.wc.emit('will-prevent-unload', t.closeEvent()); + assert.equal(t.sent.length, 2, 'the allowance covers one unload only'); +}); + +function fakeApp(t, cleanup) { + const app = new EventEmitter(); + app.quitCalls = 0; + app.quit = () => { + app.quitCalls += 1; + const e = t.closeEvent(); + app.emit('before-quit', e); + if (e.prevented) return; + const c = t.closeEvent(); + t.win.emit('close', c); + if (c.prevented) return; + app.emit('will-quit'); + }; + app.on('before-quit', (e) => { + if (t.guard.beforeQuit(e, t.win)) return; + cleanup(); + }); + return app; +} + +test('quit cleanup runs only once the user has confirmed, and Cancel leaves everything alive', async () => { + let cleaned = 0; + let app; + const t = setup({ quit: () => app.quit() }); + app = fakeApp(t, () => { cleaned += 1; }); + let willQuit = 0; + app.on('will-quit', () => { willQuit += 1; }); + + app.quit(); + assert.equal(cleaned, 0, 'no cleanup while the question is open'); + t.ack(t.sent[0].args[0]); + t.answer(t.sent[0].args[0], false); + await tick(); + assert.equal(cleaned, 0, 'Cancel: PTYs and watchers untouched'); + assert.equal(willQuit, 0); + + app.quit(); + t.answer(t.sent[1].args[0], true); + await tick(); + assert.equal(cleaned, 1); + assert.equal(willQuit, 1); + assert.equal(t.sent.length, 2, 'the window close after approval asks nothing more'); +}); diff --git a/unsaved-guard.js b/unsaved-guard.js index dce3b43b..0361309d 100644 --- a/unsaved-guard.js +++ b/unsaved-guard.js @@ -4,16 +4,23 @@ const DEFAULT_TIMEOUT_MS = 2500; -function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeoutFn = setTimeout, clearTimeoutFn = clearTimeout }) { +function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeoutFn = setTimeout, clearTimeoutFn = clearTimeout, quit = () => {} }) { const pending = new Map(); let nextId = 1; + let quitApproved = false; + let quitAsking = false; + + ipcMain.on('unsaved-check-ack', (_event, id) => { + const entry = pending.get(id); + if (!entry || entry.acked) return; + entry.acked = true; + clearTimeoutFn(entry.timer); + }); ipcMain.on('unsaved-check-result', (_event, id, proceed) => { const entry = pending.get(id); if (!entry) return; - pending.delete(id); - clearTimeoutFn(entry.timer); - entry.resolve(proceed === true); + entry.finish(proceed === true); }); function ask(win, reason) { @@ -21,28 +28,50 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou if (win.isDestroyed() || !wc || wc.isDestroyed() || wc.isCrashed()) return Promise.resolve(true); return new Promise((resolve) => { const id = nextId++; - const timer = setTimeoutFn(() => { + const entry = { acked: false, timer: null, finish: null }; + const onGone = () => entry.finish(true); + entry.finish = (proceed) => { + if (!pending.has(id)) return; pending.delete(id); - resolve(true); - }, timeoutMs); - pending.set(id, { resolve, timer }); + clearTimeoutFn(entry.timer); + wc.removeListener('render-process-gone', onGone); + wc.removeListener('destroyed', onGone); + resolve(proceed); + }; + entry.timer = setTimeoutFn(() => entry.finish(true), timeoutMs); + pending.set(id, entry); + wc.on('render-process-gone', onGone); + wc.on('destroyed', onGone); try { wc.send('unsaved-check', id, reason); } catch { - pending.delete(id); - clearTimeoutFn(timer); - resolve(true); + entry.finish(true); } }); } + function beforeQuit(event, win) { + if (quitApproved || !win || win.isDestroyed()) return false; + event.preventDefault(); + if (quitAsking) return true; + quitAsking = true; + ask(win, 'quit').then((proceed) => { + quitAsking = false; + if (!proceed) return; + quitApproved = true; + quit(); + }); + return true; + } + function attach(win) { let approved = false; let closing = false; let reloading = false; + let allowNextUnload = false; win.on('close', (event) => { - if (approved) return; + if (approved || quitApproved) return; event.preventDefault(); if (closing) return; closing = true; @@ -55,7 +84,8 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou }); win.webContents.on('will-prevent-unload', (event) => { - if (approved) { + if (approved || quitApproved || allowNextUnload) { + allowNextUnload = false; event.preventDefault(); return; } @@ -63,12 +93,14 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou reloading = true; ask(win, 'reload').then((proceed) => { reloading = false; - if (proceed && !win.isDestroyed() && !win.webContents.isDestroyed()) win.webContents.reload(); + if (!proceed || win.isDestroyed() || win.webContents.isDestroyed()) return; + allowNextUnload = true; + win.webContents.reload(); }); }); } - return { attach }; + return { attach, beforeQuit }; } module.exports = { createUnsavedGuard, DEFAULT_TIMEOUT_MS }; From 29ae1178b3554a56b2b9e09898e883dc421c574a Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 22:55:52 +0200 Subject: [PATCH 3/3] (file-panel): ask before an update install, never at session end, one question at a time electron-updater starts the installer before it quits, so the unsaved-edits check now runs before quitAndInstall and a Cancel leaves the installer unstarted. A Windows session end approves the quit so logoff does not wait on a dialog. A window close, a quit and an install share one open question. Refs #373 --- .ai/contexts/viewer-panel.md | 2 +- main.js | 5 ++-- test/unsaved-guard.test.js | 44 ++++++++++++++++++++++++++++++++++++ unsaved-guard.js | 25 ++++++++++++++++++-- 4 files changed, 71 insertions(+), 5 deletions(-) diff --git a/.ai/contexts/viewer-panel.md b/.ai/contexts/viewer-panel.md index 2f7c4f61..6433027b 100644 --- a/.ai/contexts/viewer-panel.md +++ b/.ai/contexts/viewer-panel.md @@ -167,7 +167,7 @@ It is shown with `open(…, restore)` and re-read at once, like any return to th Nothing ends the window with a dirty file tab without the user saying so. The scope is the file tabs of the file panel (the shown one and `heldFileTabs`, 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). `autoUpdater.quitAndInstall()` is not shown to go through `before-quit` on every platform and is not covered. 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 `query-session-end` or `session-end` approves the quit, so logoff and shutdown never wait on the dialog. A window close, a quit and an install share one question while it is open. 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. - **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 tab: the shown one through the viewer's own save (`ViewerPanel.saveNow()`, with its stale-disk confirm), a tab kept aside through `saveFileForPanel` with its snapshot's `agreedBase`. 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 a file tab 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/main.js b/main.js index 7977997c..4e629987 100644 --- a/main.js +++ b/main.js @@ -2957,9 +2957,10 @@ ipcMain.handle('updater-download', () => { if (!autoUpdater) return; return autoUpdater.downloadUpdate(); }); -ipcMain.handle('updater-install', () => { - activityFlushedForQuit = true; // see .ai/contexts/activitywatch.md ("Quitting") +ipcMain.handle('updater-install', async () => { if (!autoUpdater) return; + if (mainWindow && !(await unsavedGuard.confirmQuit(mainWindow))) return; + activityFlushedForQuit = true; // see .ai/contexts/activitywatch.md ("Quitting") autoUpdater.quitAndInstall(); }); diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index 0ab39d21..bed9fe4c 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -215,3 +215,47 @@ test('quit cleanup runs only once the user has confirmed, and Cancel leaves ever assert.equal(willQuit, 1); assert.equal(t.sent.length, 2, 'the window close after approval asks nothing more'); }); + +test('an updater install asks before the installer starts: a no leaves it unstarted, a yes pre-approves the quit', async () => { + const t = setup(); + const first = t.guard.confirmQuit(t.win); + t.ack(t.sent[0].args[0]); + t.answer(t.sent[0].args[0], false); + assert.equal(await first, false); + + const second = t.guard.confirmQuit(t.win); + t.answer(t.sent[1].args[0], true); + assert.equal(await second, true); + const e2 = t.closeEvent(); + assert.equal(t.guard.beforeQuit(e2, t.win), false, 'the quit the installer triggers is not asked about again'); + assert.equal(e2.prevented, false); + const c = t.closeEvent(); + t.win.emit('close', c); + assert.equal(c.prevented, false); +}); + +test('a Windows session end approves the quit so logoff never waits on the dialog', () => { + for (const name of ['query-session-end', 'session-end']) { + const t = setup(); + t.win.emit(name, {}); + const e = t.closeEvent(); + assert.equal(t.guard.beforeQuit(e, t.win), false, name); + const c = t.closeEvent(); + t.win.emit('close', c); + assert.equal(c.prevented, false, name); + assert.equal(t.sent.length, 0, name); + } +}); + +test('a window close and a quit share one question, and one answer settles both', async () => { + const t = setup(); + t.win.emit('close', t.closeEvent()); + const e = t.closeEvent(); + assert.equal(t.guard.beforeQuit(e, t.win), true); + assert.equal(t.sent.length, 1, 'one dialog, not two'); + t.answer(t.sent[0].args[0], true); + await tick(); + assert.equal(t.win.closes, 1); + const again = t.closeEvent(); + assert.equal(t.guard.beforeQuit(again, t.win), false, 'the same yes approved the quit'); +}); diff --git a/unsaved-guard.js b/unsaved-guard.js index 0361309d..5958f90f 100644 --- a/unsaved-guard.js +++ b/unsaved-guard.js @@ -9,6 +9,7 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou let nextId = 1; let quitApproved = false; let quitAsking = false; + let inflight = null; ipcMain.on('unsaved-check-ack', (_event, id) => { const entry = pending.get(id); @@ -24,9 +25,11 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou }); function ask(win, reason) { + if (inflight) return inflight; const wc = win.webContents; if (win.isDestroyed() || !wc || wc.isDestroyed() || wc.isCrashed()) return Promise.resolve(true); - return new Promise((resolve) => { + let settled = false; + const asked = new Promise((resolve) => { const id = nextId++; const entry = { acked: false, timer: null, finish: null }; const onGone = () => entry.finish(true); @@ -36,6 +39,8 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou clearTimeoutFn(entry.timer); wc.removeListener('render-process-gone', onGone); wc.removeListener('destroyed', onGone); + settled = true; + inflight = null; resolve(proceed); }; entry.timer = setTimeoutFn(() => entry.finish(true), timeoutMs); @@ -48,6 +53,8 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou entry.finish(true); } }); + if (!settled) inflight = asked; + return asked; } function beforeQuit(event, win) { @@ -70,6 +77,9 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou let reloading = false; let allowNextUnload = false; + win.on('query-session-end', approveQuit); + win.on('session-end', approveQuit); + win.on('close', (event) => { if (approved || quitApproved) return; event.preventDefault(); @@ -100,7 +110,18 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou }); } - return { attach, beforeQuit }; + function confirmQuit(win) { + return ask(win, 'quit').then((proceed) => { + if (proceed) quitApproved = true; + return proceed; + }); + } + + function approveQuit() { + quitApproved = true; + } + + return { attach, beforeQuit, confirmQuit, approveQuit }; } module.exports = { createUnsavedGuard, DEFAULT_TIMEOUT_MS };