fleetd #492 follow-up: detect_supervisor must never read "could not tell" as "none"

Two situations were silently landing in the "none" answer, which
require_drivable_supervisor accepts and the script then falls back to a raw
kill + nohup — exactly the wrong move when a supervisor actually IS present:

- installed-but-not-loaded, on either supervisor. `systemctl --user is-active`
  answers "no" for activating/deactivating/failed and while an auto-restart is
  pending too, and every one of those is a host that IS under systemd (or
  launchd) and about to act again. `*_installed` already knew this; it was
  only ever consulted for a warning line, never by the decision itself.
- a systemd probe that could not answer at all (e.g. systemctl cannot reach
  the user bus over a non-lingering ssh session) looked identical to a clean
  negative, because both probes redirected stderr straight to /dev/null.

detect_supervisor now returns a fifth answer, "unclear", for both cases.
systemd_loaded/systemd_installed capture systemctl's exit status and stderr
separately and set their own *_ERRORED flag only on a real tool failure
(non-zero exit WITH stderr), never on a clean negative. "none" now means only:
neither supervisor installed, neither loaded, neither probe errored.
require_drivable_supervisor die()s on "unclear" exactly like it already does
on "ambiguous", naming the specific supervisor and reason via the new
SUPERVISOR_UNCLEAR_DETAIL global.

Tests: 4 new cases (systemd/launchd installed-but-not-loaded, a real
systemd_loaded run through a systemctl stub that errors on stderr, and the
die() refusal for "unclear" naming the unit). All 3 new guards were verified
by mutation: each was removed from the real script, the suite caught it (a
new FAIL line naming the exact broken assertion), then the file was restored
byte-identically and the suite went green again.

scripts/redeploy-fleetd.sh

@@ -34,7 +34,11 @@
 #      three different answers, drives whichever one it finds through its own control plane
 #      (`launchctl` / `systemctl --user`), and REFUSES outright — never falls back to `kill` — when
 #      it finds a supervision signal it cannot map to exactly one of the two it knows how to drive.
-#      A wrong guess here is how two daemons end up running against one herdr session.
+#      A wrong guess here is how two daemons end up running against one herdr session. Follow-up:
+#      "not currently loaded" is not the same fact as "unsupervised" — a unit that is installed but
+#      activating/failed/pending-restart, or a `systemctl` call that could not answer at all (e.g.
+#      no user-bus access), both now read as a fifth answer, "unclear", and REFUSE the same way
+#      "ambiguous" does, rather than silently falling through to "none".
 #   8. fleetd #492 — a post-restart check counts running fleetd processes and fails the whole run if
 #      more than one is alive. That is the one thing none of the checks above (healthz 200, jar id,
 #      the fresh "listening" line) can see: every one of them is satisfied by EITHER daemon.
@@ -71,6 +75,20 @@ LAUNCHD_PLIST="$HOME/Library/LaunchAgents/$LAUNCHD_LABEL.plist"
 # "dev.ltms.fleetd" — systemd user units here are not namespaced the way the launchd label is).
 SYSTEMD_UNIT='fleetd'
 
+# fleetd #492 follow-up: set by systemd_loaded/systemd_installed when the underlying `systemctl`
+# call could not answer cleanly — it exited non-zero AND wrote something to stderr, which is a real
+# tool failure (e.g. it cannot reach the user bus over a non-lingering ssh session), never the same
+# fact as a clean negative answer ("not active", no stderr). Initialized here, not just inside the
+# probes, so detect_supervisor can read them under `set -u` even before either probe has ever run,
+# and so a test that stubs a probe with a plain `return 0`/`return 1` body (leaving these untouched)
+# reads a deterministic 0 rather than whatever a previous probe call left behind.
+SYSTEMD_LOADED_ERRORED=0
+SYSTEMD_INSTALLED_ERRORED=0
+# fleetd #492 follow-up: set by detect_supervisor alongside an "unclear" answer, naming the specific
+# supervisor and reason so require_drivable_supervisor's die() message is not just the bare word
+# "unclear". Initialized empty for the same `set -u` reason as above.
+SUPERVISOR_UNCLEAR_DETAIL=""
+
 DO_BUILD=1; ASSUME_YES=0; CHECK_ONLY=0
 for arg in "$@"; do
   case "$arg" in
@@ -100,32 +118,103 @@ launchd_loaded() { launchctl list "$LAUNCHD_LABEL" >/dev/null 2>&1; }
 # (this repo is developed on macOS) can substitute each one independently — the same seam
 # launchd_installed/launchd_loaded above already use.
 #
+# fleetd #492 follow-up: both functions used to throw `systemctl`'s stderr straight into
+# /dev/null, which meant "systemctl answered no" and "systemctl could not answer at all" (e.g. it
+# cannot reach the user bus over a non-lingering ssh session) looked identical — both a plain
+# nonzero exit. They now capture stderr separately and set their own *_ERRORED flag ONLY when the
+# call exited non-zero AND wrote something to stderr — a real tool failure, never a clean "not
+# installed"/"not active" answer (which exits non-zero with empty stderr). detect_supervisor reads
+# the flag right after calling the probe, so a probe that could not answer routes to "unclear",
+# never silently becomes "none".
+#
 # "installed": a unit FILE by this name exists, regardless of its current state — the systemd
 # analogue of the plist file existing on disk. `list-unit-files` reads unit definitions without
 # depending on runtime state, so this stays read-only and safe under --check.
 systemd_installed() {
-  command -v systemctl >/dev/null 2>&1 \
+  SYSTEMD_INSTALLED_ERRORED=0
-    && systemctl --user list-unit-files "$SYSTEMD_UNIT.service" --no-legend 2>/dev/null | grep -q .
+  command -v systemctl >/dev/null 2>&1 || return 1
+  local err_file out rc=0
+  if ! err_file="$(mktemp -t systemd-installed-err)"; then
+    SYSTEMD_INSTALLED_ERRORED=1
+    return 1
+  fi
+  out="$(systemctl --user list-unit-files "$SYSTEMD_UNIT.service" --no-legend 2>"$err_file")" || rc=$?
+  if [ "$rc" -ne 0 ]; then
+    if [ -s "$err_file" ]; then
+      SYSTEMD_INSTALLED_ERRORED=1
+    fi
+    rm -f "$err_file"
+    return "$rc"
+  fi
+  rm -f "$err_file"
+  printf '%s' "$out" | grep -q .
 }
 # "loaded": systemd currently supervises this unit as an active job — the systemd analogue of
 # `launchctl list <label>` succeeding. Measured on the second host: `systemctl --user is-active
-# fleetd` -> "active".
+# fleetd` -> "active". A clean "no" (inactive/failed/activating/deactivating) exits non-zero with
+# nothing on stderr; a probe that could not reach systemd at all exits non-zero WITH a stderr
+# message — see the fleetd #492 follow-up note above.
 systemd_loaded() {
-  command -v systemctl >/dev/null 2>&1 && systemctl --user is-active "$SYSTEMD_UNIT" >/dev/null 2>&1
+  SYSTEMD_LOADED_ERRORED=0
+  command -v systemctl >/dev/null 2>&1 || return 1
+  local err_file rc=0
+  if ! err_file="$(mktemp -t systemd-loaded-err)"; then
+    SYSTEMD_LOADED_ERRORED=1
+    return 1
+  fi
+  systemctl --user is-active "$SYSTEMD_UNIT" >/dev/null 2>"$err_file" || rc=$?
+  if [ "$rc" -ne 0 ] && [ -s "$err_file" ]; then
+    SYSTEMD_LOADED_ERRORED=1
+  fi
+  rm -f "$err_file"
+  return "$rc"
 }
 
 # fleetd #492: three real answers, not two — launchd, systemd, or genuinely unsupervised — plus a
 # fourth, "ambiguous", for the one case this script cannot tell apart: both signals firing at once.
 # That is exactly "I cannot tell who supervises this process", and guessing wrong here is how two
-# daemons end up running against one herdr session (see trap 7 in the header). Pure and
+# daemons end up running against one herdr session (see trap 7 in the header).
-# side-effect-free: reads the two probes above and decides — never mutates anything, so it is safe
+#
-# under --check and testable by overriding launchd_loaded/systemd_loaded after sourcing.
+# fleetd #492 follow-up: a fifth answer, "unclear", for two more situations that must NEVER be read
+# as "none" (measured — see the report this ticket is a follow-up to):
+#   - installed-but-not-loaded, on EITHER supervisor. `systemctl --user is-active` answers "no" for
+#     `activating`, `deactivating`, `failed`, and while an auto-restart is pending — every one of
+#     those is a host that IS under systemd (or launchd) and whose supervisor is about to act again.
+#     `*_installed` already knows the unit/agent exists; this is the first place that fact is
+#     actually consulted in the decision, not just printed as a warning.
+#   - a probe that could not answer at all. systemd_loaded/systemd_installed set their own
+#     *_ERRORED flag (see the comment above them) when `systemctl` exits non-zero WITH a stderr
+#     message — a real tool failure, e.g. it cannot reach the user bus over a non-lingering ssh
+#     session — never conflated with a clean negative answer.
+# "none" now means only: neither supervisor is installed, neither is loaded, and neither probe
+# errored. SUPERVISOR_UNCLEAR_DETAIL is set alongside "unclear" so require_drivable_supervisor's
+# die() can name the specific supervisor and reason, not just the bare word "unclear".
+#
+# Pure and side-effect-free besides setting the two globals above: reads the four probes and
+# decides — never mutates anything, so it is safe under --check and testable by overriding
+# launchd_installed/launchd_loaded/systemd_installed/systemd_loaded after sourcing.
 detect_supervisor() {
-  local ld=0 sd=0
+  local ld=0 sd=0 li=0 si=0
+  SYSTEMD_LOADED_ERRORED=0
+  SYSTEMD_INSTALLED_ERRORED=0
+  SUPERVISOR_UNCLEAR_DETAIL=""
+
   launchd_loaded && ld=1
   systemd_loaded && sd=1
-  if [ "$ld" = 1 ] && [ "$sd" = 1 ]; then
+  launchd_installed && li=1
+  systemd_installed && si=1
+
+  if [ "$SYSTEMD_LOADED_ERRORED" = 1 ] || [ "$SYSTEMD_INSTALLED_ERRORED" = 1 ]; then
+    SUPERVISOR_UNCLEAR_DETAIL="the systemd --user probe for '$SYSTEMD_UNIT' could not answer cleanly (systemctl exited non-zero and reported an error on stderr, not a clean negative — e.g. it cannot reach the user bus)"
+    echo "unclear"
+  elif [ "$ld" = 1 ] && [ "$sd" = 1 ]; then
     echo "ambiguous"
+  elif [ "$li" = 1 ] && [ "$ld" = 0 ]; then
+    SUPERVISOR_UNCLEAR_DETAIL="the launchd agent ($LAUNCHD_LABEL) is installed ($LAUNCHD_PLIST exists) but is not currently loaded"
+    echo "unclear"
+  elif [ "$si" = 1 ] && [ "$sd" = 0 ]; then
+    SUPERVISOR_UNCLEAR_DETAIL="the systemd --user unit ($SYSTEMD_UNIT) is installed but not currently active — it may be activating, deactivating, failed, or waiting on an auto-restart"
+    echo "unclear"
   elif [ "$ld" = 1 ]; then
     echo "launchd"
   elif [ "$sd" = 1 ]; then
@@ -150,6 +239,17 @@ require_drivable_supervisor() {
           OLD jar out from under it — the exact failure this ticket (fleetd #492) exists to
           prevent. Stop one of the two supervisors by hand, confirm only one remains loaded, then
           rerun." ;;
+    unclear)
+      # fleetd #492 follow-up: SUPERVISOR_UNCLEAR_DETAIL is set by detect_supervisor right before
+      # it returns "unclear"; the fallback text below only fires if this is ever called directly
+      # (as a test does) without going through detect_supervisor first.
+      die "a supervisor looks present but this script cannot tell whether it actually drives this
+          daemon: ${SUPERVISOR_UNCLEAR_DETAIL:-launchd ($LAUNCHD_LABEL) or systemd --user
+          ($SYSTEMD_UNIT) reported something other than a clean 'loaded' or a clean 'not loaded'}.
+          Guessing wrong here is the same failure 'ambiguous' above exists to prevent: driving the
+          daemon while an unseen supervisor revives the OLD jar out from under it (fleetd #492).
+          Check 'launchctl list $LAUNCHD_LABEL' and 'systemctl --user status $SYSTEMD_UNIT' by
+          hand, resolve whichever looks unclear, then rerun." ;;
     *)
       die "detect_supervisor returned an unrecognized value '$kind' — refusing to guess which
           supervisor, if any, controls this daemon." ;;

scripts/test-redeploy-fleetd.sh

@@ -56,6 +56,78 @@ test_detect_supervisor_none() {
   assert_equals "none" "$(detect_supervisor)" "unsupervised detection"
 }
 
+# fleetd #492 follow-up — detect_supervisor must never answer "none" when the truth is "could not
+# tell". `systemd_installed`/`systemd_loaded` already know a unit file exists; this proves that
+# fact is now actually consulted, not just printed as a warning: an installed-but-not-loaded unit
+# reads as unclear, because is-active answers "no" for activating/deactivating/failed/pending
+# auto-restart too, and every one of those is a host that IS under systemd.
+test_detect_supervisor_systemd_installed_not_loaded_is_unclear() {
+  launchd_installed() { return 1; }
+  launchd_loaded()    { return 1; }
+  systemd_installed() { return 0; }   # the unit file IS there
+  systemd_loaded()    { return 1; }   # is-active says no — could be activating/failed/pending restart
+  assert_equals "unclear" "$(detect_supervisor)" "systemd installed-but-not-loaded must read as unclear, not none"
+}
+
+# Same fact, the launchd side: a plist on disk that is not currently loaded (unloaded without being
+# removed, or about to be reloaded) must not read as "no supervisor" either.
+test_detect_supervisor_launchd_installed_not_loaded_is_unclear() {
+  launchd_installed() { return 0; }   # the plist IS there
+  launchd_loaded()    { return 1; }   # launchctl list says not loaded
+  systemd_installed() { return 1; }
+  systemd_loaded()    { return 1; }
+  assert_equals "unclear" "$(detect_supervisor)" "launchd installed-but-not-loaded must read as unclear, not none"
+}
+
+# Drives the REAL systemd_loaded/systemd_installed bodies (never stubbed) through a `systemctl`
+# stub placed first on PATH that exits non-zero AND writes to stderr — the shape of a systemctl
+# that runs but cannot reach the user bus (measured elsewhere as a headless ssh session with no
+# lingering). This must read as unclear, never none: a probe that could not answer at all is not
+# the same fact as "no supervisor is loaded".
+test_detect_supervisor_systemd_probe_error_is_unclear() {
+  # Re-source first to restore the REAL launchd_*/systemd_* probe bodies. Earlier tests in this
+  # file permanently override them with stub `return 0`/`return 1` bodies (that is the whole point
+  # of those tests), and a bash function definition is global for the rest of the process — without
+  # this, systemd_loaded here would still be whatever the previous test left it as, never touching
+  # a real `systemctl` call at all.
+  source "$ROOT/scripts/redeploy-fleetd.sh"
+  local bin_dir result rc=0
+  bin_dir="$TMP/stub-bin-systemctl-errors"
+  mkdir -p "$bin_dir"
+  cat > "$bin_dir/systemctl" <<'STUB'
+#!/usr/bin/env bash
+echo "Failed to connect to bus: No such file or directory" >&2
+exit 1
+STUB
+  chmod +x "$bin_dir/systemctl"
+
+  PATH="$bin_dir:$PATH" systemd_loaded && rc=0 || rc=$?
+  [ "$rc" -ne 0 ] || fail "systemd_loaded must not report loaded=true when systemctl only errored"
+  assert_equals "1" "$SYSTEMD_LOADED_ERRORED" "systemd_loaded must flag a probe error, not a clean negative"
+
+  launchd_installed() { return 1; }
+  launchd_loaded()    { return 1; }
+  result="$(PATH="$bin_dir:$PATH" detect_supervisor)"
+  assert_equals "unclear" "$result" "a systemd probe error must read as unclear, not none"
+}
+
+# The other half of the ticket: an "unclear" supervisor must refuse exactly like "ambiguous" does —
+# die(), never fall through to the `kill` path — and the refusal must name the specific supervisor
+# and reason detect_supervisor found, not just the bare word "unclear".
+test_require_drivable_supervisor_refuses_unclear() {
+  launchd_installed() { return 1; }
+  launchd_loaded()    { return 1; }
+  systemd_installed() { return 0; }
+  systemd_loaded()    { return 1; }
+  local kind output rc=0
+  kind="$(detect_supervisor)"
+  assert_equals "unclear" "$kind" "setup: expected unclear before testing the refusal"
+  output="$(require_drivable_supervisor "$kind" 2>&1)" || rc=$?
+  [ "$rc" -ne 0 ] || fail "require_drivable_supervisor accepted an unclear (undrivable) supervisor"
+  printf '%s' "$output" | grep -qF "$SYSTEMD_UNIT" \
+    || fail "refusal message does not name the systemd unit it found installed-but-not-loaded"
+}
+
 # The heart of the ticket: a supervisor this script cannot drive must refuse, never fall through to
 # `kill`. require_drivable_supervisor die()s, so it is invoked inside a command substitution — that
 # forks a subshell, so its exit() only ends the subshell and this test script keeps running under
@@ -299,7 +371,11 @@ test_unattributable_quiet_mutation_is_caught() {
 test_detect_supervisor_launchd_only
 test_detect_supervisor_systemd_only
 test_detect_supervisor_none
+test_detect_supervisor_systemd_installed_not_loaded_is_unclear
+test_detect_supervisor_launchd_installed_not_loaded_is_unclear
+test_detect_supervisor_systemd_probe_error_is_unclear
 test_require_drivable_supervisor_refuses_ambiguous
+test_require_drivable_supervisor_refuses_unclear
 test_require_drivable_supervisor_accepts_known_kinds
 test_count_daemon_pids
 test_assert_single_daemon_accepts_one_pid