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`.
This commit is contained in:
can1357
2026-06-08 06:28:40 +02:00
parent 573fb8a9a6
commit b5eff5b3d3
3 changed files with 132 additions and 2 deletions
@@ -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 <path>").
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 || "";
@@ -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);
}
}
@@ -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 <path>".
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);
});
});