diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index c249a4fa7e..861b56743e 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst differ diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index c238faf44a..0442083bc5 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -400,7 +400,7 @@ impl ThreadHistoryBuilder { match item { RolloutItem::EventMsg(event) => self.handle_event(event), RolloutItem::Compacted(payload) => self.handle_compacted(payload), - RolloutItem::ResponseItem(item) => self.handle_response_item(item), + RolloutItem::ResponseItem(item) => self.handle_response_item(&item.item), RolloutItem::InterAgentCommunication(_) | RolloutItem::InterAgentCommunicationMetadata { .. } | RolloutItem::TurnContext(_) @@ -4399,7 +4399,7 @@ mod tests { local_images: Vec::new(), ..Default::default() })), - RolloutItem::ResponseItem(hook_prompt), + RolloutItem::ResponseItem(hook_prompt.into()), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), started_at: None, @@ -4475,15 +4475,18 @@ mod tests { model_context_window: None, collaboration_mode_kind: Default::default(), })), - RolloutItem::ResponseItem(codex_protocol::models::ResponseItem::Message { - id: Some(codex_protocol::ResponseItemId::with_suffix("msg", "1")), - role: "user".into(), - content: vec![codex_protocol::models::ContentItem::InputText { - text: "plain text".into(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }), + RolloutItem::ResponseItem( + codex_protocol::models::ResponseItem::Message { + id: Some(codex_protocol::ResponseItemId::with_suffix("msg", "1")), + role: "user".into(), + content: vec![codex_protocol::models::ContentItem::InputText { + text: "plain text".into(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + ), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), started_at: None, diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index b1567c748c..83f1f0d651 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -3813,7 +3813,7 @@ impl ThreadRequestProcessor { history .iter() .cloned() - .map(RolloutItem::ResponseItem) + .map(|item| RolloutItem::ResponseItem(item.into())) .collect(), )) } @@ -5400,7 +5400,7 @@ fn preview_from_rollout_items(items: &[RolloutItem]) -> String { items .iter() .find_map(|item| match item { - RolloutItem::ResponseItem(item) => match codex_core::parse_turn_item(item) { + RolloutItem::ResponseItem(item) => match codex_core::parse_turn_item(&item.item) { Some(codex_protocol::items::TurnItem::UserMessage(user)) => Some(user.message()), _ => None, }, diff --git a/codex-rs/app-server/tests/suite/v2/git_attribution.rs b/codex-rs/app-server/tests/suite/v2/git_attribution.rs index fcd0f53d59..f8c17225b9 100644 --- a/codex-rs/app-server/tests/suite/v2/git_attribution.rs +++ b/codex-rs/app-server/tests/suite/v2/git_attribution.rs @@ -333,8 +333,8 @@ fn replace_attribution_fragment_with_legacy( .filter(|line| !line.trim().is_empty()) .map(|line| { let mut line = serde_json::from_str::(line)?; - if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = - &mut line.item + if let RolloutItem::ResponseItem(response_item) = &mut line.item + && let ResponseItem::Message { role, content, .. } = &mut response_item.item && role == "developer" { for item in content { diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 5491a4aa7a..e322bc5c8d 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -1389,15 +1389,18 @@ async fn assert_thread_fork_freezes_active_paginated_turn_as_interrupted( let source_path = rollout_path(codex_home.path(), "2025-01-05T12-00-00", &source_thread_id); let source_id = ThreadId::from_string(source_thread_id.as_str())?; let user_response_item = |id: &str| { - RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: format!("{id} model input"), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }) + RolloutItem::ResponseItem( + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: format!("{id} model input"), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + ) }; let completed_user_item = |id: &str, completed_at_ms| { RolloutItem::EventMsg(EventMsg::ItemCompleted(ItemCompletedEvent { @@ -1499,17 +1502,18 @@ async fn assert_thread_fork_freezes_active_paginated_turn_as_interrupted( .. }, RolloutLine { - item: RolloutItem::ResponseItem(codex_protocol::models::ResponseItem::Message { - role, - .. - }), + item: RolloutItem::ResponseItem(response_item), .. }, RolloutLine { item: RolloutItem::EventMsg(EventMsg::TurnAborted(aborted)), .. }, - ] if role == expected_marker_role && aborted.turn_id.as_deref() == Some("active-turn") + ] if matches!( + &response_item.item, + codex_protocol::models::ResponseItem::Message { role, .. } + if role == expected_marker_role + ) && aborted.turn_id.as_deref() == Some("active-turn") )); append_rollout_item_to_path(source_path.as_path(), &user_response_item("after-fork")).await?; diff --git a/codex-rs/app-server/tests/suite/v2/thread_inject_items.rs b/codex-rs/app-server/tests/suite/v2/thread_inject_items.rs index 89afc6e537..701d379669 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_inject_items.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_inject_items.rs @@ -79,7 +79,7 @@ async fn thread_inject_items_adds_raw_response_items_to_thread_history() -> Resu resumed_history .history .iter() - .any(|item| matches!(item, RolloutItem::ResponseItem(response_item) if strip_response_item_id(responses::strip_metadata(response_item.clone())) == injected_item)), + .any(|item| matches!(item, RolloutItem::ResponseItem(response_item) if strip_response_item_id(responses::strip_metadata(response_item.item.clone())) == injected_item)), "injected item should be persisted in rollout history" ); diff --git a/codex-rs/core/src/agent/control/spawn.rs b/codex-rs/core/src/agent/control/spawn.rs index 610d26d4fa..a589ef84af 100644 --- a/codex-rs/core/src/agent/control/spawn.rs +++ b/codex-rs/core/src/agent/control/spawn.rs @@ -46,13 +46,12 @@ pub(super) fn agent_nickname_candidates(config: &Config, role_name: Option<&str> fn keep_forked_rollout_item(item: &RolloutItem, preserve_reference_context_item: bool) -> bool { match item { - RolloutItem::ResponseItem(ResponseItem::Message { role, phase, .. }) => match role.as_str() - { - "system" | "developer" | "user" => true, - "assistant" => *phase == Some(MessagePhase::FinalAnswer), - _ => false, - }, - RolloutItem::ResponseItem( + RolloutItem::ResponseItem(envelope) => match &envelope.item { + ResponseItem::Message { role, phase, .. } => match role.as_str() { + "system" | "developer" | "user" => true, + "assistant" => *phase == Some(MessagePhase::FinalAnswer), + _ => false, + }, ResponseItem::AdditionalTools { .. } | ResponseItem::AgentMessage { .. } | ResponseItem::Reasoning { .. } @@ -68,8 +67,8 @@ fn keep_forked_rollout_item(item: &RolloutItem, preserve_reference_context_item: | ResponseItem::Compaction { .. } | ResponseItem::CompactionTrigger { .. } | ResponseItem::ContextCompaction { .. } - | ResponseItem::Other, - ) => false, + | ResponseItem::Other => false, + }, RolloutItem::InterAgentCommunication(_) | RolloutItem::InterAgentCommunicationMetadata { .. } => false, // Full-history forks preserve the cached prompt prefix and can keep diffing @@ -812,7 +811,7 @@ impl AgentControl { subagent_developer_instructions.clone(), ]) { - forked_rollout_items.push(RolloutItem::ResponseItem(developer_message)); + forked_rollout_items.push(RolloutItem::ResponseItem(developer_message.into())); } if preserve_reference_context_item && multi_agent_version == MultiAgentVersion::V2 @@ -823,7 +822,9 @@ impl AgentControl { subagent_usage_hint_text, ]) { - forked_rollout_items.push(RolloutItem::ResponseItem(subagent_usage_hint_message)); + forked_rollout_items.push(RolloutItem::ResponseItem( + subagent_usage_hint_message.into(), + )); } let mut thread_extension_init = ExtensionDataInit::new(); thread_extension_init.insert(selected_capability_roots); diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 5982e1233b..bb2525d02b 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -132,6 +132,10 @@ fn captured_op_matches(actual: &(ThreadId, Op), expected: &(ThreadId, Op)) -> bo } } +fn rollout_response_item(item: ResponseItem) -> RolloutItem { + RolloutItem::ResponseItem(item.into()) +} + fn assistant_message(text: &str, phase: Option) -> ResponseItem { ResponseItem::Message { id: None, @@ -1040,7 +1044,7 @@ async fn spawn_agent_fork_from_paginated_parent_uses_model_context_prefix() { parent_thread .session .persist_rollout_items(&[ - RolloutItem::ResponseItem(ResponseItem::Message { + rollout_response_item(ResponseItem::Message { id: None, role: "developer".to_string(), content: vec![ContentItem::InputText { @@ -1141,7 +1145,7 @@ async fn spawn_agent_fork_from_paginated_parent_uses_model_context_prefix() { .iter() .find_map(|line| match &line.item { RolloutItem::ResponseItem(response_item) - if serde_json::to_string(response_item) + if serde_json::to_string(&response_item.item) .expect("serialize response item") .contains("id-less inherited context") => { @@ -1254,21 +1258,24 @@ async fn spawn_agent_numeric_fork_from_compacted_paginated_parent_clamps_to_prov .persist_rollout_items(&[ RolloutItem::Compacted(CompactedItem { message: String::new(), - replacement_history: Some(vec![ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "compacted summary".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }]), + replacement_history: Some(vec![ + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "compacted summary".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + ]), window_number: None, first_window_id: None, previous_window_id: None, window_id: None, }), - RolloutItem::ResponseItem(ResponseItem::Message { + rollout_response_item(ResponseItem::Message { id: None, role: "user".to_string(), content: vec![ContentItem::InputText { @@ -1277,7 +1284,7 @@ async fn spawn_agent_numeric_fork_from_compacted_paginated_parent_clamps_to_prov phase: None, internal_chat_message_metadata_passthrough: None, }), - RolloutItem::ResponseItem(spawn_agent_call(&parent_spawn_call_id)), + rollout_response_item(spawn_agent_call(&parent_spawn_call_id)), ]) .await; @@ -1681,14 +1688,16 @@ async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history() { .persist_rollout_items(&[ RolloutItem::Compacted(CompactedItem { message: String::new(), - replacement_history: Some(replacement_history), + replacement_history: Some( + replacement_history.into_iter().map(Into::into).collect(), + ), window_number: None, first_window_id: None, previous_window_id: None, window_id: None, }), RolloutItem::TurnContext(turn_context.to_turn_context_item()), - RolloutItem::ResponseItem(spawn_agent_call(&parent_spawn_call_id)), + rollout_response_item(spawn_agent_call(&parent_spawn_call_id)), ]) .await; parent_thread @@ -1832,7 +1841,7 @@ async fn spawn_agent_full_fork_restores_instructions_after_compaction_discards_p parent_thread .session .persist_rollout_items(&[ - RolloutItem::ResponseItem(ResponseItem::Message { + rollout_response_item(ResponseItem::Message { id: None, role: "developer".to_string(), content: vec![ContentItem::InputText { @@ -1843,14 +1852,16 @@ async fn spawn_agent_full_fork_restores_instructions_after_compaction_discards_p }), RolloutItem::Compacted(CompactedItem { message: String::new(), - replacement_history: Some(replacement_history), + replacement_history: Some( + replacement_history.into_iter().map(Into::into).collect(), + ), window_number: None, first_window_id: None, previous_window_id: None, window_id: None, }), RolloutItem::TurnContext(turn_context.to_turn_context_item()), - RolloutItem::ResponseItem(spawn_agent_call(&parent_spawn_call_id)), + rollout_response_item(spawn_agent_call(&parent_spawn_call_id)), ]) .await; parent_thread @@ -1969,7 +1980,7 @@ async fn spawn_agent_full_fork_legacy_compaction_rebuilds_child_instructions_onc ) .await; let mut rollout_items = vec![ - RolloutItem::ResponseItem(parent_user_message), + rollout_response_item(parent_user_message), RolloutItem::Compacted(CompactedItem { message: "legacy compacted summary".to_string(), replacement_history: None, @@ -1980,7 +1991,7 @@ async fn spawn_agent_full_fork_legacy_compaction_rebuilds_child_instructions_onc }), ]; if let Some(instructions) = parent_developer_instructions { - rollout_items.push(RolloutItem::ResponseItem(ResponseItem::Message { + rollout_items.push(rollout_response_item(ResponseItem::Message { id: None, role: "developer".to_string(), content: vec![ContentItem::InputText { @@ -1993,7 +2004,7 @@ async fn spawn_agent_full_fork_legacy_compaction_rebuilds_child_instructions_onc rollout_items.push(RolloutItem::TurnContext( turn_context.to_turn_context_item(), )); - rollout_items.push(RolloutItem::ResponseItem(spawn_agent_call( + rollout_items.push(rollout_response_item(spawn_agent_call( parent_spawn_call_id, ))); parent_thread diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 1123e1b329..b58e99d9d9 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -28,6 +28,7 @@ use codex_analytics::CompactionStatus; use codex_analytics::CompactionStrategy; use codex_analytics::CompactionTrigger; use codex_analytics::now_unix_seconds; +use codex_history::CodexHarnessMetadata; use codex_history::ResponseItemEnvelope; use codex_protocol::error::CodexErr; use codex_protocol::error::CodexErrorDetails; @@ -85,7 +86,7 @@ pub(crate) struct CompactedHistoryMetadata { pub(crate) async fn build_compaction_initial_context( sess: &Session, initial_context_injection: &InitialContextInjection, -) -> (Vec, Option>) { +) -> (Vec, Option>) { // Return the rendered state with its items so history and its baseline stay identical. match initial_context_injection { InitialContextInjection::BeforeLastUserMessage { @@ -98,7 +99,10 @@ pub(crate) async fn build_compaction_initial_context( world_state.as_ref(), ) .await; - (items, Some(Arc::clone(world_state))) + ( + items.into_iter().map(ResponseItemEnvelope::new).collect(), + Some(Arc::clone(world_state)), + ) } InitialContextInjection::DoNotInject => (Vec::new(), None), } @@ -517,11 +521,15 @@ pub fn content_items_to_text(content: &[ContentItem]) -> Option { pub(crate) struct CompactedUserMessage { message: String, internal_chat_message_metadata_passthrough: Option, + harness_metadata: Option, } #[cfg(test)] pub(crate) fn collect_user_messages(items: &[ResponseItem]) -> Vec { - items.iter().filter_map(compacted_user_message).collect() + items + .iter() + .filter_map(|item| compacted_user_message(item, /*harness_metadata*/ None)) + .collect() } pub(crate) fn collect_annotated_user_messages( @@ -529,11 +537,14 @@ pub(crate) fn collect_annotated_user_messages( ) -> Vec { items .iter() - .filter_map(|envelope| compacted_user_message(&envelope.item)) + .filter_map(|envelope| compacted_user_message(&envelope.item, envelope.metadata.clone())) .collect() } -fn compacted_user_message(item: &ResponseItem) -> Option { +fn compacted_user_message( + item: &ResponseItem, + harness_metadata: Option, +) -> Option { let Some(TurnItem::UserMessage(user)) = crate::event_mapping::parse_turn_item(item) else { return None; }; @@ -549,6 +560,7 @@ fn compacted_user_message(item: &ResponseItem) -> Option { } => internal_chat_message_metadata_passthrough.clone(), _ => None, }, + harness_metadata, }) } @@ -567,13 +579,13 @@ pub(crate) fn is_summary_message(message: &str) -> bool { /// that item remains last (remote compaction may return only compaction items). /// - If there are no user messages or compaction items, append the context. pub(crate) fn insert_initial_context_before_last_real_user_or_summary( - mut compacted_history: Vec, - initial_context: Vec, -) -> Vec { + mut compacted_history: Vec, + initial_context: Vec, +) -> Vec { let mut last_user_or_summary_index = None; let mut last_real_user_index = None; for (i, item) in compacted_history.iter().enumerate().rev() { - if let ResponseItem::AgentMessage { content, .. } = item + if let ResponseItem::AgentMessage { content, .. } = &item.item && !matches!( content.first(), Some(AgentMessageInputContent::InputText { text }) @@ -583,7 +595,8 @@ pub(crate) fn insert_initial_context_before_last_real_user_or_summary( last_real_user_index = Some(i); break; } - let Some(TurnItem::UserMessage(user)) = crate::event_mapping::parse_turn_item(item) else { + let Some(TurnItem::UserMessage(user)) = crate::event_mapping::parse_turn_item(&item.item) + else { continue; }; // Compaction summaries are encoded as user messages, so track both: @@ -601,7 +614,7 @@ pub(crate) fn insert_initial_context_before_last_real_user_or_summary( .rev() .find_map(|(i, item)| { matches!( - item, + &item.item, ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. } ) .then_some(i) @@ -624,10 +637,10 @@ pub(crate) fn insert_initial_context_before_last_real_user_or_summary( } pub(crate) fn build_compacted_history( - initial_context: Vec, + initial_context: Vec, user_messages: &[CompactedUserMessage], summary_text: &str, -) -> Vec { +) -> Vec { build_compacted_history_with_limit( initial_context, user_messages, @@ -637,11 +650,11 @@ pub(crate) fn build_compacted_history( } fn build_compacted_history_with_limit( - mut history: Vec, + mut history: Vec, user_messages: &[CompactedUserMessage], summary_text: &str, max_tokens: usize, -) -> Vec { +) -> Vec { let mut selected_messages: Vec = Vec::new(); if max_tokens > 0 { let mut remaining = max_tokens; @@ -661,6 +674,7 @@ fn build_compacted_history_with_limit( internal_chat_message_metadata_passthrough: message .internal_chat_message_metadata_passthrough .clone(), + harness_metadata: message.harness_metadata.clone(), }); break; } @@ -669,16 +683,19 @@ fn build_compacted_history_with_limit( } for message in &selected_messages { - history.push(ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: message.message.clone(), - }], - phase: None, - internal_chat_message_metadata_passthrough: message - .internal_chat_message_metadata_passthrough - .clone(), + history.push(ResponseItemEnvelope { + item: ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: message.message.clone(), + }], + phase: None, + internal_chat_message_metadata_passthrough: message + .internal_chat_message_metadata_passthrough + .clone(), + }, + metadata: message.harness_metadata.clone(), }); } @@ -688,13 +705,13 @@ fn build_compacted_history_with_limit( summary_text.to_string() }; - history.push(ResponseItem::Message { + history.push(ResponseItemEnvelope::new(ResponseItem::Message { id: None, role: "user".to_string(), content: vec![ContentItem::InputText { text: summary_text }], phase: None, internal_chat_message_metadata_passthrough: None, - }); + })); history } diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 869862d03f..392fb991c1 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -284,6 +284,12 @@ async fn run_remote_compact_task_inner_impl( replacement_history: &new_history, }); } + // Legacy `/responses/compact` returns provider-normalized items without a stable link to their + // original envelopes, so it does not preserve harness metadata. Compaction-trigger/v2 does. + let new_history = new_history + .into_iter() + .map(ResponseItemEnvelope::new) + .collect(); sess.replace_compacted_history( new_history, reference_context_item, @@ -307,6 +313,28 @@ pub(crate) async fn process_compacted_history( compacted_history: Vec, initial_context_injection: &InitialContextInjection, ) -> (Vec, Option>) { + let compacted_history = compacted_history + .into_iter() + .map(ResponseItemEnvelope::new) + .collect(); + let (compacted_history, world_state_baseline) = + process_annotated_compacted_history(sess, compacted_history, initial_context_injection) + .await; + ( + compacted_history + .into_iter() + .map(ResponseItemEnvelope::into_item) + .collect(), + world_state_baseline, + ) +} + +/// Installs already-annotated remote compaction output without dropping its metadata sidecar. +pub(crate) async fn process_annotated_compacted_history( + sess: &Session, + compacted_history: Vec, + initial_context_injection: &InitialContextInjection, +) -> (Vec, Option>) { // Mid-turn compaction is the only path that must inject initial context above the last user // message in the replacement history. Pre-turn compaction instead injects context after the // compaction item, but mid-turn compaction keeps the compaction item last for model training. @@ -314,7 +342,7 @@ pub(crate) async fn process_compacted_history( build_compaction_initial_context(sess, initial_context_injection).await; let compacted_history = history_item_groups(compacted_history) - .filter(|group| should_keep_compacted_history_item(&group.source)) + .filter(|group| should_keep_compacted_history_item(&group.source.item)) .flat_map(HistoryItemGroup::into_items) .collect(); ( @@ -386,7 +414,7 @@ pub(crate) fn trim_function_call_history_to_fit_context_window( .fold(base_tokens, i128::saturating_add); let initial_estimated_tokens = i64::try_from(estimated_tokens).unwrap_or(i64::MAX); let mut rewritten_items = Vec::new(); - let mut consumed_items = 0; + let mut consumed_items: usize = 0; for group in history_item_groups(original_items.iter().map(|item| &item.item)) .collect::>() @@ -396,13 +424,20 @@ pub(crate) fn trim_function_call_history_to_fit_context_window( if i64::try_from(estimated_tokens).unwrap_or(i64::MAX) <= context_window { break; } - let Some(rewritten_item) = rewritten_output_for_context_window(group.source) else { + let group_item_count = 1 + usize::from(group.attached_notice.is_some()); + let source_index = original_items + .len() + .saturating_sub(consumed_items.saturating_add(group_item_count)); + let Some(rewritten_item) = original_items + .get(source_index) + .and_then(rewritten_output_for_context_window) + else { break; }; estimated_tokens = estimated_tokens .saturating_sub(group.estimated_token_count()) - .saturating_add(i128::from(estimate_item_token_count(&rewritten_item))); - consumed_items += 1 + usize::from(group.attached_notice.is_some()); + .saturating_add(i128::from(estimate_item_token_count(&rewritten_item.item))); + consumed_items += group_item_count; rewritten_items.push(rewritten_item); } @@ -410,12 +445,7 @@ pub(crate) fn trim_function_call_history_to_fit_context_window( if rewritten_outputs > 0 { let retained_len = original_items.len() - consumed_items; let mut items = original_items[..retained_len].to_vec(); - items.extend( - rewritten_items - .into_iter() - .rev() - .map(ResponseItemEnvelope::new), - ); + items.extend(rewritten_items.into_iter().rev()); history.replace_annotated(items); } @@ -424,8 +454,10 @@ pub(crate) fn trim_function_call_history_to_fit_context_window( (rewritten_outputs, estimated_deleted_tokens) } -fn rewritten_output_for_context_window(item: &ResponseItem) -> Option { - Some(match item { +fn rewritten_output_for_context_window( + envelope: &ResponseItemEnvelope, +) -> Option { + let item = match &envelope.item { ResponseItem::FunctionCallOutput { id, call_id, @@ -466,6 +498,10 @@ fn rewritten_output_for_context_window(item: &ResponseItem) -> Option return None, + }; + Some(ResponseItemEnvelope { + item, + metadata: envelope.metadata.clone(), }) } @@ -475,3 +511,7 @@ fn truncated_output_payload(output: &FunctionCallOutputPayload) -> FunctionCallO success: output.success, } } + +#[cfg(test)] +#[path = "compact_remote_metadata_tests.rs"] +mod metadata_tests; diff --git a/codex-rs/core/src/compact_remote_metadata_tests.rs b/codex-rs/core/src/compact_remote_metadata_tests.rs new file mode 100644 index 0000000000..c3a9d41134 --- /dev/null +++ b/codex-rs/core/src/compact_remote_metadata_tests.rs @@ -0,0 +1,24 @@ +use super::*; +use codex_history::CodexHarnessMetadata; + +#[test] +fn rewritten_output_preserves_harness_metadata() { + let envelope = ResponseItemEnvelope { + item: ResponseItem::FunctionCallOutput { + id: None, + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload { + body: FunctionCallOutputBody::Text("large output".repeat(100)), + success: Some(true), + }, + internal_chat_message_metadata_passthrough: None, + }, + metadata: Some(CodexHarnessMetadata::default()), + }; + + let rewritten = rewritten_output_for_context_window(&envelope) + .expect("function output should be rewritten"); + + assert_eq!(rewritten.metadata, envelope.metadata); + assert_ne!(rewritten.item, envelope.item); +} diff --git a/codex-rs/core/src/compact_remote_v2.rs b/codex-rs/core/src/compact_remote_v2.rs index a0af726c52..070986af63 100644 --- a/codex-rs/core/src/compact_remote_v2.rs +++ b/codex-rs/core/src/compact_remote_v2.rs @@ -11,7 +11,7 @@ use crate::compact::InitialContextInjection; use crate::compact::compaction_status_from_result; use crate::compact_model_fallback::record_model_fallback; use crate::compact_model_fallback::should_retry_with_current_model; -use crate::compact_remote::process_compacted_history; +use crate::compact_remote::process_annotated_compacted_history; use crate::compact_remote::should_keep_compacted_history_item; use crate::compact_remote_history::HistoryItemGroup; use crate::compact_remote_history::history_item_groups; @@ -32,6 +32,8 @@ use codex_analytics::CompactionImplementation; use codex_analytics::CompactionPhase; use codex_analytics::CompactionReason; use codex_analytics::CompactionTrigger; +use codex_history::CodexHarnessMetadata; +use codex_history::ResponseItemEnvelope; use codex_protocol::error::CodexErr; use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; @@ -278,6 +280,7 @@ async fn run_remote_compact_task_inner_impl( let RemoteCompactV2Attempt { trace_input_history, prompt_input, + prompt_input_metadata, compaction_output, token_usage, owned_client_session: _owned_client_session, @@ -290,12 +293,15 @@ async fn run_remote_compact_task_inner_impl( analytics_details.cache_write_input_tokens = Some(token_usage.cache_write_input_tokens); } let (compacted_history, retained_images) = - build_v2_compacted_history(&prompt_input, compaction_output); + build_v2_compacted_history(prompt_input, prompt_input_metadata, compaction_output); analytics_details.retained_image_count = Some(retained_images); let (new_window_number, new_window_ids) = sess.advance_auto_compact_window().await; - let (new_history, world_state_baseline) = - process_compacted_history(sess.as_ref(), compacted_history, &initial_context_injection) - .await; + let (new_history, world_state_baseline) = process_annotated_compacted_history( + sess.as_ref(), + compacted_history, + &initial_context_injection, + ) + .await; let reference_context_item = match initial_context_injection { InitialContextInjection::DoNotInject => None, @@ -304,9 +310,13 @@ async fn run_remote_compact_task_inner_impl( } }; if let Some(trace_input_history) = trace_input_history.as_deref() { + let replacement_history = new_history + .iter() + .map(|envelope| envelope.item.clone()) + .collect::>(); compaction_trace.record_installed(&CompactionCheckpointTracePayload { input_history: trace_input_history, - replacement_history: &new_history, + replacement_history: &replacement_history, }); } sess.replace_compacted_history( @@ -443,22 +453,28 @@ async fn collect_compaction_output( } fn build_v2_compacted_history( - prompt_input: &[ResponseItem], + prompt_input: Vec, + prompt_input_metadata: Vec>, compaction_output: ResponseItem, -) -> (Vec, usize) { +) -> (Vec, usize) { + debug_assert_eq!(prompt_input.len(), prompt_input_metadata.len()); + let prompt_input = prompt_input + .into_iter() + .zip(prompt_input_metadata) + .map(|(item, metadata)| ResponseItemEnvelope { item, metadata }) + .collect::>(); let retained = history_item_groups(prompt_input) - .filter(|group| is_retained_for_remote_compaction_v2(group.source)) - .filter(|group| should_keep_compacted_history_item(group.source)) + .filter(|group| is_retained_for_remote_compaction_v2(&group.source.item)) + .filter(|group| should_keep_compacted_history_item(&group.source.item)) .flat_map(HistoryItemGroup::into_items) - .cloned() .collect::>(); let mut retained = truncate_retained_messages_for_remote_compaction(retained, RETAINED_MESSAGE_TOKEN_BUDGET); let retained_image_count = retained .iter() - .map(retained_input_image_count) + .map(|envelope| retained_input_image_count(&envelope.item)) .sum::(); - retained.push(compaction_output); + retained.push(ResponseItemEnvelope::new(compaction_output)); (retained, retained_image_count) } @@ -492,9 +508,9 @@ fn retained_input_image_count(item: &ResponseItem) -> usize { } fn truncate_retained_messages_for_remote_compaction( - items: Vec, + items: Vec, max_tokens: usize, -) -> Vec { +) -> Vec { let mut remaining = max_tokens; let mut truncated_reversed = Vec::with_capacity(items.len()); for group in history_item_groups(items) @@ -509,8 +525,8 @@ fn truncate_retained_messages_for_remote_compaction( let notice_tokens = group .attached_notice .as_ref() - .map_or(0, |notice| message_text_token_count(notice).max(1)); - let token_count = message_text_token_count(&group.source) + .map_or(0, |notice| message_text_token_count(¬ice.item).max(1)); + let token_count = message_text_token_count(&group.source.item) .max(1) .saturating_add(notice_tokens); if token_count <= remaining { @@ -553,15 +569,19 @@ fn message_text_token_count(item: &ResponseItem) -> usize { } fn truncate_message_text_to_token_budget( - item: ResponseItem, + envelope: ResponseItemEnvelope, max_tokens: usize, -) -> Option { +) -> Option { + let ResponseItemEnvelope { + item, + metadata: harness_metadata, + } = envelope; let ResponseItem::Message { id, role, content, phase, - internal_chat_message_metadata_passthrough: metadata, + internal_chat_message_metadata_passthrough: passthrough_metadata, } = item else { return None; @@ -597,12 +617,15 @@ fn truncate_message_text_to_token_budget( return None; } - Some(ResponseItem::Message { - id, - role, - content: truncated_content, - phase, - internal_chat_message_metadata_passthrough: metadata, + Some(ResponseItemEnvelope { + item: ResponseItem::Message { + id, + role, + content: truncated_content, + phase, + internal_chat_message_metadata_passthrough: passthrough_metadata, + }, + metadata: harness_metadata, }) } @@ -627,6 +650,32 @@ mod tests { } } + fn build_without_metadata( + input: Vec, + output: ResponseItem, + ) -> (Vec, usize) { + let metadata = vec![None; input.len()]; + build_v2_compacted_history(input, metadata, output) + } + + fn annotated(items: Vec) -> Vec { + items.into_iter().map(ResponseItemEnvelope::new).collect() + } + + fn raw(items: Vec) -> Vec { + items + .into_iter() + .map(ResponseItemEnvelope::into_item) + .collect() + } + + fn truncate_without_metadata(items: Vec, max_tokens: usize) -> Vec { + raw(truncate_retained_messages_for_remote_compaction( + annotated(items), + max_tokens, + )) + } + fn response_stream(events: Vec>) -> ResponseStream { let (tx_event, rx_event) = mpsc::channel(events.len().max(1)); for event in events { @@ -670,14 +719,55 @@ mod tests { internal_chat_message_metadata_passthrough: None, }; - let (history, _) = build_v2_compacted_history(&input, output.clone()); + let (history, _) = build_without_metadata(input, output.clone()); + + assert_eq!( + raw(history), + vec![message("user", "user", /*phase*/ None), output] + ); + } + + #[test] + fn build_v2_compacted_history_preserves_retained_metadata_sidecar() { + let retained = message("user", "keep", /*phase*/ None); + let filtered = message("developer", "drop", /*phase*/ None); + let output = ResponseItem::Compaction { + id: None, + encrypted_content: "new".to_string(), + internal_chat_message_metadata_passthrough: None, + }; + let (history, _) = build_v2_compacted_history( + vec![filtered, retained.clone()], + vec![None, Some(CodexHarnessMetadata::default())], + output.clone(), + ); assert_eq!( history, - vec![message("user", "user", /*phase*/ None), output] + vec![ + ResponseItemEnvelope { + item: retained, + metadata: Some(CodexHarnessMetadata::default()), + }, + ResponseItemEnvelope::new(output), + ] ); } + #[test] + fn retained_history_truncation_preserves_metadata() { + let item = ResponseItemEnvelope { + item: message("user", "word ".repeat(200).as_str(), /*phase*/ None), + metadata: Some(CodexHarnessMetadata::default()), + }; + + let truncated = + truncate_retained_messages_for_remote_compaction(vec![item], /*max_tokens*/ 4); + + assert_eq!(truncated.len(), 1); + assert_eq!(truncated[0].metadata, Some(CodexHarnessMetadata::default())); + } + #[test] fn build_v2_compacted_history_discards_messages_before_truncating() { let old = message("user", "old", /*phase*/ None); @@ -699,9 +789,9 @@ mod tests { internal_chat_message_metadata_passthrough: None, }; - let (history, _) = build_v2_compacted_history(&input, output.clone()); + let (history, _) = build_without_metadata(input, output.clone()); - assert_eq!(history, vec![old, new, output]); + assert_eq!(raw(history), vec![old, new, output]); } #[test] @@ -731,7 +821,7 @@ mod tests { internal_chat_message_metadata_passthrough: None, }; - let (_, retained_image_count) = build_v2_compacted_history(&input, output); + let (_, retained_image_count) = build_without_metadata(input, output); assert_eq!(retained_image_count, 2); } @@ -746,8 +836,7 @@ mod tests { new.clone(), ]; - let truncated = - truncate_retained_messages_for_remote_compaction(retained, /*max_tokens*/ 3); + let truncated = truncate_without_metadata(retained, /*max_tokens*/ 3); assert_eq!( truncated, @@ -779,8 +868,7 @@ mod tests { internal_chat_message_metadata_passthrough: None, }; - let truncated = - truncate_retained_messages_for_remote_compaction(vec![item], /*max_tokens*/ 3); + let truncated = truncate_without_metadata(vec![item], /*max_tokens*/ 3); assert_eq!( truncated, @@ -824,8 +912,7 @@ mod tests { newest.clone(), ]; - let truncated = - truncate_retained_messages_for_remote_compaction(retained, /*max_tokens*/ 2); + let truncated = truncate_without_metadata(retained, /*max_tokens*/ 2); assert_eq!(truncated, vec![image_only_message, newest]); } @@ -845,8 +932,7 @@ mod tests { let newest = message("user", "new", /*phase*/ None); let retained = vec![image_only_message, newest.clone()]; - let truncated = - truncate_retained_messages_for_remote_compaction(retained, /*max_tokens*/ 1); + let truncated = truncate_without_metadata(retained, /*max_tokens*/ 1); assert_eq!(truncated, vec![newest]); } diff --git a/codex-rs/core/src/compact_remote_v2_attempt.rs b/codex-rs/core/src/compact_remote_v2_attempt.rs index 318495f4ca..c5d4d037ef 100644 --- a/codex-rs/core/src/compact_remote_v2_attempt.rs +++ b/codex-rs/core/src/compact_remote_v2_attempt.rs @@ -10,6 +10,7 @@ use crate::responses_metadata::CodexResponsesRequestKind; use crate::responses_metadata::CompactionTurnMetadata; use crate::session::session::Session; use crate::session::step_context::StepContext; +use codex_history::CodexHarnessMetadata; use codex_protocol::error::Result as CodexResult; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::EventMsg; @@ -21,6 +22,7 @@ use tracing::info; pub(super) struct RemoteCompactV2Attempt { pub(super) trace_input_history: Option>, pub(super) prompt_input: Vec, + pub(super) prompt_input_metadata: Vec>, pub(super) compaction_output: ResponseItem, pub(super) token_usage: Option, /// Keeps a session created for standalone compaction alive through lifecycle completion. @@ -66,7 +68,11 @@ pub(super) async fn run_remote_compact_v2_attempt( let trace_input_history = compaction_trace .is_enabled() .then(|| history.raw_items().cloned().collect()); - let mut input = history.for_prompt(&turn_context.model_info.input_modalities); + let (mut input, prompt_input_metadata): (Vec<_>, Vec<_>) = history + .for_prompt_annotated(&turn_context.model_info.input_modalities) + .into_iter() + .map(|envelope| (envelope.item, envelope.metadata)) + .unzip(); let tool_router = &step_context.tool_router; input.push(ResponseItem::CompactionTrigger {}); let prompt = Prompt { @@ -128,6 +134,7 @@ pub(super) async fn run_remote_compact_v2_attempt( Ok(RemoteCompactV2Attempt { trace_input_history, prompt_input, + prompt_input_metadata, compaction_output, token_usage, owned_client_session, diff --git a/codex-rs/core/src/compact_tests.rs b/codex-rs/core/src/compact_tests.rs index 1aea757c6f..967704bc84 100644 --- a/codex-rs/core/src/compact_tests.rs +++ b/codex-rs/core/src/compact_tests.rs @@ -1,4 +1,5 @@ use super::*; +use codex_history::CodexHarnessMetadata; use codex_history::ResponseItemEnvelope; use codex_protocol::ResponseItemId; use codex_protocol::models::DEFAULT_IMAGE_DETAIL; @@ -39,6 +40,17 @@ async fn process_compacted_history_with_test_session( (refreshed, initial_context) } +fn annotated(items: Vec) -> Vec { + items.into_iter().map(ResponseItemEnvelope::new).collect() +} + +fn raw(items: Vec) -> Vec { + items + .into_iter() + .map(ResponseItemEnvelope::into_item) + .collect() +} + fn user_message(text: &str) -> ResponseItem { ResponseItem::Message { id: None, @@ -55,6 +67,7 @@ fn compacted_user_message(text: &str) -> CompactedUserMessage { CompactedUserMessage { message: text.to_string(), internal_chat_message_metadata_passthrough: None, + harness_metadata: None, } } @@ -121,13 +134,23 @@ fn collect_user_messages_extracts_user_text_only() { #[test] fn collect_annotated_user_messages_extracts_user_text_only() { let items = vec![ - ResponseItemEnvelope::new(user_message("first")), + ResponseItemEnvelope { + item: user_message("first"), + metadata: Some(CodexHarnessMetadata::default()), + }, ResponseItemEnvelope::new(ResponseItem::Other), ]; let collected = collect_annotated_user_messages(&items); - assert_eq!(vec![compacted_user_message("first")], collected); + assert_eq!( + vec![CompactedUserMessage { + message: "first".to_string(), + internal_chat_message_metadata_passthrough: None, + harness_metadata: Some(CodexHarnessMetadata::default()), + }], + collected + ); } #[test] @@ -198,7 +221,11 @@ fn build_token_limited_compacted_history_truncates_overlong_user_messages() { // that oversized user content is truncated. let max_tokens = 16; let big = "word ".repeat(200); - let user_message = compacted_user_message(&big); + let user_message = CompactedUserMessage { + message: big.clone(), + internal_chat_message_metadata_passthrough: None, + harness_metadata: Some(CodexHarnessMetadata::default()), + }; let history = super::build_compacted_history_with_limit( Vec::new(), std::slice::from_ref(&user_message), @@ -207,8 +234,8 @@ fn build_token_limited_compacted_history_truncates_overlong_user_messages() { ); assert_eq!(history.len(), 2); - let truncated_message = &history[0]; - let summary_message = &history[1]; + let truncated_message = &history[0].item; + let summary_message = &history[1].item; let truncated_text = match truncated_message { ResponseItem::Message { role, content, .. } if role == "user" => { @@ -233,11 +260,13 @@ fn build_token_limited_compacted_history_truncates_overlong_user_messages() { other => panic!("unexpected item in history: {other:?}"), }; assert_eq!(summary_text, "SUMMARY"); + assert_eq!(history[0].metadata, Some(CodexHarnessMetadata::default())); + assert_eq!(history[1].metadata, None); } #[test] fn build_token_limited_compacted_history_appends_summary_message() { - let initial_context: Vec = Vec::new(); + let initial_context: Vec = Vec::new(); let user_messages = vec![compacted_user_message("first user message")]; let summary_text = "summary text"; @@ -248,7 +277,7 @@ fn build_token_limited_compacted_history_appends_summary_message() { ); let last = history.last().expect("history should have a summary entry"); - let summary = match last { + let summary = match &last.item { ResponseItem::Message { role, content, .. } if role == "user" => { content_items_to_text(content).unwrap_or_default() } @@ -269,12 +298,15 @@ fn build_compacted_history_preserves_user_message_passthrough_metadata() { ..Default::default() }, ), + harness_metadata: Some(CodexHarnessMetadata::default()), }], "summary text", ); assert_eq!(history[0].turn_id(), Some("turn-1")); assert_eq!(history[1].turn_id(), None); + assert_eq!(history[0].metadata, Some(CodexHarnessMetadata::default())); + assert_eq!(history[1].metadata, None); } #[tokio::test] @@ -623,8 +655,10 @@ fn insert_initial_context_before_last_real_user_or_summary_keeps_summary_last() internal_chat_message_metadata_passthrough: None, }]; - let refreshed = - insert_initial_context_before_last_real_user_or_summary(compacted_history, initial_context); + let refreshed = raw(insert_initial_context_before_last_real_user_or_summary( + annotated(compacted_history), + annotated(initial_context), + )); let expected = vec![ ResponseItem::Message { id: None, @@ -694,8 +728,10 @@ fn insert_initial_context_before_last_real_user_or_summary_keeps_compaction_last internal_chat_message_metadata_passthrough: None, }]; - let refreshed = - insert_initial_context_before_last_real_user_or_summary(compacted_history, initial_context); + let refreshed = raw(insert_initial_context_before_last_real_user_or_summary( + annotated(compacted_history), + annotated(initial_context), + )); let expected = vec![ ResponseItem::Message { id: None, diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 0eb2eb74ad..d953d09644 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -9,6 +9,7 @@ use crate::event_mapping::is_contextual_user_message_content; use crate::session::turn_context::TurnContext; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use codex_history::CodexHarnessMetadata; use codex_history::ResponseItemEnvelope; use codex_protocol::models::AgentMessageInputContent; use codex_protocol::models::BaseInstructions; @@ -127,30 +128,63 @@ impl ContextManager { pub(crate) fn record_items(&mut self, items: I, policy: TruncationPolicy) where I: IntoIterator, - I::Item: std::ops::Deref, + I::Item: Deref, { - for item in items { - let item_ref = item.deref(); - if !is_api_message(item_ref) { + self.record_items_with_metadata(items.into_iter().map(|item| (item, None)), policy); + } + + /// Records history envelopes while preserving their history-only metadata. + pub(crate) fn record_annotated_items( + &mut self, + items: &[ResponseItemEnvelope], + policy: TruncationPolicy, + ) { + self.record_items_with_metadata( + items + .iter() + .map(|envelope| (&envelope.item, envelope.metadata.as_ref())), + policy, + ); + } + + fn record_items_with_metadata<'a, I, T>(&mut self, items: I, policy: TruncationPolicy) + where + I: IntoIterator)>, + T: Deref, + { + for (item, metadata) in items { + let item = item.deref(); + if !is_api_message(item) { continue; } - let processed = Self::process_item(item_ref, policy); - Arc::make_mut(&mut self.items).push(ResponseItemEnvelope::new(processed)); + let processed = ResponseItemEnvelope { + item: Self::process_item(item, policy), + metadata: metadata.cloned(), + }; + Arc::make_mut(&mut self.items).push(processed); } } /// Returns the history prepared for sending to the model. This applies a proper /// normalization and drops un-suited items. Unsupported image and audio content /// is stripped from messages and tool outputs according to `input_modalities`. - pub(crate) fn for_prompt(mut self, input_modalities: &[InputModality]) -> Vec { - self.normalize_history(input_modalities); - Arc::unwrap_or_clone(self.items) + pub(crate) fn for_prompt(self, input_modalities: &[InputModality]) -> Vec { + self.for_prompt_annotated(input_modalities) .into_iter() .map(ResponseItemEnvelope::into_item) .collect() } + /// Returns normalized history envelopes for internal consumers that must retain metadata. + pub(crate) fn for_prompt_annotated( + mut self, + input_modalities: &[InputModality], + ) -> Vec { + self.normalize_history(input_modalities); + Arc::unwrap_or_clone(self.items) + } + /// Iterates over raw response items without exposing their history envelopes. pub(crate) fn raw_items( &self, @@ -165,12 +199,17 @@ impl ContextManager { /// Returns raw items in the history and consumes the snapshot. pub(crate) fn into_raw_items(self) -> Vec { - Arc::unwrap_or_clone(self.items) + self.into_annotated_items() .into_iter() .map(ResponseItemEnvelope::into_item) .collect() } + /// Returns annotated history items and consumes the snapshot. + pub(crate) fn into_annotated_items(self) -> Vec { + Arc::unwrap_or_clone(self.items) + } + pub(crate) fn history_version(&self) -> u64 { self.history_version } @@ -217,6 +256,7 @@ impl ContextManager { } } + #[cfg(test)] pub(crate) fn replace(&mut self, items: Vec) { self.replace_annotated(items.into_iter().map(ResponseItemEnvelope::new).collect()); } diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index fc6fd98491..b26d7de822 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -5,6 +5,7 @@ use crate::context::world_state::WorldState; use crate::context::world_state::WorldStateSection; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; +use codex_history::CodexHarnessMetadata; use codex_history::ResponseItemEnvelope; use codex_protocol::AgentPath; use codex_protocol::ResponseItemId; @@ -476,7 +477,10 @@ fn cloned_history_shares_items_until_mutated() { #[test] fn annotated_history_apis_preserve_envelopes() { let first_item = assistant_msg("first"); - let first_envelope = ResponseItemEnvelope::new(first_item.clone()); + let first_envelope = ResponseItemEnvelope { + item: first_item.clone(), + metadata: Some(CodexHarnessMetadata::default()), + }; let mut history = ContextManager::new(); history.replace_annotated(vec![first_envelope.clone()]); @@ -488,6 +492,62 @@ fn annotated_history_apis_preserve_envelopes() { assert_eq!(history.into_raw_items(), vec![first_item]); } +#[test] +fn record_annotated_items_preserves_metadata_while_processing_item() { + let envelope = ResponseItemEnvelope { + item: ResponseItem::FunctionCallOutput { + id: None, + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload { + body: FunctionCallOutputBody::Text("word ".repeat(100)), + success: Some(true), + }, + internal_chat_message_metadata_passthrough: None, + }, + metadata: Some(CodexHarnessMetadata::default()), + }; + let mut history = ContextManager::new(); + + history.record_annotated_items(std::slice::from_ref(&envelope), TruncationPolicy::Tokens(4)); + + assert_eq!(history.annotated_items().len(), 1); + assert_eq!( + history.annotated_items()[0].metadata, + Some(CodexHarnessMetadata::default()) + ); + assert_ne!(history.annotated_items()[0].item, envelope.item); +} + +#[test] +fn for_prompt_annotated_preserves_metadata_while_normalizing_item() { + let envelope = ResponseItemEnvelope { + item: ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ + ContentItem::InputText { + text: "keep".to_string(), + }, + ContentItem::InputImage { + image_url: "data:image/png;base64,abc".to_string(), + detail: None, + }, + ], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + metadata: Some(CodexHarnessMetadata::default()), + }; + let mut history = ContextManager::new(); + history.replace_annotated(vec![envelope.clone()]); + + let normalized = history.for_prompt_annotated(&[InputModality::Text]); + + assert_eq!(normalized.len(), 1); + assert_eq!(normalized[0].metadata, envelope.metadata); + assert_ne!(normalized[0].item, envelope.item); +} + #[test] fn drop_last_n_user_turns_treats_inter_agent_assistant_messages_as_instruction_turns() { let first_turn = user_input_text_msg("first"); diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index f8823ba858..3899b0a5e5 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -745,7 +745,7 @@ async fn spawn_guardian_review_session( ), None => ( parent_compaction - .map(|item| InitialHistory::Forked(vec![RolloutItem::ResponseItem(item)])), + .map(|item| InitialHistory::Forked(vec![RolloutItem::ResponseItem(item.into())])), 0, None, ), diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 98f2be3b64..b2e5a3987e 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -335,6 +335,7 @@ use codex_core_plugins::RecommendedPluginCandidatesInput; use codex_git_utils::get_git_repo_root; use codex_history::CompactedItem; use codex_history::InitialHistory; +use codex_history::ResponseItemEnvelope; use codex_mcp::McpConfig; use codex_mcp::effective_mcp_servers; use codex_otel::SessionTelemetry; @@ -1491,17 +1492,31 @@ impl Session { // Keep the recorded rollout unchanged. Prepare its reconstructed history before // installing it, so legacy media is processed once for this resume or fork and // will be processed again if the rollout is reconstructed in a future session. - // Never backfill resize notices during replay; only newly recorded items may - // emit them, so the historical model prefix remains unchanged. + // Replay disables image-resize notices, so media preparation remains one-to-one. Keep + // the prior batch behavior and carry history-only metadata in a positional sidecar. + let (mut prepared_history, metadata): (Vec<_>, Vec<_>) = history + .into_iter() + .map(|envelope| (envelope.item, envelope.metadata)) + .unzip(); let _ = prepare_image_response_items( - &mut history, + &mut prepared_history, ImagePreparationMode::DetailBased, ImageResizeNoticeMode::Disabled, ); - prepare_audio_response_items(&mut history); + prepare_audio_response_items(&mut prepared_history); + assert_eq!( + prepared_history.len(), + metadata.len(), + "replay media preparation must remain one-to-one when resize notices are disabled" + ); + history = prepared_history + .into_iter() + .zip(metadata) + .map(|(item, metadata)| ResponseItemEnvelope { item, metadata }) + .collect(); { let mut state = self.state.lock().await; - state.replace_history(history, reference_context_item); + state.replace_annotated_history(history, reference_context_item); if let Some(world_state) = world_state_baseline { state.history.set_world_state_baseline(world_state); } @@ -3027,7 +3042,7 @@ impl Session { fn assign_missing_rollout_response_item_ids(items: &mut [RolloutItem]) { for item in items { if let RolloutItem::ResponseItem(response_item) = item { - Self::assign_missing_response_item_id(response_item); + Self::assign_missing_response_item_id(&mut response_item.item); } } } @@ -3233,7 +3248,7 @@ impl Session { RolloutItem::InterAgentCommunicationMetadata { trigger_turn: communication.trigger_turn, }, - RolloutItem::ResponseItem(response_item), + RolloutItem::ResponseItem(response_item.into()), ]) .await; self.send_raw_response_items(turn_context, items).await; @@ -3311,12 +3326,14 @@ impl Session { pub(crate) async fn replace_compacted_history( &self, - items: Vec, + mut items: Vec, reference_context_item: Option, world_state_baseline: Option>, metadata: CompactedHistoryMetadata, ) { - let items = Self::assign_missing_response_item_ids(Cow::Owned(items)).into_owned(); + for envelope in &mut items { + Self::assign_missing_response_item_id(&mut envelope.item); + } let compacted_item = CompactedItem { message: metadata.message, replacement_history: Some(items.clone()), @@ -3332,7 +3349,7 @@ impl Session { let mut world_state_item = None; { let mut state = self.state.lock().await; - state.replace_history(items, reference_context_item.clone()); + state.replace_annotated_history(items, reference_context_item.clone()); if let Some(world_state) = world_state_baseline { let snapshot = world_state.snapshot(); world_state_item = Some(WorldStateItem::full(snapshot.clone().into_value())); @@ -3361,6 +3378,7 @@ impl Session { let rollout_items: Vec = items .iter() .cloned() + .map(ResponseItemEnvelope::new) .map(RolloutItem::ResponseItem) .collect(); self.persist_rollout_items(&rollout_items).await; @@ -3716,7 +3734,10 @@ impl Session { let (window_number, window_ids) = window; let context_items = self .build_initial_context_with_world_state(turn_context, world_state.as_ref()) - .await; + .await + .into_iter() + .map(ResponseItemEnvelope::new) + .collect(); let turn_context_item = turn_context.to_turn_context_item(); self.replace_compacted_history( context_items, diff --git a/codex-rs/core/src/session/rollout_reconstruction.rs b/codex-rs/core/src/session/rollout_reconstruction.rs index 8a51d3b521..59ae3eba7b 100644 --- a/codex-rs/core/src/session/rollout_reconstruction.rs +++ b/codex-rs/core/src/session/rollout_reconstruction.rs @@ -1,6 +1,7 @@ use super::*; use crate::context::world_state::WorldStateSnapshot; use crate::context_manager::is_user_turn_boundary; +use codex_history::ResponseItemEnvelope; use codex_protocol::protocol::SessionContextWindow; use uuid::Uuid; @@ -8,7 +9,7 @@ use uuid::Uuid; // the resume/fork hydration metadata derived from the same replay. #[derive(Debug)] pub(super) struct RolloutReconstruction { - pub(super) history: Vec, + pub(super) history: Vec, pub(super) previous_turn_settings: Option, pub(super) reference_context_item: Option, pub(super) world_state_baseline: Option, @@ -49,7 +50,7 @@ struct ActiveReplaySegment<'a> { previous_turn_settings: Option, reference_context_item: TurnReferenceContextItem, world_state_replay: Vec<&'a RolloutItem>, - base_replacement_history: Option<&'a [ResponseItem]>, + base_replacement_history: Option<&'a [ResponseItemEnvelope]>, window: Option, } @@ -60,7 +61,7 @@ fn turn_ids_are_compatible(active_turn_id: Option<&str>, item_turn_id: Option<&s fn finalize_active_segment<'a>( active_segment: ActiveReplaySegment<'a>, - base_replacement_history: &mut Option<&'a [ResponseItem]>, + base_replacement_history: &mut Option<&'a [ResponseItemEnvelope]>, previous_turn_settings: &mut Option, reference_context_item: &mut TurnReferenceContextItem, world_state_replay: &mut Vec<&'a RolloutItem>, @@ -136,7 +137,7 @@ impl Session { _ => None, }) }; - let mut base_replacement_history: Option<&[ResponseItem]> = None; + let mut base_replacement_history: Option<&[ResponseItemEnvelope]> = None; let mut previous_turn_settings = None; let mut reference_context_item = TurnReferenceContextItem::NeverSet; let mut world_state_replay = Vec::new(); @@ -271,7 +272,8 @@ impl Session { RolloutItem::ResponseItem(response_item) => { let active_segment = active_segment.get_or_insert_with(ActiveReplaySegment::default); - active_segment.counts_as_user_turn |= is_user_turn_boundary(response_item); + active_segment.counts_as_user_turn |= + is_user_turn_boundary(&response_item.item); } RolloutItem::InterAgentCommunication(_) => { let active_segment = @@ -317,7 +319,7 @@ impl Session { let mut history = ContextManager::new(); let mut saw_legacy_compaction_without_replacement_history = false; if let Some(base_replacement_history) = base_replacement_history { - history.replace(base_replacement_history.to_vec()); + history.replace_annotated(base_replacement_history.to_vec()); } // Materialize exact history semantics from the replay-derived suffix. The eventual lazy // design should keep this same replay shape, but drive it from a resumable reverse source @@ -325,8 +327,8 @@ impl Session { for item in rollout_suffix { match item { RolloutItem::ResponseItem(response_item) => { - history.record_items( - std::iter::once(response_item), + history.record_annotated_items( + std::slice::from_ref(response_item), turn_context.model_info.truncation_policy.into(), ); } @@ -342,7 +344,7 @@ impl Session { if let Some(replacement_history) = &compacted.replacement_history { // This should actually never happen, because the reverse loop above (to build rollout_suffix) // should stop before any compaction that has Some replacement_history - history.replace(replacement_history.clone()); + history.replace_annotated(replacement_history.clone()); } else { saw_legacy_compaction_without_replacement_history = true; // Legacy rollouts without `replacement_history` should rebuild the @@ -360,7 +362,7 @@ impl Session { &user_messages, &compacted.message, ); - history.replace(rebuilt); + history.replace_annotated(rebuilt); } } RolloutItem::EventMsg(EventMsg::ThreadRolledBack(rollback)) => { @@ -429,7 +431,7 @@ impl Session { id: None, }); RolloutReconstruction { - history: history.into_raw_items(), + history: history.into_annotated_items(), previous_turn_settings, reference_context_item, world_state_baseline, diff --git a/codex-rs/core/src/session/rollout_reconstruction_tests.rs b/codex-rs/core/src/session/rollout_reconstruction_tests.rs index 183a990b7a..44f597f500 100644 --- a/codex-rs/core/src/session/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/session/rollout_reconstruction_tests.rs @@ -5,6 +5,7 @@ use super::tests::make_session_and_context; use super::tests::raw_history_items; use codex_history::CompactedItem; use codex_history::InitialHistory; +use codex_history::ResponseItemEnvelope; use codex_history::ResumedHistory; use codex_protocol::AgentPath; use codex_protocol::ThreadId; @@ -44,6 +45,10 @@ fn assistant_message(text: &str) -> ResponseItem { } } +fn annotated(items: Vec) -> Vec { + items.into_iter().map(ResponseItemEnvelope::new).collect() +} + fn inter_agent_assistant_message(text: &str) -> ResponseItem { let communication = InterAgentCommunication::new( AgentPath::root(), @@ -148,6 +153,7 @@ async fn record_initial_history_restores_world_state_baseline() { let mut world_state_items = expected_history .iter() .cloned() + .map(ResponseItemEnvelope::new) .map(RolloutItem::ResponseItem) .collect::>(); world_state_items.push(RolloutItem::WorldState(WorldStateItem::full( @@ -352,8 +358,8 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com RolloutItem::WorldState(WorldStateItem::full(json!({ "test": {"environment": "first"} }))), - RolloutItem::ResponseItem(turn_one_user.clone()), - RolloutItem::ResponseItem(turn_one_assistant.clone()), + RolloutItem::ResponseItem(turn_one_user.clone().into()), + RolloutItem::ResponseItem(turn_one_assistant.clone().into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: first_turn_id, @@ -388,8 +394,8 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com RolloutItem::WorldState(WorldStateItem::patch(json!({ "test": {"environment": "rolled-back"} }))), - RolloutItem::ResponseItem(turn_two_user), - RolloutItem::ResponseItem(turn_two_assistant), + RolloutItem::ResponseItem(turn_two_user.into()), + RolloutItem::ResponseItem(turn_two_assistant.into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: rolled_back_turn_id, @@ -412,7 +418,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com assert_eq!( reconstructed.history, - vec![turn_one_user, turn_one_assistant] + annotated(vec![turn_one_user, turn_one_assistant]) ); assert_eq!( reconstructed.previous_turn_settings, @@ -469,8 +475,8 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_inc }, )), RolloutItem::TurnContext(first_context_item.clone()), - RolloutItem::ResponseItem(turn_one_user.clone()), - RolloutItem::ResponseItem(turn_one_assistant.clone()), + RolloutItem::ResponseItem(turn_one_user.clone().into()), + RolloutItem::ResponseItem(turn_one_assistant.clone().into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: first_turn_id, @@ -501,7 +507,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_inc ..Default::default() }, )), - RolloutItem::ResponseItem(turn_two_user), + RolloutItem::ResponseItem(turn_two_user.into()), RolloutItem::EventMsg(EventMsg::ThreadRolledBack( codex_protocol::protocol::ThreadRolledBackEvent { num_turns: 1 }, )), @@ -513,7 +519,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_inc assert_eq!( reconstructed.history, - vec![turn_one_user, turn_one_assistant] + annotated(vec![turn_one_user, turn_one_assistant]) ); assert_eq!( reconstructed.previous_turn_settings, @@ -568,8 +574,8 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad }, )), RolloutItem::TurnContext(first_context_item.clone()), - RolloutItem::ResponseItem(turn_one_user.clone()), - RolloutItem::ResponseItem(turn_one_assistant.clone()), + RolloutItem::ResponseItem(turn_one_user.clone().into()), + RolloutItem::ResponseItem(turn_one_assistant.clone().into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: first_turn_id, @@ -600,8 +606,8 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad ..Default::default() }, )), - RolloutItem::ResponseItem(turn_two_user), - RolloutItem::ResponseItem(turn_two_assistant), + RolloutItem::ResponseItem(turn_two_user.into()), + RolloutItem::ResponseItem(turn_two_assistant.into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: second_turn_id, @@ -622,7 +628,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad collaboration_mode_kind: ModeKind::Default, }, )), - RolloutItem::ResponseItem(standalone_assistant), + RolloutItem::ResponseItem(standalone_assistant.into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: standalone_turn_id, @@ -645,7 +651,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad assert_eq!( reconstructed.history, - vec![turn_one_user, turn_one_assistant] + annotated(vec![turn_one_user, turn_one_assistant]) ); assert_eq!( reconstructed.previous_turn_settings, @@ -700,8 +706,8 @@ async fn reconstruct_history_rollback_counts_inter_agent_assistant_turns() { }, )), RolloutItem::TurnContext(first_context_item.clone()), - RolloutItem::ResponseItem(user_message("turn 1 user")), - RolloutItem::ResponseItem(assistant_message("turn 1 assistant")), + RolloutItem::ResponseItem(user_message("turn 1 user").into()), + RolloutItem::ResponseItem(assistant_message("turn 1 assistant").into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: first_turn_id, @@ -723,8 +729,8 @@ async fn reconstruct_history_rollback_counts_inter_agent_assistant_turns() { }, )), RolloutItem::TurnContext(assistant_turn_context), - RolloutItem::ResponseItem(assistant_instruction), - RolloutItem::ResponseItem(assistant_reply), + RolloutItem::ResponseItem(assistant_instruction.into()), + RolloutItem::ResponseItem(assistant_reply.into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: assistant_turn_id, @@ -747,10 +753,10 @@ async fn reconstruct_history_rollback_counts_inter_agent_assistant_turns() { assert_eq!( reconstructed.history, - vec![ + annotated(vec![ user_message("turn 1 user"), assistant_message("turn 1 assistant") - ] + ]) ); assert_eq!( reconstructed.previous_turn_settings, @@ -797,8 +803,8 @@ async fn reconstruct_history_rollback_clears_history_and_metadata_when_exceeding }, )), RolloutItem::TurnContext(only_context_item), - RolloutItem::ResponseItem(user_message("only user")), - RolloutItem::ResponseItem(assistant_message("only assistant")), + RolloutItem::ResponseItem(user_message("only user").into()), + RolloutItem::ResponseItem(assistant_message("only assistant").into()), RolloutItem::EventMsg(EventMsg::TurnComplete( codex_protocol::protocol::TurnCompleteEvent { turn_id: only_turn_id, @@ -1200,8 +1206,8 @@ async fn reconstruct_history_legacy_compaction_without_replacement_history_does_ { let (session, turn_context) = make_session_and_context().await; let rollout_items = vec![ - RolloutItem::ResponseItem(user_message("before compact")), - RolloutItem::ResponseItem(assistant_message("assistant reply")), + RolloutItem::ResponseItem(user_message("before compact").into()), + RolloutItem::ResponseItem(assistant_message("assistant reply").into()), RolloutItem::Compacted(CompactedItem { message: "legacy summary".to_string(), replacement_history: None, @@ -1218,10 +1224,10 @@ async fn reconstruct_history_legacy_compaction_without_replacement_history_does_ assert_eq!( reconstructed.history, - vec![ + annotated(vec![ user_message("before compact"), user_message("legacy summary"), - ] + ]) ); assert!(reconstructed.reference_context_item.is_none()); } @@ -1236,7 +1242,7 @@ async fn reconstruct_history_legacy_compaction_without_replacement_history_clear .clone() .expect("turn context should have turn_id"); let rollout_items = vec![ - RolloutItem::ResponseItem(user_message("before compact")), + RolloutItem::ResponseItem(user_message("before compact").into()), RolloutItem::Compacted(CompactedItem { message: "legacy summary".to_string(), replacement_history: None, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 03bed2c0c3..08327d38d0 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -105,8 +105,10 @@ use codex_config::permissions_toml::PermissionsToml; use codex_execpolicy::Decision; use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; +use codex_history::CodexHarnessMetadata; use codex_history::CompactedItem; use codex_history::InitialHistory; +use codex_history::ResponseItemEnvelope; use codex_history::ResumedHistory; use codex_history::RolloutItem; use codex_network_proxy::NetworkProxyConfig; @@ -660,6 +662,10 @@ pub(super) fn raw_history_items(history: &ContextManager) -> Vec { history.raw_items().cloned().collect() } +fn raw_envelopes(items: &[ResponseItemEnvelope]) -> Vec { + items.iter().map(|envelope| envelope.item.clone()).collect() +} + fn developer_input_texts(items: &[ResponseItem]) -> Vec<&str> { items .iter() @@ -1988,7 +1994,7 @@ async fn reconstruct_history_matches_live_compactions() { .reconstruct_history_from_rollout(reconstruction_turn.as_ref(), &rollout_items) .await; - assert_eq!(expected, reconstructed.history); + assert_eq!(expected, raw_envelopes(&reconstructed.history)); assert_eq!(2, reconstructed.window_number); assert_eq!( reconstructed @@ -2014,8 +2020,11 @@ async fn reconstruct_history_uses_replacement_history_verbatim() { }), }; let replacement_history = vec![ - summary_item.clone(), - ResponseItem::Message { + ResponseItemEnvelope { + item: summary_item.clone(), + metadata: Some(CodexHarnessMetadata::default()), + }, + ResponseItemEnvelope::new(ResponseItem::Message { id: None, role: "developer".to_string(), content: vec![ContentItem::InputText { @@ -2023,7 +2032,7 @@ async fn reconstruct_history_uses_replacement_history_verbatim() { }], phase: None, internal_chat_message_metadata_passthrough: None, - }, + }), ]; let first_window_id = Uuid::now_v7(); let previous_window_id = Uuid::now_v7(); @@ -2213,7 +2222,7 @@ async fn record_inter_agent_communication_sets_turn_id_in_rollout_and_resume() { RolloutItem::InterAgentCommunicationMetadata { trigger_turn: false, }, - RolloutItem::ResponseItem(expected_item.clone()), + RolloutItem::ResponseItem(expected_item.clone().into()), ]; assert_eq!( strip_response_item_ids_from_json(serde_json::to_value(persisted_items).unwrap()), @@ -2271,7 +2280,11 @@ async fn record_inter_agent_communication_preserves_item_id_in_rollout_and_resum panic!("expected resumed rollout history"); }; let persisted_item_id = resumed.history.iter().find_map(|item| match item { - RolloutItem::ResponseItem(item @ ResponseItem::AgentMessage { .. }) => item.id(), + RolloutItem::ResponseItem(item) + if matches!(&item.item, ResponseItem::AgentMessage { .. }) => + { + item.id() + } _ => None, }); assert_eq!( @@ -2398,13 +2411,17 @@ async fn prepares_resumed_history_before_installing_it() { session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: Arc::new(vec![RolloutItem::ResponseItem(resumed_item)]), + history: Arc::new(vec![RolloutItem::ResponseItem(ResponseItemEnvelope { + item: resumed_item, + metadata: Some(CodexHarnessMetadata::default()), + })]), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; + let history = session.state.lock().await.clone_history(); assert_eq!( - raw_history_items(&session.state.lock().await.clone_history()), + raw_history_items(&history), vec![ResponseItem::Message { id: None, role: "user".to_string(), @@ -2424,6 +2441,10 @@ async fn prepares_resumed_history_before_installing_it() { internal_chat_message_metadata_passthrough: None, }] ); + assert_eq!( + history.annotated_items()[0].metadata, + Some(CodexHarnessMetadata::default()) + ); } #[test] @@ -3216,7 +3237,7 @@ async fn start_new_context_window_assigns_and_persists_item_ids() { }); assert_eq!( persisted_replacement_history.cloned(), - Some(raw_history_items(&live_history)) + Some(live_history.annotated_items().to_vec()) ); } @@ -3235,6 +3256,10 @@ async fn record_initial_history_assigns_and_persists_id_for_forked_response_item ]) .expect("developer message"); let mut expected_item = response_item.clone(); + let response_item = ResponseItemEnvelope { + item: response_item, + metadata: Some(CodexHarnessMetadata::default()), + }; session .record_initial_history(InitialHistory::Forked(vec![RolloutItem::ResponseItem( @@ -3254,6 +3279,10 @@ async fn record_initial_history_assigns_and_persists_id_for_forked_response_item assert!(live_item_id.starts_with("msg_")); expected_item.set_id(live_item.id().cloned()); assert_eq!(raw_history_items(&live_history), vec![expected_item]); + assert_eq!( + live_history.annotated_items()[0].metadata, + Some(CodexHarnessMetadata::default()) + ); session.flush_rollout().await.expect("rollout should flush"); let InitialHistory::Resumed(resumed) = RolloutRecorder::get_rollout_history(&rollout_path) @@ -3262,8 +3291,8 @@ async fn record_initial_history_assigns_and_persists_id_for_forked_response_item else { panic!("expected resumed rollout history"); }; - let persisted_item_id = resumed.history.iter().find_map(|item| match item { - RolloutItem::ResponseItem(response_item) => response_item.id(), + let persisted_item = resumed.history.iter().find_map(|item| match item { + RolloutItem::ResponseItem(response_item) => Some(response_item), RolloutItem::SessionMeta(_) | RolloutItem::InterAgentCommunication(_) | RolloutItem::InterAgentCommunicationMetadata { .. } @@ -3272,10 +3301,15 @@ async fn record_initial_history_assigns_and_persists_id_for_forked_response_item | RolloutItem::WorldState(_) | RolloutItem::EventMsg(_) => None, }); + let persisted_item = persisted_item.expect("forked response item should be persisted"); assert_eq!( - persisted_item_id.map(ResponseItemId::as_str), + persisted_item.id().map(ResponseItemId::as_str), Some(live_item_id.as_str()) ); + assert_eq!( + persisted_item.metadata, + Some(CodexHarnessMetadata::default()) + ); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -3532,6 +3566,7 @@ async fn thread_rollback_drops_last_turn_from_history() { .await; let rollout_items: Vec = full_history .into_iter() + .map(ResponseItemEnvelope::new) .map(RolloutItem::ResponseItem) .collect(); sess.persist_rollout_items(&rollout_items).await; @@ -3592,6 +3627,7 @@ async fn thread_rollback_clears_history_when_num_turns_exceeds_existing_turns() .await; let rollout_items: Vec = full_history .into_iter() + .map(ResponseItemEnvelope::new) .map(RolloutItem::ResponseItem) .collect(); sess.persist_rollout_items(&rollout_items).await; @@ -3677,8 +3713,8 @@ async fn thread_rollback_recomputes_previous_turn_settings_and_reference_context }, )), RolloutItem::TurnContext(first_context_item.clone()), - RolloutItem::ResponseItem(turn_one_user.clone()), - RolloutItem::ResponseItem(turn_one_assistant.clone()), + RolloutItem::ResponseItem(turn_one_user.clone().into()), + RolloutItem::ResponseItem(turn_one_assistant.clone().into()), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: first_turn_id, started_at: None, @@ -3708,8 +3744,8 @@ async fn thread_rollback_recomputes_previous_turn_settings_and_reference_context }, )), RolloutItem::TurnContext(rolled_back_context_item), - RolloutItem::ResponseItem(turn_two_user), - RolloutItem::ResponseItem(turn_two_assistant), + RolloutItem::ResponseItem(turn_two_user.into()), + RolloutItem::ResponseItem(turn_two_assistant.into()), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: rolled_back_turn_id, started_at: None, @@ -3799,8 +3835,8 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio ..Default::default() })), RolloutItem::TurnContext(first_context_item.clone()), - RolloutItem::ResponseItem(user_message("turn 1 user")), - RolloutItem::ResponseItem(assistant_message("turn 1 assistant")), + RolloutItem::ResponseItem(user_message("turn 1 user").into()), + RolloutItem::ResponseItem(assistant_message("turn 1 assistant").into()), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: first_turn_id, started_at: None, @@ -3821,7 +3857,13 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio )), RolloutItem::Compacted(CompactedItem { message: "summary after compaction".to_string(), - replacement_history: Some(compacted_history.clone()), + replacement_history: Some( + compacted_history + .iter() + .cloned() + .map(ResponseItemEnvelope::new) + .collect(), + ), window_number: Some(7), first_window_id: Some(first_window_id.to_string()), previous_window_id: Some(previous_window_id.to_string()), @@ -3859,8 +3901,8 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio comp_hash: None, ..first_context_item.clone() }), - RolloutItem::ResponseItem(user_message("turn 2 user")), - RolloutItem::ResponseItem(assistant_message("turn 2 assistant")), + RolloutItem::ResponseItem(user_message("turn 2 user").into()), + RolloutItem::ResponseItem(assistant_message("turn 2 assistant").into()), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: rolled_back_turn_id, started_at: None, @@ -3937,8 +3979,8 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() { ..Default::default() })), RolloutItem::TurnContext(turn_context_item.clone()), - RolloutItem::ResponseItem(user_message("turn 1 user")), - RolloutItem::ResponseItem(assistant_message("turn 1 assistant")), + RolloutItem::ResponseItem(user_message("turn 1 user").into()), + RolloutItem::ResponseItem(assistant_message("turn 1 assistant").into()), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-1".to_string(), started_at: None, @@ -3966,8 +4008,8 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() { ..Default::default() })), RolloutItem::TurnContext(turn_context_item.clone()), - RolloutItem::ResponseItem(user_message("turn 2 user")), - RolloutItem::ResponseItem(assistant_message("turn 2 assistant")), + RolloutItem::ResponseItem(user_message("turn 2 user").into()), + RolloutItem::ResponseItem(assistant_message("turn 2 assistant").into()), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-2".to_string(), started_at: None, @@ -3995,8 +4037,8 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() { ..Default::default() })), RolloutItem::TurnContext(turn_context_item), - RolloutItem::ResponseItem(user_message("turn 3 user")), - RolloutItem::ResponseItem(assistant_message("turn 3 assistant")), + RolloutItem::ResponseItem(user_message("turn 3 user").into()), + RolloutItem::ResponseItem(assistant_message("turn 3 assistant").into()), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-3".to_string(), started_at: None, @@ -11438,7 +11480,7 @@ async fn sample_rollout( initial_context.insert(insert_at, msg); } for item in &initial_context { - rollout_items.push(RolloutItem::ResponseItem(item.clone())); + rollout_items.push(RolloutItem::ResponseItem(item.clone().into())); } live_history.record_items( initial_context.iter(), @@ -11458,7 +11500,7 @@ async fn sample_rollout( std::iter::once(&user1), reconstruction_turn.model_info.truncation_policy.into(), ); - rollout_items.push(RolloutItem::ResponseItem(user1.clone())); + rollout_items.push(RolloutItem::ResponseItem(user1.clone().into())); let assistant1 = ResponseItem::Message { id: None, @@ -11473,7 +11515,7 @@ async fn sample_rollout( std::iter::once(&assistant1), reconstruction_turn.model_info.truncation_policy.into(), ); - rollout_items.push(RolloutItem::ResponseItem(assistant1.clone())); + rollout_items.push(RolloutItem::ResponseItem(assistant1.clone().into())); let summary1 = "summary one"; let snapshot1 = live_history @@ -11481,7 +11523,7 @@ async fn sample_rollout( .for_prompt(&reconstruction_turn.model_info.input_modalities); let user_messages1 = collect_user_messages(&snapshot1); let rebuilt1 = compact::build_compacted_history(Vec::new(), &user_messages1, summary1); - live_history.replace(rebuilt1); + live_history.replace_annotated(rebuilt1); let (window_number, window_ids) = session.advance_auto_compact_window().await; rollout_items.push(RolloutItem::Compacted(CompactedItem { message: summary1.to_string(), @@ -11505,7 +11547,7 @@ async fn sample_rollout( std::iter::once(&user2), reconstruction_turn.model_info.truncation_policy.into(), ); - rollout_items.push(RolloutItem::ResponseItem(user2.clone())); + rollout_items.push(RolloutItem::ResponseItem(user2.clone().into())); let assistant2 = ResponseItem::Message { id: None, @@ -11520,7 +11562,7 @@ async fn sample_rollout( std::iter::once(&assistant2), reconstruction_turn.model_info.truncation_policy.into(), ); - rollout_items.push(RolloutItem::ResponseItem(assistant2.clone())); + rollout_items.push(RolloutItem::ResponseItem(assistant2.clone().into())); let summary2 = "summary two"; let snapshot2 = live_history @@ -11528,7 +11570,7 @@ async fn sample_rollout( .for_prompt(&reconstruction_turn.model_info.input_modalities); let user_messages2 = collect_user_messages(&snapshot2); let rebuilt2 = compact::build_compacted_history(Vec::new(), &user_messages2, summary2); - live_history.replace(rebuilt2); + live_history.replace_annotated(rebuilt2); let (window_number, window_ids) = session.advance_auto_compact_window().await; rollout_items.push(RolloutItem::Compacted(CompactedItem { message: summary2.to_string(), @@ -11552,7 +11594,7 @@ async fn sample_rollout( std::iter::once(&user3), reconstruction_turn.model_info.truncation_policy.into(), ); - rollout_items.push(RolloutItem::ResponseItem(user3)); + rollout_items.push(RolloutItem::ResponseItem(user3.into())); let assistant3 = ResponseItem::Message { id: None, @@ -11567,7 +11609,7 @@ async fn sample_rollout( std::iter::once(&assistant3), reconstruction_turn.model_info.truncation_policy.into(), ); - rollout_items.push(RolloutItem::ResponseItem(assistant3)); + rollout_items.push(RolloutItem::ResponseItem(assistant3.into())); ( rollout_items, diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index 4dd3b6b940..7a19b98e08 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -17,6 +17,7 @@ use crate::session::PreviousTurnSettings; use crate::session::session::SessionConfiguration; use crate::session::time_reminder::CurrentTimeReminderState; use crate::session_startup_prewarm::SessionStartupPrewarmHandle; +use codex_history::ResponseItemEnvelope; use codex_protocol::protocol::RateLimitSnapshot; use codex_protocol::protocol::TokenUsage; use codex_protocol::protocol::TokenUsageInfo; @@ -115,6 +116,7 @@ impl SessionState { self.history.clone() } + #[cfg(test)] pub(crate) fn replace_history( &mut self, items: Vec, @@ -126,6 +128,17 @@ impl SessionState { self.auto_compact_window.clear_prefill(); } + pub(crate) fn replace_annotated_history( + &mut self, + items: Vec, + reference_context_item: Option, + ) { + self.history.replace_annotated(items); + self.history + .set_reference_context_item(reference_context_item); + self.auto_compact_window.clear_prefill(); + } + pub(crate) fn set_token_info(&mut self, info: Option) { self.history.set_token_info(info); } diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index aa532421af..d5297df04f 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -2120,14 +2120,14 @@ fn append_interrupted_boundary( InitialHistory::New | InitialHistory::Cleared => { let mut history = Vec::new(); if let Some(marker) = interrupted_turn_history_marker(interrupted_marker) { - history.push(RolloutItem::ResponseItem(marker)); + history.push(RolloutItem::ResponseItem(marker.into())); } history.push(aborted_event); InitialHistory::Forked(history) } InitialHistory::Forked(mut history) => { if let Some(marker) = interrupted_turn_history_marker(interrupted_marker) { - history.push(RolloutItem::ResponseItem(marker)); + history.push(RolloutItem::ResponseItem(marker.into())); } history.push(aborted_event); InitialHistory::Forked(history) @@ -2135,7 +2135,7 @@ fn append_interrupted_boundary( InitialHistory::Resumed(resumed) => { let mut history = Arc::unwrap_or_clone(resumed.history); if let Some(marker) = interrupted_turn_history_marker(interrupted_marker) { - history.push(RolloutItem::ResponseItem(marker)); + history.push(RolloutItem::ResponseItem(marker.into())); } history.push(aborted_event); InitialHistory::Forked(history) diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index afcef58be9..6589ae3d68 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -409,7 +409,7 @@ fn truncates_before_requested_user_message() { let initial: Vec = items .iter() .cloned() - .map(RolloutItem::ResponseItem) + .map(|item| RolloutItem::ResponseItem(item.into())) .collect(); let truncated = truncate_before_nth_user_message( InitialHistory::Forked(initial), @@ -423,9 +423,9 @@ fn truncates_before_requested_user_message() { ); let got_items = truncated.get_rollout_items(); let expected_items = vec![ - RolloutItem::ResponseItem(items[0].clone()), - RolloutItem::ResponseItem(items[1].clone()), - RolloutItem::ResponseItem(items[2].clone()), + RolloutItem::ResponseItem(items[0].clone().into()), + RolloutItem::ResponseItem(items[1].clone().into()), + RolloutItem::ResponseItem(items[2].clone().into()), ]; assert_eq!( serde_json::to_value(got_items).unwrap(), @@ -435,7 +435,7 @@ fn truncates_before_requested_user_message() { let initial2: Vec = items .iter() .cloned() - .map(RolloutItem::ResponseItem) + .map(|item| RolloutItem::ResponseItem(item.into())) .collect(); let truncated2 = truncate_before_nth_user_message( InitialHistory::Forked(initial2.clone()), @@ -456,10 +456,10 @@ fn truncates_before_requested_user_message() { #[test] fn out_of_range_truncation_drops_only_unfinished_suffix_mid_turn() { let items = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(assistant_msg("a1")), - RolloutItem::ResponseItem(user_msg("u2")), - RolloutItem::ResponseItem(assistant_msg("partial")), + RolloutItem::ResponseItem(user_msg("u1").into()), + RolloutItem::ResponseItem(assistant_msg("a1").into()), + RolloutItem::ResponseItem(user_msg("u2").into()), + RolloutItem::ResponseItem(assistant_msg("partial").into()), ]; let truncated = truncate_before_nth_user_message( @@ -501,8 +501,8 @@ fn fork_thread_accepts_legacy_usize_snapshot_argument() { #[test] fn out_of_range_truncation_drops_pre_user_active_turn_prefix() { let items = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(assistant_msg("a1")), + RolloutItem::ResponseItem(user_msg("u1").into()), + RolloutItem::ResponseItem(assistant_msg("a1").into()), RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent { turn_id: "turn-2".to_string(), trace_id: None, @@ -510,8 +510,8 @@ fn out_of_range_truncation_drops_pre_user_active_turn_prefix() { model_context_window: None, collaboration_mode_kind: Default::default(), })), - RolloutItem::ResponseItem(user_msg("u2")), - RolloutItem::ResponseItem(assistant_msg("partial")), + RolloutItem::ResponseItem(user_msg("u2").into()), + RolloutItem::ResponseItem(assistant_msg("partial").into()), ]; let snapshot_state = snapshot_turn_state(&InitialHistory::Forked(items.clone())); @@ -553,7 +553,7 @@ async fn ignores_session_prefix_messages_when_truncating() { let rollout_items: Vec = items .iter() .cloned() - .map(RolloutItem::ResponseItem) + .map(|item| RolloutItem::ResponseItem(item.into())) .collect(); let truncated = truncate_before_nth_user_message( @@ -569,10 +569,10 @@ async fn ignores_session_prefix_messages_when_truncating() { let got_items = truncated.get_rollout_items(); let expected: Vec = vec![ - RolloutItem::ResponseItem(items[0].clone()), - RolloutItem::ResponseItem(items[1].clone()), - RolloutItem::ResponseItem(items[2].clone()), - RolloutItem::ResponseItem(items[3].clone()), + RolloutItem::ResponseItem(items[0].clone().into()), + RolloutItem::ResponseItem(items[1].clone().into()), + RolloutItem::ResponseItem(items[2].clone().into()), + RolloutItem::ResponseItem(items[3].clone().into()), ]; assert_eq!( @@ -1596,7 +1596,7 @@ async fn rollout_path_resume_and_fork_read_history_through_thread_store() { config.clone(), InitialHistory::Resumed(ResumedHistory { conversation_id: source.thread_id, - history: Arc::new(vec![RolloutItem::ResponseItem(user_msg("hello"))]), + history: Arc::new(vec![RolloutItem::ResponseItem(user_msg("hello").into())]), rollout_path: Some(rollout_path.clone()), }), auth_manager.clone(), @@ -1751,7 +1751,7 @@ async fn injected_models_manager_controls_refresh_policy() { #[test] fn interrupted_fork_snapshot_appends_interrupt_boundary() { let committed_history = - InitialHistory::Forked(vec![RolloutItem::ResponseItem(user_msg("hello"))]); + InitialHistory::Forked(vec![RolloutItem::ResponseItem(user_msg("hello").into())]); assert_eq!( serde_json::to_value( @@ -1765,8 +1765,8 @@ fn interrupted_fork_snapshot_appends_interrupt_boundary() { ) .expect("serialize interrupted fork history"), serde_json::to_value(vec![ - RolloutItem::ResponseItem(user_msg("hello")), - RolloutItem::ResponseItem(contextual_user_interrupted_marker()), + RolloutItem::ResponseItem(user_msg("hello").into()), + RolloutItem::ResponseItem(contextual_user_interrupted_marker().into()), RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { turn_id: None, started_at: None, @@ -1789,7 +1789,7 @@ fn interrupted_fork_snapshot_appends_interrupt_boundary() { ) .expect("serialize interrupted empty fork history"), serde_json::to_value(vec![ - RolloutItem::ResponseItem(contextual_user_interrupted_marker()), + RolloutItem::ResponseItem(contextual_user_interrupted_marker().into()), RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { turn_id: None, started_at: None, @@ -1805,7 +1805,7 @@ fn interrupted_fork_snapshot_appends_interrupt_boundary() { #[test] fn disabled_interrupted_fork_snapshot_appends_only_interrupt_event() { let committed_history = - InitialHistory::Forked(vec![RolloutItem::ResponseItem(user_msg("hello"))]); + InitialHistory::Forked(vec![RolloutItem::ResponseItem(user_msg("hello").into())]); assert_eq!( serde_json::to_value( @@ -1819,7 +1819,7 @@ fn disabled_interrupted_fork_snapshot_appends_only_interrupt_event() { ) .expect("serialize disabled interrupted fork history"), serde_json::to_value(vec![ - RolloutItem::ResponseItem(user_msg("hello")), + RolloutItem::ResponseItem(user_msg("hello").into()), RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { turn_id: None, started_at: None, @@ -1857,9 +1857,9 @@ fn disabled_interrupted_fork_snapshot_appends_only_interrupt_event() { #[test] fn interrupted_snapshot_is_not_mid_turn() { let interrupted_history = InitialHistory::Forked(vec![ - RolloutItem::ResponseItem(user_msg("hello")), - RolloutItem::ResponseItem(assistant_msg("partial")), - RolloutItem::ResponseItem(contextual_user_interrupted_marker()), + RolloutItem::ResponseItem(user_msg("hello").into()), + RolloutItem::ResponseItem(assistant_msg("partial").into()), + RolloutItem::ResponseItem(contextual_user_interrupted_marker().into()), RolloutItem::EventMsg(EventMsg::TurnAborted(TurnAbortedEvent { turn_id: Some("turn-1".to_string()), started_at: None, @@ -1930,7 +1930,7 @@ fn completed_legacy_event_history_is_not_mid_turn() { #[test] fn mixed_response_and_legacy_user_event_history_is_mid_turn() { let mixed_history = InitialHistory::Forked(vec![ - RolloutItem::ResponseItem(user_msg("hello")), + RolloutItem::ResponseItem(user_msg("hello").into()), RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent { client_id: None, message: "hello".to_string(), @@ -1984,8 +1984,8 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor .resume_thread_with_history( config.clone(), InitialHistory::Forked(vec![ - RolloutItem::ResponseItem(user_msg("hello")), - RolloutItem::ResponseItem(assistant_msg("partial")), + RolloutItem::ResponseItem(user_msg("hello").into()), + RolloutItem::ResponseItem(assistant_msg("partial").into()), ]), auth_manager, /*parent_trace*/ None, @@ -2029,7 +2029,7 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) .collect(); let interrupted_marker_json = serde_json::to_value(RolloutItem::ResponseItem( - contextual_user_interrupted_marker(), + contextual_user_interrupted_marker().into(), )) .expect("serialize interrupted marker"); let interrupted_abort_json = serde_json::to_value(RolloutItem::EventMsg( @@ -2104,8 +2104,8 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { model_context_window: None, collaboration_mode_kind: Default::default(), })), - RolloutItem::ResponseItem(user_msg("hello")), - RolloutItem::ResponseItem(assistant_msg("partial")), + RolloutItem::ResponseItem(user_msg("hello").into()), + RolloutItem::ResponseItem(assistant_msg("partial").into()), ]), auth_manager, /*parent_trace*/ None, @@ -2200,8 +2200,8 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ .resume_thread_with_history( config.clone(), InitialHistory::Forked(vec![ - RolloutItem::ResponseItem(user_msg("hello")), - RolloutItem::ResponseItem(assistant_msg("partial")), + RolloutItem::ResponseItem(user_msg("hello").into()), + RolloutItem::ResponseItem(assistant_msg("partial").into()), ]), auth_manager, /*parent_trace*/ None, @@ -2244,7 +2244,7 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) .collect(); let interrupted_marker_json = serde_json::to_value(RolloutItem::ResponseItem( - contextual_user_interrupted_marker(), + contextual_user_interrupted_marker().into(), )) .expect("serialize interrupted marker"); assert_eq!( diff --git a/codex-rs/core/src/thread_rollout_truncation.rs b/codex-rs/core/src/thread_rollout_truncation.rs index 93b1727583..cdbde89414 100644 --- a/codex-rs/core/src/thread_rollout_truncation.rs +++ b/codex-rs/core/src/thread_rollout_truncation.rs @@ -40,11 +40,12 @@ pub(crate) fn user_message_positions_in_rollout(items: &[RolloutItem]) -> Vec + RolloutItem::ResponseItem(item) + if matches!(&item.item, ResponseItem::Message { .. }) + && matches!( + event_mapping::parse_turn_item(&item.item), + Some(TurnItem::UserMessage(_)) + ) => { user_positions.push(idx); } @@ -76,7 +77,7 @@ pub(crate) fn fork_turn_positions_in_rollout(items: &[RolloutItem]) -> Vec { - let has_delivery_metadata = matches!(item, ResponseItem::AgentMessage { .. }) + let has_delivery_metadata = matches!(&item.item, ResponseItem::AgentMessage { .. }) && idx.checked_sub(1).is_some_and(|previous_idx| { matches!( items.get(previous_idx), diff --git a/codex-rs/core/src/thread_rollout_truncation_tests.rs b/codex-rs/core/src/thread_rollout_truncation_tests.rs index 6785f0dcb5..9bdef1a08c 100644 --- a/codex-rs/core/src/thread_rollout_truncation_tests.rs +++ b/codex-rs/core/src/thread_rollout_truncation_tests.rs @@ -14,6 +14,10 @@ use codex_protocol::protocol::UserMessageEvent; use pretty_assertions::assert_eq; use std::sync::Arc; +fn response_item(item: ResponseItem) -> RolloutItem { + RolloutItem::ResponseItem(item.into()) +} + fn user_msg(text: &str) -> ResponseItem { ResponseItem::Message { id: None, @@ -283,20 +287,16 @@ fn truncates_rollout_from_start_before_nth_user_only() { assistant_msg("a4"), ]; - let rollout: Vec = items - .iter() - .cloned() - .map(RolloutItem::ResponseItem) - .collect(); + let rollout: Vec = items.iter().cloned().map(response_item).collect(); let truncated = truncate_rollout_before_nth_user_message_from_start( rollout.clone(), /*n_from_start*/ 1, ); let expected = vec![ - RolloutItem::ResponseItem(items[0].clone()), - RolloutItem::ResponseItem(items[1].clone()), - RolloutItem::ResponseItem(items[2].clone()), + response_item(items[0].clone()), + response_item(items[1].clone()), + response_item(items[2].clone()), ]; assert_eq!( serde_json::to_value(&truncated).unwrap(), @@ -316,9 +316,9 @@ fn truncates_rollout_from_start_before_nth_user_only() { #[test] fn truncation_max_keeps_full_rollout() { let rollout = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(assistant_msg("a1")), - RolloutItem::ResponseItem(user_msg("u2")), + response_item(user_msg("u1")), + response_item(assistant_msg("a1")), + response_item(user_msg("u2")), ]; let truncated = @@ -333,17 +333,17 @@ fn truncation_max_keeps_full_rollout() { #[test] fn truncates_rollout_from_start_applies_thread_rollback_markers() { let rollout_items = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(assistant_msg("a1")), - RolloutItem::ResponseItem(user_msg("u2")), - RolloutItem::ResponseItem(assistant_msg("a2")), + response_item(user_msg("u1")), + response_item(assistant_msg("a1")), + response_item(user_msg("u2")), + response_item(assistant_msg("a2")), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, })), - RolloutItem::ResponseItem(user_msg("u3")), - RolloutItem::ResponseItem(assistant_msg("a3")), - RolloutItem::ResponseItem(user_msg("u4")), - RolloutItem::ResponseItem(assistant_msg("a4")), + response_item(user_msg("u3")), + response_item(assistant_msg("a3")), + response_item(user_msg("u4")), + response_item(assistant_msg("a4")), ]; // Effective user history after applying rollback(1) is: u1, u3, u4. @@ -372,19 +372,15 @@ async fn ignores_session_prefix_messages_when_truncating_rollout_from_start() { items.push(user_msg("second question")); items.push(assistant_msg("answer")); - let rollout_items: Vec = items - .iter() - .cloned() - .map(RolloutItem::ResponseItem) - .collect(); + let rollout_items: Vec = items.iter().cloned().map(response_item).collect(); let truncated = truncate_rollout_before_nth_user_message_from_start(rollout_items, /*n_from_start*/ 1); let expected: Vec = vec![ - RolloutItem::ResponseItem(items[0].clone()), - RolloutItem::ResponseItem(items[1].clone()), - RolloutItem::ResponseItem(items[2].clone()), - RolloutItem::ResponseItem(items[3].clone()), + response_item(items[0].clone()), + response_item(items[1].clone()), + response_item(items[2].clone()), + response_item(items[3].clone()), ]; assert_eq!( @@ -396,20 +392,20 @@ async fn ignores_session_prefix_messages_when_truncating_rollout_from_start() { #[test] fn truncates_rollout_to_last_n_fork_turns_counts_trigger_turn_messages() { let rollout = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(assistant_msg("a1")), - RolloutItem::ResponseItem(inter_agent_msg( + response_item(user_msg("u1")), + response_item(assistant_msg("a1")), + response_item(inter_agent_msg( "queued message", /*trigger_turn*/ false, )), - RolloutItem::ResponseItem(assistant_msg("a2")), - RolloutItem::ResponseItem(inter_agent_msg( + response_item(assistant_msg("a2")), + response_item(inter_agent_msg( "triggered task", /*trigger_turn*/ true, )), - RolloutItem::ResponseItem(assistant_msg("a3")), - RolloutItem::ResponseItem(user_msg("u2")), - RolloutItem::ResponseItem(assistant_msg("a4")), + response_item(assistant_msg("a3")), + response_item(user_msg("u2")), + response_item(assistant_msg("a4")), ]; let truncated = truncate_rollout_to_last_n_fork_turns(rollout.clone(), /*n_from_end*/ 2); @@ -424,12 +420,12 @@ fn truncates_rollout_to_last_n_fork_turns_counts_trigger_turn_messages() { #[test] fn fork_turn_positions_use_inter_agent_delivery_metadata() { let rollout = vec![ - RolloutItem::ResponseItem(user_msg("user task")), + response_item(user_msg("user task")), inter_agent_communication("queued during user turn", /*trigger_turn*/ false), - RolloutItem::ResponseItem(assistant_msg("first answer")), + response_item(assistant_msg("first answer")), inter_agent_communication("follow-up task", /*trigger_turn*/ true), - RolloutItem::ResponseItem(assistant_msg("second answer")), - RolloutItem::ResponseItem(user_msg("next user task")), + response_item(assistant_msg("second answer")), + response_item(user_msg("next user task")), ]; assert_eq!(fork_turn_positions_in_rollout(&rollout), vec![0, 3, 5]); @@ -452,16 +448,16 @@ fn fork_turn_positions_use_canonical_agent_messages_and_delivery_metadata() { /*trigger_turn*/ true, ); let mut rollout = vec![ - RolloutItem::ResponseItem(user_msg("user task")), + response_item(user_msg("user task")), RolloutItem::InterAgentCommunicationMetadata { trigger_turn: false, }, - RolloutItem::ResponseItem(queued.to_model_input_item()), - RolloutItem::ResponseItem(assistant_msg("first answer")), + response_item(queued.to_model_input_item()), + response_item(assistant_msg("first answer")), RolloutItem::InterAgentCommunicationMetadata { trigger_turn: true }, - RolloutItem::ResponseItem(triggered.to_model_input_item()), - RolloutItem::ResponseItem(assistant_msg("second answer")), - RolloutItem::ResponseItem(user_msg("next user task")), + response_item(triggered.to_model_input_item()), + response_item(assistant_msg("second answer")), + response_item(user_msg("next user task")), ]; assert_eq!(fork_turn_positions_in_rollout(&rollout), vec![0, 4, 7]); @@ -478,9 +474,9 @@ fn fork_turn_positions_use_canonical_agent_messages_and_delivery_metadata() { #[test] fn truncates_rollout_to_last_n_fork_turns_drops_startup_prefix_even_when_under_limit() { let rollout = vec![ - RolloutItem::ResponseItem(developer_msg("startup developer context")), - RolloutItem::ResponseItem(user_msg("current task")), - RolloutItem::ResponseItem(assistant_msg("answer")), + response_item(developer_msg("startup developer context")), + response_item(user_msg("current task")), + response_item(assistant_msg("answer")), ]; let truncated = truncate_rollout_to_last_n_fork_turns(rollout.clone(), /*n_from_end*/ 2); @@ -495,18 +491,18 @@ fn truncates_rollout_to_last_n_fork_turns_drops_startup_prefix_even_when_under_l #[test] fn truncates_rollout_to_last_n_fork_turns_applies_thread_rollback_markers() { let rollout = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(assistant_msg("a1")), - RolloutItem::ResponseItem(inter_agent_msg( + response_item(user_msg("u1")), + response_item(assistant_msg("a1")), + response_item(inter_agent_msg( "triggered task", /*trigger_turn*/ true, )), - RolloutItem::ResponseItem(assistant_msg("a2")), + response_item(assistant_msg("a2")), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, })), - RolloutItem::ResponseItem(user_msg("u2")), - RolloutItem::ResponseItem(assistant_msg("a3")), + response_item(user_msg("u2")), + response_item(assistant_msg("a3")), ]; let truncated = truncate_rollout_to_last_n_fork_turns(rollout.clone(), /*n_from_end*/ 2); @@ -520,15 +516,15 @@ fn truncates_rollout_to_last_n_fork_turns_applies_thread_rollback_markers() { #[test] fn fork_turn_positions_ignore_zero_turn_rollback_markers() { let rollout = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(inter_agent_msg( + response_item(user_msg("u1")), + response_item(inter_agent_msg( "triggered task", /*trigger_turn*/ true, )), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 0, })), - RolloutItem::ResponseItem(user_msg("u2")), + response_item(user_msg("u2")), ]; assert_eq!(fork_turn_positions_in_rollout(&rollout), vec![0, 1, 3]); @@ -537,18 +533,18 @@ fn fork_turn_positions_ignore_zero_turn_rollback_markers() { #[test] fn truncates_rollout_to_last_n_fork_turns_discards_trigger_boundaries_in_rolled_back_suffix() { let rollout = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(user_msg("u2")), - RolloutItem::ResponseItem(inter_agent_msg( + response_item(user_msg("u1")), + response_item(user_msg("u2")), + response_item(inter_agent_msg( "triggered task", /*trigger_turn*/ true, )), - RolloutItem::ResponseItem(assistant_msg("a1")), + response_item(assistant_msg("a1")), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, })), - RolloutItem::ResponseItem(user_msg("u3")), - RolloutItem::ResponseItem(assistant_msg("a2")), + response_item(user_msg("u3")), + response_item(assistant_msg("a2")), ]; let truncated = truncate_rollout_to_last_n_fork_turns(rollout.clone(), /*n_from_end*/ 2); @@ -564,21 +560,21 @@ fn truncates_rollout_to_last_n_fork_turns_discards_trigger_boundaries_in_rolled_ #[test] fn truncates_rollout_to_last_n_fork_turns_discards_rolled_back_assistant_instruction_turns() { let rollout = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(assistant_msg("a1")), - RolloutItem::ResponseItem(inter_agent_msg( + response_item(user_msg("u1")), + response_item(assistant_msg("a1")), + response_item(inter_agent_msg( "triggered task 1", /*trigger_turn*/ true, )), - RolloutItem::ResponseItem(assistant_msg("a2")), + response_item(assistant_msg("a2")), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, })), - RolloutItem::ResponseItem(inter_agent_msg( + response_item(inter_agent_msg( "triggered task 2", /*trigger_turn*/ true, )), - RolloutItem::ResponseItem(assistant_msg("a3")), + response_item(assistant_msg("a3")), ]; let truncated = truncate_rollout_to_last_n_fork_turns(rollout.clone(), /*n_from_end*/ 1); @@ -593,13 +589,13 @@ fn truncates_rollout_to_last_n_fork_turns_discards_rolled_back_assistant_instruc #[test] fn truncates_rollout_to_last_n_fork_turns_keeps_full_rollout_when_n_is_large() { let rollout = vec![ - RolloutItem::ResponseItem(user_msg("u1")), - RolloutItem::ResponseItem(assistant_msg("a1")), - RolloutItem::ResponseItem(inter_agent_msg( + response_item(user_msg("u1")), + response_item(assistant_msg("a1")), + response_item(inter_agent_msg( "triggered task", /*trigger_turn*/ true, )), - RolloutItem::ResponseItem(assistant_msg("a2")), + response_item(assistant_msg("a2")), ]; let truncated = truncate_rollout_to_last_n_fork_turns(rollout.clone(), /*n_from_end*/ 10); diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index f8ba561dfc..e37a63588f 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -2741,15 +2741,18 @@ async fn resume_agent_restores_closed_agent_and_accepts_send_input() { let thread = manager .resume_thread_with_history( config.clone(), - InitialHistory::Forked(vec![RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "materialized".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - })]), + InitialHistory::Forked(vec![RolloutItem::ResponseItem( + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "materialized".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + )]), AuthManager::from_auth_for_testing(CodexAuth::from_api_key("dummy")), /*parent_trace*/ None, ClientMcpExtensions::default(), diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 9cea9b884c..9cc56b8473 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -100,6 +100,10 @@ const INSTALLATION_ID_FILENAME: &str = "installation_id"; const TEST_WINDOW_ID: &str = "test-thread:0"; const TEST_INSTALLATION_ID: &str = "11111111-1111-4111-8111-111111111111"; +fn rollout_response_item(item: ResponseItem) -> RolloutItem { + RolloutItem::ResponseItem(item.into()) +} + fn test_turn_responses_metadata( _client: &ModelClient, thread_id: ThreadId, @@ -516,7 +520,7 @@ async fn synthetic_call_output_id_is_stable_across_resumes() -> anyhow::Result<( RolloutLine { timestamp: "2024-01-01T00:00:01.000Z".to_string(), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::FunctionCall { + item: rollout_response_item(ResponseItem::FunctionCall { id: Some(ResponseItemId::with_suffix("fc", "existing")), name: "do_it".to_string(), namespace: None, @@ -1038,12 +1042,12 @@ async fn resume_replays_legacy_js_repl_image_rollout_shapes() { RolloutLine { timestamp: "2024-01-01T00:00:01.000Z".to_string(), ordinal: None, - item: RolloutItem::ResponseItem(legacy_custom_tool_call), + item: rollout_response_item(legacy_custom_tool_call), }, RolloutLine { timestamp: "2024-01-01T00:00:02.000Z".to_string(), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::CustomToolCallOutput { + item: rollout_response_item(ResponseItem::CustomToolCallOutput { id: None, call_id: "legacy-js-call".to_string(), name: None, @@ -1054,7 +1058,7 @@ async fn resume_replays_legacy_js_repl_image_rollout_shapes() { RolloutLine { timestamp: "2024-01-01T00:00:03.000Z".to_string(), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::Message { + item: rollout_response_item(ResponseItem::Message { id: None, role: "user".to_string(), content: vec![ContentItem::InputImage { @@ -1178,7 +1182,7 @@ async fn resume_replays_image_tool_outputs_with_detail() { RolloutLine { timestamp: "2024-01-01T00:00:01.000Z".to_string(), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::FunctionCall { + item: rollout_response_item(ResponseItem::FunctionCall { id: None, name: "view_image".to_string(), namespace: None, @@ -1191,7 +1195,7 @@ async fn resume_replays_image_tool_outputs_with_detail() { RolloutLine { timestamp: "2024-01-01T00:00:01.500Z".to_string(), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::FunctionCallOutput { + item: rollout_response_item(ResponseItem::FunctionCallOutput { id: None, call_id: function_call_id.to_string(), output: FunctionCallOutputPayload::from_content_items(vec![ @@ -1206,7 +1210,7 @@ async fn resume_replays_image_tool_outputs_with_detail() { RolloutLine { timestamp: "2024-01-01T00:00:02.000Z".to_string(), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::CustomToolCall { + item: rollout_response_item(ResponseItem::CustomToolCall { id: None, status: Some("completed".to_string()), call_id: custom_call_id.to_string(), @@ -1219,7 +1223,7 @@ async fn resume_replays_image_tool_outputs_with_detail() { RolloutLine { timestamp: "2024-01-01T00:00:02.500Z".to_string(), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::CustomToolCallOutput { + item: rollout_response_item(ResponseItem::CustomToolCallOutput { id: None, call_id: custom_call_id.to_string(), name: None, diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 12087f84f7..99de88ae02 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -357,7 +357,7 @@ fn replacement_history_from_rollout(path: &Path) -> Result> { replacement_history = Some( items .into_iter() - .map(serde_json::to_value) + .map(|envelope| serde_json::to_value(envelope.item)) .collect::, _>>()?, ); } diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 4384e8da4b..df641a6be8 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -1,6 +1,8 @@ use core_test_support::test_codex::local_selections; use std::fs; +use std::path::Path; +use anyhow::Context; use anyhow::Result; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; @@ -8,6 +10,7 @@ use codex_core::StartThreadOptions; use codex_core::X_CODEX_ROUTING_HINT_HEADER; use codex_core::compact::SUMMARY_PREFIX; use codex_features::Feature; +use codex_history::CodexHarnessMetadata; use codex_history::InitialHistory; use codex_history::RolloutItem; use codex_history::RolloutLine; @@ -369,6 +372,76 @@ fn amazon_bedrock_test_codex() -> TestCodexBuilder { }) } +fn is_retained_user_message(item: &ResponseItem, retained_text: &str) -> bool { + matches!( + item, + ResponseItem::Message { role, content, .. } + if role == "user" + && content.iter().any(|item| { + matches!(item, ContentItem::InputText { text } if text == retained_text) + }) + ) +} + +fn annotate_retained_user_in_rollout(path: &Path, retained_text: &str) -> Result<()> { + let mut rollout = fs::read_to_string(path)? + .lines() + .filter(|line| !line.trim().is_empty()) + .map(serde_json::from_str::) + .collect::, _>>()?; + rollout + .iter_mut() + .find_map(|line| match &mut line.item { + RolloutItem::ResponseItem(envelope) + if is_retained_user_message(&envelope.item, retained_text) => + { + Some(envelope) + } + _ => None, + }) + .context("persisted user response missing from rollout")? + .metadata = Some(CodexHarnessMetadata::default()); + + let contents = rollout + .iter() + .map(serde_json::to_string) + .collect::, _>>()? + .join("\n"); + fs::write(path, format!("{contents}\n"))?; + Ok(()) +} + +fn assert_compacted_user_metadata(path: &Path, retained_text: &str) -> Result<()> { + let replacement_history = fs::read_to_string(path)? + .lines() + .filter(|line| !line.trim().is_empty()) + .map(serde_json::from_str::) + .collect::, _>>()? + .into_iter() + .rev() + .find_map(|line| match line.item { + RolloutItem::Compacted(compacted) => compacted.replacement_history, + _ => None, + }) + .context("compacted replacement history missing")?; + assert!( + replacement_history.iter().any(|envelope| { + is_retained_user_message(&envelope.item, retained_text) && envelope.metadata.is_some() + }), + "compacted user message should retain its aligned harness metadata" + ); + Ok(()) +} + +fn assert_compact_request_omits_harness_metadata(request: &responses::ResponsesRequest) { + for item in request.input() { + assert!( + item.get("metadata").is_none() && item.get("replacement_history_metadata").is_none(), + "provider request must not receive harness history metadata: {item}" + ); + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn amazon_bedrock_uses_remote_compaction_endpoint() -> Result<()> { skip_if_no_network!(Ok(())); @@ -432,6 +505,70 @@ async fn amazon_bedrock_uses_remote_compaction_endpoint() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_compact_v2_retains_metadata_from_resumed_history() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = wiremock::MockServer::start().await; + let retained_text = "annotated v2 user message"; + let response_mock = responses::mount_sse_sequence( + &server, + vec![ + sse(vec![ + responses::ev_assistant_message("message-1", "before compaction"), + responses::ev_completed("response-1"), + ]), + sse(vec![ + json!({ + "type": "response.output_item.done", + "item": { + "type": "compaction", + "encrypted_content": "ANNOTATED_V2_COMPACTION_SUMMARY", + }, + }), + responses::ev_completed("response-compact"), + ]), + ], + ) + .await; + + let builder = || { + test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(|config| { + let _ = config.features.enable(Feature::RemoteCompactionV2); + }) + }; + let initial = builder().build(&server).await?; + initial.submit_turn(retained_text).await?; + let home = initial.home.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .context("rollout path")?; + initial.codex.shutdown_and_wait().await?; + annotate_retained_user_in_rollout(&rollout_path, retained_text)?; + + let resumed = builder() + .resume(&server, home, rollout_path.clone()) + .await?; + resumed.codex.submit(Op::Compact).await?; + wait_for_turn_complete(&resumed.codex).await; + resumed.codex.shutdown_and_wait().await?; + + let requests = response_mock.requests(); + let compact_request = requests.last().context("compact request")?; + assert_eq!( + compact_request.inputs_of_type("compaction_trigger").len(), + 1 + ); + assert_compact_request_omits_harness_metadata(compact_request); + assert_compacted_user_metadata(&rollout_path, retained_text)?; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn amazon_bedrock_automatic_compaction_uses_v1_endpoint_when_v2_is_enabled() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1141,7 +1278,10 @@ async fn remote_compact_v2_reuses_compaction_trigger_for_followups() -> Result<( developer_message(tool_notice), ] .into_iter() - .map(|item| serde_json::from_value(item).map(RolloutItem::ResponseItem)) + .map(|item| { + serde_json::from_value::(item) + .map(|item| RolloutItem::ResponseItem(item.into())) + }) .collect::>>()?; let codex = harness .test() @@ -2958,7 +3098,7 @@ async fn remote_compact_persists_replacement_history_in_rollout() -> Result<()> { let has_compaction_item = replacement_history.iter().any(|item| { matches!( - item, + &item.item, ResponseItem::Compaction { encrypted_content, .. } @@ -2967,7 +3107,7 @@ async fn remote_compact_persists_replacement_history_in_rollout() -> Result<()> }); let has_compacted_assistant_note = replacement_history.iter().any(|item| { matches!( - item, + &item.item, ResponseItem::Message { role, content, .. } if role == "assistant" && content.iter().any(|part| matches!( @@ -2978,7 +3118,7 @@ async fn remote_compact_persists_replacement_history_in_rollout() -> Result<()> }); let has_permissions_developer_message = replacement_history.iter().any(|item| { matches!( - item, + &item.item, ResponseItem::Message { role, content, .. } if role == "developer" && content.iter().any(|part| matches!( diff --git a/codex-rs/core/tests/suite/compact_remote_parity.rs b/codex-rs/core/tests/suite/compact_remote_parity.rs index ffc9762e57..e243ba27a5 100644 --- a/codex-rs/core/tests/suite/compact_remote_parity.rs +++ b/codex-rs/core/tests/suite/compact_remote_parity.rs @@ -794,7 +794,7 @@ fn replacement_history_from_rollout(path: &Path) -> Result { { let values = items .into_iter() - .map(|item| serde_json::to_value(item).expect("serialize replacement item")) + .map(|item| serde_json::to_value(item.item).expect("serialize replacement item")) .collect::>(); replacement_history = Some(Value::Array(values)); } diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 6de4596f09..0e01ce6726 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -8,16 +8,22 @@ use super::compact::COMPACT_WARNING_MESSAGE; use super::compact::FIRST_REPLY; use super::compact::SUMMARY_TEXT; +use anyhow::Context; use anyhow::Result; 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_history::CodexHarnessMetadata; +use codex_history::RolloutItem; +use codex_history::RolloutLine; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; use codex_protocol::mcp::ClientMcpExtensions; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseItem; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::WarningEvent; @@ -39,12 +45,14 @@ use core_test_support::wait_for_event; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; +use std::path::Path; use std::sync::Arc; use tempfile::TempDir; use wiremock::MockServer; const AFTER_SECOND_RESUME: &str = "AFTER_SECOND_RESUME"; const AFTER_ROLLBACK: &str = "AFTER_ROLLBACK"; +const CHECKPOINT_METADATA_KEY: &str = "replacement_history_metadata"; fn network_disabled() -> bool { std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() @@ -69,6 +77,73 @@ fn normalize_line_endings_str(text: &str) -> String { } } +fn response_message_contains_text(item: &ResponseItem, expected: &str) -> bool { + matches!( + item, + ResponseItem::Message { role, content, .. } + if role == "user" + && content.iter().any(|item| { + matches!(item, ContentItem::InputText { text } if text == expected) + }) + ) +} + +fn seed_first_checkpoint_harness_metadata(path: &Path, retained_text: &str) -> Result<()> { + let mut lines = std::fs::read_to_string(path)? + .lines() + .filter(|line| !line.trim().is_empty()) + .map(serde_json::from_str::) + .collect::, _>>()?; + let replacement_history = lines + .iter_mut() + .find_map(|line| match &mut line.item { + RolloutItem::Compacted(compacted) => compacted.replacement_history.as_mut(), + _ => None, + }) + .context("first compacted checkpoint missing replacement history")?; + replacement_history + .iter() + .find(|envelope| response_message_contains_text(&envelope.item, retained_text)) + .context("retained user message missing from first compacted checkpoint")?; + for envelope in replacement_history { + envelope.metadata = Some(CodexHarnessMetadata::default()); + } + + let rewritten = lines + .iter() + .map(serde_json::to_string) + .collect::, _>>()? + .join("\n"); + std::fs::write(path, format!("{rewritten}\n"))?; + Ok(()) +} + +fn assert_latest_checkpoint_retains_harness_metadata( + path: &Path, + retained_text: &str, +) -> Result<()> { + let replacement_history = std::fs::read_to_string(path)? + .lines() + .filter(|line| !line.trim().is_empty()) + .map(serde_json::from_str::) + .collect::, _>>()? + .into_iter() + .rev() + .find_map(|line| match line.item { + RolloutItem::Compacted(compacted) => compacted.replacement_history, + _ => None, + }) + .context("latest compacted checkpoint missing replacement history")?; + assert!( + replacement_history.iter().any(|envelope| { + response_message_contains_text(&envelope.item, retained_text) + && envelope.metadata.is_some() + }), + "latest compacted checkpoint should retain aligned harness metadata on {retained_text:?}" + ); + Ok(()) +} + fn extract_summary_user_text(request: &Value, summary_text: &str) -> String { json_message_input_texts(request, "user") .into_iter() @@ -307,6 +382,7 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() ); shutdown_conversation(&base).await; + seed_first_checkpoint_harness_metadata(&base_path, "hello world")?; let resumed = resume_conversation(&manager, &config, base_path).await; user_turn(&resumed, "AFTER_RESUME").await; let resumed_path = fetch_conversation_path(&resumed); @@ -327,6 +403,7 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() ); shutdown_conversation(&forked).await; + assert_latest_checkpoint_retains_harness_metadata(&forked_path, "hello world")?; let resumed_again = resume_conversation(&manager, &config, forked_path).await; user_turn(&resumed_again, AFTER_SECOND_RESUME).await; @@ -337,6 +414,15 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() .collect::>(); requests.iter_mut().for_each(normalize_line_endings); normalize_compact_prompts(&mut requests); + assert!( + requests + .iter() + .flat_map(|request| request["input"].as_array().expect("provider request input")) + .all(|item| { + item.get(CHECKPOINT_METADATA_KEY).is_none() && item.get("metadata").is_none() + }), + "provider requests must not contain harness metadata" + ); let input_after_compact = json!(requests[requests.len() - 2]["input"]); let input_after_resume = json!(requests[requests.len() - 1]["input"]); diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index f12e9f38ed..004aec9a22 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1104,7 +1104,8 @@ fn rollout_hook_prompt_texts(text: &str) -> Result> { continue; } let rollout: RolloutLine = serde_json::from_str(trimmed).context("parse rollout line")?; - if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rollout.item + if let RolloutItem::ResponseItem(envelope) = rollout.item + && let ResponseItem::Message { role, content, .. } = envelope.item && role == "user" { for item in content { diff --git a/codex-rs/core/tests/suite/image_rollout.rs b/codex-rs/core/tests/suite/image_rollout.rs index 6cdb1fac8e..23bec89cbe 100644 --- a/codex-rs/core/tests/suite/image_rollout.rs +++ b/codex-rs/core/tests/suite/image_rollout.rs @@ -48,15 +48,14 @@ fn find_user_message_with_image(text: &str) -> Option { Ok(rollout) => rollout, Err(_) => continue, }; - if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = - &rollout.item + if let RolloutItem::ResponseItem(envelope) = &rollout.item + && let ResponseItem::Message { role, content, .. } = &envelope.item && role == "user" && content .iter() .any(|span| matches!(span, ContentItem::InputImage { .. })) - && let RolloutItem::ResponseItem(item) = rollout.item.clone() { - return Some(item); + return Some(envelope.item.clone()); } } None @@ -326,16 +325,18 @@ async fn resumed_history_only_emits_resize_notices_for_new_images() -> anyhow::R .collect::>>()?; let historical_content = rollout_lines .iter_mut() - .find_map(|line| match &mut line.item { - RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) - if role == "user" - && content.iter().any(|item| { - matches!(item, ContentItem::InputText { text } if text == "historical image") - }) => - { - Some(content) - } - _ => None, + .find_map(|line| { + let RolloutItem::ResponseItem(envelope) = &mut line.item else { + return None; + }; + let ResponseItem::Message { role, content, .. } = &mut envelope.item else { + return None; + }; + (role == "user" + && content.iter().any(|item| { + matches!(item, ContentItem::InputText { text } if text == "historical image") + })) + .then_some(content) }) .context("historical user message in rollout")?; historical_content.insert( diff --git a/codex-rs/core/tests/suite/pending_input.rs b/codex-rs/core/tests/suite/pending_input.rs index fff66362b9..e0dd9ffc57 100644 --- a/codex-rs/core/tests/suite/pending_input.rs +++ b/codex-rs/core/tests/suite/pending_input.rs @@ -481,14 +481,16 @@ async fn queue_only_agent_mail_wakes_sleeping_root_and_persists_message() { assert!(history.items.iter().any(|item| { matches!( item, - RolloutItem::ResponseItem(codex_protocol::models::ResponseItem::AgentMessage { - content, - .. - }) if content.iter().any(|content| matches!( - content, - codex_protocol::models::AgentMessageInputContent::InputText { text } - if text == CHILD_MESSAGE - )) + RolloutItem::ResponseItem(envelope) + if matches!( + &envelope.item, + codex_protocol::models::ResponseItem::AgentMessage { content, .. } + if content.iter().any(|content| matches!( + content, + codex_protocol::models::AgentMessageInputContent::InputText { text } + if text == CHILD_MESSAGE + )) + ) ) })); } diff --git a/codex-rs/core/tests/suite/realtime_conversation.rs b/codex-rs/core/tests/suite/realtime_conversation.rs index df654432c0..422137935c 100644 --- a/codex-rs/core/tests/suite/realtime_conversation.rs +++ b/codex-rs/core/tests/suite/realtime_conversation.rs @@ -2700,14 +2700,14 @@ async fn conversation_startup_context_current_thread_selects_many_turns_by_budge let turn_number = index + 1; let assistant_turn = format!("assistant turn {turn_number}"); [ - RolloutItem::ResponseItem(ResponseItem::Message { + ResponseItem::Message { id: None, role: "user".to_string(), content: vec![ContentItem::InputText { text: user_turn }], phase: None, internal_chat_message_metadata_passthrough: None, - }), - RolloutItem::ResponseItem(ResponseItem::Message { + }, + ResponseItem::Message { id: None, role: "assistant".to_string(), content: vec![ContentItem::OutputText { @@ -2715,8 +2715,9 @@ async fn conversation_startup_context_current_thread_selects_many_turns_by_budge }], phase: None, internal_chat_message_metadata_passthrough: None, - }), + }, ] + .map(|item| RolloutItem::ResponseItem(item.into())) }) .collect::>(); test.codex.shutdown_and_wait().await?; diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index d1f3153c0a..a75a2d14f0 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -242,7 +242,9 @@ async fn review_op_emits_lifecycle_and_review_output() { } let v: serde_json::Value = serde_json::from_str(line).expect("jsonl line"); let rl: RolloutLine = serde_json::from_value(v).expect("rollout line"); - if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rl.item { + if let RolloutItem::ResponseItem(envelope) = rl.item + && let ResponseItem::Message { role, content, .. } = envelope.item + { if role == "user" { for c in content { if let ContentItem::InputText { text } = c { @@ -1028,7 +1030,8 @@ async fn review_input_isolated_from_parent_history() { } let v: serde_json::Value = serde_json::from_str(line).expect("jsonl line"); let rl: RolloutLine = serde_json::from_value(v).expect("rollout line"); - if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rl.item + if let RolloutItem::ResponseItem(envelope) = rl.item + && let ResponseItem::Message { role, content, .. } = envelope.item && role == "user" { for c in content { diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 20574978b8..b1f1f18808 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -1179,11 +1179,15 @@ async fn interrupt_during_mcp_startup_preserves_user_input_in_history( .position(|item| { matches!( item, - RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) - if role == "user" - && content.iter().any(|item| { - matches!(item, ContentItem::InputText { text } if text == prompt) - }) + RolloutItem::ResponseItem(envelope) + if matches!( + &envelope.item, + ResponseItem::Message { role, content, .. } + if role == "user" + && content.iter().any(|item| { + matches!(item, ContentItem::InputText { text } if text == prompt) + }) + ) ) }) .expect("an interrupted turn should retain its submitted user prompt"); @@ -1193,10 +1197,14 @@ async fn interrupt_during_mcp_startup_preserves_user_input_in_history( .position(|item| { matches!( item, - RolloutItem::ResponseItem(ResponseItem::Message { content, .. }) - if content.iter().any(|item| { - matches!(item, ContentItem::InputText { text } if text.contains("")) - }) + RolloutItem::ResponseItem(envelope) + if matches!( + &envelope.item, + ResponseItem::Message { content, .. } + if content.iter().any(|item| { + matches!(item, ContentItem::InputText { text } if text.contains("")) + }) + ) ) }) .expect("an interrupted turn should retain its interruption marker"); diff --git a/codex-rs/external-agent-migration/src/sessions/append.rs b/codex-rs/external-agent-migration/src/sessions/append.rs index 7cc4dabeb5..a9efc6f5f4 100644 --- a/codex-rs/external-agent-migration/src/sessions/append.rs +++ b/codex-rs/external-agent-migration/src/sessions/append.rs @@ -220,7 +220,7 @@ fn source_model_items(items: &[RolloutItem]) -> Option>> RolloutItem::SessionMeta(_) | RolloutItem::InterAgentCommunicationMetadata { .. } => {} RolloutItem::ResponseItem(response_item) => { model_items.push(SourceModelItem { - response_item, + response_item: &response_item.item, append_start_index: append_start_index.take().unwrap_or(index), }); } @@ -253,7 +253,7 @@ fn history_model_items(items: &[RolloutItem]) -> Option> { for item in items { match item { RolloutItem::SessionMeta(_) | RolloutItem::InterAgentCommunicationMetadata { .. } => {} - RolloutItem::ResponseItem(response_item) => model_items.push(response_item), + RolloutItem::ResponseItem(response_item) => model_items.push(&response_item.item), RolloutItem::EventMsg( EventMsg::ContextCompacted(_) | EventMsg::ThreadRolledBack(_), ) diff --git a/codex-rs/external-agent-migration/src/sessions/append_tests.rs b/codex-rs/external-agent-migration/src/sessions/append_tests.rs index 40f765f217..f86a9af882 100644 --- a/codex-rs/external-agent-migration/src/sessions/append_tests.rs +++ b/codex-rs/external-agent-migration/src/sessions/append_tests.rs @@ -75,15 +75,18 @@ fn requires_a_strict_nonempty_model_prefix() { assert!(plan_append(&source, &rewritten).is_none()); } let mut with_tool_call = history; - with_tool_call.push(RolloutItem::ResponseItem(ResponseItem::FunctionCall { - id: None, - name: "native_tool".to_string(), - namespace: None, - arguments: "{}".to_string(), - encrypted_function_args: None, - call_id: "native-call".to_string(), - internal_chat_message_metadata_passthrough: None, - })); + with_tool_call.push(RolloutItem::ResponseItem( + ResponseItem::FunctionCall { + id: None, + name: "native_tool".to_string(), + namespace: None, + arguments: "{}".to_string(), + encrypted_function_args: None, + call_id: "native-call".to_string(), + internal_chat_message_metadata_passthrough: None, + } + .into(), + )); assert!(plan_append(&source, &with_tool_call).is_none()); } @@ -105,7 +108,10 @@ fn model_messages(items: &[RolloutItem]) -> Vec<(MessageRole, &str)> { items .iter() .filter_map(|item| match item { - RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) => { + RolloutItem::ResponseItem(response_item) => { + let ResponseItem::Message { role, content, .. } = &response_item.item else { + return None; + }; match (role.as_str(), content.as_slice()) { ("user", [ContentItem::InputText { text }]) => { Some((MessageRole::User, text.as_str())) diff --git a/codex-rs/external-agent-migration/src/sessions/export.rs b/codex-rs/external-agent-migration/src/sessions/export.rs index 9f3d9dcac5..75a2d30cba 100644 --- a/codex-rs/external-agent-migration/src/sessions/export.rs +++ b/codex-rs/external-agent-migration/src/sessions/export.rs @@ -120,7 +120,7 @@ pub(super) fn rollout_items_from_messages(messages: Vec) -> ))); response_item_bytes = response_item_bytes.saturating_add(message_byte_count(&message)); - items.push(RolloutItem::ResponseItem(response_item(message))); + items.push(RolloutItem::ResponseItem(response_item(message).into())); current_turn = Some((turn_id, started_at)); } MessageRole::Assistant => { @@ -137,7 +137,7 @@ pub(super) fn rollout_items_from_messages(messages: Vec) -> memory_citation: None, }, ))); - items.push(RolloutItem::ResponseItem(response_item(message))); + items.push(RolloutItem::ResponseItem(response_item(message).into())); } } } @@ -325,7 +325,8 @@ mod tests { .filter(|item| { matches!( item, - RolloutItem::ResponseItem(ResponseItem::Message { .. }) + RolloutItem::ResponseItem(response_item) + if matches!(&response_item.item, ResponseItem::Message { .. }) ) }) .count(); diff --git a/codex-rs/history/src/lib.rs b/codex-rs/history/src/lib.rs index 7b538ae013..493841fd98 100644 --- a/codex-rs/history/src/lib.rs +++ b/codex-rs/history/src/lib.rs @@ -26,23 +26,46 @@ use schemars::JsonSchema; use serde::Deserialize; use serde::Deserializer; use serde::Serialize; +use serde::Serializer; +use serde::de::Error as _; -/// A history-owned, in-memory wrapper around a raw response item. +/// A model-history item with room for history-only metadata. +/// +/// Persistence keeps the response item intact and stores its metadata separately. #[derive(Debug, Clone, PartialEq)] pub struct ResponseItemEnvelope { pub item: ResponseItem, + pub metadata: Option, } +/// Metadata owned by the Codex harness and persisted with a response item. +/// +/// This intentionally has no fields yet. Keeping it closed prevents rollout +/// metadata from becoming an untyped extension point. +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] +pub struct CodexHarnessMetadata {} + impl ResponseItemEnvelope { + /// Wraps a raw Responses API item for persisted history. pub fn new(item: ResponseItem) -> Self { - Self { item } + Self { + item, + metadata: None, + } } + /// Unwraps the raw Responses API item. pub fn into_item(self) -> ResponseItem { self.item } } +impl From for ResponseItemEnvelope { + fn from(item: ResponseItem) -> Self { + Self::new(item) + } +} + impl Deref for ResponseItemEnvelope { type Target = ResponseItem; @@ -64,11 +87,10 @@ impl Borrow for ResponseItemEnvelope { } /// Persisted rollout item used by core history and rollout storage. -#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] -#[serde(tag = "type", content = "payload", rename_all = "snake_case")] +#[derive(Debug, Clone)] pub enum RolloutItem { SessionMeta(SessionMetaLine), - ResponseItem(ResponseItem), + ResponseItem(ResponseItemEnvelope), InterAgentCommunication(InterAgentCommunication), InterAgentCommunicationMetadata { trigger_turn: bool }, Compacted(CompactedItem), @@ -77,21 +99,84 @@ pub enum RolloutItem { EventMsg(EventMsg), } -#[derive(Serialize, Clone, Debug, PartialEq, JsonSchema)] +impl Serialize for RolloutItem { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + rollout_payload::RolloutItemWire::from(self).serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for RolloutItem { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + rollout_payload::RolloutItemWire::deserialize(deserializer).map(Into::into) + } +} + +impl JsonSchema for RolloutItem { + fn schema_name() -> String { + "RolloutItem".to_string() + } + + fn schema_id() -> std::borrow::Cow<'static, str> { + std::borrow::Cow::Borrowed(concat!(module_path!(), "::RolloutItem")) + } + + fn json_schema(generator: &mut schemars::SchemaGenerator) -> schemars::schema::Schema { + rollout_payload::RolloutItemWire::json_schema(generator) + } +} + +mod rollout_payload; + +#[derive(Clone, Debug, PartialEq)] pub struct CompactedItem { pub message: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub replacement_history: Option>, - #[serde(default, skip_serializing_if = "Option::is_none")] + pub replacement_history: Option>, pub window_number: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] pub first_window_id: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] pub previous_window_id: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] pub window_id: Option, } +impl Serialize for CompactedItem { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + rollout_payload::CompactedItemWire::from(self).serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for CompactedItem { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + rollout_payload::CompactedItemWire::deserialize(deserializer)? + .try_into() + .map_err(D::Error::custom) + } +} + +impl JsonSchema for CompactedItem { + fn schema_name() -> String { + "CompactedItem".to_string() + } + + fn schema_id() -> std::borrow::Cow<'static, str> { + std::borrow::Cow::Borrowed(concat!(module_path!(), "::CompactedItem")) + } + + fn json_schema(generator: &mut schemars::SchemaGenerator) -> schemars::schema::Schema { + rollout_payload::CompactedItemWire::json_schema(generator) + } +} + impl From for ResponseItem { fn from(value: CompactedItem) -> Self { ResponseItem::Message { @@ -106,56 +191,6 @@ impl From for ResponseItem { } } -// Before window_number was introduced, the numeric window number was serialized as -// window_id. Accept that shape so existing rollouts remain resumable. -impl<'de> Deserialize<'de> for CompactedItem { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let serialized = SerializedCompactedItem::deserialize(deserializer)?; - let mut window_number = serialized.window_number; - let window_id = match serialized.window_id { - Some(SerializedWindowId::Id(window_id)) => Some(window_id), - Some(SerializedWindowId::LegacyWindowNumber(legacy_window_number)) => { - window_number.get_or_insert(legacy_window_number); - None - } - None => None, - }; - Ok(Self { - message: serialized.message, - replacement_history: serialized.replacement_history, - window_number, - first_window_id: serialized.first_window_id, - previous_window_id: serialized.previous_window_id, - window_id, - }) - } -} - -#[derive(Deserialize)] -struct SerializedCompactedItem { - message: String, - #[serde(default)] - replacement_history: Option>, - #[serde(default)] - window_number: Option, - #[serde(default)] - first_window_id: Option, - #[serde(default)] - previous_window_id: Option, - #[serde(default)] - window_id: Option, -} - -#[derive(Deserialize)] -#[serde(untagged)] -enum SerializedWindowId { - Id(String), - LegacyWindowNumber(u64), -} - #[derive(Serialize, Deserialize, Clone, JsonSchema)] pub struct RolloutLine { pub timestamp: String, diff --git a/codex-rs/history/src/rollout_payload.rs b/codex-rs/history/src/rollout_payload.rs new file mode 100644 index 0000000000..018b370a5e --- /dev/null +++ b/codex-rs/history/src/rollout_payload.rs @@ -0,0 +1,236 @@ +use std::borrow::Cow; + +use super::CodexHarnessMetadata; +use super::CompactedItem; +use super::EventMsg; +use super::InterAgentCommunication; +use super::ResponseItem; +use super::ResponseItemEnvelope; +use super::RolloutItem; +use super::SessionMetaLine; +use super::TurnContextItem; +use super::WorldStateItem; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; + +/// Persisted rollout item used by core history and rollout storage. +#[derive(Serialize, Deserialize, JsonSchema)] +#[serde(tag = "type", rename_all = "snake_case")] +pub(super) enum RolloutItemWire<'a> { + SessionMeta { + payload: Cow<'a, SessionMetaLine>, + }, + ResponseItem { + payload: Cow<'a, ResponseItem>, + #[serde(default, skip_serializing_if = "Option::is_none")] + metadata: Option>, + }, + InterAgentCommunication { + payload: Cow<'a, InterAgentCommunication>, + }, + InterAgentCommunicationMetadata { + payload: InterAgentCommunicationMetadataPayload, + }, + Compacted { + payload: Cow<'a, CompactedItem>, + }, + TurnContext { + payload: Cow<'a, TurnContextItem>, + }, + WorldState { + payload: Cow<'a, WorldStateItem>, + }, + EventMsg { + payload: Cow<'a, EventMsg>, + }, +} + +impl<'a> From<&'a RolloutItem> for RolloutItemWire<'a> { + fn from(item: &'a RolloutItem) -> Self { + match item { + RolloutItem::SessionMeta(payload) => Self::SessionMeta { + payload: Cow::Borrowed(payload), + }, + RolloutItem::ResponseItem(envelope) => Self::ResponseItem { + payload: Cow::Borrowed(&envelope.item), + metadata: envelope.metadata.as_ref().map(Cow::Borrowed), + }, + RolloutItem::InterAgentCommunication(payload) => Self::InterAgentCommunication { + payload: Cow::Borrowed(payload), + }, + RolloutItem::InterAgentCommunicationMetadata { trigger_turn } => { + Self::InterAgentCommunicationMetadata { + payload: InterAgentCommunicationMetadataPayload { + trigger_turn: *trigger_turn, + }, + } + } + RolloutItem::Compacted(payload) => Self::Compacted { + payload: Cow::Borrowed(payload), + }, + RolloutItem::TurnContext(payload) => Self::TurnContext { + payload: Cow::Borrowed(payload), + }, + RolloutItem::WorldState(payload) => Self::WorldState { + payload: Cow::Borrowed(payload), + }, + RolloutItem::EventMsg(payload) => Self::EventMsg { + payload: Cow::Borrowed(payload), + }, + } + } +} + +impl From> for RolloutItem { + fn from(item: RolloutItemWire<'_>) -> Self { + match item { + RolloutItemWire::SessionMeta { payload } => Self::SessionMeta(payload.into_owned()), + RolloutItemWire::ResponseItem { payload, metadata } => { + Self::ResponseItem(ResponseItemEnvelope { + item: payload.into_owned(), + metadata: metadata.map(Cow::into_owned), + }) + } + RolloutItemWire::InterAgentCommunication { payload } => { + Self::InterAgentCommunication(payload.into_owned()) + } + RolloutItemWire::InterAgentCommunicationMetadata { payload } => { + Self::InterAgentCommunicationMetadata { + trigger_turn: payload.trigger_turn, + } + } + RolloutItemWire::Compacted { payload } => Self::Compacted(payload.into_owned()), + RolloutItemWire::TurnContext { payload } => Self::TurnContext(payload.into_owned()), + RolloutItemWire::WorldState { payload } => Self::WorldState(payload.into_owned()), + RolloutItemWire::EventMsg { payload } => Self::EventMsg(payload.into_owned()), + } + } +} + +#[derive(Serialize, Deserialize, JsonSchema)] +pub(super) struct InterAgentCommunicationMetadataPayload { + trigger_turn: bool, +} + +#[derive(Serialize, Deserialize, JsonSchema)] +pub(super) struct CompactedItemWire<'a> { + message: Cow<'a, str>, + #[serde(default, skip_serializing_if = "Option::is_none")] + replacement_history: Option>>, + #[serde(default, skip_serializing_if = "Option::is_none")] + replacement_history_metadata: Option>>, + #[serde(default, skip_serializing_if = "Option::is_none")] + window_number: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + first_window_id: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + previous_window_id: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[schemars(with = "Option")] + window_id: Option>, +} + +impl<'a> From<&'a CompactedItem> for CompactedItemWire<'a> { + fn from(item: &'a CompactedItem) -> Self { + let replacement_history_metadata = item + .replacement_history + .as_ref() + .filter(|items| items.iter().any(|envelope| envelope.metadata.is_some())) + .map(|items| { + items + .iter() + .map(|envelope| match envelope.metadata.as_ref() { + Some(metadata) => Cow::Borrowed(metadata), + None => Cow::Owned(CodexHarnessMetadata::default()), + }) + .collect() + }); + + Self { + message: Cow::Borrowed(&item.message), + replacement_history: item.replacement_history.as_ref().map(|items| { + items + .iter() + .map(|envelope| Cow::Borrowed(&envelope.item)) + .collect() + }), + replacement_history_metadata, + window_number: item.window_number, + first_window_id: item.first_window_id.as_deref().map(Cow::Borrowed), + previous_window_id: item.previous_window_id.as_deref().map(Cow::Borrowed), + window_id: item + .window_id + .as_deref() + .map(|window_id| WindowIdWire::Id(Cow::Borrowed(window_id))), + } + } +} + +impl TryFrom> for CompactedItem { + type Error = String; + + fn try_from(item: CompactedItemWire<'_>) -> Result { + let replacement_history = match ( + item.replacement_history, + item.replacement_history_metadata, + ) { + (Some(items), Some(metadata)) => { + let history_len = items.len(); + let metadata_len = metadata.len(); + if history_len != metadata_len { + return Err(format!( + "replacement_history_metadata has {metadata_len} entries for {history_len} history items" + )); + } + Some( + items + .into_iter() + .zip(metadata) + .map(|(item, metadata)| ResponseItemEnvelope { + item: item.into_owned(), + metadata: Some(metadata.into_owned()), + }) + .collect(), + ) + } + (Some(items), None) => Some( + items + .into_iter() + .map(|item| ResponseItemEnvelope::new(item.into_owned())) + .collect(), + ), + (None, Some(_)) => { + return Err("replacement_history_metadata requires replacement_history".to_string()); + } + (None, None) => None, + }; + + let mut window_number = item.window_number; + let window_id = match item.window_id { + Some(WindowIdWire::Id(window_id)) => Some(window_id.into_owned()), + Some(WindowIdWire::LegacyWindowNumber(legacy_window_number)) => { + window_number.get_or_insert(legacy_window_number); + None + } + None => None, + }; + + Ok(Self { + message: item.message.into_owned(), + replacement_history, + window_number, + first_window_id: item.first_window_id.map(Cow::into_owned), + previous_window_id: item.previous_window_id.map(Cow::into_owned), + window_id, + }) + } +} + +// Older rollouts stored the numeric window number in `window_id`. +#[derive(Serialize, Deserialize)] +#[serde(untagged)] +enum WindowIdWire<'a> { + Id(Cow<'a, str>), + LegacyWindowNumber(u64), +} diff --git a/codex-rs/history/src/tests.rs b/codex-rs/history/src/tests.rs index ea34bdff34..5db3d27f4b 100644 --- a/codex-rs/history/src/tests.rs +++ b/codex-rs/history/src/tests.rs @@ -35,6 +35,7 @@ fn response_item_envelope_accessors_preserve_item() { } #[test] +/// Keeps legacy response-item rollout lines readable and byte-shape compatible. fn response_item_rollout_line_preserves_shape() -> Result<()> { let legacy_line = json!({ "timestamp": "2025-01-03T12:00:00.000Z", @@ -51,15 +52,88 @@ fn response_item_rollout_line_preserves_shape() -> Result<()> { }); let line = serde_json::from_value::(legacy_line.clone())?; - let RolloutItem::ResponseItem(item) = &line.item else { + let RolloutItem::ResponseItem(envelope) = &line.item else { panic!("expected response item"); }; - assert!(matches!(item, ResponseItem::Message { .. })); + assert!(matches!(&envelope.item, ResponseItem::Message { .. })); + assert_eq!(envelope.metadata, None); assert_eq!(serde_json::to_value(line)?, legacy_line); Ok(()) } #[test] +/// Keeps harness metadata beside, rather than inside, response-item payloads. +fn response_item_envelope_stores_metadata_beside_rollout_payload() -> Result<()> { + let response_item = response_message("developer"); + let line = RolloutLine { + timestamp: "2025-01-03T12:00:00.000Z".to_string(), + ordinal: Some(7), + item: RolloutItem::ResponseItem(ResponseItemEnvelope { + item: response_item.clone(), + metadata: Some(CodexHarnessMetadata {}), + }), + }; + let serialized = serde_json::to_value(&line)?; + + assert_eq!( + serialized, + json!({ + "timestamp": "2025-01-03T12:00:00.000Z", + "ordinal": 7, + "type": "response_item", + "payload": response_item, + "metadata": {}, + }) + ); + assert_eq!(serialized["payload"].get("metadata"), None); + + let restored = serde_json::from_value::(serialized)?; + let RolloutItem::ResponseItem(envelope) = restored.item else { + panic!("expected response item"); + }; + assert_eq!(envelope.metadata, Some(CodexHarnessMetadata {})); + Ok(()) +} + +#[test] +/// Keeps future metadata fields from making older binaries reject persisted items. +fn response_item_envelope_ignores_unknown_harness_metadata_fields() -> Result<()> { + let line = serde_json::from_value::(json!({ + "timestamp": "2025-01-03T12:00:00.000Z", + "ordinal": 7, + "type": "response_item", + "payload": { + "type": "message", + "role": "user", + "content": [{ + "type": "input_text", + "text": "hello", + }], + }, + "metadata": { + "future_field": "value", + }, + }))?; + + let RolloutItem::ResponseItem(envelope) = line.item else { + panic!("expected response item"); + }; + assert_eq!(envelope.metadata, Some(CodexHarnessMetadata {})); + + let compacted = serde_json::from_value::(json!({ + "message": "summary", + "replacement_history": [response_message("user")], + "replacement_history_metadata": [{ "future_field": "value" }], + }))?; + assert_eq!( + compacted.replacement_history.expect("replacement history")[0].metadata, + Some(CodexHarnessMetadata {}) + ); + Ok(()) +} + +#[test] +/// Keeps legacy compacted replacement histories readable and shape-compatible. fn response_item_replacement_history_preserves_shape() -> Result<()> { let legacy_item = json!({ "message": "summary", @@ -79,14 +153,254 @@ fn response_item_replacement_history_preserves_shape() -> Result<()> { .as_ref() .expect("replacement history"); assert!(matches!( - &replacement_history[0], + &replacement_history[0].item, ResponseItem::Message { .. } )); + assert_eq!(replacement_history[0].metadata, None); assert_eq!(serde_json::to_value(item)?, legacy_item); Ok(()) } #[test] +/// Stores complete aligned checkpoint metadata without modifying response items. +fn compacted_replacement_history_stores_metadata_in_an_aligned_sidecar() -> Result<()> { + let developer_message = response_message("developer"); + let compaction_item = ResponseItem::Compaction { + id: None, + encrypted_content: "opaque".to_string(), + internal_chat_message_metadata_passthrough: None, + }; + let item = CompactedItem { + message: "summary".to_string(), + replacement_history: Some(vec![ + ResponseItemEnvelope { + item: developer_message.clone(), + metadata: Some(CodexHarnessMetadata {}), + }, + ResponseItemEnvelope::new(compaction_item.clone()), + ]), + window_number: None, + first_window_id: None, + previous_window_id: None, + window_id: None, + }; + + let serialized = serde_json::to_value(item)?; + assert_eq!( + serialized, + json!({ + "message": "summary", + "replacement_history": [developer_message, compaction_item], + "replacement_history_metadata": [{}, {}], + }) + ); + + let restored = serde_json::from_value::(serialized)?; + assert_eq!( + restored.replacement_history, + Some(vec![ + ResponseItemEnvelope { + item: developer_message, + metadata: Some(CodexHarnessMetadata {}), + }, + ResponseItemEnvelope { + item: compaction_item, + metadata: Some(CodexHarnessMetadata {}), + }, + ]) + ); + Ok(()) +} + +#[test] +/// Rejects checkpoint sidecars that cannot be paired unambiguously with history. +fn compacted_replacement_history_rejects_misaligned_metadata() { + let malformed_items = [ + json!({ + "message": "summary", + "replacement_history": [response_message("user")], + "replacement_history_metadata": [], + }), + json!({ + "message": "summary", + "replacement_history": [response_message("user")], + "replacement_history_metadata": [{}, {}], + }), + json!({ + "message": "summary", + "replacement_history_metadata": [{}], + }), + ]; + + for malformed in malformed_items { + let error = serde_json::from_value::(malformed) + .expect_err("misaligned checkpoint metadata must be rejected"); + assert!( + error.to_string().contains("replacement_history_metadata"), + "error: {error}" + ); + } +} + +#[test] +/// Keeps annotated checkpoints readable by binaries expecting raw response items. +fn compacted_metadata_remains_compatible_with_legacy_response_item_readers() -> Result<()> { + #[derive(Deserialize)] + #[serde(tag = "type", content = "payload", rename_all = "snake_case")] + enum LegacyRolloutItem { + ResponseItem(Box), + Compacted(LegacyCompactedItem), + } + + #[derive(Deserialize)] + struct LegacyCompactedItem { + replacement_history: Vec, + } + + let response_item = response_message("developer"); + let envelope = ResponseItemEnvelope { + item: response_item.clone(), + metadata: Some(CodexHarnessMetadata {}), + }; + let response_line = serde_json::to_value(RolloutItem::ResponseItem(envelope.clone()))?; + let LegacyRolloutItem::ResponseItem(legacy_response) = + serde_json::from_value::(response_line)? + else { + panic!("expected legacy response item"); + }; + assert_eq!(*legacy_response, response_item); + + let compacted_line = serde_json::to_value(RolloutItem::Compacted(CompactedItem { + message: "summary".to_string(), + replacement_history: Some(vec![envelope]), + window_number: None, + first_window_id: None, + previous_window_id: None, + window_id: None, + }))?; + + let LegacyRolloutItem::Compacted(legacy) = + serde_json::from_value::(compacted_line)? + else { + panic!("expected legacy compacted item"); + }; + assert_eq!(legacy.replacement_history, vec![response_item]); + Ok(()) +} + +#[test] +/// Preserves the established tagged payload representation for every rollout variant. +fn rollout_item_variants_preserve_existing_payload_shapes() -> Result<()> { + let cwd = std::env::current_dir()?; + let fixtures = [ + json!({ + "type": "session_meta", + "payload": SessionMetaLine { + meta: SessionMeta::default(), + git: None, + }, + }), + json!({ + "type": "response_item", + "payload": response_message("user"), + }), + json!({ + "type": "inter_agent_communication", + "payload": { + "author": "/root", + "recipient": "/root/child", + "other_recipients": [], + "content": "hello", + "trigger_turn": false, + }, + }), + json!({ + "type": "inter_agent_communication_metadata", + "payload": { "trigger_turn": true }, + }), + json!({ + "type": "compacted", + "payload": { "message": "summary" }, + }), + json!({ + "type": "turn_context", + "payload": { + "cwd": cwd, + "approval_policy": "never", + "sandbox_policy": { "type": "danger-full-access" }, + "model": "gpt-5", + "summary": "auto", + }, + }), + json!({ + "type": "world_state", + "payload": { "full": true, "state": { "cwd": "/tmp" } }, + }), + json!({ + "type": "event_msg", + "payload": { "type": "warning", "message": "heads up" }, + }), + ]; + + for expected in fixtures { + let item = serde_json::from_value::(expected.clone())?; + assert_eq!(serde_json::to_value(item)?, expected); + } + Ok(()) +} + +#[test] +/// Keeps the generated schema aligned with each variant's actual persisted shape. +fn rollout_item_schema_matches_tagged_payload_and_sibling_metadata() -> Result<()> { + let schema = serde_json::to_value(schemars::schema_for!(RolloutItem))?; + let variants = schema["oneOf"].as_array().expect("rollout variants"); + assert_eq!(variants.len(), 8); + + for variant in variants { + let required = variant["required"].as_array().expect("required fields"); + assert!(required.contains(&json!("type")), "schema: {variant}"); + assert!(required.contains(&json!("payload")), "schema: {variant}"); + } + + let response_item = variants + .iter() + .find(|variant| variant["properties"]["type"]["enum"] == json!(["response_item"])) + .expect("response item schema"); + assert!(response_item["properties"].get("metadata").is_some()); + assert_eq!( + response_item["properties"]["payload"]["$ref"], + json!("#/definitions/ResponseItem") + ); + + let compacted = &schema["definitions"]["CompactedItem"]; + assert_eq!( + compacted["properties"]["replacement_history"]["items"]["$ref"], + json!("#/definitions/ResponseItem") + ); + assert_eq!( + compacted["properties"]["replacement_history_metadata"]["items"]["$ref"], + json!("#/definitions/CodexHarnessMetadata") + ); + let required = compacted["required"].as_array().expect("required fields"); + assert!(!required.contains(&json!("replacement_history"))); + assert!(!required.contains(&json!("replacement_history_metadata"))); + Ok(()) +} + +fn response_message(role: &str) -> ResponseItem { + ResponseItem::Message { + id: None, + role: role.to_string(), + content: vec![ContentItem::InputText { + text: "hello".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } +} + +#[test] +/// Preserves the stored compacted-item window metadata shape. fn compacted_item_serializes_window_number_and_id() -> Result<()> { let item = CompactedItem { message: "summary".to_string(), @@ -111,6 +425,7 @@ fn compacted_item_serializes_window_number_and_id() -> Result<()> { } #[test] +/// Keeps legacy numeric window IDs readable in stored compacted items. fn compacted_item_migrates_legacy_numeric_window_id() -> Result<()> { let item = serde_json::from_value::(json!({ "message": "summary", diff --git a/codex-rs/memories/write/src/phase1.rs b/codex-rs/memories/write/src/phase1.rs index ac0684a132..f655da3ac2 100644 --- a/codex-rs/memories/write/src/phase1.rs +++ b/codex-rs/memories/write/src/phase1.rs @@ -408,7 +408,7 @@ mod job { let filtered = items .iter() .filter_map(|item| match item { - RolloutItem::ResponseItem(item) => sanitize_response_item_for_memories(item), + RolloutItem::ResponseItem(item) => sanitize_response_item_for_memories(&item.item), RolloutItem::InterAgentCommunication(communication) => { Some(communication.to_model_input_item()) } @@ -720,9 +720,9 @@ mod tests { }; let serialized = job::serialize_filtered_rollout_response_items(&[ - RolloutItem::ResponseItem(mixed_contextual_message), - RolloutItem::ResponseItem(skill_message), - RolloutItem::ResponseItem(subagent_message.clone()), + RolloutItem::ResponseItem(mixed_contextual_message.into()), + RolloutItem::ResponseItem(skill_message.into()), + RolloutItem::ResponseItem(subagent_message.clone().into()), ]) .expect("serialize"); let parsed: Vec = serde_json::from_str(&serialized).expect("parse"); @@ -759,7 +759,8 @@ mod tests { success: Some(true), }, internal_chat_message_metadata_passthrough: None, - }, + } + .into(), )]) .expect("serialize"); @@ -811,7 +812,7 @@ mod tests { let serialized = job::serialize_filtered_rollout_response_items(&[ RolloutItem::InterAgentCommunicationMetadata { trigger_turn: true }, - RolloutItem::ResponseItem(response_item.clone()), + RolloutItem::ResponseItem(response_item.clone().into()), ]) .expect("serialize"); let parsed: Vec = serde_json::from_str(&serialized).expect("parse"); diff --git a/codex-rs/memories/write/src/startup_tests.rs b/codex-rs/memories/write/src/startup_tests.rs index 7cb3d4bc78..df18be0f27 100644 --- a/codex-rs/memories/write/src/startup_tests.rs +++ b/codex-rs/memories/write/src/startup_tests.rs @@ -759,15 +759,18 @@ async fn seed_stage1_candidate( let line = RolloutLine { timestamp: updated_at.to_rfc3339(), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "remember this startup test conversation".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }), + item: RolloutItem::ResponseItem( + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "remember this startup test conversation".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + ), }; let jsonl = serde_json::to_string(&line)?; tokio::fs::write(&rollout_path, format!("{jsonl}\n")).await?; diff --git a/codex-rs/rollout/src/compression_tests.rs b/codex-rs/rollout/src/compression_tests.rs index 94ecebc12a..5fc3ad5802 100644 --- a/codex-rs/rollout/src/compression_tests.rs +++ b/codex-rs/rollout/src/compression_tests.rs @@ -6,9 +6,6 @@ use std::os::unix::fs::PermissionsExt; use std::time::Duration; use std::time::SystemTime; -use crate::InitialHistory; -use crate::RolloutItem; -use crate::RolloutLine; use codex_protocol::ThreadId; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::HistoryPosition; @@ -21,7 +18,10 @@ use tempfile::TempDir; use uuid::Uuid; use super::*; +use crate::InitialHistory; use crate::RolloutConfig; +use crate::RolloutItem; +use crate::RolloutLine; use crate::RolloutRecorder; use crate::RolloutRecorderParams; use crate::append_rollout_item_to_path; diff --git a/codex-rs/rollout/src/lib.rs b/codex-rs/rollout/src/lib.rs index b5713bfd80..2568ab0f52 100644 --- a/codex-rs/rollout/src/lib.rs +++ b/codex-rs/rollout/src/lib.rs @@ -23,6 +23,7 @@ pub mod state_db; pub use codex_history::CompactedItem; pub use codex_history::InitialHistory; +pub use codex_history::ResponseItemEnvelope; pub use codex_history::ResumedHistory; pub use codex_history::RolloutItem; pub use codex_history::RolloutLine; diff --git a/codex-rs/rollout/src/list.rs b/codex-rs/rollout/src/list.rs index 33b0560819..df1f608e4b 100644 --- a/codex-rs/rollout/src/list.rs +++ b/codex-rs/rollout/src/list.rs @@ -1240,7 +1240,7 @@ pub async fn read_head_for_summary(path: &Path) -> io::Result { - if let Ok(value) = serde_json::to_value(item) { + if let Ok(value) = serde_json::to_value(&item.item) { head.push(value); } } diff --git a/codex-rs/rollout/src/model_context.rs b/codex-rs/rollout/src/model_context.rs index f333689b35..7976e84ca8 100644 --- a/codex-rs/rollout/src/model_context.rs +++ b/codex-rs/rollout/src/model_context.rs @@ -1,3 +1,4 @@ +use crate::ResponseItemEnvelope; use crate::RolloutItem; use codex_protocol::items::TurnItem; use codex_protocol::models::ResponseItem; @@ -186,8 +187,8 @@ fn turn_ids_are_compatible(active_turn_id: Option<&str>, item_turn_id: Option<&s .is_none_or(|turn_id| item_turn_id.is_none_or(|item_turn_id| item_turn_id == turn_id)) } -fn response_item_counts_as_user_turn(response_item: &ResponseItem) -> bool { - match response_item { +fn response_item_counts_as_user_turn(response_item: &ResponseItemEnvelope) -> bool { + match &response_item.item { ResponseItem::AgentMessage { .. } => true, ResponseItem::Message { role, content, .. } => { role == "assistant" && InterAgentCommunication::is_message_content(content) diff --git a/codex-rs/rollout/src/ordinal.rs b/codex-rs/rollout/src/ordinal.rs index 14483ce706..3f002aec8a 100644 --- a/codex-rs/rollout/src/ordinal.rs +++ b/codex-rs/rollout/src/ordinal.rs @@ -6,11 +6,11 @@ use std::io::Seek; use std::io::SeekFrom; use std::path::Path; -use crate::RolloutItem; -use crate::RolloutLine; use codex_protocol::protocol::HistoryPosition; use codex_protocol::protocol::ThreadHistoryMode; +use crate::RolloutItem; +use crate::RolloutLine; use crate::reverse_jsonl_scanner::ReverseJsonlScanner; use crate::reverse_jsonl_scanner::ScanOutcome; diff --git a/codex-rs/rollout/src/persistence_metrics.rs b/codex-rs/rollout/src/persistence_metrics.rs index 3f1bc15756..79f1fd789d 100644 --- a/codex-rs/rollout/src/persistence_metrics.rs +++ b/codex-rs/rollout/src/persistence_metrics.rs @@ -2,7 +2,6 @@ use std::io::Write; use std::sync::Arc; use std::sync::Mutex; -use crate::RolloutItem; use codex_otel::MetricsClient; use codex_protocol::ThreadId; use codex_protocol::items::TurnItem; @@ -10,6 +9,8 @@ use codex_protocol::models::ResponseItem; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ThreadHistoryMode; +use crate::ResponseItemEnvelope; +use crate::RolloutItem; use crate::policy::is_persisted_rollout_item; const ITEM_BYTES_METRIC: &str = "codex.rollout.persistence.item_bytes"; @@ -265,8 +266,8 @@ fn turn_item_type(item: &TurnItem) -> &'static str { } } -fn response_item_type(item: &ResponseItem) -> &'static str { - match item { +fn response_item_type(item: &ResponseItemEnvelope) -> &'static str { + match &item.item { ResponseItem::Message { .. } => "response.message", ResponseItem::AdditionalTools { .. } => "response.additional_tools", ResponseItem::AgentMessage { .. } => "response.agent_message", diff --git a/codex-rs/rollout/src/persistence_metrics_tests.rs b/codex-rs/rollout/src/persistence_metrics_tests.rs index f53c0bb1f1..fed1b112e8 100644 --- a/codex-rs/rollout/src/persistence_metrics_tests.rs +++ b/codex-rs/rollout/src/persistence_metrics_tests.rs @@ -1,4 +1,3 @@ -use crate::RolloutItem; use codex_protocol::ThreadId; use codex_protocol::items::EnteredReviewModeItem; use codex_protocol::items::ExitedReviewModeItem; @@ -25,9 +24,11 @@ use super::TurnSizeTotals; use super::is_thread_sampled; use super::measure_and_filter_rollout_items; use super::update_turn_measurements; +use crate::ResponseItemEnvelope; +use crate::RolloutItem; fn retained_message(text: &str) -> RolloutItem { - RolloutItem::ResponseItem(ResponseItem::Message { + RolloutItem::ResponseItem(ResponseItemEnvelope::new(ResponseItem::Message { id: None, role: "user".to_string(), content: vec![ContentItem::InputText { @@ -35,7 +36,7 @@ fn retained_message(text: &str) -> RolloutItem { }], phase: None, internal_chat_message_metadata_passthrough: None, - }) + })) } fn turn_started(turn_id: &str) -> RolloutItem { @@ -106,7 +107,7 @@ fn thread_sampling_is_stable_and_selects_whole_threads() { #[test] fn mixed_batch_reports_exact_policy_counts_and_bytes() { let kept = retained_message("hello"); - let dropped = RolloutItem::ResponseItem(ResponseItem::Other); + let dropped = RolloutItem::ResponseItem(ResponseItemEnvelope::new(ResponseItem::Other)); let items = vec![kept.clone(), dropped.clone()]; let (persisted, measurement) = @@ -157,7 +158,7 @@ fn turn_measurements_span_batches_and_include_items_before_start() { retained_message("first prompt"), turn_started("turn-1"), retained_message("first response"), - RolloutItem::ResponseItem(ResponseItem::Other), + RolloutItem::ResponseItem(ResponseItemEnvelope::new(ResponseItem::Other)), turn_complete("turn-1"), ]; let second_turn = vec![ diff --git a/codex-rs/rollout/src/policy.rs b/codex-rs/rollout/src/policy.rs index 345da9e7f1..00a9b0f8e0 100644 --- a/codex-rs/rollout/src/policy.rs +++ b/codex-rs/rollout/src/policy.rs @@ -8,7 +8,7 @@ use codex_protocol::protocol::ThreadHistoryMode; /// Whether a rollout `item` should be persisted in rollout files. pub fn is_persisted_rollout_item(item: &RolloutItem, history_mode: ThreadHistoryMode) -> bool { match item { - RolloutItem::ResponseItem(item) => should_persist_response_item(item), + RolloutItem::ResponseItem(item) => should_persist_response_item(&item.item), RolloutItem::InterAgentCommunication(_) | RolloutItem::InterAgentCommunicationMetadata { .. } => true, RolloutItem::EventMsg(ev) => should_persist_event_msg(ev, history_mode), diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index aa210c2ba9..189488bc2d 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -1110,12 +1110,43 @@ fn strip_legacy_ghost_snapshot_rollout_line(value: &mut Value) -> bool { .get("payload") .is_some_and(is_legacy_ghost_snapshot_response_item), Some("compacted") => { - if let Some(replacement_history) = value - .get_mut("payload") - .and_then(|payload| payload.get_mut("replacement_history")) + let Some(payload) = value.get_mut("payload").and_then(Value::as_object_mut) else { + return false; + }; + let Some(replacement_history) = + payload.get("replacement_history").and_then(Value::as_array) + else { + return false; + }; + let remove = replacement_history + .iter() + .map(is_legacy_ghost_snapshot_response_item) + .collect::>(); + if !remove.contains(&true) { + return false; + } + + // Legacy checkpoints have no sidecar. If a sidecar is present, only filter a + // full-length array; malformed shapes should remain intact for typed deserialization + // to reject instead of silently shifting metadata onto a different history item. + match payload.get("replacement_history_metadata") { + None => {} + Some(Value::Array(metadata)) if metadata.len() == remove.len() => {} + Some(_) => return false, + } + + let Some(replacement_history) = payload + .get_mut("replacement_history") + .and_then(Value::as_array_mut) + else { + return false; + }; + retain_entries_not_marked(replacement_history, &remove); + if let Some(metadata) = payload + .get_mut("replacement_history_metadata") .and_then(Value::as_array_mut) { - replacement_history.retain(|item| !is_legacy_ghost_snapshot_response_item(item)); + retain_entries_not_marked(metadata, &remove); } false } @@ -1123,6 +1154,15 @@ fn strip_legacy_ghost_snapshot_rollout_line(value: &mut Value) -> bool { } } +fn retain_entries_not_marked(entries: &mut Vec, remove: &[bool]) { + let mut index = 0; + entries.retain(|_| { + let retain = !remove[index]; + index += 1; + retain + }); +} + fn is_legacy_ghost_snapshot_response_item(value: &Value) -> bool { value.get("type").and_then(Value::as_str) == Some("ghost_snapshot") } diff --git a/codex-rs/rollout/src/recorder_tests.rs b/codex-rs/rollout/src/recorder_tests.rs index 62d652c030..d421166915 100644 --- a/codex-rs/rollout/src/recorder_tests.rs +++ b/codex-rs/rollout/src/recorder_tests.rs @@ -1,6 +1,7 @@ #![allow(warnings, clippy::all)] use super::*; +use crate::ResponseItemEnvelope; use crate::RolloutItem; use crate::RolloutLine; use crate::config::RolloutConfig; @@ -321,7 +322,10 @@ async fn load_rollout_items_defaults_legacy_session_id() -> std::io::Result<()> assert_eq!(session_meta.meta.session_id, SessionId::from(thread_id)); assert!(matches!( items[1], - RolloutItem::ResponseItem(ResponseItem::Message { .. }) + RolloutItem::ResponseItem(ResponseItemEnvelope { + item: ResponseItem::Message { .. }, + .. + }) )); Ok(()) @@ -502,12 +506,51 @@ async fn load_rollout_items_filters_legacy_ghost_snapshots_from_compaction_histo assert_eq!(replacement_history.len(), 1); assert!(matches!( &replacement_history[0], - ResponseItem::Message { .. } + ResponseItemEnvelope { + item: ResponseItem::Message { .. }, + .. + } )); Ok(()) } +#[test] +fn strip_legacy_ghost_snapshot_keeps_checkpoint_metadata_aligned() { + let mut value = serde_json::json!({ + "type": "compacted", + "payload": { + "message": "summary", + "replacement_history": [ + {"type": "message", "role": "assistant", "content": []}, + {"type": "ghost_snapshot", "ghost_commit": {"id": "deadbeef"}}, + {"type": "message", "role": "user", "content": []} + ], + "replacement_history_metadata": [ + {"slot": "assistant"}, + {"slot": "ghost"}, + {"slot": "user"} + ] + } + }); + + assert!(!strip_legacy_ghost_snapshot_rollout_line(&mut value)); + assert_eq!( + value["payload"]["replacement_history"], + serde_json::json!([ + {"type": "message", "role": "assistant", "content": []}, + {"type": "message", "role": "user", "content": []} + ]) + ); + assert_eq!( + value["payload"]["replacement_history_metadata"], + serde_json::json!([ + {"slot": "assistant"}, + {"slot": "user"} + ]) + ); +} + #[tokio::test] async fn recorder_materializes_on_flush_with_pending_items() -> std::io::Result<()> { let home = TempDir::new().expect("temp dir"); diff --git a/codex-rs/rollout/src/search.rs b/codex-rs/rollout/src/search.rs index d292b24d33..6f0a30b1fd 100644 --- a/codex-rs/rollout/src/search.rs +++ b/codex-rs/rollout/src/search.rs @@ -5,8 +5,6 @@ use std::path::Path; use std::path::PathBuf; use std::process::Stdio; -use crate::RolloutItem; -use crate::RolloutLine; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::EventMsg; @@ -18,6 +16,9 @@ use tokio::process::Command; use super::ARCHIVED_SESSIONS_SUBDIR; use super::SESSIONS_SUBDIR; use super::compression; +use crate::ResponseItemEnvelope; +use crate::RolloutItem; +use crate::RolloutLine; const MATCH_CONTEXT_BEFORE_CHARS: usize = 48; const MATCH_CONTEXT_AFTER_CHARS: usize = 96; @@ -269,7 +270,10 @@ fn conversation_text_from_item(item: &RolloutItem) -> Option { Some(agent.message.trim().to_string()) } } - RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) => { + RolloutItem::ResponseItem(ResponseItemEnvelope { + item: ResponseItem::Message { role, content, .. }, + .. + }) => { let text = content .iter() .filter_map(content_item_text) diff --git a/codex-rs/rollout/src/tests.rs b/codex-rs/rollout/src/tests.rs index 2918804e91..16ac9cc177 100644 --- a/codex-rs/rollout/src/tests.rs +++ b/codex-rs/rollout/src/tests.rs @@ -20,6 +20,7 @@ use time::macros::format_description; use uuid::Uuid; use crate::INTERACTIVE_SESSION_SOURCES; +use crate::ResponseItemEnvelope; use crate::RolloutItem; use crate::RolloutLine; use crate::find_thread_path_by_id_str; @@ -31,6 +32,7 @@ use crate::list::get_threads; use crate::list::read_head_for_summary; use crate::rollout_date_parts; use anyhow::Result; +use codex_history::CodexHarnessMetadata; use codex_protocol::ThreadId; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; @@ -1264,6 +1266,39 @@ async fn test_base_instructions_present_in_meta_is_preserved() { assert_eq!(base, Some(base_text)); } +#[tokio::test] +async fn read_head_for_summary_omits_harness_metadata() { + let temp = TempDir::new().unwrap(); + let rollout_path = temp.path().join("rollout.jsonl"); + let response_item = ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "hello".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }; + let line = RolloutLine { + timestamp: "2025-04-03T10:30:00Z".to_string(), + ordinal: None, + item: RolloutItem::ResponseItem(ResponseItemEnvelope { + item: response_item.clone(), + metadata: Some(CodexHarnessMetadata {}), + }), + }; + fs::write( + &rollout_path, + format!("{}\n", serde_json::to_string(&line).unwrap()), + ) + .unwrap(); + + let head = read_head_for_summary(&rollout_path).await.unwrap(); + + assert_eq!(head, vec![serde_json::to_value(response_item).unwrap()]); + assert!(head[0].get("metadata").is_none()); +} + #[tokio::test] async fn test_created_at_sort_uses_file_mtime_for_updated_at() -> Result<()> { let temp = TempDir::new().unwrap(); @@ -1384,7 +1419,7 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> { let response_line = RolloutLine { timestamp: format!("{ts}-{idx:02}"), ordinal: None, - item: RolloutItem::ResponseItem(ResponseItem::Message { + item: RolloutItem::ResponseItem(ResponseItemEnvelope::new(ResponseItem::Message { id: None, role: "assistant".into(), content: vec![ContentItem::OutputText { @@ -1392,7 +1427,7 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> { }], phase: None, internal_chat_message_metadata_passthrough: None, - }), + })), }; writeln!(file, "{}", serde_json::to_string(&response_line)?)?; } diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index c60bdbdcb8..68642283a4 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -21,7 +21,7 @@ pub fn apply_rollout_item( RolloutItem::SessionMeta(meta_line) => apply_session_meta_from_item(metadata, meta_line), RolloutItem::TurnContext(turn_ctx) => apply_turn_context(metadata, turn_ctx), RolloutItem::EventMsg(event) => apply_event_msg(metadata, event), - RolloutItem::ResponseItem(item) => apply_response_item(metadata, item), + RolloutItem::ResponseItem(item) => apply_response_item(metadata, &item.item), RolloutItem::InterAgentCommunication(_) | RolloutItem::InterAgentCommunicationMetadata { .. } => {} RolloutItem::Compacted(_) => {} @@ -206,15 +206,18 @@ mod tests { #[test] fn response_item_user_messages_do_not_set_title_or_first_user_message() { let mut metadata = metadata_for_test(); - let item = RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "hello from response item".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }); + let item = RolloutItem::ResponseItem( + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "hello from response item".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + ); apply_rollout_item(&mut metadata, &item, "test-provider"); diff --git a/codex-rs/thread-store/src/local/mod.rs b/codex-rs/thread-store/src/local/mod.rs index af6bbb3423..f4f14fe730 100644 --- a/codex-rs/thread-store/src/local/mod.rs +++ b/codex-rs/thread-store/src/local/mod.rs @@ -926,12 +926,15 @@ mod tests { phase: Some(MessagePhase::Commentary), memory_citation: None, })), - RolloutItem::ResponseItem(ResponseItem::FunctionCallOutput { - id: None, - call_id: "call-1".to_string(), - output: FunctionCallOutputPayload::from_text("tool output".to_string()), - internal_chat_message_metadata_passthrough: None, - }), + RolloutItem::ResponseItem( + ResponseItem::FunctionCallOutput { + id: None, + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload::from_text("tool output".to_string()), + internal_chat_message_metadata_passthrough: None, + } + .into(), + ), RolloutItem::EventMsg(EventMsg::TokenCount( codex_protocol::protocol::TokenCountEvent { info: None, diff --git a/codex-rs/thread-store/src/local/model_context_tests.rs b/codex-rs/thread-store/src/local/model_context_tests.rs index 882e3998f0..3e43af78c4 100644 --- a/codex-rs/thread-store/src/local/model_context_tests.rs +++ b/codex-rs/thread-store/src/local/model_context_tests.rs @@ -541,15 +541,18 @@ fn turn_complete(turn_id: &str) -> RolloutItem { } fn user_message(message: &str) -> RolloutItem { - RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: message.to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }) + RolloutItem::ResponseItem( + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: message.to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + ) } fn contextual_user_message() -> RolloutItem { @@ -575,15 +578,18 @@ fn completed_user_message(turn_id: &str, message: &str) -> RolloutItem { } fn agent_message(message: &str) -> RolloutItem { - RolloutItem::ResponseItem(ResponseItem::AgentMessage { - id: None, - author: "worker".to_string(), - recipient: "root".to_string(), - content: vec![AgentMessageInputContent::InputText { - text: message.to_string(), - }], - internal_chat_message_metadata_passthrough: None, - }) + RolloutItem::ResponseItem( + ResponseItem::AgentMessage { + id: None, + author: "worker".to_string(), + recipient: "root".to_string(), + content: vec![AgentMessageInputContent::InputText { + text: message.to_string(), + }], + internal_chat_message_metadata_passthrough: None, + } + .into(), + ) } fn turn_context(root: &Path, turn_id: &str) -> RolloutItem { @@ -614,7 +620,8 @@ fn turn_context(root: &Path, turn_id: &str) -> RolloutItem { fn compacted(message: &str, replacement_history: Option>) -> RolloutItem { RolloutItem::Compacted(CompactedItem { message: message.to_string(), - replacement_history, + replacement_history: replacement_history + .map(|items| items.into_iter().map(Into::into).collect()), window_number: Some(1), first_window_id: None, previous_window_id: None, diff --git a/codex-rs/thread-store/src/local/rollout_migration/canonicalizer.rs b/codex-rs/thread-store/src/local/rollout_migration/canonicalizer.rs index 5093ddef60..6e2e8ca0ef 100644 --- a/codex-rs/thread-store/src/local/rollout_migration/canonicalizer.rs +++ b/codex-rs/thread-store/src/local/rollout_migration/canonicalizer.rs @@ -114,13 +114,13 @@ impl LegacyRolloutCanonicalizer { let bytes_before = self.bytes_written; match line.item { RolloutItem::SessionMeta(_) => return Ok(0), - RolloutItem::ResponseItem(ResponseItem::Other) => { - return Err(migration_error( - "legacy rollout contains an unsupported response item", - )); - } RolloutItem::ResponseItem(response) => { - let hook = match &response { + if matches!(&response.item, ResponseItem::Other) { + return Err(migration_error( + "legacy rollout contains an unsupported response item", + )); + } + let hook = match &response.item { ResponseItem::Message { role, content, id, .. } if role == "user" => parse_hook_prompt_message(id.as_deref(), content), diff --git a/codex-rs/thread-store/src/local/rollout_migration/rollback.rs b/codex-rs/thread-store/src/local/rollout_migration/rollback.rs index 428aecb45b..7f6d992e0c 100644 --- a/codex-rs/thread-store/src/local/rollout_migration/rollback.rs +++ b/codex-rs/thread-store/src/local/rollout_migration/rollback.rs @@ -12,6 +12,7 @@ use codex_protocol::items::parse_hook_prompt_fragment; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::InterAgentCommunication; +use std::borrow::Borrow; /// Match the rollback boundaries used by legacy history reconstruction without /// treating every persisted lifecycle as a user turn. This intentionally stays @@ -40,14 +41,17 @@ pub(super) fn is_pre_turn_context_update(response: &ResponseItem) -> bool { /// Apply the same response-history cut used by legacy cold resume to a persisted /// compaction checkpoint. The migration adapter uses a frozen contextual-fragment /// matcher because thread-store cannot depend on core's runtime fragment registry. -pub(super) fn drop_last_n_user_turns(history: &mut Vec, num_turns: u32) { +pub(super) fn drop_last_n_user_turns(history: &mut Vec, num_turns: u32) +where + T: Borrow, +{ if num_turns == 0 { return; } let user_positions = history .iter() .enumerate() - .filter_map(|(index, item)| counts_as_boundary(item).then_some(index)) + .filter_map(|(index, item)| counts_as_boundary(item.borrow()).then_some(index)) .collect::>(); let Some(&first_turn_index) = user_positions.first() else { return; @@ -58,7 +62,9 @@ pub(super) fn drop_last_n_user_turns(history: &mut Vec, num_turns: } else { user_positions[user_positions.len() - count] }; - while cut_index > first_turn_index && is_pre_turn_context_update(&history[cut_index - 1]) { + while cut_index > first_turn_index + && is_pre_turn_context_update(history[cut_index - 1].borrow()) + { cut_index -= 1; } history.truncate(cut_index); diff --git a/codex-rs/thread-store/src/local/rollout_migration/rollback_plan.rs b/codex-rs/thread-store/src/local/rollout_migration/rollback_plan.rs index 9f1b253a79..efe492584e 100644 --- a/codex-rs/thread-store/src/local/rollout_migration/rollback_plan.rs +++ b/codex-rs/thread-store/src/local/rollout_migration/rollback_plan.rs @@ -121,7 +121,9 @@ impl RollbackPlanner { _ => None, }; let paired_delivery_boundary = match (&self.pending_delivery_boundary, &line.item) { - (Some(boundary), RolloutItem::ResponseItem(ResponseItem::AgentMessage { .. })) => { + (Some(boundary), RolloutItem::ResponseItem(response)) + if matches!(&response.item, ResponseItem::AgentMessage { .. }) => + { Some(*boundary) } _ => None, @@ -134,9 +136,9 @@ impl RollbackPlanner { RolloutItem::ResponseItem(response) => { if let Some(boundary) = paired_delivery_boundary { self.record_boundaries[index] = Some(boundary); - } else if rollback::counts_as_boundary(response) { + } else if rollback::counts_as_boundary(&response.item) { let boundary = self.start_boundary(index); - if let ResponseItem::Message { role, content, .. } = response + if let ResponseItem::Message { role, content, .. } = &response.item && role == "user" { self.pending_user_response = Some(PendingUserResponse { @@ -144,7 +146,7 @@ impl RollbackPlanner { content: content.clone(), }); } - } else if rollback::is_pre_turn_context_update(response) { + } else if rollback::is_pre_turn_context_update(&response.item) { // Until another user boundary arrives, this is trailing context for the // previous turn. Keep that fallback owner so rollback drops it when there is // no later turn to attach it to. diff --git a/codex-rs/thread-store/src/local/rollout_migration/rollback_replay.rs b/codex-rs/thread-store/src/local/rollout_migration/rollback_replay.rs index bdc3d3738c..b5f4ade103 100644 --- a/codex-rs/thread-store/src/local/rollout_migration/rollback_replay.rs +++ b/codex-rs/thread-store/src/local/rollout_migration/rollback_replay.rs @@ -68,7 +68,7 @@ impl ModelReplayPlanner { RolloutItem::TurnContext(context) => ReplayRecord::TurnContext(context.turn_id.clone()), RolloutItem::EventMsg(EventMsg::UserMessage(_)) | RolloutItem::InterAgentCommunication(_) => ReplayRecord::UserBoundary, - RolloutItem::ResponseItem(response) if rollback::counts_as_boundary(response) => { + RolloutItem::ResponseItem(response) if rollback::counts_as_boundary(&response.item) => { ReplayRecord::UserBoundary } RolloutItem::SessionMeta(_) diff --git a/codex-rs/thread-store/src/local/rollout_migration_tests.rs b/codex-rs/thread-store/src/local/rollout_migration_tests.rs index ce4a9ea7af..174bdf060e 100644 --- a/codex-rs/thread-store/src/local/rollout_migration_tests.rs +++ b/codex-rs/thread-store/src/local/rollout_migration_tests.rs @@ -165,6 +165,10 @@ fn input_response_message(role: &str, text: &str) -> ResponseItem { } } +fn rollout_response_item(item: ResponseItem) -> RolloutItem { + RolloutItem::ResponseItem(item.into()) +} + fn exec_completion(turn_id: &str, call_id: &str) -> RolloutItem { serde_json::from_value(json!({ "type": "event_msg", @@ -215,7 +219,7 @@ fn started(turn_id: &str) -> RolloutItem { fn compacted(replacement_history: Vec) -> RolloutItem { RolloutItem::Compacted(CompactedItem { message: "checkpoint".to_string(), - replacement_history: Some(replacement_history), + replacement_history: Some(replacement_history.into_iter().map(Into::into).collect()), window_number: Some(1), first_window_id: None, previous_window_id: None, @@ -671,7 +675,7 @@ async fn migration_rolls_back_response_and_inter_agent_user_boundaries() { response_thread_id, SessionSource::Cli, vec![ - RolloutItem::ResponseItem(input_response_message("user", "remove response boundary")), + rollout_response_item(input_response_message("user", "remove response boundary")), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, })), @@ -703,16 +707,16 @@ async fn migration_rolls_back_response_and_inter_agent_user_boundaries() { contextual_thread_id, SessionSource::Cli, vec![ - RolloutItem::ResponseItem(input_response_message("user", "keep first boundary")), - RolloutItem::ResponseItem(input_response_message( + rollout_response_item(input_response_message("user", "keep first boundary")), + rollout_response_item(input_response_message( "developer", "context only", )), - RolloutItem::ResponseItem(input_response_message( + rollout_response_item(input_response_message( "user", "context only", )), - RolloutItem::ResponseItem(input_response_message("user", "remove real user boundary")), + rollout_response_item(input_response_message("user", "remove real user boundary")), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, })), @@ -740,7 +744,7 @@ async fn migration_rolls_back_response_and_inter_agent_user_boundaries() { read_rollout(&contextual_path) .into_iter() .filter_map(|line| match line.item { - RolloutItem::ResponseItem(response) => Some(response), + RolloutItem::ResponseItem(response) => Some(response.into_item()), _ => None, }) .collect::>(), @@ -757,9 +761,9 @@ async fn migration_drops_trailing_context_when_rollback_arrives_before_next_turn thread_id, SessionSource::Cli, vec![ - RolloutItem::ResponseItem(input_response_message("user", "keep question")), - RolloutItem::ResponseItem(input_response_message("user", "remove question")), - RolloutItem::ResponseItem(input_response_message( + rollout_response_item(input_response_message("user", "keep question")), + rollout_response_item(input_response_message("user", "remove question")), + rollout_response_item(input_response_message( "user", "remove this context too", )), @@ -780,7 +784,7 @@ async fn migration_drops_trailing_context_when_rollback_arrives_before_next_turn read_rollout(&path) .into_iter() .filter_map(|line| match line.item { - RolloutItem::ResponseItem(response) => Some(response), + RolloutItem::ResponseItem(response) => Some(response.into_item()), _ => None, }) .collect::>(), @@ -797,7 +801,7 @@ async fn migration_coalesces_response_first_user_message_rollback_boundary() { thread_id, SessionSource::Cli, vec![ - RolloutItem::ResponseItem(input_response_message("user", "remove question")), + rollout_response_item(input_response_message("user", "remove question")), user_message("remove question"), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, @@ -828,7 +832,7 @@ async fn migration_does_not_coalesce_distinct_adjacent_user_records() { thread_id, SessionSource::Cli, vec![ - RolloutItem::ResponseItem(input_response_message("user", "copied parent question")), + rollout_response_item(input_response_message("user", "copied parent question")), user_message("child question"), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, @@ -847,7 +851,7 @@ async fn migration_does_not_coalesce_distinct_adjacent_user_records() { read_rollout(&path) .into_iter() .filter_map(|line| match line.item { - RolloutItem::ResponseItem(response) => Some(response), + RolloutItem::ResponseItem(response) => Some(response.into_item()), _ => None, }) .collect::>(), @@ -988,7 +992,7 @@ async fn migration_rolls_back_inter_agent_metadata_with_its_delivery() { SessionSource::Cli, vec![ RolloutItem::InterAgentCommunicationMetadata { trigger_turn: true }, - RolloutItem::ResponseItem(delivery.to_model_input_item()), + rollout_response_item(delivery.to_model_input_item()), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, })), @@ -1002,12 +1006,12 @@ async fn migration_rolls_back_inter_agent_metadata_with_its_delivery() { .await .expect("migrate rolled-back inter-agent delivery"); - assert!(!read_rollout(&path).iter().any(|line| { - matches!( - line.item, - RolloutItem::InterAgentCommunicationMetadata { .. } - | RolloutItem::ResponseItem(ResponseItem::AgentMessage { .. }) - ) + assert!(!read_rollout(&path).iter().any(|line| match &line.item { + RolloutItem::InterAgentCommunicationMetadata { .. } => true, + RolloutItem::ResponseItem(response_item) => { + matches!(&response_item.item, ResponseItem::AgentMessage { .. }) + } + _ => false, })); } @@ -1082,7 +1086,7 @@ async fn migration_preserves_reverse_replay_anchor_after_pre_compaction_rollback thread_id, SessionSource::Cli, vec![ - RolloutItem::ResponseItem(input_response_message("user", "remove question")), + rollout_response_item(input_response_message("user", "remove question")), RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1, })), @@ -1203,7 +1207,12 @@ async fn migration_uses_turn_context_to_select_reverse_replay_anchor() { let replacement_histories = read_rollout(&path) .into_iter() .filter_map(|line| match line.item { - RolloutItem::Compacted(item) => item.replacement_history, + RolloutItem::Compacted(item) => item.replacement_history.map(|items| { + items + .into_iter() + .map(codex_rollout::ResponseItemEnvelope::into_item) + .collect::>() + }), _ => None, }) .collect::>(); @@ -1279,7 +1288,7 @@ async fn migration_drops_copied_user_fork_metadata_without_creating_a_history_ba ..SessionMeta::default() }; let copied_response = - RolloutItem::ResponseItem(input_response_message("user", "copied parent history")); + rollout_response_item(input_response_message("user", "copied parent history")); let path = write_rollout_with_fork( home.path(), thread_id, @@ -1299,7 +1308,7 @@ async fn migration_drops_copied_user_fork_metadata_without_creating_a_history_ba .into_iter() .filter_map(|line| match line.item { RolloutItem::ResponseItem(item) => { - Some(serde_json::to_value(item).expect("serialize copied response")) + Some(serde_json::to_value(item.item).expect("serialize copied response")) } _ => None, }) @@ -1333,7 +1342,7 @@ async fn migration_drops_copied_user_fork_metadata_without_creating_a_history_ba .into_iter() .filter_map(|line| match line.item { RolloutItem::ResponseItem(item) => { - Some(serde_json::to_value(item).expect("serialize migrated response")) + Some(serde_json::to_value(item.item).expect("serialize migrated response")) } _ => None, }) @@ -1361,15 +1370,18 @@ async fn migration_compacts_subagent_prefix_and_does_not_project_it() { }), RolloutItem::Compacted(CompactedItem { message: "latest checkpoint".to_string(), - replacement_history: Some(vec![ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: "latest compacted context".to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }]), + replacement_history: Some(vec![ + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "latest compacted context".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + ]), window_number: Some(2), first_window_id: None, previous_window_id: None, diff --git a/codex-rs/thread-store/src/local/thread_history_materialization_tests.rs b/codex-rs/thread-store/src/local/thread_history_materialization_tests.rs index b3edce228b..b32aae7d3d 100644 --- a/codex-rs/thread-store/src/local/thread_history_materialization_tests.rs +++ b/codex-rs/thread-store/src/local/thread_history_materialization_tests.rs @@ -2296,26 +2296,31 @@ fn turn_completed(turn_id: &str) -> RolloutItem { } fn user_message(message: &str) -> RolloutItem { - RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: message.to_string(), - }], - phase: None, - internal_chat_message_metadata_passthrough: None, - }) + RolloutItem::ResponseItem( + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: message.to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + } + .into(), + ) } fn contains_user_message(items: &[RolloutItem], expected: &str) -> bool { items.iter().any(|item| { - matches!( - item, - RolloutItem::ResponseItem(ResponseItem::Message { content, .. }) - if content.iter().any(|content| { - matches!(content, ContentItem::InputText { text } if text == expected) - }) - ) + let RolloutItem::ResponseItem(response_item) = item else { + return false; + }; + let ResponseItem::Message { content, .. } = &response_item.item else { + return false; + }; + content + .iter() + .any(|content| matches!(content, ContentItem::InputText { text } if text == expected)) }) }