fix(github-actions): refresh approvals that the merge tooling still counts

[s2ongmo]

Summary

post-approval-changes and the ng-dev merge validators recognize different reviewers, so an approval that makes a pull request merge ready can be invisible to the check that is supposed to refresh it. This aligns the action with the merge tooling:

• recognize reviews whose author_association is MEMBER, not only members of the googlers org
• ignore COMMENTED reviews when resolving a reviewer's latest review

Why

ng-dev's assertMinimumReviews accepts an approval from any repository MEMBER. The action only keeps reviews from googlers org members. When a member who is not in that org approves an external contributor's pull request and adds action: merge, the action finds no reviews at all, logs Skipping check as their are no reviews on the pull request. and returns. No review is requested when the contributor pushes another commit, so the approval of the previous head keeps satisfying the merge validators for a head nobody reviewed.

A COMMENTED review by that reviewer produced the same outcome through the other early return: it was resolved as their latest review and tripped the non-approved state check. Commenting after approving is common, GitHub does not treat a comment as replacing an approval, and assertMinimumReviews reads reviews(states: APPROVED), so the earlier approval still counted.

With both handled the action re-requests the review for the new head, reviewRequests.totalCount becomes non-zero, and assertCompletedReviews holds the merge until that review is completed. Post-approval updates performed by Googlers are unaffected, because the actor check above returns before any review is inspected.

Note on scope

An earlier version of this PR also bound assertMinimumReviews to headRefOid. That validation is canBeForceIgnored: false and would have blocked merges this repository intentionally allows (a member updates an already approved pull request and the caretaker merges it): 6 of the last 40 merged angular/angular pull requests were merged with the only MEMBER approval pointing at an earlier commit. That part has been removed. It could be added later as a separate, force-ignorable validation if the team wants it as defence in depth.

Validation

• bazel run //github-actions/post-approval-changes:main to regenerate the checked-in bundle
• bazel test //github-actions/post-approval-changes:all (main_test, main_prettierignore_test, lib_strict_deps_test) passes

[gemini-code-assist]

Code Review

This pull request updates the post-approval changes action to recognize reviews from trusted project members (MEMBER) and ensures that the minimum reviews validation requires at least one approval on the latest commit. Unit tests have been added to verify this behavior. The feedback points out a potential runtime crash in the validation logic if the commit object is null, suggesting the use of optional chaining (commit?.oid) to safely access the commit ID.

[gemini-code-assist]

In GitHub's GraphQL API, the commit field on a PullRequestReview is nullable (for example, if the commit has been garbage collected or is no longer available after a force-push). Destructuring commit and directly accessing commit.oid without a null/undefined check can lead to a runtime TypeError: Cannot read properties of null (reading 'oid') and crash the validation process.

Using optional chaining (commit?.oid) safely handles cases where commit might be null.

Suggested change

	({authorAssociation, commit}) =>
	authorAssociation === 'MEMBER' && commit.oid === pullRequest.headRefOid,
	({authorAssociation, commit}) =>
	authorAssociation === 'MEMBER' && commit?.oid === pullRequest.headRefOid,

[s2ongmo]

Rebased onto current main (6d918a6) and narrowed the change.

What I dropped. The earlier version also bound assertMinimumReviews to headRefOid. That validation is declared canBeForceIgnored: false, so it would have blocked merges this repository intentionally allows: a member updates or rebases an already approved pull request, post-approval-changes deliberately does not re-request a review because the actor is a Googler, and the caretaker merges. Of the last 40 merged angular/angular pull requests, 6 were merged while the only MEMBER approval pointed at an earlier commit than the merged head (for example #70303, approved at f6d424a1 and merged at b4325d89). Those would have failed with no way to override, so that part does not belong here.

What is left is the freshness action itself, where the trust sets disagree:

1. It only keeps reviews whose author is a member of the googlers org, while ng-dev's assertMinimumReviews accepts an approval from any repository MEMBER. When such a member approves an external contributor's pull request and marks it merge ready, this action sees no reviews at all, returns at Skipping check as their are no reviews on the pull request., and never re-requests a review after the contributor pushes another commit. The approval given to the previous head keeps satisfying the merge validators.
2. A COMMENTED review by the same reviewer reached the same outcome through the other early return, because it was resolved as that reviewer's latest review and tripped the non-approved state check. GitHub does not treat a comment as replacing an approval, and assertMinimumReviews reads reviews(states: APPROVED), so the earlier approval still counted.

With both handled, the action re-requests the review for the new head, reviewRequests.totalCount becomes non-zero, and assertCompletedReviews holds the merge until that review is completed. Post-approval updates performed by Googlers are unaffected, since the actor check above returns before any review is inspected.

bazel test //github-actions/post-approval-changes:all passes, including the checked-in bundle check.

Happy to reshape this however you prefer, including moving the head-binding invariant into a separate force-ignorable validation if you would like it as defence in depth.

[s2ongmo]

Rebased onto main (6d918a6) on 2026-08-23; CI is green and pullapprove is the only pending status. .pullapprove.yml sets request: 0, so no reviewer is auto-assigned here — hence this ping rather than more silence.

Only one file needs human review: lib/main.ts (+18/-2). main.js is the checked-in esbuild bundle, regenerated by the build.

The decision: should this action's reviewer set match the set the merge tooling already trusts?

• lib/main.ts:106-109 keeps only reviewers in the googlers org. When that empties the list, :120-121 returns success, so the head-freshness comparison at :130 and the re-request at :137 never run.
• ng-dev/pr/common/validation/assert-minimum-reviews.ts:20-22 counts any APPROVED review with authorAssociation === 'MEMBER', and no validator compares the review's commit.oid to headRefOid. assert-completed-reviews.ts only checks reviewRequests.totalCount, which the skipped re-request leaves at 0. Both are canBeForceIgnored: false.
• The two sets do differ in practice: GET /orgs/googlers/members/hawkgs - 404 in angular/angular run 32384113311 (job 96474228474, 2026-08-20), and the same for AleksanderBodurri in run 32689628312 (2026-08-24). Both are in PullApprove's users_available list.

So an approval that is sufficient to merge is not recognised as an approval worth refreshing.

This reads as a migration artefact rather than an intended invariant. f556a9a created the action to "rerequest reviews for post approval changes for non-googlers", with a hardcoded allowlist that was effectively the Angular team. #1015 replaced that array with the googlers org lookup, and its commit messages scope the change to the actor ("determine post approval change check based on the actor rather than the author"). The reviewer-side use of the same predicate quietly narrowed from "on the Angular team" to "employed by Google".

The Googler path is untouched: the actor check at :61 returns before any review is read. The action's only write is pulls.requestReviewers, so widening the reviewer set can only add friction, never remove it. CHANGES_REQUESTED still trips :125; only COMMENTED is skipped, which ng-dev already ignores (fetch-pull-request.ts:90).

Happy to split this into two commits (MEMBER handling / COMMENTED handling), or to drop either half. If the reviewer-side filter is deliberate, say so and I'll close this.

@josephperrott — you wrote the action and #1015, so you're best placed to rule on it. @alan-agius4 — cc.