diff --git a/evalbench/generators/models/codex_cli.py b/evalbench/generators/models/codex_cli.py index 50bc7702..e0ba5100 100644 --- a/evalbench/generators/models/codex_cli.py +++ b/evalbench/generators/models/codex_cli.py @@ -114,6 +114,8 @@ def __init__(self, querygenerator_config): self.setup_config = querygenerator_config.get("setup", {}) self.config_path = os.path.join(self.codex_config_dir, "config.toml") + self.inline_mcp_servers = {} + self.enabled_plugins = {} self._setup() @staticmethod @@ -248,9 +250,10 @@ def _setup(self): either be a repo with a `skills/` child or a single skill folder. """ mcp_servers_config = self.setup_config.get("mcp_servers", {}) - extra_config = dict(self._DEFAULT_TOP_LEVEL_CONFIG) - extra_config.update(self.setup_config.get("config", {})) - self._write_config_toml(mcp_servers_config, extra_config) + if isinstance(mcp_servers_config, list): + self._install_mcp_servers_from_repo(mcp_servers_config) + elif isinstance(mcp_servers_config, dict): + self.inline_mcp_servers.update(mcp_servers_config) skills_config = self.setup_config.get("skills", []) if skills_config: @@ -260,6 +263,8 @@ def _setup(self): if skills_dir_path: self._setup_skills_from_dir(skills_dir_path) + self._write_config_toml() + def _setup_skills(self, skills: list): """Installs Codex skills from repo or local path configs.""" setup_env = os.environ.copy() @@ -284,6 +289,15 @@ def _setup_skills(self, skills: list): repo_dir = self._clone_extension_repo(url, plugins_dir, setup_env) if repo_dir: self._register_codex_plugin(repo_dir, skill_config) + plugin_name = skill_config.get("plugin_name") or skill_config.get("plugin") or self._read_codex_plugin_name(repo_dir) + if not plugin_name: + plugin_name = os.path.basename(os.path.abspath(repo_dir)) + marketplace_name = skill_config.get("marketplace_name", "evalbench-local-marketplace") + plugin_id = f"{plugin_name}@{marketplace_name}" + # Enable the plugin in config.toml. Even for skills-only plugins, + # this is safe and ensures any plugin-specific configuration is passed + # to the plugin context, and matches the behavior of _install_mcp_servers_from_repo. + self.enabled_plugins.setdefault(plugin_id, {}).update(skill_config.get("config", {}) or {}) self._install_skills_from_source(repo_dir, skill_config) elif action in ("copy", "link", "install") and path: # Materialize the skill instead of symlinking so Codex sees a @@ -346,7 +360,7 @@ def _clone_extension_repo( def _register_codex_plugin(self, repo_dir: str, skill_config: dict): """Registers a local Codex plugin marketplace entry for cloned repos.""" - plugin_name = skill_config.get("plugin_name") or self._read_codex_plugin_name(repo_dir) + plugin_name = skill_config.get("plugin_name") or skill_config.get("plugin") or self._read_codex_plugin_name(repo_dir) if not plugin_name: plugin_name = os.path.basename(os.path.abspath(repo_dir)) @@ -355,7 +369,7 @@ def _register_codex_plugin(self, repo_dir: str, skill_config: dict): display_name = skill_config.get( "marketplace_display_name", "EvalBench Local Skills") - plugins_dir = os.path.join(self.codex_config_dir, "plugins") + plugins_dir = os.path.join(self.fake_home, ".agents", "plugins") os.makedirs(plugins_dir, exist_ok=True) marketplace_path = os.path.join(plugins_dir, "marketplace.json") @@ -375,11 +389,16 @@ def _register_codex_plugin(self, repo_dir: str, skill_config: dict): logging.warning( f"Failed to read Codex marketplace at {marketplace_path}: {e}") + # Compute path relative to fake_home so that it works as a local marketplace source + rel_path = os.path.relpath(os.path.abspath(repo_dir), self.fake_home) + if not (rel_path.startswith("./") or rel_path.startswith("../") or rel_path.startswith("/")): + rel_path = f"./{rel_path}" + entry = { "name": plugin_name, "source": { "source": "local", - "path": os.path.abspath(repo_dir), + "path": rel_path, }, "policy": { "installation": "AVAILABLE", @@ -400,6 +419,19 @@ def _register_codex_plugin(self, repo_dir: str, skill_config: dict): logging.info( f"Registered Codex plugin '{plugin_name}' in {marketplace_path}") + # Install the plugin via Codex CLI so it changes status from 'not installed' to 'installed, enabled' + cmd = ["npm", "exec", "--yes", self.codex_cli_version, "--", "plugin", "add", f"{plugin_name}@{marketplace_name}"] + try: + setup_env = os.environ.copy() + setup_env.update(self.env) + result = subprocess.run(cmd, env=setup_env, check=False, capture_output=True, text=True) + if result.returncode == 0: + logging.info(f"Successfully installed Codex plugin '{plugin_name}@{marketplace_name}'") + else: + logging.error(f"Failed to install Codex plugin '{plugin_name}@{marketplace_name}': {result.stderr.strip()}") + except Exception as e: + logging.error(f"Error executing plugin installation command: {e}") + @staticmethod def _read_codex_plugin_name(repo_dir: str) -> str: plugin_json_path = os.path.join(repo_dir, ".codex-plugin", "plugin.json") @@ -468,7 +500,39 @@ def _find_skill_dirs(source_dir: str) -> list[str]: if os.path.exists(os.path.join(source_dir, entry, "SKILL.md")) ] - def _write_config_toml(self, mcp_servers_config: dict, extra_config: dict): + def _install_mcp_servers_from_repo(self, mcp_servers: list): + """Clones plugin repositories that bundle MCP servers and enables them.""" + setup_env = os.environ.copy() + setup_env.update(self.env) + + plugins_dir = os.path.join(self.codex_config_dir, "plugins") + os.makedirs(plugins_dir, exist_ok=True) + + for mcp_config in mcp_servers: + if not isinstance(mcp_config, dict): + logging.warning(f"Unsupported MCP server config: {mcp_config}") + continue + action = mcp_config.get("action") + url = mcp_config.get("url") + if action != "install_from_repo" or not url: + logging.warning( + f"Unsupported MCP server config: {mcp_config}. When " + "'mcp_servers' is a list, each entry must use " + "'action: install_from_repo' with 'url'.") + continue + repo_dir = self._clone_extension_repo(url, plugins_dir, setup_env) + if repo_dir: + plugin_name = mcp_config.get("plugin") or self._read_codex_plugin_name(repo_dir) + if not plugin_name: + plugin_name = os.path.basename(os.path.abspath(repo_dir)) + self._register_codex_plugin(repo_dir, mcp_config) + self._install_skills_from_source(repo_dir, mcp_config) + + marketplace_name = mcp_config.get("marketplace_name", "evalbench-local-marketplace") + plugin_id = f"{plugin_name}@{marketplace_name}" + self.enabled_plugins.setdefault(plugin_id, {}).update(mcp_config.get("config", {}) or {}) + + def _write_config_toml(self): """Writes Codex CLI's `config.toml` with MCP server declarations. Accepts the same Gemini-style MCP shape the rest of evalbench uses @@ -486,18 +550,28 @@ def _write_config_toml(self, mcp_servers_config: dict, extra_config: dict): """ lines: list[str] = [] + extra_config = dict(self._DEFAULT_TOP_LEVEL_CONFIG) + extra_config.update(self.setup_config.get("config", {})) + for key, value in extra_config.items(): lines.append(f"{key} = {self._toml_value(value)}") if extra_config: lines.append("") - for server_name, config in mcp_servers_config.items(): + for server_name, config in self.inline_mcp_servers.items(): translated = self._translate_mcp_config(server_name, dict(config)) lines.append(f"[mcp_servers.{self._toml_key(server_name)}]") for key, value in translated.items(): lines.append(f"{key} = {self._toml_value(value)}") lines.append("") + for plugin_id, options in self.enabled_plugins.items(): + lines.append(f"[plugins.{self._toml_key(plugin_id)}]") + lines.append("enabled = true") + if options: + lines.append(f"options = {self._toml_value(options)}") + lines.append("") + with open(self.config_path, "w") as f: f.write("\n".join(lines).rstrip() + "\n") diff --git a/evalbench/test/codex_cli_test.py b/evalbench/test/codex_cli_test.py index 1027bf59..f6dbf515 100644 --- a/evalbench/test/codex_cli_test.py +++ b/evalbench/test/codex_cli_test.py @@ -57,3 +57,54 @@ def test_execute_cli_command_default_cwd(mock_popen, monkeypatch): mock_popen.assert_called_once() kwargs = mock_popen.call_args.kwargs assert kwargs.get("cwd") == generator.fake_home + + +@patch('generators.models.codex_cli.subprocess.run') +def test_register_codex_plugin_uses_merged_env(mock_run, monkeypatch): + monkeypatch.setenv("HOME", "/fake/real_home") + monkeypatch.setenv("PARENT_VAR", "parent_value") + + with ( + patch('generators.models.codex_cli.os.makedirs'), + patch('generators.models.codex_cli.open', create=True), + ): + generator = CodexCliGenerator({"model": "gpt-4", "env": {"GENERATOR_VAR": "gen_value"}}) + + with ( + patch('generators.models.codex_cli.os.path.exists', return_value=False), + patch('generators.models.codex_cli.open', create=True), + patch('generators.models.codex_cli.json.dump'), + ): + generator._register_codex_plugin("/fake/repo_dir", {"plugin_name": "my-plugin"}) + + mock_run.assert_called_once() + kwargs = mock_run.call_args.kwargs + passed_env = kwargs.get("env") + assert passed_env is not None + assert passed_env.get("PARENT_VAR") == "parent_value" + assert passed_env.get("GENERATOR_VAR") == "gen_value" + assert passed_env.get("HOME") == generator.fake_home + + +def test_write_config_toml_escapes_plugin_id(monkeypatch, tmp_path): + monkeypatch.setenv("HOME", "/fake/real_home") + + with ( + patch('generators.models.codex_cli.os.makedirs'), + patch('generators.models.codex_cli.open', create=True), + ): + generator = CodexCliGenerator({"model": "gpt-4"}) + + generator.enabled_plugins = { + "dak@evalbench-local-marketplace": {"opt1": "val1"}, + "clean_plugin": {} + } + + config_file = tmp_path / "config.toml" + generator.config_path = str(config_file) + + generator._write_config_toml() + + content = config_file.read_text() + assert '[plugins."dak@evalbench-local-marketplace"]' in content + assert '[plugins.clean_plugin]' in content