Migrate subprocess spawns to running-process + regression linter

.github/workflows/check-ubuntu.yml

@@ -29,5 +29,7 @@ jobs:
         run: soldr cargo check --workspace --all-targets
       - name: Clippy
         run: soldr cargo clippy --workspace --all-targets -- -D warnings
+      - name: Lint subprocess spawns
+        run: uv run python ci/find_direct_subprocess.py --fail
       - name: Test
         run: soldr cargo test --workspace

.github/workflows/lint-subprocess.yml

@@ -0,0 +1,28 @@
+name: Lint subprocess spawns
+
+# Regression guard for FastLED/fbuild#141: every subprocess fbuild
+# starts must flow through the fbuild-core::subprocess wrappers (which
+# are backed by running-process-core). Direct std::process::Command /
+# tokio::process::Command spawns are only allowed when annotated with
+# an // allow-direct-spawn: <reason> marker.
+#
+# Keeps pipe-deadlock and containment-drift bugs from creeping back in.
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+
+env:
+  FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true"
+
+jobs:
+  lint-subprocess:
+    name: Lint subprocess spawns
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v6
+      - uses: astral-sh/setup-uv@v3
+      - name: Check no unannotated direct Command::new spawns
+        run: uv run python ci/find_direct_subprocess.py --fail

ci/find_direct_subprocess.py

@@ -2,26 +2,38 @@
 # /// script
 # requires-python = ">=3.10"
 # ///
-"""Find direct std::process / tokio::process spawn sites in fbuild crates.
+"""Linter: forbid direct `std::process::Command::new` / `tokio::process::Command::new`
+outside of documented hold-outs.
 
-Goal: every subprocess that fbuild starts should go through
-`running-process` (via the wrappers in `fbuild-core::subprocess` and
-`fbuild-core::containment`). This script enumerates every remaining
-direct `Command::new(...)` site so they can be migrated and eliminated
-one PR at a time.
+Every subprocess fbuild starts must go through the
+`fbuild-core::subprocess` wrappers (which are themselves backed by
+[`running-process`](https://github.com/zackees/running-process) so
+that containment, concurrent pipe draining, and Windows-specific env
+handling are implemented once and cannot drift.
+
+Tracked by FastLED/fbuild#141.
 
 A site is allowlisted by placing this marker on the same line or on
 the line immediately before the `Command::new(`:
 
     // allow-direct-spawn: <one-line reason>
 
-When this script reports zero non-allowlisted sites across the whole
-workspace, delete it (and the marker comments) and rely on
-`running-process` exclusively. Tracked by FastLED/fbuild#<issue>.
+Intentional hold-outs currently allowed:
+- Daemon spawns from CLI/Python/tests (daemon must outlive parent).
+- zccache daemon bootstrap (independent lifecycle).
+- containment module's own regression tests.
+- Integration test harnesses that spawn binaries under test.
+- tokio async streaming emulator handlers (QEMU, avr8js/node) where
+  NativeProcess's blocking API is unsuitable.
+- tokio parallel async fan-out in the CLI (IWYU, clang-tidy) inside a
+  process that has no daemon containment group.
+
+Run in CI with `--fail` so any new direct spawn without a marker
+breaks the build.
 
 Usage:
     uv run python ci/find_direct_subprocess.py            # report
-    uv run python ci/find_direct_subprocess.py --fail     # exit 1 if >0
+    uv run python ci/find_direct_subprocess.py --fail     # exit 1 if any
     uv run python ci/find_direct_subprocess.py --json     # machine output
 """
 
@@ -125,7 +137,13 @@ def render_text(hits: list[Hit]) -> str:
     lines.append(f"  allowlisted: {len(allowed)}")
     if pending:
         lines.append("")
-        lines.append("To migrate (no allow-direct-spawn marker):")
+        lines.append(
+            "NEW direct spawns without an `allow-direct-spawn: <reason>` marker:"
+        )
+        lines.append(
+            "  (route via fbuild_core::subprocess::{run_command,run_command_passthrough}"
+        )
+        lines.append("   or annotate with a one-line reason — see FastLED/fbuild#141)")
         for h in pending:
             rel = h.path.relative_to(REPO_ROOT)
             lines.append(f"  {rel}:{h.line_no}: {h.text.strip()}")

crates/fbuild-build/src/compiler.rs

@@ -417,13 +417,10 @@ fn compiler_identity(path: &Path) -> String {
 }
 
 fn compiler_version(path: &Path) -> String {
-    match std::process::Command::new(path)
-        .arg("-dumpversion")
-        .output()
-    {
-        Ok(output) if output.status.success() => {
-            String::from_utf8_lossy(&output.stdout).trim().to_string()
-        }
+    let program = path.to_string_lossy();
+    let args = [program.as_ref(), "-dumpversion"];
+    match fbuild_core::subprocess::run_command(&args, None, None, None) {
+        Ok(output) if output.success() => output.stdout.trim().to_string(),
         _ => String::new(),
     }
 }

crates/fbuild-build/src/script_runtime.rs

@@ -7,7 +7,6 @@
 
 use std::collections::HashMap;
 use std::path::Path;
-use std::process::Command;
 
 use crate::flag_overlay::{
     absolutize_if_relative, values_to_args, BuildOverlay, LanguageExtraFlags, LinkExtraFlags,
@@ -89,47 +88,37 @@ pub fn resolve_extra_script_overlay(
         ))
     })?;
 
-    let mut command = Command::new(&python[0]);
-    if python.len() > 1 {
-        command.args(&python[1..]);
-    }
-    command
-        .arg(&harness_path)
-        .arg(&input_path)
-        .current_dir(project_dir)
-        .stdin(std::process::Stdio::null())
-        .stdout(std::process::Stdio::piped())
-        .stderr(std::process::Stdio::piped());
     // Route the spawn through the daemon's containment group so a
     // daemon crash mid-evaluation doesn't leave a python child running
-    // in the background. See FastLED/fbuild#32.
-    let child = fbuild_core::containment::spawn_contained(&mut command).map_err(|e| {
-        fbuild_core::FbuildError::BuildFailed(format!(
-            "failed to run extra_scripts runtime via '{}': {}",
-            python.join(" "),
-            e
-        ))
-    })?;
-    let output = child.wait_with_output().map_err(|e| {
-        fbuild_core::FbuildError::BuildFailed(format!(
-            "failed to collect extra_scripts runtime output: {}",
-            e
-        ))
-    })?;
+    // in the background. See FastLED/fbuild#32. Uses fbuild_core's
+    // NativeProcess-backed runner to drain stdout/stderr concurrently.
+    let harness_path_str = harness_path.to_string_lossy();
+    let input_path_str = input_path.to_string_lossy();
+    let mut argv: Vec<&str> = python.iter().map(|s| s.as_str()).collect();
+    argv.push(harness_path_str.as_ref());
+    argv.push(input_path_str.as_ref());
+    let output = fbuild_core::subprocess::run_command(&argv, Some(project_dir), None, None)
+        .map_err(|e| {
+            fbuild_core::FbuildError::BuildFailed(format!(
+                "failed to run extra_scripts runtime via '{}': {}",
+                python.join(" "),
+                e
+            ))
+        })?;
 
-    if !output.status.success() {
-        let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
+    if !output.success() {
+        let stderr = output.stderr.trim();
         return Err(fbuild_core::FbuildError::BuildFailed(format!(
             "extra_scripts runtime failed: {}\nRecommendation: use --platformio for this project.",
             if stderr.is_empty() {
-                format!("process exited with status {}", output.status)
+                format!("process exited with status {}", output.exit_code)
             } else {
-                stderr
+                stderr.to_string()
             }
         )));
     }
 
-    let runtime: ScriptRuntimeResult = serde_json::from_slice(&output.stdout).map_err(|e| {
+    let runtime: ScriptRuntimeResult = serde_json::from_str(&output.stdout).map_err(|e| {
         fbuild_core::FbuildError::BuildFailed(format!(
             "failed to parse extra_scripts runtime output: {}",
             e
@@ -288,12 +277,10 @@ fn find_python() -> Option<Vec<String>> {
     };
 
     for candidate in candidates {
-        let mut command = Command::new(candidate[0]);
-        if candidate.len() > 1 {
-            command.args(&candidate[1..]);
-        }
-        if let Ok(output) = command.arg("--version").output() {
-            if output.status.success() {
+        let mut argv: Vec<&str> = candidate.to_vec();
+        argv.push("--version");
+        if let Ok(output) = fbuild_core::subprocess::run_command(&argv, None, None, None) {
+            if output.success() {
                 return Some(candidate.iter().map(|s| (*s).to_string()).collect());
             }
         }

crates/fbuild-build/src/zccache.rs

@@ -132,6 +132,7 @@ pub fn ensure_running(zccache: &Path) {
     // is a no-op when it's already running, and either way the zccache
     // daemon must survive the fbuild daemon — so this spawn stays out
     // of the containment group.
+    // allow-direct-spawn: zccache daemon must outlive the fbuild daemon.
     let mut cmd = std::process::Command::new(zccache);
     cmd.arg("start")
         .stdout(std::process::Stdio::null())

crates/fbuild-build/tests/zccache_hit_across_workspace_rename.rs

@@ -263,6 +263,7 @@ fn compile_fake_zccache(root: &Path) -> PathBuf {
     fs::write(&source, FAKE_ZCCACHE).unwrap();
 
     let rustc = env::var_os("RUSTC").unwrap_or_else(|| "rustc".into());
+    // allow-direct-spawn: test helper compiling a throwaway rustc binary.
     let status = Command::new(rustc)
         .arg(&source)
         .arg("-o")

crates/fbuild-cli/src/daemon_client.rs

@@ -858,6 +858,7 @@ pub async fn ensure_daemon_running() -> fbuild_core::Result<()> {
 /// Spawn a single daemon process instance.
 async fn spawn_daemon_process() -> fbuild_core::Result<()> {
     let daemon_exe = "fbuild-daemon";
+    // allow-direct-spawn: daemon must outlive the CLI; see INTENTIONALLY DETACHED comment below.
     let mut cmd = tokio::process::Command::new(daemon_exe);
 
     if fbuild_paths::is_dev_mode() {