Skip to content

Close mcpcompat functional gaps vs mcp-go for a loss-free migration #156

Description

@JAORMX

Context

stacklok/toolhive#5729 migrates ToolHive from mark3labs/mcp-go v0.55.1 onto the mcpcompat shim (introduced in #150, released in v0.0.27, backed by modelcontextprotocol/go-sdk v1.6.1). A code-level audit of the shim against ToolHive's actual mcp-go usage found functional regressions — some disclosed in that PR, some not. ToolHive wants to complete the migration (it is the path to MCP 2026-07-28 / stateless support via go-sdk v1.7.x) without accepting any functional loss.

We verified that no newer go-sdk release helps: latest stable is v1.6.1, latest overall is v1.7.0-pre.1, and the relevant API surface is identical in both. Every fix below is therefore shim-side work against v1.6.1, except where noted as an upstream ask.

Audit findings (shim v0.0.27, file references are mcpcompat/)

# Finding Status Evidence
1a Client never registers ProgressNotificationHandler / LoggingMessageHandler (both exist in go-sdk v1.6.1 ClientOptions) — only the 3 list_changed handlers are wired. Combined with 1b, no server-initiated notification survives the ToolHive stdio bridge end-to-end Lost — shim bug client.go:417-427
1b Server side forwards only progress + message notifications; tools/list_changed and everything else is dropped (go-sdk has no public generic notification sender) Lost — upstream API gap notifications.go:32-76
2 WithHeartbeatInterval is a silent no-op: the interval is stored and never used; go-sdk StreamableHTTPOptions has no keep-alive. Long-lived SSE streams get no pings and can be dropped by LB idle timeouts Lost — undisclosed transports.go:85,142-143
3 SessionIdManager is largely wired (Generate via GetSessionID, Terminate via DELETE, Validate on cross-replica rehydration) but per-request Validate is skipped for local sessions — mcp-go validated every request. A session terminated cross-pod or via auth-failure can keep being served by the origin pod Degraded server.go:334-336, transports.go:154,199-208,216,233
4 401/auth errors are string-matched ("401", "unauthorized"); extractResourceMetadataURL always returns ""; 403-on-initialize is not mapped to ErrLegacySSEServer. The sentinels ToolHive consumes are produced, so impact is low today Degraded errors.go:66-93
5 go-sdk enables DNS-rebinding/localhost protection by default; the shim does not expose DisableLocalhostProtection. Local proxies with custom Host headers now get 403 (mcp-go had no such protection) Behavior change transports.go:166,170
6 go-sdk paginates tools/list at DefaultPageSize=1000; mcp-go returned everything. Non-cursor-following clients see silent truncation past 1000 tools Behavior change go-sdk server.go:35-36
7 Per-session dynamic tool projection (security-relevant for vMCP): genuinely implemented — per-session go-sdk Server + live AddTool/RemoveTools reconciliation Kept (needs runtime validation) server.go:298,375-423, session.go:122-239
8 Doc comments claim progress/logging notifications are forwarded (they are not, see 1a) and the WithSessionIdManager comment says it is unwired (it mostly is wired) Docs wrong client.go, notifications.go, transports.go:135-136

Work items

Shim fixes (target: v0.0.28)

  • 1a. Register client notification handlers — wire go-sdk's ProgressNotificationHandler + LoggingMessageHandler into the mcpcompat client and route to the compat notification callback. Cheapest fix, largest correctness gain.
  • 1b. list_changed via feature re-sync — instead of forwarding the raw notification (impossible without a generic sender), re-sync the mirrored tools/resources/prompts on upstream list_changed; go-sdk auto-emits list_changed to clients on feature mutation. This restores (and arguably improves) mcp-go semantics without waiting on upstream.
  • 2. Implement keep-alive shim-side — honor WithHeartbeatInterval by injecting SSE comment lines (: keepalive) on a ticker into text/event-stream responses via a ResponseWriter wrapper around the go-sdk handler. SSE comments are spec-legal and ignored by clients.
  • 3. Per-request Validate for local sessionsMcp-Session-Id is visible at the shim's HTTP layer before delegation to the go-sdk handler; call Validate on every request regardless of session locality, restoring mcp-go semantics (cross-pod/auth-failure termination takes effect immediately).
  • 4. Adopt StreamableClientTransport.OAuthHandler — replace 401 string-matching with go-sdk's native OAuth flow hook (exists in v1.6.1); populate AuthorizationRequiredError.ResourceMetadataURL; map 403-on-initialize to ErrLegacySSEServer.
  • 5. Expose DisableLocalhostProtection — plumb the option through shim config so callers can preserve pre-migration behavior for local proxies (default should keep protection on; ToolHive opts out only where it fronts localhost with custom Host headers).
  • 6. Configurable server page size — expose go-sdk's page size so aggregating servers (vMCP) can raise it; document that clients must follow cursors (client-side cursor loops are tracked in ToolHive, not here).
  • 7. Fix the false doc comments from finding 8.

Upstream asks (file against modelcontextprotocol/go-sdk, non-blocking)

  • Public generic notification sender on ServerSession (unblocks true notification passthrough; only typed NotifyProgress/Log exist today).
  • Server-side keep-alive option in StreamableHTTPOptions.
  • Typed HTTP-status errors from transports (removes string-matching).

Acceptance criteria

  • ToolHive adds regression tests (tracked in stacklok/toolhive) that fail against v0.0.27 and pass against v0.0.28: bridge end-to-end forwarding of progress/message/list_changed, cross-pod session termination taking effect on the origin pod, SSE keep-alive presence on idle streams, >1000-tool listing behavior.
  • feat: migrate from mark3labs/mcp-go to toolhive-core/mcpcompat (go-sdk) toolhive#5729 bumps to the fixed release and merges with those tests green — i.e. the mcp-go → go-sdk migration lands with zero functional loss.
  • Runtime validation of per-session tool projection (finding 7) performed as part of the ToolHive-side test work.

Why now

The 2026-07-28 stateless MCP revision ships this month; go-sdk v1.7.x is the only Go SDK line with support for it. The shim is the bridge that lets ToolHive get there — these gaps are the only thing standing between us and a loss-free migration.

Generated with Claude Code

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions