fix(codex-app-server): honor approvals.mode/yolo for gateway-context approval routing - #26533
Closed
simpolism wants to merge 1 commit into
Closed
Conversation
…approval routing When Hermes runs on the codex_app_server runtime in a gateway/cron context (Discord, Telegram, scheduled job, etc.), codex's exec / apply_patch approval requests fail closed (silent decline) because `_get_approval_callback()` returns None — only the interactive CLI thread installs a callback through `tools.terminal_tool`. The codex session in `agent.transports.codex_app_server_session._decide_*` calls default to `decline` when no callback is wired. Symptom from the user side: ask the codex-runtime bot to write a file, they appear to think about it, codex returns "patch rejected by user" with no surfaced approval prompt anywhere. From the user's POV nothing happened. They have no way to grant approval because there's no UI attached. The bot is effectively read-only on the gateway path. This is double-gating with a missing second gate: codex's own sandbox permission profile (`:read-only` / `:workspace` / `:danger-no-sandbox` in `~/.codex/config.toml`) is the user-configurable filesystem boundary; Hermes' approval router was meant to add a per-command interactive checkpoint on top. With no UI, the second gate doesn't ask, just denies. Fix: when the user has explicitly opted out of Hermes approvals, collapse to codex's sandbox-only gating. Specifically, auto-approve codex's exec / apply_patch requests at the Hermes session layer when any of these are true: - `approvals.mode: off` in the user's config.yaml - YAML 1.1 unquoted `approvals.mode: off` (parsed as `False`) — handled via `_normalize_approval_mode()` to match the rest of the approval subsystem - HERMES_YOLO_MODE=1 env var - The current session has `/yolo` toggled on (via `is_current_session_yolo_enabled()`) Default behavior (`approvals.mode: manual` or unset) is unchanged: codex requests still fail closed in gateway contexts. Users on the interactive CLI continue to see approval prompts as before, because that path goes through the wired `_approval_callback` and never reaches the auto-approve fast-path. The trust escalation is gated by Hermes' existing approval-bypass mechanisms, so users who explicitly turned off approvals for Hermes- native tools now get consistent behavior for codex-runtime tools too. This is a semantics extension worth flagging in release notes: "approvals.mode: off now also applies to codex app-server tool requests." Verified RED→GREEN with 5 new integration tests in test_codex_app_server_integration.py: - approvals.mode: "off" → auto-approve ON - approvals.mode: False (YAML) → auto-approve ON - approvals.mode: "manual" → auto-approve OFF (fail-closed preserved) - HERMES_YOLO_MODE=1 → auto-approve ON - /yolo session toggle → auto-approve ON `pytest tests/run_agent/test_codex_app_server_integration.py tests/hermes_cli/test_codex_runtime_*.py`: 117 passed.
alt-glitch
added
type/bug
simpolism
added a commit
to simpolism/hermes-agent
that referenced
this pull request
May 15, 2026
…progress display
When Hermes runs on the codex_app_server runtime, the agent loop is owned
by the codex CLI subprocess instead of run_agent.py — so the
`progress_callback("tool.started", ...)` / `("tool.completed", ...)`
events Hermes' display path expects (gateway tool-progress bubbles, CLI
activity feed) never fire. Codex-runtime turns appear opaque from the
user side: the bot takes a long time and then a message lands, with no
indication that shell commands, file edits, or web searches ran in
between.
CodexAppServerSession's `on_event(note: dict)` hook already gets the
raw codex notification stream — added when the runtime originally
landed for kawaii spinner ticks — but run_agent.py wasn't passing it in.
The only missing piece was a shape adapter between codex's `note: dict`
events and the agent's `tool_progress_callback(event_type, tool_name,
preview, args)` signature.
This PR adds `agent/transports/codex_event_display.py` providing
`make_progress_bridge(get_progress_callback) -> on_event_adapter`, and
wires it up at the one site in run_agent.py that constructs the codex
session.
Mapping (item type → display name):
| Codex item type | Display name | Notes |
|--------------------|-------------------------|------------------------------------|
| commandExecution | exec_command | matches codex_event_projector |
| fileChange | apply_patch | matches codex_event_projector |
| mcpToolCall | mcp.<server>.<tool> | user MCP servers |
| mcpToolCall | <tool> | server="hermes-tools" (see below) |
| dynamicToolCall | <tool> | matches codex_event_projector |
| webSearch | web_search | codex built-in tool |
| reasoning, agentMessage, userMessage | (skipped) | not tool calls |
Special case for the "hermes-tools" MCP server: codex calls back through
it for web_search/browser_*/vision_analyze/etc. The inner dispatch runs
in a separate hermes-tools-mcp-server subprocess that does NOT have
access to the parent agent's tool_progress_callback — so the inner call
can never surface its own native progress event. The codex-level
mcpToolCall event IS the display event for that call, and we drop the
mcp.hermes-tools.* namespacing so users see "web_search" rather than
"mcp.hermes-tools.web_search" — matching how they think about these
tools.
Streaming output deltas (item/<type>/outputDelta, etc.) are ignored;
only item/started + item/completed produce progress events. Matches the
native HA UX where users don't see streaming stdout for shell tools.
Naming convention matches CodexEventProjector exactly (exec_command,
apply_patch, mcp.{server}.{tool}, ...) so traces, memory review, and
display all see the same tool names.
Late-binding via getter (not captured callback)
================================================
`CodexAppServerSession` is cached on the AIAgent across turns, but the
gateway's `progress_callback` is per-turn — each turn has its own
progress queue, dedup state, and cleanup tracking. If the bridge
captured the callback once at session construction, turn N+1's tool
events would fire into turn N's dead queue and the user would see tool
bubbles only on the first tool-using turn, then nothing.
`make_progress_bridge` takes a zero-arg getter
(`lambda: self.tool_progress_callback`) and dereferences on every
event. The session lives across turns; the getter dereference picks up
whatever progress_callback the gateway has installed for the current
turn.
This was caught by live Discord testing on May 15 2026: first tool turn
after HermesCodex gateway restart rendered bubbles, second turn was
silent. The two regression tests
(test_callback_swap_between_events_is_observed in the unit suite and
test_on_event_late_binds_per_turn_progress_callback in the integration
suite) demonstrate RED→GREEN.
Defensive design
================
- on_event invocations wrapped in try/except so a buggy progress
callback can never crash the codex transport read loop.
- Getter invocation also wrapped — if the agent attribute disappears
during teardown, the bridge stays silent.
- Bridge returns silently when the getter returns None (gateway
context without display, batch processing, tests).
- Malformed notifications (missing method, item not a dict, unknown
item type) silently dropped.
The bridge runs on the agent's main thread (called from inside
CodexAppServerSession.run_turn via _client.take_notification), so no
locking is needed.
Bonus fix: approval-drain on_event mirror
=========================================
Found during co-review: the approval-drain loop in
CodexAppServerSession.run_turn (line ~458) drained pending notifications
to keep _pending_file_changes current for fileChange approvals, but
it never forwarded those notifications to on_event. With this PR's
display bridge wired in, any tool bubbles around an approval would
silently disappear. Mirror the main notification-handling block's
on_event invocation in the drain loop too. Regression test
test_on_event_fires_during_approval_drain in
test_codex_app_server_session.py demonstrates RED→GREEN.
Tests
=====
- agent/transports/test_codex_event_display.py (NEW): 19 unit tests
covering the 5 surfaced item types, the hermes-tools renaming rule,
ignored events (reasoning/agentMessage/turn/delta), defensive paths
(None getter, throwing callback, throwing getter, malformed notes),
start/complete pairing, and the per-turn callback late-binding
regression.
- run_agent/test_codex_app_server_integration.py: +2 tests — wiring
test that captures CodexAppServerSession kwargs to verify run_agent.py
passes on_event correctly + drives a synthetic notification end-to-
end; per-turn late-binding regression test that simulates the live
Discord failure (cached session, swapped tool_progress_callback).
- agent/transports/test_codex_app_server_session.py: +1 regression test
(test_on_event_fires_during_approval_drain) for the drain-loop fix.
`pytest tests/agent/transports/ tests/run_agent/test_codex_app_server_integration.py tests/hermes_cli/test_codex_runtime_*.py`:
428 passed.
Note on adjacency to NousResearch#26533
===========================
Both touch the CodexAppServerSession() constructor in run_agent.py with
adjacent kwargs (NousResearch#26533 adds `request_routing=`, this adds `on_event=`).
They don't logically depend on each other and the second to merge
rebases cleanly.
Co-review
=========
Three review iterations with Codex:
1. First pass caught the approval-drain on_event bypass (now fixed +
regression test).
2. First pass also caught the wrong assumption that hermes-tools
mcpToolCall events should be suppressed — the inner subprocess
can't fire tool_progress_callback at all, so suppression made those
tools invisible. Changed to emit bare tool name. (now fixed +
regression test updated)
3. Live-testing pass on Discord caught the per-turn callback staleness
bug — bridge captured the callback at session construction instead
of late-binding (now fixed + 3 regression tests, all verified
RED→GREEN with `git stash`).
austinpickett
requested changes
May 19, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Codex app-server approval handling in non-interactive contexts (gateway/cron), ensuring codex exec/apply_patch requests don’t silently fail-closed when Hermes approvals have been explicitly disabled (via approvals.mode: off, HERMES_YOLO_MODE=1, or the /yolo session toggle).
Changes:
- Read and normalize
approvals.modeduring agent init and derive a cached “auto-approve codex server requests” flag. - When spawning a
CodexAppServerSession, pass_ServerRequestRouting(auto_approve_exec/apply_patch=...)based on approvals-off / yolo signals. - Add integration tests covering config
"off", YAML booleanFalse, default/manual fail-closed, env var yolo, and session yolo.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
run_agent.py |
Normalizes approvals config and routes codex app-server approval requests to auto-approve when the user has opted out of Hermes approvals. |
tests/run_agent/test_codex_app_server_integration.py |
Adds integration tests validating the new codex app-server approval routing behavior across config/env/session toggles. |
Comments suppressed due to low confidence (1)
run_agent.py:15773
_auto_approve_requestsis only computed when the CodexAppServerSession is first created, but the session is intentionally reused across turns. That means a user enabling/yolo(or setting/unsettingHERMES_YOLO_MODE) after the first codex turn won’t affect approval routing until the codex session is dropped/recreated. If/yolois meant to be a session-scoped toggle that takes effect for subsequent turns, consider recomputing this per-turn and updating routing (or recreating the session when the desired routing changes).
# Lazy session: one CodexAppServerSession per AIAgent instance.
# Spawned on first turn, reused across turns, closed at AIAgent
# shutdown (see _cleanup hook).
if not hasattr(self, "_codex_session") or self._codex_session is None:
cwd = getattr(self, "session_cwd", None) or os.getcwd()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pass | ||
| _auto_approve_requests = ( | ||
| _auto_approve_requests | ||
| or os.getenv("HERMES_YOLO_MODE", "").lower() in {"1", "true", "yes", "on"} |
|
@simpolism i can take over on this if you don't have bandwidth |
Collaborator
|
Merged via #56534 (#56534) — your commit was cherry-picked with authorship preserved (rebase-merge, Your fix targeted One follow-up on top ( Thanks for the thorough root-cause writeup and tests — made this a clean salvage. |
hashbender
added a commit
to hashbender/hermes-agent
that referenced
this pull request
Jul 1, 2026
…approval routing (salvage of NousResearch#26533 by @simpolism, closes NousResearch#26530) (#448) Co-authored-by: qbit-mirror-bot <qbit-mirror-bot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #26530
What
When Hermes runs the codex_app_server runtime on a gateway / cron / non-CLI context (Discord, Telegram, Slack, scheduled jobs, etc.), codex's exec and apply_patch approval requests now respect
approvals.mode: off, the/yolosession toggle, andHERMES_YOLO_MODE=1instead of silently failing closed.Why
Symptom from the user's side: ask a codex-runtime Discord bot to write a file. Bot responds with "patch rejected by user" (or no output at all for shell exec). No approval prompt appears anywhere. From the user's POV, the bot is silently read-only with no path to grant approval.
The bug is in
run_agent.py:_dispatch_codex_app_server_turn()— it setsapproval_callbackfromtools.terminal_tool._get_approval_callback()which only gets installed by the interactive CLI thread. Gateway/cron paths getNone, and the session falls back to its own"decline"default for safety. With no UI surface to ask through, the second gate (Hermes approval router) silently denies everything.This is double-gating with a missing second gate: codex's own sandbox permission profile (
:read-only/:workspace/:danger-no-sandboxin~/.codex/config.toml) is the user-configurable filesystem boundary; Hermes' approval router was meant to add a per-command interactive checkpoint on top of codex's sandbox. With no UI, the second gate doesn't ask — just denies.How
When the user has explicitly opted out of Hermes approvals through any documented mechanism, pass
request_routing=_ServerRequestRouting(auto_approve_exec=True, auto_approve_apply_patch=True)to the session so codex's approval requests don't enter the Hermes approval flow at all. Codex's own sandbox profile remains the active policy gate.The opt-out mechanisms honored:
approvals.mode: "off"in~/.hermes/config.yaml(string)approvals.mode: false(YAML 1.1 parses unquotedoffasFalse) — handled via_normalize_approval_mode()to match the rest of the approval subsystemHERMES_YOLO_MODE=1environment variable/yolosession toggle (is_current_session_yolo_enabled()checked at session-spawn time so mid-session toggles take effect on next codex turn)Defaults are unchanged:
approvals.mode: manual(or unset) → fail-closed preserved. Users on the interactive CLI continue to see approval prompts as before because that path goes through the wired_approval_callbackand never reaches the auto-approve fast-path.Semantics note
This is a semantics extension worth flagging in release notes:
approvals.mode: offnow also applies to codex app-server tool requests. Users who explicitly turned off approvals for Hermes-native tools now get consistent behavior for codex-runtime tools too. This is what users probably wanted but may not have specifically considered.Tests
5 new integration tests in
tests/run_agent/test_codex_app_server_integration.py:test_approvals_mode_off_auto_approves_codex_server_requests— string"off"→ auto-approve ONtest_yaml_boolean_false_approval_mode_also_auto_approves— YAML 1.1False→ auto-approve ONtest_manual_approvals_keep_codex_server_requests_fail_closed— default → auto-approve OFF (regression coverage for the default path)test_hermes_yolo_env_auto_approves_codex_server_requests— env var pathtest_session_yolo_auto_approves_codex_server_requests— runtime toggle pathWhy not just wire the gateway approval callback?
Routing codex's exec/apply_patch approvals through Discord-side messages (e.g.
/approveand/denyslash commands) is a real feature but a much bigger one — codex's approvals are per-tool-call and would need async correlation between the codex turn loop and the gateway's message lifecycle. Worth a separate issue + PR. The "honor approvals.mode/yolo" fix unblocks the immediate "my bot is silently read-only" case and is something users can take action on today.Scope
Single-commit, two-file change. Production code change is ~35 lines (initialization read of approvals config + session-spawn-time fold of the three opt-out mechanisms). Existing tests for the manual-approval default-fail-closed behavior remain green; regression coverage is added in the new tests.