diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7571e2a01..f6572611c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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 diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 46edcb592..5d3899458 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -890,6 +890,13 @@ export class EventController { } async #handleToolExecutionEnd(event: Extract): Promise { + // 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); 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 new file mode 100644 index 000000000..1446042fa --- /dev/null +++ b/packages/coding-agent/test/custom-editor-buffered-double-esc.test.ts @@ -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(); + }); +}); 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 84c01845c..2ab4b0d77 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 @@ -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); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index a6873c67d..bee52d7be 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -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 diff --git a/packages/tui/src/stdin-buffer.ts b/packages/tui/src/stdin-buffer.ts index e3b155e81..b4f4cb2b8 100644 --- a/packages/tui/src/stdin-buffer.ts +++ b/packages/tui/src/stdin-buffer.ts @@ -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 { 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 { diff --git a/packages/tui/test/stdin-buffer.test.ts b/packages/tui/test/stdin-buffer.test.ts index 2a624fb7d..efb39aa9a 100644 --- a/packages/tui/test/stdin-buffer.test.ts +++ b/packages/tui/test/stdin-buffer.test.ts @@ -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", () => {