fix(engine): stop failed missions from respawning (#2736) - #2760
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the mission processing logic to set the mission status to Failed when a thread outcome results in a failure or reaches maximum iterations. This change prevents the scheduler from continuously re-firing broken missions, resolving a reported runaway-loop issue. Additionally, unit tests were added to verify that these outcomes correctly block subsequent refires. I have no feedback to provide.
henrypark133
left a comment
There was a problem hiding this comment.
Findings:
- High -
crates/ironclaw_engine/src/runtime/mission.rs:2260,crates/ironclaw_engine/src/runtime/mission.rs:517,crates/ironclaw_engine/src/types/mission.rs:267,src/channels/web/handlers/engine.rs:193: the fix stops refiring by marking failed/max-iteration missionsFailed, butFailedis terminal,fire_missionrefuses terminal missions, and the only exposed recovery control still callsresume_mission, which only acceptsPaused. That means the mission is no longer resumable through the public API after one failed run, despite the intended "stop until resumed" behavior.
Summary:
- Recommended verdict: Request changes
- Residual risk: I did not run the local test suite; this review is based on code inspection plus the current PR checks.
serrrfirat
left a comment
There was a problem hiding this comment.
Reviewed the latest mission changes. Allowing resume_mission to recover both Paused and Failed missions closes the API dead-end from the earlier review, and the new regression tests around failed/max-iteration outcomes cover the re-fire guard well.
One low-severity follow-up: when process_mission_outcome flips a mission to Failed, it leaves the mission id in the in-memory active / last_fire_attempt bookkeeping, unlike the pause/complete paths. Status checks still prevent refires, so this looks harmless, but it does leave stale runtime bookkeeping around until restart.
…ai#2760) * fix(engine): stop failed missions from respawning (nearai#2736) * fix(engine): address henrypark133 review — allow failed mission resume (nearai#2760)
Summary
Failedwhen a mission thread ends in a terminal failure or hits max iterationsCompletedRoot cause
Mission thread failures were recorded in
approach_history, but the mission lifecycle itself stayedActive. Because active cron/event missions remain eligible for future firings, the scheduler kept spawning fresh threads after each broken run, inflating the Missions count.Testing
Closes #2736