Skip to content

feat(ng-dev/pr): support auto-merging pull requests once pending status checks pass#3661

Open
josephperrott wants to merge 1 commit into
angular:mainfrom
josephperrott:wait-on-validation
Open

feat(ng-dev/pr): support auto-merging pull requests once pending status checks pass#3661
josephperrott wants to merge 1 commit into
angular:mainfrom
josephperrott:wait-on-validation

Conversation

@josephperrott
Copy link
Copy Markdown
Member

Enables the ng-dev pr merge command to wait on non-final pending status validations (such as running CI workflows) to complete rather than aborting immediately. Includes interactive progress feedback and bounded timeout handling.

@josephperrott josephperrott added the action: merge The PR is ready for merge by the caretaker label May 12, 2026
@josephperrott josephperrott requested a review from alan-agius4 May 12, 2026 19:12
@angular-robot angular-robot Bot added the detected: feature PR contains a feature commit label May 12, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to wait for pending pull request validations before merging, adding a new --wait-for-validations CLI flag and a polling loop that re-fetches PR data. The implementation includes an isFinal property for validation failures to distinguish between transient and permanent issues. Feedback was provided to optimize the waiting logic; currently, the tool waits for pending validations even if final failures (like a rejected review) are already present. It is suggested to only trigger the wait loop if all current failures are non-final to prevent unnecessary delays.

Comment thread ng-dev/pr/common/validation/validate-pull-request.ts Outdated
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some nits.

spinner.update(spinnerText);
}

await new Promise((resolve) => setTimeout(resolve, 60000)); // Wait 1 minute
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT

Suggested change
await new Promise((resolve) => setTimeout(resolve, 60000)); // Wait 1 minute
import {setTimeout} from 'node:timers/promises';
await setTimeout(60_000); // Wait 1 minute

@@ -33,7 +35,7 @@ import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-clien
* Active release trains may be available for additional checks or not.
*/
export async function assertValidPullRequest(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Maybe split the validations into a seperate functions, something like to make the code more readable.

/**
 * Runs all valiations that the given pull request is valid, returning a list of all failing
 * validations.
 *
 * Active release trains may be available for additional checks or not.
 */
async function runValidations(
  pullRequest: PullRequestFromGithub,
  validationConfig: PullRequestValidationConfig,
  ngDevConfig: NgDevConfig<{pullRequest: PullRequestConfig; github: GithubConfig}>,
  activeReleaseTrains: ActiveReleaseTrains | null,
  target: PullRequestTarget,
  gitClient: AuthenticatedGitClient,
): Promise<PullRequestValidationFailure[]> {
  const labels = pullRequest.labels.nodes.map((l) => l.name);
  const commitsInPr = pullRequest.commits.nodes.map((n) => {
    return parseCommitMessage(n.commit.message);
  });

  const validationPromises = [
    minimumReviewsValidation.run(validationConfig, pullRequest),
    completedReviewsValidation.run(validationConfig, pullRequest),
    mergeReadyValidation.run(validationConfig, pullRequest),
    signedClaValidation.run(validationConfig, pullRequest),
    pendingStateValidation.run(validationConfig, pullRequest),
    breakingChangeInfoValidation.run(validationConfig, commitsInPr, labels),
    passingCiValidation.run(validationConfig, pullRequest),
    enforcedStatusesValidation.run(validationConfig, pullRequest, ngDevConfig.pullRequest),
    isolatedSeparateFilesValidation.run(
      validationConfig,
      ngDevConfig,
      pullRequest.number,
      gitClient,
    ),
    enforceTestedValidation.run(validationConfig, pullRequest, gitClient),
  ];

  if (activeReleaseTrains !== null) {
    validationPromises.push(
      changesAllowForTargetLabelValidation.run(
        validationConfig,
        commitsInPr,
        target.label,
        ngDevConfig.pullRequest,
        activeReleaseTrains,
        labels,
        pullRequest,
      ),
    );
  }

  const results = await Promise.all(validationPromises);
  return results.filter((result): result is PullRequestValidationFailure => result !== null);
}

export async function assertValidPullRequest(
  originalPullRequest: PullRequestFromGithub,
  validationConfig: PullRequestValidationConfig,
  ngDevConfig: NgDevConfig<{
    pullRequest: PullRequestConfig;
    github: GithubConfig;
  }>,
  activeReleaseTrains: ActiveReleaseTrains | null,
  target: PullRequestTarget,
  gitClient: AuthenticatedGitClient,
): Promise<PullRequestValidationFailure[]> {
  let pullRequest = originalPullRequest;
  let spinner: Spinner | undefined;
  const maxAttempts = 60;

  for (let attempts = 0; attempts <= maxAttempts; attempts++) {
    const failures = await runValidations(
      pullRequest,
      validationConfig,
      ngDevConfig,
      activeReleaseTrains,
      target,
      gitClient,
    );

    const finalFailures = failures.filter((f) => f.isFinal);
    const nonFinalFailures = failures.filter((f) => !f.isFinal);

    const shouldWaitForPending =
      nonFinalFailures.length > 0 &&
      finalFailures.length === 0 &&
      validationConfig.waitIfPending;

    if (!shouldWaitForPending) {
      if (spinner) {
        spinner.complete();
      }
      return failures;
    }

    if (attempts === maxAttempts) {
      if (spinner) {
        spinner.complete();
      }
      Log.error(
        `Timed out waiting for non-final validations to complete after ${maxAttempts} minutes.`,
      );
      return failures;
    }

    const names = nonFinalFailures.map((f) => f.validationName).join(', ');
    const verb = nonFinalFailures.length === 1 ? 'is' : 'are';
    const spinnerText = `[${names}] ${verb} not final. Waiting for completion (attempt ${attempts + 1}/${maxAttempts})...`;

    if (!spinner) {
      spinner = new Spinner(spinnerText);
    } else {
      spinner.update(spinnerText);
    }

    await new Promise((resolve) => setTimeout(resolve, 60000)); // Wait 1 minute
    const freshPr = await fetchPullRequestFromGithub(gitClient, originalPullRequest.number);
    if (!freshPr) {
      throw new Error('Failed to re-fetch pull request data');
    }
    pullRequest = freshPr;
  }

  throw new Error('Unreachable');
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker detected: feature PR contains a feature commit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants