diff --git a/hermes_cli/update_cmd_config.py b/hermes_cli/update_cmd_config.py index ad8400ddec..bf9139a390 100644 --- a/hermes_cli/update_cmd_config.py +++ b/hermes_cli/update_cmd_config.py @@ -175,6 +175,13 @@ def _check_and_apply_config_migration( _run_migrate_config_fresh) print() print("→ Checking configuration for new options...") + # Evict EVERY cached Hermes module before touching migration code: the updater is the + # pre-pull process, and a migration step's call-time import (``_migrate_to_45`` → + # ``hermes_cli.tools_config._configurable_keys``) resolves against whatever old module + # object is still cached — reloading a hand-picked list re-fixes this per symptom + # (#111271). The purge is the class fix already used by the fleet-restart phase. + from hermes_cli.update_cmd import _m + _m()._purge_stale_hermes_modules() # Reload BEFORE any config reads so all checks use the updated code. _reload_config_modules() from hermes_cli.config import get_missing_env_vars, get_missing_config_fields diff --git a/tests/hermes_cli/test_update_config_migration_on_current_checkout.py b/tests/hermes_cli/test_update_config_migration_on_current_checkout.py index f35d359d06..563213507c 100644 --- a/tests/hermes_cli/test_update_config_migration_on_current_checkout.py +++ b/tests/hermes_cli/test_update_config_migration_on_current_checkout.py @@ -10,9 +10,19 @@ from __future__ import annotations from unittest.mock import MagicMock, patch +import pytest + +from hermes_cli import main as cli_main from hermes_cli import update_cmd +@pytest.fixture(autouse=True) +def _no_stale_module_purge(monkeypatch): + """The migration step evicts every cached Hermes module first (#111271); a real purge + would discard the ``hermes_cli.config`` object these tests patch.""" + monkeypatch.setattr(cli_main, "_purge_stale_hermes_modules", lambda: None) + + def test_repair_node_deps_runs_config_migration_on_version_bump(capsys): """When on-disk config version is behind, _repair_node_deps_on_current_checkout must run _check_and_apply_config_migration and migrate the config.""" diff --git a/tests/hermes_cli/test_update_config_reload_tools_config.py b/tests/hermes_cli/test_update_config_reload_tools_config.py index f5f6826889..5e4d37ba10 100644 --- a/tests/hermes_cli/test_update_config_reload_tools_config.py +++ b/tests/hermes_cli/test_update_config_reload_tools_config.py @@ -1,17 +1,12 @@ -"""Regression tests for the ``tools_config`` leg of the post-update config reload. +"""``hermes update`` runs config migrations in the PRE-pull updater process. A migration step +that imports a helper at call time (``_migrate_to_45`` → ``hermes_cli.tools_config``) resolves +against whatever OLD module object is still cached, and dies with ``cannot import name`` when the +pull added that symbol — config silently stays behind while the code moves on (#111271). -``hermes update`` runs config migrations in the updater process — the PRE-pull -process — after refreshing the config modules from the pulled tree. Migrations -import ``hermes_cli.tools_config`` helpers at call time, so a ``tools_config`` -cached in ``sys.modules`` from before the pull makes the import fail with -``ImportError`` even though the symbol exists in the freshly-written file. - -History: -- #111271: the v45 migration (``_migrate_to_45``) imports ``_configurable_keys`` - from ``hermes_cli.tools_config``; on a v0.20.6 → v0.21.3 update the cached - pre-pull module lacked that newly-added symbol, the migration aborted with - "cannot import name '_configurable_keys'", and the config silently stayed at - the old version while the code and gateway moved on (update exit 1). +The class fix: the migration step evicts every cached Hermes module (``_purge_stale_hermes_modules``, +the same primitive the fleet-restart phase uses) before importing migration code, so ANY symbol the +pull added to ANY module a migration imports is found. The pinned-list reload of ``tools_config`` is +the per-symptom half; the purge covers the class. """ from __future__ import annotations @@ -19,14 +14,20 @@ from __future__ import annotations import importlib import sys +import pytest + + +@pytest.fixture(autouse=True) +def _restore_sys_modules(): + """The purge under test evicts real Hermes modules; put the originals back for later tests.""" + snapshot = dict(sys.modules) + yield + for name, mod in snapshot.items(): + sys.modules[name] = mod + def test_reload_config_modules_restores_missing_tools_config_symbol(): - """A pre-pull ``tools_config`` cache must be reloaded before migrations run. - - Simulates the stale updater process by deleting the migration-imported - symbol from the already-loaded module, then asserts ``_reload_config_modules`` - re-executes the on-disk code and restores it. - """ + """A pre-pull ``tools_config`` cache must be reloaded before migrations run.""" tools_config = sys.modules.get("hermes_cli.tools_config") if tools_config is None: import hermes_cli.tools_config as tools_config # noqa: F811 @@ -43,13 +44,26 @@ def test_reload_config_modules_restores_missing_tools_config_symbol(): importlib.reload(reloaded) # keep other tests on a clean module -def test_reload_config_modules_is_noop_without_cached_tools_config(): - """The reload must stay safe when ``tools_config`` was never imported.""" - from hermes_cli.update_cmd_config import _reload_config_modules - was_loaded = sys.modules.pop("hermes_cli.tools_config", None) +def test_update_migration_survives_stale_module_missing_call_time_symbol(tmp_path, monkeypatch): + """The reporter's exact shape: the cached ``tools_config`` predates ``_configurable_keys`` while + the on-disk v45 step imports it at call time. The updater's migration entry must still land + v44 → v45 instead of printing "Config format update failed".""" + home = tmp_path / "flat-home" + home.mkdir() + (home / "config.yaml").write_text( + "_config_version: 44\nplatform_toolsets:\n telegram:\n - web\n - file\n", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(home)) + from hermes_constants import set_hermes_home_override + token = set_hermes_home_override(home) try: - _reload_config_modules() + import hermes_cli.tools_config as tools_config + monkeypatch.delattr(tools_config, "_configurable_keys") + + from hermes_cli.update_cmd import _check_and_apply_config_migration + _check_and_apply_config_migration(assume_yes=True, gateway_mode=False, pre_update_snapshot_id=None) finally: - if was_loaded is not None: - sys.modules["hermes_cli.tools_config"] = was_loaded - # No assertion on the module here: the point is that the helper did not raise. + from hermes_constants import reset_hermes_home_override + reset_hermes_home_override(token) + + text = (home / "config.yaml").read_text(encoding="utf-8") + assert "_config_version: 45" in text, text