feat(codex): configure app-server binary
This commit is contained in:
@@ -388,6 +388,8 @@ def _ensure_codex_session(agent) -> None:
|
||||
return
|
||||
from agent.runtime_cwd import resolve_agent_cwd
|
||||
from agent.transports.codex_app_server_session import CodexAppServerSession, _ServerRequestRouting
|
||||
from hermes_cli.codex_runtime_switch import get_configured_codex_binary
|
||||
from hermes_cli.config import load_config
|
||||
# Approval callback: Hermes' standard prompt flow when a CLI thread installed one.
|
||||
approval_callback = None
|
||||
with suppress(Exception):
|
||||
@@ -408,6 +410,7 @@ def _ensure_codex_session(agent) -> None:
|
||||
# narrower item/started-only bridge from #38835.
|
||||
agent._codex_session = CodexAppServerSession(
|
||||
cwd=getattr(agent, "session_cwd", None) or str(resolve_agent_cwd()), approval_callback=approval_callback,
|
||||
codex_bin=get_configured_codex_binary(load_config()),
|
||||
request_routing=_ServerRequestRouting(auto_approve_exec=auto_approve_requests, auto_approve_apply_patch=auto_approve_requests),
|
||||
on_event=make_codex_app_server_event_bridge(agent),
|
||||
)
|
||||
|
||||
@@ -277,7 +277,8 @@ def _strip_existing_managed_block(toml_text: str) -> str:
|
||||
|
||||
|
||||
def _query_codex_plugins(
|
||||
codex_home: Optional[Path] = None, timeout: float = 8.0) -> tuple[list[dict], Optional[str]]:
|
||||
codex_home: Optional[Path] = None, timeout: float = 8.0, codex_bin: str = "codex",
|
||||
) -> tuple[list[dict], Optional[str]]:
|
||||
"""Spawn ``codex app-server`` briefly and return ``(installed plugins, error)`` from
|
||||
``plugin/list``. Any failure yields ``([], error)`` and is non-fatal (servers and
|
||||
permissions still write). Plugins codex reports unavailable (broken install, missing OAuth,
|
||||
@@ -289,7 +290,7 @@ def _query_codex_plugins(
|
||||
except Exception as exc:
|
||||
return [], f"transport unavailable: {exc}"
|
||||
try:
|
||||
with CodexAppServerClient(codex_home=str(codex_home) if codex_home else None) as client:
|
||||
with CodexAppServerClient(codex_bin=codex_bin, codex_home=str(codex_home) if codex_home else None) as client:
|
||||
client.initialize(client_name="hermes-migration")
|
||||
resp = client.request("plugin/list", {}, timeout=timeout)
|
||||
except Exception as exc:
|
||||
@@ -417,7 +418,9 @@ def migrate(
|
||||
plugins: list[dict] = []
|
||||
plugin_query_succeeded = False
|
||||
if discover_plugins and not dry_run:
|
||||
plugins, plugin_err = _query_codex_plugins(codex_home=codex_home)
|
||||
from hermes_cli.codex_runtime_switch import get_configured_codex_binary
|
||||
plugins, plugin_err = _query_codex_plugins(
|
||||
codex_home=codex_home, codex_bin=get_configured_codex_binary(hermes_config))
|
||||
if plugin_err:
|
||||
report.plugin_query_error = plugin_err
|
||||
# An authoritative plugin/list (even an empty one) means we own [plugins.*] for this
|
||||
|
||||
@@ -63,6 +63,17 @@ def get_current_runtime(config: dict) -> str:
|
||||
return value if value in VALID_RUNTIMES else "auto"
|
||||
|
||||
|
||||
def get_configured_codex_binary(config: dict) -> str:
|
||||
"""``model.codex_bin`` (one argv element, never shell-parsed) or bare ``codex`` from PATH.
|
||||
|
||||
Gateway/service/Kanban-worker processes often run with a minimal PATH that lacks the codex
|
||||
CLI (e.g. a desktop-bundled ``.../Codex.app/Contents/Resources/codex``), so users need a
|
||||
config-level override for every codex spawn site (#61360)."""
|
||||
model_cfg = config.get("model") if isinstance(config, dict) else None
|
||||
value = model_cfg.get("codex_bin") if isinstance(model_cfg, dict) else None
|
||||
return str(value or "").strip() or "codex"
|
||||
|
||||
|
||||
def set_runtime(config: dict, new_value: str) -> str:
|
||||
"""Persist *new_value* into the config dict in place; returns the previous value."""
|
||||
if new_value not in VALID_RUNTIMES:
|
||||
@@ -74,12 +85,12 @@ def set_runtime(config: dict, new_value: str) -> str:
|
||||
return old
|
||||
|
||||
|
||||
def check_codex_binary_ok() -> tuple[bool, Optional[str]]:
|
||||
def check_codex_binary_ok(codex_bin: str = "codex") -> tuple[bool, Optional[str]]:
|
||||
"""Best-effort codex CLI install/version check → ``(ok, version_or_message)``."""
|
||||
try:
|
||||
from agent.transports.codex_app_server import check_codex_binary
|
||||
|
||||
return check_codex_binary()
|
||||
return check_codex_binary(codex_bin=codex_bin)
|
||||
except Exception as exc: # pragma: no cover
|
||||
return False, f"codex check failed: {exc}"
|
||||
|
||||
@@ -119,12 +130,13 @@ def apply(
|
||||
"""Entry point for CLI and gateway. ``config`` is mutated in place when ``new_value`` is set
|
||||
(None = show current state); ``persist_callback(config)`` writes it, skipped when None."""
|
||||
current = get_current_runtime(config)
|
||||
codex_bin = get_configured_codex_binary(config)
|
||||
|
||||
# Cached per apply() call: the enable path would otherwise spawn `codex --version` up to 3x.
|
||||
_check_binary_cached = functools.cache(check_codex_binary_ok)
|
||||
|
||||
if new_value is None:
|
||||
ok, ver = _check_binary_cached()
|
||||
ok, ver = _check_binary_cached(codex_bin)
|
||||
msg = (
|
||||
f"openai_runtime: {current}\n"
|
||||
f"codex CLI: {'OK ' + ver if ok else 'not available — ' + (ver or 'install with `npm i -g @openai/codex`')}"
|
||||
@@ -145,7 +157,7 @@ def apply(
|
||||
# Switching ON: verify codex CLI before persisting — an opt-in toggle that silently fails on
|
||||
# the first turn is the worst possible UX.
|
||||
if new_value == "codex_app_server":
|
||||
ok, ver_or_msg = _check_binary_cached()
|
||||
ok, ver_or_msg = _check_binary_cached(codex_bin)
|
||||
if not ok:
|
||||
return CodexRuntimeStatus(
|
||||
success=False, new_value=None, old_value=current,
|
||||
@@ -170,7 +182,7 @@ def apply(
|
||||
if reapplying_enable
|
||||
else f"openai_runtime: {current} → {new_value}"]
|
||||
if new_value == "codex_app_server":
|
||||
ok, ver = _check_binary_cached()
|
||||
ok, ver = _check_binary_cached(codex_bin)
|
||||
if ok:
|
||||
msg_lines.append(f"codex CLI: {ver}")
|
||||
# Migrate Hermes' MCP servers + Codex's curated plugins into ~/.codex/config.toml so the
|
||||
|
||||
@@ -378,6 +378,37 @@ class TestRunConversationCodexPath:
|
||||
|
||||
assert captured["cwd"] == str(tmp_path)
|
||||
|
||||
def test_configured_codex_binary_seeds_app_server_session(self, monkeypatch):
|
||||
configured = "/Applications/Codex.app/Contents/Resources/codex"
|
||||
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)
|
||||
|
||||
with patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value={"model": {"codex_bin": configured}},
|
||||
):
|
||||
agent = _make_codex_agent()
|
||||
with patch.object(
|
||||
agent, "_spawn_background_review", return_value=None
|
||||
):
|
||||
agent.run_conversation("hi")
|
||||
|
||||
assert captured["codex_bin"] == configured
|
||||
|
||||
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
|
||||
|
||||
@@ -274,6 +274,43 @@ class TestSpawnEnvIsolation:
|
||||
"subprocesses (gh, git, aws, npm) need the user's real HOME."
|
||||
)
|
||||
|
||||
def test_configured_binary_path_with_spaces_is_one_argv_element(
|
||||
self, monkeypatch
|
||||
):
|
||||
import subprocess
|
||||
from agent.transports import codex_app_server as cas
|
||||
|
||||
captured = {}
|
||||
|
||||
class FakePopen:
|
||||
def __init__(self, cmd, *args, **kwargs):
|
||||
captured["cmd"] = list(cmd)
|
||||
self.stdin = None
|
||||
self.stdout = None
|
||||
self.stderr = None
|
||||
self.pid = 1
|
||||
self.returncode = None
|
||||
|
||||
def poll(self):
|
||||
return None
|
||||
|
||||
def terminate(self):
|
||||
pass
|
||||
|
||||
def wait(self, timeout=None):
|
||||
return 0
|
||||
|
||||
def kill(self):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(subprocess, "Popen", FakePopen)
|
||||
configured = "/Applications/Codex Preview.app/Contents/Resources/codex"
|
||||
|
||||
client = cas.CodexAppServerClient(codex_bin=configured)
|
||||
client._closed = True
|
||||
|
||||
assert captured["cmd"][:2] == [configured, "app-server"]
|
||||
|
||||
def test_spawn_env_sets_CODEX_HOME_when_provided(self, monkeypatch):
|
||||
"""CODEX_HOME isolation must still work — that's the whole point
|
||||
of the codex_home arg."""
|
||||
|
||||
@@ -168,7 +168,7 @@ class TestMigrate:
|
||||
blocks. This is what OpenClaw calls 'migrate native codex plugins.'"""
|
||||
from hermes_cli import codex_runtime_plugin_migration as crpm
|
||||
|
||||
def fake_query(codex_home=None, timeout=8.0):
|
||||
def fake_query(codex_home=None, timeout=8.0, codex_bin="codex"):
|
||||
return [
|
||||
{"name": "google-calendar", "marketplace": "openai-curated",
|
||||
"enabled": True},
|
||||
@@ -185,13 +185,35 @@ class TestMigrate:
|
||||
assert "google-calendar@openai-curated" in report.migrated_plugins
|
||||
assert "github@openai-curated" in report.migrated_plugins
|
||||
|
||||
def test_plugin_discovery_uses_configured_codex_binary(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
from hermes_cli import codex_runtime_plugin_migration as crpm
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_query(codex_home=None, timeout=8.0, codex_bin="codex"):
|
||||
captured["codex_bin"] = codex_bin
|
||||
return [], None
|
||||
|
||||
monkeypatch.setattr(crpm, "_query_codex_plugins", fake_query)
|
||||
configured = "/Applications/Codex.app/Contents/Resources/codex"
|
||||
|
||||
migrate(
|
||||
{"model": {"codex_bin": configured}},
|
||||
codex_home=tmp_path,
|
||||
discover_plugins=True,
|
||||
expose_hermes_tools=False,
|
||||
)
|
||||
|
||||
assert captured["codex_bin"] == configured
|
||||
|
||||
def test_plugin_discovery_failure_non_fatal(self, tmp_path, monkeypatch):
|
||||
"""If codex isn't installed or RPC fails, MCP migration still
|
||||
completes. The error surfaces in the report but doesn't abort."""
|
||||
from hermes_cli import codex_runtime_plugin_migration as crpm
|
||||
|
||||
def fake_query_fails(codex_home=None, timeout=8.0):
|
||||
def fake_query_fails(codex_home=None, timeout=8.0, codex_bin="codex"):
|
||||
return [], "codex CLI not available"
|
||||
monkeypatch.setattr(crpm, "_query_codex_plugins", fake_query_fails)
|
||||
|
||||
@@ -354,7 +376,7 @@ class TestStripUnmanagedPluginTables:
|
||||
)
|
||||
|
||||
# Simulate codex's plugin/list reporting the same plugin tasks@openai-curated.
|
||||
def fake_query(codex_home=None, timeout=8.0):
|
||||
def fake_query(codex_home=None, timeout=8.0, codex_bin="codex"):
|
||||
return (
|
||||
[{"name": "tasks", "marketplace": "openai-curated", "enabled": True}],
|
||||
None,
|
||||
|
||||
@@ -45,6 +45,24 @@ class TestGetCurrentRuntime:
|
||||
) == "auto"
|
||||
|
||||
|
||||
class TestGetConfiguredCodexBinary:
|
||||
def test_defaults_for_missing_or_invalid_values(self):
|
||||
assert crs.get_configured_codex_binary({}) == "codex"
|
||||
assert crs.get_configured_codex_binary({"model": {}}) == "codex"
|
||||
assert crs.get_configured_codex_binary(
|
||||
{"model": {"codex_bin": ""}}
|
||||
) == "codex"
|
||||
assert crs.get_configured_codex_binary(
|
||||
{"model": {"codex_bin": 42}}
|
||||
) == "codex"
|
||||
|
||||
def test_returns_trimmed_configured_path(self):
|
||||
configured = "/Applications/Codex.app/Contents/Resources/codex"
|
||||
assert crs.get_configured_codex_binary(
|
||||
{"model": {"codex_bin": f" {configured} "}}
|
||||
) == configured
|
||||
|
||||
|
||||
class TestSetRuntime:
|
||||
def test_creates_model_section_if_missing(self):
|
||||
cfg = {}
|
||||
@@ -59,7 +77,21 @@ class TestSetRuntime:
|
||||
|
||||
|
||||
class TestApply:
|
||||
def test_binary_check_uses_configured_path(self):
|
||||
configured = "/Applications/Codex.app/Contents/Resources/codex"
|
||||
cfg = {
|
||||
"model": {
|
||||
"openai_runtime": "codex_app_server",
|
||||
"codex_bin": configured,
|
||||
}
|
||||
}
|
||||
with patch.object(
|
||||
crs, "check_codex_binary_ok", return_value=(True, "0.130.0")
|
||||
) as binary_check:
|
||||
result = crs.apply(cfg, None)
|
||||
|
||||
assert result.success
|
||||
binary_check.assert_called_once_with(configured)
|
||||
|
||||
def test_reapply_codex_app_server_runs_migration(self):
|
||||
"""Re-applying codex_app_server when already enabled must still
|
||||
@@ -169,4 +201,3 @@ class TestApply:
|
||||
assert "MCP migration skipped" in r.message
|
||||
assert "disk full" in r.message
|
||||
|
||||
|
||||
|
||||
@@ -197,6 +197,19 @@ model:
|
||||
openai_runtime: codex_app_server # default is "auto" (= Hermes runtime)
|
||||
```
|
||||
|
||||
If the Hermes process cannot resolve `codex` from `PATH` (common for gateway
|
||||
services and desktop-launched workers), configure the executable explicitly:
|
||||
|
||||
```yaml
|
||||
model:
|
||||
openai_runtime: codex_app_server
|
||||
codex_bin: /Applications/Codex.app/Contents/Resources/codex
|
||||
```
|
||||
|
||||
The configured path is used consistently for the enablement check, native
|
||||
plugin discovery, and the long-lived app-server subprocess. Paths containing
|
||||
spaces are passed as a single subprocess argument; do not add shell quoting.
|
||||
|
||||
## Self-improvement loop (memory + skill nudges)
|
||||
|
||||
Hermes' background self-improvement fires on counter thresholds:
|
||||
|
||||
Reference in New Issue
Block a user