From 82d83c564c2d493d1a498c497eaa12bcdc24027d Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 17 Jan 2026 19:39:29 -0500 Subject: [PATCH 1/4] ci/supportedBranches: export split() --- ci/supportedBranches.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/supportedBranches.js b/ci/supportedBranches.js index 003b437613f7..2f4bf5abfb40 100755 --- a/ci/supportedBranches.js +++ b/ci/supportedBranches.js @@ -44,7 +44,7 @@ function classify(branch) { } } -module.exports = { classify } +module.exports = { classify, split } // If called directly via CLI, runs the following tests: if (!module.parent) { From e05e9846892ea740ce26d8a07f74b82adf08b6fc Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 17 Jan 2026 20:59:12 -0500 Subject: [PATCH 2/4] ci/github-script/reviews: rework dismissal/non-posting logic Dismissals are done automatically by commits.js, even for reviews from check-target-branches.js. This is not desirable. The solution is (1) do not decline to post a review because it was already dismissed (because it may have not been dismissed by a human, and circumstances may have changed), and (2) reword the auto-dismissal message to not imply that whatever problems were present are fixed. --- ci/github-script/reviews.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ci/github-script/reviews.js b/ci/github-script/reviews.js index f10f141d84cb..ef8f4272c02c 100644 --- a/ci/github-script/reviews.js +++ b/ci/github-script/reviews.js @@ -19,13 +19,13 @@ async function dismissReviews({ github, context, dry }) { ...context.repo, pull_number, review_id: review.id, - message: 'All good now, thank you!', + message: 'Review dismissed automatically', }) } await github.graphql( `mutation($node_id:ID!) { minimizeComment(input: { - classifier: RESOLVED, + classifier: OUTDATED, subjectId: $node_id }) { clientMutationId } @@ -49,10 +49,7 @@ async function postReview({ github, context, core, dry, body }) { 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 === 'CHANGES_REQUESTED' || - // No need to post a new review, if an older one with the exact - // same content had already been dismissed. - review.body === body), + review.state === 'CHANGES_REQUESTED', ) if (dry) { From 0ba1ea61e32e1af98324311932d91d6caba4cc94 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sun, 18 Jan 2026 19:56:44 -0500 Subject: [PATCH 3/4] ci/github-script/reviews: allow leaving review comments --- ci/github-script/reviews.js | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/ci/github-script/reviews.js b/ci/github-script/reviews.js index ef8f4272c02c..e2880f3c5a01 100644 --- a/ci/github-script/reviews.js +++ b/ci/github-script/reviews.js @@ -1,3 +1,16 @@ +const eventToState = { + COMMENT: 'COMMENTED', + REQUEST_CHANGES: 'CHANGES_REQUESTED', +} + +/** + * @param {{ + * github: InstanceType, + * context: import('@actions/github/lib/context').Context + * core: import('@actions/core') + * dry: boolean + * }} CheckTargetBranchProps + */ async function dismissReviews({ github, context, dry }) { const pull_number = context.payload.pull_request.number @@ -36,7 +49,14 @@ async function dismissReviews({ github, context, dry }) { ) } -async function postReview({ github, context, core, dry, body }) { +async function postReview({ + github, + context, + core, + dry, + body, + event = 'REQUEST_CHANGES', +}) { const pull_number = context.payload.pull_request.number const pendingReview = ( @@ -49,7 +69,7 @@ async function postReview({ github, context, core, dry, body }) { 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 === 'CHANGES_REQUESTED', + review.state === eventToState[event], ) if (dry) { @@ -69,7 +89,7 @@ async function postReview({ github, context, core, dry, body }) { await github.rest.pulls.createReview({ ...context.repo, pull_number, - event: 'REQUEST_CHANGES', + event, body, }) } From a4d5f8a6b11edf4c66858fb4dcd9a1bfde49df60 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 17 Jan 2026 19:42:08 -0500 Subject: [PATCH 4/4] {workflows/eval,ci/github-script}: check for mass rebuilds targeting master/release-* branches --- .github/workflows/eval.yml | 12 ++ .github/workflows/merge-group.yml | 1 + .github/workflows/pull-request-target.yml | 1 + ci/github-script/check-target-branch.js | 135 ++++++++++++++++++++++ ci/github-script/run | 11 ++ 5 files changed, 160 insertions(+) create mode 100644 ci/github-script/check-target-branch.js diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 28b5e8c99f61..390f19d96e2f 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -168,6 +168,7 @@ jobs: needs: [eval] if: ${{ !cancelled() && !failure() }} permissions: + pull-requests: write statuses: write timeout-minutes: 5 steps: @@ -254,6 +255,17 @@ jobs: description, target_url }) + - name: Request changes if PR is against an inappropriate branch + if: ${{ github.event_name == 'pull_request_target' }} + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + script: | + require('./nixpkgs/trusted/ci/github-script/check-target-branch.js')({ + github, + context, + core, + dry: context.eventName == 'pull_request', + }) # Creates a matrix of Eval performance for various versions and systems. report: diff --git a/.github/workflows/merge-group.yml b/.github/workflows/merge-group.yml index 299f56c09dfe..0720817c6da7 100644 --- a/.github/workflows/merge-group.yml +++ b/.github/workflows/merge-group.yml @@ -84,6 +84,7 @@ jobs: # even though they are unused when working with the merge queue. permissions: # compare + pull-requests: write statuses: write secrets: CACHIX_AUTH_TOKEN_GHA: ${{ secrets.CACHIX_AUTH_TOKEN_GHA }} diff --git a/.github/workflows/pull-request-target.yml b/.github/workflows/pull-request-target.yml index 9609a440ae89..c49e2ae60d37 100644 --- a/.github/workflows/pull-request-target.yml +++ b/.github/workflows/pull-request-target.yml @@ -81,6 +81,7 @@ jobs: uses: ./.github/workflows/eval.yml permissions: # compare + pull-requests: write statuses: write with: artifact-prefix: ${{ inputs.artifact-prefix }} diff --git a/ci/github-script/check-target-branch.js b/ci/github-script/check-target-branch.js new file mode 100644 index 000000000000..2b671715d37d --- /dev/null +++ b/ci/github-script/check-target-branch.js @@ -0,0 +1,135 @@ +/// @ts-check + +// TODO: should this be combined with the branch checks in prepare.js? +// They do seem quite similar, but this needs to run after eval, +// and prepare.js obviously doesn't. + +const { classify, split } = require('../supportedBranches.js') +const { readFile } = require('node:fs/promises') +const { postReview } = require('./reviews.js') + +/** + * @param {{ + * github: InstanceType, + * context: import('@actions/github/lib/context').Context + * core: import('@actions/core') + * dry: boolean + * }} CheckTargetBranchProps + */ +async function checkTargetBranch({ github, context, core, dry }) { + const changed = JSON.parse( + await readFile('comparison/changed-paths.json', 'utf-8'), + ) + const pull_number = context.payload.pull_request?.number + if (!pull_number) { + core.warning( + 'Skipping checkTargetBranch: no pull_request number (is this being run as part of a merge group?)', + ) + return + } + const prInfo = ( + await github.rest.pulls.get({ + ...context.repo, + pull_number, + }) + ).data + const base = prInfo.base.ref + const head = prInfo.head.ref + const baseClassification = classify(base) + const headClassification = classify(head) + + // Don't run on, e.g., staging-nixos to master merges. + if (headClassification.type.includes('development')) { + core.info( + `Skipping checkTargetBranch: PR is from a development branch (${head})`, + ) + return + } + // Don't run on PRs against staging branches, wip branches, haskell-updates, etc. + if (!baseClassification.type.includes('primary')) { + core.info( + `Skipping checkTargetBranch: PR is against a non-primary base branch (${base})`, + ) + return + } + + const maxRebuildCount = Math.max( + ...Object.values(changed.rebuildCountByKernel), + ) + const rebuildsAllTests = + changed.attrdiff.changed.includes('nixosTests.simple') ?? false + core.info( + `checkTargetBranch: PR causes ${maxRebuildCount} rebuilds and ${rebuildsAllTests ? 'does' : 'does not'} rebuild all NixOS tests.`, + ) + + if (maxRebuildCount >= 1000) { + const desiredBranch = + base === 'master' ? 'staging' : `staging-${split(base).version}` + const body = [ + `The PR's base branch is set to \`${base}\`, but this PR causes ${maxRebuildCount} rebuilds.`, + 'It is therefore considered a mass rebuild.', + `Please [change the base branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request) to [the right base branch for your changes](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions) (probably \`${desiredBranch}\`).`, + ].join('\n') + + await postReview({ + github, + context, + core, + dry, + body, + event: 'REQUEST_CHANGES', + }) + + throw new Error('This PR is against the wrong branch.') + } else if (rebuildsAllTests) { + let branchText + if (base === 'master' && maxRebuildCount >= 500) { + branchText = '(probably either `staging-nixos` or `staging`)' + } else if (base === 'master') { + branchText = '(probably `staging-nixos`)' + } else { + branchText = `(probably \`staging-${split(base).version}\`)` + } + const body = [ + `The PR's base branch is set to \`${base}\`, but this PR rebuilds all NixOS tests.`, + base === 'master' && maxRebuildCount >= 500 + ? `Since this PR also causes ${maxRebuildCount} rebuilds, it may also be considered a mass rebuild.` + : '', + `Please [change the base branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request) to [the right base branch for your changes](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions) ${branchText}.`, + ].join('\n') + + await postReview({ + github, + context, + core, + dry, + body, + event: 'REQUEST_CHANGES', + }) + + throw new Error('This PR is against the wrong branch.') + } else if (maxRebuildCount >= 500) { + const stagingBranch = + base === 'master' ? 'staging' : `staging-${split(base).version}` + const body = [ + `The PR's base branch is set to \`${base}\`, and this PR causes ${maxRebuildCount} rebuilds.`, + `Please consider whether this PR causes a mass rebuild according to [our conventions](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions).`, + `If it does cause a mass rebuild, please [change the base branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request) to [the right base branch for your changes](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions) (probably \`${stagingBranch}\`).`, + `If it does not cause a mass rebuild, this message can be ignored.`, + ].join('\n') + + await postReview({ + github, + context, + core, + dry, + body, + event: 'COMMENT', + }) + } else { + // Any existing reviews were dismissed by commits.js + core.info('checkTargetBranch: this PR is against an appropriate branch.') + } +} + +module.exports = checkTargetBranch diff --git a/ci/github-script/run b/ci/github-script/run index 095768f317da..744e49f018a4 100755 --- a/ci/github-script/run +++ b/ci/github-script/run @@ -105,4 +105,15 @@ program await run(checkCommitMessages, owner, repo, pr, options) }) +program + .command('check-target-branch') + .description('Check that the PR is made against the correct branch') + .argument('', 'Owner of the GitHub repository to run on (Example: NixOS)') + .argument('', 'Name of the GitHub repository to run on (Example: nixpkgs)') + .argument('', 'Number of the Pull Request to run on') + .action(async (owner, repo, pr, options) => { + const checkCommitMessages = (await import('./check-target-branch.js')).default + await run(checkCommitMessages, owner, repo, pr, options) + }) + await program.parse()