Fix mcptool package doc: it claims a WebSocket transport the MCP go-sdk does not provide#755
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the tool/mcptool package documentation to remove an inaccurate claim that MCP servers can be connected to via WebSocket, aligning the doc with the transports available in the pinned MCP Go SDK. It also adds a regression test to prevent reintroducing “WebSocket” mentions in non-test Go sources for the package.
Changes:
- Update the
mcptoolpackage doc to describe supported connection transports as stdio (subprocess) and HTTP (SSE / streamable HTTP). - Add
TestPackageDocNoWebSocketto fail if any non-test.gofile intool/mcptoolcontains the substring “websocket”.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tool/mcptool/mcp.go | Adjusts package-level documentation to remove the WebSocket transport claim. |
| tool/mcptool/mcp_test.go | Adds a regression test scanning non-test sources for “websocket” mentions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Package mcp provides integration with the Model Context Protocol (MCP). | ||
| // It allows agents to connect to external MCP servers via stdio, HTTP, or WebSocket | ||
| // and expose their tools and prompts as agent.Tool instances. | ||
| // It allows agents to connect to external MCP servers via stdio (subprocess) | ||
| // or HTTP (SSE / streamable HTTP) and expose their tools and prompts as | ||
| // agent.Tool instances. |
There was a problem hiding this comment.
Good catch — fixed in e42f4f2: the package doc no longer claims prompt support and now says it wraps MCP tools as tool.Tool / tool.FuncTool (there is no agent.Tool type here).
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
parity-approved
Connect only wraps an mcp.Transport supplied by the pinned go-sdk, which exposes stdio and HTTP (SSE / streamable HTTP) transports but no WebSocket transport, so the package doc's "stdio, HTTP, or WebSocket" claim is inaccurate. Describe the transports that are actually reachable and add a regression guard test.
e42f4f2 to
588cdb1
Compare
Parity Review — PR #755Scope: Documentation-only fix ( Finding: No exported Go API changes. The PR corrects a misleading doc claim that the Verdict: ✅ No cross-repo parity issues. The change improves documentation consistency with upstream .NET/Python rather than diverging from it. No exported API surface was modified.
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
What
The
mcptoolpackage doc comment claimed agents could connect to MCP servers "via stdio, HTTP, or WebSocket".Connectonly wraps anmcp.Transportsupplied by the pinned go-sdk (v1.6.1), and that SDK provides no WebSocket transport — the onlyTransportimplementations areCommandTransport,IOTransport,InMemoryTransport,LoggingTransport,SSEClientTransport,SSEServerTransport,StdioTransport,StreamableClientTransport, andStreamableServerTransport. The reachable client transports for external servers are stdio (subprocess) and HTTP (SSE / streamable HTTP) only.This updates the doc to describe the transports that are actually available:
Why
Aligns the doc with the SDK surface, matching the .NET/Python MCP client wording, which likewise scopes advertised transports to stdio and HTTP-based transports (SSE / streamable HTTP) rather than WebSocket. A reader who trusted the old comment would look for a WebSocket transport that cannot be constructed or passed to
Connect.Scope is intentionally limited to the WebSocket claim; the "and prompts" clause (#628) and the
agent.Toolwording (#685) are left to their respective PRs.Testing
Doc-only change. Added
TestPackageDocNoWebSocketinmcp_test.goas a regression guard that scans the package's non-test.gosources and fails if any mention "websocket". It fails on the old wording and passes on the new.go build ./...,go vet ./tool/mcptool/..., andgo test ./tool/mcptool/...all pass.