From 3f2603f2a261f7cb8a479dcae274bfe7ad5c7155 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 13 Nov 2026 17:17:04 +0000 Subject: [PATCH] Clarify removal scope for profile MCP servers (#61357) ## Why `codex mcp get` suggests `codex mcp remove` for servers defined in the active profile, but that command only edits the global config. ## What changed For servers defined in the active profile, replace the removal hint with the profile settings file path and an explanation of the command's scope. Keep the removal hint for servers inherited from the global config. ## Testing Add CLI coverage for profile-only servers, profile overrides of global servers, and inherited global servers. GitOrigin-RevId: 454df034f7e74b10f96983a9f13821d5ababfde7 --- codex-rs/cli/src/mcp_cmd.rs | 19 ++++++++++++- codex-rs/cli/tests/mcp_list.rs | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 6e462d18e0..07e729cd09 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -7,6 +7,7 @@ use anyhow::anyhow; use anyhow::bail; use clap::ArgGroup; use clap::builder::TypedValueParser; +use codex_config::ConfigLayerSource; use codex_config::types::AppToolApproval; use codex_config::types::McpServerAuth; use codex_config::types::McpServerConfig; @@ -1131,7 +1132,23 @@ async fn run_get(config: &Config, get_args: GetArgs) -> Result<()> { }; println!(" default_tools_approval_mode: {approval_mode}"); } - println!(" remove: codex mcp remove {}", get_args.name); + if let Some(layer) = config.config_layer_stack.get_active_user_layer() + && let ConfigLayerSource::User { + file, + profile: Some(_), + } = &layer.name + && !layer.is_disabled() + && layer + .config + .get("mcp_servers") + .and_then(|servers| servers.get(&get_args.name)) + .is_some() + { + println!(" profile settings: {}", file.as_path().display()); + println!(" removal scope: `codex mcp remove` only edits the global config."); + } else { + println!(" remove: codex mcp remove {}", get_args.name); + } Ok(()) } diff --git a/codex-rs/cli/tests/mcp_list.rs b/codex-rs/cli/tests/mcp_list.rs index 175ff030a9..56366e0742 100644 --- a/codex-rs/cli/tests/mcp_list.rs +++ b/codex-rs/cli/tests/mcp_list.rs @@ -11,6 +11,7 @@ use codex_config::types::McpServerTransportConfig; use codex_core::config::edit::ConfigEditsBuilder; use codex_core::config::load_global_mcp_servers; use codex_login::CODEX_API_KEY_ENV_VAR; +use codex_utils_absolute_path::AbsolutePathBuf; use predicates::prelude::PredicateBooleanExt; use predicates::str::contains; use pretty_assertions::assert_eq; @@ -420,6 +421,56 @@ async fn list_and_get_render_expected_output() -> Result<()> { Ok(()) } +#[test] +fn get_reports_removal_scope_for_profile_servers() -> Result<()> { + let codex_home = TempDir::new()?; + let profile_path = AbsolutePathBuf::from_absolute_path(codex_home.path().canonicalize()?)? + .join("work.config.toml"); + std::fs::write( + codex_home.path().join("config.toml"), + "[mcp_servers.shadowed]\ncommand = \"basecmd\"\n\n[mcp_servers.inherited]\ncommand = \"basecmd\"\n", + )?; + std::fs::write( + profile_path.as_path(), + "[mcp_servers.shadowed]\ncommand = \"profilecmd\"\n\n[mcp_servers.profile_only]\ncommand = \"profilecmd\"\n", + )?; + + for name in ["shadowed", "profile_only"] { + let output = codex_command(codex_home.path())? + .current_dir(codex_home.path()) + .args(["-p", "work", "mcp", "get", name]) + .assert() + .success(); + let stdout = String::from_utf8_lossy(&output.get_output().stdout); + let stdout = stdout.replace( + profile_path.as_path().to_string_lossy().as_ref(), + "", + ); + let (_, details) = stdout.split_once('\n').expect("expected server details"); + insta::allow_duplicates! { + insta::assert_snapshot!(details, @r" + enabled: true + transport: stdio + command: profilecmd + args: - + cwd: - + env: - + profile settings: + removal scope: `codex mcp remove` only edits the global config. + "); + } + } + + codex_command(codex_home.path())? + .current_dir(codex_home.path()) + .args(["-p", "work", "mcp", "get", "inherited"]) + .assert() + .success() + .stdout(contains("remove: codex mcp remove inherited")); + + Ok(()) +} + #[test] fn list_and_get_redact_http_headers_helper() -> Result<()> { let codex_home = TempDir::new()?;