fix(cron): suppress false "Gateway is not running" warning when desktop ticker is alive - #67828
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting a real desktop-only false positive. Current main still prints the create/list warning after an empty gateway-PID result (hermes_cli/cron.py:81-96), while the desktop backend starts a scheduler ticker under HERMES_DESKTOP=1 (hermes_cli/web_server.py:148-166, 204-217) and the built-in provider writes a heartbeat (cron/scheduler_provider.py:222-258).
Problems
cron_status()will still contradict this change. It evaluates heartbeat state only insideif pids:(hermes_cli/cron.py:248-266); with no gateway PID it unconditionally reports that jobs will not fire (hermes_cli/cron.py:307-313). Thus a fresh desktop heartbeat suppresses create/list warnings but still produces a false failing status report.
Suggested changes
- Add the same fresh-heartbeat fallback to the no-PID
cron_status()path and report a live ticker without implying a gateway process exists. - Add a matching
cron statusregression test with no gateway PID and a fresh heartbeat.
Automated hermes-sweeper review.
| @@ -86,8 +96,19 @@ def _warn_if_gateway_not_running() -> None: | |||
|
|
|||
| if find_gateway_pids(): | |||
| return | |||
|
|
|||
There was a problem hiding this comment.
Please apply this same fresh-heartbeat fallback to cron_status(): current main only evaluates heartbeat state after find_gateway_pids() succeeds (hermes_cli/cron.py:248-266), and otherwise reports that jobs will not fire (:307-313). Without that companion path, desktop users receive conflicting create/list and status results.
SummaryEight PRs address or reference the false gateway/ticker warning, but their inspected diffs cover two distinct causes: #10636, #10684, #11293, #12849, #15318, and #46392 change macOS PID discovery; #11445 adds a runtime-PID fallback; and #67828 recognizes a live desktop-hosted ticker through its heartbeat. For the launchd-supervised case in #15225, the issue’s maintainer-bot verdict records the launchctl fix on main in commit e3db1ef at hermes_cli/gateway.py:151-166 and the warning consumer at hermes_cli/cron.py:85-93, while the macOS ps fallback remains the separately salvageable delta. Related pull requests
Duplicates#12849 is explicitly a duplicate of #11293. For the remaining macOS ps fallback, #46392 and the narrowed #11293 are substantially duplicate implementations; #10636 and #15318 are closed combined launchctl-plus-ps predecessors, while #67828 is not their duplicate because it handles a live desktop ticker rather than PID discovery. Suggested consolidationKeep #67828 open with a salvage path: retain the fresh-heartbeat handling and regression coverage across create/list/status, and retarget or remove its #15225 closure linkage because that launchd-specific issue is already recorded as implemented on main. Keep #11293 open as the focused current-main ps-fallback vehicle; close #46392 as a duplicate of #11293 despite its keep_open review because its visible launchctl hunk is already on main and its remaining macOS ps selection is the same salvage now isolated and tested in #11293. Leave the already-closed #10636, #10684, #11445, #12849, and #15318 as historical or superseded references. Cross-PR triage: Reviewed 8 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 57 kB of PR diffs, 32 kB of issue/PR text, 8 kB of discussion (13 comments), 10 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
…op ticker is alive The cron ticker runs in EITHER the gateway (gateway/run.py::_start_cron_ticker) OR the desktop dashboard backend (hermes_cli/web_server.py::_start_desktop_cron_ticker, started when HERMES_DESKTOP=1). _warn_if_gateway_not_running() only checked find_gateway_pids(), which matches 'gateway run' argv via the strict looks_like_gateway_command_line() matcher -- so the desktop backend's 'hermes serve' process was invisible to it, and every desktop-app user saw a false 'Gateway is not running -- jobs won't fire automatically' warning even though their jobs were firing correctly via the desktop-cron-ticker thread. Fix: after find_gateway_pids() comes up empty, also check the ticker heartbeat file (~/.hermes/cron/ticker_heartbeat, written every 60s by the ticker loop). A fresh heartbeat (<= TICKER_INTERVAL_SECONDS * 3 + 20 = 200s, mirroring cron_status's existing STALE_AFTER threshold) means a ticker IS alive regardless of which process hosts it, so the warning is suppressed. A stale or missing heartbeat still warns. Tests: 3 new (test_create_silent_when_desktop_ticker_alive, test_list_silent_when_desktop_ticker_alive, test_warns_when_heartbeat_stale) plus 3 existing tests updated to stub get_ticker_heartbeat_age. All 19 cron CLI tests pass. Closes NousResearch#15225.
teknium1 review on PR NousResearch#67828: the heartbeat fallback was added to _warn_if_gateway_not_running() (create/list), but cron_status() still unconditionally reported "will NOT fire" when find_gateway_pids() came up empty -- even with a fresh desktop ticker heartbeat. The two views contradicted each other. Fix: in the no-PID else branch of cron_status(), check the ticker heartbeat before declaring failure. A fresh heartbeat (<= 200s, same STALE_AFTER threshold as the create/list fix) means a ticker IS alive, so report "Cron ticker is running -- jobs will fire" instead of the false "will NOT fire". A stale or absent heartbeat still reports the historical failure message with install instructions. Tests: 2 new (test_status_reports_ticker_when_desktop_alive_no_gateway, test_status_reports_not_firing_when_heartbeat_stale_no_gateway) plus test_status_unchanged_for_builtin updated to explicitly stub get_ticker_heartbeat_age. All 21 cron CLI tests pass. Addresses teknium1 review feedback on NousResearch#67828.
13fbda7 to
c23e16b
Compare
What does this PR do?
Suppresses a false "Gateway is not running — jobs won't fire automatically" warning that fires for every desktop-app user (and any setup where the cron ticker runs outside the gateway process).
The built-in cron ticker runs in either:
gateway/run.py::_start_cron_ticker), orhermes_cli/web_server.py::_start_desktop_cron_ticker, started whenHERMES_DESKTOP=1)_warn_if_gateway_not_running()only consultedfind_gateway_pids(), which uses the strictlooks_like_gateway_command_line()matcher — it only matchesgateway runargv. The desktop backend'shermes serveprocess is invisible to it, so everyhermes cron list/hermes cron createon a desktop-app-only setup printed the warning even though jobs were firing correctly via the desktop-cron-ticker thread. The warning was wrong on both counts: the ticker was alive (heartbeat 2s old) and jobs were completing at 100%.Fix: after
find_gateway_pids()comes up empty, also check the ticker heartbeat file (~/.hermes/cron/ticker_heartbeat, written every 60s by the ticker loop incron/scheduler_provider.py). A fresh heartbeat (≤TICKER_INTERVAL_SECONDS * 3 + 20= 200s, mirroringcron_status's existingSTALE_AFTERthreshold athermes_cli/cron.py:264) means a ticker IS alive regardless of which process hosts it, so the warning is suppressed. A stale or missing heartbeat still warns — preserving the original #51038 regression guard.This reuses the heartbeat signal
cron_statusalready trusts for its own liveness report, so the two views agree on what "alive" means. No new config keys, no new files, no behavior change for gateway users (the gateway path returns before the heartbeat check).Related Issue
Related to #15225
The launchctl PID-discovery fix for #15225 is already recorded as implemented on
main; this PR targets the distinct, still-unfixed desktop-ticker cause:_warn_if_gateway_not_running()only consultsfind_gateway_pids(), so when the ticker runs inside the desktop backend (not the gateway process), create/list print a false warning even though the ticker is alive and jobs are firing. This PR adds the heartbeat fallback (and extends it tocron_status()'s no-PID path) to silence that false positive.Type of Change
Changes Made
hermes_cli/cron.py—_warn_if_gateway_not_running()now checks the ticker heartbeat afterfind_gateway_pids()comes up empty; fresh heartbeat (≤200s) suppresses the warning. Docstring updated to reflect that the ticker runs in either the gateway or the desktop backend.tests/hermes_cli/test_cron.py— 3 new tests (test_create_silent_when_desktop_ticker_alive,test_list_silent_when_desktop_ticker_alive,test_warns_when_heartbeat_stale) and 3 existing tests updated to stubget_ticker_heartbeat_ageso they exercise both paths.How to Test
Reproduction (before fix): On a machine with the Hermes desktop app running but no gateway service, run
hermes cron list. Observe the false warning appended to the job list, even thoughhermes cron statusshows a fresh ticker heartbeat and jobs are firing.Proof of fix:
hermes cron liston a desktop-app-only setup — the warning no longer appears when the ticker heartbeat is fresh.hermes cron statusalready reported the heartbeat correctly; this PR makes the create/list warning consistent with it.Manual verification against live state (desktop backend running, no gateway process):
Checklist
Code
fix(scope):,feat(scope):, etc.)hermes cron listfalsely reports "Gateway is not running" on macOS (two-stage detection failure in find_gateway_pids) #15225 covers this bug; thesweeper:implemented-on-mainlabel appears to be aspirational as the fix is not onmain.pytest tests/ -qand all tests pass —tests/hermes_cli/test_cron.py19/19 pass; broader cron suite 748/749 (the 1 failure is a pre-existingconftest.pylive-system guard artifact intests/cron/test_cron_script.py::test_script_timeout, unrelated to this change — verified by stashing and re-running)Documentation & Housekeeping
_warn_if_gateway_not_running()updated to reflect the desktop-backend ticker pathcli-config.yaml.exampleif I added/changed config keys — or N/A (no new config keys)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A (no architecture change)cron/scheduler_provider.py::record_ticker_heartbeat; the fix reads it viaget_ticker_heartbeat_age()which is already platform-agnosticScreenshots / Logs
Before (desktop app running, ticker alive, jobs firing):
After (same setup):
hermes cron statusalready correctly reported the ticker as alive on this setup; this PR makescron list/cron createagree with it.