From df2bbb9b423da747d1a23cecc4d162c140c806f0 Mon Sep 17 00:00:00 2001 From: probe Date: Sat, 8 Aug 2026 03:56:49 +0900 Subject: [PATCH 1/3] fix(tui): land deferred shell-mode output in the transcript MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `!cmd` typed while the agent was Working ran, then vanished: completion detached the block from the pending container without ever re-parenting it, so it lived on in a plain array with no parent and was never drawn. Ordinary mid-turn events made it worse — `updatePendingMessagesDisplay()` and the transcript rebuild both called `Container.clear()`, which disposes children, tearing down a still-streaming block and dropping its buffered output. Completion now moves the block into the chat transcript, and both rebuild paths detach-and-reattach parked execution components instead of disposing them. `Container.clear()` keeps its disposing contract; the retention lives in a private helper on the coding-agent side. Lore-id: 5b81e40c Constraint: Container.clear() must keep disposing children -- other callers depend on it Constraint: a running block must stay parented while it streams, not be flushed early into chat Rejected: flush parked components on the streaming submit path | an in-flight block would jump into the transcript before it finished Rejected: change clear() to detach instead of dispose | silently leaks every other container's children Confidence: high Scope-risk: narrow Reversibility: easy Tested: `!` submitted mid-turn is parented while streaming and lands in the transcript on completion Tested: pending-queue refresh and transcript rebuild no longer dispose a running execution block Not-tested: interactive TUI smoke on a real terminal --- .../modes/controllers/command-controller.ts | 13 +- .../src/modes/utils/ui-helpers.ts | 47 ++- .../test/input-controller-skill-queue.test.ts | 2 + .../modes/controllers/bash-command.test.ts | 327 +++++++++++++++--- .../render-initial-messages-dedupe.test.ts | 3 +- 5 files changed, 345 insertions(+), 47 deletions(-) diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 1690ba9698..6175102e4c 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -39,7 +39,7 @@ import { getDisplayChangelogEntries } from "../../utils/changelog"; import { copyToClipboard } from "../../utils/clipboard"; import { openPath } from "../../utils/open"; import { setSessionTerminalTitle } from "../../utils/title-generator"; -import { prepareTranscriptRebuild } from "../utils/ui-helpers"; +import { addChatChild, prepareTranscriptRebuild } from "../utils/ui-helpers"; type HindsightModule = typeof import("../../hindsight"); let hindsightModulePromise: Promise | undefined; @@ -1182,8 +1182,15 @@ export class CommandController { this.ctx.showError(`Bash command failed: ${error instanceof Error ? error.message : "Unknown error"}`); } const bashComponent = this.ctx.bashComponent; - if (isDeferred && bashComponent && this.ctx.pendingBashComponents.includes(bashComponent)) { - this.ctx.pendingMessagesContainer.detachChild(bashComponent); + if (isDeferred && bashComponent) { + const parkedIndex = this.ctx.pendingBashComponents.indexOf(bashComponent); + // A parked component is only ours to move while it is still parked: a + // non-streaming submit may already have flushed it into the transcript. + if (parkedIndex !== -1) { + this.ctx.pendingBashComponents.splice(parkedIndex, 1); + this.ctx.pendingMessagesContainer.detachChild(bashComponent); + addChatChild(this.ctx, bashComponent); + } } this.ctx.bashComponent = undefined; diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 94e83c0b4f..26e76d3751 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -899,10 +899,20 @@ export class UiHelpers { // This path is used to rebuild the visible chat transcript (e.g. after custom/debug UI). // Clear existing rendered chat first to avoid duplicating the full session in the container. const preservedChatChildren = options.preserveExistingChat ? this.ctx.chatContainer.children : undefined; + // A still-running deferred `!`/`$` block is the only rendering of output whose + // message has not been published to the session yet, so the rebuild must keep it + // parked instead of disposing it. Finished blocks are dropped here: the rebuilt + // transcript renders them from the session. + const runningExecutionComponents = this.#detachPendingMessages( + component => component === this.ctx.bashComponent || component === this.ctx.pythonComponent, + ); + this.ctx.pendingBashComponents = this.ctx.pendingBashComponents.filter(component => + runningExecutionComponents.includes(component), + ); + this.ctx.pendingPythonComponents = this.ctx.pendingPythonComponents.filter(component => + runningExecutionComponents.includes(component), + ); this.ctx.chatContainer.clear(); - this.ctx.pendingMessagesContainer.clear(); - this.ctx.pendingBashComponents = []; - this.ctx.pendingPythonComponents = []; // Reuse a pre-built context when available (e.g. from navigateTree) to avoid a second O(N) walk. const context = prebuiltContext ?? this.ctx.sessionManager.buildSessionContext(); @@ -923,6 +933,9 @@ export class UiHelpers { const times = compactionCount === 1 ? "1 time" : `${compactionCount} times`; this.ctx.showStatus(`Session compacted ${times}`); } + for (const component of runningExecutionComponents) { + this.ctx.pendingMessagesContainer.addChild(component); + } if (preservedChatChildren && preservedChatChildren.length > 0) { for (const child of preservedChatChildren) { addChatChild(this.ctx, child); @@ -978,8 +991,34 @@ export class UiHelpers { this.ctx.ui.requestRender(); } + /** + * Empty the pending container, disposing the queued-message chips but handing back + * the parked `!`/`$` execution components matched by `retain`, in render order. + * + * `Container.clear()` disposes every child, which would tear down a running + * execution block mid-flight; retained components are reused instances that the + * caller re-attaches (pending area or chat transcript). + */ + #detachPendingMessages(retain: (component: Component) => boolean): Component[] { + const parked = new Set([...this.ctx.pendingBashComponents, ...this.ctx.pendingPythonComponents]); + const retained: Component[] = []; + for (const child of this.ctx.pendingMessagesContainer.children) { + if (parked.has(child) && retain(child)) { + retained.push(child); + } else { + child.dispose?.(); + } + } + this.ctx.pendingMessagesContainer.detachAll(); + return retained; + } + updatePendingMessagesDisplay(): void { - this.ctx.pendingMessagesContainer.clear(); + // Rebuild only the queued-message chips: parked execution components stay attached + // so a mid-turn queue/dequeue event cannot dispose a streaming `!`/`$` block. + for (const component of this.#detachPendingMessages(() => true)) { + this.ctx.pendingMessagesContainer.addChild(component); + } const queuedMessages = this.ctx.session.getQueuedMessages() as QueuedMessages; const steeringMessages: Array<{ message: string; label: string }> = []; diff --git a/packages/coding-agent/test/input-controller-skill-queue.test.ts b/packages/coding-agent/test/input-controller-skill-queue.test.ts index 85ba13b51d..5133d11d42 100644 --- a/packages/coding-agent/test/input-controller-skill-queue.test.ts +++ b/packages/coding-agent/test/input-controller-skill-queue.test.ts @@ -586,6 +586,8 @@ function createStubInteractiveModeContextForUiHelpers(session: AgentSession, deq editor, ui: { requestRender }, pendingMessagesContainer, + pendingBashComponents: [], + pendingPythonComponents: [], session, compactionQueuedMessages: [], keybindings: { diff --git a/packages/coding-agent/test/modes/controllers/bash-command.test.ts b/packages/coding-agent/test/modes/controllers/bash-command.test.ts index 89d6293e20..f77e1b5127 100644 --- a/packages/coding-agent/test/modes/controllers/bash-command.test.ts +++ b/packages/coding-agent/test/modes/controllers/bash-command.test.ts @@ -1,9 +1,16 @@ +/** + * Regression coverage for issue #3639: a `!cmd` submitted while the agent is + * streaming must render its header/output/exit status continuously and land in + * the chat transcript on completion, exactly like the idle path. + */ import { beforeAll, describe, expect, it } from "bun:test"; import { BashExecutionComponent } from "@gajae-code/coding-agent/modes/components/bash-execution"; +import type { EvalExecutionComponent } from "@gajae-code/coding-agent/modes/components/eval-execution"; import { CommandController } from "@gajae-code/coding-agent/modes/controllers/command-controller"; import { getThemeByName, setThemeInstance } from "@gajae-code/coding-agent/modes/theme/theme"; import type { InteractiveModeContext } from "@gajae-code/coding-agent/modes/types"; import { UiHelpers } from "@gajae-code/coding-agent/modes/utils/ui-helpers"; +import type { SessionContext } from "@gajae-code/coding-agent/session/session-manager"; import { Container, type TUI } from "@gajae-code/tui"; beforeAll(async () => { @@ -12,45 +19,287 @@ beforeAll(async () => { setThemeInstance(theme!); }); -describe("shell command display", () => { - it("removes a completed deferred command from the pending surface", async () => { - const chatContainer = new Container(); - const pendingMessagesContainer = new Container(); - const pendingBashComponents: BashExecutionComponent[] = []; - const ui = { requestRender: () => {} } as unknown as TUI; - const ctx = { - session: { - isStreaming: true, - executeBash: async () => ({ - exitCode: 0, - cancelled: false, - output: "clean", - truncated: false, - }), +interface ExecutionResult { + exitCode: number | undefined; + cancelled: boolean; + output: string; + truncated: boolean; +} + +function emptySessionContext(): SessionContext { + return { + messages: [], + thinkingLevel: "off", + serviceTier: undefined, + models: {}, + configuredModelChains: {}, + injectedTtsrRules: [], + selectedMCPToolNames: [], + hasPersistedMCPToolSelection: false, + mode: "none", + }; +} + +interface Harness { + ctx: InteractiveModeContext; + chatContainer: Container; + pendingMessagesContainer: Container; + bashGate: PromiseWithResolvers; + evalGate: PromiseWithResolvers; + emitBashChunk(chunk: string): void; + emitEvalChunk(chunk: string): void; + queuedFollowUps: string[]; + rebuiltTranscriptRows: string[]; +} + +/** + * Interactive-mode context stub wired with the containers/arrays the deferred + * execution paths actually touch. Both executions are gated so the test can + * observe the in-flight window. + */ +function createHarness(options: { isStreaming: boolean }): Harness { + const chatContainer = new Container(); + const pendingMessagesContainer = new Container(); + const ui = { requestRender: () => {} } as unknown as TUI; + const bashGate = Promise.withResolvers(); + const evalGate = Promise.withResolvers(); + const queuedFollowUps: string[] = []; + const rebuiltTranscriptRows: string[] = []; + let bashChunkSink: ((chunk: string) => void) | undefined; + let evalChunkSink: ((chunk: string) => void) | undefined; + + const ctx = { + session: { + isStreaming: options.isStreaming, + executeBash: (_command: string, onChunk: (chunk: string) => void) => { + bashChunkSink = onChunk; + return bashGate.promise; }, - ui, - chatContainer, - pendingMessagesContainer, - pendingBashComponents, - pendingPythonComponents: [], - pendingTools: new Map(), - bashComponent: undefined, - pythonComponent: undefined, - streamingComponent: undefined, - showError: () => {}, - } as unknown as InteractiveModeContext; - - await new CommandController(ctx).handleBashCommand("printf clean"); - - expect(pendingMessagesContainer.children).toHaveLength(0); - expect(ctx.pendingBashComponents).toHaveLength(1); - expect(chatContainer.children).toHaveLength(0); - expect(ctx.bashComponent).toBeUndefined(); - - new UiHelpers(ctx).flushPendingBashComponents(); - - expect(ctx.pendingBashComponents).toHaveLength(0); - expect(chatContainer.children).toHaveLength(1); - expect(chatContainer.children[0]).toBeInstanceOf(BashExecutionComponent); + executePython: (_code: string, onChunk: (chunk: string) => void) => { + evalChunkSink = onChunk; + return evalGate.promise; + }, + getQueuedMessages: () => ({ steering: [], followUp: queuedFollowUps }), + }, + sessionManager: { + buildSessionContext: () => emptySessionContext(), + getEntries: () => [], + getCwd: () => "/tmp", + }, + renderSessionContext: () => { + for (const row of rebuiltTranscriptRows) { + chatContainer.addChild(new BashExecutionComponent(row, ui)); + } + }, + ui, + chatContainer, + pendingMessagesContainer, + pendingBashComponents: [] as BashExecutionComponent[], + pendingPythonComponents: [] as EvalExecutionComponent[], + pendingTools: new Map(), + compactionQueuedMessages: [], + keybindings: { getDisplayString: () => "Alt+Up" }, + bashComponent: undefined, + pythonComponent: undefined, + streamingComponent: undefined, + showError: () => {}, + showStatus: () => {}, + } as unknown as InteractiveModeContext; + + return { + ctx, + chatContainer, + pendingMessagesContainer, + bashGate, + evalGate, + emitBashChunk: chunk => bashChunkSink?.(chunk), + emitEvalChunk: chunk => evalChunkSink?.(chunk), + queuedFollowUps, + rebuiltTranscriptRows, + }; +} + +/** Let the controller reach its awaited execution call. */ +async function settle(): Promise { + await Bun.sleep(0); +} + +describe("deferred shell command display", () => { + it("keeps a mid-turn bash command parented and rendered while it streams", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("printf hello"); + await settle(); + + expect(harness.pendingMessagesContainer.children).toHaveLength(1); + expect(harness.ctx.pendingBashComponents).toHaveLength(1); + + harness.emitBashChunk("hello"); + const streaming = harness.pendingMessagesContainer.render(80).join("\n"); + expect(streaming).toContain("$ printf hello"); + expect(streaming).toContain("hello"); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "hello", truncated: false }); + await run; + }); + + it("moves a completed mid-turn bash command into the chat transcript with its exit status", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("exit 3"); + await settle(); + harness.bashGate.resolve({ exitCode: 3, cancelled: false, output: "boom", truncated: false }); + await run; + + expect(harness.pendingMessagesContainer.children).toHaveLength(0); + expect(harness.ctx.pendingBashComponents).toHaveLength(0); + expect(harness.chatContainer.children).toHaveLength(1); + expect(harness.chatContainer.children[0]).toBeInstanceOf(BashExecutionComponent); + expect(harness.ctx.bashComponent).toBeUndefined(); + + const transcript = harness.chatContainer.render(80).join("\n"); + expect(transcript).toContain("$ exit 3"); + expect(transcript).toContain("boom"); + expect(transcript).toContain("(exit 3)"); + }); + + it("lands an idle bash command in the same place as a deferred one", async () => { + const harness = createHarness({ isStreaming: false }); + const run = new CommandController(harness.ctx).handleBashCommand("printf idle"); + await settle(); + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "idle", truncated: false }); + await run; + + expect(harness.pendingMessagesContainer.children).toHaveLength(0); + expect(harness.chatContainer.children).toHaveLength(1); + expect(harness.chatContainer.render(80).join("\n")).toContain("$ printf idle"); + }); + + it("does not re-add a bash component that a normal submit already flushed to chat", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("sleep 1"); + await settle(); + + // Turn ended while the command was still running and the user submitted a + // new prompt: the live component is flushed into the transcript early. + new UiHelpers(harness.ctx).flushPendingBashComponents(); + expect(harness.chatContainer.children).toHaveLength(1); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "done", truncated: false }); + await run; + + expect(harness.chatContainer.children).toHaveLength(1); + expect(harness.pendingMessagesContainer.children).toHaveLength(0); + }); + + it("survives a queued-message rebuild while the bash command is still running", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("printf mid"); + await settle(); + harness.emitBashChunk("mid"); + + const liveComponent = harness.ctx.pendingBashComponents[0]; + harness.queuedFollowUps.push("queued prompt"); + new UiHelpers(harness.ctx).updatePendingMessagesDisplay(); + + expect(harness.pendingMessagesContainer.children).toContain(liveComponent); + expect(harness.ctx.pendingBashComponents).toHaveLength(1); + + const rendered = harness.pendingMessagesContainer.render(80).join("\n"); + expect(rendered).toContain("$ printf mid"); + expect(rendered).toContain("mid"); + expect(rendered).toContain("Queued: queued prompt"); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "mid", truncated: false }); + await run; + + expect(harness.chatContainer.children).toEqual([liveComponent]); + expect(harness.chatContainer.render(80).join("\n")).toContain("$ printf mid"); + }); + + it("drops only the queued chips when the pending display is rebuilt twice", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("printf twice"); + await settle(); + + harness.queuedFollowUps.push("queued prompt"); + const helpers = new UiHelpers(harness.ctx); + helpers.updatePendingMessagesDisplay(); + helpers.updatePendingMessagesDisplay(); + + // One live component + spacer + queued chip + dequeue hint, never duplicated. + expect(harness.pendingMessagesContainer.children).toHaveLength(4); + expect(harness.pendingMessagesContainer.children[0]).toBe(harness.ctx.pendingBashComponents[0]); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "twice", truncated: false }); + await run; + }); + + it("keeps a parked bash command through a transcript rebuild", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("printf parked"); + await settle(); + harness.emitBashChunk("parked"); + + const liveComponent = harness.ctx.pendingBashComponents[0]; + harness.rebuiltTranscriptRows.push("earlier command"); + new UiHelpers(harness.ctx).renderInitialMessages(); + + expect(harness.ctx.pendingBashComponents).toEqual([liveComponent]); + expect(harness.pendingMessagesContainer.children).toEqual([liveComponent]); + expect(harness.pendingMessagesContainer.render(80).join("\n")).toContain("$ printf parked"); + expect(harness.chatContainer.render(80).join("\n")).toContain("$ earlier command"); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "parked", truncated: false }); + await run; + + expect(harness.chatContainer.children).toContain(liveComponent); + }); + + it("drops a finished parked python block on a transcript rebuild so it is not rendered twice", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handlePythonCommand("print('done')"); + await settle(); + harness.evalGate.resolve({ exitCode: 0, cancelled: false, output: "done", truncated: false }); + await run; + + // Finished but still parked: the rebuilt transcript is the session's job now. + expect(harness.ctx.pendingPythonComponents).toHaveLength(1); + + harness.rebuiltTranscriptRows.push("print('done')"); + new UiHelpers(harness.ctx).renderInitialMessages(); + + expect(harness.ctx.pendingPythonComponents).toHaveLength(0); + expect(harness.pendingMessagesContainer.children).toHaveLength(0); + expect(harness.chatContainer.children).toHaveLength(1); + }); + + it("keeps a mid-turn python command visible across a queued-message rebuild and a transcript rebuild", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handlePythonCommand("print('py')"); + await settle(); + harness.emitEvalChunk("py"); + + const liveComponent = harness.ctx.pendingPythonComponents[0]; + expect(harness.pendingMessagesContainer.children).toContain(liveComponent); + + const helpers = new UiHelpers(harness.ctx); + harness.queuedFollowUps.push("queued prompt"); + helpers.updatePendingMessagesDisplay(); + expect(harness.pendingMessagesContainer.children).toContain(liveComponent); + expect(harness.pendingMessagesContainer.render(80).join("\n")).toContain("py"); + + helpers.renderInitialMessages(); + expect(harness.ctx.pendingPythonComponents).toEqual([liveComponent]); + expect(harness.pendingMessagesContainer.children).toEqual([liveComponent]); + + harness.evalGate.resolve({ exitCode: 0, cancelled: false, output: "py", truncated: false }); + await run; + + // `$` still parks until the next non-streaming submit flushes it. + expect(harness.pendingMessagesContainer.children).toEqual([liveComponent]); + expect(harness.ctx.pendingPythonComponents).toEqual([liveComponent]); + + helpers.flushPendingBashComponents(); + expect(harness.chatContainer.children).toEqual([liveComponent]); + expect(harness.ctx.pendingPythonComponents).toHaveLength(0); }); }); diff --git a/packages/coding-agent/test/modes/utils/render-initial-messages-dedupe.test.ts b/packages/coding-agent/test/modes/utils/render-initial-messages-dedupe.test.ts index 327a2a0004..913381e985 100644 --- a/packages/coding-agent/test/modes/utils/render-initial-messages-dedupe.test.ts +++ b/packages/coding-agent/test/modes/utils/render-initial-messages-dedupe.test.ts @@ -22,6 +22,7 @@ import type { InteractiveModeContext } from "@gajae-code/coding-agent/modes/type import { UiHelpers } from "@gajae-code/coding-agent/modes/utils/ui-helpers"; import type { SessionContext } from "@gajae-code/coding-agent/session/session-manager"; import { SessionManager } from "@gajae-code/coding-agent/session/session-manager"; +import { Container } from "@gajae-code/tui"; beforeAll(() => { initTheme(); @@ -59,7 +60,7 @@ function makeCtx(sessionManager?: Pick Date: Sat, 8 Aug 2026 23:48:46 +0900 Subject: [PATCH 2/3] fix(tui): re-parent a deferred block only while the container still owns it The completion guard checked array membership, not parentage. Several clearing paths (command-controller /clear flows, extension-ui, selector) clear pendingMessagesContainer without resetting pendingBashComponents, so a deferred command finishing after /clear passed the stale index check and a DISPOSED component was re-parented into a transcript it was never part of. The container now answers ownership itself: a non-disposing liveness query on Container reports whether a child is still live under it, and the completion path treats disposed as terminal. Container.clear() keeps its disposing contract for every other caller. Lore-id: 6a1c8f35 Constraint: Container.clear() semantics unchanged for existing callers Constraint: a disposed component is never re-parented Rejected: syncing the array at every clear site | five call sites today, the sixth would miss it the same way Confidence: high Scope-risk: narrow Reversibility: easy Tested: /clear during an in-flight deferred command re-parents nothing and does not crash Tested: a deferred command completing normally still lands exactly once Tested: a pending-queue refresh still does not dispose a running block Not-tested: a live TUI session under manual /clear stress --- .../modes/controllers/command-controller.ts | 15 +- .../src/modes/utils/ui-helpers.ts | 30 +++- .../modes/controllers/bash-command.test.ts | 167 +++++++++++++++++- packages/tui/src/tui.ts | 14 ++ 4 files changed, 212 insertions(+), 14 deletions(-) diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 6175102e4c..97348d8416 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -39,7 +39,7 @@ import { getDisplayChangelogEntries } from "../../utils/changelog"; import { copyToClipboard } from "../../utils/clipboard"; import { openPath } from "../../utils/open"; import { setSessionTerminalTitle } from "../../utils/title-generator"; -import { addChatChild, prepareTranscriptRebuild } from "../utils/ui-helpers"; +import { addChatChild, prepareTranscriptRebuild, syncPendingExecutionComponents } from "../utils/ui-helpers"; type HindsightModule = typeof import("../../hindsight"); let hindsightModulePromise: Promise | undefined; @@ -1183,14 +1183,17 @@ export class CommandController { } const bashComponent = this.ctx.bashComponent; if (isDeferred && bashComponent) { - const parkedIndex = this.ctx.pendingBashComponents.indexOf(bashComponent); - // A parked component is only ours to move while it is still parked: a - // non-streaming submit may already have flushed it into the transcript. - if (parkedIndex !== -1) { - this.ctx.pendingBashComponents.splice(parkedIndex, 1); + // Parentage is the container's answer, never this bookkeeping's: `/clear`, + // `/context-clear`, extension redraws and the selector all call + // `pendingMessagesContainer.clear()`, which disposes and evicts the parked + // component while leaving it listed in `pendingBashComponents`. Ask the + // container whether it still holds a live child before moving anything; + // a disposed block is terminal and must never reach a fresh transcript. + if (this.ctx.pendingMessagesContainer.hasLiveChild(bashComponent)) { this.ctx.pendingMessagesContainer.detachChild(bashComponent); addChatChild(this.ctx, bashComponent); } + syncPendingExecutionComponents(this.ctx); } this.ctx.bashComponent = undefined; diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 26e76d3751..6f30df131d 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -281,6 +281,20 @@ export function addChatChild(ctx: InteractiveModeContext, component: Component): trimChatChildren(ctx); } +/** + * Parked `!`/`$` execution components are listed in `pendingBashComponents` / + * `pendingPythonComponents`, but `pendingMessagesContainer` is the only + * authority on parentage: `/clear`, `/context-clear`, extension redraws and the + * selector all call `pendingMessagesContainer.clear()`, which disposes and + * evicts parked components without touching those arrays. Drop every entry the + * container no longer holds so nothing downstream can move a dead component. + */ +export function syncPendingExecutionComponents(ctx: InteractiveModeContext): void { + const container = ctx.pendingMessagesContainer; + ctx.pendingBashComponents = ctx.pendingBashComponents.filter(component => container.hasLiveChild(component)); + ctx.pendingPythonComponents = ctx.pendingPythonComponents.filter(component => container.hasLiveChild(component)); +} + export function trimChatChildren(ctx: InteractiveModeContext): void { const children = ctx.chatContainer.children; @@ -1000,6 +1014,7 @@ export class UiHelpers { * caller re-attaches (pending area or chat transcript). */ #detachPendingMessages(retain: (component: Component) => boolean): Component[] { + syncPendingExecutionComponents(this.ctx); const parked = new Set([...this.ctx.pendingBashComponents, ...this.ctx.pendingPythonComponents]); const retained: Component[] = []; for (const child of this.ctx.pendingMessagesContainer.children) { @@ -1276,20 +1291,21 @@ export class UiHelpers { } } - /** Move pending bash components from pending area to chat */ + /** Move pending bash/python components from the pending area to chat */ flushPendingBashComponents(): void { // Move (detach, not dispose) the live execution components from the pending // area into the chat transcript — they are reused instances, so a disposing - // removeChild() would tear them down before re-adding. - for (const component of this.ctx.pendingBashComponents) { + // removeChild() would tear them down before re-adding. Walk the container so + // the transcript keeps the order the pending area rendered, and so a + // component the container no longer holds can never be re-parented. + syncPendingExecutionComponents(this.ctx); + const parked = new Set([...this.ctx.pendingBashComponents, ...this.ctx.pendingPythonComponents]); + for (const component of [...this.ctx.pendingMessagesContainer.children]) { + if (!parked.has(component)) continue; this.ctx.pendingMessagesContainer.detachChild(component); addChatChild(this.ctx, component); } this.ctx.pendingBashComponents = []; - for (const component of this.ctx.pendingPythonComponents) { - this.ctx.pendingMessagesContainer.detachChild(component); - addChatChild(this.ctx, component); - } this.ctx.pendingPythonComponents = []; } diff --git a/packages/coding-agent/test/modes/controllers/bash-command.test.ts b/packages/coding-agent/test/modes/controllers/bash-command.test.ts index f77e1b5127..e1b9fb5160 100644 --- a/packages/coding-agent/test/modes/controllers/bash-command.test.ts +++ b/packages/coding-agent/test/modes/controllers/bash-command.test.ts @@ -60,7 +60,11 @@ interface Harness { function createHarness(options: { isStreaming: boolean }): Harness { const chatContainer = new Container(); const pendingMessagesContainer = new Container(); - const ui = { requestRender: () => {} } as unknown as TUI; + const ui = { + requestRender: () => {}, + resetViewportAnchorIntent: () => {}, + prepareViewportAnchorForTranscriptRebuild: () => {}, + } as unknown as TUI; const bashGate = Promise.withResolvers(); const evalGate = Promise.withResolvers(); const queuedFollowUps: string[] = []; @@ -80,11 +84,15 @@ function createHarness(options: { isStreaming: boolean }): Harness { return evalGate.promise; }, getQueuedMessages: () => ({ steering: [], followUp: queuedFollowUps }), + isCompacting: false, + newSession: async () => true, + clearContext: async () => true, }, sessionManager: { buildSessionContext: () => emptySessionContext(), getEntries: () => [], getCwd: () => "/tmp", + getSessionName: () => "harness session", }, renderSessionContext: () => { for (const row of rebuiltTranscriptRows) { @@ -104,6 +112,13 @@ function createHarness(options: { isStreaming: boolean }): Harness { streamingComponent: undefined, showError: () => {}, showStatus: () => {}, + statusLine: { invalidate: () => {}, setSessionStartTime: () => {} }, + updateEditorTopBorder: () => {}, + updateEditorBorderColor: () => {}, + resetIrcSidebarSession: () => {}, + resetObserverRegistry: () => {}, + reloadTodos: async () => {}, + isStopped: () => false, } as unknown as InteractiveModeContext; return { @@ -303,3 +318,153 @@ describe("deferred shell command display", () => { expect(harness.ctx.pendingPythonComponents).toHaveLength(0); }); }); + +/** + * Regression coverage for the parentage guard: array membership in + * `pendingBashComponents` proves nothing about who owns the component. Every + * transcript-clearing path calls `pendingMessagesContainer.clear()`, which + * disposes and evicts the parked block without touching the array. + */ +describe("deferred shell command parentage after the transcript is cleared", () => { + it("re-parents nothing when /clear disposes an in-flight bash block", async () => { + const harness = createHarness({ isStreaming: true }); + const controller = new CommandController(harness.ctx); + const run = controller.handleBashCommand("sleep 5"); + await settle(); + + const parked = harness.ctx.pendingBashComponents[0]; + expect(harness.pendingMessagesContainer.children).toContain(parked); + + expect(await controller.handleClearCommand()).toBe(true); + expect(harness.pendingMessagesContainer.hasLiveChild(parked)).toBe(false); + const chatChildrenAfterClear = harness.chatContainer.children.length; + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "late", truncated: false }); + await run; + + expect(harness.chatContainer.children).not.toContain(parked); + expect(harness.chatContainer.children).toHaveLength(chatChildrenAfterClear); + expect(harness.ctx.pendingBashComponents).toHaveLength(0); + expect(harness.pendingMessagesContainer.children).toHaveLength(0); + }); + + it("re-parents nothing when /context-clear disposes an in-flight bash block", async () => { + const harness = createHarness({ isStreaming: true }); + const controller = new CommandController(harness.ctx); + const run = controller.handleBashCommand("sleep 5"); + await settle(); + + const parked = harness.ctx.pendingBashComponents[0]; + await controller.handleContextClearCommand(); + expect(harness.pendingMessagesContainer.hasLiveChild(parked)).toBe(false); + const chatChildrenAfterClear = harness.chatContainer.children.length; + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "late", truncated: false }); + await run; + + expect(harness.chatContainer.children).not.toContain(parked); + expect(harness.chatContainer.children).toHaveLength(chatChildrenAfterClear); + expect(harness.ctx.pendingBashComponents).toHaveLength(0); + }); + + it("re-parents nothing after a bare pending-area clear, and a later flush cannot resurrect it", async () => { + // Shape shared by extension-ui-controller.ts:679/993 and selector-controller.ts:2681. + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("sleep 5"); + await settle(); + + const parked = harness.ctx.pendingBashComponents[0]; + harness.chatContainer.clear(); + harness.pendingMessagesContainer.clear(); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "late", truncated: false }); + await run; + + expect(harness.chatContainer.children).toHaveLength(0); + expect(harness.ctx.pendingBashComponents).toHaveLength(0); + + new UiHelpers(harness.ctx).flushPendingBashComponents(); + expect(harness.chatContainer.children).toHaveLength(0); + expect(harness.chatContainer.children).not.toContain(parked); + }); + + it("keeps a cleared python block out of the transcript on the next flush", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handlePythonCommand("print('py')"); + await settle(); + + const parked = harness.ctx.pendingPythonComponents[0]; + harness.chatContainer.clear(); + harness.pendingMessagesContainer.clear(); + + harness.evalGate.resolve({ exitCode: 0, cancelled: false, output: "py", truncated: false }); + await run; + + new UiHelpers(harness.ctx).flushPendingBashComponents(); + expect(harness.chatContainer.children).toHaveLength(0); + expect(harness.chatContainer.children).not.toContain(parked); + expect(harness.ctx.pendingPythonComponents).toHaveLength(0); + }); + + it("drops stale entries on a pending-queue refresh without disposing a live block", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("printf mid"); + await settle(); + harness.emitBashChunk("mid"); + const live = harness.ctx.pendingBashComponents[0]; + + // What a transcript clear leaves behind: an entry the container no longer holds. + harness.ctx.pendingBashComponents.push(new BashExecutionComponent("stale", harness.ctx.ui)); + + harness.queuedFollowUps.push("queued prompt"); + new UiHelpers(harness.ctx).updatePendingMessagesDisplay(); + + expect(harness.ctx.pendingBashComponents).toEqual([live]); + expect(harness.pendingMessagesContainer.children).toContain(live); + expect(harness.pendingMessagesContainer.render(80).join("\n")).toContain("$ printf mid"); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "mid", truncated: false }); + await run; + expect(harness.chatContainer.children).toEqual([live]); + }); + + it("lands a normally completing deferred bash block in the transcript exactly once", async () => { + const harness = createHarness({ isStreaming: true }); + const run = new CommandController(harness.ctx).handleBashCommand("printf once"); + await settle(); + const parked = harness.ctx.pendingBashComponents[0]; + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "once", truncated: false }); + await run; + + expect(harness.chatContainer.children.filter(child => child === parked)).toHaveLength(1); + expect(harness.pendingMessagesContainer.children).toHaveLength(0); + expect(harness.ctx.pendingBashComponents).toHaveLength(0); + + // A later flush must not append a second copy. + new UiHelpers(harness.ctx).flushPendingBashComponents(); + expect(harness.chatContainer.children.filter(child => child === parked)).toHaveLength(1); + }); + + it("flushes parked blocks into the transcript in pending render order", async () => { + const harness = createHarness({ isStreaming: true }); + const pythonRun = new CommandController(harness.ctx).handlePythonCommand("print('first')"); + await settle(); + harness.evalGate.resolve({ exitCode: 0, cancelled: false, output: "first", truncated: false }); + await pythonRun; + + const bashRun = new CommandController(harness.ctx).handleBashCommand("printf second"); + await settle(); + + const parkedPython = harness.ctx.pendingPythonComponents[0]; + const parkedBash = harness.ctx.pendingBashComponents[0]; + expect(harness.pendingMessagesContainer.children).toEqual([parkedPython, parkedBash]); + + new UiHelpers(harness.ctx).flushPendingBashComponents(); + expect(harness.chatContainer.children).toEqual([parkedPython, parkedBash]); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "second", truncated: false }); + await bashRun; + expect(harness.chatContainer.children).toEqual([parkedPython, parkedBash]); + }); +}); diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 215660a9e5..3cf5bf279b 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -534,6 +534,20 @@ export class Container implements ViewportAnchorProvider { } } + /** + * Non-disposing parentage query: is `component` still a live direct child? + * + * Callers that park a component here and later move it elsewhere must ask + * this instead of consulting their own bookkeeping — `clear()`/`dispose()` + * tear children down without notifying anyone. Disposal is terminal: a + * disposed child, or a child of a disposed container, is never live. + */ + hasLiveChild(component: Component): boolean { + if (this.#disposed) return false; + if (component instanceof Container && component.#disposed) return false; + return this.children.includes(component); + } + clear(): void { for (const child of this.children) child.dispose?.(); this.children = []; From 1027a89c0148d74edd9569ac48d80e685e68e7fe Mon Sep 17 00:00:00 2001 From: probe Date: Sun, 9 Aug 2026 10:20:20 +0900 Subject: [PATCH 3/3] fix(tui): identify a parked execution by identity, not by matching text Rebuild reconciliation matched parked execution components by command text and occurrence count, so at the history cap an older persisted execution and a currently running one sharing the same command collapsed into each other: the live block could be dropped, or a finished one revived. Components are now reconciled by their own identity, which the rebuild already has, so identical command text is no longer load-bearing. Lore-id: 5a9d3f28 Constraint: a disposed component is still never re-parented Constraint: Container.clear() semantics unchanged for every other caller Rejected: hashing command text plus timestamp | two executions can legitimately share both Confidence: high Scope-risk: narrow Reversibility: easy Tested: identical commands across the history cap stay distinct through a rebuild Tested: /clear during an in-flight deferred command still re-parents nothing Tested: a completed deferred command still lands exactly once Not-tested: a live TUI under sustained rebuild pressure --- .../src/modes/components/bash-execution.ts | 14 +++++ .../src/modes/components/eval-execution.ts | 14 +++++ .../modes/controllers/command-controller.ts | 14 +++-- .../src/modes/utils/ui-helpers.ts | 19 +++++- .../coding-agent/src/session/agent-session.ts | 44 ++++++++----- .../modes/controllers/bash-command.test.ts | 63 +++++++++++++++++++ 6 files changed, 148 insertions(+), 20 deletions(-) diff --git a/packages/coding-agent/src/modes/components/bash-execution.ts b/packages/coding-agent/src/modes/components/bash-execution.ts index deb15a5203..8cc0987bed 100644 --- a/packages/coding-agent/src/modes/components/bash-execution.ts +++ b/packages/coding-agent/src/modes/components/bash-execution.ts @@ -51,6 +51,7 @@ export class BashExecutionComponent extends Container { #chunkGate = false; #contentContainer: Container; #headerText: Text; + #resultPersisted = false; constructor( private readonly command: string, @@ -259,4 +260,17 @@ export class BashExecutionComponent extends Container { getCommand(): string { return this.command; } + + /** + * Record that this execution's message reached session state, so a transcript + * rebuild renders it from the session instead of keeping this live block parked. + */ + markResultPersisted(): void { + this.#resultPersisted = true; + } + + /** Whether the session already holds this execution's message. */ + hasPersistedResult(): boolean { + return this.#resultPersisted; + } } diff --git a/packages/coding-agent/src/modes/components/eval-execution.ts b/packages/coding-agent/src/modes/components/eval-execution.ts index 093fe5fd9a..cd3beea33d 100644 --- a/packages/coding-agent/src/modes/components/eval-execution.ts +++ b/packages/coding-agent/src/modes/components/eval-execution.ts @@ -30,6 +30,7 @@ export class EvalExecutionComponent extends Container { #expanded = false; #contentContainer: Container; #headerText: Text; + #resultPersisted = false; #highlightLang(): "python" | "javascript" { return this.language === "js" ? "javascript" : "python"; @@ -165,4 +166,17 @@ export class EvalExecutionComponent extends Container { getCode(): string { return this.code; } + + /** + * Record that this execution's message reached session state, so a transcript + * rebuild renders it from the session instead of keeping this live block parked. + */ + markResultPersisted(): void { + this.#resultPersisted = true; + } + + /** Whether the session already holds this execution's message. */ + hasPersistedResult(): boolean { + return this.#resultPersisted; + } } diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 97348d8416..f4b555e284 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -1147,7 +1147,8 @@ export class CommandController { async handleBashCommand(command: string, excludeFromContext = false): Promise { const isDeferred = this.ctx.session.isStreaming; - this.ctx.bashComponent = new BashExecutionComponent(command, this.ctx.ui, excludeFromContext); + const component = new BashExecutionComponent(command, this.ctx.ui, excludeFromContext); + this.ctx.bashComponent = component; if (isDeferred) { this.ctx.pendingMessagesContainer.addChild(this.ctx.bashComponent); @@ -1165,7 +1166,9 @@ export class CommandController { this.ctx.bashComponent.appendOutput(chunk); } }, - { excludeFromContext }, + // A transcript rebuild racing this call must drop the parked block once the + // session owns its message, otherwise the rebuilt row is rendered twice. + { excludeFromContext, onPersisted: () => component.markResultPersisted() }, ); if (this.ctx.bashComponent) { @@ -1202,7 +1205,8 @@ export class CommandController { async handlePythonCommand(code: string, excludeFromContext = false): Promise { const isDeferred = this.ctx.session.isStreaming; - this.ctx.pythonComponent = new EvalExecutionComponent(code, this.ctx.ui, excludeFromContext); + const component = new EvalExecutionComponent(code, this.ctx.ui, excludeFromContext); + this.ctx.pythonComponent = component; if (isDeferred) { this.ctx.pendingMessagesContainer.addChild(this.ctx.pythonComponent); @@ -1220,7 +1224,9 @@ export class CommandController { this.ctx.pythonComponent.appendOutput(chunk); } }, - { excludeFromContext }, + // A transcript rebuild racing this call must drop the parked block once the + // session owns its message, otherwise the rebuilt row is rendered twice. + { excludeFromContext, onPersisted: () => component.markResultPersisted() }, ); if (this.ctx.pythonComponent) { diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 6f30df131d..1319b07f44 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -295,6 +295,17 @@ export function syncPendingExecutionComponents(ctx: InteractiveModeContext): voi ctx.pendingPythonComponents = ctx.pendingPythonComponents.filter(component => container.hasLiveChild(component)); } +/** + * Whether the session already owns this execution's message, i.e. the rebuilt + * transcript renders the block from session state. Set by the controller when + * `executeBash()` / `executePython()` reports the result as persisted. + */ +function hasPersistedExecutionResult(component: Component): boolean { + if (component instanceof BashExecutionComponent) return component.hasPersistedResult(); + if (component instanceof EvalExecutionComponent) return component.hasPersistedResult(); + return false; +} + export function trimChatChildren(ctx: InteractiveModeContext): void { const children = ctx.chatContainer.children; @@ -916,9 +927,13 @@ export class UiHelpers { // A still-running deferred `!`/`$` block is the only rendering of output whose // message has not been published to the session yet, so the rebuild must keep it // parked instead of disposing it. Finished blocks are dropped here: the rebuilt - // transcript renders them from the session. + // transcript renders them from the session. A block whose result was persisted + // while its controller was still suspended is finished for this purpose — keeping + // it would render the same execution twice. const runningExecutionComponents = this.#detachPendingMessages( - component => component === this.ctx.bashComponent || component === this.ctx.pythonComponent, + component => + (component === this.ctx.bashComponent || component === this.ctx.pythonComponent) && + !hasPersistedExecutionResult(component), ); this.ctx.pendingBashComponents = this.ctx.pendingBashComponents.filter(component => runningExecutionComponents.includes(component), diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index cb48980a26..328c91e785 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -2015,7 +2015,7 @@ export class AgentSession { // Bash execution state #bashAbortControllers = new Set(); - #pendingBashMessages: BashExecutionMessage[] = []; + #pendingBashMessages: Array<{ message: BashExecutionMessage; onPersisted?: () => void }> = []; #foregroundBashBackgroundRequestHandler: (() => void) | undefined; // Python execution state @@ -2031,7 +2031,7 @@ export class AgentSession { readonly #ownedAsyncJobManager: AsyncJobManager | undefined; readonly #ownedMcpManager: MCPManager | undefined; #startupTurnBarrier: Promise | undefined; - #pendingPythonMessages: PythonExecutionMessage[] = []; + #pendingPythonMessages: Array<{ message: PythonExecutionMessage; onPersisted?: () => void }> = []; #activeEvalExecutions = new Set>(); #evalExecutionDisposing = false; @@ -16086,11 +16086,13 @@ export class AgentSession { * @param command The bash command to execute * @param onChunk Optional streaming callback for output * @param options.excludeFromContext If true, command output won't be sent to LLM (!! prefix) + * @param options.onPersisted Called once the execution's message is in session state + * (immediately when idle, at the post-turn flush while streaming) */ async executeBash( command: string, onChunk?: (chunk: string) => void, - options?: { excludeFromContext?: boolean }, + options?: { excludeFromContext?: boolean; onPersisted?: () => void }, ): Promise { const excludeFromContext = options?.excludeFromContext === true; this.#markRetryReplayUnsafe(); @@ -16145,7 +16147,11 @@ export class AgentSession { * Record a bash execution result in session history. * Used by executeBash and by extensions that handle bash execution themselves. */ - recordBashResult(command: string, result: BashResult, options?: { excludeFromContext?: boolean }): void { + recordBashResult( + command: string, + result: BashResult, + options?: { excludeFromContext?: boolean; onPersisted?: () => void }, + ): void { const meta = outputMeta().truncationFromSummary(result, { direction: "tail" }).get(); const bashMessage: BashExecutionMessage = { role: "bashExecution", @@ -16162,13 +16168,14 @@ export class AgentSession { // If agent is streaming, defer adding to avoid breaking tool_use/tool_result ordering if (this.isStreaming) { // Queue for later - will be flushed on agent_end - this.#pendingBashMessages.push(bashMessage); + this.#pendingBashMessages.push({ message: bashMessage, onPersisted: options?.onPersisted }); } else { // Add to agent state immediately this.agent.appendMessage(bashMessage); // Save to session this.sessionManager.appendMessage(bashMessage); + options?.onPersisted?.(); } } @@ -16198,12 +16205,13 @@ export class AgentSession { #flushPendingBashMessages(): void { if (this.#pendingBashMessages.length === 0) return; - for (const bashMessage of this.#pendingBashMessages) { + for (const pending of this.#pendingBashMessages) { // Add to agent state - this.agent.appendMessage(bashMessage); + this.agent.appendMessage(pending.message); // Save to session - this.sessionManager.appendMessage(bashMessage); + this.sessionManager.appendMessage(pending.message); + pending.onPersisted?.(); } this.#pendingBashMessages = []; @@ -16219,11 +16227,13 @@ export class AgentSession { * @param code The Python code to execute * @param onChunk Optional streaming callback for output * @param options.excludeFromContext If true, execution won't be sent to LLM ($$ prefix) + * @param options.onPersisted Called once the execution's message is in session state + * (immediately when idle, at the post-turn flush while streaming) */ async executePython( code: string, onChunk?: (chunk: string) => void, - options?: { excludeFromContext?: boolean }, + options?: { excludeFromContext?: boolean; onPersisted?: () => void }, ): Promise { const excludeFromContext = options?.excludeFromContext === true; this.#markRetryReplayUnsafe(); @@ -16291,7 +16301,11 @@ export class AgentSession { /** * Record a Python execution result in session history. */ - recordPythonResult(code: string, result: PythonResult, options?: { excludeFromContext?: boolean }): void { + recordPythonResult( + code: string, + result: PythonResult, + options?: { excludeFromContext?: boolean; onPersisted?: () => void }, + ): void { const meta = outputMeta().truncationFromSummary(result, { direction: "tail" }).get(); const pythonMessage: PythonExecutionMessage = { role: "pythonExecution", @@ -16307,10 +16321,11 @@ export class AgentSession { // If agent is streaming, defer adding to avoid breaking tool_use/tool_result ordering if (this.isStreaming) { - this.#pendingPythonMessages.push(pythonMessage); + this.#pendingPythonMessages.push({ message: pythonMessage, onPersisted: options?.onPersisted }); } else { this.agent.appendMessage(pythonMessage); this.sessionManager.appendMessage(pythonMessage); + options?.onPersisted?.(); } } @@ -16371,9 +16386,10 @@ export class AgentSession { #flushPendingPythonMessages(): void { if (this.#pendingPythonMessages.length === 0) return; - for (const pythonMessage of this.#pendingPythonMessages) { - this.agent.appendMessage(pythonMessage); - this.sessionManager.appendMessage(pythonMessage); + for (const pending of this.#pendingPythonMessages) { + this.agent.appendMessage(pending.message); + this.sessionManager.appendMessage(pending.message); + pending.onPersisted?.(); } this.#pendingPythonMessages = []; diff --git a/packages/coding-agent/test/modes/controllers/bash-command.test.ts b/packages/coding-agent/test/modes/controllers/bash-command.test.ts index e1b9fb5160..8e7786f44c 100644 --- a/packages/coding-agent/test/modes/controllers/bash-command.test.ts +++ b/packages/coding-agent/test/modes/controllers/bash-command.test.ts @@ -468,3 +468,66 @@ describe("deferred shell command parentage after the transcript is cleared", () expect(harness.chatContainer.children).toEqual([parkedPython, parkedBash]); }); }); + +describe("deferred shell persistence rebuild race", () => { + it("does not duplicate a deferred bash block when persistence wins the controller race", async () => { + const harness = createHarness({ isStreaming: true }); + const persisted = Promise.withResolvers(); + const releaseReturn = Promise.withResolvers(); + + harness.ctx.session.executeBash = async (_command, _onChunk, options) => { + // AgentSession appends the message and reports it as persisted before + // executeBash returns, so the controller is still suspended when the + // rebuilt transcript already owns the row. + harness.rebuiltTranscriptRows.push("printf race"); + options?.onPersisted?.(); + persisted.resolve(); + await releaseReturn.promise; + return { + exitCode: 0, + cancelled: false, + output: "race", + truncated: false, + totalLines: 1, + totalBytes: 4, + outputLines: 1, + outputBytes: 4, + }; + }; + + const run = new CommandController(harness.ctx).handleBashCommand("printf race"); + await persisted.promise; + + new UiHelpers(harness.ctx).renderInitialMessages(); + expect(harness.chatContainer.children).toHaveLength(1); + + releaseReturn.resolve(); + await run; + + // The persisted transcript row is authoritative; completion must discard + // the superseded live component instead of appending a second copy. + expect(harness.chatContainer.children).toHaveLength(1); + }); + + it("keeps an unpersisted live block when the rebuild restores an older identical command", async () => { + const harness = createHarness({ isStreaming: true }); + // An older run of the same command is already in the session transcript. + harness.rebuiltTranscriptRows.push("printf same"); + + const run = new CommandController(harness.ctx).handleBashCommand("printf same"); + await settle(); + const liveComponent = harness.ctx.pendingBashComponents[0]; + + new UiHelpers(harness.ctx).renderInitialMessages(); + + // The live result is not persisted, so the identical restored row must not + // supersede it. + expect(harness.pendingMessagesContainer.children).toEqual([liveComponent]); + + harness.bashGate.resolve({ exitCode: 0, cancelled: false, output: "same", truncated: false }); + await run; + + expect(harness.chatContainer.children).toHaveLength(2); + expect(harness.chatContainer.children).toContain(liveComponent); + }); +});