From da451afb46ce3d88193c427455b72e953a8a343f Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 12 Sep 2026 08:37:30 -0700 Subject: [PATCH] fix(update): probe configured-feature deps in the target venv, not the updater The check called the registry check_fn inside the updater's own process, whose import caches predate the install just performed (and which may be the outer Python entirely), so a healthy freshly installed SDK produced a false "will fail to load" warning. Run the same registry check in the target interpreter via the existing _venv_probe path used by the core dependency verifier. Found by independent review before merge. --- hermes_cli/main_install_repair.py | 64 ++++++++++++------- .../test_update_missing_configured_deps.py | 42 ++++++++---- 2 files changed, 71 insertions(+), 35 deletions(-) diff --git a/hermes_cli/main_install_repair.py b/hermes_cli/main_install_repair.py index f909386bf1..303e447ddc 100644 --- a/hermes_cli/main_install_repair.py +++ b/hermes_cli/main_install_repair.py @@ -984,41 +984,57 @@ def _install_python_dependencies_with_optional_fallback( print(f" ✓ Reinstalled optional extras individually: {', '.join(installed_extras)}") if failed_extras: print(f" ⚠ Skipped optional extras that still failed: {', '.join(failed_extras)}") - _warn_configured_features_missing_deps(install_cmd_prefix) + _warn_configured_features_missing_deps(install_cmd_prefix, env=env) # uv's incremental resolver has left newly added base deps silently missing on a half-stale # venv, surfacing hours later as a downstream ModuleNotFoundError. Verify here instead. _verify_core_dependencies_installed(install_cmd_prefix, env=env, group=group) _verify_console_scripts_installed(install_cmd_prefix, env=env) -def _configured_features_missing_deps() -> list[tuple[str, str]]: +_CONFIGURED_FEATURES_SCRIPT = ( + "import importlib.util\n" + "try:\n" + " from gateway.config import load_gateway_config\n" + " from gateway.platform_registry import platform_registry\n" + " for platform in load_gateway_config().get_connected_platforms():\n" + " entry = platform_registry.get(platform.value)\n" + " if entry is not None and not entry.check_fn():\n" + " print(entry.label + '\\t' + (entry.install_hint or 'reinstall the %r extra' % platform.value))\n" + "except Exception:\n" + " pass\n" + "try:\n" + " from hermes_cli.config import load_config\n" + " if (load_config().get('mcp_servers') or {}) and importlib.util.find_spec('mcp') is None:\n" + " print('MCP servers\\tinstall the ' + repr('mcp') + ' extra')\n" + "except Exception:\n" + " pass\n" +) + + +def _configured_features_missing_deps( + install_cmd_prefix: list[str], *, env: dict[str, str] | None = None) -> list[tuple[str, str]]: """``(feature, hint)`` for every configured platform / MCP whose optional deps are not importable - in this interpreter. A running gateway masks the gap until its next restart (#10651), so the - update has to say which configured feature will fail to load.""" + in the TARGET venv. A running gateway masks the gap until its next restart (#10651), so the + update has to say which configured feature will fail to load. Probed in a fresh interpreter: + the updater's own process may be the outer Python and its import caches predate the install, + so an in-process ``check_fn`` reports stale state.""" + venv_python = _resolve_install_target_python(install_cmd_prefix, env) + if venv_python is None: + return [] + try: + result = _venv_probe(venv_python, _CONFIGURED_FEATURES_SCRIPT, env=env) + except Exception as exc: # the update must finish even when the probe cannot run + logger.debug("configured-feature dependency check skipped: %s", exc) + return [] missing: list[tuple[str, str]] = [] - try: - from gateway.config import load_gateway_config - from gateway.platform_registry import platform_registry - config = load_gateway_config() - for platform in config.get_connected_platforms(): - entry = platform_registry.get(platform.value) - if entry is None or entry.check_fn(): - continue - missing.append((entry.label, entry.install_hint or f"reinstall the '{platform.value}' extra")) - except Exception as exc: # the update must finish even when the gateway config is unreadable - logger.debug("configured-platform dependency check skipped: %s", exc) - try: - from hermes_cli.config import load_config - import importlib.util - if (load_config().get("mcp_servers") or {}) and importlib.util.find_spec("mcp") is None: - missing.append(("MCP servers", "install the 'mcp' extra")) - except Exception as exc: - logger.debug("configured-MCP dependency check skipped: %s", exc) + for line in _nonblank_lines(result.stdout): + feature, _, hint = line.partition("\t") + missing.append((feature, hint or "reinstall its optional extra")) return missing -def _warn_configured_features_missing_deps(install_cmd_prefix: list[str]) -> None: - missing = _configured_features_missing_deps() +def _warn_configured_features_missing_deps(install_cmd_prefix: list[str], *, env: dict[str, str] | None = None) -> None: + missing = _configured_features_missing_deps(install_cmd_prefix, env=env) if not missing: return prefix = " ".join(shlex.quote(part) for part in install_cmd_prefix) diff --git a/tests/hermes_cli/test_update_missing_configured_deps.py b/tests/hermes_cli/test_update_missing_configured_deps.py index aea881aee1..a38f035cb2 100644 --- a/tests/hermes_cli/test_update_missing_configured_deps.py +++ b/tests/hermes_cli/test_update_missing_configured_deps.py @@ -1,8 +1,9 @@ """Update fallback must name configured features whose optional deps stayed missing (#10651).""" +import os import subprocess -from types import SimpleNamespace -from unittest import mock +import sys +from pathlib import Path from hermes_cli import main_install_repair @@ -19,7 +20,7 @@ def _run_fallback_with_failed_extra(monkeypatch, capsys, *, extra_fails: str, mi monkeypatch.setattr(main_install_repair, "_venv_scripts_dir", lambda: None) monkeypatch.setattr(main_install_repair, "_is_windows", lambda: False) monkeypatch.setattr(main_install_repair, "_load_installable_optional_extras", lambda group="all": [extra_fails, "mcp"]) - monkeypatch.setattr(main_install_repair, "_configured_features_missing_deps", lambda: missing_features) + monkeypatch.setattr(main_install_repair, "_configured_features_missing_deps", lambda *a, **k: missing_features) main_install_repair._install_python_dependencies_with_optional_fallback(["uv", "pip"]) return capsys.readouterr().out @@ -36,11 +37,30 @@ def test_fallback_names_configured_platform_whose_extra_failed(monkeypatch, caps assert "fail to load them on restart" not in quiet -def test_configured_features_reports_platform_with_missing_deps(monkeypatch): - entry = SimpleNamespace(label="Feishu / Lark", check_fn=lambda: False, install_hint="Run `hermes setup`.") - fake_registry = SimpleNamespace(get=lambda name: entry if name == "feishu" else None) - fake_config = SimpleNamespace(get_connected_platforms=lambda: [SimpleNamespace(value="feishu")]) - with mock.patch("gateway.config.load_gateway_config", return_value=fake_config), \ - mock.patch("gateway.platform_registry.platform_registry", fake_registry), \ - mock.patch("hermes_cli.config.load_config", return_value={}): - assert main_install_repair._configured_features_missing_deps() == [("Feishu / Lark", "Run `hermes setup`.")] +def test_configured_features_probe_reads_the_fresh_target_interpreter(tmp_path, monkeypatch): + """The check runs in the TARGET interpreter with a real config, so it sees the post-install + truth rather than the updater's own pre-install import caches. Positive: the SDK is absent → + the configured platform is named. Negative: only the child interpreter is told the dependency + is present (the parent is untouched) → nothing is reported, proving the verdict comes from the + fresh process.""" + home = tmp_path / "home" + home.mkdir() + (home / "config.yaml").write_text( + "platforms:\n feishu:\n enabled: true\n extra:\n app_id: cli_x\n app_secret: y\n", + encoding="utf-8") + env = {**os.environ, "HERMES_HOME": str(home)} + monkeypatch.setattr(main_install_repair, "_resolve_install_target_python", lambda *a, **k: Path(sys.executable)) + real_probe = main_install_repair._venv_probe + + def probe(python, script, *args, env=None, prelude=""): + return real_probe(python, prelude + script, *args, env=env) + + monkeypatch.setattr(main_install_repair, "_venv_probe", + lambda p, s, *a, env=None: probe(p, s, *a, env=env, prelude="import sys; sys.modules['lark_oapi'] = None\n")) + missing = main_install_repair._configured_features_missing_deps(["uv", "pip"], env=env) + assert [feature for feature, _hint in missing] == ["Feishu / Lark"] + + child_only_present = "import tools.lazy_deps as ld; ld.is_available = lambda *_a, **_k: True\n" + monkeypatch.setattr(main_install_repair, "_venv_probe", + lambda p, s, *a, env=None: probe(p, s, *a, env=env, prelude=child_only_present)) + assert main_install_repair._configured_features_missing_deps(["uv", "pip"], env=env) == []