From e059b6a31d117c28aa950d6a6d7b43883f084ee5 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 29 Sep 2025 11:26:22 +0200 Subject: [PATCH] acp_thread: Fix terminal tool incorrectly redirecting stdin to `/dev/null` --- crates/acp_thread/src/acp_thread.rs | 8 ++-- crates/assistant_tools/src/terminal_tool.rs | 45 +++++++++++---------- crates/task/src/shell_builder.rs | 20 ++++++++- 3 files changed, 45 insertions(+), 28 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 0e9cc921cd4a2f6961cd140d974cd612aeabbe0d..d3bc9a2055c30b6314cf11148abd344bc6729bb0 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -1985,7 +1985,7 @@ impl AcpThread { let terminal_id = terminal_id.clone(); async move |_this, cx| { let env = env.await; - let (command, args) = ShellBuilder::new( + let (task_command, task_args) = ShellBuilder::new( project .update(cx, |project, cx| { project @@ -1996,13 +1996,13 @@ impl AcpThread { &Shell::Program(get_default_system_shell()), ) .redirect_stdin_to_dev_null() - .build(Some(command), &args); + .build(Some(command.clone()), &args); let terminal = project .update(cx, |project, cx| { project.create_terminal_task( task::SpawnInTerminal { - command: Some(command.clone()), - args: args.clone(), + command: Some(task_command), + args: task_args, cwd: cwd.clone(), env, ..Default::default() diff --git a/crates/assistant_tools/src/terminal_tool.rs b/crates/assistant_tools/src/terminal_tool.rs index 8014a39e23137ad71b91e5c24d5d79699b530e5d..b1b2313bcd1895f5c92cd71ad5291b4622a2e6d8 100644 --- a/crates/assistant_tools/src/terminal_tool.rs +++ b/crates/assistant_tools/src/terminal_tool.rs @@ -139,18 +139,25 @@ impl Tool for TerminalTool { env }); + let build_cmd = { + let input_command = input.command.clone(); + move || { + ShellBuilder::new( + remote_shell.as_deref(), + &Shell::Program(get_default_system_shell()), + ) + .redirect_stdin_to_dev_null() + .build(Some(input_command.clone()), &[]) + } + }; + let Some(window) = window else { // Headless setup, a test or eval. Our terminal subsystem requires a workspace, // so bypass it and provide a convincing imitation using a pty. let task = cx.background_spawn(async move { let env = env.await; let pty_system = native_pty_system(); - let (command, args) = ShellBuilder::new( - remote_shell.as_deref(), - &Shell::Program(get_default_system_shell()), - ) - .redirect_stdin_to_dev_null() - .build(Some(input.command.clone()), &[]); + let (command, args) = build_cmd(); let mut cmd = CommandBuilder::new(command); cmd.args(args); for (k, v) in env { @@ -187,16 +194,10 @@ impl Tool for TerminalTool { }; }; - let command = input.command.clone(); let terminal = cx.spawn({ let project = project.downgrade(); async move |cx| { - let (command, args) = ShellBuilder::new( - remote_shell.as_deref(), - &Shell::Program(get_default_system_shell()), - ) - .redirect_stdin_to_dev_null() - .build(Some(input.command), &[]); + let (command, args) = build_cmd(); let env = env.await; project .update(cx, |project, cx| { @@ -215,18 +216,18 @@ impl Tool for TerminalTool { } }); - let command_markdown = - cx.new(|cx| Markdown::new(format!("```bash\n{}\n```", command).into(), None, None, cx)); - - let card = cx.new(|cx| { - TerminalToolCard::new( - command_markdown.clone(), - working_dir.clone(), - cx.entity_id(), + let command_markdown = cx.new(|cx| { + Markdown::new( + format!("```bash\n{}\n```", input.command).into(), + None, + None, cx, ) }); + let card = + cx.new(|cx| TerminalToolCard::new(command_markdown, working_dir, cx.entity_id(), cx)); + let output = cx.spawn({ let card = card.clone(); async move |cx| { @@ -267,7 +268,7 @@ impl Tool for TerminalTool { let previous_len = content.len(); let (processed_content, finished_with_empty_output) = process_content( &content, - &command, + &input.command, exit_status.map(portable_pty::ExitStatus::from), ); diff --git a/crates/task/src/shell_builder.rs b/crates/task/src/shell_builder.rs index c3f0646c02cc427a07505c2ff30157e84d2ca0fe..e53ee38ec58ce09a74aabae9eb43a7e66625d2b3 100644 --- a/crates/task/src/shell_builder.rs +++ b/crates/task/src/shell_builder.rs @@ -210,6 +210,7 @@ pub struct ShellBuilder { program: String, args: Vec, interactive: bool, + /// Whether to redirect stdin to /dev/null for the spawned command as a subshell. redirect_stdin: bool, kind: ShellKind, } @@ -279,10 +280,12 @@ impl ShellBuilder { if self.redirect_stdin { match self.kind { ShellKind::Posix | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh => { - combined_command.push_str(" { - combined_command.insert_str(0, "$null | "); + combined_command.insert_str(0, "$null | {"); + combined_command.push_str("}"); } ShellKind::Cmd => { combined_command.push_str("< NUL"); @@ -329,4 +332,17 @@ mod test { ] ); } + + #[test] + fn redirect_stdin_to_dev_null_precedence() { + let shell = Shell::Program("nu".to_owned()); + let shell_builder = ShellBuilder::new(None, &shell); + + 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)