diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index ad434e7254..c2ea87cb6e 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -212,17 +212,20 @@ async fn run_compact_task_inner( .collect(); new_history.extend(ghost_snapshots); // Compaction snapshots history, waits on a model call, then replaces - // session history wholesale. Background writers can append during that - // window, so re-snapshot here and preserve any append-only tail items. + // session history wholesale. Detached ghost snapshot tasks can finish in + // that window and append `/undo` metadata directly into session history. + // Those entries are stripped from `for_prompt()`, so re-snapshot here to + // preserve an append-only ghost-snapshot tail without reintroducing + // model-visible items that were never compacted. let latest_history_snapshot = sess.clone_history().await; - if !append_concurrent_history_tail_if_append_only( + if !append_concurrent_ghost_snapshot_tail_if_append_only( &mut new_history, history_items, latest_history_snapshot.raw_items(), ) { warn!( turn_id = %turn_context.sub_id, - "session history changed non-append-only during compaction; skipping concurrent tail merge" + "session history changed beyond append-only ghost snapshots during compaction; skipping concurrent ghost snapshot merge" ); } let reference_context_item = match initial_context_injection { @@ -285,15 +288,20 @@ pub(crate) fn is_summary_message(message: &str) -> bool { message.starts_with(format!("{SUMMARY_PREFIX}\n").as_str()) } -/// Appends items added after `base_history` only when `latest_history` still -/// preserves `base_history` as an exact prefix. +/// Appends ghost snapshots added after `base_history` only when +/// `latest_history` still preserves `base_history` as an exact prefix. +/// +/// Ghost snapshots are appended by detached background tasks for `/undo`, but +/// `ContextManager::for_prompt()` strips them before any model request. That +/// makes it safe to preserve a concurrent append-only ghost-snapshot tail while +/// avoiding model-visible items that the compaction request never saw. /// /// 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( +/// history differs only by append-only ghost snapshots, in which case that tail +/// is appended to `new_history`. Returns `false` when concurrent history +/// mutation rewrote or removed earlier items, or appended any non-ghost item, +/// since this helper cannot safely merge those shapes. +pub(crate) fn append_concurrent_ghost_snapshot_tail_if_append_only( new_history: &mut Vec, base_history: &[ResponseItem], latest_history: &[ResponseItem], @@ -305,7 +313,15 @@ pub(crate) fn append_concurrent_history_tail_if_append_only( return false; } - new_history.extend_from_slice(&latest_history[base_history.len()..]); + let appended_items = &latest_history[base_history.len()..]; + if !appended_items + .iter() + .all(|item| matches!(item, ResponseItem::GhostSnapshot { .. })) + { + return false; + } + + new_history.extend_from_slice(appended_items); true } diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 7bf7161fb8..aecd54e152 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -6,7 +6,7 @@ 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::append_concurrent_ghost_snapshot_tail_if_append_only; use crate::compact::insert_initial_context_before_last_real_user_or_summary; use crate::context_manager::ContextManager; use crate::context_manager::TotalTokenUsageBreakdown; @@ -152,17 +152,20 @@ async fn run_remote_compact_task_inner_impl( new_history.extend(ghost_snapshots); } // Remote compaction snapshots history, waits on an API call, then replaces - // session history wholesale. Background writers can append during that - // window, so re-snapshot here and preserve any append-only tail items. + // session history wholesale. Detached ghost snapshot tasks can finish in + // that window and append `/undo` metadata directly into session history. + // Those entries are stripped from `for_prompt()`, so re-snapshot here to + // preserve an append-only ghost-snapshot tail without reintroducing + // model-visible items that were never compacted. let latest_history_snapshot = sess.clone_history().await; - if !append_concurrent_history_tail_if_append_only( + if !append_concurrent_ghost_snapshot_tail_if_append_only( &mut new_history, history_snapshot.raw_items(), latest_history_snapshot.raw_items(), ) { warn!( turn_id = %turn_context.sub_id, - "session history changed non-append-only during remote compaction; skipping concurrent tail merge" + "session history changed beyond append-only ghost snapshots during remote compaction; skipping concurrent ghost snapshot merge" ); } let reference_context_item = match initial_context_injection { diff --git a/codex-rs/core/src/compact_tests.rs b/codex-rs/core/src/compact_tests.rs index 0c34345c62..8fd8dd689d 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 append_concurrent_history_tail_if_append_only_appends_concurrent_tail() { +fn append_concurrent_ghost_snapshot_tail_if_append_only_appends_concurrent_tail() { let base_history = vec![ResponseItem::Message { id: None, role: "user".to_string(), @@ -215,7 +215,7 @@ fn append_concurrent_history_tail_if_append_only_appends_concurrent_tail() { phase: None, }]; - let merged = append_concurrent_history_tail_if_append_only( + let merged = append_concurrent_ghost_snapshot_tail_if_append_only( &mut compacted_history, &base_history, &latest_history, @@ -240,7 +240,61 @@ fn append_concurrent_history_tail_if_append_only_appends_concurrent_tail() { } #[test] -fn append_concurrent_history_tail_if_append_only_rejects_non_append_only_changes() { +fn append_concurrent_ghost_snapshot_tail_if_append_only_rejects_model_visible_tail() { + let base_history = vec![ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "before compact".to_string(), + }], + end_turn: None, + phase: None, + }]; + let latest_history = vec![ + base_history[0].clone(), + ResponseItem::Message { + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: "not compacted".to_string(), + }], + end_turn: None, + phase: None, + }, + ]; + let mut compacted_history = vec![ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "summary".to_string(), + }], + end_turn: None, + phase: None, + }]; + + let merged = append_concurrent_ghost_snapshot_tail_if_append_only( + &mut compacted_history, + &base_history, + &latest_history, + ); + + assert!(!merged); + assert_eq!( + compacted_history, + vec![ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "summary".to_string(), + }], + end_turn: None, + phase: None, + }] + ); +} + +#[test] +fn append_concurrent_ghost_snapshot_tail_if_append_only_rejects_non_append_only_changes() { let base_history = vec![ResponseItem::Message { id: None, role: "user".to_string(), @@ -269,7 +323,7 @@ fn append_concurrent_history_tail_if_append_only_rejects_non_append_only_changes phase: None, }]; - let merged = append_concurrent_history_tail_if_append_only( + let merged = append_concurrent_ghost_snapshot_tail_if_append_only( &mut compacted_history, &base_history, &latest_history,