openenv/tbench2: per-task Daytona cloud-sandbox execution backend#1675
openenv/tbench2: per-task Daytona cloud-sandbox execution backend#1675nblintao wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for per-task Daytona cloud sandboxes as an alternative to a shared environment server in the OpenEnv Terminal-Bench-2 adapter. It includes documentation updates, new standalone evaluation and validation scripts (eval_tbench2_via_api.py and scan_golden.py), and offline unit tests. The reviewer's feedback focuses on improving code robustness, such as replacing assert with ValueError for argument validation, using isinstance checks instead of parsing exception strings, and explicitly checking for environment variables rather than relying on exception handling.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if args.router_external_host: | ||
| env["MILES_ROUTER_EXTERNAL_HOST"] = args.router_external_host | ||
| if args.openenv_tb2_tasks_dir: | ||
| assert args.daytona_api_key, "DAYTONA_API_KEY required in per-task Daytona mode" |
There was a problem hiding this comment.
According to the general rules, ValueError should be used instead of assert for validating function or constructor arguments. Please raise a ValueError if daytona_api_key is missing.
| assert args.daytona_api_key, "DAYTONA_API_KEY required in per-task Daytona mode" | |
| if not args.daytona_api_key: | |
| raise ValueError("DAYTONA_API_KEY required in per-task Daytona mode") |
References
- Use ValueError instead of assert for validating function or constructor arguments.
There was a problem hiding this comment.
Adopted in b604dc9 — switched to raise ValueError. Also makes this file consistent with the adapter, which already raises ValueError for the missing-task_id case.
| def _is_throttle_error(exc: BaseException) -> bool: | ||
| s = str(exc).lower() | ||
| return "throttler" in s or "too many requests" in s or "429" in s |
There was a problem hiding this comment.
When identifying or filtering specific exception types, prefer using isinstance checks rather than parsing the exception's string representation or checking for class names within the exception string. If ThrottlerException is a specific exception class, import and check for it directly, or check for an HTTP 429 status code attribute if applicable.
References
- When identifying or filtering specific exception types, prefer using isinstance checks rather than parsing the exception's string representation or checking for class names within the exception string.
There was a problem hiding this comment.
Deliberate, and now documented in the function docstring (b604dc9): the daytona SDK is a lazy, per-task-mode-only dependency — shared-server users don't install it — so an isinstance check on its exception classes isn't available at module scope. Rate limits also surface in several shapes across layers (SDK ThrottlerException, HTTP 429 text, wrapped errors), which one class check wouldn't cover. Same approach as _is_retryable_env_error above, which matches websockets close exceptions by name for the same no-hard-dependency reason.
There was a problem hiding this comment.
Update (1f9ed50): partially adopting after all — the module-scope constraint stands, but by the time a create has FAILED the daytona SDK has necessarily been imported, so a lazy in-function import can do a typed check. _is_throttle_error now checks isinstance(exc, DaytonaRateLimitError) first (the SDK normalizes HTTP 429 to it) and keeps the text match only as a fallback for older SDKs / text-only server messages. Covered by a new unit test asserting the typed path matches even when the message carries no throttle keywords, and that sibling error classes do not.
| try: | ||
| import tomllib | ||
|
|
||
| cfg = tomllib.loads( | ||
| (Path(os.environ["OPENENV_TB2_TASKS_DIR"]) / task_id / "task.toml").read_text() | ||
| ) | ||
| for k, v in (cfg.get("solution", {}).get("env", {}) or {}).items(): | ||
| sol_env += f" {k}={v!r}" | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Instead of relying on a caught KeyError when OPENENV_TB2_TASKS_DIR is missing from os.environ, explicitly check if the environment variable is set. This makes the code more intentional and robust.
tasks_dir = os.getenv("OPENENV_TB2_TASKS_DIR")
if tasks_dir:
try:
import tomllib
cfg = tomllib.loads(
(Path(tasks_dir) / task_id / "task.toml").read_text()
)
for k, v in (cfg.get("solution", {}).get("env", {}) or {}).items():
sol_env += f" {k}={v!r}"
except Exception:
passThere was a problem hiding this comment.
Adopted with a stronger fix in b604dc9: this tool only exists on the per-task backend at all, so main() now fails fast with a clear message when OPENENV_TB2_TASKS_DIR is unset. That makes golden_one's os.environ[...] read unconditionally valid, and the try/except is left tolerating only its real target — a task.toml without [solution].env.
6fd9669 to
1f9ed50
Compare
1f9ed50 to
21d451f
Compare
98bb8e6 to
5ce2c28
Compare
Add a second execution backend to the OpenEnv TB2 adapter: instead of a shared env server on a Docker host, every episode can run in its OWN Daytona cloud sandbox, built from the task's official image (task.toml docker_image) plus a tbench2_env server layer, and deleted when the episode ends. Same per-task image fidelity as docker mode with zero resident infrastructure (no Docker socket, no server to size or babysit) and zero cross-episode state leakage. Backend selection is episode-scoped in _multi_turn: set OPENENV_TB2_TASKS_DIR to a terminal-bench-2 checkout and each episode declaratively builds its sandbox from the task's Image definition (layer-cached by definition hash: first episode of a task ~10 min, repeats ~1 min; no named snapshots, so no org snapshot quota; sandboxes carry an openenv-tbench2-task=<task_id> label for safe sweeping in a shared org). Unset -> the existing OPENENV_ENV_URL shared-server path, behaviorally unchanged (the shared leg now reads OPENENV_ENV_URL at its own use site instead of threading it through _multi_turn's signature). The two backends deliberately score differently, each matching what its server provides: the shared-server leg keeps the adapter-driven canonical exec + reward-marker parse (compensates for an UNMODIFIED upstream server); the per-task leg uses the standard evaluate action, because the sandbox recipe (tbench2_env.task_snapshots -- tbench2_env is OpenEnv's Terminal-Bench-2 env package, envs/tbench2_env in huggingface/openenv; fixes proposed upstream in openenv#965 + #966) bakes a patched server that runs the same canonical tests/test.sh natively and resolves the task WORKDIR server-side (so no _apply_workdir prefix on this leg either -- task WORKDIRs are not uniformly /app: fix-git, prove-plus-comm). Sandbox creation is throttled process-wide with jittered backoff (Daytona rate-limits creates). Ships with the two tools that produced the validation evidence, both driving the adapter's own code paths so what they check is what training runs: scan_golden.py -- infra baseline without any LLM: replay each task's OFFICIAL solution/solve.sh (oracle-faithful: staged at /solution, DEBIAN_FRONTEND=noninteractive, task.toml [solution].env exported, task workdir cwd) through the same sandbox + evaluate scoring, expecting 1.0; --logs captures solve.log/test-log tails below 1.0. eval_tbench2_via_api.py -- the EXACT training agent-env loop (_multi_turn) with any OpenAI-compatible API standing in for the policy: no GPU, no Ray/Megatron. For end-to-end smokes and for measuring base-model solve rates when picking a variance-band training subset. plus offline unit tests (tests/, run manually: pytest examples/experimental/openenv/tests/ -q -- not collected by the repo-level suite) for the two things a live episode cannot cheaply prove: backend dispatch on both legs, and the create-throttling retry/backoff/give-up behavior that only triggers under production rate limits. Validation: - per-task leg, no LLM (scan_golden.py): golden sweep passes 82/89 of the TB2 suite (0 infra errors; the 7 remaining have upstream-broken solutions); golden regression on this exact merge (fix-git, chess-best-move, circuit-fibsqrt all 1.0 -- covering per-task WORKDIR dispatch, apt paths, long canonical evals). - per-task leg, end-to-end with a live API policy (eval_tbench2_via_api.py): full 89-task sweep with DeepSeek as the policy (single sample per task, 30-turn cap, up to 38 concurrent episodes) -- 33/89 solved, 0 systematic infra failures (10 non-scoring episodes: 8 heavyweight-task timeouts = reward 0 under training semantics, 1 transient registry network error, 1 sandbox-resource overrun). - shared-server leg: dispatch unit test asserts behavior identical to before this change (exec prefixed with cd /app, canonical-exec scoring, rm-hack present). - unit tests: 7 passed (dispatch both legs; throttle retry/give-up/passthrough; throttle-error classification incl. the typed DaytonaRateLimitError path). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5ce2c28 to
897118c
Compare
The daytona SDK is imported lazily by tbench2_env's task_snapshots (so docker-mode installs don't need it) and is not pulled in by `pip install -e tbench2_env`. On a fresh machine the per-task Daytona mode therefore fails at the WORST possible layer: every episode's sandbox start raises ModuleNotFoundError, the sample is aborted, the group is dropped by check_no_aborted, and the rollout loop refills forever -- a silent GPU-burning churn instead of an error (observed: 1900 identical failures before intervention). Fail fast instead: apply_optional_env_vars now imports daytona when OPENENV_TB2_TASKS_DIR requests the per-task backend and raises a RuntimeError with the install command at launch time. Also add the `pip install daytona` prerequisite to the README's per-task section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stacked on #1487 (base =
shi/openenv-miles-adapter, parallel to #1662). Pure additive change: the existing shared-server path is behaviorally unchanged (asserted by a dispatch unit test), and every #1487 mechanism —_apply_workdir, canonical exec + reward-marker scoring, rm-hack,policy.close()— is preserved as-is.What
A second execution backend for the OpenEnv TB2 adapter: set
OPENENV_TB2_TASKS_DIRto a terminal-bench-2 checkout and every episode runs in its own Daytona cloud sandbox, built from the task's official image (task.tomldocker_image) plus a tbench2_env server layer, deleted when the episode ends.Same per-task image fidelity as docker mode, with zero resident infrastructure: no Docker socket, no shared env server to size or babysit (no
MAX_CONCURRENT_ENVS/ulimittuning), no cross-episode state. Cost profile: first episode of a task builds its image (~10 min, layer-cached by definition hash), repeats start in ~1 min; no named snapshots, so no org snapshot quota. Sandboxes carry anopenenv-tbench2-task=<task_id>label for safe sweeping in a shared org, and creation is throttled process-wide with jittered backoff (Daytona rate-limits creates).Why the two backends score differently (on purpose)
Each leg matches what its env server actually provides:
evaluateaction. The sandbox recipe lives upstream-side intbench2_env.task_snapshots(tbench2_env = OpenEnv's Terminal-Bench-2 env package; fixes proposed in fix(tbench2_env): canonical test.sh scoring, real timeouts, task-image workdir huggingface/OpenEnv#965 + feat(tbench2_env): per-task sandbox image recipe + Daytona materialization huggingface/OpenEnv#966) and bakes a patched server that runs the same canonicaltests/test.shnatively and resolves the task WORKDIR server-side. No_apply_workdirprefix on this leg either — task WORKDIRs are not uniformly/app(fix-git=/app/personal-site,prove-plus-comm=/workspace), which the hardcoded prefix silently breaks.Once #965/#966 land upstream, a follow-up can retire the adapter-driven machinery and unify both legs on
evaluate.Tooling (ships with the backend; produced the validation evidence)
scan_golden.py— infra baseline with no LLM: replays each task's OFFICIALsolution/solve.sh(oracle-faithful: staged at/solution,DEBIAN_FRONTEND=noninteractive,[solution].envexported, task-workdir cwd) through the same sandbox +evaluatescoring;--logscaptures solve/test log tails for anything below 1.0.eval_tbench2_via_api.py— the EXACT training agent-env loop (_multi_turn) with any OpenAI-compatible API as the policy: no GPU, no Ray/Megatron. For end-to-end smokes and for measuring base-model solve rates when picking a variance-band training subset.tests/— offline unit tests (pytest examples/experimental/openenv/tests/ -q, not collected by the repo-level suite): backend dispatch on both legs, and the create-throttling retry/backoff/give-up behavior that only triggers under production rate limits.Validation
fix-git,chess-best-move,circuit-fibsqrtall 1.0 — covering per-task WORKDIR dispatch, apt paths, and long canonical evals.cd /app, canonical-exec scoring, rm-hack present,evaluatenever used).DaytonaRateLimitErrorpath).🤖 Generated with Claude Code