From e783341b705721728a8fa422416c10c3a09c7716 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Fri, 8 May 2026 13:00:57 -0700 Subject: [PATCH] [codex] Delete function-style apply_patch (#21651) ## Why `apply_patch` is now a freeform/custom tool. Keeping the old JSON/function-style registration and parsing path left another way for models and tests to invoke `apply_patch`, which made the tool surface harder to reason about. ## What changed - Removed the `ApplyPatchToolType::Function` variant, JSON `apply_patch` spec, and handler support for function payloads. - Kept `apply_patch_tool_type = freeform` as the supported model metadata path, including Bedrock catalog metadata. - Migrated `apply_patch` tests and SSE fixtures to custom/freeform tool calls. ## Verification - `cargo test -p codex-tools -p codex-protocol -p codex-model-provider` - `cargo test -p codex-core tools::handlers::apply_patch --lib` - `cargo test -p codex-core --test all apply_patch_tool_executes_and_emits_patch_events` - `cargo test -p codex-core --test all apply_patch_reports_parse_diagnostics` - `cargo test -p codex-exec test_apply_patch_tool` - `just fix -p codex-core` - `just fix -p codex-tools -p codex-protocol -p codex-model-provider -p codex-exec` --- .../core/src/tools/handlers/apply_patch.rs | 56 ++-------- .../src/tools/handlers/apply_patch_spec.rs | 103 ------------------ .../tools/handlers/apply_patch_spec_tests.rs | 26 ----- .../src/tools/handlers/apply_patch_tests.rs | 40 +------ codex-rs/core/src/tools/spec_plan.rs | 8 +- codex-rs/core/tests/common/responses.rs | 21 +--- codex-rs/core/tests/common/test_codex.rs | 4 +- codex-rs/core/tests/suite/apply_patch_cli.rs | 44 ++------ codex-rs/core/tests/suite/approvals.rs | 91 ++++++++-------- codex-rs/core/tests/suite/codex_delegate.rs | 4 +- codex-rs/core/tests/suite/hooks.rs | 18 +-- .../tests/suite/request_permissions_tool.rs | 21 +++- .../core/tests/suite/shell_serialization.rs | 15 +-- codex-rs/core/tests/suite/tool_harness.rs | 36 ++++-- codex-rs/core/tests/suite/tools.rs | 11 +- codex-rs/exec/tests/suite/apply_patch.rs | 3 +- .../src/amazon_bedrock/catalog.rs | 2 +- codex-rs/protocol/src/openai_models.rs | 1 - codex-rs/tools/src/tool_config.rs | 9 +- codex-rs/tools/src/tool_config_tests.rs | 4 +- 20 files changed, 143 insertions(+), 374 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 66b04f8e73..452bcb8d85 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -21,10 +21,7 @@ use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::handlers::apply_granted_turn_permissions; -use crate::tools::handlers::apply_patch_spec::ApplyPatchToolArgs; use crate::tools::handlers::apply_patch_spec::create_apply_patch_freeform_tool; -use crate::tools::handlers::apply_patch_spec::create_apply_patch_json_tool; -use crate::tools::handlers::parse_arguments; use crate::tools::hook_names::HookToolName; use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::registry::PostToolUsePayload; @@ -43,7 +40,6 @@ use codex_exec_server::ExecutorFileSystem; use codex_features::Feature; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::models::FileSystemPermissions; -use codex_protocol::openai_models::ApplyPatchToolType; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::PatchApplyUpdatedEvent; @@ -56,25 +52,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; const APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL: Duration = Duration::from_millis(500); -pub struct ApplyPatchHandler { - options: ApplyPatchToolType, -} - -impl Default for ApplyPatchHandler { - fn default() -> Self { - Self { - options: ApplyPatchToolType::Freeform, - } - } -} - -impl ApplyPatchHandler { - pub(crate) fn new(apply_patch_tool_type: ApplyPatchToolType) -> Self { - Self { - options: apply_patch_tool_type, - } - } -} +pub struct ApplyPatchHandler; #[derive(Default)] struct ApplyPatchArgumentDiffConsumer { @@ -264,15 +242,8 @@ fn write_permissions_for_paths( } /// Extracts the raw patch text used as the command-shaped hook input for apply_patch. -/// -/// The apply_patch tool can arrive as the older JSON/function shape or as a -/// freeform custom tool call. Both represent the same file edit operation, so -/// hooks see the raw patch body in `tool_input.command` either way. fn apply_patch_payload_command(payload: &ToolPayload) -> Option { match payload { - ToolPayload::Function { arguments } => parse_arguments::(arguments) - .ok() - .map(|args| args.input), ToolPayload::Custom { input } => Some(input.clone()), _ => None, } @@ -320,10 +291,7 @@ impl ToolHandler for ApplyPatchHandler { } fn spec(&self) -> Option { - Some(match self.options { - ApplyPatchToolType::Freeform => create_apply_patch_freeform_tool(), - ApplyPatchToolType::Function => create_apply_patch_json_tool(), - }) + Some(create_apply_patch_freeform_tool()) } fn kind(&self) -> ToolKind { @@ -331,10 +299,7 @@ impl ToolHandler for ApplyPatchHandler { } fn matches_kind(&self, payload: &ToolPayload) -> bool { - matches!( - payload, - ToolPayload::Function { .. } | ToolPayload::Custom { .. } - ) + matches!(payload, ToolPayload::Custom { .. }) } async fn is_mutating(&self, _invocation: &ToolInvocation) -> bool { @@ -380,17 +345,10 @@ impl ToolHandler for ApplyPatchHandler { .. } = invocation; - let patch_input = match payload { - ToolPayload::Function { arguments } => { - let args: ApplyPatchToolArgs = parse_arguments(&arguments)?; - args.input - } - ToolPayload::Custom { input } => input, - _ => { - return Err(FunctionCallError::RespondToModel( - "apply_patch handler received unsupported payload".to_string(), - )); - } + let ToolPayload::Custom { input: patch_input } = payload else { + return Err(FunctionCallError::RespondToModel( + "apply_patch handler received unsupported payload".to_string(), + )); }; // Re-parse and verify the patch so we can compute changes and approval. diff --git a/codex-rs/core/src/tools/handlers/apply_patch_spec.rs b/codex-rs/core/src/tools/handlers/apply_patch_spec.rs index 93a3ce4aac..fa1ffcb8c0 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_spec.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_spec.rs @@ -1,89 +1,9 @@ use codex_tools::FreeformTool; use codex_tools::FreeformToolFormat; -use codex_tools::JsonSchema; -use codex_tools::ResponsesApiTool; use codex_tools::ToolSpec; -use serde::Deserialize; -use serde::Serialize; -use std::collections::BTreeMap; const APPLY_PATCH_LARK_GRAMMAR: &str = include_str!("apply_patch.lark"); -const APPLY_PATCH_JSON_TOOL_DESCRIPTION: &str = r#"Use the `apply_patch` tool to edit files. -Your patch language is a stripped‑down, file‑oriented diff format designed to be easy to parse and safe to apply. You can think of it as a high‑level envelope: - -*** Begin Patch -[ one or more file sections ] -*** End Patch - -Within that envelope, you get a sequence of file operations. -You MUST include a header to specify the action you are taking. -Each operation starts with one of three headers: - -*** Add File: - create a new file. Every following line is a + line (the initial contents). -*** Delete File: - remove an existing file. Nothing follows. -*** Update File: - patch an existing file in place (optionally with a rename). - -May be immediately followed by *** Move to: if you want to rename the file. -Then one or more “hunks”, each introduced by @@ (optionally followed by a hunk header). -Within a hunk each line starts with: - -For instructions on [context_before] and [context_after]: -- By default, show 3 lines of code immediately above and 3 lines immediately below each change. If a change is within 3 lines of a previous change, do NOT duplicate the first change’s [context_after] lines in the second change’s [context_before] lines. -- If 3 lines of context is insufficient to uniquely identify the snippet of code within the file, use the @@ operator to indicate the class or function to which the snippet belongs. For instance, we might have: -@@ class BaseClass -[3 lines of pre-context] -- [old_code] -+ [new_code] -[3 lines of post-context] - -- If a code block is repeated so many times in a class or function such that even a single `@@` statement and 3 lines of context cannot uniquely identify the snippet of code, you can use multiple `@@` statements to jump to the right context. For instance: - -@@ class BaseClass -@@ def method(): -[3 lines of pre-context] -- [old_code] -+ [new_code] -[3 lines of post-context] - -The full grammar definition is below: -Patch := Begin { FileOp } End -Begin := "*** Begin Patch" NEWLINE -End := "*** End Patch" NEWLINE -FileOp := AddFile | DeleteFile | UpdateFile -AddFile := "*** Add File: " path NEWLINE { "+" line NEWLINE } -DeleteFile := "*** Delete File: " path NEWLINE -UpdateFile := "*** Update File: " path NEWLINE [ MoveTo ] { Hunk } -MoveTo := "*** Move to: " newPath NEWLINE -Hunk := "@@" [ header ] NEWLINE { HunkLine } [ "*** End of File" NEWLINE ] -HunkLine := (" " | "-" | "+") text NEWLINE - -A full patch can combine several operations: - -*** Begin Patch -*** Add File: hello.txt -+Hello world -*** Update File: src/app.py -*** Move to: src/main.py -@@ def greet(): --print("Hi") -+print("Hello, world!") -*** Delete File: obsolete.txt -*** End Patch - -It is important to remember: - -- You must include a header with your intended action (Add/Delete/Update) -- You must prefix new lines with `+` even when creating a new file -- File references can only be relative, NEVER ABSOLUTE. -"#; - -/// TODO(dylan): deprecate once we get rid of json tool -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct ApplyPatchToolArgs { - pub input: String, -} - /// Returns a custom tool that can be used to edit files. Well-suited for GPT-5 models /// https://platform.openai.com/docs/guides/function-calling#custom-tools pub fn create_apply_patch_freeform_tool() -> ToolSpec { @@ -98,29 +18,6 @@ pub fn create_apply_patch_freeform_tool() -> ToolSpec { }) } -/// Returns a json tool that can be used to edit files. Should only be used with gpt-oss models -pub fn create_apply_patch_json_tool() -> ToolSpec { - let properties = BTreeMap::from([( - "input".to_string(), - JsonSchema::string(Some( - "The entire contents of the apply_patch command".to_string(), - )), - )]); - - ToolSpec::Function(ResponsesApiTool { - name: "apply_patch".to_string(), - description: APPLY_PATCH_JSON_TOOL_DESCRIPTION.to_string(), - strict: false, - defer_loading: None, - parameters: JsonSchema::object( - properties, - Some(vec!["input".to_string()]), - Some(false.into()), - ), - output_schema: None, - }) -} - #[cfg(test)] #[path = "apply_patch_spec_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/handlers/apply_patch_spec_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_spec_tests.rs index beda5cc916..28bd57a991 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_spec_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_spec_tests.rs @@ -1,7 +1,5 @@ use super::*; -use codex_tools::JsonSchema; use pretty_assertions::assert_eq; -use std::collections::BTreeMap; #[test] fn create_apply_patch_freeform_tool_matches_expected_spec() { @@ -20,27 +18,3 @@ fn create_apply_patch_freeform_tool_matches_expected_spec() { }) ); } - -#[test] -fn create_apply_patch_json_tool_matches_expected_spec() { - assert_eq!( - create_apply_patch_json_tool(), - ToolSpec::Function(ResponsesApiTool { - name: "apply_patch".to_string(), - description: APPLY_PATCH_JSON_TOOL_DESCRIPTION.to_string(), - strict: false, - defer_loading: None, - parameters: JsonSchema::object( - BTreeMap::from([( - "input".to_string(), - JsonSchema::string(Some( - "The entire contents of the apply_patch command".to_string(), - ),), - )]), - Some(vec!["input".to_string()]), - Some(false.into()) - ), - output_schema: None, - }) - ); -} diff --git a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index c0d4d17f32..dfa642ade1 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -42,24 +42,6 @@ async fn invocation_for_payload(payload: ToolPayload) -> ToolInvocation { } } -#[tokio::test] -async fn pre_tool_use_payload_uses_json_patch_input() { - let patch = sample_patch(); - let payload = ToolPayload::Function { - arguments: json!({ "input": patch }).to_string(), - }; - let invocation = invocation_for_payload(payload).await; - let handler = ApplyPatchHandler::default(); - - assert_eq!( - handler.pre_tool_use_payload(&invocation), - Some(PreToolUsePayload { - tool_name: HookToolName::apply_patch(), - tool_input: json!({ "command": patch }), - }) - ); -} - #[tokio::test] async fn pre_tool_use_payload_uses_freeform_patch_input() { let patch = sample_patch(); @@ -67,7 +49,7 @@ async fn pre_tool_use_payload_uses_freeform_patch_input() { input: patch.to_string(), }; let invocation = invocation_for_payload(payload).await; - let handler = ApplyPatchHandler::default(); + let handler = ApplyPatchHandler; assert_eq!( handler.pre_tool_use_payload(&invocation), @@ -86,7 +68,7 @@ async fn post_tool_use_payload_uses_patch_input_and_tool_output() { }; let invocation = invocation_for_payload(payload).await; let output = ApplyPatchToolOutput::from_text("Success. Updated files.".to_string()); - let handler = ApplyPatchHandler::default(); + let handler = ApplyPatchHandler; assert_eq!( handler.post_tool_use_payload(&invocation, &output), @@ -99,24 +81,6 @@ async fn post_tool_use_payload_uses_patch_input_and_tool_output() { ); } -#[test] -fn diff_consumer_does_not_stream_json_tool_call_arguments() { - let mut consumer = ApplyPatchArgumentDiffConsumer::default(); - assert!( - consumer - .push_delta("call-1".to_string(), r#"{"input":"*** Begin Patch\n"#) - .is_none() - ); - assert!( - consumer - .push_delta( - "call-1".to_string(), - r#"*** Add File: hello.txt\n+hello\n*** End Patch\n"}"# - ) - .is_none() - ); -} - #[test] fn diff_consumer_streams_apply_patch_changes() { let mut consumer = ApplyPatchArgumentDiffConsumer::default(); diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 1595db7b28..4361cfe4fd 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -257,12 +257,8 @@ pub fn build_tool_registry_builder( ))); } - if config.environment_mode.has_environment() - && let Some(apply_patch_tool_type) = &config.apply_patch_tool_type - { - builder.register_handler(Arc::new(ApplyPatchHandler::new( - apply_patch_tool_type.clone(), - ))); + if config.environment_mode.has_environment() && config.apply_patch_tool_type.is_some() { + builder.register_handler(Arc::new(ApplyPatchHandler)); } if config diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 93472e72bb..eab90c5aba 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -890,7 +890,6 @@ pub fn ev_apply_patch_call( ) -> Value { match output_type { ApplyPatchModelOutput::Freeform => ev_apply_patch_custom_tool_call(call_id, patch), - ApplyPatchModelOutput::Function => ev_apply_patch_function_call(call_id, patch), ApplyPatchModelOutput::Shell => ev_apply_patch_shell_call(call_id, patch), ApplyPatchModelOutput::ShellViaHeredoc => { ev_apply_patch_shell_call_via_heredoc(call_id, patch) @@ -903,7 +902,7 @@ pub fn ev_apply_patch_call( /// Convenience: SSE event for an `apply_patch` custom tool call with raw patch /// text. This mirrors the payload produced by the Responses API when the model -/// invokes `apply_patch` directly (before we convert it to a function call). +/// invokes `apply_patch` directly. pub fn ev_apply_patch_custom_tool_call(call_id: &str, patch: &str) -> Value { serde_json::json!({ "type": "response.output_item.done", @@ -916,24 +915,6 @@ pub fn ev_apply_patch_custom_tool_call(call_id: &str, patch: &str) -> Value { }) } -/// Convenience: SSE event for an `apply_patch` function call. The Responses API -/// wraps the patch content in a JSON string under the `input` key; we recreate -/// the same structure so downstream code exercises the full parsing path. -pub fn ev_apply_patch_function_call(call_id: &str, patch: &str) -> Value { - let arguments = serde_json::json!({ "input": patch }); - let arguments = serde_json::to_string(&arguments).expect("serialize apply_patch arguments"); - - serde_json::json!({ - "type": "response.output_item.done", - "item": { - "type": "function_call", - "name": "apply_patch", - "arguments": arguments, - "call_id": call_id - } - }) -} - pub fn ev_shell_command_call(call_id: &str, command: &str) -> Value { let args = serde_json::json!({ "command": command }); ev_shell_command_call_with_args(call_id, &args) diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index c326d84ebf..bcc93d7d09 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -188,7 +188,6 @@ fn docker_command_capture_stdout(args: [&str; N]) -> Result { Box::pin(self.custom_tool_call_output(call_id)).await } - ApplyPatchModelOutput::Function - | ApplyPatchModelOutput::Shell + ApplyPatchModelOutput::Shell | ApplyPatchModelOutput::ShellViaHeredoc | ApplyPatchModelOutput::ShellCommandViaHeredoc => { Box::pin(self.function_call_stdout(call_id)).await diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index bc51fdd460..3f85d31c7c 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -24,7 +24,6 @@ use codex_protocol::user_input::UserInput; #[cfg(target_os = "linux")] use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use core_test_support::assert_regex_match; -use core_test_support::responses::ev_apply_patch_function_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -172,14 +171,14 @@ async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() - call_id, patch, "done", - ApplyPatchModelOutput::Function, + ApplyPatchModelOutput::Freeform, ) .await; harness.submit("please apply helper alias patch").await?; let out = harness - .apply_patch_output(call_id, ApplyPatchModelOutput::Function) + .apply_patch_output(call_id, ApplyPatchModelOutput::Freeform) .await; assert_regex_match( r"(?s)^Exit code: 0.*Success\. Updated the following files:\nA helper-alias\.txt\n?$", @@ -192,7 +191,6 @@ async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_cli_multiple_operations_integration( @@ -237,7 +235,6 @@ D delete.txt #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -265,7 +262,6 @@ async fn apply_patch_cli_multiple_chunks(model_output: ApplyPatchModelOutput) -> #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -294,7 +290,6 @@ async fn apply_patch_cli_moves_file_to_new_directory( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -323,7 +318,6 @@ async fn apply_patch_cli_updates_file_appends_trailing_newline( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -353,7 +347,6 @@ async fn apply_patch_cli_insert_only_hunk_modifies_file( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -385,7 +378,6 @@ async fn apply_patch_cli_move_overwrites_existing_destination( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -425,7 +417,6 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -453,7 +444,6 @@ async fn apply_patch_cli_add_overwrites_existing_file( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -485,7 +475,6 @@ async fn apply_patch_cli_rejects_invalid_hunk_header( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -521,7 +510,6 @@ async fn apply_patch_cli_reports_missing_context( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -557,7 +545,6 @@ async fn apply_patch_cli_reports_missing_target_file( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -594,7 +581,6 @@ async fn apply_patch_cli_delete_missing_file_reports_error( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -619,7 +605,6 @@ async fn apply_patch_cli_rejects_empty_patch(model_output: ApplyPatchModelOutput #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -646,7 +631,6 @@ async fn apply_patch_cli_delete_directory_reports_verification_error( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -693,7 +677,6 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -743,7 +726,6 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -1171,7 +1153,7 @@ async fn apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nest call_id, patch, "updated repo-relative path", - ApplyPatchModelOutput::Function, + ApplyPatchModelOutput::Freeform, ) .await; @@ -1260,7 +1242,7 @@ async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] -async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch( +async fn apply_patch_shell_accepts_lenient_heredoc_wrapped_patch( model_output: ApplyPatchModelOutput, ) -> Result<()> { skip_if_no_network!(Ok(())); @@ -1281,7 +1263,6 @@ async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -1303,7 +1284,6 @@ async fn apply_patch_cli_end_of_file_anchor(model_output: ApplyPatchModelOutput) #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -1340,7 +1320,6 @@ async fn apply_patch_cli_missing_second_chunk_context_rejected( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] @@ -1394,12 +1373,12 @@ async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()> let s1 = sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call1, patch1), + ev_apply_patch_custom_tool_call(call1, patch1), ev_completed("resp-1"), ]); let s2 = sse(vec![ ev_response_created("resp-2"), - ev_apply_patch_function_call(call2, patch2), + ev_apply_patch_custom_tool_call(call2, patch2), ev_completed("resp-2"), ]); let s3 = sse(vec![ @@ -1446,12 +1425,12 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result let responses = vec![ sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_success, patch_success), + ev_apply_patch_custom_tool_call(call_success, patch_success), ev_completed("resp-1"), ]), sse(vec![ ev_response_created("resp-2"), - ev_apply_patch_function_call(call_failure, patch_failure), + ev_apply_patch_custom_tool_call(call_failure, patch_failure), ev_completed("resp-2"), ]), sse(vec![ @@ -1488,7 +1467,7 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result "diff should include contents from successful patch: {diff}" ); - let failure_out = harness.function_call_stdout(call_failure).await; + let failure_out = harness.custom_tool_call_output(call_failure).await; assert!( failure_out.contains("apply_patch verification failed"), "expected verification failure output: {failure_out}" @@ -1529,12 +1508,12 @@ async fn apply_patch_clears_aggregated_diff_after_inexact_delta() -> Result<()> let responses = vec![ sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_success, patch_success), + ev_apply_patch_custom_tool_call(call_success, patch_success), ev_completed("resp-1"), ]), sse(vec![ ev_response_created("resp-2"), - ev_apply_patch_function_call(call_inexact, patch_inexact), + ev_apply_patch_custom_tool_call(call_inexact, patch_inexact), ev_completed("resp-2"), ]), sse(vec![ @@ -1573,7 +1552,6 @@ async fn apply_patch_clears_aggregated_diff_after_inexact_delta() -> Result<()> #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 2538c850e3..10bbb852eb 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -20,7 +20,7 @@ use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; use core_test_support::managed_network_requirements_loader; -use core_test_support::responses::ev_apply_patch_function_call; +use core_test_support::responses::ev_apply_patch_custom_tool_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -108,7 +108,7 @@ enum ActionKind { command: &'static str, justification: Option<&'static str>, }, - ApplyPatchFunction { + ApplyPatchFreeform { target: TargetPath, content: &'static str, }, @@ -131,7 +131,7 @@ impl ActionKind { | ActionKind::RunCommand { .. } | ActionKind::RunCommandWithPrefixRule { .. } | ActionKind::RunUnifiedExecCommand { .. } - | ActionKind::ApplyPatchFunction { .. } + | ActionKind::ApplyPatchFreeform { .. } | ActionKind::ApplyPatchShell { .. } => None, } } @@ -264,11 +264,11 @@ impl ActionKind { )?; Ok((event, Some(command.to_string()))) } - ActionKind::ApplyPatchFunction { target, content } => { + ActionKind::ApplyPatchFreeform { target, content } => { let (path, patch_path) = target.resolve_for_patch(test); let _ = fs::remove_file(&path); let patch = build_add_file_patch(&patch_path, content); - Ok((ev_apply_patch_function_call(call_id, &patch), None)) + Ok((ev_apply_patch_custom_tool_call(call_id, &patch), None)) } ActionKind::ApplyPatchShell { target, content } => { let (path, patch_path) = target.resolve_for_patch(test); @@ -1342,46 +1342,46 @@ fn scenarios() -> Vec { }, }, ScenarioSpec { - name: "apply_patch_function_auto_inside_workspace", + name: "apply_patch_freeform_auto_inside_workspace", approval_policy: OnRequest, sandbox_policy: SandboxPolicy::DangerFullAccess, - action: ActionKind::ApplyPatchFunction { - target: TargetPath::Workspace("apply_patch_function.txt"), - content: "function-apply-patch", + action: ActionKind::ApplyPatchFreeform { + target: TargetPath::Workspace("apply_patch_freeform.txt"), + content: "freeform-apply-patch", }, sandbox_permissions: SandboxPermissions::UseDefault, features: vec![], model_override: Some("gpt-5.4"), outcome: Outcome::Auto, expectation: Expectation::PatchApplied { - target: TargetPath::Workspace("apply_patch_function.txt"), - content: "function-apply-patch", + target: TargetPath::Workspace("apply_patch_freeform.txt"), + content: "freeform-apply-patch", }, }, ScenarioSpec { - name: "apply_patch_function_danger_allows_outside_workspace", + name: "apply_patch_freeform_danger_allows_outside_workspace", approval_policy: OnRequest, sandbox_policy: SandboxPolicy::DangerFullAccess, - action: ActionKind::ApplyPatchFunction { - target: TargetPath::OutsideWorkspace("apply_patch_function_danger.txt"), - content: "function-patch-danger", + action: ActionKind::ApplyPatchFreeform { + target: TargetPath::OutsideWorkspace("apply_patch_freeform_danger.txt"), + content: "freeform-patch-danger", }, sandbox_permissions: SandboxPermissions::UseDefault, features: vec![Feature::ApplyPatchFreeform], model_override: Some("gpt-5.4"), outcome: Outcome::Auto, expectation: Expectation::PatchApplied { - target: TargetPath::OutsideWorkspace("apply_patch_function_danger.txt"), - content: "function-patch-danger", + target: TargetPath::OutsideWorkspace("apply_patch_freeform_danger.txt"), + content: "freeform-patch-danger", }, }, ScenarioSpec { - name: "apply_patch_function_outside_requires_patch_approval", + name: "apply_patch_freeform_outside_requires_patch_approval", approval_policy: OnRequest, sandbox_policy: workspace_write(false), - action: ActionKind::ApplyPatchFunction { - target: TargetPath::OutsideWorkspace("apply_patch_function_outside.txt"), - content: "function-patch-outside", + action: ActionKind::ApplyPatchFreeform { + target: TargetPath::OutsideWorkspace("apply_patch_freeform_outside.txt"), + content: "freeform-patch-outside", }, sandbox_permissions: SandboxPermissions::UseDefault, features: vec![], @@ -1391,17 +1391,17 @@ fn scenarios() -> Vec { expected_reason: None, }, expectation: Expectation::PatchApplied { - target: TargetPath::OutsideWorkspace("apply_patch_function_outside.txt"), - content: "function-patch-outside", + target: TargetPath::OutsideWorkspace("apply_patch_freeform_outside.txt"), + content: "freeform-patch-outside", }, }, ScenarioSpec { - name: "apply_patch_function_outside_denied_blocks_patch", + name: "apply_patch_freeform_outside_denied_blocks_patch", approval_policy: OnRequest, sandbox_policy: workspace_write(false), - action: ActionKind::ApplyPatchFunction { - target: TargetPath::OutsideWorkspace("apply_patch_function_outside_denied.txt"), - content: "function-patch-outside-denied", + action: ActionKind::ApplyPatchFreeform { + target: TargetPath::OutsideWorkspace("apply_patch_freeform_outside_denied.txt"), + content: "freeform-patch-outside-denied", }, sandbox_permissions: SandboxPermissions::UseDefault, features: vec![], @@ -1411,7 +1411,7 @@ fn scenarios() -> Vec { expected_reason: None, }, expectation: Expectation::FileNotCreated { - target: TargetPath::OutsideWorkspace("apply_patch_function_outside_denied.txt"), + target: TargetPath::OutsideWorkspace("apply_patch_freeform_outside_denied.txt"), message_contains: &["patch rejected by user"], }, }, @@ -1436,12 +1436,12 @@ fn scenarios() -> Vec { }, }, ScenarioSpec { - name: "apply_patch_function_unless_trusted_requires_patch_approval", + name: "apply_patch_freeform_unless_trusted_requires_patch_approval", approval_policy: UnlessTrusted, sandbox_policy: workspace_write(false), - action: ActionKind::ApplyPatchFunction { - target: TargetPath::Workspace("apply_patch_function_unless_trusted.txt"), - content: "function-patch-unless-trusted", + action: ActionKind::ApplyPatchFreeform { + target: TargetPath::Workspace("apply_patch_freeform_unless_trusted.txt"), + content: "freeform-patch-unless-trusted", }, sandbox_permissions: SandboxPermissions::UseDefault, features: vec![], @@ -1451,24 +1451,24 @@ fn scenarios() -> Vec { expected_reason: None, }, expectation: Expectation::PatchApplied { - target: TargetPath::Workspace("apply_patch_function_unless_trusted.txt"), - content: "function-patch-unless-trusted", + target: TargetPath::Workspace("apply_patch_freeform_unless_trusted.txt"), + content: "freeform-patch-unless-trusted", }, }, ScenarioSpec { - name: "apply_patch_function_never_rejects_outside_workspace", + name: "apply_patch_freeform_never_rejects_outside_workspace", approval_policy: Never, sandbox_policy: workspace_write(false), - action: ActionKind::ApplyPatchFunction { - target: TargetPath::OutsideWorkspace("apply_patch_function_never.txt"), - content: "function-patch-never", + action: ActionKind::ApplyPatchFreeform { + target: TargetPath::OutsideWorkspace("apply_patch_freeform_never.txt"), + content: "freeform-patch-never", }, sandbox_permissions: SandboxPermissions::UseDefault, features: vec![], model_override: Some("gpt-5.4"), outcome: Outcome::Auto, expectation: Expectation::FileNotCreated { - target: TargetPath::OutsideWorkspace("apply_patch_function_never.txt"), + target: TargetPath::OutsideWorkspace("apply_patch_freeform_never.txt"), message_contains: &[ "patch rejected: writing outside of the project; rejected by user approval settings", ], @@ -1812,7 +1812,7 @@ async fn run_scenario_group(group: ScenarioGroup) -> Result<()> { fn scenario_group(scenario: &ScenarioSpec) -> ScenarioGroup { match &scenario.action { - ActionKind::ApplyPatchFunction { .. } | ActionKind::ApplyPatchShell { .. } => { + ActionKind::ApplyPatchFreeform { .. } | ActionKind::ApplyPatchShell { .. } => { ScenarioGroup::ApplyPatch } ActionKind::RunUnifiedExecCommand { .. } => ScenarioGroup::UnifiedExec, @@ -1982,7 +1982,12 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { } } - let output_item = results_mock.single_request().function_call_output(call_id); + let output_request = results_mock.single_request(); + let output_item = if matches!(scenario.action, ActionKind::ApplyPatchFreeform { .. }) { + output_request.custom_tool_call_output(call_id) + } else { + output_request.function_call_output(call_id) + }; let result = parse_result(&output_item); eprintln!( "approval scenario {} result: exit_code={:?} stdout={:?}", @@ -2035,7 +2040,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file() &server, sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id_1, &patch_add), + ev_apply_patch_custom_tool_call(call_id_1, &patch_add), ev_completed("resp-1"), ]), ) @@ -2070,7 +2075,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file() &server, sse(vec![ ev_response_created("resp-3"), - ev_apply_patch_function_call(call_id_2, &patch_update), + ev_apply_patch_custom_tool_call(call_id_2, &patch_update), ev_completed("resp-3"), ]), ) diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index 12669a601c..cc31aa5c77 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -7,7 +7,7 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::ReviewRequest; use codex_protocol::protocol::ReviewTarget; -use core_test_support::responses::ev_apply_patch_function_call; +use core_test_support::responses::ev_apply_patch_custom_tool_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -127,7 +127,7 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() { let patch = "*** Begin Patch\n*** Add File: delegated.txt\n+hello\n*** End Patch\n"; let sse1 = sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, patch), + ev_apply_patch_custom_tool_call(call_id, patch), ev_completed("resp-1"), ]); let review_json = serde_json::json!({ diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 92c8c10a0f..caf4642b0c 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -23,7 +23,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::hooks::trust_discovered_hooks; use core_test_support::hooks::trust_hooks; use core_test_support::managed_network_requirements_loader; -use core_test_support::responses::ev_apply_patch_function_call; +use core_test_support::responses::ev_apply_patch_custom_tool_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -1524,7 +1524,7 @@ async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result vec![ sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, &patch), + ev_apply_patch_custom_tool_call(call_id, &patch), ev_completed("resp-1"), ]), sse(vec![ @@ -1563,7 +1563,7 @@ async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result let requests = responses.requests(); assert_eq!(requests.len(), 2); - requests[1].function_call_output(call_id); + requests[1].custom_tool_call_output(call_id); assert!( target_path.exists(), "approved apply_patch should create the out-of-workspace file" @@ -2619,7 +2619,7 @@ async fn pre_tool_use_blocks_apply_patch_before_execution() -> Result<()> { vec![ sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, &patch), + ev_apply_patch_custom_tool_call(call_id, &patch), ev_completed("resp-1"), ]), sse(vec![ @@ -2652,7 +2652,7 @@ async fn pre_tool_use_blocks_apply_patch_before_execution() -> Result<()> { let requests = responses.requests(); assert_eq!(requests.len(), 2); - let output_item = requests[1].function_call_output(call_id); + let output_item = requests[1].custom_tool_call_output(call_id); let output = output_item .get("output") .and_then(Value::as_str) @@ -2693,7 +2693,7 @@ async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> { vec![ sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, &patch), + ev_apply_patch_custom_tool_call(call_id, &patch), ev_completed("resp-1"), ]), sse(vec![ @@ -2724,7 +2724,7 @@ async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> { let requests = responses.requests(); assert_eq!(requests.len(), 2); - let output_item = requests[1].function_call_output(call_id); + let output_item = requests[1].custom_tool_call_output(call_id); let output = output_item .get("output") .and_then(Value::as_str) @@ -3363,7 +3363,7 @@ async fn post_tool_use_records_additional_context_for_apply_patch() -> Result<() vec![ sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, &patch), + ev_apply_patch_custom_tool_call(call_id, &patch), ev_completed("resp-1"), ]), sse(vec![ @@ -3451,7 +3451,7 @@ async fn post_tool_use_records_apply_patch_context_with_edit_alias() -> Result<( vec![ sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, &patch), + ev_apply_patch_custom_tool_call(call_id, &patch), ev_completed("resp-1"), ]), sse(vec![ diff --git a/codex-rs/core/tests/suite/request_permissions_tool.rs b/codex-rs/core/tests/suite/request_permissions_tool.rs index 94465c18da..30b887ab08 100644 --- a/codex-rs/core/tests/suite/request_permissions_tool.rs +++ b/codex-rs/core/tests/suite/request_permissions_tool.rs @@ -17,7 +17,7 @@ use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; -use core_test_support::responses::ev_apply_patch_function_call; +use core_test_support::responses::ev_apply_patch_custom_tool_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -394,7 +394,7 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu ]), sse(vec![ ev_response_created(&format!("{response_prefix}-2")), - ev_apply_patch_function_call("apply-patch-call", &patch), + ev_apply_patch_custom_tool_call("apply-patch-call", &patch), ev_completed(&format!("{response_prefix}-2")), ]), ]; @@ -475,7 +475,22 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu } let patch_output = responses - .function_call_output_text("apply-patch-call") + .requests() + .into_iter() + .find_map(|request| { + request + .input() + .into_iter() + .find(|item| { + item.get("type").and_then(Value::as_str) == Some("custom_tool_call_output") + && item.get("call_id").and_then(Value::as_str) == Some("apply-patch-call") + }) + .and_then(|item| { + item.get("output") + .and_then(Value::as_str) + .map(str::to_string) + }) + }) .map(|output| json!({ "output": output })) .unwrap_or_else(|| panic!("expected apply-patch-call output")); let (exit_code, stdout) = parse_result(&patch_output); diff --git a/codex-rs/core/tests/suite/shell_serialization.rs b/codex-rs/core/tests/suite/shell_serialization.rs index ed474114ff..01b1563a6b 100644 --- a/codex-rs/core/tests/suite/shell_serialization.rs +++ b/codex-rs/core/tests/suite/shell_serialization.rs @@ -472,7 +472,6 @@ $"#; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_custom_tool_output_is_structured( @@ -518,7 +517,6 @@ A {file_name} #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_custom_tool_call_creates_file( @@ -566,7 +564,6 @@ A {file_name} #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_custom_tool_call_updates_existing_file( @@ -619,7 +616,6 @@ M {file_name} #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_custom_tool_call_reports_failure_output( @@ -664,20 +660,17 @@ async fn apply_patch_custom_tool_call_reports_failure_output( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Function)] #[test_case(ApplyPatchModelOutput::Shell)] #[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] -async fn apply_patch_function_call_output_is_structured( - output_type: ApplyPatchModelOutput, -) -> Result<()> { +async fn apply_patch_tool_output_is_structured(output_type: ApplyPatchModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); let harness = apply_patch_harness().await?; let call_id = "apply-patch-function"; - let file_name = "function_apply_patch.txt"; + let file_name = "freeform_apply_patch.txt"; let patch = - format!("*** Begin Patch\n*** Add File: {file_name}\n+via function call\n*** End Patch\n"); + format!("*** Begin Patch\n*** Add File: {file_name}\n+via apply_patch\n*** End Patch\n"); mount_apply_patch( &harness, call_id, @@ -689,7 +682,7 @@ async fn apply_patch_function_call_output_is_structured( harness .test() .submit_turn_with_permission_profile( - "apply the patch via function-call apply_patch", + "apply the patch via freeform apply_patch", PermissionProfile::Disabled, ) .await?; diff --git a/codex-rs/core/tests/suite/tool_harness.rs b/codex-rs/core/tests/suite/tool_harness.rs index a69ec3f7f6..a2017c99b3 100644 --- a/codex-rs/core/tests/suite/tool_harness.rs +++ b/codex-rs/core/tests/suite/tool_harness.rs @@ -14,7 +14,7 @@ use codex_protocol::user_input::UserInput; use core_test_support::assert_regex_match; use core_test_support::responses; use core_test_support::responses::ResponsesRequest; -use core_test_support::responses::ev_apply_patch_function_call; +use core_test_support::responses::ev_apply_patch_custom_tool_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -47,6 +47,24 @@ fn call_output(req: &ResponsesRequest, call_id: &str) -> (String, Option) (content, success) } +fn custom_call_output(req: &ResponsesRequest, call_id: &str) -> (String, Option) { + let raw = req.custom_tool_call_output(call_id); + assert_eq!( + raw.get("call_id").and_then(Value::as_str), + Some(call_id), + "mismatched call_id in custom_tool_call_output" + ); + let (content_opt, success) = match req.custom_tool_call_output_content_and_success(call_id) { + Some(values) => values, + None => panic!("custom_tool_call_output present"), + }; + let content = match content_opt { + Some(c) => c, + None => panic!("custom_tool_call_output content present"), + }; + (content, success) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn shell_tool_executes_command_and_streams_output() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); @@ -107,10 +125,10 @@ async fn shell_tool_executes_command_and_streams_output() -> anyhow::Result<()> let req = second_mock.single_request(); let (output_text, _) = call_output(&req, call_id); - let exec_output: Value = serde_json::from_str(&output_text)?; - assert_eq!(exec_output["metadata"]["exit_code"], 0); - let stdout = exec_output["output"].as_str().expect("stdout field"); - assert_regex_match(r"(?s)^tool harness\n?$", stdout); + assert_regex_match( + r"(?s)^Exit code: 0\nWall time: [0-9]+(?:\.[0-9]+)? seconds\nOutput:\ntool harness\n?$", + &output_text, + ); Ok(()) } @@ -328,7 +346,7 @@ async fn apply_patch_tool_executes_and_emits_patch_events() -> anyhow::Result<() let first_response = sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, &patch_content), + ev_apply_patch_custom_tool_call(call_id, &patch_content), ev_completed("resp-1"), ]); responses::mount_sse_once(&server, first_response).await; @@ -419,7 +437,7 @@ async fn apply_patch_tool_executes_and_emits_patch_events() -> anyhow::Result<() assert!(patch_end_success); let req = second_mock.single_request(); - let (output_text, _success_flag) = call_output(&req, call_id); + let (output_text, _success_flag) = custom_call_output(&req, call_id); let expected_pattern = format!( r"(?s)^Exit code: 0 @@ -466,7 +484,7 @@ async fn apply_patch_reports_parse_diagnostics() -> anyhow::Result<()> { let first_response = sse(vec![ ev_response_created("resp-1"), - ev_apply_patch_function_call(call_id, patch_content), + ev_apply_patch_custom_tool_call(call_id, patch_content), ev_completed("resp-1"), ]); responses::mount_sse_once(&server, first_response).await; @@ -507,7 +525,7 @@ async fn apply_patch_reports_parse_diagnostics() -> anyhow::Result<()> { wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; let req = second_mock.single_request(); - let (output_text, success_flag) = call_output(&req, call_id); + let (output_text, success_flag) = custom_call_output(&req, call_id); assert!( output_text.contains("apply_patch verification failed"), diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index 7ba76eaa4d..e69cf35da3 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -433,15 +433,10 @@ async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> { .function_call_output_content_and_success(call_id_success) .and_then(|(content, _)| content) .expect("success output string"); - let output_json: Value = serde_json::from_str(&success_output)?; - assert_eq!( - output_json["metadata"]["exit_code"].as_i64(), - Some(0), - "expected exit code 0 after rerunning without escalation", + assert_regex_match( + r"(?s)^Exit code: 0\nWall time: [0-9]+(?:\.[0-9]+)? seconds\nOutput:\nshell ok\n?$", + &success_output, ); - let stdout = output_json["output"].as_str().unwrap_or_default(); - let stdout_pattern = r"(?s)^shell ok\n?$"; - assert_regex_match(stdout_pattern, stdout); Ok(()) } diff --git a/codex-rs/exec/tests/suite/apply_patch.rs b/codex-rs/exec/tests/suite/apply_patch.rs index 837c55c4c0..3b0006695e 100644 --- a/codex-rs/exec/tests/suite/apply_patch.rs +++ b/codex-rs/exec/tests/suite/apply_patch.rs @@ -4,7 +4,6 @@ use anyhow::Context; use assert_cmd::prelude::*; use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; use core_test_support::responses::ev_apply_patch_custom_tool_call; -use core_test_support::responses::ev_apply_patch_function_call; use core_test_support::responses::ev_completed; use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; @@ -71,7 +70,7 @@ async fn test_apply_patch_tool() -> anyhow::Result<()> { ev_completed("request_0"), ]), sse(vec![ - ev_apply_patch_function_call("request_1", update_patch), + ev_apply_patch_custom_tool_call("request_1", update_patch), ev_completed("request_1"), ]), sse(vec![ev_completed("request_2")]), diff --git a/codex-rs/model-provider/src/amazon_bedrock/catalog.rs b/codex-rs/model-provider/src/amazon_bedrock/catalog.rs index 153c2db2ef..dc286fe6d6 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/catalog.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/catalog.rs @@ -63,7 +63,7 @@ fn gpt_5_4_cmb_bedrock_model(priority: i32) -> ModelInfo { default_reasoning_summary: ReasoningSummary::None, support_verbosity: true, default_verbosity: Some(Verbosity::Medium), - apply_patch_tool_type: Some(ApplyPatchToolType::Function), + apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), web_search_tool_type: WebSearchToolType::TextAndImage, truncation_policy: TruncationPolicyConfig::tokens(/*limit*/ 10_000), supports_parallel_tool_calls: true, diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index 96179440bd..d51e70ddf1 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -203,7 +203,6 @@ pub enum ConfigShellToolType { #[serde(rename_all = "snake_case")] pub enum ApplyPatchToolType { Freeform, - Function, } #[derive( diff --git a/codex-rs/tools/src/tool_config.rs b/codex-rs/tools/src/tool_config.rs index 2ebf0540f7..4639b404b1 100644 --- a/codex-rs/tools/src/tool_config.rs +++ b/codex-rs/tools/src/tool_config.rs @@ -219,11 +219,10 @@ impl ToolsConfig { model_shell_type }; - let apply_patch_tool_type = match model_info.apply_patch_tool_type { - Some(ApplyPatchToolType::Freeform) => Some(ApplyPatchToolType::Freeform), - Some(ApplyPatchToolType::Function) => Some(ApplyPatchToolType::Function), - None => include_apply_patch_tool.then_some(ApplyPatchToolType::Function), - }; + let apply_patch_tool_type = model_info + .apply_patch_tool_type + .clone() + .or_else(|| include_apply_patch_tool.then_some(ApplyPatchToolType::Freeform)); let agent_jobs_worker_tools = include_agent_jobs && matches!( diff --git a/codex-rs/tools/src/tool_config_tests.rs b/codex-rs/tools/src/tool_config_tests.rs index 3d1829c4e5..252ad7a320 100644 --- a/codex-rs/tools/src/tool_config_tests.rs +++ b/codex-rs/tools/src/tool_config_tests.rs @@ -156,7 +156,7 @@ fn shell_zsh_fork_prefers_shell_command_over_unified_exec() { } #[test] -fn fallback_apply_patch_models_use_function_tool_by_default() { +fn fallback_apply_patch_models_use_freeform_tool_by_default() { let model_info = model_info(); let features = Features::with_defaults(); @@ -174,7 +174,7 @@ fn fallback_apply_patch_models_use_function_tool_by_default() { assert_eq!( tools_config.apply_patch_tool_type, - Some(ApplyPatchToolType::Function) + Some(ApplyPatchToolType::Freeform) ); }