From 3fb23e683762fe516331c1b42fb5270697d6c315 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 24 Jan 2026 09:31:36 -0500 Subject: [PATCH 1/7] ci/github-script/reviews: enable typescript --- ci/github-script/reviews.js | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/ci/github-script/reviews.js b/ci/github-script/reviews.js index e2880f3c5a01..786073da2995 100644 --- a/ci/github-script/reviews.js +++ b/ci/github-script/reviews.js @@ -1,3 +1,5 @@ +// @ts-check + const eventToState = { COMMENT: 'COMMENTED', REQUEST_CHANGES: 'CHANGES_REQUESTED', @@ -6,13 +8,17 @@ const eventToState = { /** * @param {{ * github: InstanceType, - * context: import('@actions/github/lib/context').Context - * core: import('@actions/core') - * dry: boolean - * }} CheckTargetBranchProps + * context: import('@actions/github/lib/context').Context, + * core: import('@actions/core'), + * dry: boolean, + * }} DismissReviewsProps */ -async function dismissReviews({ github, context, dry }) { - const pull_number = context.payload.pull_request.number +async function dismissReviews({ github, context, core, dry }) { + const pull_number = context.payload.pull_request?.number + if (!pull_number) { + core.warning('dismissReviews called outside of pull_request context') + return + } if (dry) { return @@ -49,6 +55,16 @@ async function dismissReviews({ github, context, dry }) { ) } +/** + * @param {{ + * github: InstanceType, + * context: import('@actions/github/lib/context').Context + * core: import('@actions/core'), + * dry: boolean, + * body: string, + * event: keyof eventToState, + * }} PostReviewProps + */ async function postReview({ github, context, @@ -57,7 +73,11 @@ async function postReview({ body, event = 'REQUEST_CHANGES', }) { - const pull_number = context.payload.pull_request.number + const pull_number = context.payload.pull_request?.number + if (!pull_number) { + core.warning('postReview called outside of pull_request context') + return + } const pendingReview = ( await github.paginate(github.rest.pulls.listReviews, { @@ -67,8 +87,6 @@ async function postReview({ ).find( (review) => review.user?.login === 'github-actions[bot]' && - // If a review is still pending, we can just update this instead - // of posting a new one. review.state === eventToState[event], ) From 4447561c050f653a70a2fdd818a4b66456d99cfe Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 24 Jan 2026 09:47:58 -0500 Subject: [PATCH 2/7] 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, From 99c1a35b73f9d7463d4be672a1e9b749b45c7b48 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 24 Jan 2026 10:19:44 -0500 Subject: [PATCH 3/7] ci/github-script/check-target-branch: add type for changed-paths.json --- ci/github-script/check-target-branch.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ci/github-script/check-target-branch.js b/ci/github-script/check-target-branch.js index 653287ce6561..f17953b7ce8c 100644 --- a/ci/github-script/check-target-branch.js +++ b/ci/github-script/check-target-branch.js @@ -18,6 +18,19 @@ const reviewKey = 'check-target-branch' * }} CheckTargetBranchProps */ async function checkTargetBranch({ github, context, core, dry }) { + /** + * @type {{ + * attrdiff: { + * added: string[], + * changed: string[], + * removed: string[], + * }, + * labels: Record, + * rebuildCountByKernel: Record, + * rebuildsByKernel: Record, + * rebuildsByPlatform: Record, + * }} + */ const changed = JSON.parse( await readFile('comparison/changed-paths.json', 'utf-8'), ) From 50590d21d52048982e60ecd7980030d76d6c57a9 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 24 Jan 2026 10:41:49 -0500 Subject: [PATCH 4/7] ci/github-script/check-target-branch: simplify rebuildsAllTests variable --- ci/github-script/check-target-branch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/github-script/check-target-branch.js b/ci/github-script/check-target-branch.js index f17953b7ce8c..8c28921b52ce 100644 --- a/ci/github-script/check-target-branch.js +++ b/ci/github-script/check-target-branch.js @@ -89,7 +89,7 @@ async function checkTargetBranch({ github, context, core, dry }) { ...Object.values(changed.rebuildCountByKernel), ) const rebuildsAllTests = - changed.attrdiff.changed.includes('nixosTests.simple') ?? false + changed.attrdiff.changed.includes('nixosTests.simple') core.info( `checkTargetBranch: PR causes ${maxRebuildCount} rebuilds and ${rebuildsAllTests ? 'does' : 'does not'} rebuild all NixOS tests.`, ) From cd6bc04a06a70c80fd95416682ddccd0240cf0bc Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 24 Jan 2026 10:00:11 -0500 Subject: [PATCH 5/7] ci/github-script/check-target-branch: loosen staging threshold for home-assistant --- ci/github-script/check-target-branch.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ci/github-script/check-target-branch.js b/ci/github-script/check-target-branch.js index 8c28921b52ce..9f79388d9b04 100644 --- a/ci/github-script/check-target-branch.js +++ b/ci/github-script/check-target-branch.js @@ -90,11 +90,21 @@ async function checkTargetBranch({ github, context, core, dry }) { ) const rebuildsAllTests = changed.attrdiff.changed.includes('nixosTests.simple') + + // https://github.com/NixOS/nixpkgs/pull/483194#issuecomment-3793393218 + const isExemptHomeAssistantUpdate = + maxRebuildCount <= 1500 && head === 'wip-home-assistant' + core.info( - `checkTargetBranch: PR causes ${maxRebuildCount} rebuilds and ${rebuildsAllTests ? 'does' : 'does not'} rebuild all NixOS tests.`, + [ + `checkTargetBranch: this PR:`, + ` * causes ${maxRebuildCount} rebuilds`, + ` * ${rebuildsAllTests ? 'rebuilds' : 'does not rebuild'} all NixOS tests`, + ` * ${isExemptHomeAssistantUpdate ? 'is' : 'is not'} an exempt home-assistant update`, + ].join('\n'), ) - if (maxRebuildCount >= 1000) { + if (maxRebuildCount >= 1000 && !isExemptHomeAssistantUpdate) { const desiredBranch = base === 'master' ? 'staging' : `staging-${split(base).version}` const body = [ @@ -142,7 +152,7 @@ async function checkTargetBranch({ github, context, core, dry }) { }) throw new Error('This PR is against the wrong branch.') - } else if (maxRebuildCount >= 500) { + } else if (maxRebuildCount >= 500 && !isExemptHomeAssistantUpdate) { const stagingBranch = base === 'master' ? 'staging' : `staging-${split(base).version}` const body = [ From e1e5b8ad914da75f9b4fb3eef83d0950b8b17b58 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 24 Jan 2026 10:45:22 -0500 Subject: [PATCH 6/7] ci/github-script/check-target-branch: kernel updates should not need to go to staging-xx.xx --- ci/github-script/check-target-branch.js | 26 +++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/ci/github-script/check-target-branch.js b/ci/github-script/check-target-branch.js index 9f79388d9b04..339968e6a0bb 100644 --- a/ci/github-script/check-target-branch.js +++ b/ci/github-script/check-target-branch.js @@ -91,6 +91,23 @@ async function checkTargetBranch({ github, context, core, dry }) { const rebuildsAllTests = changed.attrdiff.changed.includes('nixosTests.simple') + // https://github.com/NixOS/nixpkgs/pull/481205#issuecomment-3790123921 + // These should go to staging-nixos instead of master, + // but release-xx.xx (not staging-xx.xx) when backported + let isExemptKernelUpdate = false + if (prInfo.changed_files === 1 && base.startsWith('release-')) { + const changedFiles = ( + await github.rest.pulls.listFiles({ + ...context.repo, + pull_number, + }) + ).data + isExemptKernelUpdate = + changedFiles.length === 1 && + changedFiles[0].filename === + 'pkgs/os-specific/linux/kernel/kernels-org.json' + } + // https://github.com/NixOS/nixpkgs/pull/483194#issuecomment-3793393218 const isExemptHomeAssistantUpdate = maxRebuildCount <= 1500 && head === 'wip-home-assistant' @@ -100,6 +117,7 @@ async function checkTargetBranch({ github, context, core, dry }) { `checkTargetBranch: this PR:`, ` * causes ${maxRebuildCount} rebuilds`, ` * ${rebuildsAllTests ? 'rebuilds' : 'does not rebuild'} all NixOS tests`, + ` * ${isExemptKernelUpdate ? 'is' : 'is not'} an exempt kernel update`, ` * ${isExemptHomeAssistantUpdate ? 'is' : 'is not'} an exempt home-assistant update`, ].join('\n'), ) @@ -124,7 +142,7 @@ async function checkTargetBranch({ github, context, core, dry }) { }) throw new Error('This PR is against the wrong branch.') - } else if (rebuildsAllTests) { + } else if (rebuildsAllTests && !isExemptKernelUpdate) { let branchText if (base === 'master' && maxRebuildCount >= 500) { branchText = '(probably either `staging-nixos` or `staging`)' @@ -152,7 +170,11 @@ async function checkTargetBranch({ github, context, core, dry }) { }) throw new Error('This PR is against the wrong branch.') - } else if (maxRebuildCount >= 500 && !isExemptHomeAssistantUpdate) { + } else if ( + maxRebuildCount >= 500 && + !isExemptKernelUpdate && + !isExemptHomeAssistantUpdate + ) { const stagingBranch = base === 'master' ? 'staging' : `staging-${split(base).version}` const body = [ From c9225057c92ecb6084cad809239aa940f0e2f047 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sun, 25 Jan 2026 19:10:17 -0500 Subject: [PATCH 7/7] ci/github-script/check-target-branch: do not "Request changes" You can technically have multiple reviews requesting changes, but there's no good way to dismiss all of the reviews requesting changes from the same user using the UI. This makes minimization impossible (because all but one of the reviews is not dismissed, even though the PR is no longer blocked due to the review in GitHub's system). As a workaround, we will only comment. CI will still fail when appropriate. --- ci/github-script/check-target-branch.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/github-script/check-target-branch.js b/ci/github-script/check-target-branch.js index 339968e6a0bb..47e36b9cb3f6 100644 --- a/ci/github-script/check-target-branch.js +++ b/ci/github-script/check-target-branch.js @@ -137,7 +137,7 @@ async function checkTargetBranch({ github, context, core, dry }) { core, dry, body, - event: 'REQUEST_CHANGES', + event: 'COMMENT', reviewKey, }) @@ -165,7 +165,7 @@ async function checkTargetBranch({ github, context, core, dry }) { core, dry, body, - event: 'REQUEST_CHANGES', + event: 'COMMENT', reviewKey, })