agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Guard core test subprocess cleanup (#27343)

Browse Source 
## Why

Local integration-heavy `codex-core` CLI tests can time out or be
interrupted after spawning `codex exec`. Stopping only the direct child
is not enough: `codex exec` can leave grandchildren behind, including
`python3`/`python3.12` processes that get reparented to PID 1 and keep
running after the test is gone.

This PR fixes that failure mode directly for the affected CLI
integration tests, without changing production code or reducing local
test concurrency.

## What

- Run the `cli_stream` `codex exec` subprocesses through a small private
wrapper in `core/tests/suite/cli_stream.rs`.
- Spawn those subprocesses in their own process group before execution.
- Keep `.output()`-style stdout/stderr capture and the existing
30-second timeout behavior.
- Own each spawned process with a drop guard that kills the whole
process group on success, timeout, panic, or other early return.

The switch from `assert_cmd::Command` to `std::process::Command` is only
for these subprocess launches; `assert_cmd` does not expose a pre-spawn
hook for setting the process group.

## Verification

- `just test -p codex-core --test all responses_mode_stream_cli`

This is limited to core integration tests; it does not change production
`src` code paths.
This commit is contained in:
Eric Traut
2026-06-10 12:19:26 -07:00
committed by GitHub
Unverified
parent 2e377ce5e5
commit 13468115fc
1 changed files with 85 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/tests/suite/cli_stream.rs
+85 -29
View File
@@ -1,4 +1,3 @@
use assert_cmd::Command as AssertCommand;
use codex_git_utils::collect_git_info;
use codex_login::CODEX_ACCESS_TOKEN_ENV_VAR;
use codex_login::CODEX_API_KEY_ENV_VAR;
@@ -7,6 +6,14 @@ use core_test_support::fs_wait;
use core_test_support::responses;
use core_test_support::skip_if_no_network;
use pretty_assertions::assert_eq;
use std::io;
#[cfg(unix)]
use std::os::unix::process::CommandExt as _;
use std::process::Command;
use std::process::Output;
use std::process::Stdio;
use std::sync::mpsc;
use std::thread;
use std::time::Duration;
use tempfile::TempDir;
use uuid::Uuid;
@@ -22,6 +29,7 @@ const PERSONAL_ACCESS_TOKEN_AUTHORIZATION: &str = "Bearer at-cli-test";
const PERSONAL_ACCESS_TOKEN_ACCOUNT_ID: &str = "account-pat";
const WHOAMI_PATH: &str = "/v1/user-auth-credential/whoami";
const CLOUD_CONFIG_BUNDLE_PATH: &str = "/backend-api/wham/config/bundle";
const CLI_TIMEOUT: Duration = Duration::from_secs(30);
fn repo_root() -> std::path::PathBuf {
#[expect(clippy::expect_used)]
@@ -60,10 +68,9 @@ async fn mount_personal_access_token_startup(server: &MockServer) {
}
#[expect(clippy::unwrap_used)]
fn personal_access_token_exec_command(server: &MockServer, home: &TempDir) -> AssertCommand {
fn personal_access_token_exec_command(server: &MockServer, home: &TempDir) -> Command {
let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
let mut cmd = AssertCommand::new(bin);
cmd.timeout(Duration::from_secs(30));
let mut cmd = Command::new(bin);
cmd.arg("exec")
.arg("--skip-git-repo-check")
.arg("-c")
@@ -81,6 +88,62 @@ fn personal_access_token_exec_command(server: &MockServer, home: &TempDir) -> As
cmd
}
struct ChildProcessCleanupGuard(u32);
impl Drop for ChildProcessCleanupGuard {
fn drop(&mut self) {
#[cfg(unix)]
{
let _ = codex_utils_pty::process_group::kill_process_group(self.0);
}
#[cfg(windows)]
{
let _ = Command::new("taskkill")
.args(["/PID", &self.0.to_string(), "/T", "/F"])
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status();
}
#[cfg(not(any(unix, windows)))]
{
let _ = self.0;
}
}
}
// Use this for new `codex exec` subprocess tests in this file. These commands
// can spawn shell/Python grandchildren, so the timeout path must reap the whole
// process group instead of only the direct CLI child.
fn run_cli_command(command: &mut Command) -> io::Result<Output> {
#[cfg(unix)]
command.process_group(0);
command
.stdin(Stdio::null())
.stdout(Stdio::piped())
.stderr(Stdio::piped());
let child = command.spawn()?;
let _cleanup = ChildProcessCleanupGuard(child.id());
let (sender, receiver) = mpsc::sync_channel(1);
let _waiter = thread::spawn(move || {
let _ = sender.send(child.wait_with_output());
});
match receiver.recv_timeout(CLI_TIMEOUT) {
Ok(output) => output,
Err(mpsc::RecvTimeoutError::Timeout) => {
Err(io::Error::new(io::ErrorKind::TimedOut, "process timed out"))
}
Err(mpsc::RecvTimeoutError::Disconnected) => {
Err(io::Error::other("process output reader thread exited"))
}
}
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn responses_mode_stream_cli_supports_personal_access_tokens() {
skip_if_no_network!();
@@ -90,9 +153,8 @@ async fn responses_mode_stream_cli_supports_personal_access_tokens() {
let resp_mock = responses::mount_sse_once(&server, cli_sse_response()).await;
let home = TempDir::new().unwrap();
let output = personal_access_token_exec_command(&server, &home)
.output()
.unwrap();
let mut cmd = personal_access_token_exec_command(&server, &home);
let output = run_cli_command(&mut cmd).unwrap();
assert!(
output.status.success(),
@@ -140,9 +202,8 @@ async fn responses_mode_stream_cli_does_not_attempt_oauth_refresh_for_personal_a
.await;
let home = TempDir::new().unwrap();
let output = personal_access_token_exec_command(&server, &home)
.output()
.unwrap();
let mut cmd = personal_access_token_exec_command(&server, &home);
let output = run_cli_command(&mut cmd).unwrap();
assert!(!output.status.success());
server.verify().await;
@@ -168,8 +229,7 @@ async fn responses_mode_stream_cli() {
server.uri()
);
let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
let mut cmd = AssertCommand::new(bin);
cmd.timeout(Duration::from_secs(30));
let mut cmd = Command::new(bin);
cmd.arg("exec")
.arg("--skip-git-repo-check")
.arg("-c")
@@ -182,7 +242,7 @@ async fn responses_mode_stream_cli() {
cmd.env("CODEX_HOME", home.path())
.env("OPENAI_API_KEY", "dummy");
let output = cmd.output().unwrap();
let output = run_cli_command(&mut cmd).unwrap();
println!("Status: {}", output.status);
println!("Stdout:\n{}", String::from_utf8_lossy(&output.stdout));
println!("Stderr:\n{}", String::from_utf8_lossy(&output.stderr));
@@ -211,8 +271,7 @@ async fn responses_mode_stream_cli_supports_openai_base_url_config_override() {
let home = TempDir::new().unwrap();
let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
let mut cmd = AssertCommand::new(bin);
cmd.timeout(Duration::from_secs(30));
let mut cmd = Command::new(bin);
cmd.arg("exec")
.arg("--skip-git-repo-check")
.arg("-c")
@@ -223,7 +282,7 @@ async fn responses_mode_stream_cli_supports_openai_base_url_config_override() {
cmd.env("CODEX_HOME", home.path())
.env("OPENAI_API_KEY", "dummy");
let output = cmd.output().unwrap();
let output = run_cli_command(&mut cmd).unwrap();
assert!(output.status.success());
let request = resp_mock.single_request();
@@ -264,7 +323,7 @@ async fn exec_cli_applies_model_instructions_file() {
let home = TempDir::new().unwrap();
let repo_root = repo_root();
let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
let mut cmd = AssertCommand::new(bin);
let mut cmd = Command::new(bin);
cmd.arg("exec")
.arg("--skip-git-repo-check")
.arg("-c")
@@ -279,7 +338,7 @@ async fn exec_cli_applies_model_instructions_file() {
cmd.env("CODEX_HOME", home.path())
.env("OPENAI_API_KEY", "dummy");
let output = cmd.output().unwrap();
let output = run_cli_command(&mut cmd).unwrap();
println!("Status: {}", output.status);
println!("Stdout:\n{}", String::from_utf8_lossy(&output.stdout));
println!("Stderr:\n{}", String::from_utf8_lossy(&output.stderr));
@@ -334,7 +393,7 @@ async fn exec_cli_profile_applies_model_instructions_file() {
let repo_root = repo_root();
let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
let mut cmd = AssertCommand::new(bin);
let mut cmd = Command::new(bin);
cmd.arg("exec")
.arg("--skip-git-repo-check")
.arg("--profile")
@@ -349,7 +408,7 @@ async fn exec_cli_profile_applies_model_instructions_file() {
cmd.env("CODEX_HOME", home.path())
.env("OPENAI_API_KEY", "dummy");
let output = cmd.output().unwrap();
let output = run_cli_command(&mut cmd).unwrap();
println!("Status: {}", output.status);
println!("Stdout:\n{}", String::from_utf8_lossy(&output.stdout));
println!("Stderr:\n{}", String::from_utf8_lossy(&output.stderr));
@@ -379,8 +438,7 @@ async fn responses_api_stream_cli() {
let home = TempDir::new().unwrap();
let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
let mut cmd = AssertCommand::new(bin);
cmd.timeout(Duration::from_secs(30));
let mut cmd = Command::new(bin);
cmd.arg("exec")
.arg("--skip-git-repo-check")
.arg("-c")
@@ -391,7 +449,7 @@ async fn responses_api_stream_cli() {
cmd.env("CODEX_HOME", home.path())
.env("OPENAI_API_KEY", "dummy");
let output = cmd.output().unwrap();
let output = run_cli_command(&mut cmd).unwrap();
assert!(output.status.success());
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(stdout.contains("fixture hello"));
@@ -421,8 +479,7 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> {
// 4. Run the codex CLI and invoke `exec`, which is what records a session.
let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
let mut cmd = AssertCommand::new(bin);
cmd.timeout(Duration::from_secs(30));
let mut cmd = Command::new(bin);
cmd.arg("exec")
.arg("--skip-git-repo-check")
.arg("-c")
@@ -433,7 +490,7 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> {
cmd.env("CODEX_HOME", home.path())
.env(CODEX_API_KEY_ENV_VAR, "dummy");
let output = cmd.output().unwrap();
let output = run_cli_command(&mut cmd).unwrap();
assert!(
output.status.success(),
"codex-cli exec failed: {}",
@@ -542,8 +599,7 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> {
let marker2 = format!("integration-resume-{}", Uuid::new_v4());
let prompt2 = format!("echo {marker2}");
let bin2 = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
let mut cmd2 = AssertCommand::new(bin2);
cmd2.timeout(Duration::from_secs(30));
let mut cmd2 = Command::new(bin2);
cmd2.arg("exec")
.arg("--skip-git-repo-check")
.arg("-c")
@@ -556,7 +612,7 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> {
cmd2.env("CODEX_HOME", home.path())
.env("OPENAI_API_KEY", "dummy");
let output2 = cmd2.output().unwrap();
let output2 = run_cli_command(&mut cmd2).unwrap();
assert!(output2.status.success(), "resume codex-cli run failed");
assert_eq!(resp_mock.requests().len(), 2);