feat(cron): per-job failure_deliver — route or suppress failure notices - #100375
Closed
victor-kyriazakos wants to merge 2 commits into
Closed
feat(cron): per-job failure_deliver — route or suppress failure notices#100375victor-kyriazakos wants to merge 2 commits into
victor-kyriazakos wants to merge 2 commits into
Conversation
…es (NS-788)
Coatue FR (Frank Long): jobs delivering into shared channels publish
engine failure notices ('⚠️ Cron X failed…') to those channels with no
opt-out. Adds an optional per-job failure_deliver field sharing
deliver's grammar: on failure, targets resolve from failure_deliver
when set (local = structural silence; state still recorded in
last_status/last_error/run history). Success delivery is unchanged;
absent field = today's behavior byte-for-byte.
Honored by every failure-category engine notice: the run_job failure
summary (+streak nudge), the escaped-failure retry path, drift-skip and
blocked-config alerts (composed into the same delivery), and the
gateway-shutdown interrupted-run notice (_notify_interrupted_cron_jobs).
Surfaces: cronjob tool create/update (same bot-chat validation as
deliver; '' clears on update), hermes cron create/edit
--failure-deliver, docs tip in automate-with-cron.
Existing fake_deliver test doubles gained **kwargs for the new
for_failure keyword — signature-compat only, no behavior change.
alt-glitch
added
type/feature
…uted through Review findings (Salt, NS-788): B1: delivery_outcome classification, unresolved_origin, and incident 'alerted' marking all read the deliver lane while the notice itself was routed through failure_deliver — a silenced failure recorded delivery_outcome='delivered' and marked its incident alerted (corrupting the 'failure seen' vs 'operator was pinged' distinction the incident store documents), and a failure delivered via failure_deliver over an unresolvable deliver=origin recorded 'not_configured'. New _delivery_lane_value() helper feeds the SAME lane to routing and bookkeeping at all five sites (both classifiers, both unresolved_origin computations, both zero-target checks). Three regression tests assert outcome + alerted-marking; verified to bite on the pre-fix classifier. S1: failure_deliver now goes through _resolve_cron_context_deliver on tool create/update, matching deliver — a job created from inside a cron run can no longer store literal 'origin' in its failure lane. S2/T1: corrected the false 'same helper' comment in create_job; the str/list flatten mirrors the tool layer for direct callers. Full cron suite + interrupt tests: 87 files, 1112 passed, 0 failed.
kshitijk4poor
added a commit
that referenced
this pull request
Sep 2, 2026
… lanes Follow-up to the failure_deliver salvage (#100375): - _preflight_check_delivery also checks the failure lane, so a typo'd failure_deliver platform blocks at config-validation time instead of surfacing only when a failure occurs — exactly when the notice must not be lost. Duplicate lanes are checked once. - The dashboard cron-update normalizer treats failure_deliver like deliver (text normalization; empty clears the optional override instead of coalescing), closing the one update path that could write an unnormalized value into jobs.json. 4 guard tests; both fixes mutation-checked (neutralize -> red, restore -> green).
Collaborator
|
Merged via #101373 — your commits were cherry-picked onto current main with your authorship preserved (the branch had drifted ~420 commits behind and conflicted with the newer cron delivery-verification work on main, so a direct merge wasn't possible). The salvage also folded in two small review follow-ups on top of your work: preflight now validates the Thanks for a genuinely thorough PR — full emission-lane coverage, the bookkeeping-lane fix in the second commit, and the mutation-checked tests made this an easy salvage. |
This was referenced Sep 2, 2026
melon-xf
added a commit
to melon-xf/hermes-agent
that referenced
this pull request
Sep 3, 2026
… lanes Follow-up to the failure_deliver salvage (NousResearch#100375): - _preflight_check_delivery also checks the failure lane, so a typo'd failure_deliver platform blocks at config-validation time instead of surfacing only when a failure occurs — exactly when the notice must not be lost. Duplicate lanes are checked once. - The dashboard cron-update normalizer treats failure_deliver like deliver (text normalization; empty clears the optional override instead of coalescing), closing the one update path that could write an unnormalized value into jobs.json. 4 guard tests; both fixes mutation-checked (neutralize -> red, restore -> green).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An enterprise fleet reported that cron jobs delivering into large shared Slack channels have no way to keep engine failure notices out of those channels. When a recurring job hard-fails,
⚠️ Cron 'X' failed …posts to the job's delivery target — noise for hundreds of readers who can't act on it, while the operator sees the failure in run history anyway.This adds an optional per-job
failure_deliverfield with the same grammar asdeliver. On failure, delivery targets resolve fromfailure_deliverwhen set;failure_deliver: localsuppresses failure notices entirely. Run state (last_status,last_error, run history, output file) is recorded exactly as before. Success delivery never reads the field, and jobs without it behave byte-for-byte as today.deliver: slack:C123deliver: slack:C123, failure_deliver: localdeliver: slack:C123, failure_deliver: slack:C_OPSImplementation
One seam:
_resolve_delivery_targets(job, for_failure=). Every failure-category engine notice flows through it:_notify_interrupted_cron_jobs)The field reuses
deliver's normalization and bot-chat validation — no parallel validation path. Surfaced on the cronjob tool (create/update,''clears) andhermes cron create/edit --failure-deliver, plus a docs tip next to the[SILENT]note it complements ([SILENT]covers successful runs only; this covers failures).Deliberately not added: a
deliver_failure: falseboolean (redundant —failure_deliver: localis that switch in existing vocabulary) and a global config gate (per-job covers the reported need; a profile-level default can layer on later if asked for).Testing
tests/cron/test_cron_failure_deliver.py+ interrupted-notice coverage: resolution grammar (comma lists, threads, list flattening), scheduler failure paths, retry path, gateway shutdown lane, tool surface validation parity, success-path isolation.for_failureresolution turns 8 tests red; restoring goes green.fake_delivertest doubles gained**kwargsto accept the new keyword — signature compatibility only.