diff --git a/src/renderer/main.test.ts b/src/renderer/main.test.ts
index 3d6427488e..e899912291 100644
--- a/src/renderer/main.test.ts
+++ b/src/renderer/main.test.ts
@@ -14,3 +14,48 @@ describe('main layout boot', () => {
)
})
})
+
+// #2474: Cmd/Ctrl+W deletes the active thread, and with Settings — or any other
+// dialog — on screen the keystroke a user meant as "close this" destroyed a
+// conversation instead. main.ts binds its shortcuts inside a boot function with
+// no seam to call, so the wiring is pinned at the source level (as the layout
+// test above does); the gate itself is unit-tested in
+// `views/dialog-shell.test.ts`.
+describe('destructive shortcuts defer to an open dialog', () => {
+ const src = readFileSync(join(process.cwd(), 'src/renderer/main.ts'), 'utf8')
+
+ it('never calls the thread delete without asking whether a dialog is open', () => {
+ // Every call, not just the one: a second unguarded call site would
+ // reintroduce the bug somewhere the first assertion never looks.
+ const calls = [...src.matchAll(/^.*confirmDeleteThread\(\).*$/gm)]
+ .map((m) => m[0])
+ .filter((line) => !line.includes('function confirmDeleteThread'))
+ assert.ok(calls.length > 0, 'expected main.ts to invoke confirmDeleteThread')
+ const unguarded = calls.filter((line) => !line.includes('!isAnyDialogOpen()'))
+ assert.deepEqual(
+ unguarded,
+ [],
+ `these delete the active thread without checking for an open dialog:\n${unguarded.join('\n')}`,
+ )
+ })
+
+ it('still swallows the keystroke whether or not the delete runs', () => {
+ // preventDefault is what keeps Cmd+W from also reaching the File ▸ Close
+ // accelerator. Moving it inside the guard would hand the keystroke back to
+ // the menu exactly when a dialog is up.
+ const block = /if \(meta && e\.key === 'w'\) \{([\s\S]*?)\n {4}\}/.exec(src)?.[1]
+ assert.ok(block, 'could not find the Cmd/Ctrl+W handler')
+ const preventIndex = block.indexOf('e.preventDefault()')
+ const guardIndex = block.indexOf('isAnyDialogOpen()')
+ assert.ok(preventIndex >= 0, 'the Cmd/Ctrl+W handler must preventDefault')
+ assert.ok(guardIndex >= 0, 'the Cmd/Ctrl+W handler must consult isAnyDialogOpen')
+ assert.ok(preventIndex < guardIndex, 'preventDefault must run before the dialog guard')
+ })
+
+ it('does not open the find bar underneath a dialog either', () => {
+ // The same gate, on the shortcut that used to name four dialogs of seventeen.
+ const block = /if \(matchFindInChatShortcut\(e\)\) \{([\s\S]*?)\n {4}\}/.exec(src)?.[1]
+ assert.ok(block, 'could not find the find-in-chat handler')
+ assert.match(block, /if \(isAnyDialogOpen\(\)\) return/)
+ })
+})
diff --git a/src/renderer/main.ts b/src/renderer/main.ts
index ccce2f4c90..5615776f54 100644
--- a/src/renderer/main.ts
+++ b/src/renderer/main.ts
@@ -34,6 +34,7 @@ import { mountTerminalRailResizers } from './views/terminal-rail-resizer.ts'
import { mountRoadmapPane } from './views/roadmap-pane.ts'
import { mountBrowserPane } from './views/browser-pane.ts'
import { mountVncPane } from './views/vnc-pane.ts'
+import { isAnyDialogOpen } from './views/dialog-shell.ts'
import {
mountSettingsDialog,
openSettingsDialog,
@@ -653,15 +654,12 @@ function registerKeyboardShortcuts(): void {
openCommandPalette()
}
// Cmd/Ctrl+F opens the in-conversation find bar (find-in-page for the chat).
- // Skipped while a modal dialog owns the screen so it can't open behind it.
+ // Skipped while a dialog owns the screen so it can't open behind it. This
+ // named four dialogs of the seventeen the renderer has; asking the DOM
+ // covers the rest — an approval, an SSH passphrase, a confirm — each of
+ // which is a question the find bar should not open underneath.
if (matchFindInChatShortcut(e)) {
- if (
- isFileSearchDialogOpen() ||
- isCommandPaletteOpen() ||
- isSettingsDialogOpen() ||
- isKeyboardShortcutsDialogOpen()
- )
- return
+ if (isAnyDialogOpen()) return
e.preventDefault()
openConversationSearch()
}
@@ -684,9 +682,16 @@ function registerKeyboardShortcuts(): void {
if (uiScaleAction === 'reset') void resetUiScale(store, api)
else void bumpUiScale(store, api, uiScaleAction === 'in' ? 1 : -1)
}
+ // Cmd/Ctrl+W deletes the active thread. `preventDefault` stays
+ // unconditional — it is what keeps the keystroke from also reaching the
+ // File ▸ Close accelerator — but with a dialog on screen the delete does
+ // not run: a user closing Settings with Cmd+W meant "close this", not
+ // "destroy this conversation" (#2474). Doing nothing is the right answer
+ // rather than closing the dialog for them; Esc already does that, right
+ // below, and every dialog handles it.
if (meta && e.key === 'w') {
e.preventDefault()
- void confirmDeleteThread()
+ if (!isAnyDialogOpen()) void confirmDeleteThread()
}
if (e.key === 'Escape') {
if (isCommandPaletteOpen()) {
diff --git a/src/renderer/views/dialog-shell.test.ts b/src/renderer/views/dialog-shell.test.ts
index fb69dca933..6589924f1e 100644
--- a/src/renderer/views/dialog-shell.test.ts
+++ b/src/renderer/views/dialog-shell.test.ts
@@ -1,7 +1,7 @@
import '../../../tests/setup-dom.ts'
import { describe, it, beforeEach } from 'node:test'
import assert from 'node:assert/strict'
-import { createOverlayDialog } from './dialog-shell.ts'
+import { createOverlayDialog, isAnyDialogOpen } from './dialog-shell.ts'
describe('createOverlayDialog', () => {
beforeEach(() => {
@@ -44,3 +44,53 @@ describe('createOverlayDialog', () => {
assert.equal(closes, 1)
})
})
+
+describe('isAnyDialogOpen', () => {
+ beforeEach(() => {
+ document.body.innerHTML = ''
+ })
+
+ it('is false with no dialogs, and false when one exists but is closed', () => {
+ assert.equal(isAnyDialogOpen(), false)
+ createOverlayDialog({ id: 'closed-overlay' })
+ assert.equal(isAnyDialogOpen(), false)
+ })
+
+ it('is true while a modal dialog is open, and false again once it closes', () => {
+ const shell = createOverlayDialog({ id: 'modal-overlay' })
+ shell.open()
+ assert.equal(isAnyDialogOpen(), true)
+ shell.close()
+ assert.equal(isAnyDialogOpen(), false)
+ })
+
+ it('counts a non-modal dialog too', () => {
+ // The approval prompt shows inline over the chat with `show()`, not
+ // `showModal()`. It is still a question the user is answering, so a global
+ // shortcut must not act on the transcript behind it.
+ const dialog = document.createElement('dialog')
+ document.body.append(dialog)
+ dialog.show()
+ assert.equal(isAnyDialogOpen(), true)
+ dialog.close()
+ assert.equal(isAnyDialogOpen(), false)
+ })
+
+ it('stays true while any one of several dialogs is still open', () => {
+ const first = createOverlayDialog({ id: 'first-overlay' })
+ const second = createOverlayDialog({ id: 'second-overlay' })
+ first.open()
+ second.open()
+ first.close()
+ assert.equal(isAnyDialogOpen(), true, 'the second dialog is still on screen')
+ second.close()
+ assert.equal(isAnyDialogOpen(), false)
+ })
+
+ it('does not need to know the dialog exists', () => {
+ // The point of reading the DOM: a dialog built anywhere, by anything, is
+ // covered without being registered (#2474).
+ document.body.insertAdjacentHTML('beforeend', '')
+ assert.equal(isAnyDialogOpen(), true)
+ })
+})
diff --git a/src/renderer/views/dialog-shell.ts b/src/renderer/views/dialog-shell.ts
index 558da775eb..3cc05a8d57 100644
--- a/src/renderer/views/dialog-shell.ts
+++ b/src/renderer/views/dialog-shell.ts
@@ -37,3 +37,25 @@ export function createOverlayDialog(opts: { id: string; className?: string }): O
isOpen: (): boolean => dialog.open,
}
}
+
+/**
+ * Whether any `