fix: reap ctrl-click url openers (#2906)

* fix: reap ctrl-click url openers

refs #2903

* fix: scope linux opener test import

refs #2903

* test: isolate url opener regression

refs #2903

---------

Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>

3 files changed, 93 insertions, 13 deletions

diff --git a/src/app/input/mod.rs b/src/app/input/mod.rs
index 45124107..597d9296 100644
--- a/src/app/input/mod.rs
+++ b/src/app/input/mod.rs
@@ -567,6 +567,15 @@ impl App {
         source_id: super::InputSourceId,
         mouse: MouseEvent,
     ) -> bool {
+        self.handle_modified_url_click_with(source_id, mouse, crate::platform::open_url)
+    }
+
+    fn handle_modified_url_click_with(
+        &mut self,
+        source_id: super::InputSourceId,
+        mouse: MouseEvent,
+        open_url: impl FnOnce(&str) -> std::io::Result<Option<std::process::Child>>,
+    ) -> bool {
         if self.state.mode != Mode::Terminal
             || !matches!(mouse.kind, MouseEventKind::Down(MouseButton::Left))
             || !mouse.modifiers.contains(modified_url_click_modifier())
@@ -595,8 +604,12 @@ impl App {
                 tracing::warn!(err = %err, url = %url, "failed to invoke plugin link handler");
             }
         }
-        if let Err(err) = crate::platform::open_url(&url) {
-            tracing::warn!(err = %err, url = %url, "failed to open pane URL");
+        match open_url(&url) {
+            Ok(Some(child)) => self.detached_process_children.push(child),
+            Ok(None) => {}
+            Err(err) => {
+                tracing::warn!(err = %err, url = %url, "failed to open pane URL");
+            }
         }
         true
     }
@@ -898,6 +911,18 @@ fn wait_for_file(path: &std::path::Path) -> String {
 }
 
 #[cfg(test)]
+#[cfg(unix)]
+async fn wait_for_detached_process_reap(app: &mut App, pid: u32) -> bool {
+    let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(2);
+    while crate::platform::process_exists(pid) && tokio::time::Instant::now() < deadline {
+        app.reap_finished_detached_processes();
+        tokio::time::sleep(std::time::Duration::from_millis(20)).await;
+    }
+    app.reap_finished_detached_processes();
+    !crate::platform::process_exists(pid)
+}
+
+#[cfg(test)]
 mod tests {
     use super::*;
 
diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs
index afe9e6e0..2cb657a7 100644
--- a/src/app/input/navigate.rs
+++ b/src/app/input/navigate.rs
@@ -937,7 +937,7 @@ impl App {
             command.current_dir(cwd);
         }
         let child = command.spawn()?;
-        self.detached_custom_command_children.push(child);
+        self.detached_process_children.push(child);
         Ok(())
     }
 
@@ -2015,9 +2015,9 @@ mod tests {
     use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, ModifierKeyCode};
     use ratatui::layout::Direction;
 
-    #[cfg(unix)]
-    use super::super::wait_for_file;
     use super::super::{state_with_workspaces, unique_temp_path};
+    #[cfg(unix)]
+    use super::super::{wait_for_detached_process_reap, wait_for_file};
     use super::*;
     use crate::{
         app::App,
@@ -3543,16 +3543,10 @@ navigate_pane_down = "ctrl+j"
         assert_eq!(app.state.mode, Mode::Terminal);
 
         std::fs::write(&release_path, b"release").expect("release command");
-        let deadline = tokio::time::Instant::now() + Duration::from_secs(2);
-        while crate::platform::process_exists(pid) && tokio::time::Instant::now() < deadline {
-            app.reap_finished_custom_commands();
-            tokio::time::sleep(Duration::from_millis(20)).await;
-        }
-        app.reap_finished_custom_commands();
-        let reaped_by_runtime = !crate::platform::process_exists(pid);
+        let reaped_by_runtime = wait_for_detached_process_reap(&mut app, pid).await;
         if !reaped_by_runtime {
             if let Some(child) = app
-                .detached_custom_command_children
+                .detached_process_children
                 .iter_mut()
                 .find(|child| child.id() == pid)
             {
diff --git a/src/app/input/terminal.rs b/src/app/input/terminal.rs
index 12d09e6a..e5b40a74 100644
--- a/src/app/input/terminal.rs
+++ b/src/app/input/terminal.rs
@@ -411,6 +411,8 @@ mod tests {
     use crossterm::event::{KeyCode, KeyEventKind, KeyModifiers, MouseButton, MouseEventKind};
     use ratatui::layout::Rect;
 
+    #[cfg(target_os = "linux")]
+    use super::super::wait_for_detached_process_reap;
     use super::super::{app_for_mouse_test, mouse, numbered_lines_bytes};
     #[cfg(unix)]
     use super::super::{unique_temp_path, wait_for_file};
@@ -885,6 +887,65 @@ mod tests {
         assert!(app.selection_highlight_clear_deadline.is_none());
     }
 
+    #[cfg(target_os = "linux")]
+    #[tokio::test]
+    async fn ctrl_click_url_reaps_failed_opener() {
+        let opener_dir = unique_temp_path("url-opener");
+        let record_path = opener_dir.join("record");
+        let opener_path = opener_dir.join("xdg-open");
+        std::fs::create_dir_all(&opener_dir).expect("fake opener directory");
+        std::fs::write(
+            &opener_path,
+            "#!/bin/sh\nprintf '%s\\n%s\\n' \"$$\" \"$1\" > \"$2\"\nexit 3\n",
+        )
+        .expect("fake opener script");
+
+        let url = "https://example.com/akbash-2903";
+        let line = format!("see {url}");
+        let (mut app, info) = app_with_screen_bytes(line.as_bytes());
+        let col = info.inner_rect.x + line.find("example").expect("url host") as u16;
+        let handled = app.handle_modified_url_click_with(
+            41,
+            modified_mouse(
+                MouseEventKind::Down(MouseButton::Left),
+                col,
+                info.inner_rect.y,
+                KeyModifiers::CONTROL,
+            ),
+            |clicked_url| {
+                std::process::Command::new("/bin/sh")
+                    .arg(&opener_path)
+                    .arg(clicked_url)
+                    .arg(&record_path)
+                    .stdin(std::process::Stdio::null())
+                    .stdout(std::process::Stdio::null())
+                    .stderr(std::process::Stdio::null())
+                    .spawn()
+                    .map(Some)
+            },
+        );
+        assert!(handled);
+
+        let record = wait_for_file(&record_path);
+        let mut lines = record.lines();
+        let pid = lines
+            .next()
+            .expect("opener pid")
+            .parse::<u32>()
+            .expect("numeric opener pid");
+        assert_eq!(lines.next(), Some(url));
+
+        let reaped = wait_for_detached_process_reap(&mut app, pid).await;
+        if !reaped {
+            unsafe {
+                libc::waitpid(pid as libc::pid_t, std::ptr::null_mut(), 0);
+            }
+        }
+
+        let _ = std::fs::remove_dir_all(&opener_dir);
+        assert!(reaped, "failed URL opener child {pid} was not reaped");
+    }
+
     #[cfg(unix)]
     #[tokio::test]
     async fn ctrl_click_url_does_not_forward_release_to_mouse_reporting_pane() {