From 44b4b913bf5e6576158fb00967971defa448d912 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 3 Mar 2026 16:10:15 -0800 Subject: [PATCH] config: support enterprise feature requirements --- .../codex_app_server_protocol.schemas.json | 9 + .../v2/ConfigRequirementsReadResponse.json | 9 + .../typescript/v2/ConfigRequirements.ts | 2 +- .../app-server-protocol/src/protocol/v2.rs | 2 + codex-rs/app-server/README.md | 2 +- codex-rs/app-server/src/config_api.rs | 17 + codex-rs/cloud-requirements/src/lib.rs | 12 + codex-rs/config/src/config_requirements.rs | 75 ++++ codex-rs/config/src/lib.rs | 1 + codex-rs/core/src/codex.rs | 10 +- codex-rs/core/src/config/managed_features.rs | 338 ++++++++++++++++++ codex-rs/core/src/config/mod.rs | 156 +++++++- codex-rs/core/src/config/service.rs | 135 +++++++ codex-rs/core/src/config_loader/mod.rs | 1 + codex-rs/core/src/config_loader/tests.rs | 25 ++ codex-rs/core/src/features.rs | 29 +- codex-rs/core/tests/common/test_codex.rs | 68 +++- codex-rs/core/tests/suite/search_tool.rs | 7 +- codex-rs/tui/src/debug_config.rs | 2 + 19 files changed, 872 insertions(+), 28 deletions(-) create mode 100644 codex-rs/core/src/config/managed_features.rs diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index a60e6c4a44..4c5b982e4c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -8987,6 +8987,15 @@ "type": "null" } ] + }, + "featureRequirements": { + "additionalProperties": { + "type": "boolean" + }, + "type": [ + "object", + "null" + ] } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json index 651e035e9c..c4a06943aa 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json @@ -81,6 +81,15 @@ "type": "null" } ] + }, + "featureRequirements": { + "additionalProperties": { + "type": "boolean" + }, + "type": [ + "object", + "null" + ] } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts index f99c880697..47a99453fe 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigRequirements.ts @@ -6,4 +6,4 @@ import type { AskForApproval } from "./AskForApproval"; import type { ResidencyRequirement } from "./ResidencyRequirement"; import type { SandboxMode } from "./SandboxMode"; -export type ConfigRequirements = {allowedApprovalPolicies: Array | null, allowedSandboxModes: Array | null, allowedWebSearchModes: Array | null, enforceResidency: ResidencyRequirement | null}; +export type ConfigRequirements = {allowedApprovalPolicies: Array | null, allowedSandboxModes: Array | null, allowedWebSearchModes: Array | null, featureRequirements: { [key in string]?: boolean } | null, enforceResidency: ResidencyRequirement | null}; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 6b26433ae0..7d47a67d94 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeMap; use std::collections::HashMap; use std::path::PathBuf; @@ -611,6 +612,7 @@ pub struct ConfigRequirements { pub allowed_approval_policies: Option>, pub allowed_sandbox_modes: Option>, pub allowed_web_search_modes: Option>, + pub feature_requirements: Option>, pub enforce_residency: Option, #[experimental("configRequirements/read.network")] pub network: Option, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index d2d8aa5445..6b1751bf4d 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -164,7 +164,7 @@ Example with notification opt-out: - `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home). - `config/value/write` — write a single config key/value to the user's config.toml on disk. - `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk. -- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), `enforceResidency`, and `network` constraints. +- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), pinned feature values (`featureRequirements`), `enforceResidency`, and `network` constraints. ### Example: Start or resume a thread diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index c9317ac9f5..4b0b66ccab 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -127,6 +127,9 @@ fn map_requirements_toml_to_api(requirements: ConfigRequirementsToml) -> ConfigR } normalized }), + feature_requirements: requirements + .feature_requirements + .map(|requirements| requirements.entries), enforce_residency: requirements .enforce_residency .map(map_residency_requirement_to_api), @@ -212,6 +215,12 @@ mod tests { allowed_web_search_modes: Some(vec![ codex_core::config_loader::WebSearchModeRequirement::Cached, ]), + feature_requirements: Some(codex_core::config_loader::FeatureRequirementsToml { + entries: std::collections::BTreeMap::from([ + ("apps".to_string(), false), + ("personality".to_string(), true), + ]), + }), mcp_servers: None, rules: None, enforce_residency: Some(CoreResidencyRequirement::Us), @@ -247,6 +256,13 @@ mod tests { mapped.allowed_web_search_modes, Some(vec![WebSearchMode::Cached, WebSearchMode::Disabled]), ); + assert_eq!( + mapped.feature_requirements, + Some(std::collections::BTreeMap::from([ + ("apps".to_string(), false), + ("personality".to_string(), true), + ])), + ); assert_eq!( mapped.enforce_residency, Some(codex_app_server_protocol::ResidencyRequirement::Us), @@ -275,6 +291,7 @@ mod tests { allowed_approval_policies: None, allowed_sandbox_modes: None, allowed_web_search_modes: Some(Vec::new()), + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index 2b51a9e9eb..9e4a8b002d 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -761,6 +761,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -803,6 +804,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -856,6 +858,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -912,6 +915,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -939,6 +943,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -986,6 +991,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -1032,6 +1038,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -1082,6 +1089,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -1133,6 +1141,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -1184,6 +1193,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -1272,6 +1282,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -1295,6 +1306,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index d895ef0736..3544254232 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -79,6 +79,7 @@ pub struct ConfigRequirements { pub approval_policy: ConstrainedWithSource, pub sandbox_policy: ConstrainedWithSource, pub web_search_mode: ConstrainedWithSource, + pub feature_requirements: Option>, pub mcp_servers: Option>>, pub exec_policy: Option>, pub enforce_residency: ConstrainedWithSource>, @@ -101,6 +102,7 @@ impl Default for ConfigRequirements { Constrained::allow_any(WebSearchMode::Cached), None, ), + feature_requirements: None, mcp_servers: None, exec_policy: None, enforce_residency: ConstrainedWithSource::new(Constrained::allow_any(None), None), @@ -227,12 +229,25 @@ impl fmt::Display for WebSearchModeRequirement { } } +#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] +pub struct FeatureRequirementsToml { + #[serde(flatten)] + pub entries: BTreeMap, +} + +impl FeatureRequirementsToml { + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } +} + /// Base config deserialized from system `requirements.toml` or MDM. #[derive(Deserialize, Debug, Clone, Default, PartialEq)] pub struct ConfigRequirementsToml { pub allowed_approval_policies: Option>, pub allowed_sandbox_modes: Option>, pub allowed_web_search_modes: Option>, + pub feature_requirements: Option, pub mcp_servers: Option>, pub rules: Option, pub enforce_residency: Option, @@ -267,6 +282,7 @@ pub struct ConfigRequirementsWithSources { pub allowed_approval_policies: Option>>, pub allowed_sandbox_modes: Option>>, pub allowed_web_search_modes: Option>>, + pub feature_requirements: Option>, pub mcp_servers: Option>>, pub rules: Option>, pub enforce_residency: Option>, @@ -302,6 +318,7 @@ impl ConfigRequirementsWithSources { allowed_approval_policies, allowed_sandbox_modes, allowed_web_search_modes, + feature_requirements, mcp_servers, rules, enforce_residency, @@ -315,6 +332,7 @@ impl ConfigRequirementsWithSources { allowed_approval_policies, allowed_sandbox_modes, allowed_web_search_modes, + feature_requirements, mcp_servers, rules, enforce_residency, @@ -324,6 +342,7 @@ impl ConfigRequirementsWithSources { allowed_approval_policies: allowed_approval_policies.map(|sourced| sourced.value), allowed_sandbox_modes: allowed_sandbox_modes.map(|sourced| sourced.value), allowed_web_search_modes: allowed_web_search_modes.map(|sourced| sourced.value), + feature_requirements: feature_requirements.map(|sourced| sourced.value), mcp_servers: mcp_servers.map(|sourced| sourced.value), rules: rules.map(|sourced| sourced.value), enforce_residency: enforce_residency.map(|sourced| sourced.value), @@ -370,6 +389,10 @@ impl ConfigRequirementsToml { self.allowed_approval_policies.is_none() && self.allowed_sandbox_modes.is_none() && self.allowed_web_search_modes.is_none() + && self + .feature_requirements + .as_ref() + .is_none_or(FeatureRequirementsToml::is_empty) && self.mcp_servers.is_none() && self.rules.is_none() && self.enforce_residency.is_none() @@ -385,6 +408,7 @@ impl TryFrom for ConfigRequirements { allowed_approval_policies, allowed_sandbox_modes, allowed_web_search_modes, + feature_requirements, mcp_servers, rules, enforce_residency, @@ -521,6 +545,8 @@ impl TryFrom for ConfigRequirements { } None => ConstrainedWithSource::new(Constrained::allow_any(WebSearchMode::Cached), None), }; + let feature_requirements = + feature_requirements.filter(|requirements| !requirements.value.is_empty()); let enforce_residency = match enforce_residency { Some(Sourced { @@ -553,6 +579,7 @@ impl TryFrom for ConfigRequirements { approval_policy, sandbox_policy, web_search_mode, + feature_requirements, mcp_servers, exec_policy, enforce_residency, @@ -588,6 +615,7 @@ mod tests { allowed_approval_policies, allowed_sandbox_modes, allowed_web_search_modes, + feature_requirements, mcp_servers, rules, enforce_residency, @@ -600,6 +628,8 @@ mod tests { .map(|value| Sourced::new(value, RequirementSource::Unknown)), allowed_web_search_modes: allowed_web_search_modes .map(|value| Sourced::new(value, RequirementSource::Unknown)), + feature_requirements: feature_requirements + .map(|value| Sourced::new(value, RequirementSource::Unknown)), mcp_servers: mcp_servers.map(|value| Sourced::new(value, RequirementSource::Unknown)), rules: rules.map(|value| Sourced::new(value, RequirementSource::Unknown)), enforce_residency: enforce_residency @@ -622,6 +652,9 @@ mod tests { WebSearchModeRequirement::Cached, WebSearchModeRequirement::Live, ]; + let feature_requirements = FeatureRequirementsToml { + entries: BTreeMap::from([("personality".to_string(), true)]), + }; let enforce_residency = ResidencyRequirement::Us; let enforce_source = source.clone(); @@ -631,6 +664,7 @@ mod tests { allowed_approval_policies: Some(allowed_approval_policies.clone()), allowed_sandbox_modes: Some(allowed_sandbox_modes.clone()), allowed_web_search_modes: Some(allowed_web_search_modes.clone()), + feature_requirements: Some(feature_requirements.clone()), mcp_servers: None, rules: None, enforce_residency: Some(enforce_residency), @@ -651,6 +685,10 @@ mod tests { allowed_web_search_modes, enforce_source.clone(), )), + feature_requirements: Some(Sourced::new( + feature_requirements, + enforce_source.clone(), + )), mcp_servers: None, rules: None, enforce_residency: Some(Sourced::new(enforce_residency, enforce_source)), @@ -683,6 +721,7 @@ mod tests { )), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -723,6 +762,7 @@ mod tests { )), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -809,6 +849,8 @@ mod tests { allowed_sandbox_modes = ["read-only"] allowed_web_search_modes = ["cached"] enforce_residency = "us" + [feature_requirements] + personality = true "#, )?; @@ -829,6 +871,13 @@ mod tests { requirements.web_search_mode.source, Some(source_location.clone()) ); + assert_eq!( + requirements + .feature_requirements + .as_ref() + .map(|requirements| requirements.source.clone()), + Some(source_location.clone()) + ); assert_eq!(requirements.enforce_residency.source, Some(source_location)); Ok(()) @@ -1038,6 +1087,32 @@ mod tests { Ok(()) } + #[test] + fn deserialize_feature_requirements() -> Result<()> { + let toml_str = r#" + [feature_requirements] + apps = false + personality = true + "#; + let config: ConfigRequirementsToml = from_str(toml_str)?; + let requirements: ConfigRequirements = with_unknown_source(config).try_into()?; + + assert_eq!( + requirements.feature_requirements, + Some(Sourced::new( + FeatureRequirementsToml { + entries: BTreeMap::from([ + ("apps".to_string(), false), + ("personality".to_string(), true), + ]), + }, + RequirementSource::Unknown, + )) + ); + + Ok(()) + } + #[test] fn network_requirements_are_preserved_as_constraints_with_source() -> Result<()> { let toml_str = r#" diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index b85e99133d..c8e25e6e60 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -16,6 +16,7 @@ pub use config_requirements::ConfigRequirements; pub use config_requirements::ConfigRequirementsToml; pub use config_requirements::ConfigRequirementsWithSources; pub use config_requirements::ConstrainedWithSource; +pub use config_requirements::FeatureRequirementsToml; pub use config_requirements::McpServerIdentity; pub use config_requirements::McpServerRequirement; pub use config_requirements::NetworkConstraints; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7736aa1280..7d2938085e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -23,11 +23,11 @@ use crate::compact::InitialContextInjection; use crate::compact::run_inline_auto_compact_task; use crate::compact::should_use_remote_compact_task; use crate::compact_remote::run_inline_remote_auto_compact_task; +use crate::config::ManagedFeatures; use crate::connectors; use crate::exec_policy::ExecPolicyManager; use crate::features::FEATURES; use crate::features::Feature; -use crate::features::Features; use crate::features::maybe_push_unstable_features_warning; #[cfg(test)] use crate::models_manager::collaboration_mode_presets::CollaborationModesConfig; @@ -611,7 +611,7 @@ pub(crate) struct Session { state: Mutex, /// The set of enabled features should be invariant for the lifetime of the /// session. - features: Features, + features: ManagedFeatures, pending_mcp_server_refresh_config: Mutex>, pub(crate) conversation: Arc, pub(crate) active_turn: Mutex>, @@ -665,7 +665,7 @@ pub(crate) struct TurnContext { pub(crate) windows_sandbox_level: WindowsSandboxLevel, pub(crate) shell_environment_policy: ShellEnvironmentPolicy, pub(crate) tools_config: ToolsConfig, - pub(crate) features: Features, + pub(crate) features: ManagedFeatures, pub(crate) ghost_snapshot: GhostSnapshotConfig, pub(crate) final_output_json_schema: Option, pub(crate) codex_linux_sandbox_exe: Option, @@ -3011,7 +3011,7 @@ impl Session { self.features.enabled(feature) } - pub(crate) fn features(&self) -> Features { + pub(crate) fn features(&self) -> ManagedFeatures { self.features.clone() } @@ -8643,7 +8643,7 @@ mod tests { #[tokio::test] async fn record_model_warning_appends_user_message() { let (mut session, turn_context) = make_session_and_context().await; - let features = Features::with_defaults(); + let features = crate::features::Features::with_defaults().into(); session.features = features; session diff --git a/codex-rs/core/src/config/managed_features.rs b/codex-rs/core/src/config/managed_features.rs new file mode 100644 index 0000000000..7d0961c1d6 --- /dev/null +++ b/codex-rs/core/src/config/managed_features.rs @@ -0,0 +1,338 @@ +use std::collections::BTreeMap; + +use codex_config::Constrained; +use codex_config::ConstrainedWithSource; +use codex_config::ConstraintError; +use codex_config::FeatureRequirementsToml; +use codex_config::RequirementSource; +use codex_config::Sourced; + +use crate::config::ConfigToml; +use crate::config::profile::ConfigProfile; +use crate::features::Feature; +use crate::features::FeatureOverrides; +use crate::features::Features; +use crate::features::canonical_feature_for_key; +use crate::features::feature_for_key; + +#[derive(Debug, Clone, PartialEq)] +pub struct ManagedFeatures { + value: ConstrainedWithSource, +} + +impl ManagedFeatures { + fn new(value: ConstrainedWithSource) -> Self { + Self { value } + } + + pub(crate) fn from_configured( + configured_features: Features, + feature_requirements: Option>, + ) -> std::io::Result { + let (pinned_features, source) = match feature_requirements { + Some(Sourced { + value: feature_requirements, + source, + }) => ( + parse_feature_requirements(feature_requirements, &source)?, + Some(source), + ), + None => (BTreeMap::new(), None), + }; + + let pinned_features_for_normalizer = pinned_features.clone(); + let constrained = Constrained::normalized(configured_features, move |mut candidate| { + for (feature, enabled) in &pinned_features_for_normalizer { + candidate.set_enabled(*feature, *enabled); + } + candidate.normalize_dependencies(); + candidate + }) + .map_err(std::io::Error::from)?; + let managed_features = Self::new(ConstrainedWithSource::new(constrained, source.clone())); + validate_pinned_features(&managed_features, &pinned_features, source.as_ref())?; + Ok(managed_features) + } + + pub fn to_features(&self) -> Features { + self.value.get().clone() + } + + fn update(&mut self, update: impl FnOnce(&mut Features)) { + let mut next = self.to_features(); + update(&mut next); + if let Err(err) = self.value.set(next) { + panic!("managed feature constraints should remain satisfiable: {err}"); + } + } + + pub fn set_enabled(&mut self, feature: Feature, enabled: bool) -> &mut Self { + self.update(|features| { + features.set_enabled(feature, enabled); + }); + self + } + + pub fn enable(&mut self, feature: Feature) -> &mut Self { + self.set_enabled(feature, true) + } + + pub fn disable(&mut self, feature: Feature) -> &mut Self { + self.set_enabled(feature, false) + } + + pub fn apply_map(&mut self, entries: &BTreeMap) -> &mut Self { + self.update(|features| { + features.apply_map(entries); + }); + self + } + + pub fn record_legacy_usage_force(&mut self, alias: &str, feature: Feature) -> &mut Self { + self.update(|features| { + features.record_legacy_usage_force(alias, feature); + }); + self + } + + pub fn record_legacy_usage(&mut self, alias: &str, feature: Feature) -> &mut Self { + self.update(|features| { + features.record_legacy_usage(alias, feature); + }); + self + } +} + +impl From for ManagedFeatures { + fn from(features: Features) -> Self { + match Self::from_configured(features, None) { + Ok(features) => features, + Err(err) => panic!("unconstrained features should always be constructible: {err}"), + } + } +} + +impl std::ops::Deref for ManagedFeatures { + type Target = Features; + + fn deref(&self) -> &Self::Target { + self.value.get() + } +} + +fn feature_requirements_display(feature_requirements: &BTreeMap) -> String { + let values = feature_requirements + .iter() + .map(|(feature, enabled)| format!("{}={enabled}", feature.key())) + .collect::>(); + format!("[{}]", values.join(", ")) +} + +fn parse_feature_requirements( + feature_requirements: FeatureRequirementsToml, + source: &RequirementSource, +) -> std::io::Result> { + let mut pinned_features = BTreeMap::new(); + for (key, enabled) in feature_requirements.entries { + if let Some(feature) = canonical_feature_for_key(&key) { + pinned_features.insert(feature, enabled); + continue; + } + + if let Some(feature) = feature_for_key(&key) { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!( + "invalid `feature_requirements` entry `{key}` from {source}: use canonical feature key `{}`", + feature.key() + ), + )); + } + + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("invalid `feature_requirements` entry `{key}` from {source}"), + )); + } + + Ok(pinned_features) +} + +fn validate_pinned_features( + normalized_features: &Features, + pinned_features: &BTreeMap, + source: Option<&RequirementSource>, +) -> std::io::Result<()> { + let Some(source) = source else { + return Ok(()); + }; + let allowed = feature_requirements_display(pinned_features); + for (feature, enabled) in pinned_features { + if normalized_features.enabled(*feature) != *enabled { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + ConstraintError::InvalidValue { + field_name: "features", + candidate: format!( + "{}={}", + feature.key(), + normalized_features.enabled(*feature) + ), + allowed, + requirement_source: source.clone(), + }, + )); + } + } + + Ok(()) +} + +fn explicit_feature_settings_in_config(cfg: &ConfigToml) -> Vec<(String, Feature, bool)> { + let mut explicit_settings = Vec::new(); + + if let Some(features) = cfg.features.as_ref() { + for (key, enabled) in &features.entries { + if let Some(feature) = feature_for_key(key) { + explicit_settings.push((format!("features.{key}"), feature, *enabled)); + } + } + } + if let Some(enabled) = cfg.experimental_use_unified_exec_tool { + explicit_settings.push(( + "experimental_use_unified_exec_tool".to_string(), + Feature::UnifiedExec, + enabled, + )); + } + if let Some(enabled) = cfg.experimental_use_freeform_apply_patch { + explicit_settings.push(( + "experimental_use_freeform_apply_patch".to_string(), + Feature::ApplyPatchFreeform, + enabled, + )); + } + if let Some(enabled) = cfg.tools.as_ref().and_then(|tools| tools.web_search) { + explicit_settings.push(( + "tools.web_search".to_string(), + Feature::WebSearchRequest, + enabled, + )); + } + + for (profile_name, profile) in &cfg.profiles { + if let Some(features) = profile.features.as_ref() { + for (key, enabled) in &features.entries { + if let Some(feature) = feature_for_key(key) { + explicit_settings.push(( + format!("profiles.{profile_name}.features.{key}"), + feature, + *enabled, + )); + } + } + } + if let Some(enabled) = profile.include_apply_patch_tool { + explicit_settings.push(( + format!("profiles.{profile_name}.include_apply_patch_tool"), + Feature::ApplyPatchFreeform, + enabled, + )); + } + if let Some(enabled) = profile.experimental_use_unified_exec_tool { + explicit_settings.push(( + format!("profiles.{profile_name}.experimental_use_unified_exec_tool"), + Feature::UnifiedExec, + enabled, + )); + } + if let Some(enabled) = profile.experimental_use_freeform_apply_patch { + explicit_settings.push(( + format!("profiles.{profile_name}.experimental_use_freeform_apply_patch"), + Feature::ApplyPatchFreeform, + enabled, + )); + } + if let Some(enabled) = profile.tools_web_search { + explicit_settings.push(( + format!("profiles.{profile_name}.tools_web_search"), + Feature::WebSearchRequest, + enabled, + )); + } + } + + explicit_settings +} + +pub(crate) fn validate_explicit_feature_settings_in_config_toml( + cfg: &ConfigToml, + feature_requirements: Option<&Sourced>, +) -> std::io::Result<()> { + let Some(Sourced { + value: feature_requirements, + source, + }) = feature_requirements + else { + return Ok(()); + }; + + let pinned_features = parse_feature_requirements(feature_requirements.clone(), source)?; + if pinned_features.is_empty() { + return Ok(()); + } + + let allowed = feature_requirements_display(&pinned_features); + for (path, feature, enabled) in explicit_feature_settings_in_config(cfg) { + if pinned_features + .get(&feature) + .is_some_and(|required| *required != enabled) + { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + ConstraintError::InvalidValue { + field_name: "features", + candidate: format!("{path}={enabled}"), + allowed, + requirement_source: source.clone(), + }, + )); + } + } + + Ok(()) +} + +pub(crate) fn validate_feature_requirements_in_config_toml( + cfg: &ConfigToml, + feature_requirements: Option<&Sourced>, +) -> std::io::Result<()> { + fn validate_profile( + cfg: &ConfigToml, + profile_name: Option<&str>, + profile: &ConfigProfile, + feature_requirements: Option<&Sourced>, + ) -> std::io::Result<()> { + let configured_features = Features::from_config(cfg, profile, FeatureOverrides::default()); + ManagedFeatures::from_configured(configured_features, feature_requirements.cloned()) + .map(|_| ()) + .map_err(|err| { + if let Some(profile_name) = profile_name { + std::io::Error::new( + err.kind(), + format!( + "invalid feature configuration for profile `{profile_name}`: {err}" + ), + ) + } else { + err + } + }) + } + + validate_profile(cfg, None, &ConfigProfile::default(), feature_requirements)?; + for (profile_name, profile) in &cfg.profiles { + validate_profile(cfg, Some(profile_name), profile, feature_requirements)?; + } + Ok(()) +} diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index bb313f25f3..ebd66bdfe8 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -91,6 +91,7 @@ use toml::Value as TomlValue; use toml_edit::DocumentMut; pub mod edit; +mod managed_features; mod network_proxy_spec; mod permissions; pub mod profile; @@ -102,6 +103,7 @@ pub use codex_config::ConstraintError; pub use codex_config::ConstraintResult; pub use codex_network_proxy::NetworkProxyAuditMetadata; +pub use managed_features::ManagedFeatures; pub use network_proxy_spec::NetworkProxySpec; pub use network_proxy_spec::StartedNetworkProxy; pub use permissions::NetworkToml; @@ -475,7 +477,7 @@ pub struct Config { pub ghost_snapshot: GhostSnapshotConfig, /// Centralized feature flags; source of truth for feature gating. - pub features: Features, + pub features: ManagedFeatures, /// When `true`, suppress warnings about unstable (under development) features. pub suppress_unstable_features_warning: bool, @@ -1739,7 +1741,11 @@ impl Config { web_search_request: override_tools_web_search_request, }; - let features = Features::from_config(&cfg, &config_profile, feature_overrides); + let configured_features = Features::from_config(&cfg, &config_profile, feature_overrides); + let features = ManagedFeatures::from_configured( + configured_features, + requirements.feature_requirements.clone(), + )?; let windows_sandbox_mode = resolve_windows_sandbox_mode(&cfg, &config_profile); let resolved_cwd = { use std::env; @@ -2058,6 +2064,7 @@ impl Config { approval_policy: mut constrained_approval_policy, sandbox_policy: mut constrained_sandbox_policy, web_search_mode: mut constrained_web_search_mode, + feature_requirements: _, mcp_servers, exec_policy: _, enforce_residency, @@ -4967,7 +4974,7 @@ model_verbosity = "high" use_experimental_unified_exec_tool: !cfg!(windows), background_terminal_max_timeout: DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS, ghost_snapshot: GhostSnapshotConfig::default(), - features: Features::with_defaults(), + features: Features::with_defaults().into(), suppress_unstable_features_warning: false, active_profile: Some("o3".to_string()), active_project: ProjectConfig { trust_level: None }, @@ -5097,7 +5104,7 @@ model_verbosity = "high" use_experimental_unified_exec_tool: !cfg!(windows), background_terminal_max_timeout: DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS, ghost_snapshot: GhostSnapshotConfig::default(), - features: Features::with_defaults(), + features: Features::with_defaults().into(), suppress_unstable_features_warning: false, active_profile: Some("gpt3".to_string()), active_project: ProjectConfig { trust_level: None }, @@ -5225,7 +5232,7 @@ model_verbosity = "high" use_experimental_unified_exec_tool: !cfg!(windows), background_terminal_max_timeout: DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS, ghost_snapshot: GhostSnapshotConfig::default(), - features: Features::with_defaults(), + features: Features::with_defaults().into(), suppress_unstable_features_warning: false, active_profile: Some("zdr".to_string()), active_project: ProjectConfig { trust_level: None }, @@ -5339,7 +5346,7 @@ model_verbosity = "high" use_experimental_unified_exec_tool: !cfg!(windows), background_terminal_max_timeout: DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS, ghost_snapshot: GhostSnapshotConfig::default(), - features: Features::with_defaults(), + features: Features::with_defaults().into(), suppress_unstable_features_warning: false, active_profile: Some("gpt5".to_string()), active_project: ProjectConfig { trust_level: None }, @@ -5394,6 +5401,7 @@ model_verbosity = "high" allowed_web_search_modes: Some(vec![ crate::config_loader::WebSearchModeRequirement::Cached, ]), + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -5998,6 +6006,7 @@ mcp_oauth_callback_url = "https://example.com/callback" crate::config_loader::SandboxModeRequirement::ReadOnly, ]), allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -6116,6 +6125,141 @@ trust_level = "untrusted" ); Ok(()) } + + #[tokio::test] + async fn feature_requirements_override_default_feature_values() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .cloud_requirements(CloudRequirementsLoader::new(async { + Ok(Some(crate::config_loader::ConfigRequirementsToml { + feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([ + ("personality".to_string(), true), + ("shell_tool".to_string(), false), + ]), + }), + ..Default::default() + })) + })) + .build() + .await?; + + assert!(config.features.enabled(Feature::Personality)); + assert!(!config.features.enabled(Feature::ShellTool)); + assert!( + !config + .startup_warnings + .iter() + .any(|warning| warning.contains("Configured value for `features`")), + "{:?}", + config.startup_warnings + ); + + Ok(()) + } + + #[tokio::test] + async fn explicit_feature_config_is_normalized_by_requirements() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +[features] +personality = false +shell_tool = true +"#, + )?; + + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .cloud_requirements(CloudRequirementsLoader::new(async { + Ok(Some(crate::config_loader::ConfigRequirementsToml { + feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([ + ("personality".to_string(), true), + ("shell_tool".to_string(), false), + ]), + }), + ..Default::default() + })) + })) + .build() + .await?; + + assert!(config.features.enabled(Feature::Personality)); + assert!(!config.features.enabled(Feature::ShellTool)); + assert!( + !config + .startup_warnings + .iter() + .any(|warning| warning.contains("Configured value for `features`")), + "{:?}", + config.startup_warnings + ); + + Ok(()) + } + + #[tokio::test] + async fn feature_requirements_normalize_runtime_feature_mutations() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + + let mut config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .cloud_requirements(CloudRequirementsLoader::new(async { + Ok(Some(crate::config_loader::ConfigRequirementsToml { + feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([ + ("personality".to_string(), true), + ("shell_tool".to_string(), false), + ]), + }), + ..Default::default() + })) + })) + .build() + .await?; + + config + .features + .disable(Feature::Personality) + .enable(Feature::ShellTool); + + assert!(config.features.enabled(Feature::Personality)); + assert!(!config.features.enabled(Feature::ShellTool)); + + Ok(()) + } + + #[tokio::test] + async fn feature_requirements_reject_legacy_aliases() { + let codex_home = TempDir::new().expect("tempdir"); + + let err = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .cloud_requirements(CloudRequirementsLoader::new(async { + Ok(Some(crate::config_loader::ConfigRequirementsToml { + feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([("collab".to_string(), true)]), + }), + ..Default::default() + })) + })) + .build() + .await + .expect_err("legacy aliases should be rejected"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidData); + assert!( + err.to_string() + .contains("use canonical feature key `multi_agent`"), + "{err}" + ); + } + #[test] fn experimental_realtime_ws_base_url_loads_from_config_toml() -> std::io::Result<()> { let cfg: ConfigToml = toml::from_str( diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index da675d2389..10e0679e57 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -1,6 +1,9 @@ use super::ConfigToml; +use super::deserialize_config_toml_with_base; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; +use crate::config::managed_features::validate_explicit_feature_settings_in_config_toml; +use crate::config::managed_features::validate_feature_requirements_in_config_toml; use crate::config_loader::CloudRequirementsLoader; use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLayerStack; @@ -331,6 +334,35 @@ impl ConfigService { format!("Invalid configuration: {err}"), ) })?; + let user_config_toml = + deserialize_config_toml_with_base(user_config.clone(), &self.codex_home).map_err( + |err| { + ConfigServiceError::write( + ConfigWriteErrorCode::ConfigValidationError, + format!("Invalid configuration: {err}"), + ) + }, + )?; + validate_explicit_feature_settings_in_config_toml( + &user_config_toml, + layers.requirements().feature_requirements.as_ref(), + ) + .map_err(|err| { + ConfigServiceError::write( + ConfigWriteErrorCode::ConfigValidationError, + format!("Invalid configuration: {err}"), + ) + })?; + validate_feature_requirements_in_config_toml( + &user_config_toml, + layers.requirements().feature_requirements.as_ref(), + ) + .map_err(|err| { + ConfigServiceError::write( + ConfigWriteErrorCode::ConfigValidationError, + format!("Invalid configuration: {err}"), + ) + })?; let updated_layers = layers.with_user_config(&provided_path, user_config.clone()); let effective = updated_layers.effective_config(); @@ -706,6 +738,7 @@ mod tests { use codex_app_server_protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + use std::collections::BTreeMap; use tempfile::tempdir; #[test] @@ -1088,6 +1121,108 @@ personality = true assert_eq!(contents.trim(), "model = \"user\""); } + #[tokio::test] + async fn write_value_rejects_feature_requirement_conflict() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); + + let service = ConfigService::new( + tmp.path().to_path_buf(), + vec![], + LoaderOverrides { + managed_config_path: None, + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + macos_managed_config_requirements_base64: None, + }, + CloudRequirementsLoader::new(async { + Ok(Some(ConfigRequirementsToml { + feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([("personality".to_string(), true)]), + }), + ..Default::default() + })) + }), + ); + + let error = service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "features.personality".to_string(), + value: serde_json::json!(false), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect_err("conflicting feature write should fail"); + + assert_eq!( + error.write_error_code(), + Some(ConfigWriteErrorCode::ConfigValidationError) + ); + assert!( + error + .to_string() + .contains("invalid value for `features`: `features.personality=false`"), + "{error}" + ); + assert_eq!( + std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).unwrap(), + "" + ); + } + + #[tokio::test] + async fn write_value_rejects_profile_feature_requirement_conflict() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); + + let service = ConfigService::new( + tmp.path().to_path_buf(), + vec![], + LoaderOverrides { + managed_config_path: None, + #[cfg(target_os = "macos")] + managed_preferences_base64: None, + macos_managed_config_requirements_base64: None, + }, + CloudRequirementsLoader::new(async { + Ok(Some(ConfigRequirementsToml { + feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([("personality".to_string(), true)]), + }), + ..Default::default() + })) + }), + ); + + let error = service + .write_value(ConfigValueWriteParams { + file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), + key_path: "profiles.enterprise.features.personality".to_string(), + value: serde_json::json!(false), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect_err("conflicting profile feature write should fail"); + + assert_eq!( + error.write_error_code(), + Some(ConfigWriteErrorCode::ConfigValidationError) + ); + assert!( + error.to_string().contains( + "invalid value for `features`: `profiles.enterprise.features.personality=false`" + ), + "{error}" + ); + assert_eq!( + std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).unwrap(), + "" + ); + } + #[tokio::test] async fn read_reports_managed_overrides_user_and_session_flags() { let tmp = tempdir().expect("tempdir"); diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 63b3be48e9..b94bf81d08 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -34,6 +34,7 @@ pub use codex_config::ConfigLoadError; pub use codex_config::ConfigRequirements; pub use codex_config::ConfigRequirementsToml; pub use codex_config::ConstrainedWithSource; +pub use codex_config::FeatureRequirementsToml; pub use codex_config::LoaderOverrides; pub use codex_config::McpServerIdentity; pub use codex_config::McpServerRequirement; diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 983acbe8d1..9e19f8e461 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -23,6 +23,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; +use std::collections::BTreeMap; use std::collections::HashMap; use std::path::Path; use tempfile::tempdir; @@ -494,6 +495,9 @@ async fn load_requirements_toml_produces_expected_constraints() -> anyhow::Resul allowed_approval_policies = ["never", "on-request"] allowed_web_search_modes = ["cached"] enforce_residency = "us" + +[feature_requirements] +personality = true "#, ) .await?; @@ -515,6 +519,15 @@ enforce_residency = "us" .cloned(), Some(vec![crate::config_loader::WebSearchModeRequirement::Cached]) ); + assert_eq!( + config_requirements_toml + .feature_requirements + .as_ref() + .map(|requirements| requirements.value.clone()), + Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([("personality".to_string(), true)]), + }) + ); let config_requirements: ConfigRequirements = config_requirements_toml.try_into()?; assert_eq!( config_requirements.approval_policy.value(), @@ -552,6 +565,15 @@ enforce_residency = "us" config_requirements.enforce_residency.value(), Some(crate::config_loader::ResidencyRequirement::Us) ); + assert_eq!( + config_requirements + .feature_requirements + .as_ref() + .map(|requirements| requirements.value.clone()), + Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([("personality".to_string(), true)]), + }) + ); Ok(()) } @@ -581,6 +603,7 @@ allowed_approval_policies = ["on-request"] allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -629,6 +652,7 @@ allowed_approval_policies = ["on-request"] allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, @@ -666,6 +690,7 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_sandbox_modes: None, allowed_web_search_modes: None, + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None, diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 30c38e835f..ef6712fd00 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -245,6 +245,14 @@ impl Features { self } + pub fn set_enabled(&mut self, f: Feature, enabled: bool) -> &mut Self { + if enabled { + self.enable(f) + } else { + self.disable(f) + } + } + pub fn record_legacy_usage_force(&mut self, alias: &str, feature: Feature) { let (summary, details) = legacy_usage_notice(alias, feature); self.legacy_usages.insert(LegacyFeatureUsage { @@ -353,10 +361,7 @@ impl Features { } overrides.apply(&mut features); - if features.enabled(Feature::JsReplToolsOnly) && !features.enabled(Feature::JsRepl) { - tracing::warn!("js_repl_tools_only requires js_repl; disabling js_repl_tools_only"); - features.disable(Feature::JsReplToolsOnly); - } + features.normalize_dependencies(); features } @@ -364,6 +369,13 @@ impl Features { pub fn enabled_features(&self) -> Vec { self.enabled.iter().copied().collect() } + + pub(crate) fn normalize_dependencies(&mut self) { + if self.enabled(Feature::JsReplToolsOnly) && !self.enabled(Feature::JsRepl) { + tracing::warn!("js_repl_tools_only requires js_repl; disabling js_repl_tools_only"); + self.disable(Feature::JsReplToolsOnly); + } + } } fn legacy_usage_notice(alias: &str, feature: Feature) -> (String, Option) { @@ -404,7 +416,7 @@ fn web_search_details() -> &'static str { } /// Keys accepted in `[features]` tables. -fn feature_for_key(key: &str) -> Option { +pub(crate) fn feature_for_key(key: &str) -> Option { for spec in FEATURES { if spec.key == key { return Some(spec.id); @@ -413,6 +425,13 @@ fn feature_for_key(key: &str) -> Option { legacy::feature_for_key(key) } +pub(crate) fn canonical_feature_for_key(key: &str) -> Option { + FEATURES + .iter() + .find(|spec| spec.key == key) + .map(|spec| spec.id) +} + /// Returns `true` if the provided string matches a known feature toggle key. pub fn is_known_feature_key(key: &str) -> bool { feature_for_key(key).is_some() diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 392d4d354f..64de6a14c6 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -11,12 +11,15 @@ use codex_core::ThreadManager; use codex_core::built_in_model_providers; use codex_core::config::Config; use codex_core::features::Feature; +use codex_core::models_manager::collaboration_mode_presets::CollaborationModesConfig; use codex_protocol::config_types::ServiceTier; +use codex_protocol::openai_models::ModelsResponse; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionConfiguredEvent; +use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; use serde_json::Value; use tempfile::TempDir; @@ -28,11 +31,13 @@ use crate::responses::output_value_to_text; use crate::responses::start_mock_server; use crate::streaming_sse::StreamingSseServer; use crate::wait_for_event; +use crate::wait_for_event_match; use wiremock::Match; use wiremock::matchers::path_regex; type ConfigMutator = dyn FnOnce(&mut Config) + Send; type PreBuildHook = dyn FnOnce(&Path) + Send + 'static; +const TEST_MODEL_WITH_EXPERIMENTAL_TOOLS: &str = "test-gpt-5.1-codex"; /// A collection of different ways the model can output an apply_patch call #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -172,11 +177,21 @@ impl TestCodexBuilder { resume_from: Option, ) -> anyhow::Result { let auth = self.auth.clone(); - let thread_manager = codex_core::test_support::thread_manager_with_models_provider_and_home( - auth.clone(), - config.model_provider.clone(), - config.codex_home.clone(), - ); + let thread_manager = if let Some(model_catalog) = config.model_catalog.clone() { + ThreadManager::new( + config.codex_home.clone(), + codex_core::test_support::auth_manager_from_auth(auth.clone()), + SessionSource::Exec, + Some(model_catalog), + CollaborationModesConfig::default(), + ) + } else { + codex_core::test_support::thread_manager_with_models_provider_and_home( + auth.clone(), + config.model_provider.clone(), + config.codex_home.clone(), + ) + }; let thread_manager = Arc::new(thread_manager); let new_conversation = match resume_from { @@ -232,6 +247,7 @@ impl TestCodexBuilder { for mutator in mutators { mutator(&mut config); } + ensure_test_model_catalog(&mut config); if config.include_apply_patch_tool { config.features.enable(Feature::ApplyPatchFreeform); @@ -243,6 +259,38 @@ impl TestCodexBuilder { } } +fn ensure_test_model_catalog(config: &mut Config) { + if config.model.as_deref() != Some(TEST_MODEL_WITH_EXPERIMENTAL_TOOLS) + || config.model_catalog.is_some() + { + return; + } + + let bundled_models_path = + codex_utils_cargo_bin::find_resource!("../models.json").expect("bundled models.json"); + let bundled_models_contents = + std::fs::read_to_string(bundled_models_path).expect("read bundled models.json"); + let bundled_models: ModelsResponse = + serde_json::from_str(&bundled_models_contents).expect("bundled models.json"); + let mut model = bundled_models + .models + .iter() + .find(|candidate| candidate.slug == "gpt-5.1-codex") + .cloned() + .unwrap_or_else(|| panic!("missing bundled model gpt-5.1-codex")); + model.slug = TEST_MODEL_WITH_EXPERIMENTAL_TOOLS.to_string(); + model.display_name = TEST_MODEL_WITH_EXPERIMENTAL_TOOLS.to_string(); + model.experimental_supported_tools = vec![ + "test_sync_tool".to_string(), + "read_file".to_string(), + "grep_files".to_string(), + "list_dir".to_string(), + ]; + config.model_catalog = Some(ModelsResponse { + models: vec![model], + }); +} + pub struct TestCodex { pub home: Arc, pub cwd: Arc, @@ -334,8 +382,14 @@ impl TestCodex { }) .await?; - wait_for_event(&self.codex, |event| { - matches!(event, EventMsg::TurnComplete(_)) + let turn_id = wait_for_event_match(&self.codex, |event| match event { + EventMsg::TurnStarted(event) => Some(event.turn_id.clone()), + _ => None, + }) + .await; + wait_for_event(&self.codex, |event| match event { + EventMsg::TurnComplete(event) => event.turn_id == turn_id, + _ => false, }) .await; Ok(()) diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index d852c9cfb3..3db889766b 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -22,6 +22,7 @@ use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_once; use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; @@ -181,13 +182,13 @@ async fn search_tool_flag_adds_tool() -> Result<()> { let server = start_mock_server().await; let apps_server = AppsTestServer::mount(&server).await?; - let mock = mount_sse_sequence( + let mock = mount_sse_once( &server, - vec![sse(vec![ + sse(vec![ ev_response_created("resp-1"), ev_assistant_message("msg-1", "done"), ev_completed("resp-1"), - ])], + ]), ) .await; diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index a3e98d85c7..91588fd3d7 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -527,6 +527,7 @@ mod tests { allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), allowed_sandbox_modes: Some(vec![SandboxModeRequirement::ReadOnly]), allowed_web_search_modes: Some(vec![WebSearchModeRequirement::Cached]), + feature_requirements: None, mcp_servers: Some(BTreeMap::from([( "docs".to_string(), McpServerRequirement { @@ -652,6 +653,7 @@ approval_policy = "never" allowed_approval_policies: None, allowed_sandbox_modes: None, allowed_web_search_modes: Some(Vec::new()), + feature_requirements: None, mcp_servers: None, rules: None, enforce_residency: None,