From c3582c8d9dbfb258c67179c587d871e33ac04517 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 12 Apr 2026 12:18:44 -0700 Subject: [PATCH] Simplify injected message envelopes --- codex-rs/core/src/codex/timer_runtime.rs | 8 +-- codex-rs/core/src/codex_tests.rs | 3 +- codex-rs/core/src/messages.rs | 77 +++++++++++++++--------- codex-rs/core/src/timers.rs | 10 +-- codex-rs/core/tests/suite/timers.rs | 5 +- 5 files changed, 62 insertions(+), 41 deletions(-) diff --git a/codex-rs/core/src/codex/timer_runtime.rs b/codex-rs/core/src/codex/timer_runtime.rs index 38f01a09f0..de1ee1b266 100644 --- a/codex-rs/core/src/codex/timer_runtime.rs +++ b/codex-rs/core/src/codex/timer_runtime.rs @@ -32,6 +32,7 @@ use super::EventMsg; use super::INITIAL_SUBMIT_ID; use super::Session; use crate::messages::MessageInvocationContext; +use crate::messages::MessageInvocationKind; use crate::messages::MessagePayload; use crate::messages::db_message_to_thread_message; use crate::messages::injected_message_event; @@ -381,11 +382,10 @@ impl Session { }; let delivery = message.delivery; let message_context = MessageInvocationContext { + kind: MessageInvocationKind::External, source: message.source.clone(), - content: message.content.clone(), - instructions: message.instructions.clone(), - meta: message.meta.clone(), - queued_at: message.queued_at, + content: message.content, + instructions: None, }; let input_item = PendingInputItem::GeneratedMessage(GeneratedMessageInput { item: message_prompt_input_item(&message_context), diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index d2dc7052f0..252a692815 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -5067,7 +5067,8 @@ async fn generated_message_records_raw_item_and_emits_injected_event() { id: None, role: "user".to_string(), content: vec![ContentItem::InputText { - text: "hidden wrapper".to_string(), + text: "hidden wrapper" + .to_string(), }], end_turn: None, phase: None, diff --git a/codex-rs/core/src/messages.rs b/codex-rs/core/src/messages.rs index 24be092789..6a20fcba11 100644 --- a/codex-rs/core/src/messages.rs +++ b/codex-rs/core/src/messages.rs @@ -8,8 +8,10 @@ use serde::Deserialize; use serde::Serialize; use std::collections::BTreeMap; -pub const CODEX_MESSAGE_OPEN_TAG: &str = ""; -pub const CODEX_MESSAGE_CLOSE_TAG: &str = ""; +const EXTERNAL_MESSAGE_OPEN_TAG: &str = ""; +const EXTERNAL_MESSAGE_CLOSE_TAG: &str = ""; +const TIMER_MESSAGE_OPEN_TAG: &str = ""; +const TIMER_MESSAGE_CLOSE_TAG: &str = ""; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct MessagePayload { @@ -32,13 +34,18 @@ pub struct ThreadMessage { pub queued_at: i64, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum MessageInvocationKind { + External, + Timer { timer_id: String }, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct MessageInvocationContext { + pub(crate) kind: MessageInvocationKind, pub(crate) source: String, pub(crate) content: String, pub(crate) instructions: Option, - pub(crate) meta: BTreeMap, - pub(crate) queued_at: i64, } pub fn validate_meta_key(key: &str) -> Result<(), String> { @@ -81,28 +88,24 @@ pub(crate) fn injected_message_event(message: &MessageInvocationContext) -> Inje pub(crate) fn render_message_prompt(message: &MessageInvocationContext) -> String { let mut rendered = String::new(); - rendered.push_str(CODEX_MESSAGE_OPEN_TAG); - rendered.push('\n'); - push_tag(&mut rendered, "source", &message.source); - push_tag(&mut rendered, "queued_at", &message.queued_at.to_string()); - push_block_tag(&mut rendered, "content", &message.content); - if let Some(instructions) = message.instructions.as_deref() { - push_block_tag(&mut rendered, "instructions", instructions); - } - if message.meta.is_empty() { - rendered.push_str("\n"); - } else { - rendered.push_str("\n"); - for (key, value) in &message.meta { - rendered.push_str(" "); - rendered.push_str(&xml_escape(value)); - rendered.push_str("\n"); + match &message.kind { + MessageInvocationKind::External => { + rendered.push_str(EXTERNAL_MESSAGE_OPEN_TAG); + rendered.push('\n'); + push_block_tag(&mut rendered, "content", &message.content); + rendered.push_str(EXTERNAL_MESSAGE_CLOSE_TAG); + } + MessageInvocationKind::Timer { timer_id } => { + rendered.push_str(TIMER_MESSAGE_OPEN_TAG); + rendered.push('\n'); + push_tag(&mut rendered, "timer_id", timer_id); + push_block_tag(&mut rendered, "content", &message.content); + if let Some(instructions) = message.instructions.as_deref() { + push_block_tag(&mut rendered, "instructions", instructions); + } + rendered.push_str(TIMER_MESSAGE_CLOSE_TAG); } - rendered.push_str("\n"); } - rendered.push_str(CODEX_MESSAGE_CLOSE_TAG); rendered } @@ -162,18 +165,34 @@ mod tests { use pretty_assertions::assert_eq; #[test] - fn renders_message_prompt_with_escaped_metadata() { + fn renders_external_message_prompt_with_only_content() { let message = MessageInvocationContext { - source: "timer timer-1".to_string(), + kind: MessageInvocationKind::External, + source: "external".to_string(), content: "run ".to_string(), instructions: Some("stay \"brief\"".to_string()), - meta: BTreeMap::from([("ticket".to_string(), "ABC&123".to_string())]), - queued_at: 100, }; assert_eq!( render_message_prompt(&message), - "\ntimer timer-1\n100\n\nrun <tests>\n\n\nstay "brief"\n\n\n ABC&123\n\n" + "\n\nrun <tests>\n\n" + ); + } + + #[test] + fn renders_timer_message_prompt_with_timer_id_and_no_metadata() { + let message = MessageInvocationContext { + kind: MessageInvocationKind::Timer { + timer_id: "timer-1".to_string(), + }, + source: "timer timer-1".to_string(), + content: "run ".to_string(), + instructions: Some("stay \"brief\"".to_string()), + }; + + assert_eq!( + render_message_prompt(&message), + "\ntimer-1\n\nrun <tests>\n\n\nstay "brief"\n\n" ); } diff --git a/codex-rs/core/src/timers.rs b/codex-rs/core/src/timers.rs index fa7b6e4474..b80a8d499f 100644 --- a/codex-rs/core/src/timers.rs +++ b/codex-rs/core/src/timers.rs @@ -5,6 +5,7 @@ //! restore timers after a harness restart. use crate::messages::MessageInvocationContext; +use crate::messages::MessageInvocationKind; use crate::messages::MessagePayload; use crate::messages::validate_meta; use crate::timer_trigger::TimerTrigger; @@ -488,11 +489,12 @@ pub(crate) fn timer_message_invocation_context( timer: &TimerInvocationContext, ) -> MessageInvocationContext { MessageInvocationContext { + kind: MessageInvocationKind::Timer { + timer_id: timer.current_timer_id.clone(), + }, source: format!("timer {}", timer.current_timer_id), content: timer_message_content(timer), instructions: timer_message_instructions(timer), - meta: timer.meta.clone(), - queued_at: timer.queued_at, } } @@ -947,7 +949,7 @@ mod tests { ResponseInputItem::Message { role: "user".to_string(), content: vec![ContentItem::InputText { - text: "\ntimer timer-1\n100\n\nTimer fired: run tests\n\n\nrun tests\n\nThis timer should keep running on its schedule after this invocation.\nDo not call delete_timer just because you completed this invocation.\nCall delete_timer with {"id":"timer-1"} only if the user's timer message included an explicit stop condition, such as "until", "stop when", or "while", and that condition is now satisfied.\nDo not expose scheduler internals unless they matter to the user.\n\n\n".to_string(), + text: "\ntimer-1\n\nTimer fired: run tests\n\n\nrun tests\n\nThis timer should keep running on its schedule after this invocation.\nDo not call delete_timer just because you completed this invocation.\nCall delete_timer with {"id":"timer-1"} only if the user's timer message included an explicit stop condition, such as "until", "stop when", or "while", and that condition is now satisfied.\nDo not expose scheduler internals unless they matter to the user.\n\n".to_string(), }], } ); @@ -969,7 +971,7 @@ mod tests { ResponseInputItem::Message { role: "user".to_string(), content: vec![ContentItem::InputText { - text: "\ntimer timer-1\n101\n\nTimer fired: run tests once\n\n\nrun tests once\n\nuser-specific instruction\n\nThis one-shot timer has already been removed from the schedule, so you do not need to call delete_timer.\nDo not expose scheduler internals unless they matter to the user.\n\n\n ABC_123\n\n".to_string(), + text: "\ntimer-1\n\nTimer fired: run tests once\n\n\nrun tests once\n\nuser-specific instruction\n\nThis one-shot timer has already been removed from the schedule, so you do not need to call delete_timer.\nDo not expose scheduler internals unless they matter to the user.\n\n".to_string(), }], } ); diff --git a/codex-rs/core/tests/suite/timers.rs b/codex-rs/core/tests/suite/timers.rs index f0771e3c6c..704665e4f1 100644 --- a/codex-rs/core/tests/suite/timers.rs +++ b/codex-rs/core/tests/suite/timers.rs @@ -117,14 +117,13 @@ async fn assert_after_turn_timer_starts_and_emits_fired_event() -> Result<()> { let user_messages = mock.single_request().message_input_texts("user"); let timer_messages = user_messages .iter() - .filter(|message| message.contains("")) + .filter(|message| message.contains("")) .collect::>(); assert_eq!(timer_messages.len(), 1); let timer_message = timer_messages[0]; - assert!(timer_message.contains(&format!("timer {}", created.id))); + assert!(timer_message.contains(&format!("{}", created.id))); assert!(timer_message.contains("\nTimer fired: run timer\n")); assert!(timer_message.contains("\nrun timer\n\nThis one-shot timer")); - assert!(timer_message.contains("")); Ok(()) }