From dee3291f6588419a2efbb73be6f10f67a96193a0 Mon Sep 17 00:00:00 2001
From: Aliaksandr <grubian2@gmail.com>
Date: Sat, 25 Apr 2026 03:12:03 +0300
Subject: [PATCH] ci/github-script/merge: refuse merge when a committer has
 requested changes

A "changes requested" review from a committer blocks both the merge
queue and auto-merge, but unlike approvals it isn't auto-dismissed
when new commits are pushed. Surface it as a checklist item so the
bot's reply explains the block instead of silently enabling
auto-merge that will never trigger.

Implementation pulls every review for the PR via `listReviews` and,
for each committer, takes the latest review whose state is
`APPROVED`/`CHANGES_REQUESTED`. The check fails if any committer's
latest stance is `CHANGES_REQUESTED`. Other review states fall out
naturally:

- A dismissed review surfaces as `DISMISSED`, so a committer
  dismissing their own changes-requested review unblocks the PR.
- A comment-only follow-up surfaces as `COMMENTED`, so it doesn't
  override an earlier actionable review - the prior stance still
  stands until the committer explicitly approves or requests changes
  again.

Assisted-by: claude-code with claude-opus-4-8[1m]-high
(cherry picked from commit b94b44d3f9e0d7d021dc4d550d2dd0185da47489)
---
 ci/README.md              |  2 ++
 ci/github-script/merge.js | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/ci/README.md b/ci/README.md
index a38aad398dc9..508e7d9c33d0 100644
--- a/ci/README.md
+++ b/ci/README.md
@@ -51,6 +51,8 @@ To ensure security and a focused utility, the bot adheres to specific limitation
   - opened by [@r-ryantm](https://nix-community.github.io/nixpkgs-update/r-ryantm/).
 - The user attempting to merge is a member of [@NixOS/nixpkgs-maintainers].
 - The user attempting to merge is a maintainer of all packages touched by the PR.
+- No [committer][@NixOS/nixpkgs-committers] has an outstanding "changes requested" review.
+  These block both the merge queue and auto-merge, so the bot refuses to merge until the review is addressed or dismissed.
 
 Once these constraints are met, the bot picks a merge strategy based on the `no PR failures` commit status:
 
diff --git a/ci/github-script/merge.js b/ci/github-script/merge.js
index e7f0cac8cc83..1d218b27b0ae 100644
--- a/ci/github-script/merge.js
+++ b/ci/github-script/merge.js
@@ -7,6 +7,7 @@ function runChecklist({
   pull_request,
   log,
   maintainers,
+  reviews,
   user,
   userIsMaintainer,
 }) {
@@ -41,6 +42,27 @@ function runChecklist({
       .filter(Boolean),
   )
 
+  // A "changes requested" review from a committer blocks both the merge queue and
+  // auto-merge, even if it was made on an older commit (unlike approvals, GitHub does
+  // not auto-dismiss changes-requested reviews on push). For each committer, take their
+  // latest actionable review; if it's CHANGES_REQUESTED, they're blocking the PR.
+  // Dismissed reviews surface as DISMISSED and comment-only follow-ups as COMMENTED, so
+  // both are skipped naturally — the prior actionable review still stands until the
+  // committer explicitly approves or requests changes again.
+  const committerReviewState = new Map()
+  for (const { user, state } of reviews) {
+    if (
+      user &&
+      committers.has(user.id) &&
+      ['APPROVED', 'CHANGES_REQUESTED'].includes(state)
+    ) {
+      committerReviewState.set(user.id, state)
+    }
+  }
+  const noBlockingReviews = !Array.from(committerReviewState.values()).includes(
+    'CHANGES_REQUESTED',
+  )
+
   const checklist = {
     'PR targets a [development branch](https://github.com/NixOS/nixpkgs/blob/-/ci/README.md#branch-classification).':
       classify(pull_request.base.ref).type.includes('development'),
@@ -60,6 +82,8 @@ function runChecklist({
     // CI state is intentionally *not* a checklist item: auto-merge exists precisely to
     // cover unfinished CI, and an already-failed CI is reported via the merge message
     // (see merge() below) rather than a blanket refusal.
+    'PR is not blocked by a "changes requested" review from a [committer](https://github.com/orgs/NixOS/teams/nixpkgs-committers).':
+      noBlockingReviews,
   }
 
   if (user) {
@@ -159,6 +183,15 @@ async function handleMerge({
     })
   ).data.find(({ context }) => context === 'no PR failures')?.state
 
+  // Reviews are returned in chronological order; the latest review per user wins below.
+  // Dismissed reviews surface with state DISMISSED and comment-only follow-ups as
+  // COMMENTED, so both naturally drop out of the committer-blocking check.
+  const reviews = await github.paginate(github.rest.pulls.listReviews, {
+    ...context.repo,
+    pull_number,
+    per_page: 100,
+  })
+
   // Only look through comments *after* the latest (force) push.
   const lastPush = events.findLastIndex(
     ({ event, sha, commit_id }) =>
@@ -309,6 +342,7 @@ async function handleMerge({
       pull_request,
       log,
       maintainers,
+      reviews,
       user: comment.user,
       userIsMaintainer: await isMaintainer(comment.user.login),
     })
@@ -380,6 +414,7 @@ async function handleMerge({
     pull_request,
     log,
     maintainers,
+    reviews,
   })
 
   // Returns a boolean, which indicates whether the PR is merge-bot eligible in principle.