Skip to content

fix(dashboard): offload cron profile scans - #45491

Closed
BlackishGreen33 wants to merge 3 commits into
NousResearch:mainfrom
BlackishGreen33:bg/cron-dashboard-offload
Closed

fix(dashboard): offload cron profile scans#45491
BlackishGreen33 wants to merge 3 commits into
NousResearch:mainfrom
BlackishGreen33:bg/cron-dashboard-offload

Conversation

@BlackishGreen33

Copy link
Copy Markdown
Contributor

What does this PR do?

Moves the dashboard cron job endpoints' profile and job file work out of the FastAPI event loop.

Before this change, endpoints like GET /api/cron/jobs?profile=all scanned profiles and read cron job files directly inside async def handlers. With many profiles, that synchronous work could block the backend event loop and delay unrelated HTTP or WebSocket traffic during desktop startup.

This PR keeps the same API behavior and response shapes, but runs the cron dashboard work in a worker thread with asyncio.to_thread().

Related Issue

Fixes #45072

Type of Change

  • Bug fix / performance fix

Changes Made

  • Added a small cron dashboard offload helper in hermes_cli/web_server.py.
  • Moved cron job list, lookup, create, update, pause, resume, trigger, delete, and run-history work behind synchronous helper functions that are called through the offload helper.
  • Added a regression test that checks both profile=all profile scanning and no-profile job lookup run outside the event-loop thread.

How to Test

scripts/run_tests.sh tests/hermes_cli/test_web_server_cron_profiles.py
/Users/blackishgreen03/workspace/hermes-agent/venv/bin/python -m py_compile hermes_cli/web_server.py tests/hermes_cli/test_web_server_cron_profiles.py

I also ran git diff --check.

Note: I tried a focused ruff check, but the shared local venv used by this checkout does not have ruff installed.

Checklist

Screenshots / Logs

N/A - backend-only change.

Copilot AI review requested due to automatic review settings June 13, 2026 09:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR moves cron dashboard operations (profile scanning and job I/O) off the FastAPI event loop by running them in a background thread, and adds a regression test to verify the offloading behavior.

Changes:

  • Refactors cron dashboard endpoints to delegate to sync helpers executed via a thread offload wrapper.
  • Introduces _run_cron_dashboard_io() and sync variants of the cron endpoints’ core logic.
  • Adds a test ensuring profile scanning and profile resolution do not run on the event-loop thread.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
hermes_cli/web_server.py Offloads cron dashboard I/O to a worker thread via a shared wrapper and sync helpers.
tests/hermes_cli/test_web_server_cron_profiles.py Adds a regression test to assert cron profile scanning happens off the event loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread hermes_cli/web_server.py Outdated
Comment thread hermes_cli/web_server.py Outdated
Comment thread tests/hermes_cli/test_web_server_cron_profiles.py Outdated
@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: clean review — no issues found.

Reviewed the diff (132 additions across hermes_cli/web_server.py + test file). Applied gateway/config checklist:

  • Thread safety: _run_cron_dashboard_io correctly rejects async callables and awaitable returns — prevents silent double-scheduling.
  • Sync wrapper correctness: Each _sync function preserves the original logic verbatim; the async @app handlers are thin wrappers.
  • Test coverage: test_cron_profile_scan_runs_off_event_loop verifies thread IDs differ from the event loop thread — good regression guard.
  • No dead code: The old HEADER_ACTION_LABEL_REVEAL constant (from fix(desktop): make right-sidebar refresh button always visible instead of hover-reveal #45450's adjacent change) is unrelated; this PR doesn't introduce dead variables.
  • Exception handling preserved: HTTPException raises inside _sync functions propagate correctly through run_in_threadpool.

Approach is sound — offloading blocking cron I/O to the threadpool prevents event loop stalls when the dashboard polls cron profiles across multiple Hermes profiles.

@alt-glitch alt-glitch added type/perf Performance improvement or optimization P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 13, 2026
@alt-glitch alt-glitch added the comp/dashboard Web dashboard / control panel UI (dashboard/, landing) label Jun 26, 2026
…load

# Conflicts:
#	hermes_cli/web_server.py
#	tests/hermes_cli/test_web_server_cron_profiles.py
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #60884 (rebase) — both your commits landed on main (346e567 + 49fa04a) with your authorship preserved. The cron dashboard endpoints now run their profile/job I/O in the threadpool. Thanks!

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 comp/cron Cron scheduler and job management comp/dashboard Web dashboard / control panel UI (dashboard/, landing) 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.

Dashboard backend blocks its event loop with synchronous work, starving HTTP/WS during desktop boot

5 participants