mirror of
https://github.com/NixOS/nixpkgs.git
synced 2026-07-06 17:13:24 -05:00
ci/github-script/merge: skip auto-merge when CI has already failed
The merge bot falls back to GitHub Auto Merge whenever the merge queue
won't accept a PR yet. That is the right move while CI is still
running, but pointless once CI has already failed: Auto Merge can never
trigger, and fixing CI requires a new push, which invalidates the merge
command anyway (the bot only acts on comments after the latest push).
Fetch the `no PR failures` commit status and, when the merge-queue
enqueue fails, branch on it. If CI has already failed
(`error`/`failure`), skip Auto Merge and reply that a fresh
`@NixOS/nixpkgs-merge-bot merge` comment is needed once CI is green
again. Otherwise (pending or missing status) enable Auto Merge as
before.
`merge()` now returns `{ reaction, messages }` so the CI-failure path
can leave a thumbs-down reaction rather than the rocket used for an
actual merge.
Closes #512554.
Assisted-by: claude-code with claude-opus-4-8[1m]-high
This commit is contained in:
parent
90d704fbd0
commit
da97bf8423
2 changed files with 63 additions and 15 deletions
|
|
@ -52,6 +52,14 @@ To ensure security and a focused utility, the bot adheres to specific limitation
|
|||
- The user attempting to merge is a member of [@NixOS/nixpkgs-maintainers].
|
||||
- The user attempting to merge is a maintainer of all packages touched by the PR.
|
||||
|
||||
Once these constraints are met, the bot picks a merge strategy based on the `no PR failures` commit status:
|
||||
|
||||
- CI passing: the PR is added to the merge queue.
|
||||
- CI unfinished (pending or missing status): the bot enables [Auto Merge], which queues the PR once required checks succeed.
|
||||
Note that if CI later fails, nothing happens until it is fixed and passes.
|
||||
- CI already failing (`error`/`failure` status): the bot does not enable Auto Merge, because it would never trigger, and fixing CI requires a new push that invalidates the merge command.
|
||||
A fresh `@NixOS/nixpkgs-merge-bot merge` comment is needed once CI is green again.
|
||||
|
||||
### Approving merge bot changes
|
||||
|
||||
Changes to the bot can usually be approved by the [@NixOS/nixpkgs-ci] team, as with other CI changes.
|
||||
|
|
@ -104,3 +112,4 @@ This script can also be run locally to print basic test cases.
|
|||
[@NixOS/nixpkgs-ci]: https://github.com/orgs/NixOS/teams/nixpkgs-ci
|
||||
[@NixOS/nixpkgs-core]: https://github.com/orgs/NixOS/teams/nixpkgs-core
|
||||
[RFC 172]: https://github.com/NixOS/rfcs/pull/172
|
||||
[Auto Merge]: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request
|
||||
|
|
|
|||
|
|
@ -57,6 +57,9 @@ function runChecklist({
|
|||
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.
|
||||
}
|
||||
|
||||
if (user) {
|
||||
|
|
@ -148,6 +151,14 @@ async function handleMerge({
|
|||
// 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 }) =>
|
||||
|
|
@ -173,10 +184,12 @@ async function handleMerge({
|
|||
)),
|
||||
)
|
||||
|
||||
// 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 ['Merge completed (dry)']
|
||||
return { reaction: 'ROCKET', messages: ['Merge completed (dry)'] }
|
||||
}
|
||||
|
||||
// Using GraphQL mutations instead of the REST /merge endpoint, because the latter
|
||||
|
|
@ -197,16 +210,37 @@ async function handleMerge({
|
|||
{ node_id: pull_request.node_id, sha: pull_request.head.sha },
|
||||
)
|
||||
log('merge', 'Queued for merge')
|
||||
return [
|
||||
`:heavy_check_mark: [Queued](${resp.enqueuePullRequest.mergeQueueEntry.mergeQueue.url}) for merge (#306934)`,
|
||||
]
|
||||
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)
|
||||
}
|
||||
|
||||
// If required status checks are not satisfied, yet, the above will fail. In this case
|
||||
// we can enable auto-merge. We could also only use auto-merge, but this often gets
|
||||
// stuck for no apparent reason.
|
||||
// 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) {
|
||||
|
|
@ -219,12 +253,15 @@ async function handleMerge({
|
|||
{ node_id: pull_request.node_id, sha: pull_request.head.sha },
|
||||
)
|
||||
log('merge', 'Auto-merge enabled')
|
||||
return [
|
||||
`:heavy_check_mark: Enabled Auto Merge (#306934)`,
|
||||
'',
|
||||
'> [!TIP]',
|
||||
'> Sometimes GitHub gets stuck after enabling Auto Merge. In this case, leaving another approval should trigger the merge.',
|
||||
]
|
||||
return {
|
||||
reaction: 'ROCKET',
|
||||
messages: [
|
||||
`:heavy_check_mark: Enabled Auto Merge (#306934)`,
|
||||
'',
|
||||
'> [!TIP]',
|
||||
'> Sometimes GitHub gets stuck after enabling Auto Merge. In this case, leaving another approval should trigger the merge.',
|
||||
],
|
||||
}
|
||||
} catch (e) {
|
||||
log('Auto Merge failed', e.response.errors[0].message)
|
||||
throw new Error(e.response.errors[0].message)
|
||||
|
|
@ -308,10 +345,12 @@ async function handleMerge({
|
|||
}
|
||||
|
||||
if (result) {
|
||||
await react('ROCKET')
|
||||
try {
|
||||
body.push(...(await merge()))
|
||||
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)`)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue