chore: upgrade go-sdk to v1.5.0 and address go-fan review items#3610
chore: upgrade go-sdk to v1.5.0 and address go-fan review items#3610
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/09a95415-798b-406d-9dbf-101e2e388180 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…, route SSE warnings to structured logger, update tool_registry comment Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/09a95415-798b-406d-9dbf-101e2e388180 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Upgrades the MCP Go SDK dependency to v1.5.0 and hardens/aligns gateway behavior with the newer SDK by using typed session-missing detection and ensuring SSE deprecation warnings flow through the gateway’s logging pipeline.
Changes:
- Bump
github.com/modelcontextprotocol/go-sdkto v1.5.0 (and refresh transitive sums). - Use
errors.Is(err, sdk.ErrSessionMissing)for session-missing detection with a string-match fallback. - Route SSE deprecation warnings through
logger.LogWarnand update the tool-registration comment to reflect v1.5.0Server.AddToolbehavior.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Upgrades MCP Go SDK requirement to v1.5.0. |
| go.sum | Updates checksums for the SDK upgrade and related transitive deps. |
| internal/mcp/http_transport.go | Switches session-missing detection to a typed sentinel with fallback string matching. |
| internal/mcp/connection.go | Sends SSE deprecation warnings through structured logging instead of log.Printf. |
| internal/server/tool_registry.go | Updates documentation comment about Server.AddTool vs sdk.AddTool behavior in v1.5.0. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/5 changed files
- Comments generated: 2
| func isSessionNotFoundError(err error) bool { | ||
| if err == nil { | ||
| return false | ||
| } | ||
| if errors.Is(err, sdk.ErrSessionMissing) { | ||
| return true | ||
| } |
There was a problem hiding this comment.
The new errors.Is(err, sdk.ErrSessionMissing) branch isn’t covered by the existing TestIsSessionNotFoundError cases (which only exercise string matching). Add a test that passes sdk.ErrSessionMissing (and ideally a wrapped %w form) so this typed-sentinel detection is validated and won’t silently regress on future SDK upgrades.
| log.Printf("Configured HTTP MCP server with SSE transport: %s", url) | ||
| logger.LogWarn("backend", "⚠️ WARNING: MCP over SSE (2024-11-05 spec) has been DEPRECATED") | ||
| logger.LogWarn("backend", "⚠️ The server at %s is using the deprecated SSE transport", url) | ||
| logger.LogWarn("backend", "⚠️ Please migrate to streamable HTTP transport (2025-03-26 spec)") |
There was a problem hiding this comment.
logConn.Printf is DEBUG-gated (and logs at debug level to the file logger), so this message will no longer be visible by default (unlike the previous log.Printf) and may not land in mcp-gateway.log at WARN/INFO. If this line is intended as part of the SSE deprecation/operator visibility, route it through the structured logger too (e.g., logger.LogWarn/LogInfo) for consistency with the other deprecation messages.
| logger.LogWarn("backend", "⚠️ Please migrate to streamable HTTP transport (2025-03-26 spec)") | |
| logger.LogWarn("backend", "⚠️ Please migrate to streamable HTTP transport (2025-03-26 spec)") | |
| logger.LogWarn("backend", "Configured HTTP MCP server with SSE transport: %s", url) |
…E warning - Add test cases for sdk.ErrSessionMissing sentinel and wrapped form to TestIsSessionNotFoundError, ensuring typed-sentinel detection won't silently regress on future SDK upgrades. - Replace debug-gated logConn.Printf with logger.LogWarn for the SSE transport configuration message, ensuring operator visibility in mcp-gateway.log at WARN level. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The project was on
go-sdkv1.4.1 while v1.5.0 was available, and two fragility concerns were identified: session expiry detection via string matching and SSE deprecation warnings bypassing the structured logger.Changes
go.mod/go.sum— upgradegithub.qkg1.top/modelcontextprotocol/go-sdkv1.4.1 → v1.5.0internal/mcp/http_transport.go— replace string-matching session error detection with the typed sentinel introduced in v1.5.0, retaining string-match as fallback for non-SDK (plain JSON-RPC) transports:internal/mcp/connection.go— route SSE deprecationlog.Printfcalls throughlogger.LogWarnso they appear inmcp-gateway.loginstead of bypassing the structured logging pipelineinternal/server/tool_registry.go— updateregisterToolWithoutValidationcomment to accurately reflect v1.5.0Server.AddToolbehaviour: now enforcestype: "object"on the input schema but still does not validate argument values at call time; notes verification against v1.5.0 sourceWarning
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/tmp/go-build347415617/b510/launcher.test /tmp/go-build347415617/b510/launcher.test -test.testlogfile=/tmp/go-build347415617/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/net -trimpath de/node/bin/as -p io -lang=go1.25 o_main.o -I uf@v1.36.11/refl-errorsas uf@v1.36.11/refl-ifaceassert x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build2356109318/b510/launcher.test /tmp/go-build2356109318/b510/launcher.test -test.testlogfile=/tmp/go-build2356109318/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true eFSy5PFyu cfg 64/pkg/tool/linux_amd64/vet --gdwarf-5 --64 -o 64/pkg/tool/linu--others pkg/�� om/davecgh/go-spew@v1.1.1/spew/bypass.go cfg 64/pkg/tool/linux_amd64/vet -plugin-opt=-pasbash g/grpc/internal/--norc -plugin-opt=-pas--noprofile 64/pkg/tool/linux_amd64/vet(dns block)/tmp/go-build2885812085/b514/launcher.test /tmp/go-build2885812085/b514/launcher.test -test.testlogfile=/tmp/go-build2885812085/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool -buildtags iginal -errorsas -ifaceassert -nilfunc iginal -ato�� /launcher/connection_pool.go /launcher/health_monitor.go x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build347415617/b492/config.test /tmp/go-build347415617/b492/config.test -test.testlogfile=/tmp/go-build347415617/b492/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true o /home/REDACTED/go/pkg/mod/github.c--64 x_amd64/compile /home/REDACTED/go//opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet fips140/ecdsa /home/REDACTED/go/-unreachable=false x_amd64/compile ual_�� azero@v1.11.0/internal/wasmdebug-p azero@v1.11.0/internal/wasmdebuggolang.org/x/net/http2 x_amd64/asm -p crypto/internal/-atomic -lang=go1.25 x_amd64/asm(dns block)/tmp/go-build2885812085/b496/config.test /tmp/go-build2885812085/b496/config.test -test.testlogfile=/tmp/go-build2885812085/b496/testlog.txt -test.paniconexit0 -test.timeout=10m0s -uns�� -unreachable=false /tmp/go-build347415617/b075/vet.cfg ash g_.a gmYLA_3tW .13/x64/as /opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linu--others -ato�� -bool -buildtags 86_64/git -errorsas -ifaceassert -nilfunc iginal(dns block)nonexistent.local/tmp/go-build347415617/b510/launcher.test /tmp/go-build347415617/b510/launcher.test -test.testlogfile=/tmp/go-build347415617/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/net -trimpath de/node/bin/as -p io -lang=go1.25 o_main.o -I uf@v1.36.11/refl-errorsas uf@v1.36.11/refl-ifaceassert x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build2356109318/b510/launcher.test /tmp/go-build2356109318/b510/launcher.test -test.testlogfile=/tmp/go-build2356109318/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true eFSy5PFyu cfg 64/pkg/tool/linux_amd64/vet --gdwarf-5 --64 -o 64/pkg/tool/linu--others pkg/�� om/davecgh/go-spew@v1.1.1/spew/bypass.go cfg 64/pkg/tool/linux_amd64/vet -plugin-opt=-pasbash g/grpc/internal/--norc -plugin-opt=-pas--noprofile 64/pkg/tool/linux_amd64/vet(dns block)/tmp/go-build2885812085/b514/launcher.test /tmp/go-build2885812085/b514/launcher.test -test.testlogfile=/tmp/go-build2885812085/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool -buildtags iginal -errorsas -ifaceassert -nilfunc iginal -ato�� /launcher/connection_pool.go /launcher/health_monitor.go x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet(dns block)slow.example.com/tmp/go-build347415617/b510/launcher.test /tmp/go-build347415617/b510/launcher.test -test.testlogfile=/tmp/go-build347415617/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/net -trimpath de/node/bin/as -p io -lang=go1.25 o_main.o -I uf@v1.36.11/refl-errorsas uf@v1.36.11/refl-ifaceassert x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build2356109318/b510/launcher.test /tmp/go-build2356109318/b510/launcher.test -test.testlogfile=/tmp/go-build2356109318/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true eFSy5PFyu cfg 64/pkg/tool/linux_amd64/vet --gdwarf-5 --64 -o 64/pkg/tool/linu--others pkg/�� om/davecgh/go-spew@v1.1.1/spew/bypass.go cfg 64/pkg/tool/linux_amd64/vet -plugin-opt=-pasbash g/grpc/internal/--norc -plugin-opt=-pas--noprofile 64/pkg/tool/linux_amd64/vet(dns block)/tmp/go-build2885812085/b514/launcher.test /tmp/go-build2885812085/b514/launcher.test -test.testlogfile=/tmp/go-build2885812085/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool -buildtags iginal -errorsas -ifaceassert -nilfunc iginal -ato�� /launcher/connection_pool.go /launcher/health_monitor.go x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet(dns block)this-host-does-not-exist-12345.com/tmp/go-build347415617/b519/mcp.test /tmp/go-build347415617/b519/mcp.test -test.testlogfile=/tmp/go-build347415617/b519/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 1.80.0/internal/channelz/channel.go 1.80.0/internal/channelz/channelmap.go x_amd64/vet 305_amd64.s --64 -o x_amd64/vet 0880�� m/grpc-gateway/v2@v2.28.0/runtim-errorsas m/grpc-gateway/v2@v2.28.0/runtim-ifaceassert x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build2356109318/b519/mcp.test /tmp/go-build2356109318/b519/mcp.test -test.testlogfile=/tmp/go-build2356109318/b519/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true olang.org/grpc@v-p cfg 64/pkg/tool/linu-lang=go1.25 0880581/b288/ --64(dns block)/tmp/go-build2885812085/b523/mcp.test /tmp/go-build2885812085/b523/mcp.test -test.testlogfile=/tmp/go-build2885812085/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet /usr�� --version -tests x_amd64/vet balancer/weight/grep(dns block)If you need me to access, download, or install something from one of these locations, you can either: