diff --git a/.ai/contexts/viewer-panel.md b/.ai/contexts/viewer-panel.md index 158361b4..6433027b 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, `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. +- **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 50c417cd..068bd1cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ 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) - The IDE Emulation label in a session's terminal header now says whether the CLI is connected: it reads "IDE Emulation" only while it is, "IDE Emulation: waiting for CLI" when Switchboard is listening but the CLI has not connected, and "IDE Emulation: failed" when it could not start for that session, with the reason in its tooltip. A session whose IDE Emulation port was already taken no longer shows the label as if it worked. (#320) - On Windows, the file panel no longer opens or saves a credential file (such as one under `.ssh`) through its 8.3 short name or a `\\?\` path. (#390) ### New diff --git a/main.js b/main.js index 5a691823..06ced8d3 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, quit: () => app.quit() }); const { cleanEnv } = require('./clean-env'); try { require('electron-reloader')(module, { watchRenderer: true }); } catch {}; @@ -357,6 +359,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. @@ -3025,9 +3029,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(); }); @@ -3221,6 +3226,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 850110f1..f6f473f2 100644 --- a/preload.js +++ b/preload.js @@ -143,6 +143,11 @@ contextBridge.exposeInMainWorld('api', { onIndexingFinished: (callback) => { ipcRenderer.on('indexing-finished', () => callback()); }, + 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 72715323..e863c95b 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -139,6 +139,20 @@ 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) => { + window.api.unsavedCheckAck(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'; @@ -566,6 +580,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 9655c70f..f84eea81 100644 --- a/public/style.css +++ b/public/style.css @@ -5163,3 +5163,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..ff1bb086 --- /dev/null +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -0,0 +1,236 @@ +'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, 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 }), + 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(); } +}); + +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 new file mode 100644 index 00000000..bed9fe4c --- /dev/null +++ b/test/unsaved-guard.test.js @@ -0,0 +1,261 @@ +'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, quit } = {}) { + 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, quit }); + guard.attach(win); + + 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 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'); + }); +}); + +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'); +}); + +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 new file mode 100644 index 00000000..5958f90f --- /dev/null +++ b/unsaved-guard.js @@ -0,0 +1,127 @@ +'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, quit = () => {} }) { + const pending = new Map(); + let nextId = 1; + let quitApproved = false; + let quitAsking = false; + let inflight = null; + + 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; + entry.finish(proceed === true); + }); + + function ask(win, reason) { + if (inflight) return inflight; + const wc = win.webContents; + if (win.isDestroyed() || !wc || wc.isDestroyed() || wc.isCrashed()) return Promise.resolve(true); + let settled = false; + const asked = new Promise((resolve) => { + const id = nextId++; + const entry = { acked: false, timer: null, finish: null }; + const onGone = () => entry.finish(true); + entry.finish = (proceed) => { + if (!pending.has(id)) return; + pending.delete(id); + 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); + pending.set(id, entry); + wc.on('render-process-gone', onGone); + wc.on('destroyed', onGone); + try { + wc.send('unsaved-check', id, reason); + } catch { + entry.finish(true); + } + }); + if (!settled) inflight = asked; + return asked; + } + + 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('query-session-end', approveQuit); + win.on('session-end', approveQuit); + + win.on('close', (event) => { + if (approved || quitApproved) 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 || quitApproved || allowNextUnload) { + allowNextUnload = false; + event.preventDefault(); + return; + } + if (reloading) return; + reloading = true; + ask(win, 'reload').then((proceed) => { + reloading = false; + if (!proceed || win.isDestroyed() || win.webContents.isDestroyed()) return; + allowNextUnload = true; + win.webContents.reload(); + }); + }); + } + + 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 };