From 89dcab832783e40a91eaa3ac038f2fe2f8ac9dc1 Mon Sep 17 00:00:00 2001 From: NocGeek Date: Thu, 30 Apr 2026 23:15:31 +0000 Subject: [PATCH] fix: persist manual cron run results (#1372) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual WebUI cron runs previously called cron.scheduler.run_job(job) and then only cleared the in-memory running flag. That meant output could be dropped and job metadata like last_run_at / last_status was not updated after a manual run. This PR matches the scheduled cron path (cron/scheduler.py:1334-1364) exactly: - Save manual-run output via save_job_output - Mark manual runs complete via mark_job_run - Treat empty final_response as a soft failure with the same error string as the scheduled path - Record manual-run failures in job metadata via mark_job_run(False) - Keep _run_cron_tracked self-contained for worker-thread execution Includes 2 behavioral regression tests using monkeypatch.setitem on sys.modules to mock cron.scheduler.run_job + cron.jobs helpers — the right test pattern (exercises the real _run_cron_tracked code path). Split out from #1352 (the larger profile-aware-cron-panel PR that's on hold) per pre-release-review feedback. Self-contained, doesn't touch the held PR's profile-filtering scope. Co-authored-by: NocGeek --- api/routes.py | 22 +++++++- tests/test_cron_manual_run_persistence.py | 69 +++++++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 tests/test_cron_manual_run_persistence.py diff --git a/api/routes.py b/api/routes.py index a5e21a7c..03ed0c62 100644 --- a/api/routes.py +++ b/api/routes.py @@ -95,10 +95,28 @@ def _cron_output_content_window(text: str, limit: int = _CRON_OUTPUT_CONTENT_LIM def _run_cron_tracked(job): """Wrapper that tracks running state around cron.scheduler.run_job.""" from cron.scheduler import run_job # import here — runs inside a worker thread + from cron.jobs import mark_job_run, save_job_output + + job_id = job.get("id", "") try: - run_job(job) + success, output, final_response, error = run_job(job) + save_job_output(job_id, output) + + # Match the scheduled cron path: an apparently successful run with no + # final response should not leave the job looking healthy. + if success and not final_response: + success = False + error = "Agent completed but produced empty response (model error, timeout, or misconfiguration)" + + mark_job_run(job_id, success, error) + except Exception as e: + logger.exception("Manual cron run failed for job %s", job_id) + try: + mark_job_run(job_id, False, str(e)) + except Exception: + logger.debug("Failed to mark manual cron run failure for %s", job_id) finally: - _mark_cron_done(job.get("id", "")) + _mark_cron_done(job_id) _PROVIDER_ALIASES = { "claude": "anthropic", diff --git a/tests/test_cron_manual_run_persistence.py b/tests/test_cron_manual_run_persistence.py new file mode 100644 index 00000000..7c1c365e --- /dev/null +++ b/tests/test_cron_manual_run_persistence.py @@ -0,0 +1,69 @@ +"""Regression tests for manual WebUI cron runs.""" + +import sys +import types + + +def test_manual_cron_run_saves_output_and_marks_job(monkeypatch): + import api.routes as routes + + calls = [] + + cron_pkg = types.ModuleType("cron") + cron_pkg.__path__ = [] + + cron_jobs = types.ModuleType("cron.jobs") + 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_scheduler = types.ModuleType("cron.scheduler") + cron_scheduler.run_job = lambda job: (True, "manual output", "done", None) + + monkeypatch.setitem(sys.modules, "cron", cron_pkg) + monkeypatch.setitem(sys.modules, "cron.jobs", cron_jobs) + monkeypatch.setitem(sys.modules, "cron.scheduler", cron_scheduler) + + routes._mark_cron_running("job123") + routes._run_cron_tracked({"id": "job123"}) + + assert calls == [ + ("save", "job123", "manual output"), + ("mark", "job123", True, None), + ] + assert routes._is_cron_running("job123") == (False, 0.0) + + +def test_manual_cron_run_marks_empty_response_as_failure(monkeypatch): + import api.routes as routes + + calls = [] + + cron_pkg = types.ModuleType("cron") + cron_pkg.__path__ = [] + + cron_jobs = types.ModuleType("cron.jobs") + 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_scheduler = types.ModuleType("cron.scheduler") + cron_scheduler.run_job = lambda job: (True, "manual output", "", None) + + monkeypatch.setitem(sys.modules, "cron", cron_pkg) + monkeypatch.setitem(sys.modules, "cron.jobs", cron_jobs) + monkeypatch.setitem(sys.modules, "cron.scheduler", cron_scheduler) + + routes._mark_cron_running("job-empty") + routes._run_cron_tracked({"id": "job-empty"}) + + 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 routes._is_cron_running("job-empty") == (False, 0.0)