diff --git a/AGENTS.md b/AGENTS.md index b81ac11..1d4515a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 @@ -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 @@ -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. @@ -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. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 87a43cb..6167d50 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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: diff --git a/bin/runpool b/bin/runpool index 747df12..226bde5 100755 --- a/bin/runpool +++ b/bin/runpool @@ -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 diff --git a/contrib/job-hook.sh b/contrib/job-hook.sh index 4e32a15..3b0f22c 100755 --- a/contrib/job-hook.sh +++ b/contrib/job-hook.sh @@ -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. # @@ -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 diff --git a/contrib/notify-webhook.sh b/contrib/notify-webhook.sh index 0612bf2..b34be42 100755 --- a/contrib/notify-webhook.sh +++ b/contrib/notify-webhook.sh @@ -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 diff --git a/contrib/telemetry-join.sh b/contrib/telemetry-join.sh index c6cb0aa..9d166fd 100755 --- a/contrib/telemetry-join.sh +++ b/contrib/telemetry-join.sh @@ -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 diff --git a/lib/apply.sh b/lib/apply.sh index 7d6d716..9ef7ded 100644 --- a/lib/apply.sh +++ b/lib/apply.sh @@ -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 @@ -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 @@ -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}" || { @@ -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 @@ -252,7 +252,7 @@ _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}" @@ -260,14 +260,14 @@ _rp_apply() { 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 ---------------- @@ -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 @@ -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 @@ -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 ------------------------------------------------ @@ -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 diff --git a/lib/common.sh b/lib/common.sh index d7d43de..c5f59de 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -1,5 +1,5 @@ # shellcheck shell=bash -# common.sh — constants, configuration, and helpers shared by every part. +# common.sh: constants, configuration, and helpers shared by every part. # # Sourced by bin/runpool. Targets bash 3.2, which is what stock macOS ships, # so nothing here may use associative arrays, mapfile, or ${var^^}. @@ -138,7 +138,7 @@ RUNPOOL_SETTLE_SECS="${RUNPOOL_SETTLE_SECS:-120}" # How long `--drain` waits for running jobs to finish before giving up. # # Derive this from the longest job the pool could serve plus the runner's own -# teardown — not from how long jobs actually take. A workflow capping jobs at +# teardown, not from how long jobs actually take. A workflow capping jobs at # `timeout-minutes: 60` and a drain bounded at exactly 60 minutes race each # other, and the drain loses in the case that matters: a job at 59m50s is # still legitimately running, GitHub has not cut it, and the drain times out @@ -243,7 +243,7 @@ _rp_valid_gh_repo() { # thing: # # '007' passes a digits-only test, is written to POOL_COUNT verbatim, and -# comes back out of _rp_status_json as "count":007 — which Python and Node +# comes back out of _rp_status_json as "count":007, which Python and Node # both reject, taking any wrapper reading that JSON down with it. # # A twenty-digit count also passes, and then `[ "${count}" -ge 1 ]` prints @@ -276,8 +276,8 @@ _rp_count_rule() { echo "a runner count is a whole number from 1 to 9999, writte # pool's value and got planned against it. A pool silently taking on its # neighbour's count, directory or labels is worse than any error. # -# The four fields below are dereferenced without a default all over this tool — -# POOL_COUNT in arithmetic, POOL_DIR as a path prefix — so a config that +# The four fields below are dereferenced without a default all over this tool +# (POOL_COUNT in arithmetic, POOL_DIR as a path prefix), so a config that # survived sourcing but defines none of them is refused here rather than # somewhere further on. # shellcheck disable=SC2034 # every POOL_* here is read by another lib/ fragment @@ -322,9 +322,9 @@ _rp_agent_loaded() { launchctl list "$1" >/dev/null 2>&1; } # The program a loaded agent actually names, read from the plist on disk. # -# PlistBuddy reports its own errors on stdout rather than stderr — a missing +# PlistBuddy reports its own errors on stdout rather than stderr, and a missing # file answers "File Doesn't Exist, Will Create: ..." and a missing key answers -# "Does Not Exist" — so redirecting stderr is not enough to tell an answer from +# "Does Not Exist", so redirecting stderr is not enough to tell an answer from # a complaint. An absolute path is the only thing worth returning. _rp_agent_program() { local plist="${HOME}/Library/LaunchAgents/$1.plist" program @@ -423,7 +423,7 @@ _rp_scope_path() { # # Shared rather than inline because it now has two callers that must agree: # `register` reads it once when a pool is created, and `doctor` reads it on -# every run — the setting can be switched on long after the pool exists. +# every run: the setting can be switched on long after the pool exists. _rp_org_allows_public() { local pub pub=$(gh api "/orgs/$1/actions/runner-groups" \ @@ -547,7 +547,7 @@ _rp_resize_lock() { age=$(( $(_rp_now) - $(stat -f %m "${lock}" 2>/dev/null || echo 0) )) if [ "${age}" -lt "${RUNPOOL_RESIZE_STALE}" ]; then - _rp_err "pool '$1' is already being reconfigured — wait for that to finish, then retry." + _rp_err "pool '$1' is already being reconfigured. Wait for that to finish, then retry." return 1 fi _rp_log "reconfigure: breaking a stale lock on '$1' (${age}s old)" @@ -564,7 +564,7 @@ _rp_resize_lock_touch() { touch "$(_rp_resize_lock_dir "$1")" 2>/dev/null; } # True when some OTHER live process holds the lock. Used by `up` and autoscale, # both of which must leave a pool alone while it is being reconfigured, and -# both of which are also called BY the holder at the end of its own work — so +# both of which are also called BY the holder at the end of its own work, so # testing only for the lock's existence would deadlock a resize against itself. # # A dead holder is not an obstacle: it left the lock behind and the stale break @@ -583,7 +583,7 @@ _rp_deregister_runner() { [ -f "${runner_dir}/.runner" ] || return 0 # never registered id=$(grep -o '"agentId"[[:space:]]*:[[:space:]]*[0-9]*' "${runner_dir}/.runner" 2>/dev/null \ | grep -o '[0-9]*$') - [ -n "${id}" ] || { _rp_err "no agentId in ${runner_dir}/.runner — cannot deregister"; return 1; } + [ -n "${id}" ] || { _rp_err "no agentId in ${runner_dir}/.runner, cannot deregister"; return 1; } url="$(_rp_scope_path "${scope}" "${target}")/actions/runners/${id}" gh api -X DELETE "${url}" >/dev/null 2>&1 || { _rp_err "DEREGISTER FAILED: runner id ${id} is still registered on ${target}. Remove it with: gh api -X DELETE ${url}" @@ -649,7 +649,7 @@ WRAP # run.sh and the job dies with it. # # `launchctl unload` sends SIGTERM and then SIGKILL once ExitTimeOut expires, -# and the default is around twenty seconds — far shorter than any real job, so +# and the default is around twenty seconds, far shorter than any real job, so # the graceful path above would never get to finish. Six hours is generous # enough that launchd never beats a drain to it. Not 0: the man page says zero # means infinity and warns it can stall shutdown forever. diff --git a/lib/lifecycle.sh b/lib/lifecycle.sh index f0d87ce..2e28bcf 100644 --- a/lib/lifecycle.sh +++ b/lib/lifecycle.sh @@ -1,5 +1,5 @@ # shellcheck shell=bash -# lifecycle.sh — creating, resizing, starting, stopping and destroying pools. +# lifecycle.sh: creating, resizing, starting, stopping and destroying pools. # # A pool is a set of runners bound to one GitHub scope. GitHub offers # repository, organisation and enterprise scopes and no user-account scope, so @@ -7,7 +7,7 @@ # repository needs its own. That constraint is why a pool is the unit here. # --------------------------------------------------------------------------- -# register — create and configure a new pool (left stopped) +# register: create and configure a new pool (left stopped) # --------------------------------------------------------------------------- _rp_register() { _rp_require gh || return 1 @@ -50,7 +50,7 @@ _rp_register() { # The same validators lib/apply.sh runs over the pools file. They lived in # lib/common.sh with only that one caller, so a target given on the command # line reached the config file unchecked and 'runpool register x --org a"b' - # wrote POOL_TARGET="a"b" — a config that then fails to source at all, taking + # wrote POOL_TARGET="a"b", a config that then fails to source at all, taking # the pool with it. if [ "${scope}" = "org" ]; then _rp_valid_gh_name "${target}" || { _rp_err "'${target}' is not an organisation name"; return 1; } @@ -68,7 +68,7 @@ _rp_register() { # for one. Refused rather than ignored: silently doing nothing is how a # pool ends up never waking and nobody knowing why. [ "${scope}" = "org" ] || { - _rp_err "--watch applies to org pools only — a repo pool polls ${target} itself" + _rp_err "--watch applies to org pools only. A repo pool polls ${target} itself" return 1 } for tok in $(echo "${watch}" | tr ',' ' '); do @@ -97,7 +97,7 @@ _rp_register() { if [ "${allow_public}" = "1" ]; then _rp_log "WARNING: ${target} is PUBLIC and --allow-public was given. Any fork's pull request can run its own workflow file here, as your user. Require approval for fork pull requests on that repository." else - _rp_err "${target} is PUBLIC — refusing to register self-hosted runners on a public repo." + _rp_err "${target} is PUBLIC, so refusing to register self-hosted runners on a public repo." _rp_err "A pull request from any fork would run its own workflow file here, as your user." _rp_err "If that is genuinely what you want: runpool register ${name} --repo ${target} --allow-public" return 1 @@ -107,7 +107,7 @@ _rp_register() { # Fails closed. An empty answer means the API call failed, not that the # repository is private, and treating those the same skipped the check # exactly when GitHub was being unreliable. - _rp_err "could not determine the visibility of ${target} — refusing." + _rp_err "could not determine the visibility of ${target}, so refusing." _rp_err "Check 'gh auth status' and that the repository exists, then retry." return 1 ;; @@ -154,7 +154,7 @@ _rp_register() { mkdir -p "${cache_dir}/work" "${cache_dir}/pnpm" "${cache_dir}/npm" "${cache_dir}/tmp" [ -f "${runner_dir}/config.sh" ] || tar -xzf "${tarball}" -C "${runner_dir}" || return 1 token="$(_rp_registration_token "${scope}" "${target}")" \ - || { _rp_err "${name} runner-${i}: no registration token — check 'gh auth status', that ${target} exists, and that you have admin on it"; return 1; } + || { _rp_err "${name} runner-${i}: no registration token. Check 'gh auth status', that ${target} exists, and that you have admin on it"; return 1; } runner_name="$(hostname -s)-${name}-${i}" _rp_log "${name} runner-${i}: registering as '${runner_name}'" ( cd "${runner_dir}" && ./config.sh --unattended --replace \ @@ -170,7 +170,7 @@ _rp_register() { # rather than added afterwards by whoever called this. An org pool with no # watch list never autoscales, so a create that succeeds and a write that # follows it leaves a window where a crash produces a pool that works, looks - # healthy, and silently never wakes — with nothing recording why. One write, + # healthy, and silently never wakes, with nothing recording why. One write, # no window. Repo pools get no line at all: they poll their own target. { cat < --repo OWNER/REPO')" + [ "${found}" = "0" ] && echo " (no pools registered: 'runpool register --repo OWNER/REPO')" return 0 } # --------------------------------------------------------------------------- -# doctor — "why is nothing picking this up", in one command +# doctor: "why is nothing picking this up", in one command # --------------------------------------------------------------------------- # Every check here is answerable today from `status`, the log, and a launchctl # invocation nobody thinks to run. The value is having them in one place with a # remedy attached, and one of them is answerable from nothing at all: if the # tick agent is not loaded, no pool autoscales, every job waits for a manual -# `runpool up`, and `status` reports every pool as perfectly healthy — because +# `runpool up`, and `status` reports every pool as perfectly healthy, because # locally they are. # # STRICTLY READ-ONLY, and that is a boundary rather than a preference. A @@ -211,7 +211,7 @@ _rp_doctor_warns=0 # Four markers, all four characters wide so the messages line up: # OK nothing to do -# INFO context, or a check that could not be run — not a judgement +# INFO context, or a check that could not be run, not a judgement # WARN worth knowing, does not fail the command # FAIL something is actually wrong, and the exit status says so # $1 headline, $2 optional remedy on a continuation line. @@ -356,7 +356,7 @@ _rp_doctor() { # every table, exactly like a pool that was never registered. pools=$(( pools + 1 )) if ! _rp_load_pool "${p}" 2>/dev/null; then - _rp_doctor_fail "${p}: $(_rp_pool_conf "${p}") cannot be read, or is missing one of POOL_SCOPE, POOL_TARGET, POOL_COUNT and POOL_DIR — every other command skips this pool silently" \ + _rp_doctor_fail "${p}: $(_rp_pool_conf "${p}") cannot be read, or is missing one of POOL_SCOPE, POOL_TARGET, POOL_COUNT and POOL_DIR. Every other command skips this pool silently" \ "fix: repair the file, or delete it and register the pool again" continue fi @@ -379,7 +379,7 @@ _rp_doctor() { # An org pool with an empty watch list never autoscales. GitHub reports # queued runs per repository and not per organisation, so _rp_autoscale has - # nothing to poll and the pool waits for a manual 'runpool up' forever — + # nothing to poll and the pool waits for a manual 'runpool up' forever, # while looking entirely healthy everywhere else, which is exactly the # failure this command exists for. Local, free, and no API call. [ "${POOL_SCOPE}" = "org" ] && [ -z "${POOL_WATCH:-}" ] && _rp_doctor_fail \ @@ -394,7 +394,7 @@ _rp_doctor() { # **It does not decide which repositories should be watched.** A # repository at this scope may legitimately route every job to a managed # runner, and nothing readable here distinguishes that from one that - # meant to reach the pool — the routing lives in a repository variable + # meant to reach the pool: the routing lives in a repository variable # whose name is a convention of whoever set it up, not of RunPool. So # the operator is told the difference and judges it. # @@ -420,7 +420,7 @@ _rp_doctor() { # "Would a drain work here?" is the question an operator has after every # upgrade, and without this the only way to answer it is to attempt the - # very thing being checked — on a busy pool, with real jobs at stake. + # very thing being checked, on a busy pool, with real jobs at stake. # # Reported rather than failed: a pool whose runners predate the graceful # shutdown setting is running perfectly well and picking up work. Nothing @@ -437,7 +437,7 @@ _rp_doctor() { done if [ -n "${drain_stale}" ]; then _rp_doctor_note "${p}: runner(s) ${drain_stale} started before graceful shutdown was available, so '--drain' will refuse on them. They run and take work normally; only draining is affected." - _rp_doctor_note "${p}: fix: 'runpool rewrite-agents', then let the pool cycle — the idle sweep does this on its own, or 'runpool down ${p}' once it is idle." + _rp_doctor_note "${p}: fix: 'runpool rewrite-agents', then let the pool cycle. The idle sweep does this on its own, or 'runpool down ${p}' once it is idle." fi # The reconfiguration lock now refuses `up` and is skipped by autoscale, so @@ -445,7 +445,7 @@ _rp_doctor() { # that silently stops waking. It clears itself on the next attempt, but # nothing says so until then. if _rp_resize_locked_by_other "${p}"; then - _rp_doctor_note "${p}: being reconfigured right now (resize or drain in progress) — it will not autoscale until that finishes" + _rp_doctor_note "${p}: being reconfigured right now (resize or drain in progress), so it will not autoscale until that finishes" elif [ -d "$(_rp_resize_lock_dir "${p}")" ]; then _rp_doctor_warn \ "${p}: a reconfiguration lock is left over from a resize or drain that did not finish, so the pool will not autoscale" \ @@ -560,7 +560,7 @@ _rp_doctor() { # The organisation runner-group setting. `register` consults it once, when a # pool is created; it can be switched on the day after and nothing would ever - # mention it again. Reported and never re-derived — enumerating an + # mention it again. Reported and never re-derived: enumerating an # organisation's public repositories to work out the same answer is # explicitly not RunPool's job. See SECURITY.md. if [ "${gh_ok}" = "1" ]; then @@ -577,7 +577,7 @@ _rp_doctor() { false) _rp_doctor_ok "${POOL_TARGET}: allows_public_repositories=false on the default runner group" ;; *) - _rp_doctor_note "${POOL_TARGET}: could not read the runner groups (needs admin:org) — check allows_public_repositories in the organisation's Actions settings" ;; + _rp_doctor_note "${POOL_TARGET}: could not read the runner groups (needs admin:org). Check allows_public_repositories in the organisation's Actions settings" ;; esac done fi @@ -620,7 +620,7 @@ _rp_unwatched_repos() { } # --------------------------------------------------------------------------- -# autoscale — bring a pool up when it has queued work +# autoscale: bring a pool up when it has queued work # --------------------------------------------------------------------------- # Only DOWN pools are polled: one that is already up is serving, so during # active work this makes no API calls at all. An org pool polls the repos named @@ -651,14 +651,14 @@ _rp_autoscale() { fi case "${queued}" in ''|*[!0-9]*) queued=0 ;; esac if [ "${queued}" -gt 0 ]; then - _rp_log "autoscale: '${p}' has ${queued} queued job(s) — bringing up" + _rp_log "autoscale: '${p}' has ${queued} queued job(s), bringing up" _rp_up "${p}" fi done } # --------------------------------------------------------------------------- -# sweep — stand pools down once nothing has run for a while +# sweep: stand pools down once nothing has run for a while # --------------------------------------------------------------------------- _rp_sweep() { local p busy_total=0 any_loaded=0 last idle @@ -673,13 +673,13 @@ _rp_sweep() { last=$(cat "${RUNPOOL_ACTIVITY}" 2>/dev/null || echo 0) idle=$(( $(_rp_now) - last )) if [ "${idle}" -ge "${RUNPOOL_IDLE_SECS}" ]; then - _rp_log "sweep: idle ${idle}s, threshold ${RUNPOOL_IDLE_SECS}s — standing all pools down" + _rp_log "sweep: idle ${idle}s, threshold ${RUNPOOL_IDLE_SECS}s, standing all pools down" _rp_down_all --force fi } # --------------------------------------------------------------------------- -# clean — prune accumulated state (skips any pool with a job running) +# clean: prune accumulated state (skips any pool with a job running) # --------------------------------------------------------------------------- # Persistent runners never clean up after themselves, which is the price of # warm caches. Everything here is either regenerable or already collected @@ -693,7 +693,7 @@ _rp_clean() { [ -n "${only}" ] && [ "${only}" != "${p}" ] && continue _rp_load_pool "${p}" || continue if [ "$(_rp_busy_in "${POOL_DIR}")" -gt 0 ]; then - _rp_log "clean: '${p}' has a job running — skipping"; continue + _rp_log "clean: '${p}' has a job running, skipping"; continue fi # Every runner directory that exists, not 1..POOL_COUNT: a pool whose count # was lowered leaves higher-numbered directories behind, and counting to @@ -838,7 +838,7 @@ _rp_clean_if_overdue() { last=$(cat "${RUNPOOL_LAST_CLEAN}" 2>/dev/null || echo 0) age=$(( $(_rp_now) - last )) if [ "${age}" -ge 86400 ]; then - _rp_log "clean: overdue by $(( age / 3600 ))h and pools are idle — running now" + _rp_log "clean: overdue by $(( age / 3600 ))h and pools are idle, running now" _rp_clean "" fi } @@ -873,7 +873,7 @@ _rp_tick() { } # --------------------------------------------------------------------------- -# pause / resume — global kill switch, default resumed +# pause / resume: global kill switch, default resumed # --------------------------------------------------------------------------- _rp_pause() { local name="${1:-}" busy @@ -887,7 +887,7 @@ _rp_pause() { _rp_pool_paused "${name}" && { _rp_log "Pool '${name}' is already paused."; return 0; } busy="$(_rp_busy_in "${POOL_DIR}")" if [ "${busy}" -gt 0 ]; then - _rp_err "'${name}' has ${busy} job(s) running — refusing to pause. Wait for them to finish, then retry." + _rp_err "'${name}' has ${busy} job(s) running, so refusing to pause. Wait for them to finish, then retry." return 1 fi : >| "$(_rp_pool_pause_flag "${name}")" @@ -909,7 +909,7 @@ _rp_resume() { } # --------------------------------------------------------------------------- -# schedule — install or remove the background agents +# schedule: install or remove the background agents # --------------------------------------------------------------------------- _rp_write_cron_plist() { local label="$1" arg="$2" interval_key="$3" interval_val="$4" bin="$5" diff --git a/lib/stats.sh b/lib/stats.sh index 861a812..6b4f204 100644 --- a/lib/stats.sh +++ b/lib/stats.sh @@ -1,5 +1,5 @@ # shellcheck shell=bash -# stats.sh — what recorded jobs actually cost. +# stats.sh: what recorded jobs actually cost. # # Deliberately a description rather than an analysis. # @@ -158,7 +158,7 @@ EOF } # --------------------------------------------------------------------------- -# --queue — the wait before a runner picked the job up +# --queue: the wait before a runner picked the job up # --------------------------------------------------------------------------- # Queue time is the figure that answers "do I need more runners", because more # runners help if and only if work is waiting. The job hook fires when a runner @@ -171,9 +171,9 @@ EOF # `status --json --local` exists to prevent, so the two readouts stay separate. # # contrib/telemetry-join.sh is INVOKED, not reimplemented here. It carries two -# correctness traps that took working out — run id plus runner name matches +# correctness traps that took working out. Run id plus runner name matches # several API jobs and needs the nearest-start tie-break, and created_at is -# run-level rather than job-level — and a second copy is a second place to get +# run-level rather than job-level, and a second copy is a second place to get # them wrong. _rp_stats_queue() { local f="$1"; shift @@ -185,7 +185,7 @@ _rp_stats_queue() { # bin/ and lib/ under libexec, so this resolves for a tap install too. join="${RUNPOOL_ROOT:-}/contrib/telemetry-join.sh" [ -x "${join}" ] || { - _rp_err "cannot run ${join} — contrib/ ships with runpool; check the install, or chmod +x it" + _rp_err "cannot run ${join}: contrib/ ships with runpool; check the install, or chmod +x it" return 1 } @@ -197,7 +197,7 @@ _rp_stats_queue() { # falls back to `runpool status --json --local` and picks up whatever runpool # is on PATH, which is not necessarily the one being run. joined="$("${join}" "${f}" "$@")" || { - _rp_err "the join failed — run '${join} ${f}' directly to see why" + _rp_err "the join failed: run '${join} ${f}' directly to see why" return 1 } @@ -241,7 +241,7 @@ EOF # "median queue 47s" is that blind spot again with a new number on it. echo "" echo "A queue time is not one thing. A wait can be a cold pool waking, a" - echo "dependency that has not finished, or no free runner — and only the last" + echo "dependency that has not finished, or no free runner, and only the last" echo "is fixed by more runners. On an on-demand pool the first job after a" echo "quiet spell always shows about a minute of queue while the pool starts," echo "and no amount of capacity removes it." diff --git a/runpool.conf.example b/runpool.conf.example index 1177aeb..5c8855c 100644 --- a/runpool.conf.example +++ b/runpool.conf.example @@ -1,4 +1,4 @@ -# runpool configuration — copy to ~/.config/runpool/config +# runpool configuration: copy to ~/.config/runpool/config # # Everything here has a working default, so an empty file is a valid config. # This lives outside the repository because it is the only place installation diff --git a/runpool.pools.example b/runpool.pools.example index a15b041..cec6eaf 100644 --- a/runpool.pools.example +++ b/runpool.pools.example @@ -1,4 +1,4 @@ -# runpool pools — copy to ~/.config/runpool/pools +# runpool pools: copy to ~/.config/runpool/pools # # What this machine is meant to have. `runpool apply` reconciles the machine to # it; `runpool apply --dry-run` prints what would change and touches nothing. @@ -28,7 +28,7 @@ # # Blank lines are ignored. A '#' comments out the line it is on and nothing # else. A trailing '\' continues onto the next line, which is what keeps a long -# --watch list readable — and the line it continues onto has to say something, +# --watch list readable, and the line it continues onto has to say something, # so a half-uncommented example is an error naming both lines rather than a # pool quietly missing half of itself. # diff --git a/skills/runpool/SIZING.md b/skills/runpool/SIZING.md index e240456..8afe246 100644 --- a/skills/runpool/SIZING.md +++ b/skills/runpool/SIZING.md @@ -35,7 +35,7 @@ One row per job: duration, queue time, load at start, concurrency, and the raw c **The standard argument is that more runners is not more throughput**, because a single test job commonly forks one worker per core, so several in parallel thrash rather than run faster. Treat that as an argument, not a fact. It may hold on a given machine and it may not, and the difference is measurable. -**If red runs come and go, suspect contention before suspecting the code.** Lower the count with `runpool set-count N --drain` — a contended pool is by definition a busy one, so without `--drain` the resize is refused exactly when you need it — or cap per-job worker counts in the test runner. The job hook in `contrib/` stamps machine load into every job so this is visible rather than guessed at. +**If red runs come and go, suspect contention before suspecting the code.** Lower the count with `runpool set-count N --drain` (a contended pool is by definition a busy one, so without `--drain` the resize is refused exactly when you need it), or cap per-job worker counts in the test runner. The job hook in `contrib/` stamps machine load into every job so this is visible rather than guessed at. ## The ceiling is not in the data diff --git a/skills/runpool/SKILL.md b/skills/runpool/SKILL.md index c1eb4cd..27971d2 100644 --- a/skills/runpool/SKILL.md +++ b/skills/runpool/SKILL.md @@ -11,8 +11,8 @@ On-demand self-hosted GitHub Actions runner pools for macOS. Pools wake when job **Reference files, loaded when the task needs them:** -- **[SIZING.md](SIZING.md)** — how many runners a machine should have, and the four traps that have each produced a confidently wrong answer. -- **[MIGRATION.md](MIGRATION.md)** — moving a legacy installation into macOS storage. +- **[SIZING.md](SIZING.md):** how many runners a machine should have, and the four traps that have each produced a confidently wrong answer. +- **[MIGRATION.md](MIGRATION.md):** moving a legacy installation into macOS storage. ## Before anything else @@ -116,21 +116,21 @@ runpool doctor It works down the whole list below in one pass, prints a remedy against each failure, and **exits non-zero when something is actually wrong**. It repairs nothing, so it is safe at any moment including mid-job, and nothing it finds is fixed until you run the command it names. -- **the tick agent is not loaded** — nothing autoscales, so every job waits for a manual `runpool up` while every pool still reports as perfectly healthy. **This is the one failure no other command surfaces.** Fix: `runpool schedule install`. -- **`gh` is not authenticated** — every API call fails and each caller degrades quietly: `status` reports GitHub as unreachable, and autoscale reads a queued count of zero and never wakes anything. Fix: `gh auth login`. -- **github has no runners registered** — GitHub prunes registrations after a long idle spell. The local install is untouched and looks entirely healthy, which is what makes this confusing. Fix: `runpool reregister `. -- **started locally and none has reached github** — the runners are up and not connecting. Same fix. -- **runpool is paused** — someone hit the kill switch. Fix: `runpool resume`. -- **launch agents missing** — `runpool up` refuses on the first plist it cannot find. Fix: `runpool rewrite-agents`. -- **an org pool with no watched repositories** — it never autoscales. Fix: give it `--watch` and `runpool apply`. -- **an org pool watching only some of its repositories** — a note rather than a failure, because a repository may legitimately route its jobs elsewhere and nothing readable tells the difference. A job queued by an unwatched one waits until a watched one happens to wake the pool. **`doctor` makes a stale list audible; it does not maintain one.** The list stays hand-maintained, so a repository added to the organisation is still a change somebody has to make here too. -- **runner(s) started before graceful shutdown was available** — a note, not a failure. The pool runs and takes work normally; only `--drain` is affected, and it refuses safely rather than killing the job. Fix: `runpool rewrite-agents`, then let the pool cycle. **This is how to check drain readiness without attempting a drain**, which on a busy pool means risking real jobs to answer a question. -- **disk, config permissions, the organisation's runner-group setting** — each with its own remedy. None of these stops a job being picked up, but they are the things nothing else ever looks at. +- **the tick agent is not loaded:** nothing autoscales, so every job waits for a manual `runpool up` while every pool still reports as perfectly healthy. **This is the one failure no other command surfaces.** Fix: `runpool schedule install`. +- **`gh` is not authenticated:** every API call fails and each caller degrades quietly: `status` reports GitHub as unreachable, and autoscale reads a queued count of zero and never wakes anything. Fix: `gh auth login`. +- **github has no runners registered:** GitHub prunes registrations after a long idle spell. The local install is untouched and looks entirely healthy, which is what makes this confusing. Fix: `runpool reregister `. +- **started locally and none has reached github:** the runners are up and not connecting. Same fix. +- **runpool is paused:** someone hit the kill switch. Fix: `runpool resume`. +- **launch agents missing:** `runpool up` refuses on the first plist it cannot find. Fix: `runpool rewrite-agents`. +- **an org pool with no watched repositories:** it never autoscales. Fix: give it `--watch` and `runpool apply`. +- **an org pool watching only some of its repositories:** a note rather than a failure, because a repository may legitimately route its jobs elsewhere and nothing readable tells the difference. A job queued by an unwatched one waits until a watched one happens to wake the pool. **`doctor` makes a stale list audible; it does not maintain one.** The list stays hand-maintained, so a repository added to the organisation is still a change somebody has to make here too. +- **runner(s) started before graceful shutdown was available:** a note, not a failure. The pool runs and takes work normally; only `--drain` is affected, and it refuses safely rather than killing the job. Fix: `runpool rewrite-agents`, then let the pool cycle. **This is how to check drain readiness without attempting a drain**, which on a busy pool means risking real jobs to answer a question. +- **disk, config permissions, the organisation's runner-group setting:** each with its own remedy. None of these stops a job being picked up, but they are the things nothing else ever looks at. **Two situations `doctor` deliberately reports as healthy, because they are.** -- **`running 0/N` with a job genuinely queued** — the tick brings a pool up within about a minute. Wait before intervening; `runpool up ` forces it. -- **A clean report and the job still waits** — the problem is routing, not capacity. The workflow's `runs-on` may not resolve to `self-hosted`, or its labels may not match the pool's. RunPool controls only whether the runners are up and cannot see either. +- **`running 0/N` with a job genuinely queued:** the tick brings a pool up within about a minute. Wait before intervening; `runpool up ` forces it. +- **A clean report and the job still waits:** the problem is routing, not capacity. The workflow's `runs-on` may not resolve to `self-hosted`, or its labels may not match the pool's. RunPool controls only whether the runners are up and cannot see either. ```bash runpool status # the same picture as a table, one row per pool @@ -150,13 +150,13 @@ runpool set-count 2 --drain # let running jobs finish first runpool down --drain # same, without resizing ``` -**The pools file is intent; the running pool is state.** `set-count` changes the pool and does not write the file, so they disagree after any resize. This is by design, not drift to repair: the file is the shape you want and what you copy between machines, the pool is what is running now, and `apply` reconciles them in the file's favour. Always `apply --dry-run` first and read the plan — a `count` line in it is a deliberate resize about to be undone. +**The pools file is intent; the running pool is state.** `set-count` changes the pool and does not write the file, so they disagree after any resize. This is by design, not drift to repair: the file is the shape you want and what you copy between machines, the pool is what is running now, and `apply` reconciles them in the file's favour. Always `apply --dry-run` first and read the plan: a `count` line in it is a deliberate resize about to be undone. **`set-count` writes an absolute number, so anything that read the count earlier must pass `--if-count`.** Between the read and the write the pool may have moved, and a command meant to grow it then shrinks it instead, deregistering runners that setting the number back does not restore. Pass the count you read as `--if-count` and the resize is refused rather than guessed at. One resize per pool runs at a time, so two callers cannot interleave. A deregistration that fails is reported as a failure, not passed over. A registration GitHub still holds for a runner that no longer exists attracts jobs that queue forever, so if a resize reports orphaned runners, clear them before moving on. -**Resizing refuses while a job is running, and `--drain` is the way through.** Stopping a runner mid-job fails that job, so the refusal is right — but on a pool serving work continuously there is never a quiet moment, and the pool that most needs resizing is the busy one. `--drain` stops the runners accepting new work, waits for what is already running to finish, then resizes. +**Resizing refuses while a job is running, and `--drain` is the way through.** Stopping a runner mid-job fails that job, so the refusal is right. But on a pool serving work continuously there is never a quiet moment, and the pool that most needs resizing is the busy one. `--drain` stops the runners accepting new work, waits for what is already running to finish, then resizes. - **It is opt-in, not the default.** A command that silently blocks for an hour is worse than one that refuses, and a blocking command is indistinguishable from a hung one. - **The wait is bounded** by `--timeout`, defaulting to `RUNPOOL_DRAIN_TIMEOUT` (4200s). Set that above the longest `timeout-minutes` any workflow on the pool allows, not above how long jobs actually take: a drain bounded at exactly the job cap can time out on a job GitHub was still happily running. diff --git a/tests/watch-list-staleness.sh b/tests/watch-list-staleness.sh index 723986c..a0645eb 100755 --- a/tests/watch-list-staleness.sh +++ b/tests/watch-list-staleness.sh @@ -63,7 +63,7 @@ check "spaces around entries do not matter" \ # A substring is not a match, and the direction matters. Without the comma # fencing, a watch list naming `acme/one-more` would satisfy `acme/one`, -# because the shorter name occurs inside the longer one — so a genuinely +# because the shorter name occurs inside the longer one, so a genuinely # unwatched repository would be reported as watched. That is the direction # that fails silently: the check would say the list is complete when it is # not, which is exactly the state it exists to catch.