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

Make apply_patch preserve line endings unconditionally (#51203)

Browse Source 
## Why

Updating a CRLF file with `apply_patch` previously normalized its line endings to LF unless preservation was explicitly enabled. Preserve the file's existing line endings without requiring an opt-in.

## What changed

- Use line-ending-preserving updates for both tool calls and standalone `apply_patch` invocations, removing the legacy normalization mode.
- Mark `apply_patch_preserve_line_endings` as removed.
- Always set `CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS=1` in child environments for compatibility with older patch executables, overriding stale values.

## Testing

Update CLI, tool-call, shell-heredoc, and app-server tests to check CRLF preservation by default. Keep coverage for overriding stale compatibility environment values.

GitOrigin-RevId: 96d0b0e5350faa3fc7a45d63627a132657b44523
This commit is contained in:
Charlie Marsh
2026-10-05 23:52:06 +00:00
committed by copyberry
parent 93f8e79fd2
commit 685270a56a
21 changed files with 80 additions and 513 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/command_exec_processor.rs
+1 -1
View File
@@ -168,7 +168,7 @@ impl CommandExecRequestProcessor {
}
}
env.retain(|name, _| !is_non_inheritable_env_var(name));
inject_apply_patch_env(&mut env, &self.config.features);
inject_apply_patch_env(&mut env);
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
+3 -34
View File
@@ -199,43 +199,12 @@ 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<()> {
async fn command_exec_apply_patch_preserves_line_endings_by_default() -> 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")?;
@@ -262,7 +231,7 @@ async fn assert_command_exec_apply_patch_rollout(
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()),
Some("0".to_string()),
)])),
size: None,
sandbox_policy: Some(SandboxPolicy::DangerFullAccess),
@@ -279,7 +248,7 @@ async fn assert_command_exec_apply_patch_rollout(
stderr: String::new(),
}
);
assert_eq!(std::fs::read(file_path)?, expected_contents);
assert_eq!(std::fs::read(file_path)?, b"after\r\n");
Ok(())
}
codex-rs/apply-patch/src/file_update.rs
+36 -167
View File
@@ -5,7 +5,6 @@ use codex_utils_path_uri::PathUri;
use similar::TextDiff;
use crate::ApplyPatchError;
use crate::ApplyPatchFileUpdateMode;
use crate::IoError;
use crate::UpdateFileChunk;
use crate::seek_sequence;
@@ -26,7 +25,6 @@ pub(crate) struct AppliedPatch {
pub(crate) async fn derive_new_contents_from_chunks(
path: &PathUri,
chunks: &[UpdateFileChunk],
update_file_mode: ApplyPatchFileUpdateMode,
fs: &dyn ExecutorFileSystem,
follow_symlinks: bool,
sandbox: Option<&FileSystemSandboxContext>,
@@ -45,36 +43,11 @@ pub(crate) async fn derive_new_contents_from_chunks(
})?;
let path_text = path.inferred_native_path_string();
let new_contents = match update_file_mode {
ApplyPatchFileUpdateMode::NormalizeToLf => {
let mut original_lines = original_contents
.split('\n')
.map(String::from)
.collect::<Vec<_>>();
// Drop the trailing empty element that results from the final newline so
// that line counts match the behaviour of standard `diff`.
if original_lines.last().is_some_and(String::is_empty) {
original_lines.pop();
}
let replacements =
compute_replacements(&original_lines, &path_text, chunks, update_file_mode)?;
let mut new_lines = apply_replacements(original_lines, &replacements);
if !new_lines.last().is_some_and(String::is_empty) {
new_lines.push(String::new());
}
new_lines.join("\n")
}
ApplyPatchFileUpdateMode::PreserveLineEndings => {
let mut source_file = SourceFile::parse(&original_contents);
let original_lines = source_file.line_texts();
let replacements =
compute_replacements(&original_lines, &path_text, chunks, update_file_mode)?;
source_file.apply_replacements(&replacements);
source_file.into_contents()
}
};
let mut source_file = SourceFile::parse(&original_contents);
let original_lines = source_file.line_texts();
let replacements = compute_replacements(&original_lines, &path_text, chunks)?;
source_file.apply_replacements(&replacements);
let new_contents = source_file.into_contents();
Ok(AppliedPatch {
original_contents,
new_contents,
@@ -88,7 +61,6 @@ fn compute_replacements(
original_lines: &[String],
path: &str,
chunks: &[UpdateFileChunk],
update_file_mode: ApplyPatchFileUpdateMode,
) -> std::result::Result<Vec<Replacement>, ApplyPatchError> {
let mut replacements: Vec<Replacement> = Vec::new();
let mut line_index: usize = 0;
@@ -102,7 +74,6 @@ fn compute_replacements(
std::slice::from_ref(ctx_line),
line_index,
/*eof*/ false,
update_file_mode,
) {
line_index = idx + 1;
} else {
@@ -113,20 +84,7 @@ fn compute_replacements(
}
if chunk.old_lines.is_empty() {
// Preserve the legacy split representation's handling of a final
// empty line. `SourceFile` only exposes real source lines, so its
// insertion point is always after the final line.
let insertion_idx = match update_file_mode {
ApplyPatchFileUpdateMode::NormalizeToLf => {
if original_lines.last().is_some_and(String::is_empty) {
original_lines.len() - 1
} else {
original_lines.len()
}
}
ApplyPatchFileUpdateMode::PreserveLineEndings => original_lines.len(),
};
replacements.push((insertion_idx, 0, chunk.new_lines.clone()));
replacements.push((original_lines.len(), 0, chunk.new_lines.clone()));
continue;
}
@@ -142,13 +100,8 @@ fn compute_replacements(
// touching the end‑of‑file can be located reliably.
let mut pattern: &[String] = &chunk.old_lines;
let mut found = seek_sequence::seek_sequence(
original_lines,
pattern,
line_index,
chunk.is_end_of_file,
update_file_mode,
);
let mut found =
seek_sequence::seek_sequence(original_lines, pattern, line_index, chunk.is_end_of_file);
let mut new_slice: &[String] = &chunk.new_lines;
@@ -165,45 +118,37 @@ fn compute_replacements(
pattern,
line_index,
chunk.is_end_of_file,
update_file_mode,
);
}
if let Some(start_idx) = found {
match update_file_mode {
ApplyPatchFileUpdateMode::NormalizeToLf => {
replacements.push((start_idx, pattern.len(), new_slice.to_vec()));
// Context lines occur in both sides of a patch chunk. Keep those
// original lines in place so their exact contents and terminators
// survive, especially when the file has mixed line endings.
let mut old_start = 0;
let mut new_start = 0;
for &(old_context, new_context) in &chunk.context_line_indices {
// A trailing empty context line can be removed from `pattern`
// and `new_slice` above when it represents the final newline.
if old_context >= pattern.len() || new_context >= new_slice.len() {
break;
}
ApplyPatchFileUpdateMode::PreserveLineEndings => {
// Context lines occur in both sides of a patch chunk. Keep those
// original lines in place so their exact contents and terminators
// survive, especially when the file has mixed line endings.
let mut old_start = 0;
let mut new_start = 0;
for &(old_context, new_context) in &chunk.context_line_indices {
// A trailing empty context line can be removed from `pattern`
// and `new_slice` above when it represents the final newline.
if old_context >= pattern.len() || new_context >= new_slice.len() {
break;
}
if old_start != old_context || new_start != new_context {
replacements.push((
start_idx + old_start,
old_context - old_start,
new_slice[new_start..new_context].to_vec(),
));
}
old_start = old_context + 1;
new_start = new_context + 1;
}
if old_start != pattern.len() || new_start != new_slice.len() {
replacements.push((
start_idx + old_start,
pattern.len() - old_start,
new_slice[new_start..].to_vec(),
));
}
if old_start != old_context || new_start != new_context {
replacements.push((
start_idx + old_start,
old_context - old_start,
new_slice[new_start..new_context].to_vec(),
));
}
old_start = old_context + 1;
new_start = new_context + 1;
}
if old_start != pattern.len() || new_start != new_slice.len() {
replacements.push((
start_idx + old_start,
pattern.len() - old_start,
new_slice[new_start..].to_vec(),
));
}
line_index = start_idx + pattern.len();
} else {
@@ -220,31 +165,6 @@ fn compute_replacements(
Ok(replacements)
}
/// Apply the `(start_index, old_len, new_lines)` replacements to `original_lines`,
/// returning the modified file contents as a vector of lines.
fn apply_replacements(mut lines: Vec<String>, replacements: &[Replacement]) -> Vec<String> {
// We must apply replacements in descending order so that earlier replacements
// don't shift the positions of later ones.
for (start_idx, old_len, new_segment) in replacements.iter().rev() {
let start_idx = *start_idx;
let old_len = *old_len;
// Remove old lines.
for _ in 0..old_len {
if start_idx < lines.len() {
lines.remove(start_idx);
}
}
// Insert new lines.
for (offset, new_line) in new_segment.iter().enumerate() {
lines.insert(start_idx + offset, new_line.clone());
}
}
lines
}
/// Intended result of a file update for apply_patch.
#[derive(Debug, Eq, PartialEq)]
pub struct ApplyPatchFileUpdate {
@@ -259,32 +179,7 @@ pub async fn unified_diff_from_chunks(
fs: &dyn ExecutorFileSystem,
sandbox: Option<&FileSystemSandboxContext>,
) -> std::result::Result<ApplyPatchFileUpdate, ApplyPatchError> {
unified_diff_from_chunks_with_mode(
path,
chunks,
ApplyPatchFileUpdateMode::default(),
fs,
sandbox,
)
.await
}
pub(crate) async fn unified_diff_from_chunks_with_mode(
path: &PathUri,
chunks: &[UpdateFileChunk],
update_file_mode: ApplyPatchFileUpdateMode,
fs: &dyn ExecutorFileSystem,
sandbox: Option<&FileSystemSandboxContext>,
) -> std::result::Result<ApplyPatchFileUpdate, ApplyPatchError> {
unified_diff_from_chunks_with_context_and_mode(
path,
chunks,
/*context*/ 1,
update_file_mode,
fs,
sandbox,
)
.await
unified_diff_from_chunks_with_context(path, chunks, /*context*/ 1, fs, sandbox).await
}
pub async fn unified_diff_from_chunks_with_context(
@@ -293,38 +188,12 @@ pub async fn unified_diff_from_chunks_with_context(
context: usize,
fs: &dyn ExecutorFileSystem,
sandbox: Option<&FileSystemSandboxContext>,
) -> std::result::Result<ApplyPatchFileUpdate, ApplyPatchError> {
unified_diff_from_chunks_with_context_and_mode(
path,
chunks,
context,
ApplyPatchFileUpdateMode::default(),
fs,
sandbox,
)
.await
}
async fn unified_diff_from_chunks_with_context_and_mode(
path: &PathUri,
chunks: &[UpdateFileChunk],
context: usize,
update_file_mode: ApplyPatchFileUpdateMode,
fs: &dyn ExecutorFileSystem,
sandbox: Option<&FileSystemSandboxContext>,
) -> std::result::Result<ApplyPatchFileUpdate, ApplyPatchError> {
let AppliedPatch {
original_contents,
new_contents,
} = derive_new_contents_from_chunks(
path,
chunks,
update_file_mode,
fs,
/*follow_symlinks*/ true,
sandbox,
)
.await?;
} = derive_new_contents_from_chunks(path, chunks, fs, /*follow_symlinks*/ true, sandbox)
.await?;
let text_diff = TextDiff::from_lines(&original_contents, &new_contents);
let unified_diff = text_diff.unified_diff().context_radius(context).to_string();
Ok(ApplyPatchFileUpdate {
codex-rs/apply-patch/src/invocation.rs
+4 -48
View File
@@ -13,13 +13,12 @@ use crate::ApplyPatchArgs;
use crate::ApplyPatchError;
use crate::ApplyPatchFileChange;
use crate::ApplyPatchFileUpdate;
use crate::ApplyPatchFileUpdateMode;
use crate::IoError;
use crate::MaybeApplyPatchVerified;
use crate::parser::Hunk;
use crate::parser::ParseError;
use crate::parser::parse_patch;
use crate::unified_diff_from_chunks_with_mode;
use crate::unified_diff_from_chunks;
use codex_utils_path_uri::PathConvention;
use codex_utils_path_uri::PathUri;
use std::str::Utf8Error;
@@ -144,25 +143,6 @@ pub async fn maybe_parse_apply_patch_verified(
cwd: &PathUri,
fs: &dyn ExecutorFileSystem,
sandbox: Option<&codex_exec_server::FileSystemSandboxContext>,
) -> MaybeApplyPatchVerified {
maybe_parse_apply_patch_verified_with_mode(
argv,
cwd,
ApplyPatchFileUpdateMode::default(),
fs,
sandbox,
)
.await
}
/// Parses and verifies an `apply_patch` invocation using the selected
/// file-update mode.
pub async fn maybe_parse_apply_patch_verified_with_mode(
argv: &[String],
cwd: &PathUri,
update_file_mode: ApplyPatchFileUpdateMode,
fs: &dyn ExecutorFileSystem,
sandbox: Option<&codex_exec_server::FileSystemSandboxContext>,
) -> MaybeApplyPatchVerified {
// Detect a raw patch body passed directly as the command or as the body of a shell
// script. In these cases, report an explicit error rather than applying the patch.
@@ -178,9 +158,7 @@ pub async fn maybe_parse_apply_patch_verified_with_mode(
}
match maybe_parse_apply_patch(argv, cwd) {
MaybeApplyPatch::Body(args) => {
verify_apply_patch_args_with_mode(args, cwd, update_file_mode, fs, sandbox).await
}
MaybeApplyPatch::Body(args) => verify_apply_patch_args(args, cwd, fs, sandbox).await,
MaybeApplyPatch::ShellParseError(e) => MaybeApplyPatchVerified::ShellParseError(e),
MaybeApplyPatch::PatchParseError(e) => MaybeApplyPatchVerified::CorrectnessError(e.into()),
MaybeApplyPatch::NotApplyPatch => MaybeApplyPatchVerified::NotApplyPatch,
@@ -193,19 +171,7 @@ pub async fn verify_apply_patch_args(
fs: &dyn ExecutorFileSystem,
sandbox: Option<&codex_exec_server::FileSystemSandboxContext>,
) -> MaybeApplyPatchVerified {
verify_apply_patch_args_with_mode(args, cwd, ApplyPatchFileUpdateMode::default(), fs, sandbox)
.await
}
/// Verifies parsed patch arguments using the selected file-update mode.
pub async fn verify_apply_patch_args_with_mode(
args: ApplyPatchArgs,
cwd: &PathUri,
update_file_mode: ApplyPatchFileUpdateMode,
fs: &dyn ExecutorFileSystem,
sandbox: Option<&codex_exec_server::FileSystemSandboxContext>,
) -> MaybeApplyPatchVerified {
match try_verify_apply_patch_args(args, cwd, update_file_mode, fs, sandbox).await {
match try_verify_apply_patch_args(args, cwd, fs, sandbox).await {
Ok(action) => MaybeApplyPatchVerified::Body(action),
Err(err) => MaybeApplyPatchVerified::CorrectnessError(err),
}
@@ -214,7 +180,6 @@ pub async fn verify_apply_patch_args_with_mode(
async fn try_verify_apply_patch_args(
args: ApplyPatchArgs,
cwd: &PathUri,
update_file_mode: ApplyPatchFileUpdateMode,
fs: &dyn ExecutorFileSystem,
sandbox: Option<&codex_exec_server::FileSystemSandboxContext>,
) -> Result<ApplyPatchAction, ApplyPatchError> {
@@ -265,14 +230,7 @@ async fn try_verify_apply_patch_args(
unified_diff,
content: contents,
..
} = unified_diff_from_chunks_with_mode(
&path,
&chunks,
update_file_mode,
fs,
sandbox,
)
.await?;
} = unified_diff_from_chunks(&path, &chunks, fs, sandbox).await?;
changes.insert(
path,
ApplyPatchFileChange::Update {
@@ -288,7 +246,6 @@ async fn try_verify_apply_patch_args(
}
Ok(ApplyPatchAction {
changes,
update_file_mode,
patch,
cwd: effective_cwd,
})
@@ -912,7 +869,6 @@ PATCH"#,
new_content: "updated session directory content\n".to_string(),
},
)]),
update_file_mode: ApplyPatchFileUpdateMode::default(),
patch: argv[1].clone(),
cwd: PathUri::from_host_native_path(session_dir.path())
.expect("absolute test path"),
codex-rs/apply-patch/src/lib.rs
+3 -39
View File
@@ -34,13 +34,10 @@ pub use file_update::ApplyPatchFileUpdate;
use file_update::derive_new_contents_from_chunks;
pub use file_update::unified_diff_from_chunks;
pub use file_update::unified_diff_from_chunks_with_context;
pub(crate) use file_update::unified_diff_from_chunks_with_mode;
pub use invocation::MaybeApplyPatch;
pub use invocation::maybe_parse_apply_patch;
pub use invocation::maybe_parse_apply_patch_verified;
pub use invocation::maybe_parse_apply_patch_verified_with_mode;
pub use invocation::verify_apply_patch_args;
pub use invocation::verify_apply_patch_args_with_mode;
pub use standalone_executable::main;
use crate::invocation::ExtractHeredocError;
@@ -54,25 +51,14 @@ use crate::invocation::ExtractHeredocError;
/// surface.
pub const CODEX_CORE_APPLY_PATCH_ARG1: &str = "--codex-run-as-apply-patch";
/// Internal environment variable used to carry the selected update mode
/// through the arg0-dispatched standalone executable.
/// Compatibility opt-in for older standalone `apply_patch` executables.
/// Current executables always preserve line endings.
pub const CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR: &str =
"CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS";
/// Controls how updates reconstruct the target file after matching a patch.
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
pub enum ApplyPatchFileUpdateMode {
/// Preserve the historical behavior of normalizing updated files to LF.
#[default]
NormalizeToLf,
/// Preserve existing line endings and use the file's preferred ending for new lines.
PreserveLineEndings,
}
/// Policy for one patch application. Standalone callers follow symlinks by default.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct ApplyPatchOptions {
pub update_file_mode: ApplyPatchFileUpdateMode,
/// Whether filesystem operations may resolve symlinks in any path component.
pub follow_symlinks: bool,
}
@@ -80,21 +66,11 @@ pub struct ApplyPatchOptions {
impl Default for ApplyPatchOptions {
fn default() -> Self {
Self {
update_file_mode: ApplyPatchFileUpdateMode::default(),
follow_symlinks: true,
}
}
}
/// Reads the update mode selected for an arg0-dispatched `apply_patch` process.
#[doc(hidden)]
pub fn apply_patch_file_update_mode_from_env() -> ApplyPatchFileUpdateMode {
match std::env::var(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR).as_deref() {
Ok("1") => ApplyPatchFileUpdateMode::PreserveLineEndings,
_ => ApplyPatchFileUpdateMode::NormalizeToLf,
}
}
#[derive(Debug, Error, PartialEq)]
pub enum ApplyPatchError {
#[error(transparent)]
@@ -193,8 +169,6 @@ pub enum MaybeApplyPatchVerified {
pub struct ApplyPatchAction {
changes: HashMap<PathUri, ApplyPatchFileChange>,
update_file_mode: ApplyPatchFileUpdateMode,
/// The raw patch argument that can be used to apply the patch. i.e., if the
/// original arg was parsed in "lenient" mode with a
/// heredoc, this should be the value without the heredoc wrapper.
@@ -214,11 +188,6 @@ impl ApplyPatchAction {
&self.changes
}
/// Returns the update mode selected while the patch was verified.
pub fn update_file_mode(&self) -> ApplyPatchFileUpdateMode {
self.update_file_mode
}
/// Should be used exclusively for testing. (Not worth the overhead of
/// creating a feature flag for this.)
pub fn new_add_for_test(path: &PathUri, content: String) -> Self {
@@ -235,7 +204,6 @@ impl ApplyPatchAction {
#[expect(clippy::expect_used)]
Self {
changes,
update_file_mode: ApplyPatchFileUpdateMode::default(),
cwd: path.parent().expect("path should have parent"),
patch,
}
@@ -475,10 +443,7 @@ async fn apply_hunks_to_files(
sandbox: Option<&FileSystemSandboxContext>,
delta: &mut AppliedPatchDelta,
) -> anyhow::Result<AffectedPaths> {
let ApplyPatchOptions {
update_file_mode,
follow_symlinks,
} = options;
let ApplyPatchOptions { follow_symlinks } = options;
if hunks.is_empty() {
anyhow::bail!("No files were modified.");
}
@@ -610,7 +575,6 @@ async fn apply_hunks_to_files(
} = derive_new_contents_from_chunks(
&path_uri,
chunks,
update_file_mode,
fs,
follow_symlinks,
sandbox,
codex-rs/apply-patch/src/seek_sequence.rs
+5 -34
View File
@@ -14,7 +14,6 @@ pub(crate) fn seek_sequence(
pattern: &[String],
start: usize,
eof: bool,
update_file_mode: crate::ApplyPatchFileUpdateMode,
) -> Option<usize> {
if pattern.is_empty() {
return Some(start);
@@ -29,10 +28,7 @@ pub(crate) fn seek_sequence(
}
let search_start = if eof && lines.len() >= pattern.len() {
let eof_start = lines.len() - pattern.len();
match update_file_mode {
crate::ApplyPatchFileUpdateMode::NormalizeToLf => eof_start,
crate::ApplyPatchFileUpdateMode::PreserveLineEndings => eof_start.max(start),
}
eof_start.max(start)
} else {
start
};
@@ -117,7 +113,6 @@ pub(crate) fn seek_sequence(
#[cfg(test)]
mod tests {
use super::seek_sequence;
use crate::ApplyPatchFileUpdateMode;
use std::string::ToString;
fn to_vec(strings: &[&str]) -> Vec<String> {
@@ -129,13 +124,7 @@ mod tests {
let lines = to_vec(&["foo", "bar", "baz"]);
let pattern = to_vec(&["bar", "baz"]);
assert_eq!(
seek_sequence(
&lines,
&pattern,
/*start*/ 0,
/*eof*/ false,
ApplyPatchFileUpdateMode::NormalizeToLf,
),
seek_sequence(&lines, &pattern, /*start*/ 0, /*eof*/ false,),
Some(1)
);
}
@@ -146,13 +135,7 @@ mod tests {
// Pattern omits trailing whitespace.
let pattern = to_vec(&["foo", "bar"]);
assert_eq!(
seek_sequence(
&lines,
&pattern,
/*start*/ 0,
/*eof*/ false,
ApplyPatchFileUpdateMode::NormalizeToLf,
),
seek_sequence(&lines, &pattern, /*start*/ 0, /*eof*/ false,),
Some(0)
);
}
@@ -163,13 +146,7 @@ mod tests {
// Pattern omits any additional whitespace.
let pattern = to_vec(&["foo", "bar"]);
assert_eq!(
seek_sequence(
&lines,
&pattern,
/*start*/ 0,
/*eof*/ false,
ApplyPatchFileUpdateMode::NormalizeToLf,
),
seek_sequence(&lines, &pattern, /*start*/ 0, /*eof*/ false,),
Some(0)
);
}
@@ -180,13 +157,7 @@ mod tests {
let pattern = to_vec(&["too", "many", "lines"]);
// Should not panic – must return None when pattern cannot possibly fit.
assert_eq!(
seek_sequence(
&lines,
&pattern,
/*start*/ 0,
/*eof*/ false,
ApplyPatchFileUpdateMode::NormalizeToLf,
),
seek_sequence(&lines, &pattern, /*start*/ 0, /*eof*/ false,),
None
);
}
codex-rs/apply-patch/src/standalone_executable.rs
+1 -6
View File
@@ -67,13 +67,8 @@ pub fn run_main() -> i32 {
};
// TODO(anp): Discover the standalone executable cwd as PathUri directly.
let cwd = codex_utils_path_uri::PathUri::from_abs_path(&cwd);
let update_file_mode = crate::apply_patch_file_update_mode_from_env();
match runtime.block_on(crate::apply_patch_with_options(
match runtime.block_on(crate::apply_patch(
&patch_arg,
crate::ApplyPatchOptions {
update_file_mode,
..Default::default()
},
&cwd,
&mut stdout,
&mut stderr,
codex-rs/apply-patch/tests/suite/no_follow.rs
-3
View File
@@ -39,7 +39,6 @@ async fn no_follow_rejects_leaf_and_ancestor_links_for_every_patch_operation() -
&patch,
ApplyPatchOptions {
follow_symlinks: false,
..Default::default()
},
&PathUri::from_host_native_path(&work)?,
&mut Vec::new(),
@@ -88,7 +87,6 @@ async fn no_follow_rechecks_paths_after_verification() -> anyhow::Result<()> {
&action.patch,
ApplyPatchOptions {
follow_symlinks: false,
..Default::default()
},
&cwd,
&mut Vec::new(),
@@ -119,7 +117,6 @@ async fn no_follow_applies_regular_files_and_default_still_follows_links() -> an
patch,
ApplyPatchOptions {
follow_symlinks: false,
..Default::default()
},
&cwd,
&mut Vec::new(),
codex-rs/apply-patch/tests/suite/scenarios.rs
-2
View File
@@ -1,5 +1,4 @@
use anyhow::Context;
use codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR;
use codex_utils_cargo_bin::find_resource;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
@@ -49,7 +48,6 @@ fn run_apply_patch_scenario(dir: &Path) -> anyhow::Result<()> {
// final filesystem state, which we compare below.
Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?)
.arg(patch)
.env(CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR, "1")
.current_dir(tmp.path())
.output()
.with_context(|| format!("failed to run scenario {}", dir.display()))?;
codex-rs/apply-patch/tests/suite/tool.rs
-41
View File
@@ -1,5 +1,4 @@
use assert_cmd::Command;
use codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR;
use pretty_assertions::assert_eq;
use std::fs;
use std::path::Path;
@@ -8,7 +7,6 @@ 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())
}
@@ -35,7 +33,6 @@ fn assert_apply_patch_updates_file(
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)
}
@@ -107,26 +104,6 @@ fn test_apply_patch_cli_rejects_overlapping_end_of_file_chunks() -> anyhow::Resu
Ok(())
}
#[test]
fn test_apply_patch_cli_allows_overlapping_eof_chunks_in_legacy_mode() -> anyhow::Result<()> {
let tmp = tempdir()?;
let target_path = tmp.path().join("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";
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");
assert_eq!(fs::read_to_string(target_path)?, "first\n");
Ok(())
}
#[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";
@@ -151,24 +128,6 @@ fn test_apply_patch_cli_appends_after_trailing_blank_crlf_line() -> anyhow::Resu
)
}
#[test]
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";
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(())
}
#[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";
codex-rs/arg0/src/lib.rs
+1 -6
View File
@@ -134,13 +134,8 @@ pub fn arg0_dispatch() -> Option<Arg0PathEntryGuard> {
Err(_) => std::process::exit(1),
};
let cwd = cwd.into();
let update_file_mode = codex_apply_patch::apply_patch_file_update_mode_from_env();
match runtime.block_on(codex_apply_patch::apply_patch_with_options(
match runtime.block_on(codex_apply_patch::apply_patch(
&patch_arg,
codex_apply_patch::ApplyPatchOptions {
update_file_mode,
..Default::default()
},
&cwd,
&mut stdout,
&mut stderr,
codex-rs/core/src/exec_env.rs
+8 -15
View File
@@ -1,6 +1,4 @@
pub use codex_apply_patch::CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR;
use codex_features::Feature;
use codex_features::Features;
use codex_protocol::SessionId;
use codex_protocol::ThreadId;
#[cfg(test)]
@@ -70,20 +68,15 @@ 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) {
/// Enables line-ending preservation for older standalone patch executables,
/// including those provided by remote executors. Override inherited values so
/// they cannot restore the legacy normalization behavior.
pub fn inject_apply_patch_env(env: &mut HashMap<String, String>) {
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(),
);
}
env.insert(
CODEX_APPLY_PATCH_PRESERVE_LINE_ENDINGS_ENV_VAR.to_string(),
"1".to_string(),
);
}
#[cfg(all(test, target_os = "windows"))]
codex-rs/core/src/exec_env_tests.rs
+2 -8
View File
@@ -42,18 +42,12 @@ fn inject_permission_profile_env_removes_stale_value_without_active_profile() {
}
#[test]
fn inject_apply_patch_env_follows_preserve_line_endings_feature() {
fn inject_apply_patch_env_enables_older_executables_despite_stale_values() {
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);
inject_apply_patch_env(&mut env);
assert_eq!(
env,
HashMap::from([(
codex-rs/core/src/tasks/user_shell.rs
+1 -1
View File
@@ -167,7 +167,7 @@ pub(crate) async fn execute_user_shell_command(
let shell_environment_policy = turn_environment.shell_environment_policy();
let mut exec_env_map = create_env(shell_environment_policy, Some(session.thread_id));
inject_session_env(&mut exec_env_map, session.session_id());
inject_apply_patch_env(&mut exec_env_map, &turn_context.config.features);
inject_apply_patch_env(&mut exec_env_map);
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
+3 -25
View File
@@ -42,7 +42,6 @@ use crate::tools::sandboxing::ToolCtx;
use crate::windows_sandbox::windows_sandbox_level_for_legacy_checks;
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;
@@ -61,18 +60,6 @@ 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)]
@@ -318,7 +305,6 @@ impl ApplyPatchHandler {
) -> Result<Box<dyn crate::tools::context::ToolOutput>, FunctionCallError> {
let ToolInvocation {
session,
turn,
step_context,
cancellation_token,
tracker,
@@ -352,10 +338,9 @@ impl ApplyPatchHandler {
)?;
let fs = turn_environment.environment.get_filesystem();
let sandbox = turn_environment.sandbox_context(/*additional_permissions*/ None);
match codex_apply_patch::verify_apply_patch_args_with_mode(
match codex_apply_patch::verify_apply_patch_args(
args,
turn_environment.cwd(),
apply_patch_file_update_mode(&turn),
fs.as_ref(),
Some(&sandbox),
)
@@ -460,16 +445,9 @@ pub(crate) async fn intercept_apply_patch(
call_id: &str,
tool_name: &str,
) -> Result<Option<FunctionToolOutput>, FunctionCallError> {
let turn = &step_context.turn;
let sandbox = turn_environment.sandbox_context(/*additional_permissions*/ None);
match codex_apply_patch::maybe_parse_apply_patch_verified_with_mode(
command,
cwd,
apply_patch_file_update_mode(turn),
fs,
Some(&sandbox),
)
.await
match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, 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
@@ -58,24 +58,6 @@ 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/runtimes/apply_patch.rs
-1
View File
@@ -179,7 +179,6 @@ impl ToolRuntime<ApplyPatchRequest, ApplyPatchRuntimeOutput> for ApplyPatchRunti
let result = codex_apply_patch::apply_patch_with_options(
&req.action.patch,
ApplyPatchOptions {
update_file_mode: req.action.update_file_mode(),
// Only reject links when an otherwise-required sandbox was bypassed.
// Executor-managed sandboxes can have SandboxType::None.
follow_symlinks: attempt.sandbox_requested
codex-rs/core/src/unified_exec/process_manager.rs
+1 -2
View File
@@ -1440,7 +1440,6 @@ impl UnifiedExecProcessManager {
cwd: PathUri,
context: &UnifiedExecContext,
) -> Result<(UnifiedExecAttempt, Option<DeferredNetworkApproval>), UnifiedExecError> {
let turn = &context.step_context.turn;
let shell_environment_policy = request.turn_environment.shell_environment_policy();
let local_policy_env = create_env(shell_environment_policy, /*thread_id*/ None);
let mut env = local_policy_env.clone();
@@ -1449,7 +1448,7 @@ impl UnifiedExecProcessManager {
context.session.thread_id.to_string(),
);
inject_session_env(&mut env, context.session.session_id());
inject_apply_patch_env(&mut env, &turn.config.features);
inject_apply_patch_env(&mut env);
let active_permission_profile = request.turn_environment.active_permission_profile();
inject_permission_profile_env(&mut env, active_permission_profile.as_ref());
let mut env = apply_unified_exec_env(env);
codex-rs/core/src/unified_exec/shell_snapshot.rs
+1 -1
View File
@@ -113,7 +113,7 @@ impl Session {
session.thread_id().to_string(),
);
inject_session_env(&mut env, session.session_id());
inject_apply_patch_env(&mut env, &config.features);
inject_apply_patch_env(&mut env);
inject_permission_profile_env(
&mut env,
environment.active_permission_profile().as_ref(),
codex-rs/core/tests/suite/apply_patch_cli.rs
+8 -59
View File
@@ -347,15 +347,11 @@ fn apply_patch_responses(
]
}
async fn assert_apply_patch_crlf_update(
configure: impl FnOnce(TestCodexBuilder) -> TestCodexBuilder,
model_output: CrLfApplyPatchModelOutput,
expected: &str,
) -> Result<()> {
async fn assert_apply_patch_crlf_update(model_output: CrLfApplyPatchModelOutput) -> Result<()> {
skip_if_no_network!(Ok(()));
let harness = apply_patch_harness_with(configure).await?;
let call_id = "apply-patch-crlf-rollout";
let harness = apply_patch_harness().await?;
let call_id = "apply-patch-crlf";
let file_name = "crlf.txt";
harness.write_file(file_name, "before\r\n").await?;
let patch = format!(
@@ -385,7 +381,7 @@ async fn assert_apply_patch_crlf_update(
)
.await?;
assert_eq!(harness.read_file_text(file_name).await?, expected);
assert_eq!(harness.read_file_text(file_name).await?, "after\r\n");
Ok(())
}
@@ -396,61 +392,14 @@ enum CrLfApplyPatchModelOutput {
}
#[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
async fn apply_patch_preserves_crlf_by_default() -> Result<()> {
assert_apply_patch_crlf_update(CrLfApplyPatchModelOutput::CustomTool).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<()> {
async fn apply_patch_shell_heredoc_preserves_crlf_by_default() -> Result<()> {
skip_if_wine_exec!(Ok(()), "uses a POSIX shell heredoc");
assert_apply_patch_crlf_update(
|builder| builder,
CrLfApplyPatchModelOutput::ExecCommandViaHeredoc,
"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::ExecCommandViaHeredoc,
"after\r\n",
)
.await
assert_apply_patch_crlf_update(CrLfApplyPatchModelOutput::ExecCommandViaHeredoc).await
}
#[cfg(target_os = "linux")]
codex-rs/features/src/lib.rs
+2 -2
View File
@@ -156,7 +156,7 @@ pub enum Feature {
TerminalVisualizationInstructions,
/// Stream structured progress while apply_patch input is being generated.
ApplyPatchStreamingEvents,
/// Preserve existing line endings when apply_patch updates files.
/// Removed compatibility flag. Patches always preserve existing line endings.
ApplyPatchPreserveLineEndings,
/// Allow exec tools to request additional permissions while staying sandboxed.
ExecPermissionApprovals,
@@ -1257,7 +1257,7 @@ pub const FEATURES: &[FeatureSpec] = &[
FeatureSpec {
id: Feature::ApplyPatchPreserveLineEndings,
key: "apply_patch_preserve_line_endings",
stage: Stage::UnderDevelopment,
stage: Stage::Removed,
default_enabled: false,
},
FeatureSpec {