diff --git a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs index 9e025953ea..f8feea4a33 100644 --- a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs +++ b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs @@ -1187,21 +1187,46 @@ async fn guardian_v2_routes_scoped_tool_approvals( vec![ json!(["additional_tools", "developer", null]), json!(["message", "developer", ["guardian.classifier_instructions"]]), + json!(["message", "user", null]), json!(["message", "developer", null]), json!(["message", "developer", ["guardian.trusted_tool"]]), json!(["message", "developer", ["guardian.trusted_skills"]]), json!(["message", "user", null]), ], ); - let content = input[5]["content"].as_array().expect("untrusted evidence"); - let texts = content + let history = input[2]["content"].as_array().expect("untrusted history"); + let history_texts = history .iter() .filter_map(|item| item["text"].as_str()) .collect::>(); - assert_eq!(texts.first().copied(), Some(">>> TRANSCRIPT START\n")); - assert!(texts.iter().any(|text| text.contains(USER_CONTEXT))); - assert!(texts.iter().any(|text| text.contains("guardian-0"))); - assert_eq!(texts.last().copied(), Some(">>> APPROVAL REQUEST END\n")); + assert_eq!( + history_texts.first().copied(), + Some(">>> TRANSCRIPT START\n") + ); + assert!(history_texts.iter().any(|text| text.contains(USER_CONTEXT))); + assert!(history_texts.iter().any(|text| text.contains("guardian-0"))); + assert_eq!( + history_texts.last().copied(), + Some(">>> TRANSCRIPT END\n\n") + ); + assert!(history.iter().all(|item| item["type"] == "input_text")); + + let content = input[6]["content"].as_array().expect("action and images"); + let action_texts = content + .iter() + .filter_map(|item| item["text"].as_str()) + .collect::>(); + assert_eq!( + &action_texts[..2], + &[ + "The Codex agent has requested the following action:\n", + ">>> APPROVAL REQUEST START\n", + ], + ); + assert_eq!( + action_texts.last().copied(), + Some(">>> APPROVAL REQUEST END\n") + ); assert!( content[..content.len() - 1] .iter() diff --git a/codex-rs/guardian-context/src/composition.rs b/codex-rs/guardian-context/src/composition.rs index 9518409be5..474dcff617 100644 --- a/codex-rs/guardian-context/src/composition.rs +++ b/codex-rs/guardian-context/src/composition.rs @@ -3,6 +3,8 @@ //! framing, message boundaries and section placement, without retaining history. //! Each content item keeps its selection policy until transport conversion. //! Long text splits losslessly only after admission, preserving whole-entry selection. +//! Action-specific attestations follow the transcript so they do not invalidate +//! the reusable history prefix when previous decisions or tool evidence change. use codex_context_fragments::ContextualUserFragment; use codex_protocol::models::ContentItem; @@ -119,31 +121,31 @@ impl CollectedContext { for section in self.sections { let (position, id, delivery) = match section { ContextSection::PreviousReviews(reviews) => ( - 1, + 6, "previous_reviews", SectionDelivery::Message(Box::new(reviews.into_message())), ), ContextSection::TrustedTool(tool) => ( - 2, + 7, "trusted_tool", SectionDelivery::Message(Box::new(ContextualUserFragment::into(tool))), ), ContextSection::TrustedSkills(skills) => ( - 3, + 8, "trusted_skills", SectionDelivery::Message(Box::new(ContextualUserFragment::into(skills))), ), ContextSection::RootConversation { items } => { - (4, "root_conversation", text_content(items)) + (1, "root_conversation", text_content(items)) } ContextSection::SenderUserMessages { items } => { - (4, "sender_user_messages", text_content(items)) + (1, "sender_user_messages", text_content(items)) } ContextSection::RetainedUserInstructions { items } => { - (5, "retained_user_instructions", text_content(items)) + (2, "retained_user_instructions", text_content(items)) } ContextSection::TrustedUserAnswers { items } => { - (6, "trusted_user_answers", text_content(items)) + (3, "trusted_user_answers", text_content(items)) } ContextSection::ConversationTranscript { .. } => { let transcript = @@ -173,7 +175,7 @@ impl CollectedContext { items.push(Budgeted::required(format!("\n{note}\n"))); } ( - 7, + 4, "conversation_transcript", SectionDelivery::UserContent( items @@ -187,7 +189,7 @@ impl CollectedContext { ) } ContextSection::PermissionContext { items } => { - (8, "permissions", text_content(items)) + (5, "permissions", text_content(items)) } ContextSection::TranscriptImages(images) => { if images.omitted_bytes > 0 { diff --git a/codex-rs/guardian-context/tests/cache_prefix.rs b/codex-rs/guardian-context/tests/cache_prefix.rs new file mode 100644 index 0000000000..82573cc857 --- /dev/null +++ b/codex-rs/guardian-context/tests/cache_prefix.rs @@ -0,0 +1,139 @@ +//! Exercises cache-prefix stability through the public collection and composition API. + +use codex_context_fragments::ContextualUserFragment; +use codex_guardian_context::ActionPresentation; +use codex_guardian_context::ContextPresentation; +use codex_guardian_context::ContextProfile; +use codex_guardian_context::ContextTarget; +use codex_guardian_context::PlannedAction; +use codex_guardian_context::PlannedActionKind; +use codex_guardian_context::PreviousReviews; +use codex_guardian_context::SectionHistory; +use codex_guardian_context::SectionInput; +use codex_guardian_context::TrustedSkills; +use codex_guardian_context::TrustedTool; +use codex_guardian_context::default_registry; +use codex_history::RetainedContext; +use codex_history::RetainedInputSource; +use codex_history::RetainedUserMessage; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseItem; +use pretty_assertions::assert_eq; + +struct History { + items: Vec, + retained: Option, +} + +impl SectionHistory for History { + fn items(&self) -> Box + Send + '_> { + Box::new(self.items.iter()) + } + + fn retained_context(&self) -> Option<&RetainedContext> { + self.retained.as_ref() + } +} + +fn user_message(texts: Vec) -> ResponseItem { + ResponseItem::Message { + id: None, + role: "user".to_owned(), + content: texts + .into_iter() + .map(|text| ContentItem::InputText { text }) + .collect(), + phase: None, + internal_chat_message_metadata_passthrough: None, + } +} + +#[test] +fn changing_attestations_preserves_history_before_the_current_action() { + let instruction = "Inspect staging only. Do not publish."; + let mut retained = RetainedContext::default(); + retained.record_user_message( + RetainedUserMessage { + turn_id: "turn-1".to_owned(), + message_id: None, + text: instruction.to_owned(), + complete: true, + }, + RetainedInputSource::Local(None), + ); + // Legacy review history and Felix's retained, thread-owned history use the + // same composer. Neither may put changing attestations before that history. + for retained in [None, Some(retained)] { + let history = History { + items: vec![user_message(vec![instruction.to_owned()])], + retained, + }; + let profile = ContextProfile::asynchronous(); + let mut previous_prefix = None; + for generation in 0..2 { + let reviews = PreviousReviews::try_from_fragments(vec![format!( + "Host-attested decision {generation}: denied." + )]) + .unwrap(); + let tool = TrustedTool { + server: format!("server-{generation}"), + connector_id: None, + source: "user configuration".to_owned(), + }; + let skills = TrustedSkills { + paths: vec![format!("/skills/skill-{generation}/SKILL.md")], + }; + let action = PlannedAction { + json: format!(r#"{{"tool":"inspect","path":"file-{generation}"}}"#), + tool_descriptions: None, + kind: PlannedActionKind::Command, + reason: None, + }; + let collected = default_registry() + .prepare(&SectionInput { + target: ContextTarget::Async, + history: &history, + transcript: &profile.transcript, + root_conversation: &[], + trusted_user_answers: &[], + planned_action: Some(&action), + permissions: None, + previous_reviews: Some(&reviews), + trusted_tool: Some(&tool), + trusted_skill_paths: &skills.paths, + images: None, + node_repl: None, + }) + .unwrap(); + let transcript = profile.render_transcript( + collected.transcript_entries(), + /*entry_number_offset*/ 0, + ); + let messages = collected + .compose(ContextPresentation::Async, transcript) + .unwrap() + .into_messages(); + let (prefix, suffix) = messages.split_first().expect("history prefix"); + let ResponseItem::Message { role, content, .. } = prefix else { + panic!("history remains untrusted user evidence"); + }; + assert_eq!(role, "user"); + assert!(content.iter().any(|item| matches!( + item, + ContentItem::InputText { text } if text == ">>> TRANSCRIPT END\n\n" + ))); + if let Some(previous) = previous_prefix.replace(prefix.clone()) { + assert_eq!(prefix, &previous); + } + assert_eq!( + suffix, + [ + reviews.into_message(), + ContextualUserFragment::into(tool), + ContextualUserFragment::into(skills), + user_message(action.render(ActionPresentation::Async)), + ] + ); + } + } +}