Repository navigation
feat: hosted MCP server at /mcp (gap #10) - #32
Merged
Merged
Conversation
Mounts the mcp SDK's Streamable HTTP transport at /mcp, gated by the same omni_sk_ API keys every other /v1 route already trusts (not the SDK's OAuth-flavored auth subsystem -- see mcp_auth.py's docstring for why publishing OAuth discovery metadata here would misrepresent what omnibioai-auth actually supports). Three tools (answer_with_citations, search_literature, list_domains) loop back to this gateway's own /v1/literature/* routes over HTTP, reusing 100% of existing rate-limit/quota/billing/BYOK logic instead of reimplementing it for a second transport. Fixed four bugs found while building this, each covered by a test: - /mcp mount was registered after the catch-all router and silently bypassed its own auth check - streamable_http_app()'s default host triggers DNS-rebinding protection that 404s every real (non-loopback) Host header (app/main.py) - the Streamable HTTP session manager's task group must be entered via session_manager.run() during the app's lifespan, not left to its own defaults (app/main.py) - FastAPI's root_path constructor override breaks Starlette's Mount child-root_path computation for everything nested under /mcp (app/services/mcp_auth.py) Still open: a full interactive OAuth 2.1 "connector" flow (discovery + dynamic client registration + PKCE) for generic MCP clients -- not buildable without fabricating capabilities omnibioai-auth doesn't have. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI installs via `pip install -e ".[dev]"` from pyproject.toml -- requirements.txt isn't read by CI or the Dockerfile (both use pyproject.toml), so the earlier requirements.txt-only addition left CI's environment without the mcp package, failing every test that imports app.main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mcpSDK's Streamable HTTP transport at/mcp, gated by the sameomni_sk_API keys every/v1route already trusts.answer_with_citations,search_literature,list_domains) loop back to this gateway's own/v1/literature/*routes over HTTP, reusing 100% of existing rate-limit/quota/billing/BYOK logic instead of reimplementing it for a second transport.token_verifier/AuthSettings): that would publish.well-known/oauth-protected-resourcediscovery metadata implying a standards-compliant authorization server with an interactive authorization-code+PKCE flow, which omnibioai-auth doesn't actually run. Built a narrower, honest Bearer-token gate instead — seeapp/services/mcp_auth.py's docstring.Bugs found and fixed while building this
/mcpwas mounted after the catch-all router, so it was silently matched (and its own auth bypassed) by/{service}/{path}instead.streamable_http_app()'s defaulthostauto-enables DNS-rebinding protection that 404s every real (non-loopback)Hostheader — fixed by passinghost="0.0.0.0".StreamableHTTPSessionManagerrequires its task group to be running (session_manager.run()) before any request arrives — wired intoapp.main'slifespan().root_path="/_svc/gateway"constructor override breaks Starlette'sMountchild-root_pathcomputation for everything nested under/mcp, 404ing all real (non-test) traffic even with 1-3 fixed — fixed by resettingscope["root_path"]insideMCPBearerAuthASGIMiddlewarebefore forwarding.Each of these is covered by a regression test (
tests/test_mcp_mount_integration.pyincludes a full realinitializeJSON-RPC handshake assertion, not just a status-code check, specifically because a weaker assertion masked bug 4 for a while during development).Explicitly still open
A full interactive OAuth 2.1 "connector" flow (discovery + dynamic client registration + PKCE) for generic third-party MCP clients. Not buildable without fabricating capabilities omnibioai-auth doesn't have today; left for a future milestone once/if that's actually built on the auth side.
Test plan
pytest tests/test_mcp_auth.py tests/test_mcp_server.py tests/test_mcp_mount_integration.py -q— 17 passedpytest -q— 423 passedapp.main.app(including its realroot_path) via a standalone MCPinitializehandshake script🤖 Generated with Claude Code