agent: Create TerminalToolCard and display shell output while it's running (#29546)

Also, don't require a worktree to run the terminal tool.

Release Notes:

- N/A

Cargo.lock

@@ -730,6 +730,9 @@ dependencies = [
  "serde",
  "serde_json",
  "settings",
+ "task",
+ "terminal",
+ "terminal_view",
  "tree-sitter-rust",
  "ui",
  "unindent",

crates/assistant_tool/src/assistant_tool.rs

@@ -51,6 +51,13 @@ impl ToolUseStatus {
             ToolUseStatus::Error(out) => out.clone(),
         }
     }
+
+    pub fn error(&self) -> Option<SharedString> {
+        match self {
+            ToolUseStatus::Error(out) => Some(out.clone()),
+            _ => None,
+        }
+    }
 }
 
 /// The result of running a tool, containing both the asynchronous output

crates/assistant_tools/Cargo.toml

@@ -34,6 +34,9 @@ regex.workspace = true
 schemars.workspace = true
 serde.workspace = true
 serde_json.workspace = true
+task.workspace = true
+terminal.workspace = true
+terminal_view.workspace = true
 ui.workspace = true
 util.workspace = true
 web_search.workspace = true
@@ -51,6 +54,7 @@ project = { workspace = true, features = ["test-support"] }
 rand.workspace = true
 pretty_assertions.workspace = true
 settings = { workspace = true, features = ["test-support"] }
+task = { workspace = true, features = ["test-support"]}
 tree-sitter-rust.workspace = true
 workspace = { workspace = true, features = ["test-support"] }
 unindent.workspace = true

crates/assistant_tools/src/edit_file_tool.rs

@@ -24,6 +24,8 @@ use ui::{Disclosure, Tooltip, Window, prelude::*};
 use util::ResultExt;
 use workspace::Workspace;
 
+pub struct EditFileTool;
+
 #[derive(Debug, Serialize, Deserialize, JsonSchema)]
 pub struct EditFileToolInput {
     /// A user-friendly markdown description of what's being replaced. This will be shown in the UI.
@@ -75,8 +77,6 @@ struct PartialInput {
     new_string: String,
 }
 
-pub struct EditFileTool;
-
 const DEFAULT_UI_TEXT: &str = "Editing file";
 
 impl Tool for EditFileTool {
@@ -627,7 +627,6 @@ mod tests {
 
     #[test]
     fn still_streaming_ui_text_with_path() {
-        let tool = EditFileTool;
         let input = json!({
             "path": "src/main.rs",
             "display_description": "",
@@ -635,12 +634,11 @@ mod tests {
             "new_string": "new code"
         });
 
-        assert_eq!(tool.still_streaming_ui_text(&input), "src/main.rs");
+        assert_eq!(EditFileTool.still_streaming_ui_text(&input), "src/main.rs");
     }
 
     #[test]
     fn still_streaming_ui_text_with_description() {
-        let tool = EditFileTool;
         let input = json!({
             "path": "",
             "display_description": "Fix error handling",
@@ -648,12 +646,14 @@ mod tests {
             "new_string": "new code"
         });
 
-        assert_eq!(tool.still_streaming_ui_text(&input), "Fix error handling");
+        assert_eq!(
+            EditFileTool.still_streaming_ui_text(&input),
+            "Fix error handling",
+        );
     }
 
     #[test]
     fn still_streaming_ui_text_with_path_and_description() {
-        let tool = EditFileTool;
         let input = json!({
             "path": "src/main.rs",
             "display_description": "Fix error handling",
@@ -661,12 +661,14 @@ mod tests {
             "new_string": "new code"
         });
 
-        assert_eq!(tool.still_streaming_ui_text(&input), "Fix error handling");
+        assert_eq!(
+            EditFileTool.still_streaming_ui_text(&input),
+            "Fix error handling",
+        );
     }
 
     #[test]
     fn still_streaming_ui_text_no_path_or_description() {
-        let tool = EditFileTool;
         let input = json!({
             "path": "",
             "display_description": "",
@@ -674,14 +676,19 @@ mod tests {
             "new_string": "new code"
         });
 
-        assert_eq!(tool.still_streaming_ui_text(&input), DEFAULT_UI_TEXT);
+        assert_eq!(
+            EditFileTool.still_streaming_ui_text(&input),
+            DEFAULT_UI_TEXT,
+        );
     }
 
     #[test]
     fn still_streaming_ui_text_with_null() {
-        let tool = EditFileTool;
         let input = serde_json::Value::Null;
 
-        assert_eq!(tool.still_streaming_ui_text(&input), DEFAULT_UI_TEXT);
+        assert_eq!(
+            EditFileTool.still_streaming_ui_text(&input),
+            DEFAULT_UI_TEXT,
+        );
     }
 }

crates/assistant_tools/src/terminal_tool.rs

@@ -1,23 +1,32 @@
 use crate::schema::json_schema_for;
 use anyhow::{Context as _, Result, anyhow};
-use assistant_tool::{ActionLog, Tool, ToolResult};
+use assistant_tool::{ActionLog, Tool, ToolCard, ToolResult, ToolUseStatus};
-use futures::io::BufReader;
+use gpui::{
-use futures::{AsyncBufReadExt, AsyncReadExt, FutureExt};
+    Animation, AnimationExt, AnyWindowHandle, App, AppContext, Empty, Entity, EntityId, Task,
-use gpui::{AnyWindowHandle, App, AppContext, Entity, Task};
+    Transformation, WeakEntity, Window, percentage,
+};
 use language_model::{LanguageModelRequestMessage, LanguageModelToolSchemaFormat};
-use project::Project;
+use project::{Project, terminals::TerminalKind};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::future;
+use std::{
-use util::get_system_shell;
+    env,
+    path::{Path, PathBuf},
+    process::ExitStatus,
+    sync::Arc,
+    time::{Duration, Instant},
+};
+use terminal_view::TerminalView;
+use ui::{Disclosure, IconName, Tooltip, prelude::*};
+use util::{
+    get_system_shell, markdown::MarkdownInlineCode, size::format_file_size,
+    time::duration_alt_display,
+};
+use workspace::Workspace;
 
-use std::path::Path;
+const COMMAND_OUTPUT_LIMIT: usize = 16 * 1024;
-use std::sync::Arc;
-use ui::IconName;
-use util::command::new_smol_command;
-use util::markdown::MarkdownInlineCode;
 
-#[derive(Debug, Serialize, Deserialize, JsonSchema)]
+#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)]
 pub struct TerminalToolInput {
     /// The one-liner command to execute.
     command: String,
@@ -75,308 +84,426 @@ impl Tool for TerminalTool {
         _messages: &[LanguageModelRequestMessage],
         project: Entity<Project>,
         _action_log: Entity<ActionLog>,
-        _window: Option<AnyWindowHandle>,
+        window: Option<AnyWindowHandle>,
         cx: &mut App,
     ) -> ToolResult {
+        let Some(window) = window else {
+            return Task::ready(Err(anyhow!("no window options"))).into();
+        };
+
         let input: TerminalToolInput = match serde_json::from_value(input) {
             Ok(input) => input,
             Err(err) => return Task::ready(Err(anyhow!(err))).into(),
         };
 
-        let project = project.read(cx);
         let input_path = Path::new(&input.cd);
-        let working_dir = if input.cd == "." {
+        let working_dir = match working_dir(cx, &input, &project, input_path) {
-            // Accept "." as meaning "the one worktree" if we only have one worktree.
+            Ok(dir) => dir,
-            let mut worktrees = project.worktrees(cx);
+            Err(err) => return Task::ready(Err(anyhow!(err))).into(),
-
-            let only_worktree = match worktrees.next() {
-                Some(worktree) => worktree,
-                None => {
-                    return Task::ready(Err(anyhow!("No worktrees found in the project"))).into();
-                }
-            };
-
-            if worktrees.next().is_some() {
-                return Task::ready(Err(anyhow!(
-                    "'.' is ambiguous in multi-root workspaces. Please specify a root directory explicitly."
-                ))).into();
-            }
-
-            only_worktree.read(cx).abs_path()
-        } else if input_path.is_absolute() {
-            // Absolute paths are allowed, but only if they're in one of the project's worktrees.
-            if !project
-                .worktrees(cx)
-                .any(|worktree| input_path.starts_with(&worktree.read(cx).abs_path()))
-            {
-                return Task::ready(Err(anyhow!(
-                    "The absolute path must be within one of the project's worktrees"
-                )))
-                .into();
-            }
-
-            input_path.into()
-        } else {
-            let Some(worktree) = project.worktree_for_root_name(&input.cd, cx) else {
-                return Task::ready(Err(anyhow!(
-                    "`cd` directory {} not found in the project",
-                    &input.cd
-                )))
-                .into();
-            };
-
-            worktree.read(cx).abs_path()
         };
+        let terminal = project.update(cx, |project, cx| {
+            project.create_terminal(
+                TerminalKind::Task(task::SpawnInTerminal {
+                    command: get_system_shell(),
+                    args: vec!["-c".into(), input.command.clone()],
+                    cwd: working_dir.clone(),
+                    ..Default::default()
+                }),
+                window,
+                cx,
+            )
+        });
 
-        cx.background_spawn(run_command_limited(working_dir, input.command))
+        let card = cx.new(|cx| {
-            .into()
+            TerminalToolCard::new(input.command.clone(), working_dir.clone(), cx.entity_id())
-    }
+        });
-}
 
-const LIMIT: usize = 16 * 1024;
+        let output = cx.spawn({
+            let card = card.clone();
+            async move |cx| {
+                let terminal = terminal.await?;
+                let workspace = window
+                    .downcast::<Workspace>()
+                    .and_then(|handle| handle.entity(cx).ok())
+                    .context("no workspace entity in root of window")?;
 
-async fn run_command_limited(working_dir: Arc<Path>, command: String) -> Result<String> {
+                let terminal_view = window.update(cx, |_, window, cx| {
-    let shell = get_system_shell();
+                    cx.new(|cx| {
+                        TerminalView::new(
+                            terminal.clone(),
+                            workspace.downgrade(),
+                            None,
+                            project.downgrade(),
+                            window,
+                            cx,
+                        )
+                    })
+                })?;
+                let _ = card.update(cx, |card, _| {
+                    card.terminal = Some(terminal_view.clone());
+                    card.start_instant = Instant::now();
+                });
 
-    let mut cmd = new_smol_command(&shell)
+                let exit_status = terminal
-        .arg("-c")
+                    .update(cx, |terminal, cx| terminal.wait_for_completed_task(cx))?
-        .arg(&command)
+                    .await;
-        .current_dir(working_dir)
+                let (content, content_line_count) = terminal.update(cx, |terminal, _| {
-        .stdout(std::process::Stdio::piped())
+                    (terminal.get_content(), terminal.total_lines())
-        .stderr(std::process::Stdio::piped())
+                })?;
-        .spawn()
-        .context("Failed to execute terminal command")?;
 
-    let mut combined_buffer = String::with_capacity(LIMIT + 1);
+                let previous_len = content.len();
+                let (processed_content, finished_with_empty_output) =
+                    process_content(content, &input.command, exit_status);
 
-    let mut out_reader = BufReader::new(cmd.stdout.take().context("Failed to get stdout")?);
+                let _ = card.update(cx, |card, _| {
-    let mut out_tmp_buffer = String::with_capacity(512);
+                    card.command_finished = true;
-    let mut err_reader = BufReader::new(cmd.stderr.take().context("Failed to get stderr")?);
+                    card.exit_status = exit_status;
-    let mut err_tmp_buffer = String::with_capacity(512);
+                    card.was_content_truncated = processed_content.len() < previous_len;
+                    card.original_content_len = previous_len;
+                    card.content_line_count = content_line_count;
+                    card.finished_with_empty_output = finished_with_empty_output;
+                    card.elapsed_time = Some(card.start_instant.elapsed());
+                });
 
-    let mut out_line = Box::pin(
+                Ok(processed_content)
-        out_reader
-            .read_line(&mut out_tmp_buffer)
-            .left_future()
-            .fuse(),
-    );
-    let mut err_line = Box::pin(
-        err_reader
-            .read_line(&mut err_tmp_buffer)
-            .left_future()
-            .fuse(),
-    );
-
-    let mut has_stdout = true;
-    let mut has_stderr = true;
-    while (has_stdout || has_stderr) && combined_buffer.len() < LIMIT + 1 {
-        futures::select_biased! {
-            read = out_line => {
-                drop(out_line);
-                combined_buffer.extend(out_tmp_buffer.drain(..));
-                if read? == 0 {
-                    out_line = Box::pin(future::pending().right_future().fuse());
-                    has_stdout = false;
-                } else {
-                    out_line = Box::pin(out_reader.read_line(&mut out_tmp_buffer).left_future().fuse());
-                }
             }
-            read = err_line => {
+        });
-                drop(err_line);
-                combined_buffer.extend(err_tmp_buffer.drain(..));
-                if read? == 0 {
-                    err_line = Box::pin(future::pending().right_future().fuse());
-                    has_stderr = false;
-                } else {
-                    err_line = Box::pin(err_reader.read_line(&mut err_tmp_buffer).left_future().fuse());
-                }
-            }
-        };
-    }
 
-    drop((out_line, err_line));
+        ToolResult {
-
+            output,
-    let truncated = combined_buffer.len() > LIMIT;
+            card: Some(card.into()),
-    combined_buffer.truncate(LIMIT);
-
-    consume_reader(out_reader, truncated).await?;
-    consume_reader(err_reader, truncated).await?;
-
-    // Handle potential errors during status retrieval, including interruption.
-    match cmd.status().await {
-        Ok(status) => {
-            let output_string = if truncated {
-                // Valid to find `\n` in UTF-8 since 0-127 ASCII characters are not used in
-                // multi-byte characters.
-                let last_line_ix = combined_buffer.bytes().rposition(|b| b == b'\n');
-                let buffer_content =
-                    &combined_buffer[..last_line_ix.unwrap_or(combined_buffer.len())];
-
-                format!(
-                    "Command output too long. The first {} bytes:\n\n{}",
-                    buffer_content.len(),
-                    output_block(buffer_content),
-                )
-            } else {
-                output_block(&combined_buffer)
-            };
-
-            let output_with_status = if status.success() {
-                if output_string.is_empty() {
-                    "Command executed successfully.".to_string()
-                } else {
-                    output_string
-                }
-            } else {
-                format!(
-                    "Command failed with exit code {} (shell: {}).\n\n{}",
-                    status.code().unwrap_or(-1),
-                    shell,
-                    output_string,
-                )
-            };
-
-            Ok(output_with_status)
-        }
-        Err(err) => {
-            // Error occurred getting status (potential interruption). Include partial output.
-            let partial_output = output_block(&combined_buffer);
-            let error_message = format!(
-                "Command failed or was interrupted.\nPartial output captured:\n\n{}",
-                partial_output
-            );
-            Err(anyhow!(err).context(error_message))
         }
     }
 }
 
-async fn consume_reader<T: AsyncReadExt + Unpin>(
+fn process_content(
-    mut reader: BufReader<T>,
+    content: String,
-    truncated: bool,
+    command: &str,
-) -> Result<(), std::io::Error> {
+    exit_status: Option<ExitStatus>,
-    loop {
+) -> (String, bool) {
-        let skipped_bytes = reader.fill_buf().await?;
+    let should_truncate = content.len() > COMMAND_OUTPUT_LIMIT;
-        if skipped_bytes.is_empty() {
+
-            break;
+    let content = if should_truncate {
+        let mut end_ix = COMMAND_OUTPUT_LIMIT.min(content.len());
+        while !content.is_char_boundary(end_ix) {
+            end_ix -= 1;
         }
-        let skipped_bytes_len = skipped_bytes.len();
+        // Don't truncate mid-line, clear the remainder of the last line
-        reader.consume_unpin(skipped_bytes_len);
+        end_ix = content[..end_ix].rfind('\n').unwrap_or(end_ix);
+        &content[..end_ix]
+    } else {
+        content.as_str()
+    };
+    let is_empty = content.trim().is_empty();
 
-        // Should only skip if we went over the limit
+    let content = format!(
-        debug_assert!(truncated);
-    }
-    Ok(())
-}
-
-fn output_block(output: &str) -> String {
-    format!(
         "```\n{}{}```",
-        output,
+        content,
-        if output.ends_with('\n') { "" } else { "\n" }
+        if content.ends_with('\n') { "" } else { "\n" }
-    )
+    );
-}
 
-#[cfg(test)]
+    let content = if should_truncate {
-#[cfg(not(windows))]
+        format!(
-mod tests {
+            "Command output too long. The first {} bytes:\n\n{}",
-    use gpui::TestAppContext;
+            content.len(),
-
+            content,
-    use super::*;
-
-    #[gpui::test(iterations = 10)]
-    async fn test_run_command_simple(cx: &mut TestAppContext) {
-        cx.executor().allow_parking();
-
-        let result =
-            run_command_limited(Path::new(".").into(), "echo 'Hello, World!'".to_string()).await;
-
-        assert!(result.is_ok());
-        assert_eq!(result.unwrap(), "```\nHello, World!\n```");
-    }
-
-    #[gpui::test(iterations = 10)]
-    async fn test_interleaved_stdout_stderr(cx: &mut TestAppContext) {
-        cx.executor().allow_parking();
-
-        let command = "echo 'stdout 1' && sleep 0.01 && echo 'stderr 1' >&2 && sleep 0.01 && echo 'stdout 2' && sleep 0.01 && echo 'stderr 2' >&2";
-        let result = run_command_limited(Path::new(".").into(), command.to_string()).await;
-
-        assert!(result.is_ok());
-        assert_eq!(
-            result.unwrap(),
-            "```\nstdout 1\nstderr 1\nstdout 2\nstderr 2\n```"
-        );
-    }
-
-    #[gpui::test(iterations = 10)]
-    async fn test_multiple_output_reads(cx: &mut TestAppContext) {
-        cx.executor().allow_parking();
-
-        // Command with multiple outputs that might require multiple reads
-        let result = run_command_limited(
-            Path::new(".").into(),
-            "echo '1'; sleep 0.01; echo '2'; sleep 0.01; echo '3'".to_string(),
         )
-        .await;
+    } else {
+        content
+    };
 
-        assert!(result.is_ok());
+    let content = match exit_status {
-        assert_eq!(result.unwrap(), "```\n1\n2\n3\n```");
+        Some(exit_status) if exit_status.success() => {
-    }
+            if is_empty {
+                "Command executed successfully.".to_string()
+            } else {
+                content.to_string()
+            }
+        }
+        Some(exit_status) => {
+            let code = exit_status.code().unwrap_or(-1);
+            if is_empty {
+                format!("Command \"{command}\" failed with exit code {code}.")
+            } else {
+                format!("Command \"{command}\" failed with exit code {code}.\n\n{content}")
+            }
+        }
+        None => {
+            format!(
+                "Command failed or was interrupted.\nPartial output captured:\n\n{}",
+                content,
+            )
+        }
+    };
+    (content, is_empty)
+}
 
-    #[gpui::test(iterations = 10)]
+fn working_dir(
-    async fn test_output_truncation_single_line(cx: &mut TestAppContext) {
+    cx: &mut App,
-        cx.executor().allow_parking();
+    input: &TerminalToolInput,
+    project: &Entity<Project>,
+    input_path: &Path,
+) -> Result<Option<PathBuf>, &'static str> {
+    let project = project.read(cx);
 
-        let cmd = format!("echo '{}'; sleep 0.01;", "X".repeat(LIMIT * 2));
+    if input.cd == "." {
+        // Accept "." as meaning "the one worktree" if we only have one worktree.
+        let mut worktrees = project.worktrees(cx);
 
-        let result = run_command_limited(Path::new(".").into(), cmd).await;
+        match worktrees.next() {
+            Some(worktree) => {
+                if worktrees.next().is_some() {
+                    return Err(
+                        "'.' is ambiguous in multi-root workspaces. Please specify a root directory explicitly.",
+                    );
+                }
+                Ok(Some(worktree.read(cx).abs_path().to_path_buf()))
+            }
+            None => Ok(None),
+        }
+    } else if input_path.is_absolute() {
+        // Absolute paths are allowed, but only if they're in one of the project's worktrees.
+        if !project
+            .worktrees(cx)
+            .any(|worktree| input_path.starts_with(&worktree.read(cx).abs_path()))
+        {
+            return Err("The absolute path must be within one of the project's worktrees");
+        }
 
-        assert!(result.is_ok());
+        Ok(Some(input_path.into()))
-        let output = result.unwrap();
+    } else {
+        let Some(worktree) = project.worktree_for_root_name(&input.cd, cx) else {
+            return Err("`cd` directory {} not found in the project");
+        };
 
-        let content_start = output.find("```\n").map(|i| i + 4).unwrap_or(0);
+        Ok(Some(worktree.read(cx).abs_path().to_path_buf()))
-        let content_end = output.rfind("\n```").unwrap_or(output.len());
+    }
-        let content_length = content_end - content_start;
+}
-
+
-        // Output should be exactly the limit
+struct TerminalToolCard {
-        assert_eq!(content_length, LIMIT);
+    input_command: String,
-    }
+    working_dir: Option<PathBuf>,
-
+    entity_id: EntityId,
-    #[gpui::test(iterations = 10)]
+    exit_status: Option<ExitStatus>,
-    async fn test_output_truncation_multiline(cx: &mut TestAppContext) {
+    terminal: Option<Entity<TerminalView>>,
-        cx.executor().allow_parking();
+    command_finished: bool,
-
+    was_content_truncated: bool,
-        let cmd = format!("echo '{}'; ", "X".repeat(120)).repeat(160);
+    finished_with_empty_output: bool,
-        let result = run_command_limited(Path::new(".").into(), cmd).await;
+    content_line_count: usize,
-
+    original_content_len: usize,
-        assert!(result.is_ok());
+    preview_expanded: bool,
-        let output = result.unwrap();
+    start_instant: Instant,
-
+    elapsed_time: Option<Duration>,
-        assert!(output.starts_with("Command output too long. The first 16334 bytes:\n\n"));
+}
-
+
-        let content_start = output.find("```\n").map(|i| i + 4).unwrap_or(0);
+impl TerminalToolCard {
-        let content_end = output.rfind("\n```").unwrap_or(output.len());
+    pub fn new(input_command: String, working_dir: Option<PathBuf>, entity_id: EntityId) -> Self {
-        let content_length = content_end - content_start;
+        Self {
-
+            input_command,
-        assert!(content_length <= LIMIT);
+            working_dir,
-    }
+            entity_id,
-
+            exit_status: None,
-    #[gpui::test(iterations = 10)]
+            terminal: None,
-    async fn test_command_failure(cx: &mut TestAppContext) {
+            command_finished: false,
-        cx.executor().allow_parking();
+            was_content_truncated: false,
-
+            finished_with_empty_output: false,
-        let result = run_command_limited(Path::new(".").into(), "exit 42".to_string()).await;
+            original_content_len: 0,
-
+            content_line_count: 0,
-        assert!(result.is_ok());
+            preview_expanded: true,
-        let output = result.unwrap();
+            start_instant: Instant::now(),
-
+            elapsed_time: None,
-        // Extract the shell name from path for cleaner test output
+        }
-        let shell_path = std::env::var("SHELL").unwrap_or("bash".to_string());
+    }
-
+}
-        let expected_output = format!(
+
-            "Command failed with exit code 42 (shell: {}).\n\n```\n\n```",
+impl ToolCard for TerminalToolCard {
-            shell_path
+    fn render(
-        );
+        &mut self,
-        assert_eq!(output, expected_output);
+        status: &ToolUseStatus,
+        _window: &mut Window,
+        _workspace: WeakEntity<Workspace>,
+        cx: &mut Context<Self>,
+    ) -> impl IntoElement {
+        let Some(terminal) = self.terminal.as_ref() else {
+            return Empty.into_any();
+        };
+
+        let tool_failed = matches!(status, ToolUseStatus::Error(_));
+        let command_failed =
+            self.command_finished && self.exit_status.is_none_or(|code| !code.success());
+        if (tool_failed || command_failed) && self.elapsed_time.is_none() {
+            self.elapsed_time = Some(self.start_instant.elapsed());
+        }
+        let time_elapsed = self
+            .elapsed_time
+            .unwrap_or_else(|| self.start_instant.elapsed());
+        let should_hide_terminal =
+            tool_failed || self.finished_with_empty_output || !self.preview_expanded;
+
+        let border_color = cx.theme().colors().border.opacity(0.6);
+        let header_bg = cx
+            .theme()
+            .colors()
+            .element_background
+            .blend(cx.theme().colors().editor_foreground.opacity(0.025));
+
+        let header_label = h_flex()
+            .w_full()
+            .max_w_full()
+            .px_1()
+            .gap_0p5()
+            .opacity(0.8)
+            .child(
+                h_flex()
+                    .child(
+                        Icon::new(IconName::Terminal)
+                            .size(IconSize::XSmall)
+                            .color(Color::Muted),
+                    )
+                    .child(
+                        div()
+                            .id(("terminal-tool-header-input-command", self.entity_id))
+                            .text_size(rems(0.8125))
+                            .font_buffer(cx)
+                            .child(self.input_command.clone())
+                            .ml_1p5()
+                            .mr_0p5()
+                            .tooltip({
+                                let path = self
+                                    .working_dir
+                                    .as_ref()
+                                    .cloned()
+                                    .or_else(|| env::current_dir().ok())
+                                    .map(|path| format!("\"{}\"", path.display()))
+                                    .unwrap_or_else(|| "current directory".to_string());
+                                Tooltip::text(if self.command_finished {
+                                    format!("Ran in {path}")
+                                } else {
+                                    format!("Running in {path}")
+                                })
+                            }),
+                    ),
+            )
+            .into_any_element();
+
+        let header = h_flex()
+            .flex_none()
+            .p_1()
+            .gap_1()
+            .justify_between()
+            .rounded_t_md()
+            .bg(header_bg)
+            .child(header_label)
+            .map(|header| {
+                let header = header
+                    .when(self.was_content_truncated, |header| {
+                        let tooltip =
+                            if self.content_line_count + 10 > terminal::MAX_SCROLL_HISTORY_LINES {
+                                "Output exceeded terminal max lines and was \
+                                truncated, the model received the first 16 KB."
+                                    .to_string()
+                            } else {
+                                format!(
+                                    "Output is {} long, to avoid unexpected token usage, \
+                                    only 16 KB was sent back to the model.",
+                                    format_file_size(self.original_content_len as u64, true),
+                                )
+                            };
+                        header.child(
+                            div()
+                                .id(("terminal-tool-truncated-label", self.entity_id))
+                                .tooltip(Tooltip::text(tooltip))
+                                .child(
+                                    Label::new("(truncated)")
+                                        .color(Color::Disabled)
+                                        .size(LabelSize::Small),
+                                ),
+                        )
+                    })
+                    .when(time_elapsed > Duration::from_secs(10), |header| {
+                        header.child(
+                            Label::new(format!("({})", duration_alt_display(time_elapsed)))
+                                .buffer_font(cx)
+                                .color(Color::Disabled)
+                                .size(LabelSize::Small),
+                        )
+                    });
+
+                if tool_failed || command_failed {
+                    header.child(
+                        div()
+                            .id(("terminal-tool-error-code-indicator", self.entity_id))
+                            .child(
+                                Icon::new(IconName::Close)
+                                    .size(IconSize::Small)
+                                    .color(Color::Error),
+                            )
+                            .when(command_failed && self.exit_status.is_some(), |this| {
+                                this.tooltip(Tooltip::text(format!(
+                                    "Exited with code {}",
+                                    self.exit_status
+                                        .and_then(|status| status.code())
+                                        .unwrap_or(-1),
+                                )))
+                            })
+                            .when(
+                                !command_failed && tool_failed && status.error().is_some(),
+                                |this| {
+                                    this.tooltip(Tooltip::text(format!(
+                                        "Error: {}",
+                                        status.error().unwrap(),
+                                    )))
+                                },
+                            ),
+                    )
+                } else if self.command_finished {
+                    header.child(
+                        Icon::new(IconName::Check)
+                            .size(IconSize::Small)
+                            .color(Color::Success),
+                    )
+                } else {
+                    header.child(
+                        Icon::new(IconName::ArrowCircle)
+                            .size(IconSize::Small)
+                            .color(Color::Info)
+                            .with_animation(
+                                "arrow-circle",
+                                Animation::new(Duration::from_secs(2)).repeat(),
+                                |icon, delta| {
+                                    icon.transform(Transformation::rotate(percentage(delta)))
+                                },
+                            ),
+                    )
+                }
+            })
+            .when(!tool_failed && !self.finished_with_empty_output, |header| {
+                header.child(
+                    Disclosure::new(
+                        ("terminal-tool-disclosure", self.entity_id),
+                        self.preview_expanded,
+                    )
+                    .opened_icon(IconName::ChevronUp)
+                    .closed_icon(IconName::ChevronDown)
+                    .on_click(cx.listener(
+                        move |this, _event, _window, _cx| {
+                            this.preview_expanded = !this.preview_expanded;
+                        },
+                    )),
+                )
+            });
+
+        v_flex()
+            .mb_2()
+            .border_1()
+            .when(tool_failed || command_failed, |card| card.border_dashed())
+            .border_color(border_color)
+            .rounded_lg()
+            .overflow_hidden()
+            .child(header)
+            .when(!should_hide_terminal, |this| {
+                this.child(div().child(terminal.clone()).min_h(px(250.0)))
+            })
+            .into_any()
     }
 }

crates/assistant_tools/src/terminal_tool/description.md

@@ -1,6 +1,8 @@
 Executes a shell one-liner and returns the combined output.
 
-This tool spawns a process using the user's current shell, combines stdout and stderr into one interleaved stream as they are produced (preserving the order of writes), and captures that stream into a string which is returned.
+This tool spawns a process using the user's shell, reads from stdout and stderr (preserving the order of writes), and returns a string with the combined output result.
+
+The output results will be shown to the user already, only list it again if necessary, avoid being redundant.
 
 Make sure you use the `cd` parameter to navigate to one of the root directories of the project. NEVER do it as part of the `command` itself, otherwise it will error.
 

crates/assistant_tools/src/web_search_tool.rs

@@ -23,7 +23,6 @@ pub struct WebSearchToolInput {
     query: String,
 }
 
-#[derive(RegisterComponent)]
 pub struct WebSearchTool;
 
 impl Tool for WebSearchTool {
@@ -84,6 +83,7 @@ impl Tool for WebSearchTool {
     }
 }
 
+#[derive(RegisterComponent)]
 struct WebSearchToolCard {
     response: Option<Result<WebSearchResponse>>,
     _task: Task<()>,
@@ -185,15 +185,11 @@ impl ToolCard for WebSearchToolCard {
     }
 }
 
-impl Component for WebSearchTool {
+impl Component for WebSearchToolCard {
     fn scope() -> ComponentScope {
         ComponentScope::Agent
     }
 
-    fn sort_name() -> &'static str {
-        "ToolWebSearch"
-    }
-
     fn preview(window: &mut Window, cx: &mut App) -> Option<AnyElement> {
         let in_progress_search = cx.new(|cx| WebSearchToolCard {
             response: None,

crates/image_viewer/src/image_info.rs

@@ -2,6 +2,7 @@ use gpui::{Context, Entity, IntoElement, ParentElement, Render, Subscription, di
 use project::image_store::{ImageFormat, ImageMetadata};
 use settings::Settings;
 use ui::prelude::*;
+use util::size::format_file_size;
 use workspace::{ItemHandle, StatusItemView, Workspace};
 
 use crate::{ImageFileSizeUnit, ImageView, ImageViewerSettings};
@@ -36,27 +37,9 @@ impl ImageInfo {
     }
 }
 
-fn format_file_size(size: u64, image_unit_type: ImageFileSizeUnit) -> String {
+fn format_image_size(size: u64, image_unit_type: ImageFileSizeUnit) -> String {
-    match image_unit_type {
+    let use_decimal = matches!(image_unit_type, ImageFileSizeUnit::Decimal);
-        ImageFileSizeUnit::Binary => {
+    format_file_size(size, use_decimal)
-            if size < 1024 {
-                format!("{size}B")
-            } else if size < 1024 * 1024 {
-                format!("{:.1}KiB", size as f64 / 1024.0)
-            } else {
-                format!("{:.1}MiB", size as f64 / (1024.0 * 1024.0))
-            }
-        }
-        ImageFileSizeUnit::Decimal => {
-            if size < 1000 {
-                format!("{size}B")
-            } else if size < 1000 * 1000 {
-                format!("{:.1}KB", size as f64 / 1000.0)
-            } else {
-                format!("{:.1}MB", size as f64 / (1000.0 * 1000.0))
-            }
-        }
-    }
 }
 
 impl Render for ImageInfo {
@@ -69,7 +52,7 @@ impl Render for ImageInfo {
 
         let mut components = Vec::new();
         components.push(format!("{}x{}", metadata.width, metadata.height));
-        components.push(format_file_size(metadata.file_size, settings.unit));
+        components.push(format_image_size(metadata.file_size, settings.unit));
 
         if let Some(colors) = metadata.colors {
             components.push(format!(

crates/project/src/terminals.rs

@@ -514,7 +514,7 @@ impl Project {
         terminal_handle: &Entity<Terminal>,
         cx: &mut App,
     ) {
-        terminal_handle.update(cx, |terminal, _| terminal.input_bytes(command.into_bytes()));
+        terminal_handle.update(cx, |terminal, _| terminal.input(command));
     }
 
     pub fn local_terminal_handles(&self) -> &Vec<WeakEntity<terminal::Terminal>> {

crates/terminal/src/terminal.rs

@@ -313,7 +313,7 @@ impl Display for TerminalError {
 
 // https://github.com/alacritty/alacritty/blob/cb3a79dbf6472740daca8440d5166c1d4af5029e/extra/man/alacritty.5.scd?plain=1#L207-L213
 const DEFAULT_SCROLL_HISTORY_LINES: usize = 10_000;
-const MAX_SCROLL_HISTORY_LINES: usize = 100_000;
+pub const MAX_SCROLL_HISTORY_LINES: usize = 100_000;
 const URL_REGEX: &str = r#"(ipfs:|ipns:|magnet:|mailto:|gemini://|gopher://|https://|http://|news:|file://|git://|ssh:|ftp://)[^\u{0000}-\u{001F}\u{007F}-\u{009F}<>"\s{-}\^⟨⟩`]+"#;
 // Optional suffix matches MSBuild diagnostic suffixes for path parsing in PathLikeWithPosition
 // https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-diagnostic-format-for-tasks
@@ -1216,15 +1216,11 @@ impl Terminal {
     }
 
     ///Write the Input payload to the tty.
-    fn write_to_pty(&self, input: String) {
+    fn write_to_pty(&self, input: impl Into<Vec<u8>>) {
-        self.pty_tx.notify(input.into_bytes());
+        self.pty_tx.notify(input.into());
     }
 
-    fn write_bytes_to_pty(&self, input: Vec<u8>) {
+    pub fn input(&mut self, input: impl Into<Vec<u8>>) {
-        self.pty_tx.notify(input);
-    }
-
-    pub fn input(&mut self, input: String) {
         self.events
             .push_back(InternalEvent::Scroll(AlacScroll::Bottom));
         self.events.push_back(InternalEvent::SetSelection(None));
@@ -1232,14 +1228,6 @@ impl Terminal {
         self.write_to_pty(input);
     }
 
-    pub fn input_bytes(&mut self, input: Vec<u8>) {
-        self.events
-            .push_back(InternalEvent::Scroll(AlacScroll::Bottom));
-        self.events.push_back(InternalEvent::SetSelection(None));
-
-        self.write_bytes_to_pty(input);
-    }
-
     pub fn toggle_vi_mode(&mut self) {
         self.events.push_back(InternalEvent::ToggleViMode);
     }
@@ -1420,6 +1408,13 @@ impl Terminal {
         }
     }
 
+    pub fn get_content(&self) -> String {
+        let term = self.term.lock_unfair();
+        let start = AlacPoint::new(term.topmost_line(), Column(0));
+        let end = AlacPoint::new(term.bottommost_line(), term.last_column());
+        term.bounds_to_string(start, end)
+    }
+
     pub fn last_n_non_empty_lines(&self, n: usize) -> Vec<String> {
         let term = self.term.clone();
         let terminal = term.lock_unfair();
@@ -1858,6 +1853,8 @@ impl Terminal {
                 let completion_receiver = task.completion_rx.clone();
                 return cx
                     .spawn(async move |_| completion_receiver.recv().await.log_err().flatten());
+            } else if let Ok(status) = task.completion_rx.try_recv() {
+                return Task::ready(status);
             }
         }
         Task::ready(None)

crates/terminal_view/src/terminal_element.rs

@@ -1036,7 +1036,7 @@ impl InputHandler for TerminalInputHandler {
         cx: &mut App,
     ) {
         self.terminal.update(cx, |terminal, _| {
-            terminal.input(text.into());
+            terminal.input(text);
         });
 
         self.workspace

crates/terminal_view/src/terminal_panel.rs

@@ -1133,7 +1133,7 @@ async fn wait_for_terminals_tasks(
             })
             .ok()
     });
-    let _: Vec<_> = join_all(pending_tasks).await;
+    join_all(pending_tasks).await;
 }
 
 fn add_paths_to_terminal(

crates/util/src/size.rs

@@ -0,0 +1,48 @@
+pub fn format_file_size(size: u64, use_decimal: bool) -> String {
+    if use_decimal {
+        if size < 1000 {
+            format!("{size}B")
+        } else if size < 1000 * 1000 {
+            format!("{:.1}KB", size as f64 / 1000.0)
+        } else {
+            format!("{:.1}MB", size as f64 / (1000.0 * 1000.0))
+        }
+    } else {
+        if size < 1024 {
+            format!("{size}B")
+        } else if size < 1024 * 1024 {
+            format!("{:.1}KiB", size as f64 / 1024.0)
+        } else {
+            format!("{:.1}MiB", size as f64 / (1024.0 * 1024.0))
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_format_file_size_decimal() {
+        assert_eq!(format_file_size(0, true), "0B");
+        assert_eq!(format_file_size(999, true), "999B");
+        assert_eq!(format_file_size(1000, true), "1.0KB");
+        assert_eq!(format_file_size(1500, true), "1.5KB");
+        assert_eq!(format_file_size(999999, true), "1000.0KB");
+        assert_eq!(format_file_size(1000000, true), "1.0MB");
+        assert_eq!(format_file_size(1500000, true), "1.5MB");
+        assert_eq!(format_file_size(10000000, true), "10.0MB");
+    }
+
+    #[test]
+    fn test_format_file_size_binary() {
+        assert_eq!(format_file_size(0, false), "0B");
+        assert_eq!(format_file_size(1023, false), "1023B");
+        assert_eq!(format_file_size(1024, false), "1.0KiB");
+        assert_eq!(format_file_size(1536, false), "1.5KiB");
+        assert_eq!(format_file_size(1048575, false), "1024.0KiB");
+        assert_eq!(format_file_size(1048576, false), "1.0MiB");
+        assert_eq!(format_file_size(1572864, false), "1.5MiB");
+        assert_eq!(format_file_size(10485760, false), "10.0MiB");
+    }
+}

crates/util/src/time.rs

@@ -0,0 +1,41 @@
+use std::time::Duration;
+
+pub fn duration_alt_display(duration: Duration) -> String {
+    if duration < Duration::from_secs(60) {
+        format!("{}s", duration.as_secs())
+    } else {
+        duration_clock_format(duration)
+    }
+}
+
+fn duration_clock_format(duration: Duration) -> String {
+    let hours = duration.as_secs() / 3600;
+    let minutes = (duration.as_secs() % 3600) / 60;
+    let seconds = duration.as_secs() % 60;
+
+    if hours > 0 {
+        format!("{hours}:{minutes:02}:{seconds:02}")
+    } else if minutes > 0 {
+        format!("{minutes}:{seconds:02}")
+    } else {
+        format!("{seconds}")
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_duration_to_clock_format() {
+        use duration_clock_format as f;
+        assert_eq!("0", f(Duration::from_secs(0)));
+        assert_eq!("59", f(Duration::from_secs(59)));
+        assert_eq!("1:00", f(Duration::from_secs(60)));
+        assert_eq!("10:00", f(Duration::from_secs(600)));
+        assert_eq!("1:00:00", f(Duration::from_secs(3600)));
+        assert_eq!("3:02:01", f(Duration::from_secs(3600 * 3 + 60 * 2 + 1)));
+        assert_eq!("23:59:59", f(Duration::from_secs(3600 * 24 - 1)));
+        assert_eq!("100:00:00", f(Duration::from_secs(3600 * 100)));
+    }
+}

crates/util/src/util.rs

@@ -4,8 +4,10 @@ pub mod fs;
 pub mod markdown;
 pub mod paths;
 pub mod serde;
+pub mod size;
 #[cfg(any(test, feature = "test-support"))]
 pub mod test;
+pub mod time;
 
 use anyhow::Result;
 use futures::Future;