From 2127b8d75868143386e816609001125a61f7bfb2 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 14 Jan 2026 10:41:03 +0100 Subject: [PATCH] remote: Fix ACP agents not working in docker (#46778) Closes https://github.com/zed-industries/zed/issues/45165 Release Notes: - Fixed external agents not working in docker remotes --- crates/project/src/agent_server_store.rs | 4 ++- crates/project/src/debugger/dap_store.rs | 5 ++- .../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(-) diff --git a/crates/project/src/agent_server_store.rs b/crates/project/src/agent_server_store.rs index 6ed482bc4068831be96ea6a3523a3219aed09f0a..ddd378b9b9f9b009e225f14d1aee155071306a87 100644 --- a/crates/project/src/agent_server_store.rs +++ b/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(( diff --git a/crates/project/src/debugger/dap_store.rs b/crates/project/src/debugger/dap_store.rs index ce4ff3e584e8bd3dbdae178e4225bafc5bb7eb83..5868083d91208f910f7fba388b2c2f7f9009c1ba 100644 --- a/crates/project/src/debugger/dap_store.rs +++ b/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, ) })?; diff --git a/crates/recent_projects/src/remote_connections.rs b/crates/recent_projects/src/remote_connections.rs index db8f743483f86fc8df56c7775decf8582f5cd24b..8298b8b039241c9b8c263748a755c8dbaf8c08ed 100644 --- a/crates/recent_projects/src/remote_connections.rs +++ b/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, path: &Path) -> boo &Default::default(), None, None, + Interactive::No, ) else { return false; }; diff --git a/crates/remote/src/remote.rs b/crates/remote/src/remote.rs index ceeeb4e6e171a6c50560f03acb0e39eeaed1f469..4513fb02e5b2311c015ce73156e874a7f610d55c 100644 --- a/crates/remote/src/remote.rs +++ b/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}; diff --git a/crates/remote/src/remote_client.rs b/crates/remote/src/remote_client.rs index 0976f1a0e739a0c27e959b8c0544cd66fbd53a4f..d5ce412e675920a8c9681f81d18745f34dcbfd46 100644 --- a/crates/remote/src/remote_client.rs +++ b/crates/remote/src/remote_client.rs @@ -112,6 +112,15 @@ pub struct CommandTemplate { pub env: HashMap, } +/// 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, working_dir: Option, port_forward: Option<(u16, String, u16)>, + ) -> Result { + self.build_command_with_options( + program, + args, + env, + working_dir, + port_forward, + Interactive::Yes, + ) + } + + pub fn build_command_with_options( + &self, + program: Option, + args: &[String], + env: &HashMap, + working_dir: Option, + port_forward: Option<(u16, String, u16)>, + interactive: Interactive, ) -> Result { 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, working_dir: Option, port_forward: Option<(u16, String, u16)>, + interactive: Interactive, ) -> Result; fn build_forward_ports_command( &self, diff --git a/crates/remote/src/transport/docker.rs b/crates/remote/src/transport/docker.rs index 379e7f7e622025ac55feeaae2899bb00b3150333..07cb9876f3468252b899993c7fafec1b5f1bb719 100644 --- a/crates/remote/src/transport/docker.rs +++ b/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, path_style: Option, - shell: Option, + 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, working_dir: Option, _port_forward: Option<(u16, String, u16)>, + interactive: Interactive, ) -> Result { 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 { diff --git a/crates/remote/src/transport/mock.rs b/crates/remote/src/transport/mock.rs index 01e74c2963e5115ace0c19825de86b5cd6cfb5ad..b7e0dc864f73085ab777a999afe1b4b656b5b1f8 100644 --- a/crates/remote/src/transport/mock.rs +++ b/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, Arc)>, + pending: HashMap, Arc)>, } impl Global for MockConnectionRegistry {} @@ -108,7 +109,7 @@ impl MockConnectionRegistry { &mut self, opts: &MockConnectionOptions, ) -> Option> + 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::() .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, _working_dir: Option, _port_forward: Option<(u16, String, u16)>, + _interactive: Interactive, ) -> Result { let shell_program = program.unwrap_or_else(|| "sh".to_string()); let mut shell_args = Vec::new(); diff --git a/crates/remote/src/transport/ssh.rs b/crates/remote/src/transport/ssh.rs index c1cf87291c18d453d7cc8acc25ccdb8e0a77211f..4504019cccaeef6a3a804c50b5390620c42efe8d 100644 --- a/crates/remote/src/transport/ssh.rs +++ b/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, working_dir: Option, port_forward: Option<(u16, String, u16)>, + interactive: Interactive, ) -> Result { 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, + interactive: Interactive, ) -> Result { 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"); diff --git a/crates/remote/src/transport/wsl.rs b/crates/remote/src/transport/wsl.rs index 94cd8bed8682fe2552fbfbfed088c92e25023647..18b4bcdfbdc169d11a4ec694ca2d7a7fef69289d 100644 --- a/crates/remote/src/transport/wsl.rs +++ b/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, working_dir: Option, port_forward: Option<(u16, String, u16)>, + _interactive: Interactive, ) -> Result { if port_forward.is_some() { bail!("WSL shares the network interface with the host system");