Skip to content

fix(cron): stop an adopted external worker from re-exec'ing and losing its payload (#124827) - #124855

Closed
PRATHAMESH75 wants to merge 1 commit into
NousResearch:mainfrom
PRATHAMESH75:fix/cron-external-worker-no-reexec
Closed

PRATHAMESH75 wants to merge 1 commit into
NousResearch:mainfrom
PRATHAMESH75:fix/cron-external-worker-no-reexec

Conversation

@PRATHAMESH75

Copy link
Copy Markdown

What

A scheduled cron job can vanish after the restart-safe external worker acknowledges durable ownership: the execution is recorded unknown (not failed), with no output or delivery, and the next occurrence advances normally.

Fixes #124827.

Root cause

The external worker imports the agent lazily inside run_one_job, which pulls in hermes_bootstrap. That module's top-level prepare_launch() (hermes_cli/venv_sync.py) can decide the process must restart — a pending self-update, or a different store interpreter — and os.execv() itself. Inside an already-adopted worker this is fatal: _run_external_worker_payload consumed and deleted the one-shot --external-worker-file payload before adoption, so the re-exec'd process starts with a dangling path, cannot resume, and the gateway — which already received the acknowledgement — records an indeterminate unknown run with no automatic retry.

This is distinct from #122222 (missing deps before acknowledgement, reported failed) and #120328 (worker death after acknowledgement).

Fix

The gateway, not a job worker, owns updates. Spawn the external worker with HERMES_DISABLE_LAZY_INSTALLS=1 in its environment. prepare_launch() already honors this flag and returns None (no update completion, no re-exec), so the adopted worker can never lose its handoff to an os.execv(). Dependency activation still runs — only the lazy update/re-exec path is suppressed, which is exactly what an unattended worker should never perform.

This is a one-line env addition alongside the existing worker-env hardening (presence-var scrubbing, PYTHONPATH pinning) in _launch_external_cron_worker, matching the mechanism the boot path already exposes.

Tests

  • tests/cron/test_restart_safe_worker.py::test_launch_external_worker_disables_lazy_reexec — the spawned worker env carries HERMES_DISABLE_LAZY_INSTALLS=1.

While in this file the Windows-footgun gate flagged four pre-existing lines (BOM-intolerant read_text(encoding="utf-8"), a bare read_text(), and a bare signal.SIGKILL); fixed them per the repo policy (utf-8-sig reads, getattr(signal, "SIGKILL", signal.SIGTERM)) so the touched file passes the gate.

The affected cron suite passes.

…g its payload (NousResearch#124827)

An external cron worker imports the agent lazily inside run_one_job, which pulls
in hermes_bootstrap; its module-level prepare_launch() can decide the process
must re-exec (a pending self-update or a different store interpreter) and
os.execv() itself. In an already-adopted worker that is fatal: the one-shot
payload named by --external-worker-file was consumed and deleted before
adoption, so the re-exec'd process starts with a dangling path, cannot resume,
and the gateway records the run 'unknown' with no output or delivery while the
next occurrence advances normally.

The gateway, not a job worker, owns updates. Spawn the external worker with
HERMES_DISABLE_LAZY_INSTALLS=1 so prepare_launch() returns None and never
re-execs; dependency activation still runs, only the lazy update/re-exec path is
suppressed. Add a regression test asserting the spawned worker env carries the
flag.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management labels Sep 27, 2026
@Enough1122

Copy link
Copy Markdown

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

Setting HERMES_DISABLE_LAZY_INSTALLS=1 on the worker env (cron/scheduler.py:3569) does stop the fatal re-exec. I loaded the real hermes_cli/venv_sync.py from this head and ran prepare_launch in both states against a fake install with updateMechanism="self": with the flag it returns None and performs no work; without it, a stale venv returns the store interpreter (reexec=True, calls finish then publish). The systemd path is also fine — subprocess.Popen(dispatch.argv, env=worker_env) at cron/scheduler.py:3578 hands the env to the systemd-run wrapper, which passes it to the child, and the degraded branch uses the same worker_env.

One correction to the comment, since the flag is load-bearing beyond the re-exec:

  1. "dependency activation still runs, only the re-exec is suppressed" understates it — cron/scheduler.py:3567-3568. The var has a second, documented consumer: lazy_installs_allowed() returns False when it is set (pm/install.py:138-143), and pm/client.py:59-67 then refuses on-demand package bootstrapping. Its own docs call it "Internal PM policy used by tests and install probes … overrides the user-facing security.allow_lazy_installs setting" (website/docs/reference/environment-variables.md:855). So the worker silently loses on-demand dependency installation for its whole life, including anything its own tools try to acquire. Probably right, given the gateway owns installs — but the comment should say so, or a future reader narrowing it to "just the re-exec" would reintroduce the install behavior this change also removes.

Minor: test_launch_external_worker_disables_lazy_reexec asserts only that the env dict carries the key; the prepare_launch behavior its docstring relies on is never exercised. Not blocking — my run confirms the docstring's claim is true. The utf-8-sig / getattr(signal, "SIGKILL", …) edits in the same file are unrelated to #124827; fine to land, just noting the bundling.

Unverified / please confirm: I could not run the repo's pytest, so the above comes from executing the real prepare_launch with pm / hermes_cli._launchers stubbed. The lazy_installs_allowed() reading is from pm/install.py:127-143 and pm/client.py:52-71, not from running them.

@kshitijk4poor

Copy link
Copy Markdown

Fixed on main by de116d8 (#134358), salvaged from #133720 (thanks @mzyas, authorship kept). The external cron worker now runs hermes_bootstrap from its __main__ before it reads the payload or publishes the ack, and keeps the PM boot marker until then. A source-update relaunch therefore replays the whole worker instead of landing after the ack with the payload gone. The interrupted-pull re-exec is covered by hermes_bootstrap's own launch-time repair.

Thanks @PRATHAMESH75 for this alternative. Closing in favour of #134358. If you still see #124827 on a build that includes de116d8, please comment and we'll reopen.

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

Labels

comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cron): adopted external worker can re-exec after deleting one-shot payload, leaving run unknown

4 participants