diff --git a/.github/scripts/traffic_control.py b/.github/scripts/traffic_control.py index 53bcd3e..efba7b2 100644 --- a/.github/scripts/traffic_control.py +++ b/.github/scripts/traffic_control.py @@ -30,9 +30,11 @@ built-in GITHUB_TOKEN instead of a PAT, so an update push no longer auto-retrigg (GitHub's anti-recursion rule) — killing the merge-cascade that cancelled every open PR's run on every merge. The cost is that a freshly-updated PR's required checks go stale/absent on its NEW head SHA, so this scheduler deliberately (re-)triggers them in priority order (a -poor-man's merge queue). A `workflow_dispatch` is EXEMPT from the anti-recursion rule, so -even the GITHUB_TOKEN's dispatch DOES start the run — no PAT needed (the workflow grants its -token `actions: write`). FAIL-OPEN, structurally: `ci.yml` KEEPS its `on: pull_request` +poor-man's merge queue). The dispatch uses the AUTOUPDATE_TOKEN PAT, NOT the built-in +GITHUB_TOKEN: a GITHUB_TOKEN-triggered run is held for MANUAL approval (`action_required`) and +never runs un-attended, whereas a PAT dispatch runs as the authorized owner with no approval gate +(#350's "no PAT needed" claim was wrong — see ci-trigger.yml + issue #351). FAIL-OPEN, +structurally: `ci.yml` KEEPS its `on: pull_request` trigger, so any human push — and a brand-new PR — always gets CI regardless of this scheduler; the scheduler only fills the gap left by GITHUB_TOKEN auto-updates and can never leave a PR un-triggerable. Fork PRs (no token/secret access) are skipped by the scheduler and @@ -685,11 +687,12 @@ def gather_trigger_snapshot() -> tuple[list[PullRequest], set[int], set[int], di def _dispatch_ci(pr_number: int, head_ref: str, head_sha: str) -> bool: - """Trigger `ci.yml` for one PR via a `workflow_dispatch` on the PR's head branch. A - workflow_dispatch is exempt from GitHub's anti-recursion rule, so even the built-in - GITHUB_TOKEN's dispatch DOES start a run — no PAT required (the workflow grants its token - `actions: write`). Running on the head branch puts the run's checks on the PR head SHA, so - they satisfy branch protection's required checks.""" + """Trigger `ci.yml` for one PR via a `workflow_dispatch` on the PR's head branch. The + dispatch runs as GH_TOKEN, which ci-trigger.yml sets to the AUTOUPDATE_TOKEN PAT: a run + triggered by the built-in GITHUB_TOKEN is held for MANUAL approval (`action_required`) and + never runs un-attended, so the PAT (authorized owner) is what actually starts the run with no + approval gate (see issue #351). Running on the head branch puts the run's checks on the PR + head SHA, so they satisfy branch protection's required checks.""" if not head_ref: _log(f"::warning::PR #{pr_number} has no head branch — cannot dispatch; skipping.") return False diff --git a/.github/workflows/autoupdate.yml b/.github/workflows/autoupdate.yml index abf594c..4cc903e 100644 --- a/.github/workflows/autoupdate.yml +++ b/.github/workflows/autoupdate.yml @@ -16,7 +16,10 @@ name: Auto-update PR branches # controller scheduler (`.github/workflows/ci-trigger.yml` -> `traffic_control.py --mode # trigger`), which triggers the updated PRs deliberately, in priority order, a few at a time. # So this workflow must NOT use the PAT for the update push (that would re-introduce the -# cascade). The AUTOUPDATE_TOKEN secret is no longer needed by this workflow. +# cascade). This workflow itself doesn't need AUTOUPDATE_TOKEN — but the secret is still REQUIRED +# by the repo: the ci-trigger.yml scheduler dispatches CI with it (a GITHUB_TOKEN dispatch would +# be held for manual approval and never run un-attended). Don't delete the secret. See +# ci-trigger.yml + issue #351. on: push: diff --git a/.github/workflows/ci-trigger.yml b/.github/workflows/ci-trigger.yml index b316ba6..03588e0 100644 --- a/.github/workflows/ci-trigger.yml +++ b/.github/workflows/ci-trigger.yml @@ -10,10 +10,15 @@ name: CI trigger (traffic-controller) # re-runs every PR" thundering herd (the cascade; see the ci-merge-cascade note + issue #349). # # HOW IT TRIGGERS: `traffic_control.py --mode trigger` runs `gh workflow run ci.yml --ref -# `. A workflow_dispatch is EXEMPT from the anti-recursion rule, so even the -# built-in GITHUB_TOKEN's dispatch DOES start the run — no PAT is required (this job grants its -# token `actions: write`). The dispatched run executes on the PR's head branch, so its checks -# land on the PR head SHA and satisfy branch protection's required checks. +# `, dispatching with the AUTOUPDATE_TOKEN PAT — NOT the built-in GITHUB_TOKEN. +# A workflow run triggered by GITHUB_TOKEN is held in the `action_required` state waiting on +# MANUAL approval and never runs un-attended (confirmed empirically on #285 / #350: it sits +# `action_required`, while the same dispatch by an authorized user runs immediately) — which +# would defeat the whole scheduler. A PAT dispatch runs AS the authorized token owner, so the +# run starts immediately with no approval gate (this is the original #349 design; #350's "no +# PAT needed / workflow_dispatch is anti-recursion-exempt" claim was WRONG — see #351). +# AUTOUPDATE_TOKEN is therefore REQUIRED for this scheduler. The dispatched run executes on the +# PR's head branch, so its checks land on the PR head SHA and satisfy branch protection. # # WHEN IT RUNS: # • workflow_run, after "Auto-update PR branches" completes — the race-free moment: autoupdate @@ -40,9 +45,13 @@ on: - cron: "*/15 * * * *" workflow_dispatch: -# Trigger-only; this workflow never gates a merge. `actions: write` lets the built-in -# GITHUB_TOKEN dispatch ci.yml (workflow_dispatch) and cancel strictly-lower runs when a P0 -# emergency preempts. `pull-requests: read` + `contents: read` cover the PR/label enumeration. +# Trigger-only; this workflow never gates a merge. The gh calls run as GH_TOKEN, which is +# normally the AUTOUPDATE_TOKEN PAT (see the step below). These permissions govern the built-in +# GITHUB_TOKEN, used only on the fail-open fallback path when AUTOUPDATE_TOKEN is absent: +# `actions: write` lets it dispatch ci.yml (workflow_dispatch) and cancel strictly-lower runs +# when a P0 emergency preempts; `pull-requests: read` + `contents: read` cover the PR/label +# enumeration. (A GITHUB_TOKEN dispatch needs manual approval, so that fallback only actually +# starts CI if repo settings don't gate GITHUB_TOKEN-triggered runs — the PAT is the real path.) permissions: contents: read pull-requests: read @@ -73,7 +82,13 @@ jobs: # — and even a total failure here leaves ci.yml's `on: pull_request` path intact. - name: Trigger CI for the highest-priority PR(s) needing a run env: - GH_TOKEN: ${{ github.token }} + # AUTOUPDATE_TOKEN (a PAT) is REQUIRED here: a CI run dispatched by the built-in + # GITHUB_TOKEN is held for MANUAL approval (`action_required`) and never runs + # un-attended, so the scheduler must dispatch AS the PAT's authorized owner to start + # runs with no approval gate. `|| github.token` keeps this fail-open when the secret is + # absent, but that GITHUB_TOKEN fallback only actually starts CI if repo settings don't + # gate GITHUB_TOKEN-triggered runs — the PAT is the intended path (see #351). + GH_TOKEN: ${{ secrets.AUTOUPDATE_TOKEN || github.token }} GH_REPO: ${{ github.repository }} # Poor-man's merge-queue width: at most this many PRs run CI concurrently under the # scheduler (a P0 emergency bypasses this cap). Kept conservative because each PR