mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
codex: move unloaded thread writes into store (#18361)
- Migrates unloaded `thread/name/set` and `thread/memoryModeSet` app-server writes behind the generic `ThreadStore::update_thread_metadata` API rather than adding one-off store methods for setting thread name or memory mode. - Implements the local ThreadStore metadata patch path for thread name and memory mode, including rollout append, legacy name index updates, SessionMeta validation/update, SQLite reconciliation, and re-reading the stored thread. - Adds focused local thread-store unit coverage plus app-server integration coverage for the migrated unloaded write paths.
This commit is contained in:
@@ -222,7 +222,6 @@ use codex_core::SessionMeta;
|
||||
use codex_core::SteerInputError;
|
||||
use codex_core::ThreadConfigSnapshot;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::append_thread_name;
|
||||
use codex_core::clear_memory_roots_contents;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
@@ -322,13 +321,11 @@ use codex_protocol::protocol::ReviewTarget as CoreReviewTarget;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::SessionConfiguredEvent;
|
||||
use codex_protocol::protocol::SessionMetaLine;
|
||||
use codex_protocol::protocol::ThreadNameUpdatedEvent;
|
||||
use codex_protocol::protocol::USER_MESSAGE_BEGIN;
|
||||
use codex_protocol::protocol::W3cTraceContext;
|
||||
use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS;
|
||||
use codex_protocol::user_input::UserInput as CoreInputItem;
|
||||
use codex_rmcp_client::perform_oauth_login_return_url;
|
||||
use codex_rollout::append_rollout_item_to_path;
|
||||
use codex_rollout::state_db::StateDbHandle;
|
||||
use codex_rollout::state_db::get_state_db;
|
||||
use codex_rollout::state_db::reconcile_rollout;
|
||||
@@ -342,9 +339,11 @@ use codex_thread_store::LocalThreadStore;
|
||||
use codex_thread_store::ReadThreadParams as StoreReadThreadParams;
|
||||
use codex_thread_store::SortDirection as StoreSortDirection;
|
||||
use codex_thread_store::StoredThread;
|
||||
use codex_thread_store::ThreadMetadataPatch as StoreThreadMetadataPatch;
|
||||
use codex_thread_store::ThreadSortKey as StoreThreadSortKey;
|
||||
use codex_thread_store::ThreadStore;
|
||||
use codex_thread_store::ThreadStoreError;
|
||||
use codex_thread_store::UpdateThreadMetadataParams as StoreUpdateThreadMetadataParams;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_json_to_toml::json_to_toml;
|
||||
use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP;
|
||||
@@ -3000,54 +2999,23 @@ impl CodexMessageProcessor {
|
||||
return;
|
||||
}
|
||||
|
||||
let rollout_path =
|
||||
match find_thread_path_by_id_str(&self.config.codex_home, &thread_id.to_string()).await
|
||||
{
|
||||
Ok(Some(path)) => Some(path),
|
||||
Ok(None) => None,
|
||||
Err(err) => {
|
||||
self.send_invalid_request_error(
|
||||
request_id,
|
||||
format!("failed to locate thread id {thread_id}: {err}"),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let Some(rollout_path) = rollout_path else {
|
||||
self.send_invalid_request_error(request_id, format!("thread not found: {thread_id}"))
|
||||
.await;
|
||||
return;
|
||||
};
|
||||
|
||||
let msg = EventMsg::ThreadNameUpdated(ThreadNameUpdatedEvent {
|
||||
thread_id,
|
||||
thread_name: Some(name.clone()),
|
||||
});
|
||||
let item = RolloutItem::EventMsg(msg);
|
||||
if let Err(err) = append_rollout_item_to_path(rollout_path.as_path(), &item).await {
|
||||
self.send_internal_error(request_id, format!("failed to set thread name: {err}"))
|
||||
if let Err(err) = self
|
||||
.thread_store
|
||||
.update_thread_metadata(StoreUpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: StoreThreadMetadataPatch {
|
||||
name: Some(name.clone()),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
{
|
||||
self.outgoing
|
||||
.send_error(request_id, thread_store_write_error("set thread name", err))
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
if let Err(err) = append_thread_name(&self.config.codex_home, thread_id, &name).await {
|
||||
self.send_internal_error(request_id, format!("failed to index thread name: {err}"))
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
|
||||
let state_db_ctx = open_state_db_for_direct_thread_lookup(&self.config).await;
|
||||
reconcile_rollout(
|
||||
state_db_ctx.as_deref(),
|
||||
rollout_path.as_path(),
|
||||
self.config.model_provider_id.as_str(),
|
||||
/*builder*/ None,
|
||||
&[],
|
||||
/*archived_only*/ None,
|
||||
/*new_thread_memory_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
self.outgoing
|
||||
.send_response(request_id, ThreadSetNameResponse {})
|
||||
@@ -3101,72 +3069,26 @@ impl CodexMessageProcessor {
|
||||
return;
|
||||
}
|
||||
|
||||
let rollout_path =
|
||||
match find_thread_path_by_id_str(&self.config.codex_home, &thread_id.to_string()).await
|
||||
{
|
||||
Ok(Some(path)) => Some(path),
|
||||
Ok(None) => None,
|
||||
Err(err) => {
|
||||
self.send_invalid_request_error(
|
||||
request_id,
|
||||
format!("failed to locate thread id {thread_id}: {err}"),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let Some(rollout_path) = rollout_path else {
|
||||
self.send_invalid_request_error(request_id, format!("thread not found: {thread_id}"))
|
||||
.await;
|
||||
return;
|
||||
};
|
||||
|
||||
let mut session_meta = match read_session_meta_line(rollout_path.as_path()).await {
|
||||
Ok(session_meta) => session_meta,
|
||||
Err(err) => {
|
||||
self.send_internal_error(
|
||||
if let Err(err) = self
|
||||
.thread_store
|
||||
.update_thread_metadata(StoreUpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: StoreThreadMetadataPatch {
|
||||
memory_mode: Some(mode.to_core()),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
{
|
||||
self.outgoing
|
||||
.send_error(
|
||||
request_id,
|
||||
format!("failed to set thread memory mode: {err}"),
|
||||
thread_store_write_error("set thread memory mode", err),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
};
|
||||
if session_meta.meta.id != thread_id {
|
||||
self.send_internal_error(
|
||||
request_id,
|
||||
format!(
|
||||
"failed to set thread memory mode: rollout session metadata id mismatch: expected {thread_id}, found {}",
|
||||
session_meta.meta.id
|
||||
),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
session_meta.meta.memory_mode = Some(mode.as_str().to_string());
|
||||
let item = RolloutItem::SessionMeta(session_meta);
|
||||
|
||||
if let Err(err) = append_rollout_item_to_path(rollout_path.as_path(), &item).await {
|
||||
self.send_internal_error(
|
||||
request_id,
|
||||
format!("failed to set thread memory mode: {err}"),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
|
||||
let state_db_ctx = open_state_db_for_direct_thread_lookup(&self.config).await;
|
||||
reconcile_rollout(
|
||||
state_db_ctx.as_deref(),
|
||||
rollout_path.as_path(),
|
||||
self.config.model_provider_id.as_str(),
|
||||
/*builder*/ None,
|
||||
&[],
|
||||
/*archived_only*/ None,
|
||||
/*new_thread_memory_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
self.outgoing
|
||||
.send_response(request_id, ThreadMemoryModeSetResponse {})
|
||||
@@ -9609,6 +9531,26 @@ fn thread_store_list_error(err: ThreadStoreError) -> JSONRPCErrorError {
|
||||
}
|
||||
}
|
||||
|
||||
fn thread_store_write_error(operation: &str, err: ThreadStoreError) -> JSONRPCErrorError {
|
||||
match err {
|
||||
ThreadStoreError::ThreadNotFound { thread_id } => JSONRPCErrorError {
|
||||
code: INVALID_REQUEST_ERROR_CODE,
|
||||
message: format!("thread not found: {thread_id}"),
|
||||
data: None,
|
||||
},
|
||||
ThreadStoreError::InvalidRequest { message } => JSONRPCErrorError {
|
||||
code: INVALID_REQUEST_ERROR_CODE,
|
||||
message,
|
||||
data: None,
|
||||
},
|
||||
err => JSONRPCErrorError {
|
||||
code: INTERNAL_ERROR_CODE,
|
||||
message: format!("failed to {operation}: {err}"),
|
||||
data: None,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn thread_from_stored_thread(
|
||||
thread: StoredThread,
|
||||
fallback_provider: &str,
|
||||
|
||||
@@ -26,7 +26,6 @@ pub use types::LoadThreadHistoryParams;
|
||||
pub use types::OptionalStringPatch;
|
||||
pub use types::ReadThreadParams;
|
||||
pub use types::ResumeThreadRecorderParams;
|
||||
pub use types::SetThreadNameParams;
|
||||
pub use types::SortDirection;
|
||||
pub use types::StoredThread;
|
||||
pub use types::StoredThreadHistory;
|
||||
|
||||
@@ -3,6 +3,7 @@ mod helpers;
|
||||
mod list_threads;
|
||||
mod read_thread;
|
||||
mod unarchive_thread;
|
||||
mod update_thread_metadata;
|
||||
|
||||
#[cfg(test)]
|
||||
mod test_support;
|
||||
@@ -17,7 +18,6 @@ use crate::ListThreadsParams;
|
||||
use crate::LoadThreadHistoryParams;
|
||||
use crate::ReadThreadParams;
|
||||
use crate::ResumeThreadRecorderParams;
|
||||
use crate::SetThreadNameParams;
|
||||
use crate::StoredThread;
|
||||
use crate::StoredThreadHistory;
|
||||
use crate::ThreadPage;
|
||||
@@ -75,15 +75,11 @@ impl ThreadStore for LocalThreadStore {
|
||||
list_threads::list_threads(self, params).await
|
||||
}
|
||||
|
||||
async fn set_thread_name(&self, _params: SetThreadNameParams) -> ThreadStoreResult<()> {
|
||||
unsupported("set_thread_name")
|
||||
}
|
||||
|
||||
async fn update_thread_metadata(
|
||||
&self,
|
||||
_params: UpdateThreadMetadataParams,
|
||||
params: UpdateThreadMetadataParams,
|
||||
) -> ThreadStoreResult<StoredThread> {
|
||||
unsupported("update_thread_metadata")
|
||||
update_thread_metadata::update_thread_metadata(self, params).await
|
||||
}
|
||||
|
||||
async fn archive_thread(&self, params: ArchiveThreadParams) -> ThreadStoreResult<()> {
|
||||
|
||||
@@ -0,0 +1,397 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::ThreadMemoryMode;
|
||||
use codex_protocol::protocol::ThreadNameUpdatedEvent;
|
||||
use codex_rollout::StateDbHandle;
|
||||
use codex_rollout::append_rollout_item_to_path;
|
||||
use codex_rollout::append_thread_name;
|
||||
use codex_rollout::find_archived_thread_path_by_id_str;
|
||||
use codex_rollout::find_thread_path_by_id_str;
|
||||
use codex_rollout::read_session_meta_line;
|
||||
|
||||
use super::LocalThreadStore;
|
||||
use crate::ReadThreadParams;
|
||||
use crate::StoredThread;
|
||||
use crate::ThreadStoreError;
|
||||
use crate::ThreadStoreResult;
|
||||
use crate::UpdateThreadMetadataParams;
|
||||
use crate::local::read_thread;
|
||||
|
||||
struct ResolvedRolloutPath {
|
||||
path: PathBuf,
|
||||
archived: bool,
|
||||
}
|
||||
|
||||
pub(super) async fn update_thread_metadata(
|
||||
store: &LocalThreadStore,
|
||||
params: UpdateThreadMetadataParams,
|
||||
) -> ThreadStoreResult<StoredThread> {
|
||||
if params.patch.git_info.is_some() {
|
||||
return Err(ThreadStoreError::Internal {
|
||||
message: "local thread store does not implement git metadata updates in this slice"
|
||||
.to_string(),
|
||||
});
|
||||
}
|
||||
if params.patch.name.is_some() && params.patch.memory_mode.is_some() {
|
||||
return Err(ThreadStoreError::InvalidRequest {
|
||||
message: "local thread store applies one metadata field per patch in this slice"
|
||||
.to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
let thread_id = params.thread_id;
|
||||
let resolved_rollout_path =
|
||||
resolve_rollout_path(store, thread_id, params.include_archived).await?;
|
||||
if let Some(name) = params.patch.name {
|
||||
apply_thread_name(store, resolved_rollout_path.path.as_path(), thread_id, name).await?;
|
||||
}
|
||||
if let Some(memory_mode) = params.patch.memory_mode {
|
||||
apply_thread_memory_mode(resolved_rollout_path.path.as_path(), thread_id, memory_mode)
|
||||
.await?;
|
||||
}
|
||||
|
||||
let state_db_ctx = open_state_db_for_direct_thread_lookup(store).await;
|
||||
codex_rollout::state_db::reconcile_rollout(
|
||||
state_db_ctx.as_deref(),
|
||||
resolved_rollout_path.path.as_path(),
|
||||
store.config.model_provider_id.as_str(),
|
||||
/*builder*/ None,
|
||||
&[],
|
||||
/*archived_only*/ resolved_rollout_path.archived.then_some(true),
|
||||
/*new_thread_memory_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
read_thread::read_thread(
|
||||
store,
|
||||
ReadThreadParams {
|
||||
thread_id,
|
||||
include_archived: params.include_archived,
|
||||
include_history: false,
|
||||
},
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn apply_thread_name(
|
||||
store: &LocalThreadStore,
|
||||
rollout_path: &std::path::Path,
|
||||
thread_id: ThreadId,
|
||||
name: String,
|
||||
) -> ThreadStoreResult<()> {
|
||||
let item = RolloutItem::EventMsg(EventMsg::ThreadNameUpdated(ThreadNameUpdatedEvent {
|
||||
thread_id,
|
||||
thread_name: Some(name.clone()),
|
||||
}));
|
||||
|
||||
append_rollout_item_to_path(rollout_path, &item)
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to set thread name: {err}"),
|
||||
})?;
|
||||
append_thread_name(store.config.codex_home.as_path(), thread_id, &name)
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to index thread name: {err}"),
|
||||
})
|
||||
}
|
||||
|
||||
async fn apply_thread_memory_mode(
|
||||
rollout_path: &std::path::Path,
|
||||
thread_id: ThreadId,
|
||||
memory_mode: ThreadMemoryMode,
|
||||
) -> ThreadStoreResult<()> {
|
||||
let mut session_meta =
|
||||
read_session_meta_line(rollout_path)
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to set thread memory mode: {err}"),
|
||||
})?;
|
||||
if session_meta.meta.id != thread_id {
|
||||
return Err(ThreadStoreError::Internal {
|
||||
message: format!(
|
||||
"failed to set thread memory mode: rollout session metadata id mismatch: expected {thread_id}, found {}",
|
||||
session_meta.meta.id
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
session_meta.meta.memory_mode = Some(memory_mode_as_str(memory_mode).to_string());
|
||||
append_rollout_item_to_path(rollout_path, &RolloutItem::SessionMeta(session_meta))
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to set thread memory mode: {err}"),
|
||||
})
|
||||
}
|
||||
|
||||
async fn open_state_db_for_direct_thread_lookup(store: &LocalThreadStore) -> Option<StateDbHandle> {
|
||||
codex_state::StateRuntime::init(
|
||||
store.config.sqlite_home.clone(),
|
||||
store.config.model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.ok()
|
||||
}
|
||||
|
||||
fn memory_mode_as_str(mode: ThreadMemoryMode) -> &'static str {
|
||||
match mode {
|
||||
ThreadMemoryMode::Enabled => "enabled",
|
||||
ThreadMemoryMode::Disabled => "disabled",
|
||||
}
|
||||
}
|
||||
|
||||
async fn resolve_rollout_path(
|
||||
store: &LocalThreadStore,
|
||||
thread_id: ThreadId,
|
||||
include_archived: bool,
|
||||
) -> ThreadStoreResult<ResolvedRolloutPath> {
|
||||
let active_path =
|
||||
find_thread_path_by_id_str(store.config.codex_home.as_path(), &thread_id.to_string())
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::InvalidRequest {
|
||||
message: format!("failed to locate thread id {thread_id}: {err}"),
|
||||
})?;
|
||||
if let Some(path) = active_path {
|
||||
return Ok(ResolvedRolloutPath {
|
||||
path,
|
||||
archived: false,
|
||||
});
|
||||
}
|
||||
if !include_archived {
|
||||
return Err(ThreadStoreError::InvalidRequest {
|
||||
message: format!("thread not found: {thread_id}"),
|
||||
});
|
||||
}
|
||||
find_archived_thread_path_by_id_str(store.config.codex_home.as_path(), &thread_id.to_string())
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::InvalidRequest {
|
||||
message: format!("failed to locate archived thread id {thread_id}: {err}"),
|
||||
})?
|
||||
.map(|path| ResolvedRolloutPath {
|
||||
path,
|
||||
archived: true,
|
||||
})
|
||||
.ok_or_else(|| ThreadStoreError::InvalidRequest {
|
||||
message: format!("thread not found: {thread_id}"),
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use tempfile::TempDir;
|
||||
use uuid::Uuid;
|
||||
|
||||
use super::*;
|
||||
use crate::ThreadMetadataPatch;
|
||||
use crate::ThreadStore;
|
||||
use crate::local::LocalThreadStore;
|
||||
use crate::local::test_support::test_config;
|
||||
use crate::local::test_support::write_archived_session_file;
|
||||
use crate::local::test_support::write_session_file;
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_metadata_sets_name_on_active_rollout_and_indexes_name() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let store = LocalThreadStore::new(test_config(home.path()));
|
||||
let uuid = Uuid::from_u128(301);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
let path =
|
||||
write_session_file(home.path(), "2025-01-03T14-00-00", uuid).expect("session file");
|
||||
|
||||
let thread = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
name: Some("A sharper name".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect("set thread name");
|
||||
|
||||
assert_eq!(thread.name.as_deref(), Some("A sharper name"));
|
||||
let latest_name = codex_rollout::find_thread_name_by_id(home.path(), &thread_id)
|
||||
.await
|
||||
.expect("find thread name");
|
||||
assert_eq!(latest_name.as_deref(), Some("A sharper name"));
|
||||
|
||||
let appended = last_rollout_item(path.as_path());
|
||||
assert_eq!(appended["type"], "event_msg");
|
||||
assert_eq!(appended["payload"]["type"], "thread_name_updated");
|
||||
assert_eq!(appended["payload"]["thread_id"], thread_id.to_string());
|
||||
assert_eq!(appended["payload"]["thread_name"], "A sharper name");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_metadata_sets_memory_mode_on_active_rollout() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let store = LocalThreadStore::new(test_config(home.path()));
|
||||
let uuid = Uuid::from_u128(302);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
let path =
|
||||
write_session_file(home.path(), "2025-01-03T14-30-00", uuid).expect("session file");
|
||||
|
||||
let thread = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
memory_mode: Some(ThreadMemoryMode::Disabled),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect("set thread memory mode");
|
||||
|
||||
assert_eq!(thread.thread_id, thread_id);
|
||||
let appended = last_rollout_item(path.as_path());
|
||||
assert_eq!(appended["type"], "session_meta");
|
||||
assert_eq!(appended["payload"]["id"], thread_id.to_string());
|
||||
assert_eq!(appended["payload"]["memory_mode"], "disabled");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_metadata_rejects_mismatched_session_meta_id() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let store = LocalThreadStore::new(test_config(home.path()));
|
||||
let filename_uuid = Uuid::from_u128(303);
|
||||
let metadata_uuid = Uuid::from_u128(304);
|
||||
let thread_id = ThreadId::from_string(&filename_uuid.to_string()).expect("valid thread id");
|
||||
let path = write_session_file(home.path(), "2025-01-03T15-00-00", filename_uuid)
|
||||
.expect("session file");
|
||||
let content = std::fs::read_to_string(&path).expect("read rollout");
|
||||
std::fs::write(
|
||||
&path,
|
||||
content.replace(&filename_uuid.to_string(), &metadata_uuid.to_string()),
|
||||
)
|
||||
.expect("rewrite rollout");
|
||||
|
||||
let err = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
memory_mode: Some(ThreadMemoryMode::Enabled),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect_err("mismatch should fail");
|
||||
|
||||
assert!(matches!(err, ThreadStoreError::Internal { .. }));
|
||||
assert!(err.to_string().contains("metadata id mismatch"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_metadata_rejects_multi_field_patch_without_partial_write() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let store = LocalThreadStore::new(test_config(home.path()));
|
||||
let uuid = Uuid::from_u128(305);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
let path =
|
||||
write_session_file(home.path(), "2025-01-03T15-30-00", uuid).expect("session file");
|
||||
let original = std::fs::read_to_string(&path).expect("read rollout");
|
||||
|
||||
let err = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
name: Some("Should not persist".to_string()),
|
||||
memory_mode: Some(ThreadMemoryMode::Disabled),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect_err("multi-field patch should fail");
|
||||
|
||||
assert!(matches!(err, ThreadStoreError::InvalidRequest { .. }));
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&path).expect("read rollout"),
|
||||
original
|
||||
);
|
||||
let latest_name = codex_rollout::find_thread_name_by_id(home.path(), &thread_id)
|
||||
.await
|
||||
.expect("find thread name");
|
||||
assert_eq!(latest_name, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_metadata_keeps_archived_thread_archived_in_sqlite() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let store = LocalThreadStore::new(config.clone());
|
||||
let uuid = Uuid::from_u128(306);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
let archived_path = write_archived_session_file(home.path(), "2025-01-03T16-00-00", uuid)
|
||||
.expect("archived session file");
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
home.path().to_path_buf(),
|
||||
config.model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
runtime
|
||||
.mark_backfill_complete(/*last_watermark*/ None)
|
||||
.await
|
||||
.expect("backfill should be complete");
|
||||
codex_rollout::state_db::reconcile_rollout(
|
||||
Some(runtime.as_ref()),
|
||||
archived_path.as_path(),
|
||||
config.model_provider_id.as_str(),
|
||||
/*builder*/ None,
|
||||
&[],
|
||||
/*archived_only*/ Some(true),
|
||||
/*new_thread_memory_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("get metadata")
|
||||
.expect("metadata")
|
||||
.archived_at
|
||||
.is_some()
|
||||
);
|
||||
|
||||
let thread = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
name: Some("Archived title".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: true,
|
||||
})
|
||||
.await
|
||||
.expect("set archived thread name");
|
||||
|
||||
assert!(thread.archived_at.is_some());
|
||||
assert!(
|
||||
runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("get metadata")
|
||||
.expect("metadata")
|
||||
.archived_at
|
||||
.is_some()
|
||||
);
|
||||
}
|
||||
|
||||
fn last_rollout_item(path: &std::path::Path) -> Value {
|
||||
let last_line = std::fs::read_to_string(path)
|
||||
.expect("read rollout")
|
||||
.lines()
|
||||
.last()
|
||||
.expect("last line")
|
||||
.to_string();
|
||||
serde_json::from_str(&last_line).expect("json line")
|
||||
}
|
||||
}
|
||||
@@ -10,7 +10,6 @@ use crate::ListThreadsParams;
|
||||
use crate::LoadThreadHistoryParams;
|
||||
use crate::ReadThreadParams;
|
||||
use crate::ResumeThreadRecorderParams;
|
||||
use crate::SetThreadNameParams;
|
||||
use crate::StoredThread;
|
||||
use crate::StoredThreadHistory;
|
||||
use crate::ThreadPage;
|
||||
@@ -82,10 +81,6 @@ impl ThreadStore for RemoteThreadStore {
|
||||
list_threads::list_threads(self, params).await
|
||||
}
|
||||
|
||||
async fn set_thread_name(&self, _params: SetThreadNameParams) -> ThreadStoreResult<()> {
|
||||
Err(not_implemented("set_thread_name"))
|
||||
}
|
||||
|
||||
async fn update_thread_metadata(
|
||||
&self,
|
||||
_params: UpdateThreadMetadataParams,
|
||||
|
||||
@@ -7,7 +7,6 @@ use crate::ListThreadsParams;
|
||||
use crate::LoadThreadHistoryParams;
|
||||
use crate::ReadThreadParams;
|
||||
use crate::ResumeThreadRecorderParams;
|
||||
use crate::SetThreadNameParams;
|
||||
use crate::StoredThread;
|
||||
use crate::StoredThreadHistory;
|
||||
use crate::ThreadPage;
|
||||
@@ -45,9 +44,6 @@ pub trait ThreadStore: Send + Sync {
|
||||
/// Lists stored threads matching the supplied filters.
|
||||
async fn list_threads(&self, params: ListThreadsParams) -> ThreadStoreResult<ThreadPage>;
|
||||
|
||||
/// Sets a user-facing thread name.
|
||||
async fn set_thread_name(&self, params: SetThreadNameParams) -> ThreadStoreResult<()>;
|
||||
|
||||
/// Applies a mutable metadata patch and returns the updated thread.
|
||||
async fn update_thread_metadata(
|
||||
&self,
|
||||
|
||||
@@ -11,6 +11,7 @@ use codex_protocol::protocol::GitInfo;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::ThreadMemoryMode as MemoryMode;
|
||||
use codex_protocol::protocol::TokenUsage;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
@@ -193,15 +194,6 @@ pub struct StoredThread {
|
||||
pub history: Option<StoredThreadHistory>,
|
||||
}
|
||||
|
||||
/// Parameters for setting a user-facing thread name.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct SetThreadNameParams {
|
||||
/// Thread id to update.
|
||||
pub thread_id: ThreadId,
|
||||
/// Normalized thread name.
|
||||
pub name: String,
|
||||
}
|
||||
|
||||
/// Optional field patch where omission leaves a value unchanged and `Some(None)` clears it.
|
||||
pub type OptionalStringPatch = Option<Option<String>>;
|
||||
|
||||
@@ -219,6 +211,10 @@ pub struct GitInfoPatch {
|
||||
/// Patch for mutable thread metadata.
|
||||
#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct ThreadMetadataPatch {
|
||||
/// Replacement user-facing thread name.
|
||||
pub name: Option<String>,
|
||||
/// Replacement thread memory behavior.
|
||||
pub memory_mode: Option<MemoryMode>,
|
||||
/// Optional Git metadata patch.
|
||||
pub git_info: Option<GitInfoPatch>,
|
||||
}
|
||||
@@ -230,6 +226,8 @@ pub struct UpdateThreadMetadataParams {
|
||||
pub thread_id: ThreadId,
|
||||
/// Patch to apply.
|
||||
pub patch: ThreadMetadataPatch,
|
||||
/// Whether archived threads are eligible.
|
||||
pub include_archived: bool,
|
||||
}
|
||||
|
||||
/// Parameters for archiving or unarchiving a thread.
|
||||
|
||||
Reference in New Issue
Block a user