fix(codex-app-server): honor approvals.mode/yolo for gateway-context approval routing
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>
This commit is contained in:
snav
kshitij
parent
9be292f1e6
commit
0b8e81996f
2 changed files with 166 additions and 1 deletions
|
|
@ -244,7 +244,10 @@ def run_codex_app_server_turn(
|
|||
Called from run_conversation() when agent.api_mode == "codex_app_server".
|
||||
Returns the same dict shape as the chat_completions path.
|
||||
"""
|
||||
from agent.transports.codex_app_server_session import CodexAppServerSession
|
||||
from agent.transports.codex_app_server_session import (
|
||||
CodexAppServerSession,
|
||||
_ServerRequestRouting,
|
||||
)
|
||||
|
||||
# Lazy session: one CodexAppServerSession per AIAgent instance.
|
||||
# Spawned on first turn, reused across turns, closed at AIAgent
|
||||
|
|
@ -262,6 +265,37 @@ def run_codex_app_server_turn(
|
|||
except Exception:
|
||||
approval_callback = None
|
||||
|
||||
# Gateway / cron contexts have no UI to surface codex's approval
|
||||
# requests through, so codex app-server exec / apply_patch requests
|
||||
# fail closed (silently decline) by default. When the user has
|
||||
# explicitly opted out of Hermes approvals — via `approvals.mode: off`
|
||||
# in config, the /yolo session toggle, or HERMES_YOLO_MODE=1 — honor
|
||||
# that and let codex's own sandbox permission profile
|
||||
# (~/.codex/config.toml) be the policy gate instead of double-gating
|
||||
# with a missing Hermes UI. Defaults (manual/smart/unset) preserve the
|
||||
# current fail-closed behavior — this is a no-op for those users.
|
||||
auto_approve_requests = False
|
||||
try:
|
||||
from tools.approval import (
|
||||
_get_approval_mode,
|
||||
is_current_session_yolo_enabled,
|
||||
)
|
||||
|
||||
auto_approve_requests = (
|
||||
_get_approval_mode() == "off"
|
||||
or is_current_session_yolo_enabled()
|
||||
)
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"codex app-server: approval-mode lookup failed; "
|
||||
"keeping fail-closed default",
|
||||
exc_info=True,
|
||||
)
|
||||
if not auto_approve_requests:
|
||||
auto_approve_requests = os.getenv(
|
||||
"HERMES_YOLO_MODE", ""
|
||||
).strip().lower() in {"1", "true", "yes", "on"}
|
||||
|
||||
def _on_codex_event(note: dict) -> None:
|
||||
# Bridge Codex app-server item/started notifications to Hermes
|
||||
# tool-progress so gateways show verbose "running X" breadcrumbs
|
||||
|
|
@ -281,6 +315,10 @@ def run_codex_app_server_turn(
|
|||
agent._codex_session = CodexAppServerSession(
|
||||
cwd=cwd,
|
||||
approval_callback=approval_callback,
|
||||
request_routing=_ServerRequestRouting(
|
||||
auto_approve_exec=auto_approve_requests,
|
||||
auto_approve_apply_patch=auto_approve_requests,
|
||||
),
|
||||
on_event=_on_codex_event,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -326,6 +326,133 @@ class TestRunConversationCodexPath:
|
|||
|
||||
assert captured["cwd"] == str(tmp_path)
|
||||
|
||||
def _capture_routing_agent(self, monkeypatch):
|
||||
"""Build a codex agent with a CodexAppServerSession stub that captures
|
||||
the request_routing passed at construction time, so we can assert how
|
||||
the gateway-context approval routing was resolved."""
|
||||
captured: dict = {}
|
||||
|
||||
def fake_init(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
self._thread_id = "thread-stub-1"
|
||||
|
||||
def fake_run_turn(self, user_input: str, **kwargs):
|
||||
return TurnResult(
|
||||
final_text="ok",
|
||||
projected_messages=[{"role": "assistant", "content": "ok"}],
|
||||
turn_id="turn-stub-1",
|
||||
thread_id="thread-stub-1",
|
||||
)
|
||||
|
||||
monkeypatch.setattr(CodexAppServerSession, "__init__", fake_init)
|
||||
monkeypatch.setattr(CodexAppServerSession, "run_turn", fake_run_turn)
|
||||
monkeypatch.setattr(
|
||||
CodexAppServerSession, "ensure_started", lambda self: "thread-stub-1"
|
||||
)
|
||||
return captured
|
||||
|
||||
def test_approvals_mode_off_auto_approves_codex_server_requests(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""When the user disables Hermes approvals, codex app-server approval
|
||||
requests should not fail closed just because no interactive callback is
|
||||
wired (the typical gateway path). Codex's own sandbox permission
|
||||
profile remains the filesystem boundary."""
|
||||
captured = self._capture_routing_agent(monkeypatch)
|
||||
with patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value={"approvals": {"mode": "off"}},
|
||||
):
|
||||
agent = _make_codex_agent()
|
||||
with patch.object(
|
||||
agent, "_spawn_background_review", return_value=None
|
||||
):
|
||||
agent.run_conversation("write something")
|
||||
routing = captured["request_routing"]
|
||||
assert routing.auto_approve_exec is True
|
||||
assert routing.auto_approve_apply_patch is True
|
||||
|
||||
def test_yaml_boolean_false_approval_mode_also_auto_approves(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""YAML 1.1 parses unquoted `off` as False; match the normal approval
|
||||
subsystem's compatibility behavior for codex app-server routing too."""
|
||||
captured = self._capture_routing_agent(monkeypatch)
|
||||
with patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value={"approvals": {"mode": False}},
|
||||
):
|
||||
agent = _make_codex_agent()
|
||||
with patch.object(
|
||||
agent, "_spawn_background_review", return_value=None
|
||||
):
|
||||
agent.run_conversation("write something")
|
||||
routing = captured["request_routing"]
|
||||
assert routing.auto_approve_exec is True
|
||||
assert routing.auto_approve_apply_patch is True
|
||||
|
||||
def test_manual_approvals_keep_codex_server_requests_fail_closed(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""Default (manual) approvals must preserve the fail-closed behavior —
|
||||
this fix is a no-op for users who haven't opted out."""
|
||||
captured = self._capture_routing_agent(monkeypatch)
|
||||
with patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value={"approvals": {"mode": "manual"}},
|
||||
):
|
||||
agent = _make_codex_agent()
|
||||
with patch.object(
|
||||
agent, "_spawn_background_review", return_value=None
|
||||
):
|
||||
agent.run_conversation("write something")
|
||||
routing = captured["request_routing"]
|
||||
assert routing.auto_approve_exec is False
|
||||
assert routing.auto_approve_apply_patch is False
|
||||
|
||||
def test_hermes_yolo_env_auto_approves_codex_server_requests(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""HERMES_YOLO_MODE should flow through to codex app-server routing so
|
||||
gateway/cron contexts do not fail closed when the user explicitly
|
||||
enabled yolo mode outside the CLI slash-command path."""
|
||||
captured = self._capture_routing_agent(monkeypatch)
|
||||
monkeypatch.setenv("HERMES_YOLO_MODE", "1")
|
||||
with patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value={"approvals": {"mode": "manual"}},
|
||||
):
|
||||
agent = _make_codex_agent()
|
||||
with patch.object(
|
||||
agent, "_spawn_background_review", return_value=None
|
||||
):
|
||||
agent.run_conversation("write something")
|
||||
routing = captured["request_routing"]
|
||||
assert routing.auto_approve_exec is True
|
||||
assert routing.auto_approve_apply_patch is True
|
||||
|
||||
def test_session_yolo_auto_approves_codex_server_requests(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""The /yolo session toggle should be honored at Codex session creation
|
||||
time, independent of the startup-time approvals config."""
|
||||
captured = self._capture_routing_agent(monkeypatch)
|
||||
with patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value={"approvals": {"mode": "manual"}},
|
||||
):
|
||||
agent = _make_codex_agent()
|
||||
with patch(
|
||||
"tools.approval.is_current_session_yolo_enabled",
|
||||
return_value=True,
|
||||
), patch.object(
|
||||
agent, "_spawn_background_review", return_value=None
|
||||
):
|
||||
agent.run_conversation("write something")
|
||||
routing = captured["request_routing"]
|
||||
assert routing.auto_approve_exec is True
|
||||
assert routing.auto_approve_apply_patch is True
|
||||
|
||||
|
||||
class TestReviewForkApiModeDowngrade:
|
||||
"""When the parent agent runs on codex_app_server, the background
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue