mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Restore thread recency with compatible migration history (#28671)
## Summary - Revert #28655, restoring the thread `recencyAt` behavior introduced by #27910. - Move `threads_recency_at` to migration 0039 so it no longer collides with `external_agent_config_imports` at version 0038. - Repair databases that already applied the recency migration as version 38 by moving the matching migration-history row to version 39 before SQLx validation. The current version-38 migration can then apply normally. ## Validation - `just test -p codex-state migrations::tests::repairs_recency_migration_that_was_applied_as_version_38` - `just test -p codex-state -p codex-rollout -p codex-thread-store -p codex-app-server-protocol -p codex-tui`: 3,439 passed; six TUI tests could not open the machine's existing read-only incident database at `~/.codex/sqlite/state_5.sqlite`. - `just fix -p codex-state` - `just fmt` - Verified that state migration versions are unique.
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,92 @@ 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()),
|
||||
metadata: None,
|
||||
}),
|
||||
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))
|
||||
|
||||
@@ -154,11 +154,17 @@ 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 {
|
||||
thread_updated_at_touch()
|
||||
};
|
||||
if advances_recency {
|
||||
update.advance_recency_at = Some(Utc::now());
|
||||
}
|
||||
self.merge_pending_update(Some(update));
|
||||
if !affects_metadata
|
||||
&& !self
|
||||
@@ -347,6 +353,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()
|
||||
@@ -379,6 +386,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;
|
||||
|
||||
@@ -495,6 +503,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