review-checklist: show non-CODEOWNER reviewer when no area owner is assigned (#6446)

* review-checklist: show non-CODEOWNER reviewer when no owner is assigned

If the only assigned CODEOWNER is removed and a non-CODEOWNER is manually
added in their place, that person previously got no checklist mention and
their approval did not check the box.

For any area with no CODEOWNER in the requested set, fall back to
non-CODEOWNER reviewers (anyone assigned who is not an owner of any
touched area). They are shown in the mention and their approval counts
as sign-off for that area.

* review-checklist: update checklist on reviewer add/remove

Adding or removing a reviewer did not trigger a workflow run, leaving
the checklist comment stale until the next push.

Add review_requested and review_request_removed to the pull_request_target
activity types so the checklist updates immediately when a reviewer is
manually added or removed.
This commit is contained in:
Scot Breitenfeld
2026-06-12 14:21:53 -05:00
committed by GitHub
parent 55d179733c
commit 41ea347fb9
2 changed files with 14 additions and 6 deletions
+13 -5
View File
@@ -157,17 +157,25 @@ function chooseReviewers(touchedAreas, {
// Builds the markdown checklist comment body (pure, no I/O).
function buildBody(touchedAreas, approvedUsers, confirmedRequested) {
// Reviewers manually assigned who are not CODEOWNERS for any touched area.
// Used as a fallback for areas that have no CODEOWNER assigned — their
// approval also counts as sign-off for that area.
const allAreaOwners = new Set(touchedAreas.flatMap(a => a.owners));
const nonOwnerReviewers = [...confirmedRequested].filter(o => !allAreaOwners.has(o));
const rowData = touchedAreas.map(area => {
const approver = area.owners.find(o => approvedUsers.has(o));
const ownerReviewers = area.owners.filter(o => confirmedRequested.has(o));
// If no CODEOWNER is assigned for this area, fall back to non-CODEOWNER
// reviewers so manually-assigned people are shown and their approval counts.
const effectiveReviewers = ownerReviewers.length > 0 ? ownerReviewers : nonOwnerReviewers;
const approver = effectiveReviewers.find(o => approvedUsers.has(o));
const signedOff = !!approver;
const box = signedOff ? 'x' : ' ';
const tick = signedOff ? ' ✅' : '';
// Signed off: show who approved. Pending: show all confirmed-requested reviewers
// (may be > 1 if a reviewer was manually added alongside the load-balanced pick).
const requested = area.owners.filter(o => confirmedRequested.has(o));
// Signed off: show who approved. Pending: show all effective reviewers.
const mention = approver
? ` — @${approver}`
: requested.length > 0 ? ` — ${requested.map(o => `@${o}`).join(', ')}` : '';
: effectiveReviewers.length > 0 ? ` — ${effectiveReviewers.map(o => `@${o}`).join(', ')}` : '';
return { text: `- [${box}] **${area.label}**${tick}${mention}`, signedOff };
});
+1 -1
View File
@@ -23,7 +23,7 @@ on:
# Safe: checkout has no ref: override so the base branch (develop) is always
# used — the fork's code is never checked out or executed.
pull_request_target:
types: [opened, synchronize, reopened]
types: [opened, synchronize, reopened, review_requested, review_request_removed]
branches: [develop]
workflow_run:
workflows: ["Review Checklist (gather)"]