fix(tui,coding-agent): make keyboard protocol fallback response-driven

closes #5188

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
 {

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
 

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<typeof setTimeout>;
 	private keyboardProtocolBufferFlushTimer?: ReturnType<typeof setTimeout>;
 	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<void> {
-		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) {

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);

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[<u").length, 1);
+			assert.equal(harness.writes.includes("\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[<u").length, 1);
-			assert.equal(harness.writes.filter((write) => 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);