core: refresh environment context before sampling#29073
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed79efe815
ℹ️ 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".
| } | ||
|
|
||
| fn environment_only(environments: Vec<EnvironmentContextEnvironment>) -> Option<Self> { | ||
| (!environments.is_empty()).then(|| { |
There was a problem hiding this comment.
When a DeferredExecutor environment is still starting on one sampling request and then its resolution completes with an error before the next request, ThreadEnvironments::snapshot drops it from both turn_environments and starting; because an empty snapshot returns None here, record_step_environment_context_if_changed returns without appending a replacement or clearing the baseline. The prompt history therefore keeps the previous <shell>still loading</shell> environment context for later requests, misleading the model about an environment that is no longer available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
we havent really modeled failed state fully. can add if it seems logical to do now, or can follow up.
| .await, | ||
| ); | ||
| } | ||
| let initial_environment_context = if should_inject_full_context |
There was a problem hiding this comment.
When a steady-state turn changes the environment selection or cwd, build_settings_update_items already records an environment-context diff before sampling, but this baseline is only seeded on the full-context path. The next pre-sampling refresh in run_turn therefore compares against the previous baseline and appends another environment-only context for the same state, adding redundant model-visible messages and cache misses whenever environment context changes after the first turn.
AGENTS.md reference: AGENTS.md:L92-L98
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
i think we get this for free once the proposed/upcoming StepContextItem lands and is the sole owner of environment context. thoughts?
| let step_context = StepContext { | ||
| environments: sess.services.turn_environments.snapshot().await, | ||
| }; | ||
| sess.record_step_environment_context_if_changed( | ||
| turn_context.as_ref(), |
There was a problem hiding this comment.
When an environment finishes starting during a turn, this live snapshot can append it to <environment_context>, but the tool router and handlers for that same turn are still built from the immutable turn_context.environments captured at turn creation (for example, resolve_tool_environment only searches that original list). In a turn that started before the remote environment was ready, the next model request can therefore see a ready environment ID that shell/request-permission tools either do not expose or reject as unknown, so the refreshed context can steer the model into unusable tool calls until the next turn rebuilds TurnContext.
Useful? React with 👍 / 👎.
sayan-oai
changed the title
ed79efe to
31281a9
Compare
2 participants
Why
Nonblocking environment snapshots allow a turn to reach the model while a remote environment is still starting. The initial context can describe that environment as still loading, but nothing currently refreshes the model-visible environment context when startup finishes during the same turn.
This adds the first request-scoped reconciliation slice on top of #28683. It is gated by
DeferredExecutorand intentionally updates only model-visible environment context; tools and other environment-derived state will migrate separately.What
StepContextcontaining the environment snapshot captured before each sampling request.still loading.Testing
just test -p codex-core deferred_executorjust test -p codex-core environment_context_baseline_deduplicates_until_history_is_replacedThe integration coverage verifies that a pending environment reaches the first request, the ready state reaches the next request, later requests do not duplicate it, and ready-at-start environments remain single-injected.
Live verification
still loading, request 2 appended its ready shell and cwd, and request 3 contained no duplicate ready update.pwdand read a marker file through the remote process runner; the command exited successfully and returned the remote cwd and marker contents.