From 330ae6a516009e127db01843f5049497d3c46805 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 23 Jun 2026 10:23:25 -0400 Subject: [PATCH] Share resumed rollout history (#28426) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Resuming a persisted thread currently deep-clones its complete rollout history several times. `InitialHistory` is retained for the app-server response, copied into thread persistence, and copied again by read-only accessors. These copies scale with the complete rollout rather than the bounded model context and add measurable latency for large sessions. This change stores resumed rollout history in `Arc>`. Rollout loading wraps the parsed vector once, while app-server response construction, session initialization, and thread persistence share it through inexpensive `Arc` clones. Read-only history access now returns a borrowed slice, and fork paths use `Arc::unwrap_or_clone` where they genuinely need mutable ownership. Rollout reconstruction also consumes its temporary context instead of cloning the reconstructed model history. The serialized representation remains unchanged. In an artificial 123 MB rollout benchmark, sharing resumed history reduced cold resume latency by roughly 9–10%. The affected crates compile with their test targets, all 80 thread-store tests pass, and the Bazel dependency lock remains valid. --- codex-rs/Cargo.toml | 2 +- .../request_processors/thread_processor.rs | 40 +++++++++---------- .../src/request_processors/turn_processor.rs | 2 +- codex-rs/core/src/agent/control/spawn.rs | 4 +- .../src/session/rollout_reconstruction.rs | 2 +- .../session/rollout_reconstruction_tests.rs | 28 +++++++------ codex-rs/core/src/session/tests.rs | 22 +++++----- codex-rs/core/src/thread_manager.rs | 21 +++++----- codex-rs/core/src/thread_manager_tests.rs | 14 +++---- codex-rs/core/tests/suite/fork_thread.rs | 4 +- codex-rs/core/tests/suite/resume_warning.rs | 6 ++- codex-rs/protocol/src/protocol.rs | 11 ++--- codex-rs/rollout/src/recorder.rs | 2 +- codex-rs/thread-store/src/in_memory.rs | 4 +- codex-rs/thread-store/src/live_thread.rs | 8 ++-- .../thread-store/src/thread_metadata_sync.rs | 14 +++++-- codex-rs/thread-store/src/types.rs | 3 +- 17 files changed, 102 insertions(+), 85 deletions(-) diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 9591f78af..3290c4ea9 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -380,7 +380,7 @@ schemars = "0.8.22" seccompiler = "0.5.0" semver = "1.0" sentry = "0.46.0" -serde = "1" +serde = { version = "1", features = ["rc"] } serde_ignored = "0.1.14" serde_json = "1" serde_path_to_error = "0.1.20" 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 a3e4b49ce..a59bb530d 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -2604,8 +2604,8 @@ impl ThreadRequestProcessor { self.resume_thread_from_history(history.as_slice()) .await .map(|thread_history| (thread_history, None)) - } else if let Some(stored_thread) = stored_thread_from_running_probe { - self.stored_thread_to_initial_history(&stored_thread) + } else if let Some(mut stored_thread) = stored_thread_from_running_probe { + self.stored_thread_to_initial_history(&mut stored_thread) .await .map(|thread_history| (thread_history, Some(*stored_thread))) } else { @@ -2758,7 +2758,7 @@ impl ThreadRequestProcessor { let token_usage_thread = include_turns.then(|| thread.clone()); let mut initial_turns_page = if let Some(params) = initial_turns_page.as_ref() { match build_thread_resume_initial_turns_page( - &response_history.get_rollout_items(), + response_history.get_rollout_items(), thread.status.clone(), /*has_live_running_thread*/ false, /*active_turn*/ None, @@ -2803,7 +2803,7 @@ impl ThreadRequestProcessor { // rebuilding history only to attribute a replayed usage update. if let Some(token_usage_thread) = token_usage_thread { let token_usage_turn_id = latest_token_usage_turn_id_from_rollout_items( - &response_history.get_rollout_items(), + response_history.get_rollout_items(), token_usage_thread.turns.as_slice(), ); // The client needs restored usage before it starts another turn. @@ -2901,7 +2901,7 @@ impl ThreadRequestProcessor { } }; - if let Some((existing_thread_id, existing_thread, source_thread)) = running_thread { + if let Some((existing_thread_id, existing_thread, mut source_thread)) = running_thread { let existing_thread_rollout_path = existing_thread.rollout_path(); let active_path = existing_thread_rollout_path .as_ref() @@ -2963,8 +2963,8 @@ impl ThreadRequestProcessor { should_redact_thread_resume_payloads(app_server_client_name.as_deref()); let history_items = source_thread .history - .as_ref() - .map(|history| history.items.clone()) + .take() + .map(|history| history.items) .ok_or_else(|| { internal_error(format!( "thread {existing_thread_id} did not include persisted history" @@ -2988,10 +2988,8 @@ impl ThreadRequestProcessor { ) .await?; - let mut summary_source_thread = source_thread; - summary_source_thread.history = None; let mut thread_summary = self.stored_thread_to_api_thread( - summary_source_thread, + source_thread, config_snapshot.model_provider_id.as_str(), /*include_turns*/ false, ); @@ -3060,11 +3058,11 @@ impl ThreadRequestProcessor { thread_id: &str, path: Option<&PathBuf>, ) -> Result<(InitialHistory, StoredThread), JSONRPCErrorError> { - let stored_thread = self + let mut stored_thread = self .read_stored_thread_for_resume(thread_id, path, /*include_history*/ true) .await?; let history = self - .stored_thread_to_initial_history(&stored_thread) + .stored_thread_to_initial_history(&mut stored_thread) .await?; Ok((history, stored_thread)) } @@ -3112,13 +3110,13 @@ impl ThreadRequestProcessor { #[tracing::instrument(level = "trace", skip_all)] async fn stored_thread_to_initial_history( &self, - stored_thread: &StoredThread, + stored_thread: &mut StoredThread, ) -> Result { let thread_id = stored_thread.thread_id; let history = stored_thread .history - .as_ref() - .map(|history| history.items.clone()) + .take() + .map(|history| history.items) .ok_or_else(|| { internal_error(format!( "thread {thread_id} did not include persisted history" @@ -3126,7 +3124,7 @@ impl ThreadRequestProcessor { })?; Ok(InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history, + history: Arc::new(history), rollout_path: stored_thread.rollout_path.clone(), })) } @@ -3255,7 +3253,7 @@ impl ThreadRequestProcessor { let history_items = thread_history.get_rollout_items(); populate_thread_turns_from_history( &mut thread, - &history_items, + history_items, /*active_turn*/ None, ); } @@ -3313,7 +3311,7 @@ impl ThreadRequestProcessor { "`permissions` cannot be combined with `sandbox`", )); } - let source_thread = self + let mut source_thread = self .read_stored_thread_for_resume(&thread_id, path.as_ref(), /*include_history*/ true) .await?; let source_thread_id = source_thread.thread_id; @@ -3323,8 +3321,8 @@ impl ThreadRequestProcessor { .and_then(codex_core::util::normalize_thread_name); let history_items = source_thread .history - .as_ref() - .map(|history| history.items.clone()) + .take() + .map(|history| Arc::new(history.items)) .ok_or_else(|| { internal_error(format!( "thread {source_thread_id} did not include persisted history" @@ -3391,7 +3389,7 @@ impl ThreadRequestProcessor { config, InitialHistory::Resumed(ResumedHistory { conversation_id: source_thread_id, - history: history_items.clone(), + history: Arc::clone(&history_items), rollout_path: source_thread.rollout_path.clone(), }), thread_source.map(Into::into), diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index 4b6e0fda3..e0d79a0c3 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -1237,7 +1237,7 @@ impl TurnRequestProcessor { config.clone(), InitialHistory::Resumed(ResumedHistory { conversation_id: parent_thread_id, - history: parent_history.items, + history: Arc::new(parent_history.items), rollout_path: parent_thread.rollout_path(), }), /*thread_source*/ None, diff --git a/codex-rs/core/src/agent/control/spawn.rs b/codex-rs/core/src/agent/control/spawn.rs index 37a3a8105..e208b418d 100644 --- a/codex-rs/core/src/agent/control/spawn.rs +++ b/codex-rs/core/src/agent/control/spawn.rs @@ -143,7 +143,7 @@ impl AgentControl { .items; let initial_history = InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history, + history: Arc::new(history), rollout_path: stored_thread.rollout_path, }); if initial_history.get_multi_agent_version() != Some(MultiAgentVersion::V2) { @@ -626,7 +626,7 @@ impl AgentControl { .items; let initial_history = InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history, + history: Arc::new(history), rollout_path: stored_thread.rollout_path, }); let parent_thread_id = stored_thread.parent_thread_id; diff --git a/codex-rs/core/src/session/rollout_reconstruction.rs b/codex-rs/core/src/session/rollout_reconstruction.rs index bf534c1e8..6989ff80d 100644 --- a/codex-rs/core/src/session/rollout_reconstruction.rs +++ b/codex-rs/core/src/session/rollout_reconstruction.rs @@ -355,7 +355,7 @@ impl Session { id: None, }); RolloutReconstruction { - history: history.raw_items().to_vec(), + history: history.into_raw_items(), previous_turn_settings, reference_context_item, window_number: window.number, diff --git a/codex-rs/core/src/session/rollout_reconstruction_tests.rs b/codex-rs/core/src/session/rollout_reconstruction_tests.rs index b28bd37c7..828c07858 100644 --- a/codex-rs/core/src/session/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/session/rollout_reconstruction_tests.rs @@ -69,7 +69,9 @@ async fn record_initial_history_reconstructs_typed_inter_agent_message() { session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: vec![RolloutItem::InterAgentCommunication(communication.clone())], + history: Arc::new(vec![RolloutItem::InterAgentCommunication( + communication.clone(), + )]), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -112,7 +114,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -189,7 +191,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -757,7 +759,7 @@ async fn record_initial_history_resumed_rollback_skips_only_user_turns() { session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -841,7 +843,7 @@ async fn record_initial_history_resumed_rollback_drops_incomplete_user_turn_comp session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -871,7 +873,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_seed_referenc session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -898,7 +900,7 @@ async fn record_initial_history_resumed_does_not_seed_reference_context_item_aft session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -1070,7 +1072,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -1215,7 +1217,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -1338,7 +1340,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -1454,7 +1456,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -1505,7 +1507,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_preserves_turn_ session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -1633,7 +1635,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 52bde2cc4..a76cacabf 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1697,7 +1697,7 @@ async fn record_initial_history_reconstructs_resumed_transcript() { session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -1862,7 +1862,7 @@ async fn prepares_resumed_history_before_installing_it() { session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: vec![RolloutItem::ResponseItem(resumed_item)], + history: Arc::new(vec![RolloutItem::ResponseItem(resumed_item)]), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -1905,7 +1905,7 @@ fn resolve_multi_agent_version_handles_unset_and_legacy_history() { resolve_multi_agent_version( &InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history: Vec::new(), + history: Arc::new(Vec::new()), rollout_path: None, }), /*inherited_multi_agent_version*/ None, @@ -1916,7 +1916,7 @@ fn resolve_multi_agent_version_handles_unset_and_legacy_history() { resolve_multi_agent_version( &InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history: Vec::new(), + history: Arc::new(Vec::new()), rollout_path: None, }), Some(MultiAgentVersion::V2), @@ -1927,10 +1927,10 @@ fn resolve_multi_agent_version_handles_unset_and_legacy_history() { resolve_multi_agent_version( &InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history: vec![session_meta_item( + history: Arc::new(vec![session_meta_item( thread_id, Some(MultiAgentVersion::Disabled) - )], + )]), rollout_path: None, }), Some(MultiAgentVersion::V2), @@ -1991,7 +1991,7 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only() session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -2085,7 +2085,7 @@ async fn record_initial_history_seeds_token_info_from_rollout() { session .record_initial_history(InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: rollout_items, + history: Arc::new(rollout_items), rollout_path: Some(PathBuf::from("/tmp/resume.jsonl")), })) .await; @@ -5508,7 +5508,7 @@ async fn resumed_root_session_uses_thread_id_as_session_id() { let (session, rx_event) = make_session_with_history_source_and_agent_control_and_rx( InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history: Vec::new(), + history: Arc::new(Vec::new()), rollout_path: None, }), SessionSource::Exec, @@ -5543,7 +5543,7 @@ async fn resumed_subagent_session_restores_persisted_session_id() { let (session, rx_event) = make_session_with_history_source_and_agent_control_and_rx( InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history: vec![RolloutItem::SessionMeta(SessionMetaLine { + history: Arc::new(vec![RolloutItem::SessionMeta(SessionMetaLine { meta: SessionMeta { session_id: parent_session_id, id: thread_id, @@ -5551,7 +5551,7 @@ async fn resumed_subagent_session_restores_persisted_session_id() { ..SessionMeta::default() }, git: None, - })], + })]), rollout_path: None, }), session_source, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index deba2864b..816df89c3 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -1602,7 +1602,7 @@ fn stored_thread_to_initial_history( })?; Ok(InitialHistory::Resumed(ResumedHistory { conversation_id: thread_id, - history: history.items, + history: Arc::new(history.items), rollout_path: rollout_path.or(stored_thread.rollout_path), })) } @@ -1639,7 +1639,7 @@ fn truncate_before_nth_user_message( n: usize, snapshot_state: &SnapshotTurnState, ) -> InitialHistory { - let items: Vec = history.get_rollout_items(); + let items = history.get_rollout_items().to_vec(); let user_positions = truncation::user_message_positions_in_rollout(&items); let rolled = if snapshot_state.ends_mid_turn && n >= user_positions.len() { if let Some(cut_idx) = snapshot_state @@ -1671,7 +1671,7 @@ struct SnapshotTurnState { fn snapshot_turn_state(history: &InitialHistory) -> SnapshotTurnState { let rollout_items = history.get_rollout_items(); let mut builder = ThreadHistoryBuilder::new(); - for item in &rollout_items { + for item in rollout_items { builder.handle_rollout_item(item); } let active_turn_id = builder.active_turn_id_if_explicit(); @@ -1695,7 +1695,7 @@ fn snapshot_turn_state(history: &InitialHistory) -> SnapshotTurnState { }; } - let Some(last_user_position) = truncation::user_message_positions_in_rollout(&rollout_items) + let Some(last_user_position) = truncation::user_message_positions_in_rollout(rollout_items) .last() .copied() else { @@ -1736,7 +1736,9 @@ fn fork_history_from_snapshot( InitialHistory::New => InitialHistory::New, InitialHistory::Cleared => InitialHistory::Cleared, InitialHistory::Forked(history) => InitialHistory::Forked(history), - InitialHistory::Resumed(resumed) => InitialHistory::Forked(resumed.history), + InitialHistory::Resumed(resumed) => { + InitialHistory::Forked(Arc::unwrap_or_clone(resumed.history)) + } }; if snapshot_state.ends_mid_turn { append_interrupted_boundary( @@ -1782,12 +1784,13 @@ fn append_interrupted_boundary( history.push(aborted_event); InitialHistory::Forked(history) } - InitialHistory::Resumed(mut resumed) => { + InitialHistory::Resumed(resumed) => { + let mut history = Arc::unwrap_or_clone(resumed.history); if let Some(marker) = interrupted_turn_history_marker(interrupted_marker) { - resumed.history.push(RolloutItem::ResponseItem(marker)); + history.push(RolloutItem::ResponseItem(marker)); } - resumed.history.push(aborted_event); - InitialHistory::Forked(resumed.history) + 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 afb103b88..6375bd914 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -116,7 +116,7 @@ fn truncates_before_requested_user_message() { RolloutItem::ResponseItem(items[2].clone()), ]; assert_eq!( - serde_json::to_value(&got_items).unwrap(), + serde_json::to_value(got_items).unwrap(), serde_json::to_value(&expected_items).unwrap() ); @@ -256,7 +256,7 @@ async fn ignores_session_prefix_messages_when_truncating() { ]; assert_eq!( - serde_json::to_value(&got_items).unwrap(), + serde_json::to_value(got_items).unwrap(), serde_json::to_value(&expected).unwrap() ); } @@ -963,7 +963,7 @@ async fn rollout_path_resume_and_fork_read_history_through_thread_store() { config.clone(), InitialHistory::Resumed(ResumedHistory { conversation_id: source.thread_id, - history: vec![RolloutItem::ResponseItem(user_msg("hello"))], + history: Arc::new(vec![RolloutItem::ResponseItem(user_msg("hello"))]), rollout_path: Some(rollout_path.clone()), }), auth_manager.clone(), @@ -1315,7 +1315,7 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor assert!(!snapshot_turn_state(&history).ends_mid_turn); let rollout_items: Vec<_> = history .get_rollout_items() - .into_iter() + .iter() .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) .collect(); let interrupted_marker_json = serde_json::to_value(RolloutItem::ResponseItem( @@ -1435,7 +1435,7 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { .expect("read forked rollout history"); let rollout_items: Vec<_> = history .get_rollout_items() - .into_iter() + .iter() .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) .collect(); @@ -1522,7 +1522,7 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ let forked_rollout_items: Vec<_> = history .get_rollout_items() - .into_iter() + .iter() .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) .collect(); let interrupted_marker_json = serde_json::to_value(RolloutItem::ResponseItem( @@ -1560,7 +1560,7 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ .expect("read re-forked rollout history"); let reforked_rollout_items: Vec<_> = reforked_history .get_rollout_items() - .into_iter() + .iter() .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) .collect(); diff --git a/codex-rs/core/tests/suite/fork_thread.rs b/codex-rs/core/tests/suite/fork_thread.rs index d51ba7d2f..cc1d4e917 100644 --- a/codex-rs/core/tests/suite/fork_thread.rs +++ b/codex-rs/core/tests/suite/fork_thread.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use codex_core::ForkSnapshot; use codex_core::NewThread; use codex_core::parse_turn_item; @@ -196,7 +198,7 @@ async fn fork_thread_from_history_does_not_require_source_rollout_path() { test.config.clone(), InitialHistory::Resumed(ResumedHistory { conversation_id: test.session_configured.thread_id, - history: source_items.clone(), + history: Arc::new(source_items.clone()), rollout_path: None, }), /*thread_source*/ None, diff --git a/codex-rs/core/tests/suite/resume_warning.rs b/codex-rs/core/tests/suite/resume_warning.rs index 073e6d8b4..50395b2ec 100644 --- a/codex-rs/core/tests/suite/resume_warning.rs +++ b/codex-rs/core/tests/suite/resume_warning.rs @@ -1,5 +1,7 @@ #![allow(clippy::unwrap_used)] +use std::sync::Arc; + use codex_core::NewThread; use codex_login::CodexAuth; use codex_protocol::ThreadId; @@ -51,7 +53,7 @@ fn resume_history( InitialHistory::Resumed(ResumedHistory { conversation_id: ThreadId::default(), - history: vec![ + history: Arc::new(vec![ RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent { turn_id: turn_id.clone(), trace_id: None, @@ -75,7 +77,7 @@ fn resume_history( duration_ms: None, time_to_first_token_ms: None, })), - ], + ]), rollout_path: Some(rollout_path.to_path_buf()), }) } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index e1fe60546..8a678281d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -10,6 +10,7 @@ use std::ops::Mul; use std::path::Path; use std::path::PathBuf; use std::str::FromStr; +use std::sync::Arc; use std::time::Duration; use strum_macros::EnumIter; @@ -2460,7 +2461,7 @@ pub struct ConversationPathResponseEvent { #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct ResumedHistory { pub conversation_id: ThreadId, - pub history: Vec, + pub history: Arc>, pub rollout_path: Option, } @@ -2505,11 +2506,11 @@ impl InitialHistory { } } - pub fn get_rollout_items(&self) -> Vec { + pub fn get_rollout_items(&self) -> &[RolloutItem] { match self { - InitialHistory::New | InitialHistory::Cleared => Vec::new(), - InitialHistory::Resumed(resumed) => resumed.history.clone(), - InitialHistory::Forked(items) => items.clone(), + InitialHistory::New | InitialHistory::Cleared => &[], + InitialHistory::Resumed(resumed) => &resumed.history, + InitialHistory::Forked(items) => items, } } diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index 084acb0d7..3229f5450 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -947,7 +947,7 @@ impl RolloutRecorder { info!("Resumed rollout successfully from {path:?}"); Ok(InitialHistory::Resumed(ResumedHistory { conversation_id, - history: items, + history: Arc::new(items), rollout_path: Some(compression::plain_rollout_path(path)), })) } diff --git a/codex-rs/thread-store/src/in_memory.rs b/codex-rs/thread-store/src/in_memory.rs index a93e3644c..84e0484f7 100644 --- a/codex-rs/thread-store/src/in_memory.rs +++ b/codex-rs/thread-store/src/in_memory.rs @@ -266,7 +266,9 @@ impl InMemoryThreadStore { let mut state = self.state.lock().await; state.calls.resume_thread += 1; if let Some(history) = params.history { - state.histories.insert(params.thread_id, history); + state + .histories + .insert(params.thread_id, Arc::unwrap_or_clone(history)); } else { state.histories.entry(params.thread_id).or_default(); } diff --git a/codex-rs/thread-store/src/live_thread.rs b/codex-rs/thread-store/src/live_thread.rs index 6b9480751..da3d092da 100644 --- a/codex-rs/thread-store/src/live_thread.rs +++ b/codex-rs/thread-store/src/live_thread.rs @@ -100,12 +100,13 @@ impl LiveThread { pub async fn resume( thread_store: Arc, - mut params: ResumeThreadParams, + params: ResumeThreadParams, ) -> ThreadStoreResult { let thread_id = params.thread_id; let should_load_history = params.history.is_none(); let include_archived = params.include_archived; - thread_store.resume_thread(params.clone()).await?; + let mut metadata_sync = ThreadMetadataSync::for_resume(¶ms); + thread_store.resume_thread(params).await?; if should_load_history { match thread_store .load_history(LoadThreadHistoryParams { @@ -114,7 +115,7 @@ impl LiveThread { }) .await { - Ok(history) => params.history = Some(history.items), + Ok(history) => metadata_sync.record_resume_history(&history.items), Err(err) => { if let Err(discard_err) = thread_store.discard_thread(thread_id).await { warn!( @@ -125,7 +126,6 @@ impl LiveThread { } } } - let metadata_sync = ThreadMetadataSync::for_resume(¶ms); Ok(Self { thread_id, thread_store, diff --git a/codex-rs/thread-store/src/thread_metadata_sync.rs b/codex-rs/thread-store/src/thread_metadata_sync.rs index 7940f1c80..852f67912 100644 --- a/codex-rs/thread-store/src/thread_metadata_sync.rs +++ b/codex-rs/thread-store/src/thread_metadata_sync.rs @@ -1,3 +1,5 @@ +#[cfg(test)] +use std::sync::Arc; use std::time::Duration; use std::time::Instant; @@ -108,13 +110,17 @@ impl ThreadMetadataSync { defer_resume_update_until_append: false, }; if let Some(history) = params.history.as_deref() { - let update = sync.observe_resume_history(history); - sync.merge_pending_update(update); - sync.defer_resume_update_until_append = sync.pending_update.is_some(); + sync.record_resume_history(history); } sync } + pub(crate) fn record_resume_history(&mut self, history: &[RolloutItem]) { + let update = self.observe_resume_history(history); + self.merge_pending_update(update); + self.defer_resume_update_until_append = self.pending_update.is_some(); + } + pub(crate) fn take_pending_update(&self) -> Option { self.pending_update .clone() @@ -555,7 +561,7 @@ mod tests { ResumeThreadParams { thread_id, rollout_path: None, - history: Some(history), + history: Some(Arc::new(history)), include_archived: false, metadata: ThreadPersistenceMetadata { cwd: None, diff --git a/codex-rs/thread-store/src/types.rs b/codex-rs/thread-store/src/types.rs index 74412bb9c..2a82e4251 100644 --- a/codex-rs/thread-store/src/types.rs +++ b/codex-rs/thread-store/src/types.rs @@ -1,4 +1,5 @@ use std::path::PathBuf; +use std::sync::Arc; use chrono::DateTime; use chrono::Utc; @@ -96,7 +97,7 @@ pub struct ResumeThreadParams { /// Known local rollout path when the caller resumed from a specific file. pub rollout_path: Option, /// Known replay history for the resumed thread, if already loaded by the caller. - pub history: Option>, + pub history: Option>>, /// Whether archived threads may be reopened. pub include_archived: bool, /// Metadata for future writes appended to the resumed live thread.