From bcdccd14c49a7f86d086ecb2ede57b41a84ef01b Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 6 Oct 2026 15:46:49 +0200 Subject: [PATCH] fix(shortcuts): Ctrl+W no longer closes a session Close Session was bound to Ctrl+W by default. Ctrl+W is delete-word in every shell, readline prompt and agent CLI, so muscle memory killed the session (its tmux pane and CLI, with no confirm) mid-sentence, and with the split pane open it was not even the pane being typed in. Close Session now has no default key: the capture-phase handler lets Ctrl+W through and xterm sends ^W to whichever pane is focused. The action stays in the registry and can be bound in App Settings -> Shortcuts; the shortcut overlay shows it as not bound. The Help modal, CLAUDE.md, the split and tile-grid specs and three wiki pages stop advertising Ctrl+W as kill. Owner decision (tile-grid decision 5). Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 4 +- docs/architecture-invariants.md | 2 +- docs/split-pane-sessions-plan.md | 3 +- docs/tile-grid-plan.md | 35 +++++----- docs/wiki/Keyboard-Shortcuts.md | 5 +- docs/wiki/Quick-Start.md | 2 +- docs/wiki/The-Dashboard.md | 2 +- src/web/public/app.js | 12 +++- src/web/public/index.html | 1 - src/web/public/styles.css | 6 ++ test/ctrl-w-never-closes.test.ts | 104 ++++++++++++++++++++++++++++ test/focused-pane-shortcuts.test.ts | 8 +-- test/help-modal-shortcuts.test.ts | 4 +- 13 files changed, 155 insertions(+), 33 deletions(-) create mode 100644 test/ctrl-w-never-closes.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index e0972b81..ccc5232a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -276,7 +276,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Full-scrollback replay**: `GET /api/sessions/:id/terminal?full=1` returns the whole tmux scrollback ALONE (`source='mux-full-history'`), superseding the byte buffer. First load of each non-shell TUI session requests it (`_fullHistoryLoaded`); Shell selection and drop recovery use a bounded 1 MiB `?tail=`, and a Shell scroll-to-top pulls a bounded `?full=1&tail=` window (a window no longer than the browser's buffer is skipped before the downgrade guard, so it never marks the session exhausted); the unbounded pull stays behind **Load full history**. A Shell split-pane Pane B has its own copy of the bounded pull against its own xterm (`TerminalTile._pullHistory`, terminal-tile.js); keep the two in step. → [architecture-invariants#split-pane-sessions](docs/architecture-invariants.md#split-pane-sessions) ⚠️ The capture ends with a RELATIVE cursor move back to the pane's caret (never `CUP`), so no line-deleting transform may run over it; those skips key on `isFullCapture`, never on `?full=1` alone. ⚠️ A re-pull must never shrink the buffer (`_replayWouldShrinkBuffer()`). ⚠️ `captureCols`/`captureRows` are absent when no frame was positioned: test `Number.isFinite`, never truthiness. ⚠️ A frame dropped at the 128 KiB render cap MUST be recovered, and the recovery verifies itself: `_scheduleDroppedOutputRecovery` re-arms (bounded by `DROP_RECOVERY_MAX_ATTEMPTS`) while `_onSessionNeedsRefresh` reports no repaint, but never after a capture-fetch `'deadline'`. → [architecture-invariants#full-scrollback-replay](docs/architecture-invariants.md#full-scrollback-replay) -**Split-pane sessions** (`showSplitButton`, header button, default OFF, desktop-only, per-device): a second live session ("Pane B") beside the active one, in a `TerminalTile` (terminal-tile.js; the picker, divider and auto-collapse stay in terminal-split.js) with its own xterm + WebSocket, resizable via a draggable divider. Pane B reconnects after a drop, sends input through the exactly-once queue over its own socket (`_registerInputSocket`), has clickable paths and image paste, and owns its geometry (no 40x10 floor, `zc` columns adopted, font changes call `tile.fit()`). ⚠️ Only typed input enters that persisted queue: xterm's query replies are dropped and focus/mouse reports go out ephemeral. ⚠️ App-level terminal actions find their pane through `_focusedPane()` (the terminal focused last), never `this.terminal`; Ctrl+W deliberately still closes `activeSessionId`. Still plainer than the primary pane (no local-echo overlay, CJK IME or touch handlers) and NOT persisted across reloads. The planned tile grid reuses `TerminalTile` (`docs/tile-grid-plan.md`). → [architecture-invariants#split-pane-sessions](docs/architecture-invariants.md#split-pane-sessions) +**Split-pane sessions** (`showSplitButton`, header button, default OFF, desktop-only, per-device): a second live session ("Pane B") beside the active one, in a `TerminalTile` (terminal-tile.js; the picker, divider and auto-collapse stay in terminal-split.js) with its own xterm + WebSocket, resizable via a draggable divider. Pane B reconnects after a drop, sends input through the exactly-once queue over its own socket (`_registerInputSocket`), has clickable paths and image paste, and owns its geometry (no 40x10 floor, `zc` columns adopted, font changes call `tile.fit()`). ⚠️ Only typed input enters that persisted queue: xterm's query replies are dropped and focus/mouse reports go out ephemeral. ⚠️ App-level terminal actions find their pane through `_focusedPane()` (the terminal focused last), never `this.terminal`. Still plainer than the primary pane (no local-echo overlay, CJK IME or touch handlers) and NOT persisted across reloads. The planned tile grid reuses `TerminalTile` (`docs/tile-grid-plan.md`). → [architecture-invariants#split-pane-sessions](docs/architecture-invariants.md#split-pane-sessions) **Terminal touch gestures: link taps and text selection**: on touch devices xterm's linkifier and SelectionService never see the gesture, so both are driven explicitly (terminal-ui.js). ⚠️ A tap activates the link under it through the SAME provider as the hover linkifier (`_terminalLinkAtPoint`), synchronously inside `touchend` (keeps the user gesture `window.open` needs) and BEFORE any mouse report; the caret's logical line (`_tapIsOnCaretLine`) and TUI-owned rows (`_isActionableMobileTerminalTap`) keep their meaning. ⚠️ Gate on the caret line, never on tap intent (a shell calls every tap `'input'`). ⚠️ Long-press selects via xterm's public `select()`; keep the three guards: suppress the compat mouse pair after `touchend`, the bounded focus guard + `contextmenu` suppression for the platform long-press, and no closing `terminal.focus()` on phones. Tests: `test/terminal-touch-tap.test.ts`. → [architecture-invariants#terminal-touch-gestures-link-taps-and-text-selection](docs/architecture-invariants.md#terminal-touch-gestures-link-taps-and-text-selection) @@ -389,7 +389,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **Respawn presets**: `solo-work` (3s/60min), `subagent-workflow` (45s/240min), `team-lead` (90s/480min), `ralph-todo` (8s/480min), `overnight-autonomous` (10s/480min). -**Keyboard shortcuts**: Escape (close), Ctrl+? (shortcut overlay), Ctrl/Cmd/Alt+K (session palette), Ctrl+W (kill), Ctrl+Tab (next), Alt+[/] (prev/next tab), Alt+1-9 (switch tab), Ctrl+Shift+{/} (move tab left/right), Shift+Enter or Ctrl+Enter (newline), Ctrl+C (copy selection, else interrupt) / Ctrl+Shift+C (copy, never interrupts), Ctrl+L (clear), Ctrl+Shift+R (restore size), Ctrl+Shift+V (voice input), Ctrl/Cmd +/- (font), Shift+Wheel (local scrollback when mouse passthrough is active), Shift+drag (start a selection in a stripped-DECSET pane, where xterm's own Shift branch is unreachable and a Shift+drag used to select nothing; `_installShiftDragSelection`), right-click (copy the selection, the mintty/PuTTY convention, since xterm paints into a canvas and the native menu has no Copy for it; with nothing selected the native menu is left alone). Rebindable via the registry. +**Keyboard shortcuts**: Escape (close), Ctrl+? (shortcut overlay), Ctrl/Cmd/Alt+K (session palette), Ctrl+Tab (next), Alt+[/] (prev/next tab), Alt+1-9 (switch tab), Ctrl+Shift+{/} (move tab left/right), Shift+Enter or Ctrl+Enter (newline), Ctrl+C (copy selection, else interrupt) / Ctrl+Shift+C (copy, never interrupts), Ctrl+L (clear), Ctrl+Shift+R (restore size), Ctrl+Shift+V (voice input), Ctrl/Cmd +/- (font), Shift+Wheel (local scrollback when mouse passthrough is active), Shift+drag (start a selection in a stripped-DECSET pane, where xterm's own Shift branch is unreachable and a Shift+drag used to select nothing; `_installShiftDragSelection`), right-click (copy the selection, the mintty/PuTTY convention, since xterm paints into a canvas and the native menu has no Copy for it; with nothing selected the native menu is left alone). Rebindable via the registry. ⚠️ **Ctrl+W is deliberately NOT bound** (owner decision): it is delete-word in every shell and agent CLI, and as Close Session it killed sessions with no confirm; `close-session` keeps `bindings: []` and stays bindable (`test/ctrl-w-never-closes.test.ts`). ### Security diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index e300944f..533ba908 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -793,7 +793,7 @@ Further detail: with many sessions the horizontal strip stops being scannable, w ### Split-pane sessions -**Split-pane sessions** (`showSplitButton`, header button, default OFF, per-device like `showFileViewerButton` — not in `SettingsUpdateSchema`, `displayKeys` in settings-ui.js): shows two live sessions side-by-side in one Codeman window. Pane A is the untouched, existing singleton terminal (`this.terminal`/`this._ws` in terminal-ui.js); Pane B is an independent `TerminalTile` (terminal-tile.js, constructed by the orchestration in terminal-split.js) with its own xterm instance and its own `/ws/sessions/:id/terminal` WebSocket, whose `cid` is the tab's identity with a `:tile` suffix, so it can never supersede the primary pane's socket (the registry supersedes by cid per session). ⚠️ **Pane B is deliberately plainer than Pane A** — no local-echo overlay, no CJK IME, no touch/mobile handlers, no keyboard accessory bar — since this is a desktop-only feature (a split view needs a wide viewport) and those features exist for mobile/touch input; `.btn-split` is hard-hidden below 1180px regardless of the setting by the `@media (max-width: 1179px)` rule in styles.css (mobile.css only carries a comment pointing at it: that file loads up to 1023px, so it cannot cover the 1024-1179px tablet range the feature also needs to stay off), and the per-device setting means turning it on at a desk can never sync it onto a phone in the first place. No persistence: closing the browser tab or reloading always returns to the normal single-pane view; there is no localStorage key for split state. ⚠️ Splitting a session against itself is disallowed (the picker excludes the active session), **as is splitting against a popped-out (detached) session** — `buildSplitPickerSessions()` excludes `detachedSessions` because a detached session's own window is already claiming its PTY size, and `MAX_WS_PER_SESSION` needs no change since Pane A/B are always two different sessions. ⚠️ Either pane's session ending (deleted locally or from another client) auto-collapses the split — Pane A's session ending promotes Pane B to the new single pane via `selectSession(id, { auto: true })` (an app-driven selection, so it must not spend the session's idle alert — see the Approvals Inbox note above), never by trying to hot-swap the lightweight `TerminalTile` object into the primary singleton state. ⚠️ Pane B refits on every window/sidebar/tab-rail resize via the SAME trailing-edge `ResizeObserver` callback that resizes Pane A (`throttledResize` in terminal-ui.js) — it only ever measured Pane A's own container, so without the explicit `this._forEachTile?.((tile) => tile.fit())` call there Pane B silently kept its stale PTY size through every resize that did not happen to be a divider drag. ⚠️ A dropped WebSocket writes a `[disconnected, reconnecting…]` marker into Pane B and reconnects on the primary pane's backoff ladder (`CodemanWsReconnect` plus jitter; the attempt count resets only on a successful open), then refreshes the buffer to close the output gap. ⚠️ The open handler clears `_wsClosed`/`_markerOwed` BEFORE that refresh, or the refresh re-owes the marker and stamps it under a healthy pane. ⚠️ 4003/4004/4009/4010 stop the pane for good, report once through `onExit(code)` and write a marker saying why (`CodemanWsReconnect` alone would retry 4003). ⚠️ A replacement socket detaches the old one first and every handler ignores a socket that is no longer `this.ws`, so a late 4010 from a superseded socket cannot stop its successor; `destroy()` cancels a pending reconnect. Related but distinct: `detachSession()` already opens one session in a separate OS-level browser window (`isSoloWindow`) — that is prior art for "two sessions visible at once" but not for one window with a draggable in-page divider, which is what this feature adds. ⚠️ Pane B installs its own `attachCustomKeyEventHandler` gating the same app-level chords Pane A's own handler gates (command palette, Alt+1-9/[/] tab nav, Alt+B sidebar toggle, Ctrl+Z suspend, Shift/Ctrl+Enter newline, smart-copy Ctrl+C) — without it the document capture-phase handler's `preventDefault()` (which never stops xterm) let each chord ALSO write its raw byte/escape sequence into Pane B's live PTY on top of whatever the app action did to Pane A (COD-153). Ctrl+Z is swallowed unless Pane B's own session is `mode === 'shell'`, mirroring terminal-ui.js's reasoning: in a plain shell it is the user's own job-control tool, everywhere else it silently suspends an unattended agent loop. Shift/Ctrl+Enter POSTs to `/api/sessions/:id/send-key` (`{key:'S-Enter'|'C-Enter'}`, typed by tmux `send-keys -H`: Ctrl+Enter is always a real 0x0a, Shift+Enter is the CLI's declared `capabilities.newline` chord, also 0x0a unless the CLI declares another; sent on keydown only, with the keypress and keyup swallowed too, see the keypress rule under Command palette and shortcut registry) targeting THIS pane's own `sessionId` rather than the primary pane's `activeSessionId` — without it xterm's plain `\r` would submit an incomplete prompt instead of adding a line to it. Smart-copy Ctrl+C/Ctrl+Shift+C is re-implemented against `this.terminal` (Pane B's own) rather than reusing `app.copyTerminalSelection()`, which reads Pane A's terminal and would copy the wrong pane's selection; Ctrl+Shift+C never falls through even with nothing to copy, mirroring terminal-ui.js's own `ev.shiftKey` branch. ⚠️ **This is a UX-parity fix, not an interrupt-safety one** — verified live in a real browser: xterm's `evaluateKeyboardEvent` routes a shifted ctrl-letter into a branch that assigns `c.key` only for two special cases (`_`→US, `@`→NUL), so it emits no data for Ctrl+Shift+C at all regardless of any application gate; a synthetic keydown with the gate removed produces zero WS frames, proving no accidental interrupt reaches the PTY either way. What gating the whole copy block on `hasSelection()` (an earlier draft) actually cost: with no selection, a selection-less Ctrl+Shift+C fell straight to `return true`, silently ceding the keystroke to the BROWSER's own handling (e.g. Chrome's Inspect-Element binding) with no feedback and no copy attempt — Pane A always intercepts it. Ctrl+V goes through the primary pane's paste trap aimed at Pane B (`_handleImagePaste({ terminal, sessionId })`), so a pasted image uploads to Pane B's session. ⚠️ `buildSplitPickerSessions()` also excludes any session with `pid === null` (an exited CLI, a crash-looped session whose breaker tripped, a restore that never re-attached): Pane B has no equivalent of `selectSession()`'s auto re-attach POST, so a pane opened onto one has nothing reading its tmux pane — no `terminal` events ever arrive, and `Session.write()` silently drops every keystroke with no ack either way, so the loss is invisible behind a socket that reports healthy. Pane B's keystrokes go through the exactly-once queue (`_sendInputAsync`) over its own socket, registered in the app's input-socket map (`_registerInputSocket` / `_inputSocketFor`), so they are seq-tagged, ACKed (`{t:'ia'}` is routed by the RECEIVING socket's session, since the frame names none), redelivered after a drop, and the ACK clears the idle alert. ⚠️ What xterm generates never enters that persisted queue: query replies are dropped (`shouldSuppressTerminalQueryResponse`, as the primary pane drops them) and focus/mouse reports go through `_sendInputEphemeral`, or a reload would replay them as typed text. ⚠️ A SHELL Pane B pulls scrollback itself when the wheel goes up at the top of its buffer (`_maybeLoadMoreHistory`/`_pullHistory`): tmux repaints a burst of output instead of scrolling it, so Pane B's own xterm holds about one screen of scrollback while tmux holds every line, and it loaded history exactly once at connect and never again. It is the same bounded pull as Pane A's (`?full=1&tail=TERMINAL_TAIL_SIZE`, no rewrite when the window holds no more rows than the pane already has or the pane is at its `scrollback + rows` cap, and a 60 s back-off instead of 4 s when that skipped window was truncated or the pane is full, since each ask costs the server a whole-history `capture-pane`), against Pane B's OWN terminal rather than `app.terminal`, so it cannot share `_maybeRefetchFullHistory`. The wheel listener is capture-phase because xterm `stopPropagation()`s the events it consumes; the alternate-screen skip (nano, vim, less) only matters for a direct-PTY shell, since under tmux the browser xterm never enters the alternate buffer; skipped too for a detached session (mirrors `_sendResize()`'s own check and app.js's `_maybeRefetchFullHistory`), since its own window already owns its PTY size and scrollback. Live frames arriving from the response onward, a `{t:'c'}` clear frame included, are held with their arrival time (`_liveQueue`, opened right after `await fetch(...)` beside `capturedAt`; a frame from before it is replaced by the capture or written unchanged, so the pane keeps painting during the round trip) and replayed in order only if they arrived after the capture (the response's arrival stands in for the capture instant, as in `_finishBufferLoad`, so a frame inside that one round trip can be lost or doubled); the request uses the primary pane's budget (`CodemanFetchDeadline.terminalFetchDeadlineMs({ full: true })`, 45 s, 10 s only if that helper is absent) and the body read gets 10 s once the headers land, because from then on the pull holds the pane's live output. The request phase holds no live output, but it does hold the single-flight flag, so a coalesced `{t:'r'}` refresh and a marker owed by a close (below) wait for the response, at worst for that whole budget (accepted: a Codeman restart resets an in-flight request along with the socket, so that pull fails at once and stamps the marker). ⚠️ The "disconnected" marker must be the LAST thing on screen. A replay's own `\x1bc` would otherwise wipe a marker written before the pull and paint a fresh, current-looking history while `onData` keeps silently dropping every keystroke on the dead socket (a Codeman restart drops the socket while the tmux session, and so the HTTP pull, still succeeds), so `_pullHistory()` re-stamps it after the live-frame flush. A close DURING any load (a pull, a refresh; `connect()` awaits the initial load before it creates the socket, so no close lands in that one) writes nothing: `_onSocketClosed()` sets `_markerOwed` while `_bufferLoading` is true, since written there it would sit above the held frames the pull flushes after a skip, a downgrade, a failed fetch or the deadline, above a refresh's replay, or land mid-way through a chunked replay. Each load settles the marker in its OWN `finally` (`_stampMarkerIfOwed()`), after the queue flush, EXCEPT when a trailing refresh is pending: that refresh's `clear()` is synchronous while xterm parses a `write()` on a later tick, so a marker stamped just before it lands in the freshly cleared buffer ABOVE the refresh's replay (a second, stale copy; the default fake terminal in `test/terminal-tile-unit.test.ts` writes synchronously and cannot show it, so an async-parse fake there pins it). The marker stays owed instead, and the trailing refresh, which re-owes it on a closed socket anyway, writes the one copy below its own replay. Anything that wipes the terminal on a closed socket (a replay's `\x1bc`, a refresh's `clear()`) sets `_markerOwed` too, so the marker is rewritten whether or not the close landed during the load. Tracked via `_wsClosed`/`_markerOwed` rather than routed through `_onLiveOutput()`, since a close landing before the response is stamped before the cutoff and would be dropped with the rest of the pre-capture queue. There is no "Load full history" banner in Pane B, so a shell history past that 1 MiB window stays out of reach there. Non-shell Pane B is unchanged: it already loads `full=1`, and its history is out of scope for this pull (codex and Claude's inline renderer do grow tmux history; this just isn't how they recover it). ⚠️ App-level terminal actions find their pane through `_focusedPane()`, the terminal focused LAST (not `document.activeElement`, which the mic or a header button takes): Ctrl+L, Ctrl+Shift+R, voice and image paste act on Pane B while it holds the keyboard. Ctrl+W deliberately still closes `activeSessionId`, since it kills with no confirm (`docs/tile-grid-plan.md`, decision 5). ⚠️ Geometry: `TerminalTile.fit()` sends the size the xterm actually holds, unfloored (the divider's 20% clamp leaves about 28 columns), skips an unchanged size, re-sends on every fresh socket, and adopts the PTY's column count from `{t:'zc'}`; the font, family and weight setters call `tile.fit()`, never a local refit alone (#464). Design: `docs/split-pane-sessions-plan.md`; the tile class: `docs/tile-grid-plan.md`. +**Split-pane sessions** (`showSplitButton`, header button, default OFF, per-device like `showFileViewerButton` — not in `SettingsUpdateSchema`, `displayKeys` in settings-ui.js): shows two live sessions side-by-side in one Codeman window. Pane A is the untouched, existing singleton terminal (`this.terminal`/`this._ws` in terminal-ui.js); Pane B is an independent `TerminalTile` (terminal-tile.js, constructed by the orchestration in terminal-split.js) with its own xterm instance and its own `/ws/sessions/:id/terminal` WebSocket, whose `cid` is the tab's identity with a `:tile` suffix, so it can never supersede the primary pane's socket (the registry supersedes by cid per session). ⚠️ **Pane B is deliberately plainer than Pane A** — no local-echo overlay, no CJK IME, no touch/mobile handlers, no keyboard accessory bar — since this is a desktop-only feature (a split view needs a wide viewport) and those features exist for mobile/touch input; `.btn-split` is hard-hidden below 1180px regardless of the setting by the `@media (max-width: 1179px)` rule in styles.css (mobile.css only carries a comment pointing at it: that file loads up to 1023px, so it cannot cover the 1024-1179px tablet range the feature also needs to stay off), and the per-device setting means turning it on at a desk can never sync it onto a phone in the first place. No persistence: closing the browser tab or reloading always returns to the normal single-pane view; there is no localStorage key for split state. ⚠️ Splitting a session against itself is disallowed (the picker excludes the active session), **as is splitting against a popped-out (detached) session** — `buildSplitPickerSessions()` excludes `detachedSessions` because a detached session's own window is already claiming its PTY size, and `MAX_WS_PER_SESSION` needs no change since Pane A/B are always two different sessions. ⚠️ Either pane's session ending (deleted locally or from another client) auto-collapses the split — Pane A's session ending promotes Pane B to the new single pane via `selectSession(id, { auto: true })` (an app-driven selection, so it must not spend the session's idle alert — see the Approvals Inbox note above), never by trying to hot-swap the lightweight `TerminalTile` object into the primary singleton state. ⚠️ Pane B refits on every window/sidebar/tab-rail resize via the SAME trailing-edge `ResizeObserver` callback that resizes Pane A (`throttledResize` in terminal-ui.js) — it only ever measured Pane A's own container, so without the explicit `this._forEachTile?.((tile) => tile.fit())` call there Pane B silently kept its stale PTY size through every resize that did not happen to be a divider drag. ⚠️ A dropped WebSocket writes a `[disconnected, reconnecting…]` marker into Pane B and reconnects on the primary pane's backoff ladder (`CodemanWsReconnect` plus jitter; the attempt count resets only on a successful open), then refreshes the buffer to close the output gap. ⚠️ The open handler clears `_wsClosed`/`_markerOwed` BEFORE that refresh, or the refresh re-owes the marker and stamps it under a healthy pane. ⚠️ 4003/4004/4009/4010 stop the pane for good, report once through `onExit(code)` and write a marker saying why (`CodemanWsReconnect` alone would retry 4003). ⚠️ A replacement socket detaches the old one first and every handler ignores a socket that is no longer `this.ws`, so a late 4010 from a superseded socket cannot stop its successor; `destroy()` cancels a pending reconnect. Related but distinct: `detachSession()` already opens one session in a separate OS-level browser window (`isSoloWindow`) — that is prior art for "two sessions visible at once" but not for one window with a draggable in-page divider, which is what this feature adds. ⚠️ Pane B installs its own `attachCustomKeyEventHandler` gating the same app-level chords Pane A's own handler gates (command palette, Alt+1-9/[/] tab nav, Alt+B sidebar toggle, Ctrl+Z suspend, Shift/Ctrl+Enter newline, smart-copy Ctrl+C) — without it the document capture-phase handler's `preventDefault()` (which never stops xterm) let each chord ALSO write its raw byte/escape sequence into Pane B's live PTY on top of whatever the app action did to Pane A (COD-153). Ctrl+Z is swallowed unless Pane B's own session is `mode === 'shell'`, mirroring terminal-ui.js's reasoning: in a plain shell it is the user's own job-control tool, everywhere else it silently suspends an unattended agent loop. Shift/Ctrl+Enter POSTs to `/api/sessions/:id/send-key` (`{key:'S-Enter'|'C-Enter'}`, typed by tmux `send-keys -H`: Ctrl+Enter is always a real 0x0a, Shift+Enter is the CLI's declared `capabilities.newline` chord, also 0x0a unless the CLI declares another; sent on keydown only, with the keypress and keyup swallowed too, see the keypress rule under Command palette and shortcut registry) targeting THIS pane's own `sessionId` rather than the primary pane's `activeSessionId` — without it xterm's plain `\r` would submit an incomplete prompt instead of adding a line to it. Smart-copy Ctrl+C/Ctrl+Shift+C is re-implemented against `this.terminal` (Pane B's own) rather than reusing `app.copyTerminalSelection()`, which reads Pane A's terminal and would copy the wrong pane's selection; Ctrl+Shift+C never falls through even with nothing to copy, mirroring terminal-ui.js's own `ev.shiftKey` branch. ⚠️ **This is a UX-parity fix, not an interrupt-safety one** — verified live in a real browser: xterm's `evaluateKeyboardEvent` routes a shifted ctrl-letter into a branch that assigns `c.key` only for two special cases (`_`→US, `@`→NUL), so it emits no data for Ctrl+Shift+C at all regardless of any application gate; a synthetic keydown with the gate removed produces zero WS frames, proving no accidental interrupt reaches the PTY either way. What gating the whole copy block on `hasSelection()` (an earlier draft) actually cost: with no selection, a selection-less Ctrl+Shift+C fell straight to `return true`, silently ceding the keystroke to the BROWSER's own handling (e.g. Chrome's Inspect-Element binding) with no feedback and no copy attempt — Pane A always intercepts it. Ctrl+V goes through the primary pane's paste trap aimed at Pane B (`_handleImagePaste({ terminal, sessionId })`), so a pasted image uploads to Pane B's session. ⚠️ `buildSplitPickerSessions()` also excludes any session with `pid === null` (an exited CLI, a crash-looped session whose breaker tripped, a restore that never re-attached): Pane B has no equivalent of `selectSession()`'s auto re-attach POST, so a pane opened onto one has nothing reading its tmux pane — no `terminal` events ever arrive, and `Session.write()` silently drops every keystroke with no ack either way, so the loss is invisible behind a socket that reports healthy. Pane B's keystrokes go through the exactly-once queue (`_sendInputAsync`) over its own socket, registered in the app's input-socket map (`_registerInputSocket` / `_inputSocketFor`), so they are seq-tagged, ACKed (`{t:'ia'}` is routed by the RECEIVING socket's session, since the frame names none), redelivered after a drop, and the ACK clears the idle alert. ⚠️ What xterm generates never enters that persisted queue: query replies are dropped (`shouldSuppressTerminalQueryResponse`, as the primary pane drops them) and focus/mouse reports go through `_sendInputEphemeral`, or a reload would replay them as typed text. ⚠️ A SHELL Pane B pulls scrollback itself when the wheel goes up at the top of its buffer (`_maybeLoadMoreHistory`/`_pullHistory`): tmux repaints a burst of output instead of scrolling it, so Pane B's own xterm holds about one screen of scrollback while tmux holds every line, and it loaded history exactly once at connect and never again. It is the same bounded pull as Pane A's (`?full=1&tail=TERMINAL_TAIL_SIZE`, no rewrite when the window holds no more rows than the pane already has or the pane is at its `scrollback + rows` cap, and a 60 s back-off instead of 4 s when that skipped window was truncated or the pane is full, since each ask costs the server a whole-history `capture-pane`), against Pane B's OWN terminal rather than `app.terminal`, so it cannot share `_maybeRefetchFullHistory`. The wheel listener is capture-phase because xterm `stopPropagation()`s the events it consumes; the alternate-screen skip (nano, vim, less) only matters for a direct-PTY shell, since under tmux the browser xterm never enters the alternate buffer; skipped too for a detached session (mirrors `_sendResize()`'s own check and app.js's `_maybeRefetchFullHistory`), since its own window already owns its PTY size and scrollback. Live frames arriving from the response onward, a `{t:'c'}` clear frame included, are held with their arrival time (`_liveQueue`, opened right after `await fetch(...)` beside `capturedAt`; a frame from before it is replaced by the capture or written unchanged, so the pane keeps painting during the round trip) and replayed in order only if they arrived after the capture (the response's arrival stands in for the capture instant, as in `_finishBufferLoad`, so a frame inside that one round trip can be lost or doubled); the request uses the primary pane's budget (`CodemanFetchDeadline.terminalFetchDeadlineMs({ full: true })`, 45 s, 10 s only if that helper is absent) and the body read gets 10 s once the headers land, because from then on the pull holds the pane's live output. The request phase holds no live output, but it does hold the single-flight flag, so a coalesced `{t:'r'}` refresh and a marker owed by a close (below) wait for the response, at worst for that whole budget (accepted: a Codeman restart resets an in-flight request along with the socket, so that pull fails at once and stamps the marker). ⚠️ The "disconnected" marker must be the LAST thing on screen. A replay's own `\x1bc` would otherwise wipe a marker written before the pull and paint a fresh, current-looking history while `onData` keeps silently dropping every keystroke on the dead socket (a Codeman restart drops the socket while the tmux session, and so the HTTP pull, still succeeds), so `_pullHistory()` re-stamps it after the live-frame flush. A close DURING any load (a pull, a refresh; `connect()` awaits the initial load before it creates the socket, so no close lands in that one) writes nothing: `_onSocketClosed()` sets `_markerOwed` while `_bufferLoading` is true, since written there it would sit above the held frames the pull flushes after a skip, a downgrade, a failed fetch or the deadline, above a refresh's replay, or land mid-way through a chunked replay. Each load settles the marker in its OWN `finally` (`_stampMarkerIfOwed()`), after the queue flush, EXCEPT when a trailing refresh is pending: that refresh's `clear()` is synchronous while xterm parses a `write()` on a later tick, so a marker stamped just before it lands in the freshly cleared buffer ABOVE the refresh's replay (a second, stale copy; the default fake terminal in `test/terminal-tile-unit.test.ts` writes synchronously and cannot show it, so an async-parse fake there pins it). The marker stays owed instead, and the trailing refresh, which re-owes it on a closed socket anyway, writes the one copy below its own replay. Anything that wipes the terminal on a closed socket (a replay's `\x1bc`, a refresh's `clear()`) sets `_markerOwed` too, so the marker is rewritten whether or not the close landed during the load. Tracked via `_wsClosed`/`_markerOwed` rather than routed through `_onLiveOutput()`, since a close landing before the response is stamped before the cutoff and would be dropped with the rest of the pre-capture queue. There is no "Load full history" banner in Pane B, so a shell history past that 1 MiB window stays out of reach there. Non-shell Pane B is unchanged: it already loads `full=1`, and its history is out of scope for this pull (codex and Claude's inline renderer do grow tmux history; this just isn't how they recover it). ⚠️ App-level terminal actions find their pane through `_focusedPane()`, the terminal focused LAST (not `document.activeElement`, which the mic or a header button takes): Ctrl+L, Ctrl+Shift+R, voice and image paste act on Pane B while it holds the keyboard. Ctrl+W is not an app shortcut at all (Close Session has no default key), so it reaches whichever pane is focused as delete-word. ⚠️ Geometry: `TerminalTile.fit()` sends the size the xterm actually holds, unfloored (the divider's 20% clamp leaves about 28 columns), skips an unchanged size, re-sends on every fresh socket, and adopts the PTY's column count from `{t:'zc'}`; the font, family and weight setters call `tile.fit()`, never a local refit alone (#464). Design: `docs/split-pane-sessions-plan.md`; the tile class: `docs/tile-grid-plan.md`. ### Gesture control: the setting diff --git a/docs/split-pane-sessions-plan.md b/docs/split-pane-sessions-plan.md index cc628486..6d7416d2 100644 --- a/docs/split-pane-sessions-plan.md +++ b/docs/split-pane-sessions-plan.md @@ -8,7 +8,8 @@ > (`terminal-tile.js`) and is no longer as plain as this spec describes: it > reconnects after a drop, delivers input exactly once, has clickable paths > and image paste, sizes its PTY without a floor and adopts `zc` columns, and -> the app-level terminal shortcuts follow the focused pane (Ctrl+W excepted). +> the app-level terminal shortcuts follow the focused pane. Ctrl+W no longer +> closes anything (Close Session has no default key). > See `docs/tile-grid-plan.md` and `architecture-invariants#split-pane-sessions`. ## Problem diff --git a/docs/tile-grid-plan.md b/docs/tile-grid-plan.md index acc8952c..bffaebe6 100644 --- a/docs/tile-grid-plan.md +++ b/docs/tile-grid-plan.md @@ -26,7 +26,7 @@ small header: status dot, session name, a `⋯` menu, maximize, `+` and `×`. - Every tile is equal: same terminal class, same features, same header. One tile is **focused** and receives the keyboard. - The rest of the app follows the focused tile: files panel, git status, - respawn and Ralph panels, subagent windows, voice, image paste, Ctrl+W. + respawn and Ralph panels, subagent windows, voice, image paste. - Tiles survive a Codeman restart (reconnect) and a page reload (per-device persistence). - The split pane stays as it is for now (decided). The grid is a separate @@ -190,9 +190,8 @@ animation frame, and sends one resize per affected tile at pointer-up. in the normal single view; the grid is remembered and one click on Tiles brings it back (decision 1). An app-driven selection (`auto: true`) never collapses the grid; see "Selections while the grid is open". -- Ctrl+L clears the focused tile; Ctrl+W closes the focused session (killing - it without a confirm, exactly as in the single view, where the focused tile - is the active session); Ctrl +/- +- Ctrl+L clears the focused tile; Ctrl+W is delete-word in the focused tile + (it is not an app shortcut, decision 5); Ctrl +/- changes the tile font size; Ctrl+Shift+R restores the focused tile's size. ### Persistence @@ -461,7 +460,7 @@ the grid. Each one gets an explicit grid-aware behavior: | Path | Today | With the grid open | |---|---|---| -| Ctrl+W / Close session on the focused tile (`closeSession`) | Reads `wasActive` before its `await`, adds the id to `_closingSessions`, then selects the first remaining `sessionOrder` entry with `auto: true`, which is often NOT tiled | A grid-aware fallback picker: remove the tile, then focus the neighboring tile (next in grid order, else previous). Only with no tiles left does it fall back to the `sessionOrder` pick, which closes the grid. Note the split's `_onSessionDeleted` wrapper deliberately skips selection for ids in `_closingSessions`, so the fallback MUST live in `closeSession` itself, not in the delete wrapper. | +| Close session on the focused tile (`closeSession`, from its menu or a user-bound key) | Reads `wasActive` before its `await`, adds the id to `_closingSessions`, then selects the first remaining `sessionOrder` entry with `auto: true`, which is often NOT tiled | A grid-aware fallback picker: remove the tile, then focus the neighboring tile (next in grid order, else previous). Only with no tiles left does it fall back to the `sessionOrder` pick, which closes the grid. Note the split's `_onSessionDeleted` wrapper deliberately skips selection for ids in `_closingSessions`, so the fallback MUST live in `closeSession` itself, not in the delete wrapper. | | Session deleted elsewhere (`_onSessionDeleted`) | The handoff selects the first remaining `sessionOrder` entry | If it was tiled: remove the tile and focus a neighbor with `auto: true`. If it was not tiled it was not active, so there is no handoff. | | Boot restore (`handleInit`) | `selectSession(restoreId, { auto: true })` | Replaced by the grid restore when a stored grid is open (see "Persistence") | | URL `#session=` link | `selectSession(id, { auto: true })` | Following a link is navigation, so this path passes `leaveTiles: true`: a tiled id focuses its tile, a non-tiled id opens the single view (grid kept in storage) | @@ -493,7 +492,7 @@ sound/title/desktop notification, so nothing is silently swallowed. | Copy | `copyTerminalSelection` / `cleanedTerminalSelection` read `this.terminal`; Pane B re-implements them | Both take `(terminal, sessionId)`; Pane B's copy is deleted | | Image paste | `_handleImagePaste()` uses the main terminal; `_uploadAndInsertImages` inserts with `sendInput()`, which re-reads `activeSessionId` AFTER the upload (an existing bug: switch tabs mid-upload and the paths land in the wrong session) | `_handleImagePaste({ terminal, sessionId })`; insert with `_sendInputAsync(sessionId, paths, { useMux: true })` | | Voice | `_insertText` re-reads `app.activeSessionId` at insert time and appends to the main local-echo overlay | Capture the target in `start()`; send with `_sendInputAsync(target, …)`; skip the overlay when the target is not the main terminal | -| Shortcuts | Ctrl+L (`clearTerminal`) and Ctrl+Shift+R (`restoreTerminalSize`) act on `this.terminal` | Resolve through `_focusedPane()` returning `{ terminal, sessionId, isPrimary }`; Ctrl+W already takes an id | +| Shortcuts | Ctrl+L (`clearTerminal`) and Ctrl+Shift+R (`restoreTerminalSize`) act on `this.terminal` | Resolve through `_focusedPane()` returning `{ terminal, sessionId, isPrimary }`; Close Session (no default key) already takes an id | | Font, family, weight, skin | `setFontSize` / `setFontFamily` / `setFontWeight` / `applyTerminalSkin` special-case `this._splitPane` | Loop over all tiles | ### 7. Fonts @@ -551,7 +550,7 @@ and share one tile class: | Situation | Behavior | |---|---| | A tiled session is deleted (here or elsewhere) | Tile removed; a neighbor gets focus with `auto: true`; the last tile gone falls through to the normal handoff | -| Ctrl+W on the focused tile | The grid stays open and the neighboring tile takes focus (grid-aware fallback in `closeSession`, see "Selections while the grid is open") | +| Closing the focused tile's session | The grid stays open and the neighboring tile takes focus (grid-aware fallback in `closeSession`, see "Selections while the grid is open") | | A tiled session is popped out to its own window | Tile removed: that window now owns the PTY size | | Session exited or not attached (`pid === null`, `paneExit`) | The tile body shows "Not attached" with an Attach button: `POST /interactive` (or `/shell` for shell mode) with NO body, at most one in flight per session (the route has no in-flight guard of its own). A tripped PTY-exit breaker goes through the existing confirm before `clearBreaker: true`; no automatic path ever sends it | | A web tab is opened | Grid hidden by CSS; sockets stay up; hidden tiles send no resizes. Selecting a tiled session's tab brings the grid back | @@ -651,11 +650,8 @@ Commits: spec documented and accepted for its v1 ("Ctrl+L or Ctrl+W typed while Pane B has focus clears or closes Pane A"). Typing into Pane B now clears its idle alert through `_ackDelivery`, which it never did. - **Ctrl+W is deliberately NOT retargeted in PR 1** (open decision 5): - `killActiveSession` calls `closeSession(id)` with `killMux = true` and no - confirm dialog, so moving it to Pane B changes which agent a muscle-memory - Ctrl+W kills outright. It keeps closing Pane A's session unless the - decision says otherwise. + **Ctrl+W no longer closes anything** (decision 5): Close Session has no + default key, so Ctrl+W reaches the focused pane as delete-word. 4. **Docs.** Update the split-pane paragraph in CLAUDE.md and `docs/architecture-invariants.md#split-pane-sessions` (Pane B now reconnects, delivers input exactly once, has links and image paste, and @@ -680,7 +676,9 @@ PR 1 tests (gate): redelivery per socket, POST fallback when no socket is registered. - `test/focused-pane-shortcuts.test.ts`: with Pane B focused, Ctrl+L clears Pane B, Ctrl+Shift+R restores Pane B's size, voice and image paste target - Pane B's session, and Ctrl+W still targets Pane A's session (decision 5); + Pane B's session, and a user-bound Close Session still targets the active + session; `test/ctrl-w-never-closes.test.ts` pins that no default shortcut + answers Ctrl+W; with Pane A focused nothing changes. - Geometry: with the split divider at its 20% clamp, Pane B's xterm and the size it sends are both under 40 columns and equal (no floor regression). @@ -737,7 +735,7 @@ Commits: `_cleanupPreviousSession`; acknowledgement only when user-initiated; an `auto: true` selection of a non-tiled session leaves the grid open; a user-initiated one closes it; `leaveTiles: true` closes it. -- `test/tile-grid-close-fallback.test.ts`: Ctrl+W (`closeSession`) on the +- `test/tile-grid-close-fallback.test.ts`: closing (`closeSession`) the focused tile keeps the grid open and focuses the neighboring tile, even when the first `sessionOrder` entry is not tiled; closing the last tile falls back to the normal pick. @@ -829,11 +827,10 @@ exits green. Use the browser runner for those files and read the file count. 3. **Persistence.** Decided: per device, restored on reload. 4. **PR shape.** Decided: two PRs. PR 1 is the tile foundation (the split improves on its own), PR 2 is the grid. -5. **Ctrl+W in the split while Pane B has focus.** Open, default applied: - keep it closing Pane A's session (today's behavior), because Ctrl+W kills - without a confirm. Alternative: retarget it to the focused pane like the - other shortcuts. In the grid it always follows focus, since the focused tile - IS the active session there. +5. **Ctrl+W.** Decided: it never closes a session. Close Session has no + default key (Ctrl+W is delete-word in every shell and agent CLI, and it + killed sessions with no confirm); it stays bindable in App Settings → + Shortcuts. ## Code anchors diff --git a/docs/wiki/Keyboard-Shortcuts.md b/docs/wiki/Keyboard-Shortcuts.md index 098f7e05..f8a88812 100644 --- a/docs/wiki/Keyboard-Shortcuts.md +++ b/docs/wiki/Keyboard-Shortcuts.md @@ -9,13 +9,16 @@ Press `Ctrl+?` in the app for the same list in a floating overlay. | Shortcut | Action | | ------------------------------- | --------------------------------------------------------------- | | `Ctrl+K` (also `Cmd+K`, `Alt+K`)| Find an open session or start a new one. | -| `Ctrl+W` | Kill the active session. | | `Ctrl+Tab` | Next session. | | `Alt+[` / `Alt+]` | Previous / next tab. | | `Alt+1` to `Alt+9` | Switch to tab N. Physical keys, so macOS Option layouts work. | | `Ctrl+Shift+{` / `Ctrl+Shift+}` | Move the active tab left / right. | | `Alt+B` | Collapse / expand the session sidebar, when that layout is on. | +`Ctrl+W` is not a Codeman shortcut: it goes to the terminal, where shells and agent CLIs +use it to delete the previous word. **Close Session** has no key by default; close a session +from its tab, or bind a key to it in App Settings → Shortcuts. + ## Terminal | Shortcut | Action | diff --git a/docs/wiki/Quick-Start.md b/docs/wiki/Quick-Start.md index e3caa75b..7e7d7f0a 100644 --- a/docs/wiki/Quick-Start.md +++ b/docs/wiki/Quick-Start.md @@ -131,7 +131,7 @@ the tmux server or rebooting the machine. | To do this | Do that | | ------------------------- | ------------------------------------------------------------------- | | Interrupt the current turn | `Ctrl+C` with nothing selected, or the **Stop** button. | -| Close one session | `Ctrl+W`, or the tab's close control. | +| Close one session | The tab's close control (`Ctrl+W` is delete-word in the terminal). | | Stop the server, keep agents | `codeman web --stop`. The tmux sessions stay alive. | | Stop everything | `tmux -L codeman kill-server`. | diff --git a/docs/wiki/The-Dashboard.md b/docs/wiki/The-Dashboard.md index 4943ab01..46a67628 100644 --- a/docs/wiki/The-Dashboard.md +++ b/docs/wiki/The-Dashboard.md @@ -64,7 +64,7 @@ reloading while a permission prompt is blocking does not lose the red tab. | Jump to tab N | `Alt+1` to `Alt+9` (the number on the tab) | | Next / previous | `Ctrl+Tab`, `Alt+[`, `Alt+]` | | Move the active tab | `Ctrl+Shift+{`, `Ctrl+Shift+}` | -| Close | `Ctrl+W` | +| Close | The tab's close control (no key by default) | | Find any session, open or past | `Ctrl+K` (also `Cmd+K` and `Alt+K`) | Tabs can also be dragged to reorder. diff --git a/src/web/public/app.js b/src/web/public/app.js index f8771a7a..96978897 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -419,7 +419,14 @@ const DEFAULT_SHORTCUTS = [ id: 'close-session', group: 'Session', label: 'Close Session', - bindings: [{ modifiers: ['ctrl'], key: 'w' }], + // ⚠️ No default key. This used to be Ctrl+W, which is "delete the previous + // word" in every shell, readline prompt and agent CLI, so muscle memory + // killed the session (tmux and the CLI, with no confirm) mid-sentence, and + // with the split open it was not even the pane being typed in. Ctrl+W now + // reaches the terminal like any other key. Closing stays on the tab's close + // control and menu (with their confirm), and anyone who wants a key binds + // one in App Settings → Shortcuts. + bindings: [], action: 'killActiveSession', }, { @@ -9134,6 +9141,9 @@ class CodemanApp { const fmtBindings = (s) => { if (s.displayBindings) return s.displayBindings.map((b) => `${escapeHtml(b)}`).join(' / '); if (!s.bindings) return ''; + // An action with no key (Close Session by default) is still listed, so the + // overlay says so instead of showing an empty key column. + if (s.bindings.length === 0) return 'not bound'; return s.bindings.map((b) => { const parts = [...(b.modifiers || []).map((m) => m.charAt(0).toUpperCase() + m.slice(1)), b.key || b.code || '']; return `${escapeHtml(parts.join('+'))}`; diff --git a/src/web/public/index.html b/src/web/public/index.html index 73515418..9cc86457 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -807,7 +807,6 @@

Session

-
Ctrl+W
Close Session
Ctrl/Cmd/Option+K
Find Open Session
Ctrl+Tab
Next Session
Alt/Option+[ / Alt/Option+]
Previous / Next Session
diff --git a/src/web/public/styles.css b/src/web/public/styles.css index f8498235..b2718c82 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -8520,6 +8520,12 @@ kbd { color: var(--text-dim); } +/* An action with no key bound (Close Session by default): say so, quietly. */ +.shortcut-overlay-unbound { + font-size: 0.75rem; + font-style: italic; +} + .shortcut-overlay-footer { margin-top: 0.75rem; padding-top: 0.75rem; diff --git a/test/ctrl-w-never-closes.test.ts b/test/ctrl-w-never-closes.test.ts new file mode 100644 index 00000000..553aa35f --- /dev/null +++ b/test/ctrl-w-never-closes.test.ts @@ -0,0 +1,104 @@ +/** + * @fileoverview Ctrl+W never closes a session; it reaches the terminal. + * + * "Close Session" used to be bound to Ctrl+W by default. Ctrl+W is "delete the + * previous word" in every shell, readline prompt and agent CLI, so muscle memory + * killed the session (its tmux pane and CLI, with no confirm) in the middle of a + * sentence; with the split pane open it was not even the pane being typed in. + * The action now has NO default key: the capture-phase shortcut handler lets + * Ctrl+W through, and xterm sends ^W to the CLI like any other key. The action + * stays in the registry so a user can still bind a key to it in App Settings → + * Shortcuts. + * + * Real code under test: constants.js + app.js (DEFAULT_SHORTCUTS, + * getShortcutRegistry, matchesShortcutEvent) in a `vm` context. + */ +import { readFileSync } from 'node:fs'; +import { performance } from 'node:perf_hooks'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +const read = (f: string) => readFileSync(resolve(import.meta.dirname, `../src/web/public/${f}`), 'utf8'); + +type Shortcut = { id: string; action?: string; bindings?: unknown[]; disabled?: boolean }; +type App = { + getShortcutRegistry(): Shortcut[]; + matchesShortcutEvent(e: Record, s: Shortcut): boolean; + loadAppSettingsFromStorage: () => Record; +}; + +function makeApp(settings: Record = {}): App { + const context = vm.createContext({ + console, + performance, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + HTMLCanvasElement: class HTMLCanvasElement {}, + WebSocket: { OPEN: 1 }, + fetch: vi.fn(), + document: { addEventListener: vi.fn() }, + localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() }, + window: { addEventListener: vi.fn(), removeEventListener: vi.fn() }, + MobileDetection: {}, + }); + vm.runInContext(`${read('constants.js')}\n${read('app.js')}\nglobalThis.__CodemanApp = CodemanApp;`, context); + const CodemanApp = (context as unknown as { __CodemanApp: { prototype: object } }).__CodemanApp; + const app = Object.create(CodemanApp.prototype) as App; + app.loadAppSettingsFromStorage = () => settings; + return app; +} + +const keydown = (overrides: Record) => ({ + type: 'keydown', + key: 'w', + code: 'KeyW', + ctrlKey: false, + metaKey: false, + shiftKey: false, + altKey: false, + ...overrides, +}); + +describe('Ctrl+W is left to the terminal', () => { + it('Close Session is still in the registry, with no default key', () => { + const close = makeApp() + .getShortcutRegistry() + .find((s) => s.id === 'close-session'); + + expect(close?.action).toBe('killActiveSession'); + expect(close?.bindings).toEqual([]); + }); + + it.each([ + ['Ctrl+W', { ctrlKey: true }], + ['Cmd+W', { metaKey: true }], + ])('no default shortcut answers %s, so the capture handler lets it reach xterm', (_name, mods) => { + const app = makeApp(); + const event = keydown(mods); + + const matched = app.getShortcutRegistry().filter((s) => !s.disabled && s.action && app.matchesShortcutEvent(event, s)); + + expect(matched.map((s) => s.id)).toEqual([]); + }); + + it('a user can still bind a key to Close Session', () => { + const app = makeApp({ + shortcutOverrides: { 'close-session': { bindings: [{ modifiers: ['ctrl', 'shift'], key: 'w' }] } }, + }); + const close = app.getShortcutRegistry().find((s) => s.id === 'close-session')!; + + expect(app.matchesShortcutEvent(keydown({ ctrlKey: true, shiftKey: true, key: 'W' }), close)).toBe(true); + expect(app.matchesShortcutEvent(keydown({ ctrlKey: true }), close)).toBe(false); + }); + + it('the shortcut overlay says an unbound action is not bound', () => { + const appSource = read('app.js'); + const overlay = appSource.slice(appSource.indexOf('renderShortcutOverlay() {'), appSource.indexOf('closeShortcutOverlay() {')); + + expect(overlay).toContain("if (s.bindings.length === 0) return 'not bound';"); + }); +}); diff --git a/test/focused-pane-shortcuts.test.ts b/test/focused-pane-shortcuts.test.ts index 562f5dd4..25d50042 100644 --- a/test/focused-pane-shortcuts.test.ts +++ b/test/focused-pane-shortcuts.test.ts @@ -8,9 +8,9 @@ * now answers with the pane whose terminal was focused last, and the actions * that are about a TERMINAL (clear, restore size) go through it. * - * Ctrl+W deliberately does NOT follow focus yet: it kills a session outright, - * with no confirm, so moving it changes which agent a muscle-memory press kills - * (docs/tile-grid-plan.md, decision 5). It stays on the active session. + * Close Session is not one of them: it has no default key any more (Ctrl+W is + * left to the terminal as delete-word, see ctrl-w-never-closes.test.ts), and a + * key a user binds to it closes the active session, as it always did. * * Real code under test: constants.js + terminal-ui.js in a `vm` context. */ @@ -136,7 +136,7 @@ describe('terminal shortcuts follow the focused pane', () => { }); }); -describe('Ctrl+W stays on the active session (decision 5)', () => { +describe('Close Session (user-bound key only) stays on the active session', () => { it('killActiveSession closes activeSessionId and never consults the focused pane', () => { const appSource = read('app.js'); const body = appSource.slice( diff --git a/test/help-modal-shortcuts.test.ts b/test/help-modal-shortcuts.test.ts index 7e41df5f..0d54e95d 100644 --- a/test/help-modal-shortcuts.test.ts +++ b/test/help-modal-shortcuts.test.ts @@ -38,7 +38,9 @@ describe('help modal shortcuts', () => { const helpModal = normalizedHtml(extractElementById(INDEX_HTML, 'helpModal')); it('documents implemented global and tab shortcuts', () => { - expectShortcut(helpModal, ['Ctrl', 'W'], 'Close Session'); + // Ctrl+W is NOT an app shortcut: it is delete-word in the terminal, and as + // Close Session it killed sessions with no confirm (ctrl-w-never-closes.test.ts). + expect(helpModal).not.toMatch(/Ctrl<\/kbd>\s*\+\s*W<\/kbd>/i); expectShortcut(helpModal, ['Ctrl', 'Tab'], 'Next Session'); expectShortcut(helpModal, ['Alt/Option', '['], 'Previous / Next Session'); expectShortcut(helpModal, ['Alt/Option', ']'], 'Previous / Next Session');