From 24049c75d708c1682c79b56dc355453d392d15c3 Mon Sep 17 00:00:00 2001 From: King Star Date: Sun, 6 Sep 2026 22:33:30 +0800 Subject: [PATCH] test(vmcp): pin that the health path never opens a standalone GET stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #6497 reported a healthy Tableau MCP backend being marked unavailable because the vMCP health probe used a bare HTTP GET, which that backend rejects with 400 for lack of a session id. Current main no longer probes with GET: the health check is BackendClient.ListCapabilities, and newStreamableHTTPClient enables transport.WithContinuousListening — the standalone server->client SSE stream, the only GET vMCP makes — solely on the forwarding tools/call path. Nothing pinned that, so add the regression the issue describes: a real go-sdk stateful streamable-HTTP backend behind a handler that answers every GET with Tableau's 400, asserting that ListCapabilities both succeeds and issues no GET at all. Verified to have teeth: appending WithContinuousListening unconditionally in newStreamableHTTPClient makes the test fail on the GET assertion. --- pkg/vmcp/client/getreject_realbackend_test.go | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 pkg/vmcp/client/getreject_realbackend_test.go diff --git a/pkg/vmcp/client/getreject_realbackend_test.go b/pkg/vmcp/client/getreject_realbackend_test.go new file mode 100644 index 0000000000..4be2ddeb3d --- /dev/null +++ b/pkg/vmcp/client/getreject_realbackend_test.go @@ -0,0 +1,110 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "context" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + mcpmcp "github.com/stacklok/toolhive-core/mcpcompat/mcp" + mcpserver "github.com/stacklok/toolhive-core/mcpcompat/server" + "github.com/stacklok/toolhive/pkg/vmcp" +) + +// newGETRejectingEchoServer stands up the same real go-sdk streamable-HTTP +// backend as newRealEchoServer, stateful (Legacy), behind a handler that +// answers every HTTP GET with 400 and records the method of every request it +// receives. +// +// That is the Tableau MCP shape from issue #6497: a healthy backend that +// requires a session id issued by initialize, so a bare GET — which carries no +// session context — is rejected rather than upgraded to a standalone SSE +// stream. +func newGETRejectingEchoServer(t *testing.T, record func(method string)) *httptest.Server { + t.Helper() + + mcpSrv := mcpserver.NewMCPServer("get-rejecting-backend", "1.0.0") + mcpSrv.AddTool( + mcpmcp.NewTool("echo", + mcpmcp.WithDescription("Echoes the input back"), + mcpmcp.WithString("input", mcpmcp.Required()), + ), + func(_ context.Context, req mcpmcp.CallToolRequest) (*mcpmcp.CallToolResult, error) { + args, _ := req.Params.Arguments.(map[string]any) + input, _ := args["input"].(string) + return &mcpmcp.CallToolResult{Content: []mcpmcp.Content{mcpmcp.NewTextContent(input)}}, nil + }, + ) + + inner := mcpserver.NewStreamableHTTPServer(mcpSrv) + mux := http.NewServeMux() + mux.Handle("/mcp", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + record(r.Method) + if r.Method == http.MethodGet { + // Tableau MCP's response to a session-less GET. + http.Error(w, "Invalid or missing session ID", http.StatusBadRequest) + return + } + inner.ServeHTTP(w, r) + })) + ts := httptest.NewServer(mux) + t.Cleanup(ts.Close) + return ts +} + +// TestListCapabilities_GETRejectingBackendIsHealthy pins the health signal +// reported in issue #6497: a backend that rejects a bare HTTP GET must not be +// treated as unavailable. +// +// vMCP's health check is BackendClient.ListCapabilities (see +// health.NewHealthChecker), which reaches the backend over POST. GET is only +// ever the standalone server->client SSE stream, and in Streamable HTTP that +// stream is optional — a server without one answers 405 per spec, and one that +// requires a session id (Tableau MCP) answers 400. Neither says anything about +// whether the backend can serve MCP. +// +// The assertion is therefore two-sided: ListCapabilities must succeed against +// such a backend, AND it must not issue a GET at all. Only the tools/call +// forwarding path enables transport.WithContinuousListening (see +// newStreamableHTTPClient); a change that opened the standalone stream on the +// non-forwarding path would put every session-requiring backend back to +// "unavailable" and drop its tools from tools/list. +func TestListCapabilities_GETRejectingBackendIsHealthy(t *testing.T) { + t.Parallel() + + var mu sync.Mutex + var methods []string + srv := newGETRejectingEchoServer(t, func(method string) { + mu.Lock() + defer mu.Unlock() + methods = append(methods, method) + }) + + h := newProbeClient(t) + target := &vmcp.BackendTarget{ + WorkloadID: "get-rejecting-backend", + WorkloadName: "GET Rejecting Backend", + BaseURL: srv.URL + "/mcp", + TransportType: "streamable-http", + } + + caps, err := h.ListCapabilities(context.Background(), target) + require.NoError(t, err, + "a backend that rejects a bare GET is still a healthy MCP backend: the health check speaks MCP over POST") + require.NotNil(t, caps) + assert.Len(t, caps.Tools, 1) + assert.Equal(t, "echo", caps.Tools[0].Name) + + mu.Lock() + defer mu.Unlock() + require.NotEmpty(t, methods, "the backend must have been reached") + assert.NotContains(t, methods, http.MethodGet, + "the health path must not open a standalone SSE GET stream; only the tools/call forwarding path may") +}