openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add a feature flag to preserve apply_patch line endings (#37758)

Browse Source 
## What changed

- Add the `apply_patch_preserve_line_endings` feature, disabled by default, to
  preserve CRLF, CR, and mixed line endings when `apply_patch` updates files.
- Apply the feature consistently to built-in patch handling and patches invoked
  through shell, user-shell, unified-exec, and app-server command execution.
- Keep the active feature configuration authoritative over inherited, shell
  snapshot, and client-provided environment values.

## Testing

- Cover line-ending behavior with the feature enabled and disabled for custom
  tool calls, shell heredocs, command execution, and the `apply_patch` CLI.

GitOrigin-RevId: 531a7c66761959c650270559f57941929f03e6c4
This commit is contained in:
Charlie Marsh
2026-08-10 01:44:30 +00:00
committed by copyberry
parent 21aa552e87
commit c9c6c0daa9
18 changed files with 462 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/command_exec_processor.rs
+2
View File
@@ -1,4 +1,5 @@
use super::*;
use codex_core::exec_env::inject_apply_patch_env;
use codex_protocol::shell_environment::is_non_inheritable_env_var;
#[derive(Clone)]
@@ -165,6 +166,7 @@ impl CommandExecRequestProcessor {
}
}
env.retain(|name, _| !is_non_inheritable_env_var(name));
inject_apply_patch_env(&mut env, &self.config.features);
let timeout_ms = match timeout_ms {
Some(timeout_ms) => match u64::try_from(timeout_ms) {
Ok(timeout_ms) => Some(timeout_ms),
codex-rs/app-server/tests/suite/v2/command_exec.rs
+85
View File
@@ -16,6 +16,7 @@ use codex_app_server_protocol::JSONRPCMessage;
use codex_app_server_protocol::JSONRPCNotification;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SandboxPolicy;
use codex_core::exec_env::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR;
use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR;
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY;
use codex_protocol::shell_environment::OPENAI_FEDERATION_RULE_ID_ENV_VAR;
@@ -198,6 +199,90 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un
Ok(())
}
#[derive(Clone, Copy)]
enum CommandExecApplyPatchRollout {
Enabled,
Disabled,
}
#[tokio::test]
async fn command_exec_apply_patch_preserves_line_endings_despite_client_override() -> Result<()> {
assert_command_exec_apply_patch_rollout(
CommandExecApplyPatchRollout::Enabled,
"0",
b"after\r\n",
)
.await
}
#[tokio::test]
async fn command_exec_apply_patch_normalizes_line_endings_despite_stale_overrides() -> Result<()> {
assert_command_exec_apply_patch_rollout(CommandExecApplyPatchRollout::Disabled, "1", b"after\n")
.await
}
async fn assert_command_exec_apply_patch_rollout(
rollout: CommandExecApplyPatchRollout,
client_override: &str,
expected_contents: &[u8],
) -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
let feature_enabled = matches!(rollout, CommandExecApplyPatchRollout::Enabled);
insert_command_exec_config(
codex_home.path(),
&format!("[features]\napply_patch_preserve_line_endings = {feature_enabled}\n"),
)?;
let workspace = TempDir::new()?;
let file_path = workspace.path().join("crlf.txt");
std::fs::write(&file_path, b"before\r\n")?;
let mut mcp = TestAppServer::builder()
.with_codex_home(codex_home.path())
.without_auto_env()
.with_env_overrides(&[(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR, Some("1"))])
.build_initialized_with_timeout(DEFAULT_READ_TIMEOUT)
.await?;
let patch = "*** Begin Patch\n*** Update File: crlf.txt\n@@\n-before\n+after\n*** End Patch\n";
let request_id = mcp
.send_command_exec_request(CommandExecParams {
command: vec!["apply_patch".to_string(), patch.to_string()],
process_id: None,
tty: false,
stream_stdin: false,
stream_stdout_stderr: false,
output_bytes_cap: None,
disable_output_cap: false,
disable_timeout: false,
timeout_ms: None,
cwd: Some(workspace.path().to_path_buf()),
env: Some(HashMap::from([(
CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
Some(client_override.to_string()),
)])),
size: None,
sandbox_policy: Some(SandboxPolicy::DangerFullAccess),
permission_profile: None,
})
.await?;
let response: CommandExecResponse = mcp.read_response(request_id).await?;
assert_eq!(
response,
CommandExecResponse {
exit_code: 0,
stdout: "Success. Updated the following files:\nM crlf.txt\n".to_string(),
stderr: String::new(),
}
);
assert_eq!(std::fs::read(file_path)?, expected_contents);
Ok(())
}
#[tokio::test]
async fn command_exec_accepts_permission_profile() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
codex-rs/apply-patch/tests/suite/scenarios.rs
+16 -11
View File
@@ -1,5 +1,6 @@
use anyhow::Context;
use codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR;
use codex_utils_cargo_bin::repo_root;
use codex_utils_cargo_bin::find_resource;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
use std::fs;
@@ -10,17 +11,18 @@ use tempfile::tempdir;
#[test]
fn test_apply_patch_scenarios() -> anyhow::Result<()> {
let scenarios_dir = repo_root()?
.join("codex-rs")
.join("apply-patch")
.join("tests")
.join("fixtures")
.join("scenarios");
for scenario in fs::read_dir(scenarios_dir)? {
let scenarios_marker = find_resource!("tests/fixtures/scenarios/.gitattributes")?;
let scenarios_dir = scenarios_marker
.parent()
.context("scenario marker should have a parent directory")?;
for scenario in fs::read_dir(scenarios_dir)
.with_context(|| format!("failed to read {}", scenarios_dir.display()))?
{
let scenario = scenario?;
let path = scenario.path();
if path.is_dir() {
run_apply_patch_scenario(&path)?;
run_apply_patch_scenario(&path)
.with_context(|| format!("failed to run scenario {}", path.display()))?;
}
}
Ok(())
@@ -38,7 +40,9 @@ fn run_apply_patch_scenario(dir: &Path) -> anyhow::Result<()> {
}
// Read the patch.txt file
let patch = fs::read_to_string(dir.join("patch.txt"))?;
let patch_path = dir.join("patch.txt");
let patch = fs::read_to_string(&patch_path)
.with_context(|| format!("failed to read {}", patch_path.display()))?;
// Run apply_patch in the temporary directory. We intentionally do not assert
// on the exit status here; the scenarios are specified purely in terms of
@@ -47,7 +51,8 @@ fn run_apply_patch_scenario(dir: &Path) -> anyhow::Result<()> {
.arg(patch)
.env(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR, "1")
.current_dir(tmp.path())
.output()?;
.output()
.with_context(|| format!("failed to run scenario {}", dir.display()))?;
// Assert that the final state matches the expected state exactly
let expected_dir = dir.join("expected");
codex-rs/apply-patch/tests/suite/tool.rs
+44 -75
View File
@@ -1,10 +1,5 @@
use assert_cmd::Command;
use codex_apply_patch::AppliedPatchDelta;
use codex_apply_patch::ApplyPatchFailure;
use codex_apply_patch::ApplyPatchFileUpdateMode;
use codex_apply_patch::apply_patch_with_mode;
use codex_exec_server::LOCAL_FS;
use codex_utils_path_uri::PathUri;
use codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR;
use pretty_assertions::assert_eq;
use std::fs;
use std::path::Path;
@@ -13,29 +8,12 @@ use tempfile::tempdir;
fn run_apply_patch_in_dir(dir: &Path, patch: &str) -> anyhow::Result<assert_cmd::assert::Assert> {
let mut cmd = Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?);
cmd.env(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR, "1");
cmd.current_dir(dir);
Ok(cmd.arg(patch).assert())
}
async fn run_apply_patch_preserving_line_endings(
cwd: &PathUri,
patch: &str,
stdout: &mut Vec<u8>,
stderr: &mut Vec<u8>,
) -> Result<AppliedPatchDelta, ApplyPatchFailure> {
apply_patch_with_mode(
patch,
ApplyPatchFileUpdateMode::PreserveLineEndings,
cwd,
stdout,
stderr,
LOCAL_FS.as_ref(),
/*sandbox*/ None,
)
.await
}
async fn assert_apply_patch_updates_file(
fn assert_apply_patch_updates_file(
file_name: &str,
original: &[u8],
patch: &str,
@@ -45,22 +23,19 @@ async fn assert_apply_patch_updates_file(
let target_path = tmp.path().join(file_name);
fs::write(&target_path, original)?;
let mut stdout = Vec::new();
let mut stderr = Vec::new();
let cwd = PathUri::from_host_native_path(tmp.path())?;
run_apply_patch_preserving_line_endings(&cwd, patch, &mut stdout, &mut stderr).await?;
run_apply_patch_in_dir(tmp.path(), patch)?
.success()
.stdout(format!(
"Success. Updated the following files:\nM {file_name}\n"
));
assert_eq!(
String::from_utf8(stdout)?,
format!("Success. Updated the following files:\nM {file_name}\n")
);
assert_eq!(stderr, Vec::<u8>::new());
assert_eq!(fs::read(target_path)?, expected);
Ok(())
}
fn apply_patch_command(dir: &Path) -> anyhow::Result<Command> {
let mut cmd = Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?);
cmd.env(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR, "1");
cmd.current_dir(dir);
Ok(cmd)
}
@@ -112,30 +87,21 @@ fn test_apply_patch_cli_applies_multiple_chunks() -> anyhow::Result<()> {
Ok(())
}
#[tokio::test]
async fn test_apply_patch_rejects_overlapping_end_of_file_chunks() -> anyhow::Result<()> {
#[test]
fn test_apply_patch_cli_rejects_overlapping_end_of_file_chunks() -> anyhow::Result<()> {
let tmp = tempdir()?;
let target_path = tmp.path().join("overlapping.txt");
let expected_target_path = resolved_under(tmp.path(), "overlapping.txt")?;
fs::write(&target_path, "one\n")?;
let patch = "*** Begin Patch\n*** Update File: overlapping.txt\n@@\n-one\n+first\n@@\n-one\n+second\n*** End of File\n*** End Patch";
let mut stdout = Vec::new();
let mut stderr = Vec::new();
let cwd = PathUri::from_host_native_path(tmp.path())?;
let error = run_apply_patch_preserving_line_endings(&cwd, patch, &mut stdout, &mut stderr)
.await
.expect_err("overlapping chunks should fail");
assert!(error.delta().is_empty());
assert_eq!(stdout, Vec::<u8>::new());
assert_eq!(
String::from_utf8(stderr)?,
format!(
run_apply_patch_in_dir(tmp.path(), patch)?
.failure()
.stderr(format!(
"Failed to find expected lines in {}:\none\n",
target_path.display()
)
);
expected_target_path.display()
));
assert_eq!(fs::read_to_string(target_path)?, "one\n");
Ok(())
@@ -149,7 +115,11 @@ fn test_apply_patch_cli_allows_overlapping_eof_chunks_in_legacy_mode() -> anyhow
let patch = "*** Begin Patch\n*** Update File: overlapping.txt\n@@\n-one\n+first\n@@\n-one\n+second\n*** End of File\n*** End Patch";
run_apply_patch_in_dir(tmp.path(), patch)?
Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?)
.env_remove(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR)
.arg(patch)
.current_dir(tmp.path())
.assert()
.success()
.stdout("Success. Updated the following files:\nM overlapping.txt\n");
@@ -157,8 +127,8 @@ fn test_apply_patch_cli_allows_overlapping_eof_chunks_in_legacy_mode() -> anyhow
Ok(())
}
#[tokio::test]
async fn test_apply_patch_preserves_crlf_from_target_file() -> anyhow::Result<()> {
#[test]
fn test_apply_patch_cli_preserves_crlf_from_target_file() -> anyhow::Result<()> {
let patch = "*** Begin Patch\n*** Update File: crlf.txt\n@@\n-one\n+uno\n@@\n two\n+\n+between\n three\n*** End Patch";
assert_apply_patch_updates_file(
@@ -167,11 +137,10 @@ async fn test_apply_patch_preserves_crlf_from_target_file() -> anyhow::Result<()
patch,
b"uno\r\ntwo\r\n\r\nbetween\r\nthree\r\n",
)
.await
}
#[tokio::test]
async fn test_apply_patch_appends_after_trailing_blank_crlf_line() -> anyhow::Result<()> {
#[test]
fn test_apply_patch_cli_appends_after_trailing_blank_crlf_line() -> anyhow::Result<()> {
let patch = "*** Begin Patch\n*** Update File: trailing_blank.txt\n@@\n+new\n*** End Patch";
assert_apply_patch_updates_file(
@@ -180,24 +149,28 @@ async fn test_apply_patch_appends_after_trailing_blank_crlf_line() -> anyhow::Re
patch,
b"a\r\n\r\nnew\r\n",
)
.await
}
#[test]
fn test_apply_patch_cli_uses_legacy_line_handling_by_default() -> anyhow::Result<()> {
fn test_apply_patch_cli_uses_legacy_line_handling_without_rollout_env() -> anyhow::Result<()> {
let tmp = tempdir()?;
let target_path = tmp.path().join("crlf.txt");
fs::write(&target_path, b"one\r\n")?;
let patch = "*** Begin Patch\n*** Update File: crlf.txt\n@@\n-one\n+uno\n*** End Patch";
run_apply_patch_in_dir(tmp.path(), patch)?.success();
Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?)
.env_remove(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR)
.arg(patch)
.current_dir(tmp.path())
.assert()
.success();
assert_eq!(fs::read(target_path)?, b"uno\n");
Ok(())
}
#[tokio::test]
async fn test_apply_patch_preserves_cr_from_target_file() -> anyhow::Result<()> {
#[test]
fn test_apply_patch_cli_preserves_cr_from_target_file() -> anyhow::Result<()> {
let patch = "*** Begin Patch\n*** Update File: cr.txt\n@@\n-one\n+uno\n@@\n two\n+\n+between\n three\n*** End Patch";
assert_apply_patch_updates_file(
@@ -206,28 +179,26 @@ async fn test_apply_patch_preserves_cr_from_target_file() -> anyhow::Result<()>
patch,
b"uno\rtwo\r\rbetween\rthree\r",
)
.await
}
#[tokio::test]
async fn test_apply_patch_preserves_change_order_with_repeated_lines() -> anyhow::Result<()> {
#[test]
fn test_apply_patch_cli_preserves_change_order_with_repeated_lines() -> anyhow::Result<()> {
let patch =
"*** Begin Patch\n*** Update File: repeated.txt\n@@\n-a\n-b\n+b\n+b\n+a\n*** End Patch";
assert_apply_patch_updates_file("repeated.txt", b"a\nb\n", patch, b"b\nb\na\n").await
assert_apply_patch_updates_file("repeated.txt", b"a\nb\n", patch, b"b\nb\na\n")
}
#[tokio::test]
async fn test_apply_patch_preserves_repeated_context_line_ending() -> anyhow::Result<()> {
#[test]
fn test_apply_patch_cli_preserves_repeated_context_line_ending() -> anyhow::Result<()> {
let patch =
"*** Begin Patch\n*** Update File: repeated_context.txt\n@@\n-same\n same\n*** End Patch";
assert_apply_patch_updates_file("repeated_context.txt", b"same\r\nsame\n", patch, b"same\n")
.await
}
#[tokio::test]
async fn test_apply_patch_preserves_untouched_mixed_line_endings() -> anyhow::Result<()> {
#[test]
fn test_apply_patch_cli_preserves_untouched_mixed_line_endings() -> anyhow::Result<()> {
let patch = "*** Begin Patch\n*** Update File: mixed.txt\n@@\n one\n two\n-three\n+THREE\n four\n*** End Patch";
assert_apply_patch_updates_file(
@@ -236,11 +207,10 @@ async fn test_apply_patch_preserves_untouched_mixed_line_endings() -> anyhow::Re
patch,
b"one\r\ntwo\rTHREE\r\nfour\r\n",
)
.await
}
#[tokio::test]
async fn test_apply_patch_uses_crlf_for_new_trailing_newline() -> anyhow::Result<()> {
#[test]
fn test_apply_patch_cli_uses_crlf_for_new_trailing_newline() -> anyhow::Result<()> {
let patch =
"*** Begin Patch\n*** Update File: no_trailing_newline.txt\n@@\n-one\n+ONE\n*** End Patch";
@@ -250,7 +220,6 @@ async fn test_apply_patch_uses_crlf_for_new_trailing_newline() -> anyhow::Result
patch,
b"ONE\r\ntwo\r\n",
)
.await
}
#[test]
codex-rs/core/config.schema.json
+6
View File
@@ -434,6 +434,9 @@
"apply_patch_freeform": {
"type": "boolean"
},
"apply_patch_preserve_line_endings": {
"type": "boolean"
},
"apply_patch_streaming_events": {
"type": "boolean"
},
@@ -5102,6 +5105,9 @@
"apply_patch_freeform": {
"type": "boolean"
},
"apply_patch_preserve_line_endings": {
"type": "boolean"
},
"apply_patch_streaming_events": {
"type": "boolean"
},
codex-rs/core/src/exec_env.rs
+19
View File
@@ -1,3 +1,6 @@
pub use codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR;
use codex_features::Feature;
use codex_features::Features;
use codex_protocol::ThreadId;
#[cfg(test)]
use codex_protocol::config_types::EnvironmentVariablePattern;
@@ -51,6 +54,22 @@ pub(crate) fn inject_permission_profile_env(
}
}
/// Carries the configured apply-patch line-ending rollout state into child
/// processes.
///
/// Apply this after inherited or client-provided environment overrides so the
/// active feature configuration remains authoritative. The in-process
/// apply-patch path reads the feature directly.
pub fn inject_apply_patch_env(env: &mut HashMap<String, String>, features: &Features) {
env.retain(|key, _| !key.eq_ignore_ascii_case(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR));
if features.enabled(Feature::ApplyPatchPreserveLineEndings) {
env.insert(
CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
"1".to_string(),
);
}
}
#[cfg(all(test, target_os = "windows"))]
fn create_env_from_vars<I>(
vars: I,
codex-rs/core/src/exec_env_tests.rs
+22
View File
@@ -41,6 +41,28 @@ fn inject_permission_profile_env_removes_stale_value_without_active_profile() {
assert_eq!(env.get(CODEX_PERMISSION_PROFILE_ENV_VAR), None);
}
#[test]
fn inject_apply_patch_env_follows_preserve_line_endings_feature() {
let mut env = HashMap::from([(
CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_ascii_lowercase(),
"stale".to_string(),
)]);
let mut features = Features::with_defaults();
inject_apply_patch_env(&mut env, &features);
assert_eq!(env, HashMap::new());
features.enable(Feature::ApplyPatchPreserveLineEndings);
inject_apply_patch_env(&mut env, &features);
assert_eq!(
env,
HashMap::from([(
CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
"1".to_string(),
)])
);
}
#[cfg(target_os = "windows")]
#[test]
fn inject_permission_profile_env_replaces_differently_cased_windows_key() {
codex-rs/core/src/tasks/user_shell.rs
+2
View File
@@ -14,6 +14,7 @@ use crate::exec::ExecCapturePolicy;
use crate::exec::StdoutStream;
use crate::exec::execute_exec_request;
use crate::exec_env::create_env;
use crate::exec_env::inject_apply_patch_env;
use crate::sandboxing::ExecRequest;
use crate::session::TurnInput;
use crate::session::turn_context::TurnContext;
@@ -159,6 +160,7 @@ pub(crate) async fn execute_user_shell_command(
&turn_context.config.permissions.shell_environment_policy,
Some(session.thread_id),
);
inject_apply_patch_env(&mut exec_env_map, &turn_context.config.features);
if exec_env_map.contains_key(PROXY_ACTIVE_ENV_KEY) {
strip_managed_proxy_env(&mut exec_env_map);
}
codex-rs/core/src/tools/handlers/apply_patch.rs
+24 -3
View File
@@ -37,6 +37,7 @@ use crate::tools::runtimes::apply_patch::ApplyPatchRuntime;
use crate::tools::sandboxing::ToolCtx;
use codex_apply_patch::ApplyPatchAction;
use codex_apply_patch::ApplyPatchFileChange;
use codex_apply_patch::ApplyPatchFileUpdateMode;
use codex_apply_patch::Hunk;
use codex_apply_patch::StreamingPatchParser;
use codex_exec_server::ExecutorFileSystem;
@@ -55,6 +56,19 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
const APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL: Duration = Duration::from_millis(500);
fn apply_patch_file_update_mode(turn: &TurnContext) -> ApplyPatchFileUpdateMode {
if turn
.config
.features
.enabled(Feature::ApplyPatchPreserveLineEndings)
{
ApplyPatchFileUpdateMode::PreserveLineEndings
} else {
ApplyPatchFileUpdateMode::NormalizeToLf
}
}
/// Handles freeform `apply_patch` requests and routes verified patches to the
/// selected environment filesystem.
#[derive(Default)]
@@ -387,9 +401,10 @@ impl ApplyPatchHandler {
let fs = turn_environment.environment.get_filesystem();
let sandbox = turn
.file_system_sandbox_context(/*additional_permissions*/ None, turn_environment);
match codex_apply_patch::verify_apply_patch_args(
match codex_apply_patch::verify_apply_patch_args_with_mode(
args,
turn_environment.cwd(),
apply_patch_file_update_mode(&turn),
fs.as_ref(),
Some(&sandbox),
)
@@ -496,8 +511,14 @@ pub(crate) async fn intercept_apply_patch(
let turn = &step_context.turn;
let sandbox =
turn.file_system_sandbox_context(/*additional_permissions*/ None, &turn_environment);
match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs, Some(&sandbox))
.await
match codex_apply_patch::maybe_parse_apply_patch_verified_with_mode(
command,
cwd,
apply_patch_file_update_mode(turn),
fs,
Some(&sandbox),
)
.await
{
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
let tool_ctx = ToolCtx {
codex-rs/core/src/tools/handlers/apply_patch_tests.rs
+18
View File
@@ -44,6 +44,24 @@ async fn invocation_for_payload(payload: ToolPayload) -> ToolInvocation {
}
}
#[tokio::test]
async fn file_update_mode_follows_preserve_line_endings_feature() {
let (_, mut turn) = make_session_and_context().await;
assert_eq!(
apply_patch_file_update_mode(&turn),
codex_apply_patch::ApplyPatchFileUpdateMode::NormalizeToLf
);
Arc::make_mut(&mut turn.config)
.features
.enable(codex_features::Feature::ApplyPatchPreserveLineEndings)
.expect("feature should be enabled");
assert_eq!(
apply_patch_file_update_mode(&turn),
codex_apply_patch::ApplyPatchFileUpdateMode::PreserveLineEndings
);
}
#[tokio::test]
async fn pre_tool_use_payload_uses_freeform_patch_input() {
let patch = sample_patch();
codex-rs/core/src/tools/handlers/shell/shell_command.rs
+2
View File
@@ -6,6 +6,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use crate::exec::ExecCapturePolicy;
use crate::exec::ExecParams;
use crate::exec_env::create_env;
use crate::exec_env::inject_apply_patch_env;
use crate::exec_env::inject_permission_profile_env;
use crate::function_tool::FunctionCallError;
use crate::maybe_emit_implicit_skill_invocation;
@@ -105,6 +106,7 @@ impl ShellCommandHandler {
&turn_context.config.permissions.shell_environment_policy,
Some(session.thread_id),
);
inject_apply_patch_env(&mut env, &turn_context.config.features);
let active_permission_profile = turn_environment.active_permission_profile();
inject_permission_profile_env(&mut env, active_permission_profile.as_ref());
let sandbox_permissions = resolve_sandbox_permissions(
codex-rs/core/src/tools/runtimes/apply_patch.rs
+2 -1
View File
@@ -164,8 +164,9 @@ impl ToolRuntime<ApplyPatchRequest, ApplyPatchRuntimeOutput> for ApplyPatchRunti
let sandbox = Self::file_system_sandbox_context_for_attempt(req, attempt);
let mut stdout = Vec::new();
let mut stderr = Vec::new();
let result = codex_apply_patch::apply_patch(
let result = codex_apply_patch::apply_patch_with_mode(
&req.action.patch,
req.action.update_file_mode(),
&req.action.cwd,
&mut stdout,
&mut stderr,
codex-rs/core/src/tools/runtimes/mod.rs
+14 -4
View File
@@ -10,6 +10,7 @@ use crate::sandboxing::SandboxPermissions;
use crate::shell::Shell;
use crate::shell::ShellType;
use crate::tools::sandboxing::ToolError;
use codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR;
#[cfg(unix)]
use codex_install_context::InstallContext;
#[cfg(target_os = "macos")]
@@ -267,14 +268,23 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
.map(|arg| format!(" '{}'", shell_single_quote(arg)))
.collect::<String>();
let mut override_env = explicit_env_overrides.clone();
for key in [CODEX_THREAD_ID_ENV_VAR, CODEX_PERMISSION_PROFILE_ENV_VAR] {
for key in [
CODEX_THREAD_ID_ENV_VAR,
CODEX_PERMISSION_PROFILE_ENV_VAR,
CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR,
] {
if let Some(value) = env.get(key) {
override_env.insert(key.to_string(), value.clone());
}
}
// Do not let a snapshot resurrect a stale profile when no named profile is active.
let (override_captures, override_exports) =
build_override_exports(&override_env, &[CODEX_PERMISSION_PROFILE_ENV_VAR]);
// Do not let a snapshot resurrect stale runtime state when it is inactive.
let (override_captures, override_exports) = build_override_exports(
&override_env,
&[
CODEX_PERMISSION_PROFILE_ENV_VAR,
CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR,
],
);
let (proxy_captures, proxy_exports) = build_proxy_env_exports();
let runtime_path_prepend_exports =
runtime_path_prepends.shell_exports_after_snapshot(explicit_env_overrides);
codex-rs/core/src/tools/runtimes/mod_tests.rs
+55
View File
@@ -607,6 +607,61 @@ fn maybe_wrap_shell_lc_with_snapshot_unsets_absent_permission_profile() {
assert_eq!(output.stdout, b"");
}
#[test]
fn maybe_wrap_shell_lc_with_snapshot_restores_apply_patch_rollout_state() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
std::fs::write(
&snapshot_path,
"# Snapshot file\nexport CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS='stale'\n",
)
.expect("write snapshot");
let (session_shell, shell_snapshot) =
shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs());
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"printenv CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS".to_string(),
];
let env = HashMap::from([(
CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
"1".to_string(),
)]);
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
Some(&shell_snapshot),
&HashMap::new(),
&env,
&RuntimePathPrepends::default(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR, "1")
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(String::from_utf8_lossy(&output.stdout), "1\n");
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
Some(&shell_snapshot),
&HashMap::new(),
&HashMap::new(),
&RuntimePathPrepends::default(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env_remove(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR)
.output()
.expect("run rewritten command");
assert_eq!(output.status.code(), Some(1));
assert_eq!(output.stdout, b"");
}
#[test]
fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() {
let dir = tempdir().expect("create temp dir");
codex-rs/core/src/unified_exec/process_manager.rs
+17 -4
View File
@@ -16,6 +16,7 @@ use crate::codex_thread::BackgroundTerminalInfo;
use crate::exec_env::CODEX_PERMISSION_PROFILE_ENV_VAR;
use crate::exec_env::CODEX_THREAD_ID_ENV_VAR;
use crate::exec_env::create_env;
use crate::exec_env::inject_apply_patch_env;
use crate::exec_env::inject_permission_profile_env;
use crate::exec_policy::ExecApprovalRequest;
use crate::sandboxing::ExecOptions;
@@ -122,10 +123,18 @@ fn exec_env_policy_from_shell_policy(
.iter()
.map(std::string::ToString::to_string)
.collect::<Vec<_>>();
exclude.push(CODEX_PERMISSION_PROFILE_ENV_VAR.to_string());
exclude.extend([
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
]);
let mut r#set = policy.r#set.clone();
r#set.retain(|key, _| {
!key.eq_ignore_ascii_case(CODEX_PERMISSION_PROFILE_ENV_VAR)
![
CODEX_PERMISSION_PROFILE_ENV_VAR,
codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR,
]
.iter()
.any(|runtime_key| key.eq_ignore_ascii_case(runtime_key))
&& !is_non_inheritable_env_var(key)
});
codex_exec_server::ExecEnvPolicy {
@@ -149,8 +158,11 @@ fn env_overlay_for_exec_server(
.iter()
.filter(|(key, value)| {
!is_non_inheritable_env_var(key)
&& (key.as_str() == CODEX_PERMISSION_PROFILE_ENV_VAR
|| local_policy_env.get(*key) != Some(*value))
&& (matches!(
key.as_str(),
CODEX_PERMISSION_PROFILE_ENV_VAR
| codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR
) || local_policy_env.get(*key) != Some(*value))
})
.map(|(key, value)| (key.clone(), value.clone()))
.collect()
@@ -1148,6 +1160,7 @@ impl UnifiedExecProcessManager {
CODEX_THREAD_ID_ENV_VAR.to_string(),
context.session.thread_id.to_string(),
);
inject_apply_patch_env(&mut env, &turn.config.features);
let active_permission_profile = request.turn_environment.active_permission_profile();
inject_permission_profile_env(&mut env, active_permission_profile.as_ref());
let env = apply_unified_exec_env(env);
codex-rs/core/src/unified_exec/process_manager_tests.rs
+20 -1
View File
@@ -47,6 +47,10 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() {
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
"current-profile".to_string(),
),
(
codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
"1".to_string(),
),
]);
let request_env = HashMap::from([
("HOME".to_string(), "/client-home".to_string()),
@@ -58,6 +62,10 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() {
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
"current-profile".to_string(),
),
(
codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
"1".to_string(),
),
(
"CODEX_SANDBOX_NETWORK_DISABLED".to_string(),
"1".to_string(),
@@ -73,6 +81,10 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() {
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
"current-profile".to_string(),
),
(
codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
"1".to_string(),
),
(
"CODEX_SANDBOX_NETWORK_DISABLED".to_string(),
"1".to_string()
@@ -93,6 +105,10 @@ fn exec_env_policy_excludes_non_inheritable_and_runtime_variables() {
"openai_identity_token_file".to_string(),
"/run/identity-token".to_string(),
),
(
"codex_apply_patch_preserve_line_endings".to_string(),
"1".to_string(),
),
("KEEP".to_string(), "value".to_string()),
]),
..Default::default()
@@ -103,7 +119,10 @@ fn exec_env_policy_excludes_non_inheritable_and_runtime_variables() {
codex_exec_server::ExecEnvPolicy {
inherit: policy.inherit,
ignore_default_excludes: policy.ignore_default_excludes,
exclude: vec![CODEX_PERMISSION_PROFILE_ENV_VAR.to_string()],
exclude: vec![
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
],
r#set: HashMap::from([("KEEP".to_string(), "value".to_string())]),
include_only: Vec::new(),
}
codex-rs/core/tests/suite/apply_patch_cli.rs
+106
View File
@@ -257,6 +257,112 @@ fn apply_patch_responses(
]
}
async fn assert_apply_patch_crlf_update(
configure: impl FnOnce(TestCodexBuilder) -> TestCodexBuilder,
model_output: CrLfApplyPatchModelOutput,
expected: &str,
) -> Result<()> {
skip_if_no_network!(Ok(()));
let harness = apply_patch_harness_with(configure).await?;
let call_id = "apply-patch-crlf-rollout";
let file_name = "crlf.txt";
harness.write_file(file_name, "before\r\n").await?;
let patch = format!(
"*** Begin Patch\n*** Update File: {file_name}\n@@\n-before\n+after\n*** End Patch\n"
);
match model_output {
CrLfApplyPatchModelOutput::CustomTool => {
mount_apply_patch(&harness, call_id, &patch, "apply_patch done").await;
}
CrLfApplyPatchModelOutput::ShellCommandViaHeredoc => {
mount_apply_patch_model_output(
&harness,
call_id,
&patch,
"apply_patch done",
ApplyPatchModelOutput::ShellCommandViaHeredoc,
)
.await;
}
}
harness
.test()
.submit_turn_with_permission_profile(
"update the CRLF file with apply_patch",
PermissionProfile::Disabled,
)
.await?;
assert_eq!(harness.read_file_text(file_name).await?, expected);
Ok(())
}
#[derive(Clone, Copy)]
enum CrLfApplyPatchModelOutput {
CustomTool,
ShellCommandViaHeredoc,
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_normalizes_crlf_without_preserve_line_endings_feature() -> Result<()> {
assert_apply_patch_crlf_update(
|builder| builder,
CrLfApplyPatchModelOutput::CustomTool,
"after\n",
)
.await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_preserves_crlf_with_preserve_line_endings_feature() -> Result<()> {
assert_apply_patch_crlf_update(
|builder| {
builder.with_config(|config| {
config
.features
.enable(Feature::ApplyPatchPreserveLineEndings)
.expect("feature should be enabled");
})
},
CrLfApplyPatchModelOutput::CustomTool,
"after\r\n",
)
.await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_shell_heredoc_normalizes_crlf_without_preserve_line_endings_feature()
-> Result<()> {
skip_if_wine_exec!(Ok(()), "uses a POSIX shell heredoc");
assert_apply_patch_crlf_update(
|builder| builder,
CrLfApplyPatchModelOutput::ShellCommandViaHeredoc,
"after\n",
)
.await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_shell_heredoc_preserves_crlf_with_preserve_line_endings_feature() -> Result<()>
{
skip_if_wine_exec!(Ok(()), "uses a POSIX shell heredoc");
assert_apply_patch_crlf_update(
|builder| {
builder.with_config(|config| {
config
.features
.enable(Feature::ApplyPatchPreserveLineEndings)
.expect("feature should be enabled");
})
},
CrLfApplyPatchModelOutput::ShellCommandViaHeredoc,
"after\r\n",
)
.await
}
#[cfg(target_os = "linux")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() -> Result<()> {
codex-rs/features/src/lib.rs
+8
View File
@@ -123,6 +123,8 @@ pub enum Feature {
TerminalVisualizationInstructions,
/// Stream structured progress while apply_patch input is being generated.
ApplyPatchStreamingEvents,
/// Preserve existing line endings when apply_patch updates files.
ApplyPatchPreserveLineEndings,
/// Allow exec tools to request additional permissions while staying sandboxed.
ExecPermissionApprovals,
/// Expose the built-in request_permissions tool.
@@ -1045,6 +1047,12 @@ pub const FEATURES: &[FeatureSpec] = &[
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::ApplyPatchPreserveLineEndings,
key: "apply_patch_preserve_line_endings",
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::ExecPermissionApprovals,
key: "exec_permission_approvals",