From 4976d3c38da3324657d650437dba954a2b107240 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 6 Jul 2026 12:00:57 +0530 Subject: [PATCH] fix(cron): guard update_job past-one-shot + enrich rejection message (#59395) Widen the #59395 fix to the sibling site: update_job's schedule-change path (cron/jobs.py) had the SAME unguarded compute_next_run -> next_run_at pattern, so updating a job's schedule to a one-shot >ONESHOT_GRACE_SECONDS in the past would re-create the ghost job (next_run_at=None, state='scheduled', never fires) that create_job now rejects. Apply the identical guard on update (raise before any disk write, so the original job is left intact), with regression tests for the reject + future-accept cases. Also surface ONESHOT_GRACE_SECONDS in the raised ValueError (not just the warning log) so a caller knows how far in the past is too far. Message from the competing PR #59410 by @isheng-eqi. Co-authored-by: isheng-eqi <265044697+isheng-eqi@users.noreply.github.com> --- cron/jobs.py | 27 +++++++++++++++++++++++++-- tests/cron/test_jobs.py | 27 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/cron/jobs.py b/cron/jobs.py index a347ab689f2..851e15b5eeb 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -988,7 +988,8 @@ def create_job( ONESHOT_GRACE_SECONDS, ) raise ValueError( - f"Requested one-shot time {run_at} is in the past and cannot be scheduled." + f"Requested one-shot time {run_at} is more than " + f"{ONESHOT_GRACE_SECONDS}s in the past and cannot be scheduled." ) job = { @@ -1150,7 +1151,29 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] updated_schedule.get("display", updated.get("schedule_display")), ) if updated.get("state") != "paused": - updated["next_run_at"] = compute_next_run(updated_schedule) + updated_next_run = compute_next_run(updated_schedule) + # Same guard as create_job: an UPDATE that sets a one-shot + # to a time >ONESHOT_GRACE_SECONDS in the past would store + # next_run_at=None with state="scheduled", re-creating the + # ghost job that never fires (#59395). Reject it here too so + # the bug can't re-enter through the update door. + if ( + updated_next_run is None + and updated_schedule.get("kind") == "once" + ): + run_at = updated_schedule.get("run_at") or updated_schedule + logger.warning( + "Rejecting one-shot cron job update '%s': run_at %s " + "is outside the %ss grace window", + updated.get("name", job_id), + run_at, + ONESHOT_GRACE_SECONDS, + ) + raise ValueError( + f"Requested one-shot time {run_at} is more than " + f"{ONESHOT_GRACE_SECONDS}s in the past and cannot be scheduled." + ) + updated["next_run_at"] = updated_next_run if inference_fields_changed: provider_snapshot, model_snapshot = _compute_provider_model_snapshots( diff --git a/tests/cron/test_jobs.py b/tests/cron/test_jobs.py index ddf1a7f2fb8..4ba0e966dc5 100644 --- a/tests/cron/test_jobs.py +++ b/tests/cron/test_jobs.py @@ -374,6 +374,33 @@ class TestUpdateJob: assert fetched["schedule"]["minutes"] == 120 assert fetched["schedule_display"] == "every 120m" + def test_update_to_past_oneshot_rejected(self, tmp_cron_dir, monkeypatch): + """Updating a job's schedule to a one-shot >ONESHOT_GRACE_SECONDS in the + past must raise ValueError — otherwise the ghost-job bug (#59395) re-enters + through the update door (next_run_at=None stored with state='scheduled'). + The original job must be left unchanged on disk.""" + now = datetime(2026, 7, 6, 12, 0, 0, tzinfo=timezone.utc) + monkeypatch.setattr("cron.jobs._hermes_now", lambda: now) + job = create_job(prompt="Recurring", schedule="every 1h", deliver="local") + past = parse_schedule((now - timedelta(minutes=10)).isoformat()) + with pytest.raises(ValueError, match="past and cannot be scheduled"): + update_job(job["id"], {"schedule": past}) + # Original job unchanged — still the recurring interval, still scheduled. + fetched = get_job(job["id"]) + assert fetched["schedule"]["kind"] == "interval" + assert fetched["next_run_at"] is not None + + def test_update_to_future_oneshot_accepted(self, tmp_cron_dir, monkeypatch): + """Updating to a FUTURE one-shot still works — only past ones are rejected.""" + now = datetime(2026, 7, 6, 12, 0, 0, tzinfo=timezone.utc) + monkeypatch.setattr("cron.jobs._hermes_now", lambda: now) + job = create_job(prompt="Recurring", schedule="every 1h", deliver="local") + future = parse_schedule((now + timedelta(hours=2)).isoformat()) + updated = update_job(job["id"], {"schedule": future}) + assert updated is not None + assert updated["schedule"]["kind"] == "once" + assert updated["next_run_at"] is not None + def test_update_enable_disable(self, tmp_cron_dir): job = create_job(prompt="Toggle me", schedule="every 1h") assert job["enabled"] is True