diff --git a/src/e2e/returnToMenu.test.ts b/src/e2e/returnToMenu.test.ts new file mode 100644 index 00000000..040c37f8 --- /dev/null +++ b/src/e2e/returnToMenu.test.ts @@ -0,0 +1,130 @@ +import {describe, expect, it} from 'vitest'; +import {execFileSync, spawnSync} from 'child_process'; +import {existsSync} from 'fs'; +import {fileURLToPath} from 'url'; +import {dirname, join} from 'path'; + +/** + * End-to-end coverage for the "return to menu" shortcut (Ctrl+E by default). + * + * Each case boots the real CLI in a PTY through + * `returnToMenuPtyHarness.ts` (a separate `bun` process, because the PTY + * needs Bun's `Bun.Terminal` API while Vitest runs under node), attaches a + * session, sends one byte sequence, and reports whether the menu came back. + * + * The sequences below are the different byte encodings a terminal can send for + * the very same Ctrl+E keypress: + * + * - the ASCII control code, sent by terminals that do not use an extended + * keyboard protocol; + * - the kitty keyboard protocol's CSI-u form `ESC [ ; u`, + * where the modifier is a bit mask of `1 (base) + 4 (ctrl)`; + * - the same CSI-u form with a lock-state bit added to the modifier mask + * (`+64` for Caps Lock, `+128` for Num Lock), which the kitty protocol + * includes whenever the corresponding lock happens to be on. This is the + * case reported in https://github.com/kbwo/ccmanager/issues/327. + */ +const SEQUENCES = { + /** Ctrl+E as an ASCII control code. */ + controlCode: '\u0005', + /** CSI-u, modifier 5 = 1 (base) + 4 (ctrl). */ + csiUCtrl: '\u001b[101;5u', + /** CSI-u, modifier 133 = 1 (base) + 4 (ctrl) + 128 (num lock). */ + csiUCtrlNumLock: '\u001b[101;133u', + /** CSI-u, modifier 69 = 1 (base) + 4 (ctrl) + 64 (caps lock). */ + csiUCtrlCapsLock: '\u001b[101;69u', +} as const; + +interface HarnessResult { + menuAppeared: boolean; + sessionStarted: boolean; + returnedToMenu: boolean; + error?: string; + tail?: string; +} + +// `bun install` runs the build through the `prepare` script, so this test also +// runs from the compiled `dist/` tree, where the harness sits next to it as +// JavaScript rather than TypeScript. +const testDir = dirname(fileURLToPath(import.meta.url)); +const harnessPath = existsSync(join(testDir, 'returnToMenuPtyHarness.ts')) + ? join(testDir, 'returnToMenuPtyHarness.ts') + : join(testDir, 'returnToMenuPtyHarness.js'); + +/** + * The harness needs the `bun` runtime (for the PTY) and a Unix pseudo terminal, + * so skip everywhere that cannot provide both instead of failing. + */ +function canRunHarness(): boolean { + if (process.platform === 'win32') return false; + return spawnSync('bun', ['--version'], {stdio: 'ignore'}).status === 0; +} + +function runHarness(sequence: string): HarnessResult { + const stdout = execFileSync('bun', [harnessPath, JSON.stringify(sequence)], { + encoding: 'utf8', + timeout: 120_000, + }); + const line = stdout + .split('\n') + .reverse() + .find(candidate => candidate.startsWith('RESULT ')); + if (!line) { + throw new Error(`harness printed no RESULT line:\n${stdout}`); + } + const result = JSON.parse(line.slice('RESULT '.length)) as HarnessResult; + // A harness that never reached the session says nothing about the shortcut, + // so surface that as a harness failure rather than a shortcut verdict. + expect( + { + menuAppeared: result.menuAppeared, + sessionStarted: result.sessionStarted, + error: result.error, + tail: result.tail, + }, + 'harness failed before it could test the shortcut', + ).toEqual({ + menuAppeared: true, + sessionStarted: true, + error: undefined, + tail: undefined, + }); + return result; +} + +describe.skipIf(!canRunHarness())('return to menu shortcut (E2E)', () => { + it( + 'returns to the menu on the plain Ctrl+E control code', + {timeout: 150_000}, + () => { + expect(runHarness(SEQUENCES.controlCode).returnedToMenu).toBe(true); + }, + ); + + it( + 'returns to the menu on the kitty CSI-u form with no lock bits set', + {timeout: 150_000}, + () => { + expect(runHarness(SEQUENCES.csiUCtrl).returnedToMenu).toBe(true); + }, + ); + + // https://github.com/kbwo/ccmanager/issues/327: the lock state is not part + // of the keypress the user made, so Ctrl+E has to work exactly the same + // whether or not Num Lock or Caps Lock happens to be on. + it( + 'returns to the menu on the kitty CSI-u form with the Num Lock bit set (issue #327)', + {timeout: 150_000}, + () => { + expect(runHarness(SEQUENCES.csiUCtrlNumLock).returnedToMenu).toBe(true); + }, + ); + + it( + 'returns to the menu on the kitty CSI-u form with the Caps Lock bit set (issue #327)', + {timeout: 150_000}, + () => { + expect(runHarness(SEQUENCES.csiUCtrlCapsLock).returnedToMenu).toBe(true); + }, + ); +}); diff --git a/src/e2e/returnToMenuPtyHarness.ts b/src/e2e/returnToMenuPtyHarness.ts new file mode 100644 index 00000000..afbac89c --- /dev/null +++ b/src/e2e/returnToMenuPtyHarness.ts @@ -0,0 +1,182 @@ +/** + * PTY harness for the return-to-menu shortcut end-to-end test. + * + * It runs the real ccmanager CLI inside a pseudo terminal (PTY) against a + * throwaway git repository, selects the first worktree so a session is + * attached, writes one key sequence into the PTY, and reports whether + * ccmanager went back to the menu. + * + * This lives in its own process because the PTY is created through Bun's + * `Bun.Terminal` API (see `src/services/bunTerminal.ts`), which only exists + * when the code runs under `bun`, while the Vitest suite runs under node. + * `returnToMenu.test.ts` spawns this file with `bun` and reads the single + * `RESULT ` line printed on stdout. + * + * Usage: bun returnToMenuPtyHarness.ts '' + */ +import {execFileSync} from 'child_process'; +import {existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync} from 'fs'; +import {tmpdir} from 'os'; +import {fileURLToPath} from 'url'; +import {dirname, join} from 'path'; +import {spawn} from '../services/bunTerminal.js'; + +/** Text rendered by `Menu.tsx`; its presence means the menu is on screen. */ +const MENU_MARKER = 'CCManager - Claude Code Worktree Manager'; +/** Text printed by the fake session command once it is running. */ +const SESSION_MARKER = 'CCMANAGER_E2E_SESSION_READY'; + +const PTY_COLS = 120; +const PTY_ROWS = 40; + +const MENU_TIMEOUT_MS = 30_000; +const SESSION_TIMEOUT_MS = 30_000; +/** + * How long to wait for the menu to come back after the key sequence is sent. + * A working shortcut re-renders the menu in well under a second; this only has + * to be long enough that a slow machine is not mistaken for a swallowed key. + */ +const RETURN_TIMEOUT_MS = 5_000; + +interface HarnessResult { + /** The menu rendered, so the CLI started up correctly. */ + menuAppeared: boolean; + /** The fake session command is running and its output reaches the screen. */ + sessionStarted: boolean; + /** The menu rendered again after the key sequence was written to the PTY. */ + returnedToMenu: boolean; + /** Set when the harness itself could not complete the scenario. */ + error?: string; + /** Last chunk of PTY output, to make a harness failure diagnosable. */ + tail?: string; +} + +const sequence = JSON.parse(process.argv[2] ?? '""') as string; + +const harnessDir = dirname(fileURLToPath(import.meta.url)); +// Running from `src/` (how the test invokes it) the entry point is the TSX +// source; from a compiled `dist/` tree it is the emitted JS next to it. +const cliEntry = existsSync(join(harnessDir, '../cli.tsx')) + ? join(harnessDir, '../cli.tsx') + : join(harnessDir, '../cli.js'); + +const root = mkdtempSync(join(tmpdir(), 'ccmanager-e2e-')); +const home = join(root, 'home'); +const repoDir = join(root, 'repo'); +mkdirSync(join(home, '.config', 'ccmanager'), {recursive: true}); +mkdirSync(repoDir, {recursive: true}); + +// ccmanager reads its global config from $HOME/.config/ccmanager/config.json, +// so pointing HOME at the throwaway directory both isolates the run from the +// developer's own config and lets us replace `claude` with a fake command. +// +// The fake command prints its readiness marker in a loop instead of once: a +// single line printed at spawn time was observed never to reach the host +// terminal (neither live nor through the restore snapshot), which left the +// harness waiting forever. Repeating it guarantees a marker arrives after the +// session view has attached. `cat` runs in the foreground so it holds the +// session open and echoes back whatever ccmanager forwards to the child. +writeFileSync( + join(home, '.config', 'ccmanager', 'config.json'), + JSON.stringify({ + shortcuts: {returnToMenu: {ctrl: true, key: 'e'}, cancel: {key: 'escape'}}, + commandPresets: { + presets: [ + { + id: '1', + name: 'E2E', + command: 'sh', + args: [ + '-c', + `while :; do echo ${SESSION_MARKER}; sleep 0.5; done & cat`, + ], + }, + ], + defaultPresetId: '1', + }, + }), +); + +const git = (...args: string[]) => + execFileSync('git', args, {cwd: repoDir, encoding: 'utf8'}); +git('init', '-b', 'main'); +git('config', 'user.email', 'e2e@example.com'); +git('config', 'user.name', 'ccmanager e2e'); +writeFileSync(join(repoDir, 'README.md'), '# ccmanager e2e fixture\n'); +git('add', '.'); +git('commit', '-m', 'initial commit'); + +const env: Record = { + ...process.env, + HOME: home, + TERM: 'xterm-256color', + // Keep the run from writing into the developer's real log file. + CCMANAGER_LOG_FILE: join(root, 'ccmanager.log'), +}; +delete env['CCMANAGER_MULTI_PROJECT_ROOT']; +// Ink stops painting frames to stdout when it believes it runs in CI: it keeps +// the frame in memory and only writes it on unmount (`isInCi` branch in +// `ink/build/ink.js`, fed by the `is-in-ci` package, which checks exactly these +// two variables). This harness drives a real pseudo terminal and has to see the +// menu as a user would, so the child must not inherit them from a CI runner. +delete env['CI']; +delete env['CONTINUOUS_INTEGRATION']; + +let output = ''; +const pty = spawn('bun', [cliEntry], { + name: 'xterm-256color', + cols: PTY_COLS, + rows: PTY_ROWS, + cwd: repoDir, + env, +}); +pty.onData(data => { + output += data; +}); + +const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + +async function waitFor(marker: string, timeoutMs: number): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (output.includes(marker)) return true; + await sleep(50); + } + return false; +} + +const result: HarnessResult = { + menuAppeared: false, + sessionStarted: false, + returnedToMenu: false, +}; + +try { + result.menuAppeared = await waitFor(MENU_MARKER, MENU_TIMEOUT_MS); + if (!result.menuAppeared) throw new Error('menu never rendered'); + + // The header renders before the worktree list is populated, so wait for a + // list entry and let the initial git status refresh settle before selecting. + await waitFor('New Worktree', MENU_TIMEOUT_MS); + await sleep(2_000); + pty.write('\r'); + + result.sessionStarted = await waitFor(SESSION_MARKER, SESSION_TIMEOUT_MS); + if (!result.sessionStarted) throw new Error('session never started'); + + await sleep(500); + // Drop the session output collected so far, so the menu can only be + // detected from what is rendered after the key sequence is sent. + output = ''; + pty.write(sequence); + result.returnedToMenu = await waitFor(MENU_MARKER, RETURN_TIMEOUT_MS); +} catch (error) { + result.error = error instanceof Error ? error.message : String(error); + result.tail = JSON.stringify(output.slice(-2_000)); +} finally { + pty.kill(); + rmSync(root, {recursive: true, force: true}); +} + +console.log(`RESULT ${JSON.stringify(result)}`); +process.exit(0); diff --git a/src/services/shortcutManager.test.ts b/src/services/shortcutManager.test.ts index b2b433b6..467647f2 100644 --- a/src/services/shortcutManager.test.ts +++ b/src/services/shortcutManager.test.ts @@ -45,4 +45,76 @@ describe('shortcutManager.matchesRawInput', () => { false, ); }); + + /** + * The kitty keyboard protocol encodes the modifiers of a keypress as + * `1 + `, and that mask carries the *lock state* of the keyboard + * (64 for Caps Lock, 128 for Num Lock) alongside the modifiers the user is + * actually holding down (1 shift, 2 alt, 4 ctrl). A lock being on is not + * part of the keypress, so Ctrl+E must still be recognised as Ctrl+E. + * + * https://github.com/kbwo/ccmanager/issues/327 + */ + describe('lock state in the modifier mask (issue #327)', () => { + it('matches CSI u with the num lock bit set (1 + 4 + 128)', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[101;133u'), + ).toBe(true); + }); + + it('matches CSI u with the caps lock bit set (1 + 4 + 64)', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[101;69u'), + ).toBe(true); + }); + + it('matches CSI u with both lock bits set (1 + 4 + 64 + 128)', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[101;197u'), + ).toBe(true); + }); + + it('matches the uppercase code point with a lock bit set', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[69;133u'), + ).toBe(true); + }); + + it('matches modifyOtherKeys with a lock bit set', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[27;133;101~'), + ).toBe(true); + }); + + it('matches when the sequence is embedded in a larger chunk', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', 'ab\u001b[101;133ucd'), + ).toBe(true); + }); + + it('does not match a different key that carries a lock bit', () => { + // Ctrl+F (code point 102) while Num Lock is on. + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[102;133u'), + ).toBe(false); + }); + + it('does not match when a real modifier is added (1 + 1 shift + 4 + 128)', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[101;134u'), + ).toBe(false); + }); + + it('does not match when ctrl is absent (1 + 128)', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[101;129u'), + ).toBe(false); + }); + + it('does not match alt instead of ctrl (1 + 2 alt + 128)', () => { + expect( + shortcutManager.matchesRawInput('returnToMenu', '\u001b[101;131u'), + ).toBe(false); + }); + }); }); diff --git a/src/services/shortcutManager.ts b/src/services/shortcutManager.ts index 659c5b0f..010d9d1e 100644 --- a/src/services/shortcutManager.ts +++ b/src/services/shortcutManager.ts @@ -2,6 +2,42 @@ import {ShortcutKey, ShortcutConfig} from '../types/index.js'; import {Key} from 'ink'; import {configReader} from './config/configReader.js'; +/** + * Bits that extended keyboard protocols add to the modifier mask of a keypress + * to report the *lock state* of the keyboard rather than a key the user is + * holding down. Laptops commonly boot with Num Lock on internally even without + * a numpad, so these bits show up on ordinary shortcuts and must be ignored + * when deciding which shortcut a keypress belongs to. + * https://github.com/kbwo/ccmanager/issues/327 + */ +const CAPS_LOCK_BIT = 64; +const NUM_LOCK_BIT = 128; +const LOCK_STATE_BITS = CAPS_LOCK_BIT | NUM_LOCK_BIT; + +/** + * Kitty keyboard protocol (also used by WezTerm and Ghostty): + * `ESC [ ; u`. + * The code point may be followed by alternate key reports + * (`code:shifted:base`), the modifiers by an event type (`modifiers:event`, + * 1 press / 2 repeat / 3 release), and the whole sequence by a trailing field + * carrying the associated text code points — all optional. + * https://sw.kovidgoyal.net/kitty/keyboard-protocol/ + */ +const CSI_U_SEQUENCE = + /\u001b\[(\d+)(?::\d+)*(?:;(\d+)(?::(\d+))?)?(?:;[\d:]+)?u/g; + +/** tmux and xterm with modifyOtherKeys: `ESC [ 27 ; ; ~`. */ +const MODIFY_OTHER_KEYS_SEQUENCE = /\u001b\[27;(\d+);(\d+)~/g; + +/** + * Reported by the setups in issues #82 and #107: + * `ESC [ 1 ; `. + */ +const CSI_LETTER_SEQUENCE = /\u001b\[1;(\d+)([A-Za-z])/g; + +/** Event types that mean the key went down; 3 (release) must not trigger. */ +const KEY_DOWN_EVENT_TYPES = new Set(['1', '2']); + export class ShortcutManager { private reservedKeys: ShortcutKey[] = [ {ctrl: true, key: 'c'}, @@ -80,40 +116,84 @@ export class ShortcutManager { codes.add('\u001b'); } - // Kitty/xterm extended keyboard sequences (CSI ;u) - if ( - shortcut.ctrl && - !shortcut.alt && - !shortcut.shift && - shortcut.key.length === 1 - ) { - const lower = shortcut.key.toLowerCase(); - const upperCode = lower.toUpperCase().charCodeAt(0); - const lowerCode = lower.charCodeAt(0); + // The extended keyboard sequences (CSI u, modifyOtherKeys, CSI 1;) + // are deliberately not listed here: their modifier field varies with the + // keyboard's lock state, so they are matched by parsing the incoming bytes + // in matchesExtendedKeySequence() instead of by string comparison. - // Include the CSI u format (ESC[;5u) used by Kitty/WezTerm for Ctrl+letters. - if (upperCode >= 32 && upperCode <= 126) { - codes.add(`\u001b[${upperCode};5u`); - } - if (lowerCode !== upperCode && lowerCode >= 32 && lowerCode <= 126) { - codes.add(`\u001b[${lowerCode};5u`); - } - // Tmux/xterm with modifyOtherKeys emit ESC[27;5;~ for the same shortcut. - if (upperCode >= 32 && upperCode <= 126) { - codes.add(`\u001b[27;5;${upperCode}~`); - } - if (lowerCode !== upperCode && lowerCode >= 32 && lowerCode <= 126) { - codes.add(`\u001b[27;5;${lowerCode}~`); - } - // Some setups (issue #82/#107 repros) send ESC[1;5; include both upper/lower. - const upperKey = lower.toUpperCase(); - codes.add(`\u001b[1;5${upperKey}`); - if (upperKey !== lower) { - codes.add(`\u001b[1;5${lower}`); + return Array.from(codes); + } + + /** + * Modifier bit mask of a shortcut, in the encoding shared by the extended + * keyboard sequences: 1 shift, 2 alt, 4 ctrl. + */ + private getModifierMask(shortcut: ShortcutKey): number { + return ( + (shortcut.shift ? 1 : 0) | + (shortcut.alt ? 2 : 0) | + (shortcut.ctrl ? 4 : 0) + ); + } + + /** + * Read the modifier parameter of an extended keyboard sequence, which + * terminals report as `1 + `, and drop the keyboard's lock state + * from it. An absent parameter means "no modifiers"; anything unparseable + * yields null, which matches no shortcut. + */ + private parseModifiers(parameter: string | undefined): number | null { + if (parameter === undefined || parameter === '') return 0; + const reported = Number.parseInt(parameter, 10); + if (!Number.isInteger(reported) || reported < 1) return null; + return (reported - 1) & ~LOCK_STATE_BITS; + } + + /** + * Match the extended keyboard sequences a terminal can send for a + * Ctrl+ shortcut. They carry the modifiers as a number, so they are + * parsed instead of compared against pre-built strings: that number also + * reports the Caps Lock and Num Lock state, which is not part of the + * keypress and would otherwise keep the shortcut from ever matching + * (https://github.com/kbwo/ccmanager/issues/327). + */ + private matchesExtendedKeySequence( + shortcut: ShortcutKey, + input: string, + ): boolean { + if (!shortcut.ctrl || shortcut.alt || shortcut.shift) return false; + if (shortcut.key.length !== 1) return false; + + const expectedModifiers = this.getModifierMask(shortcut); + const lowerKey = shortcut.key.toLowerCase(); + const upperKey = lowerKey.toUpperCase(); + // Terminals differ in whether they report the shifted or the unshifted + // code point for a Ctrl+letter press, so accept either. + const codePoints = new Set([ + lowerKey.charCodeAt(0), + upperKey.charCodeAt(0), + ]); + + for (const match of input.matchAll(CSI_U_SEQUENCE)) { + const eventType = match[3]; + if (eventType !== undefined && !KEY_DOWN_EVENT_TYPES.has(eventType)) { + continue; } + if (this.parseModifiers(match[2]) !== expectedModifiers) continue; + if (codePoints.has(Number.parseInt(match[1]!, 10))) return true; } - return Array.from(codes); + for (const match of input.matchAll(MODIFY_OTHER_KEYS_SEQUENCE)) { + if (this.parseModifiers(match[1]) !== expectedModifiers) continue; + if (codePoints.has(Number.parseInt(match[2]!, 10))) return true; + } + + for (const match of input.matchAll(CSI_LETTER_SEQUENCE)) { + if (this.parseModifiers(match[1]) !== expectedModifiers) continue; + if (match[2]!.toLowerCase() === lowerKey) return true; + } + + return false; } public matchesShortcut( @@ -186,7 +266,11 @@ export class ShortcutManager { if (!shortcut) return false; const codes = this.getRawShortcutCodes(shortcut); - return codes.some(code => input === code || input.includes(code)); + if (codes.some(code => input === code || input.includes(code))) { + return true; + } + + return this.matchesExtendedKeySequence(shortcut, input); } }