mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Honor project agent opt-ins over lower-priority legacy opt-outs (#59476)
## Why Migrating a project's multi-agent setting to `[agents] enabled = true` should preserve access to agent tools when a lower-priority config still sets `features.multi_agent = false`. ## What changed Allow an explicit project agent opt-in to override a strictly lower-priority legacy feature setting. Preserve legacy feature settings in the same or higher-priority layer, and continue enforcing managed feature requirements. ## Testing Add regression coverage for project migration, V1 and model-selected V2 tools, same-layer and CLI opt-outs, and enterprise requirements that disable multi-agent tools. GitOrigin-RevId: 5810a42d4b7e66cdcbef8dad5281dda4a66291b3
This commit is contained in:
@@ -1,5 +1,7 @@
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::Constrained;
|
||||
use codex_config::ConstrainedWithSource;
|
||||
use codex_config::ConstraintError;
|
||||
@@ -16,6 +18,31 @@ use codex_features::Features;
|
||||
use codex_features::canonical_feature_for_key;
|
||||
use codex_features::feature_for_key;
|
||||
|
||||
/// A project's explicit opt-in replaces only a strictly lower-priority V1 opt-out.
|
||||
pub(super) fn project_overrides_legacy_subagent_opt_out(layers: &ConfigLayerStack) -> bool {
|
||||
let mut project_enabled = false;
|
||||
for layer in layers.layers_high_to_low() {
|
||||
if let Some(features) = layer.config.get("features").and_then(toml::Value::as_table)
|
||||
&& features
|
||||
.keys()
|
||||
.any(|key| feature_for_key(key) == Some(Feature::Collab))
|
||||
{
|
||||
return project_enabled;
|
||||
}
|
||||
if matches!(layer.name, ConfigLayerSource::Project { .. })
|
||||
&& layer
|
||||
.config
|
||||
.get("agents")
|
||||
.and_then(|agents| agents.get("enabled"))
|
||||
.and_then(toml::Value::as_bool)
|
||||
== Some(true)
|
||||
{
|
||||
project_enabled = true;
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Wrapper around [`Features`] which enforces constraints defined in
|
||||
/// `FeatureRequirementsToml` and provides normalization to ensure constraints
|
||||
/// are satisfied. Constraints are enforced on construction and mutation of
|
||||
|
||||
@@ -3576,7 +3576,7 @@ impl Config {
|
||||
web_search_request: override_tools_web_search_request,
|
||||
};
|
||||
|
||||
let configured_features = Features::from_sources(
|
||||
let mut configured_features = Features::from_sources(
|
||||
FeatureConfigSource {
|
||||
features: cfg.features.as_ref(),
|
||||
experimental_use_unified_exec_tool: cfg.experimental_use_unified_exec_tool,
|
||||
@@ -3586,6 +3586,12 @@ impl Config {
|
||||
},
|
||||
feature_overrides,
|
||||
);
|
||||
if cfg.agents.as_ref().and_then(|agents| agents.enabled) == Some(true)
|
||||
&& !configured_features.enabled(Feature::Collab)
|
||||
&& managed_features::project_overrides_legacy_subagent_opt_out(&config_layer_stack)
|
||||
{
|
||||
configured_features.enable(Feature::Collab);
|
||||
}
|
||||
let features = ManagedFeatures::from_configured_with_warnings(
|
||||
configured_features,
|
||||
feature_requirements,
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
use anyhow::Result;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_config::loader::project_trust_key;
|
||||
use codex_config::test_support::CloudConfigBundleFixture;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_models_manager::manager::RefreshStrategy;
|
||||
@@ -30,6 +36,7 @@ use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::Duration;
|
||||
use tokio::time::Instant;
|
||||
use tokio::time::sleep;
|
||||
@@ -407,6 +414,98 @@ async fn multi_agent_config_precedence_overrides_remote_model_selector() -> Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn migrating_only_the_project_agent_setting_preserves_v1_tools() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let home = TempDir::new()?;
|
||||
let project = TempDir::new()?;
|
||||
let project_key = toml::Value::String(project_trust_key(project.path()));
|
||||
std::fs::write(
|
||||
home.path().join(CONFIG_TOML_FILE),
|
||||
format!(
|
||||
"[features]\nmulti_agent = false\n[projects.{project_key}]\ntrust_level = 'trusted'"
|
||||
),
|
||||
)?;
|
||||
let project_dir = project.path().join(".codex");
|
||||
std::fs::create_dir(&project_dir)?;
|
||||
let config = || {
|
||||
ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
cwd: Some(project.path().to_path_buf()),
|
||||
..Default::default()
|
||||
})
|
||||
};
|
||||
let migrated = "[agents]\nenabled = true";
|
||||
for (project_settings, builder, model_version, expected_namespace) in [
|
||||
("", config(), None, None),
|
||||
(
|
||||
"[features]\nmulti_agent = true",
|
||||
config(),
|
||||
None,
|
||||
Some("multi_agent_v1"),
|
||||
),
|
||||
(migrated, config(), None, Some("multi_agent_v1")),
|
||||
(
|
||||
migrated,
|
||||
config(),
|
||||
Some(MultiAgentVersion::V2),
|
||||
Some(MULTI_AGENT_V2_NAMESPACE),
|
||||
),
|
||||
(
|
||||
"[agents]\nenabled = true\n[features]\nmulti_agent = false",
|
||||
config(),
|
||||
None,
|
||||
None,
|
||||
),
|
||||
(
|
||||
migrated,
|
||||
config().cli_overrides(vec![("features.multi_agent".to_string(), false.into())]),
|
||||
None,
|
||||
None,
|
||||
),
|
||||
(
|
||||
migrated,
|
||||
config().cloud_config_bundle(
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
"[features]\nmulti_agent = false",
|
||||
),
|
||||
),
|
||||
None,
|
||||
None,
|
||||
),
|
||||
] {
|
||||
std::fs::write(project_dir.join(CONFIG_TOML_FILE), project_settings)?;
|
||||
let mut loaded = builder.build().await?;
|
||||
let mut model = remote_model("project-agent-selector");
|
||||
model.multi_agent_version = model_version;
|
||||
let body = response_body_for_remote_model(model, move |fixture| {
|
||||
// Use the real layered config with the fixture's model and mock transport.
|
||||
loaded.model = fixture.model.clone();
|
||||
loaded.model_provider = fixture.model_provider.clone();
|
||||
loaded.codex_self_exe = fixture.codex_self_exe.clone();
|
||||
*fixture = loaded;
|
||||
})
|
||||
.await?;
|
||||
let agent_tools: Vec<_> = tool_names(&body)
|
||||
.into_iter()
|
||||
.filter(|name| {
|
||||
matches!(
|
||||
name.as_str(),
|
||||
"multi_agent_v1" | MULTI_AGENT_V2_NAMESPACE | "spawn_agent"
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(
|
||||
agent_tools,
|
||||
expected_namespace.into_iter().collect::<Vec<_>>(),
|
||||
"project config: {project_settings:?}, model: {model_version:?}"
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_multi_agent_selector_uses_model_selected_before_first_turn() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user