Skip to content

fix(cron): launch external worker via installation-bound runtime command - #124694

Closed
dskwe wants to merge 1 commit into
NousResearch:mainfrom
dskwe:fix/cron-external-worker-runtime-124279
Closed

dskwe wants to merge 1 commit into
NousResearch:mainfrom
dskwe:fix/cron-external-worker-runtime-124279

Conversation

@dskwe

@dskwe dskwe commented Sep 27, 2026

Copy link
Copy Markdown

Summary

On a managed-store install (POSIX), the external cron worker is spawned as sys.executable -m cron.scheduler (cron/scheduler.py::_launch_external_cron_worker). The gateway process runs on the bare store Python with the committed dependency generation activated in-process only (hermes_bootstrap → pm.environments.activate_dependencies), and the sanitizer-built worker env deliberately drops runtime site-packages (cron/scheduler_worker_env.py). So the spawned worker re-resolves imports from scratch and dies with ModuleNotFoundError: No module named ruamel before its ownership acknowledgement — the job's execution row is left un-acked and the run fails. Observed 2026-09-26: daily-doc-update failed pre-ack on a managed install where hermes doctor passes and the runtime venv contains ruamel.yaml.

Fix

Launch the worker through the same installation-bound runtime command every other Hermes entry point uses: hermes_cli._launchers.runtime_command(repo_root, [...argv], module="cron.scheduler") — store python -I -c <bootstrap> importing hermes_bootstrap, which leases the current committed generation at child start (so a dependency update between gateway start and worker spawn is picked up correctly, and stale generations are never captured in a persisted argv). The --external-worker-file/--ack-file handoff argv is preserved verbatim after the bootstrap prefix; the existing checkout PYTHONPATH pin (#112729) still applies on top of the sanitizer-built env.

hermes_bootstrap's own module docstring already lists cron/scheduler among the entry points that must import it — the external spawn path was the one that bypassed it.

Relationship to #122951

That PR addresses the same symptom by resolving the committed venv's interpreter and swapping it in place of sys.executable (not linked to this issue). This PR implements the fix this issue's report proposes: "launch the external worker through the installation-bound runtime launcher/bootstrap, preserving the existing worker argv and PYTHONPATH pin". Two meaningful differences, both inherited from reusing runtime_command rather than resolving a venv path:

  • No new resolution logic to fail open. The AI review on fix(cron): spawn external workers with the committed venv interpreter #122951 flagged that its except Exception: return None silently maps a corrupted runtime-facts record onto the bare-interpreter fallback this bug exists to avoid. runtime_command is the already-hardened, already-tested path (resolve_store_python → fallback) used by every launch publication.
  • Generation leased in the child, not captured at spawn. The bootstrap selects the committed generation when the worker starts, mirroring cron/scheduler_script.py's store-Python/dependency-venv split for .py job scripts — the precedent this repo already sanctions for the same class of problem.

Testing

  • New tests/cron/test_external_worker_runtime_command.py (3 tests): spawned argv is -I -c bootstrap-shaped with import hermes_bootstrap + cron.scheduler; handoff argv (--external-worker-file/--ack-file) survives after the prefix; command is built via runtime_command (spied) rather than raw sys.executable. Red before the fix, green after.
  • Full tests/cron/ suite: 1374 passed, 11 skipped — includes the existing external-worker ownership/ack/adoption-grace tests (test_restart_safe_worker.py), which pass unchanged with the new command shape.
  • Not verified on a live managed-store POSIX install (no such environment at hand); the regression tests pin the command shape. A managed install smoke run (hermes cron run <job> with external workers enabled) would confirm end-to-end.

Fixes #124279

A managed-store gateway runs on the bare store Python with the committed
dependency generation activated in-process only (hermes_bootstrap ->
pm.environments.activate_dependencies), and the sanitizer-built worker
env deliberately drops runtime site-packages. A worker spawned as
`sys.executable -m cron.scheduler` therefore re-resolves imports from
scratch and dies with ModuleNotFoundError (e.g. ruamel) before its
ownership ack.

Launch through the same installation-bound runtime command every other
Hermes entry point uses (hermes_cli._launchers.runtime_command): store
python -I -c <bootstrap> importing hermes_bootstrap, runpy-ing
cron.scheduler with the --external-worker-file/--ack-file argv
preserved. The bootstrap leases the committed generation in the child,
so the worker's imports match the gateway's. Existing PYTHONPATH
checkout pin (NousResearch#112729) still applies via the sanitizer-built env.

Fixes NousResearch#124279
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cron Cron scheduler and job management area/install-update Installer, updater, packaging, wheels, doctor needs-decision Awaiting maintainer decision before any implementation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades duplicate This issue or pull request already exists labels Sep 27, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Duplicate of #122719: both replace the same external-worker bare-interpreter launch with runtime_command, with no material scope or mechanism delta.

@ehz0ah ehz0ah left a comment

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 production change is sound. runtime_command() is the right boundary here: the worker resolves the committed dependency generation when the child starts, hermes_bootstrap acquires the child-owned generation lease, and the existing ownership handoff remains unchanged.

Evidence from this exact head:

  • Focused cron, handoff, and launcher-lifetime suites: 48 passed, 0 failed, 13 OS-specific skips on macOS.
  • The same suites pass after a clean composition with current main at 9a0a162.
  • Two additional behavioral probes pass: the real cron.scheduler entry reaches payload handling without ModuleNotFoundError, and the child keeps its old generation alive after selection rotates until the child exits.
  • Ruff and git diff --check pass.
  • The head is 721 commits behind current main but merges cleanly.
  • GitHub has no hosted checks for this head, so Linux and Windows CI evidence is still absent.

One test issue remains inline. The submitted regression inspects the generated bootstrap source instead of running it. Please replace that source-shape check with the real child behavior it is meant to protect.

This PR and the earlier draft #122719 implement the same production mechanism. They should not both merge. #122719 opened first and already contains a real-spawn regression for the import boundary. My preference is to retain #122719 once its author considers it ready, or move that behavioral test here if maintainers choose #124694. Close the other candidate after that choice.

"(store python -I -c <bootstrap>), got: %r" % (command[:3],)
)
bootstrap = command[3]
assert "import hermes_bootstrap" in bootstrap

@ehz0ah ehz0ah Sep 28, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This checks the generated Python source, not the behavior that failed in production. A bootstrap refactor can break this assertion while still working, and a command can contain both strings while still failing before it imports the selected dependencies.

Please run the produced command under a managed-store fixture and assert that the real cron.scheduler entry reaches payload handling without ModuleNotFoundError. Also cover generation rotation: while the child is alive, publish a new selected generation and run collect_generations(); the child's leased generation must remain until it exits. Both probes pass against this production change, and #122719 already has the first real-spawn pattern.

@kshitijk4poor

Copy link
Copy Markdown

Closing as superseded by #126344, which fixes the same failure: the restart-safe cron external worker dying with No module named 'ruamel' before its ownership ack. It carries #122290/#122936 and boots PM's own activate_dependencies in the cron package prelude, before cron.jobs. That leases the committed generation for the worker's lifetime, so the collector cannot delete it mid-job, and a broken activation fails the dispatch before the ack instead of running on unheld dependencies. Putting the dependency path on PYTHONPATH, or choosing a different interpreter, makes the import work but does not hold that lease.

Thank you @dskwe, you're credited in the #126344 body. If #126344 stalls, I'll reopen this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cron Cron scheduler and job management duplicate This issue or pull request already exists needs-decision Awaiting maintainer decision before any implementation P1 High — major feature broken, no workaround sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cron external worker uses raw interpreter and loses runtime dependencies

4 participants