Skip to content

fix(cron): one-shot jobs report 'failed' after successful completion - #63570

Closed
natehale wants to merge 1 commit into
NousResearch:mainfrom
natehale:fix/cron-oneshot-status-report
Closed

fix(cron): one-shot jobs report 'failed' after successful completion#63570
natehale wants to merge 1 commit into
NousResearch:mainfrom
natehale:fix/cron-oneshot-status-report

Conversation

@natehale

Copy link
Copy Markdown

Symptom

hermes cron run <job-id> on a one-shot job (repeat=1) prints Ran now: failed even though:

  • Agent completed and produced correct output
  • Output file was written to ~/.hermes/cron/output/
  • Scheduler logged Job '...' completed successfully

Root cause

mark_job_run() in cron/jobs.py removes one-shot jobs from jobs.json when the repeat limit is reached (line ~1544). It sets last_status="ok" on the in-memory dict first, but that dict is then popped from the list and discarded — the job no longer exists on disk.

_execute_job_now() in tools/cronjob_tools.py then reads the job back via get_job(job_id) to check last_status. For one-shot jobs this returns None, so ok = False and execution_success is incorrectly set to False.

mark_job_run(success=True)
  → sets last_status="ok" in memory
  → completed >= times → jobs.pop(i) → save_jobs  ← job deleted
  → return

_execute_job_now:
  → get_job(job_id) → None  (job removed)
  → ok = ({}).get("last_status") == "ok" → False
  → returns success=False  ← WRONG

Fix

In _execute_job_now, after run_one_job returns True, treat a missing job as one-shot completion rather than failure:

refreshed = get_job(job_id)
if refreshed is None and processed:
    # One-shot auto-removed after success — absence = completion
    return {"claimed": True, "success": True, "error": None}
_r = refreshed or {}

Also uses the _r fallback dict consistently for last_error access (defensive against a None refreshed in the error path).

Scope

  • 1 file changed (tools/cronjob_tools.py, 6 lines net)
  • No schema changes, no new dependencies
  • Does not affect recurring jobs (they still exist after mark_job_run)
  • Does not affect delivery semantics

Tests

  • 5 new tests in tests/tools/test_oneshot_status_fix.py covering:
    • One-shot removed after success → success=True
    • One-shot agent failure → success=False
    • Recurring success → success=True (unchanged)
    • Processing failure + job gone → success=False (unchanged)
    • Claim failed → claimed=False (unchanged)
  • All existing test_cronjob_run_immediate tests pass (5/5)

Edge cases

Scenario processed get_job Result
One-shot success True None (removed) success=True (fix)
One-shot agent failure True exists, status=error success=False
Recurring success True exists, status=ok success=True
Processing failure False None success=False

One-shot jobs (repeat=1) are auto-removed by mark_job_run after success.
_execute_job_now then reads the job back to check last_status, but get_job
returns None (job removed), so success is incorrectly set to False and CLI
prints 'Ran now: failed'.

Fix: when get_job returns None and run_one_job returned True, treat the
absence as one-shot completion (the job ran and was cleaned up).

Also use the _r fallback dict for last_error access to avoid a potential
AttributeError when refreshed is None.
@natehale
natehale force-pushed the fix/cron-oneshot-status-report branch from 740f140 to 54bc587 Compare July 13, 2026 04:13
@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists labels Jul 13, 2026
@natehale

Copy link
Copy Markdown
Author

Closing as duplicate of #57689 — same root cause (mark_job_run removes one-shot jobs before _execute_job_now reads back last_status), and their approach is more complete: capturing structured success/error/delivery_error at the source via instead of inferring from absence.

Our alternative fix (inferring success when get_job returns None and processed=True) is simpler but less robust — it can't surface the actual error or delivery_error. #57689 is the better vehicle for this fix.

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 P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants