From 58a6c2fdfe01c2e6321ff9e9374a1eea0c67e380 Mon Sep 17 00:00:00 2001 From: Salma Date: Fri, 23 Oct 2026 02:09:13 +0530 Subject: [PATCH 1/2] fix(verifier): clear result files before running the container verifier In reward mode the step read reward_path from the task container after the verifier ran, without removing it first. A reward file the agent wrote during its turn was graded as the verifier's when test.sh exited without writing one (set -e, or 'pytest && echo 1 > reward.txt'): score 1.0 for a failed run. Remove every result path inside the container before the main command, and correct the comment that said a non-zero exit is recorded as 0. Closes #305 Co-Authored-By: Claude Opus 5.5 --- .../run_container_unit_tests_verifier.py | 19 ++++- .../run_container_unit_tests_verifier_test.py | 77 +++++++++++++++++++ 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py index bc092db1..ca907e58 100644 --- a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py +++ b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py @@ -276,7 +276,17 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: f"{shlex.quote(sandbox.scoped_name(self.container_name))} bash -c {shlex.quote(setup_cmd)}" ) - # 4. Run main command, bounded by in-container timeout(1) + # 4. Clear the result files. The agent worked in this container and could have left + # a reward file there; anything extracted below must come from this verifier run. + # `-w /` resolves relative paths the way `docker cp` does. + if self.result_paths: + await sandbox.exec_script( + f"docker exec -u root -w / " + f"{shlex.quote(sandbox.scoped_name(self.container_name))} " + f"rm -f -- {' '.join(shlex.quote(p) for p in self.result_paths)}" + ) + + # 5. Run main command, bounded by in-container timeout(1) logger.info( f"[{self.verifier_id}] running verifier (timeout={self.timeout_sec}s) " f"in container '{self.container_name}': {command[:160]}" @@ -297,7 +307,7 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: if stderr: logger.info(f"[{self.verifier_id}] stderr:\n{stderr}") - # 5. Extract result files (docker cp + cat + try JSON parse) + # 6. Extract result files (docker cp + cat + try JSON parse) extracted_files: dict[str, Any] = {} for result_path in self.result_paths: try: @@ -385,8 +395,9 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: } # 9. Raise on timeout / non-zero exit AFTER recording. In reward mode a - # non-zero exit is a graded failure (already recorded as score 0), not a - # step error — only an unfinished (timed-out) run is unreliable. + # non-zero exit is graded by the reward file this run wrote (score 0 when + # it wrote none), not a step error — only an unfinished (timed-out) run is + # unreliable. if timed_out: raise RuntimeError( f"Verifier '{self.verifier_id}' timed out after {self.timeout_sec}s" diff --git a/tst/unit/task_step/verifiers/run_container_unit_tests_verifier_test.py b/tst/unit/task_step/verifiers/run_container_unit_tests_verifier_test.py index 2368be11..048433c1 100644 --- a/tst/unit/task_step/verifiers/run_container_unit_tests_verifier_test.py +++ b/tst/unit/task_step/verifiers/run_container_unit_tests_verifier_test.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio +import shlex from types import SimpleNamespace import pytest @@ -294,3 +295,79 @@ async def both(): asyncio.run(both()) assert len({artifact_id for artifact_id, _ in uploads}) == len({url for _, url in uploads}) == 4 + + +class _ContainerSandbox(_Sandbox): + """The task container's files as a dict: `rm -f` and `docker cp` act on it, and the + verifier command runs `test_sh`, which may or may not write the reward file.""" + + def __init__(self, files, test_sh): + self.files = dict(files) + self.vm_files: dict[str, str] = {} + self.test_sh = test_sh + + async def exec_script(self, script: str) -> str: + words = shlex.split(script) + if words[0] == "docker" and "rm" in words: + for path in words[words.index("--") + 1:]: + self.files.pop(path, None) + elif words[:2] == ["rm", "-f"]: + self.vm_files.pop(words[2], None) + return "" + + async def exec_with_output(self, *args) -> tuple[int, str, str]: + if args[:2] == ("sudo", "cat"): + return 0, self.vm_files[args[2]], "" + return self.test_sh(self.files), "", "" + + async def docker_cp(self, source: str, destination: str) -> None: + path = source.split(":", 1)[1] + if path not in self.files: + raise RuntimeError(f"no such file: {path}") + self.vm_files[destination] = self.files[path] + + +def _test_sh_writing_reward_only_on_pass(answer): + """`pytest && echo 1 > reward.txt`: a failed run exits non-zero and writes nothing.""" + + def run(files): + if files.get("/app/answer.txt") == answer: + files["/logs/verifier/reward.txt"] = "1\n" + return 0 + return 1 + + return run + + +@pytest.mark.parametrize( + ("agent_files", "expected_score"), + [ + ({"/app/answer.txt": "42"}, 1.0), + ({"/app/answer.txt": "41"}, 0.0), + ({"/app/answer.txt": "41", "/logs/verifier/reward.txt": "1\n"}, 0.0), + ], + ids=["correct", "wrong", "wrong-with-planted-reward"], +) +def test_reward_comes_from_this_verifier_run(local_stores, monkeypatch, agent_files, expected_score): + """A reward file the agent left in the container must not be graded as the verifier's.""" + configure() + sandbox = _ContainerSandbox(agent_files, _test_sh_writing_reward_only_on_pass("42")) + provider = SimpleNamespace(get_sandbox=lambda sandbox_id: asyncio.sleep(0, result=sandbox)) + monkeypatch.setattr(sandbox_provider, "get_sandbox_provider", lambda: provider) + monkeypatch.setattr( + Step, + "_upload_text_artifact", + staticmethod(lambda text, artifact_id, description, object_url: SimpleNamespace( + id=artifact_id, version=1, object_url=object_url + )), + ) + ctx = TaskStepContext() + ctx.metadata["deployed_docker_containers"] = [{"container_name": "c", "sandbox_name": "h"}] + ctx.deployed_sandboxes.append(DeployedSandbox(sandbox_name="h", sandbox_id="sb-1", sandbox_mode="vm")) + + step = _step(command="bash /tests/test.sh", reward_path="/logs/verifier/reward.txt") + asyncio.run(step.execute(ctx)) + + [recorded] = ctx.metadata["verifications"].values() + assert recorded["score"] == expected_score + assert recorded["results"][0]["result"] is (expected_score == 1.0) From a3b73652cdb2e4c98a246ed451dbd73c06b9086f Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 22 Oct 2026 17:00:31 -0700 Subject: [PATCH 2/2] fix(verifier): clear only the reward file, as uid 0 Only the reward file is read for the score, so it is the one cleared before the command runs. Other result files stay as they were: a step can extract what an earlier step's job wrote, as the supervisor steps that adopt a running engine do, and clearing them made those steps extract nothing. The rm runs as uid 0, which needs no passwd entry, and the comment no longer claims more than the clearing gives: a process the agent left running can still write the file. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../run_container_unit_tests_verifier.py | 13 ++--- .../task_step/test_verify_sandbox_local.py | 51 +++++++++++++++++++ .../run_container_unit_tests_verifier_test.py | 49 ++++++++++++------ 3 files changed, 92 insertions(+), 21 deletions(-) diff --git a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py index ca907e58..c0e7b7d0 100644 --- a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py +++ b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py @@ -276,14 +276,15 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: f"{shlex.quote(sandbox.scoped_name(self.container_name))} bash -c {shlex.quote(setup_cmd)}" ) - # 4. Clear the result files. The agent worked in this container and could have left - # a reward file there; anything extracted below must come from this verifier run. - # `-w /` resolves relative paths the way `docker cp` does. - if self.result_paths: + # 4. Clear the reward file the score is read from, so one the agent left in this container + # isn't graded; a process the agent left running can still write it. Other result files + # stay: a step can extract what an earlier step's job wrote. `-w /` resolves a relative + # path as `docker cp` does, and uid 0 needs no passwd entry. + if self.reward_path: await sandbox.exec_script( - f"docker exec -u root -w / " + f"docker exec -u 0 -w / " f"{shlex.quote(sandbox.scoped_name(self.container_name))} " - f"rm -f -- {' '.join(shlex.quote(p) for p in self.result_paths)}" + f"rm -f -- {shlex.quote(self.reward_path)}" ) # 5. Run main command, bounded by in-container timeout(1) diff --git a/tst/integration/task_step/test_verify_sandbox_local.py b/tst/integration/task_step/test_verify_sandbox_local.py index 93a5ea79..b64d5d46 100644 --- a/tst/integration/task_step/test_verify_sandbox_local.py +++ b/tst/integration/task_step/test_verify_sandbox_local.py @@ -242,6 +242,57 @@ async def test_the_unit_tests_verifier_runs_in_a_local_container(local_agent): assert (entry["exit_code"], entry["extracted_files"]) == (0, {"/logs/reward.json": {"reward": 1}}) +def _reward_step(**options): + return RunContainerUnitTestsVerifierTaskStep( + id="verify", version=None, sandbox_name="box", container_name="agent", reward_path="/logs/verifier/reward.txt", + **options, + ) + + +@pytest.mark.asyncio +async def test_a_reward_the_agent_left_in_the_container_is_not_graded(local_agent): + sandbox, context = local_agent + _as_deployed_container(sandbox, context) + _write_in_container(sandbox, "/logs/verifier/reward.txt", "1") + step = _reward_step(command="test -f /app/answer.txt && echo 1 > /logs/verifier/reward.txt") + + entry = (await step.execute(context)).metadata["verifications"]["verify"] + + assert (entry["exit_code"], entry["score"]) == (1, 0.0) + assert entry["extracted_files"] == {"/logs/verifier/reward.txt": None} + + +@pytest.mark.asyncio +async def test_a_result_file_an_earlier_steps_job_wrote_is_extracted_as_it_is(local_agent): + """As a supervisor step that adopts an engine an earlier step started: the engine wrote the file, not this step.""" + sandbox, context = local_agent + _as_deployed_container(sandbox, context) + _write_in_container(sandbox, "/work/events.tail.json", "engine-tail") + step = RunContainerUnitTestsVerifierTaskStep( + id="verify", version=None, sandbox_name="box", container_name="agent", command="true", + result_paths=["/work/events.tail.json"], + ) + + entry = (await step.execute(context)).metadata["verifications"]["verify"] + + assert entry["extracted_files"] == {"/work/events.tail.json": "engine-tail\n"} + + +@pytest.mark.asyncio +async def test_the_reward_is_cleared_in_a_container_with_no_root_user_name(local_agent): + """A minimal image may have no passwd entry for root; its verifier runs as uid 0.""" + sandbox, context = local_agent + _as_deployed_container(sandbox, context) + _write_in_container(sandbox, "/logs/verifier/reward.txt", "1") + subprocess.run(["docker", "exec", sandbox.container_name, "sed", "-i", "/^root:/d", "/etc/passwd"], + check=True, capture_output=True) + step = _reward_step(command="exit 1", user="0") + + entry = (await step.execute(context)).metadata["verifications"]["verify"] + + assert (entry["score"], entry["extracted_files"]) == (0.0, {"/logs/verifier/reward.txt": None}) + + def _as_deployed_container(sandbox, context): """Record the agent's container as a ``run_docker_container`` container on the deployed sandbox ``box``: it is the sandbox's own ``agent``, as a step container is its own name.""" diff --git a/tst/unit/task_step/verifiers/run_container_unit_tests_verifier_test.py b/tst/unit/task_step/verifiers/run_container_unit_tests_verifier_test.py index 048433c1..6a6d362a 100644 --- a/tst/unit/task_step/verifiers/run_container_unit_tests_verifier_test.py +++ b/tst/unit/task_step/verifiers/run_container_unit_tests_verifier_test.py @@ -339,19 +339,9 @@ def run(files): return run -@pytest.mark.parametrize( - ("agent_files", "expected_score"), - [ - ({"/app/answer.txt": "42"}, 1.0), - ({"/app/answer.txt": "41"}, 0.0), - ({"/app/answer.txt": "41", "/logs/verifier/reward.txt": "1\n"}, 0.0), - ], - ids=["correct", "wrong", "wrong-with-planted-reward"], -) -def test_reward_comes_from_this_verifier_run(local_stores, monkeypatch, agent_files, expected_score): - """A reward file the agent left in the container must not be graded as the verifier's.""" +def _recorded(monkeypatch, sandbox, step): + """What ``step`` records after running against ``sandbox``.""" configure() - sandbox = _ContainerSandbox(agent_files, _test_sh_writing_reward_only_on_pass("42")) provider = SimpleNamespace(get_sandbox=lambda sandbox_id: asyncio.sleep(0, result=sandbox)) monkeypatch.setattr(sandbox_provider, "get_sandbox_provider", lambda: provider) monkeypatch.setattr( @@ -364,10 +354,39 @@ def test_reward_comes_from_this_verifier_run(local_stores, monkeypatch, agent_fi ctx = TaskStepContext() ctx.metadata["deployed_docker_containers"] = [{"container_name": "c", "sandbox_name": "h"}] ctx.deployed_sandboxes.append(DeployedSandbox(sandbox_name="h", sandbox_id="sb-1", sandbox_mode="vm")) - - step = _step(command="bash /tests/test.sh", reward_path="/logs/verifier/reward.txt") asyncio.run(step.execute(ctx)) - [recorded] = ctx.metadata["verifications"].values() + return recorded + + +@pytest.mark.parametrize( + ("agent_files", "expected_score"), + [ + ({"/app/answer.txt": "42"}, 1.0), + ({"/app/answer.txt": "41"}, 0.0), + ({"/app/answer.txt": "41", "/logs/verifier/reward.txt": "1\n"}, 0.0), + ], + ids=["correct", "wrong", "wrong-with-planted-reward"], +) +def test_reward_comes_from_this_verifier_run(local_stores, monkeypatch, agent_files, expected_score): + """A reward file the agent left in the container must not be graded as the verifier's.""" + sandbox = _ContainerSandbox(agent_files, _test_sh_writing_reward_only_on_pass("42")) + + recorded = _recorded(monkeypatch, sandbox, _step(command="bash /tests/test.sh", + reward_path="/logs/verifier/reward.txt")) + assert recorded["score"] == expected_score assert recorded["results"][0]["result"] is (expected_score == 1.0) + + +@pytest.mark.parametrize("reward_path", ["/logs/verifier/reward.txt", None], ids=["reward-mode", "exit-code-mode"]) +def test_only_the_reward_file_is_cleared(local_stores, monkeypatch, reward_path): + """A step can extract what an earlier step's job wrote, as a supervisor step adopting a running engine does.""" + files = {"/work/events.tail.json": '{"seq": 7}', "/logs/verifier/reward.txt": "1\n"} + sandbox = _ContainerSandbox(files, lambda files: 0) + + recorded = _recorded(monkeypatch, sandbox, _step(command="true", reward_path=reward_path, + result_paths=["/work/events.tail.json"])) + + assert recorded["extracted_files"]["/work/events.tail.json"] == {"seq": 7} + assert ("/logs/verifier/reward.txt" in sandbox.files) is (reward_path is None)