fix(node-agent): detect restarting/crash-looping containers — state fell through classification, crash-loops were invisible to monitoring

check_containers() classified only exited/dead, running+unhealthy and running.
Docker's "restarting", "paused", "removing" and any future state matched no
branch, so a crash-looping container (restart policy + continuous crash) emitted
ZERO events and world-state kept showing it "healthy" (observed live:
lustro/pi-watchtower-1). Every Docker state is now handled:

- restarting + RestartCount >= CRASH_LOOP_RESTART_THRESHOLD (default 3):
  reuse containers_not_running (high, crash_loop=true) — parity with exited/dead,
  rides the existing supervisor-wired remediation path.
- restarting below threshold: new observational container_restarting (low),
  visible but non-actionable so benign post-deploy restarts don't alarm.
- paused / unknown-or-future state: new diagnostic container_state_unexpected
  (medium) — no more silent fall-through; new Docker states become visible.
- removing: conscious documented skip (ephemeral teardown).
- created: unchanged skip (compose tracking artifact).

RestartCount (top-level inspect field) distinguishes a crash-loop from a one-off
restart. Adds services/node-agent/tests/test_check_containers.py pinning the full
state table.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

services/node-agent/src/node_agent.py

@@ -87,6 +87,19 @@ DISK_CRIT_PCT = 85
 MEM_WARN_PCT  = 85
 MEM_CRIT_PCT  = 95
 
+# A container Docker reports as "restarting" that has accumulated at least this
+# many restarts is treated as a genuine crash-loop (a real fault) rather than a
+# benign post-deploy / one-off restart.
+#
+# Rationale for the threshold: Docker's restart backoff keeps a *genuinely*
+# crash-looping container cycling through the "restarting" state, so a 60 s
+# health cycle is very likely to catch it there. A container that merely
+# restarted once or twice (fresh deploy, a single transient failure that then
+# recovers) is almost always back in "running" by the next cycle. Requiring
+# RestartCount >= 3 clears normal deploy churn while still catching a container
+# that is actually stuck flapping. Configurable via env for tuning per fleet.
+CRASH_LOOP_RESTART_THRESHOLD = int(os.getenv("CRASH_LOOP_RESTART_THRESHOLD", "3"))
+
 # SD-card nodes: enforce 24-hour gap between Docker cleanup runs
 CLEANUP_INTERVAL_SECS = 86_400
 LAST_CLEANUP_FILE = STATE_DIR / "last-docker-cleanup"
@@ -320,10 +333,17 @@ class NodeAgent:
                 health_status  = (c.attrs.get("State", {})
                                    .get("Health", {})
                                    .get("Status", ""))
+                # RestartCount is a top-level inspect field (sibling of State),
+                # not nested under State. It monotonically counts how many times
+                # Docker has auto-restarted this container — the signal that
+                # distinguishes a crash-loop from a one-off restart.
+                restart_count  = c.attrs.get("RestartCount", 0)
 
                 # Skip containers in "created" state — these are Docker Compose
                 # internal tracking artifacts (never started, often hash-prefixed)
                 # that appear when a container is rebuilt outside of compose.
+                # This is a conscious skip, not a blind spot: a "created" container
+                # is not a running service and has no fault to report.
                 if status == "created":
                     continue
 
@@ -332,14 +352,64 @@ class NodeAgent:
                 if not is_managed:
                     continue
 
+                base_payload = {
+                    "container": name,
+                    "status": status,
+                    "restart_policy": restart_policy,
+                    "restart_count": restart_count,
+                }
+
                 # Exited container that carries an auto-restart policy
                 if status in ("exited", "dead"):
                     logger.warning(f"Container exited: {name} (restart={restart_policy})")
                     self.emit_event(
                         "containers_not_running", "high", name,
                         f"Container '{name}' has exited (restart={restart_policy})",
-                        {"container": name, "status": status,
-                         "restart_policy": restart_policy},
+                        base_payload,
+                    )
+
+                # Container stuck in Docker's "restarting" state. This is the gap
+                # that previously made crash-loops invisible: "restarting" matched
+                # neither the exited/dead branch nor the running branch, so a
+                # container flapping under its restart policy emitted ZERO events
+                # and the observer kept showing its last (stale) "healthy" state.
+                #
+                # We split on RestartCount so a benign post-deploy restart does not
+                # alarm, while a real crash-loop escalates to the same actionable
+                # signal as an exited container.
+                elif status == "restarting":
+                    if restart_count >= CRASH_LOOP_RESTART_THRESHOLD:
+                        # Genuine crash-loop: reuse containers_not_running so it
+                        # rides the existing, supervisor-wired remediation path
+                        # (parity with exited/dead — the container is, by
+                        # definition, not staying running).
+                        logger.warning(
+                            f"Container crash-looping: {name} "
+                            f"(restarting, {restart_count} restarts, restart={restart_policy})"
+                        )
+                        self.emit_event(
+                            "containers_not_running", "high", name,
+                            f"Container '{name}' is crash-looping "
+                            f"(restarting, {restart_count} restarts, restart={restart_policy})",
+                            {**base_payload, "crash_loop": True},
+                        )
+                    else:
+                        # Fresh / transient restart. Make it VISIBLE (so the state
+                        # is no longer a silent hole) but do NOT trigger remediation
+                        # — a restart action here would be premature and noisy. If
+                        # it keeps flapping, RestartCount crosses the threshold on a
+                        # later cycle and escalates to containers_not_running above.
+                        # container_restarting is intentionally observational: it is
+                        # not wired to any supervisor trigger.
+                        logger.info(
+                            f"Container restarting: {name} "
+                            f"({restart_count} restarts) — watching for crash-loop"
+                        )
+                        self.emit_event(
+                            "container_restarting", "low", name,
+                            f"Container '{name}' is restarting "
+                            f"({restart_count} restarts) — watching for crash-loop",
+                            {**base_payload, "crash_loop": False},
                         )
 
                 # Running container with a failing health check
@@ -348,7 +418,7 @@ class NodeAgent:
                     self.emit_event(
                         "healthcheck_failed", "high", name,
                         f"Container '{name}' is running but its health check is failing",
-                        {"container": name, "health_status": health_status},
+                        {**base_payload, "health_status": health_status},
                     )
 
                 # Running container that is healthy — confirm to observer so that
@@ -359,8 +429,41 @@ class NodeAgent:
                     self.emit_event(
                         "service_healthy", "info", name,
                         f"Container '{name}' is running",
-                        {"container": name, "status": status,
-                         "health_status": health_status or "none"},
+                        {**base_payload, "health_status": health_status or "none"},
+                    )
+
+                # Deliberately paused. docker pause is always an operator/tool
+                # action (Docker never auto-pauses), so it is not a fault — but a
+                # managed service should not normally sit paused, so surface it
+                # observationally rather than swallowing it. Non-actionable.
+                elif status == "paused":
+                    logger.info(f"Container paused: {name}")
+                    self.emit_event(
+                        "container_state_unexpected", "medium", name,
+                        f"Container '{name}' is paused (restart={restart_policy})",
+                        base_payload,
+                    )
+
+                # Ephemeral teardown state: the container is being removed and will
+                # be gone (or replaced) by the next cycle. Emitting a fault here
+                # would be a guaranteed false positive on every recreate/deploy, so
+                # this is a conscious, documented skip — not a silent hole.
+                elif status == "removing":
+                    logger.debug(f"Container removing (transient teardown): {name}")
+
+                # FALLBACK — any Docker state we do not explicitly handle (including
+                # states a future Docker engine might introduce). Never fall through
+                # silently: emit a diagnostic so a new/unknown state becomes visible
+                # instead of quietly recreating the very blind spot this fix closes.
+                else:
+                    logger.warning(
+                        f"Container in unhandled Docker state: {name} (state='{status}')"
+                    )
+                    self.emit_event(
+                        "container_state_unexpected", "medium", name,
+                        f"Container '{name}' is in unhandled Docker state '{status}' "
+                        f"(restart={restart_policy})",
+                        base_payload,
                     )
 
             except Exception as exc:

services/node-agent/tests/test_check_containers.py

@@ -0,0 +1,187 @@
+"""Tests for NodeAgent.check_containers() Docker-state classification.
+
+Regression coverage for the crash-loop blind spot: a container Docker reports
+as "restarting" previously matched neither the exited/dead branch nor the
+running branch, so a crash-looping container emitted ZERO events and stayed
+"healthy" forever in world-state. These tests pin down the full state table so
+no Docker state can silently fall through classification again.
+"""
+from __future__ import annotations
+
+from unittest.mock import MagicMock
+
+import node_agent
+
+
+# ---------------------------------------------------------------------------
+# Fake Docker container helper
+# ---------------------------------------------------------------------------
+
+def make_container(
+    status,
+    *,
+    restart_policy="unless-stopped",
+    restart_count=0,
+    health="",
+    name="svc",
+    compose_service="svc",
+):
+    """Build a stand-in for a docker-py Container with the attrs check_containers reads."""
+    c = MagicMock()
+    c.status = status
+    c.name = name
+    attrs = {
+        "RestartCount": restart_count,
+        "HostConfig": {"RestartPolicy": {"Name": restart_policy}},
+        "State": {"Health": {"Status": health}} if health else {"Health": {}},
+        "Config": {"Labels": {"com.docker.compose.service": compose_service}},
+    }
+    c.attrs = attrs
+    return c
+
+
+def run_check(agent, containers, monkeypatch):
+    """Run check_containers() against a fixed container list, capturing emitted events.
+
+    Returns the list of (event_type, severity, service, message, payload) tuples.
+    """
+    emitted = []
+
+    def fake_emit(event_type, severity, service, message, payload=None):
+        emitted.append((event_type, severity, service, message, payload or {}))
+
+    monkeypatch.setattr(agent, "emit_event", fake_emit)
+
+    client = MagicMock()
+    client.containers.list.return_value = containers
+    agent.docker_client = client
+
+    agent.check_containers()
+    return emitted
+
+
+# ---------------------------------------------------------------------------
+# The regression: "restarting" must not be silent
+# ---------------------------------------------------------------------------
+
+def test_restarting_crash_loop_emits_high_containers_not_running(agent, monkeypatch):
+    """restarting + high RestartCount → containers_not_running, high, crash_loop=True."""
+    c = make_container("restarting", restart_count=7, name="pi-watchtower-1",
+                       compose_service="watchtower")
+    events = run_check(agent, [c], monkeypatch)
+
+    assert len(events) == 1
+    etype, severity, service, _msg, payload = events[0]
+    assert etype == "containers_not_running"
+    assert severity == "high"
+    assert service == "watchtower"
+    assert payload["crash_loop"] is True
+    assert payload["restart_count"] == 7
+    assert payload["status"] == "restarting"
+
+
+def test_restarting_fresh_emits_low_container_restarting_no_alarm(agent, monkeypatch):
+    """restarting + low RestartCount → observational container_restarting, low, not actionable."""
+    c = make_container("restarting", restart_count=1, name="svc")
+    events = run_check(agent, [c], monkeypatch)
+
+    assert len(events) == 1
+    etype, severity, _service, _msg, payload = events[0]
+    assert etype == "container_restarting"
+    assert severity == "low"
+    assert payload["crash_loop"] is False
+    assert payload["restart_count"] == 1
+    # It must NOT masquerade as an actionable "not running" or "healthy" signal.
+    assert etype not in ("containers_not_running", "service_healthy", "healthcheck_failed")
+
+
+def test_restarting_at_threshold_is_crash_loop(agent, monkeypatch):
+    """RestartCount exactly at the threshold counts as a crash-loop (>=)."""
+    c = make_container("restarting", restart_count=node_agent.CRASH_LOOP_RESTART_THRESHOLD)
+    events = run_check(agent, [c], monkeypatch)
+    assert events[0][0] == "containers_not_running"
+    assert events[0][1] == "high"
+
+
+# ---------------------------------------------------------------------------
+# Existing behaviour must be preserved
+# ---------------------------------------------------------------------------
+
+def test_exited_still_emits_containers_not_running(agent, monkeypatch):
+    c = make_container("exited")
+    events = run_check(agent, [c], monkeypatch)
+    assert events[0][0] == "containers_not_running"
+    assert events[0][1] == "high"
+
+
+def test_dead_still_emits_containers_not_running(agent, monkeypatch):
+    c = make_container("dead")
+    events = run_check(agent, [c], monkeypatch)
+    assert events[0][0] == "containers_not_running"
+
+
+def test_running_healthy_emits_service_healthy(agent, monkeypatch):
+    c = make_container("running")
+    events = run_check(agent, [c], monkeypatch)
+    assert events[0][0] == "service_healthy"
+    assert events[0][1] == "info"
+
+
+def test_running_unhealthy_emits_healthcheck_failed(agent, monkeypatch):
+    c = make_container("running", health="unhealthy")
+    events = run_check(agent, [c], monkeypatch)
+    assert events[0][0] == "healthcheck_failed"
+    assert events[0][1] == "high"
+
+
+# ---------------------------------------------------------------------------
+# The other previously-silent states
+# ---------------------------------------------------------------------------
+
+def test_paused_emits_state_unexpected(agent, monkeypatch):
+    c = make_container("paused")
+    events = run_check(agent, [c], monkeypatch)
+    assert len(events) == 1
+    assert events[0][0] == "container_state_unexpected"
+    assert events[0][1] == "medium"
+
+
+def test_unknown_state_emits_fallback_diagnostic(agent, monkeypatch):
+    """A Docker state we do not model must never be silent — fallback diagnostic."""
+    c = make_container("some-future-state")
+    events = run_check(agent, [c], monkeypatch)
+    assert len(events) == 1
+    etype, severity, _service, _msg, payload = events[0]
+    assert etype == "container_state_unexpected"
+    assert severity == "medium"
+    assert payload["status"] == "some-future-state"
+
+
+def test_removing_is_conscious_silent_skip(agent, monkeypatch):
+    """removing is an ephemeral teardown state — deliberately no event (documented)."""
+    c = make_container("removing")
+    events = run_check(agent, [c], monkeypatch)
+    assert events == []
+
+
+def test_created_is_skipped(agent, monkeypatch):
+    c = make_container("created")
+    events = run_check(agent, [c], monkeypatch)
+    assert events == []
+
+
+# ---------------------------------------------------------------------------
+# Managed-container filter still applies
+# ---------------------------------------------------------------------------
+
+def test_unmanaged_container_is_ignored(agent, monkeypatch):
+    """No restart policy → not a long-running managed service → no events, even if restarting."""
+    c = make_container("restarting", restart_policy="no", restart_count=9)
+    events = run_check(agent, [c], monkeypatch)
+    assert events == []
+
+
+def test_no_docker_client_is_noop(agent, monkeypatch):
+    agent.docker_client = None
+    # Should simply return without raising.
+    agent.check_containers()