feat: stop autoscale waking a pool for a run that will never start

.github/workflows/ci.yml

@@ -53,3 +53,5 @@ jobs:
         run: tests/scheduler-agent-path.sh
       - name: pool settling window
         run: tests/pool-settling-window.sh
+      - name: stuck queue guard
+        run: tests/stuck-queue-guard.sh

AGENTS.md

@@ -31,7 +31,7 @@ lib/scheduler.sh     status, doctor, autoscale, sweep, clean, schedule
 lib/notify.sh        the optional notifier hook and what triggers it
 lib/stats.sh         job durations from recorded telemetry, and queue times
                      via contrib/telemetry-join.sh
-tests/               offline test scripts, all four run by CI
+tests/               offline test scripts, every one of them run by CI
 contrib/             optional pieces the user opts into: job hook, webhook notifier,
                      demo status fixture
 skills/runpool/      agent skill for *using* runpool, shipped with the tool
@@ -59,6 +59,16 @@ assets/icon.svg      the icon, source of truth; PNGs are rendered from it
 
 **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 stuck-queue guard subtracts, it does not suppress
+
+**A queued run that never starts would otherwise wake a pool for ever**, every `RUNPOOL_IDLE_SECS` plus a tick, and nothing reports it because a pool that wakes and stands down is behaving as designed. `_rp_autoscale` therefore computes `queued > held` rather than deciding whether the pool is allowed to wake.
+
+- **Never suppress the pool.** An org pool watches many repositories, and one dead run in one of them must not blind it to the rest. Subtracting is what keeps the blast radius to the single run.
+- **A strike is earned only when `state/pools/<name>.started` has changed** since that run was last judged. That is the only available proof that waking for it achieved nothing. A tick count would give three strikes in three minutes to a pool that cannot start at all, and then refuse it real work once its agents were repaired; elapsed time would punish a laptop that slept through the night with work genuinely queued.
+- **`_rp_stuck_advance`, `_rp_stuck_held` and `_rp_stuck_new_holds` are pure**, for the same reason `_rp_unwatched_repos` is: the rule is only testable at all when it is separate from the caller that needs GitHub.
+- **The job count is corroboration, never a branch.** Zero jobs is the signature of the zombie case and it reads well in the message, but making the decision depend on a second API shape buys nothing the local rule does not already get right.
+- **A held run says nothing on later ticks.** Only `_rp_stuck_new_holds` logs or notifies, or the loop this exists to end is replaced by a line a minute.
+
 ## The reconfiguration lock
 
 **One per-pool lock covers resize and drain, and `up` and autoscale both respect it.** It was originally a resize lock; a drain needs the same exclusion for longer, so the concept widened rather than gaining a second flag to get out of step with.
@@ -80,10 +90,7 @@ Environment, then config file, then built-in default. The config file uses plain
 /bin/bash -n bin/runpool lib/*.sh contrib/*.sh tests/*.sh install.sh
 shellcheck --severity=warning bin/runpool lib/*.sh contrib/*.sh tests/*.sh install.sh
 
-tests/storage-migration.sh
-tests/set-count-guards.sh
-tests/watch-list-staleness.sh
-tests/drain-guards.sh
+for t in tests/*.sh; do "$t" || break; done
 ```
 
 Without shellcheck installed, Docker gives the same result and leaves nothing behind. Skipping the check is how CI goes red unnoticed:
@@ -93,7 +100,7 @@ docker run --rm -v "$PWD:/mnt" -w /mnt koalaman/shellcheck:stable \
   --severity=warning bin/runpool lib/*.sh contrib/*.sh tests/*.sh install.sh
 ```
 
-**The tests run offline and make no API calls**, which is what makes them safe anywhere. A new test must keep that: fabricate pool configs under a scratch `RUNPOOL_BASE`, stub `gh` where a path needs it, and prefer a guard refused early over one that reaches the network. `tests/set-count-guards.sh` probes its lock case with a mismatched `--if-count` for that reason, since a real resize fetches the runner tarball first.
+**The tests run offline and make no API calls**, which is what makes them safe anywhere. A new test must keep that: fabricate pool configs under a scratch `RUNPOOL_BASE`, stub `gh` where a path needs it, and prefer a guard refused early over one that reaches the network. `tests/set-count-guards.sh` probes its lock case with a mismatched `--if-count` for that reason, since a real resize fetches the runner tarball first. `tests/stuck-queue-guard.sh` stubs `_rp_up` to move the started stamp by a counter rather than the clock, because the rule it exercises turns on that stamp changing and real seconds would mean sleeping through every cycle.
 
 **Test against a scratch directory, never a real installation:**
 

README.md

@@ -48,7 +48,7 @@ runs-on: ${{ vars.CI_RUNNER || 'ubuntu-latest' }}
 
 - **A pool is a set of runners bound to one GitHub scope.** GitHub offers repository, organisation and enterprise scopes and **no user-account scope**, which is the most surprising thing about self-hosted runners. An organisation shares one pool across its repositories; a personal repository needs its own and cannot borrow an organisation's.
 - **Capacity and routing stay separate.** A workflow's `runs-on` decides where a job lands. RunPool decides only whether the runners are up, so a workflow pointed at a pool that is down waits for it rather than quietly rerouting to a hosted runner that costs ten times as much.
-- **Two launch agents drive everything.** A tick every 60 seconds brings up pools with queued work, stands down idle ones, and checks their registrations are still live. A clean at 04:00 prunes work directories, caches and superseded binaries, skipping any pool mid-job. Only stopped pools are polled, so active work costs no API calls at all.
+- **Two launch agents drive everything.** A tick every 60 seconds brings up pools with queued work, stands down idle ones, and checks their registrations are still live. A run that stays queued across several wake cycles stops counting as work, so a run GitHub will never start cannot wake the pool for ever. A clean at 04:00 prunes work directories, caches and superseded binaries, skipping any pool mid-job. Only stopped pools are polled, so active work costs no API calls at all.
 
 The first job after a quiet spell waits about a minute for its pool to come up. Everything after that is immediate.
 

bin/runpool

@@ -43,7 +43,7 @@ export RUNPOOL_INVOKED
 # The released version, and the only place it is written. The Homebrew formula
 # builds from a git tag, so a tag without a matching bump here ships a binary
 # that misreports itself.
-RUNPOOL_VERSION="0.10.4"
+RUNPOOL_VERSION="0.11.0"
 
 # shellcheck source=lib/common.sh
 . "${RUNPOOL_ROOT}/lib/common.sh"

lib/common.sh

@@ -23,6 +23,7 @@ _rp_env_POOLS_FILE="${RUNPOOL_POOLS_FILE:-}"
 _rp_env_LOG_DIR="${RUNPOOL_LOG_DIR:-}"
 _rp_env_LABEL_NS="${RUNPOOL_LABEL_NS:-}"
 _rp_env_IDLE_SECS="${RUNPOOL_IDLE_SECS:-}"
+_rp_env_STUCK_WAKES="${RUNPOOL_STUCK_WAKES:-}"
 _rp_env_LOAD_WARN="${RUNPOOL_LOAD_WARN:-}"
 _rp_env_NOTIFY_CMD="${RUNPOOL_NOTIFY_CMD:-}"
 _rp_env_JOB_HOOK="${RUNPOOL_JOB_HOOK:-}"
@@ -46,19 +47,21 @@ set +a
 [ -n "${_rp_env_LOG_DIR}" ]     && RUNPOOL_LOG_DIR="${_rp_env_LOG_DIR}"
 [ -n "${_rp_env_LABEL_NS}" ]    && RUNPOOL_LABEL_NS="${_rp_env_LABEL_NS}"
 [ -n "${_rp_env_IDLE_SECS}" ]   && RUNPOOL_IDLE_SECS="${_rp_env_IDLE_SECS}"
+[ -n "${_rp_env_STUCK_WAKES}" ] && RUNPOOL_STUCK_WAKES="${_rp_env_STUCK_WAKES}"
 [ -n "${_rp_env_LOAD_WARN}" ]   && RUNPOOL_LOAD_WARN="${_rp_env_LOAD_WARN}"
 [ -n "${_rp_env_NOTIFY_CMD}" ]  && RUNPOOL_NOTIFY_CMD="${_rp_env_NOTIFY_CMD}"
 [ -n "${_rp_env_JOB_HOOK}" ]    && RUNPOOL_JOB_HOOK="${_rp_env_JOB_HOOK}"
 [ -n "${_rp_env_HOOK_DIR}" ]    && RUNPOOL_HOOK_DIR="${_rp_env_HOOK_DIR}"
 [ -n "${_rp_env_TELEMETRY}" ]   && RUNPOOL_TELEMETRY="${_rp_env_TELEMETRY}"
 [ -n "${_rp_env_DRAIN_TIMEOUT}" ] && RUNPOOL_DRAIN_TIMEOUT="${_rp_env_DRAIN_TIMEOUT}"
 unset _rp_env_BASE _rp_env_CACHE_DIR _rp_env_POOLS_FILE _rp_env_LOG_DIR _rp_env_LABEL_NS \
-      _rp_env_IDLE_SECS _rp_env_LOAD_WARN _rp_env_NOTIFY_CMD _rp_env_JOB_HOOK \
+      _rp_env_IDLE_SECS _rp_env_STUCK_WAKES _rp_env_LOAD_WARN _rp_env_NOTIFY_CMD _rp_env_JOB_HOOK \
       _rp_env_HOOK_DIR _rp_env_TELEMETRY _rp_env_DRAIN_TIMEOUT
 
 # Restored values need exporting again: the restore above is a plain assignment
 # and happens after 'set -a' was turned off.
 export RUNPOOL_BASE RUNPOOL_CACHE_DIR RUNPOOL_LOG_DIR RUNPOOL_LABEL_NS RUNPOOL_IDLE_SECS \
+       RUNPOOL_STUCK_WAKES \
        RUNPOOL_LOAD_WARN RUNPOOL_NOTIFY_CMD RUNPOOL_JOB_HOOK RUNPOOL_HOOK_DIR RUNPOOL_TELEMETRY \
        RUNPOOL_DRAIN_TIMEOUT \
        RUNPOOL_CONFIG RUNPOOL_POOLS_FILE
@@ -135,6 +138,13 @@ RUNPOOL_IDLE_SECS="${RUNPOOL_IDLE_SECS:-1200}"
 # window reports every healthy start as an outage.
 RUNPOOL_SETTLE_SECS="${RUNPOOL_SETTLE_SECS:-120}"
 
+# How many fruitless wake cycles a queued run is given before it stops counting
+# as work. A run can sit `queued` forever with no jobs ever attached, and
+# autoscale counting it wakes the pool, finds nothing, idles out and wakes
+# again for as long as the run exists. Three cycles is roughly an hour of
+# proof at the default idle threshold. Set to 0 to disable the guard.
+RUNPOOL_STUCK_WAKES="${RUNPOOL_STUCK_WAKES:-3}"
+
 # 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
@@ -376,13 +386,45 @@ _rp_pool_paused() { [ -f "$(_rp_pool_pause_flag "$1")" ]; }
 # reason as the activity stamp above.
 _rp_pool_started_flag() { echo "${RUNPOOL_POOL_STATE_DIR}/$1.started"; }
 _rp_touch_pool_started() { _rp_now >| "$(_rp_pool_started_flag "$1")"; }
-_rp_pool_settling() {
+_rp_pool_started_at() {
   local started
   started=$(cat "$(_rp_pool_started_flag "$1")" 2>/dev/null || echo 0)
-  case "${started}" in ''|*[!0-9]*) return 1 ;; esac
+  case "${started}" in ''|*[!0-9]*) started=0 ;; esac
+  echo "${started}"
+}
+_rp_pool_settling() {
+  local started
+  started="$(_rp_pool_started_at "$1")"
+  [ "${started}" != "0" ] || return 1
   [ $(( $(_rp_now) - started )) -lt "${RUNPOOL_SETTLE_SECS}" ]
 }
 
+# Queued runs this pool has stopped counting as work, one record per line:
+#
+#   <owner/repo> <run_id> <strikes> <started_stamp> <strike_at>
+#
+# The started stamp is what the pool's `.started` flag held when that run was
+# last judged, and it is the whole of the rule: a strike is earned only once
+# the pool has come up and back down since, which is the only evidence that
+# waking for this run achieved nothing. A tick count would punish a pool that
+# cannot start at all, and elapsed time would punish a laptop that slept.
+#
+# Rewritten only when it changes, so an idle machine is not writing this file
+# sixty times an hour, and removed outright once nothing is queued.
+_rp_pool_stuck_file() { echo "${RUNPOOL_POOL_STATE_DIR}/$1.stuck"; }
+_rp_read_pool_stuck() { cat "$(_rp_pool_stuck_file "$1")" 2>/dev/null || true; }
+_rp_write_pool_stuck() {
+  local f cur
+  f="$(_rp_pool_stuck_file "$1")"
+  cur="$(cat "${f}" 2>/dev/null || true)"
+  if [ -z "$2" ]; then
+    rm -f "${f}"
+    return 0
+  fi
+  [ "${cur}" = "$2" ] && return 0
+  printf '%s\n' "$2" >| "${f}"
+}
+
 _rp_runner_cache_dir() { echo "${POOL_CACHE_DIR}/runner-$2"; }
 _rp_runner_work_dir() {
   if [ "${POOL_LEGACY_LAYOUT}" = "1" ]; then

lib/lifecycle.sh

@@ -881,7 +881,12 @@ _rp_remove() {
     case "${POOL_DIR}" in "${RUNPOOL_BASE}/"*) rm -rf "${POOL_DIR}" ;; esac
   fi
   [ "${POOL_LEGACY_LAYOUT}" = "1" ] || rm -rf "${POOL_CACHE_DIR}"
+  # Every per-pool state file, not just the pause flag. A pool registered again
+  # under the same name inherits whatever is left here, and inheriting strikes
+  # would mean a brand-new pool refusing to wake.
   rm -f "$(_rp_pool_pause_flag "$1")"
+  rm -f "$(_rp_pool_started_flag "$1")"
+  rm -f "$(_rp_pool_stuck_file "$1")"
   rm -f "$(_rp_pool_conf "$1")"
   _rp_log "pool '$1' removed"
 

lib/notify.sh

@@ -102,3 +102,30 @@ _rp_health_check() {
     esac
   done
 }
+
+# A run autoscale has stopped counting as work, reported once per hold.
+#
+# `warning`, not `critical`: the two criticals above are pools that cannot take
+# work at all, and nobody needs waking for a pool that has just stopped burning
+# a wake cycle every twenty minutes. The key carries the run id rather than
+# only the pool, deliberately breaking the three-segment shape of the others,
+# because the run is the incident: a receiver deduping per pool would swallow
+# the next stuck run in that pool a week later.
+#
+# $1 pool, $2 the new holds as "<repo> <run_id> ..." lines. Fires nothing when
+# there are none, and the caller is what guarantees each run appears once.
+_rp_notify_stuck_queue() {
+  local p="$1" repo id jobs
+  [ -n "${RUNPOOL_NOTIFY_CMD}" ] || return 0
+  [ -n "${2:-}" ] || return 0
+  printf '%s\n' "$2" | while read -r repo id _; do
+    [ -n "${repo}" ] && [ -n "${id}" ] || continue
+    jobs="$(_rp_run_job_count "${repo}" "${id}")"
+    case "${jobs}" in ''|*[!0-9]*) jobs="an unknown number of" ;; esac
+    _rp_notify warning \
+      "Pool '${p}' has stopped waking for a stuck queued run" \
+      "runpool/stuck-queue/${p}/${id}" \
+      "Run ${id} in ${repo} stayed queued across ${RUNPOOL_STUCK_WAKES} wake cycles, so it no longer counts as work. GitHub reports ${jobs} job(s) for it. The pool is healthy; this run will never start." \
+      "\"Pool\":\"${p}\",\"Repository\":\"${repo}\",\"Run\":\"${id}\",\"Jobs\":\"${jobs}\",\"Fix\":\"gh run cancel ${id} --repo ${repo}\""
+  done
+}