diff --git a/Cargo.lock b/Cargo.lock index 4bb36fdeee..634bacd0f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -27,6 +27,7 @@ dependencies = [ "settings", "smol", "tempfile", + "terminal", "ui", "util", "workspace-hack", @@ -195,6 +196,7 @@ dependencies = [ "cloud_llm_client", "collections", "ctor", + "editor", "env_logger 0.11.8", "fs", "futures 0.3.31", @@ -209,6 +211,7 @@ dependencies = [ "log", "lsp", "paths", + "portable-pty", "pretty_assertions", "project", "prompt_store", @@ -219,12 +222,17 @@ dependencies = [ "serde_json", "settings", "smol", + "task", + "terminal", + "theme", "ui", "util", "uuid", "watch", + "which 6.0.3", "workspace-hack", "worktree", + "zlog", ] [[package]] diff --git a/crates/acp_thread/Cargo.toml b/crates/acp_thread/Cargo.toml index 37d2920045..33e88df761 100644 --- a/crates/acp_thread/Cargo.toml +++ b/crates/acp_thread/Cargo.toml @@ -32,6 +32,7 @@ serde.workspace = true serde_json.workspace = true settings.workspace = true smol.workspace = true +terminal.workspace = true ui.workspace = true util.workspace = true workspace-hack.workspace = true diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index f2bebf7391..d632e6e570 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -1,8 +1,10 @@ mod connection; mod diff; +mod terminal; pub use connection::*; pub use diff::*; +pub use terminal::*; use action_log::ActionLog; use agent_client_protocol as acp; @@ -147,6 +149,14 @@ impl AgentThreadEntry { } } + pub fn terminals(&self) -> impl Iterator> { + if let AgentThreadEntry::ToolCall(call) = self { + itertools::Either::Left(call.terminals()) + } else { + itertools::Either::Right(std::iter::empty()) + } + } + pub fn locations(&self) -> Option<&[acp::ToolCallLocation]> { if let AgentThreadEntry::ToolCall(ToolCall { locations, .. }) = self { Some(locations) @@ -250,8 +260,17 @@ impl ToolCall { pub fn diffs(&self) -> impl Iterator> { self.content.iter().filter_map(|content| match content { - ToolCallContent::ContentBlock { .. } => None, - ToolCallContent::Diff { diff } => Some(diff), + ToolCallContent::Diff(diff) => Some(diff), + ToolCallContent::ContentBlock(_) => None, + ToolCallContent::Terminal(_) => None, + }) + } + + pub fn terminals(&self) -> impl Iterator> { + self.content.iter().filter_map(|content| match content { + ToolCallContent::Terminal(terminal) => Some(terminal), + ToolCallContent::ContentBlock(_) => None, + ToolCallContent::Diff(_) => None, }) } @@ -387,8 +406,9 @@ impl ContentBlock { #[derive(Debug)] pub enum ToolCallContent { - ContentBlock { content: ContentBlock }, - Diff { diff: Entity }, + ContentBlock(ContentBlock), + Diff(Entity), + Terminal(Entity), } impl ToolCallContent { @@ -398,19 +418,20 @@ impl ToolCallContent { cx: &mut App, ) -> Self { match content { - acp::ToolCallContent::Content { content } => Self::ContentBlock { - content: ContentBlock::new(content, &language_registry, cx), - }, - acp::ToolCallContent::Diff { diff } => Self::Diff { - diff: cx.new(|cx| Diff::from_acp(diff, language_registry, cx)), - }, + acp::ToolCallContent::Content { content } => { + Self::ContentBlock(ContentBlock::new(content, &language_registry, cx)) + } + acp::ToolCallContent::Diff { diff } => { + Self::Diff(cx.new(|cx| Diff::from_acp(diff, language_registry, cx))) + } } } pub fn to_markdown(&self, cx: &App) -> String { match self { - Self::ContentBlock { content } => content.to_markdown(cx).to_string(), - Self::Diff { diff } => diff.read(cx).to_markdown(cx), + Self::ContentBlock(content) => content.to_markdown(cx).to_string(), + Self::Diff(diff) => diff.read(cx).to_markdown(cx), + Self::Terminal(terminal) => terminal.read(cx).to_markdown(cx), } } } @@ -419,6 +440,7 @@ impl ToolCallContent { pub enum ToolCallUpdate { UpdateFields(acp::ToolCallUpdate), UpdateDiff(ToolCallUpdateDiff), + UpdateTerminal(ToolCallUpdateTerminal), } impl ToolCallUpdate { @@ -426,6 +448,7 @@ impl ToolCallUpdate { match self { Self::UpdateFields(update) => &update.id, Self::UpdateDiff(diff) => &diff.id, + Self::UpdateTerminal(terminal) => &terminal.id, } } } @@ -448,6 +471,18 @@ pub struct ToolCallUpdateDiff { pub diff: Entity, } +impl From for ToolCallUpdate { + fn from(terminal: ToolCallUpdateTerminal) -> Self { + Self::UpdateTerminal(terminal) + } +} + +#[derive(Debug, PartialEq)] +pub struct ToolCallUpdateTerminal { + pub id: acp::ToolCallId, + pub terminal: Entity, +} + #[derive(Debug, Default)] pub struct Plan { pub entries: Vec, @@ -760,7 +795,13 @@ impl AcpThread { current_call.content.clear(); current_call .content - .push(ToolCallContent::Diff { diff: update.diff }); + .push(ToolCallContent::Diff(update.diff)); + } + ToolCallUpdate::UpdateTerminal(update) => { + current_call.content.clear(); + current_call + .content + .push(ToolCallContent::Terminal(update.terminal)); } } diff --git a/crates/acp_thread/src/terminal.rs b/crates/acp_thread/src/terminal.rs new file mode 100644 index 0000000000..b800873737 --- /dev/null +++ b/crates/acp_thread/src/terminal.rs @@ -0,0 +1,87 @@ +use gpui::{App, AppContext, Context, Entity}; +use language::LanguageRegistry; +use markdown::Markdown; +use std::{path::PathBuf, process::ExitStatus, sync::Arc, time::Instant}; + +pub struct Terminal { + command: Entity, + working_dir: Option, + terminal: Entity, + started_at: Instant, + output: Option, +} + +pub struct TerminalOutput { + pub ended_at: Instant, + pub exit_status: Option, + pub was_content_truncated: bool, + pub original_content_len: usize, + pub content_line_count: usize, + pub finished_with_empty_output: bool, +} + +impl Terminal { + pub fn new( + command: String, + working_dir: Option, + terminal: Entity, + language_registry: Arc, + cx: &mut Context, + ) -> Self { + Self { + command: cx + .new(|cx| Markdown::new(command.into(), Some(language_registry.clone()), None, cx)), + working_dir, + terminal, + started_at: Instant::now(), + output: None, + } + } + + pub fn finish( + &mut self, + exit_status: Option, + original_content_len: usize, + truncated_content_len: usize, + content_line_count: usize, + finished_with_empty_output: bool, + cx: &mut Context, + ) { + self.output = Some(TerminalOutput { + ended_at: Instant::now(), + exit_status, + was_content_truncated: truncated_content_len < original_content_len, + original_content_len, + content_line_count, + finished_with_empty_output, + }); + cx.notify(); + } + + pub fn command(&self) -> &Entity { + &self.command + } + + pub fn working_dir(&self) -> &Option { + &self.working_dir + } + + pub fn started_at(&self) -> Instant { + self.started_at + } + + pub fn output(&self) -> Option<&TerminalOutput> { + self.output.as_ref() + } + + pub fn inner(&self) -> &Entity { + &self.terminal + } + + pub fn to_markdown(&self, cx: &App) -> String { + format!( + "Terminal:\n```\n{}\n```\n", + self.terminal.read(cx).get_content() + ) + } +} diff --git a/crates/agent2/Cargo.toml b/crates/agent2/Cargo.toml index c1c3f2d459..65452f60fc 100644 --- a/crates/agent2/Cargo.toml +++ b/crates/agent2/Cargo.toml @@ -33,6 +33,7 @@ language_model.workspace = true language_models.workspace = true log.workspace = true paths.workspace = true +portable-pty.workspace = true project.workspace = true prompt_store.workspace = true rust-embed.workspace = true @@ -41,16 +42,20 @@ serde.workspace = true serde_json.workspace = true settings.workspace = true smol.workspace = true +task.workspace = true +terminal.workspace = true ui.workspace = true util.workspace = true uuid.workspace = true watch.workspace = true +which.workspace = true workspace-hack.workspace = true [dev-dependencies] ctor.workspace = true client = { workspace = true, "features" = ["test-support"] } clock = { workspace = true, "features" = ["test-support"] } +editor = { workspace = true, "features" = ["test-support"] } env_logger.workspace = true fs = { workspace = true, "features" = ["test-support"] } gpui = { workspace = true, "features" = ["test-support"] } @@ -58,8 +63,11 @@ gpui_tokio.workspace = true language = { workspace = true, "features" = ["test-support"] } language_model = { workspace = true, "features" = ["test-support"] } lsp = { workspace = true, "features" = ["test-support"] } +pretty_assertions.workspace = true project = { workspace = true, "features" = ["test-support"] } reqwest_client.workspace = true settings = { workspace = true, "features" = ["test-support"] } +terminal = { workspace = true, "features" = ["test-support"] } +theme = { workspace = true, "features" = ["test-support"] } worktree = { workspace = true, "features" = ["test-support"] } -pretty_assertions.workspace = true +zlog.workspace = true diff --git a/crates/agent2/src/agent.rs b/crates/agent2/src/agent.rs index 5be3892d60..edb79003b4 100644 --- a/crates/agent2/src/agent.rs +++ b/crates/agent2/src/agent.rs @@ -1,5 +1,7 @@ use crate::{AgentResponseEvent, Thread, templates::Templates}; -use crate::{EditFileTool, FindPathTool, ReadFileTool, ThinkingTool, ToolCallAuthorization}; +use crate::{ + EditFileTool, FindPathTool, ReadFileTool, TerminalTool, ThinkingTool, ToolCallAuthorization, +}; use acp_thread::ModelSelector; use agent_client_protocol as acp; use anyhow::{Context as _, Result, anyhow}; @@ -418,6 +420,7 @@ impl acp_thread::AgentConnection for NativeAgentConnection { thread.add_tool(FindPathTool::new(project.clone())); thread.add_tool(ReadFileTool::new(project.clone(), action_log)); thread.add_tool(EditFileTool::new(cx.entity())); + thread.add_tool(TerminalTool::new(project.clone(), cx)); thread }); diff --git a/crates/agent2/src/thread.rs b/crates/agent2/src/thread.rs index a0a2a3a2b0..dd8e5476ab 100644 --- a/crates/agent2/src/thread.rs +++ b/crates/agent2/src/thread.rs @@ -1,5 +1,4 @@ use crate::{SystemPromptTemplate, Template, Templates}; -use acp_thread::Diff; use action_log::ActionLog; use agent_client_protocol as acp; use anyhow::{Context as _, Result, anyhow}; @@ -802,47 +801,6 @@ impl AgentResponseEventStream { .ok(); } - fn authorize_tool_call( - &self, - id: &LanguageModelToolUseId, - title: String, - kind: acp::ToolKind, - input: serde_json::Value, - ) -> impl use<> + Future> { - let (response_tx, response_rx) = oneshot::channel(); - self.0 - .unbounded_send(Ok(AgentResponseEvent::ToolCallAuthorization( - ToolCallAuthorization { - tool_call: Self::initial_tool_call(id, title, kind, input), - options: vec![ - acp::PermissionOption { - id: acp::PermissionOptionId("always_allow".into()), - name: "Always Allow".into(), - kind: acp::PermissionOptionKind::AllowAlways, - }, - acp::PermissionOption { - id: acp::PermissionOptionId("allow".into()), - name: "Allow".into(), - kind: acp::PermissionOptionKind::AllowOnce, - }, - acp::PermissionOption { - id: acp::PermissionOptionId("deny".into()), - name: "Deny".into(), - kind: acp::PermissionOptionKind::RejectOnce, - }, - ], - response: response_tx, - }, - ))) - .ok(); - async move { - match response_rx.await?.0.as_ref() { - "allow" | "always_allow" => Ok(()), - _ => Err(anyhow!("Permission to run tool denied by user")), - } - } - } - fn send_tool_call( &self, id: &LanguageModelToolUseId, @@ -894,18 +852,6 @@ impl AgentResponseEventStream { .ok(); } - fn update_tool_call_diff(&self, tool_use_id: &LanguageModelToolUseId, diff: Entity) { - self.0 - .unbounded_send(Ok(AgentResponseEvent::ToolCallUpdate( - acp_thread::ToolCallUpdateDiff { - id: acp::ToolCallId(tool_use_id.to_string().into()), - diff, - } - .into(), - ))) - .ok(); - } - fn send_stop(&self, reason: StopReason) { match reason { StopReason::EndTurn => { @@ -979,17 +925,71 @@ impl ToolCallEventStream { .update_tool_call_fields(&self.tool_use_id, fields); } - pub fn update_diff(&self, diff: Entity) { - self.stream.update_tool_call_diff(&self.tool_use_id, diff); + pub fn update_diff(&self, diff: Entity) { + self.stream + .0 + .unbounded_send(Ok(AgentResponseEvent::ToolCallUpdate( + acp_thread::ToolCallUpdateDiff { + id: acp::ToolCallId(self.tool_use_id.to_string().into()), + diff, + } + .into(), + ))) + .ok(); + } + + pub fn update_terminal(&self, terminal: Entity) { + self.stream + .0 + .unbounded_send(Ok(AgentResponseEvent::ToolCallUpdate( + acp_thread::ToolCallUpdateTerminal { + id: acp::ToolCallId(self.tool_use_id.to_string().into()), + terminal, + } + .into(), + ))) + .ok(); } pub fn authorize(&self, title: String) -> impl use<> + Future> { - self.stream.authorize_tool_call( - &self.tool_use_id, - title, - self.kind.clone(), - self.input.clone(), - ) + let (response_tx, response_rx) = oneshot::channel(); + self.stream + .0 + .unbounded_send(Ok(AgentResponseEvent::ToolCallAuthorization( + ToolCallAuthorization { + tool_call: AgentResponseEventStream::initial_tool_call( + &self.tool_use_id, + title, + self.kind.clone(), + self.input.clone(), + ), + options: vec![ + acp::PermissionOption { + id: acp::PermissionOptionId("always_allow".into()), + name: "Always Allow".into(), + kind: acp::PermissionOptionKind::AllowAlways, + }, + acp::PermissionOption { + id: acp::PermissionOptionId("allow".into()), + name: "Allow".into(), + kind: acp::PermissionOptionKind::AllowOnce, + }, + acp::PermissionOption { + id: acp::PermissionOptionId("deny".into()), + name: "Deny".into(), + kind: acp::PermissionOptionKind::RejectOnce, + }, + ], + response: response_tx, + }, + ))) + .ok(); + async move { + match response_rx.await?.0.as_ref() { + "allow" | "always_allow" => Ok(()), + _ => Err(anyhow!("Permission to run tool denied by user")), + } + } } } @@ -1000,7 +1000,7 @@ pub struct ToolCallEventStreamReceiver( #[cfg(test)] impl ToolCallEventStreamReceiver { - pub async fn expect_tool_authorization(&mut self) -> ToolCallAuthorization { + pub async fn expect_authorization(&mut self) -> ToolCallAuthorization { let event = self.0.next().await; if let Some(Ok(AgentResponseEvent::ToolCallAuthorization(auth))) = event { auth @@ -1008,6 +1008,18 @@ impl ToolCallEventStreamReceiver { panic!("Expected ToolCallAuthorization but got: {:?}", event); } } + + pub async fn expect_terminal(&mut self) -> Entity { + let event = self.0.next().await; + if let Some(Ok(AgentResponseEvent::ToolCallUpdate( + acp_thread::ToolCallUpdate::UpdateTerminal(update), + ))) = event + { + update.terminal + } else { + panic!("Expected terminal but got: {:?}", event); + } + } } #[cfg(test)] diff --git a/crates/agent2/src/tools.rs b/crates/agent2/src/tools.rs index 5fe13db854..df4a7a9580 100644 --- a/crates/agent2/src/tools.rs +++ b/crates/agent2/src/tools.rs @@ -1,9 +1,11 @@ mod edit_file_tool; mod find_path_tool; mod read_file_tool; +mod terminal_tool; mod thinking_tool; pub use edit_file_tool::*; pub use find_path_tool::*; pub use read_file_tool::*; +pub use terminal_tool::*; pub use thinking_tool::*; diff --git a/crates/agent2/src/tools/edit_file_tool.rs b/crates/agent2/src/tools/edit_file_tool.rs index 48e5d37586..d9a4cdf8ba 100644 --- a/crates/agent2/src/tools/edit_file_tool.rs +++ b/crates/agent2/src/tools/edit_file_tool.rs @@ -942,7 +942,7 @@ mod tests { ) }); - let event = stream_rx.expect_tool_authorization().await; + let event = stream_rx.expect_authorization().await; assert_eq!(event.tool_call.title, "test 1 (local settings)"); // Test 2: Path outside project should require confirmation @@ -959,7 +959,7 @@ mod tests { ) }); - let event = stream_rx.expect_tool_authorization().await; + let event = stream_rx.expect_authorization().await; assert_eq!(event.tool_call.title, "test 2"); // Test 3: Relative path without .zed should not require confirmation @@ -992,7 +992,7 @@ mod tests { cx, ) }); - let event = stream_rx.expect_tool_authorization().await; + let event = stream_rx.expect_authorization().await; assert_eq!(event.tool_call.title, "test 4 (local settings)"); // Test 5: When always_allow_tool_actions is enabled, no confirmation needed @@ -1088,7 +1088,7 @@ mod tests { }); if should_confirm { - stream_rx.expect_tool_authorization().await; + stream_rx.expect_authorization().await; } else { auth.await.unwrap(); assert!( @@ -1192,7 +1192,7 @@ mod tests { }); if should_confirm { - stream_rx.expect_tool_authorization().await; + stream_rx.expect_authorization().await; } else { auth.await.unwrap(); assert!( @@ -1276,7 +1276,7 @@ mod tests { }); if should_confirm { - stream_rx.expect_tool_authorization().await; + stream_rx.expect_authorization().await; } else { auth.await.unwrap(); assert!( @@ -1339,7 +1339,7 @@ mod tests { ) }); - stream_rx.expect_tool_authorization().await; + stream_rx.expect_authorization().await; // Test outside path with different modes let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); @@ -1355,7 +1355,7 @@ mod tests { ) }); - stream_rx.expect_tool_authorization().await; + stream_rx.expect_authorization().await; // Test normal path with different modes let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); diff --git a/crates/agent2/src/tools/terminal_tool.rs b/crates/agent2/src/tools/terminal_tool.rs new file mode 100644 index 0000000000..c0b34444dd --- /dev/null +++ b/crates/agent2/src/tools/terminal_tool.rs @@ -0,0 +1,489 @@ +use agent_client_protocol as acp; +use anyhow::Result; +use futures::{FutureExt as _, future::Shared}; +use gpui::{App, AppContext, Entity, SharedString, Task}; +use project::{Project, terminals::TerminalKind}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use settings::Settings; +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; +use util::{ResultExt, get_system_shell, markdown::MarkdownInlineCode}; + +use crate::{AgentTool, ToolCallEventStream}; + +const COMMAND_OUTPUT_LIMIT: usize = 16 * 1024; + +/// Executes a shell one-liner and returns the combined output. +/// +/// 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. +/// +/// Do not use this tool for commands that run indefinitely, such as servers (like `npm run start`, `npm run dev`, `python -m http.server`, etc) or file watchers that don't terminate on their own. +/// +/// Remember that each invocation of this tool will spawn a new shell process, so you can't rely on any state from previous invocations. +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct TerminalToolInput { + /// The one-liner command to execute. + command: String, + /// Working directory for the command. This must be one of the root directories of the project. + cd: String, +} + +pub struct TerminalTool { + project: Entity, + determine_shell: Shared>, +} + +impl TerminalTool { + pub fn new(project: Entity, cx: &mut App) -> Self { + let determine_shell = cx.background_spawn(async move { + if cfg!(windows) { + return get_system_shell(); + } + + if which::which("bash").is_ok() { + log::info!("agent selected bash for terminal tool"); + "bash".into() + } else { + let shell = get_system_shell(); + log::info!("agent selected {shell} for terminal tool"); + shell + } + }); + Self { + project, + determine_shell: determine_shell.shared(), + } + } + + fn authorize( + &self, + input: &TerminalToolInput, + event_stream: &ToolCallEventStream, + cx: &App, + ) -> Task> { + if agent_settings::AgentSettings::get_global(cx).always_allow_tool_actions { + return Task::ready(Ok(())); + } + + // TODO: do we want to have a special title here? + cx.foreground_executor() + .spawn(event_stream.authorize(self.initial_title(Ok(input.clone())).to_string())) + } +} + +impl AgentTool for TerminalTool { + type Input = TerminalToolInput; + type Output = String; + + fn name(&self) -> SharedString { + "terminal".into() + } + + fn kind(&self) -> acp::ToolKind { + acp::ToolKind::Execute + } + + fn initial_title(&self, input: Result) -> SharedString { + if let Ok(input) = input { + let mut lines = input.command.lines(); + let first_line = lines.next().unwrap_or_default(); + let remaining_line_count = lines.count(); + match remaining_line_count { + 0 => MarkdownInlineCode(&first_line).to_string().into(), + 1 => MarkdownInlineCode(&format!( + "{} - {} more line", + first_line, remaining_line_count + )) + .to_string() + .into(), + n => MarkdownInlineCode(&format!("{} - {} more lines", first_line, n)) + .to_string() + .into(), + } + } else { + "Run terminal command".into() + } + } + + fn run( + self: Arc, + input: Self::Input, + event_stream: ToolCallEventStream, + cx: &mut App, + ) -> Task> { + let language_registry = self.project.read(cx).languages().clone(); + let working_dir = match working_dir(&input, &self.project, cx) { + Ok(dir) => dir, + Err(err) => return Task::ready(Err(err)), + }; + let program = self.determine_shell.clone(); + let command = if cfg!(windows) { + format!("$null | & {{{}}}", input.command.replace("\"", "'")) + } else if let Some(cwd) = working_dir + .as_ref() + .and_then(|cwd| cwd.as_os_str().to_str()) + { + // Make sure once we're *inside* the shell, we cd into `cwd` + format!("(cd {cwd}; {}) self.project.update(cx, |project, cx| { + project.directory_environment(dir.as_path().into(), cx) + }), + None => Task::ready(None).shared(), + }; + + let env = cx.spawn(async move |_| { + let mut env = env.await.unwrap_or_default(); + if cfg!(unix) { + env.insert("PAGER".into(), "cat".into()); + } + env + }); + + let authorize = self.authorize(&input, &event_stream, cx); + + cx.spawn({ + async move |cx| { + authorize.await?; + + let program = program.await; + let env = env.await; + let terminal = self + .project + .update(cx, |project, cx| { + project.create_terminal( + TerminalKind::Task(task::SpawnInTerminal { + command: Some(program), + args, + cwd: working_dir.clone(), + env, + ..Default::default() + }), + cx, + ) + })? + .await?; + let acp_terminal = cx.new(|cx| { + acp_thread::Terminal::new( + input.command.clone(), + working_dir.clone(), + terminal.clone(), + language_registry, + cx, + ) + })?; + event_stream.update_terminal(acp_terminal.clone()); + + let exit_status = terminal + .update(cx, |terminal, cx| terminal.wait_for_completed_task(cx))? + .await; + let (content, content_line_count) = terminal.read_with(cx, |terminal, _| { + (terminal.get_content(), terminal.total_lines()) + })?; + + let (processed_content, finished_with_empty_output) = process_content( + &content, + &input.command, + exit_status.map(portable_pty::ExitStatus::from), + ); + + acp_terminal + .update(cx, |terminal, cx| { + terminal.finish( + exit_status, + content.len(), + processed_content.len(), + content_line_count, + finished_with_empty_output, + cx, + ); + }) + .log_err(); + + Ok(processed_content) + } + }) + } +} + +fn process_content( + content: &str, + command: &str, + exit_status: Option, +) -> (String, bool) { + let should_truncate = content.len() > COMMAND_OUTPUT_LIMIT; + + 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; + } + // Don't truncate mid-line, clear the remainder of the last line + end_ix = content[..end_ix].rfind('\n').unwrap_or(end_ix); + &content[..end_ix] + } else { + content + }; + let content = content.trim(); + let is_empty = content.is_empty(); + let content = format!("```\n{content}\n```"); + let content = if should_truncate { + format!( + "Command output too long. The first {} bytes:\n\n{content}", + content.len(), + ) + } else { + content + }; + + let content = match exit_status { + Some(exit_status) if exit_status.success() => { + if is_empty { + "Command executed successfully.".to_string() + } else { + content.to_string() + } + } + Some(exit_status) => { + if is_empty { + format!( + "Command \"{command}\" failed with exit code {}.", + exit_status.exit_code() + ) + } else { + format!( + "Command \"{command}\" failed with exit code {}.\n\n{content}", + exit_status.exit_code() + ) + } + } + None => { + format!( + "Command failed or was interrupted.\nPartial output captured:\n\n{}", + content, + ) + } + }; + (content, is_empty) +} + +fn working_dir( + input: &TerminalToolInput, + project: &Entity, + cx: &mut App, +) -> Result> { + let project = project.read(cx); + let cd = &input.cd; + + if cd == "." || cd == "" { + // Accept "." or "" as meaning "the one worktree" if we only have one worktree. + let mut worktrees = project.worktrees(cx); + + match worktrees.next() { + Some(worktree) => { + anyhow::ensure!( + worktrees.next().is_none(), + "'.' 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 { + let input_path = Path::new(cd); + + 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 Ok(Some(input_path.into())); + } + } else { + if let Some(worktree) = project.worktree_for_root_name(cd, cx) { + return Ok(Some(worktree.read(cx).abs_path().to_path_buf())); + } + } + + anyhow::bail!("`cd` directory {cd:?} was not in any of the project's worktrees."); + } +} + +#[cfg(test)] +mod tests { + use agent_settings::AgentSettings; + use editor::EditorSettings; + use fs::RealFs; + use gpui::{BackgroundExecutor, TestAppContext}; + use pretty_assertions::assert_eq; + use serde_json::json; + use settings::{Settings, SettingsStore}; + use terminal::terminal_settings::TerminalSettings; + use theme::ThemeSettings; + use util::test::TempTree; + + use crate::AgentResponseEvent; + + use super::*; + + fn init_test(executor: &BackgroundExecutor, cx: &mut TestAppContext) { + zlog::init_test(); + + executor.allow_parking(); + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + language::init(cx); + Project::init_settings(cx); + ThemeSettings::register(cx); + TerminalSettings::register(cx); + EditorSettings::register(cx); + AgentSettings::register(cx); + }); + } + + #[gpui::test] + async fn test_interactive_command(executor: BackgroundExecutor, cx: &mut TestAppContext) { + if cfg!(windows) { + return; + } + + init_test(&executor, cx); + + let fs = Arc::new(RealFs::new(None, executor)); + let tree = TempTree::new(json!({ + "project": {}, + })); + let project: Entity = + Project::test(fs, [tree.path().join("project").as_path()], cx).await; + + let input = TerminalToolInput { + command: "cat".to_owned(), + cd: tree + .path() + .join("project") + .as_path() + .to_string_lossy() + .to_string(), + }; + let (event_stream_tx, mut event_stream_rx) = ToolCallEventStream::test(); + let result = cx + .update(|cx| Arc::new(TerminalTool::new(project, cx)).run(input, event_stream_tx, cx)); + + let auth = event_stream_rx.expect_authorization().await; + auth.response.send(auth.options[0].id.clone()).unwrap(); + event_stream_rx.expect_terminal().await; + assert_eq!(result.await.unwrap(), "Command executed successfully."); + } + + #[gpui::test] + async fn test_working_directory(executor: BackgroundExecutor, cx: &mut TestAppContext) { + if cfg!(windows) { + return; + } + + init_test(&executor, cx); + + let fs = Arc::new(RealFs::new(None, executor)); + let tree = TempTree::new(json!({ + "project": {}, + "other-project": {}, + })); + let project: Entity = + Project::test(fs, [tree.path().join("project").as_path()], cx).await; + + let check = |input, expected, cx: &mut TestAppContext| { + let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); + let result = cx.update(|cx| { + Arc::new(TerminalTool::new(project.clone(), cx)).run(input, stream_tx, cx) + }); + cx.run_until_parked(); + let event = stream_rx.try_next(); + if let Ok(Some(Ok(AgentResponseEvent::ToolCallAuthorization(auth)))) = event { + auth.response.send(auth.options[0].id.clone()).unwrap(); + } + + cx.spawn(async move |_| { + let output = result.await; + assert_eq!(output.ok(), expected); + }) + }; + + check( + TerminalToolInput { + command: "pwd".into(), + cd: ".".into(), + }, + Some(format!( + "```\n{}\n```", + tree.path().join("project").display() + )), + cx, + ) + .await; + + check( + TerminalToolInput { + command: "pwd".into(), + cd: "other-project".into(), + }, + None, // other-project is a dir, but *not* a worktree (yet) + cx, + ) + .await; + + // Absolute path above the worktree root + check( + TerminalToolInput { + command: "pwd".into(), + cd: tree.path().to_string_lossy().into(), + }, + None, + cx, + ) + .await; + + project + .update(cx, |project, cx| { + project.create_worktree(tree.path().join("other-project"), true, cx) + }) + .await + .unwrap(); + + check( + TerminalToolInput { + command: "pwd".into(), + cd: "other-project".into(), + }, + Some(format!( + "```\n{}\n```", + tree.path().join("other-project").display() + )), + cx, + ) + .await; + + check( + TerminalToolInput { + command: "pwd".into(), + cd: ".".into(), + }, + None, + cx, + ) + .await; + } +} diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 01980b8fb7..2536612ece 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -1,17 +1,13 @@ +use acp_thread::{ + AcpThread, AcpThreadEvent, AgentThreadEntry, AssistantMessage, AssistantMessageChunk, + LoadError, MentionPath, ThreadStatus, ToolCall, ToolCallContent, ToolCallStatus, +}; use acp_thread::{AgentConnection, Plan}; +use action_log::ActionLog; +use agent_client_protocol as acp; use agent_servers::AgentServer; use agent_settings::{AgentSettings, NotifyWhenAgentWaiting}; use audio::{Audio, Sound}; -use std::cell::RefCell; -use std::collections::BTreeMap; -use std::path::Path; -use std::process::ExitStatus; -use std::rc::Rc; -use std::sync::Arc; -use std::time::Duration; - -use action_log::ActionLog; -use agent_client_protocol as acp; use buffer_diff::BufferDiff; use collections::{HashMap, HashSet}; use editor::{ @@ -32,6 +28,11 @@ use markdown::{HeadingLevelStyles, Markdown, MarkdownElement, MarkdownStyle}; use parking_lot::Mutex; use project::{CompletionIntent, Project}; use settings::{Settings as _, SettingsStore}; +use std::{ + cell::RefCell, collections::BTreeMap, path::Path, process::ExitStatus, rc::Rc, sync::Arc, + time::Duration, +}; +use terminal_view::TerminalView; use text::{Anchor, BufferSnapshot}; use theme::ThemeSettings; use ui::{ @@ -41,11 +42,6 @@ use util::ResultExt; use workspace::{CollaboratorId, Workspace}; use zed_actions::agent::{Chat, NextHistoryMessage, PreviousHistoryMessage}; -use ::acp_thread::{ - AcpThread, AcpThreadEvent, AgentThreadEntry, AssistantMessage, AssistantMessageChunk, - LoadError, MentionPath, ThreadStatus, ToolCall, ToolCallContent, ToolCallStatus, -}; - use crate::acp::completion_provider::{ContextPickerCompletionProvider, MentionSet}; use crate::acp::message_history::MessageHistory; use crate::agent_diff::AgentDiff; @@ -63,6 +59,7 @@ pub struct AcpThreadView { project: Entity, thread_state: ThreadState, diff_editors: HashMap>, + terminal_views: HashMap>, message_editor: Entity, message_set_from_history: Option, _message_editor_subscription: Subscription, @@ -193,6 +190,7 @@ impl AcpThreadView { notifications: Vec::new(), notification_subscriptions: HashMap::default(), diff_editors: Default::default(), + terminal_views: Default::default(), list_state: list_state.clone(), scrollbar_state: ScrollbarState::new(list_state).parent_entity(&cx.entity()), last_error: None, @@ -676,6 +674,16 @@ impl AcpThreadView { entry_ix: usize, window: &mut Window, cx: &mut Context, + ) { + self.sync_diff_multibuffers(entry_ix, window, cx); + self.sync_terminals(entry_ix, window, cx); + } + + fn sync_diff_multibuffers( + &mut self, + entry_ix: usize, + window: &mut Window, + cx: &mut Context, ) { let Some(multibuffers) = self.entry_diff_multibuffers(entry_ix, cx) else { return; @@ -739,6 +747,50 @@ impl AcpThreadView { ) } + fn sync_terminals(&mut self, entry_ix: usize, window: &mut Window, cx: &mut Context) { + let Some(terminals) = self.entry_terminals(entry_ix, cx) else { + return; + }; + + let terminals = terminals.collect::>(); + + for terminal in terminals { + if self.terminal_views.contains_key(&terminal.entity_id()) { + return; + } + + let terminal_view = cx.new(|cx| { + let mut view = TerminalView::new( + terminal.read(cx).inner().clone(), + self.workspace.clone(), + None, + self.project.downgrade(), + window, + cx, + ); + view.set_embedded_mode(None, cx); + view + }); + + let entity_id = terminal.entity_id(); + cx.observe_release(&terminal, move |this, _, _| { + this.terminal_views.remove(&entity_id); + }) + .detach(); + + self.terminal_views.insert(entity_id, terminal_view); + } + } + + fn entry_terminals( + &self, + entry_ix: usize, + cx: &App, + ) -> Option>> { + let entry = self.thread()?.read(cx).entries().get(entry_ix)?; + Some(entry.terminals().map(|terminal| terminal.clone())) + } + fn authenticate( &mut self, method: acp::AuthMethodId, @@ -1106,7 +1158,7 @@ impl AcpThreadView { _ => tool_call .content .iter() - .any(|content| matches!(content, ToolCallContent::Diff { .. })), + .any(|content| matches!(content, ToolCallContent::Diff(_))), }; let is_collapsible = !tool_call.content.is_empty() && !needs_confirmation; @@ -1303,7 +1355,7 @@ impl AcpThreadView { cx: &Context, ) -> AnyElement { match content { - ToolCallContent::ContentBlock { content } => { + ToolCallContent::ContentBlock(content) => { if let Some(md) = content.markdown() { div() .p_2() @@ -1318,9 +1370,8 @@ impl AcpThreadView { Empty.into_any_element() } } - ToolCallContent::Diff { diff, .. } => { - self.render_diff_editor(&diff.read(cx).multibuffer()) - } + ToolCallContent::Diff(diff) => self.render_diff_editor(&diff.read(cx).multibuffer()), + ToolCallContent::Terminal(terminal) => self.render_terminal(terminal), } } @@ -1389,6 +1440,21 @@ impl AcpThreadView { .into_any() } + fn render_terminal(&self, terminal: &Entity) -> AnyElement { + v_flex() + .h_72() + .child( + if let Some(terminal_view) = self.terminal_views.get(&terminal.entity_id()) { + // TODO: terminal has all the state we need to reproduce + // what we had in the terminal card. + terminal_view.clone().into_any_element() + } else { + Empty.into_any() + }, + ) + .into_any() + } + fn render_agent_logo(&self) -> AnyElement { Icon::new(self.agent.logo()) .color(Color::Muted) diff --git a/crates/terminal/Cargo.toml b/crates/terminal/Cargo.toml index 93f61622c8..b1c0dd693f 100644 --- a/crates/terminal/Cargo.toml +++ b/crates/terminal/Cargo.toml @@ -5,6 +5,13 @@ edition.workspace = true publish.workspace = true license = "GPL-3.0-or-later" +[features] +test-support = [ + "collections/test-support", + "gpui/test-support", + "settings/test-support", +] + [lints] workspace = true @@ -39,5 +46,6 @@ workspace-hack.workspace = true windows.workspace = true [dev-dependencies] +gpui = { workspace = true, features = ["test-support"] } rand.workspace = true url.workspace = true diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index d6a09a590f..3e7d9c0ad4 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -58,7 +58,7 @@ use std::{ path::PathBuf, process::ExitStatus, sync::Arc, - time::{Duration, Instant}, + time::Instant, }; use thiserror::Error; @@ -534,10 +534,15 @@ impl TerminalBuilder { 'outer: loop { let mut events = Vec::new(); + + #[cfg(any(test, feature = "test-support"))] + let mut timer = cx.background_executor().simulate_random_delay().fuse(); + #[cfg(not(any(test, feature = "test-support")))] let mut timer = cx .background_executor() - .timer(Duration::from_millis(4)) + .timer(std::time::Duration::from_millis(4)) .fuse(); + let mut wakeup = false; loop { futures::select_biased! { @@ -2104,16 +2109,56 @@ pub fn rgba_color(r: u8, g: u8, b: u8) -> Hsla { #[cfg(test)] mod tests { + use super::*; + use crate::{ + IndexedCell, TerminalBounds, TerminalBuilder, TerminalContent, content_index_for_mouse, + rgb_for_index, + }; use alacritty_terminal::{ index::{Column, Line, Point as AlacPoint}, term::cell::Cell, }; - use gpui::{Pixels, Point, bounds, point, size}; + use collections::HashMap; + use gpui::{Pixels, Point, TestAppContext, bounds, point, size}; use rand::{Rng, distributions::Alphanumeric, rngs::ThreadRng, thread_rng}; - use crate::{ - IndexedCell, TerminalBounds, TerminalContent, content_index_for_mouse, rgb_for_index, - }; + #[cfg_attr(windows, ignore = "TODO: fix on windows")] + #[gpui::test] + async fn test_basic_terminal(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + + let (completion_tx, completion_rx) = smol::channel::unbounded(); + let terminal = cx.new(|cx| { + TerminalBuilder::new( + None, + None, + None, + task::Shell::WithArguments { + program: "echo".into(), + args: vec!["hello".into()], + title_override: None, + }, + HashMap::default(), + CursorShape::default(), + AlternateScroll::On, + None, + false, + 0, + completion_tx, + cx, + ) + .unwrap() + .subscribe(cx) + }); + assert_eq!( + completion_rx.recv().await.unwrap(), + Some(ExitStatus::default()) + ); + assert_eq!( + terminal.update(cx, |term, _| term.get_content()).trim(), + "hello" + ); + } #[test] fn test_rgb_for_index() {