mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] current time reminder interval to be set to 0 (#30029)
A zero interval lets callers request a reminder at every otherwise-eligible inference boundary. ## Validation - just test -p codex-core load_config_resolves_current_time_reminder
This commit is contained in:
committed by
GitHub
Unverified
parent
31b99f65cf
commit
cc78903379
@@ -815,7 +815,7 @@
|
||||
},
|
||||
"reminder_interval_seconds": {
|
||||
"format": "uint64",
|
||||
"minimum": 1.0,
|
||||
"minimum": 0.0,
|
||||
"type": "integer"
|
||||
},
|
||||
"sleep_tool": {
|
||||
|
||||
@@ -625,12 +625,12 @@ current_time_reminder = true
|
||||
r#"
|
||||
[features.current_time_reminder]
|
||||
enabled = true
|
||||
reminder_interval_seconds = 4
|
||||
reminder_interval_seconds = 0
|
||||
clock_source = "external"
|
||||
sleep_tool = true
|
||||
"#,
|
||||
CurrentTimeReminderConfig {
|
||||
reminder_interval_seconds: 4,
|
||||
reminder_interval_seconds: 0,
|
||||
clock_source: CurrentTimeSource::External,
|
||||
sleep_tool: true,
|
||||
},
|
||||
@@ -643,26 +643,6 @@ sleep_tool = true
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_rejects_zero_current_time_reminder_interval() -> std::io::Result<()> {
|
||||
let error = load_current_time_reminder_config(
|
||||
r#"
|
||||
[features.current_time_reminder]
|
||||
enabled = true
|
||||
reminder_interval_seconds = 0
|
||||
"#,
|
||||
)
|
||||
.await
|
||||
.expect_err("zero reminder interval should be rejected");
|
||||
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
"features.current_time_reminder.reminder_interval_seconds must be positive"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn load_current_time_reminder_config(config_toml: &str) -> std::io::Result<Config> {
|
||||
let codex_home = tempdir()?;
|
||||
let config_toml = toml::from_str(config_toml).expect("TOML should deserialize");
|
||||
|
||||
@@ -2685,12 +2685,6 @@ fn resolve_current_time_reminder_config(
|
||||
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_seconds must be positive",
|
||||
));
|
||||
}
|
||||
|
||||
Ok(Some(CurrentTimeReminderConfig {
|
||||
reminder_interval_seconds,
|
||||
|
||||
@@ -21,6 +21,7 @@ impl CurrentTimeReminderState {
|
||||
interval_seconds: u64,
|
||||
) -> bool {
|
||||
let reminder_is_due = self.last_window_id.as_deref() != Some(window_id)
|
||||
|| interval_seconds == 0
|
||||
|| self.last_delivery_time.is_none_or(|last_delivery_time| {
|
||||
current_time
|
||||
.signed_duration_since(last_delivery_time)
|
||||
|
||||
@@ -39,6 +39,7 @@ use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
const FIRST_REMINDER: &str = "It is 2026-06-17 17:34:15 UTC.";
|
||||
const EARLIER_REMINDER: &str = "It is 2026-06-17 17:33: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;
|
||||
@@ -162,6 +163,44 @@ async fn current_time_reminders_follow_time_interval_and_persist_in_history() ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn zero_current_time_reminder_interval_delivers_when_time_moves_backward() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let responses = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let time_provider = Arc::new(TestTimeProvider::default());
|
||||
let test = test_codex()
|
||||
.with_config(|config| {
|
||||
enable_current_time_reminder(config, /*interval*/ 0, CurrentTimeSource::External)
|
||||
})
|
||||
.with_external_time_provider(time_provider.clone())
|
||||
.build(&server)
|
||||
.await?;
|
||||
|
||||
test.submit_turn("first turn").await?;
|
||||
time_provider
|
||||
.current_time
|
||||
.store(FIRST_TIME_UNIX_SECONDS - 60, Ordering::Relaxed);
|
||||
test.submit_turn("second turn").await?;
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
assert_eq!(current_time_reminders(&requests[0]), vec![FIRST_REMINDER]);
|
||||
assert_eq!(
|
||||
current_time_reminders(&requests[1]),
|
||||
vec![FIRST_REMINDER, EARLIER_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(()));
|
||||
|
||||
@@ -147,7 +147,6 @@ pub struct CurrentTimeReminderConfigToml {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub enabled: Option<bool>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
#[schemars(range(min = 1))]
|
||||
pub reminder_interval_seconds: Option<u64>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub clock_source: Option<CurrentTimeSource>,
|
||||
|
||||
Reference in New Issue
Block a user