Skip to content

fix(sandbox): reconcile Docker creates and resolve E2B service endpoints - #4822

Merged
marcromeyn merged 3 commits into
NVIDIA-NeMo:mainfrom
JeffPengCoder:feature/sandbox-provider-contracts
Oct 26, 2026
Merged

marcromeyn merged 3 commits into
NVIDIA-NeMo:mainfrom
JeffPengCoder:feature/sandbox-provider-contracts

Conversation

@JeffPengCoder

@JeffPengCoder JeffPengCoder commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

This splits the sandbox-provider changes out of #3591, as suggested in its review, so they can land first.

Main already uses fields that only these changes define. #3592 added them to the OSWorld configs, but the provider configs reject them, so neither provider can be built from those configs today:

Config on main Field Error on main (968e069c5)
responses_api_agents/osworld_agent/configs/osworld_agent.yaml (Docker) create.transport_retries, create.transport_retry_delay_s TypeError: DockerCreateConfig.__init__() got an unexpected keyword argument 'transport_retries'
benchmarks/osworld/configs/osworld_agentenv.yaml (E2B) connection.port_routing ValueError: Unknown E2BConnectionConfig keys: port_routing

Changes:

  • Docker. A remote Docker transport can lose the docker run response after the daemon has created the container. create.transport_retries (default 2) and create.transport_retry_delay_s retry with the same fixed name. After a timeout, a transport error or a same-name conflict, the provider inspects that exact name and reuses the container only when both the sandbox ownership label and the requested image match. An unrelated container is never removed.
  • E2B endpoints. endpoint() resolves a declared service port. connection.port_routing selects hostname routing (one DNS name per port) or gateway routing (one origin, with the SDK's per-sandbox headers plus E2b-Sandbox-Port); auto selects gateway when a sandbox URL is configured. Gateway routing needs ConnectionConfig.sandbox_headers, so the SDK range becomes e2b>=2.46.0,<3.0.0 in the dependency, the installation hint, the README and the example. A test checks that they agree.
  • E2B transport. The provider borrows Gym's aiohttp pool only when the process already has Gym's global config. Otherwise it keeps the SDK's own transport, so a Ray worker never reaches Hydra's argument parser before creating a sandbox.
  • OpenSandbox. endpoint() reports an actionable error for SDKs without Sandbox.get_endpoint and tolerates responses without endpoint headers.

These changes come from #3591's head (6055f9ea3). #3591 merged without them (8af4df7ef), so main still lacks them. On main 92407083b the two configs above still set fields that the providers do not define, and the E2B provider still has no endpoint(). There is no separate issue: this is a split of #3591.

Waiting on this PR: #4897 (Nano Omni recipe docs) should merge after it. The draft #4896 carries a copy of the same files.

Validation

October 22: rebased on main 92407083b.

These checks used macOS ARM64, Python 3.13.14, and the dev and sandbox dependencies from the lockfile (E2B SDK 2.46.0):

  • The 15 test files that exercise the sandbox providers: 741 passed, 6 skipped, 8 failed. The files are:
    • tests/unit_tests/telemetry/test_sandbox_create_retry.py;
    • test_docker_provider.py and test_tb4_harbor_sandbox_overlay.py;
    • test_e2b_build.py, test_e2b_endpoint.py, test_e2b_provider.py and test_e2b_sdk_transport_fallback.py;
    • test_opensandbox_cleanup.py, test_opensandbox_compose.py, test_opensandbox_http_transport.py, test_opensandbox_provider.py, test_opensandbox_pty.py and test_opensandbox_snapshots.py;
    • test_sandbox_stop_retry.py and test_sandbox.py.
  • The same 8 tests also fail on unmodified main and are specific to this macOS machine:
    • test_script_help_runs_by_direct_path in test_opensandbox_cleanup.py;
    • test_script_help_runs_by_direct_path in test_opensandbox_snapshots.py;
    • the six test_slurm_eval_preserves_output_paths_and_explicit_resume cases.
  • pre-commit run on the changed files and uv lock --check: passed.

Original head 726a4746e on main 968e069c5. On macOS ARM64 with Python 3.13.14 and the dev and sandbox dependencies (aiohttp 3.14.4, FastAPI 0.135.2, real E2B SDK 2.46.0):

  • Provider tests (tests/unit_tests/test_docker_provider.py, test_e2b_endpoint.py, test_e2b_provider.py, test_e2b_sdk_transport_fallback.py, test_opensandbox_provider.py): 327 passed.
  • The core unit suite plus the vLLM, simple-agent, compaction-agent and OSWorld tests, run on this branch and on its base, main 968e069c5: 10,913 passed here against 10,884 on main; the 31 new tests all pass. The remaining 27 failures are identical on main and are specific to this macOS machine. Two timing-sensitive tests, the test_checkpoint_agent restart seed and the test_cli_eval_submit invalid config, each failed once under the 8-worker run and passed when rerun alone (8/8 and 3/3). Neither imports the provider code.
  • Both OSWorld configs above now build their provider configs, checked with the providers' own parsers after OmegaConf resolution. They fail on main as shown.
  • pre-commit run on the changed files and uv lock --check --offline: passed.

Rollouts

The E2B changes ran end to end in the October 20 OSWorld score reproduction on #3591, whose head contains these exact files. That run covered 361 tasks, with Nemotron-3-Nano-Omni served by vLLM and desktops behind an AgentEnv gateway using port_routing: auto. It scored 48.13% raw and 166/361 binary, with no degradation; details are in #3591. The Docker reconciliation and the OpenSandbox change are covered by unit tests only.

Compatibility and benchmark impact

  • With the default transport_retries: 2, an ambiguous Docker create (timeout, transport error or same-name conflict) is reconciled and retried up to twice instead of failing at once. Other docker run failures still fail immediately.
  • The sandbox extra raises the E2B SDK lower bound from 2.36.0 to 2.46.0.
  • New config fields have defaults. Benchmark impact: none expected; the reproduction above showed no degradation.

Checklist

  • I have read the contributing guidelines.
  • The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs.
  • Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable).
  • Documentation added or updated, or N/A with justification. (E2B provider README.)
  • Pre-commit checks pass locally (pre-commit run --all-files) (so CI lint/format checks pass).
  • New source files include the required Apache-2.0 SPDX header.
  • All commits have DCO sign-off (git commit -s) (so the DCO check passes).

@JeffPengCoder
JeffPengCoder requested a review from a team as a code owner October 21, 2026 03:53
@copy-pr-bot

copy-pr-bot Bot commented Oct 21, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@svcnvidia-nemo-ci svcnvidia-nemo-ci added area:sandbox Sandbox providers, isolation, and sandbox runtime contracts bug Something isn't working needs-review PR is ready for code review and waiting on a reviewer labels Oct 21, 2026
JeffPengCoder added a commit to JeffPengCoder/Gym that referenced this pull request Oct 21, 2026
…#4822 and NVIDIA-NeMo#4823

The sandbox-provider changes (Docker create reconciliation, E2B service endpoints and transport fallback, OpenSandbox endpoint compatibility, the e2b>=2.46.0 range) now live in NVIDIA-NeMo#4822, and the owner-loop close of the shared aiohttp clients with wildcard-host handling lives in NVIDIA-NeMo#4823. Restore those 16 files to this branch's main base so each change is reviewed in exactly one PR. Nothing else in this PR uses them; it keeps the component runtime policy, rollout identity and the vLLM transport-log context.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
zyzhou5 pushed a commit to zyzhou5/Gym that referenced this pull request Oct 21, 2026
…VIDIA-NeMo#3591)

* fix(core): isolate runtime, sandbox and model transport contracts

Extract shared support from JeffPengCoder/Gym 360b0df. Include E2B endpoint/transport and vLLM model changes; retain upstream GymAiohttpTransport while preserving the no-config Ray worker fallback. Keep OSWorld tasks, agents and resources out of this PR.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(e2b): align SDK installation guidance with gateway requirements

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(cli): keep site-packages off server import paths in wheel installs

In a wheel install the built-in servers live in the parent interpreter's site-packages, so dir_path.parent.parent is that site-packages. Passing it as project_root put it first on each server's PYTHONPATH and NEMO_GYM_EXTRA_ROOTS, ahead of the server venv's own pinned dependencies. Apply the source-checkout guard the verifier runner already uses, and test both layouts; the wheel-install case fails without the guard.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(rollout): keep index-derived capture ids and gate dispatch logging

Rows without a caller-supplied _ng_rollout_id no longer receive a random rollout-<uuid4>. The capture key stays main's {task}-{rollout}, so a resumed attempt decodes to the same rollout as attempt 0 and existing capture files keep their names. A caller-supplied id is still forwarded unchanged. Rows are no longer mutated, so the per-dispatch deep copy is removed as well.

The per-dispatch summary is printed only when request debugging is enabled, like the reverification summary. The summary drops the NeMo-RL purpose and trajectory-identity fields and adds only the core _ng_rollout_id.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(vllm): drop the per-request purpose marker

The marker printed on every chat-completion request and ran before sampling overrides and capture/prefix preparation, so it could report temperature and top_p values the engine never received. The opt-in transport log (NEMO_GYM_VLLM_TRANSPORT_LOG) already records the final request body together with the rollout context headers. A test now checks that the log holds the post-override values and that nothing is printed per request.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* chore(agent): drop the legacy compaction session change

Publishing the final policy decision only after guard admission changes nothing observable today: a guard rejection raises before build_response, the only place the decision is published. NVIDIA-NeMo#3368 deletes this session, and its replacement client checks guards before generation and acknowledges history only after accepting a response, so the invariant belongs there. Restore session.py and its test to main's versions; this also removes the only conflict with NVIDIA-NeMo#3368.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* chore(core): leave provider and HTTP lifecycle changes to NVIDIA-NeMo#4822 and NVIDIA-NeMo#4823

The sandbox-provider changes (Docker create reconciliation, E2B service endpoints and transport fallback, OpenSandbox endpoint compatibility, the e2b>=2.46.0 range) now live in NVIDIA-NeMo#4822, and the owner-loop close of the shared aiohttp clients with wildcard-host handling lives in NVIDIA-NeMo#4823. Restore those 16 files to this branch's main base so each change is reviewed in exactly one PR. Nothing else in this PR uses them; it keeps the component runtime policy, rollout identity and the vLLM transport-log context.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(core): keep server venvs on the parent Python and fail loudly on pin conflicts

- A server's .python-version now selects its interpreter only when it excludes the
  parent's Python. A request that admits it (such as >=3.13.14,<3.14, 3.13 or
  cpython@3.13) builds the venv with the parent's exact version, because Ray rejects
  processes on different patch releases. Variant and pre-release requests
  (3.13+freethreaded, 3.13.14+debug, 3.13.14rc1) and requests for other implementations
  are passed to uv unchanged. Like uv, the first non-comment line is the request.
- Only the parent's constraints (the Ray pin) are passed as uv overrides, through process
  substitution like the existing -c flag. head_server_deps such as openai stay ordinary
  requirements, so a server pin that conflicts with them fails setup instead of being
  replaced silently.
- Drop the environment-identity file: nothing read it, so it could not detect a stale venv,
  and its tests passed whatever it contained.
- Replace the vacuous vllm_model marker-file test with one that checks the real policy, and
  document per-server .python-version/uv.toml and NEMO_GYM_ALLOWED_COMPONENT_ROOTS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

---------

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Co-authored-by: Onur Yilmaz <35306097+oyilmaz-nvidia@users.noreply.github.com>
Co-authored-by: yaoyu-33 <yaoyu.094@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Main's OSWorld configs already set create.transport_retries, create.transport_retry_delay_s (Docker) and connection.port_routing (E2B), but the provider configs do not define them, so both providers fail to build from those configs. Split from NVIDIA-NeMo#3591 so the provider contracts can land first.

- Docker: a remote Docker transport can lose the docker run response after the daemon created the container. Retry with the same fixed name, and after a timeout, transport error or same-name conflict, inspect that exact name and reuse it only when the sandbox ownership label and the requested image both match. An unrelated container is never removed.
- E2B: endpoint() resolves a declared service port. connection.port_routing selects hostname routing (one DNS name per port) or gateway routing (one origin; the SDK's per-sandbox headers plus E2b-Sandbox-Port); auto picks gateway when a sandbox URL is configured. Gateway routing needs ConnectionConfig.sandbox_headers, so the SDK range becomes e2b>=2.46.0,<3.0.0 in the dependency, installation hint, README and example.
- E2B transport: borrow Gym's aiohttp pool only when the process already has Gym's global config; otherwise keep the SDK's own transport, so a Ray worker never reaches Hydra's argument parser before creating a sandbox.
- OpenSandbox: endpoint() reports an actionable error for SDKs without Sandbox.get_endpoint and tolerates responses without endpoint headers.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
@marcromeyn

Copy link
Copy Markdown
Contributor

/ok to test bb7d9d3

@yaoyu-33 yaoyu-33 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Reviewed the Docker creation reconciliation and E2B endpoint wiring; no new blocking issues found.

@yaoyu-33 yaoyu-33 added ready-to-merge PR is approved, current, and waiting for final CI or merge and removed needs-review PR is ready for code review and waiting on a reviewer labels Oct 26, 2026
@github-actions github-actions Bot removed the sla:triage-overdue Review assignment is over the one-business-day SLA label Oct 26, 2026
@yaoyu-33

Copy link
Copy Markdown
Contributor

/ok to test 5c6e5d1

@github-actions

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40415660184

Merged via the queue into NVIDIA-NeMo:main with commit 6416352 Oct 26, 2026
53 of 56 checks passed

This branch was previously deployed

1 inactive deployment
public — 5c6e5d10 Deployed Oct 26, 2026 by copy-pr-bot[bot] via release / finalize / notify #7428
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:sandbox Sandbox providers, isolation, and sandbox runtime contracts bug Something isn't working ready-to-merge PR is approved, current, and waiting for final CI or merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants