[MCP] Add an enabled config field (#4917)

This lets users more easily toggle MCP servers.
This commit is contained in:
Gabriel Peal
2025-10-08 13:24:51 -07:00
committed by GitHub
parent e896db1180
commit d3820f4782
9 changed files with 155 additions and 14 deletions

View File

@@ -234,6 +234,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
let new_entry = McpServerConfig { let new_entry = McpServerConfig {
transport, transport,
enabled: true,
startup_timeout_sec: None, startup_timeout_sec: None,
tool_timeout_sec: None, tool_timeout_sec: None,
}; };
@@ -365,6 +366,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
serde_json::json!({ serde_json::json!({
"name": name, "name": name,
"enabled": cfg.enabled,
"transport": transport, "transport": transport,
"startup_timeout_sec": cfg "startup_timeout_sec": cfg
.startup_timeout_sec .startup_timeout_sec
@@ -385,8 +387,8 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
return Ok(()); return Ok(());
} }
let mut stdio_rows: Vec<[String; 4]> = Vec::new(); let mut stdio_rows: Vec<[String; 5]> = Vec::new();
let mut http_rows: Vec<[String; 3]> = Vec::new(); let mut http_rows: Vec<[String; 4]> = Vec::new();
for (name, cfg) in entries { for (name, cfg) in entries {
match &cfg.transport { match &cfg.transport {
@@ -409,23 +411,46 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
.join(", ") .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 { McpServerTransportConfig::StreamableHttp {
url, url,
bearer_token_env_var, bearer_token_env_var,
} => { } => {
let status = if cfg.enabled {
"enabled".to_string()
} else {
"disabled".to_string()
};
http_rows.push([ http_rows.push([
name.clone(), name.clone(),
url.clone(), url.clone(),
bearer_token_env_var.clone().unwrap_or("-".to_string()), bearer_token_env_var.clone().unwrap_or("-".to_string()),
status,
]); ]);
} }
} }
} }
if !stdio_rows.is_empty() { 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 row in &stdio_rows {
for (i, cell) in row.iter().enumerate() { for (i, cell) in row.iter().enumerate() {
widths[i] = widths[i].max(cell.len()); widths[i] = widths[i].max(cell.len());
@@ -433,28 +458,32 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
} }
println!( println!(
"{:<name_w$} {:<cmd_w$} {:<args_w$} {:<env_w$}", "{:<name_w$} {:<cmd_w$} {:<args_w$} {:<env_w$} {:<status_w$}",
"Name", "Name",
"Command", "Command",
"Args", "Args",
"Env", "Env",
"Status",
name_w = widths[0], name_w = widths[0],
cmd_w = widths[1], cmd_w = widths[1],
args_w = widths[2], args_w = widths[2],
env_w = widths[3], env_w = widths[3],
status_w = widths[4],
); );
for row in &stdio_rows { for row in &stdio_rows {
println!( println!(
"{:<name_w$} {:<cmd_w$} {:<args_w$} {:<env_w$}", "{:<name_w$} {:<cmd_w$} {:<args_w$} {:<env_w$} {:<status_w$}",
row[0], row[0],
row[1], row[1],
row[2], row[2],
row[3], row[3],
row[4],
name_w = widths[0], name_w = widths[0],
cmd_w = widths[1], cmd_w = widths[1],
args_w = widths[2], args_w = widths[2],
env_w = widths[3], env_w = widths[3],
status_w = widths[4],
); );
} }
} }
@@ -464,7 +493,12 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
} }
if !http_rows.is_empty() { 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 row in &http_rows {
for (i, cell) in row.iter().enumerate() { for (i, cell) in row.iter().enumerate() {
widths[i] = widths[i].max(cell.len()); widths[i] = widths[i].max(cell.len());
@@ -472,24 +506,28 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
} }
println!( println!(
"{:<name_w$} {:<url_w$} {:<token_w$}", "{:<name_w$} {:<url_w$} {:<token_w$} {:<status_w$}",
"Name", "Name",
"Url", "Url",
"Bearer Token Env Var", "Bearer Token Env Var",
"Status",
name_w = widths[0], name_w = widths[0],
url_w = widths[1], url_w = widths[1],
token_w = widths[2], token_w = widths[2],
status_w = widths[3],
); );
for row in &http_rows { for row in &http_rows {
println!( println!(
"{:<name_w$} {:<url_w$} {:<token_w$}", "{:<name_w$} {:<url_w$} {:<token_w$} {:<status_w$}",
row[0], row[0],
row[1], row[1],
row[2], row[2],
row[3],
name_w = widths[0], name_w = widths[0],
url_w = widths[1], url_w = widths[1],
token_w = widths[2], token_w = widths[2],
status_w = widths[3],
); );
} }
} }
@@ -526,6 +564,7 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
}; };
let output = serde_json::to_string_pretty(&serde_json::json!({ let output = serde_json::to_string_pretty(&serde_json::json!({
"name": get_args.name, "name": get_args.name,
"enabled": server.enabled,
"transport": transport, "transport": transport,
"startup_timeout_sec": server "startup_timeout_sec": server
.startup_timeout_sec .startup_timeout_sec
@@ -539,6 +578,7 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
} }
println!("{}", get_args.name); println!("{}", get_args.name);
println!(" enabled: {}", server.enabled);
match &server.transport { match &server.transport {
McpServerTransportConfig::Stdio { command, args, env } => { McpServerTransportConfig::Stdio { command, args, env } => {
println!(" transport: stdio"); println!(" transport: stdio");

View File

@@ -35,6 +35,7 @@ async fn add_and_remove_server_updates_global_config() -> Result<()> {
} }
other => panic!("unexpected transport: {other:?}"), other => panic!("unexpected transport: {other:?}"),
} }
assert!(docs.enabled);
let mut remove_cmd = codex_command(codex_home.path())?; let mut remove_cmd = codex_command(codex_home.path())?;
remove_cmd 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.len(), 2);
assert_eq!(env.get("FOO"), Some(&"bar".to_string())); assert_eq!(env.get("FOO"), Some(&"bar".to_string()));
assert_eq!(env.get("ALPHA"), Some(&"beta".to_string())); assert_eq!(env.get("ALPHA"), Some(&"beta".to_string()));
assert!(envy.enabled);
Ok(()) Ok(())
} }
@@ -116,6 +118,7 @@ async fn add_streamable_http_without_manual_token() -> Result<()> {
} }
other => panic!("unexpected transport: {other:?}"), other => panic!("unexpected transport: {other:?}"),
} }
assert!(github.enabled);
assert!(!codex_home.path().join(".credentials.json").exists()); assert!(!codex_home.path().join(".credentials.json").exists());
assert!(!codex_home.path().join(".env").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:?}"), other => panic!("unexpected transport: {other:?}"),
} }
assert!(issues.enabled);
Ok(()) Ok(())
} }

View File

@@ -1,6 +1,7 @@
use std::path::Path; use std::path::Path;
use anyhow::Result; use anyhow::Result;
use predicates::prelude::PredicateBooleanExt;
use predicates::str::contains; use predicates::str::contains;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use serde_json::Value as JsonValue; 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"));
assert!(stdout.contains("docs-server")); assert!(stdout.contains("docs-server"));
assert!(stdout.contains("TOKEN=secret")); assert!(stdout.contains("TOKEN=secret"));
assert!(stdout.contains("Status"));
assert!(stdout.contains("enabled"));
let mut list_json_cmd = codex_command(codex_home.path())?; let mut list_json_cmd = codex_command(codex_home.path())?;
let json_output = list_json_cmd.args(["mcp", "list", "--json"]).output()?; let json_output = list_json_cmd.args(["mcp", "list", "--json"]).output()?;
@@ -64,6 +67,7 @@ fn list_and_get_render_expected_output() -> Result<()> {
json!([ json!([
{ {
"name": "docs", "name": "docs",
"enabled": true,
"transport": { "transport": {
"type": "stdio", "type": "stdio",
"command": "docs-server", "command": "docs-server",
@@ -91,6 +95,7 @@ fn list_and_get_render_expected_output() -> Result<()> {
assert!(stdout.contains("command: docs-server")); assert!(stdout.contains("command: docs-server"));
assert!(stdout.contains("args: --port 4000")); assert!(stdout.contains("args: --port 4000"));
assert!(stdout.contains("env: TOKEN=secret")); assert!(stdout.contains("env: TOKEN=secret"));
assert!(stdout.contains("enabled: true"));
assert!(stdout.contains("remove: codex mcp remove docs")); assert!(stdout.contains("remove: codex mcp remove docs"));
let mut get_json_cmd = codex_command(codex_home.path())?; 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"]) .args(["mcp", "get", "docs", "--json"])
.assert() .assert()
.success() .success()
.stdout(contains("\"name\": \"docs\"")); .stdout(contains("\"name\": \"docs\"").and(contains("\"enabled\": true")));
Ok(()) Ok(())
} }

View File

@@ -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 { if let Some(timeout) = config.startup_timeout_sec {
entry["startup_timeout_sec"] = toml_edit::value(timeout.as_secs_f64()); entry["startup_timeout_sec"] = toml_edit::value(timeout.as_secs_f64());
} }
@@ -1515,6 +1519,7 @@ exclude_slash_tmp = true
args: vec!["hello".to_string()], args: vec!["hello".to_string()],
env: None, env: None,
}, },
enabled: true,
startup_timeout_sec: Some(Duration::from_secs(3)), startup_timeout_sec: Some(Duration::from_secs(3)),
tool_timeout_sec: Some(Duration::from_secs(5)), 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.startup_timeout_sec, Some(Duration::from_secs(3)));
assert_eq!(docs.tool_timeout_sec, Some(Duration::from_secs(5))); assert_eq!(docs.tool_timeout_sec, Some(Duration::from_secs(5)));
assert!(docs.enabled);
let empty = BTreeMap::new(); let empty = BTreeMap::new();
write_global_mcp_servers(codex_home.path(), &empty)?; write_global_mcp_servers(codex_home.path(), &empty)?;
@@ -1639,6 +1645,7 @@ bearer_token = "secret"
("ALPHA_VAR".to_string(), "1".to_string()), ("ALPHA_VAR".to_string(), "1".to_string()),
])), ])),
}, },
enabled: true,
startup_timeout_sec: None, startup_timeout_sec: None,
tool_timeout_sec: None, tool_timeout_sec: None,
}, },
@@ -1689,6 +1696,7 @@ ZIG_VAR = "3"
url: "https://example.com/mcp".to_string(), url: "https://example.com/mcp".to_string(),
bearer_token_env_var: Some("MCP_TOKEN".to_string()), bearer_token_env_var: Some("MCP_TOKEN".to_string()),
}, },
enabled: true,
startup_timeout_sec: Some(Duration::from_secs(2)), startup_timeout_sec: Some(Duration::from_secs(2)),
tool_timeout_sec: None, tool_timeout_sec: None,
}, },
@@ -1728,6 +1736,7 @@ startup_timeout_sec = 2.0
url: "https://example.com/mcp".to_string(), url: "https://example.com/mcp".to_string(),
bearer_token_env_var: None, bearer_token_env_var: None,
}, },
enabled: true,
startup_timeout_sec: None, startup_timeout_sec: None,
tool_timeout_sec: None, tool_timeout_sec: None,
}, },
@@ -1758,6 +1767,40 @@ url = "https://example.com/mcp"
Ok(()) 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] #[tokio::test]
async fn persist_model_selection_updates_defaults() -> anyhow::Result<()> { async fn persist_model_selection_updates_defaults() -> anyhow::Result<()> {
let codex_home = TempDir::new()?; let codex_home = TempDir::new()?;

View File

@@ -20,6 +20,10 @@ pub struct McpServerConfig {
#[serde(flatten)] #[serde(flatten)]
pub transport: McpServerTransportConfig, 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. /// Startup timeout in seconds for initializing MCP server & initially listing tools.
#[serde( #[serde(
default, default,
@@ -56,6 +60,8 @@ impl<'de> Deserialize<'de> for McpServerConfig {
startup_timeout_ms: Option<u64>, startup_timeout_ms: Option<u64>,
#[serde(default, with = "option_duration_secs")] #[serde(default, with = "option_duration_secs")]
tool_timeout_sec: Option<Duration>, tool_timeout_sec: Option<Duration>,
#[serde(default)]
enabled: Option<bool>,
} }
let raw = RawMcpServerConfig::deserialize(deserializer)?; let raw = RawMcpServerConfig::deserialize(deserializer)?;
@@ -127,10 +133,15 @@ impl<'de> Deserialize<'de> for McpServerConfig {
transport, transport,
startup_timeout_sec, startup_timeout_sec,
tool_timeout_sec: raw.tool_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)] #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
#[serde(untagged, deny_unknown_fields, rename_all = "snake_case")] #[serde(untagged, deny_unknown_fields, rename_all = "snake_case")]
pub enum McpServerTransportConfig { pub enum McpServerTransportConfig {
@@ -460,6 +471,7 @@ mod tests {
env: None env: None
} }
); );
assert!(cfg.enabled);
} }
#[test] #[test]
@@ -480,6 +492,7 @@ mod tests {
env: None env: None
} }
); );
assert!(cfg.enabled);
} }
#[test] #[test]
@@ -501,6 +514,20 @@ mod tests {
env: Some(HashMap::from([("FOO".to_string(), "BAR".to_string())])) 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] #[test]
@@ -519,6 +546,7 @@ mod tests {
bearer_token_env_var: None bearer_token_env_var: None
} }
); );
assert!(cfg.enabled);
} }
#[test] #[test]
@@ -538,6 +566,7 @@ mod tests {
bearer_token_env_var: Some("GITHUB_TOKEN".to_string()) bearer_token_env_var: Some("GITHUB_TOKEN".to_string())
} }
); );
assert!(cfg.enabled);
} }
#[test] #[test]

View File

@@ -207,6 +207,10 @@ impl McpConnectionManager {
continue; continue;
} }
if !cfg.enabled {
continue;
}
let startup_timeout = cfg.startup_timeout_sec.unwrap_or(DEFAULT_STARTUP_TIMEOUT); let startup_timeout = cfg.startup_timeout_sec.unwrap_or(DEFAULT_STARTUP_TIMEOUT);
let tool_timeout = cfg.tool_timeout_sec.unwrap_or(DEFAULT_TOOL_TIMEOUT); let tool_timeout = cfg.tool_timeout_sec.unwrap_or(DEFAULT_TOOL_TIMEOUT);

View File

@@ -86,6 +86,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
expected_env_value.to_string(), expected_env_value.to_string(),
)])), )])),
}, },
enabled: true,
startup_timeout_sec: Some(Duration::from_secs(10)), startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None, tool_timeout_sec: None,
}, },
@@ -234,6 +235,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> {
url: server_url, url: server_url,
bearer_token_env_var: None, bearer_token_env_var: None,
}, },
enabled: true,
startup_timeout_sec: Some(Duration::from_secs(10)), startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None, tool_timeout_sec: None,
}, },
@@ -414,6 +416,7 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> {
url: server_url, url: server_url,
bearer_token_env_var: None, bearer_token_env_var: None,
}, },
enabled: true,
startup_timeout_sec: Some(Duration::from_secs(10)), startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None, tool_timeout_sec: None,
}, },

View File

@@ -874,6 +874,12 @@ pub(crate) fn new_mcp_tools_output(
names.sort(); names.sort();
lines.push(vec![" • Server: ".into(), server.clone().into()].into()); 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 { match &cfg.transport {
McpServerTransportConfig::Stdio { command, args, env } => { 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()); lines.push(" • Tools: (none)".into());
} else { } else {
lines.push(vec![" • Tools: ".into(), names.join(", ").into()].into()); lines.push(vec![" • Tools: ".into(), names.join(", ").into()].into());

View File

@@ -384,6 +384,8 @@ For oauth login, you must enable `experimental_use_rmcp_client = true` and then
startup_timeout_sec = 20 startup_timeout_sec = 20
# Optional: override the default 60s per-tool timeout # Optional: override the default 60s per-tool timeout
tool_timeout_sec = 30 tool_timeout_sec = 30
# Optional: disable a server without removing it
enabled = false
``` ```
### Experimental RMCP client ### Experimental RMCP client
@@ -787,9 +789,12 @@ notifications = [ "agent-turn-complete", "approval-requested" ]
| `disable_response_storage` | boolean | Required for ZDR orgs. | | `disable_response_storage` | boolean | Required for ZDR orgs. |
| `notify` | array<string> | External program for notifications. | | `notify` | array<string> | External program for notifications. |
| `instructions` | string | Currently ignored; use `experimental_instructions_file` or `AGENTS.md`. | | `instructions` | string | Currently ignored; use `experimental_instructions_file` or `AGENTS.md`. |
| `mcp_servers.<id>.command` | string | MCP server launcher command. | | `mcp_servers.<id>.command` | string | MCP server launcher command (stdio servers only). |
| `mcp_servers.<id>.args` | array<string> | MCP server args. | | `mcp_servers.<id>.args` | array<string> | MCP server args (stdio servers only). |
| `mcp_servers.<id>.env` | map<string,string> | MCP server env vars. | | `mcp_servers.<id>.env` | map<string,string> | MCP server env vars (stdio servers only). |
| `mcp_servers.<id>.url` | string | MCP server url (streamable http servers only). |
| `mcp_servers.<id>.bearer_token_env_var` | string | environment variable containing a bearer token to use for auth (streamable http servers only). |
| `mcp_servers.<id>.enabled` | boolean | When false, Codex skips starting the server (default: true). |
| `mcp_servers.<id>.startup_timeout_sec` | number | Startup timeout in seconds (default: 10). Timeout is applied both for initializing MCP server and initially listing tools. | | `mcp_servers.<id>.startup_timeout_sec` | number | Startup timeout in seconds (default: 10). Timeout is applied both for initializing MCP server and initially listing tools. |
| `mcp_servers.<id>.tool_timeout_sec` | number | Per-tool timeout in seconds (default: 60). Accepts fractional values; omit to use the default. | | `mcp_servers.<id>.tool_timeout_sec` | number | Per-tool timeout in seconds (default: 60). Accepts fractional values; omit to use the default. |
| `model_providers.<id>.name` | string | Display name. | | `model_providers.<id>.name` | string | Display name. |