mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add server-level approval defaults for custom MCP servers (#17843)
## Summary - Add `default_tools_approval_mode` support for custom MCP server configs, matching the existing `codex_apps` behavior - Apply approval precedence as per-tool override, then server default, then `auto` - Update config serialization, CLI display, schema generation, docs, and tests ## Testing - `cargo check -p codex-config` - `cargo check -p codex-core` - `just write-config-schema` - `just fmt` - `cargo test -p codex-config` - Targeted `codex-core` tests for config parsing, config writes, and MCP approval precedence - `just fix -p codex-config -p codex-core`
This commit is contained in:
committed by
GitHub
Unverified
parent
206dd13c32
commit
71174574ad
@@ -6,6 +6,7 @@ use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use anyhow::bail;
|
||||
use clap::ArgGroup;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::McpManager;
|
||||
@@ -304,6 +305,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -879,6 +881,14 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
|
||||
if let Some(timeout) = server.tool_timeout_sec {
|
||||
println!(" tool_timeout_sec: {}", timeout.as_secs_f64());
|
||||
}
|
||||
if let Some(approval_mode) = server.default_tools_approval_mode {
|
||||
let approval_mode = match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Approve => "approve",
|
||||
};
|
||||
println!(" default_tools_approval_mode: {approval_mode}");
|
||||
}
|
||||
println!(" remove: codex mcp remove {}", get_args.name);
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -276,6 +276,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig, auth: Option<&CodexAuth>) ->
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(30)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -200,6 +200,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -223,6 +224,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -126,6 +126,7 @@ fn mcp_dependency_to_server_config(
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -154,6 +155,7 @@ fn mcp_dependency_to_server_config(
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -46,6 +46,7 @@ fn collect_missing_respects_canonical_installed_key() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -98,6 +99,7 @@ fn collect_missing_dedupes_by_canonical_key_but_preserves_original_name() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -799,6 +799,7 @@ fn mcp_init_error_display_prompts_for_github_pat() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -850,6 +851,7 @@ fn mcp_init_error_display_reports_generic_errors() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -189,6 +189,13 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem {
|
||||
if let Some(timeout) = config.tool_timeout_sec {
|
||||
entry["tool_timeout_sec"] = value(timeout.as_secs_f64());
|
||||
}
|
||||
if let Some(approval_mode) = config.default_tools_approval_mode {
|
||||
entry["default_tools_approval_mode"] = value(match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Approve => "approve",
|
||||
});
|
||||
}
|
||||
if let Some(enabled_tools) = &config.enabled_tools
|
||||
&& !enabled_tools.is_empty()
|
||||
{
|
||||
|
||||
@@ -29,6 +29,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow:
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: Some(AppToolApproval::Auto),
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -62,6 +63,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow:
|
||||
r#"[mcp_servers.docs]
|
||||
command = "docs-server"
|
||||
supports_parallel_tool_calls = true
|
||||
default_tools_approval_mode = "auto"
|
||||
|
||||
[mcp_servers.docs.tools]
|
||||
|
||||
|
||||
@@ -93,6 +93,10 @@ pub struct McpServerConfig {
|
||||
#[serde(default, with = "option_duration_secs")]
|
||||
pub tool_timeout_sec: Option<Duration>,
|
||||
|
||||
/// Approval mode for tools in this server unless a tool override exists.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub default_tools_approval_mode: Option<AppToolApproval>,
|
||||
|
||||
/// Explicit allow-list of tools exposed from this server. When set, only these tools will be registered.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub enabled_tools: Option<Vec<String>>,
|
||||
@@ -158,6 +162,8 @@ pub struct RawMcpServerConfig {
|
||||
#[serde(default)]
|
||||
pub supports_parallel_tool_calls: Option<bool>,
|
||||
#[serde(default)]
|
||||
pub default_tools_approval_mode: Option<AppToolApproval>,
|
||||
#[serde(default)]
|
||||
pub enabled_tools: Option<Vec<String>>,
|
||||
#[serde(default)]
|
||||
pub disabled_tools: Option<Vec<String>>,
|
||||
@@ -194,6 +200,7 @@ impl TryFrom<RawMcpServerConfig> for McpServerConfig {
|
||||
enabled,
|
||||
required,
|
||||
supports_parallel_tool_calls,
|
||||
default_tools_approval_mode,
|
||||
enabled_tools,
|
||||
disabled_tools,
|
||||
scopes,
|
||||
@@ -260,6 +267,7 @@ impl TryFrom<RawMcpServerConfig> for McpServerConfig {
|
||||
required: required.unwrap_or_default(),
|
||||
supports_parallel_tool_calls: supports_parallel_tool_calls.unwrap_or_default(),
|
||||
disabled_reason: None,
|
||||
default_tools_approval_mode,
|
||||
enabled_tools,
|
||||
disabled_tools,
|
||||
scopes,
|
||||
|
||||
@@ -258,6 +258,38 @@ fn deserialize_server_config_with_parallel_tool_calls() {
|
||||
assert!(cfg.supports_parallel_tool_calls);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_server_config_with_default_tool_approval_mode() {
|
||||
let cfg: McpServerConfig = toml::from_str(
|
||||
r#"
|
||||
command = "echo"
|
||||
default_tools_approval_mode = "approve"
|
||||
|
||||
[tools.search]
|
||||
approval_mode = "prompt"
|
||||
"#,
|
||||
)
|
||||
.expect("should deserialize default tool approval mode");
|
||||
|
||||
assert_eq!(
|
||||
cfg.default_tools_approval_mode,
|
||||
Some(AppToolApproval::Approve)
|
||||
);
|
||||
assert_eq!(
|
||||
cfg.tools.get("search"),
|
||||
Some(&McpServerToolConfig {
|
||||
approval_mode: Some(AppToolApproval::Prompt),
|
||||
})
|
||||
);
|
||||
|
||||
let serialized = toml::to_string(&cfg).expect("should serialize MCP config");
|
||||
assert!(serialized.contains("default_tools_approval_mode = \"approve\""));
|
||||
|
||||
let round_tripped: McpServerConfig =
|
||||
toml::from_str(&serialized).expect("should deserialize serialized MCP config");
|
||||
assert_eq!(round_tripped, cfg);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_round_trips_server_config_with_parallel_tool_calls() {
|
||||
let cfg: McpServerConfig = toml::from_str(
|
||||
@@ -304,6 +336,7 @@ fn deserialize_ignores_unknown_server_fields() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -1463,6 +1463,14 @@
|
||||
"default": null,
|
||||
"type": "string"
|
||||
},
|
||||
"default_tools_approval_mode": {
|
||||
"allOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AppToolApproval"
|
||||
}
|
||||
],
|
||||
"default": null
|
||||
},
|
||||
"disabled_tools": {
|
||||
"default": null,
|
||||
"items": {
|
||||
|
||||
@@ -90,6 +90,7 @@ fn stdio_mcp(command: &str) -> McpServerConfig {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -113,6 +114,7 @@ fn http_mcp(url: &str) -> McpServerConfig {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2130,6 +2132,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(3)),
|
||||
tool_timeout_sec: Some(Duration::from_secs(5)),
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2243,23 +2246,28 @@ fn mcp_servers_toml_parses_per_tool_approval_overrides() {
|
||||
[mcp_servers.docs]
|
||||
command = "docs-server"
|
||||
name = "Docs"
|
||||
default_tools_approval_mode = "prompt"
|
||||
|
||||
[mcp_servers.docs.tools.search]
|
||||
approval_mode = "approve"
|
||||
"#,
|
||||
)
|
||||
.expect("TOML deserialization should succeed");
|
||||
let tool = config
|
||||
let server = config
|
||||
.mcp_servers
|
||||
.get("docs")
|
||||
.and_then(|server| server.tools.get("search"))
|
||||
.expect("docs/search tool config exists");
|
||||
.expect("docs server config exists");
|
||||
|
||||
assert_eq!(
|
||||
tool,
|
||||
&McpServerToolConfig {
|
||||
server.default_tools_approval_mode,
|
||||
Some(AppToolApproval::Prompt)
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
server.tools.get("search"),
|
||||
Some(&McpServerToolConfig {
|
||||
approval_mode: Some(AppToolApproval::Approve),
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2380,6 +2388,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2455,6 +2464,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2510,6 +2520,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2563,6 +2574,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(2)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2632,6 +2644,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(2)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2713,6 +2726,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(2)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2747,6 +2761,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2816,6 +2831,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers()
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(2)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2840,6 +2856,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers()
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2927,6 +2944,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -2976,6 +2994,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -3025,6 +3044,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: Some(vec!["allowed".to_string()]),
|
||||
disabled_tools: Some(vec!["blocked".to_string()]),
|
||||
scopes: None,
|
||||
@@ -3078,6 +3098,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -237,6 +237,13 @@ mod document_helpers {
|
||||
if let Some(timeout) = config.tool_timeout_sec {
|
||||
entry["tool_timeout_sec"] = value(timeout.as_secs_f64());
|
||||
}
|
||||
if let Some(approval_mode) = config.default_tools_approval_mode {
|
||||
entry["default_tools_approval_mode"] = value(match approval_mode {
|
||||
AppToolApproval::Auto => "auto",
|
||||
AppToolApproval::Prompt => "prompt",
|
||||
AppToolApproval::Approve => "approve",
|
||||
});
|
||||
}
|
||||
if let Some(enabled_tools) = &config.enabled_tools
|
||||
&& !enabled_tools.is_empty()
|
||||
{
|
||||
|
||||
@@ -582,6 +582,7 @@ fn blocking_replace_mcp_servers_round_trips() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: Some(vec!["one".to_string(), "two".to_string()]),
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -610,6 +611,7 @@ fn blocking_replace_mcp_servers_round_trips() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(std::time::Duration::from_secs(5)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: Some(vec!["forbidden".to_string()]),
|
||||
scopes: None,
|
||||
@@ -675,6 +677,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: Some(AppToolApproval::Prompt),
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -699,6 +702,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() {
|
||||
let expected = "\
|
||||
[mcp_servers.docs]
|
||||
command = \"docs-server\"
|
||||
default_tools_approval_mode = \"prompt\"
|
||||
|
||||
[mcp_servers.docs.tools.search]
|
||||
approval_mode = \"approve\"
|
||||
@@ -737,6 +741,7 @@ foo = { command = "cmd" }
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -790,6 +795,7 @@ foo = { command = "cmd" } # keep me
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -842,6 +848,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -895,6 +902,7 @@ foo = { command = "cmd" }
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -164,6 +164,49 @@ async fn write_value_supports_nested_app_paths() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn write_value_supports_custom_mcp_server_default_tool_approval_mode() -> Result<()> {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
std::fs::write(
|
||||
tmp.path().join(CONFIG_TOML_FILE),
|
||||
"[mcp_servers.docs]\ncommand = \"docs-server\"\n",
|
||||
)?;
|
||||
|
||||
let service = ConfigService::without_managed_config_for_tests(tmp.path().to_path_buf());
|
||||
service
|
||||
.write_value(ConfigValueWriteParams {
|
||||
file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()),
|
||||
key_path: "mcp_servers.docs.default_tools_approval_mode".to_string(),
|
||||
value: serde_json::json!("approve"),
|
||||
merge_strategy: MergeStrategy::Replace,
|
||||
expected_version: None,
|
||||
})
|
||||
.await
|
||||
.expect("write mcp server default_tools_approval_mode succeeds");
|
||||
|
||||
let contents = std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE))?;
|
||||
assert!(contents.contains("default_tools_approval_mode = \"approve\""));
|
||||
|
||||
let read = service
|
||||
.read(ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: None,
|
||||
})
|
||||
.await
|
||||
.expect("config read succeeds");
|
||||
|
||||
assert_eq!(
|
||||
read.config
|
||||
.additional
|
||||
.get("mcp_servers")
|
||||
.and_then(|servers| servers.get("docs"))
|
||||
.and_then(|docs| docs.get("default_tools_approval_mode")),
|
||||
Some(&serde_json::json!("approve"))
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_includes_origins_and_layers() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
|
||||
@@ -371,6 +371,7 @@ fn mcp_dependency_to_server_config(
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -399,6 +400,7 @@ fn mcp_dependency_to_server_config(
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -676,9 +676,14 @@ fn custom_mcp_tool_approval_mode(
|
||||
.and_then(|value| {
|
||||
HashMap::<String, codex_config::types::McpServerConfig>::deserialize(value).ok()
|
||||
})
|
||||
.and_then(|servers| servers.get(server).cloned())
|
||||
.and_then(|server| server.tools.get(tool_name).cloned())
|
||||
.and_then(|tool| tool.approval_mode)
|
||||
.and_then(|servers| {
|
||||
let server_config = servers.get(server)?;
|
||||
server_config
|
||||
.tools
|
||||
.get(tool_name)
|
||||
.and_then(|tool| tool.approval_mode)
|
||||
.or(server_config.default_tools_approval_mode)
|
||||
})
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
|
||||
@@ -1146,6 +1146,43 @@ async fn persist_custom_mcp_tool_approval_writes_tool_override() {
|
||||
assert!(contents.contains("[mcp_servers.docs.tools.search]"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn custom_mcp_tool_approval_mode_uses_server_default_with_tool_override() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
std::fs::write(
|
||||
tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
[mcp_servers.docs]
|
||||
command = "docs-server"
|
||||
default_tools_approval_mode = "approve"
|
||||
|
||||
[mcp_servers.docs.tools.search]
|
||||
approval_mode = "prompt"
|
||||
"#,
|
||||
)
|
||||
.expect("seed config");
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("load config");
|
||||
let (_session, mut turn_context) = make_session_and_context().await;
|
||||
turn_context.config = Arc::new(config);
|
||||
|
||||
assert_eq!(
|
||||
custom_mcp_tool_approval_mode(&turn_context, "docs", "read"),
|
||||
AppToolApproval::Approve
|
||||
);
|
||||
assert_eq!(
|
||||
custom_mcp_tool_approval_mode(&turn_context, "docs", "search"),
|
||||
AppToolApproval::Prompt
|
||||
);
|
||||
assert_eq!(
|
||||
custom_mcp_tool_approval_mode(&turn_context, "unknown", "search"),
|
||||
AppToolApproval::Auto
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn maybe_persist_mcp_tool_approval_reloads_session_config() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
|
||||
@@ -183,6 +183,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -520,6 +521,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -630,6 +632,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -788,6 +791,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: None,
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -239,6 +239,7 @@ async fn run_code_mode_turn_with_rmcp_config(
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -629,6 +629,7 @@ async fn js_repl_can_invoke_mcp_tools_by_display_name() -> Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -151,6 +151,7 @@ fn insert_mcp_server(
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: options.tool_timeout_sec,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -703,6 +703,7 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: Some(vec!["echo".to_string(), "image".to_string()]),
|
||||
disabled_tools: Some(vec!["image".to_string()]),
|
||||
scopes: None,
|
||||
|
||||
@@ -378,6 +378,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result<
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -162,6 +162,7 @@ async fn historical_unavailable_mcp_call_is_exposed_as_placeholder_tool() -> Res
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
@@ -382,6 +382,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()>
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(std::time::Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -480,6 +481,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
@@ -761,6 +763,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> {
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(std::time::Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
default_tools_approval_mode: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
|
||||
+9
-3
@@ -28,12 +28,18 @@ resources, review those read/write race conditions before enabling this setting.
|
||||
|
||||
## MCP tool approvals
|
||||
|
||||
Codex stores per-tool approval overrides for custom MCP servers under
|
||||
`mcp_servers` in `~/.codex/config.toml`:
|
||||
Codex stores approval defaults and per-tool overrides for custom MCP servers
|
||||
under `mcp_servers` in `~/.codex/config.toml`. Set
|
||||
`default_tools_approval_mode` on the server to apply a default to every tool,
|
||||
and use per-tool `approval_mode` entries for exceptions:
|
||||
|
||||
```toml
|
||||
[mcp_servers.docs]
|
||||
command = "docs-server"
|
||||
default_tools_approval_mode = "approve"
|
||||
|
||||
[mcp_servers.docs.tools.search]
|
||||
approval_mode = "approve"
|
||||
approval_mode = "prompt"
|
||||
```
|
||||
|
||||
## Apps (Connectors)
|
||||
|
||||
Reference in New Issue
Block a user