Add sandbox providers for CoreWeave and Wandb#890
Conversation
|
Cool pull request. Thanks. Reviewing now. |
sergiopaniego
left a comment
There was a problem hiding this comment.
Thanks for the PR! Some comments from a first pass:
Transport security (S1)
Both ModalProvider and ACASandboxProvider enforce https/wss transport via _require_secure_url() (RFC 002 invariant S1), because EnvClient derives the WebSocket URL from base_url (http:// becomes cleartext ws:// in convert_to_ws_url). CWSandboxProvider defaults to url_scheme="http" and returns http://{service_address} when the address has no scheme. Combined with ingress_mode="public" by default, client traffic can travel in cleartext against a public endpoint. EnvClient does not block this on its own (it only skips the proxy for localhost, no cleartext warning for remote hosts). Could we add the same _require_secure_url guard (or force/validate https) before returning the URL?
Public ingress opt-in (S2)
ACASandboxProvider requires an explicit anonymous_port=True (RFC 002 invariant S2, "public exposure is never implicit"). Here ingress_mode="public" is the default with no explicit opt-in or warning. Could we require an explicit opt-in (or at least warn) to match the ACA pattern?
Docs
The provider table and "Per-provider setup" section in docs/source/guides/runtime-providers.md, plus the container-providers section in docs/source/reference/core.md, are not updated, so these two providers are currently undocumented. Adding table rows and setup blocks for CWSandbox and W&B (with a "Full example" link, like Daytona/Modal have) would keep the providers doc complete.
Examples
The PR adds an example for W&B (the subclass) but not for CWSandboxProvider, which is the base provider holding all the logic. It would be good to add a CWSandbox example (or generalize the current one to cover both). A couple of style nits to align the W&B example with modal_echo_env.py:
- The
try/except Exception: provider.stop_container(); raisearoundfrom_docker_imageplus a separateasync with envmixes two cleanup styles. Modal uses the cleanerwith Provider() as provider: start_container / wait_for_ready / stoppattern. - Prefer
loggingoverprintto match the other examples. - The example references
registry.hf.space/openenv-echo-env:latestwith a hardcodedSERVER_CMD, whereas Modal/Daytona build the image withProvider.image_from_dockerfile(...). A short note on where that image comes from would help.
RFC status
The PR marks RFC as "Not required", but Modal and ACA landed under the proposed "Cloud Sandbox Providers" amendment to RFC 002, which is where the S1/S2 invariants come from. A new cloud provider with public ingress should reference and satisfy those invariants rather than opt out (ties into the two points above).
Minor
wait_for_readydocstring says "respond to /health and /ws" but only polls/health.- The two new provider files and the two test files are missing the BSD copyright header that all other providers (and
__init__.py) carry. - If S1/S2 get implemented, a test asserting the resulting URL is secure and that implicit public ingress is rejected would be worth adding.
|
Adding one stack-compatibility note, and +1 to the S1/S2 feedback above.
|
|
🙏 Thanks so much for the thoughtful review. I really appreciate you taking the time, and I’ll address the feedback. |
Summary
Adds OpenEnv support for running environments in both CoreWeave Sandboxes and W&B Serverless Sandboxes.
Includes optional
cwsandboxandwandbextras, a W&B sandbox echo example, and unit tests for the new sandbox providers.Type of Change
Alignment Checklist
Before submitting, verify:
I have read
.claude/docs/PRINCIPLES.mdand this PR aligns with our principlesI have checked
.claude/docs/INVARIANTS.mdand no invariants are violatedI have run
/pre-submit-pr(orbash .claude/hooks/lint.shand tests) and addressed all issuesNote:
/pre-submit-prwas run via its underlying checks. The only remaining repo-wide formatting failures (uv run usort check src/ tests/anduv run ruff format src/ tests/ envs/ --check) reproduce onorigin/mainin a separate worktree, so they are pre-existing unrelated formatting drift and not new issues introduced by this PR.RFC Status
Test Plan
Claude Code Review
N/A
Note
Medium Risk
New remote-runtime path runs user container images with public ingress and executes shell in sandboxes; misconfiguration could orphan sandboxes or expose workloads, though cleanup and secret-handling mitigations are included.
Overview
Adds optional CoreWeave and W&B Serverless Sandbox integrations so OpenEnv environments can run on remote sandboxes instead of only local Docker or other existing providers.
CWSandboxProviderimplements the sharedContainerProvidercontract: launch an image viacwsandbox, wait until RUNNING, then start the OpenEnv uvicorn server in the sandbox (because the SDK’s default keepalive does not run the env server). It can discover the server command fromopenenv.yamlor accept an explicitcmd, builds the client base URL fromservice_address, polls/health, and on teardown stops and deletes the sandbox (with retries and guards against double-start). Startup failures avoid leaking injected env secrets in errors unlesssurface_server_logs=True(with best-effort redaction).WandbSandboxProvidersubclasses that logic and swaps in thewandb.sandboxSDK plus W&B-specific preflight and integration metadata.pyproject.tomlgains optionalcwsandboxandwandbextras (Python ≥3.11). A newexamples/wandb_sandbox_echo_env.pydemonstrates Echo on W&B sandboxes. Unit tests cover lifecycle, auth, URL scheme, and log redaction.Reviewed by Cursor Bugbot for commit 77eb3bc. Bugbot is set up for automated code reviews on this repo. Configure here.