mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
fix: preserve imported session source chronology
This commit is contained in:
@@ -22,6 +22,7 @@ use crate::AppendThreadItemsParams;
|
||||
use crate::ArchiveThreadParams;
|
||||
use crate::CreateThreadParams;
|
||||
use crate::DeleteThreadParams;
|
||||
use crate::InitialThreadTimestamps;
|
||||
use crate::ListThreadsParams;
|
||||
use crate::LoadThreadHistoryParams;
|
||||
use crate::ReadThreadByRolloutPathParams;
|
||||
@@ -612,6 +613,14 @@ impl ThreadStore for InMemoryThreadStore {
|
||||
Box::pin(InMemoryThreadStore::create_thread(self, params))
|
||||
}
|
||||
|
||||
fn create_thread_with_initial_timestamps(
|
||||
&self,
|
||||
params: CreateThreadParams,
|
||||
_timestamps: InitialThreadTimestamps,
|
||||
) -> ThreadStoreFuture<'_, ()> {
|
||||
Box::pin(InMemoryThreadStore::create_thread(self, params))
|
||||
}
|
||||
|
||||
fn resume_thread(&self, params: ResumeThreadParams) -> ThreadStoreFuture<'_, ()> {
|
||||
Box::pin(InMemoryThreadStore::resume_thread(self, params))
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ pub use types::CreateThreadParams;
|
||||
pub use types::DeleteThreadParams;
|
||||
pub use types::ExtraConfig;
|
||||
pub use types::GitInfoPatch;
|
||||
pub use types::InitialThreadTimestamps;
|
||||
pub use types::ItemPage;
|
||||
pub use types::ListItemsParams;
|
||||
pub use types::ListThreadsParams;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::LocalThreadStore;
|
||||
use crate::CreateThreadParams;
|
||||
use crate::InitialThreadTimestamps;
|
||||
use crate::ThreadStoreError;
|
||||
use crate::ThreadStoreResult;
|
||||
use crate::error::reject_paginated_history_mode;
|
||||
@@ -11,6 +12,22 @@ use codex_rollout::RolloutRecorderParams;
|
||||
pub(super) async fn create_thread(
|
||||
store: &LocalThreadStore,
|
||||
params: CreateThreadParams,
|
||||
) -> ThreadStoreResult<RolloutRecorder> {
|
||||
create_thread_recorder(store, params, /*initial_timestamps*/ None).await
|
||||
}
|
||||
|
||||
pub(super) async fn create_thread_with_initial_timestamps(
|
||||
store: &LocalThreadStore,
|
||||
params: CreateThreadParams,
|
||||
initial_timestamps: InitialThreadTimestamps,
|
||||
) -> ThreadStoreResult<RolloutRecorder> {
|
||||
create_thread_recorder(store, params, Some(initial_timestamps)).await
|
||||
}
|
||||
|
||||
async fn create_thread_recorder(
|
||||
store: &LocalThreadStore,
|
||||
params: CreateThreadParams,
|
||||
initial_timestamps: Option<InitialThreadTimestamps>,
|
||||
) -> ThreadStoreResult<RolloutRecorder> {
|
||||
reject_paginated_history_mode(params.history_mode)?;
|
||||
let cwd = params
|
||||
@@ -27,26 +44,30 @@ pub(super) async fn create_thread(
|
||||
model_provider_id: params.metadata.model_provider.clone(),
|
||||
generate_memories: matches!(params.metadata.memory_mode, ThreadMemoryMode::Enabled),
|
||||
};
|
||||
RolloutRecorder::new(
|
||||
&config,
|
||||
RolloutRecorderParams::new(
|
||||
params.thread_id,
|
||||
params.forked_from_id,
|
||||
params.parent_thread_id,
|
||||
params.source,
|
||||
params.thread_source,
|
||||
params.originator,
|
||||
params.base_instructions,
|
||||
params.dynamic_tools,
|
||||
)
|
||||
.with_session_id(params.session_id)
|
||||
.with_selected_capability_roots(params.selected_capability_roots)
|
||||
.with_multi_agent_version(params.multi_agent_version)
|
||||
.with_history_mode(params.history_mode)
|
||||
.with_initial_window_id(params.initial_window_id),
|
||||
let recorder_params = RolloutRecorderParams::new(
|
||||
params.thread_id,
|
||||
params.forked_from_id,
|
||||
params.parent_thread_id,
|
||||
params.source,
|
||||
params.thread_source,
|
||||
params.originator,
|
||||
params.base_instructions,
|
||||
params.dynamic_tools,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to initialize local thread recorder: {err}"),
|
||||
})
|
||||
.with_session_id(params.session_id)
|
||||
.with_selected_capability_roots(params.selected_capability_roots)
|
||||
.with_multi_agent_version(params.multi_agent_version)
|
||||
.with_history_mode(params.history_mode)
|
||||
.with_initial_window_id(params.initial_window_id);
|
||||
let recorder_params = match initial_timestamps {
|
||||
Some(timestamps) => {
|
||||
recorder_params.with_initial_timestamps(timestamps.created_at, timestamps.updated_at)
|
||||
}
|
||||
None => recorder_params,
|
||||
};
|
||||
RolloutRecorder::new(&config, recorder_params)
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to initialize local thread recorder: {err}"),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ use super::LocalThreadStore;
|
||||
use super::create_thread;
|
||||
use crate::AppendThreadItemsParams;
|
||||
use crate::CreateThreadParams;
|
||||
use crate::InitialThreadTimestamps;
|
||||
use crate::ReadThreadParams;
|
||||
use crate::ResumeThreadParams;
|
||||
use crate::ThreadStoreError;
|
||||
@@ -34,6 +35,21 @@ pub(super) async fn create_thread(
|
||||
.await
|
||||
}
|
||||
|
||||
pub(super) async fn create_thread_with_initial_timestamps(
|
||||
store: &LocalThreadStore,
|
||||
params: CreateThreadParams,
|
||||
timestamps: InitialThreadTimestamps,
|
||||
) -> ThreadStoreResult<()> {
|
||||
let thread_id = params.thread_id;
|
||||
let history_mode = params.history_mode;
|
||||
store.ensure_live_recorder_absent(thread_id).await?;
|
||||
let recorder =
|
||||
create_thread::create_thread_with_initial_timestamps(store, params, timestamps).await?;
|
||||
store
|
||||
.insert_live_recorder(thread_id, recorder, history_mode)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(super) async fn resume_thread(
|
||||
store: &LocalThreadStore,
|
||||
params: ResumeThreadParams,
|
||||
|
||||
@@ -26,6 +26,7 @@ use crate::AppendThreadItemsParams;
|
||||
use crate::ArchiveThreadParams;
|
||||
use crate::CreateThreadParams;
|
||||
use crate::DeleteThreadParams;
|
||||
use crate::InitialThreadTimestamps;
|
||||
use crate::ListThreadsParams;
|
||||
use crate::LoadThreadHistoryParams;
|
||||
use crate::ReadThreadByRolloutPathParams;
|
||||
@@ -246,6 +247,16 @@ impl ThreadStore for LocalThreadStore {
|
||||
Box::pin(async move { live_writer::create_thread(self, params).await })
|
||||
}
|
||||
|
||||
fn create_thread_with_initial_timestamps(
|
||||
&self,
|
||||
params: CreateThreadParams,
|
||||
timestamps: InitialThreadTimestamps,
|
||||
) -> ThreadStoreFuture<'_, ()> {
|
||||
Box::pin(async move {
|
||||
live_writer::create_thread_with_initial_timestamps(self, params, timestamps).await
|
||||
})
|
||||
}
|
||||
|
||||
fn resume_thread(&self, params: ResumeThreadParams) -> ThreadStoreFuture<'_, ()> {
|
||||
Box::pin(async move { live_writer::resume_thread(self, params).await })
|
||||
}
|
||||
@@ -325,6 +336,8 @@ impl ThreadStore for LocalThreadStore {
|
||||
mod tests {
|
||||
use std::sync::Arc;
|
||||
|
||||
use chrono::DateTime;
|
||||
use chrono::Utc;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
@@ -342,7 +355,9 @@ mod tests {
|
||||
use tempfile::TempDir;
|
||||
|
||||
use super::*;
|
||||
use crate::InitialThreadTimestamps;
|
||||
use crate::LiveThread;
|
||||
use crate::ThreadMetadataPatch;
|
||||
use crate::ThreadPersistenceMetadata;
|
||||
use crate::local::test_support::test_config;
|
||||
use crate::local::test_support::write_archived_session_file;
|
||||
@@ -398,6 +413,123 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn initial_timestamps_survive_read_repair() {
|
||||
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 = LocalThreadStore::new(config.clone(), Some(runtime.clone()));
|
||||
let thread_id = ThreadId::new();
|
||||
let created_at = DateTime::parse_from_rfc3339("2024-01-02T03:04:05Z")
|
||||
.expect("created timestamp")
|
||||
.with_timezone(&Utc);
|
||||
let updated_at = DateTime::parse_from_rfc3339("2024-02-03T04:05:06Z")
|
||||
.expect("updated timestamp")
|
||||
.with_timezone(&Utc);
|
||||
|
||||
store
|
||||
.create_thread_with_initial_timestamps(
|
||||
create_thread_params(thread_id),
|
||||
InitialThreadTimestamps {
|
||||
created_at,
|
||||
updated_at,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("create historical thread");
|
||||
store
|
||||
.append_items(AppendThreadItemsParams {
|
||||
thread_id,
|
||||
items: vec![user_message_item("source history")],
|
||||
})
|
||||
.await
|
||||
.expect("append source history");
|
||||
store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
created_at: Some(created_at),
|
||||
updated_at: Some(updated_at),
|
||||
advance_recency_at: Some(updated_at),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect("seed source chronology");
|
||||
let rollout_path = store
|
||||
.live_rollout_path(thread_id)
|
||||
.await
|
||||
.expect("read rollout path");
|
||||
store
|
||||
.persist_thread(thread_id)
|
||||
.await
|
||||
.expect("persist historical thread");
|
||||
store
|
||||
.shutdown_thread(thread_id)
|
||||
.await
|
||||
.expect("shutdown historical thread");
|
||||
|
||||
let session_meta = codex_rollout::read_session_meta_line(&rollout_path)
|
||||
.await
|
||||
.expect("read session metadata");
|
||||
let rollout_modified_at = std::fs::metadata(&rollout_path)
|
||||
.expect("rollout metadata")
|
||||
.modified()
|
||||
.map(DateTime::<Utc>::from)
|
||||
.expect("rollout modified timestamp");
|
||||
assert_eq!(session_meta.meta.timestamp, "2024-01-02T03:04:05.000Z");
|
||||
assert_eq!(rollout_modified_at, updated_at);
|
||||
let local_day = created_at
|
||||
.with_timezone(&chrono::Local)
|
||||
.format("%Y/%m/%d")
|
||||
.to_string();
|
||||
assert!(rollout_path.starts_with(home.path().join("sessions").join(local_day)));
|
||||
|
||||
assert_eq!(
|
||||
runtime
|
||||
.delete_thread(thread_id)
|
||||
.await
|
||||
.expect("delete sqlite metadata"),
|
||||
1
|
||||
);
|
||||
drop(store);
|
||||
drop(runtime);
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
config.sqlite_home.clone(),
|
||||
config.default_model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should reopen");
|
||||
let store = LocalThreadStore::new(config, Some(runtime.clone()));
|
||||
store
|
||||
.read_thread(ReadThreadParams {
|
||||
thread_id,
|
||||
include_archived: false,
|
||||
include_history: false,
|
||||
})
|
||||
.await
|
||||
.expect("read and repair historical thread");
|
||||
let repaired = runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("read repaired sqlite metadata")
|
||||
.expect("repaired sqlite metadata");
|
||||
assert_eq!(
|
||||
(
|
||||
repaired.created_at,
|
||||
repaired.updated_at,
|
||||
repaired.recency_at,
|
||||
),
|
||||
(created_at, updated_at, updated_at)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn raw_append_items_does_not_update_sqlite_metadata() {
|
||||
// This pins the ThreadStore contract: raw appends are history-only. Callers that need
|
||||
|
||||
@@ -8,6 +8,7 @@ use crate::AppendThreadItemsParams;
|
||||
use crate::ArchiveThreadParams;
|
||||
use crate::CreateThreadParams;
|
||||
use crate::DeleteThreadParams;
|
||||
use crate::InitialThreadTimestamps;
|
||||
use crate::ItemPage;
|
||||
use crate::ListItemsParams;
|
||||
use crate::ListThreadsParams;
|
||||
@@ -45,6 +46,16 @@ pub trait ThreadStore: Any + Send + Sync {
|
||||
/// Creates a new live thread.
|
||||
fn create_thread(&self, params: CreateThreadParams) -> ThreadStoreFuture<'_, ()>;
|
||||
|
||||
/// Creates a thread from pre-existing history using its original chronology.
|
||||
///
|
||||
/// This is a creation-only initialization hook. Normal appends after creation must advance
|
||||
/// update and recency timestamps through the usual live-thread path.
|
||||
fn create_thread_with_initial_timestamps(
|
||||
&self,
|
||||
params: CreateThreadParams,
|
||||
timestamps: InitialThreadTimestamps,
|
||||
) -> ThreadStoreFuture<'_, ()>;
|
||||
|
||||
/// Reopens an existing thread for live appends.
|
||||
fn resume_thread(&self, params: ResumeThreadParams) -> ThreadStoreFuture<'_, ()>;
|
||||
|
||||
|
||||
@@ -60,6 +60,15 @@ pub struct ThreadPersistenceMetadata {
|
||||
pub memory_mode: MemoryMode,
|
||||
}
|
||||
|
||||
/// Source chronology used only while creating a thread from pre-existing history.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub struct InitialThreadTimestamps {
|
||||
/// Original creation time of the imported thread.
|
||||
pub created_at: DateTime<Utc>,
|
||||
/// Original last-activity time of the imported thread.
|
||||
pub updated_at: DateTime<Utc>,
|
||||
}
|
||||
|
||||
/// Extra configuration fields for a thread.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct ExtraConfig {}
|
||||
|
||||
Reference in New Issue
Block a user