feat(auto-merge): Check if PR is mergeable before merging (#1035)

Author: ffflorian

packages/auto-merge/.automergerc.example.json

@@ -2,7 +2,6 @@
   "authToken": "my-secret-token",
   "autoApprove": true,
   "dryRun": false,
-  "mergeDrafts": false,
   "projects": {
     "gitHub": ["ffflorian/node-packages", "ffflorian/ts-boilerplate"]
   },

packages/auto-merge/README.md

@@ -27,7 +27,6 @@ Options:
   -a, --approve        approve before merging
   -c, --config <path>  specify a configuration file (default: .automergerc.json)
   -d, --dry-run        don't send any data
-  -f, --merge-drafts   merge draft PRs (default: false)
   -s, --squash         squash when merging (default: false)
   -V, --version        output the version number
   -h, --help           display help for command
@@ -41,20 +40,18 @@ The structure of the configuration file is the following:
 
 ```ts
 {
-  /** The GitHub auth token */
+  /** The GitHub auth token (needs read and write access to code and pull requests) */
   authToken: string;
   /** Approve before merging */
   autoApprove?: boolean;
   /** Don't send any data */
   dryRun?: boolean;
-  /** Merge draft PRs */
-  mergeDrafts?: boolean;
   /** All projects to include */
   projects: {
     /** All projects hosted on GitHub in the format `user/repo` */
     gitHub: string[];
   };
-  /* Squash when merging */
+  /** Squash when merging */
   squash?: boolean;
 }
 ```

packages/auto-merge/src/AutoMerge.test.ts

@@ -2,7 +2,8 @@ import {expect, describe, test, beforeEach, afterEach, vi} from 'vitest';
 import nock from 'nock';
 import {StatusCodes as HTTP_STATUS} from 'http-status-codes';
 
-import {AutoMerge, Repository} from './AutoMerge.js';
+import {AutoMerge} from './AutoMerge.js';
+import type {GitHubPullRequest, Repository} from './types/index.js';
 
 describe('AutoMerge', () => {
   describe('checkRepository', () => {
@@ -21,7 +22,7 @@ describe('AutoMerge', () => {
             number: 253,
             title: 'chore: bump eslint-plugin-typescript-sort-keys from 1.3.0 to 1.5.0',
           },
-        ],
+        ] as GitHubPullRequest[],
         repositorySlug: validRepositoryNames[0],
       },
       {
@@ -35,7 +36,7 @@ describe('AutoMerge', () => {
             number: 252,
             title: 'chore: bump typescript from 4.0.2 to 4.0.3',
           },
-        ],
+        ] as GitHubPullRequest[],
         repositorySlug: validRepositoryNames[1],
       },
     ];
@@ -49,12 +50,17 @@ describe('AutoMerge', () => {
       });
 
       nock(autoMerge['apiClient'].defaults.baseURL!)
-        .post(/^\/repos\/[^/]+\/[^/]+\/pulls\/\d+\/reviews\/?$/)
+        .post(/^\/repos\/.+?\/.+?\/pulls\/\d+\/reviews\/?$/)
         .reply(HTTP_STATUS.OK, {data: 'not-used'})
         .persist();
 
       nock(autoMerge['apiClient'].defaults.baseURL!)
-        .put(/^\/repos\/[^/]+\/[^/]+\/pulls\/\d+\/merge\/?$/)
+        .get(/^\/repos\/.+?\/.+?\/pulls\/\d+\/?$/)
+        .reply(HTTP_STATUS.OK, {data: 'not-used'})
+        .persist();
+
+      nock(autoMerge['apiClient'].defaults.baseURL!)
+        .put(/^\/repos\/.+?\/.+?\/pulls\/\d+\/merge\/?$/)
         .reply(HTTP_STATUS.OK, {data: 'not-used'})
         .persist();
     });

packages/auto-merge/src/AutoMerge.ts

@@ -3,6 +3,8 @@ import path from 'node:path';
 import axios, {AxiosError, AxiosInstance} from 'axios';
 import logdown from 'logdown';
 
+import type {AutoMergeConfig, ActionResult, GitHubPullRequest, Repository, RepositoryResult} from './types/index.js';
+
 interface PackageJson {
   bin: Record<string, string>;
   version: string;
@@ -14,55 +16,6 @@ const packageJsonPath = path.join(__dirname, '../package.json');
 const {bin, version: toolVersion}: PackageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8'));
 const toolName = Object.keys(bin)[0];
 
-/** @see https://docs.github.com/en/rest/reference/pulls#get-a-pull-request */
-interface GitHubPullRequest {
-  draft: boolean;
-  head: {
-    /** The branch name */
-    ref: string;
-    /** The commit SHA-1 hash */
-    sha: string;
-  };
-  /** The pull request number */
-  number: number;
-  /** The pull request title */
-  title: string;
-}
-
-export interface ActionResult {
-  error?: string;
-  pullNumber: number;
-  status: 'bad' | 'good';
-}
-
-export interface AutoMergeConfig {
-  /** The GitHub auth token */
-  authToken: string;
-  /** Approve before merging */
-  autoApprove?: boolean;
-  /** Don't send any data */
-  dryRun?: boolean;
-  /** Merge draft PRs */
-  mergeDrafts?: boolean;
-  /** All projects to include */
-  projects: {
-    /** All projects hosted on GitHub in the format `user/repo` */
-    gitHub: string[];
-  };
-  /** Squash when merging */
-  squash?: boolean;
-}
-
-export interface Repository {
-  pullRequests: GitHubPullRequest[];
-  repositorySlug: string;
-}
-
-export interface RepositoryResult {
-  actionResults: ActionResult[];
-  repositorySlug: string;
-}
-
 export class AutoMerge {
   private readonly apiClient: AxiosInstance;
   private readonly config: AutoMergeConfig;
@@ -78,6 +31,7 @@ export class AutoMerge {
     this.apiClient = axios.create({
       baseURL: 'https://api.github.com',
       headers: {
+        Accept: 'application/vnd.github+json',
         Authorization: `token ${this.config.authToken}`,
         'User-Agent': `${toolName} v${toolVersion}`,
       },
@@ -110,44 +64,56 @@ export class AutoMerge {
     const allRepositories = repositories || (await this.getRepositoriesWithOpenPullRequests());
     const matchingRepositories = this.getMatchingRepositories(allRepositories, regex);
 
-    const resultPromises = matchingRepositories.map(async ({pullRequests, repositorySlug}) => {
-      const actionPromises = pullRequests.map(pullRequest =>
-        this.approveByPullNumber(repositorySlug, pullRequest.number)
-      );
-      const actionResults = await Promise.all(actionPromises);
-      return {actionResults, repositorySlug};
-    });
+    const processedRepositories: RepositoryResult[] = [];
+    for (const {pullRequests, repositorySlug} of matchingRepositories) {
+      const actionResults: ActionResult[] = [];
+      for (const pullRequest of pullRequests) {
+        actionResults.push(await this.approveByPullNumber(repositorySlug, pullRequest.number));
+      }
+      processedRepositories.push({actionResults, repositorySlug});
+    }
 
-    return Promise.all(resultPromises);
+    return processedRepositories;
   }
 
   private getMatchingRepositories(repositories: Repository[], regex: RegExp): Repository[] {
-    return repositories
-      .map(repository => {
-        const matchingPullRequests = repository.pullRequests.filter(pullRequest =>
-          new RegExp(regex).test(pullRequest.head.ref)
-        );
-        if (matchingPullRequests.length) {
-          return {pullRequests: matchingPullRequests, repositorySlug: repository.repositorySlug};
-        }
-        return undefined;
-      })
-      .filter(Boolean) as Repository[];
+    const matchingRepositories: Repository[] = [];
+    for (const repository of repositories) {
+      const matchingPullRequests = repository.pullRequests.filter(pullRequest =>
+        new RegExp(regex).test(pullRequest.head.ref)
+      );
+      if (matchingPullRequests.length) {
+        matchingRepositories.push({pullRequests: matchingPullRequests, repositorySlug: repository.repositorySlug});
+      }
+    }
+    return matchingRepositories;
+  }
+
+  private async isPullRequestMergeable(repositorySlug: string, pullNumber: number): Promise<boolean> {
+    const resourceUrl = `/repos/${repositorySlug}/pulls/${pullNumber}`;
+    const response = await this.apiClient.get<GitHubPullRequest>(resourceUrl);
+    return response.data.mergeable_state === 'clean';
   }
 
   async mergeByMatch(regex: RegExp, repositories?: Repository[]): Promise<RepositoryResult[]> {
     const allRepositories = repositories || (await this.getRepositoriesWithOpenPullRequests());
     const matchingRepositories = this.getMatchingRepositories(allRepositories, regex);
 
-    const resultPromises = matchingRepositories.map(async ({pullRequests, repositorySlug}) => {
-      const actionPromises = pullRequests.map(pullRequest =>
-        this.mergePullRequest(repositorySlug, pullRequest.number, this.config.squash)
-      );
-      const actionResults = await Promise.all(actionPromises);
-      return {actionResults, repositorySlug};
-    });
+    const processedRepositories: RepositoryResult[] = [];
+    for (const {pullRequests, repositorySlug} of matchingRepositories) {
+      const actionResults: ActionResult[] = [];
+      for (const pullRequest of pullRequests) {
+        const isMergeable = this.isPullRequestMergeable(repositorySlug, pullRequest.number);
+        if (!isMergeable) {
+          this.logger.warn(`Pull request #${pullRequest.number} in "${repositorySlug}" is not mergeable. Skipping.`);
+          continue;
+        }
+        actionResults.push(await this.mergePullRequest(repositorySlug, pullRequest.number, this.config.squash));
+      }
+      processedRepositories.push({actionResults, repositorySlug});
+    }
 
-    return Promise.all(resultPromises);
+    return processedRepositories;
   }
 
   async approveByPullNumber(repositorySlug: string, pullNumber: number): Promise<ActionResult> {
@@ -189,17 +155,18 @@ export class AutoMerge {
       this.checkRepositorySlug(repositorySlug)
     );
 
-    const repositoriesPromises = repositorySlugs.map(async repositorySlug => {
+    const repositories: Repository[] = [];
+
+    for (const repositorySlug of repositorySlugs) {
       try {
         const pullRequests = await this.getPullRequestsBySlug(repositorySlug);
-        return {pullRequests, repositorySlug};
+        repositories.push({pullRequests, repositorySlug});
       } catch (error) {
         this.logger.error(`Could not get pull requests for "${repositorySlug}": ${(error as AxiosError).message}`);
-        return undefined;
       }
-    });
+    }
 
-    return (await Promise.all(repositoriesPromises)).filter(Boolean) as Repository[];
+    return repositories;
   }
 
   async getRepositoriesWithOpenPullRequests(): Promise<Repository[]> {
@@ -221,11 +188,8 @@ export class AutoMerge {
 
   private async getPullRequestsBySlug(repositorySlug: string): Promise<GitHubPullRequest[]> {
     const resourceUrl = `/repos/${repositorySlug}/pulls`;
-    const params = {state: 'open'};
+    const params = {per_page: 100, state: 'open'};
     const response = await this.apiClient.get<GitHubPullRequest[]>(resourceUrl, {params});
-    if (this.config.mergeDrafts) {
-      response.data = response.data.filter(pr => !pr.draft);
-    }
     return response.data;
   }
 }

packages/auto-merge/src/cli.ts

@@ -7,7 +7,8 @@ import {program as commander} from 'commander';
 import {cosmiconfigSync} from 'cosmiconfig';
 import logdown from 'logdown';
 
-import {AutoMergeConfig, AutoMerge, Repository, RepositoryResult} from './AutoMerge.js';
+import {AutoMerge} from './AutoMerge.js';
+import type {AutoMergeConfig, Repository, RepositoryResult} from './types/index.js';
 import {pluralize} from './util.js';
 
 const input = readline.createInterface(process.stdin, process.stdout);
@@ -52,7 +53,6 @@ const configFileData: AutoMergeConfig = {
   ...configResult.config,
   ...(commanderOptions.approve && {autoApprove: commanderOptions.approve}),
   ...(commanderOptions.dryRun && {dryRun: commanderOptions.dryRun}),
-  ...(commanderOptions.mergeDrafts && {mergeDrafts: commanderOptions.mergeDrafts}),
 };
 
 async function runAction(autoMerge: AutoMerge, repositories: Repository[], pullRequestSlug: string): Promise<void> {
@@ -67,7 +67,7 @@ async function runAction(autoMerge: AutoMerge, repositories: Repository[], pullR
   const mergeResults = await autoMerge.mergeByMatch(regex, repositories);
 
   const successCount = [...approveResults, ...mergeResults].filter(repository => {
-    return repository.actionResults.some(result => result.error === undefined);
+    return repository.actionResults.some(result => typeof result.error === 'undefined');
   }).length;
 
   const prPluralized = pluralize('PR', successCount);

packages/auto-merge/src/types/ActionResult.d.ts

@@ -0,0 +1,5 @@
+export interface ActionResult {
+  error?: string;
+  pullNumber: number;
+  status: 'bad' | 'good';
+}

packages/auto-merge/src/types/AutoMergeConfig.d.ts

@@ -0,0 +1,15 @@
+export interface AutoMergeConfig {
+  /** The GitHub auth token (needs read and write access to code and pull requests) */
+  authToken: string;
+  /** Approve before merging */
+  autoApprove?: boolean;
+  /** Don't send any data */
+  dryRun?: boolean;
+  /** All projects to include */
+  projects: {
+    /** All projects hosted on GitHub in the format `user/repo` */
+    gitHub: string[];
+  };
+  /** Squash when merging */
+  squash?: boolean;
+}