feat(osworld): add OpenSandbox backend with scoped dependency policy - #2308
Conversation
dc38304 to
435ba3b
Compare
|
/claude review |
|
SHIP — no reliability concerns that block merge. Reviewed the full diff. This adds a What I checked and found sound:
One NOTE (author's call, not blocking): |
There was a problem hiding this comment.
Recommended design: keep OpenSandboxProvider SDK-only. OSWorld supplies a non-empty compatibility image (SDK validation only), poolRef, skip_health_check=True, and no execd probe. The returned SDK Sandbox handles connect, status, endpoint lookup, and destroy. After rebasing onto main, the provider should only need a small endpoint() adapter (including server-proxy headers) and trailing-slash normalization; delete the custom REST client, facade, marker reaper, and duplicate lifecycle path.
Minimal E2E using Sandbox.create directly:
import asyncio
import os
from datetime import timedelta
from urllib.parse import urlsplit
import aiohttp
from opensandbox import Sandbox
from opensandbox.config.connection import ConnectionConfig
async def main() -> None:
domain = os.environ["OPENSANDBOX_BASE_URL"].rstrip("/")
api_key = os.environ["OPENSANDBOX_API_KEY"]
connection = ConnectionConfig(
domain=domain,
api_key=api_key,
headers={"OPEN-SANDBOX-API-KEY": api_key},
use_server_proxy=True,
)
sandbox = await Sandbox.create(
image="busybox:1.36", # SDK validation only; the Pool supplies the OSWorld VM.
extensions={"poolRef": "osworld-kvm"},
timeout=timedelta(minutes=10),
connection_config=connection,
skip_health_check=True, # The Pool VM has no execd.
)
try:
endpoint = await sandbox.get_endpoint(5000)
url = str(endpoint.endpoint)
if "://" not in url:
url = f"{urlsplit(domain).scheme or 'http'}://{url}"
headers = dict(endpoint.headers or {})
headers.setdefault("OPEN-SANDBOX-API-KEY", api_key)
async with aiohttp.ClientSession(headers=headers) as session:
async with session.get(f"{url.rstrip('/')}/screenshot") as response:
response.raise_for_status()
print(f"OSWorld screenshot endpoint is ready: {response.status}")
finally:
await sandbox.destroy()
asyncio.run(main())
Thanks @hemildesai — this is very helpful, especially the validation against the actual osworld-kvm Pool. The custom REST path was originally introduced for two reasons: OpenSandbox SDK 0.1.15 rejects an image-less Sandbox.create(), while the Pool supplies the actual OSWorld VM image; and we wanted to cover Given that a non-empty compatibility image is accepted and ignored by the Pool, and that skip_health_check=True works for the Pool VM without execd, I agree that keeping OpenSandboxProvider SDK-only is the cleaner design. I’ll rebase onto the current main implementation and:
One small SDK API detail: in 0.1.15 the lifecycle methods are kill() followed by close() rather than destroy(), so I’ll use that pair in the implementation and tests. I’ll also verify that run-attribution/epilogue cleanup covers the narrow case where a create is accepted server-side but the response never reaches the client. If that is not covered, I’ll treat it as an SDK/server lifecycle issue rather than retain a duplicate REST implementation in Gym. |
435ba3b to
8e6a019
Compare
ee63cb7 to
4380cfd
Compare
hemildesai
left a comment
There was a problem hiding this comment.
The SDK-only Pool direction is much simpler. I found three remaining places where the diff can stay smaller and keep cleanup and endpoint ownership explicit.
3e26a2b to
c9899af
Compare
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Remove the duplicate image-less REST lifecycle and allocate OSWorld Pool VMs through the SDK compatibility-image path. Keep excluded runtime dependencies scoped to the agent with overrides and an explicit opt-in installer. Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Keep generic sync Sandbox cancellation behavior unchanged, scope health-check bypass to OSWorld Pool specs, and centralize endpoint scheme and header policy in the OpenSandbox provider. Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
8611986 to
a1edee0
Compare
Summary
This PR adds a production-oriented OpenSandbox Pool backend for the OSWorld benchmark while preserving the existing Gym Docker/KVM path.
It also fixes the root dependency policy that previously made OSWorld's managed-server environment impossible to express cleanly: dependency exclusions are now scoped to the upstream packages that declare the unwanted edges instead of globally removing those package names from every server environment.
Key outcomes:
gym_opensandboxas an OSWorld execution backend backed by a pre-provisioned, server-managed KVM Pool.requirements.txtconvention and pins OSWorld to an immutable archive.uvversion that supports that policy.Motivation
The existing OSWorld integration assumes that the Gym client owns a Docker container, mounts a local qcow2 image, and can expose the guest services as plain
host:portendpoints. An OpenSandbox Pool changes all three assumptions:poolRef.The goal is therefore not to fork the OSWorld execution path. It is to keep OSWorld's behavior unchanged and translate only the lifecycle and endpoint boundary beneath it.
Architecture
For gateway endpoints, the OSWorld adapter starts one loopback forwarder per guest service:
Design principles
Preserve one OSWorld execution contract
GymSandboxDesktopProvidercontinues to implement OSWorld's existing provider interface. Backend selection changes resource acquisition and endpoint resolution only; task setup, observations, actions, proxy behavior, and evaluators remain on the same code path.Make resource ownership explicit
provider_options.extensions.poolRef; the Pool owns the image, entrypoint, and compute capacity.This prevents local Docker defaults from leaking into a server-managed Pool allocation.
Translate endpoints at the narrowest boundary
OSWorld expects one host and integer ports, whereas OpenSandbox can expose URL paths, headers, and WebSocket routes. The local forwarder performs that translation at the adapter boundary, disables ambient proxy inheritance, and leaves OSWorld itself unaware of transport-specific routing.
Bound failure and cancellation
BaseException.Clean up only owned resources
Each run carries exact run metadata. Normal shutdown releases its Sandbox; the recovery tool first audits and then optionally terminates only instances whose metadata value exactly matches the requested run ID. It checks both the OSWorld and Gym metadata keys and revalidates the match immediately before deletion.
Credentials remain environment-only and are not written to generated
env.yamlfiles.Dependency policy
The OSWorld agent now uses
responses_api_agents/osworld_agent/requirements.txt, matching the managed-server convention used throughoutresponses_api_agents.The prior root
exclude-dependencieslist acted globally when the editable root project was installed into an isolated server environment. That meant a package excluded only because it was an unwanted dependency ofmlflowwas also silently unavailable when OSWorld or another component required it directly. Keeping a component-local workaround would preserve that incorrect repository-wide behavior.This PR fixes the policy at its source:
mlflowexclusions apply only to dependency edges declared bymlflow.agp-clientedge and conflicting GUI OpenCV edges are scoped toosworld.paddleocrandalbumentations.cryptographyremain resolvable for other agents and model servers.uv >= 0.11.25is required because package-scoped exclusions need that resolver behavior; both test workflows install the same minimum version.dc23424e9f6316b181bde149e0dc9bc3c5ff78c9, with the compatible headless OpenCV wheel declared explicitly.This keeps the root policy composable for every managed server instead of special-casing OSWorld around a global exclusion.
User-facing workflow
benchmarks/osworld/prepare.pyaccepts--execution-backend gym_opensandbox, validates that--vm-pathis absent, and renders the Pool reference fromOPENSANDBOX_POOL_REF(default:osworld-kvm). The normal control and evaluation wrappers are unchanged.The OSWorld README documents:
Validation
All validation below applies to the PR patch now published as commit
435ba3bc38373c3b04ccd685f565b89b7066920aon Linux/x86_64 unless noted otherwise. Live artifacts were initially recorded at949a5e5963d241d4179712070fe954d6f7f468d5; adding missing DCO trailers and rebasing onto currentmainchanged only commit metadata and the unrelated base. The completegit diff --binary main...HEADremained byte-for-byte identical, with SHA-25649e8c919c5c39f4054d8d45398d7ba2c11142eb6344d74d65d26ce4206b6cb6d.Dependency and test gates
uv 0.11.25: 178 packages installed.requirements.txt: 336 packages installed; 157 agent tests + 32 benchmark tests = 189 passed.cryptography 49.0.0resolved without a compatibility cap; 110 passed.Live Nano Omni + OSWorld smoke matrix
2ad9387a-65d8-4e33-ad5b-7580065a27ca06fe7178-4491-4589-810f-2e2bc950212259f21cfb-0120-4326-b255-a5b827b389672ad9387a-65d8-4e33-ad5b-7580065a27ca06fe7178-4491-4589-810f-2e2bc950212259f21cfb-0120-4326-b255-a5b827b38967The Docker route used a real qcow2 VM with KVM acceleration (
accel=kvm,-enable-kvm), not a mocked lifecycle. The first independent Docker/proxy sample completed end-to-end but scored 0.0; a second sample scored 1.0, consistent with model sampling variance rather than a transport or evaluator failure.No runtime source overlay or
PYTHONPATHpatch was used. The installed pinned OSWorld package supplied the VLC gateway-auth fallback directly.Cleanup gate
Scope
This branch is based on current
mainafter #1408. It intentionally excludes developer-local ignore rules, local experiment harnesses and paths,.pre-commit-config.yamlchanges, and unrelated vLLM component metadata changes.