diff --git a/cron/scheduler.py b/cron/scheduler.py index 4bd5724a60..9a0f561b05 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 a1cc2e1277..160b55efc6 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",