From 8def4ccb4ffceefb041a61382734006f82524a83 Mon Sep 17 00:00:00 2001 From: isheng Date: Mon, 6 Jul 2026 12:25:13 +0530 Subject: [PATCH] fix(cron): reject past one-shot timestamps in update_job fallback + resume_job (#59395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the #59395 bug-class fix. create_job and update_job's schedule-change path already reject past one-shots (via #59410/#59438); this closes the two remaining doors that stored next_run_at=None for a 'once' schedule and re-created the silent ghost job: 1. update_job fallback-recompute (the safety-net that re-derives next_run_at when it's missing on an enabled, non-paused job) 2. resume_job (resuming a paused one-shot whose time has already passed — empirically confirmed to create a scheduled job that never fires) The redundant update_job schedule-change hunk from the original PR was dropped (already on main via #59438). Adds resume-reject + update-reject/ accept regression tests. Salvaged from #59428 by isheng-eqi. --- cron/jobs.py | 15 ++++++++++++++- tests/cron/test_jobs.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/cron/jobs.py b/cron/jobs.py index 851e15b5eeb..62633ff2d73 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -1186,7 +1186,14 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] updated["model_snapshot"] = model_snapshot if updated.get("enabled", True) and updated.get("state") != "paused" and not updated.get("next_run_at"): - updated["next_run_at"] = compute_next_run(updated["schedule"]) + next_run = compute_next_run(updated["schedule"]) + if next_run is None and updated["schedule"].get("kind") == "once": + run_at = updated["schedule"].get("run_at", "unknown") + raise ValueError( + f"Requested one-shot time {run_at} is in the past " + f"(grace window: {ONESHOT_GRACE_SECONDS}s) and cannot be scheduled." + ) + updated["next_run_at"] = next_run jobs[i] = updated save_jobs(jobs) @@ -1217,6 +1224,12 @@ def resume_job(job_id: str) -> Optional[Dict[str, Any]]: return None next_run_at = compute_next_run(job["schedule"]) + if next_run_at is None and job["schedule"].get("kind") == "once": + run_at = job["schedule"].get("run_at", "unknown") + raise ValueError( + f"Cannot resume: one-shot time {run_at} is in the past " + f"(grace window: {ONESHOT_GRACE_SECONDS}s) and will never fire." + ) return update_job( job["id"], { diff --git a/tests/cron/test_jobs.py b/tests/cron/test_jobs.py index 4ba0e966dc5..67f3e772f18 100644 --- a/tests/cron/test_jobs.py +++ b/tests/cron/test_jobs.py @@ -444,6 +444,35 @@ class TestPauseResumeJob: assert resumed["paused_at"] is None assert resumed["paused_reason"] is None + def test_resume_rejects_past_oneshot(self, tmp_cron_dir, monkeypatch): + """Resuming a paused one-shot whose time is now in the past must raise + ValueError — the revived job would silently never fire.""" + now = datetime(2026, 7, 6, 12, 0, 0, tzinfo=timezone.utc) + monkeypatch.setattr("cron.jobs._hermes_now", lambda: now) + # Create directly — bypass create_job's past-oneshot guard so we can + # test the resume path independently. + job = { + "id": "test-resume-past", + "name": "test-resume-past", + "prompt": "Past one-shot", + "schedule": {"kind": "once", "run_at": (now - timedelta(minutes=5)).isoformat(), "display": "once"}, + "repeat": {"times": 1, "completed": 0}, + "enabled": False, + "state": "paused", + "paused_at": now.isoformat(), + "paused_reason": "test", + "next_run_at": None, + "last_run_at": None, + "last_status": None, + "last_error": None, + "last_delivery_error": None, + "created_at": (now - timedelta(hours=1)).isoformat(), + "deliver": "local", + } + save_jobs([job]) + with pytest.raises(ValueError, match="in the past"): + resume_job("test-resume-past") + class TestResolveJobRef: """Name-based job lookup for CLI/tool callers (PR #2627, @buntingszn)."""