fix(sandbox): report OpenSandbox failures through shared contracts

environment_servers/harbor/sandboxes.py

@@ -36,6 +36,7 @@
 from nemo_gym.agents import sandbox_server_url
 from nemo_gym.client import apply_rollout_prefix
 from nemo_gym.config import get_global_config_dict
+from nemo_gym.core import FailureBoundary, NgFailureEvent, observe_failure
 from nemo_gym.data.tasks.harbor import (
     MAIN_SERVICE,
     RECORD_VERSION,
@@ -133,6 +134,9 @@ def _is_image_pull_failure(error: BaseException) -> bool:
     Walks the cause/context chain and the members of exception groups: a Compose group whose cleanup
     after a failed pull failed too surfaces as an ``ExceptionGroup`` of the cleanup errors, raised while
     the pull failure was being handled.
+
+    No image-pull failure kind is registered yet. This text heuristic preserves bounded retries of
+    fresh sandbox creation; it must not be used to replay a submitted command.
     """
     seen: set[int] = set()
     pending: list[BaseException] = [error]
@@ -848,10 +852,13 @@ async def _start_with_pull_retries(
                     async with self._pull_retry_gate:
                         await start(sandbox)
                 return sandbox
-            except (SandboxCreateError, RuntimeError, OSError, ExceptionGroup) as exc:
+            except (NgFailureEvent, SandboxCreateError, RuntimeError, OSError, ExceptionGroup) as exc:
                 # A Compose group whose cleanup failed too raises an ExceptionGroup over the pull failure.
+                if isinstance(exc, NgFailureEvent) and exc.boundary != FailureBoundary.SANDBOX:
+                    raise
                 if attempt >= attempts or not _is_image_pull_failure(exc):
                     raise
+                observe_failure(exc)
                 LOGGER.warning(
                     f"{task.task_id}: image pull failed starting {what} (attempt {attempt}/{attempts}): {exc}"
                 )

environment_servers/harbor/tests/test_app.py

@@ -72,7 +72,7 @@
 from nemo_gym.agents import AGENT_OUTCOME_METADATA_KEY, AgentOutcomePolicyConfig, OutcomeAction
 from nemo_gym.client import ServerClient
 from nemo_gym.config import AggregateMetricsRequest
-from nemo_gym.core import failure_kinds
+from nemo_gym.core import NgFailureEvent, RequestFailureLog, failure_kinds
 from nemo_gym.data.tasks.harbor import (
     BUILT_IMAGES_FILE,
     DIGEST_KEY,
@@ -4579,7 +4579,8 @@ def __init__(self, provider_config):
             async def start(self, spec):
                 starts.append(spec.image)
                 if len(starts) <= len(failures):
-                    raise SandboxCreateError(failures[len(starts) - 1])
+                    failure = failures[len(starts) - 1]
+                    raise failure if isinstance(failure, BaseException) else SandboxCreateError(failure)
 
             def __getattr__(self, name):
                 return getattr(sandbox, name)
@@ -4602,6 +4603,30 @@ async def test_exhausted_pull_retries_are_a_retryable_seed_failure(self, tmp_pat
         assert "ErrImagePull" in response.failure.failure_reason and len(starts) == 3
         assert not server._sandboxes
 
+    @pytest.mark.parametrize("message, failures", [("ErrImagePull", 1), ("ErrImagePull", 3), ("quota exceeded", 1)])
+    async def test_shared_sandbox_failure_preserves_pull_retry_policy(self, tmp_path, monkeypatch, message, failures):
+        cause = RuntimeError(message)
+        error = NgFailureEvent(
+            boundary="sandbox",
+            failure_kind="provider_request_failed",
+            operation="create",
+            failure_reason="Sandbox create failed",
+        )
+        error.__cause__ = cause
+        server, task, sandbox, starts = self.retrying_server(tmp_path, monkeypatch, [error] * failures)
+        with RequestFailureLog.capture() as log:
+            response = await run_episode(server, task)
+        if message == "ErrImagePull" and failures == 1:
+            assert response.failure is None, response.failure
+            assert response.result.reward == 1.0
+            assert starts == ["ubuntu:24.04"] * 2
+            assert log.report.events[error.event_id].foreign_error.exception_type == "RuntimeError"
+        else:
+            assert response.failure is not None
+            assert len(starts) == failures
+        assert error.__cause__ is cause
+        assert not server._sandboxes
+
     async def test_seed_time_is_not_charged_against_the_task_deadline(self, tmp_path, monkeypatch):
         """A start that recovered on a pull retry begins the task with its whole budget (review fix).
 

fern/versions/latest/pages/infrastructure/sandbox/adding-a-provider.mdx

@@ -61,14 +61,27 @@ class MySandboxProvider:
         return None
 ```
 
-Provider implementations should preserve the same lifecycle contract as the built-in providers:
+New provider implementations must follow this lifecycle and failure contract:
 
 - Return a `SandboxHandle` from `create()` only after the sandbox is ready enough to run commands and transfer files.
 - Return command status through `SandboxExecResult` for process exits, including nonzero exits.
-- Raise `SandboxCreateError` or `SandboxCreateVerificationError` for sandbox allocation and readiness failures.
+- For allocation, readiness and runtime failures, raise `NgFailureEvent` with `boundary="sandbox"`, the operation
+  and a kind registered in [failure_kinds.py](https://github.com/NVIDIA-NeMo/Gym/blob/main/nemo_gym/core/failure_kinds.py).
+  Each provider classifies its own SDK errors. The shared `nemo_gym.sandbox.failures.sandbox_failure` factory
+  builds the event from an explicit `failure_kind`, observed `http_status` and sandbox details; it does not
+  interpret SDK exceptions or choose policy actions. Use `raise ... from error` to retain the original cause.
+- A provider-request timeout leaves remote completion unknown. Report `provider_io_timeout`; do not invent a command timeout.
+- Report `provider_oom_killed` only from authoritative current-sandbox evidence, retaining its source.
+- Choose runtime actions through configurable `OutcomePolicyConfig` rules. Bound retries by the operation's
+  existing budget and never replay a command whose delivery is uncertain. Record failed attempts with
+  `observe_failure` before retrying. It records the attempt in the current request's failure report without
+  changing the result; see [Failure observations](/observability/rollout-failures#recovered-failures).
 - Make `close()` safe to call from cleanup paths.
 - Use `aclose()` for provider-scoped resources such as SDK clients.
 
+OpenSandbox implements this contract. See [Required provider migrations](./index#required-provider-migrations)
+for the built-in providers that must adopt it in follow-up changes.
+
 ## Provider Config
 
 Provider constructors receive the single provider-specific mapping from a named sandbox block:

fern/versions/latest/pages/infrastructure/sandbox/index.mdx

@@ -88,8 +88,9 @@ The public API is part of `nemo-gym`. Runtime backends can have optional depende
 | `SandboxResources` | Typed resource request with CPU, memory, disk, and GPU fields. |
 | `SandboxExecResult` | Command result with `stdout`, `stderr`, `return_code`, and optional `error_type`. |
 | `SandboxStatus` | Provider-neutral lifecycle status: `starting`, `running`, `stopped`, `error`, or `unknown`. |
-| `SandboxCreateError` | Base create-time failure for provider allocation and readiness errors. |
-| `SandboxCreateVerificationError` | Create-time failure raised when a new sandbox cannot pass provider readiness checks. |
+| `SandboxCreateError` | Creation exception retained for [providers awaiting migration](#required-provider-migrations). |
+| `SandboxCreateVerificationError` | Readiness-check exception retained for providers awaiting migration. |
+| `SandboxPtyError` | PTY-session exception retained for providers awaiting migration. |
 
 ## First-Run Example
 
@@ -332,20 +333,58 @@ Rewrites are ordered. The first matching `from` prefix wins.
 
 ## Error Handling
 
-Sandbox create failures use provider-neutral exception classes:
+Command process exits, including nonzero exits, return `SandboxExecResult`. For allocation, readiness,
+lifecycle and runtime failures, new and migrated providers must raise `NgFailureEvent` with `boundary="sandbox"`.
+Its registered `failure_kind`, `operation`, observed HTTP status, typed `details` and explicit cause
+carry the evidence; callers apply their own outcome policy. A failure kind alone does not decide
+attribution, retries, health or a zero score.
 
-- `SandboxCreateError` for sandbox allocation or readiness failures.
-- `SandboxCreateVerificationError` when a created sandbox fails Gym readiness verification.
+The registered kinds and their meanings live in
+[failure_kinds.py](https://github.com/NVIDIA-NeMo/Gym/blob/main/nemo_gym/core/failure_kinds.py).
+The `provider_*` namespace covers sandboxes, browsers and containers; `boundary="sandbox"` identifies
+the API boundary for sandbox events.
 
-In resources servers and agents, catch these errors close to the sandbox operation and return a meaningful verifier or rollout error. Do not let one bad sandbox allocation crash a long-running server.
+A command exit code of 137, a signal kill near a deadline, an error message, or a sidecar's OOM does not
+establish the command's cause. Forward `NgFailureEvent` unchanged when only forwarding; when reporting
+a distinct failure at a caller's boundary, create its event and use `raise ... from error`.
 
-```python
-from nemo_gym.sandbox import AsyncSandbox, SandboxCreateError
+Local argument/configuration validation and task cancellation are separate from operational failure events.
+Do not translate cancellation into a sandbox failure.
+PTY `read(timeout_s=...)` and `wait_exit(timeout_s=...)` raise `TimeoutError` when the caller's bounded wait expires:
+that timeout alone does not mean the PTY or command failed.
 
+Resources servers and agents forward failure events to the owning task boundary for its outcome policy,
+preserving their causes. An infrastructure failure alone does not justify a zero reward.
+See [Failure observations](/observability/rollout-failures) for recording, transport and saved reports,
+and [policy by execution stage](/observability/rollout-failures#policy-by-execution-stage) for shared OOM defaults.
 
-sandbox = AsyncSandbox(provider_config, spec)
-try:
-    await sandbox.start()
-except SandboxCreateError as error:
-    return {"reward": 0.0, "error": f"sandbox_create_failed: {error}"}
-```
+### Required provider migrations
+
+OpenSandbox implements the failure-event contract above. The following built-in providers **must migrate**
+in follow-up changes:
+
+| Provider | Config key |
+| --- | --- |
+| Apptainer | `apptainer` |
+| Daytona | `daytona` |
+| Docker | `docker` |
+| E2B | `e2b` |
+| ECS Fargate | `ecs_fargate` |
+| Enroot | `enroot` |
+| Local | `local` |
+| OpenShell | `openshell` |
+
+For each migration:
+
+- Cover allocation, readiness, lifecycle, command and file failures, plus PTY failures where supported.
+  Include operational failures currently returned as error-valued results; keep ordinary command exits as `SandboxExecResult`.
+- Classify provider evidence into registered kinds and preserve explicit causes.
+- Record recovered attempts and use configurable outcome policies within existing retry budgets and replay constraints.
+- Update callers and tests together, retaining their retry and grading behavior.
+
+A provider's migration is complete when all its supported operational failure paths and their callers use the shared contract.
+
+Until a provider migrates, callers must retain its existing exception handling. `SandboxCreateError`,
+`SandboxCreateVerificationError` and `SandboxPtyError` remain compatibility classes for those paths;
+they are not an alternative contract for new providers. Remove them only after their remaining providers
+and callers have migrated. The [provider guide](./adding-a-provider#provider-contract) describes the required implementation.

fern/versions/latest/pages/infrastructure/sandbox/opensandbox-best-practices.mdx

@@ -157,16 +157,18 @@ timeouts do not help a full cluster.
 
 ## Retries: creates yes, commands no
 
-`create.retries` is safe: a failed create leaves nothing behind. `operations.retries` (shipped:
-`5`, exponential backoff from `retry_delay_s` to `retry_max_delay_s`) covers the SDK calls after
-create — status and endpoint lookups, file transfer, close — plus a command submit that failed at
-TCP connect before the body reached the exec daemon. All of those are idempotent or provably never
-started, so retrying them is safe. `operations.command_retries`
-defaults to `0` and should stay there for agent commands, because a proxy `502` on
-`POST /command` does not mean the command never started — the server maps every transport error
-to 502, including a dropped read *after* the body reached the exec daemon, which runs commands on
-a context a dropped connection never cancels. A retried `git apply` runs twice. Raise it only for
-provably idempotent workloads.
+`create.retries` bounds allocation attempts; Gym cleans up known failed sandboxes and attempts to reap
+orphans when creation times out. `create.outcome_policy` selects whether another attempt is allowed.
+
+`operations.retries` bounds retries of eligible runtime requests, such as status, endpoint and log reads.
+`operations.outcome_policy` selects `retry` or `fail`; the provider also checks replay safety. A retry rule
+does not establish that a mutation or command submission is safe to repeat.
+
+Command submissions are single-attempt. `operations.command_retries` remains accepted for config compatibility,
+but increasing it does not enable replay. A TCP failure or proxy `502` is insufficient proof that the command
+never started: a dropped response can leave the command running. A retried `git apply` could run twice.
+After a successful background submission, Gym retries status reads for the same execution ID within the
+existing recovery and client-wait deadlines; it does not submit another command.
 
 Keep `create.skip_health_check: false`. Skipping it lets your first command race a pod whose exec
 daemon is not listening yet, and the resulting 502 kills the rollout.
@@ -243,8 +245,8 @@ Things to know:
 | `DELETE` → `404` / "Failed to terminate sandbox" | Already gone (TTL or earlier stop). | Nothing; the provider treats it as success. |
 | Status poll → `404 unknown execution` on a live sandbox | Stale sandbox→IP routing during create/delete storms. | Treat as a lost execution and recreate; do not tear down other sandboxes. |
 | `Server disconnected` / `Connection reset by peer` on a long stream | Load-balancer idle timeout or a server worker restart. | `background_exec: true`; keep `keepalive_expiry_s` below the server's idle timeout. |
-| `SandboxPtyError: PTY session already has an attached client` on every retry | The sandbox's backend died while the proxy still records the old attachment. | Report the death. Environments must catch `SandboxPtyError` in `verify()`; one unhandled 500 on `/run` can abort the whole eval. |
-| Whole sandbox dies with `137` after one heavy command | Group OOM kill (fan-out above). | Cap parallelism, bash timeout; more memory alone rarely fixes it. See [detecting OOM from the client](/infrastructure/sandbox/opensandbox-debugging#detecting-oom-from-the-client). |
+| `provider_pty_busy` / `PTY session already has an attached client` | Attachment was refused; this alone does not establish that the backend died. | Forward the sandbox `NgFailureEvent` to the owning task's outcome policy. Check the existing attachment and sandbox status before retrying. |
+| Whole sandbox dies after one heavy command, possibly with exit `137` | OOM is one possible cause; exit 137 alone does not confirm it. | Check [current-sandbox OOM evidence](/infrastructure/sandbox/opensandbox-debugging#detecting-oom-from-the-client) before classifying the failure. |
 
 Sandboxes run with the default capability set — no `SYS_PTRACE`, no `SYS_ADMIN`. `strace`/`gdb`
 attach and mount namespaces fail for that reason, not because of your config.