diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 98d3621d0c..0552571b8e 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -14,6 +14,7 @@ from types import SimpleNamespace from typing import Any, Callable, Dict, List from agent.stream_single_writer import claim_stream_writer, stream_writer_is_current +from agent.transports.hermes_tools_mcp_server import HERMES_TOOLS_MCP_SERVER_NAME from agent.usage_anchor import set_usage_anchor logger = logging.getLogger(__name__) @@ -195,7 +196,6 @@ def _record_codex_app_server_compaction(agent, turn, *, approx_tokens: int | Non _CODEX_TOOL_ITEM_TYPES = frozenset({"commandExecution", "fileChange", "mcpToolCall", "dynamicToolCall", "webSearch"}) # Internal MCP server wrapping Hermes' native tools: its inner dispatch has no tool_progress_callback, so the # codex-level mcpToolCall IS the display event and the mcp.hermes-tools.* prefix is stripped (users see Hermes tools). -_INTERNAL_MCP_SERVER = "hermes-tools" _STATIC_TOOL_NAMES = {"commandExecution": "exec_command", "fileChange": "apply_patch", "webSearch": "web_search"} _STABLE_ID_PREFIXES = {"commandExecution": "exec", "fileChange": "apply_patch"} _MCP_LIKE_ITEM_TYPES = {"mcpToolCall", "dynamicToolCall"} @@ -212,7 +212,7 @@ def _codex_item_to_tool_name(item: dict) -> str: item_type = item.get("type") or "" if item_type == "mcpToolCall": server, tool = item.get("server") or "mcp", item.get("tool") or "unknown" - return tool if server == _INTERNAL_MCP_SERVER else f"mcp.{server}.{tool}" + return tool if server == HERMES_TOOLS_MCP_SERVER_NAME else f"mcp.{server}.{tool}" if item_type == "dynamicToolCall": return item.get("tool") or "dynamic" return _STATIC_TOOL_NAMES.get(item_type) or item_type or "unknown" diff --git a/agent/transports/codex_app_server.py b/agent/transports/codex_app_server.py index e9be0c803a..fc6b7f34fb 100644 --- a/agent/transports/codex_app_server.py +++ b/agent/transports/codex_app_server.py @@ -18,6 +18,7 @@ from dataclasses import dataclass from typing import Any, Optional from agent.deadline import kill_process_tree +from agent.transports.hermes_tools_mcp_server import HERMES_TOOLS_MCP_SERVER_NAME from tools.environments.local import hermes_subprocess_env MIN_CODEX_VERSION = (0, 125, 0) @@ -100,13 +101,14 @@ class CodexAppServerClient: ) # Native shell children remain unowned. Only Hermes' managed MCP tool # endpoint acts for this worker; grant it scope via its existing per-server - # environment, never by granting the whole executor process ownership. + # environment (the entry the runtime migration registers), never by granting + # the whole executor process ownership. owned_task = os.environ.get("HERMES_KANBAN_TASK") and is_dispatcher_owned_worker_context() if owned_task: for key in (*KANBAN_ENV_KEYS, "HERMES_KANBAN_DB", "HERMES_KANBAN_BOARD"): if key in os.environ: - cmd += ["-c", f"mcp_servers.hermes-tools.env.{key}={json.dumps(os.environ[key])}"] - cmd += ["-c", f'mcp_servers.hermes-tools.env.{DELEGATED_CHILD_ENV_MARKER}=""'] + cmd += ["-c", f"mcp_servers.{HERMES_TOOLS_MCP_SERVER_NAME}.env.{key}={json.dumps(os.environ[key])}"] + cmd += ["-c", f'mcp_servers.{HERMES_TOOLS_MCP_SERVER_NAME}.env.{DELEGATED_CHILD_ENV_MARKER}=""'] spawn_env = delegated_child_subprocess_env(spawn_env) # Kanban workers must write handoff/status to the board DB outside the # workspace: keep the sandbox on, add the Kanban root as writable. diff --git a/agent/transports/codex_app_server_session.py b/agent/transports/codex_app_server_session.py index effc28e69d..25635069cf 100644 --- a/agent/transports/codex_app_server_session.py +++ b/agent/transports/codex_app_server_session.py @@ -21,6 +21,7 @@ from agent.codex_responses_adapter import _format_responses_error from agent.redact import redact_sensitive_text from agent.transports.codex_app_server import CodexAppServerClient, CodexAppServerError from agent.transports.codex_event_projector import CodexEventProjector, ProjectionResult +from agent.transports.hermes_tools_mcp_server import HERMES_TOOLS_MCP_SERVER_NAME logger = logging.getLogger(__name__) @@ -569,7 +570,7 @@ class CodexAppServerSession: def _respond_elicitation(self, params: dict) -> dict: """MCP elicitation: auto-accept our own hermes-tools server (opted in by enabling the runtime; exposes nothing codex's shell can't do); decline others so the user opts in via codex's own flow.""" - action = "accept" if (params.get("serverName") or "") == "hermes-tools" else "decline" + action = "accept" if (params.get("serverName") or "") == HERMES_TOOLS_MCP_SERVER_NAME else "decline" return {"action": action, "content": None, "_meta": None} _SERVER_REQUEST_HANDLERS: dict[str, Callable[..., dict]] = { diff --git a/agent/transports/hermes_tools_mcp_server.py b/agent/transports/hermes_tools_mcp_server.py index 48909a026c..2baacd9840 100644 --- a/agent/transports/hermes_tools_mcp_server.py +++ b/agent/transports/hermes_tools_mcp_server.py @@ -16,6 +16,12 @@ from typing import Any, Optional logger = logging.getLogger(__name__) +# The ``[mcp_servers.]`` key under which the runtime migration registers this server. Every +# codex-side reference to it (worker ``-c mcp_servers..env.*`` overrides, elicitation +# auto-accept, display-name stripping) must use this constant: a drifted name materialises a +# second env-only entry that codex rejects at bootstrap ("invalid transport"). +HERMES_TOOLS_MCP_SERVER_NAME = "hermes-tools" + # JSON Schema type -> Python type mapping for signature generation _JSON_TO_PY = {"string": str, "integer": int, "number": float, "boolean": bool, "array": list, "object": dict} @@ -63,7 +69,7 @@ def _build_server() -> Any: from model_tools import get_tool_definitions, handle_function_call mcp = MCPServer( - "hermes-tools", + HERMES_TOOLS_MCP_SERVER_NAME, instructions=( "Hermes Agent's tool surface, exposed for use inside a Codex " "session. Use these for capabilities Codex's built-in toolset " diff --git a/hermes_cli/codex_runtime_plugin_migration.py b/hermes_cli/codex_runtime_plugin_migration.py index ebacb391d1..5a27bebf4d 100644 --- a/hermes_cli/codex_runtime_plugin_migration.py +++ b/hermes_cli/codex_runtime_plugin_migration.py @@ -9,6 +9,8 @@ from dataclasses import dataclass, field from pathlib import Path from typing import Any, Optional +from agent.transports.hermes_tools_mcp_server import HERMES_TOOLS_MCP_SERVER_NAME + logger = logging.getLogger(__name__) @@ -425,9 +427,9 @@ def migrate( if default_permission_profile: report.wrote_permissions_default = default_permission_profile if expose_hermes_tools: - translated["hermes-tools"] = _build_hermes_tools_mcp_entry() - if "hermes-tools" not in report.migrated: - report.migrated.append("hermes-tools") + translated[HERMES_TOOLS_MCP_SERVER_NAME] = _build_hermes_tools_mcp_entry() + if HERMES_TOOLS_MCP_SERVER_NAME not in report.migrated: + report.migrated.append(HERMES_TOOLS_MCP_SERVER_NAME) managed_block = render_codex_toml_section( translated, plugins=plugins, default_permission_profile=default_permission_profile) new_text = managed_block diff --git a/hermes_cli/codex_runtime_switch.py b/hermes_cli/codex_runtime_switch.py index c474017d8f..6db73ffd74 100644 --- a/hermes_cli/codex_runtime_switch.py +++ b/hermes_cli/codex_runtime_switch.py @@ -88,10 +88,10 @@ def _migration_lines(config: dict) -> list[str]: """Run the ~/.codex/config.toml migration and describe it; failures are non-fatal.""" lines: list[str] = [] try: - from hermes_cli.codex_runtime_plugin_migration import migrate + from hermes_cli.codex_runtime_plugin_migration import HERMES_TOOLS_MCP_SERVER_NAME, migrate mig_report = migrate(config) # The hermes-tools callback is internal plumbing — surfaced separately below. - user_servers = [s for s in mig_report.migrated if s != "hermes-tools"] + user_servers = [s for s in mig_report.migrated if s != HERMES_TOOLS_MCP_SERVER_NAME] if user_servers: lines.append(f"Migrated {len(user_servers)} MCP server(s): {', '.join(user_servers)}") if mig_report.migrated_plugins: @@ -104,7 +104,7 @@ def _migration_lines(config: dict) -> list[str]: lines.append( f"Default sandbox: {mig_report.wrote_permissions_default} " f"(no approval prompt on every write)") - if "hermes-tools" in mig_report.migrated: + if HERMES_TOOLS_MCP_SERVER_NAME in mig_report.migrated: lines.extend(_HERMES_TOOLS_CALLBACK_NOTE) lines.append(f" (config: {mig_report.target_path})") for err in mig_report.errors: diff --git a/tests/agent/transports/test_codex_worker_mcp_overrides.py b/tests/agent/transports/test_codex_worker_mcp_overrides.py new file mode 100644 index 0000000000..4712e2f18c --- /dev/null +++ b/tests/agent/transports/test_codex_worker_mcp_overrides.py @@ -0,0 +1,87 @@ +"""Kanban worker MCP overrides must target the server entry the runtime migration registers. + +Regression for #111707: dispatcher-owned workers on the codex app-server runtime injected +``mcp_servers.hermes-mcp.env.*`` while the migration writes ``[mcp_servers.hermes-tools]``; +codex then saw an env-only entry with no transport and refused to start. +""" + +import subprocess +import tomllib + +import pytest + +from agent.delegation_context import DELEGATED_CHILD_ENV_MARKER, KANBAN_ENV_KEYS, non_dispatcher_owned_context +from agent.transports import codex_app_server as cas +from hermes_cli.codex_runtime_plugin_migration import migrate + + +class _RecordingPopen: + commands: list[list[str]] = [] + + def __init__(self, cmd, *args, **kwargs): + type(self).commands.append(list(cmd)) + self.stdin = self.stdout = 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 + + +@pytest.fixture +def launch(monkeypatch, tmp_path): + """Return ``launch(env) -> list[str]`` of the ``mcp_servers.*`` overrides in the worker argv.""" + _RecordingPopen.commands = [] + monkeypatch.setattr(subprocess, "Popen", _RecordingPopen) + for key in (*KANBAN_ENV_KEYS, DELEGATED_CHILD_ENV_MARKER, "HERMES_KANBAN_DB", "HERMES_KANBAN_BOARD"): + monkeypatch.delenv(key, raising=False) + + def _launch(env: dict[str, str]) -> list[str]: + with monkeypatch.context() as ctx: + for key, value in env.items(): + ctx.setenv(key, value) + client = cas.CodexAppServerClient(codex_bin="codex", codex_home=str(tmp_path / "codex")) + client._closed = True + cmd = _RecordingPopen.commands.pop() + return [arg for arg in cmd if arg.startswith("mcp_servers.")] + + return _launch + + +def _migrated_server_names(tmp_path) -> set[str]: + codex_home = tmp_path / "codex" + report = migrate({"mcp_servers": {}}, codex_home=codex_home, discover_plugins=False, expose_hermes_tools=True) + assert not report.errors + return set(tomllib.loads((codex_home / "config.toml").read_text(encoding="utf-8"))["mcp_servers"]) + + +def test_worker_overrides_target_the_migrated_server(launch, tmp_path): + """Producer/consumer contract: every ``-c mcp_servers..env.*`` override the worker + launcher emits names an entry the migration actually writes to config.toml.""" + overrides = launch({ + "HERMES_KANBAN_TASK": "11111111-1111-4111-8111-111111111111", + "HERMES_KANBAN_RUN_ID": "42", + "HERMES_KANBAN_DB": str(tmp_path / "board" / "kanban.db"), + }) + assert overrides, "dispatcher-owned worker must scope the managed MCP endpoint" + targeted = {arg.split(".env.", 1)[0].removeprefix("mcp_servers.") for arg in overrides} + migrated = _migrated_server_names(tmp_path) + assert targeted <= migrated, f"overrides target {targeted - migrated}, which codex has no transport for" + assert any(arg.startswith(f"mcp_servers.{next(iter(targeted))}.env.HERMES_KANBAN_TASK=") for arg in overrides) + + +def test_only_dispatcher_owned_workers_get_mcp_overrides(launch): + """An ordinary launch and a worker that does not own the dispatcher's task emit no + ``mcp_servers.*`` override at all (nothing to scope, nothing for codex to reject).""" + assert launch({}) == [] + with non_dispatcher_owned_context(): + assert launch({"HERMES_KANBAN_TASK": "11111111-1111-4111-8111-111111111111"}) == []