Repository navigation
Cron external workers no longer relaunch after their ack and leave the run unknown (#124827, salvage #133720) - #134358
Merged
kshitijk4poor merged 6 commits intoOct 7, 2026
Conversation
…e its ack The external worker first imported hermes_bootstrap through run_agent inside run_one_job, after it had read (and deleted) its payload and published its ownership ack. When the dependency inputs were not current, prepare_launch synced and relaunched the worker into an -I interpreter that ignores the pinned PYTHONPATH, and worker_bootstrap had already popped the boot marker: the new process died on `import ruamel` and the adopted run was left `unknown` (#122290 follow-up). The worker's __main__ now runs hermes_bootstrap (finish_worker_boot) before it touches the payload, and the marker is dropped only after that, so a relaunch replays the whole worker and boots its dependencies again. The call cannot live in worker_bootstrap(): during `-m cron.scheduler`'s package import sys.argv[0] is still "-m" and __main__ has no spec, so the relaunch command could not name the module. Covers the POSIX execv and Windows subprocess.call relaunch branches alike. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b537b59)
…er's ack run_agent's import also re-execs the process when it restores a tree a killed `hermes update` half-wrote (_early_recovery.relaunch_after_restore). The external worker never loads hermes_cli.main, so that check first ran inside run_one_job, after the ack: the re-exec found its payload deleted, refused the execution, and the adopted run was left `unknown`, the same class as the source-update relaunch. finish_worker_boot() now runs the recovery before the payload is read, so its re-exec replays the whole worker. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b28779f)
…nterrupted-pull repair Importing hermes_bootstrap already runs _settle_interrupted_update (restore_interrupted_pull + relaunch_after_restore) at module import, so the explicit second call in finish_worker_boot was a duplicate. The worker never imports hermes_bootstrap before __main__ (probed: not in sys.modules after `import cron.scheduler`, marked or not), so the import in finish_worker_boot is what runs the repair, before the payload is read and the ack published. The interrupted_pull case of test_relaunch_replays_the_worker_before_its_ack stays green.
…er docstrings After the previous commit the worker's pre-ack recovery is the import of hermes_bootstrap, not run_agent's copy; the module docstring and the relaunch test still credited run_agent. Also restore the scheduler comment's original wrap (whitespace-only churn from the first pick). (Lead fold: quality + thermo + reuse lenses, Low.)
૮ >ﻌ< ა ci reviewran on 06c02c5 — test(cron): pin that a booted worker drops the marker before debug infoCI timingsCI timings · View report · View jobWall time 14m32s vs 44m28s (-67.3%). 9 job(s) slower, 5 faster, 2 unchanged.
|
|
…child Review (greptile): after the boot moved before the ack, the tests checked that a relaunch keeps the marker but none checked that a boot without a relaunch consumes it. Job children would otherwise inherit it and repeat the dependency boot.
kshitijk4poor
enabled auto-merge (rebase)
October 7, 2026 07:49
13 of 14 tasks
kshitijk4poor
deleted the
fix/p1-20261007-cron-worker-relaunch-before-ack
branch
October 7, 2026 08:03
This was referenced Oct 7, 2026
fix(cron): stop an adopted external worker from re-exec'ing and losing its payload (#124827)
#124855
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A restart-safe cron external worker no longer relaunches itself after its ownership ack, which left the adopted run
unknownand dropped the job. The worker now finishes itshermes_bootstrapboot, including a pending source-update relaunch or interrupted-pull re-exec, before it reads its payload, so a relaunch replays the whole worker.Fixes #124827 (salvage #133720)
What happened
On main, the worker read and deleted its payload, adopted the execution and published the
.readyack. Only then didrun_one_jobimportrun_agent, which was the worker's firstimport hermes_bootstrap. If a source update was pending,prepare_launchrelaunched the worker into an-Iinterpreter that ignores the pinnedPYTHONPATH.worker_bootstrap()had already popped_HERMES_CRON_WORKER_BOOT, so the new process skipped the PM dependency boot, died onruamel, and its payload was already gone. An interruptedhermes updatepull re-exec had the same post-ack timing. This was observed in production on a self-managed WSL install, and it has been the behaviour since the worker marker landed (#122936).Change
cron/worker_bootstrap.pyworker_bootstrap()reads the marker (get) instead of consuming it.finish_worker_boot()importshermes_bootstrap, whose import settles an interrupted pull and finishes a pending source update. It then pops the marker.cron/scheduler.py:finish_worker_boot()is the first statement of the--external-worker-filebranch, before argparse, the payload read, adoption and the ack. It adds 1 line.tests/cron/test_restart_safe_worker.pycron/AGENTS.md: the hardening invariant describes the new order. Its wording comes from fix(cron): a source-update relaunch replays the external worker before its ack #133720 and still namesrun_agent's recovery; a one-line refresh naminghermes_bootstrapwaits on approval for protected-file edits.Credit
@mzyas (first-time contributor) wrote the fix and both tests in #133720. Their 2 commits are cherry-picked with authorship kept (
4d3b390df0,873dd42631), andcontributors/emails/mzykr@yahoo.co.jpmaps them.Our follow-ups:
8a33ea0f86drops the explicit_early_recoverycall. Main'shermes_bootstrap._settle_interrupted_update(teknium1,f1def901fb/fd73042f2d/99e954838f, 10-03/04) already runs that repair at import.e1d335d276fixes docstring wording and reverts comment rewrap churn.06c02c54ffadds a test that a boot without a relaunch consumes the marker (review).Verification
Head
06c02c54ffon main152aa695bb.cron.scheduleras__main__)hermes_bootstrapvs payload openprepare_launchreturns a relaunchRELAUNCH payload_exists=True ack_exists=False marker=1tests/cron/test_restart_safe_worker.py,tests/pm/test_runtime.py,tests/cron/test_scheduler.py: 134 passed, 4 skipped (Linux-only), 0 failed. The PR's tests on main: 3 failed, 1 passed.__main__call each turns the tests red.ruff, the Windows footgun scan,git diff --checkand code-health are clean (cron/scheduler.py+1 line).Maintainer notes
hermes_bootstrapimport before its ack (measured 0.05–0.09 s cold).no_agentjobs did not pay it before. This is deliberate: the relaunch has to happen pre-ack whatever the job type.hermes_bootstrap). It is idempotent and bounded. The package-init call must stay becausecron.jobsneeds dependencies before__main__runs.run_agent.py's own import-time recovery can still re-exec after the ack if an updater held custody at worker start and died beforerun_agentwas imported. That code is on the update-recovery surface, so it is left for a separate change.