From cd98242b8cd50d57c64227de34f4d9d8e821e33d Mon Sep 17 00:00:00 2001 From: Brennan Benson Date: Fri, 10 Jul 2026 01:00:18 -0700 Subject: [PATCH] fix(terminal): harden kitty Option-chord mirror for soft resets, replay redelivery, and non-QWERTY layouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on the kitty keyboard mirror, each verified against a live omp 16.3.15 pty session using the real production modules: - DECSTR: xterm's soft reset (CSI ! p) clears its kitty flags and stacks for both screens without switching buffers; the tracker now mirrors that, so a soft-resetting TUI stops receiving kitty-encoded Option chords. - Replay redelivery: relay reconnects can redeliver the retained replay window, and each scan of the app's one-time CSI > u push grew the mirrored stack while the renderer's post-replay reset drained xterm's copy. The TUI's single exit pop (omp emits a bare CSI < u on quit) then landed on a stale frame, leaving Option+B/F/D kitty-encoded in a plain shell. Replay paths now scan with scanReplay(), which applies pushes as idempotent sets so redelivery cannot grow the stack; the live-output funnel and the daemon's once-per-byte emulator keep full stack semantics. - Layout-correct base keys: kitty CSI-u reports must carry the key's unshifted codepoint in the active layout, but the encoder resolved it from a US-QWERTY physical-code table — on Dvorak/Colemak/AZERTY-class layouts (exactly the population whose effective Option-as-Alt default activates this path) the wrong key's chord fired, e.g. Colemak Option+P sent alt+r. A new keyboard-layout module caches Chromium's KeyboardLayoutMap (fetched at terminal setup and on focus-in, like the option-as-alt probe) and the policy resolves through it before the US fallback. Verified live: the layout-resolved bytes open omp's model selector; the US-table bytes do not. --- src/main/daemon/headless-emulator.test.ts | 20 +++++ .../terminal-pane/keyboard-handlers.ts | 13 +++- .../terminal-pane/pty-connection.ts | 13 ++-- .../terminal-shortcut-policy.test.ts | 59 ++++++++++++++ .../terminal-pane/terminal-shortcut-policy.ts | 11 ++- .../layout-base-character.test.ts | 45 +++++++++++ .../keyboard-layout/layout-base-character.ts | 78 +++++++++++++++++++ ...rminal-kitty-keyboard-mode-tracker.test.ts | 49 ++++++++++++ .../terminal-kitty-keyboard-mode-tracker.ts | 53 +++++++++++-- 9 files changed, 325 insertions(+), 16 deletions(-) create mode 100644 src/renderer/src/lib/keyboard-layout/layout-base-character.test.ts create mode 100644 src/renderer/src/lib/keyboard-layout/layout-base-character.ts diff --git a/src/main/daemon/headless-emulator.test.ts b/src/main/daemon/headless-emulator.test.ts index c633d232f08..0a5832247c6 100644 --- a/src/main/daemon/headless-emulator.test.ts +++ b/src/main/daemon/headless-emulator.test.ts @@ -518,6 +518,26 @@ describe('HeadlessEmulator', () => { expect(snapshot.modes.kittyKeyboardFlags).toBe(0) expect(snapshot.rehydrateSequences).not.toContain('u') }) + + it('re-arms kitty flags after the alt-screen switch so they land on the negotiated screen', async () => { + emulator = new HeadlessEmulator({ cols: 80, rows: 24 }) + await emulator.write('\x1b[?1049h\x1b[>1u') + + const snapshot = emulator.getSnapshot() + const altScreenIndex = snapshot.rehydrateSequences.indexOf('\x1b[?1049h') + const kittyIndex = snapshot.rehydrateSequences.indexOf('\x1b[=1;1u') + expect(altScreenIndex).toBeGreaterThanOrEqual(0) + expect(kittyIndex).toBeGreaterThan(altScreenIndex) + }) + + it('drops kitty rehydration after a TUI soft reset (DECSTR)', async () => { + emulator = new HeadlessEmulator({ cols: 80, rows: 24 }) + await emulator.write('\x1b[>1u') + await emulator.write('\x1b[!p') + + const snapshot = emulator.getSnapshot() + expect(snapshot.modes.kittyKeyboardFlags).toBe(0) + }) }) describe('dispose', () => { diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index befeb9f4c7f..b5bf46b7d0c 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -16,6 +16,10 @@ import { import type { PaneCwdMap } from './resolve-split-cwd' import type { TerminalKittyKeyboardModeTracker } from '../../../../shared/terminal-kitty-keyboard-mode-tracker' import { keyboardEventBelongsToScope } from './terminal-keyboard-scope' +import { + getLayoutBaseCharacterForCode, + prefetchLayoutBaseCharacters +} from '@/lib/keyboard-layout/layout-base-character' import { normalizeSelectedTextForFileSearch } from '@/lib/file-search-selection' import { isFindQueryTooLarge } from '@/lib/find-query-bounds' import { handleEmptyFloatingWorkspacePanelCloseShortcut } from '@/lib/floating-workspace-terminal-actions' @@ -209,6 +213,12 @@ export function useTerminalKeyboardShortcuts({ const isWindows = navigator.userAgent.includes('Windows') const shortcutPlatform: KeybindingPlatform = isMac ? 'darwin' : isWindows ? 'win32' : 'linux' + // Why: kitty Option-chord encoding resolves base keys through the async + // KeyboardLayoutMap; prefetch so the map is cached before the first chord. + if (isMac) { + prefetchLayoutBaseCharacters() + } + // Why: KeyboardEvent.location on a character key (e.g. Period) always // reports that key's own position (0 = standard), not which modifier is // held. To distinguish left vs right Option, we record the Option key's @@ -315,7 +325,8 @@ export function useTerminalKeyboardShortcuts({ isWindows, keybindings, isLocalWindowsConptyPane, - isKittyKeyboardActivePane + isKittyKeyboardActivePane, + getLayoutBaseCharacterForCode ) if (!action) { return diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index ae6225505d3..a83648b0e6d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -4135,8 +4135,10 @@ export function connectPanePty( rememberReattachPayloadAgentSignal(data, { fullScreenReplay: clearBeforeReplay }) } // Why: replayed application bytes carry the live TUI's kitty keyboard - // negotiation; the mirror must re-arm from them after a reload. - kittyKeyboardModes.scan(data) + // negotiation; the mirror must re-arm from them after a reload. Replay + // semantics: relay reconnects redeliver the same window, so pushes + // apply as sets to keep the mirrored stack from accumulating frames. + kittyKeyboardModes.scanReplay(data) await writeReplayDataAsync(data) if (clearBeforeReplay || data.length > 0) { await writeReplayDataAsync(reattachReplayResetSequence(data)) @@ -5532,7 +5534,7 @@ export function connectPanePty( // Why: the daemon snapshot's rehydrate preamble carries the live // session's kitty keyboard flags; re-arm the mirror from it so Option // chords keep their kitty encoding after a window reload. - kittyKeyboardModes.scan(connectResult.snapshot) + kittyKeyboardModes.scanReplay(connectResult.snapshot) writeReplayData(connectResult.snapshot) // Snapshot reattach keeps a live session, so avoid the broader mode // reset. We only drop renderer-owned state that should not leak from @@ -5560,8 +5562,9 @@ export function connectPanePty( // tearing down the still-running TUI's live modes. writeReplayData('\x1b[2J\x1b[3J\x1b[H') // Why: raw relay replay contains the application's own kitty pushes - // when they fall inside the retained window; re-arm the mirror. - kittyKeyboardModes.scan(connectResult.replay) + // when they fall inside the retained window; re-arm the mirror with + // replay (set) semantics so redelivery cannot grow the stack. + kittyKeyboardModes.scanReplay(connectResult.replay) writeReplayData(connectResult.replay) writeReplayData(reattachReplayResetSequence(connectResult.replay)) sendFocusedReattachFocusInAfterReplay() diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts index 18584945674..8bf8159d9d1 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts @@ -571,6 +571,26 @@ describe('kitty keyboard protocol panes', () => { expect(resolveKitty(event({ key: 'p', code: 'KeyP', altKey: true }), 'true')).toBeNull() }) + it('keeps shift+Option composition untouched in non-kitty panes', () => { + expect( + resolveKitty( + event({ key: '∏', code: 'KeyP', altKey: true, shiftKey: true }), + 'false', + 0, + kittyInactive + ) + ).toBeNull() + // Meta-side Option in 'left' mode stays shift-exempt without kitty. + expect( + resolveKitty( + event({ key: '∏', code: 'KeyP', altKey: true, shiftKey: true }), + 'left', + 1, + kittyInactive + ) + ).toBeNull() + }) + it('keeps compose-mode behavior unchanged when the pane is not kitty-active', () => { expect( resolveKitty(event({ key: 'π', code: 'KeyP', altKey: true }), 'false', 0, kittyInactive) @@ -621,4 +641,43 @@ describe('kitty keyboard protocol panes', () => { expect(resolveKitty(event({ key: 'π', code: 'KeyP', altKey: true, metaKey: true }))).toBeNull() expect(resolveKitty(event({ key: 'π', code: 'KeyP', altKey: true, ctrlKey: true }))).toBeNull() }) + + it('resolves the kitty base key through the active layout map when provided', () => { + const resolveWithLayout = ( + input: TerminalShortcutEvent, + layoutBaseCharacterForCode: (code: string) => string | undefined + ) => + resolveTerminalShortcutAction( + input, + true, + 'false', + 0, + false, + undefined, + undefined, + kittyActive, + layoutBaseCharacterForCode + ) + + // AZERTY types M at the physical Semicolon position; the layout map must + // win over the US punctuation table so the chord reports alt+m, not alt+;. + const azerty = (code: string): string | undefined => (code === 'Semicolon' ? 'm' : undefined) + expect(resolveWithLayout(event({ key: 'µ', code: 'Semicolon', altKey: true }), azerty)).toEqual( + { type: 'sendInput', data: '\x1b[109;3u' } + ) + + // Colemak types P at the physical KeyR position. + const colemak = (code: string): string | undefined => (code === 'KeyR' ? 'p' : undefined) + expect(resolveWithLayout(event({ key: 'π', code: 'KeyR', altKey: true }), colemak)).toEqual({ + type: 'sendInput', + data: '\x1b[112;3u' + }) + + // Falls back to the US table when the layout map has no entry. + const empty = (): string | undefined => undefined + expect(resolveWithLayout(event({ key: 'π', code: 'KeyP', altKey: true }), empty)).toEqual({ + type: 'sendInput', + data: '\x1b[112;3u' + }) + }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts index 749213f7f28..69bc914d048 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -82,7 +82,12 @@ export function resolveTerminalShortcutAction( // Why: lazily reports whether the active pane's application has enabled the // kitty keyboard protocol (CSI > u). Gates the Option-as-Alt compensation // below on the application's own opt-in, so shells keep composition. - isKittyKeyboardActivePane?: () => boolean + isKittyKeyboardActivePane?: () => boolean, + // Why: kitty key reports carry the key's unshifted codepoint in the active + // layout; the physical-code table above is US QWERTY and reports the wrong + // key on Dvorak/Colemak/AZERTY-class layouts. This resolves through + // Chromium's KeyboardLayoutMap when it is available. + layoutBaseCharacterForCode?: (code: string) => string | undefined ): TerminalShortcutAction | null { const platform: NodeJS.Platform = isMac ? 'darwin' : isWindows ? 'win32' : 'linux' if (!event.repeat) { @@ -286,7 +291,9 @@ export function resolveTerminalShortcutAction( // for any single letter); the other key composes, with B/F/D compensated. if (isMac && !event.metaKey && !event.ctrlKey && event.altKey && macOptionAsAlt !== 'true') { if (event.key !== 'Dead' && isKittyKeyboardActivePane?.()) { - const baseCharacter = resolveUnshiftedCharacterForCode(event.code) + const baseCharacter = + (event.code ? layoutBaseCharacterForCode?.(event.code) : undefined) ?? + resolveUnshiftedCharacterForCode(event.code) if (baseCharacter) { return { type: 'sendInput', diff --git a/src/renderer/src/lib/keyboard-layout/layout-base-character.test.ts b/src/renderer/src/lib/keyboard-layout/layout-base-character.test.ts new file mode 100644 index 00000000000..f4cf3fb943e --- /dev/null +++ b/src/renderer/src/lib/keyboard-layout/layout-base-character.test.ts @@ -0,0 +1,45 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { + _setLayoutMapForTests, + getLayoutBaseCharacterForCode, + normalizeLayoutBaseCharacter +} from './layout-base-character' + +describe('normalizeLayoutBaseCharacter', () => { + it('accepts a single printable codepoint, lowercased', () => { + expect(normalizeLayoutBaseCharacter('p')).toBe('p') + expect(normalizeLayoutBaseCharacter('P')).toBe('p') + expect(normalizeLayoutBaseCharacter('ö')).toBe('ö') + expect(normalizeLayoutBaseCharacter(';')).toBe(';') + }) + + it('rejects empty, named-key, multi-codepoint, and control values', () => { + expect(normalizeLayoutBaseCharacter(undefined)).toBeUndefined() + expect(normalizeLayoutBaseCharacter('')).toBeUndefined() + expect(normalizeLayoutBaseCharacter('Dead')).toBeUndefined() + expect(normalizeLayoutBaseCharacter('\t')).toBeUndefined() + expect(normalizeLayoutBaseCharacter(' ')).toBeUndefined() + }) +}) + +describe('getLayoutBaseCharacterForCode', () => { + afterEach(() => { + _setLayoutMapForTests(null) + }) + + it('returns undefined without a cached map, and resolves through one', () => { + expect(getLayoutBaseCharacterForCode('KeyP')).toBeUndefined() + + const azertyEntries = new Map([ + ['Semicolon', 'm'], + ['KeyE', 'Dead'] + ]) + _setLayoutMapForTests({ + get: (code) => azertyEntries.get(code), + size: azertyEntries.size + }) + expect(getLayoutBaseCharacterForCode('Semicolon')).toBe('m') + expect(getLayoutBaseCharacterForCode('KeyE')).toBeUndefined() + expect(getLayoutBaseCharacterForCode('KeyZ')).toBeUndefined() + }) +}) diff --git a/src/renderer/src/lib/keyboard-layout/layout-base-character.ts b/src/renderer/src/lib/keyboard-layout/layout-base-character.ts new file mode 100644 index 00000000000..d712d897ebd --- /dev/null +++ b/src/renderer/src/lib/keyboard-layout/layout-base-character.ts @@ -0,0 +1,78 @@ +/** + * Synchronous lookup of the active keyboard layout's base (unshifted) + * character for a physical `KeyboardEvent.code`. + * + * Why: kitty keyboard CSI-u reports must carry the codepoint of the key in + * the *current layout* with no modifiers. Deriving it from the physical code + * alone assumes US QWERTY and reports the wrong key on Dvorak, Colemak, + * AZERTY, QWERTZ, etc. — misfiring TUI hotkeys. Chromium's KeyboardLayoutMap + * is the layout-true source, but it resolves asynchronously, so this module + * prefetches it and refreshes on window focus-in (every macOS layout-switch + * path blurs and refocuses the window; Chromium has no layoutchange event — + * see option-as-alt-probe.ts). + */ +import type { LayoutMapLike } from './detect-option-as-alt' + +type NavigatorWithKeyboard = Navigator & { + keyboard?: { + getLayoutMap: () => Promise + } +} + +let cachedLayoutMap: LayoutMapLike | null = null +let focusListenerAttached = false + +async function refreshLayoutMap(): Promise { + const keyboard = (window.navigator as NavigatorWithKeyboard).keyboard + if (!keyboard?.getLayoutMap) { + return + } + try { + cachedLayoutMap = await keyboard.getLayoutMap() + } catch { + // Why: getLayoutMap can transiently reject; keep the last known map + // instead of dropping layout awareness mid-session. + } +} + +/** Idempotent. Kicks off the initial fetch and keeps the cache fresh across + * layout switches. Call from terminal keyboard setup so the map is resolved + * before the first Option chord. */ +export function prefetchLayoutBaseCharacters(): void { + if (focusListenerAttached || typeof window === 'undefined') { + return + } + focusListenerAttached = true + window.addEventListener('focus', () => { + void refreshLayoutMap() + }) + void refreshLayoutMap() +} + +/** A layout map entry is usable as a kitty base key only if it is a single + * printable codepoint (dead keys report names like 'Dead'; some entries are + * empty). Exposed for tests. */ +export function normalizeLayoutBaseCharacter(value: string | undefined): string | undefined { + if (!value) { + return undefined + } + const lowered = value.toLowerCase() + const codePoints = [...lowered] + if (codePoints.length !== 1) { + return undefined + } + const codePoint = lowered.codePointAt(0) as number + return codePoint <= 0x20 ? undefined : lowered +} + +/** The active layout's unshifted character for a physical key code, or + * undefined when the map is unavailable or the key has no single printable + * base character (callers fall back to the US table). */ +export function getLayoutBaseCharacterForCode(code: string): string | undefined { + return normalizeLayoutBaseCharacter(cachedLayoutMap?.get(code)) +} + +/** Test-only: replace or clear the cached layout map. */ +export function _setLayoutMapForTests(map: LayoutMapLike | null): void { + cachedLayoutMap = map +} diff --git a/src/shared/terminal-kitty-keyboard-mode-tracker.test.ts b/src/shared/terminal-kitty-keyboard-mode-tracker.test.ts index cf4b8746cea..6d88f56a070 100644 --- a/src/shared/terminal-kitty-keyboard-mode-tracker.test.ts +++ b/src/shared/terminal-kitty-keyboard-mode-tracker.test.ts @@ -95,4 +95,53 @@ describe('TerminalKittyKeyboardModeTracker', () => { tracker.scan('\x1b[?1049l') expect(tracker.flags).toBe(0) }) + + it('clears kitty state on DECSTR (CSI ! p) like xterm, without switching screens', () => { + const tracker = new TerminalKittyKeyboardModeTracker() + tracker.scan('\x1b[>1u\x1b[!p') + expect(tracker.flags).toBe(0) + + // xterm's soft reset wipes both screens' slots but stays on the current + // buffer; a later alt-screen exit must not resurrect pre-reset flags. + const onAlt = new TerminalKittyKeyboardModeTracker() + onAlt.scan('\x1b[>1u\x1b[?1049h\x1b[>2u') + expect(onAlt.flags).toBe(2) + onAlt.scan('\x1b[!p') + expect(onAlt.flags).toBe(0) + onAlt.scan('\x1b[?1049l') + expect(onAlt.flags).toBe(0) + }) + + it('handles DECSTR split across chunks', () => { + const tracker = new TerminalKittyKeyboardModeTracker() + tracker.scan('\x1b[>1u\x1b[!') + expect(tracker.flags).toBe(1) + tracker.scan('p') + expect(tracker.flags).toBe(0) + }) + + it('applies replayed pushes as sets so redelivered windows cannot grow the stack', () => { + const tracker = new TerminalKittyKeyboardModeTracker() + // Live negotiation, then two relay reconnects redelivering the same + // retained window containing the app's one-time push. + tracker.scan('\x1b[>1u') + tracker.scanReplay('\x1b[>1u') + tracker.scanReplay('\x1b[>1u') + expect(tracker.flags).toBe(1) + // The TUI's single exit pop must drain to zero despite the redeliveries. + tracker.scan('\x1b[ { + const fresh = new TerminalKittyKeyboardModeTracker() + fresh.scanReplay('\x1b[>1u') + expect(fresh.flags).toBe(1) + fresh.scan('\x1b[1uoutput\x1b[ u push, CSI < u pop, * CSI = u set) by scanning the raw PTY output stream, replicating xterm's * exact stack/screen algorithm including the per-screen flag slots swapped by - * DECSET/DECRST 47/1047/1049 and the full reset on RIS. + * DECSET/DECRST 47/1047/1049, the full reset on RIS, and the soft reset on + * DECSTR (CSI ! p). * * Why a mirror instead of reading xterm's internal state: Orca defensively * wipes the renderer terminal's kitty flags at moments when the TUI may have @@ -46,10 +47,29 @@ export class TerminalKittyKeyboardModeTracker { } scan(data: string): void { + this.scanInternal(data, false) + } + + /** + * Scan bytes replayed from a retained history window (reattach payloads, + * relay replays, daemon snapshots). Replays can redeliver the application's + * one-time CSI > u push — applying it with stack semantics on every delivery + * grows the mirrored stack, so the TUI's eventual single pop lands on a + * stale frame and Option chords stay kitty-encoded in a plain shell. Pushes + * seen during replay therefore apply as idempotent sets. Known limit: a + * NESTED push/push/pop inside the window collapses to flags 0 on the pop — + * unavoidable stackless-replay tradeoff (a redelivered push is byte-wise + * indistinguishable from a new one); real TUIs push once at startup. + */ + scanReplay(data: string): void { + this.scanInternal(data, true) + } + + private scanInternal(data: string, replay: boolean): void { const input = this.scanTail + data this.scanTail = this.extractScanTail(input) // oxlint-disable-next-line no-control-regex -- terminal escape sequences require control chars - const kittyModeRe = /\x1bc|(?:\x1b\[|\x9b)(?:\?([0-9;]+)([hl])|([<>=])([0-9;]*)u)/g + const kittyModeRe = /\x1bc|(?:\x1b\[|\x9b)(?:!p|\?([0-9;]+)([hl])|([<>=])([0-9;]*)u)/g let match: RegExpExecArray | null while ((match = kittyModeRe.exec(input)) !== null) { if (match[0] === '\x1bc') { @@ -59,14 +79,29 @@ export class TerminalKittyKeyboardModeTracker { this.scanTail = tail continue } + if (match[0].endsWith('!p')) { + this.applySoftReset() + continue + } if (match[1] !== undefined) { this.applyScreenSwitch(match[1], match[2] === 'h') continue } - this.applyKittySequence(match[3], match[4] ?? '') + this.applyKittySequence(match[3], match[4] ?? '', replay) } } + private applySoftReset(): void { + // Why: xterm's DECSTR (CSI ! p) wipes kitty flags and stacks for both + // screens via coreService.reset but does not switch buffers — mirror that + // so a soft-resetting TUI stops receiving kitty-encoded Option chords. + this.currentFlags = 0 + this.mainFlags = 0 + this.altFlags = 0 + this.mainStack = [] + this.altStack = [] + } + private applyScreenSwitch(params: string, enabled: boolean): void { for (const rawParam of params.split(';')) { const param = Number(rawParam) @@ -88,14 +123,16 @@ export class TerminalKittyKeyboardModeTracker { } } - private applyKittySequence(prefix: string, params: string): void { + private applyKittySequence(prefix: string, params: string, replay: boolean): void { const parsed = params.split(';').map((entry) => Number(entry)) const stack = this.alternateScreenActive ? this.altStack : this.mainStack if (prefix === '>') { - if (stack.length >= KITTY_STACK_LIMIT) { - stack.shift() + if (!replay) { + if (stack.length >= KITTY_STACK_LIMIT) { + stack.shift() + } + stack.push(this.currentFlags) } - stack.push(this.currentFlags) this.currentFlags = parsed[0] || 0 return } @@ -144,6 +181,6 @@ export class TerminalKittyKeyboardModeTracker { } private isIncompleteSequenceBody(body: string): boolean { - return /^[<>=?]?[0-9;]*$/.test(body) + return body === '!' || /^[<>=?]?[0-9;]*$/.test(body) } }