fix: make Escape and Ctrl+C actually interrupt agent during streaming#106
Merged
emal-avala merged 2 commits intomainfrom Apr 15, 2026
Merged
fix: make Escape and Ctrl+C actually interrupt agent during streaming#106emal-avala merged 2 commits intomainfrom
emal-avala merged 2 commits intomainfrom
Conversation
Three bugs prevented users from interrupting a running agent turn: 1. Signal handler held a stale cancellation token clone — the token was replaced each turn but the handler kept the original, so Ctrl+C only worked on the very first turn. 2. The LLM streaming loop used `while let Some(event) = rx.recv().await` which never checked the cancellation token — even a successful cancel would wait for the full response to finish. 3. The Escape key watcher was removed in a prior commit due to stdin contention with rustyline. It is re-added here with a safer RAII-based approach that enables raw mode only during streaming and cleans up on drop. Fixes: - Use Arc<Mutex<CancellationToken>> so the signal handler always cancels the current turn's token - Replace the streaming while-let with tokio::select! that races the stream against the cancellation token - Add Escape key watcher with proper RAII guard and cleanup - Add unit tests for cancellation propagation and stream interruption Closes #103
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
- Collapse nested if into let-chain (clippy collapsible_if) - Fix rustfmt issues in EscapeWatcherGuard::drop and test assert - Add real end-to-end test using QueryEngine with a hanging mock provider, verifying that cancelling via cancel_shared actually interrupts run_turn_with_sink (not just the select pattern in isolation). This is the true regression test for #103.
emal-avala
added a commit
that referenced
this pull request
Apr 15, 2026
Follow-up to #106. The initial e2e test only verified cancellation during a single turn. This adds coverage for the specific regression scenarios the original fix was designed to prevent. New tests: - cancel_works_across_multiple_turns: the actual #103 regression. Cancels turn 1, then runs turn 2 and verifies it's still cancellable via the same shared handle. Pre-fix this would hang forever because the signal handler held a stale token clone. - cancel_does_not_poison_next_turn: pre-cancels the shared token, then runs a normal turn to verify run_turn_with_sink resets to a fresh un-cancelled token. - cancel_before_first_event_interrupts_cleanly: race condition edge case where cancel fires during or before the first rx.recv(). - cancelled_turn_emits_warning_to_sink: verifies the sink is notified via on_warning("Cancelled"), confirming the UI feedback path works. Refactors the existing test to share a build_engine/schedule_cancel helper so new scenarios stay concise.
This was referenced Apr 15, 2026
Merged
emal-avala
added a commit
that referenced
this pull request
Apr 15, 2026
Since #106 added the escape-key watcher, the terminal is held in raw mode for the entire duration of a streaming turn. In raw mode a bare `\n` moves the cursor down one row without returning to column 0, so every newline the LLM emits caused the next line to start at the column where the previous line ended. Over a multi-paragraph reply the text drifted off the right edge of the terminal. Route every print site inside the raw-mode window through two new helpers (`raw_print` / `raw_eprint`) that translate LF to CRLF before writing. The tui renderer (`render_tool_block`, `render_turn_summary`, etc.) already emits `\r\n` via `render_lines_to_ansi`, so it's untouched. The unaffected print paths outside the turn (setup wizard, shortcut panel, session summary) are also untouched. Eight unit tests cover the translation table, including the two edge cases that matter at runtime: existing `\r\n` must not become `\r\r\n`, and bare `\r` (used by the activity indicator to rewrite the status line) must pass through unchanged.
5 tasks
emal-avala
added a commit
that referenced
this pull request
Apr 15, 2026
The Escape-key interrupt added in #106 never actually worked against a real LLM. The key press was detected, `cancel_token.cancel()` fired, and the query engine's outer select loop at `query/mod.rs:531` exited cleanly — but the provider's own streaming task (spawned in `anthropic.rs`, `openai.rs`, `azure_openai.rs`) kept polling `byte_stream.next().await` because it had no knowledge of the token. The reqwest response stayed open and the task kept emitting events into a receiver nobody was reading, so from the user's seat the turn looked uninterrupted until the LLM finished writing. Fix: - add `cancel: CancellationToken` to `ProviderRequest` - in each provider's spawned task, race `byte_stream.next().await` against `cancel.cancelled()` via `tokio::select!` (biased on cancel). On cancel we `return` from the task, which drops the byte stream, drops the `reqwest::Response`, and aborts the underlying HTTP connection immediately. - thread the token through all four `ProviderRequest` call sites: `query/mod.rs` and `services/compact.rs` pass `self.cancel.clone()` so Esc interrupts both the main turn and any inline compaction run; `memory/extraction.rs` and `memory/consolidation.rs` pass a fresh `CancellationToken::new()` since they're background tasks that should not be user-cancellable. - add `compact_with_llm(..., cancel)` parameter; update caller in `query/mod.rs`. Regression test: `provider_stream_task_observes_cancellation` introduces `CancelAwareHangingProvider`, a mock provider whose spawned task mirrors the real one — it races a `pending` future (standing in for `byte_stream.next().await`) against the `ProviderRequest::cancel` token and flips an `exit_flag` when the token fires. The test then runs a turn, schedules a cancel, and asserts the flag flipped. The existing `HangingProvider` tests kept passing while the feature was broken because that mock ignores the token and the query loop exits on its own — this new test fails if the token is dropped anywhere between `query::mod.rs` and the provider's spawn.
5 tasks
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.
Summary
Fixes three bugs that prevented users from interrupting a running agent turn (#103):
CancellationToken, butrun_turn_with_sinkreplaced it each turn. Now usesArc<Mutex<CancellationToken>>so the handler always cancels the current token.while let Some(event) = rx.recv().awaitloop never checked the token. Now usestokio::select!to race the stream againstcancel.cancelled(), breaking immediately on interrupt.077aa5b, using a safer RAII-based approach — crossterm raw mode is only enabled during streaming and properly cleaned up viaDrop.Test plan
cancel_shared_propagates_to_current_token— verifies shared handle cancels current turnstream_loop_responds_to_cancellation— verifiestokio::select!pattern breaks on cancelCloses #103