fix(codex): tolerate None output responses - #32926
xXKillerNoobYT wants to merge 2 commits into
Conversation
Co-Authored-By: Paperclip <noreply@paperclip.ing>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Improves robustness of Codex Responses streaming/normalization when the upstream SDK returns output=None, and ensures tool payloads are omitted when no tools are available.
Changes:
- Omit
toolsfrom Codex request kwargs whentoolsisNone/empty. - Add safer
output_textaccess to prevent crashes whenresponse.outputisNone. - Harden streaming logic to retry/fallback when the SDK parser hits
output=None, with new tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/agent/transports/test_codex_transport.py | Adds regression test ensuring tools is omitted when not provided. |
| tests/agent/test_codex_runtime.py | Adds tests for stream retry/fallback behavior on output=None TypeError. |
| agent/transports/codex.py | Stops sending tools key when response_tools is empty/None. |
| agent/conversation_loop.py | Uses safe output text accessor in output_text fallback path. |
| agent/codex_runtime.py | Backfills output when missing/non-list; retries/falls back on SDK TypeError for output=None. |
| agent/codex_responses_adapter.py | Introduces _safe_get_response_output_text and uses it during normalization. |
| agent/auxiliary_client.py | Aligns output backfill condition with codex runtime behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Backfill from collected items or synthesize from deltas. | ||
| _out = getattr(final_response, "output", None) | ||
| if isinstance(_out, list) and not _out: | ||
| if not isinstance(_out, list) or not _out: |
| if not isinstance(_out, list) or not _out: | ||
| if collected_output_items: | ||
| terminal_response.output = list(collected_output_items) |
| # Backfill empty output from collected stream events | ||
| _output = getattr(final, "output", None) | ||
| if isinstance(_output, list) and not _output: | ||
| if not isinstance(_output, list) or not _output: |
| raise | ||
| except TypeError as exc: | ||
| err_text = str(exc) | ||
| sdk_output_none = "'NoneType' object is not iterable" in err_text |
| _interrupt_requested = False | ||
| _codex_streamed_text_parts = [] | ||
|
|
||
| def __init__(self): |
| from agent.codex_responses_adapter import _safe_get_response_output_text | ||
| _out_text = _safe_get_response_output_text(response) |
|
@NousResearch/hermes-agent maintainers — this PR (head 987e002) addresses the upstream Codex |
alt-glitch
added
type/bug
|
Thanks @alt-glitch — appreciated the triage. Happy to defer to the maintainer call on which vehicle to land. We will track #32884 alongside #32926 and accept whichever you all choose to squash-merge. The underlying root cause #11179 is the priority; either landing resolves the downstream consumer. Will not push further changes to #32926 unless maintainers prefer this branch — let us know if you'd like us to close ours in favor of #32884. |
|
Closing as duplicate — the Codex null-output fix has been merged via #32963 (cherry-picked from @carltonawong's PR #32890, the one Gille reviewed). Thanks for jumping on the outage so quickly; appreciate the help. Closes #11179. |
Summary
Verification
venv/bin/python -m pytest tests/agent/transports/test_codex_transport.py tests/agent/test_codex_runtime.py tests/agent/test_auxiliary_client.py::TestCodexAuxiliaryAdapterTimeout -qopenai-codex/gpt-5.5no longer aborts withTypeError: 'NoneType' object is not iterable; current upstream returns emptyresponse.outputand Hermes reports that as an invalid API response after retries instead of masking it as NoneType.