mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Preserve raw apply_patch path spellings
This commit is contained in:
@@ -197,6 +197,7 @@ async fn try_verify_apply_patch_args(
|
||||
let mut changes = HashMap::new();
|
||||
for hunk in hunks {
|
||||
let path = hunk.resolve_path(&effective_cwd)?;
|
||||
let move_path = hunk.resolve_move_path(&effective_cwd)?;
|
||||
match hunk {
|
||||
Hunk::AddFile { contents, .. } => {
|
||||
changes.insert(path, ApplyPatchFileChange::Add { content: contents });
|
||||
@@ -210,9 +211,7 @@ async fn try_verify_apply_patch_args(
|
||||
})?;
|
||||
changes.insert(path, ApplyPatchFileChange::Delete { content });
|
||||
}
|
||||
Hunk::UpdateFile {
|
||||
move_path, chunks, ..
|
||||
} => {
|
||||
Hunk::UpdateFile { chunks, .. } => {
|
||||
let ApplyPatchFileUpdate {
|
||||
unified_diff,
|
||||
content: contents,
|
||||
@@ -222,9 +221,7 @@ async fn try_verify_apply_patch_args(
|
||||
path,
|
||||
ApplyPatchFileChange::Update {
|
||||
unified_diff,
|
||||
move_path: move_path
|
||||
.map(|path| effective_cwd.join(&path.to_string_lossy()))
|
||||
.transpose()?,
|
||||
move_path,
|
||||
new_content: contents,
|
||||
},
|
||||
);
|
||||
@@ -398,7 +395,6 @@ mod tests {
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use std::string::ToString;
|
||||
use tempfile::tempdir;
|
||||
|
||||
@@ -446,7 +442,7 @@ mod tests {
|
||||
|
||||
fn expected_single_add() -> Vec<Hunk> {
|
||||
vec![Hunk::AddFile {
|
||||
path: PathBuf::from("foo"),
|
||||
path: "foo".to_string(),
|
||||
contents: "hi\n".to_string(),
|
||||
}]
|
||||
}
|
||||
@@ -545,7 +541,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
hunks,
|
||||
vec![Hunk::AddFile {
|
||||
path: PathBuf::from("foo"),
|
||||
path: "foo".to_string(),
|
||||
contents: "hi\n".to_string()
|
||||
}]
|
||||
);
|
||||
@@ -573,7 +569,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
hunks,
|
||||
vec![Hunk::AddFile {
|
||||
path: PathBuf::from("foo"),
|
||||
path: "foo".to_string(),
|
||||
contents: "hi\n".to_string()
|
||||
}]
|
||||
);
|
||||
@@ -616,7 +612,7 @@ PATCH"#,
|
||||
assert_eq!(
|
||||
hunks,
|
||||
vec![Hunk::AddFile {
|
||||
path: PathBuf::from("foo"),
|
||||
path: "foo".to_string(),
|
||||
contents: "hi\n".to_string()
|
||||
}]
|
||||
);
|
||||
|
||||
@@ -351,9 +351,9 @@ pub async fn apply_hunks(
|
||||
/// Tracks file paths affected by applying a patch, preserving the path spelling
|
||||
/// from the patch for user-facing summaries.
|
||||
pub struct AffectedPaths {
|
||||
pub added: Vec<PathBuf>,
|
||||
pub modified: Vec<PathBuf>,
|
||||
pub deleted: Vec<PathBuf>,
|
||||
pub added: Vec<String>,
|
||||
pub modified: Vec<String>,
|
||||
pub deleted: Vec<String>,
|
||||
}
|
||||
|
||||
/// Apply the hunks to the filesystem, returning which files were added, modified, or deleted.
|
||||
@@ -369,9 +369,9 @@ async fn apply_hunks_to_files(
|
||||
anyhow::bail!("No files were modified.");
|
||||
}
|
||||
|
||||
let mut added: Vec<PathBuf> = Vec::new();
|
||||
let mut modified: Vec<PathBuf> = Vec::new();
|
||||
let mut deleted: Vec<PathBuf> = Vec::new();
|
||||
let mut added = Vec::new();
|
||||
let mut modified = Vec::new();
|
||||
let mut deleted = Vec::new();
|
||||
// A failed write can still have modified the target before surfacing an
|
||||
// error (for example by truncating before ENOSPC), so the accumulated
|
||||
// delta is no longer exact when a write fails.
|
||||
@@ -389,8 +389,9 @@ async fn apply_hunks_to_files(
|
||||
|
||||
// TODO(anp): Carry PathUri through committed patch deltas and the turn diff tracker.
|
||||
for hunk in hunks {
|
||||
let affected_path = hunk.path().to_path_buf();
|
||||
let affected_path = hunk.path().to_string();
|
||||
let path_uri = hunk.resolve_path(cwd)?;
|
||||
let move_path_uri = hunk.resolve_move_path(cwd)?;
|
||||
match hunk {
|
||||
Hunk::AddFile { contents, .. } => {
|
||||
let overwritten_content =
|
||||
@@ -462,16 +463,13 @@ async fn apply_hunks_to_files(
|
||||
}
|
||||
deleted.push(affected_path);
|
||||
}
|
||||
Hunk::UpdateFile {
|
||||
move_path, chunks, ..
|
||||
} => {
|
||||
Hunk::UpdateFile { chunks, .. } => {
|
||||
note_existing_path_delta_support(&path_uri, fs, sandbox, &mut delta.exact).await;
|
||||
let AppliedPatch {
|
||||
original_contents,
|
||||
new_contents,
|
||||
} = derive_new_contents_from_chunks(&path_uri, chunks, fs, sandbox).await?;
|
||||
if let Some(dest) = move_path {
|
||||
let dest_uri = cwd.join(&dest.to_string_lossy())?;
|
||||
if let Some(dest_uri) = move_path_uri {
|
||||
let overwritten_move_content =
|
||||
read_optional_file_text_for_delta(&dest_uri, fs, sandbox, &mut delta.exact)
|
||||
.await;
|
||||
@@ -874,13 +872,13 @@ pub fn print_summary(
|
||||
) -> std::io::Result<()> {
|
||||
writeln!(out, "Success. Updated the following files:")?;
|
||||
for path in &affected.added {
|
||||
writeln!(out, "A {}", path.display())?;
|
||||
writeln!(out, "A {path}")?;
|
||||
}
|
||||
for path in &affected.modified {
|
||||
writeln!(out, "M {}", path.display())?;
|
||||
writeln!(out, "M {path}")?;
|
||||
}
|
||||
for path in &affected.deleted {
|
||||
writeln!(out, "D {}", path.display())?;
|
||||
writeln!(out, "D {path}")?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -899,6 +897,23 @@ mod tests {
|
||||
format!("*** Begin Patch\n{body}\n*** End Patch")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_print_summary_preserves_local_output() {
|
||||
let affected = AffectedPaths {
|
||||
added: vec!["src/new.rs".to_string()],
|
||||
modified: vec!["src/old.rs".to_string()],
|
||||
deleted: vec!["tests/old.rs".to_string()],
|
||||
};
|
||||
let mut output = Vec::new();
|
||||
|
||||
print_summary(&affected, &mut output).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
String::from_utf8(output).unwrap(),
|
||||
"Success. Updated the following files:\nA src/new.rs\nM src/old.rs\nD tests/old.rs\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_add_file_hunk_creates_file_with_contents() {
|
||||
let dir = tempdir().unwrap();
|
||||
|
||||
@@ -29,8 +29,6 @@ use crate::streaming_parser::StreamingPatchParser;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_path_uri::PathUriParseError;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use thiserror::Error;
|
||||
|
||||
@@ -65,15 +63,15 @@ use ParseError::*;
|
||||
#[allow(clippy::enum_variant_names)]
|
||||
pub enum Hunk {
|
||||
AddFile {
|
||||
path: PathBuf,
|
||||
path: String,
|
||||
contents: String,
|
||||
},
|
||||
DeleteFile {
|
||||
path: PathBuf,
|
||||
path: String,
|
||||
},
|
||||
UpdateFile {
|
||||
path: PathBuf,
|
||||
move_path: Option<PathBuf>,
|
||||
path: String,
|
||||
move_path: Option<String>,
|
||||
|
||||
/// Chunks should be in order, i.e. the `change_context` of one chunk
|
||||
/// should occur later in the file than the previous chunk.
|
||||
@@ -87,11 +85,23 @@ impl Hunk {
|
||||
Hunk::UpdateFile { path, .. } => path,
|
||||
Hunk::AddFile { .. } | Hunk::DeleteFile { .. } => self.path(),
|
||||
};
|
||||
cwd.join(&path.to_string_lossy())
|
||||
cwd.join(path)
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_move_path(
|
||||
&self,
|
||||
cwd: &PathUri,
|
||||
) -> Result<Option<PathUri>, PathUriParseError> {
|
||||
match self {
|
||||
Hunk::UpdateFile { move_path, .. } => {
|
||||
move_path.as_deref().map(|path| cwd.join(path)).transpose()
|
||||
}
|
||||
Hunk::AddFile { .. } | Hunk::DeleteFile { .. } => Ok(None),
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the path affected by this hunk, using the move destination for rename hunks.
|
||||
pub fn path(&self) -> &Path {
|
||||
pub fn path(&self) -> &str {
|
||||
match self {
|
||||
Hunk::AddFile { path, .. } => path,
|
||||
Hunk::DeleteFile { path } => path,
|
||||
@@ -287,7 +297,7 @@ fn test_parse_patch() {
|
||||
.unwrap()
|
||||
.hunks,
|
||||
vec![AddFile {
|
||||
path: PathBuf::from("foo"),
|
||||
path: "foo".to_string(),
|
||||
contents: "hi\n".to_string()
|
||||
}]
|
||||
);
|
||||
@@ -332,15 +342,15 @@ fn test_parse_patch() {
|
||||
.hunks,
|
||||
vec![
|
||||
AddFile {
|
||||
path: PathBuf::from("path/add.py"),
|
||||
path: "path/add.py".to_string(),
|
||||
contents: "abc\ndef\n".to_string()
|
||||
},
|
||||
DeleteFile {
|
||||
path: PathBuf::from("path/delete.py")
|
||||
path: "path/delete.py".to_string()
|
||||
},
|
||||
UpdateFile {
|
||||
path: PathBuf::from("path/update.py"),
|
||||
move_path: Some(PathBuf::from("path/update2.py")),
|
||||
path: "path/update.py".to_string(),
|
||||
move_path: Some("path/update2.py".to_string()),
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: Some("def f():".to_string()),
|
||||
old_lines: vec![" pass".to_string()],
|
||||
@@ -366,7 +376,7 @@ fn test_parse_patch() {
|
||||
.hunks,
|
||||
vec![
|
||||
UpdateFile {
|
||||
path: PathBuf::from("file.py"),
|
||||
path: "file.py".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -376,7 +386,7 @@ fn test_parse_patch() {
|
||||
}],
|
||||
},
|
||||
AddFile {
|
||||
path: PathBuf::from("other.py"),
|
||||
path: "other.py".to_string(),
|
||||
contents: "content\n".to_string()
|
||||
}
|
||||
]
|
||||
@@ -396,7 +406,7 @@ fn test_parse_patch() {
|
||||
.unwrap()
|
||||
.hunks,
|
||||
vec![UpdateFile {
|
||||
path: PathBuf::from("file2.py"),
|
||||
path: "file2.py".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -416,7 +426,7 @@ fn test_parse_patch_preserves_end_of_file_marker() {
|
||||
parse_patch(patch),
|
||||
Ok(ApplyPatchArgs {
|
||||
hunks: vec![UpdateFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -457,14 +467,14 @@ fn test_parse_patch_accepts_relative_and_absolute_hunk_paths() {
|
||||
.hunks,
|
||||
vec![
|
||||
AddFile {
|
||||
path: PathBuf::from("relative-add.py"),
|
||||
path: "relative-add.py".to_string(),
|
||||
contents: "content\n".to_string()
|
||||
},
|
||||
DeleteFile {
|
||||
path: absolute_delete.to_path_buf()
|
||||
path: absolute_delete.display().to_string()
|
||||
},
|
||||
UpdateFile {
|
||||
path: absolute_update.to_path_buf(),
|
||||
path: absolute_update.display().to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -477,6 +487,41 @@ fn test_parse_patch_accepts_relative_and_absolute_hunk_paths() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_patch_preserves_windows_path_spelling() {
|
||||
let patch = r#"*** Begin Patch
|
||||
*** Add File: src\new.rs
|
||||
+new
|
||||
*** Update File: C:\repo\old.rs
|
||||
*** Move to: D:\target\new.rs
|
||||
@@
|
||||
-old
|
||||
+new
|
||||
*** End Patch"#;
|
||||
|
||||
let parsed = parse_patch_text(patch, ParseMode::Strict).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
parsed.hunks,
|
||||
vec![
|
||||
AddFile {
|
||||
path: r"src\new.rs".to_string(),
|
||||
contents: "new\n".to_string(),
|
||||
},
|
||||
UpdateFile {
|
||||
path: r"C:\repo\old.rs".to_string(),
|
||||
move_path: Some(r"D:\target\new.rs".to_string()),
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
old_lines: vec!["old".to_string()],
|
||||
new_lines: vec!["new".to_string()],
|
||||
is_end_of_file: false,
|
||||
}],
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_hunk_resolve_path_accepts_relative_and_absolute_paths() {
|
||||
let cwd_dir = tempfile::tempdir().unwrap();
|
||||
@@ -489,20 +534,20 @@ fn test_hunk_resolve_path_accepts_relative_and_absolute_paths() {
|
||||
for (hunk, expected_path) in [
|
||||
(
|
||||
AddFile {
|
||||
path: PathBuf::from("relative-add.py"),
|
||||
path: "relative-add.py".to_string(),
|
||||
contents: String::new(),
|
||||
},
|
||||
cwd.join("relative-add.py").unwrap(),
|
||||
),
|
||||
(
|
||||
DeleteFile {
|
||||
path: PathBuf::from("relative-delete.py"),
|
||||
path: "relative-delete.py".to_string(),
|
||||
},
|
||||
cwd.join("relative-delete.py").unwrap(),
|
||||
),
|
||||
(
|
||||
UpdateFile {
|
||||
path: PathBuf::from("relative-update.py"),
|
||||
path: "relative-update.py".to_string(),
|
||||
move_path: None,
|
||||
chunks: Vec::new(),
|
||||
},
|
||||
@@ -510,20 +555,20 @@ fn test_hunk_resolve_path_accepts_relative_and_absolute_paths() {
|
||||
),
|
||||
(
|
||||
AddFile {
|
||||
path: absolute_add.to_path_buf(),
|
||||
path: absolute_add.display().to_string(),
|
||||
contents: String::new(),
|
||||
},
|
||||
PathUri::from_abs_path(&absolute_add),
|
||||
),
|
||||
(
|
||||
DeleteFile {
|
||||
path: absolute_delete.to_path_buf(),
|
||||
path: absolute_delete.display().to_string(),
|
||||
},
|
||||
PathUri::from_abs_path(&absolute_delete),
|
||||
),
|
||||
(
|
||||
UpdateFile {
|
||||
path: absolute_update.to_path_buf(),
|
||||
path: absolute_update.display().to_string(),
|
||||
move_path: None,
|
||||
chunks: Vec::new(),
|
||||
},
|
||||
@@ -534,6 +579,69 @@ fn test_hunk_resolve_path_accepts_relative_and_absolute_paths() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_hunk_resolution_uses_cwd_convention() {
|
||||
for (cwd, path, expected) in [
|
||||
(
|
||||
"file:///C:/repo/worktree",
|
||||
r"src\main.rs",
|
||||
"file:///C:/repo/worktree/src/main.rs",
|
||||
),
|
||||
(
|
||||
"file:///C:/repo/worktree",
|
||||
r"..\shared\lib.rs",
|
||||
"file:///C:/repo/shared/lib.rs",
|
||||
),
|
||||
(
|
||||
"file:///C:/repo/worktree",
|
||||
r"D:\target\main.rs",
|
||||
"file:///D:/target/main.rs",
|
||||
),
|
||||
(
|
||||
"file:///repo/worktree",
|
||||
"../shared/lib.rs",
|
||||
"file:///repo/shared/lib.rs",
|
||||
),
|
||||
(
|
||||
"file:///repo/worktree",
|
||||
"/target/main.rs",
|
||||
"file:///target/main.rs",
|
||||
),
|
||||
] {
|
||||
let cwd = PathUri::parse(cwd).unwrap();
|
||||
let expected = PathUri::parse(expected).unwrap();
|
||||
let hunk = DeleteFile {
|
||||
path: path.to_string(),
|
||||
};
|
||||
|
||||
assert_eq!(hunk.resolve_path(&cwd), Ok(expected));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_hunk_move_resolution_uses_cwd_convention() {
|
||||
let cwd = PathUri::parse("file:///C:/repo/worktree").unwrap();
|
||||
for (move_path, expected) in [
|
||||
(
|
||||
r"renamed\main.rs",
|
||||
"file:///C:/repo/worktree/renamed/main.rs",
|
||||
),
|
||||
(r"..\shared\main.rs", "file:///C:/repo/shared/main.rs"),
|
||||
(r"D:\target\main.rs", "file:///D:/target/main.rs"),
|
||||
] {
|
||||
let hunk = UpdateFile {
|
||||
path: r"src\main.rs".to_string(),
|
||||
move_path: Some(move_path.to_string()),
|
||||
chunks: Vec::new(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
hunk.resolve_move_path(&cwd),
|
||||
Ok(Some(PathUri::parse(expected).unwrap()))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_patch_lenient() {
|
||||
let patch_text = r#"*** Begin Patch
|
||||
@@ -542,7 +650,7 @@ fn test_parse_patch_lenient() {
|
||||
+bar
|
||||
*** End Patch"#;
|
||||
let expected_patch = vec![UpdateFile {
|
||||
path: PathBuf::from("file2.py"),
|
||||
path: "file2.py".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -636,7 +744,7 @@ fn test_parse_patch_environment_id_preamble() {
|
||||
),
|
||||
Ok(ApplyPatchArgs {
|
||||
hunks: vec![AddFile {
|
||||
path: PathBuf::from("hello.txt"),
|
||||
path: "hello.txt".to_string(),
|
||||
contents: "hello\n".to_string(),
|
||||
}],
|
||||
patch: "*** Begin Patch\n*** Environment ID: remote\n*** Add File: hello.txt\n+hello\n*** End Patch".to_string(),
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::parser::ADD_FILE_MARKER;
|
||||
use crate::parser::BEGIN_PATCH_MARKER;
|
||||
use crate::parser::CHANGE_CONTEXT_MARKER;
|
||||
@@ -56,7 +54,7 @@ impl StreamingPatchParser {
|
||||
&& let StreamingParserMode::UpdateFile { hunk_line_number } = self.state.mode
|
||||
{
|
||||
return Err(InvalidHunkError {
|
||||
message: format!("Update file hunk for path '{}' is empty", path.display()),
|
||||
message: format!("Update file hunk for path '{path}' is empty"),
|
||||
line_number: hunk_line_number,
|
||||
});
|
||||
}
|
||||
@@ -107,7 +105,7 @@ impl StreamingPatchParser {
|
||||
if let Some(path) = trimmed.strip_prefix(ADD_FILE_MARKER) {
|
||||
self.ensure_update_hunk_is_not_empty(trimmed)?;
|
||||
self.state.hunks.push(AddFile {
|
||||
path: PathBuf::from(path),
|
||||
path: path.to_string(),
|
||||
contents: String::new(),
|
||||
});
|
||||
self.state.mode = StreamingParserMode::AddFile;
|
||||
@@ -116,7 +114,7 @@ impl StreamingPatchParser {
|
||||
if let Some(path) = trimmed.strip_prefix(DELETE_FILE_MARKER) {
|
||||
self.ensure_update_hunk_is_not_empty(trimmed)?;
|
||||
self.state.hunks.push(DeleteFile {
|
||||
path: PathBuf::from(path),
|
||||
path: path.to_string(),
|
||||
});
|
||||
self.state.mode = StreamingParserMode::DeleteFile;
|
||||
return Ok(true);
|
||||
@@ -124,7 +122,7 @@ impl StreamingPatchParser {
|
||||
if let Some(path) = trimmed.strip_prefix(UPDATE_FILE_MARKER) {
|
||||
self.ensure_update_hunk_is_not_empty(trimmed)?;
|
||||
self.state.hunks.push(UpdateFile {
|
||||
path: PathBuf::from(path),
|
||||
path: path.to_string(),
|
||||
move_path: None,
|
||||
chunks: Vec::new(),
|
||||
});
|
||||
@@ -254,7 +252,7 @@ impl StreamingPatchParser {
|
||||
&& move_path.is_none()
|
||||
&& let Some(move_to_path) = update_line.strip_prefix(MOVE_TO_MARKER)
|
||||
{
|
||||
*move_path = Some(PathBuf::from(move_to_path));
|
||||
*move_path = Some(move_to_path.to_string());
|
||||
self.state.mode = StreamingParserMode::UpdateFile { hunk_line_number };
|
||||
return Ok(());
|
||||
}
|
||||
@@ -411,7 +409,6 @@ impl StreamingPatchParser {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use super::*;
|
||||
|
||||
@@ -421,14 +418,14 @@ mod tests {
|
||||
assert_eq!(
|
||||
parser.push_delta("*** Begin Patch\n*** Add File: src/hello.txt\n+hello\n+wor"),
|
||||
Ok(vec![AddFile {
|
||||
path: PathBuf::from("src/hello.txt"),
|
||||
path: "src/hello.txt".to_string(),
|
||||
contents: "hello\n".to_string(),
|
||||
}])
|
||||
);
|
||||
assert_eq!(
|
||||
parser.push_delta("ld\n"),
|
||||
Ok(vec![AddFile {
|
||||
path: PathBuf::from("src/hello.txt"),
|
||||
path: "src/hello.txt".to_string(),
|
||||
contents: "hello\nworld\n".to_string(),
|
||||
}])
|
||||
);
|
||||
@@ -439,8 +436,8 @@ mod tests {
|
||||
"*** Begin Patch\n*** Update File: src/old.rs\n*** Move to: src/new.rs\n@@\n-old\n+new\n",
|
||||
),
|
||||
Ok(vec![UpdateFile {
|
||||
path: PathBuf::from("src/old.rs"),
|
||||
move_path: Some(PathBuf::from("src/new.rs")),
|
||||
path: "src/old.rs".to_string(),
|
||||
move_path: Some("src/new.rs".to_string()),
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
old_lines: vec!["old".to_string()],
|
||||
@@ -458,7 +455,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
parser.push_delta("\n"),
|
||||
Ok(vec![DeleteFile {
|
||||
path: PathBuf::from("gone.txt"),
|
||||
path: "gone.txt".to_string(),
|
||||
}])
|
||||
);
|
||||
|
||||
@@ -469,11 +466,11 @@ mod tests {
|
||||
),
|
||||
Ok(vec![
|
||||
AddFile {
|
||||
path: PathBuf::from("src/one.txt"),
|
||||
path: "src/one.txt".to_string(),
|
||||
contents: "one\n".to_string(),
|
||||
},
|
||||
DeleteFile {
|
||||
path: PathBuf::from("src/two.txt"),
|
||||
path: "src/two.txt".to_string(),
|
||||
},
|
||||
])
|
||||
);
|
||||
@@ -493,7 +490,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
parser.push_delta(patch),
|
||||
Ok(vec![AddFile {
|
||||
path: PathBuf::from("src/hello.txt"),
|
||||
path: "src/hello.txt".to_string(),
|
||||
contents: "hello\n".to_string(),
|
||||
}])
|
||||
);
|
||||
@@ -628,7 +625,7 @@ mod tests {
|
||||
",
|
||||
),
|
||||
Ok(vec![UpdateFile {
|
||||
path: PathBuf::from("a.txt"),
|
||||
path: "a.txt".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![
|
||||
UpdateFileChunk {
|
||||
@@ -664,7 +661,7 @@ mod tests {
|
||||
",
|
||||
),
|
||||
Ok(vec![UpdateFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -694,7 +691,7 @@ mod tests {
|
||||
"*** Begin Patch\n*** Update File: file.txt\n@@\n+quux\n*** End of File\n\n*** End Patch\n",
|
||||
),
|
||||
Ok(vec![UpdateFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -712,7 +709,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
parser.push_delta("*** Begin Patch\r\n*** Update File: file.txt\r\n@@\r\n-old\r\n+new\r\n*** End Patch\r\n"),
|
||||
Ok(vec![UpdateFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -727,7 +724,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
parser.push_delta("*** Begin Patch\r\n*** Update File: file.txt\r\n@@\r\n-old\r\r\n+new\r\n*** End Patch\r\n"),
|
||||
Ok(vec![UpdateFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -745,14 +742,14 @@ mod tests {
|
||||
assert_eq!(
|
||||
parser.push_delta("*** Begin Patch\n*** Add File: file.txt\n+hello\n*** End Patch"),
|
||||
Ok(vec![AddFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
contents: "hello\n".to_string(),
|
||||
}])
|
||||
);
|
||||
assert_eq!(
|
||||
parser.finish(),
|
||||
Ok(vec![AddFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
contents: "hello\n".to_string(),
|
||||
}])
|
||||
);
|
||||
@@ -763,7 +760,7 @@ mod tests {
|
||||
"*** Begin Patch\n*** Update File: file.txt\n@@\n-old\n+new\n *** End Patch",
|
||||
),
|
||||
Ok(vec![UpdateFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -776,7 +773,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
parser.finish(),
|
||||
Ok(vec![UpdateFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
move_path: None,
|
||||
chunks: vec![UpdateFileChunk {
|
||||
change_context: None,
|
||||
@@ -794,7 +791,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
parser.push_delta("*** Begin Patch\n*** Add File: file.txt\n+hello\n"),
|
||||
Ok(vec![AddFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
contents: "hello\n".to_string(),
|
||||
}])
|
||||
);
|
||||
@@ -824,7 +821,7 @@ mod tests {
|
||||
"*** Begin Patch\n*** Add File: file.txt\n+hello\n*** End Patch\n \t\n",
|
||||
),
|
||||
Ok(vec![AddFile {
|
||||
path: PathBuf::from("file.txt"),
|
||||
path: "file.txt".to_string(),
|
||||
contents: "hello\n".to_string(),
|
||||
}])
|
||||
);
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
use std::collections::BTreeSet;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
@@ -143,7 +142,7 @@ fn convert_apply_patch_hunks_to_protocol(hunks: &[Hunk]) -> HashMap<PathBuf, Fil
|
||||
hunks
|
||||
.iter()
|
||||
.map(|hunk| {
|
||||
let path = hunk_source_path(hunk).to_path_buf();
|
||||
let path = PathBuf::from(hunk_source_path(hunk));
|
||||
let change = match hunk {
|
||||
Hunk::AddFile { contents, .. } => FileChange::Add {
|
||||
content: contents.clone(),
|
||||
@@ -155,7 +154,7 @@ fn convert_apply_patch_hunks_to_protocol(hunks: &[Hunk]) -> HashMap<PathBuf, Fil
|
||||
chunks, move_path, ..
|
||||
} => FileChange::Update {
|
||||
unified_diff: format_update_chunks_for_progress(chunks),
|
||||
move_path: move_path.clone(),
|
||||
move_path: move_path.as_ref().map(PathBuf::from),
|
||||
},
|
||||
};
|
||||
(path, change)
|
||||
@@ -163,7 +162,7 @@ fn convert_apply_patch_hunks_to_protocol(hunks: &[Hunk]) -> HashMap<PathBuf, Fil
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn hunk_source_path(hunk: &Hunk) -> &Path {
|
||||
fn hunk_source_path(hunk: &Hunk) -> &str {
|
||||
match hunk {
|
||||
Hunk::AddFile { path, .. } | Hunk::DeleteFile { path } | Hunk::UpdateFile { path, .. } => {
|
||||
path
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::assert_regex_match;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
@@ -11,7 +12,6 @@ use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_wine_exec;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use pretty_assertions::assert_eq;
|
||||
use regex_lite::Regex;
|
||||
@@ -235,8 +235,6 @@ M {file_name}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn apply_patch_custom_tool_call_reports_failure_output() -> Result<()> {
|
||||
// TODO(anp): Remove after apply-patch assertions use target-native paths.
|
||||
skip_if_wine_exec!(Ok(()), "asserts POSIX apply_patch failure text");
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = apply_patch_harness().await?;
|
||||
@@ -258,11 +256,12 @@ async fn apply_patch_custom_tool_call_reports_failure_output() -> Result<()> {
|
||||
|
||||
let output = harness.apply_patch_output(call_id).await;
|
||||
|
||||
let expected_output = format!(
|
||||
"apply_patch verification failed: Failed to read file to update {}/{missing_file}: No such file or directory (os error 2)",
|
||||
harness.cwd().to_string_lossy()
|
||||
let missing_path = PathUri::from_abs_path(&harness.cwd_abs()).join(missing_file)?;
|
||||
let expected_pattern = format!(
|
||||
r"^apply_patch verification failed: Failed to read file to update {}: [^\r\n]+ \(os error 2\)$",
|
||||
regex_lite::escape(&missing_path.inferred_native_path_string())
|
||||
);
|
||||
assert_eq!(output, expected_output.as_str());
|
||||
assert_regex_match(&expected_pattern, output.as_str());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user