Follow-up to the salvaged #58696 (devatnull) + #41343 (annguyenNous)
commits: instead of fully suppressing commentary/analysis-phase stream
deltas, fire on_reasoning_delta so the CLI/gateway display them like
thinking text. Matches Codex CLI semantics where commentary is never
the turn's final answer, while keeping the narration visible in the
reasoning display. Adds devatnull to AUTHOR_MAP.
Self-review follow-up on the salvaged approval-routing fix.
The initial adaptation re-read os.getenv("HERMES_YOLO_MODE") at session-build
time. That diverges from the repo's security invariant: HERMES_YOLO_MODE is
frozen into tools.approval._YOLO_MODE_FROZEN at import time precisely so a skill
running mid-process cannot set the env var and instantly flip the approval
bypass (a prompt-injection escalation path). A live re-read re-opened that hole
for the codex routing path.
- Add tools.approval.is_approval_bypass_active() — the canonical three-source
bypass check (frozen --yolo/HERMES_YOLO_MODE + session /yolo + approvals.mode
off) in one place. This is the 4th inline copy of that OR-chain (the three
sites in approval.py and tui_gateway/server.py:3121 all use the same idiom);
the helper is the shared chokepoint they can collapse onto.
- codex_runtime.py now calls is_approval_bypass_active() instead of the
hand-rolled mode-or-session check plus a runtime env re-read.
- Update the env-yolo test to patch _YOLO_MODE_FROZEN (the canonical test
pattern, e.g. tests/tools/test_yolo_mode.py) rather than setenv, which is
dead-on-arrival against the frozen constant.
Fail-closed default preserved on every branch; 28 integration + 77 session/yolo
tests pass; E2E confirms the real exec decision flips decline->accept only when
bypass is active.
On gateway/cron/non-CLI contexts the codex app-server runtime has no UI to
surface codex's exec/apply_patch approval requests, so they fail closed
(silently decline) — the bot appears responsive but cannot write files, with
no approval prompt anywhere ("patch rejected by user").
When the user has explicitly opted out of Hermes approvals (approvals.mode: off,
the /yolo session toggle, or HERMES_YOLO_MODE=1), collapse to codex's own
sandbox permission profile (~/.codex/config.toml) as the policy gate by passing
_ServerRequestRouting(auto_approve_exec=True, auto_approve_apply_patch=True) to
the session. Defaults (manual/smart/unset) preserve the current fail-closed
behavior — a no-op for users who have not opted out.
Reads the mode via the canonical tools.approval._get_approval_mode() (which
already normalizes the YAML-1.1 bare-'off'->False case) at session-build time,
so a mid-session /yolo toggle is honored too.
5 integration tests: each opt-out mechanism (config off, YAML False, env var,
session yolo) plus the default fail-closed regression guard.
Closes#26530
Co-authored-by: snav <jake@nousresearch.com>
Follow-up correcting the salvaged fix's persistence approach to avoid a
duplicate user-message write (verified via E2E — the #860/#42039 bug class
the original diff aimed to avoid).
Root cause: in gateway mode the AIAgent is built WITH a session_db, so the
inbound user turn is already flushed at turn start (turn_context.
_persist_session). The original fix returned agent_persisted=False, making the
gateway re-write the whole new-message slice via append_to_transcript ->
append_message (a raw INSERT with no dedup), duplicating the already-flushed
user turn.
Corrected approach (single writer): run_codex_app_server_turn now flushes its
OWN projected assistant/tool messages via _flush_messages_to_session_db (which
dedups the already-persisted user turn through _DB_PERSISTED_MARKER) and
returns agent_persisted=True so the gateway skips its write. Net result:
session_search/distill see the full codex conversation, each message persisted
exactly once.
Adds regression coverage asserting exactly-once persistence on a real
SessionDB, agent_persisted=True, FTS visibility, and standard-runtime skip-db
behaviour preserved.
Co-authored-by: Lubos Buracinsky <lubos@komfi.health>
The codex_app_server runtime path (run_codex_app_server_turn in
agent/codex_runtime.py) is an early-return that bypasses
conversation_loop and never calls _flush_messages_to_session_db().
Meanwhile, gateway/run.py sets:
agent_persisted = self._session_db is not None # always True
and passes skip_db=agent_persisted to every append_to_transcript call,
assuming the agent self-persisted (correct for the standard runtime,
wrong for codex). The result: codex turn messages are persisted nowhere.
state.db accumulates only session_meta rows; session_search (full-text
search over state.db) and conversation-distill are blind to real gateway
conversations, causing 'the agent has no memory of what we discussed'.
Fix (three-part, all backward-compatible):
1. agent/codex_runtime.py — run_codex_app_server_turn success return
now includes 'agent_persisted': False, signalling that the codex path
did NOT self-persist its turn.
2. gateway/run.py — the agent_persisted assignment now reads:
agent_result.get('agent_persisted', self._session_db is not None)
For the standard runtime (which does not set the key) the default
(self._session_db is not None) preserves the existing skip-db
behaviour so no duplicate-write regression (#860 / #42039) occurs.
For the codex runtime the flag is False, so the gateway writes the
new turn's messages to state.db and FTS index.
3. gateway/run.py — the rebuilt result dict (run_agent return, which
becomes agent_result upstream) now includes agent_persisted passed
through from result_holder[0], with a safe True default. Without
this passthrough the flag set in step 1 was discarded when the result
was reconstructed, causing agent_result.get('agent_persisted', ...)
to always see the default True and never write codex turns.
When the main provider is the Codex app-server runtime (api_mode
codex_app_server), the gateway showed no verbose 'running X' tool-progress
breadcrumbs on Telegram while every other provider did. The app-server
session processes item/started notifications (command execution, file
changes, MCP/dynamic tool calls) but never surfaced them as Hermes
tool-progress events — the session was constructed without an on_event
hook, so the agent's tool_progress_callback was never invoked on this
route.
Add _codex_note_to_tool_progress() mapping item/started → (tool_name,
preview, args) for commandExecution / fileChange / mcpToolCall /
dynamicToolCall, and wire an on_event hook into CodexAppServerSession that
forwards mapped events to agent.tool_progress_callback('tool.started',
...) — the same signature the chat_completions path uses (tool_executor.py).
Non-tool items (agentMessage/reasoning) and non-item/started methods map
to None and are ignored.
Co-authored-by: jplew <462836+jplew@users.noreply.github.com>
Remove unused imports (F401) and duplicate/shadowed import
redefinitions (F811) across the codebase using ruff's safe
autofixes. No behavioral changes -- imports only.
- ~1400 safe autofixes applied across 644 files (net -1072 lines)
- __init__.py re-exports preserved (excluded from F401 removal so
public re-export surfaces stay intact)
- Re-exports that are imported or monkeypatched by tests but look
unused in their defining module are kept with explicit # noqa:
F401 (gateway/run.py load_dotenv; run_agent re-exports from
agent.message_sanitization, agent.context_compressor,
agent.retry_utils, agent.prompt_builder, agent.process_bootstrap,
agent.codex_responses_adapter)
- Unsafe F841 (unused-variable) fixes deliberately skipped -- those
can change behavior when the RHS has side effects
- ruff lints remain disabled in pyproject.toml (only PLW1514 is
selected); this is a one-time cleanup, not a config change
Verification:
- python -m compileall: clean
- pytest --collect-only: all 27161 tests collect (zero import errors)
- core entry points import clean (run_agent, model_tools, cli,
toolsets, hermes_state, batch_runner, gateway)
- static scan: every name any test imports directly from an edited
module still resolves
* refactor(codex): drop SDK responses.stream() helper; consume events directly
The OpenAI Python SDK's high-level `client.responses.stream(...)` helper
does post-hoc typed reconstruction from the terminal
`response.completed.response.output` field. The chatgpt.com Codex
backend has been observed (today, gpt-5.5) to ship `response.output =
null` on terminal frames, which crashes the SDK with `TypeError:
'NoneType' object is not iterable` mid-iteration.
Carlton's #32963 patched the symptom by wrapping the helper in
try/except and recovering from the same per-event accumulator the SDK
was supposed to populate. This PR removes the helper from the call
path entirely: we now use `client.responses.create(stream=True)` (raw
AsyncIterable of SSE events) and assemble the final response object
ourselves from `response.output_item.done` events as they arrive. The
terminal event's `output` field is never read for content. Same
strategy OpenClaw uses for the same backend.
This makes Hermes structurally immune to the bug class, not patched.
The next time OpenAI ships a shape change to chatgpt.com's terminal
frame, our consumer keeps working because it doesn't read that frame
for content — only for usage/status/id.
Changes
- `agent/codex_runtime.py`: new `_consume_codex_event_stream()` shared
consumer; `run_codex_stream()` uses `responses.create(stream=True)`;
`run_codex_create_stream_fallback()` collapses into a thin alias
since the primary path now does what the fallback used to do.
- `agent/auxiliary_client.py`: `_CodexCompletionsAdapter` uses the
same consumer; old null-output recovery helpers deleted as
unreferenced.
- Tests migrated: fixtures that mocked `responses.stream` now mock
`responses.create` returning a raw iterable. New regression test
asserts the auxiliary path returns streamed items even when the
terminal event's `output` is literally `null`.
Validation
- Live: tested against fresh OAuth on `chatgpt.com/backend-api/codex`
with `gpt-5.5` — response built correctly with `response.output=null`
on the terminal frame, all events consumed, usage/reasoning tokens
propagated.
- `tests/run_agent/test_run_agent_codex_responses.py` +
`tests/agent/test_auxiliary_client.py`: 242 passed.
* test+fix(codex): migrate streaming tests, raise on truncated streams
CI surfaced 10 test failures across tests/run_agent/test_streaming.py
and tests/run_agent/test_codex_xai_oauth_recovery.py — both files had
their own `responses.stream(...)` mocks I missed in the first sweep.
agent/codex_runtime.py: _consume_codex_event_stream() now raises
"Codex Responses stream did not emit a terminal response" when the
stream ends without any terminal frame AND no usable content. This
preserves the signal callers used to get from the SDK's high-level
helper, which they distinguished from "completed with empty body"
in error handling.
Tests migrated:
- test_streaming.py: text-delta callback, activity-touch, and
remote-protocol-error tests all switch from mocking responses.stream
to responses.create returning an iterable of events.
- test_codex_xai_oauth_recovery.py: prelude-error tests are recast as
wire-error-event tests (the new path raises _StreamErrorEvent
directly when the wire emits type=error, which is strictly better
than the old two-phase "SDK RuntimeError → retry → fallback"). The
retry-on-transport-error test moves from responses.stream side-effect
to responses.create side-effect.
Verified live against chatgpt.com Codex with gpt-5.5 — AIAgent.chat()
through the full codex_responses path returns correctly, 319/319
targeted tests passing.
The chatgpt.com/backend-api/codex endpoint has an intermittent failure mode
where it accepts the connection but never emits a single stream event — the
socket just hangs. Direct sequential probing reproduces it (0 events, no HTTP
status), and a fresh reconnect then succeeds in ~2s. Today the only guard is
the wall-clock stale timeout in interruptible_api_call, so a dead-on-arrival
connection is held for the full stale window (90-900s depending on context /
config) before the retry loop can reconnect — minutes of wasted wall time per
stall, at a rate of ~20% of calls during affected windows.
Add a TTFB watchdog scoped to the codex_responses path:
- codex_runtime.run_codex_stream stamps agent._codex_stream_last_event_ts on
*every* stream event (not just output-text deltas), so reasoning-only and
tool-call-only turns are not mistaken for a stall.
- interruptible_api_call resets that marker before the worker starts and, while
it is still None, kills the connection once elapsed exceeds the TTFB cutoff
(default 45s, tunable via HERMES_CODEX_TTFB_TIMEOUT_SECONDS, 0 disables). The
raised TimeoutError flows through the existing retry path unchanged.
Once any event has arrived the stream is healthy and only the existing
wall-clock stale timeout applies, so legitimate long generations are never
interrupted. Gated to codex_responses; the chat_completions non-stream,
anthropic and bedrock branches have no first-event signal and are untouched.
Adds tests/agent/test_codex_ttfb_watchdog.py covering the stall kill, the
events-flowing pass-through, and the env-disable path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Original commit 2b193907d by Teknium added a new module-level
_StreamErrorEvent class and threaded its raise into
_run_codex_create_stream_fallback in pre-refactor run_agent.py.
- _StreamErrorEvent class → run_agent.py (module-level, next to
_qwen_portal_headers; class needs to be top-level for the codex
runtime to import it)
- The fallback event-loop's 'type=error' handler → agent/codex_runtime.py
where run_codex_create_stream_fallback now lives. Imports
_StreamErrorEvent lazily from run_agent to avoid circular import.
Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
Original commit 31ba2b0cb by Teknium targeted run_codex_stream() at
its pre-refactor location in run_agent.py. Re-applied:
- Prelude error retry/fallback → agent/codex_runtime.py (in
run_codex_stream where the body now lives)
- _decorate_xai_entitlement_error helper + _summarize_api_error
wrapping → run_agent.py (these methods remained on AIAgent
as @staticmethod's; cherry-pick applied them cleanly)
The xai-oauth provider gate, encrypted_content drop on replay, etc.
landed in agent/codex_responses_adapter.py via the prior merge from main.
Closes#8133, #14634
Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
Four fixes from PR #27248 review:
1. **__init__ forwarder is now keyword-forwarded** (daimon-nous review).
Previously the run_agent.AIAgent.__init__ wrapper forwarded all 64
params positionally to agent.agent_init.init_agent, so adding a
65th param on main would require three lockstep edits (signature,
init_agent signature, forwarder call) or silently shift every value.
Keyword forwarding makes this trivially safe — adding a param now
only needs the two signatures and one extra keyword line.
2. **Drop dead _ra() in agent/codex_runtime.py** (daimon-nous + Copilot).
The lazy run_agent reference was defined but never called inside
this module — the codex paths use agent.* accessors only.
3. **Drop unused imports in agent/codex_runtime.py** (Copilot):
contextvars, threading, time, uuid, Optional. Carried over from
run_agent.py during the original extraction.
4. **Tighten three source-introspection test guards** (Copilot):
- test_memory_nudge_counter_hydration.py — was scanning the
concatenated source of run_agent.py + agent/conversation_loop.py
and matching self.X or agent.X form. Now asserts the
hydration block lives in agent/conversation_loop.py specifically
with the agent.X form — the body never moves back, so if it
ever drifts a future re-introduction fails the guard.
- test_run_agent.py::TestMemoryNudgeCounterPersistence — anchor on
agent.iteration_budget = IterationBudget exactly (was just
iteration_budget = IterationBudget) so an unrelated identifier
ending in iteration_budget can't match.
- test_run_agent.py::TestMemoryProviderTurnStart — assert the
agent._user_turn_count form directly (the extracted body uses
agent.X, not self.X — accepting either was a transitional fudge).
- test_jsondecodeerror_retryable.py — scan agent/conversation_loop.py
only, not the concatenation.
Not addressed in this commit:
* Pre-existing bugs in agent/tool_executor.py (heartbeat index
mismatch when calls are blocked, _current_tool clobber in result
loop, blocked-counted-as-completed in spinner summary, dead
result_preview computation). These were preserved byte-for-byte from
the original _execute_tool_calls_concurrent — worth a separate
follow-up PR with proper tests.
* _OpenAIProxy.__instancecheck__ concern — pre-existing, not flagged
by any of the original test patches (nothing actually does
isinstance(x, OpenAI) against the proxy instance).
* agent_init.py:949 mem_config potential NameError — pre-existing;
only triggers if _agent_cfg.get('memory', {}) itself raises, which
it can't with a stock dict.
tests/run_agent/ + tests/agent/: 4313 passed, 1 pre-existing
test_auxiliary_client failure (unchanged).
run_agent.py: 3821 -> 3937 lines (+116 from the keyword-forwarded
init call's verbosity). Final: 16083 -> 3937 (-12146, 75% reduction).
