Skip to content

fix: send only stdout delta in Bash terminal output to prevent replay#15

Merged
william0wang merged 1 commit into
mainfrom
chore/fix-terminal-dup
Jul 10, 2026
Merged

fix: send only stdout delta in Bash terminal output to prevent replay#15
william0wang merged 1 commit into
mainfrom
chore/fix-terminal-dup

Conversation

@william0wang

Copy link
Copy Markdown
Owner

No description provided.

@william0wang william0wang merged commit 2bb43cf into main Jul 10, 2026
1 check passed
@william0wang william0wang deleted the chore/fix-terminal-dup branch July 10, 2026 14:14

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the terminal output streaming and deduplication logic in dispatchTerminalUpdate to prevent duplicate lines in the Zed terminal. It splits terminal updates into separate terminal_output and terminal_exit notifications, and avoids sending rawOutput inside terminal_exit. Additionally, it skips terminal_output on result events if progress has already streamed data, unless no progress events were fired. The review feedback points out a correctness bug where skipping terminal_output entirely on result events when progress has already streamed can cause final output truncation. Any output appended after the last progress event but before completion would be lost. The reviewer suggests removing the !(isResult && alreadyStreamed) guard and relying on the existing delta-deduplication logic to handle sending only the new suffix.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/handlers/dispatch.ts
Comment on lines +175 to 187
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 } },
});
}
}

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

Correctness Bug: Final Output Truncation / Loss

By skipping the terminal_output notification entirely when isResult && alreadyStreamed is true, any new output appended to the stdout tail after the last progress event but before the command completed/failed will be completely lost.

Since terminal_exit no longer carries rawOutput (which is correct to prevent duplication in Zed), there is no other way for this final output delta to reach the client.

Solution

We do not need to skip terminal_output entirely. The existing delta/deduplication logic (const delta = lastSent && full.startsWith(lastSent) ? full.slice(lastSent.length) : full;) already perfectly handles deduplication:

  • If there is no new output, delta will be empty ("") and nothing will be sent.
  • If there is new output, only the new suffix will be sent.

We should remove the !(isResult && alreadyStreamed) guard and simply process the delta normally.

  const isResult = ev.status === "completed" || ev.status === "failed";
  if (full) {
    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 } },
      });
    }
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant