Fix v1 MCP setup in task sandboxes#1391
Conversation
ApprovabilityVerdict: Needs human review This PR significantly changes how MCP packages are installed and executed in task sandboxes, including new isolated installation via uv, shell command generation, and Python path discovery logic. These runtime infrastructure changes warrant careful human review. You can customize Macroscope's approvability policy. Learn more. |
1049158 to
284a2cc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 284a2cc075
ℹ️ 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".
| 'env PYTHONPATH="$VF_UV_SITE_PACKAGES" "$PYTHON" -m uv --no-config tool install ' | ||
| f"--python 3.11 --with {shlex.quote(requests_package)} {mcp_package_args}\n" | ||
| f'printf "%s\\n" "$UV_TOOL_DIR/mcp/bin/python" > {shlex.quote(MCP_PROXY_PYTHON_PATH)}\n' |
There was a problem hiding this comment.
Pass a single tool package to
uv tool install
This builds uv ... tool install ... {mcp_package_args} from every sandbox.packages entry starting with mcp, but uv tool install --help shows Usage: uv tool install [OPTIONS] <PACKAGE> (exactly one positional package). If a sandbox includes more than one mcp* requirement (for example the default mcp>=... plus mcp-agent), setup will fail with unexpected argument ... before installation completes, which breaks MCP-enabled rollouts that previously accepted multiple packages via pip install.
Useful? React with 👍 / 👎.
Summary
Testing
Note
Medium Risk
Changes sandbox package installation and MCP proxy invocation logic, which can affect how user programs/tools run inside task sandboxes (dependency resolution and interpreter selection). Risk is mitigated by added/updated unit tests but could still surface in diverse base images/environments.
Overview
Fixes v1 MCP execution in task sandboxes by recording and reusing a sandbox-resolved Python interpreter for the MCP proxy instead of hardcoding
python3.proxy_command()now runs via/bin/sh -lc, reads the interpreter path fromMCP_PROXY_PYTHON_PATH(fallback topython3), and tests are updated to assert the new command shape.Hardens sandbox dependency installation:
python_package_install_command()now ignores image-level pip configuration (clearsPIP_CONFIG_FILEand relatedPIP_*env vars) and, whenmcpis requested, bootstrapsuvto install MCP under Python 3.11, writing that interpreter path toMCP_PROXY_PYTHON_PATH. Adds targeted tests plus a real-sandbox test that runs MCP via the recorded interpreter path.Reviewed by Cursor Bugbot for commit 284a2cc. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix MCP proxy setup in v1 task sandboxes to use a dynamically installed Python interpreter
python_package_install_commandin sandbox_utils.py to separatemcp*packages from others:mcppackages are installed viauv tool installwith a pinned Python 3.11, while non-mcp packages usepip./tmp/vf_mcp_python(constantMCP_PROXY_PYTHON_PATH) so it can be referenced at runtime.proxy_commandin mcp_proxy_utils.py to wrap execution in/bin/sh -lc, reading the interpreter path fromMCP_PROXY_PYTHON_PATHand falling back topython3if missing.PIP_CONFIG_FILE=/dev/nulland unsets pip index env vars to prevent image-level pip config from interfering./bin/sh -lc) instead of directpython3invocations, and pip config from the sandbox image is explicitly ignored during package installation.Macroscope summarized 284a2cc.