Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd01e705

Browse files
williammartinSamMorrowDrums
authored andcommitted
Use arrays rather than comma separated lists
1 parentb083631 commitd01e705

File tree

6 files changed

+134
-183
lines changed

6 files changed

+134
-183
lines changed

‎README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
130130
-`repo`: Repository name (string, required)
131131
-`title`: Issue title (string, required)
132132
-`body`: Issue body content (string, optional)
133-
-`assignees`:Comma-separated list of usernamesto assign to this issue (string, optional)
134-
-`labels`:Comma-separated list of labelsto apply to this issue (string, optional)
133+
-`assignees`:Usernamesto assign to this issue (string[], optional)
134+
-`labels`:Labelsto apply to this issue (string[], optional)
135135

136136
-**add_issue_comment** - Add a comment to an issue
137137

@@ -145,7 +145,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
145145
-`owner`: Repository owner (string, required)
146146
-`repo`: Repository name (string, required)
147147
-`state`: Filter by state ('open', 'closed', 'all') (string, optional)
148-
-`labels`:Comma-separated list of labelsto filter by (string, optional)
148+
-`labels`:Labelsto filter by (string[], optional)
149149
-`sort`: Sort by ('created', 'updated', 'comments') (string, optional)
150150
-`direction`: Sort direction ('asc', 'desc') (string, optional)
151151
-`since`: Filter by date (ISO 8601 timestamp) (string, optional)
@@ -160,8 +160,8 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
160160
-`title`: New title (string, optional)
161161
-`body`: New description (string, optional)
162162
-`state`: New state ('open' or 'closed') (string, optional)
163-
-`labels`:Comma-separated list of newlabels (string, optional)
164-
-`assignees`:Comma-separated list of newassignees (string, optional)
163+
-`labels`:Newlabels (string[], optional)
164+
-`assignees`:Newassignees (string[], optional)
165165
-`milestone`: New milestone number (number, optional)
166166

167167
-**search_issues** - Search for issues and pull requests

‎pkg/github/helper_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,52 @@ import (
1010
"github.com/stretchr/testify/require"
1111
)
1212

13+
// expectQueryParams is a helper function to create a partial mock that expects a
14+
// request with the given query parameters, with the ability to chain a response handler.
15+
funcexpectQueryParams(t*testing.T,expectedQueryParamsmap[string]string)*partialMock {
16+
return&partialMock{
17+
t:t,
18+
expectedQueryParams:expectedQueryParams,
19+
}
20+
}
21+
22+
// expectRequestBody is a helper function to create a partial mock that expects a
23+
// request with the given body, with the ability to chain a response handler.
24+
funcexpectRequestBody(t*testing.T,expectedRequestBodyany)*partialMock {
25+
return&partialMock{
26+
t:t,
27+
expectedRequestBody:expectedRequestBody,
28+
}
29+
}
30+
31+
typepartialMockstruct {
32+
t*testing.T
33+
expectedQueryParamsmap[string]string
34+
expectedRequestBodyany
35+
}
36+
37+
func (p*partialMock)andThen(responseHandler http.HandlerFunc) http.HandlerFunc {
38+
p.t.Helper()
39+
returnfunc(w http.ResponseWriter,r*http.Request) {
40+
ifp.expectedRequestBody!=nil {
41+
varunmarshaledRequestBodyany
42+
err:=json.NewDecoder(r.Body).Decode(&unmarshaledRequestBody)
43+
require.NoError(p.t,err)
44+
45+
require.Equal(p.t,p.expectedRequestBody,unmarshaledRequestBody)
46+
}
47+
48+
ifp.expectedQueryParams!=nil {
49+
require.Equal(p.t,len(p.expectedQueryParams),len(r.URL.Query()))
50+
fork,v:=rangep.expectedQueryParams {
51+
require.Equal(p.t,v,r.URL.Query().Get(k))
52+
}
53+
}
54+
55+
responseHandler(w,r)
56+
}
57+
}
58+
1359
// mockResponse is a helper function to create a mock HTTP response handler
1460
// that returns a specified status code and marshaled body.
1561
funcmockResponse(t*testing.T,codeint,bodyinterface{}) http.HandlerFunc {

‎pkg/github/issues.go

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -228,11 +228,21 @@ func createIssue(client *github.Client, t translations.TranslationHelperFunc) (t
228228
mcp.WithString("body",
229229
mcp.Description("Issue body content"),
230230
),
231-
mcp.WithString("assignees",
232-
mcp.Description("Comma-separate list of usernames to assign to this issue"),
233-
),
234-
mcp.WithString("labels",
235-
mcp.Description("Comma-separate list of labels to apply to this issue"),
231+
mcp.WithArray("assignees",
232+
mcp.Description("Usernames to assign to this issue"),
233+
mcp.Items(
234+
map[string]interface{}{
235+
"type":"string",
236+
},
237+
),
238+
),
239+
mcp.WithArray("labels",
240+
mcp.Description("Labels to apply to this issue"),
241+
mcp.Items(
242+
map[string]interface{}{
243+
"type":"string",
244+
},
245+
),
236246
),
237247
),
238248
func(ctx context.Context,request mcp.CallToolRequest) (*mcp.CallToolResult,error) {
@@ -256,12 +266,13 @@ func createIssue(client *github.Client, t translations.TranslationHelperFunc) (t
256266
}
257267

258268
// Get assignees
259-
assignees,err:=optionalCommaSeparatedListParam(request,"assignees")
269+
assignees,err:=optionalParam[[]string](request,"assignees")
260270
iferr!=nil {
261271
returnmcp.NewToolResultError(err.Error()),nil
262272
}
273+
263274
// Get labels
264-
labels,err:=optionalCommaSeparatedListParam(request,"labels")
275+
labels,err:=optionalParam[[]string](request,"labels")
265276
iferr!=nil {
266277
returnmcp.NewToolResultError(err.Error()),nil
267278
}
@@ -312,8 +323,13 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to
312323
mcp.WithString("state",
313324
mcp.Description("Filter by state ('open', 'closed', 'all')"),
314325
),
315-
mcp.WithString("labels",
316-
mcp.Description("Comma-separated list of labels to filter by"),
326+
mcp.WithArray("labels",
327+
mcp.Description("Filter by labels"),
328+
mcp.Items(
329+
map[string]interface{}{
330+
"type":"string",
331+
},
332+
),
317333
),
318334
mcp.WithString("sort",
319335
mcp.Description("Sort by ('created', 'updated', 'comments')"),
@@ -349,7 +365,8 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to
349365
returnmcp.NewToolResultError(err.Error()),nil
350366
}
351367

352-
opts.Labels,err=optionalCommaSeparatedListParam(request,"labels")
368+
// Get labels
369+
opts.Labels,err=optionalParam[[]string](request,"labels")
353370
iferr!=nil {
354371
returnmcp.NewToolResultError(err.Error()),nil
355372
}
@@ -431,12 +448,23 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t
431448
),
432449
mcp.WithString("state",
433450
mcp.Description("New state ('open' or 'closed')"),
434-
),
435-
mcp.WithString("labels",
436-
mcp.Description("Comma-separated list of new labels"),
437-
),
438-
mcp.WithString("assignees",
439-
mcp.Description("Comma-separated list of new assignees"),
451+
mcp.Enum("open","closed"),
452+
),
453+
mcp.WithArray("labels",
454+
mcp.Description("New labels"),
455+
mcp.Items(
456+
map[string]interface{}{
457+
"type":"string",
458+
},
459+
),
460+
),
461+
mcp.WithArray("assignees",
462+
mcp.Description("New assignees"),
463+
mcp.Items(
464+
map[string]interface{}{
465+
"type":"string",
466+
},
467+
),
440468
),
441469
mcp.WithNumber("milestone",
442470
mcp.Description("New milestone number"),
@@ -484,15 +512,17 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t
484512
issueRequest.State=github.Ptr(state)
485513
}
486514

487-
labels,err:=optionalCommaSeparatedListParam(request,"labels")
515+
// Get labels
516+
labels,err:=optionalParam[[]string](request,"labels")
488517
iferr!=nil {
489518
returnmcp.NewToolResultError(err.Error()),nil
490519
}
491520
iflen(labels)>0 {
492521
issueRequest.Labels=&labels
493522
}
494523

495-
assignees,err:=optionalCommaSeparatedListParam(request,"assignees")
524+
// Get assignees
525+
assignees,err:=optionalParam[[]string](request,"assignees")
496526
iferr!=nil {
497527
returnmcp.NewToolResultError(err.Error()),nil
498528
}

‎pkg/github/issues_test.go

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -418,16 +418,23 @@ func Test_CreateIssue(t *testing.T) {
418418
mockedClient:mock.NewMockedHTTPClient(
419419
mock.WithRequestMatchHandler(
420420
mock.PostReposIssuesByOwnerByRepo,
421-
mockResponse(t,http.StatusCreated,mockIssue),
421+
expectRequestBody(t,map[string]any{
422+
"title":"Test Issue",
423+
"body":"This is a test issue",
424+
"labels": []any{"bug","help wanted"},
425+
"assignees": []any{"user1","user2"},
426+
}).andThen(
427+
mockResponse(t,http.StatusCreated,mockIssue),
428+
),
422429
),
423430
),
424431
requestArgs:map[string]interface{}{
425432
"owner":"owner",
426433
"repo":"repo",
427434
"title":"Test Issue",
428435
"body":"This is a test issue",
429-
"assignees":"user1,user2",
430-
"labels":"bug,help wanted",
436+
"assignees":[]string{"user1","user2"},
437+
"labels":[]string{"bug","help wanted"},
431438
},
432439
expectError:false,
433440
expectedIssue:mockIssue,
@@ -606,16 +613,26 @@ func Test_ListIssues(t *testing.T) {
606613
{
607614
name:"list issues with all parameters",
608615
mockedClient:mock.NewMockedHTTPClient(
609-
mock.WithRequestMatch(
616+
mock.WithRequestMatchHandler(
610617
mock.GetReposIssuesByOwnerByRepo,
611-
mockIssues,
618+
expectQueryParams(t,map[string]string{
619+
"state":"open",
620+
"labels":"bug,enhancement",
621+
"sort":"created",
622+
"direction":"desc",
623+
"since":"2023-01-01T00:00:00Z",
624+
"page":"1",
625+
"per_page":"30",
626+
}).andThen(
627+
mockResponse(t,http.StatusOK,mockIssues),
628+
),
612629
),
613630
),
614631
requestArgs:map[string]interface{}{
615632
"owner":"owner",
616633
"repo":"repo",
617634
"state":"open",
618-
"labels":"bug,enhancement",
635+
"labels":[]string{"bug","enhancement"},
619636
"sort":"created",
620637
"direction":"desc",
621638
"since":"2023-01-01T00:00:00Z",
@@ -750,7 +767,16 @@ func Test_UpdateIssue(t *testing.T) {
750767
mockedClient:mock.NewMockedHTTPClient(
751768
mock.WithRequestMatchHandler(
752769
mock.PatchReposIssuesByOwnerByRepoByIssueNumber,
753-
mockResponse(t,http.StatusOK,mockIssue),
770+
expectRequestBody(t,map[string]any{
771+
"title":"Updated Issue Title",
772+
"body":"Updated issue description",
773+
"state":"closed",
774+
"labels": []any{"bug","priority"},
775+
"assignees": []any{"assignee1","assignee2"},
776+
"milestone":float64(5),
777+
}).andThen(
778+
mockResponse(t,http.StatusOK,mockIssue),
779+
),
754780
),
755781
),
756782
requestArgs:map[string]interface{}{
@@ -760,8 +786,8 @@ func Test_UpdateIssue(t *testing.T) {
760786
"title":"Updated Issue Title",
761787
"body":"Updated issue description",
762788
"state":"closed",
763-
"labels":"bug,priority",
764-
"assignees":"assignee1,assignee2",
789+
"labels":[]string{"bug","priority"},
790+
"assignees":[]string{"assignee1","assignee2"},
765791
"milestone":float64(5),
766792
},
767793
expectError:false,

‎pkg/github/server.go

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"io"
99
"net/http"
10-
"strings"
1110

1211
"github.com/github/github-mcp-server/pkg/translations"
1312
"github.com/google/go-github/v69/github"
@@ -119,25 +118,6 @@ func isAcceptedError(err error) bool {
119118
returnerrors.As(err,&acceptedError)
120119
}
121120

122-
// parseCommaSeparatedList is a helper function that parses a comma-separated list of strings from the input string.
123-
funcparseCommaSeparatedList(inputstring) []string {
124-
ifinput=="" {
125-
returnnil
126-
}
127-
128-
parts:=strings.Split(input,",")
129-
result:=make([]string,0,len(parts))
130-
131-
for_,part:=rangeparts {
132-
trimmed:=strings.TrimSpace(part)
133-
iftrimmed!="" {
134-
result=append(result,trimmed)
135-
}
136-
}
137-
138-
returnresult
139-
}
140-
141121
// requiredParam is a helper function that can be used to fetch a requested parameter from the request.
142122
// It does the following checks:
143123
// 1. Checks if the parameter is present in the request.
@@ -221,20 +201,3 @@ func optionalIntParamWithDefault(r mcp.CallToolRequest, p string, d int) (int, e
221201
}
222202
returnv,nil
223203
}
224-
225-
// optionalCommaSeparatedListParam is a helper function that can be used to fetch a requested parameter from the request.
226-
// It does the following:
227-
// 1. Checks if the parameter is present in the request, if not, it returns an empty list
228-
// 2. If it is present, it checks if the parameter is of the expected type and uses parseCommaSeparatedList to parse it
229-
// and return the list of strings
230-
funcoptionalCommaSeparatedListParam(r mcp.CallToolRequest,pstring) ([]string,error) {
231-
v,err:=optionalParam[string](r,p)
232-
iferr!=nil {
233-
return []string{},err
234-
}
235-
l:=parseCommaSeparatedList(v)
236-
iflen(l)==0 {
237-
return []string{},nil
238-
}
239-
returnl,nil
240-
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp