Skip to content

fix: reject stale one-shot cron jobs - #59412

Closed
itsflownium wants to merge 1 commit into
NousResearch:mainfrom
itsflownium:fix/cron-past-oneshot-59395
Closed

fix: reject stale one-shot cron jobs#59412
itsflownium wants to merge 1 commit into
NousResearch:mainfrom
itsflownium:fix/cron-past-oneshot-59395

Conversation

@itsflownium

Copy link
Copy Markdown
Contributor

Summary

  • Reject one-shot cron jobs whose scheduled time is already outside the grace window.
  • Reuse the computed next run time when persisting accepted jobs.
  • Cover both direct job creation and the cronjob tool response.

Problem
One-shot cron jobs created with a stale timestamp were accepted even though compute_next_run() returned None. The job was saved with no next run time, so it appeared valid but never executed.

Validation

  • /Users/arhaandesai/.hermes/hermes-agent/venv/bin/pytest -q tests/cron/test_jobs.py::TestJobCRUD::test_rejects_stale_past_one_shot_at_creation tests/cron/test_jobs.py::TestJobCRUD::test_recent_past_one_shot_within_grace_still_creates tests/cron/test_cron_script.py::test_cronjob_tool_rejects_stale_past_one_shot
  • /Users/arhaandesai/.hermes/hermes-agent/venv/bin/pytest -q tests/cron/test_jobs.py tests/cron/test_cron_script.py
  • /Users/arhaandesai/.hermes/hermes-agent/venv/bin/python -m py_compile cron/jobs.py tests/cron/test_jobs.py tests/cron/test_cron_script.py
  • /Users/arhaandesai/.hermes/hermes-agent/venv/bin/python -m ruff check cron/jobs.py tests/cron/test_jobs.py tests/cron/test_cron_script.py

Fixes #59395

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P1 High — major feature broken, no workaround duplicate This issue or pull request already exists labels Jul 6, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #59410 — twin PR (both open, created ~3 min apart, both fix #59395). Same mechanism: reject one-shot jobs at create_job() when compute_next_run() returns None for a past run_at. #59410 was created earlier (05:43:39 vs 05:46:22) so it's the canonical version; this one adds a logger.warning but is otherwise the same fix.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #59438 (#59438). Your fix was cherry-picked with authorship preserved (commit 848089a on main) \u2014 chosen for its broader test coverage (the tool-boundary E2E test + load_jobs()==[] persistence assertion). I folded in a follow-up: (1) the grace-window seconds in the error message (from the competing #59410 by @isheng-eqi), and (2) the same guard on update_job's schedule-change path \u2014 a review found update_job had the identical unguarded compute_next_run->next_run_at pattern, so a user could re-create the ghost job by UPDATING a schedule to a past one-shot. Both are covered by new regression tests. Thanks for the fix and the thorough tests! Closing as merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cron: create_job silently accepts past one-shot timestamps, returns success with next_run_at=null

3 participants