From add51c2c4eacc10b9413849ee35c6e0461ae2cb0 Mon Sep 17 00:00:00 2001 From: William Wang Date: Fri, 10 Jul 2026 21:28:39 +0800 Subject: [PATCH] fix: send only stdout delta in Bash terminal output to prevent replay --- src/handlers/dispatch.ts | 31 +++++++++++++++--- src/server.ts | 8 +++++ tests/dispatch.test.ts | 71 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 104 insertions(+), 6 deletions(-) diff --git a/src/handlers/dispatch.ts b/src/handlers/dispatch.ts index a20092b..19eecdc 100644 --- a/src/handlers/dispatch.ts +++ b/src/handlers/dispatch.ts @@ -132,31 +132,46 @@ function dispatchToolCallUpdate( * and the Python bridge's _dispatch_event). Zed correlates by terminal_id, so * the two notifications MUST be separate: * ① terminal_output — pure data, no status/content. Sent on BOTH progress and - * result states (whenever there's data) so live output streams. + * result states (whenever there's NEW data) so live output streams. * ② terminal_exit — terminal state only: status + content[type:terminal] + * _meta.terminal_exit (with exitCode) + rawOutput text fallback. Sent ONLY * on completed/failed. * * Merging them into one notification causes Zed to clear the content once the * turn completes (the original bug); splitting keeps the output visible. + * + * DELTA DEDUP: zcode's `stdoutTail`/result `content` is a *snapshot* of the + * accumulated stdout tail, but ACP `terminal_output.data` is *append* semantics + * — each value is written to the terminal as new bytes. Without dedup, every + * progress event replays the whole tail, and the result event replays it again, + * so long-running commands show their output N times. We track the last-sent + * snapshot per callId and emit only the suffix beyond it. */ async function dispatchTerminalUpdate( - _server: ZcodeAcpServer, + server: ZcodeAcpServer, cx: acp.AgentContext, acpSid: string, ev: Extract, toolName: string, ): Promise { - // ① terminal_output (pure data) — progress and result both emit it. + // ① terminal_output (pure data, append semantics) — progress and result both + // emit it, but only the delta beyond the last-sent snapshot. let termData: unknown = ev.rawOutput ?? ev.rawResult; if (termData && typeof termData === "object" && !Array.isArray(termData)) { termData = (termData as Record)["content"] ?? ""; } - if (termData) { + const full = termData ? String(termData) : ""; + const lastSent = server.terminalSentData.get(ev.callId) ?? ""; + // Only the suffix past the previously-sent snapshot is new output. Guard + // against a non-monotonic snapshot (tail rotated) by falling back to the full + // value when the previous snapshot isn't a prefix of the current one. + const delta = lastSent && full.startsWith(lastSent) ? full.slice(lastSent.length) : full; + if (delta) { + server.terminalSentData.set(ev.callId, full); await sendSessionUpdate(cx, acpSid, { sessionUpdate: "tool_call_update", toolCallId: ev.callId, - _meta: { terminal_output: { terminal_id: ev.callId, data: String(termData) } }, + _meta: { terminal_output: { terminal_id: ev.callId, data: delta } }, }); } @@ -174,8 +189,14 @@ async function dispatchTerminalUpdate( }, }; // rawOutput gives Zed a text fallback outside the terminal render path. + // This carries the FULL output (not the delta) so a non-terminal render + // fallback shows everything once; the terminal_output stream already + // appended it above via the dedup delta. if (ev.output !== undefined) exitUpdate.rawOutput = ev.output; await sendSessionUpdate(cx, acpSid, exitUpdate); + // Terminal session ended — clear the snapshot so a future tool reusing this + // callId (shouldn't happen, but defensively) starts fresh. + server.terminalSentData.delete(ev.callId); } } diff --git a/src/server.ts b/src/server.ts index 2d178d8..639b8b4 100644 --- a/src/server.ts +++ b/src/server.ts @@ -65,6 +65,14 @@ export class ZcodeAcpServer { >(); /** Per-session model cache for configOptions model dropdown. */ readonly modelCache = new Map(); + /** + * Per-Bash-callId cumulative stdout snapshot already streamed via + * terminal_output. zcode's `stdoutTail` is a *snapshot* of the tail of the + * accumulated stdout, but ACP `terminal_output.data` is *append* semantics — + * so we must diff each new snapshot against the last-sent one and emit only + * the suffix to avoid replaying the whole output on every progress event. + */ + readonly terminalSentData = new Map(); /** Monotonic id counter; base 10_000_000 to avoid collisions with zcode-originated ids. */ private msgCounter = 10_000_000; diff --git a/tests/dispatch.test.ts b/tests/dispatch.test.ts index 0fc8c58..992c656 100644 --- a/tests/dispatch.test.ts +++ b/tests/dispatch.test.ts @@ -101,7 +101,35 @@ describe("dispatchEvent", () => { expect((sent[0] as { status?: string }).status).toBeUndefined(); }); - it("Bash completed emits 2 notifications: terminal_output + terminal_exit", async () => { + it("Bash progress emits only the delta between cumulative stdout snapshots", async () => { + // zcode sends stdoutTail as a CUMULATIVE tail snapshot on every progress + // event. terminal_output.data is append semantics, so the bridge must diff + // consecutive snapshots and emit only the suffix — otherwise long-running + // commands show their output N times. + const { cx, sent } = mockContext(); + const server = makeServer(true); + const base: Partial> = { + kind: "ToolCallUpdate", + callId: "c1", + tool: "Bash", + status: "in_progress", + }; + await dispatchEvent(server, cx, SID, { ...base, rawOutput: "line1\n" }, CHUNK); + await dispatchEvent(server, cx, SID, { ...base, rawOutput: "line1\nline2\n" }, CHUNK); + await dispatchEvent(server, cx, SID, { ...base, rawOutput: "line1\nline2\nline3\n" }, CHUNK); + expect(sent).toHaveLength(3); + expect(sent[0]).toMatchObject({ + _meta: { terminal_output: { terminal_id: "c1", data: "line1\n" } }, + }); + expect(sent[1]).toMatchObject({ + _meta: { terminal_output: { terminal_id: "c1", data: "line2\n" } }, + }); + expect(sent[2]).toMatchObject({ + _meta: { terminal_output: { terminal_id: "c1", data: "line3\n" } }, + }); + }); + + it("Bash completed emits 2 notifications: terminal_output delta + terminal_exit", async () => { const { cx, sent } = mockContext(); const server = makeServer(true); const ev: InternalEvent = { @@ -133,6 +161,47 @@ describe("dispatchEvent", () => { }); }); + it("Bash result after streamed progress emits only the unsent tail", async () => { + // Progress already streamed "line1\nline2\n"; the result carries the full + // "line1\nline2\nline3\n". Only "line3\n" should be appended. + const { cx, sent } = mockContext(); + const server = makeServer(true); + const base: Partial> = { + kind: "ToolCallUpdate", + callId: "c1", + tool: "Bash", + }; + await dispatchEvent( + server, + cx, + SID, + { ...base, status: "in_progress", rawOutput: "line1\nline2\n" }, + CHUNK, + ); + await dispatchEvent( + server, + cx, + SID, + { + ...base, + status: "completed", + rawOutput: "line1\nline2\nline3\n", + output: "line1\nline2\nline3\n", + rawResult: { success: true, content: "line1\nline2\nline3\n", perf: { exitCode: 0 } }, + }, + CHUNK, + ); + // progress (1) + terminal_output delta (1) + terminal_exit (1) + expect(sent).toHaveLength(3); + expect(sent[0]).toMatchObject({ + _meta: { terminal_output: { data: "line1\nline2\n" } }, + }); + expect(sent[1]).toMatchObject({ + _meta: { terminal_output: { data: "line3\n" } }, + }); + expect(sent[2]).toMatchObject({ status: "completed" }); + }); + it("Bash failed extracts exit code 1 when no perf.exitCode", async () => { const { cx, sent } = mockContext(); const server = makeServer(true);