From b070b00f9841006ef02a26ca918898951fbcf9b9 Mon Sep 17 00:00:00 2001 From: RossTarrant Date: Mon, 11 May 2026 12:38:19 +0100 Subject: [PATCH] Add CSV output for list tools under insiders mode --- cmd/github-mcp-server/main.go | 8 + docs/insiders-features.md | 25 ++ pkg/github/csv_output.go | 412 +++++++++++++++++++++++++++++++++ pkg/github/csv_output_test.go | 415 ++++++++++++++++++++++++++++++++++ pkg/github/feature_flags.go | 5 + pkg/github/tools.go | 4 +- pkg/http/handler_test.go | 2 +- pkg/http/server.go | 20 +- pkg/http/server_test.go | 24 +- 9 files changed, 903 insertions(+), 12 deletions(-) create mode 100644 pkg/github/csv_output.go create mode 100644 pkg/github/csv_output_test.go diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index 8f2ae58525..a6e83b2b48 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -127,6 +127,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, @@ -146,6 +153,7 @@ var ( EnabledTools: enabledTools, DynamicToolsets: viper.GetBool("dynamic_toolsets"), 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/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/feature_flags.go b/pkg/github/feature_flags.go index 3f3d7bf976..2b9212ad50 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -3,12 +3,16 @@ package github // 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" + // 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,6 +23,7 @@ var AllowedFeatureFlags = []string{ // feature flag expansion. var InsidersFeatureFlags = []string{ MCPAppsFeatureFlag, + FeatureFlagCSVOutput, } // FeatureFlags defines runtime feature toggles that adjust tool behavior. diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 559088f6d6..90c61f39e6 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -172,7 +172,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), @@ -315,7 +315,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/http/handler_test.go b/pkg/http/handler_test.go index 9887ff1f3b..da4391ca94 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -847,7 +847,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/server.go b/pkg/http/server.go index f7cdaf9093..3d8b14c5b5 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -85,6 +85,9 @@ type ServerConfig struct { // When set via CLI flag, per-request headers cannot re-include these tools. ExcludeTools []string + // EnabledFeatures is a list of feature flags that are enabled. + EnabledFeatures []string + // InsidersMode indicates if we should enable experimental features. InsidersMode bool } @@ -124,7 +127,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 { @@ -231,14 +234,17 @@ 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, then validates +// against the centralized AllowedFeatureFlags allowlist. +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..034497fa49 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,25 @@ 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: "static insiders enables insiders flags without route context", + staticInsiders: true, + flagName: github.FeatureFlagCSVOutput, + wantEnabled: true, + }, { name: "insiders mode does not enable granular flags", flagName: github.FeatureFlagIssuesGranular, @@ -84,6 +103,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)