From e9b3b8e820b1bbd37362a5bd38fc5dbd374fb37a Mon Sep 17 00:00:00 2001
From: Billard <82095453+iacker@users.noreply.github.com>
Date: Sun, 12 Apr 2026 23:53:38 +0200
Subject: [PATCH] fix(cron): treat empty agent response as error in last_status
 (fixes #8585)

When a cron job's agent run completes but produces an empty final_response
(e.g. API 404 from invalid model name), the scheduler now marks last_status
as "error" instead of "ok", so the failure is visible in job listings.

Previously, any run that didn't raise an exception was marked "ok" regardless
of whether the agent actually produced output.
---
 cron/scheduler.py            |  7 ++++++
 tests/cron/test_scheduler.py | 41 ++++++++++++++++++++++++++++++++++--
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/cron/scheduler.py b/cron/scheduler.py
index 4bd5724a6..9a0f561b0 100644
--- a/cron/scheduler.py
+++ b/cron/scheduler.py
@@ -979,6 +979,13 @@ def tick(verbose: bool = True, adapters=None, loop=None) -> int:
                         delivery_error = str(de)
                         logger.error("Delivery failed for job %s: %s", job["id"], de)
 
+                # Treat empty final_response as a soft failure so last_status
+                # is not "ok" — the agent ran but produced nothing useful.
+                # (issue #8585)
+                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, delivery_error=delivery_error)
                 executed += 1
 
diff --git a/tests/cron/test_scheduler.py b/tests/cron/test_scheduler.py
index a1cc2e127..160b55efc 100644
--- a/tests/cron/test_scheduler.py
+++ b/tests/cron/test_scheduler.py
@@ -675,7 +675,7 @@ class TestRunJobSessionPersistence:
 
     def test_run_job_empty_response_returns_empty_not_placeholder(self, tmp_path):
         """Empty final_response should stay empty for delivery logic (issue #2234).
-        
+
         The placeholder '(No response generated)' should only appear in the
         output log, not in the returned final_response that's used for delivery.
         """
@@ -693,7 +693,7 @@ class TestRunJobSessionPersistence:
              patch(
                  "hermes_cli.runtime_provider.resolve_runtime_provider",
                  return_value={
-                     "api_key": "test-key",
+                     "api_key": "***",
                      "base_url": "https://example.invalid/v1",
                      "provider": "openrouter",
                      "api_mode": "chat_completions",
@@ -714,6 +714,43 @@ class TestRunJobSessionPersistence:
         # But the output log should show the placeholder
         assert "(No response generated)" in output
 
+    def test_tick_marks_empty_response_as_error(self, tmp_path):
+        """When run_job returns success=True but final_response is empty,
+        tick() should mark the job as error so last_status != 'ok'.
+        (issue #8585)
+        """
+        from cron.scheduler import tick
+        from cron.jobs import load_jobs, save_jobs
+
+        job = {
+            "id": "empty-job",
+            "name": "empty-test",
+            "prompt": "do something",
+            "schedule": "every 1h",
+            "enabled": True,
+            "next_run_at": "2020-01-01T00:00:00",
+            "deliver": "local",
+            "last_status": None,
+        }
+
+        fake_db = MagicMock()
+
+        with patch("cron.scheduler._hermes_home", tmp_path), \
+             patch("cron.scheduler.get_due_jobs", return_value=[job]), \
+             patch("cron.scheduler.advance_next_run"), \
+             patch("cron.scheduler.mark_job_run") as mock_mark, \
+             patch("cron.scheduler.save_job_output", return_value="/tmp/out.md"), \
+             patch("cron.scheduler._resolve_origin", return_value=None), \
+             patch("cron.scheduler.run_job", return_value=(True, "output", "", None)):
+            tick(verbose=False)
+
+        # Should be called with success=False because final_response is empty
+        mock_mark.assert_called_once()
+        call_args = mock_mark.call_args
+        assert call_args[0][0] == "empty-job"
+        assert call_args[0][1] is False  # success should be False
+        assert "empty" in call_args[0][2].lower()  # error should mention empty
+
     def test_run_job_sets_auto_delivery_env_from_dotenv_home_channel(self, tmp_path, monkeypatch):
         job = {
             "id": "test-job",