From 1cfcca3a9cce7f84c620d699bae7fcf64d3d3363 Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Thu, 21 May 2026 03:46:36 +0800 Subject: [PATCH] fix: support team pull request reviewers --- README.md | 2 +- .../request_pull_request_reviewers.snap | 4 ++-- .../__toolsnaps__/update_pull_request.snap | 4 ++-- pkg/github/granular_tools_test.go | 7 ++++-- pkg/github/pullrequests.go | 6 +++-- pkg/github/pullrequests_granular.go | 24 +++++++++++++++++-- pkg/github/pullrequests_test.go | 18 ++++++++++++++ 7 files changed, 54 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index b4a5927b1b..d94e703910 100644 --- a/README.md +++ b/README.md @@ -1155,7 +1155,7 @@ The following sets of tools are available: - `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) + - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional) - `state`: New state (string, optional) - `title`: New title (string, optional) diff --git a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap index 67b7014474..7e6d33a274 100644 --- a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap +++ b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap @@ -21,7 +21,7 @@ "type": "string" }, "reviewers": { - "description": "GitHub usernames to request reviews from", + "description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from", "items": { "type": "string" }, @@ -37,4 +37,4 @@ "type": "object" }, "name": "request_pull_request_reviewers" -} \ No newline at end of file +} diff --git a/pkg/github/__toolsnaps__/update_pull_request.snap b/pkg/github/__toolsnaps__/update_pull_request.snap index ef330188ff..640df79702 100644 --- a/pkg/github/__toolsnaps__/update_pull_request.snap +++ b/pkg/github/__toolsnaps__/update_pull_request.snap @@ -34,7 +34,7 @@ "type": "string" }, "reviewers": { - "description": "GitHub usernames to request reviews from", + "description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from", "items": { "type": "string" }, @@ -61,4 +61,4 @@ "type": "object" }, "name": "update_pull_request" -} \ No newline at end of file +} diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 59eb478224..91b6b044be 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -635,7 +635,10 @@ func TestGranularUpdatePullRequestState(t *testing.T) { func TestGranularRequestPullRequestReviewers(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)}), + PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "reviewers": []any{"user1"}, + "team_reviewers": []any{"team1"}, + }).andThen(mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)})), })) deps := BaseDeps{Client: client} serverTool := GranularRequestPullRequestReviewers(translations.NullTranslationHelper) @@ -645,7 +648,7 @@ func TestGranularRequestPullRequestReviewers(t *testing.T) { "owner": "owner", "repo": "repo", "pullNumber": float64(1), - "reviewers": []string{"user1", "user2"}, + "reviewers": []string{"user1", "owner/team1"}, }) result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 9672f85244..e08119dc8a 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -752,7 +752,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo }, "reviewers": { Type: "array", - Description: "GitHub usernames to request reviews from", + Description: "GitHub usernames or ORG/team-slug team reviewers to request reviews from", Items: &jsonschema.Schema{ Type: "string", }, @@ -944,8 +944,10 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + userReviewers, teamReviewers := splitPullRequestReviewers(reviewers) reviewersRequest := github.ReviewersRequest{ - Reviewers: reviewers, + Reviewers: userReviewers, + TeamReviewers: teamReviewers, } _, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, reviewersRequest) diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index 30d7f78d62..6bc2b99f36 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -297,7 +297,7 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, "reviewers": { Type: "array", - Description: "GitHub usernames to request reviews from", + Description: "GitHub usernames or ORG/team-slug team reviewers to request reviews from", Items: &jsonschema.Schema{Type: "string"}, }, }, @@ -325,13 +325,17 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i if len(reviewers) == 0 { return utils.NewToolResultError("missing required parameter: reviewers"), nil, nil } + userReviewers, teamReviewers := splitPullRequestReviewers(reviewers) client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{Reviewers: reviewers}) + pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{ + Reviewers: userReviewers, + TeamReviewers: teamReviewers, + }) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to request reviewers", resp, err), nil, nil } @@ -351,6 +355,22 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i return st } +func splitPullRequestReviewers(reviewers []string) ([]string, []string) { + userReviewers := make([]string, 0, len(reviewers)) + teamReviewers := make([]string, 0) + + for _, reviewer := range reviewers { + org, team, ok := strings.Cut(reviewer, "/") + if ok && org != "" && team != "" && !strings.Contains(team, "/") { + teamReviewers = append(teamReviewers, team) + continue + } + userReviewers = append(userReviewers, reviewer) + } + + return userReviewers, teamReviewers +} + // GranularCreatePullRequestReview creates a tool to create a PR review. func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index a73ba2e173..b144e8c51f 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -257,6 +257,24 @@ func Test_UpdatePullRequest(t *testing.T) { expectError: false, expectedPR: mockPRWithReviewers, }, + { + name: "successful PR update with user and team reviewers", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "reviewers": []any{"reviewer1"}, + "team_reviewers": []any{"platform"}, + }).andThen(mockResponse(t, http.StatusOK, mockPRWithReviewers)), + GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPRWithReviewers), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "reviewers": []any{"reviewer1", "owner/platform"}, + }, + expectError: false, + expectedPR: mockPRWithReviewers, + }, { name: "successful PR update (title only)", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{