From 305eed102d6ab5fc1228fec0737ba240eb29826b Mon Sep 17 00:00:00 2001 From: jif Date: Mon, 31 Aug 2026 14:23:24 +0000 Subject: [PATCH] Preserve Guardian user answers across compaction (#41852) ## Why Compaction removes the original `request_user_input` tool call from model-visible history, causing its trusted user answer to be omitted from subsequent Guardian reviews. ## What changed Match recorded user answers against retained review history so they remain available through compaction and review-history eviction. Explicit rollback still removes the associated answer. ## Testing Extend the Guardian history integration test to verify trusted user answers survive compaction and eviction but not rollback. GitOrigin-RevId: 00681ee8df51cf36421b6fdc5446565dff3776fe --- .../src/context/guardian_review_evidence.rs | 4 +- codex-rs/core/tests/suite/guardian_history.rs | 66 ++++++++++++++++++- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/context/guardian_review_evidence.rs b/codex-rs/core/src/context/guardian_review_evidence.rs index 1250652b05..d46e1ba58c 100644 --- a/codex-rs/core/src/context/guardian_review_evidence.rs +++ b/codex-rs/core/src/context/guardian_review_evidence.rs @@ -95,14 +95,14 @@ impl GuardianReviewEvidence { } } - /// Returns bounded answers whose original, host-observed tool calls remain in history. + /// Returns bounded answers whose original, host-observed tool calls remain in review history. pub fn user_input_fragments(&self, history: &dyn ConversationHistorySnapshot) -> Vec { let state = self.0.lock().unwrap_or_else(PoisonError::into_inner); state .user_inputs .iter() .filter(|(recorded_call_id, _)| { - history.items().any(|item| { + history.review_items().any(|item| { matches!( item, ResponseItem::FunctionCall { call_id, .. } diff --git a/codex-rs/core/tests/suite/guardian_history.rs b/codex-rs/core/tests/suite/guardian_history.rs index 3db8fa96b6..f441d0c1e9 100644 --- a/codex-rs/core/tests/suite/guardian_history.rs +++ b/codex-rs/core/tests/suite/guardian_history.rs @@ -1,12 +1,16 @@ //! Exercises retained review history through compaction, eviction, and rollback. use anyhow::Result; +use codex_core::TurnInputRequest; use codex_core::config::Constrained; use codex_features::Feature; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; +use codex_protocol::request_user_input::RequestUserInputAnswer; +use codex_protocol::request_user_input::RequestUserInputResponse; +use codex_protocol::user_input::UserInput; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -17,7 +21,10 @@ use core_test_support::skip_if_no_network; use core_test_support::skip_if_wine_exec; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; +use core_test_support::wait_for_event_match; use pretty_assertions::assert_eq; +use serde_json::json; +use std::collections::HashMap; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn guardian_history_survives_compaction_and_eviction_but_not_rollback() -> Result<()> { @@ -30,6 +37,10 @@ async fn guardian_history_survives_compaction_and_eviction_but_not_rollback() -> let test = test_codex() .with_config(|config| { config.features.enable(Feature::TokenBudget).unwrap(); + config + .features + .enable(Feature::DefaultModeRequestUserInput) + .unwrap(); config.update_plan_enabled = true; config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); config.approvals_reviewer = ApprovalsReviewer::AutoReview; @@ -48,6 +59,20 @@ async fn guardian_history_survives_compaction_and_eviction_but_not_rollback() -> sse(inspection), sse(vec![ ev_function_call("inspect-latest", "update_plan", plan), + ev_function_call( + "confirm-publish", + "request_user_input", + &json!({ + "questions": [{ + "id": "publish", "header": "Publish", "question": "May I publish?", + "options": [ + {"label": "Yes", "description": "Publish the change."}, + {"label": "No", "description": "Keep the change local."} + ] + }] + }) + .to_string(), + ), ev_completed("latest-inspection"), ]), sse(vec![ @@ -57,8 +82,34 @@ async fn guardian_history_survives_compaction_and_eviction_but_not_rollback() -> ], ) .await; - test.submit_text_turn("Only publish to a private repository.") + test.codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "Only publish to a private repository.".into(), + text_elements: Vec::new(), + }])) .await?; + let question = wait_for_event_match(&test.codex, |event| match event { + EventMsg::RequestUserInput(request) => Some(request.clone()), + _ => None, + }) + .await; + test.codex + .submit(Op::UserInputAnswer { + id: question.turn_id, + response: RequestUserInputResponse { + answers: HashMap::from([( + "publish".to_owned(), + RequestUserInputAnswer { + answers: vec!["Do not publish anything.".to_owned()], + }, + )]), + }, + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; test.codex.submit(Op::Compact).await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) @@ -96,6 +147,14 @@ async fn guardian_history_survives_compaction_and_eviction_but_not_rollback() -> let transcript = serde_json::to_string(&guardian.input())?; assert!(transcript.contains(prompt)); if retained { + let trusted_answers = transcript + .split_once(">>> TRUSTED USER ANSWERS START") + .expect("trusted answers survive compaction") + .1 + .split_once(">>> TRUSTED USER ANSWERS END") + .expect("trusted answers end marker") + .0; + assert!(trusted_answers.contains("user: Do not publish anything.")); let positions = [ "Only publish to a private repository.", "tool update_plan call", @@ -114,7 +173,8 @@ async fn guardian_history_survives_compaction_and_eviction_but_not_rollback() -> requests[0] .input() .iter() - .all(|item| item["call_id"] != "inspect-0") + .all(|item| item["call_id"] != "inspect-0" + && item["call_id"] != "confirm-publish") ); test.codex.ensure_rollout_materialized().await; test.codex @@ -125,6 +185,8 @@ async fn guardian_history_survives_compaction_and_eviction_but_not_rollback() -> }) .await; } else { + assert!(!transcript.contains(">>> TRUSTED USER ANSWERS START")); + assert!(!transcript.contains("Do not publish anything.")); assert!(!transcript.contains("Only publish to a private repository.")); assert!(!transcript.contains("tool update_plan call")); assert!(!transcript.contains("tool update_plan result"));