feat: add stateless streamable http transport - #30
Open
shenxianpeng wants to merge 5 commits into
Open
Conversation
Add a --transport flag (with MCP_TRANSPORT/MCP_HOST/MCP_PORT environment fallbacks) so the server can run as a stateless Streamable HTTP service per the 2026-07-28 MCP specification, alongside the stdio default. Every tool here is a pure function of its inputs, so statelessness is not a toggle: HTTP mode always runs with stateless_http=True and json_response=True, letting any instance behind a plain load balancer answer any request without an initialize handshake or session header. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Self-review fixes on top of the stateless HTTP transport: - argparse never checks `choices` against an env-supplied default, so a typo in MCP_TRANSPORT would silently fall back to stdio inside a container that expects an HTTP listener; validate it after parsing. - A non-integer MCP_PORT now exits with a clean parser error instead of an unhandled ValueError traceback. - Drop a redundant local `import os` (already imported at module level). - Fix a stale README sentence claiming the server is not meant to run as a long-running HTTP service, which contradicted the new section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
📝 WalkthroughWalkthroughThe server now supports configurable stdio and stateless Streamable HTTP transports. CLI arguments and prefixed environment variables configure transport, host, port, and allowlists. Tests cover startup, validation, security settings, and HTTP responses. The README documents deployment and request usage. ChangesTransport startup support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator as CLI and environment settings
participant Main as main
participant Security as host and origin validation
participant Server as Streamable HTTP or stdio transport
Operator->>Main: configure transport, host, port, and allowlists
Main->>Main: resolve CLI and environment precedence
Main->>Security: validate HTTP security settings
Security-->>Main: return validated configuration
Main->>Server: start selected transport
Server-->>Main: run with configured options
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 257-268: Update the documented Stateless HTTP curl example for the
tools/call request to include the required MCP-Protocol-Version, Mcp-Method, and
Mcp-Name headers, using values consistent with the 2026-07-28 request format.
In `@src/commit_check_mcp/server.py`:
- Around line 656-663: Update the HTTP startup path in
src/commit_check_mcp/server.py:656-663 to configure explicit allowed hosts and
origins via TransportSecuritySettings, pass those settings to mcp.run, and
reject non-loopback binding unless authentication or an explicitly selected
trusted reverse-proxy mode is configured. Update README.md:237-251 to document
required TLS, authentication, reverse-proxy, and Host/Origin allowlists, and
remove any implication that binding to 0.0.0.0 is safe as a standalone remote
deployment.
- Around line 624-627: Remove the eager MCP_PORT integer parsing and pass the
environment value as the --port parser default, allowing argparse’s existing
type=int conversion to run only when --port is absent. Preserve the CLI override
behavior and add a regression test covering invalid MCP_PORT with --port 9000.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a24b7f45-6e49-46c3-9fdb-63ee8a4c41d5
📒 Files selected for processing (3)
README.mdsrc/commit_check_mcp/server.pytests/test_server.py
Address CodeRabbit review on the stateless HTTP transport: - Add --allowed-hosts/--allowed-origins (MCP_ALLOWED_HOSTS / MCP_ALLOWED_ORIGINS) wiring TransportSecuritySettings with strict DNS rebinding protection into HTTP mode; verified live that a foreign Host header is rejected with 421 when an allowlist is set. README now states the server has no built-in auth/TLS and that non-loopback binds belong behind a reverse proxy on a private network. - Parse MCP_PORT lazily via the argparse string-default rule so an invalid inherited MCP_PORT still fails loudly on its own but cannot veto an explicit valid --port; regression test added. - Extend the README curl example with the SEP-2243 headers plus the params._meta envelope that MCP-Protocol-Version requires — the header-only form suggested in review is rejected with 400 by the SDK; the documented form was run verbatim against a live server. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Two defects found by testing the merge risk of this branch against a live server rather than only through mocks. --allowed-origins without --allowed-hosts enabled DNS-rebinding protection with an empty host allowlist, which rejects *every* request with 421 Misdirected Request. Measured on a running server: no allowlist -> 200, origins-only -> 421, hosts set -> 200. Origins-only is therefore never a usable configuration, and the README presented both flags side by side, so reaching it took only using one of them. The combination now fails at startup instead of serving an endpoint that answers nothing. The environment variables were unprefixed. A bare MCP_TRANSPORT belongs to no particular server, so a value left over from an unrelated one turned a stdio launch -- how every desktop MCP client starts this server -- into an HTTP listener that never answers the client's handshake; confirmed by launching with MCP_TRANSPORT=http and watching uvicorn come up on a stdio invocation. They are now COMMIT_CHECK_MCP_*, and a test pins that the unprefixed names are ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #30 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 203 228 +25
=========================================
+ Hits 203 228 +25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commit_check_mcp/server.py`:
- Around line 643-647: Validate the parsed HTTP port in the argument-parser
setup around the --port option, accepting only values from 1 through 65535
before mcp.run starts. Apply the same validation to COMMIT_CHECK_MCP_PORT, and
add boundary tests covering invalid and valid values for both the CLI argument
and environment variable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e257bfad-40cb-4269-8ca3-7e6d38182456
📒 Files selected for processing (3)
README.mdsrc/commit_check_mcp/server.pytests/test_server.py
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
argparse's type=int accepted any integer and handed it to uvicorn. Measured against a live server: -1, 65536 and 99999 die deep in asyncio with a bare OverflowError traceback, and port 0 is worse -- the server starts on an arbitrary free port, so a container whose COMMIT_CHECK_MCP_PORT expands to 0 comes up "successfully" somewhere nothing can reach it. That is the same silent misconfiguration this branch already closed for the transport name and the origins-only allowlist. Ports are now validated to 1..65535 by the argument type, which keeps the lazy-conversion property: an invalid inherited environment value still fails on its own but cannot veto an explicit, valid --port. Boundary tests cover both the flag and the environment variable. Raised by CodeRabbit review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
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
3 participants
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
Adds a stateless Streamable HTTP mode per the 2026-07-28 MCP specification, alongside the existing stdio default. This completes the SDK v2 migration started in #27: the two-line SDK bump made the server run on mcp 2.0, and this PR claims the spec's actual payoff for a pure-function tool server — session-free multi-instance deployment.
commit-check-mcp --transport http [--host HOST] [--port PORT], withCOMMIT_CHECK_MCP_TRANSPORT/_HOST/_PORTenvironment fallbacks for containers.stateless_http=True,json_response=True). Every tool here is a pure function of its inputs, so statelessness is not offered as a toggle — a toggle would only invite misconfiguration.--allowed-hosts/--allowed-origins(env:COMMIT_CHECK_MCP_ALLOWED_HOSTS/_ALLOWED_ORIGINS) wireTransportSecuritySettingswith strict DNS-rebinding protection.Design notes
stdiostays the default and the recommended mode for local MCP clients; nothing changes for existing users.sseis deliberately not offered; it is legacy in SDK 2.0.main()now parsesargv, so unknown arguments exit with a parser error. Previously every argument was silently ignored, so a client config that passes one tocommit-check-mcpwill now fail to start.Four failure modes closed, each found by testing against a live server
Mock-level tests passed while the server itself was broken in three of these, so each was measured end to end:
--allowed-originswithout--allowed-hosts421— protection on with an empty host allowlist denies allMCP_TRANSPORT=httpin the environmentCOMMIT_CHECK_MCP_*--port 0port must be between 1 and 65535--port -1/65536OverflowErrortraceback from inside asyncioVerification
uv run --extra dev pytest), including tests pinning that HTTP mode always carriesstateless_http=True— a future refactor cannot silently reintroduce sessions — plus port boundary, env-precedence, and invalid-configuration rejection tests for both the flags and the environment variables.POST /mcptools/callwith no initialize handshake and noMcp-Session-Idheader returns 200 with the full validation result (server log:Terminating session: None).params._metaenvelope thatMCP-Protocol-Version: 2026-07-28requires — the header-only form is rejected with 400 by the SDK.Hostheader is rejected with421; without one, the container0.0.0.0case still serves normally.MCPServer.run()forwards kwargs torun_streamable_http_async(*, host, port, json_response, stateless_http, transport_security, ...).🤖 Generated with Claude Code
https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn