From 16309d6b68546c392fbec3d30fb65e404bd8e3fa Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 8 Sep 2025 21:39:18 -0700 Subject: [PATCH 1/2] chore: try switching to `cargo nextest` to speed up CI builds (#3323) I started looking at https://nexte.st/ because I was interested in a test harness that lets a test dynamically declare itself "skipped," which would be a nice alternative to this pattern: https://github.com/openai/codex/blob/4c46490e53076e807fd2068aa68c5a0e636d8b5f/codex-rs/core/tests/suite/cli_stream.rs#L22-L27 ChatGPT pointed me at https://nexte.st/, which also claims to be "up to 3x as fast as cargo test." Locally, in `codex-rs`, I see - `cargo nextest run` finishes in 19s - `cargo test` finishes in 37s Though looking at CI, the wins are quite as big, presumably because my laptop has more cores than our GitHub runners (which is a separate issue...). Comparing the [CI jobs from this PR](https://github.com/openai/codex/actions/runs/17561325162/job/49878216246?pr=3323) with that of a [recent open PR](https://github.com/openai/codex/actions/runs/17561066581/job/49877342753?pr=3321): | | `cargo test` | `cargo nextest` | | ----------------------------------------------- | ------------ | --------------- | | `macos-14 - aarch64-apple-darwin` | 2m16s | 1m51s | | `macos-14 - aarch64-apple-darwin` | 5m04s | 3m44s | | `ubuntu-24.04 - x86_64-unknown-linux-musl` | 2m02s | 1m56s | | `ubuntu-24.04-arm - aarch64-unknown-linux-musl` | 2m01s | 1m35s | | `windows-latest - x86_64-pc-windows-msvc` | 3m07s | 2m53s | | `windows-11-arm - aarch64-pc-windows-msvc` | 3m10s | 2m45s | I thought that, to start, we would only make this change in CI before declaring it the "official" way for the team to run the test suite. Though unfortunately, I do not believe that `cargo nextest` _actually_ supports a dynamic skip feature, so I guess I'll have to keep looking? Some related discussions: - https://internals.rust-lang.org/t/pre-rfc-skippable-tests/14611 - https://internals.rust-lang.org/t/skippable-tests/21260 --- .github/workflows/rust-ci.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index e774489301..489c5bb277 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -74,7 +74,7 @@ jobs: steps: - uses: actions/checkout@v5 - uses: dtolnay/rust-toolchain@1.89 - - uses: taiki-e/install-action@v2 + - uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 with: tool: cargo-shear version: 1.5.1 @@ -178,12 +178,17 @@ jobs: find . -name Cargo.toml -mindepth 2 -maxdepth 2 -print0 \ | xargs -0 -n1 -I{} bash -c 'cd "$(dirname "{}")" && cargo check --profile ${{ matrix.profile }}' - - name: cargo test + - uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 + with: + tool: nextest + version: 0.9.103 + + - name: tests id: test - # `cargo test` takes too long for release builds to run them on every PR + # Tests take too long for release builds to run them on every PR. if: ${{ matrix.profile != 'release' }} continue-on-error: true - run: cargo test --all-features --target ${{ matrix.target }} --profile ${{ matrix.profile }} + run: cargo nextest run --all-features --target ${{ matrix.target }} env: RUST_BACKTRACE: 1 From 15746d80f46ccfcfd43158e9ea0e8510c402737c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 8 Sep 2025 22:03:04 -0700 Subject: [PATCH 2/2] fix: include rollout_path in NewConversationResponse --- codex-rs/Cargo.lock | 1 + codex-rs/core/src/codex.rs | 2 ++ codex-rs/core/src/rollout/recorder.rs | 14 +++++++++++--- .../exec/src/event_processor_with_human_output.rs | 1 + codex-rs/mcp-server/src/codex_message_processor.rs | 1 + codex-rs/mcp-server/src/outgoing_message.rs | 6 ++++++ .../tests/suite/codex_message_processor_flow.rs | 1 + .../mcp-server/tests/suite/create_conversation.rs | 1 + codex-rs/protocol/Cargo.toml | 1 + codex-rs/protocol/src/mcp_protocol.rs | 1 + codex-rs/protocol/src/protocol.rs | 5 +++++ codex-rs/tui/src/chatwidget/tests.rs | 4 +++- codex-rs/tui/src/history_cell.rs | 1 + 13 files changed, 35 insertions(+), 4 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index e76ea6bb89..cda2e81d17 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -830,6 +830,7 @@ dependencies = [ "strum 0.27.2", "strum_macros 0.27.2", "sys-locale", + "tempfile", "tracing", "ts-rs", "uuid", diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 42960b27aa..cc8768d79b 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -416,6 +416,7 @@ impl Session { error!("failed to initialize rollout recorder: {e:#}"); anyhow::anyhow!("failed to initialize rollout recorder: {e:#}") })?; + let rollout_path = rollout_recorder.rollout_path.clone(); // Create the mutable state for the Session. let state = State { history: ConversationHistory::new(), @@ -509,6 +510,7 @@ impl Session { history_log_id, history_entry_count, initial_messages, + rollout_path, }), }) .chain(post_session_configured_error_events.into_iter()); diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 907583353c..0e0af8990b 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -72,6 +72,7 @@ pub struct SavedSession { #[derive(Clone)] pub struct RolloutRecorder { tx: Sender, + pub(crate) rollout_path: PathBuf, } #[derive(Clone)] @@ -119,13 +120,14 @@ impl RolloutRecorder { /// cannot be created or the rollout file cannot be opened we return the /// error so the caller can decide whether to disable persistence. pub async fn new(config: &Config, params: RolloutRecorderParams) -> std::io::Result { - let (file, meta) = match params { + let (file, rollout_path, meta) = match params { RolloutRecorderParams::Create { conversation_id, instructions, } => { let LogFileInfo { file, + path, conversation_id: session_id, timestamp, } = create_log_file(config, conversation_id)?; @@ -140,6 +142,7 @@ impl RolloutRecorder { ( tokio::fs::File::from_std(file), + path, Some(SessionMeta { timestamp, id: session_id, @@ -150,8 +153,9 @@ impl RolloutRecorder { RolloutRecorderParams::Resume { path } => ( tokio::fs::OpenOptions::new() .append(true) - .open(path) + .open(&path) .await?, + path, None, ), }; @@ -169,7 +173,7 @@ impl RolloutRecorder { // driver instead of blocking the runtime. tokio::task::spawn(rollout_writer(file, rx, meta, cwd)); - Ok(Self { tx }) + Ok(Self { tx, rollout_path }) } pub(crate) async fn record_items(&self, items: &[ResponseItem]) -> std::io::Result<()> { @@ -289,6 +293,9 @@ struct LogFileInfo { /// Opened file handle to the rollout file. file: File, + /// Full path to the rollout file. + path: PathBuf, + /// Session ID (also embedded in filename). conversation_id: ConversationId, @@ -328,6 +335,7 @@ fn create_log_file( Ok(LogFileInfo { file, + path, conversation_id, timestamp, }) diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 7c5e6b147c..ae4708976f 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -523,6 +523,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { history_log_id: _, history_entry_count: _, initial_messages: _, + rollout_path: _, } = session_configured_event; ts_println!( diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index e5f44b5687..2169a8f27f 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -528,6 +528,7 @@ impl CodexMessageProcessor { let response = NewConversationResponse { conversation_id, model: session_configured.model, + rollout_path: session_configured.rollout_path, }; self.outgoing.send_response(request_id, response).await; } diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index 537d29db62..5ce2e99423 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -262,6 +262,7 @@ mod tests { use codex_protocol::mcp_protocol::LoginChatGptCompleteNotification; use pretty_assertions::assert_eq; use serde_json::json; + use tempfile::NamedTempFile; use uuid::Uuid; use super::*; @@ -272,6 +273,7 @@ mod tests { let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx); let conversation_id = ConversationId::new(); + let rollout_file = NamedTempFile::new().unwrap(); let event = Event { id: "1".to_string(), msg: EventMsg::SessionConfigured(SessionConfiguredEvent { @@ -280,6 +282,7 @@ mod tests { history_log_id: 1, history_entry_count: 1000, initial_messages: None, + rollout_path: rollout_file.path().to_path_buf(), }), }; @@ -305,12 +308,14 @@ mod tests { let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx); let conversation_id = ConversationId::new(); + let rollout_file = NamedTempFile::new().unwrap(); let session_configured_event = SessionConfiguredEvent { session_id: conversation_id, model: "gpt-4o".to_string(), history_log_id: 1, history_entry_count: 1000, initial_messages: None, + rollout_path: rollout_file.path().to_path_buf(), }; let event = Event { id: "1".to_string(), @@ -340,6 +345,7 @@ mod tests { "history_log_id": session_configured_event.history_log_id, "history_entry_count": session_configured_event.history_entry_count, "type": "session_configured", + "rollout_path": rollout_file.path().to_path_buf(), } }); assert_eq!(params.unwrap(), expected_params); diff --git a/codex-rs/mcp-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/mcp-server/tests/suite/codex_message_processor_flow.rs index 092b135291..7737c49af3 100644 --- a/codex-rs/mcp-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/mcp-server/tests/suite/codex_message_processor_flow.rs @@ -90,6 +90,7 @@ async fn test_codex_jsonrpc_conversation_flow() { let NewConversationResponse { conversation_id, model, + rollout_path: _, } = new_conv_resp; assert_eq!(model, "mock-model"); diff --git a/codex-rs/mcp-server/tests/suite/create_conversation.rs b/codex-rs/mcp-server/tests/suite/create_conversation.rs index 81cc1f3ee7..5071868c85 100644 --- a/codex-rs/mcp-server/tests/suite/create_conversation.rs +++ b/codex-rs/mcp-server/tests/suite/create_conversation.rs @@ -59,6 +59,7 @@ async fn test_conversation_create_and_send_message_ok() { let NewConversationResponse { conversation_id, model, + rollout_path: _, } = to_response::(new_conv_resp) .expect("deserialize newConversation response"); assert_eq!(model, "o3"); diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index d77da7fc19..f88297a07b 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -28,6 +28,7 @@ uuid = { version = "1", features = ["serde", "v4"] } [dev-dependencies] pretty_assertions = "1.4.1" +tempfile = "3" [package.metadata.cargo-shear] # Required because the not imported as strum_macros in non-nightly builds. diff --git a/codex-rs/protocol/src/mcp_protocol.rs b/codex-rs/protocol/src/mcp_protocol.rs index 2af1a95166..70e334036f 100644 --- a/codex-rs/protocol/src/mcp_protocol.rs +++ b/codex-rs/protocol/src/mcp_protocol.rs @@ -203,6 +203,7 @@ pub struct NewConversationParams { pub struct NewConversationResponse { pub conversation_id: ConversationId, pub model: String, + pub rollout_path: PathBuf, } #[derive(Serialize, Deserialize, Debug, Clone, TS)] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index eeaa72cd93..363a33a93d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -958,6 +958,8 @@ pub struct SessionConfiguredEvent { /// When present, UIs can use these to seed the history. #[serde(skip_serializing_if = "Option::is_none")] pub initial_messages: Option>, + + pub rollout_path: PathBuf, } /// User's decision in response to an ExecApprovalRequest. @@ -1020,12 +1022,14 @@ pub enum TurnAbortReason { #[cfg(test)] mod tests { use super::*; + use tempfile::NamedTempFile; /// Serialize Event to verify that its JSON representation has the expected /// amount of nesting. #[test] fn serialize_event() { let conversation_id = ConversationId(uuid::uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8")); + let rollout_file = NamedTempFile::new().unwrap(); let event = Event { id: "1234".to_string(), msg: EventMsg::SessionConfigured(SessionConfiguredEvent { @@ -1034,6 +1038,7 @@ mod tests { history_log_id: 0, history_entry_count: 0, initial_messages: None, + rollout_path: rollout_file.path().to_path_buf(), }), }; let serialized = serde_json::to_string(&event).unwrap(); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 3b61203399..113864dba7 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -35,6 +35,7 @@ use std::fs::File; use std::io::BufRead; use std::io::BufReader; use std::path::PathBuf; +use tempfile::NamedTempFile; use tokio::sync::mpsc::unbounded_channel; fn test_config() -> Config { @@ -133,7 +134,7 @@ fn resumed_initial_messages_render_history() { let (mut chat, mut rx, _ops) = make_chatwidget_manual(); let conversation_id = ConversationId::new(); - + let rollout_file = NamedTempFile::new().unwrap(); let configured = codex_core::protocol::SessionConfiguredEvent { session_id: conversation_id, model: "test-model".to_string(), @@ -148,6 +149,7 @@ fn resumed_initial_messages_render_history() { message: "assistant reply".to_string(), }), ]), + rollout_path: rollout_file.path().to_path_buf(), }; chat.handle_codex_event(Event { diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 92bb076225..a7be1d6560 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -605,6 +605,7 @@ pub(crate) fn new_session_info( history_log_id: _, history_entry_count: _, initial_messages: _, + rollout_path: _, } = event; if is_first_event { let cwd_str = match relativize_to_home(&config.cwd) {