fix(loop): bound outer-loop error retries per turn instead of relying on max_iterations (#92450)

The outer conversation-loop except handler only left the loop on a
local-processing error or when api_call_count >= max_iterations - 1.
With the turn budget now unlimited by default (sys.maxsize), a
permanent failure that escaped the inner retry/fallback machinery
retried forever: ~64 retries/s, one core pegged, and the rotated
agent.log history overwritten within minutes.

Bound the loop with a small per-turn cap on total escaping exceptions
(_MAX_OUTER_LOOP_ERRORS = 8, scaled down by a tiny explicit
max_iterations so a manually bounded budget still governs). The legacy
local-processing and near-limit exits are byte-identical; a new
'repeated_outer_errors' exit reason gets a user-facing explanation.

The inner retry/fallback layer owns transient API recovery and
terminates on its own, so only exceptions that escape it reach this
cap - a successful turn is unaffected.

Fixes #92450

agent/conversation_loop.py

@@ -272,6 +272,18 @@ _API_CALL_MODULES = frozenset({
     "chat_completion_helpers",
 })
 
+# Maximum total outer-loop exceptions tolerated within one user turn before
+# the loop gives up (#92450). The turn budget is unlimited by default
+# (``max_iterations = sys.maxsize``), so the historical "near the limit"
+# guard no longer stops a turn whose outer loop keeps raising: permanent
+# failures spun at ~64 retries/s, pegged a core, and overwrote the rotated
+# agent.log history (days of diagnostic context) within minutes. The inner
+# retry/fallback machinery owns transient API recovery and terminates on its
+# own; only exceptions that ESCAPE it reach this bound, so the cap can be
+# small. Still scaled down by a tiny explicit ``max_iterations`` so a
+# manually bounded budget keeps governing.
+_MAX_OUTER_LOOP_ERRORS = 8
+
 
 def _is_interpreter_shutdown_error(exc: Exception) -> bool:
     """Check if *exc* is a fatal interpreter-shutdown failure.
@@ -1918,6 +1930,8 @@ def run_conversation(
     failed = False
     codex_ack_continuations = 0
     length_continue_retries = 0
+    # Total outer-loop exceptions this turn (#92450) — see _MAX_OUTER_LOOP_ERRORS.
+    _outer_error_count = 0
     truncated_tool_call_retries = 0
     truncated_response_parts: List[str] = []
     compression_attempts = 0
@@ -8353,6 +8367,11 @@ def run_conversation(
                 break
             
         except Exception as e:
+            # Count every escaped exception against the per-turn bound before
+            # classification — permanent failures must terminate even when the
+            # turn budget is unlimited (#92450).
+            _outer_error_count += 1
+
             # Phase-aware error classification. The huge outer try/except spans
             # both the actual API request and all local post-processing of the
             # returned assistant message. Deterministic local bugs (e.g.
@@ -8478,14 +8497,23 @@ def run_conversation(
 
             # If we're near the limit, break to avoid infinite loops.
             # Local processing errors are deterministic — stop immediately
-            # rather than retrying until the budget is exhausted.
+            # rather than retrying until the budget is exhausted. Repeated
+            # outer-loop errors stop after a small per-turn cap: with
+            # max_iterations now unlimited by default, a permanent failure
+            # would otherwise spin forever and overwrite the rotated log
+            # history within minutes (#92450).
+            _outer_error_cap = min(_MAX_OUTER_LOOP_ERRORS, max(1, agent.max_iterations))
             if (
                 _is_local_processing_error
                 or api_call_count >= agent.max_iterations - 1
+                or _outer_error_count >= _outer_error_cap
             ):
                 if _is_local_processing_error:
                     _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:
+                    _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}"

run_agent.py

@@ -3914,6 +3914,13 @@ class AIAgent:
                 + "an error occurred near the iteration limit before a final "
                 "answer. Check the tool output above, then send `continue`."
             )
+        if reason.startswith("repeated_outer_errors"):
+            return (
+                prefix
+                + "the turn kept failing with repeated errors and was stopped "
+                "early instead of retrying forever. Check the errors above, "
+                "then send `continue` to retry."
+            )
         if reason == "pending_tool_result":
             return (
                 prefix

tests/run_agent/test_92450_outer_error_retry_bound.py

@@ -0,0 +1,203 @@
+"""Tests for issue #92450 — outer-loop error retries must be bounded even
+when the turn budget (``max_iterations``) is unlimited.
+
+Before the fix, the outer ``except`` in ``run_conversation`` only left the
+loop on a local-processing error (#66267) or when
+``api_call_count >= agent.max_iterations - 1``. With the default budget now
+unlimited (``sys.maxsize``), a permanent failure that escaped the inner
+retry/fallback machinery spun forever (~64 retries/s measured in the issue),
+pegged a core, and overwrote days of rotated agent.log history within
+minutes.
+
+Injection points reflect the real escape path: exceptions raised INSIDE the
+inner retry loop (transport errors from ``create()``, normalization,
+fallbacks, ...) never reach the outer handler — that machinery recovers or
+terminates on its own. The outer handler sees failures from the final
+response assembly, e.g. ``_build_assistant_message`` (a chat-completion-
+helpers callable, so such crashes classify as RETRYABLE, never local —
+exactly the spin shape reported in the issue).
+
+The fix adds a small per-turn cap on total outer-loop exceptions
+(``_MAX_OUTER_LOOP_ERRORS``, scaled down by a tiny explicit
+``max_iterations``). These tests pin the behavior contract:
+
+* repeated escaping errors must terminate the turn within the cap;
+* a turn that recovers after early escaping failures must complete normally;
+* local-processing errors still exit immediately (#66267 unchanged);
+* a finite ``max_iterations`` still exhausts via the original near-limit path.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+
+@pytest.fixture()
+def loop_agent():
+    """AIAgent with a mocked OpenAI client (mirrors test_run_agent's fixture)
+    so we can stage responses on ``.chat.completions.create``."""
+    from run_agent import AIAgent
+    from tests.run_agent.test_run_agent import _mock_response
+
+    with (
+        patch("run_agent.get_tool_definitions", return_value=[]),
+        patch("run_agent.check_toolset_requirements", return_value={}),
+        patch("run_agent.OpenAI"),
+    ):
+        agent = AIAgent(
+            api_key="test-key-1234567890",
+            base_url="https://openrouter.ai/api/v1",
+            quiet_mode=True,
+            skip_context_files=True,
+            skip_memory=True,
+        )
+        agent.client = MagicMock()
+        agent._cached_system_prompt = "You are helpful."
+        agent._use_prompt_caching = False
+        agent.tool_delay = 0
+        agent.compression_enabled = False
+        agent.save_trajectories = False
+        # Every API call itself SUCCEEDS — #92450's spin happens after the
+        # response arrives, when final-response assembly keeps crashing.
+        agent.client.chat.completions.create.side_effect = (
+            lambda *a, **k: _mock_response(content="ok", finish_reason="stop")
+        )
+        return agent
+
+
+class _PermanentError(Exception):
+    """An error type no recovery path classifies as retryable-local."""
+
+
+def _make_local_frame_raiser():
+    """Compile a raiser whose frame filename lives in a local-processing
+    module, so the production traceback classifier (#66267) sees it as a
+    deterministic local bug — without mocking away the classifier itself."""
+    namespace = {}
+    code = compile(
+        "def _raise(exc):\n    raise exc\n",
+        filename="/agent/agent_runtime_helpers.py",
+        mode="exec",
+    )
+    exec(code, namespace)
+    return namespace["_raise"]
+
+
+class TestOuterErrorRetryBound:
+    def test_repeated_escaping_errors_are_bounded(self, loop_agent):
+        """A permanent final-assembly failure that escapes every retry layer
+        must end the turn within the per-turn error cap instead of spinning
+        forever under the unlimited default budget."""
+        boom = _PermanentError("permanent assembly contract violation")
+
+        with (
+            patch.object(
+                loop_agent, "_build_assistant_message", side_effect=boom
+            ),
+            patch.object(loop_agent, "_persist_session"),
+            patch.object(loop_agent, "_save_trajectory"),
+            patch.object(loop_agent, "_cleanup_task_resources"),
+        ):
+            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
+        # meaningful assertions are: bounded call count + dedicated exit
+        # reason + apology text.
+        assert result["api_calls"] <= 8, (
+            "The loop must give up within the per-turn error bound instead "
+            "of retrying an unlimited-budget failure forever."
+        )
+        assert result["turn_exit_reason"].startswith("repeated_outer_errors"), (
+            f"unexpected exit reason: {result['turn_exit_reason']}"
+        )
+        assert "repeated errors" in (result["final_response"] or "")
+
+    def test_recovery_after_failures_completes_normally(self, loop_agent):
+        """Early escaping failures followed by success must NOT trip the new
+        bound — the turn completes and the reply is delivered."""
+        good_msg = {
+            "role": "assistant",
+            "content": "Recovered fine.",
+            "finish_reason": "stop",
+        }
+
+        with (
+            patch.object(
+                loop_agent,
+                "_build_assistant_message",
+                side_effect=[
+                    _PermanentError("transient assembly hiccup"),
+                    _PermanentError("transient assembly hiccup"),
+                    dict(good_msg),
+                ],
+            ),
+            patch.object(loop_agent, "_persist_session"),
+            patch.object(loop_agent, "_save_trajectory"),
+            patch.object(loop_agent, "_cleanup_task_resources"),
+        ):
+            result = loop_agent.run_conversation("hello")
+
+        # final_response comes from the raw provider content ("ok"), so the
+        # recovery contract is: clean text_response exit exactly on the 3rd
+        # call, marked completed — proving the two earlier escapes did not
+        # trip the new bound.
+        assert result["completed"] is True, (
+            f"turn should recover: exit={result.get('turn_exit_reason')}"
+        )
+        assert result["failed"] is False
+        assert result["api_calls"] == 3
+        assert result["turn_exit_reason"].startswith("text_response"), (
+            f"unexpected exit reason: {result['turn_exit_reason']}"
+        )
+
+    def test_local_processing_error_still_exits_immediately(self, loop_agent):
+        """#66267 regression guard: a deterministic local bug exits on first
+        occurrence — the new counter must not delay or change that exit."""
+        raiser = _make_local_frame_raiser()
+        boom = TypeError("list content fed into a str regex helper")
+
+        with (
+            patch.object(
+                loop_agent,
+                "_strip_think_blocks",
+                side_effect=lambda text: raiser(boom),
+            ),
+            patch.object(loop_agent, "_persist_session"),
+            patch.object(loop_agent, "_save_trajectory"),
+            patch.object(loop_agent, "_cleanup_task_resources"),
+        ):
+            result = loop_agent.run_conversation("hello")
+
+        assert result["turn_exit_reason"].startswith("local_processing_error"), (
+            f"unexpected exit reason: {result['turn_exit_reason']}"
+        )
+        assert result["api_calls"] == 1, (
+            "A local processing error must stop immediately on first "
+            "occurrence, not consume retries."
+        )
+
+    def test_finite_budget_still_governs_near_limit_exit(self, loop_agent):
+        """With a small explicit max_iterations the ORIGINAL near-limit path
+        fires (unchanged reason string), not the new repeated-error path."""
+        loop_agent.max_iterations = 3
+        boom = _PermanentError("permanent assembly failure")
+
+        with (
+            patch.object(
+                loop_agent, "_build_assistant_message", side_effect=boom
+            ),
+            patch.object(loop_agent, "_persist_session"),
+            patch.object(loop_agent, "_save_trajectory"),
+            patch.object(loop_agent, "_cleanup_task_resources"),
+        ):
+            result = loop_agent.run_conversation("hello")
+
+        # Budget of 3 → the legacy guard fires on error #2 (api_call_count 2
+        # >= 3 - 1); the new cap would fire at error #3, so the legacy path
+        # must win here.
+        assert result["turn_exit_reason"].startswith(
+            "error_near_max_iterations"
+        ), f"legacy near-limit path must govern: {result['turn_exit_reason']}"