diff --git a/codex-rs/rollout/src/list.rs b/codex-rs/rollout/src/list.rs index f5013e7aae..9dea2ce072 100644 --- a/codex-rs/rollout/src/list.rs +++ b/codex-rs/rollout/src/list.rs @@ -1144,6 +1144,13 @@ async fn read_head_summary(path: &Path, head_limit: usize) -> io::Result { diff --git a/codex-rs/rollout/src/tests.rs b/codex-rs/rollout/src/tests.rs index e6a3c728ef..8db371b3e3 100644 --- a/codex-rs/rollout/src/tests.rs +++ b/codex-rs/rollout/src/tests.rs @@ -37,6 +37,7 @@ use codex_protocol::protocol::RolloutLine; use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; +use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::ThreadGoal; use codex_protocol::protocol::ThreadGoalStatus; use codex_protocol::protocol::ThreadGoalUpdatedEvent; @@ -981,6 +982,36 @@ async fn test_list_threads_uses_goal_objective_as_preview() { assert_eq!(item.first_user_message, None); } +#[tokio::test] +async fn test_guardian_rollout_uses_compact_preview() { + let temp = TempDir::new().unwrap(); + let home = temp.path(); + + let uuid = Uuid::from_u128(102); + let ts = "2025-05-03T10-30-00"; + write_session_file( + home, + ts, + uuid, + /*num_records*/ 1, + Some(SessionSource::SubAgent(SubAgentSource::Other( + "guardian".to_string(), + ))), + ) + .unwrap(); + + let path = home.join(format!("sessions/2025/05/03/rollout-{ts}-{uuid}.jsonl")); + let item = crate::read_thread_item_from_rollout(path) + .await + .expect("guardian rollout should produce a thread item"); + + assert_eq!( + item.preview.as_deref(), + Some(codex_state::GUARDIAN_THREAD_PREVIEW) + ); + assert_eq!(item.first_user_message, None); +} + #[tokio::test] async fn test_goal_first_thread_reads_later_user_message() { let temp = TempDir::new().unwrap(); diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index 5b8388bf83..2f2472be90 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -3,6 +3,8 @@ use codex_protocol::models::ResponseItem; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionMetaLine; +use codex_protocol::protocol::SessionSource; +use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::TurnContextItem; use codex_protocol::protocol::USER_MESSAGE_BEGIN; use codex_protocol::protocol::UserMessageEvent; @@ -10,6 +12,26 @@ use serde::Serialize; use serde_json::Value; const IMAGE_ONLY_USER_MESSAGE_PLACEHOLDER: &str = "[Image]"; +pub const GUARDIAN_THREAD_TITLE: &str = "Guardian review"; +pub const GUARDIAN_THREAD_PREVIEW: &str = "Approval review"; + +pub fn is_guardian_review_source(source: &SessionSource) -> bool { + matches!( + source, + SessionSource::SubAgent(SubAgentSource::Other(name)) if name == "guardian" + ) +} + +pub(crate) fn apply_guardian_thread_metadata_defaults(metadata: &mut ThreadMetadata) { + // Empty titles and titles copied from first_user_message are derived; preserve + // any other title as potentially explicit. + let title = metadata.title.trim(); + if title.is_empty() || metadata.first_user_message.as_deref().map(str::trim) == Some(title) { + metadata.title = GUARDIAN_THREAD_TITLE.to_string(); + } + metadata.preview = Some(GUARDIAN_THREAD_PREVIEW.to_string()); + metadata.first_user_message = None; +} /// Apply a rollout item to the metadata structure. pub fn apply_rollout_item( @@ -70,6 +92,9 @@ fn apply_session_meta_from_item(metadata: &mut ThreadMetadata, meta_line: &Sessi metadata.git_branch = git.branch.clone(); metadata.git_origin_url = git.repository_url.clone(); } + if is_guardian_review_source(&meta_line.meta.source) { + apply_guardian_thread_metadata_defaults(metadata); + } } fn apply_turn_context(metadata: &mut ThreadMetadata, turn_ctx: &TurnContextItem) { @@ -91,6 +116,9 @@ fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) { } } EventMsg::UserMessage(user) => { + if metadata_is_guardian_review(metadata) { + return; + } let preview = user_message_preview(user); if metadata.first_user_message.is_none() { metadata.first_user_message = preview.clone(); @@ -104,6 +132,9 @@ fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) { } } EventMsg::ThreadGoalUpdated(event) => { + if metadata_is_guardian_review(metadata) { + return; + } let objective = event.goal.objective.trim(); if !objective.is_empty() { set_preview_if_empty(metadata, Some(objective.to_string())); @@ -113,6 +144,12 @@ fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) { } } +pub(crate) fn metadata_is_guardian_review(metadata: &ThreadMetadata) -> bool { + serde_json::from_str::(metadata.source.as_str()) + .as_ref() + .is_ok_and(is_guardian_review_source) +} + fn apply_response_item(_metadata: &mut ThreadMetadata, _item: &ResponseItem) {} fn set_preview_if_empty(metadata: &mut ThreadMetadata, preview: Option) { @@ -170,6 +207,7 @@ mod tests { use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; + use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::ThreadGoal; use codex_protocol::protocol::ThreadGoalStatus; use codex_protocol::protocol::ThreadGoalUpdatedEvent; @@ -223,6 +261,73 @@ mod tests { assert_eq!(metadata.title, "actual user request"); } + #[test] + fn guardian_user_messages_use_defaults_without_projection() { + let mut metadata = metadata_for_test(); + let thread_id = metadata.id; + apply_rollout_item( + &mut metadata, + &RolloutItem::SessionMeta(SessionMetaLine { + meta: SessionMeta { + session_id: thread_id.into(), + id: thread_id, + source: SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())), + ..Default::default() + }, + git: None, + }), + "test-provider", + ); + apply_rollout_item( + &mut metadata, + &RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent { + client_id: None, + message: "large synthetic guardian prompt".to_string(), + images: Some(vec![]), + local_images: vec![], + text_elements: vec![], + ..Default::default() + })), + "test-provider", + ); + + assert_eq!(metadata.title, super::GUARDIAN_THREAD_TITLE); + assert_eq!( + metadata.preview.as_deref(), + Some(super::GUARDIAN_THREAD_PREVIEW) + ); + assert_eq!(metadata.first_user_message, None); + } + + #[test] + fn guardian_session_meta_preserves_existing_explicit_title() { + let mut metadata = metadata_for_test(); + let thread_id = metadata.id; + metadata.title = "Named Guardian review".to_string(); + metadata.first_user_message = Some("large synthetic guardian prompt".to_string()); + + apply_rollout_item( + &mut metadata, + &RolloutItem::SessionMeta(SessionMetaLine { + meta: SessionMeta { + session_id: thread_id.into(), + id: thread_id, + source: SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())), + ..Default::default() + }, + git: None, + }), + "test-provider", + ); + + assert_eq!(metadata.title, "Named Guardian review"); + assert_eq!( + metadata.preview.as_deref(), + Some(super::GUARDIAN_THREAD_PREVIEW) + ); + assert_eq!(metadata.first_user_message, None); + } + #[test] fn event_msg_image_only_user_message_sets_image_placeholder_preview() { let mut metadata = metadata_for_test(); diff --git a/codex-rs/state/src/lib.rs b/codex-rs/state/src/lib.rs index 6b5b907653..72fb52dd4c 100644 --- a/codex-rs/state/src/lib.rs +++ b/codex-rs/state/src/lib.rs @@ -27,10 +27,13 @@ pub use runtime::StateRuntime; pub use audit::ThreadStateAuditRow; pub use audit::read_thread_state_audit_rows; +pub use extract::GUARDIAN_THREAD_PREVIEW; +pub use extract::GUARDIAN_THREAD_TITLE; /// Low-level storage engine: useful for focused tests. /// /// Most consumers should prefer [`StateRuntime`]. pub use extract::apply_rollout_item; +pub use extract::is_guardian_review_source; pub use extract::rollout_item_affects_thread_metadata; pub use model::AgentJob; pub use model::AgentJobCreateParams; diff --git a/codex-rs/state/src/model/thread_metadata.rs b/codex-rs/state/src/model/thread_metadata.rs index c25c458986..c3c38264e7 100644 --- a/codex-rs/state/src/model/thread_metadata.rs +++ b/codex-rs/state/src/model/thread_metadata.rs @@ -203,7 +203,7 @@ impl ThreadMetadataBuilder { .recency_at .map(canonicalize_datetime) .unwrap_or(updated_at); - ThreadMetadata { + let mut metadata = ThreadMetadata { id: self.id, rollout_path: self.rollout_path.clone(), created_at, @@ -235,7 +235,11 @@ impl ThreadMetadataBuilder { git_sha: self.git_sha.clone(), git_branch: self.git_branch.clone(), git_origin_url: self.git_origin_url.clone(), + }; + if crate::extract::is_guardian_review_source(&self.source) { + crate::extract::apply_guardian_thread_metadata_defaults(&mut metadata); } + metadata } } @@ -263,7 +267,13 @@ impl ThreadMetadata { } let title = self.title.trim(); - if title.is_empty() || self.first_user_message.as_deref().map(str::trim) == Some(title) { + // Treat the compact default as derived only for Guardian threads; a + // non-Guardian thread may explicitly use the same title. + if title.is_empty() + || self.first_user_message.as_deref().map(str::trim) == Some(title) + || (title == crate::GUARDIAN_THREAD_TITLE + && crate::extract::metadata_is_guardian_review(self)) + { self.title = existing.title.clone(); } } @@ -527,11 +537,14 @@ pub struct BackfillStats { #[cfg(test)] mod tests { use super::ThreadMetadata; + use super::ThreadMetadataBuilder; use super::ThreadRow; use chrono::DateTime; use chrono::Utc; use codex_protocol::ThreadId; use codex_protocol::openai_models::ReasoningEffort; + use codex_protocol::protocol::SessionSource; + use codex_protocol::protocol::SubAgentSource; use pretty_assertions::assert_eq; use std::path::PathBuf; @@ -617,4 +630,45 @@ mod tests { expected_thread_metadata(Some(ReasoningEffort::Custom("future".to_string()))) ); } + + #[test] + fn guardian_thread_builder_uses_contextual_defaults() { + let id = + ThreadId::from_string("00000000-0000-0000-0000-000000000123").expect("valid thread id"); + let created_at = DateTime::::from_timestamp(1_700_000_000, 0).expect("timestamp"); + let metadata = ThreadMetadataBuilder::new( + id, + PathBuf::from("/tmp/rollout-123.jsonl"), + created_at, + SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())), + ) + .build("openai"); + + assert_eq!(metadata.title, crate::GUARDIAN_THREAD_TITLE); + assert_eq!( + metadata.preview.as_deref(), + Some(crate::GUARDIAN_THREAD_PREVIEW) + ); + assert_eq!(metadata.first_user_message, None); + } + + #[test] + fn guardian_default_title_does_not_replace_existing_explicit_title() { + let id = + ThreadId::from_string("00000000-0000-0000-0000-000000000123").expect("valid thread id"); + let created_at = DateTime::::from_timestamp(1_700_000_000, 0).expect("timestamp"); + let mut metadata = ThreadMetadataBuilder::new( + id, + PathBuf::from("/tmp/rollout-123.jsonl"), + created_at, + SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())), + ) + .build("openai"); + let mut existing = metadata.clone(); + existing.title = "Named Guardian review".to_string(); + + metadata.prefer_existing_explicit_title(&existing); + + assert_eq!(metadata.title, "Named Guardian review"); + } } diff --git a/codex-rs/thread-store/src/thread_metadata_sync.rs b/codex-rs/thread-store/src/thread_metadata_sync.rs index 852f679126..5679c99a60 100644 --- a/codex-rs/thread-store/src/thread_metadata_sync.rs +++ b/codex-rs/thread-store/src/thread_metadata_sync.rs @@ -63,7 +63,8 @@ impl ThreadMetadataSync { } else { None }; - let update = ThreadMetadataPatch { + let guardian_review = codex_state::is_guardian_review_source(¶ms.source); + let mut update = ThreadMetadataPatch { model_provider: Some(params.metadata.model_provider.clone()), created_at: Some(created_at), updated_at: Some(created_at), @@ -78,12 +79,18 @@ impl ThreadMetadataSync { memory_mode: Some(params.metadata.memory_mode), ..Default::default() }; + if guardian_review { + update.title = Some(codex_state::GUARDIAN_THREAD_TITLE.to_string()); + update.preview = Some(codex_state::GUARDIAN_THREAD_PREVIEW.to_string()); + } Self { thread_id: params.thread_id, cwd_seen: !cwd.as_os_str().is_empty(), - preview_seen: false, - first_user_message_seen: false, - title_seen: false, + // Guardian prompts are synthetic, so later UserMessage items must not + // rederive these fields. + preview_seen: guardian_review, + first_user_message_seen: guardian_review, + title_seen: guardian_review, pending_update: Some(update), pending_update_generation: 1, last_touch_persisted_at: None, @@ -211,6 +218,14 @@ impl ThreadMetadataSync { for item in items { match item { RolloutItem::SessionMeta(meta_line) if meta_line.meta.id == self.thread_id => { + // The first matching SessionMeta precedes this thread's user messages, so + // mark synthetic fields settled before observing the prompt. + if codex_state::is_guardian_review_source(&meta_line.meta.source) { + self.preview_seen = true; + self.first_user_message_seen = true; + self.title_seen = true; + update.preview = Some(codex_state::GUARDIAN_THREAD_PREVIEW.to_string()); + } update.created_at = parse_session_timestamp(meta_line.meta.timestamp.as_str()); update.source = Some(meta_line.meta.source.clone()); update.thread_source = Some(meta_line.meta.thread_source.clone()); @@ -249,7 +264,11 @@ impl ThreadMetadataSync { update.permission_profile = Some(turn_ctx.permission_profile()); } RolloutItem::EventMsg(EventMsg::UserMessage(user)) => { - if let Some(preview) = user_message_preview(user) { + // Only preview and first_user_message use the allocated preview; + // title derivation below borrows the message. + if (!self.first_user_message_seen || !self.preview_seen) + && let Some(preview) = user_message_preview(user) + { if !self.first_user_message_seen { self.first_user_message_seen = true; update.first_user_message = Some(preview.clone()); @@ -389,16 +408,83 @@ mod tests { use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; + use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::ThreadGoal; use codex_protocol::protocol::ThreadGoalStatus; use codex_protocol::protocol::ThreadGoalUpdatedEvent; + use codex_protocol::protocol::ThreadSource; use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::protocol::UserMessageEvent; use pretty_assertions::assert_eq; use super::*; + use crate::CreateThreadParams; use crate::ThreadPersistenceMetadata; + #[tokio::test] + async fn guardian_create_uses_defaults_without_projecting_prompt() { + let thread_id = ThreadId::new(); + let mut sync = ThreadMetadataSync::for_create(&CreateThreadParams { + session_id: thread_id.into(), + thread_id, + extra_config: None, + forked_from_id: None, + parent_thread_id: Some(ThreadId::new()), + source: guardian_source(), + thread_source: Some(ThreadSource::Subagent), + base_instructions: Default::default(), + dynamic_tools: Vec::new(), + multi_agent_version: None, + initial_window_id: uuid::Uuid::now_v7().to_string(), + metadata: ThreadPersistenceMetadata { + cwd: None, + model_provider: "test-provider".to_string(), + memory_mode: ThreadMemoryMode::Enabled, + }, + }) + .await; + + let update = sync + .observe_appended_items(&[RolloutItem::EventMsg(EventMsg::UserMessage(user_message( + "large synthetic guardian prompt", + )))]) + .expect("guardian metadata update"); + + assert_eq!( + update.patch.title.as_deref(), + Some(codex_state::GUARDIAN_THREAD_TITLE) + ); + assert_eq!( + update.patch.preview.as_deref(), + Some(codex_state::GUARDIAN_THREAD_PREVIEW) + ); + assert_eq!(update.patch.first_user_message, None); + } + + #[test] + fn guardian_resume_does_not_emit_default_title() { + let thread_id = ThreadId::new(); + let mut guardian_meta = session_meta(thread_id); + guardian_meta.meta.source = guardian_source(); + let sync = ThreadMetadataSync::for_resume(&resume_params( + thread_id, + vec![ + RolloutItem::SessionMeta(guardian_meta), + RolloutItem::EventMsg(EventMsg::UserMessage(user_message( + "large synthetic guardian prompt", + ))), + ], + )); + + let update = sync.take_pending_update().expect("pending metadata update"); + assert_eq!(update.patch.title, None); + assert_eq!( + update.patch.preview.as_deref(), + Some(codex_state::GUARDIAN_THREAD_PREVIEW) + ); + assert_eq!(update.patch.first_user_message, None); + } + #[test] fn resume_history_keeps_derived_metadata_pending_until_applied() { let thread_id = ThreadId::new(); @@ -595,6 +681,10 @@ mod tests { } } + fn guardian_source() -> SessionSource { + SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())) + } + fn goal_update(thread_id: ThreadId, objective: &str) -> ThreadGoalUpdatedEvent { ThreadGoalUpdatedEvent { thread_id,