debugger: Fix the JavaScript debug terminal scenario (#33924)

There were a couple of things preventing this from working:

- our hack to stop the node REPL from appearing broke in recent versions
of the JS DAP that started passing `--experimental-network-inspection`
by default
- we had lost the ability to create a debug terminal without specifying
a program

This PR fixes those issues. We also fixed environment variables from the
**runInTerminal** request not getting passed to the spawned program.

Release Notes:

- Debugger: Fix RunInTerminal not working for JavaScript debugger.

---------

Co-authored-by: Cole Miller <cole@zed.dev>
This commit is contained in:
Remco Smits 2025-07-06 01:48:55 +02:00 committed by GitHub
parent 66e45818af
commit 01295aa687
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 108 additions and 73 deletions

View file

@ -218,7 +218,7 @@ impl Tool for TerminalTool {
.update(cx, |project, cx| { .update(cx, |project, cx| {
project.create_terminal( project.create_terminal(
TerminalKind::Task(task::SpawnInTerminal { TerminalKind::Task(task::SpawnInTerminal {
command: program, command: Some(program),
args, args,
cwd, cwd,
env, env,

View file

@ -1,9 +1,10 @@
use adapters::latest_github_release; use adapters::latest_github_release;
use anyhow::Context as _; use anyhow::Context as _;
use collections::HashMap;
use dap::{StartDebuggingRequestArguments, adapters::DebugTaskDefinition}; use dap::{StartDebuggingRequestArguments, adapters::DebugTaskDefinition};
use gpui::AsyncApp; use gpui::AsyncApp;
use serde_json::Value; use serde_json::Value;
use std::{collections::HashMap, path::PathBuf, sync::OnceLock}; use std::{path::PathBuf, sync::OnceLock};
use task::DebugRequest; use task::DebugRequest;
use util::{ResultExt, maybe}; use util::{ResultExt, maybe};
@ -70,6 +71,8 @@ impl JsDebugAdapter {
let tcp_connection = task_definition.tcp_connection.clone().unwrap_or_default(); let tcp_connection = task_definition.tcp_connection.clone().unwrap_or_default();
let (host, port, timeout) = crate::configure_tcp_connection(tcp_connection).await?; let (host, port, timeout) = crate::configure_tcp_connection(tcp_connection).await?;
let mut envs = HashMap::default();
let mut configuration = task_definition.config.clone(); let mut configuration = task_definition.config.clone();
if let Some(configuration) = configuration.as_object_mut() { if let Some(configuration) = configuration.as_object_mut() {
maybe!({ maybe!({
@ -110,6 +113,12 @@ impl JsDebugAdapter {
} }
} }
if let Some(env) = configuration.get("env").cloned() {
if let Ok(env) = serde_json::from_value(env) {
envs = env;
}
}
configuration configuration
.entry("cwd") .entry("cwd")
.or_insert(delegate.worktree_root_path().to_string_lossy().into()); .or_insert(delegate.worktree_root_path().to_string_lossy().into());
@ -158,7 +167,7 @@ impl JsDebugAdapter {
), ),
arguments, arguments,
cwd: Some(delegate.worktree_root_path().to_path_buf()), cwd: Some(delegate.worktree_root_path().to_path_buf()),
envs: HashMap::default(), envs,
connection: Some(adapters::TcpArguments { connection: Some(adapters::TcpArguments {
host, host,
port, port,

View file

@ -973,7 +973,7 @@ impl RunningState {
let task_with_shell = SpawnInTerminal { let task_with_shell = SpawnInTerminal {
command_label, command_label,
command, command: Some(command),
args, args,
..task.resolved.clone() ..task.resolved.clone()
}; };
@ -1085,19 +1085,6 @@ impl RunningState {
.map(PathBuf::from) .map(PathBuf::from)
.or_else(|| session.binary().unwrap().cwd.clone()); .or_else(|| session.binary().unwrap().cwd.clone());
let mut args = request.args.clone();
// Handle special case for NodeJS debug adapter
// If only the Node binary path is provided, we set the command to None
// This prevents the NodeJS REPL from appearing, which is not the desired behavior
// The expected usage is for users to provide their own Node command, e.g., `node test.js`
// This allows the NodeJS debug client to attach correctly
let command = if args.len() > 1 {
Some(args.remove(0))
} else {
None
};
let mut envs: HashMap<String, String> = let mut envs: HashMap<String, String> =
self.session.read(cx).task_context().project_env.clone(); self.session.read(cx).task_context().project_env.clone();
if let Some(Value::Object(env)) = &request.env { if let Some(Value::Object(env)) = &request.env {
@ -1111,32 +1098,58 @@ impl RunningState {
} }
} }
let shell = project.read(cx).terminal_settings(&cwd, cx).shell.clone(); let mut args = request.args.clone();
let kind = if let Some(command) = command { let command = if envs.contains_key("VSCODE_INSPECTOR_OPTIONS") {
let title = request.title.clone().unwrap_or(command.clone()); // Handle special case for NodeJS debug adapter
TerminalKind::Task(task::SpawnInTerminal { // If the Node binary path is provided (possibly with arguments like --experimental-network-inspection),
id: task::TaskId("debug".to_string()), // we set the command to None
full_label: title.clone(), // This prevents the NodeJS REPL from appearing, which is not the desired behavior
label: title.clone(), // The expected usage is for users to provide their own Node command, e.g., `node test.js`
command: command.clone(), // This allows the NodeJS debug client to attach correctly
args, if args
command_label: title.clone(), .iter()
cwd, .filter(|arg| !arg.starts_with("--"))
env: envs, .collect::<Vec<_>>()
use_new_terminal: true, .len()
allow_concurrent_runs: true, > 1
reveal: task::RevealStrategy::NoFocus, {
reveal_target: task::RevealTarget::Dock, Some(args.remove(0))
hide: task::HideStrategy::Never, } else {
shell, None
show_summary: false, }
show_command: false, } else if args.len() > 0 {
show_rerun: false, Some(args.remove(0))
})
} else { } else {
TerminalKind::Shell(cwd.map(|c| c.to_path_buf())) None
}; };
let shell = project.read(cx).terminal_settings(&cwd, cx).shell.clone();
let title = request
.title
.clone()
.filter(|title| !title.is_empty())
.or_else(|| command.clone())
.unwrap_or_else(|| "Debug terminal".to_string());
let kind = TerminalKind::Task(task::SpawnInTerminal {
id: task::TaskId("debug".to_string()),
full_label: title.clone(),
label: title.clone(),
command: command.clone(),
args,
command_label: title.clone(),
cwd,
env: envs,
use_new_terminal: true,
allow_concurrent_runs: true,
reveal: task::RevealStrategy::NoFocus,
reveal_target: task::RevealTarget::Dock,
hide: task::HideStrategy::Never,
shell,
show_summary: false,
show_command: false,
show_rerun: false,
});
let workspace = self.workspace.clone(); let workspace = self.workspace.clone();
let weak_project = project.downgrade(); let weak_project = project.downgrade();

View file

@ -999,7 +999,7 @@ impl Extension {
) -> Result<Result<DebugRequest, String>> { ) -> Result<Result<DebugRequest, String>> {
match self { match self {
Extension::V0_6_0(ext) => { Extension::V0_6_0(ext) => {
let build_config_template = resolved_build_task.into(); let build_config_template = resolved_build_task.try_into()?;
let dap_request = ext let dap_request = ext
.call_run_dap_locator(store, &locator_name, &build_config_template) .call_run_dap_locator(store, &locator_name, &build_config_template)
.await? .await?

View file

@ -299,15 +299,17 @@ impl From<extension::DebugScenario> for DebugScenario {
} }
} }
impl From<SpawnInTerminal> for ResolvedTask { impl TryFrom<SpawnInTerminal> for ResolvedTask {
fn from(value: SpawnInTerminal) -> Self { type Error = anyhow::Error;
Self {
fn try_from(value: SpawnInTerminal) -> Result<Self, Self::Error> {
Ok(Self {
label: value.label, label: value.label,
command: value.command, command: value.command.context("missing command")?,
args: value.args, args: value.args,
env: value.env.into_iter().collect(), env: value.env.into_iter().collect(),
cwd: value.cwd.map(|s| s.to_string_lossy().into_owned()), cwd: value.cwd.map(|s| s.to_string_lossy().into_owned()),
} })
} }
} }

View file

@ -119,7 +119,7 @@ impl DapLocator for CargoLocator {
.context("Couldn't get cwd from debug config which is needed for locators")?; .context("Couldn't get cwd from debug config which is needed for locators")?;
let builder = ShellBuilder::new(true, &build_config.shell).non_interactive(); let builder = ShellBuilder::new(true, &build_config.shell).non_interactive();
let (program, args) = builder.build( let (program, args) = builder.build(
"cargo".into(), Some("cargo".into()),
&build_config &build_config
.args .args
.iter() .iter()

View file

@ -568,7 +568,7 @@ async fn test_fallback_to_single_worktree_tasks(cx: &mut gpui::TestAppContext) {
.into_iter() .into_iter()
.map(|(source_kind, task)| { .map(|(source_kind, task)| {
let resolved = task.resolved; let resolved = task.resolved;
(source_kind, resolved.command) (source_kind, resolved.command.unwrap())
}) })
.collect::<Vec<_>>(), .collect::<Vec<_>>(),
vec![( vec![(

View file

@ -149,7 +149,7 @@ impl Project {
let settings = self.terminal_settings(&path, cx).clone(); let settings = self.terminal_settings(&path, cx).clone();
let builder = ShellBuilder::new(ssh_details.is_none(), &settings.shell).non_interactive(); let builder = ShellBuilder::new(ssh_details.is_none(), &settings.shell).non_interactive();
let (command, args) = builder.build(command, &Vec::new()); let (command, args) = builder.build(Some(command), &Vec::new());
let mut env = self let mut env = self
.environment .environment
@ -297,7 +297,10 @@ impl Project {
.or_insert_with(|| "xterm-256color".to_string()); .or_insert_with(|| "xterm-256color".to_string());
let (program, args) = wrap_for_ssh( let (program, args) = wrap_for_ssh(
&ssh_command, &ssh_command,
Some((&spawn_task.command, &spawn_task.args)), spawn_task
.command
.as_ref()
.map(|command| (command, &spawn_task.args)),
path.as_deref(), path.as_deref(),
env, env,
python_venv_directory.as_deref(), python_venv_directory.as_deref(),
@ -317,14 +320,16 @@ impl Project {
add_environment_path(&mut env, &venv_path.join("bin")).log_err(); add_environment_path(&mut env, &venv_path.join("bin")).log_err();
} }
( let shell = if let Some(program) = spawn_task.command {
task_state,
Shell::WithArguments { Shell::WithArguments {
program: spawn_task.command, program,
args: spawn_task.args, args: spawn_task.args,
title_override: None, title_override: None,
}, }
) } else {
Shell::System
};
(task_state, shell)
} }
} }
} }

View file

@ -535,7 +535,7 @@ message DebugScenario {
message SpawnInTerminal { message SpawnInTerminal {
string label = 1; string label = 1;
string command = 2; optional string command = 2;
repeated string args = 3; repeated string args = 3;
map<string, string> env = 4; map<string, string> env = 4;
optional string cwd = 5; optional string cwd = 5;

View file

@ -149,17 +149,23 @@ impl ShellBuilder {
} }
} }
/// Returns the program and arguments to run this task in a shell. /// Returns the program and arguments to run this task in a shell.
pub fn build(mut self, task_command: String, task_args: &Vec<String>) -> (String, Vec<String>) { pub fn build(
let combined_command = task_args mut self,
.into_iter() task_command: Option<String>,
.fold(task_command, |mut command, arg| { task_args: &Vec<String>,
command.push(' '); ) -> (String, Vec<String>) {
command.push_str(&self.kind.to_shell_variable(arg)); if let Some(task_command) = task_command {
command let combined_command = task_args
}); .into_iter()
.fold(task_command, |mut command, arg| {
command.push(' ');
command.push_str(&self.kind.to_shell_variable(arg));
command
});
self.args self.args
.extend(self.kind.args_for_shell(self.interactive, combined_command)); .extend(self.kind.args_for_shell(self.interactive, combined_command));
}
(self.program, self.args) (self.program, self.args)
} }

View file

@ -46,7 +46,7 @@ pub struct SpawnInTerminal {
/// Human readable name of the terminal tab. /// Human readable name of the terminal tab.
pub label: String, pub label: String,
/// Executable command to spawn. /// Executable command to spawn.
pub command: String, pub command: Option<String>,
/// Arguments to the command, potentially unsubstituted, /// Arguments to the command, potentially unsubstituted,
/// to let the shell that spawns the command to do the substitution, if needed. /// to let the shell that spawns the command to do the substitution, if needed.
pub args: Vec<String>, pub args: Vec<String>,

View file

@ -255,7 +255,7 @@ impl TaskTemplate {
command_label command_label
}, },
), ),
command, command: Some(command),
args: self.args.clone(), args: self.args.clone(),
env, env,
use_new_terminal: self.use_new_terminal, use_new_terminal: self.use_new_terminal,
@ -635,7 +635,7 @@ mod tests {
"Human-readable label should have long substitutions trimmed" "Human-readable label should have long substitutions trimmed"
); );
assert_eq!( assert_eq!(
spawn_in_terminal.command, spawn_in_terminal.command.clone().unwrap(),
format!("echo test_file {long_value}"), format!("echo test_file {long_value}"),
"Command should be substituted with variables and those should not be shortened" "Command should be substituted with variables and those should not be shortened"
); );
@ -652,7 +652,7 @@ mod tests {
spawn_in_terminal.command_label, spawn_in_terminal.command_label,
format!( format!(
"{} arg1 test_selected_text arg2 5678 arg3 {long_value}", "{} arg1 test_selected_text arg2 5678 arg3 {long_value}",
spawn_in_terminal.command spawn_in_terminal.command.clone().unwrap()
), ),
"Command label args should be substituted with variables and those should not be shortened" "Command label args should be substituted with variables and those should not be shortened"
); );
@ -711,7 +711,7 @@ mod tests {
assert_substituted_variables(&resolved_task, Vec::new()); assert_substituted_variables(&resolved_task, Vec::new());
let resolved = resolved_task.resolved; let resolved = resolved_task.resolved;
assert_eq!(resolved.label, task.label); assert_eq!(resolved.label, task.label);
assert_eq!(resolved.command, task.command); assert_eq!(resolved.command, Some(task.command));
assert_eq!(resolved.args, task.args); assert_eq!(resolved.args, task.args);
} }

View file

@ -505,7 +505,7 @@ impl TerminalPanel {
let task = SpawnInTerminal { let task = SpawnInTerminal {
command_label, command_label,
command, command: Some(command),
args, args,
..task.clone() ..task.clone()
}; };

View file

@ -1688,7 +1688,7 @@ impl ShellExec {
id: TaskId("vim".to_string()), id: TaskId("vim".to_string()),
full_label: command.clone(), full_label: command.clone(),
label: command.clone(), label: command.clone(),
command: command.clone(), command: Some(command.clone()),
args: Vec::new(), args: Vec::new(),
command_label: command.clone(), command_label: command.clone(),
cwd, cwd,