autoupgrade: a server that is down is not a server that is busy

zeiss came back from this morning's hard reset with no container at all, and the
hourly upgrade then skipped every tick — so the host sat on old code and, worse,
stayed DOWN, with nothing in the loop that would ever have recovered it. Both
idle probes are HTTP, so an absent server is indistinguishable from a wrong
token: inflight() said "unknown" and the fail-closed guard refused to act. The
Digesta host, whose container was up, upgraded itself fourteen seconds after the
push; zeiss needed hands.

container_state() answers the one question the probes cannot — is there a server
at all — by name, then by any running container of the image being upgraded.
Only a POSITIVE "not there" counts as idle: an engine that cannot be asked
(daemon down, wrong socket) stays unknown and still fails closed, because the
point is to recognise an absent server, not to assume one. Checked before the
token, since a host with no server is in the same position as one with no token.
Both installs run their engine's ps as the service user and name the container
`coderai`, so the default matches; CONTAINER_NAME covers an install that does not.

The second half: every knob reads `VAR="${VAR:-default}"`, which looks like the
environment may override it, but the conf is sourced first and assigns
unconditionally — so `MAX_DEFERRALS=1 coderai-autoupgrade` did nothing at all,
with no line in the log to say why. I hit that landing this morning's restart and
had to source the conf from a wrapper to get round it. The environment is now
snapshotted before the conf and restored after, for that run only, and a test
checks the snapshot list against every knob the script resolves so the next one
added cannot quietly fall out. require-idle and max-deferrals are also logged
now: "it skipped again" and "it was told to skip" used to look identical.

The conf example claimed that without a token "the restart happens whenever the
timer says so". It is the opposite — the check fails closed, so a running server
is then never upgraded. Corrected, and REQUIRE_IDLE_CHECK documented next to it.

tests/test_autoupgrade_recovers_a_down_server.py RUNS the script against a stubbed
engine, runner, restart command and health file: the env restore is eval-based and
the container check branches on an exit code, neither of which reading the source
can confirm. 12 new tests, 56 across the three autoupgrade files.

No version bump: nothing in the image changed, and bumping would send both
production installs through an image upgrade and a restart for a host-side script.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019sRvvg5WSdLgcFUM96pmE4

packaging/linux/launcher/coderai-autoupgrade

@@ -24,8 +24,26 @@
 set -uo pipefail
 
 CONF="${CODERAI_AUTOUPGRADE_CONF:-${XDG_CONFIG_HOME:-$HOME/.config}/coderai/autoupgrade.conf}"
+
+# Every knob below is `VAR="${VAR:-default}"`, which reads as "the environment may
+# override it" -- but the conf is sourced FIRST and assigns unconditionally
+# (`MAX_DEFERRALS="0"`), so the conf silently won and `MAX_DEFERRALS=1
+# coderai-autoupgrade` did exactly nothing, with no line in the log to say why.
+# An operator passing a variable on the command line is overriding the unattended
+# policy deliberately and for one run, so the environment has to win. Snapshot what
+# it set, source the conf, put the snapshot back.
+AUTOUPGRADE_TUNABLES="ENGINE IMAGE_TAG UPGRADE_REF RUNNER RESTART_CMD HEALTH_URL
+    HEALTH_TIMEOUT ADMIN_TOKEN METRICS_URL STATUS_URL REQUIRE_IDLE_CHECK
+    MAX_DEFERRALS CONTAINER_NAME STATE_DIR LOG ROLLBACK_TAG LOG_MAX_LINES"
+for _t in $AUTOUPGRADE_TUNABLES; do
+    eval "[ -n \"\${$_t:-}\" ] && _ENV_$_t=\"\$$_t\""
+done
 # shellcheck disable=SC1090
 [ -f "$CONF" ] && . "$CONF"
+for _t in $AUTOUPGRADE_TUNABLES; do
+    eval "[ -n \"\${_ENV_$_t:-}\" ] && $_t=\"\$_ENV_$_t\""
+done
+unset _t
 
 # Which container engine. A rootless podman host is the common case for this
 # script, so prefer podman when both are installed.
@@ -58,6 +76,9 @@ STATUS_URL="${STATUS_URL:-$_base/v1/runpod/status}"
 # A silent upgrade that interrupts production is worse than a skipped one, and an
 # unreadable probe usually means a wrong token or URL, not an idle server.
 REQUIRE_IDLE_CHECK="${REQUIRE_IDLE_CHECK:-1}"
+# Which container is the server, for the one question the HTTP probes cannot
+# answer: is there a server at all? See container_state().
+CONTAINER_NAME="${CONTAINER_NAME:-coderai}"
 # Once the image HAS been upgraded, the restart can be deferred this many ticks
 # before going ahead anyway. 0 (the default) means never interrupt a request:
 # the new code waits in the image and every run says so.
@@ -92,8 +113,13 @@ fi
 # check fails and a perfectly good upgrade gets rolled back. One line makes it
 # obvious instead.
 IDLE_CHECK=off; [ -n "$ADMIN_TOKEN" ] && IDLE_CHECK=on
-say "config: engine=$ENGINE image=$IMAGE_TAG ref=$UPGRADE_REF"
+say "config: engine=$ENGINE image=$IMAGE_TAG ref=$UPGRADE_REF container=$CONTAINER_NAME"
 say "config: restart=[$RESTART_CMD] health=$HEALTH_URL idle-check=$IDLE_CHECK"
+# The two knobs that decide whether a run acts or waits. Neither was in the log,
+# so "it skipped again" and "it was told to skip" looked identical -- and with the
+# conf overriding the environment, an operator could be looking at the opposite of
+# what they typed.
+say "config: require-idle=$REQUIRE_IDLE_CHECK max-deferrals=$MAX_DEFERRALS"
 
 # ---------------------------------------------------------------------- the lock
 # flock on a dedicated fd, non-blocking: a run that overlaps the previous one is
@@ -110,6 +136,31 @@ baked_version(){
         | tr -d ' "' | cut -d= -f2 | head -1
 }
 
+# "running", "stopped", or "unknown" when the engine itself could not be asked.
+#
+# Both probes are HTTP, so a server that is DOWN looks exactly like a wrong token:
+# both are unreadable, inflight() says "unknown", and the fail-closed guard refuses
+# to touch the install that most needs touching. That is not hypothetical -- zeiss
+# came back from a hard reset with no container at all, and every hourly run from
+# then on skipped, so the host sat on old code and stayed DOWN until a human
+# noticed. A container that is not running cannot be serving anything, and the
+# restart that follows an upgrade is also what brings it back.
+#
+# Only a POSITIVE answer counts. If the engine cannot be asked (daemon down,
+# wrong socket) this says "unknown" and the fail-closed guard still applies: the
+# point is to recognise an absent server, not to assume one.
+container_state(){
+    local names rc
+    names="$("$ENGINE" ps --format '{{.Names}}' 2>/dev/null)"; rc=$?
+    [ "$rc" -eq 0 ] || { echo unknown; return; }
+    printf '%s\n' "$names" | grep -qx "$CONTAINER_NAME" && { echo running; return; }
+    # An install that names its container something else is still covered: any
+    # running container of the image being upgraded is the server.
+    [ -n "$("$ENGINE" ps --quiet --filter "ancestor=$IMAGE_TAG" 2>/dev/null)" ] \
+        && { echo running; return; }
+    echo stopped
+}
+
 _probe_metrics(){
     # Two numbers from one scrape, as "<inflight> <jobs>":
     #   coderai_engine_inflight  -- requests in flight, pod-served ones included
@@ -140,6 +191,13 @@ except Exception: print("unknown")' 2>/dev/null
 # the numbers in the log would not match anything an operator can see.
 inflight(){
     local out m j r best detail
+    # Cheapest and most certain signal first, and the only one that works when the
+    # server is down: no container, no work. Checked before the token, because a
+    # host with no server configured for the probes is the same situation.
+    if [ "$(container_state)" = stopped ]; then
+        echo "0 no '$CONTAINER_NAME' container running — nothing can be in flight"
+        return
+    fi
     [ -n "$ADMIN_TOKEN" ] || { echo "unknown no ADMIN_TOKEN configured"; return; }
     out="$(_probe_metrics)"; m="${out%% *}"; j="${out##* }"
     r="$(_probe_runpod)"

packaging/linux/systemd/autoupgrade.conf.example

@@ -4,6 +4,10 @@
 # restart coderai.service` assigns only "systemctl" and then RUNS the rest as a
 # command. Every value below is a default made explicit; delete what you do not
 # change.
+#
+# A value passed in the ENVIRONMENT wins over this file, for that run only — so
+# `MAX_DEFERRALS=1 coderai-autoupgrade` lands a deferred restart now without
+# editing anything. (It used to be the other way round, silently.)
 
 # podman (preferred when installed) or docker.
 ENGINE="podman"
@@ -29,12 +33,26 @@ RESTART_CMD="systemctl --user restart coderai.service"
 HEALTH_URL="http://127.0.0.1:8777/healthz"
 HEALTH_TIMEOUT="300"
 
-# Optional idle check: an admin-scoped API token lets the script ask how many
-# requests are in flight and defer the restart instead of interrupting them.
-# Without it the restart happens whenever the timer says so.
+# The container that IS the server. Only used to answer the one question the HTTP
+# probes cannot: whether a server is running at all. A host that came back from a
+# reboot with no container is idle by definition, and the upgrade's restart is also
+# what brings it back — without this the probes are unreadable, the check below
+# fails closed, and the host sits on old code and stays down.
+CONTAINER_NAME="coderai"
+
+# The idle check: an admin-scoped API token lets the script ask how many requests
+# are in flight and defer the restart instead of interrupting them. WITHOUT a token
+# the check cannot be made at all, and because it fails closed (REQUIRE_IDLE_CHECK
+# below) a running server is then never upgraded — every tick skips and says so.
+# Either set a token, or set REQUIRE_IDLE_CHECK="0" and accept that a restart can
+# interrupt a live request.
 #STATUS_URL="http://127.0.0.1:8777/v1/runpod/status"
 #ADMIN_TOKEN="cra-..."
 
+# Fail closed when the check cannot be made. 1 (the default) is right for a host
+# that serves; 0 suits one where nothing is lost by restarting at a bad moment.
+REQUIRE_IDLE_CHECK="1"
+
 # After this many deferrals the restart happens anyway, so a fleet that is never
 # idle cannot pin an old version indefinitely.
 MAX_DEFERRALS="8"

tests/test_autoupgrade_recovers_a_down_server.py

+"""The unattended upgrade has to recover a host that came back with no server.
+
+Two failure modes, both found the hard way on 2026-10-10 after zeiss was hard-reset:
+
+1. **A server that is DOWN read as "cannot tell".** Both idle probes are HTTP, so a
+   stopped container is indistinguishable from a wrong token: inflight() said
+   "unknown", the fail-closed guard refused to act, and every hourly run skipped —
+   leaving the host on old code and, worse, still down. Nothing in the loop would
+   ever have recovered it. A container that is not running cannot be serving, and
+   the restart that follows an upgrade is also what brings it back.
+
+2. **The conf silently beat the environment.** Every knob reads
+   `VAR="${VAR:-default}"`, which looks like "the environment may override" — but
+   the conf is sourced first and assigns unconditionally, so
+   `MAX_DEFERRALS=1 coderai-autoupgrade` did nothing at all, with no line in the log
+   to say why. An operator passing a variable for one run is overriding the
+   unattended policy on purpose.
+
+Unlike its sibling file, this one RUNS the script: the env-restore is eval-based and
+the container check branches on an exit code, and neither is something reading the
+source can confirm. The container engine, the host runner, the restart command and
+the health endpoint are all stubbed, so nothing here touches a real install.
+"""
+
+import os
+import pathlib
+import subprocess
+
+import pytest
+
+ROOT = pathlib.Path(__file__).resolve().parents[1]
+SCRIPT = ROOT / "packaging/linux/launcher/coderai-autoupgrade"
+SH = SCRIPT.read_text()
+
+STUB_ENGINE = """#!/usr/bin/env bash
+# A fake container engine.
+#   STUB_RUNNING  space-separated names of "running" containers
+#   STUB_BROKEN=1 the engine cannot be asked at all (daemon down, wrong socket)
+case "${1:-}" in
+    ps)
+        [ "${STUB_BROKEN:-0}" = 1 ] && { echo "cannot connect to the daemon" >&2; exit 1; }
+        case "${2:-}" in
+            --quiet) exit 0 ;;                 # no ancestor match, ever
+            *) for n in ${STUB_RUNNING:-}; do echo "$n"; done; exit 0 ;;
+        esac ;;
+    run)   echo "__version__ = \\"$(cat "$STUB_VERSION_FILE" 2>/dev/null || echo 0.0.0)\\""
+           exit 0 ;;
+    tag|image) exit 0 ;;
+esac
+exit 0
+"""
+
+# The upgrade, observable: it writes the new version the engine then reports.
+STUB_RUNNER = '#!/bin/sh\necho "$STUB_NEW_VERSION" > "$STUB_VERSION_FILE"\nexit 0\n'
+STUB_RESTART = '#!/bin/sh\necho RESTARTED >> "$STUB_RESTART_LOG"\nexit 0\n'
+
+
+@pytest.fixture
+def install(tmp_path):
+    """A throwaway install: stub engine/runner/restart, a conf, and a health file."""
+
+    def _write(name, body, mode=0o755):
+        path = tmp_path / name
+        path.write_text(body)
+        path.chmod(mode)
+        return path
+
+    engine = _write("engine", STUB_ENGINE)
+    runner = _write("runner", STUB_RUNNER)
+    restart = _write("restart", STUB_RESTART)
+    health = tmp_path / "health.json"
+    health.write_text('{"ok": true}\n')      # curl reads file:// happily
+    version = tmp_path / "version"
+    version.write_text("0.3.3\n")
+    conf = tmp_path / "autoupgrade.conf"
+    # What both production installs carry: never interrupt work, fail closed.
+    conf.write_text('MAX_DEFERRALS="0"\nREQUIRE_IDLE_CHECK="1"\nADMIN_TOKEN=""\n')
+
+    def run(**env):
+        base = {
+            "HOME": str(tmp_path / "home"),
+            "PATH": "/usr/bin:/bin",
+            "CODERAI_AUTOUPGRADE_CONF": str(conf),
+            "ENGINE": str(engine),
+            "RUNNER": str(runner),
+            "IMAGE_TAG": "testimg:latest",
+            "STATE_DIR": str(tmp_path / "state"),
+            "LOG": str(tmp_path / "state" / "log"),
+            "RESTART_CMD": str(restart),
+            "HEALTH_URL": f"file://{health}",
+            "STUB_VERSION_FILE": str(version),
+            "STUB_NEW_VERSION": "0.3.5",
+            "STUB_RESTART_LOG": str(tmp_path / "restarts"),
+        }
+        base.update({k: str(v) for k, v in env.items()})
+        proc = subprocess.run(["bash", str(SCRIPT)], env=base, text=True,
+                              capture_output=True, timeout=120)
+        return proc.stdout + proc.stderr
+
+    run.tmp = tmp_path
+    run.restarts = tmp_path / "restarts"
+    run.version = version
+    return run
+
+
+def _restarted(install):
+    return install.restarts.exists() and "RESTARTED" in install.restarts.read_text()
+
+
+# ------------------------------------------------- a server that is not running
+def test_a_stopped_container_is_idle_not_unknown():
+    """The source side of it: the check has to come BEFORE the token check, because
+    a host whose server is down is in the same position as one with no token."""
+    blk = SH.split("inflight(){")[1].split("guard_idle()")[0]
+    assert blk.index("container_state") < blk.index("no ADMIN_TOKEN configured")
+
+
+def test_a_host_that_came_back_with_no_server_upgrades_and_starts_it(install):
+    """The whole point: unattended recovery, with no human noticing anything."""
+    out = install(STUB_RUNNING="")
+    assert "idle (no 'coderai' container running" in out
+    assert "image upgraded 0.3.3 -> 0.3.5" in out
+    assert "OK: running 0.3.5 and healthy" in out
+    assert _restarted(install), "the restart is what brings the server back"
+
+
+def test_a_running_server_with_unreadable_probes_still_fails_closed(install):
+    """The protection that must survive the fix: a live request is worth more than
+    an up-to-date image, and an unreadable probe usually means a wrong token."""
+    out = install(STUB_RUNNING="coderai")
+    assert "SKIP: cannot tell whether CoderAI is serving" in out
+    assert "image upgraded" not in out
+    assert not _restarted(install)
+
+
+def test_an_engine_that_cannot_be_asked_is_not_assumed_idle(install):
+    """Only a POSITIVE "not there" counts. A daemon that is down tells us nothing
+    about the server, so the fail-closed guard still applies."""
+    out = install(STUB_BROKEN="1", STUB_RUNNING="coderai")
+    assert "SKIP: cannot tell whether CoderAI is serving" in out
+    assert not _restarted(install)
+
+
+def test_an_install_that_names_its_container_differently_is_covered(install):
+    out = install(CONTAINER_NAME="digesta-coderai", STUB_RUNNING="something-else")
+    assert "container=digesta-coderai" in out
+    assert "idle (no 'digesta-coderai' container running" in out
+
+
+def test_the_container_it_looks_for_is_in_the_log(install):
+    """A name mismatch would otherwise look exactly like a stopped server."""
+    assert "container=coderai" in install(STUB_RUNNING="coderai")
+
+
+# --------------------------------------------------------- environment vs config
+def test_the_environment_overrides_the_config_file(install):
+    """`MAX_DEFERRALS=5 coderai-autoupgrade` used to do nothing whatsoever."""
+    assert "max-deferrals=5" in install(MAX_DEFERRALS=5, STUB_RUNNING="coderai")
+
+
+def test_the_config_file_still_wins_when_the_environment_is_silent(install):
+    """The unattended path is the normal one; this fix must not invert it."""
+    assert "max-deferrals=0" in install(STUB_RUNNING="coderai")
+
+
+def test_the_idle_policy_can_be_lifted_from_the_environment(install):
+    """The escape hatch an operator reaches for when the probe is broken but they
+    can see for themselves that nothing is running."""
+    out = install(REQUIRE_IDLE_CHECK=0, STUB_BROKEN="1", STUB_RUNNING="coderai")
+    assert "require-idle=0" in out
+    assert "WARN: idle check unavailable" in out
+    assert "image upgraded 0.3.3 -> 0.3.5" in out
+
+
+def test_both_decision_knobs_are_logged_every_run(install):
+    """"It skipped again" and "it was told to skip" used to look identical."""
+    out = install(STUB_RUNNING="coderai")
+    assert "require-idle=" in out and "max-deferrals=" in out
+
+
+def test_the_snapshot_covers_every_knob_the_script_resolves():
+    """A knob added to the defaults but not to the snapshot list is a knob the conf
+    keeps silently overriding — the bug this fixed, one variable at a time."""
+    import re
+
+    resolved = set(re.findall(r'^([A-Z][A-Z0-9_]+)="\$\{\1:-', SH, re.M))
+    listed = set(SH.split("AUTOUPGRADE_TUNABLES=\"")[1].split('"')[0].split())
+    # ENGINE is resolved by a command-v probe rather than the ${VAR:-} idiom, and
+    # is in the list; the reverse direction is what matters here.
+    missing = resolved - listed
+    assert not missing, f"not overridable from the environment: {sorted(missing)}"
+
+
+def test_the_token_is_never_echoed_by_the_new_lines():
+    for line in SH.splitlines():
+        if "say " in line:
+            assert "$ADMIN_TOKEN" not in line, line