Classify -32022 and SSE backends as Legacy, sync stale comments

pkg/mcp/parser_integration_test.go

@@ -144,27 +144,26 @@ func TestParsingMiddlewareWithRealMCPClients(t *testing.T) {
 			assert.NotEmpty(t, readResult.Contents)
 
 			// Verify that all requests were parsed by the middleware
-			assert.GreaterOrEqual(t, len(parsedRequests), 4, "Expected at least 4 parsed requests (initialize, list tools, call tool, list resources, read resource)")
-
-			// Verify specific parsed requests. Session establishment is
-			// transport-aware under MCP 2026-07-28 (#5754): the mcpcompat/go-sdk
-			// v1.7 client is Modern-first (SEP-2575), so over SSE it sends
-			// server/discover first and gets no answer from this Legacy-only test
-			// server, falling back — but the fallback initialize handshake is
-			// itself carried over the SSE transport's message channel, which this
-			// harness does not capture as a distinct parsed "initialize" the way
-			// streamable-HTTP does. The streamable-HTTP arm is unaffected (no
-			// discover-first probe observed here) and still parses "initialize".
+			assert.GreaterOrEqual(t, len(parsedRequests), 4, "Expected at least 4 parsed requests (session establishment, list tools, call tool, list resources, read resource)")
+
+			// Verify specific parsed requests. mcpcompat's client is Modern-first
+			// (SEP-2575) on every transport, so both arms send server/discover
+			// first. The SSE server transport advertises 2026-07-28 unconditionally
+			// (no ProtocolVersionSupporter gate), so discover succeeds and
+			// "initialize" is never sent there. The streamable-HTTP server here is
+			// stateful (no WithStateless), which gates 2026-07-28 on being
+			// stateless, so its discover negotiates down and it additionally sends
+			// "initialize".
 			foundSessionEstablished := false
 			foundToolCall := false
 			foundResourceRead := false
+			var methodsSeen []string
 			for _, parsed := range parsedRequests {
+				methodsSeen = append(methodsSeen, parsed.Method)
 				switch parsed.Method {
 				case "initialize":
-					if tc.transport != "sse" {
-						foundSessionEstablished = true
-						assert.Equal(t, "test-client", parsed.ResourceID)
-					}
+					foundSessionEstablished = true
+					assert.Equal(t, "test-client", parsed.ResourceID)
 				case "server/discover":
 					if tc.transport == "sse" {
 						// parser.go maps server/discover to the static ResourceID
@@ -180,10 +179,10 @@ func TestParsingMiddlewareWithRealMCPClients(t *testing.T) {
 					assert.Equal(t, "test://resource", parsed.ResourceID)
 				}
 			}
+			assert.True(t, foundSessionEstablished, "session establishment request (initialize or server/discover) should have been parsed")
 			if tc.transport == "sse" {
-				assert.True(t, foundSessionEstablished, "server/discover request should have been parsed (2026-07-28 HTTP+SSE, #5754)")
-			} else {
-				assert.True(t, foundSessionEstablished, "Initialize request should have been parsed")
+				assert.Contains(t, methodsSeen, "server/discover", "SSE arm should negotiate via server/discover")
+				assert.NotContains(t, methodsSeen, "initialize", "SSE arm is Modern-first and should never fall back to initialize here")
 			}
 			assert.True(t, foundToolCall, "Tool call request should have been parsed")
 			assert.True(t, foundResourceRead, "Resource read request should have been parsed")

pkg/mcp/revision.go

@@ -551,3 +551,37 @@ func decodeSentinelName(v string) (string, error) {
 	}
 	return string(decoded), nil
 }
+
+// EncodeSentinelName encodes v into the draft spec's base64 sentinel format
+// (=?base64?<payload>?=) when required — the mirror of decodeSentinelName.
+// Encoding is required when EITHER:
+//   - v is not safely representable as a plain ASCII header value: any byte
+//     falls outside printable ASCII 0x21-0x7E, which also covers leading/
+//     trailing whitespace and CR/LF (both fall below 0x21 and would
+//     otherwise make net/http reject the request outright), or
+//   - v already matches the sentinel pattern (has both sentinelPrefix and
+//     sentinelSuffix), which must be escaped so the server doesn't mistake a
+//     literal name for an encoded payload.
+//
+// Otherwise v is returned unchanged. The result always round-trips through
+// decodeSentinelName back to v.
+func EncodeSentinelName(v string) string {
+	if !needsSentinelEncoding(v) {
+		return v
+	}
+	return sentinelPrefix + base64.StdEncoding.EncodeToString([]byte(v)) + sentinelSuffix
+}
+
+// needsSentinelEncoding reports whether v requires sentinel encoding; see
+// EncodeSentinelName for the two conditions checked.
+func needsSentinelEncoding(v string) bool {
+	if strings.HasPrefix(v, sentinelPrefix) && strings.HasSuffix(v, sentinelSuffix) {
+		return true
+	}
+	for i := 0; i < len(v); i++ {
+		if v[i] < 0x21 || v[i] > 0x7E {
+			return true
+		}
+	}
+	return false
+}

pkg/mcp/revision_test.go

@@ -619,6 +619,42 @@ func TestDecodeSentinelName(t *testing.T) {
 	}
 }
 
+// TestEncodeSentinelName pins EncodeSentinelName as the exact mirror of
+// decodeSentinelName: every case must round-trip back to the original value.
+func TestEncodeSentinelName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		input    string
+		wantSame bool // true if the value must pass through unchanged
+	}{
+		{name: "plain ASCII name unchanged", input: "my-tool", wantSame: true},
+		{name: "URI with colon and slash unchanged", input: "file:///tmp/foo.txt", wantSame: true},
+		{name: "accented character encoded", input: "café-résumé", wantSame: false},
+		{name: "CJK character encoded", input: "工具", wantSame: false},
+		{name: "CR in value encoded", input: "bad\rname", wantSame: false},
+		{name: "LF in value encoded", input: "bad\nname", wantSame: false},
+		{name: "value already shaped like a sentinel is escaped", input: sentinelEncode("my-tool"), wantSame: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := EncodeSentinelName(tt.input)
+			if tt.wantSame {
+				assert.Equal(t, tt.input, got)
+			} else {
+				assert.NotEqual(t, tt.input, got)
+			}
+
+			decoded, err := decodeSentinelName(got)
+			require.NoError(t, err)
+			assert.Equal(t, tt.input, decoded, "must round-trip through decodeSentinelName")
+		})
+	}
+}
+
 // TestStripReservedModernMeta pins the copy-before-mutate contract: it removes
 // exactly the reserved io.modelcontextprotocol/* keys, never mutates the
 // caller's map, and returns nil for empty input.

pkg/vmcp/client/client.go

@@ -154,11 +154,19 @@ type httpBackendClient struct {
 	//
 	// NOTE: never evicted, no singleflight/CAS. Concurrent first-probes are
 	// last-writer-wins-safe: writes are idempotent for a deterministic backend
-	// (every probe agrees), and a flapping backend self-heals via
-	// dispatch->reclassify (a call revealing the other era re-probes and flips).
-	// A transient probe failure caches nothing (probeRevision returns uncached),
-	// so a blip cannot pin a revision; a TTL/periodic re-probe is deferred until
-	// flapping backends surface.
+	// (every probe agrees). A transient probe failure caches nothing
+	// (probeRevision returns uncached), so a blip cannot pin a revision.
+	//
+	// Self-healing now works both ways under go-sdk v1.7. A mis-cached Modern
+	// backend that has actually negotiated down to Legacy self-heals via
+	// dispatch->reclassify (errModernNegotiatedDown re-probes and flips it). A
+	// mis-cached Legacy backend that has actually become Modern self-heals
+	// in-band instead: the SDK's own client transparently negotiates Modern on
+	// the Legacy dispatch path, and legacyInit reads the genuinely-negotiated
+	// protocol version off every Initialize result and flips the cache directly
+	// when it is Modern — no error or reclassify round trip needed (see
+	// TestListCapabilities_MisCachedLegacy_SelfHealsViaSDKNegotiation in
+	// reclassify_test.go).
 	revisions sync.Map // map[string]mcpparser.Revision
 }
 
@@ -833,9 +841,12 @@ func wrapBackendError(err error, backendID string, operation string) error {
 		vmcp.ErrBackendUnavailable, operation, backendID, err)
 }
 
-// initializeClient performs MCP protocol initialization handshake and returns server capabilities.
-// This allows the caller to determine which optional features the server supports.
-func initializeClient(ctx context.Context, c *client.Client) (*mcp.ServerCapabilities, error) {
+// initializeClient performs MCP protocol initialization handshake and returns
+// server capabilities plus the actually-negotiated protocol version. The
+// latter comes straight from the SDK's InitializeResult, so it reflects the
+// real negotiation outcome (e.g. a Modern-first Connect() that negotiated
+// down) even though ProtocolVersion is requested as mcp.LATEST_PROTOCOL_VERSION.
+func initializeClient(ctx context.Context, c *client.Client) (*mcp.ServerCapabilities, string, error) {
 	result, err := c.Initialize(ctx, mcp.InitializeRequest{
 		Params: mcp.InitializeParams{
 			ProtocolVersion: mcp.LATEST_PROTOCOL_VERSION,
@@ -854,9 +865,9 @@ func initializeClient(ctx context.Context, c *client.Client) (*mcp.ServerCapabil
 		},
 	})
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
-	return &result.Capabilities, nil
+	return &result.Capabilities, result.ProtocolVersion, nil
 }
 
 // queryTools queries tools from a backend if the server advertises tool support.
@@ -1057,8 +1068,10 @@ func discoverModernCapabilities(ctx context.Context, hc *http.Client, endpoint s
 	// one Modern wire shape, so a backend must advertise exactly it to be driven
 	// Modern. This is intentionally stricter than go-sdk's reference client
 	// (negotiated >= 2026-07-28). TRIPWIRE: when a newer Modern revision is
-	// added, this must become a set/range check or a newer-only backend is
-	// wrongly classified Legacy — TestProbeRevision_RealBackends will catch it.
+	// added, this must become a set/range check (alongside the sibling
+	// exact-match in classifyUnsupportedProtocolVersion, modern.go) or a
+	// newer-only backend is wrongly classified Legacy — TestProbeRevision_RealBackends
+	// will catch it.
 	if !slices.Contains(discover.SupportedVersions, mcpparser.MCPVersionModern) {
 		return nil, fmt.Errorf("%w: supportedVersions=%v", errModernNegotiatedDown, discover.SupportedVersions)
 	}
@@ -1090,13 +1103,34 @@ func discoverModernCapabilities(ctx context.Context, hc *http.Client, endpoint s
 // A hard error is also returned when the backend transport cannot be built at all
 // (e.g. invalid auth/CA config); that is a genuine misconfiguration. Note that
 // dispatch, the sole caller, does not distinguish these error classes — it falls
-// back to Legacy uncached on any error this function returns.
+// back to Legacy uncached on any error this function returns. A "sse" target
+// (see below) returns before that check runs, so its transport is first
+// validated on the Legacy call path instead.
 // The resolved capabilities are intentionally not returned: callers that need
 // them (ListCapabilities) re-fetch via modernDiscover, so probeRevision only
 // classifies and caches the revision.
+//
+// A "sse" target is classified Legacy without a network call. TransportType ==
+// "sse" specifically names the deprecated 2024-11-05 two-endpoint transport
+// (GET /sse + POST /messages) — see ssecommon.HTTPSSEEndpoint and the GET-only
+// httpsse proxy — not "any backend that happens to use SSE" (Modern itself uses
+// SSE response streams). modernCall is a single-endpoint POST, so it can never
+// reach a Modern endpoint through an /sse BaseURL, even for a dual-era server
+// that also hosts a Modern endpoint, because that lives at a different path.
+// This also sidesteps POSTing into a GET-only stream and hanging to the client
+// timeout (errModernTransient, uncached, re-probed on every call).
 func (h *httpBackendClient) probeRevision(
 	ctx context.Context, target *vmcp.BackendTarget,
 ) (mcpparser.Revision, error) {
+	if target.TransportType == "sse" {
+		// The 2024-11-05 two-endpoint transport (GET /sse + POST /messages) has no
+		// Modern endpoint to discover at this BaseURL — see the doc comment above.
+		// Note: HTTP+SSE is Deprecated per the MCP spec, not removed; this gate is
+		// ToolHive routing, not a protocol requirement.
+		h.setRevision(target.WorkloadID, mcpparser.RevisionLegacy)
+		return mcpparser.RevisionLegacy, nil
+	}
+
 	hc, err := h.buildModernHTTPClient(ctx, target)
 	if err != nil {
 		return 0, fmt.Errorf("failed to build transport for backend %s: %w", target.WorkloadID, err)
@@ -1408,13 +1442,24 @@ var errLegacyInitFailed = errors.New("legacy initialize step failed")
 // errLegacyInitFailed (in addition to wrapBackendError's classification) so the
 // revision-mismatch check can tell an initialize rejection apart from a
 // data-plane error later in the same call.
-func (*httpBackendClient) legacyInit(
+//
+// It also self-heals a mis-cached Legacy->Modern backend in-band: under go-sdk
+// v1.7 the SDK's own client transparently negotiates Modern on this path (see
+// the revisions field comment), so the call succeeds and dispatch's
+// reclassify-on-error trigger never fires. The negotiated protocol version
+// returned by initializeClient is genuine either way (discover or plain
+// initialize), so when it equals MCPVersionModern the cache is flipped here
+// instead of waiting for an error that will never come.
+func (h *httpBackendClient) legacyInit(
 	ctx context.Context, c *client.Client, backendID string,
 ) (*mcp.ServerCapabilities, error) {
-	caps, err := initializeClient(ctx, c)
+	caps, negotiatedVersion, err := initializeClient(ctx, c)
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", errLegacyInitFailed, wrapBackendError(err, backendID, "initialize client"))
 	}
+	if negotiatedVersion == mcpparser.MCPVersionModern {
+		h.setRevision(backendID, mcpparser.RevisionModern)
+	}
 	return caps, nil
 }
 

pkg/vmcp/client/client_test.go

@@ -1686,6 +1686,9 @@ func TestDefaultClientFactory_SSEForwarding(t *testing.T) {
 					select {
 					case events <- respBytes:
 					case <-time.After(5 * time.Second):
+						// t.Errorf, not t.Fatal: this runs on the server's own goroutine
+						// (Fatal's runtime.Goexit would kill the wrong one).
+						t.Errorf("timed out queuing server/discover fallback response for SSE delivery")
 					}
 					return
 				}
@@ -1715,6 +1718,8 @@ func TestDefaultClientFactory_SSEForwarding(t *testing.T) {
 				select {
 				case events <- respBytes:
 				case <-time.After(5 * time.Second):
+					// t.Errorf, not t.Fatal: this runs on the server's own goroutine.
+					t.Errorf("timed out queuing initialize response for SSE delivery")
 				}
 			})
 			mux.HandleFunc("/sse", func(w http.ResponseWriter, r *http.Request) {
@@ -1771,7 +1776,7 @@ func TestDefaultClientFactory_SSEForwarding(t *testing.T) {
 			// corresponding handler is installed.
 			initCtx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
 			defer cancel()
-			_, err = initializeClient(initCtx, c)
+			_, _, err = initializeClient(initCtx, c)
 			require.NoError(t, err)
 
 			mu.Lock()