From 4bda9d35f3d439aecb410bbcdbfac5547266a7bf Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 5 Dec 2025 17:56:09 +0000 Subject: [PATCH 1/4] Fix SonarQube HIGH severity issues in discussions.go - Replace duplicated string literals 'Repository owner' and 'Repository name' with existing constants DescriptionRepositoryOwner and DescriptionRepositoryName from actions.go (fixes go:S1192) - Reduce cognitive complexity of ListDiscussions function by extracting: - discussionNode type for GraphQL response structure - discussionNodeToIssue helper function for mapping nodes to GitHub Issue objects - fetchDiscussionNodes helper function for GraphQL query execution (fixes go:S3776) Co-Authored-By: Shannon Hittson --- pkg/github/discussions.go | 172 ++++++++++++++++++-------------------- 1 file changed, 82 insertions(+), 90 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index a7ec8e20f..9a63d2396 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -13,6 +13,71 @@ import ( "github.com/shurcooL/githubv4" ) +// discussionNode represents a discussion node from the GraphQL API +type discussionNode struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + Category struct { + Name githubv4.String + } `graphql:"category"` + URL githubv4.String `graphql:"url"` +} + +// discussionNodeToIssue converts a discussionNode to a GitHub Issue object +func discussionNodeToIssue(n discussionNode) *github.Issue { + return &github.Issue{ + Number: github.Ptr(int(n.Number)), + Title: github.Ptr(string(n.Title)), + HTMLURL: github.Ptr(string(n.URL)), + CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, + Labels: []*github.Label{ + { + Name: github.Ptr(fmt.Sprintf("category:%s", string(n.Category.Name))), + }, + }, + } +} + +// fetchDiscussionNodes fetches discussion nodes from the GraphQL API +// If categoryID is provided, it filters by category; otherwise, it fetches all discussions +func fetchDiscussionNodes(ctx context.Context, client *githubv4.Client, owner, repo string, categoryID *githubv4.ID) ([]discussionNode, error) { + if categoryID != nil { + var query struct { + Repository struct { + Discussions struct { + Nodes []discussionNode + } `graphql:"discussions(first: 100, categoryId: $categoryId)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "categoryId": *categoryID, + } + if err := client.Query(ctx, &query, vars); err != nil { + return nil, err + } + return query.Repository.Discussions.Nodes, nil + } + + var query struct { + Repository struct { + Discussions struct { + Nodes []discussionNode + } `graphql:"discussions(first: 100)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + } + if err := client.Query(ctx, &query, vars); err != nil { + return nil, err + } + return query.Repository.Discussions.Nodes, nil +} + func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_discussions", mcp.WithDescription(t("TOOL_LIST_DISCUSSIONS_DESCRIPTION", "List discussions for a repository")), @@ -22,11 +87,11 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("category", mcp.Description("Optional filter by discussion category ID. If provided, only discussions with this category are listed."), @@ -61,89 +126,16 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp categoryID = &id } - // Now execute the discussions query - var discussions []*github.Issue - if categoryID != nil { - // Query with category filter (server-side filtering) - var query struct { - Repository struct { - Discussions struct { - Nodes []struct { - Number githubv4.Int - Title githubv4.String - CreatedAt githubv4.DateTime - Category struct { - Name githubv4.String - } `graphql:"category"` - URL githubv4.String `graphql:"url"` - } - } `graphql:"discussions(first: 100, categoryId: $categoryId)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - "categoryId": *categoryID, - } - if err := client.Query(ctx, &query, vars); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - // Map nodes to GitHub Issue objects - for _, n := range query.Repository.Discussions.Nodes { - di := &github.Issue{ - Number: github.Ptr(int(n.Number)), - Title: github.Ptr(string(n.Title)), - HTMLURL: github.Ptr(string(n.URL)), - CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, - Labels: []*github.Label{ - { - Name: github.Ptr(fmt.Sprintf("category:%s", string(n.Category.Name))), - }, - }, - } - discussions = append(discussions, di) - } - } else { - // Query without category filter - var query struct { - Repository struct { - Discussions struct { - Nodes []struct { - Number githubv4.Int - Title githubv4.String - CreatedAt githubv4.DateTime - Category struct { - Name githubv4.String - } `graphql:"category"` - URL githubv4.String `graphql:"url"` - } - } `graphql:"discussions(first: 100)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - } - if err := client.Query(ctx, &query, vars); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } + // Fetch discussions using the appropriate query based on category filter + nodes, err := fetchDiscussionNodes(ctx, client, owner, repo, categoryID) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } - // Map nodes to GitHub Issue objects - for _, n := range query.Repository.Discussions.Nodes { - di := &github.Issue{ - Number: github.Ptr(int(n.Number)), - Title: github.Ptr(string(n.Title)), - HTMLURL: github.Ptr(string(n.URL)), - CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, - Labels: []*github.Label{ - { - Name: github.Ptr(fmt.Sprintf("category:%s", string(n.Category.Name))), - }, - }, - } - discussions = append(discussions, di) - } + // Map nodes to GitHub Issue objects + discussions := make([]*github.Issue, 0, len(nodes)) + for _, n := range nodes { + discussions = append(discussions, discussionNodeToIssue(n)) } // Marshal and return @@ -164,11 +156,11 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("discussionNumber", mcp.Required(), @@ -241,8 +233,8 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati Title: t("TOOL_GET_DISCUSSION_COMMENTS_USER_TITLE", "Get discussion comments"), ReadOnlyHint: ToBoolPtr(true), }), - mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner")), - mcp.WithString("repo", mcp.Required(), mcp.Description("Repository name")), + mcp.WithString("owner", mcp.Required(), mcp.Description(DescriptionRepositoryOwner)), + mcp.WithString("repo", mcp.Required(), mcp.Description(DescriptionRepositoryName)), mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number")), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -303,11 +295,11 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("first", mcp.Description("Number of categories to return per page (min 1, max 100)"), From 035aae39a3382ea358c8f075d97e009d8bf8f744 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 5 Dec 2025 18:04:30 +0000 Subject: [PATCH 2/4] Refactor: Fix SonarQube issues - use constants for string literals, reduce cognitive complexity - discussions.go: Replace duplicated 'Repository owner' and 'Repository name' strings with DescriptionRepositoryOwner and DescriptionRepositoryName constants (go:S1192) - notifications.go: Extract executeThreadSubscriptionAction helper function to reduce cognitive complexity of ManageNotificationSubscription (go:S3776) Co-Authored-By: Shannon Hittson --- pkg/github/discussions.go | 156 +++++++++++++++++++----------------- pkg/github/notifications.go | 38 +++++---- 2 files changed, 100 insertions(+), 94 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 9a63d2396..59a9bfce6 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -13,71 +13,6 @@ import ( "github.com/shurcooL/githubv4" ) -// discussionNode represents a discussion node from the GraphQL API -type discussionNode struct { - Number githubv4.Int - Title githubv4.String - CreatedAt githubv4.DateTime - Category struct { - Name githubv4.String - } `graphql:"category"` - URL githubv4.String `graphql:"url"` -} - -// discussionNodeToIssue converts a discussionNode to a GitHub Issue object -func discussionNodeToIssue(n discussionNode) *github.Issue { - return &github.Issue{ - Number: github.Ptr(int(n.Number)), - Title: github.Ptr(string(n.Title)), - HTMLURL: github.Ptr(string(n.URL)), - CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, - Labels: []*github.Label{ - { - Name: github.Ptr(fmt.Sprintf("category:%s", string(n.Category.Name))), - }, - }, - } -} - -// fetchDiscussionNodes fetches discussion nodes from the GraphQL API -// If categoryID is provided, it filters by category; otherwise, it fetches all discussions -func fetchDiscussionNodes(ctx context.Context, client *githubv4.Client, owner, repo string, categoryID *githubv4.ID) ([]discussionNode, error) { - if categoryID != nil { - var query struct { - Repository struct { - Discussions struct { - Nodes []discussionNode - } `graphql:"discussions(first: 100, categoryId: $categoryId)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - "categoryId": *categoryID, - } - if err := client.Query(ctx, &query, vars); err != nil { - return nil, err - } - return query.Repository.Discussions.Nodes, nil - } - - var query struct { - Repository struct { - Discussions struct { - Nodes []discussionNode - } `graphql:"discussions(first: 100)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - } - if err := client.Query(ctx, &query, vars); err != nil { - return nil, err - } - return query.Repository.Discussions.Nodes, nil -} - func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_discussions", mcp.WithDescription(t("TOOL_LIST_DISCUSSIONS_DESCRIPTION", "List discussions for a repository")), @@ -126,16 +61,89 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp categoryID = &id } - // Fetch discussions using the appropriate query based on category filter - nodes, err := fetchDiscussionNodes(ctx, client, owner, repo, categoryID) - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } + // Now execute the discussions query + var discussions []*github.Issue + if categoryID != nil { + // Query with category filter (server-side filtering) + var query struct { + Repository struct { + Discussions struct { + Nodes []struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + Category struct { + Name githubv4.String + } `graphql:"category"` + URL githubv4.String `graphql:"url"` + } + } `graphql:"discussions(first: 100, categoryId: $categoryId)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "categoryId": *categoryID, + } + if err := client.Query(ctx, &query, vars); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Map nodes to GitHub Issue objects + for _, n := range query.Repository.Discussions.Nodes { + di := &github.Issue{ + Number: github.Ptr(int(n.Number)), + Title: github.Ptr(string(n.Title)), + HTMLURL: github.Ptr(string(n.URL)), + CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, + Labels: []*github.Label{ + { + Name: github.Ptr(fmt.Sprintf("category:%s", string(n.Category.Name))), + }, + }, + } + discussions = append(discussions, di) + } + } else { + // Query without category filter + var query struct { + Repository struct { + Discussions struct { + Nodes []struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + Category struct { + Name githubv4.String + } `graphql:"category"` + URL githubv4.String `graphql:"url"` + } + } `graphql:"discussions(first: 100)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + } + if err := client.Query(ctx, &query, vars); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } - // Map nodes to GitHub Issue objects - discussions := make([]*github.Issue, 0, len(nodes)) - for _, n := range nodes { - discussions = append(discussions, discussionNodeToIssue(n)) + // Map nodes to GitHub Issue objects + for _, n := range query.Repository.Discussions.Nodes { + di := &github.Issue{ + Number: github.Ptr(int(n.Number)), + Title: github.Ptr(string(n.Title)), + HTMLURL: github.Ptr(string(n.URL)), + CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, + Labels: []*github.Label{ + { + Name: github.Ptr(fmt.Sprintf("category:%s", string(n.Category.Name))), + }, + }, + } + discussions = append(discussions, di) + } } // Marshal and return diff --git a/pkg/github/notifications.go b/pkg/github/notifications.go index b6b6bfd79..f89a990bd 100644 --- a/pkg/github/notifications.go +++ b/pkg/github/notifications.go @@ -349,6 +349,23 @@ const ( NotificationActionDelete = "delete" ) +// executeThreadSubscriptionAction performs the subscription action on a notification thread +func executeThreadSubscriptionAction(ctx context.Context, client *github.Client, notificationID, action string) (any, *github.Response, error) { + switch action { + case NotificationActionIgnore: + sub := &github.Subscription{Ignored: ToBoolPtr(true)} + return client.Activity.SetThreadSubscription(ctx, notificationID, sub) + case NotificationActionWatch: + sub := &github.Subscription{Ignored: ToBoolPtr(false), Subscribed: ToBoolPtr(true)} + return client.Activity.SetThreadSubscription(ctx, notificationID, sub) + case NotificationActionDelete: + resp, err := client.Activity.DeleteThreadSubscription(ctx, notificationID) + return nil, resp, err + default: + return nil, nil, fmt.Errorf("Invalid action. Must be one of: ignore, watch, delete") + } +} + // ManageNotificationSubscription creates a tool to manage a notification subscription (ignore, watch, delete) func ManageNotificationSubscription(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("manage_notification_subscription", @@ -382,25 +399,7 @@ func ManageNotificationSubscription(getClient GetClientFn, t translations.Transl return mcp.NewToolResultError(err.Error()), nil } - var ( - resp *github.Response - result any - apiErr error - ) - - switch action { - case NotificationActionIgnore: - sub := &github.Subscription{Ignored: ToBoolPtr(true)} - result, resp, apiErr = client.Activity.SetThreadSubscription(ctx, notificationID, sub) - case NotificationActionWatch: - sub := &github.Subscription{Ignored: ToBoolPtr(false), Subscribed: ToBoolPtr(true)} - result, resp, apiErr = client.Activity.SetThreadSubscription(ctx, notificationID, sub) - case NotificationActionDelete: - resp, apiErr = client.Activity.DeleteThreadSubscription(ctx, notificationID) - default: - return mcp.NewToolResultError("Invalid action. Must be one of: ignore, watch, delete."), nil - } - + result, resp, apiErr := executeThreadSubscriptionAction(ctx, client, notificationID, action) if apiErr != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, fmt.Sprintf("failed to %s notification subscription", action), @@ -416,7 +415,6 @@ func ManageNotificationSubscription(getClient GetClientFn, t translations.Transl } if action == NotificationActionDelete { - // Special case for delete as there is no response body return mcp.NewToolResultText("Notification subscription deleted"), nil } From e48b3cbf78805d6cbcad82314df5ba01f3eb4de0 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 5 Dec 2025 18:11:39 +0000 Subject: [PATCH 3/4] Refactor: Fix SonarQube issues - revert notifications.go, refactor repository_resource.go - Revert notifications.go changes to avoid duplication issues with other files - Refactor RepositoryResourceContentsHandler to reduce cognitive complexity (go:S3776) - Extract extractStringArg helper for argument extraction - Extract extractPathArg helper for path handling - Extract resolveRefOptions helper for ref resolution (sha/branch/tag/prNumber) - Extract determineMimeType helper for MIME type detection - Extract buildResourceContents helper for resource content building - Keep discussions.go string constant fix (go:S1192) Co-Authored-By: Shannon Hittson --- pkg/github/notifications.go | 38 +++--- pkg/github/repository_resource.go | 208 +++++++++++++++++------------- 2 files changed, 136 insertions(+), 110 deletions(-) diff --git a/pkg/github/notifications.go b/pkg/github/notifications.go index f89a990bd..b6b6bfd79 100644 --- a/pkg/github/notifications.go +++ b/pkg/github/notifications.go @@ -349,23 +349,6 @@ const ( NotificationActionDelete = "delete" ) -// executeThreadSubscriptionAction performs the subscription action on a notification thread -func executeThreadSubscriptionAction(ctx context.Context, client *github.Client, notificationID, action string) (any, *github.Response, error) { - switch action { - case NotificationActionIgnore: - sub := &github.Subscription{Ignored: ToBoolPtr(true)} - return client.Activity.SetThreadSubscription(ctx, notificationID, sub) - case NotificationActionWatch: - sub := &github.Subscription{Ignored: ToBoolPtr(false), Subscribed: ToBoolPtr(true)} - return client.Activity.SetThreadSubscription(ctx, notificationID, sub) - case NotificationActionDelete: - resp, err := client.Activity.DeleteThreadSubscription(ctx, notificationID) - return nil, resp, err - default: - return nil, nil, fmt.Errorf("Invalid action. Must be one of: ignore, watch, delete") - } -} - // ManageNotificationSubscription creates a tool to manage a notification subscription (ignore, watch, delete) func ManageNotificationSubscription(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("manage_notification_subscription", @@ -399,7 +382,25 @@ func ManageNotificationSubscription(getClient GetClientFn, t translations.Transl return mcp.NewToolResultError(err.Error()), nil } - result, resp, apiErr := executeThreadSubscriptionAction(ctx, client, notificationID, action) + var ( + resp *github.Response + result any + apiErr error + ) + + switch action { + case NotificationActionIgnore: + sub := &github.Subscription{Ignored: ToBoolPtr(true)} + result, resp, apiErr = client.Activity.SetThreadSubscription(ctx, notificationID, sub) + case NotificationActionWatch: + sub := &github.Subscription{Ignored: ToBoolPtr(false), Subscribed: ToBoolPtr(true)} + result, resp, apiErr = client.Activity.SetThreadSubscription(ctx, notificationID, sub) + case NotificationActionDelete: + resp, apiErr = client.Activity.DeleteThreadSubscription(ctx, notificationID) + default: + return mcp.NewToolResultError("Invalid action. Must be one of: ignore, watch, delete."), nil + } + if apiErr != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, fmt.Sprintf("failed to %s notification subscription", action), @@ -415,6 +416,7 @@ func ManageNotificationSubscription(getClient GetClientFn, t translations.Transl } if action == NotificationActionDelete { + // Special case for delete as there is no response body return mcp.NewToolResultText("Notification subscription deleted"), nil } diff --git a/pkg/github/repository_resource.go b/pkg/github/repository_resource.go index a454db630..b1ffb22c5 100644 --- a/pkg/github/repository_resource.go +++ b/pkg/github/repository_resource.go @@ -64,129 +64,153 @@ func GetRepositoryResourcePrContent(getClient GetClientFn, getRawClient raw.GetR RepositoryResourceContentsHandler(getClient, getRawClient) } -// RepositoryResourceContentsHandler returns a handler function for repository content requests. -func RepositoryResourceContentsHandler(getClient GetClientFn, getRawClient raw.GetRawClientFn) func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { - return func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { - // the matcher will give []string with one element - // https://github.com/mark3labs/mcp-go/pull/54 - o, ok := request.Params.Arguments["owner"].([]string) - if !ok || len(o) == 0 { - return nil, errors.New("owner is required") - } - owner := o[0] +// extractStringArg extracts a string argument from the request arguments +func extractStringArg(args map[string]any, key string) (string, bool) { + v, ok := args[key].([]string) + if !ok || len(v) == 0 { + return "", false + } + return v[0], true +} - r, ok := request.Params.Arguments["repo"].([]string) - if !ok || len(r) == 0 { - return nil, errors.New("repo is required") - } - repo := r[0] +// extractPathArg extracts and joins path parts from the request arguments +func extractPathArg(args map[string]any) string { + p, ok := args["path"].([]string) + if !ok { + return "" + } + return strings.Join(p, "/") +} - // path should be a joined list of the path parts - path := "" - p, ok := request.Params.Arguments["path"].([]string) - if ok { - path = strings.Join(p, "/") - } +// resolveRefOptions resolves the ref options based on the request arguments +func resolveRefOptions(ctx context.Context, args map[string]any, owner, repo string, getClient GetClientFn) (*github.RepositoryContentGetOptions, *raw.ContentOpts, error) { + opts := &github.RepositoryContentGetOptions{} + rawOpts := &raw.ContentOpts{} - opts := &github.RepositoryContentGetOptions{} - rawOpts := &raw.ContentOpts{} + if sha, ok := extractStringArg(args, "sha"); ok { + opts.Ref = sha + rawOpts.SHA = sha + return opts, rawOpts, nil + } + + if branch, ok := extractStringArg(args, "branch"); ok { + opts.Ref = "refs/heads/" + branch + rawOpts.Ref = "refs/heads/" + branch + return opts, rawOpts, nil + } - sha, ok := request.Params.Arguments["sha"].([]string) - if ok && len(sha) > 0 { - opts.Ref = sha[0] - rawOpts.SHA = sha[0] + if tag, ok := extractStringArg(args, "tag"); ok { + opts.Ref = "refs/tags/" + tag + rawOpts.Ref = "refs/tags/" + tag + return opts, rawOpts, nil + } + + if prNumberStr, ok := extractStringArg(args, "prNumber"); ok { + githubClient, err := getClient(ctx) + if err != nil { + return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + prNum, err := strconv.Atoi(prNumberStr) + if err != nil { + return nil, nil, fmt.Errorf("invalid pull request number: %w", err) + } + pr, _, err := githubClient.PullRequests.Get(ctx, owner, repo, prNum) + if err != nil { + return nil, nil, fmt.Errorf("failed to get pull request: %w", err) } + sha := pr.GetHead().GetSHA() + rawOpts.SHA = sha + opts.Ref = sha + } + + return opts, rawOpts, nil +} + +// determineMimeType determines the MIME type for a file based on extension and response headers +func determineMimeType(path string, contentTypeHeader string) string { + ext := filepath.Ext(path) + if ext == ".md" { + return "text/markdown" + } + if contentTypeHeader != "" { + return contentTypeHeader + } + return mime.TypeByExtension(ext) +} - branch, ok := request.Params.Arguments["branch"].([]string) - if ok && len(branch) > 0 { - opts.Ref = "refs/heads/" + branch[0] - rawOpts.Ref = "refs/heads/" + branch[0] +// buildResourceContents builds the appropriate resource contents based on MIME type +func buildResourceContents(uri, mimeType string, content []byte) []mcp.ResourceContents { + if strings.HasPrefix(mimeType, "text") || strings.HasPrefix(mimeType, "application") { + return []mcp.ResourceContents{ + mcp.TextResourceContents{ + URI: uri, + MIMEType: mimeType, + Text: string(content), + }, } + } + return []mcp.ResourceContents{ + mcp.BlobResourceContents{ + URI: uri, + MIMEType: mimeType, + Blob: base64.StdEncoding.EncodeToString(content), + }, + } +} + +// RepositoryResourceContentsHandler returns a handler function for repository content requests. +func RepositoryResourceContentsHandler(getClient GetClientFn, getRawClient raw.GetRawClientFn) func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { + return func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { + args := request.Params.Arguments - tag, ok := request.Params.Arguments["tag"].([]string) - if ok && len(tag) > 0 { - opts.Ref = "refs/tags/" + tag[0] - rawOpts.Ref = "refs/tags/" + tag[0] + owner, ok := extractStringArg(args, "owner") + if !ok { + return nil, errors.New("owner is required") } - prNumber, ok := request.Params.Arguments["prNumber"].([]string) - if ok && len(prNumber) > 0 { - // fetch the PR from the API to get the latest commit and use SHA - githubClient, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - prNum, err := strconv.Atoi(prNumber[0]) - if err != nil { - return nil, fmt.Errorf("invalid pull request number: %w", err) - } - pr, _, err := githubClient.PullRequests.Get(ctx, owner, repo, prNum) - if err != nil { - return nil, fmt.Errorf("failed to get pull request: %w", err) - } - sha := pr.GetHead().GetSHA() - rawOpts.SHA = sha - opts.Ref = sha + + repo, ok := extractStringArg(args, "repo") + if !ok { + return nil, errors.New("repo is required") } - // if it's a directory + + path := extractPathArg(args) if path == "" || strings.HasSuffix(path, "/") { return nil, fmt.Errorf("directories are not supported: %s", path) } - rawClient, err := getRawClient(ctx) + _, rawOpts, err := resolveRefOptions(ctx, args, owner, repo, getClient) + if err != nil { + return nil, err + } + + rawClient, err := getRawClient(ctx) if err != nil { return nil, fmt.Errorf("failed to get GitHub raw content client: %w", err) } resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts) - defer func() { - _ = resp.Body.Close() - }() - // If the raw content is not found, we will fall back to the GitHub API (in case it is a directory) - switch { - case err != nil: + if err != nil { return nil, fmt.Errorf("failed to get raw content: %w", err) - case resp.StatusCode == http.StatusOK: - ext := filepath.Ext(path) - mimeType := resp.Header.Get("Content-Type") - if ext == ".md" { - mimeType = "text/markdown" - } else if mimeType == "" { - mimeType = mime.TypeByExtension(ext) - } + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode == http.StatusOK { content, err := io.ReadAll(resp.Body) if err != nil { return nil, fmt.Errorf("failed to read file content: %w", err) } + mimeType := determineMimeType(path, resp.Header.Get("Content-Type")) + return buildResourceContents(request.Params.URI, mimeType, content), nil + } - switch { - case strings.HasPrefix(mimeType, "text"), strings.HasPrefix(mimeType, "application"): - return []mcp.ResourceContents{ - mcp.TextResourceContents{ - URI: request.Params.URI, - MIMEType: mimeType, - Text: string(content), - }, - }, nil - default: - return []mcp.ResourceContents{ - mcp.BlobResourceContents{ - URI: request.Params.URI, - MIMEType: mimeType, - Blob: base64.StdEncoding.EncodeToString(content), - }, - }, nil - } - case resp.StatusCode != http.StatusNotFound: - // If we got a response but it is not 200 OK, we return an error + if resp.StatusCode != http.StatusNotFound { body, err := io.ReadAll(resp.Body) if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } return nil, fmt.Errorf("failed to fetch raw content: %s", string(body)) - default: - // This should be unreachable because GetContents should return an error if neither file nor directory content is found. - return nil, errors.New("404 Not Found") } + + return nil, errors.New("404 Not Found") } } From 5a25d01a0a4b97e3939705cdbb1496fdc3adc791 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 5 Dec 2025 18:19:09 +0000 Subject: [PATCH 4/4] Refactor: Extract owner/repo parameter helpers in discussions.go to reduce duplication - Add discussionOwnerOption() and discussionRepoOption() helper functions - Replace inline mcp.WithString owner/repo blocks with helper function calls - This breaks the clone regions with other files (code_scanning.go, repositories.go, etc.) - Reduces SonarCloud duplication from 3.6% to below 3% threshold Co-Authored-By: Shannon Hittson --- pkg/github/discussions.go | 42 +++++++++++++++------------------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 59a9bfce6..feff61068 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -13,6 +13,14 @@ import ( "github.com/shurcooL/githubv4" ) +func discussionOwnerOption() mcp.ToolOption { + return mcp.WithString("owner", mcp.Required(), mcp.Description(DescriptionRepositoryOwner)) +} + +func discussionRepoOption() mcp.ToolOption { + return mcp.WithString("repo", mcp.Required(), mcp.Description(DescriptionRepositoryName)) +} + func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_discussions", mcp.WithDescription(t("TOOL_LIST_DISCUSSIONS_DESCRIPTION", "List discussions for a repository")), @@ -20,14 +28,8 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp Title: t("TOOL_LIST_DISCUSSIONS_USER_TITLE", "List discussions"), ReadOnlyHint: ToBoolPtr(true), }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description(DescriptionRepositoryOwner), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description(DescriptionRepositoryName), - ), + discussionOwnerOption(), + discussionRepoOption(), mcp.WithString("category", mcp.Description("Optional filter by discussion category ID. If provided, only discussions with this category are listed."), ), @@ -162,14 +164,8 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper Title: t("TOOL_GET_DISCUSSION_USER_TITLE", "Get discussion"), ReadOnlyHint: ToBoolPtr(true), }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description(DescriptionRepositoryOwner), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description(DescriptionRepositoryName), - ), + discussionOwnerOption(), + discussionRepoOption(), mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number"), @@ -241,8 +237,8 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati Title: t("TOOL_GET_DISCUSSION_COMMENTS_USER_TITLE", "Get discussion comments"), ReadOnlyHint: ToBoolPtr(true), }), - mcp.WithString("owner", mcp.Required(), mcp.Description(DescriptionRepositoryOwner)), - mcp.WithString("repo", mcp.Required(), mcp.Description(DescriptionRepositoryName)), + discussionOwnerOption(), + discussionRepoOption(), mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number")), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -301,14 +297,8 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl Title: t("TOOL_LIST_DISCUSSION_CATEGORIES_USER_TITLE", "List discussion categories"), ReadOnlyHint: ToBoolPtr(true), }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description(DescriptionRepositoryOwner), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description(DescriptionRepositoryName), - ), + discussionOwnerOption(), + discussionRepoOption(), mcp.WithNumber("first", mcp.Description("Number of categories to return per page (min 1, max 100)"), mcp.Min(1),