diff --git a/cli.py b/cli.py index b15a821eb63..caf31036c87 100644 --- a/cli.py +++ b/cli.py @@ -4032,6 +4032,15 @@ def save_config_value(key_path: str, value: any) -> bool: os.chmod(config_path, 0o600) except (OSError, NotImplementedError): pass + + # Model/provider changes made through /model and the TUI use this + # persistence path rather than ``hermes config set``. Surface the same + # fail-closed cron drift warning for every operator-facing model switch. + from hermes_cli.config import ( + warn_unpinned_cron_jobs_after_model_config_change, + ) + + warn_unpinned_cron_jobs_after_model_config_change(key_path, value) return True except Exception as e: diff --git a/cron/scheduler.py b/cron/scheduler.py index 91c3168268c..39d70ab5090 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -41,7 +41,11 @@ sys.path.insert(0, str(Path(__file__).parent.parent)) from hermes_constants import get_hermes_home from hermes_cli._subprocess_compat import windows_hide_flags -from hermes_cli.config import load_config, _expand_env_vars +from hermes_cli.config import ( + _expand_env_vars, + cron_model_drift_guard_enabled, + load_config, +) from hermes_cli.fallback_config import get_fallback_chain from hermes_time import now as _hermes_now @@ -3359,42 +3363,43 @@ def run_job( # Back-compat: an axis with no snapshot (pre-existing jobs, no_agent, or # any axis whose creation-time resolution failed) behaves exactly as # before — the guard never engages for it. Pinned axes are unaffected. - _drift: list[str] = [] - _provider_snapshot = (job.get("provider_snapshot") or "").strip().lower() - if _provider_snapshot and not (job.get("provider") or "").strip(): - _current_provider = str( - primary_provider_for_drift or runtime.get("provider") or "" - ).strip().lower() - if _current_provider and _current_provider != _provider_snapshot: - _drift.append( - f"provider '{_provider_snapshot}' -> '{_current_provider}'" + if cron_model_drift_guard_enabled(_cfg): + _drift: list[str] = [] + _provider_snapshot = (job.get("provider_snapshot") or "").strip().lower() + if _provider_snapshot and not (job.get("provider") or "").strip(): + _current_provider = str( + primary_provider_for_drift or runtime.get("provider") or "" + ).strip().lower() + if _current_provider and _current_provider != _provider_snapshot: + _drift.append( + f"provider '{_provider_snapshot}' -> '{_current_provider}'" + ) + _model_snapshot = (job.get("model_snapshot") or "").strip().lower() + if _model_snapshot and not (job.get("model") or "").strip(): + _current_model = str(primary_model_for_drift or "").strip().lower() + if _current_model and _current_model != _model_snapshot: + _drift.append( + f"model '{_model_snapshot}' -> '{_current_model}'" + ) + if _drift: + _changes = "; ".join(_drift) + logger.warning( + "Job '%s': SKIPPED — global inference config drifted since " + "creation (%s) and this job is unpinned. Skipped to prevent " + "unintended spend. Pin explicitly to proceed: " + "`cronjob action=update job_id=%s provider=

model=`.", + job_id, + _changes, + job_id, ) - _model_snapshot = (job.get("model_snapshot") or "").strip().lower() - if _model_snapshot and not (job.get("model") or "").strip(): - _current_model = str(primary_model_for_drift or "").strip().lower() - if _current_model and _current_model != _model_snapshot: - _drift.append( - f"model '{_model_snapshot}' -> '{_current_model}'" + raise RuntimeError( + f"Skipped to prevent unintended spend: global inference config " + f"drifted since this job was created ({_changes}), and this job " + f"is unpinned. No inference call was made. To run on the new " + f"config, pin it explicitly: `cronjob action=update " + f"job_id={job_id} provider= model=` " + f"(or pin the original values to keep them). See #44585." ) - if _drift: - _changes = "; ".join(_drift) - logger.warning( - "Job '%s': SKIPPED — global inference config drifted since " - "creation (%s) and this job is unpinned. Skipped to prevent " - "unintended spend. Pin explicitly to proceed: " - "`cronjob action=update job_id=%s provider=

model=`.", - job_id, - _changes, - job_id, - ) - raise RuntimeError( - f"Skipped to prevent unintended spend: global inference config " - f"drifted since this job was created ({_changes}), and this job " - f"is unpinned. No inference call was made. To run on the new " - f"config, pin it explicitly: `cronjob action=update " - f"job_id={job_id} provider= model=` " - f"(or pin the original values to keep them). See #44585." - ) fallback_model = get_fallback_chain(_cfg) or None credential_pool = None diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 60886675143..94eb49d6de9 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2906,6 +2906,11 @@ DEFAULT_CONFIG = { }, "cron": { + # Fail closed when an unpinned job's current global model/provider + # differs from its creation-time snapshot. This prevents unattended + # jobs from silently inheriting a paid default. Set to false only when + # jobs should deliberately track changing global inference defaults. + "model_drift_guard": True, # Active cron SCHEDULER provider (Axis B — the trigger that decides # WHEN a due job fires). Empty string = the built-in in-process 60s # ticker (default). Name an installed provider (plugins/cron_providers// or @@ -8848,6 +8853,109 @@ def edit_config(): subprocess.run([editor, str(config_path)]) +def _cron_model_drift_axis_for_config_key(key: str) -> Optional[str]: + """Return the cron drift guard axis affected by a config key, if any.""" + normalized = str(key or "").strip().lower() + if normalized in {"model", "model.default", "model.model"}: + return "model" + if normalized in {"model.provider", "provider"}: + return "provider" + return None + + +def cron_model_drift_guard_enabled( + config: Optional[Dict[str, Any]] = None, +) -> bool: + """Return whether cron must fail closed on unpinned inference drift. + + Only the literal YAML boolean ``false`` disables this spend-safety guard. + Missing, malformed, or non-boolean values stay fail-closed. When *config* + is omitted, load the active merged configuration so CLI warnings honor the + same user/managed setting as the scheduler. + """ + if config is None: + try: + config = load_config() + except Exception: + return True + if not isinstance(config, dict): + return True + cron_config = config.get("cron") + if not isinstance(cron_config, dict): + return True + return cron_config.get("model_drift_guard", True) is not False + + +def _load_cron_jobs_for_config_warning() -> List[Dict[str, Any]]: + """Best-effort direct read of the active profile's cron jobs database.""" + jobs_path = get_hermes_home() / "cron" / "jobs.json" + try: + if not jobs_path.exists(): + return [] + data = json.loads(jobs_path.read_text(encoding="utf-8")) + except Exception: + return [] + + if isinstance(data, dict): + raw_jobs = data.get("jobs", []) + elif isinstance(data, list): + raw_jobs = data + else: + return [] + if not isinstance(raw_jobs, list): + return [] + return [job for job in raw_jobs if isinstance(job, dict)] + + +def warn_unpinned_cron_jobs_after_model_config_change( + key: str, + value: Any, +) -> None: + """Warn when a global model/provider change will trip cron's drift guard. + + Cron intentionally fails closed when an unpinned agent job's current global + model/provider differs from its creation-time snapshot. Surface that outcome + when the operator changes the global axis instead of letting the next tick + be the first visible signal. + """ + axis = _cron_model_drift_axis_for_config_key(key) + if axis is None: + return + if not cron_model_drift_guard_enabled(): + return + + new_value = str(value or "").strip().lower() + if not new_value: + return + + pinned_field = axis + snapshot_field = f"{axis}_snapshot" + affected = 0 + for job in _load_cron_jobs_for_config_warning(): + if not job.get("enabled", True): + continue + if job.get("no_agent"): + continue + if str(job.get(pinned_field) or "").strip(): + continue + snapshot = str(job.get(snapshot_field) or "").strip().lower() + if snapshot and snapshot != new_value: + affected += 1 + + if affected <= 0: + return + + noun = "job" if affected == 1 else "jobs" + print( + f"⚠️ {affected} enabled unpinned cron {noun} have stored " + f"{snapshot_field} values that differ from the new global {axis}. " + "They will fail closed on their next run instead of silently using the " + "changed model/provider. Inspect with `hermes cron list`, then pin the " + "intended values with `cronjob action=update job_id= " + "provider= model=`." + ) + + def _default_value_for_key(dotted_key: str): """Return the leaf value declared for *dotted_key* in ``DEFAULT_CONFIG``. @@ -9162,6 +9270,7 @@ def set_config_value(key: str, value: str, force: bool = False): else: _display_value = value print(f"✓ Set {key} = {_display_value} in {config_path}") + warn_unpinned_cron_jobs_after_model_config_change(key, value) # Post-write unknown-key notice (#34067): value IS saved, but tell the # user the runtime may never read it and suggest the likely-intended path. diff --git a/tests/cli/test_cli_save_config_value.py b/tests/cli/test_cli_save_config_value.py index 49cdd623564..a966217065b 100644 --- a/tests/cli/test_cli_save_config_value.py +++ b/tests/cli/test_cli_save_config_value.py @@ -77,6 +77,18 @@ class TestSaveConfigValueAtomic: assert result["model"]["default"] == "doubao-pro" assert result["custom_providers"][0]["api_key"] == "${TU_ZI_API_KEY}" + def test_model_write_runs_shared_cron_drift_warning(self, config_env, monkeypatch): + warning = MagicMock() + monkeypatch.setattr( + "hermes_cli.config.warn_unpinned_cron_jobs_after_model_config_change", + warning, + ) + + from cli import save_config_value + + assert save_config_value("model.default", "new-model") is True + warning.assert_called_once_with("model.default", "new-model") + def test_preserves_comments_after_config_mutation(self, config_env): """CLI config writes should not strip existing user comments.""" config_env.write_text( diff --git a/tests/cron/test_cron_provider_pin.py b/tests/cron/test_cron_provider_pin.py index 23a42ce99b1..4e36c174508 100644 --- a/tests/cron/test_cron_provider_pin.py +++ b/tests/cron/test_cron_provider_pin.py @@ -243,12 +243,22 @@ class TestCreateJobSnapshot: assert job["model_snapshot"] is None -def _run_with_current_provider_and_model(job, current_provider, current_model, tmp_path): +def _run_with_current_provider_and_model( + job, + current_provider, + current_model, + tmp_path, + *, + model_drift_guard=None, +): """Drive run_job with resolved provider pinned and config.yaml model.default set to ``current_model`` (the unpinned-model fire-time source).""" - (tmp_path / "config.yaml").write_text( - f"model:\n default: {current_model}\n" - ) + config_yaml = f"model:\n default: {current_model}\n" + if model_drift_guard is not None: + config_yaml += ( + f"cron:\n model_drift_guard: {str(model_drift_guard).lower()}\n" + ) + (tmp_path / "config.yaml").write_text(config_yaml) fake_db = MagicMock() with patch("cron.scheduler._hermes_home", tmp_path), \ patch("cron.scheduler._get_hermes_home", return_value=tmp_path), \ @@ -335,6 +345,26 @@ class TestModelDriftGuard: assert agent_constructed is True assert success is True + def test_explicit_opt_out_allows_provider_and_model_drift(self, tmp_path): + """The opt-out lets large unpinned fleets track changing defaults.""" + job = _base_job( + provider_snapshot="old-provider", + model_snapshot="old-model", + ) + success, output, final_response, error, agent_constructed = \ + _run_with_current_provider_and_model( + job, + "new-provider", + "new-model", + tmp_path, + model_drift_guard=False, + ) + + assert agent_constructed is True + assert success is True + assert final_response == "ok" + assert error is None + class TestRuntimeResolutionTargetModel: """run_job must resolve the primary provider against the model the job diff --git a/tests/hermes_cli/test_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index 0e2cb6fd57c..ece13dc65a2 100644 --- a/tests/hermes_cli/test_set_config_value.py +++ b/tests/hermes_cli/test_set_config_value.py @@ -1,12 +1,17 @@ """Tests for set_config_value — verifying secrets route to .env and config to config.yaml.""" import argparse +import json import os from unittest.mock import patch import pytest -from hermes_cli.config import set_config_value, config_command +from hermes_cli.config import ( + config_command, + cron_model_drift_guard_enabled, + set_config_value, +) @pytest.fixture(autouse=True) @@ -370,6 +375,217 @@ class TestListNavigation: assert allowlist[1] == {"name": "bob", "role": "admin"} +# --------------------------------------------------------------------------- +# Cron drift guard warning — regression tests for #59031 +# --------------------------------------------------------------------------- + +def _write_cron_jobs(tmp_path, jobs): + cron_dir = tmp_path / "cron" + cron_dir.mkdir(parents=True, exist_ok=True) + (cron_dir / "jobs.json").write_text( + json.dumps({"jobs": jobs}), + encoding="utf-8", + ) + + +class TestCronModelDriftConfigWarning: + """Warn operators before unpinned snapshot-bearing cron jobs fail closed.""" + + def test_model_default_change_warns_for_unpinned_snapshot_jobs( + self, + _isolated_hermes_home, + capsys, + ): + _write_cron_jobs( + _isolated_hermes_home, + [ + { + "id": "model-drift-job", + "enabled": True, + "no_agent": False, + "model": None, + "provider": None, + "model_snapshot": "old-model", + "provider_snapshot": "openrouter", + "prompt": "do not print this prompt", + }, + ], + ) + + set_config_value("model.default", "new-model") + + captured = capsys.readouterr() + assert "1 enabled unpinned cron job" in captured.out + assert "model_snapshot" in captured.out + assert "fail closed" in captured.out + assert "cronjob action=update job_id= provider= model=" in captured.out + assert "do not print this prompt" not in captured.out + + def test_provider_change_warns_for_unpinned_snapshot_jobs( + self, + _isolated_hermes_home, + capsys, + ): + _write_cron_jobs( + _isolated_hermes_home, + [ + { + "id": "provider-drift-job", + "enabled": True, + "no_agent": False, + "model": None, + "provider": None, + "model_snapshot": "same-model", + "provider_snapshot": "openrouter", + }, + ], + ) + + set_config_value("model.provider", "new-provider") + + captured = capsys.readouterr() + assert "1 enabled unpinned cron job" in captured.out + assert "provider_snapshot" in captured.out + assert "new global provider" in captured.out + assert "cronjob action=update job_id= provider= model=" in captured.out + + def test_pinned_jobs_and_missing_snapshots_do_not_warn( + self, + _isolated_hermes_home, + capsys, + ): + _write_cron_jobs( + _isolated_hermes_home, + [ + { + "id": "model-pinned", + "enabled": True, + "model": "old-model", + "model_snapshot": "old-model", + }, + { + "id": "missing-model-snapshot", + "enabled": True, + "model": None, + }, + { + "id": "provider-pinned", + "enabled": True, + "provider": "openrouter", + "provider_snapshot": "openrouter", + }, + { + "id": "missing-provider-snapshot", + "enabled": True, + "provider": None, + }, + { + "id": "disabled-unpinned", + "enabled": False, + "model": None, + "model_snapshot": "old-model", + "provider": None, + "provider_snapshot": "openrouter", + }, + { + "id": "script-only", + "enabled": True, + "no_agent": True, + "model": None, + "model_snapshot": "old-model", + "provider": None, + "provider_snapshot": "openrouter", + }, + ], + ) + + set_config_value("model.default", "new-model") + set_config_value("model.provider", "new-provider") + + captured = capsys.readouterr() + assert "fail closed" not in captured.out + assert "cronjob action=update" not in captured.out + + def test_unreadable_cron_database_does_not_break_config_set( + self, + _isolated_hermes_home, + capsys, + ): + cron_dir = _isolated_hermes_home / "cron" + cron_dir.mkdir(parents=True, exist_ok=True) + (cron_dir / "jobs.json").write_text("{not-json", encoding="utf-8") + + set_config_value("model.default", "new-model") + + captured = capsys.readouterr() + assert "Set model.default = new-model" in captured.out + assert "fail closed" not in captured.out + + def test_model_name_does_not_warn_for_unread_cron_axis( + self, + _isolated_hermes_home, + capsys, + ): + _write_cron_jobs( + _isolated_hermes_home, + [ + { + "id": "model-drift-job", + "enabled": True, + "model_snapshot": "old-model", + } + ], + ) + + set_config_value("model.name", "display-only-name") + + captured = capsys.readouterr() + assert "Set model.name = display-only-name" in captured.out + assert "fail closed" not in captured.out + + def test_explicit_opt_out_suppresses_warning( + self, + _isolated_hermes_home, + capsys, + ): + _write_cron_jobs( + _isolated_hermes_home, + [ + { + "id": "model-drift-job", + "enabled": True, + "model": None, + "model_snapshot": "old-model", + } + ], + ) + + set_config_value("cron.model_drift_guard", "false") + capsys.readouterr() + set_config_value("model.default", "new-model") + + import yaml + reloaded = yaml.safe_load(_read_config(_isolated_hermes_home)) + captured = capsys.readouterr() + assert reloaded["cron"]["model_drift_guard"] is False + assert "Set model.default = new-model" in captured.out + assert "fail closed" not in captured.out + + @pytest.mark.parametrize( + ("configured_value", "expected"), + [ + (False, False), + (True, True), + ("false", True), + (0, True), + (None, True), + ], + ) + def test_only_literal_false_disables_guard(self, configured_value, expected): + config = {"cron": {"model_drift_guard": configured_value}} + assert cron_model_drift_guard_enabled(config) is expected + + # --------------------------------------------------------------------------- # String-typed config values — regression tests for #47515 # --------------------------------------------------------------------------- diff --git a/website/docs/user-guide/features/cron.md b/website/docs/user-guide/features/cron.md index 44ad20cf808..c42b1a1e137 100644 --- a/website/docs/user-guide/features/cron.md +++ b/website/docs/user-guide/features/cron.md @@ -22,7 +22,7 @@ Cron jobs can: All of this is available to Hermes itself through the `cronjob` tool, so you can create, pause, edit, and remove jobs by asking in plain language — no CLI required. :::tip -At creation, an unpinned job (one you don't give an explicit `provider`/`model`) follows the global default selected by `hermes model` — and Hermes **snapshots** that provider and model on the job. If the global default later changes, the job **fails closed**: it skips the run, makes no inference call, and sends an alert telling you to pin the provider/model explicitly (`cronjob action=update job_id=… provider=… model=…`) to proceed. This prevents an unattended job from silently inheriting a switch to a paid provider/model and spending money you didn't intend (#44585). To make a job deliberately track your global default, pin it to the new values after changing them. `hermes setup --portal` is the lowest-friction option for unattended runs since OAuth refresh is automatic. See [Nous Portal](/integrations/nous-portal). +At creation, an unpinned job (one you don't give an explicit `provider`/`model`) follows the global default selected by `hermes model` — and Hermes **snapshots** that provider and model on the job. If the global default later changes, the job **fails closed**: it skips the run, makes no inference call, and sends an alert telling you to pin the provider/model explicitly (`cronjob action=update job_id=… provider=… model=…`) to proceed. This prevents an unattended job from silently inheriting a switch to a paid provider/model and spending money you didn't intend (#44585). To make a job deliberately track your global default, pin it to the new values after changing them. Operators who intentionally maintain large fleets of unpinned jobs can [disable the drift guard](#letting-unpinned-jobs-track-global-defaults). `hermes setup --portal` is the lowest-friction option for unattended runs since OAuth refresh is automatic. See [Nous Portal](/integrations/nous-portal). ::: :::warning @@ -61,6 +61,33 @@ Every morning at 9am, check Hacker News for AI news and send me a summary on Tel Hermes will use the unified `cronjob` tool internally. +## Letting unpinned jobs track global defaults + +The model/provider drift guard is enabled by default. If your unpinned cron +jobs should deliberately follow every global model or provider change, disable +it in `config.yaml`: + +```yaml +cron: + model_drift_guard: false +``` + +Or use the config command: + +```bash +hermes config set cron.model_drift_guard false +``` + +This disables both the runtime block and the warning shown when global +inference settings change. Existing snapshots remain stored, so setting the +option back to `true` re-enables protection without recreating jobs. + +:::warning +With the guard disabled, unattended unpinned jobs immediately inherit changed +global defaults. A switch to a paid provider or model can therefore spend money +on every scheduled run. +::: + ## Skill-backed cron jobs A cron job can load one or more skills before it runs the prompt.