diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index fb0c623473a..275071180e5 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -6923,25 +6923,33 @@ class MCPServerManager: if not should_skip_health_check: try: - resolved_static_headers: Final = await self._resolve_static_headers_with_env_vars( - server=server, - user_api_key_auth=None, - raise_on_missing=False, - ) - extra_headers: Final = dict(resolved_static_headers) if resolved_static_headers else {} - client: Final = await self._create_mcp_client( - server=server, - mcp_auth_header=None, - extra_headers=extra_headers, - stdio_env=None, - ) + if server.spec_path: + # url is a REST base, not an MCP endpoint, so the spec is the only thing to probe + from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + load_openapi_spec_async, + ) - async def _noop(session): - return "ok" + await asyncio.wait_for(load_openapi_spec_async(server.spec_path), timeout=MCP_HEALTH_CHECK_TIMEOUT) + else: + resolved_static_headers: Final = await self._resolve_static_headers_with_env_vars( + server=server, + user_api_key_auth=None, + raise_on_missing=False, + ) + extra_headers: Final = dict(resolved_static_headers) if resolved_static_headers else {} + client: Final = await self._create_mcp_client( + server=server, + mcp_auth_header=None, + extra_headers=extra_headers, + stdio_env=None, + ) - # Add timeout wrapper to prevent hanging - await asyncio.wait_for(client.run_with_session(_noop), timeout=MCP_HEALTH_CHECK_TIMEOUT) - self._remember_upstream_initialize_instructions(server, client) + async def _noop(session): + return "ok" + + # Add timeout wrapper to prevent hanging + await asyncio.wait_for(client.run_with_session(_noop), timeout=MCP_HEALTH_CHECK_TIMEOUT) + self._remember_upstream_initialize_instructions(server, client) status = "healthy" except asyncio.TimeoutError: health_check_error = f"Health check timed out after {MCP_HEALTH_CHECK_TIMEOUT} seconds" diff --git a/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py b/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py index d115eb8b3c1..c60004127e2 100644 --- a/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py +++ b/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py @@ -174,8 +174,10 @@ async def load_openapi_spec_async(filepath: str, *, max_bytes: int | None = None r.raise_for_status() return r.json() - # fallback: local file - # Local filesystem path + return await asyncio.to_thread(_read_local_openapi_spec, filepath) + + +def _read_local_openapi_spec(filepath: str) -> dict[str, Any]: if not os.path.exists(filepath): raise FileNotFoundError(f"OpenAPI spec not found at {filepath}") with open(filepath, "r", encoding="utf-8") as f: diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index d56f08c4e79..986ff72e111 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -4728,6 +4728,75 @@ class TestMCPServerManager: assert result.status == "unhealthy" assert "OAuth discovery unavailable" in (result.health_check_error or "") + @pytest.mark.asyncio + async def test_health_check_server_openapi_spec_is_probed_instead_of_mcp_session(self, tmp_path): + """An OpenAPI server is healthy when its spec loads, and is never asked to speak MCP. + + Regression for #40079: these servers store the REST base url in ``url`` and only look like + http transport, so opening an MCP session against them always failed with "Session + terminated" even though every tool had loaded from the spec. ``url`` is a closed port, so + an MCP handshake would report unhealthy. + """ + spec = tmp_path / "openapi.json" + spec.write_text(json.dumps({"openapi": "3.0.0", "paths": {}})) + manager = MCPServerManager() + server = MCPServer( + server_id="openapi-server", + name="openapi-server", + transport=MCPTransport.http, + auth_type=MCPAuth.none, + url="http://127.0.0.1:1", + spec_path=str(spec), + ) + manager.config_mcp_servers[server.server_id] = server + + result = await manager.health_check_server("openapi-server") + + assert result.status == "healthy" + assert result.health_check_error is None + + @pytest.mark.asyncio + async def test_health_check_server_openapi_spec_failure_is_unhealthy(self, tmp_path): + """A spec that stopped loading is unhealthy, and the upstream error is what gets reported.""" + missing = tmp_path / "openapi.json" + manager = MCPServerManager() + server = MCPServer( + server_id="openapi-server", + name="openapi-server", + transport=MCPTransport.http, + auth_type=MCPAuth.none, + url="http://127.0.0.1:1", + spec_path=str(missing), + ) + manager.config_mcp_servers[server.server_id] = server + + result = await manager.health_check_server("openapi-server") + + assert result.status == "unhealthy" + assert result.health_check_error == f"OpenAPI spec not found at {missing}" + + @pytest.mark.asyncio + async def test_health_check_server_openapi_spec_keeps_per_user_auth_skip(self, tmp_path): + """Per-user auth still wins: fetching the spec userless could 401 and report a false failure. + + The spec file is absent, so probing it would have reported unhealthy rather than unknown. + """ + manager = MCPServerManager() + server = MCPServer( + server_id="openapi-server", + name="openapi-server", + transport=MCPTransport.http, + auth_type=MCPAuth.bearer_token, + url="http://127.0.0.1:1", + spec_path=str(tmp_path / "openapi.json"), + ) + manager.config_mcp_servers[server.server_id] = server + + result = await manager.health_check_server("openapi-server") + + assert result.status == "unknown" + assert result.health_check_error is None + @pytest.mark.asyncio async def test_health_check_server_not_found(self): """Test health check for a server that doesn't exist"""