fix(cron): warn at create time when gateway/ticker is not running (salvage of #2793 by @ygd58)

[Bartok9]

Summary

Salvages #2793 by @ygd58 onto current main, with regression tests.

Warn the user at cron-job create time when the gateway (and therefore the in-process cron ticker) is not running, so a CLI-only schedule does not silently never fire.

Motivation

Cron jobs only advance next_run_at and execute while the gateway's in-process ticker is running. A job created from a CLI-only session with no gateway up is saved but never executes until hermes cron tick is run manually. The user gets no signal — they believe the schedule is live. This is the "I scheduled it but nothing happened" trap.

What #2793 added

A _check_cron_ticker_warning() helper that checks gateway.status.get_running_pid() and, when no gateway is running, returns an actionable warning attached to the create response.

Why it needed salvage

The original PR (opened 2026-03-24) patched a now-stale layout of tools/cronjob_tools.py — the file has since been refactored (the create branch now builds a _create_message string and already appends a _local_delivery_notice). A plain rebase/cherry-pick no longer applies. Re-implemented the same behavior against current main:

• _check_cron_ticker_warning() re-added, hardened to be best-effort (any exception → None, never blocks creation).
• Wired into the existing _create_message assembly, alongside (and distinct from) _local_delivery_notice — that notice flags non-delivery; this one flags a stopped ticker.

Changes from original

• Re-targeted onto the refactored create path (append to _create_message instead of adding a separate "warning" JSON key).
• Added 3 regression tests (original PR shipped none).

Verification

• python3 -m pytest tests/tools/test_cronjob_tools.py -q — 70 passed (3 new TestCronTickerWarning cases).

Real behavior proof

Captured from a real run on this branch:

NOT running -> ⚠️ The gateway is not running — this cron job will NOT fire automatically. Start it with 'hermes gateway run', or trigger jobs manually with 'hermes cron tick'.
running     -> None

The new tests assert exactly this: gateway down → warning containing "gateway is not running" + "hermes cron tick"; gateway up → None; status probe raises → None (creation never blocked).

Credit to @ygd58 for the original fix (#2793).

[ygd58]

Thanks @Bartok9 for picking this up and rebasing onto current main! The integration with the existing _local_delivery_notice pattern and the 3 regression tests (including the best-effort status-error path) are exactly right. Happy for this to land as the active PR -- feel free to close #2793 in favor of this one.

[Bartok9]

Appreciate it @ygd58 🙏 Closing #2793 in favor of this one as you suggested. Ready for maintainer review — original design credit is yours.

[alt-glitch]

This was generated by AI during triage.

Salvage of #2793 (@ygd58). Note this warns in the cronjob agent-tool create path (tools/cronjob_tools.py), which is distinct from the already-merged #51696 that added the same warning to the hermes cron create CLI path (hermes_cli/cron.py). Related to #51021 (jobs-never-fire).

[Bartok9]

Exactly right @alt-glitch — confirmed the two are complementary, not duplicative:

• feat(cron): warn when gateway not running on cron create/list #51696 (merged): hermes_cli/cron.py — covers the hermes cron create CLI path.
• This PR: tools/cronjob_tools.py — covers the cronjob agent-tool create path.

Both surface the same gateway-down signal so neither entry point leaves a user with a silently-dead schedule (the #51021 trap). The agent-tool path was still uncovered before this. Tests are scoped to tools/cronjob_tools.py only, no overlap with the CLI suite. Ready for review.

[Bartok9]

Plate-clear 2026-07-11: Clean re-apply of _check_cron_ticker_warning + create-time append + 3 unit tests onto current main (dropped massive branch drift).

[teknium1]

Thanks for preserving the missing agent-tool warning path and adding focused regression coverage.

Problems

• tools/cronjob_tools.py:323 treats every absent gateway PID as “will NOT fire.” That is false for non-builtin providers: current main deliberately suppresses this warning for Chronos, whose managed scheduler fires jobs while the gateway is scaled to zero (hermes_cli/cron.py:66-89; tests/hermes_cli/test_cron.py:256-282; commit 0943e2a2720fd2b7eb5aee19c3bad0d495e5450a).
• The new tests cover the helper only, not the create response or that external-provider contract.

Suggested changes

• Gate this helper on the resolved scheduler being builtin before checking get_running_pid().
• Add create-path tests for builtin/no-gateway warning and non-builtin/no-gateway silence.

Automated hermes-sweeper review.

[teknium1]

This must first confirm that the resolved scheduler is builtin. Current main deliberately permits Chronos to fire jobs with no running gateway PID; see hermes_cli/cron.py:66-89 and its Chronos regression test.

[Bartok9]

Thanks @teknium1 — fixed in 42f1e8e.

• _check_cron_ticker_warning now resolves the active cron scheduler first and returns None for any non-builtin provider (Chronos et al.), matching the CLI contract in hermes_cli/cron.py:66-89. The get_running_pid() gateway-PID heuristic only speaks to the built-in ticker, so we no longer false-alarm when a managed external scheduler fires jobs with the gateway scaled to zero.
• Any failure resolving the scheduler falls back to builtin so the historical best-effort behavior is preserved.
• Added two regression tests: builtin + no-gateway warns; non-builtin + no-gateway stays silent. Existing best-effort/status-error tests updated to pin the builtin path. All 4 ticker tests green.

Ready for re-review.

[Bartok9]

Rebuilt onto latest main via patch re-apply (force-push). Please re-run CI.

— Bartok9 public PR hygiene 2026-08-01

[ygd58]

Since this salvages my original issue (#2793), I put together the direct create-response regression coverage the keep_open review requested and verified it against your exact branch. I don't have push access to your fork, so posting it here as a patch you can apply directly:

diff --git a/tests/tools/test_cronjob_tools.py b/tests/tools/test_cronjob_tools.py
index 6d063b541..d745b8725 100644
--- a/tests/tools/test_cronjob_tools.py
+++ b/tests/tools/test_cronjob_tools.py
@@ -2,6 +2,7 @@
 
 import json
 import pytest
+from unittest.mock import patch
 
 from tools.cronjob_tools import (
     _scan_cron_prompt,
@@ -240,6 +241,66 @@ class TestUnifiedCronjobTool:
         assert listing["jobs"][0]["name"] == "Server Check"
         assert listing["jobs"][0]["state"] == "scheduled"
 
+    def test_create_warns_when_gateway_not_running(self):
+        """Direct create-response regression (requested in the keep_open
+        review on #51790): with the default builtin scheduler and no
+        gateway running, the create response's own "message" field must
+        surface the ticker warning -- not just the underlying helper
+        function in isolation."""
+        with (
+            patch(
+                "cron.scheduler_provider.resolve_cron_scheduler",
+                side_effect=Exception("no external scheduler configured"),
+            ),
+            patch("gateway.status.get_running_pid", return_value=None),
+        ):
+            created = json.loads(
+                cronjob(action="create", prompt="Check", schedule="every 1h")
+            )
+
+        assert created["success"] is True
+        assert "gateway is not running" in created["message"].lower(), created["message"]
+        assert "will NOT fire automatically" in created["message"]
+
+    def test_create_does_not_warn_when_gateway_running(self):
+        """Sanity: the warning must not appear when the gateway IS
+        running (get_running_pid returns a real pid)."""
+        with (
+            patch(
+                "cron.scheduler_provider.resolve_cron_scheduler",
+                side_effect=Exception("no external scheduler configured"),
+            ),
+            patch("gateway.status.get_running_pid", return_value=12345),
+        ):
+            created = json.loads(
+                cronjob(action="create", prompt="Check", schedule="every 1h")
+            )
+
+        assert created["success"] is True
+        assert "gateway is not running" not in created["message"].lower(), created["message"]
+
+    def test_create_does_not_warn_for_non_builtin_scheduler(self):
+        """Direct create-response regression for provider-awareness: an
+        external scheduler (e.g. Chronos) fires jobs via its own managed
+        process while the gateway is legitimately scaled to zero, so a
+        missing gateway PID must NOT trigger the warning for a
+        non-builtin scheduler -- that would be a false alarm."""
+        from types import SimpleNamespace
+
+        with (
+            patch(
+                "cron.scheduler_provider.resolve_cron_scheduler",
+                return_value=SimpleNamespace(name="chronos"),
+            ),
+            patch("gateway.status.get_running_pid", return_value=None),
+        ):
+            created = json.loads(
+                cronjob(action="create", prompt="Check", schedule="every 1h")
+            )
+
+        assert created["success"] is True
+        assert "gateway is not running" not in created["message"].lower(), created["message"]
+
     def test_list_handles_partial_legacy_job_records(self):
         from cron.jobs import save_jobs
 

Verified against your current head: 3/3 new tests pass, 71/71 across the full tests/tools/test_cronjob_tools.py file (no regression). This covers the create-response warning appearing, its absence when the gateway is running, and its suppression for a non-builtin scheduler -- directly closing the "direct create-path regressions" gap the review flagged.

[Bartok9]

Thanks @ygd58 — applied the direct create-response regressions (plus restored helper-level ticker coverage that dropped in the last main rebuild).

Pushed on this branch:

• test_create_warns_when_gateway_not_running
• test_create_does_not_warn_when_gateway_running
• test_create_does_not_warn_for_non_builtin_scheduler
• TestCronTickerWarning (builtin warn / running silent / non-builtin silent / status-error best-effort)

Local: python3 -m pytest tests/tools/test_cronjob_tools.py -q → 75 passed.

This closes the remaining keep_open item from @teknium1 (create-path + external-provider contract, not just the helper). Ready for re-review.

[kshitijk4poor]

Closing as duplicate — merged via #88323 which salvages #73877 (shared cron/scheduler_readiness.py module). Your PR's approach using get_running_pid() (cheaper than find_gateway_pids()) and separate TestCronTickerWarning test class were noted as improvements. The salvage uses the shared module approach from #73877 which eliminates the duplication between CLI and tool paths. Thank you for the contribution!

[Bartok9]

Thanks @kshitijk4poor — appreciated the note on get_running_pid() / TestCronTickerWarning. Glad the shared cron/scheduler_readiness.py path landed via #88323 (salvage of #73877). Closing the loop here.