refactor: single chokepoint for the pre-import version fast path
Architecture fix for the bug class behind the Termux --version NameError (live on main sinceeb4040242): 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 —eb4040242edited 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 caughteb4040242the day it landed; (3) install-method stamp surfacing. hermes --version: ~3.8s cold / 0.2-0.4s warm -> 0.01-0.02s everywhere.
This commit is contained in:
kshitijk4poor
kshitij
parent
d3832a24bc
commit
e4257c171a
|
|
@ -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
|
||||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
Loading…
Reference in New Issue