add tests to assert cancellable=False behaves as expected

Author: davemssavage

src/mcp/client/session.py

@@ -260,6 +260,7 @@ async def call_tool(
         name: str,
         arguments: dict[str, Any] | None = None,
         read_timeout_seconds: timedelta | None = None,
+        cancellable: bool = True,
     ) -> types.CallToolResult:
         """Send a tools/call request."""
 
@@ -272,6 +273,7 @@ async def call_tool(
             ),
             types.CallToolResult,
             request_read_timeout_seconds=read_timeout_seconds,
+            cancellable=cancellable,
         )
 
     async def list_prompts(self) -> types.ListPromptsResult:

src/mcp/shared/session.py

@@ -263,39 +263,40 @@ async def send_request(
             elif self._session_read_timeout_seconds is not None:
                 timeout = self._session_read_timeout_seconds.total_seconds()
 
-            try:
-                with anyio.fail_after(timeout) as scope:
-                    response_or_error = await response_stream_reader.receive()
-
-                    if cancellable and scope.cancel_called:
-                        with anyio.CancelScope(shield=True):
-                            notification = CancelledNotification(
-                                method="notifications/cancelled",
-                                params=CancelledNotificationParams(
-                                    requestId=request_id, reason="cancelled"
-                                ),
-                            )
-                            await self._send_notification_internal(
-                                notification, request_id
+            with anyio.CancelScope(shield=not cancellable):
+                try:
+                    with anyio.fail_after(timeout) as scope:
+                        response_or_error = await response_stream_reader.receive()
+
+                        if scope.cancel_called:
+                            with anyio.CancelScope(shield=True):
+                                notification = CancelledNotification(
+                                    method="notifications/cancelled",
+                                    params=CancelledNotificationParams(
+                                        requestId=request_id, reason="cancelled"
+                                    ),
+                                )
+                                await self._send_notification_internal(
+                                    notification, request_id
+                                )
+
+                            raise McpError(
+                                ErrorData(
+                                    code=REQUEST_CANCELLED, message="Request cancelled"
+                                )
                             )
 
-                        raise McpError(
-                            ErrorData(
-                                code=REQUEST_CANCELLED, message="Request cancelled"
-                            )
+                except TimeoutError:
+                    raise McpError(
+                        ErrorData(
+                            code=httpx.codes.REQUEST_TIMEOUT,
+                            message=(
+                                f"Timed out while waiting for response to "
+                                f"{request.__class__.__name__}. Waited "
+                                f"{timeout} seconds."
+                            ),
                         )
-
-            except TimeoutError:
-                raise McpError(
-                    ErrorData(
-                        code=httpx.codes.REQUEST_TIMEOUT,
-                        message=(
-                            f"Timed out while waiting for response to "
-                            f"{request.__class__.__name__}. Waited "
-                            f"{timeout} seconds."
-                        ),
                     )
-                )
 
             if isinstance(response_or_error, JSONRPCError):
                 raise McpError(response_or_error.error)

tests/shared/test_session.py

@@ -42,7 +42,6 @@ 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."""
-    # The tool is already registered in the fixture
 
     ev_tool_called = anyio.Event()
     ev_tool_cancelled = anyio.Event()
@@ -121,3 +120,68 @@ async def make_request(client_session: ClientSession):
             # Give cancellation time to process on server
             with anyio.fail_after(1):
                 await ev_tool_cancelled.wait()
+
+@pytest.mark.anyio
+async def test_request_cancellation_uncancellable():
+    """Test that asserts."""
+    # The tool is already registered in the fixture
+
+    ev_tool_called = anyio.Event()
+    ev_tool_commplete = anyio.Event()
+    ev_cancelled = anyio.Event()
+
+    # Start the request in a separate task so we can cancel it
+    def make_server() -> Server:
+        server = Server(name="TestSessionServer")
+
+        # Register the tool handler
+        @server.call_tool()
+        async def handle_call_tool(name: str, arguments: dict | None) -> list:
+            nonlocal ev_tool_called, ev_tool_commplete
+            if name == "slow_tool":
+                ev_tool_called.set()
+                with anyio.CancelScope():
+                    with anyio.fail_after(10): # Long enough to ensure we can cancel
+                        await ev_cancelled.wait()
+                    ev_tool_commplete.set()
+                    return []
+
+            raise ValueError(f"Unknown tool: {name}")
+
+        # Register the tool so it shows up in list_tools
+        @server.list_tools()
+        async def handle_list_tools() -> list[types.Tool]:
+            return [
+                types.Tool(
+                    name="slow_tool",
+                    description="A slow tool that takes 10 seconds to complete",
+                    inputSchema={},
+                )
+            ]
+
+        return server
+
+    async def make_request(client_session: ClientSession):
+        nonlocal ev_cancelled
+        try:
+            await client_session.call_tool("slow_tool", cancellable=False)
+        except McpError as e:
+            pytest.fail("Request should not have been cancelled")
+
+    async with create_connected_server_and_client_session(
+        make_server()
+    ) as client_session:
+        async with anyio.create_task_group() as tg:
+            tg.start_soon(make_request, client_session)
+
+            # Wait for the request to be in-flight
+            with anyio.fail_after(1):  # Timeout after 1 second
+                await ev_tool_called.wait()
+
+            # Cancel the task via task group
+            tg.cancel_scope.cancel()
+            ev_cancelled.set()
+
+            # Check server completed regardless
+            with anyio.fail_after(1):
+                await ev_tool_commplete.wait()