(fix): healthcheck probe for RoonServer without extension

.github/workflows/build.yml

@@ -62,6 +62,9 @@ jobs:
       - name: Smoke tests
         run: ./tests/smoke.sh roonserver:test
 
+      - name: Healthcheck probe tests
+        run: ./tests/healthcheck.sh roonserver:test
+
       - name: Runtime tests
         run: ./tests/runtime.sh roonserver:test
 

Dockerfile

@@ -50,9 +50,19 @@ EXPOSE 9003/udp 9100-9200/tcp 9200-9250/tcp 9330-9339/tcp 55000/tcp
 
 VOLUME /Roon /RoonBackups /Music
 
-# Healthcheck uses /proc directly instead of pgrep to avoid procps dependency
+# Healthcheck uses /proc directly instead of pgrep to avoid procps dependency.
+#
+# Two probes, because the head's name depends on the package. Current builds
+# exec it via Server/.roonhost/RoonServer, so it is identifiable only by comm
+# ("RoonServer", no extension); legacy shared-runtime builds exec dotnet with
+# RoonServer.dll as an argument, where comm is just "dotnet". The comm match is
+# anchored (-x) because every install path contains a "RoonServer" directory —
+# unanchored, start.sh alone satisfies it and the container can never report
+# unhealthy. tests/healthcheck.sh pins both against this directive.
 HEALTHCHECK --interval=30s --timeout=10s --start-period=120s --retries=3 \
-    CMD grep -ql '[R]oonServer.dll' /proc/[0-9]*/cmdline 2>/dev/null || exit 1
+    CMD grep -qlxE 'RoonServer(\.exe|\.dll)?' /proc/[0-9]*/comm 2>/dev/null \
+     || grep -qlE '[R]oonServer\.(dll|exe)' /proc/[0-9]*/cmdline 2>/dev/null \
+     || exit 1
 
 # entrypoint.sh downloads RoonServer on first run (to /Roon/app), then
 # exec's into start.sh — the stock bash launcher that handles

tests/healthcheck.sh

@@ -0,0 +1,145 @@
+#!/usr/bin/env bash
+# Pins the HEALTHCHECK probe against every shape the RoonServer head has
+# shipped in, without waiting on Docker's start_period or needing a real
+# install.
+#
+# runtime.sh cannot cover this: it starts a container with `--entrypoint
+# sleep`, so no process references the install path and a probe loose enough to
+# match the "/Roon/app/RoonServer" directory still passes. This suite tests the
+# property that matters — healthy iff a server head is actually running.
+#
+# The probe is read out of the built image, so it cannot drift from Dockerfile.
+set -euo pipefail
+
+IMAGE="${1:?Usage: healthcheck.sh <image:tag>}"
+PASS=0
+FAIL=0
+CLEANUP_CONTAINERS=()
+
+cleanup() {
+    for c in ${CLEANUP_CONTAINERS+"${CLEANUP_CONTAINERS[@]}"}; do
+        docker rm -f "$c" >/dev/null 2>&1 || true
+    done
+}
+trap cleanup EXIT
+
+echo "=== Healthcheck probe tests: $IMAGE ==="
+
+# The HEALTHCHECK directive lands in image config as ["CMD-SHELL", "<script>"].
+PROBE="$(docker inspect --format '{{index .Config.Healthcheck.Test 1}}' "$IMAGE")"
+if [ -z "$PROBE" ]; then
+    echo "  FAIL  image declares no HEALTHCHECK"
+    exit 1
+fi
+printf '  probe under test: %s\n\n' "$PROBE"
+
+# Stage a fake install, then exec a spinner under the name the scenario needs.
+# `sleep` stands in for the head: comm is the basename of the execve'd path
+# whatever the binary is. Every scenario keeps start.sh running, since the
+# regression guarded against is "supervisor alive, head dead" reading healthy.
+stage() {
+    cat <<'SETUP'
+R=/Roon/app/RoonServer
+mkdir -p "$R/Server/.roonhost" "$R/Appliance" "$R/RoonDotnet"
+cp /bin/sleep "$R/Server/RoonServer.exe"
+cp /bin/sleep "$R/RoonDotnet/dotnet"
+ln -sf ../RoonServer.exe "$R/Server/.roonhost/RoonServer"
+ln -sf dotnet "$R/RoonDotnet/RoonServer"
+printf '#!/bin/bash\nsleep infinity\n' > "$R/start.sh"
+chmod +x "$R/start.sh"
+# The supervisor: always present, never sufficient on its own.
+/bin/bash "$R/start.sh" &
+SETUP
+    printf '%s\n' "$1"
+    printf 'wait\n'
+}
+
+# $1 = description, $2 = expected (healthy|unhealthy), $3 = head launch line
+probe_case() {
+    local desc="$1" expect="$2" head="$3"
+    local container
+    container="roon-hc-$(printf '%s' "$desc" | tr -cd '[:alnum:]' | cut -c1-24)"
+    CLEANUP_CONTAINERS+=("$container")
+
+    # Setup goes in via the environment, never argv: passed with `-c` it would
+    # put literal "RoonServer.exe" into PID 1's cmdline, and the negative case
+    # would match the cmdline probe and pass for the wrong reason.
+    docker run -d --name "$container" \
+        -e HC_SETUP="$(stage "$head")" \
+        --entrypoint /bin/bash "$IMAGE" -c 'eval "$HC_SETUP"' >/dev/null
+
+    # Wait for staging to finish and the head to exec into its final name.
+    local i
+    for i in $(seq 1 20); do
+        if docker exec "$container" test -x /Roon/app/RoonServer/start.sh 2>/dev/null; then
+            break
+        fi
+        sleep 0.5
+    done
+    sleep 1
+
+    local actual
+    if docker exec "$container" sh -c "$PROBE" >/dev/null 2>&1; then
+        actual=healthy
+    else
+        actual=unhealthy
+    fi
+
+    if [ "$actual" = "$expect" ]; then
+        echo "  PASS  $desc -> $actual"
+        PASS=$((PASS + 1))
+    else
+        echo "  FAIL  $desc -> $actual (expected $expect)"
+        printf '        | processes seen by the probe:\n'
+        docker exec "$container" sh -c \
+            'for p in /proc/[0-9]*/cmdline; do d=${p%/cmdline}
+                 printf "        |   comm=%-16s argv=%s\n" "$(cat "$d/comm")" "$(tr "\0" " " < "$p")"
+             done' 2>/dev/null || true
+        FAIL=$((FAIL + 1))
+    fi
+
+    docker rm -f "$container" >/dev/null 2>&1 || true
+}
+
+R=/Roon/app/RoonServer
+
+# ─── Package shapes that must report healthy ─────────────────────
+#
+# Each head is launched in a subshell that `exec`s, so no wrapper process
+# lingers carrying the scenario text in its cmdline — otherwise a shape meant
+# to exercise the comm probe could pass via the cmdline probe instead.
+
+# Legacy shared-runtime: launcher symlinks dotnet under the app name and passes
+# RoonServer.dll as argv[1]. comm is "RoonServer" here...
+probe_case "legacy FDD (dotnet alias + RoonServer.dll)" healthy \
+    "( exec -a '$R/RoonDotnet/RoonServer RoonServer.dll' '$R/RoonDotnet/RoonServer' infinity ) &"
+
+# ...but the bare variant execs dotnet directly, leaving comm as "dotnet", so
+# only the command line identifies it. This is why the cmdline branch is kept.
+probe_case "bare dotnet with RoonServer.dll argument" healthy \
+    "( exec -a '$R/RoonDotnet/dotnet RoonServer.dll' '$R/RoonDotnet/dotnet' infinity ) &"
+
+# 2.71 b1680: apphost renamed to .exe, no alias and no argv[0] override.
+probe_case "apphost as RoonServer.exe (no alias)" healthy \
+    "cd '$R/Server' && ( exec -a './RoonServer.exe' ./RoonServer.exe infinity ) &"
+
+# Same package with the launcher overriding argv[0] but still no alias: comm
+# keeps the .exe suffix, so an extensionless-only probe would miss it.
+probe_case "apphost with argv0 override, no alias" healthy \
+    "cd '$R/Server' && ( exec -a '$R/Server/RoonServer' ./RoonServer.exe infinity ) &"
+
+# 2.71 b1683+: exec'd through Server/.roonhost/RoonServer, so comm is the bare
+# "RoonServer" and no process carries a .dll or .exe on its cmdline at all.
+probe_case "apphost via .roonhost alias (current)" healthy \
+    "( exec -a '$R/Server/RoonServer' '$R/Server/.roonhost/RoonServer' infinity ) &"
+
+# ─── The case that must report unhealthy ─────────────────────────
+#
+# start.sh is alive and its argv contains "/Roon/app/RoonServer", but no head
+# is running — the case an unanchored cmdline match reports healthy forever.
+probe_case "head dead, start.sh still running" unhealthy \
+    "true"
+
+echo ""
+echo "=== $PASS passed, $FAIL failed ==="
+[ "$FAIL" -eq 0 ]

tests/runtime.sh

@@ -227,10 +227,11 @@ check "logs contain roon version" \
 # Record production version for later comparison
 PROD_VERSION=$(sed -n '2p' "$ROON_DIR/app/RoonServer/VERSION" 2>/dev/null || echo "")
 
-# HEALTHCHECK happy path — with RoonServer.dll running, the grep-based
-# healthcheck in Dockerfile should flip to "healthy" once start_period
-# elapses (120s) plus one interval (30s). Budget 240s total.
-check "HEALTHCHECK reports 'healthy' when RoonServer.dll is running" \
+# HEALTHCHECK happy path — against a real install the probe should flip to
+# "healthy" once start_period elapses (120s) plus one interval (30s). Budget
+# 240s total. Covers the Docker wiring; per-package-layout probe behaviour is
+# pinned in healthcheck.sh.
+check "HEALTHCHECK reports 'healthy' against a real install" \
     wait_for_health "$CONTAINER" healthy 240
 
 echo "    Testing clean shutdown..."
@@ -510,14 +511,15 @@ docker stop -t 10 "$CONTAINER" 2>/dev/null || true
 
 # ─── HEALTHCHECK unhealthy path ──────────────────────────────────
 #
-# Verify the HEALTHCHECK defined in Dockerfile actually flips to
-# "unhealthy" when RoonServer.dll is not running. Without this test,
-# a broken grep pattern (e.g., someone silently changing the filename)
-# would leave containers reporting "healthy" even after Roon crashed.
+# Verify the HEALTHCHECK defined in Dockerfile actually flips a container
+# to "unhealthy" at all.
 #
-# Start with `--entrypoint sleep` so RoonServer.dll never runs but the
+# Start with `--entrypoint sleep` so RoonServer never runs but the
 # HEALTHCHECK directive still applies. After start_period (120s) +
 # 3 × interval (30s each) = 210s, status should be "unhealthy".
+#
+# Weak as a test of the probe itself — nothing here references the install
+# path — so "supervisor alive, head gone" lives in healthcheck.sh instead.
 
 echo ""
 echo "=== Runtime tests (HEALTHCHECK unhealthy path): $IMAGE ==="
@@ -530,7 +532,7 @@ docker run -d --name "$CONTAINER" \
     "$IMAGE" infinity >/dev/null
 
 # Give it up to 4 minutes: start_period 120s + 3 × 30s retries + margin.
-check "HEALTHCHECK reports 'unhealthy' when RoonServer.dll is absent" \
+check "HEALTHCHECK reports 'unhealthy' when nothing is running" \
     wait_for_health "$CONTAINER" unhealthy 240
 
 docker stop -t 5 "$CONTAINER" 2>/dev/null || true