feat(ng-dev): add auto merge strategy for pull requests

alan-agius4 · alan-agius4 · commit 9b6f2b1301e6 · 2025-10-23T11:15:07.000Z
This commit introduces a new `auto` merge strategy for pull requests in `ng-dev`.
When the `auto` merge method is used, the merge strategy will automatically determine the best merge method based on the pull request's commits.

The new `auto` merge strategy can:
-   Delegate to the autosquash merge strategy if the PR has fixup/squash commits against multiple normal commits.
-   Squash commits if the PR has only one normal commit and some fixup/squash commits.
-   Rebase commits if the PR has no fixup/squash commits.

This improves the developer experience by automating the merge process and ensuring that PRs are merged in the most appropriate way.
diff --git a/.ng-dev/pull-request.mts b/.ng-dev/pull-request.mts
@@ -3,7 +3,7 @@ import {PullRequestConfig} from '../ng-dev/pr/config/index.js';
 /** Configuration for interacting with pull requests in the repo. */
 export const pullRequest: PullRequestConfig = {
   githubApiMerge: {
-    default: 'rebase-with-fixup',
+    default: 'auto',
     labels: [{pattern: 'merge: squash commits', method: 'squash'}],
   },
   requiredStatuses: [{name: 'test', type: 'check'}],
diff --git a/ng-dev/pr/config/index.ts b/ng-dev/pr/config/index.ts
@@ -8,14 +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.
+ *
+ * `auto` is a pseudo merge method that is not supported by the Github API. It allows
+ * the merge strategy to automatically determine the merge method based on the pull request.
  */
-export type GithubApiMergeMethod = 'merge' | 'squash' | 'rebase' | 'rebase-with-fixup';
+export type GithubApiMergeMethod = 'merge' | 'squash' | 'rebase' | 'auto';
 
 /** Configuration for the Github API merge strategy. */
 export interface GithubApiMergeStrategyConfig {
diff --git a/ng-dev/pr/merge/strategies/api-merge.ts b/ng-dev/pr/merge/strategies/api-merge.ts
@@ -16,19 +16,29 @@ import {isGithubApiError} from '../../../utils/git/github.js';
 import {FatalMergeToolError, MergeConflictsFatalError} from '../failures.js';
 import {Prompt} from '../../../utils/prompt.js';
 import {AutosquashMergeStrategy} from './autosquash-merge.js';
+import {Commit, parseCommitMessage} from '../../../commit-message/parse.js';
 
 /** Type describing the parameters for the Octokit `merge` API endpoint. */
 type OctokitMergeParams = RestEndpointMethodTypes['pulls']['merge']['parameters'];
 
 /** Separator between commit message header and body. */
 const COMMIT_HEADER_SEPARATOR = '\n\n';
 
+/** Interface describing a pull request commit. */
+interface PullRequestCommit {
+  message: string;
+  parsed: Commit;
+}
+
 /**
  * Merge strategy that primarily leverages the Github API. The strategy merges a given
  * pull request into a target branch using the API. This ensures that Github displays
  * the pull request as merged. The merged commits are then cherry-picked into the remaining
  * target branches using the local Git instance. The benefit is that the Github merged state
- * is properly set, but a notable downside is that PRs cannot use fixup or squash commits.
+ * is properly set.
+ *
+ * A notable downside is that fixup or squash commits are not supported when `auto` merge
+ * method is not used, as the Github API does not support this.
  */
 export class GithubApiMergeStrategy extends AutosquashMergeStrategy {
   constructor(
@@ -48,23 +58,50 @@ export class GithubApiMergeStrategy extends AutosquashMergeStrategy {
    */
   override async merge(pullRequest: PullRequest): Promise<void> {
     const {githubTargetBranch, prNumber, needsCommitMessageFixup, targetBranches} = 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 super.merge(pullRequest);
-    }
+    const commits = await this.getPullRequestCommits(pullRequest);
+    const {squashCount, fixupCount, normalCommitsCount} = await this.getCommitsInfo(pullRequest);
+    const method = this.getMergeActionFromPullRequest(pullRequest);
 
     const mergeOptions: OctokitMergeParams = {
       pull_number: prNumber,
-      merge_method: method === 'rebase-with-fixup' ? 'rebase' : method,
+      merge_method: method === 'auto' ? 'rebase' : method,
       ...this.git.remoteParams,
     };
 
+    // When the merge method is `auto`, the merge strategy will determine the best merge method
+    // based on the pull request's commits.
+    if (method === 'auto') {
+      const hasFixUpOrSquashAndMultipleCommits =
+        normalCommitsCount > 1 && (fixupCount > 0 || squashCount > 0);
+
+      // If the PR has fixup/squash commits against multiple normal commits, or if the
+      // commit message needs to be fixed up, delegate to the autosquash merge strategy.
+      if (needsCommitMessageFixup || hasFixUpOrSquashAndMultipleCommits) {
+        return super.merge(pullRequest);
+      }
+
+      const hasOnlyFixUpForOneCommit =
+        normalCommitsCount === 1 && fixupCount > 0 && squashCount === 0;
+
+      const hasOnlySquashForOneCommit = normalCommitsCount === 1 && squashCount > 1;
+
+      // If the PR has only one normal commit and some fixup commits, the PR is squashed.
+      // The commit message from the single normal commit is used.
+      if (hasOnlyFixUpForOneCommit) {
+        mergeOptions.merge_method = 'squash';
+        const [title, message] = commits[0].message.split(COMMIT_HEADER_SEPARATOR);
+
+        mergeOptions.commit_title = title;
+        mergeOptions.commit_message = message;
+        // If the PR has only one normal commit and more than one squash commit, the PR is
+        // squashed and the user is prompted to edit the commit message.
+      } else if (hasOnlySquashForOneCommit) {
+        mergeOptions.merge_method = 'squash';
+        await this._promptCommitMessageEdit(pullRequest, mergeOptions);
+      }
+    }
+
     if (needsCommitMessageFixup) {
       // Commit message fixup does not work with other merge methods as the Github API only
       // allows commit message modifications for squash merging.
@@ -74,6 +111,7 @@ export class GithubApiMergeStrategy extends AutosquashMergeStrategy {
             `modified if the PR is merged using squash.`,
         );
       }
+
       await this._promptCommitMessageEdit(pullRequest, mergeOptions);
     }
 
@@ -113,6 +151,8 @@ export class GithubApiMergeStrategy extends AutosquashMergeStrategy {
     // If the PR does not need to be merged into any other target branches,
     // we exit here as we already completed the merge.
     if (!cherryPickTargetBranches.length) {
+      await this.createMergeComment(pullRequest, targetBranches);
+
       return;
     }
 
@@ -146,27 +186,7 @@ export class GithubApiMergeStrategy extends AutosquashMergeStrategy {
     }
 
     this.pushTargetBranchesUpstream(cherryPickTargetBranches);
-
-    /** The local branch names of the github targeted branches. */
-    const banchesAndSha: [branchName: string, commitSha: string][] = targetBranches.map(
-      (targetBranch) => {
-        const localBranch = this.getLocalTargetBranchName(targetBranch);
-
-        /** The SHA of the commit pushed to github which represents closing the PR. */
-        const sha = this.git.run(['rev-parse', localBranch]).stdout.trim();
-        return [targetBranch, sha];
-      },
-    );
-    // Because our process brings changes into multiple branchces, we include a comment which
-    // expresses all of the branches the changes were merged into.
-    await this.git.github.issues.createComment({
-      ...this.git.remoteParams,
-      issue_number: pullRequest.prNumber,
-      body:
-        'This PR was merged into the repository. ' +
-        'The changes were merged into the following branches:\n\n' +
-        `${banchesAndSha.map(([branch, sha]) => `- ${branch}: ${sha}`).join('\n')}`,
-    });
+    await this.createMergeComment(pullRequest, targetBranches);
   }
 
   /**
@@ -178,7 +198,7 @@ export class GithubApiMergeStrategy extends AutosquashMergeStrategy {
     pullRequest: PullRequest,
     mergeOptions: OctokitMergeParams,
   ) {
-    const commitMessage = await this._getDefaultSquashCommitMessage(pullRequest);
+    const commitMessage = await this.getDefaultSquashCommitMessage(pullRequest);
     const result = await Prompt.editor({
       message: 'Please update the commit message',
       default: commitMessage,
@@ -198,7 +218,7 @@ export class GithubApiMergeStrategy extends AutosquashMergeStrategy {
    * multiple commit messages if a PR is merged in squash mode. We try to replicate this
    * behavior here so that we have a default commit message that can be fixed up.
    */
-  private async _getDefaultSquashCommitMessage(pullRequest: PullRequest): Promise<string> {
+  private async getDefaultSquashCommitMessage(pullRequest: PullRequest): Promise<string> {
     const commits = await this.getPullRequestCommits(pullRequest);
     const messageBase = `${pullRequest.title}${COMMIT_HEADER_SEPARATOR}`;
     if (commits.length <= 1) {
@@ -219,10 +239,60 @@ export class GithubApiMergeStrategy extends AutosquashMergeStrategy {
     return this.config.default;
   }
 
-  /** Checks whether the pull request contains fixup or squash commits. */
-  private async hasFixupOrSquashCommits(pullRequest: PullRequest): Promise<boolean> {
+  /** Returns information about the commits in the pull request. */
+  private async getCommitsInfo(pullRequest: PullRequest): Promise<
+    Readonly<{
+      fixupCount: number;
+      squashCount: number;
+      normalCommitsCount: number;
+    }>
+  > {
     const commits = await this.getPullRequestCommits(pullRequest);
+    const commitsInfo = {
+      fixupCount: 0,
+      squashCount: 0,
+      normalCommitsCount: 1,
+    };
+
+    if (commits.length === 1) {
+      return commitsInfo;
+    }
+
+    for (let index = 1; index < commits.length; index++) {
+      const {
+        parsed: {isFixup, isSquash},
+      } = commits[index];
+
+      if (isFixup) {
+        commitsInfo.fixupCount++;
+      } else if (isSquash) {
+        commitsInfo.squashCount++;
+      } else {
+        commitsInfo.normalCommitsCount++;
+      }
+    }
+
+    return commitsInfo;
+  }
+
+  /** Commits of the pull request. */
+  private commits: PullRequestCommit[] | undefined;
+  /** Gets all commit messages of commits in the pull request. */
+  protected async getPullRequestCommits({prNumber}: PullRequest): Promise<PullRequestCommit[]> {
+    if (this.commits) {
+      return this.commits;
+    }
+
+    const allCommits = await this.git.github.paginate(this.git.github.pulls.listCommits, {
+      ...this.git.remoteParams,
+      pull_number: prNumber,
+    });
+
+    this.commits = allCommits.map(({commit: {message}}) => ({
+      message,
+      parsed: parseCommitMessage(message),
+    }));
 
-    return commits.some(({parsed: {isFixup, isSquash}}) => isFixup || isSquash);
+    return this.commits;
   }
 }
diff --git a/ng-dev/pr/merge/strategies/autosquash-merge.ts b/ng-dev/pr/merge/strategies/autosquash-merge.ts
@@ -6,8 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {dirname, join} from 'path';
-import {fileURLToPath} from 'url';
+import {setTimeout as sleep} from 'node:timers/promises';
+import {dirname, join} from 'node:path';
+import {fileURLToPath} from 'node:url';
 import {PullRequest} from '../pull-request.js';
 import {MergeStrategy, TEMP_PR_HEAD_BRANCH} from './strategy.js';
 import {MergeConflictsFatalError} from '../failures.js';
@@ -17,7 +18,7 @@ import {MergeConflictsFatalError} from '../failures.js';
  * all target branches and the PR locally. The PR is then cherry-picked with autosquash
  * enabled into the target branches. The benefit is the support for fixup and squash commits.
  * A notable downside though is that Github does not show the PR as `Merged` due to non
- * fast-forward merges
+ * fast-forward merges.
  */
 export class AutosquashMergeStrategy extends MergeStrategy {
   /**
@@ -83,34 +84,11 @@ export class AutosquashMergeStrategy extends MergeStrategy {
     // Push the cherry picked branches upstream.
     this.pushTargetBranchesUpstream(targetBranches);
 
-    /** The local branch names of the github targeted branches. */
-    const banchesAndSha: [branchName: string, commitSha: string][] = targetBranches.map(
-      (targetBranch) => {
-        const localBranch = this.getLocalTargetBranchName(targetBranch);
-
-        /** The SHA of the commit pushed to github which represents closing the PR. */
-        const sha = this.git.run(['rev-parse', localBranch]).stdout.trim();
-        return [targetBranch, sha];
-      },
-    );
-
     // Allow user to set an amount of time to wait to account for rate limiting of the token usage
     // during merge otherwise just waits 0 seconds.
-    await new Promise((resolve) =>
-      setTimeout(resolve, parseInt(process.env['AUTOSQUASH_TIMEOUT'] || '0')),
-    );
-    // Github automatically closes PRs whose commits are merged into the main branch on Github.
-    // However, it does not note them as merged using the purple merge badge as occurs when done via
-    // the UI. To inform users that the PR was in fact merged, add a comment expressing the fact
-    // that the PR is merged and what branches the changes were merged into.
-    await this.git.github.issues.createComment({
-      ...this.git.remoteParams,
-      issue_number: pullRequest.prNumber,
-      body:
-        'This PR was merged into the repository. ' +
-        'The changes were merged into the following branches:\n\n' +
-        `${banchesAndSha.map(([branch, sha]) => `- ${branch}: ${sha}`).join('\n')}`,
-    });
+    await sleep(parseInt(process.env['AUTOSQUASH_TIMEOUT'] || '0'));
+
+    await this.createMergeComment(pullRequest, targetBranches);
 
     // For PRs which do not target the `main` branch on Github, Github does not automatically
     // close the PR when its commit is pushed into the repository. To ensure these PRs are
diff --git a/ng-dev/pr/merge/strategies/strategy.ts b/ng-dev/pr/merge/strategies/strategy.ts
@@ -6,7 +6,6 @@
  * 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,
@@ -202,21 +201,29 @@ export abstract class MergeStrategy {
     }
   }
 
-  /** 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, {
+  /** Creates a comment on the pull request to indicate that the PR has been merged. */
+  protected async createMergeComment(
+    pullRequest: PullRequest,
+    targetBranches: string[],
+  ): Promise<void> {
+    /** The local branch names of the github targeted branches. */
+    const banchesAndSha: [branchName: string, commitSha: string][] = targetBranches.map(
+      (targetBranch) => {
+        const localBranch = this.getLocalTargetBranchName(targetBranch);
+
+        /** The SHA of the commit pushed to github which represents closing the PR. */
+        const sha = this.git.run(['rev-parse', localBranch]).stdout.trim();
+        return [targetBranch, sha];
+      },
+    );
+
+    await this.git.github.issues.createComment({
       ...this.git.remoteParams,
-      pull_number: prNumber,
+      issue_number: pullRequest.prNumber,
+      body:
+        'This PR was merged into the repository. ' +
+        'The changes were merged into the following branches:\n\n' +
+        `${banchesAndSha.map(([branch, sha]) => `- ${branch}: ${sha}`).join('\n')}`,
     });
-
-    return allCommits.map(({commit: {message}}) => ({
-      message,
-      parsed: parseCommitMessage(message),
-    }));
   }
 }
