Skip to content

[codex] Start idle turns without reservations#24673

Open
pakrym-oai wants to merge 1 commit into
mainfrom
pakrym/no-idle-turn-reservations
Open

[codex] Start idle turns without reservations#24673
pakrym-oai wants to merge 1 commit into
mainfrom
pakrym/no-idle-turn-reservations

Conversation

@pakrym-oai
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai commented May 27, 2026

Why

Goal continuation currently reserves an ActiveTurn without a running task while it revalidates persisted goal state. That creates a partial-active state just to schedule later work, and gives goals a separate turn-start flow from other idle work.

What changed

  • Add try_start_turn_if_idle as the shared idle-only entry point, with task construction/spawning in start_turn and task execution in run_turn.
  • Keep replace_turn preemptive for user turns, standalone shell commands, compaction, and review.
  • Add turn_admission_lock so explicit replacement and idle-only startup cannot race during admission without using a taskless ActiveTurn reservation.
  • Start goal continuation and queued pending work only when idle admission succeeds, removing goal-specific turn reservation and cleanup.
  • Use new_default_turn() for automatic idle wake-ups so they follow one internal turn-id allocation path.
  • Publish a real running task when idle startup wins, rather than reserving a taskless active turn before launch.

Behavior tradeoffs

  • Goal continuation no longer claims an idle slot while checking that its goal is still current; explicit replacement retains priority when it competes with automatic idle work.
  • Trigger-turn inter-agent wake-ups now use an internally allocated turn id instead of preserving the triggering submission id as the new turn id.
  • Goal continuation and queued pending-work startup no longer use their prior pre-launch unknown-model warning call path.

Validation

  • git diff --check
  • Tests not run, per request to defer tests.

@pakrym-oai pakrym-oai force-pushed the pakrym/no-idle-turn-reservations branch 5 times, most recently from f74af74 to 354aaee Compare May 27, 2026 01:07
@pakrym-oai pakrym-oai changed the title [codex] Start idle tasks without reservations [codex] Start idle turns without reservations May 27, 2026
@pakrym-oai pakrym-oai marked this pull request as ready for review May 27, 2026 01:11
@pakrym-oai pakrym-oai requested a review from a team as a code owner May 27, 2026 01:11
@pakrym-oai
Copy link
Copy Markdown
Collaborator Author

@codex review this

Copy link
Copy Markdown
Contributor

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

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: 354aaeeccf

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

Comment thread codex-rs/core/src/tasks/mod.rs Outdated
self.abort_all_tasks(TurnAbortReason::Replaced).await;
self.clear_connector_selection().await;
self.start_task(turn_context, input, task).await;
self.try_start_turn_if_idle(turn_context, input, task).await;
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.

P1 Badge Start foreground turns even after idle races

replace_turn is used for foreground requests such as user turns and shell commands, but it now delegates to the idle-only try_start_turn_if_idle. If the session is idle after abort_all_tasks and a pending-work or goal-continuation turn starts during the await in clear_connector_selection, this call just returns and the foreground input is neither queued nor run, so the user's request can disappear instead of replacing the background work. Foreground replacement should either win this race or retry/abort the turn that won it.

Useful? React with 👍 / 👎.

// emit a parent TurnStarted. Consider giving review a full parent turn lifecycle
// (TurnStarted + TurnComplete) for consistency with other standalone tasks.
sess.spawn_task(tc.clone(), input, ReviewTask::new()).await;
sess.replace_turn(tc.clone(), input, ReviewTask).await;
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 Emit review entry before starting the review task

Because replace_turn now returns immediately after spawning the task, the ReviewTask can run to exit_review_mode before this function sends EnteredReviewMode below. In fast failure/cached review paths, clients and replay can observe ExitedReviewMode/TurnComplete before the review-entry marker, leaving the UI/history in the wrong order. Send the entry marker before starting the task, or make task startup wait until the marker is emitted.

Useful? React with 👍 / 👎.

@pakrym-oai pakrym-oai force-pushed the pakrym/no-idle-turn-reservations branch 2 times, most recently from d946761 to 083daf4 Compare May 27, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant