Merge pull request #545 from omacom/fix/build-approved-workflow-approval

Make build-approved release pending PR workflows

.github/VOUCHED.td

@@ -4,7 +4,10 @@
 # its author is trusted: repository collaborators are trusted automatically
 # and do not need listing; external contributors listed here are trusted
 # too. Anyone else gets the plan only, until a maintainer either adds them
-# here or applies the "build-approved" label to that one PR.
+# here or applies the "build-approved" label to that one PR. The label also
+# releases GitHub's approval hold for that PR's build and test workflows.
+# It remains effective while attached, without vouching for the author's
+# other PRs. An explicit denouncement cannot be overridden by the label.
 #
 # Syntax:
 #   github:username

.github/scripts/approve-pr-workflows.cjs

@@ -0,0 +1,78 @@
+const BUILD = '.github/workflows/build-pr.yml';
+const TESTS = '.github/workflows/test.yml';
+
+module.exports = async function approve({ github, context, core, vouchStatus,
+  sleep = ms => new Promise(resolve => setTimeout(resolve, ms)), attempts = 36 }) {
+  // Missing/failed vouch lookups must not become approval. Denouncements
+  // remain absolute, just as they are in the package build gate.
+  if (!['unknown', 'bot', 'collaborator', 'vouched'].includes(vouchStatus)) {
+    throw new Error(`Cannot approve workflows: vouch status is ${vouchStatus || 'missing'}.`);
+  }
+
+  const expected = context.payload.pull_request;
+  const eventTime = Date.parse(expected.updated_at);
+  if (!Number.isFinite(eventTime)) throw new Error('Missing PR event timestamp.');
+  const approved = new Set();
+  let precedingBuild;
+
+  const stillApproved = async () => {
+    const { data: pr } = await github.rest.pulls.get({
+      ...context.repo, pull_number: expected.number,
+    });
+    return pr.state === 'open' && pr.head.sha === expected.head.sha &&
+      pr.labels.some(label => label.name === 'build-approved');
+  };
+
+  // The label and PR-run events arrive independently. Wait for the build
+  // belonging to this event, rather than returning after approving an older
+  // run and leaving the new label-triggered run stuck behind GitHub's gate.
+  for (let attempt = 0; attempt < attempts; attempt++) {
+    if (attempt) await sleep(5000);
+    if (!await stillApproved()) {
+      core.info('PR closed, head changed, or build-approved removed; stopping.');
+      return;
+    }
+
+    const all = await github.paginate(github.rest.actions.listWorkflowRunsForRepo, {
+      ...context.repo, event: 'pull_request', head_sha: expected.head.sha, per_page: 100,
+    });
+    const runs = all.filter(run =>
+      run.event === 'pull_request' && run.head_sha === expected.head.sha &&
+      run.head_repository?.id === expected.head.repo.id && run.head_branch === expected.head.ref &&
+      [BUILD, TESTS].includes(run.path) &&
+      // Fork runs awaiting approval often have no pull_requests entries.
+      (!run.pull_requests?.length || run.pull_requests.some(pr => pr.number === expected.number))
+    ).sort((a, b) => a.id - b.id);
+
+    const newestBuild = runs.findLast(run => run.path === BUILD);
+    if (!newestBuild || !(Date.parse(newestBuild.created_at) >= eventTime) ||
+        !runs.some(run => run.path === TESTS &&
+          (context.payload.action === 'labeled' || Date.parse(run.created_at) >= eventTime))) continue;
+
+    if (precedingBuild) {
+      const { data: run } = await github.rest.actions.getWorkflowRun({
+        ...context.repo, run_id: precedingBuild,
+      });
+      // Approve older builds first, and let them acquire concurrency before
+      // releasing a newer build. Otherwise an older queued run could start
+      // last and cancel the label-triggered build that carries approval.
+      if (!['in_progress', 'completed'].includes(run.status) || run.conclusion === 'action_required') continue;
+      precedingBuild = undefined;
+    }
+
+    const pending = runs.filter(run => run.conclusion === 'action_required' && !approved.has(run.id) &&
+      // If the newest build already runs (e.g. a maintainer approved it),
+      // don't resurrect an obsolete hold that could cancel that newer run.
+      (run.path !== BUILD || run.id === newestBuild.id || newestBuild.conclusion === 'action_required'));
+    if (!pending.length) return;
+    const run = pending[0];
+    // Recheck after the API reads, immediately before exercising write access.
+    if (!await stillApproved()) return;
+    await github.rest.actions.approveWorkflowRun({ ...context.repo, run_id: run.id });
+    approved.add(run.id);
+    core.info(`Approved ${run.path} run ${run.id} for PR #${expected.number}.`);
+    if (run.path === BUILD) precedingBuild = run.id;
+    if (pending.length === 1) return;
+  }
+  throw new Error('Timed out waiting for PR workflows. Remove and reapply build-approved to retry.');
+};

.github/workflows/approve-pr.yml

@@ -0,0 +1,46 @@
+name: Approve PR workflows
+
+# A pull_request workflow cannot approve itself: GitHub can hold it before
+# any job starts. This workflow only runs trusted default-branch code and
+# releases the ordinary, unprivileged PR workflows after build approval.
+on:
+  pull_request_target:
+    types: [opened, synchronize, reopened, labeled]
+
+permissions:
+  contents: read
+  pull-requests: read
+  actions: write
+
+concurrency:
+  group: approve-pr-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+
+jobs:
+  approve:
+    # Match build-pr.yml's events, including other labels applied while this
+    # PR still carries build-approved: each labeled event creates a build.
+    if: contains(github.event.pull_request.labels.*.name, 'build-approved')
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      # Never check out the PR head or its merge ref with this write token.
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.repository.default_branch }}
+          persist-credentials: false
+      - id: vouch
+        uses: mitchellh/vouch/action/check-user@f23dbb5e745334f97414ec70463ce7301071a661 # v1
+        with:
+          user: ${{ github.event.pull_request.user.login }}
+          allow-fail: true
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - name: Approve this PR's pending build and test runs
+        uses: actions/github-script@v7
+        env:
+          VOUCH_STATUS: ${{ steps.vouch.outputs.status }}
+        with:
+          script: |
+            const approve = require('./.github/scripts/approve-pr-workflows.cjs');
+            await approve({ github, context, core, vouchStatus: process.env.VOUCH_STATUS });

.github/workflows/build-pr.yml

@@ -28,8 +28,7 @@ jobs:
   # collaborators, anyone in .github/VOUCHED.td (read from the default
   # branch, so a PR cannot vouch for itself), or a PR a maintainer has
   # labelled "build-approved". Everyone else gets this job's plan output
-  # and a passing `result`, which is enough for a maintainer to review
-  # before deciding to spend the compute.
+  # and a failing `result` until a maintainer approves the build.
   changes:
     runs-on: ubuntu-latest
     outputs:
@@ -64,6 +63,19 @@ jobs:
           allow-fail: true
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - id: approval
+        if: github.event_name == 'pull_request'
+        uses: actions/github-script@v7
+        with:
+          script: |
+            const { data: pr } = await github.rest.pulls.get({
+              ...context.repo, pull_number: context.payload.pull_request.number,
+            });
+            // Approving or rerunning a held run keeps its original event,
+            // which may predate the label. Read the current approval instead.
+            core.setOutput('approved', pr.state === 'open' &&
+              pr.head.sha === context.payload.pull_request.head.sha &&
+              pr.labels.some(label => label.name === 'build-approved'));
       # One matrix entry per package per architecture. Every package builds
       # once, against edge; the channels it ships to on merge are carried
       # along for information. A filename means one set of bytes.
@@ -92,16 +104,17 @@ jobs:
           fi
       - id: gate
         env:
-          STATUS: ${{ steps.vouch.outputs.status || 'dispatch' }}
+          STATUS: ${{ github.event_name == 'workflow_dispatch' && 'dispatch' || steps.vouch.outputs.status }}
           AUTHOR: ${{ github.event.pull_request.user.login }}
-          APPROVED: ${{ contains(github.event.pull_request.labels.*.name, 'build-approved') }}
+          APPROVED: ${{ steps.approval.outputs.approved || 'false' }}
           PLANNED: ${{ steps.list.outputs.planned }}
         run: |
           case "$STATUS" in
             bot|collaborator|vouched|dispatch) trusted=true ;;
             # A denouncement is absolute: the label cannot override it.
             denounced) trusted=false ;;
-            *) trusted=$APPROVED ;;
+            unknown) trusted=$APPROVED ;;
+            *) trusted=false ;;
           esac
           echo "trusted=$trusted" >> "$GITHUB_OUTPUT"
           if [[ $trusted == true ]]; then
@@ -176,8 +189,7 @@ jobs:
     steps:
       - run: |
           echo "trusted=${{ needs.changes.outputs.trusted }} build=${{ needs.build.result }}"
-          # An untrusted author's PR is held, not failed: the required check
-          # stays pending until a maintainer vouches or labels it.
+          # An untrusted author's PR fails until a maintainer vouches or labels it.
           if [[ "${{ needs.changes.outputs.trusted }}" != "true" ]]; then
             echo "::error::Builds were not run: author is not vouched. Add to .github/VOUCHED.td or apply the 'build-approved' label."
             exit 1

.github/workflows/test.yml

@@ -32,6 +32,9 @@ jobs:
         with:
           persist-credentials: false
 
+      - name: Test PR workflow approval
+        run: node --test tests/pr-workflow-approval.cjs
+
       # An Arch container for vercmp: version ordering has to be decided by
       # the same comparator pacman uses on users' machines.
       - name: Run self-tests

README.md

@@ -844,6 +844,19 @@ The repository includes GitHub workflows and systemd services for automated rele
 1. **sync-upstream.yml** (Every 6 hours): Watches direct upstream feeds and updates owned recipes. Successful package updates reach a PR even if another feed fails; failed recipes stay untouched and the workflow remains red.
 2. **sync-rebuilds.yml** (Every 6 hours): Bumps pkgrel for packages whose `rebuild_on` dependencies have moved in the official repositories and opens a PR.
 
+To approve builds for an unvouched contributor's PR, apply **`build-approved`**.
+This triggers a package build and automatically releases GitHub's pending
+build and test workflows for that PR's current commit. The approval workflow
+runs only trusted default-branch code; package builds and tests stay in the
+ordinary PR workflows. It may take a few minutes for GitHub to register and
+release all the runs.
+
+The label stays effective for that PR while attached, including later commits;
+it does not vouch for the author's other PRs. Removing it stops further label
+approvals, but does not cancel runs already released. An explicit denouncement
+in `.github/VOUCHED.td` still blocks builds. If the approval workflow times out,
+remove and reapply the label to retry.
+
 #### Systemd Services
 
 All four units run **every 5 minutes**, staggered by a minute each, so a push

tests/pr-workflow-approval.cjs

@@ -0,0 +1,242 @@
+const assert = require('node:assert/strict');
+const { readFileSync } = require('node:fs');
+const { join } = require('node:path');
+const { test } = require('node:test');
+const { execFileSync } = require('node:child_process');
+const approve = require('../.github/scripts/approve-pr-workflows.cjs');
+
+const BUILD = '.github/workflows/build-pr.yml';
+const TESTS = '.github/workflows/test.yml';
+const time = '2026-09-19T02:47:52Z';
+const earlier = '2026-09-19T02:36:04Z';
+const pr = {
+  number: 390, state: 'open', updated_at: time,
+  head: { sha: 'reviewed-sha', ref: 'ghost', repo: { id: 42 } },
+  labels: [{ name: 'build-approved' }],
+};
+const clone = value => structuredClone(value);
+const run = (id, path, overrides = {}) => ({
+  id, path, event: 'pull_request', head_sha: pr.head.sha,
+  head_repository: { id: 42 }, head_branch: 'ghost', pull_requests: [],
+  status: 'completed', conclusion: 'action_required', created_at: time,
+  ...overrides,
+});
+
+function fixture(initial = [run(1, TESTS, { created_at: earlier }), run(2, BUILD)], options = {}) {
+  const state = { pr: clone(pr), runs: clone(initial), approved: [], reads: 0, tick: 0, transitions: [] };
+  const repo = { owner: 'omacom', repo: 'omarchy-pkgs' };
+  const github = {
+    rest: {
+      pulls: { get: async args => {
+        assert.deepEqual(args, { ...repo, pull_number: 390 });
+        state.reads++;
+        options.onRead?.(state);
+        return { data: clone(state.pr) };
+      } },
+      actions: {
+        listWorkflowRunsForRepo() {},
+        getWorkflowRun: async ({ run_id }) => {
+          const current = state.runs.find(run => run.id === run_id);
+          state.transitions.push([run_id, current.status]);
+          return { data: clone(current) };
+        },
+        approveWorkflowRun: async args => {
+          assert.deepEqual(args, { ...repo, run_id: args.run_id });
+          options.onApprove?.(state, args.run_id);
+          const current = state.runs.find(run => run.id === args.run_id);
+          assert.equal(current.conclusion, 'action_required');
+          state.approved.push(current.id);
+          current.status = 'queued';
+          current.conclusion = null;
+        },
+      },
+    },
+    paginate: async (method, args) => {
+      assert.equal(method, github.rest.actions.listWorkflowRunsForRepo);
+      assert.deepEqual(args, { ...repo, event: 'pull_request', head_sha: pr.head.sha, per_page: 100 });
+      return clone(state.runs).reverse(); // GitHub returns newest first.
+    },
+  };
+  const invoke = overrides => approve({
+    github, context: { repo, payload: { action: 'labeled', pull_request: clone(pr) } },
+    core: { info() {} }, vouchStatus: 'unknown', attempts: 6,
+    sleep: async () => {
+      state.tick++;
+      for (const current of state.runs) {
+        if (current.status === 'queued' && state.tick >= (options.queueUntil ?? 1)) current.status = 'in_progress';
+      }
+      options.onSleep?.(state);
+    },
+    ...overrides,
+  });
+  return { state, invoke, github };
+}
+
+test('an unvouched, labeled fork PR releases both required workflows', async () => {
+  const { state, invoke } = fixture();
+  await invoke();
+  assert.deepEqual(state.approved, [1, 2]);
+});
+
+test('waits for the label-triggered build instead of stopping at the old build', async () => {
+  const { state, invoke } = fixture([
+    run(1, BUILD, { created_at: earlier }), run(2, TESTS, { created_at: earlier }),
+  ], {
+    onSleep(state) {
+      if (state.tick === 2) {
+        assert.deepEqual(state.approved, []);
+        state.runs.push(run(3, BUILD));
+      }
+    },
+    queueUntil: 4,
+    onApprove(state, id) {
+      if (id === 3) assert.equal(state.runs.find(run => run.id === 1).status, 'in_progress');
+    },
+  });
+  await invoke({ attempts: 10 });
+  assert.deepEqual(state.approved, [1, 2, 3]);
+  assert.ok(state.transitions.some(([id, status]) => id === 1 && status === 'queued'));
+});
+
+test('approves only the two known workflows for this fork, branch, PR and SHA', async () => {
+  const unrelated = [
+    { path: '.github/workflows/publish.yml' }, { event: 'push' },
+    { head_sha: 'other-sha' }, { head_repository: { id: 99 } },
+    { head_branch: 'other-branch' }, { pull_requests: [{ number: 391 }] },
+  ].map((overrides, i) => run(10 + i, BUILD, overrides));
+  const { state, invoke } = fixture([run(1, TESTS), run(2, BUILD), ...unrelated]);
+  await invoke();
+  assert.deepEqual(state.approved, [1, 2]);
+});
+
+test('accepts a run explicitly associated with this PR', async () => {
+  const { state, invoke } = fixture([
+    run(1, TESTS), run(2, BUILD, { pull_requests: [{ number: 390 }] }),
+  ]);
+  await invoke();
+  assert.deepEqual(state.approved, [1, 2]);
+});
+
+test('does not restart running or completed workflows', async () => {
+  const { state, invoke } = fixture([
+    run(1, TESTS, { conclusion: 'success' }),
+    run(2, BUILD, { status: 'in_progress', conclusion: null }),
+  ]);
+  await invoke();
+  assert.deepEqual(state.approved, []);
+});
+
+test('an obsolete build hold cannot cancel a newer build that was already released', async () => {
+  for (const current of [
+    { status: 'queued', conclusion: null }, { status: 'in_progress', conclusion: null },
+    { status: 'completed', conclusion: 'success' }, { status: 'completed', conclusion: 'failure' },
+  ]) {
+    const { state, invoke } = fixture([
+      run(1, BUILD, { created_at: earlier }), run(2, TESTS), run(3, BUILD, current),
+    ]);
+    await invoke();
+    assert.deepEqual(state.approved, [2]);
+  }
+});
+
+for (const status of ['denounced', '', undefined, 'unexpected']) {
+  test(`vouch status ${String(status)} fails closed`, async () => {
+    const { state, invoke } = fixture();
+    await assert.rejects(invoke({ vouchStatus: status }), /Cannot approve workflows/);
+    assert.deepEqual(state.approved, []);
+  });
+}
+
+for (const status of ['bot', 'collaborator', 'vouched']) {
+  test(`a labeled ${status} can also clear GitHub's approval gate`, async () => {
+    const { state, invoke } = fixture();
+    await invoke({ vouchStatus: status });
+    assert.deepEqual(state.approved, [1, 2]);
+  });
+}
+
+for (const [name, change] of [
+  ['removed label', pr => { pr.labels = []; }],
+  ['changed head', pr => { pr.head.sha = 'new-sha'; }],
+  ['closed PR', pr => { pr.state = 'closed'; }],
+]) {
+  test(`${name} stops approval, including changes immediately before a write`, async () => {
+    for (const read of [1, 2]) {
+      const { state, invoke } = fixture(undefined, { onRead(state) {
+        if (state.reads === read) change(state.pr);
+      } });
+      await invoke();
+      assert.deepEqual(state.approved, []);
+    }
+  });
+}
+
+test('revocation between approvals prevents releasing further workflows', async () => {
+  const { state, invoke } = fixture(undefined, { onSleep(state) { state.pr.labels = []; } });
+  await invoke();
+  assert.deepEqual(state.approved, [1]);
+});
+
+test('a delayed tests workflow is also awaited', async () => {
+  const { state, invoke } = fixture([run(2, BUILD)], {
+    onSleep(state) { if (state.tick === 2) state.runs.push(run(1, TESTS)); },
+  });
+  await invoke();
+  assert.deepEqual(state.approved, [1, 2]);
+});
+
+test('reopening a labeled PR waits for its new tests, even if old tests passed at the same SHA', async () => {
+  const { state, invoke } = fixture([
+    run(1, TESTS, { created_at: earlier, conclusion: 'success' }), run(2, BUILD),
+  ], { onSleep(state) { if (state.tick === 2) state.runs.push(run(3, TESTS)); } });
+  await invoke({ context: { repo: { owner: 'omacom', repo: 'omarchy-pkgs' },
+    payload: { action: 'reopened', pull_request: clone(pr) } } });
+  assert.deepEqual(state.approved, [2, 3]);
+});
+
+test('missing current runs time out without approving stale builds', async () => {
+  const { state, invoke } = fixture([run(1, TESTS), run(2, BUILD, { created_at: earlier })]);
+  await assert.rejects(invoke(), /Timed out/);
+  assert.deepEqual(state.approved, []);
+});
+
+test('API failure is reported rather than silently treated as approval', async () => {
+  const { state, invoke } = fixture(undefined, { onApprove() { throw new Error('Forbidden'); } });
+  await assert.rejects(invoke(), /Forbidden/);
+  assert.deepEqual(state.approved, []);
+});
+
+// Execute the actual build workflow's approval script and shell gate. This
+// covers the stale event payload that originally accompanied held PR runs.
+const workflow = readFileSync(join(__dirname, '../.github/workflows/build-pr.yml'), 'utf8');
+const approvalScript = workflow.match(/- id: approval[\s\S]*?script: \|\n([\s\S]*?)(?=      # One matrix)/)[1]
+  .split('\n').map(line => line.replace(/^            /, '')).join('\n');
+const gateScript = workflow.match(/          case "\$STATUS" in[\s\S]*?          esac/)[0] + '\nprintf "%s" "$trusted"';
+
+test('the build reads the live label rather than its pre-label event payload', async () => {
+  const execute = new (Object.getPrototypeOf(async function () {}).constructor)('github', 'context', 'core', approvalScript);
+  for (const [current, approved] of [
+    [pr, true], [{ ...pr, labels: [] }, false],
+    [{ ...pr, head: { ...pr.head, sha: 'new-sha' } }, false],
+    [{ ...pr, state: 'closed' }, false],
+  ]) {
+    const outputs = {};
+    await execute({ rest: { pulls: { get: async () => ({ data: current }) } } },
+      { repo: {}, payload: { pull_request: { ...pr, labels: [] } } },
+      { setOutput: (key, value) => { outputs[key] = value; } });
+    assert.equal(outputs.approved, approved);
+  }
+});
+
+test('the build gate permits a missing vouch only with approval, never a denouncement or lookup failure', () => {
+  for (const [status, approved, expected] of [
+    ['unknown', 'true', 'true'], ['unknown', 'false', 'false'],
+    ['denounced', 'true', 'false'], ['', 'true', 'false'], ['unexpected', 'true', 'false'],
+    ['vouched', 'false', 'true'], ['collaborator', 'false', 'true'],
+    ['bot', 'false', 'true'], ['dispatch', 'false', 'true'],
+  ]) {
+    assert.equal(execFileSync('bash', ['-c', gateScript], {
+      env: { ...process.env, STATUS: status, APPROVED: approved }, encoding: 'utf8',
+    }), expected);
+  }
+});