mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Preserve paginated history for spawned subagents (#33432)
## What changed - Inherit paginated history mode when spawning or forking a subagent from a paginated parent. - Load the parent's model context when forking and persist it as an inherited rollout prefix, while excluding that prefix from the child's projected turns, items, and metadata. - Record the boundary between inherited context and child history, and reject partially initialized paginated subagent rollouts on resume. - Resume paginated subagents from their stored model context instead of legacy rollout history. ## Testing Added coverage for fresh and forked paginated subagents, compacted-history forks, model-context resume, inherited-prefix projection, metadata isolation, and incomplete-prefix detection. GitOrigin-RevId: e57dc37c29aa4aaaf57b052b991be591d730e5ba
This commit is contained in:
@@ -132,6 +132,7 @@ mod tests {
|
||||
selected_capability_roots: Vec::new(),
|
||||
multi_agent_version: None,
|
||||
history_mode: ThreadHistoryMode::Legacy,
|
||||
subagent_history_start_ordinal: None,
|
||||
initial_window_id: uuid::Uuid::now_v7().to_string(),
|
||||
metadata: ThreadPersistenceMetadata {
|
||||
cwd: None,
|
||||
@@ -325,6 +326,7 @@ mod tests {
|
||||
selected_capability_roots: Vec::new(),
|
||||
multi_agent_version: None,
|
||||
history_mode,
|
||||
subagent_history_start_ordinal: None,
|
||||
initial_window_id: uuid::Uuid::now_v7().to_string(),
|
||||
metadata: thread_metadata(),
|
||||
}
|
||||
@@ -441,6 +443,7 @@ impl InMemoryThreadStore {
|
||||
memory_mode: matches!(params.metadata.memory_mode, ThreadMemoryMode::Disabled)
|
||||
.then_some("disabled".to_string()),
|
||||
history_mode: params.history_mode,
|
||||
subagent_history_start_ordinal: params.subagent_history_start_ordinal,
|
||||
multi_agent_version: params.multi_agent_version,
|
||||
context_window: Some(SessionContextWindow::new(params.initial_window_id.clone())),
|
||||
..SessionMeta::default()
|
||||
|
||||
@@ -21,6 +21,7 @@ use crate::StoredThread;
|
||||
use crate::StoredThreadHistory;
|
||||
use crate::ThreadMetadataPatch;
|
||||
use crate::ThreadStore;
|
||||
use crate::ThreadStoreError;
|
||||
use crate::ThreadStoreResult;
|
||||
use crate::UpdateThreadMetadataParams;
|
||||
use crate::thread_metadata_sync::ThreadMetadataSync;
|
||||
@@ -106,6 +107,42 @@ impl LiveThread {
|
||||
})
|
||||
}
|
||||
|
||||
/// Create a child thread with inherited model context already durable.
|
||||
///
|
||||
/// The boundary belongs in session metadata before the copied prefix is written so history
|
||||
/// projection can distinguish inherited context from the child's own records immediately.
|
||||
pub async fn create_with_inherited_model_context(
|
||||
thread_store: Arc<dyn ThreadStore>,
|
||||
mut params: CreateThreadParams,
|
||||
inherited_model_context: &[RolloutItem],
|
||||
) -> ThreadStoreResult<Self> {
|
||||
let persisted_prefix_item_count =
|
||||
persisted_rollout_items(inherited_model_context, params.history_mode).len();
|
||||
params.subagent_history_start_ordinal = Some(
|
||||
u64::try_from(persisted_prefix_item_count)
|
||||
.map_err(|_| ThreadStoreError::Internal {
|
||||
message: "inherited model context is too large".to_string(),
|
||||
})?
|
||||
.checked_add(1)
|
||||
.ok_or_else(|| ThreadStoreError::Internal {
|
||||
message: "inherited model context is too large".to_string(),
|
||||
})?,
|
||||
);
|
||||
let live_thread = Self::create(thread_store, params).await?;
|
||||
if let Err(err) = live_thread
|
||||
.persist_appended_items(inherited_model_context)
|
||||
.await
|
||||
{
|
||||
if let Err(discard_err) = live_thread.discard().await {
|
||||
warn!(
|
||||
"failed to discard thread persistence after inherited context append failed: {discard_err}"
|
||||
);
|
||||
}
|
||||
return Err(err);
|
||||
}
|
||||
Ok(live_thread)
|
||||
}
|
||||
|
||||
pub async fn resume(
|
||||
thread_store: Arc<dyn ThreadStore>,
|
||||
history_mode: ThreadHistoryMode,
|
||||
@@ -150,27 +187,7 @@ impl LiveThread {
|
||||
fields(item_count = raw_items.len())
|
||||
)]
|
||||
pub async fn append_items(&self, raw_items: &[RolloutItem]) -> ThreadStoreResult<()> {
|
||||
// Empty appends are intentionally ignored rather than represented as zero-sized batches.
|
||||
if raw_items.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
let (items, measurement) = if self.persistence_telemetry.is_enabled() {
|
||||
let (items, measurement) =
|
||||
measure_and_filter_rollout_items(raw_items, self.history_mode);
|
||||
(items, Some(measurement))
|
||||
} else {
|
||||
(persisted_rollout_items(raw_items, self.history_mode), None)
|
||||
};
|
||||
self.thread_store
|
||||
.append_items(AppendThreadItemsParams {
|
||||
thread_id: self.thread_id,
|
||||
items: raw_items.to_vec(),
|
||||
})
|
||||
.await?;
|
||||
if let Some(measurement) = measurement.as_ref() {
|
||||
self.persistence_telemetry
|
||||
.record_batch(raw_items, measurement);
|
||||
}
|
||||
let items = self.persist_appended_items(raw_items).await?;
|
||||
if items.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
@@ -195,6 +212,34 @@ impl LiveThread {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn persist_appended_items(
|
||||
&self,
|
||||
raw_items: &[RolloutItem],
|
||||
) -> ThreadStoreResult<Vec<RolloutItem>> {
|
||||
// Empty appends are intentionally ignored rather than represented as zero-sized batches.
|
||||
if raw_items.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
let (items, measurement) = if self.persistence_telemetry.is_enabled() {
|
||||
let (items, measurement) =
|
||||
measure_and_filter_rollout_items(raw_items, self.history_mode);
|
||||
(items, Some(measurement))
|
||||
} else {
|
||||
(persisted_rollout_items(raw_items, self.history_mode), None)
|
||||
};
|
||||
self.thread_store
|
||||
.append_items(AppendThreadItemsParams {
|
||||
thread_id: self.thread_id,
|
||||
items: raw_items.to_vec(),
|
||||
})
|
||||
.await?;
|
||||
if let Some(measurement) = measurement.as_ref() {
|
||||
self.persistence_telemetry
|
||||
.record_batch(raw_items, measurement);
|
||||
}
|
||||
Ok(items)
|
||||
}
|
||||
|
||||
pub async fn persist(&self) -> ThreadStoreResult<()> {
|
||||
self.thread_store.persist_thread(self.thread_id).await?;
|
||||
self.flush_pending_metadata_update().await
|
||||
|
||||
@@ -41,6 +41,7 @@ pub(super) async fn create_thread(
|
||||
.with_selected_capability_roots(params.selected_capability_roots)
|
||||
.with_multi_agent_version(params.multi_agent_version)
|
||||
.with_history_mode(params.history_mode)
|
||||
.with_subagent_history_start_ordinal(params.subagent_history_start_ordinal)
|
||||
.with_initial_window_id(params.initial_window_id),
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -388,6 +388,7 @@ mod tests {
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::items::UserMessageItem;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
@@ -395,13 +396,16 @@ mod tests {
|
||||
use codex_protocol::models::MessagePhase;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AgentMessageEvent;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ItemCompletedEvent;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::ThreadHistoryMode;
|
||||
use codex_protocol::protocol::ThreadMemoryMode;
|
||||
use codex_protocol::protocol::TurnCompleteEvent;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use tempfile::TempDir;
|
||||
@@ -535,6 +539,74 @@ mod tests {
|
||||
assert_eq!(metadata.title, "observed append");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn live_thread_does_not_derive_metadata_from_inherited_items() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
config.sqlite_home.clone(),
|
||||
config.default_model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
let store = Arc::new(LocalThreadStore::new(config, Some(runtime.clone())));
|
||||
let thread_id = ThreadId::default();
|
||||
let mut params = create_thread_params(thread_id);
|
||||
params.history_mode = ThreadHistoryMode::Paginated;
|
||||
let cwd = std::env::current_dir().expect("current directory");
|
||||
let turn_context = |model: &str, approval_policy| {
|
||||
RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some("turn-1".to_string()),
|
||||
cwd: serde_json::from_value(serde_json::json!(cwd)).expect("absolute cwd"),
|
||||
workspace_roots: None,
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
approval_policy,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
permission_profile: None,
|
||||
network: None,
|
||||
file_system_sandbox_policy: None,
|
||||
model: model.to_string(),
|
||||
comp_hash: None,
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
multi_agent_version: None,
|
||||
multi_agent_mode: None,
|
||||
realtime_active: None,
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
})
|
||||
};
|
||||
|
||||
let live_thread = LiveThread::create_with_inherited_model_context(
|
||||
store,
|
||||
params,
|
||||
&[turn_context("parent-model", AskForApproval::Never)],
|
||||
)
|
||||
.await
|
||||
.expect("create live thread with inherited context");
|
||||
live_thread.persist().await.expect("persist thread");
|
||||
let inherited_metadata = runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("sqlite metadata read")
|
||||
.expect("sqlite metadata");
|
||||
assert_eq!(inherited_metadata.model, None);
|
||||
|
||||
live_thread
|
||||
.append_items(&[turn_context("child-model", AskForApproval::OnRequest)])
|
||||
.await
|
||||
.expect("append child context");
|
||||
let child_metadata = runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("sqlite metadata read")
|
||||
.expect("sqlite metadata");
|
||||
assert_eq!(child_metadata.model.as_deref(), Some("child-model"));
|
||||
assert_eq!(child_metadata.approval_mode, "on-request");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn live_thread_output_advances_updated_at_but_not_recency_at() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
@@ -1393,6 +1465,7 @@ mod tests {
|
||||
selected_capability_roots: Vec::new(),
|
||||
multi_agent_version: None,
|
||||
history_mode: ThreadHistoryMode::Legacy,
|
||||
subagent_history_start_ordinal: None,
|
||||
initial_window_id: uuid::Uuid::now_v7().to_string(),
|
||||
metadata: thread_metadata(),
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::io::SeekFrom;
|
||||
use std::path::Path;
|
||||
|
||||
use chrono::DateTime;
|
||||
use codex_app_server_protocol::ThreadHistoryChangeSet;
|
||||
use codex_app_server_protocol::project_rollout_line;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
@@ -23,6 +24,11 @@ pub(super) async fn materialize_to_sqlite(
|
||||
if lines.is_empty() && start_offset == next_offset {
|
||||
return Ok(());
|
||||
}
|
||||
let subagent_history_start_ordinal = codex_rollout::read_session_meta_line(rollout_path)
|
||||
.await
|
||||
.map_err(thread_store_io_error)?
|
||||
.meta
|
||||
.subagent_history_start_ordinal;
|
||||
|
||||
let projections = lines
|
||||
.iter()
|
||||
@@ -30,7 +36,14 @@ pub(super) async fn materialize_to_sqlite(
|
||||
let created_at_ms = DateTime::parse_from_rfc3339(line.timestamp.as_str())
|
||||
.map(|timestamp| timestamp.timestamp_millis())
|
||||
.map_err(thread_history_error)?;
|
||||
Ok((line.ordinal, created_at_ms, project_rollout_line(line)))
|
||||
let changes = if line.ordinal.is_some_and(|ordinal| {
|
||||
subagent_history_start_ordinal.is_some_and(|start| ordinal < start)
|
||||
}) {
|
||||
ThreadHistoryChangeSet::default()
|
||||
} else {
|
||||
project_rollout_line(line)
|
||||
};
|
||||
Ok((line.ordinal, created_at_ms, changes))
|
||||
})
|
||||
.collect::<ThreadStoreResult<Vec<_>>>()?;
|
||||
super::thread_history::apply_projection(
|
||||
|
||||
@@ -156,6 +156,75 @@ WHERE thread_id = ?
|
||||
assert_eq!(projection_state, (rollout_len, 5));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn subagent_prefix_advances_projection_without_materializing_history() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let store = LocalThreadStore::new(test_config(home.path()), /*state_db*/ None);
|
||||
let thread_id = ThreadId::default();
|
||||
create_paginated_subagent_thread(
|
||||
&store,
|
||||
thread_id,
|
||||
/*subagent_history_start_ordinal*/ Some(4),
|
||||
)
|
||||
.await;
|
||||
store
|
||||
.persist_thread(thread_id)
|
||||
.await
|
||||
.expect("persist session metadata");
|
||||
|
||||
store
|
||||
.append_items(AppendThreadItemsParams {
|
||||
thread_id,
|
||||
items: vec![
|
||||
turn_started("parent-turn"),
|
||||
completed_item(
|
||||
thread_id,
|
||||
"parent-turn",
|
||||
TurnItem::UserMessage(UserMessageItem {
|
||||
id: "parent-user".to_string(),
|
||||
client_id: None,
|
||||
content: Vec::new(),
|
||||
}),
|
||||
),
|
||||
turn_completed("parent-turn"),
|
||||
turn_started("child-turn"),
|
||||
completed_item(
|
||||
thread_id,
|
||||
"child-turn",
|
||||
TurnItem::UserMessage(UserMessageItem {
|
||||
id: "child-user".to_string(),
|
||||
client_id: None,
|
||||
content: Vec::new(),
|
||||
}),
|
||||
),
|
||||
turn_completed("child-turn"),
|
||||
],
|
||||
})
|
||||
.await
|
||||
.expect("append inherited prefix and child history");
|
||||
|
||||
let pool = codex_state::open_thread_history_db(home.path())
|
||||
.await
|
||||
.expect("open thread history db");
|
||||
let turns = sqlx::query_as::<_, (String, i64)>(
|
||||
"SELECT turn_id, rollout_ordinal FROM thread_turns WHERE thread_id = ?",
|
||||
)
|
||||
.bind(thread_id.to_string())
|
||||
.fetch_all(&pool)
|
||||
.await
|
||||
.expect("read projected turns");
|
||||
assert_eq!(turns, vec![("child-turn".to_string(), 4)]);
|
||||
let items = sqlx::query_as::<_, (String, i64)>(
|
||||
"SELECT item_id, rollout_ordinal FROM thread_items WHERE thread_id = ?",
|
||||
)
|
||||
.bind(thread_id.to_string())
|
||||
.fetch_all(&pool)
|
||||
.await
|
||||
.expect("read projected items");
|
||||
assert_eq!(items, vec![("child-user".to_string(), 5)]);
|
||||
assert_eq!(projection_state(&pool, thread_id).await.1, 7);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_item_snapshot_updates_content_without_reordering() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
@@ -779,6 +848,17 @@ SELECT
|
||||
}
|
||||
|
||||
async fn create_paginated_thread(store: &LocalThreadStore, thread_id: ThreadId) {
|
||||
create_paginated_subagent_thread(
|
||||
store, thread_id, /*subagent_history_start_ordinal*/ None,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn create_paginated_subagent_thread(
|
||||
store: &LocalThreadStore,
|
||||
thread_id: ThreadId,
|
||||
subagent_history_start_ordinal: Option<u64>,
|
||||
) {
|
||||
store
|
||||
.create_thread(CreateThreadParams {
|
||||
session_id: thread_id.into(),
|
||||
@@ -794,6 +874,7 @@ async fn create_paginated_thread(store: &LocalThreadStore, thread_id: ThreadId)
|
||||
selected_capability_roots: Vec::new(),
|
||||
multi_agent_version: None,
|
||||
history_mode: ThreadHistoryMode::Paginated,
|
||||
subagent_history_start_ordinal,
|
||||
initial_window_id: "window-1".to_string(),
|
||||
metadata: ThreadPersistenceMetadata {
|
||||
cwd: Some(std::env::current_dir().expect("cwd")),
|
||||
|
||||
@@ -95,6 +95,8 @@ pub struct CreateThreadParams {
|
||||
pub multi_agent_version: Option<MultiAgentVersion>,
|
||||
/// Persisted thread history contract selected when the thread was created.
|
||||
pub history_mode: ThreadHistoryMode,
|
||||
/// First rollout ordinal that belongs to this subagent's projected history.
|
||||
pub subagent_history_start_ordinal: Option<u64>,
|
||||
/// Initial context-window identity captured when the thread was created.
|
||||
pub initial_window_id: String,
|
||||
/// Metadata captured for the newly created thread.
|
||||
|
||||
Reference in New Issue
Block a user