feat(acp): cancel in-flight session/prompt on session/cancel (#3192)#3698
Merged
Conversation
…3192) The ACP stdio adapter processed input strictly serially: the read loop awaited each request — including the full provider call behind `session/prompt` — before reading the next line. A `session/cancel` therefore could not even be observed until the turn it was meant to cancel had already finished, so it was a documented no-op (it just returned `null`). Editors like Zed send `session/cancel` when the user hits Esc, so the in-flight turn kept running and streamed a result the user no longer wanted. Drive `session/prompt` concurrently with the reader instead: - `run_prompt` borrows `&self` only and never touches the writer, so it is raced against `reader.next_line()` in `drive_prompt_with_cancellation` while the main task keeps exclusive ownership of stdout — stdout stays protocol-clean (the existing single-line-JSON invariant holds). - A `session/cancel` for the in-flight session (request or notification form) aborts the awaited provider call and the prompt returns `stopReason: "cancelled"`; the assistant turn is not appended to history. - A no-prompt `session/cancel` remains an idempotent `null` no-op. - The turn is single-flight: a cancel for a different session is acknowledged and ignored; any other concurrent request gets a clear "prompt in progress" error rather than being silently dropped. The cancellation control logic is extracted into a generic free function so it is unit-tested with a delayed future and an in-memory reader — no real provider call required. 4 new tests cover complete, cancel, other-session-ignored, and concurrent-request-rejected paths. Refs Hmbown#3192 (advances the ACP readiness audit in docs/ACP_REGISTRY_SUBMISSION.md). 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 |
Hmbown
pushed a commit
that referenced
this pull request
Jun 27, 2026
Follow-up to #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 #3192. Signed-off-by: findshan <224246733+findshan@users.noreply.github.com>
Hmbown
added a commit
that referenced
this pull request
Jun 27, 2026
Refs #3192. Streams ACP session/prompt text deltas as session/update chunks instead of buffering the full provider response, while preserving the single-writer stdout path and mid-stream session/cancel behavior from #3698. Verification: GitHub CI green across lint, Ubuntu/macOS/Windows tests, npm/mobile smoke, DCO/gate, Version drift, Claude review, and GitGuardian. Local targeted verification also passed in an isolated worktree: cargo test -p codewhale-tui --bin codewhale-tui --locked acp_server:: -- --nocapture (18 passed). Thank you @findshan for the careful ACP streaming slice and @Jengro777 for keeping the ACP registry/Zed path concrete.
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
2 participants
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
The ACP stdio adapter processed input strictly serially: the read loop
awaited each request — including the full provider call behindsession/prompt— before reading the next line. So asession/cancelcould not even be observed until the turn it was meant to cancel had already finished. Per the readiness audit indocs/ACP_REGISTRY_SUBMISSION.md,session/cancelwas therefore a documented no-op (returnednull).Editors like Zed send
session/cancelwhen the user hits Esc. Today that does nothing: the in-flight turn keeps running and emits asession/update+ result the user no longer wants. This closes that gap.Approach
Drive
session/promptconcurrently with the reader, keeping a single writer so stdout stays protocol-clean:run_promptborrows&selfonly and never touches the writer, so it is raced againstreader.next_line()inside a newdrive_prompt_with_cancellationwhile the main task retains exclusive ownership of stdout. The existing single-line-JSON invariant (and its test) is preserved.session/cancelfor the in-flight session (request or notification form) aborts the awaited provider call; the prompt returnsstopReason: "cancelled"and the assistant turn is not appended to history.session/cancelstays an idempotentnullno-op.-32603"prompt in progress" error instead of being silently dropped (notifications without an id are ignored).The
&mut selfprep (validate + record the user turn) and the borrow-free provider call are split intobegin_prompt/run_prompt/finish_promptso the long call can be raced without holding a mutable borrow across the await.Testing
The cancellation control logic is a generic free function over the future + reader/writer, so it is unit-tested deterministically with a delayed future and an in-memory reader — no real provider call. 4 new tests:
drive_prompt_completes_when_no_cancel_arrivesdrive_prompt_cancels_when_matching_cancel_arrivesdrive_prompt_ignores_cancel_for_a_different_sessiondrive_prompt_rejects_a_concurrent_request_but_keeps_runningLocal:
cargo fmt --all -- --checkclean;clippy(bin, repo flags) clean; full bin suite 5518 passed, 0 failed.Scope / open questions
Refs #3192. Updates the
session/cancellines indocs/ACP_REGISTRY_SUBMISSION.mdto match.