Skip to main content

spar/
proc.rs

1//! Running external commands.
2//!
3//! Two things here are load bearing. First, both output streams are captured on
4//! their own threads: an agent CLI can emit megabytes of JSONL, and reading one
5//! pipe to completion before the other deadlocks as soon as the unread pipe
6//! fills. Second, a failure message shows *both* streams, because one agent CLI
7//! reports fatal conditions on stdout with stderr empty, and another writes
8//! routine chatter to stderr on every run.
9
10use std::io::{Read, Write};
11use std::path::{Path, PathBuf};
12use std::process::{Command, Stdio};
13use std::sync::atomic::{AtomicBool, Ordering};
14use std::sync::{Arc, Mutex};
15use std::time::{Duration, Instant};
16
17use crate::error::{Result, SparError};
18
19/// How long one agent call may run before it is killed.
20///
21/// An hour, because thirty minutes was not enough: a review at the highest
22/// effort setting on a large repository, running the test suite as it went,
23/// ran past it. A timeout costs the whole call, so the default errs long and
24/// `timeout` on an agent shortens it.
25pub const DEFAULT_TIMEOUT_SECS: u64 = 3600;
26
27/// Thirty minutes was not enough for a deep review of a large repository, and a
28/// timeout costs the whole call. Checked at compile time rather than in a test,
29/// since shortening it is a decision worth stopping the build for.
30const _: () = assert!(DEFAULT_TIMEOUT_SECS >= 3600);
31
32#[derive(Debug, Clone)]
33pub struct ExecOpts {
34    pub cwd: Option<PathBuf>,
35    pub timeout: Duration,
36    /// When false, a non-zero exit returns stdout instead of an error.
37    pub check: bool,
38    pub env: Vec<(String, String)>,
39    pub stdin: Option<String>,
40}
41
42impl Default for ExecOpts {
43    fn default() -> Self {
44        Self {
45            cwd: None,
46            timeout: Duration::from_secs(DEFAULT_TIMEOUT_SECS),
47            check: true,
48            env: Vec::new(),
49            stdin: None,
50        }
51    }
52}
53
54impl ExecOpts {
55    pub fn new() -> Self {
56        Self::default()
57    }
58
59    pub fn cwd(mut self, path: impl AsRef<Path>) -> Self {
60        self.cwd = Some(path.as_ref().to_path_buf());
61        self
62    }
63
64    pub fn cwd_opt(mut self, path: Option<PathBuf>) -> Self {
65        self.cwd = path;
66        self
67    }
68
69    pub fn timeout_secs(mut self, secs: u64) -> Self {
70        self.timeout = Duration::from_secs(secs);
71        self
72    }
73
74    pub fn check(mut self, value: bool) -> Self {
75        self.check = value;
76        self
77    }
78
79    pub fn env(mut self, key: impl Into<String>, value: impl Into<String>) -> Self {
80        self.env.push((key.into(), value.into()));
81        self
82    }
83
84    pub fn stdin(mut self, text: impl Into<String>) -> Self {
85        self.stdin = Some(text.into());
86        self
87    }
88}
89
90#[derive(Debug, Clone)]
91pub struct Output {
92    pub stdout: String,
93    pub stderr: String,
94    pub code: i32,
95}
96
97impl Output {
98    pub fn ok(&self) -> bool {
99        self.code == 0
100    }
101}
102
103/// Run a command to completion, capturing both streams.
104///
105/// Only spawn failures and timeouts are errors here; a non-zero exit is a
106/// normal result that the caller decides how to treat.
107pub fn exec(argv: &[String], opts: &ExecOpts) -> Result<Output> {
108    let program = argv
109        .first()
110        .ok_or_else(|| SparError::new("cannot run an empty command"))?;
111
112    let mut command = Command::new(program);
113    command
114        .args(&argv[1..])
115        .stdout(Stdio::piped())
116        .stderr(Stdio::piped());
117
118    if opts.stdin.is_some() {
119        command.stdin(Stdio::piped());
120    } else {
121        // Agent CLIs happily block forever waiting on an inherited terminal.
122        command.stdin(Stdio::null());
123    }
124
125    if let Some(dir) = &opts.cwd {
126        command.current_dir(dir);
127    }
128    for (key, value) in &opts.env {
129        command.env(key, value);
130    }
131
132    let mut child = command
133        .spawn()
134        .map_err(|e| SparError::new(format!("could not run `{}`: {e}", abbreviate(argv))))?;
135
136    if let Some(text) = &opts.stdin {
137        if let Some(mut pipe) = child.stdin.take() {
138            let _ = pipe.write_all(text.as_bytes());
139        }
140        // Dropping the handle closes the pipe, which the child needs in order
141        // to see EOF and exit.
142    }
143
144    let out_reader = Reader::spawn(child.stdout.take().expect("stdout piped"));
145    let err_reader = Reader::spawn(child.stderr.take().expect("stderr piped"));
146
147    let deadline = Instant::now() + opts.timeout;
148    let mut poll = Duration::from_millis(5);
149    let mut timed_out = false;
150    let status = loop {
151        match child.try_wait()? {
152            Some(status) => break Some(status),
153            None => {
154                if Instant::now() >= deadline {
155                    let _ = child.kill();
156                    let _ = child.wait();
157                    timed_out = true;
158                    break None;
159                }
160                std::thread::sleep(poll);
161                // Back off so a long agent run is not a busy loop, but stay
162                // responsive for the many fast git and gh calls.
163                poll = (poll * 2).min(Duration::from_millis(100));
164            }
165        }
166    };
167
168    // Never join the readers.
169    //
170    // An agent CLI runs shell commands, and any grandchild that inherited the
171    // pipe keeps its write end open after its parent exits. `read_to_end` on
172    // such a pipe never returns, so joining would hang past the deadline and
173    // the timeout would bound nothing at all. Instead, wait for the readers to
174    // finish or for the output to stop arriving, then take what did.
175    let stdout = out_reader.collect(DRAIN_GRACE);
176    let stderr = err_reader.collect(DRAIN_GRACE);
177
178    if timed_out {
179        return Err(SparError::timed_out(format!(
180            "timed out after {}s: {}\nRaise `timeout` on this agent in spar.toml if the model \
181             legitimately needs longer. Not retried: asking again would wait exactly as long a \
182             second time.",
183            opts.timeout.as_secs(),
184            abbreviate(argv)
185        )));
186    }
187
188    Ok(Output {
189        stdout: String::from_utf8_lossy(&stdout).into_owned(),
190        stderr: String::from_utf8_lossy(&stderr).into_owned(),
191        code: status.and_then(|s| s.code()).unwrap_or(-1),
192    })
193}
194
195/// How long to keep waiting for output after the child has exited, when a
196/// surviving grandchild is holding the pipe open. Measured from the last byte
197/// received, so a slow large read is never cut short.
198const DRAIN_GRACE: Duration = Duration::from_secs(3);
199
200/// A pipe drained on its own thread into a shared buffer.
201///
202/// Reading incrementally rather than with `read_to_end` is what lets the caller
203/// take the output without joining, which is what keeps the timeout honest.
204struct Reader {
205    buf: Arc<Mutex<Vec<u8>>>,
206    done: Arc<AtomicBool>,
207}
208
209impl Reader {
210    fn spawn<R: Read + Send + 'static>(mut pipe: R) -> Self {
211        let buf = Arc::new(Mutex::new(Vec::new()));
212        let done = Arc::new(AtomicBool::new(false));
213        let (buf_w, done_w) = (Arc::clone(&buf), Arc::clone(&done));
214        std::thread::spawn(move || {
215            let mut chunk = [0u8; 16 * 1024];
216            loop {
217                match pipe.read(&mut chunk) {
218                    Ok(0) | Err(_) => break,
219                    Ok(n) => buf_w
220                        .lock()
221                        .unwrap_or_else(|e| e.into_inner())
222                        .extend_from_slice(&chunk[..n]),
223                }
224            }
225            done_w.store(true, Ordering::Release);
226        });
227        Self { buf, done }
228    }
229
230    fn len(&self) -> usize {
231        self.buf.lock().unwrap_or_else(|e| e.into_inner()).len()
232    }
233
234    /// Everything received once the reader finishes, or once `grace` passes
235    /// with no new bytes.
236    ///
237    /// The poll starts fine grained and backs off. A run makes hundreds of git
238    /// and gh calls whose pipes are already at EOF by the time the child is
239    /// reaped, and a flat ten millisecond wait on each one is real wall clock
240    /// spent on nothing.
241    fn collect(&self, grace: Duration) -> Vec<u8> {
242        let mut last_len = self.len();
243        let mut quiet_since = Instant::now();
244        let mut poll = Duration::from_micros(100);
245        while !self.done.load(Ordering::Acquire) {
246            let now_len = self.len();
247            if now_len != last_len {
248                last_len = now_len;
249                quiet_since = Instant::now();
250            } else if quiet_since.elapsed() >= grace {
251                break;
252            }
253            std::thread::sleep(poll);
254            poll = (poll * 2).min(Duration::from_millis(10));
255        }
256        self.buf.lock().unwrap_or_else(|e| e.into_inner()).clone()
257    }
258}
259
260/// Run a command and return stdout. With `check` set, a non-zero exit is an
261/// error carrying both streams.
262pub fn run(argv: &[String], opts: &ExecOpts) -> Result<String> {
263    let out = exec(argv, opts)?;
264    if opts.check && !out.ok() {
265        return Err(SparError::call_failed(failure_message(argv, &out)));
266    }
267    Ok(out.stdout)
268}
269
270/// Convenience for the many `run(&["git".into(), ...])` call sites.
271pub fn run_str(argv: &[&str], opts: &ExecOpts) -> Result<String> {
272    let owned: Vec<String> = argv.iter().map(|s| (*s).to_string()).collect();
273    run(&owned, opts)
274}
275
276/// Prompts run to many kilobytes. Echoing them whole buries the actual error.
277pub fn abbreviate(argv: &[String]) -> String {
278    argv.iter()
279        .map(|arg| {
280            let one_line = arg.split_whitespace().collect::<Vec<_>>().join(" ");
281            if one_line.chars().count() <= 60 {
282                one_line
283            } else {
284                let head: String = one_line.chars().take(57).collect();
285                format!("{head}...")
286            }
287        })
288        .collect::<Vec<_>>()
289        .join(" ")
290}
291
292/// Both streams, labelled. Preferring stderr is not enough: one agent CLI
293/// writes chatter like "Reading additional input from stdin..." to stderr on
294/// every run, which outranks the real reason and explains nothing, while
295/// another reports fatal conditions on stdout with stderr empty.
296pub fn failure_message(argv: &[String], out: &Output) -> String {
297    let mut parts = vec![format!(
298        "command failed ({}): {}",
299        out.code,
300        abbreviate(argv)
301    )];
302    for (label, stream) in [("stderr", &out.stderr), ("stdout", &out.stdout)] {
303        let text = stream.trim();
304        if !text.is_empty() {
305            parts.push(format!("--- {label} ---\n{}", tail(text, 1500)));
306        }
307    }
308    if parts.len() == 1 {
309        parts.push("(no output on either stream)".to_string());
310    }
311    parts.join("\n")
312}
313
314/// Last `max` characters, on a character boundary.
315fn tail(text: &str, max: usize) -> &str {
316    let count = text.chars().count();
317    if count <= max {
318        return text;
319    }
320    let start = text
321        .char_indices()
322        .nth(count - max)
323        .map(|(i, _)| i)
324        .unwrap_or(0);
325    &text[start..]
326}
327
328/// Whether a program is on PATH.
329pub fn which(program: &str) -> Option<PathBuf> {
330    if program.contains(std::path::MAIN_SEPARATOR) {
331        let path = PathBuf::from(program);
332        return is_executable(&path).then_some(path);
333    }
334    let paths = std::env::var_os("PATH")?;
335    std::env::split_paths(&paths).find_map(|dir| {
336        let candidate = dir.join(program);
337        is_executable(&candidate).then_some(candidate)
338    })
339}
340
341pub fn is_executable(path: &Path) -> bool {
342    #[cfg(unix)]
343    {
344        use std::os::unix::fs::PermissionsExt;
345        match std::fs::metadata(path) {
346            Ok(meta) => meta.is_file() && meta.permissions().mode() & 0o111 != 0,
347            Err(_) => false,
348        }
349    }
350    #[cfg(not(unix))]
351    {
352        path.is_file()
353    }
354}
355
356/// Expand a leading `~` against $HOME. Nothing else: a config path is not a
357/// shell word and should not behave like one.
358pub fn expand_tilde(path: &str) -> PathBuf {
359    if path == "~" {
360        if let Some(home) = home_dir() {
361            return home;
362        }
363    }
364    if let Some(rest) = path.strip_prefix("~/") {
365        if let Some(home) = home_dir() {
366            return home.join(rest);
367        }
368    }
369    PathBuf::from(path)
370}
371
372pub fn home_dir() -> Option<PathBuf> {
373    std::env::var_os("HOME").map(PathBuf::from)
374}
375
376#[cfg(test)]
377mod tests {
378    use super::*;
379
380    fn proc(out: &str, err: &str, code: i32) -> Output {
381        Output {
382            stdout: out.into(),
383            stderr: err.into(),
384            code,
385        }
386    }
387
388    fn argv(parts: &[&str]) -> Vec<String> {
389        parts.iter().map(|s| s.to_string()).collect()
390    }
391
392    #[test]
393    fn stdin_reaches_the_child_byte_for_byte() {
394        let body = "line one  \n\nline three\n";
395        let out = run_str(&["/bin/sh", "-c", "cat"], &ExecOpts::new().stdin(body))
396            .expect("the child reads stdin");
397        assert_eq!(body, out);
398    }
399
400    #[test]
401    fn stdout_used_when_stderr_is_empty() {
402        let msg = failure_message(&argv(&["claude"]), &proc("You've hit your limit.", "", 1));
403        assert!(msg.contains("hit your limit"), "{msg}");
404    }
405
406    #[test]
407    fn stderr_shown_when_present() {
408        let msg = failure_message(&argv(&["gh"]), &proc("noise", "real reason", 1));
409        assert!(msg.contains("real reason"), "{msg}");
410    }
411
412    #[test]
413    fn both_streams_are_shown_not_just_one() {
414        let msg = failure_message(&argv(&["gh"]), &proc("on stdout", "on stderr", 1));
415        assert!(
416            msg.contains("on stdout") && msg.contains("on stderr"),
417            "{msg}"
418        );
419    }
420
421    #[test]
422    fn says_something_when_both_are_empty() {
423        assert!(failure_message(&argv(&["x"]), &proc("", "", 2)).contains("no output"));
424    }
425
426    #[test]
427    fn long_arguments_are_abbreviated() {
428        let long = "word ".repeat(500);
429        let out = abbreviate(&argv(&["claude", "-p", &long]));
430        assert!(out.len() < 200, "{}", out.len());
431    }
432
433    #[test]
434    fn newlines_in_arguments_do_not_break_the_line() {
435        assert!(!abbreviate(&argv(&["claude", "a\nb\nc"])).contains('\n'));
436    }
437
438    #[test]
439    fn short_arguments_survive_intact() {
440        assert_eq!(
441            "gh pr merge 17",
442            abbreviate(&argv(&["gh", "pr", "merge", "17"]))
443        );
444    }
445
446    #[test]
447    fn abbreviation_never_splits_a_character() {
448        // A multi-byte argument longer than the cap must not panic on a slice
449        // that lands mid-character.
450        let wide = "\u{1f600}".repeat(200);
451        let out = abbreviate(&argv(&[&wide]));
452        assert!(out.ends_with("..."));
453    }
454
455    #[test]
456    fn exit_code_is_reported() {
457        let out = exec(
458            &argv(&["sh", "-c", "exit 3"]),
459            &ExecOpts::new().check(false),
460        )
461        .unwrap();
462        assert_eq!(3, out.code);
463    }
464
465    #[test]
466    fn check_false_returns_stdout_on_failure() {
467        let text = run(
468            &argv(&["sh", "-c", "echo partial; exit 1"]),
469            &ExecOpts::new().check(false),
470        )
471        .unwrap();
472        assert_eq!("partial\n", text);
473    }
474
475    #[test]
476    fn check_true_fails_loudly() {
477        let err = run(
478            &argv(&["sh", "-c", "echo why >&2; exit 1"]),
479            &ExecOpts::new(),
480        )
481        .unwrap_err();
482        assert!(err.to_string().contains("why"), "{err}");
483    }
484
485    #[test]
486    fn large_output_does_not_deadlock() {
487        // Well past a pipe buffer on every platform spar runs on.
488        let text = run(
489            &argv(&["sh", "-c", "yes hello | head -c 400000"]),
490            &ExecOpts::new().timeout_secs(60),
491        )
492        .unwrap();
493        assert_eq!(400_000, text.len());
494    }
495
496    /// The reason the readers are never joined. A grandchild that inherited
497    /// the pipe holds its write end open after its parent exits, so
498    /// `read_to_end` would never return and the deadline would bound nothing.
499    #[test]
500    fn a_surviving_grandchild_holding_the_pipe_cannot_hang_the_timeout() {
501        let start = Instant::now();
502        let err = run(
503            &argv(&["sh", "-c", "sleep 120 & echo parent-output; sleep 60"]),
504            &ExecOpts::new().timeout_secs(1),
505        )
506        .unwrap_err();
507        let elapsed = start.elapsed();
508
509        assert!(err.to_string().contains("timed out"), "{err}");
510        assert!(
511            elapsed < Duration::from_secs(20),
512            "the timeout did not bound the call: {elapsed:?}"
513        );
514    }
515
516    #[test]
517    fn a_surviving_grandchild_does_not_hang_a_normal_exit_either() {
518        let start = Instant::now();
519        let out = run(
520            &argv(&["sh", "-c", "sleep 120 & echo done"]),
521            &ExecOpts::new().timeout_secs(60),
522        )
523        .unwrap();
524        assert!(out.contains("done"), "{out:?}");
525        assert!(
526            start.elapsed() < Duration::from_secs(20),
527            "waited on a grandchild that will never exit"
528        );
529    }
530
531    #[test]
532    fn timeout_kills_and_explains() {
533        let err = run(
534            &argv(&["sh", "-c", "sleep 30"]),
535            &ExecOpts::new().timeout_secs(1),
536        )
537        .unwrap_err();
538        assert!(err.to_string().contains("timed out"), "{err}");
539    }
540
541    #[test]
542    fn missing_binary_names_the_command() {
543        let err = exec(
544            &argv(&["spar-definitely-not-a-real-binary"]),
545            &ExecOpts::new(),
546        )
547        .unwrap_err();
548        assert!(
549            err.to_string()
550                .contains("spar-definitely-not-a-real-binary"),
551            "{err}"
552        );
553    }
554
555    #[test]
556    fn tilde_expands_against_home() {
557        std::env::set_var("HOME", "/home/someone");
558        assert_eq!(PathBuf::from("/home/someone/bin"), expand_tilde("~/bin"));
559        assert_eq!(PathBuf::from("/absolute"), expand_tilde("/absolute"));
560        assert_eq!(PathBuf::from("~notauser/x"), expand_tilde("~notauser/x"));
561    }
562}
563
564#[cfg(test)]
565mod timeout_kind_tests {
566    use super::*;
567    use crate::error::ErrorKind;
568
569    /// A deadline is not a bad answer. Retrying one buys another wait of
570    /// exactly the same length, which on a review at the highest effort
571    /// setting turned a thirty minute failure into an hour of it.
572    #[test]
573    fn a_timeout_is_marked_as_one_and_is_not_worth_retrying() {
574        let err = run(
575            &["sh".to_string(), "-c".to_string(), "sleep 30".to_string()],
576            &ExecOpts::new().timeout_secs(1),
577        )
578        .unwrap_err();
579
580        assert_eq!(ErrorKind::TimedOut, err.kind());
581        assert!(!err.worth_retrying());
582        assert!(err.to_string().contains("Not retried"), "{err}");
583    }
584
585    /// A non-zero exit is the CLI reporting that it could not answer, which is
586    /// a different thing from an answer that arrived and could not be parsed.
587    /// Only the second is what the retry exists for.
588    #[test]
589    fn a_non_zero_exit_is_the_call_failing_rather_than_the_answer() {
590        let err = run(
591            &["sh".to_string(), "-c".to_string(), "exit 1".to_string()],
592            &ExecOpts::new(),
593        )
594        .unwrap_err();
595
596        assert_eq!(ErrorKind::CallFailed, err.kind());
597        // Not hopeless in the abstract, since it could have been transient.
598        // Whether to spend a second call on it is `Agent`'s decision, and it
599        // turns on whether there is a stand in to send the call to instead.
600        assert!(err.worth_retrying());
601    }
602}