task: Do not wrap tasks in extra shell invocations unless requested

Lukas Wirth created

Change summary

crates/agent_ui/src/acp/thread_view.rs          |   9 -
crates/debugger_ui/src/new_process_modal.rs     |  18 ++
crates/debugger_ui/src/session/running.rs       |   4 
crates/editor/src/editor.rs                     |  14 +
crates/editor/src/lsp_ext.rs                    |  11 +
crates/project/src/debugger/locators/go.rs      |   8 
crates/project/src/debugger/locators/python.rs  |   2 
crates/project/src/lsp_store/lsp_ext_command.rs |   9 +
crates/project/src/project_tests.rs             |   7 +
crates/project/src/task_inventory.rs            |  26 ++-
crates/project/src/task_store.rs                |   3 
crates/proto/proto/task.proto                   |   1 
crates/task/src/task.rs                         |   1 
crates/task/src/task_template.rs                | 113 +++++++++++++++---
crates/tasks_ui/src/modal.rs                    |  39 +++++
crates/terminal_view/src/terminal_panel.rs      |  78 +++---------
crates/workspace/src/tasks.rs                   |  11 +
17 files changed, 226 insertions(+), 128 deletions(-)

Detailed changes

crates/agent_ui/src/acp/thread_view.rs 🔗

@@ -1628,11 +1628,8 @@ impl AcpThreadView {
 
         window.spawn(cx, async move |cx| {
             let mut task = login.clone();
-            task.shell = task::Shell::WithArguments {
-                program: task.command.take().expect("login command should be set"),
-                args: std::mem::take(&mut task.args),
-                title_override: None
-            };
+            task.command = task.command.clone();
+            task.args = task.args.clone();
             task.full_label = task.label.clone();
             task.id = task::TaskId(format!("external-agent-{}-login", task.label));
             task.command_label = task.label.clone();
@@ -1641,7 +1638,7 @@ impl AcpThreadView {
             task.hide = task::HideStrategy::Always;
 
             let terminal = terminal_panel.update_in(cx, |terminal_panel, window, cx| {
-                terminal_panel.spawn_task(&task, window, cx)
+                terminal_panel.spawn_task(task, window, cx)
             })?;
 
             let terminal = terminal.await?;

crates/debugger_ui/src/new_process_modal.rs 🔗

@@ -193,8 +193,22 @@ impl NewProcessModal {
 
                             let (used_tasks, current_resolved_tasks) = task_inventory
                                 .update(cx, |task_inventory, cx| {
-                                    task_inventory
-                                        .used_and_current_resolved_tasks(task_contexts.clone(), cx)
+                                    let remote_shell = workspace
+                                        .read_with(cx, |workspace, cx| {
+                                            workspace
+                                                .project()
+                                                .read(cx)
+                                                .remote_client()?
+                                                .read(cx)
+                                                .shell()
+                                        })
+                                        .ok()
+                                        .flatten();
+                                    task_inventory.used_and_current_resolved_tasks(
+                                        task_contexts.clone(),
+                                        move || remote_shell.clone(),
+                                        cx,
+                                    )
                                 })?
                                 .await;
 

crates/debugger_ui/src/session/running.rs 🔗

@@ -989,7 +989,7 @@ impl RunningState {
                         (task, None)
                     }
                 };
-                let Some(mut task) = task_template.resolve_task("debug-build-task", &task_context) else {
+                let Some(mut task) = task_template.resolve_task("debug-build-task",&|| remote_shell.clone(), &task_context) else {
                     anyhow::bail!("Could not resolve task variables within a debug scenario");
                 };
 
@@ -1026,7 +1026,7 @@ impl RunningState {
                     None
                 };
 
-                if let Some(remote_shell) = remote_shell && task.resolved.shell == Shell::System {
+                if task.resolved.shell == Shell::System && let Some(remote_shell) = remote_shell.clone() {
                     task.resolved.shell = Shell::Program(remote_shell);
                 }
 

crates/editor/src/editor.rs 🔗

@@ -810,11 +810,12 @@ struct RunnableTasks {
 impl RunnableTasks {
     fn resolve<'a>(
         &'a self,
+        remote_shell: &'a dyn Fn() -> Option<String>,
         cx: &'a task::TaskContext,
     ) -> impl Iterator<Item = (TaskSourceKind, ResolvedTask)> + 'a {
         self.templates.iter().filter_map(|(kind, template)| {
             template
-                .resolve_task(&kind.to_id_base(), cx)
+                .resolve_task(&kind.to_id_base(), remote_shell, cx)
                 .map(|task| (kind.clone(), task))
         })
     }
@@ -6002,13 +6003,14 @@ impl Editor {
             Some(CodeActionSource::Indicator(_)) => Task::ready(Ok(Default::default())),
             _ => {
                 let mut task_context_task = Task::ready(None);
+                let remote_shell =
+                    maybe!({ project.as_ref()?.read(cx).remote_client()?.read(cx).shell() });
                 if let Some(tasks) = &tasks
                     && let Some(project) = project
                 {
                     task_context_task =
                         Self::build_tasks_context(&project, &buffer, buffer_row, tasks, cx);
                 }
-
                 cx.spawn_in(window, {
                     let buffer = buffer.clone();
                     async move |editor, cx| {
@@ -6018,7 +6020,9 @@ impl Editor {
                             tasks
                                 .zip(task_context.clone())
                                 .map(|(tasks, task_context)| ResolvedTasks {
-                                    templates: tasks.resolve(&task_context).collect(),
+                                    templates: tasks
+                                        .resolve(&|| remote_shell.clone(), &task_context)
+                                        .collect(),
                                     position: snapshot.buffer_snapshot().anchor_before(Point::new(
                                         multibuffer_point.row,
                                         tasks.column,
@@ -8320,9 +8324,11 @@ impl Editor {
 
         let reveal_strategy = action.reveal;
         let task_context = Self::build_tasks_context(&project, &buffer, buffer_row, &tasks, cx);
+        let remote_shell = maybe!({ project.read(cx).remote_client()?.read(cx).shell() });
         cx.spawn_in(window, async move |_, cx| {
             let context = task_context.await?;
-            let (task_source_kind, mut resolved_task) = tasks.resolve(&context).next()?;
+            let (task_source_kind, mut resolved_task) =
+                tasks.resolve(&|| remote_shell.clone(), &context).next()?;
 
             let resolved = &mut resolved_task.resolved;
             resolved.reveal = reveal_strategy;

crates/editor/src/lsp_ext.rs 🔗

@@ -21,7 +21,7 @@ use task::ResolvedTask;
 use task::TaskContext;
 use text::BufferId;
 use ui::SharedString;
-use util::ResultExt as _;
+use util::{ResultExt as _, maybe};
 
 pub(crate) fn find_specific_language_server_in_selection<F>(
     editor: &Editor,
@@ -114,7 +114,7 @@ pub fn lsp_tasks(
             server_id.zip(Some(buffers))
         })
         .collect::<Vec<_>>();
-
+    let remote_shell = maybe!({ project.read(cx).remote_client()?.read(cx).shell() });
     cx.spawn(async move |cx| {
         cx.spawn(async move |cx| {
             let mut lsp_tasks = HashMap::default();
@@ -151,8 +151,11 @@ pub fn lsp_tasks(
                     {
                         new_lsp_tasks.extend(new_runnables.runnables.into_iter().filter_map(
                             |(location, runnable)| {
-                                let resolved_task =
-                                    runnable.resolve_task(&id_base, &lsp_buffer_context)?;
+                                let resolved_task = runnable.resolve_task(
+                                    &id_base,
+                                    &|| remote_shell.clone(),
+                                    &lsp_buffer_context,
+                                )?;
                                 Some((location, resolved_task))
                             },
                         ));

crates/project/src/debugger/locators/go.rs 🔗

@@ -246,7 +246,7 @@ impl DapLocator for GoLocator {
 mod tests {
     use super::*;
     use gpui::TestAppContext;
-    use task::{HideStrategy, RevealStrategy, RevealTarget, Shell, TaskTemplate};
+    use task::{HideStrategy, RevealStrategy, RevealTarget, TaskTemplate};
 
     #[gpui::test]
     async fn test_create_scenario_for_go_build(_: &mut TestAppContext) {
@@ -262,7 +262,7 @@ mod tests {
             reveal: RevealStrategy::Always,
             reveal_target: RevealTarget::Dock,
             hide: HideStrategy::Never,
-            shell: Shell::System,
+            shell: None,
             tags: vec![],
             show_summary: true,
             show_command: true,
@@ -289,7 +289,7 @@ mod tests {
             reveal: RevealStrategy::Always,
             reveal_target: RevealTarget::Dock,
             hide: HideStrategy::Never,
-            shell: Shell::System,
+            shell: None,
             tags: vec![],
             show_summary: true,
             show_command: true,
@@ -427,7 +427,7 @@ mod tests {
             reveal: RevealStrategy::Always,
             reveal_target: RevealTarget::Dock,
             hide: HideStrategy::Never,
-            shell: Shell::System,
+            shell: None,
             tags: vec![],
             show_summary: true,
             show_command: true,

crates/project/src/debugger/locators/python.rs 🔗

@@ -116,7 +116,7 @@ mod test {
             reveal_target: task::RevealTarget::Dock,
             hide: task::HideStrategy::Never,
             tags: vec!["python-module-main-method".into()],
-            shell: task::Shell::System,
+            shell: None,
             show_summary: false,
             show_command: false,
         };

crates/project/src/lsp_store/lsp_ext_command.rs 🔗

@@ -23,7 +23,7 @@ use std::{
     path::{Path, PathBuf},
     sync::Arc,
 };
-use task::TaskTemplate;
+use task::{ShellKind, TaskTemplate};
 use text::{BufferId, PointUtf16, ToPointUtf16};
 
 pub enum LspExtExpandMacro {}
@@ -657,7 +657,12 @@ impl LspCommand for GetLspRunnables {
                     );
                     task_template.args.extend(cargo.cargo_args);
                     if !cargo.executable_args.is_empty() {
-                        let shell_kind = task_template.shell.shell_kind(cfg!(windows));
+                        let shell_kind = task_template
+                            .shell
+                            .as_ref()
+                            .map_or_else(ShellKind::system, |shell| {
+                                shell.shell_kind(cfg!(windows))
+                            });
                         task_template.args.push("--".to_string());
                         task_template.args.extend(
                             cargo

crates/project/src/project_tests.rs 🔗

@@ -9812,9 +9812,14 @@ fn get_all_tasks(
     cx: &mut App,
 ) -> Task<Vec<(TaskSourceKind, ResolvedTask)>> {
     let new_tasks = project.update(cx, |project, cx| {
+        let remote_shell = project.remote_client().and_then(|it| it.read(cx).shell());
         project.task_store.update(cx, |task_store, cx| {
             task_store.task_inventory().unwrap().update(cx, |this, cx| {
-                this.used_and_current_resolved_tasks(task_contexts, cx)
+                this.used_and_current_resolved_tasks(
+                    task_contexts,
+                    move || remote_shell.clone(),
+                    cx,
+                )
             })
         })
     });

crates/project/src/task_inventory.rs 🔗

@@ -424,6 +424,7 @@ impl Inventory {
     pub fn used_and_current_resolved_tasks(
         &self,
         task_contexts: Arc<TaskContexts>,
+        remote_shell: impl 'static + Fn() -> Option<String> + Send,
         cx: &mut Context<Self>,
     ) -> Task<(
         Vec<(TaskSourceKind, ResolvedTask)>,
@@ -518,14 +519,14 @@ impl Inventory {
                                 task_contexts.active_item_context.as_ref().filter(
                                     |(worktree_id, _, _)| Some(id) == worktree_id.as_ref(),
                                 )?;
-                            task.resolve_task(&id_base, item_context)
+                            task.resolve_task(&id_base, &remote_shell, item_context)
                         })
                         .or_else(|| {
                             let (_, worktree_context) = task_contexts
                                 .active_worktree_context
                                 .as_ref()
                                 .filter(|(worktree_id, _)| id == worktree_id)?;
-                            task.resolve_task(&id_base, worktree_context)
+                            task.resolve_task(&id_base, &remote_shell, worktree_context)
                         })
                         .or_else(|| {
                             if let TaskSourceKind::Worktree { id, .. } = &kind {
@@ -534,7 +535,7 @@ impl Inventory {
                                     .iter()
                                     .find(|(worktree_id, _)| worktree_id == id)
                                     .map(|(_, context)| context)?;
-                                task.resolve_task(&id_base, worktree_context)
+                                task.resolve_task(&id_base, &remote_shell, worktree_context)
                             } else {
                                 None
                             }
@@ -543,15 +544,15 @@ impl Inventory {
                         None.or_else(|| {
                             let (_, _, item_context) =
                                 task_contexts.active_item_context.as_ref()?;
-                            task.resolve_task(&id_base, item_context)
+                            task.resolve_task(&id_base, &remote_shell, item_context)
                         })
                         .or_else(|| {
                             let (_, worktree_context) =
                                 task_contexts.active_worktree_context.as_ref()?;
-                            task.resolve_task(&id_base, worktree_context)
+                            task.resolve_task(&id_base, &remote_shell, worktree_context)
                         })
                     }
-                    .or_else(|| task.resolve_task(&id_base, &TaskContext::default()))
+                    .or_else(|| task.resolve_task(&id_base, &remote_shell, &TaskContext::default()))
                     .map(move |resolved_task| (kind.clone(), resolved_task, not_used_score))
                 })
                 .filter(|(_, resolved_task, _)| {
@@ -896,7 +897,7 @@ mod test_inventory {
                 .update(&mut cx, |inventory, _| {
                     inventory.task_scheduled(
                         task_source_kind.clone(),
-                        task.resolve_task(&id_base, &TaskContext::default())
+                        task.resolve_task(&id_base, &|| None, &TaskContext::default())
                             .unwrap_or_else(|| {
                                 panic!("Failed to resolve task with name {task_name}")
                             }),
@@ -929,7 +930,7 @@ mod test_inventory {
                 .update(&mut cx, |inventory, _| {
                     inventory.task_scheduled(
                         task_source_kind.clone(),
-                        task.resolve_task(&id_base, &TaskContext::default())
+                        task.resolve_task(&id_base, &|| None, &TaskContext::default())
                             .unwrap_or_else(|| {
                                 panic!("Failed to resolve task with name {task_name}")
                             }),
@@ -953,7 +954,10 @@ mod test_inventory {
             .into_iter()
             .filter_map(|(source_kind, task)| {
                 let id_base = source_kind.to_id_base();
-                Some((source_kind, task.resolve_task(&id_base, task_context)?))
+                Some((
+                    source_kind,
+                    task.resolve_task(&id_base, &|| None, task_context)?,
+                ))
             })
             .map(|(source_kind, resolved_task)| (source_kind, resolved_task.resolved_label))
             .collect()
@@ -1558,7 +1562,7 @@ mod tests {
             task_contexts.active_worktree_context =
                 worktree.map(|worktree| (worktree, TaskContext::default()));
 
-            inventory.used_and_current_resolved_tasks(Arc::new(task_contexts), cx)
+            inventory.used_and_current_resolved_tasks(Arc::new(task_contexts), || None, cx)
         });
 
         cx.background_spawn(async move {
@@ -1597,7 +1601,7 @@ mod tests {
                 task_contexts.active_worktree_context =
                     worktree.map(|worktree| (worktree, TaskContext::default()));
 
-                inventory.used_and_current_resolved_tasks(Arc::new(task_contexts), cx)
+                inventory.used_and_current_resolved_tasks(Arc::new(task_contexts), || None, cx)
             })
             .await;
         let mut all = used;

crates/project/src/task_store.rs 🔗

@@ -155,6 +155,7 @@ impl TaskStore {
                 .into_iter()
                 .map(|(variable_name, variable_value)| (variable_name.to_string(), variable_value))
                 .collect(),
+            is_windows: cfg!(windows),
         })
     }
 
@@ -345,6 +346,7 @@ fn local_task_context_for_location(
             project_env: project_env.unwrap_or_default(),
             cwd: worktree_abs_path.map(|p| p.to_path_buf()),
             task_variables,
+            is_windows: cfg!(windows),
         })
     })
 }
@@ -414,6 +416,7 @@ fn remote_task_context_for_location(
                 )
                 .collect(),
             project_env: task_context.project_env.into_iter().collect(),
+            is_windows: task_context.is_windows,
         })
     })
 }

crates/proto/proto/task.proto 🔗

@@ -13,6 +13,7 @@ message TaskContext {
     optional string cwd = 1;
     map<string, string> task_variables = 2;
     map<string, string> project_env = 3;
+    bool is_windows = 4;
 }
 
 message Shell {

crates/task/src/task.rs 🔗

@@ -310,6 +310,7 @@ pub struct TaskContext {
     /// This is the environment one would get when `cd`ing in a terminal
     /// into the project's root directory.
     pub project_env: HashMap<String, String>,
+    pub is_windows: bool,
 }
 
 /// This is a new type representing a 'tag' on a 'runnable symbol', typically a test of main() function, found via treesitter.

crates/task/src/task_template.rs 🔗

@@ -8,6 +8,7 @@ use util::schemars::DefaultDenyUnknownFields;
 use util::serde::default_true;
 use util::{ResultExt, truncate_and_remove_front};
 
+use crate::ShellBuilder;
 use crate::{
     AttachRequest, ResolvedTask, RevealTarget, Shell, SpawnInTerminal, TaskContext, TaskId,
     VariableName, ZED_VARIABLE_NAME_PREFIX, serde_helpers::non_empty_string_vec,
@@ -65,7 +66,7 @@ pub struct TaskTemplate {
     pub tags: Vec<String>,
     /// Which shell to use when spawning the task.
     #[serde(default)]
-    pub shell: Shell,
+    pub shell: Option<Shell>,
     /// Whether to show the task line in the task output.
     #[serde(default = "default_true")]
     pub show_summary: bool,
@@ -132,7 +133,12 @@ impl TaskTemplate {
     ///
     /// Every [`ResolvedTask`] gets a [`TaskId`], based on the `id_base` (to avoid collision with various task sources),
     /// and hashes of its template and [`TaskContext`], see [`ResolvedTask`] fields' documentation for more details.
-    pub fn resolve_task(&self, id_base: &str, cx: &TaskContext) -> Option<ResolvedTask> {
+    pub fn resolve_task(
+        &self,
+        id_base: &str,
+        remote_shell: &dyn Fn() -> Option<String>,
+        cx: &TaskContext,
+    ) -> Option<ResolvedTask> {
         if self.label.trim().is_empty() || self.command.trim().is_empty() {
             return None;
         }
@@ -241,6 +247,55 @@ impl TaskTemplate {
             env
         };
 
+        let (shell, command, args, command_label) = match &self.shell {
+            // shell is specified, command + args may be a shell script so we force wrap it in another a shell execution
+            Some(shell) => {
+                let shell = if let Shell::System = shell
+                    && let Some(remote_shell) = remote_shell()
+                {
+                    Shell::Program(remote_shell)
+                } else {
+                    shell.clone()
+                };
+
+                let builder = ShellBuilder::new(&shell, cx.is_windows);
+                let command_label = builder.command_label(&command);
+                let (command, args) = builder.build(Some(command), &args_with_substitutions);
+
+                (shell, command, args, command_label)
+            }
+            // no shell specified but command contains whitespace, might be a shell script so spawn a shell for backwards compat
+            None if command.contains(char::is_whitespace) => {
+                let shell = match remote_shell() {
+                    Some(remote_shell) => Shell::Program(remote_shell),
+                    None => Shell::System,
+                };
+
+                let builder = ShellBuilder::new(&shell, cx.is_windows);
+                let command_label = builder.command_label(&command);
+                let (command, args) = builder.build(Some(command), &args_with_substitutions);
+
+                (shell, command, args, command_label)
+            }
+            // no shell specified, interpret as direct process spawn
+            None => {
+                let command_label = args_with_substitutions.iter().fold(
+                    command.clone(),
+                    |mut command_label, arg| {
+                        command_label.push(' ');
+                        command_label.push_str(arg);
+                        command_label
+                    },
+                );
+                (
+                    Shell::System,
+                    command,
+                    args_with_substitutions,
+                    command_label,
+                )
+            }
+        };
+
         Some(ResolvedTask {
             id: id.clone(),
             substituted_variables,
@@ -251,23 +306,16 @@ impl TaskTemplate {
                 cwd,
                 full_label,
                 label: human_readable_label,
-                command_label: args_with_substitutions.iter().fold(
-                    command.clone(),
-                    |mut command_label, arg| {
-                        command_label.push(' ');
-                        command_label.push_str(arg);
-                        command_label
-                    },
-                ),
+                command_label,
                 command: Some(command),
-                args: args_with_substitutions,
+                args: args,
                 env,
                 use_new_terminal: self.use_new_terminal,
                 allow_concurrent_runs: self.allow_concurrent_runs,
                 reveal: self.reveal,
                 reveal_target: self.reveal_target,
                 hide: self.hide,
-                shell: self.shell.clone(),
+                shell,
                 show_summary: self.show_summary,
                 show_command: self.show_command,
                 show_rerun: true,
@@ -462,7 +510,11 @@ mod tests {
             },
         ] {
             assert_eq!(
-                task_with_blank_property.resolve_task(TEST_ID_BASE, &TaskContext::default()),
+                task_with_blank_property.resolve_task(
+                    TEST_ID_BASE,
+                    &|| None,
+                    &TaskContext::default()
+                ),
                 None,
                 "should not resolve task with blank label and/or command: {task_with_blank_property:?}"
             );
@@ -480,7 +532,7 @@ mod tests {
 
         let resolved_task = |task_template: &TaskTemplate, task_cx| {
             let resolved_task = task_template
-                .resolve_task(TEST_ID_BASE, task_cx)
+                .resolve_task(TEST_ID_BASE, &|| None, task_cx)
                 .unwrap_or_else(|| panic!("failed to resolve task {task_without_cwd:?}"));
             assert_substituted_variables(&resolved_task, Vec::new());
             resolved_task.resolved
@@ -490,6 +542,7 @@ mod tests {
             cwd: None,
             task_variables: TaskVariables::default(),
             project_env: HashMap::default(),
+            is_windows: cfg!(windows),
         };
         assert_eq!(
             resolved_task(&task_without_cwd, &cx).cwd,
@@ -502,6 +555,7 @@ mod tests {
             cwd: Some(context_cwd.clone()),
             task_variables: TaskVariables::default(),
             project_env: HashMap::default(),
+            is_windows: cfg!(windows),
         };
         assert_eq!(
             resolved_task(&task_without_cwd, &cx).cwd,
@@ -518,6 +572,7 @@ mod tests {
             cwd: None,
             task_variables: TaskVariables::default(),
             project_env: HashMap::default(),
+            is_windows: cfg!(windows),
         };
         assert_eq!(
             resolved_task(&task_with_cwd, &cx).cwd,
@@ -529,6 +584,7 @@ mod tests {
             cwd: Some(context_cwd),
             task_variables: TaskVariables::default(),
             project_env: HashMap::default(),
+            is_windows: cfg!(windows),
         };
         assert_eq!(
             resolved_task(&task_with_cwd, &cx).cwd,
@@ -601,10 +657,11 @@ mod tests {
         for i in 0..15 {
             let resolved_task = task_with_all_variables.resolve_task(
                 TEST_ID_BASE,
-                &TaskContext {
+              &|| None,  &TaskContext {
                     cwd: None,
                     task_variables: TaskVariables::from_iter(all_variables.clone()),
                     project_env: HashMap::default(),
+                    is_windows: cfg!(windows),
                 },
             ).unwrap_or_else(|| panic!("Should successfully resolve task {task_with_all_variables:?} with variables {all_variables:?}"));
 
@@ -689,10 +746,12 @@ mod tests {
             let removed_variable = not_all_variables.remove(i);
             let resolved_task_attempt = task_with_all_variables.resolve_task(
                 TEST_ID_BASE,
+                &|| None,
                 &TaskContext {
                     cwd: None,
                     task_variables: TaskVariables::from_iter(not_all_variables),
                     project_env: HashMap::default(),
+                    is_windows: cfg!(windows),
                 },
             );
             assert_eq!(
@@ -711,7 +770,7 @@ mod tests {
             ..TaskTemplate::default()
         };
         let resolved_task = task
-            .resolve_task(TEST_ID_BASE, &TaskContext::default())
+            .resolve_task(TEST_ID_BASE, &|| None, &TaskContext::default())
             .unwrap();
         assert_substituted_variables(&resolved_task, Vec::new());
         let resolved = resolved_task.resolved;
@@ -729,7 +788,7 @@ mod tests {
             ..TaskTemplate::default()
         };
         assert!(
-            task.resolve_task(TEST_ID_BASE, &TaskContext::default())
+            task.resolve_task(TEST_ID_BASE, &|| None, &TaskContext::default())
                 .is_none()
         );
     }
@@ -750,6 +809,7 @@ mod tests {
                 "test_symbol".to_string(),
             ))),
             project_env: HashMap::default(),
+            is_windows: cfg!(windows),
         };
 
         for (i, symbol_dependent_task) in [
@@ -780,7 +840,7 @@ mod tests {
         .enumerate()
         {
             let resolved = symbol_dependent_task
-                .resolve_task(TEST_ID_BASE, &cx)
+                .resolve_task(TEST_ID_BASE, &|| None, &cx)
                 .unwrap_or_else(|| panic!("Failed to resolve task {symbol_dependent_task:?}"));
             assert_eq!(
                 resolved.substituted_variables,
@@ -822,7 +882,11 @@ mod tests {
         context
             .task_variables
             .insert(VariableName::Symbol, "my-symbol".to_string());
-        assert!(faulty_go_test.resolve_task("base", &context).is_some());
+        assert!(
+            faulty_go_test
+                .resolve_task("base", &|| None, &context)
+                .is_some()
+        );
     }
 
     #[test]
@@ -878,10 +942,11 @@ mod tests {
             cwd: None,
             task_variables: TaskVariables::from_iter(all_variables),
             project_env,
+            is_windows: cfg!(windows),
         };
 
         let resolved = template
-            .resolve_task(TEST_ID_BASE, &context)
+            .resolve_task(TEST_ID_BASE, &|| None, &context)
             .unwrap()
             .resolved;
 
@@ -917,10 +982,11 @@ mod tests {
                 (VariableName::Row, "123".to_string()),
             ]),
             project_env: HashMap::default(),
+            is_windows: cfg!(windows),
         };
 
         let resolved = task_with_defaults
-            .resolve_task(TEST_ID_BASE, &context_with_file)
+            .resolve_task(TEST_ID_BASE, &|| None, &context_with_file)
             .expect("Should resolve task with existing variables");
 
         assert_eq!(
@@ -939,10 +1005,11 @@ mod tests {
             cwd: None,
             task_variables: TaskVariables::from_iter(vec![(VariableName::Row, "456".to_string())]),
             project_env: HashMap::default(),
+            is_windows: cfg!(windows),
         };
 
         let resolved = task_with_defaults
-            .resolve_task(TEST_ID_BASE, &context_without_file)
+            .resolve_task(TEST_ID_BASE, &|| None, &context_without_file)
             .expect("Should resolve task using default values");
 
         assert_eq!(
@@ -965,7 +1032,7 @@ mod tests {
 
         assert!(
             task_no_default
-                .resolve_task(TEST_ID_BASE, &TaskContext::default())
+                .resolve_task(TEST_ID_BASE, &|| None, &TaskContext::default())
                 .is_none(),
             "Should fail when ZED variable has no default and doesn't exist"
         );

crates/tasks_ui/src/modal.rs 🔗

@@ -74,7 +74,10 @@ impl TasksModalDelegate {
         }
     }
 
-    fn spawn_oneshot(&mut self) -> Option<(TaskSourceKind, ResolvedTask)> {
+    fn spawn_oneshot(
+        &mut self,
+        cx: &mut Context<Picker<Self>>,
+    ) -> Option<(TaskSourceKind, ResolvedTask)> {
         if self.prompt.trim().is_empty() {
             return None;
         }
@@ -99,7 +102,23 @@ impl TasksModalDelegate {
         }
         Some((
             source_kind,
-            new_oneshot.resolve_task(&id_base, active_context)?,
+            new_oneshot.resolve_task(
+                &id_base,
+                &|| {
+                    self.workspace
+                        .read_with(cx, |workspace, cx| {
+                            workspace
+                                .project()
+                                .read(cx)
+                                .remote_client()?
+                                .read(cx)
+                                .shell()
+                        })
+                        .ok()
+                        .flatten()
+                },
+                active_context,
+            )?,
         ))
     }
 
@@ -274,10 +293,20 @@ impl PickerDelegate for TasksModalDelegate {
             Some(candidates) => Task::ready(string_match_candidates(candidates)),
             None => {
                 if let Some(task_inventory) = self.task_store.read(cx).task_inventory().cloned() {
+                    let workspace = self.workspace.clone();
                     let task_list = task_inventory.update(cx, |this, cx| {
-                        this.used_and_current_resolved_tasks(self.task_contexts.clone(), cx)
+                        let remote_shell = workspace
+                            .read_with(cx, |it, cx| {
+                                it.project().read(cx).remote_client()?.read(cx).shell()
+                            })
+                            .ok()
+                            .flatten();
+                        this.used_and_current_resolved_tasks(
+                            self.task_contexts.clone(),
+                            move || remote_shell.clone(),
+                            cx,
+                        )
                     });
-                    let workspace = self.workspace.clone();
                     let lsp_task_sources = self.task_contexts.lsp_task_sources.clone();
                     let task_position = self.task_contexts.latest_selection;
                     cx.spawn(async move |picker, cx| {
@@ -603,7 +632,7 @@ impl PickerDelegate for TasksModalDelegate {
         window: &mut Window,
         cx: &mut Context<Picker<Self>>,
     ) {
-        let Some((task_source_kind, mut task)) = self.spawn_oneshot() else {
+        let Some((task_source_kind, mut task)) = self.spawn_oneshot(cx) else {
             return;
         };
 

crates/terminal_view/src/terminal_panel.rs 🔗

@@ -19,7 +19,7 @@ use itertools::Itertools;
 use project::{Fs, Project, ProjectEntryId};
 use search::{BufferSearchBar, buffer_search::DivRegistrar};
 use settings::{Settings, TerminalDockPosition};
-use task::{RevealStrategy, RevealTarget, Shell, ShellBuilder, SpawnInTerminal, TaskId};
+use task::{RevealStrategy, RevealTarget, SpawnInTerminal, TaskId};
 use terminal::{Terminal, terminal_settings::TerminalSettings};
 use ui::{
     ButtonLike, Clickable, ContextMenu, FluentBuilder, PopoverMenu, SplitButton, Toggleable,
@@ -520,45 +520,10 @@ impl TerminalPanel {
 
     pub fn spawn_task(
         &mut self,
-        task: &SpawnInTerminal,
+        task: SpawnInTerminal,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Task<Result<WeakEntity<Terminal>>> {
-        let Some(workspace) = self.workspace.upgrade() else {
-            return Task::ready(Err(anyhow!("failed to read workspace")));
-        };
-
-        let project = workspace.read(cx).project().read(cx);
-
-        if project.is_via_collab() {
-            return Task::ready(Err(anyhow!("cannot spawn tasks as a guest")));
-        }
-
-        let remote_client = project.remote_client();
-        let is_windows = project.path_style(cx).is_windows();
-        let remote_shell = remote_client
-            .as_ref()
-            .and_then(|remote_client| remote_client.read(cx).shell());
-
-        let shell = if let Some(remote_shell) = remote_shell
-            && task.shell == Shell::System
-        {
-            Shell::Program(remote_shell)
-        } else {
-            task.shell.clone()
-        };
-
-        let builder = ShellBuilder::new(&shell, is_windows);
-        let command_label = builder.command_label(task.command.as_deref().unwrap_or(""));
-        let (command, args) = builder.build(task.command.clone(), &task.args);
-
-        let task = SpawnInTerminal {
-            command_label,
-            command: Some(command),
-            args,
-            ..task.clone()
-        };
-
         if task.allow_concurrent_runs && task.use_new_terminal {
             return self.spawn_in_new_terminal(task, window, cx);
         }
@@ -1708,7 +1673,7 @@ impl workspace::TerminalProvider for TerminalProvider {
         window.spawn(cx, async move |cx| {
             let terminal = terminal_panel
                 .update_in(cx, |terminal_panel, window, cx| {
-                    terminal_panel.spawn_task(&task, window, cx)
+                    terminal_panel.spawn_task(task, window, cx)
                 })
                 .ok()?
                 .await;
@@ -1773,13 +1738,12 @@ mod tests {
         let task = window_handle
             .update(cx, |_, window, cx| {
                 terminal_panel.update(cx, |terminal_panel, cx| {
-                    terminal_panel.spawn_task(&SpawnInTerminal::default(), window, cx)
+                    terminal_panel.spawn_task(SpawnInTerminal::default(), window, cx)
                 })
             })
             .unwrap();
 
         let terminal = task.await.unwrap();
-        let expected_shell = util::get_system_shell();
         terminal
             .update(cx, |terminal, _| {
                 let task_metadata = terminal
@@ -1791,15 +1755,11 @@ mod tests {
                 assert_eq!(task_metadata.cwd, None);
                 assert_eq!(task_metadata.shell, task::Shell::System);
                 assert_eq!(
-                    task_metadata.command,
-                    Some(expected_shell.clone()),
+                    task_metadata.command, None,
                     "Empty tasks should spawn a -i shell"
                 );
                 assert_eq!(task_metadata.args, Vec::<String>::new());
-                assert_eq!(
-                    task_metadata.command_label, expected_shell,
-                    "We show the shell launch for empty commands"
-                );
+                assert_eq!(task_metadata.command_label, "");
             })
             .unwrap();
     }
@@ -1848,6 +1808,8 @@ mod tests {
     #[cfg(unix)]
     #[gpui::test]
     async fn test_spawn_script_like_task(cx: &mut TestAppContext) {
+        use task::{TaskContext, TaskTemplate};
+
         init_test(cx);
 
         let fs = FakeFs::new(cx.executor());
@@ -1862,21 +1824,19 @@ mod tests {
             })
             .unwrap();
 
-        let user_command = r#"REPO_URL=$(git remote get-url origin | sed -e \"s/^git@\\(.*\\):\\(.*\\)\\.git$/https:\\/\\/\\1\\/\\2/\"); COMMIT_SHA=$(git log -1 --format=\"%H\" -- \"${ZED_RELATIVE_FILE}\"); echo \"${REPO_URL}/blob/${COMMIT_SHA}/${ZED_RELATIVE_FILE}#L${ZED_ROW}-$(echo $(($(wc -l <<< \"$ZED_SELECTED_TEXT\") + $ZED_ROW - 1)))\" | xclip -selection clipboard"#.to_string();
-
-        let expected_cwd = PathBuf::from("/some/work");
+        let user_command = r#"REPO_URL=$(git remote get-url origin | sed -e \"s/^git@\\(.*\\):\\(.*\\)\\.git$/https:\\/\\/\\1\\/\\2/\"); COMMIT_SHA=$(git log -1 --format=\"%H\" -- \"ZED_RELATIVE_FILE\"); echo \"REPO_URL/blob/COMMIT_SHA/ZED_RELATIVE_FILE#LZED_ROW-$(echo $(($(wc -l <<< \"ZED_SELECTED_TEXT\") + ZED_ROW - 1)))\" | xclip -selection clipboard"#.to_string();
+        let template = TaskTemplate {
+            command: user_command.clone(),
+            cwd: Some("/some/work".to_owned()),
+            label: "some task".to_owned(),
+            ..TaskTemplate::default()
+        }
+        .resolve_task("id", &|| None, &TaskContext::default())
+        .unwrap();
         let task = window_handle
             .update(cx, |_, window, cx| {
                 terminal_panel.update(cx, |terminal_panel, cx| {
-                    terminal_panel.spawn_task(
-                        &SpawnInTerminal {
-                            command: Some(user_command.clone()),
-                            cwd: Some(expected_cwd.clone()),
-                            ..SpawnInTerminal::default()
-                        },
-                        window,
-                        cx,
-                    )
+                    terminal_panel.spawn_task(template.resolved, window, cx)
                 })
             })
             .unwrap();
@@ -1891,7 +1851,7 @@ mod tests {
                     .spawned_task
                     .clone();
                 assert_eq!(task_metadata.env, HashMap::default());
-                assert_eq!(task_metadata.cwd, Some(expected_cwd));
+                assert_eq!(task_metadata.cwd.as_deref(), Some("/some/work".as_ref()));
                 assert_eq!(task_metadata.shell, task::Shell::System);
                 assert_eq!(task_metadata.command, Some(shell.clone()));
                 assert_eq!(

crates/workspace/src/tasks.rs 🔗

@@ -20,7 +20,8 @@ impl Workspace {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        match self.project.read(cx).remote_connection_state(cx) {
+        let project = self.project.read(cx);
+        match project.remote_connection_state(cx) {
             None | Some(ConnectionState::Connected) => {}
             Some(
                 ConnectionState::Connecting
@@ -33,9 +34,11 @@ impl Workspace {
             }
         }
 
-        if let Some(spawn_in_terminal) =
-            task_to_resolve.resolve_task(&task_source_kind.to_id_base(), task_cx)
-        {
+        if let Some(spawn_in_terminal) = task_to_resolve.resolve_task(
+            &task_source_kind.to_id_base(),
+            &|| project.remote_client()?.read(cx).shell(),
+            task_cx,
+        ) {
             self.schedule_resolved_task(
                 task_source_kind,
                 spawn_in_terminal,