Merge remote-tracking branch 'upstream/main' into peter_contribution

peter-luminova · peter-luminova · commit ea209b17264e · 2026-02-18T16:34:26.000+05:30
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -31,6 +31,11 @@ This document contains critical information about working with this codebase. Fo
    - IMPORTANT: Before pushing, verify 100% branch coverage on changed files by running
      `uv run --frozen pytest -x` (coverage is configured in `pyproject.toml` with `fail_under = 100`
      and `branch = true`). If any branch is uncovered, add a test for it before pushing.
+   - Avoid `anyio.sleep()` with a fixed duration to wait for async operations. Instead:
+     - Use `anyio.Event` — set it in the callback/handler, `await event.wait()` in the test
+     - For stream messages, use `await stream.receive()` instead of `sleep()` + `receive_nowait()`
+     - Exception: `sleep()` is appropriate when testing time-based features (e.g., timeouts)
+   - Wrap indefinite waits (`event.wait()`, `stream.receive()`) in `anyio.fail_after(5)` to prevent hangs
 
 Test files mirror the source tree: `src/mcp/client/streamable_http.py` → `tests/client/test_streamable_http.py`
 Add tests to the existing file for that module.
diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py
@@ -493,12 +493,6 @@ async def _validate_resource_match(self, prm: ProtectedResourceMetadata) -> None
         if not prm_resource:
             return  # pragma: no cover
         default_resource = resource_url_from_server_url(self.context.server_url)
-        # Normalize: Pydantic AnyHttpUrl adds trailing slash to root URLs
-        # (e.g. "https://example.com/") while resource_url_from_server_url may not.
-        if not default_resource.endswith("/"):
-            default_resource += "/"
-        if not prm_resource.endswith("/"):
-            prm_resource += "/"
         if not check_resource_allowed(requested_resource=default_resource, configured_resource=prm_resource):
             raise OAuthFlowError(f"Protected resource {prm_resource} does not match expected {default_resource}")
 
diff --git a/src/mcp/client/sse.py b/src/mcp/client/sse.py
@@ -138,7 +138,7 @@ async def post_writer(endpoint_url: str):
                                         json=session_message.message.model_dump(
                                             by_alias=True,
                                             mode="json",
-                                            exclude_none=True,
+                                            exclude_unset=True,
                                         ),
                                     )
                                     response.raise_for_status()
diff --git a/src/mcp/client/stdio.py b/src/mcp/client/stdio.py
@@ -167,7 +167,7 @@ async def stdin_writer():
         try:
             async with write_stream_reader:
                 async for session_message in write_stream_reader:
-                    json = session_message.message.model_dump_json(by_alias=True, exclude_none=True)
+                    json = session_message.message.model_dump_json(by_alias=True, exclude_unset=True)
                     await process.stdin.send(
                         (json + "\n").encode(
                             encoding=server.encoding,
diff --git a/src/mcp/client/streamable_http.py b/src/mcp/client/streamable_http.py
@@ -260,7 +260,7 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:
         async with ctx.client.stream(
             "POST",
             self.url,
-            json=message.model_dump(by_alias=True, mode="json", exclude_none=True),
+            json=message.model_dump(by_alias=True, mode="json", exclude_unset=True),
             headers=headers,
         ) as response:
             if response.status_code == 202:
diff --git a/src/mcp/client/websocket.py b/src/mcp/client/websocket.py
@@ -65,7 +65,7 @@ async def ws_writer():
             async with write_stream_reader:
                 async for session_message in write_stream_reader:
                     # Convert to a dict, then to JSON
-                    msg_dict = session_message.message.model_dump(by_alias=True, mode="json", exclude_none=True)
+                    msg_dict = session_message.message.model_dump(by_alias=True, mode="json", exclude_unset=True)
                     await ws.send(json.dumps(msg_dict))
 
         async with anyio.create_task_group() as tg:
diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py
@@ -490,7 +490,7 @@ async def _handle_request(
             except Exception as err:
                 if raise_exceptions:  # pragma: no cover
                     raise err
-                response = types.ErrorData(code=0, message=str(err), data=None)
+                response = types.ErrorData(code=0, message=str(err))
 
             await message.respond(response)
         else:  # pragma: no cover
diff --git a/src/mcp/server/sse.py b/src/mcp/server/sse.py
@@ -170,7 +170,7 @@ async def sse_writer():
                     await sse_stream_writer.send(
                         {
                             "event": "message",
-                            "data": session_message.message.model_dump_json(by_alias=True, exclude_none=True),
+                            "data": session_message.message.model_dump_json(by_alias=True, exclude_unset=True),
                         }
                     )
 
diff --git a/src/mcp/server/stdio.py b/src/mcp/server/stdio.py
@@ -71,7 +71,7 @@ async def stdout_writer():
         try:
             async with write_stream_reader:
                 async for session_message in write_stream_reader:
-                    json = session_message.message.model_dump_json(by_alias=True, exclude_none=True)
+                    json = session_message.message.model_dump_json(by_alias=True, exclude_unset=True)
                     await stdout.write(json + "\n")
                     await stdout.flush()
         except (anyio.ClosedResourceError, anyio.BrokenResourceError):  # pragma: no cover
diff --git a/src/mcp/server/streamable_http.py b/src/mcp/server/streamable_http.py
@@ -169,6 +169,8 @@ def __init__(
         ] = {}
         self._sse_stream_writers: dict[RequestId, MemoryObjectSendStream[dict[str, str]]] = {}
         self._terminated = False
+        # Idle timeout cancel scope; managed by the session manager.
+        self.idle_scope: anyio.CancelScope | None = None
 
     @property
     def is_terminated(self) -> bool:
@@ -298,12 +300,12 @@ def _create_error_response(
         # Return a properly formatted JSON error response
         error_response = JSONRPCError(
             jsonrpc="2.0",
-            id="server-error",  # We don't have a request ID for general errors
+            id=None,
             error=ErrorData(code=error_code, message=error_message),
         )
 
         return Response(
-            error_response.model_dump_json(by_alias=True, exclude_none=True),
+            error_response.model_dump_json(by_alias=True, exclude_unset=True),
             status_code=status_code,
             headers=response_headers,
         )
@@ -323,7 +325,7 @@ def _create_json_response(
             response_headers[MCP_SESSION_ID_HEADER] = self.mcp_session_id
 
         return Response(
-            response_message.model_dump_json(by_alias=True, exclude_none=True) if response_message else None,
+            response_message.model_dump_json(by_alias=True, exclude_unset=True) if response_message else None,
             status_code=status_code,
             headers=response_headers,
         )
@@ -336,7 +338,7 @@ def _create_event_data(self, event_message: EventMessage) -> dict[str, str]:
         """Create event data dictionary from an EventMessage."""
         event_data = {
             "event": "message",
-            "data": event_message.message.model_dump_json(by_alias=True, exclude_none=True),
+            "data": event_message.message.model_dump_json(by_alias=True, exclude_unset=True),
         }
 
         # If an event ID was provided, include it
@@ -975,12 +977,11 @@ async def message_router():
                         # Determine which request stream(s) should receive this message
                         message = session_message.message
                         target_request_id = None
-                        # Check if this is a response
-                        if isinstance(message, JSONRPCResponse | JSONRPCError):
-                            response_id = str(message.id)
-                            # If this response is for an existing request stream,
-                            # send it there
-                            target_request_id = response_id
+                        # Check if this is a response with a known request id.
+                        # Null-id errors (e.g., parse errors) fall through to
+                        # the GET stream since they can't be correlated.
+                        if isinstance(message, JSONRPCResponse | JSONRPCError) and message.id is not None:
+                            target_request_id = str(message.id)
                         # Extract related_request_id from meta if it exists
                         elif (  # pragma: no cover
                             session_message.metadata is not None
diff --git a/src/mcp/server/streamable_http_manager.py b/src/mcp/server/streamable_http_manager.py
@@ -39,23 +39,28 @@ class StreamableHTTPSessionManager:
     2. Resumability via an optional event store
     3. Connection management and lifecycle
     4. Request handling and transport setup
+    5. Idle session cleanup via optional timeout
 
     Important: Only one StreamableHTTPSessionManager instance should be created
     per application. The instance cannot be reused after its run() context has
     completed. If you need to restart the manager, create a new instance.
 
     Args:
         app: The MCP server instance
-        event_store: Optional event store for resumability support.
-                     If provided, enables resumable connections where clients
-                     can reconnect and receive missed events.
-                     If None, sessions are still tracked but not resumable.
+        event_store: Optional event store for resumability support. If provided, enables resumable connections
+            where clients can reconnect and receive missed events. If None, sessions are still tracked but not
+            resumable.
         json_response: Whether to use JSON responses instead of SSE streams
-        stateless: If True, creates a completely fresh transport for each request
-                   with no session tracking or state persistence between requests.
+        stateless: If True, creates a completely fresh transport for each request with no session tracking or
+            state persistence between requests.
         security_settings: Optional transport security settings.
-        retry_interval: Retry interval in milliseconds to suggest to clients in SSE
-                       retry field. Used for SSE polling behavior.
+        retry_interval: Retry interval in milliseconds to suggest to clients in SSE retry field. Used for SSE
+            polling behavior.
+        session_idle_timeout: Optional idle timeout in seconds for stateful sessions. If set, sessions that
+            receive no HTTP requests for this duration will be automatically terminated and removed. When
+            retry_interval is also configured, ensure the idle timeout comfortably exceeds the retry interval to
+            avoid reaping sessions during normal SSE polling gaps. Default is None (no timeout). A value of 1800
+            (30 minutes) is recommended for most deployments.
     """
 
     def __init__(
@@ -66,13 +71,20 @@ def __init__(
         stateless: bool = False,
         security_settings: TransportSecuritySettings | None = None,
         retry_interval: int | None = None,
+        session_idle_timeout: float | None = None,
     ):
+        if session_idle_timeout is not None and session_idle_timeout <= 0:
+            raise ValueError("session_idle_timeout must be a positive number of seconds")
+        if stateless and session_idle_timeout is not None:
+            raise RuntimeError("session_idle_timeout is not supported in stateless mode")
+
         self.app = app
         self.event_store = event_store
         self.json_response = json_response
         self.stateless = stateless
         self.security_settings = security_settings
         self.retry_interval = retry_interval
+        self.session_idle_timeout = session_idle_timeout
 
         # Session tracking (only used if not stateless)
         self._session_creation_lock = anyio.Lock()
@@ -184,6 +196,9 @@ async def _handle_stateful_request(self, scope: Scope, receive: Receive, send: S
         if request_mcp_session_id is not None and request_mcp_session_id in self._server_instances:
             transport = self._server_instances[request_mcp_session_id]
             logger.debug("Session already exists, handling request directly")
+            # Push back idle deadline on activity
+            if transport.idle_scope is not None and self.session_idle_timeout is not None:
+                transport.idle_scope.deadline = anyio.current_time() + self.session_idle_timeout  # pragma: no cover
             await transport.handle_request(scope, receive, send)
             return
 
@@ -210,16 +225,31 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
                         read_stream, write_stream = streams
                         task_status.started()
                         try:
-                            await self.app.run(
-                                read_stream,
-                                write_stream,
-                                self.app.create_initialization_options(),
-                                stateless=False,  # Stateful mode
-                            )
+                            # Use a cancel scope for idle timeout — when the
+                            # deadline passes the scope cancels app.run() and
+                            # execution continues after the ``with`` block.
+                            # Incoming requests push the deadline forward.
+                            idle_scope = anyio.CancelScope()
+                            if self.session_idle_timeout is not None:
+                                idle_scope.deadline = anyio.current_time() + self.session_idle_timeout
+                                http_transport.idle_scope = idle_scope
+
+                            with idle_scope:
+                                await self.app.run(
+                                    read_stream,
+                                    write_stream,
+                                    self.app.create_initialization_options(),
+                                    stateless=False,
+                                )
+
+                            if idle_scope.cancelled_caught:
+                                assert http_transport.mcp_session_id is not None
+                                logger.info(f"Session {http_transport.mcp_session_id} idle timeout")
+                                self._server_instances.pop(http_transport.mcp_session_id, None)
+                                await http_transport.terminate()
                         except Exception:
                             logger.exception(f"Session {http_transport.mcp_session_id} crashed")
                         finally:
-                            # Only remove from instances if not terminated
                             if (  # pragma: no branch
                                 http_transport.mcp_session_id
                                 and http_transport.mcp_session_id in self._server_instances
@@ -244,11 +274,11 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
             # See: https://github.com/modelcontextprotocol/python-sdk/issues/1821
             error_response = JSONRPCError(
                 jsonrpc="2.0",
-                id="server-error",
+                id=None,
                 error=ErrorData(code=INVALID_REQUEST, message="Session not found"),
             )
             response = Response(
-                content=error_response.model_dump_json(by_alias=True, exclude_none=True),
+                content=error_response.model_dump_json(by_alias=True, exclude_unset=True),
                 status_code=HTTPStatus.NOT_FOUND,
                 media_type="application/json",
             )
diff --git a/src/mcp/server/websocket.py b/src/mcp/server/websocket.py
@@ -47,7 +47,7 @@ async def ws_writer():
         try:
             async with write_stream_reader:
                 async for session_message in write_stream_reader:
-                    obj = session_message.message.model_dump_json(by_alias=True, exclude_none=True)
+                    obj = session_message.message.model_dump_json(by_alias=True, exclude_unset=True)
                     await websocket.send_text(obj)
         except anyio.ClosedResourceError:
             await websocket.close()
diff --git a/src/mcp/shared/auth_utils.py b/src/mcp/shared/auth_utils.py
@@ -51,22 +51,17 @@ def check_resource_allowed(requested_resource: str, configured_resource: str) ->
     if requested.scheme.lower() != configured.scheme.lower() or requested.netloc.lower() != configured.netloc.lower():
         return False
 
-    # Handle cases like requested=/foo and configured=/foo/
+    # Normalize trailing slashes before comparison so that
+    # "/foo" and "/foo/" are treated as equivalent.
     requested_path = requested.path
     configured_path = configured.path
-
-    # If requested path is shorter, it cannot be a child
-    if len(requested_path) < len(configured_path):
-        return False
-
-    # Check if the requested path starts with the configured path
-    # Ensure both paths end with / for proper comparison
-    # This ensures that paths like "/api123" don't incorrectly match "/api"
     if not requested_path.endswith("/"):
         requested_path += "/"
     if not configured_path.endswith("/"):
         configured_path += "/"
 
+    # Check hierarchical match: requested must start with configured path.
+    # The trailing-slash normalization ensures "/api123/" won't match "/api/".
     return requested_path.startswith(configured_path)
 
 
diff --git a/src/mcp/shared/exceptions.py b/src/mcp/shared/exceptions.py
@@ -12,7 +12,10 @@ class MCPError(Exception):
 
     def __init__(self, code: int, message: str, data: Any = None):
         super().__init__(code, message, data)
-        self.error = ErrorData(code=code, message=message, data=data)
+        if data is not None:
+            self.error = ErrorData(code=code, message=message, data=data)
+        else:
+            self.error = ErrorData(code=code, message=message)
 
     @property
     def code(self) -> int:
diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py
@@ -142,7 +142,7 @@ async def cancel(self) -> None:
         # 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", data=None),
+            response=ErrorData(code=0, message="Request cancelled"),
         )
 
     @property
@@ -458,6 +458,12 @@ async def _handle_response(self, message: SessionMessage) -> None:
         if not isinstance(message.message, JSONRPCResponse | JSONRPCError):
             return  # pragma: no cover
 
+        if message.message.id is None:
+            # Narrows to JSONRPCError since JSONRPCResponse.id is always RequestId
+            error = message.message.error
+            logging.warning(f"Received error with null ID: {error.message}")
+            await self._handle_incoming(MCPError(error.code, error.message, error.data))
+            return
         # Normalize response ID to handle type mismatches (e.g., "0" vs 0)
         response_id = self._normalize_request_id(message.message.id)
 
diff --git a/src/mcp/types/jsonrpc.py b/src/mcp/types/jsonrpc.py
@@ -75,7 +75,7 @@ class JSONRPCError(BaseModel):
     """A response to a request that indicates an error occurred."""
 
     jsonrpc: Literal["2.0"]
-    id: RequestId
+    id: RequestId | None
     error: ErrorData
 
 
diff --git a/tests/server/test_streamable_http_manager.py b/tests/server/test_streamable_http_manager.py
diff --git a/tests/shared/test_auth_utils.py b/tests/shared/test_auth_utils.py
diff --git a/tests/shared/test_session.py b/tests/shared/test_session.py
diff --git a/tests/shared/test_sse.py b/tests/shared/test_sse.py
diff --git a/tests/shared/test_ws.py b/tests/shared/test_ws.py

Original file line number	Diff line number	Diff line change
`@@ -138,7 +138,7 @@ async def post_writer(endpoint_url: str):`
`138`	`138`	`json=session_message.message.model_dump(`
`139`	`139`	`by_alias=True,`
`140`	`140`	`mode="json",`
`141`		`- exclude_none=True,`
	`141`	`+ exclude_unset=True,`
`142`	`142`	`),`
`143`	`143`	`)`
`144`	`144`	`response.raise_for_status()`
Original file line number	Diff line number	Diff line change
`@@ -170,7 +170,7 @@ async def sse_writer():`
`170`	`170`	`await sse_stream_writer.send(`
`171`	`171`	`{`
`172`	`172`	`"event": "message",`
`173`		`- "data": session_message.message.model_dump_json(by_alias=True, exclude_none=True),`
	`173`	`+ "data": session_message.message.model_dump_json(by_alias=True, exclude_unset=True),`
`174`	`174`	`}`
`175`	`175`	`)`
`176`	`176`