Skip to content

Commit 0f6b98f

Browse files
juruenjuruen
authored
Merge branch 'main' into patch-1
2 parents 03e0f5d + 3ec8699 commit 0f6b98f

17 files changed

+732
-142
lines changed

‎README.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,17 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
287287
- `draft`: Create as draft PR (boolean, optional)
288288
- `maintainer_can_modify`: Allow maintainer edits (boolean, optional)
289289

290+
- **update_pull_request** - Update an existing pull request in a GitHub repository
291+
292+
- `owner`: Repository owner (string, required)
293+
- `repo`: Repository name (string, required)
294+
- `pullNumber`: Pull request number to update (number, required)
295+
- `title`: New title (string, optional)
296+
- `body`: New description (string, optional)
297+
- `state`: New state ('open' or 'closed') (string, optional)
298+
- `base`: New base branch name (string, optional)
299+
- `maintainer_can_modify`: Allow maintainer edits (boolean, optional)
300+
290301
### Repositories
291302

292303
- **create_or_update_file** - Create or update a single file in a repository

‎cmd/github-mcp-server/main.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,11 @@ func runStdioServer(cfg runConfig) error {
137137

138138
t, dumpTranslations := translations.TranslationHelper()
139139

140+
getClient := func(_ context.Context) (*gogithub.Client, error) {
141+
return ghClient, nil // closing over client
142+
}
140143
// Create
141-
ghServer := github.NewServer(ghClient, version, cfg.readOnly, t)
144+
ghServer := github.NewServer(getClient, version, cfg.readOnly, t)
142145
stdioServer := server.NewStdioServer(ghServer)
143146

144147
stdLogger := stdlog.New(cfg.logger.Writer(), "stdioserver", 0)

‎pkg/github/code_scanning.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"github.com/mark3labs/mcp-go/server"
1414
)
1515

16-
func GetCodeScanningAlert(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
16+
func GetCodeScanningAlert(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
1717
return mcp.NewTool("get_code_scanning_alert",
1818
mcp.WithDescription(t("TOOL_GET_CODE_SCANNING_ALERT_DESCRIPTION", "Get details of a specific code scanning alert in a GitHub repository.")),
1919
mcp.WithString("owner",
@@ -43,6 +43,11 @@ func GetCodeScanningAlert(client *github.Client, t translations.TranslationHelpe
4343
return mcp.NewToolResultError(err.Error()), nil
4444
}
4545

46+
client, err := getClient(ctx)
47+
if err != nil {
48+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
49+
}
50+
4651
alert, resp, err := client.CodeScanning.GetAlert(ctx, owner, repo, int64(alertNumber))
4752
if err != nil {
4853
return nil, fmt.Errorf("failed to get alert: %w", err)
@@ -66,7 +71,7 @@ func GetCodeScanningAlert(client *github.Client, t translations.TranslationHelpe
6671
}
6772
}
6873

69-
func ListCodeScanningAlerts(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
74+
func ListCodeScanningAlerts(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
7075
return mcp.NewTool("list_code_scanning_alerts",
7176
mcp.WithDescription(t("TOOL_LIST_CODE_SCANNING_ALERTS_DESCRIPTION", "List code scanning alerts in a GitHub repository.")),
7277
mcp.WithString("owner",
@@ -110,6 +115,10 @@ func ListCodeScanningAlerts(client *github.Client, t translations.TranslationHel
110115
return mcp.NewToolResultError(err.Error()), nil
111116
}
112117

118+
client, err := getClient(ctx)
119+
if err != nil {
120+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
121+
}
113122
alerts, resp, err := client.CodeScanning.ListAlertsForRepo(ctx, owner, repo, &github.AlertListOptions{Ref: ref, State: state, Severity: severity})
114123
if err != nil {
115124
return nil, fmt.Errorf("failed to list alerts: %w", err)

‎pkg/github/code_scanning_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
func Test_GetCodeScanningAlert(t *testing.T) {
1717
// Verify tool definition once
1818
mockClient := github.NewClient(nil)
19-
tool, _ := GetCodeScanningAlert(mockClient, translations.NullTranslationHelper)
19+
tool, _ := GetCodeScanningAlert(stubGetClientFn(mockClient), translations.NullTranslationHelper)
2020

2121
assert.Equal(t, "get_code_scanning_alert", tool.Name)
2222
assert.NotEmpty(t, tool.Description)
@@ -82,7 +82,7 @@ func Test_GetCodeScanningAlert(t *testing.T) {
8282
t.Run(tc.name, func(t *testing.T) {
8383
// Setup client with mock
8484
client := github.NewClient(tc.mockedClient)
85-
_, handler := GetCodeScanningAlert(client, translations.NullTranslationHelper)
85+
_, handler := GetCodeScanningAlert(stubGetClientFn(client), translations.NullTranslationHelper)
8686

8787
// Create call request
8888
request := createMCPRequest(tc.requestArgs)
@@ -118,7 +118,7 @@ func Test_GetCodeScanningAlert(t *testing.T) {
118118
func Test_ListCodeScanningAlerts(t *testing.T) {
119119
// Verify tool definition once
120120
mockClient := github.NewClient(nil)
121-
tool, _ := ListCodeScanningAlerts(mockClient, translations.NullTranslationHelper)
121+
tool, _ := ListCodeScanningAlerts(stubGetClientFn(mockClient), translations.NullTranslationHelper)
122122

123123
assert.Equal(t, "list_code_scanning_alerts", tool.Name)
124124
assert.NotEmpty(t, tool.Description)
@@ -201,7 +201,7 @@ func Test_ListCodeScanningAlerts(t *testing.T) {
201201
t.Run(tc.name, func(t *testing.T) {
202202
// Setup client with mock
203203
client := github.NewClient(tc.mockedClient)
204-
_, handler := ListCodeScanningAlerts(client, translations.NullTranslationHelper)
204+
_, handler := ListCodeScanningAlerts(stubGetClientFn(client), translations.NullTranslationHelper)
205205

206206
// Create call request
207207
request := createMCPRequest(tc.requestArgs)

‎pkg/github/helper_test.go‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,115 @@ func getTextResult(t *testing.T, result *mcp.CallToolResult) mcp.TextContent {
9393
assert.Equal(t, "text", textContent.Type)
9494
return textContent
9595
}
96+
97+
func TestOptionalParamOK(t *testing.T) {
98+
tests := []struct {
99+
name string
100+
args map[string]interface{}
101+
paramName string
102+
expectedVal interface{}
103+
expectedOk bool
104+
expectError bool
105+
errorMsg string
106+
}{
107+
{
108+
name: "present and correct type (string)",
109+
args: map[string]interface{}{"myParam": "hello"},
110+
paramName: "myParam",
111+
expectedVal: "hello",
112+
expectedOk: true,
113+
expectError: false,
114+
},
115+
{
116+
name: "present and correct type (bool)",
117+
args: map[string]interface{}{"myParam": true},
118+
paramName: "myParam",
119+
expectedVal: true,
120+
expectedOk: true,
121+
expectError: false,
122+
},
123+
{
124+
name: "present and correct type (number)",
125+
args: map[string]interface{}{"myParam": float64(123)},
126+
paramName: "myParam",
127+
expectedVal: float64(123),
128+
expectedOk: true,
129+
expectError: false,
130+
},
131+
{
132+
name: "present but wrong type (string expected, got bool)",
133+
args: map[string]interface{}{"myParam": true},
134+
paramName: "myParam",
135+
expectedVal: "", // Zero value for string
136+
expectedOk: true, // ok is true because param exists
137+
expectError: true,
138+
errorMsg: "parameter myParam is not of type string, is bool",
139+
},
140+
{
141+
name: "present but wrong type (bool expected, got string)",
142+
args: map[string]interface{}{"myParam": "true"},
143+
paramName: "myParam",
144+
expectedVal: false, // Zero value for bool
145+
expectedOk: true, // ok is true because param exists
146+
expectError: true,
147+
errorMsg: "parameter myParam is not of type bool, is string",
148+
},
149+
{
150+
name: "parameter not present",
151+
args: map[string]interface{}{"anotherParam": "value"},
152+
paramName: "myParam",
153+
expectedVal: "", // Zero value for string
154+
expectedOk: false,
155+
expectError: false,
156+
},
157+
}
158+
159+
for _, tc := range tests {
160+
t.Run(tc.name, func(t *testing.T) {
161+
request := createMCPRequest(tc.args)
162+
163+
// Test with string type assertion
164+
if _, isString := tc.expectedVal.(string); isString || tc.errorMsg == "parameter myParam is not of type string, is bool" {
165+
val, ok, err := OptionalParamOK[string](request, tc.paramName)
166+
if tc.expectError {
167+
require.Error(t, err)
168+
assert.Contains(t, err.Error(), tc.errorMsg)
169+
assert.Equal(t, tc.expectedOk, ok) // Check ok even on error
170+
assert.Equal(t, tc.expectedVal, val) // Check zero value on error
171+
} else {
172+
require.NoError(t, err)
173+
assert.Equal(t, tc.expectedOk, ok)
174+
assert.Equal(t, tc.expectedVal, val)
175+
}
176+
}
177+
178+
// Test with bool type assertion
179+
if _, isBool := tc.expectedVal.(bool); isBool || tc.errorMsg == "parameter myParam is not of type bool, is string" {
180+
val, ok, err := OptionalParamOK[bool](request, tc.paramName)
181+
if tc.expectError {
182+
require.Error(t, err)
183+
assert.Contains(t, err.Error(), tc.errorMsg)
184+
assert.Equal(t, tc.expectedOk, ok) // Check ok even on error
185+
assert.Equal(t, tc.expectedVal, val) // Check zero value on error
186+
} else {
187+
require.NoError(t, err)
188+
assert.Equal(t, tc.expectedOk, ok)
189+
assert.Equal(t, tc.expectedVal, val)
190+
}
191+
}
192+
193+
// Test with float64 type assertion (for number case)
194+
if _, isFloat := tc.expectedVal.(float64); isFloat {
195+
val, ok, err := OptionalParamOK[float64](request, tc.paramName)
196+
if tc.expectError {
197+
// This case shouldn't happen for float64 in the defined tests
198+
require.Fail(t, "Unexpected error case for float64")
199+
} else {
200+
require.NoError(t, err)
201+
assert.Equal(t, tc.expectedOk, ok)
202+
assert.Equal(t, tc.expectedVal, val)
203+
}
204+
}
205+
})
206+
}
207+
}

‎pkg/github/issues.go‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
)
1616

1717
// GetIssue creates a tool to get details of a specific issue in a GitHub repository.
18-
func GetIssue(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
18+
func GetIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
1919
return mcp.NewTool("get_issue",
2020
mcp.WithDescription(t("TOOL_GET_ISSUE_DESCRIPTION", "Get details of a specific issue in a GitHub repository")),
2121
mcp.WithString("owner",
@@ -45,6 +45,10 @@ func GetIssue(client *github.Client, t translations.TranslationHelperFunc) (tool
4545
return mcp.NewToolResultError(err.Error()), nil
4646
}
4747

48+
client, err := getClient(ctx)
49+
if err != nil {
50+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
51+
}
4852
issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber)
4953
if err != nil {
5054
return nil, fmt.Errorf("failed to get issue: %w", err)
@@ -69,7 +73,7 @@ func GetIssue(client *github.Client, t translations.TranslationHelperFunc) (tool
6973
}
7074

7175
// AddIssueComment creates a tool to add a comment to an issue.
72-
func AddIssueComment(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
76+
func AddIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
7377
return mcp.NewTool("add_issue_comment",
7478
mcp.WithDescription(t("TOOL_ADD_ISSUE_COMMENT_DESCRIPTION", "Add a comment to an existing issue")),
7579
mcp.WithString("owner",
@@ -111,6 +115,10 @@ func AddIssueComment(client *github.Client, t translations.TranslationHelperFunc
111115
Body: github.Ptr(body),
112116
}
113117

118+
client, err := getClient(ctx)
119+
if err != nil {
120+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
121+
}
114122
createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment)
115123
if err != nil {
116124
return nil, fmt.Errorf("failed to create comment: %w", err)
@@ -135,7 +143,7 @@ func AddIssueComment(client *github.Client, t translations.TranslationHelperFunc
135143
}
136144

137145
// SearchIssues creates a tool to search for issues and pull requests.
138-
func SearchIssues(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
146+
func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
139147
return mcp.NewTool("search_issues",
140148
mcp.WithDescription(t("TOOL_SEARCH_ISSUES_DESCRIPTION", "Search for issues and pull requests across GitHub repositories")),
141149
mcp.WithString("q",
@@ -191,6 +199,10 @@ func SearchIssues(client *github.Client, t translations.TranslationHelperFunc) (
191199
},
192200
}
193201

202+
client, err := getClient(ctx)
203+
if err != nil {
204+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
205+
}
194206
result, resp, err := client.Search.Issues(ctx, query, opts)
195207
if err != nil {
196208
return nil, fmt.Errorf("failed to search issues: %w", err)
@@ -215,7 +227,7 @@ func SearchIssues(client *github.Client, t translations.TranslationHelperFunc) (
215227
}
216228

217229
// CreateIssue creates a tool to create a new issue in a GitHub repository.
218-
func CreateIssue(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
230+
func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
219231
return mcp.NewTool("create_issue",
220232
mcp.WithDescription(t("TOOL_CREATE_ISSUE_DESCRIPTION", "Create a new issue in a GitHub repository")),
221233
mcp.WithString("owner",
@@ -305,6 +317,10 @@ func CreateIssue(client *github.Client, t translations.TranslationHelperFunc) (t
305317
Milestone: milestoneNum,
306318
}
307319

320+
client, err := getClient(ctx)
321+
if err != nil {
322+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
323+
}
308324
issue, resp, err := client.Issues.Create(ctx, owner, repo, issueRequest)
309325
if err != nil {
310326
return nil, fmt.Errorf("failed to create issue: %w", err)
@@ -329,7 +345,7 @@ func CreateIssue(client *github.Client, t translations.TranslationHelperFunc) (t
329345
}
330346

331347
// ListIssues creates a tool to list and filter repository issues
332-
func ListIssues(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
348+
func ListIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
333349
return mcp.NewTool("list_issues",
334350
mcp.WithDescription(t("TOOL_LIST_ISSUES_DESCRIPTION", "List issues in a GitHub repository with filtering options")),
335351
mcp.WithString("owner",
@@ -419,6 +435,10 @@ func ListIssues(client *github.Client, t translations.TranslationHelperFunc) (to
419435
opts.PerPage = int(perPage)
420436
}
421437

438+
client, err := getClient(ctx)
439+
if err != nil {
440+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
441+
}
422442
issues, resp, err := client.Issues.ListByRepo(ctx, owner, repo, opts)
423443
if err != nil {
424444
return nil, fmt.Errorf("failed to list issues: %w", err)
@@ -443,7 +463,7 @@ func ListIssues(client *github.Client, t translations.TranslationHelperFunc) (to
443463
}
444464

445465
// UpdateIssue creates a tool to update an existing issue in a GitHub repository.
446-
func UpdateIssue(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
466+
func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
447467
return mcp.NewTool("update_issue",
448468
mcp.WithDescription(t("TOOL_UPDATE_ISSUE_DESCRIPTION", "Update an existing issue in a GitHub repository")),
449469
mcp.WithString("owner",
@@ -557,6 +577,10 @@ func UpdateIssue(client *github.Client, t translations.TranslationHelperFunc) (t
557577
issueRequest.Milestone = &milestoneNum
558578
}
559579

580+
client, err := getClient(ctx)
581+
if err != nil {
582+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
583+
}
560584
updatedIssue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest)
561585
if err != nil {
562586
return nil, fmt.Errorf("failed to update issue: %w", err)
@@ -581,7 +605,7 @@ func UpdateIssue(client *github.Client, t translations.TranslationHelperFunc) (t
581605
}
582606

583607
// GetIssueComments creates a tool to get comments for a GitHub issue.
584-
func GetIssueComments(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
608+
func GetIssueComments(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
585609
return mcp.NewTool("get_issue_comments",
586610
mcp.WithDescription(t("TOOL_GET_ISSUE_COMMENTS_DESCRIPTION", "Get comments for a GitHub issue")),
587611
mcp.WithString("owner",
@@ -632,6 +656,10 @@ func GetIssueComments(client *github.Client, t translations.TranslationHelperFun
632656
},
633657
}
634658

659+
client, err := getClient(ctx)
660+
if err != nil {
661+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
662+
}
635663
comments, resp, err := client.Issues.ListComments(ctx, owner, repo, issueNumber, opts)
636664
if err != nil {
637665
return nil, fmt.Errorf("failed to get issue comments: %w", err)

0 commit comments

Comments
 (0)