Skip to content

[MCP] Add an enabled config field #4917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+155 −14
Open

[MCP] Add an enabled config field #4917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 49 additions & 9 deletions codex-rs/cli/src/mcp_cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -409,52 +411,79 @@ 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());
}
}

println!(
"{:<name_w$} {:<cmd_w$} {:<args_w$} {:<env_w$}",
"{:<name_w$} {:<cmd_w$} {:<args_w$} {:<env_w$} {:<status_w$}",
"Name",
"Command",
"Args",
"Env",
"Status",
name_w = widths[0],
cmd_w = widths[1],
args_w = widths[2],
env_w = widths[3],
status_w = widths[4],
);

for row in &stdio_rows {
println!(
"{:<name_w$} {:<cmd_w$} {:<args_w$} {:<env_w$}",
"{:<name_w$} {:<cmd_w$} {:<args_w$} {:<env_w$} {:<status_w$}",
row[0],
row[1],
row[2],
row[3],
row[4],
name_w = widths[0],
cmd_w = widths[1],
args_w = widths[2],
env_w = widths[3],
status_w = widths[4],
);
}
}
Expand All @@ -464,32 +493,41 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
}

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());
}
}

println!(
"{:<name_w$} {:<url_w$} {:<token_w$}",
"{:<name_w$} {:<url_w$} {:<token_w$} {:<status_w$}",
"Name",
"Url",
"Bearer Token Env Var",
"Status",
name_w = widths[0],
url_w = widths[1],
token_w = widths[2],
status_w = widths[3],
);

for row in &http_rows {
println!(
"{:<name_w$} {:<url_w$} {:<token_w$}",
"{:<name_w$} {:<url_w$} {:<token_w$} {:<status_w$}",
row[0],
row[1],
row[2],
row[3],
name_w = widths[0],
url_w = widths[1],
token_w = widths[2],
status_w = widths[3],
);
}
}
Expand Down Expand Up @@ -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!({
"name": get_args.name,
"enabled": server.enabled,
"transport": transport,
"startup_timeout_sec": server
.startup_timeout_sec
Expand All @@ -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");
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/cli/tests/mcp_add_remove.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(())
}
Expand All @@ -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());
Expand Down Expand Up @@ -153,6 +156,7 @@ async fn add_streamable_http_with_custom_env_var() -> Result<()> {
}
other => panic!("unexpected transport: {other:?}"),
}
assert!(issues.enabled);
Ok(())
}

Expand Down
7 changes: 6 additions & 1 deletion codex-rs/cli/tests/mcp_list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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()?;
Expand All @@ -64,6 +67,7 @@ fn list_and_get_render_expected_output() -> Result<()> {
json!([
{
"name": "docs",
"enabled": true,
"transport": {
"type": "stdio",
"command": "docs-server",
Expand Down Expand Up @@ -91,14 +95,15 @@ 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())?;
get_json_cmd
.args(["mcp", "get", "docs", "--json"])
.assert()
.success()
.stdout(contains("\"name\": \"docs\""));
.stdout(contains("\"name\": \"docs\"").and(contains("\"enabled\": true")));

Ok(())
}
43 changes: 43 additions & 0 deletions codex-rs/core/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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)),
},
Expand All @@ -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)?;
Expand Down Expand Up @@ -1639,6 +1645,7 @@ bearer_token = "secret"
("ALPHA_VAR".to_string(), "1".to_string()),
])),
},
enabled: true,
startup_timeout_sec: None,
tool_timeout_sec: None,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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()?;
Expand Down
Loading
Loading