From 4ae8e80d12f2c5d7072dc9f192ac1f18650dd09a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 4 Aug 2026 00:35:00 -0700 Subject: [PATCH] fix(terminal): gate Ctrl+Enter CSI-u on a negotiated kitty pane Ctrl+Enter emitted \x1b[13;5u unconditionally, so a pane that never negotiated the kitty keyboard protocol (local Windows ConPTY, plain shell) printed the escape verbatim into the prompt. Mirror the Shift+Enter guard and fall back to the legacy CR every emulator sends for this chord. Keeps the intercept, so IME commit ordering and the single-send dedupe still apply. Fixes #12329 Co-authored-by: Orca --- .../preview-terminal-shortcuts.test.ts | 13 +++++ .../terminal-shortcut-policy.test.ts | 47 ++++++++++++------- .../terminal-pane/terminal-shortcut-policy.ts | 8 +++- ...an-ime-terminal-shift-enter-commit.spec.ts | 23 ++++----- tests/e2e/terminal-shortcuts.spec.ts | 6 ++- 5 files changed, 67 insertions(+), 30 deletions(-) diff --git a/src/renderer/src/components/dashboard-popout/preview-terminal-shortcuts.test.ts b/src/renderer/src/components/dashboard-popout/preview-terminal-shortcuts.test.ts index b76c7c14699..9f3f686c3a7 100644 --- a/src/renderer/src/components/dashboard-popout/preview-terminal-shortcuts.test.ts +++ b/src/renderer/src/components/dashboard-popout/preview-terminal-shortcuts.test.ts @@ -105,6 +105,19 @@ describe('resolvePreviewShortcutAction', () => { ).toEqual({ type: 'sendInput', data: '\x1b\r' }) }) + it('gates the Ctrl+Enter CSI-u chord on the preview pty kitty flags', () => { + expect( + resolvePreviewShortcutAction( + keydown({ key: 'Enter', ctrlKey: true }), + contextFor({ kitty: true }) + ) + ).toEqual({ type: 'sendInput', data: '\x1b[13;5u' }) + // Why: kittyKeyboardAdvertised is only an advertisement — live flags decide (#12329). + expect( + resolvePreviewShortcutAction(keydown({ key: 'Enter', ctrlKey: true }), contextFor()) + ).toEqual({ type: 'sendInput', data: '\r' }) + }) + it('reports pane-scoped chords so the caller can swallow them', () => { expect( resolvePreviewShortcutAction(keydown({ key: 'd', code: 'KeyD', metaKey: true }), contextFor()) 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 2736d5e0797..a0f7e9a4e28 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 @@ -270,27 +270,42 @@ describe('resolveTerminalShortcutAction', () => { expect(getWindowsShiftEnterEncoding).toHaveBeenCalledTimes(2) }) - it('forwards Ctrl+Enter as the kitty CSI-u chord so TUIs can cue instead of send', () => { - // Why: xterm.js collapses Ctrl+Enter to a bare CR; intercept upstream and - // emit the kitty sequence (modifier code 5 = Ctrl) so probing TUIs receive - // the distinct chord on every platform. - expect( - resolveTerminalShortcutAction(event({ key: 'Enter', code: 'Enter', ctrlKey: true }), true) - ).toEqual({ type: 'sendInput', data: '\x1b[13;5u' }) - expect( - resolveTerminalShortcutAction(event({ key: 'Enter', code: 'Enter', ctrlKey: true }), false) - ).toEqual({ type: 'sendInput', data: '\x1b[13;5u' }) - // Windows uses the same kitty sequence for now: no TUI is known to treat the - // CSI-u Ctrl+Enter form as inert (cf. the Shift+Enter Codex-on-PowerShell case). - expect( + it('forwards Ctrl+Enter as the kitty CSI-u chord only on a negotiated kitty pane', () => { + // Why: xterm.js collapses Ctrl+Enter to a bare CR; intercept upstream and emit the + // kitty sequence (modifier code 5 = Ctrl) so probing TUIs receive the distinct chord. + // Why: #12329 — CSI-u is application input; a pane that never negotiated it (local + // Windows ConPTY, plain shell) prints it verbatim, so send the legacy CR instead. + const getWindowsShiftEnterEncoding = vi.fn(() => 'csi-u' as const) + const csiU = { type: 'sendInput', data: '\x1b[13;5u' } + const legacyCr = { type: 'sendInput', data: '\r' } + const resolveCtrlEnter = (isMac: boolean, isWindows: boolean, kittyActive: boolean) => resolveTerminalShortcutAction( event({ key: 'Enter', code: 'Enter', ctrlKey: true }), - false, + isMac, 'false', 0, - true + isWindows, + undefined, + undefined, + () => kittyActive, + undefined, + getWindowsShiftEnterEncoding, + () => true ) - ).toEqual({ type: 'sendInput', data: '\x1b[13;5u' }) + for (const [isMac, isWindows] of [ + [true, false], + [false, false], + [false, true] + ] as const) { + expect(resolveCtrlEnter(isMac, isWindows, true)).toEqual(csiU) + expect(resolveCtrlEnter(isMac, isWindows, false)).toEqual(legacyCr) + } + // A Droid pane's Shift+Enter encoding is no authority over Ctrl+Enter. + expect(getWindowsShiftEnterEncoding).not.toHaveBeenCalled() + // Absent tracker (surfaces that report no flags) must not send CSI-u either. + expect( + resolveTerminalShortcutAction(event({ key: 'Enter', code: 'Enter', ctrlKey: true }), false) + ).toEqual(legacyCr) // Modifier combos that are NOT plain Ctrl+Enter must keep falling through. expect( 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 57cc3eb4de6..ec59e15189d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -195,8 +195,12 @@ export function resolveTerminalShortcutAction( !event.shiftKey && event.key === 'Enter' ) { - // Why: xterm.js collapses Ctrl+Enter to a bare CR, so forward kitty CSI-u (modifier 5 = Ctrl) so the chord reaches TUIs; no Windows fallback yet (#2418). - return { type: 'sendInput', data: '\x1b[13;5u' } + // Why: CSI-u (modifier 5 = Ctrl) only reads as a chord while KKP is negotiated; without it the pane prints it verbatim (#12329). + // Why: fall back to the CR xterm.js would have collapsed to, but keep intercepting so IME commit ordering and dedupe still apply. + return { + type: 'sendInput', + data: isKittyKeyboardActivePane?.() === true ? '\x1b[13;5u' : '\r' + } } if ( diff --git a/tests/e2e/korean-ime-terminal-shift-enter-commit.spec.ts b/tests/e2e/korean-ime-terminal-shift-enter-commit.spec.ts index 4c66fea3c5c..2c25306a89e 100644 --- a/tests/e2e/korean-ime-terminal-shift-enter-commit.spec.ts +++ b/tests/e2e/korean-ime-terminal-shift-enter-commit.spec.ts @@ -379,24 +379,25 @@ async function assertShiftOutcome(page: Page): Promise { ]) } +// Why: this harness never negotiates the kitty protocol, so Ctrl+Enter falls back to +// the legacy CR (#12329) instead of printing CSI-u verbatim into the prompt. async function assertCtrlOutcome(page: Page): Promise { await expect .poll(() => readReceived(page), { timeout: 10_000, message: 'PTY bytes must contain committed Hangul before exactly one Ctrl+Enter chord' }) - .toBe('하 하 하\u001b[13;5u') + .toBe('하 하 하\r') await expect .poll(() => readPromptLine(page), { timeout: 10_000, - message: 'prompt must show the committed syllables followed by exactly one CSI-u chord' + message: 'prompt must be empty — no literal escape bytes may survive the chord' }) - .toBe('하 하 하[13;5u') + .toBe('') await page.waitForTimeout(500) - expect(await readPromptLine(page), 'Ctrl+Enter must produce exactly one CSI-u chord').toBe( - '하 하 하[13;5u' - ) - expect(await readSubmitted(page), 'CSI-u must not submit the line').toEqual([]) + expect(await readSubmitted(page), 'Ctrl+Enter must produce exactly one newline').toEqual([ + '하 하 하' + ]) } const COMMITTING_ENTER_CHORDS: CommittingEnterChordCase[] = [ @@ -416,8 +417,8 @@ const COMMITTING_ENTER_CHORDS: CommittingEnterChordCase[] = [ modifiers: 2, assertOutcome: assertCtrlOutcome, expectedAfterPlainEnter: { - received: '하 하 하\u001b[13;5u\r', - submitted: ['하 하 하\u001b[13;5u'] + received: '하 하 하\r\r', + submitted: ['하 하 하', ''] } }, { @@ -455,8 +456,8 @@ const COMMITTING_ENTER_CHORDS: CommittingEnterChordCase[] = [ windowsOnly: true, assertOutcome: assertCtrlOutcome, expectedAfterPlainEnter: { - received: '하 하 하\u001b[13;5u\r', - submitted: ['하 하 하\u001b[13;5u'] + received: '하 하 하\r\r', + submitted: ['하 하 하', ''] } } ] diff --git a/tests/e2e/terminal-shortcuts.spec.ts b/tests/e2e/terminal-shortcuts.spec.ts index 5c921376ca5..a72c2571cc5 100644 --- a/tests/e2e/terminal-shortcuts.spec.ts +++ b/tests/e2e/terminal-shortcuts.spec.ts @@ -526,13 +526,17 @@ test.describe('Terminal Shortcuts', () => { await pressAndExpectWrite(orcaPage, electronApp, 'Control+Alt+;', '\x1b;') }) - test('Ctrl+Enter writes the kitty modified-enter chord for terminal TUIs', async ({ + test('Ctrl+Enter writes the kitty modified-enter chord only once the TUI negotiates it', async ({ orcaPage, electronApp }) => { await installMainProcessPtyWriteSpy(electronApp) await waitForActivePanePtyId(orcaPage) + // Why: a plain shell never negotiates kitty, so CSI-u would print verbatim (#12329). + await pressAndExpectWrite(orcaPage, electronApp, 'Control+Enter', '\r') + + await enableKittyKeyboardReporting(orcaPage, 31) await pressAndExpectWrite(orcaPage, electronApp, 'Control+Enter', '\x1b[13;5u') })