mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Preserve renamed thread titles during reconciliation (#25624)
## Summary - preserve existing explicit SQLite thread titles during rollout reconciliation/backfill when the incoming rollout title is only first-message-derived - keep stale inferred-title repair behavior while avoiding session-index scans during startup backfill - add a regression test for renamed titles surviving reconcile ## Testing - just fmt - just test -p codex-rollout - just test -p codex-state
This commit is contained in:
committed by
GitHub
Unverified
parent
f1d029cf75
commit
3cdce52865
@@ -259,6 +259,7 @@ 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;
|
||||
|
||||
@@ -1061,8 +1061,8 @@ async fn list_threads_search_repairs_stale_state_db_hits_before_returning() -> s
|
||||
builder.model_provider = Some(config.model_provider_id.clone());
|
||||
builder.cwd = home.path().to_path_buf();
|
||||
let mut metadata = builder.build(config.model_provider_id.as_str());
|
||||
metadata.title = "needle stale title".to_string();
|
||||
metadata.first_user_message = Some("stale first user".to_string());
|
||||
metadata.title = "needle stale first user".to_string();
|
||||
metadata.first_user_message = Some(metadata.title.clone());
|
||||
metadata.preview = metadata.first_user_message.clone();
|
||||
runtime
|
||||
.upsert_thread(&metadata)
|
||||
|
||||
@@ -514,6 +514,7 @@ 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() => {
|
||||
|
||||
@@ -6,7 +6,13 @@ use chrono::DateTime;
|
||||
use chrono::NaiveDateTime;
|
||||
use chrono::Timelike;
|
||||
use chrono::Utc;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
use codex_protocol::protocol::SessionMeta;
|
||||
use codex_protocol::protocol::SessionMetaLine;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
@@ -84,3 +90,91 @@ async fn try_init_times_out_waiting_for_stuck_startup_backfill() -> anyhow::Resu
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_rollout_preserves_existing_explicit_title() -> 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")?;
|
||||
let runtime =
|
||||
codex_state::StateRuntime::init(home.path().to_path_buf(), "test-provider".to_string())
|
||||
.await?;
|
||||
|
||||
let mut metadata =
|
||||
metadata::extract_metadata_from_rollout(rollout_path.as_path(), "test-provider")
|
||||
.await?
|
||||
.metadata;
|
||||
assert_eq!(metadata.title, "Hey");
|
||||
assert_eq!(metadata.first_user_message.as_deref(), Some("Hey"));
|
||||
metadata.title = "math".to_string();
|
||||
runtime.upsert_thread(&metadata).await?;
|
||||
|
||||
reconcile_rollout(
|
||||
Some(runtime.as_ref()),
|
||||
rollout_path.as_path(),
|
||||
"test-provider",
|
||||
/*builder*/ None,
|
||||
&[],
|
||||
/*archived_only*/ Some(false),
|
||||
/*new_thread_memory_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
let persisted = runtime
|
||||
.get_thread(thread_id)
|
||||
.await?
|
||||
.expect("thread should exist");
|
||||
assert_eq!(persisted.title, "math");
|
||||
assert_eq!(persisted.first_user_message.as_deref(), Some("Hey"));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_rollout_with_user_message(
|
||||
home: &Path,
|
||||
thread_id: ThreadId,
|
||||
message: &str,
|
||||
) -> anyhow::Result<std::path::PathBuf> {
|
||||
let dir = home.join("sessions/2026/06/01");
|
||||
std::fs::create_dir_all(dir.as_path())?;
|
||||
let path = dir.join(format!("rollout-2026-06-01T14-26-25-{thread_id}.jsonl"));
|
||||
let lines = [
|
||||
RolloutLine {
|
||||
timestamp: "2026-06-01T14:26:25Z".to_string(),
|
||||
item: RolloutItem::SessionMeta(SessionMetaLine {
|
||||
meta: SessionMeta {
|
||||
id: thread_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id: None,
|
||||
timestamp: "2026-06-01T14:26:25Z".to_string(),
|
||||
cwd: home.to_path_buf(),
|
||||
originator: "test".to_string(),
|
||||
cli_version: "test".to_string(),
|
||||
source: SessionSource::Cli,
|
||||
thread_source: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
agent_path: None,
|
||||
model_provider: Some("test-provider".to_string()),
|
||||
base_instructions: None,
|
||||
dynamic_tools: None,
|
||||
memory_mode: None,
|
||||
},
|
||||
git: None,
|
||||
}),
|
||||
},
|
||||
RolloutLine {
|
||||
timestamp: "2026-06-01T14:26:26Z".to_string(),
|
||||
item: RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
|
||||
message: message.to_string(),
|
||||
..Default::default()
|
||||
})),
|
||||
},
|
||||
];
|
||||
let jsonl = lines
|
||||
.iter()
|
||||
.map(serde_json::to_string)
|
||||
.collect::<Result<Vec<_>, _>>()?
|
||||
.join("\n");
|
||||
std::fs::write(path.as_path(), format!("{jsonl}\n"))?;
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
@@ -239,6 +239,21 @@ 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();
|
||||
|
||||
Reference in New Issue
Block a user