From 2f86e6c24f14c3d42f1d4845407ce032e73410eb Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Tue, 23 Sep 2025 12:55:06 +1000 Subject: [PATCH 1/3] view can recognise a dir --- crates/goose-mcp/src/developer/text_editor.rs | 92 ++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-) diff --git a/crates/goose-mcp/src/developer/text_editor.rs b/crates/goose-mcp/src/developer/text_editor.rs index f02c9742c05d..9ec51197261e 100644 --- a/crates/goose-mcp/src/developer/text_editor.rs +++ b/crates/goose-mcp/src/developer/text_editor.rs @@ -468,15 +468,105 @@ pub fn recommend_read_range(path: &Path, total_lines: usize) -> Result Result, ErrorData> { + const MAX_ITEMS: usize = 50; // Maximum number of items to display + + // List files in the directory (similar to ls output) + let entries = std::fs::read_dir(path).map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read directory: {}", e), + None, + ) + })?; + + let mut files = Vec::new(); + let mut dirs = Vec::new(); + let mut total_count = 0; + + for entry in entries { + let entry = entry.map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read directory entry: {}", e), + None, + ) + })?; + + total_count += 1; + + // Only process up to MAX_ITEMS entries + if dirs.len() + files.len() < MAX_ITEMS { + let metadata = entry.metadata().map_err(|e| { + ErrorData::new( + ErrorCode::INTERNAL_ERROR, + format!("Failed to read metadata: {}", e), + None, + ) + })?; + + let name = entry.file_name().to_string_lossy().to_string(); + + if metadata.is_dir() { + dirs.push(format!("{}/", name)); + } else { + files.push(name); + } + } + } + + // Sort for consistent output + dirs.sort(); + files.sort(); + + let mut output = format!("'{}' is a directory. Contents:\n\n", path.display()); + + if !dirs.is_empty() { + output.push_str("Directories:\n"); + for dir in &dirs { + output.push_str(&format!(" {}\n", dir)); + } + output.push('\n'); + } + + if !files.is_empty() { + output.push_str("Files:\n"); + for file in &files { + output.push_str(&format!(" {}\n", file)); + } + } + + if dirs.is_empty() && files.is_empty() { + output.push_str(" (empty directory)\n"); + } + + // If we hit the limit, indicate there are more items + if total_count > MAX_ITEMS { + output.push_str(&format!( + "\n... and {} more items (showing first {} items)\n", + total_count - MAX_ITEMS, + MAX_ITEMS + )); + } + + Ok(vec![Content::text(output)]) +} + pub async fn text_editor_view( path: &PathBuf, view_range: Option<(usize, i64)>, ) -> Result, ErrorData> { + // Check if path is a directory + if path.is_dir() { + return list_directory_contents(path); + } + if !path.is_file() { return Err(ErrorData::new( ErrorCode::INTERNAL_ERROR, format!( - "The path '{}' does not exist or is not a file.", + "The path '{}' does not exist or is not accessible.", path.display() ), None, From f8e460fd60caf1e293057a0001a9cc277818aeca Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Tue, 23 Sep 2025 13:04:01 +1000 Subject: [PATCH 2/3] test coverage --- .../goose-mcp/src/developer/rmcp_developer.rs | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/crates/goose-mcp/src/developer/rmcp_developer.rs b/crates/goose-mcp/src/developer/rmcp_developer.rs index fe94b8766130..36082ba3c561 100644 --- a/crates/goose-mcp/src/developer/rmcp_developer.rs +++ b/crates/goose-mcp/src/developer/rmcp_developer.rs @@ -2963,6 +2963,148 @@ mod tests { assert!(text.text.contains("100: Line 100")); } + #[tokio::test] + #[serial] + async fn test_text_editor_view_directory() { + let temp_dir = tempfile::tempdir().unwrap(); + let temp_path = temp_dir.path(); + + // Create some test files and directories + fs::create_dir(temp_path.join("subdir1")).unwrap(); + fs::create_dir(temp_path.join("subdir2")).unwrap(); + fs::create_dir(temp_path.join("another_dir")).unwrap(); + + fs::write(temp_path.join("file1.txt"), "content1").unwrap(); + fs::write(temp_path.join("file2.rs"), "content2").unwrap(); + fs::write(temp_path.join("README.md"), "content3").unwrap(); + + let server = create_test_server(); + + // Test viewing a directory + let result = server + .text_editor(Parameters(TextEditorParams { + command: "view".to_string(), + 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; + + assert!(result.is_ok()); + let content = result.unwrap().content; + assert_eq!(content.len(), 1); + + // Check the content is a text message with directory listing + let text_content = content[0].as_text().expect("Expected text content"); + let output = &text_content.text; + + // Check that it identifies as a directory + assert!(output.contains("is a directory")); + assert!(output.contains("Contents:")); + + // Check directories are listed with trailing slash + assert!(output.contains("Directories:")); + assert!(output.contains("another_dir/")); + assert!(output.contains("subdir1/")); + assert!(output.contains("subdir2/")); + + // Check files are listed + assert!(output.contains("Files:")); + assert!(output.contains("file1.txt")); + assert!(output.contains("file2.rs")); + assert!(output.contains("README.md")); + } + + #[tokio::test] + #[serial] + async fn test_text_editor_view_directory_with_many_files() { + let temp_dir = tempfile::tempdir().unwrap(); + let temp_path = temp_dir.path(); + + // Create more than 50 files to test the limit + for i in 0..60 { + fs::write( + temp_path.join(format!("file{:03}.txt", i)), + format!("content{}", i), + ) + .unwrap(); + } + + // Create some directories too + for i in 0..10 { + fs::create_dir(temp_path.join(format!("dir{:02}", i))).unwrap(); + } + + let server = create_test_server(); + + let result = server + .text_editor(Parameters(TextEditorParams { + command: "view".to_string(), + 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; + + assert!(result.is_ok()); + let content = result.unwrap().content; + assert_eq!(content.len(), 1); + + let text_content = content[0].as_text().expect("Expected text content"); + let output = &text_content.text; + + // Check that it shows the limit message + assert!(output.contains("... and")); + assert!(output.contains("more items")); + assert!(output.contains("(showing first 50 items)")); + + // Count the actual number of items shown (should be 50) + let dir_count = output.matches("/\n").count(); // directories end with / + let file_count = output.matches(".txt\n").count(); // only counting .txt files for simplicity + assert!(dir_count + file_count <= 50); + } + + #[tokio::test] + #[serial] + async fn test_text_editor_view_empty_directory() { + let temp_dir = tempfile::tempdir().unwrap(); + let temp_path = temp_dir.path(); + + let server = create_test_server(); + + let result = server + .text_editor(Parameters(TextEditorParams { + command: "view".to_string(), + 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; + + assert!(result.is_ok()); + let content = result.unwrap().content; + assert_eq!(content.len(), 1); + + let text_content = content[0].as_text().expect("Expected text content"); + let output = &text_content.text; + + // Check that it shows empty directory message + assert!(output.contains("is a directory")); + assert!(output.contains("(empty directory)")); + } + #[test] #[serial] fn test_shell_output_truncation() { From 08c6b3b0603bd757efa5b262dd068ec989bc055f Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Tue, 23 Sep 2025 13:31:00 +1000 Subject: [PATCH 3/3] need to set tmp for tests --- crates/goose-mcp/src/developer/rmcp_developer.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/goose-mcp/src/developer/rmcp_developer.rs b/crates/goose-mcp/src/developer/rmcp_developer.rs index 36082ba3c561..d8fd5d73e118 100644 --- a/crates/goose-mcp/src/developer/rmcp_developer.rs +++ b/crates/goose-mcp/src/developer/rmcp_developer.rs @@ -2969,6 +2969,9 @@ mod tests { let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); + // Set the current directory before creating the server + std::env::set_current_dir(&temp_path).unwrap(); + // Create some test files and directories fs::create_dir(temp_path.join("subdir1")).unwrap(); fs::create_dir(temp_path.join("subdir2")).unwrap(); @@ -3025,6 +3028,9 @@ mod tests { let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); + // Set the current directory before creating the server + std::env::set_current_dir(&temp_path).unwrap(); + // Create more than 50 files to test the limit for i in 0..60 { fs::write( @@ -3078,6 +3084,9 @@ mod tests { let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); + // Set the current directory before creating the server + std::env::set_current_dir(&temp_path).unwrap(); + let server = create_test_server(); let result = server