Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 52 additions & 14 deletions codex-rs/app-server/tests/suite/v2/external_agent_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "<ide_selection>src/auth.rs:1-5</ide_selection>";
let first_request = "Fix auth flow";
std::fs::create_dir_all(&project_root)?;
std::fs::create_dir_all(&session_dir)?;
std::fs::write(
Expand All @@ -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(),
]
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand All @@ -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: "<EXTERNAL SESSION IMPORTED>".into(),
phase: None,
memory_citation: None,
Expand Down Expand Up @@ -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:?}"),
}
Expand Down
105 changes: 102 additions & 3 deletions codex-rs/external-agent-sessions/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 = "<system-reminder>\ncontrol context\n</system-reminder>\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("<system-reminder>")
);
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 = "<ide_selection>src/auth.rs:1-5</ide_selection>";
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",
"<ide_selection>src/auth.rs:1-5</ide_selection>",
&project_root,
),
record(
"user",
"<local-command-stderr>tests failed</local-command-stderr>",
&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");
Expand Down
1 change: 1 addition & 0 deletions codex-rs/external-agent-sessions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod detect;
mod export;
mod ledger;
mod records;
mod title;

use codex_protocol::protocol::RolloutItem;
use std::io;
Expand Down
32 changes: 24 additions & 8 deletions codex-rs/external-agent-sessions/src/records.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -25,7 +27,8 @@ pub struct SessionSummary {

pub(super) struct ParsedSessionImport {
pub cwd: Option<PathBuf>,
pub source_title: Option<String>,
pub custom_title: Option<String>,
pub ai_title: Option<String>,
pub messages: Vec<ConversationMessage>,
pub content_sha256: String,
}
Expand All @@ -36,7 +39,8 @@ pub fn summarize_session(path: &Path) -> io::Result<Option<SessionSummary>> {
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;

Expand Down Expand Up @@ -65,8 +69,11 @@ pub fn summarize_session(path: &Path) -> io::Result<Option<SessionSummary>> {
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 =
Expand All @@ -88,7 +95,14 @@ pub fn summarize_session(path: &Path) -> io::Result<Option<SessionSummary>> {
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(),
},
}))
}
Expand Down Expand Up @@ -133,7 +147,8 @@ pub(super) fn read_session_import(path: &Path) -> io::Result<ParsedSessionImport
}
Ok(ParsedSessionImport {
cwd,
source_title: custom_title.or(ai_title),
custom_title,
ai_title,
messages,
content_sha256: format!("{:x}", hasher.finalize()),
})
Expand Down Expand Up @@ -373,7 +388,8 @@ mod tests {
let parsed = read_session_import(&path).expect("parse session");

assert_eq!(parsed.cwd.as_deref(), Some(root.path()));
assert_eq!(parsed.source_title.as_deref(), Some("custom title"));
assert_eq!(parsed.custom_title.as_deref(), Some("custom title"));
assert_eq!(parsed.ai_title.as_deref(), Some("generated title"));
assert_eq!(parsed.messages.len(), 1);
assert_eq!(parsed.messages[0].text, "first request");
assert_eq!(
Expand Down
Loading
Loading