Skip to content

Commit 13cfccb

Browse files
committed
make JSON format default and CSVFormat flag, refactor data and metadata handling
1 parent e5a767e commit 13cfccb

File tree

12 files changed

+154
-182
lines changed

12 files changed

+154
-182
lines changed

cmd/github-mcp-server/main.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ var (
6262
LogFilePath: viper.GetString("log-file"),
6363
ContentWindowSize: viper.GetInt("content-window-size"),
6464
LockdownMode: viper.GetBool("lockdown-mode"),
65-
JSONFormat: viper.GetBool("json-format"),
65+
CSVFormat: viper.GetBool("csv-format"),
6666
TOONFormat: viper.GetBool("toon-format"),
6767
}
6868
return ghmcp.RunStdioServer(stdioServerConfig)
@@ -86,7 +86,7 @@ func init() {
8686
rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)")
8787
rootCmd.PersistentFlags().Int("content-window-size", 5000, "Specify the content window size")
8888
rootCmd.PersistentFlags().Bool("lockdown-mode", false, "Enable lockdown mode")
89-
rootCmd.PersistentFlags().Bool("json-format", false, "Use JSON output format instead of default CSV")
89+
rootCmd.PersistentFlags().Bool("csv-format", false, "Enable CSV output format")
9090
rootCmd.PersistentFlags().Bool("toon-format", false, "Enable TOON output format")
9191

9292
// Bind flag to viper
@@ -99,7 +99,7 @@ func init() {
9999
_ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host"))
100100
_ = viper.BindPFlag("content-window-size", rootCmd.PersistentFlags().Lookup("content-window-size"))
101101
_ = viper.BindPFlag("lockdown-mode", rootCmd.PersistentFlags().Lookup("lockdown-mode"))
102-
_ = viper.BindPFlag("json-format", rootCmd.PersistentFlags().Lookup("json-format"))
102+
_ = viper.BindPFlag("csv-format", rootCmd.PersistentFlags().Lookup("csv-format"))
103103
_ = viper.BindPFlag("toon-format", rootCmd.PersistentFlags().Lookup("toon-format"))
104104

105105
// Add subcommands

internal/ghmcp/server.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ type MCPServerConfig struct {
5555
// LockdownMode indicates if we should enable lockdown mode
5656
LockdownMode bool
5757

58-
// JSONFormat controls whether tools return a JSON response
59-
JSONFormat bool
58+
// CSVFormat controls whether tools return a CSV response
59+
CSVFormat bool
6060

6161
// TOONFormat controls whether tools return a TOON response
6262
TOONFormat bool
@@ -170,7 +170,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
170170
getRawClient,
171171
cfg.Translator,
172172
cfg.ContentWindowSize,
173-
github.FeatureFlags{LockdownMode: cfg.LockdownMode, JSONFormat: cfg.JSONFormat, TOONFormat: cfg.TOONFormat},
173+
github.FeatureFlags{LockdownMode: cfg.LockdownMode, CSVFormat: cfg.CSVFormat, TOONFormat: cfg.TOONFormat},
174174
)
175175
err = tsg.EnableToolsets(enabledToolsets, nil)
176176

@@ -226,8 +226,8 @@ type StdioServerConfig struct {
226226
// LockdownMode indicates if we should enable lockdown mode
227227
LockdownMode bool
228228

229-
// JSONFormat controls whether tools return a JSON response
230-
JSONFormat bool
229+
// CSVFormat controls whether tools return a CSV response
230+
CSVFormat bool
231231

232232
// TOONFormat controls whether tools return a TOON response
233233
TOONFormat bool
@@ -251,7 +251,7 @@ func RunStdioServer(cfg StdioServerConfig) error {
251251
Translator: t,
252252
ContentWindowSize: cfg.ContentWindowSize,
253253
LockdownMode: cfg.LockdownMode,
254-
JSONFormat: cfg.JSONFormat,
254+
CSVFormat: cfg.CSVFormat,
255255
TOONFormat: cfg.TOONFormat,
256256
})
257257
if err != nil {

pkg/github/feature_flags.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ package github
33
// FeatureFlags defines runtime feature toggles that adjust tool behavior.
44
type FeatureFlags struct {
55
LockdownMode bool
6-
// JSONFormat controls whether tools return a JSON response
7-
JSONFormat bool
6+
// CSVFormat controls whether tools return a CSV response
7+
CSVFormat bool
88
// TOONFormat controls whether tools return a TOON response
99
TOONFormat bool
1010
}

pkg/github/issues.go

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,17 @@ type IssueQueryResult interface {
131131
GetIssueFragment() IssueQueryFragment
132132
}
133133

134+
// PageInfo contains pagination information
135+
type PageInfo struct {
136+
HasNextPage githubv4.Boolean `json:"hasNextPage"`
137+
HasPreviousPage githubv4.Boolean `json:"hasPreviousPage"`
138+
StartCursor githubv4.String `json:"startCursor"`
139+
EndCursor githubv4.String `json:"endCursor"`
140+
}
141+
134142
type IssueQueryFragment struct {
135-
Nodes []IssueFragment `graphql:"nodes"`
136-
PageInfo struct {
137-
HasNextPage githubv4.Boolean
138-
HasPreviousPage githubv4.Boolean
139-
StartCursor githubv4.String
140-
EndCursor githubv4.String
141-
}
143+
Nodes []IssueFragment `graphql:"nodes"`
144+
PageInfo PageInfo
142145
TotalCount int
143146
}
144147

@@ -474,7 +477,7 @@ func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string,
474477
}
475478

476479
// ListIssueTypes creates a tool to list defined issue types for an organization. This can be used to understand supported issue type values for creating or updating issues.
477-
func ListIssueTypes(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
480+
func ListIssueTypes(getClient GetClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) {
478481

479482
return mcp.NewTool("list_issue_types",
480483
mcp.WithDescription(t("TOOL_LIST_ISSUE_TYPES_FOR_ORG", "List supported issue types for repository owner (organization).")),
@@ -511,12 +514,7 @@ func ListIssueTypes(getClient GetClientFn, t translations.TranslationHelperFunc)
511514
return mcp.NewToolResultError(fmt.Sprintf("failed to list issue types: %s", string(body))), nil
512515
}
513516

514-
r, err := json.Marshal(issueTypes)
515-
if err != nil {
516-
return nil, fmt.Errorf("failed to marshal issue types: %w", err)
517-
}
518-
519-
return mcp.NewToolResultText(string(r)), nil
517+
return FormatResponse(issueTypes, flags)
520518
}
521519
}
522520

@@ -807,7 +805,7 @@ func ReprioritizeSubIssue(ctx context.Context, client *github.Client, owner stri
807805
}
808806

809807
// SearchIssues creates a tool to search for issues.
810-
func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
808+
func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) {
811809
return mcp.NewTool("search_issues",
812810
mcp.WithDescription(t("TOOL_SEARCH_ISSUES_DESCRIPTION", "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue")),
813811
mcp.WithToolAnnotation(mcp.ToolAnnotation{
@@ -847,7 +845,7 @@ func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (
847845
WithPagination(),
848846
),
849847
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
850-
return searchHandler(ctx, getClient, request, "issue", "failed to search issues")
848+
return searchHandler(ctx, getClient, request, "issue", "failed to search issues", flags)
851849
}
852850
}
853851

@@ -1367,38 +1365,33 @@ func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFun
13671365

13681366
// Extract and convert all issue nodes using the common interface
13691367
var issues []*github.Issue
1370-
var pageInfo struct {
1371-
HasNextPage githubv4.Boolean
1372-
HasPreviousPage githubv4.Boolean
1373-
StartCursor githubv4.String
1374-
EndCursor githubv4.String
1375-
}
1376-
var totalCount int
1368+
var fragment IssueQueryFragment
13771369

13781370
if queryResult, ok := issueQuery.(IssueQueryResult); ok {
1379-
fragment := queryResult.GetIssueFragment()
1371+
fragment = queryResult.GetIssueFragment()
13801372
for _, issue := range fragment.Nodes {
13811373
issues = append(issues, fragmentToIssue(issue))
13821374
}
1383-
pageInfo = fragment.PageInfo
1384-
totalCount = fragment.TotalCount
13851375
}
13861376

1387-
// Create metadata for pagination
1388-
metadata := map[string]interface{}{
1389-
"pageInfo": map[string]interface{}{
1390-
"hasNextPage": pageInfo.HasNextPage,
1391-
"hasPreviousPage": pageInfo.HasPreviousPage,
1392-
"startCursor": string(pageInfo.StartCursor),
1393-
"endCursor": string(pageInfo.EndCursor),
1394-
},
1395-
"totalCount": totalCount,
1377+
// Create response with issues
1378+
response := &IssuesListResponse{
1379+
Issues: issues,
1380+
PageInfo: fragment.PageInfo,
1381+
TotalCount: fragment.TotalCount,
13961382
}
13971383

1398-
return FormatResponse(issues, flags, "issues", metadata)
1384+
return FormatResponse(response, flags)
13991385
}
14001386
}
14011387

1388+
// IssuesListResponse represents the response from list_issues with pagination
1389+
type IssuesListResponse struct {
1390+
Issues []*github.Issue `json:"issues"`
1391+
PageInfo PageInfo `json:"pageInfo"`
1392+
TotalCount int `json:"totalCount"`
1393+
}
1394+
14021395
// mvpDescription is an MVP idea for generating tool descriptions from structured data in a shared format.
14031396
// It is not intended for widespread usage and is not a complete implementation.
14041397
type mvpDescription struct {

pkg/github/issues_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ func Test_AddIssueComment(t *testing.T) {
365365
func Test_SearchIssues(t *testing.T) {
366366
// Verify tool definition once
367367
mockClient := github.NewClient(nil)
368-
tool, _ := SearchIssues(stubGetClientFn(mockClient), translations.NullTranslationHelper)
368+
tool, _ := SearchIssues(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(nil))
369369
require.NoError(t, toolsnaps.Test(tool.Name, tool))
370370

371371
assert.Equal(t, "search_issues", tool.Name)
@@ -654,7 +654,7 @@ func Test_SearchIssues(t *testing.T) {
654654
t.Run(tc.name, func(t *testing.T) {
655655
// Setup client with mock
656656
client := github.NewClient(tc.mockedClient)
657-
_, handler := SearchIssues(stubGetClientFn(client), translations.NullTranslationHelper)
657+
_, handler := SearchIssues(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(nil))
658658

659659
// Create call request
660660
request := createMCPRequest(tc.requestArgs)
@@ -857,7 +857,7 @@ func Test_CreateIssue(t *testing.T) {
857857
func Test_ListIssues(t *testing.T) {
858858
// Verify tool definition
859859
mockClient := githubv4.NewClient(nil)
860-
tool, _ := ListIssues(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper, FeatureFlags{JSONFormat: true})
860+
tool, _ := ListIssues(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(nil))
861861
require.NoError(t, toolsnaps.Test(tool.Name, tool))
862862

863863
assert.Equal(t, "list_issues", tool.Name)
@@ -1118,7 +1118,7 @@ func Test_ListIssues(t *testing.T) {
11181118
}
11191119

11201120
gqlClient := githubv4.NewClient(httpClient)
1121-
_, handler := ListIssues(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, FeatureFlags{JSONFormat: true})
1121+
_, handler := ListIssues(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(nil))
11221122

11231123
req := createMCPRequest(tc.reqParams)
11241124
res, err := handler(context.Background(), req)
@@ -3380,7 +3380,7 @@ func Test_ReprioritizeSubIssue(t *testing.T) {
33803380
func Test_ListIssueTypes(t *testing.T) {
33813381
// Verify tool definition once
33823382
mockClient := github.NewClient(nil)
3383-
tool, _ := ListIssueTypes(stubGetClientFn(mockClient), translations.NullTranslationHelper)
3383+
tool, _ := ListIssueTypes(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(nil))
33843384
require.NoError(t, toolsnaps.Test(tool.Name, tool))
33853385

33863386
assert.Equal(t, "list_issue_types", tool.Name)
@@ -3467,7 +3467,7 @@ func Test_ListIssueTypes(t *testing.T) {
34673467
t.Run(tc.name, func(t *testing.T) {
34683468
// Setup client with mock
34693469
client := github.NewClient(tc.mockedClient)
3470-
_, handler := ListIssueTypes(stubGetClientFn(client), translations.NullTranslationHelper)
3470+
_, handler := ListIssueTypes(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(nil))
34713471

34723472
// Create call request
34733473
request := createMCPRequest(tc.requestArgs)

pkg/github/pullrequests.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun
828828
}
829829
}
830830

831-
return FormatResponse(prs, flags, "")
831+
return FormatResponse(prs, flags)
832832
}
833833
}
834834

@@ -926,7 +926,7 @@ func MergePullRequest(getClient GetClientFn, t translations.TranslationHelperFun
926926
}
927927

928928
// SearchPullRequests creates a tool to search for pull requests.
929-
func SearchPullRequests(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
929+
func SearchPullRequests(getClient GetClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) {
930930
return mcp.NewTool("search_pull_requests",
931931
mcp.WithDescription(t("TOOL_SEARCH_PULL_REQUESTS_DESCRIPTION", "Search for pull requests in GitHub repositories using issues search syntax already scoped to is:pr")),
932932
mcp.WithToolAnnotation(mcp.ToolAnnotation{
@@ -966,7 +966,7 @@ func SearchPullRequests(getClient GetClientFn, t translations.TranslationHelperF
966966
WithPagination(),
967967
),
968968
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
969-
return searchHandler(ctx, getClient, request, "pr", "failed to search pull requests")
969+
return searchHandler(ctx, getClient, request, "pr", "failed to search pull requests", flags)
970970
}
971971
}
972972

pkg/github/pullrequests_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ func Test_UpdatePullRequest_Draft(t *testing.T) {
578578
func Test_ListPullRequests(t *testing.T) {
579579
// Verify tool definition once
580580
mockClient := github.NewClient(nil)
581-
tool, _ := ListPullRequests(stubGetClientFn(mockClient), translations.NullTranslationHelper, FeatureFlags{JSONFormat: true})
581+
tool, _ := ListPullRequests(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(nil))
582582
require.NoError(t, toolsnaps.Test(tool.Name, tool))
583583

584584
assert.Equal(t, "list_pull_requests", tool.Name)
@@ -671,7 +671,7 @@ func Test_ListPullRequests(t *testing.T) {
671671
t.Run(tc.name, func(t *testing.T) {
672672
// Setup client with mock
673673
client := github.NewClient(tc.mockedClient)
674-
_, handler := ListPullRequests(stubGetClientFn(client), translations.NullTranslationHelper, FeatureFlags{JSONFormat: true})
674+
_, handler := ListPullRequests(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(nil))
675675

676676
// Create call request
677677
request := createMCPRequest(tc.requestArgs)
@@ -826,7 +826,7 @@ func Test_MergePullRequest(t *testing.T) {
826826

827827
func Test_SearchPullRequests(t *testing.T) {
828828
mockClient := github.NewClient(nil)
829-
tool, _ := SearchPullRequests(stubGetClientFn(mockClient), translations.NullTranslationHelper)
829+
tool, _ := SearchPullRequests(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(nil))
830830
require.NoError(t, toolsnaps.Test(tool.Name, tool))
831831

832832
assert.Equal(t, "search_pull_requests", tool.Name)
@@ -1091,7 +1091,7 @@ func Test_SearchPullRequests(t *testing.T) {
10911091
t.Run(tc.name, func(t *testing.T) {
10921092
// Setup client with mock
10931093
client := github.NewClient(tc.mockedClient)
1094-
_, handler := SearchPullRequests(stubGetClientFn(client), translations.NullTranslationHelper)
1094+
_, handler := SearchPullRequests(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(nil))
10951095

10961096
// Create call request
10971097
request := createMCPRequest(tc.requestArgs)

pkg/github/repositories.go

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc, flag
101101
}
102102

103103
return mcp.NewToolResultText(string(r)), nil
104-
// return FormatResponse(minimalCommit, flags, "")
104+
// return FormatResponse(minimalCommit, flags)
105105
}
106106
}
107107

@@ -192,12 +192,12 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc, fl
192192
minimalCommits[i] = convertToMinimalCommit(commit, false)
193193
}
194194

195-
return FormatResponse(minimalCommits, flags, "")
195+
return FormatResponse(minimalCommits, flags)
196196
}
197197
}
198198

199199
// ListBranches creates a tool to list branches in a GitHub repository.
200-
func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
200+
func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) {
201201
return mcp.NewTool("list_branches",
202202
mcp.WithDescription(t("TOOL_LIST_BRANCHES_DESCRIPTION", "List branches in a GitHub repository")),
203203
mcp.WithToolAnnotation(mcp.ToolAnnotation{
@@ -264,12 +264,7 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) (
264264
minimalBranches = append(minimalBranches, convertToMinimalBranch(branch))
265265
}
266266

267-
r, err := json.Marshal(minimalBranches)
268-
if err != nil {
269-
return nil, fmt.Errorf("failed to marshal response: %w", err)
270-
}
271-
272-
return mcp.NewToolResultText(string(r)), nil
267+
return FormatResponse(minimalBranches, flags)
273268
}
274269
}
275270

@@ -1219,7 +1214,7 @@ func PushFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (too
12191214
}
12201215

12211216
// ListTags creates a tool to list tags in a GitHub repository.
1222-
func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
1217+
func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) {
12231218
return mcp.NewTool("list_tags",
12241219
mcp.WithDescription(t("TOOL_LIST_TAGS_DESCRIPTION", "List git tags in a GitHub repository")),
12251220
mcp.WithToolAnnotation(mcp.ToolAnnotation{
@@ -1278,12 +1273,7 @@ func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool
12781273
return mcp.NewToolResultError(fmt.Sprintf("failed to list tags: %s", string(body))), nil
12791274
}
12801275

1281-
r, err := json.Marshal(tags)
1282-
if err != nil {
1283-
return nil, fmt.Errorf("failed to marshal response: %w", err)
1284-
}
1285-
1286-
return mcp.NewToolResultText(string(r)), nil
1276+
return FormatResponse(tags, flags)
12871277
}
12881278
}
12891279

@@ -1375,7 +1365,7 @@ func GetTag(getClient GetClientFn, t translations.TranslationHelperFunc) (tool m
13751365
}
13761366

13771367
// ListReleases creates a tool to list releases in a GitHub repository.
1378-
func ListReleases(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
1368+
func ListReleases(getClient GetClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) {
13791369
return mcp.NewTool("list_releases",
13801370
mcp.WithDescription(t("TOOL_LIST_RELEASES_DESCRIPTION", "List releases in a GitHub repository")),
13811371
mcp.WithToolAnnotation(mcp.ToolAnnotation{
@@ -1430,12 +1420,7 @@ func ListReleases(getClient GetClientFn, t translations.TranslationHelperFunc) (
14301420
return mcp.NewToolResultError(fmt.Sprintf("failed to list releases: %s", string(body))), nil
14311421
}
14321422

1433-
r, err := json.Marshal(releases)
1434-
if err != nil {
1435-
return nil, fmt.Errorf("failed to marshal response: %w", err)
1436-
}
1437-
1438-
return mcp.NewToolResultText(string(r)), nil
1423+
return FormatResponse(releases, flags)
14391424
}
14401425
}
14411426

0 commit comments

Comments
 (0)