From 4447561c050f653a70a2fdd818a4b66456d99cfe Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 24 Jan 2026 09:47:58 -0500 Subject: [PATCH] ci/github-script/{reviews,check-target-branch,commits,prepare}: add sticky review support --- ci/github-script/check-target-branch.js | 33 ++++- ci/github-script/commits.js | 5 +- ci/github-script/prepare.js | 9 +- ci/github-script/reviews.js | 173 +++++++++++++++++++----- 4 files changed, 177 insertions(+), 43 deletions(-) diff --git a/ci/github-script/check-target-branch.js b/ci/github-script/check-target-branch.js index 2b671715d37d..653287ce6561 100644 --- a/ci/github-script/check-target-branch.js +++ b/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, @@ -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, + }) } } diff --git a/ci/github-script/commits.js b/ci/github-script/commits.js index 91cfe00fa58c..56873c158af4 100644 --- a/ci/github-script/commits.js +++ b/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 }) }) } diff --git a/ci/github-script/prepare.js b/ci/github-script/prepare.js index ce058edd2af7..005626c43ab5 100644 --- a/ci/github-script/prepare.js +++ b/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) { diff --git a/ci/github-script/reviews.js b/ci/github-script/reviews.js index 786073da2995..0dacaa814e62 100644 --- a/ci/github-script/reviews.js +++ b/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( + //, + ) + const reviewKeyRegex = new RegExp( + ``, + ) + const commentResolvedRegex = new RegExp( + //, + ) + + 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, + ``, + ), + }), + ), + ]) } /** @@ -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( + ``, + ) + const reviewKeyComment = `` + 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,