diff --git a/cron/scheduler.py b/cron/scheduler.py index 082cdaedd1..19cbcbc537 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -7070,7 +7070,14 @@ def _run_one_job_body( delivery_attempted = True delivery_error = _deliver_result( job, - _summarize_cron_failure_for_delivery(job, _err_text), + # Composed exactly like the normal failure delivery above. + # mark_job_run below records THIS run in failure_streak + # whichever layer failed, so a job that fails before the + # run body every tick builds a streak nobody is ever told + # about: its alerts only ever leave through here, and the + # nudge only ever left through there (#88655). + _summarize_cron_failure_for_delivery(job, _err_text) + + _failure_streak_nudge(job), adapters=adapters, loop=loop, ) diff --git a/tests/cron/test_run_one_job.py b/tests/cron/test_run_one_job.py index 190d6049c8..6d370b9ff0 100644 --- a/tests/cron/test_run_one_job.py +++ b/tests/cron/test_run_one_job.py @@ -161,6 +161,82 @@ def test_run_one_job_exception_records_failure_alert_delivery_error(monkeypatch) ] +def _patch_escaped_failure(monkeypatch, delivered, *, exec_id, err): + """Make run_job raise, and capture what the escape handler delivers.""" + monkeypatch.setattr(s, "create_execution", lambda *_a, **_kw: {"id": exec_id}) + monkeypatch.setattr(s, "claim_dispatch", lambda _job_id: True) + monkeypatch.setattr(s, "mark_execution_running", lambda _execution_id: None) + monkeypatch.setattr( + s, + "run_job", + lambda *_a, **_kw: (_ for _ in ()).throw(RuntimeError(err)), + ) + monkeypatch.setattr( + s, + "_deliver_result", + lambda job, content, **_kw: delivered.append(content) or None, + ) + monkeypatch.setattr(s, "mark_job_run", lambda *_a, **_kw: None) + monkeypatch.setattr(s, "finish_execution", lambda *_a, **_kw: None) + # Deterministic threshold: default 3, independent of the host config. + monkeypatch.setattr(s, "load_config", lambda: {}) + + +def test_escaped_failure_delivery_carries_the_streak_nudge(monkeypatch): + """A repeatedly-failing job must be nudged even when it fails at the + scheduler layer (#88655). + + ``mark_job_run`` increments ``failure_streak`` for an escaped failure just + as it does for an agent failure, so the counter climbs either way. But the + nudge that spends it was only composed on the normal delivery path, so a + job that raises before the run body on every tick - a bad import from a + half-applied update, a provider client that cannot construct - alerts + forever and is never told it should be reviewed or paused. Nothing else + surfaces the streak in chat. + """ + delivered = [] + _patch_escaped_failure( + monkeypatch, delivered, exec_id="exec-j5", err="cannot import name X" + ) + + ok = s.run_one_job( + { + "id": "j5", + "name": "scout", + "deliver": "telegram", + "schedule": {"kind": "interval"}, + "failure_streak": 2, # + this run = 3 = default threshold + } + ) + + assert ok is False + assert len(delivered) == 1 + assert "cannot import name X" in delivered[0] + assert "failed 3 runs in a row" in delivered[0] + assert "hermes cron pause scout" in delivered[0] + + +def test_escaped_failure_delivery_stays_quiet_below_the_threshold(monkeypatch): + """The nudge is appended, not always-on: a first failure reads as before.""" + delivered = [] + _patch_escaped_failure( + monkeypatch, delivered, exec_id="exec-j6", err="provider failed" + ) + + ok = s.run_one_job( + { + "id": "j6", + "name": "scout", + "deliver": "telegram", + "schedule": {"kind": "interval"}, + "failure_streak": 0, + } + ) + + assert ok is False + assert delivered == ["⚠️ Cron 'scout' failed: provider failed"] + + def test_run_one_job_exception_after_delivery_does_not_redeliver(monkeypatch): """Once delivery has been attempted, the outer handler must not send again.""" delivered = [] diff --git a/website/docs/user-guide/features/cron.md b/website/docs/user-guide/features/cron.md index ab1e185f14..cb91709320 100644 --- a/website/docs/user-guide/features/cron.md +++ b/website/docs/user-guide/features/cron.md @@ -334,13 +334,15 @@ ledger is included in quick backups. ### Repeated-failure review nudge -Each job tracks a `failure_streak` — consecutive runs where the agent failed -(delivery failures don't count). When a *recurring* job's streak reaches the -threshold, the failure message delivered to chat gains a review nudge telling -you the job has failed N runs in a row and suggesting you fix, pause -(`hermes cron pause `), or remove it. Any successful run resets the -streak, and `hermes cron list` shows the streak alongside a failing job's last -run. One-shot jobs never nudge. +Each job tracks a `failure_streak` — consecutive failed runs (delivery +failures don't count). A run that fails before the agent is reached at all — +a bad import after a half-applied update, a provider client that cannot be +constructed — counts and alerts the same as one the agent itself failed. When +a *recurring* job's streak reaches the threshold, the failure message +delivered to chat gains a review nudge telling you the job has failed N runs +in a row and suggesting you fix, pause (`hermes cron pause `), or remove +it. Any successful run resets the streak, and `hermes cron list` shows the +streak alongside a failing job's last run. One-shot jobs never nudge. ```yaml cron: