From 984c8dd2f6be35c44e2dcadfb7db6745ae97691d Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 21 Jun 2026 06:51:37 +0200 Subject: [PATCH] fix(coding-agent): reconciled tool arguments on execution start - Synchronize tool arguments with the component state upon receipt of `tool_execution_start` to ensure visual consistency when final update events are missed. - Terminate active argument reveal streams to prevent late ticks from overwriting valid, fully-materialized tool arguments with stale partial data. - Add test coverage to verify that tool UI components render finalized arguments even in the absence of intermediate streaming updates. --- packages/coding-agent/src/edit/streaming.ts | 10 ++-- .../src/modes/controllers/event-controller.ts | 20 ++++++++ packages/coding-agent/src/tools/bash.ts | 6 +-- .../event-controller-args-reveal.test.ts | 46 +++++++++++++++++++ 4 files changed, 74 insertions(+), 8 deletions(-) diff --git a/packages/coding-agent/src/edit/streaming.ts b/packages/coding-agent/src/edit/streaming.ts index 718d164db..e3a54a5d4 100644 --- a/packages/coding-agent/src/edit/streaming.ts +++ b/packages/coding-agent/src/edit/streaming.ts @@ -85,12 +85,12 @@ export interface EditStreamingStrategy { /** * Given an edits array parsed from partial JSON, drop the last entry when the - * corresponding object in `partialJson` does not yet end with a closed `}`. + * corresponding object in `partialJson` has not yet closed with `}`. * - * This guards against `partial-json` silently coercing truncated tails like - * `"write":nu` / `"write":nul` into `{ write: null }`, which would make the - * last entry render a spurious null-write error until the value finishes - * streaming. + * The streaming parser materializes a trailing edit object from the fields seen + * so far before its closing `}` arrives, so an unfinished last entry can render + * as a (partial) edit mid-stream. Dropping it until the object closes keeps the + * preview from showing an incomplete edit. */ export function dropIncompleteLastEdit(edits: readonly T[], partialJson: string | undefined, listKey: string): T[] { if (!Array.isArray(edits) || edits.length === 0) return [...(edits ?? [])]; diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index e1ced652e..99eb94dbc 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -738,6 +738,26 @@ export class EventController { this.ctx.chatContainer.addChild(component); this.ctx.pendingTools.set(event.toolCallId, component); this.ctx.ui.requestRender(); + } else { + // The tool is about to run, so its arguments are final and validated. + // A pending component created while args streamed (message_update) may + // still show a mid-reveal prefix — or, when the closing full-args + // `message_update` never lands (smooth-streaming off leaving the + // throttled `arguments` stale, an owned-dialect projector, or a + // superseded/aborted turn that still executes the call), a stale body + // the result render then freezes at its `…` placeholder. Reconcile the + // authoritative args here and drop any live reveal so a late tick can't + // re-truncate them: tool_execution_start is the one event every + // execution path emits with the full args immediately before the result. + this.#toolArgsReveal.finish(event.toolCallId); + const component = this.ctx.pendingTools.get(event.toolCallId); + if (component && typeof component.updateArgs === "function") { + component.updateArgs(event.args, event.toolCallId); + if (typeof component.setArgsComplete === "function") { + component.setArgsComplete(event.toolCallId); + } + this.ctx.ui.requestRender(); + } } } diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 141707f3a..5605de27e 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -1132,9 +1132,9 @@ function getPartialJson(args: TArgs | undefined): string | undefined { } export function getBashEnvForDisplay(args: BashRenderArgs): Record | undefined { - // During streaming, partial-json parsing often does not surface env values until the object closes. - // Recover them from the raw JSON buffer so the pending bash preview can show `NAME="..." cmd` immediately, - // instead of rendering only the command and making the env assignment appear at the very end. + // The parsed args don't always mirror the exact current stream prefix, so recover + // env from the raw JSON buffer to surface `NAME="..." cmd` in the preview as it + // streams rather than only once the args object finishes. const partialEnv = extractPartialBashEnv(args.__partialJson); if (partialEnv && args.env) return { ...partialEnv, ...args.env }; return args.env ?? partialEnv; diff --git a/packages/coding-agent/test/modes/controllers/event-controller-args-reveal.test.ts b/packages/coding-agent/test/modes/controllers/event-controller-args-reveal.test.ts index 83569b063..bf9bf0540 100644 --- a/packages/coding-agent/test/modes/controllers/event-controller-args-reveal.test.ts +++ b/packages/coding-agent/test/modes/controllers/event-controller-args-reveal.test.ts @@ -70,6 +70,18 @@ async function dispatch(controller: EventController, message: AssistantMessage) await controller.handleEvent(event); } +async function dispatchToolStart( + controller: EventController, + payload: { toolCallId: string; toolName: string; args: Record }, +) { + await controller.handleEvent({ + type: "tool_execution_start", + toolCallId: payload.toolCallId, + toolName: payload.toolName, + args: payload.args, + } as Extract); +} + describe("EventController paces streamed tool args", () => { afterEach(() => { vi.useRealTimers(); @@ -146,4 +158,38 @@ describe("EventController paces streamed tool args", () => { vi.advanceTimersByTime(STREAMING_REVEAL_FRAME_MS * 5); expect(updateArgsSpy.mock.calls.length).toBe(calls); }); + + it("reconciles validated full args on tool_execution_start when the closing args update never lands", async () => { + await Settings.init({ inMemory: true, cwd: process.cwd() }); + vi.useFakeTimers(); + const content = "y".repeat(50); + const target = `{"path":"/tmp/exec.ts","content":"${content}"}`; + const streaming = makeStreamingMessage([ + { type: "toolCall", id: "tc-1", name: "write", arguments: {}, partialJson: target } as never, + ]); + const { controller, pendingTools } = createFixture(streaming); + + // Args still streaming: the reveal seeds the preview at an empty prefix, so + // the write head shows its `…` path placeholder rather than the real path. + await dispatch(controller, streaming); + const component = pendingTools.get("tc-1"); + if (!component) throw new Error("expected a pending write component"); + expect(Bun.stripANSI(component.render(80).join("\n"))).not.toContain("/tmp/exec.ts"); + + // The closing full-args message_update never arrives (throttled `arguments` + // with smoothing off, an owned-dialect projector, or a superseded turn that + // still runs the call). The tool executes anyway: tool_execution_start is the + // one event every path emits with the validated args, so it must reconcile. + await dispatchToolStart(controller, { + toolCallId: "tc-1", + toolName: "write", + args: { path: "/tmp/exec.ts", content }, + }); + expect(Bun.stripANSI(component.render(80).join("\n"))).toContain("/tmp/exec.ts"); + + // The reveal entry was cancelled: a late tick cannot re-truncate the body + // back to a streaming prefix. + vi.advanceTimersByTime(STREAMING_REVEAL_FRAME_MS * 5); + expect(Bun.stripANSI(component.render(80).join("\n"))).toContain("/tmp/exec.ts"); + }); });