Skip to content

Commit 01f9dca

Browse files
committed
Run setup before the agent and skip verify on a non-zero agent exit
Addresses review: run_rollout ran setup after create() had already started the agent, so setup raced the agent over the workspace. create() now takes start_agent (default True, so the loop-owning training path is unchanged), the server passes start_agent=False, runs setup, then calls session.start_agent(), giving the correct setup then agent then verify order. A non-zero agent exit is now treated like a timeout, so a crashed agent no longer runs verify or earns a reward. AI-assisted.
1 parent bec6fe5 commit 01f9dca

3 files changed

Lines changed: 21 additions & 14 deletions

File tree

envs/pi_env/harness.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,16 @@ def create(
225225
task: Any,
226226
seed: int | None = None,
227227
episode_id: str | None = None,
228+
start_agent: bool = True,
228229
) -> PiSession:
229230
"""Create one session, retrying with exponential backoff.
230231
231232
Session creation spins up a sandbox, installs Pi, and starts the proxy +
232233
agent, the flakiest step in a rollout. Each failed attempt tears its own
233234
sandbox down (see :meth:`_create_once`), so a retry never leaks.
235+
236+
Pass ``start_agent=False`` to return before launching the agent (for
237+
example to run setup first), then call ``session.start_agent()``.
234238
"""
235239
import logging
236240
import time
@@ -239,7 +243,9 @@ def create(
239243
last_exc: Exception = RuntimeError("create_attempts must be >= 1")
240244
for i in range(self._create_attempts):
241245
try:
242-
return self._create_once(task, seed=seed, episode_id=episode_id)
246+
return self._create_once(
247+
task, seed=seed, episode_id=episode_id, start_agent=start_agent
248+
)
243249
except Exception as exc: # noqa: BLE001
244250
last_exc = exc
245251
if i + 1 < self._create_attempts:
@@ -256,6 +262,7 @@ def _create_once(
256262
task: Any,
257263
seed: int | None = None,
258264
episode_id: str | None = None,
265+
start_agent: bool = True,
259266
) -> PiSession:
260267
import logging
261268
_log = logging.getLogger(__name__)
@@ -312,7 +319,8 @@ def _create_once(
312319
proxy_trace_path=proxy_trace_path_str,
313320
proxy_bg_job=proxy_bg_job,
314321
)
315-
session.start_agent()
322+
if start_agent:
323+
session.start_agent()
316324
return session
317325
except Exception as exc:
318326
_log.error("factory.create: setup failed, killing sandbox: %r", exc)

envs/pi_env/server/pi_environment.py

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -331,20 +331,15 @@ def _emit(msg: str) -> None:
331331
f"creating HF sandbox (image={image or _DEFAULT_IMAGE}) — "
332332
"this is the slow phase (Node + Pi cold-install per rollout)"
333333
)
334-
session = factory.create(task=pi_task)
334+
session = factory.create(task=pi_task, start_agent=False)
335335
result.sandbox_id = session.sandbox.sandbox_id
336336
_emit(
337-
f"sandbox ready: {result.sandbox_id} — agent started "
337+
f"sandbox ready: {result.sandbox_id} "
338338
f"({'proxy on :7000, logprobs capturing' if mode == 'transparent_proxy' else 'direct LLM, no logprobs'})"
339339
)
340340

341-
# Run setup commands one at a time, *before* the agent starts.
342-
# The factory has already started the agent in start_agent()
343-
# during create(); to keep the order "setup → agent → verify"
344-
# we'd need to restructure. As a pragmatic compromise we run
345-
# setup IMMEDIATELY after create(), which races with the agent
346-
# for ~1-2s but is fine for typical pip/git/download work
347-
# because pi itself takes >=20s to make its first model call.
341+
# Run setup one command at a time, before the agent starts. create()
342+
# was called with start_agent=False, so the agent has not launched yet.
348343
for i, cmd in enumerate(setup, 1):
349344
_emit(f"setup [{i}/{len(setup)}]: {cmd[:80]}")
350345
cr = self._exec_command(session.sandbox, cmd, timeout=SETUP_TIMEOUT_S)
@@ -356,8 +351,9 @@ def _emit(msg: str) -> None:
356351
_emit(f"setup FAILED at [{i}]: exit={cr.exit_code}")
357352
break
358353

359-
# Block until the agent is done (or setup already failed).
354+
# Setup is done and clean, so launch the agent now (deferred from create()).
360355
if result.error is None:
356+
session.start_agent()
361357
_emit(
362358
f"agent running — pi CLI in sandbox "
363359
f"(timeout {int(agent_timeout_s)}s)"
@@ -367,6 +363,9 @@ def _emit(msg: str) -> None:
367363
timeout_s=agent_timeout_s
368364
)
369365
_emit(f"agent finished: exit_code={result.agent_exit_code}")
366+
if result.agent_exit_code != 0:
367+
result.error = f"agent exited non-zero ({result.agent_exit_code})"
368+
_emit(f"agent FAILED: exit_code={result.agent_exit_code}")
370369
except TimeoutError as exc:
371370
result.error = f"agent timeout: {exc}"
372371
_emit(f"agent TIMEOUT: {exc}")

tests/envs/test_pi_factory_lifecycle.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def test_create_retries_then_succeeds(monkeypatch):
8080
calls = {"n": 0}
8181
session = object()
8282

83-
def _flaky(task, seed=None, episode_id=None):
83+
def _flaky(task, seed=None, episode_id=None, start_agent=True):
8484
calls["n"] += 1
8585
if calls["n"] < 3:
8686
raise RuntimeError("transient create failure")
@@ -97,7 +97,7 @@ def test_create_raises_after_exhausting_attempts(monkeypatch):
9797
factory = _factory(sandbox, create_attempts=3, create_backoff_s=0)
9898
calls = {"n": 0}
9999

100-
def _always_fails(task, seed=None, episode_id=None):
100+
def _always_fails(task, seed=None, episode_id=None, start_agent=True):
101101
calls["n"] += 1
102102
raise RuntimeError("persistent create failure")
103103

0 commit comments

Comments
 (0)