Merge pull request #3833 from jeremymcs/fix/pi-tui-input-hardening

fix: harden pi-tui input parsing and editor focus behavior
This commit is contained in:
Jeremy McSpadden 2026-04-08 20:37:35 -05:00 committed by GitHub
commit 94cb295604
8 changed files with 211 additions and 10 deletions

View file

@ -52,6 +52,14 @@ describe("CombinedAutocompleteProvider — slash commands", () => {
const result = provider.getSuggestions(["hello /se"], 0, 9);
assert.equal(result, null);
});
it("triggers slash commands after leading whitespace", () => {
const provider = makeProvider(sampleCommands);
const result = provider.getSuggestions([" /se"], 0, 5);
assert.ok(result);
assert.equal(result!.prefix, "/se");
assert.ok(result!.items.some((item) => item.value === "settings"));
});
});
describe("CombinedAutocompleteProvider — argument completions", () => {
@ -144,6 +152,13 @@ describe("CombinedAutocompleteProvider — applyCompletion", () => {
assert.equal(result.cursorCol, 10); // after "/settings "
});
it("preserves leading whitespace when applying slash command completion", () => {
const provider = makeProvider(sampleCommands);
const result = provider.applyCompletion([" /se"], 0, 5, { value: "settings", label: "settings" }, "/se");
assert.equal(result.lines[0], " /settings ");
assert.equal(result.cursorCol, 12);
});
it("applies file path completion for @ prefix", () => {
const provider = makeProvider();
const result = provider.applyCompletion(

View file

@ -0,0 +1,43 @@
import assert from "node:assert/strict";
import { describe, it } from "node:test";
import { setTimeout as delay } from "node:timers/promises";
import { StdinBuffer } from "../stdin-buffer.js";
describe("StdinBuffer", () => {
it("flushes a lone Escape keypress", async () => {
const buffer = new StdinBuffer({ timeout: 5 });
const received: string[] = [];
buffer.on("data", (sequence) => received.push(sequence));
buffer.process("\x1b");
await delay(20);
assert.deepEqual(received, ["\x1b"]);
assert.equal(buffer.getBuffer(), "");
});
it("keeps split CSI focus and mouse sequences buffered until completion", async () => {
const buffer = new StdinBuffer({ timeout: 5 });
const received: string[] = [];
buffer.on("data", (sequence) => received.push(sequence));
buffer.process("\x1b[");
await delay(20);
assert.deepEqual(received, []);
assert.equal(buffer.getBuffer(), "\x1b[");
buffer.process("I");
assert.deepEqual(received, ["\x1b[I"]);
assert.equal(buffer.getBuffer(), "");
buffer.process("\x1b[<35;20;");
await delay(20);
assert.deepEqual(received, ["\x1b[I"]);
assert.equal(buffer.getBuffer(), "\x1b[<35;20;");
buffer.process("5m");
assert.deepEqual(received, ["\x1b[I", "\x1b[<35;20;5m"]);
assert.equal(buffer.getBuffer(), "");
});
});

View file

@ -0,0 +1,50 @@
import assert from "node:assert/strict";
import { describe, it } from "node:test";
import { TUI } from "../tui.js";
import type { Terminal } from "../terminal.js";
function makeTerminal(): Terminal {
return {
isTTY: true,
columns: 80,
rows: 24,
kittyProtocolActive: false,
start() {},
stop() {},
drainInput: async () => {},
write() {},
moveBy() {},
hideCursor() {},
showCursor() {},
clearLine() {},
clearFromCursor() {},
clearScreen() {},
setTitle() {},
};
}
describe("TUI", () => {
it("does not swallow a bare Escape keypress while waiting for the cell-size response", () => {
const tui = new TUI(makeTerminal());
const received: string[] = [];
tui.setFocus({
render: () => [],
handleInput: (data: string) => {
received.push(data);
},
invalidate() {},
});
const anyTui = tui as any;
anyTui.cellSizeQueryPending = true;
anyTui.inputBuffer = "";
anyTui.handleInput("\x1b");
assert.deepEqual(received, ["\x1b"]);
assert.equal(anyTui.cellSizeQueryPending, false);
assert.equal(anyTui.inputBuffer, "");
});
});

View file

@ -159,6 +159,7 @@ export class CombinedAutocompleteProvider implements AutocompleteProvider {
): { items: AutocompleteItem[]; prefix: string } | null {
const currentLine = lines[cursorLine] || "";
const textBeforeCursor = currentLine.slice(0, cursorCol);
const trimmedBeforeCursor = textBeforeCursor.trimStart();
// Check for @ file reference (fuzzy search) - must be after a delimiter or at start
const atPrefix = this.extractAtPrefix(textBeforeCursor);
@ -174,12 +175,12 @@ export class CombinedAutocompleteProvider implements AutocompleteProvider {
}
// Check for slash commands
if (textBeforeCursor.startsWith("/")) {
const spaceIndex = textBeforeCursor.indexOf(" ");
if (trimmedBeforeCursor.startsWith("/")) {
const spaceIndex = trimmedBeforeCursor.indexOf(" ");
if (spaceIndex === -1) {
// No space yet - complete command names with fuzzy matching
const prefix = textBeforeCursor.slice(1); // Remove the "/"
const prefix = trimmedBeforeCursor.slice(1); // Remove the "/"
const commandItems = this.commands.map((cmd) => ({
name: "name" in cmd ? cmd.name : cmd.value,
label: "name" in cmd ? cmd.name : cmd.label,
@ -196,12 +197,12 @@ export class CombinedAutocompleteProvider implements AutocompleteProvider {
return {
items: filtered,
prefix: textBeforeCursor,
prefix: `/${prefix}`,
};
} else {
// Space found - complete command arguments
const commandName = textBeforeCursor.slice(1, spaceIndex); // Command without "/"
const argumentText = textBeforeCursor.slice(spaceIndex + 1); // Text after space
const commandName = trimmedBeforeCursor.slice(1, spaceIndex); // Command without "/"
const argumentText = trimmedBeforeCursor.slice(spaceIndex + 1); // Text after space
const command = this.commands.find((cmd) => {
const name = "name" in cmd ? cmd.name : cmd.value;
@ -269,7 +270,8 @@ export class CombinedAutocompleteProvider implements AutocompleteProvider {
// Check if we're completing a slash command (prefix starts with "/" but NOT a file path)
// Slash commands are at the start of the line and don't contain path separators after the first /
const isSlashCommand = prefix.startsWith("/") && beforePrefix.trim() === "" && !prefix.slice(1).includes("/");
const trimmedPrefix = prefix.trimStart();
const isSlashCommand = trimmedPrefix.startsWith("/") && beforePrefix.trim() === "" && !trimmedPrefix.slice(1).includes("/");
if (isSlashCommand) {
// This is a command name completion
const newLine = `${beforePrefix}/${item.value} ${adjustedAfterCursor}`;

View file

@ -0,0 +1,64 @@
import assert from "node:assert/strict";
import { describe, it } from "node:test";
import { Editor, type EditorTheme } from "../editor.js";
import { CURSOR_MARKER, TUI } from "../../tui.js";
import type { Terminal } from "../../terminal.js";
function makeTerminal(): Terminal {
return {
isTTY: true,
columns: 80,
rows: 24,
kittyProtocolActive: false,
start() {},
stop() {},
drainInput: async () => {},
write() {},
moveBy() {},
hideCursor() {},
showCursor() {},
clearLine() {},
clearFromCursor() {},
clearScreen() {},
setTitle() {},
};
}
const theme: EditorTheme = {
borderColor: (text) => text,
selectList: {
selectedPrefix: (text) => text,
selectedText: (text) => text,
description: (text) => text,
scrollInfo: (text) => text,
noMatch: (text) => text,
},
};
describe("Editor", () => {
it("clears bracketed paste state when focus is lost", () => {
const editor = new Editor(new TUI(makeTerminal()), theme);
editor.focused = true;
editor.handleInput("\x1b[200~partial");
editor.focused = false;
editor.focused = true;
editor.handleInput("hello");
assert.equal(editor.getText(), "hello");
});
it("keeps the hardware cursor marker visible while autocomplete is open", () => {
const editor = new Editor(new TUI(makeTerminal()), theme);
editor.focused = true;
editor.setText("/se");
(editor as any).autocompleteState = "regular";
(editor as any).autocompleteList = { render: () => [] };
const rendered = editor.render(40).join("\n");
assert.ok(rendered.includes(CURSOR_MARKER));
});
});

View file

@ -128,7 +128,17 @@ export class Editor implements Component, Focusable {
};
/** Focusable interface - set by TUI when focus changes */
focused: boolean = false;
private _focused: boolean = false;
get focused(): boolean {
return this._focused;
}
set focused(value: boolean) {
this._focused = value;
if (!value) {
this.isInPaste = false;
this.pasteBuffer = "";
}
}
protected tui: TUI;
private theme: EditorTheme;
@ -376,8 +386,9 @@ export class Editor implements Component, Focusable {
}
// Render each visible layout line
// Emit hardware cursor marker only when focused and not showing autocomplete
const emitCursorMarker = this.focused && !this.autocompleteState;
// Keep the hardware cursor anchored while autocomplete is open so IME
// candidate windows still attach to the editor caret.
const emitCursorMarker = this.focused;
for (const layoutLine of visibleLines) {
let displayText = layoutLine.text;

View file

@ -361,6 +361,13 @@ export class StdinBuffer extends EventEmitter<StdinBufferEventMap> {
return [];
}
// Keep incomplete escape prefixes buffered so split CSI/mouse/focus
// sequences do not get emitted as literal text on timeout.
// A lone ESC is still flushed so an actual Escape keypress is not lost.
if (this.buffer.length > 1 && this.buffer.startsWith(ESC) && isCompleteSequence(this.buffer) === "incomplete") {
return [];
}
const sequences = [this.buffer];
this.buffer = "";
return sequences;

View file

@ -590,6 +590,15 @@ export class TUI extends Container {
this.cellSizeQueryPending = false;
}
// Don't hold a bare Escape keypress hostage while waiting for the
// optional cell-size response. This is the most common early input race.
if (this.inputBuffer === "\x1b") {
const result = this.inputBuffer;
this.inputBuffer = "";
this.cellSizeQueryPending = false;
return result;
}
// Check if we have a partial cell size response starting (wait for more data)
// Patterns that could be incomplete cell size response: \x1b, \x1b[, \x1b[6, \x1b[6;...(no t yet)
const partialCellSizePattern = /\x1b(\[6?;?[\d;]*)?$/;