fix(update): purge stale Hermes modules before post-pull config migrations
The pinned-list reload (tools_config) fixes the reported symbol; any future symbol a pull adds to any module a migration imports at call time (agent.skill_utils, hermes_cli.toolset_scope, ...) would fail the same way. Evict every cached Hermes module at the migration entry point with _purge_stale_hermes_modules — the class fix the fleet-restart phase already relies on — so the migration graph is rebuilt from the new tree. Tests: the reporter's exact shape (cached tools_config lacking _configurable_keys, on-disk v44 config with an explicit platform toolset list) must still migrate v44 -> v45 through the updater's own entry point. Existing entry-point tests stub the purge like the rest of the update suite does.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user