From 236c4f76a6fd49799ec148b2bb37495a8fc5c1f5 Mon Sep 17 00:00:00 2001 From: Dylan Date: Fri, 22 Aug 2025 13:42:34 -0700 Subject: [PATCH] [apply_patch] freeform apply_patch tool (#2576) ## Summary GPT-5 introduced the concept of [custom tools](https://platform.openai.com/docs/guides/function-calling#custom-tools), which allow the model to send a raw string result back, simplifying json-escape issues. We are migrating gpt-5 to use this by default. However, gpt-oss models do not support custom tools, only normal functions. So we keep both tool definitions, and provide whichever one the model family supports. ## Testing - [x] Tested locally with various models - [x] Unit tests pass --- codex-rs/core/src/chat_completions.rs | 27 ++++ codex-rs/core/src/client.rs | 2 + codex-rs/core/src/client_common.rs | 13 +- codex-rs/core/src/codex.rs | 97 +++++++++++++- codex-rs/core/src/config.rs | 7 +- codex-rs/core/src/conversation_history.rs | 2 + codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/model_family.rs | 16 ++- codex-rs/core/src/models.rs | 21 +++ codex-rs/core/src/openai_tools.rs | 153 ++++++++-------------- codex-rs/core/src/rollout.rs | 6 + codex-rs/core/src/tool_apply_patch.rs | 145 ++++++++++++++++++++ codex-rs/core/tests/prompt_caching.rs | 33 +++-- codex-rs/exec/tests/apply_patch.rs | 149 +++++++++++++++++++++ 14 files changed, 534 insertions(+), 138 deletions(-) create mode 100644 codex-rs/core/src/tool_apply_patch.rs diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index f3ed34c6..7131b8f4 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -102,6 +102,33 @@ pub(crate) async fn stream_chat_completions( "content": output.content, })); } + ResponseItem::CustomToolCall { + id, + call_id: _, + name, + input, + status: _, + } => { + messages.push(json!({ + "role": "assistant", + "content": null, + "tool_calls": [{ + "id": id, + "type": "custom", + "custom": { + "name": name, + "input": input, + } + }] + })); + } + ResponseItem::CustomToolCallOutput { call_id, output } => { + messages.push(json!({ + "role": "tool", + "tool_call_id": call_id, + "content": output, + })); + } ResponseItem::Reasoning { .. } | ResponseItem::Other => { // Omit these items from the conversation history. continue; diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 174ac58f..43f3f1de 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -575,6 +575,8 @@ async fn process_sse( } "response.content_part.done" | "response.function_call_arguments.delta" + | "response.custom_tool_call_input.delta" + | "response.custom_tool_call_input.done" // also emitted as response.output_item.done | "response.in_progress" | "response.output_item.added" | "response.output_text.done" => { diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index f99fe945..c9a8a429 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -49,13 +49,14 @@ impl Prompt { .unwrap_or(BASE_INSTRUCTIONS); let mut sections: Vec<&str> = vec![base]; - // When there are no custom instructions, add apply_patch if either: - // - the model needs special instructions, or + // When there are no custom instructions, add apply_patch_tool_instructions if either: + // - the model needs special instructions (4.1), or // - there is no apply_patch tool present - let is_apply_patch_tool_present = self - .tools - .iter() - .any(|t| matches!(t, OpenAiTool::Function(f) if f.name == "apply_patch")); + let is_apply_patch_tool_present = self.tools.iter().any(|tool| match tool { + OpenAiTool::Function(f) => f.name == "apply_patch", + OpenAiTool::Freeform(f) => f.name == "apply_patch", + _ => false, + }); if self.base_instructions_override.is_none() && (model.needs_special_apply_patch_instructions || !is_apply_patch_tool_present) { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index f30ebcd3..4188a5df 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1406,6 +1406,18 @@ async fn run_task( }, ); } + ( + ResponseItem::CustomToolCall { .. }, + Some(ResponseInputItem::CustomToolCallOutput { call_id, output }), + ) => { + items_to_record_in_conversation_history.push(item); + items_to_record_in_conversation_history.push( + ResponseItem::CustomToolCallOutput { + call_id: call_id.clone(), + output: output.clone(), + }, + ); + } ( ResponseItem::FunctionCall { .. }, Some(ResponseInputItem::McpToolCallOutput { call_id, result }), @@ -1586,6 +1598,7 @@ async fn try_run_turn( call_id: Some(call_id), .. } => Some(call_id), + ResponseItem::CustomToolCallOutput { call_id, .. } => Some(call_id), _ => None, }) .collect::>(); @@ -1603,6 +1616,7 @@ async fn try_run_turn( call_id: Some(call_id), .. } => Some(call_id), + ResponseItem::CustomToolCall { call_id, .. } => Some(call_id), _ => None, }) .filter_map(|call_id| { @@ -1612,12 +1626,9 @@ async fn try_run_turn( Some(call_id.clone()) } }) - .map(|call_id| ResponseItem::FunctionCallOutput { + .map(|call_id| ResponseItem::CustomToolCallOutput { call_id: call_id.clone(), - output: FunctionCallOutputPayload { - content: "aborted".to_string(), - success: Some(false), - }, + output: "aborted".to_string(), }) .collect::>() }; @@ -1882,7 +1893,7 @@ async fn handle_response_item( call_id, .. } => { - info!("FunctionCall: {arguments}"); + info!("FunctionCall: {name}({arguments})"); Some( handle_function_call( sess, @@ -1939,10 +1950,32 @@ async fn handle_response_item( .await, ) } + ResponseItem::CustomToolCall { + id: _, + call_id, + name, + input, + status: _, + } => Some( + handle_custom_tool_call( + sess, + turn_context, + turn_diff_tracker, + sub_id.to_string(), + name, + input, + call_id, + ) + .await, + ), ResponseItem::FunctionCallOutput { .. } => { debug!("unexpected FunctionCallOutput from stream"); None } + ResponseItem::CustomToolCallOutput { .. } => { + debug!("unexpected CustomToolCallOutput from stream"); + None + } ResponseItem::Other => None, }; Ok(output) @@ -2032,6 +2065,58 @@ async fn handle_function_call( } } +async fn handle_custom_tool_call( + sess: &Session, + turn_context: &TurnContext, + turn_diff_tracker: &mut TurnDiffTracker, + sub_id: String, + name: String, + input: String, + call_id: String, +) -> ResponseInputItem { + info!("CustomToolCall: {name} {input}"); + match name.as_str() { + "apply_patch" => { + let exec_params = ExecParams { + command: vec!["apply_patch".to_string(), input.clone()], + cwd: turn_context.cwd.clone(), + timeout_ms: None, + env: HashMap::new(), + with_escalated_permissions: None, + justification: None, + }; + let resp = handle_container_exec_with_params( + exec_params, + sess, + turn_context, + turn_diff_tracker, + sub_id, + call_id, + ) + .await; + + // Convert function-call style output into a custom tool call output + match resp { + ResponseInputItem::FunctionCallOutput { call_id, output } => { + ResponseInputItem::CustomToolCallOutput { + call_id, + output: output.content, + } + } + // Pass through if already a custom tool output or other variant + other => other, + } + } + _ => { + debug!("unexpected CustomToolCall from stream"); + ResponseInputItem::CustomToolCallOutput { + call_id, + output: format!("unsupported custom tool call: {name}"), + } + } + } +} + fn to_exec_params(params: ShellToolCallParams, turn_context: &TurnContext) -> ExecParams { ExecParams { command: params.command, diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 31ff5f10..988d4767 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -646,7 +646,7 @@ impl Config { needs_special_apply_patch_instructions: false, supports_reasoning_summaries, uses_local_shell_tool: false, - uses_apply_patch_tool: false, + apply_patch_tool_type: None, } }); @@ -673,9 +673,6 @@ impl Config { Self::get_base_instructions(experimental_instructions_path, &resolved_cwd)?; let base_instructions = base_instructions.or(file_base_instructions); - let include_apply_patch_tool_val = - include_apply_patch_tool.unwrap_or(model_family.uses_apply_patch_tool); - let responses_originator_header: String = cfg .responses_originator_header_internal_override .unwrap_or(DEFAULT_RESPONSES_ORIGINATOR_HEADER.to_owned()); @@ -732,7 +729,7 @@ impl Config { experimental_resume, include_plan_tool: include_plan_tool.unwrap_or(false), - include_apply_patch_tool: include_apply_patch_tool_val, + include_apply_patch_tool: include_apply_patch_tool.unwrap_or(false), responses_originator_header, preferred_auth_method: cfg.preferred_auth_method.unwrap_or(AuthMode::ChatGPT), }; diff --git a/codex-rs/core/src/conversation_history.rs b/codex-rs/core/src/conversation_history.rs index 1d55b125..5955ce44 100644 --- a/codex-rs/core/src/conversation_history.rs +++ b/codex-rs/core/src/conversation_history.rs @@ -110,6 +110,8 @@ fn is_api_message(message: &ResponseItem) -> bool { ResponseItem::Message { role, .. } => role.as_str() != "system", ResponseItem::FunctionCallOutput { .. } | ResponseItem::FunctionCall { .. } + | ResponseItem::CustomToolCall { .. } + | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::LocalShellCall { .. } | ResponseItem::Reasoning { .. } => true, ResponseItem::Other => false, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 40ccbf67..37f30a72 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -50,6 +50,7 @@ pub mod seatbelt; pub mod shell; pub mod spawn; pub mod terminal; +mod tool_apply_patch; pub mod turn_diff_tracker; pub mod user_agent; mod user_notification; diff --git a/codex-rs/core/src/model_family.rs b/codex-rs/core/src/model_family.rs index 6d1c2efc..5b252e66 100644 --- a/codex-rs/core/src/model_family.rs +++ b/codex-rs/core/src/model_family.rs @@ -1,3 +1,5 @@ +use crate::tool_apply_patch::ApplyPatchToolType; + /// A model family is a group of models that share certain characteristics. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct ModelFamily { @@ -24,9 +26,9 @@ pub struct ModelFamily { // See https://platform.openai.com/docs/guides/tools-local-shell pub uses_local_shell_tool: bool, - /// True if the model performs better when `apply_patch` is provided as - /// a tool call instead of just a bash command. - pub uses_apply_patch_tool: bool, + /// Present if the model performs better when `apply_patch` is provided as + /// a tool call instead of just a bash command + pub apply_patch_tool_type: Option, } macro_rules! model_family { @@ -40,7 +42,7 @@ macro_rules! model_family { needs_special_apply_patch_instructions: false, supports_reasoning_summaries: false, uses_local_shell_tool: false, - uses_apply_patch_tool: false, + apply_patch_tool_type: None, }; // apply overrides $( @@ -60,7 +62,7 @@ macro_rules! simple_model_family { needs_special_apply_patch_instructions: false, supports_reasoning_summaries: false, uses_local_shell_tool: false, - uses_apply_patch_tool: false, + apply_patch_tool_type: None, }) }}; } @@ -88,6 +90,7 @@ pub fn find_family_for_model(slug: &str) -> Option { model_family!( slug, slug, supports_reasoning_summaries: true, + apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), ) } else if slug.starts_with("gpt-4.1") { model_family!( @@ -95,7 +98,7 @@ pub fn find_family_for_model(slug: &str) -> Option { needs_special_apply_patch_instructions: true, ) } else if slug.starts_with("gpt-oss") { - model_family!(slug, "gpt-oss", uses_apply_patch_tool: true) + model_family!(slug, "gpt-oss", apply_patch_tool_type: Some(ApplyPatchToolType::Function)) } else if slug.starts_with("gpt-4o") { simple_model_family!(slug, "gpt-4o") } else if slug.starts_with("gpt-3.5") { @@ -104,6 +107,7 @@ pub fn find_family_for_model(slug: &str) -> Option { model_family!( slug, "gpt-5", supports_reasoning_summaries: true, + apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), ) } else { None diff --git a/codex-rs/core/src/models.rs b/codex-rs/core/src/models.rs index 0c509ee2..aa092e18 100644 --- a/codex-rs/core/src/models.rs +++ b/codex-rs/core/src/models.rs @@ -24,6 +24,10 @@ pub enum ResponseInputItem { call_id: String, result: Result, }, + CustomToolCallOutput { + call_id: String, + output: String, + }, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -77,6 +81,20 @@ pub enum ResponseItem { call_id: String, output: FunctionCallOutputPayload, }, + CustomToolCall { + #[serde(default, skip_serializing_if = "Option::is_none")] + id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + status: Option, + + call_id: String, + name: String, + input: String, + }, + CustomToolCallOutput { + call_id: String, + output: String, + }, #[serde(other)] Other, } @@ -114,6 +132,9 @@ impl From for ResponseItem { ), }, }, + ResponseInputItem::CustomToolCallOutput { call_id, output } => { + Self::CustomToolCallOutput { call_id, output } + } } } } diff --git a/codex-rs/core/src/openai_tools.rs b/codex-rs/core/src/openai_tools.rs index df60d0c3..bb5e6dac 100644 --- a/codex-rs/core/src/openai_tools.rs +++ b/codex-rs/core/src/openai_tools.rs @@ -9,6 +9,9 @@ use crate::model_family::ModelFamily; use crate::plan_tool::PLAN_TOOL; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; +use crate::tool_apply_patch::ApplyPatchToolType; +use crate::tool_apply_patch::create_apply_patch_freeform_tool; +use crate::tool_apply_patch::create_apply_patch_json_tool; #[derive(Debug, Clone, Serialize, PartialEq)] pub struct ResponsesApiTool { @@ -21,6 +24,20 @@ pub struct ResponsesApiTool { pub(crate) parameters: JsonSchema, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct FreeformTool { + pub(crate) name: String, + pub(crate) description: String, + pub(crate) format: FreeformToolFormat, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct FreeformToolFormat { + pub(crate) r#type: String, + pub(crate) syntax: String, + pub(crate) definition: String, +} + /// When serialized as JSON, this produces a valid "Tool" in the OpenAI /// Responses API. #[derive(Debug, Clone, Serialize, PartialEq)] @@ -30,6 +47,8 @@ pub(crate) enum OpenAiTool { Function(ResponsesApiTool), #[serde(rename = "local_shell")] LocalShell {}, + #[serde(rename = "custom")] + Freeform(FreeformTool), } #[derive(Debug, Clone)] @@ -43,7 +62,7 @@ pub enum ConfigShellToolType { pub struct ToolsConfig { pub shell_type: ConfigShellToolType, pub plan_tool: bool, - pub apply_patch_tool: bool, + pub apply_patch_tool_type: Option, } impl ToolsConfig { @@ -65,10 +84,22 @@ impl ToolsConfig { } } + let apply_patch_tool_type = match model_family.apply_patch_tool_type { + Some(ApplyPatchToolType::Freeform) => Some(ApplyPatchToolType::Freeform), + Some(ApplyPatchToolType::Function) => Some(ApplyPatchToolType::Function), + None => { + if include_apply_patch_tool { + Some(ApplyPatchToolType::Freeform) + } else { + None + } + } + }; + Self { shell_type, plan_tool: include_plan_tool, - apply_patch_tool: include_apply_patch_tool || model_family.uses_apply_patch_tool, + apply_patch_tool_type, } } } @@ -241,102 +272,12 @@ The shell tool is used to execute shell commands. }, }) } - +/// TODO(dylan): deprecate once we get rid of json tool #[derive(Serialize, Deserialize)] pub(crate) struct ApplyPatchToolArgs { pub(crate) input: String, } -/// Returns a JSON tool that can be used to edit files. Public for testing, please use `get_openai_tools`. -fn create_apply_patch_json_tool() -> OpenAiTool { - let mut properties = BTreeMap::new(); - properties.insert( - "input".to_string(), - JsonSchema::String { - description: Some(r#"The entire contents of the apply_patch command"#.to_string()), - }, - ); - - OpenAiTool::Function(ResponsesApiTool { - name: "apply_patch".to_string(), - description: 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. -"# - .to_string(), - strict: false, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["input".to_string()]), - additional_properties: Some(false), - }, - }) -} - /// Returns JSON values that are compatible with Function Calling in the /// Responses API: /// https://platform.openai.com/docs/guides/function-calling?api-mode=responses @@ -557,8 +498,15 @@ pub(crate) fn get_openai_tools( tools.push(PLAN_TOOL.clone()); } - if config.apply_patch_tool { - tools.push(create_apply_patch_json_tool()); + if let Some(apply_patch_tool_type) = &config.apply_patch_tool_type { + match apply_patch_tool_type { + ApplyPatchToolType::Freeform => { + tools.push(create_apply_patch_freeform_tool()); + } + ApplyPatchToolType::Function => { + tools.push(create_apply_patch_json_tool()); + } + } } if let Some(mcp_tools) = mcp_tools { @@ -589,6 +537,7 @@ mod tests { .map(|tool| match tool { OpenAiTool::Function(ResponsesApiTool { name, .. }) => name, OpenAiTool::LocalShell {} => "local_shell", + OpenAiTool::Freeform(FreeformTool { name, .. }) => name, }) .collect::>(); @@ -614,7 +563,7 @@ mod tests { AskForApproval::Never, SandboxPolicy::ReadOnly, true, - model_family.uses_apply_patch_tool, + false, ); let tools = get_openai_tools(&config, Some(HashMap::new())); @@ -629,7 +578,7 @@ mod tests { AskForApproval::Never, SandboxPolicy::ReadOnly, true, - model_family.uses_apply_patch_tool, + false, ); let tools = get_openai_tools(&config, Some(HashMap::new())); @@ -644,7 +593,7 @@ mod tests { AskForApproval::Never, SandboxPolicy::ReadOnly, false, - model_family.uses_apply_patch_tool, + false, ); let tools = get_openai_tools( &config, @@ -738,7 +687,7 @@ mod tests { AskForApproval::Never, SandboxPolicy::ReadOnly, false, - model_family.uses_apply_patch_tool, + false, ); let tools = get_openai_tools( @@ -794,7 +743,7 @@ mod tests { AskForApproval::Never, SandboxPolicy::ReadOnly, false, - model_family.uses_apply_patch_tool, + false, ); let tools = get_openai_tools( @@ -845,7 +794,7 @@ mod tests { AskForApproval::Never, SandboxPolicy::ReadOnly, false, - model_family.uses_apply_patch_tool, + false, ); let tools = get_openai_tools( @@ -899,7 +848,7 @@ mod tests { AskForApproval::Never, SandboxPolicy::ReadOnly, false, - model_family.uses_apply_patch_tool, + false, ); let tools = get_openai_tools( diff --git a/codex-rs/core/src/rollout.rs b/codex-rs/core/src/rollout.rs index 0ccd8e89..f5a62ef8 100644 --- a/codex-rs/core/src/rollout.rs +++ b/codex-rs/core/src/rollout.rs @@ -132,6 +132,8 @@ impl RolloutRecorder { | ResponseItem::LocalShellCall { .. } | ResponseItem::FunctionCall { .. } | ResponseItem::FunctionCallOutput { .. } + | ResponseItem::CustomToolCall { .. } + | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::Reasoning { .. } => filtered.push(item.clone()), ResponseItem::Other => { // These should never be serialized. @@ -194,6 +196,8 @@ impl RolloutRecorder { | ResponseItem::LocalShellCall { .. } | ResponseItem::FunctionCall { .. } | ResponseItem::FunctionCallOutput { .. } + | ResponseItem::CustomToolCall { .. } + | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::Reasoning { .. } => items.push(item), ResponseItem::Other => {} }, @@ -317,6 +321,8 @@ async fn rollout_writer( | ResponseItem::LocalShellCall { .. } | ResponseItem::FunctionCall { .. } | ResponseItem::FunctionCallOutput { .. } + | ResponseItem::CustomToolCall { .. } + | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::Reasoning { .. } => { writer.write_line(&item).await?; } diff --git a/codex-rs/core/src/tool_apply_patch.rs b/codex-rs/core/src/tool_apply_patch.rs new file mode 100644 index 00000000..3868cf0e --- /dev/null +++ b/codex-rs/core/src/tool_apply_patch.rs @@ -0,0 +1,145 @@ +use serde::Deserialize; +use serde::Serialize; +use std::collections::BTreeMap; + +use crate::openai_tools::FreeformTool; +use crate::openai_tools::FreeformToolFormat; +use crate::openai_tools::JsonSchema; +use crate::openai_tools::OpenAiTool; +use crate::openai_tools::ResponsesApiTool; + +#[derive(Serialize, Deserialize)] +pub(crate) struct ApplyPatchToolArgs { + pub(crate) input: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[serde(rename_all = "snake_case")] +pub enum ApplyPatchToolType { + Freeform, + Function, +} + +/// 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(crate) fn create_apply_patch_freeform_tool() -> OpenAiTool { + OpenAiTool::Freeform(FreeformTool { + name: "apply_patch".to_string(), + description: "Use the `apply_patch` tool to edit files".to_string(), + format: FreeformToolFormat { + r#type: "grammar".to_string(), + syntax: "lark".to_string(), + definition: r#"start: begin_patch hunk+ end_patch +begin_patch: "*** Begin Patch" LF +end_patch: "*** End Patch" LF? + +hunk: add_hunk | delete_hunk | update_hunk +add_hunk: "*** Add File: " filename LF add_line+ +delete_hunk: "*** Delete File: " filename LF +update_hunk: "*** Update File: " filename LF change_move? change? + +filename: /(.+)/ +add_line: "+" /(.+)/ LF -> line + +change_move: "*** Move to: " filename LF +change: (change_context | change_line)+ eof_line? +change_context: ("@@" | "@@ " /(.+)/) LF +change_line: ("+" | "-" | " ") /(.+)/ LF +eof_line: "*** End of File" LF + +%import common.LF +"# + .to_string(), + }, + }) +} + +/// Returns a json tool that can be used to edit files. Should only be used with gpt-oss models +pub(crate) fn create_apply_patch_json_tool() -> OpenAiTool { + let mut properties = BTreeMap::new(); + properties.insert( + "input".to_string(), + JsonSchema::String { + description: Some(r#"The entire contents of the apply_patch command"#.to_string()), + }, + ); + + OpenAiTool::Function(ResponsesApiTool { + name: "apply_patch".to_string(), + description: 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. +"# + .to_string(), + strict: false, + parameters: JsonSchema::Object { + properties, + required: Some(vec!["input".to_string()]), + additional_properties: Some(false), + }, + }) +} diff --git a/codex-rs/core/tests/prompt_caching.rs b/codex-rs/core/tests/prompt_caching.rs index 958aff7b..17965e49 100644 --- a/codex-rs/core/tests/prompt_caching.rs +++ b/codex-rs/core/tests/prompt_caching.rs @@ -1,6 +1,9 @@ +#![allow(clippy::unwrap_used)] + use codex_core::ConversationManager; use codex_core::ModelProviderInfo; use codex_core::built_in_model_providers; +use codex_core::model_family::find_family_for_model; use codex_core::protocol::AskForApproval; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; @@ -25,8 +28,20 @@ fn sse_completed(id: &str) -> String { load_sse_fixture_with_id("tests/fixtures/completed_template.json", id) } +fn assert_tool_names(body: &serde_json::Value, expected_names: &[&str]) { + assert_eq!( + body["tools"] + .as_array() + .unwrap() + .iter() + .map(|t| t["name"].as_str().unwrap().to_string()) + .collect::>(), + expected_names + ); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn default_system_instructions_contain_apply_patch() { +async fn codex_mini_latest_tools() { use pretty_assertions::assert_eq; let server = MockServer::start().await; @@ -58,6 +73,10 @@ async fn default_system_instructions_contain_apply_patch() { 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(); + let codex = conversation_manager .new_conversation(config) .await @@ -173,18 +192,6 @@ async fn prompt_tools_are_consistent_across_requests() { // our internal implementation is responsible for keeping tools in sync // with the OpenAI schema, so we just verify the tool presence here let expected_tools_names: &[&str] = &["shell", "update_plan", "apply_patch"]; - fn assert_tool_names(body: &serde_json::Value, expected_names: &[&str]) { - assert_eq!( - body["tools"] - .as_array() - .unwrap() - .iter() - .map(|t| t["name"].as_str().unwrap().to_string()) - .collect::>(), - expected_names - ); - } - let body0 = requests[0].body_json::().unwrap(); assert_eq!( body0["instructions"], diff --git a/codex-rs/exec/tests/apply_patch.rs b/codex-rs/exec/tests/apply_patch.rs index ecce43d7..f05bb732 100644 --- a/codex-rs/exec/tests/apply_patch.rs +++ b/codex-rs/exec/tests/apply_patch.rs @@ -123,6 +123,155 @@ async fn test_apply_patch_tool() -> anyhow::Result<()> { // Start a mock model server let server = MockServer::start().await; + // First response: model calls apply_patch to create test.md + let first = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw( + load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_ADD, "call1"), + "text/event-stream", + ); + + Mock::given(method("POST")) + // .and(path("/v1/responses")) + .respond_with(first) + .up_to_n_times(1) + .mount(&server) + .await; + + // Second response: model calls apply_patch to update test.md + let second = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw( + load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_UPDATE, "call2"), + "text/event-stream", + ); + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(second) + .up_to_n_times(1) + .mount(&server) + .await; + + let final_completed = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw( + load_sse_fixture_with_id_from_str(SSE_TOOL_CALL_COMPLETED, "resp3"), + "text/event-stream", + ); + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(final_completed) + .expect(1) + .mount(&server) + .await; + + let tmp_cwd = TempDir::new().unwrap(); + Command::cargo_bin("codex-exec") + .context("should find binary for codex-exec")? + .current_dir(tmp_cwd.path()) + .env("CODEX_HOME", tmp_cwd.path()) + .env("OPENAI_API_KEY", "dummy") + .env("OPENAI_BASE_URL", format!("{}/v1", server.uri())) + .arg("--skip-git-repo-check") + .arg("-s") + .arg("workspace-write") + .arg("foo") + .assert() + .success(); + + // Verify final file contents + let final_path = tmp_cwd.path().join("test.md"); + let contents = std::fs::read_to_string(&final_path) + .unwrap_or_else(|e| panic!("failed reading {}: {e}", final_path.display())); + assert_eq!(contents, "Final text\n"); + Ok(()) +} + +#[cfg(not(target_os = "windows"))] +#[tokio::test] +async fn test_apply_patch_freeform_tool() -> anyhow::Result<()> { + use core_test_support::load_sse_fixture_with_id_from_str; + use tempfile::TempDir; + use wiremock::Mock; + use wiremock::MockServer; + use wiremock::ResponseTemplate; + use wiremock::matchers::method; + use wiremock::matchers::path; + + const SSE_TOOL_CALL_ADD: &str = r#"[ + { + "type": "response.output_item.done", + "item": { + "type": "custom_tool_call", + "name": "apply_patch", + "input": "*** Begin Patch\n*** Add File: test.md\n+Hello world\n*** End Patch", + "call_id": "__ID__" + } + }, + { + "type": "response.completed", + "response": { + "id": "__ID__", + "usage": { + "input_tokens": 0, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": 0 + }, + "output": [] + } + } +]"#; + + const SSE_TOOL_CALL_UPDATE: &str = r#"[ + { + "type": "response.output_item.done", + "item": { + "type": "custom_tool_call", + "name": "apply_patch", + "input": "*** Begin Patch\n*** Update File: test.md\n@@\n-Hello world\n+Final text\n*** End Patch", + "call_id": "__ID__" + } + }, + { + "type": "response.completed", + "response": { + "id": "__ID__", + "usage": { + "input_tokens": 0, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": 0 + }, + "output": [] + } + } +]"#; + + const SSE_TOOL_CALL_COMPLETED: &str = r#"[ + { + "type": "response.completed", + "response": { + "id": "__ID__", + "usage": { + "input_tokens": 0, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": 0 + }, + "output": [] + } + } +]"#; + + // Start a mock model server + let server = MockServer::start().await; + // First response: model calls apply_patch to create test.md let first = ResponseTemplate::new(200) .insert_header("content-type", "text/event-stream")