diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 03a4222310..21c0f7930a 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -93,7 +93,7 @@ jobs: run: find . -name Cargo.toml -mindepth 2 -maxdepth 2 -print0 | xargs -0 -n1 -I{} bash -c 'cd "$(dirname "{}")" && cargo build' || echo "FAILED=${FAILED:+$FAILED, }cargo build individual crates" >> $GITHUB_ENV - name: cargo test - run: cargo test --target ${{ matrix.target }} || echo "FAILED=${FAILED:+$FAILED, }cargo test" >> $GITHUB_ENV + run: cargo test --all-features --target ${{ matrix.target }} || echo "FAILED=${FAILED:+$FAILED, }cargo test" >> $GITHUB_ENV - name: Fail if any step failed if: env.FAILED != '' diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 6df8bb06be..77a9ff74b3 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -473,6 +473,7 @@ version = "0.0.0" dependencies = [ "anyhow", "clap", + "codex-common", "codex-core", "codex-exec", "codex-tui", @@ -482,6 +483,15 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "codex-common" +version = "0.1.0" +dependencies = [ + "chrono", + "clap", + "codex-core", +] + [[package]] name = "codex-core" version = "0.1.0" @@ -530,6 +540,7 @@ dependencies = [ "anyhow", "chrono", "clap", + "codex-common", "codex-core", "mcp-types", "owo-colors 4.2.0", @@ -596,6 +607,7 @@ dependencies = [ "anyhow", "clap", "codex-ansi-escape", + "codex-common", "codex-core", "color-eyre", "crossterm", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 9afcc11f4c..c16727dac3 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -4,6 +4,7 @@ members = [ "ansi-escape", "apply-patch", "cli", + "common", "core", "exec", "execpolicy", diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index 7035bf2d51..848010e137 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -19,6 +19,7 @@ path = "src/lib.rs" anyhow = "1" clap = { version = "4", features = ["derive"] } codex-core = { path = "../core" } +codex-common = { path = "../common", features = ["cli"] } codex-exec = { path = "../exec" } codex-tui = { path = "../tui" } serde_json = "1" diff --git a/codex-rs/cli/src/lib.rs b/codex-rs/cli/src/lib.rs index 8d14388ab3..82e434a0c8 100644 --- a/codex-rs/cli/src/lib.rs +++ b/codex-rs/cli/src/lib.rs @@ -4,8 +4,8 @@ pub mod proto; pub mod seatbelt; use clap::Parser; +use codex_common::SandboxPermissionOption; use codex_core::protocol::SandboxPolicy; -use codex_core::SandboxPermissionOption; #[derive(Debug, Parser)] pub struct SeatbeltCommand { diff --git a/codex-rs/common/Cargo.toml b/codex-rs/common/Cargo.toml new file mode 100644 index 0000000000..c2abd5d242 --- /dev/null +++ b/codex-rs/common/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "codex-common" +version = "0.1.0" +edition = "2021" + +[dependencies] +chrono = { version = "0.4.40", optional = true } +clap = { version = "4", features = ["derive", "wrap_help"], optional = true } +codex-core = { path = "../core" } + +[features] +# Separate feature so that `clap` is not a mandatory dependency. +cli = ["clap"] +elapsed = ["chrono"] diff --git a/codex-rs/common/README.md b/codex-rs/common/README.md new file mode 100644 index 0000000000..9d5d415126 --- /dev/null +++ b/codex-rs/common/README.md @@ -0,0 +1,5 @@ +# codex-common + +This crate is designed for utilities that need to be shared across other crates in the workspace, but should not go in `core`. + +For narrow utility features, the pattern is to add introduce a new feature under `[features]` in `Cargo.toml` and then gate it with `#[cfg]` in `lib.rs`, as appropriate. diff --git a/codex-rs/core/src/approval_mode_cli_arg.rs b/codex-rs/common/src/approval_mode_cli_arg.rs similarity index 94% rename from codex-rs/core/src/approval_mode_cli_arg.rs rename to codex-rs/common/src/approval_mode_cli_arg.rs index 6aadbd92b4..199541148a 100644 --- a/codex-rs/core/src/approval_mode_cli_arg.rs +++ b/codex-rs/common/src/approval_mode_cli_arg.rs @@ -5,9 +5,9 @@ use clap::ArgAction; use clap::Parser; use clap::ValueEnum; -use crate::config::parse_sandbox_permission_with_base_path; -use crate::protocol::AskForApproval; -use crate::protocol::SandboxPermission; +use codex_core::config::parse_sandbox_permission_with_base_path; +use codex_core::protocol::AskForApproval; +use codex_core::protocol::SandboxPermission; #[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] diff --git a/codex-rs/common/src/elapsed.rs b/codex-rs/common/src/elapsed.rs new file mode 100644 index 0000000000..72108f9dd0 --- /dev/null +++ b/codex-rs/common/src/elapsed.rs @@ -0,0 +1,72 @@ +use chrono::Utc; + +/// Returns a string representing the elapsed time since `start_time` like +/// "1m15s" or "1.50s". +pub fn format_elapsed(start_time: chrono::DateTime) -> String { + let elapsed = Utc::now().signed_duration_since(start_time); + format_time_delta(elapsed) +} + +fn format_time_delta(elapsed: chrono::TimeDelta) -> String { + let millis = elapsed.num_milliseconds(); + format_elapsed_millis(millis) +} + +pub fn format_duration(duration: std::time::Duration) -> String { + let millis = duration.as_millis() as i64; + format_elapsed_millis(millis) +} + +fn format_elapsed_millis(millis: i64) -> String { + if millis < 1000 { + format!("{}ms", millis) + } else if millis < 60_000 { + format!("{:.2}s", millis as f64 / 1000.0) + } else { + let minutes = millis / 60_000; + let seconds = (millis % 60_000) / 1000; + format!("{minutes}m{seconds:02}s") + } +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::Duration; + + #[test] + fn test_format_time_delta_subsecond() { + // Durations < 1s should be rendered in milliseconds with no decimals. + let dur = Duration::milliseconds(250); + assert_eq!(format_time_delta(dur), "250ms"); + + // Exactly zero should still work. + let dur_zero = Duration::milliseconds(0); + assert_eq!(format_time_delta(dur_zero), "0ms"); + } + + #[test] + fn test_format_time_delta_seconds() { + // Durations between 1s (inclusive) and 60s (exclusive) should be + // printed with 2-decimal-place seconds. + let dur = Duration::milliseconds(1_500); // 1.5s + assert_eq!(format_time_delta(dur), "1.50s"); + + // 59.999s rounds to 60.00s + let dur2 = Duration::milliseconds(59_999); + assert_eq!(format_time_delta(dur2), "60.00s"); + } + + #[test] + fn test_format_time_delta_minutes() { + // Durations ≥ 1 minute should be printed mmss. + let dur = Duration::milliseconds(75_000); // 1m15s + assert_eq!(format_time_delta(dur), "1m15s"); + + let dur_exact = Duration::milliseconds(60_000); // 1m0s + assert_eq!(format_time_delta(dur_exact), "1m00s"); + + let dur_long = Duration::milliseconds(3_601_000); + assert_eq!(format_time_delta(dur_long), "60m01s"); + } +} diff --git a/codex-rs/common/src/lib.rs b/codex-rs/common/src/lib.rs new file mode 100644 index 0000000000..2533718883 --- /dev/null +++ b/codex-rs/common/src/lib.rs @@ -0,0 +1,10 @@ +#[cfg(feature = "cli")] +mod approval_mode_cli_arg; + +#[cfg(feature = "elapsed")] +pub mod elapsed; + +#[cfg(feature = "cli")] +pub use approval_mode_cli_arg::ApprovalModeCliArg; +#[cfg(feature = "cli")] +pub use approval_mode_cli_arg::SandboxPermissionOption; diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index abd0e607ec..9e0105082d 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -56,9 +56,3 @@ assert_cmd = "2" predicates = "3" tempfile = "3" wiremock = "0.6" - -[features] -default = [] - -# Separate feature so that `clap` is not a mandatory dependency. -cli = ["clap"] diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index f3140e0e9f..205dea64cb 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -266,7 +266,7 @@ pub fn log_dir() -> std::io::Result { Ok(p) } -pub(crate) fn parse_sandbox_permission_with_base_path( +pub fn parse_sandbox_permission_with_base_path( raw: &str, base_path: PathBuf, ) -> std::io::Result { diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 3878fada0d..919d05f154 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -26,10 +26,3 @@ pub mod util; mod zdr_transcript; pub use codex::Codex; - -#[cfg(feature = "cli")] -mod approval_mode_cli_arg; -#[cfg(feature = "cli")] -pub use approval_mode_cli_arg::ApprovalModeCliArg; -#[cfg(feature = "cli")] -pub use approval_mode_cli_arg::SandboxPermissionOption; diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index fdd75dbd84..f6df12b6a3 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -15,7 +15,8 @@ path = "src/lib.rs" anyhow = "1" chrono = "0.4.40" clap = { version = "4", features = ["derive"] } -codex-core = { path = "../core", features = ["cli"] } +codex-core = { path = "../core" } +codex-common = { path = "../common", features = ["cli", "elapsed"] } mcp-types = { path = "../mcp-types" } owo-colors = "4.2.0" serde_json = "1" diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 4443fd3094..1248ef3b19 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -1,6 +1,6 @@ use clap::Parser; use clap::ValueEnum; -use codex_core::SandboxPermissionOption; +use codex_common::SandboxPermissionOption; use std::path::PathBuf; #[derive(Parser, Debug)] diff --git a/codex-rs/exec/src/event_processor.rs b/codex-rs/exec/src/event_processor.rs index a8208883d5..d43f9d593c 100644 --- a/codex-rs/exec/src/event_processor.rs +++ b/codex-rs/exec/src/event_processor.rs @@ -1,4 +1,5 @@ use chrono::Utc; +use codex_common::elapsed::format_elapsed; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::FileChange; @@ -145,7 +146,7 @@ impl EventProcessor { }) = exec_command { ( - format_duration(start_time), + format!(" in {}", format_elapsed(start_time)), format!("{}", escape_command(&command).style(self.bold)), ) } else { @@ -160,7 +161,7 @@ impl EventProcessor { .join("\n"); match exit_code { 0 => { - let title = format!("{call} succeded{duration}:"); + let title = format!("{call} succeeded{duration}:"); ts_println!("{}", title.style(self.green)); } _ => { @@ -221,7 +222,7 @@ impl EventProcessor { .. }) = info { - (format_duration(start_time), invocation) + (format!(" in {}", format_elapsed(start_time)), invocation) } else { (String::new(), format!("tool('{call_id}')")) }; @@ -335,7 +336,7 @@ impl EventProcessor { }) = patch_begin { ( - format_duration(start_time), + format!(" in {}", format_elapsed(start_time)), format!("apply_patch(auto_approved={})", auto_approved), ) } else { @@ -383,13 +384,3 @@ fn format_file_change(change: &FileChange) -> &'static str { } => "M", } } - -fn format_duration(start_time: chrono::DateTime) -> String { - let elapsed = Utc::now().signed_duration_since(start_time); - let millis = elapsed.num_milliseconds(); - if millis < 1000 { - format!(" in {}ms", millis) - } else { - format!(" in {:.2}s", millis as f64 / 1000.0) - } -} diff --git a/codex-rs/mcp-server/Cargo.toml b/codex-rs/mcp-server/Cargo.toml index fdd2a304cd..d50bcae97c 100644 --- a/codex-rs/mcp-server/Cargo.toml +++ b/codex-rs/mcp-server/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" edition = "2021" [dependencies] -codex-core = { path = "../core", features = ["cli"] } +codex-core = { path = "../core" } mcp-types = { path = "../mcp-types" } schemars = "0.8.22" serde = { version = "1", features = ["derive"] } diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 32ba5a827b..c6b74bbe98 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -15,7 +15,8 @@ path = "src/lib.rs" anyhow = "1" clap = { version = "4", features = ["derive"] } codex-ansi-escape = { path = "../ansi-escape" } -codex-core = { path = "../core", features = ["cli"] } +codex-core = { path = "../core" } +codex-common = { path = "../common", features = ["cli", "elapsed"] } color-eyre = "0.6.3" crossterm = "0.28.1" mcp-types = { path = "../mcp-types" } diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index b180c503d1..c260caa9f4 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -1,6 +1,6 @@ use clap::Parser; -use codex_core::ApprovalModeCliArg; -use codex_core::SandboxPermissionOption; +use codex_common::ApprovalModeCliArg; +use codex_common::SandboxPermissionOption; use std::path::PathBuf; #[derive(Parser, Debug)] diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 87bbd167b1..92859af286 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -1,4 +1,5 @@ use codex_ansi_escape::ansi_escape_line; +use codex_common::elapsed::format_duration; use codex_core::config::Config; use codex_core::protocol::FileChange; use ratatui::prelude::*; @@ -132,7 +133,12 @@ impl HistoryCell { // Title depends on whether we have output yet. let title_line = Line::from(vec![ "command".magenta(), - format!(" (code: {}, duration: {:?})", exit_code, duration).dim(), + format!( + " (code: {}, duration: {})", + exit_code, + format_duration(duration) + ) + .dim(), ]); lines.push(title_line); @@ -201,11 +207,11 @@ impl HistoryCell { success: bool, result: Option, ) -> Self { - let duration = start.elapsed(); + let duration = format_duration(start.elapsed()); let status_str = if success { "success" } else { "failed" }; let title_line = Line::from(vec![ "tool".magenta(), - format!(" {fq_tool_name} ({status_str}, duration: {:?})", duration).dim(), + format!(" {fq_tool_name} ({status_str}, duration: {})", duration).dim(), ]); let mut lines: Vec> = Vec::new();