From bcb1c928d12fc9ad0c7c95de087c617eadf9a676 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 18 Dec 2025 21:24:14 +0000 Subject: [PATCH] Fix MBA-471: Resolve High severity string duplication and cognitive complexity issues This commit addresses two SonarQube High severity violations: 1. String Duplication (go:S1192): - Created pkg/errors/constants.go with extracted string constants - Updated pkg/errors/error.go to use ErrContextMissingGitHubCtxErrors constant - Replaced 4 instances of duplicated 'context does not contain GitHubCtxErrors' string 2. Cognitive Complexity (go:S3776): - Extracted buildResourceURI helper function for URI construction - Extracted createResourceContent helper function for content type handling - Extracted fetchRawContent helper function for raw content fetching - Refactored GetFileContents handler to use new helpers, reducing nesting levels Co-Authored-By: Jia Wu --- pkg/errors/constants.go | 13 ++++ pkg/errors/error.go | 9 +-- pkg/github/repositories.go | 119 ++++++++++++++++++++----------------- 3 files changed, 83 insertions(+), 58 deletions(-) create mode 100644 pkg/errors/constants.go diff --git a/pkg/errors/constants.go b/pkg/errors/constants.go new file mode 100644 index 000000000..6ba876377 --- /dev/null +++ b/pkg/errors/constants.go @@ -0,0 +1,13 @@ +package errors + +// Error message constants to avoid string duplication (SonarQube rule go:S1192) +const ( + // ErrContextMissingGitHubCtxErrors is returned when the context does not contain GitHubCtxErrors + ErrContextMissingGitHubCtxErrors = "context does not contain GitHubCtxErrors" + + // ErrFailedToGetGitHubClient is returned when getting the GitHub client fails + ErrFailedToGetGitHubClient = "failed to get GitHub client" + + // ErrMissingRequiredParameter is a format string for missing required parameter errors + ErrMissingRequiredParameter = "missing required parameter: %s" +) diff --git a/pkg/errors/error.go b/pkg/errors/error.go index 9d81e9010..dde2cdaf6 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -2,6 +2,7 @@ package errors import ( "context" + "errors" "fmt" "github.com/google/go-github/v72/github" @@ -71,7 +72,7 @@ func GetGitHubAPIErrors(ctx context.Context) ([]*GitHubAPIError, error) { if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok { return val.api, nil // return the slice of API errors from the context } - return nil, fmt.Errorf("context does not contain GitHubCtxErrors") + return nil, errors.New(ErrContextMissingGitHubCtxErrors) } // GetGitHubGraphQLErrors retrieves the slice of GitHubGraphQLErrors from the context. @@ -79,7 +80,7 @@ func GetGitHubGraphQLErrors(ctx context.Context) ([]*GitHubGraphQLError, error) if val, ok := ctx.Value(GitHubErrorKey{}).(*GitHubCtxErrors); ok { return val.graphQL, nil // return the slice of GraphQL errors from the context } - return nil, fmt.Errorf("context does not contain GitHubCtxErrors") + return nil, errors.New(ErrContextMissingGitHubCtxErrors) } func NewGitHubAPIErrorToCtx(ctx context.Context, message string, resp *github.Response, err error) (context.Context, error) { @@ -95,7 +96,7 @@ func addGitHubAPIErrorToContext(ctx context.Context, err *GitHubAPIError) (conte val.api = append(val.api, err) // append the error to the existing slice in the context return ctx, nil } - return nil, fmt.Errorf("context does not contain GitHubCtxErrors") + return nil, errors.New(ErrContextMissingGitHubCtxErrors) } func addGitHubGraphQLErrorToContext(ctx context.Context, err *GitHubGraphQLError) (context.Context, error) { @@ -103,7 +104,7 @@ func addGitHubGraphQLErrorToContext(ctx context.Context, err *GitHubGraphQLError val.graphQL = append(val.graphQL, err) // append the error to the existing slice in the context return ctx, nil } - return nil, fmt.Errorf("context does not contain GitHubCtxErrors") + return nil, errors.New(ErrContextMissingGitHubCtxErrors) } // NewGitHubAPIErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 29f776a05..d0e7df952 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -446,6 +446,69 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun } } +// buildResourceURI constructs the resource URI based on the provided parameters. +// This helper function reduces cognitive complexity by extracting URI building logic. +func buildResourceURI(owner, repo, path, sha, ref string) (string, error) { + switch { + case sha != "": + return url.JoinPath("repo://", owner, repo, "sha", sha, "contents", path) + case ref != "": + return url.JoinPath("repo://", owner, repo, ref, "contents", path) + default: + return url.JoinPath("repo://", owner, repo, "contents", path) + } +} + +// createResourceContent creates the appropriate MCP resource content based on content type. +// This helper function reduces cognitive complexity by extracting content creation logic. +func createResourceContent(body []byte, contentType, resourceURI string) *mcp.CallToolResult { + if strings.HasPrefix(contentType, "application") || strings.HasPrefix(contentType, "text") { + return mcp.NewToolResultResource("successfully downloaded text file", mcp.TextResourceContents{ + URI: resourceURI, + Text: string(body), + MIMEType: contentType, + }) + } + return mcp.NewToolResultResource("successfully downloaded binary file", mcp.BlobResourceContents{ + URI: resourceURI, + Blob: base64.StdEncoding.EncodeToString(body), + MIMEType: contentType, + }) +} + +// fetchRawContent attempts to fetch raw content from GitHub and returns the result. +// This helper function reduces cognitive complexity by extracting raw content fetching logic. +func fetchRawContent(ctx context.Context, getRawClient raw.GetRawClientFn, owner, repo, path string, rawOpts *raw.ContentOpts) (*mcp.CallToolResult, bool) { + rawClient, err := getRawClient(ctx) + if err != nil { + return mcp.NewToolResultError("failed to get GitHub raw content client"), true + } + resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts) + if err != nil { + return mcp.NewToolResultError("failed to get raw repository content"), true + } + defer func() { + _ = resp.Body.Close() + }() + + if resp.StatusCode != http.StatusOK { + return nil, false + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return mcp.NewToolResultError("failed to read response body"), true + } + + contentType := resp.Header.Get("Content-Type") + resourceURI, err := buildResourceURI(owner, repo, path, rawOpts.SHA, rawOpts.Ref) + if err != nil { + return nil, true + } + + return createResourceContent(body, contentType, resourceURI), true +} + // GetFileContents creates a tool to get the contents of a file or directory from a GitHub repository. func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_file_contents", @@ -523,60 +586,8 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t // If the path is (most likely) not to be a directory, we will first try to get the raw content from the GitHub raw content API. if path != "" && !strings.HasSuffix(path, "/") { - - rawClient, err := getRawClient(ctx) - if err != nil { - return mcp.NewToolResultError("failed to get GitHub raw content client"), nil - } - resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts) - if err != nil { - return mcp.NewToolResultError("failed to get raw repository content"), nil - } - defer func() { - _ = resp.Body.Close() - }() - - if resp.StatusCode == http.StatusOK { - // If the raw content is found, return it directly - body, err := io.ReadAll(resp.Body) - if err != nil { - return mcp.NewToolResultError("failed to read response body"), nil - } - contentType := resp.Header.Get("Content-Type") - - var resourceURI string - switch { - case sha != "": - resourceURI, err = url.JoinPath("repo://", owner, repo, "sha", sha, "contents", path) - if err != nil { - return nil, fmt.Errorf("failed to create resource URI: %w", err) - } - case ref != "": - resourceURI, err = url.JoinPath("repo://", owner, repo, ref, "contents", path) - if err != nil { - return nil, fmt.Errorf("failed to create resource URI: %w", err) - } - default: - resourceURI, err = url.JoinPath("repo://", owner, repo, "contents", path) - if err != nil { - return nil, fmt.Errorf("failed to create resource URI: %w", err) - } - } - - if strings.HasPrefix(contentType, "application") || strings.HasPrefix(contentType, "text") { - return mcp.NewToolResultResource("successfully downloaded text file", mcp.TextResourceContents{ - URI: resourceURI, - Text: string(body), - MIMEType: contentType, - }), nil - } - - return mcp.NewToolResultResource("successfully downloaded binary file", mcp.BlobResourceContents{ - URI: resourceURI, - Blob: base64.StdEncoding.EncodeToString(body), - MIMEType: contentType, - }), nil - + if result, handled := fetchRawContent(ctx, getRawClient, owner, repo, path, rawOpts); handled { + return result, nil } }