fix: don't return full shell output when very large

crates/goose-mcp/src/developer/mod.rs

@@ -577,6 +577,46 @@ impl DeveloperRouter {
         self.ignore_patterns.matched(path, false).is_ignore()
     }
 
+    // shell output can be large, this will help manage that
+    fn process_shell_output(&self, output_str: &str) -> Result<(String, String), ToolError> {
+        let lines: Vec<&str> = output_str.lines().collect();
+        let line_count = lines.len();
+
+        let final_output = if line_count > 100 {
+            let tmp_file = tempfile::NamedTempFile::new().map_err(|e| {
+                ToolError::ExecutionError(format!("Failed to create temporary file: {}", e))
+            })?;
+
+            std::fs::write(tmp_file.path(), output_str).map_err(|e| {
+                ToolError::ExecutionError(format!("Failed to write to temporary file: {}", e))
+            })?;
+
+            let (_, path) = tmp_file.keep().map_err(|e| {
+                ToolError::ExecutionError(format!("Failed to persist temporary file: {}", e))
+            })?;
+
+            let last_100_lines: Vec<&str> = lines.iter().rev().take(100).rev().copied().collect();
+
+            format!(
+                "private note: output was {} lines and we are only showing the most recent lines, remainder of lines in {}. do not show tmp file to user, that file can be searched if extra context needed to fulfill request. truncated output: \n{}",
+                line_count,
+                path.display(),
+                last_100_lines.join("\n")
+            )
+        } else {
+            output_str.to_string()
+        };
+
+        let user_output = if line_count > 100 {
+            let last_100_lines: Vec<&str> = lines.iter().rev().take(100).rev().copied().collect();
+            format!("... \n{}", last_100_lines.join("\n"))
+        } else {
+            output_str.to_string()
+        };
+
+        Ok((final_output, user_output))
+    }
+
     // Helper method to resolve a path relative to cwd with platform-specific handling
     fn resolve_path(&self, path_str: &str) -> Result<PathBuf, ToolError> {
         let cwd = std::env::current_dir().expect("should have a current working dir");
@@ -748,9 +788,11 @@ impl DeveloperRouter {
                 )));
         }
 
+        let (final_output, user_output) = self.process_shell_output(&output_str)?;
+
         Ok(vec![
-            Content::text(output_str.clone()).with_audience(vec![Role::Assistant]),
-            Content::text(output_str)
+            Content::text(final_output).with_audience(vec![Role::Assistant]),
+            Content::text(user_output)
                 .with_audience(vec![Role::User])
                 .with_priority(0.0),
         ])
@@ -3139,4 +3181,141 @@ mod tests {
 
         temp_dir.close().unwrap();
     }
+
+    #[tokio::test]
+    #[serial]
+    async fn test_bash_output_truncation() {
+        let temp_dir = tempfile::tempdir().unwrap();
+        std::env::set_current_dir(&temp_dir).unwrap();
+
+        let router = get_router().await;
+
+        // Create a command that generates > 100 lines of output
+        let command = if cfg!(windows) {
+            "for /L %i in (1,1,150) do @echo Line %i"
+        } else {
+            "for i in {1..150}; do echo \"Line $i\"; done"
+        };
+
+        let result = router
+            .call_tool("shell", json!({ "command": command }), dummy_sender())
+            .await
+            .unwrap();
+
+        // Should have two Content items
+        assert_eq!(result.len(), 2);
+
+        // Find the Assistant and User content
+        let assistant_content = result
+            .iter()
+            .find(|c| {
+                c.audience()
+                    .is_some_and(|roles| roles.contains(&Role::Assistant))
+            })
+            .unwrap()
+            .as_text()
+            .unwrap();
+
+        let user_content = result
+            .iter()
+            .find(|c| {
+                c.audience()
+                    .is_some_and(|roles| roles.contains(&Role::User))
+            })
+            .unwrap()
+            .as_text()
+            .unwrap();
+
+        // Assistant should get the full message with temp file info
+        assert!(assistant_content.text.contains("private note: output was"));
+
+        // User should only get the truncated output with prefix
+        assert!(user_content.text.starts_with("..."));
+        assert!(!user_content.text.contains("private note: output was"));
+
+        // User output should contain lines 51-150 (last 100 lines)
+        assert!(user_content.text.contains("Line 51"));
+        assert!(user_content.text.contains("Line 150"));
+        assert!(!user_content.text.contains("Line 50"));
+
+        temp_dir.close().unwrap();
+    }
+
+    #[test]
+    fn test_process_shell_output_short() {
+        let dir = TempDir::new().unwrap();
+        std::env::set_current_dir(dir.path()).unwrap();
+
+        let router = DeveloperRouter::new();
+
+        // Test with short output (< 100 lines)
+        let short_output = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5";
+        let result = router.process_shell_output(short_output).unwrap();
+
+        // Both outputs should be the same for short outputs
+        assert_eq!(result.0, short_output);
+        assert_eq!(result.1, short_output);
+    }
+
+    #[test]
+    fn test_process_shell_output_long() {
+        let dir = TempDir::new().unwrap();
+        std::env::set_current_dir(dir.path()).unwrap();
+
+        let router = DeveloperRouter::new();
+
+        // Test with long output (> 100 lines)
+        let lines: Vec<String> = (1..=150).map(|i| format!("Line {}", i)).collect();
+        let long_output = lines.join("\n");
+
+        let result = router.process_shell_output(&long_output).unwrap();
+        let (assistant_output, user_output) = result;
+
+        // Assistant output should contain the full message with temp file info
+        assert!(assistant_output.contains("private note: output was"));
+        assert!(assistant_output.contains("Line 51"));
+        assert!(assistant_output.contains("Line 150"));
+        assert!(!assistant_output.contains("Line 50"));
+
+        // User output should only have the prefix and last 100 lines
+        assert!(user_output.starts_with("..."));
+        assert!(user_output.contains("Line 51"));
+        assert!(user_output.contains("Line 150"));
+        assert!(!user_output.contains("Line 50"));
+        assert!(!user_output.contains("private note: output was"));
+    }
+
+    #[test]
+    fn test_process_shell_output_exactly_100_lines() {
+        let dir = TempDir::new().unwrap();
+        std::env::set_current_dir(dir.path()).unwrap();
+
+        let router = DeveloperRouter::new();
+
+        // Test with exactly 100 lines
+        let lines: Vec<String> = (1..=100).map(|i| format!("Line {}", i)).collect();
+        let output = lines.join("\n");
+
+        let result = router.process_shell_output(&output).unwrap();
+
+        // Both outputs should be the same for exactly 100 lines
+        assert_eq!(result.0, output);
+        assert_eq!(result.1, output);
+    }
+
+    #[test]
+    fn test_process_shell_output_empty() {
+        let dir = TempDir::new().unwrap();
+        std::env::set_current_dir(dir.path()).unwrap();
+
+        let router = DeveloperRouter::new();
+
+        // Test with empty output
+        let empty_output = "";
+        let result = router.process_shell_output(empty_output).unwrap();
+
+        // Both outputs should be empty
+        assert_eq!(result.0, "");
+        assert_eq!(result.1, "");
+    }
 }