Simplify feature-flag handling: collapse CSV dual-variant + skip filtering when no checker

.github/workflows/mcp-diff.yml

@@ -19,32 +19,35 @@ jobs:
         with:
           fetch-depth: 0
 
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+
       - name: Build UI
         uses: ./.github/actions/build-ui
 
+      - name: Generate diff configurations
+        id: configs
+        # The generator imports pkg/github so any new entry in
+        # AllowedFeatureFlags is automatically diffed without touching this
+        # workflow. See script/print-mcp-diff-configs/main.go.
+        run: |
+          {
+            echo 'configurations<<MCP_DIFF_EOF'
+            go run ./script/print-mcp-diff-configs
+            echo 'MCP_DIFF_EOF'
+          } >> "$GITHUB_OUTPUT"
+
       - name: Run MCP Server Diff
         uses: SamMorrowDrums/mcp-server-diff@v2.3.5
         with:
-          setup_go: "true"
+          setup_go: "false"
           install_command: go mod download
           start_command: go run ./cmd/github-mcp-server stdio
           env_vars: |
             GITHUB_PERSONAL_ACCESS_TOKEN=test-token
-          configurations: |
-            [
-              {"name": "default", "args": ""},
-              {"name": "read-only", "args": "--read-only"},
-              {"name": "toolsets-repos", "args": "--toolsets=repos"},
-              {"name": "toolsets-issues", "args": "--toolsets=issues"},
-              {"name": "toolsets-context", "args": "--toolsets=context"},
-              {"name": "toolsets-pull_requests", "args": "--toolsets=pull_requests"},
-              {"name": "toolsets-repos,issues", "args": "--toolsets=repos,issues"},
-              {"name": "toolsets-issues,context", "args": "--toolsets=issues,context"},
-              {"name": "toolsets-all", "args": "--toolsets=all"},
-              {"name": "tools-get_me", "args": "--tools=get_me"},
-              {"name": "tools-get_me,list_issues", "args": "--tools=get_me,list_issues"},
-              {"name": "toolsets-repos+read-only", "args": "--toolsets=repos --read-only"}
-            ]
+          configurations: ${{ steps.configs.outputs.configurations }}
 
       - name: Add interpretation note
         if: always()

docs/insiders-features.md

@@ -67,3 +67,35 @@ github-mcp-server stdio --features csv_output
 ```
 
 Because this changes list tool response shape, clients that require JSON list responses should avoid enabling this feature.
+
+---
+
+## How feature flags are resolved
+
+> [!NOTE]
+> This section is for contributors. End users only need the table at the top of this page.
+
+Insiders is a **meta feature flag** — the same shape as `default` or `all` for toolsets. It expands once at startup into a curated set of individual feature flags, and from that point on every code path keys off concrete flags, never `InsidersMode` directly. New experimental work should always get its own flag and then be added to the insiders expansion list, never folded into `insiders` as a catch-all.
+
+### Resolution order
+
+1. **User input.** Users may opt into specific features:
+   - Local server: `--features=<flag>,<flag>` CLI flag (or `GITHUB_FEATURES` env var).
+   - Self-hosted HTTP server: `X-MCP-Features: <flag>,<flag>` request header.
+2. **Allowlist filter.** User-supplied flags are filtered against [`AllowedFeatureFlags`](../pkg/github/feature_flags.go). Anything not on the allowlist is silently dropped — flags missing from the allowlist can only be turned on by remote-server feature management, not by end users.
+3. **Insiders expansion.** If insiders mode is on (`--insiders`, `/insiders` route, or `X-MCP-Insiders: true`), every flag in [`InsidersFeatureFlags`](../pkg/github/feature_flags.go) is unioned in. The insiders expansion is **not** re-validated against the allowlist — insiders is a server-controlled switch that can reach internal-only flags.
+4. **Server-side fallback (remote server only).** Any flag not yet decided falls back to the remote server's feature manager, which can roll a feature out independently of user input or insiders membership.
+
+`AllowedFeatureFlags` and `InsidersFeatureFlags` are deliberately independent sets:
+
+- A flag in **`AllowedFeatureFlags` only** is a regular opt-in: users can turn it on, but insiders does not auto-enable it. Granular issues/PRs flags work this way.
+- A flag in **`InsidersFeatureFlags` only** is reachable through insiders (and remote-server rollouts), but cannot be enabled by user input. Internal-only experiments work this way.
+- A flag in **both** is opt-in for end users *and* automatically on under insiders.
+
+### Adding a new feature flag
+
+1. Add a constant in `pkg/github/feature_flags.go`.
+2. Add it to `AllowedFeatureFlags` if end users should be able to opt in via `--features` / `X-MCP-Features`.
+3. Add it to `InsidersFeatureFlags` if insiders mode should turn it on automatically.
+4. Gate the behavior on the concrete flag (`deps.IsFeatureEnabled(ctx, FeatureFlagX)`), never on `cfg.InsidersMode`. There is a `TestGitHubPackageDoesNotReadInsidersMode` guard test that fails if `pkg/github` reads `InsidersMode` directly.
+5. The MCP-diff CI workflow picks up new entries in `AllowedFeatureFlags` automatically — see `.github/workflows/mcp-diff.yml`.

pkg/github/csv_output.go

@@ -42,24 +42,18 @@ type csvOutputDocument struct {
 	rows     []map[string]string
 }
 
-func withCSVOutputVariants(tools []inventory.ServerTool) []inventory.ServerTool {
-	result := make([]inventory.ServerTool, 0, len(tools))
-	for _, tool := range tools {
-		if !isCSVOutputTool(tool) {
-			result = append(result, tool)
+// withCSVOutput wraps the handler of every default-toolset list_* tool so that,
+// at request time, it checks the csv_output feature flag and converts the JSON
+// text response to CSV when enabled. The tool's schema, name, and scope are
+// unchanged — only the response payload format differs.
+func withCSVOutput(tools []inventory.ServerTool) []inventory.ServerTool {
+	for i := range tools {
+		if !isCSVOutputTool(tools[i]) {
 			continue
 		}
-
-		jsonOnly := tool
-		jsonOnly.FeatureFlagDisable = FeatureFlagCSVOutput
-		result = append(result, jsonOnly)
-
-		csvCapable := tool
-		csvCapable.FeatureFlagEnable = FeatureFlagCSVOutput
-		csvCapable.HandlerFunc = wrapHandlerWithCSVOutput(tool.HandlerFunc)
-		result = append(result, csvCapable)
+		tools[i].HandlerFunc = wrapHandlerWithCSVOutput(tools[i].HandlerFunc)
 	}
-	return result
+	return tools
 }
 
 func isCSVOutputTool(tool inventory.ServerTool) bool {
@@ -75,12 +69,15 @@ func isCSVOutputTool(tool inventory.ServerTool) bool {
 func wrapHandlerWithCSVOutput(next inventory.HandlerFunc) inventory.HandlerFunc {
 	return func(deps any) mcp.ToolHandler {
 		handler := next(deps)
+		csvDeps, _ := deps.(ToolDependencies)
 		return func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
 			result, err := handler(ctx, req)
 			if err != nil || result == nil || result.IsError {
 				return result, err
 			}
-
+			if csvDeps == nil || !csvDeps.IsFeatureEnabled(ctx, FeatureFlagCSVOutput) {
+				return result, nil
+			}
 			return convertJSONTextResultToCSV(result), nil
 		}
 	}

pkg/github/csv_output_test.go

@@ -14,55 +14,55 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestCSVOutputVariantsAreFeatureGated(t *testing.T) {
+func TestCSVOutputAppliedToDefaultListTools(t *testing.T) {
 	listTool := testCSVOutputTool("list_things", `[{"number":1}]`)
 	getTool := testCSVOutputTool("get_thing", `{"number":1}`)
 
-	tools := withCSVOutputVariants([]inventory.ServerTool{listTool, getTool})
-	require.Len(t, tools, 3)
+	tools := withCSVOutput([]inventory.ServerTool{listTool, getTool})
+	require.Len(t, tools, 2)
 
-	inv := buildCSVOutputInventory(t, tools, false)
-	available := inv.AvailableTools(context.Background())
-	require.Len(t, available, 2)
+	// CSV mode does not introduce variants or change tool gating; both tools
+	// remain visible regardless of feature flag state.
+	for _, csvOutputEnabled := range []bool{false, true} {
+		inv := buildCSVOutputInventory(t, tools, csvOutputEnabled)
+		available := inv.AvailableTools(context.Background())
+		require.Len(t, available, 2)
 
-	jsonOnly := requireToolByName(t, available, "list_things")
-	assert.Empty(t, jsonOnly.FeatureFlagEnable)
-	assert.Equal(t, FeatureFlagCSVOutput, jsonOnly.FeatureFlagDisable)
+		listing := requireToolByName(t, available, "list_things")
+		assert.Empty(t, listing.FeatureFlagEnable)
+		assert.Empty(t, listing.FeatureFlagDisable)
 
-	getThing := requireToolByName(t, available, "get_thing")
-	assert.Empty(t, getThing.FeatureFlagEnable)
-	assert.Empty(t, getThing.FeatureFlagDisable)
-
-	inv = buildCSVOutputInventory(t, tools, true)
-	available = inv.AvailableTools(context.Background())
-	require.Len(t, available, 2)
-
-	csvCapable := requireToolByName(t, available, "list_things")
-	assert.Equal(t, FeatureFlagCSVOutput, csvCapable.FeatureFlagEnable)
-	assert.Empty(t, csvCapable.FeatureFlagDisable)
+		getting := requireToolByName(t, available, "get_thing")
+		assert.Empty(t, getting.FeatureFlagEnable)
+		assert.Empty(t, getting.FeatureFlagDisable)
+	}
 }
 
-func TestCSVOutputVariantsOnlyApplyToDefaultToolsets(t *testing.T) {
+func TestCSVOutputOnlyAppliesToDefaultToolsets(t *testing.T) {
 	nonDefaultListTool := testCSVOutputToolWithToolset("list_discussions", `[{"number":1}]`, ToolsetMetadataDiscussions)
 
-	tools := withCSVOutputVariants([]inventory.ServerTool{nonDefaultListTool})
+	tools := withCSVOutput([]inventory.ServerTool{nonDefaultListTool})
 	require.Len(t, tools, 1)
 
-	assert.Empty(t, tools[0].FeatureFlagEnable)
-	assert.Empty(t, tools[0].FeatureFlagDisable)
+	// Non-default toolset list tools are not wrapped: even with the flag on,
+	// the response stays in JSON form.
+	deps := newCSVOutputTestDeps(true)
+	result, err := tools[0].Handler(deps)(ContextWithDeps(context.Background(), deps), testCSVOutputRequest())
+	require.NoError(t, err)
+	assert.JSONEq(t, `[{"number":1}]`, textResult(t, result))
 }
 
-func TestCSVOutputVariantDoesNotExposeFormatParameter(t *testing.T) {
-	tools := withCSVOutputVariants([]inventory.ServerTool{testCSVOutputTool("list_things", `[{"number":1}]`)})
-	csvCapable := requireCSVOutputVariant(t, tools)
+func TestCSVOutputDoesNotExposeFormatParameter(t *testing.T) {
+	tools := withCSVOutput([]inventory.ServerTool{testCSVOutputTool("list_things", `[{"number":1}]`)})
+	require.Len(t, tools, 1)
 
-	schema, ok := csvCapable.Tool.InputSchema.(*jsonschema.Schema)
+	schema, ok := tools[0].Tool.InputSchema.(*jsonschema.Schema)
 	require.True(t, ok)
 	assert.NotContains(t, schema.Properties, "output_format")
 }
 
-func TestCSVOutputVariantConvertsJSONTextToCSV(t *testing.T) {
-	tools := withCSVOutputVariants([]inventory.ServerTool{
+func TestCSVOutputConvertsJSONTextToCSVWhenFlagOn(t *testing.T) {
+	tools := withCSVOutput([]inventory.ServerTool{
 		testCSVOutputTool("list_things", `[
 			{
 				"number": 1,
@@ -72,10 +72,10 @@ func TestCSVOutputVariantConvertsJSONTextToCSV(t *testing.T) {
 			}
 		]`),
 	})
-	inv := buildCSVOutputInventory(t, tools, true)
-	csvCapable := requireToolByName(t, inv.AvailableTools(context.Background()), "list_things")
+	require.Len(t, tools, 1)
 
-	result, err := csvCapable.Handler(nil)(context.Background(), testCSVOutputRequest())
+	deps := newCSVOutputTestDeps(true)
+	result, err := tools[0].Handler(deps)(ContextWithDeps(context.Background(), deps), testCSVOutputRequest())
 	require.NoError(t, err)
 	require.NotNil(t, result)
 	require.False(t, result.IsError)
@@ -92,6 +92,22 @@ func TestCSVOutputVariantConvertsJSONTextToCSV(t *testing.T) {
 	assert.Equal(t, "octocat", row["user.login"])
 }
 
+func TestCSVOutputPreservesOriginalJSONWhenFlagOff(t *testing.T) {
+	const jsonResponse = `[{"number":1,"user":{"login":"octocat"}}]`
+	tools := withCSVOutput([]inventory.ServerTool{testCSVOutputTool("list_things", jsonResponse)})
+	require.Len(t, tools, 1)
+
+	deps := newCSVOutputTestDeps(false)
+	result, err := tools[0].Handler(deps)(ContextWithDeps(context.Background(), deps), testCSVOutputRequest())
+	require.NoError(t, err)
+	require.NotNil(t, result)
+
+	require.Len(t, result.Content, 1)
+	text, ok := result.Content[0].(*mcp.TextContent)
+	require.True(t, ok)
+	assert.JSONEq(t, jsonResponse, text.Text)
+}
+
 func TestCSVOutputVariantMovesMetadataToPreamble(t *testing.T) {
 	csvText, err := jsonTextToCSV(`{
 		"issues": [
@@ -118,22 +134,6 @@ func TestCSVOutputVariantMovesMetadataToPreamble(t *testing.T) {
 	assert.NotContains(t, row, "totalCount")
 }
 
-func TestJSONOnlyVariantPreservesOriginalJSONText(t *testing.T) {
-	const jsonResponse = `[{"number":1,"user":{"login":"octocat"}}]`
-	tools := withCSVOutputVariants([]inventory.ServerTool{testCSVOutputTool("list_things", jsonResponse)})
-	inv := buildCSVOutputInventory(t, tools, false)
-	jsonOnly := requireToolByName(t, inv.AvailableTools(context.Background()), "list_things")
-
-	result, err := jsonOnly.Handler(nil)(context.Background(), testCSVOutputRequest())
-	require.NoError(t, err)
-	require.NotNil(t, result)
-
-	require.Len(t, result.Content, 1)
-	text, ok := result.Content[0].(*mcp.TextContent)
-	require.True(t, ok)
-	assert.JSONEq(t, jsonResponse, text.Text)
-}
-
 func TestJSONTextToCSVFlattensPrimaryRows(t *testing.T) {
 	csvText, err := jsonTextToCSV(`{
 		"discussions": [
@@ -329,40 +329,38 @@ func testCSVOutputToolWithToolset(name string, response string, toolset inventor
 	}
 }
 
-func buildCSVOutputInventory(t *testing.T, tools []inventory.ServerTool, csvOutputEnabled bool) *inventory.Inventory {
+func buildCSVOutputInventory(t *testing.T, tools []inventory.ServerTool, _ bool) *inventory.Inventory {
 	t.Helper()
 
 	inv, err := inventory.NewBuilder().
 		SetTools(tools).
-		WithFeatureChecker(func(_ context.Context, flagName string) (bool, error) {
-			return flagName == FeatureFlagCSVOutput && csvOutputEnabled, nil
-		}).
 		Build()
 	require.NoError(t, err)
 	return inv
 }
 
-func requireToolByName(t *testing.T, tools []inventory.ServerTool, name string) inventory.ServerTool {
-	t.Helper()
+func newCSVOutputTestDeps(csvOutputEnabled bool) ToolDependencies {
+	return csvOutputTestDeps{stubDeps: stubDeps{obsv: stubExporters()}, csvOn: csvOutputEnabled}
+}
 
-	for _, tool := range tools {
-		if tool.Tool.Name == name {
-			return tool
-		}
-	}
-	require.Failf(t, "tool not found", "tool %q not found", name)
-	return inventory.ServerTool{}
+type csvOutputTestDeps struct {
+	stubDeps
+	csvOn bool
 }
 
-func requireCSVOutputVariant(t *testing.T, tools []inventory.ServerTool) inventory.ServerTool {
+func (d csvOutputTestDeps) IsFeatureEnabled(_ context.Context, flag string) bool {
+	return flag == FeatureFlagCSVOutput && d.csvOn
+}
+
+func requireToolByName(t *testing.T, tools []inventory.ServerTool, name string) inventory.ServerTool {
 	t.Helper()
 
 	for _, tool := range tools {
-		if tool.FeatureFlagEnable == FeatureFlagCSVOutput {
+		if tool.Tool.Name == name {
 			return tool
 		}
 	}
-	require.Fail(t, "CSV output variant not found")
+	require.Failf(t, "tool not found", "tool %q not found", name)
 	return inventory.ServerTool{}
 }
 

pkg/github/feature_flags.go

@@ -38,8 +38,18 @@ type FeatureFlags struct {
 }
 
 // ResolveFeatureFlags computes the effective set of enabled feature flags by:
-//  1. Taking explicitly enabled features validated against AllowedFeatureFlags
-//  2. Adding features enabled by insiders mode from InsidersFeatureFlags
+//  1. Taking the user-supplied flags (from --features or X-MCP-Features) and
+//     keeping only those present in AllowedFeatureFlags. Unknown or unsafe
+//     flags from request input are silently dropped here.
+//  2. If insiders mode is on, unioning in every flag from InsidersFeatureFlags.
+//     Insiders is a server-controlled meta switch, so its expansion is NOT
+//     re-validated against AllowedFeatureFlags.
+//
+// AllowedFeatureFlags and InsidersFeatureFlags are independent sets:
+//   - A flag in AllowedFeatureFlags but not InsidersFeatureFlags is a regular
+//     opt-in flag that insiders mode does not turn on automatically.
+//   - A flag in InsidersFeatureFlags but not AllowedFeatureFlags is reachable
+//     only through insiders mode and cannot be enabled by user input.
 //
 // Returns a set (map) for O(1) lookup by the feature checker.
 func ResolveFeatureFlags(enabledFeatures []string, insidersMode bool) map[string]bool {

pkg/github/feature_flags_test.go

@@ -184,6 +184,18 @@ func TestResolveFeatureFlags(t *testing.T) {
 			expectedFlags:   []string{MCPAppsFeatureFlag},
 			unexpectedFlags: []string{"unknown_flag"},
 		},
+		{
+			name:            "user-only flags can be enabled but are not turned on by insiders",
+			enabledFeatures: []string{FeatureFlagIssuesGranular},
+			insidersMode:    false,
+			expectedFlags:   []string{FeatureFlagIssuesGranular},
+		},
+		{
+			name:            "insiders does not enable user-only allowed flags",
+			enabledFeatures: nil,
+			insidersMode:    true,
+			unexpectedFlags: []string{FeatureFlagIssuesGranular, FeatureFlagPullRequestsGranular},
+		},
 		{
 			name:            "explicit plus insiders deduplicates",
 			enabledFeatures: []string{MCPAppsFeatureFlag},

pkg/github/tools.go

@@ -167,7 +167,7 @@ var (
 // AllTools returns all tools with their embedded toolset metadata.
 // Tool functions return ServerTool directly with toolset info.
 func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool {
-	return withCSVOutputVariants([]inventory.ServerTool{
+	return withCSVOutput([]inventory.ServerTool{
 		// Context tools
 		GetMe(t),
 		GetTeams(t),