fix(engine): stop failed missions from respawning (#2736)

crates/ironclaw_engine/src/runtime/mission.rs

@@ -508,12 +508,13 @@ impl MissionManager {
         Ok(())
     }
 
-    /// Resume a paused mission.
+    /// Resume a paused or failed mission.
     ///
     /// Shared missions can only be managed by shared owners (system user).
-    /// Only `Paused` missions can be resumed — `Completed` and `Failed` are
-    /// terminal states and must not be resurrected by a stray resume call,
-    /// so anything else is rejected with a `Store` error.
+    /// `Paused` missions resume normally, and `Failed` missions may be
+    /// explicitly resumed after the caller fixes the underlying problem.
+    /// `Completed` remains terminal, so anything else is rejected with a
+    /// `Store` error.
     pub async fn resume_mission(&self, id: MissionId, user_id: &str) -> Result<(), EngineError> {
         let mut mission = self
             .store
@@ -533,10 +534,13 @@ impl MissionManager {
                 entity: format!("mission {id}"),
             });
         }
-        if mission.status != MissionStatus::Paused {
+        if !matches!(
+            mission.status,
+            MissionStatus::Paused | MissionStatus::Failed
+        ) {
             return Err(EngineError::Store {
                 reason: format!(
-                    "mission {id} is in state {:?}, only Paused missions can be resumed",
+                    "mission {id} is in state {:?}, only Paused or Failed missions can be resumed",
                     mission.status
                 ),
             });
@@ -2258,11 +2262,23 @@ async fn process_mission_outcome_and_notify(
         }
         ThreadOutcome::Completed { response: None } => {}
         ThreadOutcome::Failed { error } => {
+            // A terminal thread failure means the mission did not merely
+            // produce a disappointing result — the execution itself crashed.
+            // Leave a durable failed status so cron/event schedulers stop
+            // re-firing the same broken mission until the user explicitly
+            // resumes it after fixing the underlying problem.
+            mission.status = MissionStatus::Failed;
             mission.approach_history.push(format!("FAILED: {error}"));
             notify_response = Some(format!("Mission failed: {error}"));
             is_error = true;
         }
         ThreadOutcome::MaxIterations => {
+            // MaxIterations is also terminal for the just-fired mission run:
+            // without a failed lifecycle transition the scheduler will treat
+            // the mission as still Active and keep spawning fresh threads on
+            // every due tick, which is the runaway-loop behavior reported in
+            // #2736.
+            mission.status = MissionStatus::Failed;
             mission
                 .approach_history
                 .push("Hit max iterations without completing".into());
@@ -3506,9 +3522,9 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn resume_mission_rejects_terminal_states() {
-        // Regression: resume_mission must not resurrect Completed/Failed
-        // missions. Only Paused → Active is permitted.
+    async fn resume_mission_rejects_non_resumable_states() {
+        // Regression: resume_mission must not silently succeed for states
+        // that are not explicitly recoverable.
         let store = Arc::new(TestStore::new());
         let mgr = make_mission_manager(Arc::clone(&store) as Arc<dyn Store>);
         let project_id = ProjectId::new();
@@ -3525,7 +3541,7 @@ mod tests {
             .await
             .unwrap();
 
-        // Active → resume must fail (only Paused is resumable).
+        // Active → resume must fail (only Paused/Failed are resumable).
         let err = mgr
             .resume_mission(id, "alice")
             .await
@@ -3823,6 +3839,124 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn failed_outcome_marks_mission_failed_and_blocks_refire() {
+        let store = Arc::new(TestStore::new());
+        let mgr = make_mission_manager(Arc::clone(&store) as Arc<dyn Store>);
+        let project_id = ProjectId::new();
+
+        let id = mgr
+            .create_mission(
+                project_id,
+                "test-user",
+                "GitHub Poller",
+                "Poll the GitHub API and summarize updates",
+                MissionCadence::Cron {
+                    expression: "* * * * *".into(),
+                    timezone: None,
+                },
+                Vec::new(),
+            )
+            .await
+            .unwrap();
+
+        process_mission_outcome(
+            &(Arc::clone(&store) as Arc<dyn Store>),
+            id,
+            ThreadId::new(),
+            &ThreadOutcome::Failed {
+                error: "github api returned 404".into(),
+            },
+        )
+        .await
+        .unwrap();
+
+        let mission = mgr.get_mission(id).await.unwrap().unwrap();
+        assert_eq!(mission.status, MissionStatus::Failed);
+        assert!(
+            mission
+                .approach_history
+                .iter()
+                .any(|entry| entry.contains("github api returned 404")),
+            "failure should be recorded in approach_history"
+        );
+
+        let refire = mgr.fire_mission(id, "test-user", None).await.unwrap();
+        assert!(
+            refire.is_none(),
+            "failed missions must not keep spawning new threads until resumed"
+        );
+
+        mgr.resume_mission(id, "test-user").await.unwrap();
+        let mission = mgr.get_mission(id).await.unwrap().unwrap();
+        assert_eq!(mission.status, MissionStatus::Active);
+        assert!(
+            mission.next_fire_at.is_some(),
+            "resuming a failed cron mission should re-arm its schedule"
+        );
+
+        let refire = mgr.fire_mission(id, "test-user", None).await.unwrap();
+        assert!(
+            refire.is_some(),
+            "explicit resume should make failed missions fireable again"
+        );
+    }
+
+    #[tokio::test]
+    async fn max_iterations_marks_mission_failed_and_blocks_refire() {
+        let store = Arc::new(TestStore::new());
+        let mgr = make_mission_manager(Arc::clone(&store) as Arc<dyn Store>);
+        let project_id = ProjectId::new();
+
+        let id = mgr
+            .create_mission(
+                project_id,
+                "test-user",
+                "Long Runner",
+                "Keep checking the endpoint until it succeeds",
+                MissionCadence::Cron {
+                    expression: "* * * * *".into(),
+                    timezone: None,
+                },
+                Vec::new(),
+            )
+            .await
+            .unwrap();
+
+        process_mission_outcome(
+            &(Arc::clone(&store) as Arc<dyn Store>),
+            id,
+            ThreadId::new(),
+            &ThreadOutcome::MaxIterations,
+        )
+        .await
+        .unwrap();
+
+        let mission = mgr.get_mission(id).await.unwrap().unwrap();
+        assert_eq!(mission.status, MissionStatus::Failed);
+        assert!(
+            mission
+                .approach_history
+                .iter()
+                .any(|entry| entry.contains("max iterations")),
+            "max-iterations outcome should be recorded in approach_history"
+        );
+
+        let refire = mgr.fire_mission(id, "test-user", None).await.unwrap();
+        assert!(
+            refire.is_none(),
+            "max-iterations missions must not keep spawning new threads until resumed"
+        );
+
+        mgr.resume_mission(id, "test-user").await.unwrap();
+        let mission = mgr.get_mission(id).await.unwrap().unwrap();
+        assert_eq!(mission.status, MissionStatus::Active);
+        assert!(
+            mission.next_fire_at.is_some(),
+            "resuming a max-iterations cron mission should re-arm its schedule"
+        );
+    }
+
     #[tokio::test]
     async fn mission_evolves_via_direct_outcome_processing() {
         // Test the outcome processing directly without relying on

crates/ironclaw_engine/src/types/mission.rs

@@ -50,7 +50,10 @@ pub enum MissionStatus {
     Paused,
     /// Mission has achieved its goal.
     Completed,
-    /// Mission has been abandoned or failed irrecoverably.
+    /// Mission stopped after a terminal thread failure.
+    ///
+    /// Automatic/manual firing is blocked until the owner explicitly resumes it
+    /// after fixing the underlying problem.
     Failed,
 }