diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 04f7c9938b..f836e9324b 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -1511,9 +1511,23 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() { ); let history = child_thread.session.clone_history().await; let history_items = history.raw_items().cloned().collect::>(); - let mut expected_final_answer = - assistant_message("parent final answer", Some(MessagePhase::FinalAnswer)); - expected_final_answer.set_turn_id_if_missing(&turn_context.sub_id); + let expected_final_answer = parent_thread + .session + .clone_history() + .await + .raw_items() + .find(|item| { + matches!( + item, + ResponseItem::Message { + role, + phase: Some(MessagePhase::FinalAnswer), + .. + } if role == "assistant" + ) + }) + .cloned() + .expect("parent final answer should be recorded"); let mut expected_developer_message = ResponseItem::Message { id: None, role: "developer".to_string(), diff --git a/codex-rs/core/src/image_preparation.rs b/codex-rs/core/src/image_preparation.rs index fd650251e1..d90106929f 100644 --- a/codex-rs/core/src/image_preparation.rs +++ b/codex-rs/core/src/image_preparation.rs @@ -6,8 +6,12 @@ use crate::context::ResizedImage; use crate::original_image_detail::can_request_original_image_detail; use codex_analytics::ImageDetailSetting; use codex_analytics::ImagePreparationMetadata; +use codex_context_fragments::AnnotatedContent; +use codex_context_fragments::set_annotated_content; +use codex_context_fragments::to_annotated_content; use codex_features::Feature; use codex_protocol::models::ContentItem; +use codex_protocol::models::ContentItemKind; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ImageDetail; @@ -121,15 +125,19 @@ pub(crate) fn prepare_response_items( }) }; for mut item in std::mem::take(items) { + let mut annotated_content = to_annotated_content(&mut item); let resize_notice = match &mut item { - ResponseItem::Message { role, content, .. } => { + ResponseItem::Message { role, .. } => { + let Some(mut content) = annotated_content.take() else { + continue; + }; let resized_images = prepare_message_content( - content, + &mut content, ImageOrigin { - message_role: Some(role), + message_role: Some(role.as_str()), item_id: None, }, - if role == "user" { + if role.as_str() == "user" { resize_notice_mode } else { ImageResizeNoticeMode::Disabled @@ -137,6 +145,7 @@ pub(crate) fn prepare_response_items( &mut metadata, mode, ); + let _ = set_annotated_content(&mut item, content); (!resized_images.is_empty()).then(|| { ImageResizeNotice::new(ImageResizeNoticeSource::UserMessage, resized_images) }) @@ -172,7 +181,7 @@ pub(crate) fn prepare_response_items( } fn prepare_message_content( - items: &mut [ContentItem], + items: &mut [AnnotatedContent], origin: ImageOrigin<'_>, resize_notice_mode: ImageResizeNoticeMode, metadata: &mut Vec, @@ -180,12 +189,12 @@ fn prepare_message_content( ) -> Vec { let image_count = items .iter() - .filter(|item| matches!(item, ContentItem::InputImage { .. })) + .filter(|item| matches!(item.content(), ContentItem::InputImage { .. })) .count(); let mut image_number = 0; let mut resized_images = Vec::new(); for item in items { - if let ContentItem::InputImage { image_url, detail } = item { + if let ContentItem::InputImage { image_url, detail } = item.content_mut() { image_number += 1; match prepare_image(image_url, detail, origin, metadata, mode) { Ok(Some(resize)) if resize_notice_mode == ImageResizeNoticeMode::Enabled => { @@ -201,9 +210,10 @@ fn prepare_message_content( Ok(_) => {} Err(error) => { warn!(%error, "failed to prepare message image"); - *item = ContentItem::InputText { - text: error.placeholder().to_string(), - }; + *item = AnnotatedContent::input_text( + error.placeholder(), + ContentItemKind("images.preparation_error".to_string()), + ); } } } diff --git a/codex-rs/core/src/image_preparation_tests.rs b/codex-rs/core/src/image_preparation_tests.rs index 192d11a69d..67b1f1d4df 100644 --- a/codex-rs/core/src/image_preparation_tests.rs +++ b/codex-rs/core/src/image_preparation_tests.rs @@ -52,6 +52,16 @@ fn preparation_preserves_small_image_bytes_and_replaces_remote_urls() { phase: None, internal_chat_message_metadata_passthrough: None, }]; + items.push(ResponseItem::Message { + id: None, + role: "developer".to_string(), + content: vec![ContentItem::InputImage { + image_url: "https://example.com/developer-image.png".to_string(), + detail: Some(ImageDetail::High), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }); prepare_response_items( &mut items, @@ -71,6 +81,25 @@ fn preparation_preserves_small_image_bytes_and_replaces_remote_urls() { }; assert_eq!(decoded_image(image_url).0, original_bytes); assert_eq!(text, REMOTE_IMAGE_URL_PLACEHOLDER); + assert_eq!( + &items[1], + &ResponseItem::Message { + id: None, + role: "developer".to_string(), + content: vec![ContentItem::InputText { + text: REMOTE_IMAGE_URL_PLACEHOLDER.to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: Some( + InternalChatMessageMetadataPassthrough { + content_item_kinds: Some(vec![ContentItemKind( + "images.preparation_error".to_string() + )]), + ..Default::default() + } + ), + } + ); } #[test] @@ -199,7 +228,16 @@ fn resize_notices_preserve_original_image_positions_and_skip_failed_images() { }, ], phase: None, - internal_chat_message_metadata_passthrough: None, + internal_chat_message_metadata_passthrough: Some( + InternalChatMessageMetadataPassthrough { + content_item_kinds: Some(vec![ + ContentItemKind("user.image".to_string()), + ContentItemKind("user.image".to_string()), + ContentItemKind("user.image".to_string()), + ]), + ..Default::default() + }, + ), }, ResponseItem::FunctionCallOutput { id: None, @@ -231,9 +269,25 @@ fn resize_notices_preserve_original_image_positions_and_skip_failed_images() { "" ); - let ResponseItem::Message { content, .. } = &items[0] else { + let ResponseItem::Message { + content, + internal_chat_message_metadata_passthrough, + .. + } = &items[0] + else { panic!("expected message"); }; + assert_eq!( + internal_chat_message_metadata_passthrough, + &Some(InternalChatMessageMetadataPassthrough { + content_item_kinds: Some(vec![ + ContentItemKind("user.image".to_string()), + ContentItemKind("images.preparation_error".to_string()), + ContentItemKind("user.image".to_string()), + ]), + ..Default::default() + }), + ); let [ ContentItem::InputImage { image_url: small_message_image_url, diff --git a/codex-rs/core/src/session/rollout_reconstruction_tests.rs b/codex-rs/core/src/session/rollout_reconstruction_tests.rs index 002b69265e..034c72c3cb 100644 --- a/codex-rs/core/src/session/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/session/rollout_reconstruction_tests.rs @@ -19,6 +19,7 @@ use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::WorldStateItem; use codex_protocol::security_risk::SecurityRiskScore; +use core_test_support::responses::strip_metadata_from_items; use pretty_assertions::assert_eq; use serde_json::json; use std::collections::BTreeMap; @@ -171,7 +172,9 @@ async fn record_initial_history_ignores_security_risk_scores() { .await; assert_eq!( - raw_history_items(&session.state.lock().await.clone_history()), + strip_metadata_from_items(&raw_history_items( + &session.state.lock().await.clone_history() + )), vec![user_item] ); } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index dfe2cea2b7..3c7920dd3e 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -259,7 +259,10 @@ fn user_message(text: &str) -> ResponseItem { text: text.to_string(), }], phase: None, - internal_chat_message_metadata_passthrough: None, + internal_chat_message_metadata_passthrough: Some(InternalChatMessageMetadataPassthrough { + content_item_kinds: Some(vec![ContentItemKind("unknown".to_string())]), + ..Default::default() + }), } } @@ -322,7 +325,10 @@ fn assistant_message(text: &str) -> ResponseItem { text: text.to_string(), }], phase: None, - internal_chat_message_metadata_passthrough: None, + internal_chat_message_metadata_passthrough: Some(InternalChatMessageMetadataPassthrough { + content_item_kinds: Some(vec![ContentItemKind("unknown".to_string())]), + ..Default::default() + }), } } @@ -2504,7 +2510,16 @@ async fn prepares_resumed_history_before_installing_it() { }, ], phase: None, - internal_chat_message_metadata_passthrough: None, + internal_chat_message_metadata_passthrough: Some( + InternalChatMessageMetadataPassthrough { + content_item_kinds: Some(vec![ + ContentItemKind("images.preparation_error".to_string()), + ContentItemKind("images.preparation_error".to_string()), + ContentItemKind("unknown".to_string()), + ]), + ..Default::default() + }, + ), }] ); assert_eq!( @@ -11184,30 +11199,14 @@ async fn sample_rollout( reconstruction_turn.model_info.truncation_policy.into(), ); - let user1 = ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "first user".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }; + let user1 = user_message("first user"); live_history.record_items( std::iter::once(&user1), reconstruction_turn.model_info.truncation_policy.into(), ); rollout_items.push(RolloutItem::ResponseItem(user1.clone().into())); - let assistant1 = ResponseItem::Message { - id: None, - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - text: "assistant reply one".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }; + let assistant1 = assistant_message("assistant reply one"); live_history.record_items( std::iter::once(&assistant1), reconstruction_turn.model_info.truncation_policy.into(), @@ -11230,30 +11229,14 @@ async fn sample_rollout( window_id: Some(window_ids.window_id.to_string()), })); - let user2 = ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "second user".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }; + let user2 = user_message("second user"); live_history.record_items( std::iter::once(&user2), reconstruction_turn.model_info.truncation_policy.into(), ); rollout_items.push(RolloutItem::ResponseItem(user2.clone().into())); - let assistant2 = ResponseItem::Message { - id: None, - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - text: "assistant reply two".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }; + let assistant2 = assistant_message("assistant reply two"); live_history.record_items( std::iter::once(&assistant2), reconstruction_turn.model_info.truncation_policy.into(), @@ -11276,30 +11259,14 @@ async fn sample_rollout( window_id: Some(window_ids.window_id.to_string()), })); - let user3 = ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "third user".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }; + let user3 = user_message("third user"); live_history.record_items( std::iter::once(&user3), reconstruction_turn.model_info.truncation_policy.into(), ); rollout_items.push(RolloutItem::ResponseItem(user3.into())); - let assistant3 = ResponseItem::Message { - id: None, - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - text: "assistant reply three".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }; + let assistant3 = assistant_message("assistant reply three"); live_history.record_items( std::iter::once(&assistant3), reconstruction_turn.model_info.truncation_policy.into(), diff --git a/codex-rs/core/tests/suite/responses_lite.rs b/codex-rs/core/tests/suite/responses_lite.rs index 3179f040d1..a0bc5ed71c 100644 --- a/codex-rs/core/tests/suite/responses_lite.rs +++ b/codex-rs/core/tests/suite/responses_lite.rs @@ -266,6 +266,7 @@ async fn responses_lite_prepares_images() -> Result<()> { .await; let request = response_mock.single_request(); + assert!(request.has_content_kinds(&["user.image", "images.preparation_error"])); let user_content = request .input() .into_iter()