diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index 59de0ace4..3abc72b55 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -502,9 +502,9 @@ async def _handle_request( response = err.error except anyio.get_cancelled_exc_class(): if message.cancelled: - # Client sent CancelledNotification; responder.cancel() already - # sent an error response, so skip the duplicate. - logger.info("Request %s cancelled - duplicate response suppressed", message.request_id) + # Client sent CancelledNotification; per the MCP spec the + # receiver MUST NOT send a response for the cancelled request. + logger.info("Request %s cancelled - no response sent per spec", message.request_id) return # Transport-close cancellation from the TG in run(); re-raise so the # TG swallows its own cancellation. diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index 243eef5ae..c8663ef82 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -148,11 +148,8 @@ async def cancel(self) -> None: self._cancel_scope.cancel() self._completed = True # Mark as completed so it's removed from in_flight - # Send an error response to indicate cancellation - await self._session._send_response( # type: ignore[reportPrivateUsage] - request_id=self.request_id, - response=ErrorData(code=0, message="Request cancelled"), - ) + # Per the MCP spec, receivers of cancellation notifications SHOULD NOT + # send a response for the cancelled request. @property def in_flight(self) -> bool: # pragma: no cover diff --git a/tests/server/test_cancel_handling.py b/tests/server/test_cancel_handling.py index cff5a37c1..7d01fdf14 100644 --- a/tests/server/test_cancel_handling.py +++ b/tests/server/test_cancel_handling.py @@ -66,10 +66,11 @@ async def first_request(): await client.session.send_request( CallToolRequest(params=CallToolRequestParams(name="test_tool", arguments={})), CallToolResult, + request_read_timeout_seconds=3, ) pytest.fail("First request should have been cancelled") # pragma: no cover except MCPError: - pass # Expected + pass # Expected - request timed out since no response is sent for cancelled requests # Start first request async with anyio.create_task_group() as tg: diff --git a/tests/shared/test_session.py b/tests/shared/test_session.py index d7c6cc3b5..e953c0b0d 100644 --- a/tests/shared/test_session.py +++ b/tests/shared/test_session.py @@ -38,18 +38,27 @@ async def test_in_flight_requests_cleared_after_completion(): @pytest.mark.anyio async def test_request_cancellation(): - """Test that requests can be cancelled while in-flight.""" + """Test that requests can be cancelled while in-flight. + + Per the MCP cancellation spec, the server MUST NOT send a response for a + cancelled request. Instead, the server cancels the handler's cancel scope. + The client may time out waiting for a response that will never arrive. + """ ev_tool_called = anyio.Event() - ev_cancelled = anyio.Event() + ev_handler_cancelled = anyio.Event() request_id = None - # Create a server with a slow tool + # Create a server with a slow tool that detects cancellation async def handle_call_tool(ctx: ServerRequestContext, params: types.CallToolRequestParams) -> types.CallToolResult: - nonlocal request_id, ev_tool_called + nonlocal request_id, ev_tool_called, ev_handler_cancelled if params.name == "slow_tool": request_id = ctx.request_id ev_tool_called.set() - await anyio.sleep(10) # Long enough to ensure we can cancel + try: + await anyio.sleep(10) # Long enough to ensure we can cancel + except anyio.get_cancelled_exc_class(): + ev_handler_cancelled.set() + raise return types.CallToolResult(content=[]) # pragma: no cover raise ValueError(f"Unknown tool: {params.name}") # pragma: no cover @@ -64,27 +73,23 @@ async def handle_list_tools( on_list_tools=handle_list_tools, ) - async def make_request(client: Client): - nonlocal ev_cancelled - try: - await client.session.send_request( - types.CallToolRequest( - params=types.CallToolRequestParams(name="slow_tool", arguments={}), - ), - types.CallToolResult, - ) - pytest.fail("Request should have been cancelled") # pragma: no cover - except MCPError as e: - # Expected - request was cancelled - assert "Request cancelled" in str(e) - ev_cancelled.set() - async with Client(server) as client: - async with anyio.create_task_group() as tg: # pragma: no branch - tg.start_soon(make_request, client) + # Start the request in a task group we control + async with anyio.create_task_group() as tg: + + async def send_slow_request(): + with anyio.move_on_after(5): + await client.session.send_request( + types.CallToolRequest( + params=types.CallToolRequestParams(name="slow_tool", arguments={}), + ), + types.CallToolResult, + ) + + tg.start_soon(send_slow_request) # Wait for the request to be in-flight - with anyio.fail_after(1): # Timeout after 1 second + with anyio.fail_after(1): await ev_tool_called.wait() # Send cancellation notification @@ -93,9 +98,12 @@ async def make_request(client: Client): CancelledNotification(params=CancelledNotificationParams(request_id=request_id)) ) - # Give cancellation time to process - with anyio.fail_after(1): # pragma: no branch - await ev_cancelled.wait() + # Verify the server handler was cancelled + with anyio.fail_after(1): + await ev_handler_cancelled.wait() + + # Clean up: cancel the request task which is still waiting for a response + tg.cancel_scope.cancel() @pytest.mark.anyio