Skip to content

Fix a bug with handling FastAPI root_path parameter #163

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.

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
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

am1ter
Copy link

@am1ter am1ter commented Jun 8, 2025

Describe your changes

Issue resolving

The current version of the library does not handle FastAPI's root_path parameter correctly.

FastAPI.router includes root_path in all its routes by default, so there is no need to add root_path to /messages again, as is done in the current version of fastapi_mcp. The problem is located here.

Because of these duplicated root_path annotations, all requests to the /messages endpoints are processed incorrectly. This issue can be easily reproduced using the following snippet:

from fastapi import FastAPI
from fastapi_mcp import FastApiMCP

app = FastAPI(root_path="/api")  # Does not work, but `app = FastAPI()` works
mcp = FastApiMCP(app)
mcp.mount()

if __name__ == "__main__":
    import uvicorn
    uvicorn.run(app, host="0.0.0.0", port=8101)

When you run MCP Inspector with http://127.0.0.1:8101/mcp connection string you will see the following log in the uvicorn output:

INFO:     Uvicorn running on http://0.0.0.0:8101 (Press CTRL+C to quit)
INFO:     127.0.0.1:53784 - "GET /mcp HTTP/1.1" 200 OK
INFO:     127.0.0.1:60936 - "GET /mcp HTTP/1.1" 200 OK
INFO:     127.0.0.1:60944 - "POST /api/api/mcp/messages/?session_id=a09e48d070204eb09420f54bdeb913ae HTTP/1.1" 404 Not Found

When you run MCP Inspector with http://127.0.0.1:8101/api/mcp connection string you will see the following log in the uvicorn output:

INFO:     Uvicorn running on http://0.0.0.0:8101 (Press CTRL+C to quit)
INFO:     127.0.0.1:32798 - "GET /mcp HTTP/1.1" 200 OK
INFO:     127.0.0.1:32800 - "GET /api/mcp HTTP/1.1" 200 OK
INFO:     127.0.0.1:32802 - "POST /api/api/mcp/messages/?session_id=d52c467095fc4350924b027fd3b578a2 HTTP/1.1" 404 Not Found

This PR fixes this issue.

Tests

This PR introduces some new test to prevent this issue in the future and also it refactors some basic fixtures to enhance flexibility by supporting multiple configurations for test servers.

I think there were a better way to add new tests for this root_path issue, but unfortunately I couldn't find an easy way to resolve a full URL to FastAPI's endpoint. If you can give me an advice how it could be enhanced - please do it and I'll prepare an update.

Issue ticket number and link (if applicable)

N/A

Screenshots of the feature / bugfix

FastAPI's root_path part duplication in the POST request to messages endpoint:

CleanShot 2025-06-08 at 01 40 01

An example of the broken unit tests, that were added in this PR in the case if we do not apply the main changes to fastapi_mcp/server.py (with the changes from this PR all tests are passed correctly).

CleanShot 2025-06-08 at 01 41 01

With the changes from this PR all tests are passed correctly:

CleanShot 2025-06-08 at 01 48 43

Checklist before requesting a review

  • Added relevant tests
  • Run ruff & mypy (No new errors, but current version of the repository has some errors)
  • All tests pass

am1ter added 2 commits June 8, 2025 02:37
- Fix a bug where the mount path was not correctly appended to the root
  path of the FastAPI router
- Replace a raise statement with an assert, because the code is
  unreachable if the input attribute has correct type
- Introduced a new utility function `make_fastapi_app_base` to create
  FastAPI instances with common configurations.
- Modified the `run_server` function to accept a FastAPI app as a
  parameter, enhancing flexibility in test setups.
- Updated test fixtures to utilize the new function for better
  maintainability and consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant