mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
chore(features) rm Feature::ApplyPatchFreeform (#22711)
## Summary Removes the feature since this is effectively on by default in all cases where we should use it, or can be configured via models.json. ## Testing - [x] unit tests pass
This commit is contained in:
committed by
GitHub
Unverified
parent
7c11c14efc
commit
51b0e94105
@@ -2476,13 +2476,26 @@ async fn turn_start_streams_apply_patch_change_updates_v2() -> Result<()> {
|
||||
&server.uri(),
|
||||
"never",
|
||||
&BTreeMap::from([
|
||||
(Feature::ApplyPatchFreeform, true),
|
||||
(Feature::ApplyPatchStreamingEvents, true),
|
||||
(Feature::Plugins, false),
|
||||
(Feature::RemoteModels, false),
|
||||
(Feature::ShellSnapshot, false),
|
||||
]),
|
||||
)?;
|
||||
write_models_cache(&codex_home)?;
|
||||
let cache_path = codex_home.join("models_cache.json");
|
||||
let mut cache: serde_json::Value =
|
||||
serde_json::from_str(&std::fs::read_to_string(&cache_path)?)?;
|
||||
let models = cache["models"]
|
||||
.as_array_mut()
|
||||
.expect("models_cache.json models should be an array");
|
||||
let model = models
|
||||
.first_mut()
|
||||
.expect("models_cache.json should contain at least one model");
|
||||
model["slug"] = serde_json::Value::from("mock-model");
|
||||
model["display_name"] = serde_json::Value::from("mock-model");
|
||||
model["apply_patch_tool_type"] = serde_json::Value::from("freeform");
|
||||
std::fs::write(&cache_path, serde_json::to_string_pretty(&cache)?)?;
|
||||
|
||||
let mut mcp = McpProcess::new(&codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
@@ -457,9 +457,6 @@
|
||||
"in_app_browser": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_apply_patch_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"js_repl": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -4224,9 +4221,6 @@
|
||||
"in_app_browser": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_apply_patch_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"js_repl": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -4320,7 +4320,6 @@ async fn feature_table_overrides_legacy_flags() -> std::io::Result<()> {
|
||||
.await?;
|
||||
|
||||
assert!(!config.features.enabled(Feature::ApplyPatchFreeform));
|
||||
assert!(!config.include_apply_patch_tool);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -7505,7 +7504,6 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
compact_prompt: None,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: true,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Cached),
|
||||
web_search_config: None,
|
||||
use_experimental_unified_exec_tool: !cfg!(windows),
|
||||
@@ -7954,7 +7952,6 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
compact_prompt: None,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: true,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Cached),
|
||||
web_search_config: None,
|
||||
use_experimental_unified_exec_tool: !cfg!(windows),
|
||||
@@ -8117,7 +8114,6 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
compact_prompt: None,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: true,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Cached),
|
||||
web_search_config: None,
|
||||
use_experimental_unified_exec_tool: !cfg!(windows),
|
||||
@@ -8265,7 +8261,6 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
compact_prompt: None,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: true,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Cached),
|
||||
web_search_config: None,
|
||||
use_experimental_unified_exec_tool: !cfg!(windows),
|
||||
|
||||
@@ -348,12 +348,10 @@ pub(crate) fn validate_feature_requirements_in_config_toml(
|
||||
let configured_features = Features::from_sources(
|
||||
FeatureConfigSource {
|
||||
features: cfg.features.as_ref(),
|
||||
include_apply_patch_tool: None,
|
||||
experimental_use_unified_exec_tool: cfg.experimental_use_unified_exec_tool,
|
||||
},
|
||||
FeatureConfigSource {
|
||||
features: profile.features.as_ref(),
|
||||
include_apply_patch_tool: None,
|
||||
experimental_use_unified_exec_tool: profile.experimental_use_unified_exec_tool,
|
||||
},
|
||||
FeatureOverrides::default(),
|
||||
|
||||
@@ -764,11 +764,6 @@ pub struct Config {
|
||||
/// When set, restricts the login mechanism users may use.
|
||||
pub forced_login_method: Option<ForcedLoginMethod>,
|
||||
|
||||
/// Include the `apply_patch` tool for models that benefit from invoking
|
||||
/// file edits as a structured tool call. When unset, this falls back to the
|
||||
/// model info's default preference.
|
||||
pub include_apply_patch_tool: bool,
|
||||
|
||||
/// Explicit or feature-derived web search mode.
|
||||
pub web_search_mode: Constrained<WebSearchMode>,
|
||||
|
||||
@@ -2301,12 +2296,10 @@ impl Config {
|
||||
let configured_features = Features::from_sources(
|
||||
FeatureConfigSource {
|
||||
features: cfg.features.as_ref(),
|
||||
include_apply_patch_tool: None,
|
||||
experimental_use_unified_exec_tool: cfg.experimental_use_unified_exec_tool,
|
||||
},
|
||||
FeatureConfigSource {
|
||||
features: config_profile.features.as_ref(),
|
||||
include_apply_patch_tool: None,
|
||||
experimental_use_unified_exec_tool: config_profile
|
||||
.experimental_use_unified_exec_tool,
|
||||
},
|
||||
@@ -2845,7 +2838,6 @@ impl Config {
|
||||
config
|
||||
};
|
||||
|
||||
let include_apply_patch_tool_flag = features.enabled(Feature::ApplyPatchFreeform);
|
||||
let use_experimental_unified_exec_tool = features.enabled(Feature::UnifiedExec);
|
||||
|
||||
let forced_chatgpt_workspace_id = cfg
|
||||
@@ -3257,7 +3249,6 @@ impl Config {
|
||||
experimental_thread_store: thread_store_config(cfg.experimental_thread_store),
|
||||
forced_chatgpt_workspace_id,
|
||||
forced_login_method,
|
||||
include_apply_patch_tool: include_apply_patch_tool_flag,
|
||||
web_search_mode: constrained_web_search_mode.value,
|
||||
web_search_config,
|
||||
use_experimental_unified_exec_tool,
|
||||
|
||||
@@ -151,7 +151,6 @@ struct GuardianReviewSessionReuseKey {
|
||||
main_execve_wrapper_exe: Option<PathBuf>,
|
||||
zsh_path: Option<PathBuf>,
|
||||
features: ManagedFeatures,
|
||||
include_apply_patch_tool: bool,
|
||||
use_experimental_unified_exec_tool: bool,
|
||||
}
|
||||
|
||||
@@ -176,7 +175,6 @@ impl GuardianReviewSessionReuseKey {
|
||||
main_execve_wrapper_exe: spawn_config.main_execve_wrapper_exe.clone(),
|
||||
zsh_path: spawn_config.zsh_path.clone(),
|
||||
features: spawn_config.features.clone(),
|
||||
include_apply_patch_tool: spawn_config.include_apply_patch_tool,
|
||||
use_experimental_unified_exec_tool: spawn_config.use_experimental_unified_exec_tool,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1162,7 +1162,6 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() {
|
||||
let model_info = construct_model_info_offline("gpt-5.4", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::UnifiedExec);
|
||||
features.enable(Feature::ApplyPatchFreeform);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
|
||||
@@ -24,7 +24,6 @@ use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_extension_api::empty_extension_registry;
|
||||
use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_model_provider_info::built_in_model_providers;
|
||||
@@ -581,12 +580,6 @@ impl TestCodexBuilder {
|
||||
}
|
||||
ensure_test_model_catalog(&mut config)?;
|
||||
|
||||
if config.include_apply_patch_tool {
|
||||
config.features.enable(Feature::ApplyPatchFreeform)?;
|
||||
} else {
|
||||
config.features.disable(Feature::ApplyPatchFreeform)?;
|
||||
}
|
||||
|
||||
Ok((config, cwd))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -59,9 +59,7 @@ pub async fn apply_patch_harness() -> Result<TestCodexHarness> {
|
||||
async fn apply_patch_harness_with(
|
||||
configure: impl FnOnce(TestCodexBuilder) -> TestCodexBuilder,
|
||||
) -> Result<TestCodexHarness> {
|
||||
let builder = configure(test_codex()).with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
let builder = configure(test_codex());
|
||||
// Box harness construction so apply_patch_cli tests do not inline the
|
||||
// full test-thread startup path into each test future.
|
||||
Box::pin(TestCodexHarness::with_remote_env_builder(builder)).await
|
||||
@@ -989,15 +987,7 @@ async fn apply_patch_cli_verification_failure_has_no_side_effects(
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = apply_patch_harness_with(|builder| {
|
||||
builder.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ApplyPatchFreeform)
|
||||
.expect("test config should allow feature update");
|
||||
})
|
||||
})
|
||||
.await?;
|
||||
let harness = apply_patch_harness().await?;
|
||||
|
||||
// Compose a patch that would create a file, then fail verification on an update.
|
||||
let call_id = "apply-partial-no-side-effects";
|
||||
|
||||
@@ -1408,7 +1408,7 @@ fn scenarios() -> Vec<ScenarioSpec> {
|
||||
content: "freeform-patch-danger",
|
||||
},
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
features: vec![Feature::ApplyPatchFreeform],
|
||||
features: vec![],
|
||||
model_override: Some("gpt-5.4"),
|
||||
outcome: Outcome::Auto,
|
||||
expectation: Expectation::PatchApplied {
|
||||
|
||||
@@ -144,13 +144,11 @@ async fn run_code_mode_turn(
|
||||
server: &MockServer,
|
||||
prompt: &str,
|
||||
code: &str,
|
||||
include_apply_patch: bool,
|
||||
) -> Result<(TestCodex, ResponseMock)> {
|
||||
let mut builder = test_codex()
|
||||
.with_model("test-gpt-5.1-codex")
|
||||
.with_config(move |config| {
|
||||
let _ = config.features.enable(Feature::CodeMode);
|
||||
config.include_apply_patch_tool = include_apply_patch;
|
||||
});
|
||||
let test = builder.build(server).await?;
|
||||
|
||||
@@ -291,7 +289,6 @@ async fn code_mode_can_return_exec_command_output() -> Result<()> {
|
||||
r#"
|
||||
text(JSON.stringify(await tools.exec_command({ cmd: "printf code_mode_exec_marker" })));
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -541,7 +538,6 @@ const result = await tools.update_plan({
|
||||
});
|
||||
text(JSON.stringify(result));
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -666,7 +662,6 @@ text(JSON.stringify(await tools.exec_command({
|
||||
max_output_tokens: 100
|
||||
})));
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -705,7 +700,6 @@ text("before crash");
|
||||
text("still before crash");
|
||||
throw new Error("boom");
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -752,7 +746,6 @@ try {
|
||||
text(`caught:${error?.message ?? String(error)}`);
|
||||
}
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1769,7 +1762,6 @@ async fn code_mode_can_output_serialized_text_via_global_helper() -> Result<()>
|
||||
r#"
|
||||
text({ json: true });
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1801,7 +1793,6 @@ async fn code_mode_can_resume_after_set_timeout() -> Result<()> {
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
text("timer done");
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1830,7 +1821,6 @@ notify("code_mode_notify_marker");
|
||||
await tools.test_sync_tool({});
|
||||
text("done");
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1868,7 +1858,6 @@ text("before");
|
||||
exit();
|
||||
text("after");
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1907,7 +1896,6 @@ const circular = {};
|
||||
circular.self = circular;
|
||||
text(circular);
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1947,7 +1935,6 @@ async fn code_mode_can_output_images_via_global_helper() -> Result<()> {
|
||||
image("https://example.com/image.jpg");
|
||||
image("data:image/png;base64,AAA");
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -2137,13 +2124,8 @@ async fn code_mode_can_apply_patch_via_nested_tool() -> Result<()> {
|
||||
);
|
||||
let code = format!("text(await tools.apply_patch({patch:?}));\n");
|
||||
|
||||
let (test, second_mock) = run_code_mode_turn(
|
||||
&server,
|
||||
"use exec to run apply_patch",
|
||||
&code,
|
||||
/*include_apply_patch*/ true,
|
||||
)
|
||||
.await?;
|
||||
let (test, second_mock) =
|
||||
run_code_mode_turn(&server, "use exec to run apply_patch", &code).await?;
|
||||
|
||||
let req = second_mock.single_request();
|
||||
let items = custom_tool_output_items(&req, "call-1");
|
||||
@@ -2465,13 +2447,8 @@ const tool = ALL_TOOLS.find(({ name }) => name === "view_image");
|
||||
text(JSON.stringify(tool));
|
||||
"#;
|
||||
|
||||
let (_test, second_mock) = run_code_mode_turn(
|
||||
&server,
|
||||
"use exec to inspect ALL_TOOLS",
|
||||
code,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
let (_test, second_mock) =
|
||||
run_code_mode_turn(&server, "use exec to inspect ALL_TOOLS", code).await?;
|
||||
|
||||
let req = second_mock.single_request();
|
||||
let (output, success) = custom_tool_output_body_and_success(&req, "call-1");
|
||||
@@ -2890,7 +2867,6 @@ text(JSON.stringify({
|
||||
waited_long_enough: end_ms - start_ms >= 100,
|
||||
}));
|
||||
"#,
|
||||
/*include_apply_patch*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -153,7 +153,6 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() {
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::read_only())
|
||||
.expect("set permission profile");
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
let test = builder.build(&server).await.expect("build test codex");
|
||||
|
||||
|
||||
@@ -1658,7 +1658,6 @@ async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
trust_discovered_hooks(config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -2880,7 +2879,6 @@ async fn pre_tool_use_blocks_apply_patch_before_execution() -> Result<()> {
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
trust_discovered_hooks(config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -2959,7 +2957,6 @@ async fn pre_tool_use_rewrites_apply_patch_before_execution() -> Result<()> {
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
trust_discovered_hooks(config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -3025,7 +3022,6 @@ async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> {
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
trust_discovered_hooks(config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -3627,7 +3623,6 @@ async fn post_tool_use_records_additional_context_for_apply_patch() -> Result<()
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
trust_discovered_hooks(config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -3715,7 +3710,6 @@ async fn post_tool_use_records_apply_patch_context_with_edit_alias() -> Result<(
|
||||
}
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
trust_discovered_hooks(config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
@@ -231,10 +231,6 @@ async fn gpt_5_tools_without_apply_patch_append_apply_patch_instructions() -> an
|
||||
let TestCodex { codex, .. } = test_codex()
|
||||
.with_config(|config| {
|
||||
config.user_instructions = Some("be consistent and helpful".to_string());
|
||||
config
|
||||
.features
|
||||
.disable(Feature::ApplyPatchFreeform)
|
||||
.expect("test config should allow feature update");
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CollaborationModes)
|
||||
|
||||
@@ -347,9 +347,7 @@ async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result<
|
||||
};
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
let mut builder = test_codex();
|
||||
let test = builder.build_with_remote_and_local_env(&server).await?;
|
||||
let local_cwd = TempDir::new()?;
|
||||
let file_name = "apply_patch_remote_freeform.txt";
|
||||
@@ -435,7 +433,6 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> {
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||
config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
});
|
||||
|
||||
@@ -340,7 +340,6 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu
|
||||
let permission_profile_for_config = permission_profile.clone();
|
||||
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config
|
||||
.permissions
|
||||
|
||||
@@ -61,9 +61,7 @@ fn shell_responses(call_id: &str, command: &str, login: Option<bool>) -> Vec<Str
|
||||
async fn shell_command_harness_with(
|
||||
configure: impl FnOnce(TestCodexBuilder) -> TestCodexBuilder,
|
||||
) -> Result<TestCodexHarness> {
|
||||
let builder = configure(test_codex()).with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
let builder = configure(test_codex());
|
||||
TestCodexHarness::with_builder(builder).await
|
||||
}
|
||||
|
||||
|
||||
@@ -72,15 +72,10 @@ fn shell_responses(
|
||||
fn configure_shell_model(
|
||||
builder: TestCodexBuilder,
|
||||
output_type: ShellModelOutput,
|
||||
include_apply_patch_tool: bool,
|
||||
) -> TestCodexBuilder {
|
||||
let builder = match output_type {
|
||||
match output_type {
|
||||
ShellModelOutput::ShellCommand => builder.with_model("test-gpt-5-codex"),
|
||||
};
|
||||
|
||||
builder.with_config(move |config| {
|
||||
config.include_apply_patch_tool = include_apply_patch_tool;
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -91,11 +86,7 @@ async fn shell_output_is_structured_with_freeform_apply_patch(
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = configure_shell_model(
|
||||
test_codex(),
|
||||
output_type,
|
||||
/*include_apply_patch_tool*/ true,
|
||||
);
|
||||
let mut builder = configure_shell_model(test_codex(), output_type);
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id = "shell-structured";
|
||||
@@ -139,11 +130,7 @@ async fn shell_output_structures_fixture_with_serialization(
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = configure_shell_model(
|
||||
test_codex(),
|
||||
output_type,
|
||||
/*include_apply_patch_tool*/ true,
|
||||
);
|
||||
let mut builder = configure_shell_model(test_codex(), output_type);
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let fixture_path = test.cwd.path().join("fixture.json");
|
||||
@@ -200,11 +187,7 @@ async fn shell_output_for_freeform_tool_records_duration(
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = configure_shell_model(
|
||||
test_codex(),
|
||||
output_type,
|
||||
/*include_apply_patch_tool*/ true,
|
||||
);
|
||||
let mut builder = configure_shell_model(test_codex(), output_type);
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id = "shell-structured";
|
||||
@@ -474,11 +457,7 @@ async fn shell_output_is_structured_for_nonzero_exit(output_type: ShellModelOutp
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.4")
|
||||
.with_config(move |config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
let mut builder = test_codex().with_model("gpt-5.4");
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id = "shell-nonzero-exit";
|
||||
@@ -512,9 +491,7 @@ async fn shell_command_output_is_freeform() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
let mut builder = test_codex();
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let call_id = "shell-command";
|
||||
|
||||
@@ -531,7 +531,6 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> {
|
||||
.features
|
||||
.enable(Feature::ShellSnapshot)
|
||||
.expect("test config should allow feature update");
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
let harness = TestCodexHarness::with_builder(builder).await?;
|
||||
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
use std::fs;
|
||||
|
||||
use assert_matches::assert_matches;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::plan_tool::StepStatus;
|
||||
@@ -324,12 +323,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
|
||||
.features
|
||||
.enable(Feature::ApplyPatchFreeform)
|
||||
.expect("test config should allow feature update");
|
||||
});
|
||||
let mut builder = test_codex();
|
||||
let TestCodex {
|
||||
codex,
|
||||
cwd,
|
||||
@@ -467,12 +461,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
|
||||
.features
|
||||
.enable(Feature::ApplyPatchFreeform)
|
||||
.expect("test config should allow feature update");
|
||||
});
|
||||
let mut builder = test_codex();
|
||||
let TestCodex {
|
||||
codex,
|
||||
cwd,
|
||||
|
||||
@@ -71,7 +71,6 @@ async fn empty_turn_environments_omits_environment_backed_tools() -> Result<()>
|
||||
.features
|
||||
.enable(Feature::UnifiedExec)
|
||||
.expect("unified exec should enable for test");
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
|
||||
@@ -235,7 +235,6 @@ async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> {
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let builder = test_codex().with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
config.use_experimental_unified_exec_tool = true;
|
||||
if let Err(err) = config.features.enable(Feature::UnifiedExec) {
|
||||
panic!("test config should allow feature update: {err}");
|
||||
|
||||
@@ -21,10 +21,6 @@ const ALIASES: &[Alias] = &[
|
||||
legacy_key: "experimental_use_unified_exec_tool",
|
||||
feature: Feature::UnifiedExec,
|
||||
},
|
||||
Alias {
|
||||
legacy_key: "include_apply_patch_tool",
|
||||
feature: Feature::ApplyPatchFreeform,
|
||||
},
|
||||
Alias {
|
||||
legacy_key: "request_permissions",
|
||||
feature: Feature::ExecPermissionApprovals,
|
||||
@@ -67,18 +63,11 @@ pub(crate) fn feature_for_key(key: &str) -> Option<Feature> {
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
pub(crate) struct LegacyFeatureToggles {
|
||||
pub include_apply_patch_tool: Option<bool>,
|
||||
pub experimental_use_unified_exec_tool: Option<bool>,
|
||||
}
|
||||
|
||||
impl LegacyFeatureToggles {
|
||||
pub fn apply(self, features: &mut Features) {
|
||||
set_if_some(
|
||||
features,
|
||||
Feature::ApplyPatchFreeform,
|
||||
self.include_apply_patch_tool,
|
||||
"include_apply_patch_tool",
|
||||
);
|
||||
set_if_some(
|
||||
features,
|
||||
Feature::UnifiedExec,
|
||||
|
||||
@@ -99,7 +99,7 @@ pub enum Feature {
|
||||
ShellZshFork,
|
||||
/// Reflow transcript scrollback when the terminal is resized.
|
||||
TerminalResizeReflow,
|
||||
/// Include the freeform apply_patch tool.
|
||||
/// Removed compatibility flag for the deleted apply_patch fallback feature.
|
||||
ApplyPatchFreeform,
|
||||
/// Stream structured progress while apply_patch input is being generated.
|
||||
ApplyPatchStreamingEvents,
|
||||
@@ -292,7 +292,6 @@ pub struct FeatureOverrides {
|
||||
#[derive(Debug, Clone, Copy, Default)]
|
||||
pub struct FeatureConfigSource<'a> {
|
||||
pub features: Option<&'a FeaturesToml>,
|
||||
pub include_apply_patch_tool: Option<bool>,
|
||||
pub experimental_use_unified_exec_tool: Option<bool>,
|
||||
}
|
||||
|
||||
@@ -424,6 +423,9 @@ impl Features {
|
||||
"remote_control" => {
|
||||
continue;
|
||||
}
|
||||
"apply_patch_freeform" => {
|
||||
continue;
|
||||
}
|
||||
"image_detail_original" => {
|
||||
continue;
|
||||
}
|
||||
@@ -465,7 +467,6 @@ impl Features {
|
||||
|
||||
for source in [base, profile] {
|
||||
LegacyFeatureToggles {
|
||||
include_apply_patch_tool: source.include_apply_patch_tool,
|
||||
experimental_use_unified_exec_tool: source.experimental_use_unified_exec_tool,
|
||||
}
|
||||
.apply(&mut features);
|
||||
@@ -822,8 +823,8 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
FeatureSpec {
|
||||
id: Feature::ApplyPatchFreeform,
|
||||
key: "apply_patch_freeform",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
stage: Stage::Removed,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ApplyPatchStreamingEvents,
|
||||
|
||||
@@ -66,6 +66,16 @@ fn image_detail_original_is_removed_and_disabled_by_default() {
|
||||
assert_eq!(Feature::ImageDetailOriginal.default_enabled(), false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_patch_freeform_is_removed_and_disabled_by_default() {
|
||||
assert_eq!(Feature::ApplyPatchFreeform.stage(), Stage::Removed);
|
||||
assert_eq!(Feature::ApplyPatchFreeform.default_enabled(), false);
|
||||
assert_eq!(
|
||||
feature_for_key("apply_patch_freeform"),
|
||||
Some(Feature::ApplyPatchFreeform)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_only_requires_code_mode() {
|
||||
let mut features = Features::with_defaults();
|
||||
@@ -403,7 +413,6 @@ fn from_sources_applies_base_profile_and_overrides() {
|
||||
},
|
||||
FeatureConfigSource {
|
||||
features: Some(&profile_features),
|
||||
include_apply_patch_tool: Some(true),
|
||||
..Default::default()
|
||||
},
|
||||
FeatureOverrides {
|
||||
@@ -414,7 +423,7 @@ fn from_sources_applies_base_profile_and_overrides() {
|
||||
assert_eq!(features.enabled(Feature::Plugins), true);
|
||||
assert_eq!(features.enabled(Feature::CodeModeOnly), true);
|
||||
assert_eq!(features.enabled(Feature::CodeMode), true);
|
||||
assert_eq!(features.enabled(Feature::ApplyPatchFreeform), true);
|
||||
assert_eq!(features.enabled(Feature::ApplyPatchFreeform), false);
|
||||
assert_eq!(features.enabled(Feature::WebSearchRequest), false);
|
||||
}
|
||||
|
||||
@@ -472,6 +481,23 @@ fn from_sources_ignores_removed_js_repl_feature_keys() {
|
||||
assert_eq!(features, Features::with_defaults());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_sources_ignores_removed_apply_patch_freeform_feature_key() {
|
||||
let features_toml =
|
||||
FeaturesToml::from(BTreeMap::from([("apply_patch_freeform".to_string(), true)]));
|
||||
|
||||
let features = Features::from_sources(
|
||||
FeatureConfigSource {
|
||||
features: Some(&features_toml),
|
||||
..Default::default()
|
||||
},
|
||||
FeatureConfigSource::default(),
|
||||
FeatureOverrides::default(),
|
||||
);
|
||||
|
||||
assert_eq!(features, Features::with_defaults());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn multi_agent_v2_feature_config_deserializes_boolean_toggle() {
|
||||
let features: FeaturesToml = toml::from_str(
|
||||
@@ -587,14 +613,13 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
|
||||
proxy_url: Some("http://127.0.0.1:43128".to_string()),
|
||||
..Default::default()
|
||||
})),
|
||||
entries: BTreeMap::from([("include_apply_patch_tool".to_string(), true)]),
|
||||
entries: BTreeMap::new(),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
features_toml.materialize_resolved_enabled(&features);
|
||||
|
||||
let entries = features_toml.entries();
|
||||
assert_eq!(entries.get("include_apply_patch_tool"), None);
|
||||
for spec in crate::FEATURES {
|
||||
assert_eq!(
|
||||
entries.get(spec.key),
|
||||
@@ -627,7 +652,7 @@ fn materialize_resolved_enabled_writes_all_features_and_preserves_custom_config(
|
||||
FeatureConfigSource::default(),
|
||||
FeatureOverrides::default(),
|
||||
);
|
||||
assert_eq!(replayed.enabled(Feature::ApplyPatchFreeform), true);
|
||||
assert_eq!(replayed.enabled(Feature::ApplyPatchFreeform), false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -262,7 +262,6 @@ fn new_config(model: Option<String>, arg0_paths: Arg0DispatchPaths) -> anyhow::R
|
||||
experimental_thread_store: ThreadStoreConfig::Local,
|
||||
forced_chatgpt_workspace_id: None,
|
||||
forced_login_method: None,
|
||||
include_apply_patch_tool: false,
|
||||
web_search_mode: Constrained::allow_any(WebSearchMode::Disabled),
|
||||
web_search_config: None,
|
||||
use_experimental_unified_exec_tool: false,
|
||||
|
||||
@@ -175,7 +175,6 @@ impl ToolsConfig {
|
||||
session_source,
|
||||
..
|
||||
} = params;
|
||||
let include_apply_patch_tool = features.enabled(Feature::ApplyPatchFreeform);
|
||||
let include_code_mode = features.enabled(Feature::CodeMode);
|
||||
let include_code_mode_only = include_code_mode && features.enabled(Feature::CodeModeOnly);
|
||||
let include_goal_tools = features.enabled(Feature::Goals);
|
||||
@@ -225,10 +224,7 @@ impl ToolsConfig {
|
||||
model_shell_type
|
||||
};
|
||||
|
||||
let apply_patch_tool_type = model_info
|
||||
.apply_patch_tool_type
|
||||
.clone()
|
||||
.or_else(|| include_apply_patch_tool.then_some(ApplyPatchToolType::Freeform));
|
||||
let apply_patch_tool_type = model_info.apply_patch_tool_type.clone();
|
||||
|
||||
let agent_jobs_worker_tools = include_agent_jobs
|
||||
&& matches!(
|
||||
|
||||
@@ -4,7 +4,6 @@ use codex_features::Features;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ApplyPatchToolType;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
@@ -156,7 +155,7 @@ fn shell_zsh_fork_prefers_shell_command_over_unified_exec() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fallback_apply_patch_models_use_freeform_tool_by_default() {
|
||||
fn fallback_apply_patch_models_do_not_use_freeform_tool_by_default() {
|
||||
let model_info = model_info();
|
||||
let features = Features::with_defaults();
|
||||
|
||||
@@ -172,10 +171,7 @@ fn fallback_apply_patch_models_use_freeform_tool_by_default() {
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
tools_config.apply_patch_tool_type,
|
||||
Some(ApplyPatchToolType::Freeform)
|
||||
);
|
||||
assert_eq!(tools_config.apply_patch_tool_type, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user