From c33f8d381b87fd85dda1305a14fb7101ceb47b61 Mon Sep 17 00:00:00 2001 From: Farukest Date: Sun, 1 Mar 2026 02:27:26 +0300 Subject: [PATCH] fix: correct off-by-one in retry exhaustion checks The retry exhaustion checks used > instead of >= to compare retry_count against max_retries. Since the while loop condition is retry_count < max_retries, the check retry_count > max_retries can never be true inside the loop. When retries are exhausted, the loop exits and falls through to response.choices[0] on an invalid response, crashing with IndexError instead of returning a proper error. --- run_agent.py | 4 ++-- tests/test_run_agent.py | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/run_agent.py b/run_agent.py index 91db7cc2a..56c30e23d 100644 --- a/run_agent.py +++ b/run_agent.py @@ -2092,7 +2092,7 @@ class AIAgent: print(f"{self.log_prefix} 📝 Provider message: {error_msg[:200]}") print(f"{self.log_prefix} ⏱️ Response time: {api_duration:.2f}s (fast response often indicates rate limiting)") - if retry_count > max_retries: + if retry_count >= max_retries: print(f"{self.log_prefix}❌ Max retries ({max_retries}) exceeded for invalid responses. Giving up.") logging.error(f"{self.log_prefix}Invalid API response after {max_retries} retries.") self._persist_session(messages, conversation_history) @@ -2323,7 +2323,7 @@ class AIAgent: "partial": True } - if retry_count > max_retries: + if retry_count >= max_retries: print(f"{self.log_prefix}❌ Max retries ({max_retries}) exceeded. Giving up.") logging.error(f"{self.log_prefix}API call failed after {max_retries} retries. Last error: {api_error}") logging.error(f"{self.log_prefix}Request details - Messages: {len(api_messages)}, Approx tokens: {approx_tokens:,}") diff --git a/tests/test_run_agent.py b/tests/test_run_agent.py index 2d3703933..92ab23cba 100644 --- a/tests/test_run_agent.py +++ b/tests/test_run_agent.py @@ -758,3 +758,49 @@ class TestRunConversation: ) result = agent.run_conversation("search something") mock_compress.assert_called_once() + + +class TestRetryExhaustion: + """Regression: retry_count > max_retries was dead code (off-by-one). + + When retries were exhausted the condition never triggered, causing + the loop to exit and fall through to response.choices[0] on an + invalid response, raising IndexError. + """ + + def _setup_agent(self, agent): + agent._cached_system_prompt = "You are helpful." + agent._use_prompt_caching = False + agent.tool_delay = 0 + agent.compression_enabled = False + agent.save_trajectories = False + + def test_invalid_response_returns_error_not_crash(self, agent): + """Exhausted retries on invalid (empty choices) response must not IndexError.""" + self._setup_agent(agent) + # Return response with empty choices every time + bad_resp = SimpleNamespace( + choices=[], + model="test/model", + usage=None, + ) + agent.client.chat.completions.create.return_value = bad_resp + with ( + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("hello") + assert result.get("failed") is True or result.get("completed") is False + + def test_api_error_raises_after_retries(self, agent): + """Exhausted retries on API errors must raise, not fall through.""" + self._setup_agent(agent) + agent.client.chat.completions.create.side_effect = RuntimeError("rate limited") + with ( + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + with pytest.raises(RuntimeError, match="rate limited"): + agent.run_conversation("hello")