Skip to content

Conversation

@devin-ai-integration
Copy link

@devin-ai-integration devin-ai-integration bot commented Oct 24, 2025

Closes MBA-471

Summary

This PR addresses two High severity SonarQube violations identified for the github-mcp-server repository:

  1. String literal duplication (Go): Extracted the error message "context does not contain GitHubCtxErrors" (duplicated 4 times in pkg/errors/error.go) into a constant
  2. Cognitive complexity (Go): Refactored the get_file_contents handler in pkg/github/repositories.go by extracting helper functions to reduce nesting levels from 5+ to 2-3

SonarQube Rules Addressed:

  • String literals should not be duplicated
  • Cognitive Complexity of functions should not be too high

Changes

1. String Duplication Fix (pkg/errors/)

Created constants.go with three error message constants:

const (
    ErrContextMissingGitHubCtxErrors = "context does not contain GitHubCtxErrors"
    ErrFailedToGetGitHubClient       = "failed to get GitHub client"
    ErrMissingRequiredParameter      = "missing required parameter: %s"
)

Updated error.go to use the constant in 4 locations (lines 75, 83, 99, 107):

  • Changed from fmt.Errorf("context does not contain GitHubCtxErrors") to stderrors.New(ErrContextMissingGitHubCtxErrors)
  • Added import for standard errors package with alias to avoid naming conflict with package name
  • Fixes staticcheck lint issue SA1006 (printf-style function with dynamic format string)

2. Cognitive Complexity Fix (pkg/github/)

Extracted three helper functions in repositories.go:

  • tryRawContentFetch(): Handles raw content API call and error checking
  • buildResourceURI(): Builds resource URI based on sha/ref parameters using switch statement
  • createResourceContent(): Creates appropriate resource content (Text/Blob) based on content type

Refactored GetFileContents handler (lines 556-586):

  • Simplified deeply nested conditionals by delegating to helper functions
  • Reduced maximum nesting depth from 5+ levels to 2-3 levels
  • Maintains exact same functionality and error handling behavior

Testing

  • ✅ All existing tests pass (./script/test)
  • ✅ Lint passes with 0 issues (./script/lint)
  • ✅ Test assertions in error_test.go continue to pass (they check for the error message string which is preserved via constant)

Review Focus

Critical areas for review:

  1. Error semantics: Verify that changing from fmt.Errorf(string) to stderrors.New(string) preserves error handling behavior
  2. Helper function correctness: Confirm that the extracted helpers handle all edge cases from the original nested code
  3. Behavioral equivalence: Ensure refactored GetFileContents maintains exact same behavior as before
  4. Unused constants: Note that two constants (ErrFailedToGetGitHubClient, ErrMissingRequiredParameter) are defined but not used in this PR per scope limitation to "one issue from each category"

Tradeoffs

  • Partial constant usage: Defined three constants per requirements but only used one in this PR's scope. The other two are available for future use to address remaining string duplications across the codebase.
  • Package-private helpers: The three extracted helper functions are not exported (lowercase names) since they're only needed within the repositories package. If other packages need similar functionality in the future, these could be made public.

Notes

  • No CI checks are configured for this repository, so validation is based on local lint and test runs
  • SonarQube MCP Server verification was not possible due to technical issues, but changes align with the reported High severity violations

Link to Devin run: https://app.devin.ai/sessions/71aa3c9de3904275a1e6b836c17db8f8
Requested by: @jia-cog (Jia Wu)

…omplexity issues

- Extract error message constants to pkg/errors/constants.go
- Update pkg/errors/error.go to use ErrContextMissingGitHubCtxErrors constant
- Refactor get_file_contents handler in pkg/github/repositories.go
- Extract helper functions: tryRawContentFetch, buildResourceURI, createResourceContent
- Reduce cognitive complexity by reducing nesting levels from 5+ to 2-3

Addresses SonarQube High severity violations:
- (Go) String literals should not be duplicated
- (Go) Cognitive Complexity of functions should not be too high

Co-Authored-By: Jia Wu <jia.wu@cognition.ai>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants