From b5eff5b3d369ac285083975cd869ae53f8d8e2d2 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 06:28:40 +0200 Subject: [PATCH] fix(coding-agent/modes): sealed read groups to prevent lingering pending previews at turn end - Added a sealed state to `ReadToolGroupComponent` to force-close a read group when a turn ends without a read result. - Updated transcript finalization to treat pending read entries as active unless the group is sealed, preventing premature finalization when outputs are still in flight. - Expanded turn-end pending-tool cleanup to seal `ReadToolGroupComponent` instances in addition to `ToolExecutionComponent`. --- .../src/modes/components/read-tool-group.ts | 28 +++++- .../src/modes/controllers/event-controller.ts | 7 +- .../test/read-tool-group-freeze.test.ts | 99 +++++++++++++++++++ 3 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 packages/coding-agent/test/read-tool-group-freeze.test.ts diff --git a/packages/coding-agent/src/modes/components/read-tool-group.ts b/packages/coding-agent/src/modes/components/read-tool-group.ts index eb8aeb553..c468b74c7 100644 --- a/packages/coding-agent/src/modes/components/read-tool-group.ts +++ b/packages/coding-agent/src/modes/components/read-tool-group.ts @@ -291,6 +291,9 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa // (see TranscriptContainer / NativeScrollbackLiveRegion). The controller calls // `finalize()` once the run breaks so the block can commit to native scrollback. #finalized = false; + // Forced terminal even with a still-pending entry: the turn ended (abort or + // completion) so no late result is coming. Set via `seal()`. + #sealed = false; constructor(options: ReadToolGroupOptions = {}) { super(); @@ -301,13 +304,36 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa } isTranscriptBlockFinalized(): boolean { - return this.#finalized; + if (this.#sealed) return true; + if (!this.#finalized) return false; + // Closed to new entries, but a still-pending entry means its result is in + // flight — parallel reads can finalize the group (a sibling tool starts and + // breaks the run) before a read's `tool_execution_end` lands. Stay live so + // the late result repaints instead of freezing the pending preview into + // native scrollback on ED3-risk terminals (#issue: stuck "Read "). + return !this.#hasPendingEntries(); + } + + #hasPendingEntries(): boolean { + for (const entry of this.#entries.values()) { + if (entry.status === "pending") return true; + } + return false; } finalize(): void { this.#finalized = true; } + /** + * Force the group terminal even if an entry never received its result (the + * turn aborted or ended). Lets it freeze and stop pinning the transcript live + * region instead of lingering on a pending preview until the next thaw. + */ + seal(): void { + this.#sealed = true; + } + updateArgs(args: ReadRenderArgs, toolCallId?: string): void { if (!toolCallId) return; const basePath = args.file_path || args.path || ""; diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 97e571d0e..cf42ac24c 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -720,7 +720,12 @@ export class EventController { // seal it so it freezes (and stops animating) rather than lingering in // the transcript live region as a streaming preview until the next thaw. const component = this.ctx.pendingTools.get(toolCallId); - if (component instanceof ToolExecutionComponent) component.seal(); + // A foreground read still pending at turn end shares a group component + // keyed by every read's id; seal it too so a never-delivered read does + // not keep the group live (and pinning the live region) indefinitely. + if (component instanceof ToolExecutionComponent || component instanceof ReadToolGroupComponent) { + component.seal(); + } this.ctx.pendingTools.delete(toolCallId); } } diff --git a/packages/coding-agent/test/read-tool-group-freeze.test.ts b/packages/coding-agent/test/read-tool-group-freeze.test.ts new file mode 100644 index 000000000..dde18741a --- /dev/null +++ b/packages/coding-agent/test/read-tool-group-freeze.test.ts @@ -0,0 +1,99 @@ +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { type Component, TERMINAL } from "@oh-my-pi/pi-tui"; +import { resetSettingsForTest, Settings, settings } from "../src/config/settings"; +import { ReadToolGroupComponent } from "../src/modes/components/read-tool-group"; +import { TranscriptContainer } from "../src/modes/components/transcript-container"; +import * as themeModule from "../src/modes/theme/theme"; + +/** Minimal transcript block whose finalized state is fixed at construction. */ +class StubBlock implements Component { + constructor(private readonly finalized: boolean) {} + render(): string[] { + return ["below"]; + } + isTranscriptBlockFinalized(): boolean { + return this.finalized; + } +} + +function successResult() { + return { content: [{ type: "text", text: "x" }], isError: false }; +} + +describe("ReadToolGroupComponent transcript freezing", () => { + let prevRisk: boolean; + + beforeAll(async () => { + resetSettingsForTest(); + await Settings.init({ inMemory: true }); + await themeModule.initTheme(false, undefined, undefined, "dark", "light"); + }); + + afterEach(() => { + settings.clearOverride("tui.hyperlinks"); + TERMINAL.eagerEraseScrollbackRisk = prevRisk; + vi.restoreAllMocks(); + }); + + afterAll(() => resetSettingsForTest()); + + // Regression: a parallel sibling tool finalizes the read group (breaks the + // run) and appends a block below it before the read's result lands. On + // ED3-risk terminals the container froze the group at its pending preview, so + // the late success result never repainted — the read stuck on "⏳ Read ". + it("repaints a late read result instead of freezing the pending preview", () => { + prevRisk = TERMINAL.eagerEraseScrollbackRisk; + TERMINAL.eagerEraseScrollbackRisk = true; + + const tc = new TranscriptContainer(); + const group = new ReadToolGroupComponent(); + group.updateArgs({ path: "/tmp/example.ts", sel: "280-345" }, "id1"); + tc.addChild(group); + tc.render(120); // Frame 1: group is the live (pending) block. + + // Sibling tool starts: group is closed to new entries and a non-finalized + // block is appended below it, all before the read result arrives. + group.finalize(); + tc.addChild(new StubBlock(false)); + tc.render(120); // Frame 2: group would cross out of the live region. + + group.updateResult(successResult(), false, "id1"); // Late result. + + const out = Bun.stripANSI(tc.render(120).join("\n")); + expect(out).toContain("Read /tmp/example.ts:280-345"); + expect(out).toContain(themeModule.theme.status.enabled); + expect(out).not.toContain(themeModule.theme.status.pending); + }); + + // The finalization seam the TranscriptContainer keys off of. + it("stays live until pending entries settle, then reports finalized", () => { + prevRisk = TERMINAL.eagerEraseScrollbackRisk; + const group = new ReadToolGroupComponent(); + group.updateArgs({ path: "/tmp/a.ts" }, "id1"); + + // Open run → never finalized. + expect(group.isTranscriptBlockFinalized()).toBe(false); + + // Closed run but the read is still in flight → stay live so the result can + // still repaint. + group.finalize(); + expect(group.isTranscriptBlockFinalized()).toBe(false); + + // Result settled → safe to freeze. + group.updateResult(successResult(), false, "id1"); + expect(group.isTranscriptBlockFinalized()).toBe(true); + }); + + // Turn-end safety: a read that never delivers a result (aborted turn) must not + // pin the live region forever. seal() forces it terminal. + it("seals a never-resolved pending read so it can freeze", () => { + prevRisk = TERMINAL.eagerEraseScrollbackRisk; + const group = new ReadToolGroupComponent(); + group.updateArgs({ path: "/tmp/a.ts" }, "id1"); + group.finalize(); + expect(group.isTranscriptBlockFinalized()).toBe(false); + + group.seal(); + expect(group.isTranscriptBlockFinalized()).toBe(true); + }); +});