Skip to content

fix(cron): skip model/provider snapshot for inherit-mode jobs (#89242) - #89855

Open
RelaxJonh wants to merge 1 commit into
NousResearch:mainfrom
RelaxJonh:fix/cron-model-snapshot-inherit-89242
Open

fix(cron): skip model/provider snapshot for inherit-mode jobs (#89242)#89855
RelaxJonh wants to merge 1 commit into
NousResearch:mainfrom
RelaxJonh:fix/cron-model-snapshot-inherit-89242

Conversation

@RelaxJonh

Copy link
Copy Markdown
Contributor

Summary

_compute_provider_model_snapshots() unconditionally records a model_snapshot and provider_snapshot for every agent cron job, including inherit-mode jobs where both model and provider are None. When the user later changes the global default model, the drift guard compares the stale snapshot against the new global, sees a mismatch, and fails closed — skipping the run — even though the job was always intended to inherit.

Fix

Skip snapshotting entirely when both axes are unpinned (pure inherit mode). The drift guard still applies to partially-pinned jobs where one axis is explicit and the other inherits.

Changes

  • cron/jobs.py: Early return (None, None) when both normalized_provider and normalized_model are None
  • tests/cron/test_cron_provider_pin.py: Updated test_unpinned_job_skips_snapshot to verify inherit-mode jobs get no snapshot and the resolver is not called

Testing

  • Full cron test suite: 805 passed, 1 skipped
  • Provider pin tests: 16 passed

Fixes #89242

…search#89242)

_compute_provider_model_snapshots() unconditionally recorded a
model_snapshot and provider_snapshot for every agent cron job, including
inherit-mode jobs where both model and provider are None.  When the user
later changed the global default model, the drift guard compared the
stale snapshot against the new global, saw a mismatch, and failed
closed — skipping the run — even though the job was always intended to
inherit.

Skip snapshotting entirely when both axes are unpinned (pure inherit
mode).  The drift guard still applies to partially-pinned jobs where one
axis is explicit and the other inherits.

Fixes NousResearch#89242
@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/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 19, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

  1. cron/jobs.py:_compute_provider_model_snapshots — Positive: correct reading of the drift-guard contract — a snapshot of an inherited value records "whatever the global was at creation time" and then flags an intentional global change as drift. Skipping both axes when both are None keeps inherit-mode jobs tracking the live default, while mixed-pinning jobs still snapshot only their explicitly pinned axis via the fall-through. The updated test also asserts the runtime resolver is never called for fully-inherit jobs, pinning the no-work expectation alongside the no-snapshot one. No change requested.

@jackulau

Copy link
Copy Markdown
Contributor

Not a change request — a cross-link, because this PR and #89292 make one policy
decision between them and neither says so on its own.

This PR stops recording model_snapshot / provider_snapshot for fully-inherit
jobs, which is the right fix for #89242: a job that pinned nothing should not
fail closed when the global default moves. The consequence worth naming is that
an inherit-mode job created after this lands has nothing to drift from, so
it follows cron.model / cron.model_provider silently and permanently. That
is inherit mode working as intended, and it is also a real behavioural change
for anyone who has been treating the drift guard as a blast-radius check on
hermes config set model — they lose that signal for exactly the jobs that
never pinned anything.

I mention it because #89292 (mine, currently open) changes the remediation
text
the guard prints, and its tests parse the emitted hermes config set keys
back out of the messages and assert they name the cron-fleet default rather than
per-job pins. Those tests document the fail-closed-vs-follow-default fork
explicitly, and they are about a population this PR shrinks. Read alone, this PR
looks like a bug fix with no policy content and that one looks like advice for a
shrinking audience; read together they are coherent. A sentence in the
description here pointing at #89292 (or the reverse — happy to add it on mine
instead, or both) would keep that from being rediscovered later.

No conflict between the branches: this one touches cron/jobs.py, mine touches
the scheduler's remediation strings and hermes_cli/config.py. Either order
merges cleanly.

One thing the description could say out loud, which I checked rather than
assumed: pre-existing inherit-mode jobs keep their stale snapshot. The diff
touches _compute_provider_model_snapshots only, and the update path in
cron/jobs.py recomputes snapshots solely under if inference_fields_changed.
For a pure-inherit job the inference fields are already None, so the only edit
that clears the old snapshot is one that pins an axis, which stops it being an
inherit job at all.

So after this lands the two populations diverge permanently and invisibly: a
job created yesterday with model: null keeps fail-closing on drift, an
identical job created tomorrow follows the default, and nothing in
hermes cron list distinguishes them. That may well be the intended trade
(no migration, old jobs keep the stricter behaviour) and it is a defensible
one. It is just not currently written down anywhere, and the failure it
produces is a cron job that silently skips runs while its twin runs fine.

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

Labels

area/config Config system, migrations, profiles comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists 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.

model_snapshot recorded for inherit-mode cron jobs causes false drift guard failures

4 participants