From b8cd00f96877e2d50bba23320acb404226de1d46 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 24 Aug 2026 04:39:02 +0530 Subject: [PATCH] fix: set failed=True for repeated_outer_errors exit + drop append_message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to @BrunoBza's #93062: 1. Set failed=True only for the new repeated_outer_errors exit reason. Previously the error exit left failed=False, so finalize_turn reported completed=True for a turn that actually failed — incorrect. 2. Don't append_message the assistant response at the break. A thinking- prefill or interim assistant may already be the tail, and appending would create assistant→assistant role-alternation violation. finalize_turn (lines 341-353) handles this safely by checking _tail_role != 'assistant' before appending. 3. Update test to assert failed=True and completed=False for the repeated_outer_errors exit. --- agent/conversation_loop.py | 10 +++++++--- tests/run_agent/test_92450_outer_error_retry_bound.py | 6 ++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 7995b59f11..24ebd0edbb 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -8512,14 +8512,18 @@ def run_conversation( _turn_exit_reason = f"local_processing_error({error_msg[:80]})" final_response = f"I apologize, but I encountered an error while processing the model response: {error_msg}" elif _outer_error_count >= _outer_error_cap: + failed = True _turn_exit_reason = f"repeated_outer_errors({error_msg[:80]})" final_response = f"I apologize, but I encountered repeated errors: {error_msg}" else: _turn_exit_reason = f"error_near_max_iterations({error_msg[:80]})" final_response = f"I apologize, but I encountered repeated errors: {error_msg}" - # Append as assistant so the history stays valid for - # session resume (avoids consecutive user messages). - append_message(messages, {"role": "assistant", "content": final_response}) + # Don't append the assistant message here — a thinking-prefill + # or interim assistant may already be the tail, and appending + # would create assistant→assistant. finalize_turn handles + # this case safely (lines 341-353: appends only when + # _tail_role != "assistant"), matching the pattern at other + # break sites that set final_response without appending. break # Post-loop turn finalization extracted to agent/turn_finalizer.finalize_turn diff --git a/tests/run_agent/test_92450_outer_error_retry_bound.py b/tests/run_agent/test_92450_outer_error_retry_bound.py index 240a56d0d0..17e277da81 100644 --- a/tests/run_agent/test_92450_outer_error_retry_bound.py +++ b/tests/run_agent/test_92450_outer_error_retry_bound.py @@ -103,9 +103,9 @@ class TestOuterErrorRetryBound: result = loop_agent.run_conversation("hello") # The bound path returns an apology as final_response with - # failed=False (same contract as the legacy near-limit exit), so the + # failed=True (the turn did not complete successfully), so the # meaningful assertions are: bounded call count + dedicated exit - # reason + apology text. + # reason + apology text + failed flag. assert result["api_calls"] <= 8, ( "The loop must give up within the per-turn error bound instead " "of retrying an unlimited-budget failure forever." @@ -114,6 +114,8 @@ class TestOuterErrorRetryBound: f"unexpected exit reason: {result['turn_exit_reason']}" ) assert "repeated errors" in (result["final_response"] or "") + assert result["failed"] is True + assert result["completed"] is False def test_recovery_after_failures_completes_normally(self, loop_agent): """Early escaping failures followed by success must NOT trip the new