From bf2ad4031bbbddd2965e819e7283b4037affe3f2 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Sat, 9 May 2026 12:25:13 -0500
Subject: [PATCH] test(agent_channel): cover stuck alert resolution, deprecated
 PING, MikroTik job building

Adds 8 tests targeting AgentChannel branches missed by existing tests:

- resolve_any_stuck_down_alerts: device_down and device_up stuck alerts
  resolved on next successful poll
- handle_in result with :PING job_type (deprecated SnmpResult path used
  by older agents) creates a monitoring check without crashing
- handle_live_poll_result with malformed job_id (missing reply_topic)
  hits the warning catch-all branch
- build_mikrotik_job is included in the initial dispatch when the device
  has a discovered SNMP profile whose manufacturer is MikroTik or whose
  sys_descr mentions RouterOS, and excluded when mikrotik_enabled is false
- process_additional_polling_data path with extra OIDs runs without
  crashing the channel
---
 .../agent_channel_extra_coverage_test.exs     | 399 ++++++++++++++++++
 1 file changed, 399 insertions(+)
 create mode 100644 test/towerops_web/channels/agent_channel_extra_coverage_test.exs

diff --git a/test/towerops_web/channels/agent_channel_extra_coverage_test.exs b/test/towerops_web/channels/agent_channel_extra_coverage_test.exs
new file mode 100644
index 00000000..e401df22
--- /dev/null
+++ b/test/towerops_web/channels/agent_channel_extra_coverage_test.exs
@@ -0,0 +1,399 @@
+defmodule ToweropsWeb.AgentChannelExtraCoverageTest do
+  @moduledoc """
+  Targeted coverage for ToweropsWeb.AgentChannel paths not exercised in the
+  primary channel tests:
+
+    * Stuck device_down/device_up alert resolution after a successful poll
+    * Deprecated PING result delivered as SnmpResult (legacy agents)
+    * Live poll result with malformed job_id triggers warning branch
+    * MikroTik device job building when device.snmp_device matches RouterOS
+  """
+  use Towerops.DataCase, async: false
+
+  import Phoenix.ChannelTest
+
+  alias Towerops.AccountsFixtures
+  alias Towerops.Agent.AgentHeartbeat
+  alias Towerops.Agent.AgentJobList
+  alias Towerops.Agent.SnmpResult
+  alias Towerops.AgentsFixtures
+  alias Towerops.Alerts
+  alias Towerops.DevicesFixtures
+  alias Towerops.OrganizationsFixtures
+  alias Towerops.Snmp.AgentDiscovery
+  alias Towerops.SnmpFixtures
+  alias ToweropsWeb.AgentSocket
+
+  @endpoint ToweropsWeb.Endpoint
+
+  setup do
+    user = AccountsFixtures.user_fixture()
+    organization = OrganizationsFixtures.organization_fixture(user.id)
+
+    device =
+      DevicesFixtures.device_fixture(%{
+        organization_id: organization.id,
+        snmp_version: "2c",
+        snmp_community: "public"
+      })
+
+    {:ok, agent_token, token_string} =
+      AgentsFixtures.agent_token_fixture(organization.id)
+
+    {:ok, _assignment} =
+      AgentsFixtures.agent_assignment_fixture(agent_token.id, device.id)
+
+    {:ok, socket} = connect(AgentSocket, %{})
+
+    {:ok, _, socket} =
+      subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string})
+
+    assert_push "jobs", _initial_jobs
+
+    %{
+      socket: socket,
+      device: device,
+      agent_token: agent_token,
+      token_string: token_string,
+      organization: organization
+    }
+  end
+
+  defp encode_payload(struct) do
+    binary = struct.__struct__.encode(struct)
+    %{"binary" => Base.encode64(binary)}
+  end
+
+  defp build_heartbeat do
+    %AgentHeartbeat{
+      version: "1.0.0",
+      hostname: "test-agent",
+      uptime_seconds: 3600,
+      ip_address: "",
+      arch: "x86_64"
+    }
+  end
+
+  defp poll_until(fun, opts \\ []) do
+    max_attempts = Keyword.get(opts, :max_attempts, 100)
+    delay_ms = Keyword.get(opts, :delay_ms, 20)
+
+    Enum.reduce_while(1..max_attempts, nil, fn _, _ ->
+      case fun.() do
+        nil -> Process.sleep(delay_ms) && {:cont, nil}
+        false -> Process.sleep(delay_ms) && {:cont, nil}
+        result -> {:halt, result}
+      end
+    end) || fun.()
+  end
+
+  # Set up a device that has been discovered (so poll path runs without
+  # hitting the discovery branch first).
+  defp discover_device!(device) do
+    oid_values = %{
+      "1.3.6.1.2.1.1.1.0" => "Test Device",
+      "1.3.6.1.2.1.1.2.0" => "1.3.6.1.4.1.9.1.1",
+      "1.3.6.1.2.1.1.3.0" => "123456",
+      "1.3.6.1.2.1.1.4.0" => "admin@test.com",
+      "1.3.6.1.2.1.1.5.0" => "test-device",
+      "1.3.6.1.2.1.1.6.0" => "Test Location"
+    }
+
+    {:ok, _discovered} = AgentDiscovery.process_agent_discovery(device, oid_values)
+    Towerops.Devices.get_device_with_details(device.id)
+  end
+
+  # ── stuck device_down alert resolution ──────────────────────────
+
+  describe "resolve_any_stuck_down_alerts" do
+    setup %{device: device} do
+      device = discover_device!(device)
+      # Mark device as up — this exercises the "stuck alert" branch where
+      # device is up but a device_down alert is still active.
+      {:ok, device} = Towerops.Devices.update_device_status(device, :up)
+      %{device: device}
+    end
+
+    test "resolves stuck device_down alert after successful poll", %{
+      socket: socket,
+      device: device
+    } do
+      now = DateTime.utc_now()
+
+      {:ok, alert} =
+        Alerts.create_alert(%{
+          device_id: device.id,
+          alert_type: :device_down,
+          triggered_at: now,
+          message: "stuck"
+        })
+
+      # Sanity check — alert is active
+      assert Alerts.get_active_alert(device.id, :device_down)
+
+      result = %SnmpResult{
+        device_id: device.id,
+        job_type: :POLL,
+        job_id: "poll:#{device.id}",
+        timestamp: DateTime.to_unix(now),
+        oid_values: %{}
+      }
+
+      push(socket, "result", encode_payload(result))
+
+      # Wait for alert to be resolved
+      resolved =
+        poll_until(fn ->
+          a = Alerts.get_alert!(alert.id)
+          if a.resolved_at, do: a
+        end)
+
+      assert resolved.resolved_at
+      refute Alerts.get_active_alert(device.id, :device_down)
+    end
+
+    test "resolves stuck device_up alert after successful poll", %{
+      socket: socket,
+      device: device
+    } do
+      now = DateTime.utc_now()
+
+      {:ok, alert} =
+        Alerts.create_alert(%{
+          device_id: device.id,
+          alert_type: :device_up,
+          triggered_at: now,
+          message: "stuck up"
+        })
+
+      assert Alerts.get_active_alert(device.id, :device_up)
+
+      result = %SnmpResult{
+        device_id: device.id,
+        job_type: :POLL,
+        job_id: "poll:#{device.id}",
+        timestamp: DateTime.to_unix(now),
+        oid_values: %{}
+      }
+
+      push(socket, "result", encode_payload(result))
+
+      resolved =
+        poll_until(fn ->
+          a = Alerts.get_alert!(alert.id)
+          if a.resolved_at, do: a
+        end)
+
+      assert resolved.resolved_at
+    end
+  end
+
+  # ── deprecated PING via SnmpResult ──────────────────────────────
+
+  describe "deprecated PING SnmpResult path" do
+    test "PING job_type in SnmpResult creates a monitoring check and does not crash",
+         %{socket: socket, device: device} do
+      device = discover_device!(device)
+
+      result = %SnmpResult{
+        device_id: device.id,
+        job_type: :PING,
+        job_id: "ping:#{device.id}",
+        timestamp: DateTime.to_unix(DateTime.utc_now()),
+        oid_values: %{}
+      }
+
+      push(socket, "result", encode_payload(result))
+
+      # Channel should still respond to subsequent messages
+      ref = push(socket, "heartbeat", encode_payload(build_heartbeat()))
+      refute_reply ref, :error
+    end
+  end
+
+  # ── live poll with invalid job_id ───────────────────────────────
+
+  describe "handle_live_poll_result invalid job_id" do
+    test "result with malformed live_poll job_id is logged and ignored",
+         %{socket: socket, device: device} do
+      # job_id starts with "live_poll:" but doesn't have 3 parts
+      result = %SnmpResult{
+        device_id: device.id,
+        job_type: :POLL,
+        # Missing reply_topic segment - exercises the catch-all warning branch
+        job_id: "live_poll:invalid",
+        timestamp: DateTime.to_unix(DateTime.utc_now()),
+        oid_values: %{}
+      }
+
+      push(socket, "result", encode_payload(result))
+
+      ref = push(socket, "heartbeat", encode_payload(build_heartbeat()))
+      refute_reply ref, :error
+    end
+  end
+
+  # ── MikroTik job building when device matches RouterOS ─────────
+
+  describe "MikroTik job included when device sys_descr matches RouterOS" do
+    test "device with discovered MikroTik snmp_device gets a MIKROTIK job",
+         %{organization: organization} do
+      mt_device =
+        DevicesFixtures.device_fixture(%{
+          organization_id: organization.id,
+          snmp_version: "2c",
+          snmp_community: "public",
+          mikrotik_enabled: true,
+          mikrotik_username: "admin",
+          mikrotik_password: "password123",
+          mikrotik_credential_source: "device"
+        })
+
+      _snmp_device =
+        SnmpFixtures.snmp_device_fixture(%{
+          device_id: mt_device.id,
+          manufacturer: "MikroTik",
+          sys_descr: "RouterOS 7.10",
+          sys_object_id: "1.3.6.1.4.1.14988.1"
+        })
+
+      # Force needs_discovery? = true by leaving last_discovery_at nil
+      # (default), so build_jobs_for_device runs the MikroTik branch.
+
+      {:ok, agent_token, token_string} =
+        AgentsFixtures.agent_token_fixture(organization.id)
+
+      {:ok, _} = AgentsFixtures.agent_assignment_fixture(agent_token.id, mt_device.id)
+
+      {:ok, socket} = connect(AgentSocket, %{})
+
+      {:ok, _, _socket} =
+        subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string})
+
+      assert_push "jobs", %{binary: jobs_binary}
+
+      {:ok, decoded} = Base.decode64(jobs_binary)
+      {:ok, job_list} = AgentJobList.decode(decoded)
+
+      mikrotik_job = Enum.find(job_list.jobs, &(&1.job_type == :MIKROTIK))
+
+      assert mikrotik_job, "expected MIKROTIK job; got: #{inspect(Enum.map(job_list.jobs, & &1.job_type))}"
+      assert mikrotik_job.device_id == mt_device.id
+      assert mikrotik_job.job_id == "mikrotik:#{mt_device.id}"
+      assert mikrotik_job.mikrotik_device.username == "admin"
+      assert mikrotik_job.mikrotik_device.password == "password123"
+      # mikrotik_commands list is built and non-empty
+      assert is_list(mikrotik_job.mikrotik_commands)
+      assert mikrotik_job.mikrotik_commands != []
+    end
+
+    test "device sys_descr containing 'RouterOS' (without MikroTik manufacturer) still triggers MIKROTIK job",
+         %{organization: organization} do
+      mt_device =
+        DevicesFixtures.device_fixture(%{
+          organization_id: organization.id,
+          snmp_version: "2c",
+          snmp_community: "public",
+          mikrotik_enabled: true,
+          mikrotik_username: "admin",
+          mikrotik_password: "password123",
+          mikrotik_credential_source: "device"
+        })
+
+      _snmp_device =
+        SnmpFixtures.snmp_device_fixture(%{
+          device_id: mt_device.id,
+          # Manufacturer is NOT MikroTik but sys_descr mentions RouterOS
+          manufacturer: "Generic Vendor",
+          sys_descr: "Some device running RouterOS 6.x",
+          sys_object_id: "1.3.6.1.4.1.99999.1"
+        })
+
+      {:ok, agent_token, token_string} =
+        AgentsFixtures.agent_token_fixture(organization.id)
+
+      {:ok, _} = AgentsFixtures.agent_assignment_fixture(agent_token.id, mt_device.id)
+
+      {:ok, socket} = connect(AgentSocket, %{})
+
+      {:ok, _, _socket} =
+        subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string})
+
+      assert_push "jobs", %{binary: jobs_binary}
+
+      {:ok, decoded} = Base.decode64(jobs_binary)
+      {:ok, job_list} = AgentJobList.decode(decoded)
+
+      assert Enum.any?(job_list.jobs, &(&1.job_type == :MIKROTIK))
+    end
+
+    test "MikroTik device without credentials does not produce a MIKROTIK job",
+         %{organization: organization} do
+      mt_device =
+        DevicesFixtures.device_fixture(%{
+          organization_id: organization.id,
+          snmp_version: "2c",
+          snmp_community: "public",
+          # API not enabled
+          mikrotik_api_enabled: false,
+          mikrotik_username: nil,
+          mikrotik_password: nil
+        })
+
+      _snmp_device =
+        SnmpFixtures.snmp_device_fixture(%{
+          device_id: mt_device.id,
+          manufacturer: "MikroTik",
+          sys_descr: "RouterOS 7"
+        })
+
+      {:ok, agent_token, token_string} =
+        AgentsFixtures.agent_token_fixture(organization.id)
+
+      {:ok, _} = AgentsFixtures.agent_assignment_fixture(agent_token.id, mt_device.id)
+
+      {:ok, socket} = connect(AgentSocket, %{})
+
+      {:ok, _, _socket} =
+        subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string})
+
+      assert_push "jobs", %{binary: jobs_binary}
+
+      {:ok, decoded} = Base.decode64(jobs_binary)
+      {:ok, job_list} = AgentJobList.decode(decoded)
+
+      refute Enum.any?(job_list.jobs, &(&1.job_type == :MIKROTIK))
+    end
+  end
+
+  # ── poll with extra OIDs to trigger process_additional_polling_data ──
+
+  describe "process_additional_polling_data" do
+    test "poll result with extra OIDs triggers additional processing without crash",
+         %{socket: socket, device: device} do
+      device = discover_device!(device)
+      {:ok, _device} = Towerops.Devices.update_device_status(device, :up)
+
+      # Generate enough OIDs to exceed sensors + interfaces*6 threshold
+      # (which on a freshly discovered device is small).
+      extra_oids =
+        for i <- 1..50, into: %{} do
+          {"1.3.6.1.2.1.999.#{i}.0", "value-#{i}"}
+        end
+
+      result = %SnmpResult{
+        device_id: device.id,
+        job_type: :POLL,
+        job_id: "poll:#{device.id}",
+        timestamp: DateTime.to_unix(DateTime.utc_now()),
+        oid_values: extra_oids
+      }
+
+      push(socket, "result", encode_payload(result))
+
+      # The Task.Supervisor child runs async, but channel must still be alive.
+      ref = push(socket, "heartbeat", encode_payload(build_heartbeat()))
+      refute_reply ref, :error
+    end
+  end
+end