openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-24 18:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

cleanup pass

Browse Source
This commit is contained in:
Ryan Ragona
2025-04-26 15:34:08 -07:00
parent f1c6625bf2
commit 07911ddc3e
11 changed files with 108 additions and 400 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/session/Cargo.toml
+1 -10
View File
@@ -24,14 +24,6 @@ tokio = { version = "1", features = [
] }
tracing = { version = "0.1.41", features = ["log"] }
tracing-subscriber = { version = "0.3.19", features = ["env-filter"] }
# --- additions for PTY/TUI support ---
# Raw terminal handling when attaching to TUI sessions
# PTY helpers (unix only)
# new dependencies for session management
uuid = { version = "1", features = ["v4"] }
chrono = { version = "0.4", features = ["serde"] }
serde = { version = "1.0", features = ["derive"] }
@@ -39,8 +31,7 @@ serde_json = "1"
dirs = "5"
sysinfo = "0.29"
tabwriter = "1.3"
serde_yaml = "0.9"
names = "0.14"
names = { version = "0.14", default-features = false }
# unix-only process helpers
nix = { version = "0.27", default-features = false, features = ["process", "signal", "term", "fs"] }
codex-rs/session/src/cli.rs
+17 -68
View File
@@ -8,7 +8,6 @@
//! The `create` command therefore has mutually exclusive sub-commands so the appropriate
//! arguments can be forwarded to the underlying agent binaries.
use crate::meta::AgentCli;
use crate::meta::SessionMeta;
use crate::spawn;
use crate::store;
@@ -25,7 +24,8 @@ use clap::ValueEnum;
#[cfg(unix)]
use codex_repl as _; // Ensures the dependency is only required on Unix.
use serde::Serialize;
#[allow(unused_imports)]
use serde::Serialize; // still needed for table print rows in tests
/// A human-friendly representation of a byte count (e.g. 1.4M).
pub fn human_bytes(b: u64) -> String {
@@ -151,7 +151,7 @@ impl CreateCmd {
u32, // pid
Option<String>, // prompt preview
store::SessionKind, // kind
AgentCli, // full CLI config
Vec<String>, // raw argv used to spawn the agent
)> = (|| match self.agent {
AgentKind::Exec(cmd) => {
let args = build_exec_args(&cmd.exec_cli);
@@ -163,7 +163,7 @@ impl CreateCmd {
child.id().unwrap_or_default(),
preview,
store::SessionKind::Exec,
AgentCli::Exec(cmd.exec_cli.clone()),
args.clone(),
))
}
#[cfg(unix)]
@@ -177,12 +177,12 @@ impl CreateCmd {
child.id().unwrap_or_default(),
preview,
store::SessionKind::Repl,
AgentCli::Repl(cmd.repl_cli.clone()),
args.clone(),
))
}
})();
let (pid, prompt_preview, kind, cli_cfg) = match spawn_result {
let (pid, prompt_preview, kind, argv) = match spawn_result {
Ok(tuple) => tuple,
Err(err) => {
// Best effort clean-up – ignore failures so we don't mask the
@@ -194,7 +194,7 @@ impl CreateCmd {
// Persist metadata **after** the process has been spawned so we can record its PID.
// Persist metadata **after** the process has been spawned so we can record its PID.
let meta = SessionMeta::new(id.clone(), pid, kind, cli_cfg, prompt_preview);
let meta = SessionMeta::new(id.clone(), pid, kind, argv, prompt_preview);
store::write_meta(&paths, &meta)?;
@@ -476,9 +476,6 @@ impl DeleteCmd {
pub struct LogsCmd {
id: String,
#[arg(short, long)]
follow: bool,
#[arg(long)]
stderr: bool,
}
@@ -495,66 +492,22 @@ impl LogsCmd {
let file = tokio::fs::File::open(target).await?;
if self.follow {
// ------------------------------------------------------------------
// Improved `--follow` implementation (tail -f semantics)
//
// 1. Start at *the end* of the file so we only stream *new* output
// that appears after the command has been issued. This avoids
// re-printing potentially huge log histories when the user is
// solely interested in live updates.
// 2. Keep retrying after EOF so the behaviour matches the familiar
// `tail -f` utility.
use tokio::io::AsyncBufReadExt;
use tokio::io::AsyncSeekExt;
use tokio::io::BufReader;
use tokio::time::sleep;
use tokio::time::Duration;
// Jump to EOF before we start reading so we don't emit historical
// data. Ignore errors from `seek` on special files – in that case
// we just fall back to the normal behaviour.
let mut file = file;
let _ = file.seek(std::io::SeekFrom::End(0)).await;
let mut lines = BufReader::new(file).lines();
loop {
match lines.next_line().await? {
Some(l) => println!("{l}"),
None => {
// EOF – wait a little and retry.
sleep(Duration::from_millis(100)).await;
}
}
}
} else {
tokio::io::copy(
&mut tokio::io::BufReader::new(file),
&mut tokio::io::stdout(),
)
.await?;
}
// Stream the complete file to stdout. Users can pipe to `tail -f`,
// `less +F`, etc. if they only want live updates.
tokio::io::copy(
&mut tokio::io::BufReader::new(file),
&mut tokio::io::stdout(),
)
.await?;
Ok(())
}
}
// -----------------------------------------------------------------------------
// list
#[derive(Copy, Clone, ValueEnum, Debug)]
enum OutputFormat {
Table,
Json,
Yaml,
}
// list – newest-first overview of all sessions
#[derive(Args)]
pub struct ListCmd {
/// Output format (default: table).
#[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table)]
output: OutputFormat,
}
pub struct ListCmd {}
#[derive(Serialize)]
#[allow(missing_docs)]
@@ -615,11 +568,7 @@ impl ListCmd {
})
.collect();
match self.output {
OutputFormat::Table => print_table(&rows)?,
OutputFormat::Json => println!("{}", serde_json::to_string_pretty(&rows)?),
OutputFormat::Yaml => println!("{}", serde_yaml::to_string(&rows)?),
}
print_table(&rows)?;
Ok(())
}
codex-rs/session/src/meta.rs
+24 -41
View File
@@ -1,10 +1,12 @@
//! Rich on-disk session metadata envelope.
//! Lightweight on-disk session metadata.
//!
//! The file is written as `meta.json` inside every session directory so users
//! (and other tools) can inspect how a particular session was started even
//! months later. Keeping the full CLI invocation together with a few extra
//! bits of contextual information (like the git commit of the build) makes
//! debugging and reproducibility significantly easier.
//! The metadata is persisted as `meta.json` inside each session directory so
//! users – or other tooling – can inspect **how** a session was started even
//! months later. Instead of serialising the full, typed CLI structs (which
//! would force every agent crate to depend on `serde`) we only keep the raw
//! argument vector that was passed to the spawned process. This keeps the
//! public API surface minimal while still giving us reproducibility – a
//! session can always be re-spawned with `codex <args…>`.
use chrono::DateTime;
use chrono::Utc;
@@ -13,78 +15,59 @@ use serde::Serialize;
use crate::store::SessionKind;
/// The CLI configuration that was used to launch the underlying agent.
///
/// Depending on the chosen agent flavour (`codex-exec` vs `codex-repl`) the
/// contained configuration differs. We use an *externally tagged* enum so
/// the JSON clearly states which variant was used while still keeping the
/// nested structure as-is.
#[derive(Debug, Serialize, Deserialize)]
#[serde(tag = "agent", rename_all = "lowercase")]
pub enum AgentCli {
/// Non-interactive batch agent.
Exec(codex_exec::Cli),
/// JSON envelope version. Bump when the structure changes in a
/// backwards-incompatible way.
pub const CURRENT_VERSION: u8 = 2;
/// Interactive REPL agent (only available on Unix-like systems).
#[cfg(unix)]
Repl(codex_repl::Cli),
}
/// Versioned envelope that is persisted to disk.
///
/// A monotonically increasing `version` field allows us to evolve the schema
/// over time while still being able to parse *older* files.
/// Persisted session metadata.
#[derive(Debug, Serialize, Deserialize)]
pub struct SessionMeta {
/// Unique identifier – also doubles as the directory name.
/// Unique identifier (also doubles as directory name).
pub id: String,
/// Process ID of the *leader* process belonging to the session.
/// Leader process id (PID).
pub pid: u32,
/// Whether the session is an `exec` or `repl` one.
pub kind: SessionKind,
/// Complete CLI configuration that was used to spawn the agent.
pub cli: AgentCli,
/// Raw command-line arguments that were used to spawn the agent
/// (`codex-exec …` or `codex-repl …`).
pub argv: Vec<String>,
/// Short preview of the natural-language prompt (if present).
/// Short preview of the user prompt (if any).
#[serde(skip_serializing_if = "Option::is_none")]
pub prompt_preview: Option<String>,
/// Wall-clock timestamp when the session was created.
pub created_at: DateTime<Utc>,
/// Git commit hash of the `codex-rs` build that produced this file.
/// Git commit hash of the build that produced this file.
pub codex_commit: String,
/// Schema version so we can migrate later.
/// Schema version (see [`CURRENT_VERSION`]).
pub version: u8,
}
impl SessionMeta {
/// Bump this whenever the structure changes in a backwards-incompatible
/// way.
pub const CURRENT_VERSION: u8 = 1;
/// Convenience constructor.
#[allow(clippy::too_many_arguments)]
pub fn new(
id: String,
pid: u32,
kind: SessionKind,
cli: AgentCli,
argv: Vec<String>,
prompt_preview: Option<String>,
) -> Self {
Self {
id,
pid,
kind,
cli,
argv,
prompt_preview,
created_at: Utc::now(),
codex_commit: crate::build::git_sha().to_owned(),
version: Self::CURRENT_VERSION,
version: CURRENT_VERSION,
}
}
}
codex-rs/session/src/spawn.rs
+42 -41
View File
@@ -7,6 +7,38 @@ use std::fs::OpenOptions;
use tokio::process::Child;
use tokio::process::Command;
// -----------------------------------------------------------------------------
// Internal helpers
/// Open (and create if necessary) the log files that stdout / stderr of the
/// spawned agent will be redirected to.
fn open_log_files(paths: &Paths) -> Result<(std::fs::File, std::fs::File)> {
let stdout = OpenOptions::new()
.create(true)
.append(true)
.open(&paths.stdout)?;
let stderr = OpenOptions::new()
.create(true)
.append(true)
.open(&paths.stderr)?;
Ok((stdout, stderr))
}
/// Configure a `tokio::process::Command` with the common options that are the
/// same for both `codex-exec` and `codex-repl` sessions.
fn base_command(bin: &str, paths: &Paths) -> Result<Command> {
let (stdout, stderr) = open_log_files(paths)?;
let mut cmd = Command::new(bin);
cmd.stdin(std::process::Stdio::null())
.stdout(stdout)
.stderr(stderr);
Ok(cmd)
}
// -----------------------------------------------------------------------------
// exec – non-interactive batch agent
@@ -15,21 +47,14 @@ pub fn spawn_exec(paths: &Paths, exec_args: &[String]) -> Result<Child> {
{
use std::io;
let stdin = OpenOptions::new().read(true).open("/dev/null")?;
let stdout = OpenOptions::new()
.create(true)
.append(true)
.open(&paths.stdout)?;
let stderr = OpenOptions::new()
.create(true)
.append(true)
.open(&paths.stderr)?;
let mut cmd = base_command("codex-exec", paths)?;
cmd.args(exec_args);
let mut cmd = Command::new("codex-exec");
cmd.args(exec_args)
.stdin(stdin)
.stdout(stdout)
.stderr(stderr);
// Replace the `stdin` that `base_command` configured (null) with
// `/dev/null` opened for reading – keeps the previous behaviour while
// still leveraging the common helper.
let stdin = OpenOptions::new().read(true).open("/dev/null")?;
cmd.stdin(stdin);
unsafe {
cmd.pre_exec(|| {
@@ -52,20 +77,8 @@ pub fn spawn_exec(paths: &Paths, exec_args: &[String]) -> Result<Child> {
const DETACHED_PROCESS: u32 = 0x00000008;
const CREATE_NEW_PROCESS_GROUP: u32 = 0x00000200;
let stdout = OpenOptions::new()
.create(true)
.append(true)
.open(&paths.stdout)?;
let stderr = OpenOptions::new()
.create(true)
.append(true)
.open(&paths.stderr)?;
let mut cmd = Command::new("codex-exec");
let mut cmd = base_command("codex-exec", paths)?;
cmd.args(exec_args)
.stdin(std::process::Stdio::null())
.stdout(stdout)
.stderr(stderr)
.creation_flags(DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP);
let child = cmd.spawn().context("failed to spawn codex-exec")?;
@@ -98,20 +111,8 @@ pub fn spawn_repl(paths: &Paths, repl_args: &[String]) -> Result<Child> {
.write(true)
.open(&paths.stdin)?;
let stdout = OpenOptions::new()
.create(true)
.append(true)
.open(&paths.stdout)?;
let stderr = OpenOptions::new()
.create(true)
.append(true)
.open(&paths.stderr)?;
let mut cmd = Command::new("codex-repl");
cmd.args(repl_args)
.stdin(stdin)
.stdout(stdout)
.stderr(stderr);
let mut cmd = base_command("codex-repl", paths)?;
cmd.args(repl_args).stdin(stdin);
unsafe {
cmd.pre_exec(|| {
codex-rs/session/src/store.rs
+9 -39
View File
@@ -208,38 +208,18 @@ pub async fn kill_session(id: &str) -> Result<()> {
let pid_u32 = meta.pid;
// Helper – check if the original *leader* process is still around.
#[cfg(unix)]
fn is_alive(pid: libc::pid_t) -> bool {
unsafe { libc::kill(pid, 0) == 0 }
}
#[cfg(windows)]
// Helper – cross-platform liveness probe based on the `sysinfo` crate.
fn is_alive(pid: u32) -> bool {
use windows_sys::Win32::Foundation::CloseHandle;
use windows_sys::Win32::Foundation::HANDLE;
use windows_sys::Win32::System::Threading::GetExitCodeProcess;
use windows_sys::Win32::System::Threading::OpenProcess;
use windows_sys::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION;
const STILL_ACTIVE: u32 = 259;
use sysinfo::PidExt;
use sysinfo::SystemExt;
unsafe {
let handle: HANDLE = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid);
if handle == 0 {
return false;
}
let mut exit_code: u32 = 0;
let ok = GetExitCodeProcess(handle, &mut exit_code as *mut _);
CloseHandle(handle);
ok != 0 && exit_code == STILL_ACTIVE
}
let mut sys = sysinfo::System::new();
sys.refresh_process(sysinfo::Pid::from_u32(pid));
sys.process(sysinfo::Pid::from_u32(pid)).is_some()
}
// If the process is already gone we bail out so the caller knows the session
// directory might need manual clean-up.
#[cfg(unix)]
let mut still_running = is_alive(pid_u32 as libc::pid_t);
#[cfg(windows)]
let mut still_running = is_alive(pid_u32);
if !still_running {
@@ -277,19 +257,9 @@ pub async fn kill_session(id: &str) -> Result<()> {
let start = std::time::Instant::now();
while start.elapsed() < grace_period {
#[cfg(unix)]
{
if !is_alive(pid_u32 as libc::pid_t) {
still_running = false;
break;
}
}
#[cfg(windows)]
{
if !is_alive(pid_u32) {
still_running = false;
break;
}
if !is_alive(pid_u32) {
still_running = false;
break;
}
tokio::time::sleep(poll_interval).await;
}
codex-rs/session/tests/meta_round_trip.rs
-36
View File
@@ -1,36 +0,0 @@
//! Simple round-trip test that serialises a freshly constructed `SessionMeta`
//! and deserialises it back to ensure the schema is self-consistent.
use codex_session::meta::AgentCli;
use codex_session::meta::SessionMeta;
use codex_session::store::SessionKind;
#[test]
fn meta_round_trip() {
let exec_cli = codex_exec::Cli {
images: vec![],
model: Some("gpt-4o-mini".into()),
skip_git_repo_check: true,
disable_response_storage: false,
prompt: Some("hello world".into()),
};
let meta = SessionMeta::new(
"test-session".into(),
42,
SessionKind::Exec,
AgentCli::Exec(exec_cli.clone()),
exec_cli.prompt.clone(),
);
// Serialise with pretty printer so humans can read the file as well.
let json = serde_json::to_string_pretty(&meta).expect("serialise");
// … and parse it back.
let de: SessionMeta = serde_json::from_str(&json).expect("deserialise");
assert_eq!(de.version, SessionMeta::CURRENT_VERSION);
assert_eq!(de.id, "test-session");
assert_eq!(de.pid, 42);
assert!(matches!(de.cli, AgentCli::Exec(_)));
}