mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add thread recencyAt for sidebar ordering (#27910)
## Summary Add a server-owned `recencyAt` timestamp and `recency_at` thread-list sort key for product recency ordering while preserving the existing meaning of `updatedAt` as the latest persisted thread mutation. This is the server-side alternative to #27697. Rather than narrowing `updatedAt`, clients can sort the sidebar by `recency_at` and continue treating `updatedAt` as mutation time. Paired Codex Apps PR: [openai/openai#1024599](https://github.com/openai/openai/pull/1024599) ## Contract - `recencyAt` initializes when a thread is created. - A turn start advances `recencyAt` monotonically. - Commentary, agent output, tool results, token/accounting updates, turn completion, archive, unarchive, resume, and generic metadata writes do not advance it. - `updatedAt` retains its existing behavior and continues to advance for persisted thread mutations. - Current servers populate `recencyAt`; the response field is optional in generated TypeScript so clients connected to older servers can fall back to `updatedAt`. - Filesystem-only fallback uses existing updated/mtime ordering when SQLite is unavailable. ## Persistence and compatibility Migration 0038 adds second- and millisecond-precision recency columns, backfills them from the existing updated timestamp, creates list indexes, and includes an insert trigger so older binaries writing to a migrated database seed recency without causing later mutations to advance it. Generic metadata upserts preserve existing recency values. Turn-start updates use a dedicated monotonic touch, and process-local allocation keeps millisecond cursor values unique. State DB list, search, read, filtered-list repair, rollout fallback propagation, and app-server conversions all carry the new field. ## API `Thread` responses include: ```ts recencyAt?: number ``` `thread/list` and `thread/search` accept: ```json { "sortKey": "recency_at" } ``` Generated TypeScript and JSON schemas are included. ## Validation - `just test -p codex-state` — 146 passed - `just test -p codex-rollout` — 69 passed - `just test -p codex-thread-store` — 81 passed - `just test -p codex-app-server-protocol` — 231 passed - Focused app-server list ordering, response mapping, archive/unarchive, and resume lifecycle tests passed - Scoped `just fix` for state, rollout, thread-store, app-server-protocol, and app-server - `just fmt` - `git diff --check` - Independent correctness, simplicity, elegance, security, and test-quality reviews; actionable ordering, lifecycle, query-projection, and timestamp-uniqueness findings were addressed
This commit is contained in:
@@ -535,6 +535,9 @@ fn stored_thread_from_state(
|
||||
updated_at: metadata
|
||||
.and_then(|metadata| metadata.updated_at)
|
||||
.unwrap_or_else(Utc::now),
|
||||
recency_at: metadata
|
||||
.and_then(|metadata| metadata.advance_recency_at.or(metadata.updated_at))
|
||||
.unwrap_or_else(Utc::now),
|
||||
archived_at: None,
|
||||
cwd: metadata
|
||||
.and_then(|metadata| metadata.cwd.clone())
|
||||
|
||||
@@ -174,5 +174,6 @@ mod tests {
|
||||
.expect("thread metadata should exist");
|
||||
assert_eq!(updated.rollout_path, archived_path);
|
||||
assert!(updated.archived_at.is_some());
|
||||
assert_eq!(updated.recency_at, metadata.recency_at);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -106,6 +106,7 @@ pub(super) fn stored_thread_from_rollout_item(
|
||||
.or_else(|| thread_id_from_rollout_path(item.path.as_path()))?;
|
||||
let created_at = parse_rfc3339(item.created_at.as_deref()).unwrap_or_else(Utc::now);
|
||||
let updated_at = parse_rfc3339(item.updated_at.as_deref()).unwrap_or(created_at);
|
||||
let recency_at = parse_rfc3339(item.recency_at.as_deref()).unwrap_or(updated_at);
|
||||
let archived_at = archived.then_some(updated_at);
|
||||
let git_info = git_info_from_parts(
|
||||
item.git_sha.clone(),
|
||||
@@ -136,6 +137,7 @@ pub(super) fn stored_thread_from_rollout_item(
|
||||
reasoning_effort: None,
|
||||
created_at,
|
||||
updated_at,
|
||||
recency_at,
|
||||
archived_at,
|
||||
cwd: item.cwd.unwrap_or_default(),
|
||||
cli_version: item.cli_version.unwrap_or_default(),
|
||||
|
||||
@@ -34,6 +34,7 @@ pub(super) async fn list_threads(
|
||||
let sort_key = match params.sort_key {
|
||||
ThreadSortKey::CreatedAt => codex_rollout::ThreadSortKey::CreatedAt,
|
||||
ThreadSortKey::UpdatedAt => codex_rollout::ThreadSortKey::UpdatedAt,
|
||||
ThreadSortKey::RecencyAt => codex_rollout::ThreadSortKey::RecencyAt,
|
||||
};
|
||||
let sort_direction = match params.sort_direction {
|
||||
SortDirection::Asc => codex_rollout::SortDirection::Asc,
|
||||
|
||||
@@ -314,10 +314,16 @@ mod tests {
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::MessagePhase;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AgentMessageEvent;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::ThreadMemoryMode;
|
||||
use codex_protocol::protocol::TurnCompleteEvent;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use tempfile::TempDir;
|
||||
|
||||
@@ -449,6 +455,91 @@ mod tests {
|
||||
assert_eq!(metadata.title, "observed append");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn live_thread_output_advances_updated_at_but_not_recency_at() {
|
||||
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 live_thread = LiveThread::create(store, create_thread_params(thread_id))
|
||||
.await
|
||||
.expect("create live thread");
|
||||
|
||||
live_thread
|
||||
.append_items(&[user_message_item("start thread")])
|
||||
.await
|
||||
.expect("append initial user message");
|
||||
live_thread.flush().await.expect("flush thread");
|
||||
let before_turn_start = runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("sqlite metadata read")
|
||||
.expect("sqlite metadata");
|
||||
|
||||
live_thread
|
||||
.append_items(&[RolloutItem::EventMsg(EventMsg::TurnStarted(
|
||||
TurnStartedEvent {
|
||||
turn_id: "turn-1".to_string(),
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
},
|
||||
))])
|
||||
.await
|
||||
.expect("append turn start");
|
||||
live_thread.flush().await.expect("flush thread");
|
||||
let after_turn_start = runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("sqlite metadata read")
|
||||
.expect("sqlite metadata");
|
||||
assert!(after_turn_start.recency_at > before_turn_start.recency_at);
|
||||
|
||||
live_thread
|
||||
.append_items(&[
|
||||
RolloutItem::EventMsg(EventMsg::AgentMessage(AgentMessageEvent {
|
||||
message: "commentary".to_string(),
|
||||
phase: Some(MessagePhase::Commentary),
|
||||
memory_citation: None,
|
||||
})),
|
||||
RolloutItem::ResponseItem(ResponseItem::FunctionCallOutput {
|
||||
call_id: "call-1".to_string(),
|
||||
output: FunctionCallOutputPayload::from_text("tool output".to_string()),
|
||||
}),
|
||||
RolloutItem::EventMsg(EventMsg::TokenCount(
|
||||
codex_protocol::protocol::TokenCountEvent {
|
||||
info: None,
|
||||
rate_limits: None,
|
||||
},
|
||||
)),
|
||||
RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent {
|
||||
turn_id: "turn-1".to_string(),
|
||||
last_agent_message: None,
|
||||
completed_at: None,
|
||||
duration_ms: None,
|
||||
time_to_first_token_ms: None,
|
||||
})),
|
||||
])
|
||||
.await
|
||||
.expect("append post-start items");
|
||||
live_thread.flush().await.expect("flush thread");
|
||||
let completed = runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("sqlite metadata read")
|
||||
.expect("sqlite metadata");
|
||||
|
||||
assert!(completed.updated_at > after_turn_start.updated_at);
|
||||
assert_eq!(completed.recency_at, after_turn_start.recency_at);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn live_thread_shutdown_does_not_materialize_empty_thread_metadata() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
|
||||
@@ -55,6 +55,7 @@ pub(super) async fn read_thread(
|
||||
&& (params.include_archived || rollout_thread.archived_at.is_none())
|
||||
&& !rollout_thread.preview.is_empty()
|
||||
{
|
||||
rollout_thread.recency_at = thread.recency_at;
|
||||
if thread.name.is_some() {
|
||||
rollout_thread.name = thread.name;
|
||||
}
|
||||
@@ -115,6 +116,7 @@ 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;
|
||||
let existing_git_info = thread.git_info.take();
|
||||
let (fallback_sha, fallback_branch, fallback_origin_url) = match existing_git_info {
|
||||
Some(info) => (
|
||||
@@ -340,6 +342,7 @@ async fn stored_thread_from_sqlite_metadata(
|
||||
reasoning_effort: metadata.reasoning_effort,
|
||||
created_at: metadata.created_at,
|
||||
updated_at: metadata.updated_at,
|
||||
recency_at: metadata.recency_at,
|
||||
archived_at: metadata.archived_at,
|
||||
cwd: metadata.cwd,
|
||||
cli_version: metadata.cli_version,
|
||||
@@ -406,6 +409,7 @@ fn stored_thread_from_meta_line(
|
||||
reasoning_effort: None,
|
||||
created_at,
|
||||
updated_at,
|
||||
recency_at: updated_at,
|
||||
archived_at: archived.then_some(updated_at),
|
||||
cwd: meta_line.meta.cwd,
|
||||
cli_version: meta_line.meta.cli_version,
|
||||
@@ -547,6 +551,10 @@ mod tests {
|
||||
);
|
||||
builder.model_provider = Some(config.default_model_provider_id.clone());
|
||||
builder.git_branch = Some("sqlite-branch".to_string());
|
||||
let recency_at = chrono::DateTime::parse_from_rfc3339("2026-01-03T12:00:00Z")
|
||||
.expect("timestamp should parse")
|
||||
.with_timezone(&Utc);
|
||||
builder.recency_at = Some(recency_at);
|
||||
runtime
|
||||
.upsert_thread(&builder.build(config.default_model_provider_id.as_str()))
|
||||
.await
|
||||
@@ -562,6 +570,7 @@ mod tests {
|
||||
.expect("read thread by rollout path");
|
||||
|
||||
let git_info = thread.git_info.expect("git info should be present");
|
||||
assert_eq!(thread.recency_at, recency_at);
|
||||
assert_eq!(git_info.branch.as_deref(), Some("sqlite-branch"));
|
||||
assert_eq!(
|
||||
git_info.commit_hash.as_ref().map(|sha| sha.0.as_str()),
|
||||
|
||||
@@ -23,6 +23,10 @@ use crate::ThreadSortKey;
|
||||
use crate::ThreadStoreError;
|
||||
use crate::ThreadStoreResult;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "search_threads_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
struct ThreadSearchItem {
|
||||
item: codex_rollout::ThreadItem,
|
||||
snippet: String,
|
||||
@@ -50,6 +54,7 @@ pub(super) async fn search_threads(
|
||||
let sort_key = match params.sort_key {
|
||||
ThreadSortKey::CreatedAt => codex_rollout::ThreadSortKey::CreatedAt,
|
||||
ThreadSortKey::UpdatedAt => codex_rollout::ThreadSortKey::UpdatedAt,
|
||||
ThreadSortKey::RecencyAt => codex_rollout::ThreadSortKey::RecencyAt,
|
||||
};
|
||||
let sort_direction = match params.sort_direction {
|
||||
SortDirection::Asc => codex_rollout::SortDirection::Asc,
|
||||
@@ -179,8 +184,17 @@ fn cursor_from_thread_search_item(
|
||||
.updated_at
|
||||
.as_deref()
|
||||
.or(item.item.created_at.as_deref())?,
|
||||
ThreadSortKey::RecencyAt => item
|
||||
.item
|
||||
.recency_at
|
||||
.as_deref()
|
||||
.or(item.item.updated_at.as_deref())
|
||||
.or(item.item.created_at.as_deref())?,
|
||||
};
|
||||
parse_cursor(timestamp)
|
||||
match sort_key {
|
||||
ThreadSortKey::RecencyAt => parse_cursor(&format!("{timestamp}|{}", item.item.thread_id?)),
|
||||
ThreadSortKey::CreatedAt | ThreadSortKey::UpdatedAt => parse_cursor(timestamp),
|
||||
}
|
||||
}
|
||||
|
||||
async fn set_thread_search_result_names(
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_rollout::ThreadItem;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::ThreadSearchItem;
|
||||
use super::cursor_from_thread_search_item;
|
||||
use crate::ThreadSortKey;
|
||||
|
||||
#[test]
|
||||
fn recency_cursor_includes_thread_id_tie_breaker() {
|
||||
let thread_id = ThreadId::from_string("00000000-0000-0000-0000-000000000123")
|
||||
.expect("thread ID should parse");
|
||||
let item = ThreadSearchItem {
|
||||
item: ThreadItem {
|
||||
thread_id: Some(thread_id),
|
||||
recency_at: Some("2026-01-27T12:34:56Z".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
snippet: String::new(),
|
||||
};
|
||||
|
||||
let cursor = cursor_from_thread_search_item(&item, ThreadSortKey::RecencyAt)
|
||||
.expect("cursor should build");
|
||||
|
||||
assert_eq!(
|
||||
serde_json::to_string(&cursor).expect("cursor should serialize"),
|
||||
format!("\"2026-01-27T12:34:56Z|{thread_id}\"")
|
||||
);
|
||||
}
|
||||
@@ -196,5 +196,6 @@ mod tests {
|
||||
.expect("thread metadata should exist");
|
||||
assert_eq!(updated.rollout_path, restored_path);
|
||||
assert_eq!(updated.archived_at, None);
|
||||
assert_eq!(updated.recency_at, metadata.recency_at);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -204,6 +204,7 @@ async fn apply_metadata_update(
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to read thread metadata for {thread_id}: {err}"),
|
||||
})?;
|
||||
let advance_recency_at = patch.advance_recency_at;
|
||||
if existing.is_none() && rollout_path.is_none() {
|
||||
let resolved = resolve_rollout_path(store, thread_id, include_archived).await?;
|
||||
rollout_path_archived = resolved.archived;
|
||||
@@ -260,6 +261,11 @@ async fn apply_metadata_update(
|
||||
if let Some(updated_at) = patch.updated_at {
|
||||
metadata.updated_at = updated_at;
|
||||
}
|
||||
if existing.is_none()
|
||||
&& let Some(recency_at) = advance_recency_at
|
||||
{
|
||||
metadata.recency_at = recency_at;
|
||||
}
|
||||
if let Some(source) = patch.source {
|
||||
metadata.source = enum_to_string(&source);
|
||||
}
|
||||
@@ -310,6 +316,18 @@ async fn apply_metadata_update(
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to update thread metadata for {thread_id}: {err}"),
|
||||
})?;
|
||||
if existing.is_some()
|
||||
&& let Some(recency_at) = advance_recency_at
|
||||
{
|
||||
state_db
|
||||
.touch_thread_recency_at(thread_id, recency_at)
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!(
|
||||
"failed to advance thread recency_at for {thread_id}: {err}"
|
||||
),
|
||||
})?;
|
||||
}
|
||||
if let Some(memory_mode) = patch.memory_mode {
|
||||
state_db
|
||||
.set_thread_memory_mode(thread_id, memory_mode_as_str(memory_mode))
|
||||
|
||||
@@ -156,7 +156,10 @@ impl ThreadMetadataSync {
|
||||
let affects_metadata = items
|
||||
.iter()
|
||||
.any(codex_state::rollout_item_affects_thread_metadata);
|
||||
let update = if affects_metadata {
|
||||
let advances_recency = items
|
||||
.iter()
|
||||
.any(|item| matches!(item, RolloutItem::EventMsg(EventMsg::TurnStarted(_))));
|
||||
let mut update = if affects_metadata {
|
||||
self.observe_items(items)?
|
||||
} else {
|
||||
Some(thread_updated_at_touch())
|
||||
@@ -164,6 +167,9 @@ impl ThreadMetadataSync {
|
||||
let Some(update) = update else {
|
||||
return Ok(None);
|
||||
};
|
||||
if advances_recency {
|
||||
update.advance_recency_at = Some(Utc::now());
|
||||
}
|
||||
self.merge_pending_update(Some(update));
|
||||
if !affects_metadata
|
||||
&& !self
|
||||
@@ -367,6 +373,7 @@ fn update_has_metadata_facts(update: &ThreadMetadataPatch) -> bool {
|
||||
|| update.model.is_some()
|
||||
|| update.reasoning_effort.is_some()
|
||||
|| update.created_at.is_some()
|
||||
|| update.advance_recency_at.is_some()
|
||||
|| update.source.is_some()
|
||||
|| update.thread_source.is_some()
|
||||
|| update.agent_nickname.is_some()
|
||||
@@ -399,6 +406,7 @@ mod tests {
|
||||
use codex_protocol::protocol::ThreadGoal;
|
||||
use codex_protocol::protocol::ThreadGoalStatus;
|
||||
use codex_protocol::protocol::ThreadGoalUpdatedEvent;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
@@ -522,6 +530,27 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_start_advances_recency_at_without_changing_updated_at_behavior() {
|
||||
let thread_id = ThreadId::new();
|
||||
let mut sync = ThreadMetadataSync::for_resume(&resume_params(thread_id, Vec::new()));
|
||||
|
||||
let update = sync
|
||||
.observe_appended_items(&[RolloutItem::EventMsg(EventMsg::TurnStarted(
|
||||
TurnStartedEvent {
|
||||
turn_id: "turn-1".to_string(),
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
},
|
||||
))])
|
||||
.expect("turn start metadata update");
|
||||
|
||||
assert!(update.patch.updated_at.is_some());
|
||||
assert!(update.patch.advance_recency_at.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_history_waits_for_append_before_flushing_metadata() {
|
||||
let thread_id = ThreadId::new();
|
||||
|
||||
@@ -160,6 +160,8 @@ pub enum ThreadSortKey {
|
||||
CreatedAt,
|
||||
/// Sort by the thread last-update timestamp.
|
||||
UpdatedAt,
|
||||
/// Sort by the thread's product recency timestamp.
|
||||
RecencyAt,
|
||||
}
|
||||
|
||||
/// The direction to use when listing stored threads.
|
||||
@@ -397,6 +399,8 @@ pub struct StoredThread {
|
||||
pub created_at: DateTime<Utc>,
|
||||
/// Thread last-update timestamp.
|
||||
pub updated_at: DateTime<Utc>,
|
||||
/// Thread product-recency timestamp.
|
||||
pub recency_at: DateTime<Utc>,
|
||||
/// Thread archive timestamp, if archived.
|
||||
pub archived_at: Option<DateTime<Utc>>,
|
||||
/// Working directory captured for the thread.
|
||||
@@ -504,6 +508,8 @@ pub struct ThreadMetadataPatch {
|
||||
pub created_at: Option<DateTime<Utc>>,
|
||||
/// Last update timestamp for this metadata observation.
|
||||
pub updated_at: Option<DateTime<Utc>>,
|
||||
/// Advance product recency to at least this timestamp.
|
||||
pub advance_recency_at: Option<DateTime<Utc>>,
|
||||
/// Session source.
|
||||
pub source: Option<SessionSource>,
|
||||
/// Optional analytics source classification.
|
||||
@@ -586,6 +592,9 @@ impl ThreadMetadataPatch {
|
||||
if next.updated_at.is_some() {
|
||||
self.updated_at = next.updated_at;
|
||||
}
|
||||
if next.advance_recency_at.is_some() {
|
||||
self.advance_recency_at = next.advance_recency_at;
|
||||
}
|
||||
if next.source.is_some() {
|
||||
self.source = next.source;
|
||||
}
|
||||
@@ -639,6 +648,7 @@ impl ThreadMetadataPatch {
|
||||
&& self.reasoning_effort.is_none()
|
||||
&& self.created_at.is_none()
|
||||
&& self.updated_at.is_none()
|
||||
&& self.advance_recency_at.is_none()
|
||||
&& self.source.is_none()
|
||||
&& self.thread_source.is_none()
|
||||
&& self.agent_nickname.is_none()
|
||||
|
||||
Reference in New Issue
Block a user