fix(review-checklist): prune CODEOWNERS avalanche regardless of which event wins the race

GitHub's CODEOWNERS engine fires one review_requested event per auto-assigned
owner on PR creation, and each re-triggers this workflow. With concurrency:
cancel-in-progress, whichever run starts last wins — and that's just as
likely to be one of those review_requested runs as the opened run itself.
A surviving review_requested run fell through to the additive-fill branch,
saw every area already "covered" by the avalanche, and pruned nothing.
This hit PR #6479 itself: all 4 CODEOWNERS for .github/ stayed requested.

Branch on whether a checklist comment exists yet instead of which action
survived — that signal is race-resistant: no comment means this is the PR's
first coordination pass no matter which event got here. Threads
hasExistingComment through from run() (defaulting to true, i.e. additive-fill,
on a comment-fetch failure so an API hiccup can't be mistaken for a fresh PR).

2 new tests: the #6479 race itself, and a contrast case confirming a routine
review_requested on an already-established PR still uses additive-fill.

Author: brtnfld

.github/scripts/review-checklist.js

@@ -347,21 +347,37 @@ function planSynchronizeSwaps(eligibleAreas, allReviews, {
 //                 signal that someone, right now, individually decided this
 //                 person should be back — see the updatedExcluded comment).
 //
-//   draft, just (re)opened as one
+//   draft, just (re)opened as one — OR no checklist comment posted yet
 //               → GitHub's CODEOWNERS auto-assignment fires immediately on
 //                 creation regardless of draft status, dumping every
 //                 touched-area owner onto the PR before anyone's decided
 //                 review is even wanted yet. Cleared in full — no checklist
 //                 posted until the PR is ready for review.
 //
-//   non-draft, just (re)opened or just marked ready_for_review
+//   non-draft, just (re)opened or just marked ready_for_review —
+//   OR no checklist comment posted yet
 //               → Same CODEOWNERS avalanche — GitHub auto-requests CODEOWNERS
 //                 reviewers both on creation and again when a draft is marked
 //                 ready for review. Pruned to the load-balanced single pick
 //                 per area before the checklist is first posted, so reviewers
 //                 aren't @-mentioned en masse before the final reviewer set
 //                 is known.
 //
+//                 The "no comment posted yet" clause covers a race: GitHub's
+//                 CODEOWNERS engine fires one review_requested per
+//                 auto-assigned owner, and each re-triggers this workflow.
+//                 With concurrency: cancel-in-progress, whichever run starts
+//                 last wins — and that's just as likely to be one of those
+//                 review_requested runs as the opened run itself (PR #6479
+//                 hit exactly this: a review_requested run survived, fell
+//                 through to the additive-fill branch below, saw every area
+//                 already "covered" by the avalanche, and pruned nothing).
+//                 Whether a checklist comment exists yet is a far more
+//                 reliable signal than which specific action survived the
+//                 race: if none exists, this is the PR's first coordination
+//                 pass no matter what action got here, so the avalanche
+//                 still needs pruning.
+//
 //   synchronize with a dismissed reviewer
 //               → see planSynchronizeSwaps: re-requests a reviewer whose
 //                 approval a new push just dismissed, swapping out a fresh
@@ -393,10 +409,17 @@ function planSynchronizeSwaps(eligibleAreas, allReviews, {
 //
 async function coordinateReviewers(github, context, core, {
   owner, repo, pr_number, prData, allCodeOwners, catchAllOwners, touchedAreas, reviewerLoad,
-  excludedReviewers, allReviews, LINE_THRESHOLD, AREA_THRESHOLDS, PUBLIC_HEADER,
+  excludedReviewers, allReviews, hasExistingComment, LINE_THRESHOLD, AREA_THRESHOLDS, PUBLIC_HEADER,
 }) {
   const pr     = { owner, repo, pr_number };
   const action = context.payload.action;
+  // No checklist comment yet means this is this PR's first coordination
+  // pass, regardless of which webhook action's run happened to survive the
+  // opened-vs-review_requested cancel-in-progress race — see coordinateReviewers
+  // doc comment above. Require an explicit `false` so a caller that omits the
+  // field (or a stale/odd payload) defaults to the safer additive-fill path
+  // instead of unexpectedly pruning an established PR's reviewers.
+  const isFirstCoordinationPass = hasExistingComment === false;
 
   // A removal happening right now joins the persisted exclusion set
   // immediately, so it's enforced starting with this very run. A direct
@@ -473,7 +496,7 @@ async function coordinateReviewers(github, context, core, {
   const touchedAreaOwners = new Set([...touchedAreas.flatMap(a => a.owners), ...catchAllOwners]);
 
   if (isDraft) {
-    if (action === 'opened' || action === 'reopened') {
+    if (action === 'opened' || action === 'reopened' || isFirstCoordinationPass) {
       // (Re)opened directly as a draft — clear the CODEOWNERS avalanche from
       // this PR's creation. Owners of areas this PR actually touches, plus
       // catch-all "*" owners (who GitHub auto-assigns on every PR regardless
@@ -499,13 +522,17 @@ async function coordinateReviewers(github, context, core, {
     owners: area.owners.filter(o => !updatedExcluded.has(o)),
   }));
 
-  if (action === 'opened' || action === 'reopened' || action === 'ready_for_review') {
+  if (action === 'opened' || action === 'reopened' || action === 'ready_for_review' || isFirstCoordinationPass) {
     // Non-draft, just (re)opened — same CODEOWNERS avalanche problem as the
     // draft case: GitHub has already auto-assigned every touched-area owner.
     // ready_for_review gets the same treatment: GitHub auto-requests CODEOWNERS
     // reviewers again when a draft is marked ready, dumping the avalanche on a
     // PR that may have sat in draft (untouched, per the isDraft branch above)
-    // for a while. Prune to a load-balanced single pick per area BEFORE posting
+    // for a while. isFirstCoordinationPass catches the case where neither of
+    // those actions is the one that happened to survive the cancel-in-progress
+    // race against the avalanche's own review_requested events (see the doc
+    // comment above coordinateReviewers — this is exactly what happened on
+    // PR #6479). Prune to a load-balanced single pick per area BEFORE posting
     // the checklist so reviewers aren't @-mentioned en masse. Pass an empty
     // existingRequested so chooseReviewers treats every area as uncovered and
     // picks fresh rather than seeing "already has an owner" and returning nothing.
@@ -767,19 +794,26 @@ module.exports = async function run({ github, context, core }) {
   //    (if any), then coordinate reviewer assignment.
   // ----------------------------------------------------------------
   let existingComment;
+  let commentFetchFailed = false;
   try {
     const comments = await github.paginate(github.rest.issues.listComments, {
       owner, repo, issue_number: pr_number, per_page: 100,
     });
     existingComment = comments.find(c => c.body.includes(MARKER));
   } catch (error) {
     core.warning(`Could not fetch existing checklist comment: ${error.message}`);
+    commentFetchFailed = true;
   }
   const excludedReviewers = parseExcluded(existingComment && existingComment.body);
+  // On a fetch failure we genuinely don't know whether a comment exists —
+  // default to true (assume it does) so coordinateReviewers falls back to its
+  // non-destructive additive-fill path rather than treating an API hiccup as
+  // "first coordination pass" and pruning an established PR's reviewers.
+  const hasExistingComment = commentFetchFailed ? true : !!existingComment;
 
   const { confirmedRequested, excludedReviewers: updatedExcluded } = await coordinateReviewers(github, context, core, {
     owner, repo, pr_number, prData, allCodeOwners, catchAllOwners, touchedAreas, reviewerLoad,
-    excludedReviewers, allReviews, LINE_THRESHOLD, AREA_THRESHOLDS, PUBLIC_HEADER,
+    excludedReviewers, allReviews, hasExistingComment, LINE_THRESHOLD, AREA_THRESHOLDS, PUBLIC_HEADER,
   });
 
   // ----------------------------------------------------------------

.github/scripts/review-checklist.test.js

@@ -731,6 +731,9 @@ function makeCoordinateBaseArgs(overrides) {
     reviewerLoad: {},
     excludedReviewers: new Set(),
     allReviews: [],
+    // Most scenarios model an already-established PR (checklist already
+    // posted); tests exercising the fresh-PR race override this explicitly.
+    hasExistingComment: true,
     LINE_THRESHOLD: 300,
     AREA_THRESHOLDS: {},
     PUBLIC_HEADER: /public\.h$/,
@@ -785,6 +788,45 @@ asyncTest('coordinateReviewers: plain synchronize (no dismissed reviews) is left
   assert.deepStrictEqual([...confirmedRequested].sort(), ['glennsong09', 'hyoklee', 'jhendersonHDF']);
 });
 
+asyncTest('coordinateReviewers: review_requested survives the opened race and still prunes (PR #6479 scenario)', async () => {
+  // GitHub's CODEOWNERS engine fires one review_requested per auto-assigned
+  // owner; each re-triggers this workflow, and concurrency: cancel-in-progress
+  // means any of those runs — not necessarily the "opened" run — can be the
+  // one that actually executes. hasExistingComment: false (no checklist
+  // posted yet) is what lets a surviving review_requested run still prune
+  // the avalanche instead of falling through to additive-fill and keeping
+  // all three.
+  const github = makeGithubMock();
+  const context = {
+    eventName: 'pull_request_target',
+    payload: { action: 'review_requested', requested_reviewer: { login: 'jhendersonHDF' }, sender: { type: 'User' } },
+  };
+  const args = makeCoordinateBaseArgs({ hasExistingComment: false });
+
+  const { confirmedRequested } = await coordinateReviewers(github, context, makeCore(), args);
+
+  assert.strictEqual(confirmedRequested.size, 1);
+  assert.ok(github.calls.removeRequestedReviewers.length > 0);
+});
+
+asyncTest('coordinateReviewers: review_requested on an already-established PR is NOT treated as a fresh-PR prune', async () => {
+  // Contrast case: once a checklist comment exists, a routine review_requested
+  // later in the PR's life (e.g. a human manually adding a reviewer) must stay
+  // on the additive-fill path — it must not be reinterpreted as "first
+  // coordination pass" and prune reviewers an established PR already has.
+  const github = makeGithubMock();
+  const context = {
+    eventName: 'pull_request_target',
+    payload: { action: 'review_requested', requested_reviewer: { login: 'jhendersonHDF' }, sender: { type: 'User' } },
+  };
+  const args = makeCoordinateBaseArgs({ hasExistingComment: true });
+
+  const { confirmedRequested } = await coordinateReviewers(github, context, makeCore(), args);
+
+  assert.strictEqual(github.calls.removeRequestedReviewers.length, 0);
+  assert.deepStrictEqual([...confirmedRequested].sort(), ['glennsong09', 'hyoklee', 'jhendersonHDF']);
+});
+
 // ----------------------------------------------------------------
 // coordinateReviewers — bot-self-triggered review_request_removed must not
 // create a sticky exclusion (the bot's own removeUnselected/removeRequestedReviewers