[codex] support external clock sleeps by rka-oai · Pull Request #29923 · openai/codex

rka-oai · 2026-06-24T23:02:14Z

Summary

add an experimental currentTime/sleep notification and currentTime/wake request for external clocks
keep pending sleeps keyed by thread and sleep ID so wakeups are precise, canceled waits are removed, and stale wakes are harmless

Key Decision: I've set currentTime/sleep as a notification, but if we want to be completely safe, I think the right behavior is expect the app server client to "ack" back via {} - but i don't believe there's any existing precedent for this for app server -> client methods (only client -> app server)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37358928b3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e96cf3e82

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-25T00:21:38Z

+        .send_server_notification_to_connections(
+            &[connection_id],
+            ServerNotification::CurrentTimeSleep(CurrentTimeSleepParams {


Fail fast when sleep notifications are filtered

If the subscribed client initialized without experimentalApi (or opted out of currentTime/sleep), the transport drops this experimental ServerNotification before it is written, but notify_external_sleep still returns Ok and the sleep future then waits only for a later currentTime/wake. In that configuration a model clock.sleep call hangs indefinitely instead of reporting that the selected client cannot handle external sleeps; please verify the chosen connection can receive this notification, or use an acknowledged request/bounded wait.

Useful? React with 👍 / 👎.

not a serious issue for the limited use case here

chatgpt-codex-connector · 2026-06-25T00:21:38Z

+                        result.map_err(|err| {
+                            FunctionCallError::Fatal(format!("failed to sleep: {err:#}"))
+                        })?;


Complete the sleep item on provider errors

With an external clock, time_provider.sleep can now fail after emit_turn_item_started has already published the sleep item (for example, no unambiguous subscribed clock client within the 5s lookup). This ? returns a fatal tool error before emit_turn_item_completed runs, leaving clients with an in-progress sleep item in the live event stream even though the turn fails. Please ensure the started sleep item is completed or failed on this error path.

Useful? React with 👍 / 👎.

not a serious issue here either

jif-oai · 2026-06-25T09:40:46Z

+    let deadline = Instant::now() + EXTERNAL_CLOCK_REQUEST_TIMEOUT;
+    let connection_id =
+        wait_for_current_time_connection(thread_state_manager, thread_id, deadline).await?;
+    outgoing


This notification is required control flow, but both in-process delivery tiers still classify it as best-effort. Either bounded queue can drop it, after which this future waits forever for a wake the client never saw

notiffication should be considered required delivery now (so the bounded queue should wait until delivery).

jif-oai · 2026-06-25T09:41:26Z

            ClientRequest::Initialize { .. } => {
                panic!("Initialize should be handled before initialized request dispatch");
            }
+            ClientRequest::CurrentTimeWake { params, .. } => {


We have the caller’s connection_id here but throw it away!
Since the sleep was sent to one specific external-clock connection, the pending entry needsto record that owner and reject wakes from other connections. Otherwise a different client can complete the sleep without advancing the clock used by later currentTime/read calls

done! was originally building for specifically the 1 client case. Agreed its better we account for the general case here 👍

jif-oai · 2026-06-25T09:41:40Z

+    .await??;
+    let _: CurrentTimeWakeResponse = to_response(wake_response)?;
+
+    respond_to_current_time_read(&mut app_server, &thread.id, CURRENT_TIME_AT + 1).await?;


This requests a 12-hour sleep but advances the fake clock by one second, and we never prove the turn was still blocked before wake

improved test

jif-oai

I’m still pretty unconvinced by this shape. currentTime/sleep is required control flow, but modeling it as a notification plus a separate currentTime/wake request makes us rebuild correlation, delivery, cancellation, connection affinity, and stale-wake semantics by hand.
I can imagine the edge cases exploding here.
his should be a server-initiated request that remains pending until the client advances its clock and responds. What do you think?

jif-oai · 2026-06-25T10:49:36Z

+        {
+            bail!("external sleep can only be woken by the client that received it");
+        }
+        let pending_sleep = entries.remove(key);


NIT: This also ACKs a typo or wrong threadId/sleepId

andmis

(Leaving comments in slack.)

rka-oai · 2026-06-25T21:51:22Z

Closing as superseded by #30113, which supports external clock sleeps by polling the existing currentTime/read method without adding a separate sleep/wake protocol.

rka-oai marked this pull request as ready for review June 24, 2026 23:35

rka-oai requested a review from a team as a code owner June 24, 2026 23:35

chatgpt-codex-connector Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread codex-rs/app-server/src/current_time.rs

Comment thread codex-rs/app-server/src/current_time.rs Outdated

rka-oai marked this pull request as draft June 25, 2026 00:11

rka-oai marked this pull request as ready for review June 25, 2026 00:11

chatgpt-codex-connector Bot reviewed Jun 25, 2026

View reviewed changes

rka-oai force-pushed the codex/current-time-sleep-config branch from 15eb0ba to e7572c1 Compare June 25, 2026 00:26

Base automatically changed from codex/current-time-sleep-config to main June 25, 2026 00:49

github-actions Bot mentioned this pull request Jun 25, 2026

📊 AI CLI 工具社区动态日报 2026-06-25 zx0828/big_model_radar#174

Open

rka-oai force-pushed the codex/external-clock-sleep branch 2 times, most recently from 5796b00 to 946eccb Compare June 25, 2026 03:53

rka-oai changed the base branch from main to codex/time-provider-sleep June 25, 2026 03:53

github-actions Bot mentioned this pull request Jun 25, 2026

📊 AI CLI 工具社区动态日报 2026-06-25 96loveslife/big_model_radar#54

Open

rka-oai force-pushed the codex/external-clock-sleep branch 2 times, most recently from 258b1d9 to 0ea6315 Compare June 25, 2026 05:03

Base automatically changed from codex/time-provider-sleep to main June 25, 2026 05:17

Support external clock sleeps

950b702

rka-oai force-pushed the codex/external-clock-sleep branch from 0ea6315 to 950b702 Compare June 25, 2026 05:51

jif-oai reviewed Jun 25, 2026

View reviewed changes

rka-oai added 5 commits June 25, 2026 03:07

Make external sleep notifications lossless

1329b0b

Bind external sleeps to their clock connection

733fabf

Verify external sleeps block until wake

d549741

Format external sleep notification call

e826264

Assert sleep blocks the next clock read

ca38b19

rka-oai requested a review from jif-oai June 25, 2026 10:30

jif-oai reviewed Jun 25, 2026

View reviewed changes

andmis suggested changes Jun 25, 2026

View reviewed changes

rka-oai closed this Jun 25, 2026

Uh oh!

Conversation

rka-oai commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jif-oai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andmis left a comment

Choose a reason for hiding this comment

Uh oh!

rka-oai commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rka-oai commented Jun 24, 2026 •

edited

Loading