Feat: added stateless streamable_http transport for MCP at /mcp/http #1212
Conversation
|
""" WalkthroughThe changes introduce a new streamable HTTP endpoint for the MCP (Model Context Protocol) in the server, update related documentation, add a corresponding test script, and refactor the server's lifespan management to support multiple async context managers. Utility code is added to combine lifespans, and documentation is updated to reflect the new endpoint and testing instructions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTP_Handler as MCP HTTP Handler
participant SessionManager as StreamableHTTPSessionManager
participant MCP_Server
Client->>HTTP_Handler: HTTP request to /mcp/http
HTTP_Handler->>SessionManager: Delegate request
SessionManager->>MCP_Server: Process MCP command (e.g., list_tools)
MCP_Server-->>SessionManager: Return response
SessionManager-->>HTTP_Handler: Stream response
HTTP_Handler-->>Client: Stream response back
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
docs/md_v2/core/docker-deployment.md (1)
260-265: Unify endpoint casing to avoid copy-paste errors
Streamable_httpis the only entry using a capital “S” and an underscore inside the name. Everywhere else in the codebase the transport is referred to asstreamable_http.
Align the casing here to prevent users from hitting a 404 due to a typo.- - **Streamable_http**: `http://localhost:11235/mcp/http` + - **Streamable_http**: `http://localhost:11235/mcp/http`deploy/docker/README.md (1)
260-265: Endpoint name differs from implementationSame nit as in the main docs file – please change
Streamable_http→streamable_http(or whichever canonical form you settle on) to keep docs, tests and server logs in sync.deploy/docker/utils.py (2)
67-68: Swallowed exception hides MX lookup problemsThe broad
except ExceptionreturnsFalsesilently. Logging the exception (at debug/info) helps diagnose DNS or network issues.- except Exception as e: - return False + except Exception as exc: + logging.getLogger(__name__).debug("verify_email_domain(%s) failed: %s", email, exc) + return False
70-85:combine_lifespans– simpler and safer withAsyncExitStackcontext managerManual
__aenter__/__aexit__is easy to get wrong if anenter_async_contextfails early. You can rely on the stack’s own context-manager interface:- @contextlib.asynccontextmanager - async def combined(app): - # Nest the lifespans like contextlib.ExitStack does for async - stack = contextlib.AsyncExitStack() - await stack.__aenter__() - - try: - for lifespan in lifespans: - await stack.enter_async_context(lifespan(app)) - yield - finally: - await stack.__aexit__(None, None, None) + @contextlib.asynccontextmanager + async def combined(app): + async with contextlib.AsyncExitStack() as stack: + for lifespan in lifespans: + await stack.enter_async_context(lifespan(app)) + yieldShorter and automatically handles partial-enter failure cases.
deploy/docker/server.py (2)
10-15: Remove unused importcontextlib
contextlibis imported here but never referenced. Drop it to satisfy Ruff and keep imports tidy.🧰 Tools
🪛 Ruff (0.11.9)
10-10:
contextlibimported but unusedRemove unused import:
contextlib(F401)
601-606: Use a logger instead ofprint()for server start notice
print()executes at import time, which is undesirable with Gunicorn/Uvicorn workers and pollutes stdout.
Switch tologging.getLogger(__name__).info(...)or remove entirely—Uvicorn already logs startup.deploy/docker/mcp_bridge.py (4)
6-6: Remove unused importAny.The static analysis correctly identifies that
Anyis imported but never used in the code.-from typing import Any, AsyncContextManager, Callable, Dict, List, Tuple +from typing import AsyncContextManager, Callable, Dict, List, Tuple🧰 Tools
🪛 Ruff (0.11.9)
6-6:
typing.Anyimported but unusedRemove unused import:
typing.Any(F401)
22-22: Remove unused importMount.The static analysis correctly identifies that
Mountis imported but never used in the code.from starlette.applications import Starlette -from starlette.routing import Mount from starlette.types import Receive, Scope, Send🧰 Tools
🪛 Ruff (0.11.9)
22-22:
starlette.routing.Mountimported but unusedRemove unused import:
starlette.routing.Mount(F401)
271-280: Replace print statements with proper logging.Consider using proper logging instead of print statements for production-ready code. This will provide better control over log levels and formatting.
Add logging import at the top of the file:
import logging logger = logging.getLogger(__name__)Then update the lifespan function:
@contextlib.asynccontextmanager async def mcp_lifespan(_: Starlette) -> AsyncIterator[None]: """Context manager for session manager.""" async with session_manager.run(): - print("Application started with StreamableHTTP session manager!") + logger.info("Application started with StreamableHTTP session manager") try: yield finally: - print("Application shutting down...") + logger.info("Application shutting down")
84-297: Consider decomposing this function for better maintainability.The static analysis correctly identifies that
attach_mcphas grown quite large (29 local variables, 77 statements). While functional, consider breaking it down into smaller, focused functions in a future refactoring:
- Extract route registration logic
- Extract MCP handler setup
- Extract transport configuration (WebSocket, SSE, HTTP)
This would improve readability and testability.
🧰 Tools
🪛 Ruff (0.11.9)
135-135: Loop control variable
proxynot used within loop bodyRename unused
proxyto_proxy(B007)
🪛 Pylint (3.3.7)
[refactor] 84-84: Too many local variables (29/15)
(R0914)
[refactor] 84-84: Too many statements (77/50)
(R0915)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
deploy/docker/README.md(2 hunks)deploy/docker/mcp_bridge.py(8 hunks)deploy/docker/server.py(8 hunks)deploy/docker/utils.py(2 hunks)docs/md_v2/core/docker-deployment.md(2 hunks)tests/mcp/test_mcp_http.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
deploy/docker/utils.py (1)
deploy/docker/server.py (1)
lifespan(106-114)
🪛 Ruff (0.11.9)
tests/mcp/test_mcp_http.py
5-6: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
deploy/docker/mcp_bridge.py
6-6: typing.Any imported but unused
Remove unused import: typing.Any
(F401)
22-22: starlette.routing.Mount imported but unused
Remove unused import: starlette.routing.Mount
(F401)
deploy/docker/server.py
10-10: contextlib imported but unused
Remove unused import: contextlib
(F401)
37-37: utils.FilterType imported but unused
Remove unused import: utils.FilterType
(F401)
43-43: Redefinition of unused List from line 15
Remove definition: List
(F811)
44-44: contextlib._AsyncGeneratorContextManager imported but unused
Remove unused import
(F401)
44-44: contextlib.AsyncExitStack imported but unused
Remove unused import
(F401)
🪛 Pylint (3.3.7)
deploy/docker/mcp_bridge.py
[refactor] 84-84: Too many local variables (29/15)
(R0914)
[refactor] 84-84: Too many statements (77/50)
(R0915)
🔇 Additional comments (3)
deploy/docker/server.py (1)
615-616: Lifespan assignment works – 👍The new
combine_lifespanscall correctly merges the app and MCP lifespans and replaces the deprecatedlifespan=parameter. Good catch.deploy/docker/mcp_bridge.py (2)
84-90: Good architectural improvement for lifespan management.The refactoring to return a callable that produces an async context manager is a clean approach for integrating multiple lifespans in the FastAPI application. This enables proper lifecycle management of the new HTTP transport components.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 84-84: Too many local variables (29/15)
(R0914)
[refactor] 84-84: Too many statements (77/50)
(R0915)
259-264:❓ Verification inconclusive
Verify the
event_store=Noneconfiguration.Please confirm that passing
event_store=Noneto theStreamableHTTPSessionManageris the intended behavior for stateless operation. This might affect event persistence or streaming capabilities.
🏁 Script executed:
#!/bin/bash # Search for StreamableHTTPSessionManager usage patterns in the codebase ast-grep --pattern 'StreamableHTTPSessionManager($$$)' # Look for documentation or other usages of event_store parameter rg -A 5 -B 5 "event_store" --type pyLength of output: 1001
🏁 Script executed:
#!/bin/bash # Locate where StreamableHTTPSessionManager is defined and inspect its constructor rg -n "class StreamableHTTPSessionManager" --type py -C5 rg -n "def __init__" "$(rg -l 'class StreamableHTTPSessionManager' --type py)" -C5Length of output: 185
Verify
event_store=Noneusage
We couldn’t locate theStreamableHTTPSessionManagerdefinition in the repo to confirm the default behavior ofevent_store=None. Please double-check that passingNonehere truly disables persistence (as intended for a stateless endpoint) and doesn’t inadvertently break streaming or error handling.• File:
deploy/docker/mcp_bridge.pylines 259–264
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
deploy/docker/server.py (1)
37-38:⚠️ Potential issuePrune unused symbol
FilterType
FilterTypeisn’t used in this module; leaving it here re-introduces the linter warning flagged in the previous review. While touching the line, also drop the duplicate whitespace after the opening parenthesis.-from utils import ( - FilterType, combine_lifespans, load_config, setup_logging, verify_email_domain +from utils import ( + combine_lifespans, load_config, setup_logging, verify_email_domain )🧰 Tools
🪛 Ruff (0.11.9)
37-37:
utils.FilterTypeimported but unusedRemove unused import:
utils.FilterType(F401)
🧹 Nitpick comments (4)
deploy/docker/server.py (4)
119-119: Minor nit: stray inline comment formatting
#allis missing a space after the hash. Consider tidying for consistency:-) #all lifespans will be added just before passing app to asgi server +) # All lifespans will be added just before passing app to ASGI server
246-251: Return payload duplicates request parametersEchoing request parameters back verbatim (
url,filter,query,cache) adds bandwidth but no functional value to the client, which already knows what it sent. Unless this is required by a public contract, consider omitting them to keep responses lean.
529-544: Docstring grammar / typosSpotted a few small issues (“design” → “designed”, “Alway is BEST practice” → “It is always best practice”, “lenght”). Polishing the wording improves API self-documentation.
- This end point is design for any questions about Crawl4ai library. + This endpoint is designed for any questions about the Crawl4AI library. - Alway is BEST practice you provide a query to filter the context. + It is always best practice to provide a query to filter the context.
564-566: Consistent key orderingFor parity with the non-query path above, consider returning
doc_contextfirst or using an OrderedDict / explicit ordering to avoid flaky snapshot tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
deploy/docker/server.py(8 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
deploy/docker/server.py
10-10: contextlib imported but unused
Remove unused import: contextlib
(F401)
37-37: utils.FilterType imported but unused
Remove unused import: utils.FilterType
(F401)
🔇 Additional comments (1)
deploy/docker/server.py (1)
612-615: Verifycombine_lifespansbehaviourAssigning
app.router.lifespan_contextpost-instantiation is fine, but double-check that:
combine_lifespanspreserves the execution order you expect (original app warm-up first, MCP second or vice-versa).- No other part of the code mutates
router.lifespan_contextafterwards.If unsure, add an assertion or unit test around the combined context manager.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
deploy/docker/server.py (2)
35-37: Remove the unusedFilterTypeimport (Ruff F401).
The symbol isn’t referenced anywhere in this file and keeps triggering the linter.-from utils import ( - FilterType, combine_lifespans, load_config, setup_logging, verify_email_domain -) +from utils import ( + combine_lifespans, load_config, setup_logging, verify_email_domain +)🧰 Tools
🪛 Ruff (0.11.9)
36-36:
utils.FilterTypeimported but unusedRemove unused import:
utils.FilterType(F401)
599-604: Swap
Keeping a raw-print(f"MCP server running on {config['app']['host']}:{config['app']['port']}") +import logging +logger = logging.getLogger(__name__) +logger.info("MCP server running on %s:%s", config['app']['host'], config['app']['port'])
🧹 Nitpick comments (1)
deploy/docker/server.py (1)
528-531: Tighten up doc-string grammar.
Minor wording tweaks improve clarity/readability.- This end point is design for any questions about Crawl4ai library. It returns a plain text markdown with extensive information about Crawl4ai. - You can use this as a context for any AI assistant. Use this endpoint for AI assistants to retrieve library context for decision making or code generation tasks. - Alway is BEST practice you provide a query to filter the context. Otherwise the lenght of the response will be very long. + This endpoint is designed for any questions about the Crawl4AI library. It returns Markdown with extensive information about Crawl4AI. + AI assistants can use the response as context for decision-making or code-generation tasks. + As a best practice, always provide a query to filter the context; otherwise the response may be very long.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
deploy/docker/server.py(7 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
deploy/docker/server.py
36-36: utils.FilterType imported but unused
Remove unused import: utils.FilterType
(F401)
🔇 Additional comments (1)
deploy/docker/server.py (1)
611-614: Double-check lifespan registration order.
app.router.lifespan_contextis overwritten here; if another module mutates it later, only the last assignment will take effect.
Confirm no subsequent code (or uvicorn import side-effects) replaces this attribute; otherwise wrap all lifespans once and assign a single time.
|
Hi @aravindkarnam |
Summary
added stateless streamable_http endpoint for MCP at /mcp/http
Ref: #1158
List of files changed and why
How Has This Been Tested?
Tested all the tools of mcp server via postman and also added tests/mcp/test_mcp_http.py with test to list all tools
Checklist:
Summary by CodeRabbit