Skip to content

Fix router prefix ignored when set via include_router (#204) #329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wakqasahmed wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix router prefix ignored when set via include_router (#204) #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a6ad55
fix: resolve router mount path from app.routes instead of assuming ro…
wakqasahmed Aug 15, 2026
65449e0
fix: resolve include_router prefix across fastapi <0.137 and >=0.137 …
wakqasahmed Aug 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 130 additions & 5 deletions fastapi_mcp/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[
Expand Down Expand Up @@ -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("/"):
Expand All @@ -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 ""

@wakqasahmed wakqasahmed Aug 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ambiguity when a router is included more than once, and no re-evaluation after mount time.

Multiple inclusions — unusual but legal, and the method silently picks whichever inclusion appears first in app.routes:

app.include_router(rc, prefix="/one")
app.include_router(rc, prefix="/two")
mcp._resolve_router_include_prefix(rc)  # -> '/one'
# after mount_http(rc): ['/one/x', '/two/x', '/one/x', '/one/mcp']

There is no correct answer here from mount_http()'s signature alone — but "first wins, silently" is the wrong default. Either logger.warning on multiple distinct candidate prefixes, or let the caller disambiguate explicitly.

Ordering — this is computed once, here, at mount time and never revisited. Mounting before including still mounts at the root, and then the later include_router re-registers the MCP route a second time:

mcp.mount_http(rf)                       # rf not yet included
app.include_router(rf, prefix="/late")
# app paths: ['/z', '/mcp', '/late/z', '/late/mcp']

Two MCP endpoints, one of them at the root. Worth at least documenting the ordering requirement in the mount_http/mount_sse docstrings ("the router must already be included into the app"), and ideally warning when the router isn't found in the app at all.

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

Expand All @@ -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,
Expand Down Expand Up @@ -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("/"):
Expand All @@ -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)
Expand All @@ -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)

@wakqasahmed wakqasahmed Aug 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed correct — no double-counting when a router has both a constructor prefix and an include_router prefix, because router.routes paths already contain router.prefix, so the diff yields only the include-time prefix and include_router(..., prefix=include_prefix) re-applies router.prefix itself:

rd = APIRouter(prefix="/inner"); app.include_router(rd, prefix="/outer")
resolved='/outer', router.prefix='/inner'  ->  '/outer/inner/mcp'   # correct

Also verified mount_sse's include_prefix + router.prefix + mount_path produces the right messages_path (/sse-pfx/sse/messages/).

One pre-existing wart this change makes more visible: re-including the router now lands its own routes on exactly the same paths as the original inclusion, so every route is registered twice at the same path and app.openapi() emits UserWarning: Duplicate Operation ID .... Before this PR the duplicates landed at different (root) paths, so it was equally duplicated but less obvious. Not introduced here, and the # HACK ... TODO: Find a better way to do this comment already flags it — but if you're touching this line anyway, re-including only the newly added MCP route instead of the whole router would kill both the duplication and most of the edge cases above.


logger.info(f"MCP SSE server listening at {mount_path}")
logger.info(f"MCP SSE server listening at {base_path}")

def mount(
self,
Expand Down
155 changes: 154 additions & 1 deletion tests/test_basic_functionality.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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."""
Expand Down Expand Up @@ -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

@wakqasahmed wakqasahmed Aug 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the crux of the scope gap: the issue's other_router = APIRouter() has no routes at all, and the fix depends entirely on there being at least one route to diff against. Adding /ping here makes the test pass but moves it off the reported case.

Please keep this test and add one with a genuinely empty router asserting /empty/mcp (it currently yields /mcp), plus a case where the same endpoint function is shared by two routers. Those two are where the implementation actually breaks.


@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"