Skip to content

[codex] Bridge Codex app-server approvals to gateway sessions - #27636

Draft
rf-openclaw-bot wants to merge 2 commits into
NousResearch:mainfrom
rf-openclaw-bot:codex/codex-app-server-gateway-approval
Draft

[codex] Bridge Codex app-server approvals to gateway sessions#27636
rf-openclaw-bot wants to merge 2 commits into
NousResearch:mainfrom
rf-openclaw-bot:codex/codex-app-server-gateway-approval

Conversation

@rf-openclaw-bot

Copy link
Copy Markdown

Summary

  • add a public gateway approval prompt helper that blocks on the existing per-session gateway approval queue
  • resolve Codex app-server approval callbacks from either the terminal prompt-toolkit callback or an active gateway notifier
  • add regression coverage for gateway-backed Codex approval callbacks

Root cause

Codex app-server approval requests only checked tools.terminal_tool._get_approval_callback(). Gateway sessions register approval notifiers in tools.approval instead, so Telegram/Discord contexts had no callback and failed closed without surfacing a prompt.

Validation

  • /home/leon/.hermes/hermes-agent/venv/bin/python -m pytest tests/run_agent/test_codex_app_server_integration.py tests/agent/transports/test_codex_app_server_session.py -q
  • git diff --check

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels May 17, 2026
@alt-glitch

alt-glitch commented May 17, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to open #25889 and #66925. All bridge Codex approvals to gateway sessions, but this branch adds a callback resolver and queue helper; #66925 adds policy-floor and payload-redaction coverage. Maintainer selection is needed.

@teknium1

teknium1 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix — the premise still holds on current main. agent/codex_runtime.py:199-206 only wires tools.terminal_tool._get_approval_callback() into CodexAppServerSession, while Codex server requests decline when self._approval_callback is missing in agent/transports/codex_app_server_session.py:709-718 and :723-759. The gateway already registers the notifier queue around agent.run_conversation() in gateway/run.py:14285-14521, so bridging to tools.approval is the right bug class.

Problems

  • tools/approval.py:609 in this PR re-reads os.getenv("HERMES_YOLO_MODE") at approval time. Current main intentionally freezes that env var at import (tools/approval.py:26-29) and uses _YOLO_MODE_FROZEN in the command guard (tools/approval.py:1065-1068) so in-process code cannot flip the env var and bypass approvals.

Suggested changes

  • Use the same frozen YOLO check as check_dangerous_command, while keeping is_current_session_yolo_enabled() for gateway /yolo.
  • Keep the added regression coverage; the callback and smart-approval tests in tests/run_agent/test_codex_app_server_integration.py:87-159 cover the missing path well.

Automated hermes-sweeper review.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing a real gateway gap. Current main still gives Codex only the terminal-thread callback (agent/codex_runtime.py:346-353), while missing callbacks decline exec/apply-patch requests (agent/transports/codex_app_server_session.py:873-923). The merged bypass-only work in #56534 does not implement manual/smart gateway prompting.

Problems

  • tools/approval.py:609 re-reads HERMES_YOLO_MODE at request time. Current main freezes it at import (tools/approval.py:32-35) and centralizes bypass logic in is_approval_bypass_active() (tools/approval.py:1858-1876) to prevent in-process approval bypass.
  • The new wait loop duplicates _await_gateway_decision() but omits its interrupt handling (tools/approval.py:2513-2529), so /stop or /new can leave a Codex approval blocked until timeout.

Suggested changes

  • Salvage the callback bridge onto current main, but route the gateway request through the existing shared approval wait/helper so queue cleanup, hooks, heartbeats, and interrupts remain uniform.
  • Use the frozen bypass helper rather than a live environment reread, and add an interrupted-wait regression test.

Automated hermes-sweeper review.

Comment thread tools/approval.py
session_key = get_current_session_key()
all_keys = list(pattern_keys or [pattern_key])

if is_truthy_value(os.getenv("HERMES_YOLO_MODE")) or is_current_session_yolo_enabled():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HERMES_YOLO_MODE is intentionally frozen at import on current main (tools/approval.py:32-35) so an in-process skill cannot enable approval bypass after startup. Please use the canonical frozen bypass path (is_approval_bypass_active() on current main) rather than reading this environment variable during an approval request.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@h4ck200711

Copy link
Copy Markdown

I opened #66925 as a current-main, hardened implementation of this gateway approval bridge. It reuses the shared approval queue/wait loop, force-redacts gateway payloads, preserves smart/manual behavior, and applies hardline, sudo-stdin, and user deny policy floors before Codex auto-approval. The focused Codex/approval suites pass with the one documented pre-existing macOS baseline failure excluded.

@alt-glitch alt-glitch added codex provider/openai OpenAI / Codex Responses API needs-decision Awaiting maintainer decision before any implementation comp/tools Tool registry, model_tools, toolsets and removed sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 18, 2026
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
@alt-glitch alt-glitch removed area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/tools Tool registry, model_tools, toolsets needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have provider/openai OpenAI / Codex Responses API sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants