fix: two inert defects, before something copies them

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.12.2"
+RUNPOOL_VERSION="0.12.3"
 
 # shellcheck source=lib/common.sh
 . "${RUNPOOL_ROOT}/lib/common.sh"

lib/common.sh

@@ -24,6 +24,7 @@ _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_SETTLE_SECS="${RUNPOOL_SETTLE_SECS:-}"
 _rp_env_LOAD_WARN="${RUNPOOL_LOAD_WARN:-}"
 _rp_env_NOTIFY_CMD="${RUNPOOL_NOTIFY_CMD:-}"
 _rp_env_JOB_HOOK="${RUNPOOL_JOB_HOOK:-}"
@@ -48,20 +49,21 @@ set +a
 [ -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_SETTLE_SECS}" ] && RUNPOOL_SETTLE_SECS="${_rp_env_SETTLE_SECS}"
 [ -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_STUCK_WAKES _rp_env_LOAD_WARN _rp_env_NOTIFY_CMD _rp_env_JOB_HOOK \
+      _rp_env_IDLE_SECS _rp_env_STUCK_WAKES _rp_env_SETTLE_SECS _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_STUCK_WAKES RUNPOOL_SETTLE_SECS \
        RUNPOOL_LOAD_WARN RUNPOOL_NOTIFY_CMD RUNPOOL_JOB_HOOK RUNPOOL_HOOK_DIR RUNPOOL_TELEMETRY \
        RUNPOOL_DRAIN_TIMEOUT \
        RUNPOOL_CONFIG RUNPOOL_POOLS_FILE
@@ -136,7 +138,7 @@ RUNPOOL_IDLE_SECS="${RUNPOOL_IDLE_SECS:-1200}"
 # launchd starts it, and until that lands the pool looks exactly like a broken
 # one: agents up locally, nothing online at GitHub. Judging a pool inside that
 # window reports every healthy start as an outage.
-RUNPOOL_SETTLE_SECS="${RUNPOOL_SETTLE_SECS:-120}"
+RUNPOOL_SETTLE_SECS="${RUNPOOL_SETTLE_SECS:-120}"   # and in the precedence block above
 
 # 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

lib/notify.sh

@@ -81,6 +81,11 @@ _rp_health_check() {
   echo "${now}" >| "${RUNPOOL_HEALTH_STATE}"
   for p in $(_rp_pool_names); do
     _rp_load_pool "${p}" || continue
+    # A pool mid-resize, mid-drain or mid-rename has its registrations being
+    # rewritten, so a check landing in that window can see none and call a
+    # deliberate operation an outage. Whoever holds the lock already knows.
+    # The same predicate autoscale and `up` use.
+    _rp_resize_locked_by_other "${p}" && continue
     gh="$(_rp_gh_runners)"; reg="${gh% *}"; online="${gh#* }"
     running="$(_rp_running_in "${p}" "${POOL_COUNT}")"
     settling=0; _rp_pool_settling "${p}" && settling=1

tests/pool-settling-window.sh

@@ -90,4 +90,75 @@ echo "not-a-number" >| "$(_rp_pool_started_flag acme)"
 _rp_pool_settling acme && fail "an unreadable stamp is not settling"
 pass=$(( pass + 1 ))
 
+# --- a pool being reconfigured is not judged --------------------------------
+# set-count, reregister and rename all delete registrations and create them
+# again. A check landing in that window can see none and report a deliberate
+# operation as an outage, at critical, to whoever holds the lock and already
+# knows. Asserted by call count, because the point is that GitHub is not asked
+# at all rather than asked and forgiven.
+mkdir -p "${RUNPOOL_BASE}/pools" "${scratch_dir}/bin"
+cat >"${RUNPOOL_BASE}/pools/alpha.conf" <<CONF
+POOL_SCOPE="repo"
+POOL_TARGET="acme/widget"
+POOL_COUNT="1"
+POOL_DIR="${RUNPOOL_BASE}/runners/alpha"
+POOL_CACHE_DIR="${RUNPOOL_CACHE_DIR}/pools/alpha"
+CONF
+cat >"${scratch_dir}/bin/gh" <<STUB
+#!/bin/bash
+echo call >> "${scratch_dir}/gh-calls"
+echo "0 0"
+STUB
+chmod +x "${scratch_dir}/bin/gh"
+export PATH="${scratch_dir}/bin:${PATH}"
+gh_calls() { awk 'NF { n++ } END { print n + 0 }' "${scratch_dir}/gh-calls" 2>/dev/null || echo 0; }
+
+# shellcheck source=/dev/null
+. "${repo_dir}/lib/notify.sh"
+: >"${scratch_dir}/gh-calls"
+
+# A lock held by a live process that is not us, which is what
+# _rp_resize_locked_by_other tests for. It has to be a real running pid: $$
+# reads as our own lock and is deliberately ignored, and a pid we cannot
+# signal reads as a dead holder, which is equally deliberate.
+sleep 60 &
+holder=$!
+mkdir -p "$(_rp_resize_lock_dir alpha)"
+echo "${holder}" >"$(_rp_resize_lock_dir alpha)/pid"
+rm -f "${RUNPOOL_HEALTH_STATE}"
+_rp_health_check >/dev/null 2>&1
+check "a pool mid-reconfiguration is not asked about" "0" "$(gh_calls)"
+
+kill "${holder}" 2>/dev/null; wait "${holder}" 2>/dev/null
+rm -rf "$(_rp_resize_lock_dir alpha)"
+rm -f "${RUNPOOL_HEALTH_STATE}"
+_rp_health_check >/dev/null 2>&1
+[ "$(gh_calls)" -gt 0 ] || fail "an unlocked pool should still be judged"
+pass=$(( pass + 1 ))
+
+# --- where the window's own setting comes from ------------------------------
+# Precedence is environment, then config file, then default, and every setting
+# has to be threaded through the snapshot-source-restore block to get it. This
+# one was defined outside that block for a long time, which inverted it: a
+# value in the config beat one in the environment, the opposite of every other
+# setting and of what the documentation says.
+#
+# Each case is a subshell that sources the library fresh, because precedence is
+# decided once at load time.
+setting_in() {
+  ( export RUNPOOL_BASE RUNPOOL_CACHE_DIR RUNPOOL_POOLS_FILE RUNPOOL_LOG_DIR RUNPOOL_LOG
+    export RUNPOOL_CONFIG="$1"
+    if [ -n "${2:-}" ]; then export RUNPOOL_SETTLE_SECS="$2"; else unset RUNPOOL_SETTLE_SECS; fi
+    # shellcheck source=/dev/null
+    . "${repo_dir}/lib/common.sh" 2>/dev/null || true
+    echo "${RUNPOOL_SETTLE_SECS}" )
+}
+conf="${scratch_dir}/precedence.conf"
+printf 'RUNPOOL_SETTLE_SECS=222\n' >"${conf}"
+
+check "the default applies with neither set"     "120" "$(setting_in /dev/null "")"
+check "the config file beats the default"        "222" "$(setting_in "${conf}" "")"
+check "the environment beats the config file"    "333" "$(setting_in "${conf}" 333)"
+check "and the environment beats the default"    "333" "$(setting_in /dev/null 333)"
+
 echo "ok: ${pass} case(s)"