Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions src/handlers/dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<InternalEvent, { kind: "ToolCallUpdate" }>,
toolName: string,
): Promise<void> {
// ① 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<string, unknown>)["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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When the stdout tail buffer rotates (i.e., older output is discarded from the beginning of the cumulative snapshot), full.startsWith(lastSent) will be false. In this case, falling back to the full full snapshot causes the remaining overlapping part of the buffer to be replayed and duplicated in the terminal. Since long-running commands are exactly the ones that will trigger tail rotation, this defeats the purpose of preventing replay.

We can resolve this by finding the longest suffix of lastSent that matches a prefix of full (the overlap) and sending only the remaining suffix of full as the delta.

  let delta = full;
  if (lastSent) {
    if (full.startsWith(lastSent)) {
      delta = full.slice(lastSent.length);
    } else {
      const maxOverlap = Math.min(lastSent.length, full.length);
      for (let i = maxOverlap; i > 0; i--) {
        if (lastSent.endsWith(full.slice(0, i))) {
          delta = full.slice(i);
          break;
        }
      }
    }
  }

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 } },
});
}

Expand All @@ -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);
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ export class ZcodeAcpServer {
>();
/** Per-session model cache for configOptions model dropdown. */
readonly modelCache = new Map<string, string>();
/**
* 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<string, string>();
/** Monotonic id counter; base 10_000_000 to avoid collisions with zcode-originated ids. */
private msgCounter = 10_000_000;

Expand Down
71 changes: 70 additions & 1 deletion tests/dispatch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Extract<InternalEvent, { kind: "ToolCallUpdate" }>> = {
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" } },
});
});
Comment thread
william0wang marked this conversation as resolved.

it("Bash completed emits 2 notifications: terminal_output delta + terminal_exit", async () => {
const { cx, sent } = mockContext();
const server = makeServer(true);
const ev: InternalEvent = {
Expand Down Expand Up @@ -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<Extract<InternalEvent, { kind: "ToolCallUpdate" }>> = {
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);
Expand Down
Loading