Skip to content

fix(cronjob): warn when builtin scheduler has no gateway - #87058

Closed
zhao0112 wants to merge 1 commit into
NousResearch:mainfrom
zhao0112:fix/issue-87033-gateway-not-running-warning
Closed

fix(cronjob): warn when builtin scheduler has no gateway#87058
zhao0112 wants to merge 1 commit into
NousResearch:mainfrom
zhao0112:fix/issue-87033-gateway-not-running-warning

Conversation

@zhao0112

Copy link
Copy Markdown
Contributor

Summary

  • Add a creation-time warning when cronjob jobs are created with the builtin scheduler but no gateway process is running.
  • Preserve existing local-delivery warning behavior and keep the warning optional/non-blocking.

Changes

  • tools/cronjob_tools.py
    • Added _gateway_not_running_warning() to detect resolve_cron_scheduler().name == "builtin" and no running gateway PID.
    • Appended this warning (when applicable) to the cronjob create response message alongside existing local-delivery notice.
    • Added debug logging on scheduler/pid resolution failures instead of swallowing silently.
  • tests/tools/test_cronjob_tools.py
    • Added regression tests covering:
      • builtin scheduler with no gateway emits warning,
      • non-builtin scheduler omits warning,
      • running gateway omits warning.

Test Plan

  • python3 -m py_compile tools/cronjob_tools.py tests/tools/test_cronjob_tools.py
  • python3 -m pytest tests/tools/test_cronjob_tools.py::TestLocalDeliveryNotice::test_builtin_scheduler_without_gateway_emits_warning tests/tools/test_cronjob_tools.py::TestLocalDeliveryNotice::test_non_builtin_scheduler_omits_gateway_warning tests/tools/test_cronjob_tools.py::TestLocalDeliveryNotice::test_running_gateway_omits_warning tests/tools/test_cronjob_tools.py::TestUnifiedCronjobTool::test_create_and_list -q

Closes #87033

@alt-glitch alt-glitch added type/feature New feature or request comp/tools Tool registry, model_tools, toolsets comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Aug 15, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #51790, the earlier open PR implementing the same builtin-scheduler/no-gateway creation warning and external-scheduler suppression.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(cronjob): warn when builtin scheduler has no gateway

  1. Emoji in tool output: the warning string uses (tools/cronjob_tools.py line 399). Tool JSON output can be rendered on plain-text gateway platforms (Telegram/SMS/email) where the emoji may garble or look broken; a plain [WARNING] prefix would be more robust across surfaces. Minor.
  2. Exception swallowing: both resolve_cron_scheduler() and find_gateway_pids() failures are logged at debug and silently downgraded to "no warning" (lines 382-392) — fine for an advisory path, but note that a broken find_gateway_pids (e.g. permission error reading /proc) would hide a real "jobs won't fire" condition. Acceptable given the warning is best-effort.
  3. Race with late gateway start: the warning fires at job creation time only; a user creating a job from the CLI while the gateway is down gets the warning even if they intend to start the gateway next. Advisory text is appropriate — no change needed, just confirming the message wording ("won't fire automatically") correctly avoids over-claiming.
  4. Tests cover all three branches (builtin+no gateway, non-builtin, gateway running) with a clean scheduler stub — good.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing as duplicate — merged via #88323 which salvages #73877 (the more complete implementation with a shared cron/scheduler_readiness.py module).

Your PR's approach was correct (detect builtin scheduler + no gateway → warn), and your tests covered the right branches. #73877 went further by extracting the logic into a shared module so both the CLI and the tool use the same code, avoiding future duplication. The salvage also adds a fix for the desktop ticker false positive (HERMES_DESKTOP=1 runs its own cron ticker without a gateway process).

Thank you for the contribution!

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 comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cronjob tool returns success when no gateway is running, so the agent reports a scheduled job that can never fire

4 participants