From 3eaa3a234f36d976c14a320c6b0a2320a8bd5144 Mon Sep 17 00:00:00 2001 From: fcavalcantirj Date: Sun, 26 Jul 2026 10:56:36 -0300 Subject: [PATCH] =?UTF-8?q?fix(update):=20test=20runs=20never=20mutate=20t?= =?UTF-8?q?he=20live=20checkout=20=E2=80=94=20pytest-guard=20the=20marker?= =?UTF-8?q?=20and=20repair=20paths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unsandboxed tests driving cmd_update/recovery wrote .lazy-refresh-incomplete / .update-incomplete at the real repo root (false-arming recovery on the developer's next launch), and on a venv with genuinely broken packages, test-spawned subprocesses importing hermes_cli.main ran a real ensurepip + pip install --force-reinstall against the executing environment mid-suite. Guard predicate requires BOTH under-pytest AND target==this checkout, so tmp_path-sandboxed tests keep exercising the real paths unchanged (posture of managed_scope._under_pytest). Co-Authored-By: Claude Fable 5 --- hermes_cli/_early_recovery.py | 18 +++ hermes_cli/main.py | 18 +++ hermes_cli/update_cmd.py | 3 + .../test_checkout_mutation_guards.py | 108 ++++++++++++++++++ 4 files changed, 147 insertions(+) create mode 100644 tests/hermes_cli/test_checkout_mutation_guards.py diff --git a/hermes_cli/_early_recovery.py b/hermes_cli/_early_recovery.py index 0a19e58498b7..7a19167eca04 100644 --- a/hermes_cli/_early_recovery.py +++ b/hermes_cli/_early_recovery.py @@ -171,6 +171,22 @@ def _run_repair_install(specs: list[str], project_root: Path) -> bool: return True +def _pytest_owns_live_checkout(root: Path) -> bool: + """True when running under pytest AND ``root`` is this module's own + checkout — the one whose venv is executing the suite right now. + + Lifecycle tests spawn real subprocesses that import ``hermes_cli.main`` + with recovery armed; ``PYTEST_CURRENT_TEST`` rides the inherited env into + those children. Without this guard, a genuinely-broken dev venv gets a + REAL ``ensurepip`` + ``pip install --force-reinstall`` from inside a + running test suite. Tests that sandbox ``project_root`` to a tmp_path are + unaffected (same posture as ``managed_scope._under_pytest``).""" + return ( + "PYTEST_CURRENT_TEST" in os.environ + and root == Path(__file__).resolve().parent.parent + ) + + def recover_if_needed( project_root: Path | None = None, argv: list[str] | None = None, @@ -193,6 +209,8 @@ def recover_if_needed( if "update" in args: return root = _project_root() if project_root is None else project_root + if _pytest_owns_live_checkout(root): + return core_marker = root / ".update-incomplete" lazy_marker = root / ".lazy-refresh-incomplete" if not core_marker.exists() and not lazy_marker.exists(): diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 30c3104316e6..42a2df9f5284 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -7537,6 +7537,22 @@ def _lazy_refresh_marker_path() -> Path: return PROJECT_ROOT / ".lazy-refresh-incomplete" +def _pytest_owns_live_checkout(root: Path) -> bool: + """True when running under pytest AND ``root`` is this checkout itself. + + Tests that drive update/recovery without sandboxing ``PROJECT_ROOT`` + must neither litter the live repo root with recovery breadcrumbs + (a leftover ``.lazy-refresh-incomplete`` / ``.update-incomplete`` + false-arms recovery on the developer's next real launch) nor run a real + reinstall against the executing venv. Sandboxed tests point at a + tmp_path and are unaffected (same posture as + ``managed_scope._under_pytest``).""" + return ( + "PYTEST_CURRENT_TEST" in os.environ + and root == Path(__file__).resolve().parent.parent + ) + + def _clear_marker_file(path: Path, *, label: str) -> None: """Remove an update-recovery breadcrumb. Never raises.""" try: @@ -7583,6 +7599,8 @@ def _recover_from_interrupted_install() -> None: protocol stream (``hermes acp`` speaks JSON-RPC on stdout) must never get install noise on stdout. """ + if _pytest_owns_live_checkout(PROJECT_ROOT): + return core_marker = _update_marker_path().exists() lazy_marker = _lazy_refresh_marker_path().exists() if not core_marker and not lazy_marker: diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 5e70487698f8..012221228405 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -1392,6 +1392,9 @@ def _invalidate_update_cache(): def _write_marker_file(path: Path, *, label: str) -> None: """Drop an update-recovery breadcrumb. Never raises.""" + if _m()._pytest_owns_live_checkout(path.parent): + logger.debug("Skipping %s marker under pytest (live checkout)", label) + return try: path.write_text( f"started={_time.time()}\npid={os.getpid()}\n", encoding="utf-8" diff --git a/tests/hermes_cli/test_checkout_mutation_guards.py b/tests/hermes_cli/test_checkout_mutation_guards.py new file mode 100644 index 000000000000..18fcedae1e66 --- /dev/null +++ b/tests/hermes_cli/test_checkout_mutation_guards.py @@ -0,0 +1,108 @@ +"""The test suite must never mutate the LIVE checkout or its venv. + +Regression tests for the pytest-guards on the checkout-root mutation paths. +Before the guards, tests that drove ``cmd_update``/recovery without +sandboxing ``PROJECT_ROOT`` left ``.lazy-refresh-incomplete`` at the real +repo root (observed after full-suite runs), and — on a venv with genuinely +broken packages — test-spawned subprocesses importing ``hermes_cli.main`` +ran a REAL ``ensurepip`` + ``pip install --force-reinstall`` against the +developer's executing environment mid-suite. + +The guard predicate requires BOTH conditions (under pytest AND the target is +this checkout itself), so every tmp_path-sandboxed test keeps exercising the +real code paths unchanged. +""" + +from __future__ import annotations + +from pathlib import Path + +import hermes_cli.main as main_mod +from hermes_cli import _early_recovery as er + +CHECKOUT_ROOT = Path(er.__file__).resolve().parent.parent + + +class TestPredicate: + def test_true_for_live_checkout_under_pytest(self): + # PYTEST_CURRENT_TEST is set by pytest itself right now. + assert er._pytest_owns_live_checkout(CHECKOUT_ROOT) is True + assert main_mod._pytest_owns_live_checkout(CHECKOUT_ROOT) is True + + def test_false_for_sandboxed_root(self, tmp_path): + assert er._pytest_owns_live_checkout(tmp_path) is False + assert main_mod._pytest_owns_live_checkout(tmp_path) is False + + def test_false_outside_pytest(self, monkeypatch): + monkeypatch.delenv("PYTEST_CURRENT_TEST", raising=False) + assert er._pytest_owns_live_checkout(CHECKOUT_ROOT) is False + assert main_mod._pytest_owns_live_checkout(CHECKOUT_ROOT) is False + + +class TestMarkerWrites: + def test_refuses_breadcrumb_at_live_repo_root(self): + target = CHECKOUT_ROOT / ".lazy-refresh-incomplete" + # The marker may legitimately pre-exist: upstream currently TRACKS a + # littered copy in git (the exact pollution this guard prevents), so + # the contract is content-unchanged, not never-exists. + before = target.read_text(encoding="utf-8") if target.exists() else None + try: + main_mod._write_marker_file(target, label="lazy-refresh-incomplete") + after = ( + target.read_text(encoding="utf-8") if target.exists() else None + ) + assert after == before, ( + "marker breadcrumb written into the LIVE checkout from a test" + ) + finally: + # If the guard is broken (RED state), restore the pre-test state — + # leaving pollution behind is exactly the bug being pinned. + if before is None: + target.unlink(missing_ok=True) + else: + target.write_text(before, encoding="utf-8") + + def test_still_writes_sandboxed(self, tmp_path): + target = tmp_path / ".lazy-refresh-incomplete" + main_mod._write_marker_file(target, label="lazy-refresh-incomplete") + assert target.exists() + assert "pid=" in target.read_text(encoding="utf-8") + + +class TestEarlyRecovery: + def test_skips_live_checkout_before_any_probe_or_lock(self, monkeypatch): + # A probe call would mean recovery is proceeding against the live + # checkout; the guard must return before ANY side-effectful step. + def _boom(): + raise AssertionError("probe ran against the live checkout") + + monkeypatch.setattr(er, "_probe_broken_packages", _boom) + monkeypatch.setattr(er, "_run_repair_install", lambda *a, **k: _boom()) + er.recover_if_needed(project_root=CHECKOUT_ROOT, argv=[]) + + def test_sandboxed_root_still_recovers(self, tmp_path, monkeypatch): + # The guard must not disable recovery for sandboxed roots: with a + # marker present and a broken probe, the repair path still runs. + (tmp_path / ".lazy-refresh-incomplete").write_text("started=1\npid=1\n") + (tmp_path / "pyproject.toml").write_text("[project]\nname='x'\n") + monkeypatch.setattr(er, "_probe_broken_packages", lambda: ["PyYAML"]) + monkeypatch.setattr(er, "_pinned_specs", lambda broken, root: broken) + installs = [] + monkeypatch.setattr( + er, "_run_repair_install", lambda specs, root: installs.append(specs) or True + ) + er.recover_if_needed(project_root=tmp_path, argv=[]) + assert installs, "sandboxed recovery was wrongly disabled by the guard" + + +class TestLaunchRecovery: + def test_recover_from_interrupted_install_noops_on_live_checkout( + self, monkeypatch + ): + # PROJECT_ROOT is the live checkout in-suite; the launch-time + # recovery must return before touching markers or spawning installs. + def _boom(*a, **k): + raise AssertionError("launch recovery ran against the live checkout") + + monkeypatch.setattr(main_mod, "_update_marker_path", _boom) + main_mod._recover_from_interrupted_install()