diff --git a/src/deepscientist/runners/codex.py b/src/deepscientist/runners/codex.py index ff9c9d5d..0e3e7aed 100644 --- a/src/deepscientist/runners/codex.py +++ b/src/deepscientist/runners/codex.py @@ -2,6 +2,7 @@ import json import os +import re import signal import subprocess import sys @@ -95,6 +96,8 @@ "bash_exec", ), } +_BUILTIN_MCP_SERVER_NAMES = tuple(_BUILTIN_MCP_TOOL_APPROVALS) +_TOML_TABLE_HEADER_PATTERN = re.compile(r"^\s*(\[\[?)\s*([^\]]+?)\s*(\]\]?)\s*(?:#.*)?$") def _builtin_mcp_tool_approvals_for_profile(custom_profile: str | None) -> dict[str, tuple[str, ...]]: @@ -111,6 +114,34 @@ def _builtin_mcp_tool_approvals_for_profile(custom_profile: str | None) -> dict[ } return _BUILTIN_MCP_TOOL_APPROVALS + +def _strip_builtin_mcp_server_sections( + config_text: str, + server_names: tuple[str, ...] = _BUILTIN_MCP_SERVER_NAMES, +) -> str: + if not str(config_text or "").strip() or not server_names: + return str(config_text or "").rstrip() + + server_prefixes = tuple( + prefix + for name in server_names + for prefix in ( + f"mcp_servers.{name}", + f'mcp_servers."{name}"', + f"mcp_servers.'{name}'", + ) + ) + filtered_lines: list[str] = [] + skipping = False + for line in str(config_text or "").splitlines(): + match = _TOML_TABLE_HEADER_PATTERN.match(line) + if match is not None: + header = re.sub(r"\s+", "", match.group(2)) + skipping = any(header == prefix or header.startswith(f"{prefix}.") for prefix in server_prefixes) + if not skipping: + filtered_lines.append(line) + return "\n".join(filtered_lines).rstrip() + _PROVIDER_ENV_CONFLICT_KEYS = ( "OPENAI_API_KEY", "OPENAI_BASE_URL", @@ -1544,6 +1575,7 @@ def _inject_built_in_mcp( prefix = existing.split(marker_start, 1)[0].rstrip() else: prefix = existing.rstrip() + prefix = _strip_builtin_mcp_server_sections(prefix) pythonpath = os.environ.get("PYTHONPATH", "") resolved_runner_config = runner_config if isinstance(runner_config, dict) else self._load_runner_config() diff --git a/tests/test_codex_runner.py b/tests/test_codex_runner.py index 4be5bd52..cc795bee 100644 --- a/tests/test_codex_runner.py +++ b/tests/test_codex_runner.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import tomllib from pathlib import Path from types import SimpleNamespace @@ -695,6 +696,84 @@ def test_codex_runner_injects_builtin_mcp_tool_approval_overrides(temp_home) -> assert config_text.count('approval_mode = "approve"') >= 3 +def test_codex_runner_replaces_copied_builtin_mcp_sections_before_injection(temp_home) -> None: # type: ignore[no-untyped-def] + source_codex_home = temp_home / "source-codex-home" + source_codex_home.mkdir(parents=True, exist_ok=True) + (source_codex_home / "config.toml").write_text( + "\n".join( + [ + 'model_provider = "ananapi"', + "", + "[mcp_servers.memory]", + 'transport = "stdio"', + 'command = "old-memory-server"', + "", + "[mcp_servers.memory.env]", + 'OLD_MEMORY = "1"', + "", + "[mcp_servers.memory.tools.list_recent]", + 'approval_mode = "never"', + "", + "[mcp_servers.artifact]", + 'transport = "stdio"', + 'command = "old-artifact-server"', + "", + "[mcp_servers.bash_exec]", + 'transport = "stdio"', + 'command = "old-bash-server"', + "", + "[mcp_servers.bash_exec.env]", + 'OLD_BASH = "1"', + "", + "[mcp_servers.custom]", + 'transport = "stdio"', + 'command = "custom-server"', + "", + "[mcp_servers.custom.env]", + 'CUSTOM = "1"', + ] + ) + + "\n", + encoding="utf-8", + ) + (source_codex_home / "auth.json").write_text("{}\n", encoding="utf-8") + + quest_root = temp_home / "quest" + quest_root.mkdir(parents=True, exist_ok=True) + workspace_root = quest_root + + runner = CodexRunner( + home=temp_home, + repo_root=temp_home, + binary="codex", + logger=object(), # type: ignore[arg-type] + prompt_builder=object(), # type: ignore[arg-type] + artifact_service=object(), # type: ignore[arg-type] + ) + + codex_home = runner._prepare_project_codex_home( + workspace_root, + quest_root=quest_root, + quest_id="q-001", + run_id="run-001", + runner_config={"config_dir": str(source_codex_home)}, + ) + + config_text = (codex_home / "config.toml").read_text(encoding="utf-8") + parsed = tomllib.loads(config_text) + assert config_text.count("[mcp_servers.memory]") == 1 + assert config_text.count("[mcp_servers.artifact]") == 1 + assert config_text.count("[mcp_servers.bash_exec]") == 1 + assert "old-memory-server" not in config_text + assert "old-artifact-server" not in config_text + assert "old-bash-server" not in config_text + assert "OLD_MEMORY" not in config_text + assert "OLD_BASH" not in config_text + assert parsed["mcp_servers"]["memory"]["transport"] == "stdio" + assert parsed["mcp_servers"]["custom"]["command"] == "custom-server" + assert parsed["mcp_servers"]["custom"]["env"]["CUSTOM"] == "1" + + def test_codex_runner_restricts_settings_issue_profile_to_issue_tool_and_bash_exec_only(temp_home) -> None: # type: ignore[no-untyped-def] source_codex_home = temp_home / "source-codex-home" source_codex_home.mkdir(parents=True, exist_ok=True)