fix(codex): bridge gateway approvals safely

[h4ck200711]

Summary

Bridge Codex app-server exec and file-change approval requests into Hermes' existing gateway approval queue when a live gateway session is available.

This preserves CLI approval behavior, keeps cron/non-interactive contexts fail-closed, and reuses the shared timeout, interrupt, cleanup, heartbeat, and approval-hook machinery.

Problem

run_codex_app_server_turn() currently only resolves the CLI approval callback. Gateway sessions therefore have no callback for Codex item/commandExecution/requestApproval and item/fileChange/requestApproval requests, so they are silently declined even though the user is present on Telegram, Discord, or Desktop.

This is the remaining interactive-gateway case discussed in #26530 and the draft approach in #27636. The merged YOLO/mode-off behavior does not provide an interactive approval bridge for manual or smart mode.

Implementation

• Prefer the existing CLI callback when one is installed.
• Otherwise, expose a callback only when the active session has a registered gateway notifier.
• Send requests through prompt_gateway_approval(), which:
  • applies hardline, sudo-stdin, and approvals.deny floors before any bypass;
  • honors mode-off, YOLO, permanent/session approval, and smart approval;
  • force-redacts command and description before crossing the gateway boundary;
  • delegates queueing, timeout, interruption, cleanup, and hooks to _await_gateway_decision();
  • fails closed on missing notifier, notification failure, timeout, or unresolved decisions.
• Apply unconditional command-policy floors before Codex's auto_approve_exec and auto_approve_apply_patch shortcuts.

Safety properties

• Hardline commands cannot be approved through YOLO/mode-off auto-routing.
• sudo -S guessing and user-defined deny rules remain unconditional.
• Gateway payload redaction uses force=True, independent of security.redact_secrets.
• No notifier means no interactive callback; requests remain fail-closed unless the existing explicit bypass routing applies.
• Permanent approval remains disabled for Codex exec and apply-patch requests.

Tests

410 passed, 1 deselected
ruff: passed
git diff --check: passed

The deselected test is TestDetectDangerousRm::test_nonrecursive_verification_artifact_cleanup_is_not_dangerous, which also fails on clean main on macOS because /tmp resolves through /private/tmp; it is unrelated to this change.

New regression coverage includes:

• Gateway queue approval and decision mapping
• Forced secret redaction
• Smart auto-approval without prompting
• Hardline denial
• Missing-notifier fail-closed behavior
• Hardline denial before Codex exec auto-approval
• Safe apply-patch auto-approval without a callback

Related work

• codex app-server tool calls fail closed on gateway with no surfaceable approval prompt #26530
• [codex] Bridge Codex app-server approvals to gateway sessions #27636
• fix(gateway): bridge Codex approvals to messaging platforms #25889
• fix(codex-app-server): honor approvals.mode/yolo for gateway-context approval routing (salvage of #26533 by @simpolism, closes #26530) #56534

[alt-glitch]

This was generated by AI during triage.

Related to open #25889 and #27636. This current-main bridge adds policy-floor and forced-redaction coverage, so it is competing/superset work rather than a duplicate; a maintainer should select or consolidate the implementation.

[teknium1]

Thanks for addressing a real gateway gap. Current origin/main only passes the CLI approval callback into CodexAppServerSession at agent/codex_runtime.py:643-650, while missing callbacks decline Codex exec and file-change requests at agent/transports/codex_app_server_session.py:875-937.

Problems

• tools/approval.py:2137-2140 returns a gateway "session" choice without calling approve_session(). The existing gateway path persists that choice at current-main tools/approval.py:2804-2805; without the same persistence, every later Codex approval will prompt again.
• tests/run_agent/test_codex_app_server_integration.py:86-175 tests the resolver directly, not the run_codex_app_server_turn() construction path that must carry gateway session context and notifier registration.

Suggested changes

• Persist a resolved "session" choice under codex_app_server; keep permanent approval disabled.
• Add a lifecycle-level test from gateway notifier registration through a Codex server approval request and response.

Automated hermes-sweeper review.

[teknium1]

A session decision is returned but never persisted. The normal gateway approval path calls approve_session(session_key, pattern_key) before returning success; please do the same here so one session approval covers subsequent Codex requests.

[h4ck200711]

Fixed — prompt_gateway_approval() now calls approve_session() (and approve_permanent()/save_permanent_allowlist() for the true-permanent branch) after a resolved session/always choice, matching the exact persistence convention every other approval call site in this module already uses. Added a lifecycle test that drives a real run_conversation() turn so codex_runtime constructs an actual CodexAppServerSession with the gateway-resolved callback wired in exactly as production does, and asserts a second exec-approval request for the same pattern does NOT re-prompt the notifier, proving persistence through the real construction path rather than just the resolver in isolation. Pushed in 78657ed28.

[h4ck200711]

Addressed both points from the hermes-sweeper review:

Persistence bug (real, confirmed): prompt_gateway_approval() checked is_approved(session_key, pattern_key) before prompting but never called approve_session() after a resolved session/always choice — so every subsequent Codex exec/apply_patch request in the same session re-prompted the user despite them having already approved it for the session. Fixed to match the identical persistence convention already used at every other approval call site in this file.

Test coverage gap: added test_gateway_session_approval_persists_through_full_turn_construction, which drives a real run_conversation() turn so codex_runtime constructs an actual CodexAppServerSession with the gateway-resolved approval_callback wired in exactly as production does (not a hand-built stand-in), simulates a mid-turn Codex exec-approval request through that stored callback, and asserts a second request for the same pattern does NOT re-prompt the notifier.

Test results: tests/run_agent/test_codex_app_server_integration.py 33/33 pass; tests/tools/test_approval.py 311/312 pass (the 1 failure is the same pre-existing macOS /tmp vs /private/tmp quirk already called out in this PR's description as failing on clean main too — unrelated to this change).

Pushed to this branch in 78657ed28.

Additional corroboration: I also carry an independently-written approval bridge for this exact same use case (Codex exec/apply_patch → gateway approval UI) on a separate deployment, predating this PR. Auditing it against this review's finding turned up the identical class of bug — it checks a notifier and awaits a decision, but never persisted a resolved approval either. Fixed there with the same approach (is_approved() short-circuit + approve_session()/approve_permanent() after resolution) and live-tested: 3 sequential approval requests for the same command → only the first one actually prompted, the other two correctly auto-approved from persisted state. Consistent, reproducible evidence that this class of bug is real in practice, not just a theoretical gap caught by static review.

[GottZ]

This was generated by AI during triage.

Summary

Four PRs address the missing Codex app-server approval path in gateway sessions: #25889 adds a transport fallback with a separate wait loop, #27636 adds a runtime callback bridge with another separate wait loop, #43217 is a closed shared-waiter implementation mixed with Telegram cache-path changes, and #66925 combines the runtime bridge with the shared waiter, policy floors, forced redaction, and session-choice persistence.

Related pull requests

• fix(gateway): bridge Codex approvals to messaging platforms #25889 related — (+257/-25) — n/a: Adds transport-level exec and apply-patch gateway fallback, but duplicates the queue wait loop and omits the shared interrupt handling. Despite its keep_open review, fix(codex): bridge gateway approvals safely #66925 implements that review's requested _await_gateway_decision() salvage path, and the author reports no further work is planned on fix(gateway): bridge Codex approvals to messaging platforms #25889 and accepts supersession once maintainers select the canonical target.
• [codex] Bridge Codex app-server approvals to gateway sessions #27636 related — (+330/-8) — n/a: Adds the runtime callback bridge and smart-approval coverage, but re-reads HERMES_YOLO_MODE and duplicates the wait loop without shared interruption handling. Despite the MAINTAINER-BOT and contributor keep_open reviews, fix(codex): bridge gateway approvals safely #66925 retains this callback approach while using is_approval_bypass_active() and _await_gateway_decision(), directly addressing their stated blockers.
• [codex] Route app-server approvals through gateway #43217 [closed] related — (+268/-3) — n/a: Closed reference implementation that bridges through _await_gateway_decision() and tests queue resolution, but also includes unrelated Telegram attachment/cache-path changes. Despite its earlier approved review, it remains relevant only as a shared-waiter reference because it is already closed and fix(codex): bridge gateway approvals safely #66925 carries the focused current-main approval work.
• fix(codex): bridge gateway approvals safely #66925 related — (+378/-41) — n/a: Bridges gateway approvals through the shared waiter, enforces hardline, sudo-stdin, and user-deny floors, force-redacts outbound approval data, and persists session choices. Its keep_open review identified missing persistence and lifecycle coverage; the shown diff addresses both with approve_session() and a full-turn construction test.

Duplicates

#25889 and #27636 substantially duplicate the same Codex-to-gateway bridge through different wiring points; #43217 is an earlier closed implementation of the shared core, while #66925 consolidates that work with additional policy and redaction safeguards.

Suggested consolidation

Keep #66925 open with a salvage path: retain its shared-waiter bridge, policy floors, forced redaction, session persistence, and lifecycle coverage while maintainers validate the updated head. Close #25889 as superseded by #66925 with the author's stated consent, close #27636 as a duplicate of #66925 despite its keep_open verdict because #66925's diff implements the verdict's frozen-bypass and shared-interrupt requirements, and keep #43217 closed as a historical reference.

Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 63 kB of PR diffs, 6 kB of issue/PR text, 11 kB of discussion (15 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.