mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
state: use compact guardian thread metadata
This commit is contained in:
@@ -1144,6 +1144,13 @@ async fn read_head_summary(path: &Path, head_limit: usize) -> io::Result<HeadTai
|
||||
summary.cli_version = Some(session_meta_line.meta.cli_version);
|
||||
summary.created_at = Some(session_meta_line.meta.timestamp.clone());
|
||||
summary.saw_session_meta = true;
|
||||
|
||||
if codex_state::is_guardian_review_source(&session_meta_line.meta.source) {
|
||||
// Guardian user messages are synthetic review prompts. SessionMeta plus
|
||||
// the compact preview form the full summary, so do not scan the prompt.
|
||||
summary.preview = Some(codex_state::GUARDIAN_THREAD_PREVIEW.to_string());
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
RolloutItem::ResponseItem(_) | RolloutItem::InterAgentCommunication(_) => {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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::<SessionSource>(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<String>) {
|
||||
@@ -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();
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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::<Utc>::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::<Utc>::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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user