ci/github-script/{reviews,check-target-branch,commits,prepare}: add sticky review support

ci/github-script/check-target-branch.js

@@ -6,8 +6,9 @@
 
 const { classify, split } = require('../supportedBranches.js')
 const { readFile } = require('node:fs/promises')
-const { postReview } = require('./reviews.js')
+const { postReview, dismissReviews } = require('./reviews.js')
 
+const reviewKey = 'check-target-branch'
 /**
  * @param {{
  *  github: InstanceType<import('@actions/github/lib/utils').GitHub>,
@@ -43,6 +44,15 @@ async function checkTargetBranch({ github, context, core, dry }) {
     core.info(
       `Skipping checkTargetBranch: PR is from a development branch (${head})`,
     )
+
+    await dismissReviews({
+      github,
+      context,
+      core,
+      dry,
+      reviewKey,
+    })
+
     return
   }
   // Don't run on PRs against staging branches, wip branches, haskell-updates, etc.
@@ -50,6 +60,15 @@ async function checkTargetBranch({ github, context, core, dry }) {
     core.info(
       `Skipping checkTargetBranch: PR is against a non-primary base branch (${base})`,
     )
+
+    await dismissReviews({
+      github,
+      context,
+      core,
+      dry,
+      reviewKey,
+    })
+
     return
   }
 
@@ -78,6 +97,7 @@ async function checkTargetBranch({ github, context, core, dry }) {
       dry,
       body,
       event: 'REQUEST_CHANGES',
+      reviewKey,
     })
 
     throw new Error('This PR is against the wrong branch.')
@@ -105,6 +125,7 @@ async function checkTargetBranch({ github, context, core, dry }) {
       dry,
       body,
       event: 'REQUEST_CHANGES',
+      reviewKey,
     })
 
     throw new Error('This PR is against the wrong branch.')
@@ -125,10 +146,18 @@ async function checkTargetBranch({ github, context, core, dry }) {
       dry,
       body,
       event: 'COMMENT',
+      reviewKey,
     })
   } else {
-    // Any existing reviews were dismissed by commits.js
     core.info('checkTargetBranch: this PR is against an appropriate branch.')
+
+    await dismissReviews({
+      github,
+      context,
+      core,
+      dry,
+      reviewKey,
+    })
   }
 }
 

ci/github-script/commits.js

@@ -3,6 +3,7 @@ module.exports = async ({ github, context, core, dry, cherryPicks }) => {
   const { classify } = require('../supportedBranches.js')
   const withRateLimit = require('./withRateLimit.js')
   const { dismissReviews, postReview } = require('./reviews.js')
+  const reviewKey = 'check-commits'
 
   await withRateLimit({ github, core }, async (stats) => {
     stats.prs = 1
@@ -193,7 +194,7 @@ module.exports = async ({ github, context, core, dry, cherryPicks }) => {
     // An empty results array will always trigger this condition, which is helpful
     // to clean up reviews created by the prepare step when on the wrong branch.
     if (results.every(({ severity }) => severity === 'info')) {
-      await dismissReviews({ github, context, dry })
+      await dismissReviews({ github, context, dry, reviewKey })
       return
     }
 
@@ -316,6 +317,6 @@ module.exports = async ({ github, context, core, dry, cherryPicks }) => {
     // Posting a review could fail for very long comments. This can only happen with
     // multiple commits all hitting the truncation limit for the diff. If you ever hit
     // this case, consider just splitting up those commits into multiple PRs.
-    await postReview({ github, context, core, dry, body })
+    await postReview({ github, context, core, dry, body, reviewKey })
   })
 }

ci/github-script/prepare.js

@@ -1,5 +1,6 @@
 const { classify } = require('../supportedBranches.js')
-const { postReview } = require('./reviews.js')
+const { postReview, dismissReviews } = require('./reviews.js')
+const reviewKey = 'prepare'
 
 module.exports = async ({ github, context, core, dry }) => {
   const pull_number = context.payload.pull_request.number
@@ -46,7 +47,7 @@ module.exports = async ({ github, context, core, dry }) => {
         `Please target \`${correctBranch}\` instead.`,
       ].join('\n')
 
-      await postReview({ github, context, core, dry, body })
+      await postReview({ github, context, core, dry, body, reviewKey })
 
       throw new Error('The PR targets a channel branch.')
     }
@@ -170,12 +171,14 @@ module.exports = async ({ github, context, core, dry }) => {
           '  ```',
         ].join('\n')
 
-        await postReview({ github, context, core, dry, body })
+        await postReview({ github, context, core, dry, body, reviewKey })
 
         throw new Error(`The PR contains commits from a different base.`)
       }
     }
 
+    await dismissReviews({ github, context, core, dry, reviewKey })
+
     let mergedSha, targetSha
 
     if (prInfo.mergeable) {

ci/github-script/reviews.js

@@ -11,9 +11,10 @@ const eventToState = {
  *  context: import('@actions/github/lib/context').Context,
  *  core: import('@actions/core'),
  *  dry: boolean,
+ *  reviewKey?: string,
  * }} DismissReviewsProps
  */
-async function dismissReviews({ github, context, core, dry }) {
+async function dismissReviews({ github, context, core, dry, reviewKey }) {
   const pull_number = context.payload.pull_request?.number
   if (!pull_number) {
     core.warning('dismissReviews called outside of pull_request context')
@@ -24,35 +25,92 @@ async function dismissReviews({ github, context, core, dry }) {
     return
   }
 
-  await Promise.all(
-    (
-      await github.paginate(github.rest.pulls.listReviews, {
+  const reviews = (
+    await github.paginate(github.rest.pulls.listReviews, {
+      ...context.repo,
+      pull_number,
+    })
+  ).filter(
+    (review) =>
+      review.user?.login === 'github-actions[bot]' &&
+      review.state !== 'DISMISSED',
+  )
+  const changesRequestedReviews = reviews.filter(
+    (review) => review.state === 'CHANGES_REQUESTED',
+  )
+
+  const commentRegex = new RegExp(
+    /<!-- nixpkgs review key: (.*)(?:; resolved: .*)? -->/,
+  )
+  const reviewKeyRegex = new RegExp(
+    `<!-- (nixpkgs review key: ${reviewKey})(?:; resolved: .*)? -->`,
+  )
+  const commentResolvedRegex = new RegExp(
+    /<!-- nixpkgs review key: .*; resolved: true -->/,
+  )
+
+  let reviewsToMinimize = reviews
+  let /** @type {typeof reviews} */ reviewsToDismiss = []
+  let /** @type {typeof reviews} */ reviewsToResolve = []
+
+  if (reviewKey && reviews.every((review) => commentRegex.test(review.body))) {
+    reviewsToMinimize = reviews.filter((review) =>
+      reviewKeyRegex.test(review.body),
+    )
+  }
+
+  // If we want to dismiss all reviews with the key reviewKey,
+  // but there are other requested changes from CI, we can't dismiss,
+  // because then the other requested changes will be dismissed too.
+  if (
+    changesRequestedReviews.every(
+      (review) =>
+        commentResolvedRegex.test(review.body) ||
+        (reviewKey && reviewKeyRegex.test(review.body)),
+    )
+  ) {
+    reviewsToDismiss = changesRequestedReviews
+  } else if (reviewsToMinimize.length) {
+    reviewsToResolve = reviewsToMinimize.filter(
+      (review) =>
+        review.state === 'CHANGES_REQUESTED' &&
+        !commentResolvedRegex.test(review.body),
+    )
+  }
+
+  await Promise.all([
+    ...reviewsToMinimize.map(async (review) =>
+      github.graphql(
+        `mutation($node_id:ID!) {
+              minimizeComment(input: {
+                classifier: OUTDATED,
+                subjectId: $node_id
+              })
+              { clientMutationId }
+            }`,
+        { node_id: review.node_id },
+      ),
+    ),
+    ...reviewsToDismiss.map(async (review) =>
+      github.rest.pulls.dismissReview({
         ...context.repo,
         pull_number,
-      })
-    )
-      .filter((review) => review.user?.login === 'github-actions[bot]')
-      .map(async (review) => {
-        if (review.state === 'CHANGES_REQUESTED') {
-          await github.rest.pulls.dismissReview({
-            ...context.repo,
-            pull_number,
-            review_id: review.id,
-            message: 'Review dismissed automatically',
-          })
-        }
-        await github.graphql(
-          `mutation($node_id:ID!) {
-            minimizeComment(input: {
-              classifier: OUTDATED,
-              subjectId: $node_id
-            })
-            { clientMutationId }
-          }`,
-          { node_id: review.node_id },
-        )
+        review_id: review.id,
+        message: 'Review dismissed automatically',
       }),
-  )
+    ),
+    ...reviewsToResolve.map(async (review) =>
+      github.rest.pulls.updateReview({
+        ...context.repo,
+        pull_number,
+        review_id: review.id,
+        body: review.body.replace(
+          reviewKeyRegex,
+          `<!-- nixpkgs review key: ${reviewKey}; resolved: true -->`,
+        ),
+      }),
+    ),
+  ])
 }
 
 /**
@@ -63,6 +121,7 @@ async function dismissReviews({ github, context, core, dry }) {
  *  dry: boolean,
  *  body: string,
  *  event: keyof eventToState,
+ *  reviewKey: string,
  * }} PostReviewProps
  */
 async function postReview({
@@ -72,6 +131,7 @@ async function postReview({
   dry,
   body,
   event = 'REQUEST_CHANGES',
+  reviewKey,
 }) {
   const pull_number = context.payload.pull_request?.number
   if (!pull_number) {
@@ -79,17 +139,47 @@ async function postReview({
     return
   }
 
-  const pendingReview = (
+  const reviewKeyRegex = new RegExp(
+    `<!-- (nixpkgs review key: ${reviewKey})(?:; resolved: .*)? -->`,
+  )
+  const reviewKeyComment = `<!-- nixpkgs review key: ${reviewKey}; resolved: false -->`
+  body = body + '\n\n' + reviewKeyComment
+
+  const reviews = (
     await github.paginate(github.rest.pulls.listReviews, {
       ...context.repo,
       pull_number,
     })
-  ).find(
+  ).filter(
     (review) =>
       review.user?.login === 'github-actions[bot]' &&
-      review.state === eventToState[event],
+      review.state !== 'DISMISSED',
   )
 
+  /** @type {null | typeof reviews[number]} */
+  let pendingReview
+  const matchingReviews = reviews.filter((review) =>
+    reviewKeyRegex.test(review.body),
+  )
+
+  if (matchingReviews.length === 0) {
+    pendingReview = null
+  } else if (
+    matchingReviews.length === 1 &&
+    matchingReviews[0].state === eventToState[event]
+  ) {
+    pendingReview = matchingReviews[0]
+  } else {
+    await dismissReviews({
+      github,
+      context,
+      core,
+      dry,
+      reviewKey,
+    })
+    pendingReview = null
+  }
+
   if (dry) {
     if (pendingReview)
       core.info(`pending review found: ${pendingReview.html_url}`)
@@ -97,12 +187,23 @@ async function postReview({
     core.info(body)
   } else {
     if (pendingReview) {
-      await github.rest.pulls.updateReview({
-        ...context.repo,
-        pull_number,
-        review_id: pendingReview.id,
-        body,
-      })
+      await Promise.all([
+        github.rest.pulls.updateReview({
+          ...context.repo,
+          pull_number,
+          review_id: pendingReview.id,
+          body,
+        }),
+        github.graphql(
+          `mutation($node_id:ID!) {
+              unminimizeComment(input: {
+                subjectId: $node_id
+              })
+              { clientMutationId }
+            }`,
+          { node_id: pendingReview.node_id },
+        ),
+      ])
     } else {
       await github.rest.pulls.createReview({
         ...context.repo,