mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
Clean up incidental diffs and add live commit trailer test
- revert nonessential changes in shell_snapshot and unified exec process_manager - add ignored live model-loop test that verifies commit trailer injection via codex_git_commit + command_attribution config
This commit is contained in:
@@ -120,11 +120,10 @@ impl ShellSnapshot {
|
||||
.join(format!("{session_id}.{extension}"));
|
||||
|
||||
// Clean the (unlikely) leaked snapshot files.
|
||||
let cleanup_codex_home = codex_home.to_path_buf();
|
||||
let codex_home = codex_home.to_path_buf();
|
||||
let cleanup_session_id = session_id;
|
||||
tokio::spawn(async move {
|
||||
if let Err(err) = cleanup_stale_snapshots(&cleanup_codex_home, cleanup_session_id).await
|
||||
{
|
||||
if let Err(err) = cleanup_stale_snapshots(&codex_home, cleanup_session_id).await {
|
||||
tracing::warn!("Failed to clean up shell snapshots: {err:?}");
|
||||
}
|
||||
});
|
||||
|
||||
@@ -517,11 +517,10 @@ impl UnifiedExecProcessManager {
|
||||
cwd: PathBuf,
|
||||
context: &UnifiedExecContext,
|
||||
) -> Result<UnifiedExecProcess, UnifiedExecError> {
|
||||
let env = create_env(
|
||||
let env = apply_unified_exec_env(create_env(
|
||||
&context.turn.shell_environment_policy,
|
||||
Some(context.session.conversation_id),
|
||||
);
|
||||
let env = apply_unified_exec_env(env);
|
||||
));
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = UnifiedExecRuntime::new(self);
|
||||
let exec_approval_requirement = context
|
||||
|
||||
@@ -6,6 +6,9 @@
|
||||
|
||||
use assert_cmd::prelude::*;
|
||||
use predicates::prelude::*;
|
||||
use std::fs;
|
||||
use std::io::IsTerminal;
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
use std::process::Stdio;
|
||||
use tempfile::TempDir;
|
||||
@@ -15,24 +18,49 @@ fn require_api_key() -> String {
|
||||
.expect("OPENAI_API_KEY env var not set — skip running live tests")
|
||||
}
|
||||
|
||||
fn codex_bin_path() -> std::path::PathBuf {
|
||||
codex_utils_cargo_bin::cargo_bin("codex-rs")
|
||||
.or_else(|_| codex_utils_cargo_bin::cargo_bin("codex"))
|
||||
.expect("failed to locate codex binary")
|
||||
}
|
||||
|
||||
/// Helper that spawns the binary inside a TempDir with minimal flags. Returns (Assert, TempDir).
|
||||
fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) {
|
||||
#![expect(clippy::unwrap_used)]
|
||||
let dir = TempDir::new().unwrap();
|
||||
let assert = run_live_in_dir(prompt, dir.path(), None);
|
||||
(assert, dir)
|
||||
}
|
||||
|
||||
fn run_live_in_dir(
|
||||
prompt: &str,
|
||||
working_dir: &Path,
|
||||
codex_home: Option<&Path>,
|
||||
) -> assert_cmd::assert::Assert {
|
||||
#![expect(clippy::unwrap_used)]
|
||||
use std::io::Read;
|
||||
use std::io::Write;
|
||||
use std::thread;
|
||||
|
||||
let dir = TempDir::new().unwrap();
|
||||
|
||||
// Build a plain `std::process::Command` so we have full control over the underlying stdio
|
||||
// handles. `assert_cmd`’s own `Command` wrapper always forces stdout/stderr to be piped
|
||||
// internally which prevents us from streaming them live to the terminal (see its `spawn`
|
||||
// implementation). Instead we configure the std `Command` ourselves, then later hand the
|
||||
// resulting `Output` to `assert_cmd` for the familiar assertions.
|
||||
|
||||
let mut cmd = Command::new(codex_utils_cargo_bin::cargo_bin("codex-rs").unwrap());
|
||||
cmd.current_dir(dir.path());
|
||||
let bin_path = codex_bin_path();
|
||||
let use_piped_stdin = bin_path
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.map(|name| name == "codex-rs")
|
||||
.unwrap_or(false);
|
||||
let mut cmd = Command::new(bin_path);
|
||||
cmd.current_dir(working_dir);
|
||||
cmd.env("OPENAI_API_KEY", require_api_key());
|
||||
cmd.env("TERM", "xterm-256color");
|
||||
if let Some(home) = codex_home {
|
||||
cmd.env("CODEX_HOME", home);
|
||||
}
|
||||
|
||||
// We want three things at once:
|
||||
// 1. live streaming of the child’s stdout/stderr while the test is running
|
||||
@@ -46,24 +74,27 @@ fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) {
|
||||
// – an in‑memory buffer so we can pass it to `assert_cmd` later
|
||||
|
||||
// Pass the prompt through the `--` separator so the CLI knows when user input ends.
|
||||
cmd.arg("--allow-no-git-exec")
|
||||
.arg("-v")
|
||||
.arg("--")
|
||||
.arg(prompt);
|
||||
cmd.arg("--").arg(prompt);
|
||||
|
||||
cmd.stdin(Stdio::piped());
|
||||
if use_piped_stdin {
|
||||
cmd.stdin(Stdio::piped());
|
||||
} else {
|
||||
cmd.stdin(Stdio::inherit());
|
||||
}
|
||||
cmd.stdout(Stdio::piped());
|
||||
cmd.stderr(Stdio::piped());
|
||||
|
||||
let mut child = cmd.spawn().expect("failed to spawn codex-rs");
|
||||
|
||||
// Send the terminating newline so Session::run exits after the first turn.
|
||||
child
|
||||
.stdin
|
||||
.as_mut()
|
||||
.expect("child stdin unavailable")
|
||||
.write_all(b"\n")
|
||||
.expect("failed to write to child stdin");
|
||||
if use_piped_stdin {
|
||||
// Send the terminating newline so Session::run exits after the first turn.
|
||||
child
|
||||
.stdin
|
||||
.as_mut()
|
||||
.expect("child stdin unavailable")
|
||||
.write_all(b"\n")
|
||||
.expect("failed to write to child stdin");
|
||||
}
|
||||
|
||||
// Helper that tees a ChildStdout/ChildStderr into both the parent’s stdio and a Vec<u8>.
|
||||
fn tee<R: Read + Send + 'static>(
|
||||
@@ -107,7 +138,22 @@ fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) {
|
||||
stderr,
|
||||
};
|
||||
|
||||
(output.assert(), dir)
|
||||
output.assert()
|
||||
}
|
||||
|
||||
fn run_git(working_dir: &Path, args: &[&str]) {
|
||||
let output = Command::new("git")
|
||||
.current_dir(working_dir)
|
||||
.args(args)
|
||||
.output()
|
||||
.expect("failed to spawn git");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git {:?} failed\nstdout: {}\nstderr: {}",
|
||||
args,
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
|
||||
#[ignore]
|
||||
@@ -146,3 +192,68 @@ fn live_print_working_directory() {
|
||||
.success()
|
||||
.stdout(predicate::str::contains(dir.path().to_string_lossy()));
|
||||
}
|
||||
|
||||
#[ignore]
|
||||
#[test]
|
||||
fn live_git_commit_includes_configured_coauthor_trailer() {
|
||||
if std::env::var("OPENAI_API_KEY").is_err() {
|
||||
eprintln!(
|
||||
"skipping live_git_commit_includes_configured_coauthor_trailer – OPENAI_API_KEY not set"
|
||||
);
|
||||
return;
|
||||
}
|
||||
if !std::io::stdin().is_terminal() {
|
||||
eprintln!(
|
||||
"skipping live_git_commit_includes_configured_coauthor_trailer – stdin is not a terminal"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let repo_dir = TempDir::new().expect("failed to create repo tempdir");
|
||||
run_git(repo_dir.path(), &["init"]);
|
||||
run_git(repo_dir.path(), &["config", "user.name", "Codex Live Test"]);
|
||||
run_git(
|
||||
repo_dir.path(),
|
||||
&["config", "user.email", "codex-live-test@example.com"],
|
||||
);
|
||||
fs::write(repo_dir.path().join("tracked.txt"), "hello\n")
|
||||
.expect("failed to create tracked file");
|
||||
run_git(repo_dir.path(), &["add", "tracked.txt"]);
|
||||
|
||||
let codex_home = TempDir::new().expect("failed to create codex home tempdir");
|
||||
fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
r#"
|
||||
command_attribution = "Live Tester <live-tester@example.com>"
|
||||
|
||||
[features]
|
||||
codex_git_commit = true
|
||||
"#,
|
||||
)
|
||||
.expect("failed to write config.toml");
|
||||
|
||||
let assert = run_live_in_dir(
|
||||
"Use the shell tool to commit the currently staged changes with commit title 'live attribution test'. Then stop.",
|
||||
repo_dir.path(),
|
||||
Some(codex_home.path()),
|
||||
);
|
||||
assert.success();
|
||||
|
||||
let output = Command::new("git")
|
||||
.current_dir(repo_dir.path())
|
||||
.args(["log", "-1", "--pretty=%B"])
|
||||
.output()
|
||||
.expect("failed to read latest commit message");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"failed to read commit message\nstdout: {}\nstderr: {}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
|
||||
let commit_message = String::from_utf8(output.stdout).expect("invalid utf8 commit message");
|
||||
assert!(
|
||||
commit_message.contains("Co-authored-by: Live Tester <live-tester@example.com>"),
|
||||
"commit message missing configured co-author trailer:\n{commit_message}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user