github · MayorFaj · Apr 15, 2025 · Apr 17, 2025 · Apr 17, 2025 · Apr 18, 2025
@@ -842,6 +842,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description
   - `owner`: Repository owner (string, required)
   - `pullNumber`: Pull request number to update (number, required)
   - `repo`: Repository name (string, required)
+  - `reviewers`: GitHub usernames to request reviews from (string[], optional)
   - `state`: New state (string, optional)
   - `title`: New title (string, optional)
 

@@ -30,6 +30,13 @@
         "description": "Repository name",
         "type": "string"
       },
+      "reviewers": {
+        "description": "GitHub usernames to request reviews from",
+        "items": {
+          "type": "string"
+        },
+        "type": "array"
+      },
       "state": {
         "description": "New state",
         "enum": [

@@ -238,6 +238,12 @@ func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu
 			mcp.WithBoolean("maintainer_can_modify",
 				mcp.Description("Allow maintainer edits"),
 			),
+			mcp.WithArray("reviewers",
+				mcp.Description("GitHub usernames to request reviews from"),
+				mcp.Items(map[string]interface{}{
+					"type": "string",
+				}),
+			),
 		),
 		func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
 			owner, err := RequiredParam[string](request, "owner")
@@ -292,30 +298,109 @@ func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu
 				updateNeeded = true
 			}
 
-			if !updateNeeded {
-				return mcp.NewToolResultError("No update parameters provided."), nil
+			// Handle reviewers separately
+			reviewers, err := OptionalStringArrayParam(request, "reviewers")
+			if err != nil {
+				return mcp.NewToolResultError(err.Error()), nil
 			}
 
+			// Create the GitHub client
 			client, err := getClient(ctx)
 			if err != nil {
 				return nil, fmt.Errorf("failed to get GitHub client: %w", err)
 			}
-			pr, resp, err := client.PullRequests.Edit(ctx, owner, repo, pullNumber, update)
-			if err != nil {
-				return ghErrors.NewGitHubAPIErrorResponse(ctx,
-					"failed to update pull request",
-					resp,
-					err,
-				), nil
+
+			var pr *github.PullRequest
+			var resp *http.Response
+
+			// First, update the PR if needed
+			if updateNeeded {
+				var ghResp *github.Response
+				pr, ghResp, err = client.PullRequests.Edit(ctx, owner, repo, pullNumber, update)
+				if err != nil {
+					return ghErrors.NewGitHubAPIErrorResponse(ctx,
+						"failed to update pull request",
+						ghResp,
+						err,
+					), nil
+				}
+				resp = ghResp.Response
+				defer func() {
+					if resp != nil && resp.Body != nil {
+						_ = 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 pull request: %s", string(body))), nil
+				}
+			} else {
+				// If no update needed, just get the current PR
+				var ghResp *github.Response
+				pr, ghResp, err = client.PullRequests.Get(ctx, owner, repo, pullNumber)
+				if err != nil {
+					return ghErrors.NewGitHubAPIErrorResponse(ctx,
+						"failed to get pull request",
+						ghResp,
+						err,
+					), nil
+				}
+				resp = ghResp.Response
+				defer func() {
+					if resp != nil && resp.Body != nil {
+						_ = 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 pull request: %s", string(body))), nil
+				}
 			}
-			defer func() { _ = resp.Body.Close() }()
 
-			if resp.StatusCode != http.StatusOK {
-				body, err := io.ReadAll(resp.Body)
+			// Add reviewers if specified
+			if len(reviewers) > 0 {
+				reviewersRequest := github.ReviewersRequest{
+					Reviewers: reviewers,
+				}
+
+				// Use the direct result of RequestReviewers which includes the requested reviewers
+				updatedPR, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, reviewersRequest)
 				if err != nil {
-					return nil, fmt.Errorf("failed to read response body: %w", err)
+					return ghErrors.NewGitHubAPIErrorResponse(ctx,
+						"failed to request reviewers",
+						resp,
+						err,
+					), nil
+				}
+				defer func() {
+					if resp != nil && resp.Body != nil {
+						_ = resp.Body.Close()
+					}
+				}()
+
+				if resp.StatusCode != http.StatusCreated && 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 request reviewers: %s", string(body))), nil
 				}
-				return mcp.NewToolResultError(fmt.Sprintf("failed to update pull request: %s", string(body))), nil
+
+				// Use the updated PR with reviewers
+				pr = updatedPR
+			}
+
+			// If no updates and no reviewers, return error
+			if !updateNeeded && len(reviewers) == 0 {
+				return mcp.NewToolResultError("No update parameters provided"), nil
 			}
 
 			r, err := json.Marshal(pr)

@@ -150,6 +150,7 @@ func Test_UpdatePullRequest(t *testing.T) {
 	assert.Contains(t, tool.InputSchema.Properties, "state")
 	assert.Contains(t, tool.InputSchema.Properties, "base")
 	assert.Contains(t, tool.InputSchema.Properties, "maintainer_can_modify")
+	assert.Contains(t, tool.InputSchema.Properties, "reviewers")
 	assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"})
 
 	// Setup mock PR for success case
@@ -171,6 +172,23 @@ func Test_UpdatePullRequest(t *testing.T) {
 		State:  github.Ptr("closed"), // State updated
 	}
 
+	// Mock PR for when there are no updates but we still need a response
+	mockNoUpdatePR := &github.PullRequest{
+		Number: github.Ptr(42),
+		Title:  github.Ptr("Test PR"),
+		State:  github.Ptr("open"),
+	}
+
+	mockPRWithReviewers := &github.PullRequest{
+		Number: github.Ptr(42),
+		Title:  github.Ptr("Test PR"),
+		State:  github.Ptr("open"),
+		RequestedReviewers: []*github.User{
+			{Login: github.Ptr("reviewer1")},
+			{Login: github.Ptr("reviewer2")},
+		},
+	}
+
 	tests := []struct {
 		name           string
 		mockedClient   *http.Client
@@ -229,8 +247,40 @@ func Test_UpdatePullRequest(t *testing.T) {
 			expectedPR:  mockClosedPR,
 		},
 		{
-			name:         "no update parameters provided",
-			mockedClient: mock.NewMockedHTTPClient(), // No API call expected
+			name: "successful PR update with reviewers",
+			mockedClient: mock.NewMockedHTTPClient(
+				mock.WithRequestMatch(
+					mock.GetReposPullsByOwnerByRepoByPullNumber,
+					&github.PullRequest{
+						Number: github.Ptr(42),
+						Title:  github.Ptr("Test PR"),
+						State:  github.Ptr("open"),
+					},
+				),
+				// Mock for RequestReviewers call, returning the PR with reviewers
+				mock.WithRequestMatch(
+					mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber,
+					mockPRWithReviewers,
+				),
+			),
+			requestArgs: map[string]interface{}{
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+				"reviewers":  []interface{}{"reviewer1", "reviewer2"},
+			},
+			expectError: false,
+			expectedPR:  mockPRWithReviewers,
+		},
+		{
+			name: "no update parameters provided",
+			mockedClient: mock.NewMockedHTTPClient(
+				// Mock a response for the GET PR request in case of no updates
+				mock.WithRequestMatch(
+					mock.GetReposPullsByOwnerByRepoByPullNumber,
+					mockNoUpdatePR,
+				),
+			),
 			requestArgs: map[string]interface{}{
 				"owner":      "owner",
 				"repo":       "repo",
@@ -260,6 +310,32 @@ func Test_UpdatePullRequest(t *testing.T) {
 			expectError:    true,
 			expectedErrMsg: "failed to update pull request",
 		},
+		{
+			name: "request reviewers fails",
+			mockedClient: mock.NewMockedHTTPClient(
+				// First it gets the PR (no fields to update)
+				mock.WithRequestMatch(
+					mock.GetReposPullsByOwnerByRepoByPullNumber,
+					mockNoUpdatePR,
+				),
+				// Then reviewer request fails
+				mock.WithRequestMatchHandler(
+					mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber,
+					http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+						w.WriteHeader(http.StatusUnprocessableEntity)
+						_, _ = w.Write([]byte(`{"message": "Invalid reviewers"}`))
+					}),
+				),
+			),
+			requestArgs: map[string]interface{}{
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+				"reviewers":  []interface{}{"invalid-user"},
+			},
+			expectError:    true,
+			expectedErrMsg: "failed to request reviewers",
+		},
 	}
 
 	for _, tc := range tests {
@@ -312,6 +388,26 @@ func Test_UpdatePullRequest(t *testing.T) {
 			if tc.expectedPR.MaintainerCanModify != nil {
 				assert.Equal(t, *tc.expectedPR.MaintainerCanModify, *returnedPR.MaintainerCanModify)
 			}
+
+			// Check reviewers if they exist in the expected PR
+			if len(tc.expectedPR.RequestedReviewers) > 0 {
+				assert.NotNil(t, returnedPR.RequestedReviewers)
+				assert.Equal(t, len(tc.expectedPR.RequestedReviewers), len(returnedPR.RequestedReviewers))
+
+				// Create maps of reviewer logins for easy comparison
+				expectedReviewers := make(map[string]bool)
+				for _, reviewer := range tc.expectedPR.RequestedReviewers {
+					expectedReviewers[*reviewer.Login] = true
+				}
+
+				actualReviewers := make(map[string]bool)
+				for _, reviewer := range returnedPR.RequestedReviewers {
+					actualReviewers[*reviewer.Login] = true
+				}
+
+				// Compare the maps
+				assert.Equal(t, expectedReviewers, actualReviewers)
+			}
 		})
 	}
 }