Skip to content

Commit 79e1d14

Browse files
fix: update fs_write tool for relative and absolute paths (#256)
1 parent 11f590e commit 79e1d14

File tree

1 file changed

+101
-13
lines changed

1 file changed

+101
-13
lines changed

crates/chat-cli/src/cli/chat/tools/fs_write.rs

Lines changed: 101 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,11 @@ impl FsWrite {
169169
match self {
170170
FsWrite::Create { path, .. } => {
171171
let file_text = self.canonical_create_command_text();
172-
let relative_path = format_path(cwd, path);
173-
let prev = if ctx.fs().exists(path) {
174-
let file = ctx.fs().read_to_string_sync(path)?;
175-
stylize_output_if_able(ctx, path, &file)
172+
let path = sanitize_path_tool_arg(ctx, path);
173+
let relative_path = format_path(cwd, &path);
174+
let prev = if ctx.fs().exists(&path) {
175+
let file = ctx.fs().read_to_string_sync(&path)?;
176+
stylize_output_if_able(ctx, &path, &file)
176177
} else {
177178
Default::default()
178179
};
@@ -185,8 +186,9 @@ impl FsWrite {
185186
insert_line,
186187
new_str,
187188
} => {
188-
let relative_path = format_path(cwd, path);
189-
let file = ctx.fs().read_to_string_sync(&relative_path)?;
189+
let path = sanitize_path_tool_arg(ctx, path);
190+
let relative_path = format_path(cwd, &path);
191+
let file = ctx.fs().read_to_string_sync(&path)?;
190192

191193
// Diff the old with the new by adding extra context around the line being inserted
192194
// at.
@@ -204,8 +206,9 @@ impl FsWrite {
204206
Ok(())
205207
},
206208
FsWrite::StrReplace { path, old_str, new_str } => {
207-
let relative_path = format_path(cwd, path);
208-
let file = ctx.fs().read_to_string_sync(&relative_path)?;
209+
let path = sanitize_path_tool_arg(ctx, path);
210+
let relative_path = format_path(cwd, &path);
211+
let file = ctx.fs().read_to_string_sync(&path)?;
209212
let (start_line, _) = match line_number_at(&file, old_str) {
210213
Some((start_line, end_line)) => (start_line, end_line),
211214
_ => (0, 0),
@@ -217,8 +220,9 @@ impl FsWrite {
217220
Ok(())
218221
},
219222
FsWrite::Append { path, new_str } => {
220-
let relative_path = format_path(cwd, path);
221-
let start_line = ctx.fs().read_to_string_sync(&relative_path)?.lines().count() + 1;
223+
let path = sanitize_path_tool_arg(ctx, path);
224+
let relative_path = format_path(cwd, &path);
225+
let start_line = ctx.fs().read_to_string_sync(&path)?.lines().count() + 1;
222226
let file = stylize_output_if_able(ctx, &relative_path, new_str);
223227
print_diff(updates, &Default::default(), &file, start_line)?;
224228
Ok(())
@@ -260,7 +264,9 @@ impl FsWrite {
260264
FsWrite::Insert { path, .. } => path,
261265
FsWrite::Append { path, .. } => path,
262266
};
263-
let relative_path = format_path(cwd, path);
267+
// Sanitize the path to handle tilde expansion
268+
let path = sanitize_path_tool_arg(ctx, path);
269+
let relative_path = format_path(cwd, &path);
264270
queue!(
265271
updates,
266272
style::Print("Path: "),
@@ -294,11 +300,23 @@ impl FsWrite {
294300

295301
/// Writes `content` to `path`, adding a newline if necessary.
296302
async fn write_to_file(ctx: &Context, path: impl AsRef<Path>, mut content: String) -> Result<()> {
303+
let path_ref = path.as_ref();
304+
305+
// Log the path being written to
306+
tracing::debug!("Writing to file: {:?}", path_ref);
307+
297308
if !content.ends_with_newline() {
298309
content.push('\n');
299310
}
300-
ctx.fs().write(path.as_ref(), content).await?;
301-
Ok(())
311+
312+
// Try to write the file and provide better error context
313+
match ctx.fs().write(path_ref, content).await {
314+
Ok(_) => Ok(()),
315+
Err(e) => {
316+
tracing::error!("Failed to write to file {:?}: {}", path_ref, e);
317+
Err(eyre::eyre!("Failed to write to file {:?}: {}", path_ref, e))
318+
},
319+
}
302320
}
303321

304322
/// Returns a prefix/suffix pair before and after the content dictated by `[start_line, end_line]`
@@ -951,3 +969,73 @@ mod tests {
951969
assert_eq!(terminal_width_required_for_line_count(999), 3);
952970
}
953971
}
972+
#[tokio::test]
973+
async fn test_fs_write_with_tilde_paths() {
974+
// Create a test context
975+
let ctx = Context::builder().with_test_home().await.unwrap().build_fake();
976+
let mut stdout = std::io::stdout();
977+
978+
// Get the home directory from the context
979+
let home_dir = ctx.env().home().unwrap_or_default();
980+
println!("Test home directory: {:?}", home_dir);
981+
982+
// Create a file directly in the home directory first to ensure it exists
983+
let home_path = ctx.fs().chroot_path(&home_dir);
984+
println!("Chrooted home path: {:?}", home_path);
985+
986+
// Ensure the home directory exists
987+
ctx.fs().create_dir_all(&home_path).await.unwrap();
988+
989+
let v = serde_json::json!({
990+
"path": "~/file.txt",
991+
"command": "create",
992+
"file_text": "content in home file"
993+
});
994+
995+
let result = serde_json::from_value::<FsWrite>(v)
996+
.unwrap()
997+
.invoke(&ctx, &mut stdout)
998+
.await;
999+
1000+
match &result {
1001+
Ok(_) => println!("Writing to ~/file.txt succeeded"),
1002+
Err(e) => println!("Writing to ~/file.txt failed: {:?}", e),
1003+
}
1004+
1005+
assert!(result.is_ok(), "Writing to ~/file.txt should succeed");
1006+
1007+
// Verify content was written correctly
1008+
let file_path = home_path.join("file.txt");
1009+
println!("Checking file at: {:?}", file_path);
1010+
1011+
let content_result = ctx.fs().read_to_string(&file_path).await;
1012+
match &content_result {
1013+
Ok(content) => println!("Read content: {:?}", content),
1014+
Err(e) => println!("Failed to read content: {:?}", e),
1015+
}
1016+
1017+
assert!(content_result.is_ok(), "Should be able to read from expanded path");
1018+
assert_eq!(content_result.unwrap(), "content in home file\n");
1019+
1020+
// Test with "~/nested/path/file.txt" to ensure deep paths work
1021+
let nested_dir = home_path.join("nested").join("path");
1022+
ctx.fs().create_dir_all(&nested_dir).await.unwrap();
1023+
1024+
let v = serde_json::json!({
1025+
"path": "~/nested/path/file.txt",
1026+
"command": "create",
1027+
"file_text": "content in nested path"
1028+
});
1029+
1030+
let result = serde_json::from_value::<FsWrite>(v)
1031+
.unwrap()
1032+
.invoke(&ctx, &mut stdout)
1033+
.await;
1034+
1035+
assert!(result.is_ok(), "Writing to ~/nested/path/file.txt should succeed");
1036+
1037+
// Verify nested path content
1038+
let nested_file_path = nested_dir.join("file.txt");
1039+
let nested_content = ctx.fs().read_to_string(&nested_file_path).await.unwrap();
1040+
assert_eq!(nested_content, "content in nested path\n");
1041+
}

0 commit comments

Comments
 (0)