Skip to content

fix(shortcuts): free the dead ctrl+d/w/b swallow + registry Escape dismissal (ADR-065) - #304

Merged
aterrylu merged 3 commits into
mainfrom
terry/shortcuts-boundary-cleanup
Aug 4, 2026
Merged

fix(shortcuts): free the dead ctrl+d/w/b swallow + registry Escape dismissal (ADR-065)#304
aterrylu merged 3 commits into
mainfrom
terry/shortcuts-boundary-cleanup

Conversation

@aterrylu

@aterrylu aterrylu commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Problem

Both items were flagged-not-fixed in ADR-063 (#300), approved by Terry as one cleanup:

  1. Keys reserved for nothing. handleKeyEvent still swallowed ctrl+d/w/b on every platform — residue of the pre-dockview split shortcuts whose handlers ADR-047 deleted. On a Mac: no EOF from ctrl+d, no delete-word from ctrl+w, no tmux prefix from ctrl+b. The key vanished — no app action, no terminal action.
  2. Escape handlers dead under terminal focus. The popovers' document-level bubble-phase Escape listeners never fire while a terminal has focus (xterm stopPropagation()s handled keys) — masked by focus-follows-click, broken the moment focus returns to a terminal: ESC fed the shell's TUI while the panel stayed open.

Solution

flowchart LR
    K[Escape keydown] --> D["dispatcher (capture)"]
    D -- "stack non-empty" --> S[("escape stack<br/>LIFO")]
    S --> C["close TOPMOST panel"]
    D -- "stack empty" --> T["terminal gets \x1b"]
    P1[help overlay] -.push while open.-> S
    P2[popovers ×4] -.push while open.-> S
    P3[notification panel] -.push while open.-> S
Loading
  1. Legacy swallow deleted: ctrl+d (EOF), ctrl+w-on-Mac (delete-word), ctrl+b-on-Mac (tmux prefix) reach the shell again. mod+w/shift+W stays declined deliberately — the browser owns close-tab/close-window un-preventably; declining only stops xterm invisibly mutating the shell's line buffer as the tab dies (reasoning at the case).
  2. All Escape dismissal rides the registry's single capture-phase ui.dismiss entry (replaces help.close) backed by a LIFO escape stack: dismissible UI pushes its closer while mounted-open. Escape is reserved only while something is open, closes the topmost thing even with a terminal focused, and belongs to the terminal otherwise.

Review hardening (silent-failure-hunter pass)

  • LIFO reorder fixed: registrations are mount-scoped and read the latest closer via ref — identity churn on a bottom popover (the bell re-renders on every notification) can no longer move it above a modal on the stack.
  • A throwing closer forfeits its slot instead of permanently reserving Escape; dispatcher surfaces action errors instead of letting them vanish into the window listener.
  • IME guard: Escape during composition cancels the composition, not the panel.
  • Hunter verified: no stack leaks across all 6 registration sites, no open-but-unregistered path, Windows AltGr unaffected (xterm's third-level-shift bypasses keydown in both old and new code).

Testing

  • Unit (27 new): escape stack (LIFO, idempotent/out-of-order cleanup, throw-forfeit), key-policy matrix on both platforms (mac half via mocked platform module — isMac is a load-time constant), useClickOutside lifecycle incl. the bottom-churn reorder scenario, IME guard.
  • e2e (real Chromium): freed ctrl+d delivers \x04 to the PTY socket; Escape closes the notification panel with a terminal focused without leaking \x1b, then reaches the shell once nothing is open.
  • Headline claims mutation-verified: restoring the legacy swallow → 4 unit + 1 e2e red; restoring the bubble-phase listener → Escape e2e red.
  • Exact CI green: biome clean, make check, AUTONOMOS_INTEGRATION=1 make check (805 server), 337 dashboard unit, 18 e2e.

Risks / notes

  • Byte-level deltas fully enumerated in ADR-065, including the Linux ctrl+alt+d/b edge (now emits xterm's standard ESC-prefixed encodings — consistent with the restoration).
  • Flagged, not fixed (recorded in ADR-065): Escape inside a popover's text field still discards the draft (pre-existing); the mod-switch in handleKeyEvent remains a second hand-list (all entries fire real terminal-local actions).
  • No make hero (no visible chrome change).

🤖 Generated with Claude Code

https://claude.ai/code/session_01CwEqcpy5nXYCCqL2V6wf6k

…smissal (ADR-065)

Two corrections to the key-capture boundary, both flagged-not-fixed in
ADR-063.

(1) handleKeyEvent still swallowed ctrl+d/w/b on every platform — residue of
the pre-dockview split shortcuts whose handlers ADR-047 deleted. The actions
died; the reservations lived on: on a Mac, ctrl+d sent no EOF, ctrl+w deleted
no word, ctrl+b was no tmux prefix. Freed. mod+w (and shift+W) stays declined
deliberately: the browser owns close-tab/close-window un-preventably, so
declining only stops xterm from invisibly mutating the shell's line buffer as
the tab dies.

(2) The popovers' Escape handlers (useClickOutside x4, NotificationPanel's
duplicate) were document-level bubble-phase listeners — dead whenever a
terminal had focus (xterm stopPropagation()s handled keys), masked by
focus-follows-click. Escape dismissal now rides the registry's single
capture-phase ui.dismiss entry backed by a LIFO escape stack: dismissible UI
pushes a closer while mounted-open; Escape is reserved only while something is
open and closes the topmost thing, even with a terminal focused.

Review hardening: mount-scoped stack registration reading the latest closer
via ref (identity churn on a bottom popover must not steal the top of the
stack); a throwing closer forfeits its slot instead of permanently reserving
Escape; IME composition guard in the dispatcher.

Tests: escape stack (LIFO, idempotent/out-of-order cleanup, throw-forfeit);
key-policy matrix on BOTH platforms (mac half via mocked platform module);
useClickOutside registration lifecycle incl. the reorder scenario; e2e: freed
ctrl+d delivers \x04 to the PTY socket, Escape closes the notification panel
with a terminal focused without leaking \x1b, then reaches the shell once
nothing is open. Headline claims mutation-verified (reverting each fix turns
its tests red).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwEqcpy5nXYCCqL2V6wf6k
Comment thread packages/dashboard/src/shortcuts/ShortcutHelpOverlay.tsx

@nox-0x nox-0x left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — the freed key policy and the escape-stack migration both hold up under audit; only one doc-accuracy nit, posted inline as a follow-up.

What I verified

Key policy (handleKeyEvent) — I traced every chord the deleted ctrlKey block used to eat, on both platforms:

  • non-Mac Ctrl+D: no longer matches isReservedChord (mod+d is unregistered), passes hasPrimaryModifier, falls through the switch to default: true → xterm encodes \x04 and preventDefault()s, which also suppresses Chromium bookmark. Matches the new e2e assertion.
  • non-Mac Ctrl+B: still declined, but now via the registry (mod+b = sidebar) rather than the hand-list — so it stays synchronized if that entry ever moves.
  • Mac ⌘D: previously hit case "d": return false, now falls to default: true. Net behavior is unchanged — xterm evaluateKeyboardEvent produces no key for a bare Cmd chord, so nothing reaches the PTY either way. Not a regression, just worth knowing the byte-level delta is genuinely zero there.
  • case "w" / case "W": covers the Ctrl+Shift+W path the old key.toLowerCase() used to catch, and additionally declines Mac ⌘⇧W (previously accepted). That widening is deliberate and matches the comment.

Escape stack — the correctness of "close does not pop, the unmount does" depends entirely on every registration site unmounting when closed. I checked all six: SettingsPanel (StatusBar settingsOpen &&), UsagePanel / SetupPanel / ErrorPanel (panel === ... in UsageStatusBarItem), CodexUsagePanel (open && in both branches), NotificationPanel (open && in NotificationBell), HelpDialog (early-return in ShortcutHelpOverlay). All conditional — no path where a closer runs and the component survives, which would otherwise reserve Escape permanently and eat it from every terminal. The UsageStatusBarItem branch-switch cases (data goes error/needsSetup while a panel is open) also unmount cleanly rather than stranding a registration.

The lastIndexOf + removed guard in pushEscapeCloser handles the duplicate-callback and out-of-order-unmount cases, and the closers are fresh arrows per mount (except closeShortcutHelp, which is a stable store action but can only ever have one live registration). No leak path found. Grep confirms no surviving document-level Escape listener anywhere in the dashboard, so nothing is left silently dead behind the new capture-phase stopPropagation().

Dispatcher — the isComposing guard and the try/catch around shortcut.run() are both real fixes, not defensive noise: Escape is the first bare chord in the registry, so a mid-composition Escape genuinely reached the dispatcher before, and a window listener is the one place a throw has no React boundary to land in.

Test coverage is proportionate: the mac half via a mocked platform module is the right call given isMac is a load-time constant, the bottom-popover identity-churn test pins the exact reorder bug the ref indirection exists to prevent, and the e2e covers the two claims that unit tests structurally cannot (a real \x04 on the PTY socket; no \x1b leak with a terminal focused).

Follow-up (non-blocking)ShortcutHelpOverlay.tsx still documents the old help.close id, and its header comment now asserts Escape is reserved "only while this overlay is open", which is the inverse of the new hasEscapeCloser gate. Details inline.

aterrylu and others added 2 commits August 3, 2026 23:04
…retired help.close entry

Review catch: three comments still claimed Escape is reserved only while the
overlay is open — the real gate is the escape stack (any dismissible UI).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwEqcpy5nXYCCqL2V6wf6k
@aterrylu
aterrylu merged commit b230d72 into main Aug 4, 2026
5 checks passed
@aterrylu
aterrylu deleted the terry/shortcuts-boundary-cleanup branch August 4, 2026 06:14
aterrylu added a commit that referenced this pull request Aug 4, 2026
…ges (ADR-066) (#305)

* feat(shortcuts): hold-mod pane-digit badges — the hint IS the chord

Hold the primary modifier ~350ms and each open tab shows the digit that
focuses it (tmux display-panes for mod+digit). Badges derive from the same
orderedPaneIds visual-order walk the shortcuts execute, so they cannot
disagree with what pressing the number does; mod+9=last is honored (a last
pane beyond position 8 shows 9, unreachable middles show nothing).

The hold gate keeps quick chords (⌘C/⌘V/⌘1) from flashing badges, and
stuck-modifier protection clears on window blur / visibilitychange — ⌘Tab
away is browser-reserved, so its keyup is never seen.

Also fixes CLAUDE.md's shortcut section citing the registry as ADR-064
(it is ADR-063; 064 is the gateway ack ADR) — flagged after #304.

Tests: digitForPane mapping (incl. >9-pane edges), useModKeyHold with fake
timers (hold gate, auto-repeat non-restart, keyup/blur/missed-release
clears, disabled + unmount leave no state), e2e: badges appear on hold in
visual order, pressing the shown digit switches panes, release clears.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwEqcpy5nXYCCqL2V6wf6k

* feat(shortcuts)!: mod+digit targets the SIDEBAR agent list; badges on agent rows (ADR-066)

Terry's correction on the badge PR surfaced that the digit semantics were
wrong all along: digits should switch between AGENTS (the sidebar rows,
CMUX-tab style), not open pane positions — far more useful in the default
solo-pane workflow where only one pane is open.

mod+1..9 now switches to the Nth agent row in RENDERED order. The Sidebar
publishes its rendered order to the store (flat: pinned then unpinned;
hierarchy: DFS skipping collapsed subtrees and non-clickable stopped rows;
degraded: empty) — hierarchy order depends on component-local state no
registry action could recompute, so publishing from the render memo chain is
what makes WYSIWYG structural. The action mirrors a row click exactly
(switchPane + focusTerminal + clear unread). Hold-badges render on the agent
rows from the same published list.

The pane-position machinery built for the old semantics (orderedPaneIds,
dockviewApi handle, drift tripwire, tab badges) is deleted — nothing consumes
it. mod+9 is positional (agent 9), dropping the last-pane tab idiom.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwEqcpy5nXYCCqL2V6wf6k

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants