Skip to content

Fix invalid HERMES_CRON_TIMEOUT fallback in cron jobs - #11375

Closed
cola-runner wants to merge 3 commits into
NousResearch:mainfrom
cola-runner:codex/fix-11319-cron-timeout-parse
Closed

cola-runner wants to merge 3 commits into
NousResearch:mainfrom
cola-runner:codex/fix-11319-cron-timeout-parse

Conversation

@cola-runner

Copy link
Copy Markdown
Contributor

Summary

  • parse HERMES_CRON_TIMEOUT defensively instead of calling float(...) inline in run_job()
  • keep 0 as the unlimited sentinel, but fall back to the default inactivity timeout when the env var is malformed
  • add regression coverage for valid, unlimited, and invalid timeout values via the shared scheduler helper

Closes #11319.

Testing

  • source venv/bin/activate && python -m pytest tests/cron/test_cron_inactivity_timeout.py tests/cron/test_scheduler.py -q
  • source venv/bin/activate && python -m pytest tests/ -q (environment still has existing unrelated failures/errors in ACP, web server optional deps, transcription optional deps, and several pre-existing gateway/run_agent tests; the new cron regression is not among them)

@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 Apr 25, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Competing PRs for #11319: see also #11365 and #11328. Maintainer should pick one.

cola-runner and others added 3 commits April 26, 2026 00:11
Drop the redundant negative-value warning branch in
_get_cron_inactivity_limit().  The original (pre-fix) code mapped any
non-positive value to None (unlimited); restoring that behaviour
collapses the helper to one warning path and one return.

Tighten the except to ValueError (the only thing float() can raise on
string input) and add regression tests for negative-as-unlimited and
whitespace-as-default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cola-runner
cola-runner force-pushed the codex/fix-11319-cron-timeout-parse branch from 7ff6724 to 1c21a07 Compare April 25, 2026 16:13
@cola-runner

Copy link
Copy Markdown
Contributor Author

Rebased on main and tightened the helper:

  • Dropped the redundant negative-value warning branch — restored the original semantics where any non-positive `HERMES_CRON_TIMEOUT` is unlimited (matching the pre-fix `if _cron_timeout > 0 else None`).
  • Narrowed `except Exception` → `except ValueError` (the only thing `float()` can raise on string input).
  • Added regression tests for negative-as-unlimited and whitespace-as-default.

Net: 22 insertions / 19 deletions — smaller than the prior version and one fewer warning path.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the fix! Closing as a duplicate — #11365 (from @yeyitech, submitted ~45 minutes earlier) fixes the same issue with the same defensive parse pattern. Salvaged it onto current main in PR #17515, now merged:

#17515

Both of you independently identified and fixed the same bug the same way; credit to both. Really appreciate you taking the time to write the regression coverage.

@teknium1 teknium1 closed this Apr 29, 2026
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.

[Bug]: invalid HERMES_CRON_TIMEOUT makes run_job fail with ValueError instead of falling back

3 participants