mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
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
This commit is contained in:
@@ -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(())
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
"<profile file>",
|
||||
);
|
||||
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: <profile file>
|
||||
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()?;
|
||||
|
||||
Reference in New Issue
Block a user