Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cron/scheduler_worker_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

logger = logging.getLogger(__name__)

SAFE_PATH_PIN_MARKER = "_HERMES_PINNED_SAFEPATH"


def _installed_purelib() -> Path | None:
try:
Expand Down Expand Up @@ -78,6 +80,14 @@ def pin_hermes_tree_on_pythonpath(worker_env: dict, repo_root: Path) -> dict:
root = str(repo_root)
if _installed_purelib() == Path(root).resolve():
return worker_env
# The child's cwd (task workspace) is first on sys.path for ``-m``; a checkout there
# would shadow the pinned tree with its own dependency-less copy.
# Process-scoped: the worker latches ``sys.flags.safe_path`` at interpreter start, then
# the worker's boot (``hermes_cli.main`` / ``cron.worker_bootstrap``) drops the var so the agent's own children (terminal /
# execute_code) keep the normal script-dir/cwd sys.path entry. A user's own value
# (no marker) is left alone.
worker_env["PYTHONSAFEPATH"] = "1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pin helper overwrites any inherited PYTHONSAFEPATH and always sets the marker, so hermes_cli.main/worker_bootstrap later remove the variable even when it was set by the user before dispatch. That contradicts the comment that a user-owned value is preserved and changes the environment passed to terminal/execute_code children. Could the helper preserve the prior value (for example in a separate marker value) and restore it after interpreter startup, with a test that starts from an env already containing PYTHONSAFEPATH?

worker_env[SAFE_PATH_PIN_MARKER] = "1"
existing = [e for e in worker_env.get("PYTHONPATH", "").split(os.pathsep) if e]
dependency = _committed_dependency_site_packages(Path(root))
pinned = [root, *([str(dependency)] if dependency is not None else [])]
Expand Down
3 changes: 3 additions & 0 deletions cron/worker_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@

def worker_bootstrap() -> None:
"""Run PM's dependency boot in the marked external worker (again after a relaunch)."""
# sys.flags.safe_path is already latched; keep PYTHONSAFEPATH out of our children.
if os.environ.pop("_HERMES_PINNED_SAFEPATH", None):
os.environ.pop("PYTHONSAFEPATH", None)
if not os.environ.get(WORKER_MARKER):
return
from pm.environments import activate_dependencies
Expand Down
5 changes: 5 additions & 0 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
import re
import sys

# A worker pinned with PYTHONSAFEPATH (cron.scheduler_worker_env) latched it at interpreter
# start; drop it so this agent's own children keep their normal script-dir/cwd sys.path.
if os.environ.pop("_HERMES_PINNED_SAFEPATH", None):
os.environ.pop("PYTHONSAFEPATH", None)

# Inline path math so ``python hermes_cli/main.py`` (script mode: sys.path[0]
# is hermes_cli/, not the repo root) can import hermes_cli._startup_fast.
_bootstrap_root = os.path.realpath(os.path.join(os.path.dirname(__file__), os.pardir))
Expand Down
49 changes: 48 additions & 1 deletion tests/cron/test_restart_safe_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -739,10 +739,57 @@ def test_pin_restores_the_committed_generation_site_packages(tmp_path):
# A runner that owns its dependencies has no committed generation: tree only.
pm.environments.runtime_facts_path(repo_root).unlink()
assert worker_env_mod.pin_hermes_tree_on_pythonpath({}, repo_root) == {
"PYTHONPATH": str(repo_root)
"PYTHONPATH": str(repo_root), "PYTHONSAFEPATH": "1",
"_HERMES_PINNED_SAFEPATH": "1",
}


def test_pinned_safe_path_does_not_leak_into_worker_children(tmp_path):
"""PYTHONSAFEPATH is for the worker interpreter only: after boot, children built from
its os.environ (terminal / execute_code) must not carry it, but a user's own value
(no pin marker) must survive."""
import subprocess
import cron.scheduler_worker_env as worker_env_mod

repo_root = Path(worker_env_mod.__file__).resolve().parent.parent
env = worker_env_mod.pin_hermes_tree_on_pythonpath(dict(os.environ), repo_root)
boot = (
"import hermes_cli.main, os, sys\n"
"from tools.environments.local import _make_run_env\n"
"print(sys.flags.safe_path, 'PYTHONSAFEPATH' in _make_run_env({}), "
"'_HERMES_PINNED_SAFEPATH' in os.environ)"
)
out = subprocess.run([sys.executable, "-c", boot], env=env, cwd=tmp_path,
capture_output=True, text=True, timeout=120).stdout.split()[-3:]
assert out == ["True", "False", "False"] # latched for the worker, gone for children

user = {**os.environ, "PYTHONSAFEPATH": "1", "PYTHONPATH": str(repo_root)}
user.pop("_HERMES_PINNED_SAFEPATH", None)
out = subprocess.run([sys.executable, "-c", boot], env=user, cwd=tmp_path,
capture_output=True, text=True, timeout=120).stdout.split()[-3:]
assert out[1] == "True" # user's own value kept


def test_pinned_worker_ignores_decoy_hermes_cli_in_cwd(tmp_path):
"""cwd holding its own ``hermes_cli`` must not shadow the pinned install."""
import subprocess

import hermes_cli
from hermes_cli.kanban_db_dispatch import _module_hermes_argv, _propagate_module_import_root

decoy = tmp_path / "hermes_cli"
decoy.mkdir()
(decoy / "__init__.py").write_text("raise SystemExit('DECOY IMPORTED')")
env = dict(os.environ)
_propagate_module_import_root(_module_hermes_argv(), env) # kanban dispatcher path
out = subprocess.run(
[sys.executable, "-c", "import hermes_cli;print(hermes_cli.__file__)"],
cwd=tmp_path, env=env, capture_output=True, text=True,
)
assert out.returncode == 0, out.stderr
assert Path(out.stdout.strip()).resolve() == Path(hermes_cli.__file__).resolve()


_BOOT_ORDER_PROBE = """
import json, os, sys
import pm.environments
Expand Down