diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 42e2d92b28..52ad78d137 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -212,13 +212,11 @@ async fn run_compact_task_inner( .collect(); new_history.extend(ghost_snapshots); let latest_history_snapshot = sess.clone_history().await; - if merge_appended_history_items( + if !append_concurrent_history_tail_if_append_only( &mut new_history, history_items, latest_history_snapshot.raw_items(), - ) - .is_none() - { + ) { warn!( turn_id = %turn_context.sub_id, "session history changed non-append-only during compaction; skipping concurrent tail merge" @@ -284,21 +282,28 @@ pub(crate) fn is_summary_message(message: &str) -> bool { message.starts_with(format!("{SUMMARY_PREFIX}\n").as_str()) } -pub(crate) fn merge_appended_history_items( +/// Appends items added after `base_history` only when `latest_history` still +/// preserves `base_history` as an exact prefix. +/// +/// Returns `true` when no concurrent history change occurred or when the newer +/// history differs only by append-only tail growth, in which case that tail is +/// appended to `new_history`. Returns `false` when concurrent history mutation +/// rewrote or removed earlier items, since this helper cannot safely merge that +/// shape. +pub(crate) fn append_concurrent_history_tail_if_append_only( new_history: &mut Vec, base_history: &[ResponseItem], latest_history: &[ResponseItem], -) -> Option { +) -> bool { if latest_history == base_history { - return Some(0); + return true; } if !latest_history.starts_with(base_history) { - return None; + return false; } - let appended_count = latest_history.len() - base_history.len(); new_history.extend_from_slice(&latest_history[base_history.len()..]); - Some(appended_count) + true } /// Inserts canonical initial context into compacted replacement history at the diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 14680d5057..838a12d98f 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -6,8 +6,8 @@ use crate::codex::Session; use crate::codex::TurnContext; use crate::codex::built_tools; use crate::compact::InitialContextInjection; +use crate::compact::append_concurrent_history_tail_if_append_only; use crate::compact::insert_initial_context_before_last_real_user_or_summary; -use crate::compact::merge_appended_history_items; use crate::context_manager::ContextManager; use crate::context_manager::TotalTokenUsageBreakdown; use crate::context_manager::estimate_response_item_model_visible_bytes; @@ -152,13 +152,11 @@ async fn run_remote_compact_task_inner_impl( new_history.extend(ghost_snapshots); } let latest_history_snapshot = sess.clone_history().await; - if merge_appended_history_items( + if !append_concurrent_history_tail_if_append_only( &mut new_history, history_snapshot.raw_items(), latest_history_snapshot.raw_items(), - ) - .is_none() - { + ) { warn!( turn_id = %turn_context.sub_id, "session history changed non-append-only during remote compaction; skipping concurrent tail merge" diff --git a/codex-rs/core/src/compact_tests.rs b/codex-rs/core/src/compact_tests.rs index 2ce9986bcf..0c34345c62 100644 --- a/codex-rs/core/src/compact_tests.rs +++ b/codex-rs/core/src/compact_tests.rs @@ -186,7 +186,7 @@ fn build_token_limited_compacted_history_appends_summary_message() { } #[test] -fn merge_appended_history_items_appends_concurrent_tail() { +fn append_concurrent_history_tail_if_append_only_appends_concurrent_tail() { let base_history = vec![ResponseItem::Message { id: None, role: "user".to_string(), @@ -215,10 +215,13 @@ fn merge_appended_history_items_appends_concurrent_tail() { phase: None, }]; - let merged = - merge_appended_history_items(&mut compacted_history, &base_history, &latest_history); + let merged = append_concurrent_history_tail_if_append_only( + &mut compacted_history, + &base_history, + &latest_history, + ); - assert_eq!(merged, Some(1)); + assert!(merged); assert_eq!( compacted_history, vec![ @@ -237,7 +240,7 @@ fn merge_appended_history_items_appends_concurrent_tail() { } #[test] -fn merge_appended_history_items_rejects_non_append_only_changes() { +fn append_concurrent_history_tail_if_append_only_rejects_non_append_only_changes() { let base_history = vec![ResponseItem::Message { id: None, role: "user".to_string(), @@ -266,10 +269,13 @@ fn merge_appended_history_items_rejects_non_append_only_changes() { phase: None, }]; - let merged = - merge_appended_history_items(&mut compacted_history, &base_history, &latest_history); + let merged = append_concurrent_history_tail_if_append_only( + &mut compacted_history, + &base_history, + &latest_history, + ); - assert_eq!(merged, None); + assert!(!merged); assert_eq!( compacted_history, vec![ResponseItem::Message {