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,11 @@ 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',
+    labels: [{pattern: 'merge: squash commits', method: 'squash'}],
+  },
+  conditionalAutosquashMerge: true,
   requiredStatuses: [{name: 'test', type: 'check'}],
 
   // Disable target labeling in the dev-infra repo as we don't have

.ng-dev/pull-request.mts

@@ -23,7 +23,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.
@@ -36,6 +36,13 @@ export interface PullRequestConfig {
   /** List of statuses that are required before a pull request can be merged. */
   requiredStatuses?: {type: 'check' | 'status'; name: string}[];
 
+  /**
+   * Whether pull requests should be merged using a conditional autosquash strategy.
+   * If a pull request contains fixup or squash commits, the autosquash strategy
+   * will be used. Otherwise, the Github API merge strategy will be used.
+   */
+  conditionalAutosquashMerge?: boolean;
+
   /**
    * Whether pull requests should be merged using the Github API. This can be enabled
    * if projects want to have their pull requests show up as `Merged` in the Github UI.
@@ -63,7 +70,7 @@ export interface PullRequestConfig {
    * follow the canonical branching/versioning.
    */
   __noTargetLabeling?: boolean;
-}
+};
 
 /** Loads and validates the merge configuration. */
 export function assertValidPullRequestConfig<T extends NgDevConfig>(
@@ -76,10 +83,18 @@ export function assertValidPullRequestConfig<T extends NgDevConfig>(
     );
   }
 
-  if (config.pullRequest.githubApiMerge === undefined) {
+  const {conditionalAutosquashMerge, githubApiMerge} = config.pullRequest;
+  if (githubApiMerge === undefined) {
     errors.push('No explicit choice of merge strategy. Please set `githubApiMerge`.');
   }
 
+  if (conditionalAutosquashMerge && !githubApiMerge) {
+    errors.push(
+      '`conditionalAutosquashMerge` requires a GitHub API merge strategy to inspect commit history. ' +
+        'Please configure `githubApiMerge` or disable `conditionalAutosquashMerge`.',
+    );
+  }
+
   if (errors.length) {
     throw new ConfigValidationError('Invalid `pullRequest` configuration', errors);
   }

ng-dev/pr/config/index.ts

@@ -19,6 +19,7 @@ import {
 import {loadAndValidatePullRequest, PullRequest} from './pull-request.js';
 import {GithubApiMergeStrategy} from './strategies/api-merge.js';
 import {AutosquashMergeStrategy} from './strategies/autosquash-merge.js';
+import {ConditionalAutosquashMergeStrategy} from './strategies/conditional-autosquash-merge.js';
 import {GithubConfig, NgDevConfig} from '../../utils/config.js';
 import {assertValidReleaseConfig} from '../../release/config/index.js';
 import {
@@ -139,9 +140,7 @@ export class MergeTool {
       throw new UserAbortedMergeToolError();
     }
 
-    const strategy = this.config.pullRequest.githubApiMerge
-      ? new GithubApiMergeStrategy(this.git, this.config.pullRequest.githubApiMerge)
-      : new AutosquashMergeStrategy(this.git);
+    const strategy = this.getMergeStrategy();
 
     // Branch or revision that is currently checked out so that we can switch back to
     // it once the pull request has been merged.
@@ -194,6 +193,23 @@ export class MergeTool {
     }
   }
 
+  private getMergeStrategy():
+    | AutosquashMergeStrategy
+    | ConditionalAutosquashMergeStrategy
+    | GithubApiMergeStrategy {
+    const {conditionalAutosquashMerge, githubApiMerge} = this.config.pullRequest;
+
+    if (conditionalAutosquashMerge && githubApiMerge) {
+      return new ConditionalAutosquashMergeStrategy(this.git, githubApiMerge);
+    }
+
+    if (githubApiMerge) {
+      return new GithubApiMergeStrategy(this.git, githubApiMerge);
+    }
+
+    return new AutosquashMergeStrategy(this.git);
+  }
+
   /**
    * Modifies the pull request in place with new target branches based on user
    * selection from the available active branches.

ng-dev/pr/merge/merge-tool.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';
 
@@ -52,7 +48,7 @@ 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);
 
     const mergeOptions: OctokitMergeParams = {
@@ -195,10 +191,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,17 +200,8 @@ 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 {
+  getMergeActionFromPullRequest({labels}: PullRequest): GithubApiMergeMethod {
     if (this._config.labels) {
       const matchingLabel = this._config.labels.find(({pattern}) => labels.includes(pattern));
       if (matchingLabel !== undefined) {

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

@@ -0,0 +1,49 @@
+/**
+ * @license
+ * Copyright Google LLC
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
+import {GithubApiMergeStrategyConfig} from '../../config/index.js';
+import {PullRequest} from '../pull-request.js';
+
+import {AutosquashMergeStrategy} from './autosquash-merge.js';
+import {GithubApiMergeStrategy} from './api-merge.js';
+import {MergeStrategy} from './strategy.js';
+
+/**
+ * Merge strategy that conditionally uses autosquash or the Github API merge.
+ * If a pull request contains fixup or squash commits, the autosquash strategy
+ * will be used. Otherwise, the Github API merge strategy will be used.
+ */
+export class ConditionalAutosquashMergeStrategy extends MergeStrategy {
+  private readonly githubApiMergeStrategy: GithubApiMergeStrategy;
+
+  constructor(
+    git: AuthenticatedGitClient,
+    private config: GithubApiMergeStrategyConfig,
+  ) {
+    super(git);
+    this.githubApiMergeStrategy = new GithubApiMergeStrategy(this.git, this.config);
+  }
+
+  override async merge(pullRequest: PullRequest): Promise<void> {
+    const mergeAction = this.githubApiMergeStrategy.getMergeActionFromPullRequest(pullRequest);
+
+    // Squash and Merge will create a single commit message and thus we can use the API to merge.
+    return mergeAction === 'rebase' &&
+      (pullRequest.needsCommitMessageFixup || (await this.hasFixupOrSquashCommits(pullRequest)))
+      ? new AutosquashMergeStrategy(this.git).merge(pullRequest)
+      : this.githubApiMergeStrategy.merge(pullRequest);
+  }
+
+  /** 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/conditional-autosquash-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),
+    }));
+  }
 }