From 471b57d593d89407ce5b05f5c95bce078cc8b555 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 12:50:24 +0300 Subject: [PATCH 01/19] test(server): Spec 058 US3 failing stateless-list and stdio era tests (T020-T022) FR-012 byte-identical tools/list across two connections in both eras over an unpinned era harness; direct-tools and aggregated-prompts list filters must ignore a session's set_profile selection (fails on main); a modern stdio request must not persist a profile selection despite stdio's constant session id (fails on main: it does). --- internal/server/profile_era_test.go | 214 +++++++++++++++++ internal/server/stateless_list_test.go | 315 +++++++++++++++++++++++++ 2 files changed, 529 insertions(+) create mode 100644 internal/server/profile_era_test.go create mode 100644 internal/server/stateless_list_test.go diff --git a/internal/server/profile_era_test.go b/internal/server/profile_era_test.go new file mode 100644 index 000000000..b2aa7f6a2 --- /dev/null +++ b/internal/server/profile_era_test.go @@ -0,0 +1,214 @@ +package server + +import ( + "bufio" + "context" + "encoding/json" + "io" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + mcpserver "github.com/mark3labs/mcp-go/server" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" +) + +// Spec 058 US3 (FR-011): under the 2026-07-28 revision there is no session, so +// nothing a request does may persist per-connection state. The profile +// resolver's session tier (the legacy set_profile selection) is the one such +// state mcpproxy holds. An empty session id is NOT a sufficient guard: the +// stdio transport reports the constant "stdio" in both eras, so a modern stdio +// request would otherwise persist a selection like a legacy one (research D1, +// graft 1). The gate must be the era itself. +// +// Test names avoid the substring "MCP" (release-qa-gate.yml skips on it). + +// profileEraProxy is a bare proxy with one profile, for resolver-level cells. +func profileEraProxy(t *testing.T) *MCPProxyServer { + t.Helper() + return statelessListProxy(t) +} + +// modernRequestCtx stamps ctx as a 2026-07-28 request, exactly as mcp-go's +// dispatch loop does before any hook or handler runs (request_handler.go). +func modernRequestCtx(ctx context.Context) context.Context { + return mcpserver.WithRequestProtocolInfo(ctx, &mcpserver.RequestProtocolInfo{ + Modern: true, + ProtocolVersion: mcp.LATEST_PROTOCOL_VERSION, + ClientInfo: &mcp.Implementation{Name: "era-probe", Version: "1.0.0"}, + }) +} + +// The resolver's session tier applies to a legacy request on a session and is +// skipped — not cleared — for a modern request on the SAME non-empty session +// id. Both resolvers (v2 precedence and the Spec 108 v3 resolution) must agree. +func TestProfileEra_SessionTierSkippedOnModernRequests(t *testing.T) { + proxy := profileEraProxy(t) + proxy.sessionStore.SetActiveProfile("stdio", "research") + + legacy := sessionCtx(adminCtx(), "stdio") + modern := modernRequestCtx(legacy) + idx := proxy.profileIndexFor(proxy.currentConfig()) + + name, scope := proxy.resolveActiveProfile(legacy) + require.Equal(t, "research", name, "a legacy request on the session honours its selection") + require.NotNil(t, scope) + + name, scope = proxy.resolveActiveProfile(modern) + assert.Equal(t, "", name, "a modern request must not inherit connection state, even on a non-empty session id") + assert.Nil(t, scope) + + res := proxy.ResolveProfileV3(modern, idx) + assert.Equal(t, "", res.Name) + assert.Equal(t, string(profile.SourceNone), res.Source) + assert.Nil(t, res.Scope) + + assert.Equal(t, "research", proxy.sessionStore.GetActiveProfile("stdio"), + "skipping the tier is a read-side gate; a modern request neither applies nor clears a legacy selection") +} + +// ─── stdio surface ────────────────────────────────────────────────────────── + +// stdioProbe drives a proxy's default server over an in-process stdio +// transport, with the production stdio context (admin identity, stdio source). +type stdioProbe struct { + t *testing.T + stdin *io.PipeWriter + stdout *bufio.Reader + nextID int +} + +func newStdioProbe(t *testing.T, srv *mcpserver.MCPServer) *stdioProbe { + t.Helper() + stdio := mcpserver.NewStdioServer(srv) + stdio.SetContextFunc(stdioAuthContext) + + stdinR, stdinW := io.Pipe() + stdoutR, stdoutW := io.Pipe() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + _ = stdio.Listen(ctx, stdinR, stdoutW) + }() + t.Cleanup(func() { + cancel() + _ = stdinW.Close() + _ = stdoutW.Close() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Log("stdio server did not stop within 5s") + } + }) + return &stdioProbe{t: t, stdin: stdinW, stdout: bufio.NewReader(stdoutR), nextID: 1} +} + +// request writes one JSON-RPC request line and returns the response with the +// matching id, skipping any notifications the server emits in between. +func (p *stdioProbe) request(method string, params map[string]any) map[string]any { + p.t.Helper() + id := p.nextID + p.nextID++ + if params == nil { + params = map[string]any{} + } + line, err := json.Marshal(map[string]any{"jsonrpc": "2.0", "id": id, "method": method, "params": params}) + require.NoError(p.t, err) + _, err = p.stdin.Write(append(line, '\n')) + require.NoError(p.t, err) + + type read struct { + msg map[string]any + err error + } + for { + ch := make(chan read, 1) + go func() { + raw, err := p.stdout.ReadBytes('\n') + if err != nil { + ch <- read{err: err} + return + } + var msg map[string]any + ch <- read{msg: msg, err: json.Unmarshal(raw, &msg)} + }() + select { + case r := <-ch: + require.NoError(p.t, r.err) + if got, ok := r.msg["id"].(float64); ok && int(got) == id { + return r.msg + } + case <-time.After(10 * time.Second): + p.t.Fatalf("no response to %s (id %d) within 10s", method, id) + } + } +} + +// modernMeta is the _meta a 2026-07-28 request carries in place of initialize. +func modernMeta() map[string]any { + return map[string]any{ + mcp.MetaKeyProtocolVersion: mcp.LATEST_PROTOCOL_VERSION, + mcp.MetaKeyClientCapabilities: map[string]any{}, + mcp.MetaKeyClientInfo: map[string]any{"name": "era-probe", "version": "1.0.0"}, + } +} + +func profileEraRuntimeProxy(t *testing.T) *MCPProxyServer { + t.Helper() + proxy, _ := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { + cfg.Servers = []*config.ServerConfig{ + {Name: "research-srv", Enabled: false}, + {Name: "deploy-srv", Enabled: false}, + } + cfg.Profiles = []config.ProfileConfig{{Name: "research", Servers: []string{"research-srv"}}} + }) + return proxy +} + +// A legacy stdio session can select a profile (the control that proves the +// probe drives the real handler); a modern stdio request, whose session id is +// the same constant "stdio", cannot — the call is refused and nothing is +// stored. +func TestProfileEra_ModernStdioRequestCannotPersistSelection(t *testing.T) { + t.Run("legacy stdio session persists the selection", func(t *testing.T) { + proxy := profileEraRuntimeProxy(t) + probe := newStdioProbe(t, proxy.GetMCPServer()) + + init := probe.request(string(mcp.MethodInitialize), map[string]any{ + "protocolVersion": mcp.LATEST_LEGACY_PROTOCOL_VERSION, + "capabilities": map[string]any{}, + "clientInfo": map[string]any{"name": "era-probe", "version": "1.0.0"}, + }) + require.Nil(t, init["error"], "legacy initialize must succeed: %v", init) + + resp := probe.request(string(mcp.MethodToolsCall), map[string]any{ + "name": "set_profile", "arguments": map[string]any{"profile": "research"}, + }) + require.Nil(t, resp["error"], "legacy set_profile must not be a JSON-RPC error: %v", resp) + result := resp["result"].(map[string]any) + require.NotEqual(t, true, result["isError"], "legacy set_profile must succeed: %v", result) + assert.Equal(t, "research", proxy.sessionStore.GetActiveProfile("stdio"), + "the legacy stdio session keeps its selection") + }) + + t.Run("modern stdio request is refused and persists nothing", func(t *testing.T) { + proxy := profileEraRuntimeProxy(t) + probe := newStdioProbe(t, proxy.GetMCPServer()) + + resp := probe.request(string(mcp.MethodToolsCall), map[string]any{ + "name": "set_profile", "arguments": map[string]any{"profile": "research"}, "_meta": modernMeta(), + }) + if resp["error"] == nil { + result, ok := resp["result"].(map[string]any) + require.True(t, ok, "response must carry a result or an error: %v", resp) + assert.Equal(t, true, result["isError"], "a modern request must not be able to select a session profile: %v", result) + } + assert.Equal(t, "", proxy.sessionStore.GetActiveProfile("stdio"), + "FR-011: a modern request must not persist per-connection state, despite stdio's non-empty session id") + }) +} diff --git a/internal/server/stateless_list_test.go b/internal/server/stateless_list_test.go new file mode 100644 index 000000000..58230555a --- /dev/null +++ b/internal/server/stateless_list_test.go @@ -0,0 +1,315 @@ +package server + +import ( + "bufio" + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + mcpserver "github.com/mark3labs/mcp-go/server" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" +) + +// Spec 058 US3, FR-012: `tools/list`, `prompts/list` and `resources/list` must +// not vary per connection. Identity-scoped views are driven by request-carried +// identity (token, URL, _meta), never by connection state. The one piece of +// connection state mcpproxy holds is the legacy `set_profile` selection (tier +// 3 of the profile resolver), so these tests pin two things: +// +// - the canonical list facade is byte-identical across two connections in +// BOTH protocol eras (the modern era binds no session at all); +// - the two list filters that used to consult the session tier (direct-mode +// tools, aggregated prompts) no longer do, so a legacy session that ran +// set_profile gets the same list as a fresh one. Execution-time gates keep +// honouring the selection — the split is deliberate and asserted below. +// +// Test names avoid the substring "MCP" (release-qa-gate.yml skips on it). + +// ─── era-reachable proxy endpoint ─────────────────────────────────────────── + +// unpinnedProxyEndpoint mounts srv over an UNPINNED Streamable HTTP transport +// behind the same auth middleware production wraps every /mcp* endpoint with. +// The FR-028 pin is deliberately left off: until T041 lifts it on the listening +// endpoint, this is the only way forEachEra can reach the modern era against a +// real proxy, and the behaviour under test here is exactly what that lift will +// expose. Everything else — auth context, direct-request-kind box, the +// cancellation guard — is the production chain. +func unpinnedProxyEndpoint(t *testing.T, s *Server, srv *mcpserver.MCPServer) string { + t.Helper() + streamable := mcpserver.NewStreamableHTTPServer(srv, + mcpserver.WithDisableLocalhostProtection(true), + mcpserver.WithHTTPContextFunc(captureClientHeaders(s.liveConfig)), + ) + httpServer := httptest.NewServer(s.mcpAuthMiddleware(streamable)) + t.Cleanup(httpServer.Close) + return httpServer.URL +} + +// rawEraClient speaks JSON-RPC over HTTP by hand, pinned to one era, so a test +// can compare the exact bytes two connections receive. The mcp-go client would +// decode and re-encode them, which hides ordering and _meta differences. +type rawEraClient struct { + t *testing.T + url string + era protocolEra + sessionID string + nextID int +} + +func newRawEraClient(t *testing.T, url string, era protocolEra) *rawEraClient { + t.Helper() + c := &rawEraClient{t: t, url: url, era: era, nextID: 1} + if !c.modern() { + resp, msg := c.send(string(mcp.MethodInitialize), map[string]any{ + "protocolVersion": era.version, + "capabilities": map[string]any{}, + "clientInfo": map[string]any{"name": "raw-era-client", "version": "1.0.0"}, + }, true) + require.Equal(t, http.StatusOK, resp.StatusCode, "legacy initialize must succeed: %s", msg) + c.sessionID = resp.Header.Get(mcpserver.HeaderKeySessionID) + require.NotEmpty(t, c.sessionID, "the legacy era binds a session id on initialize") + c.send(string(mcp.MethodNotificationInitialized), nil, false) + } + return c +} + +func (c *rawEraClient) modern() bool { return mcp.IsModernProtocol(c.era.version) } + +// send posts one JSON-RPC message and returns the HTTP response plus the +// decoded JSON-RPC payload (SSE-framed responses are unwrapped). +func (c *rawEraClient) send(method string, params map[string]any, withID bool) (*http.Response, []byte) { + c.t.Helper() + if params == nil { + params = map[string]any{} + } + if c.modern() { + params["_meta"] = map[string]any{ + mcp.MetaKeyProtocolVersion: c.era.version, + mcp.MetaKeyClientCapabilities: map[string]any{}, + mcp.MetaKeyClientInfo: map[string]any{"name": "raw-era-client", "version": "1.0.0"}, + } + } + msg := map[string]any{"jsonrpc": "2.0", "method": method, "params": params} + if withID { + msg["id"] = c.nextID + c.nextID++ + } + body, err := json.Marshal(msg) + require.NoError(c.t, err) + + req, err := http.NewRequest(http.MethodPost, c.url, bytes.NewReader(body)) + require.NoError(c.t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + if c.modern() { + // SEP-2243: a modern POST mirrors its method (and the tool/prompt + // name) into routing headers; mcp-go refuses a request that omits them. + req.Header.Set(mcp.HeaderProtocolVersion, c.era.version) + req.Header.Set(mcp.HeaderMethod, method) + if name, ok := params["name"].(string); ok { + req.Header.Set(mcp.HeaderName, name) + } + } else if c.sessionID != "" { + req.Header.Set(mcpserver.HeaderKeySessionID, c.sessionID) + } + + client := &http.Client{Timeout: 15 * time.Second} + resp, err := client.Do(req) + require.NoError(c.t, err) + defer resp.Body.Close() + raw, err := io.ReadAll(resp.Body) + require.NoError(c.t, err) + return resp, unwrapJSONRPCBody(resp.Header.Get("Content-Type"), raw) +} + +// unwrapJSONRPCBody returns the JSON-RPC message carried by an HTTP response: +// the body itself for application/json, or the last `data:` frame of an SSE +// response (mcp-go upgrades to SSE when a handler emits notifications). +func unwrapJSONRPCBody(contentType string, raw []byte) []byte { + if !strings.HasPrefix(contentType, "text/event-stream") { + return raw + } + var last []byte + scanner := bufio.NewScanner(bytes.NewReader(raw)) + scanner.Buffer(make([]byte, 0, 64*1024), 16*1024*1024) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "data:") { + last = []byte(strings.TrimSpace(strings.TrimPrefix(line, "data:"))) + } + } + return last +} + +// result sends a request and returns the raw bytes of its `result` member, +// failing the test on a JSON-RPC error. +func (c *rawEraClient) result(method string, params map[string]any) json.RawMessage { + c.t.Helper() + resp, msg := c.send(method, params, true) + require.Equal(c.t, http.StatusOK, resp.StatusCode, "%s must succeed: %s", method, msg) + var envelope struct { + Result json.RawMessage `json:"result"` + Error json.RawMessage `json:"error"` + } + require.NoError(c.t, json.Unmarshal(msg, &envelope), "response must be JSON-RPC: %s", msg) + require.Nil(c.t, envelope.Error, "%s must not fail: %s", method, envelope.Error) + require.NotNil(c.t, envelope.Result, "%s must carry a result: %s", method, msg) + return envelope.Result +} + +// callTool sends tools/call and returns the decoded result. +func (c *rawEraClient) callTool(name string, args map[string]any) mcp.CallToolResult { + c.t.Helper() + raw := c.result(string(mcp.MethodToolsCall), map[string]any{"name": name, "arguments": args}) + var result mcp.CallToolResult + require.NoError(c.t, json.Unmarshal(raw, &result)) + return result +} + +// requireEra fails unless the connection observably took the era it was +// pinned to: the legacy era binds a session id, the modern era never does and +// decorates every result with a resultType. Without this a server pin would +// turn the modern subtest into a second legacy run (research D7). +func (c *rawEraClient) requireEra(result json.RawMessage) { + c.t.Helper() + var decorated struct { + ResultType string `json:"resultType"` + } + require.NoError(c.t, json.Unmarshal(result, &decorated)) + if c.modern() { + require.Empty(c.t, c.sessionID, "the modern era must not bind a session id") + require.Equal(c.t, "complete", decorated.ResultType, + "a modern result carries resultType; its absence means the %s request was served on the legacy path", c.era.version) + return + } + require.NotEmpty(c.t, c.sessionID, "the legacy era must bind a session id") + require.Empty(c.t, decorated.ResultType, "a legacy result carries no resultType") +} + +// ─── FR-012 across connections, both eras (T020) ──────────────────────────── + +// Two independent connections, in each era, receive byte-identical tools/list +// results from the base endpoint. In the modern era a set_profile attempt on +// one of them is refused and changes nothing — there is no session to bind the +// selection to — so the facade stays identical afterwards too. +func TestStatelessList_ToolsListIdenticalAcrossConnections(t *testing.T) { + env := newProfileTestEnv(t) + url := unpinnedProxyEndpoint(t, env.proxyServer, env.proxyServer.mcpProxy.GetMCPServer()) + + forEachEra(t, func(t *testing.T, era protocolEra) { + a := newRawEraClient(t, url, era) + b := newRawEraClient(t, url, era) + + listA := a.result(string(mcp.MethodToolsList), nil) + a.requireEra(listA) + listB := b.result(string(mcp.MethodToolsList), nil) + b.requireEra(listB) + + require.Equal(t, string(listA), string(listB), + "two %s-era connections must receive a byte-identical tools/list", era.name) + require.Contains(t, string(listA), `"name":"retrieve_tools"`, "the canonical facade must be the retrieve_tools surface") + + if !a.modern() { + return + } + // Stateless era: the selection cannot be persisted, so the list on this + // connection — and the other one — is unchanged by the attempt. + attempt := a.callTool("set_profile", map[string]any{"profile": "research"}) + require.True(t, attempt.IsError, "a modern request must not be able to select a session profile") + assert.Equal(t, string(listA), string(a.result(string(mcp.MethodToolsList), nil)), + "a refused set_profile must leave this connection's tools/list unchanged") + assert.Equal(t, string(listB), string(b.result(string(mcp.MethodToolsList), nil)), + "a refused set_profile on one connection must not reach another") + }) +} + +// ─── the two list filters skip the session tier (T020 unit / T021) ────────── + +// statelessListProxy is a bare proxy over two servers and one profile, enough +// for the discovery filters to resolve a stored session selection. +func statelessListProxy(t *testing.T) *MCPProxyServer { + t.Helper() + cfg := &config.Config{ + Servers: []*config.ServerConfig{ + {Name: "research-srv", Enabled: true}, + {Name: "deploy-srv", Enabled: true}, + }, + Profiles: []config.ProfileConfig{ + {Name: "research", Servers: []string{"research-srv"}}, + }, + } + return &MCPProxyServer{config: cfg, sessionStore: NewSessionStore(zap.NewNop()), logger: zap.NewNop()} +} + +// Direct-mode tools/list is the same for a legacy session that selected a +// profile with set_profile and for a fresh one. Before this change the filter +// consulted the session tier and dropped deploy-srv's tool for the first. +func TestStatelessList_DirectToolsListIgnoresSessionSelection(t *testing.T) { + proxy := statelessListProxy(t) + researchTool := FormatDirectToolName("research-srv", "search_papers") + deployTool := FormatDirectToolName("deploy-srv", "deploy_app") + publishPermsCatalog(proxy, map[string]string{ + researchTool: auth.PermRead, + deployTool: auth.PermRead, + }) + tools := []mcp.Tool{{Name: researchTool}, {Name: deployTool}} + + proxy.sessionStore.SetActiveProfile("sess-selected", "research") + selected := sessionCtx(adminCtx(), "sess-selected") + fresh := sessionCtx(adminCtx(), "sess-fresh") + + withSelection := directToolNamesForTest(proxy.filterDirectModeToolsForAuth(selected, tools)) + withoutSelection := directToolNamesForTest(proxy.filterDirectModeToolsForAuth(fresh, tools)) + + assert.Equal(t, []string{researchTool, deployTool}, withoutSelection, "a fresh session lists both servers' tools") + assert.Equal(t, withoutSelection, withSelection, + "FR-012: tools/list must not vary with a session's set_profile selection") + + // The call-time re-evaluation of the same filter chain is an execution + // gate, not a list, and keeps honouring the selection: the deploy tool is + // withheld (unregistered-name -32602) for the session that chose research. + callTime := withDirectRequestKindBox(selected) + setDirectRequestKind(callTime, directRequestKindCall) + assert.Equal(t, []string{researchTool}, directToolNamesForTest(proxy.filterDirectModeToolsForAuth(callTime, tools)), + "the session selection still scopes execution") +} + +// prompts/list is the same for a legacy session that selected a profile and +// for a fresh one — the pre-existing FR-012 violation research D1 named: on +// main, a base-/mcp session that ran set_profile got a prompts/list that varied +// by connection. +func TestStatelessList_PromptsListIgnoresSessionSelection(t *testing.T) { + proxy := statelessListProxy(t) + prompts := []mcp.Prompt{ + {Name: FormatDirectPromptName("research-srv", "summarize"), Meta: stampAggregatedPromptServer(nil, "research-srv")}, + {Name: FormatDirectPromptName("deploy-srv", "plan_rollout"), Meta: stampAggregatedPromptServer(nil, "deploy-srv")}, + } + + proxy.sessionStore.SetActiveProfile("sess-selected", "research") + selected := sessionCtx(adminCtx(), "sess-selected") + fresh := sessionCtx(adminCtx(), "sess-fresh") + + withSelection := promptNamesForTest(proxy.filterAggregatedPromptsForAuth(selected, prompts)) + withoutSelection := promptNamesForTest(proxy.filterAggregatedPromptsForAuth(fresh, prompts)) + + assert.Len(t, withoutSelection, 2, "a fresh session lists both servers' prompts") + assert.Equal(t, withoutSelection, withSelection, + "FR-012: prompts/list must not vary with a session's set_profile selection") + + // prompts/get runs the handler-side gate (Spec 104 FR-016g), which is an + // execution gate and keeps honouring the selection. + assert.NoError(t, proxy.authorizeAggregatedPromptServer(selected, "research-srv")) + assert.ErrorIs(t, proxy.authorizeAggregatedPromptServer(selected, "deploy-srv"), errPromptNotFound, + "the session selection still scopes prompts/get") +} From f3024f2047b1b7c2e5ba8f623ae37c308f65705a Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 12:52:07 +0300 Subject: [PATCH 02/19] feat(server): list-only profile resolution and era-gated session tier (T023-T025) Spec 058 US3. profileSessionIDFromContext gates the resolver's session tier on !server.IsModernRequest(ctx) (stdio reports the constant "stdio" in both eras) and on a list-only marker; resolveActiveProfileForList skips the tier so the direct-tools and aggregated-prompts list filters no longer vary per connection (FR-012). Both the v2 precedence and the Spec 108 v3 resolution read the tier through the one gate, and a list-only resolution records nothing on the session. --- internal/server/mcp_direct_scope.go | 18 +++++-- internal/server/profile_resolver.go | 67 +++++++++++++++++++++++--- internal/server/profile_resolver_v3.go | 6 ++- 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/internal/server/mcp_direct_scope.go b/internal/server/mcp_direct_scope.go index a47381a1c..3a51ede02 100644 --- a/internal/server/mcp_direct_scope.go +++ b/internal/server/mcp_direct_scope.go @@ -362,8 +362,16 @@ func (p *MCPProxyServer) filterDirectModeToolsForAuth(ctx context.Context, tools } authCtx := auth.AuthContextFromContext(ctx) - profileName, profileScope, profileIdx := p.resolveActiveProfileWithIndex(ctx) - profileResolution := p.ResolveProfileV3(ctx, profileIdx) + // Spec 058 US3 (FR-012): at tools/list the session tier is skipped, so the + // list never varies with a connection's set_profile selection. The + // call-time re-evaluation of this same chain is an execution gate and + // keeps the full precedence, exactly like the handler it admits to. + resolveCtx := ctx + if !isDirectCallTimeRequest(ctx) { + resolveCtx = listOnlyProfileResolution(ctx) + } + profileName, profileScope, profileIdx := p.resolveActiveProfileWithIndex(resolveCtx) + profileResolution := p.ResolveProfileV3(resolveCtx, profileIdx) if profileResolution.Scope != nil { profileName = profileResolution.Name profileScope = profileResolution.Scope @@ -563,7 +571,11 @@ func (p *MCPProxyServer) filterAggregatedPromptsForAuth(ctx context.Context, pro } authCtx := auth.AuthContextFromContext(ctx) - _, profileScope := p.resolveActiveProfile(ctx) + // Spec 058 US3 (FR-012): the list-only resolver skips the session tier, so + // prompts/list no longer varies with a connection's set_profile selection + // (the pre-existing violation research D1 named). prompts/get still runs + // authorizeAggregatedPromptServer with the full precedence. + _, profileScope, _ := p.resolveActiveProfileForList(ctx) isScopeRestricted := isScopeRestrictedCaller(authCtx) enforce := isScopeRestricted || profileScope != nil allowed := promptServerAllowed(authCtx, profileScope) diff --git a/internal/server/profile_resolver.go b/internal/server/profile_resolver.go index 9a9f97a24..e586c9081 100644 --- a/internal/server/profile_resolver.go +++ b/internal/server/profile_resolver.go @@ -11,11 +11,13 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" ) -// sessionIDFromContext returns the stable mcp-go per-session id available at -// tool-call time, or "" when no session is bound to the request. This is the -// id confirmed (Profiles v2 T2 first subtask) to be exposed by mcp-go via -// server.ClientSessionFromContext for both streamable-HTTP and SSE transports; -// no synthetic fallback is required. +// sessionIDFromContext returns the mcp-go session id bound to the request, or +// "" when none is. Only a LEGACY (2025-11-25 and earlier) streamable-HTTP or +// SSE connection binds a stable per-session id. A 2026-07-28 request is served +// by an ephemeral session whose id is "" (mcp-go server/streamable_http.go), and +// the stdio transport reports the constant "stdio" in both eras — so this id is +// attribution-grade only. Nothing that persists per-connection state may key +// on it without the era gate in profileSessionIDFromContext (Spec 058 US3). func sessionIDFromContext(ctx context.Context) string { if sess := mcpserver.ClientSessionFromContext(ctx); sess != nil { return sess.SessionID() @@ -23,6 +25,45 @@ func sessionIDFromContext(ctx context.Context) string { return "" } +// listOnlyProfileResolutionKey marks a context whose profile resolution must +// skip the session tier (and record nothing on the session): the `*/list` +// filters use it, because FR-012 (Spec 058) forbids a list result from varying +// with a connection's set_profile selection. Carried on the context rather +// than as a separate resolver because BOTH resolvers — the v2 precedence in +// resolveActiveProfileWithSourceFromIndex and the Spec 108 resolveProfileV3 — +// read the session tier through profileSessionIDFromContext, so one marker +// gates both and the two cannot drift. +type listOnlyProfileResolutionKey struct{} + +// listOnlyProfileResolution returns ctx marked for list-only resolution. It +// also carries noResolutionRecordKey: a list-time resolution that skipped the +// session tier is not the session's effective profile and must not overwrite +// the record profile edits use to find the sessions they notify (Spec 108 +// FR-028). +func listOnlyProfileResolution(ctx context.Context) context.Context { + ctx = context.WithValue(ctx, listOnlyProfileResolutionKey{}, struct{}{}) + return context.WithValue(ctx, noResolutionRecordKey{}, struct{}{}) +} + +// profileSessionIDFromContext is the session id the profile resolver's session +// tier may key on, or "" when that tier must not apply (Spec 058 US3): +// +// - a modern (2026-07-28) request binds no session, whatever the transport +// reports — stdio's constant "stdio" included (FR-011; research D1 graft 1), +// so a modern request can neither read nor persist a selection; +// - a list-only resolution (listOnlyProfileResolution) skips the tier so +// `*/list` cannot vary per connection (FR-012; research D1 graft 2). +// +// Execution-time callers (retrieve_tools, call_tool_*, code_execution, +// prompts/get) keep the tier for legacy sessions, which is what the +// set_profile deprecation window promises them. +func profileSessionIDFromContext(ctx context.Context) string { + if mcpserver.IsModernRequest(ctx) || ctx.Value(listOnlyProfileResolutionKey{}) != nil { + return "" + } + return sessionIDFromContext(ctx) +} + // profilePinFromContext returns the agent-token profile_pin bound to the // request, or "" when the token is unpinned. The pin is set on the AuthContext // by the MCP auth middleware after validating an agent token (Profiles v2 T3, @@ -189,6 +230,18 @@ func (p *MCPProxyServer) resolveActiveProfileWithIndex(ctx context.Context) (str return name, scope, idx } +// resolveActiveProfileForList is resolveActiveProfileWithIndex for a `*/list` +// response (Spec 058 US3, FR-012): identical precedence, except that the +// session tier — the legacy set_profile selection, the one piece of connection +// state mcpproxy holds — is skipped, so two connections receive the same list +// whatever either of them selected. Pin, URL and the Spec 108 binding/anonymous +// tiers are request-carried identity and still apply. Callers that also run +// the Spec 108 resolution for the same list must pass listOnlyProfileResolution +// (ctx) to ResolveProfileV3 so both resolvers skip the same tier. +func (p *MCPProxyServer) resolveActiveProfileForList(ctx context.Context) (string, *profile.ProfileScope, *profileIndex) { + return p.resolveActiveProfileWithIndex(listOnlyProfileResolution(ctx)) +} + // resolveActiveProfileWithSource is resolveActiveProfileWithIndex, additionally // reporting which resolution tier produced the (name, scope) pair (Spec 108 // FR-011): retrieve_tools needs this to decide whether its `profile` field may @@ -292,8 +345,10 @@ func (p *MCPProxyServer) resolveActiveProfileWithSourceFromIndex(ctx context.Con } // 3. Session selection set via the set_profile tool on the base /mcp endpoint. + // Legacy sessions only, and never for a `*/list` resolution — see + // profileSessionIDFromContext (Spec 058 US3). if p.sessionStore != nil { - if sid := sessionIDFromContext(ctx); sid != "" { + if sid := profileSessionIDFromContext(ctx); sid != "" { if name := p.sessionStore.GetActiveProfile(sid); name != "" { if scope := profileScopeFromIndex(idx, name); scope != nil { return name, scope, profile.SourceSession diff --git a/internal/server/profile_resolver_v3.go b/internal/server/profile_resolver_v3.go index fabda7f1c..992db6b05 100644 --- a/internal/server/profile_resolver_v3.go +++ b/internal/server/profile_resolver_v3.go @@ -213,9 +213,11 @@ func (p *MCPProxyServer) resolveProfileV3(ctx context.Context, idx *profileIndex } // Tier 3: session (set_profile), re-validated against the CURRENT base - // on every request; a selection no longer permitted is cleared. + // on every request; a selection no longer permitted is cleared. Legacy + // sessions only, never for a list-only resolution — see + // profileSessionIDFromContext (Spec 058 US3). if p.sessionStore != nil { - if sid := sessionIDFromContext(ctx); sid != "" { + if sid := profileSessionIDFromContext(ctx); sid != "" { if sel := p.sessionStore.GetActiveProfile(sid); sel != "" { if base == "" || admittedBySwitchableTo(basePolicy, base, sel) { if scope := profileScopeFromIndex(idx, sel); scope != nil { From 19f82552df1b06cc1ea21894f26758e3ee8da947 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 12:54:25 +0300 Subject: [PATCH 03/19] feat(server): set_profile validates the slug before the session guard and guides modern requests (T026) Spec 058 US3. An unknown slug is refused as unknown in both eras; a 2026-07-28 request (gated on the era, not the transport's session id, which stdio reports as the constant "stdio") is refused with machine-actionable guidance naming /mcp/p/, and nothing is stored. --- internal/server/profile_era_test.go | 52 +++++++++++++++++++++++++++++ internal/server/profile_tool.go | 36 +++++++++++++++++--- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/internal/server/profile_era_test.go b/internal/server/profile_era_test.go index b2aa7f6a2..2b1b76190 100644 --- a/internal/server/profile_era_test.go +++ b/internal/server/profile_era_test.go @@ -212,3 +212,55 @@ func TestProfileEra_ModernStdioRequestCannotPersistSelection(t *testing.T) { "FR-011: a modern request must not persist per-connection state, despite stdio's non-empty session id") }) } + +// ─── set_profile on a modern request (T026) ───────────────────────────────── + +func setProfileRequest(slug string) mcp.CallToolRequest { + req := mcp.CallToolRequest{} + req.Params.Name = "set_profile" + req.Params.Arguments = map[string]any{"profile": slug} + return req +} + +// On a modern request set_profile validates the slug FIRST — an unknown slug +// gets the same refusal in both eras — and then, instead of binding a +// selection it has no session for, answers machine-actionable guidance that +// names the per-request form `/mcp/p/`. Nothing is stored. +func TestProfileEra_SetProfileModernRequestGuidance(t *testing.T) { + proxy := profileEraProxy(t) + // The stdio session id is the hard case: non-empty in both eras. + modern := modernRequestCtx(sessionCtx(adminCtx(), "stdio")) + + t.Run("unknown slug is refused as unknown, not as sessionless", func(t *testing.T) { + result, err := proxy.handleSetProfile(modern, setProfileRequest("nonexistent")) + require.NoError(t, err) + require.True(t, result.IsError) + text := extractText(result) + assert.Contains(t, text, "unknown profile 'nonexistent'", "slug validation runs before the session guard: %s", text) + assert.NotContains(t, text, "/mcp/p/", "an unknown slug must not be handed a URL it cannot use: %s", text) + }) + + t.Run("known slug is refused with the /mcp/p/ form", func(t *testing.T) { + result, err := proxy.handleSetProfile(modern, setProfileRequest("research")) + require.NoError(t, err) + require.True(t, result.IsError, "a modern request cannot bind a session selection") + text := extractText(result) + assert.Contains(t, text, "/mcp/p/research", "guidance must name the exact per-request endpoint: %s", text) + assert.Contains(t, text, mcp.LATEST_PROTOCOL_VERSION, "guidance must say which protocol revision has no session: %s", text) + assert.Equal(t, "", proxy.sessionStore.GetActiveProfile("stdio"), "nothing may be stored for a modern request") + }) + + t.Run("clearing is refused with the generic form", func(t *testing.T) { + result, err := proxy.handleSetProfile(modern, setProfileRequest("")) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, extractText(result), "/mcp/p/") + }) + + t.Run("legacy request on the same session still selects", func(t *testing.T) { + result, err := proxy.handleSetProfile(sessionCtx(adminCtx(), "stdio"), setProfileRequest("research")) + require.NoError(t, err) + require.False(t, result.IsError, "%s", extractText(result)) + assert.Equal(t, "research", proxy.sessionStore.GetActiveProfile("stdio")) + }) +} diff --git a/internal/server/profile_tool.go b/internal/server/profile_tool.go index f3834b0b8..83e02ec4e 100644 --- a/internal/server/profile_tool.go +++ b/internal/server/profile_tool.go @@ -89,10 +89,10 @@ func setProfileRefusal(ctx context.Context, slug string) *mcp.CallToolResult { func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { slug := strings.TrimSpace(request.GetString("profile", "")) - sessionID := sessionIDFromContext(ctx) - if sessionID == "" { - return mcp.NewToolResultError("set_profile requires an active MCP session; no session id is bound to this request"), nil - } + // The session guard runs AFTER slug validation below (Spec 058 US3, T026): + // an unknown slug is refused as unknown in both eras, so a modern client + // is not handed a /mcp/p/ URL for a profile that does not exist, and + // the refusal wording never reveals which guard fired first. // One (index, snapshot) pair for the whole call: admission, the stored // selection's server list and the effective scope all read cfg, the @@ -154,6 +154,19 @@ func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallT } } + // Spec 058 US3 (FR-011): a 2026-07-28 request has no session to bind a + // selection to. The transport's session id is no guide — it is "" over + // Streamable HTTP but the constant "stdio" on stdio — so the gate is the + // era itself, and the answer names the per-request form the caller can + // use instead. Nothing is stored. + if mcpserver.IsModernRequest(ctx) { + return mcp.NewToolResultError(setProfileModernGuidance(slug)), nil + } + sessionID := sessionIDFromContext(ctx) + if sessionID == "" { + return mcp.NewToolResultError("set_profile requires an active MCP session; no session id is bound to this request"), nil + } + p.sessionStore.SetActiveProfile(sessionID, slug) if slug == "" && anonymousBindingGuard { // Clearing is always admitted (FR-018), but the guarded anonymous @@ -208,6 +221,21 @@ func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallT return setProfileResult(slug, profiles.EffectiveServersFor(effectiveProfileName, allowed)) } +// setProfileModernGuidance is the set_profile refusal for a request in the +// 2026-07-28 era (Spec 058 US3). It is deliberately machine-actionable: it +// names the exact per-request endpoint for the validated slug, so an agent +// can act on it without a human translating "no session" into a URL. slug is +// already validated (or empty for a clear) when this is rendered. +func setProfileModernGuidance(slug string) string { + target := "/mcp/p/" + if slug != "" { + target = "/mcp/p/" + slug + } + return fmt.Sprintf("set_profile cannot persist a selection on a %s request: that protocol revision has no session to bind it to. "+ + "Select the profile per request instead by connecting to %s (the URL form applies to every request on that endpoint).", + mcp.LATEST_PROTOCOL_VERSION, target) +} + // profileServersIn renders the pre-105 set_profile server list for a stored // selection: the named profile's effective servers in profile-declared order // (duplicates kept, exactly as EffectiveServers returns them), every From 80fb94e79e98016d9aff40d9cd46c9d779fe5f6a Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 12:55:08 +0300 Subject: [PATCH 04/19] test(server): split set_profile integration tests into legacy and modern era variants (T027-T028) TestProfile_SetProfileSessionScoped keeps its name and pins the legacy era explicitly; TestProfile_SetProfileModernRequestIsStateless covers the 2026-07-28 era over the unpinned era harness; TestProfile_SetProfileUnknown asserts the unknown-slug message in both eras. --- internal/server/profile_integration_test.go | 92 ++++++++++++++++++--- 1 file changed, 79 insertions(+), 13 deletions(-) diff --git a/internal/server/profile_integration_test.go b/internal/server/profile_integration_test.go index 6bbda5e70..1ed9e6e8f 100644 --- a/internal/server/profile_integration_test.go +++ b/internal/server/profile_integration_test.go @@ -549,17 +549,42 @@ func (e *profileTestEnv) callSetProfile(ctx context.Context, c *client.Client, s return resp.ActiveProfile, resp.Servers, false, text } -// TestProfile_SetProfileSessionScoped exercises the core T2 flow: a base /mcp -// session selects a profile via set_profile, retrieve_tools is then scoped to -// that profile (no re-index), switching swaps the scope, and clearing restores -// all servers — all within one MCP session. +// clientAtEra is clientAt pinned to one protocol era (Spec 058 era harness, +// protocol_era_test.go). Pinning is what makes the era an explicit input: an +// unpinned v1.0.0 client negotiates DOWN under the FR-028 pin, so a "modern" +// test built on clientAt would silently run on the legacy path. +func (e *profileTestEnv) clientAtEra(url string, era protocolEra) *client.Client { + e.t.Helper() + tr, err := transport.NewStreamableHTTP(url) + require.NoError(e.t, err) + return client.NewClient(tr, era.options...) +} + +// eraEndpoint serves the default /mcp server over an UNPINNED transport behind +// the production auth middleware, so the modern era is reachable before T041 +// lifts the pin on the listening endpoint (see unpinnedProxyEndpoint). +func (e *profileTestEnv) eraEndpoint() string { + e.t.Helper() + return unpinnedProxyEndpoint(e.t, e.proxyServer, e.proxyServer.mcpProxy.GetMCPServer()) +} + +// TestProfile_SetProfileSessionScoped exercises the core T2 flow on a LEGACY +// session: a base /mcp session selects a profile via set_profile, +// retrieve_tools is then scoped to that profile (no re-index), switching swaps +// the scope, and clearing restores all servers — all within one MCP session. +// +// Spec 058 US3 (T027) split this into era variants. This one keeps the +// original name because the spec names it as the Spec-057 reconciliation gate; +// the modern variant is TestProfile_SetProfileModernRequestIsStateless. func TestProfile_SetProfileSessionScoped(t *testing.T) { env := newProfileTestEnv(t) ctx := context.Background() - // Single base /mcp client → one stable MCP session across calls. - c := env.CreateProxyClient() + // Single base /mcp client, explicitly on the legacy era → one stable MCP + // session across calls. + c := env.clientAtEra(env.proxyAddr, legacyEra()) env.initClient(c) + requireEra(t, c, legacyEra()) // Switch to "research". active, servers, isErr, text := env.callSetProfile(ctx, c, "research") @@ -604,18 +629,59 @@ func TestProfile_SetProfileSessionScoped(t *testing.T) { "after clearing the profile, retrieve_tools should see both servers; got: %v", names) } -// TestProfile_SetProfileUnknown verifies set_profile rejects an unknown slug and -// lists the available profiles. -func TestProfile_SetProfileUnknown(t *testing.T) { +// TestProfile_SetProfileModernRequestIsStateless is the 2026-07-28 variant of +// TestProfile_SetProfileSessionScoped (Spec 058 US3, T027). That era binds no +// session, so set_profile cannot persist a selection: it is refused with +// guidance naming /mcp/p/, retrieve_tools stays unscoped, and nothing +// is stored. +func TestProfile_SetProfileModernRequestIsStateless(t *testing.T) { env := newProfileTestEnv(t) ctx := context.Background() - c := env.CreateProxyClient() + c := env.clientAtEra(env.eraEndpoint(), modernEra()) env.initClient(c) + requireEra(t, c, modernEra()) - _, _, isErr, text := env.callSetProfile(ctx, c, "nonexistent") - assert.True(t, isErr, "set_profile with an unknown slug must error") - assert.Contains(t, text, "unknown profile 'nonexistent'", "error must name the bad slug: %s", text) + _, _, isErr, text := env.callSetProfile(ctx, c, "research") + require.True(t, isErr, "a modern request must not be able to select a session profile: %s", text) + assert.Contains(t, text, "/mcp/p/research", "the refusal must name the per-request form: %s", text) + + names := env.retrieveTools(ctx, c, "search deploy papers rollback app") + hasResearch, hasDeploy := false, false + for _, n := range names { + if strings.Contains(n, "search_papers") || strings.Contains(n, "fetch_article") { + hasResearch = true + } + if strings.Contains(n, "deploy_app") || strings.Contains(n, "rollback") { + hasDeploy = true + } + } + assert.True(t, hasResearch && hasDeploy, + "after a refused set_profile, retrieve_tools must still see both servers; got: %v", names) + + _, _, isErr, text = env.callSetProfile(ctx, c, "") + assert.True(t, isErr, "clearing is refused too — there is no session selection to clear: %s", text) +} + +// TestProfile_SetProfileUnknown verifies set_profile rejects an unknown slug +// with the same message in both eras (Spec 058 US3, T028): slug validation runs +// before the session guard, so a modern request is told the slug is unknown, +// not handed a /mcp/p/ URL for a profile that does not exist. +func TestProfile_SetProfileUnknown(t *testing.T) { + env := newProfileTestEnv(t) + ctx := context.Background() + url := env.eraEndpoint() + + forEachEra(t, func(t *testing.T, era protocolEra) { + c := env.clientAtEra(url, era) + env.initClient(c) + requireEra(t, c, era) + + _, _, isErr, text := env.callSetProfile(ctx, c, "nonexistent") + assert.True(t, isErr, "set_profile with an unknown slug must error") + assert.Contains(t, text, "unknown profile 'nonexistent'", "error must name the bad slug: %s", text) + assert.NotContains(t, text, "/mcp/p/", "an unknown slug must not be handed a URL: %s", text) + }) } // --------------------------------------------------------------------------- From e9faf0c6231c86d5fc9ad687c74d6e05251a63e3 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 12:58:29 +0300 Subject: [PATCH 05/19] test(server): cross-client cancellation isolation under the modern era (T029) Documents mcp-go's inflightKey collision for 2026-07-28 requests (every modern request runs under session id "", so two clients using JSON-RPC id 1 share key ":1") and asserts mcpproxy's guard isolates them while a legacy session can still cancel its own request. --- internal/server/cancel_isolation_test.go | 248 +++++++++++++++++++++++ internal/server/stateless_list_test.go | 50 ++++- 2 files changed, 289 insertions(+), 9 deletions(-) create mode 100644 internal/server/cancel_isolation_test.go diff --git a/internal/server/cancel_isolation_test.go b/internal/server/cancel_isolation_test.go new file mode 100644 index 000000000..0aef74467 --- /dev/null +++ b/internal/server/cancel_isolation_test.go @@ -0,0 +1,248 @@ +package server + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + mcpserver "github.com/mark3labs/mcp-go/server" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Spec 058 US3, plan risk R1 (security). mcp-go keys in-flight request cancels +// as ":" (server/server.go inflightKey) and registers +// every request unconditionally (request_handler.go). A 2026-07-28 request is +// served by an ephemeral session whose id is "", so two modern clients that +// both use JSON-RPC id 1 — the common case — share the key ":1", and a +// `notifications/cancelled` from one cancels the other's in-flight call. +// +// mcpproxy's guard (modernCancellationGuard, mcp.go) sits in the HTTP chain: +// a modern POST carrying a cancellation notification is acknowledged (202) and +// dropped. Under the stateless protocol there is no session to correlate it +// with anyway; a client that wants to abandon a call closes its connection, +// which cancels the handler's context through the transport. +// +// Test names avoid the substring "MCP" (release-qa-gate.yml skips on it). + +// cancelProbe is a bare MCP server with one tool that blocks until released or +// cancelled, so a test can observe exactly which of the two happened. +type cancelProbe struct { + srv *mcpserver.MCPServer + started chan struct{} + release chan struct{} + once sync.Once +} + +func newCancelProbe() *cancelProbe { + p := &cancelProbe{ + srv: mcpserver.NewMCPServer("cancel-probe", "1.0.0"), + started: make(chan struct{}), + release: make(chan struct{}), + } + p.srv.AddTool( + mcp.NewTool("slow", mcp.WithDescription("blocks until released or cancelled")), + func(ctx context.Context, _ mcp.CallToolRequest) (*mcp.CallToolResult, error) { + p.once.Do(func() { close(p.started) }) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-p.release: + return mcp.NewToolResultText("done"), nil + } + }, + ) + return p +} + +// serve mounts the probe over an UNPINNED Streamable HTTP transport, optionally +// behind a middleware, and returns the endpoint URL. +func (p *cancelProbe) serve(t *testing.T, wrap func(http.Handler) http.Handler) string { + t.Helper() + var h http.Handler = mcpserver.NewStreamableHTTPServer(p.srv) + if wrap != nil { + h = wrap(h) + } + httpServer := httptest.NewServer(h) + t.Cleanup(httpServer.Close) + return httpServer.URL +} + +// asyncResult is the outcome of a tools/call issued on its own goroutine. +type asyncResult struct { + status int + body []byte + err error +} + +// callSlowAsync issues tools/call slow with the given JSON-RPC id and returns +// the channel the outcome lands on. It builds the request through the client's +// own helpers but asserts nothing on the goroutine. +func callSlowAsync(c *rawEraClient, id int) <-chan asyncResult { + out := make(chan asyncResult, 1) + req, err := c.buildRequest(string(mcp.MethodToolsCall), map[string]any{"name": "slow", "arguments": map[string]any{}}, id) + if err != nil { + out <- asyncResult{err: err} + return out + } + go func() { + status, body, err := c.do(req) + out <- asyncResult{status: status, body: body, err: err} + }() + return out +} + +// cancelledNotification is the raw notifications/cancelled a client posts for +// requestId id. +func cancelledNotificationParams(id int) map[string]any { + return map[string]any{"requestId": id, "reason": "test"} +} + +func awaitStarted(t *testing.T, p *cancelProbe) { + t.Helper() + select { + case <-p.started: + case <-time.After(10 * time.Second): + t.Fatal("the slow tool never started") + } +} + +func awaitOutcome(t *testing.T, ch <-chan asyncResult) asyncResult { + t.Helper() + select { + case r := <-ch: + require.NoError(t, r.err) + return r + case <-time.After(15 * time.Second): + t.Fatal("the in-flight call neither completed nor was cancelled within 15s") + return asyncResult{} + } +} + +// callSucceeded reports whether the raw tools/call response is the released +// tool's success result. +func callSucceeded(body []byte) bool { + var envelope struct { + Result *struct { + IsError bool `json:"isError"` + Content []json.RawMessage `json:"content"` + } `json:"result"` + Error json.RawMessage `json:"error"` + } + if json.Unmarshal(body, &envelope) != nil || envelope.Result == nil || envelope.Error != nil { + return false + } + return !envelope.Result.IsError && bytes.Contains(body, []byte(`"done"`)) +} + +// Documents the library defect the guard exists for, so the guard can be +// retired the day this test FAILS: against an unguarded mcp-go v1.0.0 +// transport, client B's notifications/cancelled for id 1 cancels client A's +// in-flight call with id 1. Both are modern, so both run under session id "". +func TestCancelIsolation_LibraryCollisionStillPresent(t *testing.T) { + probe := newCancelProbe() + url := probe.serve(t, nil) + era := modernEra() + + a := newRawEraClient(t, url, era) + b := newRawEraClient(t, url, era) + + inflight := callSlowAsync(a, 1) + awaitStarted(t, probe) + + resp, _ := b.send(string(mcp.MethodNotificationCancelled), cancelledNotificationParams(1), false) + require.Equal(t, http.StatusAccepted, resp.StatusCode) + + outcome := awaitOutcome(t, inflight) + if callSucceeded(outcome.body) { + t.Skip("mcp-go no longer lets one modern client cancel another's request: " + + "the upstream fix has landed — retire modernCancellationGuard and this test together") + } + // Reaching here is the defect: A's call ended without ever being released. + close(probe.release) +} + +// The guard: with mcpproxy's middleware in the chain, B's cancellation is +// acknowledged and dropped, and A's call runs to completion when released. +func TestCancelIsolation_ModernClientsCannotCancelEachOther(t *testing.T) { + probe := newCancelProbe() + url := probe.serve(t, modernCancellationGuard) + era := modernEra() + + a := newRawEraClient(t, url, era) + b := newRawEraClient(t, url, era) + + inflight := callSlowAsync(a, 1) + awaitStarted(t, probe) + + resp, body := b.send(string(mcp.MethodNotificationCancelled), cancelledNotificationParams(1), false) + assert.Equal(t, http.StatusAccepted, resp.StatusCode, "a dropped notification is still acknowledged: %s", body) + + // Give a wrongly-forwarded cancellation time to land before releasing. + select { + case r := <-inflight: + t.Fatalf("A's in-flight call ended before it was released — B's cancellation reached it: %s", r.body) + case <-time.After(300 * time.Millisecond): + } + close(probe.release) + + outcome := awaitOutcome(t, inflight) + require.Equal(t, http.StatusOK, outcome.status) + assert.True(t, callSucceeded(outcome.body), "A's call must complete with the released result, got: %s", outcome.body) +} + +// The guard is scoped to the modern era: a legacy session's own +// notifications/cancelled still cancels its own in-flight request, exactly as +// before (the session id keys it, so there is no collision to guard against). +func TestCancelIsolation_LegacySelfCancelStillWorks(t *testing.T) { + probe := newCancelProbe() + url := probe.serve(t, modernCancellationGuard) + + a := newRawEraClient(t, url, legacyEra()) + // buildRequest hands out ids explicitly, so the call takes 1 here and the + // notification refers to it. + inflight := callSlowAsync(a, 1) + awaitStarted(t, probe) + + resp, _ := a.send(string(mcp.MethodNotificationCancelled), cancelledNotificationParams(1), false) + require.Equal(t, http.StatusAccepted, resp.StatusCode) + + outcome := awaitOutcome(t, inflight) + assert.False(t, callSucceeded(outcome.body), "a legacy session must still be able to cancel its own request; got a completed result: %s", outcome.body) + close(probe.release) +} + +// A modern tools/call that is NOT a cancellation passes through the guard +// untouched: the body is re-readable downstream and the result is served. +func TestCancelIsolation_GuardPassesOrdinaryRequests(t *testing.T) { + probe := newCancelProbe() + close(probe.release) // the tool answers immediately + url := probe.serve(t, modernCancellationGuard) + + a := newRawEraClient(t, url, modernEra()) + result := a.callTool("slow", map[string]any{}) + assert.False(t, result.IsError) + require.NotEmpty(t, result.Content) + text, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Equal(t, "done", text.Text) + + // Malformed bodies are left to mcp-go's own parse error, not swallowed. + req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader([]byte("{not json"))) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + req.Header.Set(mcp.HeaderProtocolVersion, mcp.LATEST_PROTOCOL_VERSION) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + raw, _ := io.ReadAll(resp.Body) + assert.NotEqual(t, http.StatusAccepted, resp.StatusCode, "a malformed body is not a cancellation to acknowledge: %s", raw) +} diff --git a/internal/server/stateless_list_test.go b/internal/server/stateless_list_test.go index 58230555a..c861f0b1c 100644 --- a/internal/server/stateless_list_test.go +++ b/internal/server/stateless_list_test.go @@ -86,10 +86,10 @@ func newRawEraClient(t *testing.T, url string, era protocolEra) *rawEraClient { func (c *rawEraClient) modern() bool { return mcp.IsModernProtocol(c.era.version) } -// send posts one JSON-RPC message and returns the HTTP response plus the -// decoded JSON-RPC payload (SSE-framed responses are unwrapped). -func (c *rawEraClient) send(method string, params map[string]any, withID bool) (*http.Response, []byte) { - c.t.Helper() +// buildRequest renders one JSON-RPC message as an HTTP POST for this client's +// era. id <= 0 renders a notification (no id member). It asserts nothing, so +// a test can issue a request on its own goroutine. +func (c *rawEraClient) buildRequest(method string, params map[string]any, id int) (*http.Request, error) { if params == nil { params = map[string]any{} } @@ -101,15 +101,18 @@ func (c *rawEraClient) send(method string, params map[string]any, withID bool) ( } } msg := map[string]any{"jsonrpc": "2.0", "method": method, "params": params} - if withID { - msg["id"] = c.nextID - c.nextID++ + if id > 0 { + msg["id"] = id } body, err := json.Marshal(msg) - require.NoError(c.t, err) + if err != nil { + return nil, err + } req, err := http.NewRequest(http.MethodPost, c.url, bytes.NewReader(body)) - require.NoError(c.t, err) + if err != nil { + return nil, err + } req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json, text/event-stream") if c.modern() { @@ -123,7 +126,36 @@ func (c *rawEraClient) send(method string, params map[string]any, withID bool) ( } else if c.sessionID != "" { req.Header.Set(mcpserver.HeaderKeySessionID, c.sessionID) } + return req, nil +} + +// do performs req and returns the status plus the decoded JSON-RPC payload +// (SSE-framed responses are unwrapped). It asserts nothing. +func (c *rawEraClient) do(req *http.Request) (int, []byte, error) { + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Do(req) + if err != nil { + return 0, nil, err + } + defer resp.Body.Close() + raw, err := io.ReadAll(resp.Body) + if err != nil { + return resp.StatusCode, nil, err + } + return resp.StatusCode, unwrapJSONRPCBody(resp.Header.Get("Content-Type"), raw), nil +} +// send posts one JSON-RPC message and returns the HTTP response plus the +// decoded JSON-RPC payload (SSE-framed responses are unwrapped). +func (c *rawEraClient) send(method string, params map[string]any, withID bool) (*http.Response, []byte) { + c.t.Helper() + id := 0 + if withID { + id = c.nextID + c.nextID++ + } + req, err := c.buildRequest(method, params, id) + require.NoError(c.t, err) client := &http.Client{Timeout: 15 * time.Second} resp, err := client.Do(req) require.NoError(c.t, err) From 7ababe7cb0f8eb518a7521111f6b2445b17ba193 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 12:58:30 +0300 Subject: [PATCH 06/19] fix(server): drop modern clients' separately posted cancellations (Spec 058 R1, T030) A 2026-07-28 request has no session, so a notifications/cancelled posted on its own cannot be correlated with any in-flight request; mcp-go keys it as ":" and would cancel whichever client's call shares that id. The guard acknowledges (202) and drops such notifications in the /mcp* middleware chain; legacy requests pass through unchanged. --- internal/server/mcp.go | 64 +++++++++++++++++++++++++++++++++++++++ internal/server/server.go | 4 +++ 2 files changed, 68 insertions(+) diff --git a/internal/server/mcp.go b/internal/server/mcp.go index beb7bfbc7..ca0417131 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -1,9 +1,12 @@ package server import ( + "bytes" "context" "encoding/json" "fmt" + "io" + "net/http" "os/exec" "regexp" "sort" @@ -405,6 +408,67 @@ func (p *MCPProxyServer) currentTruncator() *truncate.Truncator { return p.truncatorFn() } +// modernCancellationGuard closes the cross-client cancellation hole the +// 2026-07-28 era opens in mcp-go (Spec 058 US3, plan risk R1; upstream issue +// filed from this task). +// +// mcp-go keys in-flight request cancels as ":" +// (server/server.go inflightKey) and registers every request under that key. +// A modern request is served by an ephemeral session whose id is "", so every +// modern client shares the key space ":": two clients that both use +// JSON-RPC id 1 — the common case — collide, and a `notifications/cancelled` +// from one cancels the other's in-flight call. The notification path runs +// before any hook (request_handler.go: `if baseMessage.ID == nil { ... }`), so +// the only place mcpproxy can intervene is the HTTP chain. +// +// The guard acknowledges (202, exactly as mcp-go answers a notification) and +// drops a cancellation notification carried by a MODERN POST. Nothing is lost: +// that era has no session to correlate a separately posted notification with, +// so it could never have targeted the right request except by accident, and a +// client that wants to abandon a call closes its connection, which cancels the +// handler's context through the transport. Legacy requests, which the session +// id keys correctly, pass through untouched. +// +// The era is read from the Mcp-Protocol-Version header: a modern POST must +// carry it (mcp-go's validateModernRequest refuses one without it), and a +// legacy POST never carries a modern value. Only then is the body buffered and +// inspected — a cancellation notification is a notification with that method +// — and re-attached for the transport, which reads it in full anyway. +func modernCancellationGuard(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || !mcp.IsModernProtocol(r.Header.Get(mcp.HeaderProtocolVersion)) { + next.ServeHTTP(w, r) + return + } + body, err := io.ReadAll(r.Body) + _ = r.Body.Close() + if err != nil { + http.Error(w, "failed to read request body", http.StatusBadRequest) + return + } + r.Body = io.NopCloser(bytes.NewReader(body)) + if isCancelledNotification(body) { + w.WriteHeader(http.StatusAccepted) + return + } + next.ServeHTTP(w, r) + }) +} + +// isCancelledNotification reports whether body is a JSON-RPC +// notifications/cancelled notification (no id, or a null one — mcp-go treats +// both as a notification). +func isCancelledNotification(body []byte) bool { + var msg struct { + Method string `json:"method"` + ID json.RawMessage `json:"id"` + } + if json.Unmarshal(body, &msg) != nil || msg.Method != string(mcp.MethodNotificationCancelled) { + return false + } + return len(msg.ID) == 0 || bytes.Equal(msg.ID, []byte("null")) +} + // NewMCPProxyServer creates a new MCP proxy server func NewMCPProxyServer( storage *storage.Manager, diff --git a/internal/server/server.go b/internal/server/server.go index 032028053..a4fd83dc4 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -500,6 +500,10 @@ func (s *Server) mcpAuthMiddleware(next http.Handler) http.Handler { // Harmless on every endpoint but /mcp/all, whose filters are the only // consumers. next = directRequestKindMiddleware(next) + // Spec 058 US3 (R1): drop a modern client's separately posted + // notifications/cancelled before mcp-go can match it against another + // client's in-flight request — every modern request shares session id "". + next = modernCancellationGuard(next) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { token := httpapi.ExtractToken(r) if token == "" { From 934c68bb33f7cae5ed25606a30bcab3571849288 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:00:56 +0300 Subject: [PATCH 07/19] docs(spec): draft the mcp-go inflight-cancellation issue (T030) mark3labs/mcp-go restricts interactions to collaborators, so the report is kept in the spec folder until it can be filed. --- internal/server/mcp.go | 6 ++- .../mcp-go-inflight-cancel-collision.md | 47 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 specs/058-mcp-2026-upgrade/upstream-issues/mcp-go-inflight-cancel-collision.md diff --git a/internal/server/mcp.go b/internal/server/mcp.go index ca0417131..01146a8bd 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -409,8 +409,10 @@ func (p *MCPProxyServer) currentTruncator() *truncate.Truncator { } // modernCancellationGuard closes the cross-client cancellation hole the -// 2026-07-28 era opens in mcp-go (Spec 058 US3, plan risk R1; upstream issue -// filed from this task). +// 2026-07-28 era opens in mcp-go (Spec 058 US3, plan risk R1). The upstream +// report is drafted at specs/058-mcp-2026-upgrade/upstream-issues/ +// mcp-go-inflight-cancel-collision.md — mark3labs/mcp-go restricted +// interactions to collaborators when this landed, so it could not be filed. // // mcp-go keys in-flight request cancels as ":" // (server/server.go inflightKey) and registers every request under that key. diff --git a/specs/058-mcp-2026-upgrade/upstream-issues/mcp-go-inflight-cancel-collision.md b/specs/058-mcp-2026-upgrade/upstream-issues/mcp-go-inflight-cancel-collision.md new file mode 100644 index 000000000..5b4ed3512 --- /dev/null +++ b/specs/058-mcp-2026-upgrade/upstream-issues/mcp-go-inflight-cancel-collision.md @@ -0,0 +1,47 @@ +# mcp-go: modern requests share the inflight cancellation key `":"` + +**Target**: https://github.com/mark3labs/mcp-go · **Status**: DRAFT — not filed. +`gh issue create` on 2026-10-20 was refused: "Interactions on this repository have +been restricted to collaborators only." File it when the restriction lifts +(GitHub interaction limits expire after at most six months), or hand it to a +collaborator. mcpproxy's mitigation is `modernCancellationGuard` +(`internal/server/mcp.go`); `TestCancelIsolation_LibraryCollisionStillPresent` +skips once the library no longer collides, which is the signal to retire both. + +**Suggested title**: Modern (2026-07-28) requests share the inflight cancellation +key `":"`: one client can cancel another's in-flight request + +--- + +## Summary + +Under protocol version `2026-07-28` every request is served by an ephemeral session whose `SessionID()` is `""`. `inflightKey` (server/server.go) keys in-flight cancellations as `":"`, and `HandleMessage` registers every request under that key unconditionally (server/request_handler.go). So **all modern clients share the key space `":"`**: two clients that both use JSON-RPC id `1` (the common case for a fresh client) collide, and a `notifications/cancelled` posted by one **cancels the other client's in-flight request**. + +The notification is handled in `handleNotification` before any hook runs (`if baseMessage.ID == nil { s.handleNotification(...) }`), so an embedding server cannot intercept it through `Hooks`. + +Affects v1.0.0; `inflightKey` is unchanged on `main` (v1.3.0). + +## Reproduction + +Against an **unpinned** `NewStreamableHTTPServer(srv)` (default options, so `2026-07-28` is served) with one tool whose handler blocks until released or `ctx.Done()`: + +1. Client A — modern request (`Mcp-Protocol-Version: 2026-07-28`, `_meta` with `io.modelcontextprotocol/protocolVersion` and `clientCapabilities`) — POSTs `tools/call` for the blocking tool with `"id": 1`. The handler starts. +2. Client B — a different HTTP client, also modern — POSTs `{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":1}}`. +3. Client A's handler context is cancelled; its call returns `context canceled` instead of the tool result. + +Observed with mcp-go v1.0.0. A Go test reproducing it (and skipping once the library no longer collides) is `TestCancelIsolation_LibraryCollisionStillPresent` in smart-mcp-proxy/mcpproxy-go, `internal/server/cancel_isolation_test.go`. + +## Expected + +A cancellation notification must only ever cancel a request from the same client. In the legacy era the session id guarantees that; in the modern era nothing does. + +## Possible fixes + +- Key in-flight cancels for an ephemeral (`SessionID() == ""`) session by a per-request nonce rather than `":"`, so a separately posted notification can never match another client's request. Since a modern notification cannot carry that nonce, this effectively means a separately posted `notifications/cancelled` cancels nothing in the modern era — which matches the stateless protocol: the client abandons a call by closing the connection, and the transport cancels the handler's context. +- Equivalently: in `handlePost`, do not forward `notifications/cancelled` to `HandleMessage` when `era.modern` (acknowledge with 202 as today). + +Either way, the collision across clients should not be possible by default. + +## Context + +Found while preparing mcpproxy to serve `2026-07-28` (smart-mcp-proxy/mcpproxy-go spec 058, risk R1). mcpproxy currently mitigates it with an HTTP middleware that drops cancellation notifications carried by a modern POST; happy to send a PR for whichever fix you prefer. From 133d974355894df42c0c80a58790ac0c35cd0694 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:01:38 +0300 Subject: [PATCH 08/19] test(serveredition): two OAuth users with differing entitlements are isolated with no session (T031) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec 058 FR-013, server edition: bearer JWT → user record → ResolveAgentTokenOwner → AllowedServers, re-derived per modern request; tool lists and calls differ per identity while every request runs under an empty session id. --- .../api/entitlement_stateless_test.go | 215 ++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 internal/serveredition/api/entitlement_stateless_test.go diff --git a/internal/serveredition/api/entitlement_stateless_test.go b/internal/serveredition/api/entitlement_stateless_test.go new file mode 100644 index 000000000..8115be7f8 --- /dev/null +++ b/internal/serveredition/api/entitlement_stateless_test.go @@ -0,0 +1,215 @@ +//go:build server + +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/mark3labs/mcp-go/client" + "github.com/mark3labs/mcp-go/client/transport" + "github.com/mark3labs/mcp-go/mcp" + mcpserver "github.com/mark3labs/mcp-go/server" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + teamsauth "github.com/smart-mcp-proxy/mcpproxy-go/internal/serveredition/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/serveredition/users" +) + +// Spec 058 US3, FR-013 (server edition): multiuser tool filtering must survive +// the stateless 2026-07-28 era. Research D4 found this the one untested half of +// FR-013 — the server-edition suite was green under v1.0.0, but nothing drove +// two OAuth users with differing entitlements over modern requests, where no +// session exists to carry identity between calls. +// +// The identity path under test is the one setup.go wires for the MCP surface +// (buildSessionPrincipalResolver): a bearer JWT on each request → the user +// record → UserHandlers.ResolveAgentTokenOwner, THE entitlement predicate → +// an AuthContext whose AllowedServers is the tenant's entitlement. Every +// input is request-carried; the test proves the resulting tool view is too. +// +// Test names avoid the substring "MCP" (release-qa-gate.yml skips on it). + +const ( + statelessUserA = "01HTEST0000000000STATELESSA" + statelessUserB = "01HTEST0000000000STATELESSB" +) + +// statelessProbe records what the MCP server observed about each request's +// session and era. +type statelessProbe struct { + mu sync.Mutex + sessionIDs []string + legacySeen int +} + +func (p *statelessProbe) observe(ctx context.Context) { + p.mu.Lock() + defer p.mu.Unlock() + if sess := mcpserver.ClientSessionFromContext(ctx); sess != nil { + p.sessionIDs = append(p.sessionIDs, sess.SessionID()) + } + if !mcpserver.IsModernRequest(ctx) { + p.legacySeen++ + } +} + +// ownerOf reads the server a direct-surface tool name belongs to. +func ownerOf(toolName string) string { + owner, _, _ := strings.Cut(toolName, "__") + return owner +} + +func TestEntitlementStateless_TwoUsersIsolatedWithoutSession(t *testing.T) { + store := newFixtureUserStore(t) + now := time.Now().UTC() + for _, u := range []struct{ id, email, name string }{ + {statelessUserA, "a@example.com", "User A"}, + {statelessUserB, "b@example.com", "User B"}, + } { + require.NoError(t, store.CreateUser(&users.User{ + ID: u.id, Email: u.email, DisplayName: u.name, Provider: "google", + ProviderSubjectID: "sub-" + u.id, CreatedAt: now, LastLoginAt: now, + })) + } + // Differing entitlements: both reach the shared server, only B has a + // personal one. + require.NoError(t, store.CreateUserServer(statelessUserB, &config.ServerConfig{ + Name: "b-private", URL: "https://b.internal/mcp", Protocol: "http", Enabled: true, + })) + shared := defaultSharedServers() // shared-github + handlers := NewUserHandlers(store, func() []*config.ServerConfig { return shared }, nil, nil, zap.NewNop().Sugar()) + + hmacKey := tokenTestHMACKey + tokenA, err := teamsauth.GenerateBearerToken(hmacKey, statelessUserA, "a@example.com", "User A", "user", "google", time.Hour) + require.NoError(t, err) + tokenB, err := teamsauth.GenerateBearerToken(hmacKey, statelessUserB, "b@example.com", "User B", "user", "google", time.Hour) + require.NoError(t, err) + + // resolvePrincipal is buildSessionPrincipalResolver's bearer branch, per + // request: validate the JWT, load the user, materialise the entitlement. + resolvePrincipal := func(ctx context.Context, r *http.Request) context.Context { + raw := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ") + claims, err := teamsauth.ValidateBearerToken(raw, hmacKey) + if err != nil { + return ctx + } + owner, err := handlers.ResolveAgentTokenOwner(claims.Subject, []string{"*"}, nil) + if err != nil || !owner.Active { + return ctx + } + ac := auth.UserContext(owner.UserID, owner.Email, "", owner.Provider) + ac.AllowedServers = owner.Entitled + ac.CredentialKind = auth.CredentialKindBearerJWT + return auth.WithAuthContext(ctx, ac) + } + + probe := &statelessProbe{} + hooks := &mcpserver.Hooks{} + hooks.AddBeforeAny(func(ctx context.Context, _ any, _ mcp.MCPMethod, _ any) { probe.observe(ctx) }) + + // The entitlement filter: a tool is visible and callable only when the + // request's identity is entitled to its owning server. An unidentified + // request sees nothing (fail closed). + entitled := func(ctx context.Context, tools []mcp.Tool) []mcp.Tool { + ac := auth.AuthContextFromContext(ctx) + out := make([]mcp.Tool, 0, len(tools)) + for _, tool := range tools { + if ac != nil && ac.CanAccessServer(ownerOf(tool.Name)) { + out = append(out, tool) + } + } + return out + } + srv := mcpserver.NewMCPServer("se-probe", "1.0.0", + mcpserver.WithToolCapabilities(true), + mcpserver.WithToolFilter(entitled), + mcpserver.WithHooks(hooks), + ) + for _, name := range []string{"shared-github__list_repos", "b-private__read_secret"} { + name := name + srv.AddTool(mcp.NewTool(name, mcp.WithDescription(name)), + func(context.Context, mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return mcp.NewToolResultText("ok:" + name), nil + }) + } + // Unpinned: the modern era is served (the FR-028 pin applies to mcpproxy's + // own listening endpoint, which is what T041 lifts). + httpServer := httptest.NewServer(mcpserver.NewStreamableHTTPServer(srv, mcpserver.WithHTTPContextFunc(resolvePrincipal))) + t.Cleanup(httpServer.Close) + + connect := func(token string) *client.Client { + tr, err := transport.NewStreamableHTTP(httpServer.URL, + transport.WithHTTPHeaders(map[string]string{"Authorization": "Bearer " + token})) + require.NoError(t, err) + c := client.NewClient(tr, client.WithProtocolVersion(mcp.LATEST_PROTOCOL_VERSION)) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + t.Cleanup(cancel) + require.NoError(t, c.Start(ctx)) + t.Cleanup(func() { _ = c.Close() }) + _, err = c.Initialize(ctx, mcp.InitializeRequest{}) + require.NoError(t, err) + require.Equal(t, mcp.LATEST_PROTOCOL_VERSION, c.ProtocolVersion(), + "this test asserts modern-era behaviour; a legacy negotiation would make it vacuous") + require.Empty(t, tr.GetSessionId(), "the modern era binds no session id") + return c + } + listNames := func(c *client.Client) []string { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + res, err := c.ListTools(ctx, mcp.ListToolsRequest{}) + require.NoError(t, err) + names := make([]string, 0, len(res.Tools)) + for _, tool := range res.Tools { + names = append(names, tool.Name) + } + return names + } + call := func(c *client.Client, name string) (*mcp.CallToolResult, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + req := mcp.CallToolRequest{} + req.Params.Name = name + req.Params.Arguments = map[string]any{} + return c.CallTool(ctx, req) + } + + a := connect(tokenA) + b := connect(tokenB) + + // Interleaved, concurrent-style use: identity is re-derived per request, so + // A's view is unchanged by B's requests in between. + first := listNames(a) + assert.Equal(t, []string{"shared-github__list_repos"}, first, "A is entitled to the shared server only") + assert.ElementsMatch(t, []string{"shared-github__list_repos", "b-private__read_secret"}, listNames(b), "B is entitled to the shared server and its own") + assert.Equal(t, first, listNames(a), "A's view must not drift after another identity's requests") + + // Calls are gated by the same request-carried entitlement. + _, err = call(a, "b-private__read_secret") + require.Error(t, err, "A must not be able to call B's private tool") + res, err := call(b, "b-private__read_secret") + require.NoError(t, err) + require.False(t, res.IsError) + res, err = call(a, "shared-github__list_repos") + require.NoError(t, err) + require.False(t, res.IsError) + + // Nothing above relied on a session: every request ran under an empty id + // in the modern era. + probe.mu.Lock() + defer probe.mu.Unlock() + require.NotEmpty(t, probe.sessionIDs, "the probe must have observed the requests") + for _, sid := range probe.sessionIDs { + assert.Equal(t, "", sid, "a modern request must carry no session id") + } + assert.Zero(t, probe.legacySeen, "every request must have been served on the modern path") +} From f7ed69954009ef4f7945c62ef9683e49b3c7a009 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:04:01 +0300 Subject: [PATCH 09/19] test(server): modern-era client identity must reach activity records (T032) Spec 058 US3: initialize never runs in the 2026-07-28 era, so the session store never learns a modern client's name. Adds ActivityAttribution.ClientVersion (metadata.client_version when the session resolver wrote none) and the failing unit + end-to-end tests. --- internal/runtime/activity_attribution.go | 23 +++- internal/server/modern_attribution_test.go | 122 +++++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 internal/server/modern_attribution_test.go diff --git a/internal/runtime/activity_attribution.go b/internal/runtime/activity_attribution.go index 98018f783..dce313f7a 100644 --- a/internal/runtime/activity_attribution.go +++ b/internal/runtime/activity_attribution.go @@ -23,7 +23,13 @@ type ActivityAttribution struct { // ClientName is the self-reported clientInfo.name: advisory, never // authoritative (a client can claim any name). ClientName string - TokenName string + // ClientVersion is the self-reported clientInfo.version that travelled + // with ClientName when the identity was read off the REQUEST rather than + // a session (Spec 058 US3: a 2026-07-28 request never ran initialize, so + // the session store cannot supply it). Empty when the session resolver + // already stamped metadata.client_version. + ClientVersion string + TokenName string // TokenPrefix is the 12-char display prefix of the calling token. It is // INTERNAL: persisted on the storage record as the ownership proof (so a // scoped caller recognises its own policy_decision and prompt_get rows), @@ -51,6 +57,7 @@ func (a ActivityAttribution) payload() map[string]any { set("profile_source", a.ProfileSource) set("client_id", a.ClientID) set("client_name", a.ClientName) + set("client_version", a.ClientVersion) set("token_name", a.TokenName) set("_token_prefix", a.TokenPrefix) return out @@ -68,6 +75,7 @@ func attributionFromPayload(payload map[string]any) ActivityAttribution { ProfileSource: getStringPayload(m, "profile_source"), ClientID: getStringPayload(m, "client_id"), ClientName: getStringPayload(m, "client_name"), + ClientVersion: getStringPayload(m, "client_version"), TokenName: getStringPayload(m, "token_name"), TokenPrefix: getStringPayload(m, "_token_prefix"), } @@ -75,7 +83,10 @@ func attributionFromPayload(payload map[string]any) ActivityAttribution { // applyAttribution copies an attribution onto the first-class record fields. // ClientName falls back to the session-resolved metadata.client_name that -// withClientInfo already wrote, so the two never disagree on a record. +// withClientInfo already wrote, so the two never disagree on a record. A +// request-carried ClientVersion lands in metadata.client_version — the key +// withClientInfo uses — when the session resolver wrote none, so a modern +// client's record carries the same shape a legacy client's does. func applyAttribution(rec *storage.ActivityRecord, a ActivityAttribution) { rec.Profile = a.Profile rec.ProfileSource = a.ProfileSource @@ -88,6 +99,14 @@ func applyAttribution(rec *storage.ActivityRecord, a ActivityAttribution) { rec.ClientName = name } } + if a.ClientVersion != "" { + if _, ok := rec.Metadata["client_version"]; !ok { + if rec.Metadata == nil { + rec.Metadata = make(map[string]interface{}) + } + rec.Metadata["client_version"] = a.ClientVersion + } + } } // NewUsageAggregate returns an empty aggregate at the current admission diff --git a/internal/server/modern_attribution_test.go b/internal/server/modern_attribution_test.go new file mode 100644 index 000000000..7ae03b2be --- /dev/null +++ b/internal/server/modern_attribution_test.go @@ -0,0 +1,122 @@ +package server + +import ( + "context" + "testing" + "time" + + "github.com/mark3labs/mcp-go/client" + "github.com/mark3labs/mcp-go/client/transport" + "github.com/mark3labs/mcp-go/mcp" + mcpserver "github.com/mark3labs/mcp-go/server" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" +) + +// Spec 058 US3 (research D5): `initialize` is METHOD_NOT_FOUND in the +// 2026-07-28 era, so AddAfterInitialize never fires for a modern client and the +// session store never learns its name — client-name attribution on activity +// records and the Spec 044 activation funnel go blind. The replacement data +// travels on every request: server.RequestProtocolInfoFromContext(ctx). +// ClientInfo, populated by mcp-go before any hook or handler runs. +// +// Test names avoid the substring "MCP" (release-qa-gate.yml skips on it). + +// The attribution helper every activity funnel feeds reads the request-carried +// identity when no session knows the client. +func TestModernAttribution_ReadsClientInfoFromRequest(t *testing.T) { + proxy := statelessListProxy(t) + + t.Run("modern request without a session", func(t *testing.T) { + attr := proxy.activityAttribution(modernRequestCtx(context.Background()), "") + assert.Equal(t, "era-probe", attr.ClientName) + assert.Equal(t, "1.0.0", attr.ClientVersion) + }) + + t.Run("modern request on the stdio session, which initialize never named", func(t *testing.T) { + attr := proxy.activityAttribution(modernRequestCtx(sessionCtx(adminCtx(), "stdio")), "stdio") + assert.Equal(t, "era-probe", attr.ClientName) + assert.Equal(t, "1.0.0", attr.ClientVersion) + }) + + t.Run("a session's recorded name still wins over the request's claim", func(t *testing.T) { + proxy.sessionStore.SetSession("legacy-1", "ide-from-initialize", "2.0.0", false, false, nil) + attr := proxy.activityAttribution(modernRequestCtx(sessionCtx(adminCtx(), "legacy-1")), "legacy-1") + assert.Equal(t, "ide-from-initialize", attr.ClientName) + assert.Empty(t, attr.ClientVersion, "the version is request-carried only when the name is") + }) + + t.Run("modern request that declared no clientInfo", func(t *testing.T) { + ctx := mcpserver.WithRequestProtocolInfo(context.Background(), &mcpserver.RequestProtocolInfo{Modern: true, ProtocolVersion: mcp.LATEST_PROTOCOL_VERSION}) + attr := proxy.activityAttribution(ctx, "") + assert.Empty(t, attr.ClientName) + assert.Empty(t, attr.ClientVersion) + }) +} + +// End to end: a modern client's name and version reach the tool_call activity +// record, and the client-seen funnel observes it, with no initialize and no +// session. +func TestModernAttribution_ReachesActivityRecord(t *testing.T) { + env := NewTestEnvironment(t) + t.Cleanup(env.Cleanup) + ctx := context.Background() + + mock := env.CreateMockUpstreamServer("attrib-srv", []mcp.Tool{{Name: "echo", Description: "Echo"}}) + old := env.proxyServer.runtime.Config() + cfgCopy := *old + cfg := &cfgCopy + cfg.Servers = append(append([]*config.ServerConfig{}, old.Servers...), &config.ServerConfig{ + Name: "attrib-srv", URL: mock.addr, Protocol: "streamable-http", Enabled: true, Quarantined: false, + }) + env.proxyServer.runtime.UpdateConfig(cfg, "") + require.NoError(t, env.proxyServer.runtime.CommitConfig(cfg)) + time.Sleep(2 * time.Second) + _ = env.proxyServer.runtime.DiscoverAndIndexTools(ctx) + time.Sleep(2 * time.Second) + + rt := env.proxyServer.runtime + before, err := rt.StorageManager().GetOnboardingState() + require.NoError(t, err) + seenBefore := 0 + if before != nil { + seenBefore = len(before.ClientLastSeen) + } + + url := unpinnedProxyEndpoint(t, env.proxyServer, env.proxyServer.mcpProxy.GetMCPServer()) + tr, err := transport.NewStreamableHTTP(url) + require.NoError(t, err) + c := client.NewClient(tr, modernEra().options...) + require.NoError(t, c.Start(ctx)) + t.Cleanup(func() { _ = c.Close() }) + init := mcp.InitializeRequest{} + init.Params.ClientInfo = mcp.Implementation{Name: "era-probe", Version: "1.0.0"} + _, err = c.Initialize(ctx, init) + require.NoError(t, err) + requireEra(t, c, modernEra()) + require.Empty(t, tr.GetSessionId(), "the modern era binds no session; attribution cannot come from one") + + call := mcp.CallToolRequest{} + call.Params.Name = "call_tool_read" + call.Params.Arguments = map[string]interface{}{ + "name": "attrib-srv:echo", + "args": map[string]interface{}{}, + "intent": map[string]interface{}{"operation_type": "read"}, + } + result, err := c.CallTool(ctx, call) + require.NoError(t, err) + require.False(t, result.IsError, "%v", result.Content) + + record := awaitToolCallActivity(t, rt, "attrib-srv", "echo") + assert.Equal(t, "era-probe", record.ClientName, "the modern client's self-reported name must reach the record") + assert.Equal(t, "1.0.0", record.Metadata["client_version"], "and so must its version") + assert.Empty(t, record.SessionID, "no session was involved") + + after, err := rt.StorageManager().GetOnboardingState() + require.NoError(t, err) + require.NotNil(t, after) + assert.Greater(t, len(after.ClientLastSeen), seenBefore, + "the client-seen funnel must observe a modern client although initialize never ran") +} From 743ca6630891f56813d21d385f0dab0414b6e157 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:05:02 +0300 Subject: [PATCH 10/19] feat(server): capture modern-era client identity from request protocol info (T033) Spec 058 US3. activityAttribution falls back to RequestProtocolInfoFromContext (ctx).ClientInfo when no session names the client, and the BeforeAny hook feeds the Spec 044 activation funnel and client-seen presence from the same data, debounced per client identity, so both survive the loss of AfterInitialize in the 2026-07-28 era. --- internal/server/mcp.go | 48 +++++++++++++++++++++ internal/server/mcp_activity_attribution.go | 25 +++++++++++ 2 files changed, 73 insertions(+) diff --git a/internal/server/mcp.go b/internal/server/mcp.go index 01146a8bd..62e4e0d4d 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -408,6 +408,35 @@ func (p *MCPProxyServer) currentTruncator() *truncate.Truncator { return p.truncatorFn() } +// modernClientRecorder debounces the per-request client identity of modern +// (2026-07-28) requests, so the activation funnel and the client-seen presence +// are fed once per distinct client per window rather than once per request. +// Bounded: an entry per distinct (name, version) ever seen by this process, +// which is the same cardinality the session store's client names have. +type modernClientRecorder struct { + mu sync.Mutex + seen map[string]time.Time + now func() time.Time +} + +func newModernClientRecorder() *modernClientRecorder { + return &modernClientRecorder{seen: map[string]time.Time{}, now: time.Now} +} + +// firstSightingWithin reports whether (name, version) has not been recorded in +// the last window, recording it when so. +func (r *modernClientRecorder) firstSightingWithin(name, version string, window time.Duration) bool { + key := name + "\x00" + version + now := r.now() + r.mu.Lock() + defer r.mu.Unlock() + if last, ok := r.seen[key]; ok && now.Sub(last) < window { + return false + } + r.seen[key] = now + return true +} + // modernCancellationGuard closes the cross-client cancellation hole the // 2026-07-28 era opens in mcp-go (Spec 058 US3, plan risk R1). The upstream // report is drafted at specs/058-mcp-2026-upgrade/upstream-issues/ @@ -530,7 +559,26 @@ func NewMCPProxyServer( // Update session activity on every MCP message to prevent premature cleanup. // Without this, sessions that only do retrieve_tools (not tool calls) would // appear inactive and get closed by the background cleanup. + // Spec 058 US3 (T033): a 2026-07-28 request never runs initialize, so the + // AfterInitialize hook below — which feeds the Spec 044 activation funnel + // and the client-seen presence — never fires for a modern client. The + // identity travels on every request instead (research D5), so it is read + // here, once per client identity per minute (the same window + // RecordClientSeen itself debounces on), never once per request. + modernClients := newModernClientRecorder() hooks.AddBeforeAny(func(ctx context.Context, id any, method mcp.MCPMethod, message any) { + if name, version := requestClientInfo(ctx); name != "" && modernClients.firstSightingWithin(name, version, time.Minute) { + if mainServer != nil && mainServer.runtime != nil { + mainServer.runtime.RecordMCPClientForActivation(name) + mainServer.runtime.RecordClientSeen(name) + } + logger.Info("MCP client identified from request metadata", + zap.String("client_name", name), + zap.String("client_version", version), + zap.String("protocol_version", mcpserver.RequestProtocolVersion(ctx)), + ) + } + session := mcpserver.ClientSessionFromContext(ctx) if session == nil { return diff --git a/internal/server/mcp_activity_attribution.go b/internal/server/mcp_activity_attribution.go index ff68ad458..6b8c5c8a1 100644 --- a/internal/server/mcp_activity_attribution.go +++ b/internal/server/mcp_activity_attribution.go @@ -3,6 +3,8 @@ package server import ( "context" + mcpserver "github.com/mark3labs/mcp-go/server" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" "github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime" ) @@ -94,5 +96,28 @@ func (p *MCPProxyServer) activityAttribution(ctx context.Context, sessionID stri } } } + + // Spec 058 US3 (T033): a 2026-07-28 request never ran initialize, so no + // session knows its client — its session id is "" over Streamable HTTP, + // or a transport constant the handshake never named. The identity travels + // on the request itself (research D5), stamped by mcp-go before any hook + // or handler runs. Still advisory, exactly like the initialize-time name. + if attr.ClientName == "" { + if name, version := requestClientInfo(ctx); name != "" { + attr.ClientName = name + attr.ClientVersion = version + } + } return attr } + +// requestClientInfo returns the client name and version a modern (2026-07-28) +// request declared in its _meta, or empty strings for a legacy request or a +// modern one that declared none. +func requestClientInfo(ctx context.Context) (name, version string) { + info := mcpserver.RequestProtocolInfoFromContext(ctx) + if info == nil || !info.Modern || info.ClientInfo == nil { + return "", "" + } + return info.ClientInfo.Name, info.ClientInfo.Version +} From bae91aee4b3a6b48a49f41bc74e45c64f5c9b887 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:06:09 +0300 Subject: [PATCH 11/19] docs(server): correct the two session comments the stateless era made false (T034) The 'stable session id, no synthetic fallback' comment on sessionIDFromContext was rewritten in T023 (the id is attribution-grade only; modern requests have "" and stdio a constant). The 'background inactivity cleanup calls RemoveSession' comment in session_store.go had already been removed by #1497; the remaining claim there (the closed-entry TTL matches the storage inactivity timeout) is true. The AfterInitialize hook comment now says it never fires in the modern era. --- internal/server/mcp.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/server/mcp.go b/internal/server/mcp.go index 62e4e0d4d..c7c0ed299 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -626,9 +626,13 @@ func NewMCPProxyServer( ) }) - // Add hook to capture client capabilities after initialize completes - // This hook is called for ALL transports (including HTTP POST), and receives the - // InitializeRequest with client info and capabilities. + // Add hook to capture client capabilities after initialize completes. + // It fires for every transport (including HTTP POST) — but only in the + // LEGACY protocol era: `initialize` is METHOD_NOT_FOUND under 2026-07-28, + // so a modern client never reaches it. Client identity for that era is + // read per request in the BeforeAny hook above (Spec 058 US3); everything + // else recorded here (capabilities, credential, serving instance) is + // session-scoped and has no modern counterpart by design. hooks.AddAfterInitialize(func(ctx context.Context, id any, request *mcp.InitializeRequest, result *mcp.InitializeResult) { // Get session from context session := mcpserver.ClientSessionFromContext(ctx) From f004516976b46452d5ff761e601a8f3da396e341 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:08:38 +0300 Subject: [PATCH 12/19] fix(server): pin the stdio surface to the client-facing protocol eras (T036a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec 058 FR-028 closes the stdio era gap: mcp-go has no stdio equivalent of WithStreamableHTTPProtocolVersions and chooses the era per request, so a stdio client could opt into 2026-07-28 while every HTTP door refused it. An OnRequestInitialization hook rejects a declared version outside clientFacingProtocolVersions — the one constant set the HTTP pin now reads too — with INVALID_REQUEST; legacy stdio and the HTTP surfaces are untouched. --- internal/server/mcp.go | 44 ++++++++++++++++++++ internal/server/server.go | 31 +++++++++----- internal/server/stdio_era_test.go | 68 +++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 internal/server/stdio_era_test.go diff --git a/internal/server/mcp.go b/internal/server/mcp.go index c7c0ed299..23c07cf98 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -9,6 +9,7 @@ import ( "net/http" "os/exec" "regexp" + "slices" "sort" "strconv" "strings" @@ -408,6 +409,43 @@ func (p *MCPProxyServer) currentTruncator() *truncate.Truncator { return p.truncatorFn() } +// rejectUnservedProtocolOnStdio is the stdio half of the FR-028 era pin (Spec +// 058 T036a). The Streamable HTTP transport refuses a request whose declared +// protocol version it has not been configured to serve, but mcp-go has no such +// option for stdio, and the shared handler chooses the era per request from +// params._meta — so without this a stdio client could opt into 2026-07-28 while +// every HTTP door refused it. Registered as an OnRequestInitialization hook, it +// runs before the era is read and before any dispatch; returning an error makes +// mcp-go answer INVALID_REQUEST without touching a handler. +// +// Scoped to the stdio transport (the context tag stdioAuthContext installs) and +// to requests that DECLARE a version in _meta: a legacy request carries none +// and passes. The served set is clientFacingProtocolVersions, the same constant +// the HTTP pin reads, so lifting one lifts the other. +func rejectUnservedProtocolOnStdio(ctx context.Context, message any) error { + if transport.GetConnectionSource(ctx) != transport.ConnectionSourceStdio { + return nil + } + raw, ok := message.(json.RawMessage) + if !ok { + return nil + } + var wrapper struct { + Params struct { + Meta *mcp.Meta `json:"_meta"` + } `json:"params"` + } + if json.Unmarshal(raw, &wrapper) != nil { + return nil // a malformed body is reported by the per-method unmarshal + } + version := wrapper.Params.Meta.ProtocolVersion() + if version == "" || slices.Contains(clientFacingProtocolVersions(), version) { + return nil + } + return fmt.Errorf("protocol version %s is not served on the stdio surface: the client-facing era is pinned to %s (Spec 058 FR-028); send a legacy initialize instead", + version, strings.Join(clientFacingProtocolVersions(), ", ")) +} + // modernClientRecorder debounces the per-request client identity of modern // (2026-07-28) requests, so the activation funnel and the client-seen presence // are fed once per distinct client per window rather than once per request. @@ -556,6 +594,12 @@ func NewMCPProxyServer( // Create hooks to capture session information hooks := &mcpserver.Hooks{} + // Spec 058 T036a: pin the stdio surface to the same protocol eras as the + // Streamable HTTP transports. This hook runs before mcp-go reads the era, + // which is why it parses _meta itself. + hooks.AddOnRequestInitialization(func(ctx context.Context, _ any, message any) error { + return rejectUnservedProtocolOnStdio(ctx, message) + }) // Update session activity on every MCP message to prevent premature cleanup. // Without this, sessions that only do retrieve_tools (not tool calls) would // appear inactive and get closed by the background cleanup. diff --git a/internal/server/server.go b/internal/server/server.go index a4fd83dc4..f0f5208d0 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -2989,15 +2989,15 @@ func (s *Server) streamingNoDeadline(next http.Handler) http.Handler { // /mcp/call and /mcp/p/) — a pin applied to four of five would leave one // door open on a different protocol era. // -// SCOPE, precisely: this covers the Streamable HTTP surfaces and nothing else. -// mcp-go offers no equivalent option for stdio (server/stdio.go exposes only -// error-logger, context-func and worker-pool options), and the protocol era is -// decided per request from params._meta by the shared handler, with the version -// gate living inside the Streamable HTTP transport. So a client speaking stdio -// to `mcpproxy serve` in stdio mode can still opt into 2026-07-28 and be served -// it. That gap is tracked with the rest of the stateless work (spec 058 US3); -// it is not reachable by default, since an empty listen address is rewritten to -// the HTTP default during config validation. +// SCOPE, precisely: this covers the Streamable HTTP surfaces. mcp-go offers no +// equivalent option for stdio (server/stdio.go exposes only error-logger, +// context-func and worker-pool options), and the protocol era is decided per +// request from params._meta by the shared handler, with the version gate living +// inside the Streamable HTTP transport. The stdio surface is therefore pinned +// separately, by rejectUnservedProtocolOnStdio (mcp.go), which reads the SAME +// set — clientFacingProtocolVersions — so lifting the pin lifts both at once +// (spec 058 T036a). Stdio is not reachable by default anyway, since an empty +// listen address is rewritten to the HTTP default during config validation. // // The options are deliberately not configurable: // @@ -3022,11 +3022,22 @@ func (s *Server) streamingNoDeadline(next http.Handler) http.Handler { func clientFacingStreamableOptions(cfgProvider func() *config.Config) []server.StreamableHTTPOption { return []server.StreamableHTTPOption{ server.WithDisableLocalhostProtection(true), - server.WithStreamableHTTPProtocolVersions(mcp.LegacyProtocolVersions()...), + server.WithStreamableHTTPProtocolVersions(clientFacingProtocolVersions()...), server.WithHTTPContextFunc(captureClientHeaders(cfgProvider)), } } +// clientFacingProtocolVersions is the ONE definition of the protocol eras the +// client-facing surfaces serve (Spec 058 FR-028): the legacy set, until the +// stateless work has landed and T041 lifts the pin. Both the Streamable HTTP +// transport option above and the stdio guard (rejectUnservedProtocolOnStdio) +// read it, so the two surfaces cannot disagree about what is served. A +// function, not a variable, for the same reason as its caller: it must not be +// flippable at runtime or from a test. +func clientFacingProtocolVersions() []string { + return mcp.LegacyProtocolVersions() +} + // liveConfig returns the runtime's current config, tolerating a nil runtime. func (s *Server) liveConfig() *config.Config { if s == nil || s.runtime == nil { diff --git a/internal/server/stdio_era_test.go b/internal/server/stdio_era_test.go new file mode 100644 index 000000000..b0e8cd30f --- /dev/null +++ b/internal/server/stdio_era_test.go @@ -0,0 +1,68 @@ +package server + +import ( + "testing" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Spec 058 T036a — the stdio era gap. FR-028 pins the client-facing surface to +// the legacy protocol era, but mcp-go offers no stdio equivalent of +// WithStreamableHTTPProtocolVersions: the era is chosen per request by the +// shared handler, so a stdio client could opt into 2026-07-28 today while the +// HTTP surface refuses it. The pin must mean the same thing on every surface, +// so a modern request on stdio is rejected before it is dispatched — exactly +// what the Streamable HTTP transport does for a version it does not serve. +// +// Lifting the pin (T041) must lift this guard with it; the guard reads the +// same constant set as clientFacingStreamableOptions. +// +// Test names avoid the substring "MCP" (release-qa-gate.yml skips on it). + +func TestStdioEra_ModernRequestIsRejectedUnderThePin(t *testing.T) { + proxy := profileEraRuntimeProxy(t) + probe := newStdioProbe(t, proxy.GetMCPServer()) + + resp := probe.request(string(mcp.MethodToolsList), map[string]any{"_meta": modernMeta()}) + errObj, ok := resp["error"].(map[string]any) + require.True(t, ok, "a %s request on stdio must be refused as a JSON-RPC error while the pin holds, got: %v", mcp.LATEST_PROTOCOL_VERSION, resp) + assert.EqualValues(t, mcp.INVALID_REQUEST, errObj["code"]) + msg, _ := errObj["message"].(string) + assert.Contains(t, msg, mcp.LATEST_PROTOCOL_VERSION, "the refusal names the version it will not serve: %s", msg) + assert.Contains(t, msg, mcp.LATEST_LEGACY_PROTOCOL_VERSION, "and the one it does: %s", msg) +} + +// The guard is era-scoped, not a stdio shutdown: a legacy stdio client still +// completes the handshake and lists tools. +func TestStdioEra_LegacyRequestsStillServed(t *testing.T) { + proxy := profileEraRuntimeProxy(t) + probe := newStdioProbe(t, proxy.GetMCPServer()) + + init := probe.request(string(mcp.MethodInitialize), map[string]any{ + "protocolVersion": mcp.LATEST_LEGACY_PROTOCOL_VERSION, + "capabilities": map[string]any{}, + "clientInfo": map[string]any{"name": "era-probe", "version": "1.0.0"}, + }) + require.Nil(t, init["error"], "legacy initialize must succeed: %v", init) + + list := probe.request(string(mcp.MethodToolsList), nil) + require.Nil(t, list["error"], "legacy tools/list must succeed: %v", list) + result, ok := list["result"].(map[string]any) + require.True(t, ok) + tools, _ := result["tools"].([]any) + assert.NotEmpty(t, tools, "the legacy stdio client lists the built-in tools") +} + +// The guard is stdio-scoped: a modern request on the HTTP surface is governed +// by the transport's own version pin, not by this hook, so the era harness's +// unpinned endpoint keeps serving modern requests. +func TestStdioEra_GuardDoesNotReachTheHTTPSurface(t *testing.T) { + env := newProfileTestEnv(t) + url := unpinnedProxyEndpoint(t, env.proxyServer, env.proxyServer.mcpProxy.GetMCPServer()) + + c := newRawEraClient(t, url, modernEra()) + list := c.result(string(mcp.MethodToolsList), nil) + c.requireEra(list) +} From aa572dcaf7f52afe5053849f7002f5beff54359b Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:08:38 +0300 Subject: [PATCH 13/19] test(server): direct-mode handler relays result _meta through proxyResultEnvelope (T036b) FR-016b on /mcp/all: over the live direct routing fixture, an upstream _meta payload key survives the hop while the hop-scoped serverInfo and protocolVersion keys are dropped, proving the registered handler calls the envelope helper. --- internal/server/mcp_routing_test.go | 28 +++++++++++++++++++++++++++ internal/server/scope_fixture_test.go | 12 +++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/internal/server/mcp_routing_test.go b/internal/server/mcp_routing_test.go index c24f04881..8c28fbcb0 100644 --- a/internal/server/mcp_routing_test.go +++ b/internal/server/mcp_routing_test.go @@ -484,6 +484,34 @@ func TestDirectModeHandler_DestructiveToolNeedsDestructivePermission(t *testing. f.admitted(t, result, "github", "delete_repo") } +// Spec 058 T036b (FR-016b on /mcp/all). The proxyResultEnvelope unit tests in +// content_forward_test.go prove what the envelope helper does; nothing proved +// the direct-mode handler CALLS it — reaching that handler needs a fully wired +// proxy with a live upstream, which the directRoutingFixture provides. The +// upstream answers with a _meta carrying the two hop-scoped keys (serverInfo, +// protocolVersion) plus a payload key; the forwarded result must keep the +// payload key and drop the hop-scoped ones. A handler that copied the upstream +// envelope wholesale would relay all three. +func TestDirectModeHandler_RelaysResultMetaThroughProxyEnvelope(t *testing.T) { + f := newDirectRoutingFixture(t, directServerSpec{server: "github", tools: []toolSpec{readSpec("list_repos")}}) + f.ups["github"].okMeta.Store(&mcp.Meta{AdditionalFields: map[string]any{ + mcp.MetaKeyServerInfo: map[string]any{"name": "upstream-github", "version": "9.9.9"}, + mcp.MetaKeyProtocolVersion: mcp.LATEST_PROTOCOL_VERSION, + "io.example/trace": "trace-123", + }}) + + result := f.call(t, adminCtx(), "github", "list_repos") + f.admitted(t, result, "github", "list_repos") + + require.NotNil(t, result.Meta, "an upstream _meta payload key must be relayed") + assert.Equal(t, "trace-123", result.Meta.AdditionalFields["io.example/trace"]) + assert.NotContains(t, result.Meta.AdditionalFields, mcp.MetaKeyServerInfo, + "relaying the upstream serverInfo would make mcpproxy's answer claim to BE the upstream (research D9)") + assert.NotContains(t, result.Meta.AdditionalFields, mcp.MetaKeyProtocolVersion, + "the era negotiated upstream is not the era mcpproxy answers on") + assert.Empty(t, result.ResultType, "ResultType is left for mcp-go to stamp for the answering era") +} + func TestRequiredPermissionForDirectTool_MapsAnnotationsToAuthPermissions(t *testing.T) { readOnly := true write := false diff --git a/internal/server/scope_fixture_test.go b/internal/server/scope_fixture_test.go index adb45ef0a..32644f4dd 100644 --- a/internal/server/scope_fixture_test.go +++ b/internal/server/scope_fixture_test.go @@ -105,6 +105,9 @@ type countingUpstream struct { // okBody, when set, replaces the default "ok" text of a successful answer // (for tests that need the upstream to return a particular payload). okBody atomic.Pointer[string] + // okMeta, when set, is attached as the successful answer's result _meta + // (Spec 058 FR-016b: what the proxy relays, and what it must strip). + okMeta atomic.Pointer[mcp.Meta] } // serve registers one more tool on the running stub upstream, for tests @@ -133,10 +136,13 @@ func (u *countingUpstream) serve(spec toolSpec) { if body := u.errBody.Load(); body != nil { return mcp.NewToolResultError(*body), nil } - if body := u.okBody.Load(); body != nil { - return mcp.NewToolResultText(*body), nil + body := "ok" + if custom := u.okBody.Load(); custom != nil { + body = *custom } - return mcp.NewToolResultText("ok"), nil + res := mcp.NewToolResultText(body) + res.Meta = u.okMeta.Load() + return res, nil }) } From eedc5b9e4658d5a12f09c84d5401b1341b8dab02 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:10:33 +0300 Subject: [PATCH 14/19] docs(spec): profiles era section, US3 decision record, tasks ticked (T035-T036) Profiles docs gain a Protocol eras section (set_profile is legacy-era only; lists never vary per connection; stdio holds the same pin). research.md D11 records the US3 decisions, including per-session token stats staying legacy-only. tasks.md ticks T020-T036b; ROADMAP.md regenerated. --- ROADMAP.md | 4 +-- docs/features/profiles.md | 22 ++++++++++++-- specs/058-mcp-2026-upgrade/research.md | 24 +++++++++++++++ specs/058-mcp-2026-upgrade/tasks.md | 42 +++++++++++++------------- 4 files changed, 66 insertions(+), 26 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 38ca672ac..61d10679e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1009,7 +1009,7 @@ graph LR | Token-efficiency benchmark: measured savings, published results | In progress | P1 | 62/64 (97%) | [103-token-bench](./specs/103-token-bench/) | | | Telemetry identity & data quality (machine_id + CI-filter hardening) | In progress | P1 | — | | | | Telemetry v7: honest funnel + churn instrumentation | In progress | P1 | — | [080-telemetry-v7-churn](./specs/080-telemetry-v7-churn/) | | -| MCP protocol upgrade to 2026-07-28 revision | In progress | P1 | 21/81 (26%) | [058-mcp-2026-upgrade](./specs/058-mcp-2026-upgrade/) | | +| MCP protocol upgrade to 2026-07-28 revision | In progress | P1 | 40/81 (49%) | [058-mcp-2026-upgrade](./specs/058-mcp-2026-upgrade/) | | | Spec 108 profiles v3: scoped profiles, client binding, scope-aware observability | In progress | P1 | 194/195 (99%) | [108-profiles-v3](./specs/108-profiles-v3/) | | | Spec 109 navigation, scope filters and cross-surface UX consistency | In progress | P1 | 257/258 (100%) | [109-ux-navigation-consistency](./specs/109-ux-navigation-consistency/) | | | Planning/docs truth automation | In progress | P2 | — | | | @@ -1120,7 +1120,7 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—` | [055-frontend-major-upgrades](./specs/055-frontend-major-upgrades/) | `shipped` | 23/24 (96%) | | [056-output-schema-validation](./specs/056-output-schema-validation/) | `in-flight` | 22/24 (92%) | | [057-in-proxy-profiles](./specs/057-in-proxy-profiles/) | `in-flight` | 20/25 (80%) | -| [058-mcp-2026-upgrade](./specs/058-mcp-2026-upgrade/) | `in-flight` | 21/81 (26%) | +| [058-mcp-2026-upgrade](./specs/058-mcp-2026-upgrade/) | `in-flight` | 40/81 (49%) | | [059-output-sanitisation](./specs/059-output-sanitisation/) | `shipped` | 24/25 (96%) | | [060-settings-page](./specs/060-settings-page/) | `shipped` | 16/16 (100%) | | [064-glass-cockpit](./specs/064-glass-cockpit/) | — | — | diff --git a/docs/features/profiles.md b/docs/features/profiles.md index 8148e7cff..45794ac28 100644 --- a/docs/features/profiles.md +++ b/docs/features/profiles.md @@ -110,7 +110,7 @@ When more than one source could select a profile, the highest wins: |---|---|---| | 1 | **pin** | A **locked** client credential, or an [agent token](./agent-tokens.md#profile-pinning) with a profile. Server-enforced and immutable for the connection. A pin whose profile was deleted denies everything instead of widening | | 2 | **url** | `/mcp/p/`. Authoritative for that request. A locked or confined caller may name only its own profile (or, if switchable, one in `switchable_to`) | -| 3 | **session** | The `set_profile` selection. Re-checked on every request | +| 3 | **session** | The `set_profile` selection. Re-checked on every request. Legacy protocol era only, and never consulted for a `*/list` response — see [Protocol eras](#protocol-eras) | | 4 | **binding** | The profile of a **switchable** client credential | | 5 | **anonymous** | `anonymous_profile`, for a caller with no credential | | 6 | **none** | No profile: all servers | @@ -186,8 +186,8 @@ The `set_profile` MCP tool switches the active profile **inside a live session** { "active_profile": "research", "servers": ["research-srv"] } ``` -- The selection is keyed by the MCP session id (stable per streamable-HTTP / SSE connection) and persists for the lifetime of that session. -- It applies to subsequent `retrieve_tools`, `call_tool_*`, `code_execution` and direct-mode (`server__tool`) calls on the base `/mcp` endpoint — `retrieve_tools` searches the profile's per-profile index directly. +- The selection is keyed by the MCP session id of a **legacy-era** connection (MCP `2025-11-25` and earlier, where streamable-HTTP and SSE bind a stable id at `initialize`) and persists for the lifetime of that session. The `2026-07-28` revision has no sessions, so `set_profile` cannot persist anything there — see [Protocol eras](#protocol-eras). +- It applies to subsequent `retrieve_tools`, `call_tool_*`, `code_execution`, direct-mode (`server__tool`) calls and `prompts/get` on the base `/mcp` endpoint — `retrieve_tools` searches the profile's per-profile index directly. It does **not** change what `tools/list` or `prompts/list` return: list responses are the same for every connection, whatever it selected (MCP `2026-07-28` forbids per-connection list variation, and MCPProxy applies that rule in both eras). The one exception during the deprecation window is the visibility of the built-in administration tools (`profiles`, `credentials`, `code_execution`, `upstream_servers`, `quarantine_security`), which a legacy session's selected profile still governs at list time. - Passing an empty string (`""`) clears the selection and returns to all servers. `active_profile` always reports the **stored session selection** — `""` after a clear, even for a token with a [`profile_pin`](./agent-tokens.md#profile-pinning) — while `servers` reports the **effective scope** the session can actually reach after the update: the pin's servers for a pinned token (nothing once the pinned profile has been deleted), the URL profile on a `/mcp/p/` endpoint, otherwise the selection or every configured server. - The `servers` list is always bounded by the caller's credential, using the same rule that scopes `retrieve_tools`: for an [agent token](./agent-tokens.md) scoped to specific servers it is the intersection of the effective profile (resolved pin > URL > session, see [Which profile applies](#which-profile-applies)) with the token's `allowed_servers`, so a token restricted to one server is never told about the others. On a `/mcp/p/` endpoint the URL still governs the request, so `set_profile("other")` there stores `other` as `active_profile` but reports ` ∩ allowed_servers` in `servers`. API-key and socket callers see the full lists. - An unknown slug is rejected. An administrator (API key, socket, anonymous back-compat) gets the discovery affordance: `unknown profile '' (available: research, deploy)`. An agent token gets `unknown profile ''` with no list at all: it may select only the profiles overlapping its `allowed_servers` (or its pin while the pin still has reach), and a profile entirely outside its reach (an empty profile, a profile whose servers are all outside `allowed_servers`, or the token's own pin once it no longer exists or no longer overlaps the token's servers) is rejected with that same error rather than confirmed as existing. A pinned token asking for any profile OTHER than its pin (see [profile pinning](./agent-tokens.md#profile-pinning)) is rejected with that same `unknown profile ''` error too — never a distinct "pinned to..." message, which would let the token confirm from the wording alone that it is pinned, and to what, from a refusal aimed at a different slug. The check looks only at the requested slug (and the token's pin) and tests the token's own `allowed_servers` against that profile's precomputed server set — its cost does not depend on how many other profiles are configured, on how many servers the requested profile declares or on how many servers are configured at all, only on the size of the token's own grant — so a token cannot learn which profiles or servers exist, or whether it is pinned, from `set_profile`, by body or by timing. @@ -196,6 +196,22 @@ The `set_profile` MCP tool switches the active profile **inside a live session** `set_profile` is available on the default `/mcp` server and the `call_tool` / `code_execution` routing-mode servers. +### Protocol eras + +MCPProxy serves two MCP protocol eras. They differ in exactly one thing that matters to profiles: whether a connection has a session. + +| Era | Session | `set_profile` | How to select a profile | +|---|---|---|---| +| **Legacy** — `2025-11-25` and earlier (the only era served on the client-facing surfaces today) | A session id is bound at `initialize` | Works as described above; the selection lives for the session | Pin or binding on the credential, `/mcp/p/`, or `set_profile` | +| **Modern** — `2026-07-28` | None. Every request carries its own identity in `_meta` and headers | Refused. The reply names the per-request form, for example `connecting to /mcp/p/research`, so an agent can act on it. An unknown slug is still answered `unknown profile ''`, exactly as in the legacy era | Pin or binding on the credential, or `/mcp/p/` — both are carried by every request | + +Rules that hold in both eras: + +- `tools/list` and `prompts/list` never vary per connection. The identity-scoped parts of a response come from the request (credential, URL path, `_meta`), never from what a connection did earlier. +- The URL form `/mcp/p/` is request-carried identity: the slug is read from every request's path, nothing is bound at `initialize`, so it keeps working without a session. +- A stdio client is held to the same era pin as the HTTP endpoints: while the client-facing surfaces serve only the legacy era, a stdio request that declares `2026-07-28` in `_meta` is refused with an `Invalid Request` error naming the served versions, instead of being served statelessly on a transport that reports a constant session id. +- Activity records of a modern client still carry its self-reported name and version, read from the request's `_meta` rather than from a session. + ### REST API | Method & path | Description | diff --git a/specs/058-mcp-2026-upgrade/research.md b/specs/058-mcp-2026-upgrade/research.md index 467b55028..e7c6170f8 100644 --- a/specs/058-mcp-2026-upgrade/research.md +++ b/specs/058-mcp-2026-upgrade/research.md @@ -203,3 +203,27 @@ Six lenses over the Phase 1–2 diff, every finding put through three skeptics. - The `proxyResultEnvelope` tests do not establish that the direct-mode handler calls it; reaching that handler needs a fully wired proxy. `/mcp/all`'s FR-016b guarantee rests on the call site staying as written. Tracked as T036b. **Reviewer noise worth noting:** 15 findings were refuted, and the stdio gap was independently raised by four of six lenses at "major" before verification settled it at minor — reviewer volume is not evidence of severity, and the per-finding skeptic pass is what separated the two real defects from the rest. Three verify agents also died on API errors, which lowers the effective vote count on their findings; a finding with one surviving vote out of two would be recorded as refuted, so the confirmed set is a floor, not a ceiling. + +--- + +## D11 — US3 implementation record (2026-10-20) + +Phase 3 landed under the FR-028 pin, on an era harness that reaches the modern era by mounting the proxy's own MCP server over an unpinned Streamable HTTP transport behind the production auth middleware (`unpinnedProxyEndpoint`, `internal/server/stateless_list_test.go`). Decisions taken while landing it, each with the evidence that forced it: + +**Per-session token stats stay legacy-only (T036).** `storage.SessionRecord` and `UpdateSessionStats` model a connection with a beginning and an end; the 2026-07-28 era has neither, and synthesizing a "session" from request-carried identity would re-invent Spec 082's work session under a second name. Modern-era usage is already attributed per record: `ActivityAttribution` now carries the request's `clientInfo` name and version (T033), alongside token and profile, and the Spec 108 usage aggregate folds filtered activity records (`GET /activity/usage` under `profile`/`client`/`token`) without consulting session rows. If grouping by "one client, one project" is wanted for modern clients, extend Spec 082's `WorkSessionIdentity` derivation to the request-carried principal + `clientInfo` (that is T075's correlation key), not `SessionRecord`. Revisit only if the Sessions UI needs modern connections listed as rows — today it would list nothing for them, which is accurate. + +**The session tier is gated on the era, not the id (T025).** `profileSessionIDFromContext` returns `""` for a modern request whatever the transport reports. The stdio probe (`TestProfileEra_ModernStdioRequestCannotPersistSelection`) showed the trap D1 predicted: before the gate, a modern stdio `set_profile` stored `research` under session `"stdio"` and the result claimed `profile_source: session`. + +**`*/list` resolution skips the session tier; execution keeps it (T023–T024).** `listOnlyProfileResolution(ctx)` marks a context so BOTH resolvers (`resolveActiveProfileWithSourceFromIndex` and the Spec 108 `resolveProfileV3`) skip tier 3 and record nothing on the session. `filterDirectModeToolsForAuth` uses it at list time only — the call-time re-evaluation of the same mcp-go filter chain is an execution gate and keeps the full precedence, as does `authorizeAggregatedPromptServer` for `prompts/get`. A legacy session that selected `research` therefore lists deploy-srv's prompt but cannot fetch it; that is the model FR-012 asks for (identity-scoped execution, canonical lists) and it holds until T077 drops the tier entirely. + +**Residual, deliberate: built-in visibility under a legacy session selection.** `filterProfileV3Tools` (Spec 108 FR-017, Spec 115 FR-011) hides `profiles`, `credentials`, `code_execution` and the management tools according to the effective profile, session tier included, and `TestProfilesTool_Visibility*` pins that contract. So in the legacy era a session that ran `set_profile` gets a `tools/list` that differs from a fresh one by those built-ins. It cannot happen in the modern era (the tier never applies), and it disappears at T077. Changing Spec 108's contract was out of US3's scope; the docs name the exception. `TestStatelessList_ToolsListIdenticalAcrossConnections` compares two fresh connections per era and a refused modern `set_profile`, not a legacy selection, for exactly this reason. + +**R1 is confirmed and guarded at the HTTP layer (T029–T030).** `TestCancelIsolation_LibraryCollisionStillPresent` reproduces the collision against an unguarded mcp-go v1.0.0 transport: client B's `notifications/cancelled` for id 1 cancels client A's in-flight call. mcp-go handles the notification before any hook runs, so the guard is middleware (`modernCancellationGuard`): a cancellation notification carried by a POST whose `Mcp-Protocol-Version` is modern is acknowledged (202) and dropped. Nothing is lost — that era has no session to correlate a separately posted notification with, and the mcp-go client never sends one; a client abandons a call by closing the connection. The upstream report could not be filed: `mark3labs/mcp-go` had interactions restricted to collaborators on 2026-10-20. The draft is in `upstream-issues/mcp-go-inflight-cancel-collision.md`; `inflightKey` is unchanged on mcp-go `main` (v1.3.0). The collision test skips when the library stops colliding, which is the signal to retire the guard. + +**Stdio era pin (T036a).** `rejectUnservedProtocolOnStdio` (an `OnRequestInitialization` hook, scoped by the stdio connection-source tag) refuses a declared `_meta` version outside `clientFacingProtocolVersions()`, the constant set the HTTP option now reads too, so T041 lifts both by editing one function. + +**Attribution without `initialize` (T032–T033).** `activityAttribution` falls back to `RequestProtocolInfoFromContext(ctx).ClientInfo`; `ActivityAttribution.ClientVersion` lands in `metadata.client_version` so a modern client's record has the legacy shape. The BeforeAny hook feeds `RecordMCPClientForActivation` / `RecordClientSeen` from the same data, debounced per `(name, version)` per minute — the window `RecordClientSeen` already uses — because a per-request BBolt write would be the alternative. + +**FR-016b on `/mcp/all` is now asserted (T036b)** over the live direct routing fixture: an upstream `_meta` payload key is relayed while `serverInfo` and `protocolVersion` are dropped. + +**T034.** The resolver comment was rewritten with the gate; the `session_store.go` comment D1 named had already been removed by #1497 (its surviving claim — the closed-entry TTL matches the storage inactivity timeout — is true). The `AfterInitialize` hook comment now says it never fires in the modern era. diff --git a/specs/058-mcp-2026-upgrade/tasks.md b/specs/058-mcp-2026-upgrade/tasks.md index ec3b933f2..b98680c56 100644 --- a/specs/058-mcp-2026-upgrade/tasks.md +++ b/specs/058-mcp-2026-upgrade/tasks.md @@ -64,27 +64,27 @@ Single Go project. Backend paths are repo-relative: `internal/`, `cmd/`, `bench/ **Independent test**: two differently-scoped agent tokens connect concurrently; each sees only its permitted tools; the canonical list facade is identical across connections; nothing relies on a session id. -- [ ] T020 [P] [US3] Write failing tests that `tools/list` is byte-identical across two connections in both eras, in `internal/server/stateless_list_test.go` -- [ ] T021 [P] [US3] Write a failing test that `prompts/list` does not change after `set_profile` — this fails on `main` today (FR-012) — in `internal/server/stateless_list_test.go` -- [ ] T022 [P] [US3] Write a failing test that a modern **stdio** request cannot persist a profile selection despite its non-empty session id, in `internal/server/profile_era_test.go` -- [ ] T023 [US3] Add `resolveActiveProfileForList` that skips the session tier, in `internal/server/profile_resolver.go` -- [ ] T024 [US3] Switch the two list filters to the list-only resolver in `internal/server/mcp_direct_scope.go` -- [ ] T025 [US3] Gate the session tier on `!server.IsModernRequest(ctx)` in addition to a non-empty id, in `internal/server/profile_resolver.go` -- [ ] T026 [US3] Validate the slug before the session guard, and return machine-actionable guidance naming the `/mcp/p/` form on modern requests, in `internal/server/profile_tool.go` -- [ ] T027 [US3] Split `TestProfile_SetProfileSessionScoped` into legacy and modern variants, keeping the original name for the legacy case so the spec's named gate still resolves, in `internal/server/profile_integration_test.go` -- [ ] T028 [US3] Fix `TestProfile_SetProfileUnknown` to assert the unknown-slug message in both eras, in `internal/server/profile_integration_test.go` -- [ ] T029 [P] [US3] Write a failing test that one client cannot cancel another's in-flight request when both use the same JSON-RPC id, in `internal/server/cancel_isolation_test.go` -- [ ] T030 [US3] Add the cross-client cancellation guard (R1) and file the upstream mcp-go issue, touching `internal/server/mcp.go` -- [ ] T031 [P] [US3] Write a failing `-tags server` test that two OAuth users with differing entitlements are isolated with no session, in `internal/serveredition/api/entitlement_stateless_test.go` -- [ ] T032 [P] [US3] Write a failing test that modern-era client name/version reach the activity record via `RequestProtocolInfoFromContext`, in `internal/server/modern_attribution_test.go` -- [ ] T033 [US3] Capture client identity from request protocol info in the `BeforeAny` hook so activation telemetry survives the loss of `AfterInitialize`, in `internal/server/mcp.go` -- [ ] T034 [P] [US3] Correct the false "stable session id" comment in `internal/server/profile_resolver.go` and the false "background cleanup calls RemoveSession" comment in `internal/server/session_store.go` -- [ ] T035 [P] [US3] Update the profile docs for era behaviour in `docs/features/profiles.md` -- [ ] T036 [US3] Decide and record whether per-session token stats become legacy-only or are re-keyed on request-carried identity, in `specs/058-mcp-2026-upgrade/research.md` -- [ ] T036a [US3] Close the stdio era gap: mcp-go offers no stdio equivalent of the Streamable HTTP version pin, and the era is chosen per request by the shared handler, so a stdio client can opt into `2026-07-28` today. Reject a modern `_meta` protocol version on the stdio surface (an `onRequestInitialization` hook or middleware on the server at `internal/server/server.go:985`), with a failing test first -- [ ] T036b [US3] Assert the direct-mode handler actually calls `proxyResultEnvelope` (FR-016b on `/mcp/all`): today nothing does, because reaching that handler needs a fully wired proxy with a live upstream manager. Build on the harness this phase adds, in `internal/server/mcp_routing_test.go` - -**Checkpoint**: US3 independently testable and complete. Only now may a pin lift. +- [x] T020 [P] [US3] Write failing tests that `tools/list` is byte-identical across two connections in both eras, in `internal/server/stateless_list_test.go` +- [x] T021 [P] [US3] Write a failing test that `prompts/list` does not change after `set_profile` — this fails on `main` today (FR-012) — in `internal/server/stateless_list_test.go` +- [x] T022 [P] [US3] Write a failing test that a modern **stdio** request cannot persist a profile selection despite its non-empty session id, in `internal/server/profile_era_test.go` +- [x] T023 [US3] Add `resolveActiveProfileForList` that skips the session tier, in `internal/server/profile_resolver.go` +- [x] T024 [US3] Switch the two list filters to the list-only resolver in `internal/server/mcp_direct_scope.go` +- [x] T025 [US3] Gate the session tier on `!server.IsModernRequest(ctx)` in addition to a non-empty id, in `internal/server/profile_resolver.go` +- [x] T026 [US3] Validate the slug before the session guard, and return machine-actionable guidance naming the `/mcp/p/` form on modern requests, in `internal/server/profile_tool.go` +- [x] T027 [US3] Split `TestProfile_SetProfileSessionScoped` into legacy and modern variants, keeping the original name for the legacy case so the spec's named gate still resolves, in `internal/server/profile_integration_test.go` +- [x] T028 [US3] Fix `TestProfile_SetProfileUnknown` to assert the unknown-slug message in both eras, in `internal/server/profile_integration_test.go` +- [x] T029 [P] [US3] Write a failing test that one client cannot cancel another's in-flight request when both use the same JSON-RPC id, in `internal/server/cancel_isolation_test.go` +- [x] T030 [US3] Add the cross-client cancellation guard (R1) and file the upstream mcp-go issue, touching `internal/server/mcp.go` — guard landed (`modernCancellationGuard`); the issue is DRAFTED in `upstream-issues/mcp-go-inflight-cancel-collision.md` because `mark3labs/mcp-go` restricted interactions to collaborators on 2026-10-20; file it when the restriction lifts +- [x] T031 [P] [US3] Write a failing `-tags server` test that two OAuth users with differing entitlements are isolated with no session, in `internal/serveredition/api/entitlement_stateless_test.go` +- [x] T032 [P] [US3] Write a failing test that modern-era client name/version reach the activity record via `RequestProtocolInfoFromContext`, in `internal/server/modern_attribution_test.go` +- [x] T033 [US3] Capture client identity from request protocol info in the `BeforeAny` hook so activation telemetry survives the loss of `AfterInitialize`, in `internal/server/mcp.go` +- [x] T034 [P] [US3] Correct the false "stable session id" comment in `internal/server/profile_resolver.go` and the false "background cleanup calls RemoveSession" comment in `internal/server/session_store.go` — the second had already been removed by #1497; the `AfterInitialize` hook comment in `mcp.go` was corrected instead (it never fires in the modern era) +- [x] T035 [P] [US3] Update the profile docs for era behaviour in `docs/features/profiles.md` +- [x] T036 [US3] Decide and record whether per-session token stats become legacy-only or are re-keyed on request-carried identity, in `specs/058-mcp-2026-upgrade/research.md` — legacy-only (research D11) +- [x] T036a [US3] Close the stdio era gap: mcp-go offers no stdio equivalent of the Streamable HTTP version pin, and the era is chosen per request by the shared handler, so a stdio client can opt into `2026-07-28` today. Reject a modern `_meta` protocol version on the stdio surface (an `onRequestInitialization` hook or middleware on the server at `internal/server/server.go:985`), with a failing test first +- [x] T036b [US3] Assert the direct-mode handler actually calls `proxyResultEnvelope` (FR-016b on `/mcp/all`): today nothing does, because reaching that handler needs a fully wired proxy with a live upstream manager. Build on the harness this phase adds, in `internal/server/mcp_routing_test.go` + +**Checkpoint**: US3 independently testable and complete (landed 2026-10-20, research D11). Only now may a pin lift. --- From cf5d238c28fa469cecd2b36f07717ca4cebda566 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:17:57 +0300 Subject: [PATCH 15/19] fix(server): era-gate the session write paths and bound the modern client recorder (review round 1) Cross-review findings, both latent until T041 lifts the pin: a modern stdio request (session id "stdio") could still write the session's latest profile resolution and mark it worked; and the per-identity debounce map grew without limit on client-supplied names. recordSessionResolution and markWorkIfToolCall now skip modern requests; the recorder is capped at 1024 identities with a stale sweep. --- internal/server/mcp.go | 35 ++++++++++++++++++---- internal/server/modern_attribution_test.go | 25 ++++++++++++++++ internal/server/profile_era_test.go | 12 ++++++++ internal/server/profile_resolver_v3.go | 4 ++- internal/server/workspace.go | 7 +++++ 5 files changed, 77 insertions(+), 6 deletions(-) diff --git a/internal/server/mcp.go b/internal/server/mcp.go index 23c07cf98..e7f4e67ae 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -449,27 +449,52 @@ func rejectUnservedProtocolOnStdio(ctx context.Context, message any) error { // modernClientRecorder debounces the per-request client identity of modern // (2026-07-28) requests, so the activation funnel and the client-seen presence // are fed once per distinct client per window rather than once per request. -// Bounded: an entry per distinct (name, version) ever seen by this process, -// which is the same cardinality the session store's client names have. +// +// The identity is client-supplied, so the map is bounded explicitly rather +// than by trust: at modernClientRecorderCap entries, sightings older than the +// window are swept, and if the map is still full the new identity is NOT +// recorded (and not fed to the funnels). A client inventing a fresh name per +// request therefore costs one map entry per name up to the cap and nothing +// after, instead of unbounded memory plus a BBolt write per request. type modernClientRecorder struct { mu sync.Mutex seen map[string]time.Time now func() time.Time } +// modernClientRecorderCap bounds distinct (name, version) pairs tracked at +// once. Real deployments see a handful of client identities; the cap only +// matters under a flood of invented names. +const modernClientRecorderCap = 1024 + func newModernClientRecorder() *modernClientRecorder { return &modernClientRecorder{seen: map[string]time.Time{}, now: time.Now} } // firstSightingWithin reports whether (name, version) has not been recorded in -// the last window, recording it when so. +// the last window, recording it when so. It returns false, recording nothing, +// when the map is at capacity with no stale entry to evict. func (r *modernClientRecorder) firstSightingWithin(name, version string, window time.Duration) bool { key := name + "\x00" + version now := r.now() r.mu.Lock() defer r.mu.Unlock() - if last, ok := r.seen[key]; ok && now.Sub(last) < window { - return false + if last, ok := r.seen[key]; ok { + if now.Sub(last) < window { + return false + } + r.seen[key] = now + return true + } + if len(r.seen) >= modernClientRecorderCap { + for k, last := range r.seen { + if now.Sub(last) >= window { + delete(r.seen, k) + } + } + if len(r.seen) >= modernClientRecorderCap { + return false + } } r.seen[key] = now return true diff --git a/internal/server/modern_attribution_test.go b/internal/server/modern_attribution_test.go index 7ae03b2be..9043c4456 100644 --- a/internal/server/modern_attribution_test.go +++ b/internal/server/modern_attribution_test.go @@ -2,6 +2,7 @@ package server import ( "context" + "fmt" "testing" "time" @@ -120,3 +121,27 @@ func TestModernAttribution_ReachesActivityRecord(t *testing.T) { assert.Greater(t, len(after.ClientLastSeen), seenBefore, "the client-seen funnel must observe a modern client although initialize never ran") } + +// The recorder debounces per identity and is bounded against invented names +// (cross-review O1): past the cap, fresh identities are neither tracked nor +// reported, and stale ones are swept once the window has passed. +func TestModernAttribution_RecorderIsBounded(t *testing.T) { + now := time.Unix(1_700_000_000, 0) + r := newModernClientRecorder() + r.now = func() time.Time { return now } + + assert.True(t, r.firstSightingWithin("ide", "1.0", time.Minute)) + assert.False(t, r.firstSightingWithin("ide", "1.0", time.Minute), "a repeat within the window is debounced") + + for i := 1; i < modernClientRecorderCap; i++ { + require.True(t, r.firstSightingWithin("flood", fmt.Sprint(i), time.Minute)) + } + assert.Len(t, r.seen, modernClientRecorderCap) + assert.False(t, r.firstSightingWithin("one-too-many", "1", time.Minute), "a full map with nothing stale records nothing") + assert.Len(t, r.seen, modernClientRecorderCap) + + now = now.Add(2 * time.Minute) + assert.True(t, r.firstSightingWithin("after-sweep", "1", time.Minute), "stale entries are swept to make room") + assert.LessOrEqual(t, len(r.seen), modernClientRecorderCap) + assert.True(t, r.firstSightingWithin("ide", "1.0", time.Minute), "a known identity is re-reported once its window passed") +} diff --git a/internal/server/profile_era_test.go b/internal/server/profile_era_test.go index 2b1b76190..f50e3cb87 100644 --- a/internal/server/profile_era_test.go +++ b/internal/server/profile_era_test.go @@ -69,6 +69,18 @@ func TestProfileEra_SessionTierSkippedOnModernRequests(t *testing.T) { assert.Equal(t, "research", proxy.sessionStore.GetActiveProfile("stdio"), "skipping the tier is a read-side gate; a modern request neither applies nor clears a legacy selection") + + // Write side: a modern request records no resolution on the session row + // either (cross-review O2), even when it resolves a request-carried + // profile the legacy path would have recorded as the session's latest. + proxy.sessionStore.SetSession("stdio", "ide", "1.0", false, false, nil) + urlModern := profile.WithProfileScope(modern, profileScopeForSlugIn(proxy.currentConfig(), "research")) + res = proxy.ResolveProfileV3(urlModern, idx) + require.Equal(t, "research", res.Name, "the URL tier is request-carried and still applies") + info := proxy.sessionStore.GetSession("stdio") + require.NotNil(t, info) + assert.Equal(t, "", info.Profile, "a modern request must not write per-connection state") + assert.Equal(t, "", proxy.markWorkIfToolCall(urlModern, mcp.MethodToolsCall), "nor mark the connection as worked") } // ─── stdio surface ────────────────────────────────────────────────────────── diff --git a/internal/server/profile_resolver_v3.go b/internal/server/profile_resolver_v3.go index 992db6b05..309e1115f 100644 --- a/internal/server/profile_resolver_v3.go +++ b/internal/server/profile_resolver_v3.go @@ -123,7 +123,9 @@ func (p *MCPProxyServer) recordSessionResolution(ctx context.Context, res Profil if p.sessionStore == nil || ctx.Value(noResolutionRecordKey{}) != nil { return } - if sid := sessionIDFromContext(ctx); sid != "" { + // Era-gated (Spec 058 US3): a 2026-07-28 request has no session to record + // a resolution on, whatever id the transport reports (stdio: "stdio"). + if sid := profileSessionIDFromContext(ctx); sid != "" { p.sessionStore.UpdateSessionProfile(sid, res.Name, res.Source) } } diff --git a/internal/server/workspace.go b/internal/server/workspace.go index fdd2935f1..9aadac44f 100644 --- a/internal/server/workspace.go +++ b/internal/server/workspace.go @@ -242,6 +242,13 @@ func (p *MCPProxyServer) markWorkIfToolCall(ctx context.Context, method mcp.MCPM if method != mcp.MethodToolsCall { return "" } + // Spec 058 US3: a 2026-07-28 request belongs to no connection, so there + // is no session to mark as worked or persist (session records are a + // legacy-era feature — research D11), even on stdio, whose transport id + // is the constant "stdio" in both eras. + if mcpserver.IsModernRequest(ctx) { + return "" + } session := mcpserver.ClientSessionFromContext(ctx) if session == nil { return "" From aa9bd1fbf3a47b0c9c6f5c7f4c9e1582ea0d4570 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:29:48 +0300 Subject: [PATCH 16/19] fix(server): modern records carry metadata.client_name; era gate inside markSessionWorked (review round 1) Sol 6.1 findings: UsageAggregate keys per-client usage on metadata.client_name, which a request-carried identity never wrote; the stdio modern-selection test accepted any error; the collision test's skip branch was unreachable; the server-edition isolation test stated a broader scope than it exercised. Adds TestStatelessList_AgentTokensScopedPerRequest through the production auth middleware (SC-004) and moves the work-session era gate into markSessionWorked so every funnel agrees. --- internal/runtime/activity_attribution.go | 22 ++++++--- internal/server/cancel_isolation_test.go | 23 ++++++--- internal/server/modern_attribution_test.go | 2 + internal/server/profile_era_test.go | 15 ++++-- internal/server/stateless_list_test.go | 49 +++++++++++++++++++ internal/server/workspace.go | 16 +++--- .../api/entitlement_stateless_test.go | 16 ++++-- 7 files changed, 114 insertions(+), 29 deletions(-) diff --git a/internal/runtime/activity_attribution.go b/internal/runtime/activity_attribution.go index dce313f7a..6d414fb2c 100644 --- a/internal/runtime/activity_attribution.go +++ b/internal/runtime/activity_attribution.go @@ -99,14 +99,24 @@ func applyAttribution(rec *storage.ActivityRecord, a ActivityAttribution) { rec.ClientName = name } } - if a.ClientVersion != "" { - if _, ok := rec.Metadata["client_version"]; !ok { - if rec.Metadata == nil { - rec.Metadata = make(map[string]interface{}) - } - rec.Metadata["client_version"] = a.ClientVersion + // A request-carried identity (Spec 058 US3) also lands under the metadata + // keys withClientInfo writes for a session-resolved one, because readers + // such as UsageAggregate.applyClientCalls key per-client usage on + // metadata.client_name, not on the first-class field. + setMetadataIfAbsent := func(key, value string) { + if value == "" { + return } + if _, ok := rec.Metadata[key]; ok { + return + } + if rec.Metadata == nil { + rec.Metadata = make(map[string]interface{}) + } + rec.Metadata[key] = value } + setMetadataIfAbsent("client_name", a.ClientName) + setMetadataIfAbsent("client_version", a.ClientVersion) } // NewUsageAggregate returns an empty aggregate at the current admission diff --git a/internal/server/cancel_isolation_test.go b/internal/server/cancel_isolation_test.go index 0aef74467..9e8f8ca93 100644 --- a/internal/server/cancel_isolation_test.go +++ b/internal/server/cancel_isolation_test.go @@ -143,7 +143,7 @@ func callSucceeded(body []byte) bool { } // Documents the library defect the guard exists for, so the guard can be -// retired the day this test FAILS: against an unguarded mcp-go v1.0.0 +// retired the day this test SKIPS: against an unguarded mcp-go v1.0.0 // transport, client B's notifications/cancelled for id 1 cancels client A's // in-flight call with id 1. Both are modern, so both run under session id "". func TestCancelIsolation_LibraryCollisionStillPresent(t *testing.T) { @@ -160,13 +160,24 @@ func TestCancelIsolation_LibraryCollisionStillPresent(t *testing.T) { resp, _ := b.send(string(mcp.MethodNotificationCancelled), cancelledNotificationParams(1), false) require.Equal(t, http.StatusAccepted, resp.StatusCode) - outcome := awaitOutcome(t, inflight) - if callSucceeded(outcome.body) { - t.Skip("mcp-go no longer lets one modern client cancel another's request: " + - "the upstream fix has landed — retire modernCancellationGuard and this test together") + // A bounded window for a cross-client cancellation to land. If A is still + // running afterwards, the library no longer collides: release A, require + // the released result, and skip — the signal to retire the guard. + select { + case r := <-inflight: + require.NoError(t, r.err) + assert.False(t, callSucceeded(r.body), "A ended before release, so it must not carry the released result: %s", r.body) + assert.Contains(t, string(r.body), "context canceled", + "A must have ended by cancellation, not by some other failure: %s", r.body) + close(probe.release) + return + case <-time.After(500 * time.Millisecond): } - // Reaching here is the defect: A's call ended without ever being released. close(probe.release) + outcome := awaitOutcome(t, inflight) + require.True(t, callSucceeded(outcome.body), "A was not cancelled yet did not complete when released: %s", outcome.body) + t.Skip("mcp-go no longer lets one modern client cancel another's request: " + + "the upstream fix has landed — retire modernCancellationGuard and this test together") } // The guard: with mcpproxy's middleware in the chain, B's cancellation is diff --git a/internal/server/modern_attribution_test.go b/internal/server/modern_attribution_test.go index 9043c4456..cc97c3068 100644 --- a/internal/server/modern_attribution_test.go +++ b/internal/server/modern_attribution_test.go @@ -113,6 +113,8 @@ func TestModernAttribution_ReachesActivityRecord(t *testing.T) { record := awaitToolCallActivity(t, rt, "attrib-srv", "echo") assert.Equal(t, "era-probe", record.ClientName, "the modern client's self-reported name must reach the record") assert.Equal(t, "1.0.0", record.Metadata["client_version"], "and so must its version") + assert.Equal(t, "era-probe", record.Metadata["client_name"], + "per-client usage accounting (UsageAggregate.applyClientCalls) reads metadata.client_name, so a modern record needs the legacy shape") assert.Empty(t, record.SessionID, "no session was involved") after, err := rt.StorageManager().GetOnboardingState() diff --git a/internal/server/profile_era_test.go b/internal/server/profile_era_test.go index f50e3cb87..0de911733 100644 --- a/internal/server/profile_era_test.go +++ b/internal/server/profile_era_test.go @@ -81,6 +81,7 @@ func TestProfileEra_SessionTierSkippedOnModernRequests(t *testing.T) { require.NotNil(t, info) assert.Equal(t, "", info.Profile, "a modern request must not write per-connection state") assert.Equal(t, "", proxy.markWorkIfToolCall(urlModern, mcp.MethodToolsCall), "nor mark the connection as worked") + assert.Equal(t, "", proxy.markSessionWorked(urlModern, "stdio"), "whichever funnel marks the work") } // ─── stdio surface ────────────────────────────────────────────────────────── @@ -215,11 +216,15 @@ func TestProfileEra_ModernStdioRequestCannotPersistSelection(t *testing.T) { resp := probe.request(string(mcp.MethodToolsCall), map[string]any{ "name": "set_profile", "arguments": map[string]any{"profile": "research"}, "_meta": modernMeta(), }) - if resp["error"] == nil { - result, ok := resp["result"].(map[string]any) - require.True(t, ok, "response must carry a result or an error: %v", resp) - assert.Equal(t, true, result["isError"], "a modern request must not be able to select a session profile: %v", result) - } + // Under the FR-028 pin the stdio era guard (T036a) refuses the request + // before dispatch, so the observable is that exact refusal — not "any + // error", which a hidden tool or a transport failure would also + // produce. The handler-level refusal for a modern request that IS + // dispatched is pinned by TestProfileEra_SetProfileModernRequestGuidance. + errObj, ok := resp["error"].(map[string]any) + require.True(t, ok, "the stdio era guard must refuse the request as a JSON-RPC error: %v", resp) + assert.EqualValues(t, mcp.INVALID_REQUEST, errObj["code"]) + assert.Contains(t, errObj["message"], mcp.LATEST_PROTOCOL_VERSION) assert.Equal(t, "", proxy.sessionStore.GetActiveProfile("stdio"), "FR-011: a modern request must not persist per-connection state, despite stdio's non-empty session id") }) diff --git a/internal/server/stateless_list_test.go b/internal/server/stateless_list_test.go index c861f0b1c..409063de7 100644 --- a/internal/server/stateless_list_test.go +++ b/internal/server/stateless_list_test.go @@ -3,6 +3,7 @@ package server import ( "bufio" "bytes" + "context" "encoding/json" "io" "net/http" @@ -11,6 +12,8 @@ import ( "testing" "time" + "github.com/mark3labs/mcp-go/client" + "github.com/mark3labs/mcp-go/client/transport" "github.com/mark3labs/mcp-go/mcp" mcpserver "github.com/mark3labs/mcp-go/server" "github.com/stretchr/testify/assert" @@ -266,6 +269,52 @@ func TestStatelessList_ToolsListIdenticalAcrossConnections(t *testing.T) { }) } +// US3 acceptance scenario 2 / SC-004, through the production chain: two agent +// tokens with different server scopes, each pinned to the modern era over the +// era harness (mcpAuthMiddleware included), see only their permitted tools on +// identity-scoped discovery, interleaved, with no session involved. Identity +// is re-derived from the Authorization header on every request. +func TestStatelessList_AgentTokensScopedPerRequest(t *testing.T) { + env := newProfileTestEnv(t) + ctx := context.Background() + url := env.eraEndpoint() + tokenA := mintProfileAgentToken(t, env, "scope-a", []string{"research-srv"}, []string{auth.PermRead}, "") + tokenB := mintProfileAgentToken(t, env, "scope-b", []string{"deploy-srv"}, []string{auth.PermRead}, "") + + connect := func(token string) *client.Client { + tr, err := transport.NewStreamableHTTP(url, transport.WithHTTPHeaders(map[string]string{"Authorization": "Bearer " + token})) + require.NoError(t, err) + c := client.NewClient(tr, modernEra().options...) + env.initClient(c) + requireEra(t, c, modernEra()) + require.Empty(t, tr.GetSessionId(), "the modern era binds no session; scoping must come from the request") + return c + } + hasAny := func(names []string, needles ...string) bool { + for _, n := range names { + for _, needle := range needles { + if strings.Contains(n, needle) { + return true + } + } + } + return false + } + a := connect(tokenA) + b := connect(tokenB) + + query := "search deploy papers rollback app" + first := env.retrieveTools(ctx, a, query) + require.True(t, hasAny(first, "search_papers", "fetch_article"), "A must see its research tools: %v", first) + require.False(t, hasAny(first, "deploy_app", "rollback"), "A must not see deploy tools: %v", first) + + viewB := env.retrieveTools(ctx, b, query) + require.True(t, hasAny(viewB, "deploy_app", "rollback"), "B must see its deploy tools: %v", viewB) + require.False(t, hasAny(viewB, "search_papers", "fetch_article"), "B must not see research tools: %v", viewB) + + assert.Equal(t, first, env.retrieveTools(ctx, a, query), "A's view must not drift after B's requests") +} + // ─── the two list filters skip the session tier (T020 unit / T021) ────────── // statelessListProxy is a bare proxy over two servers and one profile, enough diff --git a/internal/server/workspace.go b/internal/server/workspace.go index 9aadac44f..e067c682a 100644 --- a/internal/server/workspace.go +++ b/internal/server/workspace.go @@ -188,6 +188,15 @@ func (p *MCPProxyServer) markSessionWorked(ctx context.Context, sessionID string if sessionID == "" || p.sessionStore == nil { return "" } + // Spec 058 US3: a 2026-07-28 request belongs to no connection, so there + // is no session to mark as worked or persist (session records are a + // legacy-era feature, research D11) — even on stdio, whose transport id + // is the constant "stdio" in both eras. Gated here, not at the call + // sites, so every funnel that marks work (retrieve_tools, call_tool_*, + // code_execution, direct dispatch, the BeforeAny hook) agrees. + if mcpserver.IsModernRequest(ctx) { + return "" + } // Wait (briefly, once) for the project, so this connection's first record is // not filed under a different work session than its second. @@ -242,13 +251,6 @@ func (p *MCPProxyServer) markWorkIfToolCall(ctx context.Context, method mcp.MCPM if method != mcp.MethodToolsCall { return "" } - // Spec 058 US3: a 2026-07-28 request belongs to no connection, so there - // is no session to mark as worked or persist (session records are a - // legacy-era feature — research D11), even on stdio, whose transport id - // is the constant "stdio" in both eras. - if mcpserver.IsModernRequest(ctx) { - return "" - } session := mcpserver.ClientSessionFromContext(ctx) if session == nil { return "" diff --git a/internal/serveredition/api/entitlement_stateless_test.go b/internal/serveredition/api/entitlement_stateless_test.go index 8115be7f8..e65a38b37 100644 --- a/internal/serveredition/api/entitlement_stateless_test.go +++ b/internal/serveredition/api/entitlement_stateless_test.go @@ -31,11 +31,17 @@ import ( // two OAuth users with differing entitlements over modern requests, where no // session exists to carry identity between calls. // -// The identity path under test is the one setup.go wires for the MCP surface -// (buildSessionPrincipalResolver): a bearer JWT on each request → the user -// record → UserHandlers.ResolveAgentTokenOwner, THE entitlement predicate → -// an AuthContext whose AllowedServers is the tenant's entitlement. Every -// input is request-carried; the test proves the resulting tool view is too. +// What this test exercises, precisely: the production entitlement predicate +// (UserHandlers.ResolveAgentTokenOwner, the same call setup.go's +// buildSessionPrincipalResolver makes) derived per request from a bearer JWT, +// and the stateless era of the mcp-go transport. The JWT → AuthContext step +// and the tool filter are test-local stand-ins, because this package cannot +// import internal/server (import cycle through serveredition_wire.go), so the +// production MCP auth middleware and direct-surface filters are NOT on this +// path. Their modern-era behaviour is pinned in internal/server: +// TestStatelessList_AgentTokensScopedPerRequest drives two agent tokens with +// differing allowed_servers — the credential a server-edition user actually +// reaches /mcp with — through mcpAuthMiddleware over the era harness. // // Test names avoid the substring "MCP" (release-qa-gate.yml skips on it). From 1fb4ea6abce4b1b12ea128f3450bdcf629176d8d Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 13:30:35 +0300 Subject: [PATCH 17/19] docs(spec): record cross-review round 1 outcome in research D11 --- specs/058-mcp-2026-upgrade/research.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/specs/058-mcp-2026-upgrade/research.md b/specs/058-mcp-2026-upgrade/research.md index e7c6170f8..9ff912997 100644 --- a/specs/058-mcp-2026-upgrade/research.md +++ b/specs/058-mcp-2026-upgrade/research.md @@ -227,3 +227,17 @@ Phase 3 landed under the FR-028 pin, on an era harness that reaches the modern e **FR-016b on `/mcp/all` is now asserted (T036b)** over the live direct routing fixture: an upstream `_meta` payload key is relayed while `serverInfo` and `protocolVersion` are dropped. **T034.** The resolver comment was rewritten with the gate; the `session_store.go` comment D1 named had already been removed by #1497 (its surviving claim — the closed-entry TTL matches the storage inactivity timeout — is true). The `AfterInitialize` hook comment now says it never fires in the modern era. + +**Cross-review, round 1 (2026-10-20).** zcode (GLM) could not run: its project token login had expired (`project_token_login_failed`), which only a human `zcode login` fixes. The round ran on opencode `github-copilot/gpt-6.1-sol` instead (the recorded default reviewer), in three file-named chunks with explicit permissions and a no-growth watchdog; every chunk returned a verdict. Seven findings in all, each verified against the tree before acting: + +| # | Finding | Action | +|---|---|---| +| O1 | `modernClientRecorder` map unbounded on client-supplied names (latent until T041) | fixed: cap 1024 + stale sweep, test | +| O2 | `recordSessionResolution` / `markSessionWorked` keyed on the raw session id, so a modern stdio request could still write per-connection state (latent until T041) | fixed: era gate, inside `markSessionWorked` so all seven funnels agree; test proven red-then-green | +| A1 | the modern-stdio selection test accepted any JSON-RPC error | fixed: asserts the stdio guard's exact INVALID_REQUEST refusal | +| B1 | the cancellation guard buffers an unbounded body before the transport's era rejection | **declined**: mcp-go reads the whole body (`streamable_http.go:410`) before `detectRequestEra` on every POST, pinned or not, so the guard adds no exposure; introducing a body cap is a product decision outside US3 | +| B2 | the collision test's skip branch was unreachable and accepted any failure as "cancelled" | fixed: bounded window, `context canceled` asserted, release on every path | +| C1 | modern records lacked `metadata.client_name`, so `UsageAggregate.applyClientCalls` never counted them | fixed in `applyAttribution`, asserted end to end | +| C2 | the server-edition test used test-local JWT→AuthContext and filter stand-ins | scope stated accurately (package `api` cannot import `internal/server`); production-chain coverage added as `TestStatelessList_AgentTokensScopedPerRequest` (two agent tokens through `mcpAuthMiddleware`, SC-004) | + +Reviewer-noted OUT OF SCOPE: `filterProfileV3Tools` (see the residual above) and the fact that `mcpAuthMiddleware` does not accept OAuth bearer JWTs on `/mcp` (server-edition users reach MCP with per-user agent tokens, which is what the new test uses). From 1643dfb262e63683595ea09d3b23dac5c70e9797 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 17:10:45 +0300 Subject: [PATCH 18/19] test(server): storage-backed proof that modern requests mark no work session (review round 2) The earlier assertion ran over a session store with no storage manager, where EnsurePersisted returns "" regardless of the era gate. The new test uses the storage-backed work-session fixture with a counting resolver and a legacy control; removing the gate in markSessionWorked fails it. --- internal/server/profile_era_test.go | 39 ++++++++++++++++++++++++-- specs/058-mcp-2026-upgrade/research.md | 2 ++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/internal/server/profile_era_test.go b/internal/server/profile_era_test.go index 0de911733..52f9e22fc 100644 --- a/internal/server/profile_era_test.go +++ b/internal/server/profile_era_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "io" + "sync/atomic" "testing" "time" @@ -15,6 +16,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime" ) // Spec 058 US3 (FR-011): under the 2026-07-28 revision there is no session, so @@ -80,8 +82,41 @@ func TestProfileEra_SessionTierSkippedOnModernRequests(t *testing.T) { info := proxy.sessionStore.GetSession("stdio") require.NotNil(t, info) assert.Equal(t, "", info.Profile, "a modern request must not write per-connection state") - assert.Equal(t, "", proxy.markWorkIfToolCall(urlModern, mcp.MethodToolsCall), "nor mark the connection as worked") - assert.Equal(t, "", proxy.markSessionWorked(urlModern, "stdio"), "whichever funnel marks the work") +} + +// The work-session half of the write side (cross-review O2, round 2): over a +// STORAGE-BACKED session store — a bare one returns "" from EnsurePersisted +// regardless, which made the earlier assertion vacuous — a legacy tools/call +// on the stdio session persists it and resolves a work session, while a modern +// request on the same session id resolves nothing, invokes no resolver and +// leaves the row unpersisted. +func TestProfileEra_ModernRequestDoesNotMarkWork(t *testing.T) { + newProxy := func(t *testing.T) (*MCPProxyServer, *SessionStore, *atomic.Int32) { + p, store := newWorkSessionProxy(t) + var calls atomic.Int32 + p.workSessionResolver = func(id runtime.WorkSessionIdentity) string { + calls.Add(1) + return "ws-" + id.ClientName + } + store.SetSession("stdio", "ide", "1.0", false, false, nil) + return p, store, &calls + } + + t.Run("legacy control persists and resolves", func(t *testing.T) { + p, store, calls := newProxy(t) + require.Equal(t, "ws-ide", p.markWorkIfToolCall(workSessionCtx("stdio"), mcp.MethodToolsCall)) + require.EqualValues(t, 1, calls.Load()) + require.True(t, store.GetSession("stdio").persisted, "the legacy call earns the durable row") + }) + + t.Run("modern request on the same session id marks nothing", func(t *testing.T) { + p, store, calls := newProxy(t) + modern := modernRequestCtx(workSessionCtx("stdio")) + assert.Equal(t, "", p.markWorkIfToolCall(modern, mcp.MethodToolsCall)) + assert.Equal(t, "", p.markSessionWorked(modern, "stdio"), "whichever funnel marks the work") + assert.Zero(t, calls.Load(), "no work session may be resolved for a modern request") + assert.False(t, store.GetSession("stdio").persisted, "no session row may be persisted for a modern request") + }) } // ─── stdio surface ────────────────────────────────────────────────────────── diff --git a/specs/058-mcp-2026-upgrade/research.md b/specs/058-mcp-2026-upgrade/research.md index 9ff912997..867691e04 100644 --- a/specs/058-mcp-2026-upgrade/research.md +++ b/specs/058-mcp-2026-upgrade/research.md @@ -241,3 +241,5 @@ Phase 3 landed under the FR-028 pin, on an era harness that reaches the modern e | C2 | the server-edition test used test-local JWT→AuthContext and filter stand-ins | scope stated accurately (package `api` cannot import `internal/server`); production-chain coverage added as `TestStatelessList_AgentTokensScopedPerRequest` (two agent tokens through `mcpAuthMiddleware`, SC-004) | Reviewer-noted OUT OF SCOPE: `filterProfileV3Tools` (see the residual above) and the fact that `mcpAuthMiddleware` does not accept OAuth bearer JWTs on `/mcp` (server-edition users reach MCP with per-user agent tokens, which is what the new test uses). + +**Cross-review, round 2 (incremental, Sol 6.1, 2026-10-20).** One finding, verified: the work-session era assertions in `TestProfileEra_SessionTierSkippedOnModernRequests` passed without the gate, because the bare fixture's session store has no storage manager and `EnsurePersisted` returns `""` regardless. Replaced by `TestProfileEra_ModernRequestDoesNotMarkWork` over the storage-backed `newWorkSessionProxy` with a counting resolver and a legacy control; removing the gate in `markSessionWorked` fails it (proven). Everything else in the round-1 fix diff was examined and found sound. From 1c9271143331311496d7a61bfc35e2f2fad7b60a Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 20 Oct 2026 17:17:00 +0300 Subject: [PATCH 19/19] docs(spec): record cross-review round 3 (clean) in research D11 --- specs/058-mcp-2026-upgrade/research.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/058-mcp-2026-upgrade/research.md b/specs/058-mcp-2026-upgrade/research.md index 867691e04..6e0c814d5 100644 --- a/specs/058-mcp-2026-upgrade/research.md +++ b/specs/058-mcp-2026-upgrade/research.md @@ -243,3 +243,5 @@ Phase 3 landed under the FR-028 pin, on an era harness that reaches the modern e Reviewer-noted OUT OF SCOPE: `filterProfileV3Tools` (see the residual above) and the fact that `mcpAuthMiddleware` does not accept OAuth bearer JWTs on `/mcp` (server-edition users reach MCP with per-user agent tokens, which is what the new test uses). **Cross-review, round 2 (incremental, Sol 6.1, 2026-10-20).** One finding, verified: the work-session era assertions in `TestProfileEra_SessionTierSkippedOnModernRequests` passed without the gate, because the bare fixture's session store has no storage manager and `EnsurePersisted` returns `""` regardless. Replaced by `TestProfileEra_ModernRequestDoesNotMarkWork` over the storage-backed `newWorkSessionProxy` with a counting resolver and a legacy control; removing the gate in `markSessionWorked` fails it (proven). Everything else in the round-1 fix diff was examined and found sound. + +**Cross-review, round 3 (incremental, Sol 6.1, 2026-10-20).** On the round-2 test fix alone: VERDICT: CLEAN; the reviewer confirmed the new test depends on the `IsModernRequest` gate in `markSessionWorked` and that the legacy control is meaningful.