From adccb464d075a39d5450d6bc879e3bb6c97ce14b Mon Sep 17 00:00:00 2001 From: rka-oai Date: Thu, 25 Jun 2026 12:28:50 -0700 Subject: [PATCH] [codex] impl delivery_mode: current time reminders on response boundaries (#30033) ## Summary - track user-like input and tool-output boundaries in current-time reminder state - gate reminder injection when delivery_mode is after_user_or_tool_output - preserve interval debounce and forced reminders after context-window changes ## Why Training can request reminders only after user or tool-output items while keeping the existing canonical pre-inference history-injection path. ## Validation - just test -p codex-core current_time_reminders_can_follow_only_user_or_tool_outputs - just test -p codex-core current_time_reminders_follow_time_interval_and_persist_in_history - just test -p codex-core current_time_reminder_is_refreshed_after_compaction - just fix -p codex-core --- codex-rs/core/src/session/mod.rs | 2 + codex-rs/core/src/session/time_reminder.rs | 33 ++++++++- .../core/tests/suite/current_time_reminder.rs | 69 +++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 442722003..6fe61222c 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2775,6 +2775,7 @@ impl Session { let items = items.as_ref(); { let mut state = self.state.lock().await; + state.current_time_reminder.note_recorded_items(items); state.record_items( items.iter(), turn_context.model_info.truncation_policy.into(), @@ -2877,6 +2878,7 @@ impl Session { let response_item = items[0].clone(); { let mut state = self.state.lock().await; + state.current_time_reminder.note_recorded_items(items); state.record_items( items.iter(), turn_context.model_info.truncation_policy.into(), diff --git a/codex-rs/core/src/session/time_reminder.rs b/codex-rs/core/src/session/time_reminder.rs index 8f612e8fc..398ae5811 100644 --- a/codex-rs/core/src/session/time_reminder.rs +++ b/codex-rs/core/src/session/time_reminder.rs @@ -1,26 +1,56 @@ use chrono::DateTime; use chrono::Utc; +use codex_features::CurrentTimeReminderDeliveryMode; use codex_protocol::error::CodexErr; use codex_protocol::error::Result as CodexResult; +use codex_protocol::models::ResponseItem; use super::session::Session; use super::turn_context::TurnContext; use crate::context::ContextualUserFragment; +use crate::context_manager::is_user_turn_boundary; #[derive(Default)] pub(crate) struct CurrentTimeReminderState { last_delivery_time: Option>, last_window_id: Option, + pending_user_or_tool_output_boundary: bool, } impl CurrentTimeReminderState { + pub(super) fn note_recorded_items(&mut self, items: &[ResponseItem]) { + if items.iter().any(|item| { + is_user_turn_boundary(item) + || matches!( + item, + ResponseItem::FunctionCallOutput { .. } + | ResponseItem::CustomToolCallOutput { .. } + | ResponseItem::ToolSearchOutput { .. } + ) + }) { + self.pending_user_or_tool_output_boundary = true; + } + } + fn take_reminder_due( &mut self, window_id: &str, current_time: DateTime, interval_seconds: u64, + delivery_mode: CurrentTimeReminderDeliveryMode, ) -> bool { - let reminder_is_due = self.last_window_id.as_deref() != Some(window_id) + let is_new_window = self.last_window_id.as_deref() != Some(window_id); + // Consume the boundary for this inference even if the interval suppresses delivery. + let follows_user_or_tool_output = + std::mem::take(&mut self.pending_user_or_tool_output_boundary); + if delivery_mode == CurrentTimeReminderDeliveryMode::AfterUserOrToolOutput + && !is_new_window + && !follows_user_or_tool_output + { + return false; + } + + let reminder_is_due = is_new_window || interval_seconds == 0 || self.last_delivery_time.is_none_or(|last_delivery_time| { current_time @@ -60,6 +90,7 @@ pub(super) async fn maybe_record_current_time_reminder( window_id, current_time, config.reminder_interval_seconds, + config.delivery_mode, ) }; if !reminder_is_due { diff --git a/codex-rs/core/tests/suite/current_time_reminder.rs b/codex-rs/core/tests/suite/current_time_reminder.rs index c5ebd0ed1..9a9440fb3 100644 --- a/codex-rs/core/tests/suite/current_time_reminder.rs +++ b/codex-rs/core/tests/suite/current_time_reminder.rs @@ -12,6 +12,7 @@ use codex_core::SleepFuture; use codex_core::TimeFuture; use codex_core::TimeProvider; use codex_core::config::CurrentTimeReminderConfig; +use codex_features::CurrentTimeReminderDeliveryMode; use codex_features::CurrentTimeSource; use codex_features::Feature; use codex_model_provider_info::built_in_model_providers; @@ -201,6 +202,69 @@ async fn zero_current_time_reminder_interval_delivers_when_time_moves_backward() Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn current_time_reminders_can_follow_only_user_or_tool_outputs() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let tool_args = json!({ + "command": "echo current time", + "timeout_ms": 1_000, + }); + let mut continue_response = ev_completed("resp-2"); + // Ask for another inference without recording a new user message or tool output. + continue_response["response"]["end_turn"] = json!(false); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + "current-time-tool-call", + "shell_command", + &serde_json::to_string(&tool_args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-2", "continue"), + continue_response, + ]), + sse(vec![ev_response_created("resp-3"), ev_completed("resp-3")]), + ], + ) + .await; + let test = test_codex() + .with_config(|config| { + enable_current_time_reminder(config, /*interval*/ 0, CurrentTimeSource::External); + config + .current_time_reminder + .as_mut() + .expect("current-time reminder should be configured") + .delivery_mode = CurrentTimeReminderDeliveryMode::AfterUserOrToolOutput; + }) + .with_external_time_provider(Arc::new(TestTimeProvider::default())) + .build(&server) + .await?; + + test.submit_turn_with_permission_profile("first turn", PermissionProfile::Disabled) + .await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 3); + assert_eq!(current_time_reminders(&requests[0]), vec![FIRST_REMINDER]); + assert_eq!( + current_time_reminders(&requests[1]), + vec![FIRST_REMINDER, SECOND_REMINDER] + ); + assert_eq!( + current_time_reminders(&requests[2]), + vec![FIRST_REMINDER, SECOND_REMINDER] + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn system_time_source_adds_current_time_reminder() -> Result<()> { skip_if_no_network!(Ok(())); @@ -260,6 +324,11 @@ async fn current_time_reminder_is_refreshed_after_compaction() -> Result<()> { /*interval*/ 3_000, CurrentTimeSource::External, ); + config + .current_time_reminder + .as_mut() + .expect("current-time reminder should be configured") + .delivery_mode = CurrentTimeReminderDeliveryMode::AfterUserOrToolOutput; }) .with_external_time_provider(Arc::new(TestTimeProvider::default())) .build(&server)