fix(cron): make cronjob(action='run') non-blocking (#52705) - #52720
fix(cron): make cronjob(action='run') non-blocking (#52705)#52720Tranquil-Flow wants to merge 1 commit into
Conversation
action='run' called run_one_job synchronously inside the tool handler, blocking the calling agent until the cron session finished. Now uses two non-blocking paths: trigger_job (arms for ticker) when the gateway is alive, or claim_job_for_fire + daemon thread when no ticker is running.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the real blocking path: current main still calls run_one_job() inline from tools/cronjob_tools.py:641.
Problems
tools/cronjob_tools.py:580usestrigger_job()whenever a gateway PID is present.cron/jobs.py:1433-1441makestrigger_job()re-enable paused jobs, while current manual-run logic intentionally rejects paused/disabled jobs attools/cronjob_tools.py:630-637.- PID liveness is not scheduler liveness:
gateway/status.py:1535-1541only checks for a running PID. Chronos intentionally has no in-process tick loop (plugins/cron_providers/chronos/__init__.py:103-113), and this trigger path does not call provider reconciliation. - The daemon at
tools/cronjob_tools.py:588-590is not durable for a standalone CLI invocation: the process may exit immediately after returning fromhermes cron run.
Suggested changes
- Keep the existing claim/precondition behavior and use a scheduler-owned, lifecycle-managed dispatch path rather than
trigger_job()plus a per-call daemon thread. - Cover paused jobs, external providers, and CLI/no-gateway process lifetime.
This is an automated hermes-sweeper review.
| """ | ||
| if _is_gateway_active(): | ||
| # Path 1: fire-and-forget — the ticker will fire it within ~60 s. | ||
| trigger_job(job_id) |
There was a problem hiding this comment.
is_gateway_running() only proves that a gateway PID exists, not that an in-process ticker is available. This also calls trigger_job(), which re-enables paused jobs; current manual-run behavior intentionally rejects paused/disabled jobs. Please route through a provider-aware dispatch path that preserves the existing claim/precondition semantics.
| "error": "Job is already being fired; not run again."} | ||
|
|
||
| threading.Thread( | ||
| target=_run_job_in_background, args=(job,), daemon=True |
There was a problem hiding this comment.
A daemon thread is abandoned when a standalone hermes cron run process exits, so the no-gateway path does not guarantee execution. Use work owned by a process/service whose lifetime is explicitly managed, and add a process-lifecycle regression test.
|
Thanks @Tranquil-Flow — you were the first to take a run at #52705, and your diagnosis was exactly right: This is now fixed on main via #80807, which takes a different mechanism than the trigger-next-tick / daemon-thread split here: the run dispatches through the async-delegation rail (same as |
Problem
cronjob(action="run")executes the job synchronously and inline inside the tool handler via_execute_job_now()→run_one_job(). The calling agent (or CLI) is blocked until the entire cron session — LLM loop, output save, delivery — completes before the tool returns.This was introduced by #50025 (commit
65d7c7faf) which fixed #41037 (jobs never executing when no ticker was alive) by replacing the fire-and-forgettrigger_jobwith synchronous_execute_job_now.Fix
Two-path non-blocking design (as suggested in the issue):
Gateway alive →
trigger_job(job_id)setsnext_run_at = now; the scheduler ticker fires it asynchronously viaThreadPoolExecutor.submit(run_one_job)on its next cycle (~60s). At-most-once handled by the ticker's ownclaim_job_for_fire.Gateway not alive (CLI/oneshot) →
claim_job_for_fire(job_id)+ daemon thread that callsrun_one_job()directly. The thread runs the job to completion while the tool returns immediately.Either way,
run_one_jobis never called synchronously inside the tool handler. The response reportstriggered: true,trigger_mode: "scheduled"|"background", andexecuted(whether a claim/trigger was taken), directing the caller tocronjob(action="list")for post-run status.Reliability & at-most-once
claim_job_for_fire(atomic CAS that advancesnext_run_at), preventing double-fire if a ticker starts concurrently. Path 1 relies on the ticker's own claim.mark_job_run(job_id, False, error)so the claim does not wedge.Tests
10 new tests in
tests/tools/test_cronjob_run_immediate.py:trigger_job, never callsrun_one_jobsynchronouslydaemon=Truetriggered+trigger_modepresent,execution_successabsentrun_one_jobnever called synchronously regardless of gateway stateAll 10 pass. Broader cron+tools suite: 84/84 pass, no regressions.
Files changed
tools/cronjob_tools.py: replaced_execute_job_nowwith_trigger_job_nonblocking+_is_gateway_active+_run_job_in_backgroundhermes_cli/cron.py: updated CLI display for trigger modetests/tools/test_cronjob_run_immediate.py: 10 non-blocking behavior testsAuto-published by Moonsong via Path B automated pipeline.