From ea8e04e4484673102417e18a636efb1bcf165048 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 9 Nov 2026 02:22:04 +0800 Subject: [PATCH] fix(debugger): preserve Ruby output and session cleanup (#2182) Fixes found while verifying Ruby debugging. An entry point with no arguments reached Ruby's single-string exec in rdbg, so spaces and shell syntax were interpreted; it now goes through ruby -S -- and stays literal. rdbg sends no DAP output or process events, so its output streams are forwarded and drained at shutdown, and the launched process group is terminated on Stop or a failed start; attach leaves the process alone. Bundler lookup respects BUNDLE_GEMFILE and gems.rb, gem discovery uses the configuration's environment, and the missing debug gem message is translated. Refs #2170 Co-authored-by: krmbzds <1383035+krmbzds@users.noreply.github.com> --- crates/ide-dap/src/adapter.rs | 14 ++- crates/ide-dap/src/lib.rs | 29 ++++- crates/ide-dap/src/session.rs | 12 ++ crates/ide-dap/src/tests.rs | 103 +++++++++++++++++- crates/ide-host/src/ops/debug.rs | 2 +- crates/ide-sdk/src/ruby.rs | 21 +++- .../debugger/__tests__/startFailure.test.ts | 16 +++ src/features/debugger/startFailure.ts | 5 + src/platform/locales/text/en.json | 3 +- src/platform/locales/text/zh.json | 3 +- 10 files changed, 195 insertions(+), 13 deletions(-) diff --git a/crates/ide-dap/src/adapter.rs b/crates/ide-dap/src/adapter.rs index 9c0431ec6..d517510e9 100644 --- a/crates/ide-dap/src/adapter.rs +++ b/crates/ide-dap/src/adapter.rs @@ -374,8 +374,8 @@ pub struct AdapterIo { pub type ProgramOutput = tokio::sync::mpsc::UnboundedReceiver<(&'static str, String)>; type ProgramSink = tokio::sync::mpsc::UnboundedSender<(&'static str, String)>; -/// Delve's debuggee inherits its stdio; no DAP output event carries what the program prints. -const SHARES_STDIO: &str = "delve"; +/// These debuggees inherit adapter stdio; no DAP event carries their output. +const SHARES_STDIO: &[&str] = &["delve", "rdbg"]; const DELVE_BANNER: &str = "DAP server listening at:"; pub type LaunchFuture = Pin> + Send>>; @@ -477,6 +477,9 @@ impl AdapterLauncher for ProcessLauncher { cmd.env("PATH", path); } ide_core::process::prepare(cmd.as_std_mut()); + if spec.adapter == "rdbg" { + ide_core::process::own_group(cmd.as_std_mut()); + } cmd.args(&args) .envs(spec.env.iter().map(|(k, v)| (k, v))) .current_dir(&spec.cwd) @@ -486,7 +489,7 @@ impl AdapterLauncher for ProcessLauncher { .kill_on_drop(true); let mut child = cmd.spawn().map_err(|e| IdeError::unavailable(format!("failed to start {}: {e}", binary.display())))?; - let (program_sink, program_output) = if spec.adapter == SHARES_STDIO { + let (program_sink, program_output) = if SHARES_STDIO.contains(&spec.adapter.as_str()) { let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); (Some(tx), Some(rx)) } else { @@ -516,6 +519,11 @@ impl AdapterLauncher for ProcessLauncher { let stream = match connect_with_retries(LOOPBACK, port, Some(&mut child), timeout).await { Ok(s) => s, Err(e) => { + if spec.adapter == "rdbg" + && let Some(pid) = child.id() + { + ide_core::process::kill_tree_of(pid); + } let _ = child.start_kill(); let tail = stderr.lock().join("\n"); return Err(if tail.is_empty() { diff --git a/crates/ide-dap/src/lib.rs b/crates/ide-dap/src/lib.rs index 1336de244..8b335d520 100644 --- a/crates/ide-dap/src/lib.rs +++ b/crates/ide-dap/src/lib.rs @@ -310,6 +310,9 @@ pub struct DebugManager { /// In a Bundler project it runs from the bundle, which must have `debug`: the debuggee then /// loads that version, and the gems rdbg needs (irb is not a default gem from Ruby 3.5). fn rdbg_command_line(spec: &mut AdapterSpec, config: &LaunchConfig, cwd: &Path) -> IdeResult<()> { + if config.request != "launch" { + return Ok(()); + } if spec.command == "rdbg" && let Some(gemfile) = bundler_gemfile(cwd, &config.env) { @@ -324,6 +327,11 @@ fn rdbg_command_line(spec: &mut AdapterSpec, config: &LaunchConfig, cwd: &Path) } spec.env.extend(config.env.iter().map(|(k, v)| (k.clone(), v.clone()))); spec.args.push("--".into()); + // With only a command, Ruby's exec treats it as shell text. Ruby entry points + // without arguments go through -S so spaces and metacharacters stay literal. + if config.program.is_some() && config.args.is_empty() { + spec.args.extend(["ruby", "-S", "--"].map(str::to_owned)); + } spec.args.extend(config.program.iter().cloned()); spec.args.extend(config.args.iter().cloned()); Ok(()) @@ -331,9 +339,13 @@ fn rdbg_command_line(spec: &mut AdapterSpec, config: &LaunchConfig, cwd: &Path) /// The Gemfile Bundler uses in `dir`: `BUNDLE_GEMFILE`, else the closest `Gemfile` / `gems.rb` from `dir` up. fn bundler_gemfile(dir: &Path, env: &BTreeMap) -> Option { - match env.get("BUNDLE_GEMFILE").filter(|g| !g.is_empty()) { + let gemfile = env + .get("BUNDLE_GEMFILE") + .cloned() + .or_else(|| ide_core::process::host_var_os("BUNDLE_GEMFILE").map(|v| v.to_string_lossy().into_owned())); + match gemfile.filter(|g| !g.is_empty()) { Some(gemfile) => Some(dir.join(gemfile)), - None => dir.ancestors().flat_map(|d| [d.join("Gemfile"), d.join("gems.rb")]).find(|g| g.is_file()), + None => dir.ancestors().flat_map(|d| [d.join("gems.rb"), d.join("Gemfile")]).find(|g| g.is_file()), } } @@ -842,14 +854,23 @@ impl DebugManager { let stderr = stderr.clone(); Connection::start_with_output(io.reader, io.writer, Box::new(move |line| adapter::push_line(&stderr, line))) }; + let rdbg_pid = (adapter_name == "rdbg" && request == "launch") + .then(|| io.child.as_ref().and_then(tokio::process::Child::id)) + .flatten(); let session = Arc::new(Session::new(id, owner, name, adapter_name, parent, request, conn, spec, io.port, io.child)); + // rdbg execs the debuggee in its own process and sends no DAP process event. + if let Some(pid) = rdbg_pid { + session.debuggee_pid.store(i64::from(pid), Ordering::Release); + } if let Some(mut output) = io.program_output { + session.output_drained.send_replace(false); let session = session.clone(); tokio::spawn(async move { while let Some((category, output)) = output.recv().await { session.emit(ide_core::event::DebugEvent::Output { category: category.into(), output }); } + session.output_drained.send_replace(true); }); } self.sessions.lock().insert(id, session.clone()); @@ -1051,7 +1072,9 @@ impl DebugManager { // the console (`terminated`), as IDEA does with the process it owns. let pid = s.debuggee_pid.load(Ordering::Acquire); let own_terminate = terminate && s.request == "launch" && pid > 0; - let graceful = terminate && s.request == "launch" && s.supports("supportsTerminateRequest"); + // rdbg's terminate exits the Ruby process without terminating its children. + let graceful = + terminate && s.request == "launch" && s.adapter != "rdbg" && s.supports("supportsTerminateRequest"); if !s.conn.is_closed() { if graceful { let _ = s.conn.request_timeout("terminate", json!({ "restart": false }), SHUTDOWN_TIMEOUT).await; diff --git a/crates/ide-dap/src/session.rs b/crates/ide-dap/src/session.rs index bc9c81946..fe2d6f8ec 100644 --- a/crates/ide-dap/src/session.rs +++ b/crates/ide-dap/src/session.rs @@ -48,6 +48,7 @@ pub(crate) struct Session { /// Set once `configurationDone` was sent; breakpoint updates go out live after that. pub configured: AtomicBool, pub initialized: watch::Sender, + pub output_drained: watch::Sender, pub terminated: AtomicBool, /// Set by a second Stop: the one waiting for the program kills it instead. pub stop_force: AtomicBool, @@ -103,6 +104,7 @@ impl Session { program: Mutex::new(None), configured: AtomicBool::new(false), initialized: watch::channel(false).0, + output_drained: watch::channel(true).0, terminated: AtomicBool::new(false), stop_force: AtomicBool::new(false), debuggee_pid: AtomicI64::new(0), @@ -275,6 +277,11 @@ impl Session { pub fn kill_processes(&self) { self.kotlin_relay.lock().take(); if let Some(mut c) = self.adapter_process.lock().take() { + if self.adapter == "rdbg" + && let Some(pid) = c.id() + { + ide_core::process::kill_tree_of(pid); + } let _ = c.start_kill(); } for mut c in std::mem::take(&mut *self.debuggees.lock()) { @@ -448,6 +455,11 @@ pub(crate) async fn event_loop( Incoming::Closed => break, } } + // rdbg closes DAP before Ruby's remaining at_exit handlers print the test summary. + if session.adapter == "rdbg" && session.request == "launch" { + let mut drained = session.output_drained.subscribe(); + let _ = tokio::time::timeout(DEBUGGEE_EXIT_WAIT, drained.wait_for(|done| *done)).await; + } if let Some(mgr) = manager.upgrade() { mgr.session_closed(&session); } diff --git a/crates/ide-dap/src/tests.rs b/crates/ide-dap/src/tests.rs index f7dd8859b..c0ff9304f 100644 --- a/crates/ide-dap/src/tests.rs +++ b/crates/ide-dap/src/tests.rs @@ -1398,7 +1398,7 @@ fn rdbg_runs_the_debuggee_after_its_options_with_the_configuration_environment() let config = LaunchConfig { adapter: "rdbg".into(), program: Some("ruby".into()), - args: ["-Ilib", "app.rb", "--verbose"].map(String::from).to_vec(), + args: ["-Ilib", "app with spaces.rb", "$(touch sentinel); literal"].map(String::from).to_vec(), env: BTreeMap::from([("RAILS_ENV".into(), "test".into())]), extra_json: None, ..config() @@ -1406,7 +1406,10 @@ fn rdbg_runs_the_debuggee_after_its_options_with_the_configuration_environment() let mut spec = adapter::resolve(&rdbg, dir.path().into()); rdbg_command_line(&mut spec, &config, dir.path()).unwrap(); assert_eq!(spec.command, "rdbg"); - assert_eq!(spec.args, ["-O", "--port", "${port}", "-c", "--", "ruby", "-Ilib", "app.rb", "--verbose"]); + assert_eq!( + spec.args, + ["-O", "--port", "${port}", "-c", "--", "ruby", "-Ilib", "app with spaces.rb", "$(touch sentinel); literal"] + ); assert!(spec.env.contains(&("RAILS_ENV".into(), "test".into()))); } @@ -1440,6 +1443,41 @@ fn rdbg_runs_from_a_bundle_that_has_debug_and_refuses_one_without() { } } +#[test] +fn rdbg_commands_without_arguments_are_ruby_entry_points_not_shell_text() { + let dir = tempfile::tempdir().unwrap(); + let rdbg = adapter::find_config(&LanguageRegistry::from_configs(Vec::new()), "rdbg").unwrap(); + for program in ["rspec", "/app with spaces/bin/rake", "$(touch sentinel); literal"] { + let config = + LaunchConfig { adapter: "rdbg".into(), program: Some(program.into()), args: Vec::new(), ..config() }; + let mut spec = adapter::resolve(&rdbg, dir.path().into()); + rdbg_command_line(&mut spec, &config, dir.path()).unwrap(); + assert_eq!(&spec.args[4..], ["--", "ruby", "-S", "--", program]); + } +} + +#[test] +fn rdbg_remote_attach_does_not_check_the_local_bundle_or_append_a_program() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("Gemfile"), "").unwrap(); + let rdbg = adapter::find_config(&LanguageRegistry::from_configs(Vec::new()), "rdbg").unwrap(); + let config = LaunchConfig { adapter: "rdbg".into(), request: "attach".into(), ..config() }; + let mut spec = adapter::resolve(&rdbg, dir.path().into()); + let args = spec.args.clone(); + rdbg_command_line(&mut spec, &config, dir.path()).unwrap(); + assert_eq!(spec.command, "rdbg"); + assert_eq!(spec.args, args); +} + +#[test] +fn bundler_prefers_gems_rb_when_both_gemfiles_exist() { + let dir = tempfile::tempdir().unwrap(); + for name in ["Gemfile", "gems.rb"] { + std::fs::write(dir.path().join(name), "").unwrap(); + } + assert_eq!(bundler_gemfile(dir.path(), &BTreeMap::new()), Some(dir.path().join("gems.rb"))); +} + #[test] fn bundler_gemfile_and_lockfile_follow_bundle_gemfile_and_gems_rb() { let dir = tempfile::tempdir().unwrap(); @@ -1673,3 +1711,64 @@ fn delve_leftovers_are_removed_without_touching_other_files() { } std::fs::remove_dir_all(&dir).unwrap(); } + +#[cfg(unix)] +#[tokio::test] +async fn rdbg_drains_test_output_after_dap_closes_before_killing_the_process() { + struct Launcher; + impl AdapterLauncher for Launcher { + fn launch(&self, _spec: AdapterSpec, _reuse: Option) -> LaunchFuture { + Box::pin(async { + let (client, server) = tokio::io::duplex(1 << 16); + tokio::spawn(fake_adapter(server, Arc::default(), Script::default())); + let mut command = tokio::process::Command::new("/bin/sh"); + ide_core::process::own_group(command.as_std_mut()); + let mut child = command + .args(["-c", "sleep 1; printf 'test summary\\n'"]) + .stdout(std::process::Stdio::piped()) + .kill_on_drop(true) + .spawn() + .unwrap(); + let mut stdout = child.stdout.take().unwrap(); + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + tokio::spawn(async move { + let mut text = String::new(); + tokio::io::AsyncReadExt::read_to_string(&mut stdout, &mut text).await.unwrap(); + let _ = tx.send(("stdout", text)); + }); + let (reader, writer) = tokio::io::split(client); + Ok(AdapterIo { + reader: Box::new(reader), + writer: Box::new(writer), + child: Some(child), + port: None, + stderr: Arc::default(), + program_output: Some(rx), + }) + }) + } + } + let dir = tempfile::tempdir().unwrap(); + let manager = DebugManager::with_launcher(Arc::new(LanguageRegistry::from_configs(Vec::new())), Arc::new(Launcher)); + let events = Arc::new(Mutex::new(Vec::new())); + let collected = events.clone(); + let owner = SessionOwner::new("rdbg-test", Arc::new(move |e| collected.lock().push(e))); + let id = manager + .start( + LaunchConfig { adapter: "rdbg".into(), cwd: Some(dir.path().into()), extra_json: None, ..config() }, + owner, + ) + .await + .unwrap(); + manager.session(id).unwrap().conn.close(); + tokio::time::timeout(Duration::from_secs(5), async { + while !manager.sessions(None).is_empty() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + assert!(events.lock().iter().any(|e| matches!(e, + IdeEvent::Debug { event: DebugEvent::Output { output, .. }, .. } if output == "test summary\n" + ))); +} diff --git a/crates/ide-host/src/ops/debug.rs b/crates/ide-host/src/ops/debug.rs index 383cf6fde..0b61b0daa 100644 --- a/crates/ide-host/src/ops/debug.rs +++ b/crates/ide-host/src/ops/debug.rs @@ -76,7 +76,7 @@ pub async fn debug_start(host: &Host, window: &str, config: LaunchConfig) -> Ide } let root = config.cwd.clone().map(PathBuf::from).unwrap_or_default(); let mut ruby_tools = Vec::new(); - if config.adapter == "rdbg" { + if config.adapter == "rdbg" && config.request == "launch" { let project_root = host.project(window).map(|p| p.root.clone()).unwrap_or_else(|_| root.clone()); let (dir, settings) = (root.clone(), settings.clone()); (config, ruby_tools) = host diff --git a/crates/ide-sdk/src/ruby.rs b/crates/ide-sdk/src/ruby.rs index afb44fb5a..877a86018 100644 --- a/crates/ide-sdk/src/ruby.rs +++ b/crates/ide-sdk/src/ruby.rs @@ -163,7 +163,12 @@ pub fn debug_environment( let mut gem_bins: Option> = None; let mut find = |name: &str| { ide_core::process::find_executable(name, bin.as_os_str()).or_else(|| { - let dirs = gem_bins.get_or_insert_with(|| gem_bin_dirs(&ruby, dir)); + let dirs = gem_bins.get_or_insert_with(|| { + let vars: Vec<_> = env.iter().map(|(k, v)| (k.as_str(), v.as_str())).collect(); + query_in(&ruby, &["-e", ide_core::tool_path::RUBY_GEM_BIN_SCRIPT], Some(dir), &vars, QUERY_TIMEOUT) + .map(|out| ide_core::tool_path::gem_bin_dirs(&out)) + .unwrap_or_default() + }); ide_core::process::find_executable(name, &std::env::join_paths(dirs.iter()).ok()?) }) }; @@ -251,7 +256,10 @@ mod tests { } // Homebrew's Ruby installs gem executables outside its `bin`. let scripts = [ - (bin.join("ruby"), format!("#!/bin/sh\necho '{}'\n", gem_bin.display())), + ( + bin.join("ruby"), + format!("#!/bin/sh\necho \"${{GEM_HOME:-{}}}/bin\"\n", dir.path().join("gems").display()), + ), (bin.join("bundle"), String::new()), (gem_bin.join("rdbg"), String::new()), ]; @@ -266,6 +274,15 @@ mod tests { let tools = debug_environment(dir.path(), dir.path(), &s, &mut env); assert_eq!(tools, [("bundle", bin.join("bundle")), ("rdbg", gem_bin.join("rdbg"))]); assert_eq!(std::env::split_paths(&env["PATH"]).next(), Some(bin.clone())); + let custom_home = dir.path().join("custom gems"); + std::fs::create_dir_all(custom_home.join("bin")).unwrap(); + let custom_rdbg = custom_home.join("bin/rdbg"); + std::fs::write(&custom_rdbg, "").unwrap(); + std::fs::set_permissions(&custom_rdbg, std::fs::Permissions::from_mode(0o755)).unwrap(); + let mut env = + std::collections::BTreeMap::from([("GEM_HOME".into(), custom_home.to_string_lossy().into_owned())]); + let tools = debug_environment(dir.path(), dir.path(), &s, &mut env); + assert_eq!(tools, [("bundle", bin.join("bundle")), ("rdbg", custom_rdbg)]); // The configuration's own PATH stays. let mut env = std::collections::BTreeMap::from([("PATH".to_owned(), "/custom".to_owned())]); debug_environment(dir.path(), dir.path(), &s, &mut env); diff --git a/src/features/debugger/__tests__/startFailure.test.ts b/src/features/debugger/__tests__/startFailure.test.ts index 43ac10ad7..859492a89 100644 --- a/src/features/debugger/__tests__/startFailure.test.ts +++ b/src/features/debugger/__tests__/startFailure.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import { _resetI18nForTests, setLocale } from "@/platform/i18n"; import { IpcError, type LaunchConfig } from "@/platform/ipc"; import { provideNotificationService } from "@/workbench/contributions"; import { startFailureMessage } from "../startFailure"; @@ -21,6 +22,21 @@ describe("debug start failures", () => { expect(startFailureMessage(new IpcError({ kind: "invalidInput", message }))).toBe(PORT_IN_USE); }); + it("translates the missing Ruby debug gem instruction", async () => { + const message = + 'the bundle of /app with spaces/Gemfile has no `debug` gem. Add `gem "debug"` to the Gemfile and run `bundle install`'; + try { + await setLocale("en"); + expect(startFailureMessage(new Error(message))).toBe(message); + await setLocale("zh"); + expect(startFailureMessage(new Error(message))).toBe( + '/app with spaces/Gemfile 的 bundle 中没有 `debug` gem。请在 Gemfile 中添加 `gem "debug"`，然后运行 `bundle install`', + ); + } finally { + _resetI18nForTests(); + } + }); + it("keeps other failures as they are", () => { expect(startFailureMessage(new Error("launch failed: Attach requests are not supported"))).toBe( "launch failed: Attach requests are not supported", diff --git a/src/features/debugger/startFailure.ts b/src/features/debugger/startFailure.ts index 497bb94b1..dd9304194 100644 --- a/src/features/debugger/startFailure.ts +++ b/src/features/debugger/startFailure.ts @@ -6,9 +6,14 @@ import { tx } from "@/platform/i18n"; const PORT_IN_USE = /\bEADDRINUSE\b.*:(\d+)\s*$/; +const RUBY_DEBUG_MISSING = + /^the bundle of (.+) has no `debug` gem\. Add `gem "debug"` to the Gemfile and run `bundle install`$/; export function startFailureMessage(e: unknown): string { const message = e instanceof Error ? e.message : String(e); + const gemfile = RUBY_DEBUG_MISSING.exec(message)?.[1]; + if (gemfile) + return tx`the bundle of ${gemfile} has no \`debug\` gem. Add \`gem "debug"\` to the Gemfile and run \`bundle install\``; const port = PORT_IN_USE.exec(message)?.[1]; return port ? tx`Port ${port} is already in use: another IDE or debug session may be listening on it. Stop it or choose another port.` diff --git a/src/platform/locales/text/en.json b/src/platform/locales/text/en.json index 45cb292ca..c5d4989a1 100644 --- a/src/platform/locales/text/en.json +++ b/src/platform/locales/text/en.json @@ -537,5 +537,6 @@ "Select Maven toolchains.xml": "Select Maven toolchains.xml", "dev,integration": "dev,integration", "Comma-separated profiles (-P), combined with Maven tool window choices; !id excludes a profile. A run configuration’s own -P replaces these profiles.": "Comma-separated profiles (-P), combined with Maven tool window choices; !id excludes a profile. A run configuration’s own -P replaces these profiles.", - "Active profiles": "Active profiles" + "Active profiles": "Active profiles", + "the bundle of {gemfile} has no `debug` gem. Add `gem \"debug\"` to the Gemfile and run `bundle install`": "the bundle of {gemfile} has no `debug` gem. Add `gem \"debug\"` to the Gemfile and run `bundle install`" } diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index d3a893947..095f90431 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -6873,5 +6873,6 @@ "Passed as -t; selects the JDKs Maven compiles with.": "以 -t 传入，用于选择 Maven 编译使用的 JDK。", "Select Maven toolchains.xml": "选择 Maven toolchains.xml", "dev,integration": "开发,集成", - "Comma-separated profiles (-P), combined with Maven tool window choices; !id excludes a profile. A run configuration’s own -P replaces these profiles.": "以逗号分隔的 profiles（-P），与 Maven 工具窗口中的选择合并；!id 排除指定 profile。运行配置自身的 -P 会替代这些 profiles。" + "Comma-separated profiles (-P), combined with Maven tool window choices; !id excludes a profile. A run configuration’s own -P replaces these profiles.": "以逗号分隔的 profiles（-P），与 Maven 工具窗口中的选择合并；!id 排除指定 profile。运行配置自身的 -P 会替代这些 profiles。", + "the bundle of {gemfile} has no `debug` gem. Add `gem \"debug\"` to the Gemfile and run `bundle install`": "{gemfile} 的 bundle 中没有 `debug` gem。请在 Gemfile 中添加 `gem \"debug\"`，然后运行 `bundle install`" }