From dbf9da1b2d30bc3b76fb9aed0f2e4f65f419f22d Mon Sep 17 00:00:00 2001 From: Firat Sertgoz Date: Mon, 20 Apr 2026 23:33:14 +0300 Subject: [PATCH 1/2] fix(engine): stop failed missions from respawning (#2736) --- crates/ironclaw_engine/src/runtime/mission.rs | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/crates/ironclaw_engine/src/runtime/mission.rs b/crates/ironclaw_engine/src/runtime/mission.rs index 07460f3f18a..4956eb2cd20 100644 --- a/crates/ironclaw_engine/src/runtime/mission.rs +++ b/crates/ironclaw_engine/src/runtime/mission.rs @@ -2258,11 +2258,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()); @@ -3823,6 +3835,102 @@ 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); + 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), + 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" + ); + } + + #[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); + 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), + 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" + ); + } + #[tokio::test] async fn mission_evolves_via_direct_outcome_processing() { // Test the outcome processing directly without relying on From 47e440722c33c4cf4dd825fdd741068137ea1798 Mon Sep 17 00:00:00 2001 From: Firat Sertgoz Date: Tue, 21 Apr 2026 10:17:29 +0300 Subject: [PATCH 2/2] =?UTF-8?q?fix(engine):=20address=20henrypark133=20rev?= =?UTF-8?q?iew=20=E2=80=94=20allow=20failed=20mission=20resume=20(#2760)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/ironclaw_engine/src/runtime/mission.rs | 46 +++++++++++++++---- crates/ironclaw_engine/src/types/mission.rs | 5 +- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/crates/ironclaw_engine/src/runtime/mission.rs b/crates/ironclaw_engine/src/runtime/mission.rs index 4956eb2cd20..530830158dd 100644 --- a/crates/ironclaw_engine/src/runtime/mission.rs +++ b/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 ), }); @@ -3518,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); let project_id = ProjectId::new(); @@ -3537,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 @@ -3882,6 +3886,20 @@ mod tests { 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] @@ -3929,6 +3947,14 @@ mod tests { 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] diff --git a/crates/ironclaw_engine/src/types/mission.rs b/crates/ironclaw_engine/src/types/mission.rs index 06e6201a958..af6622d9666 100644 --- a/crates/ironclaw_engine/src/types/mission.rs +++ b/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, }