simplify check skip logic to use runtime checks (#78)

## Summary
- Replace guard-clause-based `should_skip_snmp/service/ping_check?` with a single `should_skip_check?` function using runtime `in` operator checks
- Guard clauses with module attributes (`@service_check_types`) were not working in Dokku buildpack deployments despite code being correct locally
- Also update `CheckWorker` (dead code safety net) to check `device_has_effective_agent?`
- Use `Logger.info` for skip messages so they appear in staging logs

## Context
PR #77 introduced `device_has_effective_agent?/1` and updated all workers, but the guard-clause-based skip functions in `CheckExecutorWorker` weren't matching at runtime on staging despite passing all tests locally. The DNS check kept producing NXDOMAIN errors from Erlang's `:inet_res` resolver.

Root cause: The `when check_type in @service_check_types` guard clause compiled against module attributes appears to have stale evaluation in the Dokku Heroku buildpack release. Moving the check to a runtime function body fixes the issue.

## Test plan
- [x] All 230 worker tests pass
- [x] All 99 agents tests pass
- [x] Verified on staging: "Skipping check eceb3d51... (dns)" appears in info logs
- [x] No new NXDOMAIN results created after deploy

Reviewed-on: graham/towerops-web#78

lib/towerops/workers/check_executor_worker.ex

@@ -39,9 +39,6 @@ defmodule Towerops.Workers.CheckExecutorWorker do
 
   require Logger
 
-  @snmp_check_types ~w(snmp_sensor snmp_interface snmp_processor snmp_storage)
-  @service_check_types ~w(http tcp dns ssl)
-
   @impl Oban.Worker
   def perform(%Oban.Job{args: %{"check_id" => check_id}}) do
     case Monitoring.get_check(check_id) do
@@ -50,53 +47,41 @@ defmodule Towerops.Workers.CheckExecutorWorker do
         :ok
 
       check ->
-        cond do
-          not check.enabled ->
-            Logger.debug("Check #{check_id} disabled, skipping execution")
-
-          should_skip_snmp_check?(check) ->
-            Logger.debug("Skipping SNMP check #{check_id} - Phoenix SNMP disabled or device assigned to agent")
-            schedule_next_check(check)
-
-          should_skip_service_check?(check) ->
-            Logger.debug("Skipping service check #{check_id} - device assigned to agent")
-            schedule_next_check(check)
-
-          should_skip_ping_check?(check) ->
-            Logger.debug("Skipping ping check #{check_id} - device assigned to agent")
-            schedule_next_check(check)
-
-          true ->
-            execute_and_record(check)
-            schedule_next_check(check)
-        end
-
+        execute_or_skip(check)
         :ok
     end
   end
 
-  defp should_skip_snmp_check?(%{check_type: check_type} = check) when check_type in @snmp_check_types do
-    Client.phoenix_snmp_disabled() or
-      (check.device_id != nil and Agents.device_has_effective_agent?(check.device_id))
+  defp execute_or_skip(%{enabled: false} = check) do
+    Logger.debug("Check #{check.id} disabled, skipping execution")
   end
 
-  defp should_skip_snmp_check?(_check), do: false
+  defp execute_or_skip(check) do
+    if should_skip_check?(check) do
+      Logger.info("Skipping check #{check.id} (#{check.check_type}) - device has agent or Phoenix SNMP disabled")
+    else
+      execute_and_record(check)
+    end
 
-  defp should_skip_service_check?(%{check_type: check_type} = check) when check_type in @service_check_types do
-    # Skip service checks (HTTP/TCP/DNS/SSL) if device has any agent (cloud or local)
-    # The agent handles all check execution - no fallback to Phoenix
+    schedule_next_check(check)
+  end
+
+  # Unified skip logic for ALL check types.
+  # Skip when the device has any effective agent assigned at any cascade level,
+  # or when Phoenix SNMP is disabled for SNMP-type checks.
+  defp should_skip_check?(check) do
+    snmp_disabled_for_snmp_check?(check) or device_has_agent?(check)
+  end
+
+  defp snmp_disabled_for_snmp_check?(check) do
+    check.check_type in ~w(snmp_sensor snmp_interface snmp_processor snmp_storage) and
+      Client.phoenix_snmp_disabled()
+  end
+
+  defp device_has_agent?(check) do
     check.device_id != nil and Agents.device_has_effective_agent?(check.device_id)
   end
 
-  defp should_skip_service_check?(_check), do: false
-
-  defp should_skip_ping_check?(%{check_type: "ping"} = check) do
-    # Skip ping if device has any agent (cloud or local)
-    check.device_id != nil and Agents.device_has_effective_agent?(check.device_id)
-  end
-
-  defp should_skip_ping_check?(_check), do: false
-
   defp execute_and_record(check) do
     Logger.debug("Executing check #{check.id} (#{check.check_type})")
 

lib/towerops/workers/check_worker.ex

@@ -57,19 +57,22 @@ defmodule Towerops.Workers.CheckWorker do
   end
 
   defp should_phoenix_execute?(check) do
-    case check.agent_token_id do
-      nil ->
-        # No agent assigned - Phoenix executes
-        true
+    # Never execute if the device has any effective agent at any cascade level
+    if check.device_id != nil and Agents.device_has_effective_agent?(check.device_id) do
+      false
+    else
+      case check.agent_token_id do
+        nil ->
+          true
 
-      agent_token_id ->
-        # Agent assigned - only Phoenix executes if it's cloud poller
-        try do
-          agent_token = Agents.get_agent_token!(agent_token_id)
-          agent_token.is_cloud_poller
-        rescue
-          Ecto.NoResultsError -> true
-        end
+        agent_token_id ->
+          try do
+            agent_token = Agents.get_agent_token!(agent_token_id)
+            agent_token.is_cloud_poller
+          rescue
+            Ecto.NoResultsError -> true
+          end
+      end
     end
   end
 

lib/towerops/workers/device_monitor_worker.ex

@@ -75,7 +75,7 @@ defmodule Towerops.Workers.DeviceMonitorWorker do
         :ok
 
       Agents.device_has_effective_agent?(device.id) ->
-        Logger.debug("Skipping Phoenix monitoring for device #{device.name} - device has agent assigned")
+        Logger.info("Skipping Phoenix monitoring for device #{device.name} - device has agent assigned")
         :ok
 
       true ->

lib/towerops/workers/device_poller_worker.ex

@@ -92,7 +92,7 @@ defmodule Towerops.Workers.DevicePollerWorker do
         :ok
 
       Agents.device_has_effective_agent?(device.id) ->
-        Logger.debug("Skipping Phoenix poll for device #{device.name} - device has agent assigned")
+        Logger.info("Skipping Phoenix poll for device #{device.name} - device has agent assigned")
         :ok
 
       true ->