From c5516bb23dac1aafe0f7abb0567507f8c466c1d5 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 21 Jul 2026 14:21:01 +0100 Subject: [PATCH 01/18] Add raw MCP HTTP client for e2e proxy tests The dual-era stateless proxy e2e tests must emit traffic a conforming MCP client never would: spoofed Mcp-Session-Id, colliding JSON-RPC ids, malformed _meta, and oversized/truncated/batch bodies. A hand-rolled, stdlib-only client gives the byte-level control those tests need without importing go-sdk v1.7 into the module (which would force an MVS bump). Adds a raw client that builds Modern (2026-07-28) and Legacy requests, allows arbitrary header/id/session/_meta overrides and raw bodies, and reuses one concurrency-safe http.Client for the crown-jewel fan-out. Co-Authored-By: Claude Opus 4.8 --- test/e2e/mcp_raw_client.go | 362 ++++++++++++++++++++++++++++++++ test/e2e/mcp_raw_client_test.go | 290 +++++++++++++++++++++++++ 2 files changed, 652 insertions(+) create mode 100644 test/e2e/mcp_raw_client.go create mode 100644 test/e2e/mcp_raw_client_test.go diff --git a/test/e2e/mcp_raw_client.go b/test/e2e/mcp_raw_client.go new file mode 100644 index 0000000000..61c7e17801 --- /dev/null +++ b/test/e2e/mcp_raw_client.go @@ -0,0 +1,362 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "maps" + "net/http" + "sync/atomic" + "time" +) + +// The following mirror the reserved wire constants defined in +// pkg/mcp/revision.go. They are duplicated here rather than imported: this +// client must stay independent of any MCP library (production or SDK) so it +// can emit traffic no conforming implementation would ever produce. +const ( + // MCPVersionModern is the Modern (stateless) MCP protocol version. + MCPVersionModern = "2026-07-28" + + // MetaKeyProtocolVersion is the reserved _meta key carrying the per-request + // protocol version on Modern requests. + MetaKeyProtocolVersion = "io.modelcontextprotocol/protocolVersion" + // MetaKeyClientInfo is the reserved _meta key carrying client implementation + // info on Modern requests. Optional: a Modern request may omit it. + MetaKeyClientInfo = "io.modelcontextprotocol/clientInfo" + // MetaKeyClientCapabilities is the reserved _meta key carrying client + // capabilities on Modern requests. + MetaKeyClientCapabilities = "io.modelcontextprotocol/clientCapabilities" + + // HeaderMCPProtocolVersion is the HTTP header carrying the Modern protocol + // version. + HeaderMCPProtocolVersion = "MCP-Protocol-Version" + // HeaderMCPSessionID is the HTTP header carrying the Legacy session id. + HeaderMCPSessionID = "Mcp-Session-Id" +) + +// requestIDCounter assigns default JSON-RPC ids so distinct RawRequests get +// distinct ids unless a test overrides one via WithID (e.g. to collide two +// ids on purpose). +var requestIDCounter atomic.Int64 + +// RawRequest builds a single JSON-RPC request for MCP-over-HTTP, or carries +// a raw pre-built body that bypasses the builder entirely. Construct one +// with NewLegacyRequest, NewLegacyInitializeRequest, or NewModernRequest, +// then chain the With*/Set*/Delete* methods to adjust it. The zero value is +// not usable. +// +// Finish building before sending: Send/SendRaw only read a RawRequest, so a +// fully-built one is safe to share read-only across goroutines (e.g. one +// RawRequest fanned out to many concurrent senders), but mutating it with +// Set*/With* concurrently with a send is a data race. +type RawRequest struct { + method string + id any + params map[string]any + meta map[string]any // nil means no _meta object at all + headers map[string]string + body []byte // when set, overrides method/id/params/meta entirely +} + +// NewLegacyRequest builds a Legacy (2025-11-25) JSON-RPC request for the +// given method. params is cloned; the caller's map is never mutated by the +// returned RawRequest. +func NewLegacyRequest(method string, params map[string]any) (*RawRequest, error) { + return newRawRequest(method, params) +} + +// NewLegacyInitializeRequest builds a standard Legacy "initialize" request. +func NewLegacyInitializeRequest(clientName, clientVersion string) *RawRequest { + r, _ := newRawRequest("initialize", map[string]any{ // method is a fixed non-empty literal; newRawRequest cannot fail here. + "protocolVersion": "2025-11-25", + "capabilities": map[string]any{}, + "clientInfo": map[string]any{ + "name": clientName, + "version": clientVersion, + }, + }) + return r +} + +// NewModernRequest builds a Modern (2026-07-28) JSON-RPC request: it sets the +// MCP-Protocol-Version header and the reserved _meta keys protocolVersion and +// clientCapabilities (clientInfo is optional per the draft schema and is +// omitted by default). Each can be independently overridden or removed via +// SetHeader/DeleteHeader and SetMeta/DeleteMeta. +func NewModernRequest(method string, params map[string]any) (*RawRequest, error) { + r, err := newRawRequest(method, params) + if err != nil { + return nil, err + } + r.headers[HeaderMCPProtocolVersion] = MCPVersionModern + r.meta = map[string]any{ + MetaKeyProtocolVersion: MCPVersionModern, + MetaKeyClientCapabilities: map[string]any{}, + } + return r, nil +} + +func newRawRequest(method string, params map[string]any) (*RawRequest, error) { + if method == "" { + return nil, errors.New("mcp_raw_client: method must not be empty") + } + return &RawRequest{ + method: method, + id: requestIDCounter.Add(1), + params: maps.Clone(params), + headers: map[string]string{}, + }, nil +} + +// WithID overrides the JSON-RPC id, e.g. to collide two requests' ids or to +// use a value outside the int64-safe float64 range. +func (r *RawRequest) WithID(id any) *RawRequest { + r.id = id + return r +} + +// WithSessionID sets the Mcp-Session-Id header, including a foreign/spoofed +// session id. +func (r *RawRequest) WithSessionID(sessionID string) *RawRequest { + return r.SetHeader(HeaderMCPSessionID, sessionID) +} + +// WithClientInfo sets the _meta clientInfo object. +func (r *RawRequest) WithClientInfo(name, version string) *RawRequest { + return r.SetMeta(MetaKeyClientInfo, map[string]any{"name": name, "version": version}) +} + +// WithRawBody replaces the entire request body with body, sent verbatim. +// Use this for truncated, oversized, or garbage bodies. body is cloned; +// mutating the caller's slice afterward has no effect on the request. +func (r *RawRequest) WithRawBody(body []byte) *RawRequest { + r.body = bytes.Clone(body) + return r +} + +// SetHeader sets an arbitrary HTTP header on the request. +func (r *RawRequest) SetHeader(key, value string) *RawRequest { + r.headers[key] = value + return r +} + +// DeleteHeader removes a header, e.g. to omit MCP-Protocol-Version entirely. +func (r *RawRequest) DeleteHeader(key string) *RawRequest { + delete(r.headers, key) + return r +} + +// SetMeta sets a params._meta key to an arbitrary value, including a +// non-object value for a key the spec requires to be an object (to trigger +// error paths). It lazily creates the _meta object if none exists yet. +func (r *RawRequest) SetMeta(key string, value any) *RawRequest { + if r.meta == nil { + r.meta = map[string]any{} + } + r.meta[key] = value + return r +} + +// DeleteMeta removes a params._meta key, e.g. to make clientInfo absent. +func (r *RawRequest) DeleteMeta(key string) *RawRequest { + delete(r.meta, key) + return r +} + +// marshal renders the JSON-RPC wire body for r, or returns r.body verbatim +// if WithRawBody was used. +func (r *RawRequest) marshal() ([]byte, error) { + if r.body != nil { + return r.body, nil + } + + wire := map[string]any{ + "jsonrpc": "2.0", + "method": r.method, + } + if r.id != nil { + wire["id"] = r.id + } + + params := maps.Clone(r.params) + if r.meta != nil { + if params == nil { + params = map[string]any{} + } + params["_meta"] = r.meta + } + if params != nil { + wire["params"] = params + } + + return json.Marshal(wire) +} + +// NewBatchBody renders a JSON-RPC batch: a JSON array of the wire bodies of +// reqs, in order. Combine with RawMCPClient.SendRaw to send it. Each element +// is validated the same as a standalone RawRequest; a deliberately malformed +// batch payload (bad JSON, wrong array shape) must be hand-built and sent +// via SendRaw directly. +func NewBatchBody(reqs ...*RawRequest) ([]byte, error) { + parts := make([]json.RawMessage, 0, len(reqs)) + for i, r := range reqs { + b, err := r.marshal() + if err != nil { + return nil, fmt.Errorf("mcp_raw_client: marshal batch element %d: %w", i, err) + } + parts = append(parts, b) + } + return json.Marshal(parts) +} + +// RawRPCError is the JSON-RPC "error" object of a RawResponse. +type RawRPCError struct { + Code int64 + Message string + Data json.RawMessage +} + +// RawResponse is the parsed result of sending a single (non-batch) JSON-RPC +// request over MCP-over-HTTP. For batch responses (a JSON array), inspect +// Body directly instead of ID/Result/Error. +type RawResponse struct { + StatusCode int + Headers http.Header + Body []byte // raw response body, always populated + ID any // echoed "id"; nil if absent, JSON null, or unparseable + Result json.RawMessage + Error *RawRPCError +} + +// RawMCPClient sends raw MCP-over-HTTP JSON-RPC requests with byte-level +// control over the wire shape. It wraps a single *http.Client (and its +// connection pool), shared and safe for concurrent use across goroutines — +// construct one RawMCPClient per test suite, not one per request. +type RawMCPClient struct { + httpClient *http.Client +} + +// NewRawMCPClient creates a RawMCPClient. Every request made through it is +// bounded by timeout, so a stuck proxy fails fast instead of hanging the +// test. +func NewRawMCPClient(timeout time.Duration) (*RawMCPClient, error) { + if timeout <= 0 { + return nil, fmt.Errorf("mcp_raw_client: timeout must be positive, got %s", timeout) + } + return &RawMCPClient{ + httpClient: &http.Client{ + Timeout: timeout, + // Raised from the net/http default of 2 idle conns/host: adversarial/ + // concurrency tests fan many goroutines out to the same proxy host, + // and the default would force constant reconnection. + Transport: &http.Transport{ + MaxIdleConns: 100, + MaxIdleConnsPerHost: 100, + IdleConnTimeout: 90 * time.Second, + }, + }, + }, nil +} + +// Send marshals req and POSTs it to url. No Accept header is set by +// default; a live streamable-HTTP MCP endpoint may require +// "Accept: application/json, text/event-stream" — set it via +// req.SetHeader("Accept", ...) when hitting a real proxy. +func (c *RawMCPClient) Send(ctx context.Context, url string, req *RawRequest) (*RawResponse, error) { + body, err := req.marshal() + if err != nil { + return nil, fmt.Errorf("mcp_raw_client: marshal request: %w", err) + } + return c.SendRaw(ctx, url, req.headers, body) +} + +// SendRaw POSTs body verbatim to url with the given headers — for batches +// built with NewBatchBody, or any payload (truncated, oversized, garbage) +// not expressible via RawRequest. +func (c *RawMCPClient) SendRaw(ctx context.Context, url string, headers map[string]string, body []byte) (*RawResponse, error) { + httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) + if err != nil { + return nil, fmt.Errorf("mcp_raw_client: build request: %w", err) + } + httpReq.Header.Set("Content-Type", "application/json") + for k, v := range headers { + httpReq.Header.Set(k, v) + } + + resp, err := c.httpClient.Do(httpReq) + if err != nil { + return nil, fmt.Errorf("mcp_raw_client: do request: %w", err) + } + defer func() { + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + }() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("mcp_raw_client: read response body: %w", err) + } + + result := &RawResponse{ + StatusCode: resp.StatusCode, + Headers: resp.Header.Clone(), + Body: respBody, + } + populateEnvelope(respBody, result) + return result, nil +} + +// wireError is the shape of a JSON-RPC "error" object on the wire. +type wireError struct { + Code int64 `json:"code"` + Message string `json:"message"` + Data json.RawMessage `json:"data"` +} + +// wireResponse is the shape of a single JSON-RPC response object. ID is kept +// as raw JSON so decodeID can preserve large integers exactly. +type wireResponse struct { + ID json.RawMessage `json:"id"` + Result json.RawMessage `json:"result"` + Error *wireError `json:"error"` +} + +// populateEnvelope best-effort parses body as a single JSON-RPC response +// object into out. A malformed or non-object body (e.g. a batch array, or +// deliberately garbled test input) leaves ID/Result/Error at their zero +// values; out.Body still carries the raw bytes for the caller to inspect. +func populateEnvelope(body []byte, out *RawResponse) { + var w wireResponse + if err := json.Unmarshal(body, &w); err != nil { + return + } + out.ID = decodeID(w.ID) + out.Result = w.Result + if w.Error != nil { + out.Error = &RawRPCError{Code: w.Error.Code, Message: w.Error.Message, Data: w.Error.Data} + } +} + +// decodeID decodes a raw JSON-RPC id, returning a json.Number for numeric +// ids (so large int64 values round-trip exactly, unlike the float64 that +// encoding/json's default decodes numbers into) or a string for string ids. +// An absent or null id decodes to nil. +func decodeID(raw json.RawMessage) any { + if len(raw) == 0 || string(raw) == "null" { + return nil + } + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + var v any + if err := dec.Decode(&v); err != nil { + return nil + } + return v +} diff --git a/test/e2e/mcp_raw_client_test.go b/test/e2e/mcp_raw_client_test.go new file mode 100644 index 0000000000..7c25bf5be1 --- /dev/null +++ b/test/e2e/mcp_raw_client_test.go @@ -0,0 +1,290 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + "encoding/json" + "io" + "math" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// wireOf marshals req and decodes it into a generic map for field assertions. +func wireOf(t *testing.T, req *RawRequest) map[string]any { + t.Helper() + b, err := req.marshal() + require.NoError(t, err) + var m map[string]any + require.NoError(t, json.Unmarshal(b, &m)) + return m +} + +func TestRawClientBuilders(t *testing.T) { + t.Parallel() + + t.Run("ModernRequest sets header and default meta keys, clientInfo omitted", func(t *testing.T) { + t.Parallel() + req, err := NewModernRequest("tools/list", nil) + require.NoError(t, err) + + require.Equal(t, MCPVersionModern, req.headers[HeaderMCPProtocolVersion]) + + wire := wireOf(t, req) + meta := wire["params"].(map[string]any)["_meta"].(map[string]any) + require.Equal(t, MCPVersionModern, meta[MetaKeyProtocolVersion]) + require.Equal(t, map[string]any{}, meta[MetaKeyClientCapabilities]) + require.NotContains(t, meta, MetaKeyClientInfo) + }) + + t.Run("ModernRequest fields are independently overridable", func(t *testing.T) { + t.Parallel() + req, err := NewModernRequest("tools/list", nil) + require.NoError(t, err) + + req.SetMeta(MetaKeyProtocolVersion, "bogus-version"). + SetMeta(MetaKeyClientCapabilities, "not-an-object"). // non-object value, to trigger error paths + WithClientInfo("probe", "1.0"). + DeleteHeader(HeaderMCPProtocolVersion) + + _, hasHeader := req.headers[HeaderMCPProtocolVersion] + require.False(t, hasHeader, "header should be omittable") + + wire := wireOf(t, req) + meta := wire["params"].(map[string]any)["_meta"].(map[string]any) + require.Equal(t, "bogus-version", meta[MetaKeyProtocolVersion]) + require.Equal(t, "not-an-object", meta[MetaKeyClientCapabilities]) + require.Equal(t, map[string]any{"name": "probe", "version": "1.0"}, meta[MetaKeyClientInfo]) + + req.DeleteMeta(MetaKeyClientInfo) + wire = wireOf(t, req) + meta = wire["params"].(map[string]any)["_meta"].(map[string]any) + require.NotContains(t, meta, MetaKeyClientInfo) + }) + + t.Run("LegacyInitializeRequest carries no _meta and the legacy protocol version", func(t *testing.T) { + t.Parallel() + req := NewLegacyInitializeRequest("probe", "1.0") + wire := wireOf(t, req) + + require.Equal(t, "initialize", wire["method"]) + params := wire["params"].(map[string]any) + require.Equal(t, "2025-11-25", params["protocolVersion"]) + require.NotContains(t, params, "_meta") + }) + + t.Run("LegacyRequest WithSessionID sets the session header, including a spoofed one", func(t *testing.T) { + t.Parallel() + req, err := NewLegacyRequest("tools/call", map[string]any{"name": "echo"}) + require.NoError(t, err) + req.WithSessionID("not-my-session") + + require.Equal(t, "not-my-session", req.headers[HeaderMCPSessionID]) + }) + + t.Run("NewLegacyRequest rejects an empty method", func(t *testing.T) { + t.Parallel() + _, err := NewLegacyRequest("", nil) + require.Error(t, err) + }) + + t.Run("WithID preserves a large int64 and allows colliding ids", func(t *testing.T) { + t.Parallel() + const big = int64(9223372036854775807) // math.MaxInt64; beyond float64's exact integer range + + reqA, err := NewLegacyRequest("ping", nil) + require.NoError(t, err) + reqA.WithID(big) + reqB, err := NewLegacyRequest("ping", nil) + require.NoError(t, err) + reqB.WithID(big) // deliberate id collision with reqA + + for _, req := range []*RawRequest{reqA, reqB} { + b, err := req.marshal() + require.NoError(t, err) + require.Contains(t, string(b), `"id":9223372036854775807`) + } + }) + + t.Run("WithRawBody sends the body verbatim, ignoring method and params", func(t *testing.T) { + t.Parallel() + req, err := NewLegacyRequest("tools/list", nil) + require.NoError(t, err) + garbage := []byte(`{not even json`) + req.WithRawBody(garbage) + + b, err := req.marshal() + require.NoError(t, err) + require.Equal(t, garbage, b) + + // Mutating the caller's slice afterward must not affect the request. + garbage[0] = 'X' + b, err = req.marshal() + require.NoError(t, err) + require.Equal(t, []byte(`{not even json`), b) + }) + + t.Run("NewBatchBody produces a JSON array of the wire bodies in order", func(t *testing.T) { + t.Parallel() + reqA, err := NewLegacyRequest("ping", nil) + require.NoError(t, err) + reqA.WithID(int64(1)) + reqB, err := NewLegacyRequest("tools/list", nil) + require.NoError(t, err) + reqB.WithID(int64(2)) + + batch, err := NewBatchBody(reqA, reqB) + require.NoError(t, err) + + var arr []map[string]any + require.NoError(t, json.Unmarshal(batch, &arr)) + require.Len(t, arr, 2) + require.Equal(t, "ping", arr[0]["method"]) + require.InDelta(t, float64(1), arr[0]["id"], 0) + require.Equal(t, "tools/list", arr[1]["method"]) + require.InDelta(t, float64(2), arr[1]["id"], 0) + }) + + t.Run("params map passed by the caller is not mutated", func(t *testing.T) { + t.Parallel() + callerParams := map[string]any{"name": "echo"} + req, err := NewModernRequest("tools/call", callerParams) + require.NoError(t, err) + req.SetMeta("extra", "value") + _, err = req.marshal() + require.NoError(t, err) + + require.Equal(t, map[string]any{"name": "echo"}, callerParams, "caller's map must be untouched") + }) +} + +// capturedRequest snapshots what the sender actually transmitted. +type capturedRequest struct { + headers http.Header + body []byte +} + +// newCapturingServer starts an httptest.Server that records the last request +// it received (via the returned *capturedRequest pointer, refreshed on every +// call) and replies with responseBody. +func newCapturingServer(t *testing.T, responseBody string) (*httptest.Server, *capturedRequest) { + t.Helper() + captured := &capturedRequest{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + captured.headers = r.Header.Clone() + captured.body = body + + w.Header().Set("Content-Type", "application/json") + w.Header().Set(HeaderMCPSessionID, "server-assigned-session") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(responseBody)) + })) + t.Cleanup(server.Close) + return server, captured +} + +func TestRawClientSend(t *testing.T) { + t.Parallel() + + client, err := NewRawMCPClient(5 * time.Second) + require.NoError(t, err) + + t.Run("transmits headers and body, and parses result/id/response headers", func(t *testing.T) { + t.Parallel() + server, captured := newCapturingServer(t, `{"jsonrpc":"2.0","id":42,"result":{"ok":true}}`) + + req := NewLegacyInitializeRequest("probe", "1.0").WithSessionID("client-picked-session") + req.WithID(int64(42)) + + resp, err := client.Send(context.Background(), server.URL, req) + require.NoError(t, err) + + require.Equal(t, "client-picked-session", captured.headers.Get(HeaderMCPSessionID), + "the client-set session header must reach the server verbatim") + + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, "server-assigned-session", resp.Headers.Get(HeaderMCPSessionID), + "response headers must be surfaced so tests can assert session-id echoing") + require.Equal(t, json.Number("42"), resp.ID) + require.JSONEq(t, `{"ok":true}`, string(resp.Result)) + require.Nil(t, resp.Error) + }) + + t.Run("parses error.code, error.data and a large-int64 id without precision loss", func(t *testing.T) { + t.Parallel() + const bigID = "9223372036854775807" + server, _ := newCapturingServer(t, + `{"jsonrpc":"2.0","id":`+bigID+`,"error":{"code":-32020,"message":"mismatch","data":{"header":"2025-11-25"}}}`) + + req, err := NewModernRequest("tools/list", nil) + require.NoError(t, err) + resp, err := client.Send(context.Background(), server.URL, req) + require.NoError(t, err) + + require.Equal(t, json.Number(bigID), resp.ID) + id, err := resp.ID.(json.Number).Int64() + require.NoError(t, err) + require.Equal(t, int64(math.MaxInt64), id) + + require.NotNil(t, resp.Error) + require.Equal(t, int64(-32020), resp.Error.Code) + require.Equal(t, "mismatch", resp.Error.Message) + require.JSONEq(t, `{"header":"2025-11-25"}`, string(resp.Error.Data)) + }) + + t.Run("SendRaw transmits a batch body and leaves envelope fields zero on a non-object response", func(t *testing.T) { + t.Parallel() + server, captured := newCapturingServer(t, `[{"jsonrpc":"2.0","id":1,"result":{}},{"jsonrpc":"2.0","id":2,"result":{}}]`) + + reqA, err := NewLegacyRequest("ping", nil) + require.NoError(t, err) + reqA.WithID(int64(1)) + reqB, err := NewLegacyRequest("tools/list", nil) + require.NoError(t, err) + reqB.WithID(int64(2)) + batch, err := NewBatchBody(reqA, reqB) + require.NoError(t, err) + + resp, err := client.SendRaw(context.Background(), server.URL, nil, batch) + require.NoError(t, err) + + var sentArr []map[string]any + require.NoError(t, json.Unmarshal(captured.body, &sentArr)) + require.Len(t, sentArr, 2, "the server must receive a two-element JSON array") + + require.Nil(t, resp.ID, "a batch (array) response is not a single envelope") + require.Nil(t, resp.Error) + require.JSONEq(t, `[{"jsonrpc":"2.0","id":1,"result":{}},{"jsonrpc":"2.0","id":2,"result":{}}]`, string(resp.Body)) + }) + + t.Run("garbage body is sent verbatim and a non-JSON response leaves the envelope zero but Body populated", func(t *testing.T) { + t.Parallel() + server, captured := newCapturingServer(t, `not json at all`) + + resp, err := client.SendRaw(context.Background(), server.URL, map[string]string{"X-Test": "1"}, []byte(`{truncated`)) + require.NoError(t, err) + + require.Equal(t, []byte(`{truncated`), captured.body) + require.Equal(t, "1", captured.headers.Get("X-Test")) + + require.Nil(t, resp.ID) + require.Nil(t, resp.Error) + require.Equal(t, []byte(`not json at all`), resp.Body) + }) +} + +func TestNewRawMCPClientRejectsNonPositiveTimeout(t *testing.T) { + t.Parallel() + _, err := NewRawMCPClient(0) + require.Error(t, err) + _, err = NewRawMCPClient(-time.Second) + require.Error(t, err) +} From f6a015b4fedb975d5a29fbcf40a1cbdc7884f203 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 16:16:06 +0200 Subject: [PATCH 02/18] Add golden Modern-request oracle for raw client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The raw client's Modern _meta encoding was written and asserted by the same author, a self-review blind spot: a wrong reserved key, wrong nesting, or wrong value type could pass its own tests. This adds an independent oracle — a 2026-07-28 tools/list request emitted by a real go-sdk v1.7.0-pre.3 client — and asserts NewModernRequest reproduces its _meta reserved-key set, nesting, and value kinds. A typo'd key or a non-object clientCapabilities now fails the test. The fixture is a committed static artifact; it is regenerated by pointing a go-sdk v1.7 client at yardstick-server --stateless (>= v1.2.0). Co-Authored-By: Claude Opus 4.8 --- test/e2e/mcp_raw_client_golden_test.go | 121 +++++++++++++++++++ test/e2e/testdata/golden_modern_request.json | 19 +++ 2 files changed, 140 insertions(+) create mode 100644 test/e2e/mcp_raw_client_golden_test.go create mode 100644 test/e2e/testdata/golden_modern_request.json diff --git a/test/e2e/mcp_raw_client_golden_test.go b/test/e2e/mcp_raw_client_golden_test.go new file mode 100644 index 0000000000..c990eec97c --- /dev/null +++ b/test/e2e/mcp_raw_client_golden_test.go @@ -0,0 +1,121 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +// goldenModernRequestFixture is a Modern (2026-07-28) "tools/list" request +// emitted by a real github.com/modelcontextprotocol/go-sdk v1.7.0-pre.3 client. +// It exists so NewModernRequest's _meta shape is checked against an independent +// oracle, not just against assertions written by the same author who wrote the +// encoder. To regenerate: point any go-sdk v1.7 client at +// `yardstick-server --stateless` (>= v1.2.0, itself on go-sdk v1.7.0-pre.3) and +// capture the raw tools/list request body it sends. +const goldenModernRequestFixture = "testdata/golden_modern_request.json" + +// extractMeta pulls params._meta out of a decoded wire object as a +// map[string]any, failing the test with a clear message if the shape isn't +// what's expected. +func extractMeta(t *testing.T, wire map[string]any) map[string]any { + t.Helper() + params, ok := wire["params"].(map[string]any) + require.True(t, ok, "wire request must have an object params field: %#v", wire["params"]) + meta, ok := params["_meta"].(map[string]any) + require.True(t, ok, "params must have an object _meta field: %#v", params["_meta"]) + return meta +} + +// jsonKind reports the JSON type of a value decoded by encoding/json into +// an any (object/array/string/number/bool/null), discarding its content. +func jsonKind(v any) string { + switch v.(type) { + case map[string]any: + return "object" + case []any: + return "array" + case string: + return "string" + case float64: + return "number" + case bool: + return "bool" + case nil: + return "null" + default: + return fmt.Sprintf("%T", v) // unreachable for encoding/json output + } +} + +// metaShape reduces a decoded _meta map to its keys and each value's JSON +// kind, discarding nested content. NewModernRequest deliberately sends a +// minimal empty clientCapabilities object rather than mirroring the go-sdk +// client's specific declared capabilities (roots, sampling, ...), so nested +// capability fields are never expected to match value-for-value — only that +// the same top-level _meta keys exist and each is the same kind of JSON +// value ("shape/keys", per the acceptance criteria). +func metaShape(meta map[string]any) map[string]string { + shape := make(map[string]string, len(meta)) + for k, v := range meta { + shape[k] = jsonKind(v) + } + return shape +} + +// TestModernRequestMatchesGoldenSDKFixture checks that NewModernRequest's +// wire output byte-matches the golden go-sdk fixture's _meta shape and keys. +// +// A literal full-envelope byte comparison would not hold even for two +// semantically identical requests: RawRequest.marshal builds the body from a +// map[string]any (encoding/json sorts map keys alphabetically), while the +// go-sdk marshals a Go struct in its own field order. So this test scopes +// the byte-level comparison to what the two sides can actually agree on: the +// _meta object's keys and each value's kind, independently re-marshaled on +// each side. Since encoding/json deterministically sorts map keys, comparing +// the re-marshaled bytes of two map[string]string shapes is still a genuine +// byte-for-byte check — just correctly scoped to the part of the AC that is +// comparable byte-for-byte ("_meta shape/keys", not deep capability content +// that NewModernRequest never claims to reproduce). +func TestModernRequestMatchesGoldenSDKFixture(t *testing.T) { + t.Parallel() + + goldenBytes, err := os.ReadFile(goldenModernRequestFixture) + require.NoError(t, err, "golden fixture must exist; see the goldenModernRequestFixture doc comment to regenerate it") + var golden map[string]any + require.NoError(t, json.Unmarshal(goldenBytes, &golden)) + require.Equal(t, "tools/list", golden["method"], "golden fixture must have been captured for tools/list to match NewModernRequest below") + goldenMeta := extractMeta(t, golden) + + req, err := NewModernRequest("tools/list", nil) + require.NoError(t, err) + rawMeta := extractMeta(t, wireOf(t, req)) + + require.Equal(t, goldenMeta[MetaKeyProtocolVersion], rawMeta[MetaKeyProtocolVersion], + "protocolVersion must match exactly, not just in shape") + + // clientInfo is present in the SDK-captured fixture (the SDK client has an + // Implementation configured) but omitted by NewModernRequest per spec + // ("SHOULD", not "MUST" — see NewModernRequest's doc comment); exclude it + // from the shape comparison below rather than requiring both sides to + // agree on an optional key. + require.Contains(t, goldenMeta, MetaKeyClientInfo, "golden fixture is expected to carry optional clientInfo") + require.NotContains(t, rawMeta, MetaKeyClientInfo, "NewModernRequest omits clientInfo by default") + delete(goldenMeta, MetaKeyClientInfo) + + goldenShapeJSON, err := json.Marshal(metaShape(goldenMeta)) + require.NoError(t, err) + rawShapeJSON, err := json.Marshal(metaShape(rawMeta)) + require.NoError(t, err) + + require.True(t, bytes.Equal(goldenShapeJSON, rawShapeJSON), + "raw client _meta must byte-match the golden go-sdk fixture's _meta shape/keys (excluding optional clientInfo)\ngolden shape: %s\nraw shape: %s", + goldenShapeJSON, rawShapeJSON) +} diff --git a/test/e2e/testdata/golden_modern_request.json b/test/e2e/testdata/golden_modern_request.json new file mode 100644 index 0000000000..3576d57959 --- /dev/null +++ b/test/e2e/testdata/golden_modern_request.json @@ -0,0 +1,19 @@ +{ + "jsonrpc": "2.0", + "id": 2, + "method": "tools/list", + "params": { + "_meta": { + "io.modelcontextprotocol/clientCapabilities": { + "roots": { + "listChanged": true + } + }, + "io.modelcontextprotocol/clientInfo": { + "name": "golden-fixture-capture", + "version": "0.0.0" + }, + "io.modelcontextprotocol/protocolVersion": "2026-07-28" + } + } +} From 68d008d1a95ffb50aa4cfc01cdc541c8b56a940f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 16:44:02 +0200 Subject: [PATCH 03/18] Add dual-era cross-delivery e2e test The streamable proxy's Modern (2026-07-28) routing must give every request a fresh routing token and never fall through to a client-supplied Mcp-Session-Id -- otherwise, when concurrent requests share the same (session, id) pair, one client's response can be delivered to another: a confidentiality bug. Unit tests can't observe this; it only surfaces when real traffic races through a running proxy. Drives yardstick-server v1.2.0 in barrier mode (buffers N requests, then releases together) behind the streamable proxy, firing N concurrent Modern requests that share one Mcp-Session-Id and one JSON-RPC id and differ only by a per-request nonce. Asserts conservation: exactly N distinct nonces, each response carrying the nonce of the request that sent it, zero timeouts, zero non-200. A one-shot positive control fires N-1 and asserts they are withheld until the barrier's safety timeout, so a barrier that silently degraded to a no-op fails the test rather than passing while forcing zero collisions. Bumps yardstick 1.1.1 -> 1.2.0 (adds barrier mode) and pre-pulls it. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/e2e-tests.yml | 6 +- test/e2e/dual_era_proxy_test.go | 264 ++++++++++++++++++++++++++++++++ test/e2e/images/images.go | 2 +- 3 files changed, 269 insertions(+), 3 deletions(-) create mode 100644 test/e2e/dual_era_proxy_test.go diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index ea409deb08..0aad9b00cf 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -132,11 +132,13 @@ jobs: docker pull ghcr.io/stacklok/toolhive/egress-proxy:latest & # yardstick is only needed for the vmcp test suite if [ "${{ matrix.label_filter }}" = "vmcp" ]; then - docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.1.1 & + docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0 & fi - # the time server is only used by the proxy test suite + # the time server and yardstick (dual-era barrier mode) are only + # used by the proxy (streamable-http) test suite if [ "${{ matrix.label_filter }}" = "proxy && !isolation" ]; then docker pull ghcr.io/stacklok/dockyard/uvx/mcp-server-time:2026.1.26 & + docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0 & fi # Envoy is only used by the network-isolation test suite if [ "${{ matrix.title }}" = "network-isolation" ]; then diff --git a/test/e2e/dual_era_proxy_test.go b/test/e2e/dual_era_proxy_test.go new file mode 100644 index 0000000000..1c29f0fcfe --- /dev/null +++ b/test/e2e/dual_era_proxy_test.go @@ -0,0 +1,264 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e_test + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strings" + "sync" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/stacklok/toolhive/test/e2e" + "github.com/stacklok/toolhive/test/e2e/images" +) + +// This spec is the crown-jewel regression test for the streamable HTTP +// proxy's per-request routing (pkg/transport/proxy/streamable): a Modern +// (2026-07-28) request always gets a fresh UUID routing token and never +// falls through to the client-supplied Mcp-Session-Id, even when many +// concurrent Modern requests share the same (session, id) pair. If that +// guarantee regresses, one client's response can be delivered to another +// client waiting on the identical composite key -- a confidentiality bug, +// see resolveSessionForRequest's doc comment in streamable_proxy.go. +// +// yardstick-server's barrier mode (BACKEND_MODE=barrier) buffers BARRIER_N +// non-lifecycle requests and releases all their responses at once, forcing +// BARRIER_N requests to be simultaneously in-flight at the proxy -- the +// precondition the leak needs. +// +// This spec loops internally (rounds), and CI additionally runs it repeated +// (ginkgo --repeat / go test -count) plus nightly, since a race window this +// narrow does not reliably reproduce on a single run. +var _ = Describe("Dual-Era Cross-Delivery Proxy", Label("proxy", "stateless", "dual-era", "e2e"), Serial, func() { + const ( + barrierN = 8 + rounds = 20 + barrierTimeout = 3 * time.Second + // clientTimeout bounds a single HTTP round trip: comfortably above + // barrierTimeout so a legitimate barrier release is never mistaken + // for a stuck proxy, but still fails the test (instead of the + // suite's global timeout) if the proxy hangs. + clientTimeout = barrierTimeout + 10*time.Second + // wellBeforeBarrierTimeout bounds how long any single response may + // take. Barrier mode's safety valve flushes a *partial* buffer on + // starvation, so "N responses came back" alone doesn't prove the + // barrier filled to N -- a response arriving near/after + // barrierTimeout means the round never actually forced all N + // requests to collide, and must fail rather than pass. + wellBeforeBarrierTimeout = barrierTimeout / 2 + + // All requests in a round deliberately share this session and id -- + // the exact leak precondition -- and differ only by nonce. + crossDeliverySessionID = "cross-delivery-probe" + crossDeliveryRequestID = int64(1) + ) + + var ( + config *e2e.TestConfig + serverName string + client *e2e.RawMCPClient + proxyURL string + ) + + BeforeEach(func() { + config = e2e.NewTestConfig() + serverName = e2e.GenerateUniqueServerName("dual-era") + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + client, err = e2e.NewRawMCPClient(clientTimeout) + Expect(err).ToNot(HaveOccurred()) + + By("Starting the yardstick backend under --transport stdio with BACKEND_MODE=barrier") + e2e.NewTHVCommand(config, "run", + "--name", serverName, + "--transport", "stdio", + "-e", "BACKEND_MODE=barrier", + "-e", fmt.Sprintf("BARRIER_N=%d", barrierN), + "-e", fmt.Sprintf("BARRIER_TIMEOUT_SECONDS=%d", int(barrierTimeout.Seconds())), + images.YardstickServerImage, + ).ExpectSuccess() + + err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "server should be running within 60 seconds") + + proxyURL, err = e2e.GetMCPServerURL(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to get proxy URL") + if !strings.HasSuffix(proxyURL, "/mcp") { + proxyURL += "/mcp" + } + }) + + AfterEach(func() { + if config.CleanupAfter { + err := e2e.StopAndRemoveMCPServer(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to stop and remove server") + } + }) + + It("never delivers one client's response to another under forced request collisions", func() { + // Positive control, run once: barrierN-1 requests can never fill the + // barrier, so they must be held until BARRIER_TIMEOUT_SECONDS's safety + // flush (~barrierTimeout), not answered immediately. Without this, a + // barrier that silently degraded into a no-op (answering every request + // right away) would be indistinguishable from a real barrier: both + // produce all-N-fast in the rounds below, so the crown jewel would stay + // green while forcing zero collisions. + By("Verifying the barrier actually withholds a short-of-N batch until its safety timeout") + results := fireCrossDeliveryBatch(client, proxyURL, "control", barrierN-1, barrierTimeout, + crossDeliverySessionID, crossDeliveryRequestID) + assertCorrelated(results, barrierN-1, "control") + for i, r := range results { + ExpectWithOffset(1, r.elapsed).To(BeNumerically(">=", wellBeforeBarrierTimeout), + "control request %d: returned too fast (%v) for a %d-of-%d batch -- the barrier is not "+ + "withholding responses below N, so it cannot be trusted to force the collision the rounds below rely on", + i, r.elapsed, barrierN-1, barrierN) + } + + for round := 0; round < rounds; round++ { + label := fmt.Sprintf("round-%d", round) + results := fireCrossDeliveryBatch(client, proxyURL, label, barrierN, barrierTimeout, + crossDeliverySessionID, crossDeliveryRequestID) + assertCorrelated(results, barrierN, label) + for i, r := range results { + ExpectWithOffset(1, r.elapsed).To(BeNumerically("<", wellBeforeBarrierTimeout), + "%s request %d: response arrived too close to BARRIER_TIMEOUT_SECONDS (%v) -- the barrier "+ + "likely flushed a partial buffer via its safety timeout instead of filling to N, meaning this "+ + "round never forced the collision", label, i, barrierTimeout) + } + } + }) +}) + +// crossDeliveryResult is one goroutine's outcome within a single barrier batch. +type crossDeliveryResult struct { + nonce string // the nonce this goroutine sent + err error + statusCode int + elapsed time.Duration + rpcErr *e2e.RawRPCError + gotNonce string // the nonce reflected back in the response, if any +} + +// fireCrossDeliveryBatch fires count concurrent Modern tools/call requests -- +// all sharing the same foreign Mcp-Session-Id and JSON-RPC id, differing only +// by a per-request nonce carried in _meta -- and returns each goroutine's +// outcome, indexed by request number. label distinguishes nonces/failure +// messages across batches (a round number, or "control"). +func fireCrossDeliveryBatch( + client *e2e.RawMCPClient, + proxyURL, label string, + count int, + barrierTimeout time.Duration, + sessionID string, + requestID int64, +) []crossDeliveryResult { + // joinTimeout bounds the whole batch: comfortably above barrierTimeout (the + // longest a request should ever be held) so a stuck proxy fails fast + // instead of hanging until the suite's global timeout. + joinTimeout := barrierTimeout + 15*time.Second + ctx, cancel := context.WithTimeout(context.Background(), joinTimeout) + defer cancel() + + results := make([]crossDeliveryResult, count) + var wg sync.WaitGroup + start := make(chan struct{}) + + for i := 0; i < count; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + nonce := fmt.Sprintf("%s-req-%d", label, i) + <-start // released together -- sequential firing deadlocks the barrier + + req, err := e2e.NewModernRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "crossdelivery"}, + }) + if err != nil { + results[i] = crossDeliveryResult{nonce: nonce, err: err} + return + } + req.WithID(requestID).WithSessionID(sessionID).SetMeta("nonce", nonce) + + reqStart := time.Now() + resp, sendErr := client.Send(ctx, proxyURL, req) + elapsed := time.Since(reqStart) + if sendErr != nil { + results[i] = crossDeliveryResult{nonce: nonce, err: sendErr, elapsed: elapsed} + return + } + results[i] = crossDeliveryResult{ + nonce: nonce, + statusCode: resp.StatusCode, + elapsed: elapsed, + rpcErr: resp.Error, + gotNonce: extractNonce(resp), + } + }(i) + } + + close(start) // fire all requests concurrently + + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(joinTimeout): + Fail(fmt.Sprintf("%s: timed out waiting for %d goroutines to finish", label, count)) + } + + return results +} + +// assertCorrelated checks the universal cross-delivery invariants for a batch: +// every request succeeded with no error/JSON-RPC error/non-200, and every +// response's nonce matches the request that goroutine sent (a mismatch is a +// cross-delivery leak), with exactly wantCount distinct nonces observed. +func assertCorrelated(results []crossDeliveryResult, wantCount int, label string) { + distinctNonces := map[string]bool{} + for i, r := range results { + ExpectWithOffset(2, r.err).ToNot(HaveOccurred(), + "%s request %d: must not error or time out", label, i) + ExpectWithOffset(2, r.rpcErr).To(BeNil(), + "%s request %d: must not get a JSON-RPC error: %+v", label, i, r.rpcErr) + ExpectWithOffset(2, r.statusCode).To(Equal(http.StatusOK), + "%s request %d: must return HTTP 200", label, i) + ExpectWithOffset(2, r.gotNonce).To(Equal(r.nonce), + "%s request %d: response nonce must match the request this goroutine sent -- "+ + "a mismatch is a cross-delivery leak", label, i) + distinctNonces[r.gotNonce] = true + } + ExpectWithOffset(2, distinctNonces).To(HaveLen(wantCount), + "%s: must see exactly %d distinct correlated nonces", label, wantCount) +} + +// extractNonce pulls the echoed "nonce" key out of a tools/call response's +// result._meta, per yardstick-server's echo tool: "Also echoes back any +// _meta field from the request for testing metadata propagation." Returns +// "" if absent or unparseable, which the caller's nonce-equality assertion +// then reports as a mismatch. +func extractNonce(resp *e2e.RawResponse) string { + if resp.Result == nil { + return "" + } + var result struct { + Meta map[string]any `json:"_meta"` + } + if err := json.Unmarshal(resp.Result, &result); err != nil { + return "" + } + nonce, _ := result.Meta["nonce"].(string) + return nonce +} diff --git a/test/e2e/images/images.go b/test/e2e/images/images.go index 09d2547f0d..6f009b3259 100644 --- a/test/e2e/images/images.go +++ b/test/e2e/images/images.go @@ -12,7 +12,7 @@ package images const ( yardstickServerImageURL = "ghcr.io/stackloklabs/yardstick/yardstick-server" - yardstickServerImageTag = "1.1.1" + yardstickServerImageTag = "1.2.0" // YardstickServerImage is used in operator tests across multiple transport protocols // (stdio, SSE, streamable-http) and tenancy modes. // Note: This image is also referenced in 8 YAML fixture files under From 1c5a1d908586b975cfd7d4f741ea3dabfa338ff0 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 17:12:28 +0200 Subject: [PATCH 04/18] Guard Modern path from client session-id reuse The streamable proxy's Modern (2026-07-28) branch mints a fresh routing token and ignores any client-supplied Mcp-Session-Id. Its safety rests on an unenforced assumption: the client session id has no downstream authority here (stateless stdio backend), unlike the transparent proxy, which maps a session id to a backend and validates it. The existing Modern tests all use unregistered session ids, so none would catch a regression that reused a *known* client session id as the routing token (fall-back-on-lookup-hit) -- silently reopening the validation-bypass class the transparent proxy already hardened against. Adds TestModernNeverReusesClientSessionIDAsRoutingToken, which registers a real session and asserts the Modern branch still mints fresh UUID tokens distinct from it (mutation-verified: fails if the branch reuses a known session id, while the pre-existing tests stay green). Strengthens the resolveSessionForRequest doc comment to tie the safety to the no-downstream-authority assumption and warn against reintroducing a lookup on this path. Co-Authored-By: Claude Opus 4.8 --- .../proxy/streamable/streamable_proxy.go | 9 +++ .../streamable_proxy_modern_test.go | 57 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/pkg/transport/proxy/streamable/streamable_proxy.go b/pkg/transport/proxy/streamable/streamable_proxy.go index 3f69efb7fa..e794736998 100644 --- a/pkg/transport/proxy/streamable/streamable_proxy.go +++ b/pkg/transport/proxy/streamable/streamable_proxy.go @@ -1042,6 +1042,15 @@ func (p *HTTPProxy) resolveSessionForRequest( // ignore any client-supplied Mcp-Session-Id. Never fall through to the // session-lookup path below with it -- see the confidentiality note // in this function's doc comment. + // + // This is safe only because, in this proxy, Mcp-Session-Id has no + // downstream authority: it is purely an in-process response-correlation + // token (the backend is stateless stdio, unlike the transparent proxy, + // which maps it to a backend-assigned SID via session metadata). If this + // proxy ever gains a shared session store or a stateful backend mapping, + // "ignore the client SID on Modern" must stay true -- silently reusing or + // looking up a known client SID here would reopen a session-validation + // bypass. See TestModernNeverReusesClientSessionIDAsRoutingToken. token, err := uuid.NewRandom() if err != nil { writeHTTPError(w, http.StatusInternalServerError, fmt.Sprintf("Failed to generate routing token: %v", err)) diff --git a/pkg/transport/proxy/streamable/streamable_proxy_modern_test.go b/pkg/transport/proxy/streamable/streamable_proxy_modern_test.go index fdd676d0c0..d226792cbd 100644 --- a/pkg/transport/proxy/streamable/streamable_proxy_modern_test.go +++ b/pkg/transport/proxy/streamable/streamable_proxy_modern_test.go @@ -10,9 +10,11 @@ import ( "fmt" "io" "net/http" + "net/http/httptest" "testing" "time" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/exp/jsonrpc2" @@ -175,6 +177,61 @@ func TestModernRequestIgnoresClientSessionID(t *testing.T) { assert.False(t, ok, "Modern path must not register the foreign session id") } +// TestModernNeverReusesClientSessionIDAsRoutingToken is the mutation-check +// analogue of the transparent proxy's TestGuardUnknownSessionFiresDespiteForgedModernRevision. +// Where that test proves an UNKNOWN session ID still triggers the Legacy +// guard, this one closes the complementary gap: it registers a REAL, known +// session in sessionManager and proves resolveSessionForRequest still mints +// a fresh routing token instead of reusing it. +// +// TestModernRequestIgnoresClientSessionID (above) only exercises a bogus, +// unregistered session ID, so it cannot detect a regression that consults +// sessionManager as a fallback before minting: on a lookup miss such code +// would still mint a fresh token and pass that test. Using a real registered +// session here closes that hole -- this test FAILS if the Modern branch is +// ever re-keyed on Mcp-Session-Id presence (i.e. reuses a known client SID as +// the routing token, or calls sessionManager.Get/AddWithID for it at all). +func TestModernNeverReusesClientSessionIDAsRoutingToken(t *testing.T) { + t.Parallel() + + proxy := NewHTTPProxy("127.0.0.1", 0, nil, nil) + t.Cleanup(func() { _ = proxy.Stop(context.Background()) }) + + knownSessionID := uuid.New().String() + require.NoError(t, proxy.sessionManager.AddWithID(knownSessionID)) + + newModernRequest := func() *jsonrpc2.Request { + req, err := jsonrpc2.NewCall(jsonrpc2.Int64ID(1), "tools/call", json.RawMessage( + `{"_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28",`+ + `"io.modelcontextprotocol/clientCapabilities":{}}}`)) + require.NoError(t, err) + return req + } + + resolve := func() string { + httpReq := httptest.NewRequest(http.MethodPost, StreamableHTTPEndpoint, http.NoBody) + httpReq.Header.Set("MCP-Protocol-Version", mcp.MCPVersionModern) + httpReq.Header.Set("Mcp-Session-Id", knownSessionID) + + token, setHeader, err := proxy.resolveSessionForRequest(httptest.NewRecorder(), httpReq, newModernRequest()) + require.NoError(t, err) + assert.False(t, setHeader, "Modern branch must never ask the client to adopt a session") + return token + } + + token1 := resolve() + token2 := resolve() + + assert.NotEqual(t, knownSessionID, token1, "Modern branch must not reuse the known client session id as its routing token") + assert.NotEqual(t, knownSessionID, token2, "Modern branch must not reuse the known client session id as its routing token") + assert.NotEqual(t, token1, token2, "each Modern request must mint its own fresh routing token") + + _, err := uuid.Parse(token1) + assert.NoError(t, err, "routing token must be a freshly minted UUID, not a passthrough of client input") + _, err = uuid.Parse(token2) + assert.NoError(t, err, "routing token must be a freshly minted UUID, not a passthrough of client input") +} + // TestModernClassificationErrorsReturn400 verifies that ClassifyRevision errors // on the single-request path are rendered as HTTP 400 JSON-RPC error responses // with the spec-defined error code. From 415d4e5cf383ebd811e4ab13da3289334f4c3f70 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 17:13:13 +0200 Subject: [PATCH 05/18] Add dual-era hostile-input e2e test The transparent proxy classifies each request's MCP revision and must reject malformed or hostile Modern (2026-07-28) inputs at the edge, before contacting the backend. This drives that boundary with a raw client through a session-aware transparent proxy fronting an in-process mock. Covers, each asserting the exact JSON-RPC error code, error.data, echoed id, and that the backend was not contacted: header/body version mismatch (-32020), unsupported version (-32022), missing clientCapabilities (-32021), and reserved _meta with no valid version (-32602). Also asserts a well-formed Modern request with a foreign Mcp-Session-Id is rejected 404 -32001 (correct-by-design: the session guard keys on header presence, not the client-forgeable revision) and with none is forwarded once (200); an oversized body is edge-rejected 413 while a truncated body is forwarded as Legacy passthrough (the proxy does not validate JSON syntax); and the proxy still serves a well-formed request afterward. Documents the deferred/known-gap cases (Mcp-Method/Mcp-Name, batch, GET/DELETE 405). Co-Authored-By: Claude Opus 4.8 --- test/e2e/hostile_input_proxy_test.go | 282 +++++++++++++++++++++++++++ 1 file changed, 282 insertions(+) create mode 100644 test/e2e/hostile_input_proxy_test.go diff --git a/test/e2e/hostile_input_proxy_test.go b/test/e2e/hostile_input_proxy_test.go new file mode 100644 index 0000000000..a890346384 --- /dev/null +++ b/test/e2e/hostile_input_proxy_test.go @@ -0,0 +1,282 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e_test + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strings" + "time" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/stacklok/toolhive/pkg/mcp" + "github.com/stacklok/toolhive/pkg/transport/session" + "github.com/stacklok/toolhive/test/e2e" +) + +// This spec is the hostile-input tier of the dual-era proxy e2e suite (issue +// #5837, tier 2): a hand-rolled raw client drives malformed/adversarial +// Modern (2026-07-28) traffic at the transparent proxy (remote MCP +// passthrough, session-aware -- no --stateless) fronting the in-process +// statelessMockMCPServer (see stateless_proxy_test.go), and asserts each +// rejection fires at the edge -- exact error.code, error.data, and echoed +// id -- without the backend ever being contacted. +// +// Known gaps, deliberately not asserted here (see the design plan and +// pkg/mcp/revision.go's ClassifyRevision doc comment): Mcp-Method/Mcp-Name +// header validation (unimplemented), batch/client-response smuggling +// (handled as Legacy today, deferred), and header-driven GET/DELETE 405 +// (that gate is stateless-mode-driven, not classifier-driven). Also a known +// spec divergence: ClassifyRevision is transport-agnostic and cannot tell +// "stdio, no header concept" (-32602 is correct) apart from "HTTP, header +// omitted" (where the draft's Server Validation rules make a missing +// MCP-Protocol-Version header itself a -32020 condition) -- see the TODO in +// revision.go. This suite exercises the HTTP transport but the classifier +// currently returns -32602 either way. +var _ = Describe("Hostile Input Proxy", Label("proxy", "stateless", "hostile-input", "e2e"), Serial, func() { + var ( + config *e2e.TestConfig + serverName string + mockServer *statelessMockMCPServer + client *e2e.RawMCPClient + proxyURL string + ) + + BeforeEach(func() { + config = e2e.NewTestConfig() + serverName = e2e.GenerateUniqueServerName("hostile-input") + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + mockServer, err = newStatelessMockMCPServer() + Expect(err).ToNot(HaveOccurred(), "should be able to start mock server") + + client, err = e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + By("Starting thv against the in-process mock (transparent proxy, session-aware)") + e2e.NewTHVCommand(config, "run", "--name", serverName, mockServer.URL()+"/mcp").ExpectSuccess() + + err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "server should be running within 60 seconds") + + proxyURL, err = e2e.GetMCPServerURL(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to get proxy URL") + if !strings.HasSuffix(proxyURL, "/mcp") { + proxyURL += "/mcp" + } + }) + + AfterEach(func() { + if mockServer != nil { + mockServer.Stop() + mockServer = nil + } + if config.CleanupAfter { + err := e2e.StopAndRemoveMCPServer(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to stop and remove server") + } + }) + + It("rejects hostile Modern inputs at the edge without contacting the backend", func() { + // A manual loop over cases, not DescribeTable: BeforeEach's thv run + // setup (~seconds) is shared once across all cases here instead of + // paying it 5x, one per DescribeTable entry. + for _, tc := range hostileRejectionCases() { + By(tc.name) + countBefore := mockServer.GetCount() + + req, err := e2e.NewModernRequest("tools/list", nil) + Expect(err).ToNot(HaveOccurred(), tc.name) + req.WithID(tc.id) + tc.mutate(req) + + resp, err := client.Send(context.Background(), proxyURL, req) + Expect(err).ToNot(HaveOccurred(), tc.name) + Expect(resp.StatusCode).To(Equal(tc.wantStatus), tc.name) + if tc.idNotEchoed { + Expect(resp.ID).To(BeNil(), "%s: echoed id", tc.name) + } else { + Expect(resp.ID).To(Equal(json.Number(fmt.Sprintf("%d", tc.id))), "%s: echoed id", tc.name) + } + Expect(resp.Error).ToNot(BeNil(), tc.name) + Expect(resp.Error.Code).To(Equal(tc.wantCode), tc.name) + if tc.wantData != nil { + var gotData map[string]any + Expect(json.Unmarshal(resp.Error.Data, &gotData)).To(Succeed(), "%s: error.data must decode", tc.name) + Expect(gotData).To(Equal(tc.wantData), "%s: error.data", tc.name) + } else { + Expect(resp.Error.Data).To(BeEmpty(), "%s: error.data should be absent", tc.name) + } + Expect(mockServer.GetCount()).To(Equal(countBefore), + "%s: backend must not be contacted for a rejected request", tc.name) + } + }) + + It("forwards a well-formed Modern request with no session id (200)", func() { + countBefore := mockServer.GetCount() + + req, err := e2e.NewModernRequest("tools/list", nil) + Expect(err).ToNot(HaveOccurred()) + req.WithID(int64(2001)) + + resp, err := client.Send(context.Background(), proxyURL, req) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + Expect(resp.Error).To(BeNil()) + Expect(resp.ID).To(Equal(json.Number("2001"))) + // "exactly once", not just ">= 1": proves the proxy forwards a + // well-formed sessionless Modern request to the backend a single + // time -- no retry, no double-send. + Expect(mockServer.GetCount()).To(Equal(countBefore+1), "backend must be contacted exactly once") + }) + + It("handles oversized and truncated bodies cleanly, without a crash or hang", func() { + By("an oversized body is rejected with 413, not forwarded") + countBefore := mockServer.GetCount() + oversized := []byte(fmt.Sprintf( + `{"jsonrpc":"2.0","id":3001,"method":"tools/list","params":{"padding":"%s"}}`, + strings.Repeat("a", 9<<20))) // 9 MiB, above bodylimit.DefaultMaxRequestBodySize (8 MiB) + resp, err := client.SendRaw(context.Background(), proxyURL, + map[string]string{"Content-Type": "application/json"}, oversized) + Expect(err).ToNot(HaveOccurred(), "must not hang or crash the proxy") + Expect(resp.StatusCode).To(Equal(http.StatusRequestEntityTooLarge)) + Expect(mockServer.GetCount()).To(Equal(countBefore), + "oversized body must be rejected by bodylimit before the backend is contacted") + + // Unlike the classification cases above, an unparseable body is NOT + // rejected at the edge: parseRPCRequest fails to decode it as either a + // single request or a batch, so RoundTrip never calls ClassifyRevision + // and forwards the body unclassified (Legacy passthrough) -- the proxy + // does not validate JSON syntax before forwarding. The 400 below comes + // from the *backend's* own json.Unmarshal failure, after it has + // already been contacted. + By("a truncated body is forwarded as Legacy passthrough; the backend rejects it") + countBefore = mockServer.GetCount() + truncated := []byte(`{"jsonrpc":"2.0","id":3002,"method":"tools/list","para`) // deliberately cut mid-object + resp, err = client.SendRaw(context.Background(), proxyURL, + map[string]string{"Content-Type": "application/json"}, truncated) + Expect(err).ToNot(HaveOccurred(), "must not hang or crash the proxy") + Expect(resp.StatusCode).To(Equal(http.StatusBadRequest)) + Expect(mockServer.GetCount()).To(Equal(countBefore+1), + "an unparseable body skips classification and IS forwarded to the backend") + + By("the proxy still serves a well-formed request after the hostile inputs above") + req, err := e2e.NewModernRequest("tools/list", nil) + Expect(err).ToNot(HaveOccurred()) + req.WithID(int64(3003)) + resp, err = client.Send(context.Background(), proxyURL, req) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusOK), "proxy must survive the oversized/truncated requests above") + }) +}) + +// hostileCase is one adversarial Modern request: start from a well-formed +// NewModernRequest, apply mutate to break exactly one classification/session +// rule, and expect the proxy to reject it before the backend is contacted. +type hostileCase struct { + name string + id int64 + mutate func(req *e2e.RawRequest) + wantStatus int + wantCode int64 + wantData map[string]any // nil means the error must carry no "data" field at all + // idNotEchoed marks a rejection path that -- unlike ClassificationErrorResponse + // -- doesn't echo the request id at all: session.NotFoundResponse hardcodes a + // nil id (see jsonrpc_errors.go's NotFoundBody(nil) call). A real minor + // asymmetry in the current code, left flagged rather than fixed (out of scope + // for a test-only step). + idNotEchoed bool +} + +// hostileRejectionCases enumerates the classification-error and session-guard +// rejections the transparent proxy must produce for adversarial Modern +// traffic. Each starts from NewModernRequest's well-formed default (header +// MCP-Protocol-Version + _meta.protocolVersion both "2026-07-28", +// _meta.clientCapabilities {}) and mutates exactly the one thing that breaks it. +func hostileRejectionCases() []hostileCase { + return []hostileCase{ + { + name: "header/body protocolVersion mismatch -> -32020", + id: 1001, + mutate: func(req *e2e.RawRequest) { + // Body still claims Modern (NewModernRequest's default _meta); + // the header disagrees. Neither value wins -- hard rejection. + req.SetHeader(e2e.HeaderMCPProtocolVersion, "2025-11-25") + }, + wantStatus: http.StatusBadRequest, + wantCode: mcp.CodeHeaderMismatch, + wantData: map[string]any{"header": "2025-11-25", "body": e2e.MCPVersionModern}, + }, + { + name: "unsupported version in _meta.protocolVersion -> -32022", + id: 1002, + mutate: func(req *e2e.RawRequest) { + req.SetMeta(e2e.MetaKeyProtocolVersion, "2099-01-01") + }, + wantStatus: http.StatusBadRequest, + wantCode: mcp.CodeUnsupportedProtocolVersion, + wantData: map[string]any{"supported": []any{e2e.MCPVersionModern}, "requested": "2099-01-01"}, + }, + { + name: "missing clientCapabilities -> -32021", + id: 1003, + mutate: func(req *e2e.RawRequest) { + req.DeleteMeta(e2e.MetaKeyClientCapabilities) + }, + wantStatus: http.StatusBadRequest, + wantCode: mcp.CodeMissingClientCapability, + wantData: map[string]any{"requiredCapabilities": map[string]any{}}, + }, + { + name: "reserved _meta key, no header, no valid version -> -32602", + id: 1004, + mutate: func(req *e2e.RawRequest) { + // clientCapabilities alone remains as the reserved-key Modern + // signal; with no header and no body protocolVersion, the draft + // spec defines no dedicated code, so this falls back to the + // standard JSON-RPC Invalid Params code. + req.DeleteHeader(e2e.HeaderMCPProtocolVersion) + req.DeleteMeta(e2e.MetaKeyProtocolVersion) + }, + wantStatus: http.StatusBadRequest, + wantCode: mcp.CodeInvalidParams, + wantData: nil, + }, + { + name: "foreign Mcp-Session-Id -> 404 session-not-found", + id: 1005, + mutate: func(req *e2e.RawRequest) { + // Otherwise well-formed Modern request, but with a session id + // never registered with this proxy. The 2026-07-28 draft spec + // says Modern SHOULD ignore session id -- and the *streamable* + // proxy does (resolveSessionForRequest mints a fresh routing + // token unconditionally, see the crown jewel). The transparent + // proxy deliberately deviates: here Mcp-Session-Id has + // downstream authority -- it's looked up in the shared session + // store, rewritten to the backend's assigned SID, and forwarded + // to a stateful backend (see RoundTrip's guard + rewrite in + // transparent_proxy.go). Branching that guard on the + // client-forgeable classified revision instead of header + // presence would be fail-open: a forged Modern _meta could + // carry a downstream-authoritative SID past session validation. + // Keying on presence alone is the fail-safe choice, and it is + // the security invariant this suite exists to pin down -- + // see TestGuardUnknownSessionFiresDespiteForgedModernRevision + // in revision_guard_regression_test.go. + req.WithSessionID("foreign-" + uuid.NewString()) + }, + wantStatus: http.StatusNotFound, + wantCode: session.CodeSessionNotFound, + wantData: nil, + idNotEchoed: true, + }, + } +} From 9b6fbb273d92ca0be70b4075974e32ac958c5388 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 17:28:20 +0200 Subject: [PATCH 06/18] Add dual-era concurrent mixing e2e test Legacy (session-based) and Modern (stateless) clients must be able to share one streamable-proxy instance concurrently without cross-delivering responses or losing either era's semantics. This fires several Legacy sessions and stateless Modern requests together each round (released via a shared start channel so both eras are genuinely in flight, asserted by a per-round timestamp-overlap check) and correlates every response by a unique _meta nonce -- a wrong nonce is a cross-era or cross-client leak. Era-distinctness is asserted where it is observable: a negative probe confirms a bogus Mcp-Session-Id is rejected (404 -32001), proving Legacy enforces its session rather than merely minting one, while Modern responses must carry no Mcp-Session-Id. Legacy initialize assigning a session id is the standing Legacy-behavior gate. Scope: with an echo backend this proves response-body delivery correctness, not session-metadata isolation; forced-collision is the crown jewel's job. Accept: text/event-stream is intentionally not set -- it flips this proxy to an SSE response body the raw client does not parse; the plain-JSON path is what this tier exercises. Co-Authored-By: Claude Opus 4.8 --- test/e2e/dual_era_mixing_test.go | 285 +++++++++++++++++++++++++++++++ 1 file changed, 285 insertions(+) create mode 100644 test/e2e/dual_era_mixing_test.go diff --git a/test/e2e/dual_era_mixing_test.go b/test/e2e/dual_era_mixing_test.go new file mode 100644 index 0000000000..9337bb72cc --- /dev/null +++ b/test/e2e/dual_era_mixing_test.go @@ -0,0 +1,285 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e_test + +import ( + "context" + "fmt" + "strings" + "sync" + "time" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/stacklok/toolhive/test/e2e" + "github.com/stacklok/toolhive/test/e2e/images" +) + +// This spec is the dual-era mixing tier of the e2e suite (issue #5837, tier +// 2): Legacy (2025-11-25, session-based) and Modern (2026-07-28, stateless) +// raw clients hit the same streamable proxy instance concurrently. This +// tier exercises concurrent mixed-era *coexistence* -- it does NOT force a +// simultaneity collision like the crown jewel (dual_era_proxy_test.go) does +// with a barrier backend. What it proves is response-body delivery +// correctness (no client ever receives another client's response body), +// bounded by the echo backend having no observable session-scoped state -- +// it does not prove session-metadata isolation inside the proxy itself. +// +// Cross-delivery correlation reuses the crown jewel's mechanism +// (dual_era_proxy_test.go's extractNonce): every request, Legacy or Modern, +// carries a unique nonce in a custom _meta key (not one of the reserved +// io.modelcontextprotocol/* keys, so it never flips a Legacy request's +// classification -- see pkg/mcp/revision.go's hasModernSignal), and +// yardstick's echo tool reflects whatever _meta it received regardless of +// era. A response with the wrong nonce means this request's response body +// was delivered to (or received from) a different client. +// +// Confirmed live (thv run + curl) before writing this: Legacy initialize +// returns Mcp-Session-Id; a Legacy tools/call reusing that session id and a +// Modern tools/call both echo a custom _meta.nonce; neither ordinary +// tools/call response (Legacy or Modern) carries Mcp-Session-Id -- only +// initialize does; and a Legacy tools/call with a bogus/unregistered +// Mcp-Session-Id gets HTTP 404 with JSON-RPC -32001 "Session not found", +// proving the proxy actually enforces the session rather than merely +// minting one (see the negative probe below). So era-distinctness isn't a +// per-response header check; it's that Legacy requires and enforces a +// session id issued by initialize while Modern never sends one at all (see +// runMixedRound's assertions). + +// Deliberately NOT setting "Accept: application/json, text/event-stream" +// (mcp_raw_client.go's Send doc comment floats it for a live streamable-HTTP +// endpoint): verified empirically that doing so makes THIS proxy switch to +// its SSE response path (handleSingleRequestSSE in streamable_proxy.go, +// triggered by any Accept header containing "text/event-stream"), and the +// raw client's populateEnvelope only parses a plain JSON object body, not an +// SSE-framed one -- so nonce extraction silently returns "". Without the +// header the proxy returns a plain JSON response, which is what every other +// spec in this suite (and this one) relies on. +var _ = Describe("Dual-Era Concurrent Mixing", Label("proxy", "stateless", "dual-era", "e2e"), Serial, func() { + const ( + legacySessions = 3 + modernPerRound = 3 + rounds = 3 + clientTimeout = 15 * time.Second + // joinTimeout bounds one round: comfortably above clientTimeout (the + // longest a single request should take) so a stuck proxy fails fast + // instead of hanging until the suite's global timeout. + joinTimeout = clientTimeout + 10*time.Second + ) + + var ( + config *e2e.TestConfig + serverName string + client *e2e.RawMCPClient + proxyURL string + ) + + BeforeEach(func() { + config = e2e.NewTestConfig() + serverName = e2e.GenerateUniqueServerName("dual-era-mix") + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + client, err = e2e.NewRawMCPClient(clientTimeout) + Expect(err).ToNot(HaveOccurred()) + + By("Starting the yardstick backend under --transport stdio with BACKEND_MODE=echo") + e2e.NewTHVCommand(config, "run", + "--name", serverName, + "--transport", "stdio", + "-e", "BACKEND_MODE=echo", + images.YardstickServerImage, + ).ExpectSuccess() + + err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "server should be running within 60 seconds") + + proxyURL, err = e2e.GetMCPServerURL(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to get proxy URL") + if !strings.HasSuffix(proxyURL, "/mcp") { + proxyURL += "/mcp" + } + }) + + AfterEach(func() { + if config.CleanupAfter { + err := e2e.StopAndRemoveMCPServer(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to stop and remove server") + } + }) + + It("mixes concurrent Legacy and Modern traffic without cross-era delivery", func() { + // Standing gate: Legacy initialize assigns a session id (existing + // Legacy behavioral assertion, not a byte-for-byte golden -- no + // committed Legacy golden fixture exists to diff against; see + // mcp_raw_client_golden_test.go, which only covers Modern). + sessionIDs := make([]string, legacySessions) + for i := range sessionIDs { + initReq := e2e.NewLegacyInitializeRequest(fmt.Sprintf("legacy-client-%d", i), "1.0") + resp, err := client.Send(context.Background(), proxyURL, initReq) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(200)) + sessionIDs[i] = resp.Headers.Get(e2e.HeaderMCPSessionID) + Expect(sessionIDs[i]).ToNot(BeEmpty(), "initialize must assign a session id") + } + + // Negative probe: Legacy must actually ENFORCE the session, not just + // mint one. The echo tool is session-agnostic, so a Legacy path that + // silently degraded to stateless would still echo correctly and this + // suite's conservation checks alone wouldn't catch it. Confirmed live + // (thv run + curl) before writing this: a bogus/unregistered session + // id gets HTTP 404, JSON-RPC -32001 "Session not found". + bogusReq, err := e2e.NewLegacyRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "shouldneverrun"}, + }) + Expect(err).ToNot(HaveOccurred()) + bogusReq.WithSessionID("bogus-" + uuid.NewString()) + resp, err := client.Send(context.Background(), proxyURL, bogusReq) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).ToNot(Equal(200), "a bogus/unregistered Mcp-Session-Id must be rejected") + Expect(resp.Error).ToNot(BeNil(), "rejection must be a JSON-RPC error") + + for round := 0; round < rounds; round++ { + runMixedRound(client, proxyURL, round, sessionIDs, modernPerRound, joinTimeout) + } + }) +}) + +// mixResult is one goroutine's outcome within a single mixed round. +type mixResult struct { + era string // "legacy" or "modern", for failure messages + nonce string + gotNonce string + err error + statusCode int + rpcErr *e2e.RawRPCError + sessionHdr string // response's Mcp-Session-Id header, if any + start, end time.Time +} + +// runMixedRound fires len(sessionIDs) Legacy requests (each reusing its own +// already-initialized session) and modernPerRound stateless Modern requests +// concurrently -- a start channel releases every goroutine together, so both +// eras are genuinely in flight at once, not sequential per era (the crown +// jewel's anti-theater fire pattern). Every request carries a unique nonce +// in _meta; a wrong echoed nonce is a cross-era or cross-client leak. +func runMixedRound( + client *e2e.RawMCPClient, + proxyURL string, + round int, + sessionIDs []string, + modernPerRound int, + joinTimeout time.Duration, +) { + total := len(sessionIDs) + modernPerRound + results := make([]mixResult, total) + var wg sync.WaitGroup + start := make(chan struct{}) + + // Derived from joinTimeout (not context.Background()) so a stuck round + // tears down its in-flight requests promptly, mirroring the crown jewel. + ctx, cancel := context.WithTimeout(context.Background(), joinTimeout) + defer cancel() + + fire := func(i int, era string, build func() (*e2e.RawRequest, error)) { + wg.Add(1) + go func() { + defer wg.Done() + nonce := fmt.Sprintf("round-%d-%s-%d", round, era, i) + <-start + + req, err := build() + if err != nil { + results[i] = mixResult{era: era, nonce: nonce, err: err} + return + } + req.SetMeta("nonce", nonce) + + reqStart := time.Now() + resp, sendErr := client.Send(ctx, proxyURL, req) + reqEnd := time.Now() + if sendErr != nil { + results[i] = mixResult{era: era, nonce: nonce, err: sendErr, start: reqStart, end: reqEnd} + return + } + results[i] = mixResult{ + era: era, + nonce: nonce, + statusCode: resp.StatusCode, + rpcErr: resp.Error, + gotNonce: extractNonce(resp), + sessionHdr: resp.Headers.Get(e2e.HeaderMCPSessionID), + start: reqStart, + end: reqEnd, + } + }() + } + + for i, sid := range sessionIDs { + sid := sid + fire(i, "legacy", func() (*e2e.RawRequest, error) { + req, err := e2e.NewLegacyRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "mixcheck"}, + }) + if err != nil { + return nil, err + } + return req.WithSessionID(sid), nil + }) + } + for i := 0; i < modernPerRound; i++ { + fire(len(sessionIDs)+i, "modern", func() (*e2e.RawRequest, error) { + return e2e.NewModernRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "mixcheck"}, + }) + }) + } + + close(start) // release every goroutine (both eras) together + + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(joinTimeout): + Fail(fmt.Sprintf("round %d: timed out waiting for %d mixed requests to finish", round, total)) + } + + distinctNonces := map[string]bool{} + var legacyOverlapsModern bool + for i, r := range results { + ExpectWithOffset(1, r.err).ToNot(HaveOccurred(), "round %d %s request %d", round, r.era, i) + ExpectWithOffset(1, r.rpcErr).To(BeNil(), "round %d %s request %d: %+v", round, r.era, i, r.rpcErr) + ExpectWithOffset(1, r.statusCode).To(Equal(200), "round %d %s request %d", round, r.era, i) + ExpectWithOffset(1, r.gotNonce).To(Equal(r.nonce), + "round %d %s request %d: echoed nonce must match -- a mismatch is a cross-era/cross-client leak", + round, r.era, i) + if r.era == "modern" { + ExpectWithOffset(1, r.sessionHdr).To(BeEmpty(), + "round %d modern request %d: stateless Modern must never carry Mcp-Session-Id", round, i) + } + distinctNonces[r.gotNonce] = true + + // Anti-theater: both eras must have genuinely overlapped in flight at + // least once this round, not fired sequentially era-by-era. + for _, other := range results { + if r.era != other.era && r.start.Before(other.end) && other.start.Before(r.end) { + legacyOverlapsModern = true + } + } + } + ExpectWithOffset(1, distinctNonces).To(HaveLen(total), "round %d: must see %d distinct correlated nonces", round, total) + ExpectWithOffset(1, legacyOverlapsModern).To(BeTrue(), + "round %d: no Legacy request overlapped in time with a Modern request -- "+ + "this round fired the eras sequentially instead of concurrently", round) +} From 97f154fdbaa74ef04cfa72bea9535cbc24b5398c Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 20:01:49 +0200 Subject: [PATCH 07/18] Add dual-era Tier-1 functional e2e tests Tier 1 exercises the proxies with a real, independent MCP SDK client (yardstick-client, go-sdk v1.7.0-pre.3) so it catches spec misreadings a hand-rolled client would share with the proxy code. Modern fidelity runs yardstick-client against the streamable proxy and confirms via -action info that it genuinely negotiated 2026-07-28 with an empty session id (server/discover, no initialize) -- not a masked downgrade. Legacy runs it against a non-stateless backend whose discover omits 2026-07-28, forcing the SDK's fall-back to a 2025-11-25 initialize handshake that the transparent proxy must faithfully pass through (session id preserved). A single raw-client check covers the transparent proxy's own Modern no-Mcp-Session-Id path. Also guards server/discover under authz: with a Cedar policy enabled it must 403 (default-deny, backend not contacted) -- a regression alarm against allow-listing discover without response-filtering (it would bypass ResponseFilteringWriter) -- while a permitted echo call still succeeds. Notes the non-conformant denial envelope (#5950). yardstick-client is go-installed into a scratch GOBIN (separate module, does not touch this repo's go.mod), version derived from the yardstick image tag. Co-Authored-By: Claude Opus 4.8 --- test/e2e/dual_era_functional_test.go | 423 +++++++++++++++++++++++++++ 1 file changed, 423 insertions(+) create mode 100644 test/e2e/dual_era_functional_test.go diff --git a/test/e2e/dual_era_functional_test.go b/test/e2e/dual_era_functional_test.go new file mode 100644 index 0000000000..58dc6440d1 --- /dev/null +++ b/test/e2e/dual_era_functional_test.go @@ -0,0 +1,423 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e_test + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/stacklok/toolhive/test/e2e" + "github.com/stacklok/toolhive/test/e2e/images" +) + +// This spec is the Tier-1 functional round-trip tier of the e2e suite (issue +// #5837, Commit 2). Modern (2026-07-28) fidelity is driven by the real, +// independent SDK client (yardstick-client) against the streamable proxy -- +// call-tool succeeds and -action info confirms it actually negotiated +// 2026-07-28 with an empty session id, not a silent downgrade -- plus one +// raw-client wire check against the transparent proxy for the +// no-Mcp-Session-Id property (that proxy's own, distinct Modern code path). +// Legacy (2025-11-25) is yardstick-client negotiating down: per Step 2.1's +// discovery, yardstick-client emits genuine Modern traffic on its own +// (server/discover, no initialize) whenever the backend's own discover +// response advertises 2026-07-28 support, so the Legacy leg deliberately +// uses a NON-stateless yardstick-server (its own StreamableHTTPOptions.Stateless +// is false), whose discover response omits 2026-07-28, forcing the SDK +// client to fall back to a real initialize handshake -- confirmed live +// before writing this (see the two legs below). + +// execTimeout bounds go install and every yardstick-client invocation below, +// so a wedged module proxy or stuck client fails fast instead of riding out +// the suite's global timeout. +const execTimeout = 90 * time.Second + +// yardstickClientVersion is derived from images.YardstickServerImage's tag +// (the part after ":") so a future image bump can't silently pair a +// mismatched go-sdk version between yardstick-server and yardstick-client. +var yardstickClientVersion = "v" + strings.SplitN(images.YardstickServerImage, ":", 2)[1] + +var ( + yardstickClientOnce sync.Once + yardstickClientPath string + yardstickClientErr error +) + +// ensureYardstickClientBinary go-installs the real yardstick-client module +// (github.com/stackloklabs/yardstick, a separate Go module -- this does NOT +// add go-sdk v1.7 to this repo's go.mod) into a scratch GOBIN, once per test +// run, and returns its path. +// +// ponytail: the scratch dir is not removed -- it's shared (via sync.Once) +// across every spec in this file, so a DeferCleanup tied to whichever spec +// happens to trigger the install first would delete it out from under later +// specs. One leaked temp dir per suite run is a cheap tradeoff; the OS temp +// dir is reaped independently. Upgrade to a suite-level AfterSuite removal +// if that ever stops being true (e.g. a long-lived CI temp volume). +func ensureYardstickClientBinary() (string, error) { + yardstickClientOnce.Do(func() { + dir, err := os.MkdirTemp("", "yardstick-client-") + if err != nil { + yardstickClientErr = fmt.Errorf("create scratch GOBIN: %w", err) + return + } + + ctx, cancel := context.WithTimeout(context.Background(), execTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, "go", "install", + "github.com/stackloklabs/yardstick/cmd/yardstick-client@"+yardstickClientVersion) + cmd.Env = append(os.Environ(), "GOBIN="+dir) + if out, err := cmd.CombinedOutput(); err != nil { + yardstickClientErr = fmt.Errorf("go install yardstick-client@%s: %w: %s", yardstickClientVersion, err, out) + return + } + yardstickClientPath = filepath.Join(dir, "yardstick-client") + }) + return yardstickClientPath, yardstickClientErr +} + +// runYardstickClient runs yardstick-client with a bounded timeout and returns +// its combined output. +func runYardstickClient(clientPath string, args ...string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), execTimeout) + defer cancel() + //nolint:gosec // fixed action/flags, test-controlled address/port + out, err := exec.CommandContext(ctx, clientPath, args...).CombinedOutput() + return string(out), err +} + +// proxyPort extracts the port from a proxy URL like "http://127.0.0.1:PORT/mcp". +func proxyPort(proxyURL string) string { + rest := strings.TrimPrefix(proxyURL, "http://") + hostPort := strings.SplitN(rest, "/", 2)[0] + return strings.SplitN(hostPort, ":", 2)[1] +} + +// sessionIDFromInfo parses yardstick-client's `-action info` output and +// returns the value of its "Session ID: " line (empty for Modern, a real id +// for Legacy). Parsing the value, rather than substring-matching the whole +// line, survives a trailing space or relabel and can't pass vacuously. +func sessionIDFromInfo(infoOutput string) string { + for _, line := range strings.Split(infoOutput, "\n") { + if rest, ok := strings.CutPrefix(line, " Session ID:"); ok { + return strings.TrimSpace(rest) + } + } + return "" +} + +var _ = Describe("Modern Functional Round-Trip — Streamable Proxy", Label("proxy", "stateless", "dual-era", "e2e"), Serial, func() { + var ( + config *e2e.TestConfig + serverName string + client *e2e.RawMCPClient + proxyURL string + ) + + BeforeEach(func() { + config = e2e.NewTestConfig() + serverName = e2e.GenerateUniqueServerName("func-streamable") + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + client, err = e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + e2e.NewTHVCommand(config, "run", + "--name", serverName, + "--transport", "stdio", + "-e", "BACKEND_MODE=echo", + images.YardstickServerImage, + ).ExpectSuccess() + + err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "server should be running within 60 seconds") + + proxyURL, err = e2e.GetMCPServerURL(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to get proxy URL") + if !strings.HasSuffix(proxyURL, "/mcp") { + proxyURL += "/mcp" + } + }) + + AfterEach(func() { + if config.CleanupAfter { + err := e2e.StopAndRemoveMCPServer(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to stop and remove server") + } + }) + + It("round-trips through a real independent SDK client (yardstick-client) as Modern", func() { + // Wire-level "no Mcp-Session-Id for Modern" is already covered by the + // crown jewel + mixing specs; this proves functional interop with a + // real go-sdk v1.7 client instead of re-asserting that. + clientPath, err := ensureYardstickClientBinary() + Expect(err).ToNot(HaveOccurred(), "yardstick-client must be installable") + port := proxyPort(proxyURL) + + out, err := runYardstickClient(clientPath, + "-transport", "streamable-http", "-address", "127.0.0.1", "-port", port, + "-action", "call-tool", "-tool", "echo", "-args", `{"input":"modernfunctional"}`, + ) + Expect(err).ToNot(HaveOccurred(), "yardstick-client call-tool should succeed: %s", out) + Expect(out).To(ContainSubstring("modernfunctional")) + + // This backend's own transport is stdio (unaffected by any Stateless + // gating -- see the file doc comment), so yardstick-client's discover + // negotiates Modern here. info confirms it, not a silent downgrade. + infoOut, err := runYardstickClient(clientPath, + "-transport", "streamable-http", "-address", "127.0.0.1", "-port", port, "-action", "info", + ) + Expect(err).ToNot(HaveOccurred(), "yardstick-client info should succeed: %s", infoOut) + Expect(infoOut).To(ContainSubstring("Protocol Version: 2026-07-28")) + Expect(sessionIDFromInfo(infoOut)).To(BeEmpty(), "Modern must have no session id (inverse of the Legacy leg)") + }) + + It("reaches server/discover (no authz configured, so it is NOT 403-denied)", func() { + // The design plan's ground truth said server/discover "currently + // default-denies (403) through authz" -- verified at Step 2.1 that + // this only holds when authz middleware is actually wired, which + // only happens when RunConfig.AuthzConfig != nil + // (pkg/runner/middleware.go:210). A bare `thv run` like this + // BeforeEach never enables it, so server/discover reaches the + // backend and succeeds. See pkg/authz/middleware.go's comment on + // why server/discover isn't allow-listed for when authz IS on. + req, err := e2e.NewModernRequest("server/discover", nil) + Expect(err).ToNot(HaveOccurred()) + + resp, err := client.Send(context.Background(), proxyURL, req) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Error).To(BeNil()) + }) +}) + +// No second yardstick-client leg here: SDK negotiation (discover/initialize) +// is backend-side logic, identical regardless of which ToolHive proxy fronts +// it -- the streamable-proxy leg above already proves it end-to-end. This +// spec keeps the raw-client no-session-id check, which does exercise the +// transparent proxy's own (distinct) Modern code path. +var _ = Describe("Modern Functional Round-Trip — Transparent Proxy", Label("proxy", "stateless", "dual-era", "e2e"), Serial, func() { + var ( + config *e2e.TestConfig + serverName string + mockServer *statelessMockMCPServer + client *e2e.RawMCPClient + proxyURL string + ) + + BeforeEach(func() { + config = e2e.NewTestConfig() + serverName = e2e.GenerateUniqueServerName("func-transparent") + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + mockServer, err = newStatelessMockMCPServer() + Expect(err).ToNot(HaveOccurred()) + + client, err = e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + e2e.NewTHVCommand(config, "run", "--name", serverName, "--stateless", mockServer.URL()+"/mcp").ExpectSuccess() + + err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "server should be running within 60 seconds") + + proxyURL, err = e2e.GetMCPServerURL(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to get proxy URL") + if !strings.HasSuffix(proxyURL, "/mcp") { + proxyURL += "/mcp" + } + }) + + AfterEach(func() { + if mockServer != nil { + mockServer.Stop() + mockServer = nil + } + if config.CleanupAfter { + err := e2e.StopAndRemoveMCPServer(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to stop and remove server") + } + }) + + It("round-trips a Modern request with no Mcp-Session-Id", func() { + req, err := e2e.NewModernRequest("tools/list", nil) + Expect(err).ToNot(HaveOccurred()) + + resp, err := client.Send(context.Background(), proxyURL, req) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Error).To(BeNil()) + Expect(resp.Headers.Get(e2e.HeaderMCPSessionID)).To(BeEmpty(), "Modern must never carry Mcp-Session-Id") + }) +}) + +var _ = Describe("Legacy Functional Round-Trip — real SDK client (yardstick-client)", Label("proxy", "stateless", "dual-era", "e2e"), Serial, func() { + var ( + config *e2e.TestConfig + backendName string + ) + + BeforeEach(func() { + config = e2e.NewTestConfig() + backendName = e2e.GenerateUniqueServerName("func-legacy-backend") + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + // Deliberately NOT stateless: yardstick-server's own discover response + // then omits 2026-07-28 (StreamableHTTPOptions.Stateless gates it -- + // confirmed live), so yardstick-client's own SDK negotiation falls + // back to a real Legacy initialize handshake. --group "default" is + // the always-present built-in group; no group create/cleanup needed. + startYardstickOnPort(config, "default", backendName, allocateVMCPPort()) + }) + + AfterEach(func() { + if config.CleanupAfter { + err := e2e.StopAndRemoveMCPServer(config, backendName) + Expect(err).ToNot(HaveOccurred(), "should be able to stop and remove server") + } + }) + + It("completes a Legacy tool call end-to-end through the transparent proxy", func() { + clientPath, err := ensureYardstickClientBinary() + Expect(err).ToNot(HaveOccurred(), "yardstick-client must be installable") + + proxyURL, err := e2e.GetMCPServerURL(config, backendName) + Expect(err).ToNot(HaveOccurred()) + port := proxyPort(proxyURL) + + out, err := runYardstickClient(clientPath, + "-transport", "streamable-http", + "-address", "127.0.0.1", + "-port", port, + "-action", "call-tool", + "-tool", "echo", + "-args", `{"input":"legacyfunctional"}`, + ) + Expect(err).ToNot(HaveOccurred(), "yardstick-client call-tool should succeed: %s", out) + Expect(out).To(ContainSubstring("legacyfunctional")) + + // info action reports the negotiated protocol version and session id; + // confirms this really was the Legacy leg, not a silent Modern upgrade. + infoOut, err := runYardstickClient(clientPath, + "-transport", "streamable-http", "-address", "127.0.0.1", "-port", port, "-action", "info") + Expect(err).ToNot(HaveOccurred(), "yardstick-client info should succeed: %s", infoOut) + Expect(infoOut).To(ContainSubstring("Protocol Version: 2025-11-25")) + Expect(sessionIDFromInfo(infoOut)).ToNot(BeEmpty(), "Legacy must have a real session id") + }) +}) + +// server/discover is deliberately excluded from pkg/authz/middleware.go's +// MCPMethodToFeatureOperation map (see its comment at middleware.go:57-61): +// discover's response enumerates every tool/resource descriptor and would +// bypass ResponseFilteringWriter (which only filters tools/list, prompts/list, +// resources/list, and find_tool) -- a fail-safe deny, not an oversight. +// Consequence (known "for now" gap, #5830): a real Modern client (like +// yardstick-client) that hits this 403 has no JSON-RPC "unsupported version" +// data to parse, so its own fallback-on-non-modern-error path kicks in and it +// silently downgrades to a Legacy 2025-11-25 initialize handshake instead of +// surfacing the denial -- which is why this guard uses the raw client, not +// yardstick-client, to actually observe the 403. +var _ = Describe("server/discover Authorization Guard", Label("proxy", "stateless", "dual-era", "e2e", "authz"), Serial, func() { + var ( + config *e2e.TestConfig + serverName string + client *e2e.RawMCPClient + proxyURL string + ) + + BeforeEach(func() { + config = e2e.NewTestConfig() + serverName = e2e.GenerateUniqueServerName("func-discover-authz") + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + client, err = e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + // Minimal Cedar policy allowing tools/call+tools/list (so the backend + // is otherwise usable) -- mirrors osv_authz_test.go's pattern. + authzConfig := `{ + "version": "1.0", + "type": "cedarv1", + "cedar": { + "policies": [ + "permit(principal, action == Action::\"call_tool\", resource == Tool::\"echo\");", + "permit(principal, action == Action::\"list_tools\", resource);" + ], + "entities_json": "[]" + } +}` + tempDir, err := os.MkdirTemp("", "func-discover-authz") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { _ = os.RemoveAll(tempDir) }) + authzConfigPath := filepath.Join(tempDir, "authz-config.json") + Expect(os.WriteFile(authzConfigPath, []byte(authzConfig), 0600)).To(Succeed()) + + e2e.NewTHVCommand(config, "run", + "--name", serverName, + "--transport", "stdio", + "-e", "BACKEND_MODE=echo", + "--authz-config", authzConfigPath, + images.YardstickServerImage, + ).ExpectSuccess() + + err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "server should be running within 60 seconds") + + proxyURL, err = e2e.GetMCPServerURL(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to get proxy URL") + if !strings.HasSuffix(proxyURL, "/mcp") { + proxyURL += "/mcp" + } + }) + + AfterEach(func() { + if config.CleanupAfter { + err := e2e.StopAndRemoveMCPServer(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to stop and remove server") + } + }) + + It("denies server/discover with 403 when authz is enabled", func() { + req, err := e2e.NewModernRequest("server/discover", nil) + Expect(err).ToNot(HaveOccurred()) + + resp, err := client.Send(context.Background(), proxyURL, req) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(403)) + Expect(resp.Error).ToNot(BeNil()) + // The denial envelope is currently non-conformant JSON-RPC (capitalized + // Result/Error/ID, no "jsonrpc":"2.0" -- tracked in #5950); this parses + // only because encoding/json's unmarshal is case-insensitive. That's the + // envelope's job to fix, not this guard's -- here we're asserting authz + // *behavior* (denied, code 403), not wire conformance. + Expect(resp.Error.Code).To(Equal(int64(403))) + + By("confirming authz still allows the policy-permitted echo tool call") + callReq, err := e2e.NewModernRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "stillworks"}, + }) + Expect(err).ToNot(HaveOccurred()) + callResp, err := client.Send(context.Background(), proxyURL, callReq) + Expect(err).ToNot(HaveOccurred()) + Expect(callResp.StatusCode).To(Equal(200)) + Expect(callResp.Error).To(BeNil()) + }) +}) From a6051eed3d95b43d18b043fc21e18518530a8fb0 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 20:14:45 +0200 Subject: [PATCH 08/18] Add dual-era observability e2e test The existing telemetry e2e tests only log-grep for the word span; none assert span attribute values. This adds a real span-attribute assertion for the Modern (2026-07-28) path: a Modern request's server span must carry mcp.protocol.version=2026-07-28 and mcp.client.name (sourced from _meta.clientInfo, the only client-attribution source in Modern since there is no initialize handshake). Stands up an in-process OTLP/HTTP receiver (httptest.Server decoding real ExportTraceServiceRequest protobuf, race-free span store) and points thv run --otel-endpoint at it, fires one Modern tools/call carrying _meta.clientInfo, and Eventually asserts a span with both attributes (polling since the BatchSpanProcessor flushes on a timer). No new dependencies: the OTLP proto types and protobuf runtime were already in the module graph (promoted from indirect to direct). Co-Authored-By: Claude Opus 4.8 --- go.mod | 4 +- test/e2e/dual_era_observability_test.go | 183 ++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 test/e2e/dual_era_observability_test.go diff --git a/go.mod b/go.mod index 532c61d48f..77972f6694 100644 --- a/go.mod +++ b/go.mod @@ -284,7 +284,7 @@ require ( go.opentelemetry.io/otel/exporters/jaeger v1.17.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.43.0 // indirect go.opentelemetry.io/otel/exporters/zipkin v1.21.0 // indirect - go.opentelemetry.io/proto/otlp v1.10.0 // indirect + go.opentelemetry.io/proto/otlp v1.10.0 go.uber.org/atomic v1.11.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect @@ -299,7 +299,7 @@ require ( google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260523011958-0a33c5d7ca68 // indirect google.golang.org/grpc v1.82.1 // indirect - google.golang.org/protobuf v1.36.11 // indirect + google.golang.org/protobuf v1.36.11 gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect diff --git a/test/e2e/dual_era_observability_test.go b/test/e2e/dual_era_observability_test.go new file mode 100644 index 0000000000..6bbceb2be5 --- /dev/null +++ b/test/e2e/dual_era_observability_test.go @@ -0,0 +1,183 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e_test + +import ( + "bytes" + "compress/gzip" + "context" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + coltracepb "go.opentelemetry.io/proto/otlp/collector/trace/v1" + "google.golang.org/protobuf/proto" + + "github.com/stacklok/toolhive/test/e2e" + "github.com/stacklok/toolhive/test/e2e/images" +) + +// This spec is the observability tier of the e2e suite (issue #5837, Commit +// 2, Step 2.3): a real OTLP/HTTP collector, not a log-grep, receives actual +// spans from a live `thv run` process and asserts real attribute values -- +// telemetry_middleware_e2e_test.go only greps proxy log lines and never +// inspects a span. mcp.protocol.version and mcp.client.name are set in +// pkg/telemetry/middleware.go from the incoming MCP-Protocol-Version header +// and _meta.clientInfo respectively (Modern's only source of client +// attribution, since Modern has no initialize request -- see +// addMethodSpecificAttributes's doc comment). + +// otlpSpan is a minimal, test-only projection of a received OTLP span. +type otlpSpan struct { + Name string + Attributes map[string]string +} + +// otlpTraceCollector is an in-process OTLP/HTTP trace receiver: it decodes +// real ExportTraceServiceRequest protobuf (the wire format +// otlptracehttp actually sends -- no compression by default, see +// pkg/telemetry/providers/otlp/tracing.go) and records every span's name and +// string attributes for assertions. +type otlpTraceCollector struct { + server *httptest.Server + mu sync.Mutex + spans []otlpSpan +} + +func newOTLPTraceCollector() *otlpTraceCollector { + c := &otlpTraceCollector{} + c.server = httptest.NewServer(http.HandlerFunc(c.handle)) + return c +} + +func (c *otlpTraceCollector) handle(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + if r.Header.Get("Content-Encoding") == "gzip" { + gz, gzErr := gzip.NewReader(bytes.NewReader(body)) + if gzErr != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + defer gz.Close() + if body, err = io.ReadAll(gz); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + } + + var req coltracepb.ExportTraceServiceRequest + if err := proto.Unmarshal(body, &req); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + + c.mu.Lock() + defer c.mu.Unlock() + for _, rs := range req.GetResourceSpans() { + for _, ss := range rs.GetScopeSpans() { + for _, s := range ss.GetSpans() { + attrs := make(map[string]string, len(s.GetAttributes())) + for _, kv := range s.GetAttributes() { + attrs[kv.GetKey()] = kv.GetValue().GetStringValue() + } + c.spans = append(c.spans, otlpSpan{Name: s.GetName(), Attributes: attrs}) + } + } + } + w.WriteHeader(http.StatusOK) +} + +// Spans returns a snapshot of every span received so far. +func (c *otlpTraceCollector) Spans() []otlpSpan { + c.mu.Lock() + defer c.mu.Unlock() + return append([]otlpSpan(nil), c.spans...) +} + +func (c *otlpTraceCollector) URL() string { return c.server.URL } +func (c *otlpTraceCollector) Stop() { c.server.Close() } + +var _ = Describe("Dual-Era Observability — OTLP Spans", Label("proxy", "stateless", "dual-era", "e2e", "telemetry"), Serial, func() { + var ( + config *e2e.TestConfig + serverName string + client *e2e.RawMCPClient + proxyURL string + collector *otlpTraceCollector + ) + + BeforeEach(func() { + config = e2e.NewTestConfig() + serverName = e2e.GenerateUniqueServerName("func-otel") + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + collector = newOTLPTraceCollector() + + client, err = e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + e2e.NewTHVCommand(config, "run", + "--name", serverName, + "--transport", "stdio", + "-e", "BACKEND_MODE=echo", + "--otel-endpoint", collector.URL(), + "--otel-insecure", + "--otel-sampling-rate", "1.0", // always-sample: assertions must not race the default 10% ratio + images.YardstickServerImage, + ).ExpectSuccess() + + err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "server should be running within 60 seconds") + + proxyURL, err = e2e.GetMCPServerURL(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to get proxy URL") + if !strings.HasSuffix(proxyURL, "/mcp") { + proxyURL += "/mcp" + } + }) + + AfterEach(func() { + if collector != nil { + collector.Stop() + } + if config.CleanupAfter { + err := e2e.StopAndRemoveMCPServer(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to stop and remove server") + } + }) + + It("emits mcp.protocol.version and mcp.client.name on a Modern span", func() { + req, err := e2e.NewModernRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "otelcheck"}, + }) + Expect(err).ToNot(HaveOccurred()) + req.WithClientInfo("otel-e2e-client", "1.0") + + resp, err := client.Send(context.Background(), proxyURL, req) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Error).To(BeNil()) + + // The SDK's BatchSpanProcessor flushes on its own timer (default 5s), + // not on every span -- poll rather than assert immediately. + Eventually(func() []otlpSpan { + return collector.Spans() + }, 20*time.Second, 500*time.Millisecond).Should(ContainElement(SatisfyAll( + HaveField("Attributes", HaveKeyWithValue("mcp.protocol.version", e2e.MCPVersionModern)), + HaveField("Attributes", HaveKeyWithValue("mcp.client.name", "otel-e2e-client")), + )), "expected a span carrying both mcp.protocol.version and mcp.client.name") + }) +}) From 5e3a62cffbf75d26ea2cdcc1f0898893f79afd5e Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 20:35:59 +0200 Subject: [PATCH 09/18] Test Modern load leaves no accumulated state The stateless Modern (2026-07-28) path must not leak state under load (the unbounded-growth concern behind toolhive-core#169). This white-box integration test drives a burst of concurrent Modern requests through the streamable proxy's real dispatch lifecycle and asserts, after they all complete, that no per-request state is left behind: the sessionManager holds zero sessions (Modern registers none), the waiters and idRestore maps are empty (per-request routing tokens are cleaned up -- cleanup is deferred in doRequest and runs before any response byte, so the check is deterministic), and the goroutine count is flat. Concurrency is capped with a semaphore (well under the proxy's fixed message-channel buffer) to sidestep an unrelated backpressure gap (#5952) without weakening the invariant, which is about state left after N requests, not N being simultaneous. Mutation-verified: blanking the waiter cleanup makes it fail with the maps holding every request. Co-Authored-By: Claude Opus 4.8 --- .../streamable_proxy_integration_test.go | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/pkg/transport/proxy/streamable/streamable_proxy_integration_test.go b/pkg/transport/proxy/streamable/streamable_proxy_integration_test.go index 31a423dbf6..7514170274 100644 --- a/pkg/transport/proxy/streamable/streamable_proxy_integration_test.go +++ b/pkg/transport/proxy/streamable/streamable_proxy_integration_test.go @@ -11,6 +11,8 @@ import ( "io" "net" "net/http" + "runtime" + "sync" "testing" "time" @@ -144,3 +146,93 @@ func TestHTTPProxy_StartMountsAuthDiscoveryEndpoint(t *testing.T) { require.NoError(t, json.NewDecoder(resp.Body).Decode(&body)) assert.Equal(t, "https://example.com", body["resource"]) } + +// syncMapLen counts entries in a sync.Map for test assertions. +func syncMapLen(m *sync.Map) int { + n := 0 + m.Range(func(_, _ any) bool { n++; return true }) + return n +} + +// TestModernLoadDoesNotAccumulateState drives a burst of concurrent Modern +// (2026-07-28) requests through the full per-request lifecycle -- +// resolveSessionForRequest mints a routing token, doRequest registers a +// waiter/idRestore entry keyed on it and deletes both via its deferred +// cleanup once the response arrives -- and proves nothing is left behind. +// This is the regression test for the unbounded-growth concern behind +// toolhive-core#169. +// +// waiters/idRestore/sessionManager are asserted as hard, exact-zero checks: +// createWaiter's cleanup runs synchronously inside doRequest before +// handleSingleRequest ever writes the HTTP response, so by the time every +// client in the burst has read its response, every cleanup has already run -- +// no settle delay needed, and a regression that skips cleanup fails this +// deterministically. +// +// The goroutine count is checked too, but only as a generous, best-effort +// bound: this package's request path spawns no per-request goroutine, so any +// growth here would come from net/http's own idle-connection handling, not +// from code owned by this package -- hence the loose tolerance rather than a +// strict equality. +func TestModernLoadDoesNotAccumulateState(t *testing.T) { + port := pickFreePort(t) + proxy, ctx, cancel := startProxyWithBackend(t, port) + t.Cleanup(cancel) + t.Cleanup(func() { _ = proxy.Stop(ctx) }) + + url := fmt.Sprintf("http://127.0.0.1:%d%s", port, StreamableHTTPEndpoint) + + const requestCount = 1000 + // Cap in-flight requests well under the proxy's 100-slot message-channel + // buffer. Firing all 1000 at once can exceed that buffer and produce + // dropped-response 504s (#5952, a proxy backpressure gap) -- + // unrelated to the accumulation invariant this test checks, so throttle + // around it rather than let it flake the run. + const maxInFlight = 50 + sem := make(chan struct{}, maxInFlight) + baseline := runtime.NumGoroutine() + + var wg sync.WaitGroup + for i := 0; i < requestCount; i++ { + wg.Add(1) + sem <- struct{}{} + go func(id int) { + defer wg.Done() + defer func() { <-sem }() + + req, err := http.NewRequest( + http.MethodPost, url, bytes.NewReader([]byte(modernBody(id, "tools/list")))) + if !assert.NoError(t, err) { + return + } + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if !assert.NoError(t, err) { + return + } + defer func() { + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + }(i) + } + + done := make(chan struct{}) + go func() { wg.Wait(); close(done) }() + select { + case <-done: + case <-time.After(30 * time.Second): + t.Fatal("timeout waiting for the request burst to complete") + } + + assert.Zero(t, proxy.sessionManager.Count(), "Modern requests must never register a session") + assert.Zero(t, syncMapLen(&proxy.waiters), "waiters must be fully cleaned up once all responses are delivered") + assert.Zero(t, syncMapLen(&proxy.idRestore), "idRestore must be fully cleaned up once all responses are delivered") + + http.DefaultClient.CloseIdleConnections() + assert.Eventually(t, func() bool { + return runtime.NumGoroutine() <= baseline+20 + }, time.Second, 20*time.Millisecond, "goroutine count did not settle back near baseline after the burst") +} From 89713ca3a944d8e624df878248534b022599bc80 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 23 Jul 2026 20:38:53 +0200 Subject: [PATCH 10/18] Add dual-era resilience e2e tests Tier-3 resilience: backend faults must degrade gracefully. Uses yardstick's hang and crash fault modes behind the streamable proxy. The hang spec proves request isolation: with one request wedged at the backend (bounded by the proxy request timeout, returning 504), a second concurrent request still completes promptly -- one slow backend call does not block unrelated traffic -- and a fresh Legacy initialize + session-keyed call afterward proves the backend process and Legacy session semantics survive the hang. The crash spec proves fail-fast: a backend that exits mid-request surfaces a clean 5xx well before the recovery window, not a hang. Redis-unavailable-to-503 is deferred to the k8s tier, where Redis session storage is actually wired via the operator (no thv run CLI flag exposes it); full crash-recovery latency is out of scope (documented). Co-Authored-By: Claude Opus 4.8 --- test/e2e/dual_era_resilience_test.go | 247 +++++++++++++++++++++++++++ 1 file changed, 247 insertions(+) create mode 100644 test/e2e/dual_era_resilience_test.go diff --git a/test/e2e/dual_era_resilience_test.go b/test/e2e/dual_era_resilience_test.go new file mode 100644 index 0000000000..dd8fea2c3e --- /dev/null +++ b/test/e2e/dual_era_resilience_test.go @@ -0,0 +1,247 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e_test + +import ( + "context" + "fmt" + "net/http" + "strings" + "sync" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/stacklok/toolhive/test/e2e" + "github.com/stacklok/toolhive/test/e2e/images" +) + +// This spec is the fault-injection tier of the e2e suite (issue #5837, +// Commit 3, Step 3.1): the streamable proxy must degrade gracefully when its +// backend hangs or crashes mid-request -- a clean bounded error to the +// caller, no proxy-wide hang, and other traffic unaffected. +// +// Confirmed live before writing this (yardstick-server 1.2.0's +// BACKEND_MODE=hang/crash, via a real `thv run` instance): +// +// - Hang: HANG_AFTER_N counts requests; the Nth one never gets a backend +// response. The proxy's own TOOLHIVE_PROXY_REQUEST_TIMEOUT bounds it (a +// plain-text 504 "Timeout waiting for response from container" via +// writeHTTPError -- not a JSON-RPC body, so RawResponse.Error stays nil; +// assert on StatusCode instead). Concurrent unrelated requests are NOT +// blocked while the hung one waits (verified: a concurrent request +// completed in 14ms while the hung one was still waiting on its own +// goroutine, and the hung one's later 504 arrived on schedule). The +// backend process itself survives a hang -- no restart, a request sent +// afterward succeeds immediately. +// - Crash: the in-flight request at CRASH_AFTER_N gets the same clean +// writeHTTPError response (a 5xx -- observed 504, "server error" is the +// load-bearing property, not the exact code) well within a few seconds -- the +// stdio transport detects the closed pipe faster than any configured +// timeout, it doesn't wait for TOOLHIVE_PROXY_REQUEST_TIMEOUT to elapse. +// Recovery is real (`pkg/workloads/manager.go`'s RunWorkload retries +// with backoff) but SLOW and layered in this environment: Docker's own +// restart policy fires first (~13s), races a doomed thv re-attach +// (409 conflict, observed), then thv's own workload-level restart +// kicks in only after re-attach gives up (~48s total in one measured +// run) -- and the proxy port is unreachable for the whole window. That +// is far outside what a per-spec e2e timeout should eat, and getting it +// reliably non-flaky would mean re-resolving the proxy URL and polling +// through repeated container recreation, which is disproportionate to +// this step. Per the plan's explicit either/or, this spec asserts only +// the clean-error window for crash, not full recovery -- documented +// here as a deliberate scope decision, not an oversight. +// +// Redis-unavailable -> 503 (case 3) is OUT OF SCOPE for this file: `thv run` +// has NO CLI flag exposing Redis-backed session storage at all -- +// RunConfig.ScalingConfig.SessionRedis (pkg/runner/config.go) is set only via +// a hand-built RunConfig (e.g. `--from-config`) or the Kubernetes operator; +// there is no `thv run --session-storage redis` equivalent to stand up in a +// local e2e test. Standing this up would mean hand-authoring a full +// RunConfig JSON with no existing local-e2e precedent (the one Redis +// fixture in this suite, vmcp_infra_features_test.go, wires it into `thv +// vmcp serve`, a different proxy entirely). Flagging per the plan's +// "STOP and report" clause rather than building something disproportionate. +var _ = Describe("Dual-Era Resilience — Fault Injection", Label("proxy", "stateless", "dual-era", "e2e"), Serial, func() { + var ( + config *e2e.TestConfig + serverName string + client *e2e.RawMCPClient + proxyURL string + ) + + AfterEach(func() { + if config == nil || !config.CleanupAfter { + return + } + // The crash spec's backend may still be mid Docker-restart / thv + // re-attach (see the file doc comment -- ~48s, one measured run) when + // this fires; retry rather than hard-failing the spec on a transient + // "container is restarting" error masking the real test result. + Eventually(func() error { + return e2e.StopAndRemoveMCPServer(config, serverName) + }, 60*time.Second, 2*time.Second).Should(Succeed(), "should be able to stop and remove server") + }) + + startFaultBackend := func(mode string, extraEnv ...string) { + config = e2e.NewTestConfig() + serverName = e2e.GenerateUniqueServerName("resilience-" + mode) + + err := e2e.CheckTHVBinaryAvailable(config) + Expect(err).ToNot(HaveOccurred(), "thv binary should be available") + + client, err = e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + args := []string{ + "run", + "--name", serverName, + "--transport", "stdio", + "-e", "BACKEND_MODE=" + mode, + } + for _, e := range extraEnv { + args = append(args, "-e", e) + } + args = append(args, images.YardstickServerImage) + + // Bound the request timeout so a hang fails fast in the test, not + // after the streamable proxy's 60s default. + e2e.NewTHVCommand(config, args...).WithEnv("TOOLHIVE_PROXY_REQUEST_TIMEOUT=5s").ExpectSuccess() + + err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "server should be running within 60 seconds") + + proxyURL, err = e2e.GetMCPServerURL(config, serverName) + Expect(err).ToNot(HaveOccurred(), "should be able to get proxy URL") + if !strings.HasSuffix(proxyURL, "/mcp") { + proxyURL += "/mcp" + } + } + + echoRequest := func(input string) *e2e.RawRequest { + req, err := e2e.NewModernRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": input}, + }) + Expect(err).ToNot(HaveOccurred()) + return req + } + + Context("backend hang", func() { + BeforeEach(func() { + // The 2nd request hangs; the 1st is a healthy baseline. + startFaultBackend("hang", "HANG_AFTER_N=2") + }) + + It("isolates a hung request from concurrent traffic, and the backend stays usable", func() { + By("a baseline request succeeds") + resp, err := client.Send(context.Background(), proxyURL, echoRequest("baseline")) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + + By("firing the hang-triggering request concurrently with an unrelated one") + var wg sync.WaitGroup + var hungErr, normalErr error + var hungStatus, normalStatus int + var hungElapsed, normalElapsed time.Duration + + // Deterministic ordering: launch the hang-trigger first and give + // it a head start to actually reach the backend as request #2, + // THEN launch "normal" -- racing both from a shared start gate + // left which request HANG_AFTER_N=2 counted as a coin flip (MoE + // flake finding). "normal" still overlaps the hang in time (fired + // while the hang-trigger is still in flight), which is exactly + // the isolation property under test. + wg.Add(1) + go func() { + defer wg.Done() + t0 := time.Now() + r, sendErr := client.Send(context.Background(), proxyURL, echoRequest("hangtrigger")) + hungElapsed = time.Since(t0) + hungErr = sendErr + if r != nil { + hungStatus = r.StatusCode + } + }() + time.Sleep(300 * time.Millisecond) + + wg.Add(1) + go func() { + defer wg.Done() + t0 := time.Now() + r, sendErr := client.Send(context.Background(), proxyURL, echoRequest("normal")) + normalElapsed = time.Since(t0) + normalErr = sendErr + if r != nil { + normalStatus = r.StatusCode + } + }() + + done := make(chan struct{}) + go func() { wg.Wait(); close(done) }() + select { + case <-done: + case <-time.After(20 * time.Second): + Fail("timed out waiting for concurrent hang/normal requests to finish") + } + + Expect(hungErr).ToNot(HaveOccurred()) + Expect(normalErr).ToNot(HaveOccurred()) + Expect(normalStatus).To(Equal(http.StatusOK), "the unrelated concurrent request must succeed") + Expect(normalElapsed).To(BeNumerically("<", 5*time.Second), + "the unrelated request must not be blocked behind the hung one") + Expect(hungStatus).To(Equal(http.StatusGatewayTimeout), "the hung request must get a clean bounded error") + Expect(hungElapsed).To(BeNumerically(">=", 4*time.Second), + "sanity: the hang must actually have been waited out by the proxy timeout, not failed for some other reason") + + By("Legacy recovers: a fresh initialize + call after the hang still works") + initReq := e2e.NewLegacyInitializeRequest("resilience-client", "1.0") + initResp, err := client.Send(context.Background(), proxyURL, initReq) + Expect(err).ToNot(HaveOccurred()) + Expect(initResp.StatusCode).To(Equal(http.StatusOK)) + sessionID := initResp.Headers.Get(e2e.HeaderMCPSessionID) + Expect(sessionID).ToNot(BeEmpty()) + + legacyReq, err := e2e.NewLegacyRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "postH"}, + }) + Expect(err).ToNot(HaveOccurred()) + legacyReq.WithSessionID(sessionID) + legacyResp, err := client.Send(context.Background(), proxyURL, legacyReq) + Expect(err).ToNot(HaveOccurred()) + Expect(legacyResp.StatusCode).To(Equal(http.StatusOK)) + Expect(legacyResp.Error).To(BeNil()) + }) + }) + + Context("backend crash", func() { + BeforeEach(func() { + // The 2nd request crashes the backend; the 1st is a healthy baseline. + startFaultBackend("crash", "CRASH_AFTER_N=2") + }) + + It("returns a clean error for the crashing request instead of hanging or panicking the proxy", func() { + By("a baseline request succeeds") + resp, err := client.Send(context.Background(), proxyURL, echoRequest("baseline")) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + + By("the crash-triggering request gets a clean, bounded error") + t0 := time.Now() + crashResp, err := client.Send(context.Background(), proxyURL, echoRequest("crashtrigger")) + elapsed := time.Since(t0) + Expect(err).ToNot(HaveOccurred(), "the client must get an HTTP response, not a connection error/hang") + Expect(crashResp.StatusCode).To(BeNumerically(">=", 500), "a crash must surface as a server error, not a silent 200") + // 10s: comfortably above the observed ~3-5s crash-detection, far below + // the ~48s full-recovery latency -- tight enough to actually fail if + // crash-handling regressed to riding out recovery instead of detecting + // the closed pipe. (The 15s client.Send timeout would fire first and + // fail the Expect above, so bounding by that alone is a tautology.) + Expect(elapsed).To(BeNumerically("<", 10*time.Second), + fmt.Sprintf("must fail fast (took %s) -- a crash must not ride out recovery", elapsed)) + }) + }) +}) From 565885d09c0e505d4d4211b02db5726025024fd2 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 24 Jul 2026 09:37:52 +0200 Subject: [PATCH 11/18] Add dual-era k8s multi-replica e2e tests The operator-deployed dual-era proxy over a shared session store is the production shape the design targets, so it needs its own tier. Deploys a multi-replica yardstick 1.2.0 MCPServer (Replicas + BackendReplicas, streamable-http, STATELESS/BACKEND_MODE echo) with Redis SessionStorage and SessionAffinity: None, in a dedicated namespace so its destructive Redis ops don't collide with sibling suites under parallel Ginkgo. Asserts: the proxy Service actually carries SessionAffinity: None (a regression to the ClientIP default is caught by a direct field read) and sessionless Modern requests succeed across the multi-replica deployment; mixed Legacy+Modern over the shared Redis stays isolated; a backend pod eviction heals back to full replicas with Modern traffic continuing; Redis-unavailable yields 503 (storage error, not 404), and after restore a fresh session works while the pre-outage session correctly 404s. The distinct-backend-pod distribution proof was intentionally NOT included: kube-proxy is L4/per-connection and both proxy hops pool connections, so pod distribution is not observable or controllable from the test -- a count could read 1 under perfectly correct behavior. The in-file comment documents this so it isn't reintroduced as a flaky/unsound check. Authored to run in test-e2e-lifecycle.yml. Co-Authored-By: Claude Opus 4.8 --- .../acceptance_tests/dual_era_k8s_test.go | 427 ++++++++++++++++++ 1 file changed, 427 insertions(+) create mode 100644 test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go diff --git a/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go b/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go new file mode 100644 index 0000000000..0749174e7b --- /dev/null +++ b/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go @@ -0,0 +1,427 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package acceptancetests + +import ( + "context" + "fmt" + "net/http" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" + ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" + "github.com/stacklok/toolhive/test/e2e" + "github.com/stacklok/toolhive/test/e2e/images" + "github.com/stacklok/toolhive/test/e2e/thv-operator/testutil" +) + +// This is Commit 4, Steps 4.1+4.2 of the dual-era e2e plan (issue #5837). +// AUTHORED FOR CI (test-e2e-lifecycle.yml) -- NOT run locally. Per the +// Step 4.0 discovery, this worktree's environment only has a shared, +// long-lived kind cluster (unrelated ngrok-operator/xaa-phase1 namespaces, +// an operator build that predates this merge) that must not be touched, and +// it lacks the NodePort host-port range this harness needs anyway. Step 4.1's +// core (BeforeAll + the pods-ready/Redis-wired It) WAS verified live against +// that cluster in an isolated, self-cleaning throwaway namespace before this +// rewrite -- see the git history of this file / the Step 4.1 report. Every +// addition since (NodePort access, traffic-driving, pod eviction, Redis +// kill) is new and verified only by go build/vet/lint, not a live run. +// +// Runs in its own namespace (not "default"): this file scales the shared +// "redis" Deployment to 0 to test the 503 path, which would otherwise pull +// Redis out from under the concurrent ratelimit_test.go Ordered block under +// --procs=8. +// +// Confirmed live (Step 4.1): the MCPServer controller creates TWO separate +// workloads -- a Deployment named after the CR (the proxy runner, one +// "toolhive" container, selected by app.kubernetes.io/name=mcpserver + +// app.kubernetes.io/instance=, sized by spec.replicas) and a +// StatefulSet with the same name (the actual MCP server backend, one "mcp" +// container running the image, selected by app=, sized by +// spec.backendReplicas). Both reached Ready with spec.sessionAffinity: None +// and Redis-backed spec.sessionStorage; the CR's Ready condition surfaces +// session storage as a (non-)warning: type SessionStorageWarning, status +// False, reason SessionStorageConfigured once Redis is wired correctly. +// +// [MoE finding, resolved] The original design tried to prove "no pod-pinning" +// by tagging each Modern request with a unique _meta.nonce and grepping +// which backend pod's log (yardstick's echoHandler does +// `log.Printf("echo tool called with metadata: %+v", req.Params.Meta)`, +// cmd/yardstick-server/main.go:69 -- confirmed from source, not a run) +// contained it, then asserting >=2 distinct pods appear. That assertion is +// UNSOUND and was dropped: kube-proxy balances per TCP CONNECTION, not per +// HTTP request, and BOTH hops (raw-client-to-proxy AND the proxy's own +// outbound connection to the single backend ClusterIP) reuse keep-alive +// connections outside this test's control. With only 2 backend replicas, +// "all N requests land in one pod's log" is an ordinary, EXPECTED outcome +// of connection reuse under perfectly correct (non-pinned) behavior -- the +// test's signal cannot distinguish "working as intended" from "pinned", +// which is the definition of an unsound assertion, not a real check. There +// is no viable alternative signal either: the proxy itself logs nothing +// per-request that's nonce-correlatable (only structured slog.Debug lines +// with no request-identifying payload), so attributing to PROXY pods +// (rather than backend pods) has the same problem from the other direction. +// +// What replaced it, per the plan's own pre-approved fallback: the +// deterministic, sound properties. (1) The k8s Service actually has +// SessionAffinity: None set (a direct field read of the live object, not an +// inference from traffic) -- this is what spec.sessionAffinity actually +// configures, and is what would silently regress to ClientIP if the CRD +// wiring broke. (2) Many independent Modern requests all succeed against +// the multi-replica deployment (functional correctness under load, still a +// real check, just not a distinct-pod count). +// +// [MoE-relevant] Separately NOT asserting the literal wording "a Modern +// request carrying a stale/foreign Mcp-Session-Id is still served, not +// 404'd": that contradicts the security-hardened, MoE-reviewed behavior +// established in Step 1.5/the hostile-input suite +// (revision_guard_regression_test.go's +// TestGuardUnknownSessionFiresDespiteForgedModernRevision) -- the session +// guard deliberately fires on Mcp-Session-Id header PRESENCE regardless of +// classified revision, precisely so a forged Modern signal can't bypass +// session validation. A foreign session id on Modern getting 404 there is +// correct, not a bug. +var _ = Describe("Dual-Era Multi-Replica Backend", Ordered, func() { + const ( + testNamespace = "dual-era-k8s" + serverName = "dual-era-echo" + backendReplicas = 2 + timeout = 3 * time.Minute + pollingInterval = 2 * time.Second + ) + + var nodePort int32 + + backendPodLabels := map[string]string{"app": serverName} + + proxyURL := func() string { + return fmt.Sprintf("http://localhost:%d/mcp", nodePort) + } + + // backendPods lists the StatefulSet-backed backend pods for this MCPServer. + backendPods := func() []corev1.Pod { + podList := &corev1.PodList{} + Expect(k8sClient.List(ctx, podList, client.InNamespace(testNamespace), client.MatchingLabels(backendPodLabels))).To(Succeed()) + return podList.Items + } + + BeforeAll(func() { + By("Creating a dedicated namespace") + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testNamespace}} + Expect(client.IgnoreAlreadyExists(k8sClient.Create(ctx, ns))).To(Succeed()) + + By("Deploying Redis for session storage") + EnsureRedis(ctx, k8sClient, testNamespace, timeout, pollingInterval) + + By("Creating a multi-replica MCPServer (proxy + backend) with Redis session storage") + server := &mcpv1beta1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: serverName, + Namespace: testNamespace, + }, + Spec: mcpv1beta1.MCPServerSpec{ + Image: images.YardstickServerImage, + Transport: "streamable-http", + ProxyPort: 8080, + MCPPort: 8080, + Replicas: ptr.To(int32(2)), + BackendReplicas: ptr.To(int32(backendReplicas)), + SessionAffinity: "None", + Env: []mcpv1beta1.EnvVar{ + {Name: "TRANSPORT", Value: "streamable-http"}, + {Name: "STATELESS", Value: "true"}, // Modern (2026-07-28) capable backend + {Name: "BACKEND_MODE", Value: "echo"}, + }, + SessionStorage: &mcpv1beta1.SessionStorageConfig{ + Provider: "redis", + Address: "redis." + testNamespace + ".svc.cluster.local:6379", + }, + }, + } + Expect(k8sClient.Create(ctx, server)).To(Succeed()) + + // Ping-free readiness: the operator's Deployment readiness probe is a + // plain HTTP GET /health (confirmed live in Step 4.0/4.1, reading the + // Deployment spec), never an MCP "ping" (which Modern removed). This + // comment describes the OPERATOR's own probe; the test's own /health + // gate below is separate, added because kube-proxy route programming + // can lag pod-Ready. + By("Waiting for MCPServer to be running") + testutil.WaitForMCPServerRunning(ctx, k8sClient, serverName, testNamespace, timeout, pollingInterval) + + By("Creating NodePort service for MCPServer proxy") + testutil.CreateNodePortService(ctx, k8sClient, serverName, testNamespace) + nodePort = testutil.GetNodePort(ctx, k8sClient, serverName+"-nodeport", testNamespace, timeout, pollingInterval) + + By("Waiting for the proxy to actually be reachable on the NodePort") + httpClient := &http.Client{Timeout: 5 * time.Second} + Eventually(func() error { + resp, err := httpClient.Get(fmt.Sprintf("http://localhost:%d/health", nodePort)) + if err != nil { + return err + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("health check returned %d", resp.StatusCode) + } + return nil + }, timeout, pollingInterval).Should(Succeed()) + }) + + AfterAll(func() { + By("Deleting the dedicated namespace (cascades: MCPServer, Redis, NodePort service)") + _ = k8sClient.Delete(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testNamespace}}) + }) + + It("brings up both proxy and backend replicas ready, with Redis wired", func() { + By("proxy runner pods are ready") + Eventually(func() error { + return testutil.CheckPodsReady(ctx, k8sClient, testNamespace, map[string]string{ + "app.kubernetes.io/name": "mcpserver", + "app.kubernetes.io/instance": serverName, + }) + }, timeout, pollingInterval).Should(Succeed()) + + By("backend (yardstick) pods are ready") + Eventually(func() error { + return testutil.CheckPodsReady(ctx, k8sClient, testNamespace, backendPodLabels) + }, timeout, pollingInterval).Should(Succeed()) + + By("Redis session storage is wired without a warning") + server := &mcpv1beta1.MCPServer{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: serverName, Namespace: testNamespace}, server)).To(Succeed()) + cond := meta.FindStatusCondition(server.Status.Conditions, "SessionStorageWarning") + Expect(cond).ToNot(BeNil(), "expected a SessionStorageWarning condition once sessionStorage is set") + Expect(cond.Status).To(Equal(metav1.ConditionFalse), "Redis session storage should be configured cleanly: %s", cond.Message) + Expect(server.Status.ReadyReplicas).To(BeEquivalentTo(2)) + }) + + It("configures SessionAffinity:None and serves many Modern requests across the multi-replica deployment", func() { + By("the proxy Service actually has SessionAffinity: None (not the ClientIP default)") + svc := &corev1.Service{} + Expect(k8sClient.Get(ctx, client.ObjectKey{ + Name: ctrlutil.CreateProxyServiceName(serverName), + Namespace: testNamespace, + }, svc)).To(Succeed()) + Expect(svc.Spec.SessionAffinity).To(Equal(corev1.ServiceAffinityNone)) + + By("many independent sessionless Modern requests all succeed") + rawClient, err := e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + for i := range 8 { + req, err := e2e.NewModernRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "anypod"}, + }) + Expect(err).ToNot(HaveOccurred()) + resp, err := rawClient.Send(context.Background(), proxyURL(), req) + Expect(err).ToNot(HaveOccurred(), "request %d", i) + Expect(resp.StatusCode).To(Equal(http.StatusOK), "request %d", i) + Expect(resp.Error).To(BeNil(), "request %d", i) + Expect(resp.Headers.Get(e2e.HeaderMCPSessionID)).To(BeEmpty(), "Modern must never carry Mcp-Session-Id") + } + }) + + It("serves mixed Legacy and Modern traffic over the shared Redis session store", func() { + rawClient, err := e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + By("Legacy: initialize then a session-keyed tools/call") + initReq := e2e.NewLegacyInitializeRequest("k8s-mixed-legacy", "1.0") + initResp, err := rawClient.Send(context.Background(), proxyURL(), initReq) + Expect(err).ToNot(HaveOccurred()) + Expect(initResp.StatusCode).To(Equal(http.StatusOK)) + sessionID := initResp.Headers.Get(e2e.HeaderMCPSessionID) + Expect(sessionID).ToNot(BeEmpty()) + + legacyReq, err := e2e.NewLegacyRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "legacymixed"}, + }) + Expect(err).ToNot(HaveOccurred()) + legacyReq.WithSessionID(sessionID) + legacyResp, err := rawClient.Send(context.Background(), proxyURL(), legacyReq) + Expect(err).ToNot(HaveOccurred()) + Expect(legacyResp.StatusCode).To(Equal(http.StatusOK)) + Expect(legacyResp.Error).To(BeNil()) + + By("Modern: stateless tools/call, no session id, alongside the Legacy session above") + modernReq, err := e2e.NewModernRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "modernmixed"}, + }) + Expect(err).ToNot(HaveOccurred()) + modernResp, err := rawClient.Send(context.Background(), proxyURL(), modernReq) + Expect(err).ToNot(HaveOccurred()) + Expect(modernResp.StatusCode).To(Equal(http.StatusOK)) + Expect(modernResp.Error).To(BeNil()) + Expect(modernResp.Headers.Get(e2e.HeaderMCPSessionID)).To(BeEmpty(), "Modern must never carry Mcp-Session-Id") + + By("the Legacy session is still usable after the Modern request") + legacyReq2, err := e2e.NewLegacyRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "legacymixed2"}, + }) + Expect(err).ToNot(HaveOccurred()) + legacyReq2.WithSessionID(sessionID) + legacyResp2, err := rawClient.Send(context.Background(), proxyURL(), legacyReq2) + Expect(err).ToNot(HaveOccurred()) + Expect(legacyResp2.StatusCode).To(Equal(http.StatusOK)) + }) + + It("degrades cleanly and self-heals when a backend pod is evicted mid-traffic", func() { + rawClient, err := e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + before := backendPods() + Expect(before).To(HaveLen(backendReplicas), "precondition: all backend replicas present before evicting") + victim := before[0].Name + + By(fmt.Sprintf("deleting backend pod %s", victim)) + Expect(k8sClient.Delete(ctx, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: victim, Namespace: testNamespace}, + })).To(Succeed()) + + By("a fresh Modern request still succeeds -- served by a surviving or replacement pod") + Eventually(func() (int, error) { + req, buildErr := e2e.NewModernRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "aftereviction"}, + }) + if buildErr != nil { + return 0, buildErr + } + resp, sendErr := rawClient.Send(context.Background(), proxyURL(), req) + if sendErr != nil { + return 0, sendErr + } + return resp.StatusCode, nil + }, timeout, pollingInterval).Should(Equal(http.StatusOK)) + + // CheckPodsReady only requires ONE ready pod, so it would pass the + // instant the surviving pod is observed -- it never proves the + // StatefulSet actually replaced the evicted one. Assert the full + // count is back AND all of them are Ready. + By("the StatefulSet replaces the evicted pod -- full replica count, all Ready") + Eventually(func() (int, error) { + pods := backendPods() + ready := 0 + for _, p := range pods { + for _, c := range p.Status.Conditions { + if c.Type == corev1.PodReady && c.Status == corev1.ConditionTrue { + ready++ + } + } + } + if len(pods) != backendReplicas { + return 0, fmt.Errorf("expected %d backend pods, got %d", backendReplicas, len(pods)) + } + return ready, nil + }, timeout, pollingInterval).Should(Equal(backendReplicas)) + }) + + It("returns 503 when Redis becomes unavailable, then recovers once Redis is restored", func() { + rawClient, err := e2e.NewRawMCPClient(15 * time.Second) + Expect(err).ToNot(HaveOccurred()) + + By("establishing a Legacy session while Redis is up") + initReq := e2e.NewLegacyInitializeRequest("k8s-redis-503", "1.0") + initResp, err := rawClient.Send(context.Background(), proxyURL(), initReq) + Expect(err).ToNot(HaveOccurred()) + Expect(initResp.StatusCode).To(Equal(http.StatusOK)) + sessionID := initResp.Headers.Get(e2e.HeaderMCPSessionID) + Expect(sessionID).ToNot(BeEmpty()) + + By("scaling Redis to 0") + Expect(scaleRedis(ctx, testNamespace, 0)).To(Succeed()) + Eventually(func() (int, error) { + podList := &corev1.PodList{} + if err := k8sClient.List(ctx, podList, client.InNamespace(testNamespace), client.MatchingLabels{"app": "redis"}); err != nil { + return 0, err + } + return len(podList.Items), nil + }, timeout, pollingInterval).Should(Equal(0)) + + By("a request against the existing session now gets 503 (session store unavailable)") + req, err := e2e.NewLegacyRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "redisdown"}, + }) + Expect(err).ToNot(HaveOccurred()) + req.WithSessionID(sessionID) + Eventually(func() (int, error) { + resp, sendErr := rawClient.Send(context.Background(), proxyURL(), req) + if sendErr != nil { + return 0, sendErr + } + return resp.StatusCode, nil + }, timeout, pollingInterval).Should(Equal(http.StatusServiceUnavailable)) + + By("restoring Redis") + Expect(scaleRedis(ctx, testNamespace, 1)).To(Succeed()) + Eventually(func() error { + return testutil.CheckPodsReady(ctx, k8sClient, testNamespace, map[string]string{"app": "redis"}) + }, timeout, pollingInterval).Should(Succeed()) + + // Redis has no persistence here (a plain in-memory Deployment, see + // EnsureRedis) -- scaling to 0 wiped every session key, so the + // pre-outage sessionID is gone, not just temporarily unreachable. + // Reusing it now would correctly 404 (session-not-found), not 200: + // that would assert session *survival* across an outage that + // destroys state, which isn't the property here. The property is + // functional recovery -- a brand new session must work. + By("Redis is functionally back: a fresh initialize + call succeeds") + var newSessionID string + Eventually(func() (int, error) { + resp, sendErr := rawClient.Send(context.Background(), proxyURL(), e2e.NewLegacyInitializeRequest("k8s-redis-503-post", "1.0")) + if sendErr != nil { + return 0, sendErr + } + newSessionID = resp.Headers.Get(e2e.HeaderMCPSessionID) + return resp.StatusCode, nil + }, timeout, pollingInterval).Should(Equal(http.StatusOK)) + Expect(newSessionID).ToNot(BeEmpty()) + + req2, err := e2e.NewLegacyRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "redisback"}, + }) + Expect(err).ToNot(HaveOccurred()) + req2.WithSessionID(newSessionID) + resp2, err := rawClient.Send(context.Background(), proxyURL(), req2) + Expect(err).ToNot(HaveOccurred()) + Expect(resp2.StatusCode).To(Equal(http.StatusOK)) + + By("the pre-outage session is gone, not silently revived -- confirms the outage really cleared state") + staleReq, err := e2e.NewLegacyRequest("tools/call", map[string]any{ + "name": "echo", + "arguments": map[string]any{"input": "stalesession"}, + }) + Expect(err).ToNot(HaveOccurred()) + staleReq.WithSessionID(sessionID) + staleResp, err := rawClient.Send(context.Background(), proxyURL(), staleReq) + Expect(err).ToNot(HaveOccurred()) + Expect(staleResp.StatusCode).To(Equal(http.StatusNotFound)) + }) +}) + +// scaleRedis patches the "redis" Deployment's replica count (0 to simulate +// an outage, 1 to restore) -- the same Deployment EnsureRedis creates. +func scaleRedis(ctx context.Context, namespace string, replicas int32) error { + deploy := &appsv1.Deployment{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: "redis", Namespace: namespace}, deploy); err != nil { + return err + } + deploy.Spec.Replicas = ptr.To(replicas) + return k8sClient.Update(ctx, deploy) +} From 8b2971375b71f2c36e9dfc5a64eb48ef22714652 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 24 Jul 2026 19:30:56 +0200 Subject: [PATCH 12/18] Adapt server/discover authz test to #5953 Rebasing onto origin/main picked up #5953 (Serve MCP 2026-07-28 Modern stateless requests through vMCP), which intentionally flipped server/discover in pkg/authz/middleware.go from not-allow-listed (default-deny, 403) to always-allowed. The dual-era authz guard, which asserted a 403, correctly failed against the new behavior. Update it to the new contract: server/discover is allowed under authz (200), which is a positive result -- a real Modern client's discover no longer gets 403'd into a Legacy downgrade, so Modern-through-authz works. The policy-permitted echo call is still asserted. Per #5953's own comment, discover is not covered by the response-filter and passes through unfiltered; that is a documented, deliberate tradeoff in that PR, not something this test enforces. Co-Authored-By: Claude Opus 4.8 --- test/e2e/dual_era_functional_test.go | 86 ++++++++++++++++++---------- 1 file changed, 57 insertions(+), 29 deletions(-) diff --git a/test/e2e/dual_era_functional_test.go b/test/e2e/dual_era_functional_test.go index 58dc6440d1..36b0362b2b 100644 --- a/test/e2e/dual_era_functional_test.go +++ b/test/e2e/dual_era_functional_test.go @@ -183,15 +183,16 @@ var _ = Describe("Modern Functional Round-Trip — Streamable Proxy", Label("pro Expect(sessionIDFromInfo(infoOut)).To(BeEmpty(), "Modern must have no session id (inverse of the Legacy leg)") }) - It("reaches server/discover (no authz configured, so it is NOT 403-denied)", func() { - // The design plan's ground truth said server/discover "currently - // default-denies (403) through authz" -- verified at Step 2.1 that - // this only holds when authz middleware is actually wired, which - // only happens when RunConfig.AuthzConfig != nil - // (pkg/runner/middleware.go:210). A bare `thv run` like this - // BeforeEach never enables it, so server/discover reaches the - // backend and succeeds. See pkg/authz/middleware.go's comment on - // why server/discover isn't allow-listed for when authz IS on. + It("reaches server/discover (baseline: no authz middleware wired at all)", func() { + // As of #5953, server/discover is always-allowed in + // pkg/authz/middleware.go's MCPMethodToFeatureOperation map, so this + // no-authz baseline and the "under authz" case in the + // "server/discover Authorization Guard" Describe below now assert + // the same 200 outcome -- kept as two separate specs because they + // exercise different code paths: this one never wires the authz + // middleware at all (RunConfig.AuthzConfig == nil, + // pkg/runner/middleware.go:210), the other wires it and relies on + // discover's always-allowed entry in that map. req, err := e2e.NewModernRequest("server/discover", nil) Expect(err).ToNot(HaveOccurred()) @@ -321,17 +322,24 @@ var _ = Describe("Legacy Functional Round-Trip — real SDK client (yardstick-cl }) }) -// server/discover is deliberately excluded from pkg/authz/middleware.go's -// MCPMethodToFeatureOperation map (see its comment at middleware.go:57-61): -// discover's response enumerates every tool/resource descriptor and would -// bypass ResponseFilteringWriter (which only filters tools/list, prompts/list, -// resources/list, and find_tool) -- a fail-safe deny, not an oversight. -// Consequence (known "for now" gap, #5830): a real Modern client (like -// yardstick-client) that hits this 403 has no JSON-RPC "unsupported version" -// data to parse, so its own fallback-on-non-modern-error path kicks in and it -// silently downgrades to a Legacy 2025-11-25 initialize handshake instead of -// surfacing the denial -- which is why this guard uses the raw client, not -// yardstick-client, to actually observe the 403. +// server/discover used to be excluded from pkg/authz/middleware.go's +// MCPMethodToFeatureOperation map (default-deny, 403) to keep its +// unfiltered tool/resource descriptors away from ResponseFilteringWriter +// (which only filters tools/list, prompts/list, resources/list, and +// find_tool). #5953 ("Serve MCP 2026-07-28 Modern stateless requests through +// vMCP") flipped this to always-allowed ({Feature:"", Operation:""} -- +// initialize parity, same rationale as "initialize" itself already being +// always-allowed): discover carries the same Capabilities/Instructions shape +// initialize does, so it adds no new exposure class, and #5953's own +// middleware comment notes discover would "just pass through unfiltered" +// under the response-filter either way -- a deliberate, documented tradeoff, +// not an oversight. +// +// Consequence for dual-era: Modern-through-authz no longer downgrades. +// Previously a real Modern client's discover got 403'd under authz, so its +// fallback-on-non-modern-error path kicked in and it silently negotiated +// down to a Legacy 2025-11-25 initialize handshake. Now discover succeeds, +// so the client stays Modern -- this guard now positively proves that. var _ = Describe("server/discover Authorization Guard", Label("proxy", "stateless", "dual-era", "e2e", "authz"), Serial, func() { var ( config *e2e.TestConfig @@ -394,20 +402,14 @@ var _ = Describe("server/discover Authorization Guard", Label("proxy", "stateles } }) - It("denies server/discover with 403 when authz is enabled", func() { + It("server/discover is allowed under authz (Modern's initialize replacement)", func() { req, err := e2e.NewModernRequest("server/discover", nil) Expect(err).ToNot(HaveOccurred()) resp, err := client.Send(context.Background(), proxyURL, req) Expect(err).ToNot(HaveOccurred()) - Expect(resp.StatusCode).To(Equal(403)) - Expect(resp.Error).ToNot(BeNil()) - // The denial envelope is currently non-conformant JSON-RPC (capitalized - // Result/Error/ID, no "jsonrpc":"2.0" -- tracked in #5950); this parses - // only because encoding/json's unmarshal is case-insensitive. That's the - // envelope's job to fix, not this guard's -- here we're asserting authz - // *behavior* (denied, code 403), not wire conformance. - Expect(resp.Error.Code).To(Equal(int64(403))) + Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Error).To(BeNil()) By("confirming authz still allows the policy-permitted echo tool call") callReq, err := e2e.NewModernRequest("tools/call", map[string]any{ @@ -420,4 +422,30 @@ var _ = Describe("server/discover Authorization Guard", Label("proxy", "stateles Expect(callResp.StatusCode).To(Equal(200)) Expect(callResp.Error).To(BeNil()) }) + + It("still denies with 403 a method the Cedar policy does not permit", func() { + // server/discover's always-allowed flip doesn't touch the general + // default-deny rule (MCPMethodToFeatureOperation's doc comment: + // "Methods not in this map are denied by default"). tools/call IS in + // the map and routes through Cedar; the policy in this BeforeEach + // only permits resource == Tool::"echo", so calling any other tool + // name still gets denied -- keeps the 403/denial path (and its + // non-conformant-envelope wire shape, #5950) covered by this suite. + req, err := e2e.NewModernRequest("tools/call", map[string]any{ + "name": "notpermitted", + "arguments": map[string]any{"input": "shouldbedenied"}, + }) + Expect(err).ToNot(HaveOccurred()) + + resp, err := client.Send(context.Background(), proxyURL, req) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(403)) + Expect(resp.Error).ToNot(BeNil()) + // The denial envelope is currently non-conformant JSON-RPC (capitalized + // Result/Error/ID, no "jsonrpc":"2.0" -- tracked in #5950); this parses + // only because encoding/json's unmarshal is case-insensitive. That's the + // envelope's job to fix, not this guard's -- here we're asserting authz + // *behavior* (denied, code 403), not wire conformance. + Expect(resp.Error.Code).To(Equal(int64(403))) + }) }) From e86cc252d0ddec5b91f53b6f87d2acc22f5553ac Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 24 Jul 2026 20:59:55 +0200 Subject: [PATCH 13/18] Bump operator e2e yardstick image to 1.2.0 The operator e2e workflow docker-pulls and kind-loads YARDSTICK_IMAGE into the cluster. The operator acceptance tests (the dual-era k8s spec and ratelimit) now reference yardstick 1.2.0 via images.YardstickServerImage, so the preloaded image must match -- otherwise 1.2.0 is never loaded into kind and those specs can't find their backend image. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/test-e2e-lifecycle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-e2e-lifecycle.yml b/.github/workflows/test-e2e-lifecycle.yml index 0a13dbfc82..fe8589a5e5 100644 --- a/.github/workflows/test-e2e-lifecycle.yml +++ b/.github/workflows/test-e2e-lifecycle.yml @@ -19,7 +19,7 @@ jobs: runs-on: ubuntu-8cores-32gb timeout-minutes: 30 env: - YARDSTICK_IMAGE: ghcr.io/stackloklabs/yardstick/yardstick-server:1.1.1 + YARDSTICK_IMAGE: ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0 defaults: run: shell: bash From 7d9b9eb149d699f2972ec17a8b479d17a73a8f9c Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 24 Jul 2026 21:48:36 +0200 Subject: [PATCH 14/18] Fix dual-era e2e CI: headers, lint, spelling The E2E Lifecycle (k8s) tier drove Modern tools/call requests at a real go-sdk v1.7 streamable-HTTP backend, which rejects any Modern request missing the Mcp-Method header (and Mcp-Name for name-bearing methods) with -32020. NewModernRequest omitted both, so every k8s Modern request got HTTP 400. - NewModernRequest now emits the full conformant go-sdk v1.7 wire shape: Mcp-Method on every Modern request and Mcp-Name (plain target name) for tools/call / resources/read / prompts/get. The ToolHive single-server proxy never reads these headers, so the transparent/streamable tiers (hostile-input, crown-jewel, mixing) are unaffected; only a real Modern backend requires them. - Mark TestModernLoadDoesNotAccumulateState //nolint:paralleltest: it reads process-wide runtime.NumGoroutine() and starts an HTTP proxy, so it must run serially. Matches the package's existing body_limit_test.go. - "unparseable" -> "unparsable" (codespell). Co-Authored-By: Claude Opus 4.8 --- .../streamable_proxy_integration_test.go | 2 + test/e2e/dual_era_proxy_test.go | 2 +- test/e2e/hostile_input_proxy_test.go | 4 +- test/e2e/mcp_raw_client.go | 49 +++++++++++++++++-- test/e2e/mcp_raw_client_test.go | 10 ++++ 5 files changed, 59 insertions(+), 8 deletions(-) diff --git a/pkg/transport/proxy/streamable/streamable_proxy_integration_test.go b/pkg/transport/proxy/streamable/streamable_proxy_integration_test.go index 7514170274..899391267b 100644 --- a/pkg/transport/proxy/streamable/streamable_proxy_integration_test.go +++ b/pkg/transport/proxy/streamable/streamable_proxy_integration_test.go @@ -174,6 +174,8 @@ func syncMapLen(m *sync.Map) int { // growth here would come from net/http's own idle-connection handling, not // from code owned by this package -- hence the loose tolerance rather than a // strict equality. +// +//nolint:paralleltest // reads process-wide runtime.NumGoroutine() as a baseline and starts an HTTP proxy; must run serially func TestModernLoadDoesNotAccumulateState(t *testing.T) { port := pickFreePort(t) proxy, ctx, cancel := startProxyWithBackend(t, port) diff --git a/test/e2e/dual_era_proxy_test.go b/test/e2e/dual_era_proxy_test.go index 1c29f0fcfe..7311361860 100644 --- a/test/e2e/dual_era_proxy_test.go +++ b/test/e2e/dual_era_proxy_test.go @@ -247,7 +247,7 @@ func assertCorrelated(results []crossDeliveryResult, wantCount int, label string // extractNonce pulls the echoed "nonce" key out of a tools/call response's // result._meta, per yardstick-server's echo tool: "Also echoes back any // _meta field from the request for testing metadata propagation." Returns -// "" if absent or unparseable, which the caller's nonce-equality assertion +// "" if absent or unparsable, which the caller's nonce-equality assertion // then reports as a mismatch. func extractNonce(resp *e2e.RawResponse) string { if resp.Result == nil { diff --git a/test/e2e/hostile_input_proxy_test.go b/test/e2e/hostile_input_proxy_test.go index a890346384..d878d938d7 100644 --- a/test/e2e/hostile_input_proxy_test.go +++ b/test/e2e/hostile_input_proxy_test.go @@ -151,7 +151,7 @@ var _ = Describe("Hostile Input Proxy", Label("proxy", "stateless", "hostile-inp Expect(mockServer.GetCount()).To(Equal(countBefore), "oversized body must be rejected by bodylimit before the backend is contacted") - // Unlike the classification cases above, an unparseable body is NOT + // Unlike the classification cases above, an unparsable body is NOT // rejected at the edge: parseRPCRequest fails to decode it as either a // single request or a batch, so RoundTrip never calls ClassifyRevision // and forwards the body unclassified (Legacy passthrough) -- the proxy @@ -166,7 +166,7 @@ var _ = Describe("Hostile Input Proxy", Label("proxy", "stateless", "hostile-inp Expect(err).ToNot(HaveOccurred(), "must not hang or crash the proxy") Expect(resp.StatusCode).To(Equal(http.StatusBadRequest)) Expect(mockServer.GetCount()).To(Equal(countBefore+1), - "an unparseable body skips classification and IS forwarded to the backend") + "an unparsable body skips classification and IS forwarded to the backend") By("the proxy still serves a well-formed request after the hostile inputs above") req, err := e2e.NewModernRequest("tools/list", nil) diff --git a/test/e2e/mcp_raw_client.go b/test/e2e/mcp_raw_client.go index 61c7e17801..5853a24fa3 100644 --- a/test/e2e/mcp_raw_client.go +++ b/test/e2e/mcp_raw_client.go @@ -39,6 +39,16 @@ const ( HeaderMCPProtocolVersion = "MCP-Protocol-Version" // HeaderMCPSessionID is the HTTP header carrying the Legacy session id. HeaderMCPSessionID = "Mcp-Session-Id" + // HeaderMCPMethod is the HTTP header carrying the JSON-RPC method on Modern + // requests. A real go-sdk v1.7 streamable-HTTP server requires it on every + // Modern request (see pkg/mcp/revision.go ValidateHeaderConsistency); the + // ToolHive single-server proxy does not read it, but a Modern backend does. + HeaderMCPMethod = "Mcp-Method" + // HeaderMCPName is the HTTP header naming the request's target + // tool/resource/prompt on name-bearing Modern methods (tools/call, + // resources/read, prompts/get). Sent as the plain identifier; the draft + // spec also permits a base64 sentinel encoding, which we do not use. + HeaderMCPName = "Mcp-Name" ) // requestIDCounter assigns default JSON-RPC ids so distinct RawRequests get @@ -85,17 +95,25 @@ func NewLegacyInitializeRequest(clientName, clientVersion string) *RawRequest { return r } -// NewModernRequest builds a Modern (2026-07-28) JSON-RPC request: it sets the -// MCP-Protocol-Version header and the reserved _meta keys protocolVersion and +// NewModernRequest builds a Modern (2026-07-28) JSON-RPC request emitting the +// full conformant wire shape a real go-sdk v1.7 client produces: the +// MCP-Protocol-Version header, the reserved _meta keys protocolVersion and // clientCapabilities (clientInfo is optional per the draft schema and is -// omitted by default). Each can be independently overridden or removed via -// SetHeader/DeleteHeader and SetMeta/DeleteMeta. +// omitted by default), the Mcp-Method header (always), and the Mcp-Name header +// for name-bearing methods (tools/call, resources/read, prompts/get). A Modern +// backend rejects a request missing these headers (-32020); the ToolHive +// single-server proxy ignores them. Each field can be independently overridden +// or removed via SetHeader/DeleteHeader and SetMeta/DeleteMeta. func NewModernRequest(method string, params map[string]any) (*RawRequest, error) { r, err := newRawRequest(method, params) if err != nil { return nil, err } r.headers[HeaderMCPProtocolVersion] = MCPVersionModern + r.headers[HeaderMCPMethod] = method + if name, ok := modernNameHeader(method, params); ok { + r.headers[HeaderMCPName] = name + } r.meta = map[string]any{ MetaKeyProtocolVersion: MCPVersionModern, MetaKeyClientCapabilities: map[string]any{}, @@ -103,6 +121,27 @@ func NewModernRequest(method string, params map[string]any) (*RawRequest, error) return r, nil } +// modernNameHeader returns the plain Mcp-Name header value for a name-bearing +// Modern method, mirroring go-sdk v1.7's streamable-HTTP encoding and +// pkg/mcp's ValidateHeaderConsistency: tools/call and prompts/get name the +// "name" param, resources/read names the "uri" param. Any other method (e.g. +// tools/list, server/discover) carries no Mcp-Name, and sending one there +// would fail the backend's consistency check, so it is omitted. +func modernNameHeader(method string, params map[string]any) (string, bool) { + key := "" + switch method { + case "tools/call", "prompts/get": + key = "name" + case "resources/read": + key = "uri" + } + if key == "" { + return "", false + } + v, ok := params[key].(string) + return v, ok && v != "" +} + func newRawRequest(method string, params map[string]any) (*RawRequest, error) { if method == "" { return nil, errors.New("mcp_raw_client: method must not be empty") @@ -230,7 +269,7 @@ type RawResponse struct { StatusCode int Headers http.Header Body []byte // raw response body, always populated - ID any // echoed "id"; nil if absent, JSON null, or unparseable + ID any // echoed "id"; nil if absent, JSON null, or unparsable Result json.RawMessage Error *RawRPCError } diff --git a/test/e2e/mcp_raw_client_test.go b/test/e2e/mcp_raw_client_test.go index 7c25bf5be1..c87419e7e8 100644 --- a/test/e2e/mcp_raw_client_test.go +++ b/test/e2e/mcp_raw_client_test.go @@ -35,6 +35,8 @@ func TestRawClientBuilders(t *testing.T) { require.NoError(t, err) require.Equal(t, MCPVersionModern, req.headers[HeaderMCPProtocolVersion]) + require.Equal(t, "tools/list", req.headers[HeaderMCPMethod], "Mcp-Method is required on every Modern request") + require.NotContains(t, req.headers, HeaderMCPName, "tools/list is not name-bearing, so no Mcp-Name") wire := wireOf(t, req) meta := wire["params"].(map[string]any)["_meta"].(map[string]any) @@ -43,6 +45,14 @@ func TestRawClientBuilders(t *testing.T) { require.NotContains(t, meta, MetaKeyClientInfo) }) + t.Run("ModernRequest sets Mcp-Name to the plain target name for name-bearing methods", func(t *testing.T) { + t.Parallel() + req, err := NewModernRequest("tools/call", map[string]any{"name": "echo"}) + require.NoError(t, err) + require.Equal(t, "tools/call", req.headers[HeaderMCPMethod]) + require.Equal(t, "echo", req.headers[HeaderMCPName], "tools/call must name its tool in Mcp-Name, matching body params.name") + }) + t.Run("ModernRequest fields are independently overridable", func(t *testing.T) { t.Parallel() req, err := NewModernRequest("tools/list", nil) From 2513163a109dd138c4e1421053feada1e88c2c65 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sat, 25 Jul 2026 09:27:22 +0200 Subject: [PATCH 15/18] Send required Accept header on raw MCP requests The k8s multi-replica tier drives Modern requests at a real go-sdk v1.7 streamable-HTTP backend (yardstick), which rejects any POST whose Accept header does not list both application/json and text/event-stream with HTTP 400 -- before any classification. The raw client sent no Accept header, so every k8s request got 400 on request 0. This was masked in local reproduction because curl auto-sends "Accept: */*", which go-sdk's streamableAccepts treats as satisfying both media types. Verified on the wire: with Accept absent the backend returns 400 "Accept must contain both ...", with it present, 200. The other tiers never hit a real go-sdk HTTP backend -- the transparent proxy's in-process mock and the streamable proxy's stdio backend both ignore Accept -- which is why they passed and hid this. thv itself is correct: it transparently forwards the client's headers; a real MCP client always sends Accept, so this deployment works in production. The gap is in the test harness, not the proxy. Default Accept in SendRaw alongside Content-Type (a transport-level requirement for both eras), overridable by a caller that sets its own. Co-Authored-By: Claude Opus 4.8 --- test/e2e/mcp_raw_client.go | 16 ++++++++++++---- test/e2e/mcp_raw_client_test.go | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/test/e2e/mcp_raw_client.go b/test/e2e/mcp_raw_client.go index 5853a24fa3..178b68d990 100644 --- a/test/e2e/mcp_raw_client.go +++ b/test/e2e/mcp_raw_client.go @@ -304,10 +304,11 @@ func NewRawMCPClient(timeout time.Duration) (*RawMCPClient, error) { }, nil } -// Send marshals req and POSTs it to url. No Accept header is set by -// default; a live streamable-HTTP MCP endpoint may require -// "Accept: application/json, text/event-stream" — set it via -// req.SetHeader("Accept", ...) when hitting a real proxy. +// Send marshals req and POSTs it to url. A default +// "Accept: application/json, text/event-stream" is applied by SendRaw (the +// streamable-HTTP transport requires it; a real go-sdk server rejects a POST +// without it as 400). Override it for a specific test via +// req.SetHeader("Accept", ...). func (c *RawMCPClient) Send(ctx context.Context, url string, req *RawRequest) (*RawResponse, error) { body, err := req.marshal() if err != nil { @@ -325,6 +326,13 @@ func (c *RawMCPClient) SendRaw(ctx context.Context, url string, headers map[stri return nil, fmt.Errorf("mcp_raw_client: build request: %w", err) } httpReq.Header.Set("Content-Type", "application/json") + // The MCP streamable-HTTP transport requires Accept to list both + // application/json and text/event-stream on every POST; a real go-sdk + // server rejects a request without it (HTTP 400) before any classification. + // ToolHive's own proxy inbound is lenient about this, but a Modern backend + // behind it is not — so default it here (like Content-Type). Set before the + // caller loop so a test can still override Accept for a specific case. + httpReq.Header.Set("Accept", "application/json, text/event-stream") for k, v := range headers { httpReq.Header.Set(k, v) } diff --git a/test/e2e/mcp_raw_client_test.go b/test/e2e/mcp_raw_client_test.go index c87419e7e8..e28899d999 100644 --- a/test/e2e/mcp_raw_client_test.go +++ b/test/e2e/mcp_raw_client_test.go @@ -289,6 +289,25 @@ func TestRawClientSend(t *testing.T) { require.Nil(t, resp.Error) require.Equal(t, []byte(`not json at all`), resp.Body) }) + + t.Run("Accept defaults to the streamable-HTTP required value and is overridable", func(t *testing.T) { + t.Parallel() + server, captured := newCapturingServer(t, `{"jsonrpc":"2.0","id":1,"result":{}}`) + + req, err := NewModernRequest("tools/list", nil) + require.NoError(t, err) + _, err = client.Send(context.Background(), server.URL, req) + require.NoError(t, err) + require.Equal(t, "application/json, text/event-stream", captured.headers.Get("Accept"), + "a real go-sdk streamable-HTTP backend rejects a POST without this Accept (HTTP 400)") + + override, err := NewModernRequest("tools/list", nil) + require.NoError(t, err) + override.SetHeader("Accept", "text/plain") + _, err = client.Send(context.Background(), server.URL, override) + require.NoError(t, err) + require.Equal(t, "text/plain", captured.headers.Get("Accept"), "a test must be able to override Accept") + }) } func TestNewRawMCPClientRejectsNonPositiveTimeout(t *testing.T) { From ab593a0f632d70adfbdc6d9822138ce95a8db1f6 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sat, 25 Jul 2026 13:24:58 +0200 Subject: [PATCH 16/18] Drop Legacy-session k8s specs; single-server can't bridge eras The k8s tier deploys one STATELESS=true backend (required so Modern 2026-07-28 per-request traffic works) but two specs then asserted Legacy session semantics against it: "serves mixed Legacy and Modern traffic" expected a Legacy initialize to mint an Mcp-Session-Id, and the Redis-503 spec built on the same session. A stateless go-sdk backend issues no sessions, and a single backend cannot be both stateless (for Modern clients) and session-issuing (for Legacy clients) at once. Verified on the wire: Legacy initialize against a stateless yardstick returns 200 but no Mcp-Session-Id, exactly as CI showed. Serving both eras over one backend is cross-generation bridging, which the epic (#5743) scopes to vMCP -- the single-server proxy does per-request version discrimination, not generation bridging. That bridge is design-only today (#5756, unimplemented). So these two specs tested an unsupported scenario against the wrong deployment type; this is a test defect, not a proxy bug. Remove both specs (and the now-orphaned scaleRedis helper / appsv1 import). The tier keeps the three Modern specs -- pods-ready + Redis wiring, Modern multi-replica routing, pod-eviction self-heal -- which is the correct single-server coverage. A file-level note records that when the vMCP cross-generation bridge (#5743/#5756) is implemented, the mixed-era and session-store-outage coverage should be re-added as a vMCP e2e test, not here. Co-Authored-By: Claude Opus 4.8 --- .../acceptance_tests/dual_era_k8s_test.go | 163 ++---------------- 1 file changed, 16 insertions(+), 147 deletions(-) diff --git a/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go b/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go index 0749174e7b..e1048af640 100644 --- a/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go +++ b/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go @@ -11,7 +11,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -37,10 +36,22 @@ import ( // addition since (NodePort access, traffic-driving, pod eviction, Redis // kill) is new and verified only by go build/vet/lint, not a live run. // -// Runs in its own namespace (not "default"): this file scales the shared -// "redis" Deployment to 0 to test the 503 path, which would otherwise pull -// Redis out from under the concurrent ratelimit_test.go Ordered block under -// --procs=8. +// Runs in its own namespace (not "default") to keep its Redis and MCPServer +// resources isolated from the concurrent ratelimit_test.go Ordered block +// under --procs=8. +// +// SCOPE (deliberate): this single-server MCPServer tier covers the MODERN +// (2026-07-28, stateless) path only -- Modern multi-replica routing and +// pod-eviction self-heal. It deliberately does NOT cover mixed Legacy+Modern +// traffic or a session-store (Redis) outage: both need a Legacy (session) +// client to open a session against this STATELESS backend, and a single +// go-sdk backend cannot be both stateless (to serve Modern per-request +// clients, spec below) and session-issuing (for Legacy clients) at once. +// Serving both eras over one backend is cross-generation bridging, which is a +// vMCP capability (epic #5743, design #5756), out of scope for a single-server +// MCPServer -- the proxy here does per-request version discrimination, not +// generation bridging. When that bridge is implemented, add the mixed-era and +// session-store-outage coverage as a vMCP e2e test, not here. // // Confirmed live (Step 4.1): the MCPServer controller creates TWO separate // workloads -- a Deployment named after the CR (the proxy runner, one @@ -232,53 +243,6 @@ var _ = Describe("Dual-Era Multi-Replica Backend", Ordered, func() { } }) - It("serves mixed Legacy and Modern traffic over the shared Redis session store", func() { - rawClient, err := e2e.NewRawMCPClient(15 * time.Second) - Expect(err).ToNot(HaveOccurred()) - - By("Legacy: initialize then a session-keyed tools/call") - initReq := e2e.NewLegacyInitializeRequest("k8s-mixed-legacy", "1.0") - initResp, err := rawClient.Send(context.Background(), proxyURL(), initReq) - Expect(err).ToNot(HaveOccurred()) - Expect(initResp.StatusCode).To(Equal(http.StatusOK)) - sessionID := initResp.Headers.Get(e2e.HeaderMCPSessionID) - Expect(sessionID).ToNot(BeEmpty()) - - legacyReq, err := e2e.NewLegacyRequest("tools/call", map[string]any{ - "name": "echo", - "arguments": map[string]any{"input": "legacymixed"}, - }) - Expect(err).ToNot(HaveOccurred()) - legacyReq.WithSessionID(sessionID) - legacyResp, err := rawClient.Send(context.Background(), proxyURL(), legacyReq) - Expect(err).ToNot(HaveOccurred()) - Expect(legacyResp.StatusCode).To(Equal(http.StatusOK)) - Expect(legacyResp.Error).To(BeNil()) - - By("Modern: stateless tools/call, no session id, alongside the Legacy session above") - modernReq, err := e2e.NewModernRequest("tools/call", map[string]any{ - "name": "echo", - "arguments": map[string]any{"input": "modernmixed"}, - }) - Expect(err).ToNot(HaveOccurred()) - modernResp, err := rawClient.Send(context.Background(), proxyURL(), modernReq) - Expect(err).ToNot(HaveOccurred()) - Expect(modernResp.StatusCode).To(Equal(http.StatusOK)) - Expect(modernResp.Error).To(BeNil()) - Expect(modernResp.Headers.Get(e2e.HeaderMCPSessionID)).To(BeEmpty(), "Modern must never carry Mcp-Session-Id") - - By("the Legacy session is still usable after the Modern request") - legacyReq2, err := e2e.NewLegacyRequest("tools/call", map[string]any{ - "name": "echo", - "arguments": map[string]any{"input": "legacymixed2"}, - }) - Expect(err).ToNot(HaveOccurred()) - legacyReq2.WithSessionID(sessionID) - legacyResp2, err := rawClient.Send(context.Background(), proxyURL(), legacyReq2) - Expect(err).ToNot(HaveOccurred()) - Expect(legacyResp2.StatusCode).To(Equal(http.StatusOK)) - }) - It("degrades cleanly and self-heals when a backend pod is evicted mid-traffic", func() { rawClient, err := e2e.NewRawMCPClient(15 * time.Second) Expect(err).ToNot(HaveOccurred()) @@ -329,99 +293,4 @@ var _ = Describe("Dual-Era Multi-Replica Backend", Ordered, func() { return ready, nil }, timeout, pollingInterval).Should(Equal(backendReplicas)) }) - - It("returns 503 when Redis becomes unavailable, then recovers once Redis is restored", func() { - rawClient, err := e2e.NewRawMCPClient(15 * time.Second) - Expect(err).ToNot(HaveOccurred()) - - By("establishing a Legacy session while Redis is up") - initReq := e2e.NewLegacyInitializeRequest("k8s-redis-503", "1.0") - initResp, err := rawClient.Send(context.Background(), proxyURL(), initReq) - Expect(err).ToNot(HaveOccurred()) - Expect(initResp.StatusCode).To(Equal(http.StatusOK)) - sessionID := initResp.Headers.Get(e2e.HeaderMCPSessionID) - Expect(sessionID).ToNot(BeEmpty()) - - By("scaling Redis to 0") - Expect(scaleRedis(ctx, testNamespace, 0)).To(Succeed()) - Eventually(func() (int, error) { - podList := &corev1.PodList{} - if err := k8sClient.List(ctx, podList, client.InNamespace(testNamespace), client.MatchingLabels{"app": "redis"}); err != nil { - return 0, err - } - return len(podList.Items), nil - }, timeout, pollingInterval).Should(Equal(0)) - - By("a request against the existing session now gets 503 (session store unavailable)") - req, err := e2e.NewLegacyRequest("tools/call", map[string]any{ - "name": "echo", - "arguments": map[string]any{"input": "redisdown"}, - }) - Expect(err).ToNot(HaveOccurred()) - req.WithSessionID(sessionID) - Eventually(func() (int, error) { - resp, sendErr := rawClient.Send(context.Background(), proxyURL(), req) - if sendErr != nil { - return 0, sendErr - } - return resp.StatusCode, nil - }, timeout, pollingInterval).Should(Equal(http.StatusServiceUnavailable)) - - By("restoring Redis") - Expect(scaleRedis(ctx, testNamespace, 1)).To(Succeed()) - Eventually(func() error { - return testutil.CheckPodsReady(ctx, k8sClient, testNamespace, map[string]string{"app": "redis"}) - }, timeout, pollingInterval).Should(Succeed()) - - // Redis has no persistence here (a plain in-memory Deployment, see - // EnsureRedis) -- scaling to 0 wiped every session key, so the - // pre-outage sessionID is gone, not just temporarily unreachable. - // Reusing it now would correctly 404 (session-not-found), not 200: - // that would assert session *survival* across an outage that - // destroys state, which isn't the property here. The property is - // functional recovery -- a brand new session must work. - By("Redis is functionally back: a fresh initialize + call succeeds") - var newSessionID string - Eventually(func() (int, error) { - resp, sendErr := rawClient.Send(context.Background(), proxyURL(), e2e.NewLegacyInitializeRequest("k8s-redis-503-post", "1.0")) - if sendErr != nil { - return 0, sendErr - } - newSessionID = resp.Headers.Get(e2e.HeaderMCPSessionID) - return resp.StatusCode, nil - }, timeout, pollingInterval).Should(Equal(http.StatusOK)) - Expect(newSessionID).ToNot(BeEmpty()) - - req2, err := e2e.NewLegacyRequest("tools/call", map[string]any{ - "name": "echo", - "arguments": map[string]any{"input": "redisback"}, - }) - Expect(err).ToNot(HaveOccurred()) - req2.WithSessionID(newSessionID) - resp2, err := rawClient.Send(context.Background(), proxyURL(), req2) - Expect(err).ToNot(HaveOccurred()) - Expect(resp2.StatusCode).To(Equal(http.StatusOK)) - - By("the pre-outage session is gone, not silently revived -- confirms the outage really cleared state") - staleReq, err := e2e.NewLegacyRequest("tools/call", map[string]any{ - "name": "echo", - "arguments": map[string]any{"input": "stalesession"}, - }) - Expect(err).ToNot(HaveOccurred()) - staleReq.WithSessionID(sessionID) - staleResp, err := rawClient.Send(context.Background(), proxyURL(), staleReq) - Expect(err).ToNot(HaveOccurred()) - Expect(staleResp.StatusCode).To(Equal(http.StatusNotFound)) - }) }) - -// scaleRedis patches the "redis" Deployment's replica count (0 to simulate -// an outage, 1 to restore) -- the same Deployment EnsureRedis creates. -func scaleRedis(ctx context.Context, namespace string, replicas int32) error { - deploy := &appsv1.Deployment{} - if err := k8sClient.Get(ctx, client.ObjectKey{Name: "redis", Namespace: namespace}, deploy); err != nil { - return err - } - deploy.Spec.Replicas = ptr.To(replicas) - return k8sClient.Update(ctx, deploy) -} From aca00b6fddf65aa93825c4b3ac089015e351de17 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sat, 25 Jul 2026 18:12:22 +0200 Subject: [PATCH 17/18] Revert global Accept default; opt in only for real backends The near-final "Send required Accept header" commit made SendRaw set Accept: application/json, text/event-stream on every request. The ToolHive streamable proxy switches to its SSE response path for any Accept containing text/event-stream, emitting `data: {...}` frames, but the raw client's populateEnvelope only parses a plain JSON body -- so on the streamable-proxy specs resp.Result stayed nil and nonce extraction returned "". That silently broke the four specs that parse response bodies (cross-delivery crown jewel, concurrent mixing, resilience hang/crash), and directly contradicted dual_era_mixing_test.go's own comment, which had already documented why Accept must NOT be set here. The resilience specs also assert HTTP 504 / >=500, which the SSE path never returns (it answers 200 with an in-band error frame) -- so SSE unwrapping alone would not have fixed them. Revert the global default so proxy requests get a plain JSON body again. The k8s tier legitimately needs the header (its real go-sdk backend 400s without it and only checks status), so add an explicit opt-in -- RawRequest.WithStreamableAccept() -- and call it on the two k8s Modern requests. This restores the documented, verified plain-JSON behavior for every proxy spec while keeping the k8s tier correct. Verified locally: LABEL_FILTER=dual-era task test-e2e -> 11/11 pass. Co-Authored-By: Claude Opus 4.8 --- test/e2e/mcp_raw_client.go | 28 +++++++++++-------- test/e2e/mcp_raw_client_test.go | 15 +++++----- .../acceptance_tests/dual_era_k8s_test.go | 4 +++ 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/test/e2e/mcp_raw_client.go b/test/e2e/mcp_raw_client.go index 178b68d990..8dbb96be78 100644 --- a/test/e2e/mcp_raw_client.go +++ b/test/e2e/mcp_raw_client.go @@ -172,6 +172,16 @@ func (r *RawRequest) WithClientInfo(name, version string) *RawRequest { return r.SetMeta(MetaKeyClientInfo, map[string]any{"name": name, "version": version}) } +// WithStreamableAccept sets "Accept: application/json, text/event-stream". +// A real go-sdk streamable-HTTP server rejects a POST without this header +// (HTTP 400), so requests bound for a real backend (e.g. the k8s tier) must +// set it. Do NOT set it for requests to the ToolHive proxy: the proxy does not +// require it and switches to an SSE response body when it is present, which +// this client's plain-JSON parser cannot read. +func (r *RawRequest) WithStreamableAccept() *RawRequest { + return r.SetHeader("Accept", "application/json, text/event-stream") +} + // WithRawBody replaces the entire request body with body, sent verbatim. // Use this for truncated, oversized, or garbage bodies. body is cloned; // mutating the caller's slice afterward has no effect on the request. @@ -304,11 +314,12 @@ func NewRawMCPClient(timeout time.Duration) (*RawMCPClient, error) { }, nil } -// Send marshals req and POSTs it to url. A default -// "Accept: application/json, text/event-stream" is applied by SendRaw (the -// streamable-HTTP transport requires it; a real go-sdk server rejects a POST -// without it as 400). Override it for a specific test via -// req.SetHeader("Accept", ...). +// Send marshals req and POSTs it to url. No Accept header is set by default: +// the ToolHive proxy returns a plain JSON body without it (which this client's +// populateEnvelope parses) and switches to an unparsable SSE response body +// whenever Accept lists text/event-stream. A request bound for a REAL +// streamable-HTTP MCP backend -- which rejects a POST lacking that Accept with +// HTTP 400 -- should opt in via req.WithStreamableAccept(). func (c *RawMCPClient) Send(ctx context.Context, url string, req *RawRequest) (*RawResponse, error) { body, err := req.marshal() if err != nil { @@ -326,13 +337,6 @@ func (c *RawMCPClient) SendRaw(ctx context.Context, url string, headers map[stri return nil, fmt.Errorf("mcp_raw_client: build request: %w", err) } httpReq.Header.Set("Content-Type", "application/json") - // The MCP streamable-HTTP transport requires Accept to list both - // application/json and text/event-stream on every POST; a real go-sdk - // server rejects a request without it (HTTP 400) before any classification. - // ToolHive's own proxy inbound is lenient about this, but a Modern backend - // behind it is not — so default it here (like Content-Type). Set before the - // caller loop so a test can still override Accept for a specific case. - httpReq.Header.Set("Accept", "application/json, text/event-stream") for k, v := range headers { httpReq.Header.Set(k, v) } diff --git a/test/e2e/mcp_raw_client_test.go b/test/e2e/mcp_raw_client_test.go index e28899d999..e0402ecfe7 100644 --- a/test/e2e/mcp_raw_client_test.go +++ b/test/e2e/mcp_raw_client_test.go @@ -290,7 +290,7 @@ func TestRawClientSend(t *testing.T) { require.Equal(t, []byte(`not json at all`), resp.Body) }) - t.Run("Accept defaults to the streamable-HTTP required value and is overridable", func(t *testing.T) { + t.Run("no Accept header by default, opt-in via WithStreamableAccept", func(t *testing.T) { t.Parallel() server, captured := newCapturingServer(t, `{"jsonrpc":"2.0","id":1,"result":{}}`) @@ -298,15 +298,16 @@ func TestRawClientSend(t *testing.T) { require.NoError(t, err) _, err = client.Send(context.Background(), server.URL, req) require.NoError(t, err) - require.Equal(t, "application/json, text/event-stream", captured.headers.Get("Accept"), - "a real go-sdk streamable-HTTP backend rejects a POST without this Accept (HTTP 400)") + require.Empty(t, captured.headers.Get("Accept"), + "no Accept by default: it makes the ToolHive proxy emit an SSE body this client can't parse") - override, err := NewModernRequest("tools/list", nil) + withAccept, err := NewModernRequest("tools/list", nil) require.NoError(t, err) - override.SetHeader("Accept", "text/plain") - _, err = client.Send(context.Background(), server.URL, override) + withAccept.WithStreamableAccept() + _, err = client.Send(context.Background(), server.URL, withAccept) require.NoError(t, err) - require.Equal(t, "text/plain", captured.headers.Get("Accept"), "a test must be able to override Accept") + require.Equal(t, "application/json, text/event-stream", captured.headers.Get("Accept"), + "WithStreamableAccept opts into the Accept a real streamable-HTTP backend requires") }) } diff --git a/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go b/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go index e1048af640..dfb751c74b 100644 --- a/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go +++ b/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go @@ -235,6 +235,9 @@ var _ = Describe("Dual-Era Multi-Replica Backend", Ordered, func() { "arguments": map[string]any{"input": "anypod"}, }) Expect(err).ToNot(HaveOccurred()) + // The backend is a real go-sdk streamable-HTTP server, which 400s a + // POST without this Accept; the ToolHive proxy tiers must NOT set it. + req.WithStreamableAccept() resp, err := rawClient.Send(context.Background(), proxyURL(), req) Expect(err).ToNot(HaveOccurred(), "request %d", i) Expect(resp.StatusCode).To(Equal(http.StatusOK), "request %d", i) @@ -265,6 +268,7 @@ var _ = Describe("Dual-Era Multi-Replica Backend", Ordered, func() { if buildErr != nil { return 0, buildErr } + req.WithStreamableAccept() // real go-sdk backend requires Accept: text/event-stream resp, sendErr := rawClient.Send(context.Background(), proxyURL(), req) if sendErr != nil { return 0, sendErr From a415ac9e40790f79371050f36433dfc0910c7dbe Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Mon, 27 Jul 2026 09:24:17 +0200 Subject: [PATCH 18/18] Isolate yardstick 1.2.0 to the dual-era tests Bumping the shared YardstickServerImage to 1.2.0 (go-sdk v1.7) broke the sibling vMCP/virtualmcp e2e suites, which were written for a Legacy backend. A v1.7 server in session mode (how those tests deploy it) answers server/discover with a Modern 200, so the vMCP classifies it Modern and drives the Modern data-plane -- which that same server then rejects ("2026-07-28 is only supported on stateless HTTP servers"). The vMCP has no Legacy fallback after a conclusive Modern probe, so those backends never aggregate (surfacing as "MCP server connection timed out" / backend-not-ready). vMCP support for Modern backends is #5993. Keep the shared YardstickServerImage on Legacy 1.1.1 for the operator and vMCP suites; add YardstickServerImageDualEra (1.2.0) used only by the dual-era transport-proxy tests, which handle Modern correctly. Wire the workflows to pull/load both images (vmcp bucket -> 1.1.1, proxy bucket -> 1.2.0; lifecycle loads both into kind). Co-Authored-By: Claude Opus 4.8 --- .github/workflows/e2e-tests.yml | 8 ++++---- .github/workflows/test-e2e-lifecycle.yml | 7 ++++++- test/e2e/dual_era_functional_test.go | 8 ++++---- test/e2e/dual_era_mixing_test.go | 2 +- test/e2e/dual_era_observability_test.go | 2 +- test/e2e/dual_era_proxy_test.go | 2 +- test/e2e/dual_era_resilience_test.go | 2 +- test/e2e/images/images.go | 14 ++++++++++++-- .../acceptance_tests/dual_era_k8s_test.go | 2 +- 9 files changed, 31 insertions(+), 16 deletions(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 0aad9b00cf..8a20a13951 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -130,12 +130,12 @@ jobs: docker pull ghcr.io/stackloklabs/osv-mcp/server:0.1.3 & docker pull ghcr.io/stackloklabs/gofetch/server:1.0.2 & docker pull ghcr.io/stacklok/toolhive/egress-proxy:latest & - # yardstick is only needed for the vmcp test suite + # yardstick (Legacy 1.1.1) is the backend for the vmcp test suite if [ "${{ matrix.label_filter }}" = "vmcp" ]; then - docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0 & + docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.1.1 & fi - # the time server and yardstick (dual-era barrier mode) are only - # used by the proxy (streamable-http) test suite + # the time server and the Modern (1.2.0) yardstick (dual-era barrier mode) + # are only used by the proxy (streamable-http) test suite if [ "${{ matrix.label_filter }}" = "proxy && !isolation" ]; then docker pull ghcr.io/stacklok/dockyard/uvx/mcp-server-time:2026.1.26 & docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0 & diff --git a/.github/workflows/test-e2e-lifecycle.yml b/.github/workflows/test-e2e-lifecycle.yml index fe8589a5e5..5fd9672aaf 100644 --- a/.github/workflows/test-e2e-lifecycle.yml +++ b/.github/workflows/test-e2e-lifecycle.yml @@ -19,7 +19,10 @@ jobs: runs-on: ubuntu-8cores-32gb timeout-minutes: 30 env: - YARDSTICK_IMAGE: ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0 + # Legacy (go-sdk v1.6.1) backend for the operator + virtualmcp suites. + YARDSTICK_IMAGE: ghcr.io/stackloklabs/yardstick/yardstick-server:1.1.1 + # Modern (go-sdk v1.7) backend used only by the dual-era k8s spec (#5837). + YARDSTICK_IMAGE_DUAL_ERA: ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0 defaults: run: shell: bash @@ -96,6 +99,7 @@ jobs: # Pull and load all test server images in parallel to speed up CI echo "Pulling and loading test server images..." docker pull ${{ env.YARDSTICK_IMAGE }} & + docker pull ${{ env.YARDSTICK_IMAGE_DUAL_ERA }} & docker pull ghcr.io/stackloklabs/gofetch/server:1.0.1 & docker pull ghcr.io/stackloklabs/osv-mcp/server:0.0.7 & docker pull python:3.9-slim & @@ -106,6 +110,7 @@ jobs: # Load all images into kind kind load docker-image --name toolhive ${{ env.YARDSTICK_IMAGE }} + kind load docker-image --name toolhive ${{ env.YARDSTICK_IMAGE_DUAL_ERA }} kind load docker-image --name toolhive ghcr.io/stackloklabs/gofetch/server:1.0.1 kind load docker-image --name toolhive ghcr.io/stackloklabs/osv-mcp/server:0.0.7 kind load docker-image --name toolhive python:3.9-slim diff --git a/test/e2e/dual_era_functional_test.go b/test/e2e/dual_era_functional_test.go index 36b0362b2b..f8330e55d5 100644 --- a/test/e2e/dual_era_functional_test.go +++ b/test/e2e/dual_era_functional_test.go @@ -41,10 +41,10 @@ import ( // the suite's global timeout. const execTimeout = 90 * time.Second -// yardstickClientVersion is derived from images.YardstickServerImage's tag +// yardstickClientVersion is derived from images.YardstickServerImageDualEra's tag // (the part after ":") so a future image bump can't silently pair a // mismatched go-sdk version between yardstick-server and yardstick-client. -var yardstickClientVersion = "v" + strings.SplitN(images.YardstickServerImage, ":", 2)[1] +var yardstickClientVersion = "v" + strings.SplitN(images.YardstickServerImageDualEra, ":", 2)[1] var ( yardstickClientOnce sync.Once @@ -137,7 +137,7 @@ var _ = Describe("Modern Functional Round-Trip — Streamable Proxy", Label("pro "--name", serverName, "--transport", "stdio", "-e", "BACKEND_MODE=echo", - images.YardstickServerImage, + images.YardstickServerImageDualEra, ).ExpectSuccess() err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) @@ -382,7 +382,7 @@ var _ = Describe("server/discover Authorization Guard", Label("proxy", "stateles "--transport", "stdio", "-e", "BACKEND_MODE=echo", "--authz-config", authzConfigPath, - images.YardstickServerImage, + images.YardstickServerImageDualEra, ).ExpectSuccess() err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) diff --git a/test/e2e/dual_era_mixing_test.go b/test/e2e/dual_era_mixing_test.go index 9337bb72cc..ea2d178da4 100644 --- a/test/e2e/dual_era_mixing_test.go +++ b/test/e2e/dual_era_mixing_test.go @@ -92,7 +92,7 @@ var _ = Describe("Dual-Era Concurrent Mixing", Label("proxy", "stateless", "dual "--name", serverName, "--transport", "stdio", "-e", "BACKEND_MODE=echo", - images.YardstickServerImage, + images.YardstickServerImageDualEra, ).ExpectSuccess() err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) diff --git a/test/e2e/dual_era_observability_test.go b/test/e2e/dual_era_observability_test.go index 6bbceb2be5..9217c7076c 100644 --- a/test/e2e/dual_era_observability_test.go +++ b/test/e2e/dual_era_observability_test.go @@ -135,7 +135,7 @@ var _ = Describe("Dual-Era Observability — OTLP Spans", Label("proxy", "statel "--otel-endpoint", collector.URL(), "--otel-insecure", "--otel-sampling-rate", "1.0", // always-sample: assertions must not race the default 10% ratio - images.YardstickServerImage, + images.YardstickServerImageDualEra, ).ExpectSuccess() err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) diff --git a/test/e2e/dual_era_proxy_test.go b/test/e2e/dual_era_proxy_test.go index 7311361860..49207f47ae 100644 --- a/test/e2e/dual_era_proxy_test.go +++ b/test/e2e/dual_era_proxy_test.go @@ -84,7 +84,7 @@ var _ = Describe("Dual-Era Cross-Delivery Proxy", Label("proxy", "stateless", "d "-e", "BACKEND_MODE=barrier", "-e", fmt.Sprintf("BARRIER_N=%d", barrierN), "-e", fmt.Sprintf("BARRIER_TIMEOUT_SECONDS=%d", int(barrierTimeout.Seconds())), - images.YardstickServerImage, + images.YardstickServerImageDualEra, ).ExpectSuccess() err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) diff --git a/test/e2e/dual_era_resilience_test.go b/test/e2e/dual_era_resilience_test.go index dd8fea2c3e..a18d985f30 100644 --- a/test/e2e/dual_era_resilience_test.go +++ b/test/e2e/dual_era_resilience_test.go @@ -104,7 +104,7 @@ var _ = Describe("Dual-Era Resilience — Fault Injection", Label("proxy", "stat for _, e := range extraEnv { args = append(args, "-e", e) } - args = append(args, images.YardstickServerImage) + args = append(args, images.YardstickServerImageDualEra) // Bound the request timeout so a hang fails fast in the test, not // after the streamable proxy's 60s default. diff --git a/test/e2e/images/images.go b/test/e2e/images/images.go index 6f009b3259..94e0711875 100644 --- a/test/e2e/images/images.go +++ b/test/e2e/images/images.go @@ -12,14 +12,24 @@ package images const ( yardstickServerImageURL = "ghcr.io/stackloklabs/yardstick/yardstick-server" - yardstickServerImageTag = "1.2.0" - // YardstickServerImage is used in operator tests across multiple transport protocols + yardstickServerImageTag = "1.1.1" + // YardstickServerImage is the Legacy (2025-11-25, go-sdk v1.6.1) yardstick backend + // used by the operator and vMCP e2e tests across multiple transport protocols // (stdio, SSE, streamable-http) and tenancy modes. // Note: This image is also referenced in 8 YAML fixture files under // test/e2e/chainsaw/operator/. Those files are declarative Kubernetes manifests // and cannot import Go constants directly. YardstickServerImage = yardstickServerImageURL + ":" + yardstickServerImageTag + yardstickServerDualEraImageTag = "1.2.0" + // YardstickServerImageDualEra is the Modern-capable (2026-07-28, go-sdk v1.7) + // yardstick backend, used ONLY by the dual-era transport-proxy tests (#5837), which + // drive it as a Modern/stateless backend. The operator/vMCP suites must stay on the + // Legacy YardstickServerImage until the vMCP can drive a Modern backend (#5993): a + // v1.7 server in session mode answers server/discover but rejects the Modern + // data-plane, so the vMCP classifies it Modern and then cannot aggregate it. + YardstickServerImageDualEra = yardstickServerImageURL + ":" + yardstickServerDualEraImageTag + gofetchServerImageURL = "ghcr.io/stackloklabs/gofetch/server" gofetchServerImageTag = "1.0.1" // GofetchServerImage is used for testing virtual MCP server features, including diff --git a/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go b/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go index dfb751c74b..387d3a9cde 100644 --- a/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go +++ b/test/e2e/thv-operator/acceptance_tests/dual_era_k8s_test.go @@ -141,7 +141,7 @@ var _ = Describe("Dual-Era Multi-Replica Backend", Ordered, func() { Namespace: testNamespace, }, Spec: mcpv1beta1.MCPServerSpec{ - Image: images.YardstickServerImage, + Image: images.YardstickServerImageDualEra, Transport: "streamable-http", ProxyPort: 8080, MCPPort: 8080,