diff --git a/.github/workflows/maintainer-approval.js b/.github/workflows/maintainer-approval.js index ca1d914a1b..bd043ab431 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 2866dc9d3d..9de66bc5bd 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: [