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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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) == []
|
||||
|
||||
Reference in New Issue
Block a user