diff --git a/codex-rs/app-server/tests/suite/v2/external_agent_config.rs b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs index 6bef968941ad..e83b63661886 100644 --- a/codex-rs/app-server/tests/suite/v2/external_agent_config.rs +++ b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs @@ -615,6 +615,8 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> { let recent_timestamp = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true); let session_dir = external_agent_home(codex_home.path()).join("projects/repo"); let session_path = session_dir.join("session.jsonl"); + let control_request = "src/auth.rs:1-5"; + let first_request = "Fix auth flow"; std::fs::create_dir_all(&project_root)?; std::fs::create_dir_all(&session_dir)?; std::fs::write( @@ -624,19 +626,21 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> { "type": "user", "cwd": &project_root, "timestamp": &recent_timestamp, - "message": { "content": "first request" }, + "message": { "content": control_request }, }) .to_string(), serde_json::json!({ - "type": "assistant", + "type": "user", "cwd": &project_root, "timestamp": &recent_timestamp, - "message": { "content": "first answer" }, + "message": { "content": first_request }, }) .to_string(), serde_json::json!({ - "type": "custom-title", - "customTitle": "source session title", + "type": "assistant", + "cwd": &project_root, + "timestamp": &recent_timestamp, + "message": { "content": "first answer" }, }) .to_string(), ] @@ -667,6 +671,14 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> { .await??; let detected: ExternalAgentConfigDetectResponse = to_response(response)?; assert_eq!(detected.items.len(), 1); + assert_eq!( + detected.items[0] + .details + .as_ref() + .and_then(|details| details.sessions.first()) + .and_then(|session| session.title.as_deref()), + Some("Fix auth flow") + ); let request_id = mcp .send_raw_request( @@ -743,8 +755,8 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> { .expect("expected imported thread") .clone(); assert_eq!(imported_thread_id, thread.id.to_string()); - assert_eq!(thread.preview, "first request"); - assert_eq!(thread.name.as_deref(), Some("source session title")); + assert_eq!(thread.preview, control_request); + assert_eq!(thread.name.as_deref(), Some("Fix auth flow")); let request_id = mcp .send_thread_read_request(ThreadReadParams { @@ -758,13 +770,39 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> { ) .await??; let response: ThreadReadResponse = to_response(response)?; - assert_eq!(response.thread.turns.len(), 1); - let items = &response.thread.turns[0].items; - assert_eq!(items.len(), 3); + assert_eq!(response.thread.turns.len(), 2); + let control_items = &response.thread.turns[0].items; + assert_eq!(control_items.len(), 1); + match &control_items[0] { + ThreadItem::UserMessage { content, .. } => { + assert_eq!( + content, + &vec![UserInput::Text { + text: control_request.to_string(), + text_elements: Vec::new(), + }] + ); + } + other => panic!("expected user message item, got {other:?}"), + } + let imported_items = &response.thread.turns[1].items; + assert_eq!(imported_items.len(), 3); + match &imported_items[0] { + ThreadItem::UserMessage { content, .. } => { + assert_eq!( + content, + &vec![UserInput::Text { + text: first_request.to_string(), + text_elements: Vec::new(), + }] + ); + } + other => panic!("expected user message item, got {other:?}"), + } assert_eq!( - items.last(), + imported_items.last(), Some(&ThreadItem::AgentMessage { - id: "item-3".into(), + id: "item-4".into(), text: "".into(), phase: None, memory_citation: None, @@ -818,8 +856,8 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> { ) .await??; let response: ThreadReadResponse = to_response(response)?; - assert_eq!(response.thread.turns.len(), 2); - match &response.thread.turns[1].items[1] { + assert_eq!(response.thread.turns.len(), 3); + match &response.thread.turns[2].items[1] { ThreadItem::AgentMessage { text, .. } => assert_eq!(text, "follow-up answer"), other => panic!("expected agent message item, got {other:?}"), } diff --git a/codex-rs/external-agent-sessions/src/export.rs b/codex-rs/external-agent-sessions/src/export.rs index 6391d27bd378..992770f85547 100644 --- a/codex-rs/external-agent-sessions/src/export.rs +++ b/codex-rs/external-agent-sessions/src/export.rs @@ -3,6 +3,9 @@ use crate::ImportedExternalAgentSession; use crate::MessageRole; use crate::records::read_session_import; use crate::summarize_for_label; +use crate::title::IMPORTED_SESSION_FALLBACK_TITLE; +use crate::title::SessionTitleCandidates; +use crate::title::fallback_title_from_user_message; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AgentMessageEvent; @@ -36,11 +39,22 @@ pub(crate) fn load_session_for_import_with_content_sha256( return Ok(None); }; let messages = parsed.messages; - let first_user_message = messages + let first_user_message_text = messages .iter() .find(|message| message.role == MessageRole::User) - .map(|message| summarize_for_label(&message.text)); - let title = parsed.source_title.or_else(|| first_user_message.clone()); + .map(|message| message.text.as_str()); + let first_user_message = first_user_message_text.map(summarize_for_label); + let fallback_title = messages + .iter() + .filter(|message| message.role == MessageRole::User) + .find_map(|message| fallback_title_from_user_message(&message.text)) + .or_else(|| first_user_message_text.map(|_| IMPORTED_SESSION_FALLBACK_TITLE.to_string())); + let title = SessionTitleCandidates { + custom_title: parsed.custom_title, + ai_title: parsed.ai_title, + fallback_title, + } + .select(); let rollout_items = rollout_items_from_messages(messages); if rollout_items.is_empty() { return Ok(None); @@ -373,6 +387,91 @@ mod tests { assert_eq!(imported.title.as_deref(), Some("named by source app")); } + #[test] + fn sanitizes_only_the_imported_session_fallback_title() { + let root = TempDir::new().expect("tempdir"); + let project_root = root.path().join("repo"); + std::fs::create_dir_all(&project_root).expect("project root"); + let path = root.path().join("session.jsonl"); + let message = "\ncontrol context\n\nFix auth flow"; + std::fs::write(&path, jsonl(&[record("user", message, &project_root)])).expect("session"); + + let imported = load_session_for_import(&path) + .expect("load") + .expect("session"); + let imported_user_message = imported.rollout_items.iter().find_map(|item| match item { + RolloutItem::EventMsg(EventMsg::UserMessage(event)) => Some(event.message.as_str()), + _ => None, + }); + + assert_eq!(imported.title.as_deref(), Some("Fix auth flow")); + assert_eq!( + imported.first_user_message.as_deref(), + Some("") + ); + assert_eq!(imported_user_message, Some(message)); + } + + #[test] + fn skips_control_only_user_messages_when_choosing_fallback_title() { + let root = TempDir::new().expect("tempdir"); + let project_root = root.path().join("repo"); + std::fs::create_dir_all(&project_root).expect("project root"); + let path = root.path().join("session.jsonl"); + let control_message = "src/auth.rs:1-5"; + std::fs::write( + &path, + jsonl(&[ + record("user", control_message, &project_root), + record("user", "Fix auth flow", &project_root), + ]), + ) + .expect("session"); + + let imported = load_session_for_import(&path) + .expect("load") + .expect("session"); + + assert_eq!(imported.title.as_deref(), Some("Fix auth flow")); + assert_eq!( + imported.first_user_message.as_deref(), + Some(control_message) + ); + } + + #[test] + fn uses_safe_fallback_after_all_user_messages_are_control_only() { + let root = TempDir::new().expect("tempdir"); + let project_root = root.path().join("repo"); + std::fs::create_dir_all(&project_root).expect("project root"); + let path = root.path().join("session.jsonl"); + std::fs::write( + &path, + jsonl(&[ + record( + "user", + "src/auth.rs:1-5", + &project_root, + ), + record( + "user", + "tests failed", + &project_root, + ), + ]), + ) + .expect("session"); + + let imported = load_session_for_import(&path) + .expect("load") + .expect("session"); + + assert_eq!( + imported.title.as_deref(), + Some(IMPORTED_SESSION_FALLBACK_TITLE) + ); + } + #[test] fn emits_token_usage_for_imported_history() { let root = TempDir::new().expect("tempdir"); diff --git a/codex-rs/external-agent-sessions/src/lib.rs b/codex-rs/external-agent-sessions/src/lib.rs index 0b7a4eb2bac9..b13322dffb44 100644 --- a/codex-rs/external-agent-sessions/src/lib.rs +++ b/codex-rs/external-agent-sessions/src/lib.rs @@ -4,6 +4,7 @@ mod detect; mod export; mod ledger; mod records; +mod title; use codex_protocol::protocol::RolloutItem; use std::io; diff --git a/codex-rs/external-agent-sessions/src/records.rs b/codex-rs/external-agent-sessions/src/records.rs index 00307fa1d52f..2901714a19fb 100644 --- a/codex-rs/external-agent-sessions/src/records.rs +++ b/codex-rs/external-agent-sessions/src/records.rs @@ -1,7 +1,9 @@ use crate::ConversationMessage; use crate::ExternalAgentSessionMigration; use crate::MessageRole; -use crate::summarize_for_label; +use crate::title::IMPORTED_SESSION_FALLBACK_TITLE; +use crate::title::SessionTitleCandidates; +use crate::title::fallback_title_from_user_message; use crate::truncate; use serde_json::Value as JsonValue; use sha2::Digest; @@ -25,7 +27,8 @@ pub struct SessionSummary { pub(super) struct ParsedSessionImport { pub cwd: Option, - pub source_title: Option, + pub custom_title: Option, + pub ai_title: Option, pub messages: Vec, pub content_sha256: String, } @@ -36,7 +39,8 @@ pub fn summarize_session(path: &Path) -> io::Result> { let mut cwd = None; let mut custom_title = None; let mut ai_title = None; - let mut title = None; + let mut fallback_title = None; + let mut saw_user_message = false; let mut latest_timestamp = None; let mut saw_message = false; @@ -65,8 +69,11 @@ pub fn summarize_session(path: &Path) -> io::Result> { continue; }; saw_message = true; - if title.is_none() && message.role == MessageRole::User { - title = Some(summarize_for_label(&message.text)); + if message.role == MessageRole::User { + saw_user_message = true; + if fallback_title.is_none() { + fallback_title = fallback_title_from_user_message(&message.text); + } } if let Some(timestamp) = message.timestamp { latest_timestamp = @@ -88,7 +95,14 @@ pub fn summarize_session(path: &Path) -> io::Result> { migration: ExternalAgentSessionMigration { path: path.to_path_buf(), cwd, - title: custom_title.or(ai_title).or(title), + title: SessionTitleCandidates { + custom_title, + ai_title, + fallback_title: fallback_title.or_else(|| { + saw_user_message.then(|| IMPORTED_SESSION_FALLBACK_TITLE.to_string()) + }), + } + .select(), }, })) } @@ -133,7 +147,8 @@ pub(super) fn read_session_import(path: &Path) -> io::Result", ""), + ("", ""), + ("", ""), + ("", ""), + ("", ""), + ("", ""), + ("", ""), + ("", ""), + ("", ""), + ("", ""), +]; + +pub(super) struct SessionTitleCandidates { + pub custom_title: Option, + pub ai_title: Option, + pub fallback_title: Option, +} + +impl SessionTitleCandidates { + pub fn select(self) -> Option { + self.custom_title.or(self.ai_title).or(self.fallback_title) + } +} + +pub(super) fn fallback_title_from_user_message(message: &str) -> Option { + let message = strip_leading_control_wrappers(message); + message + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .map(|line| truncate(line, SESSION_TITLE_MAX_LEN)) +} + +fn strip_leading_control_wrappers(message: &str) -> &str { + let mut remainder = message.trim_start(); + while let Some(wrapper_end) = leading_control_wrapper_end(remainder) { + remainder = remainder[wrapper_end..].trim_start(); + } + remainder +} + +fn leading_control_wrapper_end(text: &str) -> Option { + let (outer_tag, opening_len) = recognized_opening_tag(text)?; + let mut open_tags = vec![outer_tag]; + let mut cursor = opening_len; + + while !open_tags.is_empty() { + cursor += text.get(cursor..)?.find('<')?; + let candidate = text.get(cursor..)?; + if let Some((tag, token_len)) = recognized_opening_tag(candidate) { + open_tags.push(tag); + cursor += token_len; + continue; + } + if let Some((tag, token_len)) = recognized_closing_tag(candidate) { + if open_tags.last().copied() != Some(tag) { + return None; + } + open_tags.pop(); + cursor += token_len; + continue; + } + cursor += 1; + } + + Some(cursor) +} + +fn recognized_opening_tag(text: &str) -> Option<(usize, usize)> { + RECOGNIZED_CONTROL_WRAPPERS + .iter() + .enumerate() + .find_map(|(index, (opening, _closing))| { + text.starts_with(opening).then_some((index, opening.len())) + }) +} + +fn recognized_closing_tag(text: &str) -> Option<(usize, usize)> { + RECOGNIZED_CONTROL_WRAPPERS + .iter() + .enumerate() + .find_map(|(index, (_opening, closing))| { + text.starts_with(closing).then_some((index, closing.len())) + }) +} + +#[cfg(test)] +#[path = "title_tests.rs"] +mod tests; diff --git a/codex-rs/external-agent-sessions/src/title_tests.rs b/codex-rs/external-agent-sessions/src/title_tests.rs new file mode 100644 index 000000000000..fc068c0e3908 --- /dev/null +++ b/codex-rs/external-agent-sessions/src/title_tests.rs @@ -0,0 +1,123 @@ +use super::*; + +#[test] +fn preserves_valid_custom_title_unchanged() { + let custom_title = "Keep this custom title"; + + assert_eq!( + SessionTitleCandidates { + custom_title: Some(custom_title.to_string()), + ai_title: Some("AI title".to_string()), + fallback_title: Some("fallback title".to_string()), + } + .select(), + Some(custom_title.to_string()) + ); +} + +#[test] +fn preserves_valid_ai_title_unchanged_without_custom_title() { + let ai_title = "Keep this AI title"; + + assert_eq!( + SessionTitleCandidates { + custom_title: None, + ai_title: Some(ai_title.to_string()), + fallback_title: Some("fallback title".to_string()), + } + .select(), + Some(ai_title.to_string()) + ); +} + +#[test] +fn strips_nested_repeated_and_multiline_leading_control_wrappers() { + let message = "\ + \n\ + outer context\n\ + \n\ + nested context\n\ + \n\ + \n\ + \n\ + src/auth.rs\n\ + \n\ + \n\ + Fix auth flow\n\ + Additional details"; + + assert_eq!( + fallback_title_from_user_message(message), + Some("Fix auth flow".to_string()) + ); +} + +#[test] +fn strips_observed_external_agent_control_wrapper_families() { + let cases = [ + "\n\ + abc123\n\ + completed\n\ + \n\ + Fix auth flow", + "review\n\ + /review\n\ + src/auth.rs\n\ + Fix auth flow", + "Command output follows\n\ + tests passed\n\ + Fix auth flow", + "tests failed\n\ + Fix auth flow", + "src/auth.rs:1-5\n\ + Fix auth flow", + ]; + + for message in cases { + assert_eq!( + fallback_title_from_user_message(message), + Some("Fix auth flow".to_string()) + ); + } +} + +#[test] +fn returns_no_candidate_for_empty_or_control_only_messages() { + assert_eq!(fallback_title_from_user_message(""), None); + assert_eq!( + fallback_title_from_user_message( + "review\n\ + context" + ), + None + ); +} + +#[test] +fn uses_first_meaningful_line_from_ordinary_messages() { + assert_eq!( + fallback_title_from_user_message("\n \n Fix auth flow \nAdditional details"), + Some("Fix auth flow".to_string()) + ); +} + +#[test] +fn preserves_unknown_and_user_authored_angle_bracket_text() { + assert_eq!( + fallback_title_from_user_message("Keep this text Fix auth flow"), + Some("Keep this text Fix auth flow".to_string()) + ); + assert_eq!( + fallback_title_from_user_message("Explain tags"), + Some("Explain tags".to_string()) + ); +} + +#[test] +fn bounds_fallback_titles_to_120_characters() { + let message = "x".repeat(121); + let title = fallback_title_from_user_message(&message).expect("title"); + + assert_eq!(title.chars().count(), SESSION_TITLE_MAX_LEN); + assert_eq!(title, format!("{}...", "x".repeat(117))); +}