Stop output buffer resend in terminal_interaction stdin path#270
Open
frozename wants to merge 2 commits into
Open
Stop output buffer resend in terminal_interaction stdin path#270frozename wants to merge 2 commits into
frozename wants to merge 2 commits into
Conversation
When the ACP client does not advertise terminal_output capability, exec_command_output_delta accumulated each chunk into active_command.output and then re-sent the entire growing buffer as a ToolCallUpdate content block on every chunk. The mpsc notification channel is unbounded, so for N chunks the in-flight queue holds 1 + 2 + ... + N copies of the buffer and the process RSS grows quadratically until the kernel kills it. rg/find/fd are classified as Search/ListFiles by the parser and always hit this fallback regardless of client capability. Match claude-agent-acp's behavior: in the non-terminal fallback, accumulate silently in the delta handler and emit one ToolCallUpdate with the full formatted content block at exec_command_end. Skip the content emission for commands that produced no output to avoid surfacing empty fenced code blocks. Total wire traffic is now O(N) end-to-end, and codex-acp RSS stays flat across large rg/find runs. Closes zed-industries#225.
terminal_interaction's non-terminal fallback appended each stdin event into active_command.output and re-sent the entire growing buffer as a ToolCallUpdate content block. This is the same accumulate-and-resend shape that zed-industries#225 fixed in exec_command_output_delta, just keyed on stdin events instead of stdout chunks. Mirror the prior fix: in the non-terminal fallback, accumulate stdin into the buffer silently and let exec_command_end emit the full content block exactly once at completion. The terminal-output fast path is untouched. Stacked on top of the exec_command_output_delta fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #269.
terminal_interaction's non-terminal fallback has the same accumulate-and-resend shape that #269 fixed inexec_command_output_delta— every stdin event re-sends the entireactive_command.output(output bytes accumulated by the delta handler plus every prior stdin echo) as aToolCallUpdatecontent block. Stdin payloads are tiny on their own, but the buffer being resent is the full output buffer, so the same O(N²) growth pattern applies whenever an interactive command interleaves output streaming with stdin injections.Fix
Mirror the prior change: in the non-terminal fallback, append
stdinintoactive_command.outputand letexec_command_endemit the full content block exactly once at completion. The terminal-output fast path (clients that advertise the capability) is untouched.Stacking
This PR is stacked on top of #269 — it depends on the new
exec_command_endcontent emission introduced there to surface stdin in the final ToolCall update. If #269 lands first this rebases trivially; if a single PR is preferred I'm happy to squash.Verification
cargo build --releaseandcargo clippy --release -- -D warningsclean onaarch64-apple-darwinwith both patches applied.rgregression workload from Fix O(N²) memory growth in exec_command_output_delta fallback #269 against the layered binary: 8 patterns over the same monorepo + recursivefind, completed in 32 s, peakcodex-acpRSS held at ~93 MB — flat profile, no regression.