diff --git a/hermes_cli/cli_agent_setup_mixin.py b/hermes_cli/cli_agent_setup_mixin.py index 29173eda409ae..8b3ef0f1399d5 100644 --- a/hermes_cli/cli_agent_setup_mixin.py +++ b/hermes_cli/cli_agent_setup_mixin.py @@ -462,6 +462,10 @@ class CLIAgentSetupMixin: return True except Exception as e: ChatConsole().print(f"[bold red]Failed to initialize agent: {e}[/]") + from hermes_constants import partial_update_hint + + for line in partial_update_hint(e): + ChatConsole().print(line) return False def _preload_resumed_session(self) -> bool: diff --git a/hermes_cli/main.py b/hermes_cli/main.py index a8fb2ffc473ce..614ef37cec9c2 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -5071,6 +5071,7 @@ from hermes_cli.update_cmd import ( # noqa: F401 _update_via_zip, _upgrade_pip_before_lazy_refresh, _validate_critical_files_syntax, + _validate_critical_modules_import, _venv_core_imports_healthy, _venv_launcher_ancestors, _wait_for_windows_update_gateway_exit, @@ -5083,6 +5084,7 @@ from hermes_cli.update_cmd import ( # noqa: F401 _write_update_planned_stop_marker, _UPDATE_RUNTIME_RELOAD_MODULES, _UPDATE_CRITICAL_FILES, + _UPDATE_CRITICAL_MODULES, OFFICIAL_REPO_URLS, OFFICIAL_REPO_URL, SKIP_UPSTREAM_PROMPT_FILE, diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index a7b51e120ad52..a1f5873f007ce 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -157,6 +157,71 @@ def _validate_critical_files_syntax(root) -> tuple[bool, str | None, str | None] return False, str(path), f"could not read: {exc}" return True, None, None + +# Modules imported on every agent startup. Unlike _UPDATE_CRITICAL_FILES (which +# is only parsed), these are actually *imported* so that cross-module breakage +# is caught — a file can be syntactically perfect and still fail to import +# because a name it pulls from a sibling module no longer exists. +_UPDATE_CRITICAL_MODULES = ( + "hermes_cli.main", + "run_agent", + "model_tools", + "toolsets", +) + + +def _validate_critical_modules_import(root) -> tuple[bool, str | None, str | None]: + """Import each module in ``_UPDATE_CRITICAL_MODULES`` in a subprocess. + + ``_validate_critical_files_syntax`` only *parses* files, so it cannot see + cross-module breakage: a partially-updated tree where ``agent/`` is new but + ``tools/`` is old parses perfectly and still dies at startup with + ``ImportError: cannot import name 'TODO_INJECTION_HEADER' from + 'tools.todo_tool'``. Every file is valid Python; the *combination* is not. + + That skew is reachable on the Windows ZIP-update path, whose copy loop + walks top-level entries in ``os.listdir`` order and replaces each one + independently — ``agent/`` lands long before ``tools/``, so a failure or + interruption between them leaves exactly that mismatch on disk. + + Runs in a subprocess because importing these modules into the running + updater would pollute ``sys.modules`` and execute import-time side effects + against the half-updated tree. Costs ~0.4s. + + Returns ``(ok, failing_module, error_message)``. + """ + probe = ( + "import importlib, sys\n" + "for name in %r:\n" + " try:\n" + " importlib.import_module(name)\n" + " except ImportError as exc:\n" + " sys.stdout.write(name + '\\n' + str(exc))\n" + " raise SystemExit(3)\n" + " except Exception:\n" + " pass\n" # non-import errors (config/env) aren't update breakage + "raise SystemExit(0)\n" % (_UPDATE_CRITICAL_MODULES,) + ) + try: + result = subprocess.run( + [sys.executable, "-c", probe], + cwd=str(root), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=120, + ) + except (OSError, subprocess.SubprocessError): + # Can't run the probe — don't block the update on our own tooling. + return True, None, None + if result.returncode == 3: + parts = (result.stdout or "").split("\n", 1) + module = parts[0].strip() or "unknown" + detail = parts[1].strip() if len(parts) > 1 else "" + return False, module, detail + return True, None, None + def _gateway_prompt(prompt_text: str, default: str = "", timeout: float = 300.0) -> str: """File-based IPC prompt for gateway mode. @@ -627,6 +692,14 @@ def _update_via_zip(args): except Exception as e: print(f"✗ ZIP update failed: {e}") + print( + " The install may be partially updated — some directories were " + "replaced and others were not." + ) + print( + " Re-run `hermes update` to finish; if the agent won't start, " + "reinstall from https://hermes-agent.nousresearch.com" + ) _m().sys.exit(1) finally: shutil.rmtree(tmp_dir, ignore_errors=True) @@ -685,6 +758,27 @@ def _update_via_zip(args): # #70636). _m()._refresh_active_memory_provider_dependencies() + # Now that dependencies are installed, verify the tree actually imports. + # The copy loop above replaces top-level entries one at a time in + # os.listdir order, so an interruption between (say) `agent/` and `tools/` + # leaves a tree whose files all parse but cannot be imported together — + # the ImportError-on-startup class this guard exists to catch. Deliberately + # placed *after* the dependency reinstall so a genuinely-new third-party + # requirement isn't misreported as a partial copy. There is no SHA to roll + # back to here, so surface it with a concrete recovery step rather than + # reporting a successful update over a bricked install. + import_ok, failing_module, import_error = _validate_critical_modules_import( + _m().PROJECT_ROOT + ) + if not import_ok: + print() + print("✗ Update left the install in an unimportable state:") + print(f" {failing_module}: {import_error}") + print() + print(" This usually means the copy was interrupted partway through.") + print(" Re-run `hermes update` to complete it.") + _m().sys.exit(1) + node_failures = _update_node_dependencies() _m()._build_web_ui(_m().PROJECT_ROOT / "web") @@ -3701,9 +3795,22 @@ def _cmd_update_impl(args, gateway_mode: bool): syntax_ok, failing_path, syntax_error = _validate_critical_files_syntax( _m().PROJECT_ROOT ) + if syntax_ok: + # Parsing clean isn't enough: a tree can be syntactically valid + # and still unimportable when modules land out of sync (see + # _validate_critical_modules_import). Catch that here too so the + # same rollback path covers both failure modes. + ( + syntax_ok, + failing_path, + syntax_error, + ) = _m()._validate_critical_modules_import(_m().PROJECT_ROOT) + failure_label = "cannot be imported (partially-updated tree)" + else: + failure_label = "has a syntax error" if not syntax_ok: print() - print("✗ Pulled code has a syntax error in a critical file:") + print(f"✗ Pulled code {failure_label} in a critical file:") print(f" {failing_path}") if syntax_error: # py_compile errors can be multi-line; show the first diff --git a/hermes_constants.py b/hermes_constants.py index c5f33e47468df..4ea0c8376d1e5 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -1248,3 +1248,41 @@ OPENROUTER_BASE_URL = "https://openrouter.ai/api/v1" OPENROUTER_MODELS_URL = f"{OPENROUTER_BASE_URL}/models" AI_GATEWAY_BASE_URL = "https://ai-gateway.vercel.sh/v1" + + +# ─── Partial-update diagnostics ────────────────────────────────────────────── + +def partial_update_hint(exc: BaseException) -> list[str]: + """Return recovery guidance lines when *exc* looks like a half-updated tree. + + An interrupted or partially-applied update can leave the checkout with new + files in one package and stale files in another. Every file still parses, + so nothing is corrupt in the usual sense — but a module that imports a name + added in the same release from a sibling that wasn't refreshed dies with + ``ImportError: cannot import name 'X' from 'y'`` on every startup. + + Users hit this as an opaque crash with no indication that the *install*, + rather than their config, is the problem — and `hermes update` is exactly + the command they need but are least likely to trust after a failed update. + Return the guidance so callers can print it alongside the raw error. + + Returns an empty list for unrelated exceptions, so callers can splat it + unconditionally. + """ + if not isinstance(exc, ImportError): + return [] + # A missing third-party dependency is a different problem (bad venv, missing + # extra) with different remediation, so don't claim a partial update. + if isinstance(exc, ModuleNotFoundError): + return [] + name = getattr(exc, "name", None) + if not name or not str(name).startswith(("tools", "agent", "hermes", "gateway")): + return [] + return [ + "", + "This looks like a partially-updated install: one module was refreshed " + "and a related one was not.", + "Re-run the update to bring the whole tree to the same version:", + " hermes update", + "If that also fails, reinstall: https://hermes-agent.nousresearch.com", + ] diff --git a/tests/hermes_cli/test_update_import_guard.py b/tests/hermes_cli/test_update_import_guard.py new file mode 100644 index 0000000000000..1a0327095701c --- /dev/null +++ b/tests/hermes_cli/test_update_import_guard.py @@ -0,0 +1,117 @@ +"""Tests for the post-update *import* guard in ``hermes update``. + +``_validate_critical_files_syntax`` only parses files, so it cannot detect a +partially-updated tree: when one package is refreshed and a sibling is not, +every file still parses but importing them together raises ``ImportError``. + +Reference incident: a Windows user reported +``ImportError: cannot import name 'TODO_INJECTION_HEADER' from +'tools.todo_tool'`` on every startup after an update. ``agent/`` carried the +new ``context_compressor.py`` (which imports that name at module level) while +``tools/`` still held the pre-update ``todo_tool.py``. The ZIP-update path +replaces top-level entries one at a time in ``os.listdir`` order, so an +interruption between ``agent/`` and ``tools/`` produces exactly that skew -- +and the syntax guard reported the update as successful. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from hermes_cli import main as hermes_main +from hermes_cli import update_cmd +from hermes_constants import partial_update_hint + + +def _write_skewed_tree(root: Path, *, skewed: bool) -> None: + """Build a tiny two-package tree that mimics the real failure. + + ``consumer`` imports a name from ``provider`` at module level. When + ``skewed`` is True the name is absent -- both files still parse. + """ + (root / "provider").mkdir(parents=True, exist_ok=True) + (root / "provider" / "__init__.py").write_text("") + (root / "provider" / "thing.py").write_text( + "OTHER = 1\n" if skewed else "SHARED_NAME = 'x'\nOTHER = 1\n" + ) + (root / "consumer.py").write_text("from provider.thing import SHARED_NAME\n") + + +def test_syntax_guard_passes_but_import_guard_catches_skew(monkeypatch, tmp_path): + """The regression: a skewed tree parses cleanly but cannot be imported.""" + _write_skewed_tree(tmp_path, skewed=True) + + # Both files are valid Python -- the syntax guard sees nothing wrong. + monkeypatch.setattr( + hermes_main, "_UPDATE_CRITICAL_FILES", ("consumer.py", "provider/thing.py") + ) + syntax_ok, _, _ = hermes_main._validate_critical_files_syntax(tmp_path) + assert syntax_ok, "sanity: the skewed tree must parse cleanly" + + # The import guard catches it. + monkeypatch.setattr(update_cmd, "_UPDATE_CRITICAL_MODULES", ("consumer",)) + ok, module, error = hermes_main._validate_critical_modules_import(tmp_path) + assert ok is False + assert module == "consumer" + assert error is not None and "SHARED_NAME" in error + + +def test_import_guard_passes_on_consistent_tree(monkeypatch, tmp_path): + _write_skewed_tree(tmp_path, skewed=False) + monkeypatch.setattr(update_cmd, "_UPDATE_CRITICAL_MODULES", ("consumer",)) + + assert hermes_main._validate_critical_modules_import(tmp_path) == (True, None, None) + + +def test_import_guard_ignores_non_import_errors(monkeypatch, tmp_path): + """A module that raises at import time for config/env reasons is not + update breakage -- the guard must not roll back a good update.""" + (tmp_path / "consumer.py").write_text( + "raise RuntimeError('no API key configured')\n" + ) + monkeypatch.setattr(update_cmd, "_UPDATE_CRITICAL_MODULES", ("consumer",)) + + ok, _, _ = hermes_main._validate_critical_modules_import(tmp_path) + assert ok is True + + +def test_import_guard_is_non_fatal_when_probe_cannot_run(monkeypatch, tmp_path): + """If we can't spawn the probe, don't block the user's update.""" + + def boom(*_a, **_kw): + raise OSError("cannot spawn") + + monkeypatch.setattr(update_cmd.subprocess, "run", boom) + assert update_cmd._validate_critical_modules_import(tmp_path) == (True, None, None) + + +# --------------------------------------------------------------------------- +# partial_update_hint +# --------------------------------------------------------------------------- + +def test_hint_fires_for_first_party_import_error(): + exc = ImportError("cannot import name 'TODO_INJECTION_HEADER'") + exc.name = "tools.todo_tool" + + hint = partial_update_hint(exc) + + assert hint, "expected recovery guidance for a first-party ImportError" + assert any("hermes update" in line for line in hint) + + +@pytest.mark.parametrize( + "exc", + [ + ModuleNotFoundError("No module named 'numpy'", name="numpy"), + ValueError("unrelated"), + ImportError("third-party broke"), + ], +) +def test_hint_stays_silent_for_unrelated_failures(exc): + """Missing third-party deps and non-import errors have different + remediation -- claiming a partial update would misdirect the user.""" + if isinstance(exc, ImportError) and not isinstance(exc, ModuleNotFoundError): + exc.name = "requests" + assert partial_update_hint(exc) == []