docs: add Unix socket and SSE transport to MCP Integration Guide (#6739)#6855
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdated the MCP Integration Guide to document Unix socket and SSE transports in addition to STDIO and HTTP, adding configuration examples, best-practice guidance, and troubleshooting steps for both transports. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/MCP_INTEGRATION_GUIDE.md`:
- Around line 125-127: The doc currently lists `url`, `socket_path`, and
`headers` without stating which are required; update the
MCP_INTEGRATION_GUIDE.md to explicitly mark which fields are required vs
optional to match runtime behavior in core/framework/runner/mcp_client.py: mark
`url` as optional (defaulting to "http://localhost" when omitted) and
`socket_path` and `headers` as optional if that matches code, and explicitly
mark the SSE `url` (the stream endpoint used by the SSE logic in mcp_client.py)
as required; ensure the descriptions note the default for `url` and call out
that omitting the SSE `url` will cause runtime errors so it must be provided.
🪄 Autofix (Beta)
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
Run ID: 5b4649a0-ae55-454b-8733-700a15db37ea
📒 Files selected for processing (1)
core/MCP_INTEGRATION_GUIDE.md
| - `url`: Base URL for HTTP requests over the socket (e.g., `"http://localhost"`) | ||
| - `socket_path`: Absolute path to the Unix socket file (e.g., `"/tmp/mcp_server.sock"`) | ||
| - `headers`: HTTP headers to include (optional) |
There was a problem hiding this comment.
Clarify required vs optional fields to match runtime behavior.
The new sections describe fields but don’t explicitly mark required/optional status, and Unix url is presented like a required input though implementation defaults to http://localhost when omitted (core/framework/runner/mcp_client.py:235-265). Also, SSE url is required in code (core/framework/runner/mcp_client.py:301-335).
✏️ Proposed doc clarification
- - `url`: Base URL for HTTP requests over the socket (e.g., `"http://localhost"`)
- - `socket_path`: Absolute path to the Unix socket file (e.g., `"/tmp/mcp_server.sock"`)
- - `headers`: HTTP headers to include (optional)
+ - `socket_path`: Absolute path to the Unix socket file (required)
+ - `url`: Base URL for HTTP requests over the socket (optional, defaults to `"http://localhost"`)
+ - `headers`: HTTP headers to include (optional)
- - `url`: SSE endpoint URL (e.g., `"http://localhost:8000/sse"`)
- - `headers`: HTTP headers for the SSE connection (optional)
+ - `url`: SSE endpoint URL (required)
+ - `headers`: HTTP headers for the SSE connection (optional)Also applies to: 146-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/MCP_INTEGRATION_GUIDE.md` around lines 125 - 127, The doc currently
lists `url`, `socket_path`, and `headers` without stating which are required;
update the MCP_INTEGRATION_GUIDE.md to explicitly mark which fields are required
vs optional to match runtime behavior in core/framework/runner/mcp_client.py:
mark `url` as optional (defaulting to "http://localhost" when omitted) and
`socket_path` and `headers` as optional if that matches code, and explicitly
mark the SSE `url` (the stream endpoint used by the SSE logic in mcp_client.py)
as required; ensure the descriptions note the default for `url` and call out
that omitting the SSE `url` will cause runtime errors so it must be provided.
Description
The MCP client supports four transport types (
stdio,http,unix,sse),but the MCP Integration Guide only documents STDIO and HTTP. Unix and SSE were
added in PR #6531 but docs weren't updated.
Type of Change
Related Issues
Fixes #6739
Changes Made
Unix Socket TransportandSSE Transportsubsections with configuration fields, required/optional status, and examples
(same-host IPC) and SSE (streaming/real-time) over HTTP/STDIO
connection issues
Testing
uv run pytest core/tests/test_mcp_client.py -v)MCPServerConfigdataclass incore/framework/runner/mcp_client.pyChecklist
Summary by CodeRabbit