openenv: TB2 agentic RL adapter + GLM-4.7-Flash launcher#1487
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces experimental integration with OpenEnv, adding dataset generation scripts, concurrent environment servers, and agent adapters for both Phase-1 (Echo smoke test) and Phase-2 (Coding learning run) setups. The feedback highlights critical improvements to prevent potential crashes and resource leaks: specifically, using AsyncOpenAI as an asynchronous context manager to avoid unclosed connections, guarding against empty completion choices to prevent IndexError, and applying safe fallbacks for sample metadata to avoid AttributeError crashes.
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.
6f576df to
777fe2a
Compare
The per-episode client opened in run() was never closed on any path (success, timeout, or exception), leaking httpx connections/sessions under concurrent rollout. Close it in a finally so every path releases it.
The single-node TB2 example only ever drives the env server over --openenv-env-url; the optional Daytona sandbox pool was never exercised here. Remove the pool (adapter class + launch plumbing + launcher fields) and its docstrings so the example carries only the code path it uses.
…tling The WebSocket env server holds an FD per live session + Docker connection and leaks sockets on unclean disconnects, so a long run under the default 1024 soft nofile exhausts it and the accept loop fails every connection with EMFILE, silently throttling rollouts. Document `ulimit -n` in the start step and the leakage note.
4889fe3 to
3e97c8b
Compare
nblintao
left a comment
There was a problem hiding this comment.
LGTM. Some nits regarding the reward calculation but we can refine that in the future when we have bigger runs.
| return float(raw) if raw else 0.0 | ||
| except ValueError: | ||
| return 0.0 | ||
| return 0.0 |
There was a problem hiding this comment.
When we can't find reward, it's likely an infra issues. I think we should return None to throw away the sample instead of introducing false zero.
| # fails to run (e.g. in a reused sandbox where /logs survives across episodes). | ||
| "mkdir -p /tests /logs/verifier && rm -f /logs/verifier/reward.txt && " | ||
| f"cp -a {_TB2_TESTS_SRC}/. /tests/ 2>/dev/null || true; " | ||
| f"{_EVAL_CD_CMD}bash /tests/test.sh > /tmp/tb2_testsh.log 2>&1; " |
There was a problem hiding this comment.
Maybe we should store the exist code of test.sh too to help differientiate infra error.
Per nblintao's review: when the canonical eval produces no reward marker, an empty value (reward.txt absent), or a non-numeric value, the harness never produced a verdict -- an infra/harness failure, not a task the agent legitimately failed. Scoring it 0.0 injects a false negative into the training signal. _parse_reward_marker now returns None in those cases; run() propagates it by returning None so the sample is dropped (the same drop signal already used on episode exceptions). A genuine failure writes reward.txt = 0 and is still returned as 0.0.
Per nblintao's second review comment: capture test.sh's exit code and surface it when a sample is dropped for having no recoverable reward, so the drop can be triaged -- a nonzero rc points at a test.sh crash (infra/harness failure) vs. a clean run that wrote no verdict. The eval exec now echoes $? on a __TB2_TESTSH_RC__ marker right after test.sh; _parse_testsh_rc reads it back and the value is threaded through _multi_turn into agent_metrics and into the drop-path warning. Diagnostic only: the exit code does NOT drive the drop decision. A nonzero rc from merely-failing tests is a legitimate reward 0, not an infra error, so keying the drop on rc would be wrong; the existing "no recoverable reward" condition remains the sole drop trigger.
Summary
Adds a self-contained example for driving the HuggingFace OpenEnv Terminal-Bench-2 (tbench2) environment from miles GRPO training. A generic miles-side adapter runs the multi-turn agentic loop against an unmodified upstream OpenEnv env server (used as a dumb docker-exec transport); all TB2 fidelity lives in the adapter. Tokens / logprobs / loss-masks are captured natively through the session server, and the env-computed binary pytest reward is fed back into the policy.
Everything lives under
examples/experimental/openenv/; this PR carries no core-miles changes. The one generic hook it originally bundled (surfacing agent-reported tool time onSample.non_generation_time) has been peeled out into its own standalone PR (#1663), and this PR is stacked on top of it.Adapter-driven fidelity
miles is env-agnostic, so something must own the per-env translation (action schema, multi-turn loop, where the reward lives). Rather than forking/vendoring OpenEnv, that logic lives entirely in the miles-side adapter:
/app, viaOPENENV_TASK_WORKDIR), matching the real TB2 image contract (upstream OpenEnv otherwise defaults to/task).tests/test.shis run to produce the binary pytest reward.OPENENV_DAYTONA_*), chosen purely by env vars.Multi-turn loop
reset(task_id)→ { policy emits one shell command in a single```bashblock →step(exec)→ feed output back } untilTASK_COMPLETEorOPENENV_MAX_TURNS→evaluate→ binary reward (1.0 = all tests pass, else 0.0). Each episode is wall-clock bounded byOPENENV_MAX_ROLLOUT_TIME_SECONDS; a straggler that exceeds it is terminated and scored 0 so it can't stall the whole rollout batch.Files
openenv_agent_function.py— the generic TB2 adapter: the multi-turn agentic loop, the WebSocket env transport (handles capacity/close semantics), and the optional Daytona sandbox pool. Wired via--custom-agent-function-path openenv_agent_function.run.openenv_generate.py— the--custom-rm-pathreward fn; task-agnostic, reads the env reward the adapter stored inmetadata["reward"].openenv_launch_common.py— shared, model-agnostic launch fragments (process cleanup, GRPO / optimizer / rollout / agent flags, W&B + Prometheus wiring, OpenEnv/Daytona env-var plumbing) so per-model launchers can't silently drift.run-openenv-tbench2.py— the GLM-4.7-Flash GRPO launcher; sets theOPENENV_*env vars and the GLM serving/training profile, delegating the shared flags toopenenv_launch_common.make_tbench2_data.py— builds the prompt set from aterminal-bench-2checkout: one row per discoveredtask_id(system prompt +metadata["task_id"]); the env serves the per-task instruction atreset().The generic
miles/rollout/generate_hub/agentic_tool_call.pythroughput-accounting hook this example relies on lives in its own base PR #1663 (this PR stacks on it). The DeepSeek-V4-Flash launcher variant is split into a stacked follow-up: #1662.How to run
Validation notes
reset(task_id)→exec→evaluatereturns the real binary reward (confirmed both1.0and0.0).run-openenv-tbench2.pyagainst an OpenEnv agent server: rollouts drive the full multi-turn loop and produce env-computedrollout/raw_reward; GRPO steps advance.Operational caveat: sandbox container leakage
The real-docker path relies on unmodified upstream OpenEnv for the sandbox lifecycle, and upstream currently leaks containers on unclean disconnects:
remove=Falseand only tears it down (container.stop()+remove(force=True)) inside itsclose()handler, i.e. on a clean session end.session_timeoutisNone, no env var wires it on), so nothing sweeps sessions whose WebSocket dropped without a clean close.The adapter side bounds live episodes (
OPENENV_MAX_ROLLOUT_TIME_SECONDS, per-message timeout, max turns) and closes the env viaasync with, so a healthy run self-cleans; the leak is strictly a crashed-session residue. Until upstream enables the reaper (or we set asession_timeout), sweep orphaned containers between runs on the env host, e.g.docker rm -fof any TB2 task-image container older than the episode wall-cap (2h is a safe threshold since the cap is 3600s).Test plan
reset(task_id)→exec→evaluatereturns 1.0 / 0.0.run-openenv-tbench2.pyreaches GRPO steps withrollout/raw_rewardpopulated from the env.