Skip to content

[codex] current time reminder interval to be set to 0 #30029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rka-oai merged 2 commits into from
Jun 25, 2026
+43 −30
Merged

[codex] current time reminder interval to be set to 0 #30029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
070d3d3
Allow zero current time reminder intervals
rka-oai Jun 25, 2026
758b984
Deliver zero-interval time reminders unconditionally
rka-oai Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codex-rs/core/config.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@
},
"reminder_interval_seconds": {
"format": "uint64",
"minimum": 1.0,
"minimum": 0.0,
"type": "integer"
},
"sleep_tool": {
Expand Down
24 changes: 2 additions & 22 deletions codex-rs/core/src/config/config_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,12 +625,12 @@ current_time_reminder = true
r#"
[features.current_time_reminder]
enabled = true
reminder_interval_seconds = 4
reminder_interval_seconds = 0

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add integration coverage for zero interval reminders

The new 0 case is only exercised through config deserialization here; the behavior being introduced is agent scheduling/history injection at inference boundaries, but no core/suite/current_time_reminder integration test covers reminder_interval_seconds = 0. That leaves the user-facing guarantee for zero intervals untested through the path that records reminders into model-visible history, contrary to the repo requirement for agent-logic changes; add an integration test that drives multiple inference boundaries with interval 0.

AGENTS.md reference: AGENTS.md:L112-L118

Useful? React with 👍 / 👎.

@rka-oai rka-oai Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

clock_source = "external"
sleep_tool = true
"#,
CurrentTimeReminderConfig {
reminder_interval_seconds: 4,
reminder_interval_seconds: 0,
clock_source: CurrentTimeSource::External,
sleep_tool: true,
},
Expand All @@ -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");
Expand Down
6 changes: 0 additions & 6 deletions codex-rs/core/src/config/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/session/time_reminder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
39 changes: 39 additions & 0 deletions codex-rs/core/tests/suite/current_time_reminder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(()));
Expand Down
1 change: 0 additions & 1 deletion codex-rs/features/src/feature_configs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle zero reminders without elapsed-time gating

Allowing reminder_interval_seconds to deserialize as 0 here exposes the existing scheduler unchanged: CurrentTimeReminderState::take_reminder_due still requires the newly read clock value to be at least the previous delivery time. When clock_source = "external" (or the system clock) moves backward between inference boundaries, a zero interval will skip reminders until the clock catches up, even though zero is intended to request a reminder at every otherwise-eligible boundary. Special-case 0 as always due after the first check/window change instead of relying on elapsed-time comparison.

Useful? React with 👍 / 👎.

@rka-oai rka-oai Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

#[serde(skip_serializing_if = "Option::is_none")]
pub clock_source: Option<CurrentTimeSource>,
Expand Down
Loading