Prevent Non Collaborator Aliases Version 2: Using Permission API and …

…Batch Call (#6)

In a previous PR (#5), the `request reviewer` call was converted from a batch call to multiple individual calls in order to support cases where an alias no longer has access to the repo.

In very rare circumstances, it has been reported that the action will actually remove add (and then automatically get removed) from the pull request. Since the action doesn't invoke any `remove reviewer` APIs, the only theory so far is that making multiple edit request simultaneously has exposed a rare syncronous problem, usually when combined with [auto assignment](https://docs.github.com/en/organizations/organizing-members-into-teams/managing-code-review-settings-for-your-team#about-auto-assignment). This scenario is rare enough that it has been difficult to get reproductions of the issue in test environments.

The proposed solution is to instead async request the permission status of every alias the action will attempt to add. After locally filtering out all aliases that do not have permissions, make a batch request with all of the remaining aliases. This adds 1 extra network call compared to the solution in 5, but reduces the number of network calls attempting to edit the PR down to 1.

src/github.js

@@ -163,39 +163,72 @@ async function fetch_reviewers() {
   return [ ...reviewers ];
 }
 
-async function assign_reviewers(reviewers) {
+async function filter_only_collaborators(reviewers) {
   const context = get_context();
   const octokit = get_octokit();
 
   const [ teams_with_prefix, individuals ] = partition(reviewers, (reviewer) => reviewer.startsWith('team:'));
   const teams = teams_with_prefix.map((team_with_prefix) => team_with_prefix.replace('team:', ''));
 
-  const request_review_responses = [];
-
-  // Github's requestReviewers API will fail to add all reviewers if any of the aliases are not collaborators.
-  // Github also does not support a batch call to determine which aliases in the list are not collaborators.
-
-  // We therefore make each call individually so that we add all reviewers that are collaborators,
-  // and log failure for aliases that no longer have access.
+  // Create a list of requests for all available aliases and teams to see if they have permission
+  // to the PR associated with this action
+  const collaborator_responses = [];
   teams.forEach((team) => {
-    request_review_responses.push(octokit.pulls.requestReviewers({
+    collaborator_responses.push(octokit.teams.checkPermissionsForRepoInOrg({
+      org: context.repo.owner,
+      team_slug: team,
       owner: context.repo.owner,
       repo: context.repo.repo,
-      pull_number: context.payload.pull_request.number,
-      team_reviewers: [ team ],
+    }).then((response) => {
+      // https://docs.github.com/en/rest/teams/teams?apiVersion=2022-11-28#check-team-permissions-for-a-repository
+      // Its expected that a team with permission will return 204
+      core.info(`Received successful status code ${response?.status ?? 'Unknown'} for team: ${team}`);
+      return 'team:'.concat(team);
     }).catch((error) => core.error(`Team: ${team} failed to be added with error: ${error}`)));
   });
-
-  individuals.forEach((login) => {
-    request_review_responses.push(octokit.pulls.requestReviewers({
+  individuals.forEach((alias) => {
+    collaborator_responses.push(octokit.repos.checkCollaborator({
       owner: context.repo.owner,
       repo: context.repo.repo,
-      pull_number: context.payload.pull_request.number,
-      reviewers: [ login ],
-    }).catch((error) => core.error(`Individual: ${login} failed to be added with error: ${error}`)));
+      username: alias,
+    }).then((response) => {
+      // https://docs.github.com/en/rest/collaborators/collaborators?apiVersion=2022-11-28#check-if-a-user-is-a-repository-collaborator
+      // Its expected that a collaborator with permission will return 204
+      core.info(`Received successful status code ${response?.status ?? 'Unknown'} for alias: ${alias}`);
+      return alias;
+    }).catch((error) => core.error(`Individual: ${alias} failed to be added with error: ${error}`)));
+  });
+
+  // Store the aliases and teams of all successful responses
+  const collaborators = [];
+  await Promise.allSettled(collaborator_responses).then((results) => {
+    results.forEach((result) => {
+      if (result?.value) {
+        collaborators.push(result?.value);
+      }
+    });
   });
 
-  return Promise.allSettled(request_review_responses);
+  // Only include aliases and teams that exist as collaborators
+  const filtered_reviewers = reviewers.filter((reviewer) => collaborators.includes(reviewer));
+  core.info(`Filtered list of only collaborators ${filtered_reviewers.join(', ')}`);
+  return filtered_reviewers;
+}
+
+async function assign_reviewers(reviewers) {
+  const context = get_context();
+  const octokit = get_octokit();
+
+  const [ teams_with_prefix, individuals ] = partition(reviewers, (reviewer) => reviewer.startsWith('team:'));
+  const teams = teams_with_prefix.map((team_with_prefix) => team_with_prefix.replace('team:', ''));
+
+  return octokit.pulls.requestReviewers({
+    owner: context.repo.owner,
+    repo: context.repo.repo,
+    pull_number: context.payload.pull_request.number,
+    reviewers: individuals,
+    team_reviewers: teams,
+  });
 }
 
 /* Private */
@@ -246,6 +279,7 @@ module.exports = {
   fetch_config,
   fetch_changed_files,
   fetch_reviewers,
+  filter_only_collaborators,
   assign_reviewers,
   clear_cache,
 };

src/index.js

@@ -75,6 +75,9 @@ async function run() {
   core.info(`Possible Reviewers ${reviewers.join(', ')}, prepare filtering out already requested reviewers or approved reviewers`);
   reviewers = reviewers.filter((reviewer) => !requested_approved_reviewers.includes(reviewer));
 
+  core.info(`Possible New Reviewers ${reviewers.join(', ')}, prepare to filter to only collaborators`);
+  reviewers = await github.filter_only_collaborators(reviewers);
+
   core.info('Randomly picking reviewers if the number of reviewers is set');
   reviewers = randomly_pick_reviewers({ reviewers, config });