Fix DB pool exhaustion and settings page crashes (#88)

## Summary
- **DB pool exhaustion**: `device_has_effective_agent?` was doing 2 DB queries per call (3-table join + global settings lookup), called 2-3x per worker cycle across hundreds of devices (~6000 unnecessary queries/minute). Added an ETS-backed `AgentCache` with 120s TTL and removed redundant calls within the same worker cycle.
- **Settings page crash**: `KeyError: key :user_agent not found` on BrowserSession — field doesn't exist, replaced with `device_type`.
- **Login history IP truncation**: Changed from side-by-side grid to stacked table layout so IP addresses aren't clipped.

## Test plan
- [x] 8 new tests for AgentCache (cache hit, miss, invalidation, global default)
- [x] Full test suite passes (8651 tests, 0 failures, verified across multiple runs)
- [x] `mix format` and pre-commit hooks pass

Reviewed-on: graham/towerops-web#88

lib/towerops/agents.ex

@@ -9,6 +9,7 @@ defmodule Towerops.Agents do
   import Ecto.Query, warn: false
 
   alias Towerops.Agents.AgentAssignment
+  alias Towerops.Agents.AgentCache
   alias Towerops.Agents.AgentToken
   alias Towerops.Agents.ReleaseChecker
   alias Towerops.Devices.Device
@@ -1022,6 +1023,17 @@ defmodule Towerops.Agents do
   """
   @spec device_has_effective_agent?(Ecto.UUID.t()) :: boolean()
   def device_has_effective_agent?(device_id) do
+    AgentCache.device_has_effective_agent?(device_id)
+  end
+
+  @doc """
+  Queries the database directly to check if a device has an effective agent.
+
+  This is the uncached version used by `AgentCache`. Callers should use
+  `device_has_effective_agent?/1` which reads from the ETS cache.
+  """
+  @spec query_device_has_effective_agent?(Ecto.UUID.t()) :: boolean()
+  def query_device_has_effective_agent?(device_id) do
     query =
       from d in Device,
         left_join: aa in AgentAssignment,
@@ -1043,7 +1055,7 @@ defmodule Towerops.Agents do
         result.has_device_assignment or
           result.has_site_token or
           result.has_org_token or
-          Towerops.Settings.get_global_default_cloud_poller() != nil
+          AgentCache.global_default_cloud_poller() != nil
     end
   end
 

lib/towerops/agents/agent_cache.ex

@@ -0,0 +1,92 @@
+defmodule Towerops.Agents.AgentCache do
+  @moduledoc """
+  ETS-backed cache for device agent assignment lookups.
+
+  Reduces database load by caching the result of `device_has_effective_agent?/1`
+  queries and the global default cloud poller setting. These values are checked
+  on every check/poll cycle for every device, generating thousands of redundant
+  queries per minute.
+
+  Cache entries expire after a configurable TTL (default 2 minutes). Explicit
+  invalidation is available for immediate updates when assignments change.
+  """
+
+  use GenServer
+
+  alias Towerops.Agents
+  alias Towerops.Settings
+
+  @table :device_agent_cache
+  @ttl_ms to_timeout(second: 120)
+
+  # Client API
+
+  def start_link(opts \\ []) do
+    GenServer.start_link(__MODULE__, opts, name: __MODULE__)
+  end
+
+  @doc """
+  Checks if a device has any effective agent assigned, using cached result when available.
+
+  Returns cached value if within TTL, otherwise queries the database and caches the result.
+  """
+  @spec device_has_effective_agent?(Ecto.UUID.t()) :: boolean()
+  def device_has_effective_agent?(device_id) do
+    now = System.monotonic_time(:millisecond)
+
+    case :ets.lookup(@table, {:device, device_id}) do
+      [{_, result, expires_at}] when now < expires_at ->
+        result
+
+      _ ->
+        result = Agents.query_device_has_effective_agent?(device_id)
+        :ets.insert(@table, {{:device, device_id}, result, now + @ttl_ms})
+        result
+    end
+  end
+
+  @doc """
+  Returns the cached global default cloud poller value.
+
+  This is cached in ETS since it rarely changes and is read on every
+  `device_has_effective_agent?` call.
+  """
+  @spec global_default_cloud_poller() :: String.t() | nil
+  def global_default_cloud_poller do
+    case :ets.lookup(@table, :global_default) do
+      [{:global_default, value}] -> value
+      [] -> refresh_global_default()
+    end
+  end
+
+  @doc "Refreshes the cached global default cloud poller from the database."
+  @spec refresh_global_default() :: String.t() | nil
+  def refresh_global_default do
+    value = Settings.get_global_default_cloud_poller()
+    :ets.insert(@table, {:global_default, value})
+    value
+  end
+
+  @doc "Invalidates the cached agent check for a specific device."
+  @spec invalidate(Ecto.UUID.t()) :: true
+  def invalidate(device_id) do
+    :ets.delete(@table, {:device, device_id})
+  end
+
+  @doc "Invalidates all cached device agent checks."
+  @spec invalidate_all() :: true
+  def invalidate_all do
+    # Delete all device entries but keep the global default
+    :ets.match_delete(@table, {{:device, :_}, :_, :_})
+  end
+
+  # Server callbacks
+
+  @impl GenServer
+  def init(_opts) do
+    table = :ets.new(@table, [:named_table, :public, :set, read_concurrency: true])
+    # Pre-cache global default on startup
+    refresh_global_default()
+    {:ok, %{table: table}}
+  end
+end

lib/towerops/application.ex

@@ -110,6 +110,8 @@ defmodule Towerops.Application do
         {ErrorTrackerNotifier, []},
         # Redis connection for brute force protection and rate limiting
         redis_spec(),
+        # Cache for device agent assignment lookups (reduces DB load from workers)
+        Towerops.Agents.AgentCache,
         # Rate limiting backend (ETS-based, per-node)
         {Towerops.RateLimit, [clean_period: to_timeout(minute: 1)]},
         {DNSCluster, query: Application.get_env(:towerops, :dns_cluster_query) || :ignore},

lib/towerops/workers/check_worker.ex

@@ -76,13 +76,9 @@ defmodule Towerops.Workers.CheckWorker do
     end
   end
 
+  # Agent check already done by should_continue_checking?, no need to re-check
   defp maybe_perform_check(check) do
-    if should_phoenix_execute?(check) do
-      perform_check(check)
-    else
-      Logger.debug("Skipping Phoenix execution for check #{check.name} - assigned to remote agent")
-      :ok
-    end
+    perform_check(check)
   end
 
   defp schedule_next_check_with_error_handling(check_id, interval_seconds) do

lib/towerops/workers/device_monitor_worker.ex

@@ -66,21 +66,9 @@ defmodule Towerops.Workers.DeviceMonitorWorker do
       !Agents.device_has_effective_agent?(device.id)
   end
 
+  # Agent check already done by should_continue_monitoring?, no need to re-check
   defp maybe_perform_check(device) do
-    cond do
-      Client.phoenix_snmp_disabled() ->
-        :ok
-
-      not device.monitoring_enabled ->
-        :ok
-
-      Agents.device_has_effective_agent?(device.id) ->
-        Logger.info("Skipping Phoenix monitoring for device #{device.name} - device has agent assigned")
-        :ok
-
-      true ->
-        perform_check(device)
-    end
+    perform_check(device)
   end
 
   defp schedule_next_check_with_error_handling(device_id) do

lib/towerops/workers/device_poller_worker.ex

@@ -83,21 +83,9 @@ defmodule Towerops.Workers.DevicePollerWorker do
       !Agents.device_has_effective_agent?(device.id)
   end
 
+  # Agent check already done by should_continue_polling?, no need to re-check
   defp maybe_poll_device(device) do
-    cond do
-      Client.phoenix_snmp_disabled() ->
-        :ok
-
-      not device.snmp_enabled ->
-        :ok
-
-      Agents.device_has_effective_agent?(device.id) ->
-        Logger.info("Skipping Phoenix poll for device #{device.name} - device has agent assigned")
-        :ok
-
-      true ->
-        poll_device(device)
-    end
+    poll_device(device)
   end
 
   defp schedule_next_poll_with_error_handling(device_id, _stale_device) do

lib/towerops_web/live/user_settings_live.html.heex

@@ -971,17 +971,21 @@
       </div>
       
 <!-- Login History Section -->
-      <div class="grid max-w-7xl grid-cols-1 gap-x-8 gap-y-10 px-4 py-16 sm:px-6 md:grid-cols-3 lg:px-8 border-t border-gray-200 dark:border-white/10">
-        <div>
-          <h2 class="text-base/7 font-semibold text-gray-900 dark:text-white">
-            {t("Login History")}
-          </h2>
-          <p class="mt-1 text-sm/6 text-gray-500 dark:text-gray-400">
-            {t("Recent login attempts to your account, including successful and failed attempts.")}
-          </p>
+      <div class="max-w-7xl px-4 py-16 sm:px-6 lg:px-8 border-t border-gray-200 dark:border-white/10">
+        <div class="sm:flex sm:items-center">
+          <div class="sm:flex-auto">
+            <h2 class="text-base/7 font-semibold text-gray-900 dark:text-white">
+              {t("Login History")}
+            </h2>
+            <p class="mt-1 text-sm/6 text-gray-500 dark:text-gray-400">
+              {t(
+                "Recent login attempts to your account, including successful and failed attempts."
+              )}
+            </p>
+          </div>
         </div>
 
-        <div class="md:col-span-2">
+        <div class="mt-8">
           <%= if Enum.empty?(@login_history) do %>
             <div class="text-center rounded-lg border border-dashed border-gray-300 px-6 py-10 dark:border-gray-700">
               <.icon name="hero-clock" class="mx-auto h-12 w-12 text-gray-400" />
@@ -993,7 +997,7 @@
               </p>
             </div>
           <% else %>
-            <div class="overflow-hidden shadow ring-1 ring-black ring-opacity-5 dark:ring-white/10 sm:rounded-lg">
+            <div class="overflow-x-auto shadow ring-1 ring-black ring-opacity-5 dark:ring-white/10 sm:rounded-lg">
               <table class="min-w-full divide-y divide-gray-300 dark:divide-gray-700">
                 <thead class="bg-gray-50 dark:bg-gray-800/50">
                   <tr>
@@ -1023,7 +1027,7 @@
                     </th>
                     <th
                       scope="col"
-                      class="px-3 py-3.5 text-left text-sm font-semibold text-gray-900 dark:text-white"
+                      class="py-3.5 pl-3 pr-4 text-right text-sm font-semibold text-gray-900 dark:text-white sm:pr-6"
                     >
                       {t("IP Address")}
                     </th>
@@ -1059,7 +1063,7 @@
                       <td class="whitespace-nowrap px-3 py-4 text-sm text-gray-500 dark:text-gray-400">
                         {format_location(attempt)}
                       </td>
-                      <td class="whitespace-nowrap px-3 py-4 text-sm font-mono text-gray-500 dark:text-gray-400">
+                      <td class="whitespace-nowrap py-4 pl-3 pr-4 text-right text-sm font-mono text-gray-500 dark:text-gray-400 sm:pr-6">
                         {attempt.ip_address}
                       </td>
                     </tr>

test/towerops/agents/agent_cache_test.exs

@@ -0,0 +1,101 @@
+defmodule Towerops.Agents.AgentCacheTest do
+  use Towerops.DataCase
+
+  import Towerops.AccountsFixtures
+
+  alias Towerops.Agents.AgentCache
+
+  setup do
+    user = user_fixture()
+    {:ok, organization} = Towerops.Organizations.create_organization(%{name: "Test Org"}, user.id)
+
+    {:ok, site} =
+      Towerops.Sites.create_site(%{
+        name: "Test Site",
+        organization_id: organization.id
+      })
+
+    {:ok, device} =
+      Towerops.Devices.create_device(%{
+        name: "Test Device",
+        ip_address: "192.168.1.1",
+        site_id: site.id,
+        organization_id: organization.id,
+        monitoring_enabled: true
+      })
+
+    %{user: user, organization: organization, site: site, device: device}
+  end
+
+  describe "device_has_effective_agent?/1" do
+    test "returns false for device without any agent", %{device: device} do
+      refute AgentCache.device_has_effective_agent?(device.id)
+    end
+
+    test "returns cached result on second call", %{device: device} do
+      # First call populates cache
+      result1 = AgentCache.device_has_effective_agent?(device.id)
+      # Second call should return same result from cache
+      result2 = AgentCache.device_has_effective_agent?(device.id)
+
+      assert result1 == result2
+      refute result1
+    end
+
+    test "returns true when device has agent assignment", %{
+      device: device,
+      organization: organization
+    } do
+      {:ok, agent_token, _token} =
+        Towerops.Agents.create_agent_token(organization.id, "Test Agent")
+
+      {:ok, _assignment} =
+        Towerops.Agents.assign_device_to_agent(agent_token.id, device.id)
+
+      # Invalidate so we get fresh data
+      AgentCache.invalidate(device.id)
+
+      assert AgentCache.device_has_effective_agent?(device.id)
+    end
+
+    test "invalidate/1 clears cache for specific device", %{device: device} do
+      # Populate cache
+      AgentCache.device_has_effective_agent?(device.id)
+      # Invalidate
+      AgentCache.invalidate(device.id)
+
+      # Should query DB again (no crash = works)
+      refute AgentCache.device_has_effective_agent?(device.id)
+    end
+
+    test "invalidate_all/0 clears entire cache", %{device: device} do
+      # Populate cache
+      AgentCache.device_has_effective_agent?(device.id)
+      # Invalidate all
+      AgentCache.invalidate_all()
+
+      # Should query DB again
+      refute AgentCache.device_has_effective_agent?(device.id)
+    end
+  end
+
+  describe "global_default_cloud_poller/0" do
+    test "returns nil when no global default is set" do
+      assert AgentCache.global_default_cloud_poller() == nil
+    end
+
+    test "returns cached value after refresh" do
+      # First call
+      result1 = AgentCache.global_default_cloud_poller()
+      # Second call from cache
+      result2 = AgentCache.global_default_cloud_poller()
+      assert result1 == result2
+    end
+
+    test "refresh_global_default/0 updates cached value" do
+      assert AgentCache.global_default_cloud_poller() == nil
+      AgentCache.refresh_global_default()
+      assert AgentCache.global_default_cloud_poller() == nil
+    end
+  end
+end

test/towerops/agents_test.exs

@@ -6,6 +6,7 @@ defmodule Towerops.AgentsTest do
   alias Towerops.Admin.AuditLog
   alias Towerops.Agents
   alias Towerops.Agents.AgentAssignment
+  alias Towerops.Agents.AgentCache
   alias Towerops.Agents.AgentToken
 
   setup do
@@ -1884,10 +1885,16 @@ defmodule Towerops.AgentsTest do
       cloud_poller: cloud_poller
     } do
       {:ok, _} = Towerops.Settings.set_global_default_cloud_poller(cloud_poller.id)
+
+      # Refresh the cache so it picks up the new global default
+      AgentCache.refresh_global_default()
+      AgentCache.invalidate(device.id)
+
       assert Agents.device_has_effective_agent?(device.id) == true
 
       # Clean up
       {:ok, _} = Towerops.Settings.set_global_default_cloud_poller(nil)
+      AgentCache.refresh_global_default()
     end
   end