diff --git a/hermes_cli/doctor_platform.py b/hermes_cli/doctor_platform.py index fa26301234..48b141f030 100644 --- a/hermes_cli/doctor_platform.py +++ b/hermes_cli/doctor_platform.py @@ -18,8 +18,17 @@ from hermes_cli.doctor_report import ( from hermes_constants import is_termux as _is_termux -def _python_install_cmd() -> str: - return "python -m pip install" if _is_termux() else "uv pip install" +def _python_repair_hint() -> str: + from hermes_cli.config import detect_install_method + from hermes_cli.doctor import PROJECT_ROOT + + method = detect_install_method(PROJECT_ROOT) + if is_nix_install_method(method): + return recommended_update_command_for_method(method) + if method in ("docker", "apt"): + command = recommended_update_command_for_method(method) + return f"Run `{command}`" + (", then recreate the Hermes container" if method == "docker" else "") + return "Run `hermes pm repair`, then restart Hermes" def _system_package_install_cmd(pkg: str) -> str: @@ -198,7 +207,7 @@ def check_certificates(should_fix: bool = False, issues: "list | None" = None) - ssl.create_default_context() except Exception as e: _fail_and_issue("TLS default SSL context cannot be constructed", str(e), - "Recreate the venv or reinstall Hermes — the TLS stack is broken.", issues) + _python_repair_hint() + "; if TLS still fails, repair Python through the installation owner.", issues) return if platform_store: check_ok("TLS platform trust store configured; default SSL context available") @@ -354,10 +363,10 @@ def _staged_venv_dir() -> "Path | None": """pm's provisioned runtime venv, or None when nothing is staged. ``pm.packages.Venv().venv_dir()`` is pm's public authority for where - the runtime venv lives (sealed installs: the mutable venv in the - writable hermes root, seeded from the payload; dev installs: the repo - venv). A resolved path without a venv marker is not a provisioned - venv — pm also returns the intended location before first sync, and + the runtime venv lives, including an external selected generation or + the original source/payload environment before first sync. A resolved + path without a venv marker is not a provisioned venv — pm also returns + the intended location before first sync, and doctor must not read an empty directory as staged dependencies. """ try: @@ -392,16 +401,14 @@ def _check_python_environment(should_fix: bool, f: Finding) -> None: if src: check_info(f"SQLite source id: {(src[:48] + '…') if len(src) > 48 else src}") _report_database_journal_modes() - # Staged dependencies vs the running interpreter, reported as two - # distinct facts. When pm has provisioned the runtime venv, its - # resolved location IS the answer ("dependencies staged"); whether - # THIS process runs inside THAT venv is a separate comparison of - # resolved prefixes (sys.prefix != base_prefix alone would also be - # true for an unrelated venv). Only when nothing is staged does the - # legacy interpreter probe stand alone. + # PM launchers run base Python with the selected dependency tree on + # sys.path. Neither sys.prefix nor a stale PYTHONPATH proves activation. staged = _staged_venv_dir() if staged is not None: - running_here = Path(sys.prefix).resolve() == staged.resolve() + from hermes_cli.runtime_paths import site_packages + + selected_site = site_packages(staged).resolve() + running_here = selected_site.is_dir() and any(Path(entry).resolve() == selected_site for entry in sys.path) check_ok(f"Runtime venv staged ({staged})", "(active in this process)" if running_here else "(this process runs outside it)") else: @@ -443,7 +450,7 @@ def _check_required_packages(should_fix: bool, f: Finding) -> None: if optional: check_warn(name, "(optional, not installed)") else: - _fail_and_issue(name, "(missing)", f"Install {name}: {_python_install_cmd()} {module}", f.issues) + _fail_and_issue(name, "(missing)", f"Repair {name}: {_python_repair_hint()}", f.issues) @doctor_check() @@ -454,16 +461,36 @@ def _check_gateway_supervision(should_fix: bool, f: Finding) -> None: @doctor_check() def _check_command_installation(should_fix: bool, f: Finding) -> None: - """Venv entry point and the ~/.local/bin (or $PREFIX/bin) symlink; skipped on Windows.""" + """Check the install-owned launch contract without replacing custom commands.""" from hermes_cli.doctor import PROJECT_ROOT if sys.platform == "win32": return _section("Command Installation") - venv_bin = next((c for c in (PROJECT_ROOT / n / "bin" / "hermes" for n in ("venv", ".venv")) if c.exists()), None) - if venv_bin is None: - check_warn("Venv entry point not found", "(hermes not in venv/bin/ or .venv/bin/ — reinstall with pip install -e '.[all]')") - return f.manual_issues.append(f"Reinstall entry point: cd {PROJECT_ROOT} && source venv/bin/activate && pip install -e '.[all]'") - check_ok(f"Venv entry point exists ({venv_bin.relative_to(PROJECT_ROOT)})") + from hermes_cli.config import detect_install_method + + method = detect_install_method(PROJECT_ROOT) + if is_nix_install_method(method) or method in ("docker", "apt"): + command = shutil.which("hermes") + if command: + check_ok(f"Hermes command managed by {method} ({command})") + else: + check_warn(f"Hermes command not on PATH ({method}-managed)") + f.manual_issues.append(_python_repair_hint()) + return + from hermes_cli._launchers import resolve_store_python + from hermes_cli.runtime_paths import base_venv, selected_venv + + try: + selected = selected_venv(PROJECT_ROOT) + except (OSError, ValueError, RuntimeError) as exc: + check_fail("Cannot resolve selected dependencies", str(exc)) + return f.manual_issues.append(_python_repair_hint()) + pm_launcher = selected != base_venv(PROJECT_ROOT) or resolve_store_python(PROJECT_ROOT) is not None + venv_bin = PROJECT_ROOT / "hermes" if pm_launcher else selected / "bin" / "hermes" + if not venv_bin.is_file(): + check_warn("Hermes entry point not found", f"({venv_bin})") + return f.manual_issues.append("Repair or reinstall the Hermes launcher through the installation owner") + check_ok(f"Hermes entry point exists ({venv_bin})") # Expected command link directory (mirrors install.sh logic). prefix = os.environ.get("PREFIX", "") termux = prefix and (os.environ.get("TERMUX_VERSION") or "com.termux/files/usr" in prefix) @@ -474,9 +501,11 @@ def _check_command_installation(should_fix: bool, f: Finding) -> None: if target == expected: return check_ok(f"{display}/hermes → correct target") check_warn(f"{display}/hermes points to wrong target", f"(→ {target}, expected → {expected})") + owned_targets = {(PROJECT_ROOT / name / "bin" / "hermes").resolve() for name in ("venv", ".venv")} + if target not in owned_targets: + return f.manual_issues.append(f"Review {display}/hermes manually; its target is user-managed and was not changed") if not should_fix: return f.issues.append(f"Broken symlink at {display}/hermes — run 'hermes doctor --fix'") - link.unlink() verb = "Fixed" elif link.exists(): # regular file (wrapper script), not a symlink return check_ok(f"{display}/hermes exists (non-symlink)") @@ -486,8 +515,18 @@ def _check_command_installation(should_fix: bool, f: Finding) -> None: return f.issues.append(f"Missing {display}/hermes symlink — run 'hermes doctor --fix'") link_dir.mkdir(parents=True, exist_ok=True) verb = "Created" - link.symlink_to(venv_bin) - check_ok(f"{verb} symlink: {display}/hermes → {venv_bin}") + if pm_launcher: + from hermes_cli._launchers import stage_launcher + + if stage_launcher("hermes", PROJECT_ROOT, link_dir) is None: + check_fail("Could not publish Hermes launcher") + return f.manual_issues.append("Repair the PM store interpreter through the installation owner, then rerun 'hermes doctor --fix'") + check_ok(f"{verb} PM launcher: {display}/hermes") + else: + if link.is_symlink(): + link.unlink() + link.symlink_to(venv_bin) + check_ok(f"{verb} symlink: {display}/hermes → {venv_bin}") f.fixed += 1 if verb == "Created" and str(link_dir) not in os.environ.get("PATH", "").split(os.pathsep): check_warn(f"{display} is not on your PATH", "(add it to your shell config: export PATH=\"$HOME/.local/bin:$PATH\")") diff --git a/tests/hermes_cli/test_doctor.py b/tests/hermes_cli/test_doctor.py index 13f20561c2..e0b7fb6ccb 100644 --- a/tests/hermes_cli/test_doctor.py +++ b/tests/hermes_cli/test_doctor.py @@ -1763,11 +1763,13 @@ class TestStagedRuntimeVenv: assert "runs outside it" in out assert "active in this process" not in out - def test_staged_and_active_names_both_facts(self, monkeypatch, capsys): - staged = Path("/payload/venv") + def test_staged_and_active_names_both_facts(self, monkeypatch, capsys, tmp_path): + from hermes_cli.runtime_paths import site_packages + + staged = tmp_path / "venv" + site_packages(staged).mkdir(parents=True) monkeypatch.setattr(doctor_platform, "_staged_venv_dir", lambda: staged) - monkeypatch.setattr(doctor_platform.sys, "prefix", str(staged)) - monkeypatch.setattr(doctor_platform.sys, "base_prefix", "/usr") + monkeypatch.syspath_prepend(str(site_packages(staged))) doctor_platform._check_python_environment(False) diff --git a/tests/hermes_cli/test_doctor_command_install.py b/tests/hermes_cli/test_doctor_command_install.py index 1f3248dee3..14ef78a007 100644 --- a/tests/hermes_cli/test_doctor_command_install.py +++ b/tests/hermes_cli/test_doctor_command_install.py @@ -1,132 +1,193 @@ -"""Tests for the Command Installation check in hermes doctor.""" +"""Command diagnostics use the same selection and launch contract as setup.""" -import sys -import types +import json from argparse import Namespace from pathlib import Path +import shutil +import subprocess +import sys import pytest -import hermes_cli.doctor as doctor_mod +from hermes_cli import _launchers, doctor, doctor_platform +from hermes_cli.runtime_paths import install_state_dir, site_packages -def _setup_doctor_env(monkeypatch, tmp_path, venv_name="venv"): - """Create a minimal HERMES_HOME + PROJECT_ROOT for doctor tests.""" - home = tmp_path / ".hermes" - home.mkdir(parents=True, exist_ok=True) - (home / "config.yaml").write_text("memory: {}\n", encoding="utf-8") +def _tree(tmp_path, monkeypatch): + home = tmp_path / "data" + home.mkdir() + project = tmp_path / "source" + project.mkdir() + (project / ".install_method").write_text("git", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setenv("HERMES_RUNTIME_DIR", str(home / "tools")) + monkeypatch.delenv("PREFIX", raising=False) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setattr(doctor, "PROJECT_ROOT", project) + monkeypatch.setattr(doctor, "HERMES_HOME", home) + monkeypatch.setattr(doctor, "DOCTOR_CHECKS", ((None, doctor_platform._check_command_installation),)) + command = tmp_path / ".local" / "bin" / "hermes" + command.parent.mkdir(parents=True) + monkeypatch.setenv("PATH", str(command.parent)) + return project, home, command - project = tmp_path / "project" - project.mkdir(exist_ok=True) - # Create a fake venv entry point - venv_bin_dir = project / venv_name / "bin" - venv_bin_dir.mkdir(parents=True, exist_ok=True) - hermes_bin = venv_bin_dir / "hermes" - hermes_bin.write_text("#!/usr/bin/env python\n# entry point\n") - hermes_bin.chmod(0o755) - - monkeypatch.setattr(doctor_mod, "HERMES_HOME", home) - monkeypatch.setattr(doctor_mod, "PROJECT_ROOT", project) - monkeypatch.setattr(doctor_mod, "_DHH", str(home)) - - # Stub model_tools so doctor doesn't fail on import - fake_model_tools = types.SimpleNamespace( - check_tool_availability=lambda *a, **kw: ([], []), - TOOLSET_REQUIREMENTS={}, +def _generation(project): + selected = install_state_dir(project) / "environments" / "selected" / "venv" + site_packages(selected).mkdir(parents=True) + (selected / "pyvenv.cfg").write_text("home = fixture\n", encoding="utf-8") + (install_state_dir(project) / "facts.json").write_text( + json.dumps({"packages": {"venv": {"environment": str(selected)}}}), encoding="utf-8" ) - monkeypatch.setitem(sys.modules, "model_tools", fake_model_tools) - - # Stub auth checks - try: - from hermes_cli import auth as _auth_mod - monkeypatch.setattr(_auth_mod, "get_nous_auth_status", lambda: {}) - monkeypatch.setattr(_auth_mod, "get_codex_auth_status", lambda: {}) - except Exception: - pass - - # Stub httpx.get to avoid network calls - try: - import httpx - monkeypatch.setattr(httpx, "get", lambda *a, **kw: types.SimpleNamespace(status_code=200)) - except Exception: - pass - - return home, project, hermes_bin + return selected -def _run_doctor(fix=False): - """Run doctor and capture stdout.""" - import io - import contextlib - - buf = io.StringIO() - with contextlib.redirect_stdout(buf): - doctor_mod.run_doctor(Namespace(fix=fix)) - return buf.getvalue() +def _pm_source(project, home): + root = Path(__file__).resolve().parents[2] + for relative in ( + "hermes", "hermes_bootstrap.py", "hermes_constants.py", "hermes_cli/__init__.py", + "hermes_cli/runtime_paths.py", "hermes_cli/runtime_state.py", + "hermes_cli/_early_recovery.py", "hermes_cli/_parser.py", + ): + target = project / relative + target.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(root / relative, target) + (project / "hermes_cli/main.py").write_text( + "def main():\n import selected_probe\n print(selected_probe.VALUE)\n return 0\n", + encoding="utf-8", + ) + interpreter = home / "tools" / "python-fixture" / "bin" / "python3" + interpreter.parent.mkdir(parents=True) + interpreter.symlink_to(Path(sys._base_executable).resolve()) + (home / "tools" / "facts.json").write_text( + json.dumps({"packages": {"python": {"entry": "python-fixture"}}}), encoding="utf-8" + ) + return interpreter -class TestDoctorCommandInstallation: - """Tests for the ◆ Command Installation section.""" +@pytest.mark.platforms("posix") +def test_pm_generation_does_not_require_a_legacy_console_script(tmp_path, monkeypatch, capsys): + project, home, command = _tree(tmp_path, monkeypatch) + selected = _generation(project) + _pm_source(project, home) + assert _launchers.stage_launcher("hermes", project, command.parent) == command + + doctor.run_doctor(Namespace(fix=True)) + + out = capsys.readouterr().out + assert "All checks passed" in out + assert not (project / "venv").exists() + assert not (selected / "bin" / "hermes").exists() + assert command.is_file() and not command.is_symlink() +@pytest.mark.platforms("posix") +@pytest.mark.parametrize("prior", ["missing", "legacy"]) +def test_pm_fix_publishes_a_generation_aware_launcher(tmp_path, monkeypatch, capsys, prior): + project, home, command = _tree(tmp_path, monkeypatch) + _pm_source(project, home) + selected = _generation(project) + (site_packages(selected) / "selected_probe.py").write_text("VALUE = 'selected'\n", encoding="utf-8") + stale = project / "venv" / "bin" / "hermes" + stale.parent.mkdir(parents=True) + stale.write_text("#!/bin/sh\nexit 99\n", encoding="utf-8") + stale.chmod(0o755) + if prior == "legacy": + command.symlink_to(stale) + + doctor.run_doctor(Namespace(fix=True)) + + assert "Fixed 1 issue" in capsys.readouterr().out + assert not command.is_symlink() + result = subprocess.run([str(command)], cwd=tmp_path, capture_output=True, text=True, timeout=30) + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.strip() == "selected" + assert stale.is_file() +@pytest.mark.platforms("posix") +@pytest.mark.parametrize("kind", ["wrapper", "symlink"]) +def test_fix_preserves_user_managed_commands(tmp_path, monkeypatch, capsys, kind): + project, home, command = _tree(tmp_path, monkeypatch) + _pm_source(project, home) + _generation(project) + wrapper = tmp_path / "custom-wrapper" + body = "#!/bin/sh\nexit 42\n" + wrapper.write_text(body, encoding="utf-8") + wrapper.chmod(0o755) + if kind == "symlink": + command.symlink_to(wrapper) + else: + shutil.copy2(wrapper, command) - @pytest.mark.skipif(sys.platform == "win32", reason="Symlink check is Unix-only") - def test_fix_repairs_wrong_symlink(self, monkeypatch, tmp_path): - home, project, hermes_bin = _setup_doctor_env(monkeypatch, tmp_path) + doctor.run_doctor(Namespace(fix=True)) - # Create a symlink pointing to wrong target - cmd_link_dir = tmp_path / ".local" / "bin" - cmd_link_dir.mkdir(parents=True) - cmd_link = cmd_link_dir / "hermes" - wrong_target = tmp_path / "wrong_hermes" - wrong_target.write_text("#!/usr/bin/env python\n") - cmd_link.symlink_to(wrong_target) + out = capsys.readouterr().out + assert "Fixed 1 issue" not in out + assert command.read_text(encoding="utf-8") == body + if kind == "symlink": + assert command.is_symlink() and command.resolve() == wrapper + assert "manual" in out.lower() - monkeypatch.setattr(Path, "home", lambda: tmp_path) - out = _run_doctor(fix=True) - assert "Fixed symlink" in out +@pytest.mark.parametrize("layout", ["legacy", "generation"]) +@pytest.mark.parametrize("active", [True, False]) +def test_doctor_reports_selected_import_tree_not_interpreter_prefix(tmp_path, monkeypatch, capsys, layout, active): + import importlib + import pm.paths - # Verify the symlink now points to the correct target - assert cmd_link.is_symlink() - assert cmd_link.resolve() == hermes_bin.resolve() + project, _home, _command = _tree(tmp_path, monkeypatch) + if layout == "generation": + selected = _generation(project) + else: + selected = project / "venv" + site_packages(selected).mkdir(parents=True) + (selected / "pyvenv.cfg").write_text("home = fixture\n", encoding="utf-8") + monkeypatch.setattr(pm.paths, "repo_root", lambda: project) + monkeypatch.setattr(doctor, "DOCTOR_CHECKS", ((None, doctor_platform._check_python_environment),)) + # A venv prefix with no selected imports is not evidence of activation; + # conversely PM's base interpreter can import the selected tree directly. + monkeypatch.setattr(sys, "prefix", sys.base_prefix if active else str(selected)) + if active: + module = site_packages(selected) / "doctor_selected_probe.py" + module.write_text("VALUE = 'selected'\n", encoding="utf-8") + monkeypatch.syspath_prepend(str(site_packages(selected))) + monkeypatch.delitem(sys.modules, "doctor_selected_probe", raising=False) + loaded = importlib.import_module("doctor_selected_probe") + assert Path(loaded.__file__) == module + monkeypatch.delitem(sys.modules, "doctor_selected_probe") - @pytest.mark.skipif(sys.platform == "win32", reason="Symlink check is Unix-only") - def test_missing_venv_entry_point_shows_warn(self, monkeypatch, tmp_path): - home = tmp_path / ".hermes" - home.mkdir(parents=True, exist_ok=True) - (home / "config.yaml").write_text("memory: {}\n", encoding="utf-8") + doctor.run_doctor(Namespace(fix=False)) - project = tmp_path / "project" - project.mkdir(exist_ok=True) - # Do NOT create any venv entry point + out = capsys.readouterr().out + assert str(selected) in out + assert ("active in this process" in out) is active + assert ("runs outside it" in out) is not active - monkeypatch.setattr(doctor_mod, "HERMES_HOME", home) - monkeypatch.setattr(doctor_mod, "PROJECT_ROOT", project) - monkeypatch.setattr(doctor_mod, "_DHH", str(home)) - monkeypatch.setattr(Path, "home", lambda: tmp_path) - fake_model_tools = types.SimpleNamespace( - check_tool_availability=lambda *a, **kw: ([], []), - TOOLSET_REQUIREMENTS={}, - ) - monkeypatch.setitem(sys.modules, "model_tools", fake_model_tools) - try: - from hermes_cli import auth as _auth_mod - monkeypatch.setattr(_auth_mod, "get_nous_auth_status", lambda: {}) - monkeypatch.setattr(_auth_mod, "get_codex_auth_status", lambda: {}) - except Exception: - pass - try: - import httpx - monkeypatch.setattr(httpx, "get", lambda *a, **kw: types.SimpleNamespace(status_code=200)) - except Exception: - pass +@pytest.mark.platforms("posix") +@pytest.mark.parametrize("method, remedy", [ + ("git", "hermes pm repair"), ("nix", "Nix"), ("docker", "docker pull"), ("apt", "pkg upgrade"), +]) +def test_remedies_and_launcher_repairs_respect_install_owner(tmp_path, monkeypatch, capsys, method, remedy): + project, _home, command = _tree(tmp_path, monkeypatch) + (project / ".install_method").write_text(method, encoding="utf-8") + entry = project / "venv" / "bin" / "hermes" + entry.parent.mkdir(parents=True) + entry.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + entry.chmod(0o755) + monkeypatch.setattr(doctor_platform, "_PACKAGES", (("doctor_missing_dependency_probe", "Required dependency", False),)) + monkeypatch.setattr(doctor, "DOCTOR_CHECKS", ( + (None, doctor_platform._check_required_packages), (None, doctor_platform._check_command_installation), + )) - out = _run_doctor(fix=False) - assert "Command Installation" in out - assert "Venv entry point not found" in out + doctor.run_doctor(Namespace(fix=True)) + out = capsys.readouterr().out + assert remedy in out + assert "pip install" not in out + if method == "git": + assert command.is_symlink() and command.resolve() == entry + else: + assert not command.exists() + assert "hermes pm repair" not in out