From 042a2fd6d6834366b142c74e518fd2674de69725 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 22 Jun 2025 14:50:28 +0200 Subject: [PATCH 1/8] workflows/labels: refactor into handle() function Separate commit for better diff. --- .github/workflows/labels.yml | 254 ++++++++++++++++++----------------- 1 file changed, 128 insertions(+), 126 deletions(-) diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 83cf0511fc5e..aacaa311a0c9 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -165,6 +165,133 @@ jobs: base: context.payload.pull_request.base.ref } + async function handle(pull_request, done) { + try { + const log = (k,v,skip) => { + core.info(`PR #${pull_request.number} - ${k}: ${v}` + (skip ? ' (skipped)' : '')) + return skip + } + + if (log('Last updated at', pull_request.updated_at, new Date(pull_request.updated_at) < cutoff)) + return done() + stats.prs++ + log('URL', pull_request.html_url) + + const run_id = (await github.rest.actions.listWorkflowRuns({ + ...context.repo, + workflow_id: 'pr.yml', + event: 'pull_request_target', + // For PR events, the workflow run is still in progress with this job itself. + status: prEventCondition ? 'in_progress' : 'success', + exclude_pull_requests: true, + head_sha: pull_request.head.sha + })).data.workflow_runs[0]?.id ?? + // TODO: Remove this after 2025-09-17, at which point all eval.yml artifacts will have expired. + (await github.rest.actions.listWorkflowRuns({ + ...context.repo, + // In older PRs, we need eval.yml instead of pr.yml. + workflow_id: 'eval.yml', + event: 'pull_request_target', + status: 'success', + exclude_pull_requests: true, + head_sha: pull_request.head.sha + })).data.workflow_runs[0]?.id + + // Newer PRs might not have run Eval to completion, yet. We can skip them, because this + // job will be run as part of that Eval run anyway. + if (log('Last eval run', run_id ?? '', !run_id)) + return; + + const artifact = (await github.rest.actions.listWorkflowRunArtifacts({ + ...context.repo, + run_id, + name: 'comparison' + })).data.artifacts[0] + + // Instead of checking the boolean artifact.expired, we will give us a minute to + // actually download the artifact in the next step and avoid that race condition. + // Older PRs, where the workflow run was already eval.yml, but the artifact was not + // called "comparison", yet, will be skipped as well. + const expired = new Date(artifact?.expires_at ?? 0) < new Date(new Date().getTime() + 60 * 1000) + if (log('Artifact expires at', artifact?.expires_at ?? '', expired)) + return; + stats.artifacts++ + + await artifactClient.downloadArtifact(artifact.id, { + findBy: { + repositoryName: context.repo.repo, + repositoryOwner: context.repo.owner, + token: core.getInput('github-token') + }, + path: path.resolve(pull_request.number.toString()), + expectedHash: artifact.digest + }) + + // Create a map (Label -> Boolean) of all currently set labels. + // Each label is set to True and can be disabled later. + const before = Object.fromEntries( + (await github.paginate(github.rest.issues.listLabelsOnIssue, { + ...context.repo, + issue_number: pull_request.number + })) + .map(({ name }) => [name, true]) + ) + + const approvals = new Set( + (await github.paginate(github.rest.pulls.listReviews, { + ...context.repo, + pull_number: pull_request.number + })) + .filter(review => review.state == 'APPROVED') + .map(review => review.user?.id) + ) + + const maintainers = new Set(Object.keys( + JSON.parse(await readFile(`${pull_request.number}/maintainers.json`, 'utf-8')) + ).map(m => Number.parseInt(m, 10))) + + const evalLabels = JSON.parse(await readFile(`${pull_request.number}/changed-paths.json`, 'utf-8')).labels + + // Manage the labels + const after = Object.assign( + {}, + before, + // Ignore `evalLabels` if it's an array. + // This can happen for older eval runs, before we switched to objects. + // The old eval labels would have been set by the eval run, + // so now they'll be present in `before`. + // TODO: Simplify once old eval results have expired (~2025-10) + (Array.isArray(evalLabels) ? undefined : evalLabels), + { + '12.approvals: 1': approvals.size == 1, + '12.approvals: 2': approvals.size == 2, + '12.approvals: 3+': approvals.size >= 3, + '12.approved-by: package-maintainer': Array.from(maintainers).some(m => approvals.has(m)), + '12.first-time contribution': + [ 'NONE', 'FIRST_TIMER', 'FIRST_TIME_CONTRIBUTOR' ].includes(pull_request.author_association), + } + ) + + // No need for an API request, if all labels are the same. + const hasChanges = Object.keys(after).some(name => (before[name] ?? false) != after[name]) + if (log('Has changes', hasChanges, !hasChanges)) + return; + + // Skipping labeling on a pull_request event, because we have no privileges. + const labels = Object.entries(after).filter(([,value]) => value).map(([name]) => name) + if (log('Set labels', labels, context.eventName == 'pull_request')) + return; + + await github.rest.issues.setLabels({ + ...context.repo, + issue_number: pull_request.number, + labels + }) + } catch (cause) { + throw new Error(`Labeling PR #${pull_request.number} failed.`, { cause }) + } + } + const prs = await github.paginate( github.rest.pulls.list, { @@ -174,132 +301,7 @@ jobs: direction: 'desc', ...prEventCondition }, - (response, done) => response.data.map(async (pull_request) => { - try { - const log = (k,v,skip) => { - core.info(`PR #${pull_request.number} - ${k}: ${v}` + (skip ? ' (skipped)' : '')) - return skip - } - - if (log('Last updated at', pull_request.updated_at, new Date(pull_request.updated_at) < cutoff)) - return done() - stats.prs++ - log('URL', pull_request.html_url) - - const run_id = (await github.rest.actions.listWorkflowRuns({ - ...context.repo, - workflow_id: 'pr.yml', - event: 'pull_request_target', - // For PR events, the workflow run is still in progress with this job itself. - status: prEventCondition ? 'in_progress' : 'success', - exclude_pull_requests: true, - head_sha: pull_request.head.sha - })).data.workflow_runs[0]?.id ?? - // TODO: Remove this after 2025-09-17, at which point all eval.yml artifacts will have expired. - (await github.rest.actions.listWorkflowRuns({ - ...context.repo, - // In older PRs, we need eval.yml instead of pr.yml. - workflow_id: 'eval.yml', - event: 'pull_request_target', - status: 'success', - exclude_pull_requests: true, - head_sha: pull_request.head.sha - })).data.workflow_runs[0]?.id - - // Newer PRs might not have run Eval to completion, yet. We can skip them, because this - // job will be run as part of that Eval run anyway. - if (log('Last eval run', run_id ?? '', !run_id)) - return; - - const artifact = (await github.rest.actions.listWorkflowRunArtifacts({ - ...context.repo, - run_id, - name: 'comparison' - })).data.artifacts[0] - - // Instead of checking the boolean artifact.expired, we will give us a minute to - // actually download the artifact in the next step and avoid that race condition. - // Older PRs, where the workflow run was already eval.yml, but the artifact was not - // called "comparison", yet, will be skipped as well. - const expired = new Date(artifact?.expires_at ?? 0) < new Date(new Date().getTime() + 60 * 1000) - if (log('Artifact expires at', artifact?.expires_at ?? '', expired)) - return; - stats.artifacts++ - - await artifactClient.downloadArtifact(artifact.id, { - findBy: { - repositoryName: context.repo.repo, - repositoryOwner: context.repo.owner, - token: core.getInput('github-token') - }, - path: path.resolve(pull_request.number.toString()), - expectedHash: artifact.digest - }) - - // Create a map (Label -> Boolean) of all currently set labels. - // Each label is set to True and can be disabled later. - const before = Object.fromEntries( - (await github.paginate(github.rest.issues.listLabelsOnIssue, { - ...context.repo, - issue_number: pull_request.number - })) - .map(({ name }) => [name, true]) - ) - - const approvals = new Set( - (await github.paginate(github.rest.pulls.listReviews, { - ...context.repo, - pull_number: pull_request.number - })) - .filter(review => review.state == 'APPROVED') - .map(review => review.user?.id) - ) - - const maintainers = new Set(Object.keys( - JSON.parse(await readFile(`${pull_request.number}/maintainers.json`, 'utf-8')) - ).map(m => Number.parseInt(m, 10))) - - const evalLabels = JSON.parse(await readFile(`${pull_request.number}/changed-paths.json`, 'utf-8')).labels - - // Manage the labels - const after = Object.assign( - {}, - before, - // Ignore `evalLabels` if it's an array. - // This can happen for older eval runs, before we switched to objects. - // The old eval labels would have been set by the eval run, - // so now they'll be present in `before`. - // TODO: Simplify once old eval results have expired (~2025-10) - (Array.isArray(evalLabels) ? undefined : evalLabels), - { - '12.approvals: 1': approvals.size == 1, - '12.approvals: 2': approvals.size == 2, - '12.approvals: 3+': approvals.size >= 3, - '12.approved-by: package-maintainer': Array.from(maintainers).some(m => approvals.has(m)), - '12.first-time contribution': - [ 'NONE', 'FIRST_TIMER', 'FIRST_TIME_CONTRIBUTOR' ].includes(pull_request.author_association), - } - ) - - // No need for an API request, if all labels are the same. - const hasChanges = Object.keys(after).some(name => (before[name] ?? false) != after[name]) - if (log('Has changes', hasChanges, !hasChanges)) - return; - - // Skipping labeling on a pull_request event, because we have no privileges. - const labels = Object.entries(after).filter(([,value]) => value).map(([name]) => name) - if (log('Set labels', labels, context.eventName == 'pull_request')) - return; - - await github.rest.issues.setLabels({ - ...context.repo, - issue_number: pull_request.number, - labels - }) - } catch (cause) { - throw new Error(`Labeling PR #${pull_request.number} failed.`, { cause }) - } - }) + (response, done) => response.data.map(pull_request => handle(pull_request, done)) ); (await Promise.allSettled(prs.flat())) From f394b2741ed5fe11e79fc926a7547ec174e219e2 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 22 Jun 2025 16:30:00 +0200 Subject: [PATCH 2/8] workflows/labels: refactor moving cutoff downwards --- .github/workflows/labels.yml | 52 ++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index aacaa311a0c9..0cace4e0217b 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -131,30 +131,6 @@ jobs: if (process.env.UPDATED_WITHIN && !/^\d+$/.test(process.env.UPDATED_WITHIN)) throw new Error('Please enter "updated within" as integer in hours.') - const cutoff = new Date(await (async () => { - // Always run for Pull Request triggers, no cutoff since there will be a single - // response only anyway. 0 is the Unix epoch, so always smaller. - if (context.payload.pull_request?.number) return 0 - - // Manually triggered via UI when updatedWithin is set. Will fallthrough to the last - // option if the updatedWithin parameter is set to 0, which is the default. - const updatedWithin = Number.parseInt(process.env.UPDATED_WITHIN, 10) - if (updatedWithin) return new Date().getTime() - updatedWithin * 60 * 60 * 1000 - - // Normally a scheduled run, but could be workflow_dispatch, see above. Go back as far - // as the last successful run of this workflow to make sure we are not leaving anyone - // behind on GHA failures. - // Defaults to go back 1 hour on the first run. - return (await github.rest.actions.listWorkflowRuns({ - ...context.repo, - workflow_id: 'labels.yml', - event: 'schedule', - status: 'success', - exclude_pull_requests: true - })).data.workflow_runs[0]?.created_at ?? new Date().getTime() - 1 * 60 * 60 * 1000 - })()) - core.info('cutoff timestamp: ' + cutoff.toISOString()) - // To simplify this action's logic we fetch the pull_request data again below, even if // we are already in a pull_request event's context and would have the data readily // available. We do this by filtering the list of pull requests with head and base @@ -165,7 +141,7 @@ jobs: base: context.payload.pull_request.base.ref } - async function handle(pull_request, done) { + async function handle(pull_request, done, cutoff) { try { const log = (k,v,skip) => { core.info(`PR #${pull_request.number} - ${k}: ${v}` + (skip ? ' (skipped)' : '')) @@ -292,6 +268,30 @@ jobs: } } + const cutoff = new Date(await (async () => { + // Always run for Pull Request triggers, no cutoff since there will be a single + // response only anyway. 0 is the Unix epoch, so always smaller. + if (context.payload.pull_request?.number) return 0 + + // Manually triggered via UI when updatedWithin is set. Will fallthrough to the last + // option if the updatedWithin parameter is set to 0, which is the default. + const updatedWithin = Number.parseInt(process.env.UPDATED_WITHIN, 10) + if (updatedWithin) return new Date().getTime() - updatedWithin * 60 * 60 * 1000 + + // Normally a scheduled run, but could be workflow_dispatch, see above. Go back as far + // as the last successful run of this workflow to make sure we are not leaving anyone + // behind on GHA failures. + // Defaults to go back 1 hour on the first run. + return (await github.rest.actions.listWorkflowRuns({ + ...context.repo, + workflow_id: 'labels.yml', + event: 'schedule', + status: 'success', + exclude_pull_requests: true + })).data.workflow_runs[0]?.created_at ?? new Date().getTime() - 1 * 60 * 60 * 1000 + })()) + core.info('cutoff timestamp: ' + cutoff.toISOString()) + const prs = await github.paginate( github.rest.pulls.list, { @@ -301,7 +301,7 @@ jobs: direction: 'desc', ...prEventCondition }, - (response, done) => response.data.map(pull_request => handle(pull_request, done)) + (response, done) => response.data.map(pull_request => handle(pull_request, done, cutoff)) ); (await Promise.allSettled(prs.flat())) From 8b5101554abc12b9fdf6be4095f11b1d0b1e2892 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 22 Jun 2025 16:18:19 +0200 Subject: [PATCH 3/8] workflows/labels: save an API request when running in pull_request context We previously ran another list request in this case, but don't need to anymore - we already have the `pull_request` context available. --- .github/workflows/labels.yml | 86 ++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 49 deletions(-) diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 0cace4e0217b..7bf96fe5856d 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -131,16 +131,6 @@ jobs: if (process.env.UPDATED_WITHIN && !/^\d+$/.test(process.env.UPDATED_WITHIN)) throw new Error('Please enter "updated within" as integer in hours.') - // To simplify this action's logic we fetch the pull_request data again below, even if - // we are already in a pull_request event's context and would have the data readily - // available. We do this by filtering the list of pull requests with head and base - // branch - there can only be a single open Pull Request for any such combination. - const prEventCondition = !context.payload.pull_request ? undefined : { - // "label" is in the format of `user:branch` or `org:branch` - head: context.payload.pull_request.head.label, - base: context.payload.pull_request.base.ref - } - async function handle(pull_request, done, cutoff) { try { const log = (k,v,skip) => { @@ -157,8 +147,7 @@ jobs: ...context.repo, workflow_id: 'pr.yml', event: 'pull_request_target', - // For PR events, the workflow run is still in progress with this job itself. - status: prEventCondition ? 'in_progress' : 'success', + status: 'success', exclude_pull_requests: true, head_sha: pull_request.head.sha })).data.workflow_runs[0]?.id ?? @@ -268,47 +257,46 @@ jobs: } } - const cutoff = new Date(await (async () => { - // Always run for Pull Request triggers, no cutoff since there will be a single - // response only anyway. 0 is the Unix epoch, so always smaller. - if (context.payload.pull_request?.number) return 0 + if (context.payload.pull_request) { + await handle(context.payload.pull_request) + } else { + const cutoff = new Date(await (async () => { + // Manually triggered via UI when updatedWithin is set. Will fallthrough to the last + // option if the updatedWithin parameter is set to 0, which is the default. + const updatedWithin = Number.parseInt(process.env.UPDATED_WITHIN, 10) + if (updatedWithin) return new Date().getTime() - updatedWithin * 60 * 60 * 1000 - // Manually triggered via UI when updatedWithin is set. Will fallthrough to the last - // option if the updatedWithin parameter is set to 0, which is the default. - const updatedWithin = Number.parseInt(process.env.UPDATED_WITHIN, 10) - if (updatedWithin) return new Date().getTime() - updatedWithin * 60 * 60 * 1000 + // Normally a scheduled run, but could be workflow_dispatch, see above. Go back as far + // as the last successful run of this workflow to make sure we are not leaving anyone + // behind on GHA failures. + // Defaults to go back 1 hour on the first run. + return (await github.rest.actions.listWorkflowRuns({ + ...context.repo, + workflow_id: 'labels.yml', + event: 'schedule', + status: 'success', + exclude_pull_requests: true + })).data.workflow_runs[0]?.created_at ?? new Date().getTime() - 1 * 60 * 60 * 1000 + })()) + core.info('cutoff timestamp: ' + cutoff.toISOString()) - // Normally a scheduled run, but could be workflow_dispatch, see above. Go back as far - // as the last successful run of this workflow to make sure we are not leaving anyone - // behind on GHA failures. - // Defaults to go back 1 hour on the first run. - return (await github.rest.actions.listWorkflowRuns({ - ...context.repo, - workflow_id: 'labels.yml', - event: 'schedule', - status: 'success', - exclude_pull_requests: true - })).data.workflow_runs[0]?.created_at ?? new Date().getTime() - 1 * 60 * 60 * 1000 - })()) - core.info('cutoff timestamp: ' + cutoff.toISOString()) + const prs = await github.paginate( + github.rest.pulls.list, + { + ...context.repo, + state: 'open', + sort: 'updated', + direction: 'desc', + }, + (response, done) => response.data.map(pull_request => handle(pull_request, done, cutoff)) + ); - const prs = await github.paginate( - github.rest.pulls.list, - { - ...context.repo, - state: 'open', - sort: 'updated', - direction: 'desc', - ...prEventCondition - }, - (response, done) => response.data.map(pull_request => handle(pull_request, done, cutoff)) - ); + (await Promise.allSettled(prs.flat())) + .filter(({ status }) => status == 'rejected') + .map(({ reason }) => core.setFailed(`${reason.message}\n${reason.cause.stack}`)) - (await Promise.allSettled(prs.flat())) - .filter(({ status }) => status == 'rejected') - .map(({ reason }) => core.setFailed(`${reason.message}\n${reason.cause.stack}`)) - - core.notice(`Processed ${stats.prs} PRs, made ${stats.requests + stats.artifacts} API requests and downloaded ${stats.artifacts} artifacts.`) + core.notice(`Processed ${stats.prs} PRs, made ${stats.requests + stats.artifacts} API requests and downloaded ${stats.artifacts} artifacts.`) + } clearInterval(reservoirUpdater) - name: Log current API rate limits From d9d97fda59d81255aa606f1fccf04dc9f2b6240c Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 22 Jun 2025 12:21:20 +0200 Subject: [PATCH 4/8] workflows/labels: refactor to search instead of listing PRs This doesn't provide much value in itself, yet, but is much more flexible in the next step, when also looking at much older PRs. --- .github/workflows/labels.yml | 59 +++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 7bf96fe5856d..6d727e34d5c6 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -101,6 +101,9 @@ jobs: github.hook.wrap('request', async (request, options) => { // Requests to the /rate_limit endpoint do not count against the rate limit. if (options.url == '/rate_limit') return request(options) + // Search requests are in a different resource group, which allows 30 requests / minute. + // We do less than a handful each run, so not implementing throttling for now. + if (options.url.startsWith('/search/')) return request(options) stats.requests++ if (['POST', 'PUT', 'PATCH', 'DELETE'].includes(options.method)) return writeLimits.schedule(request.bind(null, options)) @@ -131,17 +134,28 @@ jobs: if (process.env.UPDATED_WITHIN && !/^\d+$/.test(process.env.UPDATED_WITHIN)) throw new Error('Please enter "updated within" as integer in hours.') - async function handle(pull_request, done, cutoff) { + async function handle(item) { try { const log = (k,v,skip) => { - core.info(`PR #${pull_request.number} - ${k}: ${v}` + (skip ? ' (skipped)' : '')) + core.info(`#${item.number} - ${k}: ${v}` + (skip ? ' (skipped)' : '')) return skip } - if (log('Last updated at', pull_request.updated_at, new Date(pull_request.updated_at) < cutoff)) - return done() + log('Last updated at', item.updated_at) stats.prs++ - log('URL', pull_request.html_url) + log('URL', item.html_url) + + const pull_number = item.number + const issue_number = item.number + + // The search result is of a format that works for both issues and pull requests and thus + // does not have all fields of a full pull_request response. Notably, it is missing `head.sha`, + // which we need to fetch the workflow run below. When triggered via pull_request event, + // this field is already available. + const pull_request = item.head ? item : (await github.rest.pulls.get({ + ...context.repo, + pull_number + })).data const run_id = (await github.rest.actions.listWorkflowRuns({ ...context.repo, @@ -188,7 +202,7 @@ jobs: repositoryOwner: context.repo.owner, token: core.getInput('github-token') }, - path: path.resolve(pull_request.number.toString()), + path: path.resolve(pull_number.toString()), expectedHash: artifact.digest }) @@ -197,7 +211,7 @@ jobs: const before = Object.fromEntries( (await github.paginate(github.rest.issues.listLabelsOnIssue, { ...context.repo, - issue_number: pull_request.number + issue_number })) .map(({ name }) => [name, true]) ) @@ -205,17 +219,17 @@ jobs: const approvals = new Set( (await github.paginate(github.rest.pulls.listReviews, { ...context.repo, - pull_number: pull_request.number + pull_number })) .filter(review => review.state == 'APPROVED') .map(review => review.user?.id) ) const maintainers = new Set(Object.keys( - JSON.parse(await readFile(`${pull_request.number}/maintainers.json`, 'utf-8')) + JSON.parse(await readFile(`${pull_number}/maintainers.json`, 'utf-8')) ).map(m => Number.parseInt(m, 10))) - const evalLabels = JSON.parse(await readFile(`${pull_request.number}/changed-paths.json`, 'utf-8')).labels + const evalLabels = JSON.parse(await readFile(`${pull_number}/changed-paths.json`, 'utf-8')).labels // Manage the labels const after = Object.assign( @@ -249,11 +263,11 @@ jobs: await github.rest.issues.setLabels({ ...context.repo, - issue_number: pull_request.number, + issue_number, labels }) } catch (cause) { - throw new Error(`Labeling PR #${pull_request.number} failed.`, { cause }) + throw new Error(`Labeling #${item.number} failed.`, { cause }) } } @@ -280,18 +294,21 @@ jobs: })()) core.info('cutoff timestamp: ' + cutoff.toISOString()) - const prs = await github.paginate( - github.rest.pulls.list, + const items = await github.paginate( + github.rest.search.issuesAndPullRequests, { - ...context.repo, - state: 'open', - sort: 'updated', - direction: 'desc', - }, - (response, done) => response.data.map(pull_request => handle(pull_request, done, cutoff)) + q: [ + `repo:"${process.env.GITHUB_REPOSITORY}"`, + 'type:pr', + 'is:open', + `updated:>=${cutoff.toISOString()}` + ].join(' AND '), + // TODO: Remove in 2025-10, when it becomes the default. + advanced_search: true + } ); - (await Promise.allSettled(prs.flat())) + (await Promise.allSettled(items.map(handle))) .filter(({ status }) => status == 'rejected') .map(({ reason }) => core.setFailed(`${reason.message}\n${reason.cause.stack}`)) From e55128a079e998204d0e0721cc750ee354d0ea8e Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Mon, 23 Jun 2025 19:47:22 +0200 Subject: [PATCH 5/8] workflows/labels: run on every PR eventually This replaces the manual dispatch trigger with a batched run through all pull requests every day. This has the small benefit of not having to worry about backfilling labeling after fixing bugs - and the much bigger one in being able to handle merge-conflict and stale labels properly later. For those, it's inevitable to eventually scan through all PRs. At this stage, the vast majority of PRs will still be skipped, because there won't be an eval run with artifact available. This will be improved in the next step. Technically, the workflow_dispatch trigger is kept to allow easily testing this in forks, where the scheduled jobs are disabled. The triggered job will behave similar to the scheduled job, though, and have no special inputs. --- .github/workflows/labels.yml | 85 ++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 6d727e34d5c6..95385b482af8 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -17,18 +17,12 @@ on: NIXPKGS_CI_APP_PRIVATE_KEY: required: true workflow_dispatch: - inputs: - updatedWithin: - description: 'Updated within [hours]' - type: number - required: false - default: 0 # everything since last run concurrency: # This explicitly avoids using `run_id` for the concurrency key to make sure that only - # *one* non-PR run can run at a time. + # *one* scheduled run can run at a time. group: labels-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number }} - # PR- and manually-triggered runs will be cancelled, but scheduled runs will be queued. + # PR-triggered runs will be cancelled, but scheduled runs will be queued. cancel-in-progress: ${{ github.event_name != 'schedule' }} # This is used as fallback without app only. @@ -69,8 +63,6 @@ jobs: - name: Labels from API data and Eval results uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 - env: - UPDATED_WITHIN: ${{ inputs.updatedWithin }} with: github-token: ${{ steps.app-token.outputs.token || github.token }} script: | @@ -131,9 +123,6 @@ jobs: const reservoirUpdater = setInterval(updateReservoir, 60 * 1000) process.on('uncaughtException', () => clearInterval(reservoirUpdater)) - if (process.env.UPDATED_WITHIN && !/^\d+$/.test(process.env.UPDATED_WITHIN)) - throw new Error('Please enter "updated within" as integer in hours.') - async function handle(item) { try { const log = (k,v,skip) => { @@ -274,27 +263,22 @@ jobs: if (context.payload.pull_request) { await handle(context.payload.pull_request) } else { - const cutoff = new Date(await (async () => { - // Manually triggered via UI when updatedWithin is set. Will fallthrough to the last - // option if the updatedWithin parameter is set to 0, which is the default. - const updatedWithin = Number.parseInt(process.env.UPDATED_WITHIN, 10) - if (updatedWithin) return new Date().getTime() - updatedWithin * 60 * 60 * 1000 + const workflowData = (await github.rest.actions.listWorkflowRuns({ + ...context.repo, + workflow_id: 'labels.yml', + event: 'schedule', + status: 'success', + exclude_pull_requests: true, + per_page: 1 + })).data - // Normally a scheduled run, but could be workflow_dispatch, see above. Go back as far - // as the last successful run of this workflow to make sure we are not leaving anyone - // behind on GHA failures. - // Defaults to go back 1 hour on the first run. - return (await github.rest.actions.listWorkflowRuns({ - ...context.repo, - workflow_id: 'labels.yml', - event: 'schedule', - status: 'success', - exclude_pull_requests: true - })).data.workflow_runs[0]?.created_at ?? new Date().getTime() - 1 * 60 * 60 * 1000 - })()) + // Go back as far as the last successful run of this workflow to make sure + // we are not leaving anyone behind on GHA failures. + // Defaults to go back 1 hour on the first run. + const cutoff = new Date(workflowData.workflow_runs[0]?.created_at ?? new Date().getTime() - 1 * 60 * 60 * 1000) core.info('cutoff timestamp: ' + cutoff.toISOString()) - const items = await github.paginate( + const updatedItems = await github.paginate( github.rest.search.issuesAndPullRequests, { q: [ @@ -306,9 +290,44 @@ jobs: // TODO: Remove in 2025-10, when it becomes the default. advanced_search: true } - ); + ) - (await Promise.allSettled(items.map(handle))) + const allOptions = { + q: [ + `repo:"${process.env.GITHUB_REPOSITORY}"`, + 'type:pr', + 'is:open' + ].join(' AND '), + sort: 'created', + direction: 'asc', + // TODO: Remove in 2025-10, when it becomes the default. + advanced_search: true + } + + const { total_count: total_pulls } = (await github.rest.search.issuesAndPullRequests({ + ...allOptions, + per_page: 1 + })).data + const { total_count: total_runs } = workflowData + const allItems = (await github.rest.search.issuesAndPullRequests({ + ...allOptions, + per_page: 100, + // We iterate through pages of 100 items across scheduled runs. With currently ~7000 open PRs and + // up to 6*24=144 scheduled runs per day, we hit every PR twice each day. + // We might not hit every PR on one iteration, because the pages will shift slightly when + // PRs are closed or merged. We assume this to be OK on the bigger scale, because a PR which was + // missed once, would have to move through the whole page to be missed again. This is very unlikely, + // so it should certainly be hit on the next iteration. + // TODO: Evaluate after a while, whether the above holds still true and potentially implement + // an overlap between runs. + page: total_runs % Math.ceil(total_pulls / 100) + })).data.items + + // Some items might be in both search results, so filtering out duplicates as well. + const items = [].concat(updatedItems, allItems) + .filter((thisItem, idx, arr) => idx == arr.findIndex(firstItem => firstItem.number == thisItem.number)) + + ;(await Promise.allSettled(items.map(handle))) .filter(({ status }) => status == 'rejected') .map(({ reason }) => core.setFailed(`${reason.message}\n${reason.cause.stack}`)) From 63b9355ed892b715a881f95e71fc3c22c67bb641 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Mon, 23 Jun 2025 22:18:25 +0200 Subject: [PATCH 6/8] workflows/labels: handle missing eval results gracefully We keep working through the PR, even though we don't have any eval results. This will allow actually managing labels for much older PRs as well. Most importantly, it will allow merge-conflict and stale-labeling next. --- .github/workflows/labels.yml | 76 ++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 95385b482af8..dc1c9344717f 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -165,12 +165,12 @@ jobs: head_sha: pull_request.head.sha })).data.workflow_runs[0]?.id - // Newer PRs might not have run Eval to completion, yet. We can skip them, because this - // job will be run as part of that Eval run anyway. - if (log('Last eval run', run_id ?? '', !run_id)) - return; + // Newer PRs might not have run Eval to completion, yet. + // Older PRs might not have an eval.yml workflow, yet. + // In either case we continue without fetching an artifact on a best-effort basis. + log('Last eval run', run_id ?? '') - const artifact = (await github.rest.actions.listWorkflowRunArtifacts({ + const artifact = run_id && (await github.rest.actions.listWorkflowRunArtifacts({ ...context.repo, run_id, name: 'comparison' @@ -179,21 +179,22 @@ jobs: // Instead of checking the boolean artifact.expired, we will give us a minute to // actually download the artifact in the next step and avoid that race condition. // Older PRs, where the workflow run was already eval.yml, but the artifact was not - // called "comparison", yet, will be skipped as well. - const expired = new Date(artifact?.expires_at ?? 0) < new Date(new Date().getTime() + 60 * 1000) - if (log('Artifact expires at', artifact?.expires_at ?? '', expired)) - return; - stats.artifacts++ + // called "comparison", yet, will skip the download. + const expired = !artifact || new Date(artifact?.expires_at ?? 0) < new Date(new Date().getTime() + 60 * 1000) + log('Artifact expires at', artifact?.expires_at ?? '') + if (!expired) { + stats.artifacts++ - await artifactClient.downloadArtifact(artifact.id, { - findBy: { - repositoryName: context.repo.repo, - repositoryOwner: context.repo.owner, - token: core.getInput('github-token') - }, - path: path.resolve(pull_number.toString()), - expectedHash: artifact.digest - }) + await artifactClient.downloadArtifact(artifact.id, { + findBy: { + repositoryName: context.repo.repo, + repositoryOwner: context.repo.owner, + token: core.getInput('github-token') + }, + path: path.resolve(pull_number.toString()), + expectedHash: artifact.digest + }) + } // Create a map (Label -> Boolean) of all currently set labels. // Each label is set to True and can be disabled later. @@ -214,32 +215,41 @@ jobs: .map(review => review.user?.id) ) - const maintainers = new Set(Object.keys( - JSON.parse(await readFile(`${pull_number}/maintainers.json`, 'utf-8')) - ).map(m => Number.parseInt(m, 10))) - - const evalLabels = JSON.parse(await readFile(`${pull_number}/changed-paths.json`, 'utf-8')).labels - - // Manage the labels + // Manage most of the labels, without eval results const after = Object.assign( {}, before, - // Ignore `evalLabels` if it's an array. - // This can happen for older eval runs, before we switched to objects. - // The old eval labels would have been set by the eval run, - // so now they'll be present in `before`. - // TODO: Simplify once old eval results have expired (~2025-10) - (Array.isArray(evalLabels) ? undefined : evalLabels), { '12.approvals: 1': approvals.size == 1, '12.approvals: 2': approvals.size == 2, '12.approvals: 3+': approvals.size >= 3, - '12.approved-by: package-maintainer': Array.from(maintainers).some(m => approvals.has(m)), '12.first-time contribution': [ 'NONE', 'FIRST_TIMER', 'FIRST_TIME_CONTRIBUTOR' ].includes(pull_request.author_association), } ) + // Manage labels based on eval results + if (!expired) { + const maintainers = new Set(Object.keys( + JSON.parse(await readFile(`${pull_number}/maintainers.json`, 'utf-8')) + ).map(m => Number.parseInt(m, 10))) + + const evalLabels = JSON.parse(await readFile(`${pull_number}/changed-paths.json`, 'utf-8')).labels + + Object.assign( + after, + // Ignore `evalLabels` if it's an array. + // This can happen for older eval runs, before we switched to objects. + // The old eval labels would have been set by the eval run, + // so now they'll be present in `before`. + // TODO: Simplify once old eval results have expired (~2025-10) + (Array.isArray(evalLabels) ? undefined : evalLabels), + { + '12.approved-by: package-maintainer': Array.from(maintainers).some(m => approvals.has(m)), + } + ) + } + // No need for an API request, if all labels are the same. const hasChanges = Object.keys(after).some(name => (before[name] ?? false) != after[name]) if (log('Has changes', hasChanges, !hasChanges)) From 58dd9630c38123707c6acd97fb65f33f36c70af1 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Mon, 23 Jun 2025 22:33:12 +0200 Subject: [PATCH 7/8] workflows/labels: manage stale label for pull requests This manages the `2. status: stale` label for pull requests only (not issues, yet) with the following conditions: - The last event on the timeline of the Pull Request counts. - Labeling and unlabeling of any kind are ignored. - Older than 180 days are stale. - Security labeled PRs are never stale. To handle this label correctly, it's important to go through all pull requests. Any approach to limit the list of PRs via search are not going to work: - Filtering by `updated` is not going to work, because it includes the last time that *a label was set* on the PR. To actually find out whether a PR is stale or not, the timeline of events needs to be looked at. - Filtering by an existing stale label is not going to work either, because such a label might have been added manually and thus breaking the rules we set up here. Thus any existing label needs to be confirmed as well. --- .github/workflows/labels.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index dc1c9344717f..99a5dcf7004d 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -215,11 +215,28 @@ jobs: .map(review => review.user?.id) ) + const latest_event_at = new Date( + (await github.paginate( + github.rest.issues.listEventsForTimeline, + { + ...context.repo, + issue_number, + per_page: 100 + } + )) + // We also ignore base_ref_force_pushed, which will not happen in nixpkgs, but + // is very useful for testing in forks. + .findLast(({ event }) => !['labeled', 'unlabeled', 'base_ref_force_pushed'].includes(event)) + ?.created_at ?? item.created_at + ) + const stale_at = new Date(new Date().setDate(new Date().getDate() - 180)) + // Manage most of the labels, without eval results const after = Object.assign( {}, before, { + '2.status: stale': !before['1.severity: security'] && latest_event_at < stale_at, '12.approvals: 1': approvals.size == 1, '12.approvals: 2': approvals.size == 2, '12.approvals: 3+': approvals.size >= 3, From 36e9fe9e7d8537438cf5d8f3cf197921f171d154 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Mon, 23 Jun 2025 22:53:41 +0200 Subject: [PATCH 8/8] workflows/labels: manage merge-conflict label for pull requests The code comments describe much better what we do then a commit message could ever do. --- .github/workflows/labels.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 99a5dcf7004d..c3bc9301f67f 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -141,6 +141,8 @@ jobs: // does not have all fields of a full pull_request response. Notably, it is missing `head.sha`, // which we need to fetch the workflow run below. When triggered via pull_request event, // this field is already available. + // This API request is also important for the merge-conflict label, because it triggers the + // creation of a new test merge commit. This is needed to actually determine the state of a PR. const pull_request = item.head ? item : (await github.rest.pulls.get({ ...context.repo, pull_number @@ -236,6 +238,18 @@ jobs: {}, before, { + // We intentionally don't use the mergeable or mergeable_state attributes. + // Those have an intermediate state while the test merge commit is created. + // This doesn't work well for us, because we might have just triggered another + // test merge commit creation by request the pull request via API at the start + // of this function. + // The attribute merge_commit_sha keeps the old value of null or the hash *until* + // the new test merge commit has either successfully been created or failed so. + // This essentially means we are updating the merge conflict label in two steps: + // On the first pass of the day, we just fetch the pull request, which triggers + // the creation. At this stage, the label is likely not updated, yet. + // The second pass will then read the result from the first pass and set the label. + '2.status: merge conflict': !pull_request.merge_commit_sha, '2.status: stale': !before['1.severity: security'] && latest_event_at < stale_at, '12.approvals: 1': approvals.size == 1, '12.approvals: 2': approvals.size == 2,