diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index f2efe04c6e567..b963965c4a576 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -3779,9 +3779,18 @@ class TestRunConversation: mock_handle_function_call.assert_not_called() def test_kanban_block_called_on_iteration_exhaustion(self, agent, monkeypatch): - """Regression: kanban worker must call kanban_block when iteration - budget is exhausted, otherwise the dispatcher sees a protocol - violation and gives up after 1 failure (issue #23216).""" + """Regression: kanban worker must signal the dispatcher when its + iteration budget is exhausted, otherwise the task silently re-runs + forever without ever tripping the failure_limit circuit breaker + (issue #23216 / #29747 gap 2). + + As of #29747, the exhaustion path routes through + ``kanban_db._record_task_failure(outcome="timed_out")`` so the + ``consecutive_failures`` counter increments and the dispatcher's + ``failure_limit`` breaker eventually trips. The legacy + ``kanban_block`` call was replaced because blocked-outcome runs + bypass the failure counter. + """ self._setup_agent(agent) agent.max_iterations = 2 @@ -3800,8 +3809,14 @@ class TestRunConversation: tool_resp, tool_resp, summary_resp, ] + mock_record_failure = MagicMock(return_value=False) + mock_connect = MagicMock(return_value=MagicMock()) + with ( - patch("run_agent.handle_function_call", return_value="ok") as mock_hfc, + patch("run_agent.handle_function_call", return_value="ok"), + patch("hermes_cli.kanban_db._record_task_failure", + mock_record_failure), + patch("hermes_cli.kanban_db.connect", mock_connect), patch.object(agent, "_persist_session"), patch.object(agent, "_save_trajectory"), patch.object(agent, "_cleanup_task_resources"), @@ -3811,23 +3826,24 @@ class TestRunConversation: # The agent should have reported the task as not completed. assert result["completed"] is False - # Among all handle_function_call invocations, one must be - # kanban_block with the correct task_id and a reason mentioning - # iteration exhaustion. - kanban_block_calls = [ - c for c in mock_hfc.call_args_list - if c[0][0] == "kanban_block" - ] - assert len(kanban_block_calls) == 1, ( - f"Expected exactly 1 kanban_block call, got {len(kanban_block_calls)}. " - f"All calls: {mock_hfc.call_args_list}" + # _record_task_failure should have been called exactly once for + # the exhaustion event, with outcome="timed_out". + assert mock_record_failure.call_count == 1, ( + f"Expected exactly 1 _record_task_failure call, " + f"got {mock_record_failure.call_count}. " + f"Calls: {mock_record_failure.call_args_list}" ) - call = kanban_block_calls[0] - assert call[0][1]["task_id"] == "t_test_task_123" - assert "Iteration budget exhausted" in call[0][1]["reason"] + call = mock_record_failure.call_args_list[0] + # Positional: (conn, task_id, ...) + assert call.args[1] == "t_test_task_123" + assert call.kwargs.get("outcome") == "timed_out" + assert call.kwargs.get("release_claim") is True + assert call.kwargs.get("end_run") is True + assert "Iteration budget exhausted" in call.kwargs.get("error", "") def test_no_kanban_block_when_not_in_kanban_mode(self, agent, monkeypatch): - """kanban_block must NOT be called when HERMES_KANBAN_TASK is unset.""" + """The exhaustion bridge must NOT fire when HERMES_KANBAN_TASK + is unset (non-kanban runs are unaffected by #29747 gap 2).""" self._setup_agent(agent) agent.max_iterations = 2 @@ -3844,20 +3860,20 @@ class TestRunConversation: tool_resp, tool_resp, summary_resp, ] + mock_record_failure = MagicMock(return_value=False) + with ( - patch("run_agent.handle_function_call", return_value="ok") as mock_hfc, + patch("run_agent.handle_function_call", return_value="ok"), + patch("hermes_cli.kanban_db._record_task_failure", + mock_record_failure), patch.object(agent, "_persist_session"), patch.object(agent, "_save_trajectory"), patch.object(agent, "_cleanup_task_resources"), ): agent.run_conversation("do stuff") - kanban_block_calls = [ - c for c in mock_hfc.call_args_list - if c[0][0] == "kanban_block" - ] - assert len(kanban_block_calls) == 0, ( - "kanban_block should not be called outside kanban mode" + assert mock_record_failure.call_count == 0, ( + "_record_task_failure should not be called outside kanban mode" )