fix(tui): deliver buffered double-Esc as two events and re-arm loader after task completion

StdinBuffer held a bare `\x1b\x1b` chunk (or emitted it as one when followed by
a non-CSI byte). `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 a bare `\x1b\x1b` into two ESC events at the buffer layer, mirroring
the existing split for ESC + SGR mouse report. 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
This commit is contained in:
roboomp
2026-06-30 03:41:24 +00:00
parent f18368d1f7
commit 64734021a7
7 changed files with 197 additions and 15 deletions
+1
View File
@@ -21,6 +21,7 @@
- Improved error reporting for `omp tiny-models download` by displaying the actual worker-side download error.
- Resolved status inconsistencies between `/extensions`, `/mcp list`, and the dashboard, ensuring MCP server states, allowlists/denylists, and configuration files (like `mcp.json`) stay fully synchronized.
- Improved branch-mode task merges to preserve the agent's original commit history (messages and authors) and fixed a bug where merges were rejected due to unrelated dirty changes in the parent checkout.
- Fixed the working loader disappearing after a subagent (`task`) tool completed while the focused session was still streaming: `tool_execution_end` did not re-arm the loader the way `tool_execution_update` did, so a tool result landing after a transient overlay (auto-compaction / auto-retry) left the UI looking idle ([#3857](https://github.com/can1357/oh-my-pi/issues/3857)).
## [16.2.6] - 2026-06-29
@@ -890,6 +890,13 @@ export class EventController {
}
async #handleToolExecutionEnd(event: Extract<AgentSessionEvent, { type: "tool_execution_end" }>): Promise<void> {
// A transient overlay (auto-compaction / auto-retry / handoff) that ran
// between this tool's start and end could have detached the working
// loader. `tool_execution_update` already reconciles this so the spinner
// reappears mid-tool; mirror it here so subagent (`task`) completions —
// which only fire `tool_execution_end`, never `_update` — do not leave
// the UI looking idle while the session keeps streaming (#3857).
this.#ensureWorkingLoaderWhileStreaming();
if (event.toolName === "read") {
if (this.#inlineReadToolImages(event.toolCallId, event.result)) {
const component = this.ctx.pendingTools.get(event.toolCallId);
@@ -0,0 +1,89 @@
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test";
import { CustomEditor } from "@oh-my-pi/pi-coding-agent/modes/components/custom-editor";
import { getEditorTheme, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
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
* `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 fix splits a bare `"\x1b\x1b"` into two ESC events at the buffer layer,
* matching the existing split for `"\x1b" + "\x1b[<…"` SGR mouse reports.
*/
describe("buffered double-Esc reaches CustomEditor.onEscape", () => {
beforeAll(async () => {
await initTheme();
});
beforeEach(() => {
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
vi.restoreAllMocks();
});
it("fires onEscape twice when a fast double-Esc arrives as one buffered chunk", () => {
const editor = new CustomEditor(getEditorTheme());
const onEscape = vi.fn();
editor.onEscape = onEscape;
const buf = new StdinBuffer({ timeout: 5, partialHoldTimeout: 5 });
buf.on("data", chunk => editor.handleInput(chunk));
buf.process("\x1b\x1b");
// Drain the flush timer chain (main timeout + zero-delay deferral).
vi.runAllTimers();
expect(onEscape).toHaveBeenCalledTimes(2);
buf.destroy();
});
it("fires onEscape twice when a double-Esc arrives as one inline chunk followed by a non-CSI byte", () => {
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);
});
const buf = new StdinBuffer({ timeout: 5, partialHoldTimeout: 5 });
buf.on("data", chunk => editor.handleInput(chunk));
buf.process("\x1b\x1bX");
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"]);
buf.destroy();
});
it("does not split a meta-CSI arrow into two ESC events", () => {
const editor = new CustomEditor(getEditorTheme());
const onEscape = vi.fn();
editor.onEscape = onEscape;
const buf = new StdinBuffer({ timeout: 5, partialHoldTimeout: 5 });
buf.on("data", chunk => editor.handleInput(chunk));
buf.process("\x1b\x1b[A");
vi.runAllTimers();
// alt+up is its own keypress and must never look like two ESC keys.
expect(onEscape).not.toHaveBeenCalled();
buf.destroy();
});
});
@@ -70,7 +70,13 @@ function createContext(options: { terminalProgress?: boolean } = {}) {
editor: { getText: () => "" },
sessionManager: { getSessionName: () => "test-session" },
ui: { requestRender: vi.fn(), requestComponentRender: vi.fn(), terminal: { setProgress } },
viewSession: { isCompacting: false, getLastAssistantMessage: () => undefined },
viewSession: {
isCompacting: false,
getLastAssistantMessage: () => undefined,
get isStreaming() {
return streamState.isStreaming;
},
},
session: {
get isStreaming() {
return streamState.isStreaming;
@@ -109,6 +115,15 @@ const RETRY_START = {
delayMs: 1000,
errorMessage: "overloaded",
} as unknown as AgentSessionEvent;
const TASK_TOOL_EXECUTION_END = {
type: "tool_execution_end",
toolCallId: "call-task-1",
toolName: "task",
args: {},
result: { content: [], details: {} },
isError: false,
} as unknown as AgentSessionEvent;
describe("EventController loader recovery after overflow maintenance", () => {
beforeAll(async () => {
@@ -179,6 +194,49 @@ describe("EventController loader recovery after overflow maintenance", () => {
expect(statusContainer.children).toContain(ctx.loadingAnimation);
});
it("re-shows the Working… loader after a subagent task completes while the session keeps streaming", async () => {
const { ctx, streamState, statusContainer, workingLoaders } = createContext();
const controller = new EventController(ctx);
// Turn begins: the working loader is created and attached.
await controller.handleEvent(AGENT_START);
const firstWorking = workingLoaders[0];
expect(firstWorking).toBeDefined();
// A transient overlay (auto-retry / auto-compaction) tore the loader down
// mid-tool; the session is still streaming when the subagent's task
// completes. Before the fix, `tool_execution_end` (unlike `_update`) did
// not re-arm the loader, so the UI looked idle while the agent kept going.
streamState.isStreaming = true;
ctx.loadingAnimation?.stop();
ctx.loadingAnimation = undefined;
statusContainer.clear();
await controller.handleEvent(TASK_TOOL_EXECUTION_END);
expect(ctx.loadingAnimation).toBeDefined();
expect(statusContainer.children).toContain(ctx.loadingAnimation);
expect(workingLoaders).toHaveLength(2);
});
it("does not re-arm the Working… loader on tool_execution_end once the session has stopped streaming", async () => {
const { ctx, streamState, statusContainer } = createContext();
const controller = new EventController(ctx);
await controller.handleEvent(AGENT_START);
ctx.loadingAnimation?.stop();
ctx.loadingAnimation = undefined;
statusContainer.clear();
streamState.isStreaming = false;
await controller.handleEvent(TASK_TOOL_EXECUTION_END);
// No streaming → reconciler must stay a no-op; the spinner is not the
// post-turn idle state.
expect(ctx.loadingAnimation).toBeUndefined();
expect(statusContainer.children).toHaveLength(0);
});
it("mirrors agent and auto-compaction activity to OSC 9;4 when enabled", async () => {
const { ctx, setProgress } = createContext({ terminalProgress: true });
const controller = new EventController(ctx);
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### 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)).
## [16.2.3] - 2026-06-28
### Added
+28 -9
View File
@@ -241,13 +241,20 @@ function extractCompleteSequences(buffer: string): { sequences: string[]; remain
end++;
continue;
}
// "\x1b\x1b" alone parses as "complete" (legacy alt+esc), but when the
// next byte opens a CSI/SS3 ("[" or "O") this is really ESC prefixing
// another sequence (meta-CSI, or a held Esc keypress joined by a
// follower). Consuming two bytes here would tear the follower and leak
// its tail as typed text (settings search filling with "[B" or
// "[<35;22;17M"). Keep growing; when the buffer ends here, hold the
// partial for the flush window so the disambiguating byte can arrive.
// "\x1b\x1b" is one of three things, and only the first should reach the
// caller as a combined chunk:
// 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`).
if (candidate === `${ESC}${ESC}`) {
if (end >= length) {
return { sequences, remainder: buffer.slice(pos) };
@@ -257,6 +264,10 @@ function extractCompleteSequences(buffer: string): { sequences: string[]; remain
end++;
continue;
}
sequences.push(ESC, ESC);
pos = end;
consumed = true;
break;
}
// ESC + SGR mouse report is never a meta chord: alt-modified mouse
// reports carry the modifier in the button bits, not an ESC prefix.
@@ -605,10 +616,18 @@ export class StdinBuffer extends EventEmitter<StdinBufferEventMap> {
return [];
}
const sequences = [this.#buffer];
const buffered = this.#buffer;
this.#buffer = "";
this.#pendingKittyPrintableCodepoint = undefined;
return sequences;
// Bare double-ESC remainder (no disambiguating "[" / "O" arrived in time):
// two real Esc keypresses bursted by terminal batching, not a meta-CSI/SS3
// prefix. `parseKey` returns undefined for the combined chunk, so a single
// emission swallows the double-escape gesture (#3857). Mirror the inline
// split in `extractCompleteSequences` and deliver two ESC events.
if (buffered === `${ESC}${ESC}`) {
return [ESC, ESC];
}
return [buffered];
}
clear(): void {
+9 -5
View File
@@ -181,16 +181,20 @@ describe("StdinBuffer", () => {
expect(emittedSequences).toEqual(["\x1b", "\x1b[<35;22;17M"]);
});
it("flushes a trailing double-ESC as one sequence after the timeout", async () => {
it("splits a trailing double-ESC into two ESC events after the timeout", async () => {
// A bare `\x1b\x1b` is two real Esc keypresses (or legacy alt+esc).
// `parseKey` returns undefined for the combined chunk, so emitting it
// as one swallows double-escape gestures (#3857). Split on flush so
// downstream handlers fire twice.
processInput("\x1b\x1b");
expect(emittedSequences).toEqual([]);
await waitUntil(() => emittedSequences.length > 0);
expect(emittedSequences).toEqual(["\x1b\x1b"]);
await waitUntil(() => emittedSequences.length >= 2);
expect(emittedSequences).toEqual(["\x1b", "\x1b"]);
});
it("keeps double-ESC followed by a non-CSI byte split as before", () => {
it("splits double-ESC followed by a non-CSI byte into two ESC events plus the byte", () => {
processInput("\x1b\x1bX");
expect(emittedSequences).toEqual(["\x1b\x1b", "X"]);
expect(emittedSequences).toEqual(["\x1b", "\x1b", "X"]);
});
it("consumes a whole meta-CSI arrow in one chunk", () => {