From 93817643ee6e46db16128bd3a0aa40d74aa7e484 Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Fri, 9 May 2025 11:03:58 -0700
Subject: [PATCH] chore: refactor exec() into spawn_child() and
 consume_truncated_output() (#878)

This PR is a straight refactor so that creating the `Child` process for
an `shell` tool call and consuming its output can be separate concerns.
For the actual tool call, we will always apply
`consume_truncated_output()`, but for the top-level debug commands in
the CLI (e.g., `debug seatbelt` and `debug landlock`), we only want to
use the `spawn_child()` part of `exec()`.

We want the subcommands to match the `shell` tool call usage as
faithfully as possible. This becomes more important when we introduce a
new parameter to `spawn_child()` in
https://github.com/openai/codex/pull/879.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/878).
* #879
* __->__ #878
---
 codex-rs/core/src/exec.rs | 55 +++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs
index d1939d2c..aa761d2e 100644
--- a/codex-rs/core/src/exec.rs
+++ b/codex-rs/core/src/exec.rs
@@ -12,6 +12,7 @@ use std::time::Instant;
 use tokio::io::AsyncRead;
 use tokio::io::AsyncReadExt;
 use tokio::io::BufReader;
+use tokio::process::Child;
 use tokio::process::Command;
 use tokio::sync::Notify;
 
@@ -236,32 +237,42 @@ pub async fn exec(
     }: ExecParams,
     ctrl_c: Arc<Notify>,
 ) -> Result<RawExecToolCallOutput> {
-    let mut child = {
-        if command.is_empty() {
-            return Err(CodexErr::Io(io::Error::new(
-                io::ErrorKind::InvalidInput,
-                "command args are empty",
-            )));
-        }
+    let child = spawn_child(command, cwd).await?;
+    consume_truncated_output(child, ctrl_c, timeout_ms).await
+}
 
-        let mut cmd = Command::new(&command[0]);
-        if command.len() > 1 {
-            cmd.args(&command[1..]);
-        }
-        cmd.current_dir(cwd);
+/// Spawns the appropriate child process for the ExecParams.
+async fn spawn_child(command: Vec<String>, cwd: PathBuf) -> std::io::Result<Child> {
+    if command.is_empty() {
+        return Err(std::io::Error::new(
+            io::ErrorKind::InvalidInput,
+            "command args are empty",
+        ));
+    }
 
-        // Do not create a file descriptor for stdin because otherwise some
-        // commands may hang forever waiting for input. For example, ripgrep has
-        // a heuristic where it may try to read from stdin as explained here:
-        // https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103
-        cmd.stdin(Stdio::null());
+    let mut cmd = Command::new(&command[0]);
+    cmd.args(&command[1..]);
+    cmd.current_dir(cwd);
 
-        cmd.stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .kill_on_drop(true)
-            .spawn()?
-    };
+    // Do not create a file descriptor for stdin because otherwise some
+    // commands may hang forever waiting for input. For example, ripgrep has
+    // a heuristic where it may try to read from stdin as explained here:
+    // https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103
+    cmd.stdin(Stdio::null());
 
+    cmd.stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .kill_on_drop(true)
+        .spawn()
+}
+
+/// Consumes the output of a child process, truncating it so it is suitable for
+/// use as the output of a `shell` tool call. Also enforces specified timeout.
+async fn consume_truncated_output(
+    mut child: Child,
+    ctrl_c: Arc<Notify>,
+    timeout_ms: Option<u64>,
+) -> Result<RawExecToolCallOutput> {
     let stdout_handle = tokio::spawn(read_capped(
         BufReader::new(child.stdout.take().expect("stdout is not piped")),
         MAX_STREAM_OUTPUT,