Gracefully handle numeric parameters passed as strings

pkg/github/copilot.go

@@ -209,7 +209,7 @@ func AssignCopilotToIssue(t translations.TranslationHelperFunc) inventory.Server
 				BaseRef            string `mapstructure:"base_ref"`
 				CustomInstructions string `mapstructure:"custom_instructions"`
 			}
-			if err := mapstructure.Decode(args, &params); err != nil {
+			if err := mapstructure.WeakDecode(args, &params); err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 

pkg/github/copilot_test.go

@@ -165,6 +165,115 @@ func TestAssignCopilotToIssue(t *testing.T) {
 				),
 			),
 		},
+		{
+			name: "successful assignment with string issue_number",
+			requestArgs: map[string]any{
+				"owner":        "owner",
+				"repo":         "repo",
+				"issue_number": "123", // Some MCP clients send numeric values as strings
+			},
+			mockedClient: githubv4mock.NewMockedHTTPClient(
+				githubv4mock.NewQueryMatcher(
+					struct {
+						Repository struct {
+							SuggestedActors struct {
+								Nodes []struct {
+									Bot struct {
+										ID       githubv4.ID
+										Login    githubv4.String
+										TypeName string `graphql:"__typename"`
+									} `graphql:"... on Bot"`
+								}
+								PageInfo struct {
+									HasNextPage bool
+									EndCursor   string
+								}
+							} `graphql:"suggestedActors(first: 100, after: $endCursor, capabilities: CAN_BE_ASSIGNED)"`
+						} `graphql:"repository(owner: $owner, name: $name)"`
+					}{},
+					map[string]any{
+						"owner":     githubv4.String("owner"),
+						"name":      githubv4.String("repo"),
+						"endCursor": (*githubv4.String)(nil),
+					},
+					githubv4mock.DataResponse(map[string]any{
+						"repository": map[string]any{
+							"suggestedActors": map[string]any{
+								"nodes": []any{
+									map[string]any{
+										"id":         githubv4.ID("copilot-swe-agent-id"),
+										"login":      githubv4.String("copilot-swe-agent"),
+										"__typename": "Bot",
+									},
+								},
+							},
+						},
+					}),
+				),
+				githubv4mock.NewQueryMatcher(
+					struct {
+						Repository struct {
+							ID    githubv4.ID
+							Issue struct {
+								ID        githubv4.ID
+								Assignees struct {
+									Nodes []struct {
+										ID githubv4.ID
+									}
+								} `graphql:"assignees(first: 100)"`
+							} `graphql:"issue(number: $number)"`
+						} `graphql:"repository(owner: $owner, name: $name)"`
+					}{},
+					map[string]any{
+						"owner":  githubv4.String("owner"),
+						"name":   githubv4.String("repo"),
+						"number": githubv4.Int(123),
+					},
+					githubv4mock.DataResponse(map[string]any{
+						"repository": map[string]any{
+							"id": githubv4.ID("test-repo-id"),
+							"issue": map[string]any{
+								"id": githubv4.ID("test-issue-id"),
+								"assignees": map[string]any{
+									"nodes": []any{},
+								},
+							},
+						},
+					}),
+				),
+				githubv4mock.NewMutationMatcher(
+					struct {
+						UpdateIssue struct {
+							Issue struct {
+								ID     githubv4.ID
+								Number githubv4.Int
+								URL    githubv4.String
+							}
+						} `graphql:"updateIssue(input: $input)"`
+					}{},
+					UpdateIssueInput{
+						ID:          githubv4.ID("test-issue-id"),
+						AssigneeIDs: []githubv4.ID{githubv4.ID("copilot-swe-agent-id")},
+						AgentAssignment: &AgentAssignmentInput{
+							BaseRef:            nil,
+							CustomAgent:        ptrGitHubv4String(""),
+							CustomInstructions: ptrGitHubv4String(""),
+							TargetRepositoryID: githubv4.ID("test-repo-id"),
+						},
+					},
+					nil,
+					githubv4mock.DataResponse(map[string]any{
+						"updateIssue": map[string]any{
+							"issue": map[string]any{
+								"id":     githubv4.ID("test-issue-id"),
+								"number": githubv4.Int(123),
+								"url":    githubv4.String("https://github.com/owner/repo/issues/123"),
+							},
+						},
+					}),
+				),
+			),
+		},
 		{
 			name: "successful assignment when there are existing assignees",
 			requestArgs: map[string]any{

pkg/github/discussions.go

@@ -313,7 +313,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
 				Repo             string
 				DiscussionNumber int32
 			}
-			if err := mapstructure.Decode(args, &params); err != nil {
+			if err := mapstructure.WeakDecode(args, &params); err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 			client, err := deps.GetGQLClient(ctx)
@@ -417,7 +417,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
 				Repo             string
 				DiscussionNumber int32
 			}
-			if err := mapstructure.Decode(args, &params); err != nil {
+			if err := mapstructure.WeakDecode(args, &params); err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 

pkg/github/discussions_test.go

@@ -590,6 +590,50 @@ func Test_GetDiscussion(t *testing.T) {
 	}
 }
 
+func Test_GetDiscussionWithStringNumber(t *testing.T) {
+	// Test that WeakDecode handles string discussionNumber from MCP clients
+	toolDef := GetDiscussion(translations.NullTranslationHelper)
+
+	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}}}}"
+
+	vars := map[string]any{
+		"owner":            "owner",
+		"repo":             "repo",
+		"discussionNumber": float64(1),
+	}
+
+	matcher := githubv4mock.NewQueryMatcher(qGetDiscussion, vars, githubv4mock.DataResponse(map[string]any{
+		"repository": map[string]any{"discussion": map[string]any{
+			"number":     1,
+			"title":      "Test Discussion Title",
+			"body":       "This is a test discussion",
+			"url":        "https://github.com/owner/repo/discussions/1",
+			"createdAt":  "2025-04-25T12:00:00Z",
+			"closed":     false,
+			"isAnswered": false,
+			"category":   map[string]any{"name": "General"},
+		}},
+	}))
+	httpClient := githubv4mock.NewMockedHTTPClient(matcher)
+	gqlClient := githubv4.NewClient(httpClient)
+	deps := BaseDeps{GQLClient: gqlClient}
+	handler := toolDef.Handler(deps)
+
+	// Send discussionNumber as a string instead of a number
+	reqParams := map[string]any{"owner": "owner", "repo": "repo", "discussionNumber": "1"}
+	req := createMCPRequest(reqParams)
+	res, err := handler(ContextWithDeps(context.Background(), deps), &req)
+	require.NoError(t, err)
+
+	text := getTextResult(t, res).Text
+	require.False(t, res.IsError, "expected no error, got: %s", text)
+
+	var out map[string]any
+	require.NoError(t, json.Unmarshal([]byte(text), &out))
+	assert.Equal(t, float64(1), out["number"])
+	assert.Equal(t, "Test Discussion Title", out["title"])
+}
+
 func Test_GetDiscussionComments(t *testing.T) {
 	// Verify tool definition and schema
 	toolDef := GetDiscussionComments(translations.NullTranslationHelper)
@@ -675,6 +719,67 @@ func Test_GetDiscussionComments(t *testing.T) {
 	}
 }
 
+func Test_GetDiscussionCommentsWithStringNumber(t *testing.T) {
+	// Test that WeakDecode handles string discussionNumber from MCP clients
+	toolDef := GetDiscussionComments(translations.NullTranslationHelper)
+
+	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}}}}"
+
+	vars := map[string]any{
+		"owner":            "owner",
+		"repo":             "repo",
+		"discussionNumber": float64(1),
+		"first":            float64(30),
+		"after":            (*string)(nil),
+	}
+
+	mockResponse := githubv4mock.DataResponse(map[string]any{
+		"repository": map[string]any{
+			"discussion": map[string]any{
+				"comments": map[string]any{
+					"nodes": []map[string]any{
+						{"body": "First comment"},
+					},
+					"pageInfo": map[string]any{
+						"hasNextPage":     false,
+						"hasPreviousPage": false,
+						"startCursor":     "",
+						"endCursor":       "",
+					},
+					"totalCount": 1,
+				},
+			},
+		},
+	})
+	matcher := githubv4mock.NewQueryMatcher(qGetComments, vars, mockResponse)
+	httpClient := githubv4mock.NewMockedHTTPClient(matcher)
+	gqlClient := githubv4.NewClient(httpClient)
+	deps := BaseDeps{GQLClient: gqlClient}
+	handler := toolDef.Handler(deps)
+
+	// Send discussionNumber as a string instead of a number
+	reqParams := map[string]any{
+		"owner":            "owner",
+		"repo":             "repo",
+		"discussionNumber": "1",
+	}
+	request := createMCPRequest(reqParams)
+
+	result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+	require.NoError(t, err)
+
+	textContent := getTextResult(t, result)
+	require.False(t, result.IsError, "expected no error, got: %s", textContent.Text)
+
+	var out struct {
+		Comments   []map[string]any `json:"comments"`
+		TotalCount int              `json:"totalCount"`
+	}
+	require.NoError(t, json.Unmarshal([]byte(textContent.Text), &out))
+	assert.Len(t, out.Comments, 1)
+	assert.Equal(t, "First comment", out.Comments[0]["body"])
+}
+
 func Test_ListDiscussionCategories(t *testing.T) {
 	toolDef := ListDiscussionCategories(translations.NullTranslationHelper)
 	tool := toolDef.Tool

pkg/github/params.go

@@ -39,6 +39,23 @@ func isAcceptedError(err error) bool {
 	return errors.As(err, &acceptedError)
 }
 
+// toFloat64 attempts to convert a value to float64, handling both numeric and
+// string representations. Some MCP clients send numeric values as strings.
+func toFloat64(val any) (float64, bool) {
+	switch v := val.(type) {
+	case float64:
+		return v, true
+	case string:
+		f, err := strconv.ParseFloat(v, 64)
+		if err != nil {
+			return 0, false
+		}
+		return f, true
+	default:
+		return 0, false
+	}
+}
+
 // RequiredParam is a helper function that can be used to fetch a requested parameter from the request.
 // It does the following checks:
 // 1. Checks if the parameter is present in the request.
@@ -68,32 +85,51 @@ func RequiredParam[T comparable](args map[string]any, p string) (T, error) {
 // RequiredInt is a helper function that can be used to fetch a requested parameter from the request.
 // It does the following checks:
 // 1. Checks if the parameter is present in the request.
-// 2. Checks if the parameter is of the expected type.
+// 2. Checks if the parameter is of the expected type (float64 or numeric string).
 // 3. Checks if the parameter is not empty, i.e: non-zero value
 func RequiredInt(args map[string]any, p string) (int, error) {
-	v, err := RequiredParam[float64](args, p)
-	if err != nil {
-		return 0, err
+	v, ok := args[p]
+	if !ok {
+		return 0, fmt.Errorf("missing required parameter: %s", p)
+	}
+
+	f, ok := toFloat64(v)
+	if !ok {
+		return 0, fmt.Errorf("parameter %s is not a valid number, is %T", p, v)
+	}
+
+	if f == 0 {
+		return 0, fmt.Errorf("missing required parameter: %s", p)
 	}
-	return int(v), nil
+
+	return int(f), nil
 }
 
 // RequiredBigInt is a helper function that can be used to fetch a requested parameter from the request.
 // It does the following checks:
 // 1. Checks if the parameter is present in the request.
-// 2. Checks if the parameter is of the expected type (float64).
+// 2. Checks if the parameter is of the expected type (float64 or numeric string).
 // 3. Checks if the parameter is not empty, i.e: non-zero value.
 // 4. Validates that the float64 value can be safely converted to int64 without truncation.
 func RequiredBigInt(args map[string]any, p string) (int64, error) {
-	v, err := RequiredParam[float64](args, p)
-	if err != nil {
-		return 0, err
+	val, ok := args[p]
+	if !ok {
+		return 0, fmt.Errorf("missing required parameter: %s", p)
+	}
+
+	f, ok := toFloat64(val)
+	if !ok {
+		return 0, fmt.Errorf("parameter %s is not a valid number, is %T", p, val)
+	}
+
+	if f == 0 {
+		return 0, fmt.Errorf("missing required parameter: %s", p)
 	}
 
-	result := int64(v)
+	result := int64(f)
 	// Check if converting back produces the same value to avoid silent truncation
-	if float64(result) != v {
-		return 0, fmt.Errorf("parameter %s value %f is too large to fit in int64", p, v)
+	if float64(result) != f {
+		return 0, fmt.Errorf("parameter %s value %f is too large to fit in int64", p, f)
 	}
 	return result, nil
 }
@@ -121,13 +157,19 @@ func OptionalParam[T any](args map[string]any, p string) (T, error) {
 // OptionalIntParam is a helper function that can be used to fetch a requested parameter from the request.
 // It does the following checks:
 // 1. Checks if the parameter is present in the request, if not, it returns its zero-value
-// 2. If it is present, it checks if the parameter is of the expected type and returns it
+// 2. If it is present, it checks if the parameter is of the expected type (float64 or numeric string) and returns it
 func OptionalIntParam(args map[string]any, p string) (int, error) {
-	v, err := OptionalParam[float64](args, p)
-	if err != nil {
-		return 0, err
+	val, ok := args[p]
+	if !ok {
+		return 0, nil
 	}
-	return int(v), nil
+
+	f, ok := toFloat64(val)
+	if !ok {
+		return 0, fmt.Errorf("parameter %s is not a valid number, is %T", p, val)
+	}
+
+	return int(f), nil
 }
 
 // OptionalIntParamWithDefault is a helper function that can be used to fetch a requested parameter from the request