From d85134c1e3d54d5f7635b393e689dc63befadf46 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 15:01:44 +0100 Subject: [PATCH 01/18] feat(mcp): add mount path helpers for multi-server mounts --- router/pkg/mcpserver/paths.go | 57 ++++++++++++++++++++++++++++++ router/pkg/mcpserver/paths_test.go | 55 ++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 router/pkg/mcpserver/paths.go create mode 100644 router/pkg/mcpserver/paths_test.go diff --git a/router/pkg/mcpserver/paths.go b/router/pkg/mcpserver/paths.go new file mode 100644 index 0000000000..a02331c40d --- /dev/null +++ b/router/pkg/mcpserver/paths.go @@ -0,0 +1,57 @@ +package mcpserver + +import ( + "fmt" + "strings" +) + +const ( + // MetadataPathPrefix is the RFC 9728 well-known prefix for OAuth 2.0 + // Protected Resource Metadata. The resource path is appended to it. + MetadataPathPrefix = "/.well-known/oauth-protected-resource" + + // DefaultMountPath is the path an MCP server uses when the config sets none. + DefaultMountPath = "/mcp" +) + +// ValidateMountPath reports why p cannot serve as an MCP mount path. +// +// Mount paths must be exact ServeMux patterns. A trailing slash would make a +// subtree pattern, and a wildcard would make a conflicting pattern; either can +// capture the requests of another server sharing the mux. +func ValidateMountPath(p string) error { + if p == "" { + return fmt.Errorf("path is empty") + } + if !strings.HasPrefix(p, "/") { + return fmt.Errorf("path %q must start with /", p) + } + if len(p) > 1 && strings.HasSuffix(p, "/") { + return fmt.Errorf("path %q must not end with /", p) + } + if strings.ContainsAny(p, "{}*") { + return fmt.Errorf("path %q must not contain a wildcard", p) + } + if p == MetadataPathPrefix || strings.HasPrefix(p, MetadataPathPrefix+"/") { + return fmt.Errorf("path %q is reserved for OAuth metadata", p) + } + return nil +} + +// MetadataPath returns the RFC 9728 metadata path for a mount path. +func MetadataPath(mountPath string) string { + if mountPath == "/" { + return MetadataPathPrefix + } + return MetadataPathPrefix + mountPath +} + +// ResourceIdentifier returns the OAuth 2.0 resource identifier a server +// publishes: its external origin joined to its mount path. +func ResourceIdentifier(baseURL, mountPath string) string { + base := strings.TrimRight(baseURL, "/") + if mountPath == "/" { + return base + "/" + } + return base + mountPath +} diff --git a/router/pkg/mcpserver/paths_test.go b/router/pkg/mcpserver/paths_test.go new file mode 100644 index 0000000000..2a4f71d34c --- /dev/null +++ b/router/pkg/mcpserver/paths_test.go @@ -0,0 +1,55 @@ +package mcpserver + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidateMountPath(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + path string + wantErr string + }{ + {name: "simple", path: "/mcp"}, + {name: "nested", path: "/billing/mcp"}, + {name: "root", path: "/"}, + {name: "empty", path: "", wantErr: "path is empty"}, + {name: "no leading slash", path: "mcp", wantErr: "must start with /"}, + {name: "trailing slash", path: "/mcp/", wantErr: "must not end with /"}, + {name: "wildcard", path: "/mcp/{id}", wantErr: "must not contain"}, + {name: "reserved metadata prefix", path: "/.well-known/oauth-protected-resource/x", wantErr: "reserved"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + err := ValidateMountPath(tc.path) + if tc.wantErr == "" { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, tc.wantErr) + }) + } +} + +func TestMetadataPath(t *testing.T) { + t.Parallel() + + require.Equal(t, "/.well-known/oauth-protected-resource/mcp", MetadataPath("/mcp")) + require.Equal(t, "/.well-known/oauth-protected-resource/billing/mcp", MetadataPath("/billing/mcp")) + require.Equal(t, "/.well-known/oauth-protected-resource", MetadataPath("/")) +} + +func TestResourceIdentifier(t *testing.T) { + t.Parallel() + + require.Equal(t, "https://example.com/mcp", ResourceIdentifier("https://example.com", "/mcp")) + require.Equal(t, "https://example.com/billing/mcp", ResourceIdentifier("https://example.com/", "/billing/mcp")) + require.Equal(t, "https://example.com/", ResourceIdentifier("https://example.com", "/")) +} From 844d887935d45768e25b278629e742fb1620c620 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 15:08:21 +0100 Subject: [PATCH 02/18] refactor(mcp): give the MCP server its own mount path and route registration --- router/pkg/mcpserver/server.go | 75 ++++++++++++++++++++--------- router/pkg/mcpserver/server_test.go | 43 +++++++++++++++++ 2 files changed, 94 insertions(+), 24 deletions(-) diff --git a/router/pkg/mcpserver/server.go b/router/pkg/mcpserver/server.go index 1a118a6858..f8d6dcf853 100644 --- a/router/pkg/mcpserver/server.go +++ b/router/pkg/mcpserver/server.go @@ -70,6 +70,8 @@ type Options struct { OperationsDir string // ListenAddr is the address where the server should listen to ListenAddr string + // MountPath is the HTTP path this server answers on. + MountPath string // Enabled determines whether the MCP server should be started Enabled bool // Logger is the logger to be used @@ -118,6 +120,7 @@ type GraphQLSchemaServer struct { graphName string operationsDir string listenAddr string + mountPath string logger *zap.Logger httpClient *http.Client requestTimeout time.Duration @@ -223,6 +226,7 @@ func NewGraphQLSchemaServer(ctx context.Context, routerGraphQLEndpoint string, o GraphName: "graph", OperationsDir: "operations", ListenAddr: "0.0.0.0:5025", + MountPath: DefaultMountPath, Enabled: false, Logger: zap.NewNop(), RequestTimeout: 30 * time.Second, @@ -238,6 +242,11 @@ func NewGraphQLSchemaServer(ctx context.Context, routerGraphQLEndpoint string, o ctx, cancel := context.WithCancel(ctx) + if err := ValidateMountPath(options.MountPath); err != nil { + cancel() + return nil, fmt.Errorf("invalid mcp mount path: %w", err) + } + var authMiddleware *MCPAuthMiddleware if options.OAuthConfig != nil && options.OAuthConfig.Enabled { if len(options.OAuthConfig.JWKS) == 0 { @@ -329,6 +338,7 @@ func NewGraphQLSchemaServer(ctx context.Context, routerGraphQLEndpoint string, o graphName: options.GraphName, operationsDir: options.OperationsDir, listenAddr: options.ListenAddr, + mountPath: options.MountPath, logger: options.Logger, httpClient: httpClient, requestTimeout: options.RequestTimeout, @@ -404,6 +414,13 @@ func WithListenAddr(listenAddr string) func(*Options) { } } +// WithMountPath sets the HTTP path this MCP server answers on. +func WithMountPath(p string) func(*Options) { + return func(o *Options) { + o.MountPath = p + } +} + func WithLogger(logger *zap.Logger) func(*Options) { return func(o *Options) { o.Logger = logger @@ -480,18 +497,14 @@ func WithResourceDocumentation(url string) func(*Options) { } } -// Serve starts the server with the configured options and returns the HTTP server. -func (s *GraphQLSchemaServer) Serve() (*http.Server, error) { - // Create custom HTTP server - httpServer := &http.Server{ - Addr: s.listenAddr, - ReadTimeout: 30 * time.Second, - WriteTimeout: 30 * time.Second, - IdleTimeout: 60 * time.Second, - } +// MountPath returns the HTTP path this server answers on. +func (s *GraphQLSchemaServer) MountPath() string { + return s.mountPath +} - // Create MCP streamable HTTP handler - // The getServer function returns our MCP server instance for each request +// RegisterRoutes mounts this server's handlers on mux. The caller supplies the +// middleware (CORS) so that every server on a shared mux is wrapped the same way. +func (s *GraphQLSchemaServer) RegisterRoutes(mux *http.ServeMux, middleware func(http.Handler) http.Handler) { // Disable the SDK's built-in cross-origin protection (Sec-Fetch-Site check) // because the router already applies its own CORS middleware around the handler. cop := http.NewCrossOriginProtection() @@ -507,13 +520,9 @@ func (s *GraphQLSchemaServer) Serve() (*http.Server, error) { }, ) - middleware := cors.New(s.corsConfig) - - mux := http.NewServeMux() - - // OAuth 2.0 Protected Resource Metadata (RFC 9728) — public discovery endpoint + // OAuth 2.0 Protected Resource Metadata (RFC 9728), public discovery endpoint. if s.oauthConfig != nil && s.oauthConfig.Enabled && s.oauthConfig.AuthorizationServerURL != "" { - mux.Handle("/.well-known/oauth-protected-resource/mcp", middleware(http.HandlerFunc(s.handleProtectedResourceMetadata))) + mux.Handle(MetadataPath(s.mountPath), middleware(http.HandlerFunc(s.handleProtectedResourceMetadata))) } // Inject request headers into context so tool handlers can forward them @@ -523,24 +532,42 @@ func (s *GraphQLSchemaServer) Serve() (*http.Server, error) { streamableHTTPHandler.ServeHTTP(w, r) }) if s.authMiddleware != nil { - mux.Handle("/mcp", middleware(s.authMiddleware.HTTPMiddleware(mcpHandler))) + mux.Handle(s.mountPath, middleware(s.authMiddleware.HTTPMiddleware(mcpHandler))) } else { - mux.Handle("/mcp", middleware(mcpHandler)) + mux.Handle(s.mountPath, middleware(mcpHandler)) } +} +// Close releases the background context of this server, stopping work such as +// JWKS key refresh. It does not touch any HTTP listener. +func (s *GraphQLSchemaServer) Close() { + if s.cancel != nil { + s.cancel() + } +} + +// Serve starts the server with the configured options and returns the HTTP server. +func (s *GraphQLSchemaServer) Serve() (*http.Server, error) { + httpServer := &http.Server{ + Addr: s.listenAddr, + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 60 * time.Second, + } + + mux := http.NewServeMux() + s.RegisterRoutes(mux, cors.New(s.corsConfig)) httpServer.Handler = mux - logger := []zap.Field{ + s.logger.Info("MCP server started", zap.String("listen_addr", s.listenAddr), - zap.String("path", "/mcp"), + zap.String("path", s.mountPath), zap.String("operations_dir", s.operationsDir), zap.String("graph_name", s.graphName), zap.Bool("exclude_mutations", s.excludeMutations), zap.Bool("enable_arbitrary_operations", s.enableArbitraryOperations), zap.Bool("expose_schema", s.exposeSchema), - } - - s.logger.Info("MCP server started", logger...) + ) go func() { defer s.logger.Info("MCP server stopped") diff --git a/router/pkg/mcpserver/server_test.go b/router/pkg/mcpserver/server_test.go index 3a3c044609..e5c657ae79 100644 --- a/router/pkg/mcpserver/server_test.go +++ b/router/pkg/mcpserver/server_test.go @@ -1,6 +1,9 @@ package mcpserver import ( + "context" + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -195,3 +198,43 @@ func TestReload_PrefixModeAvoidsReservedNameCollision(t *testing.T) { "get_operation_info", }, srv.registeredTools) } + +func TestRegisterRoutesUsesMountPath(t *testing.T) { + t.Parallel() + + srv, err := NewGraphQLSchemaServer( + context.Background(), + "http://localhost:3002/graphql", + WithMountPath("/billing/mcp"), + WithLogger(zap.NewNop()), + ) + require.NoError(t, err) + t.Cleanup(srv.Close) + + require.Equal(t, "/billing/mcp", srv.MountPath()) + + mux := http.NewServeMux() + srv.RegisterRoutes(mux, func(h http.Handler) http.Handler { return h }) + + // The MCP endpoint answers on the mount path. + _, pattern := mux.Handler(httptest.NewRequest(http.MethodPost, "/billing/mcp", nil)) + require.Equal(t, "/billing/mcp", pattern) + + // The default path is not registered. + _, pattern = mux.Handler(httptest.NewRequest(http.MethodPost, "/mcp", nil)) + require.Empty(t, pattern) +} + +func TestMountPathDefaultsToMcp(t *testing.T) { + t.Parallel() + + srv, err := NewGraphQLSchemaServer( + context.Background(), + "http://localhost:3002/graphql", + WithLogger(zap.NewNop()), + ) + require.NoError(t, err) + t.Cleanup(srv.Close) + + require.Equal(t, DefaultMountPath, srv.MountPath()) +} From ee8bbe9e9f4ec4f5c2e602516d0c9bcb4005c39c Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 15:15:09 +0100 Subject: [PATCH 03/18] fix(mcp): derive OAuth resource identifier and metadata path from the mount path --- router/pkg/mcpserver/server.go | 23 ++--------------- router/pkg/mcpserver/server_test.go | 40 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/router/pkg/mcpserver/server.go b/router/pkg/mcpserver/server.go index f8d6dcf853..af7c825be0 100644 --- a/router/pkg/mcpserver/server.go +++ b/router/pkg/mcpserver/server.go @@ -291,7 +291,7 @@ func NewGraphQLSchemaServer(ctx context.Context, routerGraphQLEndpoint string, o // Build resource metadata URL for WWW-Authenticate header resourceMetadataURL := "" if options.ServerBaseURL != "" { - resourceMetadataURL = fmt.Sprintf("%s/.well-known/oauth-protected-resource/mcp", options.ServerBaseURL) + resourceMetadataURL = strings.TrimRight(options.ServerBaseURL, "/") + MetadataPath(options.MountPath) } authMiddleware, err = NewMCPAuthMiddleware(tokenDecoder, resourceMetadataURL, options.OAuthConfig.Scopes, options.OAuthConfig.ScopeChallengeIncludeTokenScopes) @@ -1123,17 +1123,6 @@ func (s *GraphQLSchemaServer) handleProtectedResourceMetadata(w http.ResponseWri return } - // Determine the resource URL (this MCP server's base URL) - resourceURL := s.serverBaseURL - if resourceURL == "" { - // Fallback: construct from request if not configured - scheme := "http" - if r.TLS != nil { - scheme = "https" - } - resourceURL = fmt.Sprintf("%s://%s", scheme, r.Host) - } - // Build scopes_supported from all configured scopes (union across all levels) // plus all scopes extracted from @requiresScopes directives on operations scopesSet := make(map[string]bool) @@ -1179,7 +1168,7 @@ func (s *GraphQLSchemaServer) handleProtectedResourceMetadata(w http.ResponseWri scopes = []string{} // Ensure non-nil for JSON encoding } - mcpResourceURL := strings.TrimRight(resourceURL, "/") + "/mcp" + mcpResourceURL := ResourceIdentifier(s.serverBaseURL, s.mountPath) metadata := ProtectedResourceMetadata{ Resource: mcpResourceURL, @@ -1201,11 +1190,3 @@ func (s *GraphQLSchemaServer) handleProtectedResourceMetadata(w http.ResponseWri w.WriteHeader(http.StatusOK) _, _ = w.Write(data) } - -// GetResourceMetadataURL returns the URL for the OAuth 2.0 Protected Resource Metadata endpoint -func (s *GraphQLSchemaServer) GetResourceMetadataURL() string { - if s.serverBaseURL != "" { - return fmt.Sprintf("%s/.well-known/oauth-protected-resource/mcp", s.serverBaseURL) - } - return "" -} diff --git a/router/pkg/mcpserver/server_test.go b/router/pkg/mcpserver/server_test.go index e5c657ae79..1b8438b0dc 100644 --- a/router/pkg/mcpserver/server_test.go +++ b/router/pkg/mcpserver/server_test.go @@ -2,6 +2,7 @@ package mcpserver import ( "context" + "encoding/json" "net/http" "net/http/httptest" "os" @@ -15,6 +16,8 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zapcore" "go.uber.org/zap/zaptest/observer" + + "github.com/wundergraph/cosmo/router/pkg/config" ) const testSchema = ` @@ -238,3 +241,40 @@ func TestMountPathDefaultsToMcp(t *testing.T) { require.Equal(t, DefaultMountPath, srv.MountPath()) } + +func TestProtectedResourceMetadataUsesMountPath(t *testing.T) { + t.Parallel() + + oauthCfg := &config.MCPOAuthConfiguration{ + Enabled: true, + AuthorizationServerURL: "https://auth.example.com", + JWKS: []config.JWKSConfiguration{{ + Secret: "test-secret-value", + Algorithm: "HS256", + }}, + } + + srv, err := NewGraphQLSchemaServer( + context.Background(), + "http://localhost:3002/graphql", + WithMountPath("/billing/mcp"), + WithServerBaseURL("https://billing.example.com"), + WithOAuth(oauthCfg), + WithLogger(zap.NewNop()), + ) + require.NoError(t, err) + t.Cleanup(srv.Close) + + mux := http.NewServeMux() + srv.RegisterRoutes(mux, func(h http.Handler) http.Handler { return h }) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/.well-known/oauth-protected-resource/billing/mcp", nil) + mux.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + + var metadata ProtectedResourceMetadata + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &metadata)) + require.Equal(t, "https://billing.example.com/billing/mcp", metadata.Resource) +} From 513d6b16ed3d913bee59f6b0955c10e25c7951c1 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 15:27:28 +0100 Subject: [PATCH 04/18] feat(mcp): add the mcp.servers config map --- router/pkg/config/config.go | 27 + router/pkg/config/config.schema.json | 502 ++++++++++-------- router/pkg/config/config_test.go | 37 ++ .../pkg/config/testdata/config_defaults.json | 3 +- router/pkg/config/testdata/config_full.json | 3 +- 5 files changed, 341 insertions(+), 231 deletions(-) diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index 781709a25d..df4179769c 100644 --- a/router/pkg/config/config.go +++ b/router/pkg/config/config.go @@ -1358,6 +1358,33 @@ type MCPConfiguration struct { // ResourceDocumentation is a URL to a human-readable page describing this MCP resource, // its access policies, and how to get started. Included in RFC 9728 Protected Resource Metadata if set. ResourceDocumentation string `yaml:"resource_documentation,omitempty" env:"MCP_RESOURCE_DOCUMENTATION"` + // Servers maps a server name to one MCP server. When this map has entries, + // the router ignores the deprecated top-level options. + Servers map[string]MCPServerEntry `yaml:"servers,omitempty"` +} + +// MCPServerEntry configures one MCP server mounted on the shared MCP listener. +// The map key in mcp.servers is the server name. +// +// This type carries no env tags: the servers map is YAML-only, because env-var +// overrides cannot address map entries. +type MCPServerEntry struct { + Enabled bool `yaml:"enabled"` + Path string `yaml:"path"` + BaseURL string `yaml:"base_url,omitempty"` + Storage MCPStorageConfig `yaml:"storage,omitempty"` + GraphName string `yaml:"graph_name,omitempty"` + ExcludeMutations bool `yaml:"exclude_mutations"` + EnableArbitraryOperations bool `yaml:"enable_arbitrary_operations"` + ExposeSchema bool `yaml:"expose_schema"` + OmitToolNamePrefix bool `yaml:"omit_tool_name_prefix"` + Session MCPSessionConfig `yaml:"session,omitempty"` + OAuth MCPOAuthConfiguration `yaml:"oauth,omitempty"` + ResourceDocumentation string `yaml:"resource_documentation,omitempty"` + Title string `yaml:"title,omitempty"` + Description string `yaml:"description,omitempty"` + Version string `yaml:"version,omitempty"` + Discover MCPDiscoverConfig `yaml:"discover,omitempty"` } type MCPOAuthConfiguration struct { diff --git a/router/pkg/config/config.schema.json b/router/pkg/config/config.schema.json index 8428a4a0e4..ebd04b0226 100644 --- a/router/pkg/config/config.schema.json +++ b/router/pkg/config/config.schema.json @@ -2690,15 +2690,7 @@ "description": "A human-readable description of this MCP server, reported to MCP clients in serverInfo." }, "discover": { - "type": "object", - "description": "Configuration for the server identity exposed via the MCP server/discover method (protocol version 2026-07-28).", - "additionalProperties": false, - "properties": { - "instructions": { - "type": "string", - "description": "Natural-language guidance for MCP clients (AI agents) on how to use this server effectively. Served in the server/discover response and in the legacy initialize response, so all clients receive it." - } - } + "$ref": "#/$defs/mcp_discover" } } }, @@ -2708,28 +2700,10 @@ "format": "url" }, "storage": { - "type": "object", - "description": "Storage provider configuration for the MCP server. This specifies where GraphQL operations are loaded from.", - "additionalProperties": false, - "properties": { - "provider_id": { - "type": "string", - "description": "The ID of the storage provider to use for loading GraphQL operations. Only storage provider of type 'file_system' are supported. The provider must be configured in the storage_providers section." - } - }, - "required": ["provider_id"] + "$ref": "#/$defs/mcp_storage" }, "session": { - "type": "object", - "description": "Session configuration for the MCP server. This controls how the MCP server handles client sessions.", - "additionalProperties": false, - "properties": { - "stateless": { - "type": "boolean", - "default": true, - "description": "Whether the MCP server should operate in stateless mode. When true, the server does not maintain session state between requests. When false, the server maintains session state, which can be useful for certain AI model integrations." - } - } + "$ref": "#/$defs/mcp_session" }, "graph_name": { "type": "string", @@ -2762,207 +2736,38 @@ "format": "http-url" }, "oauth": { + "$ref": "#/$defs/mcp_oauth" + }, + "servers": { "type": "object", - "description": "OAuth/JWKS authentication configuration for the MCP server. When enabled, MCP tool calls require valid JWT authentication and the server implements OAuth 2.0 discovery mechanisms (RFC 8414, RFC 9728).", - "additionalProperties": false, - "properties": { - "enabled": { - "type": "boolean", - "default": false, - "description": "Enable OAuth/JWKS authentication for the MCP server. When true, all MCP tool calls must include a valid JWT token." - }, - "authorization_server_url": { - "type": "string", - "description": "The base URL of the OAuth 2.0 authorization server. This URL is advertised to MCP clients via the Protected Resource Metadata endpoint (RFC 9728) to enable automatic discovery of OAuth endpoints. Clients will append '/.well-known/oauth-authorization-server' to this URL to discover token, authorization, and registration endpoints. Example: 'https://auth.example.com'", - "format": "http-url" - }, - "scopes": { - "type": "object", - "description": "Configures which OAuth scopes are required for different MCP operations. All configured scopes are automatically unioned into 'scopes_supported' for OAuth metadata discovery.", - "additionalProperties": false, - "properties": { - "initialize": { - "type": "array", - "description": "Scopes required for ALL HTTP requests (checked before JSON-RPC parsing). This is the baseline scope needed to establish an MCP connection.", - "items": { "type": "string" } - }, - "tools_list": { - "type": "array", - "description": "Scopes required for the tools/list MCP method.", - "items": { "type": "string" } - }, - "tools_call": { - "type": "array", - "description": "Scopes required for the tools/call MCP method (any tool).", - "items": { "type": "string" } - }, - "execute_graphql": { - "type": "array", - "description": "Scopes required to call the execute_graphql built-in tool. Additive to tools_call scopes. Only relevant when enable_arbitrary_operations is true.", - "items": { "type": "string" } - }, - "get_operation_info": { - "type": "array", - "description": "Scopes required to call the get_operation_info built-in tool. Additive to tools_call scopes.", - "items": { "type": "string" } - }, - "get_schema": { - "type": "array", - "description": "Scopes required to call the get_schema built-in tool. Additive to tools_call scopes. Only relevant when expose_schema is true.", - "items": { "type": "string" } - } - } - }, - "scope_challenge_include_token_scopes": { - "type": "boolean", - "default": false, - "description": "When true, includes the token's existing scopes in the scope parameter of 403 insufficient_scope responses (workaround for MCP client SDKs that replace rather than accumulate scopes). When false (default), only the scopes required for the operation are returned (RFC 6750 strict)." - }, - "jwks": { - "type": "array", - "description": "List of JWKS (JSON Web Key Set) configurations for JWT token verification. Multiple JWKS providers can be configured for different authentication sources.", - "items": { - "type": "object", - "additionalProperties": false, - "properties": { - "url": { - "type": "string", - "description": "The URL of the JWKs. The JWKs are used to verify the JWT (JSON Web Token). The URL is specified as a string with the format 'scheme://host:port'.", - "format": "http-url" - }, - "audiences": { - "type": "array", - "description": "The audiences of the JWKs. The audiences are used to verify the JWT (JSON Web Token). The audiences are specified as a list of strings.", - "items": { - "type": "string" - } - }, - "secret": { - "type": "string", - "description": "The secret of the JWKs" - }, - "symmetric_algorithm": { - "type": "string", - "description": "The symmetric algorithm used", - "enum": ["HS256", "HS384", "HS512"] - }, - "header_key_id": { - "type": "string", - "description": "The KID header of the JWK token created using the secret" - }, - "allowed_use": { - "type": "array", - "description": "The allowed value of the use parameter for the JWKs. If not specified, only keys with use set to 'sig' will be used. If your server provides no use, you can add an empty value to allow those keys.", - "default": ["sig"], - "items": { - "type": "string", - "enum": ["sig", "enc", ""] - } - }, - "algorithms": { - "type": "array", - "description": "The allowed algorithms for the keys that are retrieved from the JWKs. An empty list means that all algorithms are allowed.", - "items": { - "type": "string", - "enum": [ - "HS256", - "HS384", - "HS512", - "RS256", - "RS384", - "RS512", - "ES256", - "ES384", - "ES512", - "PS256", - "PS384", - "PS512", - "EdDSA" - ] - } - }, - "refresh_interval": { - "type": "string", - "duration": { - "minimum": "5s" - }, - "description": "The interval at which the JWKs are refreshed. The period is specified as a string with a number and a unit, e.g. 10ms, 1s, 1m, 1h. The supported units are 'ms', 's', 'm', 'h'.", - "default": "1m" - }, - "refresh_unknown_kid": { - "type": "object", - "description": "Controls rate-limited refresh behavior when a JWT KID is unknown.", - "additionalProperties": false, - "properties": { - "enabled": { - "type": "boolean", - "description": "Enable refresh attempts on unknown KID.", - "default": false - }, - "max_wait": { - "type": "string", - "description": "Maximum time to wait for a refresh permit before giving up.", - "default": "2m", - "duration": { - "minimum": "0s" - } - }, - "interval": { - "type": "string", - "description": "Token refill interval for the rate limiter.", - "default": "30s", - "duration": { - "minimum": "1s" - } - }, - "burst": { - "type": "integer", - "description": "Burst size for the rate limiter.", - "default": 2, - "minimum": 1 - } - } - } - }, - "oneOf": [ - { - "required": ["url"], - "not": { - "anyOf": [ - { - "required": ["secret"] - }, - { - "required": ["symmetric_algorithm"] - }, - { - "required": ["header_key_id"] - } - ] - } - }, - { - "required": ["secret", "symmetric_algorithm", "header_key_id"], - "not": { - "anyOf": [ - { - "required": ["url"] - }, - { - "required": ["algorithms"] - }, - { - "required": ["refresh_interval"] - }, - { - "required": ["refresh_unknown_kid"] - } - ] - } - } - ] - } - } + "description": "Maps a server name to one MCP server mounted on the shared MCP listener. When this object has entries, the deprecated top-level MCP options are ignored. This object is YAML-only; environment variables cannot address its entries.", + "additionalProperties": { + "type": "object", + "additionalProperties": false, + "properties": { + "enabled": { "type": "boolean", "default": false }, + "path": { + "type": "string", + "description": "The HTTP path this MCP server is mounted on. Must start with a single slash, must not end with a slash, and must not contain '{', '}', or '*'. Must not be the reserved OAuth protected resource metadata path.", + "pattern": "^/([^/{}*][^{}*]*[^/{}*])?$", + "not": { "pattern": "^/\\.well-known/oauth-protected-resource(/|$)" } + }, + "base_url": { "type": "string", "format": "http-url" }, + "storage": { "$ref": "#/$defs/mcp_storage" }, + "graph_name": { "type": "string" }, + "exclude_mutations": { "type": "boolean", "default": false }, + "enable_arbitrary_operations": { "type": "boolean", "default": false }, + "expose_schema": { "type": "boolean", "default": false }, + "omit_tool_name_prefix": { "type": "boolean", "default": false }, + "session": { "$ref": "#/$defs/mcp_session" }, + "oauth": { "$ref": "#/$defs/mcp_oauth" }, + "resource_documentation": { "type": "string", "format": "http-url" }, + "title": { "type": "string" }, + "description": { "type": "string" }, + "version": { "type": "string" }, + "discover": { "$ref": "#/$defs/mcp_discover" } + }, + "required": ["path"] } } }, @@ -4470,6 +4275,245 @@ } }, "$defs": { + "mcp_storage": { + "type": "object", + "description": "Storage provider configuration for the MCP server. This specifies where GraphQL operations are loaded from.", + "additionalProperties": false, + "properties": { + "provider_id": { + "type": "string", + "description": "The ID of the storage provider to use for loading GraphQL operations. Only storage provider of type 'file_system' are supported. The provider must be configured in the storage_providers section." + } + }, + "required": ["provider_id"] + }, + "mcp_session": { + "type": "object", + "description": "Session configuration for the MCP server. This controls how the MCP server handles client sessions.", + "additionalProperties": false, + "properties": { + "stateless": { + "type": "boolean", + "default": true, + "description": "Whether the MCP server should operate in stateless mode. When true, the server does not maintain session state between requests. When false, the server maintains session state, which can be useful for certain AI model integrations." + } + } + }, + "mcp_discover": { + "type": "object", + "description": "Configuration for the server identity exposed via the MCP server/discover method (protocol version 2026-07-28).", + "additionalProperties": false, + "properties": { + "instructions": { + "type": "string", + "description": "Natural-language guidance for MCP clients (AI agents) on how to use this server effectively. Served in the server/discover response and in the legacy initialize response, so all clients receive it." + } + } + }, + "mcp_oauth": { + "type": "object", + "description": "OAuth/JWKS authentication configuration for the MCP server. When enabled, MCP tool calls require valid JWT authentication and the server implements OAuth 2.0 discovery mechanisms (RFC 8414, RFC 9728).", + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean", + "default": false, + "description": "Enable OAuth/JWKS authentication for the MCP server. When true, all MCP tool calls must include a valid JWT token." + }, + "authorization_server_url": { + "type": "string", + "description": "The base URL of the OAuth 2.0 authorization server. This URL is advertised to MCP clients via the Protected Resource Metadata endpoint (RFC 9728) to enable automatic discovery of OAuth endpoints. Clients will append '/.well-known/oauth-authorization-server' to this URL to discover token, authorization, and registration endpoints. Example: 'https://auth.example.com'", + "format": "http-url" + }, + "scopes": { + "type": "object", + "description": "Configures which OAuth scopes are required for different MCP operations. All configured scopes are automatically unioned into 'scopes_supported' for OAuth metadata discovery.", + "additionalProperties": false, + "properties": { + "initialize": { + "type": "array", + "description": "Scopes required for ALL HTTP requests (checked before JSON-RPC parsing). This is the baseline scope needed to establish an MCP connection.", + "items": { "type": "string" } + }, + "tools_list": { + "type": "array", + "description": "Scopes required for the tools/list MCP method.", + "items": { "type": "string" } + }, + "tools_call": { + "type": "array", + "description": "Scopes required for the tools/call MCP method (any tool).", + "items": { "type": "string" } + }, + "execute_graphql": { + "type": "array", + "description": "Scopes required to call the execute_graphql built-in tool. Additive to tools_call scopes. Only relevant when enable_arbitrary_operations is true.", + "items": { "type": "string" } + }, + "get_operation_info": { + "type": "array", + "description": "Scopes required to call the get_operation_info built-in tool. Additive to tools_call scopes.", + "items": { "type": "string" } + }, + "get_schema": { + "type": "array", + "description": "Scopes required to call the get_schema built-in tool. Additive to tools_call scopes. Only relevant when expose_schema is true.", + "items": { "type": "string" } + } + } + }, + "scope_challenge_include_token_scopes": { + "type": "boolean", + "default": false, + "description": "When true, includes the token's existing scopes in the scope parameter of 403 insufficient_scope responses (workaround for MCP client SDKs that replace rather than accumulate scopes). When false (default), only the scopes required for the operation are returned (RFC 6750 strict)." + }, + "jwks": { + "type": "array", + "description": "List of JWKS (JSON Web Key Set) configurations for JWT token verification. Multiple JWKS providers can be configured for different authentication sources.", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "url": { + "type": "string", + "description": "The URL of the JWKs. The JWKs are used to verify the JWT (JSON Web Token). The URL is specified as a string with the format 'scheme://host:port'.", + "format": "http-url" + }, + "audiences": { + "type": "array", + "description": "The audiences of the JWKs. The audiences are used to verify the JWT (JSON Web Token). The audiences are specified as a list of strings.", + "items": { + "type": "string" + } + }, + "secret": { + "type": "string", + "description": "The secret of the JWKs" + }, + "symmetric_algorithm": { + "type": "string", + "description": "The symmetric algorithm used", + "enum": ["HS256", "HS384", "HS512"] + }, + "header_key_id": { + "type": "string", + "description": "The KID header of the JWK token created using the secret" + }, + "allowed_use": { + "type": "array", + "description": "The allowed value of the use parameter for the JWKs. If not specified, only keys with use set to 'sig' will be used. If your server provides no use, you can add an empty value to allow those keys.", + "default": ["sig"], + "items": { + "type": "string", + "enum": ["sig", "enc", ""] + } + }, + "algorithms": { + "type": "array", + "description": "The allowed algorithms for the keys that are retrieved from the JWKs. An empty list means that all algorithms are allowed.", + "items": { + "type": "string", + "enum": [ + "HS256", + "HS384", + "HS512", + "RS256", + "RS384", + "RS512", + "ES256", + "ES384", + "ES512", + "PS256", + "PS384", + "PS512", + "EdDSA" + ] + } + }, + "refresh_interval": { + "type": "string", + "duration": { + "minimum": "5s" + }, + "description": "The interval at which the JWKs are refreshed. The period is specified as a string with a number and a unit, e.g. 10ms, 1s, 1m, 1h. The supported units are 'ms', 's', 'm', 'h'.", + "default": "1m" + }, + "refresh_unknown_kid": { + "type": "object", + "description": "Controls rate-limited refresh behavior when a JWT KID is unknown.", + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean", + "description": "Enable refresh attempts on unknown KID.", + "default": false + }, + "max_wait": { + "type": "string", + "description": "Maximum time to wait for a refresh permit before giving up.", + "default": "2m", + "duration": { + "minimum": "0s" + } + }, + "interval": { + "type": "string", + "description": "Token refill interval for the rate limiter.", + "default": "30s", + "duration": { + "minimum": "1s" + } + }, + "burst": { + "type": "integer", + "description": "Burst size for the rate limiter.", + "default": 2, + "minimum": 1 + } + } + } + }, + "oneOf": [ + { + "required": ["url"], + "not": { + "anyOf": [ + { + "required": ["secret"] + }, + { + "required": ["symmetric_algorithm"] + }, + { + "required": ["header_key_id"] + } + ] + } + }, + { + "required": ["secret", "symmetric_algorithm", "header_key_id"], + "not": { + "anyOf": [ + { + "required": ["url"] + }, + { + "required": ["algorithms"] + }, + { + "required": ["refresh_interval"] + }, + { + "required": ["refresh_unknown_kid"] + } + ] + } + } + ] + } + } + } + }, "jwks_configuration": { "type": "object", "additionalProperties": false, diff --git a/router/pkg/config/config_test.go b/router/pkg/config/config_test.go index 45a1e26f61..331492c161 100644 --- a/router/pkg/config/config_test.go +++ b/router/pkg/config/config_test.go @@ -2412,3 +2412,40 @@ mcp: require.Equal(t, "Query products, orders and customers.", cfg.Config.MCP.Server.Description) }) } + +func TestLoadMCPServersMap(t *testing.T) { + t.Parallel() + + f := createTempFileFromFixture(t, ` +version: "1" +mcp: + enabled: true + servers: + support: + enabled: true + path: /mcp/support + exclude_mutations: true + storage: + provider_id: support-ops + billing: + enabled: true + path: /billing/mcp + base_url: https://billing.example.com + storage: + provider_id: billing-ops +`) + + cfg, err := LoadConfig([]string{f}) + require.NoError(t, err) + + require.Len(t, cfg.Config.MCP.Servers, 2) + + support := cfg.Config.MCP.Servers["support"] + require.True(t, support.Enabled) + require.Equal(t, "/mcp/support", support.Path) + require.True(t, support.ExcludeMutations) + require.Equal(t, "support-ops", support.Storage.ProviderID) + + billing := cfg.Config.MCP.Servers["billing"] + require.Equal(t, "https://billing.example.com", billing.BaseURL) +} diff --git a/router/pkg/config/testdata/config_defaults.json b/router/pkg/config/testdata/config_defaults.json index 080b9f7a30..613f4e1f2b 100644 --- a/router/pkg/config/testdata/config_defaults.json +++ b/router/pkg/config/testdata/config_defaults.json @@ -226,7 +226,8 @@ "ScopeChallengeIncludeTokenScopes": false, "MaxScopeCombinations": 2048 }, - "ResourceDocumentation": "" + "ResourceDocumentation": "", + "Servers": null }, "ConnectRPC": { "Enabled": false, diff --git a/router/pkg/config/testdata/config_full.json b/router/pkg/config/testdata/config_full.json index 446dcc3958..9c479e7e00 100644 --- a/router/pkg/config/testdata/config_full.json +++ b/router/pkg/config/testdata/config_full.json @@ -295,7 +295,8 @@ "ScopeChallengeIncludeTokenScopes": false, "MaxScopeCombinations": 2048 }, - "ResourceDocumentation": "" + "ResourceDocumentation": "", + "Servers": null }, "ConnectRPC": { "Enabled": false, From 736064a215e24595882bc00507cd7238cdfc818a Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 15:38:44 +0100 Subject: [PATCH 05/18] fix(mcp): correct mount path validation for short and double-slash paths --- router/pkg/config/config.schema.json | 2 +- router/pkg/config/config_test.go | 43 ++++++++++++++++++++++++++++ router/pkg/mcpserver/paths.go | 3 ++ router/pkg/mcpserver/paths_test.go | 3 ++ 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/router/pkg/config/config.schema.json b/router/pkg/config/config.schema.json index ebd04b0226..c8438e9888 100644 --- a/router/pkg/config/config.schema.json +++ b/router/pkg/config/config.schema.json @@ -2749,7 +2749,7 @@ "path": { "type": "string", "description": "The HTTP path this MCP server is mounted on. Must start with a single slash, must not end with a slash, and must not contain '{', '}', or '*'. Must not be the reserved OAuth protected resource metadata path.", - "pattern": "^/([^/{}*][^{}*]*[^/{}*])?$", + "pattern": "^/([^/{}*]|[^/{}*][^{}*]*[^/{}*])?$", "not": { "pattern": "^/\\.well-known/oauth-protected-resource(/|$)" } }, "base_url": { "type": "string", "format": "http-url" }, diff --git a/router/pkg/config/config_test.go b/router/pkg/config/config_test.go index 331492c161..d08c1e7bd5 100644 --- a/router/pkg/config/config_test.go +++ b/router/pkg/config/config_test.go @@ -2449,3 +2449,46 @@ mcp: billing := cfg.Config.MCP.Servers["billing"] require.Equal(t, "https://billing.example.com", billing.BaseURL) } + +func TestLoadMCPServersMapPathValidation(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + path string + wantErr bool + }{ + {name: "single character path is valid", path: "/a"}, + {name: "leading double slash is rejected", path: "//foo", wantErr: true}, + {name: "trailing slash is rejected", path: "/mcp/", wantErr: true}, + {name: "wildcard is rejected", path: "/mcp/{id}", wantErr: true}, + {name: "reserved oauth metadata prefix is rejected", path: "/.well-known/oauth-protected-resource/x", wantErr: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + f := createTempFileFromFixture(t, fmt.Sprintf(` +version: "1" +mcp: + enabled: true + servers: + support: + enabled: true + path: "%s" + storage: + provider_id: support-ops +`, tc.path)) + + _, err := LoadConfig([]string{f}) + + if tc.wantErr { + var js *jsonschema.ValidationError + require.ErrorAs(t, err, &js) + return + } + require.NoError(t, err) + }) + } +} diff --git a/router/pkg/mcpserver/paths.go b/router/pkg/mcpserver/paths.go index a02331c40d..9bbb27c4fb 100644 --- a/router/pkg/mcpserver/paths.go +++ b/router/pkg/mcpserver/paths.go @@ -26,6 +26,9 @@ func ValidateMountPath(p string) error { if !strings.HasPrefix(p, "/") { return fmt.Errorf("path %q must start with /", p) } + if strings.HasPrefix(p, "//") { + return fmt.Errorf("path %q must not start with //", p) + } if len(p) > 1 && strings.HasSuffix(p, "/") { return fmt.Errorf("path %q must not end with /", p) } diff --git a/router/pkg/mcpserver/paths_test.go b/router/pkg/mcpserver/paths_test.go index 2a4f71d34c..487f64c4af 100644 --- a/router/pkg/mcpserver/paths_test.go +++ b/router/pkg/mcpserver/paths_test.go @@ -17,8 +17,11 @@ func TestValidateMountPath(t *testing.T) { {name: "simple", path: "/mcp"}, {name: "nested", path: "/billing/mcp"}, {name: "root", path: "/"}, + {name: "single character", path: "/a"}, + {name: "interior double slash", path: "/a//b"}, {name: "empty", path: "", wantErr: "path is empty"}, {name: "no leading slash", path: "mcp", wantErr: "must start with /"}, + {name: "leading double slash", path: "//foo", wantErr: "must not start with //"}, {name: "trailing slash", path: "/mcp/", wantErr: "must not end with /"}, {name: "wildcard", path: "/mcp/{id}", wantErr: "must not contain"}, {name: "reserved metadata prefix", path: "/.well-known/oauth-protected-resource/x", wantErr: "reserved"}, From 5a99bba9baf4cf025f9aa7c5c1653c4329e411c8 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 15:43:52 +0100 Subject: [PATCH 06/18] feat(mcp): validate mount paths across the servers map --- router/pkg/mcpserver/validation.go | 42 +++++++++++++++ router/pkg/mcpserver/validation_test.go | 72 +++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 router/pkg/mcpserver/validation.go create mode 100644 router/pkg/mcpserver/validation_test.go diff --git a/router/pkg/mcpserver/validation.go b/router/pkg/mcpserver/validation.go new file mode 100644 index 0000000000..ed49be2ca8 --- /dev/null +++ b/router/pkg/mcpserver/validation.go @@ -0,0 +1,42 @@ +package mcpserver + +import ( + "fmt" + "maps" + "slices" + + "github.com/wundergraph/cosmo/router/pkg/config" +) + +// ValidateServers checks the mcp.servers map before the router mounts anything. +// +// It only checks enabled servers, because the router never mounts a disabled +// one. Two servers may therefore share a path while one of them is off. +// +// Every mount path is an exact ServeMux pattern, so two distinct paths can +// never conflict on the mux. Checking for exact duplicates is therefore enough. +func ValidateServers(servers map[string]config.MCPServerEntry) error { + // Sort the names so the same config always reports the same error. + names := slices.Sorted(maps.Keys(servers)) + + byPath := make(map[string]string, len(servers)) + + for _, name := range names { + server := servers[name] + if !server.Enabled { + continue + } + + if err := ValidateMountPath(server.Path); err != nil { + return fmt.Errorf("mcp server %q: %w", name, err) + } + + if other, ok := byPath[server.Path]; ok { + return fmt.Errorf("mcp servers %q and %q use the same path %q", other, name, server.Path) + } + + byPath[server.Path] = name + } + + return nil +} diff --git a/router/pkg/mcpserver/validation_test.go b/router/pkg/mcpserver/validation_test.go new file mode 100644 index 0000000000..8793f8f958 --- /dev/null +++ b/router/pkg/mcpserver/validation_test.go @@ -0,0 +1,72 @@ +package mcpserver + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/wundergraph/cosmo/router/pkg/config" +) + +func TestValidateServers(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + servers map[string]config.MCPServerEntry + wantErr string + }{ + { + name: "distinct paths", + servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true, Path: "/mcp/support"}, + "billing": {Enabled: true, Path: "/billing/mcp"}, + }, + }, + { + name: "duplicate paths", + servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true, Path: "/mcp"}, + "billing": {Enabled: true, Path: "/mcp"}, + }, + wantErr: `use the same path "/mcp"`, + }, + { + name: "duplicate path is allowed when one server is disabled", + servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true, Path: "/mcp"}, + "billing": {Enabled: false, Path: "/mcp"}, + }, + }, + { + name: "missing path", + servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true}, + }, + wantErr: "path is empty", + }, + { + name: "trailing slash", + servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true, Path: "/mcp/"}, + }, + wantErr: "must not end with /", + }, + { + name: "empty map", + servers: map[string]config.MCPServerEntry{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + err := ValidateServers(tc.servers) + if tc.wantErr == "" { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, tc.wantErr) + }) + } +} From 78e808477b9c42f8685c313d212ba87cd1b77b17 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 15:49:15 +0100 Subject: [PATCH 07/18] test(mcp): assert deterministic error order across map iterations --- router/pkg/mcpserver/validation_test.go | 34 +++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/router/pkg/mcpserver/validation_test.go b/router/pkg/mcpserver/validation_test.go index 8793f8f958..af66f23aac 100644 --- a/router/pkg/mcpserver/validation_test.go +++ b/router/pkg/mcpserver/validation_test.go @@ -55,6 +55,13 @@ func TestValidateServers(t *testing.T) { name: "empty map", servers: map[string]config.MCPServerEntry{}, }, + { + name: "all servers disabled", + servers: map[string]config.MCPServerEntry{ + "support": {Enabled: false}, + "billing": {Enabled: false, Path: "/mcp/"}, + }, + }, } for _, tc := range testCases { @@ -70,3 +77,30 @@ func TestValidateServers(t *testing.T) { }) } } + +// TestValidateServers_DeterministicOrder guards the doc comment's promise +// that ValidateServers reports the same error every time for the same +// config, even though Go randomizes map iteration order. Two enabled +// servers each have a distinct, unrelated problem, so only sorted name +// order decides which one is reported first: "alpha" sorts before "zeta", +// so alpha's trailing-slash error must always win. +// +// A single call can pass by luck against an unsorted implementation, since +// Go does not guarantee a different order on every range statement. Calling +// ValidateServers many times turns that coin flip into a near-certain +// failure for a naive `for name := range servers` implementation. +func TestValidateServers_DeterministicOrder(t *testing.T) { + t.Parallel() + + servers := map[string]config.MCPServerEntry{ + "alpha": {Enabled: true, Path: "/mcp/"}, + "zeta": {Enabled: true}, + } + + const wantErr = `mcp server "alpha": path "/mcp/" must not end with /` + + for i := 0; i < 100; i++ { + err := ValidateServers(servers) + require.EqualError(t, err, wantErr) + } +} From cdaeec857f4b9a8d3204b273556ca3d6d55abc18 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 15:58:13 +0100 Subject: [PATCH 08/18] refactor(mcp): move listener and mux ownership into a Host type --- router/pkg/mcpserver/host.go | 165 ++++++++++++++++++++++++++++++ router/pkg/mcpserver/host_test.go | 61 +++++++++++ router/pkg/mcpserver/server.go | 104 ------------------- 3 files changed, 226 insertions(+), 104 deletions(-) create mode 100644 router/pkg/mcpserver/host.go create mode 100644 router/pkg/mcpserver/host_test.go diff --git a/router/pkg/mcpserver/host.go b/router/pkg/mcpserver/host.go new file mode 100644 index 0000000000..e2050c7cb8 --- /dev/null +++ b/router/pkg/mcpserver/host.go @@ -0,0 +1,165 @@ +package mcpserver + +import ( + "context" + "errors" + "fmt" + "net/http" + "time" + + nodev1 "github.com/wundergraph/cosmo/router/gen/proto/wg/cosmo/node/v1" + "github.com/wundergraph/cosmo/router/pkg/cors" + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" + "go.uber.org/zap" +) + +// HostOptions configures the shared MCP listener. +type HostOptions struct { + ListenAddr string + Logger *zap.Logger + CorsConfig cors.Config +} + +// Host owns the MCP listener and its mux. It serves one or more MCP servers, +// each on its own mount path. CORS is shared by every server on the host. +type Host struct { + listenAddr string + logger *zap.Logger + corsConfig cors.Config + servers []*GraphQLSchemaServer + byPath map[string]struct{} + httpServer *http.Server +} + +// NewHost creates a listener that has no servers registered yet. +// +// The CORS settings are normalized for MCP clients regardless of what the +// caller supplies, the same way WithCORS used to force them per server +// before CORS moved to the host: all origins are allowed, because an MCP +// client's origin is not known ahead of time, and the MCP-specific headers +// are always present. +func NewHost(opts HostOptions) *Host { + logger := opts.Logger + if logger == nil { + logger = zap.NewNop() + } + + corsConfig := opts.CorsConfig + corsConfig.AllowOrigins = []string{"*"} + corsConfig.AllowMethods = []string{"GET", "PUT", "POST", "DELETE", "OPTIONS"} + corsConfig.AllowHeaders = append(corsConfig.AllowHeaders, "Content-Type", "Accept", "Authorization", "Last-Event-ID", "Mcp-Protocol-Version", "Mcp-Session-Id") + corsConfig.ExposeHeaders = append(corsConfig.ExposeHeaders, "Mcp-Session-Id", "WWW-Authenticate") + if corsConfig.MaxAge <= 0 { + corsConfig.MaxAge = 24 * time.Hour + } + + return &Host{ + listenAddr: opts.ListenAddr, + logger: logger, + corsConfig: corsConfig, + byPath: make(map[string]struct{}), + } +} + +// Register adds a server to the host. The config validation in ValidateServers +// already rejects duplicate paths; this check keeps the host safe on its own, +// because ServeMux panics when one pattern is registered two times. +func (h *Host) Register(s *GraphQLSchemaServer) error { + path := s.MountPath() + if _, ok := h.byPath[path]; ok { + return fmt.Errorf("an mcp server is already registered on path %q", path) + } + + h.byPath[path] = struct{}{} + h.servers = append(h.servers, s) + + return nil +} + +// Servers returns the registered servers in registration order. +func (h *Host) Servers() []*GraphQLSchemaServer { + return h.servers +} + +func (h *Host) buildMux() *http.ServeMux { + mux := http.NewServeMux() + middleware := cors.New(h.corsConfig) + + for _, s := range h.servers { + s.RegisterRoutes(mux, middleware) + } + + return mux +} + +// Start binds the listener and serves every registered server. +func (h *Host) Start() error { + if len(h.servers) == 0 { + h.logger.Debug("No MCP servers registered, skipping listener") + return nil + } + + h.httpServer = &http.Server{ + Addr: h.listenAddr, + Handler: h.buildMux(), + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + IdleTimeout: 60 * time.Second, + } + + for _, s := range h.servers { + h.logger.Info("MCP server mounted", + zap.String("listen_addr", h.listenAddr), + zap.String("path", s.MountPath()), + zap.String("graph_name", s.graphName), + zap.String("operations_dir", s.operationsDir), + ) + } + + go func() { + defer h.logger.Info("MCP listener stopped") + + err := h.httpServer.ListenAndServe() + if err != nil && !errors.Is(err, http.ErrServerClosed) { + h.logger.Error("Failed to start MCP listener", zap.Error(err)) + } + }() + + return nil +} + +// Reload rebuilds the tools of every server from the shared schema document. +// One server that fails does not stop the others; the error is logged and the +// remaining servers keep their previous tools. +func (h *Host) Reload(schema *ast.Document, fieldConfigs []*nodev1.FieldConfiguration) error { + for _, s := range h.servers { + if err := s.Reload(schema, fieldConfigs); err != nil { + h.logger.Error("Failed to reload MCP server", + zap.String("path", s.MountPath()), + zap.Error(err), + ) + } + } + + return nil +} + +// Stop closes every server and shuts the listener down. +func (h *Host) Stop(ctx context.Context) error { + for _, s := range h.servers { + s.Close() + } + + if h.httpServer == nil { + return nil + } + + shutdownCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + if err := h.httpServer.Shutdown(shutdownCtx); err != nil { + return fmt.Errorf("failed to gracefully shutdown MCP listener: %w", err) + } + + return nil +} diff --git a/router/pkg/mcpserver/host_test.go b/router/pkg/mcpserver/host_test.go new file mode 100644 index 0000000000..a76b7a540f --- /dev/null +++ b/router/pkg/mcpserver/host_test.go @@ -0,0 +1,61 @@ +package mcpserver + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + "go.uber.org/zap" +) + +func newTestServer(t *testing.T, mountPath string) *GraphQLSchemaServer { + t.Helper() + + srv, err := NewGraphQLSchemaServer( + context.Background(), + "http://localhost:3002/graphql", + WithMountPath(mountPath), + WithLogger(zap.NewNop()), + ) + require.NoError(t, err) + t.Cleanup(srv.Close) + + return srv +} + +func mustRequest(method, path string) *http.Request { + req, err := http.NewRequest(method, "http://localhost"+path, nil) + if err != nil { + panic(err) + } + return req +} + +func TestHostRegistersEachServerOnItsOwnPath(t *testing.T) { + t.Parallel() + + h := NewHost(HostOptions{ListenAddr: "localhost:0", Logger: zap.NewNop()}) + require.NoError(t, h.Register(newTestServer(t, "/mcp/support"))) + require.NoError(t, h.Register(newTestServer(t, "/billing/mcp"))) + + mux := h.buildMux() + + for _, path := range []string{"/mcp/support", "/billing/mcp"} { + _, pattern := mux.Handler(mustRequest(http.MethodPost, path)) + require.Equal(t, path, pattern, "expected a handler on %s", path) + } + + _, pattern := mux.Handler(mustRequest(http.MethodPost, "/mcp")) + require.Empty(t, pattern, "no server was mounted on /mcp") +} + +func TestHostRejectsDuplicatePaths(t *testing.T) { + t.Parallel() + + h := NewHost(HostOptions{ListenAddr: "localhost:0", Logger: zap.NewNop()}) + require.NoError(t, h.Register(newTestServer(t, "/mcp"))) + + err := h.Register(newTestServer(t, "/mcp")) + require.ErrorContains(t, err, "already registered") +} diff --git a/router/pkg/mcpserver/server.go b/router/pkg/mcpserver/server.go index af7c825be0..7a491069f2 100644 --- a/router/pkg/mcpserver/server.go +++ b/router/pkg/mcpserver/server.go @@ -5,7 +5,6 @@ import ( "cmp" "context" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -23,7 +22,6 @@ import ( "github.com/wundergraph/cosmo/router/internal/headers" "github.com/wundergraph/cosmo/router/pkg/authentication" "github.com/wundergraph/cosmo/router/pkg/config" - "github.com/wundergraph/cosmo/router/pkg/cors" "github.com/wundergraph/cosmo/router/pkg/schemaloader" "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" @@ -68,8 +66,6 @@ type Options struct { GraphName string // OperationsDir is the directory where GraphQL operations are stored OperationsDir string - // ListenAddr is the address where the server should listen to - ListenAddr string // MountPath is the HTTP path this server answers on. MountPath string // Enabled determines whether the MCP server should be started @@ -88,8 +84,6 @@ type Options struct { OmitToolNamePrefix bool // Stateless determines whether the MCP server should be stateless Stateless bool - // CorsConfig is the CORS configuration for the MCP server - CorsConfig cors.Config // OAuthConfig is the OAuth/JWKS configuration for authentication OAuthConfig *config.MCPOAuthConfiguration // ServerBaseURL is the base URL of this MCP server (for resource metadata) @@ -119,13 +113,11 @@ type GraphQLSchemaServer struct { server *mcp.Server graphName string operationsDir string - listenAddr string mountPath string logger *zap.Logger httpClient *http.Client requestTimeout time.Duration routerGraphQLEndpoint string - httpServer *http.Server excludeMutations bool enableArbitraryOperations bool exposeSchema bool @@ -134,7 +126,6 @@ type GraphQLSchemaServer struct { operationsManager *OperationsManager schemaCompiler *SchemaCompiler registeredTools []string - corsConfig cors.Config cancel context.CancelFunc oauthConfig *config.MCPOAuthConfiguration serverBaseURL string @@ -225,7 +216,6 @@ func NewGraphQLSchemaServer(ctx context.Context, routerGraphQLEndpoint string, o options := &Options{ GraphName: "graph", OperationsDir: "operations", - ListenAddr: "0.0.0.0:5025", MountPath: DefaultMountPath, Enabled: false, Logger: zap.NewNop(), @@ -337,7 +327,6 @@ func NewGraphQLSchemaServer(ctx context.Context, routerGraphQLEndpoint string, o server: mcpServer, graphName: options.GraphName, operationsDir: options.OperationsDir, - listenAddr: options.ListenAddr, mountPath: options.MountPath, logger: options.Logger, httpClient: httpClient, @@ -348,7 +337,6 @@ func NewGraphQLSchemaServer(ctx context.Context, routerGraphQLEndpoint string, o exposeSchema: options.ExposeSchema, omitToolNamePrefix: options.OmitToolNamePrefix, stateless: options.Stateless, - corsConfig: options.CorsConfig, cancel: cancel, oauthConfig: options.OAuthConfig, serverBaseURL: options.ServerBaseURL, @@ -407,13 +395,6 @@ func WithOperationsDir(operationsDir string) func(*Options) { } } -// WithListenAddr sets the listen address -func WithListenAddr(listenAddr string) func(*Options) { - return func(o *Options) { - o.ListenAddr = listenAddr - } -} - // WithMountPath sets the HTTP path this MCP server answers on. func WithMountPath(p string) func(*Options) { return func(o *Options) { @@ -462,20 +443,6 @@ func WithOmitToolNamePrefix(omitToolNamePrefix bool) func(*Options) { } } -func WithCORS(corsCfg cors.Config) func(*Options) { - return func(o *Options) { - // Force specific CORS settings for MCP server - corsCfg.AllowOrigins = []string{"*"} - corsCfg.AllowMethods = []string{"GET", "PUT", "POST", "DELETE", "OPTIONS"} - corsCfg.AllowHeaders = append(corsCfg.AllowHeaders, "Content-Type", "Accept", "Authorization", "Last-Event-ID", "Mcp-Protocol-Version", "Mcp-Session-Id") - corsCfg.ExposeHeaders = append(corsCfg.ExposeHeaders, "Mcp-Session-Id", "WWW-Authenticate") - if corsCfg.MaxAge <= 0 { - corsCfg.MaxAge = 24 * time.Hour - } - o.CorsConfig = corsCfg - } -} - // WithOAuth sets the OAuth configuration func WithOAuth(oauthCfg *config.MCPOAuthConfiguration) func(*Options) { return func(o *Options) { @@ -546,53 +513,6 @@ func (s *GraphQLSchemaServer) Close() { } } -// Serve starts the server with the configured options and returns the HTTP server. -func (s *GraphQLSchemaServer) Serve() (*http.Server, error) { - httpServer := &http.Server{ - Addr: s.listenAddr, - ReadTimeout: 30 * time.Second, - WriteTimeout: 30 * time.Second, - IdleTimeout: 60 * time.Second, - } - - mux := http.NewServeMux() - s.RegisterRoutes(mux, cors.New(s.corsConfig)) - httpServer.Handler = mux - - s.logger.Info("MCP server started", - zap.String("listen_addr", s.listenAddr), - zap.String("path", s.mountPath), - zap.String("operations_dir", s.operationsDir), - zap.String("graph_name", s.graphName), - zap.Bool("exclude_mutations", s.excludeMutations), - zap.Bool("enable_arbitrary_operations", s.enableArbitraryOperations), - zap.Bool("expose_schema", s.exposeSchema), - ) - - go func() { - defer s.logger.Info("MCP server stopped") - - err := httpServer.ListenAndServe() - if err != nil && !errors.Is(err, http.ErrServerClosed) { - s.logger.Error("failed to start HTTP server", zap.Error(err)) - } - }() - - return httpServer, nil -} - -// Start loads operations and starts the server -func (s *GraphQLSchemaServer) Start() error { - ss, err := s.Serve() - if err != nil { - return fmt.Errorf("failed to create HTTP server: %w", err) - } - - s.httpServer = ss - - return nil -} - // Reload reloads the operations and schema, and computes per-tool scope // requirements from @requiresScopes directives in the field configurations. func (s *GraphQLSchemaServer) Reload(schema *ast.Document, fieldConfigs []*nodev1.FieldConfiguration) error { @@ -630,30 +550,6 @@ func (s *GraphQLSchemaServer) Reload(schema *ast.Document, fieldConfigs []*nodev return nil } -// Stop gracefully shuts down the MCP server -func (s *GraphQLSchemaServer) Stop(ctx context.Context) error { - if s.httpServer == nil { - return fmt.Errorf("server is not started") - } - - s.logger.Debug("shutting down MCP server") - - // Cancel the server's context to stop background operations (e.g., JWKS key refresh) - if s.cancel != nil { - s.cancel() - } - - // Create a shutdown context with timeout - shutdownCtx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() - - if err := s.httpServer.Shutdown(shutdownCtx); err != nil { - return fmt.Errorf("failed to gracefully shutdown MCP server: %w", err) - } - - return nil -} - // registerTools registers all tools for the MCP server func (s *GraphQLSchemaServer) registerTools() error { // Only register the schema tool if exposeSchema is enabled From 1385b184f0bcca9e26ce197fc18fd48d9e705be5 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 16:09:42 +0100 Subject: [PATCH 09/18] feat(mcp): mount one server per mcp.servers entry on a shared listener --- router/core/graph_server.go | 6 +- router/core/router.go | 254 +++++++++++++++++++++++++++-------- router/core/router_config.go | 2 +- router/core/router_test.go | 53 ++++++++ 4 files changed, 253 insertions(+), 62 deletions(-) diff --git a/router/core/graph_server.go b/router/core/graph_server.go index 3cc45d5f9e..98e2308184 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -1610,9 +1610,9 @@ func (s *graphServer) buildGraphMux( operationPlanner := NewOperationPlanner(executor, gm.planCache, gm.planFallbackCache, s.planningDurationOverride) // We support the MCP only on the base graph. Feature flags are not supported yet. - if opts.IsBaseGraph() && s.mcpServer != nil { - if mErr := s.mcpServer.Reload(executor.ClientSchema, opts.EngineConfig.FieldConfigurations); mErr != nil { - return nil, fmt.Errorf("failed to reload MCP server: %w", mErr) + if opts.IsBaseGraph() && s.mcpHost != nil { + if mErr := s.mcpHost.Reload(executor.ClientSchema, opts.EngineConfig.FieldConfigurations); mErr != nil { + return nil, fmt.Errorf("failed to reload MCP servers: %w", mErr) } } diff --git a/router/core/router.go b/router/core/router.go index 3491a7e516..15d1296cd6 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -1175,90 +1175,228 @@ func (r *Router) setupTelemetry(ctx context.Context) error { return nil } -// startMCPServer initializes and starts the MCP server if enabled. -func (r *Router) startMCPServer(ctx context.Context) error { - if !r.mcp.Enabled { - return nil +// mcpHostDeps carries everything buildMCPHost needs, so the builder can be +// tested without a running router. +type mcpHostDeps struct { + cfg config.MCPConfiguration + logger *zap.Logger + graphqlEndpoint string + routerVersion string + corsOptions *cors.Config + providerRegistry *ProviderRegistry +} + +// buildMCPHost creates the shared MCP listener and one server per enabled entry. +// +// When mcp.servers has entries the deprecated top-level options are ignored. +// When it has none, the deprecated options build a single server, so an +// existing config keeps working. +func buildMCPHost(ctx context.Context, deps mcpHostDeps) (*mcpserver.Host, error) { + entries := deps.cfg.Servers + if len(entries) == 0 { + deps.logger.Warn("The top-level mcp options are deprecated. Use the mcp.servers map instead.") + entries = map[string]config.MCPServerEntry{ + deprecatedServerName(deps.cfg): deprecatedServerEntry(deps.cfg), + } + } else { + warnIgnoredDeprecatedMCPOptions(deps.cfg, deps.logger) } - var operationsDir string + if err := mcpserver.ValidateServers(entries); err != nil { + return nil, err + } - // If storage provider ID is set, resolve it to a directory path - if r.mcp.Storage.ProviderID != "" { - r.logger.Debug("Resolving storage provider for MCP operations", - zap.String("provider_id", r.mcp.Storage.ProviderID)) + host := mcpserver.NewHost(mcpserver.HostOptions{ + ListenAddr: deps.cfg.Server.ListenAddr, + Logger: deps.logger, + CorsConfig: corsConfigOrZero(deps.corsOptions), + }) - provider, ok := r.providerRegistry.FileSystem(r.mcp.Storage.ProviderID) - if !ok { - return fmt.Errorf("storage provider with id '%s' for mcp server not found", r.mcp.Storage.ProviderID) + names := slices.Sorted(maps.Keys(entries)) + + for _, name := range names { + entry := entries[name] + if !entry.Enabled { + continue + } + + srv, err := newMCPServerFromEntry(ctx, name, entry, deps) + if err != nil { + return nil, fmt.Errorf("mcp server %q: %w", name, err) + } + + if err := host.Register(srv); err != nil { + // Register did not take ownership of srv, so it will never be + // closed by host.Stop. Close it here to avoid leaking its + // background context, which drives JWKS key refresh. + srv.Close() + return nil, fmt.Errorf("mcp server %q: %w", name, err) } - r.logger.Debug("Found file_system storage provider for MCP", - zap.String("id", provider.ID), - zap.String("path", provider.Path)) - operationsDir = provider.Path } - logFields := []zap.Field{ - zap.String("storage_provider_id", r.mcp.Storage.ProviderID), + return host, nil +} + +// newMCPServerFromEntry builds one MCP server from its config entry. +func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPServerEntry, deps mcpHostDeps) (*mcpserver.GraphQLSchemaServer, error) { + var operationsDir string + + if entry.Storage.ProviderID != "" { + fsProvider, ok := deps.providerRegistry.FileSystem(entry.Storage.ProviderID) + if !ok { + return nil, fmt.Errorf("storage provider with id '%s' not found", entry.Storage.ProviderID) + } + operationsDir = fsProvider.Path } - // Initialize the MCP server with the resolved operations directory - mcpOpts := []func(*mcpserver.Options){ - mcpserver.WithGraphName(r.mcp.GraphName), + logger := deps.logger.With( + zap.String("mcp_server", name), + zap.String("storage_provider_id", entry.Storage.ProviderID), + ) + + // graph_name defaults to the map key so that every server advertises a + // distinct name in serverInfo. + graphName := cmp.Or(entry.GraphName, name) + + baseURL := cmp.Or(entry.BaseURL, deps.cfg.Server.BaseURL) + + opts := []func(*mcpserver.Options){ + mcpserver.WithGraphName(graphName), + mcpserver.WithMountPath(entry.Path), mcpserver.WithOperationsDir(operationsDir), - mcpserver.WithListenAddr(r.mcp.Server.ListenAddr), - mcpserver.WithLogger(r.logger.With(logFields...)), - mcpserver.WithExcludeMutations(r.mcp.ExcludeMutations), - mcpserver.WithEnableArbitraryOperations(r.mcp.EnableArbitraryOperations), - mcpserver.WithExposeSchema(r.mcp.ExposeSchema), - mcpserver.WithOmitToolNamePrefix(r.mcp.OmitToolNamePrefix), - mcpserver.WithStateless(r.mcp.Session.Stateless), - mcpserver.WithInstructions(r.mcp.Server.Discover.Instructions), - mcpserver.WithServerVersion(cmp.Or(r.mcp.Server.Version, Version)), - mcpserver.WithServerTitle(r.mcp.Server.Title), - mcpserver.WithServerDescription(r.mcp.Server.Description), + mcpserver.WithLogger(logger), + mcpserver.WithExcludeMutations(entry.ExcludeMutations), + mcpserver.WithEnableArbitraryOperations(entry.EnableArbitraryOperations), + mcpserver.WithExposeSchema(entry.ExposeSchema), + mcpserver.WithOmitToolNamePrefix(entry.OmitToolNamePrefix), + mcpserver.WithStateless(entry.Session.Stateless), + mcpserver.WithInstructions(entry.Discover.Instructions), + mcpserver.WithServerVersion(cmp.Or(entry.Version, deps.routerVersion)), + mcpserver.WithServerTitle(entry.Title), + mcpserver.WithServerDescription(entry.Description), } - if r.corsOptions != nil { - mcpOpts = append(mcpOpts, mcpserver.WithCORS(*r.corsOptions)) + if entry.OAuth.Enabled { + oauth := entry.OAuth + opts = append(opts, mcpserver.WithOAuth(&oauth)) } - // Add OAuth configuration if enabled - if r.mcp.OAuth.Enabled { - mcpOpts = append(mcpOpts, mcpserver.WithOAuth(&r.mcp.OAuth)) + if baseURL != "" { + opts = append(opts, mcpserver.WithServerBaseURL(baseURL)) + } - if r.mcp.Server.BaseURL != "" { - mcpOpts = append(mcpOpts, mcpserver.WithServerBaseURL(r.mcp.Server.BaseURL)) - } + if entry.ResourceDocumentation != "" { + opts = append(opts, mcpserver.WithResourceDocumentation(entry.ResourceDocumentation)) } - if r.mcp.ResourceDocumentation != "" { - mcpOpts = append(mcpOpts, mcpserver.WithResourceDocumentation(r.mcp.ResourceDocumentation)) + return mcpserver.NewGraphQLSchemaServer(ctx, deps.graphqlEndpoint, opts...) +} + +// deprecatedServerName names the single server built from the deprecated +// top-level options. It keeps the previously advertised serverInfo name. +func deprecatedServerName(cfg config.MCPConfiguration) string { + return cmp.Or(cfg.GraphName, "mygraph") +} + +// deprecatedServerEntry maps the deprecated top-level options onto one entry. +func deprecatedServerEntry(cfg config.MCPConfiguration) config.MCPServerEntry { + return config.MCPServerEntry{ + Enabled: true, + Path: mcpserver.DefaultMountPath, + BaseURL: cfg.Server.BaseURL, + Storage: cfg.Storage, + GraphName: cfg.GraphName, + ExcludeMutations: cfg.ExcludeMutations, + EnableArbitraryOperations: cfg.EnableArbitraryOperations, + ExposeSchema: cfg.ExposeSchema, + OmitToolNamePrefix: cfg.OmitToolNamePrefix, + Session: cfg.Session, + OAuth: cfg.OAuth, + ResourceDocumentation: cfg.ResourceDocumentation, + Title: cfg.Server.Title, + Description: cfg.Server.Description, + Version: cfg.Server.Version, + Discover: cfg.Server.Discover, + } +} + +// warnIgnoredDeprecatedMCPOptions names every deprecated option the user set +// while mcp.servers has entries, because the router ignores all of them. +func warnIgnoredDeprecatedMCPOptions(cfg config.MCPConfiguration, logger *zap.Logger) { + var ignored []string + + if cfg.GraphName != "" { + ignored = append(ignored, "mcp.graph_name") + } + if cfg.Storage.ProviderID != "" { + ignored = append(ignored, "mcp.storage") + } + if cfg.ExcludeMutations { + ignored = append(ignored, "mcp.exclude_mutations") + } + if cfg.EnableArbitraryOperations { + ignored = append(ignored, "mcp.enable_arbitrary_operations") + } + if cfg.ExposeSchema { + ignored = append(ignored, "mcp.expose_schema") + } + if cfg.OmitToolNamePrefix { + ignored = append(ignored, "mcp.omit_tool_name_prefix") + } + if cfg.OAuth.Enabled { + ignored = append(ignored, "mcp.oauth") + } + if cfg.RouterURL != "" { + ignored = append(ignored, "mcp.router_url") + } + if cfg.ResourceDocumentation != "" { + ignored = append(ignored, "mcp.resource_documentation") } - mcpGraphQLEndpoint := r.graphqlEndpointURL - if r.mcp.RouterURL != "" { - mcpGraphQLEndpoint = r.mcp.RouterURL + if len(ignored) == 0 { + return } - mcpss, err := mcpserver.NewGraphQLSchemaServer( - ctx, - mcpGraphQLEndpoint, - mcpOpts..., + logger.Warn("Ignoring deprecated top-level mcp options because mcp.servers is set", + zap.Strings("ignored_options", ignored), ) +} + +// corsConfigOrZero dereferences the router CORS options, which may be nil. +func corsConfigOrZero(c *cors.Config) cors.Config { + if c == nil { + return cors.Config{} + } + return *c +} + +// startMCPServer initializes and starts the MCP servers if enabled. +func (r *Router) startMCPServer(ctx context.Context) error { + if !r.mcp.Enabled { + return nil + } + + host, err := buildMCPHost(ctx, mcpHostDeps{ + cfg: r.mcp, + logger: r.logger, + graphqlEndpoint: cmp.Or(r.mcp.RouterURL, r.graphqlEndpointURL), + routerVersion: Version, + corsOptions: r.corsOptions, + providerRegistry: r.providerRegistry, + }) if err != nil { - return fmt.Errorf("failed to create mcp server: %w", err) + return fmt.Errorf("failed to create mcp servers: %w", err) } - if err := mcpss.Start(); err != nil { - // Cleanup the server if Start() fails to prevent resource leaks - if stopErr := mcpss.Stop(ctx); stopErr != nil { - r.logger.Warn("Failed to stop MCP server during error cleanup", zap.Error(stopErr)) + if err := host.Start(); err != nil { + if stopErr := host.Stop(ctx); stopErr != nil { + r.logger.Warn("Failed to stop MCP host during error cleanup", zap.Error(stopErr)) } - return fmt.Errorf("failed to start MCP server: %w", err) + return fmt.Errorf("failed to start MCP servers: %w", err) } - r.mcpServer = mcpss + r.mcpHost = host + return nil } @@ -1900,9 +2038,9 @@ func (r *Router) Shutdown(ctx context.Context) error { }) } - if r.mcpServer != nil { + if r.mcpHost != nil { wg.Go(func() { - if subErr := r.mcpServer.Stop(ctx); subErr != nil { + if subErr := r.mcpHost.Stop(ctx); subErr != nil { err.Append(fmt.Errorf("failed to shutdown mcp server: %w", subErr)) } }) diff --git a/router/core/router_config.go b/router/core/router_config.go index c61bd97167..8d4e81d191 100644 --- a/router/core/router_config.go +++ b/router/core/router_config.go @@ -127,7 +127,7 @@ type Config struct { accessController *AccessController retryOptions retrytransport.RetryOptions redisClient rd.RDCloser - mcpServer *mcpserver.GraphQLSchemaServer + mcpHost *mcpserver.Host connectRPCServer *connectrpc.Server processStartTime time.Time developmentMode bool diff --git a/router/core/router_test.go b/router/core/router_test.go index 147a765593..d3ac5e3a2b 100644 --- a/router/core/router_test.go +++ b/router/core/router_test.go @@ -1,14 +1,18 @@ package core import ( + "context" "net/url" + "slices" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/wundergraph/cosmo/router/gen/proto/wg/cosmo/common" nodev1 "github.com/wundergraph/cosmo/router/gen/proto/wg/cosmo/node/v1" "github.com/wundergraph/cosmo/router/pkg/config" + "go.uber.org/zap" ) func TestOverrideURLConfig(t *testing.T) { @@ -407,3 +411,52 @@ func TestNewTransportRequestOptions(t *testing.T) { assert.Equal(t, defaults.MaxIdleConns, transportCfg.MaxIdleConns) assert.Equal(t, defaults.MaxIdleConnsPerHost, transportCfg.MaxIdleConnsPerHost) } + +func TestMCPServersMapMountsEachServerOnItsPath(t *testing.T) { + t.Parallel() + + cfg := config.MCPConfiguration{ + Enabled: true, + Servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true, Path: "/mcp/support"}, + "billing": {Enabled: true, Path: "/billing/mcp"}, + }, + } + + host, err := buildMCPHost(context.Background(), mcpHostDeps{ + cfg: cfg, + logger: zap.NewNop(), + graphqlEndpoint: "http://localhost:3002/graphql", + routerVersion: "test", + }) + require.NoError(t, err) + t.Cleanup(func() { _ = host.Stop(context.Background()) }) + + paths := make([]string, 0, len(host.Servers())) + for _, s := range host.Servers() { + paths = append(paths, s.MountPath()) + } + slices.Sort(paths) + + require.Equal(t, []string{"/billing/mcp", "/mcp/support"}, paths) +} + +func TestMCPServersMapRejectsDuplicatePaths(t *testing.T) { + t.Parallel() + + cfg := config.MCPConfiguration{ + Enabled: true, + Servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true, Path: "/mcp"}, + "billing": {Enabled: true, Path: "/mcp"}, + }, + } + + _, err := buildMCPHost(context.Background(), mcpHostDeps{ + cfg: cfg, + logger: zap.NewNop(), + graphqlEndpoint: "http://localhost:3002/graphql", + routerVersion: "test", + }) + require.ErrorContains(t, err, "use the same path") +} From e39bcb194d5781e6b61078a029cff4a46e47baf8 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 16:27:50 +0100 Subject: [PATCH 10/18] fix(mcp): scope router_url to deprecated path and close partial builds --- router/core/router.go | 75 +++++++++++++-- router/core/router_test.go | 182 +++++++++++++++++++++++++++++++++++++ 2 files changed, 250 insertions(+), 7 deletions(-) diff --git a/router/core/router.go b/router/core/router.go index 15d1296cd6..0aec56098a 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -1193,7 +1193,9 @@ type mcpHostDeps struct { // existing config keeps working. func buildMCPHost(ctx context.Context, deps mcpHostDeps) (*mcpserver.Host, error) { entries := deps.cfg.Servers - if len(entries) == 0 { + usingDeprecatedOptions := len(entries) == 0 + + if usingDeprecatedOptions { deps.logger.Warn("The top-level mcp options are deprecated. Use the mcp.servers map instead.") entries = map[string]config.MCPServerEntry{ deprecatedServerName(deps.cfg): deprecatedServerEntry(deps.cfg), @@ -1202,6 +1204,11 @@ func buildMCPHost(ctx context.Context, deps mcpHostDeps) (*mcpserver.Host, error warnIgnoredDeprecatedMCPOptions(deps.cfg, deps.logger) } + // mcp.router_url only applies on the deprecated path. deps is a value + // receiver, so this reassignment is local to this call and does not + // leak back to the caller. + deps.graphqlEndpoint = mcpGraphQLEndpoint(deps.cfg, deps.graphqlEndpoint, usingDeprecatedOptions) + if err := mcpserver.ValidateServers(entries); err != nil { return nil, err } @@ -1212,6 +1219,16 @@ func buildMCPHost(ctx context.Context, deps mcpHostDeps) (*mcpserver.Host, error CorsConfig: corsConfigOrZero(deps.corsOptions), }) + // Guarantee every server registered before an early return is closed. + // Host.Stop closes every registered server; it is a no-op beyond that + // when Start has not been called yet, since h.httpServer is still nil. + success := false + defer func() { + if !success { + _ = host.Stop(ctx) + } + }() + names := slices.Sorted(maps.Keys(entries)) for _, name := range names { @@ -1226,17 +1243,34 @@ func buildMCPHost(ctx context.Context, deps mcpHostDeps) (*mcpserver.Host, error } if err := host.Register(srv); err != nil { - // Register did not take ownership of srv, so it will never be - // closed by host.Stop. Close it here to avoid leaking its - // background context, which drives JWKS key refresh. + // Register did not take ownership of srv, so host.Stop will + // never see it. Close it here to avoid leaking its background + // context, which drives JWKS key refresh. srv.Close() return nil, fmt.Errorf("mcp server %q: %w", name, err) } } + success = true + return host, nil } +// mcpGraphQLEndpoint resolves the GraphQL endpoint used to build MCP servers. +// +// mcp.router_url is a deprecated top-level option. It must apply only when +// the deprecated top-level mcp options build the single server. When +// mcp.servers has entries, the router's own GraphQL endpoint is used +// unconditionally, otherwise a router_url set alongside mcp.servers would +// silently redirect every map-based server while warnIgnoredDeprecatedMCPOptions +// simultaneously reports it as ignored. +func mcpGraphQLEndpoint(cfg config.MCPConfiguration, routerEndpoint string, usingDeprecatedOptions bool) string { + if usingDeprecatedOptions { + return cmp.Or(cfg.RouterURL, routerEndpoint) + } + return routerEndpoint +} + // newMCPServerFromEntry builds one MCP server from its config entry. func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPServerEntry, deps mcpHostDeps) (*mcpserver.GraphQLSchemaServer, error) { var operationsDir string @@ -1292,10 +1326,17 @@ func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPSer return mcpserver.NewGraphQLSchemaServer(ctx, deps.graphqlEndpoint, opts...) } +// defaultMCPGraphName mirrors config.MCPConfiguration.GraphName's +// envDefault:"mygraph". LoadConfig runs env.Parse before the YAML merge, so +// GraphName is never empty in a real deployment, whether or not the user +// set mcp.graph_name. Compare against this constant, not against "", or +// every mcp.servers deployment gets a spurious ignored-options warning. +const defaultMCPGraphName = "mygraph" + // deprecatedServerName names the single server built from the deprecated // top-level options. It keeps the previously advertised serverInfo name. func deprecatedServerName(cfg config.MCPConfiguration) string { - return cmp.Or(cfg.GraphName, "mygraph") + return cmp.Or(cfg.GraphName, defaultMCPGraphName) } // deprecatedServerEntry maps the deprecated top-level options onto one entry. @@ -1325,7 +1366,10 @@ func deprecatedServerEntry(cfg config.MCPConfiguration) config.MCPServerEntry { func warnIgnoredDeprecatedMCPOptions(cfg config.MCPConfiguration, logger *zap.Logger) { var ignored []string - if cfg.GraphName != "" { + // GraphName carries envDefault:"mygraph" (see defaultMCPGraphName), so it + // is never empty in a real deployment. Compare against the default, not + // against "", or every mcp.servers user gets a spurious warning. + if cfg.GraphName != "" && cfg.GraphName != defaultMCPGraphName { ignored = append(ignored, "mcp.graph_name") } if cfg.Storage.ProviderID != "" { @@ -1352,6 +1396,23 @@ func warnIgnoredDeprecatedMCPOptions(cfg config.MCPConfiguration, logger *zap.Lo if cfg.ResourceDocumentation != "" { ignored = append(ignored, "mcp.resource_documentation") } + // mcp.session is deliberately not checked here: MCPSessionConfig.Stateless + // carries envDefault:"true", so like GraphName it is never at its zero + // value in a real deployment, and true is also a legitimate explicit + // setting. Distinguishing "set to the default" from "never touched" + // would need the same value the config loader already discards. + if cfg.Server.Title != "" { + ignored = append(ignored, "mcp.server.title") + } + if cfg.Server.Description != "" { + ignored = append(ignored, "mcp.server.description") + } + if cfg.Server.Version != "" { + ignored = append(ignored, "mcp.server.version") + } + if cfg.Server.Discover.Instructions != "" { + ignored = append(ignored, "mcp.server.discover.instructions") + } if len(ignored) == 0 { return @@ -1379,7 +1440,7 @@ func (r *Router) startMCPServer(ctx context.Context) error { host, err := buildMCPHost(ctx, mcpHostDeps{ cfg: r.mcp, logger: r.logger, - graphqlEndpoint: cmp.Or(r.mcp.RouterURL, r.graphqlEndpointURL), + graphqlEndpoint: r.graphqlEndpointURL, routerVersion: Version, corsOptions: r.corsOptions, providerRegistry: r.providerRegistry, diff --git a/router/core/router_test.go b/router/core/router_test.go index d3ac5e3a2b..26dd604d7b 100644 --- a/router/core/router_test.go +++ b/router/core/router_test.go @@ -12,7 +12,10 @@ import ( "github.com/wundergraph/cosmo/router/gen/proto/wg/cosmo/common" nodev1 "github.com/wundergraph/cosmo/router/gen/proto/wg/cosmo/node/v1" "github.com/wundergraph/cosmo/router/pkg/config" + "github.com/wundergraph/cosmo/router/pkg/mcpserver" "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" ) func TestOverrideURLConfig(t *testing.T) { @@ -460,3 +463,182 @@ func TestMCPServersMapRejectsDuplicatePaths(t *testing.T) { }) require.ErrorContains(t, err, "use the same path") } + +// TestMCPDeprecatedOptionsBuildOneServerOnDefaultPath covers the deprecated +// path: no mcp.servers entries means exactly one server, on DefaultMountPath, +// so an existing config keeps working unchanged. +func TestMCPDeprecatedOptionsBuildOneServerOnDefaultPath(t *testing.T) { + t.Parallel() + + cfg := config.MCPConfiguration{ + Enabled: true, + GraphName: "legacy-graph", + } + + host, err := buildMCPHost(context.Background(), mcpHostDeps{ + cfg: cfg, + logger: zap.NewNop(), + graphqlEndpoint: "http://localhost:3002/graphql", + routerVersion: "test", + }) + require.NoError(t, err) + t.Cleanup(func() { _ = host.Stop(context.Background()) }) + + servers := host.Servers() + require.Len(t, servers, 1) + require.Equal(t, mcpserver.DefaultMountPath, servers[0].MountPath()) +} + +// TestDeprecatedServerEntryCarriesTopLevelOptions asserts that every +// deprecated top-level mcp option actually reaches the single server built +// from it. GraphQLSchemaServer exposes no getters for most of these fields, +// so this checks the adapter that feeds them in, deprecatedServerEntry, +// directly. +func TestDeprecatedServerEntryCarriesTopLevelOptions(t *testing.T) { + t.Parallel() + + cfg := config.MCPConfiguration{ + Enabled: true, + GraphName: "legacy-graph", + ExcludeMutations: true, + EnableArbitraryOperations: true, + ExposeSchema: true, + OmitToolNamePrefix: true, + ResourceDocumentation: "https://example.com/docs", + Storage: config.MCPStorageConfig{ + ProviderID: "legacy-fs", + }, + Server: config.MCPServer{ + BaseURL: "https://example.com", + Title: "Legacy", + Description: "Legacy MCP server", + Version: "1.2.3", + }, + } + + entry := deprecatedServerEntry(cfg) + + require.True(t, entry.Enabled) + require.Equal(t, mcpserver.DefaultMountPath, entry.Path) + require.Equal(t, "legacy-graph", entry.GraphName) + require.True(t, entry.ExcludeMutations) + require.True(t, entry.EnableArbitraryOperations) + require.True(t, entry.ExposeSchema) + require.True(t, entry.OmitToolNamePrefix) + require.Equal(t, "https://example.com/docs", entry.ResourceDocumentation) + require.Equal(t, "legacy-fs", entry.Storage.ProviderID) + require.Equal(t, "https://example.com", entry.BaseURL) + require.Equal(t, "Legacy", entry.Title) + require.Equal(t, "Legacy MCP server", entry.Description) + require.Equal(t, "1.2.3", entry.Version) +} + +// TestMCPGraphQLEndpointUsesRouterURLOnDeprecatedPath asserts that +// mcp.router_url still redirects the single deprecated-path server, keeping +// existing deployments that set it working unchanged. +func TestMCPGraphQLEndpointUsesRouterURLOnDeprecatedPath(t *testing.T) { + t.Parallel() + + cfg := config.MCPConfiguration{ + Enabled: true, + RouterURL: "http://deprecated.example/graphql", + } + + got := mcpGraphQLEndpoint(cfg, "http://router.example/graphql", true) + require.Equal(t, "http://deprecated.example/graphql", got) +} + +// TestMCPGraphQLEndpointIgnoresRouterURLForServersMap asserts that +// mcp.router_url does NOT leak into servers built from mcp.servers, even +// when both are set at once. Without this, every map-based server would be +// silently redirected to router_url while warnIgnoredDeprecatedMCPOptions +// simultaneously tells the user it was ignored. +func TestMCPGraphQLEndpointIgnoresRouterURLForServersMap(t *testing.T) { + t.Parallel() + + cfg := config.MCPConfiguration{ + Enabled: true, + RouterURL: "http://deprecated.example/graphql", + Servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true, Path: "/mcp/support"}, + }, + } + + got := mcpGraphQLEndpoint(cfg, "http://router.example/graphql", false) + require.Equal(t, "http://router.example/graphql", got) +} + +// TestWarnIgnoredDeprecatedMCPOptionsSkipsUntouchedGraphName asserts that +// mcp.graph_name is not reported as ignored merely because it carries its +// envDefault value. GraphName defaults to "mygraph" via env.Parse before the +// YAML merge, so a bare non-empty check would warn on every mcp.servers +// deployment regardless of whether the user ever set it. +func TestWarnIgnoredDeprecatedMCPOptionsSkipsUntouchedGraphName(t *testing.T) { + t.Parallel() + + cfg := config.MCPConfiguration{ + Enabled: true, + GraphName: defaultMCPGraphName, + Servers: map[string]config.MCPServerEntry{ + "support": {Enabled: true, Path: "/mcp/support"}, + }, + } + + obsCore, logs := observer.New(zapcore.WarnLevel) + logger := zap.New(obsCore) + + warnIgnoredDeprecatedMCPOptions(cfg, logger) + + for _, entry := range logs.All() { + for _, field := range entry.Context { + if field.Key == "ignored_options" { + require.NotContains(t, field.Interface, "mcp.graph_name") + } + } + } +} + +// TestBuildMCPHostClosesAlreadyRegisteredServersOnFailure covers a +// multi-entry config where a later entry fails after earlier entries were +// already registered on the host. Register does not take ownership on +// failure and newMCPServerFromEntry can fail for reasons ValidateServers +// cannot catch statically (here: an unknown storage provider id), so +// buildMCPHost must guarantee cleanup of everything registered so far. +// +// GraphQLSchemaServer exposes no way to observe from outside the mcpserver +// package whether Close (and the context cancellation it drives) actually +// ran on the earlier, already-registered server, so this test cannot assert +// "the first server was closed" directly. What it does assert: the call +// fails, the error names the failing entry (not the one that already +// succeeded), and no host is handed back for the caller to accidentally +// use half-built. The cleanup guarantee itself is enforced by the +// success-flag/defer construction in buildMCPHost, which runs host.Stop on +// every early return, verified by reading the implementation. +func TestBuildMCPHostClosesAlreadyRegisteredServersOnFailure(t *testing.T) { + t.Parallel() + + cfg := config.MCPConfiguration{ + Enabled: true, + Servers: map[string]config.MCPServerEntry{ + "a-first": {Enabled: true, Path: "/mcp/a"}, + "b-second": { + Enabled: true, + Path: "/mcp/b", + Storage: config.MCPStorageConfig{ProviderID: "missing-provider"}, + }, + "c-third": {Enabled: true, Path: "/mcp/c"}, + }, + } + + host, err := buildMCPHost(context.Background(), mcpHostDeps{ + cfg: cfg, + logger: zap.NewNop(), + graphqlEndpoint: "http://localhost:3002/graphql", + routerVersion: "test", + providerRegistry: &ProviderRegistry{}, + }) + + require.Nil(t, host) + require.ErrorContains(t, err, "b-second") + require.ErrorContains(t, err, "missing-provider") +} From 845d31aea7b13d9ac45e631c5e2aaa15ed8f27ef Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 16:37:00 +0100 Subject: [PATCH 11/18] fix(mcp): keep an unreadable collection from failing the whole reload --- router/pkg/mcpserver/host_test.go | 50 +++++++++++++++++++++++++++++++ router/pkg/mcpserver/server.go | 7 ++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/router/pkg/mcpserver/host_test.go b/router/pkg/mcpserver/host_test.go index a76b7a540f..02184b988e 100644 --- a/router/pkg/mcpserver/host_test.go +++ b/router/pkg/mcpserver/host_test.go @@ -3,9 +3,13 @@ package mcpserver import ( "context" "net/http" + "path/filepath" "testing" "github.com/stretchr/testify/require" + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" + "github.com/wundergraph/graphql-go-tools/v2/pkg/asttransform" "go.uber.org/zap" ) @@ -24,6 +28,35 @@ func newTestServer(t *testing.T, mountPath string) *GraphQLSchemaServer { return srv } +func newTestServerWithOperationsDir(t *testing.T, mountPath, operationsDir string) *GraphQLSchemaServer { + t.Helper() + + srv, err := NewGraphQLSchemaServer( + context.Background(), + "http://localhost:3002/graphql", + WithMountPath(mountPath), + WithOperationsDir(operationsDir), + WithLogger(zap.NewNop()), + ) + require.NoError(t, err) + t.Cleanup(srv.Close) + + return srv +} + +// testSchemaDocument builds the same schema fixture the Reload tests in +// server_test.go use, so this test exercises the same code path. +func testSchemaDocument(t *testing.T) *ast.Document { + t.Helper() + + schemaDoc, report := astparser.ParseGraphqlDocumentString(testSchema) + require.False(t, report.HasErrors()) + err := asttransform.MergeDefinitionWithBaseSchema(&schemaDoc) + require.NoError(t, err) + + return &schemaDoc +} + func mustRequest(method, path string) *http.Request { req, err := http.NewRequest(method, "http://localhost"+path, nil) if err != nil { @@ -59,3 +92,20 @@ func TestHostRejectsDuplicatePaths(t *testing.T) { err := h.Register(newTestServer(t, "/mcp")) require.ErrorContains(t, err, "already registered") } + +func TestHostReloadIsolatesAnUnreadableCollection(t *testing.T) { + t.Parallel() + + good := newTestServerWithOperationsDir(t, "/mcp/support", t.TempDir()) + bad := newTestServerWithOperationsDir(t, "/billing/mcp", filepath.Join(t.TempDir(), "does-not-exist")) + + h := NewHost(HostOptions{ListenAddr: "localhost:0", Logger: zap.NewNop()}) + require.NoError(t, h.Register(good)) + require.NoError(t, h.Register(bad)) + + // The broken collection must not fail the reload of the whole host. + require.NoError(t, h.Reload(testSchemaDocument(t), nil)) + + // The healthy server still built its tool registry. + require.NotNil(t, good.operationsManager) +} diff --git a/router/pkg/mcpserver/server.go b/router/pkg/mcpserver/server.go index 7a491069f2..08b0a8c618 100644 --- a/router/pkg/mcpserver/server.go +++ b/router/pkg/mcpserver/server.go @@ -525,7 +525,12 @@ func (s *GraphQLSchemaServer) Reload(schema *ast.Document, fieldConfigs []*nodev if s.operationsDir != "" { if err := s.operationsManager.LoadOperationsFromDirectory(s.operationsDir); err != nil { - return fmt.Errorf("failed to load operations: %w", err) + // An unreadable collection is an environment fault that belongs to + // this server. Serve no tools rather than failing the whole reload. + s.logger.Error("Failed to load MCP operations, serving no tools", + zap.String("operations_dir", s.operationsDir), + zap.Error(err), + ) } } From 2f95ae7b6235c195502ac8f776164def37e19813 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 16:43:08 +0100 Subject: [PATCH 12/18] test(mcp): assert bad server tools to give the reload isolation test teeth --- router/pkg/mcpserver/host_test.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/router/pkg/mcpserver/host_test.go b/router/pkg/mcpserver/host_test.go index 02184b988e..e5224baaa1 100644 --- a/router/pkg/mcpserver/host_test.go +++ b/router/pkg/mcpserver/host_test.go @@ -96,7 +96,12 @@ func TestHostRejectsDuplicatePaths(t *testing.T) { func TestHostReloadIsolatesAnUnreadableCollection(t *testing.T) { t.Parallel() - good := newTestServerWithOperationsDir(t, "/mcp/support", t.TempDir()) + goodDir := t.TempDir() + writeOperationFiles(t, goodDir, map[string]string{ + "FindEmployee.graphql": findEmployeeOp, + }) + + good := newTestServerWithOperationsDir(t, "/mcp/support", goodDir) bad := newTestServerWithOperationsDir(t, "/billing/mcp", filepath.Join(t.TempDir(), "does-not-exist")) h := NewHost(HostOptions{ListenAddr: "localhost:0", Logger: zap.NewNop()}) @@ -106,6 +111,20 @@ func TestHostReloadIsolatesAnUnreadableCollection(t *testing.T) { // The broken collection must not fail the reload of the whole host. require.NoError(t, h.Reload(testSchemaDocument(t), nil)) - // The healthy server still built its tool registry. - require.NotNil(t, good.operationsManager) + // The healthy server actually registered its operation tool, not merely + // a non-nil operationsManager pointer: that field is assigned before the + // operations directory is even read, so a nil check alone proves nothing. + require.Contains(t, good.registeredTools, "execute_operation_find_employee") + + // The broken collection registers no operation tools, but Reload no + // longer aborts before registerTools runs: the built-in tools are still + // present. + require.Contains(t, bad.registeredTools, "get_schema") + require.Contains(t, bad.registeredTools, "get_operation_info") + require.NotContains(t, bad.registeredTools, "execute_operation_find_employee") + + // Calling Reload directly on the broken server, not only through the + // host, must return nil. Before this fix it returned an error and + // aborted before registerTools ran. + require.NoError(t, bad.Reload(testSchemaDocument(t), nil)) } From 359b5bb7cf13fe4cf6189ffae3b71820f315d64d Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 16:57:16 +0100 Subject: [PATCH 13/18] docs(mcp): document the mcp.servers map and the migration path --- docs-website/router/configuration.mdx | 6 + docs-website/router/mcp/configuration.mdx | 249 +++++++++++++++++- .../router/mcp/oauth/configuration.mdx | 11 + 3 files changed, 265 insertions(+), 1 deletion(-) diff --git a/docs-website/router/configuration.mdx b/docs-website/router/configuration.mdx index 99a855042c..3555fe0377 100644 --- a/docs-website/router/configuration.mdx +++ b/docs-website/router/configuration.mdx @@ -329,6 +329,12 @@ introspection: The Model Context Protocol (MCP) server allows AI models to discover and interact with your GraphQL API in a secure way. + + This table documents the deprecated, single-server options. Use `mcp.servers` to run one or more MCP servers from + one router. See [MCP Configuration](/router/mcp/configuration#running-multiple-mcp-servers) for the full + reference, including the migration path from these options. + + | Environment Variable | YAML | Required | Description | Default Value | | ------------------------------- | ------------------------------- | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------- | | MCP_ENABLED | mcp.enabled | | Enable or disable the MCP server | false | diff --git a/docs-website/router/mcp/configuration.mdx b/docs-website/router/mcp/configuration.mdx index d257a4e5fa..134c2178ab 100644 --- a/docs-website/router/mcp/configuration.mdx +++ b/docs-website/router/mcp/configuration.mdx @@ -1,9 +1,15 @@ --- title: 'Configuration' -description: 'Complete reference for all MCP Gateway configuration options, including session handling, storage providers, and environment variables.' +description: 'Complete reference for all MCP Gateway configuration options, including the mcp.servers map, session handling, storage providers, and environment variables.' icon: 'sliders-up' --- + + This page covers two configuration forms. [Multiple MCP Servers](#running-multiple-mcp-servers) documents + `mcp.servers`, the current way to configure MCP. The rest of this section documents the single-server, top-level + options. These options are deprecated. See [Migrating from the Deprecated Top-Level Options](#migrating-from-the-deprecated-top-level-options). + + ## Basic Configuration To enable MCP in your Cosmo Router, add the following to your `config.yaml`: @@ -70,6 +76,232 @@ All MCP options can also be set via environment variables: For OAuth-related environment variables, see [OAuth Configuration Reference](/router/mcp/oauth/configuration#environment-variables). + + These environment variables set the deprecated top-level options only. The `mcp.servers` map is YAML-only. + Environment variables cannot address a map entry. Set every field of a server entry directly in `config.yaml`. + + +## Running Multiple MCP Servers + +Use `mcp.servers` to run one or more MCP servers from a single router instance. Each server gets its own path, its +own operations directory, and, optionally, its own OAuth configuration and base URL. + +`mcp.servers` is a map. Each key names one server. Each value configures that server. + +```yaml +mcp: + enabled: true + server: + listen_addr: 'localhost:5025' + servers: + support: + enabled: true + path: '/mcp/support' + storage: + provider_id: 'support-ops' + billing: + enabled: true + path: '/billing/mcp' + base_url: 'https://billing.example.com' + storage: + provider_id: 'billing-ops' + +storage_providers: + file_system: + - id: 'support-ops' + path: 'operations/support' + - id: 'billing-ops' + path: 'operations/billing' +``` + +`mcp.enabled` must be `true` for any server in `mcp.servers` to start. Set it once, at the top level. When +`mcp.enabled` is `false`, no server starts, whatever the entries say. + +Every server in `mcp.servers` shares: + +- One listener: `mcp.server.listen_addr`. +- The router's global CORS configuration. See [CORS](#cors). + +### Server Entry Fields + +| Field | Description | Default | +| ------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------- | +| `enabled` | Starts this server. A server with no `enabled: true` never starts, even when `mcp.enabled` is `true`. | `false` | +| `path` | The mount path for this server on the shared listener. Required for an enabled server. See [Path Rules](#path-rules). | (required) | +| `base_url` | The public base URL for this server. Overrides `mcp.server.base_url`. Required when this server's `oauth.enabled` is `true`. Used for RFC 9728 metadata and `resource_metadata` headers. | `mcp.server.base_url` | +| `graph_name` | The name of the graph this server exposes. Feeds the `Name` field of `serverInfo` as `wundergraph-cosmo-`. | the map key | +| `storage.provider_id` | The ID of a `file_system` storage provider that holds this server's GraphQL operations. | - | +| `exclude_mutations` | Excludes mutation operations from this server's tools. | `false` | +| `enable_arbitrary_operations` | Enables the `execute_graphql` built-in tool for this server. | `false` | +| `expose_schema` | Enables the `get_schema` built-in tool for this server. | `false` | +| `omit_tool_name_prefix` | Removes the `execute_operation_` prefix from this server's tool names. | `false` | +| `session.stateless` | Runs this server without server-side session state. See the warning below. | `false` | +| `resource_documentation` | A URL to a human-readable page describing this server, its access policies, and how to get started. Included in RFC 9728 metadata if set. | - | +| `title` | A human-readable display name for this server, reported in `serverInfo`. | - | +| `description` | A human-readable description of this server, reported in `serverInfo`. | - | +| `version` | The version this server reports to MCP clients in `serverInfo`. | router version | +| `discover.instructions` | Natural-language guidance for MCP clients on how to use this server. See [Server Discovery](#server-discovery). | - | +| `oauth` | This server's OAuth configuration. Same fields as the top-level `oauth` block. See [OAuth Configuration Reference](/router/mcp/oauth/configuration). | disabled | + + + A field inside an `mcp.servers` entry takes only the value you give it in YAML. It does not inherit the default + documented for the matching top-level option elsewhere on this page. An unset field takes its Go zero value: + `false` for a boolean, an empty string, `0` for a number, or `0s` for a duration. + + Three fields commonly need an explicit value for this reason: + + - `session.stateless` defaults to `false` inside `mcp.servers`. The deprecated top-level `mcp.session.stateless` + defaults to `true`. Set `session.stateless: true` explicitly if you want a stateless server. + - `oauth.max_scope_combinations` defaults to `0` inside `mcp.servers`. The top-level default is `2048`. Set it + explicitly on any server that enables OAuth and uses `@requiresScopes` on more than one field per operation. + - `oauth.jwks[].refresh_interval` defaults to `0s` inside `mcp.servers`. The top-level default is `1m`. Set it + explicitly on any server that enables OAuth with a remote JWKS URL. + + +### Path Rules + +Each server's `path` must follow these rules: + +- It must start with `/`. +- It must not start with `//`. +- It must not end with `/`, unless the path is exactly `/`. +- It must not contain a wildcard character (`{`, `}`, or `*`). +- It must not start with `/.well-known/oauth-protected-resource`. The router reserves this prefix for OAuth + metadata. +- It must be unique among **enabled** servers. Two servers can use the same path when at least one of them is + disabled. + +### Failure Isolation + + + An unknown `storage.provider_id`, a duplicate path, or any other config error stops the router from starting. Fix + the reported server before you restart. + + +An operations directory that the router cannot read affects only its own server. That server starts, serves the +built-in tools, and serves no operation tools. The router logs an error naming the server. Every other server in +`mcp.servers` keeps running. + +### Multiple Servers with OAuth Behind a Load Balancer + + + Set `oauth.jwks[].audiences` on every server that enables OAuth. Without a matching audience, the router accepts + a token minted for one server on every other server behind the same load balancer. + + +Each server's `base_url` and `path` together determine the resource identifier it publishes and the URL where its +RFC 9728 metadata lives. Take this server: + +```yaml +mcp: + servers: + billing: + enabled: true + path: '/billing/mcp' + base_url: 'https://billing.example.com' + oauth: + enabled: true + authorization_server_url: 'https://auth.example.com' + jwks: + - url: 'https://auth.example.com/.well-known/jwks.json' + audiences: + - 'https://billing.example.com/billing/mcp' +``` + +This server publishes: + +- Resource identifier: `https://billing.example.com/billing/mcp` +- RFC 9728 metadata: `https://billing.example.com/.well-known/oauth-protected-resource/billing/mcp` + +Set `oauth.jwks[].audiences` to the resource identifier, `https://billing.example.com/billing/mcp` in this example. +A token whose `aud` claim does not match is rejected, even if it is valid for another server on the same router. + +A load balancer or reverse proxy in front of the router can change the host in these URLs. It must not change the +path. The router derives both URLs from `base_url` and `path`. It does not know the load balancer's own hostname. + +## Migrating from the Deprecated Top-Level Options + +The top-level `mcp` options documented in [Basic Configuration](#basic-configuration) and +[Configuration Options](#configuration-options) are deprecated. `mcp.server.listen_addr` and `mcp.server.base_url` +are not deprecated. They configure the shared listener and its default base URL for every server. + +When `mcp.servers` has one or more entries, the router ignores every deprecated top-level option and logs a warning +naming each one you set: + +``` +Ignoring deprecated top-level mcp options because mcp.servers is set ignored_options=["mcp.graph_name", ...] +``` + +When `mcp.servers` has no entries, the deprecated options build one server on `/mcp`, so an existing config keeps +working unchanged. + +The deprecated options are: + +- `mcp.graph_name` +- `mcp.storage` +- `mcp.exclude_mutations` +- `mcp.enable_arbitrary_operations` +- `mcp.expose_schema` +- `mcp.omit_tool_name_prefix` +- `mcp.oauth` +- `mcp.router_url` +- `mcp.resource_documentation` +- `mcp.server.title` +- `mcp.server.description` +- `mcp.server.version` +- `mcp.server.discover.instructions` + +`mcp.router_url` only applies to the deprecated single-server form. It has no effect once `mcp.servers` has +entries; the router uses its own GraphQL endpoint for every server in the map. + + + Moving to `mcp.servers` can change the identity your MCP server advertises. `graph_name` feeds the `Name` field of + MCP `serverInfo` as `wundergraph-cosmo-`. In the map form, `graph_name` defaults to the map + key, not to the old top-level `graph_name` value. Some MCP clients store trust or configuration against this + name. Set `graph_name` explicitly on the entry to keep the name your clients already trust. + + +**Before**, using the deprecated top-level options: + +```yaml +mcp: + enabled: true + server: + listen_addr: 'localhost:5025' + graph_name: 'mygraph' + exclude_mutations: true + storage: + provider_id: 'mcp' + +storage_providers: + file_system: + - id: 'mcp' + path: 'operations' +``` + +**After**, using `mcp.servers`. The entry sets `graph_name: 'mygraph'` explicitly, so `serverInfo` still reports +`wundergraph-cosmo-mygraph`: + +```yaml +mcp: + enabled: true + server: + listen_addr: 'localhost:5025' + servers: + support: + enabled: true + path: '/mcp' + graph_name: 'mygraph' + exclude_mutations: true + storage: + provider_id: 'mcp' + +storage_providers: + file_system: + - id: 'mcp' + path: 'operations' +``` + ## Storage Providers MCP loads operations from a configured storage provider. Currently, only the `file_system` provider is supported: @@ -107,6 +339,9 @@ mcp: Employee data is refreshed nightly; do not treat it as real-time. ``` +In `mcp.servers`, set `discover.instructions` on each entry. Each server sends its own instructions and does not +inherit them from another server or from the deprecated `mcp.server.discover.instructions`. + The server sends the instructions to every MCP client. This covers clients that connect with `server/discover` and clients that still use the legacy `initialize` handshake. The router advertises protocol version `2026-07-28` by default. In session-based mode (`session.stateless: false`), clients negotiate `2025-11-25` or older. @@ -123,12 +358,24 @@ To configure sticky sessions: For details, see your load balancer or reverse proxy documentation (e.g., [F5 NGINX Plus - MCP Session Affinity](https://community.f5.com/kb/technicalarticles/mcp-session-affinity-with-f5-nginx-plus/341961)). +In `mcp.servers`, sticky sessions apply per server. Each server keeps its own `Mcp-Session-Id` values; route on the +`Mcp-Session-Id` header together with the request path so a session for `/mcp/support` never reaches the +`/billing/mcp` server. + ## CORS The MCP server automatically configures CORS to allow cross-origin requests from MCP clients. It sets `Access-Control-Allow-Origin: *` and allows the required MCP headers (`Mcp-Protocol-Version`, `Mcp-Session-Id`, `Authorization`, `Last-Event-ID`). The `Mcp-Session-Id` and `WWW-Authenticate` headers are exposed in responses. If you have additional CORS headers configured on the router, they are merged with the MCP-specific headers. + + CORS is a property of the shared listener, `mcp.server.listen_addr`. Every server in `mcp.servers` uses the same + CORS configuration. There is no per-server CORS setting. + + ## Full Configuration Example +This example uses the deprecated top-level options, in the single-server form. For an example with +`mcp.servers`, see [Running Multiple MCP Servers](#running-multiple-mcp-servers). + ```yaml mcp: enabled: true diff --git a/docs-website/router/mcp/oauth/configuration.mdx b/docs-website/router/mcp/oauth/configuration.mdx index 054f288906..7ceaa732f1 100644 --- a/docs-website/router/mcp/oauth/configuration.mdx +++ b/docs-website/router/mcp/oauth/configuration.mdx @@ -124,6 +124,17 @@ GET /.well-known/oauth-protected-resource/mcp This follows the [RFC 9728](https://datatracker.ietf.org/doc/rfc9728/) path-aware format. MCP clients use this endpoint to automatically discover the authorization server and all supported scopes. + + This example uses `mcp.server.base_url` and the default `/mcp` path from the deprecated single-server form. With + `mcp.servers`, each server publishes its own metadata at its own `base_url` and `path`. See + [Multiple Servers with OAuth Behind a Load Balancer](/router/mcp/configuration#multiple-servers-with-oauth-behind-a-load-balancer). + + + + When you run more than one MCP server with OAuth, set `oauth.jwks[].audiences` on each server to that server's + resource identifier. Without it, the router accepts a token minted for one server on every other server. + + **Example response:** ```json From 0b7ce0a1487cb963d144e06c9ab8d2be6de37882 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 17:20:04 +0100 Subject: [PATCH 14/18] fix(mcp): apply session and scope defaults to mcp.servers entries --- docs-website/router/mcp/configuration.mdx | 26 ++++---- router/core/router.go | 47 +++++++++++++- router/core/router_test.go | 75 +++++++++++++++++++++++ router/pkg/config/config.go | 51 ++++++++++----- 4 files changed, 170 insertions(+), 29 deletions(-) diff --git a/docs-website/router/mcp/configuration.mdx b/docs-website/router/mcp/configuration.mdx index 134c2178ab..13d31a1aa1 100644 --- a/docs-website/router/mcp/configuration.mdx +++ b/docs-website/router/mcp/configuration.mdx @@ -135,7 +135,7 @@ Every server in `mcp.servers` shares: | `enable_arbitrary_operations` | Enables the `execute_graphql` built-in tool for this server. | `false` | | `expose_schema` | Enables the `get_schema` built-in tool for this server. | `false` | | `omit_tool_name_prefix` | Removes the `execute_operation_` prefix from this server's tool names. | `false` | -| `session.stateless` | Runs this server without server-side session state. See the warning below. | `false` | +| `session.stateless` | Runs this server without server-side session state. See the note below. | `true` | | `resource_documentation` | A URL to a human-readable page describing this server, its access policies, and how to get started. Included in RFC 9728 metadata if set. | - | | `title` | A human-readable display name for this server, reported in `serverInfo`. | - | | `description` | A human-readable description of this server, reported in `serverInfo`. | - | @@ -143,20 +143,24 @@ Every server in `mcp.servers` shares: | `discover.instructions` | Natural-language guidance for MCP clients on how to use this server. See [Server Discovery](#server-discovery). | - | | `oauth` | This server's OAuth configuration. Same fields as the top-level `oauth` block. See [OAuth Configuration Reference](/router/mcp/oauth/configuration). | disabled | - - A field inside an `mcp.servers` entry takes only the value you give it in YAML. It does not inherit the default - documented for the matching top-level option elsewhere on this page. An unset field takes its Go zero value: - `false` for a boolean, an empty string, `0` for a number, or `0s` for a duration. + + A field inside an `mcp.servers` entry takes only the value you give it in YAML. It does not read the `MCP_*` + environment variables documented for the matching top-level option elsewhere on this page. An unset field + normally takes its Go zero value: `false` for a boolean, an empty string, `0` for a number, or `0s` for a + duration. + + `session.stateless` and `oauth.max_scope_combinations` are exceptions. The router applies their documented + default when you leave them unset in an `mcp.servers` entry, so they match the deprecated top-level default: - Three fields commonly need an explicit value for this reason: + - `session.stateless` defaults to `true` inside `mcp.servers`, the same as the deprecated top-level + `mcp.session.stateless`. + - `oauth.max_scope_combinations` defaults to `2048` inside `mcp.servers`, the same as the top-level default. + + One field still needs an explicit value: - - `session.stateless` defaults to `false` inside `mcp.servers`. The deprecated top-level `mcp.session.stateless` - defaults to `true`. Set `session.stateless: true` explicitly if you want a stateless server. - - `oauth.max_scope_combinations` defaults to `0` inside `mcp.servers`. The top-level default is `2048`. Set it - explicitly on any server that enables OAuth and uses `@requiresScopes` on more than one field per operation. - `oauth.jwks[].refresh_interval` defaults to `0s` inside `mcp.servers`. The top-level default is `1m`. Set it explicitly on any server that enables OAuth with a remote JWKS URL. - + ### Path Rules diff --git a/router/core/router.go b/router/core/router.go index 0aec56098a..8d0e906574 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -1271,6 +1271,40 @@ func mcpGraphQLEndpoint(cfg config.MCPConfiguration, routerEndpoint string, usin return routerEndpoint } +// defaultMCPServerMaxScopeCombinations mirrors +// MCPOAuthConfiguration.MaxScopeCombinations's envDefault:"2048". mcp.servers +// entries carry no env tags (env vars cannot address map entries, see +// MCPServerEntry's doc comment), so env.Parse never applies that envDefault +// to a map entry: an unset mcp.servers[*].oauth.max_scope_combinations +// resolves to the Go zero value, 0, not 2048. A limit of 0 makes +// crossProduct (scope_extractor.go) reject every non-empty cross product, +// so ComputeToolScopes fails and the server registers zero tools. Applying +// this fallback in resolveMCPServerMaxScopeCombinations keeps a YAML-only +// server behaviorally identical to the deprecated top-level path for the +// same unset intent. +const defaultMCPServerMaxScopeCombinations = 2048 + +// resolveMCPServerStateless resolves the Session.Stateless setting for one +// mcp.servers entry, defaulting to true (stateless) when the user leaves it +// unset. See MCPServerSessionConfig.Stateless for why this needs a *bool. +func resolveMCPServerStateless(entry config.MCPServerEntry) bool { + if entry.Session.Stateless == nil { + return true + } + return *entry.Session.Stateless +} + +// resolveMCPServerMaxScopeCombinations resolves OAuth.MaxScopeCombinations +// for one mcp.servers entry, falling back to +// defaultMCPServerMaxScopeCombinations when the user leaves it unset (or +// sets it to 0, which is not a meaningful limit either way). +func resolveMCPServerMaxScopeCombinations(entry config.MCPServerEntry) int { + if entry.OAuth.MaxScopeCombinations == 0 { + return defaultMCPServerMaxScopeCombinations + } + return entry.OAuth.MaxScopeCombinations +} + // newMCPServerFromEntry builds one MCP server from its config entry. func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPServerEntry, deps mcpHostDeps) (*mcpserver.GraphQLSchemaServer, error) { var operationsDir string @@ -1303,7 +1337,7 @@ func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPSer mcpserver.WithEnableArbitraryOperations(entry.EnableArbitraryOperations), mcpserver.WithExposeSchema(entry.ExposeSchema), mcpserver.WithOmitToolNamePrefix(entry.OmitToolNamePrefix), - mcpserver.WithStateless(entry.Session.Stateless), + mcpserver.WithStateless(resolveMCPServerStateless(entry)), mcpserver.WithInstructions(entry.Discover.Instructions), mcpserver.WithServerVersion(cmp.Or(entry.Version, deps.routerVersion)), mcpserver.WithServerTitle(entry.Title), @@ -1312,6 +1346,7 @@ func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPSer if entry.OAuth.Enabled { oauth := entry.OAuth + oauth.MaxScopeCombinations = resolveMCPServerMaxScopeCombinations(entry) opts = append(opts, mcpserver.WithOAuth(&oauth)) } @@ -1340,6 +1375,14 @@ func deprecatedServerName(cfg config.MCPConfiguration) string { } // deprecatedServerEntry maps the deprecated top-level options onto one entry. +// +// cfg.Session is a top-level config.MCPSessionConfig, populated by env.Parse +// (envDefault:"true"), so cfg.Session.Stateless always reflects the intended +// value here. Session on config.MCPServerEntry is a *bool-based +// MCPServerSessionConfig instead (see its doc comment for why), so this +// takes the address of cfg.Session.Stateless to carry that already-resolved +// value across. This does not change MCPSessionConfig itself: the +// deprecated top-level path keeps using it unchanged. func deprecatedServerEntry(cfg config.MCPConfiguration) config.MCPServerEntry { return config.MCPServerEntry{ Enabled: true, @@ -1351,7 +1394,7 @@ func deprecatedServerEntry(cfg config.MCPConfiguration) config.MCPServerEntry { EnableArbitraryOperations: cfg.EnableArbitraryOperations, ExposeSchema: cfg.ExposeSchema, OmitToolNamePrefix: cfg.OmitToolNamePrefix, - Session: cfg.Session, + Session: config.MCPServerSessionConfig{Stateless: &cfg.Session.Stateless}, OAuth: cfg.OAuth, ResourceDocumentation: cfg.ResourceDocumentation, Title: cfg.Server.Title, diff --git a/router/core/router_test.go b/router/core/router_test.go index 26dd604d7b..7269be5321 100644 --- a/router/core/router_test.go +++ b/router/core/router_test.go @@ -642,3 +642,78 @@ func TestBuildMCPHostClosesAlreadyRegisteredServersOnFailure(t *testing.T) { require.ErrorContains(t, err, "b-second") require.ErrorContains(t, err, "missing-provider") } + +// TestResolveMCPServerStatelessDefaultsToTrue covers an mcp.servers entry +// that never sets session.stateless. The deprecated top-level path gets +// Stateless=true from MCPSessionConfig's envDefault:"true" tag, but env.Parse +// cannot reach a map entry, so entry.Session.Stateless would be the Go zero +// value if MCPServerSessionConfig used a plain bool. resolveMCPServerStateless +// must restore the documented default (true) for the unset case. +// +// GraphQLSchemaServer exposes no getter for its internal stateless field, so +// this asserts the resolution helper directly, per the package's established +// pattern (see TestBuildMCPHostClosesAlreadyRegisteredServersOnFailure above) +// for testing what package core can actually observe. +func TestResolveMCPServerStatelessDefaultsToTrue(t *testing.T) { + t.Parallel() + + entry := config.MCPServerEntry{Enabled: true, Path: "/mcp"} + + require.True(t, resolveMCPServerStateless(entry)) +} + +// TestResolveMCPServerStatelessRespectsExplicitFalse covers an mcp.servers +// entry that explicitly sets session.stateless: false. This is the assertion +// that proves *bool actually distinguishes "unset" from "explicitly false": +// a plain bool cannot, and a naive "zero value means default" fix would make +// this case indistinguishable from the unset case in +// TestResolveMCPServerStatelessDefaultsToTrue above. +func TestResolveMCPServerStatelessRespectsExplicitFalse(t *testing.T) { + t.Parallel() + + stateless := false + entry := config.MCPServerEntry{ + Enabled: true, + Path: "/mcp", + Session: config.MCPServerSessionConfig{Stateless: &stateless}, + } + + require.False(t, resolveMCPServerStateless(entry)) +} + +// TestResolveMCPServerMaxScopeCombinationsDefaultsTo2048 covers an +// mcp.servers entry that never sets oauth.max_scope_combinations. The +// deprecated top-level path gets 2048 from MCPOAuthConfiguration's +// envDefault:"2048" tag, but env.Parse cannot reach a map entry, so +// entry.OAuth.MaxScopeCombinations is 0 unless resolved. A limit of 0 makes +// crossProduct (scope_extractor.go) reject every non-empty cross product, +// so ComputeToolScopes fails and the server registers zero tools: any +// OAuth-enabled map server with @requiresScopes directives would be +// functionally dead without this fallback. +func TestResolveMCPServerMaxScopeCombinationsDefaultsTo2048(t *testing.T) { + t.Parallel() + + entry := config.MCPServerEntry{ + Enabled: true, + Path: "/mcp", + OAuth: config.MCPOAuthConfiguration{Enabled: true}, + } + + require.Equal(t, 2048, resolveMCPServerMaxScopeCombinations(entry)) +} + +// TestResolveMCPServerMaxScopeCombinationsRespectsExplicitValue covers an +// mcp.servers entry that sets its own oauth.max_scope_combinations. The +// fallback in resolveMCPServerMaxScopeCombinations must not override an +// explicit user value. +func TestResolveMCPServerMaxScopeCombinationsRespectsExplicitValue(t *testing.T) { + t.Parallel() + + entry := config.MCPServerEntry{ + Enabled: true, + Path: "/mcp", + OAuth: config.MCPOAuthConfiguration{Enabled: true, MaxScopeCombinations: 64}, + } + + require.Equal(t, 64, resolveMCPServerMaxScopeCombinations(entry)) +} diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index df4179769c..94d2e97acf 100644 --- a/router/pkg/config/config.go +++ b/router/pkg/config/config.go @@ -1369,22 +1369,22 @@ type MCPConfiguration struct { // This type carries no env tags: the servers map is YAML-only, because env-var // overrides cannot address map entries. type MCPServerEntry struct { - Enabled bool `yaml:"enabled"` - Path string `yaml:"path"` - BaseURL string `yaml:"base_url,omitempty"` - Storage MCPStorageConfig `yaml:"storage,omitempty"` - GraphName string `yaml:"graph_name,omitempty"` - ExcludeMutations bool `yaml:"exclude_mutations"` - EnableArbitraryOperations bool `yaml:"enable_arbitrary_operations"` - ExposeSchema bool `yaml:"expose_schema"` - OmitToolNamePrefix bool `yaml:"omit_tool_name_prefix"` - Session MCPSessionConfig `yaml:"session,omitempty"` - OAuth MCPOAuthConfiguration `yaml:"oauth,omitempty"` - ResourceDocumentation string `yaml:"resource_documentation,omitempty"` - Title string `yaml:"title,omitempty"` - Description string `yaml:"description,omitempty"` - Version string `yaml:"version,omitempty"` - Discover MCPDiscoverConfig `yaml:"discover,omitempty"` + Enabled bool `yaml:"enabled"` + Path string `yaml:"path"` + BaseURL string `yaml:"base_url,omitempty"` + Storage MCPStorageConfig `yaml:"storage,omitempty"` + GraphName string `yaml:"graph_name,omitempty"` + ExcludeMutations bool `yaml:"exclude_mutations"` + EnableArbitraryOperations bool `yaml:"enable_arbitrary_operations"` + ExposeSchema bool `yaml:"expose_schema"` + OmitToolNamePrefix bool `yaml:"omit_tool_name_prefix"` + Session MCPServerSessionConfig `yaml:"session,omitempty"` + OAuth MCPOAuthConfiguration `yaml:"oauth,omitempty"` + ResourceDocumentation string `yaml:"resource_documentation,omitempty"` + Title string `yaml:"title,omitempty"` + Description string `yaml:"description,omitempty"` + Version string `yaml:"version,omitempty"` + Discover MCPDiscoverConfig `yaml:"discover,omitempty"` } type MCPOAuthConfiguration struct { @@ -1430,6 +1430,25 @@ type MCPSessionConfig struct { Stateless bool `yaml:"stateless" envDefault:"true" env:"MCP_SESSION_STATELESS"` } +// MCPServerSessionConfig configures session behavior for one mcp.servers entry. +type MCPServerSessionConfig struct { + // Stateless is a *bool, not a bool. mcp.servers entries carry no env tags + // (see MCPServerEntry's doc comment), so env.Parse never applies + // MCPSessionConfig.Stateless's envDefault:"true" to a map entry. A plain + // bool cannot distinguish "the user left this unset" from "the user set + // it to false", so it would silently default every unset entry to + // session-based mode instead of the documented default, stateless. + // + // This deliberately breaks the repo convention that a bool's Go zero + // value is its default. That convention exists to keep config + // predictable. Here, following it literally would make an omitted + // mcp.servers[*].session.stateless disagree with the deprecated + // top-level mcp.session.stateless for the same user intent, which is + // worse. Resolve this with the resolveMCPServerStateless helper in + // router.go. Do not simplify this back to a plain bool. + Stateless *bool `yaml:"stateless,omitempty"` +} + type MCPStorageConfig struct { ProviderID string `yaml:"provider_id,omitempty" env:"MCP_STORAGE_PROVIDER_ID"` } From b6d33fca63426fde6a4cd397328ded4cbf73c8c6 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 17:27:24 +0100 Subject: [PATCH 15/18] docs(mcp): correct review findings and style violations --- docs-website/router/mcp/configuration.mdx | 47 ++++++++++++------- .../router/mcp/oauth/configuration.mdx | 5 +- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/docs-website/router/mcp/configuration.mdx b/docs-website/router/mcp/configuration.mdx index 13d31a1aa1..c48255e153 100644 --- a/docs-website/router/mcp/configuration.mdx +++ b/docs-website/router/mcp/configuration.mdx @@ -83,8 +83,12 @@ For OAuth-related environment variables, see [OAuth Configuration Reference](/ro ## Running Multiple MCP Servers -Use `mcp.servers` to run one or more MCP servers from a single router instance. Each server gets its own path, its -own operations directory, and, optionally, its own OAuth configuration and base URL. +Use `mcp.servers` to run one or more MCP servers from a single router instance. Each server gets its own: + +- Path +- Operations directory +- OAuth configuration (optional) +- Base URL (optional) `mcp.servers` is a map. Each key names one server. Each value configures that server. @@ -128,7 +132,7 @@ Every server in `mcp.servers` shares: | ------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------- | | `enabled` | Starts this server. A server with no `enabled: true` never starts, even when `mcp.enabled` is `true`. | `false` | | `path` | The mount path for this server on the shared listener. Required for an enabled server. See [Path Rules](#path-rules). | (required) | -| `base_url` | The public base URL for this server. Overrides `mcp.server.base_url`. Required when this server's `oauth.enabled` is `true`. Used for RFC 9728 metadata and `resource_metadata` headers. | `mcp.server.base_url` | +| `base_url` | The public base URL for this server. Overrides `mcp.server.base_url`. When this server's `oauth.enabled` is `true`, one of `base_url` or `mcp.server.base_url` must be set. Used for RFC 9728 metadata and `resource_metadata` headers. | `mcp.server.base_url` | | `graph_name` | The name of the graph this server exposes. Feeds the `Name` field of `serverInfo` as `wundergraph-cosmo-`. | the map key | | `storage.provider_id` | The ID of a `file_system` storage provider that holds this server's GraphQL operations. | - | | `exclude_mutations` | Excludes mutation operations from this server's tools. | `false` | @@ -146,8 +150,12 @@ Every server in `mcp.servers` shares: A field inside an `mcp.servers` entry takes only the value you give it in YAML. It does not read the `MCP_*` environment variables documented for the matching top-level option elsewhere on this page. An unset field - normally takes its Go zero value: `false` for a boolean, an empty string, `0` for a number, or `0s` for a - duration. + normally takes its Go zero value: + + - `false` for a boolean + - An empty string + - `0` for a number + - `0s` for a duration `session.stateless` and `oauth.max_scope_combinations` are exceptions. The router applies their documented default when you leave them unset in an `mcp.servers` entry, so they match the deprecated top-level default: @@ -155,11 +163,6 @@ Every server in `mcp.servers` shares: - `session.stateless` defaults to `true` inside `mcp.servers`, the same as the deprecated top-level `mcp.session.stateless`. - `oauth.max_scope_combinations` defaults to `2048` inside `mcp.servers`, the same as the top-level default. - - One field still needs an explicit value: - - - `oauth.jwks[].refresh_interval` defaults to `0s` inside `mcp.servers`. The top-level default is `1m`. Set it - explicitly on any server that enables OAuth with a remote JWKS URL. ### Path Rules @@ -189,8 +192,8 @@ built-in tools, and serves no operation tools. The router logs an error naming t ### Multiple Servers with OAuth Behind a Load Balancer - Set `oauth.jwks[].audiences` on every server that enables OAuth. Without a matching audience, the router accepts - a token minted for one server on every other server behind the same load balancer. + Without a matching audience, the router accepts a token minted for one server on every other server behind the + same load balancer. Set `oauth.jwks[].audiences` on every server that enables OAuth to prevent this. Each server's `base_url` and `path` together determine the resource identifier it publishes and the URL where its @@ -218,7 +221,8 @@ This server publishes: - RFC 9728 metadata: `https://billing.example.com/.well-known/oauth-protected-resource/billing/mcp` Set `oauth.jwks[].audiences` to the resource identifier, `https://billing.example.com/billing/mcp` in this example. -A token whose `aud` claim does not match is rejected, even if it is valid for another server on the same router. +The router rejects a token whose `aud` claim does not match, even when the token is valid for another server on +the same router. A load balancer or reverse proxy in front of the router can change the host in these URLs. It must not change the path. The router derives both URLs from `base_url` and `path`. It does not know the load balancer's own hostname. @@ -254,12 +258,19 @@ The deprecated options are: - `mcp.server.description` - `mcp.server.version` - `mcp.server.discover.instructions` +- `mcp.session` + +The router's warning does not name `mcp.session`. `mcp.session.stateless` carries a default of `true`, so the +router cannot tell a value you set from the default. The router still ignores it. `mcp.session.stateless: false` +at the top level has no effect once `mcp.servers` has entries, and the router does not warn you. Set +`session.stateless` on each entry that needs it. `mcp.router_url` only applies to the deprecated single-server form. It has no effect once `mcp.servers` has -entries; the router uses its own GraphQL endpoint for every server in the map. +entries. The router uses its own GraphQL endpoint for every server in the map. - Moving to `mcp.servers` can change the identity your MCP server advertises. `graph_name` feeds the `Name` field of + When you do not set `graph_name` explicitly, moving to `mcp.servers` changes the identity your MCP server + advertises. `graph_name` feeds the `Name` field of MCP `serverInfo` as `wundergraph-cosmo-`. In the map form, `graph_name` defaults to the map key, not to the old top-level `graph_name` value. Some MCP clients store trust or configuration against this name. Set `graph_name` explicitly on the entry to keep the name your clients already trust. @@ -362,9 +373,9 @@ To configure sticky sessions: For details, see your load balancer or reverse proxy documentation (e.g., [F5 NGINX Plus - MCP Session Affinity](https://community.f5.com/kb/technicalarticles/mcp-session-affinity-with-f5-nginx-plus/341961)). -In `mcp.servers`, sticky sessions apply per server. Each server keeps its own `Mcp-Session-Id` values; route on the -`Mcp-Session-Id` header together with the request path so a session for `/mcp/support` never reaches the -`/billing/mcp` server. +In `mcp.servers`, sticky sessions apply per server. Each server keeps its own `Mcp-Session-Id` values. Route on +the `Mcp-Session-Id` header together with the request path. This keeps a session for `/mcp/support` from ever +reaching the `/billing/mcp` server. ## CORS diff --git a/docs-website/router/mcp/oauth/configuration.mdx b/docs-website/router/mcp/oauth/configuration.mdx index 7ceaa732f1..b1d085525f 100644 --- a/docs-website/router/mcp/oauth/configuration.mdx +++ b/docs-website/router/mcp/oauth/configuration.mdx @@ -131,8 +131,9 @@ This follows the [RFC 9728](https://datatracker.ietf.org/doc/rfc9728/) path-awar - When you run more than one MCP server with OAuth, set `oauth.jwks[].audiences` on each server to that server's - resource identifier. Without it, the router accepts a token minted for one server on every other server. + Without a matching audience, the router accepts a token minted for one MCP server on every other server. When + you run more than one MCP server with OAuth, set `oauth.jwks[].audiences` on each server to that server's + resource identifier. **Example response:** From 29dd40e1f955734dc256ac10ce08bdc72e2b3a63 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 17:40:19 +0100 Subject: [PATCH 16/18] fix(mcp): scope max scope combinations fallback to servers map --- router/core/router.go | 54 +++++++++++++++++++++++++++++++++----- router/core/router_test.go | 43 ++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/router/core/router.go b/router/core/router.go index 8d0e906574..983fda6bba 100644 --- a/router/core/router.go +++ b/router/core/router.go @@ -1237,7 +1237,7 @@ func buildMCPHost(ctx context.Context, deps mcpHostDeps) (*mcpserver.Host, error continue } - srv, err := newMCPServerFromEntry(ctx, name, entry, deps) + srv, err := newMCPServerFromEntry(ctx, name, entry, deps, !usingDeprecatedOptions) if err != nil { return nil, fmt.Errorf("mcp server %q: %w", name, err) } @@ -1282,11 +1282,26 @@ func mcpGraphQLEndpoint(cfg config.MCPConfiguration, routerEndpoint string, usin // this fallback in resolveMCPServerMaxScopeCombinations keeps a YAML-only // server behaviorally identical to the deprecated top-level path for the // same unset intent. +// +// This fallback must apply only to entries that came from mcp.servers. The +// deprecated top-level oauth.max_scope_combinations DOES receive its +// envDefault:"2048" from env.Parse, so a 0 there was never "unset": it is +// something the operator typed, whether in YAML or via +// MCP_OAUTH_MAX_SCOPE_COMBINATIONS=0. Coercing that to 2048 would silently +// override a value the operator explicitly asked for, on a path that is +// still supported. See newMCPServerFromEntry's fromServersMap parameter. const defaultMCPServerMaxScopeCombinations = 2048 // resolveMCPServerStateless resolves the Session.Stateless setting for one // mcp.servers entry, defaulting to true (stateless) when the user leaves it // unset. See MCPServerSessionConfig.Stateless for why this needs a *bool. +// +// Unlike resolveMCPServerMaxScopeCombinations, this is safe to apply +// unconditionally to both the deprecated top-level path and mcp.servers +// entries: deprecatedServerEntry always supplies a real, non-nil pointer +// (taken from the already-env.Parse-resolved MCPSessionConfig.Stateless), +// so the nil branch here is only ever reached for a genuinely unset +// mcp.servers entry, never for the deprecated path. func resolveMCPServerStateless(entry config.MCPServerEntry) bool { if entry.Session.Stateless == nil { return true @@ -1296,8 +1311,10 @@ func resolveMCPServerStateless(entry config.MCPServerEntry) bool { // resolveMCPServerMaxScopeCombinations resolves OAuth.MaxScopeCombinations // for one mcp.servers entry, falling back to -// defaultMCPServerMaxScopeCombinations when the user leaves it unset (or -// sets it to 0, which is not a meaningful limit either way). +// defaultMCPServerMaxScopeCombinations when the user leaves it unset. Call +// this only for entries built from the mcp.servers map; see +// defaultMCPServerMaxScopeCombinations for why the deprecated top-level +// path must not go through this fallback. func resolveMCPServerMaxScopeCombinations(entry config.MCPServerEntry) int { if entry.OAuth.MaxScopeCombinations == 0 { return defaultMCPServerMaxScopeCombinations @@ -1305,8 +1322,34 @@ func resolveMCPServerMaxScopeCombinations(entry config.MCPServerEntry) int { return entry.OAuth.MaxScopeCombinations } +// resolveMCPServerOAuth resolves the OAuth config to pass to +// mcpserver.WithOAuth for one entry. fromServersMap gates the +// MaxScopeCombinations fallback: true for an mcp.servers entry, false for +// the single synthetic entry built from the deprecated top-level options. +// +// A plain bool parameter, not a second entry-adapter function, because the +// only thing that differs between the two forms here is whether the +// fallback applies; threading a bool through keeps that one difference +// visible at the call site instead of hiding it behind two similarly named +// functions a reader would have to diff against each other. +func resolveMCPServerOAuth(entry config.MCPServerEntry, fromServersMap bool) config.MCPOAuthConfiguration { + oauth := entry.OAuth + if fromServersMap { + oauth.MaxScopeCombinations = resolveMCPServerMaxScopeCombinations(entry) + } + return oauth +} + // newMCPServerFromEntry builds one MCP server from its config entry. -func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPServerEntry, deps mcpHostDeps) (*mcpserver.GraphQLSchemaServer, error) { +// +// fromServersMap distinguishes an entry that came from mcp.servers from the +// single synthetic entry deprecatedServerEntry builds for the deprecated +// top-level options. It gates resolveMCPServerMaxScopeCombinations: that +// fallback must apply only to mcp.servers entries, never to the deprecated +// path, which already gets its default from env.Parse and must keep +// whatever behavior an explicit 0 produced before mcp.servers existed. See +// defaultMCPServerMaxScopeCombinations for the full reasoning. +func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPServerEntry, deps mcpHostDeps, fromServersMap bool) (*mcpserver.GraphQLSchemaServer, error) { var operationsDir string if entry.Storage.ProviderID != "" { @@ -1345,8 +1388,7 @@ func newMCPServerFromEntry(ctx context.Context, name string, entry config.MCPSer } if entry.OAuth.Enabled { - oauth := entry.OAuth - oauth.MaxScopeCombinations = resolveMCPServerMaxScopeCombinations(entry) + oauth := resolveMCPServerOAuth(entry, fromServersMap) opts = append(opts, mcpserver.WithOAuth(&oauth)) } diff --git a/router/core/router_test.go b/router/core/router_test.go index 7269be5321..6fb6120f21 100644 --- a/router/core/router_test.go +++ b/router/core/router_test.go @@ -717,3 +717,46 @@ func TestResolveMCPServerMaxScopeCombinationsRespectsExplicitValue(t *testing.T) require.Equal(t, 64, resolveMCPServerMaxScopeCombinations(entry)) } + +// TestResolveMCPServerOAuthAppliesFallbackForServersMap covers an +// mcp.servers entry (fromServersMap: true) with OAuth enabled and +// max_scope_combinations unset. The resolved OAuth config handed to +// mcpserver.WithOAuth must carry 2048, not 0, or the server registers zero +// tools for any @requiresScopes-protected operation. See +// defaultMCPServerMaxScopeCombinations. +func TestResolveMCPServerOAuthAppliesFallbackForServersMap(t *testing.T) { + t.Parallel() + + entry := config.MCPServerEntry{ + Enabled: true, + Path: "/mcp", + OAuth: config.MCPOAuthConfiguration{Enabled: true}, + } + + oauth := resolveMCPServerOAuth(entry, true) + + require.Equal(t, 2048, oauth.MaxScopeCombinations) +} + +// TestResolveMCPServerOAuthLeavesDeprecatedPathUntouched is the regression +// test for the finding that the first version of this fix silently +// coerced an operator-set 0 to 2048 on the deprecated top-level path too. +// deprecatedServerEntry funnels the top-level options through the same +// MCPServerEntry shape as mcp.servers, so without gating on fromServersMap, +// resolveMCPServerOAuth could not tell "unset map entry" (should become +// 2048) apart from "operator explicitly configured 0 at the top level" +// (must stay 0, whatever failure that produces downstream). This asserts +// fromServersMap: false leaves an explicit 0 as 0. +func TestResolveMCPServerOAuthLeavesDeprecatedPathUntouched(t *testing.T) { + t.Parallel() + + entry := config.MCPServerEntry{ + Enabled: true, + Path: "/mcp", + OAuth: config.MCPOAuthConfiguration{Enabled: true, MaxScopeCombinations: 0}, + } + + oauth := resolveMCPServerOAuth(entry, false) + + require.Equal(t, 0, oauth.MaxScopeCombinations) +} From 73878c51d05fc12a4c6d59d9783f45cca4e3e034 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 18:05:08 +0100 Subject: [PATCH 17/18] fix(mcp): reject root mount path and unblock oauth scope limit --- docs-website/router/mcp/configuration.mdx | 20 ++++++++- router/pkg/config/config.schema.json | 9 +++- router/pkg/config/config_test.go | 54 +++++++++++++++++++++++ router/pkg/mcpserver/paths.go | 10 +++++ router/pkg/mcpserver/paths_test.go | 10 ++++- 5 files changed, 98 insertions(+), 5 deletions(-) diff --git a/docs-website/router/mcp/configuration.mdx b/docs-website/router/mcp/configuration.mdx index c48255e153..bb4ca09337 100644 --- a/docs-website/router/mcp/configuration.mdx +++ b/docs-website/router/mcp/configuration.mdx @@ -131,7 +131,7 @@ Every server in `mcp.servers` shares: | Field | Description | Default | | ------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------- | | `enabled` | Starts this server. A server with no `enabled: true` never starts, even when `mcp.enabled` is `true`. | `false` | -| `path` | The mount path for this server on the shared listener. Required for an enabled server. See [Path Rules](#path-rules). | (required) | +| `path` | The mount path for this server on the shared listener. Required on every entry, enabled or not. See [Path Rules](#path-rules). | (required) | | `base_url` | The public base URL for this server. Overrides `mcp.server.base_url`. When this server's `oauth.enabled` is `true`, one of `base_url` or `mcp.server.base_url` must be set. Used for RFC 9728 metadata and `resource_metadata` headers. | `mcp.server.base_url` | | `graph_name` | The name of the graph this server exposes. Feeds the `Name` field of `serverInfo` as `wundergraph-cosmo-`. | the map key | | `storage.provider_id` | The ID of a `file_system` storage provider that holds this server's GraphQL operations. | - | @@ -171,7 +171,10 @@ Each server's `path` must follow these rules: - It must start with `/`. - It must not start with `//`. -- It must not end with `/`, unless the path is exactly `/`. +- It must not be `/` alone. Every `mcp.servers` entry shares one listener, and Go's router treats `/` as a + catch-all that matches every request, including requests meant for another server. A future release that gives + each server its own listener can lift this restriction. +- It must not end with `/`. - It must not contain a wildcard character (`{`, `}`, or `*`). - It must not start with `/.well-known/oauth-protected-resource`. The router reserves this prefix for OAuth metadata. @@ -189,6 +192,19 @@ An operations directory that the router cannot read affects only its own server. built-in tools, and serves no operation tools. The router logs an error naming the server. Every other server in `mcp.servers` keeps running. + + A per-server reload failure never fails the router or the config reload. This applies to every reload error, + including an unreadable operations directory, a scope computation failure, and a tool registration failure. Alert + on the error log for the affected server. Do not rely on router or config reload failure to detect an MCP + problem. + + +When the router reloads its configuration, it also reloads every server in `mcp.servers`. A server whose scope +computation fails during reload keeps its previous tools. The router removes a server's old tools before it +registers the new ones, so a tool registration failure during reload can leave that server with fewer tools than +before. Both failures log an error naming the server. The router reload itself always succeeds, whatever happens to +any one MCP server. + ### Multiple Servers with OAuth Behind a Load Balancer diff --git a/router/pkg/config/config.schema.json b/router/pkg/config/config.schema.json index c8438e9888..d7a84a62c8 100644 --- a/router/pkg/config/config.schema.json +++ b/router/pkg/config/config.schema.json @@ -2748,8 +2748,8 @@ "enabled": { "type": "boolean", "default": false }, "path": { "type": "string", - "description": "The HTTP path this MCP server is mounted on. Must start with a single slash, must not end with a slash, and must not contain '{', '}', or '*'. Must not be the reserved OAuth protected resource metadata path.", - "pattern": "^/([^/{}*]|[^/{}*][^{}*]*[^/{}*])?$", + "description": "The HTTP path this MCP server is mounted on. Must start with a single slash, must not be '/' alone, must not end with a slash, and must not contain '{', '}', or '*'. Must not be the reserved OAuth protected resource metadata path.", + "pattern": "^/([^/{}*]|[^/{}*][^{}*]*[^/{}*])$", "not": { "pattern": "^/\\.well-known/oauth-protected-resource(/|$)" } }, "base_url": { "type": "string", "format": "http-url" }, @@ -4367,6 +4367,11 @@ "default": false, "description": "When true, includes the token's existing scopes in the scope parameter of 403 insufficient_scope responses (workaround for MCP client SDKs that replace rather than accumulate scopes). When false (default), only the scopes required for the operation are returned (RFC 6750 strict)." }, + "max_scope_combinations": { + "type": "integer", + "default": 2048, + "description": "Sets the upper limit on the number of OR-group combinations produced when computing the Cartesian product of @requiresScopes across fields. Increase for complex RBAC configurations." + }, "jwks": { "type": "array", "description": "List of JWKS (JSON Web Key Set) configurations for JWT token verification. Multiple JWKS providers can be configured for different authentication sources.", diff --git a/router/pkg/config/config_test.go b/router/pkg/config/config_test.go index d08c1e7bd5..6aadfd801c 100644 --- a/router/pkg/config/config_test.go +++ b/router/pkg/config/config_test.go @@ -2459,6 +2459,7 @@ func TestLoadMCPServersMapPathValidation(t *testing.T) { wantErr bool }{ {name: "single character path is valid", path: "/a"}, + {name: "root alone is rejected", path: "/", wantErr: true}, {name: "leading double slash is rejected", path: "//foo", wantErr: true}, {name: "trailing slash is rejected", path: "/mcp/", wantErr: true}, {name: "wildcard is rejected", path: "/mcp/{id}", wantErr: true}, @@ -2492,3 +2493,56 @@ mcp: }) } } + +// TestMCPOAuthMaxScopeCombinations guards against $defs/mcp_oauth rejecting +// max_scope_combinations with "additional properties ... not allowed". That +// field has no environment variable escape hatch for mcp.servers entries, so +// a schema gap there made the option completely unreachable for a server +// configured that way. +func TestMCPOAuthMaxScopeCombinations(t *testing.T) { + t.Parallel() + + t.Run("mcp.servers entry can set it and the value survives parsing", func(t *testing.T) { + t.Parallel() + + f := createTempFileFromFixture(t, ` +version: "1" +mcp: + enabled: true + servers: + support: + enabled: true + path: /mcp/support + storage: + provider_id: support-ops + oauth: + max_scope_combinations: 4096 +`) + + cfg, err := LoadConfig([]string{f}) + require.NoError(t, err) + + require.Equal(t, 4096, cfg.Config.MCP.Servers["support"].OAuth.MaxScopeCombinations) + }) + + t.Run("top-level mcp.oauth can set it and the value survives parsing", func(t *testing.T) { + t.Parallel() + + f := createTempFileFromFixture(t, ` +version: "1" + +graph: + token: "token" + +mcp: + enabled: true + oauth: + max_scope_combinations: 4096 +`) + + cfg, err := LoadConfig([]string{f}) + require.NoError(t, err) + + require.Equal(t, 4096, cfg.Config.MCP.OAuth.MaxScopeCombinations) + }) +} diff --git a/router/pkg/mcpserver/paths.go b/router/pkg/mcpserver/paths.go index 9bbb27c4fb..e975099000 100644 --- a/router/pkg/mcpserver/paths.go +++ b/router/pkg/mcpserver/paths.go @@ -19,6 +19,13 @@ const ( // Mount paths must be exact ServeMux patterns. A trailing slash would make a // subtree pattern, and a wildcard would make a conflicting pattern; either can // capture the requests of another server sharing the mux. +// +// "/" is rejected for the same reason: Go 1.22+ treats it as a subtree +// pattern too, so it matches every request the shared mux receives, whatever +// path the client asked for. This makes "/" a catch-all only because every +// mcp.servers entry shares one listener today. A future ticket that gives +// each server its own listener removes that sharing, and "/" becomes a +// normal, exact mount path again; this rule can relax then. func ValidateMountPath(p string) error { if p == "" { return fmt.Errorf("path is empty") @@ -29,6 +36,9 @@ func ValidateMountPath(p string) error { if strings.HasPrefix(p, "//") { return fmt.Errorf("path %q must not start with //", p) } + if p == "/" { + return fmt.Errorf("path %q is not allowed: on the shared listener, it is a catch-all that matches every request meant for another server", p) + } if len(p) > 1 && strings.HasSuffix(p, "/") { return fmt.Errorf("path %q must not end with /", p) } diff --git a/router/pkg/mcpserver/paths_test.go b/router/pkg/mcpserver/paths_test.go index 487f64c4af..98307fbaf2 100644 --- a/router/pkg/mcpserver/paths_test.go +++ b/router/pkg/mcpserver/paths_test.go @@ -16,7 +16,7 @@ func TestValidateMountPath(t *testing.T) { }{ {name: "simple", path: "/mcp"}, {name: "nested", path: "/billing/mcp"}, - {name: "root", path: "/"}, + {name: "root", path: "/", wantErr: "shared listener"}, {name: "single character", path: "/a"}, {name: "interior double slash", path: "/a//b"}, {name: "empty", path: "", wantErr: "path is empty"}, @@ -46,6 +46,11 @@ func TestMetadataPath(t *testing.T) { require.Equal(t, "/.well-known/oauth-protected-resource/mcp", MetadataPath("/mcp")) require.Equal(t, "/.well-known/oauth-protected-resource/billing/mcp", MetadataPath("/billing/mcp")) + + // ValidateMountPath now rejects "/", so no real config reaches this case. + // Kept because MetadataPath is a pure function with its own "/" branch; + // this is the only test covering that branch, ready for a future ticket + // that gives each server its own listener and allows "/" again. require.Equal(t, "/.well-known/oauth-protected-resource", MetadataPath("/")) } @@ -54,5 +59,8 @@ func TestResourceIdentifier(t *testing.T) { require.Equal(t, "https://example.com/mcp", ResourceIdentifier("https://example.com", "/mcp")) require.Equal(t, "https://example.com/billing/mcp", ResourceIdentifier("https://example.com/", "/billing/mcp")) + + // ValidateMountPath now rejects "/", so no real config reaches this case. + // Kept for the same reason as the "/" case in TestMetadataPath above. require.Equal(t, "https://example.com/", ResourceIdentifier("https://example.com", "/")) } From 89ac4f5832a27b9a62697106fab67e006fb089e9 Mon Sep 17 00:00:00 2001 From: Ahmet Soormally Date: Mon, 10 Aug 2026 21:09:58 +0100 Subject: [PATCH 18/18] docs(mcp): split compound sentences in the failure isolation section --- docs-website/router/mcp/configuration.mdx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs-website/router/mcp/configuration.mdx b/docs-website/router/mcp/configuration.mdx index bb4ca09337..b75bd0f067 100644 --- a/docs-website/router/mcp/configuration.mdx +++ b/docs-website/router/mcp/configuration.mdx @@ -171,9 +171,9 @@ Each server's `path` must follow these rules: - It must start with `/`. - It must not start with `//`. -- It must not be `/` alone. Every `mcp.servers` entry shares one listener, and Go's router treats `/` as a - catch-all that matches every request, including requests meant for another server. A future release that gives - each server its own listener can lift this restriction. +- It must not be `/` alone. Every `mcp.servers` entry shares one listener. Go's router treats `/` as a catch-all + that matches every request, including requests meant for another server. A future release that gives each server + its own listener can lift this restriction. - It must not end with `/`. - It must not contain a wildcard character (`{`, `}`, or `*`). - It must not start with `/.well-known/oauth-protected-resource`. The router reserves this prefix for OAuth @@ -201,9 +201,9 @@ built-in tools, and serves no operation tools. The router logs an error naming t When the router reloads its configuration, it also reloads every server in `mcp.servers`. A server whose scope computation fails during reload keeps its previous tools. The router removes a server's old tools before it -registers the new ones, so a tool registration failure during reload can leave that server with fewer tools than -before. Both failures log an error naming the server. The router reload itself always succeeds, whatever happens to -any one MCP server. +registers the new ones. A tool registration failure during reload can therefore leave that server with fewer tools +than before. Both failures log an error naming the server. The router reload always succeeds, even when one MCP +server fails to reload. ### Multiple Servers with OAuth Behind a Load Balancer