Skip to content

fix(cron): decouple heartbeat from cron_tick() returning - #60729

Open
Kewe63 wants to merge 1 commit into
NousResearch:mainfrom
Kewe63:fix/60703-cron-ticker-stall
Open

fix(cron): decouple heartbeat from cron_tick() returning#60729
Kewe63 wants to merge 1 commit into
NousResearch:mainfrom
Kewe63:fix/60703-cron-ticker-stall

Conversation

@Kewe63

@Kewe63 Kewe63 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Problem

The cron ticker's heartbeat file (~/.hermes/cron/ticker_heartbeat) only gets bumped after cron_tick() returns. Any single iteration that blocks silently — .tick.lock contention, slow jobs.json I/O, a stray synchronous code path sneaking back in, or a long-running synchronous dispatch — leaves the previous heartbeat to age until the next iteration finally completes, easily adding multiple minutes to the recorded timestamp. hermes cron status then interprets that as a STALLED ticker that "may NOT be firing" (#60703). The gateway process is alive the whole time.

Reproduction:

  1. Run Hermes with a profile and several cron jobs (some agent-driven, some no_agent scripts).
  2. hermes gateway restart.
  3. hermes cron status first reports healthy; within ~30 minutes it flips to "STALLED — no heartbeat for Ns" with no error in the gateway log.

Runtime signal: hermes cron run <id> returns "Already being fired by the scheduler; not run again" — the in-process CAS claim is honoring the running tick, so the live code path is alive, but the tick isn't observable.


Root Cause

InProcessCronScheduler.start() (the built-in 60s ticker that backs both gateway/run.py::start_gateway and hermes_cli/web_server.py::_start_desktop_cron_ticker) writes the heartbeat only after cron_tick() returns:

while not stop_event.is_set():
    cron_tick(verbose=False, ..., sync=False)
    record_ticker_heartbeat(success=True)
    stop_event.wait(interval)

If cron_tick ever blocks, the heartbeat ages unmolested. The TICKER_HEARTBEAT_FILE only signals "the ticker thread's most recent loop iteration completed" — it does NOT signal "the gateway is runnable and the ticker is alive". The same hardening gap exists in PR #39720's description (long sequential job monopolizing the ticker thread), but addressed only the symptom there; the user-facing symptom (STALLED — no heartbeat for Ns, no error) is what triggered this PR.

Sibling paths:

  • The desktop fallback (_start_desktop_cron_ticker) — uses the same InProcessCronScheduler via the same cron.scheduler.tick, reaches the same blunt edge.
  • External providers (CronScheduler.fire_due / Chronos) — explicitly already out-of-band-report provider status, so the watcher is a no-op for them.

Fix

Three small additions to InProcessCronScheduler.start(), all additive — no public API surface changed:

1. Independent watchdog thread (cron-ticker-heartbeat, daemon=True). Bumps record_ticker_heartbeat(success=False) on a fixed cadence — max(5, interval // 2) seconds, capped at the configured interval itself. The success marker is NEVER bumped by the watchdog; the main loop's existing success=True beat remains the source of truth for "did a tick actually fire". So hermes cron status resolves the exact same way for healthy / alive-but-failing / stalled — only the threshold for "alive" widens.

The watchdog runs in its own threading.Thread (not a coroutine / timer) because the parent code path is also synchronous and we want zero dependency on the asyncio loop being runnable while the cron ticker is mid-tick. It exits cleanly on watchdog_stop.set() from the main loop's finally: block — no atexit hook, no race with interpreter finalization.

2. Tick-duration guard. cron_tick() is timed with time.monotonic(). If a single iteration exceeds the configured interval, log a WARNING (not error — the jobs are still firing, just late). Previously this went completely silent; now the gateway log carries a forensic breadcrumb (Cron tick took NNs (interval=Ms)), and hermes cron status reports a fresh heartbeat instead of "STALLED".

3. _record_or_log_heartbeat wrapper. record_ticker_heartbeat is intentionally best-effort and silent (transient disk errors must never break the tick loop); but when that silence becomes the proximate cause of a STALLED report ("no heartbeat for Ns — check the filesystem"), the wrapper escalates a write failure to logger.error("Ticker heartbeat write failed (success=%s): %s", ...) so the gateway log has something to surface. The "never break the loop" contract holds — the wrapper still swallows and returns.


Files

  • cron/scheduler_provider.pyInProcessCronScheduler.start() restructure + new _record_or_log_heartbeat method
  • tests/cron/test_scheduler_provider.py — 3 new regression tests covering each guarantee

How to Test

.venv/bin/python -m pytest tests/cron/test_scheduler_provider.py -v -o addopts= -p no:cacheprovider
.venv/bin/python -m pytest tests/cron/ -o addopts= -p no:cacheprovider  # full cron suite as a regression check

Expected: 40 passed for the provider suite (37 existing + 3 new), 644 passed for the full cron directory.

The three new tests:

  • test_heartbeat_refreshes_while_tick_blocks — replaces cron.scheduler.tick with a callable that never returns; asserts the watchdog bumps heartbeat at least 3 times within ~5s while the main loop is hung, proving liveness is independent of tick progress.
  • test_watchdog_stops_when_main_loop_exits — sets stop_event, asserts the watchdog thread emits at most one trailing heartbeat write within the next 300ms; guards against runaway heartbeats masking shutdown.
  • test_slow_tick_logs_warning_but_keeps_loop_alive — runs a tick that sleeps longer than the configured interval; asserts the loop survives ≥2 iterations AND a 'Cron tick took' WARNING was emitted; proves the duration guard surfaces slowness instead of silently sliding past it.

Result


Checklist

…h#60703)

The in-process ticker only bumped record_ticker_heartbeat after cron_tick returned, so any iteration that blocks silently (lock contention, slow jobs.json I/O, a long-running synchronous code path) leaves the previous heartbeat to age until the next iteration finally completes — easily minutes, which 'hermes cron status' then reports as STALLED with no log evidence. Add an independent daemon watchdog thread (heartbeat at half the configured cadence, capped to [5s, interval]) so the liveness signal refreshes as long as the ticker thread itself is runnable, independent of whether cron_tick is making progress. Surface a WARNING when a single tick exceeds the configured interval so a slow ticker is observable instead of silent, and wrap the heartbeat write in a logger-aware helper that escalates write failures to ERROR so an actual disk issue isn't masquerading as 'no heartbeat for Ns'.

Fixes NousResearch#60703.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management labels Jul 8, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the blocked-tick observability gap. The reported #60703 lock/claim failure mode is already fixed on main by #60855 (7ecc822e1), but the generic blocked-tick work remains reviewable.

Problems

  • cron/scheduler_provider.py:201 computes max(5, min(interval // 2, interval)). With the PR test's interval=1, this is 5 seconds, not 1 second as stated at tests/cron/test_scheduler_provider.py:691-742; the expected third beat is therefore deadline-edge and not a valid cadence regression.
  • cron/scheduler_provider.py:283 cannot log the promised write failures. record_ticker_heartbeat() already suppresses _atomic_write_epoch() exceptions in cron/jobs.py:694-702, so this wrapper's except is not reached for filesystem failures.

Suggested changes

  • Correct the cadence bounds and make the watchdog test assert a non-deadline-edge schedule.
  • Return write success from the heartbeat helper or place logging at the atomic-write boundary, with a forced-write-failure logging test.

Automated hermes-sweeper review.

# Half-cadence refresh; cap to at least 5s and at most the
# tick interval itself so a misconfigured 1s interval can't
# spam the heartbeat file.
cadence = max(5, min(interval // 2, interval))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This order does not cap the cadence at interval: with the new test's interval=1, it evaluates to 5 seconds. That contradicts the test's one-second expectation and makes its third beat deadline-edge. Apply the lower bound before the upper cap, then adjust the test to avoid timing the final expected beat at the timeout boundary.

"""
try:
from cron.jobs import record_ticker_heartbeat
record_ticker_heartbeat(success=success)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

record_ticker_heartbeat() already catches write failures internally (cron/jobs.py:694-702), so filesystem failures never reach this wrapper's except and no error is logged. Expose success/failure from the lower-level helper or log at the atomic-write boundary, and cover a real forced write failure.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 10, 2026
@egilewski

Copy link
Copy Markdown
Contributor

obsolete

The issue this PR closes appears to be resolved already. Please reopen with a fresh target if this still covers a distinct gap.

Signed: GPT-5.6-terra-low in Codex

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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cron ticker silently stalls after gateway restart

4 participants