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..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,7 +276,18 @@ 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 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 0 -w / " + f"{shlex.quote(sandbox.scoped_name(self.container_name))} " + f"rm -f -- {shlex.quote(self.reward_path)}" + ) + + # 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 +308,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 +396,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/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 2368be11..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 @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio +import shlex from types import SimpleNamespace import pytest @@ -294,3 +295,98 @@ 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 + + +def _recorded(monkeypatch, sandbox, step): + """What ``step`` records after running against ``sandbox``.""" + configure() + 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")) + 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)