fix: check state never persisted due to missing cast fields (#73)

Check.changeset/2 did not include state fields (current_state,
current_state_type, current_check_attempt, last_check_at, etc.) in
its cast list. Ecto silently dropped all state updates, so
update_check_state succeeded but wrote nothing to the database.

Added Check.state_changeset/2 for internal state machine updates,
keeping the regular changeset safe from user input. Updated
Monitoring.update_check_state/3 to use it directly.

Reviewed-on: graham/towerops-web#73

lib/towerops/monitoring.ex

@@ -287,7 +287,17 @@ defmodule Towerops.Monitoring do
         attrs
       end
 
-    update_check(check, attrs)
+    result =
+      check
+      |> Check.state_changeset(attrs)
+      |> Repo.update()
+
+    case result do
+      {:ok, updated} -> broadcast_check_change(updated)
+      _ -> :ok
+    end
+
+    result
   end
 
   defp calculate_state_transition(check, _new_status, true = _state_changed) do

lib/towerops/monitoring/check.ex

@@ -197,6 +197,26 @@ defmodule Towerops.Monitoring.Check do
     |> Base.url_encode64(padding: false)
   end
 
+  @doc """
+  Changeset for internal state updates (soft/hard state transitions).
+
+  Allows updating check execution state fields that are not permitted
+  through the regular changeset (which protects them from user input).
+  """
+  def state_changeset(check, attrs) do
+    cast(check, attrs, [
+      :current_state,
+      :current_state_type,
+      :current_check_attempt,
+      :last_check_at,
+      :last_state_change_at,
+      :last_hard_state_change_at,
+      :is_flapping,
+      :flapping_state_changes,
+      :flapping_window_start_at
+    ])
+  end
+
   def state_label(0), do: "OK"
   def state_label(1), do: "WARNING"
   def state_label(2), do: "CRITICAL"

lib/towerops/workers/check_executor_worker.ex

@@ -143,7 +143,7 @@ defmodule Towerops.Workers.CheckExecutorWorker do
       {:ok, response_time, output} ->
         {:ok,
          %{
-           value: nil,
+           value: response_time,
            status: 0,
            output: output,
            response_time_ms: response_time
@@ -160,7 +160,7 @@ defmodule Towerops.Workers.CheckExecutorWorker do
       {:ok, response_time, output} ->
         {:ok,
          %{
-           value: nil,
+           value: response_time,
            status: 0,
            output: output,
            response_time_ms: response_time
@@ -177,7 +177,7 @@ defmodule Towerops.Workers.CheckExecutorWorker do
       {:ok, response_time, output} ->
         {:ok,
          %{
-           value: nil,
+           value: response_time,
            status: 0,
            output: output,
            response_time_ms: response_time
@@ -194,7 +194,7 @@ defmodule Towerops.Workers.CheckExecutorWorker do
       {:ok, status, response_time, output} ->
         {:ok,
          %{
-           value: nil,
+           value: response_time,
            status: status,
            output: output,
            response_time_ms: response_time
@@ -211,7 +211,7 @@ defmodule Towerops.Workers.CheckExecutorWorker do
       {:ok, response_time, output} ->
         {:ok,
          %{
-           value: nil,
+           value: response_time,
            status: 0,
            output: output,
            response_time_ms: response_time

test/towerops/monitoring_test.exs

@@ -423,27 +423,49 @@ defmodule Towerops.MonitoringTest do
   end
 
   describe "update_check_state/3" do
-    # NOTE: Check.changeset does not cast state fields (current_state,
-    # current_state_type, current_check_attempt, last_check_at, etc.).
-    # update_check_state computes correct transitions but the changeset
-    # silently drops them. These tests document the actual behavior.
-
-    test "succeeds but state fields are not persisted via changeset", %{organization: org} do
+    test "persists state fields to database", %{organization: org} do
       {:ok, check} = Monitoring.create_check(valid_check_attrs(org.id))
 
-      # The function returns {:ok, check} but state fields remain at defaults
       assert {:ok, updated} = Monitoring.update_check_state(check, 0, "OK")
-      # current_state stays at 3 (default) because changeset doesn't cast it
-      assert updated.current_state == 3
-      assert updated.current_state_type == "soft"
+      assert updated.current_state == 0
+      assert updated.last_check_at
+
+      # Verify persisted to DB
+      reloaded = Monitoring.get_check!(check.id)
+      assert reloaded.current_state == 0
+      assert reloaded.last_check_at
     end
 
-    test "does not crash on repeated calls", %{organization: org} do
+    test "tracks soft/hard state transitions", %{organization: org} do
+      {:ok, check} = Monitoring.create_check(valid_check_attrs(org.id))
+      # Default: state=3 (UNKNOWN), type="soft", attempt=1, max_attempts=3
+
+      # First call: state changes 3->0, attempt=2, still soft
+      assert {:ok, check} = Monitoring.update_check_state(check, 0, "OK")
+      assert check.current_state == 0
+      assert check.current_state_type == "soft"
+      assert check.current_check_attempt == 2
+
+      # Second call: same state (0->0), attempt reaches 3 (max), becomes hard
+      assert {:ok, check} = Monitoring.update_check_state(check, 0, "OK")
+      assert check.current_state == 0
+      assert check.current_state_type == "hard"
+      assert check.current_check_attempt == 3
+
+      # Third call: hard state stays hard
+      assert {:ok, check} = Monitoring.update_check_state(check, 0, "OK")
+      assert check.current_state == 0
+      assert check.current_state_type == "hard"
+    end
+
+    test "repeated calls work correctly", %{organization: org} do
       {:ok, check} = Monitoring.create_check(valid_check_attrs(org.id))
 
       assert {:ok, check} = Monitoring.update_check_state(check, 0, "OK")
       assert {:ok, check} = Monitoring.update_check_state(check, 2, "CRITICAL")
-      assert {:ok, _check} = Monitoring.update_check_state(check, 0, "OK again")
+      assert check.current_state == 2
+      assert {:ok, check} = Monitoring.update_check_state(check, 0, "OK again")
+      assert check.current_state == 0
     end
   end