fix(esc): cancel streaming API call when abort signal trips - #144
Merged
Conversation
Even after PRs #135/#140/#141/#143 wired ESC through subagents, headless SIGINT, the non-optional ``tool_context.abort_controller`` contract, and ripgrep mid-search, the user-visible ESC latency was still 20+ seconds when the model emitted a multi-tool_use response. Root cause: the provider's streaming HTTP read had no way to observe the abort controller. ``query.py`` passed no abort plumbing to ``chat_stream_response``; the only existing interrupt point was the ``on_text_chunk`` callback, which never fires for a turn that emits tool_use blocks without intervening text. The model could spend ~20s generating eight parallel ``Write`` blocks, and only after the stream returned naturally would the outer query loop check the abort signal and yield "Interrupted by user" for all eight. Thread an ``AbortSignal`` through ``chat_stream_response``. In the Anthropic provider, register a listener that calls ``stream.response.close()`` (the same close pattern the existing ``StreamWatchdog`` uses for idle timeout). Closing the underlying HTTP response causes the SDK's blocking socket read to raise in the consumer thread; the provider catches it, detects the abort via the signal state (not the exception class — different SDK versions raise different classes), and re-raises ``AbortError``. Three defenses against the registration race: * Pre-call fast-path bails when the signal was already tripped at call entry — skips the round-trip entirely. * Register-then-recheck after entering the stream context — closes the sub-microsecond window between an ``aborted`` read and a ``add_listener`` append where ``_fire`` could snapshot the listener list and silently drop our newly-appended listener. * Post-with-block recheck — surfaces a signal that fires between ``__exit__`` and ``return``. Plumbing: * ``query.py``: ``_call_model_sync`` grows ``abort_signal`` keyword and forwards to the provider; call site passes ``params.abort_controller.signal``. New ``except AbortError: raise`` in ``_call_model_sync`` and ``except AbortError: pass`` in the query loop route the cancel to the existing post-API abort-handling block in one place. * ``agent_loop.py``: ``_call_provider_for_turn`` grows ``cancel_signal`` and forwards as ``abort_signal=`` to the provider; ``run_agent_loop`` passes through. * ``minimax_provider.py`` / ``openai_compatible.py``: pre-call fast-path for parity (mid-stream listener for these providers is a future PR — same underlying anthropic SDK for Minimax makes it a small lift). Five regression tests pin the contract: pre-aborted fast-path skips ``_ensure_client``, mid-stream abort closes the stream and raises ``AbortError`` within <1s (vs the 20s+ symptom this PR fixes), unaborted streams return normally, ``abort_signal=None`` is a no-op for legacy callers, and the listener detaches after normal completion. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 15, 2026
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…ce314#144) Even after PRs agentforce314#135/agentforce314#140/agentforce314#141/agentforce314#143 wired ESC through subagents, headless SIGINT, the non-optional ``tool_context.abort_controller`` contract, and ripgrep mid-search, the user-visible ESC latency was still 20+ seconds when the model emitted a multi-tool_use response. Root cause: the provider's streaming HTTP read had no way to observe the abort controller. ``query.py`` passed no abort plumbing to ``chat_stream_response``; the only existing interrupt point was the ``on_text_chunk`` callback, which never fires for a turn that emits tool_use blocks without intervening text. The model could spend ~20s generating eight parallel ``Write`` blocks, and only after the stream returned naturally would the outer query loop check the abort signal and yield "Interrupted by user" for all eight. Thread an ``AbortSignal`` through ``chat_stream_response``. In the Anthropic provider, register a listener that calls ``stream.response.close()`` (the same close pattern the existing ``StreamWatchdog`` uses for idle timeout). Closing the underlying HTTP response causes the SDK's blocking socket read to raise in the consumer thread; the provider catches it, detects the abort via the signal state (not the exception class — different SDK versions raise different classes), and re-raises ``AbortError``. Three defenses against the registration race: * Pre-call fast-path bails when the signal was already tripped at call entry — skips the round-trip entirely. * Register-then-recheck after entering the stream context — closes the sub-microsecond window between an ``aborted`` read and a ``add_listener`` append where ``_fire`` could snapshot the listener list and silently drop our newly-appended listener. * Post-with-block recheck — surfaces a signal that fires between ``__exit__`` and ``return``. Plumbing: * ``query.py``: ``_call_model_sync`` grows ``abort_signal`` keyword and forwards to the provider; call site passes ``params.abort_controller.signal``. New ``except AbortError: raise`` in ``_call_model_sync`` and ``except AbortError: pass`` in the query loop route the cancel to the existing post-API abort-handling block in one place. * ``agent_loop.py``: ``_call_provider_for_turn`` grows ``cancel_signal`` and forwards as ``abort_signal=`` to the provider; ``run_agent_loop`` passes through. * ``minimax_provider.py`` / ``openai_compatible.py``: pre-call fast-path for parity (mid-stream listener for these providers is a future PR — same underlying anthropic SDK for Minimax makes it a small lift). Five regression tests pin the contract: pre-aborted fast-path skips ``_ensure_client``, mid-stream abort closes the stream and raises ``AbortError`` within <1s (vs the 20s+ symptom this PR fixes), unaborted streams return normally, ``abort_signal=None`` is a no-op for legacy callers, and the listener detaches after normal completion. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…viders PR agentforce314#144 added abort-signal-aware streaming for ``AnthropicProvider``, but ``OpenAICompatibleProvider`` and ``MinimaxProvider`` got only the pre-call fast-path. Users running through LiteLLM / GLM / OpenAI / DeepSeek — the most common "OpenAI-compatible proxy → Claude" stack — still saw ESC wait the full model latency before the post-API abort check fired. Same 20+ second symptom from before agentforce314#144. Port the response-close listener pattern from agentforce314#144's AnthropicProvider: * Register a listener on the abort signal that calls ``stream.response.close()`` to close the underlying HTTP socket. Closes interrupt the SDK's blocking next-chunk read so the iterator raises immediately, even when the model is in a multi-second gap between chunks (extended thinking, tool_use generation). * For OpenAI-compatible providers, additionally add an in-loop ``if abort_signal.aborted: break`` check at the top of each ``for chunk in stream`` iteration. Covers the case where chunks arrive back-to-back fast enough that the listener's close lands one iteration late, or where the SDK has already prefetched chunks past the close point. * Signal-state-authoritative exception translation in the ``except Exception`` block — different SDK versions raise different exception classes when the response is closed mid-read, so the signal is the only stable abort indicator. * Register-then-recheck ordering closes the sub-microsecond race where ``_fire`` can snapshot the listener list and silently drop a freshly-appended listener. * ``finally`` block detaches the listener so long-lived controllers (the REPL engine's, reused across many turns) don't accumulate dead listeners. Minimax wraps the anthropic SDK against its compatible endpoint, so it gets the AnthropicProvider treatment (no in-loop check — the ``with client.messages.stream(...) as stream:`` pattern only exposes ``text_stream``, not a generic iterator). Ten regression tests pin the contract: * ``test_openai_compat_abort_signal.py`` (6 tests) — pre-abort fast-path with leaf-level ``assert_not_called()``, mid-stream close via response.close + timing bound, **load-bearing** in-loop check (asserts ``on_text_chunk`` saw only "first" not "second" — mutation-verified by deleting the in-loop check and watching the test fail), normal-completion regression check, ``abort_signal=None`` legacy parity, listener detachment. * ``test_minimax_abort_signal.py`` (4 tests) — same shape as AnthropicProvider, with ``_ensure_client`` as the fast-path sentinel. Three-way duplication of the close-listener pattern (Anthropic, Minimax, OpenAI-compat) is acknowledged. Extracting a shared helper is left as a follow-up — the three providers' surrounding contexts differ enough (Anthropic has the watchdog + non-streaming fallback, OpenAI-compat has bare-iterator semantics, Minimax has the ``with``-block + ``get_final_message``) that a premature extraction would either grow the helper to a 4-knob API or leak abstraction. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…ncel PRs agentforce314#144 and agentforce314#145 added abort-signal-aware streaming for three providers (Anthropic, Minimax, OpenAI-compatible). The pattern across all three is structurally identical — pre-call fast-path, register- then-recheck listener closing the stream's response on abort, signal- state-authoritative exception translation, listener detachment, post- stream recheck — but the bookkeeping was inlined and triplicated. Adding a fourth provider would have meant a fourth copy. Extract the pattern into ``src/providers/_stream_abort.py``: * ``StreamAbortGuard(abort_signal)`` — one per call. ``abort_signal=None`` makes every method a no-op so providers can use it unconditionally. * ``raise_if_pre_aborted()`` — pre-call fast-path raising ``AbortError``. * ``raise_if_post_aborted()`` — same check at a different boundary. * ``aborted`` property — cheap in-loop check (OpenAI-compat path). * ``reraise_if_aborted(original_exc)`` — translate SDK exceptions to ``AbortError`` via ``raise ... from``; no-op if signal didn't fire. * ``with guard.attach(stream):`` — register a listener that closes ``stream.response`` on abort; detach in ``__exit__``. ``__exit__`` ALSO closes the response if ``signal.aborted`` is True at exit, as a safety net for the race window where ``AbortSignal._fire`` snapshots the listener list, the consumer thread observes ``aborted=True``, breaks out, detaches the listener, and the snapshot's iteration then runs against an empty list — that path would otherwise leak the underlying httpx response open. The close is idempotent on httpx (``if not self.is_closed`` guard) so the double-fire path is safe. Provider line counts: anthropic_provider.py -88 lines minimax_provider.py -73 lines openai_compatible.py -212 lines _stream_abort.py +211 (mostly docstring) Net: providers shrink from 374 lines of abort bookkeeping to 110 (SDK-specific iteration shape only). The provider-level tests from agentforce314#144 and agentforce314#145 (15 tests across three files) pass unmodified, proving behavior preservation end-to-end. Seventeen new unit tests in ``tests/test_stream_abort_guard.py`` pin the helper's contract directly: pre/post fast-paths, ``aborted`` property semantics, ``attach`` lifecycle (register, detach, race- recovery via register-then-recheck), close-failure tolerance, no- response-attribute graceful degradation, ``reraise_if_aborted`` translation + cause chaining + no-op, and — critically — the ``__exit__`` close-on-abort safety net. The safety-net test mutation-verified: removing the ``__exit__`` close branch makes the test fail with the exact expected assertion. The OpenAI-compat in-loop test gains a ``stream.response.close.called`` assertion to pin the close at the provider level too. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
- Codebase stats: 894 files / 177,428 lines (up from 167,034 on 2026-05-14; ~+10.4k lines in two days). - New news entries: * 2026-05-16 Image-handling parity (agentforce314#149/agentforce314#154/agentforce314#155/agentforce314#156) — Read tool TS image pipeline; @image.png mention fix; image tool_result list shape preserved; OpenAI-compat image/document block translation; validate_images_for_api promoted into BaseProvider._prepare_messages; BOM-aware text decoding. * 2026-05-16 Subagent + Bash reliability — custom subagent discovery from .claude/agents/ (agentforce314#151); Bash timeout vs ESC-abort distinction (agentforce314#152); async-subagent AbortController isolation tests (agentforce314#153); REJECT_MESSAGE on cancelled production path (agentforce314#150). * 2026-05-15 to 2026-05-16 ESC cancellation hardening across providers (agentforce314#144–agentforce314#148) — mid-stream cancel under ~50ms for every provider; shared StreamAbortGuard; LiteLLM worker-thread iteration fix. - Core Systems table: * Multi-Provider description extends to call out Anthropic→OpenAI image/document block translation. * New "Cancellation / Abort" row (✅) capturing the recent sweep. * New "Image Handling" row (✅) capturing Tier C parity. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Summary
provider.chat_stream_responsehad no way to observe a tripped abort controller — the only interrupt point wason_text_chunk, which never fires for tool_use-only turnsAbortSignalthroughchat_stream_response; the Anthropic provider registers a listener that callsstream.response.close()(same pattern as the existing idle-timeoutStreamWatchdog), translating the resulting iterator raise toAbortErrorThe user-reported symptom
User pressed ESC during a turn where the model emitted 8 parallel
Writeblocks. The abort controller tripped immediately, butclient.messages.stream(...)happily read to EOF (~20s of model generation). Only then didquery.py's post-API abort check fire and yield "Interrupted by user" for all 8 tool_use blocks. With this fix, the listener forcibly closes the streaming HTTP response within ~50ms of the abort firing.Changes
src/providers/base.py— newabort_signal: AbortSignal | None = Noneparameter onchat_stream_response; docstring explains why text-chunk-based interruption isn't sufficient (tool_use-only turns produce no text chunks to observe).src/providers/anthropic_provider.py— three abort defenses:AbortErrorbefore any network work (skips the round-trip on turn-boundary aborts).abort_signal.add_listener(_close_stream_on_abort, once=True)immediately after entering the stream context. Listener callsstream.response.close(). Register-then-recheck ordering closes the sub-microsecond race where_firecould snapshot the listener list and drop a newly-appended listener.__exit__and return.except Exceptionblock consultsabort_signal.aborted(not exception class — SDK versions vary) and re-raisesAbortErrorwhen true.finallyblock detaches the listener so long-lived controllers (the REPL engine's, reused across many turns) don't accumulate dead listeners.src/providers/minimax_provider.py+src/providers/openai_compatible.py— pre-call fast-path only. Mid-stream listener for these providers is a future PR (Minimax shares the anthropic SDK, so it should be a small lift).src/query/query.py:_call_model_syncgrowsabort_signalkeyword and forwards to the provider.except AbortError: raisein_call_model_syncprevents the cancel from routing through the substring-classification error-message gauntlet (also stops the misleading[DIAG] EXCEPTIONlog).except AbortError: passin the query loop falls through to the existingif params.abort_controller.signal.aborted:block — cancellation processing happens in one place.src/tool_system/agent_loop.py—_call_provider_for_turngrowscancel_signalkeyword and forwards asabort_signal=;run_agent_looppasses through. Same fix benefits the TUI path (which usesrun_agent_loopviaAgentBridge).tests/test_provider_abort_signal.py(new) — 5 regression tests:test_pre_aborted_signal_short_circuits_before_request—_ensure_clientis never called for a pre-tripped signaltest_mid_stream_abort_closes_stream_and_raises_abort_error— synthetic 500ms stream + mid-stream abort returns in <1s (vs the 20s symptom this fixes);stream.response.close()is calledtest_uncancelled_stream_returns_normally— no regression for never-tripped signalstest_no_abort_signal_param_preserves_legacy_callers—abort_signal=Nonedefault keeps SDK consumers workingtest_listener_detached_after_normal_completion— pins that the listener is removed after normal exitTest plan
_call_model_sync)Out of scope (deferred to follow-ups)
🤖 Generated with Claude Code