From 99337571b0012606858a54dfe3a760d5c42007b6 Mon Sep 17 00:00:00 2001 From: Gabriel Peal Date: Tue, 7 Oct 2025 21:15:33 -0700 Subject: [PATCH] Works --- codex-rs/cli/src/mcp_cmd.rs | 58 +++++++++++++++++---- codex-rs/cli/tests/mcp_add_remove.rs | 4 ++ codex-rs/cli/tests/mcp_list.rs | 7 ++- codex-rs/core/src/config.rs | 43 +++++++++++++++ codex-rs/core/src/config_types.rs | 29 +++++++++++ codex-rs/core/src/mcp_connection_manager.rs | 4 ++ codex-rs/core/tests/suite/rmcp_client.rs | 3 ++ codex-rs/tui/src/history_cell.rs | 10 +++- docs/config.md | 11 ++-- 9 files changed, 155 insertions(+), 14 deletions(-) diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 0cf3c0e228..964b69480c 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -234,6 +234,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re let new_entry = McpServerConfig { transport, + enabled: true, startup_timeout_sec: None, tool_timeout_sec: None, }; @@ -365,6 +366,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> serde_json::json!({ "name": name, + "enabled": cfg.enabled, "transport": transport, "startup_timeout_sec": cfg .startup_timeout_sec @@ -385,8 +387,8 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> return Ok(()); } - let mut stdio_rows: Vec<[String; 4]> = Vec::new(); - let mut http_rows: Vec<[String; 3]> = Vec::new(); + let mut stdio_rows: Vec<[String; 5]> = Vec::new(); + let mut http_rows: Vec<[String; 4]> = Vec::new(); for (name, cfg) in entries { match &cfg.transport { @@ -409,23 +411,46 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> .join(", ") } }; - stdio_rows.push([name.clone(), command.clone(), args_display, env_display]); + let status = if cfg.enabled { + "enabled".to_string() + } else { + "disabled".to_string() + }; + stdio_rows.push([ + name.clone(), + command.clone(), + args_display, + env_display, + status, + ]); } McpServerTransportConfig::StreamableHttp { url, bearer_token_env_var, } => { + let status = if cfg.enabled { + "enabled".to_string() + } else { + "disabled".to_string() + }; http_rows.push([ name.clone(), url.clone(), bearer_token_env_var.clone().unwrap_or("-".to_string()), + status, ]); } } } if !stdio_rows.is_empty() { - let mut widths = ["Name".len(), "Command".len(), "Args".len(), "Env".len()]; + let mut widths = [ + "Name".len(), + "Command".len(), + "Args".len(), + "Env".len(), + "Status".len(), + ]; for row in &stdio_rows { for (i, cell) in row.iter().enumerate() { widths[i] = widths[i].max(cell.len()); @@ -433,28 +458,32 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> } println!( - "{: } if !http_rows.is_empty() { - let mut widths = ["Name".len(), "Url".len(), "Bearer Token Env Var".len()]; + let mut widths = [ + "Name".len(), + "Url".len(), + "Bearer Token Env Var".len(), + "Status".len(), + ]; for row in &http_rows { for (i, cell) in row.iter().enumerate() { widths[i] = widths[i].max(cell.len()); @@ -472,24 +506,28 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> } println!( - "{: Re }; let output = serde_json::to_string_pretty(&serde_json::json!({ "name": get_args.name, + "enabled": server.enabled, "transport": transport, "startup_timeout_sec": server .startup_timeout_sec @@ -539,6 +578,7 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re } println!("{}", get_args.name); + println!(" enabled: {}", server.enabled); match &server.transport { McpServerTransportConfig::Stdio { command, args, env } => { println!(" transport: stdio"); diff --git a/codex-rs/cli/tests/mcp_add_remove.rs b/codex-rs/cli/tests/mcp_add_remove.rs index 9ccb9f73d0..705509abf5 100644 --- a/codex-rs/cli/tests/mcp_add_remove.rs +++ b/codex-rs/cli/tests/mcp_add_remove.rs @@ -35,6 +35,7 @@ async fn add_and_remove_server_updates_global_config() -> Result<()> { } other => panic!("unexpected transport: {other:?}"), } + assert!(docs.enabled); let mut remove_cmd = codex_command(codex_home.path())?; remove_cmd @@ -90,6 +91,7 @@ async fn add_with_env_preserves_key_order_and_values() -> Result<()> { assert_eq!(env.len(), 2); assert_eq!(env.get("FOO"), Some(&"bar".to_string())); assert_eq!(env.get("ALPHA"), Some(&"beta".to_string())); + assert!(envy.enabled); Ok(()) } @@ -116,6 +118,7 @@ async fn add_streamable_http_without_manual_token() -> Result<()> { } other => panic!("unexpected transport: {other:?}"), } + assert!(github.enabled); assert!(!codex_home.path().join(".credentials.json").exists()); assert!(!codex_home.path().join(".env").exists()); @@ -153,6 +156,7 @@ async fn add_streamable_http_with_custom_env_var() -> Result<()> { } other => panic!("unexpected transport: {other:?}"), } + assert!(issues.enabled); Ok(()) } diff --git a/codex-rs/cli/tests/mcp_list.rs b/codex-rs/cli/tests/mcp_list.rs index 6c83de19fa..0a80dd0cbf 100644 --- a/codex-rs/cli/tests/mcp_list.rs +++ b/codex-rs/cli/tests/mcp_list.rs @@ -1,6 +1,7 @@ use std::path::Path; use anyhow::Result; +use predicates::prelude::PredicateBooleanExt; use predicates::str::contains; use pretty_assertions::assert_eq; use serde_json::Value as JsonValue; @@ -53,6 +54,8 @@ fn list_and_get_render_expected_output() -> Result<()> { assert!(stdout.contains("docs")); assert!(stdout.contains("docs-server")); assert!(stdout.contains("TOKEN=secret")); + assert!(stdout.contains("Status")); + assert!(stdout.contains("enabled")); let mut list_json_cmd = codex_command(codex_home.path())?; let json_output = list_json_cmd.args(["mcp", "list", "--json"]).output()?; @@ -64,6 +67,7 @@ fn list_and_get_render_expected_output() -> Result<()> { json!([ { "name": "docs", + "enabled": true, "transport": { "type": "stdio", "command": "docs-server", @@ -91,6 +95,7 @@ fn list_and_get_render_expected_output() -> Result<()> { assert!(stdout.contains("command: docs-server")); assert!(stdout.contains("args: --port 4000")); assert!(stdout.contains("env: TOKEN=secret")); + assert!(stdout.contains("enabled: true")); assert!(stdout.contains("remove: codex mcp remove docs")); let mut get_json_cmd = codex_command(codex_home.path())?; @@ -98,7 +103,7 @@ fn list_and_get_render_expected_output() -> Result<()> { .args(["mcp", "get", "docs", "--json"]) .assert() .success() - .stdout(contains("\"name\": \"docs\"")); + .stdout(contains("\"name\": \"docs\"").and(contains("\"enabled\": true"))); Ok(()) } diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index d0437163fc..c715651851 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -401,6 +401,10 @@ pub fn write_global_mcp_servers( } } + if !config.enabled { + entry["enabled"] = toml_edit::value(false); + } + if let Some(timeout) = config.startup_timeout_sec { entry["startup_timeout_sec"] = toml_edit::value(timeout.as_secs_f64()); } @@ -1515,6 +1519,7 @@ exclude_slash_tmp = true args: vec!["hello".to_string()], env: None, }, + enabled: true, startup_timeout_sec: Some(Duration::from_secs(3)), tool_timeout_sec: Some(Duration::from_secs(5)), }, @@ -1535,6 +1540,7 @@ exclude_slash_tmp = true } assert_eq!(docs.startup_timeout_sec, Some(Duration::from_secs(3))); assert_eq!(docs.tool_timeout_sec, Some(Duration::from_secs(5))); + assert!(docs.enabled); let empty = BTreeMap::new(); write_global_mcp_servers(codex_home.path(), &empty)?; @@ -1639,6 +1645,7 @@ bearer_token = "secret" ("ALPHA_VAR".to_string(), "1".to_string()), ])), }, + enabled: true, startup_timeout_sec: None, tool_timeout_sec: None, }, @@ -1689,6 +1696,7 @@ ZIG_VAR = "3" url: "https://example.com/mcp".to_string(), bearer_token_env_var: Some("MCP_TOKEN".to_string()), }, + enabled: true, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, }, @@ -1728,6 +1736,7 @@ startup_timeout_sec = 2.0 url: "https://example.com/mcp".to_string(), bearer_token_env_var: None, }, + enabled: true, startup_timeout_sec: None, tool_timeout_sec: None, }, @@ -1758,6 +1767,40 @@ url = "https://example.com/mcp" Ok(()) } + #[tokio::test] + async fn write_global_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { + let codex_home = TempDir::new()?; + + let servers = BTreeMap::from([( + "docs".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: "docs-server".to_string(), + args: Vec::new(), + env: None, + }, + enabled: false, + startup_timeout_sec: None, + tool_timeout_sec: None, + }, + )]); + + write_global_mcp_servers(codex_home.path(), &servers)?; + + let config_path = codex_home.path().join(CONFIG_TOML_FILE); + let serialized = std::fs::read_to_string(&config_path)?; + assert!( + serialized.contains("enabled = false"), + "serialized config missing disabled flag:\n{serialized}" + ); + + let loaded = load_global_mcp_servers(codex_home.path()).await?; + let docs = loaded.get("docs").expect("docs entry"); + assert!(!docs.enabled); + + Ok(()) + } + #[tokio::test] async fn persist_model_selection_updates_defaults() -> anyhow::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config_types.rs b/codex-rs/core/src/config_types.rs index 8940ac5c08..0ef9248511 100644 --- a/codex-rs/core/src/config_types.rs +++ b/codex-rs/core/src/config_types.rs @@ -20,6 +20,10 @@ pub struct McpServerConfig { #[serde(flatten)] pub transport: McpServerTransportConfig, + /// When `false`, Codex skips initializing this MCP server. + #[serde(default = "default_enabled")] + pub enabled: bool, + /// Startup timeout in seconds for initializing MCP server & initially listing tools. #[serde( default, @@ -56,6 +60,8 @@ impl<'de> Deserialize<'de> for McpServerConfig { startup_timeout_ms: Option, #[serde(default, with = "option_duration_secs")] tool_timeout_sec: Option, + #[serde(default)] + enabled: Option, } let raw = RawMcpServerConfig::deserialize(deserializer)?; @@ -127,10 +133,15 @@ impl<'de> Deserialize<'de> for McpServerConfig { transport, startup_timeout_sec, tool_timeout_sec: raw.tool_timeout_sec, + enabled: raw.enabled.unwrap_or_else(default_enabled), }) } } +const fn default_enabled() -> bool { + true +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] #[serde(untagged, deny_unknown_fields, rename_all = "snake_case")] pub enum McpServerTransportConfig { @@ -460,6 +471,7 @@ mod tests { env: None } ); + assert!(cfg.enabled); } #[test] @@ -480,6 +492,7 @@ mod tests { env: None } ); + assert!(cfg.enabled); } #[test] @@ -501,6 +514,20 @@ mod tests { env: Some(HashMap::from([("FOO".to_string(), "BAR".to_string())])) } ); + assert!(cfg.enabled); + } + + #[test] + fn deserialize_disabled_server_config() { + let cfg: McpServerConfig = toml::from_str( + r#" + command = "echo" + enabled = false + "#, + ) + .expect("should deserialize disabled server config"); + + assert!(!cfg.enabled); } #[test] @@ -519,6 +546,7 @@ mod tests { bearer_token_env_var: None } ); + assert!(cfg.enabled); } #[test] @@ -538,6 +566,7 @@ mod tests { bearer_token_env_var: Some("GITHUB_TOKEN".to_string()) } ); + assert!(cfg.enabled); } #[test] diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 0c2fdc6301..768c6b01a6 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -207,6 +207,10 @@ impl McpConnectionManager { continue; } + if !cfg.enabled { + continue; + } + let startup_timeout = cfg.startup_timeout_sec.unwrap_or(DEFAULT_STARTUP_TIMEOUT); let tool_timeout = cfg.tool_timeout_sec.unwrap_or(DEFAULT_TOOL_TIMEOUT); diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index a8a5d3afbf..e111cebcb7 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -86,6 +86,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { expected_env_value.to_string(), )])), }, + enabled: true, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, }, @@ -234,6 +235,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { url: server_url, bearer_token_env_var: None, }, + enabled: true, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, }, @@ -414,6 +416,7 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { url: server_url, bearer_token_env_var: None, }, + enabled: true, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, }, diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 7c4d18e1a3..dbd4a72846 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -874,6 +874,12 @@ pub(crate) fn new_mcp_tools_output( names.sort(); lines.push(vec![" • Server: ".into(), server.clone().into()].into()); + let status_line = if cfg.enabled { + vec![" • Status: ".into(), "enabled".green()].into() + } else { + vec![" • Status: ".into(), "disabled".red()].into() + }; + lines.push(status_line); match &cfg.transport { McpServerTransportConfig::Stdio { command, args, env } => { @@ -899,7 +905,9 @@ pub(crate) fn new_mcp_tools_output( } } - if names.is_empty() { + if !cfg.enabled { + lines.push(vec![" • Tools: ".into(), "(disabled)".red()].into()); + } else if names.is_empty() { lines.push(" • Tools: (none)".into()); } else { lines.push(vec![" • Tools: ".into(), names.join(", ").into()].into()); diff --git a/docs/config.md b/docs/config.md index aa45acc8dd..fba8a3a21d 100644 --- a/docs/config.md +++ b/docs/config.md @@ -384,6 +384,8 @@ For oauth login, you must enable `experimental_use_rmcp_client = true` and then startup_timeout_sec = 20 # Optional: override the default 60s per-tool timeout tool_timeout_sec = 30 +# Optional: disable a server without removing it +enabled = false ``` ### Experimental RMCP client @@ -787,9 +789,12 @@ notifications = [ "agent-turn-complete", "approval-requested" ] | `disable_response_storage` | boolean | Required for ZDR orgs. | | `notify` | array | External program for notifications. | | `instructions` | string | Currently ignored; use `experimental_instructions_file` or `AGENTS.md`. | -| `mcp_servers..command` | string | MCP server launcher command. | -| `mcp_servers..args` | array | MCP server args. | -| `mcp_servers..env` | map | MCP server env vars. | +| `mcp_servers..command` | string | MCP server launcher command (stdio servers only). | +| `mcp_servers..args` | array | MCP server args (stdio servers only). | +| `mcp_servers..env` | map | MCP server env vars (stdio servers only). | +| `mcp_servers..url` | string | MCP server url (streamable http servers only). | +| `mcp_servers..bearer_token_env_var` | string | environment variable containing a bearer token to use for auth (streamable http servers only). | +| `mcp_servers..enabled` | boolean | When false, Codex skips starting the server (default: true). | | `mcp_servers..startup_timeout_sec` | number | Startup timeout in seconds (default: 10). Timeout is applied both for initializing MCP server and initially listing tools. | | `mcp_servers..tool_timeout_sec` | number | Per-tool timeout in seconds (default: 60). Accepts fractional values; omit to use the default. | | `model_providers..name` | string | Display name. |