From c2d6c8cf1850aa57a41e6d104f5a704cf9b5149f Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 30 Jun 2026 03:41:37 +0000 Subject: [PATCH] fix(tui): deliver buffered double-Esc as two events and re-arm loader after task completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StdinBuffer held a bare `\x1b\x1b` chunk and timer-flushed it as one sequence. `parseKey("\x1b\x1b")` returns undefined, so CustomEditor fell through to the base editor and never fired the configured `onEscape` — the double-escape gesture and the second-press single-Esc handler both went dead whenever the terminal batched the two presses into one stdin read. Split an exact bare `\x1b\x1b` into two ESC events only after the flush window proves no follower arrived. If a follower does arrive, emit the first ESC and restart parsing at the second ESC so legacy Alt chords (`\x1bd`, `\x1b\x7f`) remain one downstream keypress. Meta-CSI/SS3 chords (`\x1b\x1b[A`, `\x1b\x1bO…`) still emit as one combined sequence. EventController.tool_execution_update re-armed the working loader when a transient overlay (auto-compaction / auto-retry / handoff) had torn it down mid-tool; tool_execution_end did not. A subagent (`task`) call only fires _end, so a task result landing after such an overlay left the UI looking idle even though the session was still streaming. Mirror the reconciler call in #handleToolExecutionEnd. Fixes #3857 --- .../custom-editor-buffered-double-esc.test.ts | 30 +++++++------------ .../event-controller-loader-recovery.test.ts | 1 - packages/tui/CHANGELOG.md | 2 +- packages/tui/src/stdin-buffer.ts | 21 +++++++------ packages/tui/test/stdin-buffer.test.ts | 12 ++++++-- 5 files changed, 32 insertions(+), 34 deletions(-) diff --git a/packages/coding-agent/test/custom-editor-buffered-double-esc.test.ts b/packages/coding-agent/test/custom-editor-buffered-double-esc.test.ts index 1446042fa..8e518ef53 100644 --- a/packages/coding-agent/test/custom-editor-buffered-double-esc.test.ts +++ b/packages/coding-agent/test/custom-editor-buffered-double-esc.test.ts @@ -7,15 +7,14 @@ import { StdinBuffer } from "@oh-my-pi/pi-tui/stdin-buffer"; * Regression for #3857. * * A fast double-Esc lands as one `"\x1b\x1b"` chunk on stdin. Before the fix, - * `StdinBuffer` either held it as the buffered remainder and timer-flushed it - * as one sequence, or emitted it as one sequence when followed by a non-CSI - * byte. Either way `parseKey("\x1b\x1b")` returns `undefined`, so + * `StdinBuffer` held it as the buffered remainder, then timer-flushed it as + * one sequence. `parseKey("\x1b\x1b")` returns `undefined`, so * `CustomEditor.handleInput` fell through to the base editor and never fired - * the configured `onEscape` — breaking the double-escape gesture (and any - * single-Esc handler the second press should have hit). + * the configured `onEscape` — breaking the double-escape gesture. * - * The fix splits a bare `"\x1b\x1b"` into two ESC events at the buffer layer, - * matching the existing split for `"\x1b" + "\x1b[<…"` SGR mouse reports. + * The fix splits a bare `"\x1b\x1b"` into two ESC events only when no follower + * arrives in the disambiguation window. If a follower arrives, the second ESC + * remains attached to that follower so legacy Alt chords survive. */ describe("buffered double-Esc reaches CustomEditor.onEscape", () => { beforeAll(async () => { @@ -47,27 +46,20 @@ describe("buffered double-Esc reaches CustomEditor.onEscape", () => { buf.destroy(); }); - it("fires onEscape twice when a double-Esc arrives as one inline chunk followed by a non-CSI byte", () => { + it("preserves a legacy Alt chord batched after a bare ESC", () => { const editor = new CustomEditor(getEditorTheme()); const onEscape = vi.fn(); editor.onEscape = onEscape; - - const forwardedToBase: string[] = []; - const baseHandleInput = vi.spyOn(Object.getPrototypeOf(Object.getPrototypeOf(editor)), "handleInput"); - baseHandleInput.mockImplementation(function (this: unknown, data: unknown) { - forwardedToBase.push(data as string); - }); + editor.setText("foo bar"); const buf = new StdinBuffer({ timeout: 5, partialHoldTimeout: 5 }); buf.on("data", chunk => editor.handleInput(chunk)); - buf.process("\x1b\x1bX"); + buf.process("\x1b\x1b\x7f"); vi.runAllTimers(); - expect(onEscape).toHaveBeenCalledTimes(2); - // The trailing printable still reaches the base editor in order, after - // both ESC keypresses have fired their handler. - expect(forwardedToBase).toEqual(["X"]); + expect(onEscape).toHaveBeenCalledTimes(1); + expect(editor.getText()).toBe("foo "); buf.destroy(); }); diff --git a/packages/coding-agent/test/modes/controllers/event-controller-loader-recovery.test.ts b/packages/coding-agent/test/modes/controllers/event-controller-loader-recovery.test.ts index 2ab4b0d77..b5e1b1109 100644 --- a/packages/coding-agent/test/modes/controllers/event-controller-loader-recovery.test.ts +++ b/packages/coding-agent/test/modes/controllers/event-controller-loader-recovery.test.ts @@ -124,7 +124,6 @@ const TASK_TOOL_EXECUTION_END = { isError: false, } as unknown as AgentSessionEvent; - describe("EventController loader recovery after overflow maintenance", () => { beforeAll(async () => { await initTheme(false); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index bee52d7be..cf0d338cb 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed `StdinBuffer` swallowing a fast double-Esc that arrived as one `"\x1b\x1b"` chunk: `parseKey` returns `undefined` for the combined chunk, so the editor's double-escape gesture and any single-Esc handler the second press should have hit never fired. The buffer now splits a bare `"\x1b\x1b"` (whether buffered alone or followed by a non-CSI/SS3 byte) into two ESC events, while meta-CSI/SS3 chords like `"\x1b\x1b[A"` still emit as one sequence ([#3857](https://github.com/can1357/oh-my-pi/issues/3857)). +- Fixed `StdinBuffer` swallowing a fast double-Esc that arrived as one `"\x1b\x1b"` chunk: `parseKey` returns `undefined` for the combined chunk, so the editor's double-escape gesture and any single-Esc handler the second press should have hit never fired. The buffer now splits a bare `"\x1b\x1b"` into two ESC events only when no follower arrives in the disambiguation window; when a follower arrives, the second ESC stays attached so legacy Alt chords like `"\x1bd"` and meta-CSI/SS3 chords like `"\x1b\x1b[A"` still emit as parseable sequences ([#3857](https://github.com/can1357/oh-my-pi/issues/3857)). ## [16.2.3] - 2026-06-28 diff --git a/packages/tui/src/stdin-buffer.ts b/packages/tui/src/stdin-buffer.ts index b4f4cb2b8..e54b57647 100644 --- a/packages/tui/src/stdin-buffer.ts +++ b/packages/tui/src/stdin-buffer.ts @@ -241,20 +241,19 @@ function extractCompleteSequences(buffer: string): { sequences: string[]; remain end++; continue; } - // "\x1b\x1b" is one of three things, and only the first should reach the - // caller as a combined chunk: + // "\x1b\x1b" is one of three things: // 1. ESC prefixing CSI/SS3 (meta-CSI, held Esc joined by a follower): // next byte is "[" or "O" — keep growing so the full sequence stays // together. Consuming two bytes here would tear the follower and // leak its tail as typed text (settings search filling with "[B" // or "[<35;22;17M"). - // 2. Two real Esc keypresses bursted by terminal input batching, or - // legacy alt+esc — `parseKey` returns undefined for the combined - // chunk, so a single emission swallows double-escape gestures - // (#3857). Split into two ESC events so the caller observes both. - // When the buffer ends here, hold the partial for the flush window so - // the disambiguating byte (case 1) can still arrive; if it does not, - // the timeout-driven flush splits the held remainder (see `flush`). + // 2. ESC followed by a legacy Alt chord (`\x1bd`, `\x1b\x7f`, …): + // emit the first ESC, then restart at the second ESC so downstream + // parsing still sees the Alt chord as one keypress (#3860 review). + // 3. Two real Esc keypresses bursted by terminal input batching: + // when the buffer ends here, hold the partial for the flush window + // so case 1/2 can still arrive; if no follower arrives, `flush()` + // splits the held remainder into two ESC events (#3857). if (candidate === `${ESC}${ESC}`) { if (end >= length) { return { sequences, remainder: buffer.slice(pos) }; @@ -264,8 +263,8 @@ function extractCompleteSequences(buffer: string): { sequences: string[]; remain end++; continue; } - sequences.push(ESC, ESC); - pos = end; + sequences.push(ESC); + pos += 1; consumed = true; break; } diff --git a/packages/tui/test/stdin-buffer.test.ts b/packages/tui/test/stdin-buffer.test.ts index efb39aa9a..c7fa0bda4 100644 --- a/packages/tui/test/stdin-buffer.test.ts +++ b/packages/tui/test/stdin-buffer.test.ts @@ -192,9 +192,17 @@ describe("StdinBuffer", () => { expect(emittedSequences).toEqual(["\x1b", "\x1b"]); }); - it("splits double-ESC followed by a non-CSI byte into two ESC events plus the byte", () => { + it("preserves legacy Alt chords batched after a bare ESC", () => { processInput("\x1b\x1bX"); - expect(emittedSequences).toEqual(["\x1b", "\x1b", "X"]); + expect(emittedSequences).toEqual(["\x1b", "\x1bX"]); + + emittedSequences = []; + processInput("\x1b\x1bd"); + expect(emittedSequences).toEqual(["\x1b", "\x1bd"]); + + emittedSequences = []; + processInput("\x1b\x1b\x7f"); + expect(emittedSequences).toEqual(["\x1b", "\x1b\x7f"]); }); it("consumes a whole meta-CSI arrow in one chunk", () => {