From 7db7ea9277e6a2bffa7c7cabc7bf7791012778a7 Mon Sep 17 00:00:00 2001 From: Oliver Azevedo Barnes Date: Tue, 21 Apr 2026 09:06:40 +0100 Subject: [PATCH] terminal: Use system shell for non-terminal uses (like spinning up external agents) (#51741) Closes #46551 Zed was using the `terminal.shell` setting for some actions unrelated to the terminal GUI, like environment capture, ACP agent startup, context server startup, and vim `:!`, and so if a user set it to `tmux` or `nushell`, those paths would fail. This PR ensures paths unrelated to Zed's terminal use the system path instead. Manual testing: Set `terminal.shell` to `tmux` or any another non-bash executable. Starting an external agent thread shouldn't break. Release Notes: - Fixed ACP agent and other breakage when `terminal.shell` was set to a non-shell program like `tmux` --- Cargo.lock | 1 - crates/agent_servers/src/acp.rs | 8 ++--- crates/context_server/Cargo.toml | 1 - .../src/transport/stdio_transport.rs | 6 ++-- crates/project/src/environment.rs | 33 ++----------------- crates/project/src/terminals.rs | 14 +++++--- crates/vim/src/command.rs | 4 +-- 7 files changed, 20 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2157d065f2c1e0bf13fb8d402ea0f2970579d1bd..c662bfe5d8d2f1d8d9ea3281ddf9373419b82a9d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3558,7 +3558,6 @@ dependencies = [ "slotmap", "smol", "tempfile", - "terminal", "tiny_http", "url", "util", diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index ce080b244dd4f9560915b99978d6c746c80d2d88..62e3a526d00e6358237b8aaec38e523252fb273f 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -16,12 +16,11 @@ use project::agent_server_store::{AgentServerCommand, AgentServerStore}; use project::{AgentId, Project}; use remote::remote_client::Interactive; use serde::Deserialize; -use settings::Settings as _; use std::path::PathBuf; use std::process::Stdio; use std::rc::Rc; use std::{any::Any, cell::RefCell}; -use task::{ShellBuilder, SpawnInTerminal}; +use task::{Shell, ShellBuilder, SpawnInTerminal}; use thiserror::Error; use util::ResultExt as _; use util::path_list::PathList; @@ -34,7 +33,7 @@ use gpui::{App, AppContext as _, AsyncApp, Entity, SharedString, Task, WeakEntit use acp_thread::{AcpThread, AuthRequired, LoadError, TerminalProviderEvent}; use terminal::TerminalBuilder; -use terminal::terminal_settings::{AlternateScroll, CursorShape, TerminalSettings}; +use terminal::terminal_settings::{AlternateScroll, CursorShape}; use crate::GEMINI_ID; @@ -251,8 +250,7 @@ impl AcpConnection { ) }); - let shell = cx.update(|cx| TerminalSettings::get(None, cx).shell.clone()); - let builder = ShellBuilder::new(&shell, cfg!(windows)).non_interactive(); + let builder = ShellBuilder::new(&Shell::System, cfg!(windows)).non_interactive(); let mut child = builder.build_std_command(Some(path.clone()), &args); child.envs(env.clone()); if let Some(cwd) = project.read_with(cx, |project, _cx| { diff --git a/crates/context_server/Cargo.toml b/crates/context_server/Cargo.toml index 0a9c94a54d70196c0a0fee04dec249ea367d56c0..dea98bd69e0c28519455c8be6f8e1a5e7870759e 100644 --- a/crates/context_server/Cargo.toml +++ b/crates/context_server/Cargo.toml @@ -38,7 +38,6 @@ tempfile.workspace = true tiny_http.workspace = true url = { workspace = true, features = ["serde"] } util.workspace = true -terminal.workspace = true [dev-dependencies] gpui = { workspace = true, features = ["test-support"] } diff --git a/crates/context_server/src/transport/stdio_transport.rs b/crates/context_server/src/transport/stdio_transport.rs index c3af1aa8745a074ad545cad0518d2ffea2822b65..0b5525a3a5af44183005eb695c711ed481faa4cd 100644 --- a/crates/context_server/src/transport/stdio_transport.rs +++ b/crates/context_server/src/transport/stdio_transport.rs @@ -8,11 +8,10 @@ use futures::{ AsyncBufReadExt as _, AsyncRead, AsyncWrite, AsyncWriteExt as _, Stream, StreamExt as _, }; use gpui::AsyncApp; -use settings::Settings as _; use smol::channel; use smol::process::Child; -use terminal::terminal_settings::TerminalSettings; use util::TryFutureExt as _; +use util::shell::Shell; use util::shell_builder::ShellBuilder; use crate::client::ModelContextServerBinary; @@ -31,8 +30,7 @@ impl StdioTransport { working_directory: &Option, cx: &AsyncApp, ) -> Result { - let shell = cx.update(|cx| TerminalSettings::get(None, cx).shell.clone()); - let builder = ShellBuilder::new(&shell, cfg!(windows)).non_interactive(); + let builder = ShellBuilder::new(&Shell::System, cfg!(windows)).non_interactive(); let mut command = builder.build_smol_command(Some(binary.executable.display().to_string()), &binary.args); diff --git a/crates/project/src/environment.rs b/crates/project/src/environment.rs index 8156e172b91796ec3a9ef9446188a14bd537887e..c7b87ea8e35d0789a33c6e8354b87c471a6ae154 100644 --- a/crates/project/src/environment.rs +++ b/crates/project/src/environment.rs @@ -5,8 +5,7 @@ use remote::RemoteClient; use rpc::proto::{self, REMOTE_SERVER_PROJECT_ID}; use std::{collections::VecDeque, path::Path, sync::Arc}; use task::{Shell, shell_to_proto}; -use terminal::terminal_settings::TerminalSettings; -use util::{ResultExt, command::new_command, rel_path::RelPath}; +use util::{ResultExt, command::new_command}; use worktree::Worktree; use collections::HashMap; @@ -134,19 +133,7 @@ impl ProjectEnvironment { None if self.is_remote_project => { Some(self.local_directory_environment(&Shell::System, abs_path, cx)) } - None => Some({ - let shell = TerminalSettings::get( - Some(settings::SettingsLocation { - worktree_id: worktree.id(), - path: RelPath::empty(), - }), - cx, - ) - .shell - .clone(); - - self.local_directory_environment(&shell, abs_path, cx) - }), + None => Some(self.local_directory_environment(&Shell::System, abs_path, cx)), } .unwrap_or_else(|| Task::ready(None).shared()) } @@ -175,21 +162,7 @@ impl ProjectEnvironment { worktree_store.find_worktree(&abs_path, cx) }) .ok() - .map(|worktree| { - let shell = terminal::terminal_settings::TerminalSettings::get( - worktree - .as_ref() - .map(|(worktree, path)| settings::SettingsLocation { - worktree_id: worktree.read(cx).id(), - path: &path, - }), - cx, - ) - .shell - .clone(); - - self.local_directory_environment(&shell, abs_path, cx) - }), + .map(|_| self.local_directory_environment(&Shell::System, abs_path, cx)), } .unwrap_or_else(|| Task::ready(None).shared()) } diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index 6efddcdf7726110a61f15666c68b963181181086..e22af5d552fa8e144ec55f044e04768f1e9f88b4 100644 --- a/crates/project/src/terminals.rs +++ b/crates/project/src/terminals.rs @@ -17,7 +17,9 @@ use terminal::{ TaskState, TaskStatus, Terminal, TerminalBuilder, insert_zed_terminal_env, terminal_settings::TerminalSettings, }; -use util::{command::new_std_command, get_default_system_shell, maybe, rel_path::RelPath}; +use util::{ + command::new_std_command, get_default_system_shell, get_system_shell, maybe, rel_path::RelPath, +}; use crate::{Project, ProjectPath}; @@ -103,7 +105,7 @@ impl Project { .read(cx) .shell() .unwrap_or_else(get_default_system_shell), - None => settings.shell.program(), + None => get_system_shell(), }; let path_style = self.path_style(cx); let shell_kind = ShellKind::new(&shell, path_style.is_windows()); @@ -363,12 +365,16 @@ impl Project { .unwrap_or_else(get_default_system_shell), None => settings.shell.program(), }; + let env_shell = match &remote_client { + Some(_) => shell.clone(), + None => get_system_shell(), + }; let path_style = self.path_style(cx); // Prepare a task for resolving the environment let env_task = - self.resolve_directory_environment(&shell, path.clone(), remote_client.clone(), cx); + self.resolve_directory_environment(&env_shell, path.clone(), remote_client.clone(), cx); let lang_registry = self.languages.clone(); cx.spawn(async move |project, cx| { @@ -526,7 +532,7 @@ impl Project { .as_ref() .and_then(|remote_client| remote_client.read(cx).shell()) .map(Shell::Program) - .unwrap_or_else(|| settings.shell.clone()); + .unwrap_or(Shell::System); let is_windows = self.path_style(cx).is_windows(); let builder = ShellBuilder::new(&shell, is_windows).non_interactive(); let (command, args) = builder.build(Some(command), &Vec::new()); diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 06fa6ead775809c3df775d959fb080a93ee84aad..6c10c3212334c6658e257a347c39a4d96cda7e8b 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -28,7 +28,7 @@ use std::{ sync::OnceLock, time::Instant, }; -use task::{HideStrategy, RevealStrategy, SaveStrategy, SpawnInTerminal, TaskId}; +use task::{HideStrategy, RevealStrategy, SaveStrategy, Shell, SpawnInTerminal, TaskId}; use ui::ActiveTheme; use util::{ ResultExt, @@ -2476,7 +2476,7 @@ impl ShellExec { workspace.update(cx, |workspace, cx| { let project = workspace.project().read(cx); let cwd = project.first_project_directory(cx); - let shell = project.terminal_settings(&cwd, cx).shell.clone(); + let shell = Shell::System; let spawn_in_terminal = SpawnInTerminal { id: TaskId("vim".to_string()),