From e9e48a6195ea81772bdb5f3f9afce060ef2be244 Mon Sep 17 00:00:00 2001 From: Reese Date: Fri, 22 May 2026 12:28:28 +0000 Subject: [PATCH 1/2] fix(windows): route apply_patch through the dedicated tool path by default --- code-rs/core/src/config.rs | 19 ++++- code-rs/core/src/openai_tools.rs | 139 ++++++++++++++++++++++++------- 2 files changed, 129 insertions(+), 29 deletions(-) diff --git a/code-rs/core/src/config.rs b/code-rs/core/src/config.rs index 5316a8a1209..9be38961b28 100644 --- a/code-rs/core/src/config.rs +++ b/code-rs/core/src/config.rs @@ -1706,7 +1706,7 @@ impl Config { .or(cfg.chatgpt_base_url) .unwrap_or("https://chatgpt.com/backend-api/".to_string()), include_plan_tool: include_plan_tool.unwrap_or(false), - include_apply_patch_tool: include_apply_patch_tool.unwrap_or(false), + include_apply_patch_tool: include_apply_patch_tool.unwrap_or(cfg!(target_os = "windows")), tools_web_search_request, tools_web_search_external, tools_search_tool, @@ -3636,6 +3636,23 @@ context_mode = "disabled" assert_eq!(config.model_auto_compact_token_limit, Some(244_800)); Ok(()) } + + #[test] + fn include_apply_patch_tool_defaults_on_windows_only() -> anyhow::Result<()> { + let code_home = TempDir::new()?; + let cfg = toml::from_str::("model = \"gpt-5.4\"")?; + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides { + cwd: Some(code_home.path().to_path_buf()), + ..Default::default() + }, + code_home.path().to_path_buf(), + )?; + + assert_eq!(config.include_apply_patch_tool, cfg!(target_os = "windows")); + Ok(()) + } } #[cfg(test)] diff --git a/code-rs/core/src/openai_tools.rs b/code-rs/core/src/openai_tools.rs index dd2f0a7d512..602509780cd 100644 --- a/code-rs/core/src/openai_tools.rs +++ b/code-rs/core/src/openai_tools.rs @@ -189,17 +189,13 @@ impl ToolsConfig { // Our fork does not yet enable the experimental streamable shell tool // in the tool selection phase. Default to the existing behaviors. let use_streamable_shell_tool = false; - let mut shell_type = if use_streamable_shell_tool { - ConfigShellToolType::StreamableShell - } else if model_family.uses_local_shell_tool { - ConfigShellToolType::LocalShell - } else if model_family.uses_shell_command_tool { - ConfigShellToolType::ShellCommand { - sandbox_policy: sandbox_policy.clone(), - } - } else { - ConfigShellToolType::DefaultShell - }; + let mut shell_type = select_shell_type_for_platform( + model_family, + &sandbox_policy, + use_streamable_shell_tool, + include_apply_patch_tool, + cfg!(target_os = "windows"), + ); if matches!(approval_policy, AskForApproval::OnRequest) && !use_streamable_shell_tool && !matches!(shell_type, ConfigShellToolType::ShellCommand { .. }) @@ -209,23 +205,11 @@ impl ToolsConfig { } } - let apply_patch_tool_type = if include_apply_patch_tool { - // On Windows, grammar-based apply_patch invocations rely on heredocs - // the shell cannot parse. Force the JSON/function variant instead. - #[cfg(target_os = "windows")] - { - model_family - .apply_patch_tool_type - .clone() - .map(|_| ApplyPatchToolType::Function) - } - #[cfg(not(target_os = "windows"))] - { - model_family.apply_patch_tool_type.clone() - } - } else { - None - }; + let apply_patch_tool_type = apply_patch_tool_type_for_platform( + model_family, + include_apply_patch_tool, + cfg!(target_os = "windows"), + ); Self { shell_type, @@ -258,6 +242,57 @@ impl ToolsConfig { } } +fn select_shell_type_for_platform( + model_family: &ModelFamily, + sandbox_policy: &SandboxPolicy, + use_streamable_shell_tool: bool, + include_apply_patch_tool: bool, + is_windows: bool, +) -> ConfigShellToolType { + if use_streamable_shell_tool { + return ConfigShellToolType::StreamableShell; + } + + if model_family.uses_local_shell_tool { + return ConfigShellToolType::LocalShell; + } + + // Keep Windows on the argv-style shell path while apply_patch is enabled. + // That keeps the dedicated JSON tool as the preferred edit mechanism until + // shell_command/apply_patch parity is covered by fork tests. + let should_use_shell_command = model_family.uses_shell_command_tool + && !(is_windows && include_apply_patch_tool); + + if should_use_shell_command { + ConfigShellToolType::ShellCommand { + sandbox_policy: sandbox_policy.clone(), + } + } else { + ConfigShellToolType::DefaultShell + } +} + +fn apply_patch_tool_type_for_platform( + model_family: &ModelFamily, + include_apply_patch_tool: bool, + is_windows: bool, +) -> Option { + if !include_apply_patch_tool { + return None; + } + + if is_windows { + // Grammar-based apply_patch invocations rely on heredocs the native + // Windows shells cannot parse. Force the JSON/function variant. + model_family + .apply_patch_tool_type + .clone() + .map(|_| ApplyPatchToolType::Function) + } else { + model_family.apply_patch_tool_type.clone() + } +} + pub(crate) fn create_additional_permissions_schema() -> JsonSchema { JsonSchema::Object { properties: BTreeMap::from([ @@ -1814,6 +1849,54 @@ mod tests { ); } + #[test] + fn windows_apply_patch_prefers_default_shell_for_shell_command_models() { + let model_family = + find_family_for_model("gpt-5.4").expect("gpt-5.4 should be a valid model family"); + + let shell_type = select_shell_type_for_platform( + &model_family, + &SandboxPolicy::ReadOnly, + false, + true, + true, + ); + + assert!(matches!(shell_type, ConfigShellToolType::DefaultShell)); + } + + #[test] + fn windows_without_apply_patch_keeps_shell_command_models_unchanged() { + let model_family = + find_family_for_model("gpt-5.4").expect("gpt-5.4 should be a valid model family"); + + let shell_type = select_shell_type_for_platform( + &model_family, + &SandboxPolicy::ReadOnly, + false, + false, + true, + ); + + assert!(matches!( + shell_type, + ConfigShellToolType::ShellCommand { + sandbox_policy: SandboxPolicy::ReadOnly, + } + )); + } + + #[test] + fn windows_apply_patch_uses_function_tool() { + let model_family = + find_family_for_model("gpt-5.4").expect("gpt-5.4 should be a valid model family"); + + assert_eq!( + apply_patch_tool_type_for_platform(&model_family, true, true), + Some(ApplyPatchToolType::Function) + ); + } + #[test] fn test_get_openai_tools_mcp_tools() { let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); From a2d436e8ffd8adcae978337994597bc13097e96b Mon Sep 17 00:00:00 2001 From: Reese Date: Thu, 11 Jun 2026 00:26:37 +0000 Subject: [PATCH 2/2] fix(core): dispatch function apply_patch calls --- code-rs/core/src/codex/streaming.rs | 67 +++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/code-rs/core/src/codex/streaming.rs b/code-rs/core/src/codex/streaming.rs index 4202fa0fea6..d9da4930bac 100644 --- a/code-rs/core/src/codex/streaming.rs +++ b/code-rs/core/src/codex/streaming.rs @@ -4964,6 +4964,25 @@ async fn handle_function_call( ) .await } + "apply_patch" => { + let params = match parse_apply_patch_arguments(arguments, sess, &call_id) { + Ok(params) => params, + Err(output) => { + return *output; + } + }; + handle_container_exec_with_params( + params, + sess, + turn_diff_tracker, + sub_id, + call_id, + seq_hint, + output_index, + attempt_req, + ) + .await + } "update_plan" => handle_update_plan(sess, &ctx, arguments).await, "request_user_input" => handle_request_user_input(sess, &ctx, arguments).await, // agent tool @@ -5022,6 +5041,45 @@ async fn handle_function_call( } } +#[derive(serde::Deserialize)] +struct ApplyPatchToolCallParams { + input: String, +} + +fn parse_apply_patch_input(arguments: &str) -> Result { + serde_json::from_str::(arguments).map(|params| params.input) +} + +fn parse_apply_patch_arguments( + arguments: String, + sess: &Session, + call_id: &str, +) -> Result> { + match parse_apply_patch_input(&arguments) { + Ok(input) => Ok(ExecParams { + command: vec!["apply_patch".to_string(), input], + shell_script: None, + cwd: sess.get_cwd().to_path_buf(), + timeout_ms: None, + env: HashMap::new(), + with_escalated_permissions: None, + justification: None, + }), + Err(err) => { + let output = ResponseInputItem::FunctionCallOutput { + call_id: call_id.to_string(), + output: FunctionCallOutputPayload { + body: code_protocol::models::FunctionCallOutputBody::Text(format!( + "failed to parse function arguments: {err}" + )), + success: None, + }, + }; + Err(Box::new(output)) + } + } +} + async fn handle_request_user_input( sess: &Session, ctx: &ToolCallCtx, @@ -14283,6 +14341,7 @@ mod tests { image_generation_artifact_path, is_context_overflow_stream_error, is_usage_limit_stream_error, + parse_apply_patch_input, save_image_generation_result, save_image_generation_sidecar, ImageGenerationTurnMetadata, @@ -14437,6 +14496,14 @@ mod tests { assert!(!text.contains("base64")); } + #[test] + fn apply_patch_function_arguments_parse_input() { + let patch = "*** Begin Patch\n*** Add File: hello.txt\n+hello\n*** End Patch\n"; + let arguments = serde_json::json!({ "input": patch }).to_string(); + + assert_eq!(parse_apply_patch_input(&arguments).expect("valid args"), patch); + } + #[test] fn context_overflow_detection_matches_provider_errors() { assert!(is_context_overflow_stream_error(