mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
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>
This commit is contained in:
parent
848089ac93
commit
4976d3c38d
2 changed files with 52 additions and 2 deletions
27
cron/jobs.py
27
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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue