feat(cron): per-job failure_deliver — route or suppress failure notices (salvage #100375) - #101373
Merged
kshitijk4poor merged 3 commits intoSep 2, 2026
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.
…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.
… 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).
alt-glitch
added
type/feature
This was referenced Sep 2, 2026
4 tasks
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
Salvage of #100375 by @victor-kyriazakos onto current
main(the original branch was ~420 commits behind and conflicted with thecron.delivery.notify/ unverified-delivery work that landed since). Both original commits are cherry-picked with authorship preserved; one follow-up commit from review is added on top.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_OPSOne seam:
_resolve_delivery_targets(job, for_failure=). Every failure-category engine notice flows through it — the run failure summary + streak nudge (both scheduler delivery sites, including the escaped-failure retry path), drift-skip and blocked-config alerts, and the gateway-shutdown interrupted-run notice. Outcome bookkeeping (delivered/suppressed/not_configured, incidentalertedmarking) reads the same lane the notice was actually routed through. 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.Follow-up commit (review findings)
_preflight_check_deliverynow validates the failure lane too — a typo'dfailure_deliverplatform 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.failure_deliverlikedeliver(text normalization; empty clears the optional override), closing the one update path that could write an unnormalized value into jobs.json.Validation
tests/cron/test_cron_failure_deliver.py(20 from the original PR + 4 follow-up guards) + 2 interrupt-notice tests: all pass.for_failurelane resolution turns 11 tests red; neutralizing each follow-up fix turns its guard red; restore → green.tests/cron/+ cronjob tools + CLI cron + interrupt suite): 1270 passed; the 6 failures are order-dependent flakes reproduced identically onorigin/mainwith the same batch (5×test_monitor_kind.py, 1×test_cron_multiplex_shared_route_delivery.py— passes in isolation on both).Credit
Based on #100375 by @victor-kyriazakos — commits cherry-picked to preserve authorship. Closes #100375. Related earlier PRs on the same feature: #87564 (@koskinenjanne-source), #77866 (@moeedahmed); this implementation was selected for whole-bug-class lane coverage (4 emission sites), stored-job byte-identity, and grammar reuse. Addresses the per-job case of #86998.