fix: stamp finished_at for CandidateRecoverable terminal runs in set_run_state (#9069)

set_run_state's finished_at condition hand-listed 5 of the 6 terminal run
states, omitting CandidateRecoverable. A run that finished with a recoverable
candidate goes through set_run_state (via run_state_for_aggregate in
failure_recording) but never got its lifecycle.execution.finished_at stamped --
even though it is terminal and the legacy-record migration path in health.rs
stamps finished_at for every non-Queued/Running state. The two write paths
disagreed: the exact drift the just-merged is_terminal() consolidation (#9068)
made visible.

Replace the hand-listed subset with state.is_terminal() so the live setter
matches the canonical terminal definition and the migration path. This removes
the last hand-copied terminal-state list.

Regression test asserts a CandidateRecoverable run stamps finished_at and a
Running run does not; verified it FAILS on the pre-fix setter (panics with 'a
terminal CandidateRecoverable run must stamp finished_at') and passes with the
fix.

Verified: agents --lib clean; new test passes with the fix, fails without it.

Author: chubes4

crates/homeboy-agents/src/agent_task_lifecycle/lifecycle_record_ops.rs

@@ -27,14 +27,14 @@ pub(crate) fn set_run_state(record: &mut AgentTaskRunRecord, state: AgentTaskRun
     if state == AgentTaskRunState::Running && record.lifecycle.execution.started_at.is_none() {
         record.lifecycle.execution.started_at = Some(timestamp.clone());
     }
-    if matches!(
-        state,
-        AgentTaskRunState::Succeeded
-            | AgentTaskRunState::PartialRecoverable
-            | AgentTaskRunState::PartialFailure
-            | AgentTaskRunState::Failed
-            | AgentTaskRunState::Cancelled
-    ) {
+    // A terminal run has finished executing, so stamp `finished_at`. Use the
+    // canonical terminal set (`is_terminal`) rather than a hand-listed subset:
+    // the previous inline list omitted `CandidateRecoverable`, so a run that
+    // finished with a recoverable candidate never got a `finished_at` here —
+    // while the legacy-record migration path (`health.rs`) stamps it for every
+    // non-Queued/Running state. This aligns the live setter with that path and
+    // with the single terminal definition.
+    if state.is_terminal() {
         record.lifecycle.execution.finished_at = Some(timestamp.clone());
     }
     record.lifecycle.updated_at = Some(timestamp);

crates/homeboy-agents/src/agent_task_lifecycle/tests/status_and_recovery.rs

@@ -1311,3 +1311,37 @@ fn logs_include_normalized_event_envelopes() {
         assert_eq!(log.normalized_events[1].artifact_refs.len(), 1);
     });
 }
+
+#[test]
+fn set_run_state_stamps_finished_at_for_candidate_recoverable_terminal_runs() {
+    // A run that finished with a recoverable candidate is terminal, so
+    // set_run_state must stamp finished_at for it exactly as it does for the
+    // other terminal states. Regression guard for the drift where the setter's
+    // hand-listed terminal subset omitted CandidateRecoverable, leaving these
+    // runs without a finished_at while the legacy-record migration path stamped
+    // one.
+    with_isolated_home(|_| {
+        let record =
+            submit_plan(&test_plan(), Some("candidate-recoverable-finished-at")).expect("submit");
+        rewrite_record_for_test(&record.run_id, |record| {
+            set_run_state(record, AgentTaskRunState::CandidateRecoverable);
+            assert_eq!(record.state, AgentTaskRunState::CandidateRecoverable);
+            assert!(
+                record.lifecycle.execution.finished_at.is_some(),
+                "a terminal CandidateRecoverable run must stamp finished_at"
+            );
+        })
+        .expect("rewrite record");
+
+        // And a non-terminal state must NOT stamp finished_at.
+        rewrite_record_for_test(&record.run_id, |record| {
+            record.lifecycle.execution.finished_at = None;
+            set_run_state(record, AgentTaskRunState::Running);
+            assert!(
+                record.lifecycle.execution.finished_at.is_none(),
+                "a non-terminal Running run must not stamp finished_at"
+            );
+        })
+        .expect("rewrite record running");
+    });
+}