remote: Fix ACP agents not working in docker (#46778)

Lukas Wirth created

Closes https://github.com/zed-industries/zed/issues/45165

Release Notes:

- Fixed external agents not working in docker remotes

Change summary

crates/project/src/agent_server_store.rs         |  4 +
crates/project/src/debugger/dap_store.rs         |  5 ++
crates/recent_projects/src/remote_connections.rs |  3 +
crates/remote/src/remote.rs                      |  6 +-
crates/remote/src/remote_client.rs               | 31 +++++++++++++++
crates/remote/src/transport/docker.rs            | 23 +++++++----
crates/remote/src/transport/mock.rs              | 10 +++--
crates/remote/src/transport/ssh.rs               | 36 ++++++++++++++++-
crates/remote/src/transport/wsl.rs               |  3 +
9 files changed, 97 insertions(+), 24 deletions(-)

Detailed changes

crates/project/src/agent_server_store.rs 🔗

@@ -1,3 +1,4 @@
+use remote::Interactive;
 use std::{
     any::Any,
     borrow::Borrow,
@@ -1315,12 +1316,13 @@ impl ExternalAgentServer for RemoteExternalAgentServer {
             let root_dir = response.root_dir;
             response.env.extend(extra_env);
             let command = upstream_client.update(cx, |client, _| {
-                client.build_command(
+                client.build_command_with_options(
                     Some(response.path),
                     &response.args,
                     &response.env.into_iter().collect(),
                     Some(root_dir.clone()),
                     None,
+                    Interactive::No,
                 )
             })??;
             Ok((

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

@@ -4,6 +4,8 @@ use super::{
     locators,
     session::{self, Session, SessionStateEvent},
 };
+use remote::Interactive;
+
 use crate::{
     InlayHint, InlayHintLabel, ProjectEnvironment, ResolveState,
     debugger::session::SessionQuirks,
@@ -341,12 +343,13 @@ impl DapStore {
                     }
 
                     let command = remote.read_with(cx, |remote, _cx| {
-                        remote.build_command(
+                        remote.build_command_with_options(
                             binary.command,
                             &binary.arguments,
                             &binary.envs,
                             binary.cwd.map(|path| path.display().to_string()),
                             port_forwarding,
+                            Interactive::No,
                         )
                     })?;
 

crates/recent_projects/src/remote_connections.rs 🔗

@@ -19,7 +19,7 @@ use markdown::{Markdown, MarkdownElement, MarkdownStyle};
 use project::trusted_worktrees;
 use release_channel::ReleaseChannel;
 use remote::{
-    ConnectionIdentifier, DockerConnectionOptions, RemoteClient, RemoteConnection,
+    ConnectionIdentifier, DockerConnectionOptions, Interactive, RemoteClient, RemoteConnection,
     RemoteConnectionOptions, RemotePlatform, SshConnectionOptions,
 };
 use semver::Version;
@@ -891,6 +891,7 @@ async fn path_exists(connection: &Arc<dyn RemoteConnection>, path: &Path) -> boo
         &Default::default(),
         None,
         None,
+        Interactive::No,
     ) else {
         return false;
     };

crates/remote/src/remote.rs 🔗

@@ -7,9 +7,9 @@ mod transport;
 #[cfg(target_os = "windows")]
 pub use remote_client::OpenWslPath;
 pub use remote_client::{
-    ConnectionIdentifier, ConnectionState, RemoteArch, RemoteClient, RemoteClientDelegate,
-    RemoteClientEvent, RemoteConnection, RemoteConnectionOptions, RemoteOs, RemotePlatform,
-    connect,
+    ConnectionIdentifier, ConnectionState, Interactive, RemoteArch, RemoteClient,
+    RemoteClientDelegate, RemoteClientEvent, RemoteConnection, RemoteConnectionOptions, RemoteOs,
+    RemotePlatform, connect,
 };
 pub use transport::docker::DockerConnectionOptions;
 pub use transport::ssh::{SshConnectionOptions, SshPortForwardOption};

crates/remote/src/remote_client.rs 🔗

@@ -112,6 +112,15 @@ pub struct CommandTemplate {
     pub env: HashMap<String, String>,
 }
 
+/// Whether a command should be run with TTY allocation for interactive use.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum Interactive {
+    /// Allocate a pseudo-TTY for interactive terminal use.
+    Yes,
+    /// Do not allocate a TTY - for commands that communicate via piped stdio.
+    No,
+}
+
 pub trait RemoteClientDelegate: Send + Sync {
     fn ask_password(
         &self,
@@ -897,11 +906,30 @@ impl RemoteClient {
         env: &HashMap<String, String>,
         working_dir: Option<String>,
         port_forward: Option<(u16, String, u16)>,
+    ) -> Result<CommandTemplate> {
+        self.build_command_with_options(
+            program,
+            args,
+            env,
+            working_dir,
+            port_forward,
+            Interactive::Yes,
+        )
+    }
+
+    pub fn build_command_with_options(
+        &self,
+        program: Option<String>,
+        args: &[String],
+        env: &HashMap<String, String>,
+        working_dir: Option<String>,
+        port_forward: Option<(u16, String, u16)>,
+        interactive: Interactive,
     ) -> Result<CommandTemplate> {
         let Some(connection) = self.remote_connection() else {
             return Err(anyhow!("no remote connection"));
         };
-        connection.build_command(program, args, env, working_dir, port_forward)
+        connection.build_command(program, args, env, working_dir, port_forward, interactive)
     }
 
     pub fn build_forward_ports_command(
@@ -1282,6 +1310,7 @@ pub trait RemoteConnection: Send + Sync {
         env: &HashMap<String, String>,
         working_dir: Option<String>,
         port_forward: Option<(u16, String, u16)>,
+        interactive: Interactive,
     ) -> Result<CommandTemplate>;
     fn build_forward_ports_command(
         &self,

crates/remote/src/transport/docker.rs 🔗

@@ -25,7 +25,8 @@ use rpc::proto::Envelope;
 
 use crate::{
     RemoteClientDelegate, RemoteConnection, RemoteConnectionOptions, RemoteOs, RemotePlatform,
-    remote_client::CommandTemplate, transport::parse_platform,
+    remote_client::{CommandTemplate, Interactive},
+    transport::parse_platform,
 };
 
 #[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
@@ -42,7 +43,7 @@ pub(crate) struct DockerExecConnection {
     connection_options: DockerConnectionOptions,
     remote_platform: Option<RemotePlatform>,
     path_style: Option<PathStyle>,
-    shell: Option<String>,
+    shell: String,
 }
 
 impl DockerExecConnection {
@@ -58,7 +59,7 @@ impl DockerExecConnection {
             connection_options,
             remote_platform: None,
             path_style: None,
-            shell: None,
+            shell: "sh".to_owned(),
         };
         let (release_channel, version, commit) = cx.update(|cx| {
             (
@@ -75,8 +76,10 @@ impl DockerExecConnection {
         };
 
         this.remote_platform = Some(remote_platform);
+        log::info!("Remote platform discovered: {:?}", this.remote_platform);
 
-        this.shell = Some(this.discover_shell().await);
+        this.shell = this.discover_shell().await;
+        log::info!("Remote shell discovered: {}", this.shell);
 
         this.remote_dir_for_server = this.docker_user_home_dir().await?.trim().to_string();
 
@@ -404,6 +407,7 @@ impl DockerExecConnection {
             command.arg(arg.as_ref());
         }
         let output = command.output().await?;
+        log::debug!("{:?}: {:?}", command, output);
         anyhow::ensure!(
             output.status.success(),
             "failed to run command {command:?}: {}",
@@ -651,6 +655,7 @@ impl RemoteConnection for DockerExecConnection {
         env: &HashMap<String, String>,
         working_dir: Option<String>,
         _port_forward: Option<(u16, String, u16)>,
+        interactive: Interactive,
     ) -> Result<CommandTemplate> {
         let mut parsed_working_dir = None;
 
@@ -692,7 +697,10 @@ impl RemoteConnection for DockerExecConnection {
             docker_args.push(format!("{}={}", k, v));
         }
 
-        docker_args.push("-it".to_string());
+        match interactive {
+            Interactive::Yes => docker_args.push("-it".to_string()),
+            Interactive::No => docker_args.push("-i".to_string()),
+        }
         docker_args.push(self.connection_options.container_id.to_string());
 
         docker_args.append(&mut inner_program);
@@ -721,10 +729,7 @@ impl RemoteConnection for DockerExecConnection {
     }
 
     fn shell(&self) -> String {
-        match &self.shell {
-            Some(shell) => shell.clone(),
-            None => self.default_system_shell(),
-        }
+        self.shell.clone()
     }
 
     fn default_system_shell(&self) -> String {

crates/remote/src/transport/mock.rs 🔗

@@ -30,7 +30,8 @@
 //! ```
 
 use crate::remote_client::{
-    ChannelClient, CommandTemplate, RemoteClientDelegate, RemoteConnection, RemoteConnectionOptions,

+    ChannelClient, CommandTemplate, Interactive, RemoteClientDelegate, RemoteConnection,

+    RemoteConnectionOptions,

 };
 use anyhow::Result;
 use async_trait::async_trait;
@@ -97,7 +98,7 @@ unsafe impl Sync for SendableCx {}
 /// it retrieves the connection from this registry.
 #[derive(Default)]
 pub struct MockConnectionRegistry {
-    pending: HashMap<MockConnectionOptions, (oneshot::Receiver<()>, Arc<MockRemoteConnection>)>,

+    pending: HashMap<u64, (oneshot::Receiver<()>, Arc<MockRemoteConnection>)>,

 }
 
 impl Global for MockConnectionRegistry {}
@@ -108,7 +109,7 @@ impl MockConnectionRegistry {
         &mut self,
         opts: &MockConnectionOptions,
     ) -> Option<impl Future<Output = Arc<MockRemoteConnection>> + use<>> {
-        let (guard, con) = self.pending.remove(opts)?;

+        let (guard, con) = self.pending.remove(&opts.id)?;

         Some(async move {
             _ = guard.await;
             con
@@ -161,7 +162,7 @@ impl MockConnection {
         client_cx.update(|cx| {
             cx.default_global::<MockConnectionRegistry>()
                 .pending
-                .insert(opts.clone(), (rx, connection));

+                .insert(opts.id, (rx, connection));

         });
 
         (opts, server_client.into(), tx)
@@ -185,6 +186,7 @@ impl RemoteConnection for MockRemoteConnection {
         env: &HashMap<String, String>,
         _working_dir: Option<String>,
         _port_forward: Option<(u16, String, u16)>,
+        _interactive: Interactive,

     ) -> Result<CommandTemplate> {
         let shell_program = program.unwrap_or_else(|| "sh".to_string());
         let mut shell_args = Vec::new();

crates/remote/src/transport/ssh.rs 🔗

@@ -1,6 +1,6 @@
 use crate::{
     RemoteArch, RemoteClientDelegate, RemoteOs, RemotePlatform,
-    remote_client::{CommandTemplate, RemoteConnection, RemoteConnectionOptions},
+    remote_client::{CommandTemplate, Interactive, RemoteConnection, RemoteConnectionOptions},
     transport::{parse_platform, parse_shell},
 };
 use anyhow::{Context as _, Result, anyhow};
@@ -292,6 +292,7 @@ impl RemoteConnection for SshRemoteConnection {
         input_env: &HashMap<String, String>,
         working_dir: Option<String>,
         port_forward: Option<(u16, String, u16)>,
+        interactive: Interactive,
     ) -> Result<CommandTemplate> {
         let Self {
             ssh_path_style,
@@ -312,6 +313,7 @@ impl RemoteConnection for SshRemoteConnection {
             ssh_shell,
             *ssh_shell_kind,
             socket.ssh_args(),
+            interactive,
         )
     }
 
@@ -1487,6 +1489,7 @@ fn build_command(
     ssh_shell: &str,
     ssh_shell_kind: ShellKind,
     ssh_args: Vec<String>,
+    interactive: Interactive,
 ) -> Result<CommandTemplate> {
     use std::fmt::Write as _;
 
@@ -1556,7 +1559,12 @@ fn build_command(
     // -q suppresses the "Connection to ... closed." message that SSH prints when
     // the connection terminates with -t (pseudo-terminal allocation)
     args.push("-q".into());
-    args.push("-t".into());
+    match interactive {
+        // -t forces pseudo-TTY allocation (for interactive use)
+        Interactive::Yes => args.push("-t".into()),
+        // -T disables pseudo-TTY allocation (for non-interactive piped stdio)
+        Interactive::No => args.push("-T".into()),
+    }
     args.push(exec);
 
     Ok(CommandTemplate {
@@ -1577,6 +1585,26 @@ mod tests {
         let mut env = HashMap::default();
         env.insert("SSH_VAR".to_string(), "ssh-val".to_string());
 
+        // Test non-interactive command (interactive=false should use -T)
+        let command = build_command(
+            Some("remote_program".to_string()),
+            &["arg1".to_string(), "arg2".to_string()],
+            &input_env,
+            Some("~/work".to_string()),
+            None,
+            env.clone(),
+            PathStyle::Posix,
+            "/bin/bash",
+            ShellKind::Posix,
+            vec!["-o".to_string(), "ControlMaster=auto".to_string()],
+            Interactive::No,
+        )?;
+        assert_eq!(command.program, "ssh");
+        // Should contain -T for non-interactive
+        assert!(command.args.iter().any(|arg| arg == "-T"));
+        assert!(!command.args.iter().any(|arg| arg == "-t"));
+
+        // Test interactive command (interactive=true should use -t)
         let command = build_command(
             Some("remote_program".to_string()),
             &["arg1".to_string(), "arg2".to_string()],
@@ -1588,6 +1616,7 @@ mod tests {
             "/bin/fish",
             ShellKind::Fish,
             vec!["-p".to_string(), "2222".to_string()],
+            Interactive::Yes,
         )?;
 
         assert_eq!(command.program, "ssh");
@@ -1610,7 +1639,7 @@ mod tests {
 
         let command = build_command(
             None,
-            &["arg1".to_string(), "arg2".to_string()],
+            &[],
             &input_env,
             None,
             Some((1, "foo".to_owned(), 2)),
@@ -1619,6 +1648,7 @@ mod tests {
             "/bin/fish",
             ShellKind::Fish,
             vec!["-p".to_string(), "2222".to_string()],
+            Interactive::Yes,
         )?;
 
         assert_eq!(command.program, "ssh");

crates/remote/src/transport/wsl.rs 🔗

@@ -1,6 +1,6 @@
 use crate::{
     RemoteArch, RemoteClientDelegate, RemoteOs, RemotePlatform,
-    remote_client::{CommandTemplate, RemoteConnection, RemoteConnectionOptions},
+    remote_client::{CommandTemplate, Interactive, RemoteConnection, RemoteConnectionOptions},
     transport::{parse_platform, parse_shell},
 };
 use anyhow::{Context, Result, anyhow, bail};
@@ -421,6 +421,7 @@ impl RemoteConnection for WslRemoteConnection {
         env: &HashMap<String, String>,
         working_dir: Option<String>,
         port_forward: Option<(u16, String, u16)>,
+        _interactive: Interactive,
     ) -> Result<CommandTemplate> {
         if port_forward.is_some() {
             bail!("WSL shares the network interface with the host system");