From c18e94361ef21171c48522142adfbb1081be90bf Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Tue, 1 Jul 2025 19:28:31 +0000 Subject: [PATCH] Revert "workflows/labels: label stale issues" --- .github/stale.yml | 9 ++ .github/workflows/labels.yml | 300 +++++++++++++++++------------------ 2 files changed, 153 insertions(+), 156 deletions(-) create mode 100644 .github/stale.yml diff --git a/.github/stale.yml b/.github/stale.yml new file mode 100644 index 000000000000..d6134c7ce112 --- /dev/null +++ b/.github/stale.yml @@ -0,0 +1,9 @@ +# Configuration for probot-stale - https://github.com/probot/stale +daysUntilStale: 180 +daysUntilClose: false +exemptLabels: + - "1.severity: security" + - "2.status: never-stale" +staleLabel: "2.status: stale" +markComment: false +closeComment: false diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index e61615091486..d84f49b5895c 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -27,8 +27,10 @@ concurrency: # This is used as fallback without app only. # This happens when testing in forks without setting up that app. +# Labels will most likely not exist in forks, yet. For this case, +# we add the issues permission only here. permissions: - issues: write + issues: write # needed to create *new* labels pull-requests: write defaults: @@ -50,7 +52,8 @@ jobs: with: app-id: ${{ vars.NIXPKGS_CI_APP_ID }} private-key: ${{ secrets.NIXPKGS_CI_APP_PRIVATE_KEY }} - permission-issues: write + # No issues: write permission here, because labels in Nixpkgs should + # be created explicitly via the UI with color and description. permission-pull-requests: write - name: Log current API rate limits @@ -72,7 +75,6 @@ jobs: const artifactClient = new DefaultArtifactClient() const stats = { - issues: 0, prs: 0, requests: 0, artifacts: 0 @@ -121,125 +123,6 @@ jobs: // Update remaining requests every minute to account for other jobs running in parallel. const reservoirUpdater = setInterval(updateReservoir, 60 * 1000) - async function handlePullRequest(item) { - const log = (k,v) => core.info(`PR #${item.number} - ${k}: ${v}`) - - const pull_number = item.number - - // This API request is 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 = (await github.rest.pulls.get({ - ...context.repo, - pull_number - })).data - - const approvals = new Set( - (await github.paginate(github.rest.pulls.listReviews, { - ...context.repo, - pull_number - })) - .filter(review => review.state == 'APPROVED') - .map(review => review.user?.id) - ) - - // After creation of a Pull Request, `merge_commit_sha` will be null initially: - // The very first merge commit will only be calculated after a little while. - // To avoid labeling the PR as conflicted before that, we wait a few minutes. - // This is intentionally less than the time that Eval takes, so that the label job - // running after Eval can indeed label the PR as conflicted if that is the case. - const merge_commit_sha_valid = new Date() - new Date(pull_request.created_at) > 3 * 60 * 1000 - - const prLabels = { - // 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': merge_commit_sha_valid && !pull_request.merge_commit_sha, - '12.approvals: 1': approvals.size == 1, - '12.approvals: 2': approvals.size == 2, - '12.approvals: 3+': approvals.size >= 3, - '12.first-time contribution': - [ 'NONE', 'FIRST_TIMER', 'FIRST_TIME_CONTRIBUTOR' ].includes(pull_request.author_association), - } - - const run_id = (await github.rest.actions.listWorkflowRuns({ - ...context.repo, - workflow_id: 'pr.yml', - event: 'pull_request_target', - 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. - // 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 = run_id && (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 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 - }) - - 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( - prLabels, - // 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)), - } - ) - } - - return prLabels - } - async function handle(item) { try { const log = (k,v,skip) => { @@ -248,19 +131,87 @@ jobs: } log('Last updated at', item.updated_at) + stats.prs++ log('URL', item.html_url) + const pull_number = item.number const issue_number = item.number - const itemLabels = {} + // This API request is 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 = (await github.rest.pulls.get({ + ...context.repo, + pull_number + })).data - if (item.pull_request) { - stats.prs++ - Object.assign(itemLabels, await handlePullRequest(item)) - } else { - stats.issues++ + const run_id = (await github.rest.actions.listWorkflowRuns({ + ...context.repo, + workflow_id: 'pr.yml', + event: 'pull_request_target', + 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. + // 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 = run_id && (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 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 + }) } + // 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 + })) + .map(({ name }) => [name, true]) + ) + + const approvals = new Set( + (await github.paginate(github.rest.pulls.listReviews, { + ...context.repo, + pull_number + })) + .filter(review => review.state == 'APPROVED') + .map(review => review.user?.id) + ) + const latest_event_at = new Date( (await github.paginate( github.rest.issues.listEventsForTimeline, @@ -299,21 +250,60 @@ jobs: const stale_at = new Date(new Date().setDate(new Date().getDate() - 180)) - // 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 - })) - .map(({ name }) => [name, true]) + // After creation of a Pull Request, `merge_commit_sha` will be null initially: + // The very first merge commit will only be calculated after a little while. + // To avoid labeling the PR as conflicted before that, we wait a few minutes. + // This is intentionally less than the time that Eval takes, so that the label job + // running after Eval can indeed label the PR as conflicted if that is the case. + const merge_commit_sha_valid = new Date() - new Date(pull_request.created_at) > 3 * 60 * 1000 + + // Manage most of the labels, without eval results + const after = Object.assign( + {}, + 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': merge_commit_sha_valid && !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, + '12.approvals: 3+': approvals.size >= 3, + '12.first-time contribution': + [ 'NONE', 'FIRST_TIMER', 'FIRST_TIME_CONTRIBUTOR' ].includes(pull_request.author_association), + } ) - Object.assign(itemLabels, { - '2.status: stale': !before['1.severity: security'] && latest_event_at < stale_at, - }) + // 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 after = Object.assign({}, before, itemLabels) + 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]) @@ -359,6 +349,7 @@ jobs: { q: [ `repo:"${process.env.GITHUB_REPOSITORY}"`, + 'type:pr', 'is:open', `updated:>=${cutoff.toISOString()}` ].join(' AND '), @@ -368,11 +359,13 @@ jobs: ) // The search endpoint only allows fetching the first 1000 records, but the - // list endpoints do not support counting the total number of results. - // Thus, we use /search for counting and /issues for reading the response. - const { total_count: total_items } = (await github.rest.search.issuesAndPullRequests({ + // pull request list endpoint does not support counting the total number + // of results. + // Thus, we use /search for counting and /pulls for reading the response. + const { total_count: total_pulls } = (await github.rest.search.issuesAndPullRequests({ q: [ `repo:"${process.env.GITHUB_REPOSITORY}"`, + 'type:pr', 'is:open' ].join(' AND '), sort: 'created', @@ -383,37 +376,32 @@ jobs: })).data const { total_count: total_runs } = workflowData - // From GitHub's API docs: - // GitHub's REST API considers every pull request an issue, but not every issue is a pull request. - // For this reason, "Issues" endpoints may return both issues and pull requests in the response. - // You can identify pull requests by the pull_request key. - const allItems = (await github.rest.issues.listForRepo({ + const allPulls = (await github.rest.pulls.list({ ...context.repo, state: 'open', sort: 'created', direction: 'asc', per_page: 100, - // We iterate through pages of 100 items across scheduled runs. With currently ~7000 open PRs, - // 10000 open Issues and up to 6*24=144 scheduled runs per day, we hit every items a little less - // than once a day. - // We might not hit every item on one iteration, because the pages will shift slightly when - // items are closed or merged. We assume this to be OK on the bigger scale, because an item which was + // 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_items / 100)) + 1 + page: (total_runs % Math.ceil(total_pulls / 100)) + 1 })).data // Some items might be in both search results, so filtering out duplicates as well. - const items = [].concat(updatedItems, allItems) + const items = [].concat(updatedItems, allPulls) .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}`)) - core.notice(`Processed ${stats.prs} PRs, ${stats.issues} Issues, 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.`) } } finally { clearInterval(reservoirUpdater)