diff --git a/src/devhelm_mcp/server.py b/src/devhelm_mcp/server.py index 0a4f28d..87a1cea 100644 --- a/src/devhelm_mcp/server.py +++ b/src/devhelm_mcp/server.py @@ -11,6 +11,7 @@ import argparse import asyncio +import contextlib import os import sys from importlib.metadata import PackageNotFoundError @@ -102,7 +103,7 @@ def _package_version() -> str: mod.register(mcp) -def _strip_internal_schema_fields() -> None: +async def _strip_internal_schema_fields() -> None: """Hide server-controlled / sensitive fields from the LLM-facing schema. Two targets: @@ -127,8 +128,15 @@ def _strip_internal_schema_fields() -> None: ``run_middleware=False`` returns the source-of-truth ``Tool`` objects from the providers; the dereference middleware copies them on every ``tools/list`` and would lose any edits we made to the copies. + + This function is async because ``mcp.list_tools()`` is async-only in + FastMCP. It must be awaited from inside the running event loop — + calling it via ``asyncio.run()`` at import time crashed Uvicorn + workers (``RuntimeError: asyncio.run() cannot be called from a + running event loop``) because Uvicorn imports user code from inside + its own ``asyncio_run(self.serve(...))`` call. v0.7.2 hotfix. """ - tools = asyncio.run(mcp.list_tools(run_middleware=False)) + tools = await mcp.list_tools(run_middleware=False) for tool in tools: params = tool.parameters if not isinstance(params, dict): @@ -179,9 +187,6 @@ def _strip_field_from_object_schema(schema: dict[str, Any], field: str) -> None: schema_required.remove(field) -_strip_internal_schema_fields() - - class _NormalizeMcpPath: """Rewrite ``/mcp`` → ``/mcp/`` (and ``/{key}/mcp`` → ``/{key}/mcp/``). @@ -261,6 +266,17 @@ async def health_handler(request: Request) -> JSONResponse: # ``StreamableHTTPSessionManager`` — without it the inner app raises # "task group was not initialized" on the first POST. See # https://gofastmcp.com/deployment/asgi. + # + # We wrap that lifespan so the input-schema strip runs inside the + # running event loop *before* the inner app starts handling requests. + # Doing the strip at module import time crashed Uvicorn workers under + # Python 3.13 (``asyncio.run()`` from a running loop). v0.7.2 hotfix. + @contextlib.asynccontextmanager + async def _lifespan(app: Starlette) -> Any: + await _strip_internal_schema_fields() + async with mcp_app.lifespan(app): + yield + starlette_app = Starlette( routes=[ Route("/health", health_handler, methods=["GET"]), @@ -268,7 +284,7 @@ async def health_handler(request: Request) -> JSONResponse: Mount("/{api_key}/mcp", app=mcp_app), ], middleware=middleware, - lifespan=mcp_app.lifespan, + lifespan=_lifespan, ) # Wrap with the path normalizer so POST /mcp (no trailing slash) reaches # the inner app directly instead of bouncing through a 307. The @@ -393,6 +409,12 @@ def _run_stdio() -> None: (which would also swallow real warnings / tracebacks the user needs to see). """ + # Run the schema strip synchronously here — at this point we're still + # outside any event loop (FastMCP's ``mcp.run()`` will create its own + # below), so ``asyncio.run()`` is safe. For HTTP mode the equivalent + # call lives in the Starlette lifespan, see ``_get_app``. + asyncio.run(_strip_internal_schema_fields()) + extra: dict[str, Any] = {} try: mcp.run(show_banner=False, **extra) diff --git a/tests/test_devex_round3_fixes.py b/tests/test_devex_round3_fixes.py index b8a9eb1..495a9c9 100644 --- a/tests/test_devex_round3_fixes.py +++ b/tests/test_devex_round3_fixes.py @@ -25,13 +25,18 @@ from fastmcp import Client from starlette.testclient import TestClient -from devhelm_mcp.server import _package_version, app, mcp +from devhelm_mcp.server import _package_version, _strip_internal_schema_fields, app, mcp RegisteredTools = dict[str, Any] @pytest.fixture(scope="module") def registered_tools() -> RegisteredTools: + # The schema strip now runs only inside the Starlette lifespan (HTTP) + # or before ``mcp.run()`` (stdio) — see v0.7.2 hotfix. Tests that + # inspect the registered tool schemas have to perform the strip + # explicitly, otherwise they see the raw FastMCP-generated schema. + asyncio.run(_strip_internal_schema_fields()) tools = asyncio.run(mcp.list_tools()) return {t.name: t for t in tools} @@ -398,3 +403,48 @@ def test_health_endpoint_unaffected(self) -> None: resp = client.get("/health") assert resp.status_code == 200 assert resp.json()["status"] == "healthy" + + +# --------------------------------------------------------------------------- # +# v0.7.2 hotfix — module import + lifespan must work from a running loop +# --------------------------------------------------------------------------- # + + +class TestImportFromRunningLoopDoesNotCrash: + """v0.7.1 regressed by calling ``asyncio.run()`` at module import time. + + Uvicorn imports the user app from inside ``asyncio.run(self.serve(...))``, + so by the time ``devhelm_mcp.server`` ran its top-level + ``_strip_internal_schema_fields()`` it was already inside a running + event loop and Python raised ``RuntimeError: asyncio.run() cannot be + called from a running event loop``. Cluster pods crashed in a tight + restart loop until the deployment was reverted. + + The fix moved the strip into the Starlette lifespan (HTTP) and into + ``_run_stdio()`` (stdio). This test pins both halves: importing the + module from inside a running loop must succeed, and the lifespan + must perform the strip on entry. + """ + + def test_lifespan_entry_strips_internal_fields(self) -> None: + async def go() -> None: + # Re-import inside the running loop — this is what Uvicorn does. + import importlib + + import devhelm_mcp.server as srv + + srv = importlib.reload(srv) + + async with srv.app.router.lifespan_context(srv.app): + tools = await srv.mcp.list_tools(run_middleware=False) + api_token_leaks = [ + t.name + for t in tools + if isinstance(t.parameters, dict) + and "api_token" in (t.parameters.get("properties") or {}) + ] + assert not api_token_leaks, ( + f"lifespan must strip api_token from {api_token_leaks!r}" + ) + + asyncio.run(go()) diff --git a/uv.lock b/uv.lock index 02c3209..825c837 100644 --- a/uv.lock +++ b/uv.lock @@ -401,7 +401,7 @@ wheels = [ [[package]] name = "devhelm-mcp-server" -version = "0.7.0" +version = "0.7.1" source = { editable = "." } dependencies = [ { name = "devhelm" },