diff --git a/.changeset/newline-capability-key-tester.md b/.changeset/newline-capability-key-tester.md new file mode 100644 index 000000000..78d6b54ae --- /dev/null +++ b/.changeset/newline-capability-key-tester.md @@ -0,0 +1,5 @@ +--- +"aicodeman": patch +--- + +Shift+Enter's newline chord is now registry data (`capabilities.newline`: `line-feed` by default, `esc-enter` available for a CLI whose composer ignores a bare line feed; no stock CLI changes) instead of being chosen in the `send-key` route. Adds a Key tester under Settings → Terminal & Input that shows the keydown/keypress/keyup events a browser reports, to diagnose a device where a shortcut behaves differently. Keys pressed in the tester no longer trigger app shortcuts (Ctrl+W, Ctrl+L, Escape, ...). diff --git a/config/test-suites.ts b/config/test-suites.ts index 9f3ed295a..8414a767e 100644 --- a/config/test-suites.ts +++ b/config/test-suites.ts @@ -31,6 +31,7 @@ export const BROWSER_TEST_GLOBS = [ 'test/capture-geometry-retry.browser.test.ts', 'test/codex-predictive-echo.test.ts', // also needs a real codex binary 'test/split-pane-terminal.browser.test.ts', + 'test/key-tester.browser.test.ts', 'test/split-pane-orchestration.browser.test.ts', 'test/split-pane-auto-collapse.browser.test.ts', ]; diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index cbfdf3f25..370db66d7 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -191,7 +191,7 @@ Further detail (current geometry and colors, superseding the dip numbers above w **Three owners, one setter.** `SessionState.nameSource` is `placeholder` | `auto` | `manual`. The constructor infers a missing value from the name (`isGeneratedSessionName()` = `w-` / `s-`, or no name at all, is a placeholder; anything else was a person's), the create routes pass none, the boot restore passes the persisted one. The `name` setter is the manual path and the ONLY thing that produces `manual` after construction; `applyAutoName()` is the only thing that produces `auto`, and it does so whether or not the string changed, which is what makes "first prompt" mean the first. A prompt whose title is null (`/clear`, `! npm test`, blank) never reaches it, so the session stays eligible: the first REAL prompt names the tab. -**The origin gate defaults to "system".** `write()` / `writeViaMux()` take `SessionWriteOptions.fromUser`; only the browser WS path and `POST /api/sessions/:id/input` set it. Every other caller (Ralph, respawn, cron, approvals, the orchestrator's `/compact`, auto-ops, the trust-dialog keys) is system by omission, so a new user-input path that forgets the flag fails toward a tab that keeps its placeholder, never toward a tab named after a Ralph prompt. `_lastSubmitAt` is still stamped for every write; only the tracker feed is gated. The shell gate is `getCli(mode)?.capabilities.startMode !== 'shell'`, a capability rather than an id check (the no-id-branching guard), and the send-key route feeds `trackUserInput()` by hand because its `tmux send-keys -H` line feed never passes through the session. +**The origin gate defaults to "system".** `write()` / `writeViaMux()` take `SessionWriteOptions.fromUser`; only the browser WS path and `POST /api/sessions/:id/input` set it. Every other caller (Ralph, respawn, cron, approvals, the orchestrator's `/compact`, auto-ops, the trust-dialog keys) is system by omission, so a new user-input path that forgets the flag fails toward a tab that keeps its placeholder, never toward a tab named after a Ralph prompt. `_lastSubmitAt` is still stamped for every write; only the tracker feed is gated. The shell gate is `getCli(mode)?.capabilities.startMode !== 'shell'`, a capability rather than an id check (the no-id-branching guard), and the send-key route feeds `trackUserInput()` by hand because its `tmux send-keys -H` newline (a line feed unless the CLI declares another `capabilities.newline` chord) never passes through the session. **The tracker is a best-effort transcript with explicit per-key rules**, not a byte filter. Mirrored: printable text, backspace, Ctrl+W, Ctrl+U/Ctrl+C (composer emptied), `\n` and Alt+Enter (a newline IN the composer, joined with a space), bracketed paste (newlines inside it likewise). Ignored: cursor keys, Home/End/Delete, Shift+Tab, Tab, SGR mouse and focus reports, Alt chords, OSC/DCS, the rest of C0. Tainting: Up/Down (CSI and SS3), Ctrl+P/N/R, Ctrl+_, because the composer then holds a history line the tracker never saw and Enter must submit nothing rather than a fragment. A bare Esc is resolved at the END of the chunk it arrives in, since xterm hands each key's whole sequence to one write and the programmatic senders send Esc alone; a CSI split across chunks still resumes. The draft keeps its HEAD past 8192 code points (the title is the first sentence, so keeping the tail would title a long paste by its last line) and an escape sequence is abandoned past 64 bytes. @@ -780,7 +780,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 a new, independent `SplitTerminalPane` (terminal-split.js) with its own xterm instance and its own `/ws/sessions/:id/terminal` WebSocket. ⚠️ **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 `SplitTerminalPane` 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 an explicit `this._splitPane?.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 leaves Pane B visibly dead (a message written into its own xterm buffer) rather than silently swallowing keystrokes with nothing on screen to explain why — there is no reconnect logic for v1, matching the "deliberately plainer than Pane A" design. 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'}`, tmux `send-keys -H` for a real 0x0a) 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 stays on xterm's own default paste, since Pane B has no image-paste trap to route it to. ⚠️ `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 input frames deliberately carry no `cid`/`seq` (`ws-routes.ts` supports that), matching the no-overlay/no-IME "deliberately plainer" list above, since it has no exactly-once delivery layer to key them against. ⚠️ 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 mid-replay, a `{t:'c'}` clear frame included, are held with their arrival time (`_liveQueue`) 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 fetch has a 10 s deadline because it holds the pane's live output while it runs. ⚠️ The "Pane B 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 a pull writes nothing: `_onSocketClosed()` skips the marker while `_liveQueue` is live, since written there it would sit above the held frames the pull flushes after a skip, a downgrade, a failed fetch or the deadline, or land mid-way through a chunked replay; the pull's `finally` then writes it once, replayed or not (`closedBefore` tells the two closes apart). Both are tracked via `_wsClosed` 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). Design: `docs/split-pane-sessions-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 a new, independent `SplitTerminalPane` (terminal-split.js) with its own xterm instance and its own `/ws/sessions/:id/terminal` WebSocket. ⚠️ **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 `SplitTerminalPane` 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 an explicit `this._splitPane?.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 leaves Pane B visibly dead (a message written into its own xterm buffer) rather than silently swallowing keystrokes with nothing on screen to explain why — there is no reconnect logic for v1, matching the "deliberately plainer than Pane A" design. 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'}`, tmux `send-keys -H` for a real 0x0a, or the CLI's declared `capabilities.newline` chord) 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 stays on xterm's own default paste, since Pane B has no image-paste trap to route it to. ⚠️ `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 input frames deliberately carry no `cid`/`seq` (`ws-routes.ts` supports that), matching the no-overlay/no-IME "deliberately plainer" list above, since it has no exactly-once delivery layer to key them against. ⚠️ 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 mid-replay, a `{t:'c'}` clear frame included, are held with their arrival time (`_liveQueue`) 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 fetch has a 10 s deadline because it holds the pane's live output while it runs. ⚠️ The "Pane B 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 a pull writes nothing: `_onSocketClosed()` skips the marker while `_liveQueue` is live, since written there it would sit above the held frames the pull flushes after a skip, a downgrade, a failed fetch or the deadline, or land mid-way through a chunked replay; the pull's `finally` then writes it once, replayed or not (`closedBefore` tells the two closes apart). Both are tracked via `_wsClosed` 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). Design: `docs/split-pane-sessions-plan.md`. ### Gesture control: the setting diff --git a/docs/cli-registry.md b/docs/cli-registry.md index 351d6e49f..2a8eee76c 100644 --- a/docs/cli-registry.md +++ b/docs/cli-registry.md @@ -118,6 +118,10 @@ sure its row is one the agent cannot write. `test/cli-capability-predicates.test.ts` asserts that no two of the three are equivalent across the catalog, so collapsing them fails the build rather than a user's session. +## The newline chord + +`capabilities.newline` (`'line-feed'` | `'esc-enter'`, absent = line feed) is the byte sequence the `send-key` route types into the pane for Shift+Enter. A line feed (`0x0a`, also Ctrl+Enter) is what Claude Code's Ink input reads as "insert a newline"; `esc-enter` (`ESC CR`, the Option/Alt+Enter chord) is there for a composer that ignores a bare line feed. No stock CLI declares it today: the bytes are typed by tmux on the server, so the browser's OS cannot change what a CLI reads, and Codex 0.147.0 was checked to take a line feed (a Shift+Enter that submits is the keypress leak fixed in #520, not a byte problem). A user `clis.json` can set it for a CLI that needs it. It is an enum rather than a byte string on purpose: config never carries bytes that get typed into a pane. Settings → Terminal & Input → **Key tester** prints what a browser reports for keydown/keypress/keyup, to see whether a device is sending what you think. + ## Arg-template safety The composed command line is interpolated into `bash -c "…"` inside tmux, which makes command construction a security boundary. Four independent layers keep config out of it: diff --git a/src/config/cli-registry/schema.ts b/src/config/cli-registry/schema.ts index b72c58fda..83a22b9d4 100644 --- a/src/config/cli-registry/schema.ts +++ b/src/config/cli-registry/schema.ts @@ -377,6 +377,7 @@ const capabilitiesSchema = z privilegedEnvKeys: z.array(envName).max(8), gates: z.record(z.string(), z.object({ minVersion: z.string().max(20), failClosed: z.boolean() }).strict()), maxFrameBytes: z.number().int().positive().optional(), + newline: z.enum(['line-feed', 'esc-enter']).optional(), customModelInjection: z.discriminatedUnion('kind', [ z .object({ diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index c39007cd3..767d417c2 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -90,6 +90,9 @@ export interface CliVariant { args: ArgSpec[]; } +/** The newline chord a CLI's composer reads as "insert a line break" (see `CliCapabilities.newline`). */ +export type NewlineSequence = 'line-feed' | 'esc-enter'; + export interface CliLaunch { params: Record; /** @@ -511,6 +514,14 @@ export interface CliCapabilities { gates: Record; /** Cap on a single terminal frame, when this CLI needs a tighter one than the default. */ maxFrameBytes?: number; + /** + * The bytes the web UI types into this CLI's pane for Shift+Enter (the `send-key` route). + * `line-feed` (`0x0a`, also what Ctrl+Enter sends) is what Claude Code's Ink input and most TUIs + * read as "insert a newline"; `esc-enter` (`ESC` `CR`, the same chord as Option/Alt+Enter and + * the mobile ⌥Enter key) is for a TUI that ignores a bare line feed. Absent = `line-feed`. + * Data, not a branch on the CLI id, so supporting another CLI's quirk is one line here. + */ + newline?: NewlineSequence; /** * How this CLI is pointed at a user-supplied custom OpenAI-compatible * endpoint (local, e.g. llama.cpp, or cloud, e.g. Azure AI Foundry) — the diff --git a/src/web/public/app.js b/src/web/public/app.js index f1d9da4a5..9fde38d3e 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -1242,6 +1242,12 @@ class CodemanApp { // Use capture to handle before terminal document.addEventListener('keydown', (e) => { + // A field that exists to show what a key does (Settings → Key tester, `data-raw-keys`) must + // receive every chord untouched. Without this, probing Ctrl+W killed the active session, + // Ctrl+L cleared the terminal and Escape closed Settings: this listener runs in the capture + // phase, before the field's own handler. Must stay the first statement. + if (e.target?.closest?.('[data-raw-keys]')) return; + // Don't intercept keys during CJK IME composition if (e.isComposing || e.keyCode === 229) return; diff --git a/src/web/public/index.html b/src/web/public/index.html index 40d19750a..851b9a10b 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -1822,6 +1822,22 @@

Terminal & Input

+ +
+

Key tester

device
+
+
+
+ Key tester + Click the box and press keys to see what this browser reports (key, code, modifiers) for keydown, keypress and keyup. Useful when a shortcut such as Shift+Enter behaves differently on one device. Nothing is sent to a session. +
+ +
+ +
+
diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 6c5208e80..27783cdb9 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -1114,6 +1114,27 @@ Object.assign(CodemanApp.prototype, { this._updateCheck = null; }, + /** + * Settings → Terminal & Input → Key tester: prints what the browser reports for each key event. + * Read-only and local; it never reaches a session. keypress is shown on purpose: that event is + * why a Shift-only Enter used to submit (xterm drops Ctrl/Alt keypresses, not Shift ones). + */ + keyTesterEvent(ev) { + const log = document.getElementById('keyTesterLog'); + if (!log) return; + // Never preventDefault on keydown: that suppresses the keypress this panel exists to show. + // The field is readonly, so nothing is typed into it either way. + const mods = ['ctrlKey', 'shiftKey', 'altKey', 'metaKey'].filter((m) => ev[m]).map((m) => m.replace('Key', '')); + const line = + `${ev.type.padEnd(8)} key=${JSON.stringify(ev.key)} code=${ev.code || '-'} ` + + `mods=${mods.join('+') || 'none'}` + + (ev.type === 'keypress' ? ` charCode=${ev.charCode}` : '') + + (ev.repeat ? ' (repeat)' : ''); + const lines = (log.textContent ? log.textContent.split('\n') : []).concat(line); + log.textContent = lines.slice(-14).join('\n'); + log.style.display = 'block'; + }, + _setUpdateResult(html) { const el = this.$('updateResult'); if (el) { el.style.display = 'block'; el.innerHTML = html; } diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 6a7b78ad6..2a81f4dcd 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -107,6 +107,7 @@ import { buildAgentCaseMarker, writeAgentCaseMarker } from '../../agent-case-mar import { canUsernameRunPrivilegedCommands, resolveClaudeModeForUsername } from '../../user-store.js'; import { clampEnvOverridesForOwner } from '../../session-env-clamp.js'; import { enabledClis, getCli } from '../../config/cli-registry/registry.js'; +import type { NewlineSequence } from '../../config/cli-registry/types.js'; import { resolveCliLaunchError } from '../../utils/cli-launcher.js'; import { legacyConfigForMode } from '../../session-cli-registry-bridge.js'; import { isMultiUserMode } from '../../config/multiuser.js'; @@ -2092,21 +2093,16 @@ export function registerSessionRoutes( // ========== Send Named Key (tmux send-keys -H) ========== // Sends raw hex bytes to tmux pane for keys like Shift+Enter / Ctrl+Enter. - // Uses send-keys -H (hex) to inject 0x0a (line feed) which Claude Code's - // Ink input recognizes as "insert newline" vs 0x0d (carriage return = submit). + // Uses send-keys -H (hex) to inject a newline chord: 0x0a (line feed) by default, or the CLI's + // own `capabilities.newline`. Claude Code's Ink input recognizes 0x0a as "insert newline" vs + // 0x0d (carriage return = submit). app.post('/api/sessions/:id/send-key', async (req) => { const { id } = req.params as { id: string }; const body = req.body as Record; const key = typeof body?.key === 'string' ? body.key : ''; - // Map key names to hex byte sequences - const KEY_HEX_MAP: Record = { - 'S-Enter': ['0a'], // \n (line feed) - 'C-Enter': ['0a'], // \n (line feed) - }; - const hex = KEY_HEX_MAP[key]; - if (!hex) { + if (key !== 'S-Enter' && key !== 'C-Enter') { return createErrorResponse(ApiErrorCode.INVALID_INPUT, `Key not allowed: ${key}`); } @@ -2116,6 +2112,18 @@ export function registerSessionRoutes( return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'No tmux session'); } + // Key names map to hex byte sequences. Ctrl+Enter is always a line feed; Shift+Enter is the + // CLI's own newline chord (`capabilities.newline`, default line feed), so a CLI that wants + // Esc+Enter declares it in the registry instead of being special-cased here. + const NEWLINE_HEX: Record = { + 'line-feed': ['0a'], // \n + 'esc-enter': ['1b', '0d'], // ESC CR, the Alt/Option+Enter chord + }; + const hex = + key === 'C-Enter' + ? NEWLINE_HEX['line-feed'] + : NEWLINE_HEX[getCli(session.mode)?.capabilities.newline ?? 'line-feed']; + try { // Route through the dedicated Codeman socket — bare `tmux` would target the // user's default server and never find this session (same #80 regression class). diff --git a/test/cli-newline-capability.test.ts b/test/cli-newline-capability.test.ts new file mode 100644 index 000000000..156431ad3 --- /dev/null +++ b/test/cli-newline-capability.test.ts @@ -0,0 +1,37 @@ +// @vitest-environment node +// capabilities.newline: the bytes Shift+Enter types into a CLI's pane. Data in the registry, not +// a branch on the CLI id (test/cli-registry-no-id-branching.test.ts keeps the latter true). + +import { describe, expect, it } from 'vitest'; +import { CliEntrySchema } from '../src/config/cli-registry/schema.js'; +import { STOCK_CLIS } from '../src/config/cli-registry/stock.js'; +import type { CliEntry } from '../src/config/cli-registry/types.js'; + +const claude = () => structuredClone(STOCK_CLIS.find((e) => (e.id as string) === 'claude')!) as CliEntry; + +describe('capabilities.newline', () => { + it('no stock CLI declares a chord: every one keeps the line feed', () => { + // codex 0.147.0 takes a line feed (checked against a real tmux pane), so there is no CLI that + // needs esc-enter yet. The capability exists for a user clis.json override and the next CLI. + const declared = STOCK_CLIS.filter((e) => e.capabilities.newline).map((e) => e.id as string); + expect(declared).toEqual([]); + }); + + it.each(['line-feed', 'esc-enter'])('schema accepts %s', (value) => { + const e = claude(); + (e.capabilities as Record).newline = value; + expect(CliEntrySchema.safeParse(e).success).toBe(true); + }); + + it.each(['lf', 'crlf', '\x1b\r', '', 0])('schema rejects %j (no free-form byte strings in config)', (value) => { + const e = claude(); + (e.capabilities as Record).newline = value; + expect(CliEntrySchema.safeParse(e).success).toBe(false); + }); + + it('is optional, so an entry that declares nothing keeps the line feed', () => { + const e = claude(); + delete (e.capabilities as Record).newline; + expect(CliEntrySchema.safeParse(e).success).toBe(true); + }); +}); diff --git a/test/key-tester.browser.test.ts b/test/key-tester.browser.test.ts new file mode 100644 index 000000000..5b1fcc3aa --- /dev/null +++ b/test/key-tester.browser.test.ts @@ -0,0 +1,96 @@ +/** @fileoverview Settings → Terminal & Input → Key tester, driven with real keystrokes in Chromium. */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { chromium, type Browser, type Page } from 'playwright'; +import { WebServer } from '../src/web/server.js'; + +const PORT = 3194; + +describe('Key tester in a real browser', () => { + let server: WebServer; + let browser: Browser; + let page: Page; + + beforeAll(async () => { + server = new WebServer(PORT, false, true); + await server.start(); + browser = await chromium.launch({ headless: true }); + page = await browser.newPage(); + await page.goto(`http://localhost:${PORT}`, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 }); + await page.evaluate(() => (window as any).app.openAppSettings()); + await page.focus('#keyTesterInput'); + }, 90000); + + afterAll(async () => { + if (browser) await browser.close(); + if (server) await server.stop(); + }, 60000); + + const log = () => page.evaluate(() => document.getElementById('keyTesterLog')!.textContent ?? ''); + + it('shows keydown, keypress and keyup for Shift+Enter, with the modifier and charCode', async () => { + await page.keyboard.press('Shift+Enter'); + const text = await log(); + expect(text).toMatch(/keydown\s+key="Enter" code=Enter mods=shift/); + // The keypress is the event that used to leak a bare \r to the PTY. + expect(text).toMatch(/keypress\s+key="Enter" code=Enter mods=shift charCode=13/); + expect(text).toMatch(/keyup\s+key="Enter" code=Enter mods=shift/); + }); + + it('shows Ctrl+Enter without a keypress, as xterm would never see one for Ctrl', async () => { + await page.evaluate(() => (document.getElementById('keyTesterLog')!.textContent = '')); + await page.keyboard.press('Control+Enter'); + const text = await log(); + expect(text).toMatch(/keydown\s+key="Enter" code=Enter mods=ctrl/); + expect(text).toMatch(/keyup/); + // Chromium emits no keypress for a Ctrl chord, which is why only Shift+Enter ever leaked a \r. + expect(text).not.toMatch(/keypress/); + }); + + it('lets no app shortcut fire for keys pressed in the field (Ctrl+W, Ctrl+L, Escape, Alt+1, Ctrl+K)', async () => { + // The shortcut dispatcher is a capture-phase document listener, so without a guard it ran before + // the field's own handler: Ctrl+W killed the active session, Ctrl+L cleared the terminal and + // Escape closed Settings, while this row says nothing is sent to a session. + await page.evaluate(() => { + const app = (window as any).app; + const calls: string[] = []; + (window as any).__calls = calls; + for (const name of ['killActiveSession', 'clearTerminal', 'openCommandPalette', 'closeAllPanels']) { + app[name] = (...args: unknown[]) => void calls.push(name + args.length); + } + }); + await page.focus('#keyTesterInput'); + // [chord, what the tester must report for it]; checked one at a time because the log keeps 14 lines. + const chords: [string, RegExp][] = [ + ['Control+W', /key="w" code=KeyW mods=ctrl/i], + ['Control+L', /key="l" code=KeyL mods=ctrl/i], + ['Escape', /key="Escape" code=Escape/], + ['Alt+1', /code=Digit1 mods=alt/], + ['Control+K', /key="k" code=KeyK mods=ctrl/i], + ]; + for (const [chord, seen] of chords) { + await page.evaluate(() => (document.getElementById('keyTesterLog')!.textContent = '')); + await page.keyboard.press(chord); + expect(await log(), chord).toMatch(seen); + expect(await page.evaluate(() => (window as any).__calls), chord).toEqual([]); + } + expect(await page.evaluate(() => document.getElementById('appSettingsModal')!.classList.contains('active'))).toBe( + true + ); + }); + + it('still lets the shortcut fire anywhere else (the guard is scoped to data-raw-keys)', async () => { + await page.evaluate(() => { + (window as any).__calls.length = 0; + (document.activeElement as HTMLElement | null)?.blur(); + }); + await page.keyboard.press('Escape'); + expect(await page.evaluate(() => (window as any).__calls)).toContain('closeAllPanels0'); + }); + + it('keeps only the last 14 lines and never types into the field', async () => { + for (let i = 0; i < 8; i++) await page.keyboard.press('a'); + expect((await log()).split('\n').length).toBeLessThanOrEqual(14); + expect(await page.inputValue('#keyTesterInput')).toBe(''); + }); +}); diff --git a/test/routes/session-routes.test.ts b/test/routes/session-routes.test.ts index cc9c94223..11b1e755c 100644 --- a/test/routes/session-routes.test.ts +++ b/test/routes/session-routes.test.ts @@ -17,7 +17,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import Fastify, { type FastifyInstance } from 'fastify'; import fastifyCookie from '@fastify/cookie'; import fastifyMultipart from '@fastify/multipart'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { registryFilePath, reloadCliRegistry } from '../../src/config/cli-registry/registry.js'; import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; @@ -165,6 +167,74 @@ describe('session-routes', () => { expect(argv).toContain('-H'); }); + describe('newline chord comes from the CLI registry (capabilities.newline)', () => { + const sentHex = async (mode: string, key: string): Promise => { + execFile.mockReset(); + execFile.mockImplementation((_bin: string, _argv: string[], _opts: unknown, cb: (e: Error | null) => void) => + cb(null) + ); + const session = harness.ctx._session as unknown as { mode: string }; + const before = session.mode; + session.mode = mode; + try { + const res = await harness.app.inject({ + method: 'POST', + url: '/api/sessions/test-session-1/send-key', + payload: { key }, + }); + expect(res.statusCode).toBe(200); + } finally { + session.mode = before; + } + const argv = execFile.mock.calls[0][1] as string[]; + return argv.slice(argv.indexOf('-H') + 3); // after "-H -t " + }; + + it('sends a line feed for Shift+Enter to a CLI that declares nothing', async () => { + expect(await sentHex('claude', 'S-Enter')).toEqual(['0a']); + expect(await sentHex('opencode', 'S-Enter')).toEqual(['0a']); + }); + + it('sends a line feed to Codex too: no stock CLI declares a chord', async () => { + expect(await sentHex('codex', 'S-Enter')).toEqual(['0a']); + }); + + describe('a CLI that declares esc-enter (here via a user clis.json override of codex)', () => { + beforeEach(() => { + const file = registryFilePath(); + mkdirSync(dirname(file), { recursive: true }); + writeFileSync( + file, + JSON.stringify({ schemaVersion: 1, clis: { codex: { capabilities: { newline: 'esc-enter' } } } }), + { mode: 0o600 } + ); + reloadCliRegistry(); + }); + afterEach(() => { + rmSync(registryFilePath(), { force: true }); + reloadCliRegistry(); + }); + + it('sends Esc+Enter for Shift+Enter, and only to that CLI', async () => { + expect(await sentHex('codex', 'S-Enter')).toEqual(['1b', '0d']); + expect(await sentHex('claude', 'S-Enter')).toEqual(['0a']); + }); + + it('still sends a line feed for Ctrl+Enter', async () => { + expect(await sentHex('codex', 'C-Enter')).toEqual(['0a']); + }); + }); + + it('always sends a line feed for Ctrl+Enter', async () => { + expect(await sentHex('codex', 'C-Enter')).toEqual(['0a']); + expect(await sentHex('claude', 'C-Enter')).toEqual(['0a']); + }); + + it('falls back to a line feed for a mode the registry does not know', async () => { + expect(await sentHex('no-such-cli', 'S-Enter')).toEqual(['0a']); + }); + }); + it('rejects keys outside the hex allowlist without invoking tmux', async () => { execFile.mockReset(); const res = await harness.app.inject({