Cron job status misreported on agent failures

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #11044 fix(cron): treat empty agent response as error in last_status

  merged to main

  files: cron/scheduler.py, scripts/release.py, tests/cron/test_scheduler.py

  Show the diff

  • 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
     

  • scripts/release.py

    @@ -180,6 +180,7 @@
         "[redacted:email]": "lesterli",
         "[redacted:email]": "MagicRay1217",
         "[redacted:email]": "GenKoKo",
    +    "[redacted:email]": "iacker",
         "[redacted:email]": "spniyant",
         "[redacted:email]": "olafthiele",
         "[redacted:email]": "sprmn24",

  • tests/cron/test_scheduler.py

    @@ -675,7 +675,7 @@ def test_run_job_passes_session_db_and_cron_platform(self, tmp_path):
     
         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 @@ def test_run_job_empty_response_returns_empty_not_placeholder(self, tmp_path):
                  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 @@ def test_run_job_empty_response_returns_empty_not_placeholder(self, tmp_path):
             # 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",

• #17859 fix(cron): surface agent run_conversation failure flags as job failure

  merged to main

  files: cron/scheduler.py, tests/cron/test_scheduler.py

  Show the diff

  • cron/scheduler.py

    @@ -1150,6 +1150,21 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
                     f"agent.run_conversation returned {type(result).__name__} instead of dict: {result!r}"
                 )
     
    +        # If the agent itself reported failure (e.g. all retries exhausted on
    +        # API errors, model abort, mid-run interrupt), do not silently mark the
    +        # job as successful. run_agent populates `failed=True`/`completed=False`
    +        # on these paths and may put the error into `final_response`, which
    +        # would otherwise be delivered as if it were the agent's reply and the
    +        # job's `last_status` set to "ok". Raise so the except handler below
    +        # builds the proper failure tuple. (issue #17855)
    +        if result.get("failed") is True or result.get("completed") is False:
    +            _err_text = (
    +                result.get("error")
    +                or (result.get("final_response") or "").strip()
    +                or "agent reported failure"
    +            )
    +            raise RuntimeError(_err_text)
    +
             final_response = result.get("final_response", "") or ""
             # Strip leaked placeholder text that upstream may inject on empty completions.
             if final_response.strip() == "(No response generated)":

  • tests/cron/test_scheduler.py

    @@ -935,6 +935,120 @@ def test_run_job_empty_response_returns_empty_not_placeholder(self, tmp_path):
             # But the output log should show the placeholder
             assert "(No response generated)" in output
     
    +    @pytest.mark.parametrize(
    +        "agent_result,expected_err_substring",
    +        [
    +            (
    +                {
    +                    "final_response": "API call failed after 3 retries: Request timed out.",
    +                    "failed": True,
    +                    "completed": False,
    +                    "error": "API call failed after 3 retries: Request timed out.",
    +                },
    +                "API call failed",
    +            ),
    +            (
    +                {"final_response": None, "completed": False, "failed": True},
    +                "agent reported failure",
    +            ),
    +            (
    +                {"final_response": "", "completed": False},
    +                "agent reported failure",
    +            ),
    +            (
    +                {
    +                    "final_response": "partial reply before crash",
    +                    "failed": True,
    +                    "completed": False,
    +                    "error": "model abort: connection reset",
    +                },
    +                "model abort",
    +            ),
    +        ],
    +    )
    +    def test_run_job_treats_agent_failure_flag_as_failure(
    +        self, tmp_path, agent_result, expected_err_substring
    +    ):
    +        """Issue #17855: run_conversation returns ``failed=True``/``completed=False``
    +        when the agent's API call exhausts retries or aborts mid-run. run_job
    +        must surface this as success=False so cron's last_status reflects the
    +        failure and the user gets an error notification, instead of treating
    +        the (often non-empty) error string in final_response as a legitimate
    +        agent reply.
    +        """
    +        job = {
    +            "id": "failing-api-job",
    +            "name": "failing api",
    +            "prompt": "do something",
    +        }
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch(
    +                 "hermes_cli.runtime_provider.resolve_runtime_provider",
    +                 return_value={
    +                     "api_key": "***",
    +                     "base_url": "https://example.invalid/v1",
    +                     "provider": "openrouter",
    +                     "api_mode": "chat_completions",
    +                 },
    +             ), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = agent_result
    +            mock_agent_cls.return_value = mock_agent
    +
    +            success, output, final_response, error = run_job(job)
    +
    +        assert success is False
    +        assert final_response == ""
    +        assert error is not None and expected_err_substring in error
    +        # Output should be the FAILED template, not the success template.
    +        assert "(FAILED)" in output
    +        # Ephemeral cron agent must still be closed even on agent-flagged failure.
    +        mock_agent.close.assert_called_once()
    +
    +    def test_run_job_completed_true_without_failed_flag_succeeds(self, tmp_path):
    +        """Regression guard: a normal success result (``completed=True``,
    +        ``failed`` absent) must not trip the failure-flag check.
    +        """
    +        job = {
    +            "id": "ok-job",
    +            "name": "ok",
    +            "prompt": "hello",
    +        }
    +        fake_db = MagicMock()
    +
    +        with patch("cron.scheduler._hermes_home", tmp_path), \
    +             patch("cron.scheduler._resolve_origin", return_value=None), \
    +             patch("dotenv.load_dotenv"), \
    +             patch("hermes_state.SessionDB", return_value=fake_db), \
    +             patch(
    +                 "hermes_cli.runtime_provider.resolve_runtime_provider",
    +                 return_value={
    +                     "api_key": "***",
    +                     "base_url": "https://example.invalid/v1",
    +                     "provider": "openrouter",
    +                     "api_mode": "chat_completions",
    +                 },
    +             ), \
    +             patch("run_agent.AIAgent") as mock_agent_cls:
    +            mock_agent = MagicMock()
    +            mock_agent.run_conversation.return_value = {
    +                "final_response": "all good",
    +                "completed": True,
    +            }
    +            mock_agent_cls.return_value = mock_agent
    +
    +            success, output, final_response, error = run_job(job)
    +
    +        assert success is True
    +        assert error is None
    +        assert final_response == "all good"
    +
         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'.