From 82091e31a874845a8e84fba7dd4f4f0c19aa40c3 Mon Sep 17 00:00:00 2001
From: Developer <dev@headquarter.local>
Date: Tue, 9 Jun 2026 15:04:53 +0000
Subject: [PATCH] fix: stop health monitor spam and garbled notification
 metadata
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Root causes:
1. No dedup for monitor restarts — _last_known_state is cleared on stop,
   so every restart re-sent notifications for all unhealthy instances.
2. Aggressive error classification — any Docker state other than 'running'
   was treated as 'error', including transient 'created' and 'restarting'.
3. Confusing metadata — when new_status == previous_status (after restart),
   notifications showed previous_status equal to current status.

Fixes:
- _check_instance: when previous is None (first check) and new_status equals
  the DB status, just record the snapshot and skip _handle_state_change.
  This prevents duplicate events/notifications on monitor restart.
- _derive_status: only treat 'exited' and 'dead' as error. Preserve current
  status for transient Docker states ('created', 'restarting').
- _derive_status: if DB says 'running' but container is 'not_found',
  return 'error' instead of preserving 'running' (fixes silent failure).
- _handle_state_change: improved unhealthy message to 'Container tunnel is
  unreachable' instead of generic 'Container is now unhealthy'.

Quality gates: py_compile all backend files pass, tsc --noEmit pass,
npm run build pass, 82/82 tests pass
---
 .../src/services/instance/health_monitor.py   | 55 +++++++++++++------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/apps/api/src/services/instance/health_monitor.py b/apps/api/src/services/instance/health_monitor.py
index 15164de..95b39ec 100644
--- a/apps/api/src/services/instance/health_monitor.py
+++ b/apps/api/src/services/instance/health_monitor.py
@@ -149,12 +149,22 @@ class HealthMonitor:
             instance.status,
         )
 
-        # If first check or state changed
-        if previous is None or not self._snapshots_equal(previous, snapshot):
-            await self._handle_state_change(
-                session, instance, previous, snapshot, new_status
-            )
-            self._last_known_state[instance.id] = snapshot
+        # If first check or snapshot changed
+        if previous is None:
+            # Monitor restart / first time seeing this instance.
+            # Only act if the derived status is different from the DB status.
+            # This prevents duplicate notifications after monitor restarts.
+            if new_status == instance.status:
+                self._last_known_state[instance.id] = snapshot
+                return
+        elif self._snapshots_equal(previous, snapshot):
+            # Nothing changed since last poll — skip entirely.
+            return
+
+        await self._handle_state_change(
+            session, instance, previous, snapshot, new_status
+        )
+        self._last_known_state[instance.id] = snapshot
 
     def _derive_status(
         self,
@@ -169,24 +179,34 @@ class HealthMonitor:
         alerts for instances that are still starting or have no container yet.
         """
         if snapshot.container_status == "not_found":
-            # If the container was never seen running, assume it's still
-            # starting or was deleted intentionally; don't flag as error.
-            if previous is None and current_status == "starting":
+            # Still starting — container may not exist yet.
+            if current_status == "starting":
                 return "starting"
+            # Container disappeared while it was supposed to be running.
+            if current_status == "running":
+                return "error"
+            # If we have previous memory and the container was running,
+            # mark as error (handles monitor restart edge case).
             if previous is not None and previous.container_status == "running":
                 return "error"
             # Fall back to current status to avoid spurious errors.
             return current_status or "error"
 
-        if snapshot.container_status == "exited":
+        if snapshot.container_status in ("exited", "dead"):
             return "error"
 
-        if snapshot.container_status != "running":
-            return "error"
+        if snapshot.container_status == "running":
+            if snapshot.tunnel_healthy is False:
+                return "unhealthy"
+            return "running"
 
-        if snapshot.tunnel_healthy is False:
-            return "unhealthy"
-        return "running"
+        # Transient states (created, restarting) — preserve current status
+        # instead of treating them as an error. The next poll will resolve.
+        if snapshot.container_status in ("created", "restarting"):
+            return current_status or "starting"
+
+        # Unknown/unexpected state (paused, etc.)
+        return "error"
 
     def _snapshots_equal(self, a: HealthSnapshot, b: HealthSnapshot) -> bool:
         """Compare two snapshots for equality."""
@@ -242,7 +262,10 @@ class HealthMonitor:
                 message += f" (exit code: {snapshot.exit_code})"
         else:
             event_type = "instance.health_changed"
-            message = f"Container is now {new_status}"
+            if new_status == "unhealthy":
+                message = "Container tunnel is unreachable"
+            else:
+                message = f"Container is now {new_status}"
 
         payload: InstanceEventPayload = {
             "event": event_type,