-
Notifications
You must be signed in to change notification settings - Fork 20
[go-fan] Go Module Review: modelcontextprotocol/go-sdk #3313
Description
🐹 Go Fan Report: modelcontextprotocol/go-sdk
Module Overview
github.com/modelcontextprotocol/go-sdk (v1.4.1) is the official Go implementation of the Model Context Protocol. It provides both server-side and client-side implementations including typed API structs, multiple transport types (stdio, StreamableHTTP, SSE, in-memory), session lifecycle management, and built-in slog integration.
This is the most central dependency in gh-aw-mcpg — it is the protocol layer that the entire gateway is built upon.
Current Usage in gh-aw
- Files: 26 files across
internal/mcp/,internal/server/,internal/testutil/mcptest/, and integration tests - Import Count: 26 imports (
sdk "github.com/modelcontextprotocol/go-sdk/mcp") - Key APIs Used:
sdk.NewServer/sdk.NewClient— gateway server + backend clientssdk.CommandTransport— stdio process transport for backend MCP serverssdk.StreamableClientTransport/sdk.SSEClientTransport— HTTP client transportssdk.NewStreamableHTTPHandler— agent-facing HTTP handlersdk.ClientSession.ListTools/CallTool/ListResources/ etc. — backend protocol callssdk.NewInMemoryTransports— in-process transport for testssdk.ClientOptions.KeepAlive— keepalive pings for HTTP backends
Architecture: Dual Role
The gateway plays two roles with the SDK simultaneously:
- MCP Server (facing LLM agents):
sdk.NewServer+sdk.NewStreamableHTTPHandlerexpose tools via StreamableHTTP - MCP Client (facing backend servers):
sdk.NewClient+ transports connect to Docker-containerized MCP backends
Transport Fallback Chain
For HTTP backends, the gateway tries transports in priority order:
StreamableClientTransport (2025-03-26 spec) → 5s probe timeout
↓ fail
SSEClientTransport (2024-11-05 spec, deprecated) → log deprecation warning
↓ fail
Plain JSON-RPC over HTTP POST → custom implementation
Research Findings
Notable Design Patterns
Schema Bypass via Internal API: registerToolWithoutValidation calls server.AddTool() (instance method) rather than sdk.AddTool() (package function) to bypass JSON Schema validation. This allows backend tool schemas in JSON Schema draft-07 format to be registered without validation errors. The code includes this comment:
"This distinction relies on internal SDK behaviour and must be re-verified on every SDK upgrade."
Generic Pagination Helper: paginateAll[T] collects all pages from ListTools, ListResources, ListPrompts with a 100-page guard against runaway backends. Well-designed use of Go generics.
Custom 3-arg Handler Wrapper: Tool handlers use (ctx, req, state) → (result, data, error) instead of the SDK's (ctx, req) → (result, error), with an adapter layer. The extra state and data parameters support DIFC middleware and write-sink logging.
Session Caching in Routed Mode: filteredServerCache caches *sdk.Server instances per (backendID, sessionID) pair with TTL-based lazy eviction, preventing session fragmentation.
Protocol Version Status
The gateway implements MCPProtocolVersion = "2025-11-25" for initialization requests and uses the 2025-03-26 streamable HTTP transport as primary. SSE transport (2024-11-05) is detected and logged as deprecated.
Improvement Opportunities
🏃 Quick Wins
-
Schema bypass test:
registerToolWithoutValidationrelies on internal SDK behavior per the comment. Add a simple unit test (or assertion at startup) that detects if the SDK version has changed this behavior — e.g., register a tool with an intentionally invalid schema and verify it succeeds viaserver.AddToolbut would fail viasdk.AddTool. This creates a canary for SDK upgrades. -
Hardcoded response ID:
marshalToResponseusesID: 1as a placeholder for all SDK-to-internal-Response conversions. Add a comment explaining that this is safe because the SDK'ssession.XXX()calls are already fully resolved at this point — the ID is never used for matching. This prevents future maintainers from flagging it as a correlation bug. -
Transport probe timeout: The 5-second timeout for transport detection (
trySDKTransport) is hardcoded. Consider addingtransport_probe_timeouttoServerConfigfor backends that need more connection time.
✨ Feature Opportunities
-
Surface
InitializeResultcapabilities:session.InitializeResult()is already used invalidator.goto getserverInfo. Consider emitting backend capabilities (server name, version, supported features) to the operational log during startup — useful for debugging compatibility issues between the gateway and specific backends. -
Expand
InMemoryTransportstest coverage: Themcptestpackage usessdk.NewInMemoryTransportsfor efficient in-process testing. Integration tests intest/integration/that currently require the compiled binary could be restructured to use in-memory transports for a subset of scenarios, improving test speed and reliability. -
AddResource/AddPromptgateway exposure: The gateway proxies resources and prompts from backends viacallSDKMethoddispatch, but these are not surfaced through the gateway's ownsdk.Server. If agent-facing resource/prompt access is needed, the SDK'sAddResource/AddPromptpatterns (already demonstrated inmcptest/server.go) could be applied toUnifiedServer.
📐 Best Practice Alignment
-
Watch for
SkipValidationSDK option: The schema validation bypass viaserver.AddTool()vssdk.AddTool()is a known fragility. Monitor future SDK releases for an explicitSkipValidationorAllowAnyInputSchemaoption that would replace the internal dependency. File this in the SDK upgrade checklist. -
SSE transport removal planning: The gateway correctly warns when SSE transport is detected. Consider adding a configuration option to disable SSE fallback entirely, which would force backends to upgrade and reduce code surface.
-
Local
CallToolParamstype:mcp/types.godefines a localCallToolParamswithArguments map[string]interface{}. The marshal/unmarshal roundtrip incallParamMethodbridges this to the SDK's typed params. A comment intypes.goexplaining why a local type is maintained would help future maintainers understand the design intent.
🔧 General Improvements
-
callSDKMethodmethod registry: The switch on method strings incallSDKMethodis a manual routing table. Adding new MCP methods (e.g.,sampling/createMessageif the SDK adds support) requires updating this switch. Consider amap[string]funcdispatch table or at minimum an"unsupported method"log/metric for discoverability. -
Connection error context: The
LogConnectionErrorfunction inconnection.goprovides rich diagnostic context for failed backend connections. Consider applying the same pattern to session reconnect failures inreconnectSDKTransportandreconnectPlainJSON.
Recommendations (Prioritized)
| Priority | Item | Effort | Impact |
|---|---|---|---|
| 🔴 High | Schema bypass canary test (SDK upgrade safety) | Low | High |
| 🟡 Medium | Surface InitializeResult in startup logs |
Low | Medium |
| 🟡 Medium | Document placeholder response ID | Low | Low |
| 🟢 Low | Transport probe timeout configurability | Medium | Low |
| 🟢 Low | SSE fallback disable option | Medium | Low |
| 🟢 Low | Expand InMemoryTransports test coverage | High | Medium |
Next Steps
- Add schema bypass canary test to
internal/server/tool_registry_test.go - Log
InitializeResultserver info during backend registration inregisterToolsFromBackend - Add comment to
marshalToResponseexplainingID: 1placeholder safety - Add SDK upgrade checklist item to track
SkipValidationoption availability
Generated by Go Fan 🐹
Module summary saved to: docs/go-sdk-review.md
Workflow run: §24070033091
Note
🔒 Integrity filter blocked 18 items
The following items were blocked because they don't meet the GitHub integrity level.
- https://github.com/modelcontextprotocol/go-sdk/releases/tag/v1.4.1
get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - https://github.com/spf13/cobra/releases/tag/v1.10.2
get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - https://github.com/santhosh-tekuri/jsonschema/releases/tag/v6.0.2
get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - https://github.com/itchyny/gojq/releases/tag/v0.12.19
get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - https://github.com/stretchr/testify/releases/tag/v1.11.1
get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - https://github.com/wazero/wazero/releases/tag/v1.11.0
get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - https://github.com/open-telemetry/opentelemetry-go/releases/tag/v1.43.0
get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - https://github.com/BurntSushi/toml/releases/tag/v1.6.0
get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - BurntSushi/toml@8685fba
list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - santhosh-tekuri/jsonschema@180cde3
list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - itchyny/gojq@b7ebffb
list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - modelcontextprotocol/go-sdk@862d78a
list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - spf13/cobra@61968e8
list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - stretchr/testify@5f80e4a
list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - open-telemetry/opentelemetry-go@a00b377
list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - wazero/wazero@929e400
list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved". - ... and 2 more items
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Go Fan · ● 3.8M · ◷
- expires on Apr 14, 2026, 7:47 AM UTC