From cd7c203ab9fd6919c1e9b310783fb2d02d85c774 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 10 Jul 2026 11:23:59 +0530 Subject: [PATCH] fix(agent): preserve gated responses without masking failures Track held-back verification responses explicitly so budget exhaustion returns the composed report without a second model call. Keep unrelated error and recovery exit reasons intact, preserve Kanban timeout accounting, and cover the real run_conversation paths. --- agent/conversation_loop.py | 16 ++- agent/turn_finalizer.py | 57 +++++----- ...est_turn_finalizer_iteration_limit_exit.py | 79 ++++++++++++- .../test_verification_continuation_budget.py | 104 ++++++++++++++++++ 4 files changed, 217 insertions(+), 39 deletions(-) create mode 100644 tests/run_agent/test_verification_continuation_budget.py diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 4f29e8249ba..073a475fa0f 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -613,6 +613,11 @@ def run_conversation( truncated_response_parts: List[str] = [] compression_attempts = 0 _turn_exit_reason = "unknown" # Diagnostic: why the loop ended + # Last composed answer intentionally held back by an internal continuation + # gate. If the continuation consumes the remaining budget, this is the + # best user-facing result available; it must not be confused with error or + # recovery text produced by unrelated exit paths. + _pending_continuation_response = None # Per-turn tally of consecutive successful credential-pool token refreshes, # keyed by (provider, pool-entry-id). A persistent upstream 401 lets @@ -5173,10 +5178,11 @@ def run_conversation( # terminal. Keep a debug breadcrumb in agent.log for tracing. logger.debug("verification stop-loop nudge issued (attempt %d)", agent._verification_stop_nudges) - # The attempted answer lives in ``final_msg`` / ``messages`` for - # the verify loop; do not keep a stale ``final_response`` that - # would skip turn_finalizer's iteration-limit normalization. - # (#61631) + # Keep the attempted answer only as an explicit fallback for + # continuation-budget exhaustion. ``final_response`` itself + # must be cleared so the finalizer can distinguish this gate + # from unrelated error/recovery exits. (#61631) + _pending_continuation_response = final_response final_response = None continue @@ -5229,6 +5235,7 @@ def run_conversation( agent._session_messages = messages logger.debug("pre_verify nudge issued (attempt %d)", agent._pre_verify_nudges) + _pending_continuation_response = final_response final_response = None continue @@ -5314,6 +5321,7 @@ def run_conversation( original_user_message=original_user_message, _should_review_memory=_should_review_memory, _turn_exit_reason=_turn_exit_reason, + _pending_continuation_response=_pending_continuation_response, ) diff --git a/agent/turn_finalizer.py b/agent/turn_finalizer.py index 108262e16c8..87f0a6c1b5e 100644 --- a/agent/turn_finalizer.py +++ b/agent/turn_finalizer.py @@ -42,6 +42,7 @@ def finalize_turn( original_user_message, _should_review_memory, _turn_exit_reason, + _pending_continuation_response=None, ): """Run the post-loop finalization and return the turn ``result`` dict. @@ -50,10 +51,27 @@ def finalize_turn( """ from agent.conversation_loop import logger - if final_response is None and ( + budget_exhausted = ( api_call_count >= agent.max_iterations or agent.iteration_budget.remaining <= 0 - ): + ) + continuation_budget_exhausted = ( + final_response is None + and bool(_pending_continuation_response) + and budget_exhausted + ) + + iteration_limit_fallback = False + if continuation_budget_exhausted: + # A verification/continuation gate deliberately withheld a composed + # answer, then consumed the remaining budget before producing a newer + # one. Preserve that exact answer instead of replacing it with another + # fallible model call. The explicit pending value is the provenance + # guard: unrelated error/recovery exits can never enter this branch. + final_response = _pending_continuation_response + _turn_exit_reason = f"max_iterations_reached({api_call_count}/{agent.max_iterations})" + iteration_limit_fallback = True + elif final_response is None and budget_exhausted: # Budget exhausted — ask the model for a summary via one extra # API call with tools stripped. _handle_max_iterations injects a # user message and makes a single toolless request. @@ -68,20 +86,18 @@ def finalize_turn( "— requesting summary..." ) final_response = agent._handle_max_iterations(messages, api_call_count) + iteration_limit_fallback = True + if iteration_limit_fallback: # If running as a kanban worker, signal the dispatcher that the # worker could not complete (rather than treating it as a - # protocol violation). The agent loop strips tools before calling - # _handle_max_iterations, so the model cannot call kanban_block - # itself — we must do it on its behalf. + # protocol violation). This applies whether the user-facing fallback + # came from the summary call or an explicitly pending continuation; + # both exhausted the task budget and must advance the failure circuit. # # We route through ``_record_task_failure(outcome="timed_out")`` - # rather than ``kanban_block`` so this counts toward the - # ``consecutive_failures`` counter and the dispatcher's - # ``failure_limit`` circuit breaker (#29747 gap 2). Without this, - # a task whose worker keeps exhausting its budget would block - # silently each run, get auto-promoted by the operator (or never - # surface), and re-block in an endless loop with no signal. + # rather than ``kanban_block`` so this counts toward the dispatcher's + # consecutive-failure circuit breaker (#29747 gap 2). _kanban_task = os.environ.get("HERMES_KANBAN_TASK") if _kanban_task: try: @@ -121,24 +137,6 @@ def finalize_turn( exc_info=True, ) - elif ( - final_response is not None - and ( - api_call_count >= agent.max_iterations - or agent.iteration_budget.remaining <= 0 - ) - and not str(_turn_exit_reason).startswith("text_response(") - and not str(_turn_exit_reason).startswith("max_iterations_reached(") - ): - # verify-on-stop can assign ``final_response`` then ``continue`` the - # loop until the iteration budget is gone, leaving a composed answer - # with a non-success exit reason (``unknown``, ``budget_exhausted``). - # Normalize so cron's graceful-delivery exemption can match. - # (#61631) - _turn_exit_reason = ( - f"max_iterations_reached({api_call_count}/{agent.max_iterations})" - ) - # Determine if conversation completed successfully normal_text_response = str(_turn_exit_reason).startswith("text_response(") completed = ( @@ -322,6 +320,7 @@ def finalize_turn( # truncated partial (the "The" case from #34452). _is_partial_fragment = ( not _is_empty_terminal + and not continuation_budget_exhausted and not str(_turn_exit_reason).startswith("text_response") and len(_stripped) <= 24 and _stripped[-1:] not in {".", "!", "?", "。", "！", "？", "`", ")"} diff --git a/tests/agent/test_turn_finalizer_iteration_limit_exit.py b/tests/agent/test_turn_finalizer_iteration_limit_exit.py index dd55a62b9ea..ecca23992b9 100644 --- a/tests/agent/test_turn_finalizer_iteration_limit_exit.py +++ b/tests/agent/test_turn_finalizer_iteration_limit_exit.py @@ -1,6 +1,9 @@ """Regression tests for iteration-limit exit normalization (#61631).""" from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest from agent.turn_finalizer import finalize_turn @@ -75,7 +78,14 @@ class _LimitAgent: pass -def _finalize(agent, *, final_response, exit_reason, api_call_count=60): +def _finalize( + agent, + *, + final_response, + exit_reason, + api_call_count=60, + pending_continuation_response=None, +): return finalize_turn( agent, final_response=final_response, @@ -90,28 +100,39 @@ def _finalize(agent, *, final_response, exit_reason, api_call_count=60): original_user_message="task", _should_review_memory=False, _turn_exit_reason=exit_reason, + _pending_continuation_response=pending_continuation_response, ) -def test_stale_final_response_unknown_normalizes_for_cron_delivery(monkeypatch): - """verify-on-stop can leave a composed answer with exit reason ``unknown``.""" +def test_pending_verify_response_is_preserved_for_cron_delivery(monkeypatch): + """A held-back verification response survives last-turn exhaustion.""" monkeypatch.setattr("hermes_cli.plugins.invoke_hook", lambda *_a, **_kw: []) agent = _LimitAgent() report = "complete cron report body" - result = _finalize(agent, final_response=report, exit_reason="unknown") + result = _finalize( + agent, + final_response=None, + exit_reason="unknown", + pending_continuation_response=report, + ) assert result["final_response"] == report assert result["turn_exit_reason"] == "max_iterations_reached(60/60)" assert agent._handle_max_iterations_called is False -def test_stale_final_response_budget_exhausted_normalizes(monkeypatch): +def test_pending_pre_verify_response_is_preserved_on_budget_exhaustion(monkeypatch): monkeypatch.setattr("hermes_cli.plugins.invoke_hook", lambda *_a, **_kw: []) agent = _LimitAgent() report = "budget exhausted but complete" - result = _finalize(agent, final_response=report, exit_reason="budget_exhausted") + result = _finalize( + agent, + final_response=None, + exit_reason="budget_exhausted", + pending_continuation_response=report, + ) assert result["final_response"] == report assert result["turn_exit_reason"] == "max_iterations_reached(60/60)" @@ -132,3 +153,49 @@ def test_text_response_exit_not_rewritten_at_iteration_limit(monkeypatch): assert result["turn_exit_reason"] == exit_reason assert agent._handle_max_iterations_called is False + + +@pytest.mark.parametrize( + "exit_reason", + [ + "error_near_max_iterations(boom)", + "guardrail_halt", + "partial_stream_recovery", + "fallback_prior_turn_content", + "empty_response_exhausted", + ], +) +def test_unrelated_non_success_response_is_not_reclassified(monkeypatch, exit_reason): + monkeypatch.setattr("hermes_cli.plugins.invoke_hook", lambda *_a, **_kw: []) + agent = _LimitAgent() + + result = _finalize( + agent, + final_response="diagnostic or partial content", + exit_reason=exit_reason, + ) + + assert result["turn_exit_reason"] == exit_reason + assert result["completed"] is False + assert agent._handle_max_iterations_called is False + + +def test_pending_response_records_kanban_timeout(monkeypatch): + monkeypatch.setattr("hermes_cli.plugins.invoke_hook", lambda *_a, **_kw: []) + monkeypatch.setenv("HERMES_KANBAN_TASK", "task-123") + record = MagicMock(name="record_task_failure") + conn = SimpleNamespace(close=lambda: None) + monkeypatch.setattr("hermes_cli.kanban_db.connect", lambda: conn) + monkeypatch.setattr("hermes_cli.kanban_db._record_task_failure", record) + agent = _LimitAgent() + + result = _finalize( + agent, + final_response=None, + exit_reason="unknown", + pending_continuation_response="composed report", + ) + + assert result["turn_exit_reason"] == "max_iterations_reached(60/60)" + record.assert_called_once() + assert record.call_args.kwargs["outcome"] == "timed_out" diff --git a/tests/run_agent/test_verification_continuation_budget.py b/tests/run_agent/test_verification_continuation_budget.py new file mode 100644 index 00000000000..e55ce65a42a --- /dev/null +++ b/tests/run_agent/test_verification_continuation_budget.py @@ -0,0 +1,104 @@ +"""End-to-end regression coverage for verification budget exhaustion (#61631).""" + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +from run_agent import AIAgent + + +def _response(content="composed report"): + message = SimpleNamespace(content=content, tool_calls=None) + return SimpleNamespace( + choices=[SimpleNamespace(message=message, finish_reason="stop")], + model="test/model", + usage=None, + ) + + +@pytest.fixture +def agent(tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + with ( + patch("run_agent.get_tool_definitions", return_value=[]), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + ): + instance = AIAgent( + session_id="verify-budget-test", + api_key="test-key", + base_url="https://example.invalid/v1", + provider="openai-compat", + model="test/model", + max_iterations=1, + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + instance._cached_system_prompt = "stable test prompt" + instance._session_db = None + instance._session_json_enabled = False + instance.save_trajectories = False + instance.compression_enabled = False + instance._cleanup_task_resources = lambda *_a, **_kw: None + instance._save_trajectory = lambda *_a, **_kw: None + return instance + + +def _assert_pending_response_survives(agent, result): + assert result["final_response"] == "composed report" + assert result["turn_exit_reason"] == "max_iterations_reached(1/1)" + assert result["completed"] is False + assert agent._handle_max_iterations.call_count == 0 + assert [message["role"] for message in result["messages"]] == [ + "user", + "assistant", + "user", + "assistant", + ] + + +def test_verify_on_stop_preserves_composed_report_at_budget_limit(agent, monkeypatch): + def model_call(_api_kwargs): + agent._turn_file_mutation_paths = {"changed.py"} + return _response() + + agent._interruptible_api_call = model_call + agent._handle_max_iterations = MagicMock(return_value="replacement summary") + monkeypatch.setenv("HERMES_VERIFY_ON_STOP", "1") + + with ( + patch("agent.verification_stop.build_verify_on_stop_nudge", return_value="verify it"), + patch("hermes_cli.plugins.invoke_hook", return_value=[]), + ): + result = agent.run_conversation("edit changed.py") + + _assert_pending_response_survives(agent, result) + assert result["messages"][1]["_verification_stop_synthetic"] is True + assert result["messages"][2]["_verification_stop_synthetic"] is True + + +def test_pre_verify_preserves_composed_report_at_budget_limit(agent, monkeypatch): + def model_call(_api_kwargs): + agent._turn_file_mutation_paths = {"changed.py"} + return _response() + + agent._interruptible_api_call = model_call + agent._handle_max_iterations = MagicMock(return_value="replacement summary") + monkeypatch.setenv("HERMES_VERIFY_ON_STOP", "0") + + with ( + patch("hermes_cli.plugins.has_hook", side_effect=lambda name: name == "pre_verify"), + patch( + "hermes_cli.plugins.get_pre_verify_continue_message", + return_value="run project tests", + ), + patch("agent.verify_hooks.max_verify_nudges", return_value=2), + patch("hermes_cli.plugins.invoke_hook", return_value=[]), + ): + result = agent.run_conversation("edit changed.py") + + _assert_pending_response_survives(agent, result) + assert result["messages"][1]["_pre_verify_synthetic"] is True + assert result["messages"][2]["_pre_verify_synthetic"] is True