Revert "ci/github-script/reviewers: revoke stale review requests"

This reverts commit 2d34258228.
This commit is contained in:
Michael Daniels
2026-05-18 17:26:09 -04:00
parent acb7b7a38c
commit 843634b667
2 changed files with 28 additions and 102 deletions

View File

@@ -483,7 +483,6 @@ module.exports = async ({ github, context, core, dry }) => {
dry,
pull_request,
reviews,
events,
// TODO: Use maintainer map instead of the artifact.
user_maintainers: Object.keys(
JSON.parse(

View File

@@ -6,7 +6,6 @@ async function handleReviewers({
dry,
pull_request,
reviews,
events,
user_maintainers,
team_maintainers,
owners,
@@ -15,28 +14,20 @@ async function handleReviewers({
}) {
const pull_number = pull_request.number
// Users currently requested for review (pending).
const pending_users = new Set(
pull_request.requested_reviewers.map(({ login }) => login.toLowerCase()),
)
// Users who actually submitted a review in this PR (any state, including DISMISSED).
const users_engaged = new Set(
reviews.map(({ user }) => user.login.toLowerCase()),
)
// Users the PR has already reached: pending OR engaged.
const users_reached = pending_users.union(users_engaged)
// Users that the PR has already reached, e.g. they've left a review or have been requested for one
const users_reached = new Set([
...pull_request.requested_reviewers.map(({ login }) => login.toLowerCase()),
...reviews.map(({ user }) => user.login.toLowerCase()),
])
log('reviewers - users_reached', Array.from(users_reached).join(', '))
// Same for teams. A team is engaged only via `onBehalfOf` reviews.
const pending_teams = new Set(
pull_request.requested_teams.map(({ slug }) => slug.toLowerCase()),
)
const teams_engaged = new Set(
reviews.flatMap(({ onBehalfOf }) =>
// Same for teams
const teams_reached = new Set([
...pull_request.requested_teams.map(({ slug }) => slug.toLowerCase()),
...reviews.flatMap(({ onBehalfOf }) =>
onBehalfOf.nodes.map(({ slug }) => slug.toLowerCase()),
),
)
const teams_reached = pending_teams.union(teams_engaged)
])
log('reviewers - teams_reached', Array.from(teams_reached).join(', '))
// Early sanity check, before we start making any API requests. The list of maintainers
@@ -158,101 +149,37 @@ async function handleReviewers({
)
log('reviewers - teams_not_yet_reached', teams_not_yet_reached.join(', '))
// The usernames of bots that make review requests we may auto-revoke.
const revokable_requesters = ['github-actions[bot]', 'nixpkgs-ci[bot]']
// Find latest `review_requested` actor per reviewer / team.
const last_request_actor_for_user = new Map()
const last_request_actor_for_team = new Map()
for (const ev of events) {
if (ev.event !== 'review_requested') continue
if (ev.requested_reviewer?.login) {
last_request_actor_for_user.set(
ev.requested_reviewer.login.toLowerCase(),
ev.actor?.login ?? '',
)
}
if (ev.requested_team?.slug) {
last_request_actor_for_team.set(
ev.requested_team.slug.toLowerCase(),
ev.actor?.login ?? '',
)
}
}
// Pending requests no longer in the to_reach set, excluding the engaged
// and anything not requested by our own bot.
const users_to_remove = Array.from(
pending_users.difference(users_to_reach).difference(users_engaged),
).filter((login) =>
revokable_requesters.includes(last_request_actor_for_user.get(login)),
)
log('reviewers - users_to_remove', users_to_remove.join(', '))
// Same for teams.
const teams_to_remove = Array.from(
pending_teams.difference(teams_to_reach).difference(teams_engaged),
).filter((slug) =>
revokable_requesters.includes(last_request_actor_for_team.get(slug)),
)
log('reviewers - teams_to_remove', teams_to_remove.join(', '))
const has_adds =
users_not_yet_reached.length > 0 || teams_not_yet_reached.length > 0
const has_removals = users_to_remove.length > 0 || teams_to_remove.length > 0
if (!has_adds && !has_removals) {
if (
users_not_yet_reached.length === 0 &&
teams_not_yet_reached.length === 0
) {
log('Has reviewer changes', 'false (skipped)')
} else if (dry) {
if (has_adds) {
core.info(
`Requesting user reviewers for #${pull_number}: ${users_not_yet_reached.join(', ')} (dry)`,
)
core.info(
`Requesting team reviewers for #${pull_number}: ${teams_not_yet_reached.join(', ')} (dry)`,
)
}
if (has_removals) {
core.info(
`Revoking stale reviewers for #${pull_number}: users=[${users_to_remove.join(', ')}], teams=[${teams_to_remove.join(', ')}] (dry)`,
)
}
core.info(
`Requesting user reviewers for #${pull_number}: ${users_not_yet_reached.join(', ')} (dry)`,
)
core.info(
`Requesting team reviewers for #${pull_number}: ${teams_not_yet_reached.join(', ')} (dry)`,
)
} else {
// We had tried the "request all reviewers at once" thing in the past, but it didn't work out:
// https://github.com/NixOS/nixpkgs/commit/034613f860fcd339bd2c20c8f6bc259a2f9dc034
// If we're hitting API errors here again, we'll need to investigate - and possibly reverse
// course.
// Add and remove sets are disjoint by construction. Parallel is safe.
await Promise.all(
[
has_adds &&
github.rest.pulls.requestReviewers({
...context.repo,
pull_number,
reviewers: users_not_yet_reached,
team_reviewers: teams_not_yet_reached,
}),
has_removals &&
github.rest.pulls.removeRequestedReviewers({
...context.repo,
pull_number,
reviewers: users_to_remove,
team_reviewers: teams_to_remove,
}),
].filter(Boolean),
)
await github.rest.pulls.requestReviewers({
...context.repo,
pull_number,
reviewers: users_not_yet_reached,
team_reviewers: teams_not_yet_reached,
})
}
// Subtract the just-revoked so revoking the last pending reviewer flips the label.
const users_still_reached = users_reached.difference(new Set(users_to_remove))
const teams_still_reached = teams_reached.difference(new Set(teams_to_remove))
// Return a boolean on whether the "needs: reviewers" label should be set.
return (
users_not_yet_reached.length === 0 &&
teams_not_yet_reached.length === 0 &&
users_still_reached.size === 0 &&
teams_still_reached.size === 0
users_reached.size === 0 &&
teams_reached.size === 0
)
}