Merge pull request #122161 from NousResearch/fix/never-activate-legacy-venv

PM never activates the pre-PM in-tree venv (gateway lost pydantic_core after update)

hermes_cli/venv_sync.py

@@ -252,13 +252,22 @@ def prepare_launch(project_root: Path, argv: list[str]) -> Path | None:
         lock = UpdateLock()
         if not lock.acquire():
             raise RuntimeError("an update is still running; wait for it to exit, then relaunch Hermes")
-        # The tail imports the application, whose entry point runs this very function:
-        # under the launching process's own claim (its pid is our ancestor) we ARE that
-        # tail and owe nothing — without this, a pending marker recurses forever.
-        if not lock.acquired and read_live_update() is not None:
-            return None
         try:
-            _finish_source_update(root, current=current, pending=pending)
+            # The tail imports the application, whose entry point runs this very function:
+            # under the launching process's own claim (its pid is our ancestor) we ARE that
+            # tail and owe nothing — without this, a pending marker recurses forever.
+            if not lock.acquired and read_live_update() is not None:
+                if current:
+                    return None
+                # A process the update spawns before its dependencies are current (a restarted
+                # gateway) would boot on a tree built for another interpreter. Sync — never the
+                # tail, which is the updater's — then relaunch below into a current install.
+                _sync_source_dependencies(root, arm=False)
+                if not pm.venv_is_current(project_root=root):
+                    # Relaunching would land back here and sync again, forever.
+                    raise RuntimeError("dependency sync left this install out of date")
+            else:
+                _finish_source_update(root, current=current, pending=pending)
         finally:
             lock.release()
     python = resolve_store_python(root)
@@ -273,9 +282,8 @@ def prepare_launch(project_root: Path, argv: list[str]) -> Path | None:
 def _finish_source_update(root: Path, *, current: bool, pending: Path) -> None:
     """Sync dependencies when they are stale, then run the tail the marker still owes."""
     import sys
-    import pm
     from hermes_cli._early_recovery import _marker_owner_is_live
-    from pm.environments import activation_environment, runtime_facts_path
+    from pm.environments import activation_environment
 
     if not current:
         # Existing markers guard liveness, never create the completion obligation.
@@ -284,24 +292,7 @@ def _finish_source_update(root: Path, *, current: bool, pending: Path) -> None:
         if any(_marker_owner_is_live(marker) for marker in legacy_markers):
             raise RuntimeError("an update is still running; wait for it to exit, then relaunch Hermes")
         print("hermes: completing source-update dependencies...", file=sys.stderr, flush=True)
-        # Owed from before the sync commits: a crash between the commit and the
-        # tail must leave the tail, not a "current" install with nothing built.
-        refuse_foreign_owned_venv(root)
-        arm_completion(root)
-        # Main-era installs have no PM ledger; carry what their venv held.
-        # Established PM installs retain their recorded extras and plugin union instead.
-        from pm.client import ensure_tools_for_sync
-        from pm.extras import legacy_selection
-        extras = legacy_selection(root) if not runtime_facts_path(root).is_file() else None
-        # Same order as `hermes update`: an interrupted update or a hand-run
-        # `git pull` leaves this tree's lockfile ahead of the installed tools.
-        ensure_tools_for_sync()
-        pm.sync_venv(extras, explicit=True, project_root=root)
-        collect_superseded_generations(root)
-        # These can predate the swap. Once PM commits the replacement they
-        # must not make early recovery immediately rebuild it a second time.
-        for name in (".update-incomplete", ".lazy-refresh-incomplete"):
-            (root / name).unlink(missing_ok=True)
+        _sync_source_dependencies(root, arm=True)
     else:
         print("hermes: finishing an interrupted source update...", file=sys.stderr, flush=True)
     # Sync commits the dependency generation, but a source update also owes
@@ -330,6 +321,35 @@ def _finish_source_update(root: Path, *, current: bool, pending: Path) -> None:
     clear_completion(root)
 
 
+def _sync_source_dependencies(root: Path, *, arm: bool) -> None:
+    """Commit the tree's dependency generation; *arm* also owes the tail afterwards."""
+    import sys
+    import pm
+    from pm.client import ensure_tools_for_sync
+    from pm.environments import runtime_facts_path
+    from pm.extras import legacy_selection
+
+    if not arm:
+        print("hermes: preparing dependencies for this update...", file=sys.stderr, flush=True)
+    refuse_foreign_owned_venv(root)
+    if arm:
+        # Owed from before the sync commits: a crash between the commit and the
+        # tail must leave the tail, not a "current" install with nothing built.
+        arm_completion(root)
+    # Main-era installs have no PM ledger; carry what their venv held.
+    # Established PM installs retain their recorded extras and plugin union instead.
+    extras = legacy_selection(root) if not runtime_facts_path(root).is_file() else None
+    # Same order as `hermes update`: an interrupted update or a hand-run
+    # `git pull` leaves this tree's lockfile ahead of the installed tools.
+    ensure_tools_for_sync()
+    pm.sync_venv(extras, explicit=True, project_root=root)
+    collect_superseded_generations(root)
+    # These can predate the swap. Once PM commits the replacement they
+    # must not make early recovery immediately rebuild it a second time.
+    for name in (".update-incomplete", ".lazy-refresh-incomplete"):
+        (root / name).unlink(missing_ok=True)
+
+
 def relaunch_command(
     python: Path, root: Path, argv: list[str], original: list[str], module: str | None,
 ) -> list[str]:

pm/environments.py

@@ -80,7 +80,8 @@ def record_activation_inputs(stamps: Path, mtimes: dict[str, int], project_root:
         os.utime(stamp, ns=(mtime, mtime))
 
 
-def base_venv(project_root: Path) -> Path:
+def payload_venv(project_root: Path) -> Path | None:
+    """The environment a sealed payload ships beside its tree, or ``None``."""
     root = Path(project_root).resolve()
     manifest_path = root.parent / "manifest.json"
     if manifest_path.is_file():
@@ -90,7 +91,11 @@ def base_venv(project_root: Path) -> Path:
             if not venv.is_relative_to(root.parent):
                 raise RuntimeError("payload environment escapes its root")
             return venv
-    return project_venv_dir(root) or root / "venv"
+    return None
+
+
+def base_venv(project_root: Path) -> Path:
+    return payload_venv(project_root) or project_venv_dir(Path(project_root).resolve()) or Path(project_root).resolve() / "venv"
 
 
 def store_root(project_root: Path) -> Path:
@@ -152,11 +157,25 @@ def selected_venv(project_root: Path) -> Path:
     ``flush_before_selecting``, so the ``pyvenv.cfg`` probe below is a sanity
     check against a vanished tree, not the durability guarantee.
     """
+    return _recorded_venv(project_root) or base_venv(project_root)
+
+
+def committed_venv(project_root: Path) -> Path | None:
+    """The environment PM committed for this install (or a sealed payload's own), else ``None``.
+
+    Unlike ``selected_venv`` this never answers with the in-tree ``venv``/``.venv``: that tree
+    predates PM and is built for whichever interpreter created it, so loading it from PM's store
+    Python mixes ABIs (compiled modules vanish) and PM deletes it once a generation is committed.
+    """
+    return _recorded_venv(project_root) or payload_venv(project_root)
+
+
+def _recorded_venv(project_root: Path) -> Path | None:
     path = runtime_facts_path(project_root)
     try:
         data = json.loads(path.read_text(encoding="utf-8-sig"))
     except FileNotFoundError:
-        return base_venv(project_root)
+        return None
     except (OSError, ValueError) as exc:
         raise RuntimeError(f"cannot read dependency environment: {path}") from exc
     try:
@@ -165,7 +184,7 @@ def selected_venv(project_root: Path) -> Path:
     except AttributeError as exc:
         raise RuntimeError(f"invalid dependency environment record: {path}") from exc
     if value is None:
-        return base_venv(project_root)
+        return None
     if not isinstance(value, str):
         raise RuntimeError("invalid dependency environment path")
     environment = Path(value).resolve()
@@ -258,6 +277,20 @@ def running_from_selected_environment(project_root: Path) -> bool:
     return any(Path(entry).resolve() == selected for entry in sys.path if entry)
 
 
+def _require_own_dependencies(project_root: Path) -> None:
+    """With nothing committed, an interpreter keeps the packages it booted with.
+
+    PM's store Python boots with none, so for it there is nothing to keep: refuse instead of
+    running on whatever PYTHONPATH it inherited (historically the pre-PM in-tree venv).
+    """
+    import sys
+
+    if sys.prefix != sys.base_prefix:
+        return  # a venv interpreter (developer .venv, test env) carries its own packages
+    if Path(sys.base_prefix).resolve().is_relative_to(store_root(project_root).resolve()):
+        raise RuntimeError("no dependency environment is committed for this install")
+
+
 def activate_dependencies(project_root: Path) -> None:
     """Select the committed tree at process boot, before third-party imports.
 
@@ -275,20 +308,24 @@ def activate_dependencies(project_root: Path) -> None:
         with runtime_lock(project_root) as held:
             if held:
                 recover_publication(project_root)
-            environment = selected_venv(project_root)
+            environment = committed_venv(project_root)
+            if environment is None:
+                return _require_own_dependencies(project_root)
             release = lease_generation(environment)
             # Without the lock, an installer may commit a new generation between the
             # read and the lease, leaving the leased one unselected and collectable.
-            while not held and (current := selected_venv(project_root)) != environment:
+            while not held and (current := committed_venv(project_root)) not in (None, environment):
                 release()
                 environment, release = current, lease_generation(current)
             selected = site_packages(environment)
             if not selected.is_dir() and not runtime_facts_path(project_root).is_file():
                 return
     else:
-        # Older installs and sealed payloads still select once, before imports.
+        # Sealed payloads still select once, before imports.
         # Never consult VIRTUAL_ENV: it can describe the invoking shell's Python.
-        environment = base_venv(project_root)
+        environment = payload_venv(project_root)
+        if environment is None:
+            return _require_own_dependencies(project_root)
         selected = site_packages(environment)
         if not selected.is_dir():
             return  # External/Nix interpreter owns its original sys.path.
@@ -316,10 +353,13 @@ def activation_environment(project_root: Path) -> dict[str, str]:
     from pm.registry import all_packages
 
     env = env_for(*all_packages())
-    selected = site_packages(selected_venv(project_root))
+    environment = committed_venv(project_root)
     env.pop("PYTHONHOME", None)
     env.pop("VIRTUAL_ENV", None)
-    env["PYTHONPATH"] = os.pathsep.join([str(project_root.resolve()), str(selected)])
+    # Nothing committed: the child's own hermes_bootstrap decides (a bare store Python refuses),
+    # rather than inheriting the pre-PM in-tree venv from here.
+    env["PYTHONPATH"] = os.pathsep.join([str(project_root.resolve()),
+                                         *([str(site_packages(environment))] if environment else [])])
     # The child-process sentinel. Its VALUE is the installed-state file this
     # environment was composed against, so a consumer learns that it inherited
     # an activated shell and which checkout/profile that shell came from. Its

tests/pm/test_runtime_selection.py

@@ -110,6 +110,49 @@ def test_manual_repair_bypasses_damaged_generation_activation(tmp_path, monkeypa
     assert "hermes pm repair" in result.stdout
 
 
+@pytest.mark.parametrize("interpreter", ["store", "venv"])
+@pytest.mark.parametrize("with_state", [True, False])
+def test_boot_never_activates_the_pre_pm_venv(tmp_path, monkeypatch, interpreter, with_state):
+    """Nothing committed must not mean "load the in-tree venv": it was built for another
+    interpreter, so PM's store Python lost every compiled module from it after an update."""
+    import os
+    import subprocess
+    import sys
+    from pm import environments as runtime_paths
+
+    base_python = getattr(sys, "_base_executable", sys.executable)
+    base_prefix = Path(sys.base_prefix).resolve()
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home"))
+    # The store interpreter is PM's: a non-venv Python living under the runtime dir.
+    monkeypatch.setenv("HERMES_RUNTIME_DIR", str(base_prefix.parent))
+    root = tmp_path / "repo"
+    legacy = root / "venv"
+    (legacy / "pyvenv.cfg").parent.mkdir(parents=True)
+    (legacy / "pyvenv.cfg").write_text("home = test\n")
+    runtime_paths.site_packages(legacy).mkdir(parents=True)
+    (runtime_paths.site_packages(legacy) / "legacy_only.py").write_text("")
+    if with_state:
+        runtime_paths.install_state_dir(root).mkdir(parents=True)
+    python = base_python
+    if interpreter == "venv":
+        subprocess.run([base_python, "-m", "venv", "--without-pip", str(tmp_path / "dev")], check=True, timeout=60)
+        python = str(runtime_paths.venv_python(tmp_path / "dev"))
+    repo = Path(__file__).resolve().parents[2]
+    code = (
+        "import sys, importlib.util; from pathlib import Path; sys.path.insert(0, sys.argv[1]); "
+        "from pm.environments import activate_dependencies\n"
+        "try:\n    activate_dependencies(Path(sys.argv[2]))\n"
+        "except RuntimeError as exc:\n    print('refused:', exc); raise SystemExit(0)\n"
+        "print('legacy importable:', importlib.util.find_spec('legacy_only') is not None)"
+    )
+    result = subprocess.run([python, "-I", "-c", code, str(repo), str(root)], env=dict(os.environ),
+                            capture_output=True, text=True, timeout=30)
+    assert result.returncode == 0, result.stderr
+    expected = ("refused: no dependency environment is committed" if interpreter == "store"
+                else "legacy importable: False")
+    assert result.stdout.strip().startswith(expected), result.stdout
+
+
 @pytest.mark.parametrize("data", [[], {"packages": []}, {"packages": {"venv": []}}])
 def test_malformed_selection_has_actionable_error(tmp_path, monkeypatch, data):
     from pm import environments as runtime_paths

tests/pm/test_source_update_launch.py

@@ -268,6 +268,31 @@ def test_launch_without_marker_publishes_then_skips_and_rebuilds_on_lock_change(
     assert not (root / ".update-incomplete").exists()
 
 
+@pytest.mark.platforms("posix")
+def test_process_spawned_by_the_update_commits_dependencies_but_not_the_tail(source_launch, tmp_path):
+    """A process an update spawns before its dependencies are current (its restarted gateway)
+    must not boot on a tree built for another interpreter; it syncs, but leaves the tail alone."""
+    import time
+    from hermes_cli.update_lock import update_marker_path
+    from pm.environments import committed_venv
+
+    root, store_python, _ = source_launch
+    marker = update_marker_path()
+    marker.parent.mkdir(parents=True, exist_ok=True)
+    marker.write_text(f"{os.getppid()}\n{int(time.time())}\n", encoding="utf-8")  # the updater is our ancestor
+    assert committed_venv(root) is None
+
+    assert venv_sync.prepare_launch(root, []) == store_python
+    assert committed_venv(root) == Path(_fact(root)["environment"])
+    assert not (tmp_path / "completion-calls").exists(), "the tail is the updater's, not its child's"
+    assert not venv_sync.completion_pending_path(root).exists()
+
+    facts_bytes, receipts = runtime_facts_path(root).read_bytes(), _receipts(tmp_path)
+    venv_sync.prepare_launch(root, [])
+    assert runtime_facts_path(root).read_bytes() == facts_bytes
+    assert _receipts(tmp_path) == receipts, "a committed child synced again under the updater's claim"
+
+
 @pytest.mark.platforms("posix")
 def test_failed_real_sync_preserves_previous_selection_and_retries(source_launch, tmp_path):
     root, store_python, _ = source_launch