mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-15 14:22:43 +00:00
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.
This commit is contained in:
parent
53231fb00b
commit
cd7c203ab9
4 changed files with 217 additions and 39 deletions
|
|
@ -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,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {".", "!", "?", "。", "!", "?", "`", ")"}
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
104
tests/run_agent/test_verification_continuation_budget.py
Normal file
104
tests/run_agent/test_verification_continuation_budget.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Add a link
Reference in a new issue