From 9fde573b970ff717705f6c72844fc259848df9c0 Mon Sep 17 00:00:00 2001 From: Aleksandr Logvinenko <32199530+Oxxyg33n@users.noreply.github.com> Date: Wed, 30 Sep 2026 17:50:11 +0300 Subject: [PATCH] ci: reduce PR review-label workflow costs --- .../workflows/pull-request-review-labels.yml | 58 +++++-------------- github-actions/pr-review-labeler/README.md | 13 ++--- .../pr-review-labeler/action.test.cjs | 45 +++++++++----- 3 files changed, 50 insertions(+), 66 deletions(-) diff --git a/.github/workflows/pull-request-review-labels.yml b/.github/workflows/pull-request-review-labels.yml index 7231e52..d937dd9 100644 --- a/.github/workflows/pull-request-review-labels.yml +++ b/.github/workflows/pull-request-review-labels.yml @@ -2,61 +2,33 @@ name: Pull request review labels on: pull_request_target: - types: [opened, reopened, ready_for_review, converted_to_draft, synchronize] + types: + - opened + - reopened + - ready_for_review + - converted_to_draft + - synchronize pull_request_review: - types: [submitted, dismissed] - schedule: - - cron: "43 5 * * *" - workflow_dispatch: + types: + - submitted + - dismissed permissions: contents: read pull-requests: read jobs: - targets: - # Fork and Dependabot review events have read-only tokens; daily refresh covers them. - if: >- - github.event_name != 'pull_request_review' || - (github.event.pull_request.head.repo.full_name == github.repository && - github.event.pull_request.user.login != 'dependabot[bot]') - runs-on: ubuntu-latest - timeout-minutes: 5 - outputs: - numbers: ${{ steps.targets.outputs.result }} - steps: - - name: Select pull requests - id: targets - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 - with: - script: | - if (context.payload.pull_request) { - return [context.payload.pull_request.number]; - } - const pulls = await github.paginate(github.rest.pulls.list, { - ...context.repo, - state: 'open', - per_page: 100, - }); - return pulls.map(pull => pull.number); - update-review-labels: - needs: targets - if: needs.targets.outputs.numbers != '[]' + name: update-review-labels + if: github.event.pull_request != null runs-on: ubuntu-latest - timeout-minutes: 10 permissions: contents: read issues: write pull-requests: write - strategy: - fail-fast: false - max-parallel: 4 - matrix: - number: ${{ fromJSON(needs.targets.outputs.numbers) }} concurrency: - group: pull-request-review-labels-pr-${{ matrix.number }} - cancel-in-progress: false + group: pull-request-review-labels-pr-${{ github.event.pull_request.number }} + cancel-in-progress: true steps: - name: Check out trusted action source uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 @@ -67,10 +39,8 @@ jobs: github-actions/pr-review-labeler - name: Update review labels - # The local action is available only after the install PR reaches the default branch. - if: hashFiles('github-actions/pr-review-labeler/action.yml') != '' uses: ./github-actions/pr-review-labeler with: github-token: ${{ secrets.GITHUB_TOKEN }} - pull-request-number: ${{ matrix.number }} + pull-request-number: ${{ github.event.pull_request.number }} required-approvals: "3" diff --git a/github-actions/pr-review-labeler/README.md b/github-actions/pr-review-labeler/README.md index cfbb8c2..eeb1afc 100644 --- a/github-actions/pr-review-labeler/README.md +++ b/github-actions/pr-review-labeler/README.md @@ -14,13 +14,12 @@ removes two legacy review labels, and skips PRs closed while a refresh was queue - Only reviewers with write, maintain, or admin permission count. Approval counts use the latest opinionated review per reviewer; dismissed reviews clear that opinion. - Review labels are advisory. Branch protection, required checks, code owners, conflicts, and other merge rules remain authoritative. -The workflow refreshes labels on PR lifecycle and review events, daily, and through -**Actions → Pull request review labels → Run workflow**. Manual and daily runs cover -all open PRs, including existing PRs with no new activity. Fork and Dependabot review -events are refreshed by the daily/manual run because their review-event tokens are read-only. -Each run checks out the trusted default branch with persisted credentials disabled. -It never executes PR head code with label-write permissions. The install PR skips the -label step until this local action exists on the default branch. +The workflow matches monorepo's event-driven execution: one job updates the event PR +on lifecycle and review events, and newer runs cancel older updates for that PR. +There are no daily scans, manual refreshes, or per-PR matrices. Existing inactive PRs +and missed or read-only fork/Dependabot review events are not periodically reconciled. +Each run checks out the trusted default branch with persisted credentials disabled; +it never executes PR head code with label-write permissions. The configured approval threshold is **3**. It reflects the inspected default-branch protection and rulesets at rollout time; keep it aligned with policy changes. diff --git a/github-actions/pr-review-labeler/action.test.cjs b/github-actions/pr-review-labeler/action.test.cjs index c409da1..9480e44 100644 --- a/github-actions/pr-review-labeler/action.test.cjs +++ b/github-actions/pr-review-labeler/action.test.cjs @@ -67,23 +67,38 @@ for (const scenario of scenarios) { } const workflowDir = path.resolve(__dirname, '../../.github/workflows'); -const targetScript = new AsyncFunction('github', 'context', embeddedScript(path.join(workflowDir, 'pull-request-review-labels.yml'))); +const reviewWorkflow = fs.readFileSync(path.join(workflowDir, 'pull-request-review-labels.yml'), 'utf8'); for (const scenario of [ - {name: 'PR event selects only its PR', payload: {pull_request: {number: 17}}, pulls: [], expected: [17], calls: 0}, - {name: 'manual refresh selects all paginated open PRs', payload: {}, pulls: [{number: 17}, {number: 21}], expected: [17, 21], calls: 1}, - {name: 'empty repository emits no matrix jobs', payload: {}, pulls: [], expected: [], calls: 1}, + {name: 'review updates run only for PR lifecycle and review events', check() { + assert.equal(reviewWorkflow.match(/^on:\n([\s\S]*?)\npermissions:/m)[1], + ' pull_request_target:\n types:\n - opened\n - reopened\n - ready_for_review\n - converted_to_draft\n - synchronize\n pull_request_review:\n types:\n - submitted\n - dismissed\n'); + }}, + {name: 'one label job replaces per-PR scan and target-selection jobs', check() { + const jobs = reviewWorkflow.split('\njobs:\n')[1]; + assert.deepEqual([...jobs.matchAll(/^ ([\w-]+):$/gm)].map(match => match[1]), ['update-review-labels']); + assert.doesNotMatch(jobs, /^ (needs|strategy):/m); + assert.match(jobs, /^ if: github\.event\.pull_request != null$/m); + }}, + {name: 'superseded updates are cancelled only within the same PR', check() { + assert.match(reviewWorkflow, /^ group: pull-request-review-labels-pr-\$\{\{ github\.event\.pull_request\.number \}\}$/m); + assert.match(reviewWorkflow, /^ cancel-in-progress: true$/m); + }}, + {name: 'label-write job executes trusted source without persisted credentials', check() { + assert.match(reviewWorkflow, /^ ref: \$\{\{ github\.event\.repository\.default_branch \}\}$/m); + assert.match(reviewWorkflow, /^ persist-credentials: false$/m); + assert.match(reviewWorkflow, /^ uses: actions\/checkout@[a-f0-9]{40}/m); + assert.match(reviewWorkflow, /^ uses: \.\/github-actions\/pr-review-labeler$/m); + assert.doesNotMatch(reviewWorkflow, /pull_request\.head/); + }}, + {name: 'label updates use event PR number and explicit scoped permissions', check() { + assert.match(reviewWorkflow, /^ pull-request-number: \$\{\{ github\.event\.pull_request\.number \}\}$/m); + assert.match(reviewWorkflow, /^ permissions:\n contents: read\n issues: write\n pull-requests: write$/m); + assert.doesNotMatch(reviewWorkflow, /write-all/); + }}, ]) { - test(scenario.name, async () => { - let calls = 0; - const github = {rest: {pulls: {list() {}}}, paginate: async (_method, args) => { - calls++; - assert.equal(args.state, 'open'); - assert.equal(args.per_page, 100); - return scenario.pulls; - }}; - const numbers = await targetScript(github, {repo: {owner: 'example', repo: 'example'}, payload: scenario.payload}); - assert.deepEqual(numbers, scenario.expected); - assert.equal(calls, scenario.calls); + test(scenario.name, () => { + // Assert the execution and trust boundaries that keep label maintenance inexpensive and safe. + scenario.check(); }); }