diff --git a/codex-rs/core/tests/suite/guardian_history.rs b/codex-rs/core/tests/suite/guardian_history.rs index 8bf33956a8..6f50f1cd87 100644 --- a/codex-rs/core/tests/suite/guardian_history.rs +++ b/codex-rs/core/tests/suite/guardian_history.rs @@ -207,6 +207,116 @@ async fn guardian_history_survives_restart_and_user_fork( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn guardian_history_uses_deltas_between_eviction_batches() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_wine_exec!( + Ok(()), + "Guardian approval actions require host-native paths" + ); + let server = start_mock_server().await; + let test = test_codex() + .with_config(|config| { + config.features.enable(Feature::TokenBudget).unwrap(); + config + .features + .disable(Feature::GuardianThreadContext) + .expect("use the retained legacy history"); + config.update_plan_enabled = true; + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config.approvals_reviewer = ApprovalsReviewer::AutoReview; + }) + .build_with_auto_env(&server) + .await?; + let plan = r#"{"plan":[{"step":"inspect the repository","status":"completed"}]}"#; + let mut inspection: Vec<_> = (0..130) + .map(|index| ev_function_call(&format!("initial-{index}"), "update_plan", plan)) + .collect(); + inspection.push(ev_completed("inspection")); + mount_sse_sequence( + &server, + vec![sse(inspection), sse(vec![ev_completed("inspected")])], + ) + .await; + let restriction = "Only inspect the repository; do not publish it."; + test.submit_text_turn(restriction).await?; + test.codex.submit(Op::Compact).await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let mut responses = Vec::new(); + for index in 0..3 { + if index == 1 { + // Force eviction after the first review has saved its transcript cursor. + let mut traffic: Vec<_> = (0..130) + .map(|index| ev_function_call(&format!("later-{index}"), "update_plan", plan)) + .collect(); + traffic.push(ev_completed("more-inspection")); + responses.push(sse(traffic)); + } + responses.push(sse(vec![ + ev_function_call( + &format!("review-{index}"), + "exec_command", + &json!({ + "cmd": format!("echo review-{index}"), + "sandbox_permissions": "require_escalated" + }) + .to_string(), + ), + ev_completed(&format!("action-{index}")), + ])); + // Three consecutive denials would interrupt the parent turn before its final response. + let decision = if index == 2 { + r#"{"risk_level":"low","user_authorization":"high","outcome":"allow"}"# + } else { + r#"{"outcome":"deny"}"# + }; + responses.push(sse(vec![ + ev_assistant_message(&format!("decision-{index}"), decision), + ev_completed(&format!("reviewed-{index}")), + ])); + } + responses.push(sse(vec![ev_completed("done")])); + let reviews = mount_sse_sequence(&server, responses).await; + test.submit_text_turn("Continue inspecting.").await?; + let requests = reviews.requests(); + let guardian_requests = requests + .iter() + .filter(|request| request.body_json()["client_metadata"]["x-openai-subagent"] == "guardian") + .collect::>(); + let prompts = guardian_requests + .iter() + .map(|request| { + request + .message_input_text_groups("user") + .last() + .expect("Guardian review prompt") + .join("") + }) + .collect::>(); + assert_eq!( + prompts + .iter() + .map(|prompt| ( + prompt.contains(">>> TRANSCRIPT START\n"), + prompt.contains(">>> TRANSCRIPT DELTA START\n"), + )) + .collect::>(), + vec![(true, false), (true, false), (false, true)] + ); + assert!(prompts[1].contains(restriction)); + assert!(prompts[2].contains("review-2")); + assert_eq!( + guardian_requests[1].body_json()["client_metadata"]["thread_id"], + guardian_requests[2].body_json()["client_metadata"]["thread_id"] + ); + test.codex.shutdown_and_wait().await?; + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn guardian_history_survives_compaction_and_eviction_but_not_rollback() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/guardian-context/src/history.rs b/codex-rs/guardian-context/src/history.rs index b79715e0b3..7eef8d6743 100644 --- a/codex-rs/guardian-context/src/history.rs +++ b/codex-rs/guardian-context/src/history.rs @@ -4,6 +4,8 @@ //! Hosts append original items, restore bounded checkpoints, and trim rolled-back turns. //! Clones share immutable payloads; eviction changes the generation so readers cannot //! reuse an offset into a different retained prefix. Prompt selection remains caller-owned. +//! Non-user overflow drops the oldest half, leaving room for appends without more evictions. +//! User messages retain as much history as their separate limits allow. use std::collections::VecDeque; use std::io; @@ -47,6 +49,7 @@ impl TranscriptHistory { } /// Appends one original item, evicting only older entries of the same kind. + /// Non-user overflow removes at least half the existing entries; byte limits may need more. /// Oversized user images fall back to bounded text; other oversized items are skipped. pub fn record(&mut self, item: &ResponseItem) { let mut size = BoundedSize { @@ -117,9 +120,14 @@ impl TranscriptHistory { (count + 1, bytes + size) }); if count >= MAX_ITEMS_PER_KIND || bytes > MAX_BYTES_PER_KIND { + let retained_count = if is_user { + MAX_ITEMS_PER_KIND - 1 + } else { + count / 2 + }; self.items.retain(|(item, size)| { if item.is_user_message() == is_user - && (count >= MAX_ITEMS_PER_KIND || bytes > MAX_BYTES_PER_KIND) + && (count > retained_count || bytes > MAX_BYTES_PER_KIND) { count -= 1; bytes -= size; diff --git a/codex-rs/guardian-context/src/history_tests.rs b/codex-rs/guardian-context/src/history_tests.rs index cd0b9daf98..1d6dc4653d 100644 --- a/codex-rs/guardian-context/src/history_tests.rs +++ b/codex-rs/guardian-context/src/history_tests.rs @@ -49,7 +49,7 @@ fn each_kind_evicts_its_own_oldest_entries_without_reordering() { history.items().collect::>(), users[..2] .iter() - .chain(&tools) + .chain(&tools[63..]) .chain(&users[2..]) .collect::>() ); @@ -62,7 +62,7 @@ fn each_kind_evicts_its_own_oldest_entries_without_reordering() { } assert_eq!( history.items().collect::>(), - tools.iter().chain(&newer_users).collect::>() + tools[63..].iter().chain(&newer_users).collect::>() ); assert!(history.generation() > generation); }