feat(cron): per-job runtime_cap_seconds with wall-clock cap enforcement - #50268
noodlemctwoodle wants to merge 1 commit into
Conversation
Cron jobs have only had an inactivity-based timeout (HERMES_CRON_TIMEOUT, process-global, default 600s) — a job that emits any activity inside the window runs forever. There is no per-job wall-clock cap, no way for a gateway/UI client to set one, and no field on the persisted job record. This adds runtime_cap_seconds end-to-end, mirroring the semantics of kanban tasks' max_runtime_seconds. Wire - tools/cronjob_tools.py: cronjob() gains runtime_cap_seconds. Two-layer validation — the tool boundary rejects bad input with a friendly error; cron/jobs.py normalises for direct Python callers that bypass the tool. The tool rejects values over the 24h ceiling; the storage layer clamps. Both layers reject bool (isinstance(True, int) is True in Python, so without the guard runtime_cap_seconds=True would silently become 1s). - update with 0 clears the cap (mirrors how empty-string clears workdir/ script on the existing update branch); create with 0 is rejected — a 0s cap would fire instantly and is almost certainly a mistake. - tui_gateway/server.py: cron.manage action="add" forwards the cap when present. New action="update" carries the cap plus the other safely- mutable fields the underlying tool already supports. - tools/cronjob_tools.py _format_job exposes the cap so cron.manage list echoes it back to UI clients for display. Scheduler enforcement - cron/scheduler.py runs the cap loop in parallel with the existing inactivity poll. On overrun: agent.interrupt() for graceful shutdown followed by ThreadPoolExecutor.shutdown(wait=False, cancel_futures=True) — cron jobs run in a thread pool, not a subprocess, so this is the thread equivalent of SIGTERM. The failure record is a TimeoutError carrying the cap value and elapsed time; the existing gateway failure formatter renders it unchanged. - Poll cadence scales with the cap: max(0.2, min(5.0, cap/4)) so a 5s cap actually fires near 5s instead of at the next 5s tick. - Empty/None = no per-job cap (falls through to inactivity-only). No implicit default — existing jobs are unaffected. Tests - tests/cron/test_runtime_cap_seconds.py (new): integration coverage of the wall-clock loop, defensive coercion, and the error shape. - tests/tools/test_cronjob_tools.py: 13 new unit tests covering the validation matrix (negative, zero, over-ceiling, bool, non-int) and persistence on create and update. - tests/test_tui_gateway_server.py: 7 new gateway tests covering cron.manage add/update/list round-trip and the error envelope. Docs - website/docs/developer-guide/cron-internals.md: per-job runtime cap section documenting field semantics and overrun behaviour. Out of scope - No default runtime cap (NULL = no cap so existing jobs don't start dying). - HERMES_CRON_TIMEOUT inactivity env var name preserved (different concept — kills idle agents, not wall-clock).
|
Related: #45809 (per-job cron caps, open). Both add a per-job wall-clock cap to cron, but via different mechanisms/scope: this PR adds a single |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the end-to-end field, validation, gateway, and documentation work. The need remains: current main only has the process-wide inactivity monitor in cron/scheduler.py:3079-3211.
Problems
cron/scheduler.py:1860describesThreadPoolExecutor.shutdown(wait=False, cancel_futures=True)as SIGTERM-equivalent, but the submittedrun_conversationfuture is already running. The timeout path returns before callingagent.interrupt()(:1852-1886), so a blocking/non-cooperative run can continue after the reported cap.- The new field is accepted for
no_agentcreation attools/cronjob_tools.py:607, while the script-only return path incron/scheduler.py:1320-1356precedes the cap monitor. The setting therefore has no effect for that supported job mode. - This branch substantially predates main: its diff deletes current ticker-health and provider-notification code. Preserve current
cron/jobs.py:72-85andtools/cronjob_tools.py:38-45while salvaging the feature.
Suggested changes
- Make the cap contract either genuinely cancellable or explicitly a soft interrupt, and cover a non-cooperative run.
- Reject or enforce the setting for
no_agentjobs.
Automated hermes-sweeper review.
| if _runtime_cap_timeout: | ||
| # Wall-clock cap fired — interrupt the agent (graceful) and raise | ||
| # a TimeoutError that the existing failure formatter renders the | ||
| # same way as the inactivity path. The shutdown(cancel_futures= |
There was a problem hiding this comment.
shutdown(wait=False, cancel_futures=True) does not cancel the already-running run_conversation future, and this call occurs before agent.interrupt(). The cap can return a timeout while a non-cooperative run continues. Please make the contract a soft interrupt or use a cancellable execution boundary, with a test for that behavior.
| enabled_toolsets=enabled_toolsets or None, | ||
| workdir=_normalize_optional_job_value(workdir), | ||
| no_agent=_no_agent, | ||
| runtime_cap_seconds=runtime_cap_seconds, |
There was a problem hiding this comment.
runtime_cap_seconds is accepted for no_agent=True, but the script-only path returns before the scheduler's new agent monitor. Reject this combination or apply the cap to the script subprocess so the persisted setting is not silently ignored.
| if idle >= inactivity_limit: | ||
| inactivity_timeout = True | ||
| break | ||
| finally: |
There was a problem hiding this comment.
This duplicated polling-loop helper never exercises run_job()'s timeout branch or its agent.interrupt() call. Add a production-path test, including behavior when the submitted agent future does not stop promptly.
Summary
Adds per-job
runtime_cap_secondsto cron — a wall-clock cap that runs in parallel with the existing inactivity-basedHERMES_CRON_TIMEOUT. Mirrors the semantics of kanban tasks'max_runtime_seconds.Why
Today the only agent-side limit on a cron job is
HERMES_CRON_TIMEOUT— process-global, inactivity-based (kills jobs idle for N seconds), defaults to 600. There is no per-job wall-clock cap. A runaway/looping agent that emits any activity inside the inactivity window runs forever. Gateway clients (TUI / desktop / mobile) have no field they can set to bound a single job; the persisted job record has no slot to hold one either.This wires
runtime_cap_secondsend-to-end so a job can carry an explicit wall-clock budget, the scheduler enforces it, and the gateway can read it back for UI display.What changes
Tool boundary (
tools/cronjob_tools.py)cronjob()acceptsruntime_cap_seconds: Optional[int] = None.cron/jobs.pynormalises for direct Python callers that bypass the tool. The tool rejects values over the 24h ceiling; the storage layer clamps. Both layers rejectbool(isinstance(True, int) is Truein Python, so without the guardruntime_cap_seconds=Truewould silently become a 1-second cap).updatewith0clears the cap (mirrors how empty-string clearsworkdir/scripton the existing update branch);createwith0is rejected — a 0s cap would fire instantly and is almost certainly a mistake._format_jobexposes the cap socron.managelistechoes it back to UI clients.Gateway (
tui_gateway/server.py)cron.manageaction="add"forwardsruntime_cap_secondswhen present.cron.manageaction="update"action carries the cap plus the other safely-mutable fields the underlying tool already supports (prompt,schedule,deliver,skills, …). This closes the gateway gap separately from the cap-field gap (updatewas previously missing fromcron.manageentirely).Scheduler (
cron/scheduler.py)agent.interrupt()for graceful shutdown, thenThreadPoolExecutor.shutdown(wait=False, cancel_futures=True)— cron jobs run in a thread pool, not a subprocess, so this is the thread equivalent ofSIGTERM. The failure record is aTimeoutErrorcarrying the cap value and elapsed time; the existing gateway failure formatter at L1898–L1914 renders it unchanged.max(0.2, min(5.0, cap/4))so a 5s cap actually fires near 5s instead of waiting for the next 5s tick.Tests
tests/cron/test_runtime_cap_seconds.py(new, 223 lines): integration coverage of the wall-clock loop, defensive coercion, and the error shape.tests/tools/test_cronjob_tools.py: 13 new unit tests covering the validation matrix (negative, zero, over-ceiling, bool, non-int) and persistence oncreateandupdate.tests/test_tui_gateway_server.py: 7 new gateway tests coveringcron.manageadd/update/listround-trip and the error envelope.Suite results from the dev machine:
The one deselected test (
test_browser_manage_connect_default_local_reports_launch_hint) was verified to fail identically on cleanorigin/mainon the same host — pre-existing flake that depends on Chromium not being installed, unrelated to this change.Out of scope
HERMES_CRON_TIMEOUTenv var name preserved (different concept — kills idle agents, not wall-clock).Docs
One section added to
website/docs/developer-guide/cron-internals.mddocumenting field semantics, default behaviour (no cap unless set), and overrun semantics.