Skip to content

Commit 37682e1

Browse files
monody0007claude
andauthored
fix(connection): support $/cancel_request request cancellation (#158)
* fix(connection): honor $/cancel_request and reply -32800 to cancelled requests ACP v1 defines the `$/cancel_request` notification (part of the stable v1 schema vendored in schema/, 1.23.0; see https://agentclientprotocol.com/protocol/v1/cancellation), and the SDK already ships `PROTOCOL_METHODS["cancel_request"]`, but `Connection` never handled it: - An incoming `$/cancel_request` fell through to the router, which logged an ERROR traceback (method not found) while the targeted handler kept running and the peer never received the required response. - A handler that ended with `CancelledError` (cancelled from inside the agent) produced no response at all, leaving the peer waiting forever. - Cancelling the task awaiting `send_request` only dropped the local future; the peer kept working on the request. - `close()` hung forever when a handler answered its own cancellation with a result, because the response was queued on an already-closed sender. `Connection` now tracks in-flight incoming requests by id, cancels the handler task on `$/cancel_request`, answers cancelled requests with `-32800 Request cancelled` (a handler may still return a partial result), sends `$/cancel_request` when a still-pending outgoing request is cancelled locally, and skips replies once the connection is closed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(connection): keep request responses out of the handler's cancellation Two races could still lose the response to a cancelled request: - A `$/cancel_request` read in the same buffer as its request cancelled the request task before it first ran, so the coroutine that would have sent `-32800` never executed. - A late or repeated `$/cancel_request` arriving while the response was being sent cancelled that send, so the peer got no response at all. Each incoming request now runs its handler in its own task, which is the only thing `$/cancel_request` targets. A separate supervised task waits for the handler with `asyncio.wait` and sends its result, or `-32800` if the handler was cancelled. `close()` still cancels both, so shutdown never waits on a blocked send, and the `Task.uncancel()` / closed-send special cases are no longer needed. Also document request cancellation in the quickstart and state that a cancelled handler answers with its result or `-32800`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(connection): only cancel requests named by a valid requestId `$/cancel_request` requires a `requestId`, and a request id is `null`, an integer or a string. The handler did not check either, so a malformed cancellation could abort an unrelated request: - A notification without `params`, with non-object `params` or without `requestId` was treated as `requestId: null` and cancelled a pending request whose id is `null`. - `requestId: true` / `false` cancelled pending requests `1` / `0`, and `1.0` cancelled `1`, because Python equates these dict keys. Likewise a request sent with a non-integer id such as `true` or `1.0` could be cancelled by a cancellation for `1`. Cancellations without a `requestId`, or with one that is not a valid request id, are now ignored, and only requests with a valid id are tracked for cancellation. An explicit `requestId: null` still cancels a `null` request. --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 31f5f94 commit 37682e1

6 files changed

Lines changed: 626 additions & 17 deletions

File tree

‎docs/experimental-v2.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ Only the initial v2 request is reduced to the common v1 initialization fields
150150
when an agent selects v1.
151151

152152
Client-side fallback is application controlled and may require opening a new
153-
transport. Protocol-level request cancellation is not yet exposed by the
154-
experimental runtime; `session/cancel` remains available for cancelling active
155-
session work.
153+
transport. Protocol-level request cancellation (`$/cancel_request`) is handled
154+
by the shared connection layer, as in v1: cancelling the task awaiting a request
155+
sends a best-effort cancellation to the peer, and an incoming cancellation
156+
cancels the handler task, which answers with its result or `-32800`.
157+
`session/cancel` remains available for cancelling active session work.

‎docs/quickstart.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,21 @@ MCP requests return the inner JSON result unchanged, including `null`. Use
228228
and optional `params`. These methods share the same connections and routers
229229
across stdio, HTTP, and WebSocket transports.
230230

231+
## Request cancellation
232+
233+
Connections handle the protocol's `$/cancel_request` notification. When the
234+
peer cancels one of its requests, the SDK cancels the task running your handler.
235+
The handler may catch `asyncio.CancelledError` and return a (partial) result;
236+
otherwise the peer receives a `-32800` "Request cancelled" error. Cancellations
237+
for unknown or already finished requests are ignored; a cancelled request still
238+
gets exactly one response unless the connection is closed first.
239+
240+
Cancelling the task that awaits an outgoing request (for example through
241+
`asyncio.wait_for`) still raises `CancelledError` locally without waiting for the
242+
peer, and additionally sends a best-effort `$/cancel_request` for that request.
243+
Cancellation support is optional for peers. Use `session/cancel` to stop a
244+
prompt turn.
245+
231246
## Maintaining protocol routes
232247

233248
The `Agent` and `Client` protocols in `src/acp/interfaces.py` are the source of

‎src/acp/connection.py‎

Lines changed: 76 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
from ._transport import NdjsonTransport, Transport
1616
from .exceptions import RequestError
17+
from .meta import PROTOCOL_METHODS
1718
from .task import MessageSender, TaskSupervisor
1819
from .telemetry import span_context
1920

@@ -37,6 +38,13 @@ class StreamEvent:
3738

3839
StreamObserver = Callable[[StreamEvent], Awaitable[None] | None]
3940

41+
_CANCEL_REQUEST_METHOD = PROTOCOL_METHODS["cancel_request"]
42+
43+
44+
def _is_request_id(value: Any) -> bool:
45+
"""Whether ``value`` is a JSON-RPC ``RequestId``: ``null``, an integer or a string."""
46+
return value is None or isinstance(value, str) or (isinstance(value, int) and not isinstance(value, bool))
47+
4048

4149
class Connection:
4250
"""Minimal JSON-RPC 2.0 connection over newline-delimited JSON frames."""
@@ -54,6 +62,7 @@ def __init__(
5462
self._handler = handler
5563
self._next_request_id = 0
5664
self._pending: dict[int, asyncio.Future[Any]] = {}
65+
self._incoming: dict[Any, asyncio.Task[Any]] = {}
5766
self._tasks = TaskSupervisor(source="acp.Connection")
5867
self._tasks.add_error_handler(self._on_task_error)
5968
self._closed = False
@@ -117,16 +126,14 @@ async def send_request(self, method: str, params: JsonValue | None = None) -> An
117126
payload = {"jsonrpc": "2.0", "id": request_id, "method": method, "params": params}
118127
try:
119128
await self._transport.send(payload)
120-
except BaseException:
121-
self._pending.pop(request_id, None)
122-
future.cancel()
123-
raise
124-
self._notify_observers(StreamDirection.OUTGOING, payload)
125-
try:
129+
self._notify_observers(StreamDirection.OUTGOING, payload)
126130
return await future
127-
except asyncio.CancelledError:
128-
self._pending.pop(request_id, None)
131+
except BaseException as exc:
132+
still_pending = self._pending.pop(request_id, None) is not None
129133
future.cancel()
134+
if isinstance(exc, asyncio.CancelledError) and still_pending:
135+
# The request may already be on the wire; ask the peer to stop working on it.
136+
self._send_cancel_request(request_id)
130137
raise
131138

132139
async def send_notification(self, method: str, params: JsonValue | None = None) -> None:
@@ -152,13 +159,20 @@ async def _receive_loop(self) -> None:
152159
def _process_message(self, message: dict[str, Any]) -> None:
153160
method = message.get("method")
154161
has_id = "id" in message
162+
if method == _CANCEL_REQUEST_METHOD and not has_id:
163+
self._cancel_incoming(message.get("params"))
164+
return
155165
if method is not None: # this is a request or notification
156166
# {"jsonrpc": "2.0", "id": 1, "method": "foo", "params": {...}} # request
157167
# {"jsonrpc": "2.0", "method": "foo", "params: {...}} # notification
158-
self._tasks.create(
159-
self._run_request(message) if has_id else self._run_notification(message),
160-
name="acp.Connection.request" if has_id else "acp.Connection.notification",
161-
)
168+
if not has_id:
169+
self._tasks.create(self._run_notification(message), name="acp.Connection.notification")
170+
return
171+
# The handler gets its own task so ``$/cancel_request`` can cancel it, even before it
172+
# starts, without also cancelling delivery of the response the peer still expects.
173+
handler = self._tasks.create(self._execute_request(message), name="acp.Connection.request")
174+
self._track_incoming(message["id"], handler)
175+
self._tasks.create(self._run_request(message, handler), name="acp.Connection.response")
162176
return
163177
if has_id: # this is a response, {"id", "result" | "error"}
164178
self._handle_response(message)
@@ -184,8 +198,56 @@ def _notify_observers(self, direction: StreamDirection, message: dict[str, Any])
184198
def _on_observer_error(self, task: asyncio.Task[Any], exc: BaseException) -> None:
185199
logging.exception("Stream observer coroutine failed", exc_info=exc)
186200

187-
async def _run_request(self, message: dict[str, Any]) -> None:
188-
payload = await self._execute_request(message)
201+
def _track_incoming(self, request_id: Any, task: asyncio.Task[Any]) -> None:
202+
# A ``$/cancel_request`` can only name a valid request id, and Python would alias an invalid
203+
# one such as ``true`` or ``1.0`` with the integer ``1``.
204+
if not _is_request_id(request_id):
205+
return
206+
self._incoming[request_id] = task
207+
208+
def _forget(done: asyncio.Task[Any]) -> None:
209+
if self._incoming.get(request_id) is done:
210+
del self._incoming[request_id]
211+
212+
task.add_done_callback(_forget)
213+
214+
def _cancel_incoming(self, params: Any) -> None:
215+
# ``requestId`` is required and may be ``null``, so a missing one must not match a ``null`` id.
216+
if not isinstance(params, dict) or "requestId" not in params:
217+
return
218+
request_id = params["requestId"]
219+
if not _is_request_id(request_id):
220+
return
221+
# Unknown or already finished requests are ignored, as the protocol allows.
222+
task = self._incoming.get(request_id)
223+
if task is not None:
224+
task.cancel()
225+
226+
def _send_cancel_request(self, request_id: int) -> None:
227+
if self._closed or self._disconnected:
228+
return
229+
self._tasks.create(
230+
self.send_notification(_CANCEL_REQUEST_METHOD, {"requestId": request_id}),
231+
name="acp.Connection.cancel_request",
232+
on_error=self._on_cancel_request_error,
233+
)
234+
235+
def _on_cancel_request_error(self, task: asyncio.Task[Any], exc: BaseException) -> None:
236+
logging.debug("Failed to send %s", _CANCEL_REQUEST_METHOD, exc_info=exc)
237+
238+
async def _run_request(
239+
self, message: dict[str, Any], handler: asyncio.Future[dict[str, Any]] | None = None
240+
) -> None:
241+
if handler is None:
242+
handler = self._tasks.create(self._execute_request(message), name="acp.Connection.request")
243+
# Unlike ``await handler``, ``asyncio.wait`` does not re-raise the handler's cancellation, and
244+
# ``$/cancel_request`` only targets the handler, so only ``close()`` can abandon the response.
245+
await asyncio.wait({handler})
246+
if handler.cancelled():
247+
# Cancelled by ``$/cancel_request`` or from inside the handler.
248+
payload = {"jsonrpc": "2.0", "id": message["id"], "error": RequestError.request_cancelled().to_error_obj()}
249+
else:
250+
payload = handler.result()
189251
await self._transport.send(payload)
190252
self._notify_observers(StreamDirection.OUTGOING, payload)
191253

‎src/acp/exceptions.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,9 @@ def resource_not_found(cls, uri: str | None = None) -> RequestError:
4242
data = {"uri": uri} if uri is not None else None
4343
return cls(-32002, "Resource not found", data)
4444

45+
@classmethod
46+
def request_cancelled(cls, data: dict[str, Any] | None = None) -> RequestError:
47+
return cls(-32800, "Request cancelled", data)
48+
4549
def to_error_obj(self) -> dict[str, Any]:
4650
return {"code": self.code, "message": str(self), "data": self.data}

0 commit comments

Comments
 (0)