diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index e2bcbfc824b2..c2aee3bdf0aa 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -281,7 +281,35 @@ def _consume_codex_event_stream( terminal_error: Any = None saw_terminal = False - for event in event_iter: + event_iter = iter(event_iter) + while True: + try: + event = next(event_iter) + except StopIteration: + break + except TypeError as exc: + # The OpenAI SDK's streamed Responses iterator still runs its own + # accumulator while yielding SSE events. When chatgpt.com emits a + # terminal snapshot whose ``response.output`` is ``null``, the SDK + # trips over ``for output in response.output`` and raises + # ``TypeError: 'NoneType' object is not iterable`` *after* earlier + # ``response.output_item.done`` / ``response.output_text.delta`` + # events have already arrived. If we have recoverable streamed + # state, synthesize the final response from that state instead of + # letting a local parse bug poison credential-pool health. + if "not iterable" not in str(exc): + raise + if not collected_output_items and not collected_text_deltas: + raise + logger.debug( + "Codex Responses iterator hit SDK None-output TypeError; " + "recovering from streamed state (items=%d, chars=%d)", + len(collected_output_items), + sum(len(p) for p in collected_text_deltas), + ) + saw_terminal = True + terminal_status = terminal_status or "completed" + break if on_event is not None: try: on_event(event) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 078c62771ede..efe153affda4 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -2914,7 +2914,11 @@ def run_conversation( if is_client_error: # Try fallback before aborting — a different provider # may not have the same issue (rate limit, auth, etc.) - agent._emit_status(f"⚠️ Non-retryable error (HTTP {status_code}) — trying fallback...") + _err_kind = agent._nonretryable_error_kind(status_code, api_error) + agent._emit_status( + f"⚠️ Non-retryable error ({_err_kind}): " + f"{agent._summarize_api_error(api_error)} — trying fallback..." + ) if agent._try_activate_fallback(): retry_count = 0 compression_attempts = 0 @@ -2925,10 +2929,10 @@ def run_conversation( api_kwargs, reason="non_retryable_client_error", error=api_error, ) agent._emit_status( - f"❌ Non-retryable error (HTTP {status_code}): " + f"❌ Non-retryable error ({_err_kind}): " f"{agent._summarize_api_error(api_error)}" ) - agent._vprint(f"{agent.log_prefix}❌ Non-retryable client error (HTTP {status_code}). Aborting.", force=True) + agent._vprint(f"{agent.log_prefix}❌ Non-retryable client error ({_err_kind}). Aborting.", force=True) agent._vprint(f"{agent.log_prefix} 🔌 Provider: {_provider} Model: {_model}", force=True) agent._vprint(f"{agent.log_prefix} 🌐 Endpoint: {_base}", force=True) # Actionable guidance for common auth errors diff --git a/run_agent.py b/run_agent.py index 9c130d8d294a..cc2d21bb1b57 100644 --- a/run_agent.py +++ b/run_agent.py @@ -1563,6 +1563,18 @@ class AIAgent: prefix = f"HTTP {status_code}: " if status_code else "" return f"{prefix}{raw[:500]}" + @staticmethod + def _nonretryable_error_kind(status_code, api_error) -> str: + """Short label for a non-retryable failure. + + Returns ``HTTP `` when an HTTP status is present, otherwise the + exception class name. This avoids misleading labels like ``HTTP None`` + for local parse or transport failures. + """ + if status_code: + return f"HTTP {status_code}" + return type(api_error).__name__ + def _mask_api_key_for_logs(self, key: Any) -> Optional[str]: # Azure Foundry Entra ID bearer providers are callables — never # invoke them in log paths; identify the auth surface instead. diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index 7e4ddcae133e..ad4a06de3154 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -2591,6 +2591,35 @@ class TestCodexAuxiliaryAdapterNullOutputRecovery: assert response.choices[0].message.content == "aux survived" + def test_recovers_when_sdk_iterator_raises_none_output_typeerror(self): + """Recover from streamed items when the SDK iterator crashes post-stream.""" + output_item = SimpleNamespace( + type="message", + content=[SimpleNamespace(type="output_text", text="aux recovered")], + ) + events = [ + SimpleNamespace(type="response.created"), + SimpleNamespace(type="response.output_item.done", item=output_item), + ] + + class _BrokenCreateStream: + def __iter__(self): + for event in events: + yield event + raise TypeError("'NoneType' object is not iterable") + def close(self): pass + + class FakeResponses: + def create(self, **kwargs): + return _BrokenCreateStream() + + fake_client = SimpleNamespace(responses=FakeResponses()) + adapter = _CodexCompletionsAdapter(fake_client, "gpt-5.5") + + response = adapter.create(messages=[{"role": "user", "content": "summarize"}]) + + assert response.choices[0].message.content == "aux recovered" + # --------------------------------------------------------------------------- # Issue #23432 — auxiliary timeout poisons cached client; later aux calls fail diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 7e26cfb9dfc0..bb914b6f0275 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -5632,3 +5632,15 @@ class TestMemoryProviderTurnStart: # The extracted body uses ``agent.X`` rather than ``self.X``; # assert the extracted-form spelling directly. assert "on_turn_start(agent._user_turn_count" in src + + +def test_nonretryable_error_kind_surfaces_exception_type_without_status(): + """Transport/parse failures with no status must not render as HTTP None.""" + assert ( + AIAgent._nonretryable_error_kind( + None, TypeError("'NoneType' object is not iterable") + ) + == "TypeError" + ) + assert AIAgent._nonretryable_error_kind(0, ValueError("bad")) == "ValueError" + assert AIAgent._nonretryable_error_kind(404, RuntimeError("nope")) == "HTTP 404" diff --git a/tests/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index 638c1dd99e6f..78a4e396f932 100644 --- a/tests/run_agent/test_run_agent_codex_responses.py +++ b/tests/run_agent/test_run_agent_codex_responses.py @@ -161,12 +161,16 @@ class _FakeCreateStream: tests use this to drive it through the same code paths the wire does. """ - def __init__(self, events): + def __init__(self, events, *, iter_error=None): self._events = list(events) + self._iter_error = iter_error self.closed = False def __iter__(self): - return iter(self._events) + for event in self._events: + yield event + if self._iter_error is not None: + raise self._iter_error def close(self): self.closed = True @@ -531,6 +535,48 @@ def test_run_codex_stream_ignores_completed_response_with_null_output(monkeypatc assert response.usage.total_tokens == 11 +def test_run_codex_stream_recovers_after_sdk_none_output_typeerror(monkeypatch): + """If the SDK iterator crashes after streamed items arrived, recover from them. + + ``responses.create(stream=True)`` still runs the OpenAI SDK's internal + accumulator while yielding SSE events. When chatgpt.com sends a terminal + snapshot with ``response.output = null``, the iterator can raise + ``TypeError: 'NoneType' object is not iterable`` after earlier + ``response.output_item.done`` events were already delivered. The runtime + must synthesize the final response from those streamed items instead of + surfacing a local parse failure. + """ + agent = _build_agent(monkeypatch) + tool_call_item = SimpleNamespace( + type="function_call", + call_id="call_123", + name="lookup_weather", + arguments='{"city":"Lahore"}', + ) + create_stream = _FakeCreateStream( + [ + SimpleNamespace(type="response.created"), + SimpleNamespace(type="response.output_item.done", item=tool_call_item), + ], + iter_error=TypeError("'NoneType' object is not iterable"), + ) + + def _fake_create(**kwargs): + assert kwargs.get("stream") is True + return create_stream + + agent.client = SimpleNamespace( + responses=SimpleNamespace(create=_fake_create), + ) + + response = agent._run_codex_stream(_codex_request_kwargs()) + assert response is not None + assert create_stream.closed is True + assert response.status == "completed" + assert response.output == [tool_call_item] + assert response.output_text == "" + + def test_run_conversation_codex_plain_text(monkeypatch): agent = _build_agent(monkeypatch) monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: _codex_message_response("OK"))