Apr 15, 2025 · Apr 17, 2025 · Apr 17, 2025 · Apr 18, 2025 · Apr 22, 2025 · Apr 22, 2025
diff --git a/README.md b/README.md
  - `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)

diff --git a/pkg/github/__toolsnaps__/update_pull_request.snap b/pkg/github/__toolsnaps__/update_pull_request.snap
        "description": "Repository name",
        "type": "string"
      },
      "reviewers": {
        "description": "GitHub usernames to request reviews from",
        "items": {
          "type": "string"
        },
        "type": "array"
      },
      "state": {
        "description": "New state",
        "enum": [
diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go
 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")
 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)
diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go
 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
 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
 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",
 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 {
 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)
 }
 })
 }
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -710,6 +710,7 @@ The following sets of tools are available (all are on by default):
		- `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)

Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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": [
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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")
Expand DownExpand Up		@@ -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)
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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
Expand All		@@ -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
Expand DownExpand Up		@@ -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",
Expand DownExpand Up		@@ -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 {
Expand DownExpand Up		@@ -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)
		}
		})
		}
		}
Expand Down