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>

5 files changed, 103 insertions, 23 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() {
diff --git a/src/app/mod.rs b/src/app/mod.rs
index 35244115..2095b99e 100644
--- a/src/app/mod.rs
+++ b/src/app/mod.rs
@@ -141,7 +141,7 @@ pub struct App {
     pub(crate) selection_highlight_clear_deadline: Option<Instant>,
     pub(crate) session_save_deadline: Option<Instant>,
     pub(crate) session_save_thread: Option<std::thread::JoinHandle<()>>,
-    pub(crate) detached_custom_command_children: Vec<std::process::Child>,
+    pub(crate) detached_process_children: Vec<std::process::Child>,
     tab_bar_status_generation: u64,
     tab_bar_datetimes: Vec<tab_bar_status::TabBarDatetimeRuntime>,
     tab_bar_commands: Vec<tab_bar_status::TabBarCommandRuntime>,
@@ -773,7 +773,7 @@ impl App {
             pending_agent_resume_deadline: None,
             session_save_deadline: None,
             session_save_thread: None,
-            detached_custom_command_children: Vec::new(),
+            detached_process_children: Vec::new(),
             tab_bar_status_generation: 0,
             tab_bar_datetimes: Vec::new(),
             tab_bar_commands: Vec::new(),
@@ -940,7 +940,7 @@ impl App {
         let mut host_keyboard_report_all_active = false;
 
         while !self.state.should_quit {
-            self.reap_finished_custom_commands();
+            self.reap_finished_detached_processes();
             if self.render_dirty.is_pending() {
                 needs_render = true;
             }
diff --git a/src/app/runtime.rs b/src/app/runtime.rs
index 9551055e..d7023727 100644
--- a/src/app/runtime.rs
+++ b/src/app/runtime.rs
@@ -9,7 +9,7 @@ use super::{
     background_update_check_enabled, App, AUTO_UPDATE_CHECK_INTERVAL, MIN_RENDER_INTERVAL,
     RESIZE_POLL_INTERVAL, SELECTION_AUTOSCROLL_INTERVAL,
 };
-fn retain_custom_command_after_wait(
+fn retain_detached_process_after_wait(
     pid: u32,
     result: std::io::Result<Option<std::process::ExitStatus>>,
 ) -> bool {
@@ -18,16 +18,16 @@ fn retain_custom_command_after_wait(
         Ok(Some(_)) => false,
         Err(err) if err.kind() == std::io::ErrorKind::Interrupted => true,
         Err(err) => {
-            tracing::warn!(pid, err = %err, "failed to reap detached custom command");
+            tracing::warn!(pid, err = %err, "failed to reap detached process");
             false
         }
     }
 }
 
 impl App {
-    pub(crate) fn reap_finished_custom_commands(&mut self) {
-        self.detached_custom_command_children
-            .retain_mut(|child| retain_custom_command_after_wait(child.id(), child.try_wait()));
+    pub(crate) fn reap_finished_detached_processes(&mut self) {
+        self.detached_process_children
+            .retain_mut(|child| retain_detached_process_after_wait(child.id(), child.try_wait()));
     }
 
     pub(crate) fn shutdown_terminal_runtime(&mut self, terminal_id: crate::terminal::TerminalId) {
@@ -658,10 +658,10 @@ mod tests {
     use crate::workspace::Workspace;
 
     #[test]
-    fn interrupted_custom_command_wait_keeps_child_for_retry() {
+    fn interrupted_detached_process_wait_keeps_child_for_retry() {
         let interrupted = std::io::Error::new(std::io::ErrorKind::Interrupted, "test interrupt");
 
-        assert!(retain_custom_command_after_wait(42, Err(interrupted)));
+        assert!(retain_detached_process_after_wait(42, Err(interrupted)));
     }
 
     fn test_app_with_pane() -> (super::super::App, crate::layout::PaneId) {