fix(ng-dev/pr): require enforced statuses to be passing, not just present

[ParthShethia25]

Problem

assertEnforcedStatuses matches each entry in requiredStatuses by name and type only:

if (!statuses.some((s) => s.name === enforced.name && s.type === enforced.type)) {
  missing.push(enforced.name);
}

It never inspects the status itself, so a required check that was skipped, cancelled, timed out or failed satisfies the validation as long as an entry with the configured name exists on the pull request.

Why assertPassingCi does not already cover this

assertPassingCi gates on combinedStatus, which comes from GitHub's statusCheckRollup.state. GitHub reports that as SUCCESS when checks are skipped — skipped checks do not contribute failure to the rollup.

Sampling open pull requests across several large repositories, I found 83 with skipped checks and a SUCCESS rollup, including on angular/angular itself (#70704, #70707, #70708, #70709, #70714).

So a skipped required check currently passes both validations.

The information was already available

getStatusesForPullRequest → normalizeGithubCheckState already classifies these as failing:

case 'ACTION_REQUIRED':
case 'TIMED_OUT':
case 'CANCELLED':
case 'FAILURE':
case 'SKIPPED':
case 'STALE':
case 'STARTUP_FAILURE':
  return PullRequestStatus.FAILING;

assertEnforcedStatuses computed that value and discarded it. This change makes the validation honour ng-dev's existing normalization rather than introducing new semantics.

Change

Look up each required entry rather than testing only for its existence, and report entries that are present but not PASSING separately from entries that are missing.

Unrelated observation

While reading getStatusesForPullRequest I noticed the switch has no break after the CheckRun case, so every CheckRun also falls through into the StatusContext branch and writes an entry keyed undefined. It appears harmless today, since no consumer matches on undefined, so I have left it out of this PR — happy to send a separate one if you would like it fixed.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request enhances the pull request validation by checking not only if required statuses are present, but also ensuring that they are passing. It introduces a notPassing array to collect any required statuses that do not have a passing status and throws an error if any are found. The review feedback points out a potential runtime TypeError if config.requiredStatuses is undefined, suggesting to safeguard the loop with a nullish coalescing operator.

[gemini-code-assist]

Since config.requiredStatuses is defined as optional in PullRequestConfig, it can be undefined if not explicitly configured. Iterating over it directly in the for...of loop on line 34 will throw a runtime TypeError (e.g., Cannot read properties of undefined).\n\nTo prevent this, consider safeguarding the loop by defaulting to an empty array:\n\ntypescript\nfor (const enforced of config.requiredStatuses ?? []) {\n

[ParthShethia25]

There's already an early return for this a few lines up (line 26): if (config.requiredStatuses === undefined) { return; }. The loop isn't reachable when it's undefined, and TypeScript narrows the type there as well, so ?? [] would be dead code. Leaving it as is.