fix: deliver manual cron run results
This commit is contained in:
1 parent
603183a301
commit
00295a916a
3 files changed
+133
-23
No files matched your search
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- **PR #2452** by @Michaelyklam (fixes #2451) — Manual WebUI cron triggers now deliver the same final response or failure notice as scheduled cron runs, while still saving output files and recording delivery errors separately from job execution failures.
|
||||
|
||||
## [v0.51.82] — 2026-05-17 — Release BF (stage-375 — 2-PR batch — table renderer pipe protection + Catppuccin appearance skin)
|
||||
|
||||
### Added
|
||||
|
||||
+35
-3
@@ -754,8 +754,15 @@ def _run_cron_tracked(job, profile_home=None, execution_profile_home=None):
|
||||
agent config/.env while running. When no job profile is selected, both homes
|
||||
are the same and legacy server-default behavior is preserved.
|
||||
"""
|
||||
import importlib
|
||||
|
||||
from cron.jobs import mark_job_run, save_job_output
|
||||
|
||||
_cron_scheduler = importlib.import_module("cron.scheduler")
|
||||
|
||||
_silent_marker = getattr(_cron_scheduler, "SILENT_MARKER", "[SILENT]")
|
||||
_deliver_result = getattr(_cron_scheduler, "_deliver_result", None)
|
||||
|
||||
job_id = job.get("id", "")
|
||||
execution_profile_home = execution_profile_home or profile_home
|
||||
|
||||
@@ -772,11 +779,29 @@ def _run_cron_tracked(job, profile_home=None, execution_profile_home=None):
|
||||
job, execution_profile_home
|
||||
)
|
||||
|
||||
# Persist output and run metadata back to the job's owning cron store,
|
||||
# even when the selected execution profile is different.
|
||||
# Persist output, deliver the same content the scheduled cron path would
|
||||
# send, and write run metadata back to the job's owning cron store even
|
||||
# when the selected execution profile is different.
|
||||
def _persist_success():
|
||||
save_job_output(job_id, output)
|
||||
|
||||
deliver_content = (
|
||||
final_response
|
||||
if success
|
||||
else f"⚠️ Cron job '{job.get('name', job_id)}' failed:\n{error}"
|
||||
)
|
||||
should_deliver = bool(deliver_content)
|
||||
if should_deliver and success and _silent_marker in deliver_content.strip().upper():
|
||||
should_deliver = False
|
||||
|
||||
delivery_error = None
|
||||
if should_deliver and _deliver_result is not None:
|
||||
try:
|
||||
delivery_error = _deliver_result(job, deliver_content)
|
||||
except Exception as de:
|
||||
delivery_error = str(de)
|
||||
logger.error("Delivery failed for manual cron job %s: %s", job_id, de)
|
||||
|
||||
# Match the scheduled cron path: an apparently successful run with no
|
||||
# final response should not leave the job looking healthy.
|
||||
_success, _error = success, error
|
||||
@@ -784,7 +809,14 @@ def _run_cron_tracked(job, profile_home=None, execution_profile_home=None):
|
||||
_success = False
|
||||
_error = "Agent completed but produced empty response (model error, timeout, or misconfiguration)"
|
||||
|
||||
mark_job_run(job_id, _success, _error)
|
||||
try:
|
||||
mark_job_run(job_id, _success, _error, delivery_error=delivery_error)
|
||||
except TypeError:
|
||||
# Older/fake cron.jobs modules used by focused WebUI tests may
|
||||
# not expose the newer delivery_error parameter. Real Hermes
|
||||
# scheduler builds do, so this is only a compatibility shim for
|
||||
# legacy test doubles and deployments.
|
||||
mark_job_run(job_id, _success, _error)
|
||||
|
||||
_with_cron_home(profile_home, _persist_success)
|
||||
except Exception as e:
|
||||
|
||||
@@ -1,21 +1,32 @@
|
||||
"""Regression tests for manual WebUI cron runs."""
|
||||
|
||||
|
||||
|
||||
def test_manual_cron_run_saves_output_and_marks_job(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
calls = []
|
||||
|
||||
def _install_cron_fakes(monkeypatch, calls, deliver_result=None, silent_marker="[SILENT]"):
|
||||
cron_jobs = type("CronJobs", (), {})()
|
||||
cron_jobs.save_job_output = lambda job_id, output: calls.append(
|
||||
("save", job_id, output)
|
||||
)
|
||||
cron_jobs.mark_job_run = lambda job_id, success, error=None: calls.append(
|
||||
("mark", job_id, success, error)
|
||||
cron_jobs.mark_job_run = lambda job_id, success, error=None, delivery_error=None: calls.append(
|
||||
("mark", job_id, success, error, delivery_error)
|
||||
)
|
||||
|
||||
cron_scheduler = type("CronScheduler", (), {})()
|
||||
cron_scheduler.SILENT_MARKER = silent_marker
|
||||
if deliver_result is None:
|
||||
deliver_result = lambda job, content: calls.append(
|
||||
("deliver", job["id"], content)
|
||||
) or None
|
||||
cron_scheduler._deliver_result = deliver_result
|
||||
|
||||
monkeypatch.setitem(__import__("sys").modules, "cron.jobs", cron_jobs)
|
||||
monkeypatch.setitem(__import__("sys").modules, "cron.scheduler", cron_scheduler)
|
||||
|
||||
|
||||
def test_manual_cron_run_saves_output_delivers_and_marks_job(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
calls = []
|
||||
_install_cron_fakes(monkeypatch, calls)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_run_cron_job_in_profile_subprocess",
|
||||
@@ -27,25 +38,17 @@ def test_manual_cron_run_saves_output_and_marks_job(monkeypatch):
|
||||
|
||||
assert calls == [
|
||||
("save", "job123", "manual output"),
|
||||
("mark", "job123", True, None),
|
||||
("deliver", "job123", "done"),
|
||||
("mark", "job123", True, None, None),
|
||||
]
|
||||
assert routes._is_cron_running("job123") == (False, 0.0)
|
||||
|
||||
|
||||
def test_manual_cron_run_marks_empty_response_as_failure(monkeypatch):
|
||||
def test_manual_cron_run_marks_empty_response_as_failure_without_delivery(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
calls = []
|
||||
|
||||
cron_jobs = type("CronJobs", (), {})()
|
||||
cron_jobs.save_job_output = lambda job_id, output: calls.append(
|
||||
("save", job_id, output)
|
||||
)
|
||||
cron_jobs.mark_job_run = lambda job_id, success, error=None: calls.append(
|
||||
("mark", job_id, success, error)
|
||||
)
|
||||
|
||||
monkeypatch.setitem(__import__("sys").modules, "cron.jobs", cron_jobs)
|
||||
_install_cron_fakes(monkeypatch, calls)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_run_cron_job_in_profile_subprocess",
|
||||
@@ -58,4 +61,75 @@ def test_manual_cron_run_marks_empty_response_as_failure(monkeypatch):
|
||||
assert calls[0] == ("save", "job-empty", "manual output")
|
||||
assert calls[1][0:3] == ("mark", "job-empty", False)
|
||||
assert "empty response" in calls[1][3]
|
||||
assert calls[1][4] is None
|
||||
assert routes._is_cron_running("job-empty") == (False, 0.0)
|
||||
|
||||
|
||||
def test_manual_cron_run_records_delivery_errors_separately(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
calls = []
|
||||
|
||||
def fail_delivery(job, content):
|
||||
calls.append(("deliver", job["id"], content))
|
||||
return "discord not configured"
|
||||
|
||||
_install_cron_fakes(monkeypatch, calls, deliver_result=fail_delivery)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_run_cron_job_in_profile_subprocess",
|
||||
lambda job, execution_profile_home: (True, "manual output", "done", None),
|
||||
)
|
||||
|
||||
routes._mark_cron_running("job-delivery-error")
|
||||
routes._run_cron_tracked({"id": "job-delivery-error"})
|
||||
|
||||
assert calls == [
|
||||
("save", "job-delivery-error", "manual output"),
|
||||
("deliver", "job-delivery-error", "done"),
|
||||
("mark", "job-delivery-error", True, None, "discord not configured"),
|
||||
]
|
||||
assert routes._is_cron_running("job-delivery-error") == (False, 0.0)
|
||||
|
||||
|
||||
def test_manual_cron_run_skips_silent_success_delivery(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
calls = []
|
||||
_install_cron_fakes(monkeypatch, calls)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_run_cron_job_in_profile_subprocess",
|
||||
lambda job, execution_profile_home: (True, "manual output", "[SILENT]", None),
|
||||
)
|
||||
|
||||
routes._mark_cron_running("job-silent")
|
||||
routes._run_cron_tracked({"id": "job-silent"})
|
||||
|
||||
assert calls == [
|
||||
("save", "job-silent", "manual output"),
|
||||
("mark", "job-silent", True, None, None),
|
||||
]
|
||||
assert routes._is_cron_running("job-silent") == (False, 0.0)
|
||||
|
||||
|
||||
def test_manual_cron_run_delivers_failure_notice(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
calls = []
|
||||
_install_cron_fakes(monkeypatch, calls)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"_run_cron_job_in_profile_subprocess",
|
||||
lambda job, execution_profile_home: (False, "manual output", "", "boom"),
|
||||
)
|
||||
|
||||
routes._mark_cron_running("job-failed")
|
||||
routes._run_cron_tracked({"id": "job-failed", "name": "Nightly check"})
|
||||
|
||||
assert calls[0] == ("save", "job-failed", "manual output")
|
||||
assert calls[1][0:2] == ("deliver", "job-failed")
|
||||
assert "Nightly check" in calls[1][2]
|
||||
assert "boom" in calls[1][2]
|
||||
assert calls[2] == ("mark", "job-failed", False, "boom", None)
|
||||
assert routes._is_cron_running("job-failed") == (False, 0.0)
|
||||
Reference in new issue
Block a user