Fall back to Legacy on an inconclusive vMCP revision probe

pkg/vmcp/client/client.go

@@ -1053,7 +1053,9 @@ func (h *httpBackendClient) modernDiscover(
 //     leaving it unprobed lets the next call re-probe once the outage clears.
 //
 // 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.
+// (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.
 // The resolved capabilities are intentionally not returned: callers that need
 // them (ListCapabilities) re-fetch via modernDiscover, so probeRevision only
 // classifies and caches the revision.
@@ -1382,6 +1384,11 @@ func (*httpBackendClient) legacyInit(
 // and runs fn with it. Every call verb AND ListCapabilities route through this
 // seam so revision selection — and self-correction — lives in one place.
 //
+// A probeRevision error of any kind (inconclusive probe or transport-build
+// failure) falls back to Legacy — the pre-dual-era default — WITHOUT caching,
+// so a genuinely Modern backend re-probes on the next call instead of being
+// pinned to Legacy by one bad probe.
+//
 // On a revision mismatch (isRevisionMismatch), dispatch always re-probes
 // authoritatively and flips the cache so future calls use the corrected
 // revision. It RETRIES fn only when the failure proves the backend did NOT
@@ -1391,8 +1398,10 @@ func (*httpBackendClient) legacyInit(
 // cache is flipped but fn is NOT re-run. The retry is unconditionally single
 // (no re-check), so it can never loop.
 //
-// When the revision was just probed in THIS call (uncached), a re-probe would
-// return the same answer, so a mismatch is surfaced directly without re-probing.
+// When the revision was resolved in THIS call (uncached), a mismatch is surfaced
+// directly without re-probing: on a successful probe a re-probe would just repeat
+// the same answer, and on the Legacy fallback above there is no cached revision to
+// correct — the next call re-probes anyway once the outage clears.
 func (h *httpBackendClient) dispatch(
 	ctx context.Context, target *vmcp.BackendTarget,
 	fn func(ctx context.Context, rev mcpparser.Revision) error,
@@ -1401,9 +1410,19 @@ func (h *httpBackendClient) dispatch(
 	if !cached {
 		probed, err := h.probeRevision(ctx, target)
 		if err != nil {
-			return wrapBackendError(err, target.WorkloadID, "probe revision")
+			// A failed probe (auth blip / transient 5xx / timeout / connection
+			// refused, or a transport that could not be built) says nothing about
+			// the revision and must NOT fail a backend that is reachable on the
+			// Legacy path — which re-validates the transport itself. Fall back to Legacy —
+			// the pre-dual-era default — WITHOUT caching, so a transient outage
+			// never pins a revision and a genuinely Modern backend is corrected on
+			// the next call's re-probe.
+			slog.DebugContext(ctx, "revision probe inconclusive; attempting Legacy uncached",
+				"backend", target.WorkloadID, "probe_error", err)
+			rev = mcpparser.RevisionLegacy
+		} else {
+			rev = probed
 		}
-		rev = probed
 	}
 
 	err := fn(ctx, rev)

pkg/vmcp/client/dispatch_legacy_fallback_test.go

@@ -0,0 +1,94 @@
+// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package client
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/stacklok/toolhive/pkg/vmcp"
+)
+
+// TestDispatch_LegacyFallbackOnInconclusiveProbe pins the fix for the
+// dispatch abort regressed by merge 885e8b14f: a Modern server/discover probe
+// that comes back transient (HTTP 503, -> errModernTransient) must fall back
+// to the Legacy initialize/tools-list path uncached, not fail the whole call.
+// Caching the fallback would pin a genuinely Modern backend to Legacy past a
+// transient outage, so the revision cache must remain unset afterward.
+func TestDispatch_LegacyFallbackOnInconclusiveProbe(t *testing.T) {
+	t.Parallel()
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Mcp-Method") != "" {
+			// Modern probe: transient failure, discovered before the body is read.
+			w.WriteHeader(http.StatusServiceUnavailable)
+			return
+		}
+
+		body, err := readAll(t, r)
+		if err != nil {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		var req jsonRPCRequest
+		if err := json.Unmarshal(body, &req); err != nil {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		switch req.Method {
+		case "initialize":
+			resp := jsonRPCResponse{
+				JSONRPC: "2.0",
+				ID:      req.ID,
+				Result: json.RawMessage(`{
+					"protocolVersion": "2024-11-05",
+					"capabilities": {"tools": {}},
+					"serverInfo": {"name": "flaky-probe", "version": "1.0.0"}
+				}`),
+			}
+			_ = json.NewEncoder(w).Encode(resp)
+		case "tools/list":
+			resp := jsonRPCResponse{
+				JSONRPC: "2.0",
+				ID:      req.ID,
+				Result: json.RawMessage(`{
+					"tools": [{"name": "echo", "description": "echoes input"}]
+				}`),
+			}
+			_ = json.NewEncoder(w).Encode(resp)
+		default:
+			_ = json.NewEncoder(w).Encode(jsonRPCResponse{
+				JSONRPC: "2.0",
+				ID:      req.ID,
+				Result:  json.RawMessage(`{}`),
+			})
+		}
+	}))
+	t.Cleanup(srv.Close)
+
+	// newProbeClient, not setRevision: the probe must actually run so the
+	// Modern 503 is hit and the fallback path exercised.
+	h := newProbeClient(t)
+	target := &vmcp.BackendTarget{
+		WorkloadID:    "flaky-probe",
+		BaseURL:       srv.URL,
+		TransportType: "streamable-http",
+	}
+
+	caps, err := h.ListCapabilities(context.Background(), target)
+	require.NoError(t, err)
+	require.Len(t, caps.Tools, 1)
+	assert.Equal(t, "echo", caps.Tools[0].Name)
+
+	_, ok := h.cachedRevision("flaky-probe")
+	assert.False(t, ok, "an inconclusive probe must not pin a revision")
+}