From 591b7a47e7233b776a0b62117ebc4a0f6df83f41 Mon Sep 17 00:00:00 2001 From: anais Date: Sun, 21 Jun 2026 10:33:24 -0700 Subject: [PATCH] separate thread names and make metadata repair atomic --- codex-rs/rollout/src/metadata.rs | 1 - codex-rs/rollout/src/state_db.rs | 40 ++-- codex-rs/rollout/src/state_db_tests.rs | 7 +- .../state/migrations/0040_threads_name.sql | 6 + codex-rs/state/src/extract.rs | 1 + codex-rs/state/src/lib.rs | 3 + codex-rs/state/src/model/mod.rs | 3 + codex-rs/state/src/model/thread_metadata.rs | 92 ++++++-- codex-rs/state/src/runtime/memories.rs | 2 + codex-rs/state/src/runtime/test_support.rs | 1 + codex-rs/state/src/runtime/threads.rs | 204 +++++++++++++++++- codex-rs/thread-store/src/local/helpers.rs | 14 +- .../thread-store/src/local/list_threads.rs | 4 +- .../thread-store/src/local/read_thread.rs | 13 +- .../thread-store/src/local/search_threads.rs | 4 +- .../src/local/update_thread_metadata.rs | 27 +-- 16 files changed, 343 insertions(+), 79 deletions(-) create mode 100644 codex-rs/state/migrations/0040_threads_name.sql diff --git a/codex-rs/rollout/src/metadata.rs b/codex-rs/rollout/src/metadata.rs index dcb38b6d2d..72487d677e 100644 --- a/codex-rs/rollout/src/metadata.rs +++ b/codex-rs/rollout/src/metadata.rs @@ -262,7 +262,6 @@ pub(crate) async fn backfill_sessions_with_lease( let memory_mode = outcome.memory_mode.unwrap_or_else(|| "enabled".to_string()); if let Ok(Some(existing_metadata)) = runtime.get_thread(metadata.id).await { metadata.prefer_existing_git_info(&existing_metadata); - metadata.prefer_existing_explicit_title(&existing_metadata); } if rollout.archived && metadata.archived_at.is_none() { let fallback_archived_at = metadata.updated_at; diff --git a/codex-rs/rollout/src/state_db.rs b/codex-rs/rollout/src/state_db.rs index 96fbf7af1c..2d7e4e798b 100644 --- a/codex-rs/rollout/src/state_db.rs +++ b/codex-rs/rollout/src/state_db.rs @@ -530,7 +530,6 @@ pub async fn reconcile_rollout( metadata.cwd = normalize_cwd_for_state_db(&metadata.cwd); if let Ok(Some(existing_metadata)) = ctx.get_thread(metadata.id).await { metadata.prefer_existing_git_info(&existing_metadata); - metadata.prefer_existing_explicit_title(&existing_metadata); } match archived_only { Some(true) if metadata.archived_at.is_none() => { @@ -577,29 +576,24 @@ pub async fn read_repair_rollout_path( && let Ok(Some(metadata)) = ctx.get_thread(thread_id).await { saw_existing_metadata = true; - let mut repaired = metadata.clone(); - repaired.rollout_path = rollout_path.to_path_buf(); - repaired.cwd = normalize_cwd_for_state_db(&repaired.cwd); - match archived_only { - Some(true) if repaired.archived_at.is_none() => { - repaired.archived_at = Some(repaired.updated_at); + let cwd = normalize_cwd_for_state_db(&metadata.cwd); + let archive_state = match archived_only { + Some(true) => codex_state::ThreadArchiveState::Archived, + Some(false) => codex_state::ThreadArchiveState::Active, + None => codex_state::ThreadArchiveState::Preserve, + }; + match ctx + .repair_thread_rollout_location(thread_id, rollout_path, cwd.as_path(), archive_state) + .await + { + Ok(true) => return, + Ok(false) => saw_existing_metadata = false, + Err(err) => { + warn!( + "state db read-repair update failed for {}: {err}", + rollout_path.display() + ); } - Some(false) => { - repaired.archived_at = None; - } - Some(true) | None => {} - } - if repaired == metadata { - return; - } - warn!("state db discrepancy during read_repair_rollout_path: upsert_needed (fast path)"); - if let Err(err) = ctx.upsert_thread(&repaired).await { - warn!( - "state db read-repair upsert failed for {}: {err}", - rollout_path.display() - ); - } else { - return; } } diff --git a/codex-rs/rollout/src/state_db_tests.rs b/codex-rs/rollout/src/state_db_tests.rs index 4b2ae4f25c..8decfb896b 100644 --- a/codex-rs/rollout/src/state_db_tests.rs +++ b/codex-rs/rollout/src/state_db_tests.rs @@ -108,7 +108,7 @@ async fn try_init_times_out_waiting_for_stuck_startup_backfill() -> anyhow::Resu } #[tokio::test] -async fn reconcile_rollout_preserves_existing_explicit_title() -> anyhow::Result<()> { +async fn reconcile_rollout_preserves_existing_explicit_name() -> anyhow::Result<()> { let home = TempDir::new().expect("temp dir"); let thread_id = ThreadId::new(); let rollout_path = write_rollout_with_user_message(home.path(), thread_id, "Hey")?; @@ -122,7 +122,7 @@ async fn reconcile_rollout_preserves_existing_explicit_title() -> anyhow::Result .metadata; assert_eq!(metadata.title, "Hey"); assert_eq!(metadata.first_user_message.as_deref(), Some("Hey")); - metadata.title = "math".to_string(); + metadata.name = Some("math".to_string()); runtime.upsert_thread(&metadata).await?; reconcile_rollout( @@ -140,7 +140,8 @@ async fn reconcile_rollout_preserves_existing_explicit_title() -> anyhow::Result .get_thread(thread_id) .await? .expect("thread should exist"); - assert_eq!(persisted.title, "math"); + assert_eq!(persisted.title, "Hey"); + assert_eq!(persisted.name.as_deref(), Some("math")); assert_eq!(persisted.first_user_message.as_deref(), Some("Hey")); Ok(()) } diff --git a/codex-rs/state/migrations/0040_threads_name.sql b/codex-rs/state/migrations/0040_threads_name.sql new file mode 100644 index 0000000000..066de545fb --- /dev/null +++ b/codex-rs/state/migrations/0040_threads_name.sql @@ -0,0 +1,6 @@ +ALTER TABLE threads ADD COLUMN name TEXT; + +UPDATE threads +SET name = title +WHERE title <> '' + AND (first_user_message = '' OR trim(title) <> trim(first_user_message)); diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index aa1dd04036..57dfea86ce 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -562,6 +562,7 @@ mod tests { cwd: PathBuf::from("/tmp"), cli_version: "0.0.0".to_string(), title: String::new(), + name: None, preview: None, sandbox_policy: "read-only".to_string(), approval_mode: "on-request".to_string(), diff --git a/codex-rs/state/src/lib.rs b/codex-rs/state/src/lib.rs index 6b5b907653..b4612d1205 100644 --- a/codex-rs/state/src/lib.rs +++ b/codex-rs/state/src/lib.rs @@ -51,8 +51,11 @@ pub use model::Stage1JobClaim; pub use model::Stage1JobClaimOutcome; pub use model::Stage1Output; pub use model::Stage1StartupClaimParams; +pub use model::ThreadArchiveState; pub use model::ThreadGoal; pub use model::ThreadGoalStatus; +pub use model::ThreadListItem; +pub use model::ThreadListPage; pub use model::ThreadMetadata; pub use model::ThreadMetadataBuilder; pub use model::ThreadsPage; diff --git a/codex-rs/state/src/model/mod.rs b/codex-rs/state/src/model/mod.rs index 2c725e50ab..86218970c0 100644 --- a/codex-rs/state/src/model/mod.rs +++ b/codex-rs/state/src/model/mod.rs @@ -31,6 +31,9 @@ pub use thread_metadata::BackfillStats; pub use thread_metadata::ExtractionOutcome; pub use thread_metadata::SortDirection; pub use thread_metadata::SortKey; +pub use thread_metadata::ThreadArchiveState; +pub use thread_metadata::ThreadListItem; +pub use thread_metadata::ThreadListPage; pub use thread_metadata::ThreadMetadata; pub use thread_metadata::ThreadMetadataBuilder; pub use thread_metadata::ThreadsPage; diff --git a/codex-rs/state/src/model/thread_metadata.rs b/codex-rs/state/src/model/thread_metadata.rs index c25c458986..0c8fbf034a 100644 --- a/codex-rs/state/src/model/thread_metadata.rs +++ b/codex-rs/state/src/model/thread_metadata.rs @@ -29,6 +29,17 @@ pub enum SortDirection { Desc, } +/// Archive-state change applied while repairing a thread's rollout location. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ThreadArchiveState { + /// Keep the existing archive state. + Preserve, + /// Mark the thread active. + Active, + /// Mark the thread archived. + Archived, +} + /// A pagination anchor used for keyset pagination. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Anchor { @@ -49,6 +60,60 @@ pub struct ThreadsPage { pub num_scanned_rows: usize, } +/// A single page of lightweight thread list results. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ThreadListPage { + /// The thread summary items in this page. + pub items: Vec, + /// The next anchor to use for pagination, if any. + pub next_anchor: Option, + /// The number of rows scanned to produce this page. + pub num_scanned_rows: usize, +} + +/// Thread metadata fields needed to render list pages. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ThreadListItem { + /// The thread identifier. + pub id: ThreadId, + /// The local rollout path. + pub rollout_path: PathBuf, + /// The creation timestamp. + pub created_at: DateTime, + /// The latest metadata timestamp. + pub updated_at: DateTime, + /// The product recency timestamp. + pub recency_at: DateTime, + /// The serialized session source. + pub source: String, + /// The canonical parent thread identifier for spawned threads. + pub parent_thread_id: Option, + /// The optional agent nickname. + pub agent_nickname: Option, + /// The optional agent role. + pub agent_role: Option, + /// The model provider identifier. + pub model_provider: String, + /// The working directory. + pub cwd: PathBuf, + /// The CLI version that created the thread. + pub cli_version: String, + /// The best-effort history-derived title. + pub title: String, + /// The explicit user-assigned name. + pub name: Option, + /// The best available list preview. + pub preview: Option, + /// The first user message, when known. + pub first_user_message: Option, + /// The git commit SHA, when known. + pub git_sha: Option, + /// The git branch, when known. + pub git_branch: Option, + /// The git origin URL, when known. + pub git_origin_url: Option, +} + /// The outcome of extracting metadata from a rollout. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ExtractionOutcome { @@ -95,6 +160,8 @@ pub struct ThreadMetadata { pub cli_version: String, /// A best-effort thread title. pub title: String, + /// An explicit user-assigned thread name. + pub name: Option, /// Best available user-facing preview for discovery and list display. pub preview: Option, /// The sandbox policy (stringified enum). @@ -226,6 +293,7 @@ impl ThreadMetadataBuilder { cwd: self.cwd.clone(), cli_version: self.cli_version.clone().unwrap_or_default(), title: String::new(), + name: None, preview: None, sandbox_policy, approval_mode, @@ -253,21 +321,6 @@ impl ThreadMetadata { } } - /// Preserve an existing user-facing title when reconciling rollout-derived metadata. - pub fn prefer_existing_explicit_title(&mut self, existing: &Self) { - let existing_title = existing.title.trim(); - if existing_title.is_empty() - || existing.first_user_message.as_deref().map(str::trim) == Some(existing_title) - { - return; - } - - let title = self.title.trim(); - if title.is_empty() || self.first_user_message.as_deref().map(str::trim) == Some(title) { - self.title = existing.title.clone(); - } - } - /// Return the list of field names that differ between `self` and `other`. pub fn diff_fields(&self, other: &Self) -> Vec<&'static str> { let mut diffs = Vec::new(); @@ -313,6 +366,9 @@ impl ThreadMetadata { if self.title != other.title { diffs.push("title"); } + if self.name != other.name { + diffs.push("name"); + } if self.preview != other.preview { diffs.push("preview"); } @@ -366,6 +422,7 @@ pub(crate) struct ThreadRow { cwd: String, cli_version: String, title: String, + name: Option, preview: String, sandbox_policy: String, approval_mode: String, @@ -396,6 +453,7 @@ impl ThreadRow { cwd: row.try_get("cwd")?, cli_version: row.try_get("cli_version")?, title: row.try_get("title")?, + name: row.try_get("name")?, preview: row.try_get("preview")?, sandbox_policy: row.try_get("sandbox_policy")?, approval_mode: row.try_get("approval_mode")?, @@ -430,6 +488,7 @@ impl TryFrom for ThreadMetadata { cwd, cli_version, title, + name, preview, sandbox_policy, approval_mode, @@ -462,6 +521,7 @@ impl TryFrom for ThreadMetadata { cwd: PathBuf::from(cwd), cli_version, title, + name, preview: (!preview.is_empty()).then_some(preview), sandbox_policy, approval_mode, @@ -553,6 +613,7 @@ mod tests { cwd: "/tmp/workspace".to_string(), cli_version: "0.0.0".to_string(), title: String::new(), + name: None, preview: String::new(), sandbox_policy: "read-only".to_string(), approval_mode: "on-request".to_string(), @@ -584,6 +645,7 @@ mod tests { cwd: PathBuf::from("/tmp/workspace"), cli_version: "0.0.0".to_string(), title: String::new(), + name: None, preview: None, sandbox_policy: "read-only".to_string(), approval_mode: "on-request".to_string(), diff --git a/codex-rs/state/src/runtime/memories.rs b/codex-rs/state/src/runtime/memories.rs index 9b2c337f5f..e4b5328a8a 100644 --- a/codex-rs/state/src/runtime/memories.rs +++ b/codex-rs/state/src/runtime/memories.rs @@ -187,6 +187,7 @@ SELECT threads.cwd, threads.cli_version, threads.title, + threads.name, threads.preview, threads.sandbox_policy, threads.approval_mode, @@ -558,6 +559,7 @@ SELECT threads.cwd, threads.cli_version, threads.title, + threads.name, threads.preview, threads.sandbox_policy, threads.approval_mode, diff --git a/codex-rs/state/src/runtime/test_support.rs b/codex-rs/state/src/runtime/test_support.rs index c029a0ee2d..37eda950d2 100644 --- a/codex-rs/state/src/runtime/test_support.rs +++ b/codex-rs/state/src/runtime/test_support.rs @@ -59,6 +59,7 @@ pub(super) fn test_thread_metadata( cwd, cli_version: "0.0.0".to_string(), title: String::new(), + name: None, preview: Some("hello".to_string()), sandbox_policy: crate::extract::enum_to_string(&SandboxPolicy::new_read_only_policy()), approval_mode: crate::extract::enum_to_string(&AskForApproval::OnRequest), diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index a448dce56b..247f048119 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -25,6 +25,7 @@ SELECT threads.cwd, threads.cli_version, threads.title, + threads.name, threads.preview, threads.sandbox_policy, threads.approval_mode, @@ -376,7 +377,7 @@ ON CONFLICT(child_thread_id) DO NOTHING search_term: None, }, ); - builder.push(" AND threads.title = "); + builder.push(" AND COALESCE(NULLIF(threads.name, ''), threads.title) = "); builder.push_bind(title); if let Some(cwd) = cwd { builder.push(" AND threads.cwd = "); @@ -416,6 +417,57 @@ ON CONFLICT(child_thread_id) DO NOTHING .await } + /// List lightweight thread summaries using the underlying database. + pub async fn list_thread_summaries( + &self, + page_size: usize, + filters: ThreadFilterOptions<'_>, + ) -> anyhow::Result { + self.list_thread_summaries_matching(page_size, filters, /*parent_thread_id*/ None) + .await + } + + /// List lightweight direct children using persisted spawn edges. + pub async fn list_thread_summaries_by_parent( + &self, + page_size: usize, + parent_thread_id: ThreadId, + filters: ThreadFilterOptions<'_>, + ) -> anyhow::Result { + self.list_thread_summaries_matching(page_size, filters, Some(parent_thread_id)) + .await + } + + async fn list_thread_summaries_matching( + &self, + page_size: usize, + filters: ThreadFilterOptions<'_>, + parent_thread_id: Option, + ) -> anyhow::Result { + let limit = page_size.saturating_add(1); + let mut builder = QueryBuilder::::new(""); + push_list_thread_summaries_query(&mut builder, filters, parent_thread_id, limit); + let rows = builder.build().fetch_all(self.pool.as_ref()).await?; + let mut items = rows + .into_iter() + .map(|row| thread_list_item_from_row(&row)) + .collect::, _>>()?; + let num_scanned_rows = items.len(); + let next_anchor = if items.len() > page_size { + items.pop(); + items + .last() + .and_then(|item| anchor_from_list_item(item, filters.sort_key)) + } else { + None + }; + Ok(crate::ThreadListPage { + items, + next_anchor, + num_scanned_rows, + }) + } + async fn list_threads_matching( &self, page_size: usize, @@ -524,6 +576,7 @@ INSERT INTO threads ( cwd, cli_version, title, + name, preview, sandbox_policy, approval_mode, @@ -535,7 +588,7 @@ INSERT INTO threads ( git_branch, git_origin_url, memory_mode -) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO NOTHING "#, ) @@ -568,6 +621,7 @@ ON CONFLICT(id) DO NOTHING .bind(metadata.cwd.display().to_string()) .bind(metadata.cli_version.as_str()) .bind(metadata.title.as_str()) + .bind(metadata.name.as_deref()) .bind(preview) .bind(metadata.sandbox_policy.as_str()) .bind(metadata.approval_mode.as_str()) @@ -599,19 +653,48 @@ ON CONFLICT(id) DO NOTHING Ok(result.rows_affected() > 0) } - pub async fn update_thread_title( + pub async fn update_thread_name( &self, thread_id: ThreadId, - title: &str, + name: Option<&str>, ) -> anyhow::Result { - let result = sqlx::query("UPDATE threads SET title = ? WHERE id = ?") - .bind(title) + let result = sqlx::query("UPDATE threads SET name = ? WHERE id = ?") + .bind(name) .bind(thread_id.to_string()) .execute(self.pool.as_ref()) .await?; Ok(result.rows_affected() > 0) } + /// Repair location-owned fields without overwriting concurrently updated metadata. + pub async fn repair_thread_rollout_location( + &self, + thread_id: ThreadId, + rollout_path: &Path, + cwd: &Path, + archive_state: crate::ThreadArchiveState, + ) -> anyhow::Result { + let mut builder = QueryBuilder::::new("UPDATE threads SET rollout_path = "); + builder.push_bind(rollout_path.display().to_string()); + builder + .push(", cwd = ") + .push_bind(cwd.display().to_string()); + match archive_state { + crate::ThreadArchiveState::Preserve => {} + crate::ThreadArchiveState::Active => { + builder.push(", archived = 0, archived_at = NULL"); + } + crate::ThreadArchiveState::Archived => { + builder.push(", archived = 1, archived_at = COALESCE(archived_at, updated_at)"); + } + } + builder + .push(" WHERE id = ") + .push_bind(thread_id.to_string()); + let result = builder.build().execute(self.pool.as_ref()).await?; + Ok(result.rows_affected() > 0) + } + pub async fn touch_thread_updated_at( &self, thread_id: ThreadId, @@ -776,6 +859,7 @@ INSERT INTO threads ( cwd, cli_version, title, + name, preview, sandbox_policy, approval_mode, @@ -787,7 +871,7 @@ INSERT INTO threads ( git_branch, git_origin_url, memory_mode -) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO UPDATE SET rollout_path = excluded.rollout_path, created_at = excluded.created_at, @@ -807,6 +891,7 @@ ON CONFLICT(id) DO UPDATE SET cwd = excluded.cwd, cli_version = excluded.cli_version, title = excluded.title, + name = COALESCE(threads.name, excluded.name), preview = COALESCE(NULLIF(excluded.preview, ''), threads.preview), sandbox_policy = excluded.sandbox_policy, approval_mode = excluded.approval_mode, @@ -848,6 +933,7 @@ ON CONFLICT(id) DO UPDATE SET .bind(metadata.cwd.display().to_string()) .bind(metadata.cli_version.as_str()) .bind(metadata.title.as_str()) + .bind(metadata.name.as_deref()) .bind(preview) .bind(metadata.sandbox_policy.as_str()) .bind(metadata.approval_mode.as_str()) @@ -1125,6 +1211,62 @@ fn push_list_threads_query( ); } +fn push_list_thread_summaries_query( + builder: &mut QueryBuilder, + filters: ThreadFilterOptions<'_>, + parent_thread_id: Option, + limit: usize, +) { + push_thread_list_select_columns(builder); + builder.push(" FROM threads"); + push_thread_filters(builder, filters); + if let Some(parent_thread_id) = parent_thread_id { + builder.push( + " AND threads.id IN (SELECT child_thread_id FROM thread_spawn_edges WHERE parent_thread_id = ", + ); + builder.push_bind(parent_thread_id.to_string()); + builder.push(")"); + } + let order_by_index = match filters.cwd_filters { + Some(cwd_filters) if cwd_filters.len() > 1 => OrderByIndex::Disabled, + Some(_) | None => OrderByIndex::Enabled, + }; + push_thread_order_and_limit( + builder, + filters.sort_key, + filters.sort_direction, + order_by_index, + limit, + ); +} + +fn push_thread_list_select_columns(builder: &mut QueryBuilder) { + builder.push( + r#" +SELECT + threads.id, + threads.rollout_path, + threads.created_at_ms AS created_at, + threads.updated_at_ms AS updated_at, + threads.recency_at_ms AS recency_at, + threads.source, + (SELECT parent_thread_id FROM thread_spawn_edges WHERE child_thread_id = threads.id) AS parent_thread_id, + threads.agent_nickname, + threads.agent_role, + threads.model_provider, + threads.cwd, + threads.cli_version, + threads.title, + threads.name, + threads.preview, + threads.first_user_message, + threads.git_sha, + threads.git_branch, + threads.git_origin_url +"#, + ); +} + pub(super) fn push_thread_select_columns(builder: &mut QueryBuilder) { builder.push( r#" @@ -1145,6 +1287,7 @@ SELECT threads.cwd, threads.cli_version, threads.title, + threads.name, threads.preview, threads.sandbox_policy, threads.approval_mode, @@ -1241,7 +1384,7 @@ pub(super) fn push_thread_filters<'a>( None => {} } if let Some(search_term) = search_term { - builder.push(" AND (instr(threads.title, "); + builder.push(" AND (instr(COALESCE(NULLIF(threads.name, ''), threads.title), "); builder.push_bind(search_term); builder.push(") > 0 OR instr(threads.preview, "); builder.push_bind(search_term); @@ -1333,6 +1476,51 @@ fn metadata_preview(metadata: &crate::ThreadMetadata) -> &str { .unwrap_or_default() } +fn thread_list_item_from_row( + row: &sqlx::sqlite::SqliteRow, +) -> anyhow::Result { + let id: String = row.try_get("id")?; + let parent_thread_id = row + .try_get::, _>("parent_thread_id")? + .map(ThreadId::try_from) + .transpose()?; + let preview: String = row.try_get("preview")?; + let first_user_message: String = row.try_get("first_user_message")?; + Ok(crate::ThreadListItem { + id: ThreadId::try_from(id)?, + rollout_path: PathBuf::from(row.try_get::("rollout_path")?), + created_at: epoch_millis_to_datetime(row.try_get("created_at")?)?, + updated_at: epoch_millis_to_datetime(row.try_get("updated_at")?)?, + recency_at: epoch_millis_to_datetime(row.try_get("recency_at")?)?, + source: row.try_get("source")?, + parent_thread_id, + agent_nickname: row.try_get("agent_nickname")?, + agent_role: row.try_get("agent_role")?, + model_provider: row.try_get("model_provider")?, + cwd: PathBuf::from(row.try_get::("cwd")?), + cli_version: row.try_get("cli_version")?, + title: row.try_get("title")?, + name: row.try_get("name")?, + preview: (!preview.is_empty()).then_some(preview), + first_user_message: (!first_user_message.is_empty()).then_some(first_user_message), + git_sha: row.try_get("git_sha")?, + git_branch: row.try_get("git_branch")?, + git_origin_url: row.try_get("git_origin_url")?, + }) +} + +fn anchor_from_list_item(item: &crate::ThreadListItem, sort_key: SortKey) -> Option { + let ts = match sort_key { + SortKey::CreatedAt => item.created_at, + SortKey::UpdatedAt => item.updated_at, + SortKey::RecencyAt => item.recency_at, + }; + Some(crate::Anchor { + ts, + id: (sort_key == SortKey::RecencyAt).then_some(item.id), + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/thread-store/src/local/helpers.rs b/codex-rs/thread-store/src/local/helpers.rs index f48a0136c0..971eee8d03 100644 --- a/codex-rs/thread-store/src/local/helpers.rs +++ b/codex-rs/thread-store/src/local/helpers.rs @@ -176,13 +176,13 @@ pub(super) fn permission_profile_to_metadata_value( } } -pub(super) fn distinct_thread_metadata_title(metadata: &ThreadMetadata) -> Option { - let title = metadata.title.trim(); - if title.is_empty() || metadata.first_user_message.as_deref().map(str::trim) == Some(title) { - None - } else { - Some(title.to_string()) - } +pub(super) fn thread_metadata_name(metadata: &ThreadMetadata) -> Option { + metadata + .name + .as_deref() + .map(str::trim) + .filter(|name| !name.is_empty()) + .map(str::to_string) } pub(super) fn set_thread_name_from_title(thread: &mut StoredThread, title: String) { diff --git a/codex-rs/thread-store/src/local/list_threads.rs b/codex-rs/thread-store/src/local/list_threads.rs index 03ae163afe..9c9b7d94ed 100644 --- a/codex-rs/thread-store/src/local/list_threads.rs +++ b/codex-rs/thread-store/src/local/list_threads.rs @@ -8,9 +8,9 @@ use codex_rollout::find_thread_names_by_ids; use codex_rollout::parse_cursor; use super::LocalThreadStore; -use super::helpers::distinct_thread_metadata_title; use super::helpers::set_thread_name_from_title; use super::helpers::stored_thread_from_rollout_item; +use super::helpers::thread_metadata_name; use crate::ListThreadsParams; use crate::SortDirection; use crate::ThreadPage; @@ -86,7 +86,7 @@ pub(super) async fn list_threads( let Ok(Some(metadata)) = state_db_ctx.get_thread(thread_id).await else { continue; }; - if let Some(title) = distinct_thread_metadata_title(&metadata) { + if let Some(title) = thread_metadata_name(&metadata) { names.insert(thread_id, title); } } diff --git a/codex-rs/thread-store/src/local/read_thread.rs b/codex-rs/thread-store/src/local/read_thread.rs index 1271e4fcf5..db60d89cf5 100644 --- a/codex-rs/thread-store/src/local/read_thread.rs +++ b/codex-rs/thread-store/src/local/read_thread.rs @@ -13,12 +13,12 @@ use codex_rollout::read_thread_item_from_rollout; use codex_state::ThreadMetadata; use super::LocalThreadStore; -use super::helpers::distinct_thread_metadata_title; use super::helpers::git_info_from_parts; use super::helpers::permission_profile_from_metadata_value; use super::helpers::rollout_path_is_archived; use super::helpers::set_thread_name_from_title; use super::helpers::stored_thread_from_rollout_item; +use super::helpers::thread_metadata_name; use super::live_writer; use crate::ReadThreadParams; use crate::StoredThread; @@ -117,6 +117,9 @@ pub(super) async fn read_thread_by_rollout_path( } if let Some(metadata) = read_sqlite_metadata(store, thread.thread_id).await { thread.recency_at = metadata.recency_at; + if let Some(name) = thread_metadata_name(&metadata) { + set_thread_name_from_title(&mut thread, name); + } let existing_git_info = thread.git_info.take(); let (fallback_sha, fallback_branch, fallback_origin_url) = match existing_git_info { Some(info) => ( @@ -303,7 +306,7 @@ async fn stored_thread_from_sqlite_metadata( store: &LocalThreadStore, metadata: ThreadMetadata, ) -> StoredThread { - let name = match distinct_thread_metadata_title(&metadata) { + let name = match thread_metadata_name(&metadata) { Some(title) => Some(title), None => find_thread_name_by_id(store.config.codex_home.as_path(), &metadata.id) .await @@ -701,7 +704,7 @@ mod tests { builder.cwd = home.path().to_path_buf(); builder.cli_version = Some("test_version".to_string()); let mut metadata = builder.build(config.default_model_provider_id.as_str()); - metadata.title = "Saved title".to_string(); + metadata.name = Some("Saved title".to_string()); metadata.first_user_message = Some("Hello from user".to_string()); runtime .upsert_thread(&metadata) @@ -852,7 +855,7 @@ mod tests { builder.model_provider = Some(config.default_model_provider_id.clone()); builder.cwd = home.path().join("sqlite-workspace"); let mut metadata = builder.build(config.default_model_provider_id.as_str()); - metadata.title = "Saved title".to_string(); + metadata.name = Some("Saved title".to_string()); metadata.first_user_message = Some("Hello from sqlite".to_string()); metadata.sandbox_policy = "workspace-write".to_string(); runtime @@ -956,7 +959,7 @@ mod tests { builder.cwd = home.path().join("workspace"); builder.cli_version = Some("sqlite-cli".to_string()); let mut metadata = builder.build(config.default_model_provider_id.as_str()); - metadata.title = "Command-only thread".to_string(); + metadata.name = Some("Command-only thread".to_string()); runtime .upsert_thread(&metadata) .await diff --git a/codex-rs/thread-store/src/local/search_threads.rs b/codex-rs/thread-store/src/local/search_threads.rs index fc2ddd6e71..d5acc9ad51 100644 --- a/codex-rs/thread-store/src/local/search_threads.rs +++ b/codex-rs/thread-store/src/local/search_threads.rs @@ -10,9 +10,9 @@ use codex_rollout::parse_cursor; use codex_rollout::search_rollout_matches; use super::LocalThreadStore; -use super::helpers::distinct_thread_metadata_title; use super::helpers::set_thread_name_from_title; use super::helpers::stored_thread_from_rollout_item; +use super::helpers::thread_metadata_name; use super::list_threads::list_rollout_threads; use crate::ListThreadsParams; use crate::SearchThreadsParams; @@ -211,7 +211,7 @@ async fn set_thread_search_result_names( let Ok(Some(metadata)) = state_db_ctx.get_thread(thread_id).await else { continue; }; - if let Some(title) = distinct_thread_metadata_title(&metadata) { + if let Some(title) = thread_metadata_name(&metadata) { names.insert(thread_id, title); } } diff --git a/codex-rs/thread-store/src/local/update_thread_metadata.rs b/codex-rs/thread-store/src/local/update_thread_metadata.rs index d1c76ad327..9d8028fc25 100644 --- a/codex-rs/thread-store/src/local/update_thread_metadata.rs +++ b/codex-rs/thread-store/src/local/update_thread_metadata.rs @@ -92,7 +92,7 @@ pub(super) async fn update_thread_metadata( .await; if let Some(name) = name { - apply_thread_name(store, thread_id, name.unwrap_or_default()).await?; + apply_thread_name(store, thread_id, name).await?; } let resolved_git_info = match git_info { @@ -240,9 +240,6 @@ async fn apply_metadata_update( if let Some(preview) = patch.preview { metadata.preview = Some(preview); } - if let Some(name) = patch.name { - metadata.title = name.unwrap_or_default(); - } if let Some(title) = patch.title { metadata.title = title; } @@ -515,11 +512,11 @@ async fn apply_thread_git_info_to_rollout( async fn apply_thread_name( store: &LocalThreadStore, thread_id: ThreadId, - name: String, + name: Option, ) -> ThreadStoreResult<()> { if let Some(state_db) = store.state_db().await { let updated = state_db - .update_thread_title(thread_id, &name) + .update_thread_name(thread_id, name.as_deref()) .await .map_err(|err| ThreadStoreError::Internal { message: format!("failed to set thread name: {err}"), @@ -531,11 +528,15 @@ async fn apply_thread_name( } } - 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}"), - }) + append_thread_name( + store.config.codex_home.as_path(), + thread_id, + name.as_deref().unwrap_or_default(), + ) + .await + .map_err(|err| ThreadStoreError::Internal { + message: format!("failed to index thread name: {err}"), + }) } async fn apply_thread_memory_mode( @@ -1266,7 +1267,7 @@ mod tests { } #[tokio::test] - async fn metadata_patch_applies_title_over_existing_name() { + async fn metadata_patch_preserves_explicit_name_over_derived_title() { let home = TempDir::new().expect("temp dir"); let config = test_config(home.path()); let runtime = codex_state::StateRuntime::init( @@ -1305,7 +1306,7 @@ mod tests { .await .expect("apply observed metadata"); - assert_eq!(thread.name.as_deref(), Some("Derived first message")); + assert_eq!(thread.name.as_deref(), Some("User chosen name")); } #[tokio::test]