fix(http): give feature header precedence over query

Author: CAOShurong

docs/feature-flags.md

@@ -19,8 +19,9 @@ section in the Insiders docs](./insiders-features.md#how-feature-flags-are-resol
 
 The URL query parameter exists for clients that compose the server URL on the
 user's behalf (hosted IDEs, agent platforms) and cannot set custom headers.
-When both the query parameter and the header are present, the query parameter
-wins — the two channels are never combined.
+When both the query parameter and the header are present, the header wins —
+even when its value is empty, whitespace-only, or contains only unknown flags.
+The two channels are never combined.
 
 Only flags listed in
 [`AllowedFeatureFlags`](../pkg/github/feature_flags.go) can be enabled by

pkg/http/middleware/request_config.go

@@ -18,9 +18,9 @@ const queryParamFeatures = "features"
 
 // WithRequestConfig is a middleware that extracts MCP-related headers and sets them in the request context.
 // This includes readonly mode, toolsets, tools, lockdown mode, insiders mode, and feature flags.
-// Feature flags may also arrive via the `features` URL query parameter; when
-// both are present the query parameter wins, matching how the toolset path
-// segments take precedence over their headers.
+// Feature flags may also arrive via the `features` URL query parameter. When
+// both are present, the X-MCP-Features header takes precedence and the two
+// channels are never combined.
 func WithRequestConfig(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		ctx := r.Context()
@@ -57,18 +57,26 @@ func WithRequestConfig(next http.Handler) http.Handler {
 
 		// Feature flags: presence-based selection. The URL query parameter and
 		// the X-MCP-Features header are separate channels — whichever is
-		// present is used as-is, and they are never combined. When both are
-		// present the query parameter wins, so a client composing the server
-		// URL can always express its intent even when it cannot control
-		// headers. Unknown flags are dropped later by ResolveFeatureFlags
-		// against AllowedFeatureFlags, so neither channel is privileged.
+		// present is used as-is, and they are never combined. Header presence
+		// takes precedence even when its value is empty or contains only
+		// unknown flags, so a query parameter cannot override an explicit
+		// header. Unknown flags are dropped later by ResolveFeatureFlags
+		// against AllowedFeatureFlags.
 		queryFeatures, hasQuery := r.URL.Query()[queryParamFeatures]
-		headerFeatures := r.Header.Get(headers.MCPFeaturesHeader)
+		headerFeatures, hasHeader := r.Header[http.CanonicalHeaderKey(headers.MCPFeaturesHeader)]
 		switch {
-		case hasQuery && strings.TrimSpace(queryFeatures[0]) != "":
-			ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(queryFeatures[0]))
-		case headerFeatures != "":
-			ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(headerFeatures))
+		case hasHeader:
+			headerValue := ""
+			if len(headerFeatures) > 0 {
+				headerValue = headerFeatures[0]
+			}
+			ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(headerValue))
+		case hasQuery:
+			queryValue := ""
+			if len(queryFeatures) > 0 {
+				queryValue = queryFeatures[0]
+			}
+			ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(queryValue))
 		}
 
 		next.ServeHTTP(w, r.WithContext(ctx))

pkg/http/middleware/request_config_test.go

@@ -13,6 +13,7 @@ func TestWithRequestConfigFeatureSelection(t *testing.T) {
 	tests := []struct {
 		name         string
 		url          string
+		headerSet    bool
 		headerValue  string
 		wantFeatures []string
 	}{
@@ -24,18 +25,41 @@ func TestWithRequestConfigFeatureSelection(t *testing.T) {
 		{
 			name:         "header only",
 			url:          "/",
+			headerSet:    true,
 			headerValue:  "mcp_holdback_consolidated_projects",
 			wantFeatures: []string{"mcp_holdback_consolidated_projects"},
 		},
 		{
-			name:         "query parameter wins over header, never combined",
+			name:         "header wins over query parameter, never combined",
 			url:          "/?features=flag_from_query",
+			headerSet:    true,
 			headerValue:  "flag_from_header",
-			wantFeatures: []string{"flag_from_query"},
+			wantFeatures: []string{"flag_from_header"},
+		},
+		{
+			name:         "empty header suppresses query parameter",
+			url:          "/?features=flag_from_query",
+			headerSet:    true,
+			wantFeatures: []string{},
+		},
+		{
+			name:         "whitespace-only header suppresses query parameter",
+			url:          "/?features=flag_from_query",
+			headerSet:    true,
+			headerValue:  "  , \t ",
+			wantFeatures: []string{},
+		},
+		{
+			name:         "unknown header suppresses query parameter",
+			url:          "/?features=flag_from_query",
+			headerSet:    true,
+			headerValue:  "unknown_from_header",
+			wantFeatures: []string{"unknown_from_header"},
 		},
 		{
-			name:         "empty query value falls back to header",
+			name:         "empty query value with header",
 			url:          "/?features=",
+			headerSet:    true,
 			headerValue:  "flag_from_header",
 			wantFeatures: []string{"flag_from_header"},
 		},
@@ -55,7 +79,7 @@ func TestWithRequestConfigFeatureSelection(t *testing.T) {
 			}))
 
 			req := httptest.NewRequest(http.MethodPost, tc.url, nil)
-			if tc.headerValue != "" {
+			if tc.headerSet {
 				req.Header.Set(headers.MCPFeaturesHeader, tc.headerValue)
 			}
 			rec := httptest.NewRecorder()