mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Reuse Apps policy evaluation across MCP tool exposure (#27813)
## Summary - move `AppToolPolicyEvaluator` and the Apps config/requirements policy logic from `codex-core` into `codex-connectors` - resolve one immutable policy snapshot per exposure build and reuse it across every Codex Apps MCP tool - keep core as a thin adapter from MCP metadata to connector-owned policy input while preserving the call-time defense-in-depth check ## Why `build_mcp_tool_exposure` evaluates every Codex Apps tool on each sampling request. The old path rebuilt effective Apps configuration for every tool, and the policy implementation lived in the already-large core crate even though it is connector-specific. The connector-owned evaluator keeps the expensive config merge/decode out of the loop and gives core only the effective policy result it needs. ## Performance With the real 557-tool Apps corpus, `build_mcp_tool_exposure` measured 3.74 ms and 3.33 ms after the extraction (3.54 ms mean). The original path measured 807 ms mean, so the final result retains the 99.6% reduction. ## Validation - `cargo check -p codex-connectors -p codex-core` - `just test -p codex-connectors` — 15 passed - `just test -p codex-core --lib connectors` — 35 passed - `just test -p codex-core --lib mcp_tool_exposure` — 5 passed - `just test -p codex-core --lib mcp_tool_call` — 72 passed - `just bazel-lock-update` - `just bazel-lock-check` - `just fix -p codex-connectors` - `just fix -p codex-core` - `just fmt`
This commit is contained in:
committed by
GitHub
Unverified
parent
d7f298fe20
commit
1fbaac1e50
Generated
+1
@@ -2543,6 +2543,7 @@ version = "0.0.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"codex-app-server-protocol",
|
||||
"codex-config",
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
|
||||
@@ -10,6 +10,7 @@ workspace = true
|
||||
[dependencies]
|
||||
anyhow = { workspace = true }
|
||||
codex-app-server-protocol = { workspace = true }
|
||||
codex-config = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
sha1 = { workspace = true }
|
||||
|
||||
@@ -0,0 +1,207 @@
|
||||
use codex_config::AppsRequirementsToml;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_config::types::AppsConfigToml;
|
||||
use serde::Deserialize;
|
||||
|
||||
/// The effective enablement and approval policy for one app tool.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct AppToolPolicy {
|
||||
pub enabled: bool,
|
||||
pub approval: AppToolApproval,
|
||||
}
|
||||
|
||||
impl Default for AppToolPolicy {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Connector-owned metadata used to evaluate one app tool.
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub struct AppToolPolicyInput<'a> {
|
||||
pub connector_id: Option<&'a str>,
|
||||
pub tool_name: &'a str,
|
||||
pub tool_title: Option<&'a str>,
|
||||
pub destructive_hint: Option<bool>,
|
||||
pub open_world_hint: Option<bool>,
|
||||
}
|
||||
|
||||
/// Resolves app tool policy against one immutable config snapshot.
|
||||
///
|
||||
/// Callers should construct one evaluator and reuse it for every tool in the
|
||||
/// same exposure build so config layers are merged and decoded only once.
|
||||
pub struct AppToolPolicyEvaluator<'a> {
|
||||
apps_config: Option<AppsConfigToml>,
|
||||
requirements_apps_config: Option<&'a AppsRequirementsToml>,
|
||||
}
|
||||
|
||||
impl<'a> AppToolPolicyEvaluator<'a> {
|
||||
pub fn new(config_layer_stack: &'a ConfigLayerStack) -> Self {
|
||||
let apps_config = apps_config_from_layer_stack(config_layer_stack);
|
||||
let requirements_apps_config = config_layer_stack.requirements_toml().apps.as_ref();
|
||||
Self::from_parts(apps_config, requirements_apps_config)
|
||||
}
|
||||
|
||||
pub fn policy(&self, input: AppToolPolicyInput<'_>) -> AppToolPolicy {
|
||||
let managed_approval = managed_app_tool_approval(
|
||||
self.requirements_apps_config,
|
||||
input.connector_id,
|
||||
input.tool_name,
|
||||
);
|
||||
app_tool_policy_from_apps_config(self.apps_config.as_ref(), input, managed_approval)
|
||||
}
|
||||
|
||||
fn from_parts(
|
||||
apps_config: Option<AppsConfigToml>,
|
||||
requirements_apps_config: Option<&'a AppsRequirementsToml>,
|
||||
) -> Self {
|
||||
Self {
|
||||
apps_config: effective_apps_config(apps_config, requirements_apps_config),
|
||||
requirements_apps_config,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Reads the merged, unmanaged Apps configuration from a config-layer stack.
|
||||
pub fn apps_config_from_layer_stack(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
) -> Option<AppsConfigToml> {
|
||||
config_layer_stack
|
||||
.effective_config()
|
||||
.as_table()
|
||||
.and_then(|table| table.get("apps"))
|
||||
.cloned()
|
||||
.and_then(|value| AppsConfigToml::deserialize(value).ok())
|
||||
}
|
||||
|
||||
pub fn app_is_enabled(apps_config: &AppsConfigToml, connector_id: Option<&str>) -> bool {
|
||||
let default_enabled = apps_config
|
||||
.default
|
||||
.as_ref()
|
||||
.map(|defaults| defaults.enabled)
|
||||
.unwrap_or(true);
|
||||
|
||||
connector_id
|
||||
.and_then(|connector_id| apps_config.apps.get(connector_id))
|
||||
.map(|app| app.enabled)
|
||||
.unwrap_or(default_enabled)
|
||||
}
|
||||
|
||||
fn effective_apps_config(
|
||||
apps_config: Option<AppsConfigToml>,
|
||||
requirements_apps_config: Option<&AppsRequirementsToml>,
|
||||
) -> Option<AppsConfigToml> {
|
||||
let had_apps_config = apps_config.is_some();
|
||||
let mut apps_config = apps_config.unwrap_or_default();
|
||||
apply_requirements_apps_constraints(&mut apps_config, requirements_apps_config);
|
||||
if had_apps_config || apps_config.default.is_some() || !apps_config.apps.is_empty() {
|
||||
Some(apps_config)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_requirements_apps_constraints(
|
||||
apps_config: &mut AppsConfigToml,
|
||||
requirements_apps_config: Option<&AppsRequirementsToml>,
|
||||
) {
|
||||
let Some(requirements_apps_config) = requirements_apps_config else {
|
||||
return;
|
||||
};
|
||||
|
||||
for (app_id, requirement) in &requirements_apps_config.apps {
|
||||
if requirement.enabled == Some(false) {
|
||||
let app = apps_config.apps.entry(app_id.clone()).or_default();
|
||||
app.enabled = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn managed_app_tool_approval(
|
||||
requirements_apps_config: Option<&AppsRequirementsToml>,
|
||||
connector_id: Option<&str>,
|
||||
tool_name: &str,
|
||||
) -> Option<AppToolApproval> {
|
||||
let connector_id = connector_id?;
|
||||
requirements_apps_config?
|
||||
.apps
|
||||
.get(connector_id)?
|
||||
.tools
|
||||
.as_ref()?
|
||||
.tools
|
||||
.get(tool_name)?
|
||||
.approval_mode
|
||||
}
|
||||
|
||||
fn app_tool_policy_from_apps_config(
|
||||
apps_config: Option<&AppsConfigToml>,
|
||||
input: AppToolPolicyInput<'_>,
|
||||
managed_approval: Option<AppToolApproval>,
|
||||
) -> AppToolPolicy {
|
||||
let Some(apps_config) = apps_config else {
|
||||
return AppToolPolicy {
|
||||
approval: managed_approval.unwrap_or(AppToolApproval::Auto),
|
||||
..Default::default()
|
||||
};
|
||||
};
|
||||
|
||||
let app = input
|
||||
.connector_id
|
||||
.and_then(|connector_id| apps_config.apps.get(connector_id));
|
||||
let tools = app.and_then(|app| app.tools.as_ref());
|
||||
let tool_config = tools.and_then(|tools| {
|
||||
tools
|
||||
.tools
|
||||
.get(input.tool_name)
|
||||
.or_else(|| input.tool_title.and_then(|title| tools.tools.get(title)))
|
||||
});
|
||||
let approval = managed_approval
|
||||
.or_else(|| tool_config.and_then(|tool| tool.approval_mode))
|
||||
.or_else(|| app.and_then(|app| app.default_tools_approval_mode))
|
||||
.unwrap_or(AppToolApproval::Auto);
|
||||
|
||||
if !app_is_enabled(apps_config, input.connector_id) {
|
||||
return AppToolPolicy {
|
||||
enabled: false,
|
||||
approval,
|
||||
};
|
||||
}
|
||||
|
||||
if let Some(enabled) = tool_config.and_then(|tool| tool.enabled) {
|
||||
return AppToolPolicy { enabled, approval };
|
||||
}
|
||||
|
||||
if let Some(enabled) = app.and_then(|app| app.default_tools_enabled) {
|
||||
return AppToolPolicy { enabled, approval };
|
||||
}
|
||||
|
||||
let app_defaults = apps_config.default.as_ref();
|
||||
let destructive_enabled = app
|
||||
.and_then(|app| app.destructive_enabled)
|
||||
.unwrap_or_else(|| {
|
||||
app_defaults
|
||||
.map(|defaults| defaults.destructive_enabled)
|
||||
.unwrap_or(true)
|
||||
});
|
||||
let open_world_enabled = app
|
||||
.and_then(|app| app.open_world_enabled)
|
||||
.unwrap_or_else(|| {
|
||||
app_defaults
|
||||
.map(|defaults| defaults.open_world_enabled)
|
||||
.unwrap_or(true)
|
||||
});
|
||||
let destructive_hint = input.destructive_hint.unwrap_or(true);
|
||||
let open_world_hint = input.open_world_hint.unwrap_or(true);
|
||||
let enabled =
|
||||
(destructive_enabled || !destructive_hint) && (open_world_enabled || !open_world_hint);
|
||||
|
||||
AppToolPolicy { enabled, approval }
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "app_tool_policy_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,724 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
|
||||
use codex_config::AbsolutePathBuf;
|
||||
use codex_config::AppRequirementToml;
|
||||
use codex_config::AppToolRequirementToml;
|
||||
use codex_config::AppToolsRequirementsToml;
|
||||
use codex_config::AppsRequirementsToml;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::TomlValue;
|
||||
use codex_config::types::AppConfig;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_config::types::AppToolConfig;
|
||||
use codex_config::types::AppToolsConfig;
|
||||
use codex_config::types::AppsConfigToml;
|
||||
use codex_config::types::AppsDefaultConfig;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn evaluator_reuses_one_snapshot_across_tools() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
default_tools_enabled: Some(false),
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::from([(
|
||||
"events/create".to_string(),
|
||||
AppToolConfig {
|
||||
enabled: Some(true),
|
||||
approval_mode: Some(AppToolApproval::Prompt),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
let requirements = AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: None,
|
||||
tools: Some(AppToolsRequirementsToml {
|
||||
tools: BTreeMap::from([(
|
||||
"events/create".to_string(),
|
||||
AppToolRequirementToml {
|
||||
approval_mode: Some(AppToolApproval::Approve),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
},
|
||||
)]),
|
||||
};
|
||||
let evaluator = AppToolPolicyEvaluator::from_parts(Some(apps_config), Some(&requirements));
|
||||
|
||||
assert_eq!(
|
||||
[
|
||||
evaluator.policy(input("events/create", /*tool_title*/ None)),
|
||||
evaluator.policy(input("events/list", /*tool_title*/ None)),
|
||||
evaluator.policy(input("calendar_events/create", Some("events/create"))),
|
||||
],
|
||||
[
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
},
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
},
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Prompt,
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_uses_global_defaults_for_destructive_hints() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(defaults(
|
||||
/*enabled*/ true, /*destructive_enabled*/ false,
|
||||
/*open_world_enabled*/ true,
|
||||
)),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(true),
|
||||
/*open_world_hint*/ None,
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_defaults_missing_destructive_hint_to_true() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(defaults(
|
||||
/*enabled*/ true, /*destructive_enabled*/ false,
|
||||
/*open_world_enabled*/ true,
|
||||
)),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
Some(false),
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_defaults_missing_open_world_hint_to_true() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(defaults(
|
||||
/*enabled*/ true, /*destructive_enabled*/ true,
|
||||
/*open_world_enabled*/ false,
|
||||
)),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(false),
|
||||
/*open_world_hint*/ None,
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_enablement_uses_defaults_and_per_app_overrides() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(defaults(
|
||||
/*enabled*/ false, /*destructive_enabled*/ true,
|
||||
/*open_world_enabled*/ true,
|
||||
)),
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
[
|
||||
app_is_enabled(&apps_config, Some("calendar")),
|
||||
app_is_enabled(&apps_config, Some("drive")),
|
||||
app_is_enabled(&apps_config, /*connector_id*/ None),
|
||||
],
|
||||
[true, false, false]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_disable_overrides_enabled_app() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
let requirements = app_enabled_requirement("connector_123123", /*enabled*/ false);
|
||||
|
||||
assert_eq!(
|
||||
policy_from_config_parts(
|
||||
Some(&apps_config),
|
||||
Some(&requirements),
|
||||
Some("connector_123123"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_enable_does_not_override_disabled_app() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppConfig {
|
||||
enabled: false,
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
let requirements = app_enabled_requirement("connector_123123", /*enabled*/ true);
|
||||
|
||||
assert_eq!(
|
||||
policy_from_config_parts(
|
||||
Some(&apps_config),
|
||||
Some(&requirements),
|
||||
Some("connector_123123"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_disable_applies_without_apps_config() {
|
||||
let requirements = app_enabled_requirement("connector_123123", /*enabled*/ false);
|
||||
|
||||
assert_eq!(
|
||||
policy_from_config_parts(
|
||||
/*apps_config*/ None,
|
||||
Some(&requirements),
|
||||
Some("connector_123123"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_honors_default_app_enabled_false() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(defaults(
|
||||
/*enabled*/ false, /*destructive_enabled*/ true,
|
||||
/*open_world_enabled*/ true,
|
||||
)),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_allows_per_app_enable_when_default_is_disabled() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(defaults(
|
||||
/*enabled*/ false, /*destructive_enabled*/ true,
|
||||
/*open_world_enabled*/ true,
|
||||
)),
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
AppToolPolicy::default()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_uses_managed_approval_without_apps_config() {
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
/*apps_config*/ None,
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
Some(AppToolApproval::Approve),
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_approval_uses_raw_tool_name() {
|
||||
let requirements = app_tool_requirements(
|
||||
"connector_123123",
|
||||
"calendar/list_events",
|
||||
AppToolApproval::Approve,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
[
|
||||
policy_from_config_parts(
|
||||
/*apps_config*/ None,
|
||||
Some(&requirements),
|
||||
Some("connector_123123"),
|
||||
"calendar/list_events",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
),
|
||||
policy_from_config_parts(
|
||||
/*apps_config*/ None,
|
||||
Some(&requirements),
|
||||
Some("connector_123123"),
|
||||
"calendar/create_event",
|
||||
Some("calendar/list_events"),
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
),
|
||||
],
|
||||
[
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
},
|
||||
AppToolPolicy::default(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_approval_overrides_user_tool_approval() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::from([(
|
||||
"calendar/list_events".to_string(),
|
||||
AppToolConfig {
|
||||
enabled: None,
|
||||
approval_mode: Some(AppToolApproval::Prompt),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
let requirements = app_tool_requirements(
|
||||
"connector_123123",
|
||||
"calendar/list_events",
|
||||
AppToolApproval::Approve,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy_from_config_parts(
|
||||
Some(&apps_config),
|
||||
Some(&requirements),
|
||||
Some("connector_123123"),
|
||||
"calendar/list_events",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn per_tool_enable_overrides_app_level_hints() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
destructive_enabled: Some(false),
|
||||
open_world_enabled: Some(false),
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::from([(
|
||||
"events/create".to_string(),
|
||||
AppToolConfig {
|
||||
enabled: Some(true),
|
||||
approval_mode: None,
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(true),
|
||||
Some(true),
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
AppToolPolicy::default()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_tools_enable_overrides_app_level_hints() {
|
||||
let mut app = AppConfig {
|
||||
enabled: true,
|
||||
destructive_enabled: Some(false),
|
||||
open_world_enabled: Some(false),
|
||||
default_tools_enabled: Some(true),
|
||||
..Default::default()
|
||||
};
|
||||
let apps_config = |app: AppConfig| AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([("calendar".to_string(), app)]),
|
||||
};
|
||||
|
||||
let enabled_policy = policy_from_apps_config(
|
||||
Some(&apps_config(app.clone())),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(true),
|
||||
Some(true),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
app.destructive_enabled = Some(true);
|
||||
app.open_world_enabled = Some(true);
|
||||
app.default_tools_enabled = Some(false);
|
||||
app.default_tools_approval_mode = Some(AppToolApproval::Approve);
|
||||
let disabled_policy = policy_from_apps_config(
|
||||
Some(&apps_config(app)),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
[enabled_policy, disabled_policy],
|
||||
[
|
||||
AppToolPolicy::default(),
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Approve,
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_uses_default_tools_approval_mode() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
default_tools_approval_mode: Some(AppToolApproval::Prompt),
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::new(),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Prompt,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_matches_tool_title_for_user_config() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
destructive_enabled: Some(false),
|
||||
open_world_enabled: Some(false),
|
||||
default_tools_approval_mode: Some(AppToolApproval::Auto),
|
||||
default_tools_enabled: Some(false),
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::from([(
|
||||
"events/create".to_string(),
|
||||
AppToolConfig {
|
||||
enabled: Some(true),
|
||||
approval_mode: Some(AppToolApproval::Approve),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"calendar_events/create",
|
||||
Some("events/create"),
|
||||
Some(true),
|
||||
Some(true),
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
fn input<'a>(tool_name: &'a str, tool_title: Option<&'a str>) -> AppToolPolicyInput<'a> {
|
||||
AppToolPolicyInput {
|
||||
connector_id: Some("calendar"),
|
||||
tool_name,
|
||||
tool_title,
|
||||
destructive_hint: Some(true),
|
||||
open_world_hint: Some(true),
|
||||
}
|
||||
}
|
||||
|
||||
fn policy_from_apps_config(
|
||||
apps_config: Option<&AppsConfigToml>,
|
||||
connector_id: Option<&str>,
|
||||
tool_name: &str,
|
||||
tool_title: Option<&str>,
|
||||
destructive_hint: Option<bool>,
|
||||
open_world_hint: Option<bool>,
|
||||
managed_approval: Option<AppToolApproval>,
|
||||
) -> AppToolPolicy {
|
||||
let requirements = managed_approval.map(|approval| {
|
||||
app_tool_requirements(
|
||||
connector_id.expect("managed approval requires a connector id"),
|
||||
tool_name,
|
||||
approval,
|
||||
)
|
||||
});
|
||||
policy_from_config_parts(
|
||||
apps_config,
|
||||
requirements.as_ref(),
|
||||
connector_id,
|
||||
tool_name,
|
||||
tool_title,
|
||||
destructive_hint,
|
||||
open_world_hint,
|
||||
)
|
||||
}
|
||||
|
||||
fn policy_from_config_parts(
|
||||
apps_config: Option<&AppsConfigToml>,
|
||||
requirements_apps_config: Option<&AppsRequirementsToml>,
|
||||
connector_id: Option<&str>,
|
||||
tool_name: &str,
|
||||
tool_title: Option<&str>,
|
||||
destructive_hint: Option<bool>,
|
||||
open_world_hint: Option<bool>,
|
||||
) -> AppToolPolicy {
|
||||
let requirements = ConfigRequirementsToml {
|
||||
apps: requirements_apps_config.cloned(),
|
||||
..Default::default()
|
||||
};
|
||||
let config_layer_stack =
|
||||
ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements)
|
||||
.expect("config layer stack");
|
||||
let config_layer_stack = if let Some(apps_config) = apps_config {
|
||||
let mut user_config = TomlValue::Table(Default::default());
|
||||
user_config
|
||||
.as_table_mut()
|
||||
.expect("user config table")
|
||||
.insert(
|
||||
"apps".to_string(),
|
||||
TomlValue::try_from(apps_config).expect("serialize apps config"),
|
||||
);
|
||||
let config_toml_path =
|
||||
AbsolutePathBuf::try_from(std::env::temp_dir().join(CONFIG_TOML_FILE))
|
||||
.expect("absolute config path");
|
||||
config_layer_stack.with_user_config(&config_toml_path, user_config)
|
||||
} else {
|
||||
config_layer_stack
|
||||
};
|
||||
AppToolPolicyEvaluator::new(&config_layer_stack).policy(AppToolPolicyInput {
|
||||
connector_id,
|
||||
tool_name,
|
||||
tool_title,
|
||||
destructive_hint,
|
||||
open_world_hint,
|
||||
})
|
||||
}
|
||||
|
||||
fn app_enabled_requirement(app_id: &str, enabled: bool) -> AppsRequirementsToml {
|
||||
AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
app_id.to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: Some(enabled),
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
}
|
||||
}
|
||||
|
||||
fn app_tool_requirements(
|
||||
app_id: &str,
|
||||
tool_name: &str,
|
||||
approval_mode: AppToolApproval,
|
||||
) -> AppsRequirementsToml {
|
||||
AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
app_id.to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: None,
|
||||
tools: Some(AppToolsRequirementsToml {
|
||||
tools: BTreeMap::from([(
|
||||
tool_name.to_string(),
|
||||
AppToolRequirementToml {
|
||||
approval_mode: Some(approval_mode),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
},
|
||||
)]),
|
||||
}
|
||||
}
|
||||
|
||||
fn defaults(
|
||||
enabled: bool,
|
||||
destructive_enabled: bool,
|
||||
open_world_enabled: bool,
|
||||
) -> AppsDefaultConfig {
|
||||
AppsDefaultConfig {
|
||||
enabled,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled,
|
||||
open_world_enabled,
|
||||
}
|
||||
}
|
||||
@@ -12,11 +12,17 @@ use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
pub mod accessible;
|
||||
mod app_tool_policy;
|
||||
mod directory_cache;
|
||||
pub mod filter;
|
||||
pub mod merge;
|
||||
pub mod metadata;
|
||||
|
||||
pub use app_tool_policy::AppToolPolicy;
|
||||
pub use app_tool_policy::AppToolPolicyEvaluator;
|
||||
pub use app_tool_policy::AppToolPolicyInput;
|
||||
pub use app_tool_policy::app_is_enabled;
|
||||
pub use app_tool_policy::apps_config_from_layer_stack;
|
||||
pub use directory_cache::ConnectorDirectoryCacheContext;
|
||||
|
||||
pub const CONNECTORS_CACHE_TTL: Duration = Duration::from_secs(3600);
|
||||
|
||||
@@ -11,12 +11,12 @@ pub use codex_app_server_protocol::AppInfo;
|
||||
pub use codex_app_server_protocol::AppMetadata;
|
||||
use codex_connectors::ConnectorDirectoryCacheContext;
|
||||
use codex_connectors::ConnectorDirectoryCacheKey;
|
||||
use codex_connectors::app_is_enabled;
|
||||
use codex_connectors::apps_config_from_layer_stack;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_exec_server::ExecServerRuntimePaths;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use serde::Deserialize;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
@@ -25,10 +25,7 @@ use crate::config::Config;
|
||||
use crate::mcp::McpManager;
|
||||
use crate::plugins::list_tool_suggest_discoverable_plugins;
|
||||
use crate::session::INITIAL_SUBMIT_ID;
|
||||
use codex_config::AppsRequirementsToml;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_config::types::AppsConfigToml;
|
||||
use codex_config::types::ToolSuggestDiscoverableType;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_features::Feature;
|
||||
@@ -48,21 +45,6 @@ use codex_mcp::tool_plugin_provenance;
|
||||
|
||||
const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30);
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) struct AppToolPolicy {
|
||||
pub enabled: bool,
|
||||
pub approval: AppToolApproval,
|
||||
}
|
||||
|
||||
impl Default for AppToolPolicy {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, PartialEq, Eq)]
|
||||
struct AccessibleConnectorsCacheKey {
|
||||
chatgpt_base_url: String,
|
||||
@@ -520,7 +502,7 @@ pub(crate) fn accessible_connectors_from_mcp_tools(mcp_tools: &[ToolInfo]) -> Ve
|
||||
}
|
||||
|
||||
pub fn with_app_enabled_state(mut connectors: Vec<AppInfo>, config: &Config) -> Vec<AppInfo> {
|
||||
let user_apps_config = read_user_apps_config(config);
|
||||
let user_apps_config = apps_config_from_layer_stack(&config.config_layer_stack);
|
||||
let requirements_apps_config = config.config_layer_stack.requirements_toml().apps.as_ref();
|
||||
if user_apps_config.is_none() && requirements_apps_config.is_none() {
|
||||
return connectors;
|
||||
@@ -557,51 +539,13 @@ pub fn with_app_plugin_sources(
|
||||
connectors
|
||||
}
|
||||
|
||||
pub(crate) fn app_tool_policy(
|
||||
config: &Config,
|
||||
connector_id: Option<&str>,
|
||||
tool_name: &str,
|
||||
tool_title: Option<&str>,
|
||||
annotations: Option<&ToolAnnotations>,
|
||||
) -> AppToolPolicy {
|
||||
let apps_config = read_apps_config(config);
|
||||
let managed_approval = managed_app_tool_approval(
|
||||
config.config_layer_stack.requirements_toml().apps.as_ref(),
|
||||
connector_id,
|
||||
tool_name,
|
||||
);
|
||||
app_tool_policy_from_apps_config(
|
||||
apps_config.as_ref(),
|
||||
connector_id,
|
||||
tool_name,
|
||||
tool_title,
|
||||
annotations,
|
||||
managed_approval,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn codex_app_tool_is_enabled(config: &Config, tool_info: &ToolInfo) -> bool {
|
||||
if tool_info.server_name != CODEX_APPS_MCP_SERVER_NAME {
|
||||
return true;
|
||||
}
|
||||
|
||||
app_tool_policy(
|
||||
config,
|
||||
tool_info.connector_id.as_deref(),
|
||||
&tool_info.tool.name,
|
||||
tool_info.tool.title.as_deref(),
|
||||
tool_info.tool.annotations.as_ref(),
|
||||
)
|
||||
.enabled
|
||||
}
|
||||
|
||||
pub(crate) fn mcp_approvals_reviewer(
|
||||
config: &Config,
|
||||
server_name: &str,
|
||||
connector_id: Option<&str>,
|
||||
) -> ApprovalsReviewer {
|
||||
let app_reviewer = if server_name == CODEX_APPS_MCP_SERVER_NAME {
|
||||
read_user_apps_config(config).and_then(|apps_config| {
|
||||
apps_config_from_layer_stack(&config.config_layer_stack).and_then(|apps_config| {
|
||||
connector_id
|
||||
.and_then(|connector_id| apps_config.apps.get(connector_id))
|
||||
.and_then(|app| app.approvals_reviewer)
|
||||
@@ -629,146 +573,6 @@ pub(crate) fn mcp_approvals_reviewer(
|
||||
config.approvals_reviewer
|
||||
}
|
||||
|
||||
fn read_apps_config(config: &Config) -> Option<AppsConfigToml> {
|
||||
let apps_config = read_user_apps_config(config);
|
||||
let had_apps_config = apps_config.is_some();
|
||||
let mut apps_config = apps_config.unwrap_or_default();
|
||||
apply_requirements_apps_constraints(
|
||||
&mut apps_config,
|
||||
config.config_layer_stack.requirements_toml().apps.as_ref(),
|
||||
);
|
||||
if had_apps_config || apps_config.default.is_some() || !apps_config.apps.is_empty() {
|
||||
Some(apps_config)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn read_user_apps_config(config: &Config) -> Option<AppsConfigToml> {
|
||||
config
|
||||
.config_layer_stack
|
||||
.effective_config()
|
||||
.as_table()
|
||||
.and_then(|table| table.get("apps"))
|
||||
.cloned()
|
||||
.and_then(|value| AppsConfigToml::deserialize(value).ok())
|
||||
}
|
||||
|
||||
fn apply_requirements_apps_constraints(
|
||||
apps_config: &mut AppsConfigToml,
|
||||
requirements_apps_config: Option<&AppsRequirementsToml>,
|
||||
) {
|
||||
let Some(requirements_apps_config) = requirements_apps_config else {
|
||||
return;
|
||||
};
|
||||
|
||||
for (app_id, requirement) in &requirements_apps_config.apps {
|
||||
if requirement.enabled == Some(false) {
|
||||
let app = apps_config.apps.entry(app_id.clone()).or_default();
|
||||
app.enabled = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn managed_app_tool_approval(
|
||||
requirements_apps_config: Option<&AppsRequirementsToml>,
|
||||
connector_id: Option<&str>,
|
||||
tool_name: &str,
|
||||
) -> Option<AppToolApproval> {
|
||||
let connector_id = connector_id?;
|
||||
requirements_apps_config?
|
||||
.apps
|
||||
.get(connector_id)?
|
||||
.tools
|
||||
.as_ref()?
|
||||
.tools
|
||||
.get(tool_name)?
|
||||
.approval_mode
|
||||
}
|
||||
|
||||
fn app_is_enabled(apps_config: &AppsConfigToml, connector_id: Option<&str>) -> bool {
|
||||
let default_enabled = apps_config
|
||||
.default
|
||||
.as_ref()
|
||||
.map(|defaults| defaults.enabled)
|
||||
.unwrap_or(true);
|
||||
|
||||
connector_id
|
||||
.and_then(|connector_id| apps_config.apps.get(connector_id))
|
||||
.map(|app| app.enabled)
|
||||
.unwrap_or(default_enabled)
|
||||
}
|
||||
|
||||
fn app_tool_policy_from_apps_config(
|
||||
apps_config: Option<&AppsConfigToml>,
|
||||
connector_id: Option<&str>,
|
||||
tool_name: &str,
|
||||
tool_title: Option<&str>,
|
||||
annotations: Option<&ToolAnnotations>,
|
||||
managed_approval: Option<AppToolApproval>,
|
||||
) -> AppToolPolicy {
|
||||
let Some(apps_config) = apps_config else {
|
||||
return AppToolPolicy {
|
||||
approval: managed_approval.unwrap_or(AppToolApproval::Auto),
|
||||
..Default::default()
|
||||
};
|
||||
};
|
||||
|
||||
let app = connector_id.and_then(|connector_id| apps_config.apps.get(connector_id));
|
||||
let tools = app.and_then(|app| app.tools.as_ref());
|
||||
let tool_config = tools.and_then(|tools| {
|
||||
tools
|
||||
.tools
|
||||
.get(tool_name)
|
||||
.or_else(|| tool_title.and_then(|title| tools.tools.get(title)))
|
||||
});
|
||||
let approval = managed_approval
|
||||
.or_else(|| tool_config.and_then(|tool| tool.approval_mode))
|
||||
.or_else(|| app.and_then(|app| app.default_tools_approval_mode))
|
||||
.unwrap_or(AppToolApproval::Auto);
|
||||
|
||||
if !app_is_enabled(apps_config, connector_id) {
|
||||
return AppToolPolicy {
|
||||
enabled: false,
|
||||
approval,
|
||||
};
|
||||
}
|
||||
|
||||
if let Some(enabled) = tool_config.and_then(|tool| tool.enabled) {
|
||||
return AppToolPolicy { enabled, approval };
|
||||
}
|
||||
|
||||
if let Some(enabled) = app.and_then(|app| app.default_tools_enabled) {
|
||||
return AppToolPolicy { enabled, approval };
|
||||
}
|
||||
|
||||
let app_defaults = apps_config.default.as_ref();
|
||||
let destructive_enabled = app
|
||||
.and_then(|app| app.destructive_enabled)
|
||||
.unwrap_or_else(|| {
|
||||
app_defaults
|
||||
.map(|defaults| defaults.destructive_enabled)
|
||||
.unwrap_or(true)
|
||||
});
|
||||
let open_world_enabled = app
|
||||
.and_then(|app| app.open_world_enabled)
|
||||
.unwrap_or_else(|| {
|
||||
app_defaults
|
||||
.map(|defaults| defaults.open_world_enabled)
|
||||
.unwrap_or(true)
|
||||
});
|
||||
let destructive_hint = annotations
|
||||
.and_then(|annotations| annotations.destructive_hint)
|
||||
.unwrap_or(true);
|
||||
let open_world_hint = annotations
|
||||
.and_then(|annotations| annotations.open_world_hint)
|
||||
.unwrap_or(true);
|
||||
let enabled =
|
||||
(destructive_enabled || !destructive_hint) && (open_world_enabled || !open_world_hint);
|
||||
|
||||
AppToolPolicy { enabled, approval }
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "connectors_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -2,18 +2,12 @@ use super::*;
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::ConfigBuilder;
|
||||
use codex_config::AppRequirementToml;
|
||||
use codex_config::AppToolRequirementToml;
|
||||
use codex_config::AppToolsRequirementsToml;
|
||||
use codex_config::AppsRequirementsToml;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::test_support::CloudConfigBundleFixture;
|
||||
use codex_config::types::AppConfig;
|
||||
use codex_config::types::AppToolConfig;
|
||||
use codex_config::types::AppToolsConfig;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_config::types::AppsDefaultConfig;
|
||||
use codex_connectors::merge::plugin_connector_to_app_info;
|
||||
use codex_connectors::metadata::connector_install_url;
|
||||
use codex_connectors::metadata::sanitize_name;
|
||||
@@ -21,26 +15,14 @@ use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::ToolInfo;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::Tool;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
use tempfile::tempdir;
|
||||
|
||||
fn annotations(destructive_hint: Option<bool>, open_world_hint: Option<bool>) -> ToolAnnotations {
|
||||
ToolAnnotations::from_raw(
|
||||
/*title*/ None,
|
||||
/*read_only_hint*/ None,
|
||||
destructive_hint,
|
||||
/*idempotent_hint*/ None,
|
||||
open_world_hint,
|
||||
)
|
||||
}
|
||||
|
||||
fn app(id: &str) -> AppInfo {
|
||||
AppInfo {
|
||||
id: id.to_string(),
|
||||
@@ -264,139 +246,6 @@ fn accessible_connectors_from_mcp_tools_preserves_description() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_uses_global_defaults_for_destructive_hints() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(AppsDefaultConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: false,
|
||||
open_world_enabled: true,
|
||||
}),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(Some(true), /*open_world_hint*/ None)),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_defaults_missing_destructive_hint_to_true() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(AppsDefaultConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: false,
|
||||
open_world_enabled: true,
|
||||
}),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(/*destructive_hint*/ None, Some(false))),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_defaults_missing_open_world_hint_to_true() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(AppsDefaultConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: true,
|
||||
open_world_enabled: false,
|
||||
}),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(Some(false), /*open_world_hint*/ None)),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_is_enabled_uses_default_for_unconfigured_apps() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(AppsDefaultConfig {
|
||||
enabled: false,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: true,
|
||||
open_world_enabled: true,
|
||||
}),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
assert!(!app_is_enabled(&apps_config, Some("calendar")));
|
||||
assert!(!app_is_enabled(&apps_config, /*connector_id*/ None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_is_enabled_prefers_per_app_override_over_default() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(AppsDefaultConfig {
|
||||
enabled: false,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: true,
|
||||
open_world_enabled: true,
|
||||
}),
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: None,
|
||||
open_world_enabled: None,
|
||||
default_tools_approval_mode: None,
|
||||
default_tools_enabled: None,
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
assert!(app_is_enabled(&apps_config, Some("calendar")));
|
||||
assert!(!app_is_enabled(&apps_config, Some("drive")));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_approvals_reviewer_uses_app_then_default_then_global() {
|
||||
for (global, app_default, app, expected_global, expected_default, expected_app) in [
|
||||
@@ -522,247 +371,6 @@ approvals_reviewer = "user"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn requirements_disabled_connector_overrides_enabled_connector() {
|
||||
let mut effective_apps = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
let requirements_apps = AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: Some(false),
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
apply_requirements_apps_constraints(&mut effective_apps, Some(&requirements_apps));
|
||||
|
||||
assert_eq!(
|
||||
effective_apps
|
||||
.apps
|
||||
.get("connector_123123")
|
||||
.map(|app| app.enabled),
|
||||
Some(false)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn requirements_enabled_does_not_override_disabled_connector() {
|
||||
let mut effective_apps = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppConfig {
|
||||
enabled: false,
|
||||
..Default::default()
|
||||
},
|
||||
)]),
|
||||
};
|
||||
let requirements_apps = AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: Some(true),
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
apply_requirements_apps_constraints(&mut effective_apps, Some(&requirements_apps));
|
||||
|
||||
assert_eq!(
|
||||
effective_apps
|
||||
.apps
|
||||
.get("connector_123123")
|
||||
.map(|app| app.enabled),
|
||||
Some(false)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_config_bundle_disable_connector_overrides_user_apps_config() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
[apps.connector_123123]
|
||||
enabled = true
|
||||
"#,
|
||||
)
|
||||
.expect("write config");
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.cloud_config_bundle(
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
r#"
|
||||
[apps.connector_123123]
|
||||
enabled = false
|
||||
"#,
|
||||
),
|
||||
)
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build");
|
||||
|
||||
let policy = app_tool_policy(
|
||||
&config,
|
||||
Some("connector_123123"),
|
||||
"events.list",
|
||||
/*tool_title*/ None,
|
||||
/*annotations*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_config_bundle_disable_connector_applies_without_user_apps_table() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
std::fs::write(codex_home.path().join(CONFIG_TOML_FILE), "").expect("write config");
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.cloud_config_bundle(
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
r#"
|
||||
[apps.connector_123123]
|
||||
enabled = false
|
||||
"#,
|
||||
),
|
||||
)
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build");
|
||||
|
||||
let policy = app_tool_policy(
|
||||
&config,
|
||||
Some("connector_123123"),
|
||||
"events.list",
|
||||
/*tool_title*/ None,
|
||||
/*annotations*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_requirements_disable_connector_overrides_user_apps_config() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
let config_toml_path =
|
||||
AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE)).expect("abs path");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build");
|
||||
|
||||
let requirements = ConfigRequirementsToml {
|
||||
apps: Some(AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: Some(false),
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
config.config_layer_stack =
|
||||
ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements)
|
||||
.expect("requirements stack")
|
||||
.with_user_config(
|
||||
&config_toml_path,
|
||||
toml::from_str::<toml::Value>(
|
||||
r#"
|
||||
[apps.connector_123123]
|
||||
enabled = true
|
||||
"#,
|
||||
)
|
||||
.expect("apps config"),
|
||||
);
|
||||
|
||||
let policy = app_tool_policy(
|
||||
&config,
|
||||
Some("connector_123123"),
|
||||
"events.list",
|
||||
/*tool_title*/ None,
|
||||
/*annotations*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_requirements_disable_connector_applies_without_user_apps_table() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build");
|
||||
|
||||
let requirements = ConfigRequirementsToml {
|
||||
apps: Some(AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: Some(false),
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
config.config_layer_stack =
|
||||
ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements)
|
||||
.expect("requirements stack");
|
||||
|
||||
let policy = app_tool_policy(
|
||||
&config,
|
||||
Some("connector_123123"),
|
||||
"events.list",
|
||||
/*tool_title*/ None,
|
||||
/*annotations*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn with_app_enabled_state_preserves_unrelated_disabled_connector() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
@@ -801,483 +409,6 @@ async fn with_app_enabled_state_preserves_unrelated_disabled_connector() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_honors_default_app_enabled_false() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(AppsDefaultConfig {
|
||||
enabled: false,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: true,
|
||||
open_world_enabled: true,
|
||||
}),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(
|
||||
/*destructive_hint*/ None, /*open_world_hint*/ None,
|
||||
)),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_uses_managed_approval_without_apps_config() {
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
/*apps_config*/ None,
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*annotations*/ None,
|
||||
Some(AppToolApproval::Approve),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
fn app_tool_requirements(
|
||||
app_id: &str,
|
||||
tool_name: &str,
|
||||
approval_mode: AppToolApproval,
|
||||
) -> AppsRequirementsToml {
|
||||
AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
app_id.to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: None,
|
||||
tools: Some(AppToolsRequirementsToml {
|
||||
tools: BTreeMap::from([(
|
||||
tool_name.to_string(),
|
||||
AppToolRequirementToml {
|
||||
approval_mode: Some(approval_mode),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
},
|
||||
)]),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_app_tool_approval_uses_raw_tool_name() {
|
||||
let requirements_apps = app_tool_requirements(
|
||||
"connector_123123",
|
||||
"calendar/list_events",
|
||||
AppToolApproval::Approve,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
managed_app_tool_approval(
|
||||
Some(&requirements_apps),
|
||||
Some("connector_123123"),
|
||||
"calendar/list_events",
|
||||
),
|
||||
Some(AppToolApproval::Approve)
|
||||
);
|
||||
assert_eq!(
|
||||
managed_app_tool_approval(
|
||||
Some(&requirements_apps),
|
||||
Some("connector_123123"),
|
||||
"calendar/create_event",
|
||||
),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_config_bundle_tool_approval_overrides_user_apps_config() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
[apps.connector_123123.tools."calendar/list_events"]
|
||||
approval_mode = "prompt"
|
||||
"#,
|
||||
)
|
||||
.expect("write config");
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.cloud_config_bundle(
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
r#"
|
||||
[apps.connector_123123.tools."calendar/list_events"]
|
||||
approval_mode = "approve"
|
||||
"#,
|
||||
),
|
||||
)
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build");
|
||||
|
||||
let policy = app_tool_policy(
|
||||
&config,
|
||||
Some("connector_123123"),
|
||||
"calendar/list_events",
|
||||
/*tool_title*/ None,
|
||||
/*annotations*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_requirements_tool_approval_overrides_user_apps_config() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
let config_toml_path =
|
||||
AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE)).expect("abs path");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build");
|
||||
|
||||
let requirements = ConfigRequirementsToml {
|
||||
apps: Some(app_tool_requirements(
|
||||
"connector_123123",
|
||||
"calendar/list_events",
|
||||
AppToolApproval::Approve,
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
config.config_layer_stack =
|
||||
ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements)
|
||||
.expect("requirements stack")
|
||||
.with_user_config(
|
||||
&config_toml_path,
|
||||
toml::from_str::<toml::Value>(
|
||||
r#"
|
||||
[apps.connector_123123.tools."calendar/list_events"]
|
||||
approval_mode = "prompt"
|
||||
"#,
|
||||
)
|
||||
.expect("apps config"),
|
||||
);
|
||||
|
||||
let policy = app_tool_policy(
|
||||
&config,
|
||||
Some("connector_123123"),
|
||||
"calendar/list_events",
|
||||
/*tool_title*/ None,
|
||||
/*annotations*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_requirements_tool_approval_does_not_match_tool_title() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build");
|
||||
|
||||
let requirements = ConfigRequirementsToml {
|
||||
apps: Some(app_tool_requirements(
|
||||
"connector_123123",
|
||||
"calendar/list_events",
|
||||
AppToolApproval::Approve,
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
config.config_layer_stack =
|
||||
ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements)
|
||||
.expect("requirements stack");
|
||||
|
||||
let policy = app_tool_policy(
|
||||
&config,
|
||||
Some("connector_123123"),
|
||||
"calendar/create_event",
|
||||
Some("calendar/list_events"),
|
||||
/*annotations*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_allows_per_app_enable_when_default_is_disabled() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(AppsDefaultConfig {
|
||||
enabled: false,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: true,
|
||||
open_world_enabled: true,
|
||||
}),
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: None,
|
||||
open_world_enabled: None,
|
||||
default_tools_approval_mode: None,
|
||||
default_tools_enabled: None,
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(
|
||||
/*destructive_hint*/ None, /*open_world_hint*/ None,
|
||||
)),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_per_tool_enabled_true_overrides_app_level_disable_flags() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: Some(false),
|
||||
open_world_enabled: Some(false),
|
||||
default_tools_approval_mode: None,
|
||||
default_tools_enabled: None,
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::from([(
|
||||
"events/create".to_string(),
|
||||
AppToolConfig {
|
||||
enabled: Some(true),
|
||||
approval_mode: None,
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(Some(true), Some(true))),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_default_tools_enabled_true_overrides_app_level_tool_hints() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: Some(false),
|
||||
open_world_enabled: Some(false),
|
||||
default_tools_approval_mode: None,
|
||||
default_tools_enabled: Some(true),
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/create",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(Some(true), Some(true))),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Auto,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_default_tools_enabled_false_overrides_app_level_tool_hints() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: Some(true),
|
||||
open_world_enabled: Some(true),
|
||||
default_tools_approval_mode: Some(AppToolApproval::Approve),
|
||||
default_tools_enabled: Some(false),
|
||||
tools: None,
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(
|
||||
/*destructive_hint*/ None, /*open_world_hint*/ None,
|
||||
)),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: false,
|
||||
approval: AppToolApproval::Approve,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_uses_default_tools_approval_mode() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: None,
|
||||
open_world_enabled: None,
|
||||
default_tools_approval_mode: Some(AppToolApproval::Prompt),
|
||||
default_tools_enabled: None,
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::new(),
|
||||
}),
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
Some(&annotations(
|
||||
/*destructive_hint*/ None, /*open_world_hint*/ None,
|
||||
)),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Prompt,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_tool_policy_matches_prefix_stripped_tool_name_for_tool_config() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
enabled: true,
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled: Some(false),
|
||||
open_world_enabled: Some(false),
|
||||
default_tools_approval_mode: Some(AppToolApproval::Auto),
|
||||
default_tools_enabled: Some(false),
|
||||
tools: Some(AppToolsConfig {
|
||||
tools: HashMap::from([(
|
||||
"events/create".to_string(),
|
||||
AppToolConfig {
|
||||
enabled: Some(true),
|
||||
approval_mode: Some(AppToolApproval::Approve),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
},
|
||||
)]),
|
||||
};
|
||||
|
||||
let policy = app_tool_policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"calendar_events/create",
|
||||
Some("events/create"),
|
||||
Some(&annotations(Some(true), Some(true))),
|
||||
/*managed_approval*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
policy,
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Approve,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn tool_suggest_connector_ids_include_configured_tool_suggest_discoverables() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
|
||||
@@ -33,6 +33,9 @@ use codex_app_server_protocol::McpServerElicitationRequest;
|
||||
use codex_app_server_protocol::McpServerElicitationRequestParams;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_connectors::AppToolPolicy;
|
||||
use codex_connectors::AppToolPolicyEvaluator;
|
||||
use codex_connectors::AppToolPolicyInput;
|
||||
use codex_features::Feature;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
@@ -148,21 +151,24 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
.and_then(|metadata| metadata.plugin_id.clone()),
|
||||
};
|
||||
let app_tool_policy = if server == CODEX_APPS_MCP_SERVER_NAME {
|
||||
connectors::app_tool_policy(
|
||||
&turn_context.config,
|
||||
metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.connector_id.as_deref()),
|
||||
&tool_name,
|
||||
metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.tool_title.as_deref()),
|
||||
metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.annotations.as_ref()),
|
||||
let annotations = metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.annotations.as_ref());
|
||||
AppToolPolicyEvaluator::new(&turn_context.config.config_layer_stack).policy(
|
||||
AppToolPolicyInput {
|
||||
connector_id: metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.connector_id.as_deref()),
|
||||
tool_name: &tool_name,
|
||||
tool_title: metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.tool_title.as_deref()),
|
||||
destructive_hint: annotations.and_then(|annotations| annotations.destructive_hint),
|
||||
open_world_hint: annotations.and_then(|annotations| annotations.open_world_hint),
|
||||
},
|
||||
)
|
||||
} else {
|
||||
connectors::AppToolPolicy::default()
|
||||
AppToolPolicy::default()
|
||||
};
|
||||
let approval_mode = if server == CODEX_APPS_MCP_SERVER_NAME {
|
||||
app_tool_policy.approval
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use std::collections::HashSet;
|
||||
|
||||
use codex_connectors::AppToolPolicyEvaluator;
|
||||
use codex_connectors::AppToolPolicyInput;
|
||||
use codex_features::Feature;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::ToolInfo as McpToolInfo;
|
||||
@@ -70,6 +72,7 @@ fn filter_codex_apps_mcp_tools(
|
||||
.iter()
|
||||
.map(|connector| connector.id.as_str())
|
||||
.collect();
|
||||
let app_tool_policy = AppToolPolicyEvaluator::new(&config.config_layer_stack);
|
||||
|
||||
mcp_tools
|
||||
.iter()
|
||||
@@ -83,7 +86,19 @@ fn filter_codex_apps_mcp_tools(
|
||||
let Some(connector_id) = tool.connector_id.as_deref() else {
|
||||
return false;
|
||||
};
|
||||
allowed.contains(connector_id) && connectors::codex_app_tool_is_enabled(config, tool)
|
||||
let annotations = tool.tool.annotations.as_ref();
|
||||
allowed.contains(connector_id)
|
||||
&& app_tool_policy
|
||||
.policy(AppToolPolicyInput {
|
||||
connector_id: Some(connector_id),
|
||||
tool_name: &tool.tool.name,
|
||||
tool_title: tool.tool.title.as_deref(),
|
||||
destructive_hint: annotations
|
||||
.and_then(|annotations| annotations.destructive_hint),
|
||||
open_world_hint: annotations
|
||||
.and_then(|annotations| annotations.open_world_hint),
|
||||
})
|
||||
.enabled
|
||||
})
|
||||
.cloned()
|
||||
.collect()
|
||||
|
||||
@@ -11,8 +11,11 @@ use rmcp::model::Meta;
|
||||
use rmcp::model::Tool;
|
||||
|
||||
use super::*;
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::config::test_config;
|
||||
use crate::connectors::AppInfo;
|
||||
use tempfile::tempdir;
|
||||
|
||||
fn make_connector(id: &str, name: &str) -> AppInfo {
|
||||
AppInfo {
|
||||
@@ -182,6 +185,57 @@ async fn excludes_tools_hidden_from_model_exposure() {
|
||||
assert!(exposure.deferred_tools.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn applies_per_tool_app_policy_across_the_exposure_build() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
[apps.calendar]
|
||||
default_tools_enabled = false
|
||||
|
||||
[apps.calendar.tools."events/create"]
|
||||
enabled = true
|
||||
"#,
|
||||
)
|
||||
.expect("write config");
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build");
|
||||
let enabled_tool = make_mcp_tool(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"events/create",
|
||||
"mcp__codex_apps__calendar",
|
||||
"create",
|
||||
Some("calendar"),
|
||||
Some("Calendar"),
|
||||
);
|
||||
let disabled_tool = make_mcp_tool(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"events/list",
|
||||
"mcp__codex_apps__calendar",
|
||||
"list",
|
||||
Some("calendar"),
|
||||
Some("Calendar"),
|
||||
);
|
||||
let connectors = vec![make_connector("calendar", "Calendar")];
|
||||
|
||||
let exposure = build_mcp_tool_exposure(
|
||||
&[enabled_tool.clone(), disabled_tool],
|
||||
Some(connectors.as_slice()),
|
||||
&config,
|
||||
/*search_tool_enabled*/ false,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
tool_names(&exposure.direct_tools),
|
||||
tool_names(&[enabled_tool])
|
||||
);
|
||||
assert!(exposure.deferred_tools.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn searches_large_effective_tool_sets() {
|
||||
let config = test_config().await;
|
||||
|
||||
Reference in New Issue
Block a user