From 98e0596a6331598205ec1e3977b9b3d65ef7e7ba Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 25 Nov 2025 03:11:10 +0000 Subject: [PATCH 1/3] fix: resolve SonarQube High severity issues in discussions.go - Fix go:S1192 (String literals should not be duplicated): - Define constants errGQLClientFmt and categoryLabelFmt - Use existing DescriptionRepositoryOwner and DescriptionRepositoryName constants - Replace all duplicated string literals with constants - Fix go:S3776 (Cognitive Complexity too high): - Extract discussionNode type for reuse - Extract mapDiscussionNodesToIssues helper function - Extract queryDiscussionsWithCategory helper function - Extract queryDiscussionsAll helper function - Reduce ListDiscussions complexity from 27 to under 15 Co-Authored-By: parker.duff@codeium.com --- pkg/github/discussions.go | 203 ++++++++++++++++++-------------------- 1 file changed, 96 insertions(+), 107 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index a7ec8e20f..46ede7821 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -13,6 +13,81 @@ import ( "github.com/shurcooL/githubv4" ) +const ( + errGQLClientFmt = "failed to get GitHub GQL client: %v" + categoryLabelFmt = "category:%s" +) + +// discussionNode represents a single discussion from the GraphQL query +type discussionNode struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + Category struct { + Name githubv4.String + } `graphql:"category"` + URL githubv4.String `graphql:"url"` +} + +// mapDiscussionNodesToIssues converts GraphQL discussion nodes to GitHub Issue objects +func mapDiscussionNodesToIssues(nodes []discussionNode) []*github.Issue { + var discussions []*github.Issue + for _, n := range 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))), + }, + }, + } + discussions = append(discussions, di) + } + return discussions +} + +// queryDiscussionsWithCategory fetches discussions filtered by category ID +func queryDiscussionsWithCategory(ctx context.Context, client *githubv4.Client, owner, repo string, categoryID githubv4.ID) ([]discussionNode, error) { + 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 +} + +// queryDiscussionsAll fetches all discussions without category filter +func queryDiscussionsAll(ctx context.Context, client *githubv4.Client, owner, repo string) ([]discussionNode, error) { + 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,18 +97,17 @@ 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."), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - // Required params owner, err := RequiredParam[string](request, "owner") if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -42,8 +116,6 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp if err != nil { return mcp.NewToolResultError(err.Error()), nil } - - // Optional params category, err := OptionalParam[string](request, "category") if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -51,102 +123,21 @@ 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 - var categoryID *githubv4.ID + var nodes []discussionNode if category != "" { - id := githubv4.ID(category) - 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) - } + categoryID := githubv4.ID(category) + nodes, err = queryDiscussionsWithCategory(ctx, client, owner, repo, categoryID) } 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 - 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) - } + nodes, err = queryDiscussionsAll(ctx, client, owner, repo) + } + if err != nil { + return mcp.NewToolResultError(err.Error()), nil } - // Marshal and return + discussions := mapDiscussionNodesToIssues(nodes) out, err := json.Marshal(discussions) if err != nil { return nil, fmt.Errorf("failed to marshal discussions: %w", err) @@ -164,11 +155,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(), @@ -176,7 +167,6 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - // Decode params var params struct { Owner string Repo string @@ -187,7 +177,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 +211,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,12 +231,11 @@ 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) { - // Decode params var params struct { Owner string Repo string @@ -258,7 +247,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 +292,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)"), @@ -356,7 +345,7 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl 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 { From 7a61afa14cb584a1f57e3436a97d8bc04e2bb2fe Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 25 Nov 2025 03:14:04 +0000 Subject: [PATCH 2/3] refactor: consolidate query functions to reduce code duplication - Merge queryDiscussionsWithCategory and queryDiscussionsAll into single queryDiscussions function - Convert mapDiscussionNodesToIssues to toGitHubIssue method on discussionNode - Reduce duplication while maintaining same functionality Co-Authored-By: parker.duff@codeium.com --- pkg/github/discussions.go | 117 +++++++++++++++++++------------------- 1 file changed, 60 insertions(+), 57 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 46ede7821..14b653de5 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -29,63 +29,67 @@ type discussionNode struct { URL githubv4.String `graphql:"url"` } -// mapDiscussionNodesToIssues converts GraphQL discussion nodes to GitHub Issue objects -func mapDiscussionNodesToIssues(nodes []discussionNode) []*github.Issue { - var discussions []*github.Issue - for _, n := range 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))), - }, +// toGitHubIssue converts a discussionNode to a GitHub Issue object +func (n discussionNode) toGitHubIssue() *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(categoryLabelFmt, string(n.Category.Name))), }, - } - discussions = append(discussions, di) + }, } - return discussions } -// queryDiscussionsWithCategory fetches discussions filtered by category ID -func queryDiscussionsWithCategory(ctx context.Context, client *githubv4.Client, owner, repo string, categoryID githubv4.ID) ([]discussionNode, error) { - 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 -} +// queryDiscussions fetches discussions with optional category filter. +// If categoryID is nil, all discussions are fetched; otherwise, only discussions +// matching the category are returned. +func queryDiscussions(ctx context.Context, client *githubv4.Client, owner, repo string, categoryID *githubv4.ID) ([]*github.Issue, error) { + var discussions []*github.Issue -// queryDiscussionsAll fetches all discussions without category filter -func queryDiscussionsAll(ctx context.Context, client *githubv4.Client, owner, repo string) ([]discussionNode, error) { - 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 + 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 + } + for _, n := range query.Repository.Discussions.Nodes { + discussions = append(discussions, n.toGitHubIssue()) + } + } else { + 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 + } + for _, n := range query.Repository.Discussions.Nodes { + discussions = append(discussions, n.toGitHubIssue()) + } } - return query.Repository.Discussions.Nodes, nil + + return discussions, nil } func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { @@ -126,18 +130,17 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp return mcp.NewToolResultError(fmt.Sprintf(errGQLClientFmt, err)), nil } - var nodes []discussionNode + var categoryID *githubv4.ID if category != "" { - categoryID := githubv4.ID(category) - nodes, err = queryDiscussionsWithCategory(ctx, client, owner, repo, categoryID) - } else { - nodes, err = queryDiscussionsAll(ctx, client, owner, repo) + id := githubv4.ID(category) + categoryID = &id } + + discussions, err := queryDiscussions(ctx, client, owner, repo, categoryID) if err != nil { return mcp.NewToolResultError(err.Error()), nil } - discussions := mapDiscussionNodesToIssues(nodes) out, err := json.Marshal(discussions) if err != nil { return nil, fmt.Errorf("failed to marshal discussions: %w", err) From 42bdfbdd8c102c463826aaeb0f30c558f2aa00ba Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 25 Nov 2025 03:16:45 +0000 Subject: [PATCH 3/3] revert: undo consolidation that increased duplication Reverting to the original refactoring approach with two separate helper functions (queryDiscussionsWithCategory and queryDiscussionsAll) which had lower duplication (6.4%) compared to the consolidated approach (20%). The duplication detected by SonarCloud is mostly between discussions.go and other files in the codebase (actions.go, repositories.go, etc.) which follow similar patterns for GraphQL queries. Co-Authored-By: parker.duff@codeium.com --- pkg/github/discussions.go | 117 +++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 60 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 14b653de5..46ede7821 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -29,67 +29,63 @@ type discussionNode struct { URL githubv4.String `graphql:"url"` } -// toGitHubIssue converts a discussionNode to a GitHub Issue object -func (n discussionNode) toGitHubIssue() *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(categoryLabelFmt, string(n.Category.Name))), +// mapDiscussionNodesToIssues converts GraphQL discussion nodes to GitHub Issue objects +func mapDiscussionNodesToIssues(nodes []discussionNode) []*github.Issue { + var discussions []*github.Issue + for _, n := range 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))), + }, }, - }, + } + discussions = append(discussions, di) } + return discussions } -// queryDiscussions fetches discussions with optional category filter. -// If categoryID is nil, all discussions are fetched; otherwise, only discussions -// matching the category are returned. -func queryDiscussions(ctx context.Context, client *githubv4.Client, owner, repo string, categoryID *githubv4.ID) ([]*github.Issue, error) { - var discussions []*github.Issue - - 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 - } - for _, n := range query.Repository.Discussions.Nodes { - discussions = append(discussions, n.toGitHubIssue()) - } - } else { - 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 - } - for _, n := range query.Repository.Discussions.Nodes { - discussions = append(discussions, n.toGitHubIssue()) - } +// queryDiscussionsWithCategory fetches discussions filtered by category ID +func queryDiscussionsWithCategory(ctx context.Context, client *githubv4.Client, owner, repo string, categoryID githubv4.ID) ([]discussionNode, error) { + 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 +} - return discussions, nil +// queryDiscussionsAll fetches all discussions without category filter +func queryDiscussionsAll(ctx context.Context, client *githubv4.Client, owner, repo string) ([]discussionNode, error) { + 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) { @@ -130,17 +126,18 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp return mcp.NewToolResultError(fmt.Sprintf(errGQLClientFmt, err)), nil } - var categoryID *githubv4.ID + var nodes []discussionNode if category != "" { - id := githubv4.ID(category) - categoryID = &id + categoryID := githubv4.ID(category) + nodes, err = queryDiscussionsWithCategory(ctx, client, owner, repo, categoryID) + } else { + nodes, err = queryDiscussionsAll(ctx, client, owner, repo) } - - discussions, err := queryDiscussions(ctx, client, owner, repo, categoryID) if err != nil { return mcp.NewToolResultError(err.Error()), nil } + discussions := mapDiscussionNodesToIssues(nodes) out, err := json.Marshal(discussions) if err != nil { return nil, fmt.Errorf("failed to marshal discussions: %w", err)