feat(ng-dev): add conditional autosquash merge strategy

Introduces a new conditional autosquash merge strategy. This strategy uses the autosquash merge strategy if the pull request contains fixup or squash commits, and the GitHub API merge strategy otherwise.

This allows pull requests that do not need to be squashed to be closed as "merged" on GitHub, rather than "closed".

Author: alan-agius4

.github/local-actions/branch-manager/main.js

@@ -2,7 +2,10 @@ import {PullRequestConfig} from '../ng-dev/pr/config/index.js';
 
 /** Configuration for interacting with pull requests in the repo. */
 export const pullRequest: PullRequestConfig = {
-  githubApiMerge: false,
+  githubApiMerge: {
+    default: 'rebase-with-fixup',
+    labels: [{pattern: 'merge: squash commits', method: 'squash'}],
+  },
   requiredStatuses: [{name: 'test', type: 'check'}],
 
   // Disable target labeling in the dev-infra repo as we don't have

.ng-dev/pull-request.mts

@@ -8,11 +8,14 @@
 
 import {ConfigValidationError, GithubConfig, NgDevConfig} from '../../utils/config.js';
 
+// TODO(alanagius): remove `rebase-with-fixup` and replace it's logic with `rebase`.
+// This is just temporary to allow testing in the dev-infra repo. Without breaking workflows in other repos.
+
 /**
  * Possible merge methods supported by the Github API.
  * https://developer.github.com/v3/pulls/#merge-a-pull-request-merge-button.
  */
-export type GithubApiMergeMethod = 'merge' | 'squash' | 'rebase';
+export type GithubApiMergeMethod = 'merge' | 'squash' | 'rebase' | 'rebase-with-fixup';
 
 /** Configuration for the Github API merge strategy. */
 export interface GithubApiMergeStrategyConfig {
@@ -23,7 +26,7 @@ export interface GithubApiMergeStrategyConfig {
 }
 
 /** Configuration for the merge script. */
-export interface PullRequestConfig {
+export type PullRequestConfig = {
   /**
    * Configuration for the upstream remote. All of these options are optional as
    * defaults are provided by the common dev-infra github configuration.
@@ -63,7 +66,7 @@ export interface PullRequestConfig {
    * follow the canonical branching/versioning.
    */
   __noTargetLabeling?: boolean;
-}
+};
 
 /** Loads and validates the merge configuration. */
 export function assertValidPullRequestConfig<T extends NgDevConfig>(
@@ -76,7 +79,8 @@ export function assertValidPullRequestConfig<T extends NgDevConfig>(
     );
   }
 
-  if (config.pullRequest.githubApiMerge === undefined) {
+  const {githubApiMerge} = config.pullRequest;
+  if (githubApiMerge === undefined) {
     errors.push('No explicit choice of merge strategy. Please set `githubApiMerge`.');
   }
 

ng-dev/pr/config/index.ts

@@ -8,7 +8,6 @@
 
 import {RestEndpointMethodTypes} from '@octokit/plugin-rest-endpoint-methods';
 
-import {parseCommitMessage} from '../../../commit-message/parse.js';
 import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
 import {GithubApiMergeMethod, GithubApiMergeStrategyConfig} from '../../config/index.js';
 import {PullRequest} from '../pull-request.js';
@@ -21,9 +20,6 @@ import {Prompt} from '../../../utils/prompt.js';
 /** Type describing the parameters for the Octokit `merge` API endpoint. */
 type OctokitMergeParams = RestEndpointMethodTypes['pulls']['merge']['parameters'];
 
-type OctokitPullRequestCommitsList =
-  RestEndpointMethodTypes['pulls']['listCommits']['response']['data'];
-
 /** Separator between commit message header and body. */
 const COMMIT_HEADER_SEPARATOR = '\n\n';
 
@@ -37,7 +33,7 @@ const COMMIT_HEADER_SEPARATOR = '\n\n';
 export class GithubApiMergeStrategy extends MergeStrategy {
   constructor(
     git: AuthenticatedGitClient,
-    private _config: GithubApiMergeStrategyConfig,
+    private config: GithubApiMergeStrategyConfig,
   ) {
     super(git);
   }
@@ -52,12 +48,20 @@ export class GithubApiMergeStrategy extends MergeStrategy {
    */
   override async merge(pullRequest: PullRequest): Promise<void> {
     const {githubTargetBranch, prNumber, needsCommitMessageFixup, targetBranches} = pullRequest;
-    const method = this._getMergeActionFromPullRequest(pullRequest);
+    const method = this.getMergeActionFromPullRequest(pullRequest);
     const cherryPickTargetBranches = targetBranches.filter((b) => b !== githubTargetBranch);
 
+    // Squash and Merge will create a single commit message and thus we can use the API to merge.
+    if (
+      method === 'rebase-with-fixup' &&
+      (pullRequest.needsCommitMessageFixup || (await this.hasFixupOrSquashCommits(pullRequest)))
+    ) {
+      return new GithubApiMergeStrategy(this.git, this.config).merge(pullRequest);
+    }
+
     const mergeOptions: OctokitMergeParams = {
       pull_number: prNumber,
-      merge_method: method,
+      merge_method: method === 'rebase-with-fixup' ? 'rebase' : method,
       ...this.git.remoteParams,
     };
 
@@ -195,10 +199,7 @@ export class GithubApiMergeStrategy extends MergeStrategy {
    * behavior here so that we have a default commit message that can be fixed up.
    */
   private async _getDefaultSquashCommitMessage(pullRequest: PullRequest): Promise<string> {
-    const commits = (await this._getPullRequestCommitMessages(pullRequest)).map((message) => ({
-      message,
-      parsed: parseCommitMessage(message),
-    }));
+    const commits = await this.getPullRequestCommits(pullRequest);
     const messageBase = `${pullRequest.title}${COMMIT_HEADER_SEPARATOR}`;
     if (commits.length <= 1) {
       return `${messageBase}${commits[0].parsed.body}`;
@@ -207,23 +208,21 @@ export class GithubApiMergeStrategy extends MergeStrategy {
     return `${messageBase}${joinedMessages}`;
   }
 
-  /** Gets all commit messages of commits in the pull request. */
-  private async _getPullRequestCommitMessages({prNumber}: PullRequest) {
-    const allCommits = await this.git.github.paginate(this.git.github.pulls.listCommits, {
-      ...this.git.remoteParams,
-      pull_number: prNumber,
-    });
-    return allCommits.map(({commit}) => commit.message);
-  }
-
   /** Determines the merge action from the given pull request. */
-  private _getMergeActionFromPullRequest({labels}: PullRequest): GithubApiMergeMethod {
-    if (this._config.labels) {
-      const matchingLabel = this._config.labels.find(({pattern}) => labels.includes(pattern));
+  getMergeActionFromPullRequest({labels}: PullRequest): GithubApiMergeMethod {
+    if (this.config.labels) {
+      const matchingLabel = this.config.labels.find(({pattern}) => labels.includes(pattern));
       if (matchingLabel !== undefined) {
         return matchingLabel.method;
       }
     }
-    return this._config.default;
+    return this.config.default;
+  }
+
+  /** Checks whether the pull request contains fixup or squash commits. */
+  private async hasFixupOrSquashCommits(pullRequest: PullRequest): Promise<boolean> {
+    const commits = await this.getPullRequestCommits(pullRequest);
+
+    return commits.some(({parsed: {isFixup, isSquash}}) => isFixup || isSquash);
   }
 }

ng-dev/pr/merge/strategies/api-merge.ts

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {Commit, parseCommitMessage} from '../../../commit-message/parse.js';
 import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
 import {
   FatalMergeToolError,
@@ -200,4 +201,22 @@ export abstract class MergeStrategy {
       throw new MergeConflictsFatalError(failedBranches);
     }
   }
+
+  /** Gets all commit messages of commits in the pull request. */
+  protected async getPullRequestCommits({prNumber}: PullRequest): Promise<
+    {
+      message: string;
+      parsed: Commit;
+    }[]
+  > {
+    const allCommits = await this.git.github.paginate(this.git.github.pulls.listCommits, {
+      ...this.git.remoteParams,
+      pull_number: prNumber,
+    });
+
+    return allCommits.map(({commit: {message}}) => ({
+      message,
+      parsed: parseCommitMessage(message),
+    }));
+  }
 }