From a2230b3dd0f515cc19ec78f6b45be969022fa305 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 26 Jun 2026 02:04:19 +0000 Subject: [PATCH 1/4] fix(tui): collapsed repeated todo snapshots Kept successful todo result blocks live until a later todo update replaces them or the turn ends. Added regression coverage for same-turn todo snapshot replacement after intervening tool output. Fixes #3516 --- packages/coding-agent/CHANGELOG.md | 4 + .../components/chat-transcript-builder.ts | 28 +++++++ .../src/modes/components/tool-execution.ts | 75 +++++++++++------ .../src/modes/controllers/event-controller.ts | 47 +++++++++-- .../src/modes/utils/ui-helpers.ts | 27 +++++++ .../test/job-poll-displacement.test.ts | 80 ++++++++++++++++++- 6 files changed, 228 insertions(+), 33 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index cacd922af..0b485f4b1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed repeated todo updates in one TUI turn stacking full todo panels; superseded todo snapshots now stay live until the next todo update replaces them or the turn ends. ([#3516](https://github.com/can1357/oh-my-pi/issues/3516)) + ## [16.1.20] - 2026-06-25 ### Fixed diff --git a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts index 690e8d610..a12a18d8d 100644 --- a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts +++ b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts @@ -83,6 +83,7 @@ export class ChatTranscriptBuilder { #pendingUsage: Usage | undefined; #lastAssistantUsage: Usage | undefined; #waitingPoll: ToolExecutionComponent | null = null; + #todoSnapshot: ToolExecutionComponent | null = null; #expandables: Array<{ setExpanded(expanded: boolean): void }> = []; #expanded = false; @@ -128,6 +129,7 @@ export class ChatTranscriptBuilder { this.#pendingUsage = undefined; this.#lastAssistantUsage = undefined; this.#waitingPoll = null; + this.#todoSnapshot = null; this.#expandables = []; this.container.dispose(); this.container.clear(); @@ -153,6 +155,24 @@ export class ChatTranscriptBuilder { previous.seal(); } + #resolveTodoSnapshot(nextToolName?: string): void { + const previous = this.#todoSnapshot; + if (!previous) return; + if (!previous.isDisplaceableBlock()) { + this.#todoSnapshot = null; + return; + } + if (previous.canBeDisplacedBy(nextToolName)) { + this.#todoSnapshot = null; + this.container.removeChild(previous); + previous.seal(); + return; + } + if (nextToolName !== undefined) return; + this.#todoSnapshot = null; + previous.seal(); + } + #ensureReadGroup(): ReadToolGroupComponent { if (!this.#readGroup) { this.#readGroup = new ReadToolGroupComponent({ @@ -189,6 +209,7 @@ export class ChatTranscriptBuilder { case "developer": { // A user prompt closes the poll-displacement window, same as the live path. if (message.role === "user") this.#resolveWaitingPoll(); + if (message.role === "user") this.#resolveTodoSnapshot(); const textContent = message.role === "user" ? userMessageText(message) : ""; if (textContent) { const isSynthetic = message.role === "developer" ? true : (message.synthetic ?? false); @@ -269,6 +290,7 @@ export class ChatTranscriptBuilder { for (const content of message.content) { if (content.type !== "toolCall") continue; this.#resolveWaitingPoll(content.name); + this.#resolveTodoSnapshot(content.name); if ( content.name === "read" && @@ -345,6 +367,12 @@ export class ChatTranscriptBuilder { this.#pendingTools.delete(message.toolCallId); if (message.toolName === "job" && pending instanceof ToolExecutionComponent && pending.isDisplaceableBlock()) { this.#waitingPoll = pending; + } else if ( + message.toolName === "todo" && + pending instanceof ToolExecutionComponent && + pending.canBeDisplacedBy("todo") + ) { + this.#todoSnapshot = pending; } } diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index a5d1937f8..ed180d740 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -39,7 +39,7 @@ import { truncateToWidth, } from "../../tools/render-utils"; import { toolRenderers } from "../../tools/renderers"; -import { TODO_STRIKE_TOTAL_FRAMES } from "../../tools/todo"; +import { TODO_STRIKE_TOTAL_FRAMES, type TodoToolDetails } from "../../tools/todo"; import { isFramedBlockComponent, renderStatusLine, WidthAwareText } from "../../tui"; import { sanitizeWithOptionalSixelPassthrough } from "../../utils/sixel"; import { renderDiff } from "./diff"; @@ -75,6 +75,28 @@ function stripTrailingUnbalancedRemoval(diff: string | undefined): string | unde return lines.slice(0, lastAddIdx + 1).join("\n"); } +type DisplaceableToolName = "job" | "todo"; + +function isTodoToolDetails(details: unknown): details is TodoToolDetails { + return ( + typeof details === "object" && + details !== null && + "phases" in details && + Array.isArray((details as { phases?: unknown }).phases) + ); +} + +function displaceableToolName( + toolName: string, + result: { details?: unknown; isError?: boolean }, + isPartial: boolean, +): DisplaceableToolName | undefined { + if (result.isError === true) return undefined; + if (toolName === "job" && isWaitingPollDetails(result.details)) return "job"; + if (toolName === "todo" && !isPartial && isTodoToolDetails(result.details)) return "todo"; + return undefined; +} + function stabilizeStreamingPreviews(previews: PerFileDiffPreview[]): PerFileDiffPreview[] { let changed = false; const next = previews.map(preview => { @@ -234,11 +256,11 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac // sealed the block stays in the transcript's repaintable live region so a // late result still repaints instead of stranding the streaming preview. #sealed = false; - // A `job` poll result whose watched jobs are all still running. Such a - // block never finalizes (stays in the transcript live region) so a - // follow-up `job` call can displace it instead of stacking another - // "waiting on N jobs" frame. Cleared by `seal()`. - #displaceable = false; + // Tool result snapshots that may be superseded by a later same-tool call + // while still in the transcript live region. `job` uses this for repeated + // all-running polls; `todo` uses it for per-turn state snapshots so only the + // latest list remains visible. + #displaceableByToolName: DisplaceableToolName | undefined; // Probe into the owning transcript (absent outside the interactive // transcript, e.g. in tests): whether this block is still repaintable. #liveRegion?: TranscriptLiveRegionProbe; @@ -427,11 +449,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac this.#result = result; this.#resultVersion++; this.#isPartial = isPartial; - // A `job` poll that found every watched job still running is transient - // "still waiting" chrome; keep the block displaceable so the next `job` - // call replaces it instead of stacking another waiting frame (see the - // event controller's displaceable-poll bookkeeping). - this.#displaceable = this.#toolName === "job" && result.isError !== true && isWaitingPollDetails(result.details); + this.#displaceableByToolName = displaceableToolName(this.#toolName, result, isPartial); // When tool is complete, ensure args are marked complete so spinner stops if (!isPartial) { this.#argsComplete = true; @@ -490,10 +508,12 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac } /** - * Start or stop spinner animation based on whether this is a partial task result. + * Start or stop spinner animation for result states that visibly tick. */ #updateSpinnerAnimation(): void { - // Spinner for: task tool with partial result, or edit/write while args streaming + // Spinner for: task tool with partial result, edit/write while args + // streaming, or a still-running job poll. Todo snapshots stay live for + // displacement but should remain visually static. const isStreamingArgs = !this.#argsComplete && (isEditLikeToolName(this.#toolName) || this.#toolName === "write"); const isBackgroundAsyncRunning = (this.#result?.details as { async?: { state?: string } } | undefined)?.async?.state === "running"; @@ -502,7 +522,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac // Detached async task progress rows are static now; progress snapshots // still call #maybeFreezeBackgroundTask before applying so rows settle // once the block leaves the live region. - const needsSpinner = isStreamingArgs || isPartialTask || this.isDisplaceableBlock(); + const needsSpinner = isStreamingArgs || isPartialTask || this.#displaceableByToolName === "job"; if (needsSpinner && !this.#spinnerInterval) { const frameCount = theme.spinnerFrames.length; const frame = sharedSpinnerFrame(frameCount); @@ -610,9 +630,9 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac isTranscriptBlockFinalized(): boolean { if (this.#sealed) return true; if (this.#result === undefined) return false; - // A displaceable waiting poll stays live: its rows are kept out of - // native scrollback so a follow-up `job` call can remove the block. - if (this.#displaceable) return false; + // A displaceable snapshot stays live: its rows are kept out of native + // scrollback so a follow-up tool call can remove the block. + if (this.#displaceableByToolName) return false; if (!this.#isPartial) return true; // Partial result: a background async tool is accepted to freeze (the agent // continues while it runs and would otherwise pin an unbounded live region); @@ -630,7 +650,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac * committed prefix survives the remaining transitions. */ isTranscriptBlockCommitStable(): boolean { - if (this.#displaceable) return false; + if (this.#displaceableByToolName) return false; if (this.isTranscriptBlockFinalized()) return true; // `provisionalPendingPreview` describes only the PENDING call preview // (`renderCall`, before any result): the result render may re-anchor it @@ -658,7 +678,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac seal(): void { if (this.#sealed) return; this.#sealed = true; - this.#displaceable = false; + this.#displaceableByToolName = undefined; // A sealed detached task is abandoned history: settle its progress rows // on static gray. this.#backgroundTaskFrozen = true; @@ -668,14 +688,19 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac } /** - * Whether this block is a waiting `job` poll (every watched job still - * running) that has not been sealed. Such a block never finalized, so none - * of its rows entered native scrollback (the ticking spinner keeps the - * stable-prefix ratchet at zero) and the whole block can be removed when a - * follow-up `job` call supersedes it. + * Whether this block is a supersedable result snapshot that has not been + * sealed. Such a block never finalized, so none of its rows entered native + * scrollback and the whole block can be removed when a follow-up matching + * tool call supersedes it. */ isDisplaceableBlock(): boolean { - return this.#displaceable && !this.#sealed; + return this.#displaceableByToolName !== undefined && !this.#sealed; + } + + canBeDisplacedBy(nextToolName: string | undefined): boolean { + return ( + this.#displaceableByToolName !== undefined && this.#displaceableByToolName === nextToolName && !this.#sealed + ); } /** diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index f13186055..53d982110 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -77,6 +77,10 @@ export class EventController { // one persistent poll instead of a stack of "waiting on N jobs" frames — // and sealed in place the moment anything else lands below it. #displaceablePollComponent: ToolExecutionComponent | undefined = undefined; + // Most recent successful `todo` snapshot in the active turn. It stays live + // across intervening tool output so a later `todo` update can replace the + // old full list; the turn boundary seals the final snapshot as history. + #displaceableTodoComponent: ToolExecutionComponent | undefined = undefined; // Most recent TTSR notification block. A new ttsr_triggered event merges its // rules into this block while it is still the (live-region) transcript tail. #lastTtsrNotification: TtsrNotificationComponent | undefined = undefined; @@ -226,6 +230,7 @@ export class EventController { this.#ircExpiryTimers.clear(); this.#liveIrcCards.clear(); this.#displaceablePollComponent = undefined; + this.#displaceableTodoComponent = undefined; this.#lastTtsrNotification = undefined; this.#streamingReveal.stop(); this.#toolArgsReveal.stop(); @@ -248,6 +253,7 @@ export class EventController { this.#readToolCallArgs.clear(); this.#readToolCallAssistantComponents.clear(); this.#resetReadGroup(); + this.#resolveDisplaceableTodo(); this.#lastAssistantComponent = undefined; // Restore the previous turn's inline error in the transcript before dropping // the banner, so the error stays in history once the banner is gone. @@ -296,6 +302,7 @@ export class EventController { this.#resetReadGroup(); this.#resolveDisplaceablePoll(); + this.#resolveDisplaceableTodo(); const wasOptimistic = this.ctx.optimisticUserMessageSignature === signature; const matchedLocalSubmission = this.ctx.locallySubmittedUserSignatures.delete(signature); const replacesOptimistic = @@ -423,6 +430,26 @@ export class EventController { this.ctx.ui.requestRender(); } + #resolveDisplaceableTodo(nextToolName?: string): void { + const previous = this.#displaceableTodoComponent; + if (!previous) return; + if (!previous.isDisplaceableBlock()) { + this.#displaceableTodoComponent = undefined; + return; + } + if (previous.canBeDisplacedBy(nextToolName)) { + this.#displaceableTodoComponent = undefined; + this.ctx.chatContainer.removeChild(previous); + previous.seal(); + this.ctx.ui.requestRender(); + return; + } + if (nextToolName !== undefined) return; + this.#displaceableTodoComponent = undefined; + previous.seal(); + this.ctx.ui.requestRender(); + } + async #handleNotice(event: Extract): Promise { const message = event.source ? `${event.source}: ${event.message}` : event.message; if (event.level === "error") { @@ -512,6 +539,7 @@ export class EventController { } if (!readArgsTargetInternalUrl(content.arguments)) { if (!this.ctx.pendingTools.has(content.id)) this.#resolveDisplaceablePoll(content.name); + this.#resolveDisplaceableTodo(content.name); this.#trackReadToolCall(content.id, content.arguments); const component = this.ctx.pendingTools.get(content.id); if (component) { @@ -547,6 +575,7 @@ export class EventController { } if (!this.ctx.pendingTools.has(content.id)) { this.#resolveDisplaceablePoll(content.name); + this.#resolveDisplaceableTodo(content.name); this.#resetReadGroup(); const tool = this.ctx.viewSession.getToolByName(content.name); const component = new ToolExecutionComponent( @@ -701,6 +730,7 @@ export class EventController { this.#updateWorkingMessageFromIntent(event.intent); if (!this.ctx.pendingTools.has(event.toolCallId)) { this.#resolveDisplaceablePoll(event.toolName); + this.#resolveDisplaceableTodo(event.toolName); if (event.toolName === "read" && readArgsHaveTarget(event.args) && !readArgsTargetInternalUrl(event.args)) { this.#trackReadToolCall(event.toolCallId, event.args); const component = this.ctx.pendingTools.get(event.toolCallId); @@ -830,13 +860,15 @@ export class EventController { this.ctx.pendingTools.delete(event.toolCallId); this.#backgroundToolCallIds.delete(event.toolCallId); } - if ( - event.toolName === "job" && - component instanceof ToolExecutionComponent && - component.isDisplaceableBlock() - ) { - // Remember the waiting poll so the next `job` call can displace it. - this.#displaceablePollComponent = component; + if (component instanceof ToolExecutionComponent && component.isDisplaceableBlock()) { + if (event.toolName === "job" && component.canBeDisplacedBy("job")) { + // Remember the waiting poll so the next `job` call can displace it. + this.#displaceablePollComponent = component; + } else if (event.toolName === "todo" && component.canBeDisplacedBy("todo")) { + // Keep the latest todo snapshot live until another todo update + // replaces it or the turn ends. + this.#displaceableTodoComponent = component; + } } this.ctx.ui.requestRender(); } @@ -918,6 +950,7 @@ export class EventController { // The turn is over: nothing else lands this turn, so the waiting poll is // final history — seal it instead of letting its spinner tick while idle. this.#resolveDisplaceablePoll(); + this.#resolveDisplaceableTodo(); this.#lastAssistantComponent = undefined; this.ctx.ui.requestRender(); this.#scheduleIdleCompaction(); diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 0b5baf8dd..b7e6b1a8d 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -317,6 +317,24 @@ export class UiHelpers { // updateResult armed. previous.seal(); }; + let todoSnapshot: ToolExecutionComponent | null = null; + const resolveTodoSnapshot = (nextToolName?: string) => { + const previous = todoSnapshot; + if (!previous) return; + if (!previous.isDisplaceableBlock()) { + todoSnapshot = null; + return; + } + if (previous.canBeDisplacedBy(nextToolName)) { + todoSnapshot = null; + this.ctx.chatContainer.removeChild(previous); + previous.seal(); + return; + } + if (nextToolName !== undefined) return; + todoSnapshot = null; + previous.seal(); + }; const messages = sessionContext.messages; const count = messages.length; for (let i = 0; i < count; i++) { @@ -358,6 +376,7 @@ export class UiHelpers { continue; } resolveWaitingPoll(content.name); + resolveTodoSnapshot(content.name); if ( content.name === "read" && @@ -477,11 +496,18 @@ export class UiHelpers { component.isDisplaceableBlock() ) { waitingPoll = component; + } else if ( + message.toolName === "todo" && + component instanceof ToolExecutionComponent && + component.canBeDisplacedBy("todo") + ) { + todoSnapshot = component; } } } else { // A user prompt closes the displacement window, same as the live path. if (message.role === "user") resolveWaitingPoll(); + if (message.role === "user") resolveTodoSnapshot(); // All other messages use standard rendering this.ctx.addMessageToChat(message, options); } @@ -495,6 +521,7 @@ export class UiHelpers { // A trailing waiting poll is final history on rebuild; seal it so it // freezes (and its spinner timer stops) like every other block. resolveWaitingPoll(); + resolveTodoSnapshot(); this.ctx.pendingTools.clear(); this.ctx.ui.requestRender(); diff --git a/packages/coding-agent/test/job-poll-displacement.test.ts b/packages/coding-agent/test/job-poll-displacement.test.ts index a9ed774b5..160c534b1 100644 --- a/packages/coding-agent/test/job-poll-displacement.test.ts +++ b/packages/coding-agent/test/job-poll-displacement.test.ts @@ -39,6 +39,24 @@ function pollResult(statuses: JobStatus[], extra: { cancelled?: boolean; isError }; } +function todoResult(items = ["investigate", "fix"]) { + return { + content: [{ type: "text" as const, text: "" }], + details: { + phases: [ + { + name: "Workflow", + tasks: items.map((content, index) => ({ + content, + status: index === 0 ? ("in_progress" as const) : ("pending" as const), + })), + }, + ], + storage: "memory" as const, + }, + }; +} + function trackComponent(components: ToolExecutionComponent[], component: ToolExecutionComponent) { components.push(component); return component; @@ -96,7 +114,22 @@ describe("job waiting-poll block lifecycle", () => { expect(errored.isTranscriptBlockFinalized()).toBe(true); }); - it("never marks non-job tools displaceable", () => { + it("keeps successful todo snapshots live for replacement", () => { + const component = trackComponent( + created, + new ToolExecutionComponent("todo", { op: "view" }, {}, undefined, uiStub), + ); + component.updateResult(todoResult(), false); + + expect(component.isDisplaceableBlock()).toBe(true); + expect(component.isTranscriptBlockFinalized()).toBe(false); + + component.seal(); + expect(component.isDisplaceableBlock()).toBe(false); + expect(component.isTranscriptBlockFinalized()).toBe(true); + }); + + it("never marks ordinary non-refresh tools displaceable", () => { const component = trackComponent( created, new ToolExecutionComponent("bash", { command: "ls" }, {}, undefined, uiStub), @@ -144,6 +177,7 @@ describe("EventController displaces consecutive waiting polls", () => { session: { getToolByName: () => undefined }, viewSession: { getToolByName: () => undefined }, sessionManager: { getCwd: () => process.cwd() }, + setTodos: vi.fn(), } as unknown as InteractiveModeContext; return { controller: new EventController(ctx), children }; } @@ -167,6 +201,25 @@ describe("EventController displaces consecutive waiting polls", () => { return component; } + async function runTodo(controller: EventController, children: Component[], toolCallId: string, items?: string[]) { + await controller.handleEvent({ + type: "tool_execution_start", + toolCallId, + toolName: "todo", + args: { op: "view" }, + }); + const component = children[children.length - 1] as ToolExecutionComponent; + trackComponent(created, component); + await controller.handleEvent({ + type: "tool_execution_end", + toolCallId, + toolName: "todo", + result: todoResult(items), + isError: false, + }); + return component; + } + it("removes the previous waiting poll when the next job call starts", async () => { const { controller, children } = createFixture(); @@ -202,6 +255,31 @@ describe("EventController displaces consecutive waiting polls", () => { expect(poll.isDisplaceableBlock()).toBe(false); }); + it("removes the previous todo snapshot when a later todo update lands in the same turn", async () => { + const { controller, children } = createFixture(); + + const first = await runTodo(controller, children, "todo-1", ["plan", "read"]); + expect(children).toContain(first); + expect(first.isTranscriptBlockFinalized()).toBe(false); + + await controller.handleEvent({ + type: "tool_execution_start", + toolCallId: "bash-1", + toolName: "bash", + args: { command: "true" }, + }); + const bash = trackComponent(created, children[children.length - 1] as ToolExecutionComponent); + expect(children).toContain(first); + expect(children).toContain(bash); + + const second = await runTodo(controller, children, "todo-2", ["fix", "test"]); + + expect(children).not.toContain(first); + expect(children).toContain(bash); + expect(children).toContain(second); + expect(first.isTranscriptBlockFinalized()).toBe(true); + }); + it("does not displace a poll that observed completions", async () => { const { controller, children } = createFixture(); From 58373fdee9ad5c55768c7f6abb94c83eb08628ff Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 26 Jun 2026 02:12:43 +0000 Subject: [PATCH 2/4] fix(tui): handled streamed todo replacement Resolved existing todo components before tool execution so streamed tool-call previews can still displace stale todo snapshots. Added regression coverage for pre-created todo calls replacing a prior todo panel after intervening tool output. Fixes #3516 --- .../src/modes/controllers/event-controller.ts | 4 +- .../test/job-poll-displacement.test.ts | 53 ++++++++++++++++++- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 53d982110..42309abe9 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -728,9 +728,9 @@ export class EventController { async #handleToolExecutionStart(event: Extract): Promise { this.#updateWorkingMessageFromIntent(event.intent); + this.#resolveDisplaceablePoll(event.toolName); + this.#resolveDisplaceableTodo(event.toolName); if (!this.ctx.pendingTools.has(event.toolCallId)) { - this.#resolveDisplaceablePoll(event.toolName); - this.#resolveDisplaceableTodo(event.toolName); if (event.toolName === "read" && readArgsHaveTarget(event.args) && !readArgsTargetInternalUrl(event.args)) { this.#trackReadToolCall(event.toolCallId, event.args); const component = this.ctx.pendingTools.get(event.toolCallId); diff --git a/packages/coding-agent/test/job-poll-displacement.test.ts b/packages/coding-agent/test/job-poll-displacement.test.ts index 160c534b1..a7efff192 100644 --- a/packages/coding-agent/test/job-poll-displacement.test.ts +++ b/packages/coding-agent/test/job-poll-displacement.test.ts @@ -156,6 +156,7 @@ describe("EventController displaces consecutive waiting polls", () => { function createFixture() { const children: Component[] = []; + const pendingTools = new Map(); const ctx = { isInitialized: true, init: vi.fn(async () => {}), @@ -163,7 +164,7 @@ describe("EventController displaces consecutive waiting polls", () => { statusLine: { invalidate: vi.fn() }, updateEditorTopBorder: vi.fn(), toolOutputExpanded: false, - pendingTools: new Map(), + pendingTools, chatContainer: { children, addChild: (component: Component) => { @@ -179,7 +180,7 @@ describe("EventController displaces consecutive waiting polls", () => { sessionManager: { getCwd: () => process.cwd() }, setTodos: vi.fn(), } as unknown as InteractiveModeContext; - return { controller: new EventController(ctx), children }; + return { controller: new EventController(ctx), children, pendingTools }; } async function runPoll(controller: EventController, children: Component[], toolCallId: string) { @@ -280,6 +281,54 @@ describe("EventController displaces consecutive waiting polls", () => { expect(first.isTranscriptBlockFinalized()).toBe(true); }); + it("removes a todo snapshot when the next todo call was pre-created while args streamed", async () => { + const { controller, children, pendingTools } = createFixture(); + + await controller.handleEvent({ + type: "tool_execution_start", + toolCallId: "todo-1", + toolName: "todo", + args: { op: "view" }, + }); + const first = trackComponent(created, children[children.length - 1] as ToolExecutionComponent); + + const second = trackComponent(created, new ToolExecutionComponent("todo", { op: "view" }, {}, undefined, uiStub)); + children.push(second); + pendingTools.set("todo-2", second); + + await controller.handleEvent({ + type: "tool_execution_end", + toolCallId: "todo-1", + toolName: "todo", + result: todoResult(["plan", "read"]), + isError: false, + }); + expect(first.isTranscriptBlockFinalized()).toBe(false); + + await controller.handleEvent({ + type: "tool_execution_start", + toolCallId: "bash-1", + toolName: "bash", + args: { command: "true" }, + }); + const bash = trackComponent(created, children[children.length - 1] as ToolExecutionComponent); + expect(children).toContain(first); + expect(children).toContain(second); + expect(children).toContain(bash); + + await controller.handleEvent({ + type: "tool_execution_start", + toolCallId: "todo-2", + toolName: "todo", + args: { op: "view" }, + }); + + expect(children).not.toContain(first); + expect(children).toContain(second); + expect(children).toContain(bash); + expect(first.isTranscriptBlockFinalized()).toBe(true); + }); + it("does not displace a poll that observed completions", async () => { const { controller, children } = createFixture(); From ba3a6079be62a9108c08984be50997912dea6c83 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 26 Jun 2026 02:17:12 +0000 Subject: [PATCH 3/4] fix(tui): collapsed multi-todo rebuild snapshots Resolved any tracked todo snapshot before storing a fresh one in the rebuild paths so an assistant message replaying multiple todo tool calls collapses to the final snapshot. Added a renderSessionContext regression test for two todo tool calls in one rebuilt assistant message. Fixes #3516 --- .../components/chat-transcript-builder.ts | 4 + .../src/modes/utils/ui-helpers.ts | 4 + .../test/job-poll-displacement.test.ts | 94 ++++++++++++++++++- 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts index a12a18d8d..cc6d4b753 100644 --- a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts +++ b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts @@ -372,6 +372,10 @@ export class ChatTranscriptBuilder { pending instanceof ToolExecutionComponent && pending.canBeDisplacedBy("todo") ) { + // Multiple `todo` results in one rebuilt assistant message land here + // without an intervening assistant iteration that could displace + // them; collapse the prior snapshot before tracking the new one. + this.#resolveTodoSnapshot("todo"); this.#todoSnapshot = pending; } } diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index b7e6b1a8d..2c6489dd7 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -501,6 +501,10 @@ export class UiHelpers { component instanceof ToolExecutionComponent && component.canBeDisplacedBy("todo") ) { + // Multiple `todo` results in one rebuilt assistant message land here + // without an intervening assistant iteration that could displace + // them; collapse the prior snapshot before tracking the new one. + resolveTodoSnapshot("todo"); todoSnapshot = component; } } diff --git a/packages/coding-agent/test/job-poll-displacement.test.ts b/packages/coding-agent/test/job-poll-displacement.test.ts index a7efff192..c86adfd80 100644 --- a/packages/coding-agent/test/job-poll-displacement.test.ts +++ b/packages/coding-agent/test/job-poll-displacement.test.ts @@ -10,13 +10,16 @@ * - EventController: a follow-up `job` call removes the tracked waiting * poll from the transcript; any other tool seals it in place. */ -import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; +import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; import { EventController } from "@oh-my-pi/pi-coding-agent/modes/controllers/event-controller"; import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; -import type { Component, TUI } from "@oh-my-pi/pi-tui"; +import { UiHelpers } from "@oh-my-pi/pi-coding-agent/modes/utils/ui-helpers"; +import type { SessionContext } from "@oh-my-pi/pi-coding-agent/session/session-context"; +import { type Component, Container, type TUI } from "@oh-my-pi/pi-tui"; const uiStub = { requestRender() {} } as unknown as TUI; @@ -354,3 +357,90 @@ describe("EventController displaces consecutive waiting polls", () => { expect(children).toContain(next); }); }); + +describe("UiHelpers.renderSessionContext collapses repeated todo snapshots", () => { + beforeAll(async () => { + resetSettingsForTest(); + await Settings.init({ inMemory: true }); + await initTheme(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("removes the earlier todo snapshot when an assistant message replays two todo calls", () => { + const chatContainer = new Container(); + let helpers!: UiHelpers; + const ctx = { + chatContainer, + pendingTools: new Map(), + ui: { requestRender: vi.fn() }, + statusLine: { invalidate: vi.fn() }, + updateEditorBorderColor: vi.fn(), + settings: { get: () => false }, + addMessageToChat: (message: AgentMessage) => helpers.addMessageToChat(message), + session: { + retryAttempt: 0, + getToolByName: () => undefined, + sessionManager: { getCwd: () => process.cwd() }, + }, + get viewSession() { + return (this as { session: unknown }).session; + }, + toolOutputExpanded: false, + hideThinkingBlock: false, + lastAssistantUsage: undefined, + clearTransientSessionUi: () => {}, + } as unknown as InteractiveModeContext; + helpers = new UiHelpers(ctx); + + const usage = { + input: 1, + output: 1, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 2, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }; + const assistant = { + role: "assistant", + content: [ + { type: "toolCall", id: "todo-1", name: "todo", arguments: { op: "view" } }, + { type: "toolCall", id: "todo-2", name: "todo", arguments: { op: "view" } }, + ], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-sonnet-4-5", + stopReason: "stop", + usage, + timestamp: Date.now(), + } as unknown as AgentMessage; + const firstResult = { + role: "toolResult", + toolCallId: "todo-1", + toolName: "todo", + content: [{ type: "text", text: "" }], + details: todoResult(["plan", "read"]).details, + timestamp: Date.now(), + } as unknown as AgentMessage; + const secondResult = { + role: "toolResult", + toolCallId: "todo-2", + toolName: "todo", + content: [{ type: "text", text: "" }], + details: todoResult(["fix", "test"]).details, + timestamp: Date.now(), + } as unknown as AgentMessage; + + helpers.renderSessionContext({ messages: [assistant, firstResult, secondResult] } as SessionContext); + + const todos = chatContainer.children.filter( + (child): child is ToolExecutionComponent => child instanceof ToolExecutionComponent, + ); + // Only the latest todo snapshot survives the rebuild; the trailing one is + // sealed as final history by the end-of-rebuild flush. + expect(todos).toHaveLength(1); + expect(todos[0].isTranscriptBlockFinalized()).toBe(true); + }); +}); From bd6710bd023589e8a24c9c1a3ee3a469eff2f4a4 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 26 Jun 2026 02:21:37 +0000 Subject: [PATCH 4/4] fix(tui): deferred todo displacement to successful result Dropped eager todo snapshot displacement from tool_execution_start, streaming message_update, and the rebuild assistant-iteration step. Displacement now runs only when the next todo's successful result lands, so a failed follow-up leaves the last-good todo panel on screen. Added regression coverage for the failed follow-up case and updated the streamed-second-todo test to drive displacement from the success result. Fixes #3516 --- .../components/chat-transcript-builder.ts | 7 ++- .../src/modes/controllers/event-controller.ts | 14 +++--- .../src/modes/utils/ui-helpers.ts | 7 ++- .../test/job-poll-displacement.test.ts | 44 ++++++++++++++++++- 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts index cc6d4b753..c460ee463 100644 --- a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts +++ b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts @@ -290,7 +290,6 @@ export class ChatTranscriptBuilder { for (const content of message.content) { if (content.type !== "toolCall") continue; this.#resolveWaitingPoll(content.name); - this.#resolveTodoSnapshot(content.name); if ( content.name === "read" && @@ -372,9 +371,9 @@ export class ChatTranscriptBuilder { pending instanceof ToolExecutionComponent && pending.canBeDisplacedBy("todo") ) { - // Multiple `todo` results in one rebuilt assistant message land here - // without an intervening assistant iteration that could displace - // them; collapse the prior snapshot before tracking the new one. + // A successful todo result supersedes the prior live snapshot. Failed + // follow-ups return false from canBeDisplacedBy("todo"), so the + // last-good panel stays on screen. this.#resolveTodoSnapshot("todo"); this.#todoSnapshot = pending; } diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 42309abe9..5f656a241 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -539,7 +539,6 @@ export class EventController { } if (!readArgsTargetInternalUrl(content.arguments)) { if (!this.ctx.pendingTools.has(content.id)) this.#resolveDisplaceablePoll(content.name); - this.#resolveDisplaceableTodo(content.name); this.#trackReadToolCall(content.id, content.arguments); const component = this.ctx.pendingTools.get(content.id); if (component) { @@ -575,7 +574,6 @@ export class EventController { } if (!this.ctx.pendingTools.has(content.id)) { this.#resolveDisplaceablePoll(content.name); - this.#resolveDisplaceableTodo(content.name); this.#resetReadGroup(); const tool = this.ctx.viewSession.getToolByName(content.name); const component = new ToolExecutionComponent( @@ -729,7 +727,6 @@ export class EventController { async #handleToolExecutionStart(event: Extract): Promise { this.#updateWorkingMessageFromIntent(event.intent); this.#resolveDisplaceablePoll(event.toolName); - this.#resolveDisplaceableTodo(event.toolName); if (!this.ctx.pendingTools.has(event.toolCallId)) { if (event.toolName === "read" && readArgsHaveTarget(event.args) && !readArgsTargetInternalUrl(event.args)) { this.#trackReadToolCall(event.toolCallId, event.args); @@ -865,8 +862,15 @@ export class EventController { // Remember the waiting poll so the next `job` call can displace it. this.#displaceablePollComponent = component; } else if (event.toolName === "todo" && component.canBeDisplacedBy("todo")) { - // Keep the latest todo snapshot live until another todo update - // replaces it or the turn ends. + // Successful todo update supersedes the prior live snapshot. A failed + // follow-up never reaches this branch (canBeDisplacedBy("todo") returns + // false for errored results), so the last-good panel stays on screen. + const previous = this.#displaceableTodoComponent; + if (previous && previous !== component && previous.isDisplaceableBlock()) { + this.#displaceableTodoComponent = undefined; + this.ctx.chatContainer.removeChild(previous); + previous.seal(); + } this.#displaceableTodoComponent = component; } } diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 2c6489dd7..dd3c15893 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -376,7 +376,6 @@ export class UiHelpers { continue; } resolveWaitingPoll(content.name); - resolveTodoSnapshot(content.name); if ( content.name === "read" && @@ -501,9 +500,9 @@ export class UiHelpers { component instanceof ToolExecutionComponent && component.canBeDisplacedBy("todo") ) { - // Multiple `todo` results in one rebuilt assistant message land here - // without an intervening assistant iteration that could displace - // them; collapse the prior snapshot before tracking the new one. + // A successful todo result supersedes the prior live snapshot. Failed + // follow-ups return false from canBeDisplacedBy("todo"), so the + // last-good panel stays on screen. resolveTodoSnapshot("todo"); todoSnapshot = component; } diff --git a/packages/coding-agent/test/job-poll-displacement.test.ts b/packages/coding-agent/test/job-poll-displacement.test.ts index c86adfd80..f2435852e 100644 --- a/packages/coding-agent/test/job-poll-displacement.test.ts +++ b/packages/coding-agent/test/job-poll-displacement.test.ts @@ -179,6 +179,7 @@ describe("EventController displaces consecutive waiting polls", () => { }, }, session: { getToolByName: () => undefined }, + showWarning: vi.fn(), viewSession: { getToolByName: () => undefined }, sessionManager: { getCwd: () => process.cwd() }, setTodos: vi.fn(), @@ -284,7 +285,7 @@ describe("EventController displaces consecutive waiting polls", () => { expect(first.isTranscriptBlockFinalized()).toBe(true); }); - it("removes a todo snapshot when the next todo call was pre-created while args streamed", async () => { + it("displaces a prior todo snapshot when a streamed second todo lands a successful result", async () => { const { controller, children, pendingTools } = createFixture(); await controller.handleEvent({ @@ -326,12 +327,53 @@ describe("EventController displaces consecutive waiting polls", () => { args: { op: "view" }, }); + // Start alone is no longer enough — the prior snapshot stays so a failed + // follow-up cannot strand the user without a current todo panel. + expect(children).toContain(first); + expect(first.isTranscriptBlockFinalized()).toBe(false); + + await controller.handleEvent({ + type: "tool_execution_end", + toolCallId: "todo-2", + toolName: "todo", + result: todoResult(["fix", "test"]), + isError: false, + }); + expect(children).not.toContain(first); expect(children).toContain(second); expect(children).toContain(bash); expect(first.isTranscriptBlockFinalized()).toBe(true); }); + it("keeps the prior todo snapshot when a follow-up todo errors", async () => { + const { controller, children } = createFixture(); + + const first = await runTodo(controller, children, "todo-1", ["plan", "read"]); + expect(children).toContain(first); + expect(first.isTranscriptBlockFinalized()).toBe(false); + + await controller.handleEvent({ + type: "tool_execution_start", + toolCallId: "todo-2", + toolName: "todo", + args: { op: "view" }, + }); + const errored = trackComponent(created, children[children.length - 1] as ToolExecutionComponent); + + await controller.handleEvent({ + type: "tool_execution_end", + toolCallId: "todo-2", + toolName: "todo", + result: { content: [{ type: "text", text: "Phase missing" }], details: undefined, isError: true }, + isError: true, + }); + + expect(children).toContain(first); + expect(children).toContain(errored); + expect(first.isTranscriptBlockFinalized()).toBe(false); + }); + it("does not displace a poll that observed completions", async () => { const { controller, children } = createFixture();