oak-team/oak-gpui
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

acp_thread: Properly use project terminal API (#38186)

Browse Source 
Closes https://github.com/zed-industries/zed/issues/35603

Release Notes:

- Fixed shell selection for terminal tool
This commit is contained in:
Lukas Wirth
2025-09-15 12:43:41 +00:00
committed by GitHub
parent e9b4f59e0f
commit 92b946e8e5
13 changed files with 148 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Cargo.lock
Generated
-2
View File
@@ -39,7 +39,6 @@ dependencies = [
"util",
"uuid",
"watch",
"which 6.0.3",
"workspace-hack",
]
@@ -1023,7 +1022,6 @@ dependencies = [
"util",
"watch",
"web_search",
"which 6.0.3",
"workspace",
"workspace-hack",
"zlog",
crates/acp_thread/Cargo.toml
-1
View File
@@ -45,7 +45,6 @@ url.workspace = true
util.workspace = true
uuid.workspace = true
watch.workspace = true
which.workspace = true
workspace-hack.workspace = true
[dev-dependencies]
crates/acp_thread/src/acp_thread.rs
+18 -39
View File
@@ -7,12 +7,12 @@ use agent_settings::AgentSettings;
use collections::HashSet;
pub use connection::*;
pub use diff::*;
use futures::future::Shared;
use language::language_settings::FormatOnSave;
pub use mention::*;
use project::lsp_store::{FormatTrigger, LspFormatTarget};
use serde::{Deserialize, Serialize};
use settings::Settings as _;
use task::{Shell, ShellBuilder};
pub use terminal::*;
use action_log::ActionLog;
@@ -34,7 +34,7 @@ use std::rc::Rc;
use std::time::{Duration, Instant};
use std::{fmt::Display, mem, path::PathBuf, sync::Arc};
use ui::App;
use util::{ResultExt, get_system_shell};
use util::{ResultExt, get_default_system_shell};
use uuid::Uuid;
#[derive(Debug)]
@@ -786,7 +786,6 @@ pub struct AcpThread {
token_usage: Option<TokenUsage>,
prompt_capabilities: acp::PromptCapabilities,
_observe_prompt_capabilities: Task<anyhow::Result<()>>,
determine_shell: Shared<Task<String>>,
terminals: HashMap<acp::TerminalId, Entity<Terminal>>,
}
@@ -873,20 +872,6 @@ impl AcpThread {
}
});
let determine_shell = cx
.background_spawn(async move {
if cfg!(windows) {
return get_system_shell();
}
if which::which("bash").is_ok() {
"bash".into()
} else {
get_system_shell()
}
})
.shared();
Self {
action_log,
shared_buffers: Default::default(),
@@ -901,7 +886,6 @@ impl AcpThread {
prompt_capabilities,
_observe_prompt_capabilities: task,
terminals: HashMap::default(),
determine_shell,
}
}
@@ -1940,28 +1924,13 @@ impl AcpThread {
pub fn create_terminal(
&self,
mut command: String,
command: String,
args: Vec<String>,
extra_env: Vec<acp::EnvVariable>,
cwd: Option<PathBuf>,
output_byte_limit: Option<u64>,
cx: &mut Context<Self>,
) -> Task<Result<Entity<Terminal>>> {
for arg in args {
command.push(' ');
command.push_str(&arg);
}
let shell_command = if cfg!(windows) {
format!("$null | & {{{}}}", command.replace("\"", "'"))
} else if let Some(cwd) = cwd.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}; {}) </dev/null", command)
} else {
format!("({}) </dev/null", command)
};
let args = vec!["-c".into(), shell_command];
let env = match &cwd {
Some(dir) => self.project.update(cx, |project, cx| {
project.directory_environment(dir.as_path().into(), cx)
@@ -1982,20 +1951,30 @@ impl AcpThread {
let project = self.project.clone();
let language_registry = project.read(cx).languages().clone();
let determine_shell = self.determine_shell.clone();
let terminal_id = acp::TerminalId(Uuid::new_v4().to_string().into());
let terminal_task = cx.spawn({
let terminal_id = terminal_id.clone();
async move |_this, cx| {
let program = determine_shell.await;
let env = env.await;
let (command, args) = ShellBuilder::new(
project
.update(cx, |project, cx| {
project
.remote_client()
.and_then(|r| r.read(cx).default_system_shell())
})?
.as_deref(),
&Shell::Program(get_default_system_shell()),
)
.redirect_stdin_to_dev_null()
.build(Some(command), &args);
let terminal = project
.update(cx, |project, cx| {
project.create_terminal_task(
task::SpawnInTerminal {
command: Some(program),
args,
command: Some(command.clone()),
args: args.clone(),
cwd: cwd.clone(),
env,
..Default::default()
@@ -2008,7 +1987,7 @@ impl AcpThread {
cx.new(|cx| {
Terminal::new(
terminal_id,
command,
&format!("{} {}", command, args.join(" ")),
cwd,
output_byte_limit.map(|l| l as usize),
terminal,
crates/acp_thread/src/terminal.rs
+2 -2
View File
@@ -28,7 +28,7 @@ pub struct TerminalOutput {
impl Terminal {
pub fn new(
id: acp::TerminalId,
command: String,
command_label: &str,
working_dir: Option<PathBuf>,
output_byte_limit: Option<usize>,
terminal: Entity<terminal::Terminal>,
@@ -40,7 +40,7 @@ impl Terminal {
id,
command: cx.new(|cx| {
Markdown::new(
format!("```\n{}\n```", command).into(),
format!("```\n{}\n```", command_label).into(),
Some(language_registry.clone()),
None,
cx,
crates/assistant_tools/Cargo.toml
-1
View File
@@ -63,7 +63,6 @@ ui.workspace = true
util.workspace = true
watch.workspace = true
web_search.workspace = true
which.workspace = true
workspace-hack.workspace = true
workspace.workspace = true
crates/assistant_tools/src/assistant_tools.rs
+1 -1
View File
@@ -52,7 +52,7 @@ pub fn init(http_client: Arc<HttpClientWithUrl>, cx: &mut App) {
assistant_tool::init(cx);
let registry = ToolRegistry::global(cx);
registry.register_tool(TerminalTool::new(cx));
registry.register_tool(TerminalTool);
registry.register_tool(CreateDirectoryTool);
registry.register_tool(CopyPathTool);
registry.register_tool(DeletePathTool);
crates/assistant_tools/src/terminal_tool.rs
+29 -52
View File
@@ -6,7 +6,7 @@ use action_log::ActionLog;
use agent_settings;
use anyhow::{Context as _, Result, anyhow};
use assistant_tool::{Tool, ToolCard, ToolResult, ToolUseStatus};
use futures::{FutureExt as _, future::Shared};
use futures::FutureExt as _;
use gpui::{
AnyWindowHandle, App, AppContext, Empty, Entity, EntityId, Task, TextStyleRefinement,
WeakEntity, Window,
@@ -26,11 +26,12 @@ use std::{
sync::Arc,
time::{Duration, Instant},
};
use task::{Shell, ShellBuilder};
use terminal_view::TerminalView;
use theme::ThemeSettings;
use ui::{CommonAnimationExt, Disclosure, Tooltip, prelude::*};
use util::{
ResultExt, get_system_shell, markdown::MarkdownInlineCode, size::format_file_size,
ResultExt, get_default_system_shell, markdown::MarkdownInlineCode, size::format_file_size,
time::duration_alt_display,
};
use workspace::Workspace;
@@ -45,29 +46,10 @@ pub struct TerminalToolInput {
cd: String,
}
pub struct TerminalTool {
determine_shell: Shared<Task<String>>,
}
pub struct TerminalTool;
impl TerminalTool {
pub const NAME: &str = "terminal";
pub(crate) fn new(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() {
"bash".into()
} else {
get_system_shell()
}
});
Self {
determine_shell: determine_shell.shared(),
}
}
}
impl Tool for TerminalTool {
@@ -135,19 +117,6 @@ impl Tool for TerminalTool {
Ok(dir) => dir,
Err(err) => return Task::ready(Err(err)).into(),
};
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}; {}) </dev/null", input.command)
} else {
format!("({}) </dev/null", input.command)
};
let args = vec!["-c".into(), command];
let cwd = working_dir.clone();
let env = match &working_dir {
@@ -156,6 +125,11 @@ impl Tool for TerminalTool {
}),
None => Task::ready(None).shared(),
};
let remote_shell = project.update(cx, |project, cx| {
project
.remote_client()
.and_then(|r| r.read(cx).default_system_shell())
});
let env = cx.spawn(async move |_| {
let mut env = env.await.unwrap_or_default();
@@ -171,8 +145,13 @@ impl Tool for TerminalTool {
let task = cx.background_spawn(async move {
let env = env.await;
let pty_system = native_pty_system();
let program = program.await;
let mut cmd = CommandBuilder::new(program);
let (command, args) = ShellBuilder::new(
remote_shell.as_deref(),
&Shell::Program(get_default_system_shell()),
)
.redirect_stdin_to_dev_null()
.build(Some(input.command.clone()), &[]);
let mut cmd = CommandBuilder::new(command);
cmd.args(args);
for (k, v) in env {
cmd.env(k, v);
@@ -208,16 +187,22 @@ impl Tool for TerminalTool {
};
};
let command = input.command.clone();
let terminal = cx.spawn({
let project = project.downgrade();
async move |cx| {
let program = program.await;
let (command, args) = ShellBuilder::new(
remote_shell.as_deref(),
&Shell::Program(get_default_system_shell()),
)
.redirect_stdin_to_dev_null()
.build(Some(input.command), &[]);
let env = env.await;
project
.update(cx, |project, cx| {
project.create_terminal_task(
task::SpawnInTerminal {
command: Some(program),
command: Some(command),
args,
cwd,
env,
@@ -230,14 +215,8 @@ impl Tool for TerminalTool {
}
});
let command_markdown = cx.new(|cx| {
Markdown::new(
format!("```bash\n{}\n```", input.command).into(),
None,
None,
cx,
)
});
let command_markdown =
cx.new(|cx| Markdown::new(format!("```bash\n{}\n```", command).into(), None, None, cx));
let card = cx.new(|cx| {
TerminalToolCard::new(
@@ -288,7 +267,7 @@ impl Tool for TerminalTool {
let previous_len = content.len();
let (processed_content, finished_with_empty_output) = process_content(
&content,
&input.command,
&command,
exit_status.map(portable_pty::ExitStatus::from),
);
@@ -740,7 +719,6 @@ mod tests {
if cfg!(windows) {
return;
}
init_test(&executor, cx);
let fs = Arc::new(RealFs::new(None, executor));
@@ -763,7 +741,7 @@ mod tests {
};
let result = cx.update(|cx| {
TerminalTool::run(
Arc::new(TerminalTool::new(cx)),
Arc::new(TerminalTool),
serde_json::to_value(input).unwrap(),
Arc::default(),
project.clone(),
@@ -783,7 +761,6 @@ mod tests {
if cfg!(windows) {
return;
}
init_test(&executor, cx);
let fs = Arc::new(RealFs::new(None, executor));
@@ -798,7 +775,7 @@ mod tests {
let check = |input, expected, cx: &mut App| {
let headless_result = TerminalTool::run(
Arc::new(TerminalTool::new(cx)),
Arc::new(TerminalTool),
serde_json::to_value(input).unwrap(),
Arc::default(),
project.clone(),
crates/languages/src/python.rs
+3 -3
View File
@@ -1131,7 +1131,7 @@ impl ToolchainLister for PythonToolchainProvider {
let activate_keyword = match shell {
ShellKind::Cmd => ".",
ShellKind::Nushell => "overlay use",
ShellKind::Powershell => ".",
ShellKind::PowerShell => ".",
ShellKind::Fish => "source",
ShellKind::Csh => "source",
ShellKind::Posix => "source",
@@ -1141,7 +1141,7 @@ impl ToolchainLister for PythonToolchainProvider {
ShellKind::Csh => "activate.csh",
ShellKind::Fish => "activate.fish",
ShellKind::Nushell => "activate.nu",
ShellKind::Powershell => "activate.ps1",
ShellKind::PowerShell => "activate.ps1",
ShellKind::Cmd => "activate.bat",
};
let path = prefix.join(BINARY_DIR).join(activate_script_name);
@@ -1165,7 +1165,7 @@ impl ToolchainLister for PythonToolchainProvider {
ShellKind::Fish => Some(format!("\"{pyenv}\" shell - fish {version}")),
ShellKind::Posix => Some(format!("\"{pyenv}\" shell - sh {version}")),
ShellKind::Nushell => Some(format!("\"{pyenv}\" shell - nu {version}")),
ShellKind::Powershell => None,
ShellKind::PowerShell => None,
ShellKind::Csh => None,
ShellKind::Cmd => None,
})
crates/project/src/terminals.rs
+39 -27
View File
@@ -179,7 +179,7 @@ impl Project {
}
};
let shell = {
let (shell, env) = {
env.extend(spawn_task.env);
match remote_client {
Some(remote_client) => match activation_script.clone() {
@@ -189,8 +189,14 @@ impl Project {
let args =
vec!["-c".to_owned(), format!("{activation_script}; {to_run}")];
create_remote_shell(
Some((&shell, &args)),
&mut env,
Some((
&remote_client
.read(cx)
.shell()
.unwrap_or_else(get_default_system_shell),
&args,
)),
env,
path,
remote_client,
cx,
@@ -201,7 +207,7 @@ impl Project {
.command
.as_ref()
.map(|command| (command, &spawn_task.args)),
&mut env,
env,
path,
remote_client,
cx,
@@ -220,13 +226,16 @@ impl Project {
#[cfg(not(windows))]
let arg = format!("{activation_script}; {to_run}");
Shell::WithArguments {
program: shell,
args: vec!["-c".to_owned(), arg],
title_override: None,
}
(
Shell::WithArguments {
program: shell,
args: vec!["-c".to_owned(), arg],
title_override: None,
},
env,
)
}
_ => {
_ => (
if let Some(program) = spawn_task.command {
Shell::WithArguments {
program,
@@ -235,8 +244,9 @@ impl Project {
}
} else {
Shell::System
}
}
},
env,
),
},
}
};
@@ -330,7 +340,7 @@ impl Project {
.map(|p| self.active_toolchain(p, LanguageName::new("Python"), cx))
.collect::<Vec<_>>();
let remote_client = self.remote_client.clone();
let shell = match &remote_client {
let shell_kind = ShellKind::new(&match &remote_client {
Some(remote_client) => remote_client
.read(cx)
.shell()
@@ -344,7 +354,7 @@ impl Project {
} => program.clone(),
Shell::System => get_system_shell(),
},
};
});
let lang_registry = self.languages.clone();
let fs = self.fs.clone();
@@ -361,7 +371,7 @@ impl Project {
let lister = language?.toolchain_lister();
return Some(
lister?
.activation_script(&toolchain, ShellKind::new(&shell), fs.as_ref())
.activation_script(&toolchain, shell_kind, fs.as_ref())
.await,
);
}
@@ -370,12 +380,12 @@ impl Project {
.await
.unwrap_or_default();
project.update(cx, move |this, cx| {
let shell = {
let (shell, env) = {
match remote_client {
Some(remote_client) => {
create_remote_shell(None, &mut env, path, remote_client, cx)?
create_remote_shell(None, env, path, remote_client, cx)?
}
None => settings.shell,
None => (settings.shell, env),
}
};
TerminalBuilder::new(
@@ -545,11 +555,11 @@ fn quote_arg(argument: &str, quote: bool) -> String {
fn create_remote_shell(
spawn_command: Option<(&String, &Vec<String>)>,
env: &mut HashMap<String, String>,
mut env: HashMap<String, String>,
working_directory: Option<Arc<Path>>,
remote_client: Entity<RemoteClient>,
cx: &mut App,
) -> Result<Shell> {
) -> Result<(Shell, HashMap<String, String>)> {
// Alacritty sets its terminfo to `alacritty`, this requiring hosts to have it installed
// to properly display colors.
// We do not have the luxury of assuming the host has it installed,
@@ -565,18 +575,20 @@ fn create_remote_shell(
let command = remote_client.read(cx).build_command(
program,
args.as_slice(),
env,
&env,
working_directory.map(|path| path.display().to_string()),
None,
)?;
*env = command.env;
log::debug!("Connecting to a remote server: {:?}", command.program);
let host = remote_client.read(cx).connection_options().display_name();
Ok(Shell::WithArguments {
program: command.program,
args: command.args,
title_override: Some(format!("{} — Terminal", host).into()),
})
Ok((
Shell::WithArguments {
program: command.program,
args: command.args,
title_override: Some(format!("{} — Terminal", host).into()),
},
command.env,
))
}
crates/remote/src/remote_client.rs
+9
View File
@@ -772,6 +772,10 @@ impl RemoteClient {
Some(self.remote_connection()?.shell())
}
pub fn default_system_shell(&self) -> Option<String> {
Some(self.remote_connection()?.default_system_shell())
}
pub fn shares_network_interface(&self) -> bool {
self.remote_connection()
.map_or(false, |connection| connection.shares_network_interface())
@@ -1062,6 +1066,7 @@ pub(crate) trait RemoteConnection: Send + Sync {
fn connection_options(&self) -> RemoteConnectionOptions;
fn path_style(&self) -> PathStyle;
fn shell(&self) -> String;
fn default_system_shell(&self) -> String;
#[cfg(any(test, feature = "test-support"))]
fn simulate_disconnect(&self, _: &AsyncApp) {}
@@ -1503,6 +1508,10 @@ mod fake {
fn shell(&self) -> String {
"sh".to_owned()
}
fn default_system_shell(&self) -> String {
"sh".to_owned()
}
}
pub(super) struct Delegate;
crates/remote/src/transport/ssh.rs
+7
View File
@@ -37,6 +37,7 @@ pub(crate) struct SshRemoteConnection {
ssh_platform: RemotePlatform,
ssh_path_style: PathStyle,
ssh_shell: String,
ssh_default_system_shell: String,
_temp_dir: TempDir,
}
@@ -105,6 +106,10 @@ impl RemoteConnection for SshRemoteConnection {
self.ssh_shell.clone()
}
fn default_system_shell(&self) -> String {
self.ssh_default_system_shell.clone()
}
fn build_command(
&self,
input_program: Option<String>,
@@ -347,6 +352,7 @@ impl SshRemoteConnection {
_ => PathStyle::Posix,
};
let ssh_shell = socket.shell().await;
let ssh_default_system_shell = String::from("/bin/sh");
let mut this = Self {
socket,
@@ -356,6 +362,7 @@ impl SshRemoteConnection {
ssh_path_style,
ssh_platform,
ssh_shell,
ssh_default_system_shell,
};
let (release_channel, version, commit) = cx.update(|cx| {
crates/remote/src/transport/wsl.rs
+11 -1
View File
@@ -29,6 +29,7 @@ pub(crate) struct WslRemoteConnection {
remote_binary_path: Option<RemotePathBuf>,
platform: RemotePlatform,
shell: String,
default_system_shell: String,
connection_options: WslConnectionOptions,
}
@@ -56,6 +57,7 @@ impl WslRemoteConnection {
remote_binary_path: None,
platform: RemotePlatform { os: "", arch: "" },
shell: String::new(),
default_system_shell: String::from("/bin/sh"),
};
delegate.set_status(Some("Detecting WSL environment"), cx);
this.platform = this.detect_platform().await?;
@@ -84,7 +86,11 @@ impl WslRemoteConnection {
.run_wsl_command("sh", &["-c", "echo $SHELL"])
.await
.ok()
.and_then(|shell_path| shell_path.trim().split('/').next_back().map(str::to_string))
.and_then(|shell_path| {
Path::new(shell_path.trim())
.file_name()
.map(|it| it.to_str().unwrap().to_owned())
})
.unwrap_or_else(|| "bash".to_string()))
}
@@ -427,6 +433,10 @@ impl RemoteConnection for WslRemoteConnection {
fn shell(&self) -> String {
self.shell.clone()
}
fn default_system_shell(&self) -> String {
self.default_system_shell.clone()
}
}
/// `wslpath` is a executable available in WSL, it's a linux binary.
crates/task/src/shell_builder.rs
+29 -8
View File
@@ -10,7 +10,7 @@ pub enum ShellKind {
Posix,
Csh,
Fish,
Powershell,
PowerShell,
Nushell,
Cmd,
}
@@ -21,7 +21,7 @@ impl fmt::Display for ShellKind {
ShellKind::Posix => write!(f, "sh"),
ShellKind::Csh => write!(f, "csh"),
ShellKind::Fish => write!(f, "fish"),
ShellKind::Powershell => write!(f, "powershell"),
ShellKind::PowerShell => write!(f, "powershell"),
ShellKind::Nushell => write!(f, "nu"),
ShellKind::Cmd => write!(f, "cmd"),
}
@@ -43,7 +43,7 @@ impl ShellKind {
|| program == "pwsh"
|| program.ends_with("pwsh.exe")
{
ShellKind::Powershell
ShellKind::PowerShell
} else if program == "cmd" || program.ends_with("cmd.exe") {
ShellKind::Cmd
} else if program == "nu" {
@@ -61,7 +61,7 @@ impl ShellKind {
fn to_shell_variable(self, input: &str) -> String {
match self {
Self::Powershell => Self::to_powershell_variable(input),
Self::PowerShell => Self::to_powershell_variable(input),
Self::Cmd => Self::to_cmd_variable(input),
Self::Posix => input.to_owned(),
Self::Fish => input.to_owned(),
@@ -184,7 +184,7 @@ impl ShellKind {
fn args_for_shell(&self, interactive: bool, combined_command: String) -> Vec<String> {
match self {
ShellKind::Powershell => vec!["-C".to_owned(), combined_command],
ShellKind::PowerShell => vec!["-C".to_owned(), combined_command],
ShellKind::Cmd => vec!["/C".to_owned(), combined_command],
ShellKind::Posix | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh => interactive
.then(|| "-i".to_owned())
@@ -196,7 +196,7 @@ impl ShellKind {
pub fn command_prefix(&self) -> Option<char> {
match self {
ShellKind::Powershell => Some('&'),
ShellKind::PowerShell => Some('&'),
ShellKind::Nushell => Some('^'),
_ => None,
}
@@ -210,6 +210,7 @@ pub struct ShellBuilder {
program: String,
args: Vec<String>,
interactive: bool,
redirect_stdin: bool,
kind: ShellKind,
}
@@ -231,6 +232,7 @@ impl ShellBuilder {
args,
interactive: true,
kind,
redirect_stdin: false,
}
}
pub fn non_interactive(mut self) -> Self {
@@ -241,7 +243,7 @@ impl ShellBuilder {
/// Returns the label to show in the terminal tab
pub fn command_label(&self, command_label: &str) -> String {
match self.kind {
ShellKind::Powershell => {
ShellKind::PowerShell => {
format!("{} -C '{}'", self.program, command_label)
}
ShellKind::Cmd => {
@@ -256,6 +258,12 @@ impl ShellBuilder {
}
}
}
pub fn redirect_stdin_to_dev_null(mut self) -> Self {
self.redirect_stdin = true;
self
}
/// Returns the program and arguments to run this task in a shell.
pub fn build(
mut self,
@@ -263,11 +271,24 @@ impl ShellBuilder {
task_args: &[String],
) -> (String, Vec<String>) {
if let Some(task_command) = task_command {
let combined_command = task_args.iter().fold(task_command, |mut command, arg| {
let mut combined_command = task_args.iter().fold(task_command, |mut command, arg| {
command.push(' ');
command.push_str(&self.kind.to_shell_variable(arg));
command
});
if self.redirect_stdin {
match self.kind {
ShellKind::Posix | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh => {
combined_command.push_str(" </dev/null");
}
ShellKind::PowerShell => {
combined_command.insert_str(0, "$null | ");
}
ShellKind::Cmd => {
combined_command.push_str("< NUL");
}
}
}
self.args
.extend(self.kind.args_for_shell(self.interactive, combined_command));