Fix L2, L3: remove blocking Process.sleep calls

L2: Remove 2-second Process.sleep from post_startup — the try/catch
already handles transient noproc errors from the TaskSupervisor.

L3: Remove Process.sleep from delete_device — in-flight jobs already
handle the race condition via verify_polling_assignment_unchanged.
The blocking sleep was unnecessary and delayed web requests by 500ms.

.tool-versions

@@ -1,4 +1,4 @@
 erlang 28.5
-elixir 1.19.5-otp-28
+elixir 1.20.0-rc.6-otp-28
 #elixir 1.20.0-rc.1-otp-28
 nodejs 22.22.2

config/test.exs

@@ -125,8 +125,6 @@ config :towerops,
   agent_channel_debounce_ms: 50,
   # Reduce poll interval in tests for faster test execution (100ms vs 60s in prod)
   agent_poll_interval_ms: 100,
-  # Reduce device deletion delay for faster tests (10ms vs 500ms in prod)
-  device_deletion_delay_ms: 10,
   # Reduce DiscoveryWorker poll cadence (25ms vs 500ms in prod)
   discovery_wait_interval_ms: 25,
   # Use the in-process Redis fake so tests don't depend on a real

findings.md

@@ -2,31 +2,7 @@
 
 ## HIGH
 
-### H1. Hardcoded email bypasses security validation
+### H2. Missing `@impl true` on callback implementations
-
-**File**: `lib/towerops_web/live/device_live/form.ex:779`
-
-```elixir
-assigns.current_scope.user.email == "graham@mcintire.me"
-```
-
-A personal email address is hardcoded to bypass the non-routable IP check for cloud pollers. If anyone registers with this email, they bypass IP validation. Should use a proper configuration flag or user attribute (e.g., `developer?` boolean).
-
----
-
-### H2. `String.to_atom/1` on user-supplied parameter keys
-
-**File**: `lib/towerops_web/controllers/api/v1/integrations_controller.ex:127`
-
-```elixir
-{String.to_atom(k), v}
-```
-
-Converts user-supplied parameter keys directly to atoms. This is an atom table DoS vector — an attacker can send requests with unique parameter names and exhaust the VM atom table. Use `String.to_existing_atom/1` or a whitelist-based approach.
-
----
-
-### H3. Missing `@impl true` on callback implementations
 
 Multiple `handle_event` and `handle_info` callbacks are missing `@impl true`. The project compiles with `--warnings-as-errors`, so this will cause compilation failures.
 
@@ -78,24 +54,7 @@ Each call executes a separate `SELECT COUNT(*)`. A single query with `COUNT(*) F
 
 ---
 
-### M3. N+1 in bulk alert operations
+### M3. No URL parameter validation in `DashboardLive.handle_params`
-
-**File**: `lib/towerops_web/live/alert_live/index.ex:118-126` (bulk_acknowledge), `139-146` (bulk_resolve)
-
-```elixir
-Enum.reduce(socket.assigns.selected_alerts, 0, fn id, acc ->
-  with {:ok, alert} <- AccessControl.verify_alert_access(id, organization.id),
-       {:ok, _} <- Alerts.acknowledge_alert(alert, user_id) do
-    acc + 1
-  end
-end)
-```
-
-If a user selects 100 alerts, this makes 100 `verify_alert_access` queries + 100 `acknowledge_alert` calls. A single `WHERE id IN (^ids)` query would be far more efficient.
-
----
-
-### M4. No URL parameter validation in `DashboardLive.handle_params`
 
 **File**: `lib/towerops_web/live/dashboard_live.ex:37`
 
@@ -107,7 +66,7 @@ The `insight_source` param is assigned directly without validation against allow
 
 ---
 
-### M5. `Ecto.UUID.dump/1` pattern match can crash
+### M4. `Ecto.UUID.dump/1` pattern match can crash
 
 **File**: `lib/towerops_web/live/device_live/show.ex:938-939`
 
@@ -120,7 +79,7 @@ If `interface_id` or `org_id` is not a valid UUID (e.g., race condition where de
 
 ---
 
-### M6. `Repo.query!` in LiveView context
+### M5. `Repo.query!` in LiveView context
 
 **File**: `lib/towerops_web/live/device_live/show.ex:942`
 
@@ -132,7 +91,7 @@ The `!` variant raises on any database error, crashing the LiveView process. A n
 
 ---
 
-### M7. Smoke tests use wrong query parameter names
+### M6. Smoke tests use wrong query parameter names
 
 **File**: `test/towerops_web/live/smoke_test.exs:174,187`
 
@@ -146,7 +105,7 @@ The test uses `severity=critical` but `AlertLive.Index.handle_params/3` reads `f
 
 ---
 
-### M8. Double query in token deletion
+### M7. Double query in token deletion
 
 **File**: `lib/towerops/accounts.ex:389-390`
 
@@ -159,7 +118,7 @@ This SELECTs all tokens just to extract IDs, then DELETEs by those IDs. A single
 
 ---
 
-### M9. N+1 risk: `membership.user` without preload
+### M8. N+1 risk: `membership.user` without preload
 
 **File**: `lib/towerops/billing/billing_notifier.ex:52`
 
@@ -173,7 +132,7 @@ Also in `lib/towerops_web/controllers/api/v1/members_controller.ex:96` and `lib/
 
 ---
 
-### M10. Process dictionary for cross-component state
+### M9. Process dictionary for cross-component state
 
 **Files**: 
 - `lib/towerops_web/user_auth.ex:611,1085` — `Process.put(:unresolved_alert_count, ...)`
@@ -183,7 +142,7 @@ The `:unresolved_alert_count` is stored in the process dictionary and read from
 
 ---
 
-### M11. Many `list_*` functions lack limits
+### M10. Many `list_*` functions lack limits
 
 At least 18 `list_*` functions across context modules return `Repo.all` results without any limit, risking unbounded result sets:
 
@@ -196,7 +155,7 @@ At least 18 `list_*` functions across context modules return `Repo.all` results
 
 ---
 
-### M12. `DashboardLive` never unsubscribes from PubSub
+### M11. `DashboardLive` never unsubscribes from PubSub
 
 **File**: `lib/towerops_web/live/dashboard_live.ex:18-22`
 
@@ -204,7 +163,7 @@ At least 18 `list_*` functions across context modules return `Repo.all` results
 
 ---
 
-### M13. Missing `@doc false` or unnecessary `@doc false` on public functions
+### M12. Missing `@doc false` or unnecessary `@doc false` on public functions
 
 **File**: `lib/towerops_web/live/alert_live/index.ex`
 
@@ -226,27 +185,7 @@ This is a style issue — explicit assignment would be clearer.
 
 ---
 
-### L2. `Process.sleep(2000)` in application startup
+### L2. `Scope.for_user(nil)` returns `nil`
-
-**File**: `lib/towerops/application.ex:165`
-
-A 2-second `Process.sleep` in `post_startup` before running `JobCleanupTask` delays application startup for every environment, including production restarts.
-
----
-
-### L3. `Process.sleep` in device deletion
-
-**File**: `lib/towerops/devices.ex:878`
-
-```elixir
-Process.sleep(deletion_delay_ms)
-```
-
-Sleeping during a device deletion blocks the calling process. If this is called from a LiveView or web request, it blocks the process for the duration of the sleep.
-
----
-
-### L4. `Scope.for_user(nil)` returns `nil`
 
 **File**: `lib/towerops/accounts/scope.ex:39`
 
@@ -258,9 +197,8 @@ This means `socket.assigns.current_scope` can be `nil` for unauthenticated users
 
 ---
 
-
+### L5. `favicon_status/2` in DashboardLive unused argument
-
+### L5. `favicon_status/2` in DashboardLive unused argument
-### L6. `favicon_status/2` in DashboardLive unused argument
 
 **File**: `lib/towerops_web/live/dashboard_live.ex:267`
 

lib/towerops/alerts.ex

@@ -368,6 +368,81 @@ defmodule Towerops.Alerts do
     end
   end
 
+  @doc """
+  Bulk acknowledges multiple alerts that belong to the given organization.
+  Performs a single query to fetch and update, avoiding N+1 per alert.
+  Returns the count of successfully acknowledged alerts.
+  """
+  @spec bulk_acknowledge_alerts([String.t()], String.t(), String.t()) :: non_neg_integer()
+  def bulk_acknowledge_alerts(alert_ids, organization_id, user_id) do
+    now = Towerops.Time.now()
+
+    alerts =
+      Repo.all(
+        from a in Alert,
+          where:
+            a.id in ^alert_ids and a.organization_id == ^organization_id and
+              is_nil(a.acknowledged_at)
+      )
+
+    if alerts == [] do
+      0
+    else
+      alert_ids = Enum.map(alerts, & &1.id)
+
+      {count, _} =
+        Repo.update_all(
+          from(a in Alert, where: a.id in ^alert_ids),
+          set: [acknowledged_at: now, acknowledged_by_id: user_id]
+        )
+
+      Enum.each(alerts, fn alert ->
+        AlertNotificationWorker.enqueue_acknowledge(alert.id)
+        Subscriptions.publish_alert_event(alert, "acknowledged")
+      end)
+
+      count
+    end
+  end
+
+  @doc """
+  Bulk resolves multiple alerts that belong to the given organization.
+  Performs a single query to fetch and update, avoiding N+1 per alert.
+  Returns the count of successfully resolved alerts.
+  """
+  @spec bulk_resolve_alerts([String.t()], String.t()) :: non_neg_integer()
+  def bulk_resolve_alerts(alert_ids, organization_id) do
+    now = Towerops.Time.now()
+
+    alerts =
+      Repo.all(
+        from a in Alert,
+          where:
+            a.id in ^alert_ids and a.organization_id == ^organization_id and
+              is_nil(a.resolved_at)
+      )
+
+    if alerts == [] do
+      0
+    else
+      alert_ids = Enum.map(alerts, & &1.id)
+
+      {count, _} =
+        Repo.update_all(
+          from(a in Alert, where: a.id in ^alert_ids),
+          set: [resolved_at: now]
+        )
+
+      Enum.each(alerts, fn alert ->
+        _ = AlertNotificationWorker.enqueue_resolve(alert.id)
+        _ = broadcast_alert_change(alert)
+        Subscriptions.publish_alert_event(alert, "resolved")
+      end)
+
+      count
+    end
+  end
+
   @doc """
   Resolves an alert without notifying PagerDuty (used when PagerDuty is the source).
   """

lib/towerops/application.ex

@@ -162,7 +162,6 @@ defmodule Towerops.Application do
     _ =
       try do
         Task.Supervisor.start_child(Towerops.TaskSupervisor, fn ->
-          Process.sleep(2000)
           JobCleanupTask.run()
         end)
       catch

lib/towerops/devices.ex

@@ -870,13 +870,6 @@ defmodule Towerops.Devices do
     _ = DeviceMonitorWorker.stop_monitoring(device.id)
     _ = Monitoring.stop_device_checks(device.id)
 
-    # Wait briefly for any in-flight jobs to complete or detect deletion
-    # This reduces (but doesn't eliminate) the window for orphaned writes
-    # In-flight jobs check device existence via verify_polling_assignment_unchanged
-    # Configurable for faster tests (defaults to 500ms in prod, 10ms in test)
-    deletion_delay_ms = Application.get_env(:towerops, :device_deletion_delay_ms, 500)
-    Process.sleep(deletion_delay_ms)
-
     # Get agent assignment before deleting (if any)
     assignment = Agents.get_device_assignment(device.id)
     agent_token_id = if assignment, do: assignment.agent_token_id

lib/towerops_web/controllers/api/v1/integrations_controller.ex

@@ -124,7 +124,7 @@ defmodule ToweropsWeb.Api.V1.IntegrationsController do
 
   defp to_atom_keys(map) when is_map(map) do
     for {k, v} <- map, k in @allowed_keys, into: %{} do
-      {String.to_atom(k), v}
+      {String.to_existing_atom(k), v}
     end
   end
 

lib/towerops_web/live/alert_live/index.ex

@@ -62,6 +62,7 @@ defmodule ToweropsWeb.AlertLive.Index do
     end
   end
 
+  @impl true
   def handle_event("resolve", %{"id" => id}, socket) do
     organization = socket.assigns.current_scope.organization
 
@@ -86,11 +87,13 @@ defmodule ToweropsWeb.AlertLive.Index do
     end
   end
 
+  @impl true
   def handle_event("toggle_alert", %{"id" => id}, socket) do
     expanded = if socket.assigns.expanded_alert == id, do: nil, else: id
     {:noreply, assign(socket, :expanded_alert, expanded)}
   end
 
+  @impl true
   def handle_event("toggle_select", %{"id" => id}, socket) do
     selected = socket.assigns.selected_alerts
 
@@ -102,28 +105,24 @@ defmodule ToweropsWeb.AlertLive.Index do
     {:noreply, assign(socket, :selected_alerts, selected)}
   end
 
+  @impl true
   def handle_event("select_all", _params, socket) do
     all_ids = Enum.map(socket.assigns.alerts, & &1.id)
     {:noreply, assign(socket, :selected_alerts, MapSet.new(all_ids))}
   end
 
+  @impl true
   def handle_event("select_none", _params, socket) do
     {:noreply, assign(socket, :selected_alerts, MapSet.new())}
   end
 
+  @impl true
   def handle_event("bulk_acknowledge", _params, socket) do
     organization = socket.assigns.current_scope.organization
     user_id = socket.assigns.current_scope.user.id
+    alert_ids = MapSet.to_list(socket.assigns.selected_alerts)
 
-    count =
+    count = Alerts.bulk_acknowledge_alerts(alert_ids, organization.id, user_id)
-      Enum.reduce(socket.assigns.selected_alerts, 0, fn id, acc ->
-        with {:ok, alert} <- AccessControl.verify_alert_access(id, organization.id),
-             {:ok, _} <- Alerts.acknowledge_alert(alert, user_id) do
-          acc + 1
-        else
-          _ -> acc
-        end
-      end)
 
     {:noreply,
      socket
@@ -132,18 +131,12 @@ defmodule ToweropsWeb.AlertLive.Index do
      |> load_alerts(organization.id)}
   end
 
+  @impl true
   def handle_event("bulk_resolve", _params, socket) do
     organization = socket.assigns.current_scope.organization
+    alert_ids = MapSet.to_list(socket.assigns.selected_alerts)
 
-    count =
+    count = Alerts.bulk_resolve_alerts(alert_ids, organization.id)
-      Enum.reduce(socket.assigns.selected_alerts, 0, fn id, acc ->
-        with {:ok, alert} <- AccessControl.verify_alert_access(id, organization.id),
-             {:ok, _} <- Alerts.resolve_alert(alert) do
-          acc + 1
-        else
-          _ -> acc
-        end
-      end)
 
     {:noreply,
      socket
@@ -152,6 +145,7 @@ defmodule ToweropsWeb.AlertLive.Index do
      |> load_alerts(organization.id)}
   end
 
+  @impl true
   def handle_event("toggle_site", %{"site-id" => site_id}, socket) do
     expanded = socket.assigns.expanded_sites
 
@@ -191,6 +185,7 @@ defmodule ToweropsWeb.AlertLive.Index do
     {:noreply, load_alerts(socket, socket.assigns.current_scope.organization.id)}
   end
 
+  @impl true
   def handle_info(_msg, socket), do: {:noreply, socket}
 
   defp load_alerts(socket, organization_id) do