Cron scheduler null model resolution and caching fix

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #50011 fix(cron): resolve model.default + fail fast on missing model

  merged to main

  files: cron/scheduler.py, scripts/release.py, tests/cron/conftest.py, tests/cron/test_scheduler.py

  Show the diff

  • cron/scheduler.py

    @@ -1652,6 +1652,11 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
                     else str(delivery_target["thread_id"])
                 )
     
    +        # Model resolution precedence: per-job override > HERMES_MODEL env >
    +        # config.yaml ``model:`` (string or ``{default: ...}``). The per-job
    +        # value is intentionally re-read from storage every tick so a
    +        # ``cronjob action=update model=...`` after a failed run takes effect
    +        # on the next tick — there is no in-memory cache.
             model = job.get("model") or os.getenv("HERMES_MODEL") or ""
     
             # Load config.yaml for model, reasoning, prefill, toolsets, provider routing
    @@ -1672,15 +1677,34 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
                     except Exception:
                         pass
                     _cfg = _expand_env_vars(_cfg)
    -                _model_cfg = _cfg.get("model", {})
    +                # Coerce null/missing to {} so a falsy default never
    +                # clobbers an already-resolved env value with ``None``.
    +                _model_cfg = _cfg.get("model") or {}
                     if not job.get("model"):
                         if isinstance(_model_cfg, str):
                             model = _model_cfg
                         elif isinstance(_model_cfg, dict):
    -                        model = _model_cfg.get("default", model)
    +                        # Mirror the CLI/oneshot resolution: prefer ``default``,
    +                        # accept a ``model`` alias, overwrite only when truthy.
    +                        _default = _model_cfg.get("default") or _model_cfg.get("model")
    +                        if _default:
    +                            model = _default
             except Exception as e:
                 logger.warning("Job '%s': failed to load config.yaml, using defaults: %s", job_id, e)
     
    +        # Fail fast if no model resolved from job / env / config.yaml: an empty
    +        # model otherwise reaches the provider as an opaque 400 (#23979).
    +        if not (isinstance(model, str) and model.strip()):
    +            raise RuntimeError(
    +                f"Cron job '{job_name}' has no model configured "
    +                f"(job.model={job.get('model')!r}, "
    +                f"HERMES_MODEL={os.getenv('HERMES_MODEL', '')!r}, "
    +                "config.yaml model.default missing or empty). "
    +                f"Set a per-job model via "
    +                f"`cronjob action=update job_id={job_id} model=<name>` or set a "
    +                "default with `hermes model <name>`."
    +            )
    +
             # Apply IPv4 preference if configured.
             try:
                 from hermes_constants import apply_ipv4_preference

  • scripts/release.py

    @@ -45,6 +45,8 @@
     
     # Auto-extracted from noreply emails + manual overrides
     AUTHOR_MAP = {
    +    "[redacted:email]": "szzhoujiarui-sketch",  # cron model.default salvage co-author (#45550)
    +    "[redacted:email]": "rayjun",  # cron model.default salvage co-author (#43952)
         "[redacted:email]": "izumi0uu",  # PR #49544 salvage (native rich reply echo; #49534)
         "[redacted:email]": "w31rdm4ch1nZ",
         "[redacted:email]": "x7peeps",

  • tests/cron/conftest.py

    @@ -0,0 +1,21 @@
    +"""Cron-test fixtures.
    +
    +Provides a default ``HERMES_MODEL`` for cron run_job tests so each one
    +doesn't have to spell out a model. The global conftest blanks
    +HERMES_MODEL hermetically; without this autouse fixture every cron test
    +that exercises ``run_job`` would hit the fail-fast guard added in
    +``cron/scheduler.py`` (see issue #23979) and have to be rewritten.
    +
    +Tests that specifically need ``HERMES_MODEL`` unset — model-resolution
    +edge cases — call ``monkeypatch.delenv("HERMES_MODEL", raising=False)``
    +inside the test, which overrides this fixture's value for that scope.
    +"""
    +
    +import pytest
    +
    +
    [redacted:email](autouse=True)
    +def _default_cron_test_model(monkeypatch):
    +    """Pin a default HERMES_MODEL so cron run_job tests have a resolvable model."""
    +    monkeypatch.setenv("HERMES_MODEL", "test-cron-default-model")
    +    yield

  • tests/cron/test_scheduler.py

    @@ -1616,6 +1616,7 @@ def test_legacy_agent_prefill_messages_file_is_loaded(self, tmp_path, monkeypatc
         def test_fallback_model_env_ref_in_config_yaml_is_expanded(self, tmp_path, monkeypatch):
             """${VAR} in config.yaml fallback_providers model: is expanded."""
             (tmp_path / "config.yaml").write_text(
    +            "model: primary-model\n"
                 "fallback_providers:\n"
                 "  - provider: openrouter\n"
                 "    model: ${_HERMES_TEST_CRON_FALLBACK}\n"
    @@ -1672,6 +1673,238 @@ def test_unexpanded_ref_passthrough_when_var_unset(self, tmp_path, monkeypatch):
             assert kwargs["model"] == "${_HERMES_TEST_CRON_UNSET_VAR}"
     
     
    +class TestRunJobModelResolution:
    +    """Verify defensive model resolution for jobs stored with ``model: null``.
    +
    +    Issue #23979: a cron job created without an explicit model is stored as
    +    ``model: null``. At fire time the scheduler must:
    +      1. fall back to ``HERMES_MODEL`` env if set,
    +      2. else fall back to config.yaml ``model.default`` if set,
    +      3. else fail fast with an actionable error — never let an empty string
    +         reach the provider where it surfaces as an opaque 400.
    +    """
    +
    +    _RUNTIME = {
    +        "api_key": "[redacted:secret-assignment]",
    +        "base_url": "https://example.invalid/v1",
    +        "provider": "openrouter",
    +        "api_mode": "chat_completions",
    +    }
    +
    +    def test_null_job_model_falls_back_to_env(self, tmp_path, monkeypatch):
    +        """``model: null`` on the job uses HERMES_MODEL when set."""
    +        (tmp_path / "config.yaml").write_text("")
    +        monkeypatch.setenv("HERMES_MODEL", "env-model")
    +
    +        job = {"id": "null-model-job", "name": "null model", "prompt": "hi", "model": None}
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch("hermes_cli.runtime_provider.resolve_runtime_provider",
    +                   return_value=self._RUNTIME), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = {"final_response": "ok"}
    +            mock_agent_cls.return_value = mock_agent
    +            success, _, _, error = run_job(job)
    +
    +        assert success is True
    +        assert error is None
    +        assert mock_agent_cls.call_args.kwargs["model"] == "env-model"
    +
    +    def test_null_job_model_falls_back_to_config_default(self, tmp_path, monkeypatch):
    +        """``model: null`` on the job uses config.yaml model.default when env is empty."""
    +        (tmp_path / "config.yaml").write_text("model:\n  default: config-default-model\n")
    +        monkeypatch.delenv("HERMES_MODEL", raising=False)
    +
    +        job = {"id": "cfg-default-job", "name": "cfg default", "prompt": "hi", "model": None}
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch("hermes_cli.runtime_provider.resolve_runtime_provider",
    +                   return_value=self._RUNTIME), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = {"final_response": "ok"}
    +            mock_agent_cls.return_value = mock_agent
    +            success, _, _, error = run_job(job)
    +
    +        assert success is True
    +        assert error is None
    +        assert mock_agent_cls.call_args.kwargs["model"] == "config-default-model"
    +
    +    def test_explicit_null_model_block_in_config_does_not_overwrite_env(self, tmp_path, monkeypatch):
    +        """``model: null`` in config.yaml must not overwrite a resolved HERMES_MODEL.
    +
    +        Regression: before #23979 the resolver coerced ``model: null`` to
    +        ``{}`` only via the ``.get("model", {})`` default — which does not
    +        fire when the key is present with a None value. The resolver then
    +        skipped both branches and kept the env value, but a similar
    +        ``model: {default: null}`` shape would call ``.get("default", model)``
    +        which returns ``None`` and clobbered ``model``.
    +        """
    +        (tmp_path / "config.yaml").write_text("model:\n  default: null\n")
    +        monkeypatch.setenv("HERMES_MODEL", "env-model")
    +
    +        job = {"id": "null-default-job", "name": "null default", "prompt": "hi", "model": None}
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch("hermes_cli.runtime_provider.resolve_runtime_provider",
    +                   return_value=self._RUNTIME), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = {"final_response": "ok"}
    +            mock_agent_cls.return_value = mock_agent
    +            success, _, _, error = run_job(job)
    +
    +        assert success is True
    +        assert mock_agent_cls.call_args.kwargs["model"] == "env-model"
    +
    +    def test_no_model_anywhere_fails_with_actionable_error(self, tmp_path, monkeypatch):
    +        """All three sources empty → fail fast with a clear message, not an opaque 400."""
    +        (tmp_path / "config.yaml").write_text("")
    +        monkeypatch.delenv("HERMES_MODEL", raising=False)
    +
    +        job = {"id": "no-model-job", "name": "no model anywhere", "prompt": "hi", "model": None}
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch("hermes_cli.runtime_provider.resolve_runtime_provider",
    +                   return_value=self._RUNTIME), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            success, _, _, error = run_job(job)
    +
    +        assert success is False
    +        assert error is not None
    +        assert "no model configured" in error
    +        # AIAgent must never be constructed with an empty model — that's
    +        # precisely the bug we're guarding against.
    +        mock_agent_cls.assert_not_called()
    +
    +    def test_job_model_update_takes_effect_on_next_run(self, tmp_path, monkeypatch):
    +        """The per-job model is re-read every tick — no in-memory cache.
    +
    +        This is the property the original bug report asked for. We verify
    +        it by calling run_job twice with the same job dict mutated between
    +        calls, simulating the storage update flow.
    +        """
    +        (tmp_path / "config.yaml").write_text("")
    +        monkeypatch.delenv("HERMES_MODEL", raising=False)
    +
    +        job = {"id": "updated-model-job", "name": "updated", "prompt": "hi", "model": "first-model"}
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch("hermes_cli.runtime_provider.resolve_runtime_provider",
    +                   return_value=self._RUNTIME), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = {"final_response": "ok"}
    +            mock_agent_cls.return_value = mock_agent
    +
    +            run_job(job)
    +            assert mock_agent_cls.call_args.kwargs["model"] == "first-model"
    +
    +            job["model"] = "second-model"  # simulates jobs.json being rewritten
    +            run_job(job)
    +            assert mock_agent_cls.call_args.kwargs["model"] == "second-model"
    +
    +    def test_config_model_as_plain_string(self, tmp_path, monkeypatch):
    +        """config.yaml ``model:`` given as a bare string is used directly."""
    +        (tmp_path / "config.yaml").write_text("model: string-form-model\n")
    +        monkeypatch.delenv("HERMES_MODEL", raising=False)
    +
    +        job = {"id": "string-cfg-job", "name": "string cfg", "prompt": "hi", "model": None}
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch("hermes_cli.runtime_provider.resolve_runtime_provider",
    +                   return_value=self._RUNTIME), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = {"final_response": "ok"}
    +            mock_agent_cls.return_value = mock_agent
    +            success, _, _, error = run_job(job)
    +
    +        assert success is True
    +        assert error is None
    +        assert mock_agent_cls.call_args.kwargs["model"] == "string-form-model"
    +
    +    def test_config_model_alias_key_resolves(self, tmp_path, monkeypatch):
    +        """A ``model: {model: ...}`` alias key resolves like the CLI sibling.
    +
    +        ``hermes_cli/oneshot.py``, ``fallback_cmd.py`` and ``prompt_size.py``
    +        all accept ``model.model`` as an alias for ``model.default``. The cron
    +        resolver mirrors that so a config that works in the CLI also works in
    +        cron.
    +        """
    +        (tmp_path / "config.yaml").write_text("model:\n  model: alias-key-model\n")
    +        monkeypatch.delenv("HERMES_MODEL", raising=False)
    +
    +        job = {"id": "alias-job", "name": "alias", "prompt": "hi", "model": None}
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch("hermes_cli.runtime_provider.resolve_runtime_provider",
    +                   return_value=self._RUNTIME), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = {"final_response": "ok"}
    +            mock_agent_cls.return_value = mock_agent
    +            success, _, _, error = run_job(job)
    +
    +        assert success is True
    +        assert error is None
    +        assert mock_agent_cls.call_args.kwargs["model"] == "alias-key-model"
    +
    +    def test_corrupt_config_yaml_does_not_crash_with_job_model(self, tmp_path, monkeypatch):
    +        """A malformed config.yaml degrades gracefully when the job has a model."""
    +        (tmp_path / "config.yaml").write_text("{{{invalid yaml!!!")
    +        monkeypatch.delenv("HERMES_MODEL", raising=False)
    +
    +        job = {"id": "corrupt-job", "name": "corrupt", "prompt": "hi", "model": "explicit-model"}
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch("hermes_cli.runtime_provider.resolve_runtime_provider",
    +                   return_value=self._RUNTIME), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = {"final_response": "ok"}
    +            mock_agent_cls.return_value = mock_agent
    +            success, _, _, error = run_job(job)
    +
    +        # Explicit job model survives the corrupt-config fall-through.
    +        assert success is True
    +        assert error is None
    +        assert mock_agent_cls.call_args.kwargs["model"] == "explicit-model"
    +
    +
     class TestRunJobSkillBacked:
         def test_run_job_preserves_skill_env_passthrough_into_worker_thread(self, tmp_path):
             job = {