[codex] Add interruptible sleep tool#28429
Conversation
avinashkamat48
left a comment
There was a problem hiding this comment.
The implementation validates duration_ms after deserialization, but the tool schema still exposes it as a generic JSON number while the Rust type is u64. That leaves the model/tool caller with a schema that allows fractional values even though parse_arguments will reject them before the explicit range check runs. It would be cleaner to expose this as an integer with min/max constraints if the schema helper supports that, or add a small parse/validation test for fractional input so the model-visible error remains intentional. This matters more here because the tool description says 1..=3600000, but the machine-readable schema currently only says number.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e0bb90c77
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| async fn any_new_input_interrupts_sleep() { | ||
| const FIRST_SLEEP_CALL_ID: &str = "sleep-call-1"; | ||
| const SECOND_SLEEP_CALL_ID: &str = "sleep-call-2"; | ||
| const SLEEP_DURATION_MS: u64 = 60_000; |
There was a problem hiding this comment.
any harm in having a u64::MAX for the interruptible sleep to avoid the potential for flakes w/ infra weirdness?
Why
Models sometimes need to pause briefly while waiting for external work, but using a shell command for that delay ties the wait to a process and does not naturally resume when new turn input arrives.
What changed
sleeptool behind the under-developmentsleep_toolfeatureduration_msargument, matching the millisecond convention used by unified execSleepItemthroughitem/startedanditem/completedThreadItem::Sleepand retain it in reconstructed thread historyTest plan
just test -p codex-core sleep_tool_follows_feature_gatejust test -p codex-core any_new_input_interrupts_sleepjust test -p codex-app-server-protocoljust test -p codex-app-server sleep_emits_started_and_completed_items