feat: sandbox_agent swe resources server#2048
Conversation
Signed-off-by: Christian Munley <cmunley@nvidia.com>
ffrujeri
left a comment
There was a problem hiding this comment.
I'm keeping this pass at the design level — three design questions below, anchored in three inline comments. More granular code-level notes (grading fidelity vs the official harness, error handling, tests) can follow as a separate pass if useful.
Background
- SWE-bench — paper (arXiv 2310.06770) · repo · SWE-bench_Verified dataset · multilingual leaderboard
- R2E-Gym — paper (arXiv 2504.07164) · repo
- opencode CLI (the in-sandbox harness) · python-build-standalone (portable in-sandbox runtime) · swebench 4.1.0 (PyPI)
Where this PR sits
Dataset rows (environments/swe/) drive sandbox_agent, which provisions a per-task sandbox, runs the opencode harness inside it against the host model server, and captures the repo's git diff as model_patch; anyswe then re-grades that patch in a fresh sandbox with the official SWE-bench eval scripts.
flowchart LR
DS[Dataset rows<br/>environments/swe/] --> DRV["Rollout driver<br/>(ng_collect_rollouts / eval loop,<br/>POSTs /run per task row)"]
DRV -->|"/run"| SA["sandbox_agent run()"]
SA -->|provision| BOX1[Rollout sandbox<br/>opencode harness]
BOX1 -->|chat| MS[Model Server]
BOX1 -->|git diff → model_patch| RS[anyswe verifier]
RS -->|fresh box, official<br/>swebench eval| BOX2[Grading sandbox]
BOX2 -->|"eval log"| RS
RS -->|"reward 1/0"| DRV
DRV --> RL[(rollouts.jsonl / training)]
Design discussion
These aren't review nits — they're design choices we're actively weighing for how sandboxed environment execution should work in Gym generally. This PR is the first consumer of that machinery, so it's the right moment to align on direction; none of them needs to block the PR itself.
Design question #1: where should the config-vs-data boundary sit for the sandbox knobs?
What's implemented today: the provider, credentials, and box size are fixed at server startup via Hydra config, while the image, workdir, and grading spec arrive per task in dataset rows. For the provider + credentials half, that placement looks exactly right — credentials stay in env vars, one provider client is built at model_post_init and reused across tasks, runs stay reproducible, and data can never swap the provider — none of that is being questioned. The open question is the middle of the spectrum: box sizing and related spec knobs, where the code currently gestures at per-task configurability without delivering it, and the boundary itself (including its trust implications) is undocumented.
For clarity, the environment involves two separate sandboxes with independently configured providers — the config/data boundary is drawn differently on each side, and that asymmetry is part of what's flagged below:
| Rollout ("solve") sandbox | Grading sandbox | |
|---|---|---|
| Provisioned by | agent server (sandbox_agent, per /v1/responses) |
resources server (anyswe, fresh box per verify()) |
| Provider + credentials | startup config (SandboxAgentConfig.sandbox_provider) |
startup config (AnySweConfig.sandbox_provider) — a separate config; may even be a different backend |
| Box size (cpu/mem/disk) | startup sandbox_spec only — per-server, not per-task |
nowhere: no sandbox_spec on AnySweConfig, and the per-task hatch in build_spec is unreachable (below) |
| Image | per-task (row's docker_image) |
per-task (same row metadata, rebuilt pristine) |
The two-box split itself is a good hermeticity property — the model never touches the box it's graded in; anyswe rebuilds from the pristine instance image and applies only the extracted patch. The caveat worth documenting: the two providers can diverge across deployments (different exec shells, different default timeouts), which is a reward-fidelity consideration.
So the concrete questions, each documentation-or-small-code:
- The grading sandbox currently cannot be sized at all:
AnySweConfigexposes nosandbox_spec, andbuild_spec's per-taskresources/provider_optionskeys are unreachable becauseverify()forwards onlyinstance_dictintotask.metadata(app.py:724). Either forwarding those keys fromverifier_metadataor removing them would make the real capability match the code. - Symmetrically, the solve sandbox takes cpu/mem/disk only from startup
sandbox_spec, so a heavy instance can't request a bigger box on either side. If that's intentional (throughput predictability), a README sentence would prevent surprise. - The trust model is implicit: any
/v1/responsescaller can name an arbitrarydocker_imageand, viasandbox_eval, arbitrary in-box shell. Fine for operator-owned deployments — a README note ("not for untrusted callers") would make the boundary explicit.
Design question #2: could responses() stay out of the provisioning business?
Right now responses() carries the placement machinery — provider client, gym source tarball, setup_commands bootstrap, network probe, patch extraction, and the optional in-box grading — on top of its actual job (request → trajectory).
Concretely, in responses() (app.py:341-386):
image = meta.get("docker_image") or self.config.sandbox_image # L347 placement input from the row
handle = await self._provision_box(image, files, model_url) # L364 provisions the sandbox
r = await self._provider.exec(handle, runner_cmd, timeout_s=...) # L366 runs the harness in-box
resp = NeMoGymResponse.model_validate(
await self._download_json(handle, "/work/response.json")) # L370 artifact retrieval
r = await self._provider.exec(handle,
f"git -C {wd} add -A . && git -C {wd} diff --cached", ...) # L374 patch extraction
reward = await self._grade_in_box(handle, grade_spec) # L382 in-box gradingand _provision_box (app.py:250-293) is a full provider lifecycle: provider.create(spec) (L252), file uploads (L255-259), gym-source tarball fetch + extraction (L260-275), setup_commands execution (L276-279), and a TCP reachability probe of the model endpoint (L281-289) — none of which is "produce a trajectory".
Since the repo's own decomposition assigns per-task state (the execution context) to resources servers, an alternative shape that may age better as more sandboxed environments arrive is a dedicated sandbox-broker resources server — a third server that provisions and leases boxes to whichever component needs one (the agent's rollout box, the verifier's grading box):
- A dedicated sandbox-broker resources server owns provisioning. It holds the one provider client + credentials and exposes lease-style endpoints (create / exec / upload / download / release, with TTLs and quotas); the agent server and the task/verifier server are both just clients of it. Because Gym components are composable, this can be built today as an ordinary resources server — no core-framework change.
responses()becomes thin: it leases a box for the row'sdocker_imagefrom the broker and launches the harness through the handle — nocreate_provider, no tarball, no per-request bootstrap. Its contract returns to "request → trajectory".verify()leases its own fresh grading box from the same broker and extracts thegit diffitself rather than reading agent-attachedmodel_patch— capture policy (e.g. whether untracked scratch files the agent leaves behind should enter the patch) becomes verifier-owned, thesandbox_evalin-agent grading path can disappear (reward computation living in the agent cuts against the agent/verifier split), and the task server itself stays stateless. One provider policy, one concurrency bound, one cleanup path for both box types — instead of the two independently configured providers in the table above.
flowchart LR
DRV["Rollout driver<br/>(ng_collect_rollouts, POSTs /run)"] -->|"/run per task row"| RUN["run()<br/>(host orchestration)"]
RUN -->|"1. seed_session"| RS["Task resources server<br/>(anyswe — verify logic only,<br/>stateless)"]
RUN -->|"2. /v1/responses"| AG["Agent responses()<br/>(thin: request → trajectory)"]
AG -->|"lease box (row's docker_image)"| SB["Sandbox-broker resources server<br/>(owns provider + creds,<br/>leases, quotas, cleanup)"]
SB -->|"provision"| BOX1["Rollout sandbox"]
AG -->|"launch harness via handle"| BOX1
BOX1 -->|"model calls"| MS["Model Server (host)"]
RUN -->|"3. verify"| RS
RS -->|"lease fresh grading box"| SB
SB -->|"provision"| BOX2["Grading sandbox"]
RS -->|"extract git diff, grade,<br/>release lease"| BOX2
Supporting changes that make (2) honest: prebuilt harness images (a thin gym-harness layer over the instance images, or a versioned wheel fetched via gym_source) instead of per-request tarball + pip + installer downloads; and documenting the runner's files-in/files-out contract (request.json → response.json) as a versioned interface, so "the nested agent's responses() must be self-contained" is a stated requirement rather than a discovered one.
To be fair to the current shape: the provider API is exec/upload/download-only (no inbound networking into sandboxes), which rules out the cleanest alternative — running the nested agent as a real server in the box and reverse-proxying to it — and brokered handles need lease semantics (ownership, TTLs, cleanup on crash) that don't exist yet. So a possible phasing:
- PR-sized now: extract the provisioning/bootstrap/extraction into a
SandboxRuntimemodule the agent calls (pure layering, no behavior change, independently testable), and consider movingsandbox_evalgrading to the verifier. - Next: prebuilt harness images as the recommended path, with the auto-tar demoted to a dev fallback.
- Later: the sandbox-broker resources server itself — provider credentials, leases, quotas, and observability in one place, consumed by both agents and verifiers — making the "execution environment" something any component can request without owning a provider.
Where this could end up (follow-up-sized, not this PR): port exposure in the provider API. The exec-only constraint is itself incremental to lift: if SandboxProvider grew one optional primitive — expose/tunnel a sandbox port to a host-reachable URL — the nested harness could run as its normal FastAPI server inside the box, and SandboxAgent.responses() would collapse to a pure reverse proxy. That removes the files-in/files-out runner contract entirely (the harness gets a real Request, a real ServerClient, streaming, health checks — no importlib bootstrap in a bare interpreter), and providers without port exposure would simply keep today's exec-based runner as the fallback.
flowchart LR
subgraph BOX["Task sandbox"]
NA["Nested agent server<br/>(unchanged harness running<br/>as its own FastAPI app)"]
end
DRV["Rollout driver<br/>(ng_collect_rollouts)"] -->|"/run → /v1/responses"| AG
AG["SandboxAgent responses()<br/>(pure reverse proxy)"] -->|"proxied HTTP via<br/>provider port tunnel"| NA
NA -->|"model calls"| MS["Model Server (host)"]
PRV["SandboxProvider<br/>+ expose_port() — new capability"] -.->|"provision + tunnel"| BOX
Would this direction fit your intent, or is agent-owned provisioning deliberate (e.g. keeping the verifier stateless and the two servers' providers uncoupled)? Entirely reasonable as follow-up work rather than in this PR — flagging it now because the layering will be much cheaper to change before more environments build on sandbox_agent. If the port-exposure capability sounds right, I can open a tracking issue for it so it doesn't get lost.
Design question #3: should runtime placement live in an agent server at all?
Drawing the two modes as shipped makes the shape easier to discuss. agent_only_runner:
flowchart TB
subgraph HOST["Host Gym deployment"]
DRV["Rollout driver<br/>(ng_collect_rollouts)"] -->|"/run"| SA["sandbox_agent server"]
SA -->|"seed_session / verify"| RS["anyswe resources server"]
MS["Model server"]
end
subgraph BOX["Task sandbox (row's docker_image)"]
RUNNER["agent_runner.py<br/>importlib-loads the harness class,<br/>calls its responses() in-process"]
end
SA -->|"provision; upload runner,<br/>request.json, gym tarball"| BOX
RUNNER -->|"model calls"| MS
SA -->|"download response.json;<br/>git diff → model_patch"| BOX
and gym_runner, where the recursion is the interesting part — the host driver calls an agent that boots a second rollout driver inside the box:
flowchart TB
subgraph HOST["Host Gym deployment"]
DRV["Rollout driver<br/>(ng_collect_rollouts)"] -->|"/run"| SA["sandbox_agent server<br/>(mode: gym_runner)"]
MS["Model server"]
end
subgraph BOX["Task sandbox"]
NDRV["nested ng_collect_rollouts<br/>(a second rollout driver)"] -->|"/run"| NGYM["ng_run: full nested Gym<br/>(agent + resources servers)"]
end
SA -->|"provision; start nested Gym;<br/>poll done-file every 20s"| BOX
NGYM -->|"model calls"| MS
SA -->|"download rollouts.jsonl<br/>(already verified in-box)"| BOX
The mode switch is really placement policy — "where does this rollout execute, and at what granularity (just the harness, or the whole environment)?" — encoded as agent config. Two consequences of hosting that policy here: the agent re-implements driver concerns (concurrency bound, rollout_timeout, done-file polling, log tailing, artifact download — all things the rollout driver already owns), and in gym_runner mode a workload component re-instantiates its own orchestrator one level down, so run() has two different contracts depending on a config flag (external verify vs in-box verify).
An alternative worth considering: make runtime placement an upstream responsibility — the environment's config declares its runtime, and the rollout driver (or a small launcher layer beneath it) enforces it:
flowchart TB
CFG["Environment config<br/>runtime: host | sandbox{provider,<br/>image: row metadata, granularity}"] --> DRV
DRV["Rollout driver / launcher<br/>(owns placement, timeouts,<br/>retries, artifact collection)"]
DRV -->|"runtime: host (default)"| HA["Agent server on host<br/>(unchanged)"]
DRV -->|"runtime: sandbox"| BOX
subgraph BOX["Task sandbox"]
SAME["The same agent server / harness,<br/>launched in-box — code unchanged,<br/>unaware of placement"]
end
HA -->|"seed_session / verify"| RS["Resources server"]
SAME -->|"model calls"| MS["Model server"]
SAME -->|"artifacts"| DRV
Under that shape the two modes dissolve into a single granularity knob on the placement policy ("place the agent" vs "place the whole environment"), the polling/tarball/done-file machinery is implemented once at the layer that already owns orchestration, and harness code runs host-side or in-box without knowing the difference. Today's sandbox_agent would become the launcher's implementation detail rather than a harness impersonating one.
To be fair, this is the biggest-lift option of the three questions — the current driver is deliberately thin (it just POSTs /run), and using a custom agent server was the extension point the framework actually offers, which is why this PR's shape is a reasonable v1. Do you read the two modes the same way — as placement policy that ideally migrates upstream once a launcher/runtime abstraction exists — or is agent-hosted placement a deliberate long-term choice? If the former, this and the port-exposure capability could share one "sandbox runtime" tracking issue.
Minor: the branch is currently behind main; a rebase would freshen CI. The only existing comment thread is the copy-pr-bot notice, so nothing else pending.
| finally: | ||
| await self._close_box(handle) | ||
|
|
||
| async def responses( |
There was a problem hiding this comment.
responses_api_agents/sandbox_agent/app.py:341
Question on layering rather than a defect: was it a deliberate choice for responses() to own sandbox provisioning, gym-source shipping, and artifact extraction — rather than, say, leasing the box from a dedicated sandbox-broker resources server (per-task state being resources servers' charter) and keeping this endpoint request→trajectory? A concrete alternative sketch with a phased plan is in the review summary under "Design question #2" — curious whether broker-provided execution environments would fit your intent, or whether agent-owned provisioning is deliberate (stateless verifier, providers uncoupled across servers).
| **({"slurm_job_id": str(task.metadata["slurm_job_id"])} if task.metadata.get("slurm_job_id") else {}), | ||
| }, | ||
| resources=SandboxResources.from_mapping(task.metadata.get("resources", {})), | ||
| provider_options=dict(task.metadata.get("provider_options", {})), |
There was a problem hiding this comment.
resources_servers/anyswe/app.py:430
These per-task resources/provider_options keys are currently unreachable: verify() forwards only {"instance_dict": ...} into task.metadata (app.py:724), and AnySweConfig exposes no sandbox_spec either — so the grading sandbox can't be sized per-task or per-server. Was the intent to forward these from verifier_metadata? Either wiring them through or removing the unreachable keys would make the capability match the code. (A fuller sketch of the startup-config vs per-task-metadata boundary, which otherwise looks like the right design, is in the review summary.)
| - agent_only_runner: imports the configured agent's `responses()` inside the sandbox via | ||
| a small runner. The harness code executes unchanged, only its model base URL is patched | ||
| to a sandbox-reachable address. No Gym servers in the sandbox. | ||
| - gym_runner: starts full NeMo Gym inside the sandbox (`nested_config_paths`) and runs |
There was a problem hiding this comment.
responses_api_agents/sandbox_agent/README.md:10
Design question on the two modes (also sketched with diagrams in the review summary, "Design question #3"): both modes are really placement policy — where a rollout executes and at what granularity — encoded as agent config. In gym_runner mode the host rollout driver ends up calling an agent that boots a second ng_collect_rollouts inside the box, and the agent re-implements driver concerns (timeout budget, done-file polling, artifact download, concurrency). Would you see runtime placement as ultimately an upstream responsibility — environment config declares runtime: host | sandbox, the rollout driver/launcher enforces it, and the two modes collapse into a granularity knob — with this server as the v1 stepping stone toward that? Asking to understand the intended end-state rather than to block this PR.
builds on #2015 with new swe verifier