fix(cron): nudge review of escaped-run failures too
A recurring job that fails at the scheduler layer - an exception escaping
run_one_job's body before the agent is ever constructed - has delivered a
failure alert since 4668750fa. It has never carried the repeated-failure
review nudge the normal agent-failure delivery carries: the nudge (#80752,
2026-08-06) predates that second delivery site by eight days and only ever
composed the first one.
The streak itself is layer-agnostic. mark_job_run increments failure_streak
for an escaped failure exactly as it does for an agent failure, and the
escape handler calls it. So the counter climbs correctly and shows up in
`hermes cron list`, but the chat message that spends it is unreachable for a
job whose failures ALL escape - a half-applied update leaving a bad import,
a provider client that cannot construct. Those are precisely the failures
that repeat identically on every tick, so the operator gets the same one-line
error every 10 minutes indefinitely and is never told the automation itself
is worth reviewing or pausing.
Compose the nudge at the escape handler's delivery exactly as the normal
path does. It stays config-gated and threshold-gated by the same helper, so
a first-time escaped failure reads exactly as it did before.
Docs said the streak counts "runs where the agent failed", which is what the
reporter read and reasonably concluded their failures were out of scope. The
counter never worked that way; correct the sentence to match the code.
Tests: two cases on the escaped-failure delivery path - streak at threshold
appends the nudge (fails on the unfixed handler with the bare summary), and
streak below threshold delivers the unchanged one-liner, so the guard also
proves the nudge is not unconditional. The existing nudge tests only ever
exercised the helper in isolation, which is why the second delivery site
could be added without it.
Fixes #88655
This commit is contained in:
+8
-1
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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 = []
|
||||
|
||||
@@ -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 <job>`), 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 <job>`), 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:
|
||||
|
||||
Reference in New Issue
Block a user