fix(cron): stop one-shot At jobs from re-firing every poll (#5039)

This commit is contained in:
mysma-9403
2026-07-23 16:54:25 +03:00
committed by GitHub
parent 21ec09742c
commit d46509612f
2 changed files with 52 additions and 4 deletions
+12 -4
View File
@@ -1186,13 +1186,21 @@ async fn persist_job_result(
duration_ms,
);
if is_one_shot_auto_delete(job) {
if success {
// A fixed-instant (`Schedule::At`) job is inherently one-shot: its `at` is in
// the past the moment it runs, so `reschedule_after_run` (which writes
// next_run = at for an `At` schedule) leaves next_run <= now and the job is
// re-selected by `due_jobs` on every poll, re-executing forever. Terminate
// every `At` job after a single run, regardless of `delete_after_run`. Only an
// auto-delete job that succeeded is removed; everything else is kept disabled
// so its run history stays inspectable. (Inside this `At` branch
// `is_one_shot_auto_delete` reduces to `job.delete_after_run`.)
if matches!(job.schedule, Schedule::At { .. }) {
if is_one_shot_auto_delete(job) && success {
if let Err(e) = remove_job(config, &job.id) {
tracing::warn!("Failed to remove one-shot cron job after success: {e}");
}
} else {
let _ = record_last_run(config, &job.id, finished_at, false, output);
let _ = record_last_run(config, &job.id, finished_at, success, output);
if let Err(e) = update_job(
config,
&job.id,
@@ -1201,7 +1209,7 @@ async fn persist_job_result(
..CronJobPatch::default()
},
) {
tracing::warn!("Failed to disable failed one-shot cron job: {e}");
tracing::warn!("Failed to disable one-shot cron job: {e}");
}
}
return success;
+40
View File
@@ -1143,6 +1143,46 @@ async fn persist_job_result_failure_disables_one_shot() {
assert_eq!(updated.last_status.as_deref(), Some("error"));
}
#[tokio::test]
async fn persist_job_result_disables_at_job_without_delete_flag() {
// Regression: an `At` job created without delete_after_run (the RPC default,
// and every shell `At` job) must not be rescheduled after it runs. Its `at`
// is a fixed instant, so reschedule_after_run would write next_run = at
// (now in the past) and due_jobs would re-select it on every poll, re-firing
// the job forever.
let tmp = TempDir::new().unwrap();
let config = test_config(&tmp).await;
let at = Utc::now() + ChronoDuration::minutes(10);
let job = cron::add_agent_job(
&config,
Some("at-no-delete".into()),
crate::openhuman::cron::Schedule::At { at },
"Hello",
SessionTarget::Isolated,
None,
None,
false, // delete_after_run = false — the previously-buggy case
)
.unwrap();
let started = Utc::now();
let finished = started + ChronoDuration::milliseconds(10);
let success = persist_job_result(&config, &job, true, "ok", started, finished).await;
assert!(success);
// The row is kept (not auto-deleted) but disabled, and its run is recorded.
let updated = cron::get_job(&config, &job.id).unwrap();
assert!(!updated.enabled, "At job must be disabled after one run");
assert_eq!(updated.last_status.as_deref(), Some("ok"));
// It is never due again — even at a time past its `at` instant.
let due = cron::due_jobs(&config, at + ChronoDuration::minutes(1)).unwrap();
assert!(
!due.iter().any(|j| j.id == job.id),
"disabled At job must not be re-selected by due_jobs"
);
}
#[tokio::test]
async fn deliver_if_configured_skips_non_announce_mode() {
let tmp = TempDir::new().unwrap();