From 5fb29fe5472e35cb558634a82e37f1303ed946cd Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Tue, 6 Oct 2026 20:32:42 -0400 Subject: [PATCH] Let sync PRs approve their own builds The upstream and rebuild syncs push with GITHUB_TOKEN, so GitHub holds their build and test runs for approval. Their approve job only released those runs once a maintainer had applied build-approved, and never ran for the push that opened the PR, so every sync PR sat waiting. The sync now labels its own PR build-approved, and the approve job runs for created PRs as well as updated ones. --- .github/scripts/approve-sync-push.cjs | 9 +++++---- .github/workflows/sync-rebuilds.yml | 17 +++++++++++------ .github/workflows/sync-upstream.yml | 17 +++++++++++------ README.md | 10 +++++----- tests/pr-workflow-approval.cjs | 6 +++++- 5 files changed, 37 insertions(+), 22 deletions(-) diff --git a/.github/scripts/approve-sync-push.cjs b/.github/scripts/approve-sync-push.cjs index 7f4ea85..ecb94db 100644 --- a/.github/scripts/approve-sync-push.cjs +++ b/.github/scripts/approve-sync-push.cjs @@ -6,9 +6,10 @@ const BOT = 'github-actions[bot]'; // resulting pull_request runs for approval and, unlike a person's push, // creates no pull_request_target run, so approve-pr.yml never sees it. The // sync workflow therefore releases the runs for the commit it just pushed, -// under the same rule approve-pr.yml applies: only while a maintainer's -// build-approved label is on the PR. It acts only on its own bot-authored, -// same-repository PR for the branch and commit it pushed. +// under the same rule approve-pr.yml applies: only while build-approved is +// on the PR. The sync applies that label itself, so its pushes build without +// a maintainer. It acts only on its own bot-authored, same-repository PR for +// the branch and commit it pushed. module.exports = async function approveSyncPush({ github, context, core, number, branch, headSha, since, approve = approvePrWorkflows, ...options }) { if (!Number.isInteger(number) || !branch || !headSha || !since) { @@ -25,7 +26,7 @@ module.exports = async function approveSyncPush({ github, context, core, return; } if (!pr.labels.some(label => label.name === 'build-approved')) { - core.info(`PR #${number} has no build-approved label; its runs wait for a maintainer.`); + core.info(`PR #${number} has no build-approved label; its runs stay held.`); return; } await approve({ github, context, core, vouchStatus: 'bot', pullRequest: pr, diff --git a/.github/workflows/sync-rebuilds.yml b/.github/workflows/sync-rebuilds.yml index 8b849dd..4852140 100644 --- a/.github/workflows/sync-rebuilds.yml +++ b/.github/workflows/sync-rebuilds.yml @@ -111,7 +111,11 @@ jobs: one receives it. branch: ${{ steps.branch.outputs.branch }} delete-branch: true - labels: automated + # The bot is trusted; build-approved lets the approve job below + # release GitHub's hold on its pushes without a maintainer. + labels: | + automated + build-approved reviewers: ryanrhughes - name: Notify Basecamp on failure @@ -128,12 +132,13 @@ jobs: # GitHub holds pull_request runs from a GITHUB_TOKEN push for approval and # creates no pull_request_target run for it, so approve-pr.yml never sees - # the sync's own pushes. Once a maintainer has labelled the PR - # build-approved, release the held runs for the commit just pushed. A - # separate job, so the sync container's token never holds actions: write. + # the sync's own pushes. The sync labels its PR build-approved, so release + # the held runs for the commit just pushed, whether it opened the PR or + # updated it. A separate job, so the sync container's token never holds + # actions: write. approve: needs: sync - if: ${{ !cancelled() && needs.sync.outputs.operation == 'updated' }} + if: ${{ !cancelled() && (needs.sync.outputs.operation == 'created' || needs.sync.outputs.operation == 'updated') }} runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -144,7 +149,7 @@ jobs: - uses: actions/checkout@v4 with: persist-credentials: false - - name: Release held build and test runs if build-approved + - name: Release held build and test runs uses: actions/github-script@v7 env: NUMBER: ${{ needs.sync.outputs.number }} diff --git a/.github/workflows/sync-upstream.yml b/.github/workflows/sync-upstream.yml index 0c5ccac..7bfb34a 100644 --- a/.github/workflows/sync-upstream.yml +++ b/.github/workflows/sync-upstream.yml @@ -113,7 +113,11 @@ jobs: are left untouched; check the workflow result for outstanding failures. branch: ${{ steps.branch.outputs.branch }} delete-branch: true - labels: automated + # The bot is trusted; build-approved lets the approve job below + # release GitHub's hold on its pushes without a maintainer. + labels: | + automated + build-approved reviewers: ryanrhughes - name: Notify Basecamp on failure @@ -130,12 +134,13 @@ jobs: # GitHub holds pull_request runs from a GITHUB_TOKEN push for approval and # creates no pull_request_target run for it, so approve-pr.yml never sees - # the sync's own pushes. Once a maintainer has labelled the PR - # build-approved, release the held runs for the commit just pushed. A - # separate job, so the sync container's token never holds actions: write. + # the sync's own pushes. The sync labels its PR build-approved, so release + # the held runs for the commit just pushed, whether it opened the PR or + # updated it. A separate job, so the sync container's token never holds + # actions: write. approve: needs: sync - if: ${{ !cancelled() && needs.sync.outputs.operation == 'updated' }} + if: ${{ !cancelled() && (needs.sync.outputs.operation == 'created' || needs.sync.outputs.operation == 'updated') }} runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -146,7 +151,7 @@ jobs: - uses: actions/checkout@v4 with: persist-credentials: false - - name: Release held build and test runs if build-approved + - name: Release held build and test runs uses: actions/github-script@v7 env: NUMBER: ${{ needs.sync.outputs.number }} diff --git a/README.md b/README.md index 9cec261..02cd002 100644 --- a/README.md +++ b/README.md @@ -914,11 +914,11 @@ build artifacts. Sync PRs are pushed with `GITHUB_TOKEN`, so GitHub holds their build and test runs for approval on every push and starts no `pull_request_target` workflow -for them. Once **`build-approved`** is on a sync PR, the sync workflow's own -`approve` job releases the held runs for each commit it pushes. A push to an -`auto/sync-*` branch does not cancel the PR's in-flight build: the new build -waits for it and then reuses its artifacts, so a long aarch64 build is not -restarted by every sync. +for them. The sync workflows label their own PRs **`build-approved`**, and +their `approve` job releases the held runs for each commit they push, including +the push that opens the PR. A push to an `auto/sync-*` branch does not cancel +the PR's in-flight build: the new build waits for it and then reuses its +artifacts, so a long aarch64 build is not restarted by every sync. To approve builds for an unvouched contributor's PR, apply **`build-approved`**. Until approval, the PR shows **Awaiting build approval** and its required diff --git a/tests/pr-workflow-approval.cjs b/tests/pr-workflow-approval.cjs index d704936..d4dcd9d 100644 --- a/tests/pr-workflow-approval.cjs +++ b/tests/pr-workflow-approval.cjs @@ -363,7 +363,7 @@ test('a labelled sync PR has its bot push released', async () => { assert.deepEqual(state.approved, [1, 2]); }); -test('an unlabelled sync PR stays held for a maintainer', async () => { +test('a sync PR whose label a maintainer removed stays held', async () => { const { state, push } = syncFixture(); state.pr.labels = []; await push(); @@ -443,6 +443,10 @@ test('sync workflows push scoped runs aside and keep actions: write out of the s assert.match(sync, /branch: \$\{\{ steps\.branch\.outputs\.branch \}\}/, file); assert.doesNotMatch(sync, /^ +actions: write$/m, file); assert.match(approveJob, /^ actions: write$/m, file); + // The sync PR is bot-authored and trusted: it labels itself so its own + // pushes build, including the push that opens the PR. + assert.match(sync, /labels: \|\n +automated\n +build-approved\n/, file); + assert.match(approveJob, /needs\.sync\.outputs\.operation == 'created'/, file); assert.match(approveJob, /needs\.sync\.outputs\.operation == 'updated'/, file); } });