From c5018cfd89ace829b0db55a81ef35ef656283895 Mon Sep 17 00:00:00 2001 From: Shawn Azman Date: Fri, 6 Feb 2026 16:18:14 +0000 Subject: [PATCH 1/3] SonarQube: fix Go S1192 duplicated strings; reduce cognitive complexity in ListDiscussionCategories Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- pkg/github/discussions.go | 66 ++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index a7ec8e20f..432c47f70 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -13,6 +13,13 @@ import ( "github.com/shurcooL/githubv4" ) +const ( + descRepoOwner = "Repository owner" + descRepoName = "Repository name" + errGQLClientFmt = "failed to get GitHub GQL client: %v" + categoryLabelFmt = "category:%s" +) + 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 +29,11 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(descRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(descRepoName), ), mcp.WithString("category", mcp.Description("Optional filter by discussion category ID. If provided, only discussions with this category are listed."), @@ -51,7 +58,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp client, err := getGQLClient(ctx) if err != nil { - return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil + return mcp.NewToolResultError(fmt.Sprintf(errGQLClientFmt, err)), nil } // If category filter is specified, use it as the category ID for server-side filtering @@ -98,7 +105,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, Labels: []*github.Label{ { - Name: github.Ptr(fmt.Sprintf("category:%s", string(n.Category.Name))), + Name: github.Ptr(fmt.Sprintf(categoryLabelFmt, string(n.Category.Name))), }, }, } @@ -138,7 +145,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, Labels: []*github.Label{ { - Name: github.Ptr(fmt.Sprintf("category:%s", string(n.Category.Name))), + Name: github.Ptr(fmt.Sprintf(categoryLabelFmt, string(n.Category.Name))), }, }, } @@ -164,11 +171,11 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(descRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(descRepoName), ), mcp.WithNumber("discussionNumber", mcp.Required(), @@ -187,7 +194,7 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper } client, err := getGQLClient(ctx) if err != nil { - return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil + return mcp.NewToolResultError(fmt.Sprintf(errGQLClientFmt, err)), nil } var q struct { @@ -221,7 +228,7 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper CreatedAt: &github.Timestamp{Time: d.CreatedAt.Time}, Labels: []*github.Label{ { - Name: github.Ptr(fmt.Sprintf("category:%s", string(d.Category.Name))), + Name: github.Ptr(fmt.Sprintf(categoryLabelFmt, string(d.Category.Name))), }, }, } @@ -241,8 +248,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(descRepoOwner)), + mcp.WithString("repo", mcp.Required(), mcp.Description(descRepoName)), mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number")), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -258,7 +265,7 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati client, err := getGQLClient(ctx) if err != nil { - return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil + return mcp.NewToolResultError(fmt.Sprintf(errGQLClientFmt, err)), nil } var q struct { @@ -303,11 +310,11 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(descRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(descRepoName), ), mcp.WithNumber("first", mcp.Description("Number of categories to return per page (min 1, max 100)"), @@ -341,22 +348,13 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl } // Validate pagination parameters - if params.First != 0 && params.Last != 0 { - return mcp.NewToolResultError("only one of 'first' or 'last' may be specified"), nil - } - if params.After != "" && params.Before != "" { - return mcp.NewToolResultError("only one of 'after' or 'before' may be specified"), nil - } - if params.After != "" && params.Last != 0 { - return mcp.NewToolResultError("'after' cannot be used with 'last'. Did you mean to use 'before' instead?"), nil - } - if params.Before != "" && params.First != 0 { - return mcp.NewToolResultError("'before' cannot be used with 'first'. Did you mean to use 'after' instead?"), nil + if err := validatePaginationParams(params.First, params.Last, params.After, params.Before); err != nil { + return mcp.NewToolResultError(err.Error()), nil } client, err := getGQLClient(ctx) if err != nil { - return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil + return mcp.NewToolResultError(fmt.Sprintf(errGQLClientFmt, err)), nil } var q struct { Repository struct { @@ -389,3 +387,19 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl return mcp.NewToolResultText(string(out)), nil } } + +func validatePaginationParams(first, last int32, after, before string) error { + if first != 0 && last != 0 { + return fmt.Errorf("only one of 'first' or 'last' may be specified") + } + if after != "" && before != "" { + return fmt.Errorf("only one of 'after' or 'before' may be specified") + } + if after != "" && last != 0 { + return fmt.Errorf("'after' cannot be used with 'last'. Did you mean to use 'before' instead?") + } + if before != "" && first != 0 { + return fmt.Errorf("'before' cannot be used with 'first'. Did you mean to use 'after' instead?") + } + return nil +} From b7c3e12c271865cd562603a2c6e7c600813d9c97 Mon Sep 17 00:00:00 2001 From: Shawn Azman Date: Fri, 6 Feb 2026 16:29:51 +0000 Subject: [PATCH 2/3] Refactor ListDiscussions mapping to reduce duplication (Sonar new-code duplication) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- pkg/github/discussions.go | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 432c47f70..e0265c2e4 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -98,17 +98,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp // 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(categoryLabelFmt, string(n.Category.Name))), - }, - }, - } + di := newIssueFromNode(n.Number, n.Title, n.CreatedAt, n.Category.Name, n.URL) discussions = append(discussions, di) } } else { @@ -138,17 +128,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp // 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(categoryLabelFmt, string(n.Category.Name))), - }, - }, - } + di := newIssueFromNode(n.Number, n.Title, n.CreatedAt, n.Category.Name, n.URL) discussions = append(discussions, di) } } @@ -403,3 +383,17 @@ func validatePaginationParams(first, last int32, after, before string) error { } return nil } + +func newIssueFromNode(number githubv4.Int, title githubv4.String, createdAt githubv4.DateTime, categoryName githubv4.String, url githubv4.String) *github.Issue { + return &github.Issue{ + Number: github.Ptr(int(number)), + Title: github.Ptr(string(title)), + HTMLURL: github.Ptr(string(url)), + CreatedAt: &github.Timestamp{Time: createdAt.Time}, + Labels: []*github.Label{ + { + Name: github.Ptr(fmt.Sprintf(categoryLabelFmt, string(categoryName))), + }, + }, + } +} From 9185b143a5ac67d6064a34e630a8fad81a3683a9 Mon Sep 17 00:00:00 2001 From: Shawn Azman Date: Fri, 6 Feb 2026 16:37:44 +0000 Subject: [PATCH 3/3] Refactor ListDiscussions: unify query nodes and single mapping loop to cut new-code duplication Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- pkg/github/discussions.go | 46 +++++++++++++++------------------------ 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index e0265c2e4..b6a29db25 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -20,6 +20,14 @@ const ( categoryLabelFmt = "category:%s" ) +type discussionNode struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + Category struct{ Name githubv4.String } `graphql:"category"` + URL githubv4.String `graphql:"url"` +} + 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")), @@ -70,20 +78,13 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp // Now execute the discussions query var discussions []*github.Issue + var nodes []discussionNode 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"` - } + Nodes []discussionNode } `graphql:"discussions(first: 100, categoryId: $categoryId)"` } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -96,25 +97,13 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp return mcp.NewToolResultError(err.Error()), nil } - // Map nodes to GitHub Issue objects - for _, n := range query.Repository.Discussions.Nodes { - di := newIssueFromNode(n.Number, n.Title, n.CreatedAt, n.Category.Name, n.URL) - discussions = append(discussions, di) - } + nodes = query.Repository.Discussions.Nodes } 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"` - } + Nodes []discussionNode } `graphql:"discussions(first: 100)"` } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -126,11 +115,12 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp return mcp.NewToolResultError(err.Error()), nil } - // Map nodes to GitHub Issue objects - for _, n := range query.Repository.Discussions.Nodes { - di := newIssueFromNode(n.Number, n.Title, n.CreatedAt, n.Category.Name, n.URL) - discussions = append(discussions, di) - } + nodes = query.Repository.Discussions.Nodes + } + + // Map nodes to GitHub Issues + for _, n := range nodes { + discussions = append(discussions, newIssueFromNode(n.Number, n.Title, n.CreatedAt, n.Category.Name, n.URL)) } // Marshal and return