Skip to content

fix: route codex payloads around the SDK's GIL-holding request transform (#93650) - #93681

Closed
kchernev wants to merge 1 commit into
NousResearch:mainfrom
kchernev:fix/codex-sdk-transform-freeze-93650
Closed

kchernev wants to merge 1 commit into
NousResearch:mainfrom
kchernev:fix/codex-sdk-transform-freeze-93650

Conversation

@kchernev

Copy link
Copy Markdown

Fixes #93650.

Problem

A codex_responses call can freeze the entire agent for hours before a single byte leaves the process. The OpenAI SDK's responses.create re-walks the whole request body against the ResponseCreateParams union graph (openai/_utils/_transform.py::maybe_transform) client-side, holding the GIL.

In the incident documented in #93650 (openai 2.24.0, CPython 3.11.16), one such walk over a ~1.4 MB / 1,093-item conversation wedged for 12.5 CPU-hours at ~99% of a core. Core-dump forensics (pystack + gdb, full stacks in the issue):

  • the worker held the GIL inside maybe_transform → isinstance(obj, Mapping) → typing.__subclasscheck__;
  • 9 other threads were captured in take_gil futex waits — including the TTFB/stale watchdog poller, parked at t.join(timeout=0.3), which never got to run any of its detectors;
  • reading the transform's list_iterator out of the core showed the walk mid-list (item 513 of 1,093) after 12.5h — progressing at ~88 s/item, vs 0.35 s for the whole payload in a fresh interpreter.

No in-process watchdog can rescue this: the hang starves them via the GIL, and even when they run their remedy is socket-level (_close_request_client_once) — a pre-network hang has no socket to kill, and Thread.join can't cancel the worker. The agent froze silently mid-task; in that incident it happened seconds before a scheduled Vast.ai instance teardown, which then billed all night.

Fix

Hermes assembles Codex payloads from JSON round-trips — they are already wire format, so the transform walk is pure overhead plus this failure mode. The SDK merges extra_body into the JSON body after the transform (_base_client._build_request, shallow merge), a mechanism this codebase already relies on for prompt_cache_retention. This PR routes the bulk payload fields (input, tools) through extra_body, skipping maybe_transform for them entirely while producing an identical request body.

Safety rails:

  • Plain-JSON guard — a field is only moved if every node is a plain JSON type (dict/list/str/int/float/bool/None with str keys). Pydantic models, generators, or anything else keep the typed SDK path, which still needs the transform. The guard is a single cheap C-level isinstance walk (~tens of ms on multi-MB payloads, less than the transform it replaces).
  • Caller precedence — an explicit caller-provided extra_body entry wins over a moved field, matching what the SDK's own post-transform merge would have resolved.
  • Escape hatch — HERMES_CODEX_SDK_TRANSFORM=1 restores the pre-fix behavior.

Applied at both live responses.create call sites: the primary stream path (codex_runtime._open_codex_stream) and the auxiliary adapter (auxiliary_client._CodexCompletionsAdapter), whose approval calls carry ~300K-token contexts through the same walk.

Tests

  • New tests/run_agent/test_codex_sdk_transform_bypass.py (10 tests): field movement, extra_body merge/precedence, non-JSON fallback to the typed path, string-input passthrough, env escape hatch, and an end-to-end run_codex_stream assertion that create receives the payload via extra_body.
  • test_run_agent_codex_responses.py::test_run_codex_stream_strips_nested_request_override_retention updated for the new wire shape (its contract — retention never crosses the wire — is asserted more precisely now).
  • test_native_compaction.py fake-client capture updated to accept the payload in either wire shape.
  • tests/run_agent/ suite: no regressions vs main (7 pre-existing environment-dependent failures reproduce identically on a clean main worktree; details on request).

Follow-up (out of scope here)

A process-external liveness check (e.g. gateway-side "worker thread >N min at ~100% CPU with no log progress → restart TUI") would harden against any future GIL-holding client-side hang, since no in-process watchdog can be trusted against that class. Happy to file separately.

🤖 Generated with Claude Code

…orm (NousResearch#93650)

responses.create re-walks the entire request body against the
ResponseCreateParams union graph client-side while holding the GIL.
NousResearch#93650 documents that walk wedging for 12+ hours on a ~1.4 MB
conversation, starving every other thread including the TTFB/stale
watchdogs — and no socket kill can unblock a pre-network hang.

Hermes payloads are JSON round-trips and already wire format, so the
bulk fields (input, tools) are now routed through extra_body, which the
SDK merges into the JSON body after the transform. Guarded by a
plain-JSON check (anything else keeps the typed path) and a
HERMES_CODEX_SDK_TRANSFORM=1 escape hatch. Applied to both the primary
stream path and the auxiliary adapter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API labels Aug 24, 2026
@kshitijk4poor

Copy link
Copy Markdown

Merged via #93773. Your commits were cherry-picked with authorship preserved.

What this PR does: Routes the bulk payload fields (input, tools) through extra_body instead of the typed SDK kwargs, bypassing the OpenAI SDK's maybe_transform walk that can wedge for hours holding the GIL on large Codex Responses API calls (#93650).

Salvage notes:

  • Cherry-picked your commit 3f92a1d onto latest main — authorship preserved
  • Added AUTHOR_MAP entry for kaloyan.chernev@aifnet.com → kchernev
  • Applied ruff format to the new test file
  • 245 targeted tests passed, 6/6 E2E tests with real imports
  • 3-agent parallel review (reuse/quality/efficiency) — zero blocking findings
  • ty type checker — no new errors introduced
  • CI all green (Python tests, e2e, ruff+ty, check-attribution, supply-chain scan)

Thank you for the thorough forensic analysis in #93650 and the well-crafted fix. The _is_plain_json_data guard and caller-precedence handling via setdefault were exactly right.

#93773

pull Bot pushed a commit to uk0/hermes-agent that referenced this pull request Aug 24, 2026
@kchernev
kchernev deleted the fix/codex-sdk-transform-freeze-93650 branch August 24, 2026 13:19
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround provider/openai OpenAI / Codex Responses API type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent-wide freeze: SDK maybe_transform can wedge for hours holding the GIL before the request is sent (codex_responses)

3 participants