Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ assets/icon.svg the icon, source of truth; PNGs are rendered from it

**Two plist keys are load-bearing and neither looks it.** `RUNNER_MANUALLY_TRAP_SIG` is what makes GitHub's `run.sh` finish its current job on SIGTERM instead of dying with it, and `ExitTimeOut` is what stops launchd sending SIGKILL twenty seconds later. Together they are the whole of `--drain`; remove either and the drain silently becomes the job-killing behaviour it exists to replace.

**The consequence is that an agent already loaded is not necessarily an agent that behaves correctly.** A plist rewritten on disk changes nothing until the pool cycles. Anything depending on agent behaviour must therefore read the *loaded* environment with `launchctl print`, not the file `_rp_agent_traps_signals` is the example, and `_rp_drain_pool` refuses per runner on the strength of it. The file on disk is what somebody intended; the loaded environment is what is true.
**The consequence is that an agent already loaded is not necessarily an agent that behaves correctly.** A plist rewritten on disk changes nothing until the pool cycles. Anything depending on agent behaviour must therefore read the *loaded* environment with `launchctl print`, not the file. `_rp_agent_traps_signals` is the example, and `_rp_drain_pool` refuses per runner on the strength of it. The file on disk is what somebody intended; the loaded environment is what is true.

## The reconfiguration lock

Expand All @@ -69,9 +69,9 @@ assets/icon.svg the icon, source of truth; PNGs are rendered from it

## Configuration precedence

Environment, then config file, then built-in default. The config file uses plain assignments, so `lib/common.sh` snapshots any `RUNPOOL_*` already in the environment, sources the config, then restores the snapshot. **A new setting has to be added in four places in that block** the snapshot, the restore, the `unset`, and the `export` — or it silently becomes un-overridable, or leaks a `_rp_env_*` variable, or fails to reach a child process.
Environment, then config file, then built-in default. The config file uses plain assignments, so `lib/common.sh` snapshots any `RUNPOOL_*` already in the environment, sources the config, then restores the snapshot. **A new setting has to be added in four places in that block:** the snapshot, the restore, the `unset`, and the `export`. Otherwise it silently becomes un-overridable, or leaks a `_rp_env_*` variable, or fails to reach a child process.

`RUNPOOL_POOLS_FILE` is deliberately derived from `XDG_CONFIG_HOME` rather than from `RUNPOOL_CONFIG`'s directory, so that pointing the config at `/dev/null` to isolate a test does not also move the pools file somewhere unexpected. **The corollary is that neither `RUNPOOL_CONFIG` nor `RUNPOOL_BASE` isolates it** — it has to be set on its own, or overridden per run with `apply --file`. `/dev/null` is a valid value: `apply` tests for a readable non-directory rather than a regular file, so the isolation idiom means the same thing for both settings.
`RUNPOOL_POOLS_FILE` is deliberately derived from `XDG_CONFIG_HOME` rather than from `RUNPOOL_CONFIG`'s directory, so that pointing the config at `/dev/null` to isolate a test does not also move the pools file somewhere unexpected. **The corollary is that neither `RUNPOOL_CONFIG` nor `RUNPOOL_BASE` isolates it:** it has to be set on its own, or overridden per run with `apply --file`. `/dev/null` is a valid value: `apply` tests for a readable non-directory rather than a regular file, so the isolation idiom means the same thing for both settings.

## Working on it

Expand Down Expand Up @@ -103,7 +103,7 @@ RUNPOOL_BASE=/tmp/rp-test RUNPOOL_CONFIG=/dev/null ./bin/runpool status --json

**`RUNPOOL_BASE` does not move the pools file**, per *Configuration precedence* above. Anything touching `apply` needs `RUNPOOL_POOLS_FILE` or `--file` too, or it reads the real one and plans against real GitHub targets. `RUNPOOL_LOG_DIR` too, or the log lands in a real installation's. `CONTRIBUTING.md` has the full isolated invocation.

**`register`, `set-count`, `reregister` and `remove` talk to the real GitHub API and real runners.** None has a dry-run and there is nowhere sensible to add one, because the work *is* the API call. Verify destructive changes against a throwaway private repository, never while a job is in flight — the tool refuses on purpose, and forcing past that kills live jobs.
**`register`, `set-count`, `reregister` and `remove` talk to the real GitHub API and real runners.** None has a dry-run and there is nowhere sensible to add one, because the work *is* the API call. Verify destructive changes against a throwaway private repository, never while a job is in flight. The tool refuses on purpose, and forcing past that kills live jobs.

**`apply --dry-run` is the one exception, and only for the plan.** It reads the pools file and the pool configs, prints what it would do, and calls nothing, which makes all of `lib/apply.sh` testable with no GitHub account. What it does *not* cover: visibility is checked inside `register`, which a dry run never reaches, so a `+` line is a plan and not a promise.

Expand Down Expand Up @@ -135,7 +135,7 @@ The pattern is already established: configuration precedence was found and fixed

The workflow then re-runs the checks that gate `main`, attaches a source tarball to a GitHub release, and bumps the formula in the tap to point at it. Nothing else is done by hand.

**The tag message is the release notes.** There is no second copy anywhere: no `CHANGELOG.md`, nothing generated from commits, nothing typed into the GitHub UI afterwards. Write them where you cut the tag, and keep them short: a headline and a paragraph or two, plus any action the reader must take on upgrading. A reader wants to know what changed and whether it affects them; anyone who needs more than that can read the code. Left unstated, this drifts the notes were once twenty words and had reached four hundred. **The workflow refuses a lightweight tag** for this reason, and the check is `git cat-file -t` rather than `for-each-ref`, because `for-each-ref` falls through to the *commit* message on a lightweight tag and would publish something plausible and wrong.
**The tag message is the release notes.** There is no second copy anywhere: no `CHANGELOG.md`, nothing generated from commits, nothing typed into the GitHub UI afterwards. Write them where you cut the tag, and keep them short: a headline and a paragraph or two, plus any action the reader must take on upgrading. A reader wants to know what changed and whether it affects them; anyone who needs more than that can read the code. Left unstated, this drifts: the notes were once twenty words and had reached four hundred. **The workflow refuses a lightweight tag** for this reason, and the check is `git cat-file -t` rather than `for-each-ref`, because `for-each-ref` falls through to the *commit* message on a lightweight tag and would publish something plausible and wrong.

**The tag and `RUNPOOL_VERSION` must agree, and the workflow enforces it** before spending a macOS runner. A tag without a matching bump ships a binary that misreports itself, which was previously only a line in this file asking someone to remember.

Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Most changes need runners, which means a GitHub account and a Mac. Two things ma

- **`contrib/demo-status.sh`** answers `status` with invented pools, so anything consuming the JSON can be developed with no runners at all.
- **`RUNPOOL_BASE`** points RunPool at a scratch directory, so you can register throwaway pools without touching a real setup. Environment beats config, deliberately, so a single invocation can be isolated. Set `RUNPOOL_CACHE_DIR` and `RUNPOOL_LOG_DIR` alongside it: otherwise a test writes caches and logs into a real installation's macOS storage roots.
- **`RUNPOOL_POOLS_FILE`, or `apply --file PATH`, as well.** `RUNPOOL_BASE` does **not** move the pools file: that is derived from `XDG_CONFIG_HOME`, deliberately, so that isolating the config does not lose it. Isolate the base and forget this one and `runpool apply` reads your **real** pools file and tries to register every pool in it into the scratch base with real registrations against real GitHub targets.
- **`RUNPOOL_POOLS_FILE`, or `apply --file PATH`, as well.** `RUNPOOL_BASE` does **not** move the pools file: that is derived from `XDG_CONFIG_HOME`, deliberately, so that isolating the config does not lose it. Isolate the base and forget this one and `runpool apply` reads your **real** pools file and tries to register every pool in it into the scratch base, with real registrations against real GitHub targets.
- **`runpool apply --dry-run`** calls nothing and changes nothing, so all of `lib/apply.sh` can be exercised against hand-written pool config files with no GitHub account.

Which makes the whole isolated invocation:
Expand Down
2 changes: 1 addition & 1 deletion bin/runpool
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash
#
# runpool on-demand self-hosted GitHub Actions runner pools for macOS.
# runpool: on-demand self-hosted GitHub Actions runner pools for macOS.
#
# A pool is a set of runners bound to one GitHub scope. Pools come up when jobs
# queue and stand down when nothing has run for a while, so nothing sits in the
Expand Down
4 changes: 2 additions & 2 deletions contrib/job-hook.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env bash
# Optional job hook stamps machine state into every job, and optionally
# Optional job hook: stamps machine state into every job, and optionally
# records it locally so the right runner count can be measured rather than
# guessed.
#
Expand Down Expand Up @@ -66,7 +66,7 @@ echo "[runpool ${phase}] ${line}"
if [ -n "${GITHUB_STEP_SUMMARY:-}" ] && [ -w "${GITHUB_STEP_SUMMARY:-/nonexistent}" ]; then
{
echo ""
echo "**Machine at job ${phase}** — ${line}"
echo "**Machine at job ${phase}:** ${line}"
} >> "${GITHUB_STEP_SUMMARY}" 2>/dev/null || true
fi

Expand Down
2 changes: 1 addition & 1 deletion contrib/notify-webhook.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env bash
# Reference notifier POST whatever runpool reports to a webhook.
# Reference notifier: POST whatever runpool reports to a webhook.
#
# runpool never delivers anything itself. It writes one JSON object to this
# script's stdin and that is the whole contract, so replacing this with a
Expand Down
2 changes: 1 addition & 1 deletion contrib/telemetry-join.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env bash
# telemetry-join.sh one row per job, with everything needed to reason about
# telemetry-join.sh: one row per job, with everything needed to reason about
# runner count. Joins local telemetry to what GitHub knows about the same run.
#
# This deliberately does no analysis. It plumbs, and leaves the thinking to
Expand Down
26 changes: 13 additions & 13 deletions lib/apply.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# shellcheck shell=bash
# apply.sh reconcile the machine's pools to a file describing them.
# apply.sh: reconcile the machine's pools to a file describing them.
#
# `register` creates one pool from one command, so a machine's setup exists
# nowhere except as a sequence somebody ran once. That is fine for one machine
Expand Down Expand Up @@ -61,7 +61,7 @@ _rp_parse_pools_file() (
lineno=$(( lineno + 1 ))

# Comments are stripped per PHYSICAL line, before the trailing '\' is
# looked at. The other order continue first, strip the joined line
# looked at. The other order (continue first, strip the joined line)
# reads a commented line ending in '\' as a continuation, and the '#'
# then eats whatever it was joined to. The shipped template taught exactly
# that: two commented lines, the first ending in '\'. Uncommenting only
Expand Down Expand Up @@ -194,13 +194,13 @@ _rp_parse_pools_file() (
# Refused rather than ignored: silently doing nothing is how a pool ends
# up never waking and nobody knowing why.
[ "${scope}" = "org" ] || {
_rp_err "${file}:${lineno}: --watch applies to org pools only — a repo pool polls ${target} itself"
_rp_err "${file}:${lineno}: --watch applies to org pools only. A repo pool polls ${target} itself"
return 1
}
# Rebuilt from the entries that were actually validated, not stored as
# written. Splitting on commas drops empty entries before the check, so
# ',acme-inc/api' and 'a/b,,c/d' passed validation and were then written
# to POOL_WATCH verbatim approving one string and storing another.
# to POOL_WATCH verbatim, approving one string and storing another.
clean=""
for tok in $(echo "${watch}" | tr ',' ' '); do
_rp_valid_gh_repo "${tok}" || {
Expand Down Expand Up @@ -243,7 +243,7 @@ _rp_apply() {
esac
done

# Present, then not a directory, then readable three messages rather than
# Present, then not a directory, then readable: three messages rather than
# one, because the three are fixed differently.
#
# '-e' and '-r' rather than '-f'. A '-f' test refuses /dev/null, which is the
Expand All @@ -252,22 +252,22 @@ _rp_apply() {
#
# The readability test is the one that matters. The redirect feeding the
# parser was unchecked, so a pools file the user could not read parsed as
# zero pools and returned 0 and every pool actually on the machine was then
# zero pools and returned 0, and every pool actually on the machine was then
# reported as '? not in the file', which is the exact inverse of the truth.
if [ ! -e "${file}" ]; then
_rp_err "no pools file at ${file}"
_rp_err "Write one (runpool.pools.example is a commented template), or pass --file PATH."
return 1
fi
[ ! -d "${file}" ] || { _rp_err "${file} is a directory, not a pools file"; return 1; }
[ -r "${file}" ] || { _rp_err "cannot read ${file} check its permissions"; return 1; }
[ -r "${file}" ] || { _rp_err "cannot read ${file}: check its permissions"; return 1; }

# Parsed in full before anything is touched, so a typo on the last line
# cannot leave the machine half reconciled.
records="$(_rp_parse_pools_file "${file}")" || return 1

echo "plan from ${file}"
[ "${dry}" = "1" ] || echo " (not a dry run changes are being made)"
[ "${dry}" = "1" ] || echo " (not a dry run: changes are being made)"
echo

# ---- pass one: decide everything and print the whole plan ----------------
Expand All @@ -282,7 +282,7 @@ _rp_apply() {
# runner's config.sh and to gh, and anything in the loop body that read stdin
# would eat the rest of the plan.
while IFS='|' read -r name scope target count watch allow <&3; do
# A file that declares no pools is a valid state, not an error but an
# A file that declares no pools is a valid state, not an error, but an
# empty record set still feeds one empty line through the printf below,
# and that reads back as a pool with no name and plans a create for it.
[ -n "${name}" ] || continue
Expand All @@ -308,7 +308,7 @@ _rp_apply() {
if [ "${scope}" != "${POOL_SCOPE}" ] || [ "${target}" != "${POOL_TARGET}" ]; then
n_conflict=$(( n_conflict + 1 )); rc=1
_rp_plan_line "!" "${name}" "${POOL_SCOPE}" "${POOL_TARGET}" \
"registered here, but the file says ${scope} ${target} 'runpool remove ${name}' first"
"registered here, but the file says ${scope} ${target}: 'runpool remove ${name}' first"
continue
fi

Expand Down Expand Up @@ -350,7 +350,7 @@ _rp_apply() {
_rp_load_pool "${p}" || { n_failed=$(( n_failed + 1 )); rc=1; continue; }
n_absent=$(( n_absent + 1 ))
_rp_plan_line "?" "${p}" "${POOL_SCOPE}" "${POOL_TARGET}" \
"not in the file left alone ('runpool remove ${p}' to delete it)"
"not in the file, left alone ('runpool remove ${p}' to delete it)"
done

# ---- pass two: act on it ------------------------------------------------
Expand Down Expand Up @@ -388,8 +388,8 @@ _rp_apply() {

echo
echo " ${n_create} to create, ${n_change} to change, ${n_same} unchanged, ${n_absent} not in the file"
[ "${n_conflict}" -gt 0 ] && echo " ${n_conflict} conflict(s): scope or target differs — see the '!' lines above"
[ "${n_failed}" -gt 0 ] && echo " ${n_failed} failed see the messages above"
[ "${n_conflict}" -gt 0 ] && echo " ${n_conflict} conflict(s): scope or target differs. See the '!' lines above"
[ "${n_failed}" -gt 0 ] && echo " ${n_failed} failed: see the messages above"
if [ "${dry}" = "1" ]; then
echo " dry run: nothing was changed"
# A repository's visibility is checked by `register`, which a dry run never
Expand Down
Loading