skip discovery for devices with snmp disabled (#58)

add early bail in DiscoveryWorker when snmp_enabled is false.
mark snmp_not_enabled and device_unresponsive as non-retryable
errors to prevent wasted retries.

Reviewed-on: graham/towerops-web#58

lib/towerops/workers/discovery_worker.ex

@@ -131,6 +131,15 @@ defmodule Towerops.Workers.DiscoveryWorker do
     end
   end
 
+  defp perform_discovery(%{snmp_enabled: false} = device) do
+    Logger.info(
+      "Skipping discovery for #{device.name} (#{device.ip_address}): SNMP disabled",
+      device_id: device.id
+    )
+
+    {:error, :snmp_not_enabled}
+  end
+
   defp perform_discovery(device) do
     start_time = System.monotonic_time(:millisecond)
 
@@ -139,7 +148,6 @@ defmodule Towerops.Workers.DiscoveryWorker do
       device_id: device.id,
       device_name: device.name,
       device_ip: device.ip_address,
-      snmp_enabled: device.snmp_enabled,
       snmp_port: device.snmp_port || 161
     )
 
@@ -282,6 +290,8 @@ defmodule Towerops.Workers.DiscoveryWorker do
   def retryable_error?(:invalid_config), do: false
   def retryable_error?(:no_snmp_credentials), do: false
   def retryable_error?(:device_deleted), do: false
+  def retryable_error?(:snmp_not_enabled), do: false
+  def retryable_error?(:device_unresponsive), do: false
   def retryable_error?(_reason), do: true
 
   # Private helpers

test/towerops/workers/discovery_worker_test.exs

@@ -79,15 +79,44 @@ defmodule Towerops.Workers.DiscoveryWorkerTest do
     end
 
     test "returns error for transient failures to allow retry", %{device: device} do
-      # Mock SNMP failure with a transient error (timeout)
-      stub(SnmpMock, :get, fn _target, _oid, _opts ->
-        {:error, :timeout}
+      # Mock SNMP to pass speed categorization but fail during discovery
+      stub(SnmpMock, :get, fn _target, oid, _opts ->
+        case oid do
+          # Speed categorization probe succeeds (device is responsive)
+          "1.3.6.1.2.1.1.1.0" -> {:ok, "Test"}
+          "1.3.6.1.2.1.1.2.0" -> {:ok, [1, 3, 6, 1]}
+          "1.3.6.1.2.1.1.3.0" -> {:ok, 1}
+          # Other gets fail with transient error
+          _ -> {:error, :timeout}
+        end
       end)
 
+      stub(SnmpMock, :walk, fn _target, _oid, _opts -> {:error, :timeout} end)
+
       # Transient failures should return {:error, reason} so Oban can retry
       assert {:error, _reason} = DiscoveryWorker.perform(%Oban.Job{args: %{"device_id" => device.id}})
     end
 
+    test "skips discovery when snmp_enabled is false", %{org: organization} do
+      {:ok, site} =
+        Towerops.Sites.create_site(%{
+          name: "No SNMP Site",
+          organization_id: organization.id
+        })
+
+      {:ok, device} =
+        Towerops.Devices.create_device(%{
+          name: "No SNMP Device",
+          ip_address: "192.168.1.99",
+          snmp_enabled: false,
+          site_id: site.id,
+          organization_id: organization.id
+        })
+
+      # Should discard immediately without attempting SNMP
+      assert :discard = DiscoveryWorker.perform(%Oban.Job{args: %{"device_id" => device.id}})
+    end
+
     test "successfully completes discovery with proper mocks", %{device: device} do
       # Mock SNMP responses - use stub for flexibility as discovery calls vary
       stub(SnmpMock, :get, fn _target, oid, _opts ->
@@ -198,6 +227,14 @@ defmodule Towerops.Workers.DiscoveryWorkerTest do
       refute DiscoveryWorker.retryable_error?(:device_deleted)
     end
 
+    test "snmp_not_enabled is not retryable" do
+      refute DiscoveryWorker.retryable_error?(:snmp_not_enabled)
+    end
+
+    test "device_unresponsive is not retryable" do
+      refute DiscoveryWorker.retryable_error?(:device_unresponsive)
+    end
+
     test "unknown errors are retryable by default" do
       assert DiscoveryWorker.retryable_error?(:some_unknown_error)
     end