fix(ui): stop Cmd+W deleting a thread out from under a dialog (#2474) (#2551)

Closes #2474 (P2, `area:ui`).

## The bug

Cmd/Ctrl+W deletes the active thread. Nothing checked whether a dialog
was on screen, so **closing Settings with the keystroke that closes
things everywhere else destroyed a conversation instead.** There is no
undo.

```ts
if (meta && e.key === 'w') {
  e.preventDefault()
  void confirmDeleteThread()   // ← no idea Settings is open
}
```

## The fix

The guard reads the DOM — *is any `<dialog>` open* — rather than asking
a list of `isXOpen()` predicates.

That shape is deliberate. The renderer has **seventeen** dialogs, and
the one hand-maintained list of them sits three lines above this in the
Cmd/Ctrl+F handler, naming **four**: settings, the command palette, file
search, keyboard shortcuts. That list is exactly the failure the issue
anticipates when it says *"or any other dialog for that matter"*, so the
check is built so a new dialog is covered the moment it exists, with
nobody having to remember to register it.

**A behaviour change worth noticing:** the find bar now uses the same
check, so it also defers to the thirteen dialogs the list missed — an
approval, an SSH passphrase, a confirm. Each is a question it should not
open underneath, but it is a change beyond the reported bug, so push
back if you'd rather I left the four-item list alone.

`show()` counts as well as `showModal()`: the approval prompt appears
**inline over the chat** rather than modally, and it is still something
the user is answering.

### `preventDefault()` stays outside the guard

It is what keeps the keystroke from also reaching macOS's File ▸ Close
accelerator (`app-menu-file-items.ts` registers `{ role: 'close' }`
there). Moving it inside would hand Cmd+W back to the menu exactly when
a dialog is up. So the keystroke is still swallowed; only the delete is
skipped.

Doing nothing is the right answer rather than closing the dialog for
them — Esc already does that, in the handler immediately below, and
every dialog honours it.

## Left alone deliberately

The shortcut is matched inline as `meta && e.key === 'w'` rather than
through a `matchXShortcut` helper like its neighbours in
`keyboard-shortcuts.ts`. One consequence: with Caps Lock on, `e.key` is
`'W'` and the shortcut silently does nothing.

Routing it through a matcher would be tidier and testable in the same
place as its siblings — but it would also **widen** which keystrokes
delete a thread, and that is the wrong direction to move a destructive
shortcut without being asked. Happy to do it if you want it.

## Testing

`main.ts` binds its shortcuts inside a boot function with no seam to
call, so the wiring is pinned at the source level — the file's existing
test does the same for the layout boot — while the guard itself is
unit-tested against real `<dialog>` elements in happy-dom.

The wiring assertions test the **property**, not the text: *every*
`confirmDeleteThread()` call site is guarded, so a second unguarded one
added later fails the suite rather than slipping past an assertion that
only looked at the first.

| Check | Result |
|---|---|
| `node scripts/run-tests.mts` (whole repo) | **8641 tests, 8631 pass,
10 skipped, 0 fail** |
| `tsc --noEmit` (node + web) | clean |
| `pnpm run lint` / `oxfmt --check .` | clean |
| `check:dead-code` (840 modules) / `check:oracle` (243 specs, 15
invariants) | clean |
| `check:e2e-syntax` (269 files) / `demo:site:check` | clean |
| all three wiring assertions with the guard removed | fail, as designed
|

9 new tests: 5 on `isAnyDialogOpen` (closed dialog, modal, non-modal,
several at once, and one inserted as raw HTML that nothing registered),
3 on the Cmd+W / Cmd+F wiring, and the existing suite unchanged.

Two local-environment notes, neither affecting CI: reaching a full run
needs an uncommitted shim over node-pty's native loader (it cannot be
rebuilt in this sandbox — the agent proxy denies `iojs.org` and
`www.electronjs.org`), reverted before committing; and this container's
`node_modules` predates #2383, so
`patches/@anthropic-ai__sandbox-runtime@0.0.74.patch` was not applied
and `git-commit-signing.test.ts` failed until I applied its one-line
hunk by hand. That failure reproduces identically on `52c839a` with my
changes stashed, so it is a stale install here, not a base-branch break.

Refs #2474

---
🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01Hpk1gEma9LhMUuvr2TwUj6

---
_Generated by [Claude
Code](https://claude.ai/code/session_01Hpk1gEma9LhMUuvr2TwUj6)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Jonathan Kingston <KingstonMailBox@gmail.com>

Author: 3 people

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/)
+  })
+})

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()) {

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', '<dialog open id="ad-hoc"></dialog>')
+    assert.equal(isAnyDialogOpen(), true)
+  })
+})

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 `<dialog>` in the document is currently open.
+ *
+ * Asked by global keyboard shortcuts, which must defer to whatever the user is
+ * answering rather than act on the screen behind it. Cmd/Ctrl+W is the case
+ * that made this necessary: it 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 (#2474).
+ *
+ * Read off the DOM rather than from a list of `isXOpen()` predicates. There are
+ * seventeen dialogs in the renderer and the one hand-maintained list of them
+ * named four, which is the failure this is shaped to avoid: a new dialog is
+ * covered the moment it exists, without anyone remembering to add it here.
+ *
+ * The `open` attribute is set by both `show()` and `showModal()`, so a
+ * non-modal prompt sitting over the chat counts too — it is still a question
+ * the user is in the middle of.
+ */
+export function isAnyDialogOpen(): boolean {
+  return document.querySelector('dialog[open]') !== null
+}

tests/e2e/dialog-shortcuts.e2e.ts

@@ -0,0 +1,87 @@
+import { $, $$, browser, expect } from '@wdio/globals'
+import { resetUserData, seedE2eViewport, writeSeedConfig } from './helpers/seed-config.ts'
+import { saveElementScreenshot } from './helpers/screenshot.ts'
+
+const PROJECT_ID = 'e2e-dialog-shortcuts'
+
+describe('dialog keyboard shortcuts', () => {
+  before(async function () {
+    this.timeout(90_000)
+    resetUserData()
+    const now = Date.now()
+    writeSeedConfig({
+      projects: [{ id: PROJECT_ID, path: process.cwd(), name: 'workspace' }],
+      activeProjectId: PROJECT_ID,
+      activeThreadId: 'thread-b',
+      [`threads:${PROJECT_ID}`]: [
+        {
+          id: 'thread-a',
+          title: 'Keep me',
+          status: 'idle',
+          messages: [
+            {
+              id: 'msg-a',
+              role: 'user',
+              content: 'Keep this conversation.',
+              toolCalls: [],
+              createdAt: now,
+            },
+          ],
+          usage: { inputTokens: 0, outputTokens: 0 },
+          createdAt: now,
+          updatedAt: now,
+        },
+        {
+          id: 'thread-b',
+          title: 'Active conversation',
+          status: 'idle',
+          messages: [
+            {
+              id: 'msg-b',
+              role: 'user',
+              content: 'Keep this active conversation too.',
+              toolCalls: [],
+              createdAt: now + 1,
+            },
+          ],
+          usage: { inputTokens: 0, outputTokens: 0 },
+          createdAt: now + 1,
+          updatedAt: now + 1,
+        },
+      ],
+    })
+    seedE2eViewport()
+    await browser.reloadSession()
+    await $('.prompt-input').waitForExist({ timeout: 60_000 })
+  })
+
+  after(() => {
+    resetUserData()
+  })
+
+  it('does not delete the active thread while Settings is open', async () => {
+    await browser.waitUntil(async () => (await $$('.chats-list .chat-row')).length === 2, {
+      timeout: 15_000,
+      timeoutMsg: 'expected two seeded chat rows',
+    })
+    await expect($('.chat-row.selected .chat-title')).toHaveText('Active conversation')
+
+    await $('[aria-label="Settings"]').click()
+    const settings = $('#settings-dialog')
+    await settings.waitForDisplayed({ timeout: 10_000 })
+
+    // Dispatch in-page: Electron may consume a physical Cmd/Ctrl+W as the native
+    // File ▸ Close accelerator before the renderer shortcut handler sees it.
+    await browser.execute(() => {
+      document.dispatchEvent(
+        new KeyboardEvent('keydown', { key: 'w', metaKey: true, ctrlKey: true, bubbles: true }),
+      )
+    })
+
+    await expect(settings).toBeDisplayed()
+    await expect($('#confirm-dialog')).not.toBeDisplayed()
+    await expect($('.chat-row.selected .chat-title')).toHaveText('Active conversation')
+    expect(await $$('.chats-list .chat-row')).toHaveLength(2)
+    await saveElementScreenshot('#settings-dialog', 'cmd-w-settings-dialog-safe.png')
+  })
+})