fix(cron): no_agent and script failures stop masquerading as provider errors - #85536
Conversation
`_summarize_cron_failure_for_delivery` classifies a failed job by
substring-matching the error prose — "timed out", "429",
`authenticat|authoriz` — and maps any hit onto a provider-shaped
explanation, without consulting the job's execution mode.
A `no_agent` job IS its script: `run_job` short-circuits it before any
model is reached. Provider timeouts, rate limits, auth errors and
fallback chains are therefore structurally impossible for it, yet those
branches are tested first.
`_run_job_script` reports a timeout as "Script timed out after {n}s:
{path}". That contains "timed out", so a shell script exceeding its
timeout is delivered to chat as:
⚠️ Cron 'x' failed: provider timeout. Fallback chain was exhausted
or unavailable.
for a job that never opened a socket, sending the reader to inspect
model routing while the actual fault is a shell script. "429" or
"authentication" appearing anywhere in a script's output misfires the
same way.
Gate the three provider branches on `not job.get("no_agent")` and let
script jobs fall through to the existing generic cleaner, which already
reports the real error and names the script. No new message text.
The auth branch carries a word-boundary guard so "oauth" and "4015" do
not trip it, which addresses one substring false-positive; gating on
mode removes the remaining class for script jobs.
Tests: the summarizer had no direct coverage — the only test referencing
it patches it out and asserts on its arguments. Adds parametrized cases
pinning both directions: script jobs are never blamed on a provider
(including when their output contains "429" or "authentication"), and
agent-mode jobs keep the existing provider summaries unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…composed no_agent gate The cherry-picked tests predate #85508's honest fallback-chain phrasing and each other: assertions pinned the old 'exhausted or unavailable' literal and #83188's no_agent fallback-note behavior, which #77648's mode gate supersedes (no provider classification at all for no_agent jobs). Assert the composed contract instead.
૮ >ﻌ< ა ci reviewran on f2b4e96 — chore: map contributor emails for cherry-picked commits
|
|
Credit correction: the post-merge sweep surfaced four earlier submissions of these fixes that the pre-merge duplicate sweep missed — #60593 (@cpahgw-rgb, Jul 8) was the earliest no_agent mode-gate fix, #61969 (@angroni84-wq, Jul 10) the earliest script-timeout contract classification, and #70977 (@brian717) + #70913 (@webtecnica) also predate the cherry-picked PRs. All four closed with credit; first-submitter acknowledgment belongs to @cpahgw-rgb and @angroni84-wq respectively. |
|
Thanks @teknium1 for the careful post-merge sweep and the credit correction — closing #70913 with credit is the right call, and the first-submitter acknowledgment to @cpahgw-rgb and @angroni84-wq is exactly how it should work. Glad the consolidated salvage landed cleanly (674 passed). Appreciate the diligence on attribution. |
…ll_process_tree The script-timeout path used a site-local process-group kill, which cannot reach a grandchild that created its OWN session (start_new_session background jobs, watchdogs). Such descendants kept running after the job reported failure (NousResearch#71148, NousResearch#59549). Migrate the timeout handler to the unified deadline layer's kill_process_tree (NousResearch#85147, d6a5cb9): psutil snapshots the descendant set before signalling, so own-session grandchildren are reached too. Fallback to the site-local group kill if the import ever fails, so the path cannot re-wedge. The explicit script-timeout message stays the classification anchor (NousResearch#85536's contract), keeping cron timeouts distinct from provider timeouts. Co-authored-by: dante32683 <dante32683@users.noreply.github.com> Co-authored-by: supotato-ipj <supotato-ipj@users.noreply.github.com>
…ll_process_tree The script-timeout path used a site-local process-group kill, which cannot reach a grandchild that created its OWN session (start_new_session background jobs, watchdogs). Such descendants kept running after the job reported failure (NousResearch#71148, NousResearch#59549). Migrate the timeout handler to the unified deadline layer's kill_process_tree (NousResearch#85147, d6a5cb9): psutil snapshots the descendant set before signalling, so own-session grandchildren are reached too. Fallback to the site-local group kill if the import ever fails, so the path cannot re-wedge. The explicit script-timeout message stays the classification anchor (NousResearch#85536's contract), keeping cron timeouts distinct from provider timeouts. Co-authored-by: dante32683 <dante32683@users.noreply.github.com> Co-authored-by: supotato-ipj <supotato-ipj@users.noreply.github.com>
…ll_process_tree The script-timeout path used a site-local process-group kill, which cannot reach a grandchild that created its OWN session (start_new_session background jobs, watchdogs). Such descendants kept running after the job reported failure (NousResearch#71148, NousResearch#59549). Migrate the timeout handler to the unified deadline layer's kill_process_tree (NousResearch#85147, d6a5cb9): psutil snapshots the descendant set before signalling, so own-session grandchildren are reached too. Fallback to the site-local group kill if the import ever fails, so the path cannot re-wedge. The explicit script-timeout message stays the classification anchor (NousResearch#85536's contract), keeping cron timeouts distinct from provider timeouts. Co-authored-by: dante32683 <dante32683@users.noreply.github.com> Co-authored-by: supotato-ipj <supotato-ipj@users.noreply.github.com>
…ll_process_tree The script-timeout path used a site-local process-group kill, which cannot reach a grandchild that created its OWN session (start_new_session background jobs, watchdogs). Such descendants kept running after the job reported failure (NousResearch#71148, NousResearch#59549). Migrate the timeout handler to the unified deadline layer's kill_process_tree (NousResearch#85147, d6a5cb9): psutil snapshots the descendant set before signalling, so own-session grandchildren are reached too. Fallback to the site-local group kill if the import ever fails, so the path cannot re-wedge. The explicit script-timeout message stays the classification anchor (NousResearch#85536's contract), keeping cron timeouts distinct from provider timeouts. Co-authored-by: dante32683 <dante32683@users.noreply.github.com> Co-authored-by: supotato-ipj <supotato-ipj@users.noreply.github.com>
…ll_process_tree The script-timeout path used a site-local process-group kill, which cannot reach a grandchild that created its OWN session (start_new_session background jobs, watchdogs). Such descendants kept running after the job reported failure (NousResearch#71148, NousResearch#59549). Migrate the timeout handler to the unified deadline layer's kill_process_tree (NousResearch#85147, d6a5cb9): psutil snapshots the descendant set before signalling, so own-session grandchildren are reached too. Fallback to the site-local group kill if the import ever fails, so the path cannot re-wedge. The explicit script-timeout message stays the classification anchor (NousResearch#85536's contract), keeping cron timeouts distinct from provider timeouts. Co-authored-by: dante32683 <dante32683@users.noreply.github.com> Co-authored-by: supotato-ipj <supotato-ipj@users.noreply.github.com>
…ll_process_tree The script-timeout path used a site-local process-group kill, which cannot reach a grandchild that created its OWN session (start_new_session background jobs, watchdogs). Such descendants kept running after the job reported failure (NousResearch#71148, NousResearch#59549). Migrate the timeout handler to the unified deadline layer's kill_process_tree (NousResearch#85147, d6a5cb9): psutil snapshots the descendant set before signalling, so own-session grandchildren are reached too. Fallback to the site-local group kill if the import ever fails, so the path cannot re-wedge. The explicit script-timeout message stays the classification anchor (NousResearch#85536's contract), keeping cron timeouts distinct from provider timeouts. Co-authored-by: dante32683 <dante32683@users.noreply.github.com> Co-authored-by: supotato-ipj <supotato-ipj@users.noreply.github.com>
Cron failure summaries stop blaming LLM providers for failures in script jobs and script timeouts, stop matching stray "429" digits, and the script timeout becomes a recognized config key.
Consolidated salvage of the no_agent/script-timeout misclassification cluster, cherry-picked with authorship preserved:
cron.script_timeout_secondsadded to config defaults sohermes config setrecognizes the key the scheduler already reads (alongsideHERMES_CRON_SCRIPT_TIMEOUT).429matched as a whole token, so identifiers likehash429abc.mdno longer trip a false rate-limit alert. The PR's no_agent fallback-note half is superseded by fix(cron): don't attribute no_agent script failures to a provider #77648's stronger mode gate; its tests were reconciled to the composed contract.Composition note: all four land in
_summarize_cron_failure_for_deliveryon top of yesterday's #85508 (honest chain wording, inactivity/lock classification). Order in the function: drift → mode gate → script-timeout contract → rate limit (token match) → inactivity → cwd-lock → provider timeout → auth.Validation
hash429abc.mdhermes config set cron.script_timeout_seconds 600Tests: 171 passed across the three touched test files; full cron suite 674 passed, 1 skipped.
Closes #77648, closes #79451, closes #81629, closes #82460, closes #78503, closes #83188.
Infographic