core: use current step environments for tools - #29547
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26c11ecd14
ℹ️ 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".
|
@codex review this |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
3be0e76 to
3cd0872
Compare
|
@codex review this |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
3cd0872 to
f6f0154
Compare
f6f0154 to
3be800b
Compare
Why
With deferred executors, an environment can become ready between two sampling requests in the same turn. The model-visible environment update, advertised tools, and eventual tool execution must all describe the same request-time view.
Otherwise, a request built while only environment B is ready can advertise a tool without an
environment_id; if higher-priority environment A becomes ready before execution, that call could silently run in A instead.This PR is stacked on #29527.
Design
run_turncaptures oneArc<StepContext>at each sampling-request boundary. That step owns the request'sTurnContextand environment snapshot.ToolCallRuntimeretains theArcwhile asynchronous tool calls execute.ToolInvocationcarries the step to handlers; its temporaryturncompatibility field is derived from the same object.ToolRouterdoes not retainStepContext; it only uses it while constructing the request's tool set.DeferredExecutordisabled, step capture keeps using the environments frozen at turn start.Simply: every sampling request gets one consistent picture of its environments, from what the model sees through where its tool calls run.
What changed
StepContext.apply_patch,view_image, andrequest_permissionsexecution.request_permissions, while no environment is attached.Follow-up
StepContext.Test plan
just test -p codex-core deferred_executor_updates_context_and_tools_after_startupjust test -p codex-core environment_count_controls_environment_backed_toolsjust test -p codex-core build_prompt_input_includes_context_and_user_message