fix(computer): isolate provider instances across concurrent runs - #3843
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8113663664
ℹ️ 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".
| dataclasses.replace(tool, computer=resolved_by_tool[tool]) | ||
| if isinstance(tool, ComputerTool) |
There was a problem hiding this comment.
Preserve concrete ComputerTool identity for hooks
When a ComputerTool already wraps a concrete Computer, this still replaces it with a fresh ComputerTool even though there is no provider state to isolate. That copy is later stored in ToolRunComputerAction, so ComputerAction.execute passes the copy to on_tool_start/on_tool_end and custom-data callbacks; hooks that key off the configured agent.tools[0] object stop recognizing their tool on ordinary concrete-computer runs. Please avoid cloning already-resolved computer tools, or otherwise preserve the original tool identity for hook-facing surfaces.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, good catch. You're right that cloning a concrete ComputerTool unnecessarily changes its hook-facing identity. I'll update the implementation so only provider/factory-backed tools receive run-local copies, and add a regression test confirming concrete tools preserve their original identity.
Summary
This pull request fixes
ComputerToolprovider isolation when the same agent is reused by concurrent runs.Per-run computer providers were introduced in #2191 so applications can reuse an agent while creating and disposing independent computer resources for each request. The existing initialization cache is run-scoped, but model serialization still observes the mutable
computerfield on the shared tool.initialize_computer_tools()now returns run-localComputerToolcopies for provider/factory-backed tools, bound to the computer resolved for the currentRunContextWrapper. The original tool continues to own the existing per-run cache and disposal lifecycle, while model serialization and action processing receive stable run-local state instead of reading a shared mutable field. Tools configured with an existing concreteComputerretain their original identity for hooks and custom-data callbacks.This prevents preview-compatible Responses requests from serializing another run's computer dimensions or failing when a sibling run restores the shared tool to its provider during cleanup. The change does not alter the public API, the wire payload, or the GA
computertool behavior.Test plan
Runner.run()regression test that interleaves two runs sharing one agent and provider-backedComputerTool.ComputerToolinstances preserve their identity in model input and run hooks.codex review --uncommittedwith no actionable findings.All formatting, lint, type-checking, and test steps passed.
Issue number
Closes #3842
Checks
.agents/skills/code-change-verification/scripts/run.sh/reviewbefore submitting this PR