mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(cron): reject past one-shot timestamps in update_job fallback + resume_job (#59395)
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.
This commit is contained in:
parent
0800af0b8a
commit
8def4ccb4f
2 changed files with 43 additions and 1 deletions
15
cron/jobs.py
15
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"],
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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)."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue