feat(acp): stream session/prompt deltas as session/update chunks (#3192)#3702
Merged
Conversation
…own#3192) Follow-up to Hmbown#3698. The ACP adapter buffered the whole turn: it called the non-streaming `create_message` and emitted a single `session/update` chunk only after the provider finished. Editors like Zed show agent output incrementally, so users waited for the entire response with no feedback, and the readiness audit listed non-streaming as a known limitation. Stream the turn instead, reusing the existing `LlmClient::create_message_stream` path that the TUI engine already uses: - `open_prompt_stream` builds the request with `stream: true` and returns the `'static` `StreamEventBox`. It borrows `&self` only to read config/model and open the stream, so the caller can race it against the reader without holding a borrow on the server. - `drive_prompt_stream` consumes the stream concurrently with the input reader: each text delta (`content_block_delta`/`content_block_start` text) is emitted as its own `session/update` agent_message_chunk as it arrives; `message_stop` or stream end completes the turn with the accumulated text (recorded in history); a provider stream error surfaces as a JSON-RPC error. - Cancellation is preserved and now interrupts mid-stream: a matching `session/cancel` returns `stopReason: "cancelled"` and dropping the stream aborts the underlying provider connection. Single-flight semantics are unchanged (other-session cancel ignored; concurrent request rejected with a clear error). The single-writer invariant holds, so streamed chunks and acknowledgements stay on one protocol-clean stdout stream. The streaming + cancellation control logic stays a free function over the reader/writer + boxed stream, so it is unit-tested with canned in-memory streams (delta-by-delta chunking, cancel mid-stream, other-session cancel ignored, concurrent request rejected) — no real provider call. Full bin suite 5521 green; fmt + clippy clean. Updates docs/ACP_REGISTRY_SUBMISSION.md. Refs Hmbown#3192. Signed-off-by: findshan <224246733+findshan@users.noreply.github.com>
|
Thanks @findshan for taking the time to contribute. This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered. Please read |
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
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.
What & why
Follow-up to #3698 (concurrent
session/cancel). The ACP adapter still buffered the whole turn: it called the non-streamingcreate_messageand emitted a singlesession/updatechunk only after the provider finished. Editors like Zed render agent output incrementally, so a user saw nothing until the entire response landed — and the readiness audit indocs/ACP_REGISTRY_SUBMISSION.mdexplicitly listed non-streaming as a known limitation. This closes that gap.Approach
Stream the turn, reusing the existing
LlmClient::create_message_streampath the TUI engine already uses (turn_loop.rs), so no new provider plumbing:open_prompt_streambuilds the request withstream: trueand returns the'staticStreamEventBox. It borrows&selfonly to read config/model and open the stream, so the caller can race it against the reader without holding a server borrow. (Replaces the oldrun_promptthat awaited a fullcreate_message.)drive_prompt_streamconsumes the stream concurrently with the input reader:content_block_delta/ textcontent_block_start) → its ownsession/updateagent_message_chunk, emitted as it arrives;message_stop(or stream end) →PromptOutcome::Completed(full_text), recorded in session history;-32603.session/cancelreturnsstopReason: "cancelled"and dropping the stream aborts the underlying provider connection. Single-flight semantics unchanged (other-session cancel ignored; concurrent request rejected with a clear error). The single-writer invariant holds, so streamed chunks + acks all stay on one protocol-clean stdout stream.Testing
The streaming + cancellation control logic stays a free function over the reader/writer + boxed stream, so it's unit-tested deterministically with canned in-memory streams — no real provider. 4 tests:
drive_prompt_streams_each_delta_as_a_chunk_then_completes— two deltas → twosession/updatechunks, full text accumulated.drive_prompt_cancels_when_matching_cancel_arrives— cancel wins over a pending stream →Cancelled.drive_prompt_ignores_cancel_for_a_different_session— other-session cancel acked + ignored, turn completes.drive_prompt_rejects_a_concurrent_request_but_keeps_running— concurrent request gets-32603, turn completes.Local:
cargo fmt --all -- --checkclean; clippy (bin, repo flags) clean; full bin suite 5521 passed, 0 failed.Scope
tools: None). Doc updated to say so.Refs #3192.