Skip to content

Commit c4ad757

Browse files
almaleksiaalmaleksia
authored
Merge branch 'main' into feature/fix-empty-file-error
2 parents 6568249 + ccb9b53 commit c4ad757

File tree

14 files changed

+752
-150
lines changed

14 files changed

+752
-150
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1171,7 +1171,7 @@ The following sets of tools are available:
11711171
- `owner`: Repository owner (username or organization) (string, required)
11721172
- `path`: Path where to create/update the file (string, required)
11731173
- `repo`: Repository name (string, required)
1174-
- `sha`: The blob SHA of the file being replaced. (string, optional)
1174+
- `sha`: The blob SHA of the file being replaced. Required if the file already exists. (string, optional)
11751175

11761176
- **create_repository** - Create repository
11771177
- **Required OAuth Scopes**: `repo`

‎pkg/github/__toolsnaps__/create_or_update_file.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"annotations": {
33
"title": "Create or update file"
44
},
5-
"description": "Create or update a single file in a GitHub repository. \nIf updating, you should provide the SHA of the file you want to update. Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations.\n\nIn order to obtain the SHA of original file version before updating, use the following git command:\ngit ls-tree HEAD \u003cpath to file\u003e\n\nIf the SHA is not provided, the tool will attempt to acquire it by fetching the current file contents from the repository, which may lead to rewriting latest committed changes if the file has changed since last retrieval.\n",
5+
"description": "Create or update a single file in a GitHub repository. \nIf updating, you should provide the SHA of the file you want to update. Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations.\n\nIn order to obtain the SHA of original file version before updating, use the following git command:\ngit rev-parse \u003cbranch\u003e:\u003cpath to file\u003e\n\nSHA MUST be provided for existing file updates.\n",
66
"inputSchema": {
77
"properties": {
88
"branch": {
@@ -30,7 +30,7 @@
3030
"type": "string"
3131
},
3232
"sha": {
33-
"description": "The blob SHA of the file being replaced.",
33+
"description": "The blob SHA of the file being replaced. Required if the file already exists.",
3434
"type": "string"
3535
}
3636
},

‎pkg/github/copilot.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func AssignCopilotToIssue(t translations.TranslationHelperFunc) inventory.Server
209209
BaseRef string `mapstructure:"base_ref"`
210210
CustomInstructions string `mapstructure:"custom_instructions"`
211211
}
212-
if err := mapstructure.Decode(args, &params); err != nil {
212+
if err := mapstructure.WeakDecode(args, &params); err != nil {
213213
return utils.NewToolResultError(err.Error()), nil, nil
214214
}
215215

‎pkg/github/copilot_test.go‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,115 @@ func TestAssignCopilotToIssue(t *testing.T) {
165165
),
166166
),
167167
},
168+
{
169+
name: "successful assignment with string issue_number",
170+
requestArgs: map[string]any{
171+
"owner": "owner",
172+
"repo": "repo",
173+
"issue_number": "123", // Some MCP clients send numeric values as strings
174+
},
175+
mockedClient: githubv4mock.NewMockedHTTPClient(
176+
githubv4mock.NewQueryMatcher(
177+
struct {
178+
Repository struct {
179+
SuggestedActors struct {
180+
Nodes []struct {
181+
Bot struct {
182+
ID githubv4.ID
183+
Login githubv4.String
184+
TypeName string `graphql:"__typename"`
185+
} `graphql:"... on Bot"`
186+
}
187+
PageInfo struct {
188+
HasNextPage bool
189+
EndCursor string
190+
}
191+
} `graphql:"suggestedActors(first: 100, after: $endCursor, capabilities: CAN_BE_ASSIGNED)"`
192+
} `graphql:"repository(owner: $owner, name: $name)"`
193+
}{},
194+
map[string]any{
195+
"owner": githubv4.String("owner"),
196+
"name": githubv4.String("repo"),
197+
"endCursor": (*githubv4.String)(nil),
198+
},
199+
githubv4mock.DataResponse(map[string]any{
200+
"repository": map[string]any{
201+
"suggestedActors": map[string]any{
202+
"nodes": []any{
203+
map[string]any{
204+
"id": githubv4.ID("copilot-swe-agent-id"),
205+
"login": githubv4.String("copilot-swe-agent"),
206+
"__typename": "Bot",
207+
},
208+
},
209+
},
210+
},
211+
}),
212+
),
213+
githubv4mock.NewQueryMatcher(
214+
struct {
215+
Repository struct {
216+
ID githubv4.ID
217+
Issue struct {
218+
ID githubv4.ID
219+
Assignees struct {
220+
Nodes []struct {
221+
ID githubv4.ID
222+
}
223+
} `graphql:"assignees(first: 100)"`
224+
} `graphql:"issue(number: $number)"`
225+
} `graphql:"repository(owner: $owner, name: $name)"`
226+
}{},
227+
map[string]any{
228+
"owner": githubv4.String("owner"),
229+
"name": githubv4.String("repo"),
230+
"number": githubv4.Int(123),
231+
},
232+
githubv4mock.DataResponse(map[string]any{
233+
"repository": map[string]any{
234+
"id": githubv4.ID("test-repo-id"),
235+
"issue": map[string]any{
236+
"id": githubv4.ID("test-issue-id"),
237+
"assignees": map[string]any{
238+
"nodes": []any{},
239+
},
240+
},
241+
},
242+
}),
243+
),
244+
githubv4mock.NewMutationMatcher(
245+
struct {
246+
UpdateIssue struct {
247+
Issue struct {
248+
ID githubv4.ID
249+
Number githubv4.Int
250+
URL githubv4.String
251+
}
252+
} `graphql:"updateIssue(input: $input)"`
253+
}{},
254+
UpdateIssueInput{
255+
ID: githubv4.ID("test-issue-id"),
256+
AssigneeIDs: []githubv4.ID{githubv4.ID("copilot-swe-agent-id")},
257+
AgentAssignment: &AgentAssignmentInput{
258+
BaseRef: nil,
259+
CustomAgent: ptrGitHubv4String(""),
260+
CustomInstructions: ptrGitHubv4String(""),
261+
TargetRepositoryID: githubv4.ID("test-repo-id"),
262+
},
263+
},
264+
nil,
265+
githubv4mock.DataResponse(map[string]any{
266+
"updateIssue": map[string]any{
267+
"issue": map[string]any{
268+
"id": githubv4.ID("test-issue-id"),
269+
"number": githubv4.Int(123),
270+
"url": githubv4.String("https://github.com/owner/repo/issues/123"),
271+
},
272+
},
273+
}),
274+
),
275+
),
276+
},
168277
{
169278
name: "successful assignment when there are existing assignees",
170279
requestArgs: map[string]any{

‎pkg/github/discussions.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
313313
Repo string
314314
DiscussionNumber int32
315315
}
316-
if err := mapstructure.Decode(args, &params); err != nil {
316+
if err := mapstructure.WeakDecode(args, &params); err != nil {
317317
return utils.NewToolResultError(err.Error()), nil, nil
318318
}
319319
client, err := deps.GetGQLClient(ctx)
@@ -417,7 +417,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
417417
Repo string
418418
DiscussionNumber int32
419419
}
420-
if err := mapstructure.Decode(args, &params); err != nil {
420+
if err := mapstructure.WeakDecode(args, &params); err != nil {
421421
return utils.NewToolResultError(err.Error()), nil, nil
422422
}
423423

‎pkg/github/discussions_test.go‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,50 @@ func Test_GetDiscussion(t *testing.T) {
590590
}
591591
}
592592

593+
func Test_GetDiscussionWithStringNumber(t *testing.T) {
594+
// Test that WeakDecode handles string discussionNumber from MCP clients
595+
toolDef := GetDiscussion(translations.NullTranslationHelper)
596+
597+
qGetDiscussion := "query($discussionNumber:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){number,title,body,createdAt,closed,isAnswered,answerChosenAt,url,category{name}}}}"
598+
599+
vars := map[string]any{
600+
"owner": "owner",
601+
"repo": "repo",
602+
"discussionNumber": float64(1),
603+
}
604+
605+
matcher := githubv4mock.NewQueryMatcher(qGetDiscussion, vars, githubv4mock.DataResponse(map[string]any{
606+
"repository": map[string]any{"discussion": map[string]any{
607+
"number": 1,
608+
"title": "Test Discussion Title",
609+
"body": "This is a test discussion",
610+
"url": "https://github.com/owner/repo/discussions/1",
611+
"createdAt": "2025-04-25T12:00:00Z",
612+
"closed": false,
613+
"isAnswered": false,
614+
"category": map[string]any{"name": "General"},
615+
}},
616+
}))
617+
httpClient := githubv4mock.NewMockedHTTPClient(matcher)
618+
gqlClient := githubv4.NewClient(httpClient)
619+
deps := BaseDeps{GQLClient: gqlClient}
620+
handler := toolDef.Handler(deps)
621+
622+
// Send discussionNumber as a string instead of a number
623+
reqParams := map[string]any{"owner": "owner", "repo": "repo", "discussionNumber": "1"}
624+
req := createMCPRequest(reqParams)
625+
res, err := handler(ContextWithDeps(context.Background(), deps), &req)
626+
require.NoError(t, err)
627+
628+
text := getTextResult(t, res).Text
629+
require.False(t, res.IsError, "expected no error, got: %s", text)
630+
631+
var out map[string]any
632+
require.NoError(t, json.Unmarshal([]byte(text), &out))
633+
assert.Equal(t, float64(1), out["number"])
634+
assert.Equal(t, "Test Discussion Title", out["title"])
635+
}
636+
593637
func Test_GetDiscussionComments(t *testing.T) {
594638
// Verify tool definition and schema
595639
toolDef := GetDiscussionComments(translations.NullTranslationHelper)
@@ -675,6 +719,67 @@ func Test_GetDiscussionComments(t *testing.T) {
675719
}
676720
}
677721

722+
func Test_GetDiscussionCommentsWithStringNumber(t *testing.T) {
723+
// Test that WeakDecode handles string discussionNumber from MCP clients
724+
toolDef := GetDiscussionComments(translations.NullTranslationHelper)
725+
726+
qGetComments := "query($after:String$discussionNumber:Int!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){comments(first: $first, after: $after){nodes{body},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}}"
727+
728+
vars := map[string]any{
729+
"owner": "owner",
730+
"repo": "repo",
731+
"discussionNumber": float64(1),
732+
"first": float64(30),
733+
"after": (*string)(nil),
734+
}
735+
736+
mockResponse := githubv4mock.DataResponse(map[string]any{
737+
"repository": map[string]any{
738+
"discussion": map[string]any{
739+
"comments": map[string]any{
740+
"nodes": []map[string]any{
741+
{"body": "First comment"},
742+
},
743+
"pageInfo": map[string]any{
744+
"hasNextPage": false,
745+
"hasPreviousPage": false,
746+
"startCursor": "",
747+
"endCursor": "",
748+
},
749+
"totalCount": 1,
750+
},
751+
},
752+
},
753+
})
754+
matcher := githubv4mock.NewQueryMatcher(qGetComments, vars, mockResponse)
755+
httpClient := githubv4mock.NewMockedHTTPClient(matcher)
756+
gqlClient := githubv4.NewClient(httpClient)
757+
deps := BaseDeps{GQLClient: gqlClient}
758+
handler := toolDef.Handler(deps)
759+
760+
// Send discussionNumber as a string instead of a number
761+
reqParams := map[string]any{
762+
"owner": "owner",
763+
"repo": "repo",
764+
"discussionNumber": "1",
765+
}
766+
request := createMCPRequest(reqParams)
767+
768+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
769+
require.NoError(t, err)
770+
771+
textContent := getTextResult(t, result)
772+
require.False(t, result.IsError, "expected no error, got: %s", textContent.Text)
773+
774+
var out struct {
775+
Comments []map[string]any `json:"comments"`
776+
TotalCount int `json:"totalCount"`
777+
}
778+
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &out))
779+
assert.Len(t, out.Comments, 1)
780+
assert.Equal(t, "First comment", out.Comments[0]["body"])
781+
}
782+
678783
func Test_ListDiscussionCategories(t *testing.T) {
679784
toolDef := ListDiscussionCategories(translations.NullTranslationHelper)
680785
tool := toolDef.Tool

0 commit comments

Comments
 (0)