Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,15 @@ The following sets of tools are available (all are on by default):
- `pullNumber`: Pull request number (number, required)
- `repo`: Repository name (string, required)

- **update_pull_request_review** - Update pull request review
- `body`: New body text for the review (string, optional)
- `commitID`: SHA of commit to update the review against (string, optional)
- `event`: New review state (string, optional)
- `owner`: Repository owner (string, required)
- `pullNumber`: Pull request number (number, required)
- `repo`: Repository name (string, required)
- `reviewID`: The ID of the review to update (number, required)

</details>

<details>
Expand Down
52 changes: 52 additions & 0 deletions pkg/github/__toolsnaps__/update_pull_request_review.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
{
"annotations": {
"title": "Update pull request review",
"readOnlyHint": false
},
"description": "Update an existing pull request review (body, event, commit ID).",
"inputSchema": {
"properties": {
"body": {
"description": "New body text for the review",
"type": "string"
},
"commitID": {
"description": "SHA of commit to update the review against",
"type": "string"
},
"event": {
"description": "New review state",
"enum": [
"COMMENT",
"APPROVE",
"REQUEST_CHANGES"
],
"type": "string"
},
"owner": {
"description": "Repository owner",
"type": "string"
},
"pullNumber": {
"description": "Pull request number",
"type": "number"
},
"repo": {
"description": "Repository name",
"type": "string"
},
"reviewID": {
"description": "The ID of the review to update",
"type": "number"
}
},
"required": [
"owner",
"repo",
"pullNumber",
"reviewID"
],
"type": "object"
},
"name": "update_pull_request_review"
}
132 changes: 132 additions & 0 deletions pkg/github/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
"net/http"
"bytes"

"github.com/go-viper/mapstructure/v2"
"github.com/google/go-github/v74/github"
Expand Down Expand Up @@ -1870,6 +1871,137 @@ func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelpe
}
}


// UpdatePullRequestReview provides a tool to update an existing pull request review.
// Use this when you want to modify your own BOT review (body, event, or commit SHA)
// instead of dismissing and recreating a new one.
// This keeps the PR timeline clean (no dismissed markers).
func UpdatePullRequestReview(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) {
return mcp.NewTool(
"update_pull_request_review",
mcp.WithDescription(t(
"TOOL_UPDATE_PULL_REQUEST_REVIEW_DESCRIPTION",
"Update an existing pull request review (body, event, commit ID).",
)),
mcp.WithToolAnnotation(mcp.ToolAnnotation{
Title: t("TOOL_UPDATE_PULL_REQUEST_REVIEW_USER_TITLE", "Update pull request review"),
ReadOnlyHint: ToBoolPtr(false),
}),
mcp.WithString("owner",
mcp.Required(),
mcp.Description("Repository owner"),
),
mcp.WithString("repo",
mcp.Required(),
mcp.Description("Repository name"),
),
mcp.WithNumber("pullNumber",
mcp.Required(),
mcp.Description("Pull request number"),
),
mcp.WithNumber("reviewID",
mcp.Required(),
mcp.Description("The ID of the review to update"),
),
mcp.WithString("body",
mcp.Description("New body text for the review"),
),
mcp.WithString("event",
mcp.Description("New review state"),
mcp.Enum("COMMENT", "APPROVE", "REQUEST_CHANGES"),
),
mcp.WithString("commitID",
mcp.Description("SHA of commit to update the review against"),
),
), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
owner, err := RequiredParam[string](request, "owner")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
repo, err := RequiredParam[string](request, "repo")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
pullNumber, err := RequiredInt(request, "pullNumber")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
reviewID, err := RequiredInt(request, "reviewID")
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
body, _ := OptionalParam[string](request, "body")
event, _ := OptionalParam[string](request, "event")
commitID, _ := OptionalParam[string](request, "commitID")

client, err := getClient(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
}

// Build request body
reqBody := map[string]any{}
if body != "" {
reqBody["body"] = body
}
if event != "" {
reqBody["event"] = event
}
if commitID != "" {
reqBody["commit_id"] = commitID
}

// Encode JSON
jsonBody, _ := json.Marshal(reqBody)

// Construct API path
url := fmt.Sprintf("repos/%s/%s/pulls/%d/reviews/%d", owner, repo, pullNumber, reviewID)

// Create PATCH request
req, err := client.NewRequest("PATCH", url, bytes.NewReader(jsonBody))
if err != nil {
return mcp.NewToolResultError(fmt.Sprintf("failed to build request: %v", err)), nil
}

// Execute request
review := new(github.PullRequestReview)
resp, err := client.Do(ctx, req, review)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx,
"failed to update pull request review",
resp,
err,
), nil
}
defer func() {
if resp != nil && resp.Body != nil {
_ = resp.Body.Close()
}
}()

if resp.StatusCode != http.StatusOK {
bodyBytes, _ := io.ReadAll(resp.Body)
return mcp.NewToolResultError(fmt.Sprintf("failed to update review: %s", string(bodyBytes))), nil
}

// Minimal response back to client
type minimal struct {
ReviewID int64 `json:"reviewID"`
State string `json:"state"`
Body string `json:"body,omitempty"`
URL string `json:"url,omitempty"`
}
out := minimal{
ReviewID: int64(reviewID),
State: review.GetState(),
Body: review.GetBody(),
URL: review.GetHTMLURL(),
}
b, _ := json.Marshal(out)
return mcp.NewToolResultText(string(b)), nil
}
}

// newGQLString like takes something that approximates a string (of which there are many types in shurcooL/githubv4)
// and constructs a pointer to it, or nil if the string is empty. This is extremely useful because when we parse
// params from the MCP request, we need to convert them to types that are pointers of type def strings and it's
Expand Down
120 changes: 120 additions & 0 deletions pkg/github/pullrequests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"

Expand Down Expand Up @@ -2910,3 +2912,121 @@ func getLatestPendingReviewQuery(p getLatestPendingReviewQueryParams) githubv4mo
),
)
}

func TestUpdatePullRequestReview(t *testing.T) {
// Snapshot test for tool definition
mockClient := github.NewClient(nil)
tool, _ := UpdatePullRequestReview(stubGetClientFn(mockClient), translations.NullTranslationHelper)
require.NoError(t, toolsnaps.Test(tool.Name, tool))

assert.Equal(t, "update_pull_request_review", tool.Name)
assert.NotEmpty(t, tool.Description)
assert.Contains(t, tool.InputSchema.Properties, "owner")
assert.Contains(t, tool.InputSchema.Properties, "repo")
assert.Contains(t, tool.InputSchema.Properties, "pullNumber")
assert.Contains(t, tool.InputSchema.Properties, "reviewID")
// body, event, commitID are optional
assert.Subset(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber", "reviewID"})

t.Run("successful update", func(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/repos/owner/repo/pulls/42/reviews/201", func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, http.MethodPatch, r.Method)

_ = json.NewEncoder(w).Encode(&github.PullRequestReview{
ID: github.Ptr(int64(201)),
State: github.Ptr("COMMENT"),
Body: github.Ptr("updated body"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42#review-201"),
})
})
srv := httptest.NewServer(mux)
defer srv.Close()

getClient := func(_ context.Context) (*github.Client, error) {
c := github.NewClient(nil)
c.BaseURL = mustParseURL(srv.URL + "/")
return c, nil
}

_, handler := UpdatePullRequestReview(getClient, translations.NullTranslationHelper)
req := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"reviewID": float64(201),
"body": "updated body",
"event": "COMMENT",
})

result, err := handler(context.Background(), req)
require.NoError(t, err)
require.False(t, result.IsError)

text := getTextResult(t, result)
assert.Contains(t, text.Text, `"state":"COMMENT"`)
assert.Contains(t, text.Text, `"body":"updated body"`)
})

t.Run("API error", func(t *testing.T) {
mux := http.NewServeMux()
mux.HandleFunc("/repos/owner/repo/pulls/42/reviews/202", func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, http.MethodPatch, r.Method)
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"message":"Bad request"}`))
})
srv := httptest.NewServer(mux)
defer srv.Close()

getClient := func(_ context.Context) (*github.Client, error) {
c := github.NewClient(nil)
c.BaseURL = mustParseURL(srv.URL + "/")
return c, nil
}

_, handler := UpdatePullRequestReview(getClient, translations.NullTranslationHelper)
req := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"reviewID": float64(202),
"body": "new",
})

result, err := handler(context.Background(), req)
require.NoError(t, err)
require.True(t, result.IsError)

errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, "failed to update pull request review")
})

t.Run("missing required parameter", func(t *testing.T) {
client := github.NewClient(nil)
_, handler := UpdatePullRequestReview(stubGetClientFn(client), translations.NullTranslationHelper)

// missing reviewID
req := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
// no reviewID
})

result, err := handler(context.Background(), req)
require.NoError(t, err)
require.True(t, result.IsError)

errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, "missing required parameter")
})
}

// Helper for parsing test server URLs
func mustParseURL(raw string) *url.URL {
u, err := url.Parse(raw)
if err != nil {
panic(err)
}
return u
}
1 change: 1 addition & 0 deletions pkg/github/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG
toolsets.NewServerTool(AddCommentToPendingReview(getGQLClient, t)),
toolsets.NewServerTool(SubmitPendingPullRequestReview(getGQLClient, t)),
toolsets.NewServerTool(DeletePendingPullRequestReview(getGQLClient, t)),
toolsets.NewServerTool(UpdatePullRequestReview(getClient, t)),
)
codeSecurity := toolsets.NewToolset("code_security", "Code security related tools, such as GitHub Code Scanning").
AddReadTools(
Expand Down