# Objective Fix the agent `terminal` tool in headless eval environments. In the eval sandbox, terminal commands failed before the shell ran with `IOError: Not a tty (os error 25)` because PTY setup attempted to acquire a controlling terminal. ## Solution - Add a `terminal::HeadlessTerminal` global that is set by `eval_cli` only. - When headless mode is enabled, run terminal task commands as plain subprocesses with piped stdout/stderr instead of opening a PTY. - Pump subprocess output through the existing terminal emulator/event channel so output capture, completion, and task killing keep working. - Build ACP terminal commands non-interactively in headless mode. - Keep the normal editor terminal path unchanged when the global is unset. - Handle non-PTY output edge cases by preserving split CRLF sequences and avoiding an indefinite wait if subprocess exit-status polling errors. ## Testing - `cargo fmt --all` - `cargo test -p util non_interactive_omits_interactive_flag` - `cargo test -p terminal test_no_pty_task_terminal_captures_output` - `cargo test -p terminal test_convert_lf_to_crlf_preserves_split_crlf` - `cargo test -p terminal test_write_output` - `cargo check -p eval_cli` - `git --no-pager diff --check` ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed the agent terminal tool failing with "Not a tty" in headless/eval environments
341 lines
11 KiB
Rust
341 lines
11 KiB
Rust
use std::borrow::Cow;
|
|
|
|
use crate::shell::get_system_shell;
|
|
use crate::shell::{Shell, ShellKind};
|
|
|
|
/// ShellBuilder is used to turn a user-requested task into a
|
|
/// program that can be executed by the shell.
|
|
pub struct ShellBuilder {
|
|
/// The shell to run
|
|
program: String,
|
|
args: Vec<String>,
|
|
interactive: bool,
|
|
/// Whether to redirect stdin to /dev/null for the spawned command as a subshell.
|
|
redirect_stdin: bool,
|
|
kind: ShellKind,
|
|
}
|
|
|
|
impl ShellBuilder {
|
|
/// Create a new ShellBuilder as configured.
|
|
pub fn new(shell: &Shell, is_windows: bool) -> Self {
|
|
let (program, args) = match shell {
|
|
Shell::System => (get_system_shell(), Vec::new()),
|
|
Shell::Program(shell) => (shell.clone(), Vec::new()),
|
|
Shell::WithArguments { program, args, .. } => (program.clone(), args.clone()),
|
|
};
|
|
|
|
let kind = ShellKind::new(&program, is_windows);
|
|
Self {
|
|
program,
|
|
args,
|
|
interactive: true,
|
|
kind,
|
|
redirect_stdin: false,
|
|
}
|
|
}
|
|
pub fn non_interactive(mut self) -> Self {
|
|
self.interactive = false;
|
|
self
|
|
}
|
|
|
|
/// Returns the label to show in the terminal tab
|
|
pub fn command_label(&self, command_to_use_in_label: &str) -> String {
|
|
if command_to_use_in_label.trim().is_empty() {
|
|
self.program.clone()
|
|
} else {
|
|
match self.kind {
|
|
ShellKind::PowerShell | ShellKind::Pwsh => {
|
|
format!("{} -C '{}'", self.program, command_to_use_in_label)
|
|
}
|
|
ShellKind::Cmd => {
|
|
format!("{} /C \"{}\"", self.program, command_to_use_in_label)
|
|
}
|
|
ShellKind::Posix
|
|
| ShellKind::Nushell
|
|
| ShellKind::Fish
|
|
| ShellKind::Csh
|
|
| ShellKind::Tcsh
|
|
| ShellKind::Rc
|
|
| ShellKind::Xonsh
|
|
| ShellKind::Elvish => {
|
|
let interactivity = self.interactive.then_some("-i ").unwrap_or_default();
|
|
format!(
|
|
"{PROGRAM} {interactivity}-c '{command_to_use_in_label}'",
|
|
PROGRAM = self.program
|
|
)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
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,
|
|
task_command: Option<String>,
|
|
task_args: &[String],
|
|
) -> (String, Vec<String>) {
|
|
if let Some(task_command) = task_command {
|
|
let task_command = if !task_args.is_empty() {
|
|
match self.kind.try_quote_prefix_aware(&task_command) {
|
|
Some(task_command) => task_command.into_owned(),
|
|
None => task_command,
|
|
}
|
|
} else {
|
|
task_command
|
|
};
|
|
let mut combined_command = task_args.iter().fold(task_command, |mut command, arg| {
|
|
command.push(' ');
|
|
let shell_variable = self.kind.to_shell_variable(arg);
|
|
command.push_str(&match self.kind.try_quote(&shell_variable) {
|
|
Some(shell_variable) => shell_variable,
|
|
None => Cow::Owned(shell_variable),
|
|
});
|
|
command
|
|
});
|
|
if self.redirect_stdin {
|
|
match self.kind {
|
|
ShellKind::Fish | ShellKind::Posix => {
|
|
combined_command.insert_str(0, "exec </dev/null; ");
|
|
}
|
|
ShellKind::Nushell
|
|
| ShellKind::Csh
|
|
| ShellKind::Tcsh
|
|
| ShellKind::Rc
|
|
| ShellKind::Xonsh
|
|
| ShellKind::Elvish => {
|
|
combined_command.insert(0, '(');
|
|
combined_command.push_str("\n) </dev/null");
|
|
}
|
|
ShellKind::PowerShell | ShellKind::Pwsh => {
|
|
combined_command.insert_str(0, "$null | & {");
|
|
combined_command.push_str("}");
|
|
}
|
|
ShellKind::Cmd => {
|
|
combined_command.push_str("< NUL");
|
|
}
|
|
}
|
|
}
|
|
|
|
self.args
|
|
.extend(self.kind.args_for_shell(self.interactive, combined_command));
|
|
}
|
|
|
|
(self.program, self.args)
|
|
}
|
|
|
|
// This should not exist, but our task infra is broken beyond repair right now
|
|
#[doc(hidden)]
|
|
pub fn build_no_quote(
|
|
mut self,
|
|
task_command: Option<String>,
|
|
task_args: &[String],
|
|
) -> (String, Vec<String>) {
|
|
if let Some(task_command) = task_command {
|
|
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::Fish | ShellKind::Posix => {
|
|
combined_command.insert_str(0, "exec </dev/null; ");
|
|
}
|
|
ShellKind::Nushell
|
|
| ShellKind::Csh
|
|
| ShellKind::Tcsh
|
|
| ShellKind::Rc
|
|
| ShellKind::Xonsh
|
|
| ShellKind::Elvish => {
|
|
combined_command.insert(0, '(');
|
|
combined_command.push_str("\n) </dev/null");
|
|
}
|
|
ShellKind::PowerShell | ShellKind::Pwsh => {
|
|
combined_command.insert_str(0, "$null | & {");
|
|
combined_command.push_str("}");
|
|
}
|
|
ShellKind::Cmd => {
|
|
combined_command.push_str("< NUL");
|
|
}
|
|
}
|
|
}
|
|
|
|
self.args
|
|
.extend(self.kind.args_for_shell(self.interactive, combined_command));
|
|
}
|
|
|
|
(self.program, self.args)
|
|
}
|
|
|
|
/// Builds a `smol::process::Command` with the given task command and arguments.
|
|
///
|
|
/// Prefer this over manually constructing a command with the output of `Self::build`,
|
|
/// as this method handles `cmd` weirdness on windows correctly.
|
|
pub fn build_smol_command(
|
|
self,
|
|
task_command: Option<String>,
|
|
task_args: &[String],
|
|
) -> smol::process::Command {
|
|
smol::process::Command::from(self.build_std_command(task_command, task_args))
|
|
}
|
|
|
|
/// Builds a `std::process::Command` with the given task command and arguments.
|
|
///
|
|
/// Prefer this over manually constructing a command with the output of `Self::build`,
|
|
/// as this method handles `cmd` weirdness on windows correctly.
|
|
pub fn build_std_command(
|
|
self,
|
|
mut task_command: Option<String>,
|
|
task_args: &[String],
|
|
) -> std::process::Command {
|
|
#[cfg(windows)]
|
|
let kind = self.kind;
|
|
if task_args.is_empty() {
|
|
task_command = task_command
|
|
.as_ref()
|
|
.map(|cmd| self.kind.try_quote_prefix_aware(&cmd).map(Cow::into_owned))
|
|
.unwrap_or(task_command);
|
|
}
|
|
let (program, args) = self.build(task_command, task_args);
|
|
|
|
let mut child = crate::command::new_std_command(program);
|
|
|
|
#[cfg(windows)]
|
|
if kind == ShellKind::Cmd {
|
|
use std::os::windows::process::CommandExt;
|
|
|
|
for arg in args {
|
|
child.raw_arg(arg);
|
|
}
|
|
} else {
|
|
child.args(args);
|
|
}
|
|
|
|
#[cfg(not(windows))]
|
|
child.args(args);
|
|
|
|
child
|
|
}
|
|
|
|
pub fn kind(&self) -> ShellKind {
|
|
self.kind
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod test {
|
|
use super::*;
|
|
|
|
#[test]
|
|
fn test_nu_shell_variable_substitution() {
|
|
let shell = Shell::Program("nu".to_owned());
|
|
let shell_builder = ShellBuilder::new(&shell, false);
|
|
|
|
let (program, args) = shell_builder.build(
|
|
Some("echo".into()),
|
|
&[
|
|
"${hello}".to_string(),
|
|
"$world".to_string(),
|
|
"nothing".to_string(),
|
|
"--$something".to_string(),
|
|
"$".to_string(),
|
|
"${test".to_string(),
|
|
],
|
|
);
|
|
|
|
assert_eq!(program, "nu");
|
|
assert_eq!(
|
|
args,
|
|
vec![
|
|
"-i",
|
|
"-c",
|
|
"echo '$env.hello' '$env.world' nothing '--($env.something)' '$' '${test'"
|
|
]
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn redirect_stdin_to_dev_null_precedence() {
|
|
let shell = Shell::Program("nu".to_owned());
|
|
let shell_builder = ShellBuilder::new(&shell, false);
|
|
|
|
let (program, args) = shell_builder
|
|
.redirect_stdin_to_dev_null()
|
|
.build(Some("echo".into()), &["nothing".to_string()]);
|
|
|
|
assert_eq!(program, "nu");
|
|
assert_eq!(args, vec!["-i", "-c", "(echo nothing\n) </dev/null"]);
|
|
}
|
|
|
|
#[test]
|
|
fn redirect_stdin_to_dev_null_fish() {
|
|
let shell = Shell::Program("fish".to_owned());
|
|
let shell_builder = ShellBuilder::new(&shell, false);
|
|
|
|
let (program, args) = shell_builder
|
|
.redirect_stdin_to_dev_null()
|
|
.build(Some("echo".into()), &["test".to_string()]);
|
|
|
|
assert_eq!(program, "fish");
|
|
assert_eq!(args, vec!["-i", "-c", "exec </dev/null; echo test"]);
|
|
}
|
|
|
|
#[test]
|
|
fn redirect_stdin_to_dev_null_preserves_heredoc() {
|
|
let shell = Shell::Program("sh".to_owned());
|
|
let shell_builder = ShellBuilder::new(&shell, false);
|
|
|
|
let command = "cat <<EOF\nhello\nEOF";
|
|
let (program, args) = shell_builder
|
|
.redirect_stdin_to_dev_null()
|
|
.build(Some(command.into()), &[]);
|
|
|
|
assert_eq!(program, "sh");
|
|
assert_eq!(
|
|
args,
|
|
vec!["-i", "-c", "exec </dev/null; cat <<EOF\nhello\nEOF"]
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn non_interactive_omits_interactive_flag() {
|
|
// Headless hosts (e.g. the eval CLI) build the agent's shell command
|
|
// non-interactively so it works without a controlling TTY.
|
|
let shell = Shell::Program("sh".to_owned());
|
|
let shell_builder = ShellBuilder::new(&shell, false).non_interactive();
|
|
|
|
let (program, args) = shell_builder.build(Some("echo hello".into()), &[]);
|
|
|
|
assert_eq!(program, "sh");
|
|
assert_eq!(args, vec!["-c", "echo hello"]);
|
|
assert!(
|
|
!args.iter().any(|arg| arg == "-i"),
|
|
"non-interactive shell command must not include `-i`"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn does_not_quote_sole_command_only() {
|
|
let shell = Shell::Program("fish".to_owned());
|
|
let shell_builder = ShellBuilder::new(&shell, false);
|
|
|
|
let (program, args) = shell_builder.build(Some("echo".into()), &[]);
|
|
|
|
assert_eq!(program, "fish");
|
|
assert_eq!(args, vec!["-i", "-c", "echo"]);
|
|
|
|
let shell = Shell::Program("fish".to_owned());
|
|
let shell_builder = ShellBuilder::new(&shell, false);
|
|
|
|
let (program, args) = shell_builder.build(Some("echo oo".into()), &[]);
|
|
|
|
assert_eq!(program, "fish");
|
|
assert_eq!(args, vec!["-i", "-c", "echo oo"]);
|
|
}
|
|
}
|