Add Streamable HTTP transport, CORS support, and SSE fixes to MCP server#68
Add Streamable HTTP transport, CORS support, and SSE fixes to MCP server#68danstarns wants to merge 1 commit intoneo4j-labs:mainfrom
Conversation
|
@danstarns is attempting to deploy a commit to the lyonwj's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds browser-friendly MCP HTTP serving options and improves SSE compatibility, plus updates docs to help users get started quickly.
Changes:
- Added Streamable HTTP transport (
--transport streamable-http) with CORS support. - Improved SSE transport compatibility (no-op response +
/messagesmounted as ASGI app) and added CORS middleware. - Added
--allow-originand--openai-api-keyCLI flags; added new MCP Server Quick Start doc and linked it from README.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/neo4j_agent_memory/mcp/server.py |
Adds Streamable HTTP transport, CORS configuration, SSE routing/response fixes, and new CLI flags integration. |
README.md |
Links to the new MCP server quick start. |
QUICKSTART.md |
New quick start guide documenting transports, CORS, and flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| inner_app = Starlette( | ||
| routes=[ | ||
| Mount("/", app=handle_mcp), |
There was a problem hiding this comment.
run_streamable_http mounts the Streamable HTTP handler at / (via Mount("/", ...)), but the Quick Start/docs in this PR state the endpoint is /mcp. This mismatch will lead to 404s for clients following the docs; either mount at /mcp here or update the docs to point to /.
| Mount("/", app=handle_mcp), | |
| Mount("/mcp", app=handle_mcp), |
| parser.add_argument( | ||
| "--allow-origin", | ||
| action="append", | ||
| dest="allow_origins", | ||
| help="Allowed CORS origin (repeatable, defaults to '*'). " | ||
| "Example: --allow-origin https://example.com --allow-origin https://app.example.com", | ||
| ) | ||
| parser.add_argument( | ||
| "--openai-api-key", | ||
| default=os.environ.get("OPENAI_API_KEY", ""), | ||
| help="OpenAI API key (for embeddings/extraction)", | ||
| ) |
There was a problem hiding this comment.
Now that --transport supports both SSE and streamable HTTP, the CLI help text for --host/--port still says “for SSE transport”. Consider updating those help strings to refer to “HTTP transports” to match the updated run_server docstring and avoid user confusion.
| Server runs at `http://localhost:5000/mcp`. All CORS origins allowed by default. | ||
|
|
There was a problem hiding this comment.
Docs say the Streamable HTTP server runs at http://localhost:5000/mcp, but the implementation in Neo4jMemoryMCPServer.run_streamable_http() mounts the handler at /. Update the documented URL or adjust the server mount path so the Quick Start is accurate.
| Endpoint: `http://localhost:5000/mcp` — POST JSON-RPC, get JSON back. | ||
|
|
There was a problem hiding this comment.
The Streamable HTTP “Endpoint:” line references /mcp, but the current server code mounts the Streamable HTTP handler at /. Please align this endpoint string with the actual mount path so users can copy/paste the example successfully.
| ## CORS | ||
|
|
||
| All origins allowed by default. To restrict: | ||
|
|
||
| ```bash | ||
| uv run python -m neo4j_agent_memory.mcp.server \ | ||
| --transport streamable-http --port 5000 --neo4j-password test-password \ | ||
| --allow-origin https://app.example.com \ | ||
| --allow-origin https://admin.example.com | ||
| ``` |
There was a problem hiding this comment.
The documentation here promotes a default CORS configuration of allowing all origins for the MCP HTTP endpoints, which combined with the server’s lack of authentication makes data exfiltration from a locally running instance trivial via a malicious web page. A site an operator visits can issue cross-origin fetch/EventSource calls to http://localhost:<port>/mcp or the SSE endpoints and, because of Access-Control-Allow-Origin: *, read sensitive memory and graph data from their Neo4j-backed store. Consider recommending a restricted default (or explicitly marking * as development-only) and documenting that in production or when binding to 0.0.0.0, operators should explicitly set --allow-origin to trusted origins only.
--transport streamable-http) — single endpoint, no SSE handshake, works with browser clients*, configurable via--allow-origin_NoOpResponsefor Starlette compat, changed/messagesfromRoutetoMountsince the handler is a raw ASGI app--openai-api-keyCLI flag