fix(tbench2_env): bring Docker mode up to the canonical scoring contract - #1012
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| _, output = self._exec_in_container(_fallback_eval_cmd(workdir)) | ||
| exit_code = _parse_exit_code_marker(output) | ||
| reward = 1.0 if exit_code == 0 else 0.0 | ||
| info = {"tests_passed": exit_code == 0, "exit_code": exit_code} |
There was a problem hiding this comment.
Docker fallback scoring lacks timeout
Medium Severity
Docker mode reads verifier_timeout_s for every evaluate, but only passes it into _canonical_eval_cmd. The pytest fallback via _fallback_eval_cmd has no in-shell timeout, and Docker exec has no server-side budget either, so a hung custom-task pytest can block the session indefinitely. Local mode still bounds that path through the terminal toolkit.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 31d0fa6. Configure here.
1a6f0c7 to
3542f41
Compare
Docker mode (Tbench2DockerEnvironment) was left behind when huggingface#965/huggingface#972 landed the canonical contract in local mode: its evaluate still ran bare `pytest tests/` from /task (the task-source copy), skipping the task's own tests/test.sh (pinned toolchain, reward.txt verdict) and the image's real WORKDIR — the same fidelity gaps huggingface#965 fixed for local mode. - evaluate now stages tests/ at the official fixed path (/tests), runs canonical tests/test.sh from the agent's workdir bounded by the task's verifier budget, and reads /logs/verifier/reward.txt; task dirs without test.sh fall back to pytest. Both fixed paths are wiped with the verify window. The command construction and marker parsing are factored into module helpers shared with local mode. - agent commands run in the task image's own WORKDIR, resolved from image metadata (Config.WorkingDir, which sees base-image WORKDIRs) with the Dockerfile parse as fallback. - exec commands ride as a bash -c argv element instead of being spliced into a single-quoted shell string, which mangled any agent command containing a single quote. - a task dir that declares no [environment] docker_image is rejected at reset. The old silent fallback ran agent commands directly on the env-server host: a containment hole and a scoring-fidelity hole at once. The containerless code paths (host subprocess exec, _evaluate_local) are gone with it. - TB2_MODE=auto is removed: it claimed to auto-detect Docker but always ran local mode; unknown modes now fail at startup instead of silently serving local. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3542f41 to
7bef0ba
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4180b7c. Configure here.
| "in task.toml; Docker mode runs every episode in the task's " | ||
| "official container and does not fall back to executing on the " | ||
| "server host. Fix the task, or use TB2_MODE=local." | ||
| ) |
There was a problem hiding this comment.
Rejected reset leaves stale state
Low Severity
Imageless reset calls close() then raises, but close() clears the container and _task_dir without resetting _state or _task_image. After a rejected reset, /state can still report the previous episode as ready even though step will fail because no task is initialized.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4180b7c. Configure here.
burtenshaw
left a comment
There was a problem hiding this comment.
LGTM. I handled the left over container issue.
…pter compensation With huggingface/OpenEnv#1012 bringing docker mode up to the canonical scoring contract (#965/#972 had it in local mode only), both legs speak one protocol: raw exec commands (the server resolves the task image's WORKDIR) and the standard `evaluate` action (canonical tests/test.sh, server-side). - delete the adapter-side compensation machinery the shared leg carried for older deployments: _apply_workdir, _CANONICAL_EVAL_CMD, the reward / test.sh-rc markers and their parsers, the OPENENV_TASK_WORKDIR / OPENENV_TB2_TESTS_SRC knobs, and the testsh_rc metrics plumbing. - the native_evaluate episode-wiring parameter goes with it: legs now differ only in run_body (how an env comes into being) and post_episode (the shared server keeps its trial-dir purge; a sandbox needs none). - runtime contract guard, replacing the source preflight that is impossible against a remote server: evaluate must carry the canonical harness marker (info.harness == "tests/test.sh"). An older deployment's bare-pytest reward looks valid but is untrustworthy -- it is dropped with a warning, so an out-of-date server surfaces as every sample dropping, never as a plausible reward curve. Composes with the existing error/None-reward drop. Behavior change: episodes whose task dir ships no tests/test.sh (scored by the server's pytest fallback, harness marker absent) are now dropped too -- all 89 official TB2 tasks and the synth pools ship test.sh. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


Summary
#965 / #972 landed the canonical Terminal-Bench-2 contract in local mode (
Tbench2Environment): scoring via the task's owntests/test.sh(pinned toolchain,/logs/verifier/reward.txtverdict), the task image's realWORKDIR, and verifier assets kept out of the agent's reach. Docker mode (Tbench2DockerEnvironment) only received the verifier-asset withholding half of #972 — itsevaluatestill ran barepytest tests/from/task(the task-source copy), skipping the canonical harness and the image's realWORKDIR. This PR closes that gap so both execution modes speak one scoring contract, and removes two silent-degradation paths found along the way.Changes
evaluatein Docker mode: stagetests/at the official fixed path (/tests) for exactly the verify window (pre-wipe fail-closed, post-wipe of both/testsand/logs/verifier, as before), runtests/test.shfrom the agent's workdir bounded by the task's own verifier budget (task.toml [verifier].timeout_sec, via coreutilstimeoutwhen present in the image), read the verdict from/logs/verifier/reward.txt. Task dirs withouttest.shfall back to pytest, mirroring local mode's_evaluate_fallback. Command construction and marker parsing are factored into module helpers shared with local mode.execcommands and scoring run in the task image's ownWORKDIR, resolved from image metadata (Config.WorkingDir, which sees base-image WORKDIRs) with the task Dockerfile parse as fallback, then/task.bash -cargv element instead of being spliced into a single-quoted shell string, which mangled any agent command containing a single quote.[environment] docker_imageis rejected atreset. The old silent fallback ran agent commands directly on the env-server host — a containment hole (arbitrary agent shell outside any container) and a scoring-fidelity hole at once. The containerless code paths (host subprocess exec, docker-class_evaluate_local) are removed with it.TB2_MODE=autoremoved: it claimed to auto-detect Docker but unconditionally served local mode; unknown modes now fail at startup instead of silently serving local.Who is affected?
Local mode — the default, and what HF Spaces run — is completely unchanged. The refactor generates byte-identical commands there, and the existing local-mode tests pass without modification. If you never set
TB2_MODE=docker, you can stop reading.For
TB2_MODE=dockeroperators, the intended effect is simply: official TB2 tasks now score the official way (previously: barepytestin the wrong directory, without the task's pinned toolchain — i.e. the old rewards were wrong). Scores will move; that's the fix working.The only setups that need a change on their side:
/task, tests at/task/tests) and whose image declares aWORKDIR. Images without aWORKDIRkeep/taskas cwd, so most custom setups are untouched.docker_imagein task.toml: these used to silently run agent commands directly on the server host — outside any container. That's a containment hole, soresetnow rejects them with an error pointing atTB2_MODE=local. (If someone has a real use case for the old behavior, happy to gate it behind an explicit opt-in env var instead.)TB2_MODE=auto: never actually auto-detected anything (it silently ran local mode), so it now errors at startup; setlocalexplicitly to keep the exact same behavior.Nothing here can fail silently: every changed path either scores the official way or refuses loudly with an actionable message. One cosmetic note for downstream parsers: docker-mode
evaluatenow reportsinfo["harness"]on the canonical path (same shape as local mode) instead ofinfo["exit_code"]; the pytest-fallback path keepsexit_code.Validation
tests/envs/test_tbench2_env.py): 23 passed. Existing docker-mode tests updated to the new contract; new coverage for the pytest fallback withouttest.sh, workdir resolution precedence (image metadata → Dockerfile →/task), imageless reset rejection, and argv-form exec (single quotes survive byte-identical).TB2_MODE=dockerserver from this branch with a plain OpenEnv client (reset → stage the task's officialsolution/solve.sh→ exec it → standardevaluate):fix-git: reward 1.0,info.harness=tests/test.sh, and the in-containerpwdprobe returned/app/personal-site— the task's non-trivial image WORKDIR, resolved correctly (the old code would have run everything in/task).chess-best-move: reward 1.0.evaluate): reward 0.0 — the canonical harness genuinely ran and failed the tests.resetraisedRuntimeError: task imageless-task declares no [environment] docker_image …as intended.🤖 Generated with Claude Code
Note
Medium Risk
Docker-mode rewards and cwd change for official tasks (intended fix); breaking for imageless tasks, TB2_MODE=auto, and any setup that relied on host fallback or old pytest-in-/task scoring.
Overview
Docker mode now matches local mode for scoring and isolation:
evaluatestages tests at/tests, runs the task'stests/test.sh(verifier timeout viatimeoutwhen available), and reads/logs/verifier/reward.txtinstead of barepytestunder/task. Agentexecand scoring use the image's resolvedWORKDIR(image config → Dockerfile →/task).Shared helpers (
_canonical_eval_cmd, reward/exit-code parsing) dedupe local and Docker paths; local behavior is intended to stay byte-identical.Fail-closed behavior: tasks without
docker_imageerror onreset(host subprocess fallback removed).TB2_MODE=autoand unknown modes error at startup. Containerexecuses argv-formbash -cso agent commands with quotes are not mangled. Failed resetsclose()the prior container so stale sessions cannot mix tasks.Docs note the stricter Docker contract. Unit tests cover canonical evaluate, workdir, imageless reset, and exec quoting.
Reviewed by Cursor Bugbot for commit 4180b7c. Bugbot is set up for automated code reviews on this repo. Configure here.