From 3cdb0c80a62aa4b36b32e04e1132808601cda0b7 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 27 Sep 2026 13:50:10 +0530 Subject: [PATCH] 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). --- cron/scheduler_script.py | 92 ++++++++--------- tests/cron/test_cron_script.py | 174 ++++++++++----------------------- 2 files changed, 95 insertions(+), 171 deletions(-) diff --git a/cron/scheduler_script.py b/cron/scheduler_script.py index 843be875b0..5d9efea3a7 100644 --- a/cron/scheduler_script.py +++ b/cron/scheduler_script.py @@ -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 diff --git a/tests/cron/test_cron_script.py b/tests/cron/test_cron_script.py index 10621f718f..052190abda 100644 --- a/tests/cron/test_cron_script.py +++ b/tests/cron/test_cron_script.py @@ -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).