From 80ebf8b5494459b2aed6016e15e0e8c123540753 Mon Sep 17 00:00:00 2001 From: "Claude (Codeman maintainer)" Date: Sun, 14 Jun 2026 22:27:36 +0200 Subject: [PATCH] fix(shortcuts): stop Alt/Option nav keys leaking ESC sequences into the terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR migrated the app.js tab-nav handler to physical e.code but left xterm's pass-through gate (terminal-ui.js) matching ev.key digits. Consequences: - Alt+[ / Alt+] (the new bindings) were never in the gate, so xterm sent ESC[ / ESC] to the PTY on every platform AS WELL AS switching the session. - Alt+digit on a remapped macOS Option layout (Option+1 -> "¡") didn't match the ev.key '0'-'9' gate either, so xterm injected ESC — on exactly the layouts this PR exists to fix. Update the xterm gate to mirror app.js exactly: suppress when `ev.altKey && !ctrl && !shift && /^(Digit[1-9]|BracketLeft|BracketRight)$/.test(ev.code)`. Returning false there tells xterm not to write to the PTY, so the shortcut switches the tab with no stray escape sequence. Also: relabel the docs Alt/Option (the mechanism is layout/OS-independent, so the shortcut works for Linux/Windows Alt users too — "Option" alone was Mac-only wording), and add a keyboard-shortcuts test asserting terminal-ui.js gates on the same physical codes so this desync can't regress (a grep the original test missed). Verified: keyboard-shortcuts test 4/4, check:frontend-syntax, check:public-assets, format:check all clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 4 ++-- src/web/public/index.html | 4 ++-- src/web/public/terminal-ui.js | 10 ++++++++-- test/keyboard-shortcuts.test.ts | 20 ++++++++++++++------ 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index c7527928..6bcfbaab 100644 --- a/README.md +++ b/README.md @@ -483,8 +483,8 @@ Single-digit selection (1-9), color-coded status, token counts, auto-refresh. De |----------|--------| | `Ctrl/Cmd+W` | Kill active session | | `Ctrl/Cmd+Tab` | Next session | -| `Option+[` / `Option+]` | Previous / next session | -| `Option+1`-`Option+9` | Switch to tab N (physical keys, so macOS Option layouts work) | +| `Alt/Option+[` / `Alt/Option+]` | Previous / next session | +| `Alt/Option+1`-`Alt/Option+9` | Switch to tab N (physical keys, so macOS Option layouts work) | | `Ctrl+Shift+{` / `Ctrl+Shift+}` | Move active tab left / right | | `Ctrl/Cmd+L` | Clear terminal | | `Ctrl+Shift+R` | Restore terminal size | diff --git a/src/web/public/index.html b/src/web/public/index.html index 38194fb8..09588109 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -505,8 +505,8 @@
Ctrl+W
Close Session
Ctrl+Tab
Next Session
-
Option+[ / Option+]
Previous / Next Session
-
Option+1-9
Switch to Tab N
+
Alt/Option+[ / Alt/Option+]
Previous / Next Session
+
Alt/Option+1-9
Switch to Tab N
Ctrl+L
Clear Terminal
Ctrl++
Increase Font
Ctrl+-
Decrease Font
diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index c6edc476..c39f91a2 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -114,8 +114,14 @@ Object.assign(CodemanApp.prototype, { this.terminal.attachCustomKeyEventHandler((ev) => { if (ev.isComposing || ev.keyCode === 229) return false; - // Let Alt+digit pass through to browser (tab switching) - if (ev.altKey && ev.key >= '0' && ev.key <= '9') return false; + // Let the app's Alt/Option session-nav shortcuts reach the document keydown handler + // (app.js switches tabs by PHYSICAL e.code) instead of xterm injecting ESC into + // the PTY. Mirror app.js's gate exactly — same physical codes + modifier guard — so + // macOS Option layouts (Option+1 -> "¡", Option+[ -> "“") are suppressed here too and + // don't leak an escape sequence into the focused terminal on every tab switch. + if (ev.altKey && !ev.ctrlKey && !ev.shiftKey && /^(Digit[1-9]|BracketLeft|BracketRight)$/.test(ev.code || '')) { + return false; + } // Ctrl+V / Cmd+V: intercept before xterm sends ^V to PTY. // Route through our paste trap which handles both images and text. diff --git a/test/keyboard-shortcuts.test.ts b/test/keyboard-shortcuts.test.ts index b02f43d6..7b93e541 100644 --- a/test/keyboard-shortcuts.test.ts +++ b/test/keyboard-shortcuts.test.ts @@ -2,6 +2,7 @@ import { readFileSync } from 'node:fs'; import { describe, expect, it } from 'vitest'; const appSource = readFileSync('src/web/public/app.js', 'utf8'); +const terminalUiSource = readFileSync('src/web/public/terminal-ui.js', 'utf8'); const helpHtml = readFileSync('src/web/public/index.html', 'utf8'); const readme = readFileSync('README.md', 'utf8'); @@ -19,11 +20,18 @@ describe('keyboard shortcuts', () => { expect(appSource).toContain('this.nextSession()'); }); - it('documents mac-friendly Option shortcuts in help and README', () => { - expect(helpHtml).toContain('Option+['); - expect(helpHtml).toContain('Option+]'); - expect(helpHtml).toContain('Option+1-9'); - expect(readme).toContain('`Option+[` / `Option+]`'); - expect(readme).toContain('`Option+1`-`Option+9`'); + it('suppresses xterm PTY injection for the same physical Alt nav codes (no ESC leak)', () => { + // terminal-ui.js must gate its xterm pass-through on the SAME physical e.code set the + // app.js handler consumes; otherwise Alt+[ / Alt+] (and Option+digit on remapped macOS + // layouts) switch tabs AND inject ESC into the focused terminal. Keep in sync. + expect(terminalUiSource).toContain('/^(Digit[1-9]|BracketLeft|BracketRight)$/.test(ev.code'); + }); + + it('documents the Alt/Option shortcuts in help and README', () => { + expect(helpHtml).toContain('Alt/Option+['); + expect(helpHtml).toContain('Alt/Option+]'); + expect(helpHtml).toContain('Alt/Option+1-9'); + expect(readme).toContain('`Alt/Option+[` / `Alt/Option+]`'); + expect(readme).toContain('`Alt/Option+1`-`Alt/Option+9`'); }); });