fix(security): remediate SonarQube vulnerabilities in discussions.go#89
Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Open
Conversation
…ons.go - S1192: Extract duplicated string literals into package-level constants (descRepoOwner, descRepoName, errGQLClientFmt, categoryFmt) - S3776: Reduce cognitive complexity of ListDiscussions (27->15) by extracting queryDiscussionsWithCategory and queryDiscussionsAll helpers - S3776: Reduce cognitive complexity of ListDiscussionCategories (22->12) by extracting validatePaginationParams helper - Extract shared discussionNode type and mapDiscussionNodesToIssues helper Co-Authored-By: Ian Moritz <ian.moritz@cognition.ai>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Closes: N/A — Addresses 6 SonarQube CRITICAL findings in
pkg/github/discussions.go.Summary
Remediates 6 SonarQube issues (4× S1192 duplicated string literals, 2× S3776 cognitive complexity) without changing any external behavior. All existing tests pass unmodified.
S1192 — Duplicated string literals → package-level constants
descRepoOwner"Repository owner"descRepoName"Repository name"errGQLClientFmt"failed to get GitHub GQL client: %v"categoryFmt"category:%s"S3776 — Cognitive complexity → extracted helpers
ListDiscussions(27 → 15): The two nearly-identical inline GraphQL query branches and their node-mapping loops were extracted into:discussionNode— shared struct type for GraphQL response nodesmapDiscussionNodesToIssues()— converts nodes to[]*github.IssuequeryDiscussionsWithCategory()/queryDiscussionsAll()— execute the filtered/unfiltered query respectivelyListDiscussionCategories(22 → 12): The four pagination-validation if-blocks were extracted intovalidatePaginationParams().Tradeoffs
discussionNodetype that replaces the anonymous structs previously inlined in each GraphQL query. Thegithubv4library uses reflection on struct field names and tags to build the query string, so named vs anonymous should produce identical queries — confirmed by all mock-based tests passing.validatePaginationParamsreturns a plainerrorinstead of directly constructingmcp.NewToolResultError. The caller wraps it via.Error(), which is functionally equivalent since the original messages had no format verbs.Human review checklist
discussionNodetype in GraphQL query structs (instead of anonymous structs) does not change the generated GraphQL query shape. Tests pass, but worth a sanity check.validatePaginationParamserror messages match the originals exactly (they do — same string literals, just moved intofmt.Errorf).categoryIDintermediate variable and nil-check were collapsed intocategory != "". Confirm this is logically equivalent (it is, sincecategoryIDwas only set whencategory != "").Link to Devin run: https://app.devin.ai/sessions/e78b576d8aad41b5b653f2e1a1af06ef
Requested by: @iancmoritz