diff --git a/src/handlers/dispatch.ts b/src/handlers/dispatch.ts index 19eecdc..08661d2 100644 --- a/src/handlers/dispatch.ts +++ b/src/handlers/dispatch.ts @@ -131,21 +131,23 @@ function dispatchToolCallUpdate( * Bash terminal update — the 2-notification split (matches acp-agent.ts:5061-5094 * 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 NEW data) so live output streams. + * ① terminal_output — pure data, no status/content. Streams live output. * ② terminal_exit — terminal state only: status + content[type:terminal] + - * _meta.terminal_exit (with exitCode) + rawOutput text fallback. Sent ONLY - * on completed/failed. + * _meta.terminal_exit (with exitCode). 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. + * OUTPUT DEDUP (two distinct hazards, both fixed here): + * - Progress replay: zcode's `stdoutTail` is a CUMULATIVE snapshot of the tail, + * but terminal_output.data is APPEND semantics. Without diffing, each progress + * event replays the whole tail → N× duplication. We track the last-sent + * snapshot per callId and emit only the suffix beyond it. + * - Result replay: zcode's result payload re-sends the COMPLETE output. Once + * progress has streamed it, emitting it again would duplicate every line. + * So on completed/failed we SKIP terminal_output IF any progress already + * streamed data for this callId; for short commands (scheduled → result, no + * progress) nothing was sent yet, so we emit the full output once. */ async function dispatchTerminalUpdate( server: ZcodeAcpServer, @@ -154,29 +156,41 @@ async function dispatchTerminalUpdate( ev: Extract, toolName: string, ): Promise { - // ① terminal_output (pure data, append semantics) — progress and result both - // emit it, but only the delta beyond the last-sent snapshot. + // Extract the textual output from whichever payload field carries it. let termData: unknown = ev.rawOutput ?? ev.rawResult; if (termData && typeof termData === "object" && !Array.isArray(termData)) { termData = (termData as Record)["content"] ?? ""; } 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: delta } }, - }); + + // ① terminal_output (pure data, append semantics). + // - progress (in_progress): zcode's stdoutTail is a CUMULATIVE snapshot of + // the tail, but terminal_output.data is APPEND semantics, so we diff and + // emit only the suffix beyond the last-sent snapshot. + // - result (completed/failed): the full output was already streamed during + // progress, and zcode's result re-sends the complete tail → skip, UNLESS + // no progress ever fired (short command: scheduled → result). In that + // case nothing was streamed yet, so emit the full output once. + const isResult = ev.status === "completed" || ev.status === "failed"; + const alreadyStreamed = server.terminalSentData.has(ev.callId); + if (full && !(isResult && alreadyStreamed)) { + 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: delta } }, + }); + } } // ② terminal_exit (terminal state) — only on completed/failed. - if (ev.status === "completed" || ev.status === "failed") { + // Deliberately omits rawOutput: the output was already streamed via + // terminal_output.data above, and Zed renders BOTH the terminal buffer and + // the rawOutput fallback — including rawOutput here duplicates every line. + if (isResult) { const exitCode = extractExitCode(ev.rawResult, ev.status === "failed"); const exitUpdate: acp.SessionUpdate = { sessionUpdate: "tool_call_update", @@ -188,11 +202,6 @@ async function dispatchTerminalUpdate( terminal_exit: { terminal_id: ev.callId, exit_code: exitCode, signal: null }, }, }; - // 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. diff --git a/src/server.ts b/src/server.ts index 639b8b4..7c5ee28 100644 --- a/src/server.ts +++ b/src/server.ts @@ -66,11 +66,12 @@ 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. + * Per-Bash-callId stdout snapshot already streamed via terminal_output. Used + * by dispatchTerminalUpdate for two dedup guards: + * - progress: diff cumulative stdoutTail snapshots, emit only the suffix. + * - result: if present, the output was already streamed → skip replay. + * Presence of a key also signals "progress fired for this call" (absent = + * short command with no progress, so the result emits output once). */ readonly terminalSentData = new Map(); /** Monotonic id counter; base 10_000_000 to avoid collisions with zcode-originated ids. */ diff --git a/tests/dispatch.test.ts b/tests/dispatch.test.ts index 992c656..6949175 100644 --- a/tests/dispatch.test.ts +++ b/tests/dispatch.test.ts @@ -147,7 +147,9 @@ describe("dispatchEvent", () => { expect(sent[0]).toMatchObject({ _meta: { terminal_output: { terminal_id: "c1", data: "done" } }, }); - // ② terminal_exit (terminal state) + // ② terminal_exit (terminal state) — no rawOutput: the output was already + // streamed via terminal_output, and including rawOutput makes Zed render + // it a second time (the duplication bug). expect(sent[1]).toMatchObject({ sessionUpdate: "tool_call_update", toolCallId: "c1", @@ -157,13 +159,13 @@ describe("dispatchEvent", () => { claudeCode: { toolName: "Bash" }, terminal_exit: { terminal_id: "c1", exit_code: 0, signal: null }, }, - rawOutput: "done", }); + expect((sent[1] as { rawOutput?: string }).rawOutput).toBeUndefined(); }); - 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. + it("Bash result after streamed progress skips terminal_output (no replay)", async () => { + // Progress already streamed the full output; zcode's result re-sends the + // complete tail. terminal_output must NOT fire again — only terminal_exit. const { cx, sent } = mockContext(); const server = makeServer(true); const base: Partial> = { @@ -175,7 +177,7 @@ describe("dispatchEvent", () => { server, cx, SID, - { ...base, status: "in_progress", rawOutput: "line1\nline2\n" }, + { ...base, status: "in_progress", rawOutput: "line1\nline2\nline3\n" }, CHUNK, ); await dispatchEvent( @@ -191,15 +193,34 @@ describe("dispatchEvent", () => { }, CHUNK, ); - // progress (1) + terminal_output delta (1) + terminal_exit (1) - expect(sent).toHaveLength(3); + // progress terminal_output (1) + result terminal_exit (1). No second data. + expect(sent).toHaveLength(2); expect(sent[0]).toMatchObject({ - _meta: { terminal_output: { data: "line1\nline2\n" } }, + _meta: { terminal_output: { data: "line1\nline2\nline3\n" } }, }); - expect(sent[1]).toMatchObject({ - _meta: { terminal_output: { data: "line3\n" } }, + expect(sent[1]).toMatchObject({ status: "completed" }); + }); + + it("Bash result with no prior progress emits full output once", async () => { + // Short command: scheduled → result, no progress event. Nothing was streamed + // yet, so the result must emit terminal_output once so output is visible. + const { cx, sent } = mockContext(); + const server = makeServer(true); + const ev: InternalEvent = { + kind: "ToolCallUpdate", + callId: "c1", + tool: "Bash", + status: "completed", + rawOutput: "quick result\n", + output: "quick result\n", + rawResult: { success: true, content: "quick result\n", perf: { exitCode: 0 } }, + }; + await dispatchEvent(server, cx, SID, ev, CHUNK); + expect(sent).toHaveLength(2); + expect(sent[0]).toMatchObject({ + _meta: { terminal_output: { data: "quick result\n" } }, }); - expect(sent[2]).toMatchObject({ status: "completed" }); + expect(sent[1]).toMatchObject({ status: "completed" }); }); it("Bash failed extracts exit code 1 when no perf.exitCode", async () => {