Merge pull request #5091 from earendil-works/fix/kitty-da-negotiation

fix(tui): harden keyboard protocol negotiation

packages/tui/src/terminal.ts

@@ -12,6 +12,31 @@ const TERMINAL_PROGRESS_KEEPALIVE_MS = 1000;
 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`;
+
+export type KeyboardProtocolNegotiationSequence =
+	| { type: "kitty-flags"; flags: number }
+	| { type: "device-attributes" };
+
+export function parseKeyboardProtocolNegotiationSequence(
+	sequence: string,
+): KeyboardProtocolNegotiationSequence | undefined {
+	const kittyFlags = sequence.match(/^\x1b\[\?(\d+)u$/);
+	if (kittyFlags) {
+		return { type: "kitty-flags", flags: Number.parseInt(kittyFlags[1]!, 10) };
+	}
+	if (/^\x1b\[\?[\d;]*c$/.test(sequence)) {
+		return { type: "device-attributes" };
+	}
+	return undefined;
+}
+
+function isKeyboardProtocolNegotiationSequencePrefix(sequence: string, allowBareEscapePrefix: boolean): boolean {
+	return (allowBareEscapePrefix && sequence === "\x1b") || sequence === "\x1b[" || /^\x1b\[\?[\d;]*$/.test(sequence);
+}
 
 export function isAppleTerminalSession(): boolean {
 	return process.platform === "darwin" && process.env.TERM_PROGRAM === "Apple_Terminal";
@@ -78,6 +103,12 @@ export class ProcessTerminal implements Terminal {
 	private resizeHandler?: () => void;
 	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;
 	private progressInterval?: ReturnType<typeof setInterval>;
@@ -147,37 +178,30 @@ export class ProcessTerminal implements Terminal {
 	private setupStdinBuffer(): void {
 		this.stdinBuffer = new StdinBuffer({ timeout: 10 });
 
-		// Kitty protocol response pattern: \x1b[?<flags>u
-		const kittyResponsePattern = /^\x1b\[\?(\d+)u$/;
-
 		// Forward individual sequences to the input handler
 		this.stdinBuffer.on("data", (sequence) => {
-			// Check for Kitty protocol response (only if not already enabled)
+			if (this.keyboardProtocolNegotiationPending) {
-			if (!this._kittyProtocolActive) {
+				const negotiationSequence = this.readKeyboardProtocolNegotiationSequence(sequence, true);
-				const match = sequence.match(kittyResponsePattern);
+				if (negotiationSequence === "pending") {
-				if (match) {
+					return; // Wait for the rest of a split negotiation response.
-					this._kittyProtocolActive = true;
+				}
-					setKittyProtocolActive(true);
+				if (this.handleKeyboardProtocolNegotiationSequence(negotiationSequence)) {
-
+					return;
-					// Enable Kitty keyboard protocol (push flags)
-					// Flag 1 = disambiguate escape codes
-					// Flag 2 = report event types (press/repeat/release)
-					// Flag 4 = report alternate keys (shifted key, base layout key)
-					// Base layout key enables shortcuts to work with non-Latin keyboard layouts
-					process.stdout.write("\x1b[>7u");
-					return; // Don't forward protocol response to TUI
 				}
 			}
 
-			if (this.inputHandler) {
+			if (this.keyboardProtocolLateResponsePending) {
-				const isAppleTerminal = sequence === "\r" && isAppleTerminalSession();
+				const negotiationSequence = this.readKeyboardProtocolNegotiationSequence(sequence, false);
-				const input = normalizeAppleTerminalInput(
+				if (negotiationSequence === "pending") {
-					sequence,
+					this.scheduleKeyboardProtocolNegotiationBufferFlush();
-					isAppleTerminal,
+					return; // Wait for the rest of a split late negotiation response.
-					isAppleTerminal && isNativeModifierPressed("shift"),
+				}
-				);
+				if (this.handleKeyboardProtocolNegotiationSequence(negotiationSequence)) {
-				this.inputHandler(input);
+					return;
+				}
 			}
+
+			this.forwardInputSequence(sequence);
 		});
 
 		// Re-wrap paste content with bracketed paste markers for existing editor handling
@@ -194,29 +218,143 @@ export class ProcessTerminal implements Terminal {
 	}
 
 	/**
-	 * Query terminal for Kitty keyboard protocol support and enable if available.
+	 * Query terminal for Kitty keyboard protocol support and enable it if available.
 	 *
-	 * Sends CSI ? u to query current flags. If terminal responds with CSI ? <flags> u,
+	 * Kitty's progressive enhancement detection requires requesting the desired
-	 * it supports the protocol and we enable it with CSI > 1 u.
+	 * 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.
 	 *
-	 * If no Kitty response arrives shortly after startup, fall back to enabling
+	 * The requested flags are:
-	 * xterm modifyOtherKeys mode 2. This is needed for tmux, which can forward
+	 * - 1 = disambiguate escape codes
-	 * modified enter keys as CSI-u when extended-keys is enabled, but may not
+	 * - 2 = report event types (press/repeat/release)
-	 * answer the Kitty protocol query.
+	 * - 4 = report alternate keys (shifted key, base layout key)
-	 *
-	 * The response is detected in setupStdinBuffer's data handler, which properly
-	 * handles the case where the response arrives split across multiple stdin events.
 	 */
 	private queryAndEnableKittyProtocol(): void {
 		this.setupStdinBuffer();
 		process.stdin.on("data", this.stdinDataHandler!);
-		process.stdout.write("\x1b[?u");
+		this.keyboardProtocolPushed = true;
-		setTimeout(() => {
+		this.keyboardProtocolNegotiationPending = true;
-			if (!this._kittyProtocolActive && !this._modifyOtherKeysActive) {
+		this.keyboardProtocolLateResponsePending = false;
-				process.stdout.write("\x1b[>4;2m");
+		this.clearKeyboardProtocolNegotiationBuffer();
-				this._modifyOtherKeysActive = true;
+		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();
 			}
-		}, 150);
+			this.enableModifyOtherKeys();
+		}, KITTY_KEYBOARD_PROTOCOL_FALLBACK_TIMEOUT_MS);
+	}
+
+	private handleKeyboardProtocolNegotiationSequence(
+		negotiationSequence: KeyboardProtocolNegotiationSequence | undefined,
+	): boolean {
+		if (!negotiationSequence) return false;
+		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();
+			}
+			return true;
+		}
+
+		this.keyboardProtocolNegotiationPending = false;
+		this.keyboardProtocolLateResponsePending = true;
+		this.clearKeyboardProtocolNegotiationBuffer();
+		this.clearKeyboardProtocolFallbackTimer();
+		this.enableModifyOtherKeys();
+		return true;
+	}
+
+	private readKeyboardProtocolNegotiationSequence(
+		sequence: string,
+		allowBareEscapePrefix: boolean,
+	): KeyboardProtocolNegotiationSequence | "pending" | undefined {
+		if (this.keyboardProtocolNegotiationBuffer) {
+			const bufferedSequence = this.keyboardProtocolNegotiationBuffer + sequence;
+			const negotiationSequence = parseKeyboardProtocolNegotiationSequence(bufferedSequence);
+			if (negotiationSequence) {
+				this.clearKeyboardProtocolNegotiationBuffer();
+				return negotiationSequence;
+			}
+			if (isKeyboardProtocolNegotiationSequencePrefix(bufferedSequence, allowBareEscapePrefix)) {
+				this.setKeyboardProtocolNegotiationBuffer(bufferedSequence);
+				return "pending";
+			}
+			this.flushKeyboardProtocolNegotiationBufferAsInput();
+		}
+
+		const negotiationSequence = parseKeyboardProtocolNegotiationSequence(sequence);
+		if (negotiationSequence) return negotiationSequence;
+		if (isKeyboardProtocolNegotiationSequencePrefix(sequence, allowBareEscapePrefix)) {
+			this.setKeyboardProtocolNegotiationBuffer(sequence);
+			return "pending";
+		}
+		return undefined;
+	}
+
+	private setKeyboardProtocolNegotiationBuffer(sequence: string): void {
+		this.clearKeyboardProtocolNegotiationBufferFlushTimer();
+		this.keyboardProtocolNegotiationBuffer = sequence;
+	}
+
+	private clearKeyboardProtocolNegotiationBuffer(): void {
+		this.clearKeyboardProtocolNegotiationBufferFlushTimer();
+		this.keyboardProtocolNegotiationBuffer = "";
+	}
+
+	private flushKeyboardProtocolNegotiationBufferAsInput(): void {
+		if (!this.keyboardProtocolNegotiationBuffer) return;
+		const sequence = this.keyboardProtocolNegotiationBuffer;
+		this.clearKeyboardProtocolNegotiationBuffer();
+		this.forwardInputSequence(sequence);
+	}
+
+	private scheduleKeyboardProtocolNegotiationBufferFlush(): void {
+		if (!this.keyboardProtocolNegotiationBuffer || this.keyboardProtocolBufferFlushTimer) return;
+		this.keyboardProtocolBufferFlushTimer = setTimeout(() => {
+			this.keyboardProtocolBufferFlushTimer = undefined;
+			this.flushKeyboardProtocolNegotiationBufferAsInput();
+		}, KEYBOARD_PROTOCOL_RESPONSE_FRAGMENT_TIMEOUT_MS);
+	}
+
+	private clearKeyboardProtocolNegotiationBufferFlushTimer(): void {
+		if (!this.keyboardProtocolBufferFlushTimer) return;
+		clearTimeout(this.keyboardProtocolBufferFlushTimer);
+		this.keyboardProtocolBufferFlushTimer = undefined;
+	}
+
+	private forwardInputSequence(sequence: string): void {
+		if (!this.inputHandler) return;
+		const isAppleTerminal = sequence === "\r" && isAppleTerminalSession();
+		const input = normalizeAppleTerminalInput(
+			sequence,
+			isAppleTerminal,
+			isAppleTerminal && isNativeModifierPressed("shift"),
+		);
+		this.inputHandler(input);
+	}
+
+	private enableModifyOtherKeys(): void {
+		if (this._kittyProtocolActive || this._modifyOtherKeysActive) return;
+		process.stdout.write("\x1b[>4;2m");
+		this._modifyOtherKeysActive = true;
+	}
+
+	private clearKeyboardProtocolFallbackTimer(): void {
+		if (!this.keyboardProtocolFallbackTimer) return;
+		clearTimeout(this.keyboardProtocolFallbackTimer);
+		this.keyboardProtocolFallbackTimer = undefined;
 	}
 
 	/**
@@ -256,13 +394,20 @@ export class ProcessTerminal implements Terminal {
 	}
 
 	async drainInput(maxMs = 1000, idleMs = 50): Promise<void> {
-		if (this._kittyProtocolActive) {
+		const shouldDisableKittyProtocol =
+			this.keyboardProtocolPushed || this._kittyProtocolActive || this.keyboardProtocolNegotiationPending;
+		this.keyboardProtocolLateResponsePending = false;
+		this.clearKeyboardProtocolNegotiationBuffer();
+		this.clearKeyboardProtocolFallbackTimer();
+		if (shouldDisableKittyProtocol) {
 			// Disable Kitty keyboard protocol first so any late key releases
 			// do not generate new Kitty escape sequences.
 			process.stdout.write("\x1b[<u");
+			this.keyboardProtocolPushed = false;
 			this._kittyProtocolActive = false;
 			setKittyProtocolActive(false);
 		}
+		this.keyboardProtocolNegotiationPending = false;
 		if (this._modifyOtherKeysActive) {
 			process.stdout.write("\x1b[>4;0m");
 			this._modifyOtherKeysActive = false;
@@ -301,12 +446,20 @@ 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;
+		this.clearKeyboardProtocolNegotiationBuffer();
+		this.clearKeyboardProtocolFallbackTimer();
+
 		// Disable Kitty keyboard protocol if not already done by drainInput()
-		if (this._kittyProtocolActive) {
+		if (shouldDisableKittyProtocol) {
 			process.stdout.write("\x1b[<u");
+			this.keyboardProtocolPushed = false;
 			this._kittyProtocolActive = false;
 			setKittyProtocolActive(false);
 		}
+		this.keyboardProtocolNegotiationPending = false;
 		if (this._modifyOtherKeysActive) {
 			process.stdout.write("\x1b[>4;0m");
 			this._modifyOtherKeysActive = false;

packages/tui/test/terminal.test.ts

@@ -1,5 +1,6 @@
 import assert from "node:assert";
-import { describe, it } from "node:test";
+import { describe, it, mock } from "node:test";
+import { setKittyProtocolActive } from "../src/keys.ts";
 import { normalizeAppleTerminalInput, ProcessTerminal } from "../src/terminal.ts";
 
 describe("normalizeAppleTerminalInput", () => {
@@ -21,6 +22,157 @@ describe("normalizeAppleTerminalInput", () => {
 	});
 });
 
+describe("ProcessTerminal Kitty keyboard protocol negotiation", () => {
+	type NegotiationHarness = {
+		terminal: ProcessTerminal;
+		writes: string[];
+		send(data: string): void;
+		getInput(): string | undefined;
+		cleanup(): void;
+	};
+
+	function setupNegotiation(): NegotiationHarness {
+		const terminal = new ProcessTerminal();
+		const writes: string[] = [];
+		let input: string | undefined;
+		let dataHandler: ((data: string) => void) | undefined;
+		let cleaned = false;
+		const previousWrite = process.stdout.write;
+		const previousOn = process.stdin.on;
+
+		process.stdout.write = ((chunk: string | Uint8Array) => {
+			writes.push(String(chunk));
+			return true;
+		}) as typeof process.stdout.write;
+		process.stdin.on = ((event: string | symbol, listener: (...args: unknown[]) => void) => {
+			if (event === "data") dataHandler = listener as (data: string) => void;
+			return process.stdin;
+		}) as typeof process.stdin.on;
+
+		(
+			terminal as unknown as {
+				inputHandler?: (data: string) => void;
+				queryAndEnableKittyProtocol(): void;
+			}
+		).inputHandler = (data) => {
+			input = data;
+		};
+		(terminal as unknown as { queryAndEnableKittyProtocol(): void }).queryAndEnableKittyProtocol();
+
+		return {
+			terminal,
+			writes,
+			send(data: string): void {
+				dataHandler?.(data);
+			},
+			getInput(): string | undefined {
+				return input;
+			},
+			cleanup(): void {
+				if (cleaned) return;
+				cleaned = true;
+				try {
+					terminal.stop();
+				} finally {
+					process.stdout.write = previousWrite;
+					process.stdin.on = previousOn;
+					setKittyProtocolActive(false);
+				}
+			},
+		};
+	}
+
+	it("activates Kitty mode for non-zero negotiated flags", () => {
+		mock.timers.enable({ apis: ["setTimeout"] });
+		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.getInput(), undefined);
+			assert.equal(harness.terminal.kittyProtocolActive, true);
+
+			harness.cleanup();
+			assert.equal(harness.writes.filter((write) => write === "\x1b[<u").length, 1);
+		} finally {
+			harness.cleanup();
+			mock.timers.reset();
+		}
+	});
+
+	it("falls back to modifyOtherKeys for unsupported or silent terminals", () => {
+		const unsupported = setupNegotiation();
+		try {
+			unsupported.send("\x1b[?62;4;52c");
+
+			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);
+		} 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();
+		}
+	});
+
+	it("tracks late split Kitty confirmation after fallback", () => {
+		mock.timers.enable({ apis: ["setTimeout"] });
+		const harness = setupNegotiation();
+		try {
+			mock.timers.tick(150);
+			harness.send("\x1b[?7");
+			mock.timers.tick(10);
+
+			assert.equal(harness.getInput(), undefined);
+
+			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);
+		} finally {
+			harness.cleanup();
+			mock.timers.reset();
+		}
+	});
+
+	it("replays buffered CSI-prefix input after fallback", () => {
+		mock.timers.enable({ apis: ["setTimeout"] });
+		const harness = setupNegotiation();
+		try {
+			harness.send("\x1b[");
+			mock.timers.tick(150);
+
+			assert.equal(harness.writes.includes("\x1b[>4;2m"), true);
+			assert.equal(harness.getInput(), undefined);
+
+			mock.timers.tick(150);
+
+			assert.equal(harness.getInput(), "\x1b[");
+		} finally {
+			harness.cleanup();
+			mock.timers.reset();
+		}
+	});
+});
+
 describe("ProcessTerminal dimensions", () => {
 	it("falls back to COLUMNS and LINES before default dimensions", () => {
 		const previousColumnsDescriptor = Object.getOwnPropertyDescriptor(process.stdout, "columns");