From f99956c05ef8f3a8534977aeac6d08b480821b0f Mon Sep 17 00:00:00 2001 From: K4bain <296577378+K4bain@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:24:35 +0500 Subject: [PATCH] fix(server): respect include_router prefix when mounting MCP endpoints mount_http()/mount_sse() ignored the prefix a router was included with via app.include_router(router, prefix=...), mounting MCP endpoints at the app root instead (#204). Resolve the include prefix from FastAPI's own include-time bookkeeping (>= 0.137 _IncludedRouter metadata) with a path-diffing fallback for older FastAPI; ambiguous or unresolvable prefixes fall back to the safe empty prefix instead of guessing. Adopted from #329 (original patch by @wakqasahmed). --- fastapi_mcp/server.py | 135 +++++++++++++++++++++++++- tests/test_basic_functionality.py | 155 +++++++++++++++++++++++++++++- 2 files changed, 284 insertions(+), 6 deletions(-) diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index bb751067..28f7ce87 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -309,6 +309,117 @@ def _setup_auth(self): else: logger.info("No auth config provided, skipping auth setup") + def _resolve_include_prefix_from_metadata(self, router: APIRouter) -> List[str]: + """ + Resolve the `include_router(router, prefix=...)` prefix using FastAPI's own + include-time bookkeeping (FastAPI >= 0.137, Starlette's `_IncludedRouter`). + + Since FastAPI 0.137.0, `app.include_router()` no longer flattens a sub-router's + routes into `app.routes`; instead it appends one `_IncludedRouter` object per + `include_router()` call that carries `.original_router` (the router instance + that was included) and `.include_context.prefix` (the prefix it was included + under). Matching on `.original_router is router` identity is exact -- unlike + diffing paths, it works even when `router` has zero routes of its own, and it + cannot be confused by two different routers sharing the same endpoint function. + + Returns one prefix string per matching inclusion (usually zero or one; more + than one means `router` was `include_router`-ed more than once under different + prefixes). Returns an empty list on FastAPI < 0.137, where `app.routes` has no + such per-inclusion record at all. + """ + prefixes = [] + for app_route in self.fastapi.routes: + if getattr(app_route, "original_router", None) is not router: + continue + include_context = getattr(app_route, "include_context", None) + prefix = getattr(include_context, "prefix", None) + if prefix is not None: + prefixes.append(prefix) + return prefixes + + def _resolve_include_prefix_by_diff(self, router: APIRouter) -> List[str]: + """ + Resolve the `include_router(router, prefix=...)` prefix by diffing `router`'s + own routes against their counterparts on `self.fastapi.routes` (same endpoint + function, same HTTP methods, path ending in the router's own route path). + + Fallback for FastAPI < 0.137, where `app.include_router()` flattens sub-routes + directly into `app.routes` and there is no other way to recover the prefix. + + Requires *every* route defined on `router` to agree on the same candidate + prefix before accepting it -- an endpoint function reused across two different + routers (or a router included more than once under different prefixes) can + otherwise make an unrelated prefix look like a valid match. Returns a list of + the distinct candidate prefixes found: empty if none of `router`'s routes could + be matched at all, or more than one entry if there is a genuine ambiguity. + """ + own_routes = [r for r in router.routes if getattr(r, "path", None) and getattr(r, "endpoint", None) is not None] + if not own_routes: + return [] + + per_route_candidates = [] + for router_route in own_routes: + router_path: str = getattr(router_route, "path") + router_endpoint = getattr(router_route, "endpoint") + router_methods = getattr(router_route, "methods", None) + matches = set() + for app_route in self.fastapi.routes: + app_path = getattr(app_route, "path", None) + if not app_path or not app_path.endswith(router_path): + continue + if getattr(app_route, "endpoint", None) is not router_endpoint: + continue + if router_methods is not None and getattr(app_route, "methods", None) != router_methods: + continue + matches.add(app_path[: -len(router_path)]) + if not matches: + return [] + per_route_candidates.append(matches) + + return sorted(set.intersection(*per_route_candidates)) + + def _resolve_router_include_prefix(self, router: APIRouter) -> str: + """ + Resolve the prefix under which `router` was actually mounted onto `self.fastapi`. + + When a router is included via `app.include_router(router, prefix="/x")` without + setting `prefix` on the `APIRouter()` constructor itself, that prefix is only + reflected on `self.fastapi`'s route table -- the router object's own `.prefix` + attribute stays empty. Tries the exact, identity-based resolution first + (FastAPI >= 0.137) and falls back to diffing route paths (FastAPI < 0.137). + + Returns an empty string, and logs a warning, if no prefix could be resolved + unambiguously -- e.g. the router hasn't been included into the app yet, or (on + FastAPI < 0.137 only) it has no routes of its own to diff against. This + preserves the previous behavior of relying solely on `router.prefix` rather + than risk mounting under the wrong prefix. + """ + prefixes = self._resolve_include_prefix_from_metadata(router) + if not prefixes: + prefixes = self._resolve_include_prefix_by_diff(router) + + if not prefixes: + logger.warning( + f"Could not determine the prefix that {router!r} was included under. " + "Mounting at the router's own prefix; if this router was included via " + "`app.include_router(router, prefix=...)`, make sure `include_router` is " + "called before `mount_http`/`mount_sse`, or set `prefix` on the " + "`APIRouter()` constructor directly." + ) + return "" + + unique_prefixes = sorted(set(prefixes)) + if len(unique_prefixes) > 1: + logger.warning( + f"{router!r} appears to be included in the app under multiple different " + f"prefixes {unique_prefixes}. Could not unambiguously resolve where its " + "MCP endpoint should be mounted, so falling back to the router's own " + "prefix rather than guessing." + ) + return "" + + return unique_prefixes[0] + def mount_http( self, router: Annotated[ @@ -336,6 +447,11 @@ def mount_http( There is no requirement that the FastAPI app or APIRouter is the same as the one that the MCP server was created from. + + If `router` is an `APIRouter`, it must already be included into the app (via + `app.include_router(router, ...)`) *before* calling this method -- the effective + mount path is resolved from the app's current route table at call time and is + not re-evaluated if the router is included afterwards. """ # Normalize mount path if not mount_path.startswith("/"): @@ -348,6 +464,9 @@ def mount_http( assert isinstance(router, (FastAPI, APIRouter)), f"Invalid router type: {type(router)}" + include_prefix = self._resolve_router_include_prefix(router) if isinstance(router, APIRouter) else "" + full_mount_path = mount_path if isinstance(router, FastAPI) else include_prefix + router.prefix + mount_path + http_transport = FastApiHttpSessionManager(mcp_server=self.server) dependencies = self._auth_config.dependencies if self._auth_config else None @@ -361,9 +480,9 @@ def mount_http( # # TODO: Find a better way to do this. if isinstance(router, APIRouter): - self.fastapi.include_router(router) + self.fastapi.include_router(router, prefix=include_prefix) - logger.info(f"MCP HTTP server listening at {mount_path}") + logger.info(f"MCP HTTP server listening at {full_mount_path}") def mount_sse( self, @@ -392,6 +511,11 @@ def mount_sse( There is no requirement that the FastAPI app or APIRouter is the same as the one that the MCP server was created from. + + If `router` is an `APIRouter`, it must already be included into the app (via + `app.include_router(router, ...)`) *before* calling this method -- the effective + mount path is resolved from the app's current route table at call time and is + not re-evaluated if the router is included afterwards. """ # Normalize mount path if not mount_path.startswith("/"): @@ -404,7 +528,8 @@ def mount_sse( # Build the base path correctly for the SSE transport assert isinstance(router, (FastAPI, APIRouter)), f"Invalid router type: {type(router)}" - base_path = mount_path if isinstance(router, FastAPI) else router.prefix + mount_path + include_prefix = self._resolve_router_include_prefix(router) if isinstance(router, APIRouter) else "" + base_path = mount_path if isinstance(router, FastAPI) else include_prefix + router.prefix + mount_path messages_path = f"{base_path}/messages/" sse_transport = FastApiSseTransport(messages_path) @@ -419,9 +544,9 @@ def mount_sse( # # TODO: Find a better way to do this. if isinstance(router, APIRouter): - self.fastapi.include_router(router) + self.fastapi.include_router(router, prefix=include_prefix) - logger.info(f"MCP SSE server listening at {mount_path}") + logger.info(f"MCP SSE server listening at {base_path}") def mount( self, diff --git a/tests/test_basic_functionality.py b/tests/test_basic_functionality.py index 3557226e..8820351c 100644 --- a/tests/test_basic_functionality.py +++ b/tests/test_basic_functionality.py @@ -1,8 +1,49 @@ -from fastapi import FastAPI +from importlib.metadata import version as pkg_version + +from fastapi import APIRouter, FastAPI +from fastapi.testclient import TestClient from mcp.server.lowlevel.server import Server +from packaging.version import Version from fastapi_mcp import FastApiMCP +# FastAPI >= 0.137.0 stopped flattening `include_router()`'d sub-routes into +# `app.routes`; it now records each inclusion as a `_IncludedRouter` object carrying +# `.original_router` / `.include_context.prefix` instead. `_resolve_router_include_prefix` +# uses that metadata when available (see fastapi_mcp/server.py), which is strictly more +# capable than the path-diffing fallback used on older FastAPI: it can resolve a router's +# prefix even when the router has no routes of its own, and it can't be confused by two +# routers sharing the same endpoint function. A few of the tests below only hold on one +# side of that boundary; they say so explicitly rather than silently skip. +FASTAPI_HAS_INCLUDED_ROUTER_METADATA = Version(pkg_version("fastapi")) >= Version("0.137.0") + + +def _mcp_endpoint_exists(app: FastAPI, path: str) -> bool: + """ + Whether an MCP streamable-HTTP endpoint is actually reachable at `path`. + + Deliberately goes through a real request/response cycle instead of inspecting + `app.routes` -- FastAPI's internal route-table representation is not a stable + contract (see FASTAPI_HAS_INCLUDED_ROUTER_METADATA above), so asserting on it + directly is what let the fastapi>=0.137 regression slip through review previously. + """ + client = TestClient(app) + response = client.post( + path, + json={ + "jsonrpc": "2.0", + "method": "initialize", + "id": 1, + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "test-client", "version": "1.0.0"}, + }, + }, + headers={"Accept": "application/json, text/event-stream", "Content-Type": "application/json"}, + ) + return response.status_code != 404 + def test_create_mcp_server(simple_fastapi_app: FastAPI): """Test creating an MCP server without mounting it.""" @@ -64,3 +105,115 @@ def test_normalize_paths(simple_fastapi_app: FastAPI): # Check that the route was added with a normalized path route_found = any("/test-mcp2" in str(route) for route in simple_fastapi_app.routes) assert route_found, "Normalized mount path not found in app routes" + + +def test_mount_router_with_include_router_prefix(simple_fastapi_app: FastAPI): + """ + Regression test for https://github.com/tadata-org/fastapi_mcp/issues/204. + + When an APIRouter is created without a `prefix` and is instead given one via + `app.include_router(router, prefix=...)`, the MCP server should still be mounted + at the router's effective path, not at the app root. + """ + other_router = APIRouter() # No prefix on the constructor + + @other_router.get("/ping", operation_id="ping") + async def ping() -> dict: + return {"ping": "pong"} + + simple_fastapi_app.include_router(other_router, prefix="/other/route") + + mcp = FastApiMCP(simple_fastapi_app) + mcp.mount_http(other_router) + + assert _mcp_endpoint_exists(simple_fastapi_app, "/other/route/mcp"), "Expected MCP endpoint not reachable" + assert not _mcp_endpoint_exists( + simple_fastapi_app, "/mcp" + ), "MCP endpoint should not be mounted at the app root" + + +def test_mount_empty_router_with_include_router_prefix(simple_fastapi_app: FastAPI): + """ + The literal repro from https://github.com/tadata-org/fastapi_mcp/issues/204: a router + used purely as a mount point, with zero routes of its own. + + On FastAPI >= 0.137, `_resolve_router_include_prefix` can recover the prefix from + FastAPI's own include-time metadata (`_IncludedRouter`) even though there is nothing + to diff paths against, so this now resolves correctly. On FastAPI < 0.137 there is no + such metadata and no routes to diff, so there is genuinely no information available to + recover the prefix from -- this is a documented limitation, not a bug in the fix. + """ + empty_router = APIRouter() # zero routes: a pure mount point, as in the issue + + simple_fastapi_app.include_router(empty_router, prefix="/empty") + + mcp = FastApiMCP(simple_fastapi_app) + mcp.mount_http(empty_router) + + if FASTAPI_HAS_INCLUDED_ROUTER_METADATA: + assert _mcp_endpoint_exists(simple_fastapi_app, "/empty/mcp"), "Expected MCP endpoint not reachable" + assert not _mcp_endpoint_exists( + simple_fastapi_app, "/mcp" + ), "MCP endpoint should not be mounted at the app root" + else: + assert _mcp_endpoint_exists(simple_fastapi_app, "/mcp"), ( + "Known limitation on fastapi < 0.137: a router with no routes of its own " + "carries no recoverable prefix information, so mounting safely falls back " + "to the app root instead of guessing." + ) + + +def test_resolve_prefix_with_endpoint_shared_across_routers(simple_fastapi_app: FastAPI): + """ + Regression test: reusing the same endpoint function on two different routers is + legitimate FastAPI, but `endpoint is router_endpoint` identity alone can't tell the + two registrations apart. Picking the wrong router's prefix is worse than the + pre-fix behavior (mounting a router's MCP endpoint under an unrelated router's + prefix), so this must never happen -- either resolve correctly, or fall back safely. + """ + + async def ping() -> dict: + return {"ping": "pong"} + + router_a = APIRouter() + router_a.add_api_route("/ping", ping, methods=["GET"]) + router_b = APIRouter() + router_b.add_api_route("/ping", ping, methods=["GET"]) + + simple_fastapi_app.include_router(router_a, prefix="/alpha") + simple_fastapi_app.include_router(router_b, prefix="/beta") + + mcp = FastApiMCP(simple_fastapi_app) + resolved = mcp._resolve_router_include_prefix(router_b) + + assert resolved != "/alpha", "Must never resolve to the other router's prefix" + if FASTAPI_HAS_INCLUDED_ROUTER_METADATA: + assert resolved == "/beta", "Should resolve exactly via include-time identity metadata" + else: + assert resolved == "", ( + "Known limitation on fastapi < 0.137: path-diffing alone cannot disambiguate " + "two routers sharing the same endpoint function, so this must fall back to " + "the safe empty prefix rather than guess." + ) + + +def test_resolve_prefix_with_router_included_at_multiple_prefixes(simple_fastapi_app: FastAPI): + """ + A router included more than once under different prefixes is unusual but legal. + There is no single correct prefix to pick from `mount_http`/`mount_sse`'s signature + alone, so resolution must not silently pick the first one -- it should fall back to + the safe empty prefix instead. + """ + router = APIRouter() + + @router.get("/x", operation_id="shared_x") + async def x() -> dict: + return {} + + simple_fastapi_app.include_router(router, prefix="/one") + simple_fastapi_app.include_router(router, prefix="/two") + + mcp = FastApiMCP(simple_fastapi_app) + resolved = mcp._resolve_router_include_prefix(router) + + assert resolved == "", "Ambiguous multi-prefix inclusion must fall back to the safe empty prefix"