From a85d6dfd1e99bb6f21a9f2ac1c2ba65165702559 Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod <62684960+ssddOnTop@users.noreply.github.com> Date: Tue, 24 Mar 2026 02:11:43 -0400 Subject: [PATCH 01/11] feat(shell): add background process manager with log lifecycle and persistence --- ...plate_engine_renders_background_shell.snap | 26 + crates/forge_domain/src/background_process.rs | 290 +++++++++++ .../src/tool_services/background_process.rs | 461 ++++++++++++++++++ 3 files changed, 777 insertions(+) create mode 100644 crates/forge_app/src/snapshots/forge_app__compact__tests__template_engine_renders_background_shell.snap create mode 100644 crates/forge_domain/src/background_process.rs create mode 100644 crates/forge_services/src/tool_services/background_process.rs diff --git a/crates/forge_app/src/snapshots/forge_app__compact__tests__template_engine_renders_background_shell.snap b/crates/forge_app/src/snapshots/forge_app__compact__tests__template_engine_renders_background_shell.snap new file mode 100644 index 0000000000..3bc8e4ea1f --- /dev/null +++ b/crates/forge_app/src/snapshots/forge_app__compact__tests__template_engine_renders_background_shell.snap @@ -0,0 +1,26 @@ +--- +source: crates/forge_app/src/compact.rs +expression: actual +--- +Use the following summary frames as the authoritative reference for all coding suggestions and decisions. Do not re-explain or revisit it unless I ask. Additional summary frames will be added as the conversation progresses. + +## Summary + +### 1. User + +```` +Start the dev server +```` + +### 2. Assistant + +**Execute:** +``` +npm start +``` +**Background Process:** PID=12345, Log=`/tmp/forge-bg-npm-start.log` + + +--- + +Proceed with implementation based on this context. diff --git a/crates/forge_domain/src/background_process.rs b/crates/forge_domain/src/background_process.rs new file mode 100644 index 0000000000..d28d653df0 --- /dev/null +++ b/crates/forge_domain/src/background_process.rs @@ -0,0 +1,290 @@ +use std::path::PathBuf; +use std::sync::Mutex; + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +/// Metadata for a single background process spawned by the shell tool. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct BackgroundProcess { + /// OS process ID. + pub pid: u32, + /// The original command string that was executed. + pub command: String, + /// Absolute path to the log file capturing stdout/stderr. + pub log_file: PathBuf, + /// When the process was spawned. + pub started_at: DateTime, +} + +/// Owns the temp-file handles for background process log files so that they +/// are automatically cleaned up when the manager is dropped. +struct OwnedLogFile { + /// Keeping the `NamedTempFile` alive prevents cleanup; when dropped the + /// file is deleted. + _handle: tempfile::NamedTempFile, + /// Associated PID so we can remove the handle when the process is killed. + pid: u32, +} + +impl std::fmt::Debug for OwnedLogFile { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("OwnedLogFile") + .field("pid", &self.pid) + .finish() + } +} + +/// Thread-safe registry of background processes spawned during the current +/// session. +/// +/// When the manager is dropped all owned temp-file handles are released, +/// causing the underlying log files to be deleted automatically. +#[derive(Debug, Default)] +pub struct BackgroundProcessManager { + processes: Mutex>, + log_handles: Mutex>, +} + +impl BackgroundProcessManager { + /// Creates a new, empty manager. + pub fn new() -> Self { + Self::default() + } + + /// Register a newly spawned background process. + /// + /// # Arguments + /// + /// * `pid` - OS process id of the spawned process. + /// * `command` - The command string that was executed. + /// * `log_file` - Absolute path to the log file. + /// * `log_handle` - The `NamedTempFile` handle that owns the log file on + /// disk. Kept alive until the process is removed or the manager is + /// dropped. + pub fn register( + &self, + pid: u32, + command: String, + log_file: PathBuf, + log_handle: tempfile::NamedTempFile, + ) -> BackgroundProcess { + let process = BackgroundProcess { + pid, + command, + log_file, + started_at: Utc::now(), + }; + self.processes + .lock() + .expect("lock poisoned") + .push(process.clone()); + self.log_handles + .lock() + .expect("lock poisoned") + .push(OwnedLogFile { _handle: log_handle, pid }); + process + } + + /// Returns a snapshot of all tracked background processes. + pub fn list(&self) -> Vec { + self.processes + .lock() + .expect("lock poisoned") + .clone() + } + + /// Find a background process by PID. + pub fn find(&self, pid: u32) -> Option { + self.processes + .lock() + .expect("lock poisoned") + .iter() + .find(|p| p.pid == pid) + .cloned() + } + + /// Remove a background process by PID. + /// + /// This also drops the associated log-file handle. If `delete_log` is + /// `false` the handle is persisted (leaked) so the file survives on disk. + pub fn remove(&self, pid: u32, delete_log: bool) { + self.processes + .lock() + .expect("lock poisoned") + .retain(|p| p.pid != pid); + + if delete_log { + // Simply removing the OwnedLogFile will drop the NamedTempFile, + // deleting the file on disk. + self.log_handles + .lock() + .expect("lock poisoned") + .retain(|h| h.pid != pid); + } else { + // Persist the file by taking ownership and calling `persist` (or + // `keep`) so the drop won't delete it. + let mut handles = self.log_handles.lock().expect("lock poisoned"); + if let Some(pos) = handles.iter().position(|h| h.pid == pid) { + let owned = handles.remove(pos); + // Persist: consumes the handle without deleting the file. + let _ = owned._handle.keep(); + } + } + } + + /// Returns the number of tracked processes. + pub fn len(&self) -> usize { + self.processes.lock().expect("lock poisoned").len() + } + + /// Returns true if no background processes are tracked. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } +} + +#[cfg(test)] +mod tests { + use std::io::Write; + + use pretty_assertions::assert_eq; + + use super::*; + + fn create_temp_log() -> tempfile::NamedTempFile { + let mut f = tempfile::Builder::new() + .prefix("forge-bg-test-") + .suffix(".log") + .tempfile() + .unwrap(); + writeln!(f, "test log content").unwrap(); + f + } + + #[test] + fn test_register_and_list() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + fixture.register(1234, "npm start".to_string(), log_path.clone(), log); + + let actual = fixture.list(); + + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].pid, 1234); + assert_eq!(actual[0].command, "npm start"); + assert_eq!(actual[0].log_file, log_path); + } + + #[test] + fn test_find_existing() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + fixture.register(42, "python server.py".to_string(), log_path, log); + + let actual = fixture.find(42); + + assert!(actual.is_some()); + assert_eq!(actual.unwrap().pid, 42); + } + + #[test] + fn test_find_missing() { + let fixture = BackgroundProcessManager::new(); + + let actual = fixture.find(999); + + assert!(actual.is_none()); + } + + #[test] + fn test_remove_with_log_deletion() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + fixture.register(100, "node app.js".to_string(), log_path.clone(), log); + assert_eq!(fixture.len(), 1); + + fixture.remove(100, true); + + assert_eq!(fixture.len(), 0); + assert!(fixture.find(100).is_none()); + // The temp file should be deleted + assert!(!log_path.exists()); + } + + #[test] + fn test_remove_without_log_deletion() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + fixture.register(200, "cargo watch".to_string(), log_path.clone(), log); + + fixture.remove(200, false); + + assert_eq!(fixture.len(), 0); + // The temp file should be persisted (kept) + assert!(log_path.exists()); + + // Cleanup for test hygiene + let _ = std::fs::remove_file(&log_path); + } + + #[test] + fn test_multiple_processes() { + let fixture = BackgroundProcessManager::new(); + + let log1 = create_temp_log(); + let path1 = log1.path().to_path_buf(); + let log2 = create_temp_log(); + let path2 = log2.path().to_path_buf(); + + fixture.register(10, "server1".to_string(), path1, log1); + fixture.register(20, "server2".to_string(), path2, log2); + + assert_eq!(fixture.len(), 2); + assert!(fixture.find(10).is_some()); + assert!(fixture.find(20).is_some()); + + fixture.remove(10, true); + + assert_eq!(fixture.len(), 1); + assert!(fixture.find(10).is_none()); + assert!(fixture.find(20).is_some()); + } + + #[test] + fn test_is_empty() { + let fixture = BackgroundProcessManager::new(); + + assert!(fixture.is_empty()); + + let log = create_temp_log(); + let path = log.path().to_path_buf(); + fixture.register(1, "cmd".to_string(), path, log); + + assert!(!fixture.is_empty()); + } + + #[test] + fn test_drop_cleans_up_temp_files() { + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + { + let manager = BackgroundProcessManager::new(); + manager.register(300, "temp cmd".to_string(), log_path.clone(), log); + assert!(log_path.exists()); + // manager dropped here + } + + // After drop, the temp file should be deleted + assert!(!log_path.exists()); + } +} diff --git a/crates/forge_services/src/tool_services/background_process.rs b/crates/forge_services/src/tool_services/background_process.rs new file mode 100644 index 0000000000..efab4d42d1 --- /dev/null +++ b/crates/forge_services/src/tool_services/background_process.rs @@ -0,0 +1,461 @@ +use std::path::PathBuf; +use std::sync::Mutex; + +use chrono::Utc; +use forge_domain::BackgroundProcess; + +/// Owns the temp-file handles for background process log files so that they +/// are automatically cleaned up when the manager is dropped. +struct OwnedLogFile { + /// Keeping the `NamedTempFile` alive prevents cleanup; when dropped the + /// file is deleted. + _handle: tempfile::NamedTempFile, + /// Associated PID so we can remove the handle when the process is killed. + pid: u32, +} + +impl std::fmt::Debug for OwnedLogFile { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("OwnedLogFile") + .field("pid", &self.pid) + .finish() + } +} + +/// Thread-safe registry of background processes spawned during the current +/// session. +/// +/// When the manager is dropped all owned temp-file handles are released, +/// causing the underlying log files to be deleted automatically. +/// +/// Optionally persists process metadata to a JSON file so that other processes +/// (e.g. the ZSH plugin) can list and kill background processes that were +/// spawned in earlier invocations. +#[derive(Debug)] +pub struct BackgroundProcessManager { + processes: Mutex>, + log_handles: Mutex>, + /// Optional path for persisting process metadata to disk. + persist_path: Option, +} + +impl Default for BackgroundProcessManager { + fn default() -> Self { + Self { + processes: Mutex::new(Vec::new()), + log_handles: Mutex::new(Vec::new()), + persist_path: None, + } + } +} + +impl BackgroundProcessManager { + /// Creates a new, empty manager. + pub fn new() -> Self { + Self::default() + } + + /// Creates a manager that persists process metadata to the given path. + /// + /// If the file already exists, previously tracked processes are loaded + /// (without their log-file handles - those belong to the original session). + pub fn with_persistence(path: PathBuf) -> Self { + let processes = Self::load_from_disk(&path).unwrap_or_default(); + Self { + processes: Mutex::new(processes), + log_handles: Mutex::new(Vec::new()), + persist_path: Some(path), + } + } + + /// Saves the current process list to the persistence file, if configured. + fn persist(&self) { + if let Some(ref path) = self.persist_path { + let procs = self.processes.lock().expect("lock poisoned"); + if let Some(parent) = path.parent() { + let _ = std::fs::create_dir_all(parent); + } + let _ = std::fs::write(path, serde_json::to_string_pretty(&*procs).unwrap_or_default()); + } + } + + /// Loads process list from a JSON file on disk. + fn load_from_disk(path: &PathBuf) -> Option> { + let data = std::fs::read_to_string(path).ok()?; + serde_json::from_str(&data).ok() + } + + /// Register a newly spawned background process. + /// + /// # Arguments + /// + /// * `pid` - OS process id of the spawned process. + /// * `command` - The command string that was executed. + /// * `log_file` - Absolute path to the log file. + /// * `log_handle` - The `NamedTempFile` handle that owns the log file on + /// disk. Kept alive until the process is removed or the manager is + /// dropped. + pub fn register( + &self, + pid: u32, + command: String, + log_file: PathBuf, + log_handle: tempfile::NamedTempFile, + ) -> BackgroundProcess { + let process = BackgroundProcess { + pid, + command, + log_file, + started_at: Utc::now(), + }; + self.processes + .lock() + .expect("lock poisoned") + .push(process.clone()); + self.log_handles + .lock() + .expect("lock poisoned") + .push(OwnedLogFile { _handle: log_handle, pid }); + self.persist(); + process + } + + /// Returns a snapshot of all tracked background processes. + pub fn list(&self) -> Vec { + self.processes + .lock() + .expect("lock poisoned") + .clone() + } + + /// Find a background process by PID. + pub fn find(&self, pid: u32) -> Option { + self.processes + .lock() + .expect("lock poisoned") + .iter() + .find(|p| p.pid == pid) + .cloned() + } + + /// Remove a background process by PID. + /// + /// This also drops the associated log-file handle. If `delete_log` is + /// `false` the handle is persisted (leaked) so the file survives on disk. + pub fn remove(&self, pid: u32, delete_log: bool) { + self.processes + .lock() + .expect("lock poisoned") + .retain(|p| p.pid != pid); + + if delete_log { + self.log_handles + .lock() + .expect("lock poisoned") + .retain(|h| h.pid != pid); + } else { + let mut handles = self.log_handles.lock().expect("lock poisoned"); + if let Some(pos) = handles.iter().position(|h| h.pid == pid) { + let owned = handles.remove(pos); + let _ = owned._handle.keep(); + } + } + self.persist(); + } + + /// Returns the number of tracked processes. + pub fn len(&self) -> usize { + self.processes.lock().expect("lock poisoned").len() + } + + /// Returns true if no background processes are tracked. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Kills a background process by PID and removes it from tracking. + /// + /// Returns `Ok(())` if the process was killed or was already dead. + /// The `delete_log` flag controls whether the log file is deleted. + pub fn kill(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { + kill_process(pid)?; + self.remove(pid, delete_log); + Ok(()) + } + + /// Returns a snapshot of all tracked processes with their alive status. + pub fn list_with_status(&self) -> Vec<(BackgroundProcess, bool)> { + self.processes + .lock() + .expect("lock poisoned") + .iter() + .map(|p| { + let alive = is_process_alive(p.pid); + (p.clone(), alive) + }) + .collect() + } +} + +/// Cross-platform check whether a process is still running. +#[cfg(unix)] +fn is_process_alive(pid: u32) -> bool { + unsafe { libc::kill(pid as libc::pid_t, 0) == 0 } +} + +#[cfg(windows)] +fn is_process_alive(pid: u32) -> bool { + use std::process::Command; + Command::new("tasklist") + .args(["/FI", &format!("PID eq {pid}"), "/NH"]) + .output() + .map(|o| { + let stdout = String::from_utf8_lossy(&o.stdout); + stdout.contains(&pid.to_string()) + }) + .unwrap_or(false) +} + +#[cfg(not(any(unix, windows)))] +fn is_process_alive(_pid: u32) -> bool { + false +} + +/// Cross-platform process termination. +#[cfg(unix)] +fn kill_process(pid: u32) -> anyhow::Result<()> { + let ret = unsafe { libc::kill(pid as libc::pid_t, libc::SIGTERM) }; + if ret != 0 { + let err = std::io::Error::last_os_error(); + if err.raw_os_error() == Some(libc::ESRCH) { + return Ok(()); + } + return Err(anyhow::anyhow!("Failed to kill process {pid}: {err}")); + } + Ok(()) +} + +#[cfg(windows)] +fn kill_process(pid: u32) -> anyhow::Result<()> { + use std::process::Command; + let output = Command::new("taskkill") + .args(["/PID", &pid.to_string(), "/F"]) + .output()?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + if !stderr.contains("not found") && !stderr.contains("not running") { + return Err(anyhow::anyhow!( + "Failed to kill process {pid}: {stderr}" + )); + } + } + Ok(()) +} + +#[cfg(not(any(unix, windows)))] +fn kill_process(_pid: u32) -> anyhow::Result<()> { + Err(anyhow::anyhow!( + "Killing background processes is not supported on this platform" + )) +} + +#[cfg(test)] +mod tests { + use std::io::Write; + + use pretty_assertions::assert_eq; + + use super::*; + + fn create_temp_log() -> tempfile::NamedTempFile { + let mut f = tempfile::Builder::new() + .prefix("forge-bg-test-") + .suffix(".log") + .tempfile() + .unwrap(); + writeln!(f, "test log content").unwrap(); + f + } + + #[test] + fn test_register_and_list() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + fixture.register(1234, "npm start".to_string(), log_path.clone(), log); + + let actual = fixture.list(); + + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].pid, 1234); + assert_eq!(actual[0].command, "npm start"); + assert_eq!(actual[0].log_file, log_path); + } + + #[test] + fn test_find_existing() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + fixture.register(42, "python server.py".to_string(), log_path, log); + + let actual = fixture.find(42); + + assert!(actual.is_some()); + assert_eq!(actual.unwrap().pid, 42); + } + + #[test] + fn test_find_missing() { + let fixture = BackgroundProcessManager::new(); + + let actual = fixture.find(999); + + assert!(actual.is_none()); + } + + #[test] + fn test_remove_with_log_deletion() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + fixture.register(100, "node app.js".to_string(), log_path.clone(), log); + assert_eq!(fixture.len(), 1); + + fixture.remove(100, true); + + assert_eq!(fixture.len(), 0); + assert!(fixture.find(100).is_none()); + assert!(!log_path.exists()); + } + + #[test] + fn test_remove_without_log_deletion() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + fixture.register(200, "cargo watch".to_string(), log_path.clone(), log); + + fixture.remove(200, false); + + assert_eq!(fixture.len(), 0); + assert!(log_path.exists()); + + let _ = std::fs::remove_file(&log_path); + } + + #[test] + fn test_multiple_processes() { + let fixture = BackgroundProcessManager::new(); + + let log1 = create_temp_log(); + let path1 = log1.path().to_path_buf(); + let log2 = create_temp_log(); + let path2 = log2.path().to_path_buf(); + + fixture.register(10, "server1".to_string(), path1, log1); + fixture.register(20, "server2".to_string(), path2, log2); + + assert_eq!(fixture.len(), 2); + assert!(fixture.find(10).is_some()); + assert!(fixture.find(20).is_some()); + + fixture.remove(10, true); + + assert_eq!(fixture.len(), 1); + assert!(fixture.find(10).is_none()); + assert!(fixture.find(20).is_some()); + } + + #[test] + fn test_is_empty() { + let fixture = BackgroundProcessManager::new(); + + assert!(fixture.is_empty()); + + let log = create_temp_log(); + let path = log.path().to_path_buf(); + fixture.register(1, "cmd".to_string(), path, log); + + assert!(!fixture.is_empty()); + } + + #[test] + fn test_drop_cleans_up_temp_files() { + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + { + let manager = BackgroundProcessManager::new(); + manager.register(300, "temp cmd".to_string(), log_path.clone(), log); + assert!(log_path.exists()); + } + + assert!(!log_path.exists()); + } + + #[test] + fn test_persistence_write_and_reload() { + let dir = tempfile::tempdir().unwrap(); + let persist_path = dir.path().join("processes.json"); + + { + let manager = BackgroundProcessManager::with_persistence(persist_path.clone()); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + manager.register(500, "persistent cmd".to_string(), log_path, log); + } + + assert!(persist_path.exists()); + + let reloaded = BackgroundProcessManager::with_persistence(persist_path); + let actual = reloaded.list(); + + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].pid, 500); + assert_eq!(actual[0].command, "persistent cmd"); + } + + #[test] + fn test_persistence_removes_entry() { + let dir = tempfile::tempdir().unwrap(); + let persist_path = dir.path().join("processes.json"); + + let manager = BackgroundProcessManager::with_persistence(persist_path.clone()); + let log1 = create_temp_log(); + let log2 = create_temp_log(); + let path1 = log1.path().to_path_buf(); + let path2 = log2.path().to_path_buf(); + + manager.register(600, "cmd1".to_string(), path1, log1); + manager.register(700, "cmd2".to_string(), path2, log2); + assert_eq!(manager.len(), 2); + + manager.remove(600, true); + + let reloaded = BackgroundProcessManager::with_persistence(persist_path); + let actual = reloaded.list(); + + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].pid, 700); + } + + #[test] + fn test_list_with_status() { + let fixture = BackgroundProcessManager::new(); + let log = create_temp_log(); + let path = log.path().to_path_buf(); + + fixture.register(99999, "ghost".to_string(), path, log); + + let actual = fixture.list_with_status(); + + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].0.pid, 99999); + assert!(!actual[0].1); + } +} From 1faa70cc498874a4ae793a0387b316da6e1d41d0 Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod <62684960+ssddOnTop@users.noreply.github.com> Date: Tue, 24 Mar 2026 02:11:47 -0400 Subject: [PATCH 02/11] feat(shell): add background execution mode for shell commands --- Cargo.lock | 2 + crates/forge_api/src/api.rs | 6 + crates/forge_api/src/forge_api.rs | 12 +- crates/forge_app/src/fmt/fmt_input.rs | 13 +- crates/forge_app/src/fmt/fmt_output.rs | 14 +- crates/forge_app/src/git_app.rs | 36 +- crates/forge_app/src/infra.rs | 16 +- crates/forge_app/src/operation.rs | 115 +++--- crates/forge_app/src/orch_spec/orch_runner.rs | 16 +- .../src/orch_spec/orch_system_spec.rs | 10 +- crates/forge_app/src/services.rs | 51 ++- ...plate_engine_renders_background_shell.snap | 26 -- ...istry__all_rendered_tool_descriptions.snap | 11 + crates/forge_app/src/system_prompt.rs | 6 +- crates/forge_app/src/tool_executor.rs | 49 +-- .../src/transformers/trim_context_summary.rs | 2 +- crates/forge_domain/Cargo.toml | 1 + crates/forge_domain/src/background_process.rs | 275 +------------- crates/forge_domain/src/lib.rs | 2 + crates/forge_domain/src/shell.rs | 19 + crates/forge_domain/src/tools/catalog.rs | 12 + ..._definition__usage__tests__tool_usage.snap | 2 +- .../src/tools/descriptions/shell.md | 11 + ..._catalog__tests__tool_definition_json.snap | 4 + crates/forge_infra/src/executor.rs | 112 +++++- crates/forge_infra/src/forge_infra.rs | 15 +- crates/forge_main/src/built_in_commands.json | 4 + crates/forge_main/src/cli.rs | 15 + crates/forge_main/src/model.rs | 6 + crates/forge_main/src/ui.rs | 122 ++++++ crates/forge_repo/src/forge_repo.rs | 11 + ...s__openai_responses_all_catalog_tools.snap | 7 +- crates/forge_services/Cargo.toml | 2 + .../src/tool_services/background_process.rs | 358 +++++------------- .../forge_services/src/tool_services/mod.rs | 2 + .../forge_services/src/tool_services/shell.rs | 166 ++++++-- shell-plugin/lib/dispatcher.zsh | 3 + 37 files changed, 829 insertions(+), 705 deletions(-) delete mode 100644 crates/forge_app/src/snapshots/forge_app__compact__tests__template_engine_renders_background_shell.snap diff --git a/Cargo.lock b/Cargo.lock index cbbb1dc096..62ad75c6d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1915,6 +1915,7 @@ dependencies = [ "serde_yml", "strum 0.28.0", "strum_macros 0.28.0", + "tempfile", "thiserror 2.0.18", "tokio", "tokio-stream", @@ -2199,6 +2200,7 @@ dependencies = [ "strip-ansi-escapes", "strum 0.28.0", "strum_macros 0.28.0", + "sysinfo 0.38.2", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/crates/forge_api/src/api.rs b/crates/forge_api/src/api.rs index 2a58651881..596b7c793c 100644 --- a/crates/forge_api/src/api.rs +++ b/crates/forge_api/src/api.rs @@ -252,4 +252,10 @@ pub trait API: Sync + Send { &self, data_parameters: DataGenerationParameters, ) -> Result>>; + + /// Returns all tracked background processes with their alive status. + fn list_background_processes(&self) -> Result>; + + /// Kills a background process by PID and optionally deletes its log file. + fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()>; } diff --git a/crates/forge_api/src/forge_api.rs b/crates/forge_api/src/forge_api.rs index 037ae8565f..49e8baa130 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -8,8 +8,8 @@ use forge_app::{ AgentProviderResolver, AgentRegistry, AppConfigService, AuthService, CommandInfra, CommandLoaderService, ConversationService, DataGenerationApp, EnvironmentInfra, EnvironmentService, FileDiscoveryService, ForgeApp, GitApp, GrpcInfra, McpConfigManager, - McpService, ProviderAuthService, ProviderService, Services, User, UserUsage, Walker, - WorkspaceService, + McpService, ProviderAuthService, ProviderService, Services, ShellService, User, UserUsage, + Walker, WorkspaceService, }; use forge_domain::{Agent, ConsoleWriter, InitAuth, LoginInfo, *}; use forge_infra::ForgeInfra; @@ -415,6 +415,14 @@ impl< app.execute(data_parameters).await } + fn list_background_processes(&self) -> Result> { + self.services.shell_service().list_background_processes() + } + + fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()> { + self.services.shell_service().kill_background_process(pid, delete_log) + } + async fn get_default_provider(&self) -> Result> { let provider_id = self.services.get_default_provider().await?; self.services.get_provider(provider_id).await diff --git a/crates/forge_app/src/fmt/fmt_input.rs b/crates/forge_app/src/fmt/fmt_input.rs index 931bcdcbfd..61701dadb2 100644 --- a/crates/forge_app/src/fmt/fmt_input.rs +++ b/crates/forge_app/src/fmt/fmt_input.rs @@ -100,11 +100,14 @@ impl FormatContent for ToolCatalog { let display_path = display_path_for(&input.path); Some(TitleFormat::debug("Undo").sub_title(display_path).into()) } - ToolCatalog::Shell(input) => Some( - TitleFormat::debug(format!("Execute [{}]", env.shell)) - .sub_title(&input.command) - .into(), - ), + ToolCatalog::Shell(input) => { + let label = if input.background { + format!("Spawned [{}]", env.shell) + } else { + format!("Execute [{}]", env.shell) + }; + Some(TitleFormat::debug(label).sub_title(&input.command).into()) + } ToolCatalog::Fetch(input) => { Some(TitleFormat::debug("GET").sub_title(&input.url).into()) } diff --git a/crates/forge_app/src/fmt/fmt_output.rs b/crates/forge_app/src/fmt/fmt_output.rs index 22327bef2f..3e503be8ca 100644 --- a/crates/forge_app/src/fmt/fmt_output.rs +++ b/crates/forge_app/src/fmt/fmt_output.rs @@ -65,7 +65,7 @@ mod tests { use crate::operation::ToolOperation; use crate::{ Content, FsRemoveOutput, FsUndoOutput, FsWriteOutput, HttpResponse, Match, MatchResult, - PatchOutput, ReadOutput, ResponseContext, SearchResult, ShellOutput, + PatchOutput, ReadOutput, ResponseContext, SearchResult, ShellOutput, ShellOutputKind, }; // ContentFormat methods are now implemented in ChatResponseContent @@ -421,12 +421,12 @@ mod tests { fn test_shell_success() { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "ls -la".to_string(), stdout: "file1.txt\nfile2.txt".to_string(), stderr: "".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -443,12 +443,12 @@ mod tests { fn test_shell_success_with_stderr() { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "command_with_warnings".to_string(), stdout: "output line".to_string(), stderr: "warning line".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -465,12 +465,12 @@ mod tests { fn test_shell_failure() { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "failing_command".to_string(), stdout: "".to_string(), stderr: "Error: command not found".to_string(), exit_code: Some(127), - }, + }), shell: "/bin/bash".to_string(), description: None, }, diff --git a/crates/forge_app/src/git_app.rs b/crates/forge_app/src/git_app.rs index c798aa463f..dbc9ead852 100644 --- a/crates/forge_app/src/git_app.rs +++ b/crates/forge_app/src/git_app.rs @@ -162,23 +162,25 @@ where let commit_result = self .services - .execute(commit_command, cwd, false, true, None, None) + .execute(commit_command, cwd, false, true, false, None, None) .await .context("Failed to commit changes")?; - if !commit_result.output.success() { - anyhow::bail!("Git commit failed: {}", commit_result.output.stderr); + let output = commit_result.foreground().expect("git commit runs in foreground"); + + if !output.success() { + anyhow::bail!("Git commit failed: {}", output.stderr); } // Combine stdout and stderr for logging - let git_output = if commit_result.output.stdout.is_empty() { - commit_result.output.stderr.clone() - } else if commit_result.output.stderr.is_empty() { - commit_result.output.stdout.clone() + let git_output = if output.stdout.is_empty() { + output.stderr.clone() + } else if output.stderr.is_empty() { + output.stdout.clone() } else { format!( "{}\n{}", - commit_result.output.stdout, commit_result.output.stderr + output.stdout, output.stderr ) }; @@ -230,6 +232,7 @@ where cwd.to_path_buf(), false, true, + false, None, None, ), @@ -238,6 +241,7 @@ where cwd.to_path_buf(), false, true, + false, None, None, ), @@ -246,7 +250,10 @@ where let recent_commits = recent_commits.context("Failed to get recent commits")?; let branch_name = branch_name.context("Failed to get branch name")?; - Ok((recent_commits.output.stdout, branch_name.output.stdout)) + Ok(( + recent_commits.foreground().expect("git log runs in foreground").stdout.clone(), + branch_name.foreground().expect("git rev-parse runs in foreground").stdout.clone(), + )) } /// Fetches diff from git (staged or unstaged) @@ -257,6 +264,7 @@ where cwd.to_path_buf(), false, true, + false, None, None, ), @@ -265,6 +273,7 @@ where cwd.to_path_buf(), false, true, + false, None, None, ) @@ -274,17 +283,18 @@ where let unstaged_diff = unstaged_diff.context("Failed to get unstaged changes")?; // Use staged changes if available, otherwise fall back to unstaged changes - let has_staged_files = !staged_diff.output.stdout.trim().is_empty(); + let has_staged_files = !staged_diff.foreground().expect("git diff runs in foreground").stdout.trim().is_empty(); let diff_output = if has_staged_files { staged_diff - } else if !unstaged_diff.output.stdout.trim().is_empty() { + } else if !unstaged_diff.foreground().expect("git diff runs in foreground").stdout.trim().is_empty() { unstaged_diff } else { return Err(GitAppError::NoChangesToCommit.into()); }; - let size = diff_output.output.stdout.len(); - Ok((diff_output.output.stdout, size, has_staged_files)) + let fg = diff_output.foreground().expect("git diff runs in foreground"); + let size = fg.stdout.len(); + Ok((fg.stdout.clone(), size, has_staged_files)) } /// Resolves the provider and model from the active agent's configuration. diff --git a/crates/forge_app/src/infra.rs b/crates/forge_app/src/infra.rs index 6e9f8d86f9..aa7c3f5e91 100644 --- a/crates/forge_app/src/infra.rs +++ b/crates/forge_app/src/infra.rs @@ -5,8 +5,8 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use bytes::Bytes; use forge_domain::{ - AuthCodeParams, CommandOutput, Environment, FileInfo, McpServerConfig, OAuthConfig, - OAuthTokenResponse, ToolDefinition, ToolName, ToolOutput, + AuthCodeParams, BackgroundCommandOutput, CommandOutput, Environment, FileInfo, McpServerConfig, + OAuthConfig, OAuthTokenResponse, ToolDefinition, ToolName, ToolOutput, }; use reqwest::Response; use reqwest::header::HeaderMap; @@ -143,6 +143,18 @@ pub trait CommandInfra: Send + Sync { working_dir: PathBuf, env_vars: Option>, ) -> anyhow::Result; + + /// Spawns a command as a detached background process. + /// + /// The process's stdout/stderr are redirected to a temporary log file. + /// Returns a `BackgroundCommandOutput` with the PID, log path, and the + /// temp-file handle that owns the log file on disk. + async fn execute_command_background( + &self, + command: String, + working_dir: PathBuf, + env_vars: Option>, + ) -> anyhow::Result; } #[async_trait::async_trait] diff --git a/crates/forge_app/src/operation.rs b/crates/forge_app/src/operation.rs index c81dc55965..0893f52b8a 100644 --- a/crates/forge_app/src/operation.rs +++ b/crates/forge_app/src/operation.rs @@ -18,7 +18,7 @@ use crate::truncation::{ use crate::utils::{compute_hash, format_display_path}; use crate::{ FsRemoveOutput, FsUndoOutput, FsWriteOutput, HttpResponse, PatchOutput, PlanCreateOutput, - ReadOutput, ResponseContext, SearchResult, ShellOutput, + ReadOutput, ResponseContext, SearchResult, ShellOutput, ShellOutputKind, }; #[derive(Debug, Default, Setters)] @@ -549,38 +549,59 @@ impl ToolOperation { forge_domain::ToolOutput::text(elm) } ToolOperation::Shell { output } => { - let mut parent_elem = Element::new("shell_output") - .attr("command", &output.output.command) - .attr("shell", &output.shell); + let mut parent_elem = Element::new("shell_output"); - if let Some(description) = &output.description { - parent_elem = parent_elem.attr("description", description); - } + match &output.kind { + ShellOutputKind::Background { command, pid, log_file } => { + parent_elem = parent_elem + .attr("command", command) + .attr("shell", &output.shell) + .attr("mode", "background"); - if let Some(exit_code) = output.output.exit_code { - parent_elem = parent_elem.attr("exit_code", exit_code); - } + if let Some(description) = &output.description { + parent_elem = parent_elem.attr("description", description); + } - let truncated_output = truncate_shell_output( - &output.output.stdout, - &output.output.stderr, - env.stdout_max_prefix_length, - env.stdout_max_suffix_length, - env.stdout_max_line_length, - ); + let bg_elem = Element::new("background") + .attr("pid", *pid) + .attr("log_file", log_file.display().to_string()); + parent_elem = parent_elem.append(bg_elem); + } + ShellOutputKind::Foreground(cmd_output) => { + parent_elem = parent_elem + .attr("command", &cmd_output.command) + .attr("shell", &output.shell); - let stdout_elem = create_stream_element( - &truncated_output.stdout, - content_files.stdout.as_deref(), - ); + if let Some(description) = &output.description { + parent_elem = parent_elem.attr("description", description); + } - let stderr_elem = create_stream_element( - &truncated_output.stderr, - content_files.stderr.as_deref(), - ); + if let Some(exit_code) = cmd_output.exit_code { + parent_elem = parent_elem.attr("exit_code", exit_code); + } - parent_elem = parent_elem.append(stdout_elem); - parent_elem = parent_elem.append(stderr_elem); + let truncated_output = truncate_shell_output( + &cmd_output.stdout, + &cmd_output.stderr, + env.stdout_max_prefix_length, + env.stdout_max_suffix_length, + env.stdout_max_line_length, + ); + + let stdout_elem = create_stream_element( + &truncated_output.stdout, + content_files.stdout.as_deref(), + ); + + let stderr_elem = create_stream_element( + &truncated_output.stderr, + content_files.stderr.as_deref(), + ); + + parent_elem = parent_elem.append(stdout_elem); + parent_elem = parent_elem.append(stderr_elem); + } + } forge_domain::ToolOutput::text(parent_elem) } @@ -990,12 +1011,12 @@ mod tests { fn test_shell_output_no_truncation() { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "echo hello".to_string(), stdout: "hello\nworld".to_string(), stderr: "".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -1023,12 +1044,12 @@ mod tests { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "long_command".to_string(), stdout, stderr: "".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -1058,12 +1079,12 @@ mod tests { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "error_command".to_string(), stdout: "".to_string(), stderr, exit_code: Some(1), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -1099,12 +1120,12 @@ mod tests { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "complex_command".to_string(), stdout, stderr, exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -1135,12 +1156,12 @@ mod tests { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "boundary_command".to_string(), stdout, stderr: "".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -1161,12 +1182,12 @@ mod tests { fn test_shell_output_single_line_each() { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "simple_command".to_string(), stdout: "single stdout line".to_string(), stderr: "single stderr line".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -1187,12 +1208,12 @@ mod tests { fn test_shell_output_empty_streams() { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "silent_command".to_string(), stdout: "".to_string(), stderr: "".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -1226,12 +1247,12 @@ mod tests { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "line_test_command".to_string(), stdout, stderr, exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -2252,12 +2273,12 @@ mod tests { fn test_shell_success() { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "ls -la".to_string(), stdout: "total 8\ndrwxr-xr-x 2 user user 4096 Jan 1 12:00 .\ndrwxr-xr-x 10 user user 4096 Jan 1 12:00 ..".to_string(), stderr: "".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }, @@ -2279,12 +2300,12 @@ mod tests { fn test_shell_with_description() { let fixture = ToolOperation::Shell { output: ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { command: "git status".to_string(), stdout: "On branch main\nnothing to commit, working tree clean".to_string(), stderr: "".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: Some("Shows working tree status".to_string()), }, diff --git a/crates/forge_app/src/orch_spec/orch_runner.rs b/crates/forge_app/src/orch_spec/orch_runner.rs index 2fb029fd99..f5461da3c3 100644 --- a/crates/forge_app/src/orch_spec/orch_runner.rs +++ b/crates/forge_app/src/orch_spec/orch_runner.rs @@ -18,7 +18,8 @@ use crate::set_conversation_id::SetConversationId; use crate::system_prompt::SystemPrompt; use crate::user_prompt::UserPromptGenerator; use crate::{ - AgentService, AttachmentService, ShellOutput, ShellService, SkillFetchService, TemplateService, + AgentService, AttachmentService, ShellOutput, ShellOutputKind, ShellService, + SkillFetchService, TemplateService, }; static TEMPLATE_DIR: Dir<'static> = include_dir!("$CARGO_MANIFEST_DIR/../../templates"); @@ -229,6 +230,7 @@ impl ShellService for Runner { _cwd: std::path::PathBuf, _keep_ansi: bool, _silent: bool, + _background: bool, _env_vars: Option>, _description: Option, ) -> anyhow::Result { @@ -237,15 +239,23 @@ impl ShellService for Runner { Ok(output) } else { Ok(ShellOutput { - output: forge_domain::CommandOutput { + kind: ShellOutputKind::Foreground(forge_domain::CommandOutput { stdout: String::new(), stderr: String::new(), command: String::new(), exit_code: Some(1), - }, + }), shell: "/bin/bash".to_string(), description: None, }) } } + + fn list_background_processes(&self) -> anyhow::Result> { + Ok(Vec::new()) + } + + fn kill_background_process(&self, _pid: u32, _delete_log: bool) -> anyhow::Result<()> { + Ok(()) + } } diff --git a/crates/forge_app/src/orch_spec/orch_system_spec.rs b/crates/forge_app/src/orch_spec/orch_system_spec.rs index c704f3f520..08b7997c27 100644 --- a/crates/forge_app/src/orch_spec/orch_system_spec.rs +++ b/crates/forge_app/src/orch_spec/orch_system_spec.rs @@ -1,7 +1,7 @@ use forge_domain::{ChatCompletionMessage, CommandOutput, Content, FinishReason, Workflow}; use insta::assert_snapshot; -use crate::ShellOutput; +use crate::{ShellOutput, ShellOutputKind}; use crate::orch_spec::orch_runner::TestContext; #[tokio::test] @@ -44,12 +44,12 @@ async fn test_system_prompt_tool_supported() { #[tokio::test] async fn test_system_prompt_with_extensions() { let shell_output = ShellOutput { - output: CommandOutput { + kind: ShellOutputKind::Foreground(CommandOutput { stdout: include_str!("../fixtures/git_ls_files_mixed.txt").to_string(), stderr: String::new(), command: "git ls-files".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }; @@ -81,12 +81,12 @@ async fn test_system_prompt_with_extensions_truncated() { let stdout = files.join("\n"); let shell_output = ShellOutput { - output: CommandOutput { + kind: ShellOutputKind::Foreground(CommandOutput { stdout, stderr: String::new(), command: "git ls-files".to_string(), exit_code: Some(0), - }, + }), shell: "/bin/bash".to_string(), description: None, }; diff --git a/crates/forge_app/src/services.rs b/crates/forge_app/src/services.rs index 0e1525af5f..3a93fbabef 100644 --- a/crates/forge_app/src/services.rs +++ b/crates/forge_app/src/services.rs @@ -19,13 +19,44 @@ use url::Url; use crate::Walker; use crate::user::{User, UserUsage}; +/// Distinguishes foreground (ran to completion) from background (spawned and +/// still running) shell execution results. +#[derive(Debug, Clone)] +pub enum ShellOutputKind { + /// Command ran to completion (or timed out). Contains the full output. + Foreground(CommandOutput), + /// Command was spawned in the background. Contains process metadata. + Background { + /// The command string that was executed. + command: String, + /// OS process ID of the background process. + pid: u32, + /// Absolute path to the log file capturing stdout/stderr. + log_file: PathBuf, + }, +} + #[derive(Debug, Clone)] pub struct ShellOutput { - pub output: CommandOutput, + /// The execution result -- either foreground output or background metadata. + pub kind: ShellOutputKind, + /// Shell used to execute the command (e.g. "bash", "zsh"). pub shell: String, + /// Optional human-readable description of the command. pub description: Option, } +impl ShellOutput { + /// Returns a reference to the foreground `CommandOutput` if this is a + /// foreground result, or `None` if this is a background result. + pub fn foreground(&self) -> Option<&CommandOutput> { + match &self.kind { + ShellOutputKind::Foreground(output) => Some(output), + ShellOutputKind::Background { .. } => None, + } + } +} + #[derive(Debug)] pub struct PatchOutput { pub errors: Vec, @@ -480,9 +511,16 @@ pub trait ShellService: Send + Sync { cwd: PathBuf, keep_ansi: bool, silent: bool, + background: bool, env_vars: Option>, description: Option, ) -> anyhow::Result; + + /// Returns all tracked background processes with their alive status. + fn list_background_processes(&self) -> anyhow::Result>; + + /// Kills a background process by PID and removes it from tracking. + fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()>; } #[async_trait::async_trait] @@ -906,13 +944,22 @@ impl ShellService for I { cwd: PathBuf, keep_ansi: bool, silent: bool, + background: bool, env_vars: Option>, description: Option, ) -> anyhow::Result { self.shell_service() - .execute(command, cwd, keep_ansi, silent, env_vars, description) + .execute(command, cwd, keep_ansi, silent, background, env_vars, description) .await } + + fn list_background_processes(&self) -> anyhow::Result> { + self.shell_service().list_background_processes() + } + + fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { + self.shell_service().kill_background_process(pid, delete_log) + } } impl EnvironmentService for I { diff --git a/crates/forge_app/src/snapshots/forge_app__compact__tests__template_engine_renders_background_shell.snap b/crates/forge_app/src/snapshots/forge_app__compact__tests__template_engine_renders_background_shell.snap deleted file mode 100644 index 3bc8e4ea1f..0000000000 --- a/crates/forge_app/src/snapshots/forge_app__compact__tests__template_engine_renders_background_shell.snap +++ /dev/null @@ -1,26 +0,0 @@ ---- -source: crates/forge_app/src/compact.rs -expression: actual ---- -Use the following summary frames as the authoritative reference for all coding suggestions and decisions. Do not re-explain or revisit it unless I ask. Additional summary frames will be added as the conversation progresses. - -## Summary - -### 1. User - -```` -Start the dev server -```` - -### 2. Assistant - -**Execute:** -``` -npm start -``` -**Background Process:** PID=12345, Log=`/tmp/forge-bg-npm-start.log` - - ---- - -Proceed with implementation based on this context. diff --git a/crates/forge_app/src/snapshots/forge_app__tool_registry__all_rendered_tool_descriptions.snap b/crates/forge_app/src/snapshots/forge_app__tool_registry__all_rendered_tool_descriptions.snap index c2def812f4..466d57f21e 100644 --- a/crates/forge_app/src/snapshots/forge_app__tool_registry__all_rendered_tool_descriptions.snap +++ b/crates/forge_app/src/snapshots/forge_app__tool_registry__all_rendered_tool_descriptions.snap @@ -151,6 +151,17 @@ Good examples: Bad example: cd /foo/bar && pytest tests +Background execution: + - Set `background: true` to run long-lived processes (web servers, file watchers, dev servers) as detached background jobs. + - The command returns immediately with a **log file path** and **process ID (PID)** instead of waiting for completion. + - The process continues running independently even after the session ends. + - CRITICAL: Always remember the log file path returned by background commands. You will need it to check output, diagnose errors, or verify the process is working. After compaction the log file path will still be available in the summary. + - Use `read` on the log file path to inspect process output at any time. + - Examples of when to use background: + - Starting a web server: `npm start`, `python manage.py runserver`, `cargo run --bin server` + - Starting a file watcher: `npm run watch`, `cargo watch` + - Starting any process that runs indefinitely and should not block your workflow + Returns complete output including stdout, stderr, and exit code for diagnostic purposes. --- diff --git a/crates/forge_app/src/system_prompt.rs b/crates/forge_app/src/system_prompt.rs index b8131ed2d8..38ef7226ae 100644 --- a/crates/forge_app/src/system_prompt.rs +++ b/crates/forge_app/src/system_prompt.rs @@ -45,6 +45,7 @@ impl SystemPrompt { self.environment.cwd.clone(), false, true, + false, None, None, ) @@ -52,11 +53,12 @@ impl SystemPrompt { .ok()?; // If git command fails (e.g., not in a git repo), return None - if output.output.exit_code != Some(0) { + let fg = output.foreground()?; + if fg.exit_code != Some(0) { return None; } - parse_extensions(&output.output.stdout, max_extensions) + parse_extensions(&fg.stdout, max_extensions) } pub async fn add_system_message( diff --git a/crates/forge_app/src/tool_executor.rs b/crates/forge_app/src/tool_executor.rs index b96bf2c708..b6e9c9f54f 100644 --- a/crates/forge_app/src/tool_executor.rs +++ b/crates/forge_app/src/tool_executor.rs @@ -6,7 +6,7 @@ use forge_domain::{CodebaseQueryResult, ToolCallContext, ToolCatalog, ToolOutput use crate::fmt::content::FormatContent; use crate::operation::{TempContentFiles, ToolOperation}; -use crate::services::{Services, ShellService}; +use crate::services::{Services, ShellOutputKind, ShellService}; use crate::{ AgentRegistry, ConversationService, EnvironmentService, FollowUpService, FsPatchService, FsReadService, FsRemoveService, FsSearchService, FsUndoService, FsWriteService, @@ -83,30 +83,34 @@ impl< Ok(files) } ToolOperation::Shell { output } => { - let env = self.services.get_environment(); - let stdout_lines = output.output.stdout.lines().count(); - let stderr_lines = output.output.stderr.lines().count(); - let stdout_truncated = - stdout_lines > env.stdout_max_prefix_length + env.stdout_max_suffix_length; - let stderr_truncated = - stderr_lines > env.stdout_max_prefix_length + env.stdout_max_suffix_length; + if let ShellOutputKind::Foreground(ref cmd_output) = output.kind { + let env = self.services.get_environment(); + let stdout_lines = cmd_output.stdout.lines().count(); + let stderr_lines = cmd_output.stderr.lines().count(); + let stdout_truncated = + stdout_lines > env.stdout_max_prefix_length + env.stdout_max_suffix_length; + let stderr_truncated = + stderr_lines > env.stdout_max_prefix_length + env.stdout_max_suffix_length; - let mut files = TempContentFiles::default(); + let mut files = TempContentFiles::default(); - if stdout_truncated { - files = files.stdout( - self.create_temp_file("forge_shell_stdout_", ".txt", &output.output.stdout) - .await?, - ); - } - if stderr_truncated { - files = files.stderr( - self.create_temp_file("forge_shell_stderr_", ".txt", &output.output.stderr) - .await?, - ); - } + if stdout_truncated { + files = files.stdout( + self.create_temp_file("forge_shell_stdout_", ".txt", &cmd_output.stdout) + .await?, + ); + } + if stderr_truncated { + files = files.stderr( + self.create_temp_file("forge_shell_stderr_", ".txt", &cmd_output.stderr) + .await?, + ); + } - Ok(files) + Ok(files) + } else { + Ok(TempContentFiles::default()) + } } _ => Ok(TempContentFiles::default()), } @@ -261,6 +265,7 @@ impl< PathBuf::from(normalized_cwd), input.keep_ansi, false, + input.background, input.env.clone(), input.description.clone(), ) diff --git a/crates/forge_app/src/transformers/trim_context_summary.rs b/crates/forge_app/src/transformers/trim_context_summary.rs index 333a14b9fb..ef64a1473d 100644 --- a/crates/forge_app/src/transformers/trim_context_summary.rs +++ b/crates/forge_app/src/transformers/trim_context_summary.rs @@ -49,7 +49,7 @@ fn to_op(tool: &SummaryTool) -> Operation<'_> { SummaryTool::FileUpdate { path } => Operation::File(path), SummaryTool::FileRemove { path } => Operation::File(path), SummaryTool::Undo { path } => Operation::File(path), - SummaryTool::Shell { command } => Operation::Shell(command), + SummaryTool::Shell { command, .. } => Operation::Shell(command), SummaryTool::Search { pattern } => Operation::Search(pattern), SummaryTool::SemSearch { queries } => Operation::CodebaseSearch { queries }, SummaryTool::Fetch { url } => Operation::Fetch(url), diff --git a/crates/forge_domain/Cargo.toml b/crates/forge_domain/Cargo.toml index 966e2af9f6..e8b265e702 100644 --- a/crates/forge_domain/Cargo.toml +++ b/crates/forge_domain/Cargo.toml @@ -25,6 +25,7 @@ tokio-stream.workspace = true uuid.workspace = true tracing.workspace = true url.workspace = true +tempfile.workspace = true merge.workspace = true serde_yml.workspace = true forge_template.workspace = true diff --git a/crates/forge_domain/src/background_process.rs b/crates/forge_domain/src/background_process.rs index d28d653df0..9251facad4 100644 --- a/crates/forge_domain/src/background_process.rs +++ b/crates/forge_domain/src/background_process.rs @@ -1,5 +1,4 @@ use std::path::PathBuf; -use std::sync::Mutex; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; @@ -11,280 +10,10 @@ pub struct BackgroundProcess { pub pid: u32, /// The original command string that was executed. pub command: String, + /// Working directory where the command was spawned. + pub cwd: PathBuf, /// Absolute path to the log file capturing stdout/stderr. pub log_file: PathBuf, /// When the process was spawned. pub started_at: DateTime, } - -/// Owns the temp-file handles for background process log files so that they -/// are automatically cleaned up when the manager is dropped. -struct OwnedLogFile { - /// Keeping the `NamedTempFile` alive prevents cleanup; when dropped the - /// file is deleted. - _handle: tempfile::NamedTempFile, - /// Associated PID so we can remove the handle when the process is killed. - pid: u32, -} - -impl std::fmt::Debug for OwnedLogFile { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("OwnedLogFile") - .field("pid", &self.pid) - .finish() - } -} - -/// Thread-safe registry of background processes spawned during the current -/// session. -/// -/// When the manager is dropped all owned temp-file handles are released, -/// causing the underlying log files to be deleted automatically. -#[derive(Debug, Default)] -pub struct BackgroundProcessManager { - processes: Mutex>, - log_handles: Mutex>, -} - -impl BackgroundProcessManager { - /// Creates a new, empty manager. - pub fn new() -> Self { - Self::default() - } - - /// Register a newly spawned background process. - /// - /// # Arguments - /// - /// * `pid` - OS process id of the spawned process. - /// * `command` - The command string that was executed. - /// * `log_file` - Absolute path to the log file. - /// * `log_handle` - The `NamedTempFile` handle that owns the log file on - /// disk. Kept alive until the process is removed or the manager is - /// dropped. - pub fn register( - &self, - pid: u32, - command: String, - log_file: PathBuf, - log_handle: tempfile::NamedTempFile, - ) -> BackgroundProcess { - let process = BackgroundProcess { - pid, - command, - log_file, - started_at: Utc::now(), - }; - self.processes - .lock() - .expect("lock poisoned") - .push(process.clone()); - self.log_handles - .lock() - .expect("lock poisoned") - .push(OwnedLogFile { _handle: log_handle, pid }); - process - } - - /// Returns a snapshot of all tracked background processes. - pub fn list(&self) -> Vec { - self.processes - .lock() - .expect("lock poisoned") - .clone() - } - - /// Find a background process by PID. - pub fn find(&self, pid: u32) -> Option { - self.processes - .lock() - .expect("lock poisoned") - .iter() - .find(|p| p.pid == pid) - .cloned() - } - - /// Remove a background process by PID. - /// - /// This also drops the associated log-file handle. If `delete_log` is - /// `false` the handle is persisted (leaked) so the file survives on disk. - pub fn remove(&self, pid: u32, delete_log: bool) { - self.processes - .lock() - .expect("lock poisoned") - .retain(|p| p.pid != pid); - - if delete_log { - // Simply removing the OwnedLogFile will drop the NamedTempFile, - // deleting the file on disk. - self.log_handles - .lock() - .expect("lock poisoned") - .retain(|h| h.pid != pid); - } else { - // Persist the file by taking ownership and calling `persist` (or - // `keep`) so the drop won't delete it. - let mut handles = self.log_handles.lock().expect("lock poisoned"); - if let Some(pos) = handles.iter().position(|h| h.pid == pid) { - let owned = handles.remove(pos); - // Persist: consumes the handle without deleting the file. - let _ = owned._handle.keep(); - } - } - } - - /// Returns the number of tracked processes. - pub fn len(&self) -> usize { - self.processes.lock().expect("lock poisoned").len() - } - - /// Returns true if no background processes are tracked. - pub fn is_empty(&self) -> bool { - self.len() == 0 - } -} - -#[cfg(test)] -mod tests { - use std::io::Write; - - use pretty_assertions::assert_eq; - - use super::*; - - fn create_temp_log() -> tempfile::NamedTempFile { - let mut f = tempfile::Builder::new() - .prefix("forge-bg-test-") - .suffix(".log") - .tempfile() - .unwrap(); - writeln!(f, "test log content").unwrap(); - f - } - - #[test] - fn test_register_and_list() { - let fixture = BackgroundProcessManager::new(); - let log = create_temp_log(); - let log_path = log.path().to_path_buf(); - - fixture.register(1234, "npm start".to_string(), log_path.clone(), log); - - let actual = fixture.list(); - - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].pid, 1234); - assert_eq!(actual[0].command, "npm start"); - assert_eq!(actual[0].log_file, log_path); - } - - #[test] - fn test_find_existing() { - let fixture = BackgroundProcessManager::new(); - let log = create_temp_log(); - let log_path = log.path().to_path_buf(); - - fixture.register(42, "python server.py".to_string(), log_path, log); - - let actual = fixture.find(42); - - assert!(actual.is_some()); - assert_eq!(actual.unwrap().pid, 42); - } - - #[test] - fn test_find_missing() { - let fixture = BackgroundProcessManager::new(); - - let actual = fixture.find(999); - - assert!(actual.is_none()); - } - - #[test] - fn test_remove_with_log_deletion() { - let fixture = BackgroundProcessManager::new(); - let log = create_temp_log(); - let log_path = log.path().to_path_buf(); - - fixture.register(100, "node app.js".to_string(), log_path.clone(), log); - assert_eq!(fixture.len(), 1); - - fixture.remove(100, true); - - assert_eq!(fixture.len(), 0); - assert!(fixture.find(100).is_none()); - // The temp file should be deleted - assert!(!log_path.exists()); - } - - #[test] - fn test_remove_without_log_deletion() { - let fixture = BackgroundProcessManager::new(); - let log = create_temp_log(); - let log_path = log.path().to_path_buf(); - - fixture.register(200, "cargo watch".to_string(), log_path.clone(), log); - - fixture.remove(200, false); - - assert_eq!(fixture.len(), 0); - // The temp file should be persisted (kept) - assert!(log_path.exists()); - - // Cleanup for test hygiene - let _ = std::fs::remove_file(&log_path); - } - - #[test] - fn test_multiple_processes() { - let fixture = BackgroundProcessManager::new(); - - let log1 = create_temp_log(); - let path1 = log1.path().to_path_buf(); - let log2 = create_temp_log(); - let path2 = log2.path().to_path_buf(); - - fixture.register(10, "server1".to_string(), path1, log1); - fixture.register(20, "server2".to_string(), path2, log2); - - assert_eq!(fixture.len(), 2); - assert!(fixture.find(10).is_some()); - assert!(fixture.find(20).is_some()); - - fixture.remove(10, true); - - assert_eq!(fixture.len(), 1); - assert!(fixture.find(10).is_none()); - assert!(fixture.find(20).is_some()); - } - - #[test] - fn test_is_empty() { - let fixture = BackgroundProcessManager::new(); - - assert!(fixture.is_empty()); - - let log = create_temp_log(); - let path = log.path().to_path_buf(); - fixture.register(1, "cmd".to_string(), path, log); - - assert!(!fixture.is_empty()); - } - - #[test] - fn test_drop_cleans_up_temp_files() { - let log = create_temp_log(); - let log_path = log.path().to_path_buf(); - - { - let manager = BackgroundProcessManager::new(); - manager.register(300, "temp cmd".to_string(), log_path.clone(), log); - assert!(log_path.exists()); - // manager dropped here - } - - // After drop, the temp file should be deleted - assert!(!log_path.exists()); - } -} diff --git a/crates/forge_domain/src/lib.rs b/crates/forge_domain/src/lib.rs index 70c543ed8b..78b2eff736 100644 --- a/crates/forge_domain/src/lib.rs +++ b/crates/forge_domain/src/lib.rs @@ -3,6 +3,7 @@ mod agent_definition; mod app_config; mod attachment; mod auth; +mod background_process; mod chat_request; mod chat_response; mod commit_config; @@ -61,6 +62,7 @@ mod xml; pub use agent::*; pub use agent_definition::*; pub use attachment::*; +pub use background_process::*; pub use chat_request::*; pub use chat_response::*; pub use commit_config::*; diff --git a/crates/forge_domain/src/shell.rs b/crates/forge_domain/src/shell.rs index 84df33a2cc..9657f074b4 100644 --- a/crates/forge_domain/src/shell.rs +++ b/crates/forge_domain/src/shell.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + /// Output from a command execution #[derive(Debug, Clone)] pub struct CommandOutput { @@ -12,3 +14,20 @@ impl CommandOutput { self.exit_code.is_none_or(|code| code >= 0) } } + +/// Output from a background (detached) command execution. +/// +/// Wraps a `CommandOutput` with the process ID and the `NamedTempFile` handle +/// that owns the log file on disk. Keeping the handle alive prevents the temp +/// file from being deleted. +#[derive(Debug)] +pub struct BackgroundCommandOutput { + /// The original command string that was executed. + pub command: String, + /// OS process ID of the spawned background process. + pub pid: u32, + /// Absolute path to the log file capturing stdout/stderr. + pub log_file: PathBuf, + /// The temp-file handle; dropping it deletes the log from disk. + pub log_handle: tempfile::NamedTempFile, +} diff --git a/crates/forge_domain/src/tools/catalog.rs b/crates/forge_domain/src/tools/catalog.rs index d17aefa00b..3ab14130ba 100644 --- a/crates/forge_domain/src/tools/catalog.rs +++ b/crates/forge_domain/src/tools/catalog.rs @@ -564,6 +564,15 @@ pub struct Shell { #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, + + /// If true, runs the command in the background as a detached process. + /// The command's stdout/stderr are redirected to a temporary log file. + /// The tool returns immediately with the log file path and process ID + /// instead of waiting for the command to complete. + /// Use this for long-running processes like web servers or file watchers. + #[serde(default)] + #[serde(skip_serializing_if = "is_default")] + pub background: bool, } /// Input type for the net fetch tool @@ -1702,6 +1711,7 @@ mod tests { keep_ansi: false, env: None, description: Some("Shows working tree status".to_string()), + background: false, }; let actual = serde_json::to_value(&fixture).unwrap(); @@ -1725,6 +1735,7 @@ mod tests { keep_ansi: false, env: None, description: None, + background: false, }; let actual = serde_json::to_value(&fixture).unwrap(); @@ -1747,6 +1758,7 @@ mod tests { keep_ansi: false, env: None, description: None, + background: false, }; let actual = serde_json::to_value(&fixture).unwrap(); diff --git a/crates/forge_domain/src/tools/definition/snapshots/forge_domain__tools__definition__usage__tests__tool_usage.snap b/crates/forge_domain/src/tools/definition/snapshots/forge_domain__tools__definition__usage__tests__tool_usage.snap index 940884ef49..8858498071 100644 --- a/crates/forge_domain/src/tools/definition/snapshots/forge_domain__tools__definition__usage__tests__tool_usage.snap +++ b/crates/forge_domain/src/tools/definition/snapshots/forge_domain__tools__definition__usage__tests__tool_usage.snap @@ -9,7 +9,7 @@ expression: prompt {"name":"remove","description":"Request to remove a file at the specified path. Use when you need to delete an existing file. The path must be absolute. This operation can be undone using the `{{tool_names.undo}}` tool.","arguments":{"path":{"description":"The path of the file to remove (absolute path required)","type":"string","is_required":true}}} {"name":"patch","description":"Performs exact string replacements in files.\nUsage:\n- You must use your `{{tool_names.read}}` tool at least once in the conversation before editing. This tool will error if you attempt an edit without reading the file. \n- When editing text from `{{tool_names.read}}` tool output, ensure you preserve the exact indentation (tabs/spaces) as it appears AFTER the line number prefix. The line number prefix format is: 'line_number:'. Everything after that line_number: is the actual file content to match. Never include any part of the line number prefix in the old_string or new_string.\n- ALWAYS prefer editing existing files in the codebase. NEVER write new files unless explicitly required.\n- Only use emojis if the user explicitly requests it. Avoid adding emojis to files unless asked.\n- The edit will FAIL if `old_string` is not unique in the file. Either provide a larger string with more surrounding context to make it unique or use `replace_all` to change every instance of `old_string`. \n- Use `replace_all` for replacing and renaming strings across the file. This parameter is useful if you want to rename a variable for instance.","arguments":{"file_path":{"description":"The absolute path to the file to modify","type":"string","is_required":true},"new_string":{"description":"The text to replace it with (must be different from old_string)","type":"string","is_required":true},"old_string":{"description":"The text to replace","type":"string","is_required":true},"replace_all":{"description":"Replace all occurrences of old_string (default false)","type":"boolean","is_required":false}}} {"name":"undo","description":"Reverts the most recent file operation (create/modify/delete) on a specific file. Use this tool when you need to recover from incorrect file changes or if a revert is requested by the user.","arguments":{"path":{"description":"The absolute path of the file to revert to its previous state.","type":"string","is_required":true}}} -{"name":"shell","description":"Executes shell commands. The `cwd` parameter sets the working directory for command execution. If not specified, defaults to `{{env.cwd}}`.\n\nCRITICAL: Do NOT use `cd` commands in the command string. This is FORBIDDEN. Always use the `cwd` parameter to set the working directory instead. Any use of `cd` in the command is redundant, incorrect, and violates the tool contract.\n\nIMPORTANT: This tool is for terminal operations like git, npm, docker, etc. DO NOT use it for file operations (reading, writing, editing, searching, finding files) - use the specialized tools for this instead.\n\nBefore executing the command, please follow these steps:\n\n1. Directory Verification:\n - If the command will create new directories or files, first use `shell` with `ls` to verify the parent directory exists and is the correct location\n - For example, before running \"mkdir foo/bar\", first use `ls foo` to check that \"foo\" exists and is the intended parent directory\n\n2. Command Execution:\n - Always quote file paths that contain spaces with double quotes (e.g., python \"path with spaces/script.py\")\n - Examples of proper quoting:\n - mkdir \"/Users/name/My Documents\" (correct)\n - mkdir /Users/name/My Documents (incorrect - will fail)\n - python \"/path/with spaces/script.py\" (correct)\n - python /path/with spaces/script.py (incorrect - will fail)\n - After ensuring proper quoting, execute the command.\n - Capture the output of the command.\n\nUsage notes:\n - The command argument is required.\n - It is very helpful if you write a clear, concise description of what this command does in 5-10 words.\n - If the output exceeds {{env.stdoutMaxPrefixLength}} prefix lines or {{env.stdoutMaxSuffixLength}} suffix lines, or if a line exceeds {{env.stdoutMaxLineLength}} characters, it will be truncated and the full output will be written to a temporary file. You can use read with start_line/end_line to read specific sections or fs_search to search the full content. Because of this, you do NOT need to use `head`, `tail`, or other truncation commands to limit output - just run the command directly.\n - Avoid using {{tool_names.shell}} with the `find`, `grep`, `cat`, `head`, `tail`, `sed`, `awk`, or `echo` commands, unless explicitly instructed or when these commands are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands:\n - File search: Use `{{tool_names.fs_search}}` (NOT find or ls)\n - Content search: Use `{{tool_names.fs_search}}` with regex (NOT grep or rg)\n - Read files: Use `{{tool_names.read}}` (NOT cat/head/tail)\n - Edit files: Use `{{tool_names.patch}}`(NOT sed/awk)\n - Write files: Use `{{tool_names.write}}` (NOT echo >/cat < && `. Use the `cwd` parameter to change directories instead.\n\nGood examples:\n - With explicit cwd: cwd=\"/foo/bar\" with command: pytest tests\n\nBad example:\n cd /foo/bar && pytest tests\n\nReturns complete output including stdout, stderr, and exit code for diagnostic purposes.","arguments":{"command":{"description":"The shell command to execute.","type":"string","is_required":true},"cwd":{"description":"The working directory where the command should be executed.\nIf not specified, defaults to the current working directory from the\nenvironment.","type":"string","is_required":false},"description":{"description":"Clear, concise description of what this command does. Recommended to be\n5-10 words for simple commands. For complex commands with pipes or\nmultiple operations, provide more context. Examples: \"Lists files in\ncurrent directory\", \"Installs package dependencies\", \"Compiles Rust\nproject with release optimizations\".","type":"string","is_required":false},"env":{"description":"Environment variable names to pass to command execution (e.g., [\"PATH\",\n\"HOME\", \"USER\"]). The system automatically reads the specified\nvalues and applies them during command execution.","type":"array","is_required":false},"keep_ansi":{"description":"Whether to preserve ANSI escape codes in the output.\nIf true, ANSI escape codes will be preserved in the output.\nIf false (default), ANSI escape codes will be stripped from the output.","type":"boolean","is_required":false}}} +{"name":"shell","description":"Executes shell commands. The `cwd` parameter sets the working directory for command execution. If not specified, defaults to `{{env.cwd}}`.\n\nCRITICAL: Do NOT use `cd` commands in the command string. This is FORBIDDEN. Always use the `cwd` parameter to set the working directory instead. Any use of `cd` in the command is redundant, incorrect, and violates the tool contract.\n\nIMPORTANT: This tool is for terminal operations like git, npm, docker, etc. DO NOT use it for file operations (reading, writing, editing, searching, finding files) - use the specialized tools for this instead.\n\nBefore executing the command, please follow these steps:\n\n1. Directory Verification:\n - If the command will create new directories or files, first use `shell` with `ls` to verify the parent directory exists and is the correct location\n - For example, before running \"mkdir foo/bar\", first use `ls foo` to check that \"foo\" exists and is the intended parent directory\n\n2. Command Execution:\n - Always quote file paths that contain spaces with double quotes (e.g., python \"path with spaces/script.py\")\n - Examples of proper quoting:\n - mkdir \"/Users/name/My Documents\" (correct)\n - mkdir /Users/name/My Documents (incorrect - will fail)\n - python \"/path/with spaces/script.py\" (correct)\n - python /path/with spaces/script.py (incorrect - will fail)\n - After ensuring proper quoting, execute the command.\n - Capture the output of the command.\n\nUsage notes:\n - The command argument is required.\n - It is very helpful if you write a clear, concise description of what this command does in 5-10 words.\n - If the output exceeds {{env.stdoutMaxPrefixLength}} prefix lines or {{env.stdoutMaxSuffixLength}} suffix lines, or if a line exceeds {{env.stdoutMaxLineLength}} characters, it will be truncated and the full output will be written to a temporary file. You can use read with start_line/end_line to read specific sections or fs_search to search the full content. Because of this, you do NOT need to use `head`, `tail`, or other truncation commands to limit output - just run the command directly.\n - Avoid using {{tool_names.shell}} with the `find`, `grep`, `cat`, `head`, `tail`, `sed`, `awk`, or `echo` commands, unless explicitly instructed or when these commands are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands:\n - File search: Use `{{tool_names.fs_search}}` (NOT find or ls)\n - Content search: Use `{{tool_names.fs_search}}` with regex (NOT grep or rg)\n - Read files: Use `{{tool_names.read}}` (NOT cat/head/tail)\n - Edit files: Use `{{tool_names.patch}}`(NOT sed/awk)\n - Write files: Use `{{tool_names.write}}` (NOT echo >/cat < && `. Use the `cwd` parameter to change directories instead.\n\nGood examples:\n - With explicit cwd: cwd=\"/foo/bar\" with command: pytest tests\n\nBad example:\n cd /foo/bar && pytest tests\n\nBackground execution:\n - Set `background: true` to run long-lived processes (web servers, file watchers, dev servers) as detached background jobs.\n - The command returns immediately with a **log file path** and **process ID (PID)** instead of waiting for completion.\n - The process continues running independently even after the session ends.\n - CRITICAL: Always remember the log file path returned by background commands. You will need it to check output, diagnose errors, or verify the process is working. After compaction the log file path will still be available in the summary.\n - Use `read` on the log file path to inspect process output at any time.\n - Examples of when to use background:\n - Starting a web server: `npm start`, `python manage.py runserver`, `cargo run --bin server`\n - Starting a file watcher: `npm run watch`, `cargo watch`\n - Starting any process that runs indefinitely and should not block your workflow\n\nReturns complete output including stdout, stderr, and exit code for diagnostic purposes.","arguments":{"background":{"description":"If true, runs the command in the background as a detached process.\nThe command's stdout/stderr are redirected to a temporary log file.\nThe tool returns immediately with the log file path and process ID\ninstead of waiting for the command to complete.\nUse this for long-running processes like web servers or file watchers.","type":"boolean","is_required":false},"command":{"description":"The shell command to execute.","type":"string","is_required":true},"cwd":{"description":"The working directory where the command should be executed.\nIf not specified, defaults to the current working directory from the\nenvironment.","type":"string","is_required":false},"description":{"description":"Clear, concise description of what this command does. Recommended to be\n5-10 words for simple commands. For complex commands with pipes or\nmultiple operations, provide more context. Examples: \"Lists files in\ncurrent directory\", \"Installs package dependencies\", \"Compiles Rust\nproject with release optimizations\".","type":"string","is_required":false},"env":{"description":"Environment variable names to pass to command execution (e.g., [\"PATH\",\n\"HOME\", \"USER\"]). The system automatically reads the specified\nvalues and applies them during command execution.","type":"array","is_required":false},"keep_ansi":{"description":"Whether to preserve ANSI escape codes in the output.\nIf true, ANSI escape codes will be preserved in the output.\nIf false (default), ANSI escape codes will be stripped from the output.","type":"boolean","is_required":false}}} {"name":"fetch","description":"Retrieves content from URLs as markdown or raw text. Enables access to current online information including websites, APIs and documentation. Use for obtaining up-to-date information beyond training data, verifying facts, or retrieving specific online content. Handles HTTP/HTTPS and converts HTML to readable markdown by default. Cannot access private/restricted resources requiring authentication. Respects robots.txt and may be blocked by anti-scraping measures. For large pages, returns the first 40,000 characters and stores the complete content in a temporary file for subsequent access.","arguments":{"raw":{"description":"Get raw content without any markdown conversion (default: false)","type":"boolean","is_required":false},"url":{"description":"URL to fetch","type":"string","is_required":true}}} {"name":"followup","description":"Use this tool when you encounter ambiguities, need clarification, or require more details to proceed effectively. Use this tool judiciously to maintain a balance between gathering necessary information and avoiding excessive back-and-forth.","arguments":{"multiple":{"description":"If true, allows selecting multiple options; if false (default), only one\noption can be selected","type":"boolean","is_required":false},"option1":{"description":"First option to choose from","type":"string","is_required":false},"option2":{"description":"Second option to choose from","type":"string","is_required":false},"option3":{"description":"Third option to choose from","type":"string","is_required":false},"option4":{"description":"Fourth option to choose from","type":"string","is_required":false},"option5":{"description":"Fifth option to choose from","type":"string","is_required":false},"question":{"description":"Question to ask the user","type":"string","is_required":true}}} {"name":"plan","description":"Creates a new plan file with the specified name, version, and content. Use this tool to create structured project plans, task breakdowns, or implementation strategies that can be tracked and referenced throughout development sessions.","arguments":{"content":{"description":"The content to write to the plan file. This should be the complete\nplan content in markdown format.","type":"string","is_required":true},"plan_name":{"description":"The name of the plan (will be used in the filename)","type":"string","is_required":true},"version":{"description":"The version of the plan (e.g., \"v1\", \"v2\", \"1.0\")","type":"string","is_required":true}}} diff --git a/crates/forge_domain/src/tools/descriptions/shell.md b/crates/forge_domain/src/tools/descriptions/shell.md index 24d62c33af..e508437cf8 100644 --- a/crates/forge_domain/src/tools/descriptions/shell.md +++ b/crates/forge_domain/src/tools/descriptions/shell.md @@ -44,4 +44,15 @@ Good examples: Bad example: cd /foo/bar && pytest tests +Background execution: + - Set `background: true` to run long-lived processes (web servers, file watchers, dev servers) as detached background jobs. + - The command returns immediately with a **log file path** and **process ID (PID)** instead of waiting for completion. + - The process continues running independently even after the session ends. + - CRITICAL: Always remember the log file path returned by background commands. You will need it to check output, diagnose errors, or verify the process is working. After compaction the log file path will still be available in the summary. + - Use `read` on the log file path to inspect process output at any time. + - Examples of when to use background: + - Starting a web server: `npm start`, `python manage.py runserver`, `cargo run --bin server` + - Starting a file watcher: `npm run watch`, `cargo watch` + - Starting any process that runs indefinitely and should not block your workflow + Returns complete output including stdout, stderr, and exit code for diagnostic purposes. \ No newline at end of file diff --git a/crates/forge_domain/src/tools/snapshots/forge_domain__tools__catalog__tests__tool_definition_json.snap b/crates/forge_domain/src/tools/snapshots/forge_domain__tools__catalog__tests__tool_definition_json.snap index ce2da28a9c..dc034a8ff7 100644 --- a/crates/forge_domain/src/tools/snapshots/forge_domain__tools__catalog__tests__tool_definition_json.snap +++ b/crates/forge_domain/src/tools/snapshots/forge_domain__tools__catalog__tests__tool_definition_json.snap @@ -232,6 +232,10 @@ expression: tools "title": "Shell", "type": "object", "properties": { + "background": { + "description": "If true, runs the command in the background as a detached process.\nThe command's stdout/stderr are redirected to a temporary log file.\nThe tool returns immediately with the log file path and process ID\ninstead of waiting for the command to complete.\nUse this for long-running processes like web servers or file watchers.", + "type": "boolean" + }, "command": { "description": "The shell command to execute.", "type": "string" diff --git a/crates/forge_infra/src/executor.rs b/crates/forge_infra/src/executor.rs index f4952c9046..8fa89f9685 100644 --- a/crates/forge_infra/src/executor.rs +++ b/crates/forge_infra/src/executor.rs @@ -3,7 +3,9 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use forge_app::CommandInfra; -use forge_domain::{CommandOutput, ConsoleWriter as OutputPrinterTrait, Environment}; +use forge_domain::{ + BackgroundCommandOutput, CommandOutput, ConsoleWriter as OutputPrinterTrait, Environment, +}; use tokio::io::AsyncReadExt; use tokio::process::Command; use tokio::sync::Mutex; @@ -224,6 +226,114 @@ impl CommandInfra for ForgeCommandExecutorService { Ok(prepared_command.spawn()?.wait().await?) } + + async fn execute_command_background( + &self, + command: String, + working_dir: PathBuf, + env_vars: Option>, + ) -> anyhow::Result { + // Create a temp log file that will capture stdout/stderr + let log_file = tempfile::Builder::new() + .prefix("forge-bg-") + .suffix(".log") + .tempfile() + .map_err(|e| anyhow::anyhow!("Failed to create background log file: {e}"))?; + let log_path = log_file.path().to_path_buf(); + let log_path_str = log_path.display().to_string(); + + tracing::info!( + command = %command, + log_path = %log_path_str, + "Spawning background process" + ); + + // NOTE: We intentionally do NOT acquire self.ready here. + // Background spawns should not block foreground commands. + + let pid = spawn_background_process( + &command, + &working_dir, + &log_path_str, + &self.env.shell, + self.restricted, + env_vars, + ) + .await?; + + tracing::info!(pid = pid, log_path = %log_path_str, "Background process spawned"); + + Ok(BackgroundCommandOutput { command, pid, log_file: log_path, log_handle: log_file }) + } +} + +/// Spawn a background process, returning its PID. +async fn spawn_background_process( + command: &str, + working_dir: &Path, + log_path: &str, + shell: &str, + restricted: bool, + env_vars: Option>, +) -> anyhow::Result { + let is_windows = cfg!(target_os = "windows"); + + let mut cmd = if is_windows { + let bg_command = format!("{command} > \"{log_path}\" 2>&1"); + let mut cmd = Command::new("cmd"); + cmd.args(["/C", "start", "/b", "cmd", "/C", &bg_command]); + cmd + } else { + let shell_bin = if restricted { "rbash" } else { shell }; + let bg_command = format!("nohup {command} > {log_path} 2>&1 & echo $!"); + let mut cmd = Command::new(shell_bin); + cmd.arg("-c").arg(&bg_command); + cmd + }; + + cmd.current_dir(working_dir) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .kill_on_drop(false); + + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + cmd.creation_flags(0x00000010); // DETACHED_PROCESS + } + + if let Some(vars) = env_vars { + for var in vars { + if let Ok(value) = std::env::var(&var) { + cmd.env(&var, value); + } + } + } + + if is_windows { + let child = cmd.spawn()?; + child + .id() + .ok_or_else(|| anyhow::anyhow!("Failed to get PID of background process on Windows")) + } else { + let output = cmd.output().await?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + anyhow::bail!("Failed to spawn background process: {stderr}"); + } + let stdout = String::from_utf8_lossy(&output.stdout); + stdout + .trim() + .lines() + .last() + .unwrap_or("") + .trim() + .parse() + .map_err(|e| { + anyhow::anyhow!("Failed to parse PID from shell output '{stdout}': {e}") + }) + } } #[cfg(test)] diff --git a/crates/forge_infra/src/forge_infra.rs b/crates/forge_infra/src/forge_infra.rs index 1a79373409..0167e9256e 100644 --- a/crates/forge_infra/src/forge_infra.rs +++ b/crates/forge_infra/src/forge_infra.rs @@ -10,8 +10,8 @@ use forge_app::{ StrategyFactory, UserInfra, WalkerInfra, }; use forge_domain::{ - AuthMethod, CommandOutput, Environment, FileInfo as FileInfoData, McpServerConfig, ProviderId, - URLParam, + AuthMethod, BackgroundCommandOutput, CommandOutput, Environment, + FileInfo as FileInfoData, McpServerConfig, ProviderId, URLParam, }; use reqwest::header::HeaderMap; use reqwest::{Response, Url}; @@ -209,6 +209,17 @@ impl CommandInfra for ForgeInfra { .execute_command_raw(command, working_dir, env_vars) .await } + + async fn execute_command_background( + &self, + command: String, + working_dir: PathBuf, + env_vars: Option>, + ) -> anyhow::Result { + self.command_executor_service + .execute_command_background(command, working_dir, env_vars) + .await + } } #[async_trait::async_trait] diff --git a/crates/forge_main/src/built_in_commands.json b/crates/forge_main/src/built_in_commands.json index 8584b2cd85..48e72c935d 100644 --- a/crates/forge_main/src/built_in_commands.json +++ b/crates/forge_main/src/built_in_commands.json @@ -126,5 +126,9 @@ { "command": "setup", "description": "Setup zsh integration by updating .zshrc" + }, + { + "command": "processes", + "description": "List and manage background processes [alias: ps]" } ] diff --git a/crates/forge_main/src/cli.rs b/crates/forge_main/src/cli.rs index c3f93a1db0..62046c1042 100644 --- a/crates/forge_main/src/cli.rs +++ b/crates/forge_main/src/cli.rs @@ -176,6 +176,21 @@ pub enum TopLevelCommand { /// Run diagnostics on shell environment (alias for `zsh doctor`). Doctor, + + /// List and manage background processes spawned during shell sessions. + Processes { + /// Output in machine-readable format (tab-separated). + #[arg(long)] + porcelain: bool, + + /// Kill the process with the given PID. + #[arg(long)] + kill: Option, + + /// When used with --kill, also delete the log file. + #[arg(long)] + delete_log: bool, + }, } /// Command group for custom command management. diff --git a/crates/forge_main/src/model.rs b/crates/forge_main/src/model.rs index 4673213e9a..3e0e78c551 100644 --- a/crates/forge_main/src/model.rs +++ b/crates/forge_main/src/model.rs @@ -282,6 +282,7 @@ impl ForgeCommandManager { Ok(SlashCommand::Commit { max_diff_size }) } "/index" => Ok(SlashCommand::Index), + "/processes" | "/ps" => Ok(SlashCommand::Processes), text => { let parts = text.split_ascii_whitespace().collect::>(); @@ -437,6 +438,10 @@ pub enum SlashCommand { /// Index the current workspace for semantic code search #[strum(props(usage = "Index the current workspace for semantic search"))] Index, + + /// List and manage background processes spawned during this session + #[strum(props(usage = "List and manage background processes [alias: ps]"))] + Processes, } impl SlashCommand { @@ -469,6 +474,7 @@ impl SlashCommand { SlashCommand::Delete => "delete", SlashCommand::AgentSwitch(agent_id) => agent_id, SlashCommand::Index => "index", + SlashCommand::Processes => "processes", } } diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index ad3231cdf1..21cad06897 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -683,6 +683,10 @@ impl A + Send + Sync> UI { self.on_zsh_doctor().await?; return Ok(()); } + TopLevelCommand::Processes { porcelain, kill, delete_log } => { + self.on_processes_cli(porcelain, kill, delete_log).await?; + return Ok(()); + } } Ok(()) } @@ -1995,6 +1999,9 @@ impl A + Send + Sync> UI { )); } } + SlashCommand::Processes => { + self.on_processes().await?; + } } Ok(false) @@ -2017,6 +2024,121 @@ impl A + Send + Sync> UI { Ok(()) } + /// Shows all tracked background processes and lets the user select one to + /// kill. After killing, asks whether to also delete the log file. + async fn on_processes(&mut self) -> anyhow::Result<()> { + let processes = self.api.list_background_processes()?; + if processes.is_empty() { + self.writeln_title(TitleFormat::debug("No background processes running"))?; + return Ok(()); + } + + // Build display strings for the picker. + // Format: "command | dir | elapsed | status" + let display_items: Vec = processes + .iter() + .map(|(p, alive)| { + let status = if *alive { "running" } else { "stopped" }; + let elapsed = humanize_time(p.started_at); + let dir = p.cwd.file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_else(|| p.cwd.display().to_string()); + format!( + "{} | {} | {} | {}", + p.command, + dir, + elapsed, + status, + ) + }) + .collect(); + + let selected = + ForgeWidget::select("Select a background process to kill", display_items.clone()) + .prompt()?; + + let Some(selected_str) = selected else { + return Ok(()); + }; + + // Find the PID by matching the selected display string back to the + // processes list (same order as display_items). + let idx = display_items + .iter() + .position(|s| s == &selected_str) + .ok_or_else(|| anyhow::anyhow!("Failed to match selection"))?; + let pid = processes[idx].0.pid; + let log_file = processes[idx].0.log_file.clone(); + + // Kill the process and remove from manager, keeping log file for now + self.api.kill_background_process(pid, false)?; + self.writeln_title(TitleFormat::action(format!("Killed process {pid}")))?; + + // Ask about log file deletion + let delete_log = ForgeWidget::confirm("Delete the log file?") + .with_default(false) + .prompt()?; + + if delete_log == Some(true) { + let _ = std::fs::remove_file(&log_file); + self.writeln_title(TitleFormat::debug(format!( + "Deleted log file: {}", + log_file.display() + )))?; + } + + Ok(()) + } + + /// CLI handler for `forge processes`. Supports porcelain output and + /// --kill/--delete-log flags. + async fn on_processes_cli( + &mut self, + porcelain: bool, + kill_pid: Option, + delete_log: bool, + ) -> anyhow::Result<()> { + if let Some(pid) = kill_pid { + self.api.kill_background_process(pid, delete_log)?; + if !porcelain { + self.writeln_title(TitleFormat::action(format!("Killed process {pid}")))?; + } + return Ok(()); + } + + let processes = self.api.list_background_processes()?; + if porcelain { + // Tab-separated output for machine consumption + for (p, alive) in &processes { + let status = if *alive { "running" } else { "stopped" }; + println!( + "{}\t{}\t{}\t{}\t{}", + p.pid, + status, + p.command, + p.started_at.to_rfc3339(), + p.log_file.display() + ); + } + } else if processes.is_empty() { + self.writeln_title(TitleFormat::debug("No background processes running"))?; + } else { + for (p, alive) in &processes { + let status = if *alive { "running" } else { "stopped" }; + let elapsed = humanize_time(p.started_at); + self.writeln_title(TitleFormat::debug(format!( + "PID {} | {} | {} | {} | log: {}", + p.pid, + status, + p.command, + elapsed, + p.log_file.display() + )))?; + } + } + Ok(()) + } + /// Select a model from all configured providers using porcelain-style /// tabular display matching the shell plugin's `:model` UI. /// diff --git a/crates/forge_repo/src/forge_repo.rs b/crates/forge_repo/src/forge_repo.rs index dde20149e0..1d1c3c300a 100644 --- a/crates/forge_repo/src/forge_repo.rs +++ b/crates/forge_repo/src/forge_repo.rs @@ -475,6 +475,17 @@ where .execute_command_raw(command, working_dir, env_vars) .await } + + async fn execute_command_background( + &self, + command: String, + working_dir: PathBuf, + env_vars: Option>, + ) -> anyhow::Result { + self.infra + .execute_command_background(command, working_dir, env_vars) + .await + } } #[async_trait::async_trait] diff --git a/crates/forge_repo/src/provider/openai_responses/snapshots/forge_repo__provider__openai_responses__request__tests__openai_responses_all_catalog_tools.snap b/crates/forge_repo/src/provider/openai_responses/snapshots/forge_repo__provider__openai_responses__request__tests__openai_responses_all_catalog_tools.snap index 55a46263c1..1d4bd36ca8 100644 --- a/crates/forge_repo/src/provider/openai_responses/snapshots/forge_repo__provider__openai_responses__request__tests__openai_responses_all_catalog_tools.snap +++ b/crates/forge_repo/src/provider/openai_responses/snapshots/forge_repo__provider__openai_responses__request__tests__openai_responses_all_catalog_tools.snap @@ -385,6 +385,10 @@ expression: actual.tools "parameters": { "additionalProperties": false, "properties": { + "background": { + "description": "If true, runs the command in the background as a detached process.\nThe command's stdout/stderr are redirected to a temporary log file.\nThe tool returns immediately with the log file path and process ID\ninstead of waiting for the command to complete.\nUse this for long-running processes like web servers or file watchers.", + "type": "boolean" + }, "command": { "description": "The shell command to execute.", "type": "string" @@ -431,6 +435,7 @@ expression: actual.tools } }, "required": [ + "background", "command", "cwd", "description", @@ -441,7 +446,7 @@ expression: actual.tools "type": "object" }, "strict": true, - "description": "Executes shell commands. The `cwd` parameter sets the working directory for command execution. If not specified, defaults to `{{env.cwd}}`.\n\nCRITICAL: Do NOT use `cd` commands in the command string. This is FORBIDDEN. Always use the `cwd` parameter to set the working directory instead. Any use of `cd` in the command is redundant, incorrect, and violates the tool contract.\n\nIMPORTANT: This tool is for terminal operations like git, npm, docker, etc. DO NOT use it for file operations (reading, writing, editing, searching, finding files) - use the specialized tools for this instead.\n\nBefore executing the command, please follow these steps:\n\n1. Directory Verification:\n - If the command will create new directories or files, first use `shell` with `ls` to verify the parent directory exists and is the correct location\n - For example, before running \"mkdir foo/bar\", first use `ls foo` to check that \"foo\" exists and is the intended parent directory\n\n2. Command Execution:\n - Always quote file paths that contain spaces with double quotes (e.g., python \"path with spaces/script.py\")\n - Examples of proper quoting:\n - mkdir \"/Users/name/My Documents\" (correct)\n - mkdir /Users/name/My Documents (incorrect - will fail)\n - python \"/path/with spaces/script.py\" (correct)\n - python /path/with spaces/script.py (incorrect - will fail)\n - After ensuring proper quoting, execute the command.\n - Capture the output of the command.\n\nUsage notes:\n - The command argument is required.\n - It is very helpful if you write a clear, concise description of what this command does in 5-10 words.\n - If the output exceeds {{env.stdoutMaxPrefixLength}} prefix lines or {{env.stdoutMaxSuffixLength}} suffix lines, or if a line exceeds {{env.stdoutMaxLineLength}} characters, it will be truncated and the full output will be written to a temporary file. You can use read with start_line/end_line to read specific sections or fs_search to search the full content. Because of this, you do NOT need to use `head`, `tail`, or other truncation commands to limit output - just run the command directly.\n - Avoid using {{tool_names.shell}} with the `find`, `grep`, `cat`, `head`, `tail`, `sed`, `awk`, or `echo` commands, unless explicitly instructed or when these commands are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands:\n - File search: Use `{{tool_names.fs_search}}` (NOT find or ls)\n - Content search: Use `{{tool_names.fs_search}}` with regex (NOT grep or rg)\n - Read files: Use `{{tool_names.read}}` (NOT cat/head/tail)\n - Edit files: Use `{{tool_names.patch}}`(NOT sed/awk)\n - Write files: Use `{{tool_names.write}}` (NOT echo >/cat < && `. Use the `cwd` parameter to change directories instead.\n\nGood examples:\n - With explicit cwd: cwd=\"/foo/bar\" with command: pytest tests\n\nBad example:\n cd /foo/bar && pytest tests\n\nReturns complete output including stdout, stderr, and exit code for diagnostic purposes." + "description": "Executes shell commands. The `cwd` parameter sets the working directory for command execution. If not specified, defaults to `{{env.cwd}}`.\n\nCRITICAL: Do NOT use `cd` commands in the command string. This is FORBIDDEN. Always use the `cwd` parameter to set the working directory instead. Any use of `cd` in the command is redundant, incorrect, and violates the tool contract.\n\nIMPORTANT: This tool is for terminal operations like git, npm, docker, etc. DO NOT use it for file operations (reading, writing, editing, searching, finding files) - use the specialized tools for this instead.\n\nBefore executing the command, please follow these steps:\n\n1. Directory Verification:\n - If the command will create new directories or files, first use `shell` with `ls` to verify the parent directory exists and is the correct location\n - For example, before running \"mkdir foo/bar\", first use `ls foo` to check that \"foo\" exists and is the intended parent directory\n\n2. Command Execution:\n - Always quote file paths that contain spaces with double quotes (e.g., python \"path with spaces/script.py\")\n - Examples of proper quoting:\n - mkdir \"/Users/name/My Documents\" (correct)\n - mkdir /Users/name/My Documents (incorrect - will fail)\n - python \"/path/with spaces/script.py\" (correct)\n - python /path/with spaces/script.py (incorrect - will fail)\n - After ensuring proper quoting, execute the command.\n - Capture the output of the command.\n\nUsage notes:\n - The command argument is required.\n - It is very helpful if you write a clear, concise description of what this command does in 5-10 words.\n - If the output exceeds {{env.stdoutMaxPrefixLength}} prefix lines or {{env.stdoutMaxSuffixLength}} suffix lines, or if a line exceeds {{env.stdoutMaxLineLength}} characters, it will be truncated and the full output will be written to a temporary file. You can use read with start_line/end_line to read specific sections or fs_search to search the full content. Because of this, you do NOT need to use `head`, `tail`, or other truncation commands to limit output - just run the command directly.\n - Avoid using {{tool_names.shell}} with the `find`, `grep`, `cat`, `head`, `tail`, `sed`, `awk`, or `echo` commands, unless explicitly instructed or when these commands are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands:\n - File search: Use `{{tool_names.fs_search}}` (NOT find or ls)\n - Content search: Use `{{tool_names.fs_search}}` with regex (NOT grep or rg)\n - Read files: Use `{{tool_names.read}}` (NOT cat/head/tail)\n - Edit files: Use `{{tool_names.patch}}`(NOT sed/awk)\n - Write files: Use `{{tool_names.write}}` (NOT echo >/cat < && `. Use the `cwd` parameter to change directories instead.\n\nGood examples:\n - With explicit cwd: cwd=\"/foo/bar\" with command: pytest tests\n\nBad example:\n cd /foo/bar && pytest tests\n\nBackground execution:\n - Set `background: true` to run long-lived processes (web servers, file watchers, dev servers) as detached background jobs.\n - The command returns immediately with a **log file path** and **process ID (PID)** instead of waiting for completion.\n - The process continues running independently even after the session ends.\n - CRITICAL: Always remember the log file path returned by background commands. You will need it to check output, diagnose errors, or verify the process is working. After compaction the log file path will still be available in the summary.\n - Use `read` on the log file path to inspect process output at any time.\n - Examples of when to use background:\n - Starting a web server: `npm start`, `python manage.py runserver`, `cargo run --bin server`\n - Starting a file watcher: `npm run watch`, `cargo watch`\n - Starting any process that runs indefinitely and should not block your workflow\n\nReturns complete output including stdout, stderr, and exit code for diagnostic purposes." }, { "type": "function", diff --git a/crates/forge_services/Cargo.toml b/crates/forge_services/Cargo.toml index 6071cf19cb..eba21c25bb 100644 --- a/crates/forge_services/Cargo.toml +++ b/crates/forge_services/Cargo.toml @@ -52,6 +52,8 @@ http.workspace = true infer.workspace = true uuid.workspace = true tonic.workspace = true +tempfile.workspace = true +sysinfo.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["macros", "rt", "time", "test-util"] } diff --git a/crates/forge_services/src/tool_services/background_process.rs b/crates/forge_services/src/tool_services/background_process.rs index efab4d42d1..d63465f865 100644 --- a/crates/forge_services/src/tool_services/background_process.rs +++ b/crates/forge_services/src/tool_services/background_process.rs @@ -1,6 +1,7 @@ use std::path::PathBuf; use std::sync::Mutex; +use anyhow::{Context, Result}; use chrono::Utc; use forge_domain::BackgroundProcess; @@ -27,26 +28,10 @@ impl std::fmt::Debug for OwnedLogFile { /// /// When the manager is dropped all owned temp-file handles are released, /// causing the underlying log files to be deleted automatically. -/// -/// Optionally persists process metadata to a JSON file so that other processes -/// (e.g. the ZSH plugin) can list and kill background processes that were -/// spawned in earlier invocations. -#[derive(Debug)] +#[derive(Default, Debug)] pub struct BackgroundProcessManager { processes: Mutex>, log_handles: Mutex>, - /// Optional path for persisting process metadata to disk. - persist_path: Option, -} - -impl Default for BackgroundProcessManager { - fn default() -> Self { - Self { - processes: Mutex::new(Vec::new()), - log_handles: Mutex::new(Vec::new()), - persist_path: None, - } - } } impl BackgroundProcessManager { @@ -55,34 +40,22 @@ impl BackgroundProcessManager { Self::default() } - /// Creates a manager that persists process metadata to the given path. - /// - /// If the file already exists, previously tracked processes are loaded - /// (without their log-file handles - those belong to the original session). - pub fn with_persistence(path: PathBuf) -> Self { - let processes = Self::load_from_disk(&path).unwrap_or_default(); - Self { - processes: Mutex::new(processes), - log_handles: Mutex::new(Vec::new()), - persist_path: Some(path), - } - } - - /// Saves the current process list to the persistence file, if configured. - fn persist(&self) { - if let Some(ref path) = self.persist_path { - let procs = self.processes.lock().expect("lock poisoned"); - if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); - } - let _ = std::fs::write(path, serde_json::to_string_pretty(&*procs).unwrap_or_default()); - } + /// Acquires the processes lock, returning an error if poisoned. + fn lock_processes( + &self, + ) -> Result>> { + self.processes + .lock() + .map_err(|e| anyhow::anyhow!("processes lock poisoned: {e}")) } - /// Loads process list from a JSON file on disk. - fn load_from_disk(path: &PathBuf) -> Option> { - let data = std::fs::read_to_string(path).ok()?; - serde_json::from_str(&data).ok() + /// Acquires the log handles lock, returning an error if poisoned. + fn lock_log_handles( + &self, + ) -> Result>> { + self.log_handles + .lock() + .map_err(|e| anyhow::anyhow!("log handles lock poisoned: {e}")) } /// Register a newly spawned background process. @@ -91,172 +64,108 @@ impl BackgroundProcessManager { /// /// * `pid` - OS process id of the spawned process. /// * `command` - The command string that was executed. + /// * `cwd` - Working directory where the command was spawned. /// * `log_file` - Absolute path to the log file. /// * `log_handle` - The `NamedTempFile` handle that owns the log file on /// disk. Kept alive until the process is removed or the manager is /// dropped. + /// + /// # Errors + /// + /// Returns an error if the internal lock is poisoned. pub fn register( &self, pid: u32, command: String, + cwd: PathBuf, log_file: PathBuf, log_handle: tempfile::NamedTempFile, - ) -> BackgroundProcess { - let process = BackgroundProcess { - pid, - command, - log_file, - started_at: Utc::now(), - }; - self.processes - .lock() - .expect("lock poisoned") - .push(process.clone()); - self.log_handles - .lock() - .expect("lock poisoned") + ) -> Result { + let process = BackgroundProcess { pid, command, cwd, log_file, started_at: Utc::now() }; + self.lock_processes()?.push(process.clone()); + self.lock_log_handles()? .push(OwnedLogFile { _handle: log_handle, pid }); - self.persist(); - process - } - - /// Returns a snapshot of all tracked background processes. - pub fn list(&self) -> Vec { - self.processes - .lock() - .expect("lock poisoned") - .clone() - } - - /// Find a background process by PID. - pub fn find(&self, pid: u32) -> Option { - self.processes - .lock() - .expect("lock poisoned") - .iter() - .find(|p| p.pid == pid) - .cloned() + Ok(process) } /// Remove a background process by PID. /// /// This also drops the associated log-file handle. If `delete_log` is /// `false` the handle is persisted (leaked) so the file survives on disk. - pub fn remove(&self, pid: u32, delete_log: bool) { - self.processes - .lock() - .expect("lock poisoned") - .retain(|p| p.pid != pid); + /// + /// # Errors + /// + /// Returns an error if the internal lock is poisoned. + fn remove(&self, pid: u32, delete_log: bool) -> Result<()> { + self.lock_processes()?.retain(|p| p.pid != pid); if delete_log { - self.log_handles - .lock() - .expect("lock poisoned") - .retain(|h| h.pid != pid); + self.lock_log_handles()?.retain(|h| h.pid != pid); } else { - let mut handles = self.log_handles.lock().expect("lock poisoned"); + let mut handles = self.lock_log_handles()?; if let Some(pos) = handles.iter().position(|h| h.pid == pid) { let owned = handles.remove(pos); let _ = owned._handle.keep(); } } - self.persist(); - } - - /// Returns the number of tracked processes. - pub fn len(&self) -> usize { - self.processes.lock().expect("lock poisoned").len() - } - - /// Returns true if no background processes are tracked. - pub fn is_empty(&self) -> bool { - self.len() == 0 + Ok(()) } /// Kills a background process by PID and removes it from tracking. /// /// Returns `Ok(())` if the process was killed or was already dead. /// The `delete_log` flag controls whether the log file is deleted. - pub fn kill(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { - kill_process(pid)?; - self.remove(pid, delete_log); + /// + /// # Errors + /// + /// Returns an error if the process could not be killed or the lock is + /// poisoned. + pub fn kill(&self, pid: u32, delete_log: bool) -> Result<()> { + kill_process(pid).context("failed to kill background process")?; + self.remove(pid, delete_log)?; Ok(()) } /// Returns a snapshot of all tracked processes with their alive status. - pub fn list_with_status(&self) -> Vec<(BackgroundProcess, bool)> { - self.processes - .lock() - .expect("lock poisoned") + /// + /// # Errors + /// + /// Returns an error if the internal lock is poisoned. + pub fn list_with_status(&self) -> Result> { + Ok(self + .lock_processes()? .iter() .map(|p| { let alive = is_process_alive(p.pid); (p.clone(), alive) }) - .collect() + .collect()) } } /// Cross-platform check whether a process is still running. -#[cfg(unix)] fn is_process_alive(pid: u32) -> bool { - unsafe { libc::kill(pid as libc::pid_t, 0) == 0 } -} - -#[cfg(windows)] -fn is_process_alive(pid: u32) -> bool { - use std::process::Command; - Command::new("tasklist") - .args(["/FI", &format!("PID eq {pid}"), "/NH"]) - .output() - .map(|o| { - let stdout = String::from_utf8_lossy(&o.stdout); - stdout.contains(&pid.to_string()) - }) - .unwrap_or(false) -} - -#[cfg(not(any(unix, windows)))] -fn is_process_alive(_pid: u32) -> bool { - false + let s = sysinfo::System::new_with_specifics( + sysinfo::RefreshKind::nothing() + .with_processes(sysinfo::ProcessRefreshKind::nothing()), + ); + s.process(sysinfo::Pid::from_u32(pid)).is_some() } /// Cross-platform process termination. -#[cfg(unix)] fn kill_process(pid: u32) -> anyhow::Result<()> { - let ret = unsafe { libc::kill(pid as libc::pid_t, libc::SIGTERM) }; - if ret != 0 { - let err = std::io::Error::last_os_error(); - if err.raw_os_error() == Some(libc::ESRCH) { - return Ok(()); + let s = sysinfo::System::new_with_specifics( + sysinfo::RefreshKind::nothing() + .with_processes(sysinfo::ProcessRefreshKind::nothing()), + ); + match s.process(sysinfo::Pid::from_u32(pid)) { + Some(process) => { + process.kill(); + Ok(()) } - return Err(anyhow::anyhow!("Failed to kill process {pid}: {err}")); + // Process already gone -- nothing to kill. + None => Ok(()), } - Ok(()) -} - -#[cfg(windows)] -fn kill_process(pid: u32) -> anyhow::Result<()> { - use std::process::Command; - let output = Command::new("taskkill") - .args(["/PID", &pid.to_string(), "/F"]) - .output()?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - if !stderr.contains("not found") && !stderr.contains("not running") { - return Err(anyhow::anyhow!( - "Failed to kill process {pid}: {stderr}" - )); - } - } - Ok(()) -} - -#[cfg(not(any(unix, windows)))] -fn kill_process(_pid: u32) -> anyhow::Result<()> { - Err(anyhow::anyhow!( - "Killing background processes is not supported on this platform" - )) } #[cfg(test)] @@ -278,42 +187,19 @@ mod tests { } #[test] - fn test_register_and_list() { + fn test_register_and_list_with_status() { let fixture = BackgroundProcessManager::new(); let log = create_temp_log(); let log_path = log.path().to_path_buf(); - fixture.register(1234, "npm start".to_string(), log_path.clone(), log); + fixture.register(1234, "npm start".to_string(), PathBuf::from("/test"), log_path.clone(), log).unwrap(); - let actual = fixture.list(); + let actual = fixture.list_with_status().unwrap(); assert_eq!(actual.len(), 1); - assert_eq!(actual[0].pid, 1234); - assert_eq!(actual[0].command, "npm start"); - assert_eq!(actual[0].log_file, log_path); - } - - #[test] - fn test_find_existing() { - let fixture = BackgroundProcessManager::new(); - let log = create_temp_log(); - let log_path = log.path().to_path_buf(); - - fixture.register(42, "python server.py".to_string(), log_path, log); - - let actual = fixture.find(42); - - assert!(actual.is_some()); - assert_eq!(actual.unwrap().pid, 42); - } - - #[test] - fn test_find_missing() { - let fixture = BackgroundProcessManager::new(); - - let actual = fixture.find(999); - - assert!(actual.is_none()); + assert_eq!(actual[0].0.pid, 1234); + assert_eq!(actual[0].0.command, "npm start"); + assert_eq!(actual[0].0.log_file, log_path); } #[test] @@ -322,13 +208,12 @@ mod tests { let log = create_temp_log(); let log_path = log.path().to_path_buf(); - fixture.register(100, "node app.js".to_string(), log_path.clone(), log); - assert_eq!(fixture.len(), 1); + fixture.register(100, "node app.js".to_string(), PathBuf::from("/test"), log_path.clone(), log).unwrap(); + assert_eq!(fixture.list_with_status().unwrap().len(), 1); - fixture.remove(100, true); + fixture.remove(100, true).unwrap(); - assert_eq!(fixture.len(), 0); - assert!(fixture.find(100).is_none()); + assert_eq!(fixture.list_with_status().unwrap().len(), 0); assert!(!log_path.exists()); } @@ -338,11 +223,11 @@ mod tests { let log = create_temp_log(); let log_path = log.path().to_path_buf(); - fixture.register(200, "cargo watch".to_string(), log_path.clone(), log); + fixture.register(200, "cargo watch".to_string(), PathBuf::from("/test"), log_path.clone(), log).unwrap(); - fixture.remove(200, false); + fixture.remove(200, false).unwrap(); - assert_eq!(fixture.len(), 0); + assert_eq!(fixture.list_with_status().unwrap().len(), 0); assert!(log_path.exists()); let _ = std::fs::remove_file(&log_path); @@ -357,31 +242,16 @@ mod tests { let log2 = create_temp_log(); let path2 = log2.path().to_path_buf(); - fixture.register(10, "server1".to_string(), path1, log1); - fixture.register(20, "server2".to_string(), path2, log2); + fixture.register(10, "server1".to_string(), PathBuf::from("/proj1"), path1, log1).unwrap(); + fixture.register(20, "server2".to_string(), PathBuf::from("/proj2"), path2, log2).unwrap(); - assert_eq!(fixture.len(), 2); - assert!(fixture.find(10).is_some()); - assert!(fixture.find(20).is_some()); + assert_eq!(fixture.list_with_status().unwrap().len(), 2); - fixture.remove(10, true); + fixture.remove(10, true).unwrap(); - assert_eq!(fixture.len(), 1); - assert!(fixture.find(10).is_none()); - assert!(fixture.find(20).is_some()); - } - - #[test] - fn test_is_empty() { - let fixture = BackgroundProcessManager::new(); - - assert!(fixture.is_empty()); - - let log = create_temp_log(); - let path = log.path().to_path_buf(); - fixture.register(1, "cmd".to_string(), path, log); - - assert!(!fixture.is_empty()); + let actual = fixture.list_with_status().unwrap(); + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].0.pid, 20); } #[test] @@ -391,7 +261,7 @@ mod tests { { let manager = BackgroundProcessManager::new(); - manager.register(300, "temp cmd".to_string(), log_path.clone(), log); + manager.register(300, "temp cmd".to_string(), PathBuf::from("/test"), log_path.clone(), log).unwrap(); assert!(log_path.exists()); } @@ -399,60 +269,14 @@ mod tests { } #[test] - fn test_persistence_write_and_reload() { - let dir = tempfile::tempdir().unwrap(); - let persist_path = dir.path().join("processes.json"); - - { - let manager = BackgroundProcessManager::with_persistence(persist_path.clone()); - let log = create_temp_log(); - let log_path = log.path().to_path_buf(); - manager.register(500, "persistent cmd".to_string(), log_path, log); - } - - assert!(persist_path.exists()); - - let reloaded = BackgroundProcessManager::with_persistence(persist_path); - let actual = reloaded.list(); - - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].pid, 500); - assert_eq!(actual[0].command, "persistent cmd"); - } - - #[test] - fn test_persistence_removes_entry() { - let dir = tempfile::tempdir().unwrap(); - let persist_path = dir.path().join("processes.json"); - - let manager = BackgroundProcessManager::with_persistence(persist_path.clone()); - let log1 = create_temp_log(); - let log2 = create_temp_log(); - let path1 = log1.path().to_path_buf(); - let path2 = log2.path().to_path_buf(); - - manager.register(600, "cmd1".to_string(), path1, log1); - manager.register(700, "cmd2".to_string(), path2, log2); - assert_eq!(manager.len(), 2); - - manager.remove(600, true); - - let reloaded = BackgroundProcessManager::with_persistence(persist_path); - let actual = reloaded.list(); - - assert_eq!(actual.len(), 1); - assert_eq!(actual[0].pid, 700); - } - - #[test] - fn test_list_with_status() { + fn test_list_with_status_shows_dead_process() { let fixture = BackgroundProcessManager::new(); let log = create_temp_log(); let path = log.path().to_path_buf(); - fixture.register(99999, "ghost".to_string(), path, log); + fixture.register(99999, "ghost".to_string(), PathBuf::from("/test"), path, log).unwrap(); - let actual = fixture.list_with_status(); + let actual = fixture.list_with_status().unwrap(); assert_eq!(actual.len(), 1); assert_eq!(actual[0].0.pid, 99999); diff --git a/crates/forge_services/src/tool_services/mod.rs b/crates/forge_services/src/tool_services/mod.rs index 64a5c6f3c0..3f67bf25fe 100644 --- a/crates/forge_services/src/tool_services/mod.rs +++ b/crates/forge_services/src/tool_services/mod.rs @@ -1,3 +1,4 @@ +mod background_process; mod fetch; mod followup; mod fs_patch; @@ -11,6 +12,7 @@ mod plan_create; mod shell; mod skill; +pub use background_process::*; pub use fetch::*; pub use followup::*; pub use fs_patch::*; diff --git a/crates/forge_services/src/tool_services/shell.rs b/crates/forge_services/src/tool_services/shell.rs index 3d9d34334b..35111b233e 100644 --- a/crates/forge_services/src/tool_services/shell.rs +++ b/crates/forge_services/src/tool_services/shell.rs @@ -3,7 +3,9 @@ use std::sync::Arc; use anyhow::bail; use forge_app::domain::Environment; -use forge_app::{CommandInfra, EnvironmentInfra, ShellOutput, ShellService}; +use forge_app::{CommandInfra, EnvironmentInfra, ShellOutput, ShellOutputKind, ShellService}; + +use super::BackgroundProcessManager; use strip_ansi_escapes::strip; // Strips out the ansi codes from content. @@ -20,13 +22,16 @@ fn strip_ansi(content: String) -> String { pub struct ForgeShell { env: Environment, infra: Arc, + bg_manager: Arc, } impl ForgeShell { - /// Create a new Shell with environment configuration + /// Create a new Shell with environment configuration and a background + /// process manager for tracking long-running detached processes. pub fn new(infra: Arc) -> Self { let env = infra.get_environment(); - Self { env, infra } + let bg_manager = Arc::new(BackgroundProcessManager::new()); + Self { env, infra, bg_manager } } fn validate_command(command: &str) -> anyhow::Result<()> { @@ -45,11 +50,39 @@ impl ShellService for ForgeShell { cwd: PathBuf, keep_ansi: bool, silent: bool, + background: bool, env_vars: Option>, description: Option, ) -> anyhow::Result { Self::validate_command(&command)?; + if background { + let bg_output = self + .infra + .execute_command_background(command, cwd.clone(), env_vars) + .await?; + + // Register with the background process manager which takes + // ownership of the temp-file handle (keeps the log file alive). + self.bg_manager.register( + bg_output.pid, + bg_output.command.clone(), + cwd, + bg_output.log_file.clone(), + bg_output.log_handle, + )?; + + return Ok(ShellOutput { + kind: ShellOutputKind::Background { + command: bg_output.command, + pid: bg_output.pid, + log_file: bg_output.log_file, + }, + shell: self.env.shell.clone(), + description, + }); + } + let mut output = self .infra .execute_command(command, cwd, silent, env_vars) @@ -60,7 +93,21 @@ impl ShellService for ForgeShell { output.stderr = strip_ansi(output.stderr); } - Ok(ShellOutput { output, shell: self.env.shell.clone(), description }) + Ok(ShellOutput { + kind: ShellOutputKind::Foreground(output), + shell: self.env.shell.clone(), + description, + }) + } + + fn list_background_processes( + &self, + ) -> anyhow::Result> { + self.bg_manager.list_with_status() + } + + fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { + self.bg_manager.kill(pid, delete_log) } } #[cfg(test)] @@ -108,6 +155,26 @@ mod tests { ) -> anyhow::Result { unimplemented!() } + + async fn execute_command_background( + &self, + command: String, + _working_dir: PathBuf, + _env_vars: Option>, + ) -> anyhow::Result { + let log_file = tempfile::Builder::new() + .prefix("forge-bg-test-") + .suffix(".log") + .tempfile() + .unwrap(); + let log_path = log_file.path().to_path_buf(); + Ok(forge_domain::BackgroundCommandOutput { + command, + pid: 9999, + log_file: log_path, + log_handle: log_file, + }) + } } impl EnvironmentInfra for MockCommandInfra { @@ -129,11 +196,19 @@ mod tests { } } + fn make_shell(expected_env_vars: Option>) -> ForgeShell { + ForgeShell::new(Arc::new(MockCommandInfra { expected_env_vars })) + } + + /// Extracts the foreground CommandOutput from a ShellOutput, panicking if + /// the variant is Background. + fn unwrap_foreground(output: &ShellOutput) -> &forge_domain::CommandOutput { + output.foreground().expect("Expected Foreground variant") + } + #[tokio::test] async fn test_shell_service_forwards_env_vars() { - let fixture = ForgeShell::new(Arc::new(MockCommandInfra { - expected_env_vars: Some(vec!["PATH".to_string(), "HOME".to_string()]), - })); + let fixture = make_shell(Some(vec!["PATH".to_string(), "HOME".to_string()])); let actual = fixture .execute( @@ -141,19 +216,21 @@ mod tests { PathBuf::from("."), false, false, + false, Some(vec!["PATH".to_string(), "HOME".to_string()]), None, ) .await .unwrap(); - assert_eq!(actual.output.stdout, "Mock output"); - assert_eq!(actual.output.exit_code, Some(0)); + let fg = unwrap_foreground(&actual); + assert_eq!(fg.stdout, "Mock output"); + assert_eq!(fg.exit_code, Some(0)); } #[tokio::test] async fn test_shell_service_forwards_no_env_vars() { - let fixture = ForgeShell::new(Arc::new(MockCommandInfra { expected_env_vars: None })); + let fixture = make_shell(None); let actual = fixture .execute( @@ -161,21 +238,21 @@ mod tests { PathBuf::from("."), false, false, + false, None, None, ) .await .unwrap(); - assert_eq!(actual.output.stdout, "Mock output"); - assert_eq!(actual.output.exit_code, Some(0)); + let fg = unwrap_foreground(&actual); + assert_eq!(fg.stdout, "Mock output"); + assert_eq!(fg.exit_code, Some(0)); } #[tokio::test] async fn test_shell_service_forwards_empty_env_vars() { - let fixture = ForgeShell::new(Arc::new(MockCommandInfra { - expected_env_vars: Some(vec![]), - })); + let fixture = make_shell(Some(vec![])); let actual = fixture .execute( @@ -183,19 +260,21 @@ mod tests { PathBuf::from("."), false, false, + false, Some(vec![]), None, ) .await .unwrap(); - assert_eq!(actual.output.stdout, "Mock output"); - assert_eq!(actual.output.exit_code, Some(0)); + let fg = unwrap_foreground(&actual); + assert_eq!(fg.stdout, "Mock output"); + assert_eq!(fg.exit_code, Some(0)); } #[tokio::test] async fn test_shell_service_with_description() { - let fixture = ForgeShell::new(Arc::new(MockCommandInfra { expected_env_vars: None })); + let fixture = make_shell(None); let actual = fixture .execute( @@ -203,14 +282,20 @@ mod tests { PathBuf::from("."), false, false, + false, None, Some("Prints hello to stdout".to_string()), ) .await .unwrap(); - assert_eq!(actual.output.stdout, "Mock output"); - assert_eq!(actual.output.exit_code, Some(0)); + match &actual.kind { + ShellOutputKind::Foreground(output) => { + assert_eq!(output.stdout, "Mock output"); + assert_eq!(output.exit_code, Some(0)); + } + _ => panic!("Expected Foreground"), + } assert_eq!( actual.description, Some("Prints hello to stdout".to_string()) @@ -219,7 +304,7 @@ mod tests { #[tokio::test] async fn test_shell_service_without_description() { - let fixture = ForgeShell::new(Arc::new(MockCommandInfra { expected_env_vars: None })); + let fixture = make_shell(None); let actual = fixture .execute( @@ -227,14 +312,49 @@ mod tests { PathBuf::from("."), false, false, + false, None, None, ) .await .unwrap(); - assert_eq!(actual.output.stdout, "Mock output"); - assert_eq!(actual.output.exit_code, Some(0)); + match &actual.kind { + ShellOutputKind::Foreground(output) => { + assert_eq!(output.stdout, "Mock output"); + assert_eq!(output.exit_code, Some(0)); + } + _ => panic!("Expected Foreground"), + } assert_eq!(actual.description, None); } + + #[tokio::test] + async fn test_shell_service_background_execution() { + let fixture = make_shell(None); + + let actual = fixture + .execute( + "npm start".to_string(), + PathBuf::from("."), + false, + false, + true, + None, + Some("Start dev server".to_string()), + ) + .await + .unwrap(); + + match &actual.kind { + ShellOutputKind::Background { pid, .. } => { + assert_eq!(*pid, 9999); + } + _ => panic!("Expected Background"), + } + + let tracked = fixture.list_background_processes().unwrap(); + assert_eq!(tracked.len(), 1); + assert_eq!(tracked[0].0.pid, 9999); + } } diff --git a/shell-plugin/lib/dispatcher.zsh b/shell-plugin/lib/dispatcher.zsh index 1f4b539b45..533751ec26 100644 --- a/shell-plugin/lib/dispatcher.zsh +++ b/shell-plugin/lib/dispatcher.zsh @@ -241,6 +241,9 @@ function forge-accept-line() { keyboard-shortcuts|kb) _forge_action_keyboard ;; + processes|ps) + _forge_action_processes + ;; *) _forge_action_default "$user_action" "$input_text" ;; From a4c9865106e0790d12355dbdb29829c4d19fcc39 Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod <62684960+ssddOnTop@users.noreply.github.com> Date: Tue, 24 Mar 2026 02:16:50 -0400 Subject: [PATCH 03/11] refactor(executor): remove restricted shell mode from background process spawning --- Cargo.lock | 2 +- crates/forge_infra/src/executor.rs | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62ad75c6d2..a3288c149b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2200,7 +2200,7 @@ dependencies = [ "strip-ansi-escapes", "strum 0.28.0", "strum_macros 0.28.0", - "sysinfo 0.38.2", + "sysinfo 0.38.4", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/crates/forge_infra/src/executor.rs b/crates/forge_infra/src/executor.rs index 8fa89f9685..ae5dcae891 100644 --- a/crates/forge_infra/src/executor.rs +++ b/crates/forge_infra/src/executor.rs @@ -256,7 +256,6 @@ impl CommandInfra for ForgeCommandExecutorService { &working_dir, &log_path_str, &self.env.shell, - self.restricted, env_vars, ) .await?; @@ -273,7 +272,6 @@ async fn spawn_background_process( working_dir: &Path, log_path: &str, shell: &str, - restricted: bool, env_vars: Option>, ) -> anyhow::Result { let is_windows = cfg!(target_os = "windows"); @@ -284,9 +282,8 @@ async fn spawn_background_process( cmd.args(["/C", "start", "/b", "cmd", "/C", &bg_command]); cmd } else { - let shell_bin = if restricted { "rbash" } else { shell }; let bg_command = format!("nohup {command} > {log_path} 2>&1 & echo $!"); - let mut cmd = Command::new(shell_bin); + let mut cmd = Command::new(shell); cmd.arg("-c").arg(&bg_command); cmd }; From 00eca24675234d143cc4143182676cbcb1e7a90e Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 24 Mar 2026 06:19:07 +0000 Subject: [PATCH 04/11] [autofix.ci] apply automated fixes --- crates/forge_api/src/forge_api.rs | 4 +- crates/forge_app/src/git_app.rs | 40 ++++++--- crates/forge_app/src/orch_spec/orch_runner.rs | 8 +- .../src/orch_spec/orch_system_spec.rs | 2 +- crates/forge_app/src/services.rs | 21 ++++- crates/forge_app/src/tool_executor.rs | 16 +++- crates/forge_infra/src/executor.rs | 4 +- crates/forge_infra/src/forge_infra.rs | 4 +- crates/forge_main/src/ui.rs | 12 +-- .../src/tool_services/background_process.rs | 84 +++++++++++++++---- .../forge_services/src/tool_services/shell.rs | 2 +- 11 files changed, 143 insertions(+), 54 deletions(-) diff --git a/crates/forge_api/src/forge_api.rs b/crates/forge_api/src/forge_api.rs index 49e8baa130..f3589a325f 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -420,7 +420,9 @@ impl< } fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()> { - self.services.shell_service().kill_background_process(pid, delete_log) + self.services + .shell_service() + .kill_background_process(pid, delete_log) } async fn get_default_provider(&self) -> Result> { diff --git a/crates/forge_app/src/git_app.rs b/crates/forge_app/src/git_app.rs index dbc9ead852..ce50beefc3 100644 --- a/crates/forge_app/src/git_app.rs +++ b/crates/forge_app/src/git_app.rs @@ -166,7 +166,9 @@ where .await .context("Failed to commit changes")?; - let output = commit_result.foreground().expect("git commit runs in foreground"); + let output = commit_result + .foreground() + .expect("git commit runs in foreground"); if !output.success() { anyhow::bail!("Git commit failed: {}", output.stderr); @@ -178,10 +180,7 @@ where } else if output.stderr.is_empty() { output.stdout.clone() } else { - format!( - "{}\n{}", - output.stdout, output.stderr - ) + format!("{}\n{}", output.stdout, output.stderr) }; Ok(CommitResult { message, committed: true, has_staged_files, git_output }) @@ -251,8 +250,16 @@ where let branch_name = branch_name.context("Failed to get branch name")?; Ok(( - recent_commits.foreground().expect("git log runs in foreground").stdout.clone(), - branch_name.foreground().expect("git rev-parse runs in foreground").stdout.clone(), + recent_commits + .foreground() + .expect("git log runs in foreground") + .stdout + .clone(), + branch_name + .foreground() + .expect("git rev-parse runs in foreground") + .stdout + .clone(), )) } @@ -283,16 +290,29 @@ where let unstaged_diff = unstaged_diff.context("Failed to get unstaged changes")?; // Use staged changes if available, otherwise fall back to unstaged changes - let has_staged_files = !staged_diff.foreground().expect("git diff runs in foreground").stdout.trim().is_empty(); + let has_staged_files = !staged_diff + .foreground() + .expect("git diff runs in foreground") + .stdout + .trim() + .is_empty(); let diff_output = if has_staged_files { staged_diff - } else if !unstaged_diff.foreground().expect("git diff runs in foreground").stdout.trim().is_empty() { + } else if !unstaged_diff + .foreground() + .expect("git diff runs in foreground") + .stdout + .trim() + .is_empty() + { unstaged_diff } else { return Err(GitAppError::NoChangesToCommit.into()); }; - let fg = diff_output.foreground().expect("git diff runs in foreground"); + let fg = diff_output + .foreground() + .expect("git diff runs in foreground"); let size = fg.stdout.len(); Ok((fg.stdout.clone(), size, has_staged_files)) } diff --git a/crates/forge_app/src/orch_spec/orch_runner.rs b/crates/forge_app/src/orch_spec/orch_runner.rs index f5461da3c3..7cb3a187e1 100644 --- a/crates/forge_app/src/orch_spec/orch_runner.rs +++ b/crates/forge_app/src/orch_spec/orch_runner.rs @@ -18,8 +18,8 @@ use crate::set_conversation_id::SetConversationId; use crate::system_prompt::SystemPrompt; use crate::user_prompt::UserPromptGenerator; use crate::{ - AgentService, AttachmentService, ShellOutput, ShellOutputKind, ShellService, - SkillFetchService, TemplateService, + AgentService, AttachmentService, ShellOutput, ShellOutputKind, ShellService, SkillFetchService, + TemplateService, }; static TEMPLATE_DIR: Dir<'static> = include_dir!("$CARGO_MANIFEST_DIR/../../templates"); @@ -251,7 +251,9 @@ impl ShellService for Runner { } } - fn list_background_processes(&self) -> anyhow::Result> { + fn list_background_processes( + &self, + ) -> anyhow::Result> { Ok(Vec::new()) } diff --git a/crates/forge_app/src/orch_spec/orch_system_spec.rs b/crates/forge_app/src/orch_spec/orch_system_spec.rs index 08b7997c27..710835ac26 100644 --- a/crates/forge_app/src/orch_spec/orch_system_spec.rs +++ b/crates/forge_app/src/orch_spec/orch_system_spec.rs @@ -1,8 +1,8 @@ use forge_domain::{ChatCompletionMessage, CommandOutput, Content, FinishReason, Workflow}; use insta::assert_snapshot; -use crate::{ShellOutput, ShellOutputKind}; use crate::orch_spec::orch_runner::TestContext; +use crate::{ShellOutput, ShellOutputKind}; #[tokio::test] async fn test_system_prompt() { diff --git a/crates/forge_app/src/services.rs b/crates/forge_app/src/services.rs index 3a93fbabef..364c550764 100644 --- a/crates/forge_app/src/services.rs +++ b/crates/forge_app/src/services.rs @@ -517,7 +517,9 @@ pub trait ShellService: Send + Sync { ) -> anyhow::Result; /// Returns all tracked background processes with their alive status. - fn list_background_processes(&self) -> anyhow::Result>; + fn list_background_processes( + &self, + ) -> anyhow::Result>; /// Kills a background process by PID and removes it from tracking. fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()>; @@ -949,16 +951,27 @@ impl ShellService for I { description: Option, ) -> anyhow::Result { self.shell_service() - .execute(command, cwd, keep_ansi, silent, background, env_vars, description) + .execute( + command, + cwd, + keep_ansi, + silent, + background, + env_vars, + description, + ) .await } - fn list_background_processes(&self) -> anyhow::Result> { + fn list_background_processes( + &self, + ) -> anyhow::Result> { self.shell_service().list_background_processes() } fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { - self.shell_service().kill_background_process(pid, delete_log) + self.shell_service() + .kill_background_process(pid, delete_log) } } diff --git a/crates/forge_app/src/tool_executor.rs b/crates/forge_app/src/tool_executor.rs index b6e9c9f54f..98e4e9609a 100644 --- a/crates/forge_app/src/tool_executor.rs +++ b/crates/forge_app/src/tool_executor.rs @@ -96,14 +96,22 @@ impl< if stdout_truncated { files = files.stdout( - self.create_temp_file("forge_shell_stdout_", ".txt", &cmd_output.stdout) - .await?, + self.create_temp_file( + "forge_shell_stdout_", + ".txt", + &cmd_output.stdout, + ) + .await?, ); } if stderr_truncated { files = files.stderr( - self.create_temp_file("forge_shell_stderr_", ".txt", &cmd_output.stderr) - .await?, + self.create_temp_file( + "forge_shell_stderr_", + ".txt", + &cmd_output.stderr, + ) + .await?, ); } diff --git a/crates/forge_infra/src/executor.rs b/crates/forge_infra/src/executor.rs index ae5dcae891..3f4301228e 100644 --- a/crates/forge_infra/src/executor.rs +++ b/crates/forge_infra/src/executor.rs @@ -327,9 +327,7 @@ async fn spawn_background_process( .unwrap_or("") .trim() .parse() - .map_err(|e| { - anyhow::anyhow!("Failed to parse PID from shell output '{stdout}': {e}") - }) + .map_err(|e| anyhow::anyhow!("Failed to parse PID from shell output '{stdout}': {e}")) } } diff --git a/crates/forge_infra/src/forge_infra.rs b/crates/forge_infra/src/forge_infra.rs index 0167e9256e..fe7d253676 100644 --- a/crates/forge_infra/src/forge_infra.rs +++ b/crates/forge_infra/src/forge_infra.rs @@ -10,8 +10,8 @@ use forge_app::{ StrategyFactory, UserInfra, WalkerInfra, }; use forge_domain::{ - AuthMethod, BackgroundCommandOutput, CommandOutput, Environment, - FileInfo as FileInfoData, McpServerConfig, ProviderId, URLParam, + AuthMethod, BackgroundCommandOutput, CommandOutput, Environment, FileInfo as FileInfoData, + McpServerConfig, ProviderId, URLParam, }; use reqwest::header::HeaderMap; use reqwest::{Response, Url}; diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 21cad06897..2e02e4cff7 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -2040,16 +2040,12 @@ impl A + Send + Sync> UI { .map(|(p, alive)| { let status = if *alive { "running" } else { "stopped" }; let elapsed = humanize_time(p.started_at); - let dir = p.cwd.file_name() + let dir = p + .cwd + .file_name() .map(|n| n.to_string_lossy().to_string()) .unwrap_or_else(|| p.cwd.display().to_string()); - format!( - "{} | {} | {} | {}", - p.command, - dir, - elapsed, - status, - ) + format!("{} | {} | {} | {}", p.command, dir, elapsed, status,) }) .collect(); diff --git a/crates/forge_services/src/tool_services/background_process.rs b/crates/forge_services/src/tool_services/background_process.rs index d63465f865..f34ff2329a 100644 --- a/crates/forge_services/src/tool_services/background_process.rs +++ b/crates/forge_services/src/tool_services/background_process.rs @@ -41,18 +41,14 @@ impl BackgroundProcessManager { } /// Acquires the processes lock, returning an error if poisoned. - fn lock_processes( - &self, - ) -> Result>> { + fn lock_processes(&self) -> Result>> { self.processes .lock() .map_err(|e| anyhow::anyhow!("processes lock poisoned: {e}")) } /// Acquires the log handles lock, returning an error if poisoned. - fn lock_log_handles( - &self, - ) -> Result>> { + fn lock_log_handles(&self) -> Result>> { self.log_handles .lock() .map_err(|e| anyhow::anyhow!("log handles lock poisoned: {e}")) @@ -146,8 +142,7 @@ impl BackgroundProcessManager { /// Cross-platform check whether a process is still running. fn is_process_alive(pid: u32) -> bool { let s = sysinfo::System::new_with_specifics( - sysinfo::RefreshKind::nothing() - .with_processes(sysinfo::ProcessRefreshKind::nothing()), + sysinfo::RefreshKind::nothing().with_processes(sysinfo::ProcessRefreshKind::nothing()), ); s.process(sysinfo::Pid::from_u32(pid)).is_some() } @@ -155,8 +150,7 @@ fn is_process_alive(pid: u32) -> bool { /// Cross-platform process termination. fn kill_process(pid: u32) -> anyhow::Result<()> { let s = sysinfo::System::new_with_specifics( - sysinfo::RefreshKind::nothing() - .with_processes(sysinfo::ProcessRefreshKind::nothing()), + sysinfo::RefreshKind::nothing().with_processes(sysinfo::ProcessRefreshKind::nothing()), ); match s.process(sysinfo::Pid::from_u32(pid)) { Some(process) => { @@ -192,7 +186,15 @@ mod tests { let log = create_temp_log(); let log_path = log.path().to_path_buf(); - fixture.register(1234, "npm start".to_string(), PathBuf::from("/test"), log_path.clone(), log).unwrap(); + fixture + .register( + 1234, + "npm start".to_string(), + PathBuf::from("/test"), + log_path.clone(), + log, + ) + .unwrap(); let actual = fixture.list_with_status().unwrap(); @@ -208,7 +210,15 @@ mod tests { let log = create_temp_log(); let log_path = log.path().to_path_buf(); - fixture.register(100, "node app.js".to_string(), PathBuf::from("/test"), log_path.clone(), log).unwrap(); + fixture + .register( + 100, + "node app.js".to_string(), + PathBuf::from("/test"), + log_path.clone(), + log, + ) + .unwrap(); assert_eq!(fixture.list_with_status().unwrap().len(), 1); fixture.remove(100, true).unwrap(); @@ -223,7 +233,15 @@ mod tests { let log = create_temp_log(); let log_path = log.path().to_path_buf(); - fixture.register(200, "cargo watch".to_string(), PathBuf::from("/test"), log_path.clone(), log).unwrap(); + fixture + .register( + 200, + "cargo watch".to_string(), + PathBuf::from("/test"), + log_path.clone(), + log, + ) + .unwrap(); fixture.remove(200, false).unwrap(); @@ -242,8 +260,24 @@ mod tests { let log2 = create_temp_log(); let path2 = log2.path().to_path_buf(); - fixture.register(10, "server1".to_string(), PathBuf::from("/proj1"), path1, log1).unwrap(); - fixture.register(20, "server2".to_string(), PathBuf::from("/proj2"), path2, log2).unwrap(); + fixture + .register( + 10, + "server1".to_string(), + PathBuf::from("/proj1"), + path1, + log1, + ) + .unwrap(); + fixture + .register( + 20, + "server2".to_string(), + PathBuf::from("/proj2"), + path2, + log2, + ) + .unwrap(); assert_eq!(fixture.list_with_status().unwrap().len(), 2); @@ -261,7 +295,15 @@ mod tests { { let manager = BackgroundProcessManager::new(); - manager.register(300, "temp cmd".to_string(), PathBuf::from("/test"), log_path.clone(), log).unwrap(); + manager + .register( + 300, + "temp cmd".to_string(), + PathBuf::from("/test"), + log_path.clone(), + log, + ) + .unwrap(); assert!(log_path.exists()); } @@ -274,7 +316,15 @@ mod tests { let log = create_temp_log(); let path = log.path().to_path_buf(); - fixture.register(99999, "ghost".to_string(), PathBuf::from("/test"), path, log).unwrap(); + fixture + .register( + 99999, + "ghost".to_string(), + PathBuf::from("/test"), + path, + log, + ) + .unwrap(); let actual = fixture.list_with_status().unwrap(); diff --git a/crates/forge_services/src/tool_services/shell.rs b/crates/forge_services/src/tool_services/shell.rs index 35111b233e..9cfb2928c7 100644 --- a/crates/forge_services/src/tool_services/shell.rs +++ b/crates/forge_services/src/tool_services/shell.rs @@ -4,9 +4,9 @@ use std::sync::Arc; use anyhow::bail; use forge_app::domain::Environment; use forge_app::{CommandInfra, EnvironmentInfra, ShellOutput, ShellOutputKind, ShellService}; +use strip_ansi_escapes::strip; use super::BackgroundProcessManager; -use strip_ansi_escapes::strip; // Strips out the ansi codes from content. fn strip_ansi(content: String) -> String { From 556e2d44fcc0153e0007f7be4adcc09a4a9ea35f Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod <62684960+ssddOnTop@users.noreply.github.com> Date: Tue, 24 Mar 2026 14:54:27 -0400 Subject: [PATCH 05/11] feat(shell): add nickname resolver and process metadata persistence service --- crates/forge_domain/src/nickname.rs | 216 ++++++++++++++++ .../src/tool_services/process_metadata.rs | 233 ++++++++++++++++++ 2 files changed, 449 insertions(+) create mode 100644 crates/forge_domain/src/nickname.rs create mode 100644 crates/forge_services/src/tool_services/process_metadata.rs diff --git a/crates/forge_domain/src/nickname.rs b/crates/forge_domain/src/nickname.rs new file mode 100644 index 0000000000..250a02a6d8 --- /dev/null +++ b/crates/forge_domain/src/nickname.rs @@ -0,0 +1,216 @@ +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +/// Resolves short, disambiguated nicknames for a list of directory paths. +/// +/// The algorithm starts with the last path component as the nickname for each +/// path. When two or more paths share the same nickname, each conflicting one +/// is extended by prepending the next parent component. This repeats until all +/// nicknames are unique or the full path is consumed. +/// +/// # Arguments +/// * `paths` - Slice of directory paths to resolve nicknames for. +/// +/// # Returns +/// A map from original path to its disambiguated nickname string. +pub fn resolve_nicknames(paths: &[PathBuf]) -> HashMap { + if paths.is_empty() { + return HashMap::new(); + } + + // Collect the components of each path in reverse order (leaf first). + let components: Vec> = paths + .iter() + .map(|p| { + p.components() + .rev() + .map(|c| c.as_os_str().to_string_lossy().into_owned()) + .collect() + }) + .collect(); + + // Start with 1 component (just the leaf) for every path. + let mut depths: Vec = vec![1; paths.len()]; + + loop { + // Build current nicknames at each path's depth. + let nicknames: Vec = components + .iter() + .zip(depths.iter()) + .map(|(comps, &depth)| build_nickname(comps, depth)) + .collect(); + + // Group indices by nickname to find collisions. + let mut groups: HashMap<&str, Vec> = HashMap::new(); + for (i, nick) in nicknames.iter().enumerate() { + groups.entry(nick.as_str()).or_default().push(i); + } + + let mut any_extended = false; + for indices in groups.values() { + if indices.len() > 1 { + // Collision: try to extend each conflicting path by one more + // component, but only if it has more components available. + for &i in indices { + if depths[i] < components[i].len() { + depths[i] += 1; + any_extended = true; + } + } + } + } + + if !any_extended { + // No further disambiguation possible or all unique. + let mut result = HashMap::with_capacity(paths.len()); + for (i, path) in paths.iter().enumerate() { + result + .entry(path.clone()) + .or_insert_with(|| nicknames[i].clone()); + } + return result; + } + } +} + +/// Builds a nickname from reversed path components at the given depth. +fn build_nickname(reversed_components: &[String], depth: usize) -> String { + let take = depth.min(reversed_components.len()); + let parts: Vec<&str> = reversed_components[..take] + .iter() + .rev() + .map(|s| s.as_str()) + .collect(); + // Build using PathBuf to correctly handle the root separator. + let mut path = PathBuf::new(); + for part in parts { + path.push(part); + } + path.display().to_string() +} + +/// Looks up the nickname for a specific path from a pre-computed nickname map. +/// +/// Falls back to the full path display string if the path is not in the map. +pub fn nickname_for(path: &Path, nicknames: &HashMap) -> String { + nicknames + .get(path) + .cloned() + .unwrap_or_else(|| path.display().to_string()) +} + +#[cfg(test)] +mod tests { + use pretty_assertions::assert_eq; + + use super::*; + + #[test] + fn test_all_unique_last_components() { + let paths = vec![ + PathBuf::from("/a/b/alpha"), + PathBuf::from("/c/d/beta"), + PathBuf::from("/e/f/gamma"), + ]; + + let actual = resolve_nicknames(&paths); + + assert_eq!(actual[&paths[0]], "alpha"); + assert_eq!(actual[&paths[1]], "beta"); + assert_eq!(actual[&paths[2]], "gamma"); + } + + #[test] + fn test_two_paths_same_last_component() { + let paths = vec![ + PathBuf::from("/a/b/server"), + PathBuf::from("/c/d/server"), + ]; + + let actual = resolve_nicknames(&paths); + + assert_eq!(actual[&paths[0]], format!("b{}server", std::path::MAIN_SEPARATOR)); + assert_eq!(actual[&paths[1]], format!("d{}server", std::path::MAIN_SEPARATOR)); + } + + #[test] + fn test_three_level_disambiguation() { + let paths = vec![ + PathBuf::from("/x/a/c/server"), + PathBuf::from("/y/b/c/server"), + ]; + + let actual = resolve_nicknames(&paths); + + // Both share "server" and "c/server", so need "a/c/server" vs + // "b/c/server". + let sep = std::path::MAIN_SEPARATOR; + assert_eq!(actual[&paths[0]], format!("a{sep}c{sep}server")); + assert_eq!(actual[&paths[1]], format!("b{sep}c{sep}server")); + } + + #[test] + fn test_single_path() { + let paths = vec![PathBuf::from("/home/user/my-project")]; + + let actual = resolve_nicknames(&paths); + + assert_eq!(actual[&paths[0]], "my-project"); + } + + #[test] + fn test_identical_paths() { + let paths = vec![ + PathBuf::from("/a/b/c"), + PathBuf::from("/a/b/c"), + ]; + + let actual = resolve_nicknames(&paths); + + // Both map to the same key so only one entry in the HashMap. + // The nickname should be the full path since they cannot be + // disambiguated. + let sep = std::path::MAIN_SEPARATOR; + assert_eq!(actual[&paths[0]], format!("{sep}a{sep}b{sep}c")); + } + + #[test] + fn test_empty_input() { + let paths: Vec = vec![]; + + let actual = resolve_nicknames(&paths); + + assert!(actual.is_empty()); + } + + #[test] + fn test_mixed_unique_and_conflicting() { + let paths = vec![ + PathBuf::from("/a/b/server"), + PathBuf::from("/c/d/server"), + PathBuf::from("/e/f/client"), + ]; + + let actual = resolve_nicknames(&paths); + + let sep = std::path::MAIN_SEPARATOR; + assert_eq!(actual[&paths[0]], format!("b{sep}server")); + assert_eq!(actual[&paths[1]], format!("d{sep}server")); + assert_eq!(actual[&paths[2]], "client"); + } + + #[test] + fn test_nickname_for_helper() { + let paths = vec![ + PathBuf::from("/a/b/server"), + PathBuf::from("/c/d/client"), + ]; + let nicknames = resolve_nicknames(&paths); + + let actual = nickname_for(&PathBuf::from("/a/b/server"), &nicknames); + assert_eq!(actual, "server"); + + let missing = nickname_for(&PathBuf::from("/unknown/path"), &nicknames); + assert_eq!(missing, "/unknown/path"); + } +} diff --git a/crates/forge_services/src/tool_services/process_metadata.rs b/crates/forge_services/src/tool_services/process_metadata.rs new file mode 100644 index 0000000000..449c1ef312 --- /dev/null +++ b/crates/forge_services/src/tool_services/process_metadata.rs @@ -0,0 +1,233 @@ +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result}; +use forge_domain::BackgroundProcess; +use forge_fs::ForgeFS; + +/// Handles reading and writing background process metadata JSON files. +/// +/// Each CWD gets its own metadata file named `.json` +/// under the configured processes directory. Each file contains a JSON +/// array of `BackgroundProcess` entries. +#[derive(Debug)] +pub struct ProcessMetadataService { + processes_dir: PathBuf, +} + +impl ProcessMetadataService { + /// Creates a new service that stores metadata under the given directory. + pub fn new(processes_dir: PathBuf) -> Self { + Self { processes_dir } + } + + /// Returns the path to the metadata file for the given CWD. + fn metadata_path(&self, cwd: &Path) -> PathBuf { + let hash = BackgroundProcess::cwd_hash(cwd); + self.processes_dir.join(format!("{hash}.json")) + } + + /// Persists a background process entry to the metadata file for its CWD. + /// + /// If the file already exists, the new entry is appended to the existing + /// array. Otherwise a new file is created. The parent directory is created + /// if it does not exist. + /// + /// # Errors + /// + /// Returns an error if directory creation or file I/O fails. + pub async fn save_process(&self, process: &BackgroundProcess) -> Result<()> { + ForgeFS::create_dir_all(&self.processes_dir).await?; + + let path = self.metadata_path(&process.cwd); + let mut entries = self.read_entries(&path).await; + entries.push(process.clone()); + self.write_entries(&path, &entries).await + } + + /// Removes a background process entry by PID from the metadata file for the + /// given CWD. + /// + /// If the array becomes empty after removal the file is deleted. + /// + /// # Errors + /// + /// Returns an error if file I/O fails. + pub async fn remove_process(&self, cwd: &Path, pid: u32) -> Result<()> { + let path = self.metadata_path(cwd); + if !ForgeFS::exists(&path) { + return Ok(()); + } + + let mut entries = self.read_entries(&path).await; + entries.retain(|p| p.pid != pid); + + if entries.is_empty() { + ForgeFS::remove_file(&path).await?; + } else { + self.write_entries(&path, &entries).await?; + } + Ok(()) + } + + /// Lists all persisted background processes across all CWD metadata files. + /// + /// # Errors + /// + /// Returns an error if directory reading or file deserialization fails. + pub async fn list_all_processes(&self) -> Result> { + if !ForgeFS::exists(&self.processes_dir) { + return Ok(Vec::new()); + } + + let mut all = Vec::new(); + let mut dir = ForgeFS::read_dir(&self.processes_dir).await?; + + while let Some(entry) = dir.next_entry().await? { + let file_path = entry.path(); + if file_path.extension().is_some_and(|ext| ext == "json") { + let entries = self.read_entries(&file_path).await; + all.extend(entries); + } + } + + Ok(all) + } + + /// Reads and deserializes the entries from a metadata file. + /// + /// Returns an empty vec if the file doesn't exist or cannot be parsed. + async fn read_entries(&self, path: &Path) -> Vec { + if !ForgeFS::exists(path) { + return Vec::new(); + } + + match ForgeFS::read_to_string(path).await { + Ok(content) => serde_json::from_str(&content).unwrap_or_default(), + Err(_) => Vec::new(), + } + } + + /// Serializes and writes entries to a metadata file. + async fn write_entries(&self, path: &Path, entries: &[BackgroundProcess]) -> Result<()> { + let json = serde_json::to_string_pretty(entries) + .context("failed to serialize process metadata")?; + ForgeFS::write(path, json.as_bytes()).await?; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use chrono::Utc; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + + use super::*; + + fn make_process(pid: u32, cwd: &str, command: &str) -> BackgroundProcess { + BackgroundProcess { + pid, + command: command.to_string(), + cwd: PathBuf::from(cwd), + log_file: PathBuf::from(format!("/tmp/forge-bg-{pid}.log")), + started_at: Utc::now(), + } + } + + #[tokio::test] + async fn test_save_and_list_round_trip() { + let dir = TempDir::new().unwrap(); + let fixture = ProcessMetadataService::new(dir.path().to_path_buf()); + let process = make_process(100, "/a/b/c", "npm start"); + + fixture.save_process(&process).await.unwrap(); + let actual = fixture.list_all_processes().await.unwrap(); + + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].pid, 100); + assert_eq!(actual[0].command, "npm start"); + } + + #[tokio::test] + async fn test_save_multiple_to_same_cwd() { + let dir = TempDir::new().unwrap(); + let fixture = ProcessMetadataService::new(dir.path().to_path_buf()); + let p1 = make_process(10, "/proj", "server1"); + let p2 = make_process(20, "/proj", "server2"); + + fixture.save_process(&p1).await.unwrap(); + fixture.save_process(&p2).await.unwrap(); + let actual = fixture.list_all_processes().await.unwrap(); + + assert_eq!(actual.len(), 2); + } + + #[tokio::test] + async fn test_remove_by_pid() { + let dir = TempDir::new().unwrap(); + let fixture = ProcessMetadataService::new(dir.path().to_path_buf()); + let p1 = make_process(10, "/proj", "server1"); + let p2 = make_process(20, "/proj", "server2"); + + fixture.save_process(&p1).await.unwrap(); + fixture.save_process(&p2).await.unwrap(); + fixture.remove_process(&PathBuf::from("/proj"), 10).await.unwrap(); + + let actual = fixture.list_all_processes().await.unwrap(); + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].pid, 20); + } + + #[tokio::test] + async fn test_remove_last_process_deletes_file() { + let dir = TempDir::new().unwrap(); + let fixture = ProcessMetadataService::new(dir.path().to_path_buf()); + let process = make_process(100, "/proj", "npm start"); + let meta_path = fixture.metadata_path(&PathBuf::from("/proj")); + + fixture.save_process(&process).await.unwrap(); + assert!(ForgeFS::exists(&meta_path)); + + fixture.remove_process(&PathBuf::from("/proj"), 100).await.unwrap(); + assert!(!ForgeFS::exists(&meta_path)); + } + + #[tokio::test] + async fn test_list_with_empty_directory() { + let dir = TempDir::new().unwrap(); + let fixture = ProcessMetadataService::new(dir.path().to_path_buf()); + + let actual = fixture.list_all_processes().await.unwrap(); + + assert!(actual.is_empty()); + } + + #[tokio::test] + async fn test_list_with_nonexistent_directory() { + let fixture = ProcessMetadataService::new(PathBuf::from("/nonexistent/dir")); + + let actual = fixture.list_all_processes().await.unwrap(); + + assert!(actual.is_empty()); + } + + #[tokio::test] + async fn test_list_across_multiple_cwds() { + let dir = TempDir::new().unwrap(); + let fixture = ProcessMetadataService::new(dir.path().to_path_buf()); + let p1 = make_process(10, "/proj-a", "server"); + let p2 = make_process(20, "/proj-b", "worker"); + + fixture.save_process(&p1).await.unwrap(); + fixture.save_process(&p2).await.unwrap(); + + let actual = fixture.list_all_processes().await.unwrap(); + assert_eq!(actual.len(), 2); + + let pids: Vec = actual.iter().map(|p| p.pid).collect(); + assert!(pids.contains(&10)); + assert!(pids.contains(&20)); + } +} From dda44305ab9da86b672c7bec035f802f965115a3 Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod <62684960+ssddOnTop@users.noreply.github.com> Date: Tue, 24 Mar 2026 14:54:38 -0400 Subject: [PATCH 06/11] feat(shell): persist background process metadata to disk for cross-session discovery --- crates/forge_api/src/api.rs | 4 +- crates/forge_api/src/forge_api.rs | 7 +- crates/forge_app/src/orch_spec/orch_runner.rs | 4 +- crates/forge_app/src/services.rs | 11 +- crates/forge_domain/src/background_process.rs | 10 + crates/forge_domain/src/env.rs | 4 + crates/forge_domain/src/lib.rs | 2 + crates/forge_infra/src/executor.rs | 1 + crates/forge_main/src/ui.rs | 108 ++++--- .../src/tool_services/background_process.rs | 277 ++++++++++++++---- .../forge_services/src/tool_services/mod.rs | 2 + .../forge_services/src/tool_services/shell.rs | 28 +- 12 files changed, 346 insertions(+), 112 deletions(-) diff --git a/crates/forge_api/src/api.rs b/crates/forge_api/src/api.rs index 596b7c793c..1ce83e456e 100644 --- a/crates/forge_api/src/api.rs +++ b/crates/forge_api/src/api.rs @@ -254,8 +254,8 @@ pub trait API: Sync + Send { ) -> Result>>; /// Returns all tracked background processes with their alive status. - fn list_background_processes(&self) -> Result>; + async fn list_background_processes(&self) -> Result>; /// Kills a background process by PID and optionally deletes its log file. - fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()>; + async fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()>; } diff --git a/crates/forge_api/src/forge_api.rs b/crates/forge_api/src/forge_api.rs index f3589a325f..2cb1995f86 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -415,14 +415,15 @@ impl< app.execute(data_parameters).await } - fn list_background_processes(&self) -> Result> { - self.services.shell_service().list_background_processes() + async fn list_background_processes(&self) -> Result> { + self.services.shell_service().list_background_processes().await } - fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()> { + async fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()> { self.services .shell_service() .kill_background_process(pid, delete_log) + .await } async fn get_default_provider(&self) -> Result> { diff --git a/crates/forge_app/src/orch_spec/orch_runner.rs b/crates/forge_app/src/orch_spec/orch_runner.rs index 7cb3a187e1..117ba1d2b0 100644 --- a/crates/forge_app/src/orch_spec/orch_runner.rs +++ b/crates/forge_app/src/orch_spec/orch_runner.rs @@ -251,13 +251,13 @@ impl ShellService for Runner { } } - fn list_background_processes( + async fn list_background_processes( &self, ) -> anyhow::Result> { Ok(Vec::new()) } - fn kill_background_process(&self, _pid: u32, _delete_log: bool) -> anyhow::Result<()> { + async fn kill_background_process(&self, _pid: u32, _delete_log: bool) -> anyhow::Result<()> { Ok(()) } } diff --git a/crates/forge_app/src/services.rs b/crates/forge_app/src/services.rs index 364c550764..e2abd3db80 100644 --- a/crates/forge_app/src/services.rs +++ b/crates/forge_app/src/services.rs @@ -517,12 +517,12 @@ pub trait ShellService: Send + Sync { ) -> anyhow::Result; /// Returns all tracked background processes with their alive status. - fn list_background_processes( + async fn list_background_processes( &self, ) -> anyhow::Result>; /// Kills a background process by PID and removes it from tracking. - fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()>; + async fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()>; } #[async_trait::async_trait] @@ -963,15 +963,16 @@ impl ShellService for I { .await } - fn list_background_processes( + async fn list_background_processes( &self, ) -> anyhow::Result> { - self.shell_service().list_background_processes() + self.shell_service().list_background_processes().await } - fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { + async fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { self.shell_service() .kill_background_process(pid, delete_log) + .await } } diff --git a/crates/forge_domain/src/background_process.rs b/crates/forge_domain/src/background_process.rs index 9251facad4..cf830b15a0 100644 --- a/crates/forge_domain/src/background_process.rs +++ b/crates/forge_domain/src/background_process.rs @@ -1,3 +1,4 @@ +use std::hash::Hasher; use std::path::PathBuf; use chrono::{DateTime, Utc}; @@ -17,3 +18,12 @@ pub struct BackgroundProcess { /// When the process was spawned. pub started_at: DateTime, } + +impl BackgroundProcess { + /// Creates an FNV-64 hash of the CWD path for use as a metadata filename. + pub fn cwd_hash(cwd: &std::path::Path) -> String { + let mut hasher = fnv_rs::Fnv64::default(); + hasher.write(cwd.to_string_lossy().as_bytes()); + format!("{:x}", hasher.finish()) + } +} diff --git a/crates/forge_domain/src/env.rs b/crates/forge_domain/src/env.rs index 903fd20c73..2551d2c70b 100644 --- a/crates/forge_domain/src/env.rs +++ b/crates/forge_domain/src/env.rs @@ -140,6 +140,10 @@ impl Environment { pub fn snapshot_path(&self) -> PathBuf { self.base_path.join("snapshots") } + /// Returns the directory where background process metadata files are stored. + pub fn processes_path(&self) -> PathBuf { + self.base_path.join("processes") + } pub fn mcp_user_config(&self) -> PathBuf { self.base_path.join(".mcp.json") } diff --git a/crates/forge_domain/src/lib.rs b/crates/forge_domain/src/lib.rs index 78b2eff736..52a06775ac 100644 --- a/crates/forge_domain/src/lib.rs +++ b/crates/forge_domain/src/lib.rs @@ -30,6 +30,7 @@ mod message; mod message_pattern; mod migration; mod model; +mod nickname; mod node; mod point; mod policies; @@ -89,6 +90,7 @@ pub use message::*; pub use message_pattern::*; pub use migration::*; pub use model::*; +pub use nickname::*; pub use node::*; pub use point::*; pub use policies::*; diff --git a/crates/forge_infra/src/executor.rs b/crates/forge_infra/src/executor.rs index 3f4301228e..69e03324d4 100644 --- a/crates/forge_infra/src/executor.rs +++ b/crates/forge_infra/src/executor.rs @@ -237,6 +237,7 @@ impl CommandInfra for ForgeCommandExecutorService { let log_file = tempfile::Builder::new() .prefix("forge-bg-") .suffix(".log") + .disable_cleanup(true) .tempfile() .map_err(|e| anyhow::anyhow!("Failed to create background log file: {e}"))?; let log_path = log_file.path().to_path_buf(); diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 2e02e4cff7..554ef75b7e 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -2024,53 +2024,86 @@ impl A + Send + Sync> UI { Ok(()) } - /// Shows all tracked background processes and lets the user select one to - /// kill. After killing, asks whether to also delete the log file. + /// Lists all tracked background processes in a tabular fzf picker and + /// lets the user select one to kill. Press Escape to cancel without + /// killing anything. After killing, asks whether to also delete the log + /// file. async fn on_processes(&mut self) -> anyhow::Result<()> { - let processes = self.api.list_background_processes()?; + let processes = self.api.list_background_processes().await?; if processes.is_empty() { self.writeln_title(TitleFormat::debug("No background processes running"))?; return Ok(()); } - // Build display strings for the picker. - // Format: "command | dir | elapsed | status" - let display_items: Vec = processes - .iter() - .map(|(p, alive)| { - let status = if *alive { "running" } else { "stopped" }; - let elapsed = humanize_time(p.started_at); - let dir = p - .cwd - .file_name() - .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_else(|| p.cwd.display().to_string()); - format!("{} | {} | {} | {}", p.command, dir, elapsed, status,) - }) - .collect(); + // Resolve disambiguated nicknames for all CWD paths. + let cwds: Vec = + processes.iter().map(|(p, _)| p.cwd.clone()).collect(); + let nicknames = forge_domain::resolve_nicknames(&cwds); - let selected = - ForgeWidget::select("Select a background process to kill", display_items.clone()) - .prompt()?; + // Build a tabular Info display (same pattern as model selector). + let mut info = Info::new(); + for (p, alive) in &processes { + let status = if *alive { status::YES } else { status::NO }; + let elapsed = humanize_time(p.started_at); + let dir = forge_domain::nickname_for(&p.cwd, &nicknames); + info = info + .add_title(&p.pid.to_string()) + .add_key_value("Command", &p.command) + .add_key_value("Directory", dir) + .add_key_value("Uptime", elapsed) + .add_key_value("Alive", status) + .add_key_value("Log", p.log_file.display().to_string()); + } - let Some(selected_str) = selected else { + let porcelain_output = Porcelain::from(&info).drop_col(0).uppercase_headers(); + let porcelain_str = porcelain_output.to_string(); + let all_lines: Vec<&str> = porcelain_str.lines().collect(); + if all_lines.is_empty() { + return Ok(()); + } + + // Typed row that carries the process index for lookup after selection. + #[derive(Clone)] + struct ProcessRow { + index: Option, + display: String, + } + impl std::fmt::Display for ProcessRow { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.display) + } + } + + let mut rows: Vec = Vec::with_capacity(all_lines.len()); + // Header row (non-selectable via header_lines=1). + rows.push(ProcessRow { index: None, display: all_lines[0].to_string() }); + // Data rows. + for (i, line) in all_lines.iter().skip(1).enumerate() { + rows.push(ProcessRow { index: Some(i), display: line.to_string() }); + } + + let selected = ForgeWidget::select( + "Background processes (select to kill, Esc to cancel)", + rows, + ) + .with_header_lines(1) + .prompt()?; + + let Some(row) = selected else { + return Ok(()); + }; + let Some(idx) = row.index else { return Ok(()); }; - // Find the PID by matching the selected display string back to the - // processes list (same order as display_items). - let idx = display_items - .iter() - .position(|s| s == &selected_str) - .ok_or_else(|| anyhow::anyhow!("Failed to match selection"))?; let pid = processes[idx].0.pid; let log_file = processes[idx].0.log_file.clone(); - // Kill the process and remove from manager, keeping log file for now - self.api.kill_background_process(pid, false)?; + // Kill the process and remove from manager, keeping log file for now. + self.api.kill_background_process(pid, false).await?; self.writeln_title(TitleFormat::action(format!("Killed process {pid}")))?; - // Ask about log file deletion + // Ask about log file deletion. let delete_log = ForgeWidget::confirm("Delete the log file?") .with_default(false) .prompt()?; @@ -2095,14 +2128,14 @@ impl A + Send + Sync> UI { delete_log: bool, ) -> anyhow::Result<()> { if let Some(pid) = kill_pid { - self.api.kill_background_process(pid, delete_log)?; + self.api.kill_background_process(pid, delete_log).await?; if !porcelain { self.writeln_title(TitleFormat::action(format!("Killed process {pid}")))?; } return Ok(()); } - let processes = self.api.list_background_processes()?; + let processes = self.api.list_background_processes().await?; if porcelain { // Tab-separated output for machine consumption for (p, alive) in &processes { @@ -2119,14 +2152,21 @@ impl A + Send + Sync> UI { } else if processes.is_empty() { self.writeln_title(TitleFormat::debug("No background processes running"))?; } else { + // Resolve disambiguated nicknames for all CWD paths. + let cwds: Vec = + processes.iter().map(|(p, _)| p.cwd.clone()).collect(); + let nicknames = forge_domain::resolve_nicknames(&cwds); + for (p, alive) in &processes { let status = if *alive { "running" } else { "stopped" }; let elapsed = humanize_time(p.started_at); + let dir = forge_domain::nickname_for(&p.cwd, &nicknames); self.writeln_title(TitleFormat::debug(format!( - "PID {} | {} | {} | {} | log: {}", + "PID {} | {} | {} | {} | {} | log: {}", p.pid, status, p.command, + dir, elapsed, p.log_file.display() )))?; diff --git a/crates/forge_services/src/tool_services/background_process.rs b/crates/forge_services/src/tool_services/background_process.rs index f34ff2329a..16b234340a 100644 --- a/crates/forge_services/src/tool_services/background_process.rs +++ b/crates/forge_services/src/tool_services/background_process.rs @@ -5,6 +5,8 @@ use anyhow::{Context, Result}; use chrono::Utc; use forge_domain::BackgroundProcess; +use super::ProcessMetadataService; + /// Owns the temp-file handles for background process log files so that they /// are automatically cleaned up when the manager is dropped. struct OwnedLogFile { @@ -26,18 +28,26 @@ impl std::fmt::Debug for OwnedLogFile { /// Thread-safe registry of background processes spawned during the current /// session. /// -/// When the manager is dropped all owned temp-file handles are released, -/// causing the underlying log files to be deleted automatically. -#[derive(Default, Debug)] +/// Processes are tracked both in-memory (for the current session) and persisted +/// to disk (for cross-session discovery). When the manager is dropped all owned +/// temp-file handles are released, causing the underlying log files to be +/// deleted automatically. +#[derive(Debug)] pub struct BackgroundProcessManager { processes: Mutex>, log_handles: Mutex>, + metadata: ProcessMetadataService, } impl BackgroundProcessManager { - /// Creates a new, empty manager. - pub fn new() -> Self { - Self::default() + /// Creates a new, empty manager that persists process metadata under the + /// given directory. + pub fn new(processes_dir: PathBuf) -> Self { + Self { + processes: Mutex::new(Vec::new()), + log_handles: Mutex::new(Vec::new()), + metadata: ProcessMetadataService::new(processes_dir), + } } /// Acquires the processes lock, returning an error if poisoned. @@ -56,6 +66,9 @@ impl BackgroundProcessManager { /// Register a newly spawned background process. /// + /// The process is stored both in-memory and persisted to disk so that + /// other sessions can discover it. + /// /// # Arguments /// /// * `pid` - OS process id of the spawned process. @@ -68,8 +81,8 @@ impl BackgroundProcessManager { /// /// # Errors /// - /// Returns an error if the internal lock is poisoned. - pub fn register( + /// Returns an error if the internal lock is poisoned or disk I/O fails. + pub async fn register( &self, pid: u32, command: String, @@ -81,6 +94,7 @@ impl BackgroundProcessManager { self.lock_processes()?.push(process.clone()); self.lock_log_handles()? .push(OwnedLogFile { _handle: log_handle, pid }); + self.metadata.save_process(&process).await?; Ok(process) } @@ -88,11 +102,19 @@ impl BackgroundProcessManager { /// /// This also drops the associated log-file handle. If `delete_log` is /// `false` the handle is persisted (leaked) so the file survives on disk. + /// The process is also removed from disk metadata. /// /// # Errors /// - /// Returns an error if the internal lock is poisoned. - fn remove(&self, pid: u32, delete_log: bool) -> Result<()> { + /// Returns an error if the internal lock is poisoned or disk I/O fails. + async fn remove(&self, pid: u32, delete_log: bool) -> Result<()> { + // Look up the CWD before removing so we can update the disk metadata. + let cwd = self + .lock_processes()? + .iter() + .find(|p| p.pid == pid) + .map(|p| p.cwd.clone()); + self.lock_processes()?.retain(|p| p.pid != pid); if delete_log { @@ -104,6 +126,10 @@ impl BackgroundProcessManager { let _ = owned._handle.keep(); } } + + if let Some(cwd) = cwd { + self.metadata.remove_process(&cwd, pid).await?; + } Ok(()) } @@ -111,31 +137,66 @@ impl BackgroundProcessManager { /// /// Returns `Ok(())` if the process was killed or was already dead. /// The `delete_log` flag controls whether the log file is deleted. + /// The process is also removed from disk metadata only after confirming + /// the process is no longer alive. /// /// # Errors /// /// Returns an error if the process could not be killed or the lock is /// poisoned. - pub fn kill(&self, pid: u32, delete_log: bool) -> Result<()> { + pub async fn kill(&self, pid: u32, delete_log: bool) -> Result<()> { kill_process(pid).context("failed to kill background process")?; - self.remove(pid, delete_log)?; + + // Give the OS a moment to reap the process, then verify it's gone. + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + if is_process_alive(pid) { + anyhow::bail!( + "process {pid} is still alive after kill signal; metadata preserved" + ); + } + + self.remove(pid, delete_log).await?; Ok(()) } - /// Returns a snapshot of all tracked processes with their alive status. + /// Returns all tracked processes with their alive status. + /// + /// Merges in-memory processes (current session) with disk-persisted + /// processes (other sessions). Deduplicates by PID. Dead processes from + /// crashed sessions are automatically garbage-collected from disk. /// /// # Errors /// - /// Returns an error if the internal lock is poisoned. - pub fn list_with_status(&self) -> Result> { - Ok(self - .lock_processes()? - .iter() - .map(|p| { - let alive = is_process_alive(p.pid); - (p.clone(), alive) - }) - .collect()) + /// Returns an error if the internal lock is poisoned or disk I/O fails. + pub async fn list_with_status(&self) -> Result> { + // Start with in-memory processes. + let in_memory: Vec = self.lock_processes()?.clone(); + let in_memory_pids: std::collections::HashSet = + in_memory.iter().map(|p| p.pid).collect(); + + // Load persisted processes from disk and merge (skip duplicates). + let disk_processes = self.metadata.list_all_processes().await?; + + let mut all = in_memory; + for dp in disk_processes { + if !in_memory_pids.contains(&dp.pid) { + all.push(dp); + } + } + + // Check alive status and garbage-collect dead disk-only processes. + let mut result = Vec::with_capacity(all.len()); + for p in &all { + let alive = is_process_alive(p.pid); + if !alive && !in_memory_pids.contains(&p.pid) { + // Dead process from another session -- remove from disk. + let _ = self.metadata.remove_process(&p.cwd, p.pid).await; + } else { + result.push((p.clone(), alive)); + } + } + + Ok(result) } } @@ -148,17 +209,45 @@ fn is_process_alive(pid: u32) -> bool { } /// Cross-platform process termination. +/// +/// Kills the process and all its descendants by walking the process tree +/// via `sysinfo` and killing every child recursively before killing the +/// root. This ensures that servers spawned as grandchildren (e.g. `nohup +/// npm start` which spawns `node`) are also terminated. fn kill_process(pid: u32) -> anyhow::Result<()> { let s = sysinfo::System::new_with_specifics( - sysinfo::RefreshKind::nothing().with_processes(sysinfo::ProcessRefreshKind::nothing()), + sysinfo::RefreshKind::nothing() + .with_processes(sysinfo::ProcessRefreshKind::nothing()), ); - match s.process(sysinfo::Pid::from_u32(pid)) { - Some(process) => { + + let target = sysinfo::Pid::from_u32(pid); + + // Collect all descendants (children, grandchildren, etc.) bottom-up. + let mut to_kill = Vec::new(); + collect_descendants(&s, target, &mut to_kill); + // Add the root last so children die first. + to_kill.push(target); + + for pid in &to_kill { + if let Some(process) = s.process(*pid) { process.kill(); - Ok(()) } - // Process already gone -- nothing to kill. - None => Ok(()), + } + + Ok(()) +} + +/// Recursively collects all descendant PIDs of `parent` into `out`. +fn collect_descendants( + system: &sysinfo::System, + parent: sysinfo::Pid, + out: &mut Vec, +) { + for (child_pid, proc) in system.processes() { + if proc.parent() == Some(parent) && *child_pid != parent { + collect_descendants(system, *child_pid, out); + out.push(*child_pid); + } } } @@ -167,6 +256,7 @@ mod tests { use std::io::Write; use pretty_assertions::assert_eq; + use tempfile::TempDir; use super::*; @@ -180,9 +270,15 @@ mod tests { f } - #[test] - fn test_register_and_list_with_status() { - let fixture = BackgroundProcessManager::new(); + fn make_manager() -> (BackgroundProcessManager, TempDir) { + let dir = TempDir::new().unwrap(); + let manager = BackgroundProcessManager::new(dir.path().to_path_buf()); + (manager, dir) + } + + #[tokio::test] + async fn test_register_and_list_with_status() { + let (fixture, _dir) = make_manager(); let log = create_temp_log(); let log_path = log.path().to_path_buf(); @@ -194,9 +290,10 @@ mod tests { log_path.clone(), log, ) + .await .unwrap(); - let actual = fixture.list_with_status().unwrap(); + let actual = fixture.list_with_status().await.unwrap(); assert_eq!(actual.len(), 1); assert_eq!(actual[0].0.pid, 1234); @@ -204,9 +301,9 @@ mod tests { assert_eq!(actual[0].0.log_file, log_path); } - #[test] - fn test_remove_with_log_deletion() { - let fixture = BackgroundProcessManager::new(); + #[tokio::test] + async fn test_remove_with_log_deletion() { + let (fixture, _dir) = make_manager(); let log = create_temp_log(); let log_path = log.path().to_path_buf(); @@ -218,18 +315,19 @@ mod tests { log_path.clone(), log, ) + .await .unwrap(); - assert_eq!(fixture.list_with_status().unwrap().len(), 1); + assert_eq!(fixture.list_with_status().await.unwrap().len(), 1); - fixture.remove(100, true).unwrap(); + fixture.remove(100, true).await.unwrap(); - assert_eq!(fixture.list_with_status().unwrap().len(), 0); + assert_eq!(fixture.list_with_status().await.unwrap().len(), 0); assert!(!log_path.exists()); } - #[test] - fn test_remove_without_log_deletion() { - let fixture = BackgroundProcessManager::new(); + #[tokio::test] + async fn test_remove_without_log_deletion() { + let (fixture, _dir) = make_manager(); let log = create_temp_log(); let log_path = log.path().to_path_buf(); @@ -241,19 +339,20 @@ mod tests { log_path.clone(), log, ) + .await .unwrap(); - fixture.remove(200, false).unwrap(); + fixture.remove(200, false).await.unwrap(); - assert_eq!(fixture.list_with_status().unwrap().len(), 0); + assert_eq!(fixture.list_with_status().await.unwrap().len(), 0); assert!(log_path.exists()); let _ = std::fs::remove_file(&log_path); } - #[test] - fn test_multiple_processes() { - let fixture = BackgroundProcessManager::new(); + #[tokio::test] + async fn test_multiple_processes() { + let (fixture, _dir) = make_manager(); let log1 = create_temp_log(); let path1 = log1.path().to_path_buf(); @@ -268,6 +367,7 @@ mod tests { path1, log1, ) + .await .unwrap(); fixture .register( @@ -277,24 +377,26 @@ mod tests { path2, log2, ) + .await .unwrap(); - assert_eq!(fixture.list_with_status().unwrap().len(), 2); + assert_eq!(fixture.list_with_status().await.unwrap().len(), 2); - fixture.remove(10, true).unwrap(); + fixture.remove(10, true).await.unwrap(); - let actual = fixture.list_with_status().unwrap(); + let actual = fixture.list_with_status().await.unwrap(); assert_eq!(actual.len(), 1); assert_eq!(actual[0].0.pid, 20); } - #[test] - fn test_drop_cleans_up_temp_files() { + #[tokio::test] + async fn test_drop_cleans_up_temp_files() { let log = create_temp_log(); let log_path = log.path().to_path_buf(); { - let manager = BackgroundProcessManager::new(); + let dir = TempDir::new().unwrap(); + let manager = BackgroundProcessManager::new(dir.path().to_path_buf()); manager .register( 300, @@ -303,6 +405,7 @@ mod tests { log_path.clone(), log, ) + .await .unwrap(); assert!(log_path.exists()); } @@ -310,9 +413,9 @@ mod tests { assert!(!log_path.exists()); } - #[test] - fn test_list_with_status_shows_dead_process() { - let fixture = BackgroundProcessManager::new(); + #[tokio::test] + async fn test_list_with_status_shows_dead_process() { + let (fixture, _dir) = make_manager(); let log = create_temp_log(); let path = log.path().to_path_buf(); @@ -324,12 +427,72 @@ mod tests { path, log, ) + .await .unwrap(); - let actual = fixture.list_with_status().unwrap(); + let actual = fixture.list_with_status().await.unwrap(); + // The process is in-memory (current session), so it's kept even if dead. assert_eq!(actual.len(), 1); assert_eq!(actual[0].0.pid, 99999); assert!(!actual[0].1); } + + #[tokio::test] + async fn test_register_persists_to_disk() { + let dir = TempDir::new().unwrap(); + let processes_dir = dir.path().to_path_buf(); + + // Register a process in one manager. + let manager1 = BackgroundProcessManager::new(processes_dir.clone()); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + manager1 + .register( + 42, + "persisted cmd".to_string(), + PathBuf::from("/project"), + log_path, + log, + ) + .await + .unwrap(); + + // A second manager reading the same directory should see the persisted + // process (it will be GC'd since PID 42 is dead, so we just verify + // the metadata service sees it). + let metadata = ProcessMetadataService::new(processes_dir); + let persisted = metadata.list_all_processes().await.unwrap(); + assert_eq!(persisted.len(), 1); + assert_eq!(persisted[0].pid, 42); + assert_eq!(persisted[0].command, "persisted cmd"); + } + + #[tokio::test] + async fn test_remove_cleans_up_disk_metadata() { + let dir = TempDir::new().unwrap(); + let processes_dir = dir.path().to_path_buf(); + let manager = BackgroundProcessManager::new(processes_dir.clone()); + let log = create_temp_log(); + let log_path = log.path().to_path_buf(); + + manager + .register( + 55, + "killable".to_string(), + PathBuf::from("/proj"), + log_path, + log, + ) + .await + .unwrap(); + + // Use remove() directly since kill() requires a real OS process. + manager.remove(55, true).await.unwrap(); + + // Verify removed from disk. + let metadata = ProcessMetadataService::new(processes_dir); + let persisted = metadata.list_all_processes().await.unwrap(); + assert!(persisted.is_empty()); + } } diff --git a/crates/forge_services/src/tool_services/mod.rs b/crates/forge_services/src/tool_services/mod.rs index 3f67bf25fe..f4e3618000 100644 --- a/crates/forge_services/src/tool_services/mod.rs +++ b/crates/forge_services/src/tool_services/mod.rs @@ -9,6 +9,7 @@ mod fs_undo; mod fs_write; mod image_read; mod plan_create; +mod process_metadata; mod shell; mod skill; @@ -23,5 +24,6 @@ pub use fs_undo::*; pub use fs_write::*; pub use image_read::*; pub use plan_create::*; +pub use process_metadata::*; pub use shell::*; pub use skill::*; diff --git a/crates/forge_services/src/tool_services/shell.rs b/crates/forge_services/src/tool_services/shell.rs index 9cfb2928c7..aa23c1fae4 100644 --- a/crates/forge_services/src/tool_services/shell.rs +++ b/crates/forge_services/src/tool_services/shell.rs @@ -30,7 +30,7 @@ impl ForgeShell { /// process manager for tracking long-running detached processes. pub fn new(infra: Arc) -> Self { let env = infra.get_environment(); - let bg_manager = Arc::new(BackgroundProcessManager::new()); + let bg_manager = Arc::new(BackgroundProcessManager::new(env.processes_path())); Self { env, infra, bg_manager } } @@ -70,7 +70,7 @@ impl ShellService for ForgeShell { cwd, bg_output.log_file.clone(), bg_output.log_handle, - )?; + ).await?; return Ok(ShellOutput { kind: ShellOutputKind::Background { @@ -100,14 +100,14 @@ impl ShellService for ForgeShell { }) } - fn list_background_processes( + async fn list_background_processes( &self, ) -> anyhow::Result> { - self.bg_manager.list_with_status() + self.bg_manager.list_with_status().await } - fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { - self.bg_manager.kill(pid, delete_log) + async fn kill_background_process(&self, pid: u32, delete_log: bool) -> anyhow::Result<()> { + self.bg_manager.kill(pid, delete_log).await } } #[cfg(test)] @@ -125,6 +125,8 @@ mod tests { struct MockCommandInfra { expected_env_vars: Option>, + _temp_dir: tempfile::TempDir, + base_path: PathBuf, } #[async_trait] @@ -180,7 +182,9 @@ mod tests { impl EnvironmentInfra for MockCommandInfra { fn get_environment(&self) -> Environment { use fake::{Fake, Faker}; - Faker.fake() + let mut env: Environment = Faker.fake(); + env.base_path = self.base_path.clone(); + env } fn get_env_var(&self, _key: &str) -> Option { @@ -197,7 +201,13 @@ mod tests { } fn make_shell(expected_env_vars: Option>) -> ForgeShell { - ForgeShell::new(Arc::new(MockCommandInfra { expected_env_vars })) + let temp_dir = tempfile::tempdir().unwrap(); + let base_path = temp_dir.path().to_path_buf(); + ForgeShell::new(Arc::new(MockCommandInfra { + expected_env_vars, + _temp_dir: temp_dir, + base_path, + })) } /// Extracts the foreground CommandOutput from a ShellOutput, panicking if @@ -353,7 +363,7 @@ mod tests { _ => panic!("Expected Background"), } - let tracked = fixture.list_background_processes().unwrap(); + let tracked = fixture.list_background_processes().await.unwrap(); assert_eq!(tracked.len(), 1); assert_eq!(tracked[0].0.pid, 9999); } From 5fa99923388505257e9adfbf6bb6129afa511c17 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:57:20 +0000 Subject: [PATCH 07/11] [autofix.ci] apply automated fixes --- crates/forge_api/src/api.rs | 4 ++- crates/forge_api/src/forge_api.rs | 9 +++++-- crates/forge_domain/src/env.rs | 3 ++- crates/forge_domain/src/nickname.rs | 25 ++++++++----------- crates/forge_main/src/ui.rs | 15 +++++------ .../src/tool_services/background_process.rs | 7 ++---- .../src/tool_services/process_metadata.rs | 10 ++++++-- .../forge_services/src/tool_services/shell.rs | 16 ++++++------ 8 files changed, 48 insertions(+), 41 deletions(-) diff --git a/crates/forge_api/src/api.rs b/crates/forge_api/src/api.rs index 1ce83e456e..f995d34cf8 100644 --- a/crates/forge_api/src/api.rs +++ b/crates/forge_api/src/api.rs @@ -254,7 +254,9 @@ pub trait API: Sync + Send { ) -> Result>>; /// Returns all tracked background processes with their alive status. - async fn list_background_processes(&self) -> Result>; + async fn list_background_processes( + &self, + ) -> Result>; /// Kills a background process by PID and optionally deletes its log file. async fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()>; diff --git a/crates/forge_api/src/forge_api.rs b/crates/forge_api/src/forge_api.rs index 2cb1995f86..a944cf3cb4 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -415,8 +415,13 @@ impl< app.execute(data_parameters).await } - async fn list_background_processes(&self) -> Result> { - self.services.shell_service().list_background_processes().await + async fn list_background_processes( + &self, + ) -> Result> { + self.services + .shell_service() + .list_background_processes() + .await } async fn kill_background_process(&self, pid: u32, delete_log: bool) -> Result<()> { diff --git a/crates/forge_domain/src/env.rs b/crates/forge_domain/src/env.rs index 2551d2c70b..158d7bb2eb 100644 --- a/crates/forge_domain/src/env.rs +++ b/crates/forge_domain/src/env.rs @@ -140,7 +140,8 @@ impl Environment { pub fn snapshot_path(&self) -> PathBuf { self.base_path.join("snapshots") } - /// Returns the directory where background process metadata files are stored. + /// Returns the directory where background process metadata files are + /// stored. pub fn processes_path(&self) -> PathBuf { self.base_path.join("processes") } diff --git a/crates/forge_domain/src/nickname.rs b/crates/forge_domain/src/nickname.rs index 250a02a6d8..9a50981cb1 100644 --- a/crates/forge_domain/src/nickname.rs +++ b/crates/forge_domain/src/nickname.rs @@ -122,15 +122,18 @@ mod tests { #[test] fn test_two_paths_same_last_component() { - let paths = vec![ - PathBuf::from("/a/b/server"), - PathBuf::from("/c/d/server"), - ]; + let paths = vec![PathBuf::from("/a/b/server"), PathBuf::from("/c/d/server")]; let actual = resolve_nicknames(&paths); - assert_eq!(actual[&paths[0]], format!("b{}server", std::path::MAIN_SEPARATOR)); - assert_eq!(actual[&paths[1]], format!("d{}server", std::path::MAIN_SEPARATOR)); + assert_eq!( + actual[&paths[0]], + format!("b{}server", std::path::MAIN_SEPARATOR) + ); + assert_eq!( + actual[&paths[1]], + format!("d{}server", std::path::MAIN_SEPARATOR) + ); } #[test] @@ -160,10 +163,7 @@ mod tests { #[test] fn test_identical_paths() { - let paths = vec![ - PathBuf::from("/a/b/c"), - PathBuf::from("/a/b/c"), - ]; + let paths = vec![PathBuf::from("/a/b/c"), PathBuf::from("/a/b/c")]; let actual = resolve_nicknames(&paths); @@ -201,10 +201,7 @@ mod tests { #[test] fn test_nickname_for_helper() { - let paths = vec![ - PathBuf::from("/a/b/server"), - PathBuf::from("/c/d/client"), - ]; + let paths = vec![PathBuf::from("/a/b/server"), PathBuf::from("/c/d/client")]; let nicknames = resolve_nicknames(&paths); let actual = nickname_for(&PathBuf::from("/a/b/server"), &nicknames); diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 554ef75b7e..bbf686833b 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -2036,8 +2036,7 @@ impl A + Send + Sync> UI { } // Resolve disambiguated nicknames for all CWD paths. - let cwds: Vec = - processes.iter().map(|(p, _)| p.cwd.clone()).collect(); + let cwds: Vec = processes.iter().map(|(p, _)| p.cwd.clone()).collect(); let nicknames = forge_domain::resolve_nicknames(&cwds); // Build a tabular Info display (same pattern as model selector). @@ -2047,7 +2046,7 @@ impl A + Send + Sync> UI { let elapsed = humanize_time(p.started_at); let dir = forge_domain::nickname_for(&p.cwd, &nicknames); info = info - .add_title(&p.pid.to_string()) + .add_title(p.pid.to_string()) .add_key_value("Command", &p.command) .add_key_value("Directory", dir) .add_key_value("Uptime", elapsed) @@ -2082,12 +2081,10 @@ impl A + Send + Sync> UI { rows.push(ProcessRow { index: Some(i), display: line.to_string() }); } - let selected = ForgeWidget::select( - "Background processes (select to kill, Esc to cancel)", - rows, - ) - .with_header_lines(1) - .prompt()?; + let selected = + ForgeWidget::select("Background processes (select to kill, Esc to cancel)", rows) + .with_header_lines(1) + .prompt()?; let Some(row) = selected else { return Ok(()); diff --git a/crates/forge_services/src/tool_services/background_process.rs b/crates/forge_services/src/tool_services/background_process.rs index 16b234340a..d1221e6c23 100644 --- a/crates/forge_services/src/tool_services/background_process.rs +++ b/crates/forge_services/src/tool_services/background_process.rs @@ -150,9 +150,7 @@ impl BackgroundProcessManager { // Give the OS a moment to reap the process, then verify it's gone. tokio::time::sleep(std::time::Duration::from_millis(100)).await; if is_process_alive(pid) { - anyhow::bail!( - "process {pid} is still alive after kill signal; metadata preserved" - ); + anyhow::bail!("process {pid} is still alive after kill signal; metadata preserved"); } self.remove(pid, delete_log).await?; @@ -216,8 +214,7 @@ fn is_process_alive(pid: u32) -> bool { /// npm start` which spawns `node`) are also terminated. fn kill_process(pid: u32) -> anyhow::Result<()> { let s = sysinfo::System::new_with_specifics( - sysinfo::RefreshKind::nothing() - .with_processes(sysinfo::ProcessRefreshKind::nothing()), + sysinfo::RefreshKind::nothing().with_processes(sysinfo::ProcessRefreshKind::nothing()), ); let target = sysinfo::Pid::from_u32(pid); diff --git a/crates/forge_services/src/tool_services/process_metadata.rs b/crates/forge_services/src/tool_services/process_metadata.rs index 449c1ef312..e74114857b 100644 --- a/crates/forge_services/src/tool_services/process_metadata.rs +++ b/crates/forge_services/src/tool_services/process_metadata.rs @@ -173,7 +173,10 @@ mod tests { fixture.save_process(&p1).await.unwrap(); fixture.save_process(&p2).await.unwrap(); - fixture.remove_process(&PathBuf::from("/proj"), 10).await.unwrap(); + fixture + .remove_process(&PathBuf::from("/proj"), 10) + .await + .unwrap(); let actual = fixture.list_all_processes().await.unwrap(); assert_eq!(actual.len(), 1); @@ -190,7 +193,10 @@ mod tests { fixture.save_process(&process).await.unwrap(); assert!(ForgeFS::exists(&meta_path)); - fixture.remove_process(&PathBuf::from("/proj"), 100).await.unwrap(); + fixture + .remove_process(&PathBuf::from("/proj"), 100) + .await + .unwrap(); assert!(!ForgeFS::exists(&meta_path)); } diff --git a/crates/forge_services/src/tool_services/shell.rs b/crates/forge_services/src/tool_services/shell.rs index aa23c1fae4..67ec3386b4 100644 --- a/crates/forge_services/src/tool_services/shell.rs +++ b/crates/forge_services/src/tool_services/shell.rs @@ -64,13 +64,15 @@ impl ShellService for ForgeShell { // Register with the background process manager which takes // ownership of the temp-file handle (keeps the log file alive). - self.bg_manager.register( - bg_output.pid, - bg_output.command.clone(), - cwd, - bg_output.log_file.clone(), - bg_output.log_handle, - ).await?; + self.bg_manager + .register( + bg_output.pid, + bg_output.command.clone(), + cwd, + bg_output.log_file.clone(), + bg_output.log_handle, + ) + .await?; return Ok(ShellOutput { kind: ShellOutputKind::Background { From 4446b04665b1e0b38a8d89435748ab0f9dd586e7 Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod <62684960+ssddOnTop@users.noreply.github.com> Date: Tue, 24 Mar 2026 16:09:17 -0400 Subject: [PATCH 08/11] feat(shell): add fzf-based background process management action --- shell-plugin/lib/actions/processes.zsh | 55 ++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 shell-plugin/lib/actions/processes.zsh diff --git a/shell-plugin/lib/actions/processes.zsh b/shell-plugin/lib/actions/processes.zsh new file mode 100644 index 0000000000..3c64ac3cef --- /dev/null +++ b/shell-plugin/lib/actions/processes.zsh @@ -0,0 +1,55 @@ +#!/usr/bin/env zsh + +# Action handlers for background process management + +# Action handler: List and manage background processes +# Uses fzf to select a process, then offers to kill it and optionally delete +# the log file. +function _forge_action_processes() { + echo + + # Get process list in porcelain format (tab-separated) + local output=$($_FORGE_BIN processes --porcelain 2>/dev/null) + + if [[ -z "$output" ]]; then + _forge_log info "No background processes running" + return 0 + fi + + # Build display lines for fzf + local -a display_lines + local -a pids + local -a cwds + while IFS=$'\t' read -r pid proc_status command cwd started_at log_file; do + display_lines+=("PID ${pid} | ${proc_status} | ${command} | ${cwd} | log: ${log_file}") + pids+=("$pid") + cwds+=("$cwd") + done <<< "$output" + + # Use fzf to select a process + local selected=$(printf '%s\n' "${display_lines[@]}" | fzf --prompt="Select process to kill > " --height=~40% --border) + + if [[ -z "$selected" ]]; then + return 0 + fi + + # Extract PID from selection + local selected_pid=$(echo "$selected" | grep -oP 'PID \K[0-9]+') + + if [[ -z "$selected_pid" ]]; then + _forge_log error "Failed to parse PID from selection" + return 1 + fi + + # Kill the process (keep log file by default) + $_FORGE_BIN processes --kill "$selected_pid" 2>/dev/null + _forge_log info "Killed process ${selected_pid}" + + # Ask about log file deletion + echo -n "Delete the log file? [y/N] " + read -r response + if [[ "${response:l}" == "y" || "${response:l}" == "yes" ]]; then + $_FORGE_BIN processes --kill "$selected_pid" --delete-log 2>/dev/null + _forge_log info "Deleted log file" + fi +} From 4649c061ee71ad4a969f5ea87107bbbd93afe971 Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod <62684960+ssddOnTop@users.noreply.github.com> Date: Tue, 24 Mar 2026 16:09:30 -0400 Subject: [PATCH 09/11] refactor(shell): improve background process list with fzf preview and deduplicate nicknames --- crates/forge_domain/src/nickname.rs | 27 ++++--- crates/forge_main/src/ui.rs | 32 +++++---- .../src/tool_services/background_process.rs | 9 +-- shell-plugin/lib/actions/processes.zsh | 71 ++++++++++--------- 4 files changed, 79 insertions(+), 60 deletions(-) diff --git a/crates/forge_domain/src/nickname.rs b/crates/forge_domain/src/nickname.rs index 9a50981cb1..184f4f1c8e 100644 --- a/crates/forge_domain/src/nickname.rs +++ b/crates/forge_domain/src/nickname.rs @@ -18,8 +18,19 @@ pub fn resolve_nicknames(paths: &[PathBuf]) -> HashMap { return HashMap::new(); } - // Collect the components of each path in reverse order (leaf first). - let components: Vec> = paths + // Deduplicate: the algorithm only needs to disambiguate *distinct* paths. + // Identical paths share the same nickname by definition. + let unique_paths: Vec = { + let mut seen = std::collections::HashSet::new(); + paths + .iter() + .filter(|p| seen.insert((*p).clone())) + .cloned() + .collect() + }; + + // Collect the components of each unique path in reverse order (leaf first). + let components: Vec> = unique_paths .iter() .map(|p| { p.components() @@ -30,7 +41,7 @@ pub fn resolve_nicknames(paths: &[PathBuf]) -> HashMap { .collect(); // Start with 1 component (just the leaf) for every path. - let mut depths: Vec = vec![1; paths.len()]; + let mut depths: Vec = vec![1; unique_paths.len()]; loop { // Build current nicknames at each path's depth. @@ -62,8 +73,8 @@ pub fn resolve_nicknames(paths: &[PathBuf]) -> HashMap { if !any_extended { // No further disambiguation possible or all unique. - let mut result = HashMap::with_capacity(paths.len()); - for (i, path) in paths.iter().enumerate() { + let mut result = HashMap::with_capacity(unique_paths.len()); + for (i, path) in unique_paths.iter().enumerate() { result .entry(path.clone()) .or_insert_with(|| nicknames[i].clone()); @@ -168,10 +179,8 @@ mod tests { let actual = resolve_nicknames(&paths); // Both map to the same key so only one entry in the HashMap. - // The nickname should be the full path since they cannot be - // disambiguated. - let sep = std::path::MAIN_SEPARATOR; - assert_eq!(actual[&paths[0]], format!("{sep}a{sep}b{sep}c")); + // Duplicates are collapsed, so the nickname is just the leaf. + assert_eq!(actual[&paths[0]], "c"); } #[test] diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index fb444b5dd0..d779eab292 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -2042,16 +2042,14 @@ impl A + Send + Sync> UI { // Build a tabular Info display (same pattern as model selector). let mut info = Info::new(); - for (p, alive) in &processes { - let status = if *alive { status::YES } else { status::NO }; + for (p, _alive) in &processes { let elapsed = humanize_time(p.started_at); let dir = forge_domain::nickname_for(&p.cwd, &nicknames); info = info .add_title(p.pid.to_string()) .add_key_value("Command", &p.command) - .add_key_value("Directory", dir) + .add_key_value("Name", dir) .add_key_value("Uptime", elapsed) - .add_key_value("Alive", status) .add_key_value("Log", p.log_file.display().to_string()); } @@ -2135,18 +2133,22 @@ impl A + Send + Sync> UI { let processes = self.api.list_background_processes().await?; if porcelain { - // Tab-separated output for machine consumption - for (p, alive) in &processes { - let status = if *alive { "running" } else { "stopped" }; - println!( - "{}\t{}\t{}\t{}\t{}", - p.pid, - status, - p.command, - p.started_at.to_rfc3339(), - p.log_file.display() - ); + // Porcelain output with aligned columns and header (matches + // provider list format). Columns: PID, COMMAND, NAME, LOG. + let cwds: Vec = + processes.iter().map(|(p, _)| p.cwd.clone()).collect(); + let nicknames = forge_domain::resolve_nicknames(&cwds); + let mut info = Info::new(); + for (p, _) in &processes { + let dir = forge_domain::nickname_for(&p.cwd, &nicknames); + info = info + .add_title(p.pid.to_string()) + .add_key_value("command", &p.command) + .add_key_value("name", dir) + .add_key_value("log", p.log_file.display().to_string()); } + let porcelain_output = Porcelain::from(&info).uppercase_headers(); + print!("{porcelain_output}"); } else if processes.is_empty() { self.writeln_title(TitleFormat::debug("No background processes running"))?; } else { diff --git a/crates/forge_services/src/tool_services/background_process.rs b/crates/forge_services/src/tool_services/background_process.rs index d1221e6c23..94c261b320 100644 --- a/crates/forge_services/src/tool_services/background_process.rs +++ b/crates/forge_services/src/tool_services/background_process.rs @@ -1,11 +1,11 @@ use std::path::PathBuf; use std::sync::Mutex; +use super::ProcessMetadataService; use anyhow::{Context, Result}; use chrono::Utc; use forge_domain::BackgroundProcess; - -use super::ProcessMetadataService; +use forge_fs::ForgeFS; /// Owns the temp-file handles for background process log files so that they /// are automatically cleaned up when the manager is dropped. @@ -187,8 +187,9 @@ impl BackgroundProcessManager { for p in &all { let alive = is_process_alive(p.pid); if !alive && !in_memory_pids.contains(&p.pid) { - // Dead process from another session -- remove from disk. - let _ = self.metadata.remove_process(&p.cwd, p.pid).await; + // Dead process from another session -- remove metadata and log. + self.metadata.remove_process(&p.cwd, p.pid).await.ok(); + ForgeFS::remove_file(&p.log_file).await.ok(); } else { result.push((p.clone(), alive)); } diff --git a/shell-plugin/lib/actions/processes.zsh b/shell-plugin/lib/actions/processes.zsh index 3c64ac3cef..b693950a38 100644 --- a/shell-plugin/lib/actions/processes.zsh +++ b/shell-plugin/lib/actions/processes.zsh @@ -3,53 +3,60 @@ # Action handlers for background process management # Action handler: List and manage background processes -# Uses fzf to select a process, then offers to kill it and optionally delete -# the log file. +# Uses fzf with log file preview to select a process, then offers to kill it. +# Porcelain format: PID COMMAND NAME LOG (multi-space aligned with header) +# PID is hidden from fzf display using --with-nth=2.. function _forge_action_processes() { echo - - # Get process list in porcelain format (tab-separated) + local output=$($_FORGE_BIN processes --porcelain 2>/dev/null) - + if [[ -z "$output" ]]; then _forge_log info "No background processes running" return 0 fi - - # Build display lines for fzf - local -a display_lines - local -a pids - local -a cwds - while IFS=$'\t' read -r pid proc_status command cwd started_at log_file; do - display_lines+=("PID ${pid} | ${proc_status} | ${command} | ${cwd} | log: ${log_file}") - pids+=("$pid") - cwds+=("$cwd") - done <<< "$output" - - # Use fzf to select a process - local selected=$(printf '%s\n' "${display_lines[@]}" | fzf --prompt="Select process to kill > " --height=~40% --border) - + + # fzf shows columns 2+ (COMMAND, NAME, LOG), hiding PID (col 1). + # {1} = PID, {-1} = LOG path for preview. + local selected + selected=$(echo "$output" | \ + _forge_fzf \ + --header-lines=1 \ + --delimiter="$_FORGE_DELIMITER" \ + --with-nth=2.. \ + --prompt="Kill a process ❯ " \ + --preview="${_FORGE_CAT_CMD} {-1}" \ + $_FORGE_PREVIEW_WINDOW) + if [[ -z "$selected" ]]; then return 0 fi - - # Extract PID from selection - local selected_pid=$(echo "$selected" | grep -oP 'PID \K[0-9]+') - + + # Extract PID (first multi-space delimited field, hidden from display) + local selected_pid="${selected%% *}" + selected_pid="${selected_pid## }" + selected_pid="${selected_pid%% }" + if [[ -z "$selected_pid" ]]; then _forge_log error "Failed to parse PID from selection" return 1 fi - - # Kill the process (keep log file by default) + + # Extract log file path (last field) + local selected_log="${selected##* }" + selected_log="${selected_log## }" + selected_log="${selected_log%% }" + + # Kill the process $_FORGE_BIN processes --kill "$selected_pid" 2>/dev/null - _forge_log info "Killed process ${selected_pid}" - + # Ask about log file deletion - echo -n "Delete the log file? [y/N] " - read -r response - if [[ "${response:l}" == "y" || "${response:l}" == "yes" ]]; then - $_FORGE_BIN processes --kill "$selected_pid" --delete-log 2>/dev/null - _forge_log info "Deleted log file" + if [[ -n "$selected_log" && -f "$selected_log" ]]; then + echo -n "Delete the log file? [y/N] " + read -r response + if [[ "${response:l}" == "y" || "${response:l}" == "yes" ]]; then + rm -f "$selected_log" 2>/dev/null + _forge_log info "Deleted log file: ${selected_log}" + fi fi } From 8cf1b85dff390fe8d3660a848b490646b8bd55ab Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 24 Mar 2026 20:26:56 +0000 Subject: [PATCH 10/11] [autofix.ci] apply automated fixes --- crates/forge_services/src/tool_services/background_process.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/forge_services/src/tool_services/background_process.rs b/crates/forge_services/src/tool_services/background_process.rs index 94c261b320..89c2008ed2 100644 --- a/crates/forge_services/src/tool_services/background_process.rs +++ b/crates/forge_services/src/tool_services/background_process.rs @@ -1,12 +1,13 @@ use std::path::PathBuf; use std::sync::Mutex; -use super::ProcessMetadataService; use anyhow::{Context, Result}; use chrono::Utc; use forge_domain::BackgroundProcess; use forge_fs::ForgeFS; +use super::ProcessMetadataService; + /// Owns the temp-file handles for background process log files so that they /// are automatically cleaned up when the manager is dropped. struct OwnedLogFile { From 120168a5a9fc04f68a0c8722a705240c8c9ba1f6 Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod Date: Wed, 25 Mar 2026 00:40:46 -0400 Subject: [PATCH 11/11] Update crates/forge_infra/src/executor.rs Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com> --- crates/forge_infra/src/executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/forge_infra/src/executor.rs b/crates/forge_infra/src/executor.rs index 69e03324d4..a84cc61efd 100644 --- a/crates/forge_infra/src/executor.rs +++ b/crates/forge_infra/src/executor.rs @@ -283,7 +283,7 @@ async fn spawn_background_process( cmd.args(["/C", "start", "/b", "cmd", "/C", &bg_command]); cmd } else { - let bg_command = format!("nohup {command} > {log_path} 2>&1 & echo $!"); + let bg_command = format!("nohup {command} > '{log_path}' 2>&1 & echo $!"); let mut cmd = Command::new(shell); cmd.arg("-c").arg(&bg_command); cmd