mirror of
https://github.com/NixOS/nixpkgs.git
synced 2026-06-29 13:35:44 +00:00
`bot.js` already pulls reviews via GraphQL for the approval-count
labels. Move that fetch above the `handleMerge` call, add `commit
{ oid }` to the query, and pass the result through. `handleMerge` no
longer issues its own `listReviews` REST call.
While here, the `approvals` set is now derived from the same reviews
data. Two upsides:
- A reviewer who approved and was later dismissed no longer counts
towards the approval check; their review now surfaces with state
DISMISSED instead of leaving a stale `reviewed` event behind.
- `events` is no longer needed by `runChecklist` (it was only feeding
the approvals filter); the parameter is dropped from both call
sites. `handleMerge` still uses `events` for tracking the latest
push and merge-command comments.
Also drop the redundant `user` truthy checks (and the stale "some
users have been deleted" comment) in `runChecklist`. The GraphQL
query uses `author { ... on User { login id } }` and bot.js then
filters via `r.user?.login`, so by the time reviews reach
`runChecklist` every entry already has a populated `user`. Verified
by querying real PRs: bots surface as `{__typename: "Bot"}` (no
`login`/`id`, filtered out by bot.js), and deleted accounts surface
as the "ghost" user (login `"ghost"`, id `10137`), which passes the
filter but matches no committer - harmless for both checks.
Assisted-by: claude-code with claude-opus-4-8[1m]-high
416 lines
14 KiB
JavaScript
416 lines
14 KiB
JavaScript
const { classify } = require('../supportedBranches.js')
|
|
|
|
function runChecklist({
|
|
committers,
|
|
files,
|
|
pull_request,
|
|
log,
|
|
maintainers,
|
|
reviews,
|
|
user,
|
|
userIsMaintainer,
|
|
}) {
|
|
const allByName = files.every(
|
|
({ filename }) =>
|
|
filename.startsWith('pkgs/by-name/') && filename.split('/').length > 4,
|
|
)
|
|
|
|
const packages = files
|
|
.filter(({ filename }) => filename.startsWith('pkgs/by-name/'))
|
|
.map(({ filename }) => filename.split('/')[3])
|
|
.filter(Boolean)
|
|
|
|
const eligible = !packages.length
|
|
? new Set()
|
|
: packages
|
|
.map((pkg) => new Set(maintainers[pkg]))
|
|
.reduce((acc, cur) => acc?.intersection(cur) ?? cur)
|
|
|
|
const approvals = new Set(
|
|
reviews
|
|
.filter(
|
|
({ state, commit }) =>
|
|
state === 'APPROVED' &&
|
|
// Only approvals for the current head SHA count, otherwise authors could push
|
|
// bad code between the approval and the merge.
|
|
commit?.oid === pull_request.head.sha,
|
|
)
|
|
.map(({ user }) => user.id),
|
|
)
|
|
|
|
// A "changes requested" review from a committer blocks both the merge queue and
|
|
// auto-merge, even if it was made on an older commit (unlike approvals, GitHub does
|
|
// not auto-dismiss changes-requested reviews on push). For each committer, take their
|
|
// latest actionable review; if it's CHANGES_REQUESTED, they're blocking the PR.
|
|
// Dismissed reviews surface as DISMISSED and comment-only follow-ups as COMMENTED, so
|
|
// both are skipped naturally — the prior actionable review still stands until the
|
|
// committer explicitly approves or requests changes again.
|
|
const committerReviewState = new Map()
|
|
for (const { user, state } of reviews) {
|
|
if (
|
|
committers.has(user.id) &&
|
|
['APPROVED', 'CHANGES_REQUESTED'].includes(state)
|
|
) {
|
|
committerReviewState.set(user.id, state)
|
|
}
|
|
}
|
|
const noBlockingReviews = !Array.from(committerReviewState.values()).includes(
|
|
'CHANGES_REQUESTED',
|
|
)
|
|
|
|
const checklist = {
|
|
'PR targets a [development branch](https://github.com/NixOS/nixpkgs/blob/-/ci/README.md#branch-classification).':
|
|
classify(pull_request.base.ref).type.includes('development'),
|
|
'PR touches only files of packages in `pkgs/by-name/`.': allByName,
|
|
'PR is at least one of:': {
|
|
'Approved by a [committer](https://github.com/orgs/NixOS/teams/nixpkgs-committers).':
|
|
committers.intersection(approvals).size > 0,
|
|
'Backported via label.':
|
|
pull_request.user.login === 'nixpkgs-ci[bot]' &&
|
|
pull_request.head.ref.startsWith('backport-'),
|
|
'Opened by a [committer](https://github.com/orgs/NixOS/teams/nixpkgs-committers).':
|
|
committers.has(pull_request.user.id),
|
|
'Opened by [@r-ryantm](https://nix-community.github.io/nixpkgs-update/r-ryantm/).':
|
|
pull_request.user.login === 'r-ryantm',
|
|
},
|
|
'PR is not a draft': !pull_request.draft,
|
|
// CI state is intentionally *not* a checklist item: auto-merge exists precisely to
|
|
// cover unfinished CI, and an already-failed CI is reported via the merge message
|
|
// (see merge() below) rather than a blanket refusal.
|
|
'PR is not blocked by a "changes requested" review from a [committer](https://github.com/orgs/NixOS/teams/nixpkgs-committers).':
|
|
noBlockingReviews,
|
|
}
|
|
|
|
if (user) {
|
|
checklist[
|
|
`${user.login} is a member of [@NixOS/nixpkgs-maintainers](https://github.com/orgs/NixOS/teams/nixpkgs-maintainers).`
|
|
] = userIsMaintainer
|
|
if (allByName) {
|
|
// We can only determine the below, if all packages are in by-name, since
|
|
// we can't reliably relate changed files to packages outside by-name.
|
|
checklist[
|
|
`${user.login} is a maintainer of all touched packages on the ${pull_request.base.ref} branch.`
|
|
] = eligible.has(user.id)
|
|
}
|
|
} else {
|
|
// This is only used when no user is passed, i.e. for labeling.
|
|
checklist['PR has maintainers eligible to merge.'] = eligible.size > 0
|
|
}
|
|
|
|
const result = Object.values(checklist).every((v) =>
|
|
typeof v === 'boolean' ? v : Object.values(v).some(Boolean),
|
|
)
|
|
|
|
log('checklist', JSON.stringify(checklist))
|
|
log('eligible', JSON.stringify(Array.from(eligible)))
|
|
log('result', result)
|
|
|
|
return {
|
|
checklist,
|
|
eligible,
|
|
result,
|
|
}
|
|
}
|
|
|
|
// 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*$/im)
|
|
}
|
|
|
|
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,
|
|
reviews,
|
|
maintainers,
|
|
getTeamMembers,
|
|
getUser,
|
|
}) {
|
|
const pull_number = pull_request.number
|
|
|
|
const committers = new Set(
|
|
(await getTeamMembers('nixpkgs-committers')).map(({ id }) => id),
|
|
)
|
|
|
|
const files = (
|
|
await github.rest.pulls.listFiles({
|
|
...context.repo,
|
|
pull_number,
|
|
per_page: 100,
|
|
})
|
|
).data
|
|
|
|
// Early exit to prevent treewides from using up a lot of API requests (and time!) to list
|
|
// all the files in the pull request. For now, the merge-bot will not work when 100 or more
|
|
// files are touched in a PR - which should be more than fine.
|
|
// TODO: Find a more efficient way of downloading all the *names* of the touched files,
|
|
// including an early exit when the first non-by-name file is found.
|
|
if (files.length >= 100) return false
|
|
|
|
const noPrFailuresState = (
|
|
await github.rest.repos.listCommitStatusesForRef({
|
|
...context.repo,
|
|
ref: pull_request.head.sha,
|
|
per_page: 100,
|
|
})
|
|
).data.find(({ context }) => context === 'no PR failures')?.state
|
|
|
|
// Only look through comments *after* the latest (force) push.
|
|
const lastPush = events.findLastIndex(
|
|
({ event, sha, commit_id }) =>
|
|
['committed', 'head_ref_force_pushed'].includes(event) &&
|
|
(sha ?? commit_id) === pull_request.head.sha,
|
|
)
|
|
|
|
const comments = events.slice(lastPush + 1).filter(
|
|
({ event, body, user, node_id }) =>
|
|
['commented', 'reviewed'].includes(event) &&
|
|
hasMergeCommand(body) &&
|
|
// Ignore comments where the user has been deleted already.
|
|
user &&
|
|
// Ignore comments which had already been responded to by the bot.
|
|
(dry ||
|
|
!events.some(
|
|
({ event, 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(`^<!-- comment: ${node_id} -->$`, 'm')),
|
|
)),
|
|
)
|
|
|
|
// Returns `{ reaction, messages }`: the reaction to leave on the merge comment and the
|
|
// lines to append to the bot's reply. Throws only on an unexpected API error.
|
|
async function merge() {
|
|
if (dry) {
|
|
core.info(`Merging #${pull_number}... (dry)`)
|
|
return { reaction: 'ROCKET', messages: ['Merge completed (dry)'] }
|
|
}
|
|
|
|
// Using GraphQL mutations instead of the REST /merge endpoint, because the latter
|
|
// doesn't work with Merge Queues. We now have merge queues enabled on all development
|
|
// branches, so we don't need a fallback for regular merges.
|
|
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: pull_request.head.sha },
|
|
)
|
|
log('merge', 'Queued for merge')
|
|
return {
|
|
reaction: 'ROCKET',
|
|
messages: [
|
|
`:heavy_check_mark: [Queued](${resp.enqueuePullRequest.mergeQueueEntry.mergeQueue.url}) for merge (#306934)`,
|
|
],
|
|
}
|
|
} catch (e) {
|
|
log('Enqueuing failed', e.response.errors[0].message)
|
|
}
|
|
|
|
// Enqueuing fails when the required status checks are not satisfied, yet. If CI has
|
|
// already failed, enabling auto-merge would be pointless: it would never fire, and
|
|
// fixing CI requires a new push, which invalidates this merge command anyway (we only
|
|
// act on comments after the latest push). So we don't enable auto-merge and instead
|
|
// ask for a fresh command once CI is green again.
|
|
if (['error', 'failure'].includes(noPrFailuresState)) {
|
|
log('merge', 'CI has failed, not enabling auto-merge')
|
|
return {
|
|
reaction: 'THUMBS_DOWN',
|
|
messages: [
|
|
':x: Pull Request could not be merged: CI has failed (#305350).',
|
|
'',
|
|
'> [!TIP]',
|
|
'> PRs cannot be merged while CI is failing.',
|
|
'> Once CI is passing, comment `@NixOS/nixpkgs-merge-bot merge` again.',
|
|
],
|
|
}
|
|
}
|
|
|
|
// CI has not finished yet, so we enable auto-merge. We could also only use auto-merge,
|
|
// but this often gets stuck for no apparent reason.
|
|
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: pull_request.head.sha },
|
|
)
|
|
log('merge', 'Auto-merge enabled')
|
|
return {
|
|
reaction: 'ROCKET',
|
|
messages: [
|
|
`:heavy_check_mark: Enabled Auto Merge (#306934)`,
|
|
'',
|
|
'> [!TIP]',
|
|
'> [Auto Merge](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request) will queue this PR once required CI checks succeed.',
|
|
'> If CI fails instead, fixing it needs a new push, which disables Auto Merge and invalidates this command — comment `@NixOS/nixpkgs-merge-bot merge` again once CI is green.',
|
|
'> If GitHub gets stuck even though CI passed (it sometimes does), leaving another approval should kick off the merge.',
|
|
],
|
|
}
|
|
} catch (e) {
|
|
log('Auto Merge 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 { result, eligible, checklist } = runChecklist({
|
|
committers,
|
|
files,
|
|
pull_request,
|
|
log,
|
|
maintainers,
|
|
reviews,
|
|
user: comment.user,
|
|
userIsMaintainer: await isMaintainer(comment.user.login),
|
|
})
|
|
|
|
const body = [
|
|
`<!-- comment: ${comment.node_id} -->`,
|
|
`@${comment.user.login} wants to merge this PR.`,
|
|
'',
|
|
'Requirements to merge this PR with `@NixOS/nixpkgs-merge-bot merge`:',
|
|
...Object.entries(checklist).flatMap(([msg, res]) =>
|
|
typeof res === 'boolean'
|
|
? `- :${res ? 'white_check_mark' : 'x'}: ${msg}`
|
|
: [
|
|
`- :${Object.values(res).some(Boolean) ? 'white_check_mark' : 'x'}: ${msg}`,
|
|
...Object.entries(res).map(
|
|
([msg, res]) =>
|
|
` - ${res ? ':white_check_mark:' : ':white_large_square:'} ${msg}`,
|
|
),
|
|
],
|
|
),
|
|
'',
|
|
]
|
|
|
|
if (eligible.size > 0 && !eligible.has(comment.user.id)) {
|
|
const users = await Promise.all(
|
|
Array.from(eligible, async (id) => (await getUser(id)).login),
|
|
)
|
|
body.push(
|
|
'> [!TIP]',
|
|
'> Maintainers eligible to merge are:',
|
|
...users.map((login) => `> - ${login}`),
|
|
'',
|
|
)
|
|
}
|
|
|
|
if (result) {
|
|
try {
|
|
const { reaction, messages } = await merge()
|
|
await react(reaction)
|
|
body.push(...messages)
|
|
} catch (e) {
|
|
await react('THUMBS_DOWN')
|
|
// 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 (result) break
|
|
}
|
|
|
|
const { result } = runChecklist({
|
|
committers,
|
|
files,
|
|
pull_request,
|
|
log,
|
|
maintainers,
|
|
reviews,
|
|
})
|
|
|
|
// Returns a boolean, which indicates whether the PR is merge-bot eligible in principle.
|
|
// This is used to set the respective label in bot.js.
|
|
return result
|
|
}
|
|
|
|
module.exports = {
|
|
handleMerge,
|
|
handleMergeComment,
|
|
}
|