fix(cron): external worker dies on missing dependency site-packages (ruamel)

[etrisnanto]

Summary

The restart-safe external cron worker (python -m cron.scheduler) dies before its ownership acknowledgement with ModuleNotFoundError: No module named 'ruamel', which fails every dispatched cron job on a managed-systemd gateway install.

Root cause:

• The worker entry point never runs the launcher bootstrap, so the PM dependency environment (which holds ruamel.yaml etc.) never reaches its sys.path.
• The shared subprocess sanitizer then strips the gateway's Hermes-owned PYTHONPATH entries (as intended for user children — but this child is Hermes), and pin_hermes_tree_on_pythonpath only re-pinned the repo root.

Observed failure: Restart-safe cron worker dispatch failed: cron external worker exited before ownership acknowledgement (exit 1) with the traceback ending in hermes_yaml.py:10 -> from ruamel.yaml import YAML.

Fix

cron/scheduler_worker_env.py: pin_hermes_tree_on_pythonpath now also prepends the committed dependency environment's site-packages (pm.environments.selected_venv + site_packages) alongside the repo root, so the worker's import chain resolves. Resolved lazily at spawn time in the gateway process (which itself runs on the committed environment), with a try/except fallback to the previous behavior for installs without a committed environment.

Test plan

• Reproduced the failure: worker-style invocation with repo-root-only PYTHONPATH dies on import hermes_yaml with exactly the observed traceback.
• Verified the fix: same invocation with the pinned PYTHONPATH from the patched code imports hermes_bootstrap, cron.scheduler successfully.
• End-to-end on a live gateway (systemd-managed): cronjob run of a real job now completes with status ok (was error before).

[Enough1122]

AI code review — automated follow-up for reference; not a maintainer.

This PR fixes the restart-safe external cron worker dying on import hermes_yaml → ModuleNotFoundError: No module named 'ruamel' by adding the PM dependency environment's site-packages to the worker's PYTHONPATH in cron/scheduler_worker_env.py. The new _committed_site_packages() helper resolves site_packages(selected_venv(repo_root)) and the pin function now builds a two-entry list instead of short-circuiting. One real defect: the wheel/pipx early-return contract that this same PR's existing test pins is now unreachable, so the two assertions at the end of test_launch_external_worker_pin_extends_the_sanitized_env_not_os_environ fail on this head. No comments or reviews exist on the thread yet.

1. repo_root == purelib no longer returns the env untouched — the wheel/pipx skip is dead code, and the PR's own test for it now fails. cron/scheduler_worker_env.py:56-67 (post-image): the guard used to be

   if _installed_purelib() == Path(root).resolve():
       return worker_env

   and is now

   entries = []
   if _installed_purelib() != Path(root).resolve():
       entries.append(root)
   dep_site = _committed_site_packages(repo_root)
   if dep_site is not None:
       entries.append(str(dep_site))
   if not entries:
       return worker_env

   The return worker_env now requires both "repo root is not purelib" and "_committed_site_packages() returned None". _committed_site_packages() is a try/except Exception: return None wrapper around site_packages(selected_venv(repo_root)), and selected_venv() is _recorded_venv(project_root) or base_venv(project_root) — base_venv() is payload_venv(project_root) or project_venv_dir(...) or <root>/"venv", whose last term is an invented path that is returned unconditionally, never probed. So on a pipx/uv-tool install with no in-tree venv/.venv, no sealed manifest.json, and no facts.json, selected_venv() answers <repo_root>/venv and _committed_site_packages() answers <repo_root>/venv/Lib/site-packages (POSIX: .../lib/python<X.Y>/site-packages) — a directory that does not exist. entries is non-empty, the function returns a modified env, and a non-existent path is prepended to the worker's PYTHONPATH. That is precisely the situation the original comment warned about: "pinning it would move site-packages ahead of the stdlib on sys.path" — the deletion of that guard is the defect, not the addition of dep_site.

   Reproduction (probe run against the real post-image cron/scheduler_worker_env.py and pm/environments.py at 2968af34, with only hermes_constants' three imported names stubbed; HERMES_HOME set to a scratch dir, nothing written to the live runtime):

   # pipx / wheel layout: repo_root IS purelib, no in-tree venv
   dep_site : ...\checkout\venv\Lib\site-packages        # does not exist
   result == untouched        -> False   [test line 647 asserts True]
   'PYTHONPATH' not in result -> False   [test line 648 asserts True]
   

   And the two lines the existing test already asserts, tests/cron/test_restart_safe_worker.py:645-648:

   monkeypatch.setattr(worker_env_mod, "_installed_purelib", lambda: repo_root)
   untouched = {"PYTHONPATH": str(tmp_path / "kept-by-sanitizer")}
   assert worker_env_mod.pin_hermes_tree_on_pythonpath(dict(untouched), repo_root) == untouched
   assert "PYTHONPATH" not in worker_env_mod.pin_hermes_tree_on_pythonpath({}, repo_root)

   Both fail on this head. The second one is the more dangerous direction: an empty input env gains a PYTHONPATH key pointing into a directory that was never created.

   Note the test only fails by accident of layout — it monkeypatches _installed_purelib but passes the real repo root as repo_root, and in CI the checkout has no in-tree venv/.venv (the test env is a separate .venv beside it), so base_venv()'s or <root>/"venv" fallback fires and the assertions break. If CI happened to run with the checkout's own venv, the test would pass while the pin still misbehaved. That asymmetry is worth fixing rather than accommodating.

   Suggested fix — keep the purelib skip load-bearing, and only trust a dep_site that actually exists:

   if _installed_purelib() == Path(root).resolve():
       return worker_env
   entries = [root]
   dep_site = _committed_site_packages(repo_root)
   if dep_site is not None and Path(dep_site).is_dir():
       entries.append(str(dep_site))

   If the intent really is to also serve pipx installs, then the existence check belongs in _committed_site_packages() (return None unless site_packages(...) is a real directory), and the docstring should say so — as written, the helper's contract is "always answers a path", which is what makes the swallowed except dangerous.

2. The bare except Exception: return None makes item 1 silent. cron/scheduler_worker_env.py:39-44: every real failure inside pm.environments — a corrupt facts.json (which _recorded_venv deliberately turns into RuntimeError("cannot read dependency environment: …")), an ImportError, a venv replaced mid-flight — is converted into "no dependency environment", i.e. the original No module named 'ruamel' failure returns with no log line and no distinguishing signal. The same pattern in _installed_purelib() right above it is defensible because it is a display-only probe; here the return value is load-bearing for the worker's import path. Catching the expected ImportError/OSError and logging anything else would keep the diagnosis intact. (This is why my first probe run reported the opposite conclusion — an ImportError in the harness was swallowed the same way. Worth flagging as a review hazard, not a style point.)

Minor:

• The docstring for pin_hermes_tree_on_pythonpath still describes only the repo-root pin ("Prepend repo_root … The repo-root pin is skipped when …") and never states what happens to dep_site in the purelib case, which is exactly the case that changed behaviour. Worth one sentence saying the skip applies to repo_root only.
• site_packages() on POSIX derives the version from pyvenv.cfg via venv_python_version(venv) or (sys.version_info.major, sys.version_info.minor). The or fallback dates the path from the caller, which the function's own docstring in pm/environments.py:220-225 calls out as a bug that caused a real failure. A missing pyvenv.cfg in a synthesized-venv path therefore yields lib/python3.11/site-packages derived from the launching interpreter, not the tree. The .is_dir() check above makes this harmless for the empty case, but it is the same trap.

Unverified / please confirm:

• Derived by reading the code on head 2968af34, and reproduced with a standalone probe (real post-image cron/scheduler_worker_env.py + pm/environments.py, stubbed hermes_constants). I did not run the repository's pytest suite: this host has no usable dependency environment for it, so the two failing assertions above are predicted from the probe's inputs, not observed as a pytest failure. Please confirm by running tests/cron/test_restart_safe_worker.py::test_launch_external_worker_pin_extends_the_sanitized_env_not_os_environ on a machine where CI can execute it — the check-runs API reports no results for this head, so I could not read CI's verdict either.
• The exact directory string differs by platform (Lib/site-packages on Windows, lib/python<X.Y>/site-packages on POSIX); I ran the probe on Windows, so the POSIX spelling is read from pm/environments.py:250-256, not executed.
• Whether the "wheel / pipx layout" case is still a supported install mode, or already retired in favour of PM-managed installs, changes the severity: if pipx installs are gone, the practical impact drops to the two failing test assertions rather than a shipped bad PYTHONPATH. I did not establish that.

[kshitijk4poor]

Closing as superseded by #126344, which fixes the same failure (the cron external worker dying on the missing dependency site-packages / ruamel) by carrying #122290/#122936. It boots PM's dependencies in the cron package prelude, leases the generation for the worker's lifetime, and fails closed on activation errors. You're credited in the PR body. Thank you. If #126344 stalls, I'll reopen this.