From 60cd720ae5c8e95776826fe075c8a741aecb72e0 Mon Sep 17 00:00:00 2001 From: jh-block Date: Thu, 26 Feb 2026 14:48:00 +0100 Subject: [PATCH] refactor: split text_editor into read/edit/write tools with fuzzy matching and unified diffs - Split monolithic text_editor tool (with command subparam) into three focused tools: read_file, edit_file, write_file - Remove insert-by-line-number and undo_edit commands - Remove in-memory file_history state - Add fuzzy text matching: strip trailing whitespace, normalize smart quotes, Unicode dashes, and special spaces before matching - Add BOM stripping/preservation for UTF-8 files - Preserve original line endings (CRLF/LF) instead of forcing platform default - Generate proper unified diffs for user-facing edit output using similar crate - Update downstream references in CLI output, export, and ACP server --- Cargo.lock | 1 + crates/goose-acp/src/server.rs | 30 +- crates/goose-cli/src/session/export.rs | 5 +- crates/goose-cli/src/session/output.rs | 5 +- crates/goose-mcp/Cargo.toml | 1 + .../goose-mcp/src/developer/rmcp_developer.rs | 1111 +++-------------- crates/goose-mcp/src/developer/shell.rs | 10 - .../src/developer/tests/test_diff.rs | 74 +- crates/goose-mcp/src/developer/text_editor.rs | 805 +++++++----- 9 files changed, 759 insertions(+), 1283 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index badf42c536..8bc14b5a24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4492,6 +4492,7 @@ dependencies = [ "serial_test", "shell-words", "shellexpand", + "similar", "tempfile", "tokio", "tokio-stream", diff --git a/crates/goose-acp/src/server.rs b/crates/goose-acp/src/server.rs index 3f953ab6ad..f9615d102f 100644 --- a/crates/goose-acp/src/server.rs +++ b/crates/goose-acp/src/server.rs @@ -109,7 +109,15 @@ fn extract_tool_locations( let mut locations = Vec::new(); if let Ok(tool_call) = &tool_request.tool_call { - if tool_call.name != "developer__text_editor" { + let tool_name = tool_call.name.to_string(); + let is_file_tool = matches!( + tool_name.as_str(), + "developer__text_editor" + | "developer__read_file" + | "developer__edit_file" + | "developer__write_file" + ); + if !is_file_tool { return locations; } @@ -120,25 +128,31 @@ fn extract_tool_locations( .and_then(|p| p.as_str()); if let Some(path_str) = path_str { - let command = tool_call - .arguments - .as_ref() - .and_then(|args| args.get("command")) - .and_then(|c| c.as_str()); + // Determine the operation from either the new tool name or legacy command param + let operation = match tool_name.as_str() { + "developer__read_file" => Some("view"), + "developer__edit_file" => Some("str_replace"), + "developer__write_file" => Some("write"), + _ => tool_call + .arguments + .as_ref() + .and_then(|args| args.get("command")) + .and_then(|c| c.as_str()), + }; if let Ok(result) = &tool_response.tool_result { for content in &result.content { if let RawContent::Text(text_content) = &content.raw { let text = &text_content.text; - match command { + match operation { Some("view") => { let line = extract_view_line_range(text) .map(|range| range.0 as u32) .or(Some(1)); locations.push(create_tool_location(path_str, line)); } - Some("str_replace") | Some("insert") => { + Some("str_replace") => { let line = extract_first_line_number(text) .map(|l| l as u32) .or(Some(1)); diff --git a/crates/goose-cli/src/session/export.rs b/crates/goose-cli/src/session/export.rs index e05daecc21..7602312ed4 100644 --- a/crates/goose-cli/src/session/export.rs +++ b/crates/goose-cli/src/session/export.rs @@ -157,7 +157,10 @@ pub fn tool_request_to_markdown(req: &ToolRequest, export_all_content: bool) -> )); } } - "developer__text_editor" => { + "developer__text_editor" + | "developer__read_file" + | "developer__edit_file" + | "developer__write_file" => { if let Some(Value::String(path)) = call.arguments.as_ref().and_then(|args| args.get("path")) { diff --git a/crates/goose-cli/src/session/output.rs b/crates/goose-cli/src/session/output.rs index c458744b09..46f879e555 100644 --- a/crates/goose-cli/src/session/output.rs +++ b/crates/goose-cli/src/session/output.rs @@ -485,7 +485,10 @@ fn render_thinking_streaming( fn render_tool_request(req: &ToolRequest, theme: Theme, debug: bool) { match &req.tool_call { Ok(call) => match call.name.to_string().as_str() { - "developer__text_editor" => render_text_editor_request(call, debug), + "developer__text_editor" + | "developer__read_file" + | "developer__edit_file" + | "developer__write_file" => render_text_editor_request(call, debug), "developer__shell" => render_shell_request(call, debug), "execute" | "execute_code" => render_execute_code_request(call, debug), "delegate" => render_delegate_request(call, debug), diff --git a/crates/goose-mcp/Cargo.toml b/crates/goose-mcp/Cargo.toml index c928ce00cd..14c0c1e72e 100644 --- a/crates/goose-mcp/Cargo.toml +++ b/crates/goose-mcp/Cargo.toml @@ -67,6 +67,7 @@ libc = "0.2" mpatch = "=0.2.0" tokio-util = { workspace = true } shell-words = "1.1.1" +similar = "2.7.0" [dev-dependencies] serial_test = { workspace = true } diff --git a/crates/goose-mcp/src/developer/rmcp_developer.rs b/crates/goose-mcp/src/developer/rmcp_developer.rs index 0a1b3ce147..40dd25cbef 100644 --- a/crates/goose-mcp/src/developer/rmcp_developer.rs +++ b/crates/goose-mcp/src/developer/rmcp_developer.rs @@ -49,7 +49,7 @@ use std::{ future::Future, io::Cursor, path::{Path, PathBuf}, - sync::{Arc, Mutex}, + sync::Arc, }; use xcap::{Monitor, Window}; @@ -65,9 +65,7 @@ use crate::developer::{paths::get_shell_path_dirs, shell::ShellConfig}; use super::analyze::{types::AnalyzeParams, CodeAnalyzer}; use super::editor_models::{create_editor_model, EditorModel}; use super::shell::{configure_shell_command, expand_path, is_absolute_path, kill_process_group}; -use super::text_editor::{ - text_editor_insert, text_editor_replace, text_editor_undo, text_editor_view, text_editor_write, -}; +use super::text_editor::{text_editor_replace, text_editor_view, text_editor_write}; /// Parameters for the screen_capture tool #[derive(Debug, Serialize, Deserialize, JsonSchema)] @@ -81,36 +79,44 @@ pub struct ScreenCaptureParams { pub window_title: Option, } -/// Parameters for the text_editor tool +/// Parameters for the read_file tool #[derive(Debug, Serialize, Deserialize, JsonSchema)] -pub struct TextEditorParams { +pub struct ReadFileParams { /// Absolute path to file or directory, e.g. `/repo/file.py` or `/repo`. pub path: String, - /// The operation to perform. Allowed options are: `view`, `write`, `str_replace`, `insert`, `undo_edit`. - pub command: String, - - /// Unified diff to apply. Supports editing multiple files simultaneously. Cannot create or delete files - /// Example: "--- a/file\n+++ b/file\n@@ -1,3 +1,3 @@\n context\n-old\n+new\n context" - /// Preferred edit method. - pub diff: Option, - /// Optional array of two integers specifying the start and end line numbers to view. /// Line numbers are 1-indexed, and -1 for the end line means read to the end of the file. /// This parameter only applies when viewing files, not directories. pub view_range: Option>, +} - /// The content to write to the file. Required for `write` command. - pub file_text: Option, +/// Parameters for the edit_file tool +#[derive(Debug, Serialize, Deserialize, JsonSchema)] +pub struct EditFileParams { + /// Absolute path to file or directory, e.g. `/repo/file.py` or `/repo`. + pub path: String, - /// The old string to replace. + /// The old string to replace. Must match exactly once in the file. pub old_str: Option, - /// The new string to replace with. Required for `insert` command. + /// The new string to replace old_str with. pub new_str: Option, - /// The line number after which to insert text (0 for beginning). Required for `insert` command. - pub insert_line: Option, + /// Unified diff to apply. Supports editing multiple files simultaneously. + /// Example: "--- a/file\n+++ b/file\n@@ -1,3 +1,3 @@\n context\n-old\n+new\n context" + /// Preferred for multi-file edits. + pub diff: Option, +} + +/// Parameters for the write_file tool +#[derive(Debug, Serialize, Deserialize, JsonSchema)] +pub struct WriteFileParams { + /// Absolute path to file, e.g. `/repo/file.py`. + pub path: String, + + /// The content to write to the file. This is a full overwrite. + pub file_text: String, } /// Parameters for the shell tool @@ -227,7 +233,6 @@ fn load_prompt_files() -> HashMap { #[derive(Clone)] pub struct DeveloperServer { tool_router: ToolRouter, - file_history: Arc>>>, ignore_patterns: Gitignore, editor_model: Option, prompts: HashMap, @@ -302,64 +307,24 @@ impl ServerHandler for DeveloperServer { } }; - // Check if editor model exists and augment with custom llm editor tool description - let editor_description = if let Some(ref editor) = self.editor_model { + let editor_description = { + let editor_extra = if let Some(ref editor) = self.editor_model { + format!("\n{}", editor.get_str_replace_description()) + } else { + String::new() + }; formatdoc! {r#" - Additional Text Editor Tool Instructions: + Additional File Tool Instructions: - Perform text editing operations on files. - The `command` parameter specifies the operation to perform. Allowed options are: - - `view`: View the content of a file. - - `write`: Create or overwrite a file with the given content - - `str_replace`: Replace text in one or more files. - - `insert`: Insert text at a specific line location in the file. - - `undo_edit`: Undo the last edit made to a file. - - To use the write command, you must specify `file_text` which will become the new content of the file. Be careful with - existing files! This is a full overwrite, so you must include everything - not just sections you are modifying. - - To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning, -1 for end) - and `new_str` (the text to insert). - - To use the str_replace command to edit multiple files, use the `diff` parameter with a unified diff. - To use the str_replace command to edit one file, you must specify both `old_str` and `new_str` - the `old_str` needs to exactly match one - unique section of the original file, including any whitespace. Make sure to include enough context that the match is not - ambiguous. The entire original string will be replaced with `new_str` - - When possible, batch file edits together by using a multi-file unified `diff` within a single str_replace tool call. - - {} - - "#, editor.get_str_replace_description()} - } else { - formatdoc! {r#" - - Additional Text Editor Tool Instructions: - - Perform text editing operations on files. - - The `command` parameter specifies the operation to perform. Allowed options are: - - `view`: View the content of a file. - - `write`: Create or overwrite a file with the given content - - `str_replace`: Replace text in one or more files. - - `insert`: Insert text at a specific line location in the file. - - `undo_edit`: Undo the last edit made to a file. - - To use the write command, you must specify `file_text` which will become the new content of the file. Be careful with - existing files! This is a full overwrite, so you must include everything - not just sections you are modifying. - - To use the str_replace command to edit multiple files, use the `diff` parameter with a unified diff. - To use the str_replace command to edit one file, you must specify both `old_str` and `new_str` - the `old_str` needs to exactly match one - unique section of the original file, including any whitespace. Make sure to include enough context that the match is not - ambiguous. The entire original string will be replaced with `new_str` - - When possible, batch file edits together by using a multi-file unified `diff` within a single str_replace tool call. - - To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning, -1 for end) - and `new_str` (the text to insert). + Use `read_file` to view file contents or list directories. Use `view_range` for large files. + Use `edit_file` to make surgical edits. Provide `old_str` and `new_str` for single replacements - + `old_str` must match exactly one unique section of the file including whitespace. + For multi-file edits, use the `diff` parameter with a unified diff format. + Use `write_file` to create new files or fully overwrite existing ones. Include the complete file content. + {editor_extra} "#} }; @@ -604,7 +569,6 @@ impl DeveloperServer { Self { tool_router: Self::tool_router(), - file_history: Arc::new(Mutex::new(HashMap::new())), ignore_patterns, editor_model, prompts: load_prompt_files(), @@ -745,26 +709,18 @@ impl DeveloperServer { ])) } - /// Perform text editing operations on files. - /// - /// The `command` parameter specifies the operation to perform. Allowed options are: - /// - `view`: View the content of a file. - /// - `write`: Create or overwrite a file with the given content - /// - `str_replace`: Replace old_str with new_str in the file. - /// - `insert`: Insert text at a specific line location in the file. - /// - `undo_edit`: Undo the last edit made to a file. + /// Read the contents of a file or list a directory. #[tool( - name = "text_editor", - description = "Perform text editing operations on files. Commands: view (show file content), write (create/overwrite file), str_replace (edit file), insert (insert at line), undo_edit (undo last change)." + name = "read_file", + description = "Read the contents of a file or list a directory. Supports optional line range for viewing specific sections." )] - pub async fn text_editor( + pub async fn read_file( &self, - params: Parameters, + params: Parameters, ) -> Result { let params = params.0; let path = self.resolve_path(¶ms.path)?; - // Check if file is ignored before proceeding with any text editor operation if self.is_ignored(&path) { return Err(ErrorData::new( ErrorCode::INTERNAL_ERROR, @@ -776,101 +732,90 @@ impl DeveloperServer { )); } - match params.command.as_str() { - "view" => { - let view_range = params.view_range.as_ref().and_then(|vr| { - if vr.len() == 2 { - Some((vr[0] as usize, vr[1])) - } else { - None - } - }); - let content = text_editor_view(&path, view_range).await?; - Ok(CallToolResult::success(content)) + let view_range = params.view_range.as_ref().and_then(|vr| { + if vr.len() == 2 { + Some((vr[0] as usize, vr[1])) + } else { + None } - "write" => { - let file_text = params.file_text.ok_or_else(|| { - ErrorData::new( - ErrorCode::INVALID_PARAMS, - "Missing 'file_text' parameter for write command".to_string(), - None, - ) - })?; - let content = text_editor_write(&path, &file_text).await?; - Ok(CallToolResult::success(content)) - } - "str_replace" => { - // Check if diff parameter is provided - if let Some(ref diff) = params.diff { - // When diff is provided, old_str and new_str are not required - let content = text_editor_replace( - &path, - "", // old_str not used with diff - "", // new_str not used with diff - Some(diff), - &self.editor_model, - &self.file_history, - ) - .await?; - Ok(CallToolResult::success(content)) - } else { - // Traditional str_replace with old_str and new_str - let old_str = params.old_str.ok_or_else(|| { - ErrorData::new( - ErrorCode::INVALID_PARAMS, - "Missing 'old_str' parameter for str_replace command".to_string(), - None, - ) - })?; - let new_str = params.new_str.ok_or_else(|| { - ErrorData::new( - ErrorCode::INVALID_PARAMS, - "Missing 'new_str' parameter for str_replace command".to_string(), - None, - ) - })?; - let content = text_editor_replace( - &path, - &old_str, - &new_str, - None, - &self.editor_model, - &self.file_history, - ) - .await?; - Ok(CallToolResult::success(content)) - } - } - "insert" => { - let insert_line = params.insert_line.ok_or_else(|| { - ErrorData::new( - ErrorCode::INVALID_PARAMS, - "Missing 'insert_line' parameter for insert command".to_string(), - None, - ) - })? as usize; - let new_str = params.new_str.ok_or_else(|| { - ErrorData::new( - ErrorCode::INVALID_PARAMS, - "Missing 'new_str' parameter for insert command".to_string(), - None, - ) - })?; - let content = - text_editor_insert(&path, insert_line as i64, &new_str, &self.file_history) - .await?; - Ok(CallToolResult::success(content)) - } - "undo_edit" => { - let content = text_editor_undo(&path, &self.file_history).await?; - Ok(CallToolResult::success(content)) - } - _ => Err(ErrorData::new( - ErrorCode::INVALID_PARAMS, - format!("Unknown command '{}'", params.command), + }); + let content = text_editor_view(&path, view_range).await?; + Ok(CallToolResult::success(content)) + } + + /// Edit a file by replacing exact text or applying a unified diff. + #[tool( + name = "edit_file", + description = "Edit a file by replacing exact text (old_str/new_str) or applying a unified diff. For old_str/new_str, the old text must match exactly once in the file." + )] + pub async fn edit_file( + &self, + params: Parameters, + ) -> Result { + let params = params.0; + let path = self.resolve_path(¶ms.path)?; + + if self.is_ignored(&path) { + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Access to '{}' is restricted by .gooseignore", + path.display() + ), None, - )), + )); } + + if let Some(ref diff) = params.diff { + let content = + text_editor_replace(&path, "", "", Some(diff), &self.editor_model).await?; + Ok(CallToolResult::success(content)) + } else { + let old_str = params.old_str.ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Either 'diff' or both 'old_str' and 'new_str' must be provided".to_string(), + None, + ) + })?; + let new_str = params.new_str.ok_or_else(|| { + ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Missing 'new_str' parameter".to_string(), + None, + ) + })?; + let content = + text_editor_replace(&path, &old_str, &new_str, None, &self.editor_model).await?; + Ok(CallToolResult::success(content)) + } + } + + /// Write content to a file, creating it if needed or overwriting if it exists. + #[tool( + name = "write_file", + description = "Write content to a file. Creates the file if it doesn't exist, overwrites if it does. Automatically creates parent directories." + )] + pub async fn write_file( + &self, + params: Parameters, + ) -> Result { + let params = params.0; + let path = self.resolve_path(¶ms.path)?; + + if self.is_ignored(&path) { + return Err(ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!( + "Access to '{}' is restricted by .gooseignore", + path.display() + ), + None, + )); + } + + let content = text_editor_write(&path, ¶ms.file_text).await?; + Ok(CallToolResult::success(content)) } /// Execute a command in the shell. @@ -1668,18 +1613,12 @@ mod tests { let content = "x".repeat(3 * 1024 * 1024); // 3MB fs::write(&large_file_path, content).unwrap(); - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: large_file_path.to_str().unwrap().to_string(), - command: "view".to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let result = server.text_editor(view_params).await; + let result = server.read_file(view_params).await; assert!(result.is_err()); let err = result.err().unwrap(); @@ -1695,18 +1634,12 @@ mod tests { let content = "x".repeat(500_000); fs::write(&many_chars_path, content).unwrap(); - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: many_chars_path.to_str().unwrap().to_string(), - command: "view".to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let result = server.text_editor(view_params).await; + let result = server.read_file(view_params).await; assert!(result.is_err()); let err = result.err().unwrap(); @@ -1726,32 +1659,20 @@ mod tests { let server = create_test_server(); // Create a new file - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some("Hello, world!".to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: "Hello, world!".to_string(), }); - server.text_editor(write_params).await.unwrap(); + server.write_file(write_params).await.unwrap(); // View the file - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let view_result = server.text_editor(view_params).await.unwrap(); + let view_result = server.read_file(view_params).await.unwrap(); assert!(!view_result.content.is_empty()); let user_content = view_result @@ -1797,32 +1718,22 @@ mod tests { let server = create_test_server(); // Create a new file - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some("Hello, world!".to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: "Hello, world!".to_string(), }); - server.text_editor(write_params).await.unwrap(); + server.write_file(write_params).await.unwrap(); // Replace string - let replace_params = Parameters(TextEditorParams { + let replace_params = Parameters(EditFileParams { path: file_path_str.to_string(), - command: "str_replace".to_string(), - view_range: None, - file_text: None, old_str: Some("world".to_string()), new_str: Some("Rust".to_string()), - insert_line: None, diff: None, }); - let replace_result = server.text_editor(replace_params).await.unwrap(); + let replace_result = server.edit_file(replace_params).await.unwrap(); let assistant_content = replace_result .content @@ -1844,76 +1755,6 @@ mod tests { assert!(content.contains("Hello, Rust!")); } - #[tokio::test] - #[serial] - async fn test_text_editor_undo_edit() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Create a file - let write_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some("Original content".to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - server.text_editor(write_params).await.unwrap(); - - // Make an edit - let replace_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "str_replace".to_string(), - view_range: None, - file_text: None, - old_str: Some("Original".to_string()), - new_str: Some("Modified".to_string()), - insert_line: None, - diff: None, - }); - - server.text_editor(replace_params).await.unwrap(); - - // Verify the edit was made - let content = fs::read_to_string(&file_path).unwrap(); - assert!(content.contains("Modified content")); - - // Undo the edit - let undo_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "undo_edit".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - let undo_result = server.text_editor(undo_params).await.unwrap(); - - // Verify undo worked - let content = fs::read_to_string(&file_path).unwrap(); - assert!(content.contains("Original content")); - - let undo_content = undo_result - .content - .iter() - .find(|c| c.as_text().is_some()) - .unwrap() - .as_text() - .unwrap(); - assert!(undo_content.text.contains("Undid the last edit")); - } - #[tokio::test] #[serial] async fn test_goose_ignore_basic_patterns() { @@ -1967,18 +1808,12 @@ mod tests { // Try to write to an ignored file let secret_path = temp_dir.path().join("secret.txt"); - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: secret_path.to_str().unwrap().to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some("test content".to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: "test content".to_string(), }); - let result = server.text_editor(write_params).await; + let result = server.write_file(write_params).await; assert!( result.is_err(), "Should not be able to write to ignored file" @@ -1987,18 +1822,12 @@ mod tests { // Try to write to a non-ignored file let allowed_path = temp_dir.path().join("allowed.txt"); - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: allowed_path.to_str().unwrap().to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some("test content".to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: "test content".to_string(), }); - let result = server.text_editor(write_params).await; + let result = server.write_file(write_params).await; assert!( result.is_ok(), "Should be able to write to non-ignored file" @@ -2074,21 +1903,14 @@ mod tests { let temp_dir = tempfile::tempdir().unwrap(); std::env::set_current_dir(&temp_dir).unwrap(); - // Test without editor API configured (should be the case in tests due to cfg!(test)) let server = create_test_server(); - // Get server info which contains tool descriptions let server_info = server.get_info(); let instructions = server_info.instructions.unwrap_or_default(); - // Should use traditional description with str_replace command - assert!(instructions.contains("Replace text in one or more files")); - assert!(instructions.contains("str_replace")); - - // Should not contain editor API description or edit_file command - assert!(!instructions.contains("Edit the file with the new content")); - assert!(!instructions.contains("edit_file")); - assert!(!instructions.contains("work out how to place old_str with it intelligently")); + assert!(instructions.contains("read_file")); + assert!(instructions.contains("edit_file")); + assert!(instructions.contains("write_file")); } #[tokio::test] @@ -2104,32 +1926,20 @@ mod tests { // Create a multi-line file let content = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\nLine 6\nLine 7\nLine 8\nLine 9\nLine 10"; - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: content.to_string(), }); - server.text_editor(write_params).await.unwrap(); + server.write_file(write_params).await.unwrap(); // Test viewing specific range - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: Some(vec![3, 6]), - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let view_result = server.text_editor(view_params).await.unwrap(); + let view_result = server.read_file(view_params).await.unwrap(); let text = view_result .content @@ -2165,32 +1975,20 @@ mod tests { // Create a multi-line file let content = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5"; - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: content.to_string(), }); - server.text_editor(write_params).await.unwrap(); + server.write_file(write_params).await.unwrap(); // Test viewing from line 3 to end using -1 - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: Some(vec![3, -1]), - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let view_result = server.text_editor(view_params).await.unwrap(); + let view_result = server.read_file(view_params).await.unwrap(); let text = view_result .content @@ -2225,475 +2023,26 @@ mod tests { // Create a small file let content = "Line 1\nLine 2\nLine 3"; - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: content.to_string(), }); - server.text_editor(write_params).await.unwrap(); + server.write_file(write_params).await.unwrap(); // Test invalid range - start line beyond file - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: Some(vec![10, 15]), - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let result = server.text_editor(view_params).await; + let result = server.read_file(view_params).await; assert!(result.is_err()); let error = result.unwrap_err(); assert_eq!(error.code, ErrorCode::INVALID_PARAMS); assert!(error.message.contains("beyond the end of the file")); } - #[tokio::test] - #[serial] - async fn test_text_editor_insert_at_beginning() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Create a file with some content - let content = "Line 2\nLine 3\nLine 4"; - let write_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - server.text_editor(write_params).await.unwrap(); - - // Insert at the beginning (line 0) - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: Some("Line 1".to_string()), - insert_line: Some(0), - diff: None, - }); - - let insert_result = server.text_editor(insert_params).await.unwrap(); - - let text = insert_result - .content - .iter() - .find(|c| { - c.audience() - .is_some_and(|roles| roles.contains(&Role::Assistant)) - }) - .unwrap() - .as_text() - .unwrap(); - - assert!(text.text.contains("Successfully inserted") && text.text.contains("at line 1")); - - // Verify the file content by reading it directly - let file_content = fs::read_to_string(&file_path).unwrap(); - assert!(file_content.contains("Line 1\nLine 2\nLine 3\nLine 4")); - } - - #[tokio::test] - #[serial] - async fn test_text_editor_insert_in_middle() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Create a file with some content - let content = "Line 1\nLine 2\nLine 4\nLine 5"; - let write_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - server.text_editor(write_params).await.unwrap(); - - // Insert after line 2 - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: Some("Line 3".to_string()), - insert_line: Some(2), - diff: None, - }); - - let insert_result = server.text_editor(insert_params).await.unwrap(); - - let text = insert_result - .content - .iter() - .find(|c| { - c.audience() - .is_some_and(|roles| roles.contains(&Role::Assistant)) - }) - .unwrap() - .as_text() - .unwrap(); - - assert!(text.text.contains("Successfully inserted") && text.text.contains("at line 3")); - - // Verify the file content by reading it directly - let file_content = fs::read_to_string(&file_path).unwrap(); - let lines: Vec<&str> = file_content.lines().collect(); - assert_eq!(lines[0], "Line 1"); - assert_eq!(lines[1], "Line 2"); - assert_eq!(lines[2], "Line 3"); - assert_eq!(lines[3], "Line 4"); - assert_eq!(lines[4], "Line 5"); - } - - #[tokio::test] - #[serial] - async fn test_text_editor_insert_at_end() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Create a file with some content - let content = "Line 1\nLine 2\nLine 3"; - let write_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - server.text_editor(write_params).await.unwrap(); - - // Insert at the end (after line 3) - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: Some("Line 4".to_string()), - insert_line: Some(3), - diff: None, - }); - - let insert_result = server.text_editor(insert_params).await.unwrap(); - - let text = insert_result - .content - .iter() - .find(|c| { - c.audience() - .is_some_and(|roles| roles.contains(&Role::Assistant)) - }) - .unwrap() - .as_text() - .unwrap(); - - assert!(text.text.contains("Successfully inserted") && text.text.contains("at line 4")); - - // Verify the file content by reading it directly - let file_content = fs::read_to_string(&file_path).unwrap(); - assert!(file_content.contains("Line 1\nLine 2\nLine 3\nLine 4")); - } - - #[tokio::test] - #[serial] - async fn test_text_editor_insert_at_end_negative() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Create a file with some content - let content = "Line 1\nLine 2\nLine 3"; - let write_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - server.text_editor(write_params).await.unwrap(); - - // Insert at the end using -1 - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: Some("Line 4".to_string()), - insert_line: Some(-1), - diff: None, - }); - - let insert_result = server.text_editor(insert_params).await.unwrap(); - - let text = insert_result - .content - .iter() - .find(|c| { - c.audience() - .is_some_and(|roles| roles.contains(&Role::Assistant)) - }) - .unwrap() - .as_text() - .unwrap(); - - assert!(text.text.contains("Successfully inserted") && text.text.contains("at line 4")); - - // Verify the file content by reading it directly - let file_content = fs::read_to_string(&file_path).unwrap(); - assert!(file_content.contains("Line 1\nLine 2\nLine 3\nLine 4")); - } - - #[tokio::test] - #[serial] - async fn test_text_editor_insert_invalid_line() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Create a file with some content - let content = "Line 1\nLine 2\nLine 3"; - let write_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - server.text_editor(write_params).await.unwrap(); - - // Try to insert beyond the end of the file - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: Some("Line 11".to_string()), - insert_line: Some(10), - diff: None, - }); - - let result = server.text_editor(insert_params).await; - - assert!(result.is_err()); - let err = result.err().unwrap(); - assert_eq!(err.code, ErrorCode::INVALID_PARAMS); - assert!(err.message.contains("beyond the end of the file")); - } - - #[tokio::test] - #[serial] - async fn test_text_editor_insert_missing_parameters() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Create a file first - let write_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some("Initial content".to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - server.text_editor(write_params).await.unwrap(); - - // Test insert without new_str parameter - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: None, // Missing required parameter - insert_line: Some(1), - diff: None, - }); - - let result = server.text_editor(insert_params).await; - assert!(result.is_err()); - let error = result.unwrap_err(); - assert_eq!(error.code, ErrorCode::INVALID_PARAMS); - assert!(error.message.contains("Missing 'new_str' parameter")); - - // Test insert without insert_line parameter - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: Some("New text".to_string()), - insert_line: None, // Missing required parameter - diff: None, - }); - - let result = server.text_editor(insert_params).await; - assert!(result.is_err()); - let error = result.unwrap_err(); - assert_eq!(error.code, ErrorCode::INVALID_PARAMS); - assert!(error.message.contains("Missing 'insert_line' parameter")); - } - - #[tokio::test] - #[serial] - async fn test_text_editor_insert_with_undo() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Create a file with some content - let content = "Line 1\nLine 2"; - let write_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content.to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - server.text_editor(write_params).await.unwrap(); - - // Insert a line - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: Some("Inserted Line".to_string()), - insert_line: Some(1), - diff: None, - }); - - server.text_editor(insert_params).await.unwrap(); - - // Undo the insert - let undo_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "undo_edit".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, - }); - - let undo_result = server.text_editor(undo_params).await.unwrap(); - - let text = undo_result - .content - .iter() - .find(|c| c.as_text().is_some()) - .unwrap() - .as_text() - .unwrap(); - assert!(text.text.contains("Undid the last edit")); - - // Verify the file is back to original content - let file_content = fs::read_to_string(&file_path).unwrap(); - assert!(file_content.contains("Line 1\nLine 2")); - assert!(!file_content.contains("Inserted Line")); - } - - #[tokio::test] - #[serial] - async fn test_text_editor_insert_nonexistent_file() { - let temp_dir = tempfile::tempdir().unwrap(); - let file_path = temp_dir.path().join("nonexistent.txt"); - let file_path_str = file_path.to_str().unwrap(); - std::env::set_current_dir(&temp_dir).unwrap(); - - let server = create_test_server(); - - // Try to insert into a nonexistent file - let insert_params = Parameters(TextEditorParams { - path: file_path_str.to_string(), - command: "insert".to_string(), - view_range: None, - file_text: None, - old_str: None, - new_str: Some("New line".to_string()), - insert_line: Some(0), - diff: None, - }); - - let result = server.text_editor(insert_params).await; - - assert!(result.is_err()); - let err = result.err().unwrap(); - assert_eq!(err.code, ErrorCode::INVALID_PARAMS); - assert!(err.message.contains("does not exist")); - } - #[tokio::test] #[serial] async fn test_text_editor_view_large_file_without_range() { @@ -2710,32 +2059,20 @@ mod tests { content.push_str(&format!("Line {}\n", i)); } - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: content, }); - server.text_editor(write_params).await.unwrap(); + server.write_file(write_params).await.unwrap(); // Test viewing without view_range - should trigger the error - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let result = server.text_editor(view_params).await; + let result = server.read_file(view_params).await; assert!(result.is_err()); let err = result.err().unwrap(); @@ -2749,18 +2086,12 @@ mod tests { .contains("please pass in view_range with [1, 2001]")); // Test viewing with view_range - should work - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: Some(vec![1, 100]), - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let result = server.text_editor(view_params).await; + let result = server.read_file(view_params).await; assert!(result.is_ok()); let view_result = result.unwrap(); @@ -2781,18 +2112,12 @@ mod tests { assert!(!text.text.contains("101: Line 101")); // Test viewing with explicit full range - should work - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: Some(vec![1, 2001]), - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let result = server.text_editor(view_params).await; + let result = server.read_file(view_params).await; assert!(result.is_ok()); } @@ -2812,32 +2137,20 @@ mod tests { content.push_str(&format!("Line {}\n", i)); } - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: content, }); - server.text_editor(write_params).await.unwrap(); + server.write_file(write_params).await.unwrap(); // Test viewing without view_range - should work since it's exactly 2000 lines - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let result = server.text_editor(view_params).await; + let result = server.read_file(view_params).await; assert!(result.is_ok()); let view_result = result.unwrap(); @@ -2873,32 +2186,20 @@ mod tests { content.push_str(&format!("Line {}\n", i)); } - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: file_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some(content), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: content, }); - server.text_editor(write_params).await.unwrap(); + server.write_file(write_params).await.unwrap(); // Test viewing without view_range - should work fine - let view_params = Parameters(TextEditorParams { + let view_params = Parameters(ReadFileParams { path: file_path_str.to_string(), - command: "view".to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, }); - let result = server.text_editor(view_params).await; + let result = server.read_file(view_params).await; assert!(result.is_ok()); let view_result = result.unwrap(); @@ -2940,15 +2241,9 @@ mod tests { // Test viewing a directory let result = server - .text_editor(Parameters(TextEditorParams { - command: "view".to_string(), + .read_file(Parameters(ReadFileParams { path: temp_path.to_str().unwrap().to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, })) .await; @@ -3003,15 +2298,9 @@ mod tests { let server = create_test_server(); let result = server - .text_editor(Parameters(TextEditorParams { - command: "view".to_string(), + .read_file(Parameters(ReadFileParams { path: temp_path.to_str().unwrap().to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, })) .await; @@ -3050,15 +2339,9 @@ mod tests { } let result = server - .text_editor(Parameters(TextEditorParams { - command: "view".to_string(), + .read_file(Parameters(ReadFileParams { path: temp_path.to_str().unwrap().to_string(), view_range: None, - file_text: None, - old_str: None, - new_str: None, - insert_line: None, - diff: None, })) .await; @@ -3390,18 +2673,12 @@ mod tests { let absolute_path = temp_dir.path().join("absolute_test.txt"); let absolute_path_str = absolute_path.to_str().unwrap(); - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: absolute_path_str.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some("Absolute path test".to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: "Absolute path test".to_string(), }); - let result = server.text_editor(write_params).await; + let result = server.write_file(write_params).await; assert!(result.is_ok()); let content = fs::read_to_string(&absolute_path).unwrap(); @@ -3417,18 +2694,12 @@ mod tests { let server = create_test_server(); let relative_path = "relative_test.txt"; - let write_params = Parameters(TextEditorParams { + let write_params = Parameters(WriteFileParams { path: relative_path.to_string(), - command: "write".to_string(), - view_range: None, - file_text: Some("Relative path test".to_string()), - old_str: None, - new_str: None, - insert_line: None, - diff: None, + file_text: "Relative path test".to_string(), }); - let result = server.text_editor(write_params).await; + let result = server.write_file(write_params).await; assert!(result.is_ok()); let absolute_path = temp_dir.path().join(relative_path); diff --git a/crates/goose-mcp/src/developer/shell.rs b/crates/goose-mcp/src/developer/shell.rs index 9196a48b40..3a47845984 100644 --- a/crates/goose-mcp/src/developer/shell.rs +++ b/crates/goose-mcp/src/developer/shell.rs @@ -92,16 +92,6 @@ pub fn is_absolute_path(path_str: &str) -> bool { } } -pub fn normalize_line_endings(text: &str) -> String { - if cfg!(windows) { - // Ensure CRLF line endings on Windows - text.replace("\r\n", "\n").replace("\n", "\r\n") - } else { - // Ensure LF line endings on Unix - text.replace("\r\n", "\n") - } -} - /// Configure a shell command with process group support for proper child process tracking. /// /// On Unix systems, creates a new process group so child processes can be killed together. diff --git a/crates/goose-mcp/src/developer/tests/test_diff.rs b/crates/goose-mcp/src/developer/tests/test_diff.rs index fa0b3136a4..4cb1b7e502 100644 --- a/crates/goose-mcp/src/developer/tests/test_diff.rs +++ b/crates/goose-mcp/src/developer/tests/test_diff.rs @@ -2,9 +2,7 @@ mod tests { use crate::developer::text_editor::*; use mpatch::parse_diffs; - use std::collections::HashMap; - use std::sync::{Arc, Mutex}; use tempfile::TempDir; #[test] @@ -86,8 +84,7 @@ new file mode 100644 +modified_line2 line3"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; assert!(result.is_ok()); let content = std::fs::read_to_string(&file_path).unwrap(); @@ -96,9 +93,6 @@ new file mode 100644 content == "line1\nmodified_line2\nline3" || content == "line1\nmodified_line2\nline3\n" ); - - // Verify history was saved - assert!(history.lock().unwrap().contains_key(&file_path)); } #[tokio::test] @@ -119,8 +113,7 @@ new file mode 100644 +if __name__ == "__main__": + main()"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; if let Err(e) = &result { eprintln!("Error in test_add_lines_at_end: {:?}", e); @@ -149,8 +142,7 @@ new file mode 100644 -remove2 keep2"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; assert!(result.is_ok()); let content = std::fs::read_to_string(&file_path).unwrap(); @@ -173,8 +165,7 @@ new file mode 100644 -old +new"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; // mpatch with fuzzy matching may return OK but with a warning message // The test now verifies that if it succeeds, it's a partial application @@ -205,10 +196,9 @@ new file mode 100644 -old +new"#; - let history = Arc::new(Mutex::new(HashMap::new())); // For nonexistent files, apply_diff will try to apply the patch // which should fail since the file doesn't exist - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; // The behavior might be different with patcher - it might create the file // or it might fail. Let's check what happens. @@ -237,14 +227,12 @@ new file mode 100644 println!("Hello"); }"#; - let history = Arc::new(Mutex::new(HashMap::new())); let result = text_editor_replace( &file_path, "", // old_str (ignored when diff is provided) "", // new_str (ignored when diff is provided) Some(diff), &None, // editor_model - &history, ) .await; @@ -267,8 +255,7 @@ new file mode 100644 @@ -0,0 +1 @@ +new content"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; assert!(result.is_ok()); let content = std::fs::read_to_string(&file_path).unwrap(); @@ -276,40 +263,6 @@ new file mode 100644 assert!(content == "new content" || content == "new content\n"); } - #[tokio::test] - async fn test_undo_after_diff() { - let temp_dir = TempDir::new().unwrap(); - let file_path = temp_dir.path().join("test.txt"); - - std::fs::write(&file_path, "original\n").unwrap(); - - let diff = r#"--- a/test.txt -+++ b/test.txt -@@ -1 +1 @@ --original -+modified"#; - - let history = Arc::new(Mutex::new(HashMap::new())); - - // Apply diff - let result = apply_diff(&file_path, diff, &history).await; - if let Err(e) = &result { - eprintln!("Error applying diff in test_undo_after_diff: {:?}", e); - } - assert!(result.is_ok()); - // patcher doesn't preserve trailing newlines in the same way - let content_after = std::fs::read_to_string(&file_path).unwrap(); - assert!(content_after == "modified" || content_after == "modified\n"); - - // Undo should restore original - let undo_result = text_editor_undo(&file_path, &history).await; - if let Err(e) = &undo_result { - eprintln!("Error undoing in test_undo_after_diff: {:?}", e); - } - assert!(undo_result.is_ok()); - assert_eq!(std::fs::read_to_string(&file_path).unwrap(), "original\n"); - } - #[tokio::test] async fn test_multi_file_diff() { let temp_dir = TempDir::new().unwrap(); @@ -332,8 +285,7 @@ diff --git a/file2.txt b/file2.txt -content2 +modified2"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(base_path, diff, &history).await; + let result = apply_diff(base_path, diff).await; assert!(result.is_ok()); let content1 = std::fs::read_to_string(base_path.join("file1.txt")).unwrap(); @@ -361,8 +313,7 @@ diff --git a/file2.txt b/file2.txt +modified_line3 line4"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; // mpatch should handle this with fuzzy matching assert!(result.is_ok()); @@ -393,8 +344,7 @@ diff --git a/file2.txt b/file2.txt + print('goodbye') return True"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; // Should work with fuzzy matching at 70% threshold assert!(result.is_ok()); @@ -457,8 +407,7 @@ diff --git a/file2.txt b/file2.txt +line2_modified line3"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; assert!(result.is_ok()); let content = std::fs::read_to_string(&file_path).unwrap(); @@ -484,8 +433,7 @@ diff --git a/file2.txt b/file2.txt +line2_modified line3"#; - let history = Arc::new(Mutex::new(HashMap::new())); - let result = apply_diff(&file_path, diff, &history).await; + let result = apply_diff(&file_path, diff).await; assert!(result.is_ok()); let content = std::fs::read_to_string(&file_path).unwrap(); diff --git a/crates/goose-mcp/src/developer/text_editor.rs b/crates/goose-mcp/src/developer/text_editor.rs index 6cd45a37d7..82605f7a55 100644 --- a/crates/goose-mcp/src/developer/text_editor.rs +++ b/crates/goose-mcp/src/developer/text_editor.rs @@ -3,15 +3,245 @@ use indoc::formatdoc; use mpatch::{apply_patch, parse_diffs, PatchError}; use rmcp::model::{Content, ErrorCode, ErrorData, Role}; use std::{ - collections::HashMap, fs::File, io::Read, path::{Path, PathBuf}, }; +use similar::ChangeTag; + use super::editor_models::EditorModel; use super::lang; -use super::shell::normalize_line_endings; + +// --- Line ending detection and preservation --- + +/// Detect the dominant line ending style in a file's content. +fn detect_line_ending(content: &str) -> &'static str { + let crlf_idx = content.find("\r\n"); + let lf_idx = content.find('\n'); + match (crlf_idx, lf_idx) { + (Some(c), Some(l)) if c < l => "\r\n", + _ => "\n", + } +} + +/// Normalize all line endings to LF for consistent internal processing. +fn normalize_to_lf(text: &str) -> String { + text.replace("\r\n", "\n").replace('\r', "\n") +} + +/// Restore line endings to the original style detected from the file. +fn restore_line_endings(text: &str, ending: &str) -> String { + if ending == "\r\n" { + text.replace('\n', "\r\n") + } else { + text.to_string() + } +} + +// --- BOM handling --- + +/// Strip UTF-8 BOM if present. Returns the BOM prefix (empty string if none) and the text without it. +fn strip_bom(content: &str) -> (&str, &str) { + if let Some(stripped) = content.strip_prefix('\u{FEFF}') { + ("\u{FEFF}", stripped) + } else { + ("", content) + } +} + +// --- Fuzzy text matching --- + +/// Normalize text for fuzzy matching: +/// - Strip trailing whitespace from each line +/// - Normalize smart quotes to ASCII +/// - Normalize Unicode dashes/hyphens to ASCII hyphen +/// - Normalize special Unicode spaces to regular space +fn normalize_for_fuzzy_match(text: &str) -> String { + let mut result = String::with_capacity(text.len()); + for (i, line) in text.split('\n').enumerate() { + if i > 0 { + result.push('\n'); + } + result.push_str(line.trim_end()); + } + // Smart single quotes → ' + let result = result.replace(['\u{2018}', '\u{2019}', '\u{201A}', '\u{201B}'], "'"); + // Smart double quotes → " + let result = result.replace(['\u{201C}', '\u{201D}', '\u{201E}', '\u{201F}'], "\""); + // Various dashes → - + let result = result.replace( + [ + '\u{2010}', '\u{2011}', '\u{2012}', '\u{2013}', '\u{2014}', '\u{2015}', '\u{2212}', + ], + "-", + ); + // Special spaces → regular space + result.replace( + [ + '\u{00A0}', '\u{2002}', '\u{2003}', '\u{2004}', '\u{2005}', '\u{2006}', '\u{2007}', + '\u{2008}', '\u{2009}', '\u{200A}', '\u{202F}', '\u{205F}', '\u{3000}', + ], + " ", + ) +} + +struct FuzzyMatchResult { + found: bool, + /// The text that was actually matched (to use with `replacen`). + matched_text: String, + /// The content string to perform the replacement on. + /// When exact match: the original content. When fuzzy: the normalized content. + content_for_replacement: String, +} + +/// Find `old_text` in `content`, trying exact match first, then fuzzy. +fn fuzzy_find_text(content: &str, old_text: &str) -> FuzzyMatchResult { + // Try exact match first + if content.contains(old_text) { + return FuzzyMatchResult { + found: true, + matched_text: old_text.to_string(), + content_for_replacement: content.to_string(), + }; + } + + // Try fuzzy match + let fuzzy_content = normalize_for_fuzzy_match(content); + let fuzzy_old = normalize_for_fuzzy_match(old_text); + if fuzzy_content.contains(&fuzzy_old) { + return FuzzyMatchResult { + found: true, + matched_text: fuzzy_old, + content_for_replacement: fuzzy_content, + }; + } + + FuzzyMatchResult { + found: false, + matched_text: String::new(), + content_for_replacement: content.to_string(), + } +} + +// --- Unified diff generation for user-facing output --- + +struct DiffOutput { + diff: String, +} + +/// Generate a compact unified diff with line numbers and context. +/// Shows added/removed lines with `+`/`-` prefixes and surrounding context. +fn generate_diff_string(old_content: &str, new_content: &str, context_lines: usize) -> DiffOutput { + let diff = similar::TextDiff::from_lines(old_content, new_content); + let changes: Vec<_> = diff.iter_all_changes().collect(); + + let max_line = old_content.lines().count().max(new_content.lines().count()); + let width = max_line.to_string().len().max(1); + + let mut output = Vec::new(); + let mut old_line: usize = 1; + let mut new_line: usize = 1; + // Group changes into regions: each region is a run of changes plus context + // We need to decide for each line whether to show it. + // Strategy: collect line info, then select which to display. + struct LineInfo { + tag: ChangeTag, + text: String, + old_num: usize, + new_num: usize, + } + + let mut all_lines = Vec::new(); + for change in &changes { + let text = change.value().trim_end_matches('\n').to_string(); + all_lines.push(LineInfo { + tag: change.tag(), + text, + old_num: old_line, + new_num: new_line, + }); + match change.tag() { + ChangeTag::Equal => { + old_line += 1; + new_line += 1; + } + ChangeTag::Delete => { + old_line += 1; + } + ChangeTag::Insert => { + new_line += 1; + } + } + } + + // Find indices of changed lines + let change_indices: Vec = all_lines + .iter() + .enumerate() + .filter(|(_, l)| l.tag != ChangeTag::Equal) + .map(|(i, _)| i) + .collect(); + + if change_indices.is_empty() { + return DiffOutput { + diff: String::new(), + }; + } + + // Build set of lines to display (changes + context) + let mut visible = vec![false; all_lines.len()]; + for &ci in &change_indices { + let start = ci.saturating_sub(context_lines); + let end = (ci + context_lines + 1).min(all_lines.len()); + for v in &mut visible[start..end] { + *v = true; + } + } + + let mut last_visible = false; + for (i, line) in all_lines.iter().enumerate() { + if !visible[i] { + if last_visible { + output.push(format!(" {:>width$} ...", "", width = width)); + } + last_visible = false; + continue; + } + last_visible = true; + + match line.tag { + ChangeTag::Equal => { + output.push(format!( + " {:>width$} {}", + line.old_num, + line.text, + width = width + )); + } + ChangeTag::Delete => { + output.push(format!( + "-{:>width$} {}", + line.old_num, + line.text, + width = width + )); + } + ChangeTag::Insert => { + output.push(format!( + "+{:>width$} {}", + line.new_num, + line.text, + width = width + )); + } + } + } + + DiffOutput { + diff: output.join("\n"), + } +} // Constants pub const LINE_READ_LIMIT: usize = 2000; @@ -169,14 +399,7 @@ fn generate_summary(results: &DiffResults, is_single_file: bool, base_path: &Pat ) }; - let user_message = if is_single_file { - format!("{}\n\nUse 'undo_edit' to revert if needed.\n\n", summary) - } else { - format!( - "{}\n\nUse 'undo_edit' on individual files to revert if needed.\n\n", - summary - ) - }; + let user_message = format!("{}\n\n", summary); vec![ Content::text(summary.clone()).with_audience(vec![Role::Assistant]), @@ -207,7 +430,6 @@ fn adjust_base_dir_for_overlap(base_dir: &Path, file_path: &Path) -> PathBuf { fn apply_single_patch( patch: &mpatch::Patch, base_dir: &Path, - file_history: &std::sync::Arc>>>, results: &mut DiffResults, failed_hunks: &mut Vec, ) -> Result<(), ErrorData> { @@ -218,11 +440,7 @@ fn apply_single_patch( // Validate path safety validate_path_safety(&adjusted_base_dir, &file_path)?; - // Save history before modifying let file_existed = file_path.exists(); - if file_existed { - save_file_history(&file_path, file_history)?; - } // Apply patch with fuzzy matching (70% similarity threshold) let success = apply_patch(patch, &adjusted_base_dir, false, 0.7).map_err(|e| match e { @@ -359,7 +577,7 @@ fn report_partial_failures(failed_hunks: &[String]) { • The file has changed significantly from when the diff was created\n\ • Line numbers in the diff are incorrect\n\ • The context lines don't match exactly\n\n\ - Review the changes and use 'undo_edit' if needed.", + Review the changes carefully.", failed_hunks.join("\n") ); @@ -368,11 +586,7 @@ fn report_partial_failures(failed_hunks: &[String]) { } /// Applies any diff (single or multi-file) using mpatch for fuzzy matching -pub async fn apply_diff( - base_path: &Path, - diff_content: &str, - file_history: &std::sync::Arc>>>, -) -> Result, ErrorData> { +pub async fn apply_diff(base_path: &Path, diff_content: &str) -> Result, ErrorData> { validate_diff_size(diff_content)?; let patches = parse_diff_content(diff_content)?; @@ -398,13 +612,7 @@ pub async fn apply_diff( let mut failed_hunks = Vec::new(); for patch in &patches { - apply_single_patch( - patch, - &base_dir, - file_history, - &mut results, - &mut failed_hunks, - )?; + apply_single_patch(patch, &base_dir, &mut results, &mut failed_hunks)?; } ensure_trailing_newlines(&patches, &base_dir)?; @@ -688,23 +896,30 @@ pub async fn text_editor_view( } pub async fn text_editor_write(path: &PathBuf, file_text: &str) -> Result, ErrorData> { - // Normalize line endings based on platform - let mut normalized_text = normalize_line_endings(file_text); // Make mutable + // Detect existing file's line ending style, or use platform default + let original_ending = if path.exists() { + std::fs::read_to_string(path) + .map(|c| detect_line_ending(&c).to_string()) + .unwrap_or_else(|_| "\n".to_string()) + } else if cfg!(windows) { + "\r\n".to_string() + } else { + "\n".to_string() + }; - // Ensure the text ends with a newline + let mut normalized_text = normalize_to_lf(file_text); if !normalized_text.ends_with('\n') { normalized_text.push('\n'); } + let final_text = restore_line_endings(&normalized_text, &original_ending); - // Write to the file - std::fs::write(path, &normalized_text) // Write the potentially modified text - .map_err(|e| { - ErrorData::new( - ErrorCode::INTERNAL_ERROR, - format!("Failed to write file: {}", e), - None, - ) - })?; + std::fs::write(path, &final_text).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to write file: {}", e), + None, + ) + })?; // Try to detect the language from the file extension let language = lang::get_language_identifier(path); @@ -723,7 +938,7 @@ pub async fn text_editor_write(path: &PathBuf, file_text: &str) -> Result, editor_model: &Option, - file_history: &std::sync::Arc< - std::sync::Mutex>>, - >, ) -> Result, ErrorData> { // Check if diff is provided if let Some(diff_content) = diff { @@ -752,22 +964,21 @@ pub async fn text_editor_replace( )); } - return apply_diff(path, diff_content, file_history).await; + return apply_diff(path, diff_content).await; } - // Check if file exists and is active if !path.exists() { return Err(ErrorData::new( ErrorCode::INVALID_PARAMS, format!( - "File '{}' does not exist, you can write a new file with the `write` command", + "File '{}' does not exist, you can write a new file with the `write_file` command", path.display() ), None, )); } - // Read content - let content = std::fs::read_to_string(path).map_err(|e| { + // Read raw content and preserve original encoding details + let raw_content = std::fs::read_to_string(path).map_err(|e| { ErrorData::new( ErrorCode::INTERNAL_ERROR, format!("Failed to read file: {}", e), @@ -775,21 +986,24 @@ pub async fn text_editor_replace( ) })?; + let (bom, content_without_bom) = strip_bom(&raw_content); + let original_ending = detect_line_ending(content_without_bom); + let content = normalize_to_lf(content_without_bom); + let normalized_old = normalize_to_lf(old_str); + let normalized_new = normalize_to_lf(new_str); + // Check if Editor API is configured and use it as the primary path if let Some(ref editor) = editor_model { - // Editor API path - save history then call API directly - save_file_history(path, file_history)?; - match editor.edit_code(&content, old_str, new_str).await { Ok(updated_content) => { - // Write the updated content directly - let mut normalized_content = normalize_line_endings(&updated_content); - - if !normalized_content.ends_with('\n') { - normalized_content.push('\n'); + let mut result = normalize_to_lf(&updated_content); + if !result.ends_with('\n') { + result.push('\n'); } + let final_content = + format!("{}{}", bom, restore_line_endings(&result, original_ending)); - std::fs::write(path, &normalized_content).map_err(|e| { + std::fs::write(path, &final_content).map_err(|e| { ErrorData::new( ErrorCode::INTERNAL_ERROR, format!("Failed to write file: {}", e), @@ -797,7 +1011,6 @@ pub async fn text_editor_replace( ) })?; - // Simple success message for Editor API return Ok(vec![ Content::text(format!("Successfully replaced text in {}.", path.display())) .with_audience(vec![Role::Assistant]), @@ -811,169 +1024,57 @@ pub async fn text_editor_replace( "Editor API call failed: {}, falling back to string replacement", e ); - // Fall through to traditional path below } } } - // Traditional string replacement path (original logic) - // Ensure 'old_str' appears exactly once - if content.matches(old_str).count() > 1 { + // Fuzzy find: tries exact match first, then normalizes whitespace/unicode + let match_result = fuzzy_find_text(&content, &normalized_old); + + if !match_result.found { return Err(ErrorData::new( ErrorCode::INVALID_PARAMS, - "'old_str' must appear exactly once in the file, but it appears multiple times" - .to_string(), + "'old_str' was not found in the file. Make sure it matches existing file content, including whitespace.".to_string(), None, )); } - if content.matches(old_str).count() == 0 { - return Err(ErrorData::new(ErrorCode::INVALID_PARAMS, "'old_str' must appear exactly once in the file, but it does not appear in the file. Make sure the string exactly matches existing file content, including whitespace!".to_string(), None)); - } - // Save history for undo (original behavior - after validation) - save_file_history(path, file_history)?; - - let new_content = content.replace(old_str, new_str); - let mut normalized_content = normalize_line_endings(&new_content); - - if !normalized_content.ends_with('\n') { - normalized_content.push('\n'); - } - - std::fs::write(path, &normalized_content).map_err(|e| { - ErrorData::new( - ErrorCode::INTERNAL_ERROR, - format!("Failed to write file: {}", e), - None, - ) - })?; - - let new_line_count = new_str.lines().count(); - - // Count newlines before the replacement to find the line number - let replacement_line = content - .split(old_str) - .next() - .expect("should split on already matched content") - .matches('\n') - .count() - + 1; // 1-indexed - - let summary = format!("Successfully replaced text in {}.", path.display()); - - // Try to detect the language from the file extension - let language = lang::get_language_identifier(path); - - // Show a snippet of the changed content with context for the user only - const SNIPPET_LINES: usize = 4; - let start_line = replacement_line.saturating_sub(SNIPPET_LINES + 1); - let end_line = replacement_line + SNIPPET_LINES + new_line_count; - let lines: Vec<&str> = new_content.lines().collect(); - let snippet = lines - .iter() - .skip(start_line) - .take(end_line - start_line + 1) - .cloned() - .collect::>() - .join("\n"); - - let user_output = formatdoc! {r#" - Successfully replaced text in {path} at line {line}. - ```{language} - {snippet} - ``` - "#, - path=path.display(), - line=replacement_line, - language=language, - snippet=snippet - }; - - Ok(vec![ - Content::text(summary).with_audience(vec![Role::Assistant]), - Content::text(user_output) - .with_audience(vec![Role::User]) - .with_priority(0.2), - ]) -} - -pub async fn text_editor_insert( - path: &PathBuf, - insert_line_spec: i64, - new_str: &str, - file_history: &std::sync::Arc< - std::sync::Mutex>>, - >, -) -> Result, ErrorData> { - // Check if file exists - if !path.exists() { + // Check uniqueness using fuzzy-normalized content for consistency + let fuzzy_content = normalize_for_fuzzy_match(&content); + let fuzzy_old = normalize_for_fuzzy_match(&normalized_old); + let occurrences = fuzzy_content.matches(&fuzzy_old).count(); + if occurrences > 1 { return Err(ErrorData::new( ErrorCode::INVALID_PARAMS, format!( - "File '{}' does not exist, you can write a new file with the `write` command", - path.display() + "'old_str' matches {} locations in the file. Provide more context to make it unique.", + occurrences ), None, )); } - // Read content - let content = std::fs::read_to_string(path).map_err(|e| { - ErrorData::new( - ErrorCode::INTERNAL_ERROR, - format!("Failed to read file: {}", e), + // Perform replacement (exactly once) + let base = &match_result.content_for_replacement; + let new_content = base.replacen(&match_result.matched_text, &normalized_new, 1); + + if *base == new_content { + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + "No changes made — the replacement produced identical content.".to_string(), None, - ) - })?; - - // Save history for undo - save_file_history(path, file_history)?; - - let lines: Vec<&str> = content.lines().collect(); - let total_lines = lines.len(); - - // Allow insert_line to be negative - let insert_line = if insert_line_spec < 0 { - // -1 == end of file, -2 == before the last line, etc. - (total_lines as i64 + 1 + insert_line_spec) as usize - } else { - insert_line_spec as usize - }; - - // Validate insert_line parameter - if insert_line > total_lines { - return Err(ErrorData::new(ErrorCode::INVALID_PARAMS, format!( - "Insert line {} is beyond the end of the file (total lines: {}). Use 0 to insert at the beginning or {} to insert at the end.", - insert_line, total_lines, total_lines - ), None)); + )); } - // Create new content with inserted text - let mut new_lines = Vec::new(); - - // Add lines before the insertion point - for (i, line) in lines.iter().enumerate() { - if i == insert_line { - // Insert the new text at this position - new_lines.push(new_str.to_string()); - } - new_lines.push(line.to_string()); + let mut final_lf = new_content; + if !final_lf.ends_with('\n') { + final_lf.push('\n'); } - - // If inserting at the end (after all existing lines) - if insert_line == total_lines { - new_lines.push(new_str.to_string()); - } - - let new_content = new_lines.join("\n"); - let normalized_content = normalize_line_endings(&new_content); - - // Ensure the file ends with a newline - let final_content = if !normalized_content.ends_with('\n') { - format!("{}\n", normalized_content) - } else { - normalized_content - }; + let final_content = format!( + "{}{}", + bom, + restore_line_endings(&final_lf, original_ending) + ); std::fs::write(path, &final_content).map_err(|e| { ErrorData::new( @@ -983,39 +1084,17 @@ pub async fn text_editor_insert( ) })?; - let insertion_line = insert_line + 1; // Convert to 1-indexed for display - let inserted_line_count = new_str.lines().count(); - - let summary = format!( - "Successfully inserted {} lines at line {} in {}", - inserted_line_count, - insertion_line, - path.display() - ); - - // Try to detect the language from the file extension - let language = lang::get_language_identifier(path); - - // Show a snippet of the inserted content with context for the user only - const SNIPPET_LINES: usize = 4; - let start_line = insertion_line.saturating_sub(SNIPPET_LINES); - let end_line = std::cmp::min(insertion_line + SNIPPET_LINES, new_lines.len()); - let snippet_lines: Vec = new_lines[start_line.saturating_sub(1)..end_line] - .iter() - .enumerate() - .map(|(i, line)| format!("{}: {}", start_line + i, line)) - .collect(); - let snippet = snippet_lines.join("\n"); + let diff_output = generate_diff_string(base, &final_lf, 4); + let summary = format!("Successfully replaced text in {}.", path.display()); let user_output = formatdoc! {r#" {summary} - ```{language} - {snippet} + ```diff + {diff} ``` "#, summary=summary, - language=language, - snippet=snippet + diff=diff_output.diff, }; Ok(vec![ @@ -1026,58 +1105,224 @@ pub async fn text_editor_insert( ]) } -pub async fn text_editor_undo( - path: &PathBuf, - file_history: &std::sync::Arc< - std::sync::Mutex>>, - >, -) -> Result, ErrorData> { - let mut history = file_history.lock().unwrap(); - if let Some(contents) = history.get_mut(path) { - if let Some(previous_content) = contents.pop() { - // Write previous content back to file - std::fs::write(path, previous_content).map_err(|e| { - ErrorData::new( - ErrorCode::INTERNAL_ERROR, - format!("Failed to write file: {}", e), - None, - ) - })?; - Ok(vec![Content::text("Undid the last edit")]) - } else { - Err(ErrorData::new( - ErrorCode::INVALID_PARAMS, - "No edit history available to undo".to_string(), - None, - )) - } - } else { - Err(ErrorData::new( - ErrorCode::INVALID_PARAMS, - "No edit history available to undo".to_string(), - None, - )) +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_detect_line_ending_lf() { + assert_eq!(detect_line_ending("hello\nworld\n"), "\n"); + } + + #[test] + fn test_detect_line_ending_crlf() { + assert_eq!(detect_line_ending("hello\r\nworld\r\n"), "\r\n"); + } + + #[test] + fn test_detect_line_ending_empty() { + assert_eq!(detect_line_ending("no newlines"), "\n"); + } + + #[test] + fn test_normalize_to_lf() { + assert_eq!(normalize_to_lf("a\r\nb\r\n"), "a\nb\n"); + assert_eq!(normalize_to_lf("a\rb\r"), "a\nb\n"); + assert_eq!(normalize_to_lf("a\nb\n"), "a\nb\n"); + } + + #[test] + fn test_restore_line_endings() { + assert_eq!(restore_line_endings("a\nb\n", "\r\n"), "a\r\nb\r\n"); + assert_eq!(restore_line_endings("a\nb\n", "\n"), "a\nb\n"); + } + + #[test] + fn test_strip_bom() { + assert_eq!(strip_bom("\u{FEFF}hello"), ("\u{FEFF}", "hello")); + assert_eq!(strip_bom("hello"), ("", "hello")); + } + + #[test] + fn test_normalize_for_fuzzy_match_trailing_whitespace() { + assert_eq!( + normalize_for_fuzzy_match("hello \nworld \n"), + "hello\nworld\n" + ); + } + + #[test] + fn test_normalize_for_fuzzy_match_smart_quotes() { + assert_eq!( + normalize_for_fuzzy_match("\u{201C}hello\u{201D}"), + "\"hello\"" + ); + assert_eq!( + normalize_for_fuzzy_match("\u{2018}it\u{2019}s\u{2019}"), + "'it's'" + ); + } + + #[test] + fn test_normalize_for_fuzzy_match_dashes() { + // em-dash and en-dash normalize to hyphen + assert_eq!(normalize_for_fuzzy_match("a\u{2014}b"), "a-b"); + assert_eq!(normalize_for_fuzzy_match("a\u{2013}b"), "a-b"); + } + + #[test] + fn test_normalize_for_fuzzy_match_special_spaces() { + assert_eq!(normalize_for_fuzzy_match("a\u{00A0}b"), "a b"); + } + + #[test] + fn test_fuzzy_find_exact_match() { + let result = fuzzy_find_text("hello world", "world"); + assert!(result.found); + assert_eq!(result.matched_text, "world"); + assert_eq!(result.content_for_replacement, "hello world"); + } + + #[test] + fn test_fuzzy_find_trailing_whitespace() { + // LLM sends without trailing spaces, file has them + let result = fuzzy_find_text("hello \nworld \n", "hello\nworld\n"); + assert!(result.found); + } + + #[test] + fn test_fuzzy_find_smart_quotes() { + let result = fuzzy_find_text("say \u{201C}hello\u{201D}", "say \"hello\""); + assert!(result.found); + } + + #[test] + fn test_fuzzy_find_not_found() { + let result = fuzzy_find_text("hello world", "xyz"); + assert!(!result.found); + } + + #[tokio::test] + async fn test_replace_preserves_crlf() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test.txt"); + std::fs::write(&path, "hello\r\nworld\r\n").unwrap(); + + let result = text_editor_replace(&path, "world", "rust", None, &None) + .await + .unwrap(); + + let content = std::fs::read_to_string(&path).unwrap(); + assert!(content.contains("\r\n"), "CRLF should be preserved"); + assert!(content.contains("rust")); + assert!(!result.is_empty()); + } + + #[tokio::test] + async fn test_replace_strips_bom() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test.txt"); + std::fs::write(&path, "\u{FEFF}hello world\n").unwrap(); + + text_editor_replace(&path, "world", "rust", None, &None) + .await + .unwrap(); + + let content = std::fs::read_to_string(&path).unwrap(); + assert!(content.starts_with('\u{FEFF}'), "BOM should be preserved"); + assert!(content.contains("rust")); + } + + #[tokio::test] + async fn test_replace_fuzzy_smart_quotes() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test.txt"); + // File has smart quotes + std::fs::write(&path, "say \u{201C}hello\u{201D}\n").unwrap(); + + // LLM sends ASCII quotes + let result = text_editor_replace(&path, "say \"hello\"", "say \"hi\"", None, &None).await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_replace_fuzzy_trailing_whitespace() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test.txt"); + std::fs::write(&path, "hello \nworld \n").unwrap(); + + // LLM sends without trailing whitespace + let result = text_editor_replace(&path, "hello\nworld", "hi\nearth", None, &None).await; + assert!(result.is_ok()); + } + + #[test] + fn test_generate_diff_string_simple() { + let old = "line1\nline2\nline3\n"; + let new = "line1\nmodified\nline3\n"; + let result = generate_diff_string(old, new, 4); + assert!(result.diff.contains("-"), "should have removed lines"); + assert!(result.diff.contains("+"), "should have added lines"); + assert!(result.diff.contains("line2"), "should show old line"); + assert!(result.diff.contains("modified"), "should show new line"); + assert!(result.diff.contains("line1"), "should show context"); + assert!(result.diff.contains("line3"), "should show context"); + } + + #[test] + fn test_generate_diff_string_context_limit() { + // 10 unchanged lines, then a change, then 10 more unchanged + let lines: Vec = (1..=21).map(|i| format!("line{}", i)).collect(); + let old = lines.join("\n") + "\n"; + let mut new_lines = lines.clone(); + new_lines[10] = "CHANGED".to_string(); + let new = new_lines.join("\n") + "\n"; + + let result = generate_diff_string(&old, &new, 2); + // Should have ellipsis for skipped context + assert!(result.diff.contains("..."), "should elide distant context"); + // Should NOT show line2 (too far from change at line 11) + assert!( + !result.diff.contains("line2"), + "should not show distant lines" + ); + // Should show nearby context + assert!(result.diff.contains("line10"), "should show nearby context"); + assert!(result.diff.contains("line12"), "should show nearby context"); + } + + #[test] + fn test_generate_diff_string_no_changes() { + let content = "same\n"; + let result = generate_diff_string(content, content, 4); + assert!(result.diff.is_empty()); + } + + #[tokio::test] + async fn test_replace_shows_diff_in_user_output() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test.txt"); + std::fs::write(&path, "hello\nworld\nfoo\n").unwrap(); + + let result = text_editor_replace(&path, "world", "rust", None, &None) + .await + .unwrap(); + + let user_content = result + .iter() + .find(|c| { + c.audience() + .is_some_and(|roles| roles.contains(&Role::User)) + }) + .unwrap() + .as_text() + .unwrap(); + + assert!( + user_content.text.contains("```diff"), + "should use diff format" + ); + assert!(user_content.text.contains("-"), "should show removed"); + assert!(user_content.text.contains("+"), "should show added"); } } - -pub fn save_file_history( - path: &PathBuf, - file_history: &std::sync::Arc< - std::sync::Mutex>>, - >, -) -> Result<(), ErrorData> { - let mut history = file_history.lock().unwrap(); - let content = if path.exists() { - std::fs::read_to_string(path).map_err(|e| { - ErrorData::new( - ErrorCode::INTERNAL_ERROR, - format!("Failed to read file: {}", e), - None, - ) - })? - } else { - String::new() - }; - history.entry(path.clone()).or_default().push(content); - Ok(()) -}