diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40a67a8..b06eff7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,24 @@ name: CI # There is no minutes argument for either. The job runs in ~15s and bills as one # rounded-up minute whether it does work or not, and this is a public repo where # hosted runners are free. Skipping saves nothing and risks everything. +# +# `runs-on` below is a THIRD route into the same "waiting for status" trap, and +# the only one an adopter can trigger from repository settings rather than from +# this file. Measured, not assumed: with the variable set to the tempting wrong +# value `ubuntu-latest` (a bare string instead of a JSON array), fromJSON fails, +# the run completes with conclusion=failure and the message "This run likely +# failed because of a workflow file issue", ZERO jobs are created, and ZERO +# check runs are attached to the commit. The `ci` check is therefore never +# reported and the PR cannot merge. Recovery, which is not obvious from the +# symptom: +# +# gh variable delete RUNNER_LABELS +# +# Self-hosted runners are NOT safe here while this repository is public: this +# workflow runs on pull_request, which includes fork PRs, and it checks out and +# executes the PR's own Makefile and scripts. See docs/setup/runners.md. +# +# See docs/setup/runners.md before setting it. on: pull_request: @@ -33,7 +51,7 @@ concurrency: jobs: ci: - runs-on: ubuntu-latest + runs-on: ${{ fromJSON(vars.RUNNER_LABELS || '["ubuntu-latest"]') }} timeout-minutes: 15 steps: - name: Checkout diff --git a/.github/workflows/issue-labeler.yml b/.github/workflows/issue-labeler.yml index e11db28..ad9e559 100644 --- a/.github/workflows/issue-labeler.yml +++ b/.github/workflows/issue-labeler.yml @@ -22,7 +22,10 @@ concurrency: jobs: label: - runs-on: ubuntu-latest + # Runner selection: the RUNNER_LABELS repo variable, a JSON ARRAY of labels. + # Unset = ubuntu-latest. A malformed value stops this job reporting any + # check at all -- see docs/setup/runners.md and ci.yml's header. + runs-on: ${{ fromJSON(vars.RUNNER_LABELS || '["ubuntu-latest"]') }} timeout-minutes: 5 permissions: issues: write diff --git a/.github/workflows/maintenance.yml b/.github/workflows/maintenance.yml index 82a6d00..c553f68 100644 --- a/.github/workflows/maintenance.yml +++ b/.github/workflows/maintenance.yml @@ -19,7 +19,10 @@ concurrency: jobs: link-check: - runs-on: ubuntu-latest + # Runner selection: the RUNNER_LABELS repo variable, a JSON ARRAY of labels. + # Unset = ubuntu-latest. A malformed value stops this job reporting any + # check at all -- see docs/setup/runners.md and ci.yml's header. + runs-on: ${{ fromJSON(vars.RUNNER_LABELS || '["ubuntu-latest"]') }} timeout-minutes: 10 steps: - name: Checkout diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index a1f6ae7..58e4759 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -26,7 +26,10 @@ concurrency: jobs: release-please: - runs-on: ubuntu-latest + # Runner selection: the RUNNER_LABELS repo variable, a JSON ARRAY of labels. + # Unset = ubuntu-latest. A malformed value stops this job reporting any + # check at all -- see docs/setup/runners.md and ci.yml's header. + runs-on: ${{ fromJSON(vars.RUNNER_LABELS || '["ubuntu-latest"]') }} timeout-minutes: 10 permissions: contents: write diff --git a/AGENTS.md b/AGENTS.md index 768461c..5c82357 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,7 @@ Agent-specific entry files (`CLAUDE.md`, `GEMINI.md`, `.github/copilot-instructi ## Build and validation -The Makefile is the only executable contract in this repository. CI calls make targets; customize the Makefile, never the workflows. +The Makefile is the only executable contract in this repository. CI calls make targets; customize the Makefile, never the workflows. The single exception is runner selection, which GitHub resolves before any make target exists to be called: set the `RUNNER_LABELS` repository variable instead of editing `runs-on` (see `docs/setup/runners.md`). | Level | Name | Command | When required | | --- | --- | --- | --- | diff --git a/docs/adr/ADR-0005-runner-selection-variable.md b/docs/adr/ADR-0005-runner-selection-variable.md new file mode 100644 index 0000000..c45db09 --- /dev/null +++ b/docs/adr/ADR-0005-runner-selection-variable.md @@ -0,0 +1,39 @@ +# ADR-0005: Runner selection is an adopter variable, not a workflow edit + +- **Status**: Accepted +- **Date**: 2026-09-02 + +## Context + +All four workflows hardcoded `runs-on: ubuntu-latest`. An adopter needing a different runner — self-hosted, larger, or on their own hardware for compliance — had to edit the workflow YAML. + +That contradicts the template's own contract twice. `AGENTS.md` says "customize the Makefile, never the workflows", and `docs/template/design-principles.md` promises "workflow YAML stays untouched and upgradable". But `runs-on` is resolved by GitHub when it schedules the job, before any make target exists to be called, so it is the one adopter-facing property the Makefile physically cannot own. The rule was unsatisfiable for it, and following the rule was impossible. + +It also degrades upgrades: `docs/template/upgrading.md` recommends bulk cherry-picking `.github/workflows/` precisely because it is rarely customized locally. A forked `runs-on` line makes every template update a manual merge. + +## Decision + +All four workflows resolve their runner from a single repository variable, defaulting to today's behaviour: + +```yaml +runs-on: ${{ fromJSON(vars.RUNNER_LABELS || '["ubuntu-latest"]') }} +``` + +1. **One variable, not one per workflow.** Design principle 1 is "one good default beats three options". Three of the four jobs are trivial and identical in shape, so there is no plausible case for a bigger runner on the issue labeler but not on CI, and every extra name is another thing to typo. Splitting one variable into several later is easy; merging four after adopters have set them is not. +2. **A JSON array, via `fromJSON`.** Multi-label selection (`["self-hosted","linux","x64"]`) is the main reason self-hosted users need this at all. +3. **`AGENTS.md` is amended to name the exception**, rather than leaving it as folklore. An undocumented sanctioned exception is how a contract rots. `skills/github-actions-hygiene/SKILL.md` rule 1 is amended to match, or the skill would contradict the shipped workflows. +4. **The failure mode is documented at the point of risk**, in `ci.yml`'s header and in `docs/setup/runners.md`, with the recovery command. + +## Consequences + +- Adopters change runners from repository settings, and `.github/workflows/` stays byte-identical to upstream and cherry-pickable. +- **A new route into the "unmergeable repository" failure.** Measured with a throwaway probe workflow rather than assumed: with the variable set to the bare string `ubuntu-latest` instead of a JSON array, `fromJSON` fails during scheduling, the run completes with conclusion `failure` and the message "This run likely failed because of a workflow file issue", **zero jobs are created, and zero check runs are attached to the commit**. Applied to `ci.yml` — this repository's only required status check — the check is never reported, the pull request parks on "Expected — waiting for status", and the Actions tab never names the variable. Recovery is `gh variable delete RUNNER_LABELS`. +- The variable is bounded to linux x86_64 by `scripts/install-ci-tools.sh`, whose tarball tools ship no other assets. That constraint now has to be documented, because the failure surfaces as a `uname` message rather than a configuration error. +- Self-hosted runners acquire caveats that did not previously need writing down: the npm and pip installs escape `INSTALL_DIR` and run globally on the runner host every job. + +## Alternatives considered + +- **Four variables**, one per workflow, as the original field report proposed. Rejected per decision 1. +- **`${{ vars.RUNNER_LABELS || 'ubuntu-latest' }}` — a plain string, no `fromJSON`.** This removes the malformed-JSON footgun entirely, which is the single largest cost of this change. Rejected because it cannot express multi-label self-hosted selection, which is the primary use case. This is the closest call in this ADR; if the footgun proves worse in practice than the multi-label capability is worth, switching is a one-line change. +- **Leave `runs-on` hardcoded and accept that adopters fork the YAML.** Rejected: it makes two documented promises false, and the fork is permanent and invisible, whereas a misconfigured variable is transient and recoverable. +- **A `.github/actionlint.yaml` with declared self-hosted labels.** Only needed if `runs-on` held a hardcoded custom label; an expression bypasses actionlint's `runner-label` check entirely (verified against the pinned actionlint 1.7.12). Not needed. diff --git a/docs/adr/README.md b/docs/adr/README.md index 3ec458c..908183d 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -26,3 +26,4 @@ Routine choices (a library patch bump, a wording tweak) do not get ADRs. When in | [ADR-0002](ADR-0002-release-flow.md) | Release flow: release-please with human-gated release PRs | Accepted | | [ADR-0003](ADR-0003-metadata-single-home.md) | Metadata single-home policy | Accepted | | [ADR-0004](ADR-0004-adopter-licence-choice.md) | The adopter's licence is an explicit bootstrap decision | Accepted | +| [ADR-0005](ADR-0005-runner-selection-variable.md) | Runner selection is an adopter variable, not a workflow edit | Accepted | diff --git a/docs/setup/runners.md b/docs/setup/runners.md new file mode 100644 index 0000000..61144ca --- /dev/null +++ b/docs/setup/runners.md @@ -0,0 +1,152 @@ +# Runner selection + +Every workflow in this template resolves its runner from one repository +variable: + +```yaml +runs-on: ${{ fromJSON(vars.RUNNER_LABELS || '["ubuntu-latest"]') }} +``` + +Leave `RUNNER_LABELS` unset and behaviour is identical to hardcoding +`ubuntu-latest`. Set it once and all four workflows move together. + +This is the single sanctioned exception to "customize the Makefile, never the +workflows" (`AGENTS.md`). It has to be an exception because GitHub resolves +`runs-on` when it schedules the job — before any `make` target exists to be +called — so it is the one adopter-facing knob the Makefile cannot own. + +## Setting it + +Before setting it to a self-hosted runner, read the security note below — on a +public repository that combination lets anyone who opens a pull request run code +on your machine. + +The value is a **JSON array**, not a bare string: + +```bash +gh variable set RUNNER_LABELS --body '["ubuntu-latest-4-cores"]' +gh variable set RUNNER_LABELS --body '["self-hosted","linux","x64"]' +gh variable delete RUNNER_LABELS # back to the default +``` + +## Get this wrong and the repository stops being mergeable + +The tempting mistake is the bare string: + +```bash +gh variable set RUNNER_LABELS --body 'ubuntu-latest' # WRONG - not an array +``` + +Measured on this repository with a throwaway probe workflow, not assumed: + +```text +workflow run : completed, conclusion = failure +run message : "This run likely failed because of a workflow file issue." +jobs created : 0 +check runs : 0 +``` + +`fromJSON` fails while the job is being scheduled, so **no job and no check run +are ever created.** For the `ci` workflow that is the worst case available: `ci` +is this repository's only required status check, so the pull request sits on +"Expected — waiting for status to be reported" forever, cannot merge, and the +only evidence is a failed run in the Actions tab whose message never mentions +the variable. + +A valid-but-unknown label fails the same way from the other direction: the job +queues for a runner that never appears, for up to 24 hours. + +**Recovery, which is not obvious from the symptom:** + +```bash +gh variable delete RUNNER_LABELS +``` + +Then push any commit to re-trigger. Because of this, change the variable and +immediately open a throwaway pull request to confirm `ci` still reports, before +you rely on it. + +## Hard constraint: linux x86_64 only + +`scripts/install-ci-tools.sh` installs actionlint, gitleaks and lychee as +checksum-verified `linux_amd64` tarballs, and `require_supported_platform` +hard-fails on anything else. Point `RUNNER_LABELS` at `macos-latest` or an arm64 +runner and `make ci-tools` fails at runtime with a message about `uname -m`, +which reads like a broken script rather than a misconfigured variable. + +Supporting other architectures means adding per-tool asset names to that script, +not relaxing the guard. + +## Self-hosted runners: do not use them on a public repository + +**This is a security boundary, not a preference.** `.github/workflows/ci.yml` +triggers on every `pull_request`. On a **public** repository that includes pull +requests from forks, and the job checks out the pull request's own tree and then +runs `make ci-tools` and `make ci-pr` from it. Anyone on the internet who opens +a pull request therefore executes their own `Makefile` and their own +`scripts/` on your machine — with your filesystem, your network position, and +any credentials reachable from that host. Ephemeral cleanup does not help: the +damage happens while the job is running. + +GitHub's default "require approval for first-time contributors" narrows the +window; it does not close it, because approval is per-contributor, not +per-diff, and a returning contributor's next pull request runs unreviewed. + +So: **on a public repository, leave `RUNNER_LABELS` unset.** If you genuinely +need self-hosted CI on public code, the only safe shapes are to make the +repository private, or to split the workflow so that fork pull requests stay on +GitHub-hosted runners and self-hosted runners only ever run on `push` to +branches you control. That second option is a real workflow change, not a +variable. + +On a **private** repository, where every contributor already has write access, +the rest of this section applies. + +### Operational caveats, private repositories + +Even a *compatible* self-hosted Linux x86_64 runner behaves differently from a +hosted one, because a hosted runner is destroyed after every job and yours is +not: + +- `install_npm_global` and `install_python_tool` install **globally**, escaping + `INSTALL_DIR` entirely. +- `require_install_consent` waves those through whenever `CI` is set, and GitHub + Actions always sets it. +- `place_binary` uses `sudo` when `/usr/local/bin` is not writable, so the + runner account needs passwordless sudo. + +In practice: every run performs a real global `npm install -g` and +`pip install --user` on the runner host. That is fine on a throwaway container +and a slow accumulating mess on a long-lived VM. Prefer an ephemeral +self-hosted runner, or pre-install the five pinned tools into the image and +accept that `make ci-tools` will reinstall them anyway. + +## Sizing: do not downsize to save money + +The instinct to move CI onto a smaller, cheaper runner is usually wrong for a +repository shaped like this one, and `ci.yml`'s header already explains why for +the skip-CI case. The same arithmetic applies here: + +- On a **public** repository, GitHub-hosted runners are free. A smaller runner + saves exactly nothing. +- On a **private** repository, Actions minutes are billed **rounded up to the + whole minute**. This template's lint job finishes in well under a minute, so + it already bills the one-minute floor. A smaller, slower runner cannot go + below that floor — it can only push the job over it and start billing two. +- Minimal images reinstall at runtime what a standard image preinstalls. + Measured by an adopter on a 1 vCPU runner: the same lint job took 32 seconds + to 2 minutes 7 seconds, against 21 to 45 seconds on `ubuntu-latest` — mostly + spent installing markdownlint-cli2 via npm and yamllint via pip on one core. + +Downsize only when a job **materially exceeds a minute** *and* you are past your +plan's included minutes. For a lint-shaped job neither is usually true. + +Upsizing is the more common real need: a big test suite, or a compliance +requirement that builds run on your own hardware. That is what this variable is +for. + +## See also + +- `docs/setup/bootstrap.md` — the rest of the GitHub-side configuration +- `` `skills/github-actions-hygiene/SKILL.md` `` — why workflows stay thin +- `docs/adr/ADR-0005-runner-selection-variable.md` — why this is a variable diff --git a/docs/template/architecture.md b/docs/template/architecture.md index 07528d1..40cf28d 100644 --- a/docs/template/architecture.md +++ b/docs/template/architecture.md @@ -15,7 +15,7 @@ Why each piece of this repository exists, and what it costs to keep. A component | `.github/labels.yml` | Labels as code; bootstrap re-run = sync | Edit alongside label changes; allowlist in `issue-labeler.yml` must match | | `.github/ISSUE_TEMPLATE/` (3 forms) | Set native types; feed the labeler | Sync option lists with `labels.yml` | | `.github/PULL_REQUEST_TEMPLATE.md` | Validation ladder + RISK convention at point of use | Near zero | -| `.github/workflows/ci.yml` | L0 gate; installs tools via `make ci-tools`, runs `make ci-pr`. The only required status check — see its header before touching `on:` | SHA-pin bumps via Dependabot | +| `.github/workflows/ci.yml` | L0 gate; installs tools via `make ci-tools`, runs `make ci-pr`. The only required status check — see its header before touching `on:` or `runs-on:` | SHA-pin bumps via Dependabot | | `.github/workflows/issue-labeler.yml` | Form selections → labels (single-home preserving) | Allowlist sync with `labels.yml` | | `.github/workflows/maintenance.yml` | Weekly drift detectors: external link check + CI tool version check | Near zero | | `.github/workflows/release-please.yml` + configs | Human-gated release automation (ADR-0002) | Action SHA bumps; `release-as` removed after first release | @@ -24,10 +24,10 @@ Why each piece of this repository exists, and what it costs to keep. A component | `Makefile` | The only executable contract; adopter customization point | Grows with adopter stack, not with the template | | `scripts/bootstrap.sh` | Applies everything a template can't ship as files; idempotent sync | Highest-cost component — E2E-verified each release (below) | | `scripts/check-*.sh` | Self-consistency: skills index, local-md hygiene, licence marker | Near zero | -| `scripts/install-ci-tools.sh` | Checksum-verified CI tool installs; single home for all five tool version pins, shared by `ci.yml` and `maintenance.yml` via `make ci-tools` | Hand-bump a pin when the drift check flags it | +| `scripts/install-ci-tools.sh` | Checksum-verified CI tool installs; single home for all five tool version pins, shared by `ci.yml` and `maintenance.yml` via `make ci-tools`; bounds `RUNNER_LABELS` to linux x86_64 | Hand-bump a pin when the drift check flags it | | `scripts/check-tool-versions.sh` | Diffs those pins against upstream weekly and fails on drift — Dependabot cannot see them, so nothing else would | Near zero; add a row when a tool is added | | `docs/adr/` | Decision records; the "why" layer | Grows slowly by trigger criteria | -| `docs/setup/` | Bootstrap + licensing reference and manual fallback | Update alongside `bootstrap.sh` | +| `docs/setup/` | Bootstrap, licensing and runner-selection reference, plus manual fallbacks | Update alongside `bootstrap.sh` and the workflows | | `docs/template/` | Template-product meta (this dir); deleted on adoption | Only exists upstream | ## Release exit checklist (template releases) diff --git a/scripts/install-ci-tools.sh b/scripts/install-ci-tools.sh index a82b346..7e6895e 100755 --- a/scripts/install-ci-tools.sh +++ b/scripts/install-ci-tools.sh @@ -50,8 +50,11 @@ MARKDOWNLINT_CLI2_VERSION="0.23.2" YAMLLINT_VERSION="1.38.0" # --- preflight -------------------------------------------------------------- -# The three tarball tools ship linux x86_64 assets only, which is correct for -# the ubuntu-latest hosted runners this repo uses. On any other architecture the +# The three tarball tools ship linux x86_64 assets only, which is correct for the +# default ubuntu-latest runner. This is what bounds the RUNNER_LABELS repository +# variable (docs/setup/runners.md) to a linux x86_64 runner: point it at macOS or +# arm64 and the failure surfaces here, as a uname message, not as a config error. +# On any other architecture the # download would succeed and then fail at exec time with a confusing "exec # format error", so fail loudly and early instead. Supporting arm64 means adding # per-tool asset names here, not relaxing this check. diff --git a/skills/github-actions-hygiene/SKILL.md b/skills/github-actions-hygiene/SKILL.md index b274986..400d7d6 100644 --- a/skills/github-actions-hygiene/SKILL.md +++ b/skills/github-actions-hygiene/SKILL.md @@ -12,7 +12,7 @@ Workflows are the highest-privilege, least-reviewed code in most repositories ## Rules -1. **Workflows call `make` targets; they never contain logic.** All branching, tool installation flags, and conditionals belong in the `Makefile`. A workflow step should read as `run: make lint`, not a shell script with real decisions in it. Adopters customize the Makefile — never the workflow YAML — to change what runs. +1. **Workflows call `make` targets; they never contain logic.** All branching, tool installation flags, and conditionals belong in the `Makefile`. A workflow step should read as `run: make lint`, not a shell script with real decisions in it. Adopters customize the Makefile — never the workflow YAML — to change what runs. The one thing the Makefile cannot own is `runs-on`, which GitHub resolves before any make target exists; expose it as a repository variable (`RUNNER_LABELS` here) so *where* a job runs stays configurable without forking the file. 2. **Least privilege by default.** Set `permissions: contents: read` at the workflow (top) level; escalate only inside the specific job that needs more, and only to the exact scope needed (e.g. `pull-requests: write` on a labeler job, not on the whole workflow). 3. **Pin every third-party action to a full commit SHA**, with the human-readable version as a trailing comment — never a floating tag like `@v4`: