refactor: centralize feature flag resolution and improve feature checker logic

Author: mattdholloway

internal/ghmcp/server.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 	"os"
 	"os/signal"
-	"slices"
 	"strings"
 	"syscall"
 	"time"
@@ -115,18 +114,8 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
 		return nil, fmt.Errorf("failed to create GitHub clients: %w", err)
 	}
 
-	// Create feature checker — insiders mode expands InsidersFeatureFlags
-	enabledFeatures := cfg.EnabledFeatures
-	if cfg.InsidersMode {
-		enabledFeatures = slices.Clone(enabledFeatures)
-		for _, flag := range github.InsidersFeatureFlags {
-			if !slices.Contains(enabledFeatures, flag) {
-				enabledFeatures = append(enabledFeatures, flag)
-			}
-		}
-	}
-	featureChecker := createFeatureChecker(enabledFeatures)
-	mcpAppsEnabled := slices.Contains(enabledFeatures, github.MCPAppsFeatureFlag)
+	// Create feature checker — resolves explicit features + insiders expansion
+	featureChecker := createFeatureChecker(cfg.EnabledFeatures, cfg.InsidersMode)
 
 	// Create dependencies for tool handlers
 	obs, err := observability.NewExporters(cfg.Logger, metrics.NewNoopMetrics())
@@ -155,8 +144,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
 		WithTools(github.CleanTools(cfg.EnabledTools)).
 		WithExcludeTools(cfg.ExcludeTools).
 		WithServerInstructions().
-		WithFeatureChecker(featureChecker).
-		WithMCPApps(mcpAppsEnabled)
+		WithFeatureChecker(featureChecker)
 
 	// Apply token scope filtering if scopes are known (for PAT filtering)
 	if cfg.TokenScopes != nil {
@@ -177,6 +165,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
 	// and UI assets are available (requires running script/build-ui).
 	// We check availability to allow the feature flag to be enabled without
 	// requiring a UI build (graceful degradation).
+	mcpAppsEnabled, _ := featureChecker(context.Background(), github.MCPAppsFeatureFlag)
 	if mcpAppsEnabled && github.UIAssetsAvailable() {
 		github.RegisterUIResources(ghServer)
 	}
@@ -346,15 +335,11 @@ func RunStdioServer(cfg StdioServerConfig) error {
 	return nil
 }
 
-// createFeatureChecker returns a FeatureFlagChecker that checks if a flag name
-// is present in the provided list of enabled features. For the local server,
-// this is populated from the --features CLI flag.
-func createFeatureChecker(enabledFeatures []string) inventory.FeatureFlagChecker {
-	// Build a set for O(1) lookup
-	featureSet := make(map[string]bool, len(enabledFeatures))
-	for _, f := range enabledFeatures {
-		featureSet[f] = true
-	}
+// createFeatureChecker returns a FeatureFlagChecker that resolves features
+// using the centralized ResolveFeatureFlags function. For the local server,
+// features are resolved once at startup from --features CLI flag + insiders mode.
+func createFeatureChecker(enabledFeatures []string, insidersMode bool) inventory.FeatureFlagChecker {
+	featureSet := github.ResolveFeatureFlags(enabledFeatures, insidersMode)
 	return func(_ context.Context, flagName string) (bool, error) {
 		return featureSet[flagName], nil
 	}

pkg/github/feature_flags.go

@@ -3,6 +3,14 @@ package github
 // MCPAppsFeatureFlag is the feature flag name for MCP Apps (interactive UI forms).
 const MCPAppsFeatureFlag = "remote_mcp_ui_apps"
 
+// AllowedFeatureFlags is the allowlist of feature flags that can be enabled
+// by users via --features CLI flag or X-MCP-Features HTTP header.
+// Only flags in this list are accepted; unknown flags are silently ignored.
+// This is the single source of truth for which flags are user-controllable.
+var AllowedFeatureFlags = []string{
+	MCPAppsFeatureFlag,
+}
+
 // InsidersFeatureFlags is the list of feature flags that insiders mode enables.
 // When insiders mode is active, all flags in this list are treated as enabled.
 // This is the single source of truth for what "insiders" means in terms of
@@ -16,3 +24,31 @@ type FeatureFlags struct {
 	LockdownMode bool
 	InsidersMode bool
 }
+
+// ResolveFeatureFlags computes the effective set of enabled feature flags by:
+//  1. Taking explicitly enabled features (from CLI flags or HTTP headers)
+//  2. Adding insiders-expanded features when insiders mode is active
+//  3. Validating all features against the AllowedFeatureFlags allowlist
+//
+// Returns a set (map) for O(1) lookup by the feature checker.
+func ResolveFeatureFlags(enabledFeatures []string, insidersMode bool) map[string]bool {
+	allowed := make(map[string]bool, len(AllowedFeatureFlags))
+	for _, f := range AllowedFeatureFlags {
+		allowed[f] = true
+	}
+
+	effective := make(map[string]bool)
+	for _, f := range enabledFeatures {
+		if allowed[f] {
+			effective[f] = true
+		}
+	}
+	if insidersMode {
+		for _, f := range InsidersFeatureFlags {
+			if allowed[f] {
+				effective[f] = true
+			}
+		}
+	}
+	return effective
+}

pkg/github/feature_flags_test.go

@@ -136,6 +136,70 @@ func TestHelloWorld_ConditionalBehavior_Featureflag(t *testing.T) {
 	}
 }
 
+func TestResolveFeatureFlags(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name            string
+		enabledFeatures []string
+		insidersMode    bool
+		expectedFlags   []string
+		unexpectedFlags []string
+	}{
+		{
+			name:            "no features, no insiders",
+			enabledFeatures: nil,
+			insidersMode:    false,
+			expectedFlags:   nil,
+			unexpectedFlags: []string{MCPAppsFeatureFlag},
+		},
+		{
+			name:            "explicit feature enabled",
+			enabledFeatures: []string{MCPAppsFeatureFlag},
+			insidersMode:    false,
+			expectedFlags:   []string{MCPAppsFeatureFlag},
+		},
+		{
+			name:            "insiders mode enables insiders flags",
+			enabledFeatures: nil,
+			insidersMode:    true,
+			expectedFlags:   InsidersFeatureFlags,
+		},
+		{
+			name:            "unknown flags are filtered out",
+			enabledFeatures: []string{"unknown_flag", "another_unknown"},
+			insidersMode:    false,
+			unexpectedFlags: []string{"unknown_flag", "another_unknown"},
+		},
+		{
+			name:            "mix of known and unknown flags",
+			enabledFeatures: []string{MCPAppsFeatureFlag, "unknown_flag"},
+			insidersMode:    false,
+			expectedFlags:   []string{MCPAppsFeatureFlag},
+			unexpectedFlags: []string{"unknown_flag"},
+		},
+		{
+			name:            "explicit plus insiders deduplicates",
+			enabledFeatures: []string{MCPAppsFeatureFlag},
+			insidersMode:    true,
+			expectedFlags:   []string{MCPAppsFeatureFlag},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			result := ResolveFeatureFlags(tt.enabledFeatures, tt.insidersMode)
+			for _, flag := range tt.expectedFlags {
+				assert.True(t, result[flag], "expected flag %q to be enabled", flag)
+			}
+			for _, flag := range tt.unexpectedFlags {
+				assert.False(t, result[flag], "expected flag %q to not be enabled", flag)
+			}
+		})
+	}
+}
+
 func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) {
 	t.Parallel()
 

pkg/http/handler.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"log/slog"
 	"net/http"
-	"slices"
 
 	ghcontext "github.com/github/github-mcp-server/pkg/context"
 	"github.com/github/github-mcp-server/pkg/github"
@@ -254,22 +253,16 @@ func DefaultInventoryFactory(_ *ServerConfig, t translations.TranslationHelperFu
 }
 
 // InventoryFiltersForRequest applies filters to the inventory builder
-// based on the request context and headers
+// based on the request context and headers.
+// MCP Apps UI metadata is handled by the builder via the feature checker —
+// no need to check headers here.
 func InventoryFiltersForRequest(r *http.Request, builder *inventory.Builder) *inventory.Builder {
 	ctx := r.Context()
 
 	if ghcontext.IsReadonly(ctx) {
 		builder = builder.WithReadOnly(true)
 	}
 
-	// Enable MCP Apps if the feature flag is present in the request headers
-	// or if insiders mode is active (insiders expands InsidersFeatureFlags).
-	headerFeatures := ghcontext.GetHeaderFeatures(ctx)
-	mcpApps := slices.Contains(headerFeatures, github.MCPAppsFeatureFlag) || ghcontext.IsInsidersMode(ctx)
-	if mcpApps {
-		builder = builder.WithMCPApps(true)
-	}
-
 	toolsets := ghcontext.GetToolsets(ctx)
 	tools := ghcontext.GetTools(ctx)
 

pkg/http/server.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 	"os"
 	"os/signal"
-	"slices"
 	"syscall"
 	"time"
 
@@ -25,12 +24,6 @@ import (
 	"github.com/go-chi/chi/v5"
 )
 
-// knownFeatureFlags are the feature flags that can be enabled via X-MCP-Features header.
-// Only these flags are accepted from headers.
-var knownFeatureFlags = []string{
-	github.MCPAppsFeatureFlag,
-}
-
 type ServerConfig struct {
 	// Version of the server
 	Version string
@@ -213,30 +206,14 @@ func initGlobalToolScopeMap(t translations.TranslationHelperFunc) error {
 	return nil
 }
 
-// createHTTPFeatureChecker creates a feature checker that reads header features from context
-// and validates them against the knownFeatureFlags whitelist.
-// When insiders mode is active, InsidersFeatureFlags are also treated as enabled.
+// createHTTPFeatureChecker creates a feature checker that resolves features
+// per-request by reading header features and insiders mode from context,
+// then validating against the centralized AllowedFeatureFlags allowlist.
 func createHTTPFeatureChecker() inventory.FeatureFlagChecker {
-	// Pre-compute whitelist as set for O(1) lookup
-	knownSet := make(map[string]bool, len(knownFeatureFlags))
-	for _, f := range knownFeatureFlags {
-		knownSet[f] = true
-	}
-
-	// Pre-compute insiders flags as set for O(1) lookup
-	insidersSet := make(map[string]bool, len(github.InsidersFeatureFlags))
-	for _, f := range github.InsidersFeatureFlags {
-		insidersSet[f] = true
-	}
-
 	return func(ctx context.Context, flag string) (bool, error) {
-		if knownSet[flag] && slices.Contains(ghcontext.GetHeaderFeatures(ctx), flag) {
-			return true, nil
-		}
-		// Insiders mode enables all InsidersFeatureFlags
-		if insidersSet[flag] && ghcontext.IsInsidersMode(ctx) {
-			return true, nil
-		}
-		return false, nil
+		headerFeatures := ghcontext.GetHeaderFeatures(ctx)
+		insidersMode := ghcontext.IsInsidersMode(ctx)
+		effective := github.ResolveFeatureFlags(headerFeatures, insidersMode)
+		return effective[flag], nil
 	}
 }

pkg/inventory/builder.go

@@ -14,6 +14,11 @@ var (
 	ErrUnknownTools = errors.New("unknown tools specified in WithTools")
 )
 
+// mcpAppsFeatureFlag is the feature flag name that controls MCP Apps UI metadata.
+// This is defined here to avoid importing pkg/github (which imports pkg/inventory).
+// The value must match github.MCPAppsFeatureFlag.
+const mcpAppsFeatureFlag = "remote_mcp_ui_apps"
+
 // ToolFilter is a function that determines if a tool should be included.
 // Returns true if the tool should be included, false to exclude it.
 type ToolFilter func(ctx context.Context, tool *ServerTool) (bool, error)
@@ -48,7 +53,6 @@ type Builder struct {
 	featureChecker       FeatureFlagChecker
 	filters              []ToolFilter // filters to apply to all tools
 	generateInstructions bool
-	mcpApps              bool
 }
 
 // NewBuilder creates a new Builder.
@@ -154,15 +158,6 @@ func (b *Builder) WithExcludeTools(toolNames []string) *Builder {
 	return b
 }
 
-// WithMCPApps enables or disables MCP Apps UI features.
-// When disabled (default), the "ui" Meta key is stripped from tools
-// so clients won't attempt to load UI resources.
-// Returns self for chaining.
-func (b *Builder) WithMCPApps(enabled bool) *Builder {
-	b.mcpApps = enabled
-	return b
-}
-
 // CreateExcludeToolsFilter creates a ToolFilter that excludes tools by name.
 // Any tool whose name appears in the excluded list will be filtered out.
 // The input slice should already be cleaned (trimmed, deduplicated).
@@ -195,6 +190,19 @@ func cleanTools(tools []string) []string {
 	return cleaned
 }
 
+// checkFeatureFlag checks a feature flag at build time using the builder's feature checker.
+// Returns false if no checker is configured or the flag is not enabled.
+func (b *Builder) checkFeatureFlag(flag string) bool {
+	if b.featureChecker == nil {
+		return false
+	}
+	enabled, err := b.featureChecker(context.Background(), flag)
+	if err != nil {
+		return false
+	}
+	return enabled
+}
+
 // Build creates the final Inventory with all configuration applied.
 // This processes toolset filtering, tool name resolution, and sets up
 // the inventory for use. The returned Inventory is ready for use with
@@ -206,8 +214,10 @@ func cleanTools(tools []string) []string {
 func (b *Builder) Build() (*Inventory, error) {
 	tools := b.tools
 
-	// When MCP Apps is disabled, strip UI metadata from tools
-	if !b.mcpApps {
+	// When MCP Apps feature flag is not enabled, strip UI metadata from tools
+	// so clients won't attempt to load UI resources.
+	// The feature checker is the single source of truth for flag evaluation.
+	if !b.checkFeatureFlag(mcpAppsFeatureFlag) {
 		tools = stripMCPAppsMetadata(tools)
 	}
 

pkg/inventory/registry_test.go

@@ -1723,6 +1723,10 @@ func TestFilteringOrder(t *testing.T) {
 		WithFeatureChecker(checker).
 		WithFilter(filter))
 
+	// Reset call order — Build() may call the checker for MCP Apps metadata.
+	// We're testing the AvailableTools filter order here.
+	callOrder = callOrder[:0]
+
 	_ = reg.AvailableTools(context.Background())
 
 	// Expected order: Enabled, FeatureFlag, ReadOnly (stops here because it's write tool)
@@ -1881,11 +1885,14 @@ func TestWithMCPApps_EnabledPreservesUIMetadata(t *testing.T) {
 		"description": "kept",
 	})
 
-	// MCP Apps enabled - UI meta should be preserved
+	// Feature checker enables MCP Apps - UI meta should be preserved
+	mcpAppsChecker := func(_ context.Context, flag string) (bool, error) {
+		return flag == mcpAppsFeatureFlag, nil
+	}
 	reg := mustBuild(t, NewBuilder().
 		SetTools([]ServerTool{toolWithUI}).
 		WithToolsets([]string{"all"}).
-		WithMCPApps(true))
+		WithFeatureChecker(mcpAppsChecker))
 	available := reg.AvailableTools(context.Background())
 
 	require.Len(t, available, 1)