fix(cron): reap stale execution claims before a one-shot hermes cron run dispatch - #86862
Closed
ygd58 wants to merge 1 commit into
Closed
fix(cron): reap stale execution claims before a one-shot hermes cron run dispatch#86862ygd58 wants to merge 1 commit into
hermes cron run dispatch#86862ygd58 wants to merge 1 commit into
Conversation
… run` dispatch Fixes NousResearch#86721. `hermes cron run <job_id>` (a one-shot CLI invocation) dispatches manual runs via the same background-delegation path as an agent's `cronjob(action='run')` tool call (tools/cronjob_tools.py's _try_dispatch_background_run -> dispatch_async_delegation(role= "cron_run", runner=_runner, ...)). The runner thread lives in the calling process's shared daemon executor. When the one-shot process exits right after printing "Triggered job: ...", the in-flight runner dies mid-execution, leaving its cron/executions.db row permanently stuck at status='claimed' -- every subsequent `hermes cron run` on the same job then reports "Ran now: failed" because of the still-claimed row. cron/executions.py already has the exact self-heal this needs: recover_interrupted_executions() correctly identifies and reclassifies 'claimed'/'running' rows whose owner process has provably exited (_owner_is_live checks PID existence AND matches process start-time, so a reused PID isn't mistaken for the original live owner) to 'unknown', unblocking the job for a fresh claim. But it was only ever called once, at the long-lived scheduler ticker's own startup (cron/scheduler.py:379's self.recover_interrupted()) -- a one-shot CLI invocation has no equivalent "startup" moment of its own, so this self-heal never ran for it. Added a call to recover_interrupted_executions() at the top of _try_dispatch_background_run, right after the async-delivery-supported gate and before any claim attempt for the current job -- mirroring exactly what the long-lived scheduler already does at its own startup, just triggered per one-shot invocation instead of once at daemon startup. Wrapped in try/except: pass (best-effort; a failure here must not block the actual dispatch this function exists for). Traced (but did not attempt to fix) the deeper "why does the runner die with the process at all" question -- that's the harder problem options 1/2 in the issue describe (route to the persistent scheduler, or block the one-shot process until completion). This fix addresses the more urgent, more clearly-scoped symptom: a stranded stale claim permanently blocking ALL future manual runs of the affected job, which is option 3 from the issue and the one with an existing, already- correct implementation just needing to be wired into this call site. Added 3 regression tests to a new file, following the established real-subprocess dead-owner pattern already used in tests/cron/test_execution_ledger.py (a genuinely-dead PID, not a mock, matching the real-world failure mode exactly): a sanity test confirming the stale claim sits unrecovered without the fix; a direct test of recover_interrupted_executions() reaping such a claim; and a unit test on _try_dispatch_background_run itself confirming recovery is called before any claim attempt. Verified as a genuine regression by reverting the fix and confirming the unit test fails with recovery never having been called. 35/35 pass across the new test file plus tests/cron/test_execution_ledger.py and tests/tools/test_cronjob_run_background.py (no regression).
Contributor
|
Merged via #86899 — your commit was cherry-picked onto current main with your authorship preserved in git log (merge commit bd3a966). We added a small follow-up on top replacing the bare Nice find wiring |
atirna
pushed a commit
to atirna/hermes-agent
that referenced
this pull request
Aug 17, 2026
…g it Follow-up to the salvaged NousResearch#86862: surface reclaim counts at warning level (mirrors the scheduler tick's reap handling from NousResearch#86853) and keep a debug trace when the best-effort recovery itself fails, instead of a bare pass.
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.
Fixes #86721.
Root cause
hermes cron run <job_id>dispatches manual runs via_try_dispatch_background_run->dispatch_async_delegation(role="cron_run", runner=_runner, ...). The runner thread lives in the calling process's shared daemon executor. When the one-shot process exits right after printing "Triggered job: ...", the in-flight runner dies mid-execution, leaving itscron/executions.dbrow permanently stuck atstatus='claimed'-- every subsequenthermes cron runon the same job then reports "Ran now: failed".cron/executions.pyalready has the exact self-heal this needs:recover_interrupted_executions()correctly identifies and reclassifies stranded 'claimed'/'running' rows whose owner process has provably exited (checking both PID existence and process start-time, so a reused PID isn't mistaken for the original owner). But it was only ever called once, at the long-lived scheduler ticker's own startup -- a one-shot CLI invocation has no equivalent "startup" moment, so this self-heal never ran for it.Fix
Added a call to
recover_interrupted_executions()at the top of_try_dispatch_background_run, before any claim attempt -- mirroring exactly what the long-lived scheduler already does at its own startup, just triggered per one-shot invocation instead. Wrapped intry/except: pass(best-effort; a failure here must not block the actual dispatch).Traced but did not attempt to fix the deeper "why does the runner die with the process at all" question -- that's the harder problem the issue's options 1/2 describe. This fix addresses the more urgent, clearly-scoped symptom: a stranded stale claim permanently blocking ALL future manual runs, using an already-correct existing implementation that just needed wiring into this call site.
Verification
Added 3 regression tests following the established real-subprocess dead-owner pattern already used in
tests/cron/test_execution_ledger.py(a genuinely-dead PID, not a mock). Verified as a genuine regression by reverting the fix and confirming the unit test fails with recovery never having been called.35/35 pass across the new test file plus two related existing test files (no regression).