windows: Add support for fetching shell environment in remote projects (#39831)

Cole Miller created

Closes #39216

Note that this affects all platforms, I'm just using the prefix to make
auto-cherry-picking easier.

Release Notes:

- Fixed shell commands run by agents failing to find installed programs
in some cases.

Change summary

Cargo.lock                                          |  1 
crates/activity_indicator/src/activity_indicator.rs | 13 -
crates/project/src/agent_server_store.rs            | 25 +++
crates/project/src/debugger/dap_store.rs            |  8 +
crates/project/src/environment.rs                   | 80 +++++++++------
crates/project/src/git_store.rs                     |  3 
crates/project/src/project.rs                       | 18 ++-
crates/project/src/toolchain_store.rs               | 13 ++
crates/proto/proto/task.proto                       | 10 +
crates/proto/proto/zed.proto                        |  5 
crates/proto/src/proto.rs                           |  4 
crates/remote_server/Cargo.toml                     |  1 
crates/remote_server/src/headless_project.rs        | 25 ++++
crates/task/src/task.rs                             | 33 ++++++
14 files changed, 180 insertions(+), 59 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -13051,6 +13051,7 @@ dependencies = [
  "shellexpand 2.1.2",
  "smol",
  "sysinfo",
+ "task",
  "thiserror 2.0.12",
  "toml 0.8.20",
  "unindent",

crates/activity_indicator/src/activity_indicator.rs 🔗

@@ -20,7 +20,6 @@ use std::{
     cmp::Reverse,
     collections::HashSet,
     fmt::Write,
-    path::Path,
     sync::Arc,
     time::{Duration, Instant},
 };
@@ -328,17 +327,13 @@ impl ActivityIndicator {
             .flatten()
     }
 
-    fn pending_environment_errors<'a>(
-        &'a self,
-        cx: &'a App,
-    ) -> impl Iterator<Item = (&'a Arc<Path>, &'a EnvironmentErrorMessage)> {
-        self.project.read(cx).shell_environment_errors(cx)
+    fn pending_environment_error<'a>(&'a self, cx: &'a App) -> Option<&'a EnvironmentErrorMessage> {
+        self.project.read(cx).peek_environment_error(cx)
     }
 
     fn content_to_render(&mut self, cx: &mut Context<Self>) -> Option<Content> {
         // Show if any direnv calls failed
-        if let Some((abs_path, error)) = self.pending_environment_errors(cx).next() {
-            let abs_path = abs_path.clone();
+        if let Some(error) = self.pending_environment_error(cx) {
             return Some(Content {
                 icon: Some(
                     Icon::new(IconName::Warning)
@@ -348,7 +343,7 @@ impl ActivityIndicator {
                 message: error.0.clone(),
                 on_click: Some(Arc::new(move |this, window, cx| {
                     this.project.update(cx, |project, cx| {
-                        project.remove_environment_error(&abs_path, cx);
+                        project.pop_environment_error(cx);
                     });
                     window.dispatch_action(Box::new(workspace::OpenLog), cx);
                 })),

crates/project/src/agent_server_store.rs 🔗

@@ -21,6 +21,7 @@ use rpc::{AnyProtoClient, TypedEnvelope, proto};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use settings::{SettingsContent, SettingsStore};
+use task::Shell;
 use util::{ResultExt as _, debug_panic};
 
 use crate::ProjectEnvironment;
@@ -850,7 +851,11 @@ impl ExternalAgentServer for LocalGemini {
         cx.spawn(async move |cx| {
             let mut env = project_environment
                 .update(cx, |project_environment, cx| {
-                    project_environment.get_directory_environment(root_dir.clone(), cx)
+                    project_environment.get_local_directory_environment(
+                        &Shell::System,
+                        root_dir.clone(),
+                        cx,
+                    )
                 })?
                 .await
                 .unwrap_or_default();
@@ -937,7 +942,11 @@ impl ExternalAgentServer for LocalClaudeCode {
         cx.spawn(async move |cx| {
             let mut env = project_environment
                 .update(cx, |project_environment, cx| {
-                    project_environment.get_directory_environment(root_dir.clone(), cx)
+                    project_environment.get_local_directory_environment(
+                        &Shell::System,
+                        root_dir.clone(),
+                        cx,
+                    )
                 })?
                 .await
                 .unwrap_or_default();
@@ -1023,7 +1032,11 @@ impl ExternalAgentServer for LocalCodex {
         cx.spawn(async move |cx| {
             let mut env = project_environment
                 .update(cx, |project_environment, cx| {
-                    project_environment.get_directory_environment(root_dir.clone(), cx)
+                    project_environment.get_local_directory_environment(
+                        &Shell::System,
+                        root_dir.clone(),
+                        cx,
+                    )
                 })?
                 .await
                 .unwrap_or_default();
@@ -1163,7 +1176,11 @@ impl ExternalAgentServer for LocalCustomAgent {
         cx.spawn(async move |cx| {
             let mut env = project_environment
                 .update(cx, |project_environment, cx| {
-                    project_environment.get_directory_environment(root_dir.clone(), cx)
+                    project_environment.get_local_directory_environment(
+                        &Shell::System,
+                        root_dir.clone(),
+                        cx,
+                    )
                 })?
                 .await
                 .unwrap_or_default();

crates/project/src/debugger/dap_store.rs 🔗

@@ -49,7 +49,7 @@ use std::{
     path::{Path, PathBuf},
     sync::{Arc, Once},
 };
-use task::{DebugScenario, SpawnInTerminal, TaskContext, TaskTemplate};
+use task::{DebugScenario, Shell, SpawnInTerminal, TaskContext, TaskTemplate};
 use util::{ResultExt as _, rel_path::RelPath};
 use worktree::Worktree;
 
@@ -231,7 +231,11 @@ impl DapStore {
                                 .unwrap()
                                 .environment
                                 .update(cx, |environment, cx| {
-                                    environment.get_directory_environment(cwd, cx)
+                                    environment.get_local_directory_environment(
+                                        &Shell::System,
+                                        cwd,
+                                        cx,
+                                    )
                                 })
                         })?
                         .await;

crates/project/src/environment.rs 🔗

@@ -1,6 +1,8 @@
 use futures::{FutureExt, future::Shared};
 use language::Buffer;
-use std::{path::Path, sync::Arc};
+use remote::RemoteClient;
+use rpc::proto::{self, REMOTE_SERVER_PROJECT_ID};
+use std::{collections::VecDeque, path::Path, sync::Arc};
 use task::Shell;
 use util::ResultExt;
 use worktree::Worktree;
@@ -16,10 +18,9 @@ use crate::{
 
 pub struct ProjectEnvironment {
     cli_environment: Option<HashMap<String, String>>,
-    environments: HashMap<Arc<Path>, Shared<Task<Option<HashMap<String, String>>>>>,
-    shell_based_environments:
-        HashMap<(Shell, Arc<Path>), Shared<Task<Option<HashMap<String, String>>>>>,
-    environment_error_messages: HashMap<Arc<Path>, EnvironmentErrorMessage>,
+    local_environments: HashMap<(Shell, Arc<Path>), Shared<Task<Option<HashMap<String, String>>>>>,
+    remote_environments: HashMap<(Shell, Arc<Path>), Shared<Task<Option<HashMap<String, String>>>>>,
+    environment_error_messages: VecDeque<EnvironmentErrorMessage>,
 }
 
 pub enum ProjectEnvironmentEvent {
@@ -32,8 +33,8 @@ impl ProjectEnvironment {
     pub fn new(cli_environment: Option<HashMap<String, String>>) -> Self {
         Self {
             cli_environment,
-            environments: Default::default(),
-            shell_based_environments: Default::default(),
+            local_environments: Default::default(),
+            remote_environments: Default::default(),
             environment_error_messages: Default::default(),
         }
     }
@@ -48,19 +49,6 @@ impl ProjectEnvironment {
         }
     }
 
-    /// Returns an iterator over all pairs `(abs_path, error_message)` of
-    /// environment errors associated with this project environment.
-    pub(crate) fn environment_errors(
-        &self,
-    ) -> impl Iterator<Item = (&Arc<Path>, &EnvironmentErrorMessage)> {
-        self.environment_error_messages.iter()
-    }
-
-    pub(crate) fn remove_environment_error(&mut self, abs_path: &Path, cx: &mut Context<Self>) {
-        self.environment_error_messages.remove(abs_path);
-        cx.emit(ProjectEnvironmentEvent::ErrorsUpdated);
-    }
-
     pub(crate) fn get_buffer_environment(
         &mut self,
         buffer: &Entity<Buffer>,
@@ -115,15 +103,16 @@ impl ProjectEnvironment {
             abs_path = parent.into();
         }
 
-        self.get_directory_environment(abs_path, cx)
+        self.get_local_directory_environment(&Shell::System, abs_path, cx)
     }
 
     /// Returns the project environment, if possible.
     /// If the project was opened from the CLI, then the inherited CLI environment is returned.
     /// If it wasn't opened from the CLI, and an absolute path is given, then a shell is spawned in
     /// that directory, to get environment variables as if the user has `cd`'d there.
-    pub fn get_directory_environment(
+    pub fn get_local_directory_environment(
         &mut self,
+        shell: &Shell,
         abs_path: Arc<Path>,
         cx: &mut Context<Self>,
     ) -> Shared<Task<Option<HashMap<String, String>>>> {
@@ -136,26 +125,53 @@ impl ProjectEnvironment {
             return Task::ready(Some(cli_environment)).shared();
         }
 
-        self.environments
-            .entry(abs_path.clone())
+        self.local_environments
+            .entry((shell.clone(), abs_path.clone()))
             .or_insert_with(|| {
-                get_directory_env_impl(&Shell::System, abs_path.clone(), cx).shared()
+                get_local_directory_environment_impl(shell, abs_path.clone(), cx).shared()
             })
             .clone()
     }
 
-    /// Returns the project environment, if possible, with the given shell.
-    pub fn get_directory_environment_for_shell(
+    pub fn get_remote_directory_environment(
         &mut self,
         shell: &Shell,
         abs_path: Arc<Path>,
+        remote_client: Entity<RemoteClient>,
         cx: &mut Context<Self>,
     ) -> Shared<Task<Option<HashMap<String, String>>>> {
-        self.shell_based_environments
+        if cfg!(any(test, feature = "test-support")) {
+            return Task::ready(Some(HashMap::default())).shared();
+        }
+
+        self.remote_environments
             .entry((shell.clone(), abs_path.clone()))
-            .or_insert_with(|| get_directory_env_impl(shell, abs_path.clone(), cx).shared())
+            .or_insert_with(|| {
+                let response =
+                    remote_client
+                        .read(cx)
+                        .proto_client()
+                        .request(proto::GetDirectoryEnvironment {
+                            project_id: REMOTE_SERVER_PROJECT_ID,
+                            shell: Some(shell.clone().to_proto()),
+                            directory: abs_path.to_string_lossy().to_string(),
+                        });
+                cx.spawn(async move |_, _| {
+                    let environment = response.await.log_err()?;
+                    Some(environment.environment.into_iter().collect())
+                })
+                .shared()
+            })
             .clone()
     }
+
+    pub fn peek_environment_error(&self) -> Option<&EnvironmentErrorMessage> {
+        self.environment_error_messages.front()
+    }
+
+    pub fn pop_environment_error(&mut self) -> Option<EnvironmentErrorMessage> {
+        self.environment_error_messages.pop_front()
+    }
 }
 
 fn set_origin_marker(env: &mut HashMap<String, String>, origin: EnvironmentOrigin) {
@@ -307,7 +323,7 @@ async fn load_shell_environment(
     }
 }
 
-fn get_directory_env_impl(
+fn get_local_directory_environment_impl(
     shell: &Shell,
     abs_path: Arc<Path>,
     cx: &Context<ProjectEnvironment>,
@@ -341,8 +357,8 @@ fn get_directory_env_impl(
 
         if let Some(error) = error_message {
             this.update(cx, |this, cx| {
-                log::error!("{error}",);
-                this.environment_error_messages.insert(abs_path, error);
+                log::error!("{error}");
+                this.environment_error_messages.push_back(error);
                 cx.emit(ProjectEnvironmentEvent::ErrorsUpdated)
             })
             .log_err();

crates/project/src/git_store.rs 🔗

@@ -62,6 +62,7 @@ use std::{
     time::Instant,
 };
 use sum_tree::{Edit, SumTree, TreeSet};
+use task::Shell;
 use text::{Bias, BufferId};
 use util::{
     ResultExt, debug_panic,
@@ -4607,7 +4608,7 @@ impl Repository {
                 .upgrade()
                 .context("missing project environment")?
                 .update(cx, |project_environment, cx| {
-                    project_environment.get_directory_environment(work_directory_abs_path.clone(), cx)
+                    project_environment.get_local_directory_environment(&Shell::System, work_directory_abs_path.clone(), cx)
                 })?
                 .await
                 .unwrap_or_else(|| {

crates/project/src/project.rs 🔗

@@ -1907,20 +1907,24 @@ impl Project {
         cx: &mut App,
     ) -> Shared<Task<Option<HashMap<String, String>>>> {
         self.environment.update(cx, |environment, cx| {
-            environment.get_directory_environment_for_shell(shell, abs_path, cx)
+            if let Some(remote_client) = self.remote_client.clone() {
+                environment.get_remote_directory_environment(shell, abs_path, remote_client, cx)
+            } else {
+                environment.get_local_directory_environment(shell, abs_path, cx)
+            }
         })
     }
 
-    pub fn shell_environment_errors<'a>(
+    pub fn peek_environment_error<'a>(
         &'a self,
         cx: &'a App,
-    ) -> impl Iterator<Item = (&'a Arc<Path>, &'a EnvironmentErrorMessage)> {
-        self.environment.read(cx).environment_errors()
+    ) -> Option<&'a EnvironmentErrorMessage> {
+        self.environment.read(cx).peek_environment_error()
     }
 
-    pub fn remove_environment_error(&mut self, abs_path: &Path, cx: &mut Context<Self>) {
-        self.environment.update(cx, |environment, cx| {
-            environment.remove_environment_error(abs_path, cx);
+    pub fn pop_environment_error(&mut self, cx: &mut Context<Self>) {
+        self.environment.update(cx, |environment, _| {
+            environment.pop_environment_error();
         });
     }
 

crates/project/src/toolchain_store.rs 🔗

@@ -19,6 +19,7 @@ use rpc::{
     },
 };
 use settings::WorktreeId;
+use task::Shell;
 use util::{ResultExt as _, rel_path::RelPath};
 
 use crate::{
@@ -521,7 +522,11 @@ impl LocalToolchainStore {
 
             let project_env = environment
                 .update(cx, |environment, cx| {
-                    environment.get_directory_environment(abs_path.as_path().into(), cx)
+                    environment.get_local_directory_environment(
+                        &Shell::System,
+                        abs_path.as_path().into(),
+                        cx,
+                    )
                 })
                 .ok()?
                 .await;
@@ -574,7 +579,11 @@ impl LocalToolchainStore {
 
             let project_env = environment
                 .update(cx, |environment, cx| {
-                    environment.get_directory_environment(path.as_path().into(), cx)
+                    environment.get_local_directory_environment(
+                        &Shell::System,
+                        path.as_path().into(),
+                        cx,
+                    )
                 })?
                 .await;
             cx.background_spawn(async move { toolchain_lister.resolve(path, project_env).await })

crates/proto/proto/task.proto 🔗

@@ -48,3 +48,13 @@ message SpawnInTerminal {
     map<string, string> env = 4;
     optional string cwd = 5;
 }
+
+message GetDirectoryEnvironment {
+    uint64 project_id = 1;
+    Shell shell = 2;
+    string directory = 3;
+}
+
+message DirectoryEnvironment {
+    map<string, string> environment = 1;
+}

crates/proto/proto/zed.proto 🔗

@@ -418,7 +418,10 @@ message Envelope {
 
         GitRenameBranch git_rename_branch = 380;
 
-        RemoteStarted remote_started = 381; // current max
+        RemoteStarted remote_started = 381;
+
+        GetDirectoryEnvironment get_directory_environment = 382;
+        DirectoryEnvironment directory_environment = 383; // current max
     }
 
     reserved 87 to 88;

crates/proto/src/proto.rs 🔗

@@ -319,6 +319,8 @@ messages!(
     (GitClone, Background),
     (GitCloneResponse, Background),
     (ToggleLspLogs, Background),
+    (GetDirectoryEnvironment, Background),
+    (DirectoryEnvironment, Background),
     (GetAgentServerCommand, Background),
     (AgentServerCommand, Background),
     (ExternalAgentsUpdated, Background),
@@ -497,6 +499,7 @@ request_messages!(
     (GetDefaultBranch, GetDefaultBranchResponse),
     (GitClone, GitCloneResponse),
     (ToggleLspLogs, Ack),
+    (GetDirectoryEnvironment, DirectoryEnvironment),
     (GetProcesses, GetProcessesResponse),
     (GetAgentServerCommand, AgentServerCommand),
     (RemoteStarted, Ack),
@@ -634,6 +637,7 @@ entity_messages!(
     GitCheckoutFiles,
     SetIndexText,
     ToggleLspLogs,
+    GetDirectoryEnvironment,
 
     Push,
     Fetch,

crates/remote_server/Cargo.toml 🔗

@@ -60,6 +60,7 @@ settings.workspace = true
 shellexpand.workspace = true
 smol.workspace = true
 sysinfo.workspace = true
+task.workspace = true
 util.workspace = true
 watch.workspace = true
 worktree.workspace = true

crates/remote_server/src/headless_project.rs 🔗

@@ -50,6 +50,7 @@ pub struct HeadlessProject {
     pub languages: Arc<LanguageRegistry>,
     pub extensions: Entity<HeadlessExtensionStore>,
     pub git_store: Entity<GitStore>,
+    pub environment: Entity<ProjectEnvironment>,
     // Used mostly to keep alive the toolchain store for RPC handlers.
     // Local variant is used within LSP store, but that's a separate entity.
     pub _toolchain_store: Entity<ToolchainStore>,
@@ -198,7 +199,7 @@ impl HeadlessProject {
             let mut agent_server_store = AgentServerStore::local(
                 node_runtime.clone(),
                 fs.clone(),
-                environment,
+                environment.clone(),
                 http_client.clone(),
                 cx,
             );
@@ -254,6 +255,7 @@ impl HeadlessProject {
         session.add_entity_request_handler(Self::handle_open_new_buffer);
         session.add_entity_request_handler(Self::handle_find_search_candidates);
         session.add_entity_request_handler(Self::handle_open_server_settings);
+        session.add_entity_request_handler(Self::handle_get_directory_environment);
         session.add_entity_message_handler(Self::handle_toggle_lsp_logs);
 
         session.add_entity_request_handler(BufferStore::handle_update_buffer);
@@ -294,6 +296,7 @@ impl HeadlessProject {
             languages,
             extensions,
             git_store,
+            environment,
             _toolchain_store: toolchain_store,
         }
     }
@@ -763,6 +766,26 @@ impl HeadlessProject {
 
         Ok(proto::GetProcessesResponse { processes })
     }
+
+    async fn handle_get_directory_environment(
+        this: Entity<Self>,
+        envelope: TypedEnvelope<proto::GetDirectoryEnvironment>,
+        mut cx: AsyncApp,
+    ) -> Result<proto::DirectoryEnvironment> {
+        let shell = task::Shell::from_proto(envelope.payload.shell.context("missing shell")?)?;
+        let directory = PathBuf::from(envelope.payload.directory);
+        let environment = this
+            .update(&mut cx, |this, cx| {
+                this.environment.update(cx, |environment, cx| {
+                    environment.get_local_directory_environment(&shell, directory.into(), cx)
+                })
+            })?
+            .await
+            .context("failed to get directory environment")?
+            .into_iter()
+            .collect();
+        Ok(proto::DirectoryEnvironment { environment })
+    }
 }
 
 fn prompt_to_proto(

crates/task/src/task.rs 🔗

@@ -9,6 +9,7 @@ mod task_template;
 mod vscode_debug_format;
 mod vscode_format;
 
+use anyhow::Context as _;
 use collections::{HashMap, HashSet, hash_map};
 use gpui::SharedString;
 use schemars::JsonSchema;
@@ -361,6 +362,38 @@ impl Shell {
             Shell::System => ShellKind::system(),
         }
     }
+
+    pub fn from_proto(proto: proto::Shell) -> anyhow::Result<Self> {
+        let shell_type = proto.shell_type.context("invalid shell type")?;
+        let shell = match shell_type {
+            proto::shell::ShellType::System(_) => Self::System,
+            proto::shell::ShellType::Program(program) => Self::Program(program),
+            proto::shell::ShellType::WithArguments(program) => Self::WithArguments {
+                program: program.program,
+                args: program.args,
+                title_override: None,
+            },
+        };
+        Ok(shell)
+    }
+
+    pub fn to_proto(self) -> proto::Shell {
+        let shell_type = match self {
+            Shell::System => proto::shell::ShellType::System(proto::System {}),
+            Shell::Program(program) => proto::shell::ShellType::Program(program),
+            Shell::WithArguments {
+                program,
+                args,
+                title_override: _,
+            } => proto::shell::ShellType::WithArguments(proto::shell::WithArguments {
+                program,
+                args,
+            }),
+        };
+        proto::Shell {
+            shell_type: Some(shell_type),
+        }
+    }
 }
 
 type VsCodeEnvVariable = String;