From 130ae577acd76ea2f040f2e357b0f3e730a53883 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Sun, 7 Jun 2026 17:04:19 +0200 Subject: [PATCH] fix(tui,coding-agent): make keyboard protocol fallback response-driven closes #5188 --- packages/coding-agent/docs/terminal-setup.md | 38 +++++- packages/tui/CHANGELOG.md | 1 + packages/tui/src/terminal.ts | 116 ++++++------------- packages/tui/test/key-tester.ts | 38 ++++-- packages/tui/test/terminal.test.ts | 91 +++++++++------ 5 files changed, 156 insertions(+), 128 deletions(-) diff --git a/packages/coding-agent/docs/terminal-setup.md b/packages/coding-agent/docs/terminal-setup.md index cccf683fc..ff1638b5e 100644 --- a/packages/coding-agent/docs/terminal-setup.md +++ b/packages/coding-agent/docs/terminal-setup.md @@ -40,7 +40,7 @@ If you want `Shift+Enter` to keep working in tmux via that remap, add `ctrl+j` t ## WezTerm -Create `~/.wezterm.lua`: +WezTerm usually works out of the box for `Shift+Enter` via xterm modifyOtherKeys. To use the Kitty keyboard protocol explicitly, create `~/.wezterm.lua`: ```lua local wezterm = require 'wezterm' @@ -49,16 +49,50 @@ config.enable_kitty_keyboard = true return config ``` +On macOS, WezTerm binds `Option+Enter` to fullscreen by default. To use `Option+Enter` for pi follow-up queueing, add this key override: + +```lua +local wezterm = require 'wezterm' +local config = wezterm.config_builder() +config.keys = { + { + key = 'Enter', + mods = 'ALT', + action = wezterm.action.SendString('\x1b[13;3u'), + }, +} +return config +``` + +If you already have a `config.keys` table, add the entry to it. + On WSL, WezTerm may require a visible hardware cursor for IME candidate window positioning. If CJK IME candidates do not follow the text cursor, set `PI_HARDWARE_CURSOR=1` before running pi or set `showHardwareCursor` to `true` in settings. +## Alacritty + +Alacritty usually works out of the box for `Shift+Enter`. On macOS, `Option+Enter` may arrive as plain `Enter`. To use `Option+Enter` for pi follow-up queueing, add to `~/.config/alacritty/alacritty.toml`: + +```toml +[[keyboard.bindings]] +key = "Enter" +mods = "Alt" +chars = "\u001b[13;3u" +``` + +Restart Alacritty after changing the config. + ## VS Code (Integrated Terminal) +VS Code 1.109.5 and newer enable Kitty keyboard protocol in the integrated terminal by default, so `Shift+Enter` should work out of the box. + +VS Code versions older than 1.109.5 need an explicit terminal keybinding for `Shift+Enter`. + `keybindings.json` locations: - macOS: `~/Library/Application Support/Code/User/keybindings.json` - Linux: `~/.config/Code/User/keybindings.json` - Windows: `%APPDATA%\\Code\\User\\keybindings.json` -Add to `keybindings.json` to enable `Shift+Enter` for multi-line input: +Add to `keybindings.json`: ```json { diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index dd487722b..3682b5293 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed prompt history navigation to place the cursor at the start when browsing upward and at the end when browsing downward, so repeated Up/Down traverses multiline prompts immediately ([#5454](https://github.com/earendil-works/pi/issues/5454)). +- Fixed intermittent Shift+Enter handling by making Kitty keyboard protocol fallback response-driven instead of timeout-driven ([#5188](https://github.com/earendil-works/pi/issues/5188)). ## [0.78.1] - 2026-06-04 diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 563537d70..2542286ae 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -13,7 +13,6 @@ const TERMINAL_PROGRESS_ACTIVE_SEQUENCE = "\x1b]9;4;3\x07"; const TERMINAL_PROGRESS_CLEAR_SEQUENCE = "\x1b]9;4;0;\x07"; const APPLE_TERMINAL_SHIFT_ENTER_SEQUENCE = "\x1b[13;2u"; const DESIRED_KITTY_KEYBOARD_PROTOCOL_FLAGS = 7; -const KITTY_KEYBOARD_PROTOCOL_FALLBACK_TIMEOUT_MS = 150; const KEYBOARD_PROTOCOL_RESPONSE_FRAGMENT_TIMEOUT_MS = 150; const KITTY_KEYBOARD_PROTOCOL_QUERY = `\x1b[>${DESIRED_KITTY_KEYBOARD_PROTOCOL_FLAGS}u\x1b[?u\x1b[c`; @@ -34,8 +33,8 @@ export function parseKeyboardProtocolNegotiationSequence( return undefined; } -function isKeyboardProtocolNegotiationSequencePrefix(sequence: string, allowBareEscapePrefix: boolean): boolean { - return (allowBareEscapePrefix && sequence === "\x1b") || sequence === "\x1b[" || /^\x1b\[\?[\d;]*$/.test(sequence); +function isKeyboardProtocolNegotiationSequencePrefix(sequence: string): boolean { + return sequence === "\x1b[" || /^\x1b\[\?[\d;]*$/.test(sequence); } export function isAppleTerminalSession(): boolean { @@ -104,10 +103,7 @@ export class ProcessTerminal implements Terminal { private _kittyProtocolActive = false; private _modifyOtherKeysActive = false; private keyboardProtocolPushed = false; - private keyboardProtocolNegotiationPending = false; - private keyboardProtocolLateResponsePending = false; private keyboardProtocolNegotiationBuffer = ""; - private keyboardProtocolFallbackTimer?: ReturnType; private keyboardProtocolBufferFlushTimer?: ReturnType; private stdinBuffer?: StdinBuffer; private stdinDataHandler?: (data: string) => void; @@ -131,6 +127,10 @@ export class ProcessTerminal implements Terminal { return this._kittyProtocolActive; } + get modifyOtherKeysActive(): boolean { + return this._modifyOtherKeysActive; + } + start(onInput: (data: string) => void, onResize: () => void): void { this.inputHandler = onInput; this.resizeHandler = onResize; @@ -161,8 +161,7 @@ export class ProcessTerminal implements Terminal { // since that resets console mode flags. this.enableWindowsVTInput(); - // Query and enable Kitty keyboard protocol - // The query handler intercepts input temporarily, then installs the user's handler + // Query Kitty keyboard protocol and fall back to modifyOtherKeys when DA confirms no Kitty response. // See: https://sw.kovidgoyal.net/kitty/keyboard-protocol/ this.queryAndEnableKittyProtocol(); } @@ -180,25 +179,13 @@ export class ProcessTerminal implements Terminal { // Forward individual sequences to the input handler this.stdinBuffer.on("data", (sequence) => { - if (this.keyboardProtocolNegotiationPending) { - const negotiationSequence = this.readKeyboardProtocolNegotiationSequence(sequence, true); - if (negotiationSequence === "pending") { - return; // Wait for the rest of a split negotiation response. - } - if (this.handleKeyboardProtocolNegotiationSequence(negotiationSequence)) { - return; - } + const negotiationSequence = this.readKeyboardProtocolNegotiationSequence(sequence); + if (negotiationSequence === "pending") { + this.scheduleKeyboardProtocolNegotiationBufferFlush(); + return; // Wait briefly for the rest of a split Kitty response. } - - if (this.keyboardProtocolLateResponsePending) { - const negotiationSequence = this.readKeyboardProtocolNegotiationSequence(sequence, false); - if (negotiationSequence === "pending") { - this.scheduleKeyboardProtocolNegotiationBufferFlush(); - return; // Wait for the rest of a split late negotiation response. - } - if (this.handleKeyboardProtocolNegotiationSequence(negotiationSequence)) { - return; - } + if (this.handleKeyboardProtocolNegotiationSequence(negotiationSequence)) { + return; } this.forwardInputSequence(sequence); @@ -222,9 +209,8 @@ export class ProcessTerminal implements Terminal { * * Kitty's progressive enhancement detection requires requesting the desired * flags before querying them. The trailing DA query is a sentinel supported by - * terminals that do not know Kitty keyboard protocol. A short timeout remains - * as a backup for terminals, PTYs, and SSH sessions that delay, split, or drop - * the DA response. + * terminals that do not know Kitty keyboard protocol; receiving DA before a + * Kitty response enables modifyOtherKeys fallback without a startup timeout. * * The requested flags are: * - 1 = disambiguate escape codes @@ -235,50 +221,36 @@ export class ProcessTerminal implements Terminal { this.setupStdinBuffer(); process.stdin.on("data", this.stdinDataHandler!); this.keyboardProtocolPushed = true; - this.keyboardProtocolNegotiationPending = true; - this.keyboardProtocolLateResponsePending = false; this.clearKeyboardProtocolNegotiationBuffer(); process.stdout.write(KITTY_KEYBOARD_PROTOCOL_QUERY); - this.keyboardProtocolFallbackTimer = setTimeout(() => { - this.keyboardProtocolFallbackTimer = undefined; - this.keyboardProtocolNegotiationPending = false; - this.keyboardProtocolLateResponsePending = true; - if (this.keyboardProtocolNegotiationBuffer === "\x1b") { - this.flushKeyboardProtocolNegotiationBufferAsInput(); - } else { - this.scheduleKeyboardProtocolNegotiationBufferFlush(); - } - this.enableModifyOtherKeys(); - }, KITTY_KEYBOARD_PROTOCOL_FALLBACK_TIMEOUT_MS); } private handleKeyboardProtocolNegotiationSequence( negotiationSequence: KeyboardProtocolNegotiationSequence | undefined, ): boolean { if (!negotiationSequence) return false; + this.clearKeyboardProtocolNegotiationBuffer(); if (negotiationSequence.type === "kitty-flags") { - if (negotiationSequence.flags !== 0 && !this._kittyProtocolActive) { - this._kittyProtocolActive = true; - setKittyProtocolActive(true); - this.keyboardProtocolNegotiationPending = false; - this.keyboardProtocolLateResponsePending = true; - this.clearKeyboardProtocolNegotiationBuffer(); - this.clearKeyboardProtocolFallbackTimer(); + if (negotiationSequence.flags !== 0) { + this.disableModifyOtherKeys(); + if (!this._kittyProtocolActive) { + this._kittyProtocolActive = true; + setKittyProtocolActive(true); + } + } else { + this.enableModifyOtherKeys(); } return true; } - this.keyboardProtocolNegotiationPending = false; - this.keyboardProtocolLateResponsePending = true; - this.clearKeyboardProtocolNegotiationBuffer(); - this.clearKeyboardProtocolFallbackTimer(); - this.enableModifyOtherKeys(); + if (!this._kittyProtocolActive) { + this.enableModifyOtherKeys(); + } return true; } private readKeyboardProtocolNegotiationSequence( sequence: string, - allowBareEscapePrefix: boolean, ): KeyboardProtocolNegotiationSequence | "pending" | undefined { if (this.keyboardProtocolNegotiationBuffer) { const bufferedSequence = this.keyboardProtocolNegotiationBuffer + sequence; @@ -287,7 +259,7 @@ export class ProcessTerminal implements Terminal { this.clearKeyboardProtocolNegotiationBuffer(); return negotiationSequence; } - if (isKeyboardProtocolNegotiationSequencePrefix(bufferedSequence, allowBareEscapePrefix)) { + if (isKeyboardProtocolNegotiationSequencePrefix(bufferedSequence)) { this.setKeyboardProtocolNegotiationBuffer(bufferedSequence); return "pending"; } @@ -296,7 +268,7 @@ export class ProcessTerminal implements Terminal { const negotiationSequence = parseKeyboardProtocolNegotiationSequence(sequence); if (negotiationSequence) return negotiationSequence; - if (isKeyboardProtocolNegotiationSequencePrefix(sequence, allowBareEscapePrefix)) { + if (isKeyboardProtocolNegotiationSequencePrefix(sequence)) { this.setKeyboardProtocolNegotiationBuffer(sequence); return "pending"; } @@ -351,10 +323,10 @@ export class ProcessTerminal implements Terminal { this._modifyOtherKeysActive = true; } - private clearKeyboardProtocolFallbackTimer(): void { - if (!this.keyboardProtocolFallbackTimer) return; - clearTimeout(this.keyboardProtocolFallbackTimer); - this.keyboardProtocolFallbackTimer = undefined; + private disableModifyOtherKeys(): void { + if (!this._modifyOtherKeysActive) return; + process.stdout.write("\x1b[>4;0m"); + this._modifyOtherKeysActive = false; } /** @@ -394,11 +366,8 @@ export class ProcessTerminal implements Terminal { } async drainInput(maxMs = 1000, idleMs = 50): Promise { - const shouldDisableKittyProtocol = - this.keyboardProtocolPushed || this._kittyProtocolActive || this.keyboardProtocolNegotiationPending; - this.keyboardProtocolLateResponsePending = false; + const shouldDisableKittyProtocol = this.keyboardProtocolPushed || this._kittyProtocolActive; this.clearKeyboardProtocolNegotiationBuffer(); - this.clearKeyboardProtocolFallbackTimer(); if (shouldDisableKittyProtocol) { // Disable Kitty keyboard protocol first so any late key releases // do not generate new Kitty escape sequences. @@ -407,11 +376,7 @@ export class ProcessTerminal implements Terminal { this._kittyProtocolActive = false; setKittyProtocolActive(false); } - this.keyboardProtocolNegotiationPending = false; - if (this._modifyOtherKeysActive) { - process.stdout.write("\x1b[>4;0m"); - this._modifyOtherKeysActive = false; - } + this.disableModifyOtherKeys(); const previousHandler = this.inputHandler; this.inputHandler = undefined; @@ -446,11 +411,8 @@ export class ProcessTerminal implements Terminal { // Disable bracketed paste mode process.stdout.write("\x1b[?2004l"); - const shouldDisableKittyProtocol = - this.keyboardProtocolPushed || this._kittyProtocolActive || this.keyboardProtocolNegotiationPending; - this.keyboardProtocolLateResponsePending = false; + const shouldDisableKittyProtocol = this.keyboardProtocolPushed || this._kittyProtocolActive; this.clearKeyboardProtocolNegotiationBuffer(); - this.clearKeyboardProtocolFallbackTimer(); // Disable Kitty keyboard protocol if not already done by drainInput() if (shouldDisableKittyProtocol) { @@ -459,11 +421,7 @@ export class ProcessTerminal implements Terminal { this._kittyProtocolActive = false; setKittyProtocolActive(false); } - this.keyboardProtocolNegotiationPending = false; - if (this._modifyOtherKeysActive) { - process.stdout.write("\x1b[>4;0m"); - this._modifyOtherKeysActive = false; - } + this.disableModifyOtherKeys(); // Clean up StdinBuffer if (this.stdinBuffer) { diff --git a/packages/tui/test/key-tester.ts b/packages/tui/test/key-tester.ts index 650b98cb2..dce060d4f 100755 --- a/packages/tui/test/key-tester.ts +++ b/packages/tui/test/key-tester.ts @@ -2,6 +2,7 @@ import { matchesKey } from "../src/keys.ts"; import { ProcessTerminal } from "../src/terminal.ts"; import { type Component, TUI } from "../src/tui.ts"; +import { truncateToWidth } from "../src/utils.ts"; /** * Simple key code logger component @@ -10,9 +11,11 @@ class KeyLogger implements Component { private log: string[] = []; private maxLines = 20; private tui: TUI; + private terminal: ProcessTerminal; - constructor(tui: TUI) { + constructor(tui: TUI, terminal: ProcessTerminal) { this.tui = tui; + this.terminal = terminal; } handleInput(data: string): void { @@ -52,18 +55,29 @@ class KeyLogger implements Component { // No cached state to invalidate currently } + private protocolName(): string { + if (this.terminal.kittyProtocolActive) return "kitty"; + if (this.terminal.modifyOtherKeysActive) return "modifyOtherKeys"; + return "legacy"; + } + + private fit(line: string, width: number): string { + return truncateToWidth(line, width).padEnd(width); + } + render(width: number): string[] { const lines: string[] = []; // Title lines.push("=".repeat(width)); - lines.push("Key Code Tester - Press keys to see their codes (Ctrl+C to exit)".padEnd(width)); + lines.push(this.fit("Key Code Tester - Press keys to see their codes (Ctrl+C to exit)", width)); + lines.push(this.fit(`Protocol: ${this.protocolName()}`, width)); lines.push("=".repeat(width)); lines.push(""); // Log entries for (const entry of this.log) { - lines.push(entry.padEnd(width)); + lines.push(this.fit(entry, width)); } // Fill remaining space @@ -74,12 +88,12 @@ class KeyLogger implements Component { // Footer lines.push("=".repeat(width)); - lines.push("Test these:".padEnd(width)); - lines.push(" - Shift + Enter (should show: \\x1b[13;2u with Kitty protocol)".padEnd(width)); - lines.push(" - Alt/Option + Enter".padEnd(width)); - lines.push(" - Option/Alt + Backspace".padEnd(width)); - lines.push(" - Cmd/Ctrl + Backspace".padEnd(width)); - lines.push(" - Regular Backspace".padEnd(width)); + lines.push(this.fit("Test these:", width)); + lines.push(this.fit(" - Shift + Enter (should show: \\x1b[13;2u with Kitty protocol)", width)); + lines.push(this.fit(" - Alt/Option + Enter", width)); + lines.push(this.fit(" - Option/Alt + Backspace", width)); + lines.push(this.fit(" - Cmd/Ctrl + Backspace", width)); + lines.push(this.fit(" - Regular Backspace", width)); lines.push("=".repeat(width)); return lines; @@ -89,7 +103,7 @@ class KeyLogger implements Component { // Set up TUI const terminal = new ProcessTerminal(); const tui = new TUI(terminal); -const logger = new KeyLogger(tui); +const logger = new KeyLogger(tui, terminal); tui.addChild(logger); tui.setFocus(logger); @@ -103,3 +117,7 @@ process.on("SIGINT", () => { // Start the TUI tui.start(); + +// Protocol negotiation completes asynchronously after the first render. +// Refresh briefly/continuously so the displayed protocol state is not stale. +setInterval(() => tui.requestRender(), 100); diff --git a/packages/tui/test/terminal.test.ts b/packages/tui/test/terminal.test.ts index 73328142b..a356bff1d 100644 --- a/packages/tui/test/terminal.test.ts +++ b/packages/tui/test/terminal.test.ts @@ -82,58 +82,80 @@ describe("ProcessTerminal Kitty keyboard protocol negotiation", () => { }; } - it("activates Kitty mode for non-zero negotiated flags", () => { - mock.timers.enable({ apis: ["setTimeout"] }); + it("queries Kitty mode before enabling modifyOtherKeys fallback", () => { const harness = setupNegotiation(); try { - harness.send("\x1b[?1u"); - mock.timers.tick(150); - assert.equal(harness.writes[0], "\x1b[>7u\x1b[?u\x1b[c"); assert.equal(harness.writes.includes("\x1b[>4;2m"), false); + assert.equal(harness.terminal.kittyProtocolActive, false); + } finally { + harness.cleanup(); + } + }); + + it("activates Kitty mode for non-zero negotiated flags", () => { + const harness = setupNegotiation(); + try { + harness.send("\x1b[?7u"); + assert.equal(harness.getInput(), undefined); assert.equal(harness.terminal.kittyProtocolActive, true); + assert.equal(harness.writes.includes("\x1b[>4;2m"), false); + assert.equal(harness.writes.includes("\x1b[>4;0m"), false); harness.cleanup(); assert.equal(harness.writes.filter((write) => write === "\x1b[4;0m"), false); } finally { harness.cleanup(); - mock.timers.reset(); } }); - it("falls back to modifyOtherKeys for unsupported or silent terminals", () => { - const unsupported = setupNegotiation(); + it("falls back to modifyOtherKeys for zero Kitty flags", () => { + const harness = setupNegotiation(); try { - unsupported.send("\x1b[?62;4;52c"); + harness.send("\x1b[?0u"); - assert.equal(unsupported.writes[0], "\x1b[>7u\x1b[?u\x1b[c"); - assert.equal(unsupported.writes.includes("\x1b[>4;2m"), true); - assert.equal(unsupported.getInput(), undefined); - assert.equal(unsupported.terminal.kittyProtocolActive, false); + assert.equal(harness.getInput(), undefined); + assert.equal(harness.terminal.kittyProtocolActive, false); + assert.equal(harness.writes.filter((write) => write === "\x1b[>4;2m").length, 1); + + harness.cleanup(); + assert.equal(harness.writes.filter((write) => write === "\x1b[>4;0m").length, 1); } finally { - unsupported.cleanup(); - } - - mock.timers.enable({ apis: ["setTimeout"] }); - const silent = setupNegotiation(); - try { - mock.timers.tick(150); - - assert.equal(silent.writes[0], "\x1b[>7u\x1b[?u\x1b[c"); - assert.equal(silent.writes.includes("\x1b[>4;2m"), true); - assert.equal(silent.terminal.kittyProtocolActive, false); - } finally { - silent.cleanup(); - mock.timers.reset(); + harness.cleanup(); } }); - it("tracks late split Kitty confirmation after fallback", () => { + it("falls back to modifyOtherKeys for device attributes without Kitty flags", () => { + const harness = setupNegotiation(); + try { + harness.send("\x1b[?62;4;52c"); + + assert.equal(harness.getInput(), undefined); + assert.equal(harness.terminal.kittyProtocolActive, false); + assert.equal(harness.writes.filter((write) => write === "\x1b[>4;2m").length, 1); + } finally { + harness.cleanup(); + } + }); + + it("forwards normal input while waiting for Kitty response", () => { + const harness = setupNegotiation(); + try { + harness.send("a"); + + assert.equal(harness.getInput(), "a"); + assert.equal(harness.terminal.kittyProtocolActive, false); + } finally { + harness.cleanup(); + } + }); + + it("tracks split Kitty confirmation", () => { mock.timers.enable({ apis: ["setTimeout"] }); const harness = setupNegotiation(); try { - mock.timers.tick(150); harness.send("\x1b[?7"); mock.timers.tick(10); @@ -141,26 +163,21 @@ describe("ProcessTerminal Kitty keyboard protocol negotiation", () => { harness.send("u"); - assert.equal(harness.writes.includes("\x1b[>4;2m"), true); assert.equal(harness.terminal.kittyProtocolActive, true); - - harness.cleanup(); - assert.equal(harness.writes.filter((write) => write === "\x1b[ write === "\x1b[>4;0m").length, 1); + assert.equal(harness.writes.includes("\x1b[>4;2m"), false); } finally { harness.cleanup(); mock.timers.reset(); } }); - it("replays buffered CSI-prefix input after fallback", () => { + it("replays buffered CSI-prefix input when it is not a Kitty response", () => { mock.timers.enable({ apis: ["setTimeout"] }); const harness = setupNegotiation(); try { harness.send("\x1b["); - mock.timers.tick(150); + mock.timers.tick(10); - assert.equal(harness.writes.includes("\x1b[>4;2m"), true); assert.equal(harness.getInput(), undefined); mock.timers.tick(150);