From 2e57a490ccbbda5b9ac30d28bafed7ea74099bdd Mon Sep 17 00:00:00 2001 From: anais Date: Tue, 23 Jun 2026 12:11:30 -0700 Subject: [PATCH] fix compact resume review feedback --- .../core/tests/suite/compact_resume_fork.rs | 30 ------------------- codex-rs/rollout/src/list.rs | 3 +- codex-rs/rollout/src/tests.rs | 11 ++++++- 3 files changed, 11 insertions(+), 33 deletions(-) diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index b0edc4b3fb..1c4b1e4f73 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -13,7 +13,6 @@ use codex_core::CodexThread; use codex_core::ThreadManager; use codex_core::compact::SUMMARIZATION_PROMPT; use codex_core::config::Config; -use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_login::CodexAuth; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -50,10 +49,6 @@ use wiremock::MockServer; const AFTER_SECOND_RESUME: &str = "AFTER_SECOND_RESUME"; const AFTER_ROLLBACK: &str = "AFTER_ROLLBACK"; -fn network_disabled() -> bool { - std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() -} - fn body_contains_text(body: &str, text: &str) -> bool { body.contains(&json_fragment(text)) } @@ -132,11 +127,6 @@ fn normalize_compact_prompts(requests: &mut [Value]) { /// Scenario: compact an initial conversation, resume it, fork one turn back, and /// ensure the model-visible history matches expectations at each request. async fn compact_resume_and_fork_preserve_model_history_view() { - if network_disabled() { - println!("Skipping test because network is disabled in this sandbox"); - return; - } - // 1. Arrange mocked SSE responses for the initial compact/resume/fork flow. let server = MockServer::start().await; let request_log = mount_initial_flow(&server).await; @@ -288,11 +278,6 @@ async fn compact_resume_and_fork_preserve_model_history_view() { /// Scenario: a compacted resume history slice should reconstruct the same /// model-visible context when the next user turn runs. async fn compacted_resume_history_slice_preserves_next_turn_context() -> Result<()> { - if network_disabled() { - println!("Skipping test because network is disabled in this sandbox"); - return Ok(()); - } - const AFTER_COMPACTED_SLICE_RESUME: &str = "AFTER_COMPACTED_SLICE_RESUME"; let server = MockServer::start().await; @@ -355,11 +340,6 @@ async fn compacted_resume_history_slice_preserves_next_turn_context() -> Result< /// Scenario: after the forked branch is compacted, resuming again should reuse /// the compacted history and only append the new user message. async fn compact_resume_after_second_compaction_preserves_history() -> Result<()> { - if network_disabled() { - println!("Skipping test because network is disabled in this sandbox"); - return Ok(()); - } - // 1. Arrange mocked SSE responses as a single ordered stream so assertions // observe the real request sequence instead of per-mock duplicate captures. let server = MockServer::start().await; @@ -493,11 +473,6 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() /// append-only history from the rollout file and keep earlier compacted /// history visible. async fn snapshot_rollback_past_compaction_replays_append_only_history() -> Result<()> { - if network_disabled() { - println!("Skipping test because network is disabled in this sandbox"); - return Ok(()); - } - const EDITED_AFTER_COMPACT: &str = "EDITED_AFTER_COMPACT"; const SECOND_REPLY: &str = "SECOND_REPLY"; @@ -583,11 +558,6 @@ async fn snapshot_rollback_past_compaction_replays_append_only_history() -> Resu /// diffs should trim those context updates so the next request includes them /// only once. async fn snapshot_rollback_followup_turn_trims_context_updates() -> Result<()> { - if network_disabled() { - println!("Skipping test because network is disabled in this sandbox"); - return Ok(()); - } - const MODEL: &str = "gpt-5.4"; const TURN_ONE_USER: &str = "turn 1 user"; const TURN_TWO_USER: &str = "turn 2 user"; diff --git a/codex-rs/rollout/src/list.rs b/codex-rs/rollout/src/list.rs index 70837c1f54..ffb3c64b29 100644 --- a/codex-rs/rollout/src/list.rs +++ b/codex-rs/rollout/src/list.rs @@ -774,8 +774,7 @@ async fn build_thread_item( let mut summary = read_head_summary(&path, HEAD_RECORD_LIMIT) .await .unwrap_or_default(); - if summary.saw_session_meta && summary.preview.is_none() && summary.has_fork_parent_rollout_ref - { + if summary.saw_session_meta && summary.has_fork_parent_rollout_ref { if let Ok(expanded_summary) = read_expanded_rollout_summary(&path).await { if expanded_summary.preview.is_some() { summary = expanded_summary; diff --git a/codex-rs/rollout/src/tests.rs b/codex-rs/rollout/src/tests.rs index 06cac3151f..a1fa67b2fa 100644 --- a/codex-rs/rollout/src/tests.rs +++ b/codex-rs/rollout/src/tests.rs @@ -1399,7 +1399,16 @@ async fn read_thread_item_from_cow_child_uses_parent_preview() -> Result<()> { "fork_parent_rollout_byte_len": parent_byte_len, } }); - fs::write(&child_path, format!("{child_meta}\n"))?; + let child_user_event = serde_json::json!({ + "timestamp": child_ts, + "type": "event_msg", + "payload": { + "type": "user_message", + "message": "Child suffix user", + "kind": "plain" + } + }); + fs::write(&child_path, format!("{child_meta}\n{child_user_event}\n"))?; let item = read_thread_item_from_rollout(child_path.clone()) .await