Skip to content

Resolve isHTTPConnectionError TODO and update MCPProtocolVersion to 2025-11-25#1619

Merged
lpcox merged 2 commits intomainfrom
copilot/review-go-sdk-module
Mar 6, 2026
Merged

Resolve isHTTPConnectionError TODO and update MCPProtocolVersion to 2025-11-25#1619
lpcox merged 2 commits intomainfrom
copilot/review-go-sdk-module

Conversation

Copy link
Contributor

Copilot AI commented Mar 6, 2026

Two quick wins from the go-sdk module review: replace fragile string-based connection error detection with idiomatic errors.As, and align the plain JSON-RPC transport's protocol version with the current MCP spec.

MCPProtocolVersion: "2024-11-05""2025-11-25"

The plain JSON-RPC fallback transport was advertising the old protocol version in initialize requests. Updated the single source-of-truth constant in internal/mcp/http_transport.go; internal/server/unified.go re-exports it via mcp.MCPProtocolVersion and picks up the change automatically.

isHTTPConnectionError: string matching → errors.As

Replaced the three-string strings.Contains heuristic with a proper type assertion:

// Before
errMsg := err.Error()
return strings.Contains(errMsg, "connection refused") ||
    strings.Contains(errMsg, "no such host") ||
    strings.Contains(errMsg, "network is unreachable")

// After
var opErr *net.OpError
if errors.As(err, &opErr) {
    return opErr.Op == "dial"
}
return false

Op == "dial" covers all connection-time failures (refused, DNS, unreachable) while correctly excluding post-connection errors (read/write). errors.As unwraps error chains automatically, so *url.Error*net.OpError works without extra handling.

Tests updated to use real *net.OpError instances, including a case for wrapped errors and a negative case for non-dial op errors.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build197167032/b301/launcher.test /tmp/go-build197167032/b301/launcher.test -test.testlogfile=/tmp/go-build197167032/b301/testlog.txt -test.paniconexit0 -test.timeout=10m0s --un�� fips140/aes/gcm --scope x_amd64/vet --property=Memor/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/vet 3628313/b095/ --property=MemoryHigh=170M x_amd64/vet push�� ache/go/1.25.7/x64/src/runtime/cgo origin x_amd64/vet (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build197167032/b283/config.test /tmp/go-build197167032/b283/config.test -test.testlogfile=/tmp/go-build197167032/b283/testlog.txt -test.paniconexit0 -test.timeout=10m0s ortc�� g_.a 64/src/crypto/ingithub.com/github/gh-aw-mcpg/internal/config/rules ache/go/1.25.7/x-lang=go1.25 (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build197167032/b301/launcher.test /tmp/go-build197167032/b301/launcher.test -test.testlogfile=/tmp/go-build197167032/b301/testlog.txt -test.paniconexit0 -test.timeout=10m0s --un�� fips140/aes/gcm --scope x_amd64/vet --property=Memor/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/vet 3628313/b095/ --property=MemoryHigh=170M x_amd64/vet push�� ache/go/1.25.7/x64/src/runtime/cgo origin x_amd64/vet (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build197167032/b301/launcher.test /tmp/go-build197167032/b301/launcher.test -test.testlogfile=/tmp/go-build197167032/b301/testlog.txt -test.paniconexit0 -test.timeout=10m0s --un�� fips140/aes/gcm --scope x_amd64/vet --property=Memor/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/vet 3628313/b095/ --property=MemoryHigh=170M x_amd64/vet push�� ache/go/1.25.7/x64/src/runtime/cgo origin x_amd64/vet (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build197167032/b310/mcp.test /tmp/go-build197167032/b310/mcp.test -test.testlogfile=/tmp/go-build197167032/b310/testlog.txt -test.paniconexit0 -test.timeout=10m0s 3628�� ache/go/1.25.7/x-errorsas .cfg x_amd64/compile -p internal/godebug--version -lang=go1.25 x_amd64/compile 3628�� 64/src/net -trimpath x_amd64/vet -p crypto/internal//usr/bin/runc -lang=go1.25 IcXyQNR/PHvijypvdfhvgj9aAq4s (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…n to 2025-11-25

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Review Go SDK for modelcontextprotocol Resolve isHTTPConnectionError TODO and update MCPProtocolVersion to 2025-11-25 Mar 6, 2026
Copilot finished work on behalf of lpcox March 6, 2026 12:20
@lpcox lpcox marked this pull request as ready for review March 6, 2026 12:30
Copilot AI review requested due to automatic review settings March 6, 2026 12:30
@lpcox lpcox merged commit a1284a5 into main Mar 6, 2026
14 checks passed
@lpcox lpcox deleted the copilot/review-go-sdk-module branch March 6, 2026 12:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the MCP HTTP transport to advertise the current MCP protocol version and makes HTTP connection error detection more robust by switching from string matching to typed error inspection.

Changes:

  • Bumped internal/mcp/http_transport.go’s MCPProtocolVersion from 2024-11-05 to 2025-11-25.
  • Replaced fragile string matching in isHTTPConnectionError with errors.As against *net.OpError and a dial-operation check.
  • Updated unit tests to construct real *net.OpError instances (including a wrapped-error scenario and a non-dial negative case).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/mcp/http_transport.go Updates protocol version constant and switches connection error classification to errors.As(*net.OpError)
internal/mcp/connection_test.go Updates tests for the new typed connection error classification
Comments suppressed due to low confidence (1)

internal/mcp/http_transport.go:36

  • The comment says this protocol version is used in initialization requests generally, but in this file it’s only applied to the plain JSON-RPC fallback (initializeHTTPSession). Consider tightening the comment to clarify the scope (plain JSON transport) so readers don’t assume it affects the SDK-backed streamable/SSE transports as well.
// MCPProtocolVersion is the MCP protocol version used in initialization requests.
const MCPProtocolVersion = "2025-11-25"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 35 to +36
// MCPProtocolVersion is the MCP protocol version used in initialization requests.
const MCPProtocolVersion = "2024-11-05"
const MCPProtocolVersion = "2025-11-25"
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MCPProtocolVersion is used as a single source of truth, but there are still many docs/tests/examples in the repo that hard-code the old value (2024-11-05). To avoid confusing drift, update those references (or have tests/docs refer to this constant where feasible) so the documented/example protocolVersion matches what the code now sends.

This issue also appears on line 35 of the same file.

Copilot uses AI. Check for mistakes.
Comment on lines +664 to +667
name: "wrapped net.OpError dial via fmt.Errorf",
err: fmt.Errorf("Post %q: %w", "http://example.com", &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connection refused")}),
expected: true,
},
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “wrapped” test case uses fmt.Errorf(...%w...), but http.Client.Do typically wraps dial failures in *url.Error. If the goal is to validate the real error chain that executeHTTPRequest will see, use a url.Error wrapper here so the test covers *url.Error -> *net.OpError unwrapping explicitly.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants