From 771f1ca6ab8a9c430026ac608752f8fcb4ce3c2f Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Mon, 5 Jan 2026 17:41:23 -0800 Subject: [PATCH] fix: accept whitespace-padded patch markers (#8746) Trim whitespace when validating '*** Begin Patch'/'*** End Patch' markers in codex-apply-patch so padded marker lines parse as intended, and add regression coverage (unit + fixture scenario); this avoids apply_patch failures when models include extra spacing. Tested with cargo test -p codex-apply-patch. --- codex-rs/apply-patch/src/parser.rs | 26 +++++++++++++++++-- .../expected/file.txt | 1 + .../input/file.txt | 1 + .../patch.txt | 6 +++++ 4 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/expected/file.txt create mode 100644 codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/input/file.txt create mode 100644 codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/patch.txt diff --git a/codex-rs/apply-patch/src/parser.rs b/codex-rs/apply-patch/src/parser.rs index 768c89ad7..8785b3851 100644 --- a/codex-rs/apply-patch/src/parser.rs +++ b/codex-rs/apply-patch/src/parser.rs @@ -227,11 +227,14 @@ fn check_start_and_end_lines_strict( first_line: Option<&&str>, last_line: Option<&&str>, ) -> Result<(), ParseError> { + let first_line = first_line.map(|line| line.trim()); + let last_line = last_line.map(|line| line.trim()); + match (first_line, last_line) { - (Some(&first), Some(&last)) if first == BEGIN_PATCH_MARKER && last == END_PATCH_MARKER => { + (Some(first), Some(last)) if first == BEGIN_PATCH_MARKER && last == END_PATCH_MARKER => { Ok(()) } - (Some(&first), _) if first != BEGIN_PATCH_MARKER => Err(InvalidPatchError(String::from( + (Some(first), _) if first != BEGIN_PATCH_MARKER => Err(InvalidPatchError(String::from( "The first line of the patch must be '*** Begin Patch'", ))), _ => Err(InvalidPatchError(String::from( @@ -444,6 +447,25 @@ fn test_parse_patch() { "The last line of the patch must be '*** End Patch'".to_string() )) ); + + assert_eq!( + parse_patch_text( + concat!( + "*** Begin Patch", + " ", + "\n*** Add File: foo\n+hi\n", + " ", + "*** End Patch" + ), + ParseMode::Strict + ) + .unwrap() + .hunks, + vec![AddFile { + path: PathBuf::from("foo"), + contents: "hi\n".to_string() + }] + ); assert_eq!( parse_patch_text( "*** Begin Patch\n\ diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/expected/file.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/expected/file.txt new file mode 100644 index 000000000..f719efd43 --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/expected/file.txt @@ -0,0 +1 @@ +two diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/input/file.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/input/file.txt new file mode 100644 index 000000000..5626abf0f --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/input/file.txt @@ -0,0 +1 @@ +one diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/patch.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/patch.txt new file mode 100644 index 000000000..3d2a1dbe5 --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/patch.txt @@ -0,0 +1,6 @@ +*** Begin Patch +*** Update File: file.txt +@@ +-one ++two + *** End Patch