feat: feature flag (#4948)

Add proper feature flag instead of having custom flags for everything.
This is just for experimental/wip part of the code
It can be used through CLI:
```bash
codex --enable unified_exec --disable view_image_tool
```

Or in the `config.toml`
```toml
# Global toggles applied to every profile unless overridden.
[features]
apply_patch_freeform = true
view_image_tool = false
```

Follow-up:
In a following PR, the goal is to have a default have `bundles` of
features that we can associate to a model
This commit is contained in:
jif-oai
2025-10-14 18:50:00 +01:00
committed by GitHub
parent d6c5df9a0a
commit f7b4e29609
16 changed files with 762 additions and 176 deletions

View File

@@ -4,6 +4,7 @@ use codex_core::CodexAuth;
use codex_core::ConversationManager;
use codex_core::ModelProviderInfo;
use codex_core::built_in_model_providers;
use codex_core::features::Feature;
use codex_core::model_family::find_family_for_model;
use codex_core::protocol::EventMsg;
use codex_core::protocol::InputItem;
@@ -56,12 +57,12 @@ async fn collect_tool_identifiers_for_model(model: &str) -> Vec<String> {
config.model = model.to_string();
config.model_family =
find_family_for_model(model).unwrap_or_else(|| panic!("unknown model family for {model}"));
config.include_plan_tool = false;
config.include_apply_patch_tool = false;
config.include_view_image_tool = false;
config.tools_web_search_request = false;
config.use_experimental_streamable_shell_tool = false;
config.use_experimental_unified_exec_tool = false;
config.features.disable(Feature::PlanTool);
config.features.disable(Feature::ApplyPatchFreeform);
config.features.disable(Feature::ViewImageTool);
config.features.disable(Feature::WebSearchRequest);
config.features.disable(Feature::StreamableShell);
config.features.disable(Feature::UnifiedExec);
let conversation_manager =
ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key"));

View File

@@ -5,6 +5,7 @@ use codex_core::ConversationManager;
use codex_core::ModelProviderInfo;
use codex_core::built_in_model_providers;
use codex_core::config::OPENAI_DEFAULT_MODEL;
use codex_core::features::Feature;
use codex_core::model_family::find_family_for_model;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg;
@@ -99,10 +100,10 @@ async fn codex_mini_latest_tools() {
config.cwd = cwd.path().to_path_buf();
config.model_provider = model_provider;
config.user_instructions = Some("be consistent and helpful".to_string());
config.features.disable(Feature::ApplyPatchFreeform);
let conversation_manager =
ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key"));
config.include_apply_patch_tool = false;
config.model = "codex-mini-latest".to_string();
config.model_family = find_family_for_model("codex-mini-latest").unwrap();
@@ -185,7 +186,7 @@ async fn prompt_tools_are_consistent_across_requests() {
config.cwd = cwd.path().to_path_buf();
config.model_provider = model_provider;
config.user_instructions = Some("be consistent and helpful".to_string());
config.include_plan_tool = true;
config.features.enable(Feature::PlanTool);
let conversation_manager =
ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key"));

View File

@@ -9,6 +9,7 @@ use std::time::UNIX_EPOCH;
use codex_core::config_types::McpServerConfig;
use codex_core::config_types::McpServerTransportConfig;
use codex_core::features::Feature;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg;
@@ -74,7 +75,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
let fixture = test_codex()
.with_config(move |config| {
config.use_experimental_use_rmcp_client = true;
config.features.enable(Feature::RmcpClient);
config.mcp_servers.insert(
server_name.to_string(),
McpServerConfig {
@@ -227,7 +228,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> {
let fixture = test_codex()
.with_config(move |config| {
config.use_experimental_use_rmcp_client = true;
config.features.enable(Feature::RmcpClient);
config.mcp_servers.insert(
server_name.to_string(),
McpServerConfig {
@@ -408,7 +409,7 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> {
let fixture = test_codex()
.with_config(move |config| {
config.use_experimental_use_rmcp_client = true;
config.features.enable(Feature::RmcpClient);
config.mcp_servers.insert(
server_name.to_string(),
McpServerConfig {

View File

@@ -1,6 +1,7 @@
#![cfg(not(target_os = "windows"))]
use anyhow::Result;
use codex_core::features::Feature;
use codex_core::model_family::find_family_for_model;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg;
@@ -77,7 +78,7 @@ async fn shell_output_stays_json_without_freeform_apply_patch() -> Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex().with_config(|config| {
config.include_apply_patch_tool = false;
config.features.disable(Feature::ApplyPatchFreeform);
config.model = "gpt-5".to_string();
config.model_family = find_family_for_model("gpt-5").expect("gpt-5 is a model family");
});
@@ -143,7 +144,7 @@ async fn shell_output_is_structured_with_freeform_apply_patch() -> Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex().with_config(|config| {
config.include_apply_patch_tool = true;
config.features.enable(Feature::ApplyPatchFreeform);
});
let test = builder.build(&server).await?;

View File

@@ -1,6 +1,7 @@
#![cfg(not(target_os = "windows"))]
use assert_matches::assert_matches;
use codex_core::features::Feature;
use codex_core::model_family::find_family_for_model;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg;
@@ -104,7 +105,7 @@ async fn update_plan_tool_emits_plan_update_event() -> anyhow::Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex().with_config(|config| {
config.include_plan_tool = true;
config.features.enable(Feature::PlanTool);
});
let TestCodex {
codex,
@@ -191,7 +192,7 @@ async fn update_plan_tool_rejects_malformed_payload() -> anyhow::Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex().with_config(|config| {
config.include_plan_tool = true;
config.features.enable(Feature::PlanTool);
});
let TestCodex {
codex,
@@ -285,7 +286,7 @@ async fn apply_patch_tool_executes_and_emits_patch_events() -> anyhow::Result<()
let server = start_mock_server().await;
let mut builder = test_codex().with_config(|config| {
config.include_apply_patch_tool = true;
config.features.enable(Feature::ApplyPatchFreeform);
});
let TestCodex {
codex,
@@ -403,7 +404,7 @@ async fn apply_patch_reports_parse_diagnostics() -> anyhow::Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex().with_config(|config| {
config.include_apply_patch_tool = true;
config.features.enable(Feature::ApplyPatchFreeform);
});
let TestCodex {
codex,

View File

@@ -2,6 +2,7 @@
#![allow(clippy::unwrap_used, clippy::expect_used)]
use anyhow::Result;
use codex_core::features::Feature;
use codex_core::model_family::find_family_for_model;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg;
@@ -293,7 +294,11 @@ async fn collect_tools(use_unified_exec: bool) -> Result<Vec<String>> {
let mock = mount_sse_sequence(&server, responses).await;
let mut builder = test_codex().with_config(move |config| {
config.use_experimental_unified_exec_tool = use_unified_exec;
if use_unified_exec {
config.features.enable(Feature::UnifiedExec);
} else {
config.features.disable(Feature::UnifiedExec);
}
});
let test = builder.build(&server).await?;

View File

@@ -3,6 +3,7 @@
use std::collections::HashMap;
use anyhow::Result;
use codex_core::features::Feature;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg;
use codex_core::protocol::InputItem;
@@ -42,7 +43,13 @@ fn collect_tool_outputs(bodies: &[Value]) -> Result<HashMap<String, Value>> {
if let Some(call_id) = item.get("call_id").and_then(Value::as_str) {
let content = extract_output_text(item)
.ok_or_else(|| anyhow::anyhow!("missing tool output content"))?;
let parsed: Value = serde_json::from_str(content)?;
let trimmed = content.trim();
if trimmed.is_empty() {
continue;
}
let parsed: Value = serde_json::from_str(trimmed).map_err(|err| {
anyhow::anyhow!("failed to parse tool output content {trimmed:?}: {err}")
})?;
outputs.insert(call_id.to_string(), parsed);
}
}
@@ -59,7 +66,7 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex().with_config(|config| {
config.use_experimental_unified_exec_tool = true;
config.features.enable(Feature::UnifiedExec);
});
let TestCodex {
codex,
@@ -176,6 +183,7 @@ async fn unified_exec_streams_after_lagged_output() -> Result<()> {
let mut builder = test_codex().with_config(|config| {
config.use_experimental_unified_exec_tool = true;
config.features.enable(Feature::UnifiedExec);
});
let TestCodex {
codex,
@@ -300,7 +308,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex().with_config(|config| {
config.use_experimental_unified_exec_tool = true;
config.features.enable(Feature::UnifiedExec);
});
let TestCodex {
codex,