Skip to content

fix(server): offload cron job listing from dashboard event loop - #50979

Closed
PINKIIILQWQ wants to merge 1 commit into
NousResearch:mainfrom
PINKIIILQWQ:fix/45072-cron-event-loop-offload
Closed

fix(server): offload cron job listing from dashboard event loop#50979
PINKIIILQWQ wants to merge 1 commit into
NousResearch:mainfrom
PINKIIILQWQ:fix/45072-cron-event-loop-offload

Conversation

@PINKIIILQWQ

@PINKIIILQWQ PINKIIILQWQ commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Summary

GET /api/cron/jobs is an async dashboard endpoint, but it performs synchronous profile enumeration and per-profile cron file I/O directly on the event loop thread.

If those filesystem operations become slow, the dashboard event loop is blocked for the duration of the sync call, starving other HTTP requests and WebSocket traffic.

This PR offloads only the synchronous cron/profile work to asyncio.to_thread(...):

  • _cron_profile_dicts() — profile enumeration
  • _call_cron_for_profile(...) — per-profile cron module calls, for both the single-profile and all-profiles paths

The handler shape, response format, profile parameter behavior, error handling, and per-profile sequential processing order are preserved.

Related Issue

Relates to #45072.

This addresses only the /api/cron/jobs portion of the broader event-loop blocking issue.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)
  • ✨ New feature
  • 🔒 Security fix
  • 📝 Documentation update
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill

Changes Made

  • hermes_cli/web_server.py

    • Wrapped _cron_profile_dicts() and both _call_cron_for_profile(...) calls in list_cron_jobs() with await asyncio.to_thread(...).
    • Only the synchronous I/O-bound calls are offloaded; the handler loop, response assembly, and parameter handling stay on the event loop.
    • Per-profile processing remains sequential.
  • tests/hermes_cli/test_web_server_cron_profiles.py

    • Added a regression test that monkeypatches slow synchronous cron/profile functions and verifies a concurrent heartbeat continues ticking.
    • Added a test verifying exceptions from the thread-pool calls preserve the existing behavior.

How to Test

Targeted tests:

pytest tests/hermes_cli/test_web_server_cron_profiles.py -v
pytest tests/hermes_cli/test_web_server.py -k "cron"
pytest tests/hermes_cli/test_cron*.py -q

Results:

  • tests/hermes_cli/test_web_server_cron_profiles.py: 9 passed
  • tests/hermes_cli/test_web_server.py -k "cron": 6 passed
  • tests/hermes_cli/test_cron*.py -q: 19 passed

Broader filtered run (pytest tests -q -k "cron or web_server") has pre-existing
failures on origin/main, unrelated to this change:

  • tests/cron/test_scheduler.py::test_all_token_case_insensitive
  • tests/hermes_cli/test_web_server.py::TestPtyWebSocket (3 tests)

The cron/event-loop tests added by this PR pass.

Regression Proof

Before this change, the slow-sync regression test failed: a concurrent 10ms
heartbeat received 0 ticks while list_cron_jobs() was running with simulated
synchronous I/O.

After this change, the heartbeat continues ticking while the cron/profile work
runs in the thread pool.

Checklist

Code

  • I've read the Contributing Guide

  • My commit message follows Conventional Commits (fix(server): offload cron job listing from event loop)

  • I searched for existing PRs to make sure this isn't a duplicate

  • My PR contains only changes related to this fix

  • I've run pytest tests/ -q and all tests pass

    Not checked: broader filtered runs have pre-existing failures on origin/main, documented above.

  • I've added tests for my changes

  • I've tested on my platform: macOS, Apple Silicon, Python 3.11.14

Documentation & Housekeeping

  • N/A — No documentation changes needed
  • N/A — No config key changes
  • N/A — No architecture or workflow changes
  • N/A — No tool behavior changes

@alt-glitch alt-glitch added type/perf Performance improvement or optimization comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jun 22, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #45491 — same /api/cron/jobs event-loop offload (wrapping _cron_profile_dicts()/_call_cron_for_profile() in asyncio.to_thread) for #45072. #45491 is the earliest open PR for this scope; #50948 also covers the /api/cron/jobs portion. (Note: #50978 was this author's closed twin.)

@PINKIIILQWQ

Copy link
Copy Markdown
Contributor Author

Thanks for the triage — I missed #45491 and #50948.

I'll close this PR as a duplicate of #45491. The only potentially useful extra here is the regression test that reproduces the event-loop starvation with slow synchronous cron/profile I/O; if helpful, I'm happy to port that test or leave the reproduction notes on #45491.

Thanks for pointing me to the earlier PRs.

@PINKIIILQWQ

Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #45491.

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants