fix(cron): POSIX cron scripts import the live checkout; broken PM selection fails the run
Follow-up to the venv-interpreter change above. - The selected venv resolves Hermes from its generation's workspace snapshot, which only a dependency change (uv.lock / extras / Python / plugins, pm/packages.py::expected_stamp) rebuilds. After a code-only update, scripts imported older Hermes code than the gateway runs. A `python -c` bootstrap now puts the live checkout right after the script's directory, in-process, so nothing is inherited by the script's children (no PYTHONPATH, #123440). - POSIX dispatch moves into _script_argv (platform branch), so _windows_cron_python_invocation is Windows-only again and the PYTHONPATH-keyed bootstrap gate goes back to its original form. - The interpreter path comes from pm.environments.venv_python. - _script_argv now runs inside _run_job_script's try. PM record reads (store manifest, facts, selection) can raise ValueError/KeyError as well as RuntimeError; before, those escaped the runner and stranded the execution row, and on POSIX the local fallback handed the script to the bare store interpreter (the #123044 symptom). A broken selection is now a failed run with the PM error, per selected_venv's contract. This also covers the same pre-existing gap on the Windows committed_venv path. - Tests: two invariant tests replace the three change-detector tests (live checkout beats a snapshot on sys.path, venv site-packages resolve, script-dir sys.path[0], no PYTHONPATH; broken selection fails the run instead of escaping).
This commit is contained in:
@@ -114,55 +114,47 @@ def _read_windows_pyvenv_cfg(venv_dir: Path) -> dict[str, str]:
|
||||
}
|
||||
|
||||
|
||||
def _posix_cron_python_invocation(python_exe: str) -> tuple[str, dict[str, str]]:
|
||||
"""POSIX managed-store installs: cron ``.py`` scripts run on the selected dependency venv's
|
||||
interpreter, not the bare store Python. The store Python only carries the repo and the
|
||||
managed site-packages on its in-process ``sys.path``; a spawned child re-resolves imports
|
||||
from scratch and dies with ``ModuleNotFoundError`` after the venv → PM runtime switch
|
||||
(#123044). The venv interpreter finds its dependency site-packages through ``pyvenv.cfg``
|
||||
(editable installs included), so no ``PYTHONPATH`` overlay is needed — and one must not be
|
||||
used: it would be inherited by every child the script spawns, where a foreign interpreter
|
||||
(another venv, system Python) would import the store's CPython-3.14 extension modules first
|
||||
and crash (#123440). Lazy installs are disabled for script children so a script importing
|
||||
``hermes_bootstrap`` off the store-record venv cannot republish launchers (#123440).
|
||||
Installs without a committed store (source checkouts, pre-PM venvs) keep the caller's
|
||||
interpreter. A broken committed selection degrades to the caller's interpreter too:
|
||||
``selected_venv`` is documented as a raising function, and this runs before
|
||||
``_run_job_script``'s ``try``, so an escaping error would crash the tick and leave the
|
||||
execution row in ``running`` forever."""
|
||||
# ``python -c`` leaves sys.path[0] = cwd; restore the script-dir entry a plain
|
||||
# ``python script.py`` gets, with the live checkout right after it.
|
||||
_POSIX_SCRIPT_BOOTSTRAP = (
|
||||
"import os, runpy, sys;"
|
||||
"repo, script = sys.argv[1], sys.argv[2];"
|
||||
"sys.argv = [script] + sys.argv[3:];"
|
||||
"sys.path[0:1] = [os.path.dirname(os.path.abspath(script)), repo];"
|
||||
"runpy.run_path(script, run_name='__main__')"
|
||||
)
|
||||
|
||||
|
||||
def _posix_cron_script_argv(script: Path) -> tuple[list[str], dict[str, str]]:
|
||||
"""POSIX managed-store installs run cron ``.py`` scripts on the selected dependency venv's
|
||||
interpreter: the store Python carries the repo and managed site-packages only on its
|
||||
in-process ``sys.path``, so a child of it cannot import either (#123044). No ``PYTHONPATH``
|
||||
overlay — every child the script spawns would inherit it, and a foreign interpreter would
|
||||
then import the store's compiled extensions (#123440). The venv resolves Hermes itself from
|
||||
its generation's workspace SNAPSHOT, which only a dependency change rebuilds, so the live
|
||||
checkout goes in front in-process via the bootstrap. Lazy installs stay off in the script's
|
||||
process tree: a script importing Hermes must not complete a source update or republish
|
||||
launchers from a cron child. Without a committed store, the caller's interpreter as before."""
|
||||
from hermes_cli._launchers import resolve_store_python
|
||||
from pm.environments import selected_venv, venv_python
|
||||
|
||||
repo = Path(__file__).resolve().parents[1]
|
||||
if resolve_store_python(repo) is None:
|
||||
return python_exe, {}
|
||||
|
||||
from pm.environments import selected_venv, venv_bin_dir
|
||||
|
||||
try:
|
||||
venv = selected_venv(repo)
|
||||
except (RuntimeError, OSError) as exc:
|
||||
# Degrade, don't crash — same contract as the Windows bootstrap's unresolvable-venv
|
||||
# fallback below (a silent fallback would make the misconfiguration undiagnosable).
|
||||
logger.warning(
|
||||
"POSIX cron script: cannot select the dependency venv (%s); running on the "
|
||||
"caller's interpreter",
|
||||
exc,
|
||||
)
|
||||
return python_exe, {}
|
||||
venv_python = venv_bin_dir(venv) / "python"
|
||||
if not venv_python.is_file():
|
||||
return python_exe, {}
|
||||
return str(venv_python), {"HERMES_DISABLE_LAZY_INSTALLS": "1"}
|
||||
if resolve_store_python(repo) is not None:
|
||||
# selected_venv (not committed_venv as on Windows): with nothing committed yet, the
|
||||
# pre-PM venv runs on its OWN interpreter here, so there is no ABI mix (#122183).
|
||||
python = venv_python(selected_venv(repo))
|
||||
if python.is_file():
|
||||
return ([str(python), "-c", _POSIX_SCRIPT_BOOTSTRAP, str(repo), str(script)],
|
||||
{"HERMES_DISABLE_LAZY_INSTALLS": "1"})
|
||||
return [sys.executable, str(script)], {}
|
||||
|
||||
|
||||
def _windows_cron_python_invocation(python_exe: str) -> tuple[str, dict[str, str]]:
|
||||
"""Hidden, output-capable Python invocation for Windows cron scripts. ``pythonw.exe`` loses
|
||||
captured output; uv venv launchers can re-exec the base console python and flash a window
|
||||
even with CREATE_NO_WINDOW, so run the base python directly with venv paths overlaid in env.
|
||||
Off-Windows callers are routed to ``_posix_cron_python_invocation`` (venv interpreter
|
||||
selection, no env overlay)."""
|
||||
even with CREATE_NO_WINDOW, so run the base python directly with venv paths overlaid in env."""
|
||||
if sys.platform != "win32":
|
||||
return _posix_cron_python_invocation(python_exe)
|
||||
return python_exe, {}
|
||||
|
||||
interpreter = _sched.Path(python_exe)
|
||||
venv_dir = interpreter.parent.parent
|
||||
@@ -364,7 +356,8 @@ def _script_argv(path: Path) -> tuple[Optional[list[str]], dict[str, str], Optio
|
||||
"""``(argv, env_overlay, error)`` for a validated script. Interpreter by extension — the
|
||||
shebang is deliberately NOT honoured (small, auditable surface): ``.sh``/``.bash`` → bash,
|
||||
else ``sys.executable`` (Windows managed/uv overlays get the ``.pth`` bootstrap; POSIX
|
||||
managed installs run on the selected venv's interpreter)."""
|
||||
managed installs run on the selected venv's interpreter).
|
||||
Selection reads PM's install records and may raise; callers run this inside their ``try``."""
|
||||
if path.suffix.lower() in {".sh", ".bash"}:
|
||||
# which() finds Git Bash on Windows; None there → clear error instead of a "[WinError 2]".
|
||||
_bash = shutil.which("bash") or ("/bin/bash" if os.path.isfile("/bin/bash") else None)
|
||||
@@ -375,13 +368,11 @@ def _script_argv(path: Path) -> tuple[Optional[list[str]], dict[str, str], Optio
|
||||
"or rewrite the script as Python (.py)."
|
||||
)
|
||||
return [_bash, str(path)], {}, None
|
||||
if sys.platform != "win32":
|
||||
argv, env_overlay = _posix_cron_script_argv(path)
|
||||
return argv, env_overlay, None
|
||||
python_exe, env_overlay = _windows_cron_python_invocation(sys.executable)
|
||||
if env_overlay.get("PYTHONPATH"):
|
||||
# The bootstrap exists to give PYTHONPATH overlays .pth processing (editable installs);
|
||||
# non-PYTHONPATH overlays (the POSIX venv-interpreter path) pass through plain. Both
|
||||
# overlay producers that exist today always set PYTHONPATH (the managed-store branch
|
||||
# sets only it, the uv re-exec sets it alongside VIRTUAL_ENV); a future overlay-only
|
||||
# producer must revisit this gate or it silently loses .pth processing.
|
||||
if env_overlay:
|
||||
return _windows_cron_bootstrap_argv(python_exe, env_overlay, str(path)), env_overlay, None
|
||||
return [python_exe, str(path)], env_overlay, None
|
||||
|
||||
@@ -404,11 +395,10 @@ def _run_job_script(
|
||||
if path is None:
|
||||
return False, err
|
||||
script_timeout = _get_script_timeout()
|
||||
argv, env_overlay, err = _script_argv(path)
|
||||
if argv is None:
|
||||
return False, err
|
||||
|
||||
try:
|
||||
argv, env_overlay, err = _script_argv(path)
|
||||
if argv is None:
|
||||
return False, err
|
||||
from tools.environments.local import build_subprocess_env
|
||||
# Lossy decode only: keep the platform-default (locale) encoding — gating ``encoding=``
|
||||
# to win32 was deliberate (#66566: unconditional UTF-8 leaked into POSIX) — but
|
||||
|
||||
@@ -306,143 +306,77 @@ class TestRunJobScript:
|
||||
assert argv == [sys.executable, str(script)]
|
||||
|
||||
@pytest.mark.platforms("posix")
|
||||
def test_posix_invocation_selects_venv_interpreter(self, tmp_path, monkeypatch):
|
||||
"""POSIX managed-store installs: cron ``.py`` scripts run on the selected dependency
|
||||
venv's interpreter — the bare store Python carries the repo/dependencies only
|
||||
in-process (#123044) — with lazy installs disabled and NO ``PYTHONPATH`` overlay: an
|
||||
inherited overlay makes foreign-interpreter children import the store's 3.14 extension
|
||||
modules first (#123440). No committed store → the caller's interpreter passes through
|
||||
untouched (pre-PM venvs, source checkouts), as does a store whose venv python
|
||||
vanished (half-migrated install must not crash the scheduler) or whose committed
|
||||
selection record is broken (``selected_venv`` raises → degrade, not propagate — the
|
||||
call site runs before ``_run_job_script``'s ``try``)."""
|
||||
def test_posix_managed_store_script_runs_on_venv_with_live_checkout(
|
||||
self, cron_env, tmp_path, monkeypatch
|
||||
):
|
||||
"""#123044/#123440: on a POSIX managed-store install a cron ``.py`` script imports the
|
||||
selected venv's packages, resolves Hermes from the LIVE checkout ahead of the venv's
|
||||
workspace snapshot, keeps ``python script.py`` path semantics, and leaves no
|
||||
``PYTHONPATH`` for its own children to inherit."""
|
||||
from cron import scheduler_script
|
||||
from pm.environments import site_packages
|
||||
|
||||
venv = tmp_path / "selected-venv" / "bin"
|
||||
venv.mkdir(parents=True)
|
||||
venv_python = venv / "python"
|
||||
venv_python.touch()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli._launchers.resolve_store_python", lambda repo: venv_python
|
||||
)
|
||||
monkeypatch.setattr("pm.environments.selected_venv", lambda repo: venv.parent)
|
||||
|
||||
assert scheduler_script._posix_cron_python_invocation(sys.executable) == (
|
||||
str(venv_python),
|
||||
{"HERMES_DISABLE_LAZY_INSTALLS": "1"},
|
||||
)
|
||||
|
||||
venv_python.unlink()
|
||||
assert scheduler_script._posix_cron_python_invocation(sys.executable) == (
|
||||
sys.executable,
|
||||
{},
|
||||
)
|
||||
|
||||
def _broken_selection(repo):
|
||||
raise RuntimeError(
|
||||
"dependency environment is missing or outside this install"
|
||||
)
|
||||
|
||||
monkeypatch.setattr("pm.environments.selected_venv", _broken_selection)
|
||||
assert scheduler_script._posix_cron_python_invocation(sys.executable) == (
|
||||
sys.executable,
|
||||
{},
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli._launchers.resolve_store_python", lambda repo: None
|
||||
)
|
||||
assert scheduler_script._posix_cron_python_invocation(sys.executable) == (
|
||||
sys.executable,
|
||||
{},
|
||||
)
|
||||
|
||||
@pytest.mark.platforms("posix")
|
||||
def test_posix_managed_store_script_argv_stays_plain(
|
||||
self, cron_env, tmp_path, monkeypatch
|
||||
):
|
||||
"""The POSIX venv-interpreter path must not go through the ``addsitedir`` bootstrap:
|
||||
the venv interpreter processes its own ``.pth`` files through ``pyvenv.cfg``, and a
|
||||
plain argv keeps the script's ``__file__``/``sys.path[0]`` semantics untouched."""
|
||||
from cron.scheduler_script import _script_argv
|
||||
|
||||
venv = tmp_path / "selected-venv" / "bin"
|
||||
venv.mkdir(parents=True)
|
||||
venv_python = venv / "python"
|
||||
venv_python.touch()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli._launchers.resolve_store_python", lambda repo: venv_python
|
||||
)
|
||||
monkeypatch.setattr("pm.environments.selected_venv", lambda repo: venv.parent)
|
||||
|
||||
script = cron_env / "scripts" / "probe.py"
|
||||
script.write_text('print("ok")\n', encoding="utf-8")
|
||||
|
||||
argv, overlay, err = _script_argv(script)
|
||||
assert err is None
|
||||
assert argv == [str(venv_python), str(script)]
|
||||
assert overlay == {"HERMES_DISABLE_LAZY_INSTALLS": "1"}
|
||||
|
||||
@pytest.mark.platforms("posix")
|
||||
def test_posix_managed_store_script_imports_via_venv(
|
||||
self, cron_env, tmp_path, monkeypatch
|
||||
):
|
||||
"""End-to-end #123044/#123440 shape: on a POSIX managed-store install a cron ``.py``
|
||||
script must import managed dependencies and Hermes modules by running on the selected
|
||||
venv's interpreter, and the environment it runs in must be clean — no ``PYTHONPATH``
|
||||
pointing at the store's paths, so children the script spawns never import the store's
|
||||
extension modules on a foreign interpreter."""
|
||||
from cron.scheduler_script import _run_job_script
|
||||
from pm.environments import site_packages as dependency_site
|
||||
|
||||
fake_venv = tmp_path / "selected-venv"
|
||||
# Standard venv layout: bin/python symlink + pyvenv.cfg + lib/pythonX.Y/site-packages.
|
||||
venv_bin = fake_venv / "bin"
|
||||
venv_bin.mkdir(parents=True)
|
||||
venv_python = venv_bin / "python"
|
||||
venv_python.symlink_to(sys.executable)
|
||||
(fake_venv / "pyvenv.cfg").write_text(
|
||||
f"home = {Path(sys.base_prefix) / 'bin'}\n"
|
||||
"include-system-site-packages = false\n",
|
||||
venv = tmp_path / "selected-venv"
|
||||
(venv / "bin").mkdir(parents=True)
|
||||
(venv / "bin" / "python").symlink_to(sys.executable)
|
||||
(venv / "pyvenv.cfg").write_text(
|
||||
f"home = {Path(sys.base_prefix) / 'bin'}\ninclude-system-site-packages = false\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
deps = dependency_site(fake_venv)
|
||||
deps = site_packages(venv)
|
||||
deps.mkdir(parents=True)
|
||||
(deps / "probe_pkg.py").write_text("VALUE = 42\n", encoding="utf-8")
|
||||
# Editable-style repo exposure, as a real PM venv carries for the checkout. Caveat:
|
||||
# a real generation venv gets its repo pointer from uv's editable install of the
|
||||
# generated workspace (not a hand-written .pth), and sealed-payload installs prune
|
||||
# editable .pth files outright because the payload wires the repo snapshot itself
|
||||
# (pm/environment.py prune_site_pth) — do not generalize this .pth shape to payloads.
|
||||
repo = Path(__file__).resolve().parents[2]
|
||||
(deps / "zz_repo.pth").write_text(f"{repo}\n", encoding="utf-8")
|
||||
snapshot = tmp_path / "workspace-snapshot"
|
||||
snapshot.mkdir()
|
||||
(snapshot / "hermes_constants.py").write_text("STALE = True\n", encoding="utf-8")
|
||||
(deps / "snapshot.pth").write_text(f"{snapshot}\n", encoding="utf-8")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli._launchers.resolve_store_python",
|
||||
lambda repo: Path(sys.executable),
|
||||
"hermes_cli._launchers.resolve_store_python", lambda repo: Path(sys.executable)
|
||||
)
|
||||
monkeypatch.setattr("pm.environments.selected_venv", lambda repo: fake_venv)
|
||||
monkeypatch.setattr("pm.environments.selected_venv", lambda repo: venv)
|
||||
monkeypatch.delenv("PYTHONPATH", raising=False)
|
||||
|
||||
script = cron_env / "scripts" / "probe.py"
|
||||
script.write_text(
|
||||
"import os\n"
|
||||
"import probe_pkg\n"
|
||||
"import hermes_constants\n"
|
||||
"import os, sys, probe_pkg, hermes_constants\n"
|
||||
"print(probe_pkg.VALUE)\n"
|
||||
"print('PP=' + (os.environ.get('PYTHONPATH') or ''))\n",
|
||||
"print(hermes_constants.__file__)\n"
|
||||
"print(sys.path[0])\n"
|
||||
"print('PYTHONPATH=' + (os.environ.get('PYTHONPATH') or ''))\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
success, output = _run_job_script("probe.py")
|
||||
success, output = scheduler_script._run_job_script("probe.py")
|
||||
assert success is True, output
|
||||
lines = output.strip().splitlines()
|
||||
assert lines[0] == "42"
|
||||
pp_line = next(line for line in lines if line.startswith("PP="))
|
||||
# The #123440 contract: the store's repo/dependency paths never leak onto PYTHONPATH.
|
||||
assert str(deps) not in pp_line
|
||||
assert str(repo) not in pp_line
|
||||
value, constants_file, path0, pythonpath = output.splitlines()
|
||||
assert value == "42"
|
||||
repo = Path(scheduler_script.__file__).resolve().parents[1]
|
||||
assert Path(constants_file).resolve() == repo / "hermes_constants.py"
|
||||
assert Path(path0).resolve() == script.parent.resolve()
|
||||
assert pythonpath == "PYTHONPATH="
|
||||
|
||||
@pytest.mark.platforms("posix")
|
||||
def test_posix_unusable_store_selection_fails_the_run_not_the_tick(
|
||||
self, cron_env, monkeypatch
|
||||
):
|
||||
"""A broken PM selection record is reported as a failed run (``selected_venv``'s
|
||||
contract: never silently load another environment) instead of escaping
|
||||
``_run_job_script`` and stranding the execution row."""
|
||||
from cron.scheduler_script import _run_job_script
|
||||
|
||||
def _broken(repo):
|
||||
raise RuntimeError("dependency environment is missing or outside this install")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli._launchers.resolve_store_python", lambda repo: Path(sys.executable)
|
||||
)
|
||||
monkeypatch.setattr("pm.environments.selected_venv", _broken)
|
||||
(cron_env / "scripts" / "probe.py").write_text('print("ok")\n', encoding="utf-8")
|
||||
|
||||
success, output = _run_job_script("probe.py")
|
||||
assert success is False
|
||||
assert "dependency environment is missing" in output
|
||||
|
||||
def test_emoji_stdout_round_trips_through_script_capture(self, cron_env):
|
||||
"""Emoji in script stdout must reach the caller intact (#42384).
|
||||
|
||||
Reference in New Issue
Block a user