feat(pipeline): centralize bounded process execution

crates/renderflow-core/src/adapters/command.rs

@@ -1,65 +1,56 @@
-use anyhow::{bail, Result};
-use std::io::ErrorKind;
-use std::process::Command;
+use anyhow::Result;
 use tracing::{error, info};
 
-pub fn run_command(program: &str, args: &[&str]) -> Result<()> {
-    info!(program = program, args = ?args, "Running command");
-
-    let output = Command::new(program).args(args).output().map_err(|e| {
-        if e.kind() == ErrorKind::NotFound {
-            anyhow::anyhow!(
-                "`{}` was not found. Make sure it is installed and available in your PATH.",
-                program
-            )
-        } else {
-            anyhow::anyhow!("Failed to launch `{}`: {}", program, e)
-        }
-    })?;
-
-    let stdout = String::from_utf8_lossy(&output.stdout);
-    let stderr = String::from_utf8_lossy(&output.stderr);
+use crate::process::{
+    is_explicit_shell_invocation, ProcessExecutor, ProcessRequest, DEFAULT_CAPTURE_LIMIT_BYTES,
+    DEFAULT_PROCESS_TIMEOUT,
+};
 
-    if !stdout.is_empty() {
-        info!(stdout = %stdout.trim_end(), "Command stdout");
+/// Run one external command through Renderflow's canonical bounded process executor.
+///
+/// `program` is executed directly with the provided argv. If the caller
+/// explicitly names a shell program and supplies a shell-evaluation flag such as
+/// `-c`, the request is classified as an explicit shell invocation so the wider
+/// trust boundary remains visible in process diagnostics.
+pub fn run_command(program: &str, args: &[&str]) -> Result<()> {
+    let owned_args: Vec<String> = args.iter().map(|value| (*value).to_string()).collect();
+    let request = if is_explicit_shell_invocation(program, &owned_args) {
+        ProcessRequest::shell(program)
+    } else {
+        ProcessRequest::direct(program)
     }
+    .args(owned_args)
+    .timeout(DEFAULT_PROCESS_TIMEOUT)
+    .capture_limit(DEFAULT_CAPTURE_LIMIT_BYTES);
 
-    if !stderr.is_empty() {
-        if output.status.success() {
-            info!(stderr = %stderr.trim_end(), "Command stderr");
-        } else {
-            error!(stderr = %stderr.trim_end(), "Command stderr");
-        }
+    let result = ProcessExecutor::new().execute(request)?;
+
+    if !result.stdout().redacted_text().is_empty() {
+        info!(
+            stdout = %result.stdout().redacted_text().trim_end(),
+            truncated = result.stdout().truncated(),
+            "Command stdout"
+        );
     }
 
-    if !output.status.success() {
-        let stderr_hint = if stderr.trim().is_empty() {
-            String::new()
+    if !result.stderr().redacted_text().is_empty() {
+        if result.is_success() {
+            info!(
+                stderr = %result.stderr().redacted_text().trim_end(),
+                truncated = result.stderr().truncated(),
+                "Command stderr"
+            );
         } else {
-            format!("\nStderr: {}", stderr.trim_end())
-        };
-        match output.status.code() {
-            Some(code) => {
-                error!(program = program, exit_code = code, "Command failed");
-                bail!(
-                    "Command `{}` failed with exit code {}{}",
-                    program,
-                    code,
-                    stderr_hint
-                );
-            }
-            None => {
-                error!(program = program, "Command terminated by signal");
-                bail!(
-                    "Command `{}` was terminated by a signal{}",
-                    program,
-                    stderr_hint
-                );
-            }
+            error!(
+                stderr = %result.stderr().redacted_text().trim_end(),
+                truncated = result.stderr().truncated(),
+                "Command stderr"
+            );
         }
     }
 
-    info!(program = program, "Command completed successfully");
+    result.ensure_success()?;
+    info!(program = program, duration_ms = result.duration_ms(), "Command completed successfully");
     Ok(())
 }
 
@@ -79,13 +70,14 @@ mod tests {
         assert!(result.is_ok(), "echo with multiple args should succeed");
     }
 
+    #[cfg(unix)]
     #[test]
     fn test_failure() {
         let result = run_command("false", &[]);
         assert!(result.is_err(), "false should fail");
         let err = result.unwrap_err().to_string();
         assert!(
-            err.contains("failed with exit code"),
+            err.contains("exited with code"),
             "error message should mention exit code"
         );
     }
@@ -105,10 +97,10 @@ mod tests {
         );
     }
 
+    #[cfg(unix)]
     #[test]
     fn test_failure_error_includes_stderr() {
-        // `sh -c` lets us write to stderr and exit non-zero in a portable way.
-        let result = run_command("sh", &["-c", "echo 'some error output' >&2; exit 1"]);
+        let result = run_command("sh", &["-c", "printf '%s' 'some error output' >&2; exit 1"]);
         assert!(result.is_err(), "command should fail");
         let err = result.unwrap_err().to_string();
         assert!(

crates/renderflow-core/src/commands/plugin.rs

@@ -1,18 +1,13 @@
 use anyhow::Result;
-use std::process::Command;
 
+use crate::process::ProcessExecutor;
 use crate::transforms::plugin::{PluginCapabilities, PluginMetadata, PluginRegistry};
+
 // ── list ──────────────────────────────────────────────────────────────────────
 
 /// Run `renderflow plugin list`.
 ///
-/// Prints a summary table of every plugin in `registry`:
-///
-/// ```text
-/// Registered plugins (2):
-///   upper       1.0.0   A test plugin
-///   lower       0.5.0   Converts text to lowercase
-/// ```
+/// Prints a summary table of every plugin in `registry`.
 pub fn run_list(registry: &PluginRegistry) -> Result<()> {
     let mut names: Vec<&str> = registry.plugin_names();
     names.sort();
@@ -40,9 +35,6 @@ pub fn run_list(registry: &PluginRegistry) -> Result<()> {
 // ── info ──────────────────────────────────────────────────────────────────────
 
 /// Run `renderflow plugin info <name>`.
-///
-/// Prints the full [`PluginMetadata`] for the named plugin, or returns an
-/// error when the plugin is not registered.
 pub fn run_info(registry: &PluginRegistry, name: &str) -> Result<()> {
     let info = registry
         .plugin_info(name)
@@ -109,9 +101,6 @@ fn print_capabilities(caps: &PluginCapabilities) {
 // ── validate ─────────────────────────────────────────────────────────────────
 
 /// Run `renderflow plugin validate`.
-///
-/// Validates all plugin metadata in `registry` and prints a report.
-/// Returns an error when any validation issue is found.
 pub fn run_validate(registry: &PluginRegistry) -> Result<()> {
     let issues = registry.validate_all();
 
@@ -132,12 +121,8 @@ pub fn run_validate(registry: &PluginRegistry) -> Result<()> {
 
 /// Run `renderflow plugin doctor`.
 ///
-/// Runs diagnostics on every registered plugin:
-/// * Validates metadata.
-/// * Checks that required external tools are present on `PATH`.
-///
-/// Prints a report and returns `Ok(())` even when issues are found (so the
-/// caller can decide whether to treat the output as advisory or fatal).
+/// Validates metadata and probes required external tools through Renderflow's
+/// canonical bounded process executor.
 pub fn run_doctor(registry: &PluginRegistry) -> Result<()> {
     let mut names: Vec<&str> = registry.plugin_names();
     names.sort();
@@ -154,13 +139,11 @@ pub fn run_doctor(registry: &PluginRegistry) -> Result<()> {
     for name in &names {
         let mut issues: Vec<String> = Vec::new();
 
-        // 1. Metadata validation.
         if let Some(meta) = registry.metadata(name) {
             if let Err(e) = meta.validate() {
                 issues.push(format!("invalid metadata: {}", e));
             }
 
-            // 2. Required tools check.
             for tool in &meta.required_tools {
                 if !tool_is_available(tool) {
                     issues.push(format!("required tool '{}' was not found on PATH", tool));
@@ -188,13 +171,9 @@ pub fn run_doctor(registry: &PluginRegistry) -> Result<()> {
     Ok(())
 }
 
-/// Return `true` when `tool` can be found on the system `PATH`.
+/// Return `true` when `tool` can be found and successfully version-probed.
 fn tool_is_available(tool: &str) -> bool {
-    Command::new(tool)
-        .arg("--version")
-        .output()
-        .map(|o| o.status.success())
-        .unwrap_or(false)
+    ProcessExecutor::new().probe_version(tool).is_available()
 }
 
 #[cfg(test)]
@@ -220,8 +199,6 @@ mod tests {
             .with_author("Tester")
     }
 
-    // ── run_list ──────────────────────────────────────────────────────────────
-
     #[test]
     fn test_list_empty_registry_succeeds() {
         let registry = PluginRegistry::new();
@@ -244,8 +221,6 @@ mod tests {
         assert!(run_list(&registry).is_ok());
     }
 
-    // ── run_info ──────────────────────────────────────────────────────────────
-
     #[test]
     fn test_info_known_plugin_succeeds() {
         let mut registry = PluginRegistry::new();
@@ -270,8 +245,6 @@ mod tests {
         assert!(run_info(&registry, "bare").is_ok());
     }
 
-    // ── run_validate ──────────────────────────────────────────────────────────
-
     #[test]
     fn test_validate_empty_registry_succeeds() {
         let registry = PluginRegistry::new();
@@ -287,8 +260,6 @@ mod tests {
         assert!(run_validate(&registry).is_ok());
     }
 
-    // ── run_doctor ────────────────────────────────────────────────────────────
-
     #[test]
     fn test_doctor_empty_registry_succeeds() {
         let registry = PluginRegistry::new();
@@ -312,12 +283,9 @@ mod tests {
         registry
             .register_with_metadata(Arc::new(DummyPlugin("needs-tool")), meta)
             .unwrap();
-        // doctor returns Ok even when tools are missing (advisory output only)
         assert!(run_doctor(&registry).is_ok());
     }
 
-    // ── tool_is_available ─────────────────────────────────────────────────────
-
     #[test]
     fn test_tool_is_available_returns_false_for_nonexistent() {
         assert!(!tool_is_available("__renderflow_nonexistent_xyz__"));

crates/renderflow-core/src/commands/system.rs

@@ -1,5 +1,7 @@
 use anyhow::{bail, Result};
-use std::{env, path::PathBuf, process::Command};
+use std::{env, path::PathBuf};
+
+use crate::process::{ProcessExecutor, ToolProbeStatus};
 
 struct ToolCheck {
     name: &'static str,
@@ -24,23 +26,19 @@ const TOOL_CHECKS: [ToolCheck; 3] = [
 ];
 
 fn probe_tool_version(name: &str) -> Result<String, String> {
-    Command::new(name)
-        .arg("--version")
-        .output()
-        .map_err(|_| format!("missing ({name} not found in PATH)"))
-        .and_then(|output| {
-            if output.status.success() {
-                let stdout = String::from_utf8_lossy(&output.stdout);
-                let stderr = String::from_utf8_lossy(&output.stderr);
-                let line = stdout.lines().next().or_else(|| stderr.lines().next());
-                Ok(line.unwrap_or("available").trim().to_string())
-            } else {
-                Err(format!(
-                    "installed but failed to execute ({name} --version)"
-                ))
-            }
-        })
+    let probe = ProcessExecutor::new().probe_version(name);
+    match probe.status {
+        ToolProbeStatus::Available => Ok(probe
+            .version_line
+            .unwrap_or_else(|| "available".to_string())),
+        ToolProbeStatus::Missing => Err(format!("missing ({name} not found in PATH)")),
+        ToolProbeStatus::TimedOut => Err(format!("installed but version probe timed out ({name} --version)")),
+        ToolProbeStatus::Failed => Err(probe
+            .diagnostic
+            .unwrap_or_else(|| format!("installed but failed to execute ({name} --version)"))),
+    }
 }
+
 pub fn run_version() {
     println!("renderflow {}", env!("CARGO_PKG_VERSION"));
 }

crates/renderflow-core/src/deps.rs

@@ -1,15 +1,12 @@
 use anyhow::Result;
-use std::process::Command;
 
 use crate::error::RenderError;
+use crate::process::ProcessExecutor;
 
-/// Check whether a tool is available in the system PATH.
+/// Check whether a tool is available in the system PATH using the canonical
+/// bounded version-probe path.
 fn tool_available(name: &str) -> bool {
-    Command::new(name)
-        .arg("--version")
-        .output()
-        .map(|o| o.status.success())
-        .unwrap_or(false)
+    ProcessExecutor::new().probe_version(name).is_available()
 }
 
 /// Verify that `pandoc` is installed and available in PATH.
@@ -113,7 +110,7 @@ mod tests {
     fn test_check_pandoc_error_contains_install_hint() {
         let result = check_pandoc();
         if pandoc_available() {
-            return; // nothing to assert
+            return;
         }
         let msg = result.unwrap_err().to_string();
         assert!(
@@ -158,8 +155,6 @@ mod tests {
 
     #[test]
     fn test_validate_dependencies_without_pdf_only_checks_pandoc() {
-        // When pdf_requested is false, tectonic is not checked.
-        // We can only verify the outcome matches what pandoc availability predicts.
         let result = validate_dependencies(false);
         if pandoc_available() {
             assert!(
@@ -194,7 +189,6 @@ mod tests {
 
     #[test]
     fn test_tool_available_with_known_tool() {
-        // `cargo` is always available in a Rust build environment and supports `--version`.
         assert!(
             tool_available("cargo"),
             "cargo should always be available in a Rust build environment"

crates/renderflow-core/src/lib.rs

@@ -24,6 +24,7 @@ mod incremental;
 mod input_format;
 pub mod optimization;
 mod pipeline;
+pub mod process;
 mod sdk;
 pub mod strategies;
 mod template;