From c48e0b427cbea715bcb9eb246fc5a356d50abb8c Mon Sep 17 00:00:00 2001 From: embwl0x Date: Thu, 9 Jul 2026 14:54:45 -0400 Subject: [PATCH] feat(codex): configure app-server binary --- agent/codex_runtime.py | 3 ++ hermes_cli/codex_runtime_plugin_migration.py | 9 +++-- hermes_cli/codex_runtime_switch.py | 22 ++++++++--- .../test_codex_app_server_integration.py | 31 ++++++++++++++++ .../test_codex_app_server_runtime.py | 37 +++++++++++++++++++ .../test_codex_runtime_plugin_migration.py | 28 ++++++++++++-- tests/hermes_cli/test_codex_runtime_switch.py | 33 ++++++++++++++++- .../features/codex-app-server-runtime.md | 13 +++++++ 8 files changed, 164 insertions(+), 12 deletions(-) diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 7f9f4dba2f..0a48666fce 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -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), ) diff --git a/hermes_cli/codex_runtime_plugin_migration.py b/hermes_cli/codex_runtime_plugin_migration.py index 5a27bebf4d..745e843b6b 100644 --- a/hermes_cli/codex_runtime_plugin_migration.py +++ b/hermes_cli/codex_runtime_plugin_migration.py @@ -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 diff --git a/hermes_cli/codex_runtime_switch.py b/hermes_cli/codex_runtime_switch.py index 6db73ffd74..9278e9040d 100644 --- a/hermes_cli/codex_runtime_switch.py +++ b/hermes_cli/codex_runtime_switch.py @@ -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 diff --git a/tests/agent/test_codex_app_server_integration.py b/tests/agent/test_codex_app_server_integration.py index b0b1277c12..056a9b31f5 100644 --- a/tests/agent/test_codex_app_server_integration.py +++ b/tests/agent/test_codex_app_server_integration.py @@ -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 diff --git a/tests/agent/transports/test_codex_app_server_runtime.py b/tests/agent/transports/test_codex_app_server_runtime.py index b3cae82c3a..56db322b95 100644 --- a/tests/agent/transports/test_codex_app_server_runtime.py +++ b/tests/agent/transports/test_codex_app_server_runtime.py @@ -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.""" diff --git a/tests/hermes_cli/test_codex_runtime_plugin_migration.py b/tests/hermes_cli/test_codex_runtime_plugin_migration.py index 62284fbd8d..cc9e741df2 100644 --- a/tests/hermes_cli/test_codex_runtime_plugin_migration.py +++ b/tests/hermes_cli/test_codex_runtime_plugin_migration.py @@ -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, diff --git a/tests/hermes_cli/test_codex_runtime_switch.py b/tests/hermes_cli/test_codex_runtime_switch.py index f6382ee066..36ed4b1af0 100644 --- a/tests/hermes_cli/test_codex_runtime_switch.py +++ b/tests/hermes_cli/test_codex_runtime_switch.py @@ -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 - diff --git a/website/docs/user-guide/features/codex-app-server-runtime.md b/website/docs/user-guide/features/codex-app-server-runtime.md index 9c85c01891..2f9075f692 100644 --- a/website/docs/user-guide/features/codex-app-server-runtime.md +++ b/website/docs/user-guide/features/codex-app-server-runtime.md @@ -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: