From e4257c171ad4d7941dbac772aae40bd8898ebcfe Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:56:25 +0530 Subject: [PATCH] refactor: single chokepoint for the pre-import version fast path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Architecture fix for the bug class behind the Termux --version NameError (live on main since eb4040242): version-printing kept being reimplemented as *_fast() copies at the top of hermes_cli/main.py, each duplicating canonical logic (project-root resolution, container detection, profile detection). The copies drift silently — eb4040242 edited the canonical output and referenced the PROJECT_ROOT module constant inside the fast function, which doesn't exist yet at the fast exit point. - hermes_cli/_startup_fast.py: THE implementations, stdlib-only. main.py's *_fast() names become thin delegates (kept for test/back-compat), and PROJECT_ROOT itself derives from the same helper — the constant and the fast path can no longer disagree. - Fast output now includes the .install_method stamp (one cheap file read) and a 'Run hermes version for update status' pointer, so globalizing the fast path doesn't silently drop slow-path info. - Guard tests: (1) import-weight — subprocess-imports _startup_fast and fails if any heavy module (config/yaml/argparse/cli/run_agent/httpx) lands in sys.modules; (2) subprocess parity on+off Termux — the test that would have caught eb4040242 the day it landed; (3) install-method stamp surfacing. hermes --version: ~3.8s cold / 0.2-0.4s warm -> 0.01-0.02s everywhere. --- hermes_cli/_startup_fast.py | 222 +++++++++++++++++++ hermes_cli/main.py | 118 ++-------- tests/hermes_cli/test_startup_fast_guards.py | 106 +++++++++ 3 files changed, 348 insertions(+), 98 deletions(-) create mode 100644 hermes_cli/_startup_fast.py create mode 100644 tests/hermes_cli/test_startup_fast_guards.py diff --git a/hermes_cli/_startup_fast.py b/hermes_cli/_startup_fast.py new file mode 100644 index 0000000000000..c1c38f364d0f0 --- /dev/null +++ b/hermes_cli/_startup_fast.py @@ -0,0 +1,222 @@ +"""Pre-import startup fast paths — THE canonical lightweight helpers. + +This module is imported by ``hermes_cli/main.py`` BEFORE its heavy import +wall (config, argparse tree, logging, providers). Everything here must stay +**stdlib-only and cheap** (os/sys file probes; no yaml, no hermes_cli.config, +no argparse). A guard test (``test_startup_fast_import_weight``) subprocess- +imports this module and fails if any heavy module sneaks into sys.modules. + +Why this module exists (the bug class it kills): version-printing kept being +reimplemented as ``*_fast()`` copies at the top of main.py (Termux first, +then globally), each duplicating canonical logic — project-root resolution, +container detection, profile detection. The copies drifted: eb4040242 +changed the canonical output and referenced ``PROJECT_ROOT`` inside the fast +function, which doesn't exist yet on the fast path → the Termux fast path +NameError'd on --version and nobody noticed. One implementation, imported +by both the fast path and the module constants, makes that drift +structurally impossible; the parity guard test would have caught eb4040242 +the day it landed. + +``hermes_cli/config.py``'s ``get_container_exec_info()`` reads the same +``.container-mode`` file; keep the file-format assumptions here and there in +sync (this module deliberately only PROBES existence/typos cheaply and errs +toward the slow path, which then does the authoritative parse). +""" + +from __future__ import annotations + +import os +import sys + +__all__ = [ + "project_root_str", + "ensure_project_root_on_path", + "is_termux_env", + "is_termux_fast_version_argv", + "is_global_fast_version_argv", + "is_container_startup_environment", + "active_profile_may_override_home", + "container_mode_may_be_active", + "read_openai_version", + "read_install_method", + "print_fast_version_info", + "try_fast_version", +] + + +def project_root_str() -> str: + """Repo root as a str — the single source for main.py's PROJECT_ROOT.""" + return os.path.realpath(os.path.join(os.path.dirname(__file__), os.pardir)) + + +def ensure_project_root_on_path() -> None: + """Put the project root at sys.path[0], deduping realpath-equivalents.""" + project_root = project_root_str() + normalized_root = os.path.normcase(os.path.realpath(project_root)) + sys.path[:] = [ + entry + for entry in sys.path + if not entry + or os.path.normcase(os.path.realpath(entry)) != normalized_root + ] + sys.path.insert(0, project_root) + + +def is_termux_env() -> bool: + """Tiny Termux check for pre-import startup shortcuts.""" + prefix = os.environ.get("PREFIX", "") + return bool( + os.environ.get("TERMUX_VERSION") + or "com.termux/files/usr" in prefix + or prefix.startswith("/data/data/com.termux/") + ) + + +def is_termux_fast_version_argv(argv: list[str]) -> bool: + return argv in (["--version"], ["-V"], ["version"]) + + +def is_global_fast_version_argv(argv: list[str]) -> bool: + return argv in (["--version"], ["-V"]) + + +def is_container_startup_environment() -> bool: + """True when we're already INSIDE a container (fast path is then safe).""" + if os.path.exists("/.dockerenv") or os.path.exists("/run/.containerenv"): + return True + try: + with open("/proc/1/cgroup", encoding="utf-8") as handle: + cgroup = handle.read() + except OSError: + return False + return "docker" in cgroup or "podman" in cgroup or "/lxc/" in cgroup + + +def active_profile_may_override_home(hermes_root: str) -> bool: + """Cheap probe: does an active non-default profile redirect HERMES_HOME?""" + active_profile = os.path.join(hermes_root, "active_profile") + try: + if os.path.exists(active_profile): + with open(active_profile, encoding="utf-8") as handle: + active = handle.read().strip() + return bool(active and active != "default") + except (OSError, UnicodeDecodeError): + pass + return False + + +def _resolved_home() -> str: + hermes_home = os.environ.get("HERMES_HOME", "").strip() + if hermes_home: + return hermes_home + return os.path.join(os.path.expanduser("~"), ".hermes") + + +def container_mode_may_be_active() -> bool: + """Conservative probe for NixOS container-mode routing. + + False positives are fine (we fall through to the slow path, whose + ``get_container_exec_info()`` does the authoritative check and routes + into the container). False negatives are NOT fine — they'd print the + host's version instead of the container's. Hence: any profile + ambiguity → assume container mode may be active. + """ + if os.environ.get("HERMES_DEV") == "1": + return False + if is_container_startup_environment(): + return False + + hermes_home = os.environ.get("HERMES_HOME", "").strip() + if hermes_home: + if os.path.exists(os.path.join(hermes_home, ".container-mode")): + return True + parent_name = os.path.basename(os.path.dirname(os.path.normpath(hermes_home))) + return ( + parent_name != "profiles" + and active_profile_may_override_home(hermes_home) + ) + + default_home = os.path.join(os.path.expanduser("~"), ".hermes") + if active_profile_may_override_home(default_home): + return True + return os.path.exists(os.path.join(default_home, ".container-mode")) + + +def read_openai_version() -> str | None: + """Read OpenAI SDK version without importing ``importlib.metadata``.""" + for base in sys.path: + if not base: + base = os.getcwd() + version_file = os.path.join(base, "openai", "_version.py") + try: + with open(version_file, encoding="utf-8") as handle: + for line in handle: + stripped = line.strip() + if not stripped.startswith("__version__"): + continue + _key, _sep, value = stripped.partition("=") + value = value.split("#", 1)[0].strip().strip("\"'") + return value or None + except OSError: + continue + return None + + +def read_install_method() -> str | None: + """Read the installer's ``.install_method`` stamp, if present. + + Only the stamp (step 1 of ``config.detect_install_method``'s resolution + order) — the managed/git/pip fallbacks need heavier imports and stay on + the slow path. On the fast path home ambiguity is already excluded: + ``container_mode_may_be_active()`` bails to the slow path whenever a + non-default profile might redirect HERMES_HOME. + """ + stamp = os.path.join(_resolved_home(), ".install_method") + try: + with open(stamp, encoding="utf-8") as handle: + method = handle.read().strip().lower() + return method or None + except OSError: + return None + + +def print_fast_version_info() -> None: + from hermes_cli import __release_date__, __version__ + + print(f"Hermes Agent v{__version__} ({__release_date__})") + print(f"Install directory: {project_root_str()}") + install_method = read_install_method() + if install_method: + print(f"Install method: {install_method}") + + print(f"Python: {sys.version.split()[0]}") + + openai_version = read_openai_version() + print(f"OpenAI SDK: {openai_version}" if openai_version else "OpenAI SDK: Not installed") + print("Run 'hermes version' for update status.") + + +def try_fast_version(argv: list[str] | None = None) -> bool: + """Handle ``hermes --version`` before the heavy import wall. + + Termux keeps its historical contract (also accepts the ``version`` + subcommand + the HERMES_TERMUX_DISABLE_FAST_CLI escape hatch). Everywhere + else: only ``--version``/``-V`` (the ``version`` subcommand stays on the + slow path for full output incl. update check), and never when container + mode may need to route the command into the container. + """ + if argv is None: + argv = sys.argv[1:] + is_termux = is_termux_env() + if is_termux and os.environ.get("HERMES_TERMUX_DISABLE_FAST_CLI") == "1": + return False + if is_termux: + if not is_termux_fast_version_argv(argv): + return False + elif not is_global_fast_version_argv(argv): + return False + elif container_mode_may_be_active(): + return False + + print_fast_version_info() + return True diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 533767d7c5dd6..fc30c53cd6ccf 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -73,6 +73,15 @@ suppress_platform_ver_console() import os import sys +# ── Startup fast-path bootstrap ───────────────────────────────────────── +# Two lines of inline path math so ``python hermes_cli/main.py`` (script +# mode — sys.path[0] is hermes_cli/, not the repo root) can import the +# canonical helpers; everything else lives in hermes_cli._startup_fast. +_bootstrap_root = os.path.realpath(os.path.join(os.path.dirname(__file__), os.pardir)) +if _bootstrap_root not in sys.path: + sys.path.insert(0, _bootstrap_root) +from hermes_cli import _startup_fast # noqa: E402 + # Early venv self-heal — MUST run before any third-party import below. When # a prior ``hermes update`` left a recovery marker and a core package's import # files were wiped (#57828 — failed lazy backend refresh), the module-level @@ -213,19 +222,11 @@ def _run_and_exit_oneshot( def _project_root_str_fast() -> str: - return os.path.realpath(os.path.join(os.path.dirname(__file__), os.pardir)) + return _startup_fast.project_root_str() def _ensure_project_root_on_path_fast() -> None: - project_root = _project_root_str_fast() - normalized_root = os.path.normcase(os.path.realpath(project_root)) - sys.path[:] = [ - entry - for entry in sys.path - if not entry - or os.path.normcase(os.path.realpath(entry)) != normalized_root - ] - sys.path.insert(0, project_root) + _startup_fast.ensure_project_root_on_path() def _set_process_title() -> None: @@ -372,120 +373,41 @@ _suppress_mouse_residue_early() def _is_termux_startup_environment_fast() -> bool: """Tiny Termux check for pre-import startup shortcuts.""" - prefix = os.environ.get("PREFIX", "") - return bool( - os.environ.get("TERMUX_VERSION") - or "com.termux/files/usr" in prefix - or prefix.startswith("/data/data/com.termux/") - ) + return _startup_fast.is_termux_env() def _is_termux_fast_version_argv(argv: list[str]) -> bool: - return argv in (["--version"], ["-V"], ["version"]) + return _startup_fast.is_termux_fast_version_argv(argv) def _is_global_fast_version_argv(argv: list[str]) -> bool: - return argv in (["--version"], ["-V"]) + return _startup_fast.is_global_fast_version_argv(argv) def _is_container_startup_environment_fast() -> bool: - if os.path.exists("/.dockerenv") or os.path.exists("/run/.containerenv"): - return True - try: - with open("/proc/1/cgroup", encoding="utf-8") as handle: - cgroup = handle.read() - except OSError: - return False - return "docker" in cgroup or "podman" in cgroup or "/lxc/" in cgroup + return _startup_fast.is_container_startup_environment() def _active_profile_may_override_home_fast(hermes_root: str) -> bool: - active_profile = os.path.join(hermes_root, "active_profile") - try: - if os.path.exists(active_profile): - with open(active_profile, encoding="utf-8") as handle: - active = handle.read().strip() - return bool(active and active != "default") - except (OSError, UnicodeDecodeError): - pass - return False + return _startup_fast.active_profile_may_override_home(hermes_root) def _container_mode_may_be_active_fast() -> bool: - if os.environ.get("HERMES_DEV") == "1": - return False - if _is_container_startup_environment_fast(): - return False - - hermes_home = os.environ.get("HERMES_HOME", "").strip() - if hermes_home: - if os.path.exists(os.path.join(hermes_home, ".container-mode")): - return True - parent_name = os.path.basename(os.path.dirname(os.path.normpath(hermes_home))) - return ( - parent_name != "profiles" - and _active_profile_may_override_home_fast(hermes_home) - ) - - default_home = os.path.join(os.path.expanduser("~"), ".hermes") - if _active_profile_may_override_home_fast(default_home): - return True - return os.path.exists(os.path.join(default_home, ".container-mode")) + return _startup_fast.container_mode_may_be_active() def _read_openai_version_fast() -> str | None: """Read OpenAI SDK version without importing ``importlib.metadata``.""" - for base in sys.path: - if not base: - base = os.getcwd() - version_file = os.path.join(base, "openai", "_version.py") - try: - with open(version_file, encoding="utf-8") as handle: - for line in handle: - stripped = line.strip() - if not stripped.startswith("__version__"): - continue - _key, _sep, value = stripped.partition("=") - value = value.split("#", 1)[0].strip().strip("\"'") - return value or None - except OSError: - continue - return None + return _startup_fast.read_openai_version() def _print_fast_version_info() -> None: - from hermes_cli import __release_date__, __version__ - - print(f"Hermes Agent v{__version__} ({__release_date__})") - # PROJECT_ROOT (module constant) is defined AFTER the ultrafast exit — - # referencing it here NameErrors (live bug on main since eb4040242 broke - # the Termux fast path). Compute it locally. - print(f"Install directory: {_project_root_str_fast()}") - - print(f"Python: {sys.version.split()[0]}") - - openai_version = _read_openai_version_fast() - print(f"OpenAI SDK: {openai_version}" if openai_version else "OpenAI SDK: Not installed") + _startup_fast.print_fast_version_info() def _try_ultrafast_version() -> bool: """Handle ``hermes --version`` before config/logging imports.""" - is_termux = _is_termux_startup_environment_fast() - if ( - is_termux - and os.environ.get("HERMES_TERMUX_DISABLE_FAST_CLI") == "1" - ): - return False - if is_termux: - if not _is_termux_fast_version_argv(sys.argv[1:]): - return False - elif not _is_global_fast_version_argv(sys.argv[1:]): - return False - elif _container_mode_may_be_active_fast(): - return False - - _print_fast_version_info() - return True + return _startup_fast.try_fast_version() def _try_termux_ultrafast_version() -> bool: diff --git a/tests/hermes_cli/test_startup_fast_guards.py b/tests/hermes_cli/test_startup_fast_guards.py new file mode 100644 index 0000000000000..4628b8fabd8ba --- /dev/null +++ b/tests/hermes_cli/test_startup_fast_guards.py @@ -0,0 +1,106 @@ +"""Guards for hermes_cli._startup_fast — the pre-import version fast path. + +Two invariants, each of which has been broken before: + +1. IMPORT WEIGHT: _startup_fast must stay stdlib-only. The whole point of + the module is to run before main.py's heavy import wall; one careless + ``from hermes_cli.config import ...`` silently makes `hermes --version` + slow again for everyone (the regression would be invisible — everything + still works, just 40x slower). + +2. OUTPUT PARITY / LIVENESS: the fast path must actually produce version + output and exit 0 in a real subprocess, on and off Termux. This is the + test that would have caught eb4040242, which changed the canonical + version output to reference the PROJECT_ROOT module constant inside the + fast function — a name that doesn't exist yet at the fast exit point — + NameError-ing the Termux fast path in production for weeks. +""" + +import json +import os +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] + +# Modules that must NEVER be imported by the fast path. Each one either +# pulls yaml/argparse/logging config or is itself a god-module. +_FORBIDDEN_MODULES = ( + "hermes_cli.config", + "hermes_cli.main", + "yaml", + "argparse", + "cli", + "run_agent", + "model_tools", + "httpx", + "openai", +) + + +def test_startup_fast_import_weight(): + """Importing _startup_fast must not drag in any heavy module.""" + probe = ( + "import sys, json\n" + "import hermes_cli._startup_fast\n" + "print(json.dumps(sorted(sys.modules.keys())))\n" + ) + result = subprocess.run( + [sys.executable, "-c", probe], + capture_output=True, + text=True, + timeout=30, + cwd=REPO_ROOT, + ) + assert result.returncode == 0, result.stderr + loaded = set(json.loads(result.stdout)) + offenders = [m for m in _FORBIDDEN_MODULES if m in loaded] + assert not offenders, ( + f"hermes_cli._startup_fast imported heavy modules: {offenders} — " + "the fast path must stay stdlib-only (see module docstring)." + ) + + +def _run_version(env_overrides: dict) -> subprocess.CompletedProcess: + env = {**os.environ, **env_overrides} + env.pop("HERMES_DEV", None) + return subprocess.run( + [sys.executable, "-m", "hermes_cli.main", "--version"], + capture_output=True, + text=True, + timeout=60, + cwd=REPO_ROOT, + env=env, + ) + + +def test_fast_version_parity_off_termux(tmp_path): + home = tmp_path / ".hermes" + home.mkdir() + result = _run_version({"HERMES_HOME": str(home), "TERMUX_VERSION": ""}) + assert result.returncode == 0, result.stderr + out = result.stdout + for field in ("Hermes Agent v", "Install directory:", "Python:", "OpenAI SDK:"): + assert field in out, f"fast --version output missing {field!r}:\n{out}" + + +def test_fast_version_parity_on_termux(tmp_path): + """The historical Termux path — the one eb4040242 broke.""" + home = tmp_path / ".hermes" + home.mkdir() + result = _run_version( + {"HERMES_HOME": str(home), "TERMUX_VERSION": "0.118"} + ) + assert result.returncode == 0, result.stderr + assert "Hermes Agent v" in result.stdout + assert "Traceback" not in result.stderr + + +def test_fast_version_reports_install_method_stamp(tmp_path): + home = tmp_path / ".hermes" + home.mkdir() + (home / ".install_method").write_text("git\n", encoding="utf-8") + result = _run_version({"HERMES_HOME": str(home), "TERMUX_VERSION": ""}) + assert result.returncode == 0, result.stderr + assert "Install method: git" in result.stdout