diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index ec948ab6e0..ab8b27bb3c 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -126,6 +126,13 @@ var ( } } + var enabledFeatures []string + if viper.IsSet("features") { + if err := viper.UnmarshalKey("features", &enabledFeatures); err != nil { + return fmt.Errorf("failed to unmarshal features: %w", err) + } + } + ttl := viper.GetDuration("repo-access-cache-ttl") httpConfig := ghhttp.ServerConfig{ Version: version, @@ -144,6 +151,7 @@ var ( EnabledToolsets: enabledToolsets, EnabledTools: enabledTools, ExcludeTools: excludeTools, + EnabledFeatures: enabledFeatures, InsidersMode: viper.GetBool("insiders"), } diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 911257ae4f..59147c4f58 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -42,3 +42,28 @@ MCP Apps requires a host that supports the [MCP Apps extension](https://modelcon - **VS Code Insiders** — enable via the `chat.mcp.apps.enabled` setting - **Visual Studio Code** — enable via the `chat.mcp.apps.enabled` setting + +--- + +## CSV output for list tools + +CSV output mode returns supported list tool responses as CSV instead of JSON. This is intended to reduce response context for agents when scanning or summarising lists of GitHub data. + +CSV output applies only to tools in default toolsets whose names start with `list_`, such as `list_issues`, `list_pull_requests`, `list_commits`, and `list_branches`. It does not add new tools or expose a tool argument for selecting the format; the server controls the response format through the Insiders feature flag. + +### Format + +- Nested objects are flattened into dot-notation columns, for example `user.login`, `category.name`, or `head.ref`. +- Arrays are represented as compact single-cell values joined with `;`. +- `body` fields are whitespace-normalized so multiline Markdown does not expand a list response into many output lines. +- Response metadata present in wrapped responses, such as `pageInfo.*` and `totalCount`, is emitted as `#`-prefixed lines before the CSV rows, followed by a blank line. Tools that return a root JSON array do not include metadata preamble lines. + +### Enabling CSV output + +CSV output is enabled by Insiders Mode. For local development, it can also be enabled explicitly with the `csv_output` feature flag: + +```bash +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. diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 3ca249dd17..38106b6d9a 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -143,7 +143,6 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se cfg.Translator, github.FeatureFlags{ LockdownMode: cfg.LockdownMode, - InsidersMode: cfg.InsidersMode, }, cfg.ContentWindowSize, featureChecker, @@ -229,7 +228,7 @@ type StdioServerConfig struct { // LockdownMode indicates if we should enable lockdown mode LockdownMode bool - // InsidersMode indicates if we should enable experimental features + // InsidersMode expands to the curated set of feature flags enabled for insiders. InsidersMode bool // ExcludeTools is a list of tool names to disable regardless of other settings. @@ -345,7 +344,7 @@ func RunStdioServer(cfg StdioServerConfig) error { // 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. +// features are resolved once at startup from --features CLI flag and insiders mode. func createFeatureChecker(enabledFeatures []string, insidersMode bool) inventory.FeatureFlagChecker { featureSet := github.ResolveFeatureFlags(enabledFeatures, insidersMode) return func(_ context.Context, flagName string) (bool, error) { diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index 191e562793..4008c2f4aa 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -106,7 +106,7 @@ func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool { } result := MarshalledTextResult(minimalUser) - if deps.GetFlags(ctx).InsidersMode { + if deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { if result.Meta == nil { result.Meta = mcp.Meta{} } diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index 2b17be86d1..ade54aba17 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -139,7 +139,7 @@ func Test_GetMe(t *testing.T) { } } -func Test_GetMe_IFC_InsidersMode(t *testing.T) { +func Test_GetMe_IFC_FeatureFlag(t *testing.T) { t.Parallel() serverTool := GetMe(translations.NullTranslationHelper) @@ -153,11 +153,21 @@ func Test_GetMe_IFC_InsidersMode(t *testing.T) { GetUser: mockResponse(t, http.StatusOK, mockUser), }) - t.Run("insiders mode disabled omits ifc label from result meta", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, mockedHTTPClient), - Flags: FeatureFlags{InsidersMode: false}, - } + depsWithIFCFeature := func(enabled bool) *BaseDeps { + return NewBaseDeps( + mustNewGHClient(t, mockedHTTPClient), nil, nil, nil, + translations.NullTranslationHelper, + FeatureFlags{}, + 0, + func(_ context.Context, flagName string) (bool, error) { + return flagName == FeatureFlagIFCLabels && enabled, nil + }, + stubExporters(), + ) + } + + t.Run("feature disabled omits ifc label from result meta", func(t *testing.T) { + deps := depsWithIFCFeature(false) handler := serverTool.Handler(deps) request := createMCPRequest(map[string]any{}) @@ -165,14 +175,11 @@ func Test_GetMe_IFC_InsidersMode(t *testing.T) { require.NoError(t, err) require.False(t, result.IsError) - assert.Nil(t, result.Meta, "result meta should be nil when insiders mode is disabled") + assert.Nil(t, result.Meta, "result meta should be nil when IFC labels are disabled") }) - t.Run("insiders mode enabled includes ifc label in result meta", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, mockedHTTPClient), - Flags: FeatureFlags{InsidersMode: true}, - } + t.Run("feature enabled includes ifc label in result meta", func(t *testing.T) { + deps := depsWithIFCFeature(true) handler := serverTool.Handler(deps) request := createMCPRequest(map[string]any{}) @@ -180,7 +187,7 @@ func Test_GetMe_IFC_InsidersMode(t *testing.T) { require.NoError(t, err) require.False(t, result.IsError) - require.NotNil(t, result.Meta, "result meta should be set when insiders mode is enabled") + require.NotNil(t, result.Meta, "result meta should be set when IFC labels are enabled") ifcLabel, ok := result.Meta["ifc"] require.True(t, ok, "result meta should contain ifc key") diff --git a/pkg/github/csv_output.go b/pkg/github/csv_output.go new file mode 100644 index 0000000000..5c06a1c8c7 --- /dev/null +++ b/pkg/github/csv_output.go @@ -0,0 +1,412 @@ +package github + +import ( + "bytes" + "context" + "encoding/csv" + "encoding/json" + "fmt" + "sort" + "strings" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// Ordered by preference when a response wrapper contains multiple arrays. +var primaryCSVRowKeys = []string{ + "items", + "issues", + "discussions", + "categories", + "labels", + "alerts", + "advisories", + "notifications", + "gists", + "repositories", + "commits", + "branches", + "tags", + "releases", + "users", + "teams", + "members", + "projects", + "nodes", +} + +type csvOutputDocument struct { + metadata map[string]string + 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) + continue + } + + jsonOnly := tool + jsonOnly.FeatureFlagDisable = FeatureFlagCSVOutput + result = append(result, jsonOnly) + + csvCapable := tool + csvCapable.FeatureFlagEnable = FeatureFlagCSVOutput + csvCapable.HandlerFunc = wrapHandlerWithCSVOutput(tool.HandlerFunc) + result = append(result, csvCapable) + } + return result +} + +func isCSVOutputTool(tool inventory.ServerTool) bool { + if !tool.Toolset.Default { + return false + } + if !strings.HasPrefix(tool.Tool.Name, "list_") { + return false + } + return tool.FeatureFlagEnable == "" && tool.FeatureFlagDisable == "" +} + +func wrapHandlerWithCSVOutput(next inventory.HandlerFunc) inventory.HandlerFunc { + return func(deps any) mcp.ToolHandler { + handler := next(deps) + 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 + } + + return convertJSONTextResultToCSV(result), nil + } + } +} + +func convertJSONTextResultToCSV(result *mcp.CallToolResult) *mcp.CallToolResult { + if len(result.Content) != 1 { + return utils.NewToolResultError("failed to convert response to CSV: expected a single text content response") + } + + text, ok := result.Content[0].(*mcp.TextContent) + if !ok { + return utils.NewToolResultError("failed to convert response to CSV: expected a text content response") + } + + csvText, err := jsonTextToCSV(text.Text) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to convert response to CSV", err) + } + + result.Content = []mcp.Content{&mcp.TextContent{Text: csvText}} + result.StructuredContent = nil + return result +} + +func jsonTextToCSV(text string) (string, error) { + decoder := json.NewDecoder(strings.NewReader(text)) + decoder.UseNumber() + + var value any + if err := decoder.Decode(&value); err != nil { + return "", fmt.Errorf("failed to unmarshal JSON text: %w", err) + } + + doc := csvDocument(value) + if len(doc.metadata) == 0 && len(doc.rows) == 0 { + return "", nil + } + + var buf bytes.Buffer + writeCSVMetadata(&buf, doc.metadata) + if len(doc.rows) == 0 { + return buf.String(), nil + } + + headers := csvHeaders(doc.rows) + if len(headers) == 0 { + return buf.String(), nil + } + + writer := csv.NewWriter(&buf) + if err := writer.Write(headers); err != nil { + return "", fmt.Errorf("failed to write CSV header: %w", err) + } + + for _, row := range doc.rows { + record := make([]string, len(headers)) + for i, header := range headers { + record[i] = row[header] + } + if err := writer.Write(record); err != nil { + return "", fmt.Errorf("failed to write CSV row: %w", err) + } + } + + writer.Flush() + if err := writer.Error(); err != nil { + return "", fmt.Errorf("failed to flush CSV: %w", err) + } + return buf.String(), nil +} + +func csvDocument(value any) csvOutputDocument { + switch v := value.(type) { + case []any: + return csvOutputDocument{rows: csvRowsFromArray(v)} + case map[string]any: + if rows, metadata, ok := primaryRowsFromMap(v); ok { + return csvOutputDocument{ + metadata: newFlattenedCSVRow(metadata), + rows: csvRowsFromArray(rows), + } + } + return csvOutputDocument{rows: []map[string]string{newFlattenedCSVRow(v)}} + default: + return csvOutputDocument{rows: []map[string]string{scalarCSVRow(v)}} + } +} + +func primaryRowsFromMap(value map[string]any) ([]any, map[string]any, bool) { + if rows, path, ok := primaryRowsAtCurrentLevel(value); ok { + return rows, metadataWithoutPath(value, path), true + } + if rows, path, ok := primaryRowsOneLevelDown(value); ok { + return rows, metadataWithoutPath(value, path), true + } + return nil, nil, false +} + +func primaryRowsAtCurrentLevel(value map[string]any) ([]any, []string, bool) { + if key, ok := preferredPrimaryRowKey(value); ok { + rows, _ := value[key].([]any) + return rows, []string{key}, true + } + if key, ok := singleArrayKey(value); ok { + rows, _ := value[key].([]any) + return rows, []string{key}, true + } + return nil, nil, false +} + +func primaryRowsOneLevelDown(value map[string]any) ([]any, []string, bool) { + var matchedRows []any + var matchedPath []string + for key, raw := range value { + child, ok := raw.(map[string]any) + if !ok { + continue + } + rows, path, ok := primaryRowsAtCurrentLevel(child) + if !ok { + continue + } + if matchedPath != nil { + return nil, nil, false + } + matchedRows = rows + matchedPath = append([]string{key}, path...) + } + if matchedPath == nil { + return nil, nil, false + } + return matchedRows, matchedPath, true +} + +func metadataWithoutPath(value map[string]any, path []string) map[string]any { + metadata := make(map[string]any, len(value)) + for key, raw := range value { + if key != path[0] { + metadata[key] = raw + continue + } + + if len(path) == 1 { + continue + } + child, ok := raw.(map[string]any) + if !ok { + continue + } + childMetadata := metadataWithoutPath(child, path[1:]) + if len(childMetadata) > 0 { + metadata[key] = childMetadata + } + } + return metadata +} + +func csvRowsFromArray(values []any) []map[string]string { + if len(values) == 0 { + return nil + } + + rows := make([]map[string]string, 0, len(values)) + for _, value := range values { + var row map[string]string + switch v := value.(type) { + case map[string]any: + row = make(map[string]string) + appendFlattenedCSVFields(row, v, "") + default: + row = scalarCSVRow(v) + } + rows = append(rows, row) + } + return rows +} + +func writeCSVMetadata(buf *bytes.Buffer, metadata map[string]string) { + if len(metadata) == 0 { + return + } + + headers := make([]string, 0, len(metadata)) + for header := range metadata { + headers = append(headers, header) + } + sort.Strings(headers) + + for _, header := range headers { + fmt.Fprintf(buf, "# %s: %s\n", header, normalizeCSVWhitespace(metadata[header])) + } + buf.WriteByte('\n') +} + +func newFlattenedCSVRow(value map[string]any) map[string]string { + row := make(map[string]string) + appendFlattenedCSVFields(row, value, "") + return row +} + +func appendFlattenedCSVFields(row map[string]string, value map[string]any, prefix string) { + if value == nil { + return + } + + for key, raw := range value { + column := csvColumnName(prefix, key) + switch v := raw.(type) { + case map[string]any: + appendFlattenedCSVFields(row, v, column) + case []any: + row[column] = csvArrayValue(v) + default: + row[column] = csvColumnValue(column, v) + } + } +} + +func csvHeaders(rows []map[string]string) []string { + headerSet := make(map[string]struct{}) + for _, row := range rows { + for header := range row { + headerSet[header] = struct{}{} + } + } + + headers := make([]string, 0, len(headerSet)) + for header := range headerSet { + headers = append(headers, header) + } + sort.Strings(headers) + return headers +} + +func csvColumnName(prefix, key string) string { + if prefix == "" { + return key + } + return prefix + "." + key +} + +func preferredPrimaryRowKey(value map[string]any) (string, bool) { + for _, key := range primaryCSVRowKeys { + if _, ok := value[key].([]any); ok { + return key, true + } + } + return "", false +} + +func singleArrayKey(value map[string]any) (string, bool) { + var arrayKey string + for key, raw := range value { + if _, ok := raw.([]any); !ok { + continue + } + if arrayKey != "" { + return "", false + } + arrayKey = key + } + if arrayKey == "" { + return "", false + } + return arrayKey, true +} + +func csvColumnValue(column string, value any) string { + str := scalarCSVValue(value) + if isBodyColumn(column) { + return normalizeCSVWhitespace(str) + } + return str +} + +func csvArrayValue(values []any) string { + if len(values) == 0 { + return "" + } + + // Scalar arrays use semicolons for compactness. This is lossy if an + // element contains a semicolon; use JSON mode when exact reconstruction matters. + parts := make([]string, 0, len(values)) + for _, value := range values { + switch value.(type) { + case map[string]any, []any: + encoded, err := json.Marshal(value) + if err != nil { + parts = append(parts, scalarCSVValue(value)) + } else { + parts = append(parts, string(encoded)) + } + default: + parts = append(parts, scalarCSVValue(value)) + } + } + return strings.Join(parts, ";") +} + +func scalarCSVRow(value any) map[string]string { + return map[string]string{"value": scalarCSVValue(value)} +} + +func scalarCSVValue(value any) string { + switch v := value.(type) { + case nil: + return "" + case string: + return v + case json.Number: + return v.String() + case bool: + if v { + return "true" + } + return "false" + default: + return fmt.Sprint(v) + } +} + +func isBodyColumn(column string) bool { + return column == "body" || strings.HasSuffix(column, ".body") +} + +func normalizeCSVWhitespace(value string) string { + return strings.Join(strings.Fields(value), " ") +} diff --git a/pkg/github/csv_output_test.go b/pkg/github/csv_output_test.go new file mode 100644 index 0000000000..80ff8a7892 --- /dev/null +++ b/pkg/github/csv_output_test.go @@ -0,0 +1,415 @@ +package github + +import ( + "context" + "encoding/csv" + "encoding/json" + "strings" + "testing" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCSVOutputVariantsAreFeatureGated(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) + + inv := buildCSVOutputInventory(t, tools, false) + 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) + + 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) +} + +func TestCSVOutputVariantsOnlyApplyToDefaultToolsets(t *testing.T) { + nonDefaultListTool := testCSVOutputToolWithToolset("list_discussions", `[{"number":1}]`, ToolsetMetadataDiscussions) + + tools := withCSVOutputVariants([]inventory.ServerTool{nonDefaultListTool}) + require.Len(t, tools, 1) + + assert.Empty(t, tools[0].FeatureFlagEnable) + assert.Empty(t, tools[0].FeatureFlagDisable) +} + +func TestCSVOutputVariantDoesNotExposeFormatParameter(t *testing.T) { + tools := withCSVOutputVariants([]inventory.ServerTool{testCSVOutputTool("list_things", `[{"number":1}]`)}) + csvCapable := requireCSVOutputVariant(t, tools) + + schema, ok := csvCapable.Tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok) + assert.NotContains(t, schema.Properties, "output_format") +} + +func TestCSVOutputVariantConvertsJSONTextToCSV(t *testing.T) { + tools := withCSVOutputVariants([]inventory.ServerTool{ + testCSVOutputTool("list_things", `[ + { + "number": 1, + "body": "first line\n\tsecond line", + "labels": ["bug", "help wanted"], + "user": {"login": "octocat"} + } + ]`), + }) + inv := buildCSVOutputInventory(t, tools, true) + csvCapable := requireToolByName(t, inv.AvailableTools(context.Background()), "list_things") + + result, err := csvCapable.Handler(nil)(context.Background(), testCSVOutputRequest()) + require.NoError(t, err) + require.NotNil(t, result) + require.False(t, result.IsError) + + assert.NotContains(t, textResult(t, result), "#") + + records := readCSVResult(t, result) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "first line second line", row["body"]) + assert.Equal(t, "bug;help wanted", row["labels"]) + assert.Equal(t, "1", row["number"]) + assert.Equal(t, "octocat", row["user.login"]) +} + +func TestCSVOutputVariantMovesMetadataToPreamble(t *testing.T) { + csvText, err := jsonTextToCSV(`{ + "issues": [ + {"number": 1, "title": "First issue"} + ], + "pageInfo": { + "endCursor": "cursor-1", + "hasNextPage": true + }, + "totalCount": 2 + }`) + require.NoError(t, err) + assert.Contains(t, csvText, "# pageInfo.endCursor: cursor-1\n") + assert.Contains(t, csvText, "# pageInfo.hasNextPage: true\n") + assert.Contains(t, csvText, "# totalCount: 2\n\n") + + records := readCSVText(t, csvText) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "1", row["number"]) + assert.Equal(t, "First issue", row["title"]) + assert.NotContains(t, row, "pageInfo.endCursor") + 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": [ + { + "number": 5, + "title": "Discussion tools testing", + "category": {"name": "Q&A"}, + "user": {"login": "octocat"} + } + ] + }`) + require.NoError(t, err) + + records := readCSVText(t, csvText) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "Q&A", row["category.name"]) + assert.Equal(t, "5", row["number"]) + assert.Equal(t, "Discussion tools testing", row["title"]) + assert.Equal(t, "octocat", row["user.login"]) +} + +func TestJSONTextToCSVFindsPrimaryRowsOneLevelDeeper(t *testing.T) { + csvText, err := jsonTextToCSV(`{ + "issues": { + "nodes": [ + {"number": 5, "title": "Nested issue"} + ], + "pageInfo": {"hasNextPage": false}, + "totalCount": 1 + } + }`) + require.NoError(t, err) + + assert.Contains(t, csvText, "# issues.pageInfo.hasNextPage: false\n") + assert.Contains(t, csvText, "# issues.totalCount: 1\n\n") + + records := readCSVText(t, csvText) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "5", row["number"]) + assert.Equal(t, "Nested issue", row["title"]) +} + +func TestJSONTextToCSVUsesSingleArrayAsPrimaryRows(t *testing.T) { + csvText, err := jsonTextToCSV(`{ + "results": [ + {"number": 1, "title": "Single array result"} + ], + "pageInfo": {"hasNextPage": true} + }`) + require.NoError(t, err) + + assert.Contains(t, csvText, "# pageInfo.hasNextPage: true\n\n") + + records := readCSVText(t, csvText) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "1", row["number"]) + assert.Equal(t, "Single array result", row["title"]) + assert.NotContains(t, row, "pageInfo.hasNextPage") +} + +func TestJSONTextToCSVFlattensRootObjectWithoutPrimaryRows(t *testing.T) { + csvText, err := jsonTextToCSV(`{ + "name": "summary", + "pageInfo": {"hasNextPage": false}, + "totalCount": 2 + }`) + require.NoError(t, err) + assert.NotContains(t, csvText, "#") + + records := readCSVText(t, csvText) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "summary", row["name"]) + assert.Equal(t, "false", row["pageInfo.hasNextPage"]) + assert.Equal(t, "2", row["totalCount"]) +} + +func TestJSONTextToCSVConvertsScalarToValueRow(t *testing.T) { + csvText, err := jsonTextToCSV(`"plain value"`) + require.NoError(t, err) + + records := readCSVText(t, csvText) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "plain value", row["value"]) +} + +func TestJSONTextToCSVReturnsEmptyForEmptyArray(t *testing.T) { + csvText, err := jsonTextToCSV(`[]`) + require.NoError(t, err) + assert.Empty(t, csvText) +} + +func TestJSONTextToCSVReturnsEmptyForEmptyObject(t *testing.T) { + csvText, err := jsonTextToCSV(`{}`) + require.NoError(t, err) + assert.Empty(t, csvText) +} + +func TestJSONTextToCSVReturnsEmptyForOnlyEmptyNestedObjects(t *testing.T) { + csvText, err := jsonTextToCSV(`{ + "repository": { + "owner": {} + } + }`) + require.NoError(t, err) + assert.Empty(t, csvText) +} + +func TestJSONTextToCSVReturnsMetadataOnlyWhenRowsHaveNoColumns(t *testing.T) { + csvText, err := jsonTextToCSV(`{ + "items": [ + {} + ], + "totalCount": 1 + }`) + require.NoError(t, err) + assert.Equal(t, "# totalCount: 1\n\n", csvText) +} + +func TestJSONTextToCSVFlattensAmbiguousArraysAsSingleRow(t *testing.T) { + csvText, err := jsonTextToCSV(`{ + "foo": ["a", "b"], + "bar": ["c"] + }`) + require.NoError(t, err) + assert.NotContains(t, csvText, "#") + + records := readCSVText(t, csvText) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "c", row["bar"]) + assert.Equal(t, "a;b", row["foo"]) +} + +func TestJSONTextToCSVUsesPreferredArrayWhenMultipleArraysExist(t *testing.T) { + csvText, err := jsonTextToCSV(`{ + "items": [ + {"id": 1} + ], + "other": [ + {"id": 2} + ], + "totalCount": 1 + }`) + require.NoError(t, err) + + assert.Contains(t, csvText, "# other: {\"id\":2}\n") + assert.Contains(t, csvText, "# totalCount: 1\n\n") + + records := readCSVText(t, csvText) + require.Len(t, records, 2) + + row := csvRow(t, records[0], records[1]) + assert.Equal(t, "1", row["id"]) +} + +func testCSVOutputTool(name string, response string) inventory.ServerTool { + return testCSVOutputToolWithToolset(name, response, ToolsetMetadataRepos) +} + +func testCSVOutputToolWithToolset(name string, response string, toolset inventory.ToolsetMetadata) inventory.ServerTool { + return inventory.ServerTool{ + Tool: mcp.Tool{ + Name: name, + Annotations: &mcp.ToolAnnotations{ + ReadOnlyHint: true, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{}, + }, + }, + Toolset: toolset, + HandlerFunc: func(_ any) mcp.ToolHandler { + return func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: response}, + }, + }, nil + } + }, + } +} + +func buildCSVOutputInventory(t *testing.T, tools []inventory.ServerTool, csvOutputEnabled 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() + + 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{} +} + +func requireCSVOutputVariant(t *testing.T, tools []inventory.ServerTool) inventory.ServerTool { + t.Helper() + + for _, tool := range tools { + if tool.FeatureFlagEnable == FeatureFlagCSVOutput { + return tool + } + } + require.Fail(t, "CSV output variant not found") + return inventory.ServerTool{} +} + +func testCSVOutputRequest() *mcp.CallToolRequest { + return &mcp.CallToolRequest{ + Params: &mcp.CallToolParamsRaw{ + Arguments: json.RawMessage(`{}`), + }, + } +} + +func readCSVResult(t *testing.T, result *mcp.CallToolResult) [][]string { + t.Helper() + + require.Len(t, result.Content, 1) + text, ok := result.Content[0].(*mcp.TextContent) + require.True(t, ok) + + return readCSVText(t, text.Text) +} + +func textResult(t *testing.T, result *mcp.CallToolResult) string { + t.Helper() + + require.Len(t, result.Content, 1) + text, ok := result.Content[0].(*mcp.TextContent) + require.True(t, ok) + return text.Text +} + +func readCSVText(t *testing.T, text string) [][]string { + t.Helper() + + reader := csv.NewReader(strings.NewReader(text)) + reader.Comment = '#' + records, err := reader.ReadAll() + require.NoError(t, err) + return records +} + +func csvRow(t *testing.T, headers []string, record []string) map[string]string { + t.Helper() + require.Len(t, record, len(headers)) + + row := make(map[string]string, len(headers)) + for i, header := range headers { + row[header] = record[i] + } + return row +} diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index eb856e0bd6..e3a031f999 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -410,7 +410,6 @@ func (d *RequestDeps) GetT() translations.TranslationHelperFunc { return d.T } func (d *RequestDeps) GetFlags(ctx context.Context) FeatureFlags { return FeatureFlags{ LockdownMode: d.lockdownMode && ghcontext.IsLockdownMode(ctx), - InsidersMode: ghcontext.IsInsidersMode(ctx), } } diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 3f3d7bf976..9adfa38d2a 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -1,14 +1,23 @@ package github +import "slices" + // MCPAppsFeatureFlag is the feature flag name for MCP Apps (interactive UI forms). const MCPAppsFeatureFlag = "remote_mcp_ui_apps" +// FeatureFlagCSVOutput is the feature flag name for CSV output on list tools. +const FeatureFlagCSVOutput = "csv_output" + +// FeatureFlagIFCLabels is the feature flag name for IFC security labels in tool results. +const FeatureFlagIFCLabels = "ifc_labels" + // 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, + FeatureFlagCSVOutput, FeatureFlagIssuesGranular, FeatureFlagPullRequestsGranular, } @@ -19,37 +28,30 @@ var AllowedFeatureFlags = []string{ // feature flag expansion. var InsidersFeatureFlags = []string{ MCPAppsFeatureFlag, + FeatureFlagCSVOutput, + FeatureFlagIFCLabels, } // FeatureFlags defines runtime feature toggles that adjust tool behavior. 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 +// 1. Taking explicitly enabled features validated against AllowedFeatureFlags +// 2. Adding features enabled by insiders mode from InsidersFeatureFlags // // 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] { + if slices.Contains(AllowedFeatureFlags, f) { effective[f] = true } } if insidersMode { for _, f := range InsidersFeatureFlags { - if allowed[f] { - effective[f] = true - } + effective[f] = true } } return effective diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index b0c1a4305c..9bc1be473f 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -18,10 +18,14 @@ import ( // RemoteMCPEnthusiasticGreeting is a dummy test feature flag . const RemoteMCPEnthusiasticGreeting = "remote_mcp_enthusiastic_greeting" -// FeatureChecker is an interface for checking if a feature flag is enabled. -type FeatureChecker interface { - // IsFeatureEnabled checks if a feature flag is enabled. - IsFeatureEnabled(ctx context.Context, flagName string) bool +func featureCheckerFor(enabledFlags ...string) func(context.Context, string) (bool, error) { + enabled := make(map[string]bool, len(enabledFlags)) + for _, flag := range enabledFlags { + enabled[flag] = true + } + return func(_ context.Context, flagName string) (bool, error) { + return enabled[flagName], nil + } } // HelloWorld returns a simple greeting tool that demonstrates feature flag conditional behavior. @@ -45,9 +49,6 @@ func HelloWorldTool(t translations.TranslationHelperFunc) inventory.ServerTool { if deps.IsFeatureEnabled(ctx, RemoteMCPEnthusiasticGreeting) { greeting += " Welcome to the future of MCP! 🎉" } - if deps.GetFlags(ctx).InsidersMode { - greeting += " Experimental features are enabled! 🚀" - } // Build response response := map[string]any{ @@ -89,12 +90,9 @@ func TestHelloWorld_ConditionalBehavior_Featureflag(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - // Create feature checker based on test case - checker := func(_ context.Context, flagName string) (bool, error) { - if flagName == RemoteMCPEnthusiasticGreeting { - return tt.featureFlagEnabled, nil - } - return false, nil + var enabledFlags []string + if tt.featureFlagEnabled { + enabledFlags = append(enabledFlags, RemoteMCPEnthusiasticGreeting) } // Create deps with the checker @@ -103,7 +101,7 @@ func TestHelloWorld_ConditionalBehavior_Featureflag(t *testing.T) { translations.NullTranslationHelper, FeatureFlags{}, 0, - checker, + featureCheckerFor(enabledFlags...), stubExporters(), ) @@ -149,14 +147,12 @@ func TestResolveFeatureFlags(t *testing.T) { { name: "no features, no insiders", enabledFeatures: nil, - insidersMode: false, expectedFlags: nil, - unexpectedFlags: []string{MCPAppsFeatureFlag}, + unexpectedFlags: []string{MCPAppsFeatureFlag, FeatureFlagIFCLabels}, }, { name: "explicit feature enabled", enabledFeatures: []string{MCPAppsFeatureFlag}, - insidersMode: false, expectedFlags: []string{MCPAppsFeatureFlag}, }, { @@ -165,16 +161,26 @@ func TestResolveFeatureFlags(t *testing.T) { insidersMode: true, expectedFlags: InsidersFeatureFlags, }, + { + name: "insiders mode enables internal-only flags", + enabledFeatures: nil, + insidersMode: true, + expectedFlags: []string{FeatureFlagIFCLabels}, + }, + { + name: "internal-only flags are not directly enabled", + enabledFeatures: []string{FeatureFlagIFCLabels}, + expectedFlags: nil, + unexpectedFlags: []string{FeatureFlagIFCLabels}, + }, { 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"}, }, @@ -182,7 +188,7 @@ func TestResolveFeatureFlags(t *testing.T) { name: "explicit plus insiders deduplicates", enabledFeatures: []string{MCPAppsFeatureFlag}, insidersMode: true, - expectedFlags: []string{MCPAppsFeatureFlag}, + expectedFlags: InsidersFeatureFlags, }, } @@ -199,66 +205,3 @@ func TestResolveFeatureFlags(t *testing.T) { }) } } - -func TestHelloWorld_ConditionalBehavior_Config(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - insidersMode bool - expectedGreeting string - }{ - { - name: "Experimental disabled - default greeting", - insidersMode: false, - expectedGreeting: "Hello, world!", - }, - { - name: "Experimental enabled - experimental greeting", - insidersMode: true, - expectedGreeting: "Hello, world! Experimental features are enabled! 🚀", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - // Create deps with the checker - deps := NewBaseDeps( - nil, nil, nil, nil, - translations.NullTranslationHelper, - FeatureFlags{InsidersMode: tt.insidersMode}, - 0, - nil, - stubExporters(), - ) - - // Get the tool and its handler - tool := HelloWorldTool(translations.NullTranslationHelper) - handler := tool.Handler(deps) - - // Call the handler with deps in context - ctx := ContextWithDeps(context.Background(), deps) - result, err := handler(ctx, &mcp.CallToolRequest{ - Params: &mcp.CallToolParamsRaw{ - Arguments: json.RawMessage(`{}`), - }, - }) - require.NoError(t, err) - require.NotNil(t, result) - require.Len(t, result.Content, 1) - - // Parse the response - should be TextContent - textContent, ok := result.Content[0].(*mcp.TextContent) - require.True(t, ok, "expected content to be TextContent") - - var response map[string]any - err = json.Unmarshal([]byte(textContent.Text), &response) - require.NoError(t, err) - - // Verify the greeting matches expected based on feature flag - assert.Equal(t, tt.expectedGreeting, response["greeting"]) - }) - } -} diff --git a/pkg/github/issues.go b/pkg/github/issues.go index fe1e7b5011..6389947df4 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -348,10 +348,10 @@ Options are: } // attachIFC adds the IFC label to a successful tool result when - // InsidersMode is enabled. If the visibility lookup fails the + // IFC labels are enabled. If the visibility lookup fails the // label is omitted rather than misclassifying the result. attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult { - if r == nil || r.IsError || !deps.GetFlags(ctx).InsidersMode { + if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { return r } isPrivate, err := FetchRepoIsPrivate(ctx, client, owner, repo) @@ -1044,7 +1044,7 @@ func SearchIssues(t translations.TranslationHelperFunc) inventory.ServerTool { []scopes.Scope{scopes.Repo}, func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { var options []searchOption - if deps.GetFlags(ctx).InsidersMode { + if deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { options = append(options, withSearchPostProcess(searchIssuesIFCPostProcess(deps))) } result, err := searchIssuesHandler(ctx, deps, args, options...) @@ -1400,12 +1400,12 @@ Options are: return utils.NewToolResultError(err.Error()), nil, nil } - // When insiders mode is enabled and the client supports MCP Apps UI, + // When MCP Apps are enabled and the client supports UI, // check if this is a UI form submission. The UI sends _ui_submitted=true // to distinguish form submissions from LLM calls. uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted") - if deps.GetFlags(ctx).InsidersMode && clientSupportsUI(ctx, req) && !uiSubmitted { + if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted { if method == "update" { // Skip the UI form when a state change is requested because // the form only handles title/body editing and would lose the @@ -1900,7 +1900,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } result := MarshalledTextResult(resp) - if deps.GetFlags(ctx).InsidersMode { + if deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { if result.Meta == nil { result.Meta = mcp.Meta{} } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index ff4cb93a16..18bab11c1b 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -325,7 +325,6 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode disabled omits ifc label", func(t *testing.T) { deps := BaseDeps{ Client: mustNewGHClient(t, makeMockClient(false, 0)), - Flags: FeatureFlags{InsidersMode: false}, } handler := serverTool.Handler(deps) @@ -339,8 +338,8 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode enabled on public repo emits public untrusted", func(t *testing.T) { deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false, 0)), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, makeMockClient(false, 0)), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -357,8 +356,8 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode enabled on private repo with get_comments emits private untrusted", func(t *testing.T) { deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(true, 0)), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, makeMockClient(true, 0)), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -375,8 +374,8 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) { deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false, http.StatusInternalServerError)), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, makeMockClient(false, http.StatusInternalServerError)), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -910,7 +909,6 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) { searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "public-repo", 1)}} deps := BaseDeps{ Client: mustNewGHClient(t, makeMockClient(searchResult, []repoFixture{{owner: "octocat", repo: "public-repo"}})), - Flags: FeatureFlags{InsidersMode: false}, } handler := serverTool.Handler(deps) @@ -924,8 +922,8 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode all public emits public untrusted", func(t *testing.T) { searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "public-repo", 1)}} deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(searchResult, []repoFixture{{owner: "octocat", repo: "public-repo"}})), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, makeMockClient(searchResult, []repoFixture{{owner: "octocat", repo: "public-repo"}})), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -950,7 +948,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) { {owner: "octocat", repo: "private-repo", isPrivate: true}, {owner: "octocat", repo: "public-repo"}, })), - Flags: FeatureFlags{InsidersMode: true}, + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -971,7 +969,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) { Client: mustNewGHClient(t, makeMockClient(searchResult, []repoFixture{ {owner: "octocat", repo: "broken", repoStatus: http.StatusInternalServerError}, })), - Flags: FeatureFlags{InsidersMode: true}, + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -989,8 +987,8 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode empty results emits public untrusted", func(t *testing.T) { searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{}} deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(searchResult, nil)), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, makeMockClient(searchResult, nil)), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -1266,9 +1264,9 @@ func Test_CreateIssue(t *testing.T) { } } -// Test_IssueWrite_InsidersMode_UIGate verifies the insiders mode UI gate +// Test_IssueWrite_MCPAppsFeature_UIGate verifies the MCP Apps feature UI gate // behavior: UI clients get a form message, non-UI clients execute directly. -func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) { +func Test_IssueWrite_MCPAppsFeature_UIGate(t *testing.T) { t.Parallel() mockIssue := &github.Issue{ @@ -1284,9 +1282,9 @@ func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) { })) deps := BaseDeps{ - Client: client, - GQLClient: githubv4.NewClient(nil), - Flags: FeatureFlags{InsidersMode: true}, + Client: client, + GQLClient: githubv4.NewClient(nil), + featureChecker: featureCheckerFor(MCPAppsFeatureFlag), } handler := serverTool.Handler(deps) @@ -1401,9 +1399,9 @@ func Test_IssueWrite_InsidersMode_UIGate(t *testing.T) { )) closeDeps := BaseDeps{ - Client: closeClient, - GQLClient: closeGQLClient, - Flags: FeatureFlags{InsidersMode: true}, + Client: closeClient, + GQLClient: closeGQLClient, + featureChecker: featureCheckerFor(MCPAppsFeatureFlag), } closeHandler := serverTool.Handler(closeDeps) @@ -1876,7 +1874,6 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) { gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) deps := BaseDeps{ GQLClient: gqlClient, - Flags: FeatureFlags{InsidersMode: false}, } handler := serverTool.Handler(deps) @@ -1892,8 +1889,8 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) { matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(false)) gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) deps := BaseDeps{ - GQLClient: gqlClient, - Flags: FeatureFlags{InsidersMode: true}, + GQLClient: gqlClient, + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -1919,8 +1916,8 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) { matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true)) gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) deps := BaseDeps{ - GQLClient: gqlClient, - Flags: FeatureFlags{InsidersMode: true}, + GQLClient: gqlClient, + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 9672f85244..7e2391455f 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -611,12 +611,12 @@ func CreatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo return utils.NewToolResultError(err.Error()), nil, nil } - // When insiders mode is enabled and the client supports MCP Apps UI, + // When MCP Apps are enabled and the client supports UI, // check if this is a UI form submission. The UI sends _ui_submitted=true // to distinguish form submissions from LLM calls. uiSubmitted, _ := OptionalParam[bool](args, "_ui_submitted") - if deps.GetFlags(ctx).InsidersMode && clientSupportsUI(ctx, req) && !uiSubmitted { + if deps.IsFeatureEnabled(ctx, MCPAppsFeatureFlag) && clientSupportsUI(ctx, req) && !uiSubmitted { return utils.NewToolResultText(fmt.Sprintf("Ready to create a pull request in %s/%s. IMPORTANT: The PR has NOT been created yet. Do NOT tell the user the PR was created. The user MUST click Submit in the form to create it.", owner, repo)), nil, nil } diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index a73ba2e173..097651b66e 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -2393,9 +2393,9 @@ func Test_CreatePullRequest(t *testing.T) { } } -// Test_CreatePullRequest_InsidersMode_UIGate verifies the insiders mode UI gate +// Test_CreatePullRequest_MCPAppsFeature_UIGate verifies the MCP Apps feature UI gate // behavior: UI clients get a form message, non-UI clients execute directly. -func Test_CreatePullRequest_InsidersMode_UIGate(t *testing.T) { +func Test_CreatePullRequest_MCPAppsFeature_UIGate(t *testing.T) { t.Parallel() mockPR := &github.PullRequest{ @@ -2414,9 +2414,9 @@ func Test_CreatePullRequest_InsidersMode_UIGate(t *testing.T) { })) deps := BaseDeps{ - Client: client, - GQLClient: githubv4.NewClient(nil), - Flags: FeatureFlags{InsidersMode: true}, + Client: client, + GQLClient: githubv4.NewClient(nil), + featureChecker: featureCheckerFor(MCPAppsFeatureFlag), } handler := serverTool.Handler(deps) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 2ca1cf3a7a..d682b5c3d7 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -741,7 +741,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool } // attachIFC adds the IFC label to a successful tool result when - // InsidersMode is enabled. The visibility lookup is performed + // IFC labels are enabled. The visibility lookup is performed // lazily on first use and cached because GetFileContents has // many possible return paths and would otherwise re-fetch on // each. If the visibility lookup fails we skip the label rather @@ -752,7 +752,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool ifcIsPrivate bool ) attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult { - if r == nil || r.IsError || !deps.GetFlags(ctx).InsidersMode { + if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { return r } if !ifcLabelKnown { diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index a44bad65b6..03535f1d26 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -521,7 +521,6 @@ func Test_GetFileContents_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode disabled omits ifc label from result meta", func(t *testing.T) { deps := BaseDeps{ Client: mustNewGHClient(t, makeMockClient(false)), - Flags: FeatureFlags{InsidersMode: false}, } handler := serverTool.Handler(deps) @@ -535,8 +534,8 @@ func Test_GetFileContents_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode enabled on public repo emits public untrusted label", func(t *testing.T) { deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false)), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, makeMockClient(false)), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -560,8 +559,8 @@ func Test_GetFileContents_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode enabled on private repo emits private trusted label", func(t *testing.T) { deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(true)), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, makeMockClient(true)), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -604,8 +603,8 @@ func Test_GetFileContents_IFC_InsidersMode(t *testing.T) { }, }) deps := BaseDeps{ - Client: mustNewGHClient(t, mockedClient), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, mockedClient), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) diff --git a/pkg/github/search.go b/pkg/github/search.go index 9d50a63103..9a8d182887 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -163,7 +163,7 @@ func SearchRepositories(t translations.TranslationHelperFunc) inventory.ServerTo } callResult := utils.NewToolResultText(string(r)) - if deps.GetFlags(ctx).InsidersMode { + if deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { attachSearchRepositoriesIFCLabel(result.Repositories, callResult) } return callResult, nil, nil diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index f1acec3e28..fa48bf19a1 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -207,7 +207,6 @@ func Test_SearchRepositories_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode disabled omits ifc label", func(t *testing.T) { deps := BaseDeps{ Client: mustNewGHClient(t, makeMockClient([]repoFixture{{owner: "octocat", name: "public-repo"}})), - Flags: FeatureFlags{InsidersMode: false}, } handler := serverTool.Handler(deps) @@ -224,7 +223,7 @@ func Test_SearchRepositories_IFC_InsidersMode(t *testing.T) { {owner: "octocat", name: "public-a"}, {owner: "octocat", name: "public-b"}, })), - Flags: FeatureFlags{InsidersMode: true}, + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -245,7 +244,7 @@ func Test_SearchRepositories_IFC_InsidersMode(t *testing.T) { {owner: "octocat", name: "private-repo", isPrivate: true}, {owner: "octocat", name: "public-repo"}, })), - Flags: FeatureFlags{InsidersMode: true}, + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) @@ -262,8 +261,8 @@ func Test_SearchRepositories_IFC_InsidersMode(t *testing.T) { t.Run("insiders mode empty results emits public untrusted", func(t *testing.T) { deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(nil)), - Flags: FeatureFlags{InsidersMode: true}, + Client: mustNewGHClient(t, makeMockClient(nil)), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } handler := serverTool.Handler(deps) diff --git a/pkg/github/server.go b/pkg/github/server.go index 41e502db3c..9df7c59b6c 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -50,7 +50,7 @@ type MCPServerConfig struct { // LockdownMode indicates if we should enable lockdown mode LockdownMode bool - // InsidersMode indicates if we should enable experimental features + // InsidersMode expands to the curated set of feature flags enabled for insiders. InsidersMode bool // Logger is used for logging within the server diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index be37ca949d..7f909f431c 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -130,7 +130,6 @@ func mockRESTPermissionServer(t *testing.T, defaultPerm string, overrides map[st func stubFeatureFlags(enabledFlags map[string]bool) FeatureFlags { return FeatureFlags{ LockdownMode: enabledFlags["lockdown-mode"], - InsidersMode: enabledFlags["insiders-mode"], } } @@ -164,7 +163,6 @@ func TestNewMCPServer_CreatesSuccessfully(t *testing.T) { Translator: translations.NullTranslationHelper, ContentWindowSize: 5000, LockdownMode: false, - InsidersMode: false, } deps := stubDeps{obsv: stubExporters()} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 7d22c72fc9..e7530b672d 100644 --- a/pkg/github/tools.go +++ b/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 []inventory.ServerTool{ + return withCSVOutputVariants([]inventory.ServerTool{ // Context tools GetMe(t), GetTeams(t), @@ -313,7 +313,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GranularAddPullRequestReviewComment(t), GranularResolveReviewThread(t), GranularUnresolveReviewThread(t), - } + }) } // ToBoolPtr converts a bool to a *bool pointer. diff --git a/pkg/github/tools_validation_test.go b/pkg/github/tools_validation_test.go index 90e3c744cb..0a4a4eb7b0 100644 --- a/pkg/github/tools_validation_test.go +++ b/pkg/github/tools_validation_test.go @@ -1,6 +1,11 @@ package github import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "strings" "testing" "github.com/github/github-mcp-server/pkg/inventory" @@ -184,3 +189,29 @@ func TestToolsetMetadataConsistency(t *testing.T) { } } } + +func TestGitHubPackageDoesNotReadInsidersMode(t *testing.T) { + files, err := filepath.Glob("*.go") + require.NoError(t, err) + + for _, file := range files { + if strings.HasSuffix(file, "_test.go") { + continue + } + + fset := token.NewFileSet() + node, err := parser.ParseFile(fset, file, nil, 0) + require.NoError(t, err, "failed to parse %s", file) + + ast.Inspect(node, func(n ast.Node) bool { + selector, ok := n.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != "InsidersMode" { + return true + } + + position := fset.Position(selector.Sel.Pos()) + t.Errorf("%s reads InsidersMode directly; gate behavior on concrete feature flags instead", position) + return true + }) + } +} diff --git a/pkg/github/ui_embed.go b/pkg/github/ui_embed.go index 257856e156..c3f1cef9d2 100644 --- a/pkg/github/ui_embed.go +++ b/pkg/github/ui_embed.go @@ -34,7 +34,7 @@ func MustGetUIAsset(name string) string { // UIAssetsAvailable returns true if the MCP App UI assets have been built. // This checks for a known UI asset file to determine if `script/build-ui` has been run. // Use this to gracefully skip UI registration when assets aren't available, -// allowing Insiders mode to work for non-UI features without requiring a UI build. +// allowing non-UI features to work without requiring a UI build. func UIAssetsAvailable() bool { _, err := GetUIAsset("get-me.html") return err == nil diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 90423d93cc..dd6161d52f 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -321,8 +321,7 @@ func hasStaticConfig(cfg *ServerConfig) bool { return cfg.ReadOnly || cfg.EnabledToolsets != nil || cfg.EnabledTools != nil || - len(cfg.ExcludeTools) > 0 || - cfg.InsidersMode + len(cfg.ExcludeTools) > 0 } // buildStaticInventory pre-filters the full tool/resource/prompt universe using @@ -353,8 +352,12 @@ func buildStaticInventory(cfg *ServerConfig, t translations.TranslationHelperFun return github.AllTools(t), github.AllResources(t), github.AllPrompts(t) } + // Static filtering defines an upper bound for all requests. Do not apply + // feature flags here: HTTP feature flags can come from request context + // (/insiders, X-MCP-Features), so variants must be preserved until the + // per-request inventory evaluates them. ctx := context.Background() - return inv.AvailableTools(ctx), inv.AvailableResourceTemplates(ctx), inv.AvailablePrompts(ctx) + return inv.AvailableToolsWithoutFeatureFiltering(ctx), inv.AvailableResourceTemplatesWithoutFeatureFiltering(ctx), inv.AvailablePromptsWithoutFeatureFiltering(ctx) } // InventoryFiltersForRequest applies filters to the inventory builder diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index fd2966fd05..dfb402093b 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -632,6 +632,31 @@ func TestStaticConfigEnforcement(t *testing.T) { } } +func TestStaticInventoryPreservesPerRequestFeatureVariants(t *testing.T) { + tools := []inventory.ServerTool{ + mockToolWithFeatureFlag("list_issues", "issues", true, "", github.FeatureFlagCSVOutput), + mockToolWithFeatureFlag("list_issues", "issues", true, github.FeatureFlagCSVOutput, ""), + } + cfg := &ServerConfig{Version: "test", EnabledToolsets: []string{"issues"}} + featureChecker := createHTTPFeatureChecker(nil, false) + + staticTools, _, _ := buildStaticInventoryFromTools(cfg, tools, featureChecker) + require.Len(t, staticTools, 2, "static upper bounds should preserve both feature variants") + + inv, err := inventory.NewBuilder(). + SetTools(staticTools). + WithFeatureChecker(featureChecker). + WithToolsets([]string{"all"}). + Build() + require.NoError(t, err) + + ctx := ghcontext.WithInsidersMode(context.Background(), true) + available := inv.AvailableTools(ctx) + require.Len(t, available, 1) + assert.Equal(t, "list_issues", available[0].Tool.Name) + assert.Equal(t, github.FeatureFlagCSVOutput, available[0].FeatureFlagEnable) +} + // TestContentTypeHandling verifies that the MCP StreamableHTTP handler // accepts Content-Type values with additional parameters like charset=utf-8. // This is a regression test for https://github.com/github/github-mcp-server/issues/2333 @@ -754,7 +779,7 @@ func buildStaticInventoryFromTools(cfg *ServerConfig, tools []inventory.ServerTo } ctx := context.Background() - return inv.AvailableTools(ctx), inv.AvailableResourceTemplates(ctx), inv.AvailablePrompts(ctx) + return inv.AvailableToolsWithoutFeatureFiltering(ctx), inv.AvailableResourceTemplatesWithoutFeatureFiltering(ctx), inv.AvailablePromptsWithoutFeatureFiltering(ctx) } func TestCrossOriginProtection(t *testing.T) { @@ -847,7 +872,7 @@ func TestInsidersRoutePreservesUIMeta(t *testing.T) { uiTool := mockTool("with_ui", "repos", true) uiTool.Tool.Meta = mcp.Meta{"ui": map[string]any{"resourceUri": uiURI}} - checker := createHTTPFeatureChecker() + checker := createHTTPFeatureChecker(nil, false) build := func() *inventory.Inventory { inv, err := inventory.NewBuilder(). SetTools([]inventory.ServerTool{uiTool}). diff --git a/pkg/http/middleware/cors.go b/pkg/http/middleware/cors.go index 2eaf4227b4..fb8e8e548d 100644 --- a/pkg/http/middleware/cors.go +++ b/pkg/http/middleware/cors.go @@ -17,6 +17,7 @@ func SetCorsHeaders(h http.Handler) http.Handler { "Mcp-Session-Id", "Mcp-Protocol-Version", "Last-Event-ID", + "X-Custom-Auth-Headers", headers.AuthorizationHeader, headers.MCPReadOnlyHeader, headers.MCPToolsetsHeader, diff --git a/pkg/http/server.go b/pkg/http/server.go index b8c419ea04..6fd19a8b9b 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -82,7 +82,10 @@ type ServerConfig struct { // When set via CLI flag, per-request headers cannot re-include these tools. ExcludeTools []string - // InsidersMode indicates if we should enable experimental features. + // EnabledFeatures is a list of feature flags that are enabled. + EnabledFeatures []string + + // InsidersMode expands to the curated set of feature flags enabled for insiders. InsidersMode bool } @@ -121,7 +124,7 @@ func RunHTTPServer(cfg ServerConfig) error { repoAccessOpts = append(repoAccessOpts, lockdown.WithTTL(*cfg.RepoAccessCacheTTL)) } - featureChecker := createHTTPFeatureChecker() + featureChecker := createHTTPFeatureChecker(cfg.EnabledFeatures, cfg.InsidersMode) obs, err := observability.NewExporters(logger, metrics.NewNoopMetrics()) if err != nil { @@ -228,14 +231,16 @@ func initGlobalToolScopeMap(t translations.TranslationHelperFunc) error { return nil } -// 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 { +// createHTTPFeatureChecker creates a feature checker that resolves static CLI +// features plus per-request header features and insiders mode. +func createHTTPFeatureChecker(enabledFeatures []string, insidersMode bool) inventory.FeatureFlagChecker { return func(ctx context.Context, flag string) (bool, error) { headerFeatures := ghcontext.GetHeaderFeatures(ctx) - insidersMode := ghcontext.IsInsidersMode(ctx) - effective := github.ResolveFeatureFlags(headerFeatures, insidersMode) + features := make([]string, 0, len(enabledFeatures)+len(headerFeatures)) + features = append(features, enabledFeatures...) + features = append(features, headerFeatures...) + + effective := github.ResolveFeatureFlags(features, insidersMode || ghcontext.IsInsidersMode(ctx)) return effective[flag], nil } } diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index 23c82d0486..5458a6b395 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -11,10 +11,10 @@ import ( ) func TestCreateHTTPFeatureChecker(t *testing.T) { - checker := createHTTPFeatureChecker() - tests := []struct { name string + staticFeatures []string + staticInsiders bool flagName string headerFeatures []string insidersMode bool @@ -74,6 +74,37 @@ func TestCreateHTTPFeatureChecker(t *testing.T) { insidersMode: true, wantEnabled: true, }, + { + name: "static feature is enabled without header", + staticFeatures: []string{github.FeatureFlagCSVOutput}, + flagName: github.FeatureFlagCSVOutput, + wantEnabled: true, + }, + { + name: "static features combine with header features", + staticFeatures: []string{github.FeatureFlagCSVOutput}, + flagName: github.FeatureFlagIssuesGranular, + headerFeatures: []string{github.FeatureFlagIssuesGranular}, + wantEnabled: true, + }, + { + name: "internal-only flag in header is ignored", + flagName: github.FeatureFlagIFCLabels, + headerFeatures: []string{github.FeatureFlagIFCLabels}, + wantEnabled: false, + }, + { + name: "static insiders enables insiders flags without route context", + staticInsiders: true, + flagName: github.FeatureFlagCSVOutput, + wantEnabled: true, + }, + { + name: "insiders mode enables internal-only insiders flags", + flagName: github.FeatureFlagIFCLabels, + insidersMode: true, + wantEnabled: true, + }, { name: "insiders mode does not enable granular flags", flagName: github.FeatureFlagIssuesGranular, @@ -84,6 +115,7 @@ func TestCreateHTTPFeatureChecker(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + checker := createHTTPFeatureChecker(tt.staticFeatures, tt.staticInsiders) ctx := context.Background() if len(tt.headerFeatures) > 0 { ctx = ghcontext.WithHeaderFeatures(ctx, tt.headerFeatures) diff --git a/pkg/inventory/filters.go b/pkg/inventory/filters.go index 604aa1000d..20a157de63 100644 --- a/pkg/inventory/filters.go +++ b/pkg/inventory/filters.go @@ -58,6 +58,10 @@ func (r *Inventory) isFeatureFlagAllowed(ctx context.Context, enableFlag, disabl // 4. Builder filters (via WithFilter) // 5. Toolset/additional tools func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool) bool { + return r.isToolEnabledWithFeatureFlags(ctx, tool, true) +} + +func (r *Inventory) isToolEnabledWithFeatureFlags(ctx context.Context, tool *ServerTool, checkFeatureFlags bool) bool { // 1. Check tool's own Enabled function first if tool.Enabled != nil { enabled, err := tool.Enabled(ctx) @@ -70,7 +74,7 @@ func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool) bool { } } // 2. Check feature flags - if !r.isFeatureFlagAllowed(ctx, tool.FeatureFlagEnable, tool.FeatureFlagDisable) { + if checkFeatureFlags && !r.isFeatureFlagAllowed(ctx, tool.FeatureFlagEnable, tool.FeatureFlagDisable) { return false } // 3. Check read-only filter (applies to all tools) @@ -99,6 +103,15 @@ func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool) bool { return true } +func sortTools(tools []ServerTool) { + sort.Slice(tools, func(i, j int) bool { + if tools[i].Toolset.ID != tools[j].Toolset.ID { + return tools[i].Toolset.ID < tools[j].Toolset.ID + } + return tools[i].Tool.Name < tools[j].Tool.Name + }) +} + // AvailableTools returns the tools that pass all current filters, // sorted deterministically by toolset ID, then tool name. // The context is used for feature flag evaluation. @@ -112,16 +125,36 @@ func (r *Inventory) AvailableTools(ctx context.Context) []ServerTool { } // Sort deterministically: by toolset ID, then by tool name - sort.Slice(result, func(i, j int) bool { - if result[i].Toolset.ID != result[j].Toolset.ID { - return result[i].Toolset.ID < result[j].Toolset.ID + sortTools(result) + + return result +} + +// AvailableToolsWithoutFeatureFiltering returns tools that pass every filter +// except FeatureFlagEnable/FeatureFlagDisable. +func (r *Inventory) AvailableToolsWithoutFeatureFiltering(ctx context.Context) []ServerTool { + var result []ServerTool + for i := range r.tools { + tool := &r.tools[i] + if r.isToolEnabledWithFeatureFlags(ctx, tool, false) { + result = append(result, *tool) } - return result[i].Tool.Name < result[j].Tool.Name - }) + } + + sortTools(result) return result } +func sortResourceTemplates(resourceTemplates []ServerResourceTemplate) { + sort.Slice(resourceTemplates, func(i, j int) bool { + if resourceTemplates[i].Toolset.ID != resourceTemplates[j].Toolset.ID { + return resourceTemplates[i].Toolset.ID < resourceTemplates[j].Toolset.ID + } + return resourceTemplates[i].Template.Name < resourceTemplates[j].Template.Name + }) +} + // AvailableResourceTemplates returns resource templates that pass all current filters, // sorted deterministically by toolset ID, then template name. // The context is used for feature flag evaluation. @@ -139,16 +172,36 @@ func (r *Inventory) AvailableResourceTemplates(ctx context.Context) []ServerReso } // Sort deterministically: by toolset ID, then by template name - sort.Slice(result, func(i, j int) bool { - if result[i].Toolset.ID != result[j].Toolset.ID { - return result[i].Toolset.ID < result[j].Toolset.ID + sortResourceTemplates(result) + + return result +} + +// AvailableResourceTemplatesWithoutFeatureFiltering returns resource templates +// that pass every filter except FeatureFlagEnable/FeatureFlagDisable. +func (r *Inventory) AvailableResourceTemplatesWithoutFeatureFiltering(_ context.Context) []ServerResourceTemplate { + var result []ServerResourceTemplate + for i := range r.resourceTemplates { + res := &r.resourceTemplates[i] + if r.isToolsetEnabled(res.Toolset.ID) { + result = append(result, *res) } - return result[i].Template.Name < result[j].Template.Name - }) + } + + sortResourceTemplates(result) return result } +func sortPrompts(prompts []ServerPrompt) { + sort.Slice(prompts, func(i, j int) bool { + if prompts[i].Toolset.ID != prompts[j].Toolset.ID { + return prompts[i].Toolset.ID < prompts[j].Toolset.ID + } + return prompts[i].Prompt.Name < prompts[j].Prompt.Name + }) +} + // AvailablePrompts returns prompts that pass all current filters, // sorted deterministically by toolset ID, then prompt name. // The context is used for feature flag evaluation. @@ -166,12 +219,23 @@ func (r *Inventory) AvailablePrompts(ctx context.Context) []ServerPrompt { } // Sort deterministically: by toolset ID, then by prompt name - sort.Slice(result, func(i, j int) bool { - if result[i].Toolset.ID != result[j].Toolset.ID { - return result[i].Toolset.ID < result[j].Toolset.ID + sortPrompts(result) + + return result +} + +// AvailablePromptsWithoutFeatureFiltering returns prompts that pass every filter +// except FeatureFlagEnable/FeatureFlagDisable. +func (r *Inventory) AvailablePromptsWithoutFeatureFiltering(_ context.Context) []ServerPrompt { + var result []ServerPrompt + for i := range r.prompts { + prompt := &r.prompts[i] + if r.isToolsetEnabled(prompt.Toolset.ID) { + result = append(result, *prompt) } - return result[i].Prompt.Name < result[j].Prompt.Name - }) + } + + sortPrompts(result) return result }