diff --git a/codex-rs/app-server/tests/suite/v2/current_time.rs b/codex-rs/app-server/tests/suite/v2/current_time.rs index 74e734ae3..2179a86cb 100644 --- a/codex-rs/app-server/tests/suite/v2/current_time.rs +++ b/codex-rs/app-server/tests/suite/v2/current_time.rs @@ -115,7 +115,7 @@ model_provider = "mock_provider" [features.current_time_reminder] enabled = true -reminder_interval_model_requests = 1 +reminder_interval_seconds = 1 clock_source = "external" [model_providers.mock_provider] diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 31dcc667d..ffcbd7563 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -826,7 +826,7 @@ "enabled": { "type": "boolean" }, - "reminder_interval_model_requests": { + "reminder_interval_seconds": { "format": "uint64", "minimum": 1.0, "type": "integer" diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 67cb0932d..6b281c408 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -620,11 +620,11 @@ current_time_reminder = true r#" [features.current_time_reminder] enabled = true -reminder_interval_model_requests = 4 +reminder_interval_seconds = 4 clock_source = "external" "#, CurrentTimeReminderConfig { - reminder_interval_model_requests: 4, + reminder_interval_seconds: 4, clock_source: CurrentTimeSource::External, }, ), @@ -642,7 +642,7 @@ async fn load_config_rejects_zero_current_time_reminder_interval() -> std::io::R r#" [features.current_time_reminder] enabled = true -reminder_interval_model_requests = 0 +reminder_interval_seconds = 0 "#, ) .await @@ -651,7 +651,7 @@ reminder_interval_model_requests = 0 assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); assert_eq!( error.to_string(), - "features.current_time_reminder.reminder_interval_model_requests must be positive" + "features.current_time_reminder.reminder_interval_seconds must be positive" ); Ok(()) } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 81d2870c1..923dd6758 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1115,14 +1115,14 @@ pub struct RolloutBudgetConfig { #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] pub struct CurrentTimeReminderConfig { - pub reminder_interval_model_requests: u64, + pub reminder_interval_seconds: u64, pub clock_source: CurrentTimeSource, } impl Default for CurrentTimeReminderConfig { fn default() -> Self { Self { - reminder_interval_model_requests: 1, + reminder_interval_seconds: 1, clock_source: CurrentTimeSource::System, } } @@ -2657,18 +2657,18 @@ fn resolve_current_time_reminder_config( let base = current_time_reminder_toml_config(config_toml.features.as_ref()); let default = CurrentTimeReminderConfig::default(); - let reminder_interval_model_requests = base - .and_then(|config| config.reminder_interval_model_requests) - .unwrap_or(default.reminder_interval_model_requests); - if reminder_interval_model_requests == 0 { + let reminder_interval_seconds = base + .and_then(|config| config.reminder_interval_seconds) + .unwrap_or(default.reminder_interval_seconds); + if reminder_interval_seconds == 0 { return Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, - "features.current_time_reminder.reminder_interval_model_requests must be positive", + "features.current_time_reminder.reminder_interval_seconds must be positive", )); } Ok(Some(CurrentTimeReminderConfig { - reminder_interval_model_requests, + reminder_interval_seconds, clock_source: base .and_then(|config| config.clock_source) .unwrap_or(default.clock_source), diff --git a/codex-rs/core/src/session/config_lock.rs b/codex-rs/core/src/session/config_lock.rs index 035f7bce4..13b6b4824 100644 --- a/codex-rs/core/src/session/config_lock.rs +++ b/codex-rs/core/src/session/config_lock.rs @@ -357,7 +357,7 @@ mod tests { features.current_time_reminder, Some(FeatureToml::Config(CurrentTimeReminderConfigToml { enabled: Some(true), - reminder_interval_model_requests: Some(1), + reminder_interval_seconds: Some(1), clock_source: Some(codex_features::CurrentTimeSource::System), })) ); diff --git a/codex-rs/core/src/session/time_reminder.rs b/codex-rs/core/src/session/time_reminder.rs index 058132a89..85831d294 100644 --- a/codex-rs/core/src/session/time_reminder.rs +++ b/codex-rs/core/src/session/time_reminder.rs @@ -1,3 +1,5 @@ +use chrono::DateTime; +use chrono::Utc; use codex_protocol::error::CodexErr; use codex_protocol::error::Result as CodexResult; @@ -7,18 +9,27 @@ use crate::context::ContextualUserFragment; #[derive(Default)] pub(crate) struct CurrentTimeReminderState { - model_requests_since_delivery: u64, + last_delivery_time: Option>, last_window_id: Option, } impl CurrentTimeReminderState { - fn take_reminder_due(&mut self, window_id: &str, interval: u64) -> bool { - self.model_requests_since_delivery = self.model_requests_since_delivery.saturating_add(1); + fn take_reminder_due( + &mut self, + window_id: &str, + current_time: DateTime, + interval_seconds: u64, + ) -> bool { let reminder_is_due = self.last_window_id.as_deref() != Some(window_id) - || self.model_requests_since_delivery >= interval; + || self.last_delivery_time.is_none_or(|last_delivery_time| { + current_time + .signed_duration_since(last_delivery_time) + .num_seconds() + >= i64::try_from(interval_seconds).unwrap_or(i64::MAX) + }); if reminder_is_due { - self.model_requests_since_delivery = 0; + self.last_delivery_time = Some(current_time); self.last_window_id = Some(window_id.to_string()); } @@ -35,16 +46,6 @@ pub(super) async fn maybe_record_current_time_reminder( return Ok(()); }; - let reminder_is_due = { - let mut state = sess.state.lock().await; - state - .current_time_reminder - .take_reminder_due(window_id, config.reminder_interval_model_requests) - }; - if !reminder_is_due { - return Ok(()); - } - let current_time = sess .services .time_provider @@ -52,6 +53,18 @@ pub(super) async fn maybe_record_current_time_reminder( .await .map_err(|err| CodexErr::Fatal(format!("failed to read current time: {err:#}")))?; + let reminder_is_due = { + let mut state = sess.state.lock().await; + state.current_time_reminder.take_reminder_due( + window_id, + current_time, + config.reminder_interval_seconds, + ) + }; + if !reminder_is_due { + return Ok(()); + } + let response_item = ContextualUserFragment::into(crate::context::CurrentTimeReminder::new(current_time)); sess.record_conversation_items(turn_context, std::slice::from_ref(&response_item)) diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 14bee0fb8..9ec8182c6 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -881,7 +881,7 @@ text(JSON.stringify(result)); .enable(Feature::CurrentTimeReminder) .expect("test config should allow current-time reminders"); config.current_time_reminder = Some(CurrentTimeReminderConfig { - reminder_interval_model_requests: 50, + reminder_interval_seconds: 3_000, clock_source: CurrentTimeSource::System, }); }, diff --git a/codex-rs/core/tests/suite/current_time_reminder.rs b/codex-rs/core/tests/suite/current_time_reminder.rs index fda9b85b0..ec55da431 100644 --- a/codex-rs/core/tests/suite/current_time_reminder.rs +++ b/codex-rs/core/tests/suite/current_time_reminder.rs @@ -37,6 +37,7 @@ use serde_json::json; const FIRST_REMINDER: &str = "It is 2026-06-17 17:34:15 UTC."; const SECOND_REMINDER: &str = "It is 2026-06-17 17:35:15 UTC."; +const THIRD_REMINDER: &str = "It is 2026-06-17 17:36:15 UTC."; const FIRST_TIME_UNIX_SECONDS: i64 = 1_781_717_655; struct TestTimeProvider(AtomicI64); @@ -83,13 +84,13 @@ fn enable_current_time_reminder( .enable(Feature::CurrentTimeReminder) .expect("test config should allow current-time reminders"); config.current_time_reminder = Some(CurrentTimeReminderConfig { - reminder_interval_model_requests: interval, + reminder_interval_seconds: interval, clock_source, }); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn current_time_reminders_follow_request_interval_and_persist_in_history() -> Result<()> { +async fn current_time_reminders_follow_time_interval_and_persist_in_history() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -120,7 +121,7 @@ async fn current_time_reminders_follow_request_interval_and_persist_in_history() .await; let test = test_codex() .with_config(|config| { - enable_current_time_reminder(config, /*interval*/ 2, CurrentTimeSource::External) + enable_current_time_reminder(config, /*interval*/ 120, CurrentTimeSource::External) }) .with_external_time_provider(Arc::new(TestTimeProvider::default())) .build(&server) @@ -136,7 +137,7 @@ async fn current_time_reminders_follow_request_interval_and_persist_in_history() assert_eq!(current_time_reminders(&requests[1]), vec![FIRST_REMINDER]); assert_eq!( current_time_reminders(&requests[2]), - vec![FIRST_REMINDER, SECOND_REMINDER] + vec![FIRST_REMINDER, THIRD_REMINDER] ); Ok(()) } @@ -195,7 +196,11 @@ async fn current_time_reminder_is_refreshed_after_compaction() -> Result<()> { let test = test_codex() .with_config(move |config| { config.model_provider = model_provider; - enable_current_time_reminder(config, /*interval*/ 50, CurrentTimeSource::External); + enable_current_time_reminder( + config, + /*interval*/ 3_000, + CurrentTimeSource::External, + ); }) .with_external_time_provider(Arc::new(TestTimeProvider::default())) .build(&server) @@ -295,7 +300,11 @@ async fn current_time_tool_returns_the_latest_time() -> Result<()> { .await; let test = test_codex() .with_config(|config| { - enable_current_time_reminder(config, /*interval*/ 50, CurrentTimeSource::External) + enable_current_time_reminder( + config, + /*interval*/ 3_000, + CurrentTimeSource::External, + ) }) .with_external_time_provider(Arc::new(TestTimeProvider::default())) .build(&server) diff --git a/codex-rs/features/src/feature_configs.rs b/codex-rs/features/src/feature_configs.rs index 35b669b0a..be06bd157 100644 --- a/codex-rs/features/src/feature_configs.rs +++ b/codex-rs/features/src/feature_configs.rs @@ -144,7 +144,7 @@ pub struct CurrentTimeReminderConfigToml { pub enabled: Option, #[serde(skip_serializing_if = "Option::is_none")] #[schemars(range(min = 1))] - pub reminder_interval_model_requests: Option, + pub reminder_interval_seconds: Option, #[serde(skip_serializing_if = "Option::is_none")] pub clock_source: Option, }