Skip to content

[codex] impl delivery_mode: current time reminders on response boundaries - #30033

Merged
rka-oai merged 1 commit into
mainfrom
codex/current-time-reminder-boundaries
Jun 25, 2026
Merged

[codex] impl delivery_mode: current time reminders on response boundaries#30033
rka-oai merged 1 commit into
mainfrom
codex/current-time-reminder-boundaries

Conversation

@rka-oai

@rka-oai rka-oai commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • track user-like input and tool-output boundaries in current-time reminder state
  • gate reminder injection when delivery_mode is after_user_or_tool_output
  • preserve interval debounce and forced reminders after context-window changes

Why

Training can request reminders only after user or tool-output items while keeping the existing canonical pre-inference history-injection path.

Validation

  • just test -p codex-core current_time_reminders_can_follow_only_user_or_tool_outputs
  • just test -p codex-core current_time_reminders_follow_time_interval_and_persist_in_history
  • just test -p codex-core current_time_reminder_is_refreshed_after_compaction
  • just fix -p codex-core

@rka-oai
rka-oai force-pushed the codex/current-time-reminder-delivery-mode branch from 69ac853 to 06603fb Compare June 25, 2026 18:13
@rka-oai
rka-oai force-pushed the codex/current-time-reminder-boundaries branch 2 times, most recently from 19c142f to 74916d0 Compare June 25, 2026 18:29
@rka-oai
rka-oai marked this pull request as ready for review June 25, 2026 18:34
@rka-oai
rka-oai requested a review from a team as a code owner June 25, 2026 18:34
@rka-oai rka-oai changed the title [codex] gate current time reminders on response boundaries [codex] impl delivery_mode: current time reminders on response boundaries Jun 25, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 74916d0d9e

ℹ️ 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".

Comment on lines +46 to +50
if delivery_mode == CurrentTimeReminderDeliveryMode::AfterUserOrToolOutput
&& !is_new_window
&& !follows_user_or_tool_output
{
return false;

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 Avoid reading the clock for suppressed reminders

When delivery_mode is AfterUserOrToolOutput and an assistant-only continuation has no pending user/tool-output boundary, this gate correctly returns false, but it runs only after maybe_record_current_time_reminder has already awaited time_provider.current_time() and mapped failures to a fatal turn error. In that ineligible-continuation scenario, an external clock outage can still abort the model request even though no reminder could be injected; split the boundary/window eligibility check so suppressed reminders do not read the clock.

Useful? React with 👍 / 👎.

Base automatically changed from codex/current-time-reminder-delivery-mode to main June 25, 2026 19:06
@rka-oai
rka-oai force-pushed the codex/current-time-reminder-boundaries branch from 74916d0 to c861d4d Compare June 25, 2026 19:08
@rka-oai
rka-oai enabled auto-merge (squash) June 25, 2026 19:09
@rka-oai
rka-oai merged commit adccb46 into main Jun 25, 2026
45 of 47 checks passed
@rka-oai
rka-oai deleted the codex/current-time-reminder-boundaries branch June 25, 2026 19:28
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants