From 07911ddc3e82050f28b4d00f4a71d2c7e4ea8bfd Mon Sep 17 00:00:00 2001 From: Ryan Ragona Date: Sat, 26 Apr 2025 15:34:08 -0700 Subject: [PATCH] cleanup pass --- codex-rs/Cargo.lock | 168 ++-------------------- codex-rs/exec/Cargo.toml | 1 - codex-rs/exec/src/cli.rs | 6 +- codex-rs/interactive/Cargo.toml | 1 - codex-rs/interactive/src/cli.rs | 4 +- codex-rs/session/Cargo.toml | 11 +- codex-rs/session/src/cli.rs | 85 +++-------- codex-rs/session/src/meta.rs | 65 ++++----- codex-rs/session/src/spawn.rs | 83 +++++------ codex-rs/session/src/store.rs | 48 ++----- codex-rs/session/tests/meta_round_trip.rs | 36 ----- 11 files changed, 108 insertions(+), 400 deletions(-) delete mode 100644 codex-rs/session/tests/meta_round_trip.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 989ae385ca..a66c4fd6a1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -235,17 +235,6 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" -[[package]] -name = "atty" -version = "0.2.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" -dependencies = [ - "hermit-abi 0.1.19", - "libc", - "winapi", -] - [[package]] name = "autocfg" version = "1.4.0" @@ -398,23 +387,6 @@ dependencies = [ "windows-link", ] -[[package]] -name = "clap" -version = "3.2.25" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ea181bf566f71cb9a5d17a59e1871af638180a18fb0035c92ae62b705207123" -dependencies = [ - "atty", - "bitflags 1.3.2", - "clap_derive 3.2.25", - "clap_lex 0.2.4", - "indexmap 1.9.3", - "once_cell", - "strsim 0.10.0", - "termcolor", - "textwrap 0.16.2", -] - [[package]] name = "clap" version = "4.5.37" @@ -422,7 +394,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eccb054f56cbd38340b380d4a8e69ef1f02f1af43db2f0cc817a4774d80ae071" dependencies = [ "clap_builder", - "clap_derive 4.5.32", + "clap_derive", ] [[package]] @@ -433,45 +405,23 @@ checksum = "efd9466fac8543255d3b1fcad4762c5e116ffe808c8a3043d4263cd4fd4862a2" dependencies = [ "anstream", "anstyle", - "clap_lex 0.7.4", + "clap_lex", "strsim 0.11.1", "terminal_size", ] -[[package]] -name = "clap_derive" -version = "3.2.25" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae6371b8bdc8b7d3959e9cf7b22d4435ef3e79e138688421ec654acf8c81b008" -dependencies = [ - "heck 0.4.1", - "proc-macro-error", - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "clap_derive" version = "4.5.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09176aae279615badda0765c0c0b3f6ed53f4709118af73cf4655d85d1530cd7" dependencies = [ - "heck 0.5.0", + "heck", "proc-macro2", "quote", "syn 2.0.100", ] -[[package]] -name = "clap_lex" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2850f2f5a82cbf437dd5af4d49848fbdfc27c157c3d010345776f952765261c5" -dependencies = [ - "os_str_bytes", -] - [[package]] name = "clap_lex" version = "0.7.4" @@ -522,7 +472,7 @@ name = "codex-cli" version = "0.1.0" dependencies = [ "anyhow", - "clap 4.5.37", + "clap", "codex-core", "codex-exec", "codex-interactive", @@ -543,7 +493,7 @@ dependencies = [ "async-channel", "base64 0.21.7", "bytes", - "clap 4.5.37", + "clap", "codex-apply-patch", "dirs 6.0.0", "env-flags", @@ -577,9 +527,8 @@ name = "codex-exec" version = "0.1.0" dependencies = [ "anyhow", - "clap 4.5.37", + "clap", "codex-core", - "serde", "tokio", "tracing", "tracing-subscriber", @@ -591,7 +540,7 @@ version = "0.1.0" dependencies = [ "allocative", "anyhow", - "clap 4.5.37", + "clap", "derive_more", "env_logger", "log", @@ -610,9 +559,8 @@ name = "codex-interactive" version = "0.1.0" dependencies = [ "anyhow", - "clap 4.5.37", + "clap", "codex-core", - "serde", "tokio", ] @@ -621,7 +569,7 @@ name = "codex-repl" version = "0.1.0" dependencies = [ "anyhow", - "clap 4.5.37", + "clap", "codex-core", "owo-colors 4.2.0", "rand 0.9.1", @@ -637,7 +585,7 @@ version = "0.1.0" dependencies = [ "anyhow", "chrono", - "clap 4.5.37", + "clap", "codex-core", "codex-exec", "codex-repl", @@ -647,7 +595,6 @@ dependencies = [ "nix 0.27.1", "serde", "serde_json", - "serde_yaml", "sysinfo", "tabwriter", "tempfile", @@ -663,7 +610,7 @@ name = "codex-tui" version = "0.1.0" dependencies = [ "anyhow", - "clap 4.5.37", + "clap", "codex-ansi-escape", "codex-core", "color-eyre", @@ -1465,27 +1412,12 @@ dependencies = [ "foldhash", ] -[[package]] -name = "heck" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" - [[package]] name = "heck" version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" -[[package]] -name = "hermit-abi" -version = "0.1.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62b467343b94ba476dcb2500d242dadbb39557df889310ac77c5d99100aaac33" -dependencies = [ - "libc", -] - [[package]] name = "hermit-abi" version = "0.3.9" @@ -2170,7 +2102,6 @@ version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7bddcd3bf5144b6392de80e04c347cd7fab2508f6df16a85fc496ecd5cec39bc" dependencies = [ - "clap 3.2.25", "rand 0.8.5", ] @@ -2394,12 +2325,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" -[[package]] -name = "os_str_bytes" -version = "6.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" - [[package]] name = "overload" version = "0.1.1" @@ -2601,30 +2526,6 @@ dependencies = [ "yansi", ] -[[package]] -name = "proc-macro-error" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" -dependencies = [ - "proc-macro-error-attr", - "proc-macro2", - "quote", - "syn 1.0.109", - "version_check", -] - -[[package]] -name = "proc-macro-error-attr" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" -dependencies = [ - "proc-macro2", - "quote", - "version_check", -] - [[package]] name = "proc-macro2" version = "1.0.95" @@ -3203,19 +3104,6 @@ dependencies = [ "syn 2.0.100", ] -[[package]] -name = "serde_yaml" -version = "0.9.34+deprecated" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" -dependencies = [ - "indexmap 2.9.0", - "itoa", - "ryu", - "serde", - "unsafe-libyaml", -] - [[package]] name = "sharded-slab" version = "0.1.7" @@ -3346,7 +3234,7 @@ dependencies = [ "starlark_syntax", "static_assertions", "strsim 0.10.0", - "textwrap 0.11.0", + "textwrap", "thiserror 1.0.69", ] @@ -3451,7 +3339,7 @@ version = "0.26.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4c6bee85a5a24955dc440386795aa378cd9cf82acd5f764469152d2270e581be" dependencies = [ - "heck 0.5.0", + "heck", "proc-macro2", "quote", "rustversion", @@ -3575,15 +3463,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "termcolor" -version = "1.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" -dependencies = [ - "winapi-util", -] - [[package]] name = "terminal_size" version = "0.4.2" @@ -3609,12 +3488,6 @@ dependencies = [ "unicode-width 0.1.14", ] -[[package]] -name = "textwrap" -version = "0.16.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c13547615a44dc9c452a8a534638acdf07120d4b6847c8178705da06306a3057" - [[package]] name = "thiserror" version = "1.0.69" @@ -4025,12 +3898,6 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" -[[package]] -name = "unsafe-libyaml" -version = "0.2.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" - [[package]] name = "untrusted" version = "0.9.0" @@ -4237,15 +4104,6 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" -[[package]] -name = "winapi-util" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" -dependencies = [ - "windows-sys 0.59.0", -] - [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index 228e537e74..6ed6a4807a 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -26,4 +26,3 @@ tracing = { version = "0.1.41", features = ["log"] } tracing-subscriber = { version = "0.3.19", features = ["env-filter"] } # For serialising the `Cli` struct into the on-disk session metadata. -serde = { version = "1.0", features = ["derive"] } diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index d7f6547488..12be13480a 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -1,13 +1,9 @@ use clap::Parser; -use serde::Deserialize; -use serde::Serialize; use std::path::PathBuf; /// Command-line interface for the non-interactive `codex-exec` agent. /// -/// The struct needs to be serialisable so the full invocation can be stored -/// in the on-disk session `meta.json` for later introspection. -#[derive(Parser, Debug, Clone, Serialize, Deserialize)] +#[derive(Parser, Debug, Clone)] #[command(version)] pub struct Cli { /// Optional image(s) to attach to the initial prompt. diff --git a/codex-rs/interactive/Cargo.toml b/codex-rs/interactive/Cargo.toml index 8af96cf50c..b2a7234e26 100644 --- a/codex-rs/interactive/Cargo.toml +++ b/codex-rs/interactive/Cargo.toml @@ -15,7 +15,6 @@ path = "src/lib.rs" anyhow = "1" clap = { version = "4", features = ["derive"] } codex-core = { path = "../core", features = ["cli"] } -serde = { version = "1.0", features = ["derive"] } tokio = { version = "1", features = [ "io-std", "macros", diff --git a/codex-rs/interactive/src/cli.rs b/codex-rs/interactive/src/cli.rs index a090f8f700..ff09447b65 100644 --- a/codex-rs/interactive/src/cli.rs +++ b/codex-rs/interactive/src/cli.rs @@ -1,11 +1,9 @@ use clap::Parser; use codex_core::ApprovalModeCliArg; use codex_core::SandboxModeCliArg; -use serde::Deserialize; -use serde::Serialize; use std::path::PathBuf; -#[derive(Parser, Debug, Clone, Serialize, Deserialize)] +#[derive(Parser, Debug, Clone)] #[command(version)] pub struct Cli { /// Optional image(s) to attach to the initial prompt. diff --git a/codex-rs/session/Cargo.toml b/codex-rs/session/Cargo.toml index e0b8f550a7..2a8e166e2f 100644 --- a/codex-rs/session/Cargo.toml +++ b/codex-rs/session/Cargo.toml @@ -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"] } diff --git a/codex-rs/session/src/cli.rs b/codex-rs/session/src/cli.rs index dd7ccd636c..038257882b 100644 --- a/codex-rs/session/src/cli.rs +++ b/codex-rs/session/src/cli.rs @@ -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, // prompt preview store::SessionKind, // kind - AgentCli, // full CLI config + Vec, // 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(()) } diff --git a/codex-rs/session/src/meta.rs b/codex-rs/session/src/meta.rs index 88ac72bcbc..31b371c357 100644 --- a/codex-rs/session/src/meta.rs +++ b/codex-rs/session/src/meta.rs @@ -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 `. 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, - /// 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, /// Wall-clock timestamp when the session was created. pub created_at: DateTime, - /// 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, prompt_preview: Option, ) -> 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, } } } + diff --git a/codex-rs/session/src/spawn.rs b/codex-rs/session/src/spawn.rs index 7cde581150..e436fbde24 100644 --- a/codex-rs/session/src/spawn.rs +++ b/codex-rs/session/src/spawn.rs @@ -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 { + 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 { { 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 { 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 { .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(|| { diff --git a/codex-rs/session/src/store.rs b/codex-rs/session/src/store.rs index 70d73a4e1e..26208d109f 100644 --- a/codex-rs/session/src/store.rs +++ b/codex-rs/session/src/store.rs @@ -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; } diff --git a/codex-rs/session/tests/meta_round_trip.rs b/codex-rs/session/tests/meta_round_trip.rs deleted file mode 100644 index 25f4c8e240..0000000000 --- a/codex-rs/session/tests/meta_round_trip.rs +++ /dev/null @@ -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(_))); -}