From b57d82a104fefad99838ff087adce42c0d8ff5f0 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 8 Jan 2026 16:19:35 +0000 Subject: [PATCH 1/2] Fix SonarQube issues: string literal duplication and cognitive complexity - Extract duplicated string literals into constants in pullrequests.go and repositories.go - Add buildPullRequestUpdate helper function to reduce cognitive complexity in UpdatePullRequest - Add checkResponseStatus, createDeleteFileTree, and createDeleteFileCommit helper functions to reduce cognitive complexity in DeleteFile Addresses SonarQube rules: - go:S1192 (String literals should not be duplicated) - go:S3776 (Cognitive Complexity of functions should not be too high) Co-Authored-By: Eashan Sinha --- pkg/github/pullrequests.go | 288 ++++++++++++++++++++----------------- pkg/github/repositories.go | 242 +++++++++++++++---------------- 2 files changed, 281 insertions(+), 249 deletions(-) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index bad822b13..686359bf4 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -17,6 +17,73 @@ import ( "github.com/github/github-mcp-server/pkg/translations" ) +const ( + descRepoOwner = "Repository owner" + descRepoName = "Repository name" + descPullRequestNumber = "Pull request number" + errFailedGetClient = "failed to get GitHub client: %w" + errFailedReadBody = "failed to read response body: %w" + errFailedMarshal = "failed to marshal response: %w" + errFailedGetGQLClient = "failed to get GitHub GQL client: %w" + errNoPendingReview = "No pending review found for the viewer" + errReviewNotPending = "The latest review, found at %s is not pending" + errNoUpdateParams = "No update parameters provided." +) + +// buildPullRequestUpdate builds a PullRequest update struct from the request parameters. +// Returns the update struct, whether any updates were requested, and any error encountered. +func buildPullRequestUpdate(request mcp.CallToolRequest) (*github.PullRequest, bool, error) { + update := &github.PullRequest{} + updateNeeded := false + + title, ok, err := OptionalParamOK[string](request, "title") + if err != nil { + return nil, false, err + } + if ok { + update.Title = github.Ptr(title) + updateNeeded = true + } + + body, ok, err := OptionalParamOK[string](request, "body") + if err != nil { + return nil, false, err + } + if ok { + update.Body = github.Ptr(body) + updateNeeded = true + } + + state, ok, err := OptionalParamOK[string](request, "state") + if err != nil { + return nil, false, err + } + if ok { + update.State = github.Ptr(state) + updateNeeded = true + } + + base, ok, err := OptionalParamOK[string](request, "base") + if err != nil { + return nil, false, err + } + if ok { + update.Base = &github.PullRequestBranch{Ref: github.Ptr(base)} + updateNeeded = true + } + + maintainerCanModify, ok, err := OptionalParamOK[bool](request, "maintainer_can_modify") + if err != nil { + return nil, false, err + } + if ok { + update.MaintainerCanModify = github.Ptr(maintainerCanModify) + updateNeeded = true + } + + return update, updateNeeded, nil +} + // GetPullRequest creates a tool to get details of a specific pull request. func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("get_pull_request", @@ -27,15 +94,15 @@ func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) }), 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -54,7 +121,7 @@ func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) if err != nil { @@ -69,14 +136,14 @@ func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil } r, err := json.Marshal(pr) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -93,11 +160,11 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu }), 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("title", mcp.Required(), @@ -173,7 +240,7 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } pr, resp, err := client.PullRequests.Create(ctx, owner, repo, newPR) if err != nil { @@ -188,14 +255,14 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu if resp.StatusCode != http.StatusCreated { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to create pull request: %s", string(body))), nil } r, err := json.Marshal(pr) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -212,11 +279,11 @@ func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu }), 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("pullNumber", mcp.Required(), @@ -253,52 +320,17 @@ func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu return mcp.NewToolResultError(err.Error()), nil } - // Build the update struct only with provided fields - update := &github.PullRequest{} - updateNeeded := false - - if title, ok, err := OptionalParamOK[string](request, "title"); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } else if ok { - update.Title = github.Ptr(title) - updateNeeded = true - } - - if body, ok, err := OptionalParamOK[string](request, "body"); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } else if ok { - update.Body = github.Ptr(body) - updateNeeded = true - } - - if state, ok, err := OptionalParamOK[string](request, "state"); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } else if ok { - update.State = github.Ptr(state) - updateNeeded = true - } - - if base, ok, err := OptionalParamOK[string](request, "base"); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } else if ok { - update.Base = &github.PullRequestBranch{Ref: github.Ptr(base)} - updateNeeded = true - } - - if maintainerCanModify, ok, err := OptionalParamOK[bool](request, "maintainer_can_modify"); err != nil { + update, updateNeeded, err := buildPullRequestUpdate(request) + if err != nil { return mcp.NewToolResultError(err.Error()), nil - } else if ok { - update.MaintainerCanModify = github.Ptr(maintainerCanModify) - updateNeeded = true } - if !updateNeeded { - return mcp.NewToolResultError("No update parameters provided."), nil + return mcp.NewToolResultError(errNoUpdateParams), nil } client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } pr, resp, err := client.PullRequests.Edit(ctx, owner, repo, pullNumber, update) if err != nil { @@ -313,14 +345,14 @@ func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to update pull request: %s", string(body))), nil } r, err := json.Marshal(pr) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -337,11 +369,11 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun }), 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("state", mcp.Description("Filter by state"), @@ -411,7 +443,7 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } prs, resp, err := client.PullRequests.List(ctx, owner, repo, opts) if err != nil { @@ -426,14 +458,14 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to list pull requests: %s", string(body))), nil } r, err := json.Marshal(prs) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -450,15 +482,15 @@ func MergePullRequest(getClient GetClientFn, t translations.TranslationHelperFun }), 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), mcp.WithString("commit_title", mcp.Description("Title for merge commit"), @@ -504,7 +536,7 @@ func MergePullRequest(getClient GetClientFn, t translations.TranslationHelperFun client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } result, resp, err := client.PullRequests.Merge(ctx, owner, repo, pullNumber, commitMessage, options) if err != nil { @@ -519,14 +551,14 @@ func MergePullRequest(getClient GetClientFn, t translations.TranslationHelperFun if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to merge pull request: %s", string(body))), nil } r, err := json.Marshal(result) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -588,15 +620,15 @@ func GetPullRequestFiles(getClient GetClientFn, 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), WithPagination(), ), @@ -620,7 +652,7 @@ func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelper client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } opts := &github.ListOptions{ PerPage: pagination.perPage, @@ -639,14 +671,14 @@ func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelper if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request files: %s", string(body))), nil } r, err := json.Marshal(files) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -663,15 +695,15 @@ func GetPullRequestStatus(getClient GetClientFn, t translations.TranslationHelpe }), 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -690,7 +722,7 @@ func GetPullRequestStatus(getClient GetClientFn, t translations.TranslationHelpe // First get the PR to find the head SHA client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) if err != nil { @@ -705,7 +737,7 @@ func GetPullRequestStatus(getClient GetClientFn, t translations.TranslationHelpe if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil } @@ -724,14 +756,14 @@ func GetPullRequestStatus(getClient GetClientFn, t translations.TranslationHelpe if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get combined status: %s", string(body))), nil } r, err := json.Marshal(status) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -748,15 +780,15 @@ func UpdatePullRequestBranch(getClient GetClientFn, t translations.TranslationHe }), 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), mcp.WithString("expectedHeadSha", mcp.Description("The expected SHA of the pull request's HEAD ref"), @@ -786,7 +818,7 @@ func UpdatePullRequestBranch(getClient GetClientFn, t translations.TranslationHe client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } result, resp, err := client.PullRequests.UpdateBranch(ctx, owner, repo, pullNumber, opts) if err != nil { @@ -806,14 +838,14 @@ func UpdatePullRequestBranch(getClient GetClientFn, t translations.TranslationHe if resp.StatusCode != http.StatusAccepted { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to update pull request branch: %s", string(body))), nil } r, err := json.Marshal(result) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -830,15 +862,15 @@ func GetPullRequestComments(getClient GetClientFn, t translations.TranslationHel }), 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -863,7 +895,7 @@ func GetPullRequestComments(getClient GetClientFn, t translations.TranslationHel client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } comments, resp, err := client.PullRequests.ListComments(ctx, owner, repo, pullNumber, opts) if err != nil { @@ -878,14 +910,14 @@ func GetPullRequestComments(getClient GetClientFn, t translations.TranslationHel if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request comments: %s", string(body))), nil } r, err := json.Marshal(comments) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -902,15 +934,15 @@ func GetPullRequestReviews(getClient GetClientFn, 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.WithNumber("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -929,7 +961,7 @@ func GetPullRequestReviews(getClient GetClientFn, t translations.TranslationHelp client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(errFailedGetClient, err) } reviews, resp, err := client.PullRequests.ListReviews(ctx, owner, repo, pullNumber, nil) if err != nil { @@ -944,14 +976,14 @@ func GetPullRequestReviews(getClient GetClientFn, t translations.TranslationHelp if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request reviews: %s", string(body))), nil } r, err := json.Marshal(reviews) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(errFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -970,15 +1002,15 @@ func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translation // internally for now. 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), mcp.WithString("body", mcp.Required(), @@ -1073,15 +1105,15 @@ func CreatePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. // internally for now. 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), mcp.WithString("commitID", mcp.Description("SHA of commit to review"), @@ -1171,15 +1203,15 @@ func AddPullRequestReviewCommentToPendingReview(getGQLClient GetGQLClientFn, t t // ), 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), mcp.WithString("path", mcp.Required(), @@ -1228,7 +1260,7 @@ func AddPullRequestReviewCommentToPendingReview(getGQLClient GetGQLClientFn, t t client, err := getGQLClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub GQL client: %w", err) + return nil, fmt.Errorf(errFailedGetGQLClient, err) } // First we'll get the current user @@ -1275,12 +1307,12 @@ func AddPullRequestReviewCommentToPendingReview(getGQLClient GetGQLClientFn, t t // Validate there is one review and the state is pending if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return mcp.NewToolResultError("No pending review found for the viewer"), nil + return mcp.NewToolResultError(errNoPendingReview), nil } review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] if review.State != githubv4.PullRequestReviewStatePending { - errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) + errText := fmt.Sprintf(errReviewNotPending, review.URL) return mcp.NewToolResultError(errText), nil } @@ -1332,15 +1364,15 @@ func SubmitPendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. // the latest review from a user, since only one can be active at a time. 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), mcp.WithString("event", mcp.Required(), @@ -1365,7 +1397,7 @@ func SubmitPendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. client, err := getGQLClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub GQL client: %w", err) + return nil, fmt.Errorf(errFailedGetGQLClient, err) } // First we'll get the current user @@ -1412,12 +1444,12 @@ func SubmitPendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. // Validate there is one review and the state is pending if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return mcp.NewToolResultError("No pending review found for the viewer"), nil + return mcp.NewToolResultError(errNoPendingReview), nil } review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] if review.State != githubv4.PullRequestReviewStatePending { - errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) + errText := fmt.Sprintf(errReviewNotPending, review.URL) return mcp.NewToolResultError(errText), nil } @@ -1466,15 +1498,15 @@ func DeletePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. // the latest pending review from a user, since only one can be active at a time. 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -1489,7 +1521,7 @@ func DeletePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. client, err := getGQLClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub GQL client: %w", err) + return nil, fmt.Errorf(errFailedGetGQLClient, err) } // First we'll get the current user @@ -1536,12 +1568,12 @@ func DeletePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. // Validate there is one review and the state is pending if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return mcp.NewToolResultError("No pending review found for the viewer"), nil + return mcp.NewToolResultError(errNoPendingReview), nil } review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] if review.State != githubv4.PullRequestReviewStatePending { - errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) + errText := fmt.Sprintf(errReviewNotPending, review.URL) return mcp.NewToolResultError(errText), nil } @@ -1581,15 +1613,15 @@ func GetPullRequestDiff(getClient GetClientFn, t translations.TranslationHelperF }), 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -1625,7 +1657,7 @@ func GetPullRequestDiff(getClient GetClientFn, t translations.TranslationHelperF if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request diff: %s", string(body))), nil } @@ -1649,15 +1681,15 @@ func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelpe }), 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("pullNumber", mcp.Required(), - mcp.Description("Pull request number"), + mcp.Description(descPullRequestNumber), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -1703,7 +1735,7 @@ func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelpe if resp.StatusCode != http.StatusCreated { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(errFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to request copilot review: %s", string(body))), nil } diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 29f776a05..9f4e26b39 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -19,6 +19,53 @@ import ( "github.com/mark3labs/mcp-go/server" ) +const ( + repoDescRepoOwner = "Repository owner" + repoDescRepoName = "Repository name" + repoDescRepoOwnerOrg = "Repository owner (username or organization)" + repoDescCommitMessage = "Commit message" + repoErrFailedGetClient = "failed to get GitHub client: %w" + repoErrFailedReadBody = "failed to read response body: %w" + repoErrFailedMarshal = "failed to marshal response: %w" + repoRefsHeadsPrefix = "refs/heads/" +) + +// checkResponseStatus checks if the response status code matches the expected status. +// Returns a tool result error if the status doesn't match, nil otherwise. +func checkResponseStatus(resp *github.Response, expectedStatus int, errorMsg string) (*mcp.CallToolResult, error) { + if resp.StatusCode != expectedStatus { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf(repoErrFailedReadBody, err) + } + return mcp.NewToolResultError(fmt.Sprintf("%s: %s", errorMsg, string(body))), nil + } + return nil, nil +} + +// createDeleteFileTree creates a tree entry for file deletion. +func createDeleteFileTree(ctx context.Context, client *github.Client, owner, repo, path, baseTreeSHA string) (*github.Tree, *github.Response, error) { + treeEntries := []*github.TreeEntry{ + { + Path: github.Ptr(path), + Mode: github.Ptr("100644"), + Type: github.Ptr("blob"), + SHA: nil, + }, + } + return client.Git.CreateTree(ctx, owner, repo, baseTreeSHA, treeEntries) +} + +// createDeleteFileCommit creates a commit for file deletion. +func createDeleteFileCommit(ctx context.Context, client *github.Client, owner, repo, message string, tree *github.Tree, parentSHA string) (*github.Commit, *github.Response, error) { + commit := &github.Commit{ + Message: github.Ptr(message), + Tree: tree, + Parents: []*github.Commit{{SHA: github.Ptr(parentSHA)}}, + } + return client.Git.CreateCommit(ctx, owner, repo, commit, nil) +} + func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_commit", mcp.WithDescription(t("TOOL_GET_COMMITS_DESCRIPTION", "Get details for a commit from a GitHub repository")), @@ -28,11 +75,11 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(repoDescRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("sha", mcp.Required(), @@ -65,7 +112,7 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } commit, resp, err := client.Repositories.GetCommit(ctx, owner, repo, sha, opts) if err != nil { @@ -80,14 +127,14 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too if resp.StatusCode != 200 { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get commit: %s", string(body))), nil } r, err := json.Marshal(commit) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -104,11 +151,11 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(repoDescRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("sha", mcp.Description("The commit SHA, branch name, or tag name to list commits from. If not specified, defaults to the repository's default branch."), @@ -155,7 +202,7 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } commits, resp, err := client.Repositories.ListCommits(ctx, owner, repo, opts) if err != nil { @@ -170,14 +217,14 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t if resp.StatusCode != 200 { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to list commits: %s", string(body))), nil } r, err := json.Marshal(commits) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -194,11 +241,11 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(repoDescRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), WithPagination(), ), @@ -225,7 +272,7 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } branches, resp, err := client.Repositories.ListBranches(ctx, owner, repo, opts) @@ -241,14 +288,14 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to list branches: %s", string(body))), nil } r, err := json.Marshal(branches) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -265,11 +312,11 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner (username or organization)"), + mcp.Description(repoDescRepoOwnerOrg), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("path", mcp.Required(), @@ -281,7 +328,7 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF ), mcp.WithString("message", mcp.Required(), - mcp.Description("Commit message"), + mcp.Description(repoDescCommitMessage), ), mcp.WithString("branch", mcp.Required(), @@ -339,7 +386,7 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF // Create or update the file client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } fileContent, resp, err := client.Repositories.CreateFile(ctx, owner, repo, path, opts) if err != nil { @@ -354,14 +401,14 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF if resp.StatusCode != 200 && resp.StatusCode != 201 { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to create/update file: %s", string(body))), nil } r, err := json.Marshal(fileContent) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -378,7 +425,7 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun }), mcp.WithString("name", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("description", mcp.Description("Repository description"), @@ -417,7 +464,7 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } createdRepo, resp, err := client.Repositories.Create(ctx, "", repo) if err != nil { @@ -432,14 +479,14 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun if resp.StatusCode != http.StatusCreated { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to create repository: %s", string(body))), nil } r, err := json.Marshal(createdRepo) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -456,11 +503,11 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner (username or organization)"), + mcp.Description(repoDescRepoOwnerOrg), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("path", mcp.Required(), @@ -503,7 +550,7 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t // fetch the PR from the API to get the latest commit and use SHA githubClient, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } prNum, err := strconv.Atoi(prNumber) if err != nil { @@ -624,11 +671,11 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(repoDescRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("organization", mcp.Description("Organization to fork to"), @@ -655,7 +702,7 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } forkedRepo, resp, err := client.Repositories.CreateFork(ctx, owner, repo, opts) if err != nil { @@ -675,14 +722,14 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) if resp.StatusCode != http.StatusAccepted { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to fork repository: %s", string(body))), nil } r, err := json.Marshal(forkedRepo) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -705,11 +752,11 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner (username or organization)"), + mcp.Description(repoDescRepoOwnerOrg), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("path", mcp.Required(), @@ -717,7 +764,7 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to ), mcp.WithString("message", mcp.Required(), - mcp.Description("Commit message"), + mcp.Description(repoDescCommitMessage), ), mcp.WithString("branch", mcp.Required(), @@ -748,11 +795,11 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } // Get the reference for the branch - ref, resp, err := client.Git.GetRef(ctx, owner, repo, "refs/heads/"+branch) + ref, resp, err := client.Git.GetRef(ctx, owner, repo, repoRefsHeadsPrefix+branch) if err != nil { return nil, fmt.Errorf("failed to get branch reference: %w", err) } @@ -761,93 +808,46 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to // Get the commit object that the branch points to baseCommit, resp, err := client.Git.GetCommit(ctx, owner, repo, *ref.Object.SHA) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get base commit", - resp, - err, - ), nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get base commit", resp, err), nil } defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get commit: %s", string(body))), nil - } - - // Create a tree entry for the file deletion by setting SHA to nil - treeEntries := []*github.TreeEntry{ - { - Path: github.Ptr(path), - Mode: github.Ptr("100644"), // Regular file mode - Type: github.Ptr("blob"), - SHA: nil, // Setting SHA to nil deletes the file - }, + if result, err := checkResponseStatus(resp, http.StatusOK, "failed to get commit"); result != nil || err != nil { + return result, err } // Create a new tree with the deletion - newTree, resp, err := client.Git.CreateTree(ctx, owner, repo, *baseCommit.Tree.SHA, treeEntries) + newTree, resp, err := createDeleteFileTree(ctx, client, owner, repo, path, *baseCommit.Tree.SHA) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to create tree", - resp, - err, - ), nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create tree", resp, err), nil } defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusCreated { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to create tree: %s", string(body))), nil + if result, err := checkResponseStatus(resp, http.StatusCreated, "failed to create tree"); result != nil || err != nil { + return result, err } // Create a new commit with the new tree - commit := &github.Commit{ - Message: github.Ptr(message), - Tree: newTree, - Parents: []*github.Commit{{SHA: baseCommit.SHA}}, - } - newCommit, resp, err := client.Git.CreateCommit(ctx, owner, repo, commit, nil) + newCommit, resp, err := createDeleteFileCommit(ctx, client, owner, repo, message, newTree, *baseCommit.SHA) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to create commit", - resp, - err, - ), nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create commit", resp, err), nil } defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusCreated { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to create commit: %s", string(body))), nil + if result, err := checkResponseStatus(resp, http.StatusCreated, "failed to create commit"); result != nil || err != nil { + return result, err } // Update the branch reference to point to the new commit ref.Object.SHA = newCommit.SHA _, resp, err = client.Git.UpdateRef(ctx, owner, repo, ref, false) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to update reference", - resp, - err, - ), nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update reference", resp, err), nil } defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to update reference: %s", string(body))), nil + if result, err := checkResponseStatus(resp, http.StatusOK, "failed to update reference"); result != nil || err != nil { + return result, err } // Create a response similar to what the DeleteFile API would return @@ -858,7 +858,7 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to r, err := json.Marshal(response) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -875,11 +875,11 @@ func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) ( }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(repoDescRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("branch", mcp.Required(), @@ -909,7 +909,7 @@ func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) ( client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } // Get the source branch SHA @@ -959,7 +959,7 @@ func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) ( r, err := json.Marshal(createdRef) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -976,11 +976,11 @@ func PushFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (too }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(repoDescRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("branch", mcp.Required(), @@ -1008,7 +1008,7 @@ func PushFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (too ), mcp.WithString("message", mcp.Required(), - mcp.Description("Commit message"), + mcp.Description(repoDescCommitMessage), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -1037,11 +1037,11 @@ func PushFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (too client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } // Get the reference for the branch - ref, resp, err := client.Git.GetRef(ctx, owner, repo, "refs/heads/"+branch) + ref, resp, err := client.Git.GetRef(ctx, owner, repo, repoRefsHeadsPrefix+branch) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get branch reference", @@ -1131,7 +1131,7 @@ func PushFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (too r, err := json.Marshal(updatedRef) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -1148,11 +1148,11 @@ func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(repoDescRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), WithPagination(), ), @@ -1177,7 +1177,7 @@ func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } tags, resp, err := client.Repositories.ListTags(ctx, owner, repo, opts) @@ -1193,14 +1193,14 @@ func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to list tags: %s", string(body))), nil } r, err := json.Marshal(tags) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil @@ -1217,11 +1217,11 @@ func GetTag(getClient GetClientFn, t translations.TranslationHelperFunc) (tool m }), mcp.WithString("owner", mcp.Required(), - mcp.Description("Repository owner"), + mcp.Description(repoDescRepoOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("Repository name"), + mcp.Description(repoDescRepoName), ), mcp.WithString("tag", mcp.Required(), @@ -1244,7 +1244,7 @@ func GetTag(getClient GetClientFn, t translations.TranslationHelperFunc) (tool m client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, fmt.Errorf(repoErrFailedGetClient, err) } // First get the tag reference @@ -1261,7 +1261,7 @@ func GetTag(getClient GetClientFn, t translations.TranslationHelperFunc) (tool m if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get tag reference: %s", string(body))), nil } @@ -1280,14 +1280,14 @@ func GetTag(getClient GetClientFn, t translations.TranslationHelperFunc) (tool m if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf(repoErrFailedReadBody, err) } return mcp.NewToolResultError(fmt.Sprintf("failed to get tag object: %s", string(body))), nil } r, err := json.Marshal(tagObj) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, fmt.Errorf(repoErrFailedMarshal, err) } return mcp.NewToolResultText(string(r)), nil From bf2f0d5b97656cbb7c216574c4741abc79dca53d Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 8 Jan 2026 16:24:54 +0000 Subject: [PATCH 2/2] Consolidate duplicate constants to reduce code duplication - Remove duplicate descRepoOwner and descRepoName constants from pullrequests.go - Remove duplicate repoDescRepoOwner and repoDescRepoName constants from repositories.go - Use existing DescriptionRepositoryOwner and DescriptionRepositoryName from actions.go - This reduces cross-file string literal duplication flagged by SonarCloud Co-Authored-By: Eashan Sinha --- pkg/github/pullrequests.go | 70 ++++++++++++++++++-------------------- pkg/github/repositories.go | 42 +++++++++++------------ 2 files changed, 54 insertions(+), 58 deletions(-) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 686359bf4..d22611f83 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -18,8 +18,6 @@ import ( ) const ( - descRepoOwner = "Repository owner" - descRepoName = "Repository name" descPullRequestNumber = "Pull request number" errFailedGetClient = "failed to get GitHub client: %w" errFailedReadBody = "failed to read response body: %w" @@ -94,11 +92,11 @@ func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -160,11 +158,11 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("title", mcp.Required(), @@ -279,11 +277,11 @@ func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -369,11 +367,11 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("state", mcp.Description("Filter by state"), @@ -482,11 +480,11 @@ func MergePullRequest(getClient GetClientFn, t translations.TranslationHelperFun }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -620,11 +618,11 @@ func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelper }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -695,11 +693,11 @@ func GetPullRequestStatus(getClient GetClientFn, t translations.TranslationHelpe }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -780,11 +778,11 @@ func UpdatePullRequestBranch(getClient GetClientFn, t translations.TranslationHe }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -862,11 +860,11 @@ func GetPullRequestComments(getClient GetClientFn, t translations.TranslationHel }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -934,11 +932,11 @@ func GetPullRequestReviews(getClient GetClientFn, t translations.TranslationHelp }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -1002,11 +1000,11 @@ func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translation // internally for now. mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -1105,11 +1103,11 @@ func CreatePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. // internally for now. mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -1203,11 +1201,11 @@ func AddPullRequestReviewCommentToPendingReview(getGQLClient GetGQLClientFn, t t // ), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -1364,11 +1362,11 @@ func SubmitPendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. // the latest review from a user, since only one can be active at a time. mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -1498,11 +1496,11 @@ func DeletePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. // the latest pending review from a user, since only one can be active at a time. mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -1613,11 +1611,11 @@ func GetPullRequestDiff(getClient GetClientFn, t translations.TranslationHelperF }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), @@ -1681,11 +1679,11 @@ func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelpe }), mcp.WithString("owner", mcp.Required(), - mcp.Description(descRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(descRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithNumber("pullNumber", mcp.Required(), diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 9f4e26b39..ce5d1fd6b 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -20,8 +20,6 @@ import ( ) const ( - repoDescRepoOwner = "Repository owner" - repoDescRepoName = "Repository name" repoDescRepoOwnerOrg = "Repository owner (username or organization)" repoDescCommitMessage = "Commit message" repoErrFailedGetClient = "failed to get GitHub client: %w" @@ -75,11 +73,11 @@ func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (too }), mcp.WithString("owner", mcp.Required(), - mcp.Description(repoDescRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("sha", mcp.Required(), @@ -151,11 +149,11 @@ func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (t }), mcp.WithString("owner", mcp.Required(), - mcp.Description(repoDescRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("sha", mcp.Description("The commit SHA, branch name, or tag name to list commits from. If not specified, defaults to the repository's default branch."), @@ -241,11 +239,11 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( }), mcp.WithString("owner", mcp.Required(), - mcp.Description(repoDescRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), WithPagination(), ), @@ -316,7 +314,7 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("path", mcp.Required(), @@ -425,7 +423,7 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun }), mcp.WithString("name", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("description", mcp.Description("Repository description"), @@ -507,7 +505,7 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("path", mcp.Required(), @@ -671,11 +669,11 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) }), mcp.WithString("owner", mcp.Required(), - mcp.Description(repoDescRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("organization", mcp.Description("Organization to fork to"), @@ -756,7 +754,7 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("path", mcp.Required(), @@ -875,11 +873,11 @@ func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) ( }), mcp.WithString("owner", mcp.Required(), - mcp.Description(repoDescRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("branch", mcp.Required(), @@ -976,11 +974,11 @@ func PushFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (too }), mcp.WithString("owner", mcp.Required(), - mcp.Description(repoDescRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("branch", mcp.Required(), @@ -1148,11 +1146,11 @@ func ListTags(getClient GetClientFn, t translations.TranslationHelperFunc) (tool }), mcp.WithString("owner", mcp.Required(), - mcp.Description(repoDescRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), WithPagination(), ), @@ -1217,11 +1215,11 @@ func GetTag(getClient GetClientFn, t translations.TranslationHelperFunc) (tool m }), mcp.WithString("owner", mcp.Required(), - mcp.Description(repoDescRepoOwner), + mcp.Description(DescriptionRepositoryOwner), ), mcp.WithString("repo", mcp.Required(), - mcp.Description(repoDescRepoName), + mcp.Description(DescriptionRepositoryName), ), mcp.WithString("tag", mcp.Required(),