From 0c29551e8b9a64aa217f6029f369415bcfb447b3 Mon Sep 17 00:00:00 2001 From: Rob MacEachern Date: Wed, 6 May 2026 12:43:22 -0500 Subject: [PATCH 1/5] Add Codex MCP config support --- docs/agents.md | 2 +- docs/mcp.md | 3 + docs/project-structure.md | 4 + src/agents/codex.rs | 393 +++++++++++++++++++++++++++++++++++++- src/commands/clean.rs | 2 + src/commands/generate.rs | 10 + 6 files changed, 412 insertions(+), 2 deletions(-) diff --git a/docs/agents.md b/docs/agents.md index 62dc639..46d0fab 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -7,7 +7,7 @@ | **AMP** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | - | | | **Claude Code** | `CLAUDE.md` -> inlined file | `CLAUDE.md` -> `ai-rules/AGENTS.md` | `.mcp.json` | | | **Cline** | `.clinerules/AGENTS.md` -> inlined file | `.clinerules/AGENTS.md` -> `../ai-rules/AGENTS.md` | - | | -| **Codex** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | - | | +| **Codex** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | `.codex/config.toml` | Project-scoped config with generated MCP block | | **Copilot** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | - | | | **Cursor** | `.cursor/rules/*.mdc` | `AGENTS.md` -> `ai-rules/AGENTS.md` | `.cursor/mcp.json` | Symlink mode: only project root level | | **Firebender** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | `firebender.json` | Commands live in `.firebender/`; skills reuse `.agents/skills`; overlay supported | diff --git a/docs/mcp.md b/docs/mcp.md index 51d7e0e..02dcd4b 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -35,7 +35,10 @@ See the [Supported Agents](agents.md) table for which agents support MCP and the | Agent | MCP File Location | |-------|-------------------| | Claude Code | `.mcp.json` | +| Codex | `.codex/config.toml` | | Cursor | `.cursor/mcp.json` | | Firebender | `firebender.json` (generated when MCP and/or overlay config exists) | | Gemini | Embedded in `.gemini/settings.json` | | Roo | `.roo/mcp.json` | + +Codex uses TOML instead of JSON. `ai-rules generate` converts the shared `ai-rules/mcp.json` source into project-scoped `[mcp_servers.]` tables inside a managed block in `.codex/config.toml`, preserving any content outside that block. diff --git a/docs/project-structure.md b/docs/project-structure.md index 8dd87a2..97acb2b 100644 --- a/docs/project-structure.md +++ b/docs/project-structure.md @@ -44,6 +44,8 @@ monorepo/ │ ├── commands/ # Firebender commands (generated symlinks) ├── .agents/ │ └── skills/ # Shared AMP/Firebender skills (generated symlinks) +├── .codex/ +│ └── config.toml # Codex MCP config (generated block, if mcp.json exists) ├── .mcp.json # Root MCP config (generated) └── firebender.json # Root Firebender supplemental config (generated when needed) ``` @@ -70,6 +72,8 @@ project/ │ ├── commands/ # Firebender commands (generated symlinks) ├── .agents/ │ └── skills/ # Shared AMP/Firebender skills (generated symlinks) +├── .codex/ +│ └── config.toml # Codex MCP config (if mcp.json exists) ├── firebender.json # Supplemental Firebender config (if mcp.json or overlay exists) ├── .clinerules/ │ └── AGENTS.md # Symlink -> ../ai-rules/AGENTS.md diff --git a/src/agents/codex.rs b/src/agents/codex.rs index cdfaa72..5f841dc 100644 --- a/src/agents/codex.rs +++ b/src/agents/codex.rs @@ -1,4 +1,5 @@ use crate::agents::external_skills_generator::ExternalSkillsGenerator; +use crate::agents::mcp_generator::McpGeneratorTrait; use crate::agents::rule_generator::AgentRuleGenerator; use crate::agents::single_file_based::{ check_in_sync, clean_generated_files, generate_agent_file_contents, @@ -6,14 +7,20 @@ use crate::agents::single_file_based::{ use crate::agents::skills_generator::SkillsGeneratorTrait; use crate::constants::{AGENTS_MD_FILENAME, CODEX_SKILLS_DIR}; use crate::models::SourceFile; +use crate::operations::mcp_reader::{read_mcp_config, McpConfig, McpServerConfig}; use crate::utils::file_utils::{ check_agents_md_symlink, check_inlined_file_symlink, create_symlink_to_agents_md, - create_symlink_to_inlined_file, + create_symlink_to_inlined_file, ensure_trailing_newline, }; use anyhow::Result; use std::collections::HashMap; +use std::fs; use std::path::{Path, PathBuf}; +const CODEX_CONFIG_TOML: &str = ".codex/config.toml"; +const MCP_GENERATED_START: &str = "# AI Rules MCP - Generated Servers"; +const MCP_GENERATED_END: &str = "# End AI Rules MCP"; + pub struct CodexGenerator { name: String, output_filename: String, @@ -96,11 +103,228 @@ impl AgentRuleGenerator for CodexGenerator { check_inlined_file_symlink(current_dir, &output_file) } + fn mcp_generator(&self) -> Option> { + Some(Box::new(CodexMcpGenerator)) + } + fn skills_generator(&self) -> Option> { Some(Box::new(ExternalSkillsGenerator::new(CODEX_SKILLS_DIR))) } } +struct CodexMcpGenerator; + +impl McpGeneratorTrait for CodexMcpGenerator { + fn generate_mcp(&self, current_dir: &Path) -> HashMap { + let mut files = HashMap::new(); + + let source_mcp_content = match read_mcp_config(current_dir) { + Ok(Some(content)) => content, + _ => return files, + }; + + let source_config: McpConfig = match serde_json::from_str(&source_mcp_content) { + Ok(config) => config, + Err(_) => return files, + }; + + let target_path = current_dir.join(CODEX_CONFIG_TOML); + let existing_content = fs::read_to_string(&target_path).unwrap_or_default(); + let base_content = remove_generated_mcp_block(&existing_content); + + let generated_block = generate_codex_mcp_block(&source_config); + if generated_block.is_empty() && base_content.trim().is_empty() { + return files; + } + + let content = merge_generated_mcp_block(&base_content, &generated_block); + files.insert(target_path, content); + files + } + + fn clean_mcp(&self, current_dir: &Path) -> Result<()> { + let target_path = current_dir.join(CODEX_CONFIG_TOML); + if !target_path.exists() { + return Ok(()); + } + + let content = fs::read_to_string(&target_path)?; + let cleaned = remove_generated_mcp_block(&content); + if cleaned.trim().is_empty() { + fs::remove_file(target_path)?; + } else if cleaned != content { + fs::write(target_path, ensure_trailing_newline(cleaned))?; + } + + Ok(()) + } + + fn check_mcp(&self, current_dir: &Path) -> Result { + let target_path = current_dir.join(CODEX_CONFIG_TOML); + + let source_mcp_content = match read_mcp_config(current_dir)? { + Some(content) => content, + None => { + if !target_path.exists() { + return Ok(true); + } + + let content = fs::read_to_string(&target_path)?; + return Ok(!content.contains(MCP_GENERATED_START)); + } + }; + + if !target_path.exists() { + return Ok(false); + } + + let source_config: McpConfig = serde_json::from_str(&source_mcp_content)?; + let actual = fs::read_to_string(&target_path)?; + let base_content = remove_generated_mcp_block(&actual); + let expected = + merge_generated_mcp_block(&base_content, &generate_codex_mcp_block(&source_config)); + + Ok(actual == expected) + } + + fn mcp_gitignore_patterns(&self) -> Vec { + vec![CODEX_CONFIG_TOML.to_string()] + } + + fn box_clone(&self) -> Box { + Box::new(Self) + } +} + +fn remove_generated_mcp_block(content: &str) -> String { + let Some(start) = content.find(MCP_GENERATED_START) else { + return content.to_string(); + }; + let Some(relative_end) = content[start..].find(MCP_GENERATED_END) else { + return content.to_string(); + }; + + let end = start + relative_end + MCP_GENERATED_END.len(); + let mut result = content.to_string(); + + let range_start = start.saturating_sub(if start > 0 && result.as_bytes()[start - 1] == b'\n' { + 1 + } else { + 0 + }); + let range_end = if result.as_bytes().get(end) == Some(&b'\n') { + end + 1 + } else { + end + }; + + result.replace_range(range_start..range_end, ""); + result.trim_end().to_string() +} + +fn merge_generated_mcp_block(base_content: &str, generated_block: &str) -> String { + if generated_block.is_empty() { + return ensure_trailing_newline(base_content.trim_end().to_string()); + } + + let mut content = base_content.trim_end().to_string(); + if !content.is_empty() { + content.push_str("\n\n"); + } + content.push_str(generated_block); + ensure_trailing_newline(content) +} + +fn generate_codex_mcp_block(config: &McpConfig) -> String { + if config.mcp_servers.is_empty() { + return String::new(); + } + + let mut server_names: Vec<_> = config.mcp_servers.keys().collect(); + server_names.sort(); + + let mut block = String::from(MCP_GENERATED_START); + block.push('\n'); + + for server_name in server_names { + let server_config = config + .mcp_servers + .get(server_name) + .expect("server name came from mcp_servers keys"); + + block.push('\n'); + block.push_str(&format!("[mcp_servers.{}]\n", toml_quoted_key(server_name))); + + match server_config { + McpServerConfig::Command { command, args, env } => { + block.push_str(&format!("command = {}\n", toml_string(command))); + if let Some(args) = args { + block.push_str(&format!("args = {}\n", toml_string_array(args))); + } + if let Some(env) = env { + append_toml_string_table(&mut block, server_name, "env", env); + } + } + McpServerConfig::Http { url, headers, .. } => { + block.push_str(&format!("url = {}\n", toml_string(url))); + if let Some(headers) = headers { + append_toml_string_table(&mut block, server_name, "http_headers", headers); + } + } + } + } + + block.push('\n'); + block.push_str(MCP_GENERATED_END); + block +} + +fn append_toml_string_table( + block: &mut String, + server_name: &str, + table_name: &str, + values: &HashMap, +) { + if values.is_empty() { + return; + } + + block.push('\n'); + block.push_str(&format!( + "[mcp_servers.{}.{}]\n", + toml_quoted_key(server_name), + table_name + )); + + let mut keys: Vec<_> = values.keys().collect(); + keys.sort(); + for key in keys { + let value = values.get(key).expect("value key came from values keys"); + block.push_str(&format!( + "{} = {}\n", + toml_quoted_key(key), + toml_string(value) + )); + } +} + +fn toml_string(value: &str) -> String { + serde_json::to_string(value).expect("serializing a string should not fail") +} + +fn toml_string_array(values: &[String]) -> String { + let values = values + .iter() + .map(|value| toml_string(value)) + .collect::>() + .join(", "); + format!("[{values}]") +} + +fn toml_quoted_key(value: &str) -> String { + toml_string(value) +} + #[cfg(test)] mod tests { use super::*; @@ -226,4 +450,171 @@ mod tests { let result = generator.check_symlink(temp_dir.path()).unwrap(); assert!(result); } + + const TEST_MCP_CONFIG: &str = r#"{ + "mcpServers": { + "test-server": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-test"], + "env": { + "API_KEY": "${API_KEY}" + } + } + } +}"#; + + const TEST_HTTP_MCP_CONFIG: &str = r#"{ + "mcpServers": { + "figma": { + "type": "http", + "url": "https://mcp.figma.com/mcp", + "headers": { + "X-Figma-Region": "us-east-1" + } + } + } +}"#; + + #[test] + fn test_codex_generator_has_mcp_generator() { + let generator = CodexGenerator::new(); + + assert!(generator.mcp_generator().is_some()); + } + + #[test] + fn test_codex_mcp_generator_writes_config_toml() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + + let mcp_gen = generator.mcp_generator().unwrap(); + let files = mcp_gen.generate_mcp(temp_dir.path()); + + let expected_path = temp_dir.path().join(".codex/config.toml"); + let content = files.get(&expected_path).unwrap(); + + assert!(content.contains(MCP_GENERATED_START)); + assert!(content.contains("[mcp_servers.\"test-server\"]")); + assert!(content.contains("command = \"npx\"")); + assert!(content.contains("args = [\"-y\", \"@modelcontextprotocol/server-test\"]")); + assert!(content.contains("[mcp_servers.\"test-server\".env]")); + assert!(content.contains("\"API_KEY\" = \"${API_KEY}\"")); + } + + #[test] + fn test_codex_mcp_generator_converts_http_headers() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_HTTP_MCP_CONFIG); + + let mcp_gen = generator.mcp_generator().unwrap(); + let files = mcp_gen.generate_mcp(temp_dir.path()); + + let expected_path = temp_dir.path().join(".codex/config.toml"); + let content = files.get(&expected_path).unwrap(); + + assert!(content.contains("[mcp_servers.\"figma\"]")); + assert!(content.contains("url = \"https://mcp.figma.com/mcp\"")); + assert!(content.contains("[mcp_servers.\"figma\".http_headers]")); + assert!(content.contains("\"X-Figma-Region\" = \"us-east-1\"")); + assert!(!content.contains("type = \"http\"")); + assert!(!content.contains("headers =")); + } + + #[test] + fn test_codex_mcp_generator_preserves_existing_config() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + create_file( + temp_dir.path(), + ".codex/config.toml", + "model = \"gpt-5.2\"\n\n[mcp_servers.user]\ncommand = \"custom\"\n", + ); + + let mcp_gen = generator.mcp_generator().unwrap(); + let files = mcp_gen.generate_mcp(temp_dir.path()); + let content = files + .get(&temp_dir.path().join(".codex/config.toml")) + .unwrap(); + + assert!(content.contains("model = \"gpt-5.2\"")); + assert!(content.contains("[mcp_servers.user]")); + assert!(content.contains("command = \"custom\"")); + assert!(content.contains("[mcp_servers.\"test-server\"]")); + } + + #[test] + fn test_codex_mcp_generator_clean_removes_only_generated_block() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + let mcp_gen = generator.mcp_generator().unwrap(); + + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + let files = mcp_gen.generate_mcp(temp_dir.path()); + let generated = files + .get(&temp_dir.path().join(".codex/config.toml")) + .unwrap(); + create_file( + temp_dir.path(), + ".codex/config.toml", + &format!("model = \"gpt-5.2\"\n\n{generated}"), + ); + + mcp_gen.clean_mcp(temp_dir.path()).unwrap(); + + let content = std::fs::read_to_string(temp_dir.path().join(".codex/config.toml")).unwrap(); + assert_eq!(content, "model = \"gpt-5.2\"\n"); + } + + #[test] + fn test_codex_mcp_generator_check_in_sync() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + let mcp_gen = generator.mcp_generator().unwrap(); + + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + let files = mcp_gen.generate_mcp(temp_dir.path()); + let content = files + .get(&temp_dir.path().join(".codex/config.toml")) + .unwrap(); + create_file(temp_dir.path(), ".codex/config.toml", content); + + assert!(mcp_gen.check_mcp(temp_dir.path()).unwrap()); + } + + #[test] + fn test_codex_mcp_generator_check_out_of_sync() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + let mcp_gen = generator.mcp_generator().unwrap(); + + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + create_file( + temp_dir.path(), + ".codex/config.toml", + "model = \"gpt-5.2\"\n", + ); + + assert!(!mcp_gen.check_mcp(temp_dir.path()).unwrap()); + } + + #[test] + fn test_codex_mcp_generator_check_no_source_with_generated_block() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + let mcp_gen = generator.mcp_generator().unwrap(); + + create_file( + temp_dir.path(), + ".codex/config.toml", + "# AI Rules MCP - Generated Servers\n\n[mcp_servers.\"test-server\"]\ncommand = \"npx\"\n# End AI Rules MCP\n", + ); + + assert!(!mcp_gen.check_mcp(temp_dir.path()).unwrap()); + } } diff --git a/src/commands/clean.rs b/src/commands/clean.rs index a9427c4..96e12fc 100644 --- a/src/commands/clean.rs +++ b/src/commands/clean.rs @@ -205,6 +205,7 @@ Test rule content"#; crate::cli::ResolvedGenerateArgs { agents: Some(vec![ "claude".to_string(), + "codex".to_string(), "cursor".to_string(), "roo".to_string(), ]), @@ -226,6 +227,7 @@ Test rule content"#; ".cursor/rules/ai-rules-generated-test.mdc", AGENTS_MD_FILENAME, ".mcp.json", + ".codex/config.toml", ".cursor/mcp.json", ".roo/mcp.json", ]; diff --git a/src/commands/generate.rs b/src/commands/generate.rs index 006963a..762bbed 100644 --- a/src/commands/generate.rs +++ b/src/commands/generate.rs @@ -642,6 +642,7 @@ New body content"#; let args = ResolvedGenerateArgs { agents: Some(vec![ "claude".to_string(), + "codex".to_string(), "cursor".to_string(), "roo".to_string(), ]), @@ -657,12 +658,21 @@ New body content"#; assert_file_exists(temp_dir.path(), AGENTS_MD_FILENAME); // Roo now uses AGENTS.md assert_file_exists(temp_dir.path(), ".mcp.json"); + assert_file_exists(temp_dir.path(), ".codex/config.toml"); assert_file_exists(temp_dir.path(), ".cursor/mcp.json"); assert_file_exists(temp_dir.path(), ".roo/mcp.json"); let mcp_content = std::fs::read_to_string(temp_dir.path().join(".mcp.json")).unwrap(); assert_eq!(mcp_content.trim(), TEST_MCP_CONFIG.trim()); + let codex_mcp_content = + std::fs::read_to_string(temp_dir.path().join(".codex/config.toml")).unwrap(); + assert!(codex_mcp_content.contains("[mcp_servers.\"test-server\"]")); + assert!(codex_mcp_content.contains("command = \"npx\"")); + assert!( + codex_mcp_content.contains("args = [\"-y\", \"@modelcontextprotocol/server-test\"]") + ); + let cursor_mcp_content = std::fs::read_to_string(temp_dir.path().join(".cursor/mcp.json")).unwrap(); assert_eq!(cursor_mcp_content.trim(), TEST_MCP_CONFIG.trim()); From 78c84e681f0a60447c00d2b4a6b7568df1f96221 Mon Sep 17 00:00:00 2001 From: Rob MacEachern Date: Wed, 6 May 2026 14:46:08 -0500 Subject: [PATCH 2/5] Match Codex MCP config overlay behavior --- Cargo.lock | 60 +++++++ Cargo.toml | 1 + docs/agents.md | 13 +- docs/mcp.md | 2 +- docs/project-structure.md | 6 +- src/agents/codex.rs | 369 +++++++++++++++++++++----------------- src/commands/generate.rs | 15 +- 7 files changed, 291 insertions(+), 175 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 676c39b..09e72b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -24,6 +24,7 @@ dependencies = [ "serde_json", "serde_yaml", "tempfile", + "toml", "which", ] @@ -508,6 +509,15 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_spanned" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +dependencies = [ + "serde", +] + [[package]] name = "serde_yaml" version = "0.9.34+deprecated" @@ -568,6 +578,47 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "toml" +version = "0.8.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit", +] + +[[package]] +name = "toml_datetime" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" +dependencies = [ + "serde", +] + +[[package]] +name = "toml_edit" +version = "0.22.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "toml_write", + "winnow", +] + +[[package]] +name = "toml_write" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" + [[package]] name = "unicode-ident" version = "1.0.18" @@ -859,6 +910,15 @@ version = "0.53.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "271414315aff87387382ec3d271b52d7ae78726f5d44ac98b4f4030c91880486" +[[package]] +name = "winnow" +version = "0.7.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" +dependencies = [ + "memchr", +] + [[package]] name = "winsafe" version = "0.0.19" diff --git a/Cargo.toml b/Cargo.toml index 5a71ee9..3e4f920 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ clap = { version = "4.4", features = ["derive"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" serde_yaml = "0.9" +toml = "0.8" cliclack = "0.3.6" regex = "1.10" which = "8.0.0" diff --git a/docs/agents.md b/docs/agents.md index 46d0fab..22c8da5 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -7,7 +7,7 @@ | **AMP** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | - | | | **Claude Code** | `CLAUDE.md` -> inlined file | `CLAUDE.md` -> `ai-rules/AGENTS.md` | `.mcp.json` | | | **Cline** | `.clinerules/AGENTS.md` -> inlined file | `.clinerules/AGENTS.md` -> `../ai-rules/AGENTS.md` | - | | -| **Codex** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | `.codex/config.toml` | Project-scoped config with generated MCP block | +| **Codex** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | `.codex/config.toml` | Optional `ai-rules/codex-config.toml` overlay supported | | **Copilot** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | - | | | **Cursor** | `.cursor/rules/*.mdc` | `AGENTS.md` -> `ai-rules/AGENTS.md` | `.cursor/mcp.json` | Symlink mode: only project root level | | **Firebender** | `AGENTS.md` -> inlined file | `AGENTS.md` -> `ai-rules/AGENTS.md` | `firebender.json` | Commands live in `.firebender/`; skills reuse `.agents/skills`; overlay supported | @@ -26,3 +26,14 @@ Firebender supports overlay configuration files. To customize your configuration `firebender.json` is generated as supplemental config when `ai-rules/mcp.json` and/or `ai-rules/firebender-overlay.json` exists. **MCP Integration:** If you have `ai-rules/mcp.json`, the MCP servers are merged into `firebender.json` first, then the overlay is applied. This allows you to override MCP configurations in the overlay if needed. + +## Codex Overlay Support + +Codex supports overlay configuration files. To customize your configuration: + +1. Create `ai-rules/codex-config.toml` in the same parent directory as your generated `.codex/config.toml` +2. Any values defined in the overlay file will be merged into the generated Codex configuration, with overlay values taking precedence + +`.codex/config.toml` is generated when `ai-rules/mcp.json` and/or `ai-rules/codex-config.toml` exists. + +**MCP Integration:** If you have `ai-rules/mcp.json`, the MCP servers are converted into Codex `[mcp_servers]` TOML first, then the overlay is applied. This allows you to override MCP configurations in the overlay if needed. diff --git a/docs/mcp.md b/docs/mcp.md index 02dcd4b..ed66ef8 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -41,4 +41,4 @@ See the [Supported Agents](agents.md) table for which agents support MCP and the | Gemini | Embedded in `.gemini/settings.json` | | Roo | `.roo/mcp.json` | -Codex uses TOML instead of JSON. `ai-rules generate` converts the shared `ai-rules/mcp.json` source into project-scoped `[mcp_servers.]` tables inside a managed block in `.codex/config.toml`, preserving any content outside that block. +Codex uses TOML instead of JSON. `ai-rules generate` converts the shared `ai-rules/mcp.json` source into project-scoped `[mcp_servers.]` tables in `.codex/config.toml`. If `ai-rules/codex-config.toml` exists, it is merged over the generated config so projects can add or override Codex settings. diff --git a/docs/project-structure.md b/docs/project-structure.md index 97acb2b..2369aae 100644 --- a/docs/project-structure.md +++ b/docs/project-structure.md @@ -17,6 +17,7 @@ monorepo/ │ │ └── SKILL.md │ ├── general.md # Repository-wide rules │ ├── ai-rules-config.yaml # Configuration +│ ├── codex-config.toml # Codex config overlay (optional) │ └── mcp.json # MCP server configuration │ ├── frontend/ # Frontend application @@ -45,7 +46,7 @@ monorepo/ ├── .agents/ │ └── skills/ # Shared AMP/Firebender skills (generated symlinks) ├── .codex/ -│ └── config.toml # Codex MCP config (generated block, if mcp.json exists) +│ └── config.toml # Codex config (generated when mcp.json or codex-config.toml exists) ├── .mcp.json # Root MCP config (generated) └── firebender.json # Root Firebender supplemental config (generated when needed) ``` @@ -63,6 +64,7 @@ project/ │ ├── skills/ # User-defined skills (optional) │ │ └── debugging/ │ │ └── SKILL.md +│ ├── codex-config.toml # Codex config overlay (optional) │ └── mcp.json # MCP config (optional) │ ├── CLAUDE.md # Symlink -> ai-rules/AGENTS.md @@ -73,7 +75,7 @@ project/ ├── .agents/ │ └── skills/ # Shared AMP/Firebender skills (generated symlinks) ├── .codex/ -│ └── config.toml # Codex MCP config (if mcp.json exists) +│ └── config.toml # Codex config (if mcp.json or codex-config.toml exists) ├── firebender.json # Supplemental Firebender config (if mcp.json or overlay exists) ├── .clinerules/ │ └── AGENTS.md # Symlink -> ../ai-rules/AGENTS.md diff --git a/src/agents/codex.rs b/src/agents/codex.rs index 5f841dc..0714434 100644 --- a/src/agents/codex.rs +++ b/src/agents/codex.rs @@ -5,21 +5,21 @@ use crate::agents::single_file_based::{ check_in_sync, clean_generated_files, generate_agent_file_contents, }; use crate::agents::skills_generator::SkillsGeneratorTrait; -use crate::constants::{AGENTS_MD_FILENAME, CODEX_SKILLS_DIR}; +use crate::constants::{AGENTS_MD_FILENAME, AI_RULE_SOURCE_DIR, CODEX_SKILLS_DIR}; use crate::models::SourceFile; use crate::operations::mcp_reader::{read_mcp_config, McpConfig, McpServerConfig}; use crate::utils::file_utils::{ check_agents_md_symlink, check_inlined_file_symlink, create_symlink_to_agents_md, create_symlink_to_inlined_file, ensure_trailing_newline, }; -use anyhow::Result; +use anyhow::{Context, Result}; use std::collections::HashMap; use std::fs; use std::path::{Path, PathBuf}; +use toml::{Table, Value}; const CODEX_CONFIG_TOML: &str = ".codex/config.toml"; -const MCP_GENERATED_START: &str = "# AI Rules MCP - Generated Servers"; -const MCP_GENERATED_END: &str = "# End AI Rules MCP"; +const CODEX_CONFIG_OVERLAY_TOML: &str = "codex-config.toml"; pub struct CodexGenerator { name: String, @@ -118,73 +118,29 @@ impl McpGeneratorTrait for CodexMcpGenerator { fn generate_mcp(&self, current_dir: &Path) -> HashMap { let mut files = HashMap::new(); - let source_mcp_content = match read_mcp_config(current_dir) { - Ok(Some(content)) => content, - _ => return files, - }; - - let source_config: McpConfig = match serde_json::from_str(&source_mcp_content) { - Ok(config) => config, - Err(_) => return files, - }; - - let target_path = current_dir.join(CODEX_CONFIG_TOML); - let existing_content = fs::read_to_string(&target_path).unwrap_or_default(); - let base_content = remove_generated_mcp_block(&existing_content); - - let generated_block = generate_codex_mcp_block(&source_config); - if generated_block.is_empty() && base_content.trim().is_empty() { - return files; + if let Ok(Some(config)) = generate_codex_config(current_dir) { + files.insert(current_dir.join(CODEX_CONFIG_TOML), config); } - let content = merge_generated_mcp_block(&base_content, &generated_block); - files.insert(target_path, content); files } fn clean_mcp(&self, current_dir: &Path) -> Result<()> { let target_path = current_dir.join(CODEX_CONFIG_TOML); - if !target_path.exists() { - return Ok(()); - } - - let content = fs::read_to_string(&target_path)?; - let cleaned = remove_generated_mcp_block(&content); - if cleaned.trim().is_empty() { - fs::remove_file(target_path)?; - } else if cleaned != content { - fs::write(target_path, ensure_trailing_newline(cleaned))?; + if target_path.exists() { + fs::remove_file(&target_path) + .with_context(|| format!("Failed to remove {}", target_path.display()))?; } - Ok(()) } fn check_mcp(&self, current_dir: &Path) -> Result { let target_path = current_dir.join(CODEX_CONFIG_TOML); - let source_mcp_content = match read_mcp_config(current_dir)? { - Some(content) => content, - None => { - if !target_path.exists() { - return Ok(true); - } - - let content = fs::read_to_string(&target_path)?; - return Ok(!content.contains(MCP_GENERATED_START)); - } - }; - - if !target_path.exists() { - return Ok(false); + match generate_codex_config(current_dir)? { + Some(expected_content) => file_matches_expected(&target_path, &expected_content), + None => Ok(!target_path.exists()), } - - let source_config: McpConfig = serde_json::from_str(&source_mcp_content)?; - let actual = fs::read_to_string(&target_path)?; - let base_content = remove_generated_mcp_block(&actual); - let expected = - merge_generated_mcp_block(&base_content, &generate_codex_mcp_block(&source_config)); - - Ok(actual == expected) } fn mcp_gitignore_patterns(&self) -> Vec { @@ -196,133 +152,126 @@ impl McpGeneratorTrait for CodexMcpGenerator { } } -fn remove_generated_mcp_block(content: &str) -> String { - let Some(start) = content.find(MCP_GENERATED_START) else { - return content.to_string(); - }; - let Some(relative_end) = content[start..].find(MCP_GENERATED_END) else { - return content.to_string(); - }; - - let end = start + relative_end + MCP_GENERATED_END.len(); - let mut result = content.to_string(); - - let range_start = start.saturating_sub(if start > 0 && result.as_bytes()[start - 1] == b'\n' { - 1 - } else { - 0 - }); - let range_end = if result.as_bytes().get(end) == Some(&b'\n') { - end + 1 - } else { - end - }; - - result.replace_range(range_start..range_end, ""); - result.trim_end().to_string() -} +fn generate_codex_config(current_dir: &Path) -> Result> { + let mut codex_config = Value::Table(Table::new()); + let mut has_content = false; -fn merge_generated_mcp_block(base_content: &str, generated_block: &str) -> String { - if generated_block.is_empty() { - return ensure_trailing_newline(base_content.trim_end().to_string()); + if let Some(source_mcp_content) = read_mcp_config(current_dir)? { + let source_config: McpConfig = serde_json::from_str(&source_mcp_content)?; + if let Some(config_table) = codex_config.as_table_mut() { + config_table.insert( + "mcp_servers".to_string(), + Value::Table(generate_codex_mcp_servers_table(&source_config)), + ); + } + has_content = true; } - let mut content = base_content.trim_end().to_string(); - if !content.is_empty() { - content.push_str("\n\n"); + let overlay_path = current_dir + .join(AI_RULE_SOURCE_DIR) + .join(CODEX_CONFIG_OVERLAY_TOML); + if overlay_path.exists() { + let overlay_content = fs::read_to_string(&overlay_path) + .with_context(|| format!("Failed to read overlay file: {}", overlay_path.display()))?; + + let overlay_config: Value = overlay_content + .parse() + .with_context(|| format!("Invalid TOML in overlay file: {}", overlay_path.display()))?; + + merge_toml_values(&mut codex_config, &overlay_config); + has_content = true; } - content.push_str(generated_block); - ensure_trailing_newline(content) -} -fn generate_codex_mcp_block(config: &McpConfig) -> String { - if config.mcp_servers.is_empty() { - return String::new(); + if !has_content { + return Ok(None); } + let toml_string = toml::to_string_pretty(&codex_config) + .with_context(|| "Failed to serialize Codex configuration to TOML")?; + Ok(Some(ensure_trailing_newline(toml_string))) +} + +fn generate_codex_mcp_servers_table(config: &McpConfig) -> Table { + let mut mcp_servers = Table::new(); + let mut server_names: Vec<_> = config.mcp_servers.keys().collect(); server_names.sort(); - let mut block = String::from(MCP_GENERATED_START); - block.push('\n'); - for server_name in server_names { let server_config = config .mcp_servers .get(server_name) .expect("server name came from mcp_servers keys"); - block.push('\n'); - block.push_str(&format!("[mcp_servers.{}]\n", toml_quoted_key(server_name))); + let mut server_table = Table::new(); match server_config { McpServerConfig::Command { command, args, env } => { - block.push_str(&format!("command = {}\n", toml_string(command))); + server_table.insert("command".to_string(), Value::String(command.clone())); if let Some(args) = args { - block.push_str(&format!("args = {}\n", toml_string_array(args))); + server_table.insert( + "args".to_string(), + Value::Array(args.iter().cloned().map(Value::String).collect()), + ); } if let Some(env) = env { - append_toml_string_table(&mut block, server_name, "env", env); + server_table.insert("env".to_string(), Value::Table(string_map_to_table(env))); } } McpServerConfig::Http { url, headers, .. } => { - block.push_str(&format!("url = {}\n", toml_string(url))); + server_table.insert("url".to_string(), Value::String(url.clone())); if let Some(headers) = headers { - append_toml_string_table(&mut block, server_name, "http_headers", headers); + server_table.insert( + "http_headers".to_string(), + Value::Table(string_map_to_table(headers)), + ); } } } - } - block.push('\n'); - block.push_str(MCP_GENERATED_END); - block -} - -fn append_toml_string_table( - block: &mut String, - server_name: &str, - table_name: &str, - values: &HashMap, -) { - if values.is_empty() { - return; + mcp_servers.insert(server_name.clone(), Value::Table(server_table)); } - block.push('\n'); - block.push_str(&format!( - "[mcp_servers.{}.{}]\n", - toml_quoted_key(server_name), - table_name - )); + mcp_servers +} +fn string_map_to_table(values: &HashMap) -> Table { + let mut table = Table::new(); let mut keys: Vec<_> = values.keys().collect(); keys.sort(); + for key in keys { let value = values.get(key).expect("value key came from values keys"); - block.push_str(&format!( - "{} = {}\n", - toml_quoted_key(key), - toml_string(value) - )); + table.insert(key.clone(), Value::String(value.clone())); } -} -fn toml_string(value: &str) -> String { - serde_json::to_string(value).expect("serializing a string should not fail") + table } -fn toml_string_array(values: &[String]) -> String { - let values = values - .iter() - .map(|value| toml_string(value)) - .collect::>() - .join(", "); - format!("[{values}]") +fn merge_toml_values(base: &mut Value, overlay: &Value) { + if let (Some(base_table), Some(overlay_table)) = (base.as_table_mut(), overlay.as_table()) { + for (key, value) in overlay_table { + match base_table.get_mut(key) { + Some(base_value) if base_value.is_table() && value.is_table() => { + merge_toml_values(base_value, value); + } + _ => { + base_table.insert(key.clone(), value.clone()); + } + } + } + } } -fn toml_quoted_key(value: &str) -> String { - toml_string(value) +fn file_matches_expected(file_path: &Path, expected_content: &str) -> Result { + if !file_path.exists() { + return Ok(false); + } + + let actual_content = fs::read_to_string(file_path) + .with_context(|| format!("Failed to read {}", file_path.display()))?; + + Ok(actual_content == expected_content) } #[cfg(test)] @@ -494,13 +443,19 @@ mod tests { let expected_path = temp_dir.path().join(".codex/config.toml"); let content = files.get(&expected_path).unwrap(); + let parsed: Value = content.parse().unwrap(); + let test_server = &parsed["mcp_servers"]["test-server"]; - assert!(content.contains(MCP_GENERATED_START)); - assert!(content.contains("[mcp_servers.\"test-server\"]")); - assert!(content.contains("command = \"npx\"")); - assert!(content.contains("args = [\"-y\", \"@modelcontextprotocol/server-test\"]")); - assert!(content.contains("[mcp_servers.\"test-server\".env]")); - assert!(content.contains("\"API_KEY\" = \"${API_KEY}\"")); + assert_eq!(test_server["command"].as_str(), Some("npx")); + assert_eq!( + test_server["args"].as_array().unwrap()[0].as_str(), + Some("-y") + ); + assert_eq!( + test_server["args"].as_array().unwrap()[1].as_str(), + Some("@modelcontextprotocol/server-test") + ); + assert_eq!(test_server["env"]["API_KEY"].as_str(), Some("${API_KEY}")); } #[test] @@ -515,25 +470,35 @@ mod tests { let expected_path = temp_dir.path().join(".codex/config.toml"); let content = files.get(&expected_path).unwrap(); + let parsed: Value = content.parse().unwrap(); + let figma = &parsed["mcp_servers"]["figma"]; - assert!(content.contains("[mcp_servers.\"figma\"]")); - assert!(content.contains("url = \"https://mcp.figma.com/mcp\"")); - assert!(content.contains("[mcp_servers.\"figma\".http_headers]")); - assert!(content.contains("\"X-Figma-Region\" = \"us-east-1\"")); - assert!(!content.contains("type = \"http\"")); - assert!(!content.contains("headers =")); + assert_eq!(figma["url"].as_str(), Some("https://mcp.figma.com/mcp")); + assert_eq!( + figma["http_headers"]["X-Figma-Region"].as_str(), + Some("us-east-1") + ); + assert!(figma.get("type").is_none()); + assert!(figma.get("headers").is_none()); } #[test] - fn test_codex_mcp_generator_preserves_existing_config() { + fn test_codex_mcp_generator_merges_overlay_with_mcp() { let temp_dir = TempDir::new().unwrap(); let generator = CodexGenerator::new(); create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); create_file( temp_dir.path(), - ".codex/config.toml", - "model = \"gpt-5.2\"\n\n[mcp_servers.user]\ncommand = \"custom\"\n", + "ai-rules/codex-config.toml", + r#"model = "gpt-5.2" + +[mcp_servers."test-server"] +command = "python" + +[mcp_servers.user] +command = "custom" +"#, ); let mcp_gen = generator.mcp_generator().unwrap(); @@ -541,34 +506,70 @@ mod tests { let content = files .get(&temp_dir.path().join(".codex/config.toml")) .unwrap(); + let parsed: Value = content.parse().unwrap(); - assert!(content.contains("model = \"gpt-5.2\"")); - assert!(content.contains("[mcp_servers.user]")); - assert!(content.contains("command = \"custom\"")); - assert!(content.contains("[mcp_servers.\"test-server\"]")); + assert_eq!(parsed["model"].as_str(), Some("gpt-5.2")); + assert_eq!( + parsed["mcp_servers"]["test-server"]["command"].as_str(), + Some("python") + ); + assert_eq!( + parsed["mcp_servers"]["test-server"]["args"] + .as_array() + .unwrap()[0] + .as_str(), + Some("-y") + ); + assert_eq!( + parsed["mcp_servers"]["user"]["command"].as_str(), + Some("custom") + ); } #[test] - fn test_codex_mcp_generator_clean_removes_only_generated_block() { + fn test_codex_mcp_generator_overlay_only() { let temp_dir = TempDir::new().unwrap(); let generator = CodexGenerator::new(); - let mcp_gen = generator.mcp_generator().unwrap(); - create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + create_file( + temp_dir.path(), + "ai-rules/codex-config.toml", + r#"model = "gpt-5.2" + +[mcp_servers.user] +command = "custom" +"#, + ); + + let mcp_gen = generator.mcp_generator().unwrap(); let files = mcp_gen.generate_mcp(temp_dir.path()); - let generated = files + let content = files .get(&temp_dir.path().join(".codex/config.toml")) .unwrap(); + let parsed: Value = content.parse().unwrap(); + + assert_eq!(parsed["model"].as_str(), Some("gpt-5.2")); + assert_eq!( + parsed["mcp_servers"]["user"]["command"].as_str(), + Some("custom") + ); + } + + #[test] + fn test_codex_mcp_generator_clean_removes_config() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + let mcp_gen = generator.mcp_generator().unwrap(); + create_file( temp_dir.path(), ".codex/config.toml", - &format!("model = \"gpt-5.2\"\n\n{generated}"), + "model = \"gpt-5.2\"\n\n[mcp_servers.user]\ncommand = \"custom\"\n", ); mcp_gen.clean_mcp(temp_dir.path()).unwrap(); - let content = std::fs::read_to_string(temp_dir.path().join(".codex/config.toml")).unwrap(); - assert_eq!(content, "model = \"gpt-5.2\"\n"); + assert!(!temp_dir.path().join(".codex/config.toml").exists()); } #[test] @@ -612,9 +613,43 @@ mod tests { create_file( temp_dir.path(), ".codex/config.toml", - "# AI Rules MCP - Generated Servers\n\n[mcp_servers.\"test-server\"]\ncommand = \"npx\"\n# End AI Rules MCP\n", + "[mcp_servers.\"test-server\"]\ncommand = \"npx\"\n", ); assert!(!mcp_gen.check_mcp(temp_dir.path()).unwrap()); } + + #[test] + fn test_codex_mcp_generator_overlay_collision_writes_valid_toml() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + let mcp_gen = generator.mcp_generator().unwrap(); + + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + create_file( + temp_dir.path(), + "ai-rules/codex-config.toml", + r#"[mcp_servers."test-server"] +command = "custom" +"#, + ); + + let files = mcp_gen.generate_mcp(temp_dir.path()); + let content = files + .get(&temp_dir.path().join(".codex/config.toml")) + .unwrap(); + let parsed: Value = content.parse().unwrap(); + + assert_eq!( + parsed["mcp_servers"]["test-server"]["command"].as_str(), + Some("custom") + ); + assert_eq!( + parsed["mcp_servers"]["test-server"]["args"] + .as_array() + .unwrap()[0] + .as_str(), + Some("-y") + ); + } } diff --git a/src/commands/generate.rs b/src/commands/generate.rs index 762bbed..dd846f9 100644 --- a/src/commands/generate.rs +++ b/src/commands/generate.rs @@ -667,10 +667,17 @@ New body content"#; let codex_mcp_content = std::fs::read_to_string(temp_dir.path().join(".codex/config.toml")).unwrap(); - assert!(codex_mcp_content.contains("[mcp_servers.\"test-server\"]")); - assert!(codex_mcp_content.contains("command = \"npx\"")); - assert!( - codex_mcp_content.contains("args = [\"-y\", \"@modelcontextprotocol/server-test\"]") + let codex_mcp: toml::Value = codex_mcp_content.parse().unwrap(); + assert_eq!( + codex_mcp["mcp_servers"]["test-server"]["command"].as_str(), + Some("npx") + ); + assert_eq!( + codex_mcp["mcp_servers"]["test-server"]["args"] + .as_array() + .unwrap()[0] + .as_str(), + Some("-y") ); let cursor_mcp_content = From 366dd72c08f4712b96b7ae6d7e43d8733602f5b9 Mon Sep 17 00:00:00 2001 From: Rob MacEachern Date: Wed, 6 May 2026 15:11:02 -0500 Subject: [PATCH 3/5] Expand Codex MCP overlay test coverage --- src/agents/codex.rs | 46 ++++++++++++++++++++++++++++++ src/commands/status.rs | 64 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/src/agents/codex.rs b/src/agents/codex.rs index 0714434..b713bd6 100644 --- a/src/agents/codex.rs +++ b/src/agents/codex.rs @@ -526,6 +526,38 @@ command = "custom" ); } + #[test] + fn test_codex_mcp_generator_merges_nested_overlay_tables() { + let temp_dir = TempDir::new().unwrap(); + let generator = CodexGenerator::new(); + + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + create_file( + temp_dir.path(), + "ai-rules/codex-config.toml", + r#"[mcp_servers."test-server".env] +API_KEY = "override" +NODE_ENV = "test" +"#, + ); + + let mcp_gen = generator.mcp_generator().unwrap(); + let files = mcp_gen.generate_mcp(temp_dir.path()); + let content = files + .get(&temp_dir.path().join(".codex/config.toml")) + .unwrap(); + let parsed: Value = content.parse().unwrap(); + let test_server = &parsed["mcp_servers"]["test-server"]; + + assert_eq!(test_server["command"].as_str(), Some("npx")); + assert_eq!( + test_server["args"].as_array().unwrap()[0].as_str(), + Some("-y") + ); + assert_eq!(test_server["env"]["API_KEY"].as_str(), Some("override")); + assert_eq!(test_server["env"]["NODE_ENV"].as_str(), Some("test")); + } + #[test] fn test_codex_mcp_generator_overlay_only() { let temp_dir = TempDir::new().unwrap(); @@ -555,6 +587,20 @@ command = "custom" ); } + #[test] + fn test_generate_codex_config_invalid_overlay_errors() { + let temp_dir = TempDir::new().unwrap(); + create_file(temp_dir.path(), "ai-rules/codex-config.toml", "model =\n"); + + let result = generate_codex_config(temp_dir.path()); + + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Invalid TOML in overlay file")); + } + #[test] fn test_codex_mcp_generator_clean_removes_config() { let temp_dir = TempDir::new().unwrap(); diff --git a/src/commands/status.rs b/src/commands/status.rs index ddbd2e3..7a7f14d 100644 --- a/src/commands/status.rs +++ b/src/commands/status.rs @@ -666,6 +666,70 @@ Test rule content"#; assert!(status.agent_statuses["claude"]); } + #[test] + fn test_status_with_codex_overlay_tracks_overlay_changes() { + let temp_dir = TempDir::new().unwrap(); + + create_file(temp_dir.path(), "ai-rules/test.md", TEST_RULE_CONTENT); + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + create_file( + temp_dir.path(), + "ai-rules/codex-config.toml", + r#"model = "gpt-5.2" + +[mcp_servers."test-server"] +command = "custom" +"#, + ); + + let generate_result = crate::commands::generate::run_generate( + temp_dir.path(), + crate::cli::ResolvedGenerateArgs { + agents: Some(vec!["codex".to_string()]), + command_agents: None, + gitignore: false, + nested_depth: NESTED_DEPTH, + }, + ); + assert!(generate_result.is_ok()); + + let result = check_project_status( + temp_dir.path(), + ResolvedStatusArgs { + agents: Some(vec!["codex".to_string()]), + command_agents: None, + nested_depth: NESTED_DEPTH, + }, + ) + .unwrap(); + assert!(result.has_ai_rules); + assert!(!result.body_files_out_of_sync); + assert!(result.agent_statuses["codex"]); + + create_file( + temp_dir.path(), + "ai-rules/codex-config.toml", + r#"model = "gpt-5.4" + +[mcp_servers."test-server"] +command = "custom" +"#, + ); + + let result = check_project_status( + temp_dir.path(), + ResolvedStatusArgs { + agents: Some(vec!["codex".to_string()]), + command_agents: None, + nested_depth: NESTED_DEPTH, + }, + ) + .unwrap(); + assert!(result.has_ai_rules); + assert!(!result.body_files_out_of_sync); + assert!(!result.agent_statuses["codex"]); + } + const TEST_COMMAND_CONTENT: &str = r#"--- description: Test command --- From 86f3be8a7556c3095f134acf99321d5b0b474bff Mon Sep 17 00:00:00 2001 From: Rob MacEachern Date: Thu, 7 May 2026 15:16:06 -0500 Subject: [PATCH 4/5] Propagate MCP generation errors --- src/agents/codex.rs | 20 +++++++------- src/agents/firebender.rs | 10 +++---- src/agents/gemini.rs | 25 ++++++++--------- src/agents/mcp_generator.rs | 12 ++++----- src/agents/roo.rs | 2 +- src/commands/generate.rs | 54 ++++++++++++++++++++++++++++--------- 6 files changed, 75 insertions(+), 48 deletions(-) diff --git a/src/agents/codex.rs b/src/agents/codex.rs index b713bd6..a24a887 100644 --- a/src/agents/codex.rs +++ b/src/agents/codex.rs @@ -115,14 +115,14 @@ impl AgentRuleGenerator for CodexGenerator { struct CodexMcpGenerator; impl McpGeneratorTrait for CodexMcpGenerator { - fn generate_mcp(&self, current_dir: &Path) -> HashMap { + fn generate_mcp(&self, current_dir: &Path) -> Result> { let mut files = HashMap::new(); - if let Ok(Some(config)) = generate_codex_config(current_dir) { + if let Some(config) = generate_codex_config(current_dir)? { files.insert(current_dir.join(CODEX_CONFIG_TOML), config); } - files + Ok(files) } fn clean_mcp(&self, current_dir: &Path) -> Result<()> { @@ -439,7 +439,7 @@ mod tests { create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); let mcp_gen = generator.mcp_generator().unwrap(); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); let expected_path = temp_dir.path().join(".codex/config.toml"); let content = files.get(&expected_path).unwrap(); @@ -466,7 +466,7 @@ mod tests { create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_HTTP_MCP_CONFIG); let mcp_gen = generator.mcp_generator().unwrap(); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); let expected_path = temp_dir.path().join(".codex/config.toml"); let content = files.get(&expected_path).unwrap(); @@ -502,7 +502,7 @@ command = "custom" ); let mcp_gen = generator.mcp_generator().unwrap(); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); let content = files .get(&temp_dir.path().join(".codex/config.toml")) .unwrap(); @@ -542,7 +542,7 @@ NODE_ENV = "test" ); let mcp_gen = generator.mcp_generator().unwrap(); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); let content = files .get(&temp_dir.path().join(".codex/config.toml")) .unwrap(); @@ -574,7 +574,7 @@ command = "custom" ); let mcp_gen = generator.mcp_generator().unwrap(); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); let content = files .get(&temp_dir.path().join(".codex/config.toml")) .unwrap(); @@ -625,7 +625,7 @@ command = "custom" let mcp_gen = generator.mcp_generator().unwrap(); create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); let content = files .get(&temp_dir.path().join(".codex/config.toml")) .unwrap(); @@ -680,7 +680,7 @@ command = "custom" "#, ); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); let content = files .get(&temp_dir.path().join(".codex/config.toml")) .unwrap(); diff --git a/src/agents/firebender.rs b/src/agents/firebender.rs index 2be3fb5..e9a02fa 100644 --- a/src/agents/firebender.rs +++ b/src/agents/firebender.rs @@ -108,14 +108,14 @@ impl AgentRuleGenerator for FirebenderGenerator { } impl McpGeneratorTrait for FirebenderConfigGenerator { - fn generate_mcp(&self, current_dir: &Path) -> HashMap { + fn generate_mcp(&self, current_dir: &Path) -> Result> { let mut files = HashMap::new(); - if let Ok(Some(config)) = generate_firebender_config(current_dir) { + if let Some(config) = generate_firebender_config(current_dir)? { files.insert(current_dir.join(FIREBENDER_JSON), config); } - files + Ok(files) } fn clean_mcp(&self, current_dir: &Path) -> Result<()> { @@ -382,7 +382,7 @@ mod tests { create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); assert_eq!(files.len(), 1); let expected_path = temp_dir.path().join(FIREBENDER_JSON); @@ -395,7 +395,7 @@ mod tests { let generator = FirebenderGenerator; let mcp_gen = generator.mcp_generator().unwrap(); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); assert!(files.is_empty()); } diff --git a/src/agents/gemini.rs b/src/agents/gemini.rs index 1103b1f..1ae205c 100644 --- a/src/agents/gemini.rs +++ b/src/agents/gemini.rs @@ -98,15 +98,15 @@ impl AgentRuleGenerator for GeminiGenerator { struct GeminiMcpGenerator; impl McpGeneratorTrait for GeminiMcpGenerator { - fn generate_mcp(&self, current_dir: &Path) -> HashMap { + fn generate_mcp(&self, current_dir: &Path) -> Result> { let mut files = HashMap::new(); // 1. Read source MCP config (ai-rules/mcp.json) - let source_mcp_content = match read_mcp_config(current_dir) { - Ok(Some(c)) => c, - _ => return files, // No source config, nothing to generate + let source_mcp_content = match read_mcp_config(current_dir)? { + Some(c) => c, + None => return Ok(files), // No source config, nothing to generate }; - let source_json: Value = serde_json::from_str(&source_mcp_content).unwrap_or(json!({})); + let source_json: Value = serde_json::from_str(&source_mcp_content)?; let mut source_servers = source_json.get("mcpServers").unwrap_or(&json!({})).clone(); // Apply Gemini-specific transformations @@ -148,11 +148,10 @@ impl McpGeneratorTrait for GeminiMcpGenerator { // 4. Format and return // Use pretty print - if let Ok(content) = serde_json::to_string_pretty(&target_json) { - files.insert(target_path, content); - } + let content = serde_json::to_string_pretty(&target_json)?; + files.insert(target_path, content); - files + Ok(files) } fn clean_mcp(&self, current_dir: &Path) -> Result<()> { @@ -419,7 +418,7 @@ mod tests { create_file(temp_dir.path(), ".gemini/settings.json", existing_target); // Generate MCP - let files = generator.generate_mcp(temp_dir.path()); + let files = generator.generate_mcp(temp_dir.path()).unwrap(); // Verify the result - get the first (and only) file assert_eq!(files.len(), 1); @@ -454,10 +453,8 @@ mod tests { // Create invalid JSON source create_file(temp_dir.path(), "ai-rules/mcp.json", "{ invalid json }"); - // Should return empty map - read_mcp_config returns Err for invalid JSON, - // which is caught by the wildcard pattern and returns empty files - let files = generator.generate_mcp(temp_dir.path()); - assert!(files.is_empty()); + let result = generator.generate_mcp(temp_dir.path()); + assert!(result.is_err()); } #[test] diff --git a/src/agents/mcp_generator.rs b/src/agents/mcp_generator.rs index 5c31006..ff76e94 100644 --- a/src/agents/mcp_generator.rs +++ b/src/agents/mcp_generator.rs @@ -5,7 +5,7 @@ use std::fs; use std::path::{Path, PathBuf}; pub trait McpGeneratorTrait { - fn generate_mcp(&self, current_dir: &Path) -> HashMap; + fn generate_mcp(&self, current_dir: &Path) -> Result>; fn clean_mcp(&self, current_dir: &Path) -> Result<()>; @@ -33,14 +33,14 @@ impl ExternalMcpGenerator { } impl McpGeneratorTrait for ExternalMcpGenerator { - fn generate_mcp(&self, current_dir: &Path) -> HashMap { + fn generate_mcp(&self, current_dir: &Path) -> Result> { let mut files = HashMap::new(); - if let Ok(Some(mcp_content)) = read_mcp_config(current_dir) { + if let Some(mcp_content) = read_mcp_config(current_dir)? { files.insert(current_dir.join(&self.output_path), mcp_content); } - files + Ok(files) } fn clean_mcp(&self, current_dir: &Path) -> Result<()> { @@ -99,7 +99,7 @@ mod tests { create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); - let files = generator.generate_mcp(temp_dir.path()); + let files = generator.generate_mcp(temp_dir.path()).unwrap(); assert_eq!(files.len(), 1); let expected_path = temp_dir.path().join(".mcp.json"); @@ -111,7 +111,7 @@ mod tests { let temp_dir = TempDir::new().unwrap(); let generator = ExternalMcpGenerator::new(PathBuf::from(".mcp.json")); - let files = generator.generate_mcp(temp_dir.path()); + let files = generator.generate_mcp(temp_dir.path()).unwrap(); assert_eq!(files.len(), 0); } diff --git a/src/agents/roo.rs b/src/agents/roo.rs index 8144471..d380a3d 100644 --- a/src/agents/roo.rs +++ b/src/agents/roo.rs @@ -120,7 +120,7 @@ mod tests { create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); let mcp_gen = generator.mcp_generator().unwrap(); - let files = mcp_gen.generate_mcp(temp_dir.path()); + let files = mcp_gen.generate_mcp(temp_dir.path()).unwrap(); assert_eq!(files.len(), 1); let expected_path = temp_dir.path().join(".roo/mcp.json"); diff --git a/src/commands/generate.rs b/src/commands/generate.rs index dd846f9..1c8fb46 100644 --- a/src/commands/generate.rs +++ b/src/commands/generate.rs @@ -57,6 +57,19 @@ fn generate_files( registry: &AgentToolRegistry, result: &mut GenerationResult, ) -> Result<()> { + let mut mcp_files_to_write: HashMap = HashMap::new(); + for agent in agents { + if let Some(tool) = registry.get_tool(agent) { + if let Some(mcp_gen) = tool.mcp_generator() { + let mcp_files = mcp_gen.generate_mcp(current_dir)?; + for path in mcp_files.keys() { + result.add_file(agent, path.clone()); + } + mcp_files_to_write.extend(mcp_files); + } + } + } + operations::clean_generated_files(current_dir, agents, registry)?; if detect_symlink_mode(current_dir) { @@ -100,18 +113,6 @@ fn generate_files( } } - let mut mcp_files_to_write: HashMap = HashMap::new(); - for agent in agents { - if let Some(tool) = registry.get_tool(agent) { - if let Some(mcp_gen) = tool.mcp_generator() { - let mcp_files = mcp_gen.generate_mcp(current_dir); - for path in mcp_files.keys() { - result.add_file(agent, path.clone()); - } - mcp_files_to_write.extend(mcp_files); - } - } - } write_directory_files(&mcp_files_to_write)?; // Generate command symlinks - use command_agents instead of agents @@ -689,6 +690,35 @@ New body content"#; assert_eq!(roo_mcp_content.trim(), TEST_MCP_CONFIG.trim()); } + #[test] + fn test_run_generate_invalid_codex_overlay_preserves_existing_config() { + let temp_dir = TempDir::new().unwrap(); + + create_file(temp_dir.path(), "ai-rules/test.md", TEST_RULE_CONTENT); + create_file(temp_dir.path(), "ai-rules/mcp.json", TEST_MCP_CONFIG); + create_file(temp_dir.path(), "ai-rules/codex-config.toml", "model =\n"); + create_file( + temp_dir.path(), + ".codex/config.toml", + "model = \"existing\"\n", + ); + + let args = ResolvedGenerateArgs { + agents: Some(vec!["codex".to_string()]), + command_agents: None, + gitignore: false, + nested_depth: 0, + }; + let result = run_generate(temp_dir.path(), args); + + assert!(result.is_err()); + assert_file_content( + temp_dir.path(), + ".codex/config.toml", + "model = \"existing\"\n", + ); + } + #[test] fn test_run_generate_without_mcp_source() { let temp_dir = TempDir::new().unwrap(); From 80c01284ca26d9020cf67acf1131e38d8ea07e39 Mon Sep 17 00:00:00 2001 From: Rob MacEachern Date: Thu, 7 May 2026 15:16:51 -0500 Subject: [PATCH 5/5] Remove redundant Codex MCP docs note --- docs/mcp.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index ed66ef8..47d9d3d 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -40,5 +40,3 @@ See the [Supported Agents](agents.md) table for which agents support MCP and the | Firebender | `firebender.json` (generated when MCP and/or overlay config exists) | | Gemini | Embedded in `.gemini/settings.json` | | Roo | `.roo/mcp.json` | - -Codex uses TOML instead of JSON. `ai-rules generate` converts the shared `ai-rules/mcp.json` source into project-scoped `[mcp_servers.]` tables in `.codex/config.toml`. If `ai-rules/codex-config.toml` exists, it is merged over the generated config so projects can add or override Codex settings.