From 486d632cc2d1d6e22bb50d22787e71cdcecfaeec Mon Sep 17 00:00:00 2001
From: teknium1 <127238744+teknium1@users.noreply.github.com>
Date: Wed, 27 May 2026 11:01:47 -0700
Subject: [PATCH] fix(auxiliary): coerce None final.output to empty list in
 Codex aux adapter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #33368.

`_CodexCompletionsAdapter.create()` iterates `final.output` from the
Codex Responses stream. The event-driven consumer (introduced in #33042)
always sets `final.output` to a list, so this shape can't come from our
own code path. But:

- Mocked clients in tests can return a typed Response with `output=None`
- Third-party shims / compatibility layers that bypass the consumer can
  do the same
- A future code path that wraps a different consumer could regress

The old code `getattr(final, "output", [])` returns `None` (not the
default `[]`) when the attribute EXISTS but is `None`. Iterating
`None` then raises `TypeError: 'NoneType' object is not iterable` —
the exact error logged by title-generation when this fires.

Fix: `getattr(final, "output", None) or []` — single-line defensive
coerce. Cheap; zero risk.

Regression test asserts the auxiliary path handles a final whose
`.output` is `None` (via monkey-patched consumer) without raising and
returns the expected chat.completions-shaped response.

Reporter: @pavegrid-1 (issue #33368).
---
 agent/auxiliary_client.py            |  2 +-
 tests/agent/test_auxiliary_client.py | 57 ++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py
index 233c299758c..1e6abb779e8 100644
--- a/agent/auxiliary_client.py
+++ b/agent/auxiliary_client.py
@@ -828,7 +828,7 @@ class _CodexCompletionsAdapter:
                     val = obj.get(key, default)
                 return val if val is not None else default
 
-            for item in getattr(final, "output", []):
+            for item in (getattr(final, "output", None) or []):
                 item_type = _item_get(item, "type")
                 if item_type == "message":
                     for part in (_item_get(item, "content") or []):
diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py
index 7e4ddcae133..07d3688272c 100644
--- a/tests/agent/test_auxiliary_client.py
+++ b/tests/agent/test_auxiliary_client.py
@@ -2591,6 +2591,63 @@ class TestCodexAuxiliaryAdapterNullOutputRecovery:
 
         assert response.choices[0].message.content == "aux survived"
 
+    def test_handles_final_output_is_none_after_consumer(self):
+        """Regression for #33368 — defense against ``final.output`` being ``None``.
+
+        The event-driven consumer always sets ``final.output`` to a list, so this
+        shape can't come from our own path. But a mocked client / compatibility
+        shim that returns a typed Response with ``output=None`` directly (or a
+        future code path that wraps a different consumer) would crash on
+        ``for item in getattr(final, "output", [])`` because ``getattr`` returns
+        ``None`` (not the default) when the attribute exists but is ``None``.
+        Coerce with ``or []`` to handle this defensively.
+        """
+        # Stream that returns no items but a terminal with output=None.
+        # The consumer assembles an empty list. We then mock the consumer's
+        # return to simulate a third-party path that returns final.output=None.
+        empty_events = [
+            SimpleNamespace(type="response.completed", response=SimpleNamespace(
+                status="completed", id="r", output=None, usage=None,
+            )),
+        ]
+
+        class _Stream:
+            def __iter__(self): return iter(empty_events)
+            def close(self): pass
+
+        # Monkey-patch the consumer to return a final whose .output is None
+        # (mimics third-party shim behavior the defensive guard protects against).
+        from agent import codex_runtime
+        original_consume = codex_runtime._consume_codex_event_stream
+
+        def _consume_returning_none_output(*args, **kwargs):
+            return SimpleNamespace(
+                output=None,  # the defensive guard target
+                output_text="",
+                usage=None,
+                status="completed",
+                id="r",
+                model=kwargs.get("model"),
+                incomplete_details=None,
+                error=None,
+            )
+
+        codex_runtime._consume_codex_event_stream = _consume_returning_none_output
+        try:
+            class FakeResponses:
+                def create(self, **kwargs):
+                    return _Stream()
+
+            fake_client = SimpleNamespace(responses=FakeResponses())
+            adapter = _CodexCompletionsAdapter(fake_client, "gpt-5.5")
+
+            # Should not raise TypeError: 'NoneType' object is not iterable
+            response = adapter.create(messages=[{"role": "user", "content": "x"}])
+            assert response.choices[0].message.content is None
+            assert response.choices[0].finish_reason == "stop"
+        finally:
+            codex_runtime._consume_codex_event_stream = original_consume
+
 
 # ---------------------------------------------------------------------------
 # Issue #23432 — auxiliary timeout poisons cached client; later aux calls fail