Skip to content

fix(cron): preserve actionable drift recovery alerts - #72056

Closed
sashmatash wants to merge 1 commit into
NousResearch:mainfrom
sashmatash:fix/cron-one-shot-drift-guidance-upstream
Closed

fix(cron): preserve actionable drift recovery alerts#72056
sashmatash wants to merge 1 commit into
NousResearch:mainfrom
sashmatash:fix/cron-one-shot-drift-guidance-upstream

Conversation

@sashmatash

Copy link
Copy Markdown

What changed

  • Keep the unpinned provider/model drift guard before agent construction and inference.
  • Preserve explicit pin/update guidance for recurring or otherwise repeatable jobs.
  • Tell consumed finite one-shots to create a new future job with an explicit provider and model.
  • Deliver those lifecycle-specific actions before the generic 180-character failure-summary truncation.
  • Correct the cron documentation so cronjob action=update is not presented as a remedy for a consumed finite one-shot.

Why

A finite one-shot is consumed when its dispatch is claimed, even if the later drift guard blocks inference. The existing update-oriented recovery advice therefore pointed to a job that could no longer be updated. The generic chat failure summarizer also truncated both recreation and update guidance before users could see it.

This is a focused follow-up to #44585.

How to test

scripts/run_tests.sh \
  tests/cron/test_cron_provider_pin.py \
  tests/cron/test_scheduler.py \
  tests/cron/test_jobs.py -q

Result: 396 passed, 0 failed.

Additional verification:

  • Regression tests were observed failing before the delivery-summary fix and passing afterward.
  • Mocked finite and repeatable probes both failed closed before AIAgent construction and delivered the correct lifecycle-specific action.
  • git diff --check passed.
  • Independent read-only review returned APPROVED with no blockers, suggestions, or nits.

Platform tested

  • macOS 26.5.2
  • Python 3.11.14

Notes

The repository-wide suite and Docusaurus build were not run. The documentation change is a single Markdown line; CI remains the authority for full-repository compatibility.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management area/billing Account usage, credit usage, billing (cross-cutting) labels Jul 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for preserving the actionable drift alert; the underlying lifecycle issue is present on current main.

Problems

  • The proposed repeatable-job instruction uses cronjob action=update ... provider=<provider> model=<model>. Current main intentionally ignores those agent-facing arguments in tools/cronjob_tools.py:1089-1093; tests/tools/test_cronjob_tools.py:411-444 verifies an agent-dispatched update leaves an existing pin unchanged. The new alert and docs would therefore recommend a recovery action that cannot pin the job.
  • The added tests do not exercise the lifecycle ordering behind the report. The production path claims a finite one-shot before run_job() (cron/scheduler.py:3897-3904), and claim_dispatch() persists the consumed count (cron/jobs.py:1828-1884).

Suggested changes

  • Use the supported user-owned recovery surface in alert text and docs: dashboard or hermes cron edit <job_id> --model ... --provider ...; retain new-job guidance for consumed one-shots.
  • Add one scheduler-level regression covering claim → drift failure → delivery, with no AIAgent construction.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 30, 2026
teknium1 added a commit that referenced this pull request Aug 13, 2026
…cific comments

- Widen the scheduler-internal timeout classification to the sibling
  TERMINAL_CWD lock-wait TimeoutError (#79768), which also matched the
  generic 'timed out' branch and was delivered as a provider timeout.
- Reconcile the drift-guard alert with #72056's lifecycle-aware
  remediation: finite one-shots are told to recreate the job, not to
  update a consumed one.
- Scrub environment-specific references from comments/docstrings.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #85508 (rebase-merge) — your commit was cherry-picked onto current main with your authorship preserved in git log.

You were the earliest submitter of the drift-alert delivery fix (untruncated, lifecycle-aware remediation), 18 days ahead of the overlapping work — your one-shot-vs-recurring remediation logic and the docs correction landed as you wrote them, reconciled with the newer alert-once dedup machinery. Thanks!

jackulau added a commit to jackulau/hermes-agent that referenced this pull request Aug 19, 2026
The NousResearch#44585 drift guard fails an unpinned cron job closed when the global
provider/model moves under it, and every message it emits offers exactly one
remedy: pin this job. That is correct for one job and wrong for a fleet.
NousResearch#59031 was 34 jobs breaking on a single global switch, i.e. 34 per-job edits
for one config change, and NousResearch#89242 is the same complaint arriving again with a
proposed fix that would disarm the guard entirely.

The fleet-wide remedy already exists. NousResearch#73532 added `cron.model` /
`cron.model_provider`, and `_cron_fleet_default_covers_axis` makes the guard
skip any axis they cover, so one command decouples the whole unpinned fleet
from the chat model without giving up spend protection on the axes it does not
cover. The cron guide has documented this since NousResearch#73532; none of the runtime
messages did, so the only operators who find it are the ones who read docs
after being told to make N edits.

Teach all three surfaces to name it alongside the pin:

- the scheduler's skip log and alert blob, on both the recurring and the
  NousResearch#72056 finite-one-shot path, naming `cron.model_provider` for the provider
  axis rather than interpolating the axis name into a key that does not exist
- the chat delivery line, which is the surface an operator reads unprompted
  rather than after something sent them to a log; kept to one terse sentence
  so the notification stays one line
- the `hermes config set model.default` warning, which fires before any job
  has failed and is therefore the cheapest place to have omitted it

No behaviour change: the guard engages in exactly the cases it engaged in
before, and the per-job and one-shot remedies are unchanged. The warning is
still suppressed for an axis the fleet default already covers, so an operator
who has taken the advice is never given it.

Tests parse the config key back out of each emitted message and feed it to
`_cron_fleet_default_covers_axis`, so the advice cannot go stale against the
mechanism the way it just did.

Fixes NousResearch#89242
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/billing Account usage, credit usage, billing (cross-cutting) comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants