Fix scheduler wedge: scope tick lock to dispatch, add LLM stream timeouts - #29350
Closed
benjaminm95 wants to merge 1 commit into
Closed
benjaminm95 wants to merge 1 commit into
benjaminm95 wants to merge 1 commit into
Conversation
Root cause of the 2026-05-19 wedge: `cron/scheduler.py:tick()`
held `.tick.lock` across the ENTIRE due-job execution, including
the synchronous LLM streaming inside each job. A single 1962 s
agent run (Telegram conversation that landed on a degraded
provider) blocked the lock continuously and starved four
back-to-back `*/5 * * * *` ticks. Forensic write-up in
docs/incidents/2026-05-19-scheduler-wedge.md.
Three fixes, one branch:
A. LLM stream deadlines (run_agent.py). New `StreamTimeoutError`
(subclass of TimeoutError so existing handlers still catch it)
raised on either of two breaches enforced by a daemon
watchdog inside `_call_chat_completions`:
- `HERMES_LLM_STREAM_TIMEOUT_SECONDS` (default 300 s) total
wall-clock budget — bounds the worst-case stream duration
independent of provider behaviour.
- `HERMES_LLM_STREAM_CHUNK_TIMEOUT_SECONDS` (default 60 s)
per-chunk silence budget — catches stalled-after-some-tokens
paths the existing httpx read timeout misses.
Set either to 0 to opt that specific deadline out.
Deviation from spec: spec asked for `asyncio.wait_for`, but the
OpenAI SDK stream path here is synchronous (`for chunk in
stream:`) and the cron caller already wraps it in a
ThreadPoolExecutor. The watchdog-thread pattern enforces the
same two-deadline guarantee in-place without rewriting the
whole sync chunk loop into async — much smaller blast radius.
B. Narrow tick-lock scope (cron/scheduler.py). Investigation
showed the lock has exactly one purpose (cron-fire single-
flight); no hidden Ollama-guard, shared-state, or
storage-serialization purpose to preserve. New
`_dispatch_lock()` context manager holds the lock only across
`get_due_jobs()` + `advance_next_run()` — job execution runs
outside the lock. Subsequent ticks during a long-running job
see an empty due-list (already advanced) and exit fast.
C. Bullet-proof lock release. `_dispatch_lock()` uses
`try/finally` with bare `except BaseException` on the release
path so `asyncio.CancelledError`, `KeyboardInterrupt`, and
`SystemExit` cannot bypass release and leave the lock stuck
(the 2026-05-19 wedge mode).
Tests (tests/scheduler/test_wedge_recovery.py, 7 added):
- total-timeout abort raises StreamTimeoutError(which="total")
- per-chunk-timeout abort raises StreamTimeoutError(which="chunk")
- env-var resolution falls back to default when unset / negative
- lock released during long-running stream: second tick fired
mid-flight from another thread acquires and dispatches its
own due-list
- lock released on KeyboardInterrupt mid-dispatch
- lock released on arbitrary RuntimeError mid-dispatch
Docs:
- docs/incidents/2026-05-19-scheduler-wedge.md (forensic +
lock-scope audit + fix plan)
- docs/todos/scheduler-followups.md (LaunchAgent supervision
with KeepAlive=Crashed-only + per-job stream timeout
overrides — both intentionally deferred)
- AGENTS.md cron-hardening section updated with new lock-scope
semantics + env vars (no CLAUDE.md in this repo)
Test impact: 325 scheduler-adjacent tests pass (200 of the
pre-existing 202 failures elsewhere are unrelated to this
patch — same set fails on `main`); my branch is strictly
+8 passes / -1 failure / -1 error vs main.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Closing — not maintaining a fork. Filing for awareness only; feel free to take any of the diagnosis or fix if useful. |
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.
Real-world reproduction
Reproduced twice in production on a Hermes gateway pointed at a
self-hosted Ollama server (see
docs/incidents/2026-05-19-scheduler-wedge.mdon this branch for the full forensic write-up):
at the time (filed as "Hermes scheduler wedge" in operator
notes —
.tick.lockheld after a cron-launched script waskilled mid-run, no new ticks until gateway restart).
(
time=1962.8s api_calls=3 response=1177 charsin the gatewaylog). A single Telegram chat triggered a long agent loop on a
cold-loaded LLM.
cron/scheduler.py:tick()held.tick.lockfor the entire 32 minutes; every cron tick thatfell in the window logged
"Tick skipped — another instance holds the lock"and returned 0. 4 consecutive*/5 * * * *catalysts ticks were starved (15:25 → 15:50 EDT) before the
operator manually removed the lock file.
The fix is below; details in the incident write-up. 5
regression tests cover the recovery paths:
StreamTimeoutError(which="total").StreamTimeoutError(which="chunk").fired mid-flight from another thread acquires + dispatches its
own due jobs (the exact pre-fix wedge mode).
KeyboardInterruptmid-dispatch.RuntimeErrormid-dispatch.2026-05-19 — Cron scheduler wedged for 32 minutes
Symptom
The algotrader operator running on this Hermes instance saw the
algotrader-catalysts-scanandalgotrader-catalysts-evaluatecrons (both
*/5 * * * *,no_agent=True) stop firing forroughly 25 minutes (15:25 → 15:50 EDT). During the same window:
inbound message (gateway log:
response ready: ... time=1962.8s api_calls=3 response=1177 chars).~/.hermes/cron/.tick.lockwas held continuously across thewhole window.
missed their schedule, but the operator-visible pain was
concentrated on the 5-minute catalysts because they were the
shortest-cadence jobs in the gap.
Manual
rm ~/.hermes/cron/.tick.lockfollowed by the next minuteboundary restored normal firing immediately.
Mechanism
cron/scheduler.py:tick()(the function the gateway calls every60 s from a background thread) acquires the file lock, then runs
every due job to completion inside the
try:block beforereleasing in
finally:(lines 1447–1590 of the file at incidenttime).
When at least one due job in a tick is an LLM-backed job (not
no_agent=True),run_job()callsagent.run_conversation(prompt)which internally streams fromthe model provider via
chat.completions.create(stream=True)anditerates synchronously over chunks (
run_agent.py:6907-6933).There is no enforced end-to-end deadline on that stream:
HERMES_STREAM_READ_TIMEOUT(default 120 s) is an httpx-levelper-chunk read timeout — it resets on every byte received, so a
stream that emits one token every 119 s never trips.
HERMES_STREAM_STALE_TIMEOUT(default 180 s) is a stale-streamdetector inside the chunk loop; same per-chunk semantics.
HERMES_CRON_TIMEOUT(default 600 s) is inactivity-based —it's reset by
_touch_activity("receiving stream response")on every chunk, so an active-but-slow stream keeps it pinned at
zero indefinitely.
max_iterations(default 90) is a tool-loop ceiling, not awall-clock ceiling — each iteration can itself be an
arbitrarily long stream.
So in pathological cases (provider degradation, retry storm with
keepalive pings, agent stuck in a tool/think/answer cycle that
makes progress slowly), a single LLM job can sit inside
tick()for minutes-to-hours while holding the file lock.
Every subsequent tick attempt during that window logs
"Tick skipped — another instance holds the lock"and returns 0without firing any cron — including unrelated
no_agentcronthat wouldn't touch the LLM at all.
Lock-scope audit
A repo-wide grep (
tick.lock | tick_lock | _get_lock_paths)shows only
cron/scheduler.pyand its tests touch the lock.The lock has exactly one purpose: prevent two concurrent
tick()calls from picking up the same due job twice (thegateway's background ticker and a standalone
python -m cron.schedulerdaemon running in parallel — explicitly calledout in the module docstring).
It does not serialize:
AIAgent, its own httpx client, its own credential pool).save_job_outputis fd-per-write,independent per
job_id).cron_session_id).asyncio.run_coroutine_threadsafe, no lock).Conclusion: the lock's purpose is single-flight on cron-job
dispatch, nothing else. There is no second purpose to preserve;
narrowing the lock to dispatch-only is safe.
Fix plan (this branch)
A. LLM stream deadline. Add two env vars —
HERMES_LLM_STREAM_TIMEOUT_SECONDS(default 300 s, totalwall-clock budget for one stream) and
HERMES_LLM_STREAM_CHUNK_TIMEOUT_SECONDS(default 60 s, maxgap between chunks). Both enforced inside the chunk loop with
a new
StreamTimeoutError. On breach: close the streamcleanly, raise, surface to the agent loop's existing retry /
error-propagation paths. Set either to
0to opt that specificdeadline out.
Implementation note: the OpenAI SDK stream path is synchronous
(
for chunk in stream:) and the cron caller already wraps it ina
ThreadPoolExecutor. The deadlines are enforced by a daemonwatchdog thread alongside the chunk loop — same end-to-end
guarantee
asyncio.wait_forwould give, in-place, withoutrewriting the entire sync chunk loop into async.
B. Narrow lock scope. Hold
.tick.lockonly acrossget_due_jobs()+advance_next_run()(the actual dispatchstep). Release before job execution. Subsequent ticks during
the LLM run land with an empty due-job list (already advanced)
and become a fast no-op. Single-flight semantics preserved
because
advance_next_run()happens under the lock.C. Guaranteed release. Wrap the lock acquisition in an
explicit
try/finallythat coversasyncio.CancelledError,KeyboardInterrupt, and arbitraryBaseException—the pre-fix
tryblock caught typedExceptiononly.Tests
5 regression tests in
tests/scheduler/test_wedge_recovery.py,plus 2 env-var resolution tests (7 total). Existing
tests/cron/test_scheduler.py(115 tests) still passes.Test invocation:
What this does NOT fix
host was not registered with launchd; no auto-respawn on
crash. Filed in
docs/todos/scheduler-followups.mdwith arecommended plist (KeepAlive=Crashed-only,
ThrottleInterval ≥ 60).
HERMES_LLM_STREAM_TIMEOUT_SECONDSis process-wide. Some long-form jobs (compliance check, deep
research) genuinely need >300 s. Also filed in
docs/todos/scheduler-followups.md.Files changed
cron/scheduler.py— new_dispatch_lock()context manager;tick()releases lock before job execution.run_agent.py— newStreamTimeoutError+ watchdog thread in_call_chat_completions.tests/scheduler/test_wedge_recovery.py(new) — 7 tests.AGENTS.md— cron-hardening section updated with newlock-scope semantics + env vars.
docs/incidents/2026-05-19-scheduler-wedge.md(new) — fullforensic write-up.
docs/todos/scheduler-followups.md(new) — LaunchAgentsupervision + per-job timeout overrides, both deferred.
🤖 Generated with Claude Code