[2/3] core: track starting environments in snapshots#28683
Conversation
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9cd1c8697
ℹ️ 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".
ff3f3a6 to
f4f57bb
Compare
e9cd1c8 to
fea9d37
Compare
jif-oai
left a comment
There was a problem hiding this comment.
I think the state machine we're trying to build here is not fully closed... this illustrate well my concerns on the difficulty to handle failure modes
| Some(Err(_)) => {} | ||
| None => starting.push(StartingTurnEnvironment { | ||
| selection: environment.selection.clone(), | ||
| resolution: environment.resolution.clone(), |
There was a problem hiding this comment.
Doesn't this make the remote’s AGENTS.md disappear for the whole loaded session, not just the turn that races startup?
There was a problem hiding this comment.
yep. the end state is going to be having these react to env readiness changes and get injected. that will be in a follow up, so i added a deferred_executor feature flag so we can land stuff incrementally without causing issues for existing testing.
f4f57bb to
918b9bc
Compare
77c7b18 to
aad2139
Compare
253a6ba to
bb8869d
Compare
Why
Remote environments may still be resolving when Codex creates a session or turn. Waiting for the existing all-or-nothing environment snapshot can hold startup until the selected environment is usable.
Behind the default-off
deferred_executorfeature, let callers take a useful snapshot immediately: completed environments remain available normally, while unfinished environments are reported without blocking startup. With the feature disabled, snapshots preserve the existing blocking behavior.Depends on #28674.
What changed
ThreadEnvironments. Each selection owns one shared resolution that produces its completeTurnEnvironment.remote_handle(), allowing snapshots and the future wait tool to share the same result while cancellation follows the retained handles.snapshot()a read-only operation: nonblocking snapshots collect completed resolutions and retain handles for unfinished ones, while blocking snapshots await every resolution.to_selections()derives from attached environments, so child threads do not inherit an environment that is still starting.Test plan
just test -p codex-core environment_selectionjust test -p codex-core deferred_executor_reaches_model_before_remote_environment_is_readyLanding note
Keep
deferred_executordisabled for slow-starting executors until configurableenvironment/addconnection timeouts and caller support land. When enabled, an environment that attaches after session startup may remain absent from environment-derived model context, tools, instructions, skills, and related state until follow-up refresh work lands.