From 4a6ad5531d475c7fcac22aac9e22aeacd5ea5ac3 Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Sat, 15 Aug 2026 23:35:32 +0200 Subject: [PATCH 1/2] fix: resolve router mount path from app.routes instead of assuming router.prefix (#204) --- fastapi_mcp/server.py | 35 ++++++++++++++++++++++++++++--- tests/test_basic_functionality.py | 27 +++++++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index bb751067..6e35de98 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -309,6 +309,32 @@ def _setup_auth(self): else: logger.info("No auth config provided, skipping auth setup") + 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 in the resulting routes on `app.routes` -- the router object's own + `.prefix` attribute stays empty. To find the effective prefix, match a route + already registered on `router` against its counterpart on `self.fastapi.routes` + (same endpoint function) and diff the paths. + + Returns an empty string if no match is found (e.g. the router hasn't been + included into the app yet), preserving the previous behavior of relying solely + on `router.prefix`. + """ + for router_route in router.routes: + router_path = getattr(router_route, "path", None) + router_endpoint = getattr(router_route, "endpoint", None) + if not router_path or router_endpoint is None: + continue + for app_route in self.fastapi.routes: + app_path = getattr(app_route, "path", "") + if getattr(app_route, "endpoint", None) is router_endpoint and app_path.endswith(router_path): + return app_path[: -len(router_path)] + return "" + def mount_http( self, router: Annotated[ @@ -348,6 +374,8 @@ 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 "" + http_transport = FastApiHttpSessionManager(mcp_server=self.server) dependencies = self._auth_config.dependencies if self._auth_config else None @@ -361,7 +389,7 @@ 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}") @@ -404,7 +432,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,7 +448,7 @@ 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}") diff --git a/tests/test_basic_functionality.py b/tests/test_basic_functionality.py index 3557226e..9df9401c 100644 --- a/tests/test_basic_functionality.py +++ b/tests/test_basic_functionality.py @@ -1,4 +1,4 @@ -from fastapi import FastAPI +from fastapi import APIRouter, FastAPI from mcp.server.lowlevel.server import Server from fastapi_mcp import FastApiMCP @@ -64,3 +64,28 @@ 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) + + mounted_paths = [getattr(route, "path", None) for route in simple_fastapi_app.routes] + + assert "/other/route/mcp" in mounted_paths, f"Expected mount path not found in: {mounted_paths}" + assert "/mcp" not in mounted_paths, "MCP endpoint should not be mounted at the app root" From 65449e0d7542b2a4092933884452f0f10af7237e Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Sun, 16 Aug 2026 00:00:27 +0200 Subject: [PATCH 2/2] fix: resolve include_router prefix across fastapi <0.137 and >=0.137 (#204) Address blocking review findings on PR #329: - FastAPI >= 0.137 stopped flattening include_router()'d sub-routes into app.routes (it appends an opaque _IncludedRouter object instead), which made the previous path-diffing resolver silently return "" and regress to the original bug. Now resolve the prefix from FastAPI's own _IncludedRouter.original_router / include_context.prefix metadata when available, which is exact (object identity, not endpoint-function identity) and works even when the router has no routes of its own. - Falls back to the previous path-diffing approach on fastapi < 0.137, but hardened: requires every route on the router to agree on the same candidate prefix (matching endpoint identity and HTTP methods) before accepting it, and warns + falls back to the safe empty prefix on disagreement, instead of silently mounting under an unrelated router's prefix when two routers share the same endpoint function. - Warn (and fall back to the safe empty prefix) when a router is included under more than one distinct prefix, instead of silently picking the first one. - mount_http/mount_sse now log the actually-resolved mount path instead of the pre-resolution relative mount_path. - Document that mount_http/mount_sse must be called after the router has already been include_router()'d -- resolution happens once at mount time and is not re-evaluated later. Tested against fastapi 0.115.11 (this repo's uv.lock pin) and 0.141.1 (current latest as of writing) -- full suite green on both. Fixes #204 fully on fastapi >= 0.137 (including the issue's literal zero-route repro). On fastapi < 0.137 the zero-route case remains unresolvable (no data in app.routes to recover the prefix from) and is left as a documented, safe fallback to the app root rather than a crash or a silently wrong prefix -- see the docstrings on _resolve_include_prefix_by_diff and the new test_mount_empty_router_with_include_router_prefix test. --- fastapi_mcp/server.py | 136 +++++++++++++++++++++++++----- tests/test_basic_functionality.py | 134 ++++++++++++++++++++++++++++- 2 files changed, 247 insertions(+), 23 deletions(-) diff --git a/fastapi_mcp/server.py b/fastapi_mcp/server.py index 6e35de98..28f7ce87 100644 --- a/fastapi_mcp/server.py +++ b/fastapi_mcp/server.py @@ -309,31 +309,116 @@ 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 in the resulting routes on `app.routes` -- the router object's own - `.prefix` attribute stays empty. To find the effective prefix, match a route - already registered on `router` against its counterpart on `self.fastapi.routes` - (same endpoint function) and diff the paths. - - Returns an empty string if no match is found (e.g. the router hasn't been - included into the app yet), preserving the previous behavior of relying solely - on `router.prefix`. + 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. """ - for router_route in router.routes: - router_path = getattr(router_route, "path", None) - router_endpoint = getattr(router_route, "endpoint", None) - if not router_path or router_endpoint is None: - continue - for app_route in self.fastapi.routes: - app_path = getattr(app_route, "path", "") - if getattr(app_route, "endpoint", None) is router_endpoint and app_path.endswith(router_path): - return app_path[: -len(router_path)] - return "" + 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, @@ -362,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("/"): @@ -375,6 +465,7 @@ 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 @@ -391,7 +482,7 @@ def mount_http( if isinstance(router, APIRouter): 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, @@ -420,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("/"): @@ -450,7 +546,7 @@ def mount_sse( if isinstance(router, APIRouter): 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 9df9401c..8820351c 100644 --- a/tests/test_basic_functionality.py +++ b/tests/test_basic_functionality.py @@ -1,8 +1,49 @@ +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.""" @@ -85,7 +126,94 @@ async def ping() -> dict: mcp = FastApiMCP(simple_fastapi_app) mcp.mount_http(other_router) - mounted_paths = [getattr(route, "path", None) for route in simple_fastapi_app.routes] + 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 "/other/route/mcp" in mounted_paths, f"Expected mount path not found in: {mounted_paths}" - assert "/mcp" not in mounted_paths, "MCP endpoint should not be mounted at the app root" + assert resolved == "", "Ambiguous multi-prefix inclusion must fall back to the safe empty prefix"