Skip to content

exec-server: simplify pending environment readiness - #32205

Closed
pakrym-oai wants to merge 2 commits into
mainfrom
pakrym/full-ci-pending-environment-readiness
Closed

pakrym-oai wants to merge 2 commits into
mainfrom
pakrym/full-ci-pending-environment-readiness

Conversation

@pakrym-oai

@pakrym-oai pakrym-oai commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Why

Environment providers need to register an environment before its stable exec-server URL is available. Completing that registration through a later manager lookup by environment ID couples provisioning to mutable registry state: a stale completion can target a replacement that reused the same ID.

This change gives each pending registration its own one-shot completion capability. The connection path only consumes the terminal result associated with its captured environment, which keeps replacement semantics isolated and makes the readiness flow linear.

What changed

  • Make register_pending_environment return a PendingEnvironmentRegistration that is consumed by complete with either the stable URL or a terminal error.
  • Represent pending WebSocket readiness as a shared one-shot result, awaited before entering the existing WebSocket connection and initialization path.
  • Cache the shared terminal result so reconnects can resolve the same URL again.
  • Remove the redundant URL copy and the public Environment::exec_server_url() getter. There are no in-tree production callers; test infrastructure now retains its own configured URL.
  • Add exec-server integration coverage for successful completion, explicit failure, dropped or invalid registrations, and late completion after replacement.
  • Update the deferred-executor core integration tests to register an environment without a URL, select it for the turn, and then exercise both late readiness and terminal provisioning failure.

This is an alternative implementation of #31988.

Testing

  • just test -p codex-exec-server --test pending_environment
  • just test -p codex-exec-server --lib (181 passed)
  • just test -p codex-core --test all deferred_executor_updates_context_and_tools_after_startup
  • just test -p codex-core --test all deferred_executor_wait_reports_startup_failure

@pakrym-oai
pakrym-oai marked this pull request as ready for review July 10, 2026 16:16
@pakrym-oai pakrym-oai closed this Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant