mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Propagate MCP disabled reason (#9207)
Indicate why MCP servers are disabled when they are disabled by
requirements:
```
➜ codex git:(main) ✗ just codex mcp list
cargo run --bin codex -- "$@"
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.27s
Running `target/debug/codex mcp list`
Name Command Args Env Cwd Status Auth
docs docs-mcp - - - disabled: requirements (MDM com.openai.codex:requirements_toml_base64) Unsupported
hello_world hello-world-mcp - - - disabled: requirements (MDM com.openai.codex:requirements_toml_base64) Unsupported
➜ codex git:(main) ✗ just c
cargo run --bin codex -- "$@"
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.90s
Running `target/debug/codex`
╭─────────────────────────────────────────────╮
│ >_ OpenAI Codex (v0.0.0) │
│ │
│ model: gpt-5.2 xhigh /model to change │
│ directory: ~/code/codex/codex-rs │
╰─────────────────────────────────────────────╯
/mcp
🔌 MCP Tools
• No MCP tools available.
• docs (disabled)
• Reason: requirements (MDM com.openai.codex:requirements_toml_base64)
• hello_world (disabled)
• Reason: requirements (MDM com.openai.codex:requirements_toml_base64)
```
This commit is contained in:
@@ -1124,6 +1124,7 @@ gpt-5 = "gpt-5.1"
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: Some(vec!["one".to_string(), "two".to_string()]),
|
||||
@@ -1145,6 +1146,7 @@ gpt-5 = "gpt-5.1"
|
||||
env_http_headers: None,
|
||||
},
|
||||
enabled: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(std::time::Duration::from_secs(5)),
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -1209,6 +1211,7 @@ foo = { command = "cmd" }
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -1252,6 +1255,7 @@ foo = { command = "cmd" } # keep me
|
||||
cwd: None,
|
||||
},
|
||||
enabled: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -1294,6 +1298,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -1337,6 +1342,7 @@ foo = { command = "cmd" }
|
||||
cwd: None,
|
||||
},
|
||||
enabled: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
|
||||
@@ -2,6 +2,7 @@ use crate::auth::AuthCredentialsStoreMode;
|
||||
use crate::config::types::DEFAULT_OTEL_ENVIRONMENT;
|
||||
use crate::config::types::History;
|
||||
use crate::config::types::McpServerConfig;
|
||||
use crate::config::types::McpServerDisabledReason;
|
||||
use crate::config::types::McpServerTransportConfig;
|
||||
use crate::config::types::Notice;
|
||||
use crate::config::types::Notifications;
|
||||
@@ -19,6 +20,7 @@ use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::LoaderOverrides;
|
||||
use crate::config_loader::McpServerIdentity;
|
||||
use crate::config_loader::McpServerRequirement;
|
||||
use crate::config_loader::Sourced;
|
||||
use crate::config_loader::load_config_layers_state;
|
||||
use crate::features::Feature;
|
||||
use crate::features::FeatureOverrides;
|
||||
@@ -539,25 +541,32 @@ fn deserialize_config_toml_with_base(
|
||||
|
||||
fn filter_mcp_servers_by_requirements(
|
||||
mcp_servers: &mut HashMap<String, McpServerConfig>,
|
||||
mcp_requirements: Option<&BTreeMap<String, McpServerRequirement>>,
|
||||
mcp_requirements: Option<&Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
) {
|
||||
let Some(allowlist) = mcp_requirements else {
|
||||
return;
|
||||
};
|
||||
|
||||
let source = allowlist.source.clone();
|
||||
for (name, server) in mcp_servers.iter_mut() {
|
||||
let allowed = allowlist
|
||||
.value
|
||||
.get(name)
|
||||
.is_some_and(|requirement| mcp_server_matches_requirement(requirement, server));
|
||||
if !allowed {
|
||||
if allowed {
|
||||
server.disabled_reason = None;
|
||||
} else {
|
||||
server.enabled = false;
|
||||
server.disabled_reason = Some(McpServerDisabledReason::Requirements {
|
||||
source: source.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn constrain_mcp_servers(
|
||||
mcp_servers: HashMap<String, McpServerConfig>,
|
||||
mcp_requirements: Option<&BTreeMap<String, McpServerRequirement>>,
|
||||
mcp_requirements: Option<&Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
) -> ConstraintResult<Constrained<HashMap<String, McpServerConfig>>> {
|
||||
if mcp_requirements.is_none() {
|
||||
return Ok(Constrained::allow_any(mcp_servers));
|
||||
@@ -1707,6 +1716,7 @@ mod tests {
|
||||
use crate::config::types::HistoryPersistence;
|
||||
use crate::config::types::McpServerTransportConfig;
|
||||
use crate::config::types::Notifications;
|
||||
use crate::config_loader::RequirementSource;
|
||||
use crate::features::Feature;
|
||||
|
||||
use super::*;
|
||||
@@ -1728,6 +1738,7 @@ mod tests {
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -1744,6 +1755,7 @@ mod tests {
|
||||
env_http_headers: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -1976,9 +1988,9 @@ trust_level = "trusted"
|
||||
(MATCHED_URL_SERVER.to_string(), http_mcp(GOOD_URL)),
|
||||
(DIFFERENT_NAME_SERVER.to_string(), stdio_mcp("same-cmd")),
|
||||
]);
|
||||
filter_mcp_servers_by_requirements(
|
||||
&mut servers,
|
||||
Some(&BTreeMap::from([
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(
|
||||
BTreeMap::from([
|
||||
(
|
||||
MISMATCHED_URL_SERVER.to_string(),
|
||||
McpServerRequirement {
|
||||
@@ -2011,20 +2023,29 @@ trust_level = "trusted"
|
||||
},
|
||||
},
|
||||
),
|
||||
])),
|
||||
]),
|
||||
source.clone(),
|
||||
);
|
||||
filter_mcp_servers_by_requirements(&mut servers, Some(&requirements));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (name.clone(), server.enabled))
|
||||
.collect::<HashMap<String, bool>>(),
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
(MISMATCHED_URL_SERVER.to_string(), false),
|
||||
(MISMATCHED_COMMAND_SERVER.to_string(), false),
|
||||
(MATCHED_URL_SERVER.to_string(), true),
|
||||
(MATCHED_COMMAND_SERVER.to_string(), true),
|
||||
(DIFFERENT_NAME_SERVER.to_string(), false),
|
||||
(MISMATCHED_URL_SERVER.to_string(), (false, reason.clone())),
|
||||
(
|
||||
MISMATCHED_COMMAND_SERVER.to_string(),
|
||||
(false, reason.clone()),
|
||||
),
|
||||
(MATCHED_URL_SERVER.to_string(), (true, None)),
|
||||
(MATCHED_COMMAND_SERVER.to_string(), (true, None)),
|
||||
(DIFFERENT_NAME_SERVER.to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
@@ -2041,11 +2062,14 @@ trust_level = "trusted"
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (name.clone(), server.enabled))
|
||||
.collect::<HashMap<String, bool>>(),
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
("server-a".to_string(), true),
|
||||
("server-b".to_string(), true),
|
||||
("server-a".to_string(), (true, None)),
|
||||
("server-b".to_string(), (true, None)),
|
||||
])
|
||||
);
|
||||
}
|
||||
@@ -2057,16 +2081,22 @@ trust_level = "trusted"
|
||||
("server-b".to_string(), http_mcp("https://example.com/b")),
|
||||
]);
|
||||
|
||||
filter_mcp_servers_by_requirements(&mut servers, Some(&BTreeMap::new()));
|
||||
let source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let requirements = Sourced::new(BTreeMap::new(), source.clone());
|
||||
filter_mcp_servers_by_requirements(&mut servers, Some(&requirements));
|
||||
|
||||
let reason = Some(McpServerDisabledReason::Requirements { source });
|
||||
assert_eq!(
|
||||
servers
|
||||
.iter()
|
||||
.map(|(name, server)| (name.clone(), server.enabled))
|
||||
.collect::<HashMap<String, bool>>(),
|
||||
.map(|(name, server)| (
|
||||
name.clone(),
|
||||
(server.enabled, server.disabled_reason.clone())
|
||||
))
|
||||
.collect::<HashMap<String, (bool, Option<McpServerDisabledReason>)>>(),
|
||||
HashMap::from([
|
||||
("server-a".to_string(), false),
|
||||
("server-b".to_string(), false),
|
||||
("server-a".to_string(), (false, reason.clone())),
|
||||
("server-b".to_string(), (false, reason)),
|
||||
])
|
||||
);
|
||||
}
|
||||
@@ -2491,6 +2521,7 @@ trust_level = "trusted"
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(3)),
|
||||
tool_timeout_sec: Some(Duration::from_secs(5)),
|
||||
enabled_tools: None,
|
||||
@@ -2644,6 +2675,7 @@ bearer_token = "secret"
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -2712,6 +2744,7 @@ ZIG_VAR = "3"
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -2760,6 +2793,7 @@ ZIG_VAR = "3"
|
||||
cwd: Some(cwd_path.clone()),
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -2806,6 +2840,7 @@ ZIG_VAR = "3"
|
||||
env_http_headers: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(2)),
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -2868,6 +2903,7 @@ startup_timeout_sec = 2.0
|
||||
)])),
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(2)),
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -2942,6 +2978,7 @@ X-Auth = "DOCS_AUTH"
|
||||
)])),
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(2)),
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -2969,6 +3006,7 @@ X-Auth = "DOCS_AUTH"
|
||||
env_http_headers: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -3034,6 +3072,7 @@ url = "https://example.com/mcp"
|
||||
)])),
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(2)),
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -3051,6 +3090,7 @@ url = "https://example.com/mcp"
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -3131,6 +3171,7 @@ url = "https://example.com/mcp"
|
||||
cwd: None,
|
||||
},
|
||||
enabled: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -3173,6 +3214,7 @@ url = "https://example.com/mcp"
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: Some(vec!["allowed".to_string()]),
|
||||
|
||||
@@ -3,11 +3,13 @@
|
||||
// Note this file should generally be restricted to simple struct/enum
|
||||
// definitions that do not contain business logic.
|
||||
|
||||
use crate::config_loader::RequirementSource;
|
||||
pub use codex_protocol::config_types::AltScreenMode;
|
||||
pub use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::fmt;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
use wildmatch::WildMatchPattern;
|
||||
@@ -20,6 +22,23 @@ use serde::de::Error as SerdeError;
|
||||
|
||||
pub const DEFAULT_OTEL_ENVIRONMENT: &str = "dev";
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum McpServerDisabledReason {
|
||||
Unknown,
|
||||
Requirements { source: RequirementSource },
|
||||
}
|
||||
|
||||
impl fmt::Display for McpServerDisabledReason {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
McpServerDisabledReason::Unknown => write!(f, "unknown"),
|
||||
McpServerDisabledReason::Requirements { source } => {
|
||||
write!(f, "requirements ({source})")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Debug, Clone, PartialEq)]
|
||||
pub struct McpServerConfig {
|
||||
#[serde(flatten)]
|
||||
@@ -29,6 +48,10 @@ pub struct McpServerConfig {
|
||||
#[serde(default = "default_enabled")]
|
||||
pub enabled: bool,
|
||||
|
||||
/// Reason this server was disabled after applying requirements.
|
||||
#[serde(skip)]
|
||||
pub disabled_reason: Option<McpServerDisabledReason>,
|
||||
|
||||
/// Startup timeout in seconds for initializing MCP server & initially listing tools.
|
||||
#[serde(
|
||||
default,
|
||||
@@ -160,6 +183,7 @@ impl<'de> Deserialize<'de> for McpServerConfig {
|
||||
startup_timeout_sec,
|
||||
tool_timeout_sec,
|
||||
enabled,
|
||||
disabled_reason: None,
|
||||
enabled_tools,
|
||||
disabled_tools,
|
||||
})
|
||||
|
||||
@@ -44,7 +44,7 @@ impl fmt::Display for RequirementSource {
|
||||
pub struct ConfigRequirements {
|
||||
pub approval_policy: Constrained<AskForApproval>,
|
||||
pub sandbox_policy: Constrained<SandboxPolicy>,
|
||||
pub mcp_servers: Option<BTreeMap<String, McpServerRequirement>>,
|
||||
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
}
|
||||
|
||||
impl Default for ConfigRequirements {
|
||||
@@ -273,7 +273,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
Ok(ConfigRequirements {
|
||||
approval_policy,
|
||||
sandbox_policy,
|
||||
mcp_servers: mcp_servers.map(|sourced| sourced.value),
|
||||
mcp_servers,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -571,24 +571,27 @@ mod tests {
|
||||
|
||||
assert_eq!(
|
||||
requirements.mcp_servers,
|
||||
Some(BTreeMap::from([
|
||||
(
|
||||
"docs".to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "codex-mcp".to_string(),
|
||||
Some(Sourced::new(
|
||||
BTreeMap::from([
|
||||
(
|
||||
"docs".to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Command {
|
||||
command: "codex-mcp".to_string(),
|
||||
},
|
||||
},
|
||||
},
|
||||
),
|
||||
(
|
||||
"remote".to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: "https://example.com/mcp".to_string(),
|
||||
),
|
||||
(
|
||||
"remote".to_string(),
|
||||
McpServerRequirement {
|
||||
identity: McpServerIdentity::Url {
|
||||
url: "https://example.com/mcp".to_string(),
|
||||
},
|
||||
},
|
||||
},
|
||||
),
|
||||
]))
|
||||
),
|
||||
]),
|
||||
RequirementSource::Unknown,
|
||||
))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ pub use config_requirements::McpServerIdentity;
|
||||
pub use config_requirements::McpServerRequirement;
|
||||
pub use config_requirements::RequirementSource;
|
||||
pub use config_requirements::SandboxModeRequirement;
|
||||
pub use config_requirements::Sourced;
|
||||
pub use merge::merge_toml_values;
|
||||
pub(crate) use overrides::build_cli_overrides_layer;
|
||||
pub use state::ConfigLayerEntry;
|
||||
|
||||
@@ -1171,6 +1171,7 @@ mod tests {
|
||||
env_http_headers: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
@@ -1215,6 +1216,7 @@ mod tests {
|
||||
env_http_headers: None,
|
||||
},
|
||||
enabled: true,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
|
||||
Reference in New Issue
Block a user