diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index a1597669..7f98f530 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -843,7 +843,7 @@ impl App { } fn spawn_custom_command( - &self, + &mut self, binding: &crate::config::CustomCommandKeybind, ) -> std::io::Result<()> { let mut command = crate::platform::detached_custom_command_process(&binding.command); @@ -856,7 +856,8 @@ impl App { if let Some(cwd) = cwd { command.current_dir(cwd); } - command.spawn()?; + let child = command.spawn()?; + self.detached_custom_command_children.push(child); Ok(()) } @@ -2943,9 +2944,11 @@ navigate_pane_down = "ctrl+j" app.state.mode = Mode::Terminal; let output_path = unique_temp_path("custom-command-keybind"); + let release_path = unique_temp_path("custom-command-release"); let command = format!( - "printf '%s\\n%s\\n%s\\n' \"$HERDR_ACTIVE_WORKSPACE_ID\" \"$HERDR_ACTIVE_TAB_ID\" \"$HERDR_ACTIVE_PANE_ID\" > '{}'", - output_path.display() + "printf '%s\\n%s\\n%s\\n%s\\n' \"$$\" \"$HERDR_ACTIVE_WORKSPACE_ID\" \"$HERDR_ACTIVE_TAB_ID\" \"$HERDR_ACTIVE_PANE_ID\" > '{}'; i=0; while [ ! -e '{}' ] && [ \"$i\" -lt 250 ]; do sleep 0.02; i=$((i + 1)); done", + output_path.display(), + release_path.display(), ); app.state.keybinds.custom_commands = vec![crate::config::CustomCommandKeybind { bindings: crate::config::ActionKeybinds::prefix("m"), @@ -2962,18 +2965,48 @@ navigate_pane_down = "ctrl+j" .await; assert_eq!(app.state.mode, Mode::Prefix); + let launch_started = std::time::Instant::now(); app.handle_key(TerminalKey::new(KeyCode::Char('m'), KeyModifiers::empty())) .await; + assert!(launch_started.elapsed() < Duration::from_secs(2)); let content = wait_for_file(&output_path); let lines: Vec<&str> = content.lines().collect(); - assert_eq!(lines.len(), 3); - assert_eq!(lines[0], app.state.workspaces[0].id); - assert_eq!(lines[1], format!("{}:t1", app.state.workspaces[0].id)); - assert_eq!(lines[2], format!("{}:p1", app.state.workspaces[0].id)); + assert_eq!(lines.len(), 4); + let pid = lines[0] + .parse::() + .expect("command should report its pid"); + assert!(crate::platform::process_exists(pid)); + assert_eq!(lines[1], app.state.workspaces[0].id); + assert_eq!(lines[2], format!("{}:t1", app.state.workspaces[0].id)); + assert_eq!(lines[3], format!("{}:p1", app.state.workspaces[0].id)); 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); + if !reaped_by_runtime { + if let Some(child) = app + .detached_custom_command_children + .iter_mut() + .find(|child| child.id() == pid) + { + let _ = child.kill(); + let _ = child.wait(); + } + } + assert!( + reaped_by_runtime, + "detached command child {pid} was not reaped" + ); + let _ = std::fs::remove_file(output_path); + let _ = std::fs::remove_file(release_path); } #[cfg(unix)] diff --git a/src/app/mod.rs b/src/app/mod.rs index bff04c44..16cfeafb 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -130,6 +130,7 @@ pub struct App { pub(crate) selection_highlight_clear_deadline: Option, pub(crate) session_save_deadline: Option, pub(crate) session_save_thread: Option>, + pub(crate) detached_custom_command_children: Vec, pub(crate) persist_pane_history: bool, pub(crate) last_render_at: Option, pub(crate) suppressed_repeat_keys: @@ -725,6 +726,7 @@ impl App { pending_agent_resume_deadline: None, session_save_deadline: None, session_save_thread: None, + detached_custom_command_children: Vec::new(), selection_autoscroll_deadline: None, selection_highlight_clear_deadline: None, persist_pane_history: config.experimental.pane_history, @@ -879,6 +881,7 @@ impl App { let mut host_mouse_capture_active = self.state.mouse_capture; while !self.state.should_quit { + self.reap_finished_custom_commands(); if self.render_dirty.load(Ordering::Acquire) { needs_render = true; } diff --git a/src/app/runtime.rs b/src/app/runtime.rs index 88081743..25275cd7 100644 --- a/src/app/runtime.rs +++ b/src/app/runtime.rs @@ -38,7 +38,27 @@ pub(crate) struct WorkspaceGitRefreshOutput { pub(crate) cache_updates: Vec<(std::path::PathBuf, GitStatusCacheEntry)>, } +fn retain_custom_command_after_wait( + pid: u32, + result: std::io::Result>, +) -> bool { + match result { + Ok(None) => true, + 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"); + 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 shutdown_detached_terminal_runtimes(&mut self) { let terminal_ids = std::mem::take(&mut self.state.terminal_runtime_shutdowns); for terminal_id in terminal_ids { @@ -697,6 +717,13 @@ mod tests { use crate::workspace::Workspace; use std::path::PathBuf; + #[test] + fn interrupted_custom_command_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))); + } + fn test_app_with_pane() -> (super::super::App, crate::layout::PaneId) { let mut app = super::super::App::new( &crate::config::Config::default(), diff --git a/src/server/headless.rs b/src/server/headless.rs index 6cec130a..c965d8fb 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -445,6 +445,7 @@ impl HeadlessServer { loop { crate::render_prof::event("loop.tick"); crate::render_prof::flush_if_due(); + self.app.reap_finished_custom_commands(); // If shutdown has been initiated, complete it and exit. if self.shutting_down { diff --git a/src/ui/sidebar.rs b/src/ui/sidebar.rs index 18d13b26..cedae025 100644 --- a/src/ui/sidebar.rs +++ b/src/ui/sidebar.rs @@ -2067,6 +2067,9 @@ mod tests { workspace_with_worktree_space("issue", Some("repo-key"), "/repo/herdr-issue"), Workspace::test_new("notes"), ]; + for workspace in &mut app.workspaces { + workspace.cached_git_branch = Some("main".into()); + } app.collapsed_space_keys.insert("repo-key".into()); app.active = None; app.mode = Mode::Terminal;