diff --git a/.github/workflows/bot.yml b/.github/workflows/bot.yml index 1fbef1134f14..cb1df7bdb0a1 100644 --- a/.github/workflows/bot.yml +++ b/.github/workflows/bot.yml @@ -39,6 +39,10 @@ jobs: 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 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/review.yml b/.github/workflows/review.yml index 6679f6852766..44ac3f0088d4 100644 --- a/.github/workflows/review.yml +++ b/.github/workflows/review.yml @@ -6,6 +6,8 @@ on: - 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 @@ -14,18 +16,32 @@ defaults: 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 + 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 @@ -44,19 +60,33 @@ jobs: 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 } - )) + })).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/reviewed.yml b/.github/workflows/reviewed.yml index ab959d8c2117..d7d368739eaa 100644 --- a/.github/workflows/reviewed.yml +++ b/.github/workflows/reviewed.yml @@ -2,7 +2,7 @@ name: Reviewed on: pull_request_review: - types: [dismissed] + types: [submitted, dismissed] permissions: {} diff --git a/ci/github-script/bot.js b/ci/github-script/bot.js index 807b134843dc..e0a67a747c82 100644 --- a/ci/github-script/bot.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: diff --git a/ci/github-script/merge.js b/ci/github-script/merge.js new file mode 100644 index 000000000000..b08e2b4e2fdc --- /dev/null +++ b/ci/github-script/merge.js @@ -0,0 +1,257 @@ +// 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) { + 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, +}