Repository navigation
feat(058): adopt mcp-go v1.0.0, pinned inert on both protocol hops - #1204
Merged
Merged
Conversation
Related #532 Spec 058 Phases 1-2. The library now implements MCP 2026-07-28, so this change adopts it while leaving the wire behaviour byte-identical in both directions, and fixes two latent defects the bump exposed. ## The bump is not the risky part; the constant it redefines is v1.0.0 redefines mcp.LATEST_PROTOCOL_VERSION from 2025-11-25 to 2026-07-28, and client.Initialize became modern-first. internal/upstream/core sent that constant, so bumping the dependency alone would have switched every upstream hop to the new protocol era — where Ping returns success without sending anything, making the spec-074 health loop report dead upstreams as healthy, and where server-initiated requests are gone, so roots-based workspace discovery cannot work. FR-027 is amended to name both hops, and both are pinned here: - clientFacingStreamableOptions() pins the client-facing transports and is applied at all five NewStreamableHTTPServer sites. It is a function, not a package variable, so it cannot be flipped at runtime or from a test, and it is shared so a pin applied to four of five doors is not expressible. - The upstream handshake is pinned to mcp.LATEST_LEGACY_PROTOCOL_VERSION. Both are asserted on the wire rather than by reading the constants back — the whole failure mode here is a constant that changes underneath us, and a test comparing code to code would have passed straight through it. ## Two latent defects Tool-approval hashes (spec 032) are computed over the library-DECODED schema, because mcp.Tool.RawInputSchema is json:"-" and the upstream's bytes are discarded. v1.0.0 rewrites draft-07 "#/definitions/" refs to "#/$defs/", so a hash stored by the old decoder could not be reproduced by the new one, and the formula-migration hatch does not absorb it: every affected tool would be reported as changed — a rug-pull signal — after an upgrade that changed nothing upstream. NormalizeJSON now canonicalizes that spelling. The guard is narrow on purpose: it fires only when a document has a root $defs and no root definitions. NormalizeJSON is also applied to raw output schemas that reach the spec-056 validator, where a "#/definitions/" pointer is CORRECT and rewriting it would aim it at a member that does not exist. Second, both forwarding paths copied the upstream's mcp.Result envelope wholesale, which replays its ResultType. Since CallToolResult embeds MultiRoundTripResult separately, an upstream "input_required" reached the client stripped of the inputRequests and requestState needed to answer it — told to supply input, with no way to supply it and no way to know why. The envelope decision is now one shared proxyResultEnvelope() so the two paths cannot drift. ## Verification Full internal/server suite green, including TestProfile_SetProfileSessionScoped and TestProfile_SetProfileUnknown, which fail on v1.0.0 unpinned — the pin alone fixes them, with no change to profile code. -race green across hash, upstream, runtime and config. Server edition green. 14 tool-surface golden tests pass with no golden file regenerated. golangci-lint v2 with the CI config reports no issues; two mcp-go deprecations are suppressed with the reason and the follow-up task named inline. Each new guard was mutation-tested: reverting the normalizer, either pin, or the envelope fix makes the corresponding test fail.
Deploying mcpproxy-docs with
|
| Latest commit: |
9545083
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d9faa48a.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://feat-058-mcpgo-v1-bump.mcpproxy-docs.pages.dev |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 33781592023 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Related #532 Found by cross-review of the Phase 1-2 change, in code that change had just added. proxyResultEnvelope relayed the upstream's _meta wholesale. mcp-go stamps mcpproxy's own identity into an outgoing modern result only when the field is still absent (server/response.go decorateResult: `if meta.ServerInfo() == nil`), so relaying an upstream serverInfo does not merely add noise — it suppresses mcpproxy's identity and makes mcpproxy's own response claim to BE the upstream server. A client has no way to detect the misattribution. protocolVersion is hop-scoped for the same reason: the era negotiated upstream is not the era mcpproxy is answering on. Both keys are now filtered out of the relayed envelope. Everything else — trace context, progress tokens, upstream-specific fields — is preserved, and a _meta left with nothing relay-safe is dropped entirely rather than emitted empty, since an empty object still reads as "present" to the identity check. One earlier assertion is corrected too: the relayed _meta is deliberately a copy now, so asserting pointer identity was testing the wrong contract.
… and hashes Related #532 Four findings survived verification; two were real defects, two were claims of mine that overstated what the code guarantees. ## The upstream pin constrained the request, not the outcome initRequest.Params.ProtocolVersion controls what mcpproxy ASKS for. The era actually in force is whatever the upstream ANSWERS, and mcp-go accepts a modern answer to a legacy request: initializeLegacy validates only mcp.IsValidProtocolVersion, which is true for 2026-07-28, then applies it. So a server answering modern silently flipped the hop to the era the pin exists to avoid, taking Ping and server-initiated requests with it. Rejecting restores the pre-bump contract rather than inventing one: under v0.57.0 that version was absent from ValidProtocolVersions, so the same answer failed the handshake outright. A compliant server cannot trigger this, since it must answer with a version the client can speak. ## Only one of two hash normalizers was canonicalized internal/hash holds two normalizers over the same decoded schema, backing different hashes. NormalizeJSON feeds the Spec-032 approval hash and was fixed; canonicalSchemaFromBytes feeds ToolHash/ComputeToolHashWithOutputSchema — that is ToolMetadata.Hash, written by the upstream client — and was not. It still drifted across the bump, which is the exact defect the canonicalization exists to prevent, just on the other path. Both hash paths now carry guards. ## Two comments that claimed more than the code delivers - clientFacingStreamableOptions covers the Streamable HTTP surfaces only. mcp-go offers no stdio equivalent, and the era is chosen per request by the shared handler, so a stdio client can still opt into 2026-07-28. Not reachable by default (an empty listen address is rewritten to the HTTP default), and tracked with the US3 stateless work — but the doc comment implied coverage it does not have. - The proxyResultEnvelope tests do not establish that the direct-mode handler calls the helper; reaching it needs a fully wired proxy. The /mcp/all surface's FR-016b guarantee rests on the call site staying as written, and the comment now says so instead of claiming coverage. Both new guards are mutation-tested. Full internal/server suite, -race across hash and upstream, server edition, and golangci-lint v2 with the CI config are all green.
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related #532 · Plan: #1202 (specs/058-mcp-2026-upgrade)
Spec 058 Phases 1–2. Adopts
mark3labs/mcp-gov1.0.0 while leaving wire behaviour byte-identical in both directions, and fixes two latent defects the bump exposed.The bump is not the risky part; the constant it redefines is
v1.0.0 redefines
mcp.LATEST_PROTOCOL_VERSIONfrom2025-11-25to2026-07-28, andclient.Initializebecame modern-first.internal/upstream/coresent that constant, so bumping the dependency alone would have switched every upstream hop to the new protocol era — wherePingreturns success without sending anything (so the Spec-074 health loop would report dead upstreams as healthy) and server-initiated requests are gone (so roots-based workspace discovery cannot work). FR-027 is amended to name both hops, and both are pinned:clientFacingStreamableOptions()pins the client-facing transports, applied at all fiveNewStreamableHTTPServersites. It is a function rather than a package variable so it cannot be flipped at runtime or from a test, and it is shared so "pinned on four of five doors" is not expressible.mcp.LATEST_LEGACY_PROTOCOL_VERSION.Both are asserted on the wire rather than by reading the constants back. The failure mode here is a constant changing underneath us, and a test comparing code to code would have passed straight through it.
Two latent defects
Tool-approval hashes were coupled to the library version. Spec-032 hashes are computed over the library-decoded schema, because
mcp.Tool.RawInputSchemaisjson:"-"and the upstream's bytes are discarded. v1.0.0 rewrites draft-07#/definitions/refs to#/$defs/, so a hash stored by the old decoder could not be reproduced by the new one, and the formula-migration hatch does not absorb it — every affected tool would be reported as changed, a rug-pull signal, after an upgrade that changed nothing upstream.NormalizeJSONnow canonicalizes that spelling.The guard is deliberately narrow: it fires only when a document has a root
$defsand no rootdefinitions.NormalizeJSONis also applied to raw output schemas that reach the Spec-056 validator, and there a#/definitions/pointer is correct — rewriting it would aim it at a member that does not exist.An upstream's result envelope was replayed as mcpproxy's own. Both forwarding paths copied
ctr.Resultwholesale, which carriesResultType. SinceCallToolResultembedsMultiRoundTripResultseparately, an upstreaminput_requiredreached the client stripped of theinputRequestsandrequestStateneeded to answer it — told to supply input, with no way to supply it and no way to know why. The envelope decision is now one sharedproxyResultEnvelope(), so the two paths cannot drift.Verification
internal/serversuite green, includingTestProfile_SetProfileSessionScopedandTestProfile_SetProfileUnknown, which fail on v1.0.0 unpinned. The pin alone fixes them, with no change to profile code.-racegreen across hash, upstream, runtime and config. Server edition green.golangci-lintv2 with the CI config: no issues. Two mcp-go deprecations are suppressed with the reason and the follow-up task named inline.What this does not do
Serving
2026-07-28is deliberately out of scope. The stateless work (Spec 058 US3 — profile resolution, session-free attribution, and the cross-client cancellation collision in mcp-go'sinflightKey) must land before either pin lifts; lifting first would expose that collision to real users rather than to tests.