mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Allow omitting MCP tool prefixes per server (#34991)
## What changed - Accept a table form of `features.non_prefixed_mcp_tool_names` with an optional `server_names` list while preserving the existing boolean form. - Omit the legacy `mcp__` namespace prefix only for tools from selected MCP servers. When no server list is provided, the enabled feature continues to omit the prefix for every server. - Cover configuration resolution, tool normalization, and an MCP stdio round trip with selected servers. GitOrigin-RevId: bdfb7ac54226de5051f06610e2c6b78b23912ef0
This commit is contained in:
@@ -149,6 +149,7 @@ pub(crate) struct McpConnectionSet {
|
||||
codex_apps_refresh_lock: Mutex<()>,
|
||||
tool_plugin_provenance: Arc<ToolPluginProvenance>,
|
||||
prefix_mcp_tool_names: bool,
|
||||
non_prefixed_mcp_tool_servers: Vec<String>,
|
||||
elicitation_requests: ElicitationRequestManager,
|
||||
}
|
||||
|
||||
@@ -185,6 +186,7 @@ impl McpConnectionSet {
|
||||
let initial_permission_profile = config.permission_profile.clone();
|
||||
let codex_home = config.codex_home.clone();
|
||||
let prefix_mcp_tool_names = config.prefix_mcp_tool_names;
|
||||
let non_prefixed_mcp_tool_servers = config.non_prefixed_mcp_tool_servers.clone();
|
||||
let client_elicitation_capability = config.client_elicitation_capability.clone();
|
||||
let tool_plugin_provenance = crate::mcp::tool_plugin_provenance(&config);
|
||||
let auth = auth.as_ref();
|
||||
@@ -461,6 +463,7 @@ impl McpConnectionSet {
|
||||
codex_apps_refresh_lock: Mutex::new(()),
|
||||
tool_plugin_provenance,
|
||||
prefix_mcp_tool_names,
|
||||
non_prefixed_mcp_tool_servers,
|
||||
elicitation_requests: elicitation_requests.clone(),
|
||||
};
|
||||
let summary_publication_gate = publication_gate;
|
||||
@@ -517,6 +520,7 @@ impl McpConnectionSet {
|
||||
codex_apps_refresh_lock: Mutex::new(()),
|
||||
tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()),
|
||||
prefix_mcp_tool_names,
|
||||
non_prefixed_mcp_tool_servers: Vec::new(),
|
||||
elicitation_requests: ElicitationRequestManager::new(
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::default(),
|
||||
|
||||
@@ -110,7 +110,11 @@ impl McpConnectionSet {
|
||||
.map(|tool| Self::with_server_metadata(tool, &view.metadata)),
|
||||
);
|
||||
}
|
||||
let tools = normalize_tools_for_model_with_prefix(tools, self.prefix_mcp_tool_names);
|
||||
let tools = normalize_tools_for_model_with_prefix(
|
||||
tools,
|
||||
self.prefix_mcp_tool_names,
|
||||
&self.non_prefixed_mcp_tool_servers,
|
||||
);
|
||||
trace!(
|
||||
available_server_count,
|
||||
unavailable_server_count,
|
||||
@@ -170,8 +174,11 @@ impl McpConnectionSet {
|
||||
);
|
||||
}
|
||||
let clients = Arc::new(McpBindingClients::new(clients));
|
||||
let listed_tools =
|
||||
normalize_tools_for_model_with_prefix(listed_tools, self.prefix_mcp_tool_names);
|
||||
let listed_tools = normalize_tools_for_model_with_prefix(
|
||||
listed_tools,
|
||||
self.prefix_mcp_tool_names,
|
||||
&self.non_prefixed_mcp_tool_servers,
|
||||
);
|
||||
let mut tools = Vec::with_capacity(listed_tools.len());
|
||||
let mut calls = std::collections::HashMap::with_capacity(listed_tools.len());
|
||||
for tool_info in listed_tools {
|
||||
@@ -298,7 +305,11 @@ impl McpConnectionSet {
|
||||
)
|
||||
.into_iter()
|
||||
.map(|tool| Self::with_server_metadata(tool, &view.metadata));
|
||||
let tools = normalize_tools_for_model_with_prefix(tools, self.prefix_mcp_tool_names);
|
||||
let tools = normalize_tools_for_model_with_prefix(
|
||||
tools,
|
||||
self.prefix_mcp_tool_names,
|
||||
&self.non_prefixed_mcp_tool_servers,
|
||||
);
|
||||
emit_duration(
|
||||
CODEX_APPS_REFRESH_DURATION_METRIC,
|
||||
refresh_start.elapsed(),
|
||||
|
||||
@@ -91,6 +91,7 @@ impl McpConnectionSet {
|
||||
codex_apps_refresh_lock: Mutex::new(()),
|
||||
tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()),
|
||||
prefix_mcp_tool_names,
|
||||
non_prefixed_mcp_tool_servers: Vec::new(),
|
||||
elicitation_requests: ElicitationRequestManager::new(
|
||||
approval_policy.value(),
|
||||
permission_profile.get().clone(),
|
||||
@@ -806,7 +807,7 @@ fn test_normalize_tools_short_non_duplicated_names() {
|
||||
];
|
||||
|
||||
let model_tools =
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true);
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true, &[]);
|
||||
|
||||
assert_eq!(
|
||||
model_tool_names(&model_tools),
|
||||
@@ -817,6 +818,69 @@ fn test_normalize_tools_short_non_duplicated_names() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_normalize_tools_omits_prefix_only_for_selected_servers() {
|
||||
let tools = vec![
|
||||
create_test_tool("history", "search"),
|
||||
create_test_tool("notes", "read"),
|
||||
create_test_tool("calendar", "list"),
|
||||
];
|
||||
|
||||
let model_tools = normalize_tools_for_model_with_prefix(
|
||||
tools,
|
||||
/*prefix_mcp_tool_names*/ true,
|
||||
&["history".to_string(), "notes".to_string()],
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
model_tool_names(&model_tools),
|
||||
HashSet::from([
|
||||
ToolName::namespaced("history", "search"),
|
||||
ToolName::namespaced("notes", "read"),
|
||||
ToolName::namespaced("mcp__calendar", "list"),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_normalize_tools_selects_raw_server_name() {
|
||||
let mut tool = create_test_tool("codex_apps", "search");
|
||||
tool.callable_namespace = "codex_apps__calendar".to_string();
|
||||
|
||||
let model_tools = normalize_tools_for_model_with_prefix(
|
||||
vec![tool],
|
||||
/*prefix_mcp_tool_names*/ true,
|
||||
&["codex_apps".to_string()],
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
model_tool_names(&model_tools),
|
||||
HashSet::from([ToolName::namespaced("codex_apps__calendar", "search")])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_normalize_tools_global_feature_omits_prefix_for_every_server() {
|
||||
let tools = vec![
|
||||
create_test_tool("history", "search"),
|
||||
create_test_tool("calendar", "list"),
|
||||
];
|
||||
|
||||
let model_tools = normalize_tools_for_model_with_prefix(
|
||||
tools,
|
||||
/*prefix_mcp_tool_names*/ false,
|
||||
&["history".to_string()],
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
model_tool_names(&model_tools),
|
||||
HashSet::from([
|
||||
ToolName::namespaced("history", "search"),
|
||||
ToolName::namespaced("calendar", "list"),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_normalize_tools_duplicated_names_skipped() {
|
||||
let tools = vec![
|
||||
@@ -825,7 +889,7 @@ fn test_normalize_tools_duplicated_names_skipped() {
|
||||
];
|
||||
|
||||
let model_tools =
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true);
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true, &[]);
|
||||
|
||||
// Only the first tool should remain, the second is skipped
|
||||
assert_eq!(
|
||||
@@ -850,7 +914,7 @@ fn test_normalize_tools_long_names_same_server() {
|
||||
];
|
||||
|
||||
let model_tools =
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true);
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true, &[]);
|
||||
|
||||
assert_eq!(model_tools.len(), 2);
|
||||
|
||||
@@ -873,7 +937,7 @@ fn test_normalize_tools_sanitizes_invalid_characters() {
|
||||
let tools = vec![create_test_tool("server.one", "tool.two-three")];
|
||||
|
||||
let model_tools =
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true);
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true, &[]);
|
||||
|
||||
assert_eq!(model_tools.len(), 1);
|
||||
let tool = model_tools.into_iter().next().expect("one tool");
|
||||
@@ -904,7 +968,7 @@ fn test_normalize_tools_keeps_hyphenated_mcp_tools_callable() {
|
||||
let tools = vec![create_test_tool("music-studio", "get-strudel-guide")];
|
||||
|
||||
let model_tools =
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true);
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true, &[]);
|
||||
|
||||
assert_eq!(model_tools.len(), 1);
|
||||
let tool = model_tools.into_iter().next().expect("one tool");
|
||||
@@ -925,7 +989,7 @@ fn test_normalize_tools_disambiguates_sanitized_namespace_collisions() {
|
||||
];
|
||||
|
||||
let model_tools =
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true);
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true, &[]);
|
||||
|
||||
assert_eq!(model_tools.len(), 2);
|
||||
let mut namespaces = model_tools
|
||||
@@ -956,7 +1020,7 @@ fn test_normalize_tools_disambiguates_sanitized_tool_name_collisions() {
|
||||
];
|
||||
|
||||
let model_tools =
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true);
|
||||
normalize_tools_for_model_with_prefix(tools, /*prefix_mcp_tool_names*/ true, &[]);
|
||||
|
||||
assert_eq!(model_tools.len(), 2);
|
||||
let raw_tool_names = model_tools
|
||||
|
||||
@@ -155,6 +155,8 @@ pub struct McpConfig {
|
||||
/// Whether model-visible MCP tool namespaces should keep the legacy
|
||||
/// `mcp__` prefix.
|
||||
pub prefix_mcp_tool_names: bool,
|
||||
/// MCP servers whose model-visible tool namespaces omit the `mcp__` prefix.
|
||||
pub non_prefixed_mcp_tool_servers: Vec<String>,
|
||||
/// Client-side elicitation capabilities advertised during MCP initialization.
|
||||
pub client_elicitation_capability: ElicitationCapability,
|
||||
/// Resolved MCP registrations keyed by logical server name.
|
||||
|
||||
@@ -36,6 +36,7 @@ pub(crate) fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
|
||||
use_legacy_landlock: false,
|
||||
apps_enabled: false,
|
||||
prefix_mcp_tool_names: true,
|
||||
non_prefixed_mcp_tool_servers: Vec::new(),
|
||||
client_elicitation_capability: ElicitationCapability::default(),
|
||||
mcp_server_catalog: ResolvedMcpCatalog::default(),
|
||||
connector_snapshot: codex_connectors::ConnectorSnapshot::default(),
|
||||
|
||||
@@ -109,10 +109,11 @@ pub(crate) fn filter_tools(tools: Vec<ToolInfo>, filter: &ToolFilter) -> Vec<Too
|
||||
/// every model-visible name is unique and <= 64 bytes.
|
||||
///
|
||||
/// When `prefix_mcp_tool_names` is true, the historical `mcp__` namespace
|
||||
/// prefix is added without restoring the old trailing `__` namespace suffix.
|
||||
/// prefix is added except for tools from `non_prefixed_mcp_tool_servers`.
|
||||
pub(crate) fn normalize_tools_for_model_with_prefix<I>(
|
||||
tools: I,
|
||||
prefix_mcp_tool_names: bool,
|
||||
non_prefixed_mcp_tool_servers: &[String],
|
||||
) -> Vec<ToolInfo>
|
||||
where
|
||||
I: IntoIterator<Item = ToolInfo>,
|
||||
@@ -137,7 +138,7 @@ where
|
||||
|
||||
let callable_namespace = callable_namespace_with_prefix(
|
||||
&sanitize_responses_api_tool_name(&tool.callable_namespace),
|
||||
prefix_mcp_tool_names,
|
||||
prefix_mcp_tool_names && !non_prefixed_mcp_tool_servers.contains(&tool.server_name),
|
||||
);
|
||||
|
||||
candidates.push(CallableToolCandidate {
|
||||
|
||||
@@ -35,6 +35,15 @@ pub fn features_schema(schema_gen: &mut SchemaGenerator) -> Schema {
|
||||
);
|
||||
continue;
|
||||
}
|
||||
if feature.id == codex_features::Feature::NonPrefixedMcpToolNames {
|
||||
validation.properties.insert(
|
||||
feature.key.to_string(),
|
||||
schema_gen.subschema_for::<codex_features::FeatureToml<
|
||||
codex_features::NonPrefixedMcpToolNamesConfigToml,
|
||||
>>(),
|
||||
);
|
||||
continue;
|
||||
}
|
||||
if feature.id == codex_features::Feature::MultiAgentV2 {
|
||||
validation.properties.insert(
|
||||
feature.key.to_string(),
|
||||
|
||||
@@ -591,7 +591,7 @@
|
||||
"$ref": "#/definitions/FeatureToml_for_NetworkProxyConfigToml"
|
||||
},
|
||||
"non_prefixed_mcp_tool_names": {
|
||||
"type": "boolean"
|
||||
"$ref": "#/definitions/FeatureToml_for_NonPrefixedMcpToolNamesConfigToml"
|
||||
},
|
||||
"personality": {
|
||||
"type": "boolean"
|
||||
@@ -1007,6 +1007,16 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"FeatureToml_for_NonPrefixedMcpToolNamesConfigToml": {
|
||||
"anyOf": [
|
||||
{
|
||||
"type": "boolean"
|
||||
},
|
||||
{
|
||||
"$ref": "#/definitions/NonPrefixedMcpToolNamesConfigToml"
|
||||
}
|
||||
]
|
||||
},
|
||||
"FeatureToml_for_RolloutBudgetConfigToml": {
|
||||
"anyOf": [
|
||||
{
|
||||
@@ -2085,6 +2095,22 @@
|
||||
"NetworkUnixSocketPermissionsToml": {
|
||||
"type": "object"
|
||||
},
|
||||
"NonPrefixedMcpToolNamesConfigToml": {
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
"enabled": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"server_names": {
|
||||
"description": "MCP servers whose tools should omit the legacy `mcp__` namespace prefix.",
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": "array"
|
||||
}
|
||||
},
|
||||
"type": "object"
|
||||
},
|
||||
"Notice": {
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
@@ -5076,7 +5102,7 @@
|
||||
"$ref": "#/definitions/FeatureToml_for_NetworkProxyConfigToml"
|
||||
},
|
||||
"non_prefixed_mcp_tool_names": {
|
||||
"type": "boolean"
|
||||
"$ref": "#/definitions/FeatureToml_for_NonPrefixedMcpToolNamesConfigToml"
|
||||
},
|
||||
"personality": {
|
||||
"type": "boolean"
|
||||
|
||||
@@ -443,6 +443,65 @@ async fn load_config_resolves_experimental_request_user_input_enabled() -> std::
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_resolves_non_prefixed_mcp_tool_servers() -> std::io::Result<()> {
|
||||
let cases = [
|
||||
(
|
||||
"[features]\nnon_prefixed_mcp_tool_names = false\n",
|
||||
None,
|
||||
true,
|
||||
),
|
||||
(
|
||||
"[features]\nnon_prefixed_mcp_tool_names = true\n",
|
||||
None,
|
||||
false,
|
||||
),
|
||||
(
|
||||
"[features.non_prefixed_mcp_tool_names]\nenabled = true\n",
|
||||
None,
|
||||
false,
|
||||
),
|
||||
(
|
||||
"[features.non_prefixed_mcp_tool_names]\nenabled = true\nserver_names = [\"history\", \"notes\"]\n",
|
||||
Some(vec!["history".to_string(), "notes".to_string()]),
|
||||
true,
|
||||
),
|
||||
(
|
||||
"[features.non_prefixed_mcp_tool_names]\nenabled = true\nserver_names = []\n",
|
||||
Some(Vec::new()),
|
||||
true,
|
||||
),
|
||||
(
|
||||
"[features.non_prefixed_mcp_tool_names]\nenabled = false\nserver_names = [\"history\"]\n",
|
||||
None,
|
||||
true,
|
||||
),
|
||||
];
|
||||
|
||||
for (config_contents, expected_servers, expected_prefix) in cases {
|
||||
let codex_home = tempdir()?;
|
||||
let config_toml = toml::from_str::<ConfigToml>(config_contents)
|
||||
.expect("TOML deserialization should succeed");
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides::default(),
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(config.non_prefixed_mcp_tool_servers, expected_servers);
|
||||
assert_eq!(config.prefix_mcp_tool_names(), expected_prefix);
|
||||
let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
let mcp_config = config.to_mcp_config(&plugins_manager).await;
|
||||
assert_eq!(mcp_config.prefix_mcp_tool_names, expected_prefix);
|
||||
assert_eq!(
|
||||
mcp_config.non_prefixed_mcp_tool_servers,
|
||||
expected_servers.unwrap_or_default()
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_resolves_code_mode_config() -> std::io::Result<()> {
|
||||
let codex_home = tempdir()?;
|
||||
@@ -6225,10 +6284,25 @@ async fn to_mcp_config_flows_mcp_tool_prefix_from_feature() -> std::io::Result<(
|
||||
|
||||
let mcp_config = config.to_mcp_config(&plugins_manager).await;
|
||||
assert!(mcp_config.prefix_mcp_tool_names);
|
||||
assert!(mcp_config.non_prefixed_mcp_tool_servers.is_empty());
|
||||
|
||||
let _ = config.features.enable(Feature::NonPrefixedMcpToolNames);
|
||||
let mcp_config = config.to_mcp_config(&plugins_manager).await;
|
||||
assert!(!mcp_config.prefix_mcp_tool_names);
|
||||
assert!(mcp_config.non_prefixed_mcp_tool_servers.is_empty());
|
||||
|
||||
config.non_prefixed_mcp_tool_servers = Some(vec!["history".to_string(), "notes".to_string()]);
|
||||
let mcp_config = config.to_mcp_config(&plugins_manager).await;
|
||||
assert!(mcp_config.prefix_mcp_tool_names);
|
||||
assert_eq!(
|
||||
mcp_config.non_prefixed_mcp_tool_servers,
|
||||
vec!["history".to_string(), "notes".to_string()]
|
||||
);
|
||||
|
||||
let _ = config.features.disable(Feature::NonPrefixedMcpToolNames);
|
||||
let mcp_config = config.to_mcp_config(&plugins_manager).await;
|
||||
assert!(mcp_config.prefix_mcp_tool_names);
|
||||
assert!(mcp_config.non_prefixed_mcp_tool_servers.is_empty());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -829,6 +829,9 @@ pub struct Config {
|
||||
/// Definition for MCP servers that Codex can reach out to for tool calls.
|
||||
pub mcp_servers: Constrained<HashMap<String, McpServerConfig>>,
|
||||
|
||||
/// When present, only these MCP servers omit the legacy `mcp__` namespace prefix.
|
||||
pub non_prefixed_mcp_tool_servers: Option<Vec<String>>,
|
||||
|
||||
/// Preferred store for MCP OAuth credentials.
|
||||
/// keyring: Use an OS-specific keyring service.
|
||||
/// Credentials stored in the keyring will only be readable by Codex unless the user explicitly grants access via OS-level keyring access.
|
||||
@@ -1641,6 +1644,16 @@ impl Config {
|
||||
use_legacy_landlock: self.features.use_legacy_landlock(),
|
||||
apps_enabled: self.features.enabled(Feature::Apps),
|
||||
prefix_mcp_tool_names: self.prefix_mcp_tool_names(),
|
||||
non_prefixed_mcp_tool_servers: if self
|
||||
.features
|
||||
.enabled(Feature::NonPrefixedMcpToolNames)
|
||||
{
|
||||
self.non_prefixed_mcp_tool_servers
|
||||
.clone()
|
||||
.unwrap_or_default()
|
||||
} else {
|
||||
Vec::new()
|
||||
},
|
||||
client_elicitation_capability: if self.features.enabled(Feature::AuthElicitation) {
|
||||
ElicitationCapability {
|
||||
form: Some(FormElicitationCapability::default()),
|
||||
@@ -1661,6 +1674,7 @@ impl Config {
|
||||
|
||||
pub(crate) fn prefix_mcp_tool_names(&self) -> bool {
|
||||
!self.features.enabled(Feature::NonPrefixedMcpToolNames)
|
||||
|| self.non_prefixed_mcp_tool_servers.is_some()
|
||||
}
|
||||
|
||||
pub async fn rebuild_preserving_session_layers(
|
||||
@@ -3203,6 +3217,17 @@ impl Config {
|
||||
feature_requirements,
|
||||
&mut startup_warnings,
|
||||
)?;
|
||||
let non_prefixed_mcp_tool_servers = if features.enabled(Feature::NonPrefixedMcpToolNames) {
|
||||
cfg.features
|
||||
.as_ref()
|
||||
.and_then(|features| features.non_prefixed_mcp_tool_names.as_ref())
|
||||
.and_then(|feature| match feature {
|
||||
FeatureToml::Enabled(_) => None,
|
||||
FeatureToml::Config(config) => config.server_names.clone(),
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let respect_system_proxy = features.enabled(Feature::RespectSystemProxy);
|
||||
let enable_network_proxy = features.enabled(Feature::NetworkProxy);
|
||||
let configured_windows_sandbox_mode = resolve_windows_sandbox_mode(&cfg);
|
||||
@@ -3960,6 +3985,7 @@ impl Config {
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
mcp_servers,
|
||||
non_prefixed_mcp_tool_servers,
|
||||
// The config.toml omits "_mode" because it's a config file. However, "_mode"
|
||||
// is important in code to differentiate the mode from the store implementation.
|
||||
mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode(
|
||||
|
||||
@@ -695,6 +695,119 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn stdio_mcp_tool_names_respect_selected_servers() -> anyhow::Result<()> {
|
||||
skip_if_wine_exec!(
|
||||
Ok(()),
|
||||
"requires a Windows test_stdio_server in the Wine-exec environment"
|
||||
);
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let call_id = "history-echo";
|
||||
let search_call_id = "search-mcp-echo";
|
||||
mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_tool_search_call(
|
||||
search_call_id,
|
||||
&json!({"query": "echo message and environment data"}),
|
||||
),
|
||||
responses::ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let call_mock = mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-2"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
call_id,
|
||||
"history",
|
||||
"echo",
|
||||
r#"{"message":"ping"}"#,
|
||||
),
|
||||
responses::ev_completed("resp-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let final_mock = mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
responses::ev_assistant_message("msg-1", "history echo completed successfully."),
|
||||
responses::ev_completed("resp-3"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let command = remote_aware_stdio_server_bin()?;
|
||||
let fixture = test_codex()
|
||||
.with_pre_build_hook(move |codex_home| {
|
||||
fs::write(
|
||||
codex_home.join("config.toml"),
|
||||
r#"
|
||||
[features.non_prefixed_mcp_tool_names]
|
||||
enabled = true
|
||||
server_names = ["history", "notes"]
|
||||
"#,
|
||||
)
|
||||
.expect("write MCP namespace configuration");
|
||||
})
|
||||
.with_config(move |config| {
|
||||
for server_name in ["history", "notes", "other"] {
|
||||
insert_mcp_server(
|
||||
config,
|
||||
server_name,
|
||||
stdio_transport(command.clone(), /*env*/ None, Vec::new()),
|
||||
TestMcpServerOptions {
|
||||
environment_id: remote_aware_environment_id(),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
}
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
wait_for_mcp_server(&fixture.codex, "history").await?;
|
||||
|
||||
fixture
|
||||
.submit_turn_with_permission_profile(
|
||||
"call the history echo tool",
|
||||
PermissionProfile::read_only(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let search_output = call_mock
|
||||
.single_request()
|
||||
.tool_search_output(search_call_id);
|
||||
let mut actual_namespaces = [
|
||||
"history",
|
||||
"notes",
|
||||
"other",
|
||||
"mcp__history",
|
||||
"mcp__notes",
|
||||
"mcp__other",
|
||||
]
|
||||
.into_iter()
|
||||
.filter(|namespace| {
|
||||
responses::namespace_child_tool(&search_output, namespace, "echo").is_some()
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
actual_namespaces.sort_unstable();
|
||||
assert_eq!(actual_namespaces, ["history", "mcp__other", "notes"]);
|
||||
|
||||
let output = final_mock.single_request().function_call_output(call_id);
|
||||
let output_text = output["output"]
|
||||
.as_str()
|
||||
.expect("MCP function-call output should be a string");
|
||||
let output_json: Value = serde_json::from_str(split_wall_time_wrapped_output(output_text))?;
|
||||
assert_eq!(output_json["echo"], "ECHOING: ping");
|
||||
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn shutdown_cancels_startup_prewarm_waiting_for_mcp_startup() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -29,6 +29,26 @@ impl FeatureConfig for CodeModeConfigToml {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct NonPrefixedMcpToolNamesConfigToml {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub enabled: Option<bool>,
|
||||
/// MCP servers whose tools should omit the legacy `mcp__` namespace prefix.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub server_names: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
impl FeatureConfig for NonPrefixedMcpToolNamesConfigToml {
|
||||
fn enabled(&self) -> Option<bool> {
|
||||
self.enabled
|
||||
}
|
||||
|
||||
fn set_enabled(&mut self, enabled: bool) {
|
||||
self.enabled = Some(enabled);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct MultiAgentV2ConfigToml {
|
||||
|
||||
@@ -25,6 +25,7 @@ pub use feature_configs::NetworkProxyConfigToml;
|
||||
pub use feature_configs::NetworkProxyDomainPermissionToml;
|
||||
pub use feature_configs::NetworkProxyModeToml;
|
||||
pub use feature_configs::NetworkProxyUnixSocketPermissionToml;
|
||||
pub use feature_configs::NonPrefixedMcpToolNamesConfigToml;
|
||||
use feature_configs::RemovedAppsMcpPathOverrideConfigToml;
|
||||
pub use feature_configs::RolloutBudgetConfigToml;
|
||||
pub use feature_configs::TokenBudgetConfigToml;
|
||||
@@ -647,6 +648,8 @@ pub struct FeaturesToml {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub code_mode: Option<FeatureToml<CodeModeConfigToml>>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub non_prefixed_mcp_tool_names: Option<FeatureToml<NonPrefixedMcpToolNamesConfigToml>>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub multi_agent_v2: Option<FeatureToml<MultiAgentV2ConfigToml>>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub token_budget: Option<FeatureToml<TokenBudgetConfigToml>>,
|
||||
@@ -683,6 +686,13 @@ impl FeaturesToml {
|
||||
if let Some(enabled) = self.code_mode.as_ref().and_then(FeatureToml::enabled) {
|
||||
entries.insert(Feature::CodeMode.key().to_string(), enabled);
|
||||
}
|
||||
if let Some(enabled) = self
|
||||
.non_prefixed_mcp_tool_names
|
||||
.as_ref()
|
||||
.and_then(FeatureToml::enabled)
|
||||
{
|
||||
entries.insert(Feature::NonPrefixedMcpToolNames.key().to_string(), enabled);
|
||||
}
|
||||
if let Some(enabled) = self.multi_agent_v2.as_ref().and_then(FeatureToml::enabled) {
|
||||
entries.insert(Feature::MultiAgentV2.key().to_string(), enabled);
|
||||
}
|
||||
@@ -709,6 +719,7 @@ impl FeaturesToml {
|
||||
self.clear_removed_compatibility_entries();
|
||||
let Self {
|
||||
code_mode,
|
||||
non_prefixed_mcp_tool_names,
|
||||
multi_agent_v2,
|
||||
token_budget,
|
||||
rollout_budget,
|
||||
@@ -724,6 +735,8 @@ impl FeaturesToml {
|
||||
let enabled = features.enabled(spec.id);
|
||||
if spec.id == Feature::CodeMode {
|
||||
materialize_resolved_feature_enabled(code_mode, enabled);
|
||||
} else if spec.id == Feature::NonPrefixedMcpToolNames {
|
||||
materialize_resolved_feature_enabled(non_prefixed_mcp_tool_names, enabled);
|
||||
} else if spec.id == Feature::MultiAgentV2 {
|
||||
materialize_resolved_feature_enabled(multi_agent_v2, enabled);
|
||||
} else if spec.id == Feature::TokenBudget {
|
||||
|
||||
@@ -444,12 +444,54 @@ non_code_mode_only = true
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_prefixed_mcp_tool_names_feature_config_deserializes_boolean_toggle() {
|
||||
let features: FeaturesToml = toml::from_str("non_prefixed_mcp_tool_names = true")
|
||||
.expect("features table should deserialize");
|
||||
|
||||
assert_eq!(
|
||||
features.entries(),
|
||||
BTreeMap::from([("non_prefixed_mcp_tool_names".to_string(), true)])
|
||||
);
|
||||
assert_eq!(
|
||||
features.non_prefixed_mcp_tool_names,
|
||||
Some(FeatureToml::Enabled(true))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_prefixed_mcp_tool_names_feature_config_deserializes_table() {
|
||||
let features: FeaturesToml = toml::from_str(
|
||||
r#"
|
||||
[non_prefixed_mcp_tool_names]
|
||||
enabled = true
|
||||
server_names = ["history", "notes"]
|
||||
"#,
|
||||
)
|
||||
.expect("features table should deserialize");
|
||||
|
||||
assert_eq!(
|
||||
features.entries(),
|
||||
BTreeMap::from([("non_prefixed_mcp_tool_names".to_string(), true)])
|
||||
);
|
||||
assert_eq!(
|
||||
features.non_prefixed_mcp_tool_names,
|
||||
Some(FeatureToml::Config(
|
||||
crate::NonPrefixedMcpToolNamesConfigToml {
|
||||
enabled: Some(true),
|
||||
server_names: Some(vec!["history".to_string(), "notes".to_string()]),
|
||||
}
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config() {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeMode);
|
||||
features.enable(Feature::MultiAgentV2);
|
||||
features.enable(Feature::NetworkProxy);
|
||||
features.enable(Feature::NonPrefixedMcpToolNames);
|
||||
features.enable(Feature::RespectSystemProxy);
|
||||
|
||||
let mut features_toml = FeaturesToml {
|
||||
@@ -463,6 +505,12 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
|
||||
proxy_url: Some("http://127.0.0.1:43128".to_string()),
|
||||
..Default::default()
|
||||
})),
|
||||
non_prefixed_mcp_tool_names: Some(FeatureToml::Config(
|
||||
crate::NonPrefixedMcpToolNamesConfigToml {
|
||||
enabled: Some(false),
|
||||
server_names: Some(vec!["history".to_string(), "notes".to_string()]),
|
||||
},
|
||||
)),
|
||||
entries: BTreeMap::new(),
|
||||
..Default::default()
|
||||
};
|
||||
@@ -494,6 +542,15 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
|
||||
..Default::default()
|
||||
}))
|
||||
);
|
||||
assert_eq!(
|
||||
features_toml.non_prefixed_mcp_tool_names,
|
||||
Some(FeatureToml::Config(
|
||||
crate::NonPrefixedMcpToolNamesConfigToml {
|
||||
enabled: Some(true),
|
||||
server_names: Some(vec!["history".to_string(), "notes".to_string()]),
|
||||
}
|
||||
))
|
||||
);
|
||||
let replayed = Features::from_sources(
|
||||
FeatureConfigSource {
|
||||
features: Some(&features_toml),
|
||||
|
||||
@@ -235,6 +235,7 @@ fn new_config(model: Option<String>, arg0_paths: Arg0DispatchPaths) -> anyhow::R
|
||||
workspace_roots_explicit: false,
|
||||
cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File,
|
||||
mcp_servers: Constrained::allow_any(HashMap::new()),
|
||||
non_prefixed_mcp_tool_servers: None,
|
||||
mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode::File,
|
||||
mcp_oauth_callback_port: None,
|
||||
mcp_oauth_callback_url: None,
|
||||
|
||||
Reference in New Issue
Block a user