Review ACP cancellation token implementation #507
Merged
Conversation
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
…ation - Replace custom CancellationError with asyncio.CancelledError for idiomatic async - Remove ExtendedAgentSideConnection class (fix non-compliant clients instead) - Implement task-based cancellation via asyncio.Task.cancel() in ACP server - Track asyncio tasks per session for proper cancellation support - Update all LLM providers (Anthropic, OpenAI, Google, Bedrock) to use asyncio.CancelledError - Add proper CancelledError handling in ACP prompt method This aligns with Python asyncio best practices and provides more immediate cancellation when LLM providers are blocked on network calls.
Following review recommendations, this removes the now-redundant CancellationToken class entirely. Cancellation is now handled purely through asyncio.Task.cancel(), which raises CancelledError at the next await point. Changes: - Remove CancellationToken class from cancellation.py (kept module for docs) - Remove cancellation_token parameter from all LLM providers and generate methods - Remove token polling checks from streaming loops (task.cancel() handles it) - Update ACP server to use only task-based cancellation - Update all agents to remove cancellation_token parameters - Update tests to remove CancellationToken usage This simplifies the cancellation architecture by relying on Python's native asyncio patterns. The _session_tasks dict in ACP server is sufficient for cancellation - no separate token tracking needed.
evalstate
deleted the
claude/review-acp-cancellation-016pZUtKGNYygvM657fvbFvC
branch
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.
…ation
This aligns with Python asyncio best practices and provides more immediate cancellation when LLM providers are blocked on network calls.