Skip to content

Commit 07d94ee

Browse files
committed
fix: late peer-cancel cannot double-respond; mcpserver completion gets typed params
JSONRPCDispatcher._handle_request: pop from _in_flight in the inner finally (right after the handler returns, before _write_result). No checkpoint between handler return and pop, so a late notifications/cancelled finds nothing and is a no-op; scope.cancel_called is only true if the cancel landed during the handler. Previously a cancel arriving during _write_result's checkpoint after the result was buffered would send both the result and a code=0 "Request cancelled" for the same id. mcpserver/server.py: register completion/complete via add_request_handler with CompleteRequestParams (was the legacy _add_request_handler which defaulted params_type=RequestParams, so the handler got base RequestParams and params.ref AttributeErrored). Delete _add_request_handler (last caller).
1 parent fa20352 commit 07d94ee

3 files changed

Lines changed: 14 additions & 24 deletions

File tree

‎src/mcp/server/lowlevel/server.py‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -272,15 +272,6 @@ def add_notification_handler(
272272
"""
273273
self._notification_handlers[method] = HandlerEntry(params_type, handler)
274274

275-
def _add_request_handler(
276-
self,
277-
method: str,
278-
handler: RequestHandler[LifespanResultT, Any],
279-
) -> None:
280-
# TODO: remove once experimental tasks plumbing and remaining callers
281-
# migrate to `add_request_handler` with an explicit params_type.
282-
self.add_request_handler(method, types.RequestParams, handler)
283-
284275
# --- ServerRegistry protocol (consumed by ServerRunner) ------------------
285276

286277
def get_request_handler(self, method: str) -> HandlerEntry[LifespanResultT] | None:

‎src/mcp/server/mcpserver/server.py‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -607,11 +607,7 @@ async def handler(
607607
completion=result if result is not None else Completion(values=[], total=None, has_more=None),
608608
)
609609

610-
# TODO(maxisbey): remove private access — completion needs post-construction
611-
# handler registration, find a better pattern for this
612-
self._lowlevel_server._add_request_handler( # pyright: ignore[reportPrivateUsage]
613-
"completion/complete", handler
614-
)
610+
self._lowlevel_server.add_request_handler("completion/complete", CompleteRequestParams, handler)
615611
return func
616612

617613
return decorator

‎src/mcp/shared/jsonrpc_dispatcher.py‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -565,20 +565,23 @@ async def _handle_request(
565565
try:
566566
result = await on_request(dctx, req.method, req.params)
567567
finally:
568-
# Close the back-channel the moment the handler exits
569-
# (success or raise), before the response write - a handler
570-
# spawning detached work that later calls
571-
# `dctx.send_raw_request()` should see `NoBackChannelError`.
568+
# Handler done: close the back-channel (detached work that
569+
# later calls `dctx.send_raw_request()` should see
570+
# `NoBackChannelError`) and drop from `_in_flight` so a
571+
# late `notifications/cancelled` is a no-op rather than
572+
# racing the result write below. No checkpoint between
573+
# handler return and the pop, so the cancel can't
574+
# interleave there.
572575
dctx.close()
576+
self._in_flight.pop(req.id, None)
573577
await self._write_result(req.id, result)
574578
if scope.cancel_called:
575-
# Peer-cancel: `_dispatch_notification` cancelled this scope.
576-
# anyio swallows a scope's *own* cancel at __exit__, so the
577-
# result write (or the handler) is interrupted and execution
578-
# lands here rather than the `except cancelled` arm below.
579+
# Peer-cancel: `_dispatch_notification` cancelled this scope
580+
# while the handler was running. anyio swallows a scope's *own*
581+
# cancel at __exit__, so execution lands here rather than the
582+
# `except cancelled` arm below.
579583
# TODO(maxisbey): spec says SHOULD NOT respond after cancel.
580-
# The existing server always has and the interaction suite pins
581-
# that; revisit once the suite's divergence entry is resolved.
584+
# The existing server always has, so match that for now.
582585
await self._write_error(req.id, ErrorData(code=0, message="Request cancelled"))
583586
except anyio.get_cancelled_exc_class():
584587
# Outer-cancel: run()'s task group is shutting down. Any bare

0 commit comments

Comments
 (0)