From cf0ac5d02dc7f02bab5a9857704e8ab4895cfe1c Mon Sep 17 00:00:00 2001 From: Sandy Chapman Date: Thu, 10 Sep 2026 09:39:41 -0300 Subject: [PATCH 1/2] fix(sandboxed-gym): frame rollout responses and honour an offline environment Two fixes landed on NeMo-RL's fork of this code while the branch that deletes that fork sat unmerged. Neither exists here, so completing that consolidation would drop both -- the exact drift consolidating is meant to end. Response framing. `/rollouts/run` emits a whitespace heartbeat while a batch runs, because the body can take minutes and the status line has to leave first. The body went out under HTTP/1.0 with neither a length nor chunking, delimited only by the connection closing -- and the OpenSandbox proxy does not wait for that close. It returned 200 with the lone " " heartbeat as the finished response, and the caller failed with `JSONDecodeError: Expecting value: line 1 column 1 (char 0)`. Chunked framing is the delimiter and it is 1.1-only, so the handler moves to 1.1 and takes only that half: `parse_request` declines connection reuse, which this server gains nothing from -- a rollout runs for minutes, so a handshake per request rounds to zero -- and which would cost a real hazard, because an unread request body (the 413 path declines to read one on purpose) is what the server would otherwise parse as the next request line. Every response is now explicitly framed through one of `_send_empty`, `_send_body`, `_send_json` or the chunked path. A 1.0 caller cannot parse chunks, so it gets the batch buffered behind a Content-Length instead: a late answer is a loud failure where a truncated one is a wrong reward. Offline environments. `nmp/rl`'s grpo_config already sets `environment_offline` on a manifest whose wheelhouse is a complete closure, and this package had no such field. `NemoGymSandboxedConfig` is `extra="forbid"`, so that key does not pass through unnoticed -- it fails validation, and a sandboxed run with an offline environment could not start once NeMo-RL consumes this package. The field now exists on both the caller dialect and the serve config, reaches the host as `NMP_ENVIRONMENT_OFFLINE`, and sets `UV_OFFLINE` alongside the wheelhouse's `UV_FIND_LINKS`. Index fallback is otherwise retained, which is a liability only when an index is configured but unreachable: uv then fails to resolve `uv venv --seed` rather than falling back to `--find-links`. Not derivable from the package format -- a wheels-v1 package can ship wheels and still need an index for its agent -- so the caller decides, and an operator's own `UV_OFFLINE` wins. Signed-off-by: Sandy Chapman --- .../src/sandboxed_gym/host/models.py | 1 + .../src/sandboxed_gym/orchestrator.py | 3 + .../sandboxed_gym/runtime/gym_host_runtime.py | 83 ++++-- .../src/sandboxed_gym/serve_config.py | 1 + .../tests/test_gym_host_runtime.py | 251 +++++++++++++++++- .../tests/test_sandboxed_gym_host.py | 66 +++++ 6 files changed, 383 insertions(+), 22 deletions(-) diff --git a/packages/sandboxed_gym/src/sandboxed_gym/host/models.py b/packages/sandboxed_gym/src/sandboxed_gym/host/models.py index b08f107809..6bf38dd1dd 100644 --- a/packages/sandboxed_gym/src/sandboxed_gym/host/models.py +++ b/packages/sandboxed_gym/src/sandboxed_gym/host/models.py @@ -223,6 +223,7 @@ class NemoGymSandboxedConfig(BaseModel): sandboxed: bool = False host_provider: str = "opensandbox" environment_path: str | None = None + environment_offline: bool = False sandbox: SandboxConfig | None = None job_id: str = DEFAULT_JOB_ID episode_broker: dict[str, Any] = Field(default_factory=dict) diff --git a/packages/sandboxed_gym/src/sandboxed_gym/orchestrator.py b/packages/sandboxed_gym/src/sandboxed_gym/orchestrator.py index ab465e1027..e41b1b8366 100644 --- a/packages/sandboxed_gym/src/sandboxed_gym/orchestrator.py +++ b/packages/sandboxed_gym/src/sandboxed_gym/orchestrator.py @@ -34,6 +34,7 @@ ) from sandboxed_gym.host.provider import SandboxedGymHostProvider, get_host_provider from sandboxed_gym.runtime.gym_host_runtime import ( + ENVIRONMENT_OFFLINE_ENV_KEY, ENVIRONMENT_PACKAGE_REQUIRED_ENV_KEY, GYM_GLOBAL_CONFIG_ENV_KEY, ) @@ -276,6 +277,8 @@ def build_gym_host_spec( # The mount path is ``/job/environment`` with or without a FileSet. This flag is how the # host distinguishes a required package from an image-bundled tree at the same path. bootstrap_extra[ENVIRONMENT_PACKAGE_REQUIRED_ENV_KEY] = "true" + if cfg.environment_offline: + bootstrap_extra[ENVIRONMENT_OFFLINE_ENV_KEY] = "true" bootstrap_env = build_bootstrap_env( cfg.job_id, cfg.environment_path or sandbox.env_mount_path, diff --git a/packages/sandboxed_gym/src/sandboxed_gym/runtime/gym_host_runtime.py b/packages/sandboxed_gym/src/sandboxed_gym/runtime/gym_host_runtime.py index dc2b3d4ce8..38bc78a2f1 100644 --- a/packages/sandboxed_gym/src/sandboxed_gym/runtime/gym_host_runtime.py +++ b/packages/sandboxed_gym/src/sandboxed_gym/runtime/gym_host_runtime.py @@ -40,6 +40,7 @@ #: for image-bundled Gym too; this flag is how a missing ``nemo-environment.yaml`` becomes a #: FileSet error instead of a silent fallback to the image-shipped environment. ENVIRONMENT_PACKAGE_REQUIRED_ENV_KEY = "NMP_ENVIRONMENT_PACKAGE_REQUIRED" +ENVIRONMENT_OFFLINE_ENV_KEY = "NMP_ENVIRONMENT_OFFLINE" UV_CACHE_DIR_KEY = "uv_cache_dir" UV_VENV_DIR_KEY = "uv_venv_dir" # Writable /job/work subdirectory where wheels are installed for the running Gym host. @@ -58,6 +59,7 @@ MODEL_CALLS_RESULT_KEY = "_nmp_model_calls" # uv setting that points Gym's per-server dependency resolver at the staged wheelhouse. UV_FIND_LINKS_ENV_KEY = "UV_FIND_LINKS" +UV_OFFLINE_ENV_KEY = "UV_OFFLINE" NEMO_GYM_EXTRA_ROOTS_ENV_KEY = "NEMO_GYM_EXTRA_ROOTS" #: Which agent, resources server, and model to run. Gym has no schema for this key, so #: the host pops it and rewrites ``config_paths`` before Gym parses the dict. @@ -220,6 +222,10 @@ def _environment_package_required() -> bool: return os.environ.get(ENVIRONMENT_PACKAGE_REQUIRED_ENV_KEY, "").strip().lower() in {"1", "true", "yes"} +def _environment_offline() -> bool: + return os.environ.get(ENVIRONMENT_OFFLINE_ENV_KEY, "").strip().lower() in {"1", "true", "yes"} + + def _load_runtime_environment_package( environment_path: str, *, @@ -287,6 +293,16 @@ def _install_wheels_v1_dependencies(package: EnvironmentPackage | None, work_pat # for image-owned Gym and its core dependencies. os.environ[UV_FIND_LINKS_ENV_KEY] = wheels_dir + if _environment_offline(): + if UV_OFFLINE_ENV_KEY in os.environ: + print( + f"gym-host: offline requested, but {UV_OFFLINE_ENV_KEY}=" + f"{os.environ[UV_OFFLINE_ENV_KEY]} is already set", + flush=True, + ) + else: + os.environ[UV_OFFLINE_ENV_KEY] = "1" + # Gym starts agent and resource servers as child Python processes. Prepend the wheel target so # those processes can import the environment's vendored dependencies. existing_pythonpath = os.environ.get("PYTHONPATH") @@ -606,33 +622,32 @@ def run_rollouts_sync( class Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + _chunked: bool = False max_request_bytes: int = 268_435_456 max_response_bytes: int = 268_435_456 heartbeat_interval_s: float = _HEARTBEAT_INTERVAL_S rollout_deadline_s: float = _DEFAULT_ROLLOUT_DEADLINE_S + def parse_request(self) -> bool: + parsed = super().parse_request() + self.close_connection = True + return parsed + def do_GET(self) -> None: if not self.path.startswith("/health"): - self.send_response(404) - self.end_headers() + self._send_empty(404) return # Must match do_POST: a host that passes /health and then 503s every rollout is # invisible to wait_ready. if not _READY or _HEAD_SERVER_CONFIG is None or _ROLLOUT_HELPER is None: - body = json.dumps({"status": "starting"}).encode("utf-8") - self.send_response(503) + self._send_json(503, {"status": "starting"}) else: - body = json.dumps({"status": "ready"}).encode("utf-8") - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) + self._send_json(200, {"status": "ready"}) def do_POST(self) -> None: if not self.path.startswith("/rollouts/run"): - self.send_response(404) - self.end_headers() + self._send_empty(404) return if not _READY or _HEAD_SERVER_CONFIG is None or _ROLLOUT_HELPER is None: self._send_json(503, _runtime_error("bootstrap_failed", "Gym host not ready")) @@ -690,14 +705,20 @@ def do_POST(self) -> None: # Committed to 200 before the work is done, so the first byte leaves immediately and no hop # can mistake a long batch for a dead one. Everything judgeable from the request alone was # rejected with a real status above; failures from here travel in the body as - # {"error": ...}, which the caller already treats as fatal. No Content-Length: the body is - # delimited by the close that `Connection: close` promises, which is what allows the - # heartbeats below to precede a payload of unknown length. + # {"error": ...}, which the caller already treats as fatal. + self._chunked = self.request_version >= "HTTP/1.1" + if not self._chunked: + self._send_body(200, self._await_results(future, started)) + return + self.send_response(200) self.send_header("Content-Type", "application/json") - self.send_header("Connection", "close") + self.send_header("Transfer-Encoding", "chunked") + self._announce_close() self.end_headers() - self.wfile.write(self._await_results(future, started)) + self._write_chunk(self._await_results(future, started)) + self.wfile.write(b"0\r\n\r\n") + self.wfile.flush() def _await_results(self, future: concurrent.futures.Future[list[dict]], started: float) -> bytes: """Wait for ``future``, heartbeating while it runs, and return the body to send. @@ -718,9 +739,10 @@ def _await_results(self, future: concurrent.futures.Future[list[dict]], started: # its own, which is not this loop's tick. done, _ = concurrent.futures.wait([future], timeout=min(self.heartbeat_interval_s, remaining)) if not done: + if not self._chunked: + continue try: - self.wfile.write(b" ") - self.wfile.flush() + self._write_chunk(b" ") except OSError as exc: # The caller is gone. Nothing will read this batch, so stop paying for it: # cancelling the future propagates to the collector task on the shared loop. @@ -765,14 +787,33 @@ def _await_results(self, future: concurrent.futures.Future[list[dict]], started: def _error_body(self, code: str, message: str) -> bytes: return json.dumps(_runtime_error(code, message)).encode("utf-8") - def _send_json(self, status: int, payload: dict[str, Any]) -> None: - body = json.dumps(payload).encode("utf-8") + def _write_chunk(self, data: bytes) -> None: + if not data: + return + self.wfile.write(b"%X\r\n%s\r\n" % (len(data), data)) + self.wfile.flush() + + def _announce_close(self) -> None: + if self.close_connection: + self.send_header("Connection", "close") + + def _send_empty(self, status: int) -> None: self.send_response(status) + self._announce_close() + self.send_header("Content-Length", "0") + self.end_headers() + + def _send_body(self, status: int, body: bytes) -> None: + self.send_response(status) + self._announce_close() self.send_header("Content-Type", "application/json") self.send_header("Content-Length", str(len(body))) self.end_headers() self.wfile.write(body) + def _send_json(self, status: int, payload: dict[str, Any]) -> None: + self._send_body(status, json.dumps(payload).encode("utf-8")) + def log_message(self, format: str, *args: Any) -> None: return diff --git a/packages/sandboxed_gym/src/sandboxed_gym/serve_config.py b/packages/sandboxed_gym/src/sandboxed_gym/serve_config.py index 9a8ada6b89..2fcb9d2d26 100644 --- a/packages/sandboxed_gym/src/sandboxed_gym/serve_config.py +++ b/packages/sandboxed_gym/src/sandboxed_gym/serve_config.py @@ -30,6 +30,7 @@ class SandboxedGymServeConfig(BaseModel): job_id: str = DEFAULT_JOB_ID host_provider: str = "opensandbox" environment_path: str | None = None + environment_offline: bool = False sandbox: SandboxConfig episode_broker: EpisodeBrokerConfig | dict[str, Any] = Field(default_factory=dict) gym_global_config: dict[str, Any] = Field(default_factory=dict) diff --git a/packages/sandboxed_gym/tests/test_gym_host_runtime.py b/packages/sandboxed_gym/tests/test_gym_host_runtime.py index f4366a4be8..faaf3e29b9 100644 --- a/packages/sandboxed_gym/tests/test_gym_host_runtime.py +++ b/packages/sandboxed_gym/tests/test_gym_host_runtime.py @@ -7,7 +7,7 @@ import threading import time from http.server import HTTPServer, ThreadingHTTPServer -from types import ModuleType +from types import ModuleType, SimpleNamespace from typing import Any, cast from unittest.mock import MagicMock from urllib.parse import urlsplit @@ -33,6 +33,8 @@ def ready_server(): runtime._ROLLOUT_HELPER = _FakeRolloutHelper() runtime.Handler.max_request_bytes = 1024 runtime.Handler.max_response_bytes = 4096 + runtime.Handler.heartbeat_interval_s = runtime._HEARTBEAT_INTERVAL_S + runtime.Handler.rollout_deadline_s = runtime._DEFAULT_ROLLOUT_DEADLINE_S server = HTTPServer(("127.0.0.1", 0), runtime.Handler) port = server.server_address[1] @@ -46,6 +48,8 @@ def ready_server(): runtime._READY = False runtime._HEAD_SERVER_CONFIG = None runtime._ROLLOUT_HELPER = None + runtime.Handler.heartbeat_interval_s = runtime._HEARTBEAT_INTERVAL_S + runtime.Handler.rollout_deadline_s = runtime._DEFAULT_ROLLOUT_DEADLINE_S def test_health_not_ready(): @@ -580,6 +584,44 @@ def test_wheels_v1_installs_every_wheel_with_no_index_access(tmp_path, monkeypat # Child processes and the already-running host both prefer staged packages over image packages. assert runtime.os.environ["PYTHONPATH"] == f"{install_dir}{runtime.os.pathsep}/image/packages" assert runtime.sys.path[0] == str(install_dir) + # Index fallback is left in place unless the caller declares the wheelhouse self-sufficient. + assert runtime.UV_OFFLINE_ENV_KEY not in runtime.os.environ + + +def _stage_wheelhouse(tmp_path, monkeypatch): + """A wheels-v1 package staged for install, with uv stubbed out.""" + _write_manifest(tmp_path, format="wheels-v1") + wheels_dir = tmp_path / WHEELS_V1_SUBDIR + wheels_dir.mkdir() + (wheels_dir / "a_dep-1.0-py3-none-any.whl").write_bytes(b"") + monkeypatch.setattr(runtime.subprocess, "run", lambda *a, **k: None) + return runtime._load_runtime_environment_package(str(tmp_path), required=True) + + +def test_an_offline_environment_takes_uv_off_the_index(tmp_path, monkeypatch, isolated_gym_host_process_state): + """A wheelhouse the caller calls complete must resolve without an index. + + `--find-links` alone is not enough: a configured but unreachable index makes uv fail to + resolve the `uv venv --seed` packages for Gym's per-component venvs rather than fall back + to the staged wheels. + """ + package = _stage_wheelhouse(tmp_path, monkeypatch) + monkeypatch.setenv(runtime.ENVIRONMENT_OFFLINE_ENV_KEY, "true") + + runtime._install_wheels_v1_dependencies(package, str(tmp_path / "work")) + + assert runtime.os.environ[runtime.UV_OFFLINE_ENV_KEY] == "1" + + +def test_an_operators_own_uv_offline_setting_is_not_overwritten(tmp_path, monkeypatch, isolated_gym_host_process_state): + """Whoever set it in the image knows something this flag does not.""" + package = _stage_wheelhouse(tmp_path, monkeypatch) + monkeypatch.setenv(runtime.ENVIRONMENT_OFFLINE_ENV_KEY, "true") + monkeypatch.setenv(runtime.UV_OFFLINE_ENV_KEY, "0") + + runtime._install_wheels_v1_dependencies(package, str(tmp_path / "work")) + + assert runtime.os.environ[runtime.UV_OFFLINE_ENV_KEY] == "0" def test_bootstrap_composes_a_wheels_package_like_native_v1(tmp_path, monkeypatch): @@ -1089,3 +1131,210 @@ def test_a_capture_that_is_not_valid_utf8_costs_the_timing_not_the_rollout(tmp_p (tmp_path / "0-1.capture.jsonl").write_bytes(b'{"model_call_id": "\xff\xfe"}\n') assert runtime._read_capture(str(tmp_path), {"_ng_task_index": 0, "_ng_rollout_index": 1}, budget=10_000) == ([], 0) + + +# ------------------------------------------------------------------------------------------ +# Response framing +# ------------------------------------------------------------------------------------------ + + +class _DelayedRolloutHelper: + """A helper whose rollouts take long enough for the heartbeat to fire. + + Distinct from `_SlowRolloutHelper` above, which blocks until a test releases it: these + tests read the wire to the end of the body, so the rollout has to finish on its own. + """ + + def __init__(self, delay_s: float) -> None: + self.delay_s = delay_s + + def run_examples(self, examples, head_server_config=None): + async def _one(row): + await asyncio.sleep(self.delay_s) + return row, {"response": {"output": []}, "reward": 0.0} + + return [_one(row) for row in examples] + + +def _body_is_complete(received: bytes) -> bool: + """Whether the framing in ``received`` marks the body as finished.""" + head, sep, body = received.partition(b"\r\n\r\n") + if not sep: + return False + if b"Transfer-Encoding: chunked" in head: + return body.endswith(b"0\r\n\r\n") + for line in head.split(b"\r\n"): + if line.lower().startswith(b"content-length:"): + return len(body) >= int(line.split(b":")[1]) + return False + + +def _raw_rollout_exchange(base_url: str, payload: str, version: str) -> bytes: + """POST /rollouts/run over a bare socket and return the response bytes as sent. + + urllib de-chunks transparently, which is the framing under test, so these assertions + have to read the wire. + """ + parsed = urlsplit(base_url) + body = payload.encode() + request = ( + f"POST /rollouts/run {version}\r\n" + f"Host: {parsed.hostname}\r\n" + "Content-Type: application/json\r\n" + f"Content-Length: {len(body)}\r\n" + "\r\n" + ).encode() + body + + with socket.create_connection((parsed.hostname, parsed.port), timeout=10) as sock: + sock.sendall(request) + sock.settimeout(10) + received = b"" + while not _body_is_complete(received): + part = sock.recv(4096) + if not part: + break + received += part + return received + + +def _one_example() -> str: + return json.dumps({"examples": [{"agent_ref": {"name": "a"}, "_rowidx": 0}]}) + + +def test_rollouts_run_frames_the_body_so_a_heartbeat_cannot_end_it(ready_server): + """The heartbeat and the payload must be separately framed chunks. + + Regression guard for nvbug 6716627. The host used to flush the status line with neither + a length nor chunking, leaving the body delimited only by connection close. The + OpenSandbox proxy does not wait for that close: it returned HTTP 200 with the lone " " + heartbeat -- or nothing at all -- as the finished response, and the caller failed with + `JSONDecodeError: Expecting value: line 1 column 1 (char 0)`. Only the zero-length chunk + may end this body. + """ + runtime.Handler.heartbeat_interval_s = 0.02 + runtime._ROLLOUT_HELPER = _DelayedRolloutHelper(0.3) + + received = _raw_rollout_exchange(ready_server, _one_example(), "HTTP/1.1") + + head, _, body = received.partition(b"\r\n\r\n") + assert b"Transfer-Encoding: chunked" in head + assert b"Content-Length" not in head + # The terminator, and nothing after it. + assert body.endswith(b"0\r\n\r\n") + # A heartbeat is its own chunk -- length-prefixed, so it cannot read as the end. + assert body.startswith(b"1\r\n \r\n") + + decoded = "" + rest = body + while True: + size_line, _, rest = rest.partition(b"\r\n") + size = int(size_line, 16) + if size == 0: + break + decoded += rest[:size].decode() + rest = rest[size + 2 :] + assert len(json.loads(decoded)["results"]) == 1 + + +def test_rollouts_run_sends_a_length_to_an_http_10_caller(ready_server): + """A 1.0 caller cannot parse chunks, so it gets the whole body with a length. + + Still explicitly framed -- that is the point. 1.0's own answer for a body of unknown + length is the close-delimited one that lost the response, so the host buffers instead and + pays for it in latency rather than in correctness. + """ + runtime.Handler.heartbeat_interval_s = 0.02 + runtime._ROLLOUT_HELPER = _DelayedRolloutHelper(0.1) + + received = _raw_rollout_exchange(ready_server, _one_example(), "HTTP/1.0") + + head, _, body = received.partition(b"\r\n\r\n") + assert b"Transfer-Encoding" not in head + assert b"Content-Length: " in head + # Buffered whole, so no heartbeat leaked into a body that is now length-delimited. + assert not body.startswith(b" ") + assert len(json.loads(body.decode())["results"]) == 1 + + +def test_connections_are_never_reused(ready_server): + """1.1 is here for chunked framing only; its connection reuse is declined. + + The sharp hazard: a request whose body the handler never reads (the 413 path declines on + purpose) leaves those bytes in the socket, and the server parses them as the next request + line. Pipelined here to prove it cannot happen -- before the connection was closed per + request, this exact exchange answered `400 Bad request syntax` and swallowed the real + /health. + """ + parsed = urlsplit(ready_server) + body = _one_example().encode() + pipelined = ( + ( + "POST /rollouts/run HTTP/1.1\r\n" + f"Host: {parsed.hostname}\r\n" + "Content-Type: application/json\r\n" + # Oversize by declaration: the 413 check reads Content-Length, not the body. + f"Content-Length: {runtime.Handler.max_request_bytes + 1}\r\n" + "\r\n" + ).encode() + + body + + f"GET /health HTTP/1.1\r\nHost: {parsed.hostname}\r\n\r\n".encode() + ) + + with socket.create_connection((parsed.hostname, parsed.port), timeout=10) as sock: + sock.settimeout(10) + sock.sendall(pipelined) + received = b"" + while True: + part = sock.recv(4096) + if not part: + break + received += part + + assert received.startswith(b"HTTP/1.1 413") + # One response, and the leftover body was never parsed as a request of its own. + assert received.count(b"HTTP/1.1") == 1 + assert b"Bad request" not in received + # Advertised, so a pooling proxy drops the socket instead of reusing it. + assert b"Connection: close" in received + + +def test_the_rollout_response_declines_reuse_too(ready_server): + """The zero-length chunk says where the body ends; Connection: close says the socket is + done. Neither substitutes for the other.""" + runtime.Handler.heartbeat_interval_s = 0.02 + runtime._ROLLOUT_HELPER = _DelayedRolloutHelper(0.1) + + received = _raw_rollout_exchange(ready_server, _one_example(), "HTTP/1.1") + + head, _, body = received.partition(b"\r\n\r\n") + assert b"Transfer-Encoding: chunked" in head + assert b"Connection: close" in head + assert body.endswith(b"0\r\n\r\n") + + +def test_bodiless_responses_carry_a_length(ready_server): + """HTTP/1.1 keep-alive reuses the socket, so even a 404 must say where it ends.""" + import urllib.error + import urllib.request + + with pytest.raises(urllib.error.HTTPError) as excinfo: + urllib.request.urlopen(f"{ready_server}/nope", timeout=10) + assert excinfo.value.code == 404 + assert excinfo.value.headers.get("Content-Length") == "0" + + +def test_an_empty_chunk_is_never_written(): + """A zero-length chunk is the terminator, so writing one for empty data ends the body early. + + Reached when a heartbeat or a result body is empty: the caller would see a well-formed but + truncated response rather than an error. + """ + written: list[bytes] = [] + handler = runtime.Handler.__new__(runtime.Handler) + handler.wfile = cast(Any, SimpleNamespace(write=written.append, flush=lambda: None)) + + handler._write_chunk(b"") + assert written == [] + + handler._write_chunk(b" ") + assert written == [b"1\r\n \r\n"] diff --git a/packages/sandboxed_gym/tests/test_sandboxed_gym_host.py b/packages/sandboxed_gym/tests/test_sandboxed_gym_host.py index 921c3a9b1f..31dfd869f9 100644 --- a/packages/sandboxed_gym/tests/test_sandboxed_gym_host.py +++ b/packages/sandboxed_gym/tests/test_sandboxed_gym_host.py @@ -480,3 +480,69 @@ def test_uv_env_passthrough_carries_no_credentials(monkeypatch): monkeypatch.setenv("NEMO_GYM_VENV_DIR", "/opt/gym_venvs") validate_bootstrap_env(uv_env_passthrough()) + + +def _offline_spec(*, environment_offline: bool): + from sandboxed_gym.config import BrokerEndpoint + from sandboxed_gym.orchestrator import build_gym_host_spec + from sandboxed_gym.serve_config import SandboxedGymServeConfig + + cfg = SandboxedGymServeConfig.model_validate( + { + "job_id": "job-1", + "environment_offline": environment_offline, + "sandbox": { + "image": "runtime:dev", + "network_policy": {"egress_allow": []}, + "environment_pvc_claim": "env", + "workspace_pvc_claim": "work", + }, + } + ) + broker = BrokerEndpoint(url="http://broker:1", host="broker", port=1, token="t") + return build_gym_host_spec(cfg, broker) + + +def test_gym_host_spec_forwards_an_offline_environment(): + """Only the caller who built the package knows the wheelhouse is a complete closure. + + The sandbox has no way to find out: a wheels-v1 package can ship wheels and still need an + index for its agent, so without this flag the host leaves uv's index fallback in place and a + configured-but-unreachable index fails the per-component venvs. + """ + from sandboxed_gym.runtime.gym_host_runtime import ENVIRONMENT_OFFLINE_ENV_KEY + + spec = _offline_spec(environment_offline=True) + + assert spec.bootstrap_env[ENVIRONMENT_OFFLINE_ENV_KEY] == "true" + + +def test_gym_host_spec_leaves_the_index_alone_by_default(): + from sandboxed_gym.runtime.gym_host_runtime import ENVIRONMENT_OFFLINE_ENV_KEY + + spec = _offline_spec(environment_offline=False) + + assert ENVIRONMENT_OFFLINE_ENV_KEY not in spec.bootstrap_env + + +def test_the_caller_dialect_accepts_an_offline_environment(): + """`env.nemo_gym` is `extra="forbid"`, so a key the platform emits and this model lacks is + not ignored -- it fails validation. nmp/rl's grpo_config sets `environment_offline` on an + offline manifest, so without the field a sandboxed run with one cannot start at all.""" + from sandboxed_gym.host.models import NemoGymSandboxedConfig + + cfg = NemoGymSandboxedConfig.model_validate( + { + "sandboxed": True, + "environment_path": "/job/environment", + "environment_offline": True, + "sandbox": { + "image": "runtime:dev", + "network_policy": {"egress_allow": []}, + "environment_pvc_claim": "env", + "workspace_pvc_claim": "work", + }, + } + ) + + assert cfg.environment_offline is True From 84797993aa4bd421abb97fb753d39cab0a7cdefb Mon Sep 17 00:00:00 2001 From: Sandy Chapman Date: Thu, 10 Sep 2026 10:52:04 -0300 Subject: [PATCH 2/2] fix(sandboxed-gym): drop the launcher's fallback to NeMo-RL's copy `gym_host.sh` fell back to `$root/nemo_rl/environments/sandbox/gym_host_runtime.py` when no runtime was given. That path is NeMo-RL's fork of this file, which is being deleted as RL moves onto this package, so the fallback would name a file that cannot exist. Unreachable through the package's own API -- `default_gym_host_entrypoint` always passes the runtime as argv[4] -- but reachable by anyone invoking the script with three arguments, and stale either way: `gym_host_runtime_path` in entrypoint.py already falls back to the `packages/` path alone, with no RL branch. The shell now matches it. Signed-off-by: Sandy Chapman --- packages/sandboxed_gym/src/sandboxed_gym/host/gym_host.sh | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/sandboxed_gym/src/sandboxed_gym/host/gym_host.sh b/packages/sandboxed_gym/src/sandboxed_gym/host/gym_host.sh index cbfde8afc8..eb404efcce 100755 --- a/packages/sandboxed_gym/src/sandboxed_gym/host/gym_host.sh +++ b/packages/sandboxed_gym/src/sandboxed_gym/host/gym_host.sh @@ -16,12 +16,7 @@ gym_rw=${3:-/tmp/gym-src/Gym} runtime=${4:-} if [ -z "$runtime" ]; then - # Prefer an explicitly packaged module path if present in PYTHONPATH layout. - if [ -f "$root/packages/sandboxed_gym/src/sandboxed_gym/runtime/gym_host_runtime.py" ]; then - runtime=$root/packages/sandboxed_gym/src/sandboxed_gym/runtime/gym_host_runtime.py - else - runtime=$root/nemo_rl/environments/sandbox/gym_host_runtime.py - fi + runtime=$root/packages/sandboxed_gym/src/sandboxed_gym/runtime/gym_host_runtime.py fi gym_tree=${SANDBOXED_GYM_TREE:-$root/3rdparty/Gym-workspace/Gym}