diff --git a/.github/labeler-development-branches.yml b/.github/labeler-development-branches.yml index 5ce799c2f8d4..bb1c47a23fa0 100644 --- a/.github/labeler-development-branches.yml +++ b/.github/labeler-development-branches.yml @@ -1,4 +1,4 @@ -# This file is used by .github/workflows/labels.yml +# This file is used by .github/workflows/bot.yml # This version is only run for Pull Requests from development branches like staging-next, haskell-updates or python-updates. "4.workflow: package set update": diff --git a/.github/labeler-no-sync.yml b/.github/labeler-no-sync.yml index 69249f6ac27d..99d1b85c3c7b 100644 --- a/.github/labeler-no-sync.yml +++ b/.github/labeler-no-sync.yml @@ -1,4 +1,4 @@ -# This file is used by .github/workflows/labels.yml +# This file is used by .github/workflows/bot.yml # This version uses `sync-labels: false`, meaning that a non-match will NOT remove the label # keep-sorted start case=no numeric=yes newline_separated=yes skip_lines=1 diff --git a/.github/labeler.yml b/.github/labeler.yml index adfc49ef6ee2..abec7c37288c 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -1,4 +1,4 @@ -# This file is used by .github/workflows/labels.yml +# This file is used by .github/workflows/bot.yml # This version uses `sync-labels: true`, meaning that a non-match will remove the label # keep-sorted start case=no numeric=yes newline_separated=yes skip_lines=1 diff --git a/.github/workflows/labels.yml b/.github/workflows/bot.yml similarity index 90% rename from .github/workflows/labels.yml rename to .github/workflows/bot.yml index e3dfe2642e50..cb1df7bdb0a1 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/bot.yml @@ -3,7 +3,7 @@ # access to the GitHub API. This means that it should not evaluate user input in # a way that allows code injection. -name: Labels +name: Bot on: schedule: @@ -21,7 +21,7 @@ on: concurrency: # This explicitly avoids using `run_id` for the concurrency key to make sure that only # *one* scheduled run can run at a time. - group: labels-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number }} + group: bot-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number }} # PR-triggered runs will be cancelled, but scheduled runs will be queued. cancel-in-progress: ${{ github.event_name != 'schedule' }} @@ -36,9 +36,13 @@ defaults: shell: bash jobs: - update: + run: runs-on: ubuntu-24.04-arm if: github.event_name != 'schedule' || github.repository_owner == 'NixOS' + env: + # TODO: Remove after 2026-03-04, when Node 24 becomes the default. + # https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: @@ -56,6 +60,7 @@ jobs: with: app-id: ${{ vars.NIXPKGS_CI_APP_ID }} private-key: ${{ secrets.NIXPKGS_CI_APP_PRIVATE_KEY }} + permission-contents: write permission-issues: write permission-pull-requests: write @@ -64,13 +69,13 @@ jobs: GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} run: gh api /rate_limit | jq - - name: Labels from API data and Eval results + - name: Run bot uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 with: github-token: ${{ steps.app-token.outputs.token || github.token }} retries: 3 script: | - require('./ci/github-script/labels.js')({ + require('./ci/github-script/bot.js')({ github, context, core, diff --git a/.github/workflows/comment.yml b/.github/workflows/comment.yml new file mode 100644 index 000000000000..d09df6956c8c --- /dev/null +++ b/.github/workflows/comment.yml @@ -0,0 +1,54 @@ +name: Comment + +on: + issue_comment: + types: [created] + +# This is used as fallback without app only. +# This happens when testing in forks without setting up that app. +permissions: + pull-requests: write + +defaults: + run: + shell: bash + +jobs: + # The `bot` workflow reacts to comments with @NixOS/nixpkgs-merge-bot references, but might only + # pick up a comment after up to 10 minutes. To give the user instant feedback, this job adds + # a reaction to these comments. + react: + name: React with eyes + runs-on: ubuntu-24.04-arm + timeout-minutes: 2 + if: contains(github.event.comment.body, '@NixOS/nixpkgs-merge-bot merge') + steps: + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + with: + persist-credentials: false + sparse-checkout: | + ci/github-script + + # Use the GitHub App to make sure the reaction happens with the same user who will later merge. + - uses: actions/create-github-app-token@67018539274d69449ef7c02e8e71183d1719ab42 # v2.1.4 + if: github.event_name != 'pull_request' && vars.NIXPKGS_CI_APP_ID + id: app-token + with: + app-id: ${{ vars.NIXPKGS_CI_APP_ID }} + private-key: ${{ secrets.NIXPKGS_CI_APP_PRIVATE_KEY }} + permission-pull-requests: write + + - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + github-token: ${{ steps.app-token.outputs.token || github.token }} + retries: 3 + script: | + const { handleMergeComment } = require('./ci/github-script/merge.js') + const { body, node_id } = context.payload.comment + + await handleMergeComment({ + github, + body, + node_id, + reaction: 'EYES', + }) diff --git a/.github/workflows/dismissed-review.yml b/.github/workflows/dismissed-review.yml deleted file mode 100644 index db5799c80913..000000000000 --- a/.github/workflows/dismissed-review.yml +++ /dev/null @@ -1,66 +0,0 @@ -name: Dismissed review - -on: - workflow_run: - workflows: - - Review dismissed - types: [completed] - -concurrency: - group: dismissed-review-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} - cancel-in-progress: true - -permissions: - pull-requests: write - -defaults: - run: - shell: bash - -jobs: - # The `check-cherry-picks` workflow creates review comments which reviewers - # are encouraged to manually dismiss if they're not relevant. - # When a CI-generated review is dismissed, this job automatically minimizes - # it, preventing it from cluttering the PR. - minimize: - name: Minimize as resolved - runs-on: ubuntu-24.04-arm - timeout-minutes: 2 - steps: - - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 - with: - script: | - // PRs from forks don't have any PRs associated by default. - // Thus, we request the PR number with an API call *to* the fork's repo. - // Multiple pull requests can be open from the same head commit, either via - // different base branches or head branches. - const { head_repository, head_sha, repository } = context.payload.workflow_run - await Promise.all( - (await github.paginate(github.rest.repos.listPullRequestsAssociatedWithCommit, { - owner: head_repository.owner.login, - repo: head_repository.name, - commit_sha: head_sha - })) - .filter(pull_request => pull_request.base.repo.id == repository.id) - .map(async (pull_request) => - Promise.all( - (await github.paginate(github.rest.pulls.listReviews, { - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: pull_request.number - })).filter(review => - review.user?.login == 'github-actions[bot]' && - review.state == 'DISMISSED' - ).map(review => github.graphql(` - mutation($node_id:ID!) { - minimizeComment(input: { - classifier: RESOLVED, - subjectId: $node_id - }) - { clientMutationId } - }`, - { node_id: review.node_id } - )) - ) - ) - ) diff --git a/.github/workflows/pull-request-target.yml b/.github/workflows/pull-request-target.yml index 8a305a953933..d73f6fdc7e5c 100644 --- a/.github/workflows/pull-request-target.yml +++ b/.github/workflows/pull-request-target.yml @@ -100,10 +100,10 @@ jobs: systems: ${{ needs.prepare.outputs.systems }} testVersions: ${{ contains(fromJSON(needs.prepare.outputs.touched), 'pinned') && !contains(fromJSON(needs.prepare.outputs.headBranch).type, 'development') }} - labels: - name: Labels + bot: + name: Bot needs: [prepare, eval] - uses: ./.github/workflows/labels.yml + uses: ./.github/workflows/bot.yml permissions: issues: write pull-requests: write diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml new file mode 100644 index 000000000000..44ac3f0088d4 --- /dev/null +++ b/.github/workflows/review.yml @@ -0,0 +1,92 @@ +name: Review + +on: + workflow_run: + workflows: + - Reviewed + types: [completed] + +# This is used as fallback without app only. +# This happens when testing in forks without setting up that app. +permissions: + pull-requests: write + +defaults: + run: + shell: bash + +jobs: + process: + runs-on: ubuntu-24.04-arm + timeout-minutes: 2 + steps: + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + with: + persist-credentials: false + sparse-checkout: | + ci/github-script + + # Use the GitHub App to make sure the reaction happens with the same user who will later merge. + - uses: actions/create-github-app-token@67018539274d69449ef7c02e8e71183d1719ab42 # v2.1.4 + if: github.event_name != 'pull_request' && vars.NIXPKGS_CI_APP_ID + id: app-token + with: + app-id: ${{ vars.NIXPKGS_CI_APP_ID }} + private-key: ${{ secrets.NIXPKGS_CI_APP_PRIVATE_KEY }} + permission-pull-requests: write + + - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + github-token: ${{ steps.app-token.outputs.token || github.token }} + retries: 3 + script: | + const { handleMergeComment } = require('./ci/github-script/merge.js') + + // PRs from forks don't have any PRs associated by default. + // Thus, we request the PR number with an API call *to* the fork's repo. + // Multiple pull requests can be open from the same head commit, either via + // different base branches or head branches. + const { head_repository, head_sha, repository } = context.payload.workflow_run + await Promise.all( + (await github.paginate(github.rest.repos.listPullRequestsAssociatedWithCommit, { + owner: head_repository.owner.login, + repo: head_repository.name, + commit_sha: head_sha + })) + .filter(pull_request => pull_request.base.repo.id == repository.id) + .map(async (pull_request) => + Promise.all( + (await github.paginate(github.rest.pulls.listReviews, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pull_request.number + })).map(review => { + // The `check` workflow creates review comments which reviewers + // are encouraged to manually dismiss if they're not relevant. + // When a CI-generated review is dismissed, this job automatically minimizes + // it, preventing it from cluttering the PR. + if (review.user?.login == 'github-actions[bot]' && review.state == 'DISMISSED') + return github.graphql(` + mutation($node_id:ID!) { + minimizeComment(input: { + classifier: RESOLVED, + subjectId: $node_id + }) + { clientMutationId } + }`, + { node_id: review.node_id } + ) + + // The `bot` workflow reacts to comments with @NixOS/nixpkgs-merge-bot references, but might only + // pick up a comment after up to 10 minutes. To give the user instant feedback, this job adds + // a reaction to these comments. + return handleMergeComment({ + github, + body: review.body, + node_id: review.node_id, + reaction: 'EYES', + }) + }) + ) + ) + ) diff --git a/.github/workflows/review-dismissed.yml b/.github/workflows/reviewed.yml similarity index 81% rename from .github/workflows/review-dismissed.yml rename to .github/workflows/reviewed.yml index 988b4a47df14..d7d368739eaa 100644 --- a/.github/workflows/review-dismissed.yml +++ b/.github/workflows/reviewed.yml @@ -1,8 +1,8 @@ -name: Review dismissed +name: Reviewed on: pull_request_review: - types: [dismissed] + types: [submitted, dismissed] permissions: {} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b26f7ee98998..f938ea5f64b1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -58,10 +58,10 @@ jobs: if (files.some(file => [ '.github/actions/checkout/action.yml', + '.github/workflows/bot.yml', '.github/workflows/build.yml', '.github/workflows/check.yml', '.github/workflows/eval.yml', - '.github/workflows/labels.yml', '.github/workflows/lint.yml', '.github/workflows/pull-request-target.yml', '.github/workflows/reviewers.yml', diff --git a/ci/github-script/labels.js b/ci/github-script/bot.js similarity index 97% rename from ci/github-script/labels.js rename to ci/github-script/bot.js index bd6c81bd9d99..e0a67a747c82 100644 --- a/ci/github-script/labels.js +++ b/ci/github-script/bot.js @@ -4,6 +4,7 @@ module.exports = async ({ github, context, core, dry }) => { const { readFile, writeFile } = require('node:fs/promises') const withRateLimit = require('./withRateLimit.js') const { classify } = require('../supportedBranches.js') + const { handleMerge } = require('./merge.js') const artifactClient = new DefaultArtifactClient() @@ -95,7 +96,7 @@ module.exports = async ({ github, context, core, dry }) => { return maintainerMaps[branch] } - async function handlePullRequest({ item, stats }) { + async function handlePullRequest({ item, stats, events }) { const log = (k, v) => core.info(`PR #${item.number} - ${k}: ${v}`) const pull_number = item.number @@ -109,6 +110,19 @@ module.exports = async ({ github, context, core, dry }) => { }) ).data + const maintainers = await getMaintainerMap(pull_request.base.ref) + + await handleMerge({ + github, + context, + core, + log, + dry, + pull_request, + events, + maintainers, + }) + // When the same change has already been merged to the target branch, a PR will still be // open and display the same changes - but will not actually have any effect. This causes // strange CI behavior, because the diff of the merge-commit is empty, no rebuilds will @@ -305,8 +319,6 @@ module.exports = async ({ github, context, core, dry }) => { ) } - const maintainers = await getMaintainerMap(pull_request.base.ref) - Object.assign(prLabels, evalLabels, { '11.by: package-maintainer': packages.length && @@ -377,9 +389,21 @@ module.exports = async ({ github, context, core, dry }) => { const itemLabels = {} + const events = await github.paginate( + github.rest.issues.listEventsForTimeline, + { + ...context.repo, + issue_number, + per_page: 100, + }, + ) + if (item.pull_request || context.payload.pull_request) { stats.prs++ - Object.assign(itemLabels, await handlePullRequest({ item, stats })) + Object.assign( + itemLabels, + await handlePullRequest({ item, stats, events }), + ) } else { stats.issues++ if (item.labels.some(({ name }) => name === '4.workflow: auto-close')) { @@ -391,13 +415,7 @@ module.exports = async ({ github, context, core, dry }) => { } const latest_event_at = new Date( - ( - await github.paginate(github.rest.issues.listEventsForTimeline, { - ...context.repo, - issue_number, - per_page: 100, - }) - ) + events .filter(({ event }) => [ // These events are hand-picked from: @@ -484,7 +502,7 @@ module.exports = async ({ github, context, core, dry }) => { const lastRun = ( await github.rest.actions.listWorkflowRuns({ ...context.repo, - workflow_id: 'labels.yml', + workflow_id: 'bot.yml', event: 'schedule', status: 'success', exclude_pull_requests: true, diff --git a/ci/github-script/merge.js b/ci/github-script/merge.js new file mode 100644 index 000000000000..3c7576d659bb --- /dev/null +++ b/ci/github-script/merge.js @@ -0,0 +1,259 @@ +// Caching the list of committers saves API requests when running the bot on the schedule and +// processing many PRs at once. +let committers + +async function runChecklist({ github, context, pull_request, maintainers }) { + const pull_number = pull_request.number + + if (!committers) { + committers = github + .paginate(github.rest.teams.listMembersInOrg, { + org: context.repo.owner, + team_slug: 'nixpkgs-committers', + per_page: 100, + }) + .then((members) => new Set(members.map(({ id }) => id))) + } + + const files = await github.paginate(github.rest.pulls.listFiles, { + ...context.repo, + pull_number, + per_page: 100, + }) + + const packages = files + .filter(({ filename }) => filename.startsWith('pkgs/by-name/')) + .map(({ filename }) => filename.split('/')[3]) + + const eligible = !packages.length + ? new Set() + : packages + .map((pkg) => new Set(maintainers[pkg])) + .reduce((acc, cur) => acc?.intersection(cur) ?? cur) + + const checklist = { + 'PR targets one of the allowed branches: master, staging, staging-next.': [ + 'master', + 'staging', + 'staging-next', + ].includes(pull_request.base.ref), + 'PR touches only files in `pkgs/by-name/`.': files.every(({ filename }) => + filename.startsWith('pkgs/by-name/'), + ), + 'PR authored by r-ryantm or committer.': + pull_request.user.login === 'r-ryantm' || + (await committers).has(pull_request.user.id), + 'PR has maintainers eligible for merge.': eligible.size > 0, + } + + return { + checklist, + eligible, + result: Object.values(checklist).every(Boolean), + } +} + +// The merge command must be on a separate line and not within codeblocks or html comments. +// Codeblocks can have any number of ` larger than 3 to open/close. We only look at code +// blocks that are not indented, because the later regex wouldn't match those anyway. +function hasMergeCommand(body) { + return (body ?? '') + .replace(//gms, '') + .replace(/(^`{3,})[^`].*?\1/gms, '') + .match(/^@NixOS\/nixpkgs-merge-bot merge\s*$/m) +} + +async function handleMergeComment({ github, body, node_id, reaction }) { + if (!hasMergeCommand(body)) return + + await github.graphql( + `mutation($node_id: ID!, $reaction: ReactionContent!) { + addReaction(input: { + content: $reaction, + subjectId: $node_id + }) + { clientMutationId } + }`, + { node_id, reaction }, + ) +} + +async function handleMerge({ + github, + context, + core, + log, + dry, + pull_request, + events, + maintainers, +}) { + const pull_number = pull_request.number + + const { checklist, eligible, result } = await runChecklist({ + github, + context, + pull_request, + maintainers, + }) + log('checklist', JSON.stringify(checklist)) + log('eligible', JSON.stringify(Array.from(eligible))) + log('result', result) + + // Only look through comments *after* the latest (force) push. + const latestChange = events.findLast(({ event }) => + ['committed', 'head_ref_force_pushed'].includes(event), + ) ?? { sha: pull_request.head.sha } + const latestSha = latestChange.sha ?? latestChange.commit_id + log('latest sha', latestSha) + const latestIndex = events.indexOf(latestChange) + + const comments = events.slice(latestIndex + 1).filter( + ({ event, body, node_id }) => + ['commented', 'reviewed'].includes(event) && + hasMergeCommand(body) && + // Ignore comments which had already been responded to by the bot. + !events.some( + ({ event, user, body }) => + ['commented'].includes(event) && + // We're only testing this hidden reference, but not the author of the comment. + // We'll just assume that nobody creates comments with this marker on purpose. + // Additionally checking the author is quite annoying for local debugging. + body.match(new RegExp(`^$`, 'm')), + ), + ) + + async function merge() { + if (dry) { + core.info(`Merging #${pull_number}... (dry)`) + return 'Merge completed (dry)' + } + + // Using GraphQL's enablePullRequestAutoMerge mutation instead of the REST + // /merge endpoint, because the latter doesn't work with Merge Queues. + // This mutation works both with and without Merge Queues. + // It doesn't work when there are no required status checks for the target branch. + // All development branches have these enabled, so this is a non-issue. + try { + await github.graphql( + `mutation($node_id: ID!, $sha: GitObjectID) { + enablePullRequestAutoMerge(input: { + expectedHeadOid: $sha, + pullRequestId: $node_id + }) + { clientMutationId } + }`, + { node_id: pull_request.node_id, sha: latestSha }, + ) + return 'Enabled Auto Merge' + } catch (e) { + log('Auto Merge failed', e.response.errors[0].message) + } + + // Auto-merge doesn't work if the target branch has already run all CI, in which + // case the PR must be enqueued explicitly. + // We now have merge queues enabled on all development branches, thus don't need a + // fallback after this. + try { + const resp = await github.graphql( + `mutation($node_id: ID!, $sha: GitObjectID) { + enqueuePullRequest(input: { + expectedHeadOid: $sha, + pullRequestId: $node_id + }) + { + clientMutationId, + mergeQueueEntry { mergeQueue { url } } + } + }`, + { node_id: pull_request.node_id, sha: latestSha }, + ) + return `[Queued](${resp.enqueuePullRequest.mergeQueueEntry.mergeQueue.url}) for merge` + } catch (e) { + log('Enqueing failed', e.response.errors[0].message) + throw new Error(e.response.errors[0].message) + } + } + + for (const comment of comments) { + log('comment', comment.node_id) + + async function react(reaction) { + if (dry) { + core.info(`Reaction ${reaction} on ${comment.node_id} (dry)`) + return + } + + await handleMergeComment({ + github, + body: comment.body, + node_id: comment.node_id, + reaction, + }) + } + + async function isMaintainer(username) { + try { + return ( + ( + await github.rest.teams.getMembershipForUserInOrg({ + org: context.repo.owner, + team_slug: 'nixpkgs-maintainers', + username, + }) + ).data.state === 'active' + ) + } catch (e) { + if (e.status === 404) return false + else throw e + } + } + + const canUseMergeBot = await isMaintainer(comment.user.login) + const isEligible = eligible.has(comment.user.id) + const canMerge = result && canUseMergeBot && isEligible + + const body = [ + ``, + '', + 'Requirements to merge this PR:', + ...Object.entries(checklist).map( + ([msg, res]) => `- :${res ? 'white_check_mark' : 'x'}: ${msg}`, + ), + `- :${canUseMergeBot ? 'white_check_mark' : 'x'}: ${comment.user.login} can use the merge bot.`, + `- :${isEligible ? 'white_check_mark' : 'x'}: ${comment.user.login} is eligible to merge changes to the touched packages.`, + '', + ] + + if (canMerge) { + await react('ROCKET') + try { + body.push(`:heavy_check_mark: ${await merge()} (#306934)`) + } catch (e) { + // Remove the HTML comment with node_id reference to allow retrying this merge on the next run. + body.shift() + body.push(`:x: Merge failed with: ${e} (#371492)`) + } + } else { + await react('THUMBS_DOWN') + body.push(':x: Pull Request could not be merged (#305350)') + } + + if (dry) { + core.info(body.join('\n')) + } else { + await github.rest.issues.createComment({ + ...context.repo, + issue_number: pull_number, + body: body.join('\n'), + }) + } + + if (canMerge) break + } +} + +module.exports = { + handleMerge, + handleMergeComment, +} diff --git a/ci/github-script/run b/ci/github-script/run index 782e3fa7db1f..8a2906606cac 100755 --- a/ci/github-script/run +++ b/ci/github-script/run @@ -65,19 +65,19 @@ program }) program - .command('labels') - .description('Manage labels on pull requests.') + .command('bot') + .description('Run automation on pull requests and issues.') .argument('', 'Owner of the GitHub repository to label (Example: NixOS)') .argument('', 'Name of the GitHub repository to label (Example: nixpkgs)') .argument('[pr]', 'Number of the Pull Request to label') .option('--no-dry', 'Make actual modifications') .action(async (owner, repo, pr, options) => { - const labels = (await import('./labels.js')).default + const bot = (await import('./bot.js')).default const tmp = mkdtempSync(join(tmpdir(), 'github-script-')) try { process.env.GITHUB_WORKSPACE = tmp process.chdir(tmp) - await run(labels, owner, repo, pr, options) + await run(bot, owner, repo, pr, options) } finally { rmSync(tmp, { recursive: true }) }