From 84dc19b0a8fd03456f2e642431b8c385da306360 Mon Sep 17 00:00:00 2001
From: Victor Sotero <victor.sotero@zeekrtech.eu>
Date: Thu, 25 Jun 2026 17:02:09 +0200
Subject: [PATCH] Respect latest PR review state for approvals

The maintainer-approval workflow treated any historical APPROVED review as current, so a later CHANGES_REQUESTED review from the same user did not revoke approval.

The workflow now reduces reviews to the latest state per reviewer before evaluating maintainer, maintainer-authored, and per-path approvals.

Constraint: GitHub returns the full review history, but the approval decision should reflect each reviewer's current state.

Rejected: Filter out CHANGES_REQUESTED reviews only | stale approvals would still count after later comments or dismissals

Confidence: high

Scope-risk: narrow

Tested: node --test .github/workflows/maintainer-approval.test.js

Related: #5322
---
 .github/workflows/maintainer-approval.js      | 16 +++++--
 .github/workflows/maintainer-approval.test.js | 48 +++++++++++++++++++
 2 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/maintainer-approval.js b/.github/workflows/maintainer-approval.js
index ca1d914a1b5..bd043ab4319 100644
--- a/.github/workflows/maintainer-approval.js
+++ b/.github/workflows/maintainer-approval.js
@@ -103,6 +103,15 @@ const STATUS_CONTEXT = "maintainer-approval";
 
 const loginCache = {};
 
+function latestReviewsByUser(reviews) {
+  const latest = new Map();
+  for (const review of reviews) {
+    if (!review.user?.login) continue;
+    latest.set(review.user.login.toLowerCase(), review);
+  }
+  return Array.from(latest.values());
+}
+
 function classifyFile(filepath, totalFiles) {
   const base = path.basename(filepath);
   if (base.startsWith("out.") || base === "output.txt") {
@@ -465,9 +474,10 @@ module.exports = async ({ github, context, core }) => {
     repo: context.repo.repo,
     pull_number: context.issue.number,
   });
+  const latestReviews = latestReviewsByUser(reviews);
 
   // Maintainer approval -> success with simple comment
-  const maintainerApproval = reviews.find(
+  const maintainerApproval = latestReviews.find(
     ({ state, user }) =>
       state === "APPROVED" && user && maintainers.includes(user.login)
   );
@@ -486,7 +496,7 @@ module.exports = async ({ github, context, core }) => {
 
   // Maintainer-authored PR with any approval -> success
   if (authorLogin && maintainers.includes(authorLogin)) {
-    const hasAnyApproval = reviews.some(
+    const hasAnyApproval = latestReviews.some(
       ({ state, user }) =>
         state === "APPROVED" && user && user.login !== authorLogin
     );
@@ -504,7 +514,7 @@ module.exports = async ({ github, context, core }) => {
   }
 
   // Gather approved logins (excluding the PR author).
-  const approverLogins = reviews
+  const approverLogins = latestReviews
     .filter(
       ({ state, user }) =>
         state === "APPROVED" && user && user.login !== authorLogin
diff --git a/.github/workflows/maintainer-approval.test.js b/.github/workflows/maintainer-approval.test.js
index 2866dc9d3d7..9de66bc5bdd 100644
--- a/.github/workflows/maintainer-approval.test.js
+++ b/.github/workflows/maintainer-approval.test.js
@@ -158,6 +158,22 @@ describe("maintainer-approval", () => {
     assert.equal(github._updatedComments.length, 0);
   });
 
+  it("maintainer approval is ignored after same reviewer requests changes", async () => {
+    const github = makeGithub({
+      reviews: [
+        { state: "APPROVED", user: { login: "maintainer1" } },
+        { state: "CHANGES_REQUESTED", user: { login: "maintainer1" } },
+      ],
+      files: [{ filename: "cmd/pipelines/foo.go" }],
+    });
+    const core = makeCore();
+    const context = makeContext();
+
+    await runModule({ github, context, core });
+
+    assert.equal(github._checkRuns.length, 0);
+  });
+
   it("approval cleans up stale pending comment", async () => {
     const github = makeGithub({
       reviews: [
@@ -198,6 +214,22 @@ describe("maintainer-approval", () => {
     assert.equal(github._updatedComments.length, 0);
   });
 
+  it("maintainer-authored PR ignores stale approval after changes requested", async () => {
+    const github = makeGithub({
+      reviews: [
+        { state: "APPROVED", user: { login: "randomreviewer" } },
+        { state: "CHANGES_REQUESTED", user: { login: "randomreviewer" } },
+      ],
+      files: [{ filename: "cmd/pipelines/foo.go" }],
+    });
+    const core = makeCore();
+    const context = makeContext({ author: "maintainer1" });
+
+    await runModule({ github, context, core });
+
+    assert.equal(github._checkRuns.length, 0);
+  });
+
   it("single domain, owner approved -> success, no comment", async () => {
     const github = makeGithub({
       reviews: [
@@ -219,6 +251,22 @@ describe("maintainer-approval", () => {
     assert.equal(github._updatedComments.length, 0);
   });
 
+  it("per-path approval ignores stale approval after owner requests changes", async () => {
+    const github = makeGithub({
+      reviews: [
+        { state: "APPROVED", user: { login: "jefferycheng1" } },
+        { state: "CHANGES_REQUESTED", user: { login: "jefferycheng1" } },
+      ],
+      files: [{ filename: "cmd/pipelines/foo.go" }],
+    });
+    const core = makeCore();
+    const context = makeContext();
+
+    await runModule({ github, context, core });
+
+    assert.equal(github._checkRuns.length, 0);
+  });
+
   it("cross-domain, both approved -> success, no comment", async () => {
     const github = makeGithub({
       reviews: [