From 2e1d39d8f9621d6f884177575a769fc6b72421b7 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 12 Oct 2026 13:21:00 +0200 Subject: [PATCH 01/10] (ui): confirmations and messages use the app's own dialog; native ones are banned (#585) Native confirm/alert looked foreign, blocked the renderer and were auto-dismissed by Playwright, so e2e could not drive them. appConfirm and appAlert (public/app-dialogs.js) build on showChoiceDialog, queue instead of dropping a second dialog, focus the safe button on a destructive confirm and give focus back to the opener. Every renderer call site is migrated, injected confirms in resume-guard are awaited, and ESLint rejects confirm, alert and prompt under public/. --- .ai/contexts/app-dialogs.md | 11 +++ CHANGELOG.md | 1 + e2e/app-dialogs.spec.js | 36 ++++++++ eslint.config.js | 6 ++ public/activity-trace-panel.js | 6 +- public/agents-view.js | 2 +- public/app-dialogs.js | 18 ++++ public/app.js | 8 +- public/choice-dialog.js | 63 ++++++++++--- public/file-panel.js | 92 +++++++++++-------- public/index.html | 1 + public/resume-guard.js | 12 +-- public/sidebar.js | 18 ++-- public/touched-files-view.js | 10 +- public/viewer-panel.js | 30 +++--- test/app-dialogs-lint.test.js | 16 ++++ test/confirm-and-stop-session.test.js | 19 +++- test/continuations-harness.js | 2 +- test/dom-activity-trace-panel.test.js | 14 ++- test/dom-agents-view.test.js | 36 +++++++- test/dom-app-dialogs.test.js | 106 ++++++++++++++++++++++ test/dom-choice-dialog.test.js | 4 +- test/dom-file-panel-changes.test.js | 18 +++- test/dom-file-panel-diff-save.test.js | 6 +- test/dom-file-panel-goto-line.test.js | 2 +- test/dom-file-panel-touched.test.js | 9 +- test/dom-file-panel-unsaved-guard.test.js | 67 +++++++++++++- test/dom-project-archive-folder.test.js | 10 +- test/dom-setup.js | 1 + test/dom-sidebar-dialogs.test.js | 69 ++++++++++++++ test/dom-sidebar-merged-members.test.js | 2 +- test/keyboard-focus.test.js | 1 + test/open-session-merged-members.test.js | 2 +- test/open-session-once.test.js | 4 +- test/panel-terminal.test.js | 6 +- test/restore-live-elsewhere.test.js | 4 +- test/restore-merged-member.test.js | 2 +- test/resume-guard.test.js | 4 +- test/schedule-live-resume.test.js | 2 +- test/viewer-panel-external-change.test.js | 94 ++++++++++++++++++- 40 files changed, 682 insertions(+), 132 deletions(-) create mode 100644 .ai/contexts/app-dialogs.md create mode 100644 e2e/app-dialogs.spec.js create mode 100644 public/app-dialogs.js create mode 100644 test/app-dialogs-lint.test.js create mode 100644 test/dom-app-dialogs.test.js create mode 100644 test/dom-sidebar-dialogs.test.js diff --git a/.ai/contexts/app-dialogs.md b/.ai/contexts/app-dialogs.md new file mode 100644 index 00000000..4e614511 --- /dev/null +++ b/.ai/contexts/app-dialogs.md @@ -0,0 +1,11 @@ +# Renderer confirmations and messages + +Use async `appConfirm({ title, message, confirmLabel, danger })` and `appAlert({ title, message })` from `public/app-dialogs.js`. Native `confirm`, `alert`, and `prompt` (bare or on `window`) are forbidden by ESLint throughout `public/**`. + +The adapter uses `showChoiceDialog`. All choice dialogs share a FIFO queue, including existing checkbox dialogs, so a busy dialog never becomes a cancellation. Confirm resolves to a boolean; an alert offers OK. Escape, the close button, and the backdrop dismiss the current dialog. Enter activates the focused button and never accepts a checkbox. Tab stays inside the dialog. Destructive confirmations focus Cancel; other dialogs focus the first action. Closing returns focus to the connected opener. + +Every caller must await the decision before its side effect. The Changes and Touched discard paths are async, including close, back, row switch, toggle, and reload. The quit/reload IPC already awaits `askAboutUnsavedEdits`, acknowledges immediately, and returns the decision only after the existing Save/Discard/Cancel guard. The beforeunload veto and main-process protocol are unchanged. + +Main-process Electron dialogs remain outside this renderer rule. + +The quit/reload guard ACKs immediately, then waits on `appDialogsIdle()` before showing its Save/Discard/Cancel controls and recollects the current buffers. It does not hold the choice queue while saving; a foreground message can therefore be dismissed without deadlock. Each dialog owns keys originating in its overlay. The unsaved guard focuses Cancel, traps Tab, and restores its connected opener. diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f5be70c..bc49a964 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc - **Archive Project** now keeps your settings and suspends schedules until you restore it from Settings → **Archived projects**. A separate **Delete Project** confirms the removal of settings, cache and schedule registration, while keeping your folders and files. (#568) - Markdown previews now show a file's frontmatter as a readable header above its content, including its name, description and metadata. (#593) - Closing the window (the close button, `Alt`+`F4`) asks for confirmation first, with Cancel selected; a window that is busy at that moment asks once it is free. Quitting from the ☰ menu (Quit, or Exit on Windows) does not ask again, and a quit from outside (a logout, an update install) is not stopped by the question. On macOS it asks "Close the window?": Switchboard stays in the Dock, but its sessions stop. If the window has stopped responding, a single close waits, and closing it again a few seconds later ends it; never while one of its own confirmation boxes waits for an answer. A window closed while it is still starting asks once it has loaded. (#478) +- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. (#585) ### Fixed - A session sent to the background is reattached at every restart, not only the first. (#546) diff --git a/e2e/app-dialogs.spec.js b/e2e/app-dialogs.spec.js new file mode 100644 index 00000000..47ef968e --- /dev/null +++ b/e2e/app-dialogs.spec.js @@ -0,0 +1,36 @@ +const { test, expect, makeRepo } = require('./fixtures'); + +test('background Delete is a designed confirmation: Escape cancels and Delete proceeds', async ({ home, env, launch }) => { + makeRepo(home, env, { 'dialog.txt': 'fixture\n' }); + const { app, page } = await launch(); + const native = []; + page.on('dialog', dialog => { native.push(dialog.type()); dialog.dismiss(); }); + await app.evaluate(({ ipcMain }) => { + globalThis.dialogDeletes = []; + ipcMain.removeHandler('get-bg-agents'); + ipcMain.handle('get-bg-agents', () => ({ daemonReachable: true, roster: [ + { id: 'aaaaaaaa', sessionId: 'dialog-session', name: 'Dialog fixture', cwd: '/fixture', kind: 'background', state: 'done', attachedHere: false }, + ] })); + ipcMain.removeHandler('bg-agent-verb'); + ipcMain.handle('bg-agent-verb', (_event, verb, id) => { + globalThis.dialogDeletes.push([verb, id]); + return { ok: true }; + }); + }); + await page.locator('#agents-toggle-btn').click(); + await page.locator('.agents-row[data-key="bg:aaaaaaaa"]').click(); + const opener = page.locator('#agents-detail [data-verb="rm"]'); + await opener.focus(); + await page.keyboard.press('Enter'); + const dialog = page.locator('.choice-dialog-overlay'); + await expect(dialog).toBeVisible(); + await expect(dialog.locator('.choice-dialog-cancel')).toBeFocused(); + await page.keyboard.press('Escape'); + await expect(dialog).toHaveCount(0); + await expect(opener).toBeFocused(); + expect(await app.evaluate(() => globalThis.dialogDeletes)).toEqual([]); + await opener.click(); + await dialog.getByRole('button', { name: 'Delete', exact: true }).click(); + await expect.poll(() => app.evaluate(() => globalThis.dialogDeletes)).toEqual([['rm', 'aaaaaaaa']]); + expect(native).toEqual([]); +}); diff --git a/eslint.config.js b/eslint.config.js index 199fe494..5325ce1a 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -124,6 +124,10 @@ const rendererCrossFileGlobals = { shortProjectPath: 'readonly', // public/choice-dialog.js and public/worktree-nesting.js showChoiceDialog: 'readonly', + appConfirm: 'readonly', + appAlert: 'readonly', + appDialogsIdle: 'readonly', + whenChoiceDialogsIdle: 'readonly', SIDEBAR_WORKTREE_RE: 'readonly', worktreeParentPath: 'readonly', worktreeName: 'readonly', @@ -415,6 +419,8 @@ module.exports = [ rules: { 'no-undef': 'error', 'no-unused-vars': ['warn', { args: 'none', varsIgnorePattern: '^_' }], + 'no-restricted-globals': ['error', ...['confirm', 'alert', 'prompt'].map(name => ({ name, message: 'Use public/app-dialogs.js (appConfirm/appAlert).' }))], + 'no-restricted-properties': ['error', ...['confirm', 'alert', 'prompt'].map(property => ({ object: 'window', property, message: 'Use public/app-dialogs.js (appConfirm/appAlert).' }))], 'no-implicit-globals': 'off', // renderer relies on script-scope globals by design 'no-empty': ['warn', { allowEmptyCatch: true }], 'no-unreachable': 'warn', diff --git a/public/activity-trace-panel.js b/public/activity-trace-panel.js index 5eddd9ca..f1ab1cfe 100644 --- a/public/activity-trace-panel.js +++ b/public/activity-trace-panel.js @@ -67,7 +67,7 @@ async function openActivityTraceFile(file) { result = { ok: false, error: (err && err.message) || String(err) }; } if (!result || !result.ok) { - window.alert(`Could not read ${file.name}: ${(result && result.error) || 'unknown error'}`); + await appAlert({ title: 'Switchboard', message: `Could not read ${file.name}: ${(result && result.error) || 'unknown error'}` }); return; } if (typeof hideAllViewers === 'function') hideAllViewers(); @@ -117,7 +117,7 @@ function buildActivityTraceRow(file, listEl) { deleteBtn.className = 'settings-check-updates-btn activity-trace-delete-btn'; deleteBtn.textContent = 'Delete'; deleteBtn.addEventListener('click', async () => { - if (!window.confirm(`Delete "${file.name}"?\n\nThis cannot be undone.`)) return; + if (!await appConfirm({ title: 'Delete?', message: `Delete "${file.name}"?\n\nThis cannot be undone.`, confirmLabel: 'Delete', danger: true })) return; deleteBtn.disabled = true; let result; try { @@ -130,7 +130,7 @@ function buildActivityTraceRow(file, listEl) { if (result && result.ok) { renderActivityTraceFiles(listEl); } else { - window.alert(`Delete failed: ${(result && result.error) || 'unknown error'}`); + await appAlert({ title: 'Switchboard', message: `Delete failed: ${(result && result.error) || 'unknown error'}` }); } }); diff --git a/public/agents-view.js b/public/agents-view.js index 7f917054..5bf7f83d 100644 --- a/public/agents-view.js +++ b/public/agents-view.js @@ -418,7 +418,7 @@ async function runAgentVerb(verb, entry) { attachBgAgent(entry); return; } - if (verb === 'rm' && !window.confirm('Delete this background session? Its conversation goes, and its worktree when that is safe.')) return; + if (verb === 'rm' && !await appConfirm({ title: 'Delete?', message: 'Delete this background session? Its conversation goes, and its worktree when that is safe.', confirmLabel: 'Delete', danger: true })) return; agentsPendingVerbs.add(key); agentsVerbErrors.delete(key); if (agentsViewActive) renderAgentsView(); diff --git a/public/app-dialogs.js b/public/app-dialogs.js new file mode 100644 index 00000000..cf15dc65 --- /dev/null +++ b/public/app-dialogs.js @@ -0,0 +1,18 @@ +// see .ai/contexts/app-dialogs.md +async function appConfirm({ title = 'Confirm', message, confirmLabel = 'OK', danger = false } = {}) { + return (await showChoiceDialog({ + title, message: appDialogMessage(message), confirmLabel, danger, + initialFocus: danger ? 'cancel' : 'confirm', + })) !== null; +} +async function appAlert({ title = 'Switchboard', message } = {}) { + await showChoiceDialog({ title, message: appDialogMessage(message), confirmLabel: 'OK', cancelLabel: null }); +} + +function appDialogsIdle() { + return whenChoiceDialogsIdle(); +} + +function appDialogMessage(message) { + return typeof message === 'string' ? message.split('\n') : message; +} diff --git a/public/app.js b/public/app.js index 54544631..e1b5df64 100644 --- a/public/app.js +++ b/public/app.js @@ -306,7 +306,7 @@ async function runRestore(list, { retryAfterIndexing = !restoreIndexingDone } = const originalId = item.sessionId; const originalSession = s; let waitingForIndex = false; - s = await resolveResumeSession({ ...s, projectPath: item.projectPath, mergedMemberIds: [] }, { automatic: true, api: window.api, confirm: msg => window.confirm(msg), resolutions: window._startupResumeResolutions, onHold: reason => { waitingForIndex = reason.waitingForIndex; } }); + s = await resolveResumeSession({ ...s, projectPath: item.projectPath, mergedMemberIds: [] }, { automatic: true, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session', danger: true }), resolutions: window._startupResumeResolutions, onHold: reason => { waitingForIndex = reason.waitingForIndex; } }); if (!s) { const index = restoreSavedIndex.has(originalId) ? restoreSavedIndex.get(originalId) : position; skippedWorkingSetEntries.set(originalId, { item, index, retryContinuation: retryAfterIndexing }); @@ -1054,7 +1054,7 @@ async function triggerRebuildAndSearch() { async function confirmAndStopSession(sessionId, btn, rowSession) { const openEntry = openSessions.get(sessionId); const plan = resolveSessionStop(sessionMap.get(sessionId) || openEntry?.session || rowSession, { attach: !!(openEntry && openEntry.attach) }); - if (!confirm(plan.confirmText)) return; + if (!await appConfirm({ title: plan.attach ? 'Detach session?' : 'Stop session?', message: plan.confirmText, confirmLabel: plan.attach ? 'Detach' : 'Stop', danger: true })) return; const result = plan.remote ? await window.api.remoteStopSession(plan.alias, sessionId) : await window.api.stopSession(sessionId); @@ -1520,7 +1520,7 @@ async function openSessionNow(session, customOptions, { automatic = false, live, } if (!continuationResolved && customOptions?.type !== 'attach' && (!openSessions.has(session.sessionId) || openSessions.get(session.sessionId).closed)) { const originalId = session.sessionId; - session = await resolveResumeSession(session, { automatic, api: window.api, confirm: msg => window.confirm(msg), resolutions: window._startupResumeResolutions }); + session = await resolveResumeSession(session, { automatic, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session', danger: true }), resolutions: window._startupResumeResolutions }); if (!session) return false; if (session.sessionId !== originalId) { live = undefined; @@ -1552,7 +1552,7 @@ async function openSessionNow(session, customOptions, { automatic = false, live, } // see .ai/contexts/cli-session-state.md ("Live elsewhere") and .ai/contexts/bg-agents.md ("Attach") - const verdict = customOptions?.type === 'attach' ? true : await guardResume(session, { automatic, live, allowBgAttach, api: window.api, confirm: (msg) => window.confirm(msg) }); + const verdict = customOptions?.type === 'attach' ? true : await guardResume(session, { automatic, live, allowBgAttach, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session', danger: true }) }); if (verdict === false) return false; if (verdict && typeof verdict === 'object' && verdict.attach) { customOptions = { type: 'attach', jobId: verdict.attach, cwd: verdict.cwd || projectPath }; diff --git a/public/choice-dialog.js b/public/choice-dialog.js index dc694ce9..d433416b 100644 --- a/public/choice-dialog.js +++ b/public/choice-dialog.js @@ -8,15 +8,49 @@ function writeRememberedChoice(rememberKey, checked) { try { localStorage.setItem(rememberKey, checked ? '1' : '0'); } catch {} } +// see .ai/contexts/app-dialogs.md +const choiceDialogQueue = []; +const choiceDialogIdleWaiters = []; +let choiceDialogActive = false; +let choiceDialogReturnFocus = null; let closeOpenChoiceDialog = null; -/** - * A modal with optional checkboxes. Resolves to `{ [choice.id]: boolean }` on - * confirm, null on cancel, and null at once while another one is open, unless - * `replace` cancels the open one and shows this one. `closeWith`, a promise, - * closes the dialog with the value it resolves to. - */ -function showChoiceDialog({ +function showChoiceDialog(options = {}) { + const active = document.activeElement; + const returnFocus = options.returnFocus || (active?.closest('.choice-dialog-overlay') ? choiceDialogReturnFocus : active); + return new Promise((resolve) => { + const entry = { options: { ...options, returnFocus }, resolve }; + if (options.replace && closeOpenChoiceDialog) { + choiceDialogQueue.unshift(entry); + closeOpenChoiceDialog(null); + } else choiceDialogQueue.push(entry); + drainChoiceDialogs(); + }); +} + +function whenChoiceDialogsIdle() { + if (!choiceDialogActive && !choiceDialogQueue.length) return Promise.resolve(); + return new Promise(resolve => choiceDialogIdleWaiters.push(resolve)); +} + +function drainChoiceDialogs() { + if (choiceDialogActive) return; + if (!choiceDialogQueue.length) { + for (const resolve of choiceDialogIdleWaiters.splice(0)) resolve(); + return; + } + choiceDialogActive = true; + const { options, resolve } = choiceDialogQueue.shift(); + if (options.replace && options.returnFocus && options.returnFocus.isConnected === false) options.returnFocus = document.activeElement; + choiceDialogReturnFocus = options.returnFocus; + openChoiceDialog(options).then((value) => { + choiceDialogActive = false; + resolve(value); + drainChoiceDialogs(); + }); +} + +function openChoiceDialog({ title, message, choices = [], @@ -28,11 +62,7 @@ function showChoiceDialog({ replace = false, closeWith = null, } = {}) { - if (document.querySelector('.choice-dialog-overlay')) { - if (!replace || !closeOpenChoiceDialog) return Promise.resolve(null); - closeOpenChoiceDialog(null); - if (returnFocus && returnFocus.isConnected === false) returnFocus = document.activeElement; - } + return new Promise((resolve) => { const overlay = document.createElement('div'); @@ -86,6 +116,7 @@ function showChoiceDialog({ cancelBtn.type = 'button'; cancelBtn.className = 'new-session-cancel-btn choice-dialog-cancel'; cancelBtn.textContent = cancelLabel; + cancelBtn.hidden = cancelLabel === null; const confirmBtn = document.createElement('button'); confirmBtn.type = 'button'; confirmBtn.className = (danger ? 'delete-worktree-confirm-btn' : 'new-session-start-btn') + ' choice-dialog-confirm'; @@ -101,9 +132,10 @@ function showChoiceDialog({ function close(value) { if (closed) return; closed = true; + if (closeOpenChoiceDialog === close) closeOpenChoiceDialog = null; overlay.remove(); document.removeEventListener('keydown', onKey, true); - if (returnFocus && typeof returnFocus.focus === 'function') returnFocus.focus(); + if (returnFocus?.isConnected && typeof returnFocus.focus === 'function') returnFocus.focus(); resolve(value); } @@ -117,13 +149,14 @@ function showChoiceDialog({ } function onKey(event) { + if (!overlay.contains(event.target)) return; if (event.key === 'Escape') { event.preventDefault(); event.stopImmediatePropagation(); close(null); } else if (event.key === 'Tab') { event.preventDefault(); - const controls = [...dialog.querySelectorAll('button, input')]; + const controls = [...dialog.querySelectorAll('button, input')].filter(control => !control.hidden); const at = controls.indexOf(document.activeElement); const step = event.shiftKey ? -1 : 1; const next = at === -1 ? (event.shiftKey ? controls.length - 1 : 0) : (at + step + controls.length) % controls.length; @@ -142,9 +175,9 @@ function showChoiceDialog({ overlay.addEventListener('click', (event) => { if (event.target === overlay) close(null); }); document.body.appendChild(overlay); + document.addEventListener('keydown', onKey, true); closeOpenChoiceDialog = close; if (closeWith) closeWith.then(close, () => {}); - document.addEventListener('keydown', onKey, true); (initialFocus === 'cancel' ? cancelBtn : confirmBtn).focus(); }); } diff --git a/public/file-panel.js b/public/file-panel.js index 29ab885b..09ec54f0 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -138,10 +138,9 @@ function initFilePanel() { if (event.defaultPrevented || event.isComposing) return; if (event.target.closest('.cm-panels, .cm-search, .cm-tooltip')) return; if (event.target.closest('input, textarea') && !event.target.closest('.cm-content')) return; - if (returnToPanelList()) { - event.preventDefault(); - event.stopPropagation(); - } + event.preventDefault(); + event.stopPropagation(); + returnToPanelList(); }); window.addEventListener('beforeunload', (event) => { @@ -337,16 +336,17 @@ function initFilePanel() { // ── Handlers ──────────────────────────────────────────────────────── -function handleClose() { - if (!currentPanelSessionId) return; - const state = getSessionState(currentPanelSessionId); +async function handleClose() { + const sessionId = currentPanelSessionId; + if (!sessionId) return; + const state = getSessionState(sessionId); const tab = state.currentTab; let pending = null; if (tab) { - if (!confirmDiscardChangesEdits(tab)) return; + if ((hasUnsavedChangesEdits(tab) && !await confirmDiscardChangesEdits(tab))) return; if (tab.type === 'diff' && !tab.resolved) { - window.api.mcpDiffResponse(currentPanelSessionId, tab.diffId, 'reject', null); + window.api.mcpDiffResponse(sessionId, tab.diffId, 'reject', null); } if (tab.type === 'diff') { pending = tab.pendingTouchedOpen || null; @@ -357,29 +357,30 @@ function handleClose() { tab.editorView = null; } if (tab.type === 'changes') { - unwatchChangesFile(currentPanelSessionId, tab); + unwatchChangesFile(sessionId, tab); destroyChangesEditor(tab); } state.currentTab = null; } - endCurrentTab(currentPanelSessionId, state, { pending }); + endCurrentTab(sessionId, state, { pending }); } -function returnToPanelList() { - if (!currentPanelSessionId) return false; - const state = getSessionState(currentPanelSessionId); +async function returnToPanelList() { + const sessionId = currentPanelSessionId; + if (!sessionId) return false; + const state = getSessionState(sessionId); const tab = state.currentTab; if (tab?.type === 'changes' && tab.selectedFile && !tab.returnList) { - closeChangesDiff(currentPanelSessionId); + await closeChangesDiff(sessionId); return !tab.selectedFile; } if (!tab?.returnList) return false; - if (!confirmDiscardChangesEdits(tab)) return false; + if ((hasUnsavedChangesEdits(tab) && !await confirmDiscardChangesEdits(tab))) return false; const previous = tab.returnList; destroyCurrentTab(state, { stash: false }); state.currentTab = previous; - renderPanel(currentPanelSessionId); + if (currentPanelSessionId === sessionId) renderPanel(sessionId); return true; } @@ -454,8 +455,7 @@ async function saveDiffTab(tab) { try { let result = await window.api.saveFileForPanel(tab.filePath, content, tab.diskBaseline); while (result && result.reason === 'stale' && typeof result.disk === 'string' - && typeof window.confirm === 'function' - && window.confirm('This file changed on disk since the diff opened. Overwrite it with your edits?')) { + && await appConfirm({ title: 'Overwrite?', message: 'This file changed on disk since the diff opened. Overwrite it with your edits?', confirmLabel: 'Overwrite', danger: true })) { tab.diskBaseline = result.disk; result = await window.api.saveFileForPanel(tab.filePath, content, tab.diskBaseline); } @@ -464,11 +464,11 @@ async function saveDiffTab(tab) { tab.diskBaseline = content.replace(/\r\n?/g, '\n'); const btn = diffToolbarEl.querySelector('.fp-save-btn'); if (btn) flashButtonText(btn, 'Saved!'); - } else if (result && result.reason !== 'stale' && typeof window.alert === 'function') { - window.alert(`Save failed: ${result.error || 'unknown error'}`); + } else if (result && result.reason !== 'stale') { + await appAlert({ title: 'Switchboard', message: `Save failed: ${result.error || 'unknown error'}` }); } } catch (err) { - if (typeof window.alert === 'function') window.alert(`Save failed: ${(err && err.message) || 'unknown error'}`); + await appAlert({ title: 'Switchboard', message: `Save failed: ${(err && err.message) || 'unknown error'}` }); } finally { tab.saving = false; const queued = tab.saveQueued; @@ -705,16 +705,28 @@ function showUnsavedEditsDialog(tabs) { dialog.appendChild(actions); overlay.appendChild(dialog); document.body.appendChild(overlay); - saveBtn.focus(); + cancelBtn.focus(); function finish(proceed) { overlay.remove(); document.removeEventListener('keydown', onKey); - if (!proceed && returnFocus && typeof returnFocus.focus === 'function') returnFocus.focus(); + if (returnFocus?.isConnected) returnFocus.focus(); resolve(proceed); } function onKey(e) { - if (e.key === 'Escape') finish(false); + if (!overlay.contains(e.target)) return; + if (e.key === 'Escape') { + e.preventDefault(); + e.stopImmediatePropagation(); + finish(false); + } else if (e.key === 'Tab') { + e.preventDefault(); + const controls = [cancelBtn, discardBtn, saveBtn].filter(button => !button.disabled); + if (!controls.length) return; + const at = controls.indexOf(document.activeElement); + const step = e.shiftKey ? -1 : 1; + controls[(at + step + controls.length) % controls.length].focus(); + } } document.addEventListener('keydown', onKey); @@ -776,7 +788,11 @@ function askAboutUnsavedEdits() { if (unsavedPrompt) return unsavedPrompt; const tabs = collectUnsavedFileTabs(); if (!tabs.length) return Promise.resolve(true); - unsavedPrompt = showUnsavedEditsDialog(tabs).then((proceed) => { + unsavedPrompt = (async () => { + if (typeof appDialogsIdle === 'function') await appDialogsIdle(); + const currentTabs = collectUnsavedFileTabs(); + return currentTabs.length ? showUnsavedEditsDialog(currentTabs) : true; + })().then((proceed) => { unsavedPrompt = null; if (proceed) { unloadApproved = true; @@ -1147,10 +1163,10 @@ function handleDiffAction(sessionId, tab, action) { // ── Changes Mode — see .ai/contexts/changes-view.md ────────────────── -function toggleChangesTab(sessionId) { +async function toggleChangesTab(sessionId) { const state = getSessionState(sessionId); if (state.currentTab && state.currentTab.type === 'changes' && !state.currentTab.returnList) { - if (!confirmDiscardChangesEdits(state.currentTab)) return; + if ((hasUnsavedChangesEdits(state.currentTab) && !await confirmDiscardChangesEdits(state.currentTab))) return; destroyCurrentTab(state, { stash: false }); state.currentTab = null; endCurrentTab(sessionId, state); @@ -1366,7 +1382,7 @@ async function openChangesDiff(sessionId, file) { if (!state || !state.currentTab || state.currentTab.type !== 'changes') return; const tab = state.currentTab; - if (tab.selectedFile && !isSelectedChangesRow(tab, file) && !confirmDiscardChangesEdits(tab)) return; + if (tab.selectedFile && !isSelectedChangesRow(tab, file) && (hasUnsavedChangesEdits(tab) && !await confirmDiscardChangesEdits(tab))) return; if (!tab.selectedFile && currentPanelSessionId === sessionId) snapshotPanelList(changesListEl, tab); tab.selectedFile = file; @@ -1471,15 +1487,15 @@ function applyUntrackedCounts(tab, expectedData, filePath, added, deleted, count tab.listRevision = (tab.listRevision || 0) + 1; } -function closeChangesDiff(sessionId) { +async function closeChangesDiff(sessionId) { const state = filePanelState.get(sessionId); if (!state || !state.currentTab || state.currentTab.type !== 'changes') return; const tab = state.currentTab; if (tab.returnList) { - if (currentPanelSessionId === sessionId) returnToPanelList(); + if (currentPanelSessionId === sessionId) await returnToPanelList(); return; } - if (!confirmDiscardChangesEdits(tab)) return; + if ((hasUnsavedChangesEdits(tab) && !await confirmDiscardChangesEdits(tab))) return; unwatchChangesFile(sessionId, tab); destroyChangesEditor(tab); tab.selectedFile = null; @@ -1503,11 +1519,13 @@ function hasUnsavedChangesEdits(tab) { return !!tab && tab.type === 'changes' && isChangesBufferDirty(tab); } -// Asks, and does nothing else. -function confirmDiscardChangesEdits(tab) { +// see .ai/contexts/app-dialogs.md +async function confirmDiscardChangesEdits(tab) { if (!hasUnsavedChangesEdits(tab)) return true; - if (typeof window.confirm !== 'function') return true; - return window.confirm('This file has unsaved edits. Discard them?'); + const state = [...filePanelState.values()].find(state => state.currentTab === tab); + const content = readChangesEditorContent(tab); + const confirmed = await appConfirm({ title: 'Discard edits?', message: 'This file has unsaved edits. Discard them?', confirmLabel: 'Discard', danger: true }); + return confirmed && (!state || state.currentTab === tab) && readChangesEditorContent(tab) === content; } function renderChangesContent(sessionId, tab) { @@ -2158,7 +2176,7 @@ async function reloadChangesFile(sessionId) { const state = filePanelState.get(sessionId); const tab = state && state.currentTab; if (!tab || tab.type !== 'changes' || !tab.editable || !tab.selectedFile) return; - if (!confirmDiscardChangesEdits(tab)) return; + if ((hasUnsavedChangesEdits(tab) && !await confirmDiscardChangesEdits(tab))) return; const file = tab.selectedFile; const result = await readChangesPair(sessionId, tab, file); diff --git a/public/index.html b/public/index.html index 6cb7c3f2..a3d06f21 100644 --- a/public/index.html +++ b/public/index.html @@ -181,6 +181,7 @@ + diff --git a/public/resume-guard.js b/public/resume-guard.js index 745669e8..e5f3b3c2 100644 --- a/public/resume-guard.js +++ b/public/resume-guard.js @@ -30,7 +30,7 @@ async function guardResume(session, { automatic = false, api, confirm, live, all if (automatic && !(allowBgAttach && live.kind === 'bg')) return false; if (live.kind === 'schedule') { const where = live.cwd ? ` in ${live.cwd}` : ''; - confirm(`This session's scheduled task is already running (pid ${live.pid}${where}).\n\n` + await confirm(`This session's scheduled task is already running (pid ${live.pid}${where}).\n\n` + 'Wait for the scheduled run to finish before opening it here.'); return false; } @@ -39,7 +39,7 @@ async function guardResume(session, { automatic = false, api, confirm, live, all if (typeof live.jobId === 'string' && live.jobId) return { attach: live.jobId, cwd: live.cwd || null }; return false; } - return !!confirm(liveElsewhereMessage(live)); + return !!(await confirm(liveElsewhereMessage(live))); } // see .ai/contexts/cli-session-state.md ("Conversation continuations") @@ -53,23 +53,23 @@ async function resolveResumeSession(session, { automatic = false, api, confirm, } else result = await api.getSessionContinuations(session.sessionId); } catch { if (automatic) return null; - return confirm('Could not check this conversation for continuations.\n\nOpen the original session ' + session.sessionId + '?') ? session : null; + return await confirm('Could not check this conversation for continuations.\n\nOpen the original session ' + session.sessionId + '?') ? session : null; } const candidates = result?.candidates || []; const listing = candidates.map(c => c.sessionId + ' (last activity: ' + (c.modified || 'unknown') + ')').join('\n'); if (!result || result.unresolved) { if (automatic) { onHold?.({ waitingForIndex: !!result?.waitingForIndex }); return null; } - return confirm('This conversation has an unresolved continuation (cycle, missing transcript or malformed continuation record).\n' + return await confirm('This conversation has an unresolved continuation (cycle, missing transcript or malformed continuation record).\n' + listing + '\nYou can retry after indexing or open a candidate from the sidebar.\n\nOpen the original session ' + session.sessionId + '?') ? session : null; } if (!candidates.length) return session; if (automatic) return candidates.length === 1 ? { ...session, sessionId: candidates[0].sessionId } : null; for (const candidate of candidates) { - if (confirm('This conversation continued under another id:\n' + listing + '\n\nOpen ' + candidate.sessionId + '?')) { + if (await confirm('This conversation continued under another id:\n' + listing + '\n\nOpen ' + candidate.sessionId + '?')) { return { ...session, sessionId: candidate.sessionId }; } } - return confirm('No continuation selected.\n\nOpen the original session ' + session.sessionId + '?') ? session : null; + return await confirm('No continuation selected.\n\nOpen the original session ' + session.sessionId + '?') ? session : null; } if (typeof module !== 'undefined' && module.exports) { diff --git a/public/sidebar.js b/public/sidebar.js index 60d203e8..f5bbc54c 100644 --- a/public/sidebar.js +++ b/public/sidebar.js @@ -1166,7 +1166,7 @@ async function archiveProjectFolder(project, button) { const plan = await window.api.getProjectArchivePlan(groupsArg); if (!plan || plan.indexing) { - alert(ARCHIVE_INDEXING_MESSAGE); + await appAlert({ title: 'Switchboard', message: ARCHIVE_INDEXING_MESSAGE }); return; } const schedules = plan.schedules || []; @@ -1233,13 +1233,13 @@ async function archiveProjectFolder(project, button) { disableSchedules: result.disableSchedules ? disableable.map(s => s.filePath) : [], }); if (res && res.error === 'indexing') { - alert(ARCHIVE_INDEXING_MESSAGE); + await appAlert({ title: 'Switchboard', message: ARCHIVE_INDEXING_MESSAGE }); } else if (res && res.error) { - alert('The folder could not be archived: ' + res.error - + (Array.isArray(res.disabled) && res.disabled.length > 0 ? `. These schedules were turned off: ${res.disabled.join(', ')}` : '')); + await appAlert({ title: 'Switchboard', message: 'The folder could not be archived: ' + res.error + + (Array.isArray(res.disabled) && res.disabled.length > 0 ? `. These schedules were turned off: ${res.disabled.join(', ')}` : '') }); } else if (res && Array.isArray(res.failed) && res.failed.length > 0) { - alert('The folder is archived, but these schedules could not be disabled:\n' - + res.failed.map(f => `${f.name}: ${f.error}`).join('\n')); + await appAlert({ title: 'Switchboard', message: 'The folder is archived, but these schedules could not be disabled:\n' + + res.failed.map(f => `${f.name}: ${f.error}`).join('\n') }); } pollActiveSessions(); loadProjects(); @@ -1310,10 +1310,10 @@ function rebindSidebarEvents(projects) { const newPath = await window.api.browseFolder(); if (!newPath) return; const projectShortName = project.projectPath.split('/').filter(Boolean).slice(-2).join('/'); - if (!confirm(`Remap ${projectShortName} to:\n${newPath}?`)) return; + if (!await appConfirm({ title: 'Remap?', message: `Remap ${projectShortName} to:\n${newPath}?`, confirmLabel: 'Remap', danger: true })) return; const result = await window.api.remapProject(project.projectPath, newPath); if (result.error) { - alert('Failed to remap: ' + result.error); + await appAlert({ title: 'Switchboard', message: 'Failed to remap: ' + result.error }); } else { loadProjects(); } @@ -1370,7 +1370,7 @@ function rebindSidebarEvents(projects) { loadProjects(); } else { const msg = (result && result.error) ? result.error : 'Unknown error'; - alert(`Failed to delete worktree: ${msg}`); + await appAlert({ title: 'Switchboard', message: `Failed to delete worktree: ${msg}` }); } }; } diff --git a/public/touched-files-view.js b/public/touched-files-view.js index 64b236b1..073d019b 100644 --- a/public/touched-files-view.js +++ b/public/touched-files-view.js @@ -181,10 +181,10 @@ function renderTouchedTab(sessionId, tab) { } } -function toggleTouchedTab(sessionId) { +async function toggleTouchedTab(sessionId) { const state = getSessionState(sessionId); if (state.currentTab && (state.currentTab.type === 'touched' || state.currentTab.returnList?.type === 'touched')) { - if (!confirmDiscardChangesEdits(state.currentTab)) return; + if ((hasUnsavedChangesEdits(state.currentTab) && !await confirmDiscardChangesEdits(state.currentTab))) return; destroyCurrentTab(state, { stash: false }); state.currentTab = null; endCurrentTab(sessionId, state); @@ -320,14 +320,14 @@ async function openTouchedFile(sessionId, tab, filePath, { line = null, origin = if (currentPanelSessionId === sessionId) renderPanel(sessionId); return; } - if (!confirmDiscardChangesEdits(current)) return; + if ((hasUnsavedChangesEdits(current) && !await confirmDiscardChangesEdits(current))) return; tab.selection = filePath; if (currentPanelSessionId === sessionId) window.snapshotPanelList(touchedListEl, tab); destroyCurrentTab(state, { stash: false }); restoreTouchedStash(sessionId, state, { key: stashKey, returnList: tab, line }); return; } - if (asks && !confirmDiscardChangesEdits(current)) return; + if (asks && (hasUnsavedChangesEdits(current) && !await confirmDiscardChangesEdits(current))) return; const agreedContent = readChangesEditorContent(current); tab.opening = true; let result; @@ -346,7 +346,7 @@ async function openTouchedFile(sessionId, tab, filePath, { line = null, origin = } if (origin !== 'row') addTouchedOpened(state, filePath); const keep = asks - ? readChangesEditorContent(current) !== agreedContent && !confirmDiscardChangesEdits(current) + ? readChangesEditorContent(current) !== agreedContent && (hasUnsavedChangesEdits(current) && !await confirmDiscardChangesEdits(current)) : hasUnsavedChangesEdits(current); if (keep) { if (currentPanelSessionId === sessionId) renderPanel(sessionId); diff --git a/public/viewer-panel.js b/public/viewer-panel.js index 91f65fb2..f09be654 100644 --- a/public/viewer-panel.js +++ b/public/viewer-panel.js @@ -278,19 +278,19 @@ class ViewerPanel { async _delete() { if (!this.opts.onDelete || !this.filePath) return; - // Confirm via native-ish browser confirm - const name = this.filePath.split('/').pop(); - if (!window.confirm(`Delete "${name}"?\n\nThis cannot be undone.`)) return; + const filePath = this.filePath; + const myGen = this._openGen; + const name = filePath.split('/').pop(); + if (!await appConfirm({ title: 'Delete?', message: `Delete "${name}"?\n\nThis cannot be undone.`, confirmLabel: 'Delete', danger: true }) || this._openGen !== myGen) return; try { - const result = await this.opts.onDelete(this.filePath); + const result = await this.opts.onDelete(filePath); if (result && result.ok !== false) { - // Close panel and trigger refresh through onClose - if (this.opts.onClose) this.opts.onClose(); + if (this._openGen === myGen && this.opts.onClose) this.opts.onClose(); } else { - window.alert(`Delete failed: ${result?.error || 'unknown error'}`); + await appAlert({ title: 'Switchboard', message: `Delete failed: ${result?.error || 'unknown error'}` }); } } catch (err) { - window.alert(`Delete failed: ${err.message}`); + await appAlert({ title: 'Switchboard', message: `Delete failed: ${err.message}` }); } } @@ -467,12 +467,12 @@ class ViewerPanel { try { let result = await this.opts.onSave(this.filePath, content, this._agreedBase); while (this._openGen === myGen && result && result.reason === 'stale' && typeof result.disk === 'string') { - this._lastSeenDisk = asEditorText(result.disk); + const overwriteBase = asEditorText(result.disk); + this._lastSeenDisk = overwriteBase; this._setNotice('stale'); - if (typeof window.confirm !== 'function' - || !window.confirm('This file changed on disk since you opened it. Overwrite it with your edits?') + if (!await appConfirm({ title: 'Overwrite?', message: 'This file changed on disk since you opened it. Overwrite it with your edits?', confirmLabel: 'Overwrite', danger: true }) || this._openGen !== myGen) break; - this._agreedBase = this._lastSeenDisk; + this._agreedBase = overwriteBase; result = await this.opts.onSave(this.filePath, content, this._agreedBase); } if (this._token !== token) { @@ -590,10 +590,10 @@ class ViewerPanel { } async _reloadDiscardingEdits() { - if (this._isDirty() && typeof window.confirm === 'function' - && !window.confirm('This file has unsaved edits. Discard them?')) return; - if (!this.filePath || !window.api.readFileForPanel) return; const myGen = this._openGen; + if (this._isDirty() + && !await appConfirm({ title: 'Discard?', message: 'This file has unsaved edits. Discard them?', confirmLabel: 'Discard', danger: true })) return; + if (this._openGen !== myGen || !this.filePath || !window.api.readFileForPanel) return; const result = await window.api.readFileForPanel(this.filePath); if (this._openGen !== myGen || !this.editorView || !result) return; if (!result.ok) { diff --git a/test/app-dialogs-lint.test.js b/test/app-dialogs-lint.test.js new file mode 100644 index 00000000..ac40b32d --- /dev/null +++ b/test/app-dialogs-lint.test.js @@ -0,0 +1,16 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); +const { ESLint } = require('eslint'); + +for (const name of ['confirm', 'alert', 'prompt']) { + for (const prefix of ['', 'window.']) { + test('renderer lint rejects ' + prefix + name, async () => { + const lint = new ESLint({ cwd: path.join(__dirname, '..') }); + const [result] = await lint.lintText(prefix + name + '("native");', { filePath: 'public/dialog-ban-fixture.js' }); + assert.equal(result.errorCount, 1); + assert.match(result.messages[0].message, /public\/app-dialogs\.js/); + assert.equal(result.messages[0].ruleId, prefix ? 'no-restricted-properties' : 'no-restricted-globals'); + }); + } +} diff --git a/test/confirm-and-stop-session.test.js b/test/confirm-and-stop-session.test.js index 5ce09e6d..01c5fb59 100644 --- a/test/confirm-and-stop-session.test.js +++ b/test/confirm-and-stop-session.test.js @@ -29,7 +29,7 @@ function withHarness(overrides, fn) { window.refreshSidebar = () => { calls.refreshed++; }; window.applyRemoteStopped = (id) => calls.remoteStopped.push(id); window.flashButtonText = (b, text, ms) => calls.flashed.push({ b, text, ms }); - window.confirm = () => true; + window.appConfirm = async () => true; window.setTimeout = (cb) => cb(); window.console = { error: (...args) => calls.logged.push(args) }; window.api = { @@ -111,7 +111,7 @@ test('a local session is stopped through stopSession, not the remote channel', a }); test('declining the confirm() dialog calls neither IPC', async () => { - const declining = () => ({ confirm: () => false }); + const declining = () => ({ appConfirm: async () => false }); await withHarness(declining, async ({ confirmAndStopSession, window, calls }) => { await confirmAndStopSession('s1', null); @@ -119,3 +119,18 @@ test('declining the confirm() dialog calls neither IPC', async () => { assert.ok(window.activePtyIds.has('s1')); }); }); + +test('an attached background session names its confirmation Detach', async () => { + let options; + const attached = () => ({ + sessionMap: new Map(), + openSessions: new Map([['s1', { attach: true }]]), + appConfirm: async value => { options = value; return false; }, + }); + await withHarness(attached, async ({ confirmAndStopSession, calls }) => { + await confirmAndStopSession('s1', null); + assert.equal(options.confirmLabel, 'Detach'); + assert.match(options.message, /keeps running/); + assert.deepEqual(calls.ipc, []); + }); +}); diff --git a/test/continuations-harness.js b/test/continuations-harness.js index 9368438f..0dd5fe54 100644 --- a/test/continuations-harness.js +++ b/test/continuations-harness.js @@ -40,7 +40,7 @@ function setup(t, graph, { live = {}, answer = false, savedEntries, chunkBytes } const saved = savedEntries || [{ sessionId: 'old', projectPath: root, active: true }]; fs.writeFileSync(settingsFile, JSON.stringify({ openWorkingSet: saved })); const spawned = [], prompts = []; - dom.window.confirm = message => { prompts.push(message); return typeof answer === 'function' ? answer(message) : answer; }; + dom.window.appConfirm = async ({ message: message }) => { prompts.push(message); return typeof answer === 'function' ? answer(message) : answer; }; dom.window.api = { getSetting: async () => JSON.parse(fs.readFileSync(settingsFile, 'utf8')), setSetting: async (_, value) => fs.writeFileSync(settingsFile, JSON.stringify(value)), diff --git a/test/dom-activity-trace-panel.test.js b/test/dom-activity-trace-panel.test.js index abf8d92a..3439ec29 100644 --- a/test/dom-activity-trace-panel.test.js +++ b/test/dom-activity-trace-panel.test.js @@ -79,8 +79,8 @@ function setup({ traceState, files = [], readResult, deleteResult = { ok: true } const confirms = []; const alerts = []; - window.confirm = (msg) => { confirms.push(msg); return window.__confirmAnswer !== false; }; - window.alert = (msg) => { alerts.push(String(msg)); }; + window.appConfirm = async ({ message: msg }) => { confirms.push(msg); return window.__confirmAnswer !== false; }; + window.appAlert = async ({ message: msg }) => { alerts.push(String(msg)); }; evalInWindow(dom, path.join(PUBLIC_DIR, 'setting-defaults.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'utils.js')); @@ -301,3 +301,13 @@ test('an IPC rejection is reported instead of leaving a dead button', async () = assert.match(ctx.alerts[0], /handler exploded/); } finally { ctx.destroy(); } }); + +test('a failed trace read reports its message through appAlert and opens no content', async () => { + const ctx = setup({ files: SAMPLE_FILES, readResult: { ok: false, error: 'gone' } }); + try { + await ctx.window.openActivityTraceFile(SAMPLE_FILES[0]); + assert.equal(ctx.opened.length, 0); + assert.equal(ctx.alerts.length, 1); + assert.match(ctx.alerts[0], /Could not read.*gone/); + } finally { ctx.destroy(); } +}); diff --git a/test/dom-agents-view.test.js b/test/dom-agents-view.test.js index e0692171..8591fbd6 100644 --- a/test/dom-agents-view.test.js +++ b/test/dom-agents-view.test.js @@ -77,7 +77,7 @@ function setup({ storage = { agentsGroupBy: 'none' }, settingsPanel = false } = showJsonlViewer: (session) => calls.jsonl.push(session), refreshSidebar: () => { calls.sidebarRefreshes++; }, fitAndScroll: () => {}, - confirm: () => true, + appConfirm: async () => true, }; for (const [k, v] of Object.entries(g)) Object.defineProperty(window, k, { value: v, writable: true, configurable: true }); vm.runInContext(fs.readFileSync(path.join(__dirname, '..', 'node_modules', 'morphdom', 'dist', 'morphdom-umd.js'), 'utf8'), dom.getInternalVMContext()); @@ -899,3 +899,37 @@ test('the header counts the External rows it displays, and omits the segment whe ctx.emitChanged({ roster: [ROSTER[0], ROSTER[1]], daemonReachable: true }); assert.equal(count(), '1 running · 1 finished'); }); + +test('background Delete waits for its async confirmation and cancellation sends no verb', async t => { + const ctx = setup(); t.after(() => ctx.destroy()); + ctx.setSnapshot({ roster: ROSTER, daemonReachable: true }); + await ctx.window.showAgentsView(); + let answer; + ctx.window.appConfirm = () => new Promise(resolve => { answer = resolve; }); + const pending = ctx.window.runAgentVerb('rm', ROSTER[1]); + assert.deepEqual(ctx.calls.verbs, []); + answer(false); + await pending; + assert.deepEqual(ctx.calls.verbs, []); + ctx.window.appConfirm = async () => true; + await ctx.window.runAgentVerb('rm', ROSTER[1]); + assert.deepEqual(ctx.calls.verbs, [['rm', ROSTER[1].id]]); +}); +for (const agree of [false, true]) { + test('project Hide ' + (agree ? 'proceeds' : 'cancels') + ' after an async decision', async t => { + const ctx = setup({ settingsPanel: true }); t.after(() => ctx.destroy()); + const removed = []; + ctx.window.api.removeProject = async (...args) => { removed.push(args); }; + await ctx.window.openSettingsViewer('project', '/project', 'folder-key'); + let answer; + let options; + ctx.window.appConfirm = value => { options = value; return new Promise(resolve => { answer = resolve; }); }; + ctx.document.getElementById('sv-remove-btn').click(); + assert.deepEqual(removed, []); + assert.equal(options.confirmLabel, 'Hide'); + answer(agree); + await new Promise(resolve => setImmediate(resolve)); + assert.deepEqual(removed, agree ? [['/project', 'folder-key']] : []); + assert.equal(ctx.document.getElementById('settings-viewer').style.display, agree ? 'none' : 'flex'); + }); +} diff --git a/test/dom-app-dialogs.test.js b/test/dom-app-dialogs.test.js new file mode 100644 index 00000000..30f519e5 --- /dev/null +++ b/test/dom-app-dialogs.test.js @@ -0,0 +1,106 @@ +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 { setupSidebarDom } = require('./dom-setup'); +function setup() { + const ctx = setupSidebarDom(); + const filename = path.join(__dirname, '../public/app-dialogs.js'); + vm.runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); + return ctx; +} +function key(ctx, name) { + ctx.document.activeElement.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: name, bubbles: true, cancelable: true })); +} +test('app dialogs queue confirmations and alerts without treating busy as cancel', async () => { + const ctx = setup(); + try { + const first = ctx.window.appConfirm({ title: 'First', message: 'One' }); + const second = ctx.window.appConfirm({ title: 'Second', message: 'Two' }); + const third = ctx.window.appAlert({ title: 'Third', message: 'Three' }); + assert.equal(ctx.document.querySelectorAll('.choice-dialog-overlay').length, 1); + key(ctx, 'Enter'); + assert.equal(await first, true); + assert.equal(ctx.document.querySelector('h2').textContent, 'Second'); + key(ctx, 'Escape'); + assert.equal(await second, false); + assert.equal(ctx.document.querySelector('h2').textContent, 'Third'); + assert.equal(ctx.document.querySelector('.choice-dialog-cancel').hidden, true); + key(ctx, 'Enter'); + await third; + assert.equal(ctx.document.querySelector('.choice-dialog-overlay'), null); + } finally { ctx.destroy(); } +}); +test('destructive confirm focuses Cancel, styles danger, and restores its connected opener', async () => { + const ctx = setup(); + try { + const opener = ctx.document.createElement('button'); + ctx.document.body.append(opener); + opener.focus(); + const pending = ctx.window.appConfirm({ title: 'Delete', message: 'Really?', confirmLabel: 'Delete', danger: true }); + assert.equal(ctx.document.activeElement, ctx.document.querySelector('.choice-dialog-cancel')); + assert.ok(ctx.document.querySelector('.choice-dialog-confirm').classList.contains('delete-worktree-confirm-btn')); + key(ctx, 'Enter'); + assert.equal(await pending, false); + assert.equal(ctx.document.activeElement, opener); + const again = ctx.window.appConfirm({ message: 'Again' }); + assert.equal(ctx.document.activeElement, ctx.document.querySelector('.choice-dialog-confirm')); + key(ctx, 'Escape'); + assert.equal(await again, false); + assert.equal(ctx.document.activeElement, opener); + } finally { ctx.destroy(); } +}); +test('app confirmation waits behind an existing choice dialog', async () => { + const ctx = setup(); + try { + const existing = ctx.window.showChoiceDialog({ title: 'Choices' }); + const pending = ctx.window.appConfirm({ title: 'Queued' }); + key(ctx, 'Enter'); + await existing; + assert.equal(ctx.document.querySelector('h2').textContent, 'Queued'); + key(ctx, 'Enter'); + assert.equal(await pending, true); + } finally { ctx.destroy(); } +}); +test('closing does not focus a disconnected opener', async () => { + const ctx = setup(); + try { + const opener = ctx.document.createElement('button'); + ctx.document.body.append(opener); + opener.focus(); + const pending = ctx.window.appConfirm({ message: 'Really?' }); + opener.remove(); + let focused = false; + opener.focus = () => { focused = true; }; + key(ctx, 'Escape'); + assert.equal(await pending, false); + assert.equal(focused, false); + } finally { ctx.destroy(); } +}); + +test('queued dialogs restore the external opener after the entire queue closes', async () => { + const ctx = setup(); + try { + const opener = ctx.document.createElement('button'); + ctx.document.body.append(opener); + opener.focus(); + const first = ctx.window.appConfirm({ title: 'First' }); + const second = ctx.window.appConfirm({ title: 'Second' }); + key(ctx, 'Enter'); + await first; + key(ctx, 'Escape'); + await second; + assert.equal(ctx.document.activeElement, opener); + } finally { ctx.destroy(); } +}); + +test('messages preserve the line breaks used by resume and error details', async () => { + const ctx = setup(); + try { + const pending = ctx.window.appAlert({ message: 'First line\nSecond line' }); + assert.deepEqual([...ctx.document.querySelectorAll('.choice-dialog-body p')].map(p => p.textContent), ['First line', 'Second line']); + key(ctx, 'Enter'); + await pending; + } finally { ctx.destroy(); } +}); diff --git a/test/dom-choice-dialog.test.js b/test/dom-choice-dialog.test.js index a6061913..9311daa7 100644 --- a/test/dom-choice-dialog.test.js +++ b/test/dom-choice-dialog.test.js @@ -187,11 +187,11 @@ test('choice dialog: one paragraph per message line', async () => { } finally { ctx.destroy(); } }); -test('choice dialog: a second call while one is open resolves null and leaves the first', async () => { +test('choice dialog: a second call waits until the first closes', async () => { const ctx = setupSidebarDom(); try { const first = open(ctx); - assert.equal(await open(ctx), null); + const second = open(ctx); assert.equal(ctx.document.querySelectorAll('.modal-overlay').length, 1); confirmBtn(ctx).click(); assert.deepEqual({ ...(await first) }, { a: true, b: true }); diff --git a/test/dom-file-panel-changes.test.js b/test/dom-file-panel-changes.test.js index 720b5f31..7e98573d 100644 --- a/test/dom-file-panel-changes.test.js +++ b/test/dom-file-panel-changes.test.js @@ -169,7 +169,7 @@ function setupFilePanelDom({ statusImpl, diffImpl, fileImpl, saveImpl, confirmIm // jsdom's own window.confirm throws "not implemented"; the panel asks before // discarding unsaved edits, so the suite answers for the user. - window.confirm = (message) => { + window.appConfirm = async ({ message: message }) => { calls.confirm.push(message); return confirmImpl ? confirmImpl(message) : true; }; @@ -2509,3 +2509,19 @@ test('a diff failure on a subagent row is shown, not thrown', async () => { assert.match(ctx.document.getElementById('changes-diff-view').textContent, /no longer exists/); } finally { ctx.destroy(); } }); + +test('delayed discard consent cannot close a replacement MCP tab', async () => { + const ctx = setupFilePanelDom(); + try { + await openFile(ctx, 's1', 'src/a.js'); + ctx.editors[0].box.text = 'unsaved'; + let answer; + ctx.window.appConfirm = () => new Promise(resolve => { answer = resolve; }); + const closing = ctx.window.handleClose(); + ctx.window.openDiffTab('s1', 'replacement', { oldFilePath: '/repo/other.js', oldContent: 'a', newContent: 'b' }); + await flush(); + answer(true); + await closing; + assert.equal(ctx.stateOf('s1').currentTab?.diffId, 'replacement'); + } finally { ctx.destroy(); } +}); diff --git a/test/dom-file-panel-diff-save.test.js b/test/dom-file-panel-diff-save.test.js index d77bed8b..860d70b3 100644 --- a/test/dom-file-panel-diff-save.test.js +++ b/test/dom-file-panel-diff-save.test.js @@ -41,8 +41,8 @@ function setup({ saveImpl, confirmAnswer = true } = {}) { return () => Promise.resolve({ ok: true }); }, }); - window.confirm = (msg) => { calls.confirms.push(msg); return confirmAnswer; }; - window.alert = (msg) => { calls.alerts.push(msg); }; + window.appConfirm = async ({ message: msg }) => { calls.confirms.push(msg); return confirmAnswer; }; + window.appAlert = async ({ message: msg }) => { calls.alerts.push(msg); }; window.loadCodeMirrorBundle = () => Promise.resolve(); window.localStorage.setItem('filePanelDiffMode', 'side-by-side'); @@ -261,7 +261,7 @@ test('the diff tab asks again when the disk moves during its confirm, and never const disk = fakeDisk('session\n'); const ctx = setup({ saveImpl: disk.save }); let asked = 0; - ctx.window.confirm = (msg) => { + ctx.window.appConfirm = async ({ message: msg }) => { ctx.calls.confirms.push(msg); asked += 1; if (asked === 1) { disk.text = 'second session write\n'; return true; } diff --git a/test/dom-file-panel-goto-line.test.js b/test/dom-file-panel-goto-line.test.js index 9501e46f..69980e43 100644 --- a/test/dom-file-panel-goto-line.test.js +++ b/test/dom-file-panel-goto-line.test.js @@ -38,7 +38,7 @@ function setup({ pair } = {}) { unwatchFile: () => Promise.resolve({ ok: true }), saveFileForPanel: () => Promise.resolve({ ok: true }), }; - window.confirm = () => true; + window.appConfirm = async () => true; window.loadCodeMirrorBundle = () => Promise.resolve(); window.cmRevealLine = (view, line) => calls.revealed.push({ view, line }); diff --git a/test/dom-file-panel-touched.test.js b/test/dom-file-panel-touched.test.js index 0126cc61..d06f1c3b 100644 --- a/test/dom-file-panel-touched.test.js +++ b/test/dom-file-panel-touched.test.js @@ -307,7 +307,7 @@ function setupDom({ touchedImpl, readImpl, confirmImpl, storedRatio, storageThro calls.save.push({ filePath, content, expected, options }); return Promise.resolve(saveImpl ? saveImpl(calls.save.length - 1) : { ok: true, version: 'v2' }); }; - window.confirm = (...args) => { calls.confirm++; return (confirmImpl || (() => true))(...args); }; + window.appConfirm = async ({ message }) => { calls.confirm++; return (confirmImpl || (() => true))(message); }; window.cmRevealLine = (_view, line) => calls.revealed.push(line); if (ownerOf) window.panelTerminalOwnerOf = ownerOf; const editors = []; @@ -732,6 +732,8 @@ test('close and Escape restore the same Touched rows, scroll and selection witho const back = ctx.document.getElementById('changes-diff-close-btn'); assert.ok(back, 'a shared panel back button'); back.click(); + await flush(); + await flush(); assert.equal(ctx.stateOf('s1').currentTab?.type, 'touched'); assert.equal(rows(ctx)[0], first); assert.equal(list.scrollTop, 123); @@ -942,9 +944,12 @@ test('Close keeps a dirty file on refusal and discards it without stashing it af ctx.editors.at(-1).setText('unsaved edit'); const back = ctx.document.getElementById('changes-diff-close-btn'); back.click(); + await flush(); + await flush(); assert.equal(ctx.stateOf('s1').currentTab?.type, 'changes'); agree = true; back.click(); + await flush(); assert.equal(ctx.stateOf('s1').currentTab?.type, 'touched'); assert.equal(ctx.stateOf('s1').touchedStashes?.size ?? 0, 0); } finally { ctx.destroy(); } @@ -1214,6 +1219,7 @@ test('round 3: a symlink Touched pair uses the shared chrome read-only and canno await ctx.window.handleChangesSave('s1'); assert.equal(ctx.calls.save.length, 0); ctx.document.getElementById('changes-diff-close-btn').click(); + await flush(); assert.equal(ctx.stateOf('s1').currentTab, list); assert.equal(rows(ctx)[0], row); assert.equal(ctx.calls.touched.length, 1); @@ -1370,6 +1376,7 @@ for (const [dirty, git] of [[true, true], [false, true], [true, false], [false, assert.equal(ctx.editors.at(-1).current, dirty ? 'typing' : 'external'); assert.equal(ctx.stateOf('s1').currentTab.externalChange, dirty); ctx.document.getElementById('changes-diff-close-btn').click(); + await flush(); assert.deepEqual(ctx.watchCalls.at(-1), ['unwatch', '/work/a.txt']); assert.equal(ctx.calls.touched.length, 1); } finally { ctx.destroy(); } diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index a6e86edc..57b65753 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -72,6 +72,7 @@ function setup() { }, }); window.confirm = (msg) => { calls.confirms.push(msg); return calls.onConfirm ? calls.onConfirm(msg) : false; }; + window.appConfirm = async ({ message: msg }) => { calls.confirms.push(msg); return false; }; window.createEditableViewer = (parent, content) => { editor = fakeEditor(content); editor.dom = window.document.createElement('div'); @@ -98,7 +99,7 @@ function setup() { window.appShortcuts = {}; window.initFilePanel(); registerPanelTerminals(dom, ['s1', 's3']); - return { window, disk, calls, editor: () => editor, destroy: () => window.close() }; + return { window, context: dom.getInternalVMContext(), disk, calls, editor: () => editor, destroy: () => window.close() }; } const flush = () => new Promise((r) => setTimeout(r, 5)); @@ -653,3 +654,67 @@ test('a check delivered twice, before and after the page said it is ready, is an assert.deepEqual(ctx.calls.answers, [{ id: 9, proceed: true }]); } finally { ctx.destroy(); } }); +function loadAppDialogs(ctx) { + for (const f of ['choice-dialog.js', 'app-dialogs.js']) { + const filename = path.join(PUBLIC_DIR, f); + vm.runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); + } +} +function dialogKey(ctx, key, init = {}) { + ctx.window.document.activeElement.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init })); +} +test('reload ACK is immediate while confirmations queue; quit upgrade waits for one focused unsaved guard', async () => { + const ctx = setup(); + try { + loadAppDialogs(ctx); + await dirtyTab(ctx, 's1', A); + const opener = ctx.window.document.createElement('button'); + ctx.window.document.body.append(opener); + opener.focus(); + let flushed = 0; + ctx.window.flushStateForExit = async () => { flushed++; }; + const confirming = ctx.window.appConfirm({ title: 'Delete?', danger: true }); + const alerting = ctx.window.appAlert({ message: 'Queued message' }); + const checking = ctx.calls.check(44, 'reload'); + ctx.calls.reason(44, 'quit'); + assert.deepEqual(ctx.calls.acks.map(x => x.id), [44]); + await flush(); + assert.equal(dialog(ctx), null, 'the unsaved guard must wait for the dialog queue'); + dialogKey(ctx, 'Escape'); + assert.equal(await confirming, false); + dialogKey(ctx, 'Enter'); + await alerting; + await flush(); + assert.ok(dialog(ctx)); + assert.equal(ctx.window.document.querySelector('.choice-dialog-overlay'), null); + assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-cancel')); + dialogKey(ctx, 'Tab', { shiftKey: true }); + assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-save')); + dialogKey(ctx, 'Tab'); + assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-cancel')); + button(ctx, 'unsaved-discard').click(); + await checking; + assert.deepEqual(ctx.calls.answers, [{ id: 44, proceed: true }]); + assert.equal(flushed, 1); + assert.equal(ctx.window.document.activeElement, opener); + } finally { ctx.destroy(); } +}); +test('a foreground alert cannot cancel or steal focus into an underlying unsaved guard', async () => { + const ctx = setup(); + try { + loadAppDialogs(ctx); + await dirtyTab(ctx, 's1', A); + const checking = ctx.calls.check(45, 'reload'); + await flush(); + const alerting = ctx.window.appAlert({ message: 'Foreground' }); + dialogKey(ctx, 'Tab'); + assert.ok(ctx.window.document.activeElement.closest('.choice-dialog-overlay')); + dialogKey(ctx, 'Escape'); + await alerting; + assert.ok(dialog(ctx)); + assert.deepEqual(ctx.calls.answers, []); + dialogKey(ctx, 'Escape'); + await checking; + assert.deepEqual(ctx.calls.answers, [{ id: 45, proceed: false }]); + } finally { ctx.destroy(); } +}); diff --git a/test/dom-project-archive-folder.test.js b/test/dom-project-archive-folder.test.js index a399cd9d..8dd8375d 100644 --- a/test/dom-project-archive-folder.test.js +++ b/test/dom-project-archive-folder.test.js @@ -238,7 +238,7 @@ test('archive folder: while indexing, an alert and nothing else', async () => { const project = { projectPath: '/p', folder: '-p', sessions: [session('s1')] }; const calls = installApi(ctx, { getProjectArchivePlan: () => Promise.resolve({ indexing: true }) }); let alerted = null; - ctx.window.alert = (m) => { alerted = m; }; + ctx.window.appAlert = async ({ message: m }) => { alerted = m; }; render(ctx, [project]); await clickArchive(ctx, '/p'); @@ -317,7 +317,7 @@ test('archive folder: an archive that failed after disabling schedules names the try { installApi(ctx, { archiveProject: () => Promise.resolve({ error: 'SQLITE_BUSY', disabled: ['nightly'] }) }); let alerted = null; - ctx.window.alert = (m) => { alerted = m; }; + ctx.window.appAlert = async ({ message: m }) => { alerted = m; }; render(ctx, [{ projectPath: '/p', folder: '-p', sessions: [session('s1')] }]); await clickArchive(ctx, '/p', { confirm: true }); @@ -383,7 +383,7 @@ test('archive folder: a schedule that could not be disabled is reported', async archiveProject: () => Promise.resolve({ ok: true, disabled: [], failed: [{ name: 'a', error: 'EACCES' }] }), }); let alerted = null; - ctx.window.alert = (m) => { alerted = m; }; + ctx.window.appAlert = async ({ message: m }) => { alerted = m; }; render(ctx, [project]); await clickArchive(ctx, '/p', { confirm: true }); @@ -397,7 +397,7 @@ test('archive folder: an archive refused for indexing is reported', async () => const project = { projectPath: '/p', folder: '-p', sessions: [session('s1')] }; installApi(ctx, { archiveProject: () => Promise.resolve({ error: 'indexing' }) }); let alerted = null; - ctx.window.alert = (m) => { alerted = m; }; + ctx.window.appAlert = async ({ message: m }) => { alerted = m; }; render(ctx, [project]); await clickArchive(ctx, '/p', { confirm: true }); @@ -411,7 +411,7 @@ test('archive folder: an archive that failed in the main process is reported', a const project = { projectPath: '/p', folder: '-p', sessions: [session('s1')] }; installApi(ctx, { archiveProject: () => Promise.resolve({ error: 'SQLITE_BUSY' }) }); let alerted = null; - ctx.window.alert = (m) => { alerted = m; }; + ctx.window.appAlert = async ({ message: m }) => { alerted = m; }; render(ctx, [project]); await clickArchive(ctx, '/p', { confirm: true }); diff --git a/test/dom-setup.js b/test/dom-setup.js index f9ee01fc..94a60c0d 100644 --- a/test/dom-setup.js +++ b/test/dom-setup.js @@ -148,6 +148,7 @@ function setupSidebarDom() { evalInWindow(dom, path.join(PUBLIC_DIR, 'bridge-url.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'worktree-nesting.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'choice-dialog.js')); + evalInWindow(dom, path.join(PUBLIC_DIR, 'app-dialogs.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'sidebar.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'remote-activity-ui.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'local-transcript-adapter.js')); diff --git a/test/dom-sidebar-dialogs.test.js b/test/dom-sidebar-dialogs.test.js new file mode 100644 index 00000000..8031c23d --- /dev/null +++ b/test/dom-sidebar-dialogs.test.js @@ -0,0 +1,69 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { setupSidebarDom, makeSampleProject } = require('./dom-setup'); + +for (const agree of [false, true]) { + test('worktree Hide ' + (agree ? 'proceeds' : 'cancels') + ' after an async decision', async () => { + const ctx = setupSidebarDom(); + try { + const parent = makeSampleProject({ projectPath: '/p' }); + const child = makeSampleProject({ projectPath: '/p/.claude/worktrees/w' }); + const removed = []; + ctx.window.api = { removeProject: async path => { removed.push(path); } }; + ctx.window.cachedAllProjects = [parent, child]; + ctx.sidebar.renderProjects([parent, child], true); + const button = ctx.document.querySelector('.worktree-hide-btn'); + assert.ok(button); + let answer; + ctx.window.appConfirm = options => { + assert.equal(options.confirmLabel, 'Hide'); + return new Promise(resolve => { answer = resolve; }); + }; + const pending = button.onclick(new ctx.window.MouseEvent('click')); + assert.deepEqual(removed, []); + answer(agree); + await pending; + assert.deepEqual(removed, agree ? [child.projectPath] : []); + } finally { ctx.destroy(); } + }); +} + +for (const agree of [false, true]) { + test('Remap ' + (agree ? 'proceeds and reports failure' : 'cancels') + ' after an async decision', async () => { + const ctx = setupSidebarDom(); + try { + const project = makeSampleProject({ projectPath: '/missing', missing: true }); + const remapped = []; + const alerts = []; + ctx.window.api = { + browseFolder: async () => '/replacement', + remapProject: async (...args) => { remapped.push(args); return { error: 'busy' }; }, + }; + ctx.window.appAlert = async ({ message }) => { alerts.push(message); }; + ctx.sidebar.renderProjects([project], true); + let answer; + ctx.window.appConfirm = () => new Promise(resolve => { answer = resolve; }); + const pending = ctx.document.querySelector('.project-remap-btn').onclick(new ctx.window.MouseEvent('click')); + await new Promise(resolve => setImmediate(resolve)); + assert.deepEqual(remapped, []); + answer(agree); + await pending; + assert.deepEqual(remapped, agree ? [['/missing', '/replacement']] : []); + assert.deepEqual(alerts, agree ? ['Failed to remap: busy'] : []); + } finally { ctx.destroy(); } + }); +} +test('a failed worktree delete reports the error through appAlert', async () => { + const ctx = setupSidebarDom(); + try { + const parent = makeSampleProject({ projectPath: '/p' }); + const child = makeSampleProject({ projectPath: '/p/.claude/worktrees/w' }); + const alerts = []; + ctx.window.api = { deleteWorktree: async () => ({ ok: false, error: 'busy' }) }; + ctx.window.showDeleteWorktreeDialog = async () => true; + ctx.window.appAlert = async ({ message }) => { alerts.push(message); }; + ctx.sidebar.renderProjects([parent, child], true); + await ctx.document.querySelector('.worktree-delete-btn').onclick(new ctx.window.MouseEvent('click')); + assert.deepEqual(alerts, ['Failed to delete worktree: busy']); + } finally { ctx.destroy(); } +}); diff --git a/test/dom-sidebar-merged-members.test.js b/test/dom-sidebar-merged-members.test.js index 4b5e93fe..fd3309f0 100644 --- a/test/dom-sidebar-merged-members.test.js +++ b/test/dom-sidebar-merged-members.test.js @@ -188,7 +188,7 @@ test('a remote merged member stays alive across render and Stop keeps its host', assert.ok(item.classList.contains('is-alive')); assert.ok(render(ctx, [remoteRow]).classList.contains('is-alive')); const stopped = []; - Object.assign(ctx.window, { confirm: () => true, gridViewActive: false, refreshSidebar() {} }); + Object.assign(ctx.window, { appConfirm: async () => true, gridViewActive: false, refreshSidebar() {} }); ctx.window.api.remoteStopSession = async (alias, id) => { stopped.push([alias, id]); return { ok: true }; }; ctx.window.api.stopSession = async id => { stopped.push(['local', id]); return { ok: true }; }; loadAppFunctions(ctx.context, { functions: ['confirmAndStopSession'] }); diff --git a/test/keyboard-focus.test.js b/test/keyboard-focus.test.js index c68aa3fc..7297a43f 100644 --- a/test/keyboard-focus.test.js +++ b/test/keyboard-focus.test.js @@ -87,6 +87,7 @@ for (const dialog of ['choice', 'New Session']) { const ctx = setup(); try { loadPublic(ctx, 'choice-dialog.js'); + loadPublic(ctx, 'app-dialogs.js'); loadPublic(ctx, 'setting-defaults.js'); loadPublic(ctx, 'dialogs.js'); const doc = ctx.window.document; diff --git a/test/open-session-merged-members.test.js b/test/open-session-merged-members.test.js index f0bb4da0..c0d2b17f 100644 --- a/test/open-session-merged-members.test.js +++ b/test/open-session-merged-members.test.js @@ -15,7 +15,7 @@ function setup(t) { Object.assign(ctx.window, { restoringWorkingSet: false, sessionOpenedOutsideRestore: false, continuationRetryCancelled: false, showSession: id => shown.push(id), - confirm: message => { prompts.push(message); return false; }, + appConfirm: async ({ message }) => { prompts.push(message); return false; }, }); ctx.window.api.getSessionContinuations = async id => { checked.push(id); diff --git a/test/open-session-once.test.js b/test/open-session-once.test.js index 099bb24d..36c304be 100644 --- a/test/open-session-once.test.js +++ b/test/open-session-once.test.js @@ -128,7 +128,7 @@ test('a click while an automatic open of a session live elsewhere is in flight a const lookups = []; h.window.api.getSessionLiveElsewhere = () => new Promise((resolve) => lookups.push(resolve)); const confirms = []; - h.window.confirm = (msg) => { confirms.push(msg); return true; }; + h.window.appConfirm = async ({ message: msg }) => { confirms.push(msg); return true; }; const automatic = h.openSession(h.window.sessionMap.get('s1'), undefined, { automatic: true }); h.row().click(); assert.equal(h.window.eval('continuationRetryCancelled'), true, 'the click cancels held retries at once'); @@ -151,7 +151,7 @@ test('a double click on the row of a session live elsewhere asks once', async () h.window.guardResume = require('../public/resume-guard').guardResume; h.window.api.getSessionLiveElsewhere = async () => ({ pid: 42 }); let confirms = 0; - h.window.confirm = () => { confirms++; return false; }; + h.window.appConfirm = async () => { confirms++; return false; }; h.row().click(); h.row().click(); for (let i = 0; i < 5; i++) await settle(); diff --git a/test/panel-terminal.test.js b/test/panel-terminal.test.js index 087935e7..5086e551 100644 --- a/test/panel-terminal.test.js +++ b/test/panel-terminal.test.js @@ -718,7 +718,7 @@ test('the panel X over a dirty buffer asks first, and a refusal keeps both the e try { const { window, document } = ctx; const asked = []; - window.confirm = (message) => { asked.push(message); return asked.length > 1; }; + window.appConfirm = async ({ message: message }) => { asked.push(message); return asked.length > 1; }; stubChangesEditors(window); window.switchPanel('owner'); @@ -730,7 +730,7 @@ test('the panel X over a dirty buffer asks first, and a refusal keeps both the e await window.togglePanelTerminal('owner'); ctx.inCtx("filePanelState.get('owner').currentTab").editorView.text = 'edited\n'; - window.handleClose(); // the X on the tab toolbar, first answer: keep the edits + await window.handleClose(); // the X on the tab toolbar, first answer: keep the edits assert.equal(asked.length, 1, 'unsaved edits are never dropped without asking'); assert.equal(document.getElementById('file-panel-changes').style.display, 'flex', @@ -738,7 +738,7 @@ test('the panel X over a dirty buffer asks first, and a refusal keeps both the e assert.ok(ctx.inCtx("filePanelState.get('owner').currentTab"), 'and the buffer is still there to save'); assert.ok(window.openSessions.has('panel:owner'), 'the shell below it is untouched'); - window.handleClose(); // second answer: discard + await window.handleClose(); // second answer: discard assert.equal(asked.length, 2); assert.equal(document.getElementById('file-panel-changes').style.display, 'none'); diff --git a/test/restore-live-elsewhere.test.js b/test/restore-live-elsewhere.test.js index 8eee25a1..d88c6860 100644 --- a/test/restore-live-elsewhere.test.js +++ b/test/restore-live-elsewhere.test.js @@ -59,7 +59,7 @@ function setup({ savedSet, liveIds, batchFails = false }) { }, openTerminal: async (sessionId) => { openTerminalCalls.push(sessionId); return { ok: true }; }, }; - dom.window.confirm = () => { throw new Error('an automatic resume must never prompt'); }; + dom.window.appConfirm = async () => { throw new Error('an automatic resume must never prompt'); }; vm.runInContext(read('utils.js'), ctx); vm.runInContext(read('resume-guard.js'), ctx); @@ -174,7 +174,7 @@ test('a failed batch check fails open: every session is resumed, nothing is skip test('once the user opens the skipped session and closes it, it leaves the working set like any other', async () => { const h = setup({ savedSet: SAVED, liveIds: ['x'] }); await h.restore(); - h.dom.window.confirm = () => true; + h.dom.window.appConfirm = async () => true; await vm.runInContext('openSession(sessionMap.get("x"))', h.ctx); assert.deepEqual(h.openTerminalCalls, ['a', 'b', 'x']); vm.runInContext('openSessions.get("x").closed = true;', h.ctx); diff --git a/test/restore-merged-member.test.js b/test/restore-merged-member.test.js index a34bedcb..a69774fa 100644 --- a/test/restore-merged-member.test.js +++ b/test/restore-merged-member.test.js @@ -49,7 +49,7 @@ function setup(t, { id = 'B', ids = [id], members = ['B'], mode = 'auto', live = return { ok: true }; }, }); - h.window.confirm = () => { throw new Error('automatic restore must not prompt'); }; + h.window.appConfirm = async () => { throw new Error('automatic restore must not prompt'); }; h.evalPublic('restore-plan.js'); h.evalPublic('resume-guard.js'); const functions = loadAppFunctions(h.context, { diff --git a/test/resume-guard.test.js b/test/resume-guard.test.js index 027999a1..0df851a5 100644 --- a/test/resume-guard.test.js +++ b/test/resume-guard.test.js @@ -37,7 +37,7 @@ function makeApi(answer) { function makeConfirm(result) { const messages = []; - const confirm = (msg) => { messages.push(msg); return result; }; + const confirm = async (msg) => { messages.push(msg); return result; }; return { confirm, messages }; } @@ -99,7 +99,7 @@ test('a failed check is silence: the resume proceeds as it did before the guard' test('loaded as a classic script, the guard is a global the renderer can call', async () => { const ctx = vm.createContext({}); - vm.runInContext(fs.readFileSync(GUARD_PATH, 'utf8'), ctx); + vm.runInContext(fs.readFileSync(GUARD_PATH, 'utf8'), ctx, { filename: GUARD_PATH }); assert.equal(typeof ctx.guardResume, 'function'); const api = makeApi(LIVE); assert.equal(await ctx.guardResume(SESSION, { automatic: true, api, confirm: () => true }), false); diff --git a/test/schedule-live-resume.test.js b/test/schedule-live-resume.test.js index e460af51..efc092f4 100644 --- a/test/schedule-live-resume.test.js +++ b/test/schedule-live-resume.test.js @@ -72,7 +72,7 @@ function setup(t, { sandbox = false } = {}) { getSetting: async (key) => settings[key], setSetting: async (key, value) => { settings[key] = value; }, }; - dom.window.confirm = (message) => { messages.push(message); return true; }; + dom.window.appConfirm = async ({ message: message }) => { messages.push(message); return true; }; vm.runInContext(read('public/utils.js'), ctx); vm.runInContext(read('public/resume-guard.js'), ctx); vm.runInContext(` diff --git a/test/viewer-panel-external-change.test.js b/test/viewer-panel-external-change.test.js index 00517a4c..a1f6100c 100644 --- a/test/viewer-panel-external-change.test.js +++ b/test/viewer-panel-external-change.test.js @@ -60,7 +60,7 @@ function setup({ disk }) { ? { ok: false, error: 'ENOENT: no such file or directory', code: 'ENOENT' } : { ok: true, content: state.disk }), }; - window.confirm = (msg) => { state.confirms.push(msg); return state.confirmAnswer; }; + window.appConfirm = async ({ message: msg }) => { state.confirms.push(msg); return state.confirmAnswer; }; const realCreate = window.document.createElement.bind(window.document); window.document.createElement = function (tag, ...args) { @@ -637,7 +637,7 @@ test('Overwrite while the disk moves during its confirm asks again and never wri try { await unagreed(t); let asked = 0; - t.window.confirm = (msg) => { + t.window.appConfirm = async ({ message: msg }) => { t.state.confirms.push(msg); asked += 1; if (asked === 1) { t.state.disk = 'second session write\n'; return true; } @@ -802,7 +802,7 @@ test('a stale refusal nothing had announced shows its notice and says the file c await t.open('first read\n'); t.editor().type('mine'); let noticeAtConfirm = null; - t.window.confirm = (msg) => { + t.window.appConfirm = async ({ message: msg }) => { t.state.confirms.push(msg); noticeAtConfirm = visible(t.notice()) ? noticeText(t) : null; return false; @@ -820,7 +820,7 @@ test('switching file during the confirm sends no retry, for either file', async await t.open('one\n'); t.editor().type('mine'); const other = '/home/u/.claude/projects/p/memory/other.md'; - t.window.confirm = (msg) => { + t.window.appConfirm = async ({ message: msg }) => { t.state.confirms.push(msg); t.panel.open('other', other, 'other\n'); return true; @@ -851,3 +851,89 @@ test('a stale refusal that comes back after the file was switched asks nothing a assert.equal(visible(t.notice()), false); } finally { t.destroy(); } }); + +test('reload consent for a previous file never reloads the new dirty file', async () => { + const t = setup({ disk: 'one' }); + try { + await t.open('one'); + t.editor().type('mine'); + let answer; + t.window.appConfirm = () => new Promise(resolve => { answer = resolve; }); + const reload = t.panel._reloadDiscardingEdits(); + await t.open('other', '/other.md'); + t.editor().type('keep'); + answer(true); + await reload; + assert.equal(t.panel.getContent(), 'otherkeep'); + } finally { t.destroy(); } +}); +test('overwrite consent retries against the disk value shown before a watcher update', async () => { + const t = setup({ disk: 'session' }); + try { + await t.open('one'); + t.editor().type('mine'); + let answer; + let asked = 0; + t.window.appConfirm = () => ++asked === 1 ? new Promise(resolve => { answer = resolve; }) : false; + const saving = t.panel._save(); + await tick(); + await t.externalWrite('new session'); + answer(true); + await saving; + assert.equal(t.state.disk, 'new session'); + assert.equal(t.state.saves[1].expected, 'session'); + assert.equal(asked, 2); + } finally { t.destroy(); } +}); + +test('Delete consent for an old viewer file never deletes its replacement', async () => { + const t = setup({ disk: 'one' }); + try { + await t.open('one'); + const deleted = []; + t.panel.opts.onDelete = async file => { deleted.push(file); return { ok: true }; }; + let answer; + t.window.appConfirm = () => new Promise(resolve => { answer = resolve; }); + const deleting = t.panel._delete(); + await t.open('other', '/other.md'); + answer(true); + await deleting; + assert.deepEqual(deleted, []); + assert.equal(t.panel.getContent(), 'other'); + } finally { t.destroy(); } +}); +for (const agree of [false, true]) { + test('viewer Delete waits and ' + (agree ? 'proceeds' : 'cancels'), async () => { + const t = setup({ disk: 'one' }); + try { + await t.open('one'); + const deleted = []; + let closed = 0; + t.panel.opts.onDelete = async file => { deleted.push(file); return { ok: true }; }; + t.panel.opts.onClose = () => { closed++; }; + let answer; + t.window.appConfirm = () => new Promise(resolve => { answer = resolve; }); + const deleting = t.panel._delete(); + assert.deepEqual(deleted, []); + answer(agree); + await deleting; + assert.deepEqual(deleted, agree ? [FILE] : []); + assert.equal(closed, agree ? 1 : 0); + } finally { t.destroy(); } + }); +} + +for (const throws of [false, true]) { + test('viewer Delete reports ' + (throws ? 'rejected IPC' : 'refusal') + ' through appAlert', async () => { + const t = setup({ disk: 'one' }); + try { + await t.open('one'); + const alerts = []; + t.window.appConfirm = async () => true; + t.window.appAlert = async ({ message }) => { alerts.push(message); }; + t.panel.opts.onDelete = async () => { if (throws) throw new Error('busy'); return { ok: false, error: 'busy' }; }; + await t.panel._delete(); + assert.deepEqual(alerts, ['Delete failed: busy']); + } finally { t.destroy(); } + }); +} From 6deb66a9259351793f0fb607d08c4db84a43b83d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 12 Oct 2026 13:56:54 +0200 Subject: [PATCH 02/10] (ui): fix review findings on the app dialogs (#585) The scheduled-run notice is an OK-only alert again, continuation prompts are not styled as dangerous so Enter accepts them, keys follow the topmost dialog even when focus drifted, global shortcuts wait while a dialog is open, the unsaved-edits question focuses Save again, Escape in the diff view is swallowed only when it acted, and Enter on a dangerous dialog's body does nothing. The lint ban also covers globalThis and self. --- .ai/contexts/app-dialogs.md | 8 +-- CHANGELOG.md | 2 +- eslint.config.js | 2 +- public/app.js | 5 +- public/choice-dialog.js | 5 +- public/file-panel.js | 30 +++++++----- public/resume-guard.js | 4 +- test/app-dialogs-lint.test.js | 2 +- test/continuations-harness.js | 3 +- test/dom-choice-dialog.test.js | 60 ++++++++++++++++++++++- test/dom-file-panel-changes.test.js | 48 ++++++++++++++++++ test/dom-file-panel-unsaved-guard.test.js | 22 +++++++-- test/restore-continuations.test.js | 24 +++++++++ test/restore-live-elsewhere.test.js | 3 +- test/resume-guard.test.js | 55 ++++++++++++--------- test/schedule-live-resume.test.js | 12 +++-- test/tool-bar.test.js | 17 +++++++ 17 files changed, 244 insertions(+), 58 deletions(-) diff --git a/.ai/contexts/app-dialogs.md b/.ai/contexts/app-dialogs.md index 4e614511..5c693668 100644 --- a/.ai/contexts/app-dialogs.md +++ b/.ai/contexts/app-dialogs.md @@ -1,11 +1,11 @@ # Renderer confirmations and messages -Use async `appConfirm({ title, message, confirmLabel, danger })` and `appAlert({ title, message })` from `public/app-dialogs.js`. Native `confirm`, `alert`, and `prompt` (bare or on `window`) are forbidden by ESLint throughout `public/**`. +Use async `appConfirm({ title, message, confirmLabel, danger })` and `appAlert({ title, message })` from `public/app-dialogs.js`. Native `confirm`, `alert`, and `prompt` (bare or on `window`, `globalThis`, or `self`) are forbidden by ESLint throughout `public/**`. The property rule matches named objects and cannot express the nested `document.defaultView` receiver; that spelling is not covered. -The adapter uses `showChoiceDialog`. All choice dialogs share a FIFO queue, including existing checkbox dialogs, so a busy dialog never becomes a cancellation. Confirm resolves to a boolean; an alert offers OK. Escape, the close button, and the backdrop dismiss the current dialog. Enter activates the focused button and never accepts a checkbox. Tab stays inside the dialog. Destructive confirmations focus Cancel; other dialogs focus the first action. Closing returns focus to the connected opener. +The adapter uses `showChoiceDialog`. All choice dialogs share a FIFO queue, including existing checkbox dialogs, so a busy dialog never becomes a cancellation. Confirm resolves to a boolean; an alert offers OK. The topmost choice overlay owns Escape and Tab even when focus drifts outside it. Escape, the close button, and the backdrop dismiss the current dialog. Tab stays inside it. Enter on a button activates that button; on a checkbox or outside the overlay it does nothing. Enter on the container accepts a non-danger dialog, but does nothing for `danger: true`, which requires a focused button. Destructive confirmations and the live-elsewhere warning focus Cancel; continuation prompts focus Open session. A running scheduled task shows an OK-only alert and refuses resume. Closing returns focus to the connected opener. App navigation and tool shortcuts are suppressed while a choice overlay is open. -Every caller must await the decision before its side effect. The Changes and Touched discard paths are async, including close, back, row switch, toggle, and reload. The quit/reload IPC already awaits `askAboutUnsavedEdits`, acknowledges immediately, and returns the decision only after the existing Save/Discard/Cancel guard. The beforeunload veto and main-process protocol are unchanged. +Every caller must await the decision before its side effect. The Changes and Touched discard paths are async, including close, back, row switch, toggle, and reload. `returnToPanelList()` returns a boolean for an immediate transition, or a promise when a discard decision is pending. The diff's Escape listener swallows the event only when the return has already succeeded; it leaves an unhandled or pending Escape available to other listeners. Main-process Electron dialogs remain outside this renderer rule. -The quit/reload guard ACKs immediately, then waits on `appDialogsIdle()` before showing its Save/Discard/Cancel controls and recollects the current buffers. It does not hold the choice queue while saving; a foreground message can therefore be dismissed without deadlock. Each dialog owns keys originating in its overlay. The unsaved guard focuses Cancel, traps Tab, and restores its connected opener. +The quit/reload guard ACKs immediately, waits on `appDialogsIdle()`, recollects the current buffers and shows its Save/Discard/Cancel controls. It does not hold the choice queue while saving; a foreground message can therefore be dismissed without deadlock. Its unsaved guard focuses Save (Save all for multiple files), so Enter invokes the browser's button activation, traps Tab and Shift+Tab, cancels with Escape, and restores its connected opener. The beforeunload veto and main-process protocol are unchanged. diff --git a/CHANGELOG.md b/CHANGELOG.md index bc49a964..3253ac09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,8 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ### Changed - **Archive Project** now keeps your settings and suspends schedules until you restore it from Settings → **Archived projects**. A separate **Delete Project** confirms the removal of settings, cache and schedule registration, while keeping your folders and files. (#568) - Markdown previews now show a file's frontmatter as a readable header above its content, including its name, description and metadata. (#593) -- Closing the window (the close button, `Alt`+`F4`) asks for confirmation first, with Cancel selected; a window that is busy at that moment asks once it is free. Quitting from the ☰ menu (Quit, or Exit on Windows) does not ask again, and a quit from outside (a logout, an update install) is not stopped by the question. On macOS it asks "Close the window?": Switchboard stays in the Dock, but its sessions stop. If the window has stopped responding, a single close waits, and closing it again a few seconds later ends it; never while one of its own confirmation boxes waits for an answer. A window closed while it is still starting asks once it has loaded. (#478) - Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. (#585) +- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. The unsaved-edits guard keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) ### Fixed - A session sent to the background is reattached at every restart, not only the first. (#546) diff --git a/eslint.config.js b/eslint.config.js index 5325ce1a..b1645fe1 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -420,7 +420,7 @@ module.exports = [ 'no-undef': 'error', 'no-unused-vars': ['warn', { args: 'none', varsIgnorePattern: '^_' }], 'no-restricted-globals': ['error', ...['confirm', 'alert', 'prompt'].map(name => ({ name, message: 'Use public/app-dialogs.js (appConfirm/appAlert).' }))], - 'no-restricted-properties': ['error', ...['confirm', 'alert', 'prompt'].map(property => ({ object: 'window', property, message: 'Use public/app-dialogs.js (appConfirm/appAlert).' }))], + 'no-restricted-properties': ['error', ...['window', 'globalThis', 'self'].flatMap(object => ['confirm', 'alert', 'prompt'].map(property => ({ object, property, message: 'Use public/app-dialogs.js (appConfirm/appAlert).' })))], 'no-implicit-globals': 'off', // renderer relies on script-scope globals by design 'no-empty': ['warn', { allowEmptyCatch: true }], 'no-unreachable': 'warn', diff --git a/public/app.js b/public/app.js index e1b5df64..b68f307a 100644 --- a/public/app.js +++ b/public/app.js @@ -1520,7 +1520,7 @@ async function openSessionNow(session, customOptions, { automatic = false, live, } if (!continuationResolved && customOptions?.type !== 'attach' && (!openSessions.has(session.sessionId) || openSessions.get(session.sessionId).closed)) { const originalId = session.sessionId; - session = await resolveResumeSession(session, { automatic, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session', danger: true }), resolutions: window._startupResumeResolutions }); + session = await resolveResumeSession(session, { automatic, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session' }), resolutions: window._startupResumeResolutions }); if (!session) return false; if (session.sessionId !== originalId) { live = undefined; @@ -1552,7 +1552,7 @@ async function openSessionNow(session, customOptions, { automatic = false, live, } // see .ai/contexts/cli-session-state.md ("Live elsewhere") and .ai/contexts/bg-agents.md ("Attach") - const verdict = customOptions?.type === 'attach' ? true : await guardResume(session, { automatic, live, allowBgAttach, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session', danger: true }) }); + const verdict = customOptions?.type === 'attach' ? true : await guardResume(session, { automatic, live, allowBgAttach, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session', danger: true }), alert: message => appAlert({ title: 'Scheduled task already running', message }) }); if (verdict === false) return false; if (verdict && typeof verdict === 'object' && verdict.attach) { customOptions = { type: 'attach', jobId: verdict.attach, cwd: verdict.cwd || projectPath }; @@ -1760,6 +1760,7 @@ initAgentsView(); function handleGlobalShortcut(e) { if (e._handled || e.type !== 'keydown') return; + if (document.querySelector('.choice-dialog-overlay')) return; if (typeof handleFocusShortcut === 'function' && handleFocusShortcut(e)) return; if (matchShortcut('agentsToggle', e, isMac, appShortcuts)) { e.preventDefault(); diff --git a/public/choice-dialog.js b/public/choice-dialog.js index d433416b..ff54c85e 100644 --- a/public/choice-dialog.js +++ b/public/choice-dialog.js @@ -149,7 +149,8 @@ function openChoiceDialog({ } function onKey(event) { - if (!overlay.contains(event.target)) return; + const overlays = document.querySelectorAll('.choice-dialog-overlay'); + if (overlays[overlays.length - 1] !== overlay) return; if (event.key === 'Escape') { event.preventDefault(); event.stopImmediatePropagation(); @@ -165,7 +166,7 @@ function openChoiceDialog({ if (!overlay.contains(event.target)) return; event.preventDefault(); if (buttons.includes(event.target)) event.target.click(); - else if (!(event.target.matches && event.target.matches('input[type="checkbox"]'))) accept(); + else if (!danger && !(event.target.matches && event.target.matches('input[type="checkbox"]'))) accept(); } } diff --git a/public/file-panel.js b/public/file-panel.js index 09ec54f0..5468e560 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -138,9 +138,10 @@ function initFilePanel() { if (event.defaultPrevented || event.isComposing) return; if (event.target.closest('.cm-panels, .cm-search, .cm-tooltip')) return; if (event.target.closest('input, textarea') && !event.target.closest('.cm-content')) return; - event.preventDefault(); - event.stopPropagation(); - returnToPanelList(); + if (returnToPanelList() === true) { + event.preventDefault(); + event.stopPropagation(); + } }); window.addEventListener('beforeunload', (event) => { @@ -366,22 +367,25 @@ async function handleClose() { endCurrentTab(sessionId, state, { pending }); } -async function returnToPanelList() { +function returnToPanelList() { const sessionId = currentPanelSessionId; if (!sessionId) return false; const state = getSessionState(sessionId); const tab = state.currentTab; if (tab?.type === 'changes' && tab.selectedFile && !tab.returnList) { - await closeChangesDiff(sessionId); - return !tab.selectedFile; + const closing = closeChangesDiff(sessionId); + return !tab.selectedFile || closing.then(() => !tab.selectedFile); } if (!tab?.returnList) return false; - if ((hasUnsavedChangesEdits(tab) && !await confirmDiscardChangesEdits(tab))) return false; - const previous = tab.returnList; - destroyCurrentTab(state, { stash: false }); - state.currentTab = previous; - if (currentPanelSessionId === sessionId) renderPanel(sessionId); - return true; + const restore = () => { + const previous = tab.returnList; + destroyCurrentTab(state, { stash: false }); + state.currentTab = previous; + if (currentPanelSessionId === sessionId) renderPanel(sessionId); + return true; + }; + if (hasUnsavedChangesEdits(tab)) return confirmDiscardChangesEdits(tab).then(confirmed => confirmed && restore()); + return restore(); } function snapshotPanelList(list, tab) { @@ -705,7 +709,7 @@ function showUnsavedEditsDialog(tabs) { dialog.appendChild(actions); overlay.appendChild(dialog); document.body.appendChild(overlay); - cancelBtn.focus(); + saveBtn.focus(); function finish(proceed) { overlay.remove(); diff --git a/public/resume-guard.js b/public/resume-guard.js index e5f3b3c2..56977b9b 100644 --- a/public/resume-guard.js +++ b/public/resume-guard.js @@ -17,7 +17,7 @@ async function liveElsewhereMany(sessionIds, { api } = {}) { } } -async function guardResume(session, { automatic = false, api, confirm, live, allowBgAttach = false } = {}) { +async function guardResume(session, { automatic = false, api, confirm, alert, live, allowBgAttach = false } = {}) { if (!session || session.type === 'terminal') return true; if (live === undefined) { try { @@ -30,7 +30,7 @@ async function guardResume(session, { automatic = false, api, confirm, live, all if (automatic && !(allowBgAttach && live.kind === 'bg')) return false; if (live.kind === 'schedule') { const where = live.cwd ? ` in ${live.cwd}` : ''; - await confirm(`This session's scheduled task is already running (pid ${live.pid}${where}).\n\n` + await alert(`This session's scheduled task is already running (pid ${live.pid}${where}).\n\n` + 'Wait for the scheduled run to finish before opening it here.'); return false; } diff --git a/test/app-dialogs-lint.test.js b/test/app-dialogs-lint.test.js index ac40b32d..850794ed 100644 --- a/test/app-dialogs-lint.test.js +++ b/test/app-dialogs-lint.test.js @@ -4,7 +4,7 @@ const path = require('node:path'); const { ESLint } = require('eslint'); for (const name of ['confirm', 'alert', 'prompt']) { - for (const prefix of ['', 'window.']) { + for (const prefix of ['', 'window.', 'globalThis.', 'self.']) { test('renderer lint rejects ' + prefix + name, async () => { const lint = new ESLint({ cwd: path.join(__dirname, '..') }); const [result] = await lint.lintText(prefix + name + '("native");', { filePath: 'public/dialog-ban-fixture.js' }); diff --git a/test/continuations-harness.js b/test/continuations-harness.js index 0dd5fe54..0cefa79f 100644 --- a/test/continuations-harness.js +++ b/test/continuations-harness.js @@ -41,6 +41,7 @@ function setup(t, graph, { live = {}, answer = false, savedEntries, chunkBytes } fs.writeFileSync(settingsFile, JSON.stringify({ openWorkingSet: saved })); const spawned = [], prompts = []; dom.window.appConfirm = async ({ message: message }) => { prompts.push(message); return typeof answer === 'function' ? answer(message) : answer; }; + dom.window.appAlert = async ({ message }) => { prompts.push(message); }; dom.window.api = { getSetting: async () => JSON.parse(fs.readFileSync(settingsFile, 'utf8')), setSetting: async (_, value) => fs.writeFileSync(settingsFile, JSON.stringify(value)), @@ -50,7 +51,7 @@ function setup(t, graph, { live = {}, answer = false, savedEntries, chunkBytes } getSessionsLiveElsewhere: async ids => Object.fromEntries(ids.filter(id => live[id]).map(id => [id, live[id]])), openTerminal: async (id, project, isNew, options) => { spawned.push({ id, project, options }); return { ok: true }; }, }; - vm.runInContext(fs.readFileSync(path.join(__dirname, '../public/resume-guard.js'), 'utf8'), ctx); + vm.runInContext(fs.readFileSync(path.join(__dirname, '../public/resume-guard.js'), 'utf8'), ctx, { filename: path.join(__dirname, '../public/resume-guard.js') }); vm.runInContext(` var openSessions = new Map(), sessionMap = new Map(), pendingSessions = new Map(); var activeSessionId = null, restoringWorkingSet = true, restorePlanner = null; diff --git a/test/dom-choice-dialog.test.js b/test/dom-choice-dialog.test.js index 9311daa7..e39658a3 100644 --- a/test/dom-choice-dialog.test.js +++ b/test/dom-choice-dialog.test.js @@ -191,10 +191,68 @@ test('choice dialog: a second call waits until the first closes', async () => { const ctx = setupSidebarDom(); try { const first = open(ctx); - const second = open(ctx); + let secondSettled = false; + const second = open(ctx, { title: 'Second dialog' }).then(value => { secondSettled = true; return value; }); + await Promise.resolve(); + assert.equal(secondSettled, false, 'the queued call must remain pending'); assert.equal(ctx.document.querySelectorAll('.modal-overlay').length, 1); confirmBtn(ctx).click(); assert.deepEqual({ ...(await first) }, { a: true, b: true }); + assert.equal(shown(ctx).querySelector('h2').textContent, 'Second dialog'); + assert.equal(secondSettled, false, 'opening the queued dialog must not settle it'); + cancelBtn(ctx).click(); + assert.equal(await second, null); + assert.equal(overlay(ctx), null); + } finally { ctx.destroy(); } +}); + +for (const keyName of ['Escape', 'Tab']) { + test(`choice dialog: ${keyName} works after focus drifts to the body`, async () => { + const ctx = setupSidebarDom(); + try { + const pending = open(ctx); + ctx.document.activeElement.blur(); + assert.equal(ctx.document.activeElement, ctx.document.body); + const event = key(ctx, ctx.document.body, keyName); + assert.equal(event.defaultPrevented, true); + if (keyName === 'Escape') assert.equal(await pending, null); + else { + assert.equal(ctx.document.activeElement, shown(ctx).querySelector('button')); + cancelBtn(ctx).click(); + assert.equal(await pending, null); + } + } finally { ctx.destroy(); } + }); +} + +test('choice dialog: only the topmost overlay handles keys from the body', async () => { + const ctx = setupSidebarDom(); + try { + const pending = open(ctx); + const foreground = ctx.document.createElement('div'); + foreground.className = 'choice-dialog-overlay'; + ctx.document.body.append(foreground); + key(ctx, ctx.document.body, 'Escape'); + assert.equal(ctx.document.querySelectorAll('.choice-dialog-overlay').length, 2); + foreground.remove(); + key(ctx, ctx.document.body, 'Escape'); + assert.equal(ctx.document.querySelector('.choice-dialog-overlay'), null); + assert.equal(await pending, null); + } finally { ctx.destroy(); } +}); + +test('choice dialog: Enter on a danger dialog container waits for a focused button', async () => { + const ctx = setupSidebarDom(); + try { + const pending = open(ctx, { danger: true, initialFocus: 'cancel' }); + const dialog = shown(ctx).querySelector('.choice-dialog'); + dialog.focus(); + key(ctx, dialog, 'Enter'); + assert.ok(overlay(ctx), 'container Enter must not accept a dangerous action'); + assert.equal(ctx.window.localStorage.length, 0); + confirmBtn(ctx).focus(); + key(ctx, ctx.document.activeElement, 'Enter'); + assert.deepEqual({ ...(await pending) }, { a: true, b: true }); } finally { ctx.destroy(); } }); diff --git a/test/dom-file-panel-changes.test.js b/test/dom-file-panel-changes.test.js index 7e98573d..8830e8ab 100644 --- a/test/dom-file-panel-changes.test.js +++ b/test/dom-file-panel-changes.test.js @@ -302,6 +302,54 @@ async function openFile(ctx, sessionId, filePath) { await flush(); } +test('Escape in a Changes diff with no return action reaches other listeners', async () => { + const ctx = setupFilePanelDom(); + try { + ctx.window.switchPanel('s1'); + await ctx.window.openChangesTab('s1'); + const diff = ctx.document.getElementById('changes-diff-view'); + let reached = 0; + ctx.document.addEventListener('keydown', event => { if (event.key === 'Escape') reached++; }); + const event = new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }); + diff.dispatchEvent(event); + await flush(); + assert.equal(event.defaultPrevented, false); + assert.equal(reached, 1); + assert.equal(ctx.stateOf('s1').currentTab.selectedFile, null); + } finally { ctx.destroy(); } +}); + +test('Escape that returns a clean Changes diff to its list is swallowed immediately', async () => { + const ctx = setupFilePanelDom(); + try { + await openFile(ctx, 's1', 'src/a.js'); + let reached = 0; + ctx.document.addEventListener('keydown', () => { reached++; }); + const event = new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }); + ctx.document.getElementById('changes-diff-view').dispatchEvent(event); + assert.equal(event.defaultPrevented, true); + assert.equal(reached, 0); + assert.equal(ctx.stateOf('s1').currentTab.selectedFile, null); + } finally { ctx.destroy(); } +}); + +test('Escape leaves a dirty Changes diff open and propagates when discard is cancelled', async () => { + const ctx = setupFilePanelDom({ confirmImpl: () => false }); + try { + await openFile(ctx, 's1', 'src/a.js'); + ctx.editors[0].box.text = 'unsaved edits'; + let reached = 0; + ctx.document.addEventListener('keydown', () => { reached++; }); + const event = new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }); + ctx.document.getElementById('changes-diff-view').dispatchEvent(event); + await flush(); + assert.equal(event.defaultPrevented, false); + assert.equal(reached, 1); + assert.ok(ctx.stateOf('s1').currentTab.selectedFile); + assert.equal(ctx.editors[0].box.text, 'unsaved edits'); + } finally { ctx.destroy(); } +}); + // --- Rendering --------------------------------------------------------- test('openChangesTab loads status and renders the summary, branch info, and one row per file', async () => { diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index 57b65753..e81c4c39 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -166,6 +166,22 @@ function beforeUnload(ctx) { return event; } +test('quit focuses Save so Enter saves the edits before approving quit', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A); + const checking = ctx.calls.check(60, 'quit'); + await flush(); + const save = button(ctx, 'unsaved-save'); + assert.equal(ctx.window.document.activeElement, save); + dialogKey(ctx, 'Enter'); + save.click(); + await checking; + assert.equal(ctx.disk.get(A), 'x0\nmine'); + assert.deepEqual(ctx.calls.answers, [{ id: 60, proceed: true }]); + } finally { ctx.destroy(); } +}); + test('with nothing unsaved the check is answered at once and no dialog shows', async () => { const ctx = setup(); try { @@ -687,11 +703,11 @@ test('reload ACK is immediate while confirmations queue; quit upgrade waits for await flush(); assert.ok(dialog(ctx)); assert.equal(ctx.window.document.querySelector('.choice-dialog-overlay'), null); - assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-cancel')); - dialogKey(ctx, 'Tab', { shiftKey: true }); assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-save')); + dialogKey(ctx, 'Tab', { shiftKey: true }); + assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-discard')); dialogKey(ctx, 'Tab'); - assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-cancel')); + assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-save')); button(ctx, 'unsaved-discard').click(); await checking; assert.deepEqual(ctx.calls.answers, [{ id: 44, proceed: true }]); diff --git a/test/restore-continuations.test.js b/test/restore-continuations.test.js index b5aa4666..12b1c45a 100644 --- a/test/restore-continuations.test.js +++ b/test/restore-continuations.test.js @@ -63,6 +63,30 @@ test('B6 a plain attach tab is excluded from the saved set', { timeout: 9000 }, assert.deepEqual(h.settings().openWorkingSet, []); }); +test('a continuation opens with Enter; only the live-elsewhere warning focuses Cancel', async t => { + const h = setup(t, { old: ['new'], new: [] }, { live: { new: { pid: 4242, cwd: '/elsewhere' } } }); + for (const file of ['choice-dialog.js', 'app-dialogs.js']) { + const filename = path.join(__dirname, '../public', file); + vm.runInContext(fs.readFileSync(filename, 'utf8'), h.ctx, { filename }); + } + const opening = h.click(); + const flush = () => new Promise(resolve => setImmediate(resolve)); + await flush(); + const doc = h.dom.window.document; + assert.ok(doc.activeElement.classList.contains('choice-dialog-confirm')); + assert.ok(!doc.activeElement.classList.contains('delete-worktree-confirm-btn')); + assert.equal(doc.activeElement.textContent, 'Open session'); + doc.activeElement.dispatchEvent(new h.dom.window.KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true })); + await flush(); + assert.match(doc.querySelector('.choice-dialog-body').textContent, /already running/); + assert.ok(doc.activeElement.classList.contains('choice-dialog-cancel')); + assert.deepEqual(h.spawned, []); + doc.querySelector('.choice-dialog-confirm').focus(); + doc.activeElement.dispatchEvent(new h.dom.window.KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true })); + await opening; + assert.deepEqual(h.spawned.map(call => call.id), ['new']); +}); + for (const children of [['new', 'missing'], ['missing', 'new']]) { test(`R3 restore drops a disk-missing sibling (${children.join(', ')}) and persists the existing id`, { timeout: 9000 }, async t => { const h = setup(t, { old: children, new: [] }); diff --git a/test/restore-live-elsewhere.test.js b/test/restore-live-elsewhere.test.js index d88c6860..79aa801a 100644 --- a/test/restore-live-elsewhere.test.js +++ b/test/restore-live-elsewhere.test.js @@ -60,9 +60,10 @@ function setup({ savedSet, liveIds, batchFails = false }) { openTerminal: async (sessionId) => { openTerminalCalls.push(sessionId); return { ok: true }; }, }; dom.window.appConfirm = async () => { throw new Error('an automatic resume must never prompt'); }; + dom.window.appAlert = async () => { throw new Error('an automatic resume must never alert'); }; vm.runInContext(read('utils.js'), ctx); - vm.runInContext(read('resume-guard.js'), ctx); + vm.runInContext(read('resume-guard.js'), ctx, { filename: path.join(__dirname, '..', 'public', 'resume-guard.js') }); vm.runInContext(` var openSessions = new Map(); var pendingSessions = new Map(); diff --git a/test/resume-guard.test.js b/test/resume-guard.test.js index 0df851a5..2e6820ec 100644 --- a/test/resume-guard.test.js +++ b/test/resume-guard.test.js @@ -38,23 +38,23 @@ function makeApi(answer) { function makeConfirm(result) { const messages = []; const confirm = async (msg) => { messages.push(msg); return result; }; - return { confirm, messages }; + return { confirm, alert: async msg => { messages.push(msg); }, messages }; } // --- Automatic resume (reload path, working-set restore) ------------------- test('an automatic resume of a session live elsewhere is refused, without asking', async () => { const api = makeApi(LIVE); - const { confirm, messages } = makeConfirm(true); - assert.equal(await guardResume(SESSION, { automatic: true, api, confirm }), false); + const { confirm, alert, messages } = makeConfirm(true); + assert.equal(await guardResume(SESSION, { automatic: true, api, confirm, alert }), false); assert.deepEqual(api.asked, ['sess-1']); assert.equal(messages.length, 0, 'nobody asked for this resume, so there is nobody to ask'); }); test('an automatic resume of a session live nowhere else goes ahead', async () => { const api = makeApi(null); - const { confirm, messages } = makeConfirm(false); - assert.equal(await guardResume(SESSION, { automatic: true, api, confirm }), true); + const { confirm, alert, messages } = makeConfirm(false); + assert.equal(await guardResume(SESSION, { automatic: true, api, confirm, alert }), true); assert.equal(messages.length, 0); }); @@ -62,8 +62,8 @@ test('an automatic resume of a session live nowhere else goes ahead', async () = test('a user resume of a session live elsewhere asks, and a cancel stops it', async () => { const api = makeApi(LIVE); - const { confirm, messages } = makeConfirm(false); - assert.equal(await guardResume(SESSION, { api, confirm }), false); + const { confirm, alert, messages } = makeConfirm(false); + assert.equal(await guardResume(SESSION, { api, confirm, alert }), false); assert.equal(messages.length, 1); assert.match(messages[0], /pid 4242/); assert.match(messages[0], /\/work\/proj/); @@ -71,14 +71,14 @@ test('a user resume of a session live elsewhere asks, and a cancel stops it', as test('a user resume of a session live elsewhere goes ahead once confirmed', async () => { const api = makeApi(LIVE); - const { confirm } = makeConfirm(true); - assert.equal(await guardResume(SESSION, { api, confirm }), true); + const { confirm, alert } = makeConfirm(true); + assert.equal(await guardResume(SESSION, { api, confirm, alert }), true); }); test('a user resume of a session live nowhere else does not ask', async () => { const api = makeApi(null); - const { confirm, messages } = makeConfirm(false); - assert.equal(await guardResume(SESSION, { api, confirm }), true); + const { confirm, alert, messages } = makeConfirm(false); + assert.equal(await guardResume(SESSION, { api, confirm, alert }), true); assert.equal(messages.length, 0); }); @@ -86,15 +86,15 @@ test('a user resume of a session live nowhere else does not ask', async () => { test('a plain terminal is never checked: it has no claude session to duplicate', async () => { const api = makeApi(LIVE); - const { confirm } = makeConfirm(false); - assert.equal(await guardResume({ sessionId: 't', type: 'terminal' }, { automatic: true, api, confirm }), true); + const { confirm, alert } = makeConfirm(false); + assert.equal(await guardResume({ sessionId: 't', type: 'terminal' }, { automatic: true, api, confirm, alert }), true); assert.equal(api.asked.length, 0); }); test('a failed check is silence: the resume proceeds as it did before the guard', async () => { const api = makeApi(new Error('No handler registered')); - const { confirm } = makeConfirm(false); - assert.equal(await guardResume(SESSION, { automatic: true, api, confirm }), true); + const { confirm, alert } = makeConfirm(false); + assert.equal(await guardResume(SESSION, { automatic: true, api, confirm, alert }), true); }); test('loaded as a classic script, the guard is a global the renderer can call', async () => { @@ -102,13 +102,24 @@ test('loaded as a classic script, the guard is a global the renderer can call', vm.runInContext(fs.readFileSync(GUARD_PATH, 'utf8'), ctx, { filename: GUARD_PATH }); assert.equal(typeof ctx.guardResume, 'function'); const api = makeApi(LIVE); - assert.equal(await ctx.guardResume(SESSION, { automatic: true, api, confirm: () => true }), false); + assert.equal(await ctx.guardResume(SESSION, { automatic: true, api, confirm: () => true, alert: () => assert.fail('an automatic resume must not alert') }), false); }); // --- Wiring in the shipped files ------------------------------------------- const APP_SRC = read('public/app.js'); +for (const [file, expression] of [ + ['test/restore-live-elsewhere.test.js', "read('resume-guard.js')"], + ['test/schedule-live-resume.test.js', "read('public/resume-guard.js')"], + ['test/continuations-harness.js', "fs.readFileSync(path.join(__dirname, '../public/resume-guard.js'), 'utf8')"], +]) { + test(`guard VM diagnostics name resume-guard.js in ${file}`, () => { + const line = read(file).split('\n').find(line => line.includes('vm.runInContext(' + expression)); + assert.ok(line?.includes('{ filename'), 'guard evaluation must carry its source filename'); + }); +} + function functionBody(src, signature) { const start = src.indexOf(signature); assert.notEqual(start, -1, `${signature} not found in app.js`); @@ -173,22 +184,22 @@ const LIVE_BG = { pid: 346590, cwd: '/w/em', startedAt: 1, kind: 'bg', jobId: 'b test('a user click on a session the daemon runs answers "attach", without asking', async () => { const api = makeApi(LIVE_BG); - const { confirm, messages } = makeConfirm(false); - assert.deepEqual(await guardResume(SESSION, { api, confirm }), { attach: 'bc3fd129', cwd: '/w/em' }); + const { confirm, alert, messages } = makeConfirm(false); + assert.deepEqual(await guardResume(SESSION, { api, confirm, alert }), { attach: 'bc3fd129', cwd: '/w/em' }); assert.equal(messages.length, 0); }); test('an automatic resume of a session the daemon runs is still refused', async () => { const api = makeApi(LIVE_BG); - const { confirm } = makeConfirm(true); - assert.equal(await guardResume(SESSION, { automatic: true, api, confirm }), false); + const { confirm, alert } = makeConfirm(true); + assert.equal(await guardResume(SESSION, { automatic: true, api, confirm, alert }), false); }); test('a live bg descriptor without a usable jobId is refused: no resume offer, no attach', async () => { for (const jobId of [null, undefined, '']) { const api = makeApi({ ...LIVE_BG, jobId }); - const { confirm, messages } = makeConfirm(true); - assert.equal(await guardResume(SESSION, { api, confirm }), false); + const { confirm, alert, messages } = makeConfirm(true); + assert.equal(await guardResume(SESSION, { api, confirm, alert }), false); assert.equal(messages.length, 0); } }); diff --git a/test/schedule-live-resume.test.js b/test/schedule-live-resume.test.js index efc092f4..e081396e 100644 --- a/test/schedule-live-resume.test.js +++ b/test/schedule-live-resume.test.js @@ -62,6 +62,7 @@ function setup(t, { sandbox = false } = {}) { const ctx = dom.getInternalVMContext(); const openTerminalCalls = []; const messages = []; + const confirmations = []; const settings = { global: {} }; const session = { sessionId: SID, projectPath: dir, name: 'Scheduled task' }; dom.window.api = { @@ -72,9 +73,10 @@ function setup(t, { sandbox = false } = {}) { getSetting: async (key) => settings[key], setSetting: async (key, value) => { settings[key] = value; }, }; - dom.window.appConfirm = async ({ message: message }) => { messages.push(message); return true; }; + dom.window.appConfirm = async ({ message }) => { confirmations.push(message); return true; }; + dom.window.appAlert = async ({ message }) => { messages.push(message); }; vm.runInContext(read('public/utils.js'), ctx); - vm.runInContext(read('public/resume-guard.js'), ctx); + vm.runInContext(read('public/resume-guard.js'), ctx, { filename: path.join(ROOT, 'public/resume-guard.js') }); vm.runInContext(` var openSessions = new Map(); var sessionMap = new Map(); @@ -117,7 +119,7 @@ function setup(t, { sandbox = false } = {}) { }); return { get child() { return children[0]; }, - spawns, done, messages, openTerminalCalls, session, ctx, settings, + spawns, done, messages, confirmations, openTerminalCalls, session, ctx, settings, dom, start: () => { const { claudeArgs } = buildScheduleCommand(SID, { name: 'Scheduled task', cli: {} }); main.runScheduleCommand(claudeArgs, dir, 'Scheduled task', () => done.push(true)); @@ -142,6 +144,7 @@ test('a click during a scheduled run refuses a second process even when the dial await h.open(); assert.equal(h.openTerminalCalls.length, 0, 'a live scheduled run must not spawn a second claude --resume'); assert.equal(h.messages.length, 1); + assert.deepEqual(h.confirmations, [], 'a scheduled run offers an OK-only notice, never an Open session confirmation'); assert.match(h.messages[0], /scheduled.*running/i); h.child.emit('exit', 0); await h.open(); @@ -223,7 +226,8 @@ test('the schedule verdict never offers a resume override or a daemon attach', a const messages = []; const result = await guardResume({ sessionId: SID }, { live: { kind: 'schedule', pid: 4242, cwd: '/scheduled' }, - confirm: (message) => { messages.push(message); return true; }, + confirm: () => assert.fail('a scheduled run must never offer a resume override'), + alert: async (message) => { messages.push(message); }, }); assert.equal(result, false); assert.equal(messages.length, 1); diff --git a/test/tool-bar.test.js b/test/tool-bar.test.js index 98d0b20d..c65bfdc8 100644 --- a/test/tool-bar.test.js +++ b/test/tool-bar.test.js @@ -61,6 +61,23 @@ function event(window, key, extra = {}) { return new window.KeyboardEvent(extra.type || 'keydown', { key, ctrlKey: true, shiftKey: true, bubbles: true, cancelable: true, ...extra }); } +test('an open choice dialog suppresses the global Changes shortcut and resumes it after closing', async () => { + const ctx = setup(); + try { + for (const file of ['choice-dialog.js', 'app-dialogs.js']) ctx.inCtx(fs.readFileSync(path.join(__dirname, '../public', file), 'utf8')); + const { handleGlobalShortcut } = loadAppFunctions(ctx.context, { functions: ['handleGlobalShortcut'] }); + const calls = []; + ctx.window.toggleChangesTab = id => calls.push(id); + const pending = ctx.window.appConfirm({ title: 'Discard edits?', danger: true }); + handleGlobalShortcut(event(ctx.window, 'e')); + assert.deepEqual(calls, []); + ctx.window.document.querySelector('.choice-dialog-cancel').click(); + assert.equal(await pending, false); + handleGlobalShortcut(event(ctx.window, 'e')); + assert.deepEqual(calls, ['s1']); + } finally { ctx.destroy(); } +}); + for (const [route, invoke] of [ ['Changes button', ctx => ctx.window.document.getElementById('changes-toggle-btn').click()], ['Touched button', ctx => ctx.window.document.getElementById('touched-toggle-btn').click()], From a600aecfd72a6803f84ab12d48cd0086d7765be9 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 12 Oct 2026 15:20:59 +0200 Subject: [PATCH 03/10] (ui): one modal stack routes keys to the topmost dialog (#585) Each dialog routed keys on its own, which left gaps: Enter from the body reached a dialog underneath, the unsaved-edits guard ignored Escape once focus drifted, and the terminal still took shortcuts and input under an open dialog. Every modal now registers on a stack; a single capture-phase router sends Escape, Enter and Tab to the topmost one, and app shortcuts, xterm shortcuts and PTY input wait while any modal is open. The unsaved guard waits for the dialog queue, follow-up questions included. --- .ai/contexts/app-dialogs.md | 22 ++- CHANGELOG.md | 4 +- eslint.config.js | 4 + public/app.js | 4 +- public/choice-dialog.js | 66 ++------ public/dialogs.js | 73 ++++---- public/file-panel.js | 26 +-- public/index.html | 1 + public/jsonl-viewer.js | 45 ++--- public/modal-stack.js | 119 +++++++++++++ public/resume-guard.js | 3 +- public/sidebar.js | 22 ++- public/terminal-manager.js | 5 +- public/touched-files-view.js | 14 +- public/whats-new.js | 11 +- test/delete-session.test.js | 1 - test/dom-choice-dialog.test.js | 7 +- test/dom-dispatch-dialog.test.js | 13 +- test/dom-file-panel-changes.test.js | 1 + test/dom-file-panel-diff-save.test.js | 2 +- test/dom-file-panel-goto-line.test.js | 2 +- test/dom-file-panel-touched.test.js | 13 +- test/dom-file-panel-unsaved-guard.test.js | 88 +++++++++- test/dom-ide-emulation-indicator.test.js | 2 +- test/dom-modal-routing.test.js | 197 ++++++++++++++++++++++ test/dom-new-session-menu.test.js | 2 +- test/dom-permission-mode-picker.test.js | 1 + test/dom-sandbox-toggle.test.js | 1 + test/dom-send-prompt-dialog.test.js | 5 +- test/dom-setup.js | 1 + test/jsonl-viewer-sanitize.test.js | 21 +++ test/modal-stack-order.test.js | 37 ++++ test/new-session-dialog-fit.test.js | 2 +- test/restore-continuations.test.js | 2 +- test/resume-guard.test.js | 13 ++ test/terminal-manager-harness.js | 3 +- test/theme-controls.test.js | 2 +- test/tool-bar.test.js | 5 +- test/whats-new.test.js | 2 + 39 files changed, 635 insertions(+), 207 deletions(-) create mode 100644 public/modal-stack.js create mode 100644 test/dom-modal-routing.test.js create mode 100644 test/modal-stack-order.test.js diff --git a/.ai/contexts/app-dialogs.md b/.ai/contexts/app-dialogs.md index 5c693668..79670386 100644 --- a/.ai/contexts/app-dialogs.md +++ b/.ai/contexts/app-dialogs.md @@ -1,11 +1,21 @@ -# Renderer confirmations and messages +# Renderer modal stack and questions -Use async `appConfirm({ title, message, confirmLabel, danger })` and `appAlert({ title, message })` from `public/app-dialogs.js`. Native `confirm`, `alert`, and `prompt` (bare or on `window`, `globalThis`, or `self`) are forbidden by ESLint throughout `public/**`. The property rule matches named objects and cannot express the nested `document.defaultView` receiver; that spelling is not covered. +`public/modal-stack.js` loads before every renderer consumer. It owns the modal stack and the only modal `keydown` listener, installed on `document` in capture phase. Each modal calls `registerModal({ overlay, cancel, enter, bodyEnter })` after mounting and calls the returned unregister function on every close path. The stack's last registration owns the keys; neither DOM class order nor the current event target chooses another modal. -The adapter uses `showChoiceDialog`. All choice dialogs share a FIFO queue, including existing checkbox dialogs, so a busy dialog never becomes a cancellation. Confirm resolves to a boolean; an alert offers OK. The topmost choice overlay owns Escape and Tab even when focus drifts outside it. Escape, the close button, and the backdrop dismiss the current dialog. Tab stays inside it. Enter on a button activates that button; on a checkbox or outside the overlay it does nothing. Enter on the container accepts a non-danger dialog, but does nothing for `danger: true`, which requires a focused button. Destructive confirmations and the live-elsewhere warning focus Cancel; continuation prompts focus Open session. A running scheduled task shows an OK-only alert and refuses resume. Closing returns focus to the connected opener. App navigation and tool shortcuts are suppressed while a choice overlay is open. +Registered surfaces are the choice dialog (including `appConfirm` and `appAlert`), New Session, Resume, remote launch, Send a prompt, Add Project, dispatch-agent, the unsaved-edits guard, session/worktree deletion, Touched info, What's new, and fullscreen transcript images. The New session popover is a menu and retains its own arrow-key navigation. -Every caller must await the decision before its side effect. The Changes and Touched discard paths are async, including close, back, row switch, toggle, and reload. `returnToPanelList()` returns a boolean for an immediate transition, or a promise when a discard decision is pending. The diff's Escape listener swallows the event only when the return has already succeeded; it leaves an unhandled or pending Escape available to other listeners. +Escape cancels only the top modal from anywhere, including the body and a terminal underneath. Tab and Shift+Tab cycle through that modal's enabled, visible controls; hidden controls and descendants of hidden or inert containers are excluded. A modal without controls focuses its overlay. Enter is stopped before any underlying listener or terminal can act. A focused button activates once, with default activation prevented. Each modal may supply its own Enter action. A default-prevented Enter remains consumed without an action. -Main-process Electron dialogs remain outside this renderer rule. +Choice questions keep the existing rules: Enter on a checkbox does nothing, and Enter on the container accepts an ordinary question. A dangerous question requires a focused button; container, body, or outside Enter cannot accept it. New Session retains #586: the selected permission mode receives initial focus, Enter on a mode or checkbox starts, Cancel cancels, and Enter in a text input does not start. Its body Enter also starts when it is the top modal. Resume, Add Project and dispatch retain their body Enter actions through the same router's explicit `bodyEnter` option. Enter in Add Project's path submits; in remote launch's path it validates and starts; Send a prompt uses Ctrl/Cmd+Enter. Other text-field keys retain their normal browser behavior inside the top modal. Keys outside the top modal have their defaults and propagation blocked. -The quit/reload guard ACKs immediately, waits on `appDialogsIdle()`, recollects the current buffers and shows its Save/Discard/Cancel controls. It does not hold the choice queue while saving; a foreground message can therefore be dismissed without deadlock. Its unsaved guard focuses Save (Save all for multiple files), so Enter invokes the browser's button activation, traps Tab and Shift+Tab, cancels with Escape, and restores its connected opener. The beforeunload veto and main-process protocol are unchanged. +`hasOpenModal()` suppresses `app.js`'s `handleGlobalShortcut`, the xterm custom key handler before tool/session-navigation/search/fullscreen/newline shortcuts, and the xterm `onData` route. This covers single terminals, grid terminals, panel shells, composition/input data, and direct handler calls even when focus drifts underneath a modal. Closing the last modal enables these paths again. Choice questions and the unsaved guard restore their connected opener; New Session keeps its +/terminal fallback restoration. + +## Question queue and the unsaved guard + +Use async `appConfirm({ title, message, confirmLabel, danger })` and `appAlert({ title, message })` from `public/app-dialogs.js`. They adapt `showChoiceDialog`, which shares `enqueueModalQuestion` with session/worktree deletion and the Save/Discard/Cancel guard. Busy questions stay pending rather than resolving as Cancel. Regular questions use FIFO order; only one queued question is mounted at a time. A question may sit on top of a configuration modal. + +The quit/reload guard ACKs immediately, then enqueues an idle question rather than waiting on an idle snapshot and opening separately. Idle questions yield to regular questions and wait for the modal stack to empty. A zero-delay task checks idleness after the current promise reactions, allowing a caller's next continuation question (including `resolveResumeSession`'s candidate loop and fallback) to enter the queue first. Buffers are recollected when the guard actually opens. The guard holds the question queue during saving; an alert requested meanwhile opens after the guard closes. Save failures stay inline. Save receives initial focus, Enter really saves, Escape cancels, and Tab/Shift+Tab are handled by the shared router. Disabled controls cannot act during an in-flight save. The beforeunload veto and main-process protocol are unchanged. `appDialogsIdle()` remains available and resolves after the question queue settles. + +Every decision caller awaits its answer before performing its side effect. Changes and Touched discard paths remain async. `returnToPanelList()` returns a boolean for an immediate transition or a promise while a discard decision is pending; the diff's Escape listener consumes only a return that already succeeded. Destructive confirmations and the live-elsewhere warning focus Cancel; continuation prompts focus Open session. The scheduled-task notice is an OK-only alert and refuses resume. `guardResume` defaults its notice adapter to `appAlert` in the renderer; outside the renderer, a missing adapter produces an explicit contract error. + +Native `confirm`, `alert`, and `prompt` (bare or on `window`, `globalThis`, or `self`) remain forbidden by ESLint throughout `public/**`. The property rule matches named objects and cannot express nested `document.defaultView`; that spelling is not covered. Main-process Electron dialogs remain outside this renderer rule. diff --git a/CHANGELOG.md b/CHANGELOG.md index 3253ac09..368021eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,10 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased ### Changed +- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. Only the top dialog receives keys, with terminal input and app shortcuts paused while it is open. The unsaved-edits guard waits for pending questions and keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) - **Archive Project** now keeps your settings and suspends schedules until you restore it from Settings → **Archived projects**. A separate **Delete Project** confirms the removal of settings, cache and schedule registration, while keeping your folders and files. (#568) - Markdown previews now show a file's frontmatter as a readable header above its content, including its name, description and metadata. (#593) -- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. (#585) -- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. The unsaved-edits guard keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) +- Closing the window (the close button, `Alt`+`F4`) asks for confirmation first, with Cancel selected; a window that is busy at that moment asks once it is free. Quitting from the ☰ menu (Quit, or Exit on Windows) does not ask again, and a quit from outside (a logout, an update install) is not stopped by the question. On macOS it asks "Close the window?": Switchboard stays in the Dock, but its sessions stop. If the window has stopped responding, a single close waits, and closing it again a few seconds later ends it; never while one of its own confirmation boxes waits for an answer. A window closed while it is still starting asks once it has loaded. (#478) ### Fixed - A session sent to the background is reattached at every restart, not only the first. (#546) diff --git a/eslint.config.js b/eslint.config.js index b1645fe1..2832c7d7 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -24,6 +24,10 @@ const ACTIVITY_CLASS_MESSAGE = 'Only public/session-activity-dom.js may write .c // public/sidebar.js (and equivalents). Kept readonly so reassignment // from another file gets flagged. const rendererCrossFileGlobals = { + registerModal: 'readonly', + hasOpenModal: 'readonly', + enqueueModalQuestion: 'readonly', + whenModalQuestionsIdle: 'readonly', TOOL_BAR: 'readonly', filePanelState: 'readonly', currentPanelSessionId: 'readonly', diff --git a/public/app.js b/public/app.js index b68f307a..2601b96c 100644 --- a/public/app.js +++ b/public/app.js @@ -306,7 +306,7 @@ async function runRestore(list, { retryAfterIndexing = !restoreIndexingDone } = const originalId = item.sessionId; const originalSession = s; let waitingForIndex = false; - s = await resolveResumeSession({ ...s, projectPath: item.projectPath, mergedMemberIds: [] }, { automatic: true, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session', danger: true }), resolutions: window._startupResumeResolutions, onHold: reason => { waitingForIndex = reason.waitingForIndex; } }); + s = await resolveResumeSession({ ...s, projectPath: item.projectPath, mergedMemberIds: [] }, { automatic: true, api: window.api, confirm: message => appConfirm({ title: 'Open session?', message, confirmLabel: 'Open session' }), resolutions: window._startupResumeResolutions, onHold: reason => { waitingForIndex = reason.waitingForIndex; } }); if (!s) { const index = restoreSavedIndex.has(originalId) ? restoreSavedIndex.get(originalId) : position; skippedWorkingSetEntries.set(originalId, { item, index, retryContinuation: retryAfterIndexing }); @@ -1760,7 +1760,7 @@ initAgentsView(); function handleGlobalShortcut(e) { if (e._handled || e.type !== 'keydown') return; - if (document.querySelector('.choice-dialog-overlay')) return; + if (hasOpenModal()) return; if (typeof handleFocusShortcut === 'function' && handleFocusShortcut(e)) return; if (matchShortcut('agentsToggle', e, isMac, appShortcuts)) { e.preventDefault(); diff --git a/public/choice-dialog.js b/public/choice-dialog.js index ff54c85e..6ddcd1fa 100644 --- a/public/choice-dialog.js +++ b/public/choice-dialog.js @@ -9,45 +9,22 @@ function writeRememberedChoice(rememberKey, checked) { } // see .ai/contexts/app-dialogs.md -const choiceDialogQueue = []; -const choiceDialogIdleWaiters = []; -let choiceDialogActive = false; let choiceDialogReturnFocus = null; let closeOpenChoiceDialog = null; function showChoiceDialog(options = {}) { const active = document.activeElement; const returnFocus = options.returnFocus || (active?.closest('.choice-dialog-overlay') ? choiceDialogReturnFocus : active); - return new Promise((resolve) => { - const entry = { options: { ...options, returnFocus }, resolve }; - if (options.replace && closeOpenChoiceDialog) { - choiceDialogQueue.unshift(entry); - closeOpenChoiceDialog(null); - } else choiceDialogQueue.push(entry); - drainChoiceDialogs(); + if (options.replace && closeOpenChoiceDialog) closeOpenChoiceDialog(null); + return enqueueModalQuestion(() => { + const restore = options.replace && returnFocus && returnFocus.isConnected === false ? document.activeElement : returnFocus; + choiceDialogReturnFocus = restore; + return openChoiceDialog({ ...options, returnFocus: restore }); }); } function whenChoiceDialogsIdle() { - if (!choiceDialogActive && !choiceDialogQueue.length) return Promise.resolve(); - return new Promise(resolve => choiceDialogIdleWaiters.push(resolve)); -} - -function drainChoiceDialogs() { - if (choiceDialogActive) return; - if (!choiceDialogQueue.length) { - for (const resolve of choiceDialogIdleWaiters.splice(0)) resolve(); - return; - } - choiceDialogActive = true; - const { options, resolve } = choiceDialogQueue.shift(); - if (options.replace && options.returnFocus && options.returnFocus.isConnected === false) options.returnFocus = document.activeElement; - choiceDialogReturnFocus = options.returnFocus; - openChoiceDialog(options).then((value) => { - choiceDialogActive = false; - resolve(value); - drainChoiceDialogs(); - }); + return whenModalQuestionsIdle(); } function openChoiceDialog({ @@ -59,11 +36,8 @@ function openChoiceDialog({ initialFocus = 'confirm', danger = false, returnFocus, - replace = false, closeWith = null, } = {}) { - - return new Promise((resolve) => { const overlay = document.createElement('div'); overlay.className = 'modal-overlay choice-dialog-overlay'; @@ -134,7 +108,7 @@ function openChoiceDialog({ closed = true; if (closeOpenChoiceDialog === close) closeOpenChoiceDialog = null; overlay.remove(); - document.removeEventListener('keydown', onKey, true); + unregister(); if (returnFocus?.isConnected && typeof returnFocus.focus === 'function') returnFocus.focus(); resolve(value); } @@ -148,26 +122,10 @@ function openChoiceDialog({ close(result); } - function onKey(event) { - const overlays = document.querySelectorAll('.choice-dialog-overlay'); - if (overlays[overlays.length - 1] !== overlay) return; - if (event.key === 'Escape') { - event.preventDefault(); - event.stopImmediatePropagation(); - close(null); - } else if (event.key === 'Tab') { - event.preventDefault(); - const controls = [...dialog.querySelectorAll('button, input')].filter(control => !control.hidden); - const at = controls.indexOf(document.activeElement); - const step = event.shiftKey ? -1 : 1; - const next = at === -1 ? (event.shiftKey ? controls.length - 1 : 0) : (at + step + controls.length) % controls.length; - controls[next].focus(); - } else if (event.key === 'Enter') { - if (!overlay.contains(event.target)) return; - event.preventDefault(); - if (buttons.includes(event.target)) event.target.click(); - else if (!danger && !(event.target.matches && event.target.matches('input[type="checkbox"]'))) accept(); - } + function enter(event) { + if (buttons.includes(event.target)) event.target.click(); + else if (!danger && !event.target.matches('input[type="checkbox"]')) accept(); + return true; } closeBtn.addEventListener('click', () => close(null)); @@ -176,7 +134,7 @@ function openChoiceDialog({ overlay.addEventListener('click', (event) => { if (event.target === overlay) close(null); }); document.body.appendChild(overlay); - document.addEventListener('keydown', onKey, true); + const unregister = registerModal({ overlay, cancel: () => close(null), enter }); closeOpenChoiceDialog = close; if (closeWith) closeWith.then(close, () => {}); (initialFocus === 'cancel' ? cancelBtn : confirmBtn).focus(); diff --git a/public/dialogs.js b/public/dialogs.js index 060a997f..9dab29dc 100644 --- a/public/dialogs.js +++ b/public/dialogs.js @@ -332,7 +332,7 @@ async function showNewSessionDialog(project, opener = document.activeElement) { function close(returnFocus = true) { overlay.remove(); - document.removeEventListener('keydown', onKey); + unregister(); if (!returnFocus) return; const targets = [ opener, @@ -377,17 +377,14 @@ async function showNewSessionDialog(project, opener = document.activeElement) { dialog.querySelector('.new-session-start-btn').onclick = start; overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); - // Keyboard support - function onKey(e) { - if (e.defaultPrevented || (!overlay.contains(e.target) && e.target !== document.body)) return; - if (e.key === 'Escape') close(); - if (e.key === 'Enter' && !e.target.matches('input:not([type="checkbox"])')) { - e.preventDefault(); + const unregister = registerModal({ overlay, cancel: () => close(), bodyEnter: true, enter: e => { + if (!e.target.matches('input:not([type="checkbox"])')) { if (e.target.closest('.new-session-cancel-btn')) close(); else start(); + return true; } - } - document.addEventListener('keydown', onKey); + return false; + } }); focusSelectedMode(); } @@ -465,7 +462,7 @@ function showRemoteLaunchDialog(project) { function close() { overlay.remove(); - document.removeEventListener('keydown', onKey); + unregister(); } function start() { @@ -485,11 +482,11 @@ function showRemoteLaunchDialog(project) { dialog.querySelector('.new-session-start-btn').onclick = start; overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); - function onKey(e) { - if (e.key === 'Escape') close(); - if (e.key === 'Enter' && e.target.matches('input')) start(); - } - document.addEventListener('keydown', onKey); + const unregister = registerModal({ overlay, cancel: close, enter: e => { + if (!e.target.matches('input')) return false; + start(); + return true; + } }); } async function showResumeSessionDialog(session) { @@ -587,7 +584,7 @@ async function showResumeSessionDialog(session) { function close() { overlay.remove(); - document.removeEventListener('keydown', onKey); + unregister(); } function resume() { @@ -616,11 +613,11 @@ async function showResumeSessionDialog(session) { dialog.querySelector('.new-session-start-btn').onclick = resume; overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); - function onKey(e) { - if (e.key === 'Escape') close(); - if (e.key === 'Enter' && !e.target.matches('input')) resume(); - } - document.addEventListener('keydown', onKey); + const unregister = registerModal({ overlay, cancel: close, bodyEnter: true, enter: e => { + if (e.target.matches('input, button')) return false; + resume(); + return true; + } }); } // see .ai/contexts/session-cache.md ("Remote hosts — sending a prompt") @@ -658,6 +655,7 @@ function showSendPromptDialog(session) { function close() { overlay.remove(); + unregister(); } async function send() { @@ -684,10 +682,11 @@ function showSendPromptDialog(session) { sendBtn.onclick = send; overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); - overlay.addEventListener('keydown', (e) => { - if (e.key === 'Escape') close(); - if (e.key === 'Enter' && (e.ctrlKey || e.metaKey)) send(); - }); + const unregister = registerModal({ overlay, cancel: close, enter: e => { + if (!e.ctrlKey && !e.metaKey) return false; + send(); + return true; + } }); } // Settings viewer is in settings-panel.js (openSettingsViewer / closeSettingsViewer) @@ -723,7 +722,7 @@ function showAddProjectDialog() { function close() { overlay.remove(); - document.removeEventListener('keydown', onKey); + unregister(); } async function addProject() { @@ -754,11 +753,11 @@ function showAddProjectDialog() { dialog.querySelector('.add-project-add-btn').onclick = addProject; overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); - function onKey(e) { - if (e.key === 'Escape') close(); - if (e.key === 'Enter') addProject(); - } - document.addEventListener('keydown', onKey); + const unregister = registerModal({ overlay, cancel: close, bodyEnter: true, enter: e => { + if (e.target.matches('button')) return false; + addProject(); + return true; + } }); } // see .ai/contexts/bg-agents.md @@ -926,7 +925,7 @@ async function showDispatchAgentDialog(project) { function close() { overlay.remove(); - document.removeEventListener('keydown', onKey); + unregister(); } async function start() { @@ -962,10 +961,10 @@ async function showDispatchAgentDialog(project) { startBtn.onclick = start; overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); - function onKey(e) { - if (e.key === 'Escape') close(); - if (e.key === 'Enter' && !e.target.matches('input, textarea, select, button')) start(); - } - document.addEventListener('keydown', onKey); + const unregister = registerModal({ overlay, cancel: close, bodyEnter: true, enter: e => { + if (e.target.matches('input, textarea, select, button')) return false; + start(); + return true; + } }); dialog.querySelector('#dad-prompt').focus(); } diff --git a/public/file-panel.js b/public/file-panel.js index 5468e560..8f0798e3 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -713,26 +713,13 @@ function showUnsavedEditsDialog(tabs) { function finish(proceed) { overlay.remove(); - document.removeEventListener('keydown', onKey); + unregister(); if (returnFocus?.isConnected) returnFocus.focus(); resolve(proceed); } - function onKey(e) { - if (!overlay.contains(e.target)) return; - if (e.key === 'Escape') { - e.preventDefault(); - e.stopImmediatePropagation(); - finish(false); - } else if (e.key === 'Tab') { - e.preventDefault(); - const controls = [cancelBtn, discardBtn, saveBtn].filter(button => !button.disabled); - if (!controls.length) return; - const at = controls.indexOf(document.activeElement); - const step = e.shiftKey ? -1 : 1; - controls[(at + step + controls.length) % controls.length].focus(); - } - } - document.addEventListener('keydown', onKey); + const unregister = registerModal({ overlay, cancel: () => { + if (!cancelBtn.disabled) finish(false); + } }); cancelBtn.onclick = () => finish(false); discardBtn.onclick = () => finish(true); @@ -792,11 +779,10 @@ function askAboutUnsavedEdits() { if (unsavedPrompt) return unsavedPrompt; const tabs = collectUnsavedFileTabs(); if (!tabs.length) return Promise.resolve(true); - unsavedPrompt = (async () => { - if (typeof appDialogsIdle === 'function') await appDialogsIdle(); + unsavedPrompt = enqueueModalQuestion(() => { const currentTabs = collectUnsavedFileTabs(); return currentTabs.length ? showUnsavedEditsDialog(currentTabs) : true; - })().then((proceed) => { + }, { whenIdle: true }).then((proceed) => { unsavedPrompt = null; if (proceed) { unloadApproved = true; diff --git a/public/index.html b/public/index.html index a3d06f21..0ba1324c 100644 --- a/public/index.html +++ b/public/index.html @@ -141,6 +141,7 @@ + diff --git a/public/jsonl-viewer.js b/public/jsonl-viewer.js index 7cb7f03e..6b1f7607 100644 --- a/public/jsonl-viewer.js +++ b/public/jsonl-viewer.js @@ -522,7 +522,26 @@ function mergeLocalCommandBlocks(blocks) { return result; } -// Render a tool result into a container, handling images, text, and mixed content +// see .ai/contexts/app-dialogs.md +function showJsonlScreenshot(src) { + const returnFocus = document.activeElement; + const overlay = document.createElement('div'); + overlay.className = 'jsonl-screenshot-fullscreen'; + overlay.tabIndex = -1; + const image = document.createElement('img'); + image.src = src; + overlay.appendChild(image); + function close() { + overlay.remove(); + unregister(); + if (returnFocus?.isConnected) returnFocus.focus(); + } + overlay.onclick = close; + document.body.appendChild(overlay); + const unregister = registerModal({ overlay, cancel: close }); + overlay.focus(); +} + function renderToolResult(resultData, container) { // Try to extract image data from the result const images = extractImages(resultData); @@ -537,13 +556,7 @@ function renderToolResult(resultData, container) { imgEl.src = img.src; if (img.alt) imgEl.alt = img.alt; imgEl.onclick = () => { - const overlay = document.createElement('div'); - overlay.className = 'jsonl-screenshot-fullscreen'; - const fullImg = document.createElement('img'); - fullImg.src = img.src; - overlay.appendChild(fullImg); - overlay.onclick = () => overlay.remove(); - document.body.appendChild(overlay); + showJsonlScreenshot(img.src); }; container.appendChild(imgEl); } @@ -792,13 +805,7 @@ async function showJsonlViewer(session) { // Click-to-fullscreen for inline images jsonlViewerBody.querySelectorAll('.jsonl-clickable-img').forEach(img => { img.onclick = () => { - const overlay = document.createElement('div'); - overlay.className = 'jsonl-screenshot-fullscreen'; - const fullImg = document.createElement('img'); - fullImg.src = img.src; - overlay.appendChild(fullImg); - overlay.onclick = () => overlay.remove(); - document.body.appendChild(overlay); + showJsonlScreenshot(img.src); }; }); @@ -883,13 +890,7 @@ async function showSubagentTranscript(session) { // Click-to-fullscreen for inline images jsonlViewerBody.querySelectorAll('.jsonl-clickable-img').forEach(img => { img.onclick = () => { - const overlay = document.createElement('div'); - overlay.className = 'jsonl-screenshot-fullscreen'; - const fullImg = document.createElement('img'); - fullImg.src = img.src; - overlay.appendChild(fullImg); - overlay.onclick = () => overlay.remove(); - document.body.appendChild(overlay); + showJsonlScreenshot(img.src); }; }); diff --git a/public/modal-stack.js b/public/modal-stack.js new file mode 100644 index 00000000..d6e3ad4b --- /dev/null +++ b/public/modal-stack.js @@ -0,0 +1,119 @@ +// see .ai/contexts/app-dialogs.md +const modalStack = []; +const modalQuestions = []; +const modalIdleQuestions = []; +const modalQuestionIdleWaiters = []; +let modalQuestionActive = false; +let modalIdleTimer = null; + +function hasOpenModal() { + return modalStack.length > 0; +} + +function registerModal({ overlay, cancel, enter, bodyEnter = false }) { + const previousZIndex = overlay.style.zIndex; + const top = modalStack[modalStack.length - 1]; + overlay.style.zIndex = String(Math.max(Number(getComputedStyle(overlay).zIndex) || 0, + Number(top?.overlay.style.zIndex) || 0) + 1); + const modal = { overlay, cancel, enter, bodyEnter }; + modalStack.push(modal); + return () => { + const index = modalStack.indexOf(modal); + if (index !== -1) { + modalStack.splice(index, 1); + overlay.style.zIndex = previousZIndex; + } + drainModalQuestions(); + }; +} + +function modalControls(overlay) { + return [...overlay.querySelectorAll('button, input, select, textarea, a[href], [tabindex]')] + .filter(control => control.tabIndex >= 0 && !control.matches(':disabled') && !control.closest('[hidden], [inert]')) + .filter(control => { + for (let element = control; element; element = element.parentElement) { + const style = getComputedStyle(element); + if (style.display === 'none' || style.visibility === 'hidden' || style.visibility === 'collapse') return false; + } + return true; + }); +} + +function routeModalKey(event) { + const modal = modalStack[modalStack.length - 1]; + if (!modal) return; + const inside = modal.overlay.contains(event.target); + if (event.key === 'Escape') { + event.preventDefault(); + event.stopImmediatePropagation(); + modal.cancel(); + } else if (event.key === 'Tab') { + event.preventDefault(); + event.stopImmediatePropagation(); + const controls = modalControls(modal.overlay); + if (!controls.length) { modal.overlay.focus(); return; } + const at = controls.indexOf(document.activeElement); + const next = at === -1 ? (event.shiftKey ? controls.length - 1 : 0) + : (at + (event.shiftKey ? -1 : 1) + controls.length) % controls.length; + controls[next].focus(); + } else if (event.key === 'Enter') { + event.stopImmediatePropagation(); + if (event.defaultPrevented) return; + if (!inside && !(modal.bodyEnter && event.target === document.body)) { event.preventDefault(); return; } + if (modal.enter?.(event)) event.preventDefault(); + else if (event.target.matches?.('button:not(:disabled)')) { + event.preventDefault(); + event.target.click(); + } + } else if (!inside) { + event.preventDefault(); + event.stopImmediatePropagation(); + } +} + +document.addEventListener('keydown', routeModalKey, true); + +function enqueueModalQuestion(open, { whenIdle = false } = {}) { + return new Promise((resolve, reject) => { + (whenIdle ? modalIdleQuestions : modalQuestions).push({ open, resolve, reject }); + drainModalQuestions(); + }); +} + +function openModalQuestion(question) { + modalQuestionActive = true; + let result; + try { result = question.open(); } catch (error) { result = Promise.reject(error); } + Promise.resolve(result).then(value => finish(null, value), error => finish(error)); + function finish(error, value) { + modalQuestionActive = false; + if (error) question.reject(error); + else question.resolve(value); + drainModalQuestions(); + } +} + +function drainModalQuestions() { + if (modalQuestionActive) return; + if (modalQuestions.length) { + openModalQuestion(modalQuestions.shift()); + return; + } + if (modalIdleTimer !== null) return; + modalIdleTimer = setTimeout(() => { + modalIdleTimer = null; + if (modalQuestionActive || modalQuestions.length) { drainModalQuestions(); return; } + if (modalIdleQuestions.length) { + if (!hasOpenModal()) openModalQuestion(modalIdleQuestions.shift()); + return; + } + for (const resolve of modalQuestionIdleWaiters.splice(0)) resolve(); + }, 0); +} + +function whenModalQuestionsIdle() { + return new Promise(resolve => { + modalQuestionIdleWaiters.push(resolve); + drainModalQuestions(); + }); +} diff --git a/public/resume-guard.js b/public/resume-guard.js index 56977b9b..3b337b0b 100644 --- a/public/resume-guard.js +++ b/public/resume-guard.js @@ -17,7 +17,7 @@ async function liveElsewhereMany(sessionIds, { api } = {}) { } } -async function guardResume(session, { automatic = false, api, confirm, alert, live, allowBgAttach = false } = {}) { +async function guardResume(session, { automatic = false, api, confirm, alert = typeof appAlert === 'function' ? message => appAlert({ message }) : undefined, live, allowBgAttach = false } = {}) { if (!session || session.type === 'terminal') return true; if (live === undefined) { try { @@ -29,6 +29,7 @@ async function guardResume(session, { automatic = false, api, confirm, alert, li if (!live) return true; if (automatic && !(allowBgAttach && live.kind === 'bg')) return false; if (live.kind === 'schedule') { + if (typeof alert !== 'function') throw new TypeError('guardResume requires an alert adapter for a scheduled task notice'); const where = live.cwd ? ` in ${live.cwd}` : ''; await alert(`This session's scheduled task is already running (pid ${live.pid}${where}).\n\n` + 'Wait for the scheduled run to finish before opening it here.'); diff --git a/public/sidebar.js b/public/sidebar.js index f5bbc54c..6faec65c 100644 --- a/public/sidebar.js +++ b/public/sidebar.js @@ -1905,7 +1905,7 @@ async function showDeleteSessionDialog(session) { ? window.api.deleteSessionPreview(session.sessionId) : Promise.resolve(null); - return new Promise((resolve) => { + return enqueueModalQuestion(() => new Promise((resolve) => { const overlay = document.createElement('div'); overlay.className = 'new-session-overlay'; @@ -1953,7 +1953,7 @@ async function showDeleteSessionDialog(session) { function close(confirmed) { overlay.remove(); - document.removeEventListener('keydown', onKey); + unregister(); resolve(confirmed); } @@ -1961,9 +1961,9 @@ async function showDeleteSessionDialog(session) { dialog.querySelector('#dss-confirm').onclick = () => close(true); overlay.addEventListener('click', (e) => { if (e.target === overlay) close(false); }); - function onKey(e) { if (e.key === 'Escape') close(false); } - document.addEventListener('keydown', onKey); - }); + const unregister = registerModal({ overlay, cancel: () => close(false) }); + dialog.querySelector('#dss-cancel').focus(); + })); } // --- Delete worktree confirmation dialog --- @@ -1972,7 +1972,7 @@ async function showDeleteWorktreeDialog(name, worktreePath) { // Fetch worktree status (dirty files) while the dialog is shown const statusPromise = window.api.worktreeStatus(worktreePath); - return new Promise((resolve) => { + return enqueueModalQuestion(() => new Promise((resolve) => { const overlay = document.createElement('div'); overlay.className = 'new-session-overlay'; @@ -2023,7 +2023,7 @@ async function showDeleteWorktreeDialog(name, worktreePath) { function close(confirmed) { overlay.remove(); - document.removeEventListener('keydown', onKey); + unregister(); resolve(confirmed); } @@ -2031,9 +2031,7 @@ async function showDeleteWorktreeDialog(name, worktreePath) { dialog.querySelector('#dwt-confirm').onclick = () => close(true); overlay.addEventListener('click', (e) => { if (e.target === overlay) close(false); }); - function onKey(e) { - if (e.key === 'Escape') close(false); - } - document.addEventListener('keydown', onKey); - }); + const unregister = registerModal({ overlay, cancel: () => close(false) }); + dialog.querySelector('#dwt-cancel').focus(); + })); } diff --git a/public/terminal-manager.js b/public/terminal-manager.js index 776985ff..7b321d31 100644 --- a/public/terminal-manager.js +++ b/public/terminal-manager.js @@ -79,8 +79,7 @@ function handleFocusShortcut(e) { const toSidebar = matchShortcut('focusSidebar', e, isMac, appShortcuts); if (!toSidebar && !matchShortcut('focusTerminal', e, isMac, appShortcuts)) return false; if (e.type !== 'keydown' || e._handled) return true; - const overlays = document.querySelectorAll('.modal-overlay, .new-session-overlay, .add-project-overlay, .whats-new-overlay, .jsonl-screenshot-fullscreen, [aria-modal="true"]'); - if ([...overlays].some(isFocusTargetVisible)) return false; + if (hasOpenModal()) return false; const focused = document.activeElement; if (toSidebar ? !focused?.closest('.terminal-container') : focused !== document.body && !focused?.closest('#sidebar')) return false; e.preventDefault(); @@ -106,6 +105,7 @@ function handleFocusShortcut(e) { function setupTerminalKeyBindings(terminal, container, getSessionId, { onFind } = {}) { terminal.attachCustomKeyEventHandler((e) => { + if (hasOpenModal()) return false; if (handleFocusShortcut(e)) return false; // Cmd/Ctrl+F → open terminal search bar if (e.key === 'f' && (isMac ? e.metaKey : e.ctrlKey) && !e.shiftKey && !e.altKey) { @@ -1185,6 +1185,7 @@ function createTerminalEntry(session, opts = {}) { // Wire up IPC (use entry.session.sessionId so fork re-keying works) terminal.onData(data => { + if (hasOpenModal()) return; if (data === '\x1b[I' || data === '\x1b[O') return; window.api.sendInput(entry.session.sessionId, data); }); diff --git a/public/touched-files-view.js b/public/touched-files-view.js index 073d019b..8155a114 100644 --- a/public/touched-files-view.js +++ b/public/touched-files-view.js @@ -139,23 +139,13 @@ function showTouchedInfo(button) { const closeButton = overlay.querySelector('button'); const close = () => { overlay.remove(); - document.removeEventListener('keydown', onKey, true); + unregister(); button.focus(); }; - function onKey(event) { - if (event.key === 'Escape') { - event.preventDefault(); - event.stopImmediatePropagation(); - close(); - } else if (event.key === 'Tab') { - event.preventDefault(); - closeButton.focus(); - } - } closeButton.addEventListener('click', close); overlay.addEventListener('click', event => { if (event.target === overlay) close(); }); document.body.appendChild(overlay); - document.addEventListener('keydown', onKey, true); + const unregister = registerModal({ overlay, cancel: close }); closeButton.focus(); } diff --git a/public/whats-new.js b/public/whats-new.js index 2d30e17d..6550f546 100644 --- a/public/whats-new.js +++ b/public/whats-new.js @@ -68,16 +68,9 @@ function showWhatsNew(doc, api, escape, payload) { function close() { overlay.remove(); - doc.removeEventListener('keydown', onKey, true); + unregister(); api.whatsNewDismissed(); } - function onKey(e) { - if (e.key === 'Escape') { - e.preventDefault(); - e.stopPropagation(); - close(); - } - } overlay.querySelector('.whats-new-close').addEventListener('click', close); overlay.addEventListener('click', (e) => { @@ -89,7 +82,7 @@ function showWhatsNew(doc, api, escape, payload) { close(); } }); - doc.addEventListener('keydown', onKey, true); + const unregister = doc.defaultView.registerModal({ overlay, cancel: close }); overlay.querySelector('.whats-new-close').focus(); return overlay; } diff --git a/test/delete-session.test.js b/test/delete-session.test.js index 6ca1caf2..a422b51c 100644 --- a/test/delete-session.test.js +++ b/test/delete-session.test.js @@ -132,7 +132,6 @@ test('delete button: uses the styled dialog, not a renderer-blocking native prom const dlg = sidebar.slice(start, sidebar.indexOf('// --- Delete worktree', start)); assert.match(dlg, /deleteSessionPreview/, 'it must state what will be lost'); assert.match(dlg, /Archive/, 'and point at the non-destructive alternative'); - assert.match(dlg, /Escape/, 'Esc must cancel, like the worktree dialog'); }); test('delete button: closes the tab so it cannot point at a deleted transcript', () => { diff --git a/test/dom-choice-dialog.test.js b/test/dom-choice-dialog.test.js index e39658a3..c78d0695 100644 --- a/test/dom-choice-dialog.test.js +++ b/test/dom-choice-dialog.test.js @@ -232,9 +232,10 @@ test('choice dialog: only the topmost overlay handles keys from the body', async const foreground = ctx.document.createElement('div'); foreground.className = 'choice-dialog-overlay'; ctx.document.body.append(foreground); + const unregister = ctx.window.registerModal({ overlay: foreground, cancel: () => { foreground.remove(); unregister(); } }); key(ctx, ctx.document.body, 'Escape'); - assert.equal(ctx.document.querySelectorAll('.choice-dialog-overlay').length, 2); - foreground.remove(); + assert.equal(ctx.document.querySelectorAll('.choice-dialog-overlay').length, 1); + assert.ok(overlay(ctx), 'only the registered topmost modal closes'); key(ctx, ctx.document.body, 'Escape'); assert.equal(ctx.document.querySelector('.choice-dialog-overlay'), null); assert.equal(await pending, null); @@ -306,7 +307,7 @@ test('choice dialog: Enter outside the dialog does nothing', async () => { const pending = open(ctx); const event = key(ctx, outside, 'Enter'); assert.ok(overlay(ctx), 'the dialog must stay open'); - assert.equal(event.defaultPrevented, false); + assert.equal(event.defaultPrevented, true); cancelBtn(ctx).click(); assert.equal(await pending, null); } finally { ctx.destroy(); } diff --git a/test/dom-dispatch-dialog.test.js b/test/dom-dispatch-dialog.test.js index 3d151c2e..75c9f85c 100644 --- a/test/dom-dispatch-dialog.test.js +++ b/test/dom-dispatch-dialog.test.js @@ -27,7 +27,7 @@ function setup({ dispatchResult = { ok: true, id: 'cccccccc' }, projects, effect openSessions: new Map(), activePtyIds: new Set(), refreshSidebar: () => {}, pollActiveSessions: () => {}, }; for (const [k, v] of Object.entries(g)) Object.defineProperty(window, k, { value: v, writable: true, configurable: true }); - for (const f of ['setting-defaults.js', 'utils.js', 'icons.js', 'dialogs.js']) { + for (const f of ['modal-stack.js', 'setting-defaults.js', 'utils.js', 'icons.js', 'dialogs.js']) { vm.runInContext(fs.readFileSync(path.join(PUBLIC, f), 'utf8'), dom.getInternalVMContext(), { filename: path.join(PUBLIC, f) }); } return { window, document: window.document, calls, destroy: () => window.close() }; @@ -80,15 +80,20 @@ test('Enter inside the prompt does not start; Escape closes', async (t) => { assert.equal(d.querySelector('.new-session-overlay'), null); }); -test('Enter on a focused button is left to the button: Enter on Cancel never starts; Enter elsewhere does', async (t) => { +test('Enter on Cancel closes without dispatching; Enter on the dialog starts', async (t) => { const ctx = setup(); t.after(ctx.destroy); await ctx.window.showDispatchAgentDialog(null); const d = ctx.document; d.querySelector('#dad-prompt').value = 'go'; - d.querySelector('.new-session-cancel-btn').dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + const cancel = d.querySelector('.new-session-cancel-btn'); + cancel.focus(); + cancel.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true })); await tick(); await tick(); assert.equal(ctx.calls.dispatched.length, 0); - d.body.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + assert.equal(d.querySelector('.new-session-overlay'), null); + await ctx.window.showDispatchAgentDialog(null); + d.querySelector('#dad-prompt').value = 'go'; + d.querySelector('.new-session-dialog').dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true })); await tick(); await tick(); assert.equal(ctx.calls.dispatched.length, 1); }); diff --git a/test/dom-file-panel-changes.test.js b/test/dom-file-panel-changes.test.js index 8830e8ab..8bd0ddab 100644 --- a/test/dom-file-panel-changes.test.js +++ b/test/dom-file-panel-changes.test.js @@ -201,6 +201,7 @@ function setupFilePanelDom({ statusImpl, diffImpl, fileImpl, saveImpl, confirmIm Object.defineProperty(window, 'activeSessionId', { value: null, writable: true, configurable: true }); + evalInWindow(dom, path.join(PUBLIC_DIR, 'modal-stack.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'splitter.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'session-state.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'session-activity-dom.js')); diff --git a/test/dom-file-panel-diff-save.test.js b/test/dom-file-panel-diff-save.test.js index 860d70b3..0ed81a1d 100644 --- a/test/dom-file-panel-diff-save.test.js +++ b/test/dom-file-panel-diff-save.test.js @@ -60,7 +60,7 @@ function setup({ saveImpl, confirmAnswer = true } = {}) { }); Object.defineProperty(window, 'activeSessionId', { value: null, writable: true, configurable: true }); - for (const f of ['viewer-toolbar.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'file-panel.js']) { + for (const f of ['modal-stack.js', 'viewer-toolbar.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'file-panel.js']) { vm.runInContext(fs.readFileSync(path.join(PUBLIC_DIR, f), 'utf8'), dom.getInternalVMContext(), { filename: path.join(PUBLIC_DIR, f) }); } window.openSessions = new Map(); diff --git a/test/dom-file-panel-goto-line.test.js b/test/dom-file-panel-goto-line.test.js index 69980e43..74de95ce 100644 --- a/test/dom-file-panel-goto-line.test.js +++ b/test/dom-file-panel-goto-line.test.js @@ -53,7 +53,7 @@ function setup({ pair } = {}) { window.createEditableViewer = (parent) => makeView(parent, 'plain'); Object.defineProperty(window, 'activeSessionId', { value: null, writable: true, configurable: true }); - for (const f of ['splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'viewer-toolbar.js', 'file-panel.js', 'touched-files-view.js']) { + for (const f of ['modal-stack.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'viewer-toolbar.js', 'file-panel.js', 'touched-files-view.js']) { vm.runInContext(fs.readFileSync(path.join(PUBLIC_DIR, f), 'utf8'), dom.getInternalVMContext(), { filename: f }); } window.openSessions = new Map(); diff --git a/test/dom-file-panel-touched.test.js b/test/dom-file-panel-touched.test.js index d06f1c3b..289a7c62 100644 --- a/test/dom-file-panel-touched.test.js +++ b/test/dom-file-panel-touched.test.js @@ -332,7 +332,7 @@ function setupDom({ touchedImpl, readImpl, confirmImpl, storedRatio, storageThro const bundledMarked = window.marked; delete window.marked; window.loadCodeMirrorBundle = () => { window.marked = bundledMarked; return Promise.resolve(); }; - for (const file of ['viewer-toolbar.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'file-panel.js', 'touched-files-view.js']) { + for (const file of ['modal-stack.js', 'viewer-toolbar.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'file-panel.js', 'touched-files-view.js']) { evalInWindow(dom, path.join(PUBLIC_DIR, file)); } window.openSessions = new Map(); @@ -369,6 +369,14 @@ function flush() { return p; } +async function waitForUnsavedDialog(ctx) { + for (let i = 0; i < 100; i++) { + if (ctx.document.getElementById('unsaved-edits-dialog')) return; + await new Promise(resolve => setTimeout(resolve, 0)); + } + assert.fail('the queued unsaved dialog did not open'); +} + async function openTab(ctx, sessionId = 's1') { ctx.window.switchPanel(sessionId); ctx.document.getElementById('touched-toggle-btn').click(); @@ -1348,6 +1356,7 @@ test('Touched edits participate in the existing unsaved-file close prompt', asyn await flush(); ctx.editors.at(-1).setText('unsaved'); const pending = ctx.window.askAboutUnsavedEdits(); + await waitForUnsavedDialog(ctx); const dialog = ctx.document.getElementById('unsaved-save'); assert.ok(dialog, 'the unsaved-file prompt includes the Touched editor'); dialog.click(); @@ -2159,6 +2168,7 @@ test('the quit check lists a stashed Touched file and Save writes it with its ve await dirtyTouched(ctx, '/work/a.txt', 'unsaved'); ctx.window.openDiffTab('s1', 'd1', DIFF); const pending = ctx.window.askAboutUnsavedEdits(); + await waitForUnsavedDialog(ctx); const items = [...ctx.document.querySelectorAll('#unsaved-edits-dialog li')]; assert.deepEqual(items.map(li => [li.textContent, li.title]), [['a.txt', '/work/a.txt']]); ctx.document.getElementById('unsaved-save').click(); @@ -2290,6 +2300,7 @@ test('the quit check keeps a stash whose save is refused, and forgets only the s await dirtyTouched(ctx, '/work/a.txt', 'two'); ctx.window.openDiffTab('s2', 'd2', DIFF); const pending = ctx.window.askAboutUnsavedEdits(); + await waitForUnsavedDialog(ctx); ctx.document.getElementById('unsaved-save').click(); await flush(); assert.equal(ctx.calls.save.length, 2); diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index e81c4c39..c334e378 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -12,6 +12,7 @@ const { JSDOM } = require('jsdom'); const { registerPanelTerminals } = require('./terminal-manager-harness'); const { EventEmitter } = require('node:events'); const { createUnsavedGuard } = require('../unsaved-guard'); +const { loadAppFunctions } = require('./app-source'); const PUBLIC_DIR = path.join(__dirname, '..', 'public'); @@ -89,7 +90,7 @@ function setup() { return el; }; - for (const f of ['viewer-toolbar.js', 'viewer-panel.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'choice-dialog.js', 'file-panel.js', 'touched-files-view.js']) { + for (const f of ['modal-stack.js', 'viewer-toolbar.js', 'viewer-panel.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'choice-dialog.js', 'file-panel.js', 'touched-files-view.js']) { vm.runInContext(fs.readFileSync(path.join(PUBLIC_DIR, f), 'utf8'), dom.getInternalVMContext(), { filename: path.join(PUBLIC_DIR, f) }); } window.openSessions = new Map(); @@ -175,7 +176,8 @@ test('quit focuses Save so Enter saves the edits before approving quit', async ( const save = button(ctx, 'unsaved-save'); assert.equal(ctx.window.document.activeElement, save); dialogKey(ctx, 'Enter'); - save.click(); + await flush(); + assert.equal(ctx.disk.get(A), 'x0\nmine', 'Enter itself must save'); await checking; assert.equal(ctx.disk.get(A), 'x0\nmine'); assert.deepEqual(ctx.calls.answers, [{ id: 60, proceed: true }]); @@ -671,7 +673,7 @@ test('a check delivered twice, before and after the page said it is ready, is an } finally { ctx.destroy(); } }); function loadAppDialogs(ctx) { - for (const f of ['choice-dialog.js', 'app-dialogs.js']) { + for (const f of ['app-dialogs.js']) { const filename = path.join(PUBLIC_DIR, f); vm.runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); } @@ -679,6 +681,74 @@ function loadAppDialogs(ctx) { function dialogKey(ctx, key, init = {}) { ctx.window.document.activeElement.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init })); } +test('a session deletion question waits for the unsaved guard to close', async () => { + const ctx = setup(); + try { + loadAppDialogs(ctx); + ctx.window.cleanDisplayName = value => value; + ctx.window.escapeHtml = value => value; + const { showDeleteSessionDialog } = loadAppFunctions(ctx.context, { + sourcePath: path.join(PUBLIC_DIR, 'sidebar.js'), functions: ['showDeleteSessionDialog'], + }); + await dirtyTab(ctx, 's1', A); + const checking = ctx.calls.check(72, 'quit'); + await flush(); + const deleting = showDeleteSessionDialog({ sessionId: 's1' }); + assert.equal(ctx.window.document.querySelector('.new-session-overlay'), null); + dialogKey(ctx, 'Escape'); + await checking; + assert.equal(dialog(ctx), null); + assert.ok(ctx.window.document.querySelector('#dss-cancel')); + dialogKey(ctx, 'Escape'); + assert.equal(await deleting, false); + } finally { ctx.destroy(); } +}); +test('N2: unsaved guard traps Tab and cancels Escape from the body', async () => { + const ctx = setup(); + try { + loadAppDialogs(ctx); + await dirtyTab(ctx, 's1', A); + const checking = ctx.calls.check(71, 'quit'); + await flush(); + ctx.window.document.activeElement.blur(); + dialogKey(ctx, 'Tab'); + assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-cancel')); + ctx.window.document.activeElement.blur(); + dialogKey(ctx, 'Escape'); + await flush(); + assert.equal(dialog(ctx), null); + await checking; + assert.deepEqual(ctx.calls.answers, [{ id: 71, proceed: false }]); + } finally { ctx.destroy(); } +}); +test('N4: unsaved guard waits for every continuation question, including the final fallback', async () => { + const ctx = setup(); + try { + loadAppDialogs(ctx); + const filename = path.join(PUBLIC_DIR, 'resume-guard.js'); + vm.runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); + await dirtyTab(ctx, 's1', A); + const chain = ctx.window.resolveResumeSession({ sessionId: 'original' }, { + api: { getSessionContinuations: async () => ({ candidates: [{ sessionId: 'first' }, { sessionId: 'second' }] }) }, + confirm: message => ctx.window.appConfirm({ message }), + }); + await flush(); + const checking = ctx.calls.check(70, 'quit'); + for (let i = 0; i < 3; i++) { + await flush(); + assert.equal(dialog(ctx), null, `guard must wait for question ${i + 1}`); + assert.equal(ctx.window.document.querySelectorAll('.choice-dialog-overlay').length, 1); + dialogKey(ctx, 'Escape'); + } + assert.equal(await chain, null); + await flush(); + assert.ok(dialog(ctx)); + assert.equal(ctx.window.document.querySelector('.choice-dialog-overlay'), null); + dialogKey(ctx, 'Escape'); + await checking; + assert.deepEqual(ctx.calls.answers, [{ id: 70, proceed: false }]); + } finally { ctx.destroy(); } +}); test('reload ACK is immediate while confirmations queue; quit upgrade waits for one focused unsaved guard', async () => { const ctx = setup(); try { @@ -715,7 +785,7 @@ test('reload ACK is immediate while confirmations queue; quit upgrade waits for assert.equal(ctx.window.document.activeElement, opener); } finally { ctx.destroy(); } }); -test('a foreground alert cannot cancel or steal focus into an underlying unsaved guard', async () => { +test('an alert waits for the unsaved guard, then owns focus and Escape', async () => { const ctx = setup(); try { loadAppDialogs(ctx); @@ -724,13 +794,15 @@ test('a foreground alert cannot cancel or steal focus into an underlying unsaved await flush(); const alerting = ctx.window.appAlert({ message: 'Foreground' }); dialogKey(ctx, 'Tab'); - assert.ok(ctx.window.document.activeElement.closest('.choice-dialog-overlay')); - dialogKey(ctx, 'Escape'); - await alerting; - assert.ok(dialog(ctx)); + assert.ok(dialog(ctx).contains(ctx.window.document.activeElement)); + assert.equal(ctx.window.document.querySelector('.choice-dialog-overlay'), null); assert.deepEqual(ctx.calls.answers, []); dialogKey(ctx, 'Escape'); await checking; assert.deepEqual(ctx.calls.answers, [{ id: 45, proceed: false }]); + assert.equal(dialog(ctx), null); + assert.ok(ctx.window.document.activeElement.closest('.choice-dialog-overlay')); + dialogKey(ctx, 'Escape'); + await alerting; } finally { ctx.destroy(); } }); diff --git a/test/dom-ide-emulation-indicator.test.js b/test/dom-ide-emulation-indicator.test.js index 6d8c67b6..45b0149c 100644 --- a/test/dom-ide-emulation-indicator.test.js +++ b/test/dom-ide-emulation-indicator.test.js @@ -39,7 +39,7 @@ function setup() { }, }); Object.defineProperty(window, 'activeSessionId', { value: null, writable: true, configurable: true }); - for (const f of ['viewer-toolbar.js', 'viewer-panel.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'file-panel.js']) { + for (const f of ['modal-stack.js', 'viewer-toolbar.js', 'viewer-panel.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'file-panel.js']) { vm.runInContext(fs.readFileSync(path.join(PUBLIC_DIR, f), 'utf8'), dom.getInternalVMContext(), { filename: path.join(PUBLIC_DIR, f) }); } window.openSessions = new Map(); diff --git a/test/dom-modal-routing.test.js b/test/dom-modal-routing.test.js new file mode 100644 index 00000000..cfa1088f --- /dev/null +++ b/test/dom-modal-routing.test.js @@ -0,0 +1,197 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const test = require('node:test'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { setupTerminalDom } = require('./terminal-manager-harness'); +const { loadAppFunctions } = require('./app-source'); +const { setupSidebarDom } = require('./dom-setup'); + +function setup() { + const writes = []; + const ctx = setupTerminalDom({ api: { sendInput: (...args) => writes.push(args), getEffectiveSettings: async () => ({}) } }); + const { window } = ctx; + window.cachedAllProjects = []; + for (const name of ['choice-dialog.js', 'app-dialogs.js', 'setting-defaults.js', 'dialogs.js']) { + const filename = path.join(__dirname, '../public', name); + vm.runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); + } + const doc = window.document; + const container = doc.createElement('div'); + container.className = 'terminal-container visible'; + container.innerHTML = ''; + doc.getElementById('terminals').append(container); + const textarea = container.firstElementChild; + const terminal = { attachCustomKeyEventHandler(handler) { this.handler = handler; } }; + window.setupTerminalKeyBindings(terminal, container, () => 's1'); + textarea.addEventListener('keydown', e => { if (terminal.handler(e)) window.api.sendInput('s1', e.key); }); + const { handleGlobalShortcut } = loadAppFunctions(ctx.context, { functions: ['handleGlobalShortcut'] }); + doc.addEventListener('keydown', handleGlobalShortcut); + const press = (target, key, init = {}) => { + const event = new window.KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init }); + target.dispatchEvent(event); + return event; + }; + return { ...ctx, doc, writes, textarea, terminal, press, handleGlobalShortcut }; +} + +for (const kind of ['Touched info', "What's new"]) { + test(`the topmost ${kind} modal owns Escape over a choice question`, async () => { + const ctx = setupSidebarDom(); + try { + const doc = ctx.document; + const pending = ctx.window.appConfirm({ title: 'Underneath' }); + const origin = doc.createElement('button'); + doc.body.append(origin); + if (kind === 'Touched info') { + const filename = path.join(__dirname, '../public/touched-files-view.js'); + vm.runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); + ctx.window.showTouchedInfo(origin); + } else { + require('../public/whats-new').showWhatsNew(doc, { whatsNewDismissed() {} }, ctx.window.escapeHtml, + { sections: [] }); + } + doc.body.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })); + assert.ok(doc.querySelector('.choice-dialog-overlay'), 'the underlying question must stay open'); + assert.equal(doc.querySelectorAll('.choice-dialog-overlay, .touched-info-overlay, .new-session-overlay, .whats-new-overlay').length, 1); + doc.querySelector('.choice-dialog-cancel').click(); + await pending; + } finally { ctx.destroy(); } + }); +} + +for (const kind of ['session', 'worktree']) { + test(`a ${kind} deletion question waits for the active choice question`, async () => { + const ctx = setupSidebarDom(); + try { + const first = ctx.window.appConfirm({ title: 'First' }); + const deleting = kind === 'session' ? ctx.window.showDeleteSessionDialog({ sessionId: 's1' }) + : ctx.window.showDeleteWorktreeDialog('fixture', '/fixture'); + assert.equal(ctx.document.querySelector('.new-session-overlay'), null); + ctx.document.querySelector('.choice-dialog-confirm').click(); + assert.equal(await first, true); + assert.ok(ctx.document.querySelector('.new-session-overlay')); + ctx.document.body.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })); + assert.equal(await deleting, false); + assert.equal(ctx.window.hasOpenModal(), false); + } finally { ctx.destroy(); } + }); +} + +for (const kind of ['New Session', 'Resume', 'Add Project', 'Remote launch', 'Send prompt', 'Dispatch agent']) { + test(`${kind} traps body Tab, blocks background shortcuts and cancels from outside`, async () => { + const ctx = setup(); + try { + if (kind === 'New Session') await ctx.window.showNewSessionDialog({ projectPath: '/fixture' }); + else if (kind === 'Resume') await ctx.window.showResumeSessionDialog({ sessionId: 's1', projectPath: '/fixture' }); + else if (kind === 'Add Project') ctx.window.showAddProjectDialog(); + else if (kind === 'Remote launch') ctx.window.showRemoteLaunchDialog({ projectPath: '/fixture', remoteAlias: 'box' }); + else if (kind === 'Send prompt') ctx.window.showSendPromptDialog({ sessionId: 's1', remoteAlias: 'box' }); + else await ctx.window.showDispatchAgentDialog({ projectPath: '/fixture' }); + const overlay = ctx.doc.querySelector('.new-session-overlay, .add-project-overlay'); + const tools = []; + ctx.window.handleToolShortcut = e => { tools.push(e.key); return false; }; + ctx.textarea.focus(); + ctx.press(ctx.textarea, 'y'); + ctx.handleGlobalShortcut(new ctx.window.KeyboardEvent('keydown', { key: 'X' })); + assert.equal(ctx.terminal.handler(new ctx.window.KeyboardEvent('keydown', { key: 'y' })), false); + assert.deepEqual(tools, []); + assert.deepEqual(ctx.writes, []); + ctx.textarea.blur(); + ctx.press(ctx.doc.body, 'Tab'); + assert.ok(overlay.contains(ctx.doc.activeElement)); + ctx.press(ctx.doc.body, 'Tab', { shiftKey: true }); + assert.ok(overlay.contains(ctx.doc.activeElement)); + const field = overlay.querySelector('input[type="text"], textarea'); + if (field) { + if (kind === 'Remote launch') field.value = ''; + field.focus(); + assert.equal(ctx.press(field, 'y').defaultPrevented, false); + assert.equal(ctx.press(field, 'Enter').defaultPrevented, kind === 'Add Project' || kind === 'Remote launch'); + } + ctx.press(ctx.textarea, 'Escape'); + assert.equal(overlay.isConnected, false); + assert.equal(ctx.window.hasOpenModal(), false); + ctx.handleGlobalShortcut(new ctx.window.KeyboardEvent('keydown', { key: 'X' })); + assert.deepEqual(tools, ['X']); + } finally { ctx.destroy(); } + }); +} + +test('N1: terminal and document shortcuts and PTY input stop under an open question', async () => { + const ctx = setup(); + try { + const tools = [], navigation = []; + ctx.window.handleToolShortcut = e => { tools.push(e.key); return false; }; + ctx.window.isSessionNavKey = e => e.key === 'ArrowRight'; + ctx.window.handleSessionNavKey = e => navigation.push(e.key); + const pending = ctx.window.appConfirm({ danger: true }); + ctx.textarea.focus(); + for (const key of ['X', 'ArrowRight', 'Enter', 'y']) ctx.press(ctx.textarea, key); + assert.deepEqual(tools, []); + assert.deepEqual(navigation, []); + assert.deepEqual(ctx.writes, []); + for (const key of ['X', 'ArrowRight', 'Enter', 'y']) { + assert.equal(ctx.terminal.handler(new ctx.window.KeyboardEvent('keydown', { key })), false); + ctx.handleGlobalShortcut(new ctx.window.KeyboardEvent('keydown', { key })); + } + assert.deepEqual(tools, []); + assert.deepEqual(navigation, []); + assert.deepEqual(ctx.writes, []); + ctx.doc.querySelector('.choice-dialog-cancel').click(); + assert.equal(await pending, false); + assert.equal(ctx.terminal.handler(new ctx.window.KeyboardEvent('keydown', { key: 'y' })), true); + } finally { ctx.destroy(); } +}); + +test('N1: xterm data cannot bypass the modal key handler', async () => { + const ctx = setup(); + try { + const entry = ctx.window.createTerminalEntry({ sessionId: 's1' }); + const pending = ctx.window.appConfirm({ danger: true }); + entry.terminal._onData('composed text'); + assert.deepEqual(ctx.writes, []); + ctx.doc.querySelector('.choice-dialog-cancel').click(); + await pending; + entry.terminal._onData('allowed'); + assert.deepEqual(ctx.writes, [['s1', 'allowed']]); + } finally { ctx.destroy(); } +}); + +for (const kind of ['New Session', 'Resume', 'Add Project']) { + for (const target of ['body', 'button']) { + test(`N3: choice Enter from ${target} never submits the underlying ${kind}`, async () => { + const ctx = setup(); + try { + const actions = []; + ctx.window.launchNewSession = () => actions.push('launch'); + ctx.window.openSession = () => actions.push('resume'); + ctx.window.api.addProject = async () => { actions.push('add'); return {}; }; + ctx.window.loadProjects = async () => {}; + if (kind === 'New Session') await ctx.window.showNewSessionDialog({ projectPath: '/fixture' }); + else if (kind === 'Resume') await ctx.window.showResumeSessionDialog({ sessionId: 's1', projectPath: '/fixture' }); + else { + ctx.window.showAddProjectDialog(); + ctx.doc.getElementById('add-project-path').value = '/fixture'; + } + const underneath = ctx.doc.querySelector('.new-session-overlay, .add-project-overlay'); + const pending = ctx.window.appConfirm({ title: 'Top', danger: true }); + if (target === 'body') ctx.doc.activeElement.blur(); + else ctx.doc.querySelector('.choice-dialog-confirm').focus(); + const event = ctx.press(ctx.doc.activeElement, 'Enter'); + assert.equal(actions.length, 0); + assert.equal(underneath.isConnected, true); + assert.equal(event.defaultPrevented, true); + if (target === 'body') { + assert.ok(ctx.doc.querySelector('.choice-dialog-overlay')); + ctx.doc.querySelector('.choice-dialog-cancel').click(); + assert.equal(await pending, false); + } else assert.equal(await pending, true); + ctx.press(ctx.doc.body, 'Escape'); + assert.equal(underneath.isConnected, false); + } finally { ctx.destroy(); } + }); + } +} diff --git a/test/dom-new-session-menu.test.js b/test/dom-new-session-menu.test.js index 92dc3f33..5d1b0bbf 100644 --- a/test/dom-new-session-menu.test.js +++ b/test/dom-new-session-menu.test.js @@ -218,7 +218,7 @@ for (const handled of [false, true]) { ctx.document.body.appendChild(outside); const target = handled ? dialog.querySelector('.new-session-start-btn') : outside; target.focus(); - if (handled) target.addEventListener('keydown', e => e.preventDefault(), { once: true }); + if (handled) ctx.window.addEventListener('keydown', e => e.preventDefault(), { once: true, capture: true }); ctx.press('Enter'); assert.equal(launches.length, 0); assert.ok(ctx.document.querySelector('.new-session-overlay')); diff --git a/test/dom-permission-mode-picker.test.js b/test/dom-permission-mode-picker.test.js index 7925a0b4..f88511b4 100644 --- a/test/dom-permission-mode-picker.test.js +++ b/test/dom-permission-mode-picker.test.js @@ -64,6 +64,7 @@ function setupDialogsDom() { Object.defineProperty(window, k, { value: v, writable: true, configurable: true }); } + evalInWindow(dom, path.join(PUBLIC_DIR, 'modal-stack.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'setting-defaults.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'utils.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'dialogs.js')); diff --git a/test/dom-sandbox-toggle.test.js b/test/dom-sandbox-toggle.test.js index afb55d77..f5e3738f 100644 --- a/test/dom-sandbox-toggle.test.js +++ b/test/dom-sandbox-toggle.test.js @@ -56,6 +56,7 @@ function setupDialogsDom(platform) { Object.defineProperty(window, k, { value: v, writable: true, configurable: true }); } + evalInWindow(dom, path.join(PUBLIC_DIR, 'modal-stack.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'setting-defaults.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'utils.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'dialogs.js')); diff --git a/test/dom-send-prompt-dialog.test.js b/test/dom-send-prompt-dialog.test.js index 8ada56d7..28cc3449 100644 --- a/test/dom-send-prompt-dialog.test.js +++ b/test/dom-send-prompt-dialog.test.js @@ -24,7 +24,7 @@ function setup(sendResult) { for (const [k, v] of Object.entries(stubs)) { Object.defineProperty(window, k, { value: v, writable: true, configurable: true }); } - for (const f of ['setting-defaults.js', 'utils.js', 'dialogs.js']) { + for (const f of ['modal-stack.js', 'setting-defaults.js', 'utils.js', 'dialogs.js']) { const file = path.join(PUBLIC_DIR, f); vm.runInContext(fs.readFileSync(file, 'utf8'), dom.getInternalVMContext(), { filename: file }); } @@ -117,7 +117,8 @@ test('Cancel and Escape close the dialog without sending', () => { assert.equal(ctx.document.querySelector('.new-session-overlay'), null); ctx.window.showSendPromptDialog(SESSION); ctx.document.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); - assert.notEqual(ctx.document.querySelector('.new-session-overlay'), null, 'an Escape aimed elsewhere does not close it'); + assert.equal(ctx.document.querySelector('.new-session-overlay'), null, 'Escape outside cancels the topmost modal'); + ctx.window.showSendPromptDialog(SESSION); ctx.document.querySelector('textarea').dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); assert.equal(ctx.document.querySelector('.new-session-overlay'), null); assert.equal(ctx.calls.length, 0); diff --git a/test/dom-setup.js b/test/dom-setup.js index 94a60c0d..c539c923 100644 --- a/test/dom-setup.js +++ b/test/dom-setup.js @@ -147,6 +147,7 @@ function setupSidebarDom() { // renderProjects) and local-transcript-adapter.js (onSessionTranscriptActivity). evalInWindow(dom, path.join(PUBLIC_DIR, 'bridge-url.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'worktree-nesting.js')); + evalInWindow(dom, path.join(PUBLIC_DIR, 'modal-stack.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'choice-dialog.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'app-dialogs.js')); evalInWindow(dom, path.join(PUBLIC_DIR, 'sidebar.js')); diff --git a/test/jsonl-viewer-sanitize.test.js b/test/jsonl-viewer-sanitize.test.js index 6b90980b..0048ee7e 100644 --- a/test/jsonl-viewer-sanitize.test.js +++ b/test/jsonl-viewer-sanitize.test.js @@ -40,6 +40,7 @@ function setupDom({ installMarked = true, installDOMPurify = true } = {}) { }); evalInWindow(dom, path.join(PUBLIC_DIR, 'utils.js')); + evalInWindow(dom, path.join(PUBLIC_DIR, 'modal-stack.js')); if (installMarked) evalInWindow(dom, MARKED_PATH); if (installDOMPurify) evalInWindow(dom, DOMPURIFY_PATH); evalInWindow(dom, path.join(PUBLIC_DIR, 'jsonl-viewer.js')); @@ -47,6 +48,26 @@ function setupDom({ installMarked = true, installDOMPurify = true } = {}) { return dom; } +test('a fullscreen transcript screenshot traps Tab and closes with Escape from outside', () => { + const dom = setupDom(); + try { + const { window } = dom; + const doc = window.document; + const result = doc.createElement('div'); + doc.body.append(result); + window.renderToolResult([{ type: 'image', source: { data: 'AA==', media_type: 'image/png' } }], result); + result.querySelector('img').click(); + const overlay = doc.querySelector('.jsonl-screenshot-fullscreen'); + assert.ok(overlay); + doc.activeElement.blur(); + doc.body.dispatchEvent(new window.KeyboardEvent('keydown', { key: 'Tab', bubbles: true, cancelable: true })); + assert.equal(doc.activeElement, overlay); + doc.body.dispatchEvent(new window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })); + assert.equal(overlay.isConnected, false); + assert.equal(window.hasOpenModal(), false); + } finally { dom.window.close(); } +}); + test('renderJsonlText: javascript: URL in markdown link produces no href', () => { const dom = setupDom(); try { diff --git a/test/modal-stack-order.test.js b/test/modal-stack-order.test.js new file mode 100644 index 00000000..52ca1753 --- /dev/null +++ b/test/modal-stack-order.test.js @@ -0,0 +1,37 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { setupSidebarDom } = require('./dom-setup'); + +test('the last registered modal is visually above the previous question', async () => { + const ctx = setupSidebarDom(); + try { + const style = ctx.document.createElement('style'); + const css = fs.readFileSync(path.join(__dirname, '../public/style.css'), 'utf8'); + style.textContent = ['.modal-overlay,', '.new-session-overlay {'] + .map(selector => { + const start = css.indexOf(selector); + assert.notEqual(start, -1); + return css.slice(start, css.indexOf('}', start) + 1); + }).join('\n'); + ctx.document.head.append(style); + const question = ctx.window.appConfirm({ title: 'Underneath', danger: true }); + const underneath = ctx.document.querySelector('.choice-dialog-overlay'); + const top = ctx.document.createElement('div'); + top.className = 'new-session-overlay'; + top.innerHTML = ''; + ctx.document.body.append(top); + const unregister = ctx.window.registerModal({ overlay: top, cancel: () => { top.remove(); unregister(); } }); + assert.ok(Number(ctx.window.getComputedStyle(top).zIndex) >= Number(ctx.window.getComputedStyle(underneath).zIndex), + 'keyboard ownership and visible stacking must agree'); + top.firstElementChild.focus(); + top.firstElementChild.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })); + assert.equal(top.isConnected, false); + assert.equal(underneath.isConnected, true); + underneath.querySelector('.choice-dialog-cancel').click(); + assert.equal(await question, false); + } finally { ctx.destroy(); } +}); diff --git a/test/new-session-dialog-fit.test.js b/test/new-session-dialog-fit.test.js index 1ebaea66..46bddabb 100644 --- a/test/new-session-dialog-fit.test.js +++ b/test/new-session-dialog-fit.test.js @@ -31,7 +31,7 @@ function setup() { for (const [k, v] of Object.entries(stubs)) { Object.defineProperty(window, k, { value: v, writable: true, configurable: true }); } - for (const f of ['setting-defaults.js', 'utils.js', 'dialogs.js']) { + for (const f of ['modal-stack.js', 'setting-defaults.js', 'utils.js', 'dialogs.js']) { const file = path.join(PUBLIC_DIR, f); vm.runInContext(fs.readFileSync(file, 'utf8'), dom.getInternalVMContext(), { filename: file }); } diff --git a/test/restore-continuations.test.js b/test/restore-continuations.test.js index 12b1c45a..5c18fad9 100644 --- a/test/restore-continuations.test.js +++ b/test/restore-continuations.test.js @@ -65,7 +65,7 @@ test('B6 a plain attach tab is excluded from the saved set', { timeout: 9000 }, test('a continuation opens with Enter; only the live-elsewhere warning focuses Cancel', async t => { const h = setup(t, { old: ['new'], new: [] }, { live: { new: { pid: 4242, cwd: '/elsewhere' } } }); - for (const file of ['choice-dialog.js', 'app-dialogs.js']) { + for (const file of ['modal-stack.js', 'choice-dialog.js', 'app-dialogs.js']) { const filename = path.join(__dirname, '../public', file); vm.runInContext(fs.readFileSync(filename, 'utf8'), h.ctx, { filename }); } diff --git a/test/resume-guard.test.js b/test/resume-guard.test.js index 2e6820ec..6dd35e7a 100644 --- a/test/resume-guard.test.js +++ b/test/resume-guard.test.js @@ -23,6 +23,19 @@ const read = (rel) => fs.readFileSync(path.join(ROOT, rel), 'utf8'); const LIVE = { pid: 4242, cwd: '/work/proj', startedAt: 1790685077444 }; const SESSION = { sessionId: 'sess-1', projectPath: '/work/proj' }; +test('a schedule notice without injection uses the renderer appAlert fallback', async () => { + const messages = []; + const ctx = vm.createContext({ appAlert: async options => messages.push(options) }); + vm.runInContext(fs.readFileSync(GUARD_PATH, 'utf8'), ctx, { filename: GUARD_PATH }); + assert.equal(await ctx.guardResume(SESSION, { live: { ...LIVE, kind: 'schedule' } }), false); + assert.equal(messages.length, 1); + assert.match(messages[0].message, /scheduled task/); +}); + +test('a schedule notice without any alert adapter fails with an explicit contract error', async () => { + await assert.rejects(guardResume(SESSION, { live: { ...LIVE, kind: 'schedule' } }), /guardResume requires an alert adapter/); +}); + function makeApi(answer) { const asked = []; return { diff --git a/test/terminal-manager-harness.js b/test/terminal-manager-harness.js index 9037dd81..37b0f898 100644 --- a/test/terminal-manager-harness.js +++ b/test/terminal-manager-harness.js @@ -74,7 +74,7 @@ function makeTerminalStub(spies) { hasSelection() { return false; } getSelection() { return ''; } attachCustomKeyEventHandler(cb) { this._customKeyHandler = cb; } - onData() {} + onData(cb) { this._onData = cb; } onResize(cb) { this._onResize = cb; } onTitleChange() {} onBell() {} @@ -237,6 +237,7 @@ function setupTerminalDom(opts = {}) { // Same order as index.html: header-controls.js, process-exit.js and file-panel.js first, the panel-shell pair last. if (opts.filePanel) files.unshift('header-controls.js', 'tool-bar.js', 'process-exit.js', 'viewer-toolbar.js', 'file-panel.js', 'touched-files-view.js'); if (opts.filePanel) files.push('splitter.js', 'panel-terminal.js'); + if (!window.registerModal) files.unshift('modal-stack.js'); for (const file of files) { const fullPath = path.join(PUBLIC_DIR, file); // Absolute filename: what c8/V8 attributes the coverage of these files to. diff --git a/test/theme-controls.test.js b/test/theme-controls.test.js index a32c0c13..c2afec48 100644 --- a/test/theme-controls.test.js +++ b/test/theme-controls.test.js @@ -147,7 +147,7 @@ function setupPanel() { window.openSessions = new Map(['terminal', 'transcript', 's1'].map(id => [id, { terminal: { focus() {} } }])); window.isMac = false; window.appShortcuts = {}; - for (const file of ['viewer-toolbar.js', 'viewer-panel.js', 'splitter.js', 'session-state.js', + for (const file of ['modal-stack.js', 'viewer-toolbar.js', 'viewer-panel.js', 'splitter.js', 'session-state.js', 'session-activity-dom.js', 'session-activity.js', 'shortcuts.js', 'header-controls.js', 'tool-bar.js', 'file-panel.js', 'touched-files-view.js']) { evaluate(source(file)); } diff --git a/test/tool-bar.test.js b/test/tool-bar.test.js index c65bfdc8..23d2df9f 100644 --- a/test/tool-bar.test.js +++ b/test/tool-bar.test.js @@ -64,7 +64,10 @@ function event(window, key, extra = {}) { test('an open choice dialog suppresses the global Changes shortcut and resumes it after closing', async () => { const ctx = setup(); try { - for (const file of ['choice-dialog.js', 'app-dialogs.js']) ctx.inCtx(fs.readFileSync(path.join(__dirname, '../public', file), 'utf8')); + for (const file of ['choice-dialog.js', 'app-dialogs.js']) { + const filename = path.join(__dirname, '../public', file); + require('node:vm').runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); + } const { handleGlobalShortcut } = loadAppFunctions(ctx.context, { functions: ['handleGlobalShortcut'] }); const calls = []; ctx.window.toggleChangesTab = id => calls.push(id); diff --git a/test/whats-new.test.js b/test/whats-new.test.js index f284f0ff..d5108b14 100644 --- a/test/whats-new.test.js +++ b/test/whats-new.test.js @@ -18,6 +18,8 @@ function makeDom() { const dom = new JSDOM('', { runScripts: 'outside-only' }); const utils = path.join(ROOT, 'public', 'utils.js'); vm.runInContext(fs.readFileSync(utils, 'utf8'), dom.getInternalVMContext(), { filename: utils }); + const modal = path.join(ROOT, 'public', 'modal-stack.js'); + vm.runInContext(fs.readFileSync(modal, 'utf8'), dom.getInternalVMContext(), { filename: modal }); return { doc: dom.window.document, window: dom.window, escape: dom.window.escapeHtml }; } From 3be07bcbf7f64a7a85ab4a4054e21c78d6ae0180 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 12 Oct 2026 17:07:55 +0200 Subject: [PATCH 04/10] (ui): keep terminal protocol replies and focus right under the modal stack (#585) Dropping all xterm output under a modal also dropped its replies to programs (cursor position, device attributes, colours); typed input is already stopped by the key router, and opening a modal now takes focus off the terminal. Resume and remote launch focus their first control, and the unsaved-edits guard waits only for other questions, opening above any other modal. --- .ai/contexts/app-dialogs.md | 8 +- CHANGELOG.md | 2 +- eslint.config.js | 4 +- public/app-dialogs.js | 4 - public/choice-dialog.js | 4 - public/dialogs.js | 5 +- public/modal-stack.js | 21 ++-- public/terminal-manager.js | 1 - test/dom-file-panel-unsaved-guard.test.js | 31 ++++++ test/dom-modal-routing.test.js | 116 ++++++++++++++++++++-- 10 files changed, 158 insertions(+), 38 deletions(-) diff --git a/.ai/contexts/app-dialogs.md b/.ai/contexts/app-dialogs.md index 79670386..dbcb651f 100644 --- a/.ai/contexts/app-dialogs.md +++ b/.ai/contexts/app-dialogs.md @@ -4,17 +4,17 @@ Registered surfaces are the choice dialog (including `appConfirm` and `appAlert`), New Session, Resume, remote launch, Send a prompt, Add Project, dispatch-agent, the unsaved-edits guard, session/worktree deletion, Touched info, What's new, and fullscreen transcript images. The New session popover is a menu and retains its own arrow-key navigation. -Escape cancels only the top modal from anywhere, including the body and a terminal underneath. Tab and Shift+Tab cycle through that modal's enabled, visible controls; hidden controls and descendants of hidden or inert containers are excluded. A modal without controls focuses its overlay. Enter is stopped before any underlying listener or terminal can act. A focused button activates once, with default activation prevented. Each modal may supply its own Enter action. A default-prevented Enter remains consumed without an action. +Escape cancels only the top modal from anywhere, including the body and a terminal underneath. Tab and Shift+Tab cycle through that modal's enabled, visible controls; hidden controls and descendants of hidden or inert containers are excluded. A modal without controls focuses its overlay. The router stops Enter, Escape and Tab with `stopImmediatePropagation` at document capture: handlers inside the modal and document-bubble handlers do not receive them. Only window-capture listeners see those keys before the router. A focused button activates once, with default activation prevented. Each modal may supply its own Enter action. A default-prevented Enter remains consumed without an action. -Choice questions keep the existing rules: Enter on a checkbox does nothing, and Enter on the container accepts an ordinary question. A dangerous question requires a focused button; container, body, or outside Enter cannot accept it. New Session retains #586: the selected permission mode receives initial focus, Enter on a mode or checkbox starts, Cancel cancels, and Enter in a text input does not start. Its body Enter also starts when it is the top modal. Resume, Add Project and dispatch retain their body Enter actions through the same router's explicit `bodyEnter` option. Enter in Add Project's path submits; in remote launch's path it validates and starts; Send a prompt uses Ctrl/Cmd+Enter. Other text-field keys retain their normal browser behavior inside the top modal. Keys outside the top modal have their defaults and propagation blocked. +Choice questions keep the existing rules: Enter on a checkbox does nothing, and Enter on the container accepts an ordinary question. A dangerous question requires a focused button; container, body, or outside Enter cannot accept it. New Session retains #586: the selected permission mode receives initial focus, Enter on a mode or checkbox starts, Cancel cancels, and Enter in a text input does not start. Its body Enter also starts when it is the top modal. Resume opens with its selected permission mode focused, and Enter on that mode resumes with the selection. Resume, Add Project and dispatch retain their body Enter actions through the same router's explicit `bodyEnter` option. Enter in Add Project's path submits; remote launch initially focuses its path, where Enter validates and starts; Send a prompt uses Ctrl/Cmd+Enter. Other text-field keys retain their normal browser behavior inside the top modal. Keys outside the top modal have propagation blocked, but only Enter, Escape, Tab and matching configurable or fixed app shortcuts have their defaults prevented. Other keys keep their native defaults, including Electron menu accelerators for DevTools, Undo and Select All. -`hasOpenModal()` suppresses `app.js`'s `handleGlobalShortcut`, the xterm custom key handler before tool/session-navigation/search/fullscreen/newline shortcuts, and the xterm `onData` route. This covers single terminals, grid terminals, panel shells, composition/input data, and direct handler calls even when focus drifts underneath a modal. Closing the last modal enables these paths again. Choice questions and the unsaved guard restore their connected opener; New Session keeps its +/terminal fallback restoration. +`hasOpenModal()` suppresses `app.js`'s `handleGlobalShortcut` and the xterm custom key handler before tool/session-navigation/search/fullscreen/newline shortcuts, including direct handler calls when focus drifts underneath a modal. Registration moves focus into the dialog (with `tabIndex=-1`) when the current focus is outside its overlay, taking IME and paste input away from a terminal textarea. Dialogs may then focus their initial control. The xterm `onData` route stays active: terminal protocol replies such as DA, cursor-position reports, DECRQM and OSC colour replies must reach the PTY even for background terminals and panel shells while a modal is open. Closing the last modal enables keyboard paths again. Choice questions and the unsaved guard restore their connected opener; New Session keeps its +/terminal fallback restoration. ## Question queue and the unsaved guard Use async `appConfirm({ title, message, confirmLabel, danger })` and `appAlert({ title, message })` from `public/app-dialogs.js`. They adapt `showChoiceDialog`, which shares `enqueueModalQuestion` with session/worktree deletion and the Save/Discard/Cancel guard. Busy questions stay pending rather than resolving as Cancel. Regular questions use FIFO order; only one queued question is mounted at a time. A question may sit on top of a configuration modal. -The quit/reload guard ACKs immediately, then enqueues an idle question rather than waiting on an idle snapshot and opening separately. Idle questions yield to regular questions and wait for the modal stack to empty. A zero-delay task checks idleness after the current promise reactions, allowing a caller's next continuation question (including `resolveResumeSession`'s candidate loop and fallback) to enter the queue first. Buffers are recollected when the guard actually opens. The guard holds the question queue during saving; an alert requested meanwhile opens after the guard closes. Save failures stay inline. Save receives initial focus, Enter really saves, Escape cancels, and Tab/Shift+Tab are handled by the shared router. Disabled controls cannot act during an in-flight save. The beforeunload veto and main-process protocol are unchanged. `appDialogsIdle()` remains available and resolves after the question queue settles. +The quit/close/reload guard ACKs immediately, then enqueues an idle question rather than waiting on an idle snapshot and opening separately. Idle questions yield to regular questions and wait only for the question queue to settle. Configuration and informational modals do not delay the guard: it registers last and opens on top of them. A zero-delay task checks question idleness after the current promise reactions, allowing a caller's next continuation question (including `resolveResumeSession`'s candidate loop and fallback) to enter the queue first. Buffers are recollected when the guard actually opens. The guard holds the question queue during saving; an alert requested meanwhile opens after the guard closes. Save failures stay inline. Save receives initial focus, Enter really saves, Escape cancels, and Tab/Shift+Tab are handled by the shared router. Disabled controls cannot act during an in-flight save. The beforeunload veto and main-process protocol are unchanged. The unused `appDialogsIdle`, `whenChoiceDialogsIdle` and `whenModalQuestionsIdle` adapters have been removed. Every decision caller awaits its answer before performing its side effect. Changes and Touched discard paths remain async. `returnToPanelList()` returns a boolean for an immediate transition or a promise while a discard decision is pending; the diff's Escape listener consumes only a return that already succeeded. Destructive confirmations and the live-elsewhere warning focus Cancel; continuation prompts focus Open session. The scheduled-task notice is an OK-only alert and refuses resume. `guardResume` defaults its notice adapter to `appAlert` in the renderer; outside the renderer, a missing adapter produces an explicit contract error. diff --git a/CHANGELOG.md b/CHANGELOG.md index 368021eb..aabd86e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased ### Changed -- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. Only the top dialog receives keys, with terminal input and app shortcuts paused while it is open. The unsaved-edits guard waits for pending questions and keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) +- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. Only the top dialog receives keys, with terminal input and app shortcuts paused while it is open. The unsaved-edits guard waits for pending questions, opens over other dialogs, and keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) - **Archive Project** now keeps your settings and suspends schedules until you restore it from Settings → **Archived projects**. A separate **Delete Project** confirms the removal of settings, cache and schedule registration, while keeping your folders and files. (#568) - Markdown previews now show a file's frontmatter as a readable header above its content, including its name, description and metadata. (#593) - Closing the window (the close button, `Alt`+`F4`) asks for confirmation first, with Cancel selected; a window that is busy at that moment asks once it is free. Quitting from the ☰ menu (Quit, or Exit on Windows) does not ask again, and a quit from outside (a logout, an update install) is not stopped by the question. On macOS it asks "Close the window?": Switchboard stays in the Dock, but its sessions stop. If the window has stopped responding, a single close waits, and closing it again a few seconds later ends it; never while one of its own confirmation boxes waits for an answer. A window closed while it is still starting asks once it has loaded. (#478) diff --git a/eslint.config.js b/eslint.config.js index 2832c7d7..d0706b05 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -27,7 +27,6 @@ const rendererCrossFileGlobals = { registerModal: 'readonly', hasOpenModal: 'readonly', enqueueModalQuestion: 'readonly', - whenModalQuestionsIdle: 'readonly', TOOL_BAR: 'readonly', filePanelState: 'readonly', currentPanelSessionId: 'readonly', @@ -130,8 +129,6 @@ const rendererCrossFileGlobals = { showChoiceDialog: 'readonly', appConfirm: 'readonly', appAlert: 'readonly', - appDialogsIdle: 'readonly', - whenChoiceDialogsIdle: 'readonly', SIDEBAR_WORKTREE_RE: 'readonly', worktreeParentPath: 'readonly', worktreeName: 'readonly', @@ -361,6 +358,7 @@ const rendererCrossFileGlobals = { // Configurable keyboard shortcuts (public/shortcuts.js + grid-view.js) DEFAULT_SHORTCUTS: 'readonly', SHORTCUT_DEFS: 'readonly', + FIXED_SHORTCUTS: 'readonly', normalizeShortcuts: 'readonly', keyFamily: 'readonly', matchShortcut: 'readonly', diff --git a/public/app-dialogs.js b/public/app-dialogs.js index cf15dc65..a69efefe 100644 --- a/public/app-dialogs.js +++ b/public/app-dialogs.js @@ -9,10 +9,6 @@ async function appAlert({ title = 'Switchboard', message } = {}) { await showChoiceDialog({ title, message: appDialogMessage(message), confirmLabel: 'OK', cancelLabel: null }); } -function appDialogsIdle() { - return whenChoiceDialogsIdle(); -} - function appDialogMessage(message) { return typeof message === 'string' ? message.split('\n') : message; } diff --git a/public/choice-dialog.js b/public/choice-dialog.js index 6ddcd1fa..7e8eea23 100644 --- a/public/choice-dialog.js +++ b/public/choice-dialog.js @@ -23,10 +23,6 @@ function showChoiceDialog(options = {}) { }); } -function whenChoiceDialogsIdle() { - return whenModalQuestionsIdle(); -} - function openChoiceDialog({ title, message, diff --git a/public/dialogs.js b/public/dialogs.js index 9dab29dc..fc95c118 100644 --- a/public/dialogs.js +++ b/public/dialogs.js @@ -487,6 +487,7 @@ function showRemoteLaunchDialog(project) { start(); return true; } }); + dialog.querySelector('#rld-path').focus(); } async function showResumeSessionDialog(session) { @@ -614,10 +615,12 @@ async function showResumeSessionDialog(session) { overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); const unregister = registerModal({ overlay, cancel: close, bodyEnter: true, enter: e => { - if (e.target.matches('input, button')) return false; + if (e.target.matches('input, button:not(.permission-option)')) return false; resume(); return true; } }); + (modeGrid.querySelector(dangerousSkip ? '[data-mode="dangerous-skip"]' : '.permission-option.selected') || + modeGrid.querySelector('button')).focus(); } // see .ai/contexts/session-cache.md ("Remote hosts — sending a prompt") diff --git a/public/modal-stack.js b/public/modal-stack.js index d6e3ad4b..ee7371d9 100644 --- a/public/modal-stack.js +++ b/public/modal-stack.js @@ -2,7 +2,6 @@ const modalStack = []; const modalQuestions = []; const modalIdleQuestions = []; -const modalQuestionIdleWaiters = []; let modalQuestionActive = false; let modalIdleTimer = null; @@ -17,6 +16,11 @@ function registerModal({ overlay, cancel, enter, bodyEnter = false }) { Number(top?.overlay.style.zIndex) || 0) + 1); const modal = { overlay, cancel, enter, bodyEnter }; modalStack.push(modal); + if (!overlay.contains(document.activeElement)) { + const dialog = overlay.querySelector('[role="dialog"], .modal-dialog, .new-session-dialog') || overlay; + dialog.tabIndex = -1; + dialog.focus(); + } return () => { const index = modalStack.indexOf(modal); if (index !== -1) { @@ -66,7 +70,9 @@ function routeModalKey(event) { event.target.click(); } } else if (!inside) { - event.preventDefault(); + if (typeof matchShortcut === 'function' && typeof SHORTCUT_DEFS !== 'undefined' && + [...SHORTCUT_DEFS.map(def => def.id), ...Object.keys(FIXED_SHORTCUTS)] + .some(id => matchShortcut(id, event, isMac, appShortcuts))) event.preventDefault(); event.stopImmediatePropagation(); } } @@ -104,16 +110,7 @@ function drainModalQuestions() { modalIdleTimer = null; if (modalQuestionActive || modalQuestions.length) { drainModalQuestions(); return; } if (modalIdleQuestions.length) { - if (!hasOpenModal()) openModalQuestion(modalIdleQuestions.shift()); - return; + openModalQuestion(modalIdleQuestions.shift()); } - for (const resolve of modalQuestionIdleWaiters.splice(0)) resolve(); }, 0); } - -function whenModalQuestionsIdle() { - return new Promise(resolve => { - modalQuestionIdleWaiters.push(resolve); - drainModalQuestions(); - }); -} diff --git a/public/terminal-manager.js b/public/terminal-manager.js index 7b321d31..9162b4bf 100644 --- a/public/terminal-manager.js +++ b/public/terminal-manager.js @@ -1185,7 +1185,6 @@ function createTerminalEntry(session, opts = {}) { // Wire up IPC (use entry.session.sessionId so fork re-keying works) terminal.onData(data => { - if (hasOpenModal()) return; if (data === '\x1b[I' || data === '\x1b[O') return; window.api.sendInput(entry.session.sessionId, data); }); diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index c334e378..8a5e9844 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -681,6 +681,37 @@ function loadAppDialogs(ctx) { function dialogKey(ctx, key, init = {}) { ctx.window.document.activeElement.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init })); } + +for (const reason of ['quit', 'close', 'reload']) { + test(`${reason} opens the unsaved guard on top of New Session without closing it first`, async () => { + const ctx = setup(); + try { + loadAppDialogs(ctx); + for (const f of ['setting-defaults.js', 'dialogs.js']) { + const filename = path.join(PUBLIC_DIR, f); + vm.runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); + } + await dirtyTab(ctx, 's1', A); + ctx.window.api.getEffectiveSettings = async () => ({}); + await ctx.window.showNewSessionDialog({ projectPath: '/repo' }); + const underneath = ctx.window.document.querySelector('.new-session-overlay'); + const checking = ctx.calls.check(73, reason); + assert.deepEqual(ctx.calls.acks.map(x => x.id), [73]); + await flush(); + assert.ok(dialog(ctx), 'a configuration modal must not delay the unsaved guard'); + assert.equal(underneath.isConnected, true); + assert.ok(Number(dialog(ctx).parentElement.style.zIndex) > Number(underneath.style.zIndex)); + assert.equal(ctx.window.document.activeElement, button(ctx, 'unsaved-save')); + dialogKey(ctx, 'Escape'); + await checking; + assert.deepEqual(ctx.calls.answers, [{ id: 73, proceed: false }]); + assert.equal(underneath.isConnected, true); + dialogKey(ctx, 'Escape'); + assert.equal(underneath.isConnected, false); + } finally { ctx.destroy(); } + }); +} + test('a session deletion question waits for the unsaved guard to close', async () => { const ctx = setup(); try { diff --git a/test/dom-modal-routing.test.js b/test/dom-modal-routing.test.js index cfa1088f..c8a6c5b4 100644 --- a/test/dom-modal-routing.test.js +++ b/test/dom-modal-routing.test.js @@ -9,9 +9,9 @@ const { setupTerminalDom } = require('./terminal-manager-harness'); const { loadAppFunctions } = require('./app-source'); const { setupSidebarDom } = require('./dom-setup'); -function setup() { +function setup(api = {}) { const writes = []; - const ctx = setupTerminalDom({ api: { sendInput: (...args) => writes.push(args), getEffectiveSettings: async () => ({}) } }); + const ctx = setupTerminalDom({ api: { sendInput: (...args) => writes.push(args), getEffectiveSettings: async () => ({}), ...api } }); const { window } = ctx; window.cachedAllProjects = []; for (const name of ['choice-dialog.js', 'app-dialogs.js', 'setting-defaults.js', 'dialogs.js']) { @@ -146,17 +146,117 @@ test('N1: terminal and document shortcuts and PTY input stop under an open quest } finally { ctx.destroy(); } }); -test('N1: xterm data cannot bypass the modal key handler', async () => { +test('N1: xterm protocol replies reach every PTY while a modal is open', async () => { const ctx = setup(); + const entries = []; try { - const entry = ctx.window.createTerminalEntry({ sessionId: 's1' }); + vm.runInContext(fs.readFileSync(require.resolve('@xterm/xterm'), 'utf8'), ctx.context); + const Xterm = ctx.window.Terminal; + ctx.window.Terminal = class extends Xterm { + loadAddon() {} + get unicode() { return { activeVersion: '' }; } + open() {} + }; + for (const [sessionId, options] of [['s1', {}], ['background', {}], ['shell', { panel: true }]]) { + entries.push(ctx.window.createTerminalEntry({ sessionId }, options)); + } const pending = ctx.window.appConfirm({ danger: true }); - entry.terminal._onData('composed text'); - assert.deepEqual(ctx.writes, []); + for (const entry of entries) { + await new Promise(resolve => entry.terminal.write('\x1b[6n\x1b[c\x1b[?1$p', resolve)); + } + assert.deepEqual(ctx.writes, ['s1', 'background', 'shell'].flatMap(id => [ + [id, '\x1b[1;1R'], [id, '\x1b[?1;2c'], [id, '\x1b[?1;2$y'], + ])); ctx.doc.querySelector('.choice-dialog-cancel').click(); await pending; - entry.terminal._onData('allowed'); - assert.deepEqual(ctx.writes, [['s1', 'allowed']]); + await new Promise(resolve => entries[0].terminal.write('\x1b[6n', resolve)); + assert.deepEqual(ctx.writes.at(-1), ['s1', '\x1b[1;1R']); + } finally { + for (const entry of entries) entry.terminal.dispose(); + ctx.destroy(); + } +}); + +test('registering a modal removes focus from the terminal textarea', () => { + const ctx = setup(); + try { + ctx.textarea.focus(); + const overlay = ctx.doc.createElement('div'); + overlay.innerHTML = '
Dialog
'; + ctx.doc.body.append(overlay); + const unregister = ctx.window.registerModal({ overlay, cancel() {} }); + assert.equal(ctx.doc.activeElement, overlay.firstElementChild); + assert.equal(ctx.doc.activeElement.tabIndex, -1); + unregister(); + overlay.remove(); + } finally { ctx.destroy(); } +}); + +for (const mode of [null, 'plan', 'dangerous-skip']) { + test(`Resume focuses its selected ${mode || 'default'} mode and Enter resumes from the opener`, async () => { + const ctx = setup({ getEffectiveSettings: async () => mode === 'dangerous-skip' + ? { dangerouslySkipPermissions: true } : { permissionMode: mode } }); + try { + const actions = []; + ctx.window.openSession = (session, options) => actions.push({ session, options }); + const session = { sessionId: 's1', projectPath: '/fixture' }; + ctx.textarea.focus(); + await ctx.window.showResumeSessionDialog(session); + assert.equal(ctx.doc.activeElement.dataset.mode, String(mode)); + assert.equal(ctx.press(ctx.doc.activeElement, 'Enter').defaultPrevented, true); + assert.equal(actions.length, 1); + assert.equal(actions[0].session, session); + assert.equal(actions[0].options.permissionMode, mode === 'plan' ? 'plan' : undefined); + assert.equal(actions[0].options.dangerouslySkipPermissions, mode === 'dangerous-skip' ? true : undefined); + assert.equal(ctx.window.hasOpenModal(), false); + assert.deepEqual(ctx.writes, []); + } finally { ctx.destroy(); } + }); +} + +test('remote launch focuses its path and Enter launches from the opener', () => { + const ctx = setup(); + try { + const actions = []; + ctx.window.launchRemoteSession = (project, request) => actions.push({ project, request }); + const project = { projectPath: '/fixture', remoteAlias: 'box' }; + ctx.textarea.focus(); + ctx.window.showRemoteLaunchDialog(project); + assert.equal(ctx.doc.activeElement, ctx.doc.getElementById('rld-path')); + assert.equal(ctx.press(ctx.doc.activeElement, 'Enter').defaultPrevented, true); + assert.equal(actions.length, 1); + assert.equal(actions[0].project, project); + assert.equal(actions[0].request.cwd, '/fixture'); + assert.equal(ctx.window.hasOpenModal(), false); + assert.deepEqual(ctx.writes, []); + } finally { ctx.destroy(); } +}); + +test('outside keys keep native defaults while app shortcuts stay suppressed under a modal', async () => { + const ctx = setup(); + try { + const pending = ctx.window.appConfirm({ danger: true }); + const native = [['I', { ctrlKey: true, shiftKey: true }], ['z', { ctrlKey: true }], + ['a', { ctrlKey: true }], ['a', { metaKey: true }], ['F12', {}], ['F11', {}], ['y', {}]]; + for (const [key, init] of native) { + assert.equal(ctx.press(ctx.textarea, key, init).defaultPrevented, false, key); + } + assert.equal(ctx.press(ctx.textarea, 'g', { ctrlKey: true, shiftKey: true }).defaultPrevented, true); + ctx.window.setAppShortcuts({ gridToggle: { primary: true, alt: false, shift: true, key: 'x' } }); + assert.equal(ctx.press(ctx.textarea, 'x', { ctrlKey: true, shiftKey: true }).defaultPrevented, true); + assert.equal(ctx.press(ctx.textarea, 'F6').defaultPrevented, true); + assert.deepEqual(ctx.writes, []); + ctx.press(ctx.doc.body, 'Escape'); + assert.equal(await pending, false); + } finally { ctx.destroy(); } +}); + +test('unused question-idle adapters are absent from the renderer API', () => { + const ctx = setup(); + try { + for (const name of ['appDialogsIdle', 'whenChoiceDialogsIdle', 'whenModalQuestionsIdle']) { + assert.equal(typeof ctx.window[name], 'undefined', name); + } } finally { ctx.destroy(); } }); From ec3bf9dc31c0d83b1d252c47206783a8ec8ced4a Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 12 Oct 2026 17:56:52 +0200 Subject: [PATCH 05/10] (ui): trap focus in the topmost modal and give it back on close (#585) Letting xterm replies through also let typed text and paste reach a terminal that regained focus under a modal (after F11, or a restored session opening behind What's new). Focus landing outside the topmost modal now goes back into it, paste and input on a terminal textarea are cancelled while a modal is open, and the fullscreen and showSession paths no longer focus the terminal under a modal. Closing a modal gives focus back to what had it, or to the active terminal. --- .ai/contexts/app-dialogs.md | 4 +- CHANGELOG.md | 2 +- public/app.js | 2 +- public/choice-dialog.js | 2 +- public/dialogs.js | 6 +- public/file-panel.js | 4 +- public/jsonl-viewer.js | 2 +- public/modal-stack.js | 54 +++-- public/terminal-manager.js | 2 +- public/touched-files-view.js | 2 +- test/dom-file-panel-unsaved-guard.test.js | 19 ++ test/dom-modal-routing.test.js | 241 ++++++++++++++++++++++ test/dom-new-session-menu.test.js | 3 +- test/jsonl-viewer-sanitize.test.js | 19 ++ 14 files changed, 336 insertions(+), 26 deletions(-) diff --git a/.ai/contexts/app-dialogs.md b/.ai/contexts/app-dialogs.md index dbcb651f..0422d75a 100644 --- a/.ai/contexts/app-dialogs.md +++ b/.ai/contexts/app-dialogs.md @@ -8,13 +8,13 @@ Escape cancels only the top modal from anywhere, including the body and a termin Choice questions keep the existing rules: Enter on a checkbox does nothing, and Enter on the container accepts an ordinary question. A dangerous question requires a focused button; container, body, or outside Enter cannot accept it. New Session retains #586: the selected permission mode receives initial focus, Enter on a mode or checkbox starts, Cancel cancels, and Enter in a text input does not start. Its body Enter also starts when it is the top modal. Resume opens with its selected permission mode focused, and Enter on that mode resumes with the selection. Resume, Add Project and dispatch retain their body Enter actions through the same router's explicit `bodyEnter` option. Enter in Add Project's path submits; remote launch initially focuses its path, where Enter validates and starts; Send a prompt uses Ctrl/Cmd+Enter. Other text-field keys retain their normal browser behavior inside the top modal. Keys outside the top modal have propagation blocked, but only Enter, Escape, Tab and matching configurable or fixed app shortcuts have their defaults prevented. Other keys keep their native defaults, including Electron menu accelerators for DevTools, Undo and Select All. -`hasOpenModal()` suppresses `app.js`'s `handleGlobalShortcut` and the xterm custom key handler before tool/session-navigation/search/fullscreen/newline shortcuts, including direct handler calls when focus drifts underneath a modal. Registration moves focus into the dialog (with `tabIndex=-1`) when the current focus is outside its overlay, taking IME and paste input away from a terminal textarea. Dialogs may then focus their initial control. The xterm `onData` route stays active: terminal protocol replies such as DA, cursor-position reports, DECRQM and OSC colour replies must reach the PTY even for background terminals and panel shells while a modal is open. Closing the last modal enables keyboard paths again. Choice questions and the unsaved guard restore their connected opener; New Session keeps its +/terminal fallback restoration. +`hasOpenModal()` suppresses `app.js`'s `handleGlobalShortcut` and the xterm custom key handler before tool/session-navigation/search/fullscreen/newline shortcuts, including direct handler calls when focus drifts underneath a modal. Registration records the current opener and moves focus into the dialog (with `tabIndex=-1`) when it is outside the overlay. Dialogs register before focusing their initial control. A document-capture `focusin` listener returns outside focus to the top modal’s last focused connected control, or its dialog when that control is gone. Fullscreen transitions and single-view `showSession` skip terminal focus while a modal is open. Document-capture listeners cancel terminal-textarea `paste`, `beforeinput` and `input` events, including non-cancelable input propagation, while leaving modal fields editable. The xterm `onData` route stays active: terminal protocol replies such as DA, cursor-position reports, DECRQM and OSC colour replies must reach the PTY even for background terminals and panel shells while a modal is open. Unregistering returns focus to the remaining top modal, or restores the last modal’s connected opener. A missing opener, body opener or disabled opener falls back to the active session’s terminal. Unregister is idempotent, so restoration happens once. Dialogs with their own explicit restoration register with `restoreFocus: false`: choice questions, the unsaved guard, New Session, Touched info and fullscreen screenshots retain their existing opener and fallback behavior without a second restoration from the stack. Closing the last modal enables keyboard paths again. ## Question queue and the unsaved guard Use async `appConfirm({ title, message, confirmLabel, danger })` and `appAlert({ title, message })` from `public/app-dialogs.js`. They adapt `showChoiceDialog`, which shares `enqueueModalQuestion` with session/worktree deletion and the Save/Discard/Cancel guard. Busy questions stay pending rather than resolving as Cancel. Regular questions use FIFO order; only one queued question is mounted at a time. A question may sit on top of a configuration modal. -The quit/close/reload guard ACKs immediately, then enqueues an idle question rather than waiting on an idle snapshot and opening separately. Idle questions yield to regular questions and wait only for the question queue to settle. Configuration and informational modals do not delay the guard: it registers last and opens on top of them. A zero-delay task checks question idleness after the current promise reactions, allowing a caller's next continuation question (including `resolveResumeSession`'s candidate loop and fallback) to enter the queue first. Buffers are recollected when the guard actually opens. The guard holds the question queue during saving; an alert requested meanwhile opens after the guard closes. Save failures stay inline. Save receives initial focus, Enter really saves, Escape cancels, and Tab/Shift+Tab are handled by the shared router. Disabled controls cannot act during an in-flight save. The beforeunload veto and main-process protocol are unchanged. The unused `appDialogsIdle`, `whenChoiceDialogsIdle` and `whenModalQuestionsIdle` adapters have been removed. +The quit/close/reload guard ACKs immediately, then enqueues an idle question rather than waiting on an idle snapshot and opening separately. Idle questions yield to regular questions and wait only for the question queue to settle. Configuration and informational modals do not delay the guard: it registers last and opens on top of them. A zero-delay task checks question idleness after the current promise reactions, allowing a caller's next continuation question (including `resolveResumeSession`'s candidate loop and fallback) to enter the queue first. Buffers are recollected when the guard actually opens. The guard holds the question queue during saving; an alert requested meanwhile opens after the guard closes. Save failures stay inline. Save receives initial focus, Enter really saves, Escape cancels, and Tab/Shift+Tab are handled by the shared router. Disabled controls cannot act during an in-flight save. The beforeunload veto and main-process protocol are unchanged. Every decision caller awaits its answer before performing its side effect. Changes and Touched discard paths remain async. `returnToPanelList()` returns a boolean for an immediate transition or a promise while a discard decision is pending; the diff's Escape listener consumes only a return that already succeeded. Destructive confirmations and the live-elsewhere warning focus Cancel; continuation prompts focus Open session. The scheduled-task notice is an OK-only alert and refuses resume. `guardResume` defaults its notice adapter to `appAlert` in the renderer; outside the renderer, a missing adapter produces an explicit contract error. diff --git a/CHANGELOG.md b/CHANGELOG.md index aabd86e7..3e1154c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased ### Changed -- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. Only the top dialog receives keys, with terminal input and app shortcuts paused while it is open. The unsaved-edits guard waits for pending questions, opens over other dialogs, and keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) +- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. Only the top dialog receives keys and keeps focus, with terminal typing, paste and app shortcuts paused while it is open; closing it returns focus to its opener or the active terminal. The unsaved-edits guard waits for pending questions, opens over other dialogs, and keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) - **Archive Project** now keeps your settings and suspends schedules until you restore it from Settings → **Archived projects**. A separate **Delete Project** confirms the removal of settings, cache and schedule registration, while keeping your folders and files. (#568) - Markdown previews now show a file's frontmatter as a readable header above its content, including its name, description and metadata. (#593) - Closing the window (the close button, `Alt`+`F4`) asks for confirmation first, with Cancel selected; a window that is busy at that moment asks once it is free. Quitting from the ☰ menu (Quit, or Exit on Windows) does not ask again, and a quit from outside (a logout, an update install) is not stopped by the question. On macOS it asks "Close the window?": Switchboard stays in the Dock, but its sessions stop. If the window has stopped responding, a single close waits, and closing it again a few seconds later ends it; never while one of its own confirmation boxes waits for an answer. A window closed while it is still starting asks once it has loaded. (#478) diff --git a/public/app.js b/public/app.js index 2601b96c..1e1f096c 100644 --- a/public/app.js +++ b/public/app.js @@ -1953,7 +1953,7 @@ document.getElementById('indexing-banner-dismiss').addEventListener('click', dis window.api.onFullScreenChanged(() => { requestAnimationFrame(() => { const entry = activeSessionId && openSessions.get(activeSessionId); - if (entry && !entry.closed) entry.terminal.focus(); + if (entry && !entry.closed && !hasOpenModal()) entry.terminal.focus(); }); }); diff --git a/public/choice-dialog.js b/public/choice-dialog.js index 7e8eea23..f92cbdac 100644 --- a/public/choice-dialog.js +++ b/public/choice-dialog.js @@ -130,7 +130,7 @@ function openChoiceDialog({ overlay.addEventListener('click', (event) => { if (event.target === overlay) close(null); }); document.body.appendChild(overlay); - const unregister = registerModal({ overlay, cancel: () => close(null), enter }); + const unregister = registerModal({ overlay, cancel: () => close(null), enter, restoreFocus: false }); closeOpenChoiceDialog = close; if (closeWith) closeWith.then(close, () => {}); (initialFocus === 'cancel' ? cancelBtn : confirmBtn).focus(); diff --git a/public/dialogs.js b/public/dialogs.js index fc95c118..b031cb1b 100644 --- a/public/dialogs.js +++ b/public/dialogs.js @@ -377,7 +377,7 @@ async function showNewSessionDialog(project, opener = document.activeElement) { dialog.querySelector('.new-session-start-btn').onclick = start; overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); - const unregister = registerModal({ overlay, cancel: () => close(), bodyEnter: true, enter: e => { + const unregister = registerModal({ overlay, cancel: () => close(), bodyEnter: true, restoreFocus: false, enter: e => { if (!e.target.matches('input:not([type="checkbox"])')) { if (e.target.closest('.new-session-cancel-btn')) close(); else start(); @@ -653,7 +653,6 @@ function showSendPromptDialog(session) { overlay.appendChild(dialog); document.body.appendChild(overlay); overlay.tabIndex = -1; - textarea.focus(); dialog.addEventListener('click', (e) => { if (e.target !== textarea) textarea.focus(); }); function close() { @@ -690,6 +689,7 @@ function showSendPromptDialog(session) { send(); return true; } }); + textarea.focus(); } // Settings viewer is in settings-panel.js (openSettingsViewer / closeSettingsViewer) @@ -721,7 +721,6 @@ function showAddProjectDialog() { const pathInput = dialog.querySelector('#add-project-path'); const errorEl = dialog.querySelector('#add-project-error'); - pathInput.focus(); function close() { overlay.remove(); @@ -761,6 +760,7 @@ function showAddProjectDialog() { addProject(); return true; } }); + pathInput.focus(); } // see .ai/contexts/bg-agents.md diff --git a/public/file-panel.js b/public/file-panel.js index 8f0798e3..6392c4b2 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -709,7 +709,6 @@ function showUnsavedEditsDialog(tabs) { dialog.appendChild(actions); overlay.appendChild(dialog); document.body.appendChild(overlay); - saveBtn.focus(); function finish(proceed) { overlay.remove(); @@ -717,9 +716,10 @@ function showUnsavedEditsDialog(tabs) { if (returnFocus?.isConnected) returnFocus.focus(); resolve(proceed); } - const unregister = registerModal({ overlay, cancel: () => { + const unregister = registerModal({ overlay, restoreFocus: false, cancel: () => { if (!cancelBtn.disabled) finish(false); } }); + saveBtn.focus(); cancelBtn.onclick = () => finish(false); discardBtn.onclick = () => finish(true); diff --git a/public/jsonl-viewer.js b/public/jsonl-viewer.js index 6b1f7607..d9c59c67 100644 --- a/public/jsonl-viewer.js +++ b/public/jsonl-viewer.js @@ -538,7 +538,7 @@ function showJsonlScreenshot(src) { } overlay.onclick = close; document.body.appendChild(overlay); - const unregister = registerModal({ overlay, cancel: close }); + const unregister = registerModal({ overlay, cancel: close, restoreFocus: false }); overlay.focus(); } diff --git a/public/modal-stack.js b/public/modal-stack.js index ee7371d9..c72c3c7f 100644 --- a/public/modal-stack.js +++ b/public/modal-stack.js @@ -9,23 +9,38 @@ function hasOpenModal() { return modalStack.length > 0; } -function registerModal({ overlay, cancel, enter, bodyEnter = false }) { +function focusModal(modal) { + const focused = modal.lastFocused; + const target = focused?.isConnected && modal.overlay.contains(focused) && !focused.matches(':disabled') + ? focused : modal.overlay.querySelector('[role="dialog"], .modal-dialog, .new-session-dialog') || modal.overlay; + if (target !== focused) target.tabIndex = -1; + target.focus(); +} + +function registerModal({ overlay, cancel, enter, bodyEnter = false, restoreFocus = true }) { + const opener = document.activeElement; const previousZIndex = overlay.style.zIndex; const top = modalStack[modalStack.length - 1]; overlay.style.zIndex = String(Math.max(Number(getComputedStyle(overlay).zIndex) || 0, Number(top?.overlay.style.zIndex) || 0) + 1); - const modal = { overlay, cancel, enter, bodyEnter }; + const modal = { overlay, cancel, enter, bodyEnter, lastFocused: overlay.contains(opener) ? opener : null }; modalStack.push(modal); - if (!overlay.contains(document.activeElement)) { - const dialog = overlay.querySelector('[role="dialog"], .modal-dialog, .new-session-dialog') || overlay; - dialog.tabIndex = -1; - dialog.focus(); - } + if (!overlay.contains(opener)) focusModal(modal); return () => { const index = modalStack.indexOf(modal); - if (index !== -1) { - modalStack.splice(index, 1); - overlay.style.zIndex = previousZIndex; + if (index === -1) return; + modalStack.splice(index, 1); + overlay.style.zIndex = previousZIndex; + const remaining = modalStack[modalStack.length - 1]; + if (remaining && restoreFocus) { + if (!remaining.overlay.contains(document.activeElement)) focusModal(remaining); + } else if (!remaining && restoreFocus) { + if (opener?.isConnected && opener !== document.body && opener !== document.documentElement && !opener.matches(':disabled')) { + opener.focus(); + } else if (typeof activeSessionId !== 'undefined' && typeof openSessions !== 'undefined') { + const entry = activeSessionId && openSessions.get(activeSessionId); + if (entry && !entry.closed) entry.terminal.focus(); + } } drainModalQuestions(); }; @@ -71,13 +86,28 @@ function routeModalKey(event) { } } else if (!inside) { if (typeof matchShortcut === 'function' && typeof SHORTCUT_DEFS !== 'undefined' && - [...SHORTCUT_DEFS.map(def => def.id), ...Object.keys(FIXED_SHORTCUTS)] - .some(id => matchShortcut(id, event, isMac, appShortcuts))) event.preventDefault(); + [...SHORTCUT_DEFS.map(def => def.id), ...Object.keys(typeof FIXED_SHORTCUTS === 'undefined' ? {} : FIXED_SHORTCUTS)] + .some(id => matchShortcut(id, event, typeof isMac === 'undefined' ? false : isMac, + typeof appShortcuts === 'undefined' ? {} : appShortcuts))) event.preventDefault(); event.stopImmediatePropagation(); } } document.addEventListener('keydown', routeModalKey, true); +document.addEventListener('focusin', event => { + const modal = modalStack[modalStack.length - 1]; + if (!modal) return; + if (modal.overlay.contains(event.target)) modal.lastFocused = event.target; + else focusModal(modal); +}, true); + +for (const type of ['paste', 'beforeinput', 'input']) { + document.addEventListener(type, event => { + if (!hasOpenModal() || !event.target.matches?.('.xterm-helper-textarea')) return; + event.preventDefault(); + event.stopImmediatePropagation(); + }, true); +} function enqueueModalQuestion(open, { whenIdle = false } = {}) { return new Promise((resolve, reject) => { diff --git a/public/terminal-manager.js b/public/terminal-manager.js index 9162b4bf..ae013353 100644 --- a/public/terminal-manager.js +++ b/public/terminal-manager.js @@ -1474,7 +1474,7 @@ function showSession(sessionId) { if (tr) revealLap(tr, 'restoreMs'); entry.element.classList.add('visible'); if (tr) revealLap(tr, 'visibleMs'); - entry.terminal.focus(); + if (!hasOpenModal()) entry.terminal.focus(); fitAndScroll(entry, tr, entry.webglWanted); if (tr) revealLap(tr, 'focusMs'); } diff --git a/public/touched-files-view.js b/public/touched-files-view.js index 8155a114..c78f50d5 100644 --- a/public/touched-files-view.js +++ b/public/touched-files-view.js @@ -145,7 +145,7 @@ function showTouchedInfo(button) { closeButton.addEventListener('click', close); overlay.addEventListener('click', event => { if (event.target === overlay) close(); }); document.body.appendChild(overlay); - const unregister = registerModal({ overlay, cancel: close }); + const unregister = registerModal({ overlay, cancel: close, restoreFocus: false }); closeButton.focus(); } diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index 8a5e9844..1aa90fdf 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -150,6 +150,25 @@ const A = '/repo/a.md'; const B = '/repo/b.md'; const C = '/repo/c.md'; +test('the unsaved guard keeps its explicit opener restoration to one focus call', async () => { + const ctx = setup(); + try { + const opener = ctx.window.document.createElement('button'); + ctx.window.document.body.append(opener); + opener.focus(); + let restored = 0; + const focus = opener.focus.bind(opener); + opener.focus = () => { restored++; focus(); }; + const pending = ctx.window.showUnsavedEditsDialog([]); + ctx.window.document.activeElement.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { + key: 'Escape', bubbles: true, cancelable: true, + })); + assert.equal(await pending, false); + assert.equal(ctx.window.document.activeElement, opener); + assert.equal(restored, 1); + } finally { ctx.destroy(); } +}); + async function dirtyTab(ctx, sessionId, file, text = 'mine') { ctx.disk.set(file, 'x0\n'); ctx.window.switchPanel(sessionId); diff --git a/test/dom-modal-routing.test.js b/test/dom-modal-routing.test.js index c8a6c5b4..981cfb39 100644 --- a/test/dom-modal-routing.test.js +++ b/test/dom-modal-routing.test.js @@ -8,6 +8,8 @@ const vm = require('node:vm'); const { setupTerminalDom } = require('./terminal-manager-harness'); const { loadAppFunctions } = require('./app-source'); const { setupSidebarDom } = require('./dom-setup'); +const { JSDOM } = require('jsdom'); +const { createCanvas } = require('@napi-rs/canvas'); function setup(api = {}) { const writes = []; @@ -192,6 +194,245 @@ test('registering a modal removes focus from the terminal textarea', () => { } finally { ctx.destroy(); } }); +function realTerminal(ctx) { + const canvases = new WeakMap(); + ctx.window.HTMLCanvasElement.prototype.getContext = function (type) { + if (!canvases.has(this)) canvases.set(this, createCanvas(this.width, this.height)); + return canvases.get(this).getContext(type); + }; + ctx.window.matchMedia = () => Object.assign(new ctx.window.EventTarget(), { + matches: false, addListener() {}, removeListener() {}, + }); + vm.runInContext(fs.readFileSync(require.resolve('@xterm/xterm'), 'utf8'), ctx.context); + const Xterm = ctx.window.Terminal; + ctx.window.Terminal = class extends Xterm { + loadAddon() {} + get unicode() { return { activeVersion: '' }; } + }; + return ctx.window.createTerminalEntry({ sessionId: 's1' }); +} + +function typeAndPaste(ctx, textarea) { + ctx.press(textarea, 'a'); + textarea.dispatchEvent(new ctx.window.KeyboardEvent('keypress', { + key: 'a', charCode: 97, keyCode: 97, bubbles: true, cancelable: true, + })); + textarea.dispatchEvent(new ctx.window.InputEvent('beforeinput', { + inputType: 'insertText', data: 'a', bubbles: true, cancelable: true, + })); + textarea.value = 'a'; + textarea.dispatchEvent(new ctx.window.InputEvent('input', { + inputType: 'insertText', data: 'a', bubbles: true, + })); + const paste = new ctx.window.Event('paste', { bubbles: true, cancelable: true }); + Object.defineProperty(paste, 'clipboardData', { value: { getData: () => 'PASTED' } }); + textarea.dispatchEvent(paste); +} + +for (const pathToFocus of ['showSession', 'fullscreen', 'direct focus']) { + test(`What's new keeps focus and blocks real xterm input after ${pathToFocus}`, async () => { + let fullscreen; + const ctx = setup({ onFullScreenChanged: cb => { fullscreen = cb; } }); + let entry; + try { + entry = realTerminal(ctx); + const textarea = entry.element.querySelector('.xterm-helper-textarea'); + entry.terminal.focus(); + const overlay = require('../public/whats-new').showWhatsNew(ctx.doc, { whatsNewDismissed() {} }, + ctx.window.escapeHtml, { sections: [] }); + const focused = ctx.doc.activeElement; + let focusCalls = 0; + const focus = entry.terminal.focus.bind(entry.terminal); + entry.terminal.focus = () => { focusCalls++; focus(); }; + if (pathToFocus === 'showSession') ctx.window.showSession('s1'); + else if (pathToFocus === 'fullscreen') { + ctx.window.activeSessionId = 's1'; + const source = fs.readFileSync(path.join(__dirname, '../public/app.js'), 'utf8'); + const start = source.indexOf('window.api.onFullScreenChanged(() => {'); + assert.notEqual(start, -1); + vm.runInContext(source.slice(start, source.indexOf('\n});', start) + 5), ctx.context); + fullscreen(); + await new Promise(resolve => ctx.window.requestAnimationFrame(resolve)); + } else entry.terminal.focus(); + typeAndPaste(ctx, textarea); + assert.deepEqual(ctx.writes, [], 'typing and paste must not reach the PTY'); + assert.equal(ctx.doc.activeElement, focused, 'focus must stay on the last focused modal control'); + if (pathToFocus !== 'direct focus') assert.equal(focusCalls, 0, 'background paths must skip terminal.focus'); + ctx.press(focused, 'Escape'); + assert.equal(overlay.isConnected, false); + entry.terminal.focus(); + typeAndPaste(ctx, textarea); + assert.ok(ctx.writes.some(([id, data]) => id === 's1' && data === 'a')); + assert.ok(ctx.writes.some(([id, data]) => id === 's1' && data === 'PASTED')); + } finally { + entry?.terminal.dispose(); + ctx.destroy(); + } + }); +} + +for (const type of ['paste', 'beforeinput', 'input']) { + test(`the modal stack cancels terminal ${type} before its target handler but allows modal editing`, () => { + const ctx = setup(); + try { + ctx.window.showSendPromptDialog({ sessionId: 's1', remoteAlias: 'box' }); + const field = ctx.doc.querySelector('.new-session-dialog textarea'); + let delivered = 0; + ctx.textarea.addEventListener(type, () => { delivered++; }); + field.addEventListener(type, () => { delivered++; }); + const blocked = new ctx.window.Event(type, { bubbles: true, cancelable: true }); + ctx.textarea.dispatchEvent(blocked); + assert.equal(blocked.defaultPrevented, true); + assert.equal(delivered, 0); + const allowed = new ctx.window.Event(type, { bubbles: true, cancelable: true }); + field.dispatchEvent(allowed); + assert.equal(allowed.defaultPrevented, false); + assert.equal(delivered, 1); + ctx.press(field, 'Escape'); + ctx.textarea.dispatchEvent(new ctx.window.Event(type, { bubbles: true, cancelable: true })); + assert.equal(delivered, 2); + } finally { ctx.destroy(); } + }); +} + +test('the focus trap keeps the top modal control and falls back when that control is removed', () => { + const ctx = setup(); + try { + const lower = ctx.doc.createElement('div'); + lower.innerHTML = '
'; + ctx.doc.body.append(lower); + const unregisterLower = ctx.window.registerModal({ overlay: lower, cancel() {} }); + const lowerButton = lower.querySelector('button'); + lowerButton.focus(); + const upper = ctx.doc.createElement('div'); + upper.innerHTML = '
'; + ctx.doc.body.append(upper); + const unregisterUpper = ctx.window.registerModal({ overlay: upper, cancel() {} }); + const upperButton = upper.querySelector('button'); + upperButton.focus(); + lowerButton.focus(); + assert.equal(ctx.doc.activeElement, upperButton); + upperButton.remove(); + ctx.textarea.focus(); + assert.equal(ctx.doc.activeElement, upper.firstElementChild); + upper.remove(); + unregisterUpper(); + assert.equal(ctx.doc.activeElement, lowerButton, 'closing the top modal restores its underlying modal'); + lower.remove(); + unregisterLower(); + } finally { ctx.destroy(); } +}); + +for (const kind of ["What's new", 'Send prompt', 'Add Project']) { + test(`${kind} restores its connected opener once on Escape`, () => { + const ctx = setup(); + try { + const opener = ctx.doc.createElement('button'); + ctx.doc.body.append(opener); + opener.focus(); + let restored = 0; + const focus = opener.focus.bind(opener); + opener.focus = () => { restored++; focus(); }; + if (kind === "What's new") require('../public/whats-new').showWhatsNew(ctx.doc, + { whatsNewDismissed() {} }, ctx.window.escapeHtml, { sections: [] }); + else if (kind === 'Send prompt') ctx.window.showSendPromptDialog({ sessionId: 's1', remoteAlias: 'box' }); + else ctx.window.showAddProjectDialog(); + ctx.press(ctx.doc.activeElement, 'Escape'); + assert.equal(ctx.doc.activeElement, opener); + assert.equal(restored, 1); + } finally { ctx.destroy(); } + }); +} + +for (const openerState of ['removed', 'body']) { + test(`closing the last modal with a ${openerState} opener focuses the active terminal once`, () => { + const ctx = setup(); + try { + const opener = ctx.doc.createElement('button'); + ctx.doc.body.append(opener); + if (openerState === 'removed') opener.focus(); + let restored = 0; + ctx.window.activeSessionId = 's1'; + ctx.window.openSessions.set('s1', { terminal: { focus() { restored++; ctx.textarea.focus(); } } }); + const overlay = ctx.doc.createElement('div'); + overlay.innerHTML = '
Dialog
'; + ctx.doc.body.append(overlay); + const unregister = ctx.window.registerModal({ overlay, cancel() {} }); + opener.remove(); + overlay.remove(); + unregister(); + unregister(); + assert.equal(ctx.doc.activeElement, ctx.textarea); + assert.equal(restored, 1); + } finally { ctx.destroy(); } + }); +} + +for (const kind of ['appConfirm', 'New Session', 'Touched info']) { + test(`${kind} keeps its explicit opener restore without a second focus call`, async () => { + const ctx = setup(); + try { + const opener = ctx.doc.createElement('button'); + ctx.doc.body.append(opener); + opener.focus(); + let restored = 0; + const focus = opener.focus.bind(opener); + opener.focus = () => { restored++; focus(); }; + let pending; + if (kind === 'appConfirm') pending = ctx.window.appConfirm({ title: 'Question' }); + else if (kind === 'Touched info') { + const filename = path.join(__dirname, '../public/touched-files-view.js'); + vm.runInContext(fs.readFileSync(filename, 'utf8'), ctx.context, { filename }); + ctx.window.showTouchedInfo(opener); + } + else await ctx.window.showNewSessionDialog({ projectPath: '/fixture' }, opener); + ctx.press(ctx.doc.activeElement, 'Escape'); + if (pending) await pending; + assert.equal(ctx.doc.activeElement, opener); + assert.equal(restored, 1); + } finally { ctx.destroy(); } + }); +} + +for (const missing of ['FIXED_SHORTCUTS', 'isMac', 'appShortcuts']) { + test(`outside modal keys remain consumed when ${missing} is not loaded`, () => { + const dom = new JSDOM('
', { runScripts: 'outside-only' }); + try { + const context = dom.getInternalVMContext(); + for (const [name, value] of Object.entries({ + SHORTCUT_DEFS: [{ id: 'fixture' }], FIXED_SHORTCUTS: {}, isMac: false, appShortcuts: {}, + })) if (name !== missing) dom.window[name] = value; + dom.window.matchShortcut = () => false; + const filename = path.join(__dirname, '../public/modal-stack.js'); + vm.runInContext(fs.readFileSync(filename, 'utf8'), context, { filename }); + dom.window.registerModal({ overlay: dom.window.document.getElementById('overlay'), cancel() {} }); + const event = new dom.window.KeyboardEvent('keydown', { key: 'a', bubbles: true, cancelable: true }); + assert.doesNotThrow(() => dom.window.routeModalKey(event)); + let delivered = 0; + dom.window.document.addEventListener('keydown', () => { delivered++; }); + dom.window.document.body.dispatchEvent(event); + assert.equal(delivered, 0); + } finally { dom.window.close(); } + }); +} + +test('appConfirm above another modal restores that modal opener only once', async () => { + const ctx = setup(); + try { + ctx.window.showSendPromptDialog({ sessionId: 's1', remoteAlias: 'box' }); + const opener = ctx.doc.activeElement; + const pending = ctx.window.appConfirm({ title: 'Question' }); + let restored = 0; + const focus = opener.focus.bind(opener); + opener.focus = () => { restored++; focus(); }; + ctx.press(ctx.doc.activeElement, 'Escape'); + assert.equal(await pending, false); + assert.equal(ctx.doc.activeElement, opener); + assert.equal(restored, 1); + ctx.press(opener, 'Escape'); + } finally { ctx.destroy(); } +}); + for (const mode of [null, 'plan', 'dangerous-skip']) { test(`Resume focuses its selected ${mode || 'default'} mode and Enter resumes from the opener`, async () => { const ctx = setup({ getEffectiveSettings: async () => mode === 'dangerous-skip' diff --git a/test/dom-new-session-menu.test.js b/test/dom-new-session-menu.test.js index 5d1b0bbf..53e1532c 100644 --- a/test/dom-new-session-menu.test.js +++ b/test/dom-new-session-menu.test.js @@ -218,8 +218,9 @@ for (const handled of [false, true]) { ctx.document.body.appendChild(outside); const target = handled ? dialog.querySelector('.new-session-start-btn') : outside; target.focus(); + assert.ok(dialog.contains(ctx.document.activeElement), 'outside focus is returned to the dialog'); if (handled) ctx.window.addEventListener('keydown', e => e.preventDefault(), { once: true, capture: true }); - ctx.press('Enter'); + target.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true })); assert.equal(launches.length, 0); assert.ok(ctx.document.querySelector('.new-session-overlay')); } finally { ctx.destroy(); } diff --git a/test/jsonl-viewer-sanitize.test.js b/test/jsonl-viewer-sanitize.test.js index 0048ee7e..700dd50c 100644 --- a/test/jsonl-viewer-sanitize.test.js +++ b/test/jsonl-viewer-sanitize.test.js @@ -68,6 +68,25 @@ test('a fullscreen transcript screenshot traps Tab and closes with Escape from o } finally { dom.window.close(); } }); +test('closing a fullscreen screenshot restores its explicit opener once', () => { + const dom = setupDom(); + try { + const { window } = dom; + const opener = window.document.createElement('button'); + window.document.body.append(opener); + opener.focus(); + let restored = 0; + const focus = opener.focus.bind(opener); + opener.focus = () => { restored++; focus(); }; + window.showJsonlScreenshot('data:image/png;base64,AA=='); + window.document.activeElement.dispatchEvent(new window.KeyboardEvent('keydown', { + key: 'Escape', bubbles: true, cancelable: true, + })); + assert.equal(window.document.activeElement, opener); + assert.equal(restored, 1); + } finally { dom.window.close(); } +}); + test('renderJsonlText: javascript: URL in markdown link produces no href', () => { const dom = setupDom(); try { From a9d8d0722572da229a6b7b8eb23266a93179f502 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 12 Oct 2026 18:15:18 +0200 Subject: [PATCH 06/10] (ui): fall back to the active terminal when a modal's opener cannot take focus (#585) An opener still in the DOM but hidden (the visible session changed under the modal) took the restore and left focus nowhere. Restore now checks that focus actually landed on the opener before skipping the terminal fallback. --- CHANGELOG.md | 2 +- public/modal-stack.js | 5 ++++- test/dom-modal-routing.test.js | 7 ++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e1154c6..c0f233bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased ### Changed -- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. Only the top dialog receives keys and keeps focus, with terminal typing, paste and app shortcuts paused while it is open; closing it returns focus to its opener or the active terminal. The unsaved-edits guard waits for pending questions, opens over other dialogs, and keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) +- Confirmations and error messages now use Switchboard’s own dialog, matching the rest of the app. Only the top dialog receives keys and keeps focus, with terminal typing, paste and app shortcuts paused while it is open; closing it gives focus back to the control that opened it when that control is still there. The unsaved-edits guard waits for pending questions, opens over other dialogs, and keeps its Save/Discard/Cancel layout: Enter saves, Escape cancels, and Tab or Shift+Tab moves between its buttons. (#585) - **Archive Project** now keeps your settings and suspends schedules until you restore it from Settings → **Archived projects**. A separate **Delete Project** confirms the removal of settings, cache and schedule registration, while keeping your folders and files. (#568) - Markdown previews now show a file's frontmatter as a readable header above its content, including its name, description and metadata. (#593) - Closing the window (the close button, `Alt`+`F4`) asks for confirmation first, with Cancel selected; a window that is busy at that moment asks once it is free. Quitting from the ☰ menu (Quit, or Exit on Windows) does not ask again, and a quit from outside (a logout, an update install) is not stopped by the question. On macOS it asks "Close the window?": Switchboard stays in the Dock, but its sessions stop. If the window has stopped responding, a single close waits, and closing it again a few seconds later ends it; never while one of its own confirmation boxes waits for an answer. A window closed while it is still starting asks once it has loaded. (#478) diff --git a/public/modal-stack.js b/public/modal-stack.js index c72c3c7f..f6551ff9 100644 --- a/public/modal-stack.js +++ b/public/modal-stack.js @@ -35,9 +35,12 @@ function registerModal({ overlay, cancel, enter, bodyEnter = false, restoreFocus if (remaining && restoreFocus) { if (!remaining.overlay.contains(document.activeElement)) focusModal(remaining); } else if (!remaining && restoreFocus) { + let restored = false; if (opener?.isConnected && opener !== document.body && opener !== document.documentElement && !opener.matches(':disabled')) { opener.focus(); - } else if (typeof activeSessionId !== 'undefined' && typeof openSessions !== 'undefined') { + restored = document.activeElement === opener; + } + if (!restored && typeof activeSessionId !== 'undefined' && typeof openSessions !== 'undefined') { const entry = activeSessionId && openSessions.get(activeSessionId); if (entry && !entry.closed) entry.terminal.focus(); } diff --git a/test/dom-modal-routing.test.js b/test/dom-modal-routing.test.js index 981cfb39..3c47357f 100644 --- a/test/dom-modal-routing.test.js +++ b/test/dom-modal-routing.test.js @@ -344,13 +344,14 @@ for (const kind of ["What's new", 'Send prompt', 'Add Project']) { }); } -for (const openerState of ['removed', 'body']) { +for (const openerState of ['removed', 'body', 'unfocusable']) { test(`closing the last modal with a ${openerState} opener focuses the active terminal once`, () => { const ctx = setup(); try { const opener = ctx.doc.createElement('button'); ctx.doc.body.append(opener); - if (openerState === 'removed') opener.focus(); + if (openerState !== 'body') opener.focus(); + if (openerState === 'unfocusable') opener.focus = () => {}; let restored = 0; ctx.window.activeSessionId = 's1'; ctx.window.openSessions.set('s1', { terminal: { focus() { restored++; ctx.textarea.focus(); } } }); @@ -358,7 +359,7 @@ for (const openerState of ['removed', 'body']) { overlay.innerHTML = '
Dialog
'; ctx.doc.body.append(overlay); const unregister = ctx.window.registerModal({ overlay, cancel() {} }); - opener.remove(); + if (openerState !== 'unfocusable') opener.remove(); overlay.remove(); unregister(); unregister(); From 1923c1e05b735ac72e8a73688622e35c689a8be3 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Tue, 13 Oct 2026 11:49:37 +0200 Subject: [PATCH 07/10] (ui): route #478's close question through the modal stack (#585) The close question asks with replace: true and may carry closeWith. With choice dialogs queued by the modal stack, replace now cancels the open one and puts the question at the front of the queue instead of resolving it as Cancel. Every close path (answer, replace, closeWith) unregisters from the stack. The tests follow the queue: a second choice dialog waits instead of answering null, and the native-dialog test waits for the idle question. --- .ai/contexts/app-dialogs.md | 2 ++ .ai/contexts/window-frame.md | 12 +++++--- public/choice-dialog.js | 2 +- public/modal-stack.js | 5 +-- test/dom-choice-dialog.test.js | 37 +++++++++++++++++++++++ test/dom-file-panel-unsaved-guard.test.js | 20 +++++++++--- 6 files changed, 66 insertions(+), 12 deletions(-) diff --git a/.ai/contexts/app-dialogs.md b/.ai/contexts/app-dialogs.md index 0422d75a..232d1292 100644 --- a/.ai/contexts/app-dialogs.md +++ b/.ai/contexts/app-dialogs.md @@ -12,6 +12,8 @@ Choice questions keep the existing rules: Enter on a checkbox does nothing, and ## Question queue and the unsaved guard +`showChoiceDialog({ replace: true })` is the one way to jump the queue: it cancels the open choice dialog (resolving it as Cancel) and enqueues its question at the front (`enqueueModalQuestion(open, { front: true })`). `closeWith`, a promise, closes the dialog with the value it resolves to; both paths close through the same function that unregisters from the modal stack. The window-close question (#478, [window-frame](window-frame.md), "Closing the window") uses both. + Use async `appConfirm({ title, message, confirmLabel, danger })` and `appAlert({ title, message })` from `public/app-dialogs.js`. They adapt `showChoiceDialog`, which shares `enqueueModalQuestion` with session/worktree deletion and the Save/Discard/Cancel guard. Busy questions stay pending rather than resolving as Cancel. Regular questions use FIFO order; only one queued question is mounted at a time. A question may sit on top of a configuration modal. The quit/close/reload guard ACKs immediately, then enqueues an idle question rather than waiting on an idle snapshot and opening separately. Idle questions yield to regular questions and wait only for the question queue to settle. Configuration and informational modals do not delay the guard: it registers last and opens on top of them. A zero-delay task checks question idleness after the current promise reactions, allowing a caller's next continuation question (including `resolveResumeSession`'s candidate loop and fallback) to enter the queue first. Buffers are recollected when the guard actually opens. The guard holds the question queue during saving; an alert requested meanwhile opens after the guard closes. Save failures stay inline. Save receives initial focus, Enter really saves, Escape cancels, and Tab/Shift+Tab are handled by the shared router. Disabled controls cannot act during an in-flight save. The beforeunload veto and main-process protocol are unchanged. diff --git a/.ai/contexts/window-frame.md b/.ai/contexts/window-frame.md index 99343d5d..5d3f6643 100644 --- a/.ai/contexts/window-frame.md +++ b/.ai/contexts/window-frame.md @@ -246,10 +246,14 @@ session, so there the question reads "Close the window?" and says the app stays in the Dock. The question is the in-page choice dialog (`confirmWindowClose` in `file-panel.js`), not a native `confirm()`: Cancel has the focus, so `Enter` keeps the app, and the page keeps running while it -is shown, which the probe below relies on. Only one choice dialog shows at a -time, so the close question is asked with `replace: true`: another one that is -open (the Archive folder dialog, say) is cancelled first, and a close is never -answered with a silent Cancel because something else was on screen. +is shown, which the probe below relies on. Choice dialogs wait in the modal +stack's question queue ([app-dialogs](app-dialogs.md)), so the close question +is asked with `replace: true`: the one that is open (the Archive folder dialog, +say) is cancelled first and the question goes to the front of the queue, so a +close is never answered with a silent Cancel or held behind a queued question. +It registers with the modal stack like any choice dialog: `Escape` cancels, +`Tab` stays inside it, and every close path (answer, `replace`, `closeWith`) +unregisters it. It rides on the unsaved-edits guard (`unsaved-guard.js`, see [viewer-panel](viewer-panel.md), "Unsaved edits on quit, reload and close"): diff --git a/public/choice-dialog.js b/public/choice-dialog.js index f92cbdac..47bb3252 100644 --- a/public/choice-dialog.js +++ b/public/choice-dialog.js @@ -20,7 +20,7 @@ function showChoiceDialog(options = {}) { const restore = options.replace && returnFocus && returnFocus.isConnected === false ? document.activeElement : returnFocus; choiceDialogReturnFocus = restore; return openChoiceDialog({ ...options, returnFocus: restore }); - }); + }, { front: !!options.replace }); } function openChoiceDialog({ diff --git a/public/modal-stack.js b/public/modal-stack.js index f6551ff9..f8c2784d 100644 --- a/public/modal-stack.js +++ b/public/modal-stack.js @@ -112,9 +112,10 @@ for (const type of ['paste', 'beforeinput', 'input']) { }, true); } -function enqueueModalQuestion(open, { whenIdle = false } = {}) { +function enqueueModalQuestion(open, { whenIdle = false, front = false } = {}) { return new Promise((resolve, reject) => { - (whenIdle ? modalIdleQuestions : modalQuestions).push({ open, resolve, reject }); + const queue = whenIdle ? modalIdleQuestions : modalQuestions; + queue[front ? 'unshift' : 'push']({ open, resolve, reject }); drainModalQuestions(); }); } diff --git a/test/dom-choice-dialog.test.js b/test/dom-choice-dialog.test.js index c78d0695..ad32ee1b 100644 --- a/test/dom-choice-dialog.test.js +++ b/test/dom-choice-dialog.test.js @@ -337,3 +337,40 @@ test('choice dialog: closeWith closes it with its value, and does nothing once t assert.equal(ctx.document.activeElement, elsewhere, 'the focus is not taken back a second time'); } finally { ctx.destroy(); } }); + +test('choice dialog: replace cancels the open one and goes ahead of the queue; Escape on it gives the focus back', async () => { + const ctx = setupSidebarDom(); + try { + const origin = ctx.document.createElement('button'); + ctx.document.body.appendChild(origin); + origin.focus(); + const first = open(ctx, { title: 'First' }); + const queued = open(ctx, { title: 'Queued' }); + const closing = open(ctx, { title: 'Close?', replace: true, initialFocus: 'cancel', returnFocus: origin }); + assert.equal(await first, null, 'the open dialog is cancelled'); + await new Promise((r) => setTimeout(r)); + assert.equal(ctx.document.querySelectorAll('.choice-dialog-overlay').length, 1); + assert.equal(shown(ctx).querySelector('#choice-dialog-title').textContent, 'Close?', 'it is not behind the queued one'); + assert.equal(ctx.document.activeElement, cancelBtn(ctx)); + key(ctx, ctx.document.activeElement, 'Escape'); + assert.equal(await closing, null); + await new Promise((r) => setTimeout(r)); + assert.equal(shown(ctx).querySelector('#choice-dialog-title').textContent, 'Queued', 'the queue resumes'); + key(ctx, ctx.document.activeElement, 'Escape'); + assert.equal(await queued, null); + assert.equal(ctx.window.hasOpenModal(), false, 'every path unregistered from the modal stack'); + assert.equal(ctx.document.activeElement, origin); + } finally { ctx.destroy(); } +}); + +test('choice dialog: closeWith closes a replaced-in dialog and leaves the modal stack empty', async () => { + const ctx = setupSidebarDom(); + try { + let settle; + const pending = open(ctx, { replace: true, closeWith: new Promise((r) => { settle = r; }) }); + assert.equal(ctx.window.hasOpenModal(), true); + settle({}); + assert.deepEqual(await pending, {}); + assert.equal(ctx.window.hasOpenModal(), false); + } finally { ctx.destroy(); } +}); diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index 1aa90fdf..1749f708 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -543,13 +543,23 @@ test('a window close while another choice dialog is open cancels that one and as } finally { ctx.destroy(); } }); -test('a choice dialog that does not ask to replace still yields to one already open', async () => { +test('a choice dialog that does not ask to replace waits behind the open one, and shows once it closes', async () => { const ctx = setup(); try { - ctx.window.showChoiceDialog({ title: 'first' }); - const second = await ctx.window.showChoiceDialog({ title: 'second' }); + const { document } = ctx.window; + const first = ctx.window.showChoiceDialog({ title: 'first' }); + let second; + ctx.window.showChoiceDialog({ title: 'second' }).then((v) => { second = v; }); + await flush(); + assert.equal(second, undefined, 'the second question is not answered as a silent Cancel'); + assert.equal(document.querySelector('#choice-dialog-title').textContent, 'first'); + document.activeElement.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })); + assert.equal(await first, null); + await flush(); + assert.equal(document.querySelector('#choice-dialog-title').textContent, 'second'); + document.querySelector('.choice-dialog-cancel').click(); + await flush(); assert.equal(second, null); - assert.equal(ctx.window.document.querySelector('#choice-dialog-title').textContent, 'first'); } finally { ctx.destroy(); } }); @@ -593,7 +603,7 @@ test('a native dialog the page shows is reported around it, and the shipped guar }; ctx.window.confirm('Delete "x"?'); assert.deepEqual(during, { pings: 0, destroys: 0 }, 'a close that arrives behind a native dialog waits for the user to answer it'); - await flush(); + for (let i = 0; i < 40 && !dialog(ctx); i++) await flush(); assert.ok(dialog(ctx), 'the page answers once the dialog is gone'); ctx.calls.onConfirm = () => { From f168a2d2dc41af7b5a09c3cf6cdb881f740dac0f Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Tue, 13 Oct 2026 12:10:44 +0200 Subject: [PATCH 08/10] (ui): a close question is never held behind another question (#585) closeWith now settles a close question that has not opened yet, so a quit that arrives first answers yes without waiting for the question ahead of it. A replace question asked while a non-choice question (session or worktree deletion) is open opens on top of it instead of waiting, and Escape returns to the question underneath. --- .ai/contexts/window-frame.md | 10 ++-- eslint.config.js | 1 + public/choice-dialog.js | 6 ++- public/modal-stack.js | 17 ++++++- test/dom-file-panel-unsaved-guard.test.js | 58 +++++++++++++++++++++++ 5 files changed, 86 insertions(+), 6 deletions(-) diff --git a/.ai/contexts/window-frame.md b/.ai/contexts/window-frame.md index 5d3f6643..2b87846c 100644 --- a/.ai/contexts/window-frame.md +++ b/.ai/contexts/window-frame.md @@ -248,9 +248,13 @@ stays in the Dock. The question is the in-page choice dialog has the focus, so `Enter` keeps the app, and the page keeps running while it is shown, which the probe below relies on. Choice dialogs wait in the modal stack's question queue ([app-dialogs](app-dialogs.md)), so the close question -is asked with `replace: true`: the one that is open (the Archive folder dialog, -say) is cancelled first and the question goes to the front of the queue, so a -close is never answered with a silent Cancel or held behind a queued question. +is asked with `replace: true`: an open choice dialog (the Archive folder dialog, +say) is cancelled first and the question goes to the front of the queue; any +other open question (Delete session, Delete worktree) is left alone and the +close question opens on top of it, like the unsaved guard, and Escape returns +to it. Queued questions never go ahead of it. `closeWith` also settles a close +question that has not opened yet, so a quit that arrives first is never held +behind another question. It registers with the modal stack like any choice dialog: `Escape` cancels, `Tab` stays inside it, and every close path (answer, `replace`, `closeWith`) unregisters it. diff --git a/eslint.config.js b/eslint.config.js index d0706b05..83df9997 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -27,6 +27,7 @@ const rendererCrossFileGlobals = { registerModal: 'readonly', hasOpenModal: 'readonly', enqueueModalQuestion: 'readonly', +hasActiveModalQuestion: 'readonly', TOOL_BAR: 'readonly', filePanelState: 'readonly', currentPanelSessionId: 'readonly', diff --git a/public/choice-dialog.js b/public/choice-dialog.js index 47bb3252..48fbb1fe 100644 --- a/public/choice-dialog.js +++ b/public/choice-dialog.js @@ -16,11 +16,15 @@ function showChoiceDialog(options = {}) { const active = document.activeElement; const returnFocus = options.returnFocus || (active?.closest('.choice-dialog-overlay') ? choiceDialogReturnFocus : active); if (options.replace && closeOpenChoiceDialog) closeOpenChoiceDialog(null); + else if (options.replace && hasActiveModalQuestion()) { + choiceDialogReturnFocus = returnFocus; + return openChoiceDialog({ ...options, returnFocus }); + } return enqueueModalQuestion(() => { const restore = options.replace && returnFocus && returnFocus.isConnected === false ? document.activeElement : returnFocus; choiceDialogReturnFocus = restore; return openChoiceDialog({ ...options, returnFocus: restore }); - }, { front: !!options.replace }); + }, { front: !!options.replace, settleWith: options.closeWith || null }); } function openChoiceDialog({ diff --git a/public/modal-stack.js b/public/modal-stack.js index f8c2784d..99aa5524 100644 --- a/public/modal-stack.js +++ b/public/modal-stack.js @@ -112,10 +112,23 @@ for (const type of ['paste', 'beforeinput', 'input']) { }, true); } -function enqueueModalQuestion(open, { whenIdle = false, front = false } = {}) { +function hasActiveModalQuestion() { + return modalQuestionActive; +} + +function enqueueModalQuestion(open, { whenIdle = false, front = false, settleWith = null } = {}) { return new Promise((resolve, reject) => { const queue = whenIdle ? modalIdleQuestions : modalQuestions; - queue[front ? 'unshift' : 'push']({ open, resolve, reject }); + const question = { open, resolve, reject }; + queue[front ? 'unshift' : 'push'](question); + if (settleWith) { + settleWith.then((value) => { + const index = queue.indexOf(question); + if (index === -1) return; + queue.splice(index, 1); + resolve(value); + }, () => {}); + } drainModalQuestions(); }); } diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index 1749f708..af1e3ce5 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -866,3 +866,61 @@ test('an alert waits for the unsaved guard, then owns focus and Escape', async ( await alerting; } finally { ctx.destroy(); } }); + +function openOtherQuestion(ctx) { + const { document } = ctx.window; + const overlay = document.createElement('div'); + overlay.className = 'new-session-overlay'; + const dialog = document.createElement('div'); + dialog.className = 'new-session-dialog'; + const ok = document.createElement('button'); + ok.textContent = 'Delete'; + dialog.append(ok); + overlay.append(dialog); + return ctx.window.enqueueModalQuestion(() => new Promise((resolve) => { + document.body.append(overlay); + const unregister = ctx.window.registerModal({ overlay, cancel: () => { overlay.remove(); unregister(); resolve(false); }, restoreFocus: false }); + })); +} + +test('a quit that arrives while the close question waits behind another question answers yes without that question', async () => { + const ctx = setup(); + const { document } = ctx.window; + try { + let answered = null; + openOtherQuestion(ctx).then((v) => { answered = v; }); + await flush(); + ctx.calls.check(1, 'close'); + await flush(); + ctx.calls.reason(1, 'quit'); + await flush(); + assert.deepEqual(ctx.calls.answers, [{ id: 1, proceed: true }]); + assert.equal(answered, null, 'the other question is still the user\'s to answer'); + assert.ok(document.querySelector('.new-session-overlay')); + } finally { ctx.destroy(); } +}); + +test('the close question opens on top of another question and Escape returns to it', async () => { + const ctx = setup(); + const { document } = ctx.window; + try { + let answered = null; + openOtherQuestion(ctx).then((v) => { answered = v; }); + await flush(); + const underneath = document.querySelector('.new-session-overlay'); + ctx.calls.check(1, 'close'); + await flush(); + assert.equal(document.querySelectorAll('.choice-dialog').length, 1, 'the close question is shown, not held behind'); + assert.ok(document.querySelector('.new-session-overlay'), 'the other question is not cancelled'); + document.activeElement.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })); + await flush(); + assert.deepEqual(ctx.calls.answers, [{ id: 1, proceed: false }]); + assert.equal(answered, null); + assert.equal(document.querySelector('.choice-dialog'), null); + assert.ok(underneath.isConnected); + assert.ok(underneath.contains(document.activeElement), 'focus returns to the other question'); + document.activeElement.dispatchEvent(new ctx.window.KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true })); + await flush(); + assert.equal(answered, false, 'Escape now reaches the other question'); + } finally { ctx.destroy(); } +}); From 0099c25b77b4772e303e7d334a135d60468bb1cd Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Tue, 13 Oct 2026 12:42:45 +0200 Subject: [PATCH 09/10] test(ui): settle a queued question before it opens (#585) --- eslint.config.js | 2 +- test/modal-stack-order.test.js | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/eslint.config.js b/eslint.config.js index 83df9997..b1994113 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -27,7 +27,7 @@ const rendererCrossFileGlobals = { registerModal: 'readonly', hasOpenModal: 'readonly', enqueueModalQuestion: 'readonly', -hasActiveModalQuestion: 'readonly', + hasActiveModalQuestion: 'readonly', TOOL_BAR: 'readonly', filePanelState: 'readonly', currentPanelSessionId: 'readonly', diff --git a/test/modal-stack-order.test.js b/test/modal-stack-order.test.js index 52ca1753..055ac3f1 100644 --- a/test/modal-stack-order.test.js +++ b/test/modal-stack-order.test.js @@ -35,3 +35,22 @@ test('the last registered modal is visually above the previous question', async assert.equal(await question, false); } finally { ctx.destroy(); } }); + +test('settleWith resolves a queued question before it opens and removes it from the queue', async () => { + const ctx = setupSidebarDom(); + try { + const { enqueueModalQuestion, hasActiveModalQuestion } = ctx.window; + let release; + enqueueModalQuestion(() => new Promise((r) => { release = r; })); + assert.equal(hasActiveModalQuestion(), true); + let opened = 0; + let settle; + const queued = enqueueModalQuestion(() => { opened++; return 'opened'; }, { settleWith: new Promise((r) => { settle = r; }) }); + settle('settled'); + assert.equal(await Promise.race([queued, new Promise((r) => setTimeout(() => r('still queued'), 50))]), 'settled'); + release(); + await new Promise((r) => setTimeout(r, 10)); + assert.equal(opened, 0, 'a settled question never opens'); + assert.equal(hasActiveModalQuestion(), false); + } finally { ctx.destroy(); } +}); From 2dc54c3ad069d97fc23affe3efaf577b09481155 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Tue, 13 Oct 2026 12:58:35 +0200 Subject: [PATCH 10/10] (ui): follow #599's Archive dialogs under the modal stack (#585) The Hide confirmations these tests drove no longer exist; Archive and Delete use the choice dialog, which now needs the modal stack loaded. --- test/dom-agents-view.test.js | 18 ------------------ test/dom-sidebar-dialogs.test.js | 26 -------------------------- test/project-settings-archive.test.js | 2 +- 3 files changed, 1 insertion(+), 45 deletions(-) diff --git a/test/dom-agents-view.test.js b/test/dom-agents-view.test.js index 8591fbd6..03a390af 100644 --- a/test/dom-agents-view.test.js +++ b/test/dom-agents-view.test.js @@ -915,21 +915,3 @@ test('background Delete waits for its async confirmation and cancellation sends await ctx.window.runAgentVerb('rm', ROSTER[1]); assert.deepEqual(ctx.calls.verbs, [['rm', ROSTER[1].id]]); }); -for (const agree of [false, true]) { - test('project Hide ' + (agree ? 'proceeds' : 'cancels') + ' after an async decision', async t => { - const ctx = setup({ settingsPanel: true }); t.after(() => ctx.destroy()); - const removed = []; - ctx.window.api.removeProject = async (...args) => { removed.push(args); }; - await ctx.window.openSettingsViewer('project', '/project', 'folder-key'); - let answer; - let options; - ctx.window.appConfirm = value => { options = value; return new Promise(resolve => { answer = resolve; }); }; - ctx.document.getElementById('sv-remove-btn').click(); - assert.deepEqual(removed, []); - assert.equal(options.confirmLabel, 'Hide'); - answer(agree); - await new Promise(resolve => setImmediate(resolve)); - assert.deepEqual(removed, agree ? [['/project', 'folder-key']] : []); - assert.equal(ctx.document.getElementById('settings-viewer').style.display, agree ? 'none' : 'flex'); - }); -} diff --git a/test/dom-sidebar-dialogs.test.js b/test/dom-sidebar-dialogs.test.js index 8031c23d..2c5e6993 100644 --- a/test/dom-sidebar-dialogs.test.js +++ b/test/dom-sidebar-dialogs.test.js @@ -2,32 +2,6 @@ const test = require('node:test'); const assert = require('node:assert/strict'); const { setupSidebarDom, makeSampleProject } = require('./dom-setup'); -for (const agree of [false, true]) { - test('worktree Hide ' + (agree ? 'proceeds' : 'cancels') + ' after an async decision', async () => { - const ctx = setupSidebarDom(); - try { - const parent = makeSampleProject({ projectPath: '/p' }); - const child = makeSampleProject({ projectPath: '/p/.claude/worktrees/w' }); - const removed = []; - ctx.window.api = { removeProject: async path => { removed.push(path); } }; - ctx.window.cachedAllProjects = [parent, child]; - ctx.sidebar.renderProjects([parent, child], true); - const button = ctx.document.querySelector('.worktree-hide-btn'); - assert.ok(button); - let answer; - ctx.window.appConfirm = options => { - assert.equal(options.confirmLabel, 'Hide'); - return new Promise(resolve => { answer = resolve; }); - }; - const pending = button.onclick(new ctx.window.MouseEvent('click')); - assert.deepEqual(removed, []); - answer(agree); - await pending; - assert.deepEqual(removed, agree ? [child.projectPath] : []); - } finally { ctx.destroy(); } - }); -} - for (const agree of [false, true]) { test('Remap ' + (agree ? 'proceeds and reports failure' : 'cancels') + ' after an async decision', async () => { const ctx = setupSidebarDom(); diff --git a/test/project-settings-archive.test.js b/test/project-settings-archive.test.js index daa8ccd3..4a26ea2c 100644 --- a/test/project-settings-archive.test.js +++ b/test/project-settings-archive.test.js @@ -330,7 +330,7 @@ function settingsDom(t, h) { dom.window.alert = () => { throw new Error('native alert must not be used'); }; let reloads = 0; dom.window.loadProjects = async () => { reloads++; }; - for (const file of ['setting-defaults.js', 'utils.js', 'shortcuts.js', 'terminal-themes.js', 'choice-dialog.js', 'settings-panel.js']) { + for (const file of ['setting-defaults.js', 'utils.js', 'shortcuts.js', 'terminal-themes.js', 'modal-stack.js', 'choice-dialog.js', 'settings-panel.js']) { vm.runInContext(read('public/' + file), dom.getInternalVMContext(), { filename: path.join(ROOT, 'public', file) }); } return { window: dom.window, document: dom.window.document, calls, reloads: () => reloads };